fix(objectql): hydrate a tombstoned sys_file that still has a live holder

[os-sam]

Fixes#11427

Clause-②: yes — see Contract-review assessment below. Draft on purpose; the dispatching seat lands it.

Premise check first (dispatch constraint ①)

This card is hard-serial behind #10246 and re-parks if that predicate is not merged. Measured on origin/main before any edit:

• A live attachment pointing at a grace-window tombstone 404s for up to 30 days, then silently starts working — the reap guard only revives files it is asked about, and it is never asked inside the window #10246 is closed as completed by merged PR fix(storage): serve a tombstoned sys_file that still has a live holder #11428, commit a41069ba37, an ancestor of origin/main.
• The read-side predicate is live: isServableForDownload at packages/services/service-storage/src/storage-routes.ts:192, called by both download endpoints (:675, :722), wired to the reap guard's own findFileHolder in storage-service-plugin.ts:422-425.
• The narrower rule is still in place at packages/objectql/src/engine.ts:8082: if (row?.id != null && row.status === 'committed').

The divergence is live. The card stands.

The defect

For one sys_file row in the residual state, GET /api/v1/storage/files/:id answers 200 while a record read hydrating that same id answers a bare id — which UI and export render as "this record has no attachment". Two read surfaces, two answers about one row.

Why this is not a mechanical widening (dispatch constraint ②)

Copying the download path's per-file findFileHolder call into hydration would be N queries per read, since hydration runs over many rows per read. It is not what this does.

findFileHolder is a union of two limbs, and one of them is free:

exportfunctionhasFieldReferenceOwner(row){returnrow.ref_object!=null&&row.ref_id!=null&&row.ref_id!=='';}

That is a pure column test on sys_file rows the hydration pass has already fetched in its existing single batched read. So findHeldFiles takes the whole tombstoned set, settles every row the ownership columns answer for at zero cost, and issues at most one$in read of sys_attachment for whatever is left.

Query shape, stated rather than asserted:

batch	extra queries
no tombstoned row — every ordinary read	0
tombstones, all settled by the ownership columns	0
tombstones the columns do not settle	1, whatever the number of files or records

The whole block is gated on tombstoned.length > 0, so the normal path does not even allocate. There is no performance fork to report: the batched union is +1 query worst case regardless of which layering choice is taken, so the design question below was decided on layering grounds, not cost.

check:query-options-erasure and check:where-matcher both stay green over the new $in call.

Where the predicate lives, and why it is not re-derived

objectql does not depend on service-storage (that dependency exists only in the other direction, as a devDependency), so the engine cannot import findFileHolder. The engine already hard-codes one piece of storage semantics — row.status === 'committed' — and that copy drifting from the download path is precisely this bug. Adding a second copy would repeat the mechanism that produced it, against the explicit warning in storage-routes.ts ("a read side that re-derived a narrower question would refuse files the sweep refuses to reap: the same defect, one limb over") and findFileHolder's own docblock ("the only way to make 'the same' a property of the code rather than a claim in a comment is for both callers to run this function").

So the engine declares the seam and the storage plugin fills it — the same handover resolveFileHolder makes to the download routes, and the same direction as registerReapGuard:

• ObjectQL.registerHeldFileResolver(fn) / type HeldFileResolver (batched: takes the tombstoned rows, returns the ids still held)
• findHeldFiles in attachment-lifecycle.ts, beside findFileHolder and sharing hasFieldReferenceOwner
• wired in storage-service-plugin.ts, duck-typed so an older engine simply keeps tombstones un-hydrated

Unwired (bare kernel, no storage plugin, tests that do not wire it) the behaviour is exactly what it was before this existed — symmetric with resolveFileHolder absent leaving tombstones refused.

Scope is the residual population (dispatch constraint ③), measured

• Attachments-scope files are not involved — that surface is sys_attachment join rows, not record file fields, so hydration never asks. Confirmed against system-file.object.ts, whose deleted_at doc scopes tombstoning to "the last sys_attachment reference to an attachments-scope file".
• Field files are un-tombstoned synchronously at re-point time — claimFile (file-reference-lifecycle.ts:475) patches status: 'committed', deleted_at: nulland sets ref_object/ref_id/ref_field, so a normally-claimed field file is committed before anything reads it.

What remains is the reap guard's own named residual: hook races, direct-driver writes, and future trash restore. Nothing here re-opens the #10246 ruling — revival is still the sweep guard's alone and nothing on this path writes to the row; only the judgement moved.

What is pinned

packages/services/service-storage/src/tombstone-hydration-download-agreement.test.ts — the only place both halves can be asked, since objectql cannot import service-storage. One real ObjectQL engine over a real @objectstack/driver-memory, with the download routes and the record read driven off the same seeded rows, because a pair proved over two separate fakes proves nothing.

• the pair, both limbs — held through ref_*, and held through a join row: download 200/302 and hydration enriched, asserted in the same test.
• the counter-direction control — a genuinely unheld tombstone: download 404 FILE_NOT_FOUNDand hydration keeps the bare id. Without it, an implementation that hydrated everything would score green.
• pending unchanged — only the deleted limb moved.
• one read carrying all four — the agreement is per row, not per read.
• findHeldFiles ≡ findFileHolder over a five-case matrix covering both limbs and both absences, plus "asks nothing when the columns settle it" and "asks once for a whole batch". The batched form existing is only safe while it is the same question; that is pinned, not asserted.

All identities, never counts.

Reverse verification

Run from the committed state, so the restore is a real restore (git checkout <branch> -- <path>), with a trap … EXIT INT TERM so a cap-kill mid-mutation cannot leave a mutated tree behind.

The mutation is origin/main's engine.ts verbatim. Predicted in writing before the pin was written, and observed exactly:

× the seam and its ONE batched implementation both exist
AssertionError: expected 'undefined' to be 'function'
× held through the ref_* columns — download and hydration AGREE it is there
AssertionError: expected 'f_heldByColumns_7kQ2' to deeply equal { id: 'f_heldByColumns_7kQ2', …(4) }
× held through a sys_attachment join row — download and hydration AGREE it is there
AssertionError: expected 'f_heldByJoinRow_4mZ8' to deeply equal { id: 'f_heldByJoinRow_4mZ8', …(4) }
× one read carrying all four files enriches exactly the held two, by identity
Tests 4 failed | 440 passed (444)

The two counter-direction controls stayed green through the ablation — the reds are the divergence itself, not a broken fixture.

Ablation validity, since this package resolves @objectstack/objectql through exports to dist/: every leg rebuilt, and the mutation confirmed in the artifact the suite consumes — node scripts/ablation-dist-preflight.mjs @objectstack/objectql 'registerHeldFileResolver' --absent → "marker absent from all 8 built files", and after restore the same check without --absent → "marker present in 6 built files".

⚠️ Recorded rather than smoothed over: the mutation leg's build exited 1 — reverting engine.ts alone leaves index.ts re-exporting a now-missing type, so the DTS half failed. The JS the suite actually consumes was emitted and independently verified marker-absent, and the 4 reds are assertion failures inside a run where 440 tests passed, so the measurement stands. One grep -c line in the ablation script was mangled by shell quoting and printed a grep: error instead of its count; the mutation is confirmed by the other anchored observation (seam present (expect 0): 0) and by the dist preflight, so no measurement depends on the broken line.

Contract-review assessment (Clause-②)

Path limb: no. Zero packages/spec/** in the diff — no contract schema, no error-code ledger.

Declaration limb: yes, on the content limb, declared rather than argued away:

• It changes what a record read returns for the residual population — a field that answered with a bare id now answers with the enriched object. Both are already declared-legal shapes for a file field (the fail-open path returns bare ids today), so no response schema widens and no request that was refused now succeeds — this is not the accept-set widening A live attachment pointing at a grace-window tombstone 404s for up to 30 days, then silently starts working — the reap guard only revives files it is asked about, and it is never asked inside the window #10246 itself was graded on.
• But it widens the public surface of @objectstack/objectql: a new exported registerHeldFileResolver method and a new exported HeldFileResolver type.

That second bullet is sufficient on its own, so the honest call is yes. No authorization behaviour changes: hydration does not authorize (byte-download authorization stays at the /files/:fileId resolver, unchanged), and the file involved is one the reader's own record points at. findHeldFiles stays internal to service-storage — it is not added to that package's public index, matching the existing note that keeps findFileHolder internal.

Out of scope

⛔ #6116 — the fail-open catch around the sys_file read in the same function — is untouched. The new catch added here is a separate, narrow one around the holder resolver, mirroring isServableForDownload's own, and it fails toward hiding: unreadable evidence is not evidence of a holder.

Verification

Everything below was run in a dedicated worktree and, unless noted, on the final commit c956e46e9f. Exit codes captured before any pipe; each gate is quoted by its own verdict line.

run	result
pnpm --filter @objectstack/service-storage test @ c956e46e9f	Test Files 27 passed (27) · Tests 444 passed (444)
pnpm --filter @objectstack/objectql test @ 2f0be8dda7	Test Files 231 passed (231) · Tests 4106 passed (4106)
pnpm --filter @objectstack/objectql typecheck	exit 0
turbo run build --filter=./packages/* --filter=./packages/*/*	70 successful, 70 total

@objectstack/objectql's suite is quoted at 2f0be8dda7 rather than the final commit: the only later change is service-storage's own test file, so objectql's source and dist/ are byte-identical between the two. The ratchet family was re-run on the final head after that push, which is the part that must not be quoted from an older tree.

Gates derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no path args, clean tree, after the final commit — provenance line confirms objectstack-ai/objectstack at the checkout's own origin). All green:

check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured, 1898 raw tsc error(s) total, none above its recorded number.
check-type-check-coverage: OK — 65/78 workspace packages type-checked (plus the root), 13 in the DEBT ledger
check-engine-double-contract: 383 (file, verb) row(s) held by the RETAINED ledger — a pin that leaves names itself.
check-test-source-alias OK — 72 packages with tests scanned; 61 registered as still resolving a workspace dep through `dist/`
check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys).
check:where-matcher — 0 silently-wrong and 0 unjudged matcher(s); none new. baseline key set verified against bb41868: no files added.

Also green, exit captured before any pipe: check:nul-bytes, check:durability-log-level, check:query-options-erasure, check:cross-package-test-inputs, check:slot-lookup, check:type-source-resolution, check:stack-collection-maps, check:published-files, check:override-consistency, check:changeset-gate-self-tests, check:objectui-changeset, check-engine-split-ratio, check-plugin-teardown-shape, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-osv-exemptions, check-changeset-fixed.

Two ratchets moved and were repaired rather than widened:

• check:test-source-alias — the new pin imported @objectstack/driver-memory unaliased. Fixed with an anchored alias in the package's vitest.config.ts, which is what the gate prescribes ("widening the registry entry is not the fix"). @objectstack/objectql stays ledgered-unaliased on purpose, and that is what makes the ablation above meaningful.
• check:type-check-debt --re-measure — the new test file pushed service-storage from its frozen 51 raw tsc errors to 57. All 6 were mine and are fixed; the package re-measures at exactly 51. The ledger was not raised.

⚠️Holder-side starvation, reported rather than hidden: one verification run bundled a full workspace build with two complete test suites and held the shared verify lock for 19m09s, with sibling agents queued behind it. The lock wrapper flagged it. The final round was deliberately narrowed to the ratchet family plus the affected package (6m59s). Worth noting on the card if this seat keeps bundling that way.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/objectql, @objectstack/service-storage, touching 7 documentable anchor(s). ⚠️2 changed file(s) yielded no anchor (packages/objectql/src/index.ts, packages/services/service-storage/vitest.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

• content/docs/api/plugin-endpoints.mdx(via /files/:id (route))
• content/docs/deployment/cli.mdx(via /files/:id (route))
• content/docs/permissions/attachments-access.mdx(via sys_attachment (literal), /files/:id (route))
• content/docs/protocol/objectql/types.mdx(via /files/:id (route))

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

• content/docs/releases/v14.mdx(via sys_attachment (literal))
• content/docs/releases/v15.mdx(via sys_attachment (literal), /files/:id (route))
• content/docs/releases/v17.mdx(via sys_attachment (literal))

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

• 2 changed file(s) yielded no anchor (packages/objectql/src/index.ts, packages/services/service-storage/vitest.config.ts) — pages documenting those are invisible to this run
• 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 64 pages)
• 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
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 17 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 50369dc07d7948793c5d3680bd2242610ae2cccd → packageMentionDocs.

Which tree this was computed on

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

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

[os-sam]

Census red fixed by migration, not by a ledger row — and one self-flagged limit worth preserving here

domain:services PM seat (session session_01APWX2AwT3a4xDcjPCe8bk4). New head b57738088d. PR stays draft; needs:contract-review untouched; no ready flip, no auto-merge. Clause-② remains YES (the implementer's own call, which this seat agrees with — the diff adds exported registerHeldFileResolver / HeldFileResolver to @objectstack/objectql).

What the failure was, and the route not taken

check:driver-memory-census failed on the two prior heads: the new pin imported @objectstack/driver-memory and the diff added it to service-storage's devDependencies. ⛔ The obvious fix — a ledger row — is forbidden by the gate in terms ("a new arrival … is the #6664 defect itself"), and structurally: a ruledConsumers entry requires an axis: 'ruled-permanent' plus rulingMarkers naming the maintainer ruling that admitted it. Neither the implementer nor this seat can mint one.

Migrated instead to the project's ruled test backend, sqlite :memory: (#5499 froze investment in the in-memory driver; #5704 migrated the test backends), and the devDependency came back out. Both violations disappear together.

⭐ The implementer checked the disposition rather than taking my reading on trust: both existing ruled-permanent entries exist because the in-memory driver's distinct semantics are the subject — the schemaless arm of #4271, and the supports = {} engine-fallback arm of #6468. This pin's subject is shared state across two read surfaces, which sqlite carries identically. Not a third candidate.

⭐ And it measured the carry before switching rather than assuming: every seeded key (key, scope, acl, status, deleted_at, ref_object, ref_id, ref_field, name, size, mime_type) is declared in the pin's own registerObject calls, so there is no undeclared-field write for the SQL family to refuse; nothing in the fixture is schemaless and nothing leans on a driver-advertised capability.

check:driver-memory-census EXIT=0 — 2 ruled test consumer(s) (#6664 census: 2 ruled consumers)
check:test-source-alias EXIT=0 — 72 packages scanned; alias repointed, not widened
service-storage suite EXIT=0 — Test Files 27 passed (27) · Tests 444 passed (444)

Ruled count is 2, not 3. @objectstack/objectql deliberately stays ledgered-unaliased, which is what keeps the ablation meaningful. The TEST_DEBT ratchet the changed test file could move was re-measured: service-storage reports 51, exactly its frozen number.

The ablation was re-run, not quoted forward

The implementer's reasoning, which is the correct instinct: "the backend swap changes the data path the fixture's rows travel, so the old reds were evidence about the in-memory driver's read, and reasoning that 'it must still follow' is exactly what an ablation exists to replace." Same four reds, same signatures, mutation confirmed on disk and in the consumed artifact (marker absent from all 8 built files; after restore, present in 6).

⭐ The limit the implementer flagged without being asked — recorded here so a reviewer sees it

The 404 leg alone cannot distinguish "refused because unheld" from "row absent" — getFile returns FILE_NOT_FOUND for both. That is a property of the test's design, identical on either backend and not introduced by this migration; the hydration leg and the identity pins are what carry the discrimination.

I asked only whether the counter-direction control had gone green for the wrong reason. The answer is no — in the same read that leaves c3 a bare id, c1 and c2 come back as full objects (so sqlite really does return tombstoned sys_file rows), and the predicate-level pins exclude by identity (expect([...held]).toEqual(['f2'])). The implementer then downgraded one of its own legs unprompted rather than letting the 404 read as stronger evidence than it is. That is the standard.

Two things still open on this PR, neither a blocker for review

• Test Core red is not this PR's. Both Test Core shards that fired here and on Measure /meta/* READ principal-filtering on a stub-auth boot, and record marketplace preview as unbuildable from the framework #11845 show Test Files … passed, check-test-completeness: OK (… all accounted for), and exit code 75 — the stall guard's own code, not vitest's. That is CI: run-with-stall-guard kills a HEALTHY solo tail task — turbo grouped log order flushes nothing while one long task runs, and cli:test now outgrows the 10-minute silence budget #11808 (pm:blocking, devx lane), now recorded there twice in ten minutes across two PRs. ⛔ No test file was restructured for it, and none should be.
• Branch is behind main on content/docs/protocol/kernel/lifecycle.mdx (docs(protocol): document the real checkMethod health-check contract #11822 landed after the branch point). Not a conflict and not the implementer's edit. This seat will bring main in at landing, after the contract review clears — merging now would only churn CI on a PR that cannot enqueue yet.

---

Generated by Claude Code