fix: make path checks and tool output cross-platform on Windows#32
Merged
Merged
Conversation
Three production bugs that broke on Windows because path separator handling assumed POSIX. CI is Linux-only so they were invisible there. 1. ingest.ts cloneRepo rejected every repo with `Unsafe clone path` because the safety guard checked `startsWith(`/`)` while `resolve()` produces `\` separators on Windows. Replaced with a path.relative-based helper that works on either platform. 2. web/routes.ts file-download endpoint returned 400 for every legitimate file for the same reason. Same fix. 3. tools.ts search shim `line.replace(repoPath + '/', '')` failed to strip the absolute repo path on Windows, leaking it into LLM-facing output. Switched to running ripgrep with cwd + relative target + --path-separator / so output is always relative forward-slash paths regardless of platform. list_files now also normalizes its displayed path to forward slashes for consistency with the tool description. Added two helpers in utils.ts: `isPathInsideDir` (resolves both paths internally to avoid misuse) and `toPosixPath` for display normalization, mirroring the pattern already used in impact.ts. Test fixes: 21 unit + 1 e2e test failed on Windows because they baked in POSIX-only assumptions. Switched literal `/repo/...` expectations to `path.resolve(...)` and normalized separators in fs mock predicates. The e2e helper now mirrors HOME to USERPROFILE on Windows so cache-redirection tests work cross-platform. All checks pass on Windows now: typecheck, lint, build, 965/965 unit tests, 9/9 CLI e2e tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Arthur742Ramos
added a commit
that referenced
this pull request
May 15, 2026
The previous PR (#32) fixed three production bugs that were invisible in CI because every job ran on ubuntu-latest. This adds Windows coverage so the same class of bug fails CI before it lands. New jobs: - test-windows: full matrix of Node 20, 22, 24 on windows-latest. Mirrors the existing ubuntu test matrix step-for-step except it skips the coverage upload (already produced by the ubuntu Node 20 leg). fail-fast: false so a single Node-version failure does not mask others. - e2e-windows: runs npm run test:e2e:cli only. The CLI e2e suite uncovered the HOME->USERPROFILE bug fixed in #32, so it is the highest-value Windows e2e to gate on. The Playwright-driven web e2e is left ubuntu-only because it has not been validated end-to-end on Windows in this change. Both new jobs are added to the build and sbom needs lists so a Windows failure blocks the rest of the pipeline. Lint, typecheck, dependency-review, build, and sbom remain ubuntu-only. They are platform-agnostic and adding Windows there would not catch additional bugs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Arthur742Ramos
added a commit
that referenced
this pull request
May 15, 2026
Today `bootcamp cache` lets you prune or clear the analysis cache, but there's no way to see what's actually in it. That makes hygiene blind ā you can't tell which repos/phases/SHAs are cached, how big the cache is, or whether a stale entry is from your last run or three months ago.
Adds `bootcamp cache list` (alias `ls`):
- Human-readable table: Repository, Phase, SHA (7-char), Age (relative: `2h`, `3d`), Size, Model, Style. Header shows cache dir + schema version; footer shows totals.
- `--json` flag for scripting: stable shape `{ dir, version, entries, totalEntries, totalBytes }`, always valid JSON (even when empty), preserves mtime-desc order with deterministic filename tiebreaker.
Surfaces problem entries instead of hiding them: legacy (older schema version), malformed (parse error or wrong shape), and unreadable (IO error). These are shown with `(legacy) filename.json` rows so users can see disk usage from stray files ā the exact case that today causes 'huh, my disk is full' surprises.
Implementation:
- `listCacheEntries()` in src/cache.ts: validates against the same compatibility rules as readPhaseCache (missing `generationOptions` is fine, normalized to empty strings) so readable v2 entries aren't falsely flagged as malformed. Per-file errors are isolated; the listing never fails because of a single bad file.
- Pure renderer in src/commands/cache-list.ts (`buildHumanOutput`/`buildJsonOutput`/`formatAge`/`formatBytes`) so unit tests can pin output without spawning a CLI process.
- `getCacheVersion()` exported to avoid duplicating the CACHE_VERSION constant in tests and CLI.
Tests: 20 new unit tests (in test/cache.test.ts and test/cache-list.test.ts) covering valid/legacy/malformed entries, missing generationOptions, mtime-desc sorting with filename tiebreaker, byte/age formatting, and JSON shape. 2 new e2e tests covering the populated-cache and empty-cache cases through the real CLI process (uses the existing HOME/USERPROFILE redirect from PR #32).
All checks pass on Windows: typecheck, lint, build, 985/985 unit, 11/11 CLI e2e.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three production bugs broke this package completely on Windows because path-separator handling assumed POSIX. CI is
ubuntu-latestonly, so they were invisible there. Found while runningnpm teston Windows ā 21 unit tests + 1 e2e test fail onmain.Production bugs fixed
src/ingest.ts:183clonePath.startsWith(${tempRoot}/)āresolve()returns\separatorsUnsafe clone pathsrc/web/routes.ts:447startsWith(prefix + "/")patternsrc/tools.ts:248line.replace(context.repoPath + "/", "")doesn't strip on WindowsPlus a polish fix:
list_filesnow normalizes displayed paths to forward slashes so they match what the tool description tells the LLM to use ('src/index.ts').Approach
src/utils.tshelpers:isPathInsideDir(parent, child)ā resolves inputs internally, usespath.relative(the same correct patternsafePathalready uses intools.ts).toPosixPath(p)ā for display normalization, mirroring whatsrc/impact.ts:74already does.src/tools.tssearchwas rewritten to invoke ripgrep withcwd: repoRoot+ a relative target +--path-separator /. This eliminates the path-stripping shim entirely and makes output deterministic on both platforms (rubber-duck suggested this approach over parsingpath:line:col:texttext output, which is fragile when paths contain:likeC:\ā¦).Test fixes
The 21 failing tests were all test-only assertion bugs ā production code correctly used
path.resolve/path.join, but the assertions baked in POSIX-only forms like"/repo/src/index.ts"andp.endsWith("/custom/file.md"). Switched them topath.resolve(...)/ normalized predicates so they work on either platform.The 1 e2e failure (
cache-command.e2e.test.ts) was caused by the test settingHOMEto redirect~/.cacheā but Node'sos.homedir()readsUSERPROFILEon Windows. Fixed in the sharedrunClihelper by mirroringHOMEāUSERPROFILEon Windows, so future tests get this for free.Validation on Windows
npm run typechecknpm run lintnpm testnpm run test:e2e:clinpm run buildLinux behavior is unchanged: the helpers and
path.relative-based check are semantically equivalent to the oldstartsWith(prefix + "/")when separators are/.Out of scope