Skip to content

fix(rest): honour a hook refusal's declared statusCode at the /api/v1/data error boundary (#7525) - #7575

Merged
os-help merged 1 commit into
mainfrom
claude/issue-7525-hook-refusal-status-passthrough
Aug 11, 2026
Merged

fix(rest): honour a hook refusal's declared statusCode at the /api/v1/data error boundary (#7525)#7575
os-help merged 1 commit into
mainfrom
claude/issue-7525-hook-refusal-status-passthrough

Conversation

@os-help

Copy link
Copy Markdown
Collaborator

Fixes#7525

Where the status was lost

The card asked for the seam, not another widening of the passthrough. It is one line.

mapDataError — the error exit for the ~11 CRUD data routes, which bypass resolveErrorResponse entirely — opened its explicit-status passthrough on:

if(typeoferror?.status==='number'&&error.status>=400&&error.status<600){

An engine lifecycle hook declares its status as statusCode:

// packages/plugins/plugin-approvals/src/lifecycle-hooks.tserr.code='RECORD_LOCKED';err.statusCode=409;// :122 a pending lockRecord approvalerr.code='FORBIDDEN';err.statusCode=403;// :440 a delegation row the caller does not own

So the refusal never entered that branch at all. It fell past every structured branch (DELETE_RESTRICTED, VALIDATION_FAILED, PERMISSION_DENIED, …), matched no message heuristic, and left through UNCLASSIFIED_FAULT as 500 INTERNAL_ERROR with no code — for a deliberate, well-understood business refusal.

#5582 (PR #7402) is not the fix and could not have been. It widened this same branch's range (4xx → 400-599) for producers that declared status. The loss here is one question earlier: whether a status was declared. Both QA cases carry a 4xx status anyway (409 / 403), so they were already inside #5582's old range — the range was never what excluded them.

The fix, and why the boundary rather than the two hooks

statusstatusCode → default is what every other HTTP exit in this repo already reads:

exitread
runtimeHttpDispatcher.errorFromThrown (#3867).status.statusCode → 500
runtimedispatcher-plugin.errorResponseBase:494.status.statusCode → 500
runtimeendpoint-executor:299, domains/actions:436same
plugin-hono-servercurrent-user-endpoints:182.statusCode
restmapDataError.status only ← the outlier

That is why one thrown error answered 403 through a dispatcher route and 500 through /api/v1/data. The gate is now a named declaredHttpStatus(error) helper asking the same 400-599 band over both spellings.

Teaching the two approvals hooks to spell it status would fix two producers and leave the boundary answering 500 for the next one — including runtime's own action-execution.ts ({ statusCode: 503 | 501 | 400 }) and metadata-protocol ({ statusCode: 404 }), both of which reach these routes. The hooks are unchanged.

Scope, per the serial constraints on rest-server.ts

Error-mapping region only. packages/runtime/src/http-dispatcher.ts is not touched — the analysis did not put the fix there; that dispatcher already reads both spellings correctly, which is precisely what made the REST face's single-spelling read visible as a divergence. The reports-delete handler and /meta route registration are untouched.

What deliberately did NOT change

  • declaresServerFault's own read is still status-only./analytics/query 仍把 RLS 策略字段名回显给调用方 —— read-scope 拒收的泄漏在姐妹面上没堵,#5367 只堵了 dataset 路由 #5811 ruled that a disclosure rule must not depend on a producer's spelling; untouched. This is status resolution, a different question. The one call site inside the passthrough is handed the status the boundary just resolved (declaresServerFault({ status: declaredStatus, code: error?.code })) — otherwise a { statusCode: 503, code } producer would take the 5xx arm and then be told it declared no fault, shipping the status with its ADR-0112 code silently dropped.
  • The 5xx withhold is unconditional as before — generic sentence plus the code, full text to the operator log.
  • A hook that declares NO status is unchanged — still the terminal sanitised 500 INTERNAL_ERROR. Promoting a bare code to a 4xx would be consumer-side leniency (that is QA run · api-backend (FULL area) · a86db175 · 2026-08-10 · 6 PASS / 2 PARTIAL / 3 FAIL #7463's family, not this one).
  • The structured branches keep their precedenceOBJECT_NOT_FOUND / DELETE_RESTRICTED / VALIDATION_FAILED still sit above the passthrough and still win, statusCode or not.
  • resolveErrorResponse still reads status only — it delegates to mapDataError for everything it does not pass through, so both doors already give one wire answer without a second copy of the two-spelling read.

Behaviour change

requestbeforeafter
PATCH /api/v1/data/{obj}/{id} on a record with a pending lockRecord approval500 INTERNAL_ERROR409 RECORD_LOCKED + the refusal's own guidance
POST /api/v1/data/sys_approval_delegation, delegator_id ≠ caller500 INTERNAL_ERROR403 FORBIDDEN + the refusal's own guidance

Monitoring that counted these as server faults sees a 5xx disappear and a 4xx appear; a client retrying on 5xx stops retrying a request that can never succeed. Both are stated in the changeset rather than buried.

Tests

packages/rest/src/rest-hook-refusal-status-passthrough.test.ts — 26 cases:

Mutation table (measured, not predicted)

#mutationred
Abaseline: the file against unmodified main (≡ deleting the ?? error.statusCode limb)15/26 — §1 6/8, §2 4/4, §3 0/3, §4 5/6, §5 0/6
BdeclaredHttpStatus returns 400 for an undeclared error4 — §3 ×2, §5 ×2
Cthe passthrough outranks the structured branches; sandbox unwrap removed4 — §3 ×1, §5 ×3
Dband check dropped + precedence inverted + declaresServerFault call deleted4 — the exact four A left green

Union covers all 26; no case is unfalsifiable. Two predictions were wrong and are recorded as measured in the file header: §1 was predicted 7/8 red (measured 6/8 — two cases are direction-insensitive by construction), and §4 was predicted 2/6 (measured 5/6 — the empty/numeric-code cases fail on their status before their code assertion is the interesting half).

Gates

  • packages/rest83 files / 1367 tests pass
  • packages/plugins/plugin-approvals20 files / 452 tests pass (5 initial failures were unbuilt workspace deps, service-automation and trigger-record-change; green after building them)
  • tsc --noEmit on @objectstack/rest — clean
  • eslint --no-inline-config on both changed files — clean
  • Changeset: .changeset/rest-hook-refusal-status-passthrough.md (patch, @objectstack/rest)
  • content/docs/releases/** untouched

🤖 Generated with Claude Code

https://claude.ai/code/session_01FGZ4X1ezVSLg8E9NvJitAE


Generated by Claude Code

…/v1/data` error boundary (#7525)
A write refused by an engine lifecycle hook that declared an explicit status
came back as an opaque `500 INTERNAL_ERROR` with no `code`, while the server log
held the correctly-shaped refusal. Two cases, each reproduced 2x by QA:
`PATCH` a record with a `lockRecord:true` approval pending (log: `RECORD_LOCKED`)
and `POST sys_approval_delegation` with a `delegator_id` the caller does not own
(log: `FORBIDDEN ... statusCode 403`).
The seam is `mapDataError`'s explicit-status passthrough, which opened on
`typeof error.status === 'number'` and nothing else. The hooks declare their
status as `statusCode`, so the refusal never entered that branch at all: it fell
past every structured branch, matched no message heuristic, and left through
`UNCLASSIFIED_FAULT`. #5582 widened the same branch's RANGE (4xx -> 400-599) and
could not have covered this — the status was lost one question earlier.
Fixed at the boundary, not at the two hooks: `status` -> `statusCode` -> default is
already what every other HTTP exit in this repo reads (`errorFromThrown`,
`errorResponseBase`, `endpoint-executor`, `domains/actions`, plugin-hono-server),
so one thrown error answered 403 through a dispatcher route and 500 through
`/api/v1/data`. The gate becomes a named `declaredHttpStatus()` asking the same
400-599 band over both spellings. `declaresServerFault`'s own `status`-only read
is unchanged (#5811 ruled a DISCLOSURE rule must not depend on a spelling); the
one call site inside the passthrough is handed the status just resolved, so a
`{ statusCode: 5xx, code }` producer cannot ship its status with the code dropped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGZ4X1ezVSLg8E9NvJitAE
@vercel

vercelBot commented Aug 11, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 11, 2026 5:33am

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.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rest] Hook refusals carrying an explicit statusCode are not mapped by /api/v1/data — they leak as HTTP 500 INTERNAL_ERROR with no located guidance

1 participant

@os-help