Uh oh!
There was an error while loading. Please reload this page.
feat(desktop): warn that deleting a parent keeps and archives its subtasks - #3781
Conversation
Astro-Han
commented
Aug 25, 2026
The gap is real — archived rows labelled 原父任务已删除 appearing with no warning is confusing, and telling people beforehand is the right fix. One note on how, non-blocking. [P2] The count is a second implementation of the Host's plan, and already disagrees
One of them can't be implemented here. The Host filters Parent resolution differs. Both skew high. Normal user path, display-only, recoverable — hence P2. The smaller shapeThe toast doesn't need an estimate at all. By then the Host has executed its plan and knows the real number; having For the confirm, I'd drop the number. Nobody decides differently between "3 subtasks will be kept and archived" and "5" — what matters is the fact that they survive. Without the number, If you'd rather keep an exact count in the confirm, that needs a removal-plan preview from the Host — a new operation, but then the renderer stops guessing. Your call; I'm fine either way, just not with a number that can be wrong. 简体中文这个缺口是真的——归档列表里冒出标着「原父任务已删除」的行却毫无预告,确实让人困惑,提前告知是对的解法。关于怎么做有一条意见,不阻塞。 [P2] 这个数字是 Host 计划的第二次实现,而且已经不一致
其中一条在这里实现不了。 Host 在两处过滤 父级判定不同。 两条都让数字偏大。正常用户路径,纯显示,可恢复——所以是 P2。 更小的形状toast 那半边根本不需要估算。到那时 Host 已经执行完计划,知道真实数字;让 confirm 那半边,我会把数字去掉。没有人会因为「3 个子任务会被保留并归档」和「5 个」而做出不同决定——重要的是它们会存活这个事实。不带数字的话, 如果你更希望 confirm 里保留精确数字,那需要 Host 提供一次 removal-plan preview——多一个操作,但渲染层不再猜。你定;两种我都可以,只是不希望留一个可能是错的数字。 |
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed this head and found no blocking issues.
Deleting a parent now correctly warns that ordinary subtasks are retained and archived (count via live catalog, deduplicated, excluding graph operators), matching Host readRemovalPlan. No new state or protocol; notifications now match the post-delete visible row.
No P0-P3.
简体中文
该头未发现阻断。Addresses review on apache#3781: the renderer's countArchivableSubtasks was a second implementation of the Host's removal plan and diverged from it (the conversationCopy 'preparing' filter is unimplementable off SessionSummary, and the parent-link fallback reads a field the Host ignores), so the confirm could name a subtask count that was wrong. Drop the number from the confirm — it now warns only that linked subtasks are kept and moved to Archived, the fact nobody decides differently about — and reduce the renderer helper to a presence check. The completion toast now reports the Host's executed count: the session.remove result carries archivedSubtaskCount (present only when it archived something), deduplicated by revision family in the coordinator, threaded through removeSession, the sessions:remove IPC, and the preload bridge to the toast. Deletion semantics are unchanged. Generated-by: Claude Code
Thanks — took the smaller shape (see the latest revision on the branch). Toast: Confirm: dropped the number. It now warns only that linked subtasks are kept and moved to Archived — the fact that they survive. Both divergences you flagged are moot now — the renderer no longer re-derives the plan, so the Also rebased onto current 简体中文多谢——按更小的形状改了(见分支上的最新一版)。 toast: confirm:去掉了数字。现在只警告子任务会被保留并移入归档这个事实。 你指出的两条不一致现在都不成立了——渲染层不再重新推导计划,所以那条实现不了的 另外已 rebase 到最新 |
Addresses review on apache#3781: the renderer's countArchivableSubtasks was a second implementation of the Host's removal plan and diverged from it (the conversationCopy 'preparing' filter is unimplementable off SessionSummary, and the parent-link fallback reads a field the Host ignores), so the confirm could name a subtask count that was wrong. Drop the number from the confirm — it now warns only that linked subtasks are kept and moved to Archived, the fact nobody decides differently about — and reduce the renderer helper to a presence check. The completion toast now reports the Host's executed count: the session.remove result carries archivedSubtaskCount (present only when it archived something), deduplicated by revision family in the coordinator, threaded through removeSession, the sessions:remove IPC, and the preload bridge to the toast. Deletion semantics are unchanged. Bump RUNTIME_HOST_COMPATIBILITY_EPOCH to 50: older peers reject the new optional field on the closed removed-result shape. Generated-by: Claude Code
70afc15 to
d70366eCompareAddresses review on apache#3781: the renderer's countArchivableSubtasks was a second implementation of the Host's removal plan and diverged from it (the conversationCopy 'preparing' filter is unimplementable off SessionSummary, and the parent-link fallback reads a field the Host ignores), so the confirm could name a subtask count that was wrong. Drop the number from the confirm — it now warns only that linked subtasks are kept and moved to Archived, the fact nobody decides differently about — and reduce the renderer helper to a presence check. The completion toast now reports the Host's executed count: the session.remove result carries archivedSubtaskCount (present only when it archived something), deduplicated by revision family in the coordinator, threaded through removeSession, the sessions:remove IPC, and the preload bridge to the toast. Deletion semantics are unchanged. Bump RUNTIME_HOST_COMPATIBILITY_EPOCH to 50: older peers reject the new optional field on the closed removed-result shape. Generated-by: Claude Code
d70366e to
71462f1CompareAddresses review on apache#3781: the renderer's countArchivableSubtasks was a second implementation of the Host's removal plan and diverged from it (the conversationCopy 'preparing' filter is unimplementable off SessionSummary, and the parent-link fallback reads a field the Host ignores), so the confirm could name a subtask count that was wrong. Drop the number from the confirm — it now warns only that linked subtasks are kept and moved to Archived, the fact nobody decides differently about — and reduce the renderer helper to a presence check. The completion toast now reports the Host's executed count: the session.remove result carries archivedSubtaskCount (present only when it archived something), deduplicated by revision family in the coordinator, threaded through removeSession, the sessions:remove IPC, and the preload bridge to the toast. Deletion semantics are unchanged. Bump RUNTIME_HOST_COMPATIBILITY_EPOCH to 50: older peers reject the new optional field on the closed removed-result shape. Generated-by: Claude Code
71462f1 to
f2b4bcbCompareAddresses review on apache#3781: the renderer's countArchivableSubtasks was a second implementation of the Host's removal plan and diverged from it (the conversationCopy 'preparing' filter is unimplementable off SessionSummary, and the parent-link fallback reads a field the Host ignores), so the confirm could name a subtask count that was wrong. Drop the number from the confirm — it now warns only that linked subtasks are kept and moved to Archived, the fact nobody decides differently about — and reduce the renderer helper to a presence check. The completion toast now reports the Host's executed count: the session.remove result carries archivedSubtaskCount (present only when it archived something), deduplicated by revision family in the coordinator, threaded through removeSession, the sessions:remove IPC, and the preload bridge to the toast. Deletion semantics are unchanged. Bump RUNTIME_HOST_COMPATIBILITY_EPOCH to 50: older peers reject the new optional field on the closed removed-result shape. Generated-by: Claude Code
f2b4bcb to
a3432ebCompare
Astro-Han
left a comment
There was a problem hiding this comment.
Update on a3432eba7e:
[P2] Deletion warning uses missing graph marker — false archive promise
parentHasArchivableSubtasks() checks session.subagentParent.graph, but catalog projection strips it to {parentSessionId} only; renderer always takes fallback and claims subtasks will be archived, while Host actually retires graph children with count 0 — dialog contradicts execution.
Fix: carry graph/operator marker through catalog authority to renderer (bump epoch) or make this a Host-owned preflight with real projection test.
Checks on a3432eba7e930ebbea11c2b4681f2ea41bf6584d are test: SUCCESS (32876519613) — code is NO-GO.
简体中文
删除提示与实际执行不一致。
Astro-Han
left a comment
There was a problem hiding this comment.
Update on 15dd64a11f:
[P2] Preview is fail-open and not bound to remove plan
app-shell-session-row-actions.ts:170-185 swallows preview errors and proceeds with 0 count; protocol carries only sessionId without revision/digest. Race: preview says no archivable children, but by confirm time a subtask was unarchived or preview transiently failed — delete silently archives without warning, contradicting the confirmation purpose.
Fix: fail-closed on preview error or show uncertainty, and bind revision/plan digest to remove with re-confirm on change.
Prior graph-only P2 is now closed (Host preview returns 0). Hosted test: IN_PROGRESS — not green.
简体中文
确认预览与执行脱节。liuxiaocs7
commented
Aug 25, 2026
Fixed on the latest head ( Confirmed: the catalog projection carries a linked child as Fix: the renderer stops re-deriving the plan entirely. A read-only Epoch 51 covers this round (the removed-result field plus the new query). Checks are green on 简体中文已在最新 head( 已核实:catalog 投影把链接子任务表示为 修法:渲染层彻底不再推导计划。新增只读的 epoch 51 覆盖本轮(removed 结果新字段 + 新查询)。 |
Astro-Han
commented
Aug 25, 2026
Need rebase! |
15dd64a to
be094cbCompare
Astro-Han
left a comment
There was a problem hiding this comment.
Update on be094cb043:
[P2] Preview fail-open plus batch purge bypass
- Rows:
session-row-actions.ts:177-188swallows preview errors as 0 and shows delete confirm without archive warning, while remove may still archive linked subtasks; no revision binding. - Batch:
tasks-settings-pagepurgePaths omit HostarchivedSubtaskCountpreview and toast, silently archiving active subtasks.
Fix: fail-closed or warn on preview error, bind plan token, and gate bulk purges via Host preview.
Checks on be094cb043dcebbdeda1a7357b663950a681952f are test: IN_PROGRESS — not green.
简体中文
批量删除绕过归档预览。liuxiaocs7
commented
Aug 26, 2026
Addressed the two concrete gaps on the latest head ( ① Preview fail-open → fail-safe. The confirm no longer swallows a preview error as "0 subtasks". On preview failure it falls back to an uncertain note — "Its linked subtasks, if any, will be kept and moved to Archived." — so it never silently hides that subtasks may survive. The delete still proceeds (a transient preview hiccup shouldn't block deletion), and the toast reports the Host's executed count afterwards. ③ Bulk purge. ② Plan-token binding / re-confirm — I'd argue against it here. The confirm is a best-effort heads-up before an action that, for the subtasks, is non-destructive — they're kept and archived, not deleted. The authoritative account is the post-delete toast, which reports the Host's executed count and is correct by construction. So even in the race you describe (a child unarchived between preview and remove), the user is still told the true number immediately after — the outcome isn't hidden, only the pre-confirm estimate is momentarily stale. With ① in place, the "preview unavailable" sub-case is also covered. A plan-digest token + a new 简体中文最新 head( ① 预检 fail-open → fail-safe。 确认弹窗不再把预检错误吞成「0 个子任务」。预检失败时改为显示不确定提示——「其链接的子任务(如有)不会被删除,将保留并移入归档。」——绝不静默隐藏子任务可能存活这件事。删除照常进行(一次瞬时预检抖动不该阻塞删除),事后 toast 仍报 Host 执行后的真实数。 ③ 批量 purge。 ② plan-token 绑定 / 重确认——这里我倾向不做。 确认弹窗本质是事前提醒,而对子任务来说这个动作是非破坏性的——它们被保留归档、不是删除。权威账目是删除后的 toast,报的是 Host 执行后的真实数、天然正确。所以即便发生你说的竞态(预检到执行之间某个子任务被 unarchive),用户事后仍会立刻看到真实数字——结果没被隐藏,只是确认前的估算短暂过期。加上 ① 之后,「预检不可用」这个子情况也覆盖了。为一个已有事后安全网的窄窗口、而且是关于「会存活的任务」的提示,引入 plan 摘要 token + 新的 |
142794d to
c56d32fCompare2313f20 to
a3f535dCompare848c98a to
1cf21b6Compare1e4e0ba to
b82cc06Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for moving the executed count and the delete preview to the Runtime Host instead of keeping a renderer-side copy of the removal plan. I found two bounded cases where the confirmation can still promise something different from the Host operation below. These are suggestions rather than a Request Changes review; please push back if I have missed a stronger admission or product wording invariant.\n\nAI-assisted review disclosure: Codex ran independent production/concurrency, UI, and test-quality lanes; Astro-Han independently verified the exact-head Host plan, renderer paths, CI, and severity and owns this review.
| // reports the real executed count afterwards. | ||
| let previewSubtaskCount: number | undefined; | ||
| try { | ||
| previewSubtaskCount = await service.previewRemoval(sessionId); |
There was a problem hiding this comment.
Thanks for making the preview Host-owned. [P2] (category ② — supported concurrent restore/create path): the preview and removal still do not bind the same plan. After this read returns 0, another client can restore or create an ordinary linked subtask before the user confirms; remove then recomputes a stable plan and archives it, but the confirmation showed no survival warning. The completion toast reports the fact only after the destructive choice, so it cannot replace the promised pre-delete notice. Could the preview return a plan/revision token that removal validates, or otherwise make the confirmation wording remain true when the plan changes? Please push back if an admission lease actually spans this read through the user decision.
| count === 1 ? 'Delete the 1 task you searched for?' : `Delete the ${count} tasks you searched for?`, | ||
| purgeConfirmBody: | ||
| 'The tasks and all of their messages are removed permanently. This cannot be undone.', | ||
| purgeSubtaskNote: 'Any linked subtasks are kept and moved to Archived.', |
There was a problem hiding this comment.
Thanks for covering the bulk-cleanup path. [P2] (category ① — normal archived parent with Agent Graph children): this says every linked subtask is kept, while the Host removal family intentionally includes graph-operator Sessions in removeSessionIds and excludes them from archiveSessionIds; a graph-only parent therefore deletes those children and archives zero. The same over-certainty appears in deleteSubtaskNoteUncertain, which says subtasks “will be kept” even when the preview failed and the Host may later classify them as graph operators. Could the copy say that ordinary subagent subtasks may be kept/moved (or use Host previews for the actual targets) rather than promising survival for every linked child? Please push back if graph operators are deliberately not presented as linked subtasks on either surface.
b82cc06 to
e6eb63bCompareAddresses review on apache#3781: the delete/purge confirmations promised that every linked subtask survives, but the Host retires Agent Graph operator Sessions with the parent (they are in the removal family, not the archive family), so a graph-only parent deletes those children and archives zero. The bulk purgeSubtaskNote and the preview-failure deleteSubtaskNoteUncertain were the over-certain surfaces; deleteSubtaskNote shares the same wording, so all three now say "ordinary subtasks" rather than "linked subtasks". The exact-count case is unaffected (the Host preview already counts only ordinary archivable subtasks, and a graph-only parent previews zero, hiding the note). Generated-by: Claude Code
liuxiaocs7
commented
Aug 31, 2026
Thanks — both are fair, and the framing as suggestions is appreciated. Fixed ① on ① Over-certain copy — fixed. You're right that a graph-only parent deletes its operator children (they're in the removal family, excluded from the archive family) and archives zero, so "every linked subtask is kept" was wrong. All three kept-subtask notices — ② Preview↔removal plan binding — I'd still argue against it. You're right that no admission lease spans the preview read through the user's decision — the preview is a plain read, and Where I land differently is on proportionality:
So I'd prefer to leave the pre-notice best-effort rather than bind a token. If you or a maintainer feel the pre-delete promise must hold exactly, I'm happy to file a follow-up for the token-bound preview rather than fold it into this PR. Your call — I won't treat this as blocking unless you'd like it to be. 简体中文多谢——两条都合理,也感谢标为建议。① 已在 ① 文案过度承诺——已修。 你说得对:只有 graph 算子子任务的父任务会删掉这些算子(它们在移除族里、被排除出归档族),归档数为 0,所以「所有链接子任务都会保留」是错的。三处「保留子任务」文案—— ② preview 与 remove 的 plan 绑定——我仍倾向不做。 你说得对,没有 admission lease 覆盖「预检读 → 用户决定 → remove」这整段;预检只是普通读,remove 在 admission gate 下重算自己的稳定 plan。所以竞态是真的:预检到确认之间,别的客户端可能新建/恢复一个普通子任务,确认弹窗没警告而 remove 归档了它。 我判断不同的地方在性价比:
所以我倾向让事前提示保持 best-effort,而不绑定 token。如果你或 maintainer 认为这个事前承诺必须严格成立,我乐意开一个 follow-up 单独做 token 绑定的预检,而不是塞进本 PR。你定——除非你希望它阻塞,否则我不当作阻塞项。 |
…tasks Deleting a parent task permanently removes it, but its ordinary linked subagent subtasks are intentionally kept and moved to the archive rather than cascade-deleted (apache#1467 / apache#3151). The confirm said nothing and the completion gave no feedback, so the archived rows that appeared next read as tasks from nowhere. The delete confirm now warns, when the task has linked subtasks, that they are kept and moved to Archived; the completion toast reports how many moved. Both counts are owned by the Host, not estimated by the renderer, whose catalog projection carries linked children as `subagent: { parentSessionId }` with no operator marker or copy state: - `session.remove` returns `archivedSubtaskCount` (deduplicated by revision family), which the toast reports verbatim. - A read-only `session.remove.preview` query returns how many subtasks a delete would archive; the confirm warns off it. Graph operators (retired with the parent), already-archived children, copies mid-preparation, and absent targets all preview zero. On preview failure the confirm falls back to an uncertain note rather than hiding the warning; the delete still proceeds. - Bulk purge sums the executed counts into SessionPurgeOutcome.archivedSubtasks; the archived-tasks purge confirm warns and its toast reports how many moved. Copy lives in shell-copy.ts / settings-tasks-copy.ts (zh + en). Deletion semantics are unchanged. Bumps RUNTIME_HOST_COMPATIBILITY_EPOCH for the new removed-result field and the new query. Fixesapache#3780 Generated-by: Claude Code
Addresses review on apache#3781: the delete/purge confirmations promised that every linked subtask survives, but the Host retires Agent Graph operator Sessions with the parent (they are in the removal family, not the archive family), so a graph-only parent deletes those children and archives zero. The bulk purgeSubtaskNote and the preview-failure deleteSubtaskNoteUncertain were the over-certain surfaces; deleteSubtaskNote shares the same wording, so all three now say "ordinary subtasks" rather than "linked subtasks". The exact-count case is unaffected (the Host preview already counts only ordinary archivable subtasks, and a graph-only parent previews zero, hiding the note). Generated-by: Claude Code
d9b38bc to
a6a6c95Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Both rounds land. Reviewed at head a6a6c957, test and windows_recovery green.
① Over-certain copy — verified fixed. I checked that the new wording actually matches Host behavior rather than just reading better. #readRemovalPlanSessionIds filters subagentParent.graph === undefined and conversationCopy?.state !== 'preparing', and archivableSubtaskCount is derived from archiveHeaders — the very set that gets archived — deduplicated by revision family. So "ordinary subtasks" is exactly what the preview counts, and a graph-only parent previews zero and shows no note at all. This also closes out my first-round point: the renderer no longer re-derives the plan anywhere, so there is one owner for it.
② Preview/removal plan binding — withdrawn, your argument holds. You are right and I graded this too high. The race is real and you conceded it plainly, but severity should come from consequence and recoverability, not from the race existing. The action is non-destructive for the subtasks, the outcome is reported truthfully by the toast immediately after, and closing it would cost a plan-digest token, a plan_changed rejection, and a renderer re-confirm loop. Best-effort pre-notice with an authoritative post-report is a coherent contract, not a defect. No follow-up needed from my side.
Three optional suggestions, none blocking:
protocol/session-retirement.ts— theremovedresult decoder was loosened fromrequireExactRecordtorequireShapedRecordto admit an optionalarchivedSubtaskCount. Epoch 82 already excludes older peers, so the field could be required and always present; that keeps the strict decoder, drops the?? 0in the client and the conditional spread in the codec, and removes "absent means zero" as a third state alongside0.session-row-actions.ts— the localSessionRemoveOutcomeduplicatesSessionNavigationRemoveOutcomefromports.tsin the same feature directory, comment included. Import the port type instead.- The single-delete toast's
deletedSubtaskNote(count)restates the confirm the user just read, and no decision follows it. Worth considering dropping it there. The bulk purge case is different and should keep its count, since that confirm has no preview and the toast is the only quantitative signal.
One line worth naming rather than letting it pass in the diff: this adds session.remove.preview to REMOTE_OWNER_OPERATION_GRANTS, which is a deliberate security-policy change, not a registration chore. I read it as correct — session.remove is already granted and the preview is a strictly weaker read of the same plan, so there is no privilege expansion — but it deserves to be seen.
Separately, the nine-layer cost of adding one Host operation prompted #4395. Almost all of it is earned (separate process, contextIsolation, the ports test seam, the fail-closed grant list); the one part that is not is a duplicated grouping in operation-dispatcher.ts that predates this PR. Nothing for you to do here.
AI use: Claude Code traced the information flow from #readRemovalPlanSessionIds through the protocol, IPC, preload, ports, and controller, and drafted this review. I verified the count derivation, the epoch, the copy/behavior match, and the remote grant myself, and I own this approval.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Deleting a parent task permanently removes it, but its ordinary linked subagent subtasks are intentionally kept and moved to the archive rather than cascade-deleted (#1467 / #3151 — each subtask owns its own transcript, artifacts, and outputs). The delete confirm never said so and the completion gave no feedback, so the archived rows that appeared next — labelled
原父任务已删除on Settings › 活动 › 已归档任务 — read as tasks from nowhere.This adds two notices, without touching deletion semantics:
The confirm's decision to warn is a renderer presence check off the live catalog (
parentHasArchivableSubtasksinapp-shell-session-row-actions.ts): ordinary subagents count, Agent Graph operators (subagentParent.graph) do not, and already-archived children do not. The exact number is owned by the Host:session.removenow returnsarchivedSubtaskCount(present only when it archived something, deduplicated by revision family insession-retirement-coordinator.ts), threaded throughremoveSession, thesessions:removeIPC, and the preload bridge to the toast. A restore that calls the delete off shows no subtask notice, since nothing moved. Copy lives inshell-copy.ts(zh + en).Fixes#3780
Review follow-up (superseding an earlier revision of this PR)
The first version computed the count in the renderer (
countArchivableSubtasks) as a second implementation of the Host's removal plan. As raised in review, that estimate diverged from the Host — theconversationCopy?.state !== 'preparing'filter is not implementable offSessionSummary, and the parent-link fallback read a field the Host ignores — so it could name a wrong number. The count now comes from the Host, and the confirm names no number at all.Verification
biome linton the 14 changed files — clean.tsc -p tsconfig.main.json,tsconfig.renderer.json,tsconfig.preload.json(--noEmit) — all pass;@maka/runtime-hostbuild — clean.session-retirement-protocol.test.ts—archivedSubtaskCountround-trips on a removed result, is absent when nothing was archived, and a negative value is rejected.session-retirement-coordinator.test.ts— a parent with 32 distinct subagent children reportsarchivedSubtaskCount: 32; graph-operator and already-archived cases stay countless.runtime-host-client-operations.test.ts/runtime-host-client-uds.test.ts—removeSession/ thesessions:removeIPC surface the count verbatim ({ disposition, archivedSubtaskCount }).app-shell-session-row-actions-revisions.test.ts— the confirm warns without a number when the family has archivable subtasks; the toast reports the Host's count; graph operators / archived children / other parents raise no notice; a concurrent restore leaves the toast silent.parent-session-deletion.spec.ts— asserts the confirm warns about the subtask (no count); not run in this environment (needs the Electron build + Playwright). Run withnpm --workspace @maka/desktop run e2e -- parent-session-deletion.Screenshots
Delete confirm — warns the linked subtasks are kept and moved to Archived (no count named):
Completion toast — reports how many subtasks moved to Archived (now the Host's executed count):
Same two shots as generated by the e2e run (raw copies, in case the attachments above ever fail to load):
AI use
Select exactly one:
Tool(s) and scope: Claude Code — traced the delete/archive path, wrote the copy, the
parentHasArchivableSubtaskspresence check, thearchivedSubtaskCountwiring from the Host coordinator through the protocol, client, IPC, and preload to the toast, and the unit/e2e coverage. Commits carry aGenerated-by: Claude Codetrailer.Checklist
Does this PR entail a change in behavior?