diff --git a/src/workflow/__tests__/selectors.test.ts b/src/workflow/__tests__/selectors.test.ts index c67cc5d55..fca0179a4 100644 --- a/src/workflow/__tests__/selectors.test.ts +++ b/src/workflow/__tests__/selectors.test.ts @@ -63,6 +63,57 @@ test('mergePhases: actual but undeclared phase appended to the end', () => { expect(mergePhases(r).map(p => p.title)).toEqual(['Find', 'Adhoc']) }) +// Regression: scripts that pass opts.phase directly to agent() without a phase() hook call +// (the ultracode canonical pipeline pattern). phase_started is never emitted for those phases, +// so run.phases lacks them. The sidebar used to show them as pending forever while agents were +// clearly running under them — and worse, the previous phase stayed "running" because phase_done +// only fires on the next phase() call. Derive status from agents when no actual record exists. +test('mergePhases: derives status from agents when phase_started was never emitted', () => { + // Mirrors the real .claude/workflow-runs/wnxct9u3q/script.js shape: + // phase('Map') called, 8 Map agents done; pipeline stage with phase:'Find' running (1/4); + // Verify / Synthesize declared but not started; phase('Synthesize') not yet reached so + // phase_done Map has not fired either — actual Map is still 'running'. + const r = run({ + declaredPhases: ['Map', 'Find', 'Verify', 'Synthesize'], + phases: [{ title: 'Map', status: 'running' }], + agents: [ + ...Array.from({ length: 8 }, (_, i) => ({ + id: i, + phase: 'Map', + status: 'done' as const, + resultKind: 'ok', + })), + { id: 100, phase: 'Find', status: 'done', resultKind: 'ok' }, + { id: 101, phase: 'Find', status: 'running' }, + { id: 102, phase: 'Find', status: 'running' }, + { id: 103, phase: 'Find', status: 'running' }, + ], + }) + expect(mergePhases(r)).toEqual([ + { title: 'Map', status: 'done', done: 8, total: 8 }, + { title: 'Find', status: 'running', done: 1, total: 4 }, + { title: 'Verify', status: 'pending', done: 0, total: 0 }, + { title: 'Synthesize', status: 'pending', done: 0, total: 0 }, + ]) +}) + +// A phase that appears only on agents (not in declaredPhases, not in run.phases) is still +// surfaced so the user sees it in the sidebar. +test('mergePhases: phase only present on agents is appended and derived from agent states', () => { + const r = run({ + declaredPhases: ['Scan'], + phases: [], + agents: [ + { id: 1, phase: 'AdhocFromAgent', status: 'running' }, + { id: 2, phase: 'AdhocFromAgent', status: 'done', resultKind: 'ok' }, + ], + }) + expect(mergePhases(r)).toEqual([ + { title: 'Scan', status: 'pending', done: 0, total: 0 }, + { title: 'AdhocFromAgent', status: 'running', done: 1, total: 2 }, + ]) +}) + test('filterAgentsByPhase: All / undefined → all; specified → only that phase', () => { const agents: AgentProgress[] = [ { id: 1, phase: 'A', status: 'running' }, diff --git a/src/workflow/panel/selectors.ts b/src/workflow/panel/selectors.ts index 0d18eddf5..b7d7345c9 100644 --- a/src/workflow/panel/selectors.ts +++ b/src/workflow/panel/selectors.ts @@ -13,9 +13,40 @@ export type MergedPhase = { } /** - * Merge declaredPhases (declared by meta) and run.phases (actually running/done): - * - Declared order takes priority; phases present in actual but not declared are appended at the end. - * - No actual record -> pending; otherwise take the actual status. + * Derive a phase's sidebar status from the actual record + the agents grouped under it. + * + * The actual record comes from `phase_started`/`phase_done` events. Scripts that follow the + * ultracode canonical pipeline pattern pass `opts.phase` directly to `agent()` inside + * `pipeline()`/`parallel()` stages and never call `phase()` for those phases — so no + * `phase_started` ever fires and `run.phases` lacks them. Worse, because `phase_done` only + * emits when the *next* `phase()` runs, the previous phase stays "running" in `run.phases` + * even after all its agents finish. + * + * Rules (checked in order): + * 1. `phase_done` already fired → done is authoritative, respect it. + * 2. Agents exist under this phase → derive from their states + * (all done → done; otherwise → running). This is what the user actually sees. + * 3. No agents yet → fall back to the actual record + * (`running` if `phase()` was called and is still active, else pending). + */ +function derivePhaseStatus( + actual: { status: 'running' | 'done' } | undefined, + inPhase: AgentProgress[], +): PhaseStatus { + if (actual?.status === 'done') return 'done' + if (inPhase.length > 0) { + return inPhase.every(a => a.status === 'done') ? 'done' : 'running' + } + return actual?.status === 'running' ? 'running' : 'pending' +} + +/** + * Merge declaredPhases (declared by meta), run.phases (actually running/done), + * and phases that appear only on agents: + * - Declared order takes priority; then actual-but-undeclared; then agent-only phases. + * Agent-only phases surface in the sidebar even when the script never called `phase()` + * for them — otherwise the user sees agents running under a phase that isn't listed. + * - Status is derived via {@link derivePhaseStatus}. * - done/total = done under that phase / total agents under that phase. */ export function mergePhases( @@ -28,17 +59,22 @@ export function mergePhases( if (seen.has(title)) return seen.add(title) const actual = actualByTitle.get(title) - const status: PhaseStatus = !actual ? 'pending' : actual.status const inPhase = run.agents.filter(a => a.phase === title) out.push({ title, - status, + status: derivePhaseStatus(actual, inPhase), done: inPhase.filter(a => a.status === 'done').length, total: inPhase.length, }) } for (const t of run.declaredPhases) push(t) for (const p of run.phases) push(p.title) + // Scripts that pass opts.phase directly to agent() (the ultracode pipeline pattern) + // may have agents grouped under phases that never got a phase() call — surface them + // so the sidebar reflects every phase the user can actually observe agents running in. + for (const a of run.agents) { + if (a.phase) push(a.phase) + } return out }