Uh oh!
There was an error while loading. Please reload this page.
tooling(pm): take node:path as a namespace in dispatch-gates so no fixture can shadow it - #14988
Merged
Merged
Conversation
…xture can shadow it
`selfTest()` binds fixture values to two names the module imports from
`node:path` — `const relative = [...].join('\n')` (function scope, so it
shadows for the whole battery) and `const resolve = () => 'pnpm check:x'`
(one nested block). A later case reaching for either gets the fixture, not
the function, and dies at run time with `TypeError: relative is not a
function`. The message names a real binding, so it reads like a broken
import rather than a shadow; parse-time checks cannot see it at all.
The module now takes one namespace binding instead of five bare ones, so
there is no module-scope `node:path` name left for a fixture to shadow —
the class closes rather than today's two instances. The namespace is spelled
`nodePath`, NOT `path`: this file already binds `path` locally 16 times (3
block consts, 13 parameters), and one of them, `readTrackedSource(path)`,
calls `join(ROOT, path)` — under a `path` namespace that line would become
`path.join(ROOT, path)` on a string parameter, re-creating the very defect
being closed. `nodePath` is bound nowhere in the tree.
Binding names only. Stripping the inserted `nodePath.` prefix from the
result reproduces the previous file byte for byte except the import line, so
no assertion, fixture value, string or comment moved; the battery's verdict
is identical before and after.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfoxos-steve
marked this pull request as ready for review
September 3, 2026 16:50
os-steve
enabled auto-merge
September 3, 2026 16:50
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Fixes#14688
selfTest()binds fixture values to two names this module imports fromnode:path, so a later case reaching for either gets the fixture instead of the function and dies at run time. Parse-time checks cannot see it; the run-time message names a real binding, so it reads like a broken import rather than a shadow.What changed
The module took five bare names from
node:path. It now takes one namespace binding, and the 149 genuine call sites are spelled through it. Nothing else moved — this is a binding-name change, as the triage scope ruling requires.The namespace is spelled
nodePath, notpath— measured, not stylisticThe obvious spelling is refuted by this file. Measured on the merged tree with a scope-resolving AST pass over every identifier reference (not a grep — the file is full of
resolve(/join(inside fixture strings and prose):pathnodePathThis file already binds
pathlocally 16 times — 3 block consts and 13 parameters. One of them is load-bearing:Under a
pathnamespace that line becomespath.join(ROOT, path), calling.joinon a string parameter — re-creating the exact defect this PR closes, in the same file, on the same day.nodePathoccurs nowhere inscripts/.Why the namespace rather than renaming the two fixtures
Renaming the fixtures fixes today's two instances. The namespace removes the possibility: after this change zero module-scope
node:pathbindings remain, so there is nothing left for a fixture to shadow. The AST pass confirms the end state — the only surviving bare references to the five names are the 3 deliberate fixture uses, each binding to its own local fixture.There is a second, structural gain. The two shadowed names were dangerous precisely because no existing case inside
selfTest()called them, so the shadow armed a trap for a future case instead of breaking a present one.nodePathis called ~100 times insideselfTest()itself, so any future binding of that name fails immediately, at the moment it is written, rather than lying in wait.The diff is mechanically reviewable
Every changed line is a prefix insertion. Stripping the inserted prefix from the result reproduces the previous file byte for byte except the import line:
That single hunk is the whole semantic content of a 140-line diff. No assertion, fixture value, string literal or comment moved; the file is the same 17,974 lines.
Verification
The battery is the verdict, and it is identical.
pnpm check:pm-dispatch-gatesbefore and after, both underscripts/pm/os-verify-lock.sh:Stronger than the count: all 1,289 streamed case lines diff byte-identical between the two runs, 0 failures on each side.
Reverse verification, both legs injected at the same program point (immediately after the fixture binding), each mutation proven on disk by occurrence count and object hash, each restored to the
HEADblob withgit diff HEADempty:relative(ROOT, ROOT)TypeError: relative is not a functionnodePath.relative(ROOT, ROOT)"", andtypeof relativeis stillstringLeg B's second reading is the point worth stating plainly: the fixture is untouched and the bare name is still a string. The defect is closed by removing the module binding that could be shadowed, not by making the bare spelling work.
node --checkpasses on both trees and is not offered as evidence — by construction it cannot see this defect class.Gates. Re-derived after the final commit from the actual change set (
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, no paths): 20 runnable families. 19 ran green;check-test-completeness.mjsexits 3, which is its own NOT MEASURED code — it grades a savedturbo run testlog and states "⛔ It is not a red, and there is nothing here to fix."ESLint narrowed to the changed file, with the invariance evidence: eslint's own
--print-configreports 2 active rules for this file, neither type-aware;--format jsonreports 1 file linted, 0 errors, 0 warnings. Noproject/projectServiceis configured anywhere ineslint.config.mjs, so this diff cannot move the verdict of any file it does not touch.Scope
Binding names only, one file. Out of scope and untouched, as the dispatch fences them: the
--tier --residuerefusal table (#14753) and the derivation coverage work (#14880) both remain open and queued behind this. #14672 stays exactly as merged — itsROOT-derived workaround is left in place and is still correct; only the comment above it now describes a hazard that no longer exists, which is a follow-up rather than a rider here.skip-changeset:scripts/pm/**publishes nothing from any package.🤖 Generated with Claude Code
Generated by Claude Code
Generated by Claude Code