Skip to content

fix(desktop): replay pending approvals after reconnect - #82087

Closed
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/desktop-replay-pending-approvals
Closed

fix(desktop): replay pending approvals after reconnect#82087
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/desktop-replay-pending-approvals

Conversation

@konsisumer

Copy link
Copy Markdown
Contributor

What changed and why

Per the reporter's clarification on 2026-08-05, a Desktop approval can remain queued after its original WebSocket delivery is lost. session.resume and session.activate now return the oldest unresolved approval using the existing session-scoped approval queue. Desktop restores that prompt only after its runtime session is bound, so the existing approval controls can respond normally without re-running the tool or extending the timeout.

Addressing maintainer feedback

How to test

  • Start a Desktop session that reaches a dangerous-command approval.
  • Disconnect and reconnect the Desktop gateway, then resume the same session.
  • Confirm the original approval controls return and approving or denying resolves the original blocked operation once.
  • Automated: pytest tests/tui_gateway/test_protocol.py -q -x --timeout=60 (43 passed).
  • Automated: pytest tests/tools/test_approval.py::TestApprovalTimeoutIsNotConsent -q -x --timeout=60 (4 passed).
  • The required full-suite command was attempted but stops at the unrelated existing tests/tools/test_approval.py::TestDetectDangerousRm::test_nonrecursive_verification_artifact_cleanup_is_not_dangerous assertion.

What platforms tested on

  • Platform-independent Python gateway protocol tests.
  • Desktop TypeScript typecheck was not run because apps/desktop/node_modules is unavailable in this worker environment; no manual Desktop run was performed.

Refs #53839

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 8, 2026
teknium1 added a commit that referenced this pull request Aug 15, 2026
Clarify prompts share the same emitted-while-detached failure class the
pending-approval replay fixed: `clarify.request` rides `_block()`'s pending
registry, so a client whose transport was down when the event fired never
sees the question and the agent thread stays parked until timeout.

Widen the resume snapshot the same way:

- tui_gateway/server.py: `_live_session_payload` now carries
  `pending_clarify` — a read-only snapshot of the clarify prompt still
  blocking the session, scoped to the owning runtime sid. The registry stays
  authoritative; the embedded request_id resolves via clarify.respond.
- Desktop resume paths (`use-session-actions`) restore the parked clarify
  into the clarify store (multi_select preserved) and flag needsInput,
  mirroring restorePendingApproval on both the activate and resume paths.
- pending_approval replay now also forwards the queue-injected request_id so
  the restored prompt responds with exact-request correlation.
- Tests: server-side replay + scoping test; harmonized the #82087 replay
  test with the request_id `_ApprovalEntry` now injects.
@konsisumer

Copy link
Copy Markdown
Contributor Author

Closing — this change already landed on the default branch (commit 38b9005); deferring to that. If that's incorrect, feel free to reopen.

@konsisumer konsisumer closed this Aug 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thank you for this fix! It was salvaged into #86596 (cherry-picked onto current main with your authorship preserved in the commit history) and is now merged. Closing this PR since the work has landed.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants