Skip to content

Hermetic ambient walks, node:* declaration imports, and a warning-free build - #1310

Merged
nickna merged 3 commits into
mainfrom
fix/ambient-walk-hermeticity
Jul 28, 2026
Merged

Hermetic ambient walks, node:* declaration imports, and a warning-free build#1310
nickna merged 3 commits into
mainfrom
fix/ambient-walk-hermeticity

Conversation

@nickna

@nickna nickna commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Three independent fixes that shipped on this branch together.

Stop ambient upward walks at the temp and user-profile ceilings

FileDiscovery already refused to let a stray tsconfig.json in %TEMP% reconfigure whatever compiles below it, but six other upward walks never got that policy: ModuleResolver''s node_modules probe, subpath-import and self-reference package.json walks, and @types roots; CommonJsDetector''s package.json type walk; and TsConfigLoader''s bare extends walk. Any of them could pick up debris that unrelated tooling left in the global temp folder or the user profile.

The ceiling policy is now shared through FileDiscovery.AmbientParent: the start directory is still probed even when it is itself a ceiling, but no walk ascends into one. Validated by running the full suite with a hostile %TEMP% (a commonjs package.json, a strict tsconfig.json, a sharpts.json, and node_modules/@types/node all present).

Bind declaration-file node:* imports ambiently, not as files

The declaration-file facade guard skipped a node:* import only when the stdlib chain could serve the bare name, so @types/node''s own web-globals/console.d.tsimport * as console from "node:console", a module SharpTS''s stdlib does not provide — fell through to bare-specifier file resolution and threw. The CLI path hid this by deferring resolution failures to the checker; embedding callers using TypeScriptProgramOptions.Disabled crashed outright.

A node:* specifier in a declaration file can only mean an ambient module declaration or the stdlib facade, never a loadable source file, so the guard now skips every node:* import unconditionally. Regression test mirrors the real @types/node shape (verified against @types/node 25.9.1 on disk).

Clear all build warnings (11 → 0)

Two were substantive:

  • CS8604 in ILCompiler.RegisterModuleDocuments — the debug-scope table indexed a "normalized spelling" that NormalizeToEmissionPath can never produce: it either returns the path unchanged or folds a script path to null, so the insert was a duplicate key write today and a null-key ArgumentNullException if the phases ever reorder. Registration now keys by raw path only and documents that null-path emission is served by the entry-point fallback.
  • IDE0052 in MockHttpServer — the accept-loop task was assigned but never observed, so Dispose could tear down the listener and token source while the loop was still touching them during parallel test cleanup. Dispose now joins the loop (2s bound) before closing either.

The rest: removed never-called members (the Parser span-recording wrappers left over from the span-table work, an unused REPL-completion test helper overload, an unused diagnostic assertion helper) and switched DebugSymbolsTests to the Assert.Single predicate overload (xUnit2031).

Testing

  • Full suite: 16,093 passed, 0 failed
  • Build: 0 warnings, 0 errors
  • Hermeticity commit additionally validated against a hostile %TEMP% as described above

nickna added 3 commits July 27, 2026 20:56
FileDiscovery already refused to let a stray tsconfig.json in %TEMP%
reconfigure whatever happens to compile below it, but six other upward
walks never got that policy: ModuleResolver's node_modules probe,
subpath-import and self-reference package.json walks, and @types roots;
CommonJsDetector's package.json type walk; and TsConfigLoader's bare
"extends" walk. Any of them could pick up debris that unrelated tooling
left in the global temp folder or the user profile.

Share the ceiling policy through FileDiscovery.AmbientParent: the start
directory is still probed even when it is itself a ceiling, but no walk
ascends into one. Validated by running the full suite with a hostile
%TEMP% (a commonjs package.json, a strict tsconfig.json, a sharpts.json,
and node_modules/@types/node all present).
The declaration-file facade guard skipped a node:* import only when the
stdlib chain could serve the bare name, so @types/node's own
web-globals/console.d.ts — "import * as console from `node:console`", a
module SharpTS's stdlib does not provide — fell through to bare-specifier
file resolution and threw. The CLI path hid this by deferring resolution
failures to the checker; embedding callers using
TypeScriptProgramOptions.Disabled crashed outright.

A node:* specifier in a declaration file can only mean an ambient module
declaration or the stdlib facade, never a loadable source file, so the
guard now skips every node:* import unconditionally. Regression test
mirrors the real @types/node shape (verified against @types/node 25.9.1
on disk).
The debug-scope registration indexed a "normalized spelling" that
NormalizeToEmissionPath can never produce: it either returns the path
unchanged or folds a script path to null, so the insert was a duplicate
key write today and a null-key ArgumentNullException if the phases ever
reorder (CS8604). Registration now keys by raw path only and documents
that null-path emission is served by the entry-point fallback.

MockHttpServer assigned its accept-loop task but never observed it, so
Dispose could tear down the listener and token source while the loop was
still touching them during parallel test cleanup (IDE0052). Dispose now
joins the loop with a 2s bound before closing either.

The rest is removal of never-called members — the Parser span-recording
wrappers left over from the span-table work (transforms use SpanTable
directly), an unused REPL-completion test helper overload, an unused
diagnostic assertion helper — and the xUnit2031 Assert.Single predicate
overload in DebugSymbolsTests.
@nickna
nickna merged commit 7f37a0c into main Jul 28, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant