Fix terminal lifecycle, agent status, dock layout, and remote input ordering - #607
Closed
iamjakeirl wants to merge 25 commits into
Closed
iamjakeirl wants to merge 25 commits into
iamjakeirl wants to merge 25 commits into
Conversation
added 18 commits
September 12, 2026 21:43
added 6 commits
September 14, 2026 01:07
Adapt terminal persistence and status tests to the emulator worker, retaining lifetime guards after snapshot reads and concurrent panel state updates. Combine ordered remote input with upstream's safe-read retry policy.
Author
|
also sorry this PR is massive i've only ever solo'd projects before this and am getting used to github. future prs will be branched properly |
This was referenced Sep 26, 2026
This was referenced Sep 26, 2026
Member
|
Thanks for this, @iamjakeirl. The fixes are solid, and you tracked down some nasty bugs. To make review and landing easier, we've split this PR into five smaller ones. Your commits are kept as authored by you:
Each change was checked against current main, and all of them are still needed. Closing this one in favor of those. Review comments go on the new PRs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Switching or closing Panes could leave asynchronous work touching a disposed terminal, report an agent as working or finished at the wrong time, or restore an agent into the shell dock. Remote typing also issued concurrent requests that could reach the host out of order. This PR fixes those paths and gives fresh repository workspaces the same tool launcher and shell dock as worktree workspaces.
The changes span the terminal's lifecycle: launch → input/output and status detection → renderer attach/reconnect → close/archive. The sections below explain the problem, the resulting behavior, and where to review each part.
Branch and review scope
This revision is based on upstream v2.4.129,
f977e553, merged at12e94a0e. Against that upstream commit, the contribution is 54 files, +2,536 / −394 lines, including tests, documentation, and one existing review screenshot. There are 20 non-merge commits and five upstream merge commits in the PR history. Upstream release, dependency, skill-bundle, and other changes brought in by those merges are already onmain; they are not additional features introduced by this PR.Since the previously published head,
b51e7fab, this update adds12e94a0e:Earlier updates remain in the history:
09da5133integrated v2.4.115;b68ff505preserved bare-Escape input boundaries and handled discarded keyboard input;b51e7fabintegrated v2.4.125 and adapted lifecycle guards to worker-backed terminal snapshots.Suggested review order: 1–2 for terminal ownership and teardown, 3–4 for status detection and consumers, 5 for dock/launcher behavior, then 6 for remote input. Each section lists its implementation and regression coverage so these can be reviewed in smaller groups.
1. Keep asynchronous work attached to the correct terminal
Problem: A refresh or refocus operation can await a resize/state reply while the user switches or archives a session. Its captured xterm may have been disposed when the reply arrives, producing the original Windows terminal
dimensionserror. Main-process callbacks can similarly outlive the terminal they were created for, even when a replacement reuses the same panel ID.Changes and reasons:
restoreSnapshot(); saves also recheck panel identity. A deleted/replaced panel cannot receive a late snapshot. The save reads the currentpanel.stateafter the await, preserving changes such as input-delivery timestamps, selection state, and dimensions made while the worker was responding.Implementation:
frontend/src/components/panels/TerminalPanel.tsx;main/src/services/terminalPanelManager.ts.Coverage:
tests/terminal-blur-recovery.spec.tsholds resize replies across session switches and verifies no disposed-terminal error or change to the replacement terminal.terminalPanelManager.status.test.tscovers replacement callbacks, stale worker snapshots, panel deletion/replacement, concurrent panel-state updates, delayed initial input, and write failure followed by a real exit.2. Drain and persist before teardown; reclaim resources on failure
Problem: Fire-and-forget persistence raced emulator disposal and panel deletion. Repeated close requests, natural exit during a save, and errors while delivering output could also leave inconsistent terminal ownership or cleanup.
Changes and reasons:
destroyTerminal()returns a shared promise for a terminal's teardown. Repeated callers join the same operation; detection and new input/output processing stop as teardown begins.{ saveState: false }, avoiding a snapshot/database write for a panel about to be deleted. Session archive and Pane Chat agent switching await normal teardown before proceeding.exitfails.Implementation:
terminalPanelManager.ts; its callers inmain/src/ipc/panels.ts,main/src/ipc/session.ts, andmain/src/services/paneChatManager.ts. TheterminalStateEmulator.tscomment now describes the awaited teardown behavior.Coverage:
terminalPanelManager.status.test.tscovers repeated destruction, drain-before-dispose, successful/signaled exit during save, deletion without persistence, event-delivery failure, disposal failure, and WSL exit-write failure.terminalPanelManager.persistence.test.tsawaits teardown in its cleanup/restore scenarios.3. Distinguish startup, actual work, reliable idle, and live blockers
Problem: Boot banners, inherited shell titles, cursor redraws, and typing could create false working/completed transitions. Conversely, a prompt that remains visible during a turn could mark real work idle. Broad prompt matching could keep an agent blocked by answered questions in scrollback.
Changes and reasons:
unknown. Detection waits for a pending initial command to be injected, clears the launch shell's title in the headless model, and restarts the monitor's grace window at injection. Legacy idle waits remain pending during startup instead of succeeding before the tool launches.unknownmeans no detected status yet.Implementation:
main/src/services/agentStatus/agentStatusMonitor.ts,manifests.ts, andterminalPanelManager.ts; thevisibleIdlecontract inshared/types/agentStatus.ts; related frontend status comments.docs/ADDING_NEW_CLI_TOOLS.mddocuments reliable idle evidence, precedence, and startup handling for future integrations.Coverage:
agentStatusMonitor.test.ts,manifests.test.ts,terminalPanelManager.status.test.ts, andterminalPanelManager.persistence.test.tscover boot/grace timing, delayed command injection, inherited titles, real startup work, idle redraws, typing, live blockers, answered prompts, and wrapped CLI output.4. Reconcile status after reconnect without announcing false completions
Problem: Live events alone cannot reconstruct status when a renderer attaches to an already-running daemon or misses transitions while disconnected. Applying an older snapshot over a newer event can regress state; interpreting a snapshot or terminal shutdown as a completed turn can trigger false notifications and unseen-completion badges.
Changes and reasons:
panels:agent-statusesread, returning authoritative monitor state for terminal panels in non-archived sessions, including hidden Pane Chat. A panel without a detected state is returned asunknown.Appinstalls a shared status subscription that listens before requesting its initial snapshot and refreshes on remote resync. Live events received during a read win over that snapshot; superseded requests and replies after unsubscribe are ignored.exit/destroyedevents update display state without inventing a new completed turn. A subsequent real working → idle transition still marks a background session as completed when the session has settled.terminal_start, so successive processes using the same panel ID can each record an exit.Implementation:
frontend/src/services/panelStatusSync.ts,App.tsx,stores/panelStore.ts,types/panelStore.ts, andhooks/useNotifications.ts;main/src/ipc/panels.ts,services/workspaceJournal.ts, anddaemon/mobilePushSender.ts.Coverage:
panelStatusSync.test.ts,panelStore.test.ts,panels.status.test.ts,daemonRegistryBindings.test.ts,workspaceJournal.test.ts, andmobilePushSender.test.ts.tests/agent-status.spec.tschecks sidebar/tab state and notification behavior through attach, reconnect, real completion, terminal ending, and deletion.5. Keep shells in the dock and agents in working tabs
Problem: Treating the first terminal as the dock could move an agent there after the original shell was deleted or the workspace reopened. Main repository workspaces also differed from worktree workspaces: their empty state only offered “Open a terminal,” and their shell occupied the main stage. Dock promotion and asynchronous tab restoration could leave the selected tab out of sync with persisted state.
Changes and reasons:
getDockTerminalPanel()selects the first plain shell. It excludes terminals with an initial command or CLI/agent metadata, including tools whose runtime metadata has not arrived yet. Additional shells remain working tabs until one is promoted into the dock.TerminalDockandEmptyPanelStage. The launcher offers Terminal, environment-appropriate agent presets, configured custom commands, icons, and shortcuts; it remains available above an open dock when there are no working tabs.Implementation:
frontend/src/components/ProjectView.tsx,SessionView.tsx,panels/TerminalDock.tsx,panels/EmptyPanelStage.tsx, andutils/terminalDock.ts. ExistingCHANGELOG.mdentries describe the dock default and tab-retention behavior.Coverage:
terminalDock.test.tsandtests/terminal-dock.spec.tscover launcher availability, agent launching, runtime-metadata restoration, shell removal, single/split layout promotion, saved collapse state, and active-tab persistence. The Electron browser fixture now persists layout/selection and emits panel creation/deletion events. Existing adaptive-layout, font-settings, and selection-popover tests explicitly request a collapsed starting dock where their scenario requires one, preserving their original assertions under the new default. The keyboard-copy test waits for a mounted, restored terminal before selecting text, so startup cannot clear the test selection.6. Preserve remote typing order and Escape boundaries
Problem: Concurrent HTTP input requests could arrive in a different order from the user's keystrokes. Serializing every buffered key into a separate request would add avoidable latency. Retrying an interrupted write could duplicate text or replay part of a command that already reached the host; combining a bare Escape with the next key could turn it into an Alt shortcut.
Changes and reasons:
RemoteInputQueue. It allows one input request in flight per panel acrossterminal:inputandpanels:send-terminal-input; different panels and non-input commands remain independent.sendTerminalInput()helper handles delivery rejection for fire-and-forget typing, special-key, paste, and interceptor paths. It logs the failure instead of allowing one global unhandled-rejection alert per discarded keystroke.Implementation:
shared/remoteInputQueue.ts;main/src/daemon/client/remotePaneClient.ts;frontend/src/remote/runtime/remoteDaemonBrowserClient.ts;frontend/src/utils/terminalInput.tsand itsTerminalPanel.tsxcall sites.docs/remote-daemon-lifecycle.mddocuments ordering, batching, Escape, timeout, and no-replay semantics.Coverage:
remoteTerminalInput.test.tsexercises both client implementations against a delayed local HTTP server, covering fast typing, Unicode/control sequences, batching, independent panels, both input channels, HTTP errors, and disconnects.remoteInputQueue.test.tscovers Escape boundaries, batch sizing, timeout, and late completion after cancellation.terminalInput.test.tsverifies delivery rejection is handled.remotePwaBrowserRuntime.test.tsnow supplies valid input arguments when checking an invalid response from the host.Compatibility and areas needing care
panels:agent-statuses. An older daemon without that handler cannot supply the new baseline; the renderer logs the failure and retains live-event handling. Mixed-version status reconciliation was not exercised in this refresh.destroyTerminal()is now asynchronous. Ordering-sensitive deletion, archive, and agent-switch callers await it; the persistence format and database schema are unchanged.Validation of this revision
Fresh local checks for
12e94a0e, using Node 22.18.0, pnpm 10.19.0, and Electron 41.10.3 in WSL/Linux:pnpm lintpnpm typecheckpnpm build:mainpnpm build:frontendpnpm --filter frontend testpnpm --filter main exec vitest run src/services/skillCacheManager.test.tsunder Nodepnpm exec electron-builder --linux AppImage --x64 --publish never --config.npmRebuild=false12e94a0e.git diff --check upstream/main...HEADshellPath.ts; no whitespace errors are introduced by the contribution against upstream.Launcher-test diagnosis: The unchanged upstream test helper replaces its child-process PATH with the directory containing
process.execPath. Under Electron that directory has neithernodenornpx, which the generated launcher requires. A direct probe confirmed both are absent from the restricted PATH; the complete skill-cache suite passes under regular Node. The Windows-only test is skipped on Linux. No production or test behavior was weakened to hide these failures.Browser timing limitation: The remaining combined-run failure was
clock.pauseAt: Cannot fast-forward to the pastin the 10,001 ms blur case. The complete 14-test blur/recovery file passed when rerun alone. The original font/clipboard failures were addressed by the two fixture changes described above.Exact focused backend and browser commands
Installed personal-build verification: Installed the merged
12e94a0ebuild as both the Windows desktop application and the primary WSL AppImage daemon. Verified version 2.4.129, commit identity, and personal lifecycle-fix markers directly in both installed packages, including the archive mounted by the running daemon. The Windows application reported that version/commit at runtime, opened a responding Pane window, and established a connection to the primary WSL daemon. Both primary and development daemon health endpoints returned ready. The Windows package reuses the existing, matching Electron 41.10.3 runtime and unchanged Windows native dependencies; 1,201 application files were compared with the new build and 22 native files with the previous Windows installation. Previous application resources and local session databases were backed up before replacement. This is startup/connection verification, not exhaustive interactive testing.Validation limits: The complete backend suite, full Playwright suite, packaged macOS DMG, and exhaustive interactive Windows flows were not run for this revision. Browser fixtures validate renderer behavior with mocked IPC. The direct remote-input tests exercise both client implementations against a local HTTP server; they do not cover every real-network condition.
Earlier evidence, retained for context only: The original PR description reported manual dock/startup/status testing with a Windows development client connected to the WSL daemon. It also reported a full backend run with 996 passed, 3 failed, and 2 skipped, with failures reproduced on then-unchanged upstream: two skill-cache launcher tests and one macOS Tailscale DNS timeout in WSL. Those are historical results, not a full-suite result for this revision.
Existing UI reference
Fresh repository launcher above the terminal dock, captured for the earlier revision using the browser fixture. The shell is shown loading; this is a layout reference, not a new screenshot of the v2.4.129 build.
Related work
The earlier PR description links #572 for neighboring activation/launcher-flashing work. The disposed-terminal guards here specifically address asynchronous refresh work completing after its terminal has been replaced or disposed.