Repository navigation
Description
Found and measured while implementing #10171 (PR #10241). Deliberately not fixed there, and filed unassigned rather than ridden along: the fix would be Clause-②: yes (it turns a currently-404 download into a 200), and the services lane has no contract-review tier available right now. The #10171 dev stopped and reported it rather than implementing or filing unilaterally; this card is the PM recording it.
The shape
Re-pointing a sys_attachment row onto a file that is inside its 30-day grace-window tombstone is byte-safe — that half is already owned by createSysFileReapGuard, whose sweep-time re-verification resolves current references, un-tombstones the file and vetoes the reap. #10241 measured exactly that and therefore added no revival leg, correctly.
But the guard only ever sees rows the sweep nominates, and the sweep nominates a tombstone only after its ttl { field: 'deleted_at', expireAfter: '30d' } has expired. Measured: candidates within the window = [].
So inside the window the file is not a sweep candidate at all, and simply stays status='deleted'. Meanwhile storage-routes.ts:606 and :653 refuse any file whose status !== 'committed' with 404 FILE_NOT_FOUND.
Net: a live attachment row can point at a file that 404s for up to 30 days, and then silently starts working once the sweep finally runs and un-tombstones it. Nothing is lost, and nothing tells the operator why.
Why this is worth a card rather than a comment
The revival mechanism is correct and the bytes are safe — what is wrong is when it runs. "Eventually correct, on a 30-day timer, with a 404 in between" is a defensible retention posture and an indefensible download posture, and today the system has no way to tell those two apart because one timer serves both.
file_id mints them too. The scenario is pre-existing, but the set of ways to reach it just grew — which is why it is being recorded now rather than left as a curiosity.
Why the fix is Clause-② and needs the contract-review tier
Any fix makes a request that is refused today succeed — an endpoint's accept set widens. That is categorically different from #10241's own change, which applied an already-declared rule (an attachments-scope file with zero join rows is tombstoned, enforced on the delete verb since #2755) to the verb that was missing it, minting no new refusal category and no new accepted input.
⛔ So this must not be folded into a lower-tier card.
Directions (not a recommendation — this needs triage/maintainer grading)
- Revive at re-point time, not at sweep time: when an update points a row at a tombstoned file, un-tombstone it immediately. Closes the window entirely; costs a second implementation of the reference-resolution question the guard already answers — the duplicate-mechanism hazard fix(service-storage): tombstone the prior file when an attachment re-points file_id #10241 deliberately avoided.
- Let the download path resolve it:
storage-routes.tstreats a tombstoned file with at least one live join row as downloadable. Keeps one revival mechanism, moves the judgement to the read side; the tombstone stays until the sweep tidies it. - Accept and document: state that a re-attached file may 404 until the next sweep. Cheapest, and honest only if someone actually wants a 30-day 404 in a live attachment.
Refs: #10171 (the update-verb detach leg), PR #10241, #2755 (the orphan rule), createSysFileReapGuard, storage-routes.ts:606 / :653, system-file.object.ts (the ttl / retention lifecycle declaration).
domain:services assigned by the discovering lane; retriage if it belongs elsewhere.
Activity
os-zhuang commentedon Aug 23, 2026
pm:retriagere-judgment (triage seat, sessionsession_01XQuU19PeYR7zWiEZX9LJWm): objection upheld — re-gradedpm:queue→needs-user-decision,pm:retriagecleared in the same write.Grounds, verified against the card's own text: (1) every fix direction widens an endpoint's accept set — the card's own Clause-② self-grading; (2) the direction is explicitly un-chosen ("not a recommendation — this needs triage/maintainer grading"), and direction 3 is a product posture, not an implementation. Choosing between "close the window" and "declare the window acceptable" is a decision about what the product promises — manual floor (public-contract change). The services seat's note stands: once a direction is ruled, implementation is an ordinary
Clause-②: yesdispatch.Decision facets (per the standard block; 分析用中文):
[facets-block]
推荐:方向 2(下载路径对「有 ≥1 条活 join 行的墓碑文件」放行,墓碑仍由 sweep 收尾)。回「2」即可;裁后实施卡为
Clause-②: yes、走契约复审档位。Generated by Claude Code
claude commentedon Aug 23, 2026
Maintainer ruling recorded (atomic four-part write)
Ruled 2026-08-23, live PM chat, verbatim: 「10950 不考虑存量,其他接受你的建议」 — this card falls under the accepted recommendations. Ruled: 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.
needs-user-decision→pm:queuein the same stroke (domain:services, Bug). Implementation isClause-②: yes(a currently-404 download becomes 200) — contract-review tier at dispatch.Generated by Claude Code
claude commentedon Aug 23, 2026
os-dev-report
{ "issue": 10246, "status": "done", "branch": "claude/issue-10246-tombstone-download-live-join", "pr": "https://github.com/objectstack-ai/objectstack/pull/11428", "premise_still_valid": true, "summary": "Ruled direction 2, implemented on the read side only. 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 plugin binds that to the reap guard's own findFileHolder, imported and never re-derived, so the download verdict and the sweep verdict are one question answered by one function. No lifecycle verb was added and nothing on the read path writes to the row: revival stays solely the sweep guard's. ZONE 2 RESULTS. (1) CONFIRMED, with the reverse control the dispatch required: the refusal is exactly two sites, both in storage-routes.ts, now one predicate. storage-route-ledger.ts independently enumerates exactly two family:'download' routes; the third FILE_NOT_FOUND in the file is POST /upload/complete, not a download. The reverse control (every non-service-storage reader of sys_file across packages/apps/examples) shows the search reaches objectql, spec, cli, client, plugins and qa, so the zero-hit is a measurement, not a blind spot. (2) CONFIRMED: findFileHolder is the guard's single definition and already had a second caller (the #10950 inventory). I additionally verified the read side can answer its FULL union rather than a narrower question, which is the hazard the card names: FileRecord declares ref_object/ref_id/ref_field and store.getFile returns the whole engine row, so the ref_* limb is genuinely answerable on the read side. (3) CONFIRMED: the reap path is untouched, and because both sides call the same function, a file the download serves is by construction one the next sweep vetoes. DOCS-DRIFT ADDENDUM, per page as requested. All five named pages were read against the diff; none states, enumerates or implies when a download 404s, so none was edited. content/docs/api/client-sdk.mdx: only a bare usage line for getDownloadUrl; its error table lists generic RESOURCE_NOT_FOUND, never FILE_NOT_FOUND, and states no file-status precondition. content/docs/api/plugin-endpoints.mdx: 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 or committed precondition; its other 404 talk is the #4087 unmounted-route history. content/docs/deployment/cli.mdx: its 'Released-file collection' row says a released field file is tombstoned into the 30-day window and '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 it stays true; it makes no claim about downloads in the interim. content/docs/kernel/runtime-services/storage-service.mdx: documents the SERVER-SIDE key-addressed IStorageService and carries an explicit callout that getDownloadUrl(fileId) is the client accessor and a different shape; its Typical Errors list is adapter-level ENOENT on download(key), with no sys_file.status involvement. content/docs/protocol/objectql/types.mdx: its only files/ occurrence is \"url\": \"https://cdn.example.com/files/abc123.jpg\" in a JSON example — a substring false positive, not this route. content/docs/releases/v15.mdx was flagged and deliberately NOT touched (release-owned/read-only). No factual error found in it either: 'tombstone on last detach (deleted_at, TTL 30d; pending uploads 7d), re-attach un-tombstones' stays accurate, because re-attach still un-tombstones and no lifecycle verb was added. Nothing to file. The anchor list was used as a narrowed search, NOT as the scope. Both prose repairs this PR actually makes — docs/qa/platform-checklist/areas/attachments-storage.json clause 2, which stated the refusal as 'status != committed' and would have made a runner score the FIXED behaviour as a failure, and content/docs/permissions/attachments-access.mdx — came from an independent sweep, and NEITHER page appears in the anchor list. That is the #11356 blind spot showing up concretely on this card. DISPATCH PREMISE CORRECTION: the previous dev's work was NOT lost. It survived as an unpushed local commit in the leftover worktree /home/user/objectstack-10246. I pushed it first, before any other action, and preserved it as aa7d0ce821 rather than rewriting it; every reading reported here was taken independently against that code, not inherited from its claims. NOT REPAIRED, needs an owner: packages/spec/src/api/storage.zod.ts:55 still describes the route as resolving 'a committed file', which this change falsifies. This card carries zero ownership of packages/spec, so the conflict is reported rather than quietly resolved — see open_questions. PR HOUSEKEEPING: PR #11428 is draft with needs:contract-review; not flipped, not enqueued, no auto-merge, label not cleared. Labels documentation/tests/tooling/size/m were added by repo bots, not by me, and are left alone. On the body PATCH the platform APPENDED a second, bare-form signature footer after my session-URL one; my footer is intact and I did not start an edit loop over it.", "tests": "All readings taken at 8c018711f7, this branch's head; tree clean, nothing landed after the gate runs. BUILD FIRST: pnpm --workspace-concurrency=2 --filter '@objectstack/service-storage^...' build via scripts/pm/os-verify-lock.sh -> 'os-verify-lock: VERDICT command-exit 0 - held the lock 280s - waited 417s'. SUITE: pnpm --filter @objectstack/service-storage test -- --maxWorkers=2 -> 'Test Files 26 passed (26)' / 'Tests 435 passed (435)', 'VERDICT command-exit 0'. THE PAIR (both download endpoints, one shared fixture, the sweep's verdict asserted beside the download's): tombstoned + live join row -> 200 while the sweep VETOES, un-tombstones and reclaims nothing; last holder removed -> 404 FILE_NOT_FOUND while the sweep CONFIRMS the reap and deletes the bytes. Boundaries pinned unmoved: pending refused, bare kernel refused, a throwing holder question refuses rather than falling open, a released ref_* field file refused, and the authorization gate still yields 401 AUTH_REQUIRED / 403 ATTACHMENT_DOWNLOAD_DENIED on a served tombstone. ABLATION, signature predicted IN WRITING BEFORE the run (scratchpad ablation-prediction.md): delete the holder limb from isServableForDownload and EXACTLY 3 of 9 fail — the two 'serves a tombstone' cases plus 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 EXACTLY THAT: 'Tests 3 failed | 6 passed (9)', the three failures matching by name. REBUILD: none needed, and this is measured rather than assumed — the subject is imported by relative path ('./storage-routes.js'), so vitest resolves it from source, not dist, and the RED is itself the proof the mutation reached the subject. A GREEN would have been treated as NOT MEASURED and investigated, never as a pass. MUTATION CONFIRMED ON DISK: literal marker count 0 -> 1, plus the mutated predicate dumped out of the file before the run. RESTORE ran under trap restore EXIT INT TERM and is byte-identical: git hash-object 7270174267508d0632eca8c62073ab776e957f10 == the committed blob; marker back to 0, anchor line back to 1. Restore leg re-ran green at 435/435. HONEST CORRECTION: the FIRST on-disk check used an ERE anchor containing unescaped ( and |, so it matched nothing in EITHER direction and its '0' was void rather than informative. Re-done with grep -F on the literal, plus a reverse control showing the pattern does find the line when present. The deletion direction rests on the on-disk dump, the byte-identical restore, and the predicted-and-observed red. GATES: derived by 'node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack' with NO hand-supplied paths (7 paths vs merge base cbf8b2c8a). Exit captured before any pipe in every case; quoting each gate's own verdict line: check:engine-double-contract 'OK - 389 pinned, 133 in the DEBT ledger, 2 exempt' with the new double listed 'pinned [update]'; check:where-matcher '287 matcher(s) discovered ... none new'; check:cross-package-test-inputs 'OK: 14 package(s) read outside themselves, all declared'; check:i18n 'OK (9 package(s) - all bundles in sync)' with service-storage 'in sync (4 bundle(s))'; check:nul-bytes 'OK (scanned 6412 text file(s))'. Also exit 0: 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:plugin-teardown-shape, scripts/docs-audit/check-affected-docs.mjs, and the changeset gates (empty-changeset, changeset-no-major, adr-0087-registration, doc-frontmatter). TYPE-CHECK RATCHET measured rather than assumed, because this package is margin-less ('@objectstack/service-storage: 51', whose own note reads THE MARGIN IS GONE): 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: repo-wide pnpm lint was not run locally; CI runs the farm exactly once regardless. The narrowed run carries all three required evidences — (1) population 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 from --format json: 7 entries, 3 linted, 0 errors and 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. NOT MEASURED: nothing. No gate reported PREREQUISITE NOT MET and no lock call exited 99. CI convergence is left to the PM per the dispatch contract.", "open_questions": [ { "question": "packages/spec/src/api/storage.zod.ts:55 documents GET /api/v1/storage/files/:fileId/url as resolving 'a committed file' to a short-lived signed URL. This PR falsifies that wording, since the route now also serves a tombstoned file that still has a holder. The dispatch requires reference docs this PR falsifies to be repaired in-PR AND gives this card zero ownership of packages/spec. I did not edit it; I am reporting the conflict rather than silently picking a side.", "options": [ "A - leave it to a spec-owning card; this PR reports it and the PR body flags it", "B - grant this card a one-line docstring exemption in packages/spec and repair it here", "C - fold it into the contract-review pass, since that reviewer is already reading this exact surface" ], "recommendation": "A, with C as a free upgrade. The zero-ownership instruction is categorical, and the claim is a route docstring rather than a 404 enumeration a reader would act on; a one-line spec edit would also pull packages/spec's gate families onto a card that deliberately has none. C costs nothing extra because the contract reviewer is already in this surface." } ], "out_of_scope_findings": [ "filed as #11427: record file-field hydration in objectql engine.ts still drops any non-committed sys_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. The 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 the 'revive at re-point time' shape that was 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." ] }Generated by Claude Code
claude commentedon Aug 23, 2026
Contract review — PASS (fable seat, session
5213b871-5164-5bc3-8874-28b336bbcd40, hourly sub-round; fuse readingget_session→external_metadata.last_served_model=claude-fable-5, matchingCONTRACT_REVIEW_TIERread from origin/main; authorization: maintainer 2026-08-23 「要不还是你挂个定时处理审核吧」). Independence: dispatched by the services seat — independent review. Ruling verified on-card: direction 2, 2026-08-23 「10950 不考虑存量,其他接受你的建议」.Reviewed PR #11428 @
8c018711f7against the actual diff.sys_filewith ≥1 live holder serves 200; the triage constraint kept verbatim — no lifecycle verb, nothing on the read path writes, revival stays solely the sweep guard's.status !== 'committed'checks replaced by one sharedisServableForDownload, whose holder question isopts.resolveFileHolderbound to the reap guard'sfindFileHolder(join rows ∪ref_*owners) — the read side cannot diverge from what the sweep refuses to reap. Direction 1 correctly not taken.status === 'deleted'WITH a bound resolver even asks; bare kernels and throwing holder questions refuse;pendingrefuses unconditionally;FILE_NOT_FOUNDcode unchanged (old message string pinned nowhere — checked).Verdict: PASS. Clearing
needs:contract-reviewon both carriers. Enqueue/flip belongs to the dispatching seat's landing window.Generated by Claude Code
fix(objectql): hydrate a tombstoned sys_file that still has a live ho…
docs(service-storage): re-anchor the dead tracker citations to the co…