fix(metadata-protocol): audit the allowed publish/rollback and the 409 conflict denial (#7748)

[os-zhuang]

Fixes#7748

The defect is an inversion

protocol.ts wrote an allowed-outcome audit row at exactly two sites — the save path and the delete path. publishMetaItem and rollbackMetaItem reached recordMetadataAudit through one route only, assertLockAllowsWrite, which records on the deny path and returns before any write on allow:

const refusal = evaluateLockForWrite(state.lock);
if (!refusal) return null; // allowed: returns before any audit write
…
await this.recordMetadataAudit({ …, outcome: 'denied', … });

So a refused publish was audited and a successful one was not — the inverse of what an audit trail is for. The 409 METADATA_CONFLICT refusal is raised outside that helper (it comes back from the repository's parent-version check), so it wrote nothing either: a caller repeatedly losing an optimistic-concurrency race was indistinguishable, in the trail, from a caller who never tried.

The QA run this came from: 3 publishes + 2 rollbacks, all 200, all with X-Actor, produced Counter({'save': 5}).

What changed

All three gaps the card names, in packages/metadata-protocol/src/protocol.ts:

route	row now written
publishMetaItem	operation: 'publish', outcome: 'allowed', code: 'ok'
rollbackMetaItem	operation: 'rollback', outcome: 'allowed', note names the restored version
409 METADATA_CONFLICT — all four sites (save / publish / rollback / delete)	outcome: 'denied', code: 'metadata_conflict'

The two allowed rows are placed exactly where the two pre-existing ones sit: after persistence has committed, before the projector. The four conflict sites route through one new private helper, recordOptimisticConflictAudit, rather than four hand-written copies — those sites already carry four near-identical hand-written conflict Errors, and this is the lane that has paid twice this shift for one idea acquiring several spellings.

sys_metadata_audit has always declared publish and rollback as operation options, so no schema work was needed — the table was designed for these rows and the writes were simply never made.

Anti-vacuity

Every other multi-table fake engine in this repo opens insert with if (table === 'sys_metadata_audit') return { id: 'audit_skip' };. That is correct for a suite about something else and makes every assertion here vacuous — it reports "no audit rows" identically before and after the fix. The new harness persists audit rows like any other table, and a save-row control proves it can see them at all.

It also separates attempted from landed. recordMetadataAudit is best-effort by contract (ADR-0010 §3.6): it swallows its own insert failure. That makes a missing row ambiguous between "nothing ever tried to write it" (this defect) and "the write was attempted and failed" (a provisioning problem). The last case drives the second explicitly — audit table rejecting, publish still succeeds, attempt observed, row absent.

Reverse verification

Direction predicted before running; the prediction was wrong and the correction is recorded in the test header rather than smoothed over. Predicted 5 red / 2 green; measured 6 red / 1 green. The best-effort-swallow case also goes red, correctly: under the defect no publish audit write is attempted, so there is nothing for the failing table to reject. Only the save control stayed green — the asymmetry that matters, proving the fix did not disturb the site that already worked.

Taken by reverting protocol.ts to origin/main from the commit, then restoring with git checkout from the branch; git status clean afterwards, so the numbers were taken against the bytes that ship.

Verification

• metadata-protocol — 80 files / 1170 tests green (re-run after merging main)
• Downstream consumer sweep (prefix filter ...@objectstack/metadata-protocol, i.e. consumers): objectql 196/3496, rest 110/1817, plus the two other suites whose fake engines drive publish/rollback without an audit branch (specapi/protocol.test.ts 51, plugin-email template-runtime-write 14). These were checked deliberately: their stubs lack a sys_metadata_audit branch, so a new insert could have collided in their row maps.
• All 45 check:* gates parsed out of the ESLint job, plus check:i18n, check:type-check-coverage, check:agent-model-declared — 48/48 green. check:i18n first reported a prerequisite miss (workspace CLI not built) and was re-run green after building it.
• Re-derived against the actual changed paths: this surfaced check:changeset-gate-self-tests, check:objectui-changeset, check:query-options-erasure and scripts/check-changeset-no-major.mjs, which the dispatch list did not name — all green.

Scope

Batch publishPackageDrafts is deliberately untouched and filed separately as #8400 — it promotes drafts inside one engine.transaction(), where an audit row would roll back with the batch rather than record the refusal that caused it. That needs a Phase 2 placement and its own test, not a rider here. The actor stamped on the rows that are written stays out — that is #7749, which remains open.

---

Generated by Claude Code

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 13, 2026 11:05am

[github-actions]

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

[os-zhuang]

ACCEPT — PM review, domain:metadata seat

Green at 63308c43: all 25 checks, each job's own conclusion verified individually. Path-fork check clean — 3 files (.changeset/lifecycle-audit-rows.md, protocol.lifecycle-audit-rows.test.ts, protocol.ts), none under docs/adr/**, .claude/skills/** or skills/**.

The three bars this card was dispatched under, all cleared

1. ⚠️ The ExtendedOperation trap was avoided structurally, not just verbally. The risk was closing this on sys_metadata_history's 'publish' — a different table. The harness carries historyRows and auditRows as separate stores and every assertion reads the audit one. The card is closed on sys_metadata_audit, which has always declared publish and rollback as operation options: the table was designed for these rows and the writes were simply never made.
2. The vacuity trap is named and closed. Every other multi-table stub in this repo opens insert with if (table === 'sys_metadata_audit') return { id: 'audit_skip' } — correct for a suite about something else, and fatal here, since it reports "no audit rows" identically before and after the fix. This harness persists them, and the save control exists precisely to prove the number 0 means something.
3. A bearer-authenticated admin metadata write is stamped actor: 'system' — req.user / req.userId are unset on the /meta PUT path #7749 stayed out, explicitly.

What lifts this above the bar

The reverse verification was wrong and said so. Predicted save control green, swallow case green, five assertions red. Measured 6 red / 1 green. Rather than retro-fitting the prediction, the header records the mismatch and explains it: the swallow case's load-bearing assertion is auditAttempts.some(a => a.operation === 'publish'), and under the defect no publish audit write is ever attempted, so the failing table has nothing to reject. That is the correct direction, and only the save control stayed green — the asymmetry that proves the fix did not disturb the one site that already worked. This is what predict-before-running is for, and it is worth more than a prediction that happened to be right.

It found a third state I did not ask for.recordMetadataAudit is best-effort by contract (ADR-0010 §3.6) — it swallows its own insert failure — so a missing row is ambiguous between nothing ever tried (this defect) and the write was attempted and rejected (a provisioning problem). A test that only counts rows passes forever on the second. Recording auditAttempts before any simulated failure keeps them distinguishable, and the last case drives the second explicitly.

One idea, one spelling. All four 409 routes go through recordOptimisticConflictAudit rather than four hand-written blocks beside the four near-identical hand-written conflict Errors already there. code: 'metadata_conflict' is the persisted column's own lower-case vocabulary with an adr0112-ok: D6b marker, and the test pins that it is deliberately not the wire code. The dispatch assertions import from @objectstack/metadata-core, never objectql — with the dependency cycle named in a comment, which is the lesson #8334 paid for earlier in this shift.

The scope boundary is a decision, not an omission. The publish row sits outsidepromoteDraftForPublish, because publishPackageDrafts calls that inside one engine.transaction() where an audit row would roll back with the batch — including the refusal that caused the rollback, which is the exact defect this card exists to close. That reasoning is in the docblock, the changeset and #8400, rather than the batch path being quietly swept in.

Residues:#8400 (bug / pm:queue, this lane — the batch path needs a Phase 2 placement off promoted[] and its own test) and #7749 (the actor stamped on these rows — remains open, deliberately untouched).

Marking ready and enqueueing. Fixes #7748 closes the card on merge.

---

Generated by Claude Code