fix(metadata-protocol): deleteMetaItem's catch re-wrap carries the error code (#7426) - #7466
Conversation
…ror code (#7426) `deleteMetaItem` is the only verb in `protocol.ts` that re-wraps a thrown error instead of rethrowing it — both of its catches build a fresh `Error` carrying the "failed to delete" context. They carried `status` forward and dropped `code`, so a `SysMetadataRepository` refusal with a full ADR-0112 envelope reached the caller as 403 with `code: undefined`, its code surviving only as prose in the message. The envelope therefore depended on the deployment topology: a project kernel got `NOT_OVERRIDABLE` from `deleteMetaItem`'s own two-tier block, a control-plane kernel — which skips that block — got the code-less 403. Both re-wrap exits now carry `code`, gated on membership in the declared ADR-0112 vocabulary (StandardErrorCode ∪ ERROR_CODE_LEDGER) — verbatim the predicate `toRowApiError` in the same file already applies to thrown codes. A driver's own dialect (42P01, SQLITE_CONSTRAINT, ECONNREFUSED) is not in the catalog and stays out of the envelope, so the wrapper cannot smuggle an unregistered code onto a field `ApiErrorSchema` declares as a closed union. `status` is untouched at both sites, and the message text is unchanged. The new pin walks the whole failure-kind matrix through those catches rather than the one symptom, and #6960's own pin drops its topology branch: `expectRefused` now asserts `code` + `status` on every leg instead of falling back to a message substring on the control-plane one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TyY4g6DC854MCzyJDRh5FR
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
ACCEPT — PM review (step 7)Head File surface vs declarationDeclared four files; the diff is exactly four, and the line counts match the report:
CI, per job25 runs, all What I am accepting⭐ You read my STOP constraint more precisely than I wrote it, and the distinction is the right one. I said "touching D9's pin file is a STOP". You left ⭐ The affected-failure-kinds table is what this card was really about, and it is complete. Eight kinds, each with before / after / what pinned it / disposition — including two entries that are measured unreachable rather than assumed away (the runtime-only refusal, because ⭐ The deliberate non-fix is the load-bearing decision, and the variant experiment proves it. Uncatalogued driver codes ( ⭐ The missed prediction is reported and is genuinely instructive. Promoting the message-substring check from the control-plane leg to every leg went red on the project kernel, because the two producers spell the marker differently in prose — The triage seat's open question is answered with a measurement: no sibling verb carries this shape. Predicted 17 red on revert, measured 17. Unmeasured: none. And the two reds landing in #6960's pin were flagged as carrying a Two notes for the recordCloud-mode artifact, disclosed and acceptable: the container's primary checkout was pre-created on the designated branch (a consequence of Open question 3, judged and left as judged: the legacy raw-engine exit keeps a literal RulingACCEPT. Flipping to ready, auto-merge SQUASH. Open question 1 — tightening D9's control-plane pin, now that it can be tightened — is being filed as its own card rather than left in this thread. Generated by Claude Code |
Fixes #7426
Premise check first
The issue was measured on
f16e54e1d, and #6960 landed a delete carve-out in between — so the premise was re-measured onorigin/main@08363a09before a line was written. It holds: on a control-plane kernel (environmentId === undefined), deleting an artifact-backed item of asupportsOverlay: falsetype answersstatus: 403withcode: undefined, the code surviving only as text inside the message. Measured as 15 red in the new pin file against unmodifiedmain(details under Reverse verification).premise_still_valid: true.What changed
deleteMetaItemis the only verb inprotocol.tsthat re-wraps a thrown error instead of rethrowing it — measured, not assumed: the file has exactly twonew Error(\Failed to …`)re-wrap sites and both are in this method, while every sibling verb (saveMetaItem,publishMetaItem,revertCommit,rollbackMetaItem) translatesConflictErrorand thenthrow errs the original untouched. That answers triage's "check thesaveMetaItemsiblings in the same pass": **there is nothing to check — the sibling shape does not exist.** (saveMetaItem`'s was deleted with its legacy raw-engine branch in #5264.)Both re-wrap exits now carry
codeforward, gated on membership in the declared ADR-0112 vocabulary (StandardErrorCode ∪ ERROR_CODE_LEDGER). That predicate is not new: it is verbatim whattoRowApiError, 11k lines up the same file, already applies to decide which thrown code may become a wire code.statusis untouched at both sites, and the message text is unchanged.The re-wrap is the exit of every non-conflict failure on this path, so changing it changes the envelope of every kind at once. Full enumeration — each row measured through the real path, not read off the source:
override-artifact(NOT_OVERRIDABLE) — control-plane kernel, the wholesupportsOverlay:false && allowRuntimeCreatetier:actiondatasourcedocexternal_catalogfieldflowhookmappingobjectseed403, no code403+NOT_OVERRIDABLEruntime-only(NOT_CREATABLE)useRepoPathrequires the protocol'sisRuntimeCreateAllowed/isOverlayAllowed, and the repository'sassertAllowedreads the same registry, so it returns before it can throw. No disposition needed; recorded so its absence is not read as an omission.ERR_DATASOURCE_UNAVAILABLE,ERR_DRIVER_CONNECT) — at the probe read and inside the transactionstatusonly, no codestatus+ that code42P01,SQLITE_CONSTRAINT,ECONNREFUSED,ER_DUP_ENTRY)500, no code[GUARD]casesError, no code (history-table outage, missingidcolumn, closed repo)500, no codeConflictError(optimistic-lock / row vanished)409+METADATA_CONFLICT500, no code500+ code, same predicate;statusstays the literal500Row 8 is the reason this is not a one-line fix applied to one site. Fixing only the repository catch would leave
deleteMetaItemanswering an envelope that varies by which path served the delete — harder to reason about than the gap it replaced. Thecoderule is unified across both exits; each catch'sstatusexpression is left exactly as it was, because the legacy path carries no authorization gate (deliberately, #5264) so every failure through it really is a fault, and moving its status is a separate contract decision this card does not take.Real file surface, file by file
packages/metadata-protocol/src/protocol.tscarryCatalogedErrorCode(next totoRowApiError, whose predicate it reuses), and one line added to each of the two catches indeleteMetaItem— the repository exit (~:12391) and the legacy raw-engine exit (~:12483). No control flow, no status expression, no message text touched.packages/metadata-protocol/src/protocol.delete-rewrap-envelope.test.tspackages/metadata-protocol/src/protocol.legacy-overlay-delete.test.tsexpectRefusedbranched on topology and could only reach the code as a message substring on the control-plane leg — with a comment saying so and naming #7426. That comment would be false after this change, so it is rewritten and the branch is deleted: every leg now assertscode+status..changeset/delete-meta-item-rewrap-carries-code.md@objectstack/metadata-protocol.Not touched, and both were STOP conditions carried into the dispatch:
packages/objectql/src/protocol-object-overlay-layer.test.ts— ADR-0029 D9's pin. Its control-plane case assertstoContain('NOT_OVERRIDABLE')on the message because of this defect. This PR makes that pin tightenable; tightening it belongs to a follow-up, and the file is byte-identical here. (Its whole suite is green:objectql176 files / 3127 tests.)packages/specacceptance surface andpackages/restbehaviour — read only.restalready forwards a declaredcodewhen the producer sets one (resolveErrorResponse, both the 5xx and 4xx limbs), so the improvement reaches the wire with no change on that side;rest-5xx-message-sanitization.test.tsre-run green (13/13), including its live-protocol section that walks this exact catch with a failing driver.Reverse verification
Direction declared before each run; base is this branch's own base SHA
08363a09, never a movingmain.1. Predicted red, measured red — 17/17, exact.
Reverting only
protocol.tsto the base SHA while keeping both test files:protocol.delete-rewrap-envelope.test.ts— the 10-type refusal sweep, the plural spelling, the cross-topology equality, the 2 catalogued-engine-code cases, the legacy-path catalogued case. Every one fails oncodebeingundefinedwhilestatusis already correct, which is the defect stated as an assertion.protocol.legacy-overlay-delete.test.ts— the two control-plane legs ofexpectRefused.[GUARD]label in their names; that label is A legacy env overlay on an artifact-backed item of a rolled-back type can no longer be REMOVED through the ordinary delete path (403) — only via OS_METADATA_WRITABLE #6960's, scoped to A legacy env overlay on an artifact-backed item of a rolled-back type can no longer be REMOVED through the ordinary delete path (403) — only via OS_METADATA_WRITABLE #6960's change, and they are genuine red-in-reverse evidence for this one. Left as-is rather than relabelled: renaming another card's guard would misreport what it guards.2. Green in both directions — GUARDS, labelled
[GUARD]in the report and in the test file.The 4 uncatalogued-driver-code cases, the plain-
Errorcase, theConflictErrorcase, the post-persistence-tail case, the project-kernel refusal case, and the vocabulary premise case. They pass before and after. What makes them load-bearing is the variant experiment, declared and run: replacing the predicate with an unconditionalif (typeof code === 'string')copy — the over-broad shape the issue names — turned exactly 5 red (42P01,SQLITE_CONSTRAINT,ECONNREFUSED,ER_DUP_ENTRY, and the legacy-path twin) and nothing else, which is the damage case those guards exist to catch. Prediction and measurement matched exactly. Variant reverted.3. Missed predictions — one, kept rather than tidied away.
Promoting
expectRefused's message substring check to every leg was predicted green-in-both and measured red on the project kernel: the two producers spell the marker inside their prose differently —deleteMetaItem's own block writes[not_overridable],SysMetadataRepositorywrites[NOT_OVERRIDABLE]. Only thecodefield is uniform, which is ADR-0112's own point (the catalog governserror.code; message prose is a different surface) and is precisely why the field is the assertion that belongs there. The check is now case-insensitive and the correction is written into the file. The prose divergence is pre-existing and outside this card.4. Unmeasured — none. Every assertion in the new file was run in both directions; the ones that cannot go red on this change say so in their own names and are covered by the variant experiment above.
distwas rebuilt (pnpm --filter @objectstack/metadata-protocol build) before any dependent suite was read.Gates
check:error-code-casingcheck:route-envelopecheck:engine-double-contractcheck:nul-bytesmetadata-protocolsuiteobjectqlsuiterest-5xx-message-sanitization.test.tsThe rest of the lint farm is left to CI by instruction.
Follow-ups this deliberately does not take
code+statuslike its project-kernel sibling — now possible, out of scope here by explicit dispatch constraint.[not_overridable]/[NOT_OVERRIDABLE]prose divergence between the two producers (finding 3 above). Cosmetic for machines, confusing for humans grepping logs; not anerror.codedefect.Generated by Claude Code