fix(chat): retry released queue inputs - #2137
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds durable ChangesExplicit Queue Input Retry
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant PendingInputLane
participant PendingInputStore
participant SessionClient
participant SessionTurn
participant PendingInputAdmissionCoordinator
participant PendingSessionInputStore
PendingInputLane->>PendingInputStore: retryQueueInput(sessionId, itemId)
PendingInputStore->>SessionClient: retryPendingQueueInput(sessionId, itemId)
SessionClient->>SessionTurn: invoke retry route
SessionTurn->>PendingInputAdmissionCoordinator: retryPendingQueueInput(sessionId, itemId)
PendingInputAdmissionCoordinator->>PendingSessionInputStore: retryReleasedQueueInput(itemId)
PendingInputAdmissionCoordinator->>PendingInputAdmissionCoordinator: drain or schedule queue
PendingInputStore->>PendingInputLane: refresh item and clear retrying state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/src/i18n/ru-RU/chat.json`:
- Line 77: Update the retryRequired translation in the Russian chat localization
to use “Требуется повторная попытка,” clearly indicating that the unsent queue
message must be retried.
In `@test/renderer/stores/pendingInputStore.test.ts`:
- Line 97: Remove the duplicate const retry declaration in the affected test
scope, keeping the single existing createDeferred<{ accepted: boolean; started:
boolean }>() declaration and all references to retry unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d28fb894-de3d-44e5-b52b-d2ae20846b5e
📒 Files selected for processing (53)
docs/issues/pending-input-explicit-retry/spec.mdsrc/main/agent/deepchat/harness/deepChatAgentHarness.tssrc/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.tssrc/main/agent/deepchat/runtime/pendingInputContracts.tssrc/main/agent/deepchat/runtime/pendingInputPump.tssrc/main/agent/manager/deepChatAgentBackend.tssrc/main/agent/manager/sessionHandles.tssrc/main/app/composition.tssrc/main/session/contracts.tssrc/main/session/data/pendingInputStore.tssrc/main/session/data/pendingInputs.tssrc/main/session/routes.tssrc/main/session/turn.tssrc/renderer/api/SessionClient.tssrc/renderer/src/components/chat/PendingInputLane.vuesrc/renderer/src/features/chat-page/ChatPage.vuesrc/renderer/src/features/chat-page/composables/usePendingInputActions.tssrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/de-DE/chat.jsonsrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/es-ES/chat.jsonsrc/renderer/src/i18n/fa-IR/chat.jsonsrc/renderer/src/i18n/fr-FR/chat.jsonsrc/renderer/src/i18n/he-IL/chat.jsonsrc/renderer/src/i18n/id-ID/chat.jsonsrc/renderer/src/i18n/it-IT/chat.jsonsrc/renderer/src/i18n/ja-JP/chat.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/ms-MY/chat.jsonsrc/renderer/src/i18n/pl-PL/chat.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/ru-RU/chat.jsonsrc/renderer/src/i18n/tr-TR/chat.jsonsrc/renderer/src/i18n/vi-VN/chat.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/zh-HK/chat.jsonsrc/renderer/src/i18n/zh-TW/chat.jsonsrc/renderer/src/stores/ui/pendingInput.tssrc/shared/contracts/routes.tssrc/shared/contracts/routes/sessions.routes.tssrc/shared/types/agent-interface.d.tstest/main/agent/deepchat/harness/deepChatAgentHarness.test.tstest/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.test.tstest/main/agent/deepchat/runtime/pendingInputPump.test.tstest/main/agent/manager/deepChatAgentBackend.test.tstest/main/routes/dispatcher.test.tstest/main/session/data/pendingInputStore.test.tstest/main/session/data/pendingInputs.test.tstest/main/session/data/tables/deepchatPendingInputsTable.test.tstest/main/session/turn.test.tstest/renderer/components/PendingInputLane.test.tstest/renderer/features/chat-page/composables/usePendingInputActions.test.tstest/renderer/stores/pendingInputStore.test.ts
There was a problem hiding this comment.
Pull request overview
Adds durable, explicit retry handling for failed Queue inputs while preserving FIFO, restart, ACP, and attachment behavior.
Changes:
- Introduces
retry_requiredpersistence and retry routing. - Adds localized retry UI and edit-as-retry support.
- Expands regression coverage across runtime, persistence, routes, and renderer.
Reviewed changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
test/renderer/stores/pendingInputStore.test.ts |
Tests retry deduplication and deferred acceptance. |
test/renderer/features/chat-page/composables/usePendingInputActions.test.ts |
Tests retry actions and payload preservation. |
test/renderer/components/PendingInputLane.test.ts |
Tests retry-required lane controls. |
test/main/session/turn.test.ts |
Tests gated retry delegation and ACP rejection. |
test/main/session/data/tables/deepchatPendingInputsTable.test.ts |
Tests native persistence and FIFO blocking. |
test/main/session/data/pendingInputStore.test.ts |
Tests retry transitions and ordering constraints. |
test/main/session/data/pendingInputs.test.ts |
Tests ownership and restart recovery. |
test/main/routes/dispatcher.test.ts |
Tests retry route dispatch. |
test/main/agent/manager/deepChatAgentBackend.test.ts |
Tests backend retry delegation. |
test/main/agent/deepchat/runtime/pendingInputPump.test.ts |
Tests release, FIFO, and restart behavior. |
test/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.test.ts |
Tests retry admission and failure recovery. |
test/main/agent/deepchat/harness/deepChatAgentHarness.test.ts |
Updates end-to-end failure-state expectations. |
src/shared/types/agent-interface.d.ts |
Adds the retry_required state. |
src/shared/contracts/routes/sessions.routes.ts |
Defines the typed retry route. |
src/shared/contracts/routes.ts |
Registers the retry route. |
src/renderer/src/stores/ui/pendingInput.ts |
Manages retry state and requests. |
src/renderer/src/i18n/zh-TW/chat.json |
Adds Traditional Chinese retry copy. |
src/renderer/src/i18n/zh-HK/chat.json |
Adds Hong Kong Chinese retry copy. |
src/renderer/src/i18n/zh-CN/chat.json |
Adds Simplified Chinese retry copy. |
src/renderer/src/i18n/vi-VN/chat.json |
Adds Vietnamese retry copy. |
src/renderer/src/i18n/tr-TR/chat.json |
Adds Turkish retry copy. |
src/renderer/src/i18n/ru-RU/chat.json |
Adds Russian retry copy. |
src/renderer/src/i18n/pt-BR/chat.json |
Adds Brazilian Portuguese retry copy. |
src/renderer/src/i18n/pl-PL/chat.json |
Adds Polish retry copy. |
src/renderer/src/i18n/ms-MY/chat.json |
Adds Malay retry copy. |
src/renderer/src/i18n/ko-KR/chat.json |
Adds Korean retry copy. |
src/renderer/src/i18n/ja-JP/chat.json |
Adds Japanese retry copy. |
src/renderer/src/i18n/it-IT/chat.json |
Adds Italian retry copy. |
src/renderer/src/i18n/id-ID/chat.json |
Adds Indonesian retry copy. |
src/renderer/src/i18n/he-IL/chat.json |
Adds Hebrew retry copy. |
src/renderer/src/i18n/fr-FR/chat.json |
Adds French retry copy. |
src/renderer/src/i18n/fa-IR/chat.json |
Adds Persian retry copy. |
src/renderer/src/i18n/es-ES/chat.json |
Adds Spanish retry copy. |
src/renderer/src/i18n/en-US/chat.json |
Adds English retry copy. |
src/renderer/src/i18n/de-DE/chat.json |
Adds German retry copy. |
src/renderer/src/i18n/da-DK/chat.json |
Adds Danish retry copy. |
src/renderer/src/features/chat-page/composables/usePendingInputActions.ts |
Implements renderer retry handling. |
src/renderer/src/features/chat-page/ChatPage.vue |
Connects retry state and events. |
src/renderer/src/components/chat/PendingInputLane.vue |
Displays retry status and controls. |
src/renderer/api/SessionClient.ts |
Exposes the retry client method. |
src/main/session/turn.ts |
Gates retry through session operations. |
src/main/session/routes.ts |
Handles the retry route. |
src/main/session/data/pendingInputStore.ts |
Persists retry state and transitions. |
src/main/session/data/pendingInputs.ts |
Coordinates retry ownership and recovery. |
src/main/session/contracts.ts |
Extends session retry contracts. |
src/main/app/composition.ts |
Wires retry into runtime composition. |
src/main/agent/manager/sessionHandles.ts |
Extends the DeepChat control facet. |
src/main/agent/manager/deepChatAgentBackend.ts |
Delegates backend retries. |
src/main/agent/deepchat/runtime/pendingInputPump.ts |
Enforces retry-aware release and draining. |
src/main/agent/deepchat/runtime/pendingInputContracts.ts |
Adds mutation-specific release disposition. |
src/main/agent/deepchat/runtime/pendingInputAdmissionCoordinator.ts |
Implements retry admission and scheduling. |
src/main/agent/deepchat/harness/deepChatAgentHarness.ts |
Exposes runtime retry operations. |
docs/issues/pending-input-explicit-retry/spec.md |
Documents the design and validation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
zerob13
left a comment
zerob13
left a comment
There was a problem hiding this comment.
Two actionable state-machine issues remain. The first is reproduced by a failing regression on the current head; the second combines two already-supported paths that are only tested separately.
| private getWaitingQueueRows(sessionId: string): DeepChatPendingInputRow[] { | ||
| return this.getQueueRows(sessionId).filter( | ||
| (row) => row.state === 'pending' || row.state === 'blocked' | ||
| (row) => row.state === 'pending' || row.state === 'blocked' || row.state === 'retry_required' |
There was a problem hiding this comment.
[P1] Preserve the claimed Queue slot during every resequence. getWaitingQueueRows() excludes the active claimed row, while move/delete/conversion renumber the visible tails from 1. Reproduced on this head: reorder a newer item to the head, let the pump claim it, reorder an older tail, then release the claimed item to retry_required; both rows get queue_order = 1, the older tail wins created_at, and getNextPendingQueueInput() returns that tail instead of null. A later wake can therefore send the wrong item before the required retry. Include active claimed rows when resequencing (or preserve their occupied slots) and cover move/delete/conversion during a claim followed by retry release.
| if (row.state !== 'retry_required') { | ||
| throw new Error(`Pending queue item ${itemId} does not require retry.`) | ||
| } | ||
| if (this.getWaitingQueueRows(row.session_id)[0]?.id !== itemId) { |
There was a problem hiding this comment.
[P2] Do not require a retry-required row to already be the FIFO head. shouldClaimImmediately() intentionally lets an explicit Send bypass restart-held Queue drafts (and the question-follow-up fast path can bypass ordinary drafts), so that claimed row receives a later queue_order. If it fails before the user fact, this PR turns it into a non-head retry_required row; the renderer shows Retry, but this check makes every click silently return accepted: false until unrelated earlier rows are cleared. Transitioning that row to pending is safe because the existing dispatcher still enforces FIFO. Let the transactional operation return accepted/no-op without this head restriction, delete the duplicated coordinator-side sort/preflight, and add a held-head + failing fast-path Send + Retry regression.
zerob13
left a comment
zerob13
left a comment
There was a problem hiding this comment.
One additional persisted-data compatibility issue:
| ## Fix Design | ||
|
|
||
| - Add durable `retry_required` to `PendingSessionInputState`. SQLite already stores unconstrained | ||
| text, so no schema migration is required. |
There was a problem hiding this comment.
[P2] Treat retry_required as a persisted contract change even though the SQLite column needs no DDL. A downgrade binary still sees schema v46 and reads this row, but its waiting-row filter excludes the state while its renderer presents ordinary edit/reorder/Steer controls; those mutations reject the unknown state, leaving deletion as the only recovery. The repository SDD contract requires upgrade/rollback considerations for stored user data. Add an explicit downgrade strategy or schema/version fence, and cover/document the behavior of active retry rows when rolling back.
Summary
retry_requiredstate for Queue inputs released after a pre-user-fact failure or transcript rollback.Closes #2112.
Root Cause
PR #2023 centralized pending input claim, release, and single-flight draining in
PendingInputPump.When a claimed Queue input failed before its user fact was committed, or after the fact was rolled back, it was released to the FIFO head as an ordinary
pendingrow. The pump intentionally did not replay that row automatically, but the renderer could not distinguish it from a normal draft and exposed no explicit recovery action.PR #2129 added restart recovery and Resume Queue behavior, which made the issue more visible, but did not introduce the original defect.
Changes
retry_requiredtoPendingSessionInputState.claimedtoretry_required.pending.sessions.retryPendingQueueInputroute.acceptedandstartedresults so an accepted retry delayed by a runtime gate is not reported as a failure.attachmentFallbackPolicywhen editing a Queue item.No database migration is required because the pending input state column is unconstrained text.
UI
Before
The failed item remained at the Queue head without explaining why the Queue was blocked or how to continue it.
After
Retry-required rows cannot be reordered or converted to Steer. They can be retried, edited and retried, or removed.
Safety and Compatibility
pending.Validation
pnpm run formatpnpm run i18npnpm run lintpnpm run typecheck:nodepnpm run typecheck:webTwelve native SQLite tests were skipped by the repository harness because the locally installed native module ABI does not match the Node ABI used by Vitest. Equivalent store, FIFO, retry, and restart contracts passed in the non-native suites.
Summary by CodeRabbit
New Features
Bug Fixes