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

.changeset/storage-tombstone-download-live-holder.md

@@ -0,0 +1,18 @@
+---
+"@objectstack/service-storage": patch
+---
+
+**Fix:** a tombstoned `sys_file` that something still holds is downloadable again — no 30-day 404 in between (#10246).
+
+Re-pointing a `sys_attachment` join row onto a file inside its 30-day grace-window tombstone has always been byte-safe: the reap guard re-verifies references at sweep time, finds the new holder, un-tombstones the row and vetoes the reap. But the sweep 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 — measured candidates inside the window: `[]`. So the file simply sat at `status='deleted'` while `GET /api/v1/storage/files/:fileId` and `/files/:fileId/url` refused anything not `committed`. A live attachment could point at a file that 404s for up to 30 days and then silently starts working.
+
+**What changed:** the two download endpoints stop treating the tombstone as the last word. They now ask the reap guard's own `findFileHolder` — the single definition of "is anything still holding this file?", a union over `sys_attachment` join rows and the `ref_*` ownership columns — and serve the file for exactly as long as that answers yes.
+
+**What did not change**, deliberately:
+
+- **No lifecycle verb was added.** There is no un-tombstone, revive or resurrect on the read path; the download writes nothing to the row. Revival remains solely the sweep guard's, which is why the fix is a read-side predicate and not a second revival mechanism (the duplicate-mechanism hazard #10241 avoided). The tombstone stays, and the sweep still reaps when the last holder goes.
+- **`pending` is still refused.** Only the `deleted` limb widened; an upload that was never completed has no bytes to promise.
+- **Authorization is untouched.** A served tombstone goes through the same `authorizeFileRead` gate as any other file — `AUTH_REQUIRED` (401) and `ATTACHMENT_DOWNLOAD_DENIED` / `FILE_DOWNLOAD_DENIED` (403) are unaffected. Servability is not authorization.
+- **Bare kernels are unaffected.** With no data engine there is no holder question to ask, so tombstones stay refused exactly as before.
+
+The read side and the sweep now answer the same question from the same code, so a file the download path serves is by construction a file the next sweep would veto rather than reap — and the instant the last holder goes, both flip together. That pair is what the new tests pin; the 404 text on the refusal changed from "File not found or not committed" to "File not found or not downloadable" to match (the `FILE_NOT_FOUND` code is unchanged).

content/docs/permissions/attachments-access.mdx

@@ -109,6 +109,13 @@ can be shared across records). Reclamation is handled by the platform LifecycleS
  attachments-scope file is deleted, the file is tombstoned; a reap guard
  re-verifies zero references at sweep time and deletes the storage bytes
  before the row is reaped (abandoned `pending` uploads are reaped too).
+ **A tombstone is recoverable state, not a delete — and downloads treat it that
+ way.** Re-attaching the file, or re-claiming it through a record field, makes
+ it downloadable again *immediately*, with no sweep in between: the download
+ endpoints ask the same "is anything still holding this file?" question the
+ reap guard asks before it reclaims anything. The row itself stays tombstoned
+ until a sweep tidies it, and a file with no holder left still answers
+ `FILE_NOT_FOUND` (404).
 - **`sys_upload_session`** — abandoned/terminal chunked-upload sessions are
  reaped, and a reap guard aborts the underlying backend multipart upload
  (S3 `AbortMultipartUpload` / local parts dir) first, so already-uploaded

docs/qa/platform-checklist/areas/attachments-storage.json

@@ -433,7 +433,7 @@
  "title": "sys_file status pipeline: pending → committed → deleted (tombstone) with un-tombstone on re-attach; shared files never tombstone early",
  "since": "v15.1",
  "status": "active",
- "revision": 1,
+ "revision": 2,
  "priority": "P2",
  "surface": "api",
  "personas": ["seeded admin (admin@objectos.ai)"],
@@ -445,11 +445,13 @@
  },
  "steps": [
  "presign an attachments-scope upload and read the sys_file row: status 'pending'",
- "attempt GET /api/v1/storage/files/<fileId>/url while still pending and capture the refusal (downloads only serve committed files)",
+ "attempt GET /api/v1/storage/files/<fileId>/url while still pending and capture the refusal (a never-completed upload has no bytes to promise — this is the PENDING refusal, not a 'committed-only' rule)",
  "complete the upload; re-read: status 'committed'",
  "attach the file to 'Website Relaunch' AND 'Data Platform' (two sys_attachment join rows over ONE file — the Salesforce ContentDocumentLink share pattern)",
  "delete the 'Data Platform' join row and re-read sys_file: still 'committed' (a remaining reference blocks the tombstone)",
  "delete the LAST join row and re-read: status 'deleted' with deleted_at set (the tombstone)",
+ "while tombstoned AND holder-less, GET /api/v1/storage/files/<fileId>/url: 404 FILE_NOT_FOUND",
+ "attach a NEW join row onto the still-tombstoned file_id and immediately GET the download URL again, WITHOUT waiting for any sweep: 200 (#10246 — the download path asks the reap guard's own holder question, so a file the sweep would refuse to reap is a file the download path serves)",
  "re-attach the same file_id to 'Website Relaunch' within the grace window and re-read: status back to 'committed', deleted_at null",
  "verify a NON-attachments-scope file (e.g. an invoice-line receipt, scope from the field-upload path) is never tombstoned by these join-row hooks"
  ],
@@ -461,11 +463,17 @@
  "evidence": "the read sequence, one per transition"
  },
  {
- "clause": "a pending (never-completed) file is not downloadable: the download routes answer 404 FILE_NOT_FOUND for status != committed",
+ "clause": "a pending (never-completed) file is not downloadable: the download routes answer 404 FILE_NOT_FOUND while status is 'pending'",
  "oracle": "api",
  "verify": "the /url GET during the pending window returns 404 with that code",
  "evidence": "the 404 body"
  },
+ {
+ "clause": "the refusal is keyed to NO REMAINING HOLDER, not to the tombstone: a 'deleted' file that still has at least one live sys_attachment join row (or a live ref_* owner) downloads with 200 immediately, with no sweep in between; the tombstone row is NOT rewritten by the download (#10246)",
+ "oracle": "api",
+ "verify": "GET /url on the tombstoned file 404s while holder-less and 200s once a join row is attached, in the same grace window; a sys_file re-read after the 200 still shows status 'deleted' with deleted_at set",
+ "evidence": "the two /url responses plus the post-download sys_file read"
+ },
  {
  "clause": "one file shared by two join rows survives losing one of them — deleting an attachment deletes only the join row; the tombstone fires only when the LAST reference goes",
  "oracle": "api",
@@ -499,7 +507,8 @@
  "docs/plans/release-15.1-test-plan.md §C4 (#2755)"
  ],
  "history": [
- { "revision": 1, "date": "2026-08-07", "change": "new item: the full status pipeline as a variants matrix over the sys_file.status enum, with the shared-file and re-attach transitions from the lifecycle-hook source", "ref": "claude/platform-test-checklist-ocwugl" }
+ { "revision": 1, "date": "2026-08-07", "change": "new item: the full status pipeline as a variants matrix over the sys_file.status enum, with the shared-file and re-attach transitions from the lifecycle-hook source", "ref": "claude/platform-test-checklist-ocwugl" },
+ { "revision": 2, "date": "2026-08-23", "change": "clause 2 was FALSIFIED by #10246 and is repaired here, not merely re-worded. It stated the download refusal as 'status != committed', which was true when written and is not any more: the download routes now ask the reap guard's own findFileHolder before refusing a tombstone, so a 'deleted' file with a live sys_attachment join row (or a live ref_* owner) serves 200 inside the grace window instead of 404ing for up to 30 days and then silently starting to work once a sweep ran. A runner scoring the old clause would have marked the FIXED behaviour as a failure. The clause is now keyed to 'pending', which is the limb that genuinely still refuses; a new clause pins the widened one AS A PAIR (404 while holder-less, 200 once a holder exists, tombstone row unrewritten) because proving only the 200 proves that something got wider and not that the boundary held. Two steps added for the new clause, and step 2's parenthetical stopped asserting a committed-only rule. Note for the next author: docs-drift could not have caught this — it is symbol-anchor precision-first (#9192) and cannot see a prose claim inside a clause string.", "ref": "#10246" }
  ]
  },
  {

packages/services/service-storage/src/storage-routes.ts

@@ -6,6 +6,10 @@ import type { IHttpServer, IHttpRequest, IHttpResponse, IStorageService } from '
 import { sendOk, sendError } from '@objectstack/types';
 import type { StorageMetadataStore, FileRecord, UploadSessionRecord } from './metadata-store.js';
 import type { LocalStorageAdapter } from './local-storage-adapter.js';
+// Type only. The PREDICATE is never re-implemented in this file (#10246): it
+// arrives through `opts.resolveFileHolder`, which the plugin binds to the reap
+// guard's own `findFileHolder`.
+import type { FileHolder } from './attachment-lifecycle.js';
 import { contentDispositionValue } from './content-disposition.js';
 
 /** Authorization verdict for an attachments-scope download (#2970 item 2). */
@@ -47,6 +51,35 @@ export interface StorageRoutesOptions {
  * When absent (bare kernels, tests), all downloads stay open (back-compat).
  */
  authorizeFileRead?: (file: FileRecord, req: IHttpRequest) => Promise<FileReadVerdict>;
+ /**
+ * "Is anything still holding this file?" for a TOMBSTONED row (#10246).
+ *
+ * A `sys_file` tombstone (`status: 'deleted'` + `deleted_at`) is recoverable
+ * state, not a delete: re-pointing a `sys_attachment` join row onto it, or
+ * re-claiming it through the `ref_*` ownership columns, makes it live again
+ * — and the sweep already honours that, un-tombstoning and vetoing the reap
+ * instead of reclaiming the bytes. But the sweep is the only thing that ever
+ * asks, and it asks only AFTER the declared 30d TTL expires, so inside the
+ * grace window a live attachment pointed at a tombstone downloaded as 404
+ * for up to 30 days and then silently started working.
+ *
+ * ⛔ This does NOT add a second revival mechanism. Revival stays solely the
+ * sweep guard's; nothing on the read path writes to the row. What moves here
+ * is the JUDGEMENT — the download path stops treating the tombstone as the
+ * last word and asks the same question the guard asks.
+ *
+ * ⚠️ Wire this to `findFileHolder` (`attachment-lifecycle.ts`) and to
+ * nothing else. That function is the ONE definition of "still held", a
+ * deliberate union of the two surfaces that can hold a `sys_file` —
+ * `sys_attachment` join rows AND the `ref_*` ownership columns — and it is
+ * what decides whether the next sweep reaps this row. A read side that
+ * re-derived a narrower question (join rows only, say) would refuse files
+ * the sweep refuses to reap: the same defect, one limb over.
+ *
+ * Absent (bare kernels, no data engine, tests that don't wire it): tombstones
+ * stay refused, exactly as before this option existed.
+ */
+ resolveFileHolder?: (file: FileRecord) => Promise<FileHolder>;
  /**
  * TTL (seconds) for the signed URL minted on a GATED attachments download.
  * Short by design — the link is followed immediately after an explicit
@@ -133,6 +166,42 @@ export function registerStorageRoutes(
  return downloadTtl;
  };
 
+ // ── Download servability (#10246) ────────────────────────────────────
+ // Written ONCE and called by both download endpoints. They used to carry a
+ // copy each of `file.status !== 'committed'`, which is how a rule that needs
+ // to widen turns into two rules that drift; `/files/:fileId/url` and
+ // `/files/:fileId` are the same decision reached through two doors.
+ //
+ // - `committed` → servable, unconditionally and unchanged.
+ // - `pending` → refused, unconditionally and unchanged: an upload that
+ // was never completed has no bytes to promise.
+ // - `deleted` → servable for exactly as long as something still holds
+ // it. The tombstone is NOT the last word; it is a claim
+ // about the future (this row is reapable when the grace
+ // window ends) that the sweep re-checks and often
+ // withdraws. Asking the guard's own question here makes
+ // the two agree by construction: a file this returns
+ // `true` for is a file the next sweep would un-tombstone
+ // rather than reap, and the moment the last holder goes it
+ // returns `false` again — same instant the sweep starts
+ // reaping it.
+ //
+ // The row is never written to. Revival remains the sweep guard's alone
+ // (triage's ruling on this card: 复活机制仍唯一归 sweep guard,判断移到读侧,
+ // 不新增生命周期动词).
+ const isServableForDownload = async (file: FileRecord): Promise<boolean> => {
+ if (file.status === 'committed') return true;
+ if (file.status !== 'deleted' || !opts.resolveFileHolder) return false;
+ try {
+ return (await opts.resolveFileHolder(file)) !== null;
+ } catch {
+ // Unreadable evidence is not evidence of a holder. Refuse — the same
+ // answer this route gave before #10246, and the same direction the reap
+ // guard fails in (it vetoes rather than reaps when it cannot tell).
+ return false;
+ }
+ };
+
  // ── Upload auth gate (#2755) ─────────────────────────────────────────
  // `false` ⇒ the 401 was already sent and the handler must stop.
  // `null` ⇒ open mode (no resolver wired) — proceed unauthenticated.
@@ -603,8 +672,8 @@ export function registerStorageRoutes(
  try {
  const { fileId } = req.params;
  const file = await store.getFile(fileId);
- if (!file || file.status !== 'committed') {
- sendError(res, 404, 'FILE_NOT_FOUND', 'File not found or not committed');
+ if (!file || !(await isServableForDownload(file))) {
+ sendError(res, 404, 'FILE_NOT_FOUND', 'File not found or not downloadable');
  return;
  }
 
@@ -650,8 +719,8 @@ export function registerStorageRoutes(
  try {
  const { fileId } = req.params;
  const file = await store.getFile(fileId);
- if (!file || file.status !== 'committed') {
- sendError(res, 404, 'FILE_NOT_FOUND', 'File not found or not committed');
+ if (!file || !(await isServableForDownload(file))) {
+ sendError(res, 404, 'FILE_NOT_FOUND', 'File not found or not downloadable');
  return;
  }
 

packages/services/service-storage/src/storage-service-plugin.ts

@@ -21,7 +21,7 @@ import { StorageMetadataStore } from './metadata-store.js';
 import type { FileRecord } from './metadata-store.js';
 import { registerStorageRoutes } from './storage-routes.js';
 import type { FileReadVerdict } from './storage-routes.js';
-import { installAttachmentLifecycleHooks, createSysFileReapGuard, createUploadSessionReapGuard } from './attachment-lifecycle.js';
+import { installAttachmentLifecycleHooks, createSysFileReapGuard, createUploadSessionReapGuard, findFileHolder } from './attachment-lifecycle.js';
 import { installFileReferenceHooks } from './file-reference-lifecycle.js';
 import { installAttachmentAccessHooks, installAttachmentReadVisibility } from './attachment-access-hooks.js';
 import { SystemFile, SystemUploadSession } from './objects/index.js';
@@ -413,6 +413,16 @@ export class StorageServicePlugin implements Plugin {
  sessionTtl: this.options.sessionTtl,
  resolveSession: buildAuthSessionResolver(ctx),
  authorizeFileRead: buildFileReadAuthorizer(ctx, engine),
+ // "Is anything still holding this tombstone?" on the READ side
+ // (#10246) — the reap guard's own `findFileHolder`, handed over
+ // rather than re-derived. One definition of "still held", asked by
+ // the sweep before it reaps and by the download path before it
+ // refuses, so the two cannot answer differently. No engine (bare
+ // kernel) leaves it undefined and tombstones stay refused.
+ resolveFileHolder:
+ engine && typeof (engine as any).find === 'function'
+ ? (file: FileRecord) => findFileHolder(engine as any, file.id, file as any)
+ : undefined,
  logger: ctx.logger,
  });