Track per-terminal subprocess activity in thread state#105
Merged
juliusmarminge merged 1 commit intomainfrom Feb 27, 2026
Merged
Track per-terminal subprocess activity in thread state#105juliusmarminge merged 1 commit intomainfrom
juliusmarminge merged 1 commit intomainfrom
Conversation
WalkthroughThis PR introduces terminal activity tracking by adding a new Redux action to manage subprocess status per terminal, a utility function to derive subprocess state from terminal events, and integrating event listeners into the root component to dispatch state updates. Changes
Sequence DiagramsequenceDiagram
participant TerminalAPI as Terminal API
participant Root as Root Component
participant Utility as terminalRunningSubprocessFromEvent
participant Store as Redux Store
participant State as Thread State
TerminalAPI->>Root: onEvent (TerminalEvent)
Root->>Utility: terminalRunningSubprocessFromEvent(event)
Utility-->>Root: boolean | null
Root->>Store: dispatch SET_THREAD_TERMINAL_ACTIVITY
Store->>Store: reducer: setThreadTerminalActivity()
Store->>State: update thread.runningTerminalIds
State-->>Root: state updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
juliusmarminge
force-pushed
the
codething/2860c0a0
branch
from
454ee7e to
051f352
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/store.ts (1)
379-398: Avoid no-op state churn on repeated terminal activity events.Lines 379-398 and Lines 566-572 currently rebuild thread/state even when
hasRunningSubprocessdoesn’t change. On high-frequency terminal events, this can trigger unnecessary rerenders.♻️ Proposed refactor (short-circuit unchanged updates)
function setThreadTerminalActivity( thread: Thread, terminalId: string, hasRunningSubprocess: boolean, ): Thread { - const normalizedThread = normalizeThreadTerminals(thread); - if (!normalizedThread.terminalIds.includes(terminalId)) { - return normalizedThread; + if (!thread.terminalIds.includes(terminalId)) { + return thread; } - const runningTerminalIds = new Set(normalizedThread.runningTerminalIds); - if (hasRunningSubprocess) { - runningTerminalIds.add(terminalId); - } else { - runningTerminalIds.delete(terminalId); + const isRunning = thread.runningTerminalIds.includes(terminalId); + if (isRunning === hasRunningSubprocess) { + return thread; } + + const normalizedThread = normalizeThreadTerminals(thread); + const runningTerminalIds = hasRunningSubprocess + ? [...new Set([...normalizedThread.runningTerminalIds, terminalId])] + : normalizedThread.runningTerminalIds.filter((id) => id !== terminalId); + return normalizeThreadTerminals({ ...normalizedThread, - runningTerminalIds: [...runningTerminalIds], + runningTerminalIds, }); } @@ - case "SET_THREAD_TERMINAL_ACTIVITY": - return { - ...state, - threads: updateThread(state.threads, action.threadId, (thread) => - setThreadTerminalActivity(thread, action.terminalId, action.hasRunningSubprocess), - ), - }; + case "SET_THREAD_TERMINAL_ACTIVITY": { + let changed = false; + const threads = state.threads.map((thread) => { + if (thread.id !== action.threadId) return thread; + const nextThread = setThreadTerminalActivity( + thread, + action.terminalId, + action.hasRunningSubprocess, + ); + changed ||= nextThread !== thread; + return nextThread; + }); + return changed ? { ...state, threads } : state; + }Also applies to: 566-572
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/store.ts` around lines 379 - 398, setThreadTerminalActivity is rebuilding the Thread object on every terminal event even when the runningTerminalIds set doesn’t change; fix by short-circuiting: compute currentHasRunning = normalizedThread.runningTerminalIds.includes(terminalId) and if currentHasRunning === hasRunningSubprocess return normalizedThread immediately (no spread/normalize) otherwise proceed to add/delete and return the updated normalized thread; apply the same short-circuit pattern to the analogous terminal-activity updater that also manipulates runningTerminalIds (the other function handling terminal activity in this file) and reuse normalizeThreadTerminals consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/store.ts`:
- Around line 379-398: setThreadTerminalActivity is rebuilding the Thread object
on every terminal event even when the runningTerminalIds set doesn’t change; fix
by short-circuiting: compute currentHasRunning =
normalizedThread.runningTerminalIds.includes(terminalId) and if
currentHasRunning === hasRunningSubprocess return normalizedThread immediately
(no spread/normalize) otherwise proceed to add/delete and return the updated
normalized thread; apply the same short-circuit pattern to the analogous
terminal-activity updater that also manipulates runningTerminalIds (the other
function handling terminal activity in this file) and reuse
normalizeThreadTerminals consistently.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/src/routes/__root.tsxapps/web/src/store.test.tsapps/web/src/store.tsapps/web/src/terminalActivity.test.tsapps/web/src/terminalActivity.ts
juliusmarminge
force-pushed
the
codething/2860c0a0
branch
from
051f352 to
0ef1bce
Compare
- subscribe to terminal events in root route and dispatch terminal activity updates - add reducer support to maintain `runningTerminalIds` per thread/terminal - add unit tests for terminal event mapping and store activity state transitions
juliusmarminge
force-pushed
the
codething/2860c0a0
branch
from
0ef1bce to
c901709
Compare
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
1 participant
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
SET_THREAD_TERMINAL_ACTIVITYreducer action to maintainrunningTerminalIdsper threadterminalRunningSubprocessFromEventhelper to map terminal events to running/idle/no-op activity signalsTesting
apps/web/src/store.test.ts: verifies add/remove/ignore behavior forrunningTerminalIdsapps/web/src/terminalActivity.test.ts: verifies activity parsing foractivity,started/restarted/exited, and ignored event typesNote
Medium Risk
Adds a new terminal event subscription and reducer path that continuously mutates per-thread
runningTerminalIds, which can affect UI behavior that depends on terminal busy/idle state. Risk is moderate due to new event-driven state updates, but it’s scoped to client-side state and covered by focused tests.Overview
Tracks per-terminal subprocess activity in client state. The root
EventRouternow subscribes toapi.terminal.onEvent, maps terminal events to a running/idle signal, and dispatches a newSET_THREAD_TERMINAL_ACTIVITYaction.The store reducer adds
setThreadTerminalActivityto maintain each thread’srunningTerminalIds(ignoring unknown terminal IDs and normalizing terminal state). AddsterminalRunningSubprocessFromEventplus Vitest coverage for both the event mapping and reducer add/remove/ignore behavior.Written by Cursor Bugbot for commit c901709. This will update automatically on new commits. Configure here.
Note
Track per-thread terminal subprocess activity and dispatch
SET_THREAD_TERMINAL_ACTIVITYfromEventRouterin __root.tsxAdd terminal event subscription that derives subprocess activity via
terminalActivity.terminalRunningSubprocessFromEventand updates thread state through the reducer, with tests covering event mapping and store updates.📍Where to Start
Start with
EventRouterin apps/web/src/routes/__root.tsx, then reviewterminalActivity.terminalRunningSubprocessFromEventin apps/web/src/terminalActivity.ts and theSET_THREAD_TERMINAL_ACTIVITYcase in apps/web/src/store.ts.Macroscope summarized c901709.
Summary by CodeRabbit
New Features
Tests