fix(desktop): clarify delivery states and recover failed messages

[jackeyfaker77]

Summary

Failed local messages now offer Edit and resend and Delete failed message. Editing restores the original text and attachment data into an empty composer, protects drafts created during the read, and keeps the failed copy until explicitly deleted. Unknown delivery outcomes retain the original message identity for reconciliation.

Delivery labels distinguish waiting, sending, failure, checking, live queue placement and processing. Errors and recovery actions stay with their message, and local rows share the canonical conversation column. Presentation refreshes no longer recreate queued messages retracted by Stop.

Fixes #5010

Verification

• Passed: 37 focused desktop tests, 448 shared UI tests, build, desktop/UI typechecks and Knip, source-scoped lint/format, renderer architecture comparison, E2E budget and diff checks.
• The repository recovery E2E passed, including durable attachment recovery, draft protection, resend, deletion and Stop/next-send regression coverage.
• Additional direct Electron acceptance: 4/4 passed, with no renderer page errors. Covered duplicate clicks, task switches during recovery, deletion during a reply, unknown-outcome identity reconciliation, renderer reload and full application restart. Used a deterministic fake model backend and controlled failures; the native file picker was stubbed.
• Independent clean-build desktop comparison: baseline 2475 passed / 24 failed / 8 skipped; candidate 2482 passed / 23 failed / 8 skipped. All 23 shared failures have matching normalized errors (Windows shell/path fixtures, symlink permissions and SQLite cleanup). Six added tests passed; the remaining fail-to-pass change limits an existing POSIX permission assertion to non-Windows platforms. These failures are outside this issue's scope.
• That comparison preceded the final renderer Stop fix; the Stop regression was separately reproduced against baseline and candidate, then verified with the official recovery E2E and complete Electron acceptance after fixing it.
• Root lint/format encounter an unrelated nested Biome configuration under untracked tmp/; source-scoped checks passed. The complete repository test suite was not run.

Screenshots

Before — waiting to send

After — waiting to send

Failed-message recovery with attachment

Stop and subsequent send

Checklist

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

The second checkbox remains unchecked because the full desktop suite has the baseline failures described above.

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

Generative tooling

OpenAI Codex contributed substantively to implementation, regression tests, and review/validation assistance for this PR.

[hqhq1025]

Technical NO-GO on exact head 24219c259697474a9092948d605b229bee45173f: two current-head P2 regressions remain in failed-message recovery and Stop retraction persistence. No other P0-P3 findings were identified in the reviewed scope.\n\nValidation completed: build:test; UI 420/420; Desktop 2367/2367; focused local-message tests 37/37; renderer architecture 101/101; E2E budget 38/38; recovery Electron E2E 4/4 under Xvfb; Desktop typecheck; Biome; repository format check; ASF headers; git diff --check. Hosted test and label checks are green, and the head cleanly merges with current main 9cb5cc93c03f8d07550e739164d5e33168892563.\n\n> Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P2] Preserve one-shot orchestration when restoring a failed draft\n\nreadFailedMessage() reconstructs the editable content but drops command.turnOrchestration. For a normal /swarm audit repository send, the production composer removes /swarm before enqueueing, stores text = "audit repository", and records { mode: "swarm", source: "slash_command" } separately. If that intent definitively fails, Edit therefore restores only the task tail; pressing Send follows the current/default mode and silently changes the requested execution. A probe through the compiled DesktopSessionLocalStore and this service reproduced the lost field on this head. Please preserve or reconstruct the one-shot /swarm and /graph intent and add a regression through the actual slash-command send/edit/resend path.

[jackeyfaker77]

Fixed in 70f5c05. Failed-draft recovery now reconstructs the original /swarm or /graph command before skill tokens and rebases inline references accordingly. Added regression coverage through the actual composer send/edit/resend path to verify that the original orchestration mode is preserved.

[hqhq1025]

[P2] Keep Stop retractions durable across navigation and restart\n\nThis suppression set is reset whenever the session changes and is recreated on renderer restart. A Host message_admission: retracted event only removes the renderer-owned transient row; the accepted outbox record remains in SQLite until a canonical transcript observes it. Returning to the session or restarting while the Host is offline makes listMessages() return that record again, so this branch calls publish and resurrects the cancelled follow-up as “Delivered for a later reply”. The cancellation proof query cannot repair it while disconnected (and intentionally leaves presentation intact on failure). Persist/retire the retraction in the local store, or retain an equivalent durable tombstone, and cover Stop followed by session switch/restart with the Host unavailable.

[jackeyfaker77]

Fixed in 70f5c05, with cross-Host-restart handling added in b9dd787. Confirmed cancellations now retire the local SQLite outbox records instead of relying only on renderer suppression. Durable Host cancellation proofs can also reconcile records from an earlier Host epoch while preserving authority and session boundaries.

Added coverage for offline recovery, a real Host restart with missed local cleanup, and late submit acknowledgements.

[hqhq1025]

One additional current-head P3 found while reconciling the attachment recovery path. The two P2 findings in review 5147194929 remain the readiness blockers.\n\n> Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P3] Preserve filename-based attachment classification on recovery\n\nThe normal staging paths call attachmentKindFromMimeType(mimeType, file.name), but recovered staged bytes omit item.name. That changes reachable attachment kinds after Edit: for example, text/plain + handler.ts restores as other instead of code, and application/octet-stream + report.docx restores as other instead of doc, so the composer and resent message use the generic attachment presentation. The new E2E uses recovery.txt and only checks the name, so it cannot catch this. Pass item.name here and add an extension-driven recovery case.

[jackeyfaker77]

Fixed in 70f5c05. Recovery now passes item.name to attachmentKindFromMimeType, preserving extension-based attachment classification.

[hqhq1025]

Technical NO-GO on exact head 2a84181144e1f34d449c92161bd3d73e1a170a9b: the prior orchestration-recovery and attachment-classification findings are fixed, and the previous cancellation presentation issue no longer republishes accepted queue rows. One P2 remains in the durable Stop cleanup across a Host restart.\n\nValidation completed: build:test; UI 432/432; Desktop 2453/2453; focused changed-path tests 151/151; renderer architecture 112/112; recovery Electron E2E 6/6 under Xvfb; Desktop and UI typecheck; Biome lint/format; ASF headers; E2E budget; git diff --check. Hosted test is green. The head is currently conflicting with main 4cd71eaed26dbf296f1142db3296b27136d7d131 in three files, so it also needs a freshness update before merge.\n\n> Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P2] Retire a stopped queue entry across Host epochs\n\nAfter a Host restart, an accepted local outbox row still carries the epoch that originally admitted it, but this Stop callback always supplies the current client.hostEpoch. DesktopSessionLocalStore.retireRetractedMessages() then rejects the deletion when those epochs differ. A probe through the compiled store/service and the registered production sessions:stop IPC retracted 256 old-epoch queue entries successfully under a new Host epoch; all 256 remained accepted after reopening the database, and the next send failed with Local message storage is full. The observer-side durable query is asynchronous and cannot make the successful Stop transaction reliable; the new E2E also invokes queryCancelledMessages() manually rather than exercising this normal Stop path. Please retire the exact message identities returned by a successful retraction without comparing them to the current Host epoch (while keeping the active-target fence), and add a regression for an old-epoch accepted row stopped after Host restart.

[jackeyfaker77]

Fixed the remaining cross-epoch Stop cleanup issue in 5846f4b27.

The normal sessions:stop path now durably retires exactly the message IDs confirmed as retracted, without comparing their original dispatch epoch with the current Host epoch. The owner/active-target fence and authority/session boundaries remain intact.

Added regression coverage through the registered production IPC with a full local outbox containing old-epoch records. It verifies cleanup before Stop returns, persistence after reopening SQLite, preservation of unrelated records, and a successful subsequent send. This does not manually invoke queryCancelledMessages() or depend on observer events. Rejected Stop and inactive-target cases are also covered.

The subsequent pushes additionally:

• Synced with upstream 5263fb78a and resolved the conflicts. Recovery orchestration now stays within the conversation feature boundary.
• Hardened draft recovery against newer drafts and pending Session references, and prevented local rows from reappearing after Host queue handoff.
• Corrected started-Turn placement, disabled recovery actions, and visible feedback with narrow-pane wrapping.
• Updated E2E synchronization for actual Host readiness and persisted reply content, without increasing timeouts, adding retries, or skipping tests.

Current head: 9238cf06a. The complete CI run is green, including strict architecture checks, affected workspace tests, desktop E2E, browser/WorkHub smoke, Storybook smoke, and transcript geometry checks. GitHub also reports no merge conflicts.

[hqhq1025]

Technical NO-GO on exact head 9238cf06abae35d2803480a14bc56806c66ca12b: two P2 findings remain. The inline finding covers a reachable failed-draft recovery race that can overwrite a newer attachment-only draft and merge its context with the failed message. Separately, the PR description does not state whether generative tooling contributed or name the tool, although the first five material commits carry Generated-by: OpenAI Codex; CONTRIBUTING.md:36 requires that declaration in each pull request. Please update the PR description.\n\nThe prior old-Host-epoch Stop cleanup defect is fixed: successful queue retraction and Turn interruption now retire exact dispatched message identities without requiring the current Host epoch, while authority/session boundaries and never-dispatched records remain protected. The production candidate/IPC/store path and offline restart regression pass.\n\nValidation completed on this head: clean install; build:test; Desktop 2657/2657; UI 620/620; focused recovery/observer/IPC tests 201/201; recovery Electron E2E 6/6 under Xvfb; full typecheck; lint; format; ASF headers; renderer architecture 112/112; E2E budget 38; git diff --check. Hosted test is green. The PR is 14 commits ahead and 62 behind current main 8d5a3cac3504c36cb1849b060153b58188e2c7b2, with merge conflicts in seven Desktop files, so it also needs rebasing and re-review before merge. Native Windows/macOS recovery was not exercised.\n\n> Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

[P2] Make this pending-context check read live state. hasPendingContext is a boolean captured by the last render. If a file selection finishes after the first guard but before readFailedMessage() resolves, setPendingState() can be queued without a render; this second guard still reads false, and restoreDraft() overwrites the composer text and appends the failed message context to the newly selected file. On this exact head, a hook-level probe using the production useComposerAttachments and recovery helper staged new.txt and resolved the failed draft in one React batch; canRestoreDraft() returned true, the text became the old failed draft, and pending attachments became [new.txt, old.txt]. The new race tests cover same-tick Session references via a live ref and typed text, but not attachment staging. Please expose a live pending-context getter/ref and add this regression.

[hqhq1025]

Technical NO-GO on exact head 98a720c668784a5b7fbc08c14dd7bc6436acbdd6: two P1 and two P2 findings remain. The previous same-batch attachment/directory recovery race and the generative-tooling disclosure gap are fixed, and the old cross-Host Stop cleanup regression remains fixed.

Validation completed on this head: clean install; build:test; Desktop 2776/2776; UI 629/629; focused recovery/observer/IPC tests 211/211; recovery Electron E2E 6/6 under Xvfb; full typecheck; lint; format; ASF headers; renderer architecture 112/112; E2E budget; git diff --check; and a clean merge tree against current main 714b0b687fedf931e885cf4e681dc3b101471289. Storybook built successfully, but the required hosted test job and an isolated local smoke both fail the same prompt-sent-before-turn-lands assertion. The PR is 15 commits ahead and 1 behind main.

I did not test native Windows/macOS recovery or a packaged production build.

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

[hqhq1025]

[P2] Do not discard a positive not_admitted execution result

The observer maps both cancelled and not_admitted to a retraction, but this callback then re-queries queryMessages(), which deliberately returns only cancellation tombstones. A not_admitted result therefore becomes an empty ID list and the local accepted row is never retired or settled. A real store/service probe through the observer path produced one execution query and one cancellation query, then still had durableState: accepted with its staged bytes intact. Since accepted rows are skipped by the delivery scheduler, this record can survive restart and later reappear without converging. Preserve the resolution kind in this callback and settle not_admitted explicitly (or retire it under an equivalent durable contract), with a regression that checks the SQLite row after observation reconciliation.

[hqhq1025]

Correction to my review #5348971164 at this same head: I withdraw its "no substantiated P0–P3" conclusion. The P2 reported in review #5349054273 is supported by the code: session-local-messages.tsx offers Delete for a paused original without clearing the composer replacement link; composer.tsx retains that link after a failed send; session-local-store.ts rejects a replacement when the original is no longer paused. The edited draft is therefore stuck after Delete. I did not run a full UI reproduction. The existing P2 review has the detailed finding; this is a correction to my earlier assessment, not an additional issue.\n\n> Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[jackeyfaker77]

Thanks @Astro-Han and @hqhq1025. I addressed the delete/edit issue and the downgrade risk in two separate commits:

• b3fd1d9 — Protect paused originals during editing. Delete now checks the live composer draft and staged context before calling Main, using the same guard as Continue sending. If content remains, deletion is blocked with guidance; the edited draft can still be sent. Main’s strict replacement validation remains unchanged. New regressions cover text + attachment and attachment-only edits, blocked deletion followed by successful replacement, and deletion after clearing the draft/context.

• 4beb7a8 — Prevent automatic dispatch after downgrade. Paused records now use failed in the SQLite state column while retaining paused in the payload. Older readers skip them; current readers preserve paused semantics. Opening the database also converts existing paused rows without changing message content or attachments. This protection requires opening the database with the fixed version before downgrading; older UI may show the message as failed.

For the remaining P3 observations:

• Ordering after editing: retained as an intentional behavior. A paused local message does not block later messages, and sending the edited draft creates a new queue entry. Editing therefore does not preserve the original queue position. Preserving its timestamp alone would not undo later messages already dispatched while it was paused.
• Paused after a refused restore: also retained intentionally. Clicking Edit expresses an intent to stop sending the original unchanged. If restoration is refused or the user navigates away, we preserve the original in a visible paused state and require an explicit Continue sending action, rather than automatically resuming it.

Validation: 88 related tests passed for the delete protection; 101 passed after the downgrade fix. Compatibility probes using the actual store/service implementations from 96741deb, 2f3220552, and b3fd1d916 confirmed that ordinary messages still dispatch, paused originals do not, and reopening with the current implementation preserves recovery and explicit resumption. Desktop type checks, lint, formatting, and independent agent review also passed.

These are targeted service/React and compatibility checks, not packaged Electron or full application downgrade E2E validation. The hosted CI checks for 4beb7a85e also passed.

中文对照

感谢 @Astro-Han 和 @hqhq1025。我通过两个独立提交处理了删除编辑稿的问题和降级风险：

• b3fd1d9 — 编辑期间保护暂停原件。 删除操作现在会在调用 Main 前检查输入框的实时草稿和暂存上下文，与“继续发送”使用相同的保护条件。如果仍有内容，就阻止删除并显示提示；编辑稿仍可正常发送。Main 的严格替换校验保持不变。新增回归覆盖正文加附件、仅附件、删除被阻止后成功替换，以及清空草稿和上下文后允许删除。

• 4beb7a8 — 防止降级后自动发送。 暂停记录现在在 SQLite 状态列中存为 failed，同时在记录内容中保留 paused。旧版本读取后会跳过自动发送，新版本仍按暂停状态处理。打开数据库时也会转换已有暂停记录，不改变消息内容或附件。此保护要求先用修复版打开数据库，再降级；旧版界面可能将消息显示为失败。

对于剩余两个 P3：

• 编辑后的顺序变化： 这是有意保留的行为。暂停的本地消息不阻塞后续消息，发送编辑稿时会创建新的队列条目，因此编辑不保证保留原队列位置。仅保留原时间戳，也无法改变暂停期间后续消息已经发出的事实。
• 恢复被拒绝后仍暂停： 同样是有意保留的行为。点击“编辑”表示用户不希望原消息继续按原样发送。如果恢复被拒绝或用户切换会话，我们会保留原件及可见的暂停状态，要求用户明确点击“继续发送”，而不会自动恢复发送。

验证方面：删除保护的相关测试 88 项通过，降级修复后 101 项通过。使用 96741deb、2f3220552 和 b3fd1d916 的真实存储及调度实现进行兼容性实验，确认普通消息仍正常发送、暂停原件不会自动发送，重新使用新版本打开后仍可恢复编辑和显式继续发送。Desktop 类型检查、lint、格式检查和独立 agent 复审也均通过。

这些验证属于定向服务、React 和兼容性检查，不代表已完成打包版 Electron 或完整应用降级的端到端验证。4beb7a85e 的远端 CI 检查也已通过。

[Astro-Han]

Re-reviewed exact head 4beb7a85eb58389f908f22cf083dea2c3f401cef against my earlier review on 0d7f2b7c.

The earlier P2 is fixed. You can no longer delete a paused original while the composer still holds content that might be linked to it:

• session-local-messages.tsx:122-127 refuses Delete on a paused row unless canRestoreDraft() holds and there are no pending references. This is the same guard Continue sending uses.
• use-composer-draft.ts:184-187 defines when the draft can no longer link to the original.
• The strict missing-original check in session-local-store.ts is unchanged, which is right because it prevents duplicates after a lost ack.

The new tests in message-queue-ui-state.test.ts:176-243 cover:

• Delete blocked, then send, for a text+attachment edit and for an attachment-only edit.
• Delete after the draft is cleared.

With the guard disabled in the compiled output, those tests fail. With it restored, 24/24 pass.

The earlier downgrade P3 is also fixed. Paused rows are stored as failed in the SQLite state column, with paused kept in the record, so older builds skip them and the new code reads them back as paused. The migration runs once and is safe to repeat. Removing the mapping makes the new test fail. The two other P3s (queue position after an edit, and a refused restore staying paused) are intentional per your explanation, and I agree.

New P3 (inline, UX only): the Delete guard also blocks when the composer is disabled, or when the draft has unrelated text after a restart. In those cases the "clear the draft… send the draft instead" message is misleading. It errs on the safe side and no data is lost.

Checks run:

• check:asf-headers: passes.
• check:app-shell-hooks: passes.
• git diff --check: clean.
• Targeted Desktop tests: 184/184 and 59/59.
• Composer UI tests: 30/30.

Not run or verified:

• Electron end-to-end tests.
• Multi-window behaviour. The guard only sees the draft in the same window.
• A real downgrade to a shipped older build.

Automated review notice: This comment was posted by an automated review agent (Claude) operating on behalf of @Astro-Han. It is not an independent human review and does not replace one.

[Astro-Han]

P3 (UX): this also blocks Delete when the composer is disabled (e.g. while editing a sent turn), and when the draft holds text unrelated to this message (e.g. after a restart drops the replacement link). In those cases the "clear the draft… send the draft instead" hint is misleading, because sending creates a new message rather than replacing this one. It's safe as-is. Could the guard check for an actual replacement link to this message, or could the copy be adjusted?

[hqhq1025]

Reviewed exact head 4beb7a85eb58389f908f22cf083dea2c3f401cef. I found no remaining P0-P2 in the two-commit follow-up.

The earlier delete/data-loss issue is fixed: session-local-messages.tsx:122-127 now checks the live composer state before deleting a paused original, so text+attachment and attachment-only edits keep the durable source needed for atomic replacement. The replacement ownership is cleared only when both text and pending context are gone (use-composer-draft.ts:107-113,180-185).

The downgrade auto-dispatch issue is also fixed. session-local-store.ts:69-77,123-125,275-286 stores semantic paused as legacy-visible failed in the SQLite column while retaining paused in the payload, and migrates the earlier raw encoding without deleting content or attachment rows.

One P3 UX issue remains: the Delete guard only knows whether the composer is empty/enabled, not whether it actually owns a replacement for this paused message. Unrelated draft content or a disabled composer therefore blocks Delete too, while the message says sending that draft will preserve the edit. This is conservative and does not lose data, but the action/copy is misleading outside the actual replacement case.

The new regressions discriminate the prior failures: removing the Delete guard made all three focused delete tests fail, and restoring the raw paused column write made the legacy-reader test fail. After restoring the exact head, build:test, Desktop typecheck, 93 focused Desktop/UI tests, lint, format, ASF headers, renderer architecture (112/112), E2E budget, and git diff --check passed. The hosted test check is green.

This head is not merge-ready against current main 53f566b41f8b3c540b2e8717e0c324f46e002368: git merge-tree reports content conflicts in apps/desktop/renderer-architecture.json and apps/desktop/src/renderer/app-shell.tsx. The resolved head needs re-review. I did not run packaged Electron, native Windows/macOS, multi-window behavior, or an actual older Desktop binary against the migrated database.

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

[jackeyfaker77]

Thanks @Astro-Han and @hqhq1025 for re-reviewing 4beb7a85e. Updated to e9107b87f with two follow-up commits:

• 3223af1ef — Resolve the conflicts with main.
  Merged main 53f566b41, preserving its extracted submit controller and unchanged-text edit-and-resend fix. The original-message replacement ID and attachment retention were carried into the new controller. The existing Delete protection, strict replacement validation, and downgrade protection remain intact.

• e9107b87f — Clarify the paused-message Delete hint.
  Updated English, Simplified Chinese, and Traditional Chinese copy. The hint now asks users to retry when the composer is available and has no draft, attachments, or references. It no longer suggests that sending an unrelated draft will replace the paused message.

For the two suggested approaches:

• Check the actual replacement link: this would allow deleting a paused message while an unrelated draft is present, but would change the guard’s behavior and require validating ownership across draft changes and in-flight replacement submissions.
• Adjust the copy: this preserves the conservative protection while removing the misleading guidance. I chose this option to keep the follow-up within the agreed scope. Unrelated drafts or an unavailable composer can still block Delete.

Validation: 122 targeted tests passed after conflict resolution. After the copy change and corresponding assertion update, all 24 message-queue UI-state tests passed, retaining the blocked-delete, attachment preservation, and successful replacement checks. Desktop Main compilation, scoped lint/format checks, and git diff --check passed. Independent agent review found no remaining blockers, and hosted CI for e9107b87f is green.

These checks do not establish packaged Electron, multi-window, or actual older Desktop binary compatibility; those were not revalidated in this follow-up. Please re-review the updated head when convenient.

中文对照

感谢 @Astro-Han 和 @hqh1025 对 4beb7a85e 的再次审查。现已通过两个后续提交更新至 e9107b87f：

• 3223af1ef — 解决与 main 的冲突。
  合并 main 53f566b41，保留其抽取后的发送控制器，以及“原文不变也能编辑后重发”的修复。原消息替换 ID 和附件保留逻辑已迁入新控制器。既有删除保护、严格替换校验及降级保护保持完整。

• e9107b87f — 明确暂停消息的删除提示。
  更新英文、简体中文和繁体中文文案。新提示要求在输入框可用，且没有草稿、附件或引用时重试，不再暗示发送无关草稿能够替换这条暂停消息。

对于提出的两种方案：

• 检查实际替换关联： 可以允许在输入框存在无关草稿时删除暂停消息，但会改变保护条件，需要验证草稿变化及发送中替换请求的归属。
• 调整文案： 保留保守保护，同时消除误导。为了将后续修复控制在已确认的范围内，我选择了此方案。无关草稿或输入框不可用时，仍可能阻止删除。

验证方面：解决冲突后，122 项定向测试通过。更新文案及对应断言后，消息队列 UI 状态测试全部 24 项通过，保留了删除被阻止、附件保留及成功替换的检查。Desktop Main 编译、涉及文件的 lint/格式检查和 git diff --check 通过。独立 agent 复审未发现剩余阻塞，e9107b87f 的远端 CI 也已通过。

这些检查不代表已验证打包版 Electron、多窗口或实际旧版 Desktop 二进制兼容性；此次后续修复未重新验证这些场景。方便时请复核更新后的提交。

[hqhq1025]

Reviewed exact head e9107b87feaa3e65220f058740437e4af31fafeb. I found no remaining P0-P3 issue in the current-head changes.

The merge resolution keeps current main's feature-owned submit factory while preserving this PR's replacement identity and attachment lease across ordinary, /swarm, and /graph sends (apps/desktop/src/renderer/features/conversation/controller/composer-submit.ts:157-165,268-286,320-338,346-380; apps/desktop/src/renderer/app-shell.tsx:1376-1413). The previous P3 copy issue is fixed: deletion remains conservative, but the en/zh-CN/zh-TW text now accurately states that the composer must be available and free of drafts, attachments, or references (apps/desktop/src/renderer/locales/session-local-copy.ts:28,51,71; apps/desktop/src/main/__tests__/message-queue-ui-state.test.ts:214-216). The earlier paused-original deletion and downgrade protections remain intact.

The merge regression is discriminating: temporarily removing ordinary-send replacesLocalMessageId forwarding made submit preserves replacement and attachment ownership: edited message fail before send; restoring the exact head returned the suite to green.

Validation on Node 24.18.1: clean npm ci; build:test; Desktop 2986/2986; UI 713/713; focused recovery/submit tests 98/98; Desktop typecheck; lint; format; ASF headers; Windows test inventory; renderer architecture 112/112; E2E budget (37 tests); and git diff --check. The exact-head hosted test check is green. The PR is 26 commits ahead and 0 behind current main 53f566b41f8b3c540b2e8717e0c324f46e002368, and git merge-tree is clean. GitHub still reports BLOCKED, so branch protection or other repository requirements remain outside this code review.

I did not run packaged Electron, native Windows/macOS, multi-window behavior, or an actual older Desktop binary against the migrated database.

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

[Astro-Han]

Reviewed 86f9b2dadd55dd97665e57712ba90e25466c6fe8 (75 files, +5657/−432, 30 commits). This card was scoped to shape plus gate, with prior findings and the long-standing conflict checked, and big items reported back rather than deep-dived — so that is what this is.

No P0–P3 from my own pass, and one open finding I could not reproduce.

Shape. Only 11 of the 75 files are production; the rest are tests and e2e fixtures, which is where the +5657 mostly lives. The production surface is tightly scoped to the stated subject: session-local-attachment-recovery.ts (+153, new), session-local-service.ts (+183/−37), session-local-store.ts (+81/−9), runtime-host-session-observer.ts (+29/−3), runtime-host-desktop-candidate.ts (+27), small IPC/boot edits, one e2e spec (+314) and two ledger lines.

The conflict is genuinely resolved. main is an ancestor of this head — the history carries "Merge main and preserve recovery through composer staging owner" — and mergeable is true, so the long shelving is behind it.

The previous round's clean bill still stands at a comparable point. The newest prior review (e9107b87, which is an ancestor of this head) reported no remaining P0–P3 and described the merge resolution as faithful: current main's feature-owned submit factory kept, with this PR's replacement identity and attachment lease preserved across ordinary, /swarm and /graph sends. It also recorded the earlier copy P3 as fixed, with the delete text now accurate in all three locales.

The open P2 — I read the code and could not reproduce its premise. The inline on this head says the observer maps both cancelled and not_admitted to a retraction and that "this callback then re-queries", discarding a positive result. At the callback in runtime-host-desktop-candidate.ts the two proofs are separated and not_admitted returns before any query:

// Execution resolution is already positive Host proof. Preserve its
// kind: not_admitted has no cancellation tombstone to query, and its
// unsent content must remain recoverable instead of being discarded.
if (proof === 'not_admitted') {
  deps.failNotAdmittedMessages?.(scope, sessionId, messageIds);
  return;
}
if (proof === 'cancelled') { retireCancelledMessages(sessionId, messageIds); return; }
// Projection disappearance alone is not durable cancellation proof …

So at this layer not_admitted does not retire anything; it routes to failNotAdmittedMessages with a comment stating exactly why the content must stay recoverable. The observer does emit a retraction for both kinds — its own test asserts that — so the finding may be aimed one layer up. Either way, as written against this callback it does not hold, and since the dispatch asked me to report big items rather than settle them, I am flagging it rather than declaring it invalid: this is the one place in this PR that deserves a second opinion.

My line has an earlier P3 at session-local-messages.tsx:125 on this head as well — that the Delete guard also blocks Delete while the composer is disabled or holds an unrelated draft. It is a UX trade-off in a conservative guard, unchanged, and I am not re-grading it here.

Gate on this head: test is not finished (in_progress when I looked), so I make no CI claim; mergeable is true.

What I did not judge

• The internal correctness of the recovery path across the 11 production files; that is the deep pass this PR would need, and the dispatch deliberately reserved it.
• No Electron run; no reproduction of the recovery scenarios beyond the tests present.

I did not approve, request changes, or merge.

---

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

[jackeyfaker77]

Thanks @Astro-Han for the review. Following up on the open P2 and the gate status for exact head 86f9b2dadd55dd97665e57712ba90e25466c6fe8:

• The not_admitted finding was addressed earlier. The original inline was written against 98a720c6. The fix landed in cd56e5dfd, which is included in the current head. The observer preserves the resolution kind; the candidate routes not_admitted directly to failNotAdmittedMessages and returns before any cancellation query. The production boot wiring reaches the local service/store, which persists failed, clears the old result, and retains the message content and attachment bytes. The queue retraction therefore does not discard the durable recovery copy.

• The second check covered persistence as well as the callback. The existing candidate integration regression drives observation reconciliation through the real observer, candidate, local service and SQLite store. It asserts zero cancellation queries, then closes and reopens the database and checks the persisted failed state and retained attachment bytes. The targeted removed queue|not_admitted checks passed 13/13, including ownership boundaries, the no-Turn case and late completion fencing. This was a targeted verification of the open finding.

• The exact-head gate is now green. The hosted test job has completed successfully, and GitHub currently reports mergeable: true / clean.

The conservative Delete guard remains the previously documented UX choice; the corrected three-locale hint remains in place.

Could you confirm whether this evidence resolves the old not_admitted P2 on the current head?

中文对照

感谢这次审查。针对当前固定版本 86f9b2dadd55dd97665e57712ba90e25466c6fe8，补充旧 P2 和 CI 状态：

• not_admitted 问题此前已经修复。 原始 inline 针对的是 98a720c6，修复提交为 cd56e5dfd，已包含在当前版本中。observer 会保留实际证明类型；candidate 遇到 not_admitted 时直接调用 failNotAdmittedMessages，在查询取消记录之前返回。生产环境的 boot 接线会调用本地 service/store，将状态持久化为 failed、清除旧结果，同时保留消息内容和附件字节。因此，撤回队列展示不会丢弃可恢复的持久化原件。

• 此次复核也检查了持久化结果。 现有 candidate 集成回归通过真实 observer、candidate、本地 service 和 SQLite store 驱动观察状态协调，断言取消查询次数为零，并在关闭、重新打开数据库后检查 failed 状态和附件字节仍然保留。相关定向检查 13/13 通过，包含归属边界、没有 Turn 的情况及迟到完成结果的保护。本次属于对该未结案问题的定向复核。

• 当前固定版本的 CI 已通过。 上述 hosted test job 已完成且成功，GitHub 当前显示 mergeable: true / clean。

保守的 Delete 保护仍是此前说明过的 UX 取舍，三种语言的修正文案也保持有效。

请确认这些证据是否足以将当前版本上的旧 not_admitted P2 结案。

[jackeyfaker77]

Follow-up in 5ead6ce71: fixed a newly reproduced P2 in paused-message editing.

• Problem: the send boundary accepts 128,000 characters, while the ordinary per-session draft cache retains only the last 120,000. Editing a longer paused message, switching sessions and returning could truncate the draft while keeping its replacement ID. Sending then replaced the complete durable original with the shortened text.

• Scoped fix: drafts with an actual replacement link now retain their full body. Ownership is established before the first cache write, including recovery into an inactive session. Ordinary drafts keep their existing character limit, and all text entries still share the 32-entry eviction policy. Clearing, eviction and successful-send ownership cleanup remain covered. This follow-up changes two UI implementation files and two existing test files.

• Regression evidence: all three new tests failed against the previous code with 120000 !== 128000; they pass with the fix. They cover active/inactive restoration and the real Composer send. Additional assertions cover reference offsets, repeated append/save, ordinary draft limits and eviction. A Composer + recovery-service + SQLite probe also confirms complete replacement content for both 120,000- and 128,000-character inputs.

• Validation and review: 255/255 focused Desktop/UI tests, UI build, UI/Desktop typechecks, lint, format check under the existing configuration, ASF headers and git diff --check passed. An automated review-agent pass found no new P0–P3 in this four-file follow-up and independently reran 19/19 related tests. The hosted test check for exact head 5ead6ce71 is now green.

Local validation was targeted; the full local suite and packaged Electron were not rerun. The Composer probe uses linkedom and SQLite, not an Electron window.

中文对照

补充提交 5ead6ce71：修复了暂停消息编辑中新复现的一个 P2。

• 问题： 发送接口允许 128,000 字符，而普通会话草稿缓存只保留末尾 120,000 字符。编辑较长的暂停消息、切换会话再返回时，草稿可能被截短，但替换 ID 仍然保留。随后发送会用截短内容替换完整的持久化原件。

• 修复范围： 实际带有替换关联的草稿现在完整保留正文。关联在首次缓存写入前建立，也覆盖恢复到非当前会话的情况。普通草稿仍使用原有字符上限，所有正文条目仍共用 32 项淘汰规则。清空、淘汰和发送成功后的关联清理仍有测试覆盖。这次只修改了两个 UI 实现文件和两个已有测试文件。

• 回归证据： 三个新增测试在旧代码上均以 120000 !== 128000 失败，修复后全部通过，覆盖当前／非当前会话恢复和真实 Composer 发送。额外断言检查了引用位置、重复追加与保存、普通草稿上限和淘汰行为。Composer＋恢复服务＋SQLite 探针也确认，120,000 和 128,000 字符输入在替换后都保留了完整内容。

• 验证与审查： 255/255 项 Desktop/UI 定向测试、UI 构建、UI/Desktop 类型检查、lint、按现有配置执行的格式检查、ASF 头检查和 git diff --check 均通过。自动化 review-agent 对这四个文件的补充改动未发现新的 P0–P3，并独立复跑了 19/19 项相关测试。固定提交 5ead6ce71 的上述 hosted test 检查现已通过。

本地采用定向验证，没有重复运行全部本地测试或打包 Electron。Composer 探针使用 linkedom 和 SQLite，并非 Electron 窗口。

[Astro-Han]

Reviewed the increment 86f9b2da..5ead6ce71b8ee94a517202ba26a5bc2b87180f84 only, as scoped (4 files, +106/−12): the earlier head has been cleared by two lineages including a second opinion on its three P2s.

No P0–P3 findings, and the gate is green on this head (test: completed/success).

What the increment fixes. A replacement draft could be truncated on its first cache write, and the reason that matters is not cosmetic: sending a replacement can delete the durable original, so a shortened body would lose content the user still needed. Two things were wrong and both are addressed:

1. The ownership marker was established too late. setDraft now records replacesMessageId before the first cache write, with the comment "establish ownership before the first cache write, including restores to an inactive Session whose full text is never held in the live input". Previously the marker was set after the write, so that first write was treated as an ordinary draft and hit the character bound. This is the ordering bug, and the new test pins exactly it ("the first cache write must retain the full edit").
2. The bound is now exemption-aware. rememberComposerDraft takes preserveFullText, and the truncation is skipped when it is set — the call site passes it only for keys that are marked replacements. The comment states why: "Replacement drafts must stay lossless: sending them can delete the durable original. They still share the entry limit and whole-draft eviction below." Ordinary drafts keep the 120k character bound, and the cap's documentation was updated to say "ordinary draft" so the asymmetry is visible.

The test covers the paths that matter at 128,000 characters, for an active and an inactive draft: the first write retains the full body, an inactive draft's own value is untouched, switching sessions does not truncate the replacement, references survive, replacementMessageId is preserved, and a later append plus save still yields the full body followed by the addition.

The trade-off, stated rather than hidden. A replacement draft is now unbounded in characters; it remains bounded by the entry count and whole-draft eviction. That is deliberate — losslessness over the size bound — and is documented in the code, so I am recording it rather than objecting to it.

Gate on this head: test completed successfully; mergeable is true; no Grok involvement.

What I could not judge

• I did not run the new tests; the above is read from the diff and the test's assertions.
• No desktop run, so the interactive restore path is verified at the hook level rather than exercised.

I did not approve, request changes, or merge.

---

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

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent (Claude) operating on behalf of @Astro-Han. It is not an independent human review and does not replace one.

Incremental review of 5ead6ce7..867299d9. The increment is one merge of main 3597abe84, with no new author commits. No P0–P3 findings. CI test is green on this head, and the PR is mergeable with 0 commits behind main.

What changed. Every Main-process, shared-contract and packages/ui file in this PR has the same patch as at 5ead6ce7. The increment only moves the recovery wiring into the Composer owners that main introduced in #5954, #5935 and #5936.

The conflict resolution keeps main's behaviour:

• Recovery mount. Main mounted SessionLocalMessages and registered draftContextRestorer inside useComposerSubmission. Both now come from StagedLocalMessages, which ComposerSubmissionProvider mounts once and unconditionally (composer-submission-provider.tsx:73-76, staged-local-messages.tsx:41-47). The queue-edit restorer is therefore always installed, as on main.
• restoreContext. It now resolves to restoreQueuedDraftContext, which appends attachments, staged approvals, directories (keyed by host) and quotes. That covers everything main's version restored.
• Provider position. ComposerSubmissionProvider moved inside ModuleHubSkillCatalogRevisionBoundary. Every reader of its contexts still renders inside it, and the boundary is a render prop with no key, so the provider does not remount when the skill catalog revision changes.
• Recovery gate. Dropping canStageComposerContext from the gate changes nothing, because canRestoreDraft already requires a Session id.
• canStageContext and allowAttachmentOnlySend. These are now derived in conversation-readers.tsx from the same contextPickEnabled input that app-shell passed before.
• Replacement id. replacesLocalMessageId is forwarded at chat-actions.ts:315,477 and through composer-submit.ts for ordinary, /swarm and /graph sends. The new test "one recovery owner restores attachment approvals and sends the linked replacement" covers Edit, then the blocked Delete, then the linked submit, end to end.

For awareness: a main test was rewritten. #5954 added the test "a cancelled local message hands its text and staged context back to the Session it left". The merge replaced it with "navigation during local editing keeps the original paused and releases unused recovery approvals". This follows the PR's existing pause-and-refuse-stale-restore design, which was accepted earlier as intentional. It is not a new regression, but please confirm with #5954's owner.

Prior findings. Nothing above P3 was open at 5ead6ce7. All earlier P1 and P2 findings stay fixed, since those files are unchanged. The Delete-guard copy P3 and the downgrade P3 stay fixed. The queue-position and refused-restore behaviours are unchanged and remain intentional.

Runtime-host protocol. Not touched, so no epoch change is needed.

Gate: test passes. GitHub reports MERGEABLE / BLOCKED. I did not run tests locally, and did not do an Electron or multi-window run. I did not approve, request changes, or merge.

[jackeyfaker77]

Thanks @Astro-Han for flagging the rewritten main regression in your review.

@chihumyum and maintainers, since #5954 is already merged, I would like to confirm the intended navigation behavior before treating either option as the agreed resolution.

The scenario is a local, never-dispatched message: the user starts Edit in Session A, then switches to B before the recovery request completes.

1. Keep the pause-and-refuse behavior described in this review. The original remains durably paused, with its text and attachment bytes preserved. After navigation, the late restore is refused and unused temporary recovery approvals are released. Returning to A, the user can choose Edit again or Continue sending. This keeps the recovery guard simpler, but changes refactor(desktop): move the Composer's reads, edits and delivery recovery below the shell (R2 M3) #5954's automatic background-draft restoration and requires another user action.

2. Preserve refactor(desktop): move the Composer's reads, edits and delivery recovery below the shell (R2 M3) #5954's navigation behavior while retaining fix(desktop): clarify delivery states and recover failed messages #5058's safeguards. Restore the complete text, staged context and replacement identity into A if A's draft has not changed, leaving B's content and focus untouched. Subsequent edits or clearing A, Session deletion, or loss of the recovery owner must invalidate the pending restore. This needs reliable draft-change tracking—including typing and then deleting back to empty—and coordinated ownership of text, context, attachment approvals and the replacement link. Future draft-editing paths and lifecycle tests would need to preserve that contract.

Both options must retain #5058's durable paused original and atomic replacement on successful send. Refusing a restore must release only unclaimed temporary approvals, not the stored source. The paused-original Delete protection and downgrade safeguards should remain intact.

My preference is to keep option 1 if this behavior difference is acceptable to maintainers, to limit additional state coordination in this PR. If preserving #5954's automatic restoration is a requirement, option 2 would be the compatibility work to do, ideally through the existing draft owner and one conditional restore operation.

Could you confirm whether option 1 is acceptable, or whether #5954's navigation behavior must be retained via option 2? The test change records a real behavior choice; the earlier discussion within #5058 established intent, but I would like explicit confirmation of that choice against merged main. The regression tests should then enforce the agreed behavior and its content-preservation guarantees.

中文对照

感谢 @Astro-Han 在 review 中指出 main 的回归测试被改写。

@chihumyum 及维护者，考虑到 #5954 已经合并，希望先确认导航时应保留哪种行为，再把其中一种视为已经达成一致的处理方案。

场景是尚未发送到 Host 的本地消息：用户在会话 A 点击编辑，在恢复请求完成前切换到 B。

1. 保留本次 review 所描述的“暂停原件、拒绝恢复”行为。 原消息以暂停状态持久化，原文和附件字节仍保留。切换会话后拒绝迟到的恢复，并释放未使用的临时恢复凭证。用户回到 A 后，可再次编辑或继续发送。这种方案的恢复判断较简单，但改变了 refactor(desktop): move the Composer's reads, edits and delivery recovery below the shell (R2 M3) #5954 自动恢复到后台草稿的体验，需要用户再操作一次。

2. 兼容 refactor(desktop): move the Composer's reads, edits and delivery recovery below the shell (R2 M3) #5954 的导航行为，同时保留 fix(desktop): clarify delivery states and recover failed messages #5058 的安全保护。 只要 A 的草稿未发生变化，就将完整文本、暂存上下文和替换关联恢复到 A，保持 B 的内容与焦点不变。如果 A 后续被编辑或清空、会话被删除，或者负责恢复的组件已失效，则拒绝旧恢复。这需要可靠的草稿变更记录，覆盖“输入后又删空”等情况，并统一协调文本、上下文、附件凭证及替换关联的归属。后续新增草稿修改入口和生命周期测试时，也需要维护这一约定。

两种方案都必须保留 #5058 的持久化暂停原件，以及发送成功时的原子替换。 拒绝恢复时只释放未接管的临时凭证，不删除保存的原始内容。暂停原件的 Delete 保护和降级兼容保护也应继续保留。

如果维护者接受这项行为差异，我倾向于保留方案一，限制本 PR 新增的状态协调复杂度。如果 #5954 的自动恢复体验必须保留，那么方案二就是需要完成的兼容工作，优先考虑复用现有草稿管理能力，并集中为一个条件恢复操作。

请确认：是否接受方案一，还是必须采用方案二保留 #5954 的导航行为？ 这次测试变化对应一项真实的行为取舍；此前在 #5058 内的讨论明确了设计意图，但希望再就它与已合并 main 的差异获得明确确认。之后的回归测试应固定双方认可的行为及内容保全保证。

[Astro-Han]

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

At 087baf6f2c5fc69043bfe657c5a6ded7a498d477, the transcript-retry test regression is fixed. The Composer fixture receives the configured conversation services instead of shadowing them with defaults, and the test observes error feedback from both toast providers. All 11 retry tests pass locally, closing the preceding head's P2 CI regression. At the preceding head afbee248, all 11 failed with "Transcript observation not configured"; the same suite passed against the merged main commit 66da38ef.

The shutdown merge in apps/desktop/src/main/session-local-service.ts:578 preserves both catalog cancellation and recovery-approval cleanup. Its regression test confirms that closing twice aborts active catalog reads, prevents queued reads and late notifications/database writes, and rejects both old approvals and already-prepared leases. The paused original and its attachment bytes survive reopening, and recovery issues fresh approvals.

All 178 related tests pass locally, including transcript retry, local storage/recovery, attachment approval, presentation, Composer ownership and draft restoration. The dependency and Desktop test builds pass; the changed source files pass Biome.

Previously fixed recovery, paused-original deletion and downgrade issues are unchanged by this increment. The existing paused-message design is retained. There are no new P0–P3 findings in the merge and test update.

The Runtime Host protocol is unchanged and retains main's epoch 212. No epoch increment is required by these changes.

The hosted CI run for this head is still in progress: CI run. The previous head's failed run is not evidence that this head fails, and local tests do not establish that hosted CI is green. Electron, Storybook and the full local test suite were not rerun.