feat(storage): reclaim the prepared-request captures already on disk

[Astro-Han]

Summary

#4722 stops producing prepared-request captures. This reclaims the ones already on disk. On my own 814 MB installation they are 772.7 MB — 87% of the Artifact population, and 99.4% of their bytes are re-serialized duplicates of messages already in the ledger.

A bounded background sweep drains them through the same purge-intent path a Session delete uses, then stops. It reclaims the files and the Artifact rows naming them — nothing else. The per-segment detail #4722 stopped writing sits in the AgentRun ledger, which is append-only by design, so those rows stay where they are and only new calls are cheaper.

#4722 has landed, so this now stands on its own: 6 commits on current main.

Refs #4037, #4704.

🔀 What this changes about deletion

flowchart TB
    subgraph BEFORE["until now"]
        direction LR
        P1["purgeSessionArtifacts"] --> B1["Artifact bytes"]
        P1 --> E1["events naming them"]
    end
    subgraph AFTER["this sweep"]
        direction LR
        P2["purgeRetiredCaptures"] --> B2["capture bytes"]
        E2["attempts naming them<br/>append-only, cannot be retracted"]
    end
    BEFORE -.-> R["⇒ a capture reference<br/>must tolerate a missing referent"]
    AFTER --> R

    classDef gone fill:#fcebeb,stroke:#e24b4a,color:#a32d2d
    classDef live fill:#e1f5ee,stroke:#1d9e75,color:#0f6e56
    classDef rule fill:#fff6e0,stroke:#d99a1f,color:#8a5f00
    class B1,E1,B2 gone
    class E2 live
    class R rule

Loading

Artifact bytes and the events naming them have always been removed together. This sweep is the first mechanism that removes bytes while the events referencing them live on, because the AgentRun ledger is append-only: a historical ModelCallAttempt keeps its captureArtifactId forever.

That is the review question — who reads a capture reference, and does it survive the referent being gone.

• captureArtifactId in a conversation copy was a required mapping. Once a capture was reclaimed, branching or copying any Session holding a historical model call threw Conversation copy is missing Artifact … and could never succeed again. The key now leaves with the bytes; the attempt still copies and still decodes.
• Every other Artifact reference in conversation-copy.ts — attachment refs, tool-result Artifacts, snapshot lists, linked children — still throws on a missing target, and should. The sweep filters on source === 'provider_request_capture', and none of those name that source.
• A conversation copy no longer carries retired captures over. A fork copied every Artifact of the source Session, these included — putting condemned bytes into a new Session, and putting them back after the sweep had seen an empty residue and stopped for good. Those copies were never reclaimed by anything. The copy now leaves that one source out, which also stops it racing the sweep over the same record.
• Decoding is untouched. captureArtifactId stays in the ModelCallAttempt schema and its validator: hasExactShape fails a whole record on an unknown key, so dropping the field would strand exactly the records this exists to free the bytes of.

Also removes the two capture decoders #4631 left behind in conversation-copy.ts when it retired both provider-request event types from the emitted catalogue — a copy no longer reaches either.

🧹 The sweep

starts	after openedArtifactStore.recover() — a write authority refuses every mutation until it has recovered
batch	16–256 Artifacts, steered toward a batch costing ~100 ms — a purge rewrites the whole Artifact table, so a batch costs what the store costs, not what its own size costs
pause	max(250 ms, 3 × last batch) — a batch holds the writer lock and its cost rises with the store, so a fixed pause would not stay behind live turns
on failure	retries after 1 s; gives up only after five consecutive failures
stops	when nothing is left, or when the Runtime Host closes

Each batch is durable on its own, so stopping only means the next one does not start; a later run continues from the residue.

Giving up on the first failed batch is what the earlier revision did, and it was wrong in the way that costs the most: the failure that says the least about the next batch is exactly the one it quit on. A purge that fails part way leaves the write authority refusing every mutation until something recovers it — so a single bad batch stopped all reclamation for that user, permanently and silently. onError is now where the repair belongs, and the host recovers the store there; that same recovery is what hands the live turn's own writes back.

✅ Verification

• format, lint clean. test:dist: @maka/storage 1,126, @maka/runtime 3,219, @maka/runtime-host 1,687 — 0 failures. typecheck also on @maka/desktop, @maka/ui, @maka/mcp, @maka/eval, maka-agent.
• The sweep tests now drive a real writer end to end rather than a fake, which is what would have caught the give-up-on-first-failure bug: wired ahead of recovery, every batch is refused.
• The copy test fails without the fix — verified by putting rewriteOwnedArtifactId back: Error: Conversation copy is missing Artifact artifact-gone.
• Not run: full-repo suite, Playwright E2E. Nothing user-visible changes; the reclaimed Artifacts have had no reader since refactor: replace Headless with minimal Eval kernel #2605.

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — traced the reference points, wrote the implementation and tests. Reviewed and verified by me.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[likun666661]

I think the core causal spine is sound: identify the retired provider_request_capture records, run them through the existing durable purge path in bounded background batches, and make the append-only attempt ledger tolerate the missing referent.

However, on the current head (70fe70b), several claims in the PR body do not appear to be implemented:

1. Conversation copies can still recreate retired captures. copyConversationArtifacts() still selects source-session records by turn/exclusion only; it does not filter out source === 'provider_request_capture'. If the sweep reaches zero and stops, a later copy of an old Session can copy those captures into the target Session. They will remain until a later Runtime Host start. Either exclude this source in the copy path and add a regression test, or describe the guarantee as eventual cleanup on a later restart.

2. The sweep still gives up after the first failure. startRetiredCaptureSweep() calls onError and immediately returns, and the test explicitly asserts one call. I do not see the documented 1-second retry, five-consecutive-failure limit, or store recovery in the error path. Because a failed purge invalidates writer state, recovery is also relevant to foreground writes, not only reclamation.

3. The documented batch behavior differs from the code. The body says 16-256 Artifacts steered toward ~100 ms; the current implementation uses a fixed batch of 256. GitHub also reports 4 commits on the PR while the body says 6, so these may simply be missing commits.

From an Occam perspective, I would define the minimum complete requirement as:

Delete exactly the obsolete capture Artifacts already on disk, without breaking the retained ledger/copy path, blocking live writes, or allowing copies to recreate the retired population after the sweep has stopped.

The source filter, durable purge path, bounded pacing, post-recovery start, droppable captureArtifactId join, copy exclusion, and failure recovery/retry all protect a necessary link or boundary in that causal chain. The exact batch constants and old decoder cleanup are implementation/detail cleanup rather than part of the problem definition.

Could you either push the missing implementation described in the body or align the body and guarantees with the current head?

[Astro-Han]

Sorry — you were reading the right code; I pushed the fixes to the wrong remote, so the head stayed at 70fe70b while the body described work you couldn't see. Head is now 82ff4a487d (6 commits).

1. Copy exclusion — done, with a regression test. Your framing of the consequence is sharper than mine: since the sweep exits for good at remaining === 0, copies made after it finished were never reclaimed by anything, not just deferred to the next restart.
2. Failure handling — retries after 1 s, gives up after five consecutive failures, and onError now calls openedArtifactStore.recover(). You're right that this matters beyond reclamation: that recovery is what hands the live turn's own writes back. The sweep tests now drive a real writer, which is what would have caught the original bug.
3. Batch pacing — nextSweepBatch() steers 16–256 toward a ~100 ms batch, as the body says. The commit-count mismatch was the same wrong-remote problem.

Happy to split the old decoder cleanup out if you'd rather keep this to the minimum.

[jackwener]

Reviewed at exact head 82ff4a487d9aae0f7296d62c6c3efb9b36f18aa1. Five independent reviews ran on this, each sealing before reading the others. Four of them cleared the deletion itself. One found a reachable gap that I am raising as [P1], so I am not approving this head — everything else here is sound, and the fix looks small.

[P1] The retired-capture filter guards one of three selection paths

copyConversationArtifacts selects records in three passes. Only the first excludes the source, and its comment states the property the whole design depends on:

// A retired capture is on its way off disk and nothing reads one.
// Copying it would hand the new Session bytes already condemned,
// and would put records back after the sweep finished and stopped
// looking. Leaving them out also keeps the two from racing: the
// sweep can no longer delete a record this copy is holding.
record.source !== RETIRED_CAPTURE_ARTIFACT_SOURCE,

The next two passes do not look at source at all:

• linkedArtifacts (artifact-store.ts:440-451) matches on sessionId, id and status !== 'deleted', then pushes the record.
• includedArtifactIds (:453-463) matches on sessionId and the include/exclude sets.

This is reachable in production, not hypothetical. Side Conversation copy builds a linkedArtifacts request — kind === 'side_conversation' && linkedReferences.references.size > 0 — and one reviewer's probe confirmed by measurement that both unfiltered paths copy a capture into the target with source: provider_request_capture intact.

The consequences follow directly from the comment above:

1. Condemned bytes land in a new Session. The copy hands the target rows that are on their way off disk.
2. They are re-created after the sweep has finished. The sweep stops when nothing is left; a copy afterwards puts fresh provider_request_capture rows back, and the sweep is no longer looking.
3. A later sweep reclaims them, hollowing out the snapshot's rewritten artifactIds.
4. The race the comment says is prevented is not prevented on these paths. linkedArtifacts throws Linked Artifact <id> could not be copied when the record is gone — so a sweep that removes a capture between selection and copy turns a Side Conversation into a hard failure rather than a dropped key.

The PR summary's claim — "A conversation copy no longer carries retired captures over … which also stops it racing the sweep over the same record" — holds for the turn-scoped pass and not for the other two.

Smallest fix: apply the same source !== RETIRED_CAPTURE_ARTIFACT_SOURCE exclusion to all three selection passes. A regression should drive a real parent+child state through the Side Conversation coordinator, sweep, and then a real revision — asserting that ordinary child artifacts still copy.

What five reviews cleared

The deletion scope is correct. Exact-source deletion measured at 6/6 including tombstones and copy-rebuilt rows; all 14 remaining rows across the other 13 ArtifactSource values plus source-less rows survive; non-capture hardlink inodes and bytes stay readable.

Nothing else reads a capture reference. Enumerated independently by three seats, converging: the only production readers of captureArtifactId are the Core validator and conversation-copy.ts. Diagnostics and the ledger never dereference it.

Keeping captureArtifactId in the schema is load-bearing, not conservative. hasExactShape rejects any unknown key outright — measured directly: a record carrying the field validates while allowed contains it, and the same record fails the moment the key is removed from allowed. Dropping the field would strand exactly the records this change exists to free the bytes of.

The sweep's failure handling is sound. Five consecutive failures stop the loop in this process only — there is no persistent cursor, and the next Runtime Host start resumes over what remains. A successful batch resets the counter. stop() prevents the next batch rather than interrupting the current one, and an interrupted batch leaves a purge intent that recovery replays.

Two defensive observations that do not block, from the cross-team seat:

• Closure does not wait for an in-flight sweep. stop() sets a flag and close proceeds to storage.close; if closure lands between file deletion and the metadata write, the batch can delete its targets and fail the metadata write, leaving a purge intent that the next recovery replays. The target set is still limited to retired captures. Worth having stop await the in-flight batch, or folding it into a unified drain.
• On a large store each batch still resolves removal entries across every non-batch record as a path guard, so the 16–256 adaptive batch cannot shorten lock hold time — shrinking the batch does not help. That is live-turn latency rather than wrong deletion.

(The reference-filter finding above was reached independently by two seats and, from the opposite direction, by a third that had classified it as unreachable in production until the Side Conversation caller was identified.)

Everything except the filter gap is in good shape, and the premise — that these bytes are duplicates nothing reads — is well supported.

简体中文

在 82ff4a487d9aae0f7296d62c6c3efb9b36f18aa1 上评审。五次独立评审在这个 head 上进行,各自封存后才互看。其中四次确认删除本身是正确的;一次找到了一个生产可达的缺口,我按 [P1] 提出,因此这个 head 我不批准——除此之外都是扎实的,而且修法看起来很小。

[P1] retired-capture 过滤只守住了三条选择路径中的一条

copyConversationArtifacts 分三段选记录。只有第一段排除了这个 source,而它的注释恰好道出了整个设计所依赖的性质:

// 一条退役 capture 正在离开磁盘,而且没有人读它。
// 复制它等于把已被判定要删的字节交给新 Session,
// 并且会在 sweep 结束、不再查看之后把记录放回去。
// 把它们排除在外也让两者不再竞争:
// sweep 不能再删掉这次复制正持有的记录。
record.source !== RETIRED_CAPTURE_ARTIFACT_SOURCE,

而后两段根本不看 source:

• linkedArtifacts(artifact-store.ts:440-451)只匹配 sessionId、id 和 status !== 'deleted',然后就把记录推入。
• includedArtifactIds(:453-463)只匹配 sessionId 和 include/exclude 集合。

这在生产中是可达的,不是假设。 Side Conversation 的复制会构造 linkedArtifacts 请求——kind === 'side_conversation' && linkedReferences.references.size > 0——而一位评审的探针实测确认:这两条未过滤的路径都会把 capture 复制进目标,且 source: provider_request_capture 原样保留。

后果直接来自上面那段注释:

1. 被判定要删的字节进入了新 Session。 复制把正在离开磁盘的行交给了目标。
2. 它们在 sweep 结束之后被重新造出来。 sweep 在没有剩余时停止;之后的一次复制会放回新的 provider_request_capture 行,而 sweep 已经不再查看了。
3. 之后的 sweep 会回收它们,把 snapshot 中被重写过的 artifactIds 挖空。
4. 注释声称被阻止的那个竞争,在这两条路径上并没有被阻止。 当记录已不存在时,linkedArtifacts 会抛出 Linked Artifact <id> could not be copied——所以一次在「选择」与「复制」之间移除了 capture 的 sweep,会把一次 Side Conversation 变成硬失败,而不是丢掉一个 key。

PR 摘要里那句——「conversation copy 不再把退役 capture 带过去……这也阻止了它与 sweep 争抢同一条记录」——对 turn 那一段成立,对另外两段不成立。

最小修法: 把同一个 source !== RETIRED_CAPTURE_ARTIFACT_SOURCE 排除应用到全部三段选择上。回归测试应当驱动一个真实的 parent+child 状态,经过 Side Conversation coordinator、sweep,再走一次真实 revision——并断言普通的 child artifacts 仍然会被复制。

五次评审确认没问题的部分

删除范围是正确的。 精确 source 删除实测 6/6(含 tombstone 与两条 copy 重建行);其余 13 个 ArtifactSource 加上无 source 的共 14 行全部保留;非 capture 的 hardlink inode 与字节仍可读。

没有别的地方读 capture 引用。 三个席位独立穷举并收敛:captureArtifactId 的生产读者只有 Core 校验器和 conversation-copy.ts,diagnostics 与 ledger 从不解引用它。

把 captureArtifactId 留在 schema 里是承重的,不是保守。 hasExactShape 对任何未知 key 一律否决——直接实测过:一条带该字段的记录在 allowed 含它时通过校验,而同一条记录在该 key 被移出 allowed 的瞬间失败。删掉这个字段,会让正是这次改动要释放其字节的那批记录全部失效。

sweep 的失败处理是稳的。 连续五次失败只停止本进程内的循环——没有持久游标,下一次 Runtime Host 启动会从剩余部分继续。成功一批会重置计数。stop() 阻止的是下一批而不是打断当前批,而被打断的一批会留下 purge intent 供 recovery 重放。

两条不阻塞的防御性观察,来自跨团队席位:

• 关闭路径不等待进行中的 sweep。 stop() 只置标志,close 继续走 storage.close;若关闭恰好落在文件删除与 metadata 写入之间,该批次可能删完目标却写 metadata 失败,留下一个 purge intent 由下次 recovery 重放。目标集合仍只限于退役 capture。建议让 stop 等待在飞批次,或纳入统一的 drain。
• 大库上每批仍会把「非本批的每一条记录」都解析一遍作为路径守卫,所以 16–256 的自适应批次缩不短锁持有时间——把批次调小并没有帮助。这是活跃 turn 的延迟问题,不是删错数据。

(上面那条引用过滤的发现由两个席位独立得出;第三个席位从相反方向得到同一处,并在 Side Conversation 调用方被指认之前一直将其归类为「生产不可达」。)

除了这个过滤缺口,其余都处于良好状态;而这次改动的前提——这些字节是无人读取的重复数据——是有充分支撑的。

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.