Skip to content

Commit 4c52e11

Browse files
committed
Improve task status reliability
1 parent d4d856e commit 4c52e11

5 files changed

Lines changed: 197 additions & 17 deletions

File tree

src/components/TerminalView.tsx

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,6 @@ interface TerminalViewProps {
8383
isFocused?: boolean;
8484
}
8585

86-
// Status parsing only needs recent output. Capping forwarded bytes avoids
87-
// expensive full-chunk decoding during large terminal bursts.
88-
const STATUS_ANALYSIS_MAX_BYTES = 8 * 1024;
89-
9086
/** Terminal-layer bindings — filtered from resolved bindings.
9187
* Called in the key handler (hot path); resolveBindings walks the full
9288
* defaults list on each call, which is fine at human typing speed. */
@@ -416,11 +412,6 @@ export function TerminalView(props: TerminalViewProps) {
416412
}
417413
}
418414

419-
const statusPayload =
420-
payload.length > STATUS_ANALYSIS_MAX_BYTES
421-
? payload.subarray(payload.length - STATUS_ANALYSIS_MAX_BYTES)
422-
: payload;
423-
424415
outputWriteInFlight = true;
425416
// eslint-disable-next-line solid/reactivity -- write callback is not a reactive context
426417
term.write(payload, () => {
@@ -436,7 +427,6 @@ export function TerminalView(props: TerminalViewProps) {
436427
});
437428
}
438429

439-
props.onData?.(statusPayload);
440430
if (outputQueue.length > 0) {
441431
scheduleOutputFlush();
442432
return;
@@ -458,6 +448,9 @@ export function TerminalView(props: TerminalViewProps) {
458448
}
459449

460450
function enqueueOutput(chunk: Uint8Array) {
451+
// Status analysis runs before xterm rendering/backpressure and receives
452+
// full chunks so UTF-8 decoder state stays valid across PTY boundaries.
453+
props.onData?.(chunk);
461454
outputQueue.push(chunk);
462455
outputQueuedBytes += chunk.length;
463456
watermark += chunk.length;

src/store/taskStatus.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import {
5656
isAutoTrustSettling,
5757
isAgentAskingQuestion,
5858
getTaskAttentionState,
59+
getTaskDotStatus,
5960
taskNeedsAttention,
6061
markAgentSpawned,
6162
markAgentOutput,
@@ -528,12 +529,83 @@ describe('task attention state', () => {
528529
it('returns ready for committed clean tasks without active attention', () => {
529530
setMockTask('task-1', { agentIds: ['agent-1'] });
530531
setMockAgent('agent-1', { status: 'running' });
532+
vi.setSystemTime(new Date('2026-05-10T10:00:00Z'));
531533
mockTaskGitStatus['task-1'] = {
532534
has_committed_changes: true,
533535
has_uncommitted_changes: false,
536+
current_branch: 'task/example',
537+
refreshedAt: Date.now(),
534538
};
535539

536540
expect(getTaskAttentionState('task-1')).toBe('ready');
541+
expect(getTaskDotStatus('task-1')).toBe('ready');
542+
expect(taskNeedsAttention('task-1')).toBe(false);
543+
});
544+
545+
it('does not report ready from a stale git status snapshot', () => {
546+
setMockTask('task-1', { agentIds: ['agent-1'] });
547+
setMockAgent('agent-1', { status: 'running' });
548+
vi.setSystemTime(new Date('2026-05-10T10:00:00Z'));
549+
mockTaskGitStatus['task-1'] = {
550+
has_committed_changes: true,
551+
has_uncommitted_changes: false,
552+
current_branch: 'task/example',
553+
refreshedAt: Date.now() - 6 * 60_000,
554+
};
555+
556+
expect(getTaskAttentionState('task-1')).toBe('idle');
557+
expect(getTaskDotStatus('task-1')).toBe('waiting');
558+
expect(taskNeedsAttention('task-1')).toBe(false);
559+
});
560+
561+
it('does not report ready from a failed git status refresh', () => {
562+
setMockTask('task-1', { agentIds: ['agent-1'] });
563+
setMockAgent('agent-1', { status: 'running' });
564+
vi.setSystemTime(new Date('2026-05-10T10:00:00Z'));
565+
mockTaskGitStatus['task-1'] = {
566+
has_committed_changes: true,
567+
has_uncommitted_changes: false,
568+
current_branch: 'task/example',
569+
refreshedAt: Date.now(),
570+
error: 'worktree missing',
571+
};
572+
573+
expect(getTaskAttentionState('task-1')).toBe('idle');
574+
expect(getTaskDotStatus('task-1')).toBe('waiting');
575+
expect(taskNeedsAttention('task-1')).toBe(false);
576+
});
577+
578+
it('does not report ready while a git status refresh is pending', () => {
579+
setMockTask('task-1', { agentIds: ['agent-1'] });
580+
setMockAgent('agent-1', { status: 'running' });
581+
vi.setSystemTime(new Date('2026-05-10T10:00:00Z'));
582+
mockTaskGitStatus['task-1'] = {
583+
has_committed_changes: true,
584+
has_uncommitted_changes: false,
585+
current_branch: 'task/example',
586+
refreshedAt: Date.now(),
587+
refreshing: true,
588+
};
589+
590+
expect(getTaskAttentionState('task-1')).toBe('idle');
591+
expect(getTaskDotStatus('task-1')).toBe('waiting');
592+
expect(taskNeedsAttention('task-1')).toBe(false);
593+
});
594+
595+
it('does not report ready after a git status snapshot has been marked stale', () => {
596+
setMockTask('task-1', { agentIds: ['agent-1'] });
597+
setMockAgent('agent-1', { status: 'running' });
598+
vi.setSystemTime(new Date('2026-05-10T10:00:00Z'));
599+
mockTaskGitStatus['task-1'] = {
600+
has_committed_changes: true,
601+
has_uncommitted_changes: false,
602+
current_branch: 'task/example',
603+
refreshedAt: Date.now(),
604+
stale: true,
605+
};
606+
607+
expect(getTaskAttentionState('task-1')).toBe('idle');
608+
expect(getTaskDotStatus('task-1')).toBe('waiting');
537609
expect(taskNeedsAttention('task-1')).toBe(false);
538610
});
539611

src/store/taskStatus.ts

Lines changed: 112 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { invoke } from '../lib/ipc';
33
import { IPC } from '../../electron/ipc/channels';
44
import { store, setStore } from './core';
55
import type { WorktreeStatus } from '../ipc/types';
6+
import type { TaskGitStatusSnapshot } from './types';
67
import { warn as logWarn } from '../lib/log';
78

89
// --- Trust-specific patterns (subset of QUESTION_PATTERNS) ---
@@ -754,7 +755,9 @@ export function clearAgentActivity(agentId: string): void {
754755

755756
function isTaskReady(taskId: string): boolean {
756757
const git = store.taskGitStatus[taskId];
757-
return Boolean(git?.has_committed_changes && !git?.has_uncommitted_changes);
758+
return Boolean(
759+
isGitStatusUsable(git) && git.has_committed_changes && !git.has_uncommitted_changes,
760+
);
758761
}
759762

760763
function hasTaskAgentError(taskId: string): boolean {
@@ -818,18 +821,121 @@ export function getTaskDotStatus(taskId: string): TaskDotStatus {
818821

819822
// --- Git status polling ---
820823

821-
async function refreshTaskGitStatus(taskId: string): Promise<void> {
824+
const GIT_STATUS_STALE_MS = 5 * 60_000;
825+
const gitRefreshVersions = new Map<string, number>();
826+
const gitStatusStaleTimers = new Map<string, ReturnType<typeof setTimeout>>();
827+
828+
function gitStatusErrorMessage(err: unknown): string {
829+
return err instanceof Error ? err.message : String(err);
830+
}
831+
832+
function isGitStatusUsable(git: TaskGitStatusSnapshot | undefined): git is TaskGitStatusSnapshot {
833+
return Boolean(
834+
git &&
835+
!git.error &&
836+
!git.refreshing &&
837+
!git.stale &&
838+
Date.now() - git.refreshedAt <= GIT_STATUS_STALE_MS,
839+
);
840+
}
841+
842+
function nextGitRefreshVersion(taskId: string): number {
843+
const version = (gitRefreshVersions.get(taskId) ?? 0) + 1;
844+
gitRefreshVersions.set(taskId, version);
845+
return version;
846+
}
847+
848+
function isCurrentGitRefresh(taskId: string, version: number): boolean {
849+
return gitRefreshVersions.get(taskId) === version;
850+
}
851+
852+
function clearGitStatusStaleTimer(taskId: string): void {
853+
const timer = gitStatusStaleTimers.get(taskId);
854+
if (timer !== undefined) {
855+
clearTimeout(timer);
856+
gitStatusStaleTimers.delete(taskId);
857+
}
858+
}
859+
860+
function scheduleGitStatusStaleTimer(taskId: string, refreshedAt: number): void {
861+
clearGitStatusStaleTimer(taskId);
862+
const delay = Math.max(0, refreshedAt + GIT_STATUS_STALE_MS - Date.now() + 1);
863+
const timer = setTimeout(() => {
864+
gitStatusStaleTimers.delete(taskId);
865+
const current = store.taskGitStatus[taskId];
866+
if (!store.tasks[taskId] || current?.refreshedAt !== refreshedAt) return;
867+
if (current.error || current.refreshing || current.stale) return;
868+
setStore('taskGitStatus', taskId, { ...current, stale: true });
869+
}, delay);
870+
gitStatusStaleTimers.set(taskId, timer);
871+
}
872+
873+
export function clearTaskGitStatusTracking(taskId: string): void {
874+
clearGitStatusStaleTimer(taskId);
875+
gitRefreshVersions.delete(taskId);
876+
}
877+
878+
async function refreshTaskGitStatus(
879+
taskId: string,
880+
options: { invalidateExisting?: boolean } = {},
881+
): Promise<void> {
822882
const task = store.tasks[taskId];
823883
if (!task || task.gitIsolation === 'none') return;
884+
const version = nextGitRefreshVersion(taskId);
885+
886+
if (options.invalidateExisting) {
887+
clearGitStatusStaleTimer(taskId);
888+
const previous = store.taskGitStatus[taskId];
889+
setStore(
890+
'taskGitStatus',
891+
taskId,
892+
previous
893+
? { ...previous, refreshing: true, stale: false }
894+
: {
895+
has_committed_changes: false,
896+
has_uncommitted_changes: false,
897+
current_branch: null,
898+
refreshedAt: 0,
899+
refreshing: true,
900+
},
901+
);
902+
}
824903

825904
try {
826905
const status = await invoke<WorktreeStatus>(IPC.GetWorktreeStatus, {
827906
worktreePath: task.worktreePath,
828907
baseBranch: task.baseBranch,
829908
});
830-
setStore('taskGitStatus', taskId, status);
831-
} catch {
832-
// Worktree may not exist yet or was removed — ignore
909+
if (!store.tasks[taskId] || !isCurrentGitRefresh(taskId, version)) return;
910+
const refreshedAt = Date.now();
911+
const next: TaskGitStatusSnapshot = {
912+
...status,
913+
refreshedAt,
914+
};
915+
setStore('taskGitStatus', taskId, next);
916+
scheduleGitStatusStaleTimer(taskId, refreshedAt);
917+
} catch (err) {
918+
if (!store.tasks[taskId] || !isCurrentGitRefresh(taskId, version)) return;
919+
clearGitStatusStaleTimer(taskId);
920+
const previous = store.taskGitStatus[taskId];
921+
setStore(
922+
'taskGitStatus',
923+
taskId,
924+
previous
925+
? {
926+
...previous,
927+
refreshing: false,
928+
error: gitStatusErrorMessage(err),
929+
}
930+
: {
931+
has_committed_changes: false,
932+
has_uncommitted_changes: false,
933+
current_branch: null,
934+
refreshedAt: 0,
935+
refreshing: false,
936+
error: gitStatusErrorMessage(err),
937+
},
938+
);
833939
}
834940
}
835941

@@ -877,7 +983,7 @@ async function refreshActiveTaskGitStatus(): Promise<void> {
877983

878984
/** Refresh git status for a single task (e.g. after agent exits). */
879985
export function refreshTaskStatus(taskId: string): void {
880-
refreshTaskGitStatus(taskId);
986+
refreshTaskGitStatus(taskId, { invalidateExisting: true });
881987
}
882988

883989
let allTasksTimer: ReturnType<typeof setInterval> | null = null;

src/store/tasks.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
markAgentSpawned,
1111
markAgentBusy,
1212
clearAgentActivity,
13+
clearTaskGitStatusTracking,
1314
isAgentIdle,
1415
rescheduleTaskStatusPolling,
1516
} from './taskStatus';
@@ -360,6 +361,7 @@ function removeTaskFromStore(taskId: string, agentIds: string[]): void {
360361

361362
// Phase 2: actually delete after animation completes
362363
setTimeout(() => {
364+
clearTaskGitStatusTracking(taskId);
363365
setStore(
364366
produce((s) => {
365367
delete s.tasks[taskId];

src/store/types.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ export type KeybindingOverride = Partial<Pick<KeyBinding, 'key' | 'modifiers'>>
88

99
export type GitIsolationMode = 'worktree' | 'direct' | 'none';
1010

11+
export interface TaskGitStatusSnapshot extends WorktreeStatus {
12+
refreshedAt: number;
13+
error?: string;
14+
refreshing?: boolean;
15+
stale?: boolean;
16+
}
17+
1118
export type TaskViewportVisibility = 'visible' | 'offscreen-left' | 'offscreen-right';
1219

1320
export interface TerminalBookmark {
@@ -200,7 +207,7 @@ export interface AppStore {
200207
* flex-absorbing. */
201208
panelUserSize: Record<string, number>;
202209
globalScale: number;
203-
taskGitStatus: Record<string, WorktreeStatus>;
210+
taskGitStatus: Record<string, TaskGitStatusSnapshot>;
204211
taskViewportVisibility: Record<string, TaskViewportVisibility>;
205212
focusedPanel: Record<string, PanelId>;
206213
sidebarFocused: boolean;

0 commit comments

Comments
 (0)