From edd66450db2db7ed26f648a88adfc76ac9aae9aa Mon Sep 17 00:00:00 2001 From: Tom Owers Date: Sat, 19 Sep 2026 14:02:05 +0200 Subject: [PATCH] fix(orchestrator): report the steps a failed dependency stopped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A task still pending when the drain ends never ran and never will: a dependency failed, so `nextRunnable` can never return it. Nothing marks it terminal, so it emits no completed/skipped/failed event at all — a step the user was offered and accepted disappears from the funnel rather than showing up as an outcome. The abort message had the same gap, reporting only how many steps never ran and not which. Emit `orchestrator task blocked` per still-pending task at drain end, and name those steps in the abort message. The queue is left as it is: the run cache is already wiped by this point, and writing to it would recreate the folder the cleanup just removed. Co-Authored-By: Claude Opus 5 Generated-By: PostHog Desktop Task-Id: e3b8a0c8-f668-4172-950b-91242cb09e59 --- .../__tests__/optional-tasks.test.ts | 47 +++++++++++++++++- .../orchestrator/orchestrator-runner.ts | 49 +++++++++++++++++-- 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/src/lib/agent/runner/sequence/orchestrator/__tests__/optional-tasks.test.ts b/src/lib/agent/runner/sequence/orchestrator/__tests__/optional-tasks.test.ts index 85e130bce..1e4e2d7f4 100644 --- a/src/lib/agent/runner/sequence/orchestrator/__tests__/optional-tasks.test.ts +++ b/src/lib/agent/runner/sequence/orchestrator/__tests__/optional-tasks.test.ts @@ -8,7 +8,10 @@ vi.mock('@utils/analytics', () => ({ })); import { QueueStore } from '@lib/agent/runner/sequence/orchestrator/queue'; -import { drainVerdict } from '@lib/agent/runner/sequence/orchestrator/orchestrator-runner'; +import { + describeDrainFailure, + drainVerdict, +} from '@lib/agent/runner/sequence/orchestrator/orchestrator-runner'; describe('drainVerdict', () => { let dir: string; @@ -52,5 +55,47 @@ describe('drainVerdict', () => { const v = drainVerdict(store.list()); expect(v.requiredFailedTypes).toEqual(['install']); expect(v.blocked).toBe(1); + expect(v.blockedTypes).toEqual(['report']); + }); + + it('names a step the user accepted that never became runnable', () => { + const install = store.enqueue({ type: 'install' }); + store.enqueue({ + type: 'warehouse', + optional: true, + dependsOn: [install.id], + }); + finish(install.id, false); + + expect(drainVerdict(store.list()).blockedTypes).toEqual(['warehouse']); + }); +}); + +describe('describeDrainFailure', () => { + it('names the failure and the steps it stopped', () => { + expect( + describeDrainFailure({ + requiredFailedTypes: ['install'], + blockedTypes: ['warehouse', 'report'], + }), + ).toBe('the install step failed, so the warehouse, report steps never ran'); + }); + + it('names blocked steps when nothing failed outright', () => { + expect( + describeDrainFailure({ + requiredFailedTypes: [], + blockedTypes: ['warehouse'], + }), + ).toBe('the warehouse step never ran'); + }); + + it('names the failure alone when nothing was left pending', () => { + expect( + describeDrainFailure({ + requiredFailedTypes: ['report'], + blockedTypes: [], + }), + ).toBe('the report step failed'); }); }); diff --git a/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts b/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts index 8778431ad..fcbf4c15b 100644 --- a/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts +++ b/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts @@ -370,8 +370,10 @@ export function drainVerdict(tasks: readonly QueuedTask[]): { requiredFailedTypes: string[]; optionalFailedTypes: string[]; blocked: number; + blockedTypes: string[]; } { const failed = tasks.filter((t) => t.status === TaskStatus.Failed); + const pending = tasks.filter((t) => t.status === TaskStatus.Pending); return { requiredFailedTypes: failed .filter((t) => t.optional !== true) @@ -379,10 +381,37 @@ export function drainVerdict(tasks: readonly QueuedTask[]): { optionalFailedTypes: failed .filter((t) => t.optional === true) .map((t) => t.type), - blocked: tasks.filter((t) => t.status === TaskStatus.Pending).length, + blocked: pending.length, + blockedTypes: pending.map((t) => t.type), }; } +/** + * The one-line "what went wrong" the abort message leads with. + * + * Both halves are named. A drain that ends with work still pending used to + * report only how many steps never ran, which is the least useful fact about + * them: a user who agreed to connect their data sources and then read that + * "2 steps never ran" had no way to tell whether that step was one of them. + */ +export function describeDrainFailure(verdict: { + requiredFailedTypes: string[]; + blockedTypes: string[]; +}): string { + const parts: string[] = []; + if (verdict.requiredFailedTypes.length > 0) { + parts.push(`the ${verdict.requiredFailedTypes.join(', ')} step failed`); + } + if (verdict.blockedTypes.length > 0) { + parts.push( + `the ${verdict.blockedTypes.join(', ')} step${ + verdict.blockedTypes.length === 1 ? '' : 's' + } never ran`, + ); + } + return parts.join(', so '); +} + /** How many tasks deep in the graph a task sits — 0 when it depends on nothing. */ function graphDepth( task: QueuedTask, @@ -1155,11 +1184,23 @@ export async function runOrchestrator( // A failed optional task is exempt: reported per-task, never run-failing. const verdict = drainVerdict(store.list()); const blocked = verdict.blocked; + // A pending task at this point never ran and never will — its dependency + // failed. No transition fires for it, so without this the step leaves no + // terminal event at all: a step the user was offered and accepted simply + // drops out of the funnel. The queue itself is left alone, because the run + // cache is already wiped by here and writing to it would recreate the folder + // the cleanup just removed. + for (const task of store.list()) { + if (task.status !== TaskStatus.Pending) continue; + analytics.wizardCapture('orchestrator task blocked', { + type: task.type, + optional: task.optional === true, + failed_types: verdict.requiredFailedTypes.join(',') || 'none', + }); + } if (verdict.requiredFailedTypes.length > 0 || blocked > 0) { const failedTypes = verdict.requiredFailedTypes.join(', '); - const whatFailed = failedTypes - ? `the ${failedTypes} step failed` - : `${blocked} steps never ran`; + const whatFailed = describeDrainFailure(verdict); // A grant narrowed at login is the one failure cause the user can fix // alone — lead with the fix, and only fall back to the report-a-bug line // when trying again doesn't work.