Skip to content

fix(metadata-protocol): deleteMetaItem's catch re-wrap carries the error code (#7426) - #7466

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-7426-delete-rewrap-code
Aug 10, 2026
Merged

fix(metadata-protocol): deleteMetaItem's catch re-wrap carries the error code (#7426)#7466
os-zhuang merged 1 commit into
mainfrom
claude/issue-7426-delete-rewrap-code

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

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 on origin/main @ 08363a09 before a line was written. It holds: on a control-plane kernel (environmentId === undefined), deleting an artifact-backed item of a supportsOverlay: false type answers status: 403 with code: undefined, the code surviving only as text inside the message. Measured as 15 red in the new pin file against unmodified main (details under Reverse verification).

premise_still_valid: true.

What changed

deleteMetaItem is the only verb in protocol.ts that re-wraps a thrown error instead of rethrowing it — measured, not assumed: the file has exactly two new Error(\Failed to …`) re-wrap sites and both are in this method, while every sibling verb (saveMetaItem, publishMetaItem, revertCommit, rollbackMetaItem) translates ConflictErrorand thenthrow errs the original untouched. That answers triage's "check the saveMetaItem siblings 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 code forward, gated on membership in the declared ADR-0112 vocabulary (StandardErrorCode ∪ ERROR_CODE_LEDGER). That predicate is not new: it is verbatim what toRowApiError, 11k lines up the same file, already applies to decide which thrown code may become a wire code. status is untouched at both sites, and the message text is unchanged.

⚠️ The hard part of this card: the blast radius, kind by kind

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:

# Failure kind Envelope BEFORE Envelope AFTER Pinned by
1 Repository authorization refusal, override-artifact (NOT_OVERRIDABLE) — control-plane kernel, the whole supportsOverlay:false && allowRuntimeCreate tier: action datasource doc external_catalog field flow hook mapping object seed 403, no code 403 + NOT_OVERRIDABLE new file (10 cases + plural + cross-topology)
2 Repository authorization refusal, runtime-only (NOT_CREATABLE) Unreachable through this catch, measured. useRepoPath requires the protocol's isRuntimeCreateAllowed/isOverlayAllowed, and the repository's assertAllowed reads the same registry, so it returns before it can throw. No disposition needed; recorded so its absence is not read as an omission.
3 Engine failure carrying a registered code (ERR_DATASOURCE_UNAVAILABLE, ERR_DRIVER_CONNECT) — at the probe read and inside the transaction status only, no code status + that code new file (2 cases)
4 Engine failure carrying an unregistered driver code (42P01, SQLITE_CONSTRAINT, ECONNREFUSED, ER_DUP_ENTRY) 500, no code unchanged — deliberately dropped new file, 4 [GUARD] cases
5 Plain Error, no code (history-table outage, missing id column, closed repo) 500, no code unchanged new file (1 case)
6 ConflictError (optimistic-lock / row vanished) 409 + METADATA_CONFLICT unchanged — caught one branch above the re-wrap, never passes through it new file (1 case)
7 Post-persistence tail: registry heal, table drop, audit write, ADR-0094 projector, mutation listeners never reaches the catch unchanged new file (1 case that makes three of them throw at once and asserts the receipt survives)
8 Legacy raw-engine exit (code-only types on a control-plane kernel) — the second re-wrap 500, no code 500 + code, same predicate; status stays the literal 500 new file (2 cases)

Row 8 is the reason this is not a one-line fix applied to one site. Fixing only the repository catch would leave deleteMetaItem answering an envelope that varies by which path served the delete — harder to reason about than the gap it replaced. The code rule is unified across both exits; each catch's status expression 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

File What changed
packages/metadata-protocol/src/protocol.ts +1 module-scope helper carryCatalogedErrorCode (next to toRowApiError, whose predicate it reuses), and one line added to each of the two catches in deleteMetaItem — 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.ts New. The affected-kinds matrix above, executable — 26 cases over the real write path against a stub engine that injects a throw at a named seam (probe read / transactional delete / history insert / audit insert / registry heal).
packages/metadata-protocol/src/protocol.legacy-overlay-delete.test.ts #6960's own pin, tightened. Its expectRefused branched 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 asserts code + status.
.changeset/delete-meta-item-rewrap-carries-code.md patch changeset for @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 asserts toContain('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: objectql 176 files / 3127 tests.)
  • packages/spec acceptance surface and packages/rest behaviour — read only. rest already forwards a declared code when 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.ts re-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 moving main.

1. Predicted red, measured red — 17/17, exact.
Reverting only protocol.ts to the base SHA while keeping both test files:

2. Green in both directions — GUARDS, labelled [GUARD] in the report and in the test file.
The 4 uncatalogued-driver-code cases, the plain-Error case, the ConflictError case, 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 unconditional if (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], SysMetadataRepository writes [NOT_OVERRIDABLE]. Only the code field is uniform, which is ADR-0112's own point (the catalog governs error.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.

dist was rebuilt (pnpm --filter @objectstack/metadata-protocol build) before any dependent suite was read.

Gates

Gate Result
check:error-code-casing ✅ 17 self-test cases, 3443 files, no lowercase codes
check:route-envelope ✅ 8 modules + 16 dispatcher domains, 0 ratcheted
check:engine-double-contract ✅ 138 pinned, 133 DEBT, 2 exempt — unchanged
check:nul-bytes ✅ 6840 files
metadata-protocol suite ✅ 70 files / 1035 tests
objectql suite ✅ 176 files / 3127 tests (D9 pin file untouched and green)
rest-5xx-message-sanitization.test.ts ✅ 13/13 (affected downstream surface)

The rest of the lint farm is left to CI by instruction.

Follow-ups this deliberately does not take

  1. Tighten ADR-0029 D9's control-plane pin to assert code + status like its project-kernel sibling — now possible, out of scope here by explicit dispatch constraint.
  2. The [not_overridable] / [NOT_OVERRIDABLE] prose divergence between the two producers (finding 3 above). Cosmetic for machines, confusing for humans grepping logs; not an error.code defect.

Generated by Claude Code

…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
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 10, 2026 2:27pm

Request Review

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/metadata-protocol)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/v9.mdx (via @objectstack/metadata-protocol)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Copy link
Copy Markdown
Contributor Author

ACCEPT — PM review (step 7)

Head 09fa628a2b33688c8703d110e7e9339df3afb79b, base 08363a09.

File surface vs declaration

Declared four files; the diff is exactly four, and the line counts match the report:

.changeset/delete-meta-item-rewrap-carries-code.md   +40
packages/metadata-protocol/src/protocol.delete-rewrap-envelope.test.ts  +562
packages/metadata-protocol/src/protocol.legacy-overlay-delete.test.ts    42 ±
packages/metadata-protocol/src/protocol.ts                              +64

packages/objectql/ is absent from the diff entirely — that is the mechanical proof the D9 STOP held, not a claim I have to take on trust. No packages/spec, no packages/rest, no content/docs/releases/, no docs/adr/**.

CI, per job

25 runs, all completed: 23 success, 2 skipped (Build Docs, Console Pin Gate, path-filtered). ESLint success, TypeScript Type Check success, Build Core, Test Core 3/3 + rollup, Dogfood Regression Gate 3/3 + rollup, Dogfood Verify CLI, Temporal Conformance, Check Changeset, Check PR Size, ADR maintainer approval — all green. Zero failures.

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 packages/objectql/src/protocol-object-overlay-layer.test.ts byte-identical — and tightened a different file: protocol.legacy-overlay-delete.test.ts, #6960's own pin, whose expectRefused branched on topology and reached the code only as a message substring on the control-plane leg, with a comment naming #7426 as the reason. This change makes that comment false. Leaving it would have shipped a lie about why the assertion is weak. Deleting the branch so every leg asserts code + status is not scope creep; it is the cleanup this fix obliges. Exactly right, and I would have missed the distinction.

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 useRepoPath and assertAllowed read the same registry so it returns before it can throw; and the whole post-persistence tail, proven by a case that makes three of them throw at once and asserts the receipt survives). Recording why a kind is absent is what stops the next reader treating the absence as an oversight.

The deliberate non-fix is the load-bearing decision, and the variant experiment proves it. Uncatalogued driver codes (42P01, SQLITE_CONSTRAINT, ECONNREFUSED, ER_DUP_ENTRY) stay dropped, because ApiErrorSchema declares code a closed union (ADR-0112 D4) and an unconditional e.code = err.code would put a driver dialect into it. The variant — direction declared first — replaced the predicate with an unconditional copy and predicted exactly the five uncatalogued guards would redden, nothing else. Measured: exactly five, exactly those. That converts nine "green in both directions" cases from decoration into evidence that the predicate is necessary.

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 — deleteMetaItem's own block writes [not_overridable], SysMetadataRepository writes [NOT_OVERRIDABLE]. Only the code field is uniform. That is ADR-0112's own point restated by measurement: the catalog governs error.code; message prose is a different surface. The correction is written into the test file rather than tidied away.

The triage seat's open question is answered with a measurement: no sibling verb carries this shape. protocol.ts has exactly two re-wrap sites and both are in deleteMetaItem; saveMetaItem, publishMetaItem, revertCommit and rollbackMetaItem translate ConflictError and then re-throw the original untouched, so code already survives there. saveMetaItem's own re-wrap was deleted with its legacy raw-engine branch in #5264.

Predicted 17 red on revert, measured 17. Unmeasured: none. And the two reds landing in #6960's pin were flagged as carrying a [GUARD] label that belongs to that card's change — left unrenamed rather than relabelled to look like this card's evidence. That is the kind of precision that makes a report worth reading.

Two notes for the record

Cloud-mode artifact, disclosed and acceptable: the container's primary checkout was pre-created on the designated branch (a consequence of outcome_branch), git checkout main was refused by the permission classifier, so the dedicated worktree was created with git worktree add --force onto the same branch. All edits were made in the worktree, no git stash at any point. The parallel-agent hazard that --force normally threatens does not exist in a single-agent container. Recording it because it will recur on every cloud card and the next reviewer should not read it as a rule breach.

Open question 3, judged and left as judged: the legacy raw-engine exit keeps a literal 500 even when it now carries a code whose natural status differs. Flagged rather than taken, on the grounds that the path has no authorization gate so every failure through it really is a fault. I agree — moving it is a contract decision, not a rider.

Ruling

ACCEPT. 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

@os-zhuang
os-zhuang marked this pull request as ready for review August 10, 2026 14:47
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 10, 2026
Merged via the queue into main with commit 2c28df9 Aug 10, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7426-delete-rewrap-code branch August 10, 2026 15:03
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/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

deleteMetaItem's catch re-wrap drops the error code, so a repository refusal reaches the caller as a 403 with no catalogued code

2 participants