Skip to content

feat(desktop): garbage-collect redundant recovery branches to trash#6102

Merged
SivanCola merged 2 commits into
esengine:main-v2from
SivanCola:feat/recovery-branch-gc
Jul 6, 2026
Merged

feat(desktop): garbage-collect redundant recovery branches to trash#6102
SivanCola merged 2 commits into
esengine:main-v2from
SivanCola:feat/recovery-branch-gc

Conversation

@SivanCola

Copy link
Copy Markdown
Collaborator

Summary

补上 #5993 的最后半截:冲突触发源已在 1.17.x 修复,但历史上产生的每一个恢复分支仍堆在会话列表里,只能手动清理(该 issue 明确抱怨的"会话列表被大量 recovery 文件污染")。

清理策略(保守优先——recovery 分支的存在意义就是防数据丢失,宁可漏收不可误删):

agent.ReclaimableRecoveryBranches 仅当全部条件成立才判定可回收:

  1. meta 标记 Recovered 且记录了 fork 时的内容摘要;
  2. 当前 transcript 仍与该摘要一致——从未被续聊过(用户在分支上多聊一轮即永久豁免);
  3. 父会话(meta.ParentID,同目录)仍存在且覆盖分支内容(摘要相等,或分支是父的前缀、允许兼容的 system 消息替换)——与 SaveRecoveryBranch 判定"无需恢复"用的是同一组检查,即该分支未保留任何独有内容;
  4. 无任何运行时持有该分支的会话租约(新增 agent.SessionLeaseHeld,含本进程——SessionLeaseHeldByOtherRuntime 对本进程故意返回 false,GC 需要更严的问法);
  5. 空闲超过 24h 宽限期(新 fork 属于进行中的冲突流程,用户可能正在对比)。

处置方式:desktop 启动时 + 每 6 小时后台扫描所有会话目录(全局 + legacy 共享 + 每个已保存项目),处置前再次核查租约与"是否在某个 tab 打开"(扫描是快照,期间用户可能打开了分支),然后走完整的 DeleteSession 流程——移入可恢复的会话回收站,绝不硬删;即使启发式对某个分支误判,用户也能从回收站找回。

Verification

  • Agent 层:TestReclaimableRecoveryBranchesCollectsOnlyCoveredIdleForks(covered 回收 / diverged 保留 / 续聊过保留)与 TestReclaimableRecoveryBranchesRespectsGraceLeaseAndMissingParent(宽限期内保留、租约持有保留并验证 SessionLeaseHeld 本进程语义、释放后回收、父缺失保留)。fixtures 通过真实的 SaveRecoveryBranch 冲突流程构造。
  • Desktop 层:TestRecoveryGCTrashesCoveredForkAndKeepsParent(回收进回收站、父会话不动、二次扫描零回收)与 TestRecoveryGCSkipsBranchOpenInTab
  • 反向验证:临时移除"父覆盖"守卫后,diverged 分支(内容仅存于此)被误判可回收、测试恰在该处失败——数据丢失守卫真实生效。
  • go test ./internal/agent(19.7s)与完整 desktop 套件(40.8s)全绿;-race 覆盖 GC/租约路径;go vet 干净。

Cache impact

Cache-impact: none - session file lifecycle only; no provider request, prompt, or tool schema changes.
Cache-guard: not applicable (no cache-sensitive files); suites above cover the change.
System-prompt-review: N/A

Refs #5993(收尾其"列表污染"部分), #6054.

Conflict recovery forks a copy of the transcript on every save conflict
(esengine#5993). The triggers are fixed, but every fork that ever happened still
sits in the session list until the user trashes it by hand — the
"会话列表被污染" half of that report was never addressed.

Add a conservative GC. agent.ReclaimableRecoveryBranches reclaims a
branch only when EVERY condition holds, because a recovery branch exists
precisely to prevent data loss:
  - meta says Recovered with a fork digest;
  - the transcript still matches that digest (never continued on — one
    follow-up turn disqualifies it permanently);
  - the parent (meta.ParentID) still exists and COVERS the branch content
    (equal digest, or branch is a compatible prefix — the same checks
    SaveRecoveryBranch uses to declare recovery unnecessary);
  - no runtime holds the branch's session lease;
  - it has been idle past a 24h grace period.

The desktop sweeps every session directory it lists from, once at startup
and every 6h, re-checking liveness (lease + open-in-tab) immediately
before disposal and routing removal through DeleteSession — so a swept
branch is moved to the recoverable session trash, never hard-deleted.

Verified the coverage guard by reverting it: a diverged fork (content
present nowhere else) then becomes reclaimable and the test fails.
@SivanCola SivanCola requested a review from esengine as a code owner July 6, 2026 12:25
@github-actions github-actions Bot added v2 Go rewrite (1.x) — main-v2 branch, active development desktop Wails desktop app (desktop/**) agent Core agent loop (internal/agent, internal/control) labels Jul 6, 2026
Review catch: startup launches restoreOrBuildTabs on a goroutine and
started the GC immediately after, so the first sweep could run against
the pre-restore empty tab map. Two consequences: sessionOpenInAnyTab
would judge every saved tab's session as closed (a recovery tab about
to be restored could be swept to trash), and DeleteSession's tab-list
persistence would overwrite desktop-tabs.json with the empty snapshot,
wiping the user's saved tab layout.

Add a tabsRestored gate: armed in startup before the restore goroutine
launches, closed via defer when restoreOrBuildTabs finishes (both the
saved-tabs and first-launch paths, and the panic-recover path). The
GC's first sweep waits on the gate or ctx cancellation. markTabsRestored
is idempotent; tabsRestoredSignal reports already-restored when the gate
was never armed, so App instances built directly in tests keep working.

Regression test forces the exact race: gate closed → sweep provably
blocked and the branch untouched; restore lands the tab holding the
branch and opens the gate → the sweep runs and skips the now-open
branch.
@SivanCola

Copy link
Copy Markdown
Collaborator Author

Valid P1 — fixed in 89f52f1.

Confirmed all three legs of the race: startup launches restoreOrBuildTabs on a goroutine and started the GC immediately after with no ordering guarantee; sessionOpenInAnyTab against the pre-restore empty map judges every saved tab's session as closed; and DeleteSessionremoveSessionRuntimeBindingssaveTabsCollectLocked unconditionally persists the current a.tabs snapshot — so an early sweep could both trash a recovery branch that a saved tab was about to restore AND overwrite desktop-tabs.json with an empty tab list.

Fix: a tabsRestored gate. Armed in startup before the restore goroutine launches; closed via defer when restoreOrBuildTabs finishes — covering the saved-tabs path, the first-launch path, and the panic path (the defer runs before recoverToPending's recovery). The GC's first sweep now waits on the gate or ctx cancellation. markTabsRestored is idempotent, and tabsRestoredSignal reports already-restored when the gate was never armed, so test-constructed App instances are unaffected.

Regression test TestRecoveryGCFirstSweepWaitsForTabRestore forces the exact interleaving from the report: with the gate closed the sweep is provably blocked (100ms window, branch untouched on disk); restore then lands the saved tab holding the branch and opens the gate; the sweep runs and skips the now-open branch. Also asserts markTabsRestored idempotency.

Verification: full desktop suite green (38.6s), -race over the GC tests green, go vet clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Core agent loop (internal/agent, internal/control) desktop Wails desktop app (desktop/**) v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant