fix(paths): terminate rootDir inference at volume roots and mirror tsc pattern precedence#314
Conversation
…c pattern precedence Three defects in the paths rewriter's path/pattern math, found while root-causing #310: - commonSourceDir spun forever at a Windows volume root: filepath.Dir("C:/") hands back the backslash form ("C:\") while the termination guard compared it against the slash-normalized cursor ("C:/"), re-normalizing the same directory endlessly. A tsconfig `files` list spanning two volumes always shrinks the shared prefix to a root, so every such project hung the sidecar for the full 10-minute test timeout (and orphaned plugin processes when the parent was killed). The walk is now the same per-component intersection as TypeScript-Go's computeCommonSourceDirectoryOfFilenames -- canonical comparison under the host's case sensitivity, "" when inputs share no root -- and the no-rootDir fallback follows tsgo's full GetCommonSourceDirectory chain: tsconfig directory first, computed common directory only for config-less programs. That anchors output mapping exactly where tsgo anchors its own emit. - matchPattern panicked on overlapping literal halves: "@lib/x" satisfies both the prefix and suffix probes of "@lib/x*x", and slicing the star capture panicked on inverted bounds. tsc's isPatternMatch requires candidate.length >= prefix.length + suffix.length; the same guard applies now, plus tsc's rule that a pattern with two wildcards is no pattern at all. (TypeScript-Go's own core.Pattern.MatchedText has the identical overlap panic in module resolution -- upstream, out of reach from ttsc.) - Pattern precedence ranked by total literal length instead of tsc's matchPatternOrExact order (exact first, then longest literal prefix), so "@app/foo-styles" was steered at "*-styles" while the type checker resolved it through "@app/*". orderPatterns now sorts exact-first / prefix-length, stable on ties like tsc's first-longest-prefix-wins scan. The cross-volume e2e case runs the real sidecar under a 2-minute deadline and skips on single-volume machines; windows-latest runners (repo on D:, TEMP on C:) exercise it for real. Docs updated to the new rootDir anchoring and precedence. Resolves #310. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AivFfPoUxoU26Dko5Jbm21
There was a problem hiding this comment.
Pull request overview
This PR fixes a Windows hang in @ttsc/paths rootDir inference when a tsconfig files list spans multiple volumes, and aligns the plugin’s path-pattern matching/ordering with tsc/TypeScript-Go behavior to prevent panics and resolution mismatches.
Changes:
- Replaces
rootDirinference with a TypeScript-Go–parity implementation that terminates at volume roots and returns""for cross-volume inputs (avoiding infinite loops). - Hardens wildcard matching to reject multi-
*patterns and prevent inverted-slice panics for overlapping prefix/suffix matches. - Updates paths pattern precedence to match tsc (
exactfirst, then longest literal prefix), and adds/updates tests and docs accordingly.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| website/src/content/docs/plugins/paths.mdx | Documents updated rootDir inference anchoring and tsc precedence. |
| website/src/content/docs/development/walkthroughs/paths.mdx | Walkthrough updated to reflect new inference and precedence logic. |
| packages/paths/README.md | README updated to reflect new rootDir default behavior. |
| packages/paths/driver/paths.go | Implements inferred rootDir/common source dir parity, wildcard guards, and tsc-style pattern ordering. |
| packages/paths/test/command_rewrites_all_module_specifier_forms_test.go | Updates fixture commentary to reference the new dedicated cross-volume regression test. |
| packages/paths/test/command_check_completes_on_cross_volume_files_list_test.go | Adds end-to-end regression test ensuring check completes on cross-volume inputs. |
| packages/paths/test/linkname_helpers_test.go | Updates linkname helpers for the renamed/added driver helpers. |
| packages/paths/test/rewriter_helpers_cover_resolution_edges_test.go | Updates helper-coverage assertions for the new helper surface/signatures. |
| packages/paths/test/rewriter_common_source_dir_terminates_at_volume_roots_test.go | Adds unit test coverage for termination and cross-volume behavior. |
| packages/paths/test/rewriter_match_pattern_rejects_overlapping_prefix_suffix_test.go | Adds tests for overlap/multi-wildcard rejection (panic prevention). |
| packages/paths/test/rewriter_resolve_source_prefers_longest_prefix_pattern_test.go | Adds tests pinning tsc precedence (exact-first, longest-prefix wildcard). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…closure Self-review follow-ups on the same rewriter, both aligning it further with its oracles: - resolveSource fell through to weaker patterns when the best-precedence match's targets named no program source, rewriting the import at a module tsc never resolved. tsc's tryLoadModuleUsingPaths commits to the single matched pattern -- when its substitutions fail, paths resolution fails -- so the rewriter now stops there and leaves the specifier untouched. - visitModuleSpecifiers handed ForEachChild a fresh closure at every node, one allocation per AST node per apply pass across the whole program. The recursion now runs through one closure per walk, the same discipline linthost's walkDescendants documents for exactly this reason. Refs #310. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AivFfPoUxoU26Dko5Jbm21
|
Self-review round (multi-angle, per request) landed two more fixes in 98d229d:
Also verified end-to-end during the review: with Copilot's loop-variable comment is answered inline: Full paths suite green locally on Windows both rounds (two-volume machine, cross-volume e2e exercised for real). |
…patterns Third self-review pass, verified against the pinned typescript-go source: module.MatchPatternOrExact consults the exact-string set before any wildcard, and core.FindBestPatternMatch displaces the best match only on a strictly greater prefix length — so between wildcards with identical literal prefixes the first declared pattern wins. orderPatterns already reproduces this via SliceStable; this locks the observable contract so an unstable sort or a >= comparison cannot silently diverge from the type checker on order-sensitive configs. Refs #310. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AivFfPoUxoU26Dko5Jbm21
Intent
Resolve #310 at the root, and sweep the same driver for every adjacent defect the audit surfaced, per request, all in one PR.
Root cause of #310
The hang was never in project loading.
paths.go::commonSourceDir(the no-rootDirfallback) walked the shared prefix upward byte-by-byte and comparedfilepath.Dir's native result against a slash-normalized cursor to decide termination. At a Windows volume root,filepath.Dir("C:/")returns"C:\", which never equals"C:/", so the loop re-normalized the same directory forever — a pure CPU spin, which is also why orphanedplugin.exeprocesses kept burning CPU after their parents died. Afileslist spanning two volumes always shrinks the shared prefix to a root, so the repro hung deterministically; same-volume lists that share only the drive root (e.g.C:\a\x.ts+C:\b\y.ts) hung the same way.Verified on this machine (repo on D:, TEMP on C:): the issue repro hung for the full 45s kill deadline before the fix and exits 0 in ~1s after; the same fixture with all files on one volume was unaffected (~1s before and after).
Scope
All in
packages/paths(the audit found no other non-terminating walk in the tree; the banner/strip/lint upward walks compare in native form and terminate):commonSourceDiris now the same per-component intersection as TypeScript-Go'scomputeCommonSourceDirectoryOfFilenames(canonical comparison under host case sensitivity,""when inputs share no root), and the fallback follows tsgo's fullGetCommonSourceDirectorychain — tsconfig directory first, computed common directory only for config-less programs. Output mapping now anchors exactly where tsgo anchors emit. Note: tsc itself exits 0 on the issue's exact repro config (nooutDir), and so does the fixed sidecar; the TS5009/TS6059-class errors remain tsgo's to raise, and they now surface instead of the hang eating them."@lib/x"against pattern"@lib/x*x"satisfied both prefix and suffix probes and panicked on inverted slice bounds. tsc'sisPatternMatchlength guard applies now, and two-wildcard patterns are rejected the way tsc'sTryParsePatterndiscards them."@app/foo-styles"at"*-styles"while the type checker resolved it through"@app/*".orderPatternsnow followsmatchPatternOrExact: exact first, then longest literal prefix, stable on ties.Docs updated in the same change:
@ttsc/pathsREADME,plugins/paths.mdx, and thewalkthroughs/paths.mdxcode listings/prose that quoted the old implementation.Out of reach (documented, not fixed here)
core.Pattern.MatchedTexthas the identical inverted-bounds panic (internal/core/pattern.go:38;Matchescheckslen(candidate) >= StarIndexinstead of prefix+suffix). An overlapping paths pattern plus a short specifier crashes module resolution inside the checker pool — reproduced through the sidecar with a panic stack straight intomodule.(*resolutionState). It fires in tsgo goroutines ttsc cannot recover from and needs an upstream fix; the driver-level guard in this PR covers the positions tsgo's resolver never touches (e.g. ambientdeclare modulenames).go run/process trees, not of this bug; with the spin gone, the observed leak vector is closed.Test plan
rewriter_common_source_dir_terminates_at_volume_roots_test.go— pure intersection contract: volume-root stop, cross-volume"", POSIX roots, case folding, plus a 30s watchdog so a regression fails instead of spinning.command_check_completes_on_cross_volume_files_list_test.go— the real [Bug] project check hangs (~10min timeout) when the tsconfigfileslist spans two Windows volumes #310 repro through the sidecar under a 2-minute deadline; skips on single-volume machines, runs for real on windows-latest (repo D:, TEMP C:) and on this dev box.rewriter_match_pattern_rejects_overlapping_prefix_suffix_test.go— overlap and two-wildcard guards (the overlap case panicked before the fix).rewriter_resolve_source_prefers_longest_prefix_pattern_test.go— exact-over-wildcard and longest-prefix-over-longest-suffix resolution.rewriter_helpers_cover_resolution_edges_test.go/linkname_helpers_test.gofor the new helper surface (existingpatternRankpin replaced by the tsc-precedence helpers — deliberate, called out here).packages/pathssuite green locally on Windows (21s), including all pre-existing command tests.🤖 Generated with Claude Code
https://claude.ai/code/session_01AivFfPoUxoU26Dko5Jbm21