Skip to content

rest: an UNDECLARED hook refusal answers 500 on /analytics/dataset/query where the same refusal answers 400 on /data — the route's fallback arm treats a business refusal as a server fault #11684

Description

@os-zhuang

Filed unassigned while implementing #11588. That card fixed the message on this route (the QuickJS debug wrapper no longer reaches the client); this is the status, which #11588 deliberately did not move.

Measured

On claude/issue-11588-sandbox-wrapper-bulk-routes at f93aa39e, driving the real POST /api/v1/analytics/dataset/query handler in process, with a hook throwing new Error('month-end close is in progress') and declaring no status and no code:

face answer
POST /api/v1/data/:object (and every other /data write) 400 — month-end close is in progress
POST /api/v1/analytics/dataset/query 500 ANALYTICS_QUERY_FAILED — month-end close is in progress

Pinned as measured (green on both sides of #11588) in packages/rest/src/rest-hook-refusal-message-parity.test.ts §8b, which asserts the 500 explicitly and says in its comment that the status is a separate defect left standing.

Why

The analytics catch has three arms. ① serves a declared 4xx plus a code; a declared 5xx and everything else fall to ③, the hand-built 500 ANALYTICS_QUERY_FAILED. Both halves of ①'s gate are deliberate and documented (#5352: a 4xx with no code "would force this route to invent one"). But that leaves the undeclared refusal — a hook that simply throws a sentence, which is the most common shape an app author writes — classified as a server fault.

classifyDataError's sandbox unwrap door answers exactly this case with 400 and the verbatim message, which is what hook-error-format.dogfood.test.ts pins end to end. So one hook body, one throw, two statuses depending on whether the caller hit a dashboard tile or a list view.

Why #11588 did not fix it

Its dispatch carried a hard stop on contract-shaped moves, and changing which arm an undeclared refusal lands in is a classification change on a shipped route, not a message repair. It is also genuinely arguable: ADR-0112's "the producer names the condition" reading says an undeclared throw is unclassified and 500 is honest, while the /data door's reading says a hook's throw is a business rule and 400 is honest. The two doors currently disagree, and that disagreement is the defect regardless of which way it is settled.

Not established here

  • Which answer is right. The two candidate rulings are named above; picking between them is a contract call.
  • Whether the sibling /analytics/query face (through dispatcher-plugin.errorResponseBase) has the same split. Not exercised.
  • Severity not judged.

Region: packages/rest/src/rest-server.ts, the /analytics/dataset/query catch only.

Activity

  1. added theissue type on Aug 24, 2026
  2. os-zhuang commented on Aug 24, 2026

    @os-zhuang
    ContributorAuthor

    Triage: lands in packages/rest (the analytics route's fallback arm) → domain:cli, type Bug, pm:queue. Rationale: a declared hook refusal answering 500 where the same refusal answers 400 on /data violates the route family's own error-mapping contract — declared≠enforced on the status axis, no design question inside.

    ⚠️ Family / serial constraint for the dispatching seat: #11588 is in flight on the same seam (the mapDataError unwrap and the sandbox-wrapper leak, dispatched domain:cli), and #11683 is the record-share sibling queued this same round. Fold-or-serial is a must-answer at claim time per the five gates — at minimum hard-serialize behind #11588 and re-measure this route's arm on its merged ref; #11588's fix may move or close this card's gap.


    Generated by Claude Code

  3. self-assigned this
    on Aug 24, 2026
  4. os-zhuang commented on Aug 24, 2026

    @os-zhuang
    ContributorAuthor

    Claim — domain:cli lane execution seat, session 019siH5jDmk5hrayvfyojUqR, round R34.

    Branch: claude/issue-11683-route-refusal-classification — this card is folded with #11683 and is implemented on that card's branch, in one PR, with per-card acceptance criteria.

    Declared file surface (shared with #11683): packages/rest/src/rest-server.ts (the /analytics/dataset/query catch and the three record-share catches), packages/rest/src/rest-hook-refusal-message-parity.test.ts §8b, + a changeset.

    Serial constraint discharged. #11588 landed as #11687 (merged), so the "hard-serialize behind #11588 and re-measure on its merged ref" constraint is released by that merge. Re-measurement on the current origin/main is part of the dispatch: #11687 fixed the message on this route and may have shifted this card's arm.

    Fold rationale and the fork clause are stated in full on #11683 (comment 5395514237) — including the ⛔ binding stop: if both candidate readings of "what status does an undeclared hook refusal deserve" turn out to have live in-repo pins, the dev reports the fork and lands nothing on this arm; this card then returns to pm:queue while the share half may land alone.

    Acceptance criterion for this card specifically: an undeclared hook throw on POST /api/v1/analytics/dataset/query answers the same status as the same throw on POST /api/v1/data/:object. §8b's pin on the 500 is inverted, not deleted — it currently records the defect as measured, so it becomes the pin that the two doors now agree, with its comment rewritten to say which reading won and why.


    Generated by Claude Code

  5. claude commented on Aug 24, 2026

    @claude
    Contributor
    {
      "issues": [11683, 11684],
      "issue": 11683,
      "status": "done",
      "branch": "claude/issue-11683-route-refusal-classification",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/11731",
      "pr_url": "https://github.com/objectstack-ai/objectstack/pull/11731",
      "premise_still_valid": true,
      "fork_status": "settled — the `/data` door's reading won; the fork clause did NOT fire and both halves land. The two readings do not compete, they govern different questions. (a) ADR-0112's 'the producer names the condition' is a rule about the CODE: D1-D9 and all five amendments rule on the code vocabulary, its closure and the `declaredCode` demote channel, and the phrase itself is not in the ADR at all — it is `error-response.ts` prose governing a DECLARED 5xx that carries no code ('a half-declaration is honoured for the half that was declared'). Nothing in ADR-0112 speaks to the status of an undeclared throw, and this PR contradicts it nowhere: the new arm invents no code either. (b) The `/data` door's reading rules the STATUS and is structural, not a preference: `classifyDataError`'s sandbox unwrap door answers `declaredHttpStatus(error) ?? 400` with the verbatim `.innerMessage` for a body that REPORTED, and the sanitised 500 for one that CRASHED (`isScriptFaultMessage`, #7543) — pinned end to end by `hook-error-format.dogfood.test.ts` ('DELETE blocked by a sandboxed hook returns ONLY the business message', 400) and in process by parity §3. So 'an undeclared throw is unclassified' was never the repo's rule for this class: a sandboxed body that reports has classified itself structurally, and only a CRASH is unclassified (still 500 on both faces). NO live in-repo pin was found on the ADR-0112 side of the question — §8b was the only one, and its own comment declared it a record of a defect left standing, not a ruling.",
      "share_half_status": "done — all three record-share routes. A declared `{code,status}` envelope (both `status` and `statusCode` spellings) is answered with that status and code instead of `500 SHARE_*_FAILED`; no sandbox wrapper text reaches the client on any of the three, including on the CRASH arm the classification door cannot reach (`sharingFaultMessage` withholds there, matching `/data`'s `UNCLASSIFIED_FAULT`). The five-prefix `CODE: message` idiom and the three `SHARE_*_FAILED` 500 codes are kept and re-pinned; the nested ADR-0112 D5 envelope (#8111) does not move, so `check:route-envelope`'s ratchet is untouched.",
      "analytics_half_status": "done — `POST /api/v1/analytics/dataset/query`. A new arm ①b between ① and ③ asks the same seam, so an UNDECLARED sandboxed hook refusal answers 400 with the hook's own sentence — the same status `POST /api/v1/data/:object` answers for the identical throw — and the `statusCode` spelling of a declared 4xx+code resolves too (#7525). Arm ① is untouched, so #5352's both-halves rule stands; arm ③ is untouched for a declared 5xx, a crashed body, a driver fault and anything unclassified. §8b is INVERTED, not deleted: its original text is quoted in the new comment above the evidence for which reading won.",
      "string_convention_producers_found": "YES — and problem 1 is nevertheless LIVE, not latent. Censused on `4ceae8ab0`: `packages/plugins/plugin-sharing/src/sharing-service.ts` has 11 throw sites, every one a bare `new Error('CODE: …')`, and the file contains no `code =` / `status =` / `statusCode =` assignment anywhere — so the prefix idiom is the only channel that service has and backward compatibility is required, kept, and re-pinned (5 cases). But TWO declared-envelope producers reach these same three catches today: `plugin-sharing`'s own write gate throws `{ code: 'FORBIDDEN', status: 403 }` (`sharing-plugin.ts`) and `FORBIDDEN` is not one of the five prefixes, so a refusal that declared 403 twice over was answered 500 — pinned as the live in-repo witness; and a sandboxed hook on the `sys_record_share` write, which is problem 2's producer. `sharing-service.ts` was READ ONLY (declared trigger file of on-hold #6736); nothing in this PR touches it.",
      "clause_2": "yes — declared in the PR body. The changeset names, per route, the old answer and the new one (status + code) in two tables, so a consumer reading release notes can tell whether their error handling breaks.",
      "files_touched": [
        "packages/rest/src/error-response.ts (new export `classifiedRefusalAnswer`; no existing behaviour changed)",
        "packages/rest/src/rest-server.ts (analytics arm ①b; `respondSharingError` asks the door first; new `sharingFaultMessage` on the three 500 arms; one stale #11588 comment amended)",
        "packages/rest/src/rest-hook-refusal-message-parity.test.ts (§8 header amended, §8b INVERTED, §8e and §8f added)",
        "packages/rest/src/rest-share-refusal-classification.test.ts (new)",
        ".changeset/olive-donkeys-repeat.md (new)"
      ],
      "pins_added": [
        {
          "pin": "share §1 — a declared `status`/`code` envelope is honoured on all three routes (3 cases: `RECORD_LOCKED`/409, the `statusCode` spelling, and plugin-sharing's own `FORBIDDEN`/403 write-gate throw)",
          "red_before": "3 red. `AssertionError: GET shares: {\"success\":false,\"error\":{\"code\":\"SHARES_LIST_FAILED\",\"message\":\"This account is locked while month-end close runs.\"}}: expected 500 to be 409` · `… \"message\":\"FORBIDDEN: insufficient privileges to delete account a1\"}}: expected 500 to be 403`",
          "green_after": "green in `Test Files 5 passed (5) · Tests 96 passed (96)`"
        },
        {
          "pin": "share §2 — no sandbox wrapper reaches the client (business refusal, refusal+envelope, and a ⭐ control that a CRASHED body is NOT served as a refusal and has its text withheld)",
          "red_before": "3 red. `AssertionError: GET shares leaked the wrapper: {\"success\":false,\"error\":{\"code\":\"SHARES_LIST_FAILED\",\"message\":\"hook 'guard' threw: Error: Sharing is frozen until the quarterly access review closes.\"}}: expected … not to match /threw:|hook '/` · `AssertionError: GET shares: expected 'hook \\'guard\\' threw: Error: TypeErro…' to be 'Internal server error'`",
          "green_after": "green in the same run"
        },
        {
          "pin": "share §3 — REGRESSION GUARD, green both sides by design: the five `CODE:` prefixes still map with the prefix stripped, an unclassified fault still leaves through this route's own 500 verbatim, and every arm still parses as `ApiErrorSchema`",
          "red_before": "green pre-fix (7 cases) — deliberately a control, not a defect pin. Its value is that it stayed green: the prefix idiom and the 500 terminal are what backward compatibility rests on.",
          "green_after": "green"
        },
        {
          "pin": "share §4 — door-to-door: every classified refusal gets the same status at the share door and the `/data` door (5 error shapes × 3 routes)",
          "red_before": "1 red. `AssertionError: declared 409 + code @ GET shares: share door 500 vs /data door 409: expected 500 to be 409`",
          "green_after": "green"
        },
        {
          "pin": "parity §8b — INVERTED (was: 'an UNDECLARED refusal reaches the 500 arm unwrapped — the status is NOT moved'). Now: answers 400 with the business sentence and no invented code",
          "red_before": "1 red. `AssertionError: expected 500 to be 400`",
          "green_after": "green"
        },
        {
          "pin": "parity §8e — the analytics face and `POST /data/:object` answer one hook `throw` with ONE status; both REAL handlers driven in process, 4 shapes, and neither status is named so a future move on either side reddens",
          "red_before": "1 red, and the message reproduces #11684's own measurement table: `AssertionError: undeclared refusal: analytics 500 {\"code\":\"ANALYTICS_QUERY_FAILED\",\"error\":\"month-end close is in progress\"} vs /data 400 {\"error\":\"month-end close is in progress\",\"object\":\"crm_account\"}: expected 500 to be 400`",
          "green_after": "green"
        },
        {
          "pin": "parity §8f — MEASURED AND NOT REPAIRED: the two faces still disagree in the 5xx band (declared 503 → `503 SERVICE_UNAVAILABLE` on `/data`, `500 ANALYTICS_QUERY_FAILED` here)",
          "red_before": "found by §8e's first draft, which included this case and reddened WITH the fix in place: `AssertionError: declared 5xx: analytics 500 … vs /data 503 {\"error\":\"Internal server error\",\"code\":\"SERVICE_UNAVAILABLE\"}: expected 500 to be 503`",
          "green_after": "green as an assertion OF the divergence — it pins the bound on §8e's parity claim rather than a repair. Filed as #11718."
        }
      ],
      "checks_run": "All on `b468109e1`, the final commit. (1) `pnpm --filter @objectstack/rest test` → `Test Files 144 passed (144) · Tests 2317 passed (2317)`. (2) `pnpm --filter @objectstack/rest typecheck` → `tsc --noEmit`, exit 0. (3) `pnpm lint` (WHOLE repo, `eslint . --no-inline-config`, not narrowed) → `VERDICT command-exit 0`. (4) The 18-gate family re-derived from the ACTUAL change set via `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (5 paths, merge base `4ceae8ab0`) → `AGGREGATE: FAIL=0`; it includes `check:route-envelope` (this file's dialect ratchet) and `check:dispatcher-error-vocabulary`, the latter reporting `OK — 21 unregistered code-stamping site(s), all classified`. (5) Ratchet family + the three pin files re-run AFTER the final commit on `b468109e1` → `Test Files 3 passed (3) · Tests 55 passed (55)`, gates OK. Exit codes captured before any pipe (`cmd > file 2>&1; EXIT=$?`), and every verdict quoted above is the gate's or vitest's own printed line, never a bare `$?`. Reverse verification ran from the COMMITTED fix: `git restore --source=origin/main` on the two source files (presence of `classifiedRefusalAnswer` grepped to 0 on disk both files), then `git restore --source=HEAD --staged --worktree` (grepped back to 5 and 1), `git diff HEAD` empty and `git status --porcelain` clean. 9 red predicted, 9 red measured — exact.",
      "summary": "Both halves landed on one branch and one draft PR. A single new seam in `error-response.ts` — `classifiedRefusalAnswer` — returns the `/data` door's classification WITHOUT choosing a dialect, for a refusal the producer classified: a declared 4xx `status`/`statusCode` plus a `code` (both halves, so #5352's ruling stands), or a sandboxed body's business `throw`. It hands a 5xx straight back, so each route's own fault terminal is untouched. The three record-share catches ask it before the `CODE:` prefix map and re-dress the answer in the nested ADR-0112 D5 envelope #8111 converted them onto, so the dialect does not move and `check:route-envelope`'s ratchet is untouched; their three 500 arms now withhold a sandbox error's text, which closes the wrapper leak on the CRASH arm the classification door cannot reach. The analytics catch gains arm ①b between ① and ③. The fork clause did not fire — see `fork_status`. Two things measured and deliberately not repaired are pinned (§8f) and filed rather than fixed.",
      "tests": "See `checks_run` for the command list and the verdict lines. Every pin was shown failing without the behaviour and passing with it, from the committed fix; the predicted/measured table is in the PR body. No ablation of a built artifact was involved — all pins drive `RestServer`'s real registered handlers in process from `src`, so there is no `dist/` leg to rebuild, and `check:test-source-alias` passes.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #11718: a declared 5xx answers `500 ANALYTICS_QUERY_FAILED` on `/analytics/dataset/query` where `/data` answers the declared status and code (a 503 lifecycle outcome collapsed onto 500) — found by §8e's first draft reddening WITH the fix in place, pinned as §8f",
        "filed as #11719: `sendError`'s `extra` cannot carry `declaredCode`, so every nested-envelope route silently drops an unregistered producer's own code spelling, against ADR-0112's #9106/#9232 'every door' ruling — the repair is in `packages/types`, outside this card's declared surface"
      ],
      "notes_for_pm": [
        "Coordinator addendum discharged: `packages/plugins/plugin-sharing/src/sharing-service.ts` (trigger file of on-hold #6736) was READ ONLY, for the producer census. The fix needed no edit there and the PR does not touch it, so #6736's opportunistic-restart clause did not fire.",
        "Declared file surface held exactly — nothing outside it was edited. Two repairs that WOULD have needed to widen it were filed instead (#11718, #11719).",
        "Findings were filed with the `finding` label per this dispatch's explicit instruction. Note the standing os-dev clause routes a CONCRETE defect unlabelled to triage and reserves `finding` for observational items; both of these are concrete defects, so the dispatch's instruction and the standing clause disagree here. Flagging rather than choosing silently — relabel if triage prefers the standing rule.",
        "⛔ Not armed, not marked ready, auto-merge not enabled."
      ]
    }

    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions