Skip to content

test(rest): pin the six approvals wire-code rows with no live emission - #15354

Merged
os-litant merged 2 commits into
mainfrom
claude/issue-14849-approval-error-wire-code-pins
Sep 4, 2026
Merged

test(rest): pin the six approvals wire-code rows with no live emission#15354
os-litant merged 2 commits into
mainfrom
claude/issue-14849-approval-error-wire-code-pins

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #14849

handleApprovalError (packages/rest/src/rest-server.ts, a closure inside registerApprovalsEndpoints) maps nine message prefixes to (status, code). This adds a live-emission pin for the six rows that had none — driven through a real route, never asserted off the mapping table.

rest-server.ts is untouched. The diff is one test file. The door was mutated only transiently for the ablation, under a restore trap, and restored byte-for-byte (hash compared against the HEAD blob) before anything was committed.

Method — every row confirmed by ABLATION, not by grep

This card exists because a file-scoped grep got exactly this wrong once already (#14573 read FORBIDDEN as unpinned; the pin was in rest-data-door-code-prefix.test.ts §7, a file whose declared subject is the #13095 strip). So no row here was judged by grep.

The instrument: corrupt the row's regex prefix (/^CODE/ becomes /^ZZABLATED_CODE/) so the row can never match while its literal text stays in the file. A red under that mutation is therefore an observation of live emission, never of literal presence — which is the property the card is about. Then run the whole 176-file packages/rest suite, one row at a time.

Every leg proved the mutation reached disk before anything was measured (injected token counted at 1, original counted at 0), and every restore was proved by git diff HEAD empty plus a hash-object match against the HEAD blob.

Per-row result (measured 2026-09-04)

row status ablation red verdict
THROTTLED 429 1 already pinned — rest-approvals-wire-codes.test.ts
FORBIDDEN 403 3 already pinned — 1 here + BOTH §7 cases in rest-data-door-code-prefix.test.ts
READ_BACK_FAILED 500 1 already pinned — rest-approvals-wire-codes.test.ts
VALIDATION_FAILED 400 0 genuinely uncovered — pinned by this PR
DUPLICATE_REQUEST 409 0 genuinely uncovered — pinned by this PR
INVALID_STATE 409 0 genuinely uncovered — pinned by this PR
REQUEST_NOT_FOUND 404 0 genuinely uncovered — pinned by this PR
RESUME_TARGET_LOST 409 0 genuinely uncovered — pinned by this PR
RESUME_FAILED 500 0 genuinely uncovered — pinned by this PR

The FORBIDDEN leg is the instrument's positive control. It reproduces the three reds the #14573 correction measured — two of them in a different file, on a different declared contract — so the same instrument, returning zero for the other six, is returning zero with demonstrated resolving power. This is the check the card asked for, and it passes.

⭐ Correction to the card: the prediction did not hold

The card and its triage both predicted that at least one of the six would turn out already covered somewhere unobvious, and said finding that would be a result. Measured: none of them was. All six were genuinely bare, so the card's table was right as a worklist even though it was only a hypothesis when written. That prediction is now answered by measurement instead of being carried forward as a caveat — reported here because "the expected surprise did not happen" is as much a result as the surprise would have been.

Two supporting screens, so "repo-wide" is measured rather than argued:

  • Consumer side, through dist. All nine rows ablated, @objectstack/rest rebuilt (dist marker preflight: the ablation token present 9 times in dist/index.js and dist/index.cjs), then the dependent packages' tests run: @objectstack/client 214 passed, @objectstack/runtime 303 passed, @objectstack/plugin-auth 11 passed. No consumer-side pin exists. Three further files were NOT MEASURED there (@objectstack/runtime had no built entry point at that moment): client/src/analytics-automation-json-erasure.test.ts drives only POST /analytics/dataset/query, client/src/meta-delete-item-carriers.test.ts drives meta routes, and verify/src/automation-trigger-paused-run.test.ts drives no route at all — none can address the approvals door.
  • Reachability. The door is a closure with four call sites, all inside registerApprovalsEndpoints, reachable only through POST .../approvals/requests/:id/ACTION. Only six packages depend on @objectstack/rest (cli, client, plugin-auth, plugin-dev, runtime, verify) and no test in any of them addresses that path. packages/plugins/plugin-approvals mentions handleApprovalError only in prose — it does not depend on @objectstack/rest and cannot observe it; its RESUME_FAILED / INVALID_STATE / DUPLICATE_REQUEST hits assert the throw-site prefix, a different fact.

What was added

Six cases in packages/rest/src/rest-approvals-wire-codes.test.ts, each asserting the ADR-0112 minimum — code and status — plus the #13095 strip and closed-union membership against ApiErrorSchema.

Each drives a different route, so the six also cover all four catch arms that call the door (decisionRoute, the recall route, flowMoveRoute, threadRoute):

row route driven real throw site mirrored
VALIDATION_FAILED 400 reassign approval-service.ts reassign()
DUPLICATE_REQUEST 409 resubmit approval-service.ts resubmit()
INVALID_STATE 409 recall approval-service.ts recall()
REQUEST_NOT_FOUND 404 revise sendBack() then loadPendingRow()
RESUME_TARGET_LOST 409 approve assertRunResumable() via decide()
RESUME_FAILED 500 reject resumeRecordedOutcome() via decide()

Reverse verification, run after the implementation was committed so the restore leg had the implementation to restore to: ablating each of the six reds exactly its own new case and nothing else. Five red on status (expected 500 to be 400 / 409 / 409 / 404 / 409). RESUME_FAILED reds on the code instead — expected 'APPROVAL_REJECT_FAILED' to be 'RESUME_FAILED' — because the degraded shape is a 500 too. That is exactly why ADR-0112 asks for both halves, and the case says so in place.

Verification at f334fcfe

  • pnpm --filter @objectstack/rest exec vitest runTest Files 176 passed (176), Tests 2993 passed (2993).
  • pnpm --filter @objectstack/rest run typecheck — exit 0; check:test-typecheck: OK — @objectstack/rest's test layer compiles under packages/rest/tsconfig.test.json. Coverage is real, not phantom: tsc -p packages/rest/tsconfig.test.json --listFiles lists the edited file.
  • pnpm lint — repo-wide (eslint . --no-inline-config), exit 0. No narrowing was needed.
  • Gate union derived at this HEAD on a clean tree with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (35 commands), plus the four artifact-roster gates it flags as keeping their roster under packages/. 37 of 39 exit 0. The two exceptions are NOT MEASURED, not red — both print PREREQUISITE NOT MET and exit 3 because they read built output the worktree does not have: pnpm check:dual-build-cjs-loads and pnpm check:type-check-debt (both want turbo run build over the whole workspace). Neither can be moved by this diff in any case — the only changed file is a *.test.ts, and @objectstack/rest publishes dist alone.
  • Six further families this card's path reaches carry a workflow variable ($RUNNER_TEMP, $MERGE_BASE) and have no local invocation; they run in CI.

skip-changeset: the diff adds one test file and publishes nothing from any package.


Generated by Claude Code

`handleApprovalError` maps nine prefixes to (status, code). Ablation — each
row's regex corrupted so it can never match, the whole 176-file
`packages/rest` suite run per row — measured which rows anything actually
observes: THROTTLED 1 red, FORBIDDEN 3, READ_BACK_FAILED 1, and zero for
VALIDATION_FAILED, DUPLICATE_REQUEST, INVALID_STATE, REQUEST_NOT_FOUND,
RESUME_TARGET_LOST and RESUME_FAILED.

Adds a live-emission pin for each of those six, driven through a real route
rather than asserted off the mapping table, each asserting the ADR-0112
minimum (`code` AND `status`) plus the #13095 strip and closed-union
membership. The six use six different routes, so they also cover all four
catch arms that call the door.

`rest-server.ts` is untouched: this pins rows that already exist.

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

github-actions Bot commented Sep 4, 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 c4d1354e32539eb418e35757ccbafb3dea57ba93packageMentionDocs.

@os-litant
os-litant marked this pull request as ready for review September 4, 2026 13:17
@os-litant
os-litant enabled auto-merge September 4, 2026 13:18
@os-litant
os-litant added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 226e724 Sep 4, 2026
39 checks passed
@os-litant
os-litant deleted the claude/issue-14849-approval-error-wire-code-pins branch September 4, 2026 13:47
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 tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] 6 of handleApprovalError's 9 wire-code rows have no live-emission pin — counted, not fixed

2 participants