Skip to content

test(rest): pin the approvals FORBIDDEN → 403 row here too — and correct the record that it was unpinned - #14838

Merged
os-project-manager merged 3 commits into
mainfrom
claude/issue-14573-approvals-forbidden-live-emission-pin
Sep 3, 2026
Merged

test(rest): pin the approvals FORBIDDEN → 403 row here too — and correct the record that it was unpinned#14838
os-project-manager merged 3 commits into
mainfrom
claude/issue-14573-approvals-forbidden-live-emission-pin

Conversation

@os-trump

@os-trump os-trump commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Part of #14573.

⚠️ The card's premise is falsified, and that is this PR's main finding. #14573 says the
FORBIDDEN → 403 row of handleApprovalError "has no live-emission pin — only the
service-side prefix is pinned". It has one. It has two, in fact, and they discriminate.
Read the ablation below before deciding what to do with this branch.

File face

packages/rest/src/rest-approvals-wire-codes.test.tsone file, test-only, no production
code touched.
Specifically NOT packages/rest/src/error-response.ts, which PR #14713 is in
flight on and card #14725 is queued on. handleApprovalError lives in rest-server.ts; this
branch reads it and never edits it.

Anchors, verified against origin/main 521eaf9e41

Card's claim Verdict
handleApprovalError at rest-server.ts:11557 ✅ exact
[/^FORBIDDEN/, 403, 'FORBIDDEN'], is row 5 of the table ✅ exact, :11564
rest-approvals-wire-codes.test.ts greps 0 for FORBIDDEN|403 ✅ (positive control: THROTTLED → 6)
the row has no live-emission pin FALSE — see below
a dropped row fails closed at 500 ✅ confirmed by the ablation's message

The falsification, measured

rest-data-door-code-prefix.test.ts §7 (describe('[#13095] the approvals door strips the code it answers, never a blanket pattern')) boots a RestServer with an approvals service
whose decide rejects, drives the real POST /api/v1/approvals/requests/:id/approve
route, and asserts status === 403, body.code === 'FORBIDDEN' and the strip. That is
exactly the shape the triage comment specified, already in the tree, on a different route.

The card enumerated that file and characterised it as pinning "the code-prefix strip … not the
403 row". That characterisation is wrong, and the triage re-ran the grep only against
rest-approvals-wire-codes.test.ts, so it inherited the error.

Ablation — delete the row from rest-server.ts, no rebuild (subject reached by a
./rest-server.js relative in-package import, which vitest transforms from source; the red
below is the proof that source is what ran):

anchor occurrences BEFORE=1  bytes=742980
anchor occurrences AFTER=0   bytes=742930  delta=50
HEAD blob ce00225a072be73a12eedd7143c5f50fa2a901c6
MUT  blob a6d7d5d00968850eff63f1430a1dccc53885454a   ← differs ⇒ mutation on disk
residual /^FORBIDDEN/ rows in table: 0
Leg Result
baseline (tree at HEAD) Tests 27 passed (27)
mutated Tests 3 failed | 24 passed (27)
restored Tests 27 passed (27)

The three reds, all reading AssertionError: expected 500 to be 403:

  • rest-approvals-wire-codes.test.ts → the case this PR adds
  • rest-data-door-code-prefix.test.ts §7 → the well-formed idiom is unchanged…pre-existing
  • rest-data-door-code-prefix.test.ts §7 → a LONGER token sharing the matched spelling…pre-existing

24 neighbours stayed green, including every other case in both files — the red is specific
to the row, not a file-wide collapse.

Restore proven by blob-hash equality, not by an exit code:
POST blob ce00225a072be73a12eedd7143c5f50fa2a901c6 = HEAD blob, and git diff HEAD -- PATH
empty. Mutation and restore both ran under trap restore EXIT INT TERM on absolute paths.

What is actually true, and what this PR does

The row is pinned. What was not pinned is narrower and still real: the file that owns the
approvals wire-code contract carried no FORBIDDEN case, so an audit of approvals wire codes
read a gap that a strip-contract file was silently covering. §7's two cases sit under a
describe whose declared subject is #13095's anchored strip — retire or refactor that contract
and the 403 row loses its live pin with nobody noticing.

This PR adds one it() closing that locality gap, through the real recall route with the
real catalog refusal sentence, asserting 403 + code + the strip — and, in the same commit,
names §7 from the new test so the duplicate is labelled in both directions. An unlabelled
duplicate is what got this card mis-filed; a labelled one cannot.

This is a disposition call for the maintainer, not a done card. Three defensible answers:
land this as the labelled pin in the owning file; drop the it() and keep only a pointer to §7;
or close #14573 as already-covered. My recommendation is the first — one row silently pinned by
two files that do not know about each other is the shape that produced this card.

⛔ Not done here

  • No production change. The mapping table is untouched — no row added, reordered or edited.
  • No case per refusing operation (the triage forbade it, and the ablation confirms one route
    is enough: one row, one closure, all four call sites).
  • packages/spec read, never editedBUILTIN_OPERATION_MESSAGES is imported from
    @objectstack/spec/system, the same construction approval-revise.test.ts already uses.
  • Neighbouring rows not audited or fixed. handleApprovalError's table has 9 rows; 3 now
    have live-emission pins here (THROTTLED, READ_BACK_FAILED, FORBIDDEN) and 6 do not
    VALIDATION_FAILED, DUPLICATE_REQUEST, INVALID_STATE, REQUEST_NOT_FOUND,
    RESUME_TARGET_LOST, RESUME_FAILED. Counted, filed as its own card, not fixed here.

Changeset fork: skip-changeset, measured not assumed

packages/rest declares files: ["dist","README.md","CHANGELOG.md"] and the root
tsup.config.ts builds entry: ['src/index.ts'] — a src/**/*.test.ts file cannot reach
dist, so this diff publishes nothing from any released package. Label, not a changeset.

Verification

pnpm --filter '@objectstack/rest^...' build (VERDICT command-exit 0), then the targeted
vitest above. Gate families derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (27 owed by path + kind). Per-run exit codes captured before any
pipe. Full list of what ran, what was narrowed and what is left to CI is in the report on the
issue.

🤖 Generated with Claude Code

https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza


Generated by Claude Code

`handleApprovalError`'s `[/^FORBIDDEN/, 403, 'FORBIDDEN']` row is the one
every authorisation refusal rides, and it had no live-emission pin: the
service suites assert the `FORBIDDEN:` message prefix at the throw site,
which is a different fact from what the route answers on the wire.

Adds one `it()` to `rest-approvals-wire-codes.test.ts` driving the real
recall route with a service that throws the real refusal, asserting
status 403, `code === 'FORBIDDEN'`, and that the [#13095] anchored strip
removed the prefix. Losing the row fails closed (500
`APPROVAL_RECALL_FAILED` with the raw message), so the third assertion
catches the strip half of the regression as well as the status half.

Test-only; no production behaviour changed.

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

Measured, not read: deleting `[/^FORBIDDEN/, 403, 'FORBIDDEN']` from
`handleApprovalError` reds THREE cases, not one — this new pin and both
cases of `rest-data-door-code-prefix.test.ts` §7, which already drive the
real approve route and already assert 403, `code: 'FORBIDDEN'` and the
anchored strip.

#14573 was filed and triaged on the reading that the row had no
live-emission pin anywhere. That is wrong. What is true is narrower: the
file that OWNS the approvals wire-code contract did not pin it, so an
audit of wire codes here saw a gap a strip-contract file was silently
covering. Naming §7 from here is half the fix — the unlabelled duplicate
is what got the card mis-filed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
@os-trump os-trump added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed tests labels Sep 3, 2026 — with Claude
@github-actions

github-actions Bot commented Sep 3, 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 f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8packageMentionDocs.

@github-actions github-actions Bot added the tests label Sep 3, 2026
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit e79107a Sep 3, 2026
33 of 34 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-14573-approvals-forbidden-live-emission-pin branch September 3, 2026 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

3 participants