Skip to content

fix(desktop): preserve fresh-chat file submits - #65708

Open
alijasero-ui wants to merge 1 commit into
NousResearch:mainfrom
alijasero-ui:fix/desktop-new-chat-file-upload
Open

fix(desktop): preserve fresh-chat file submits#65708
alijasero-ui wants to merge 1 commit into
NousResearch:mainfrom
alijasero-ui:fix/desktop-new-chat-file-upload

Conversation

@alijasero-ui

Copy link
Copy Markdown

Summary

  • keep route-token drift protection while a fresh chat has no runtime session
  • after the submit creates and pins its own runtime/stored session, use those stable IDs instead of treating its delayed route commit as a user switch
  • add a regression test for a remote file upload whose profile route commits while file.attach is awaiting I/O

Root cause

A fresh-chat submit creates a backend session and navigates to its stored-session route. That navigation can commit asynchronously, especially during a profile swap while file bytes are uploading. The submit pipeline compared the post-upload route with its pre-create route and aborted before prompt.submit, leaving an empty session. Established chats do not hit this transition.

The guard still aborts real user switches through the pinned stored/runtime session IDs.

Related to the session-isolation guard introduced around #54527.

Verification

  • npx vitest run src/app/session/hooks/use-prompt-actions/index.test.tsx — 46/46 passed
  • npm run typecheck — passed
  • regression test reproduced false before the fix and passes after the fix

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) needs-decision Awaiting maintainer decision before any implementation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 16, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #64759 and #64327. This focused fix stops raw route-token checks after a new session is created; the open alternatives retain explicit created-session or semantic route identity checks. Please choose the intended guard semantics.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the fresh-chat attachment race. The underlying premise is still present on current main: apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts:429-432 re-pins the raw route token after creation, and the post-upload check at :437-443 can abort before prompt.submit when that route commit lands during attachment synchronization.

Problems

  • apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts:346 turns off route validation for the entire created-session remainder. That accepts an unrelated route-only navigation as well as the expected delayed navigation; it does not prove the changed route belongs to the session this submit created.

Suggested changes

  • Preserve a created-session or semantic route-identity check: accept the created session's delayed route commit, but keep rejecting a route to another session.
  • Add the counterpart regression for unrelated route-only navigation during file.attach, asserting that prompt.submit is not sent.

This is an automated hermes-sweeper review.

// session ids — not the still-settling route — guard this phase.
startingStoredSessionId = selectedStoredSessionIdRef.current
startingRouteToken = getRouteToken()
guardRouteToken = false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This disables route drift detection for every post-create route change, not just the created session's delayed navigation. Please retain an expected created-session/semantic route check and add a regression for a route-only change to a different session during attachment sync.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 18, 2026
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-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

3 participants