Skip to content

feat(spec,rest): register READ_BACK_FAILED as a named wire row mapped in handleApprovalError - #13296

Merged
hotlong merged 3 commits into
mainfrom
claude/issue-13182-read-back-failed-wire-row
Aug 30, 2026
Merged

feat(spec,rest): register READ_BACK_FAILED as a named wire row mapped in handleApprovalError#13296
hotlong merged 3 commits into
mainfrom
claude/issue-13182-read-back-failed-wire-row

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

Part of #13182 — executing the RULED Option B (maintainer, 2026-08-29, verbatim 「同意」): register READ_BACK_FAILED as a named wire row, aligned with the RESUME_FAILED precedent. This PR deliberately does not close the card — a residual noted below lands as a follow-up commit on this branch once PR #13181 merges. #12769 is not addressed here (it belongs to PR #13181); #13181 remains open territory of its own review.

What this delivers (ruling point 1 — complete)

  • Error-code ledger rowREAD_BACK_FAILED registered under @objectstack/rest in packages/spec/src/api/error-code-ledger.zod.ts, trailing provenance comment carrying the substance (write recorded, read-back filtered by the caller's org scope, NOT rolled back), no tracker id — mirroring the RESUME_FAILED row shape.
  • handleApprovalError mapping armpackages/rest/src/rest-server.ts maps the READ_BACK_FAILED: message prefix to HTTP 500 with code: 'READ_BACK_FAILED' on the wire, directly under the RESUME_FAILED arm it mirrors (same dispatch mechanism: message-prefix regex, measured, not assumed). 500 semantics stay (ruling point 2): it is genuinely a server-side inconsistency.
  • Doc line — the IApprovalService contract docblock (packages/spec/src/contracts/approval-service.ts) now states the refusal contract: the write is recorded and NOT rolled back; read the request back with a system or matching-organization context. Substance without tracker ids, per the doc-authoring rule; verified against the precedent — RESUME_FAILED's ledger row and thrown messages carry no tracker ids either.
  • Generated referenceserror-code-ledger.mdx and contract.mdx regenerated via check:generated --fix (the only artifact proved stale; diff is exactly the two list lines plus the enum count).
  • Pin testrest-approvals-wire-codes.test.ts gains an org-filtered read-back case: 500, code: 'READ_BACK_FAILED', prefix-stripped body sentence, closed-union membership. Reverse-verified by ablation: with the mapping arm removed the case goes red in the predicted direction (the throw falls through to the template-generated APPROVAL_APPROVE_FAILED arm — exactly the misnamed-code state the card describes); restore proved byte-identical against HEAD by blob hash.
  • Changesetminor for @objectstack/spec and @objectstack/rest, matching how RESUME_FAILED was introduced. Wire vocabulary change is ADDITIVE only (ruling point 4): no existing code, status, or message changes.

Sequencing — do NOT land before PR #13181

The thrower (ApprovalService.readBackRequest throwing the READ_BACK_FAILED: prefix, from #12769) is NOT on origin/main yet: measured at dispatch, git grep READ_BACK_FAILED origin/main answers zero hits, and PR #13181 sits in the merge queue (page state QUEUED, unmerged). Until #13181 lands, this row would be registered-but-unemittable — the exact class the ledger header names as ADR-0112's "no silent fourth state" read backwards (the OVERLAY_PERSISTENCE_FAILED precedent). The review chain owns enqueue for this clause-② PR; it must land only after #13181 is on main.

Residual — pending follow-up commit on this branch, blocked on #13181 landing

Ruling point 3 plus the standing doc-authoring rule: the thrown message on PR #13181's branch ends with a "(#12769)" tracker-id suffix, and customer-facing prose carries no internal tracker ids (the RESUME_FAILED thrown messages carry none — verified). Once #13181 is on main, this branch merges main and trims that suffix (substance unchanged, one line in packages/plugins/plugin-approvals/src/approval-service.ts). Authoring that edit now would mean importing an unlanded PR's diff, which the dispatch forbids.

Verification (all at commit 21f8c61)

  • pnpm --filter @objectstack/spec run test — 443 files, 11776 tests, all pass
  • pnpm --filter @objectstack/rest run test — 158 files, 2624 tests, all pass
  • pnpm --filter @objectstack/plugin-approvals run test — 34 files, 626 tests, all pass
  • typecheck green for spec, rest, plugin-approvals (note: rest's tsc excludes test files by config; the edited test file is covered by the check:test-typecheck half of the same script, and by the vitest run itself)
  • All 39 dispatch-derived local gates green — including check:error-code-casing, check:dispatcher-error-vocabulary, check:generated, check:docs, check:authorable-surface, and check:dual-build-cjs-loads (after a full pnpm build; its earlier refusal was PREREQUISITE NOT MET, not a red) — plus check:nul-bytes
  • Ablation legs: mutation proved on disk by targeted line-count (arm lines 1 to 0), test red with expected READ_BACK_FAILED, received APPROVAL_APPROVE_FAILED, restore via git checkout HEAD verified byte-identical (blob hash d538f059 matches HEAD), suite re-run green on the restored tree

Session: https://claude.ai/code/session_01KX8wnyjStaZcuMyAMNsy3N

Generated by Claude Code


Generated by Claude Code

…p it in handleApprovalError
Part of #13182 (ruled Option B, aligned with the RESUME_FAILED precedent).
Ledger row + handleApprovalError mapping arm + contract doc line + generated
references docs + wire-code pin test. 500 semantics stay; additive only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KX8wnyjStaZcuMyAMNsy3N
@github-actions

github-actionsBot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-approvals, @objectstack/rest, @objectstack/spec, touching 2 documentable anchor(s). ⚠️2 changed file(s) yielded no anchor (packages/rest/src/rest-server.ts, packages/spec/src/contracts/approval-service.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx(via ERROR_CODE_LEDGER (symbol))
  • content/docs/api/error-catalog.mdx(via ERROR_CODE_LEDGER (symbol))
  • content/docs/api/error-handling-server.mdx(via ERROR_CODE_LEDGER (symbol))
  • content/docs/kernel/contracts/data-engine.mdx(via ERROR_CODE_LEDGER (symbol))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via ERROR_CODE_LEDGER (symbol))

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.

What this run could not see
  • 2 changed file(s) yielded no anchor (packages/rest/src/rest-server.ts, packages/spec/src/contracts/approval-service.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 — 128 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 426ad5890e72a128d7fb0e63bfe9ea6cfed0dcbdpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 224fb0f5fbeca161c7dc5bac44c93617c8fd2d71 — the merge of head 5a16c9e104da2b61f1f0c19af2f816271d0957b5 into base 426ad5890e72a128d7fb0e63bfe9ea6cfed0dcbd, 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 224fb0f5fbeca161c7dc5bac44c93617c8fd2d71 && git checkout 224fb0f5fbeca161c7dc5bac44c93617c8fd2d71
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 426ad5890e72a128d7fb0e63bfe9ea6cfed0dcbd 5a16c9e104da2b61f1f0c19af2f816271d0957b5 && git checkout -B drift-repro 426ad5890e72a128d7fb0e63bfe9ea6cfed0dcbd && git merge --no-ff 5a16c9e104da2b61f1f0c19af2f816271d0957b5
node scripts/docs-audit/affected-docs.mjs --json 426ad5890e72a128d7fb0e63bfe9ea6cfed0dcbd

⚠️ 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 426ad5890e72a128d7fb0e63bfe9ea6cfed0dcbd → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added size/s documentation Improvements or additions to documentation tests tooling labels Aug 29, 2026
…K_FAILED refusal text
Part of #13182. Patch round on PR #13296: merges main (b579b03, #13181)
and drops the tracker-id suffix from the read-back refusal message —
matching the wire-code test fixture in rest-approvals-wire-codes.test.ts,
which already mocked the message without it.
@os-project-managerClaude

Copy link
Copy Markdown
CollaboratorAuthor

os-dev-report (patch round)

Patch round on PR #13296 per PM dispatch (main's #13181 landed as b579b038). Two items only, both done.

1) merge main
git merge origin/main --no-edit from branch head 21f8c61a onto origin/main at 289cf91c (which includes b579b038#13181). Clean merge, zero conflicts (git status empty after merge, nothing to resolve). Merge commit 5d2d08ec.

2) trim the (#12769) tracker-id suffix

  • Before merge: grep -n '(#12769)' packages/plugins/plugin-approvals/src/approval-service.ts — 0 hits (the suffix lives on origin/main, brought in by the merge, not on the pre-merge PR branch).
  • After merge: same grep — 1 hit, line 4783 (the readBackRequest refusal text). A second, unrelated occurrence at line 4761 is a JSDoc reference in [#12769] bracket form (not the trailing-parens suffix this task targets) — left untouched per "只摘这个尾缀,文案其余部分不动".
  • Edit: dropped (#12769) from the end of the thrown READ_BACK_FAILED: message. Substance unchanged.
  • After trim: same grep — 0 hits.
  • Confirms alignment with this PR's own fixture: packages/rest/src/rest-approvals-wire-codes.test.ts already mocks the read-back message without the suffix (verified by grep before editing), so the trim brings approval-service.ts in sync with what feat(spec,rest): register READ_BACK_FAILED as a named wire row mapped in handleApprovalError #13296's own pin test expected all along.

3) reverify

  • Build: pnpm --filter @objectstack/plugin-approvals... build — full dependency closure, green (os-verify-lock VERDICT command-exit 0).
  • Tests: pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2 — 34 files / 632 tests, all passed. Targeted rerun of approval-service.test.ts alone (verbose reporter): 298/298 passed, including all sys_approval_delegation, quorum, decision_progress and status-mirror suites. (The ERROR-level log lines in the run are expected fault-injection noise from unrelated driver tests, not failures.)
  • Gates derived via node scripts/pm/dispatch-gates.mjs for the file actually edited (approval-service.ts): check:nul-bytes, check:keyed-text-bounds, check:dispatcher-error-vocabulary, check:comment-mask-adoption, check:plugin-teardown-shape, check:undeclared-dep-imports, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:where-matcher, check:spec-parsed-alias, check:i18n-stale-fill — all green (each printed its own PASS marker, joined with &&/checked individually after the tool's pipefail warning on a ;-joined batch).
  • check:i18n — NOT MEASURED: it needs the CLI + a 9-package build closure it doesn't otherwise need. Confirmed not applicable to this edit before skipping the build: the trimmed string is a raw throw new Error(...) inside a service method, not present in any *.object.ts metadata declaration or in packages/plugins/plugin-approvals/src/translations/, and the plugin's i18n-extract.config.ts runs --objects-only extraction, which cannot see this string.

Head sha: 5a16c9e104da2b61f1f0c19af2f816271d0957b5 (pushed, no force). Draft state and needs:contract-review label untouched, as instructed.


Generated by Claude Code

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

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@os-project-manager@hotlong@claude