Skip to content

Commit 8914915

Browse files
os-zhuangclaude
andauthored
fix(metadata-protocol): report a failed sys_metadata_commit write instead of swallowing it (#9066) (#9159)
`recordPackageCommit`'s bare `catch` answered `null` for every reason with nothing logged. The publish really had succeeded — but the commit row is the only record of the turn's revert plan (`existedBefore`/`prevVersion` per artifact), so a failed insert left live artifacts, `success: true` with `commitId` merely absent, a turn that can never be reverted, and no line anywhere saying so. Discriminated by error TYPE through the shared `isMissingTableError` predicate: an unprovisioned commit store is informational and said once per protocol instance; every other failure logs at `error` per turn with the package, the operation, the item count, the driver's reason, the consequence and the fix. Publish semantics are untouched — the catch still returns `null` and no response field was added. The insert moves behind a named `persistPackageCommitRow` so the seam can be declared in `DURABILITY_CRITICAL_CALLEES`; `check:durability-log-level` now fails if this catch is ever quieted again. Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2ad91c3 commit 8914915

4 files changed

Lines changed: 547 additions & 7 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(metadata-protocol): a failed `sys_metadata_commit` write is reported instead of swallowed — the turn that cannot be reverted is now visible to an operator (#9066)
6+
7+
`recordPackageCommit` — the ADR-0067 commit writer `publishPackageDrafts` calls
8+
with the revert plan it just captured — sat behind a bare `catch` that answered
9+
`null` for every reason, with nothing logged. The comment's premise was true
10+
(the publish already succeeded and cannot be unwound) but its conclusion —
11+
"grouping is a best-effort overlay" — understated the row: `sys_metadata_commit`
12+
is the ONLY record of a turn's revert plan (`existedBefore` / `prevVersion` per
13+
artifact), the thing `revertCommit` and `rollbackToPackageCommit` act on. When
14+
the insert failed, the artifacts went live, the response read `success: true`
15+
with `commitId` merely absent, the turn could never be reverted, and no line
16+
anywhere said so — so a commit store that was failing kept failing, losing every
17+
later publish's plan the same silent way.
18+
19+
The failure is now discriminated by error TYPE, through the shared
20+
`isMissingTableError` predicate the read seams in this file already ask:
21+
22+
- an **unprovisioned** `sys_metadata_commit` (a first boot, or an environment
23+
kernel composed without the commit log) is a configuration fact, identical on
24+
every publish and fixed in one place — reported at `info`, once per protocol
25+
instance, naming the consequence and how to provision the store;
26+
- **every other** failure (connection drop, timeout, permission denial, schema
27+
drift on that table) is a durability degradation and is reported at `error`,
28+
once per turn, naming the package, the operation, the item count, the driver's
29+
own reason, that the publish itself succeeded and still reports success, and
30+
the fix.
31+
32+
Publish semantics are unchanged: the `catch` still returns `null`, the publish
33+
still succeeds, and no response field was added — whether the caller should be
34+
told the turn is unrevertible is a separate, undecided question.
35+
36+
The gate that stops this from regressing is extended in the same change: the
37+
insert now goes through a named `persistPackageCommitRow`, declared in
38+
`DURABILITY_CRITICAL_CALLEES` in
39+
`scripts/check-durability-degradation-log-level.mjs`, so a future edit that
40+
quiets this `catch` fails CI instead of shipping.

0 commit comments

Comments
 (0)