fix(service-storage): tombstone the prior file when an attachment re-points file_id

[os-warren]

Fixes#10171

An UPDATE that changes a sys_attachment row's file_id detaches the prior file exactly as deleting that row would, but installAttachmentLifecycleHooks registered no update-side handler. The old sys_file was left at status='committed' with zero join rows and no deleted_at.

That is outside the module's stated "fail toward retention, not data loss" bias, which buys a later look. sys_file's declared lifecycle nominates a sweep candidate only through ttl { field: 'deleted_at' } or retention { onlyWhen: { status: 'pending' } }, and a silently detached file matches neither — so it never becomes a candidate, the reap guard is never asked about it, and the bytes are stranded permanently rather than reclaimed late.

The card's first question, answered by measurement

#10171 asked for two legs and flagged the second as "worth verifying against the reap guard's sweep-time re-verification — if that re-check resolves current references, only leg 1 stands." It does, so only leg 1 stands.

Re-pointing a join row onto a grace-window tombstone and then running the real createSysFileReapGuard over that row:

guard confirmed: [] (vetoed)
storage.delete calls: 0 (no bytes reclaimed)
f_t after sweep: status "committed", deleted_at null (un-tombstoned by the guard)

The guard resolves current references immediately before reclaiming, so the bytes were never at risk. No revival leg is added here — a second revival mechanism would be a duplicate answer to a question that already has one, and the failure mode of that is two implementations that drift apart, not a missing feature. The behaviour is pinned by the pre-existing case "vetoes and un-tombstones a row that regained references (sweep-time re-verification)" and now by a case driving it through the re-point path specifically.

Why ctx.previous, and not the beforeUpdate stash the issue suggested

#10171 proposed beforeUpdate/afterUpdate twins of the delete pair, stashing prior ids on the hook context. Measured on the wired engine, that shape would have been silently half-dead:

stash set in beforeUpdate, read in afterUpdate
dispatch.mode === 'record' (by-id): SET
dispatch.mode === 'per-row' (predicate): LOST

Since #5574 (ADR-0058 Addendum II D1/D2) a predicate write dispatches one context per matched row, and those row contexts are fresh objects built independently per phase (dispatchPerRowBeforeHooks / buildPerRowAfterContexts), so a stash dies with the row that held it — exactly the multi-row updates that orphan the most files.

The handler instead reads the engine-bound pre-image ctx.previous.file_id, which is bound on both phases and both dispatch paths, and adds no driver round trip: the prior-row read is memoized per operation and already demanded on this object, because attachment-access-hooks.ts registers a beforeUpdate here and the engine asks that demand per object.

The handler fires only when the payload actually carries file_id and the value actually changes; with no pre-image it tombstones nothing, keeping the file. The orphan rule itself is now one function shared by afterDelete and afterUpdate rather than two copies.

Verification

Ablation — predicted signature stated before running: neutering only the afterUpdate body should redden exactly the 6 detach-positive cases while all 8 negative cases and, decisively, the wired delete-leg case stay green (a red delete case would mean the shared helper was hit instead of the update leg).

Tests 6 failed | 33 passed (39)
× tombstones the prior file when the re-pointed row was its LAST reference
× tombstones on the PREDICATE path too — where a beforeUpdate stash would have been lost
× a failing lookup never blocks the update (best-effort)
× an untombstoned orphan is never a sweep candidate — the leak is permanent, not deferred
× a by-id re-point tombstones the prior file (`dispatch.mode === "record"`)
× a PREDICATE re-point tombstones the prior file (`dispatch.mode === "per-row"`)

Matched exactly; the delete-leg case stayed green. Restored byte-identically — git hash-object8ecd3650e4dd572d7c23d6d690bb7c04e5d0e560 before and after, with the mutation hashing 24a4903c… in between.

Rebuild, argued from the files rather than asserted. The mutated module is imported by the test as from './attachment-lifecycle.js' — a relative specifier, which never resolves through package exports, so vitest loads src/ directly and no dist/ of this package participates in either leg; ablation-dist-preflight.mjs does not apply to it. The wired cases' engine, @objectstack/objectql, is loaded from dist/ — it is a ledger-confirmed unaliased import for this package (KNOWN_UNALIASED_TEST_IMPORTS, scripts/check-test-source-alias.mjs) — and it was built at the start of this task and never mutated in either leg, so both legs ran against the same current dist. That is the half that could have produced a false-green ablation, and it did not: both wired cases reddened.

Tests.pnpm --filter @objectstack/service-storage test → Test Files 24 passed (24), Tests 397 passed (397). New cases cover both dispatch paths on the fake and, because a fake asserting its own model of previous would be circular, four cases driving real ObjectQL.

One pre-existing pin moved: the sys_attachment hook inventory in storage-service-plugin.test.ts gained afterUpdate. That is an inventory that genuinely grew, not a pin on a removed branch.

Gate union — node scripts/pm/dispatch-gates.mjs (no paths passed) re-derived after the final commit, run on a clean worktree at aa76ba2f7, exit codes captured before any pipe, each quoted from the gate's own verdict line. All 17 families green; the notable ones:

• check:engine-double-contract — OK — 331 pinned, 133 in the DEBT ledger, 2 exempt. (no new pin, no baseline raised)
• check:where-matcher — ✓ where-matcher conformance holds: 265 matcher(s) discovered, 265 answer the combinator battery correctly or refuse it loudly (160 refuse).
• check:type-check-debt — check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 298.4s, 1924 raw tsc error(s) total, none above its recorded number.
• check:type-check-coverage — OK — 64/77 workspace packages type-checked (plus the root), 13 in the DEBT ledger
• check:test-source-alias — OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through dist/.
• check:nul-bytes — OK (scanned 6076 text file(s) ... no raw ASCII control bytes).
• check:i18n — OK (9 package(s) — all bundles in sync, no undeclared authoring keys).
• check:slot-lookup, check:query-options-erasure — both ratchet holds ... none new
• check:type-source-resolution, check:changeset-gate-self-tests, check:objectui-changeset, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-affected-docs — all green

No ledger entry was raised in check-type-check-coverage.mjs or anywhere else.

Out of scope

Filed, not fixed: #10240 — the same leak through the delete verb. beforeDelete→afterDelete hands ids over on the context stash, which the measurement above shows is lost on the predicate path, so a multi: true delete of the last join row writes no tombstone either. Left alone deliberately: it is a different verb with a larger blast radius, and folding it in here would make this PR unreviewable as a single change. The divergence is named in a code comment rather than left implicit, so the two seams in this file are not silently disagreeing.

#10091 is not addressed here and #10170 remains open — neither file was touched.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-storage, touching 7 documentable anchor(s).

23 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 923c42470344b29a86f4bae37850e4305292c3ee.

⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see

• the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 5 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 923c42470344b29a86f4bae37850e4305292c3ee → packageMentionDocs.

Which tree this was computed on

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

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