fix(tabs): keep the in-progress indicator on parallel threads - #1319
Open
pedramamini wants to merge 1 commit into
Open
fix(tabs): keep the in-progress indicator on parallel threads#1319pedramamini wants to merge 1 commit into
pedramamini wants to merge 1 commit into
Conversation
With several threads running under one agent, a thread could keep working
while its tab chip showed no yellow in-progress dot.
Two causes, both in the Auto Run exit reducer in spawnAgentForSession:
1. When the exit dequeued a queued item, it appended the user log and
focused the target tab but never set that tab's `state: 'busy'` /
`thinkingStartTime`. The turn ran with no indicator, and because the
same branch sets `activeTabId`, the affected thread was the focused
one. The three sibling dispatch paths (useQueueProcessing,
useAgentExitListener, useBatchHandlers) all mark the tab busy; this
copy was the outlier. Route it through the shared
markTabRunningQueuedItem helper instead, which also picks up the
forceParallel/readOnly log flags the hand-rolled entry dropped.
2. With an empty queue, the exit force-idled EVERY busy tab and the
session. A batch task spawns under its own `-batch-` process id and
deliberately never marks a tab busy, so any tab busy at that moment
has its own live `-ai-{tabId}` agent and is cleared by that agent's
own onExit. Derive session state from whether a tab is still busy,
mirroring the same pattern in useAgentExitListener.
Adds regression coverage for both branches plus the orphaned-tab case;
all three fail against the previous reducer.
Closes #1318
📝 WalkthroughWalkthroughAuto Run batch exit handling now preserves still-running tab state, dispatches queued prompts through shared tab helpers, updates orphaned targets, and clears session activity metadata only when no AI tabs remain busy. Regression tests cover parallel, queued, orphaned, and idle outcomes. ChangesAuto Run exit state handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR corrects Auto Run exit handling for parallel agent tabs.
Confidence Score: 4/5The orphan-only running case must be fixed before merging because a batch exit can still mark a session idle while its closed tab’s agent is executing. The new reducer correctly preserves busy live tabs, but its state derivation excludes orphanedThinkingTabs even though busy tabs are deliberately moved there and the sibling exit reducer treats them as active work. Files Needing Attention: src/renderer/hooks/agent/useAgentExecution.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
E[Auto Run batch exits] --> Q{Runnable queued item?}
Q -->|Yes| T[Resolve live or orphan target]
T --> M[Mark target busy and dispatch item]
Q -->|No| B{Any live or orphan tab busy?}
B -->|Yes| K[Keep session busy]
B -->|No| I[Set session idle]
Reviews (1): Last reviewed commit: "fix(tabs): keep the in-progress indicato..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // still running in parallel - that drops the in-progress indicator | ||
| // from threads that are very much still working. Each of those tabs | ||
| // is cleared by its own onExit handler. | ||
| const anyTabStillBusy = s.aiTabs?.some((tab) => tab.state === 'busy') ?? false; |
There was a problem hiding this comment.
When an Auto Run batch exits with an empty queue while the only running agent belongs to a closed tab, anyTabStillBusy checks only aiTabs and marks the session idle even though orphanedThinkingTabs still contains active work, causing session-level running indicators and filters to report that no work is active.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a regression where AI tabs running in parallel under the same agent could lose their in-progress indicator, especially on the focused tab, due to inconsistent busy-state updates in the Auto Run onExit reducer.
Changes:
- Route dequeued execution-queue items through
markTabRunningQueuedItemso the target tab is marked busy and gets the correct user-log fields. - Stop Auto Run batch exit from force-idling all busy tabs, preserving parallel thread indicators.
- Add focused regression tests covering dequeue, empty-queue, and orphaned-tab exit behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/renderer/hooks/agent/useAgentExecution.ts | Fixes Auto Run onExit reducer to correctly mark dequeued targets busy and avoid clearing parallel busy tabs. |
| src/tests/renderer/hooks/useAgentExecution.test.ts | Updates an existing batch spawn test setup to reflect that batch runs do not mark tabs busy. |
| src/tests/renderer/hooks/agent/useAgentExecution.exitQueueState.test.tsx | Adds regression coverage for the queue-exit reducer behaviors that caused missing in-progress indicators. |
Comments suppressed due to low confidence (1)
src/tests/renderer/hooks/agent/useAgentExecution.exitQueueState.test.tsx:131
installMaestroMock()replaceswindow.maestro, but the file never restores it after each test. Adding anafterEachrestoration (matching the pattern used inuseAgentExecution.test.ts) helps avoid leaking this stub into other renderer tests.
describe('useAgentExecution - Auto Run exit vs. parallel tab busy state', () => {
beforeEach(() => {
vi.clearAllMocks();
installMaestroMock();
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| state: 'idle' as SessionState, | ||
| busySource: undefined, | ||
| thinkingStartTime: undefined, | ||
| state: anyTabStillBusy ? s.state : ('idle' as SessionState), |
Comment on lines
+17
to
+21
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { renderHook, act } from '@testing-library/react'; | ||
| import { createMockSession, createMockAITab } from '../../../helpers'; | ||
| import type { Session, QueuedItem } from '../../../../renderer/types'; | ||
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/hooks/agent/useAgentExecution.ts`:
- Around line 436-449: Update the busy-state gate in the no-queued-items branch
to include busy entries from orphanedThinkingTabs alongside s.aiTabs. Preserve
the existing session field clearing behavior only when neither visible nor
orphaned tabs remain busy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 40c86361-9fe2-439c-b99c-a8b48197bc87
📒 Files selected for processing (3)
src/__tests__/renderer/hooks/agent/useAgentExecution.exitQueueState.test.tsxsrc/__tests__/renderer/hooks/useAgentExecution.test.tssrc/renderer/hooks/agent/useAgentExecution.ts
Comment on lines
+436
to
449
| // No queued items. This spawn ran under its own `-batch-` process id | ||
| // and deliberately never marked a tab busy (see the spawn site), so | ||
| // its exit must not clear tabs whose own `-ai-{tabId}` agents are | ||
| // still running in parallel - that drops the in-progress indicator | ||
| // from threads that are very much still working. Each of those tabs | ||
| // is cleared by its own onExit handler. | ||
| const anyTabStillBusy = s.aiTabs?.some((tab) => tab.state === 'busy') ?? false; | ||
|
|
||
| return { | ||
| ...s, | ||
| state: 'idle' as SessionState, | ||
| busySource: undefined, | ||
| thinkingStartTime: undefined, | ||
| state: anyTabStillBusy ? s.state : ('idle' as SessionState), | ||
| busySource: anyTabStillBusy ? s.busySource : undefined, | ||
| thinkingStartTime: anyTabStillBusy ? s.thinkingStartTime : undefined, | ||
| pendingAICommandForSynopsis: undefined, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find where orphanedThinkingTabs / session busy state are consumed together
rg -n "orphanedThinkingTabs" -C4 src/renderer | head -150
rg -nP "aiTabs\??:\s*AITab" src/renderer/types* -C3Repository: RunMaestro/Maestro
Length of output: 13398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== useAgentExecution relevant lines =="
sed -n '350,465p' src/renderer/hooks/agent/useAgentExecution.ts | nl -ba -v350
echo
echo "== tests mentioning orphaned thinking tabs / busy state =="
rg -n "orphaned|ThinkingTabs|busySource|thinkingStartTime|idle" src --glob '*test*.tsx' --glob '*test*.ts' -C3 | head -240
echo
echo "== consumers of session state/busySource/thinkingStartTime =="
rg -n "busySource|thinkingStartTime|\\.state\\s*=\\s*['\"]idle|state === 'idle'|state \\?\\? .*idle|useAgentExecutor|useSession|busy" src/renderer -C2 | head -360Repository: RunMaestro/Maestro
Length of output: 235
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== useAgentExecution relevant lines =="
python3 - <<'PY'
from pathlib import Path
p=Path("src/renderer/hooks/agent/useAgentExecution.ts")
lines=p.read_text().splitlines()
for i in range(349,min(len(lines),470)):
print(f"{i+1}: {lines[i]}")
PY
echo
echo "== tests mentioning orphaned thinking tabs / busy state =="
rg -n "orphaned|ThinkingTabs|busySource|thinkingStartTime|idle" src --glob '*test*.tsx' --glob '*test*.ts' -C3 | head -360 || true
echo
echo "== consumers of session state/busySource/thinkingStartTime =="
rg -n "busySource|thinkingStartTime|session\\.state|state:\\s*['\"]idle|state === 'idle'|state \\?\\? .*idle|\\.orphanedThinkingTabs" src/renderer -C2 | head -400 || trueRepository: RunMaestro/Maestro
Length of output: 50374
Include orphanedThinkingTabs in the busy-state gate.
anyTabStillBusy only checks s.aiTabs, so when all visible AI tabs are idle while an orphaned tab is still busy, this branch clears the session-level state/busySource/thinkingStartTime even though the orphaned send is still executing.
🐛 Proposed fix
- const anyTabStillBusy = s.aiTabs?.some((tab) => tab.state === 'busy') ?? false;
+ const anyTabStillBusy =
+ (s.aiTabs?.some((tab) => tab.state === 'busy') ?? false) ||
+ (s.orphanedThinkingTabs?.some((tab) => tab.state === 'busy') ?? false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // No queued items. This spawn ran under its own `-batch-` process id | |
| // and deliberately never marked a tab busy (see the spawn site), so | |
| // its exit must not clear tabs whose own `-ai-{tabId}` agents are | |
| // still running in parallel - that drops the in-progress indicator | |
| // from threads that are very much still working. Each of those tabs | |
| // is cleared by its own onExit handler. | |
| const anyTabStillBusy = s.aiTabs?.some((tab) => tab.state === 'busy') ?? false; | |
| return { | |
| ...s, | |
| state: 'idle' as SessionState, | |
| busySource: undefined, | |
| thinkingStartTime: undefined, | |
| state: anyTabStillBusy ? s.state : ('idle' as SessionState), | |
| busySource: anyTabStillBusy ? s.busySource : undefined, | |
| thinkingStartTime: anyTabStillBusy ? s.thinkingStartTime : undefined, | |
| pendingAICommandForSynopsis: undefined, | |
| // No queued items. This spawn ran under its own `-batch-` process id | |
| // and deliberately never marked a tab busy (see the spawn site), so | |
| // its exit must not clear tabs whose own `-ai-{tabId}` agents are | |
| // still running in parallel - that drops the in-progress indicator | |
| // from threads that are very much still working. Each of those tabs | |
| // is cleared by its own onExit handler. | |
| const anyTabStillBusy = | |
| (s.aiTabs?.some((tab) => tab.state === 'busy') ?? false) || | |
| (s.orphanedThinkingTabs?.some((tab) => tab.state === 'busy') ?? false); | |
| return { | |
| ...s, | |
| state: anyTabStillBusy ? s.state : ('idle' as SessionState), | |
| busySource: anyTabStillBusy ? s.busySource : undefined, | |
| thinkingStartTime: anyTabStillBusy ? s.thinkingStartTime : undefined, | |
| pendingAICommandForSynopsis: undefined, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/hooks/agent/useAgentExecution.ts` around lines 436 - 449, Update
the busy-state gate in the no-queued-items branch to include busy entries from
orphanedThinkingTabs alongside s.aiTabs. Preserve the existing session field
clearing behavior only when neither visible nor orphaned tabs remain busy.
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.
Closes #1318
Problem
With multiple threads running under a single agent, a thread could keep working while its tab chip showed no yellow in-progress dot. The reporter saw 3 threads running and only 2 indicators.
The reporter's support package confirms the setup: three live agent processes on one agent (
{sessionId}-ai-{tabId}, uptimes 26m / 12m / 9m). Their screenshot shows the indicator missing specifically on the focused tab, while two background tabs still had theirs.Root cause
Both causes are in the Auto Run exit reducer inside
spawnAgentForSession(src/renderer/hooks/agent/useAgentExecution.ts).1. Dequeue branch never marked the target tab busy.
When the exit dequeued a queued item, it appended the user log and set
activeTabId: target.tabId, but never set that tab'sstate: 'busy'/thinkingStartTime. The turn ran with no indicator - and because the same branch focuses the tab, the affected thread was the one in view, matching the screenshot exactly.The three sibling dispatch paths all mark the tab busy; this copy was the outlier:
useQueueProcessing.dispatchQueuedItemmarkTabRunningQueuedItemuseAgentExitListeneronExitmarkTabRunningQueuedItemuseBatchHandlersqueue drainuseAgentExecutiononExitNow routed through the shared
markTabRunningQueuedItemhelper, which also picks up theforceParallel/readOnlylog flags the hand-rolled entry dropped.2. Empty-queue branch force-idled every busy tab.
With no queued items, the exit set
state: 'idle'on ALL busy tabs and on the session. A batch task spawns under its own{sessionId}-batch-{ts}process id and, per the note at its own spawn site, deliberately never marks a tab busy. So any tab busy at that moment has its own live-ai-{tabId}agent and is cleared by that agent's ownonExit. Session state is now derived from whether a tab is still busy, mirroring the same pattern already used inuseAgentExitListener.Testing
New
useAgentExecution.exitQueueState.test.tsxcovers the empty-queue, dequeue, and orphaned-tab branches. Three of its four cases fail against the previous reducer:One assertion in the existing
useAgentExecution.test.tsseeded an artificially busy tab and asserted the batch exit cleared it - that codified cause 2. Updated to seed an idle tab (the real batch case, since Auto Run never marks a tab busy), with a pointer to the new file for the still-running case.npm run lint(all three tsconfigs): cleannpx eslinton changed files: cleanLocal validation is macOS-only; leaving the Ubuntu and Windows CI legs to confirm.
Summary by CodeRabbit
Bug Fixes
Tests