fix(storage): serve a tombstoned sys_file that still has a live holder

[claude]

Fixes#10246

Clause-②: yes — a currently-404 download becomes 200, so an endpoint's accept set widens. Draft, and stays draft; needs:contract-review is on the card and on this PR. This seat measures claude-opus-5, below CONTRACT_REVIEW_TIER: it will not flip ready, will not enqueue, will not arm auto-merge, and will not clear the label.

What was wrong

A sys_file tombstone (status='deleted' + deleted_at) is recoverable state, not a delete. Re-pointing a sys_attachment join row onto a tombstoned file is byte-safe already: the reap guard re-verifies references at sweep time, finds the regained holder, un-tombstones the row and vetoes the reap.

But the guard is the only thing that ever asked, and sys_file's declared lifecycle (ttl { field: 'deleted_at', expireAfter: '30d' }) nominates a tombstone only after the window expires — candidates inside the window are []. So inside the window nothing asked, while both download endpoints refused anything not committed. Net: a live attachment could point at a file that 404s for up to 30 days and then silently started working once a sweep finally ran.

What changed — the ruled direction 2, and only that

Maintainer ruling of 2026-08-23 (recorded on the card), verbatim: 「10950 不考虑存量，其他接受你的建议」 — direction 2: the download path serves a tombstoned file while it still has ≥1 live join row; the tombstone stays and the sweep still reaps when the last join row goes.

Triage's constraint, kept verbatim because rewriting a ruling is rewriting it:

复活机制仍唯一归 sweep guard,判断移到读侧,不新增生命周期动词。

Accordingly:

• No lifecycle verb was added. No un-tombstone, revive or resurrect on the read path. Nothing on the download path writes to the row. Revival remains solely the sweep guard's.
• The judgement, not the mechanism, moved. The two download handlers each carried a copy of file.status !== 'committed'. They now share one predicate, isServableForDownload, which serves committed unconditionally, refuses pending unconditionally, and asks opts.resolveFileHolder about a tombstone.
• The predicate is the guard's own, imported rather than re-derived. The plugin binds resolveFileHolder to findFileHolder — the single definition of "is anything still holding this file?", a deliberate union over sys_attachment join rows and the ref_* ownership columns, and the same function that decides whether the next sweep reaps the row. A read side that re-derived a narrower question (join rows only) would refuse files the sweep refuses to reap: the same defect, one limb over. Direction 1 was not taken, and no second reference-resolution implementation exists here.
• Bare kernels (no data engine) leave resolveFileHolder undefined and tombstones stay refused exactly as before. An unreadable holder question refuses, matching the direction the reap guard fails in.
• The 404 message moved from "File not found or not committed" to "File not found or not downloadable"; the FILE_NOT_FOUND code is unchanged. Nothing in the tree pinned the old string (checked).

Verification

All readings below were taken at 8c018711f7, which is this branch's head.

The pair, not the widening. Proving only "tombstoned + live join ⇒ 200" would prove that something got wider, not that the boundary held. Both halves are asked of one shared fixture, on both download endpoints, with the sweep's verdict on the same row asserted alongside the download's:

• tombstoned + a live holder ⇒ 200, and the sweep vetoes and reclaims nothing;
• the last holder removed ⇒ 404, and the sweep reaps — bytes deleted, row confirmed — exactly as before.

Boundaries pinned unmoved: pending still refused, bare kernel still refused, a throwing holder question refuses rather than falling open, a released (ref_* cleared) field file still refused, and the full authorization gate still applies to a served tombstone (AUTH_REQUIRED 401 / ATTACHMENT_DOWNLOAD_DENIED 403 — servability is not authorization).

Suite.pnpm --filter @objectstack/service-storage test → Test Files 26 passed (26) / Tests 435 passed (435), lock verdict command-exit 0. Dependency closure built first (pnpm --workspace-concurrency=2 --filter '@objectstack/service-storage^...' build, verdict command-exit 0).

Ablation — signature predicted in writing before the run. Prediction recorded first: delete the holder limb from isServableForDownload and exactly 3 of the 9 tests fail — the two "serves a tombstone" cases and the authorization case (which never reaches the gate once the file 404s) — while the other 6, including the boundary half of the pair, stay green. Observed: Tests 3 failed | 6 passed (9), and the three failures were those three by name. A green ablation would have meant the mutation never reached the subject; the red is what proves it did, and it also answers the rebuild question empirically — the subject is imported by relative path, so vitest resolves it from source, not dist.

The mutation was confirmed on disk (marker count 0 → 1) and the mutated predicate dumped from the file before the run. Restore ran under trap restore EXIT INT TERM and is byte-identical: git hash-object = 7270174267508d0632eca8c62073ab776e957f10, equal to the committed blob; marker count back to 0, anchor line back at 1. The restore leg re-ran green (435/435).

⚠️ One correction on the record: the first of the two on-disk confirmations used an ERE anchor containing unescaped ( and |, so it matched nothing in either direction and its "0" was void rather than informative. It was re-done with grep -F on the literal, with a reverse control showing the pattern does find the line when present. The deletion direction is evidenced by the on-disk dump, the byte-identical restore, and the predicted-and-observed red.

Gates. Derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, no hand-supplied paths (7 paths vs merge base). Run to a real verdict, exit captured before any pipe: check:engine-double-contract (OK — the new double is pinned [update] and routes through assertEngineUpdateDispatch), check:where-matcher, check:cross-package-test-inputs, check:i18n (service-storage in sync), check:route-envelope, check:test-source-alias, check:query-options-erasure, check:type-check-coverage, check:type-source-resolution, check:slot-lookup, check:entry-guard, check:parse-guard, check:published-files, check:doc-anchors, check:doc-authoring, check:role-word, check:docs-audit-scope, check:nul-bytes, check:plugin-teardown-shape, scripts/docs-audit/check-affected-docs.mjs, and the changeset gates (check-empty-changeset, check-changeset-no-major, check-adr-0087-registration, check-doc-frontmatter).

check:type-check-debt carries a margin-less ratchet for this package (@objectstack/service-storage: 51, its own note reads "THE MARGIN IS GONE"), so it was measured directly rather than assumed: tsc --noEmit --pretty false -p packages/services/service-storage/tsconfig.json reports 51 raw errors — exactly the ledgered number — with zero diagnostics in any of the three files this diff touches. The ratchet does not move.

Declared narrowing. The repo-wide pnpm lint was not run locally; CI runs the farm exactly once regardless. The narrowed run is a measurement, not a skip, and all three pieces are here: (1) the linted population is read from ESLint's own config, which reported 4 of the 7 changed paths as "File ignored because no matching configuration was supplied" — so the governed set is the 3 TypeScript files; (2) counts come from --format json: 7 entries, 3 linted, 0 errors, 0 warnings; (3) invariance — eslint.config.mjs declares no parserOptions.project and no typed @typescript-eslint rules, so this diff cannot move the verdict on any file it does not touch.

Docs this change falsifies, repaired here

docs-drift is symbol-anchor precision-first and cannot see prose or table claims, so its green is not evidence. The prose corpus was swept by hand:

• docs/qa/platform-checklist/areas/attachments-storage.json — clause 2 stated the download refusal as "status != committed". That was true when written and is not any more; a runner scoring it would have marked the fixed behaviour as a failure. It is re-keyed to pending (the limb that genuinely still refuses), a new clause pins the widened one as a pair, and the revision history records the falsification rather than silently re-wording it.
• content/docs/permissions/attachments-access.mdx — states that a tombstone is recoverable state and that re-attaching makes the file downloadable immediately, with no sweep in between, while a file with no holder left still answers FILE_NOT_FOUND (404).

Checked and found not to need repair: content/docs/references/api/contract.mdx and error-code-ledger.mdx list FILE_NOT_FOUND as a code name only, with no claim about when it fires; the download table in attachments-access.mdx enumerates only the 401/403 authorization refusals; no file under skills/ mentions this surface at all, so there is no skills/** diff and no published-skill budget question.

⚠️One falsified claim was deliberately left alone, and it needs someone with ownership: packages/spec/src/api/storage.zod.ts:55 describes the route as resolving "a committed file to a short-lived signed URL", which after this change is narrower than the behaviour. This card carries zero ownership of packages/spec, so it is reported rather than edited — flagging the conflict instead of quietly picking a side.

The five pages docs-drift named — read individually, verdict per page

docs-drift on this PR named 1 changed package, 7 documentable anchors and 5 hand-written pages. Each was read against the actual diff, asking one question: does this page state, enumerate or imply when a download 404s / when a tombstoned file is unreachable? None did, so none was edited — reasons per page, not in aggregate:

• content/docs/api/client-sdk.mdx (anchors getDownloadUrl, storage.getDownloadUrl) — no repair. The only getDownloadUrl occurrence is a bare usage line (await client.storage.getDownloadUrl('file-123')); the page's error table lists the generic RESOURCE_NOT_FOUND, never FILE_NOT_FOUND, and states no precondition on file status. Its other 404 talk is the historical never-mounted-route story, which this diff does not touch.
• content/docs/api/plugin-endpoints.mdx (anchors both routes + the sdk symbol) — no repair. The two rows describe what the routes do ("Resolve a short-lived signed download URL"; "Stable browser URL; 302s to the same signed URL"), and enumerate no refusal, no committed precondition and no tombstone behaviour. The 404s discussed elsewhere on the page are the dispatcher 的 /storage/upload 用 upload(file, {request}) 调用契约里的 upload(key, data, options?) —— 对任何实现都会 TypeError #4087 unmounted-route history.
• content/docs/deployment/cli.mdx (anchor /files/:fileId) — no repair, and worth stating why it stays true: its "Released-file collection" row says a released field file "is tombstoned into the declared 30-day grace window; re-referencing the id within the window revives it". That describes claimFile, which un-tombstones synchronously, and this PR adds no lifecycle verb and changes no revival, so the sentence is as true after as before. It makes no claim about downloads in the interim.
• content/docs/kernel/runtime-services/storage-service.mdx (anchor getDownloadUrl) — no repair. The page documents the server-side IStorageService, which is key-addressed and returns Buffers; it carries an explicit callout that getDownloadUrl(fileId) belongs to the client accessor and is "a different shape from the server-side IStorageService documented on this page". Its "Typical Errors" list is adapter-level (ENOENT on download(key)), with no sys_file.status involvement. This diff changes the HTTP route's status predicate, not the adapter.
• content/docs/protocol/objectql/types.mdx (anchor /files/:fileId) — no repair. The page's only files/ occurrence is "url": "https://cdn.example.com/files/abc123.jpg" inside a JSON example — an illustrative CDN URL, not this platform route. A substring false positive.

⛔ content/docs/releases/v15.mdx was flagged and deliberately not touched — release-owned and read-only; release notes are written centrally at release time. For the record, no factual error was found in it either: its line "tombstone on last detach (deleted_at, TTL 30d; pending uploads 7d), re-attach un-tombstones" stays accurate, because re-attach still un-tombstones and this change adds no lifecycle verb. Nothing to file.

⚠️ This list was used as a narrowed search, not as the scope. docs-drift is precision-first on symbol anchors, so its silence proves nothing about prose or table claims — the repairs above in attachments-storage.json and attachments-access.mdx came from an independent prose sweep, and neither page appears in the anchor list. A second independent sweep for sentences this change could falsify (only committed, must be committed, committed files are, FILE_NOT_FOUND enumerations, tombstone/grace-window prose across content/, docs/ and skills/) turned up nothing further beyond the packages/spec docstring flagged above.

Out-of-scope finding

Filed unassigned as #11427: record file-field hydration in engine.ts still drops any non-committedsys_file to a bare id, so it and the download path now answer the tombstone question differently. Deliberately not folded in — hydration runs over many rows per read, so asking the holder question there is an N-queries-per-read design decision, not the same one-predicate change. That issue also records something triage may want: claimFile in file-reference-lifecycle.ts already un-tombstones a field file at re-point time, so the field-file surface already implements "revive at re-point time" — the shape declined for the download path. Not a contradiction of the ruling, but a fact the direction-1/direction-2 framing did not have in front of it.

Note on this branch

The branch existed at main with nothing pushed, the previous dev's session having ended. Their work was not lost — it survived as an unpushed local commit in the leftover worktree, and it is preserved here as aa7d0ce821 (pushed first, before anything else, so a second session limit could not take it again) rather than rewritten. Every reading in this PR was taken independently against that code, not inherited from it; the merge commit brings the branch up to current main.

---

Generated by Claude Code

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

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

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/api/client-sdk.mdx(via getDownloadUrl (sdk), storage.getDownloadUrl (sdk))
• content/docs/api/plugin-endpoints.mdx(via getDownloadUrl (sdk), storage.getDownloadUrl (sdk), /files/:fileId (route), /files/:fileId/url (route))
• content/docs/deployment/cli.mdx(via /files/:fileId (route))
• content/docs/kernel/runtime-services/storage-service.mdx(via getDownloadUrl (sdk))
• content/docs/protocol/objectql/types.mdx(via /files/:fileId (route))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v15.mdx(via /files/:fileId (route))

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.

What this run could not see

• 1 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 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 — 6 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 68e8b4b53c387eb56d3c6ec4a976acf385eb977d → packageMentionDocs.

Which tree this was computed on

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

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