diff --git a/src/__tests__/wizard-abort.test.ts b/src/__tests__/wizard-abort.test.ts index 8a72e0cf..b35bc2c7 100644 --- a/src/__tests__/wizard-abort.test.ts +++ b/src/__tests__/wizard-abort.test.ts @@ -2,6 +2,7 @@ import { wizardAbort, WizardError, + abortFingerprint, registerCleanup, clearCleanup, runCleanups, @@ -108,7 +109,9 @@ describe('wizardAbort', () => { await expect(wizardAbort({ error })).rejects.toThrow('process.exit called'); - expect(mockAnalytics.captureException).toHaveBeenCalledWith(error, {}); + expect(mockAnalytics.captureException).toHaveBeenCalledWith(error, { + $exception_fingerprint: 'wizard_abort_something_broke', + }); expect(mockAnalytics.shutdown).toHaveBeenCalledWith('error'); }); @@ -127,11 +130,24 @@ describe('wizardAbort', () => { await expect(wizardAbort({ error })).rejects.toThrow('process.exit called'); expect(mockAnalytics.captureException).toHaveBeenCalledWith(error, { + $exception_fingerprint: 'wizard_abort_mcp_missing', integration: 'nextjs', error_type: 'MCP_MISSING', }); }); + it('lets a WizardError context override the default fingerprint', async () => { + const error = new WizardError('OAuth error: timeout', { + $exception_fingerprint: 'wizard_oauth_timeout', + }); + + await expect(wizardAbort({ error })).rejects.toThrow('process.exit called'); + + expect(mockAnalytics.captureException).toHaveBeenCalledWith(error, { + $exception_fingerprint: 'wizard_oauth_timeout', + }); + }); + it('runs registered cleanup functions before analytics and display', async () => { const callOrder: string[] = []; @@ -178,6 +194,56 @@ describe('wizardAbort', () => { }); }); +describe('abortFingerprint', () => { + it('is identical for the same cause raised from different install paths', () => { + const fromNpx = new WizardError( + 'orchestrator drain ended with failed tasks', + ); + fromNpx.stack = + 'WizardError\n at /home/u/.npm/_npx/2f0a/node_modules/@posthog/wizard/dist/index.js:1:1'; + const fromPnpmDlx = new WizardError( + 'orchestrator drain ended with failed tasks', + ); + fromPnpmDlx.stack = + 'WizardError\n at /root/.cache/pnpm/dlx/9b7c1f/node_modules/@posthog/wizard/dist/index.js:1:1'; + + expect(abortFingerprint(fromNpx)).toBe(abortFingerprint(fromPnpmDlx)); + expect(abortFingerprint(fromNpx)).toBe( + 'wizard_abort_orchestrator_drain_ended_with_failed_tasks', + ); + }); + + it('separates the failed-tasks guard from the never-ran guard', () => { + expect( + abortFingerprint( + new WizardError('orchestrator drain ended with failed tasks'), + ), + ).not.toBe( + abortFingerprint( + new WizardError('orchestrator drain ended with tasks that never ran'), + ), + ); + }); + + it('truncates long messages so a shared cause stays one group', () => { + const suffix = + 'API Error: The socket connection was closed unexpectedly. For more information, pass `verbose: true`'; + const once = new WizardError(`API error: ${suffix}`); + const twice = new WizardError(`API error: ${suffix}\n${suffix}`); + + expect(abortFingerprint(once)).toBe(abortFingerprint(twice)); + // Prefix + 80 chars of slug, and never a trailing separator. + expect(abortFingerprint(once).length).toBeLessThanOrEqual( + 'wizard_abort_'.length + 80, + ); + expect(abortFingerprint(once)).not.toMatch(/_$/); + }); + + it('falls back to the error name when there is no message', () => { + expect(abortFingerprint(new TypeError(''))).toBe('wizard_abort_typeerror'); + }); +}); + describe('runCleanups', () => { beforeEach(() => { clearCleanup(); diff --git a/src/lib/agent/runner/sequence/orchestrator/__tests__/drain-failure.test.ts b/src/lib/agent/runner/sequence/orchestrator/__tests__/drain-failure.test.ts new file mode 100644 index 00000000..f6fe205a --- /dev/null +++ b/src/lib/agent/runner/sequence/orchestrator/__tests__/drain-failure.test.ts @@ -0,0 +1,45 @@ +import { drainFailure } from '../drain-failure'; + +describe('drainFailure', () => { + it('returns nothing when the drain finished everything', () => { + expect( + drainFailure({ failed: 0, blocked: 0, failedTypes: '' }), + ).toBeUndefined(); + }); + + it('reports failed tasks when retries were exhausted', () => { + expect( + drainFailure({ failed: 1, blocked: 0, failedTypes: 'instrument' }), + ).toEqual({ + error: 'orchestrator drain ended with failed tasks', + reason: 'the instrument step failed', + }); + }); + + it('does not call a blocked-only drain a failure', () => { + const failure = drainFailure({ failed: 0, blocked: 3, failedTypes: '' }); + + expect(failure?.error).toBe( + 'orchestrator drain ended with tasks that never ran', + ); + expect(failure?.error).not.toContain('failed'); + expect(failure?.reason).toBe('3 steps never ran'); + }); + + it('keeps the reason grammatical for a single blocked task', () => { + expect( + drainFailure({ failed: 0, blocked: 1, failedTypes: '' })?.reason, + ).toBe('1 step never ran'); + }); + + it('prefers the failure when tasks both failed and stayed blocked', () => { + // Blocked tasks are usually downstream of the failure, so the failed types + // are the actionable detail. + expect( + drainFailure({ failed: 1, blocked: 2, failedTypes: 'review' }), + ).toEqual({ + error: 'orchestrator drain ended with failed tasks', + reason: 'the review step failed', + }); + }); +}); diff --git a/src/lib/agent/runner/sequence/orchestrator/drain-failure.ts b/src/lib/agent/runner/sequence/orchestrator/drain-failure.ts new file mode 100644 index 00000000..2c39fd7e --- /dev/null +++ b/src/lib/agent/runner/sequence/orchestrator/drain-failure.ts @@ -0,0 +1,38 @@ +/** + * Which guard, if any, should fail an orchestrator run once the drain returns. + * + * A drain can end short two ways, and they are different defects: a task + * exhausted its retries (`Failed`), or a task never ran at all because a + * dependency never completed (still `Pending` with nothing runnable). Both mean + * the wizard did not set PostHog up, so both abort — but the exception has to + * name the one that happened. A blocked-only drain has nothing in `Failed`, so + * reporting it as "failed tasks" sent triage hunting for a failure the queue + * state does not contain. + */ +export type DrainFailure = { + /** Stable exception message — also the error-tracking group key. */ + error: string; + /** The one-liner the user sees, minus the contact line. */ + reason: string; +}; + +export function drainFailure(args: { + failed: number; + blocked: number; + /** Comma-joined types of the tasks in `Failed`, for the user-facing reason. */ + failedTypes: string; +}): DrainFailure | undefined { + if (args.failed > 0) { + return { + error: 'orchestrator drain ended with failed tasks', + reason: `the ${args.failedTypes} step failed`, + }; + } + if (args.blocked > 0) { + return { + error: 'orchestrator drain ended with tasks that never ran', + reason: `${args.blocked} step${args.blocked === 1 ? '' : 's'} never ran`, + }; + } + return undefined; +} diff --git a/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts b/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts index 3b623ef8..196339b6 100644 --- a/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts +++ b/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts @@ -54,6 +54,7 @@ import { type QueuedTask, } from './queue'; import { drainQueue, type RunTask } from './executor'; +import { drainFailure } from './drain-failure'; import { RunMetrics } from './run-metrics'; import { agentRunTools, @@ -620,22 +621,24 @@ export async function runOrchestrator( const notRequired = summary[TaskStatus.Skipped]; // A drain that ends with failed tasks (retries exhausted) or tasks still - // pending (blocked behind a failed dependency) did NOT set PostHog up — - // abort like a linear agent failure instead of claiming success. + // pending (blocked behind a dependency that never completed) did NOT set + // PostHog up — abort like a linear agent failure instead of claiming success. + // `drainFailure` picks which of the two it was so the exception says so. const blocked = summary[TaskStatus.Pending]; - if (summary.failed > 0 || blocked > 0) { - const failedTypes = store - .list() - .filter((t) => t.status === TaskStatus.Failed) - .map((t) => t.type) - .join(', '); + const failedTypes = store + .list() + .filter((t) => t.status === TaskStatus.Failed) + .map((t) => t.type) + .join(', '); + const failure = drainFailure({ + failed: summary.failed, + blocked, + failedTypes, + }); + if (failure) { await wizardAbort({ - message: `The wizard was unable to set up PostHog: ${ - failedTypes - ? `the ${failedTypes} step failed` - : `${blocked} steps never ran` - }.\n\nPlease report this to: ${WIZARD_CONTACT_EMAIL}`, - error: new WizardError('orchestrator drain ended with failed tasks', { + message: `The wizard was unable to set up PostHog: ${failure.reason}.\n\nPlease report this to: ${WIZARD_CONTACT_EMAIL}`, + error: new WizardError(failure.error, { tasks_failed: summary.failed, tasks_blocked: blocked, failed_types: failedTypes, diff --git a/src/utils/wizard-abort.ts b/src/utils/wizard-abort.ts index fe844728..94339c07 100644 --- a/src/utils/wizard-abort.ts +++ b/src/utils/wizard-abort.ts @@ -29,6 +29,34 @@ interface WizardAbortOptions { exitCode?: number; } +/** + * Stable error-tracking group key for an abort. + * + * Error tracking fingerprints server-side from the stack trace, and every + * install path is different — pnpm dlx content hashes, `.npm/_npx` dirs, CI + * checkouts, local clones. So one cause shatters into an issue per install, + * each reading "1 occurrence, 1 user, 1 session", and the real recurrence rate + * becomes unmeasurable. Aborts are raised from a fixed set of guard sites, so + * the error's own message is the stable identity; supplying it collapses those + * variants into one issue. + * + * Truncation is deliberate: it folds together messages that share a cause but + * differ in their tail (a repeated API error suffix, an inlined quota URL). + * + * A callsite that wants different grouping sets `$exception_fingerprint` in the + * WizardError context — that wins over this default. + */ +export function abortFingerprint(error: Error): string { + const slug = (value: string) => + value + .toLowerCase() + .replace(/[^a-z0-9]+/g, '_') + .replace(/^_+|_+$/g, '') + .slice(0, 80) + .replace(/_+$/, ''); + return `wizard_abort_${slug(error.message) || slug(error.name) || 'unknown'}`; +} + const cleanupFns: Array<() => void> = []; export function registerCleanup(fn: () => void): void { @@ -72,6 +100,7 @@ export async function wizardAbort( // 2. Capture error in analytics (if provided) if (error) { analytics.captureException(error, { + $exception_fingerprint: abortFingerprint(error), ...((error instanceof WizardError && error.context) || {}), }); }