Skip to content

fix(rest): converge every declared-4xx exit on the declared-code prefix strip (#13095) - #14120

Merged
os-steve merged 5 commits into
mainfrom
claude/issue-13095-resolve-error-response-code-prefix
Sep 1, 2026
Merged

fix(rest): converge every declared-4xx exit on the declared-code prefix strip (#13095)#14120
os-steve merged 5 commits into
mainfrom
claude/issue-13095-resolve-error-response-code-prefix

Conversation

@os-steve

@os-steve os-steve commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes #13095

Implements the 2026-08-31 director-seat ruling (option 1, maintainer verbatim 「同意」, comment 5478591418) — adopted verbatim, not re-derived.

Clause-②: yes

This PR changes what a published REST door puts in error / message on the wire, and parks as a draft for at-tier contract review by design.

Measured wire movement (real registered route handlers, one producer per row — a thrown Error carrying code: 'FORBIDDEN', status: 403, message FORBIDDEN: + the localized sentence, the exact shape plugin-sharing/src/sharing-plugin.ts's by-id write gate throws):

exit before after
PATCH /data/:object/:id (by-id, ruled by the #12975 ruling) bare sentence bare sentence — unchanged
POST /data/:object/batch and every bulk/clone exit through handleRouteError FORBIDDEN: + sentence bare sentence
GET/POST/DELETE /data/:object/:id/shares... classified arm (nested envelope error.message) FORBIDDEN: + sentence bare sentence
approvals door (handleApprovalError) — a message opening with a DIFFERENT SCREAMING_SNAKE token than the answered code token eaten by blanket regex token kept; only a duplicate of the answered code is ever stripped

All movement is subtractive on message text only; statuses, code, declaredCode, userMessage untouched. Everything else is pinned unchanged: no-code 4xx keeps its prefix (the token is nowhere else on the wire), a prefix not naming the declared code stays (driver prose), declared-5xx prose withholding, the share family's bare-Error prefix-idiom arm, and the passthrough's empty-string TYPE-keyed degrade (a standing pin this ruling did not authorize moving — deliberately preserved, see below).

First-verify premise: the prefix-consumer census

Does any consumer branch on the ADR-0111 CODE: prefix in the wire error text?

  • objectstack: zero (PM baseline at cc837dbfe zero; increment cc837dbfe..836a29c27a re-scanned = zero new, control: 7 generic startsWith( added lines read)
  • objectui: zero (baseline a5a799d zero; increment a5a799d..6c1d95d = zero new, control: 5)
  • hotcrm (supplementary, reachable): zero wire consumers (one includes hit reads its own test's stdout marker, not an error body; control: 79 startsWith( hits)
  • cloud: ⛔ NOT MEASURED — unreachable from this session (repo attach denied; the session's repo listing shows only objectstack, objectui, hotcrm). Explicitly not reported as zero — this seat's own reasoning, stated as such rather than as compliance with a quoted instruction: an unreachable repo is not a repo with no consumers, and collapsing "could not measure" into "measured zero" would manufacture exactly the kind of over-claim item 2's discipline forbids in the #8111 comment. The ruling itself was made with cloud unmeasured — it records 「cloud 本席不可达」 (comment 5478591418) — and its only stop condition is a NON-ZERO census. With every reachable scope at zero and cloud carried as NOT MEASURED in every artifact of this change (this body, both #8111 comment homes, the changeset), implementation proceeds and the bound ships with it.

No non-zero anywhere reachable ⇒ no fork.

The four mandatory items

  1. resolveErrorResponse's passThroughStatus 4xx arm now applies the existing withoutDeclaredCodePrefix — declared-code anchored, never a regex anchor. Strip runs before the rest-server 的 4xx 直通把 ≥500 字符的 message 整条换成 "Request failed" —— #5368 刚写好的过滤器拒收措辞,客户端一个字也收不到(实测) #5423 truncation bound; a nothing-but-prefix message degrades to Request failed (the sibling arm's rule travelling with the strip). The record-share classified arm converges automatically because classifiedRefusalAnswer re-dresses this same answer.
  2. #8111 comment corrected in two places, one write (commit 4633fd31a2): rest-server.ts (located by symbol, above respondSharingError) and the sharing-envelope.test.ts docblock. The false mechanism claim ("stripped below and never reaches the wire") is corrected to name the classified limb that shipped the prefix until this PR; the consequence stays exactly as measured — no consumer branches on the prefix in the two reachable repos, cloud NOT measured — not restated as a guarantee.
  3. Third strip point (handleApprovalError in rest-server.ts): the blanket SCREAMING_SNAKE-colon regex — the shape the The /data door ships the ADR-0111 CODE: prefix inside the user-facing error string, so a localized refusal renders as FORBIDDEN: … in a toast #12975 ruling rejected — is converged onto the code the matched row answers, the same anchored semantics as respondSharingError's prefix arm and withoutDeclaredCodePrefix.
  4. The MEASURED-NOT-REPAIRED pin is moved deliberately, not turned green: rewritten as the CONVERGENCE [#13095] pin asserting the new truth, with a docblock recording what moved and why; new §6 (bulk door) and §7 (approvals door) pin the moved arms' own anchoring controls.

Everything located by symbol; the card's and ruling's line numbers had rotted as predicted (#8111 comment and third strip point both moved again since the ruling was written).

Verification

  • pnpm --filter @objectstack/rest test: 164 files / 2771 passed (pre-merge head; targeted 12-file re-run at final head below). typecheck green; both edited test files confirmed present in tsc --listFiles (coverage measured, not assumed); the only raw tsc errors are the pre-existing pinned debt entries in test-typecheck-debt.json.
  • Ablation, two legs, predictions written before running, mutation AND restore each proven on disk (blob-hash equality against the named rev + single-occurrence anchor counts both ways; trap in the same process as the measurement; no rebuild between legs — every subject is reached by relative in-package imports vitest transforms from source):
    • Leg A (error-response.ts at pre-fix bytes): predicted exactly 2 red — §5 CONVERGENCE pin + §6 nothing-but-prefix; measured 2 red / 50 green, with rest-hook-refusal-message-parity.test.ts all green both sides — the preserved empty-string pin measurably did not move. §6's no-code and non-matching-prefix controls stayed green on both sides (they red under a pattern-anchored strip, not under the missing fix — controls that can fail for the right reason, wired to a different failure than the fix's absence).
    • Leg B (rest-server.ts at pre-fix bytes): predicted exactly 1 red — §7 longer-token; measured 1 red / 25 green, rest-approvals-wire-codes.test.ts all green both sides (the anchored strip answers the well-formed idiom byte-identically).
  • Gate union at final commit 4ed81ba8b3 (derived by dispatch-gates.mjs --repo objectstack-ai/objectstack on the merged head; origin/main merged first, all repo-level readings taken after the merge): named 37, ran 37, unreconciled 0 (exact-string comm both directions). All green except check-test-completeness.mjs, which exits 3 on its own documented no-log-named local branch: NOT MEASURED locally by its own verdict text; CI measures it with the teed test log. check:type-check-debt verdict line: "check-type-check-coverage --re-measure: OK — 28 ledger entr(ies) re-measured in 660.9s, 1468 raw tsc error(s) total, none above its recorded number." check:system-context-census green with no regeneration needed. pnpm lint (never named by the derivation): full-repo eslint, exit 0.
  • Review round at 209965648d (the at-tier review's changeset edit): the only file changed vs 4ed81ba8b3 is .changeset/rest-passthrough-strips-declared-code-prefix.md (git diff --stat, one file). Every changeset-reading gate re-ran green on the new head (nul-bytes, adr-0087-registration, changeset-no-major, empty-changeset, changeset-gate-self-tests, objectui-changeset, doc-authoring, both docs-audit gates); every other gate's input set is byte-identical to 4ed81ba8b3 where the full union ran, and the full-repo lint reading stands by eslint's own config answer for this file: "File ignored because no matching configuration was supplied".

Grading

minor + migration note in the changeset, per the ruling and the #13347 precedent (an additive envelope change was ruled minor; a subtractive change to wire text is at least as strong a case). Not regraded.

Out of scope, untouched here: #13753 and #13906 wait behind this PR on rest-server.ts; #12975 and #13910 are context only — none of those cards is addressed here and all remain as they are.

Generated by Claude Code

… declared-code prefix (#13095)

Spread the #12975 ruling's declared-code-anchored strip
(withoutDeclaredCodePrefix) to resolveErrorResponse's passThroughStatus 4xx
arm, converging every /data exit (batch/createMany/updateMany/deleteMany/
clone via handleRouteError) and, because the record-share classified arm
re-dresses the same classification, the share family with it. Converge the
approvals door's blanket /^[A-Z_]+:\s*/ strip onto the code the row answers
(the regex shape #12975 rejected). Correct the #8111 comment's false
mechanism claim in both homes (rest-server.ts and the sharing-envelope.test
docblock) without over-claiming the measured consequence. Move the
MEASURED-NOT-REPAIRED pin to the CONVERGENCE pin it announced, and add the
moved arm's own anchoring controls.

Per the 2026-08-31 maintainer ruling (option 1) on #13095.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk
…set (minor + migration note)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk
…#13095)

The header's ablation record spelled the approvals door's old blanket strip
as a slash-delimited regex literal inside a block comment; its closing */
ended the comment 40 lines early. check:engine-double-contract's ts-parse
refusal caught it. Spelled without delimiters.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation labels Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ⚠️ 1 changed file(s) yielded no anchor (packages/rest/src/rest-server.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/rest/src/rest-server.ts) — pages documenting those are invisible to this run
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • 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 — 13 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 556ebc1509b1a6dcc842425ec58e74d095b7c966packageMentionDocs.

Which tree this was computed on

This run read content/docs from 602727d5d4dcbbe3c8f4e0d487e4c98433bedb44 — the merge of head 209965648d4871bf2d20df5ccf4fc9357deae682 into base 556ebc1509b1a6dcc842425ec58e74d095b7c966, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 602727d5d4dcbbe3c8f4e0d487e4c98433bedb44 && git checkout 602727d5d4dcbbe3c8f4e0d487e4c98433bedb44
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 556ebc1509b1a6dcc842425ec58e74d095b7c966 209965648d4871bf2d20df5ccf4fc9357deae682 && git checkout -B drift-repro 556ebc1509b1a6dcc842425ec58e74d095b7c966 && git merge --no-ff 209965648d4871bf2d20df5ccf4fc9357deae682

node scripts/docs-audit/affected-docs.mjs --json 556ebc1509b1a6dcc842425ec58e74d095b7c966

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

os-steve commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

REQUEST CHANGES — one required edit, everything else verified as ruled: the changeset's migration note must carry the cloud NOT-MEASURED bound that every other artifact in this PR carries. .changeset/rest-passthrough-strips-declared-code-prefix.md currently closes with "A cross-repo census (objectstack, objectui, hotcrm; re-run 2026-09-01) found zero consumers branching on the prefix" — literally true, but it restates the census consequence with the bound dropped, which is the exact over-claim shape the ruling's item on the #8111 comment forbade (「⛔ 不过度声明后果」). The PR body, rest-server.ts, and the sharing-envelope.test.ts docblock all say "cloud … NOT measured"; the changeset — the one artifact that feeds release notes and reaches consumers — does not. Add one sentence naming cloud as unmeasured (e.g. "; objectstack-ai/cloud was not reachable from the implementing session and is NOT measured — a cloud-side consumer, if one exists, will surface at its next .objectstack-sha pin bump and should be read against this note"). Nothing else needs to change.

This is an at-tier contract review under CONTRACT_REVIEW_TIER: scripts/pm/dispatch-gates.mjs on origin/main (c75962ac3e) declares export const CONTRACT_REVIEW_TIER = 'claude-fable-5' (located by symbol), which is the model running this review. Reviewed head 4ed81ba8b3 against base b8c4762c78; diff read in full (5 files, +264/−25, no content/docs/releases/ rider).

The census premise (the question that matters most)

What I measured myself, each zero with a control that returned non-zero:

  • objectui @ origin/main dd35800: git grep -nE "startsWith\(['\"\][A-Z][A-Z_]{2,}"→ exactly 1 hit,packages/app-shell/src/chrome/ConditionalAuthWrapper.tsx:117 err.message?.startsWith('HTTP ')— an HTTP-status label, not an ADR-0111CODE:prefix. Control: 491 genericstartsWith(lines read. I additionally found three sites the startsWith patterns don't catch:plugin-detail/src/DetailView.tsx:663, plugin-detail/src/InlineEditSaveBar.tsx:156, react/src/utils/error-message.ts:87, each .replace(/^[A-Z][A-Z0-9_]+:\s*/, '')` on error text. These are display scrubbers, not branching consumers — they read the prefix only to delete it, and the server-side strip turns them into no-ops with byte-identical rendered output. They are affirmative evidence for the ruling's premise: the UI already treats the prefix as noise.
  • objectstack @ origin/main c75962ac3e: outside packages/rest, every startsWith('SCREAMING… hit is SQL/env/Dockerfile string handling (driver-sql/src/sql-driver.ts:5477 reads a driver code, migration tests read SQL verbs, etc.) — zero wire-error readers. Inside rest: rest-server.ts's 7 hits are the server's own service→REST route mappings (in-process, the [finding] registerSharingEndpoints / respondSharingError still answers the bare-string error dialect (ADR-0112) — the sharing half split out of #8073 #8111-censused class). The two WIRE_ERROR test helpers (plugin-sharing/src/write-denial-user-copy.test.ts, plugin-approvals/src/recall-refusal-user-copy.test.ts) assert the prefix is absent — they pull toward this PR, not against it.
  • hotcrm @ local aff9cb2: zero; the single includes( hit is test/verify-log-decoy-pin.test.ts:240 reading its own test's stdout marker (CAPTURED:${MARKER}), not an error body — exactly as the dev reported. Control: 79 startsWith( lines, matching the dev's control count exactly.

What I could not reach: objectstack-ai/cloud, on two independent channels. GitHub MCP: Access denied: repository "objectstack-ai/cloud" is not configured for this session. Allowed repositories: objectstack-ai/objectstack, objectstack-ai/objectui. Platform repo-attach: you don't have access to objectstack-ai/cloud. That is now three seats (filing, dev, review) structurally unable to run the census; the dev's refusal to record the unreachable repo as a zero was correct and I am not undoing it by a softer route — cloud remains NOT MEASURED, including by this review.

Is cloud materially exposed? From what is reachable: cloud consumes this framework at a pinned SHA (.objectstack-sha), advanced only by serialized pin-train cards with their own build-and-test (seat post #6026: the #1614#1620 train, ancestor readings taken per bump). So this change cannot reach cloud silently at merge time — it arrives at a deliberate pin bump, inside cloud's own verification, which converts the failure mode from silent production divergence to diagnosable-at-bump. Cloud's own surfaces in the seat record are deploy scripts, EE images, and a console presumably built on the objectui packages measured above; the acceptance-anchor app in that lane (hotcrm, cloud#1512) measured zero. A cloud-native reader of /data error bodies can't be excluded from here — that is precisely the residue — but nothing reachable makes one plausible, and everything reachable that does touch the prefix scrubs or ignores it.

My judgement, stated separately as asked: the unmeasured cloud census is an acceptable residue to be recorded, not a merge blockerconditional on the changeset edit above, which is what makes the residue "recorded" where a cloud engineer would look. Reasons: the director seat issued the ruling knowing cloud was unreachable from its own seat (comment 5478591418: 「cloud 本席不可达」) and made only a non-zero census a fork trigger, not unreachability a gate; the pinned-SHA seam bounds the blast radius to a diagnosable pin-bump failure; and the residue is one grep (the ready-made command in comment 5475900981) that an authorized cloud-lane seat can run in minutes. I recommend the PM file a cloud-lane card (the #6026 lane) to run that census before cloud's next .objectstack-sha bump crosses this merge — that window, not the merge itself, is when the reading matters. The maintainer may still prefer to hold the merge for that reading; I flag that as their call, distinctly from this review's verdict. One precision nit while checking measurement vs. sentence: the PR body says "the dispatch rules NOT-MEASURED-cloud as proceed-and-report" — the dispatch comment (5487075633) orders the census with non-zero⇒stop and notes cloud was unreachable from the filing seat, but does not verbatim contain a proceed-and-report ruling for the unmeasurable case. The dev's reading is defensible (the ruling seat wrote the prerequisite knowing cloud was unreachable); the sentence just attributes to the dispatch words it doesn't carry.

Clause ② — both limbs, measured

  • Accept/reject change: yes. Real 4xx rejection envelopes move on the wire through registered route handlers — I reproduced the movement (ablation below): POST /data/:object/batch error and the record-share nested error.message go from FORBIDDEN: <sentence> to the bare sentence; the approvals door stops eating FORBIDDEN_BY_POLICY:. Statuses and code/declaredCode are pinned unchanged (§5 asserts code: 'FORBIDDEN' survives; §6 asserts 403 and that a no-code body still omits code). Which requests are accepted or rejected does not change; what a rejection says does — that is contract reject-side behaviour, so the limb holds and the yes declaration is correct.
  • Public-surface widening: no. The diff adds no route, field, export, or code path visible to a consumer; movement is subtractive on message text only. Clause ② applies via the first limb alone — the at-tier review requirement was real, and this review discharges it.

The four ruled items — delivered as ruled

Ablation — reproduced independently, controls verified

In a detached review worktree at 4ed81ba8b3 (deps built via turbo run build --filter=@objectstack/rest): head-side run of rest-data-door-code-prefix.test.ts + rest-hook-refusal-message-parity.test.ts + rest-approvals-wire-codes.test.ts + sharing-envelope.test.ts4 files, 68 tests, all green.

  • Leg A: packages/rest/src/error-response.ts set to base bytes (blob 424c28ee7625ed097978, equal to b8c4762c78's blob by git hash-object): 2 failed / 50 passed — exactly §5 CONVERGENCE (expected 'FORBIDDEN: 您无权…' to be '您无权…') and §6 nothing-but-prefix (expected 'FORBIDDEN:' to be 'Request failed'); rest-hook-refusal-message-parity.test.ts all green, so the preserved empty-string pin measurably did not move. Restored, blob hash re-equal to head.
  • Leg B: packages/rest/src/rest-server.ts set to base bytes (blob 9d82c08ec82057e27731): 1 failed / 25 passed — exactly §7 longer-token (expected 'contact your administrator' to be 'FORBIDDEN_BY_POLICY: contact your administrator'); rest-approvals-wire-codes.test.ts all green both sides. Restored; git status --porcelain empty, git diff HEAD empty.
  • The controls are controls, not decoration — observed, not just argued. With the fix absent (Leg A), §6's no-code and non-matching-prefix cases stayed green: the old code ships those messages untouched, so they cannot red under the fix's absence. What they red under is a pattern-anchored strip: a blanket ^[A-Z_]+: in that arm would eat FORBIDDEN: ${ZH} off a no-code 403 (deleting the only machine token on that wire) and eat SQLITE_ERROR: driver prose — both asserted kept. Leg B's §7 red is the in-tree demonstration that a pattern-anchored strip does exactly that to these shapes. So each zero-movement claim has a case that could have returned non-zero, wired to the wrong-fix shape rather than to the fix's absence — the right reason.

The deliberate non-convergence — right call

The empty-string TYPE-keyed degrade is pinned at rest-hook-refusal-message-parity.test.ts:449-458 ("Deliberately not 'Request failed': this arm's degrade is keyed on the TYPE…"), a pin that predates this PR and was not among the ones the ruling authorized moving. An empty message carries no CODE: prefix, so it is outside this ruling's subject entirely; converging it would have been an unauthorized pin move — the precise offense this lane's discipline (and this card's own history) exists to prevent. Drafting the convergence, watching the pin red, reverting that half, and documenting the boundary in the arm's comment is the correct absorption; no escalation was owed, because the asymmetry was already pinned and documented before this PR existed. A follow-up card on the TYPE-vs-length degrade asymmetry is optional, not required.

Verdict mechanics

  • needs:contract-review: clears once the one-sentence changeset edit lands — the contract substance is verified and needs no second full pass; a diff-check of that single sentence suffices.
  • Cloud census as merge blocker: no, in my judgement — acceptable residue to be recorded, conditional on the changeset edit, with a recommended cloud-lane census card before the next .objectstack-sha bump; escalation of the hold-vs-proceed question to the maintainer is appropriate and I support making it their explicit call.
  • Per the hard constraints of this review: no merge, no label writes, no code — this comment is the review's entire output.

Generated by Claude Code

…hipping artifact (#13095)

At-tier contract review finding on PR #14120: the changeset said a
cross-repo census found zero prefix-branching consumers without naming
objectstack-ai/cloud as unmeasured. The PR body and both #8111 comment
homes carried the bound; the changeset -- the artifact that feeds release
notes and reaches operators -- did not, so its zero read as 'checked
everywhere'. The census statement now names the three repos it covered
(each with a positive control) and states that cloud was NOT MEASURED and
is deliberately not reported as clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UngCYXF98BVpYA9hfz6NYk
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 1, 2026
@os-steve
os-steve marked this pull request as ready for review September 1, 2026 06:31
@os-steve
os-steve added this pull request to the merge queue Sep 1, 2026
Merged via the queue into main with commit bb4ea80 Sep 1, 2026
42 of 43 checks passed
@os-steve
os-steve deleted the claude/issue-13095-resolve-error-response-code-prefix branch September 1, 2026 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants