Skip to content

fix(rest): consult the bespoke structured arms before the declared-status passthrough, so both error doors answer one refusal with one body - #14713

Merged
os-project-manager merged 11 commits into
mainfrom
claude/issue-14541-error-response-passthrough-ordering
Sep 3, 2026
Merged

fix(rest): consult the bespoke structured arms before the declared-status passthrough, so both error doors answer one refusal with one body#14713
os-project-manager merged 11 commits into
mainfrom
claude/issue-14541-error-response-passthrough-ordering

Conversation

@os-trump

@os-trump os-trump commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14541

resolveErrorResponse — the door behind handleRouteError / sendThrownError, which every bulk, import/export, metadata and UI route reports through — took its own declared-status passthrough before delegating to mapDataError, the door the single-record /data routes call directly. An engine envelope that DECLARES status therefore short-circuited, and every bespoke structured arm behind the delegation was unreachable from those routes. One refusal, two bodies, decided by which route caught it.

The project had already ruled on this exact shape, for one code. From error-response.ts itself:

[#3770] OBJECT_NOT_FOUND is deliberately excluded from this status-passthrough: mapDataError owns its canonical envelope, and short-circuiting here would ship a second wire code for the same condition depending on which route caught it.

This PR generalises that accepted rule rather than proposing a new one. The exclusion never grew past its first case, and the measurements below are the bill for that.

Patch round 2 — the seven adopted contract-review conditions. This body has been rewritten under the verdict adopted verbatim at #14541 (comment). Four claims it previously made were false or backwards and are corrected here rather than left standing: the ApiErrorSchema sentence (condition 1), "no status moves" (condition 5), the driver-memory direction (condition 7), and the omitted classifiedRefusalAnswer door families (condition 6). The vocabulary fork (condition 2) is now disclosed instead of implied away. A per-condition account is in the round's report on the card.

The fix

Per triage's scope guard 2 — "prefer lifting the structured arms into a classification both doors ask first, over adding a second exclusion list":

  • The declared-code structured arms of classifyDataError — every arm ahead of its first message-derived read — move into one structuredCodeAnswer(error, object). classifyDataError now begins by asking it: same arms, same order, same position.
  • resolveErrorResponse asks the same classification before its passthrough, and answers a match by delegating to mapDataError. The two doors are therefore identical by construction — same arm, same withDeclaredUserMessage wrapper — not by coincidence.
  • The boundary is statable: an arm belongs to the shared classification when it is decided on what the PRODUCER DECLARED (code, or the error name where a class is the contract). Nothing in it reads message TEXT to decide which condition this is. That is why the PERMISSION_DENIED arm (whose third limb sniffs a [Security] Access denied prefix) and the sandbox unwrap door stay where they are.
  • One condition BOTH doors put on taking the answer (fiveXxArmDisplacesDeclared4xx, added in this patch round under condition 4): an arm that answers a 5xx never displaces a status the producer declared in the 4xx band. The review measured the two doors answering 503 and 400 for one synthesised error, because that guard existed only in resolveErrorResponse. It is now asked at both, which is the honest fix — no producer declares a status on the only 5xx-answering arm, so nothing moves on the wire.

Wire-contract delta — measured, not predicted

Both doors driven in process on the same error object, one producer at a time, before and after. BULK is sendThrownError (identical to handleRouteError; pinned in §3 of the new test). SINGLE is mapDataError.

The producers whose bodies change

DELETE_RESTRICTED  (engine.ts sets code/status=409/object/dependentObject/dependentCount/developerMessage)
  BEFORE bulk   409 {"error":"Cannot delete this 客户 because 3 联系人 still reference it.","code":"DELETE_RESTRICTED"}
  AFTER  bulk   409 {"error":"Cannot delete this 客户 because 3 联系人 still reference it.","code":"DELETE_RESTRICTED",
                     "developerMessage":"Cannot delete account (a1): 3 dependent contact record(s) reference it via
                     account_id. Delete or reassign them first, or set deleteBehavior:'cascade' on contact.account_id.",
                     "dependentObject":"contact","dependentCount":3,"object":"account"}
  single door unchanged, and AFTER-bulk is byte-identical to it

CONCURRENT_UPDATE  (metadata-protocol's ConcurrentUpdateError: readonly status = 409)
  BEFORE bulk   409 {"error":"Record account/r1 was modified by another user (current version …, expected …)",
                     "code":"CONCURRENT_UPDATE"}
  AFTER  bulk   409 {…same error/code…,"currentVersion":"2026-09-02T10:00:00.000Z",
                     "currentRecord":{"id":"r1","name":"after"},"object":"account"}

DUPLICATE_RECORD  (objectql's DuplicateRecordError, #14095/#14389: readonly status = 409)
  BEFORE bulk   409 {"error":"Duplicate record refused on 'duly_note': a unique constraint on 'email' already holds
                     this value. No record was written.","code":"DUPLICATE_RECORD"}
  AFTER  bulk   409 {"error":"A record with this email already exists","code":"UNIQUE_VIOLATION",
                     "developerMessage":"Duplicate record refused on 'duly_note': a unique constraint on 'email'
                     already holds this value. No record was written.","field":"email","object":"duly_note"}

FEEDS_DISABLED / FILES_DISABLED  (plugin-audit: err.status = 403, err.object)
  BEFORE bulk   403 {"error":"Comments are disabled for object 'account' (enable.feeds: false)","code":"FEEDS_DISABLED"}
  AFTER  bulk   403 {…same error/code…,"object":"account"}

ATTACHMENT_PARENT_ACCESS / ATTACHMENT_DELETE_DENIED / RECORD_NOT_ACCESSIBLE
                   (service-storage `forbid()` and plugin-audit's deny: err.status = 403, err.object)
  BEFORE bulk   403 {"error":"Record access denied","code":"RECORD_NOT_ACCESSIBLE"}
  AFTER  bulk   403 {…same error/code…,"object":"account"}

INVALID_FIELD  (engine.ts sets status=400/code/field/fields/object)
  BEFORE bulk   400 {"error":"Unknown field 'emial' on object 'account'","code":"INVALID_FIELD"}
  AFTER  bulk   400 {…same error/code…,"field":"emial","object":"account"}

userMessage carrier (#9934), on a DELETE_RESTRICTED marked by its producer
  BEFORE bulk   409 {"error":"…","code":"DELETE_RESTRICTED","userMessage":"请先处理关联的联系人。"}
  AFTER  bulk   409 {"error":"…","code":"DELETE_RESTRICTED","object":"account","userMessage":"请先处理关联的联系人。"}

Every field that appears where it did not before

developerMessage, dependentObject, dependentCount, object (DELETE_RESTRICTED) · currentVersion, currentRecord, object (CONCURRENT_UPDATE) · developerMessage, field, object (DUPLICATE_RECORD) · object (FEEDS_DISABLED, FILES_DISABLED, ATTACHMENT_PARENT_ACCESS, ATTACHMENT_DELETE_DENIED, RECORD_NOT_ACCESSIBLE) · field, object (INVALID_FIELD).

Where these keys come from, corrected under condition 1. An earlier revision of this body said they are "already declared on ApiErrorSchema". ⛔ That was false and is withdrawn. packages/spec/src/api/contract.zod.ts declares exactly code, declaredCode, message, userMessage, category, httpStatus, details, requestId — and none of these keys (re-measured on this tree: zero declarations for all seven). What is true is narrower: every one of them is a key the single-record /data door already ships for the same refusal, and this change puts it on the bulk doors too. Their published status, measured on origin/main:

key published status
field, object documented — content/docs/protocol/kernel/http-protocol.mdx, the 409 "Constraint Violations" body
developerMessage, dependentObject, dependentCount documented — content/docs/protocol/objectql/types.mdx ("Required foreign keys") and content/docs/api/data-api.mdx
currentVersion documented — content/docs/api/wire-format.mdx §7, "Concurrent Update — 409 Conflict"
currentRecord shipped but undocumented — no content/docs/** page describes it (the only textual match is the unrelated currentRecordCount tenant quota)

⚠️ One deviation from the verdict's literal text, stated rather than slipped in: condition 1 asks that both currentVersion and currentRecord be named shipped-but-undocumented. Re-measured, currentVersion is documented — content/docs/api/wire-format.mdx:327 names it in the envelope sentence and :376 shows it in the "Concurrent Update — 409 Conflict" example body — and that file was present, with that section, at the reviewer's own base 3c1bbd2a87 (last touched 2026-08-31 by #13870, before the review). Writing it up as undocumented would put a new falsehood into the changeset in order to satisfy a condition whose stated purpose is "the sentence must be corrected to what is true". So currentRecord alone is named as undocumented. The reviewer's grep is otherwise confirmed: currentRecord has zero hits.

No key is removed anywhere. ⛔ "No status moves anywhere" — an earlier claim in this body — is not strictly true; see "Two status moves" below.

Every field whose VALUE changes

Exactly two, both on DuplicateRecordError:

  • code: DUPLICATE_RECORDUNIQUE_VIOLATION.
  • error: the engine's sentence → the curated one ("A record with this email already exists").

Both are restorations, and this is the one row a reviewer should weigh most:

driver-memory, corrected under condition 7. An earlier revision of this body said that on driver-memory "UNIQUE_VIOLATION on the bulk doors is new rather than restored". ⛔ That was backwards and is withdrawn. packages/drivers/driver-memory/src/memory-unique-constraint.ts conflictRefusal sets err.code = UNIQUE_VIOLATION_CODE and err.status = UNIQUE_VIOLATION_STATUS on the raw refusal itself, so it took the passthrough — and the passthrough relays a registered code verbatim through thrownCodeFields. The bulk-door code was therefore already UNIQUE_VIOLATION on that driver before #14095, and it never moved. What changes for that driver is the sentence: its raw message quotes the offending values as JSON and the curated one does not, so the change there is a withheld value, not a new code.

⚠️ A vocabulary fork this puts side by side — disclosed under condition 2

The DUPLICATE_RECORD arm answers the wire spelling UNIQUE_VIOLATION (#14389's ruling). A batch or import row does not go through this classification at all: metadata-protocol's toRowApiError puts a thrown registered code on the row verbatim (ErrorCode.safeParse(err.code).success ? …, protocol.ts:1987), and import-runner.ts:293 (first?.code ?? e?.code ?? 'IMPORT_ROW_FAILED') does the same for row reports — deliberately, per #14095's changeset ("Import row reports improve … now DUPLICATE_RECORD").

⇒ After this change, a whole-request failure on POST /data/:object/batch or POST /data/:object/import answers UNIQUE_VIOLATION, while a row-level failure on the same route answers DUPLICATE_RECORD.

Neither half is new and neither is a regression — the fork has existed between the single door and the rows since #14389. What this PR does is move the UNIQUE_VIOLATION side onto the routes whose rows speak the other spelling, and the "one condition, one wire code" framing above is true of the DOORS, not of the rows beside them. Stating that plainly is the point of this section.

This PR does not pick a winner. The ledger's rule (packages/spec/src/api/error-code-ledger.zod.ts header — "a code either IS the standard member for its condition, or it is registered here — and if it merely re-spells a standard member, that registration is a recorded waiver") and ADR-0112's one-name-per-concept both bear on it, and moving either spelling is a published-contract change rather than a door's call. #14723 carries the decision.

Two status moves, stated under condition 5

Both are synthesised shapes that no producer in this repo actually throws, so no wire in service moves — but they are real, they were measured rather than reasoned, and they are pinned in §4 rather than described:

  1. A sandboxed producer declaring a 5xx with OBJECT_NOT_FOUND or INVALID_FIELD now keeps that 5xx. On origin/main such an error fell PAST the sandbox unwrap door (declared ≥ 500) into the arm below it and answered 404 / 400. The arms now carry an explicit !isSandboxOrigin clause, so it answers the declared status with the prose withheld — mapDataError 的显式状态直通只覆盖 4xx,数据路由上一个声明了 502/503 的生产者拿不回自己的状态码(与 resolveErrorResponse 不对等) #5582's rule, and the better answer. Measured: OBJECT_NOT_FOUND + declared 503 answered 404 {"error":"Object 'nope' is not registered",…} on both doors and now answers 503 {"error":"Internal server error","code":"OBJECT_NOT_FOUND"} on both; INVALID_FIELD + declared 502 moves on the single-record door only (the bulk door already answered 502).
  2. A structured arm answering a 5xx no longer overrides a 4xx the producer declared, on either door — the condition-4 fix described above. Only ERR_DATASOURCE_UNAVAILABLE's 503 answers a 5xx and its producer declares no status, so nothing moves on the wire; the two doors now agree where they answered 503 and 400.

⇒ "classifyDataError is behaviour-identical" is therefore not claimed. It is behaviour-identical except for exception 1, which is a deliberate consequence of the arms moving above the unwrap door.

The OBJECT_NOT_FOUND arm's docblock is corrected too (condition 5's second half): it justified its !isSandboxOrigin clause by "the sentence this arm ships is error.message", which is wrong for that arm — it ships the fixed sentence Object 'NAME' is not registered. The clause is justified by position only, and the docblock now says so and explicitly disclaims the sibling arm's reason.

Is any of this a breaking change on a published surface?

Stated plainly rather than argued away — the review decides, not me:

Which doors, and which are published

Every route whose catch calls handleRouteError / sendThrownError, enumerated from rest-server.ts by brace-matching each call site to its enclosing route registration (read only — the file is fenced out of this PR), plus each environment-scoped twin:

door published?
POST /api/v1/data/:object/createMany, /updateMany, /deleteMany, /batch, and POST /api/v1/batch published data API
POST /api/v1/data/:object/:id/clone published data API
GET /api/v1/data/:object/export published data API
POST /api/v1/data/:object/import, /import/jobs, and the five /api/v1/data/import/jobs/:jobId… routes published data API
GET /api/v1/discovery, GET /api/v1/openapi.json published
the /api/v1/meta/** family — types, diagnostics, _drafts, _migrate-stored, :type, :type/:name (GET/PUT/DELETE), references, layers, history, audit, publish, rollback, diff, published, book/:name/tree, object/:name/state/:field on the public /api/v1 surface; first-party Console/Studio is the intended consumer
GET /api/v1/ui/view/:object/:type on the public /api/v1 surface; first-party Console/Studio is the intended consumer

Two further families reach this same door through classifiedRefusalAnswer rather than through a route catch — added under condition 6, which the earlier table omitted:

door published? does its body change?
POST /api/v1/analytics/dataset/query (and its environment-scoped twin) published yes — it spreads every classified key onto its own envelope (const { error: refusalText, ...refusalFields } = refusal.body), and service-analytics throws INVALID_FIELD with status, field and object at three sites (analytics-service.ts:1710-1713, 1824-1827, 1954-1957), so that route's error body gains field and object
the record-share family published keys: no — it re-dresses only code / declaredCode / userMessage / error into the nested ADR-0112 D5 envelope. Its error SENTENCE does change for a DuplicateRecordError

The widened analytics body is newly pinned at key level (§6 of the new test asserts the exact key set ['code','error','field','object'], and that param is not relayed), because its own envelope tests — analytics-dataset-unlisted-refusal-envelope.test.ts and analytics-filter-refusal-envelope.test.ts — assert code and a message regex only, so nothing held the keys.

Not on these lists, and deliberately so: the single-record /data CRUD routes and GET /api/v1/data/:object call mapDataError directly — they are the reference behaviour this change converges the doors ON, and none of their bodies move.

Which affected code can actually reach which door: DUPLICATE_RECORD and the 403 capability/access gates through createMany / updateMany / batch / clone / import; DELETE_RESTRICTED and CONCURRENT_UPDATE through deleteMany / updateMany / batch; INVALID_FIELD through export (which lists) and the write-path unknown-field refusal on the bulk writes.

packages/rest/src/package-routes.ts is not affected: it defines its own local sendThrownError and never calls this one.

What deliberately does NOT change

The measurement triage left open

Triage asked for CONCURRENT_UPDATE's producer to be measured rather than assumed from the other two. Done, and posted on the issue: packages/metadata-protocol/src/protocol.tsclass ConcurrentUpdateError declares readonly code = 'CONCURRENT_UPDATE' and readonly status = 409, plus currentVersion and currentRecord. So the passthrough fires and all three of those, plus object, were dropped on the bulk doors. It behaves exactly as DELETE_RESTRICTED and DUPLICATE_RECORD do.

The pin triage asked for (guard 3)

packages/rest/src/error-response-structured-arm-door-parity.test.ts:

  • §1 pins the two doors against each other per producer; §2 names the restored fields per code; §3 pins handleRouteError against sendThrownError.
  • §4 pins the guards — and, under condition 4, every case now asserts BOTH doors, each one labelled CONVERGED, ACCEPTED DIVERGENCE (with the card that owns it) or MOVED (stated). The two shapes the review found pinned on one side only — the declared-4xx-vs-5xx-arm case and the sandboxed producer — are now asserted on both doors, and the two status moves above have their own cases.
  • §5 is the drift guard, and under condition 3 it now scans both halves of classifyDataError, not just the lifted one. A new bespoke arm added below the consult — exactly where OBJECT_NOT_FOUND and INVALID_FIELD sat before this card — is the shape that reproduces the defect, and it is now caught. Every error?.code === '…' literal in the whole function must be either a §1 parity case or an entry in an explicit allowlist, and the allowlist distinguishes two kinds: by-design (today: PERMISSION_DENIED, whose third limb is message-text-gated) and known-gap (an entry that MUST cite the card carrying it). A third test refuses stale cover: every entry must name a live arm, carry a reason, and — if a gap — cite a card.
  • That guard found a real one on its first run: RECORD_NOT_FOUND is a genuine remaining instance of this card's defect (recordNotFoundError declares status = 404 and object, so the bulk doors drop object). It is left open on purpose — its second limb is a message-text gate, and an eleventh body change after the verdict would be an unreviewed delta — and recorded as a known-gap citing rest: the GENERIC declared-status passthrough still disagrees on object between the two error doors — plus one bespoke arm (RECORD_NOT_FOUND) still reachable from one door only #14725 rather than quietly excused.
  • §6 pins the two classifiedRefusalAnswer families (condition 6).

Verification

⚠️ Everything below was re-run from scratch in this patch round, on the final head 9c87f281a5, by a third seat. The reviewer explicitly did not run the suite, typecheck, lint or the gates, and the previous seat's report was voided when its container was restarted mid-run. No reading here is inherited.

Every exit captured before any pipe (cmd > log 2>&1; EXIT=$?). All heavy runs went through scripts/pm/os-verify-lock.sh, verdicts read from its VERDICT line.

pnpm --filter '@objectstack/rest^...' build        → VERDICT command-exit 0   (dependency closure first)
pnpm --filter @objectstack/rest typecheck          → VERDICT command-exit 0
      tsc --noEmit  → exit 0
      check:test-typecheck → "OK — @objectstack/rest's test layer compiles under
      packages/rest/tsconfig.test.json; 0 file(s) / 0 error(s) / 0 pinned signature(s)"
pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2
                                                   → VERDICT command-exit 0
      Test Files  174 passed (174)
      Tests      2926 passed (2926)
pnpm lint   (whole repo: eslint . --no-inline-config, NOT narrowed)
                                                   → VERDICT command-exit 0

Is the new test file actually type-checked? Measured with --listFiles rather than asserted, and reported either way:

tsc -p tsconfig.test.json --noEmit --listFiles → exit 0, 520 files,
      error-response-structured-arm-door-parity.test.ts  1 hit
tsc -p tsconfig.json      --noEmit --listFiles → exit 0, 351 files,
      error-response-structured-arm-door-parity.test.ts  0 hits

tsconfig.json excludes **/*.test.ts by design (ci.yml gates that no test file reaches the published artifact), so the build program legitimately does not see it. typecheck runs both programs, and the test-layer one does — so the green typecheck genuinely covers the new file.

Gate union, re-derived on the final tree after git merge origin/main:

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands
  → "gate list derived from the tree of 'objectstack-ai/objectstack' at commit 9c87f281a5"
    "--repo … checked against this checkout's 'origin' remote — it holds"
    change set: 3 path(s) vs merge base 7a17f3bf1 of 'origin/main' and HEAD (three-dot semantics)
    36 command(s) — 20 pnpm, 16 direct node
  → 34 exit 0 · 2 exit 3 (NOT MEASURED)

The two that did not measure — ⛔ neither a red, neither a pass, each quoted in its own words:

  • node scripts/check-test-completeness.mjs — exit 3: "That list names this script with NO argument, which is this branch. There is no local log to hand it, so the local reading for this gate is NOT MEASURED. ⛔ It is not a red, and there is nothing here to fix." CI tees the turbo run test log and passes the path, so CI behaviour is unchanged.
  • node scripts/pm/check-half-states.mjs — exit 3: "Nothing was swept: no issue was listed, no predicate (H1–H16) ran … It is not a clean board and it is not a dirty one — it is no reading at all." This seat's egress denies repo-scoped REST (probed directly: GET /repos/objectstack-ai/objectstack/issues/14541 answers 403), which is the prerequisite it is refusing on.

⚠️ One run that had to be repeated, reported rather than quietly dropped. pnpm check:type-check-debt (--self-test && --re-measure) is a repo-scale scan, and on its first attempt the container's 10-minute foreground ceiling killed the --re-measure phase with SIGTERM (exit 143) — a resource kill, ⛔ not a finding. Re-run detached from that ceiling it passed, exit 0 in 179s under the verify lock, and that is the reading counted above. Its sibling pnpm check:type-check-coverage — the same script without --re-measure — passed, exit 0, on the first attempt. Both readings are stated so neither is hidden. (The previous seat hit the same ceiling on this same one gate, with SIGKILL 137.)

Changeset

patch on @objectstack/rest, by the precedent in this repo for response-contract fixes on shipped public routes (analytics-dataset-query-declared-5xx-relay.md, analytics-dataset-query-user-message.md — both patch, both response-body changes on published doors; the latter's own framing is "exactly one optional key is ADDED"). Nothing here is removed, the one code VALUE change restores the spelling those same doors answered before #14095, and the two status moves are on synthesised shapes no producer in this repo throws.

Notes


🤖 Generated with Claude Code

https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 12 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 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 f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7964fd61772560903b0b639d1ca17afb828cd657 — the merge of head d595638d6a1b007e7c951670a83c1f39561cd118 into base f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8, 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 7964fd61772560903b0b639d1ca17afb828cd657 && git checkout 7964fd61772560903b0b639d1ca17afb828cd657
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8 d595638d6a1b007e7c951670a83c1f39561cd118 && git checkout -B drift-repro f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8 && git merge --no-ff d595638d6a1b007e7c951670a83c1f39561cd118

node scripts/docs-audit/affected-docs.mjs --json f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

os-trump and others added 4 commits September 2, 2026 20:44
…orrect three docblock claims

Contract-review conditions 2, 4(A), 5 and 7 on #14541.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…or drift, pin the refusal families

Contract-review conditions 3, 4, 5 and 6 on #14541. The extended drift guard
found a real tenth instance on its first run (RECORD_NOT_FOUND), recorded as a
known-gap entry citing #14725 rather than silently excused.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…asured wrong

Conditions 1, 2, 5, 6 and 7 on #14541: the added keys are not ApiErrorSchema
members; the row/door vocabulary fork is disclosed and carried by #14723; the
two status moves are stated with per-door measurements; the analytics and
record-share families are enumerated; the driver-memory direction is corrected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza

os-trump commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Red-first ablation, re-run in patch round 2 — belongs to the Verification section above.

The previous seat's ablation was taken against the pre-condition test file and its report was voided with its container, so it could not speak for §5 and §6 — the two sections conditions 3 and 6 added. This one was taken against the test as it now stands, on head 9c87f281a5.

Discipline: the implementation was already committed, so the restore leg points at a HEAD that actually contains it; mutation and restore are each proven on disk by content and by blob identity; the script carries trap restore EXIT INT TERM on absolute paths resolved from git rev-parse --show-toplevel; and an empty hash is treated as failure rather than as "nothing to compare".

HEAD_BLOB = 16c523968355ff477659827ed0f5f7e7cb6fc353
BASE_BLOB = 04de83069936670b6de25700c0d0f1f37d24143b   (origin/main)

BEFORE MUTATION  structuredCodeAnswer=6  isSandboxOrigin=6  fiveXxArm=5  #14541=10  lines=2348
                 on-disk hash = 16c5239683…  == HEAD blob
AFTER  MUTATION  structuredCodeAnswer=0  isSandboxOrigin=0  fiveXxArm=0  #14541=0   lines=2110
                 on-disk hash = 04de830699…  == BASE blob, != HEAD blob
                 ⇒ MUTATION CONFIRMED ON DISK (not inferred from an editor's exit code)

RED RUN   exit 1 — Test Files 1 failed (1) · Tests 25 failed | 21 passed (46)
          failures by section:  §1 x11   §2 x9   §4 x4   §5 x1   §6 x2

RESTORED  on-disk hash = 16c5239683…  == HEAD blob
          `git diff HEAD` = []  (empty)
          markers back: structuredCodeAnswer=6
          ⇒ RESTORE CONFIRMED by state, not by exit code

Why no rebuild is involved, stated rather than assumed. The test imports its subject relatively — from './error-response.js' at line 81 — so vitest resolves it to the mutated source, not across a package exports boundary into dist. Its two cross-package imports (@objectstack/objectql, @objectstack/metadata-protocol) do resolve to dist, and the dependency closure was built first (pnpm --filter '@objectstack/rest^...' build, VERDICT command-exit 0), so those are real artifacts. No vitest alias redirects the specifier.

The reading that matters, and the reason this was re-run rather than inherited: §5 and §6 go red too. A drift guard and a key-level pin that stayed green against the pre-fix file would have been cover rather than a check — the mutation removes structuredCodeAnswer entirely, and §5's slice assertion and §6's analytics key-set assertion both fail, so neither is vacuous.

Predicted direction was RED, and RED is what was observed; nothing here reversed or produced more diagnostics than expected.


Generated by Claude Code

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33719431395 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test:  FAIL   integration  test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
      ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 75 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33721321327 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test:  FAIL   integration  test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
      ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • ⚠️ 本 PR 过去 24h 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 79 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33722710905 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/6) — 失败步骤: Run this shard's tests

    @objectstack/cli:test:  FAIL   integration  test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
      ↳ 失败原因: @objectstack/cli:test: AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 1801
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

⚠️ 断言这一侧有一类例外,判据是断言在测什么,不是它是不是 AssertionError 断言的对象是产品行为(一个值、一个形状、一次拒收)⇒ 照上面读:真实的行为改变,去查,⛔ 不要重排掉;
断言的对象是这次实验自身的有效性前提(跑完的耗时、负载下的先后、任何只在时间预算内才成立的条件)⇒ 它跟超时是同一类,同样对负载敏感,重排一次是合法的判别手段。
识别是机械的:断言的消息或它比较的值本身点名了一段时长、一个时间戳、一个耗时计数。实测过的一对 —— AssertionError: SecurityPlugin.init() ran: expected false to be true 测的是产品行为(真回归);
AssertionError: this run took over a second, so second-precision stamps could have differed too: expected 1006 to be less than 1000 测的是实验前提:它守护的那条不变式当时是绿的,同一个 head 原样重排一次即成功。
穿着 AssertionError 外衣的时间测量,仍然是时间测量。(⛔ 这只改「怎么读一次红」,不改「哪些测试可以重排」——后者由别处管。)

跨 PR 相同签名(24h,按失败测试文件聚合):

历史信号:

  • ⚠️ 本 PR 过去 24h 已在队列失败 2 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 86 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@os-project-manager
os-project-manager added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit 6d178a4 Sep 3, 2026
34 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-14541-error-response-passthrough-ordering branch September 3, 2026 13:09
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/xl tests tooling

Projects

None yet

3 participants