Skip to content

fix(terminal): prevent restoring stale terminal sessions#1943

Merged
bajrangCoder merged 2 commits intoAcode-Foundation:mainfrom
bajrangCoder:fix/terminal-restore-stale-sessions
Mar 12, 2026
Merged

fix(terminal): prevent restoring stale terminal sessions#1943
bajrangCoder merged 2 commits intoAcode-Foundation:mainfrom
bajrangCoder:fix/terminal-restore-stale-sessions

Conversation

@bajrangCoder
Copy link
Member

No description provided.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR improves terminal session restoration by making stale/unavailable sessions fail gracefully instead of disrupting the user at startup. The two main changes are:

  • terminal.jsconnectToSession now wraps the WebSocket handshake in a Promise with a 5-second timeout and proper pre-connection rejection guards (settled/hasOpened flags). Before this change, a failed reconnect attempt had no way to propagate the failure back to the caller; now the promise rejects cleanly, enabling restorePersistedSessions to catch and skip stale PIDs.
  • terminalManager.js — Session persistence is refactored into a synchronous readPersistedSessions helper (safe to call from persistTerminalSession/removePersistedSession without async overhead) and an async getPersistedSessions that also validates the backend is running. The restorePersistedSessions startup flow now silently skips unavailable sessions with a toast instead of an intrusive alert, and stale entries are properly await-ed before removal.

Key issues found:

  • The new getPersistedSessions reads localStorage a second time just to compare array lengths; this work is redundant since readPersistedSessions already has the parsed data.
  • The length-only guard for saving normalized sessions back to storage won't trigger for format-only migrations (legacy string PIDs vs. object entries), so stale string-format data in localStorage is re-normalized in memory on every startup but never actually persisted in the upgraded format.
  • console.error("WebSocket error:", error) inside onerror fires unconditionally for expected stale-session failures, undermining the "keep startup quiet" intent of the PR with low-information noise in the developer console.

Confidence Score: 4/5

  • Safe to merge; the core reconnection logic is sound and UX regressions from stale sessions are resolved.
  • The fundamental fix — making connectToSession reject on pre-connection failures — is correct and well-guarded. The session manager refactoring is clean. Two minor logic gaps (redundant localStorage read and format-migration not persisted) exist in getPersistedSessions but neither causes data loss or a visible regression at runtime.
  • src/components/terminal/terminalManager.js — getPersistedSessions has the redundant read and the length-only migration guard.

Important Files Changed

Filename Overview
src/components/terminal/terminalManager.js Adds normalizePersistedSessions/readPersistedSessions helpers and improves session restoration UX; contains a redundant second localStorage read in getPersistedSessions and a length-only normalization guard that silently skips persisting format migrations.
src/components/terminal/terminal.js Wraps WebSocket setup in a Promise with a 5s timeout and proper pre-connection rejection guards; logic is sound with correct settled/hasOpened flags preventing double-rejection.

Sequence Diagram

sequenceDiagram
    participant App
    participant TerminalManager
    participant localStorage
    participant Backend as AXS Backend
    participant WebSocket

    App->>TerminalManager: restorePersistedSessions()
    TerminalManager->>TerminalManager: getPersistedSessions()
    TerminalManager->>localStorage: getItem(TERMINAL_SESSION_STORAGE_KEY)
    localStorage-->>TerminalManager: raw sessions
    TerminalManager->>TerminalManager: normalizePersistedSessions(raw)
    TerminalManager->>Backend: isAxsRunning()
    alt Backend not running
        Backend-->>TerminalManager: false
        TerminalManager->>localStorage: savePersistedSessions([])
        TerminalManager-->>App: []
    else Backend running
        Backend-->>TerminalManager: true
        TerminalManager-->>App: sessions[]
        loop For each session
            App->>TerminalManager: createServerTerminal({pid, reconnecting: true})
            TerminalManager->>TerminalManager: createTerminal({reconnecting: true})
            TerminalManager->>TerminalManager: connectToSession(pid)
            TerminalManager->>WebSocket: new WebSocket(wsUrl)
            Note over TerminalManager,WebSocket: 5s connection timeout
            alt Connection succeeds
                WebSocket-->>TerminalManager: onopen
                TerminalManager-->>App: terminal instance
            else Connection fails (onerror / onclose / timeout)
                WebSocket-->>TerminalManager: reject (no alert shown)
                TerminalManager->>localStorage: removePersistedSession(pid)
                TerminalManager->>App: toast("Skipped unavailable terminal: ...")
            end
        end
    end
Loading

Last reviewed commit: 92068d3

@bajrangCoder bajrangCoder merged commit c0947df into Acode-Foundation:main Mar 12, 2026
6 checks passed
@bajrangCoder bajrangCoder deleted the fix/terminal-restore-stale-sessions branch March 12, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant