fix(lint): resolve the durability gate's wrapper hop through the call site's lexical scope - #13784
Merged
os-project-manager merged 3 commits intoAug 31, 2026
Conversation
… site's lexical scope `indexFunctionBodies` built one flat `Map<name, body>` in source order, so a file that declared the same name more than once resolved EVERY call to that name to the LAST declaration, wherever the call was written. That is not an approximation with a direction: a collision where only some of the same-named bodies read either invents a read seam or drops a real one, and nothing in the output distinguishes either case from a correct resolution. The index now records every declaration with the scope it is visible from, and `get(name, from)` walks outward from the call site, innermost scope first. Two deliberate departures from a strict lexical resolver, both measured: - a name with exactly ONE body in the file resolves from anywhere. There is nothing to choose, and this keeps every hop the census depends on -- notably a wrapper reached on an identifier receiver, whose owning class is not on the caller's ancestor chain. - a name with SEVERAL bodies and no enclosing one is refused rather than guessed. File order is not evidence, and declining is the under-counting direction this family declares. `contradictsWrapperResolution` is unchanged: it answers a different question (collisions across receivers), both of its clauses pass correctly on the live instance, and it stays load-bearing for a unique name reached on a compound receiver. The sibling `indexLogAliases` stays flat and last-wins, and its docblock now says so with the measurement behind it: zero same-file collisions among the names that index holds, across the whole scan surface. Regression control: four fixtures in `selfTestReadSeams` pinning both error directions (dropped seam, invented seam), the live three-body `lookup` shape, and the non-narrowing rule that catches an over-fix. Measured on this tree: `--list` and `--depth-cost` are byte-identical before and after, at the shipped bound and at every probed depth up to 8. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
…osed
The header recorded the flat-index collision as an OPEN hazard, in the present
tense, pinned to line numbers that had already drifted by ~131 lines. It also
supplied the third of three reasons for leaving MAX_READ_WRAPPER_DEPTH alone
("the hazard grows with depth"), so closing the hazard silently would have left
a maintainer-reserved decision resting on an argument that no longer holds.
The note now says what was true, what closed it, and what survives of the depth
argument in its narrower form: a longer chain no longer needs every name in it
to be unique, only resolvable at its own call site -- but a name declared
exactly once is still answered by name alone from anywhere, so a longer chain
still crosses more of those. The other two reasons are untouched and the bound
is not moved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
…xical-scope-wrapper-resolution
This was referenced Aug 31, 2026
os-project-manager
marked this pull request as ready for review
August 31, 2026 12:32
os-project-manager
deleted the
claude/issue-13474-lexical-scope-wrapper-resolution
branch
August 31, 2026 12:50
This was referenced Aug 31, 2026
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.
Fixes #13474
indexFunctionBodiesinscripts/check-durability-degradation-log-level.mjsbuilt one flatMapkeyed by bare NAME to BODY, filled in source order, so a file that declared the same name more than once resolved every call to that name to the last declaration, wherever the call was written. The wrapper hop inisReadCallresolved through that index.It now records every declaration with the scope it is visible from, and
get(name, from)walks outward from the call site, innermost scope first.The live instance, re-anchored on strings (the card's line numbers had drifted)
The card cited
6657 / 6717 / 7129and a call at6674. Those are stale. Locating commands, and what they return on this branch's tree:(Return-type annotations elided as
…— the same convention the card used, and because a GitHub body loses short angle-bracketed fragments.)The call at
6805sits insidefindDraft(6786), whose ownlookupis the one at6788. The flat index answered with7260— a different method entirely, chosen only because it is last in the file. Measured with a replica of the old index:lookup -> flat index resolves to body at line 7260.Anchor on the text, not these numbers: they have now drifted twice.
protocol.tscarries two colliding names, not one — the card namedlookup(3 bodies);findOverlay(2 bodies, 6843 / 7257) collides the same way and is fixed by the same change.A2.2 — which of the four
byName.set(...)sites changeThree of the four (
indexFunctionBodies) change. The fourth belongs toindexLogAliases, a different index answering a different question, and it is deliberately left flat and last-wins. That is measured, not assumed: instrumentingindexLogAliasesto record a duplicate on every insert and running it over the whole scan surface reportsLOG-ALIAS COLLISIONS: 0. There is nothing there for a scope chain to disambiguate, and widening the key anyway would be an unmeasured change to a resolver whose errors have no safe direction — which is the argument against making it. Its docblock now says all of this instead of cross-referencing a key model that no longer exists.A2.3 — the card's "0 seams affected today", re-measured on today's tree
The card's zero was taken at
71627f7b4e. Re-measured at this branch's head (d6cad4a65, which mergesorigin/mainat597020aa5), by running the unmodifiedorigin/mainscript and this branch's script over the same tree and diffing:--list(both rules, every seam)diffexit 0--depth-cost(depths 1..8)diffexit 0Census unchanged: 66 read seams, and the depth-cost table's
6 seams behind the shipped boundis unmoved. So the zero still holds, and it holds at every probed depth — not only at the shipped bound where the card measured it. No seam is affected, so there is no bigger finding to report here.Still by luck rather than by construction, which is the point of the card: all three
lookupbodies readsys_metadatathroughthis.engine.findOne, so the two seams that hop through that call were genuine under any of the three resolutions.What the resolver deliberately does NOT change
Two departures from a strict lexical resolver, both measured:
store.fetch(...)), whose owning class is not on the caller's ancestor chain. It confines this change to exactly the collisions.A2.4 — the regression control, and the ablations that prove it
Four fixtures in
selfTestReadSeams. Withholding the fix means reducing the resolver to the old flat last-wins map. The mutation was confirmed on disk before anything was read — injected-marker count 1, removed-text count 0, blob hash moved from98fe4ce6— because an edit tool that matches nothing still exits 0.Ablation 1, the flat last-wins resolver,
--self-testexit 1, 3 of 4 red and each in its own direction:Both directions of the defect, named and separated — not merely "red at something". The fourth stays green, which is what makes it a test of the other failure.
Ablation 2, the over-fix — strict lexical only, the one-declaration fallback deleted:
Exactly one fixture red, and it is the right one. ⭐ The over-fix is invisible to the live tree: with the fallback deleted,
--listand--depth-coststay byte-identical to the shipped output at every probed depth. Today's corpus cannot tell strict-lexical from the shipped rule, so the pin is the only instrument that can — the card's own "0 cost today is luck, not construction" argument, applied to this change's own design decision.Restore was proven by observed state, not by an exit code, after each ablation:
git diff HEADempty, worktree blob hash98fe4ce6ff63f8f53fb000b735c82f8d6e18dc52equal to theHEADblob hash, injected marker count 0, removed text back at count 1. The gate is executed directly from this source file (node scripts/NAME.mjs) with nodist/on its resolution path, so there is no rebuild leg to skip.Header note retired
The file's header recorded this collision as an open hazard, in the present tense, on the drifted line numbers — and it supplied the third of three reasons for leaving
MAX_READ_WRAPPER_DEPTHalone. Closing the hazard silently would have left a maintainer-reserved decision resting on an argument that no longer holds. The note now says what was true, what closed it, and what survives in the narrower form: a longer chain no longer needs every name in it to be unique, only resolvable at its own call site — but a name declared exactly once is still answered by name alone from anywhere, so a longer chain still crosses more of those. The bound is not moved, and the other two reasons are untouched.Deliberately not touched
contradictsWrapperResolution— predicate and docblock, byte-identical. Its docblock describes this index as flat, which this change makes stale in one sentence; that is left exactly as it stands rather than half-obeying the ruling, and it is raised in the dev report for the maintainer to settle. The guard stays load-bearing either way: a unique name reached on a compound receiver is admitted by the one-declaration rule above and refused by that predicate.Verification
Union run after the final commit, at
d6cad4a65. Exit codes captured before any pipe.Path-derived family (
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, no staleness warning at this head):pnpm check:agent-test-spellingpnpm check:bash32-floorpnpm check:cli-command-idspnpm check:cross-package-test-inputspnpm check:durability-log-levelpnpm check:entry-guardpnpm check:parse-guardpnpm check:pnpm-filter-targetspnpm check:watch-hint-literalnode scripts/check-ci-filter-parity.mjsnode scripts/check-cross-package-test-inputs.mjsnode scripts/check-shard-attestation.mjsnode scripts/check-test-completeness.mjsturbo run testlog, and no log was named … It is NOT a red". Exit 3 is its prerequisite branch, distinct from a finding's 1.Convention-triggered (this diff edits a gate script), run in addition:
node scripts/pm/bare-root-worklist.mjs --self-testpnpm check:pm-dispatch-gatesThe edited script's own suites and the tests that name it:
node scripts/check-durability-degradation-log-level.mjs --self-testpnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2 src/protocol.metadata-store-outage.test.ts src/sys-metadata-repository.draft-drain.test.tspnpm check:nul-bytesscanned 7581 text file(s) … no raw ASCII control bytespnpm lint(repo-wideeslint . --no-inline-config)The dependency closure was built first (
pnpm --workspace-concurrency=2 --filter '@objectstack/metadata-protocol^...' build); the first vitest attempt without it failed onFailed to resolve entry for package "@objectstack/metadata-core", which is the missing-build trap, not a red test.No changeset:
scripts/publishes nothing from any package and the root manifest isprivate: true. Precedent on this tree for a single-file gate-script change —b05ce97c4(scripts/check-doc-anchors.mjs) and5604c445d(scripts/check-type-check-coverage.mjs) — both landed with none.skip-changesetapplied.Generated by Claude Code