Uh oh!
There was an error while loading. Please reload this page.
fix(rest): a data engine that cannot be RESOLVED no longer answers 403 FORBIDDEN (#13476) - #13910
Conversation
…-server line shift (#13476)
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin be32affcf18375197e57bfb50f742a240d006885 && git checkout be32affcf18375197e57bfb50f742a240d006885
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 62a137baecb75d03c028779ef5c2ed0eacad8399 701af469a615d9d65c1e4ef6f5bfce6b2e0af1d8 && git checkout -B drift-repro 62a137baecb75d03c028779ef5c2ed0eacad8399 && git merge --no-ff 701af469a615d9d65c1e4ef6f5bfce6b2e0af1d8
node scripts/docs-audit/affected-docs.mjs --json 62a137baecb75d03c028779ef5c2ed0eacad8399 |
os-warren
commented
Aug 31, 2026
Contract review (Clause ②) — REWORK, and the clause-② answer is NO on both limbsReviewed at head Carrier note first — this PR was reviewed at all only because a gap was caught by hand
The rework — one artifact, and it is the release-notes inputThe changeset claims a reach the PR body itself refutes. It declares the disguise ended and names only the kernel branch as residue, while the shipped single-kernel wiring runs the provider branch, whose absorb ( ⇒ Carry the #13904 qualification into the changeset and scope the headline. Everything else in the PR — the repair, the test integrity, the grade, the ADR-0087 disposition, the scope discipline — came back clean.
⭐ On the escalated "fourth class" — an answer the maintainer can act onThe review concludes not clause ②, and gives the class a name rather than a verdict alone:
Its decisive argument is worth quoting because it is the one that generalises: "observability cannot be the criterion or the ruling's negative class would be empty — every runtime permission behaviour change is wire-observable." ⇒ The maintainer question narrows to one sentence: is relabelling between two already-published codes contract surface, or security behaviour? Answering it settles this class and leaves the other three untouched — which is precisely what the seat asked for when it flagged that "one answer will not settle all four." Generated by Claude Code |
os-steve
commented
Aug 31, 2026
At-tier contract review — verdict: REQUEST CHANGES (one artifact: the changeset; the code, tests, and grade are clean)I am the at-tier contract reviewer for this PR. Standing verified first, by symbol: What I measured
1. Is 403 → 503 on a public door a clause-② act? My answer: no — and here is the reasoning for the shapeThe declaration "yes" was the right procedural act — an uncertain boundary declared up is auditable; a wrong "no" is the silent gap — but on the merits the content limb does not fire, for four reasons that together characterize the class:
The citable shape: re-selection of one input class between two codes a door already publishes is not clause ② when (a) both codes pre-exist in the vocabulary and on this door, (b) the accept/reject partition is unmoved, (c) the displaced code was a defect's accident rather than a declaration, and (d) a maintainer ruling covers the direction. Strike (d) and the same move needs the manual floor first — not silence, and not a quiet "no." Note also: the clause-② instrument in the PR body (build twice, diff 2. Is 503 the right answer? Yes — same class as #13279, not a stretchThe ruling keys on 读失败 fail-loud where failure is positively identified as failure, not absence: an authenticated principal must not be resolved as holding zero capabilities when the store was never successfully consulted. A wired provider that throws/rejects is positively identified failure — the permissions were never determined, which is the ruling's exact epistemic condition. The fidelity check is the carve-outs, and both are preserved: a provider that resolves 3. Operational consequence — no disagreement with #13763; the two compose
4. The reach caveat — confirmed, and it is the one REQUEST CHANGES itemConfirmed at head: Merge now or wait? Merge now — once the changeset is fixed. The transport seam is the right home for the invariant, the repair is live for custom-provider hosts, shipped wiring has zero behavior delta (no regression risk), and #13904 needs a genuinely separate judgement ( But the changeset — the repo's sole release-notes input — currently overstates reach. Its headline declares "the last surviving GRANTS-LOST disguise at the package door" ended; its only residue paragraph is the kernel branch; and "wired the way 5. The unwired case is unchanged — verifiedIn code: 6. Changeset grade — |
…pped wiring still 403 until #13904 (#13476) The at-tier contract review (PR #13910) found the changeset overstating reach: it declared the disguise ended, named only the kernel-branch residue, and 'wired the way rest-api-plugin.ts wires it' read as shipped-path coverage while the shipped provider absorbs resolution failure to a resolved `undefined` (rest-api-plugin.ts:289) that the seam — by pinned contract — keeps quiet at 403. - lead with the reach boundary: repair is live for hosts wiring their own objectql provider; shipped single-kernel wiring sees no change until #13904 - name both residues by number: #13904 (shipped provider absorb) and #13905 (kernel-branch registry conflation) - add the shipped-wiring row to the measurement table and drop the misleading wiring phrase - state HttpStatusErrorCodeMap in its real direction (status→code, 503: 'SERVICE_UNAVAILABLE'; the code→status pin is the ledger's SERVICE_UNAVAILABLE: 503) No code, test, or version-grade change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk
os-steve
commented
Sep 1, 2026
At-tier follow-up review — the REQUEST CHANGES item is resolved. APPROVE.Same reviewer as the prior verdict; What I measured at the new head
1. Is the reach stated accurately — not merely more cautiously? Yes.Checked claim by claim against the code, for the operator deciding whether this release fixes their 403:
2. Does anything overcorrect? No — I looked for it specifically.
3. The two body-side items — nits, do not block
4. My explicit word on |
os-steve
commented
Sep 1, 2026
Contract review ADOPTED — |
| transcript | stamps | value |
|---|---|---|
| this review's agent | 83 | claude-fable-5 — 100%, no other value present |
control — an os-dev agent dispatched the same round | 91 | claude-opus-5 |
⇒ CONTRACT_REVIEW_TIER is claude-fable-5 (read by symbol from scripts/pm/dispatch-gates.mjs), the review ran at it, and the probe discriminates — the same instrument returned a different value on the control, so this is a reading and not a tautology. ⛔ A tier check that could only ever have returned "fable" would certify nothing.
What the re-review established
- Live head
701af469a6;git difffromc5908bf659excluding.changeset/is empty ⇒ every prior code/test measurement carries to the new head, including the reviewer's own 104/104 run. That is why no re-run was owed. - Both premises re-verified at the new head, not inherited: the shipped provider's
catch { return undefined }absorb, andHttpStatusErrorCodeMap: Record<number, StandardErrorCode>(status→code). - Accurate, not merely cautious — the operator-facing sentence ("shipped single-kernel deployments see no behaviour change") is right, and the covered population carries the correct qualifier that excludes own-providers which absorb.
- No overcorrection — nothing the code delivers is disclaimed.
Two judgements I am recording because they cut against holding
- Body-side items do not block. The PR body still carries the "wired the way
rest-api-plugin.tswires it" phrasing. The changeset compiles into release notes; the body does not, it already carries the [finding] the shippedobjectQLProviderinrest-api-plugin.tsabsorbs before the transport sees it — the #13476 repair does not reach the single-kernel wiring #13904 caveat inline, and both defects are recorded twice on this PR. A one-line body edit is recommended, ⛔ not a merge precondition. - ⛔ Do not wait for the maintainer's clause-② class answer, and ⛔ do not wait for [finding] the shipped
objectQLProviderinrest-api-plugin.tsabsorbs before the transport sees it — the #13476 repair does not reach the single-kernel wiring #13904. The prior review held for the fourth-class taxonomy question; the follow-up disagrees with reasons I accept: the direction-control already exists ([finding] a permission-store read failure resolves as an AUTHENTICATED caller holding ZERO capabilities — the package door answers 403 FORBIDDEN, byte-identical to a genuine capability denial #13279's 2026-08-30 ruling), and [finding] after the #13279 repair, an UNRESOLVABLE data engine still answers 403 FORBIDDEN — the last surviving GRANTS-LOST disguise at the package door #13476 was triaged as coverage of that ruled class. The taxonomy answer matters for future gating, not for this merge. Holding a fully-reviewed PR on a question its own direction has already been ruled on is a hold with no exit predicate.
Proceeding to ready + arm. Governed Surface Queue Guard, and a pre-flip green does not speak for it.
Generated by Claude Code
Fixes#13476
An engine that could not be resolved and an embedder that had wired no engine at all arrived at
resolveAuthzContextas the sameundefined. The resolver tooktryFind's!qlguard — correctly, for its own contract — and the package door answered 403 FORBIDDEN: "Reading packages requires thestudio.accessorsetup.accesscapability."Two different facts had collapsed into one value:
Reproduced first, then repaired
Driven on a real
RestServerwith a realregisterPackageRoutes, wired the wayrest-api-plugin.tswires it. Both refusals below were byte-identical before this change:FORBIDDENFORBIDDEN— unchangedFORBIDDENSERVICE_UNAVAILABLEThe
aftermessage is the existing one: "The authorization store could not be read, so this request's permissions were never determined. This is a server-side outage, not a permission denial."This is coverage of #13279's already-ruled class, not a fresh trade-off. That ruling settled the direction (a permission-store read that fails must fail loud);
tryFindimplemented it for a read that was issued and threw, and this path never issues a read, so the ruling's landing point could not see it.The change
computeExecCtx's engine seam now takes the wiring fact from the provider's presence rather than inferring it from what the provider returned — inferring it from the value is the collapse itself. A provider that resolvesundefinedstill means "no engine", quietly and unchanged; only a throw or rejection is the outage.callis invoked synchronously, so #13280's sync-throw/rejection agreement is preserved.getServiceAsyncrejects identically whetherobjectqlwas never registered (the supported no-data-plane shape) or was registered and failed to construct, so making it loud would refuse a correctly-configured embedder. Both remaining halves are written up in the code and filed: #13905 (the registry conflation) and #13904 (the shipped plugin provider absorbing one layer out, which is why this repair reaches hosts that wire their own provider but not yet the shipped single-kernel wiring).Mandatory enumeration — does this absorb pattern have further consumers?
Yes, and they are named rather than fixed: filed as #13906 (tenancy posture and the ADR-0069 auth gate, both authorization inputs; plus the settings seam, localization only).
Answered by mechanical enumeration with a positive control, not by impression: the criterion was run over the merge base and this head, and at the merge base it flags the objectql provider branch — the known-positive this card exists to repair — proving the criterion detects the shape it is looking for. 10 absorb seams before, 9 after, 1 kept-apart after. The auth-service seams and the
getSessionswallow collapse the same way but are already recorded as #13255 and are not re-filed.Tests
packages/rest/src/package-door-execctx-fault-reachability.test.tsis inverted in place, not re-baselined, with each superseded assertion quoted beside its replacement — the idiom the file already uses for #13279 and #13280.DATA_ENGINE_UNRESOLVABLEmoves from thegrants/FORBIDcohort to theloud/UNAVAILABLEone, and two pins are added: the two facts driven side by side and asserted to differ (each named, so breaking the innocent shape would not satisfy it), and a pin that a provider resolvingundefinedstays on the quiet path.pnpm --filter @objectstack/rest test— 164 files / 2766 tests passedpnpm --filter @objectstack/rest typecheck— green;check:test-typecheck: OK. It caught a real TS6133 in the edited test on the first run, which is the positive control that this package's test layer is genuinely inside the checked zone.packages/coreauthz-store-unavailable.test.ts+assemble-execution-context.test.ts(they scanrest-server.tsfrom disk) — 244 passedpackages/qa/dogfoodauthz-probe-blind-spot(33) andauthz-conformance(47) — both read this file and count registrars; greenAblation — direction predicted in writing before running
Predicted RED, more failures, at least 4, with the two facts becoming identical again; predicted to stay green: the "provider resolves
undefined" pin and the settings (200) / auth (401) legs. Observed: 7 failed / 35 passed, and every predicted-green pin stayed green.Mutation proven on disk before measuring — anchor
await wiredEngineOrLoud(1 → 0, injected form 0 → 1, blob hash902319cd→cd5ab431, plus a re-read of the mutated line at measurement time. The landing check was chosen for discriminating power: the bare tokenwiredEngineOrLoudoccurs 6 times (doc comments) and counting it would have proved nothing. Restore proven by hash equality with theHEADblob, emptygit diff HEADand cleangit status --porcelain.trap … EXITlived in a child script, so it restored the tree when that script exited — before vitest started. It was re-run with mutation, measurement and restore in one process.Clause ②: yes
Measured as suggested — the package built twice at the same head, with and without the change:
dist/index.d.tsidentical (the helper is module-private, no type surface moves) anddist/index.jsdiffers, which is the control proving the rebuild picked the change up. Nothing is newly accepted (both answers are refusals), but the declared ADR-0112 code on a public door changes for a real deployment condition, so it is declared yes rather than talked down. The PR stays draft and is not armed.Changeset
@objectstack/rest: minor— a runtime behaviour change on a public door, matching the convention #13279's own changeset names for this class ("shipped asminorunder the repo's launch-window convention"). Graded up rather thanpatchon the seat rule: an understated behaviour change slipping into a patch release is the worse failure mode; an inflated version is release noise. The ADR-0087 disposition is answered in the changeset (no metadata surface in either direction;SERVICE_UNAVAILABLEis an existingStandardErrorCode).Gates
Union derived with
scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsafter the last edit, re-derived after the census regeneration added a docs path (37 → 59 families), and stable at 59 on the final commit. Exit codes captured before any pipe.union named 59, ran 59, unreconciled 0 (
comm -23, exact comparison; the reverse direction is empty too). Gate union and the runs below are onc5908bf659.Five non-zero, each in the gate's own words:
check-system-context-census— was a real red caused by this change (the added lines shiftedrest-server.tsanchors). A control at the merge base was green, confirming it was mine. Regenerated with--fix, never hand-edited: 16 flagged / 10 rewritten (the 16 were 6 site-without-a-row + 8 anchor-is-not-a-read-site + 2 ledger-row-unused, describing 10 distinct rotted anchors). Re-run on the final head:OK — 109 elevation read sites … 145 anchors resolve.check-test-completeness(exit 3) — NOT MEASURED: "the local reading for this gate is NOT MEASURED. ⛔ It is not a red, and there is nothing here to fix."check:dual-build-cjs-loads(exit 3) — NOT MEASURED: "PREREQUISITE NOT MET — this gate reads built output… ⛔ This is NOT a pass: nothing was measured."check:type-check-debt(exit 1) — NOT MEASURED: "--re-measurecannot run: 31 workspace dependenc(ies) … have no built type entry point on disk … measuring now would not fail, it would silently measure a DIFFERENT WORLD." A control at the merge base refuses identically (56 there vs 31 here), so it is prerequisite-driven, not change-driven.check:skill-examples(exit 1) — NOT MEASURED:packages/client-react/distis unbuilt, and the gate refuses because "a verdict now … would reach its conclusion without ever reading the declarations under test — a FALSE GREEN".Ratchet families re-run on the final commit after the last push: census,
check:type-check-coverage,check:test-source-alias,check:cross-package-test-inputs,check:authz-resolver,check:route-envelope— all exit 0.execctx-consumer-censusdid not move: applying its ownsiteTable()predicate to both trees gives 75resolveExecCtxsites / 22 caught / 53 uncaught, identical on both sides, so there was nothing to regenerate.Generated by Claude Code