Skip to content

fix(desktop): recover a dead first-submit draft instead of "session not found" - #67503

Closed
rapsealk wants to merge 3 commits into
NousResearch:mainfrom
rapsealk:fix/new-chat-first-submit-recovery
Closed

fix(desktop): recover a dead first-submit draft instead of "session not found"#67503
rapsealk wants to merge 3 commits into
NousResearch:mainfrom
rapsealk:fix/new-chat-first-submit-recovery

Conversation

@rapsealk

Copy link
Copy Markdown
Contributor

Fixes #67502

Problem

The first message of a new chat can dead-end with a raw "Prompt failed / session not found" toast, losing the user's text with no way to retry.

A new chat's state.db row is only persisted by the gateway on its first successful prompt.submit (deliberately lazy, so abandoned drafts never leave "Untitled" rows). If the live session dies before that first submit lands — sleep/wake WS drop followed by the orphan reap, a backend restart, a stale cached runtime id — then:

  1. prompt.submit fails with 4001 session not found, and
  2. the sleep/wake recovery's session.resume fails with the same 4007 session not found, because there is no row to resume.

That second rejection was uncaught in useSubmitPrompt — it escaped to the outer catch and surfaced verbatim, and every subsequent submit into the draft repeated the loop.

Fix

  • Wrap the recovery session.resume in try/catch instead of letting it reject the whole submit.
  • When both the live id and the stored row report "session not found" (the never-persisted-draft signature), mint a fresh backend session via createBackendSessionForSend and land the message there — moving the optimistic message, the drift baseline, and the attachment sync over to the minted chat (attachments re-sync so @file: refs resolve in the new session's workspace).
  • Introduce a mutable optimisticStoredSessionId for the optimistic/error-bubble state key so the re-homed send is never keyed under the dead stored id (which would cross-wire the session-state cache).

The fallback is scoped tightly; all of the following keep the existing surface-the-error behavior:

Tests

Four new tests in the "sleep/wake session recovery" block:

  • double-404 → fresh create → submit lands in the minted session (and the optimistic message is re-seeded under it);
  • non-404 resume failure → no session minted, original error surfaces;
  • timed-out submit whose recovery resume 404s → no session minted (double-send guard);
  • background queue drain hitting the double-404 shape → no session minted, view untouched.

apps/desktop: full vitest suite green (the one failure, toolset-config-panel.test.tsx, also fails on clean main in a full parallel run — pre-existing flake, passes in isolation); tsc --noEmit clean; eslint clean for the touched files.

🤖 Generated with Claude Code

…ot found"

A new chat's state.db row is only persisted by the gateway on its first
successful prompt.submit (deliberately lazy, so abandoned drafts never
leave "Untitled" rows behind). If the live session dies before that first
submit lands — sleep/wake WS drop followed by the orphan reap, a backend
restart — the next submit 4001s "session not found" and the sleep/wake
recovery's session.resume 4007s the SAME "session not found" because
there is no row to resume. That rejection escaped uncaught, surfaced as a
raw "Prompt failed / session not found" toast, and dropped the user's
message with no way forward.

Catch the recovery resume and, when BOTH the live id and the stored row
report "session not found" (the never-persisted-draft signature), mint a
fresh backend session and land the message there — moving the optimistic
message, drift baseline, and attachment sync over to the minted chat.

Scoped tightly: a timed-out submit may have actually reached the backend
(re-sending elsewhere would double-send), a non-404 resume failure says
nothing about whether the stored chat exists (minting would split a real
conversation, NousResearch#55578 symptom b), and a background queue drain must never
re-home the user's view — all of those keep the existing
surface-the-error behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #67502 and earlier merged recovery work #42688/#60874. This handles the distinct first-submit/no-row fallback and includes double-send and background-queue guards.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the distinct no-row path after the earlier resume-success recovery work.

Problems

  • The new recovery begins only after the initial syncAttachmentsForSubmit() call. On current main that call precedes prompt recovery (apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts:438) and invokes session-requiring image/file attach RPCs (tui_gateway/server.py:10067, tui_gateway/server.py:10397). Therefore a dead first-submit draft with a newly selected attachment still fails before the new double-404 branch can create a replacement session. The re-sync in the proposed branch helps already-staged attachments, but not this initial-sync failure.

Suggested changes

  • Cover session not found from the initial attachment-sync phase, or establish the replacement runtime before staging attachments; add file and image regressions that verify attachment upload and prompt submission on the minted runtime.
  • In the optimistic-state test, assert the stored-session argument passed to updateSessionState, not only the runtime id, because the cache maps runtime state through that stored id (apps/desktop/src/app/session/hooks/use-session-state-cache.ts:96-121).

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 19, 2026
…submit

file.attach / image.attach are session-scoped RPCs and the initial
attachment sync runs BEFORE prompt.submit, so a dead first-submit draft
with a newly selected attachment failed in staging — ahead of the
double-404 recovery, which never got a chance to run.

Extract the resume-or-mint recovery into recoverDeadRuntime and invoke it
from both session-scoped failure points: the initial attachment sync and
prompt.submit itself. A sync-time "session not found" now resumes the
stored session (or mints a replacement for the never-persisted-draft
double-404 signature), then re-stages the attachments against the live
runtime the recovery produced. Behavior at the prompt.submit failure
point is unchanged — same guards (timeout double-send, non-404 resume
failure, background drains), now routed through the shared helper.

Tests: file and image staging regressions covering attach-404 → recovery
→ re-stage → submit on the minted runtime; a resumed-runtime rebind case
(stored row exists → no minting); and the optimistic-state test now
asserts the stored-session key passed to updateSessionState, pinning
that post-recovery state is keyed under the minted stored id rather than
cross-wiring the dead draft's.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rapsealk

Copy link
Copy Markdown
Contributor Author

Addressed both findings in d904d12:

Attachment staging before recovery — the recovery is no longer anchored to prompt.submit. The resume-or-mint logic is extracted into a shared recoverDeadRuntime helper invoked from both session-scoped failure points: the initial syncAttachmentsForSubmit (where file.attach / image.attach 404 on a dead runtime) and prompt.submit itself. A sync-time "session not found" now resumes the stored session — or mints a replacement on the double-404 never-persisted-draft signature — and then re-stages the attachments against the live runtime the recovery produced before continuing to submit. The prompt.submit failure point keeps its exact previous behavior (timeout double-send guard, non-404 resume failures, background drains all still surface the original error), just routed through the shared helper.

Regressions added per the review:

  • recovers a dead draft whose FILE staging 404s before prompt.submit ever runsfile.attach (stale) → session.resume 404 → mint → file.attach (minted) → prompt.submit on the minted runtime with the re-staged @file: ref in the text.
  • recovers a dead draft whose IMAGE staging 404s before prompt.submit ever runs — same shape through image.attach.
  • rebinds attachment staging to the RESUMED runtime when the stored row still exists — pins that a real stored conversation rebinds via resume and never mints.

Stored-session keying — the optimistic-state test now asserts the stored-session argument passed to updateSessionState, not just the runtime id: the create stub re-homes selectedStoredSessionIdRef (mirroring the real createBackendSessionForSend), the test asserts the re-seed carries the minted stored id, and additionally that no post-recovery write still keys state under the dead draft's stored id (the cross-wiring hazard in use-session-state-cache). Backing this, the implementation's optimisticStoredSessionId re-pin now lives inside the shared helper so every recovery path gets it.

apps/desktop: target file 59 tests green, tsc --noEmit clean, full vitest suite green apart from the pre-existing toolset-config-panel.test.tsx parallel-run flake (also fails on clean main, passes in isolation).

@alt-glitch alt-glitch added the needs-decision Awaiting maintainer decision before any implementation label Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Re-triage confirms the reviewer-requested attachment-stage and stored-ID rehoming fixes are now present. Related to open #67539 (narrower) and #67575 (competing double-404 recovery); #67503 additionally covers initial file/image staging and stored-session optimistic-state keying, so this remains open for maintainer consolidation.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Three open PRs address #67502's dead first-submit/double-404 path. #67539 adds a narrow resume-error fallback, #67575 adds guarded prompt-submit recovery and re-homing, and #67503 additionally handles pre-submit file/image staging, updates the stored-session state key, and adds regressions for the critical recovery boundaries.

Related pull requests

Duplicates

#67503, #67539, and #67575 are competing implementations of the same #67502 recovery; #67539 and #67575 can be treated as superseded duplicates of the broader #67503 implementation.

Suggested consolidation

Keep #67503 open with a salvage path: retain its shared attachment/prompt recovery, fresh stored-ID re-homing, timeout/queue safeguards, and regression coverage for maintainer consolidation. Close #67539 and #67575 as duplicates of #67503 because their visible diffs leave respectively the attachment/timeout/queue gaps and the stale stored-ID/test gaps documented above.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I67502(["issue #67502 (open)"])
    subgraph Dup67503 ["PRs duplicating each other"]
        P67503["PR #67503 (open)"]
        P67539["PR #67539 (open)"]
        P67575["PR #67575 (open)"]
    end
    P67503 -->|best fix| I67502
    class I67502 open
    class P67503 open
    class P67539 open
    class P67575 open
    class P67503 best
    class P67503 target
    click I67502 "https://github.com/NousResearch/hermes-agent/issues/67502"
    click P67503 "https://github.com/NousResearch/hermes-agent/pull/67503"
    click P67539 "https://github.com/NousResearch/hermes-agent/pull/67539"
    click P67575 "https://github.com/NousResearch/hermes-agent/pull/67575"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 37 kB of PR diffs, 9 kB of issue/PR text, 5 kB of discussion (7 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Conflicts in use-prompt-actions/submit.ts, both sides touching the
sleep/wake recovery path:

- sessionContextDrifted() -> sessionDriftReason() everywhere the branch's
  shared recoverDeadRuntime helper checked for drift, with upstream's
  [submit-drift-abort] warn logging.
- recoverDeadRuntime's session.resume now goes through
  resolveSessionProfile + omit_messages (upstream NousResearch#67603), so a recovery
  can't fork the conversation into another profile's DB.
- The recovery retry submits through upstream's submitParams(), so a
  re-homed send keeps the interrupted/queued flags; submitParams takes an
  optional text override for the minted-session re-stage.
- The minted fallback titles the new chat with bubbleText, matching the
  primary create path.

Test: resume params assertion updated for omit_messages.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@OutThisLife

Copy link
Copy Markdown
Collaborator

Superseded by #81261.

Thanks for this — the first-submit draft recovery is carried over there, with you credited via Co-authored-by.

Consolidating because this is one bug class across several PRs: main had three hand-rolled copies of the same stale-session recovery (prompt.submit, session.interrupt, session.redirect), and the RPCs that never got a copy — attach, /compress, checkpoint restore, tile actions — each failed the same way. #81261 makes it one resolver and routes every path through it, so the next session-scoped RPC we add inherits the fix instead of needing a fourth copy.

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/*) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

Desktop: first message of a new chat dead-ends with "Prompt failed / session not found" when the draft's live session dies

5 participants