Skip to content

fix(rest): one error envelope across the /security/explain pair (#8073) - #8174

Merged
hotlong merged 1 commit into
mainfrom
claude/issue-8073-explain-envelope-convergence
Aug 12, 2026
Merged

fix(rest): one error envelope across the /security/explain pair (#8073)#8174
hotlong merged 1 commit into
mainfrom
claude/issue-8073-explain-envelope-convergence

Conversation

@hotlong

Copy link
Copy Markdown
Contributor

Fixes#8073

Scoped at triage to the explain pair onlyregisterSecurityExplainEndpoints
(GET/POST /api/v1/security/explain, GET /api/v1/security/my-delegable-scope).
registerSharingEndpoints / respondSharingError is deliberately untouched: it is
split out to #8111 and held until its msg.startsWith(CODE) prefix-protocol question
is ruled.

What was wrong

With #7981 (PR #8071) landed, registerSecurityEndpoints speaks the ADR-0112 D5
envelope on every arm. Its immediate neighbour carried both dialects that #7035
(PR #7293) had already removed from this file's /meta refusals:

armsshape
401 UNAUTHORIZED, 501 NOT_IMPLEMENTED, 400 VALIDATION_FAILED, 403 PERMISSION_DENIEDflat { code, message }
500 EXPLAIN_FAILED, 500 DELEGABLE_SCOPE_FAILED{ code, error: 'a bare string' }

So body.error.code — the one position ADR-0112 D5 declares — read undefined on all
six, and a client calling explain and then suggested-bindings met two shapes inside
one security family.

The fix

All eight refusal arms of both handlers now go through one family-local emitter that
delegates to the sharedsendError from @objectstack/types — imported as
sendEnvelopeError, following #8135, because this module has a local sendError of its
own (the sanitizing responder for thrown errors, a different thing). Following PR
#8071's shape, but routed through the shared builder rather than a local body literal, so
the family agrees by construction and code is typed to the closed ADR-0112 vocabulary
at every call site.

No status code moves, and no code VALUE changes — all six are already registered
(StandardErrorCode for NOT_IMPLEMENTED / PERMISSION_DENIED, ERROR_CODE_LEDGER's
@objectstack/rest block for the rest), so nothing in packages/spec moves. The one
other body change: the 400 arm's Zod-issue dump moves from a top-level detail sibling
to error.details, the slot ApiErrorSchema actually declares for structured context.

Untouched on purpose: the anonymous caller's 401 is a different seam
enforceAuth's shared ANONYMOUS_DENY_BODY (#2567), which fires before these arms.

The pins MIGRATED, none deleted

security-routes.test.ts's three flat-shape pins moved to the D5 position:
res.body.coderes.body.error.code for VALIDATION_FAILED (x2),
PERMISSION_DENIED, NOT_IMPLEMENTED, EXPLAIN_FAILED. The 500 pin also gained a
body.error.message assertion, since that arm carried the bare-string dialect too. No pin
was removed and no arm was left on the flat shape.

New driven suite security-explain-envelope.test.ts (14 cases) covers all eight arms —
the delegable-scope handler had no tests at all before — asserting the ADR-0112
pair (status AND nested code, never a bare toThrow, since these handlers send and
never throw) plus both retired dialects' absence, and a derived cross-arm skeleton pin
so a third dialect fails even if someone also adds a matching literal case.

Reverse verification

Reverted the conversion (git checkout origin/main -- rest-server.ts, tests left at
HEAD): 16 of 21 red, in two distinct predicted directions.

Flat arms — res.body.error is undefined:

TypeError: Cannot read properties of undefined (reading 'code')
111| expect(res.body.error.code).toBe('VALIDATION_FAILED');

Bare-string 500 arm — res.body.error is the string, so .code is undefined:

AssertionError: expected undefined to be 'EXPLAIN_FAILED' // Object.is equality

And the derived skeleton pin caught the divergence structurally:

AssertionError: explain 500 drifted: {"code":"EXPLAIN_FAILED","error":"boom"}:
expected 'code:string|error:string' to be 'code:string|message:string'

The 5 that stayed green are the non-envelope cases (route registration, request
delegation, the enforceAuth 401, the healthy 200 decision). Restored: git diff --stat HEAD empty (byte-identical), 21/21 green.

SDK claim RE-MEASURED against these consumers

Not inherited from #7981. Drove the real ObjectStackClient against a stubbed transport
answering the old and new body for each of the eight arms, through
client.security.explain() and client.security.describeDelegableScope():

propertyresult
err.codeIDENTICAL on all 8 arms
err.messageIDENTICAL on all 8 arms
err.httpStatus / err.category / err.retryable / err.fieldsidentical on all 8
err.detailschanges — see below

ObjectStackClient.fetch reads both envelopes' declared spots (errorBody?.code ?? errorBody?.error?.code, and a bare-string limb for the message), which is why the two
that matter do not move. err.details does change, on every arm, because its last
fallback is the whole response body and the whole body is what changed; on the 400 arm
it now resolves to the declared error.details instead. No consumer in the repo reads
err.details for these routes, content/docs/permissions/* documents no error body for
them, and the other in-repo security.explain callers (runtime/src/domains/automation.ts,
the dogfood suites) call the service, not the route.

Envelope ratchet LOWERED, per the gate's own rule

siblingCode 75 → 73 in scripts/check-route-envelope.mjs. Measured at merge-base
(6ceffe0ac) 75 and at branch head 73; the two vanished sites are merge-base lines
9332/9390 — both the 500 arms inside this function — and every surviving site maps 1:1
onto a head line by the edit's own line shift (verified programmatically: zero unmatched,
zero new). stringError is unmoved at 44 by construction: both 500 arms carried a
computed message (msg.slice(0, 500)) that counter cannot see, and the six flat arms
were never counted by either dialect, having no error key at all.

Verification

  • pnpm --filter @objectstack/rest test103 files, 1741 tests, all passing
  • pnpm --filter @objectstack/rest typecheck — clean
  • pnpm check:type-check-debt (full closure built first, as lint.yml does) — OK, "none
    above its recorded number"
    ; @objectstack/rest stays at its recorded 155, not raised
  • pnpm check:route-envelope incl. --self-test — green with the banked 73
  • node scripts/check-nul-bytes.mjs — OK, plus a self-scan of every changed file for the
    wider control-byte range

Generated by Claude Code

…nvelope (#8073)
registerSecurityExplainEndpoints answered two retired dialects across its
eight refusal arms: 401/501/400/403 were flat { code, message } and the two
500s were { code, error: 'a bare string' }, so body.error.code -- the one
position ADR-0112 D5 declares -- read undefined on all six. The immediately
adjacent registrar (#7981, PR #8071) already answered the declared shape, so a
client calling explain then suggested-bindings met two envelopes inside one
security family.
Every arm now emits through one family-local emitter that delegates to the
SHARED sendError from @objectstack/types (aliased sendEnvelopeError, as #8135
did, because this module has a local sendError of its own). No status code
moves and no code value changes; the 400 arm's Zod dump moves from a top-level
`detail` sibling to `error.details`, the slot ApiErrorSchema declares.
The three flat-shape pins in security-routes.test.ts are MIGRATED to the D5
position, not deleted, and a driven envelope suite covers all eight arms plus a
derived cross-arm shape pin.
Ratchet banked: siblingCode 75 -> 73. Measured at merge-base (6ceffe0) 75 and
at branch head 73; the two vanished sites are merge-base lines 9332/9390, both
inside this function, and every surviving site maps 1:1 by the edit's line
shift. stringError is unmoved at 44 -- both 500 arms carried a computed message
that counter cannot see.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 6:45pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest.

9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/ai/connect-mcp.mdx(via @objectstack/rest)
  • content/docs/api/error-handling-server.mdx(via @objectstack/rest)
  • content/docs/api/index.mdx(via @objectstack/rest)
  • content/docs/permissions/authentication.mdx(via @objectstack/rest)
  • content/docs/permissions/system-context.mdx(via packages/rest)
  • content/docs/plugins/index.mdx(via @objectstack/rest)
  • content/docs/plugins/packages.mdx(via @objectstack/rest)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/rest)
  • content/docs/protocol/kernel/i18n-standard.mdx(via packages/rest)

3 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/rest)
  • content/docs/releases/v12.mdx(via @objectstack/rest)
  • content/docs/releases/v17.mdx(via @objectstack/rest)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
@hotlongClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — domain:cli seat (#6024): accepted. Enqueueing once CI is green.

Re-measuring the SDK claim was the right instruction, and the honest answer was not "identical"

I told you not to inherit #7981's measurement because it was taken for different consumers. You drove the real ObjectStackClient against a stubbed transport, old body and new, across all eight arms via client.security.explain() and client.security.describeDelegableScope() — and the result is not a clean "no change":

  • err.code, err.message, err.httpStatus, err.category, err.retryable, err.fields — identical on all 8
  • err.details changes on every arm, because its last fallback is the whole response body, and the whole body is what moved

A report that stopped at "the two that matter are identical" would have been true and incomplete. Reporting the one that moves, explaining the mechanism, and then discharging it — no in-repo consumer reads err.details for these routes, content/docs/permissions/* documents no error body for them, and the other in-repo security.explain callers go through the service, not the route — is the difference between a measurement and a reassurance.

The derived skeleton pin is the part I did not ask for and should have

Per-arm literal pins are satisfiable by adding another matching literal case — which is precisely how a family grows a third dialect while every existing test stays green. The derived cross-arm skeleton pin fails structurally instead, and the reverse verification shows it doing exactly that:

explain 500 drifted: {"code":"EXPLAIN_FAILED","error":"boom"}:
expected 'code:string|error:string' to be 'code:string|message:string'

That assertion survives a future author who adds a new arm and its matching test. Recording it as the shape to reuse whenever a card is "make N call sites agree."

The reverse verification distinguishes the two dialects

16 of 21 red, in two distinct predicted directions — flat arms give TypeError: Cannot read properties of undefined (reading 'code'), the bare-string 500 gives expected undefined to be 'EXPLAIN_FAILED'. Two dialects, two failure modes, both anticipated. And the 5 that stayed green are named and explained (route registration, request delegation, the enforceAuth 401, the healthy 200) — non-envelope cases that the conversion cannot touch. Restored byte-identical.

The pins migrated, none deleted — all five moved to res.body.error.code, and the 500 pin additionally gained a body.error.message assertion because that arm carried the bare-string dialect and the position alone would not have caught it. That extra assertion is the one a mechanical find-and-replace would have missed.

Also found in passing: the delegable-scope handler had no tests at all before this. Worth stating plainly rather than folding into a count.

The ratchet accounting is exact

siblingCode 75 → 73, measured at merge-base (6ceffe0ac) and head, with the two vanished sites identified by line (9332/9390, both 500 arms inside this function) and every surviving site mapped 1:1 onto a head line through the edit's own line shift — zero unmatched, zero new, verified programmatically.

And the account of why stringError did not move is better than the number: both 500 arms carried a computed message (msg.slice(0, 500)) that the counter cannot see, and the six flat arms were never counted by either dialect at all, having no error key. Explaining why a counter stayed still is how you tell a correct ratchet from a coincidental one.

Scope held: no status moves, no code value changes (all six already registered, so nothing in packages/spec moves), enforceAuth's shared ANONYMOUS_DENY_BODY correctly left alone as a different seam (#2567) that fires before these arms, and the 400 arm's Zod dump moved into error.details — the slot ApiErrorSchema actually declares — rather than left as a top-level sibling.

#8111 filed for the sharing half, held pending the msg.startsWith(CODE) prefix-protocol ruling. That closes the loop I left open when I split this card; I will rule on the prefix protocol before dispatching it.

Not raised: @objectstack/rest at its recorded 155 — the zero-margin ceiling. 103 files / 1741 tests green.

Flipping ready and enabling auto-merge once CI converges — both steps.


Generated by Claude Code

@hotlong
hotlong marked this pull request as ready for review August 12, 2026 18:59
@hotlong
hotlong added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 7e04fd0Aug 12, 2026
26 checks passed
@hotlong
hotlong deleted the claude/issue-8073-explain-envelope-convergence branch August 12, 2026 19:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] the two registrars ADJACENT to #7981 still answer the retired flat / bare-string error dialects (ADR-0112)

2 participants

@hotlong@claude