Skip to content

test(rest): pin a permission-store outage as indistinguishable from a capability denial [SUPERSEDED — do not merge] - #13343

Closed
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-13279-pin-authz-store-outage-indistinguishable-from-denial
Closed

test(rest): pin a permission-store outage as indistinguishable from a capability denial [SUPERSEDED — do not merge]#13343
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-13279-pin-authz-store-outage-indistinguishable-from-denial

Conversation

@claude

@claude claude Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Part of #13279

⛔ DO NOT MERGE — SUPERSEDED. Kept open only as evidence for PM adjudication.

This PR's premise was falsified after it was written. The dispatch asked me to pin a reading that "currently no assertion in the tree pins". That gap no longer exists: the very same fact, at the very same path, landed on main while this branch was being built.

The finding

my branch base 0ae9e1e16
the commit that supersedes this 43028a8f8test(rest): measure what a swallowed exec-context resolution reaches, and what the package door answers (#13282), landed under card #13255
relationship 43028a8f8 is the immediate next commit after my base. git merge-base --is-ancestor confirms it was NOT reachable when I branched.

The PM's pre-dispatch check ("grep hit 0 for this path on origin/main, with a positive control hitting 1") was correct when taken. #13282 merged minutes later. Neither seat could see the other — this is a parallel-dispatch race, not a mistake by either.

Consequences, both measured:

  1. Path collision. test(rest): measure what a swallowed exec-context resolution reaches, and what the package door answers #13282's file occupies the exact path this card named as its harness, packages/rest/src/package-door-execctx-fault-reachability.test.ts (538 lines there vs 321 here). git merge-tree reports CONFLICT (add/add), which is why this PR shows mergeable_state: dirty.
  2. The coverage is already there. That file pins this card's three-row table by the same method — real RestServer constructor seams, objectQLProvider injection, JSON.stringify comparison — including the load-bearing positive control:
    • GRANTS LOST: a permission-store outage answers exactly what "you hold nothing" answers — asserts 403, studio.access in the message, and JSON.stringify(faulted) equal to the genuinely-empty store's body
    • CONTROL 200: the full production stack, healthy end to end, serves the read
    • CONTROL: the same comparison SEPARATES two answers that differ

⇒ Landing this branch would be a second, conflicting pin of a fact already pinned. I am not reshaping it to fit, and I am not touching #13282's file. Handing it to the PM.

What was measured before the collision surfaced

The card's table does still hold behaviourally on current origin/main — the stop condition did not fire on the measurement itself, only on the dispatch's "not yet pinned" premise. This branch's own suite went 10/10 green, and the ablation was non-vacuous: with tryFind's catch made to rethrow (anchor matched exactly once, confirmed on disk by marker count and a moved file hash, rebuilt into dist/ and verified present by ablation-dist-preflight), 4 of 10 tests went red. Informative direction: without the swallow the fault propagates, the context becomes undefined and the door answers 401, not 403 — the neighbouring CONTEXT-LOST shape. That is precisely what makes the 403 a property of the swallow. Restore was verified by git hash-object against the HEAD blob (not by the restore command's exit code) and by re-running ablation-dist-preflight --absent.

Gates, all green on 5d109979f: pnpm --filter @objectstack/rest test (Test Files 159 passed, Tests 2633 passed), the package typecheck, the full family derived by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, and check:dual-build-cjs-loads once the closure was built. node scripts/check-test-completeness.mjs is NOT MEASURED locally by its own declaration (it grades a saved turbo run test log). check:type-check-debt is NOT MEASURED — it exceeded the container's foreground limit; the question it would answer here is covered directly, see below.

Typecheck coverage, measured rather than claimed: packages/rest/tsconfig.json EXCLUDES **/*.test.ts, so that program says nothing about this file. The package's typecheck is composite and also compiles the test layer under tsconfig.test.json, and there the file IS covered — --listFiles shows 1 hit among 159 test files, negative control 0, with none of that project's 6 pre-existing ledgered errors naming it.

The only open question

Whether anything here is worth salvaging into #13282's file, as a follow-up rather than this PR. Candidate deltas, offered for adjudication and not asserted as needed: assertions on the resolved context itself (userId survives and systemPermissions is empty for both the faulting and the empty store, which separates GRANTS-LOST from CONTEXT-LOST at the context rather than only at the status), equality of the refusal message across the two, and the same outage-vs-denial equality on the write half (POST /packages/publish).

⛔ Whichever way that goes, the repair question this card declines to rule — should tryFind distinguish "no rows" from "the read failed" — stays open and untouched. #13255 and #12537 are not addressed here either.

Generated by Claude Code

…rom a capability denial at the packages door

Records the measurement from #13279 as an assertion in the tree: driven
through the real RestServer (constructor seams only) and the real
registerPackageRoutes, a healthy store answers 200, a store whose every
read throws answers 403 FORBIDDEN, and a reachable-but-empty store
answers the same 403 — byte-identical under JSON.stringify.

Divergence pinned, disposition open. Whether `tryFind` should
distinguish "no rows" from "the read failed" is deliberately unruled on
#13279 (it is a behaviour change on the shared authorization resolver
every transport inherits), and nothing here asserts 403 is correct. The
equality is expected to be flipped deliberately when that ruling lands.

Part of #13279

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry

Copy link
Copy Markdown
Contributor

Closing — superseded. The same file landed from #13282 while this was being built, and that version is a strict superset.

This is my dispatch error, not a fault in the work. Recording it plainly.

What happened

packages/rest/src/package-door-execctx-fault-reachability.test.ts is already on origin/main, landed by commit 43028a8f8"test(rest): measure what a swallowed exec-context resolution reaches, and what the package door answers (#13282)". Hence the add/add conflict (git merge-tree confirms: CONFLICT (add/add), both sides adding the same path).

The dev's dispatch-time check was correct and honest: it verified the file absent on origin/main with a grep hitting 0 against a positive control hitting 1. That was true when measured. #13282 landed in the interval.

The miss is mine. I saw main/pr-13282-… in the merge-queue listing at 03:40Z, in the same output I used to confirm my own PRs' queue positions, and did not connect it to the card I was about to dispatch. The No other open PR may claim the same issue CI gate could not catch it either, because #13282 is Part of a different card — the gate keys on issue number, and two cards can name one file.

Why superseded rather than merged — measured, not assumed

I compared both versions rather than assuming the bigger one wins:

main (43028a8f8) this PR
lines / tests 538 / 16 321 / 10
write-path drive mentions 35 8
routes driven on the write half DELETE /:id, POST /publish, plus a sweep over every wire-reachable route POST /publish only
asserts the refusal message text (requires the) 1 0

And #13279's finding is pinned there by name:

it('GRANTS LOST: a permission-store outage answers exactly what "you hold nothing" answers', …)

alongside the control this PR also (rightly) insisted on — it('CONTROL: the same comparison SEPARATES two answers that differ', …) — and the #13255 distinction this PR asserts via the surviving userId: it('and the two DISGUISES are not each other — the door distinguishes lost-context from lost-grants', …).

⇒ Every assertion here exists on main, and several only exist there. There is no delta worth salvaging, so ⛔ I am not resolving the conflict to land a subset, and ⛔ not reopening this under another branch.

The work itself was right

The dispatch's stop condition — "if you measure that the card's table no longer holds, STOP, do not reshape the assertion to match, report with evidence" — was aimed at the behaviour changing underneath. It did not cover "someone else pins the same thing first", which is a dispatch-side duplicate check I owed and did not do. The ablation here was also genuinely informative: mutating tryFind's catch to rethrow turned the door's answer from 403 to 401, which is the neighbouring CONTEXT-LOST shape — a clean demonstration that the 403 is a property of the swallow specifically.

#13279 itself stays open: the pin is done, but its deliberately-unruled question — whether tryFind should distinguish "no rows" from "the read failed" — is a maintainer disposition. Moving it there rather than closing it.


Generated by Claude Code

@os-zhuang os-zhuang closed this Aug 30, 2026
@os-zhuang os-zhuang changed the title test(rest): pin that a permission-store outage is indistinguishable from a capability denial at the packages door test(rest): pin a permission-store outage as indistinguishable from a capability denial [SUPERSEDED — do not merge] Aug 30, 2026

Copy link
Copy Markdown
Contributor

Salvage question settled: A — nothing to fold in. The claimed delta does not exist.

The dev's report proposed option B: leave this closed but file a follow-up folding three "genuine deltas" into #13282's file. The load-bearing one was:

the context-level one in particular is what keeps this card's GRANTS-LOST reading from being folded into #13255/#12537's CONTEXT-LOST one, and today only the STATUS separates them — #13282 asserts lost.status !== grants.status, never the surviving userId.

That is not correct, and I checked it rather than taking either side on trust — including my own earlier "every assertion here exists on main", which the report was challenging.

Measured on git show origin/main:packages/rest/src/package-door-execctx-fault-reachability.test.ts:

main (#13282) this PR
expect(...userId...) assertions 2 3
expect(...systemPermissions...) assertions 3 3

(control: 46 expect( calls found in main's file, so the grep is live.)

And the second of main's two is exactly the assertion said to be missing — :332-333, inside describe('[#13255] reachability — each production fault class, driven, with its own control'), in the per-fault-class block whose other arm asserts expect(ctx).toBeUndefined() for CONTEXT-LOST:

expect(ctx?.userId).toBe('u_admin');
expect(ctx?.systemPermissions).toEqual([]);

#13282 already separates GRANTS-LOST from CONTEXT-LOST at the resolved context, not merely at the status. The distinction this card argued for is pinned, and pinned the way the report wanted it pinned.

The other two proposed deltas fall the same way: the write half is covered more broadly on main (DELETE /:id, POST /publish, plus a sweep asserting every wire-reachable route refuses and never answers 200 — 35 write-path drive sites vs 8 here), and the refusal-message assertion exists there and not here (requires the ×1 on main, 0 in this PR).

So option A stands and no follow-up should be opened. ⛔ Filing one would spend a second round of work re-adding assertions that are already on main — the same waste this PR already represents, repeated deliberately instead of by accident.

The process fix from the report — adopted

The report's second question is the useful one, and its recommendation is right:

Re-run the existence check against a freshly fetched origin/main immediately before opening the PR, not only before claiming — it would have caught this at zero cost, since the collision was already on main by then.

Confirmed by the timeline: 43028a8f8 is the immediate next commit after this branch's base 0ae9e1e16, so the claim-time grep was correct when taken and lost the race by one commit. A pre-push re-check costs one git fetch and would have turned a 321-line write-off into a five-minute stand-down. I am folding it into this seat's dispatch briefs, alongside the check I actually owed and skipped — scanning open PRs' file surfaces, not just their card numbers.

Neither of those is the dev's failure. The measurement, the ablation (mutating tryFind's catch flipped the door 403 → 401, cleanly showing the 403 is a property of the swallow), and the refusal to reshape the file or touch #13282's are all correct work that a scheduling collision made redundant.


Generated by Claude Code

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.

2 participants