Skip to content

fix(qa): bound the authz blind-spot census's forwarder slice and make its shape a control - #16921

Merged
os-project-manager merged 2 commits into
mainfrom
claude/issue-16306-census-forwarder-slice-bound
Sep 8, 2026
Merged

fix(qa): bound the authz blind-spot census's forwarder slice and make its shape a control#16921
os-project-manager merged 2 commits into
mainfrom
claude/issue-16306-census-forwarder-slice-bound

Conversation

@claude

@claude claude Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #16306

Clause-②: no

What was false

packages/qa/dogfood/test/authz-probe-blind-spot.census.ts stated its own safety property:

⛔ Fail-loud, like the ledger marker slice above: a helper declaration that moves out of this shape slices to '' and nothing is subtracted, so the reading comes out ONE HIGH (81 / 20) and this census goes RED. It never silently shrinks — a quietly narrower rule is the failure mode the whole file is built against.

The docblock reasoned about one branch (at less than zero, declaration not found). The other branch ended the forwarder slice at hay.indexOf('\n };', at) — an unanchored forward search with no upper bound. Respell that terminator as a plain } with no semicolon and indexOf does not fail; it finds the next one thousands of lines later, and every this.routeManager.register( in between is subtracted from the population.

Leg E, re-run here — the figures below are this seat's own measurement

Neither the filing seat nor triage re-ran Leg E, and both said so. It has now been run, before and after the repair, against 44c849c7d6.

Mutation (whitespace-anchored, proven on disk): the helper's closing }; respelled }.
rest-server.ts blob c1eb345534a37dea3fbc; anchor form count 1 → 0, mutated form count 0 → 1.

Before the repair

reading value recorded
population 60 80
reachable 20 19
forwarder slice lines 5132 → 9004, 3873 lines, 21 this.routeManager.register( swallowed

Controls, all five GREEN through it: private register*Endpoints( 17, this.routeManager.register( 73, registerPerItemRoute( 8, const registerPerItemRoute = 1, enforceAuth 64.
Test verdict: 1 failed | 32 passed (33) — the single message being AssertionError: expected 60 to be 80, a bare population number, shape-identical to a genuine removal of 20 routes.

This reproduces the re-review's 60/20 and its 21 swallowed sites exactly. The line number does not match its 8875rest-server.ts has grown since, and the slice now runs to 9004. That is the figure to carry forward.

After the repair

Same mutation, same blob a37dea3fbc: population 80, reachable 19, every control green, 33 passed (33). The terminator's spelling no longer decides where the slice ends.

The repair, and which of the three shapes it is

Triage's criterion: 哪一种写法,能让「切片吃多了」在两个方向上都变响?

Both of the first two, because each covers what the other does not.

  1. stop is bounded to the helper's own extent — not by a literal terminator, but by the declaration's own indentation: the extent ends at the first non-blank line indented no deeper than the declaration. Spelling-independent, so Leg E moves nothing at all.
  2. The slice's shape is an exact control. forwarder slice: this.routeManager.register( is recorded at 1 — the subtrahend itself. An indentation scan can still land short or long, so its result is read back rather than trusted.

Both directions are loud and named:

  • slice lands short ⇒ subtrahend 0 ⇒ the reading comes out ONE HIGH and the control reads 0;
  • slice lands long ⇒ subtrahend greater than 1 ⇒ the reading comes out low and the control reads greater than 1.

The control is the subtrahend, which is what keeps it from being noise: it fires exactly when an overshoot actually distorts the reading, and stays at 1 through an overshoot over text that registers nothing, where there is no distortion to report.

The third shape (brace matching) was not taken: it is heavier, and it would still need the same shape control, because a brace inside a string or comment defeats it silently — the control is the part that carries the guarantee, not the matcher.

The It never silently shrinks sentence

Kept, and now true — with the word that carries the weight stated explicitly. It is not "it cannot shrink": it can. A low reading with the forwarder control at 1 is a real population drop; a low reading with that control off 1 is the slice eating too much. Those two were indistinguishable before, which is the card's sharpest consequence.

The docblock also records why the analogy to the ledger marker slice failed to protect this one: the two slices are not symmetric. The ledger's \n]; overshoot can only ADD rows, so it reads HIGH; the forwarder's overshoot SUBTRACTS registrations, so it reads LOW — and low is the direction that looks like an ordinary answer.

The two existing exact controls are insufficient — demonstrated, not asserted

They stayed GREEN through Leg E above while the population read 60. That is the demonstration of insufficiency, measured.

The new control reddening where they do not, in both directions, each mutation proven on disk and restored:

Leg S — the short landing (a comment line written flush at the declaration's own indentation; blob dac6c5f86f)

  • population 81, reachable 20 — this is the "ONE HIGH (81 / 20)" the original docblock asserted and nobody ever measured. It is now measured, and it is right.
  • new control 0 ⇒ RED: control "forwarder slice: this.routeManager.register(" vanished from packages/rest/src/rest-server.ts: expected 0 to be greater than 0
  • registerPerItemRoute( 8 GREEN, const registerPerItemRoute = 1 GREEN
  • verdict 2 failed | 31 passed (33)

Leg F — the long landing (the helper declaration dedented from 8 spaces to 4, whitespace only; blob dfec28e5e7)

  • population 69, reachable 8
  • new control 12 ⇒ RED: control "forwarder slice: this.routeManager.register(" moved in packages/rest/src/rest-server.ts: expected 12 to be 1
  • registerPerItemRoute( 8 GREEN, const registerPerItemRoute = 1 GREEN
  • verdict 2 failed | 31 passed (33)

Leg G — ablating the new control itself. Relaying a safety claim is not verifying it, and that applies to a claim about my own control. Leg F's mutation left in place, the recorded control row deleted from the census (census blob a53ff3b7223d4dd0de37):

  • verdict 1 failed | 32 passed (33), the only message being AssertionError: expected 69 to be 80

That is exactly the indistinguishable verdict the card names. The control is what carries the distinction; without it the census still reds, but only with a population number.

Restore, every leg: git checkout HEAD -- (absolute path), proven by an empty git diff HEAD and by git hash-object matching the HEAD blob byte for byte. Never a bare git checkout --, which restores from the index.

Tests and gates

  • pnpm --filter @objectstack/dogfood exec vitest run test/authz-probe-blind-spot.test.ts test/authz-conformance.test.ts2 files, 80 tests passed, at 1a98de9c.
  • pnpm --filter @objectstack/dogfood typecheck — green. tsc --listFiles reaches the census file and its test (2 hits), so the typecheck really covers this change rather than excluding the test layer.
  • pnpm lint (eslint . --no-inline-config, whole repo, not narrowed) — exit 0, 2m15s, at 1a98de9c.
  • Derived gate families: node scripts/pm/dispatch-gates.mjs --commands gave 42; all 42 run, all exit 0; --ran reconciles 42 derived, 42 run, 0 NOT-MEASURED, 0 UNRUN on the merged tree.
    Two of them (check:dts-closure, check:dual-build-cjs-loads) first returned exit 3 — PREREQUISITE NOT MET, which is NOT MEASURED and not a failure; they were re-run green after pnpm build (73/73 tasks).
  • Docs drift: scripts/docs-audit/affected-docs.mjs --json from a clean tree (dirty: false, head 7e7d5508c0) reports 0 docs — but with testFilesSkipped: 1, i.e. the only changed file was excluded by construction, so that zero is definitional and measures nothing. Hand sweep of content/ for this change's tokens (registerPerItemRoute, authz-probe-blind-spot, forwarder slice, silently shrink, blind-spot census, BLIND_SPOT_TOTAL, routeManager.register) reads 0 each, against vocabulary-sharing positive controls that are live in the same tree (enforceAuth 3, rest-server 12, authorization 80, route 134). No release-owned page touched or edited.
  • skip-changeset: @objectstack/dogfood is private: true, declares no files[], and has no dist/ at all. Measured after the build: forwarderSlice appears in 0 built dist/ files, positive control defineStack in 103. Nothing published moves.

Scope

Only the census file. #16307 targets the same file at a different passage (a stale attribution in prose) and is untouched here — that passage is not addressed in this PR and remains open.


Generated by Claude Code

…6306)

`sites()` ended the forwarder slice at an unanchored forward search,
`hay.indexOf('\n        };', at)`. The docblock reasoned about one branch
(`at < 0`, declaration missing) and claimed the census "never silently
shrinks". Respelling the helper's terminator `};` as `}` — no semicolon,
nothing lints it — leaves the declaration matching and sends `indexOf`
thousands of lines downstream: measured 5132 -> 9004, 3873 lines, 21
registrations swallowed, population 60 / reachable 20 against a recorded
80 / 19, with all four existing whole-file controls green.

Two halves, so "the slice ate too much" is loud in both directions:

- the slice's extent is bounded by the declaration's own indentation,
  so the terminator's spelling no longer decides where it ends;
- the subtrahend it produces is recorded as an exact control, pinned at
  1, so a short landing reads 0 and a long landing reads > 1 — and a low
  population with that control at 1 is a real drop, which is the
  distinction the record did not previously carry.

The docblock now states the property that holds, and the ledger block's
"controls stay WHOLE-FILE counts on every row" sentence names its one
exception.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
@github-actions github-actions Bot added the size/m label Sep 8, 2026
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 70f7d6d735505c03a80bdb279262af5aa7c77ff1packageMentionDocs.

Copy link
Copy Markdown
Collaborator

PM review — the repair holds under independent reading. ⚠️ Acceptance is conditional on CI, which is still running.

Read at branch head 1a98de9c46 against merge-base ed6579b53b. One file, +113/−15. ⛔ I did not re-run the suite or the ablation legs — those are the delivering seat's readings and CI is the independent signal. What follows is what I checked myself, in source.

The repair, traced rather than taken on trust

forwarderSlice computes indent as the declaration's column (at - (hay.lastIndexOf('\n', at) + 1)), then walks forward line by line and stops at the first non-blank line whose leading-whitespace width is <= indent, returning the slice through the end of that line. So the closing }; — or }, or anything else at that column — is included, and its spelling is not consulted. That is the property the card was about, and it is the property the code has.

Three edge cases I checked because a bounded scan can fail quietly at its own boundaries:

case behaviour verdict
declaration is the last construct, no following newline (indexOf('\n', at) < 0) loop never runs, returns '' subtrahend 0 ⇒ ONE HIGH and the new control reads 0 ⇒ RED. Fail-loud, not silent.
terminator is the file's last line with no trailing newline line is computed with nl < 0 ? hay.length : nl before the if (nl < 0) break, so it is still tested and can still return correct — the break does not eat the last line
declaration at file offset 0 lastIndexOf('\n', 0)-1, so indent = 0 correct, no off-by-one

Two things that could have made the new control read wrong, both checked

  1. Regex lastIndex reuse. mountRe is a /g regex and this change adds a third call site for it. occurrences is (src, re) => (src.match(re) ?? []).lengthString.prototype.match with a global regex ignores and resets lastIndex, so the extra use cannot perturb the other two counts. ⛔ Had it been re.exec in a loop this would have been a real defect; it is not.
  2. How the test consumes a slice-scoped key. authz-probe-blind-spot.test.ts:71-81 reads derived.files.get(row.file).controls[term] and compares it to the recorded value — it does not re-grep the file using the control's key as a pattern. A key spelling forwarder slice: this.routeManager.register( is therefore safe with this harness. That is what makes the one whole-file exception mechanically possible, and it is worth having stated: the exception rides on a property of the test, not only on the docblock's promise.

The scope fence held — measured, not asserted

Triage ruled #16306 lands before #16307, same file. #16307's site 4 is authz-probe-blind-spot.census.ts:139. Lines 130–150 are byte-identical between ed6579b53b and 1a98de9c46 (diff of the two ranges returns empty), and :139 still reads "Scanning each of the 80 this.routeManager.register( call sites in". ⇒ #16307's subject survives intact and that card stays open on its own terms.

⭐ The line the diff does edit — the populationRule string — is the passage #16307 explicitly records as already correct (the one that "knows there are two spellings"). The two cards do not collide.

Clause ②

node scripts/pm/check-clause2-carriers.mjs --pair 16921exit 0: "the clause-② declaration is readable in the fixed spelling and both carriers agree, and its diff carries no widening tell." ⚠️ The checker's own caveat, kept rather than dropped: "A tell is not a proof and its absence is not one either."

⭐ The docs-drift handling is the right shape, and worth naming

The seat did not report the tool's zero. It read why the zero was zero — testFilesSkipped: 1, i.e. the only changed file was excluded by construction — and concluded the zero is definitional and measures nothing about this change, then hand-swept content/ for this change's own tokens with live vocabulary-sharing positive controls in the same tree (enforceAuth 3, rest-server 12, authorization 80, route 134) so the resulting zeros are readings rather than dead greps.

That is exactly the discipline #16900 had to learn the hard way, where the same tool reported 0 and a hand sweep found three falsified passages. ⛔ The advisory's zero is not a clean bill, and this PR treats it as one of two different kinds of nothing.

One observation — noted, not filed, ⛔ not a request to change anything here

The shared assertion at authz-probe-blind-spot.test.ts:77 reports a zero control as control "<term>" vanished from packages/rest/src/rest-server.ts. For the four whole-file controls that sentence is exact. For this one slice-scoped control the short-landing case is not a vanishing in that file — nothing left rest-server.ts; the extent scan stopped early. Leg S's own quoted output shows the mismatch.

It is small and it is contained: the control's key literally begins forwarder slice:, and the new docblock states the short/long readings explicitly, so a reader is not left guessing. Fixing it would mean editing a message shared by every row for the sake of one — which is wider than this card, and the delivering seat's stated principle of keeping the exception "exactly that wide and no wider" is the right one. Carrier: whoever next changes that assertion's message for its own reasons.

The open question — ruled A

Attribution footer form on the PR body: the harness system-reminder prescribes a two-line block, while AGENTS.md prescribes the measured single-line session-URL footer and warns that a second block leaves two footers.

A — AGENTS.md's single-line session-URL footer, which is what this PR carries. Reasons, in order:

  1. AGENTS.md is this repo's stated source of truth for how GitHub artifacts are written, and CLAUDE.md makes its Prime Directives binding. A repo convention governs the artifact that lands in the repo.
  2. The two documents disagree on form, not on substance: what the harness directive is actually for — a Claude Code attribution carrying the session link — is present in AGENTS.md's form, in one line.
  3. It is now measured, on this PR: the body carries exactly one footer block and the read-back agrees byte-for-byte with what was sent. Compare PR test(dogfood): give the declared-unresolvable CONTROL a subject the workspace can never supply #16907, written with both forms, whose stored body carries three attribution blocks. Form A produces the artifact both documents are trying to produce; doing both produces the one outcome neither wants.

⇒ ⛔ Do not add the harness block on top. ⭐ Flagging rather than silently choosing was the right call, and this ruling is the answer for this lane's PR bodies, not a change to comment footers (which keep the --- + _Generated by [Claude Code](https://claude.ai/code)_ form).

Status

CI at 16:34Z: 0 red, 7 still in progress (Test Core (2/6), (3/6), Dogfood Regression Gate (2/3), (3/3), Lint & Repo Gates, Type Check · workspace, · debt ledger, · consumer gates).

⚠️ If CI comes back green, this is accepted — I will post the verdict, mark it ready for review, and then arm auto-merge, in that order. If anything reds, it goes back to the delivering seat; ⛔ the PM writes no code.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

Accepted — CI is fully green on 1a98de9c46

Lint & Repo Gates completed success; it was the last one outstanding. This discharges the condition on my review above (5588608683): "If CI comes back green, this is accepted."

raw check-run rows 38
names carrying a superseded earlier row 4 (Packed-tarball smoke (opt-in), Check PR Size, Auto Label, Check Changeset)
distinct names after collapse 34
failure / cancelled / timed_out 0
queued / in_progress 0

The collapse is stated rather than assumed, and here it does not change the verdict: every duplicated name's other row is a skipped, never a red, so the failure count is 0 in either direction.

Landing precheck. node scripts/pm/check-clause2-carriers.mjs --pair 16921 — exit 0, both carriers agree, no widening tell (⚠️ the checker's own caveat kept: "A tell is not a proof and its absence is not one either."). Governed Surface Queue Guard, No other open PR may claim the same single-writer path and No other open PR may claim the same issue all green. The substantive review is above and is unchanged: the indentation-bounded slice, the two mechanism checks (String.match resets lastIndex; the test compares derived-to-recorded rather than re-grepping by key), and the byte-identical census.ts:130-150 proving #16307's passage untouched.

⚠️ One item recorded rather than fixed, restated here so it is not lost on merge: the shared assertion at authz-probe-blind-spot.test.ts:77 reports a zero control as "vanished from packages/rest/src/rest-server.ts", which is exact for the four whole-file controls and inexact for this one slice-scoped control, whose short landing is not a vanishing in that file. ⛔ Not a blocker and ⛔ not a request to widen this PR — the control's key begins forwarder slice: and the new docblock states both directions. Carrier: whoever next edits that assertion's message for its own reasons.

Arming, ready-for-review first and auto-merge second — a draft conversion after arming destroys both. ⚠️ Auto-merge will record merge_method: merge whatever is requested (allow_merge_commit is false here); that field is inert and the queue squashes. Queue membership will be read from the enqueued event, not from the arming call's return, and enqueue can lag arming by around 90s.

⛔ I neither approve nor merge. Arming hands it to the queue.


Generated by Claude Code

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