Skip to content

fix(lint): resolve the durability gate's wrapper hop through the call site's lexical scope - #13784

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-13474-lexical-scope-wrapper-resolution
Aug 31, 2026
Merged

fix(lint): resolve the durability gate's wrapper hop through the call site's lexical scope#13784
os-project-manager merged 3 commits into
mainfrom
claude/issue-13474-lexical-scope-wrapper-resolution

Conversation

@claude

@claude claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes #13474

indexFunctionBodies in scripts/check-durability-degradation-log-level.mjs built one flat Map keyed 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 in isReadCall resolved 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 / 7129 and a call at 6674. Those are stale. Locating commands, and what they return on this branch's tree:

$ grep -n "const lookup = async" packages/metadata-protocol/src/protocol.ts
6788:                    const lookup = async (t: string): … => {
6848:                const lookup = async (t: string): … => {
7260:                const lookup = async (t: string) => {

$ grep -n "const findDraft = async" packages/metadata-protocol/src/protocol.ts
6786:                const findDraft = async (oid: string | null): … => {

$ grep -n "const rec = await lookup(request.type);" packages/metadata-protocol/src/protocol.ts
6805:                    const rec = await lookup(request.type);
6871:                const rec = await lookup(request.type);

(Return-type annotations elided as — the same convention the card used, and because a GitHub body loses short angle-bracketed fragments.)

The call at 6805 sits inside findDraft (6786), whose own lookup is the one at 6788. The flat index answered with 7260 — 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.ts carries two colliding names, not one — the card named lookup (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 change

Three of the four (indexFunctionBodies) change. The fourth belongs to indexLogAliases, a different index answering a different question, and it is deliberately left flat and last-wins. That is measured, not assumed: instrumenting indexLogAliases to record a duplicate on every insert and running it over the whole scan surface reports LOG-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 merges origin/main at 597020aa5), by running the unmodified origin/main script and this branch's script over the same tree and diffing:

comparison result
--list (both rules, every seam) byte-identical, diff exit 0
--depth-cost (depths 1..8) byte-identical, diff exit 0

Census unchanged: 66 read seams, and the depth-cost table's 6 seams behind the shipped bound is 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 lookup bodies read sys_metadata through this.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:

  • one declaration ⇒ unchanged. 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 (store.fetch(...)), whose owning class is not on the caller's ancestor chain. It confines this change to exactly the collisions.
  • ambiguous and out of scope ⇒ refuse. Several bodies, none enclosing the call site: the hop is not followed. File order is not evidence, and declining is the under-counting direction this family declares.

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 from 98fe4ce6 — because an edit tool that matches nothing still exits 0.

Ablation 1, the flat last-wins resolver, --self-test exit 1, 3 of 4 red and each in its own direction:

✗ flags: #13474 — the enclosing same-named body is the one resolved …
      expected violation=true seams=1, got violation=false count=0 seams=0     <- DROPPED a real seam
✗ passes: #13474 — a later same-named body that reads must not INVENT a seam …
      expected violation=false seams=0, got violation=true count=1 seams=1     <- INVENTED a seam
✗ flags: #13474 — three same-named bodies, resolved from inside the first …
      expected violation=true seams=1, got violation=false count=0 seams=0     <- DROPPED (the live shape)
✓ flags: #13474 — a name declared ONCE still resolves from outside its scope …

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:

✗ flags: #13474 — a name declared ONCE still resolves from outside its scope (the fix must not narrow the census)
      expected violation=true seams=1, got violation=false count=0 seams=0

Exactly one fixture red, and it is the right one. ⭐ The over-fix is invisible to the live tree: with the fallback deleted, --list and --depth-cost stay 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 HEAD empty, worktree blob hash 98fe4ce6ff63f8f53fb000b735c82f8d6e18dc52 equal to the HEAD blob 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 no dist/ 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_DEPTH alone. 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):

gate exit verdict
pnpm check:agent-test-spelling 0 GREEN
pnpm check:bash32-floor 0 GREEN
pnpm check:cli-command-ids 0 GREEN
pnpm check:cross-package-test-inputs 0 GREEN
pnpm check:durability-log-level 0 GREEN
pnpm check:entry-guard 0 GREEN
pnpm check:parse-guard 0 GREEN
pnpm check:pnpm-filter-targets 0 GREEN
pnpm check:watch-hint-literal 0 GREEN
node scripts/check-ci-filter-parity.mjs 0 GREEN
node scripts/check-cross-package-test-inputs.mjs 0 GREEN
node scripts/check-shard-attestation.mjs 0 GREEN
node scripts/check-test-completeness.mjs 3 NOT MEASURED — its own text: "this gate grades a saved turbo run test log, 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:

gate exit verdict
node scripts/pm/bare-root-worklist.mjs --self-test 0 GREEN
pnpm check:pm-dispatch-gates 0 GREEN

The edited script's own suites and the tests that name it:

run exit verdict
node scripts/check-durability-degradation-log-level.mjs --self-test 0 GREEN — log-level rule 63 cases, read-seam rule 57 cases (53 before this PR)
pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2 src/protocol.metadata-store-outage.test.ts src/sys-metadata-repository.draft-drain.test.ts 0 GREEN — 2 files, 44 tests. These are the only two test files in the tree that name this script.
pnpm check:nul-bytes 0 GREEN — scanned 7581 text file(s) … no raw ASCII control bytes
pnpm lint (repo-wide eslint . --no-inline-config) 0 GREEN — full farm, no narrowing claimed

The dependency closure was built first (pnpm --workspace-concurrency=2 --filter '@objectstack/metadata-protocol^...' build); the first vitest attempt without it failed on Failed 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 is private: true. Precedent on this tree for a single-file gate-script change — b05ce97c4 (scripts/check-doc-anchors.mjs) and 5604c445d (scripts/check-type-check-coverage.mjs) — both landed with none. skip-changeset applied.


Generated by Claude Code

claude added 3 commits August 31, 2026 11:46
… 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants