From 580c5069b42606125463eb206cf70ce607af5956 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 13:45:40 -0400 Subject: [PATCH 01/12] refactor(shared): move the skill menu out of the agent Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- .../architecture/known-violations.json | 1 - src/__tests__/programs-cli.test.ts | 6 +- src/__tests__/skill-cli.test.ts | 6 +- .../__tests__/run-agent-standalone.test.ts | 4 +- src/agent/__tests__/wizard-tools.test.ts | 45 ---------- src/agent/runner/harness/pi/tools.ts | 2 +- .../__tests__/variant-resolution.test.ts | 3 +- .../orchestrator/orchestrator-runner.ts | 7 +- src/agent/tools/mcp.ts | 3 +- src/agent/tools/tools.ts | 90 +------------------ .../factories/family-command-factory.ts | 2 +- src/commands/skill.ts | 2 +- src/lib/__tests__/wizard-spellbook.test.ts | 8 +- src/lib/programs/dispatch-family.ts | 2 +- src/lib/wizard-spellbook.ts | 4 +- src/shared/__tests__/skill-menu.test.ts | 90 +++++++++++++++++++ src/shared/skill-menu.ts | 90 +++++++++++++++++++ src/ui/tui/__tests__/frames.test.tsx | 5 +- .../__tests__/keyboard-equivalence.test.tsx | 4 +- src/ui/tui/__tests__/skill-entry.test.ts | 2 +- src/ui/tui/screens/SkillSourceInfo.tsx | 2 +- .../tui/screens/health/HealthCheckScreen.tsx | 3 +- 22 files changed, 215 insertions(+), 166 deletions(-) create mode 100644 src/shared/__tests__/skill-menu.test.ts create mode 100644 src/shared/skill-menu.ts diff --git a/src/__tests__/architecture/known-violations.json b/src/__tests__/architecture/known-violations.json index 1f0f7bd21..0eecf3f85 100644 --- a/src/__tests__/architecture/known-violations.json +++ b/src/__tests__/architecture/known-violations.json @@ -66,7 +66,6 @@ "src/steps/upload-environment-variables/index.ts -> src/telemetry.ts", "src/ui/agent-progress.ts -> src/agent/progress.ts", "src/ui/index.ts -> src/ui/logging-ui.ts", - "src/ui/tui/screens/SkillSourceInfo.tsx -> src/agent/tools/index.ts", "src/ui/tui/screens/health/HealthCheckScreen.tsx -> src/agent/tools/index.ts", "src/ui/tui/services/mcp-suggested-prompts-services.ts -> src/agent/mcp-prompt-streaming.ts", "src/ui/wizard-ui.ts -> src/agent/progress.ts" diff --git a/src/__tests__/programs-cli.test.ts b/src/__tests__/programs-cli.test.ts index 9f24ddd10..c7a5e074e 100644 --- a/src/__tests__/programs-cli.test.ts +++ b/src/__tests__/programs-cli.test.ts @@ -8,8 +8,8 @@ vi.mock('@lib/runners', () => ({ runWizardCI: mockRunWizardCI, })); -vi.mock('@agent/tools', async (importOriginal) => { - const actual = await importOriginal(); +vi.mock('@shared/skill-menu', async (importOriginal) => { + const actual = await importOriginal(); return { ...actual, fetchSkillMenu: vi.fn(), @@ -31,7 +31,7 @@ import { pickerChildrenToShow, } from '@lib/programs/dispatch-family'; import type { Command } from '../commands/command'; -import { fetchSkillMenu, type CliEntry } from '@agent/tools'; +import { fetchSkillMenu, type CliEntry } from '@shared/skill-menu'; import { auditConfig } from '@lib/programs/audit/index'; import { webAnalyticsDoctorConfig } from '@lib/programs/web-analytics-doctor/index'; import { parseCommand } from './helpers/parse-command.no-jest'; diff --git a/src/__tests__/skill-cli.test.ts b/src/__tests__/skill-cli.test.ts index aed48a047..676023cf1 100644 --- a/src/__tests__/skill-cli.test.ts +++ b/src/__tests__/skill-cli.test.ts @@ -4,13 +4,13 @@ vi.mock('../commands/basic-integration/skill', () => ({ runSkillMode: vi.fn(), })); -vi.mock('@agent/tools', async (importOriginal) => { - const actual = await importOriginal(); +vi.mock('@shared/skill-menu', async (importOriginal) => { + const actual = await importOriginal(); return { ...actual, fetchSkillMenu: vi.fn() }; }); import { runSkillMode } from '../commands/basic-integration/skill'; -import { fetchSkillMenu } from '@agent/tools'; +import { fetchSkillMenu } from '@shared/skill-menu'; import { analytics } from '@utils/analytics'; import { skillCommand } from '../commands/skill'; import { parseCommand } from './helpers/parse-command.no-jest'; diff --git a/src/agent/__tests__/run-agent-standalone.test.ts b/src/agent/__tests__/run-agent-standalone.test.ts index 9e436dbe4..d03bd9890 100644 --- a/src/agent/__tests__/run-agent-standalone.test.ts +++ b/src/agent/__tests__/run-agent-standalone.test.ts @@ -174,8 +174,8 @@ vi.mock('@agent/agent-prompt-loader', async (original) => { ), }; }); -vi.mock('@agent/tools', async (original) => ({ - ...(await original()), +vi.mock('@shared/skill-menu', async (original) => ({ + ...(await original()), fetchSkillMenu: vi.fn().mockResolvedValue({ categories: {} }), })); diff --git a/src/agent/__tests__/wizard-tools.test.ts b/src/agent/__tests__/wizard-tools.test.ts index 40bbd129e..d61a953b1 100644 --- a/src/agent/__tests__/wizard-tools.test.ts +++ b/src/agent/__tests__/wizard-tools.test.ts @@ -21,7 +21,6 @@ import { ensureGitignoreCoverage, describeAskCancellation, evaluateAskCap, - fetchSkillMenu, mergeEnvValues, normaliseAskSubject, parseEnvKeys, @@ -1398,47 +1397,3 @@ describe('downloadSkill (e2e over HTTP)', () => { } }); }); - -describe('fetchSkillMenu', () => { - const noSleep = () => Promise.resolve(); - const menu = { categories: { integration: [] } }; - const menuResponse = () => - Promise.resolve({ - ok: true, - status: 200, - statusText: 'OK', - json: () => Promise.resolve(menu), - }); - - it('retries a flaky menu fetch before succeeding', async () => { - let attempts = 0; - - const result = await fetchSkillMenu('http://localhost:8765', { - fetchImpl: (() => { - attempts += 1; - if (attempts < 3) return Promise.reject(new Error('reset')); - return menuResponse(); - }) as any, - sleepImpl: noSleep, - }); - - expect(attempts).toBe(3); - expect(result).toEqual(menu); - }); - - it('returns null after exhausting retries', async () => { - let attempts = 0; - - const result = await fetchSkillMenu('http://localhost:8765', { - fetchImpl: (() => { - attempts += 1; - return Promise.reject(new Error('network down')); - }) as any, - sleepImpl: noSleep, - maxAttempts: 3, - }); - - expect(attempts).toBe(3); - expect(result).toBeNull(); - }); -}); diff --git a/src/agent/runner/harness/pi/tools.ts b/src/agent/runner/harness/pi/tools.ts index 06c752160..a24daacc5 100644 --- a/src/agent/runner/harness/pi/tools.ts +++ b/src/agent/runner/harness/pi/tools.ts @@ -38,7 +38,6 @@ import { createAskAccounting, describeAskCancellation, ensureGitignoreCoverage, - fetchSkillMenu, installSkillById, mergeEnvValues, normaliseAskSubject, @@ -53,6 +52,7 @@ import { WIZARD_ASK_SUBJECT_DESCRIPTION, WIZARD_ASK_TOOL_DESCRIPTION, } from '@agent/tools/tools'; +import { fetchSkillMenu } from '@shared/skill-menu'; import type { LLMProvider } from '@posthog/warlock'; import type { ProgressEmitter } from '@agent/progress'; import { diff --git a/src/agent/runner/sequence/orchestrator/__tests__/variant-resolution.test.ts b/src/agent/runner/sequence/orchestrator/__tests__/variant-resolution.test.ts index afbc48a15..155c51e9c 100644 --- a/src/agent/runner/sequence/orchestrator/__tests__/variant-resolution.test.ts +++ b/src/agent/runner/sequence/orchestrator/__tests__/variant-resolution.test.ts @@ -1,8 +1,7 @@ import { describe, it, expect } from 'vitest'; import { resolveSkillVariantId } from '../orchestrator-runner'; import { Integration } from '@shared/constants'; -import { expandBundleEntry } from '@agent/tools'; -import type { SkillEntry } from '@agent/tools'; +import { expandBundleEntry, type SkillEntry } from '@shared/skill-menu'; // Pinned from the real built skill-menu.json, so this suite tests the actual cross-repo contract. const INTEGRATION_ENTRIES = [ diff --git a/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts b/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts index 7233f4b8f..2d28b1d61 100644 --- a/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts +++ b/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts @@ -24,11 +24,8 @@ import { import * as path from 'path'; import { OutroKind, type TaskNotice } from '@agent/progress'; import { POSTHOG_DOCS_URL, WIZARD_CONTACT_EMAIL } from '@shared/constants'; -import { - installSkillById, - fetchSkillMenu, - type SkillEntry, -} from '@agent/tools'; +import { installSkillById } from '@agent/tools'; +import { fetchSkillMenu, type SkillEntry } from '@shared/skill-menu'; import { analytics } from '@utils/analytics'; import { ciExcludedTaskTypes } from '@utils/ci-flag-overrides'; import { logToFile } from '@utils/debug'; diff --git a/src/agent/tools/mcp.ts b/src/agent/tools/mcp.ts index 84c4b79a6..ce01bc29e 100644 --- a/src/agent/tools/mcp.ts +++ b/src/agent/tools/mcp.ts @@ -45,7 +45,6 @@ import { ensureGitignoreCoverage, createAskAccounting, describeAskCancellation, - fetchSkillMenu, checkEnvKeys as checkEnvKeysCore, mergeEnvValues, normaliseAskSubject, @@ -57,7 +56,6 @@ import { templateEnvWriteRefusal, legacyKeyNameRefusal, vaultSensitiveAnswers, - type SkillEntry, AUDIT_ADD_CHECKS_DESCRIPTION, AUDIT_ADD_CHECKS_PARAM_DESCRIPTION, AUDIT_RESOLVE_CHECKS_DESCRIPTION, @@ -70,6 +68,7 @@ import { WIZARD_ASK_SUBJECT_DESCRIPTION, WIZARD_ASK_TOOL_DESCRIPTION, } from './tools'; +import { fetchSkillMenu, type SkillEntry } from '@shared/skill-menu'; const auditCheckSchema = z.object({ id: z.string().min(1), diff --git a/src/agent/tools/tools.ts b/src/agent/tools/tools.ts index c33ff5e80..7c1421017 100644 --- a/src/agent/tools/tools.ts +++ b/src/agent/tools/tools.ts @@ -32,26 +32,7 @@ import { import { CANCELLED_SENTINEL } from '../wizard-ask-bridge'; import type { SecretVault } from '@shared/secret-vault'; import { fetchWithRetry, type RetryOpts } from '@shared/fetch-retry'; - -// --------------------------------------------------------------------------- -// Skill types -// --------------------------------------------------------------------------- - -export type SkillEntry = { - id: string; - name: string; - downloadUrl: string; - /** The hyphenated skill-group prefix of `id` (e.g. `posthog-integration-install`). */ - group?: string; - /** The detection id this variant serves (e.g. `rails`, `react-router`). */ - framework?: string; - /** The variant a bare framework id resolves to when its family has several. */ - default?: boolean; - /** This entry's download is a bundle JSON of every variant, not a single skill's zip. */ - bundle?: boolean; - /** Menu-only: the variants inside a bundle, expanded into entries of their own on fetch. */ - variants?: { id: string; framework?: string; default?: boolean }[]; -}; +import { fetchSkillMenu, type SkillEntry } from '@shared/skill-menu'; /** A bundle's files, keyed by variant short id then path. */ export type SkillBundle = { @@ -59,75 +40,6 @@ export type SkillBundle = { variants: Record>; }; -/** - * Entry in the wizard's runtime CLI registry. Mirrors the shape context-mill - * publishes under `cliEntries` inside `skill-menu.json`. The wizard uses these - * to register skill-backed subcommands at runtime instead of from a baked - * build-time snapshot. - */ -export type CliEntry = { - skillId: string; - role: 'command' | 'skill' | 'internal'; - command?: string; - parentCommand?: string; - default?: boolean; - displayName: string; - description: string; -}; - -export interface SkillMenu { - categories: Record; - /** - * Skills exposed as CLI commands. Optional because context-mill releases - * older than the runtime-resolver cutover don't emit this field. - */ - cliEntries?: CliEntry[]; -} - -// --------------------------------------------------------------------------- -// Standalone skill helpers (usable before the MCP server is created) -// --------------------------------------------------------------------------- - -/** Expand a bundle entry into one entry per variant, so the menu reads the same whether a group ships bundled or as zips. */ -export function expandBundleEntry(entry: SkillEntry): SkillEntry[] { - if (!entry.bundle || !entry.variants) return [entry]; - return entry.variants.map((variant) => ({ - ...variant, - name: entry.name, - group: entry.group, - bundle: true, - downloadUrl: entry.downloadUrl, - })); -} - -/** - * Fetch the skill menu from the skills server. - * Returns parsed data on success, `null` on failure. - */ -export async function fetchSkillMenu( - skillsBaseUrl: string, - opts: RetryOpts = {}, -): Promise { - const menuUrl = `${skillsBaseUrl}/skill-menu.json`; - try { - logToFile(`fetchSkillMenu: fetching from ${menuUrl}`); - const resp = await fetchWithRetry(menuUrl, opts); - const data = (await resp.json()) as SkillMenu; - for (const [category, entries] of Object.entries(data.categories)) { - data.categories[category] = entries.flatMap(expandBundleEntry); - } - logToFile( - `fetchSkillMenu: loaded (${ - Object.keys(data.categories).length - } categories)`, - ); - return data; - } catch (err: any) { - logToFile(`fetchSkillMenu: error: ${err.message}`); - return null; - } -} - /** Extract a zip buffer, refusing entries that escape destDir (zip-slip). */ function extractZipArchive(zip: Uint8Array, destDir: string): number { const root = path.resolve(destDir); diff --git a/src/commands/factories/family-command-factory.ts b/src/commands/factories/family-command-factory.ts index 9ae0f164c..087bd5d21 100644 --- a/src/commands/factories/family-command-factory.ts +++ b/src/commands/factories/family-command-factory.ts @@ -7,7 +7,7 @@ import { pickerChildrenToShow, } from '@lib/programs/dispatch-family'; import { getSkillsBaseUrl } from '@shared/constants'; -import { fetchSkillMenu } from '@agent/tools'; +import { fetchSkillMenu } from '@shared/skill-menu'; import type { Command } from '../command'; import { createFamilyPickerDefault } from './family-picker'; diff --git a/src/commands/skill.ts b/src/commands/skill.ts index c72fb93ec..0ffb51adf 100644 --- a/src/commands/skill.ts +++ b/src/commands/skill.ts @@ -1,7 +1,7 @@ import type { Arguments } from 'yargs'; import { getSkillsBaseUrl } from '@shared/constants'; -import { fetchSkillMenu, type CliEntry } from '@agent/tools'; +import { fetchSkillMenu, type CliEntry } from '@shared/skill-menu'; import { analytics } from '@utils/analytics'; import { runSkillMode } from './basic-integration/skill'; diff --git a/src/lib/__tests__/wizard-spellbook.test.ts b/src/lib/__tests__/wizard-spellbook.test.ts index e23af172b..2f15a66b7 100644 --- a/src/lib/__tests__/wizard-spellbook.test.ts +++ b/src/lib/__tests__/wizard-spellbook.test.ts @@ -5,13 +5,17 @@ import { Integration } from '@shared/constants'; import type { ProgramConfig } from '../programs/program-step'; import { buildSession } from '../wizard-session'; import { writeWizardSpellbook } from '../wizard-spellbook'; -import { downloadSkill, fetchSkillMenu } from '@agent/tools/tools'; +import { downloadSkill } from '@agent/tools/tools'; +import { fetchSkillMenu } from '@shared/skill-menu'; vi.mock('@agent/tools/tools', async (importOriginal) => ({ ...(await importOriginal()), - fetchSkillMenu: vi.fn(), downloadSkill: vi.fn(), })); +vi.mock('@shared/skill-menu', async (importOriginal) => ({ + ...(await importOriginal()), + fetchSkillMenu: vi.fn(), +})); const program: ProgramConfig = { id: 'example-setup', diff --git a/src/lib/programs/dispatch-family.ts b/src/lib/programs/dispatch-family.ts index e36831674..f2922077b 100644 --- a/src/lib/programs/dispatch-family.ts +++ b/src/lib/programs/dispatch-family.ts @@ -7,7 +7,7 @@ import { agentSkillConfig } from '@lib/programs/program-registry'; import { webAnalyticsDoctorConfig } from '@lib/programs/web-analytics-doctor/index'; import type { ProgramConfig } from '@lib/programs/program-step'; import { getSkillsBaseUrl } from '@shared/constants'; -import { fetchSkillMenu, type CliEntry } from '@agent/tools'; +import { fetchSkillMenu, type CliEntry } from '@shared/skill-menu'; import { analytics } from '@utils/analytics'; import { dispatchProgram } from '../../commands/factories/shared'; diff --git a/src/lib/wizard-spellbook.ts b/src/lib/wizard-spellbook.ts index 894ae1f6e..be7105e51 100644 --- a/src/lib/wizard-spellbook.ts +++ b/src/lib/wizard-spellbook.ts @@ -3,12 +3,12 @@ import path from 'path'; import { getSkillsBaseUrl, POSTHOG_DOCS_URL } from '@shared/constants'; import type { ProgramConfig } from './programs/program-step'; import type { WizardSession } from './wizard-session'; +import { downloadSkill } from '@agent/tools/tools'; import { - downloadSkill, fetchSkillMenu, type SkillEntry, type SkillMenu, -} from '@agent/tools/tools'; +} from '@shared/skill-menu'; export type WizardSpellbook = { path: string; skillsIncluded: boolean }; type SpellbookSession = Pick< diff --git a/src/shared/__tests__/skill-menu.test.ts b/src/shared/__tests__/skill-menu.test.ts new file mode 100644 index 000000000..bc5884f0d --- /dev/null +++ b/src/shared/__tests__/skill-menu.test.ts @@ -0,0 +1,90 @@ +import { fetchSkillMenu, type SkillMenu } from '@shared/skill-menu'; + +describe('fetchSkillMenu', () => { + const noSleep = () => Promise.resolve(); + const menu: SkillMenu = { categories: { integration: [] } }; + const respond = (body: unknown) => () => + Promise.resolve({ + ok: true, + status: 200, + statusText: 'OK', + json: () => Promise.resolve(body), + }); + + it('retries a flaky menu fetch before succeeding', async () => { + let attempts = 0; + + const result = await fetchSkillMenu('http://localhost:8765', { + fetchImpl: (() => { + attempts += 1; + if (attempts < 3) return Promise.reject(new Error('reset')); + return respond(menu)(); + }) as any, + sleepImpl: noSleep, + }); + + expect(attempts).toBe(3); + expect(result).toEqual(menu); + }); + + it('returns null after exhausting retries', async () => { + let attempts = 0; + + const result = await fetchSkillMenu('http://localhost:8765', { + fetchImpl: (() => { + attempts += 1; + return Promise.reject(new Error('network down')); + }) as any, + sleepImpl: noSleep, + maxAttempts: 3, + }); + + expect(attempts).toBe(3); + expect(result).toBeNull(); + }); + + it('expands a bundle entry into one entry per variant', async () => { + const bundled: SkillMenu = { + categories: { + integration: [ + { + id: 'capture', + name: 'Capture', + group: 'capture', + bundle: true, + downloadUrl: 'http://localhost:8765/capture.json', + variants: [ + { id: 'capture-rails', framework: 'rails', default: true }, + { id: 'capture-react', framework: 'react' }, + ], + }, + ], + }, + }; + + const result = await fetchSkillMenu('http://localhost:8765', { + fetchImpl: respond(bundled) as any, + sleepImpl: noSleep, + }); + + expect(result?.categories.integration).toEqual([ + { + id: 'capture-rails', + framework: 'rails', + default: true, + name: 'Capture', + group: 'capture', + bundle: true, + downloadUrl: 'http://localhost:8765/capture.json', + }, + { + id: 'capture-react', + framework: 'react', + name: 'Capture', + group: 'capture', + bundle: true, + downloadUrl: 'http://localhost:8765/capture.json', + }, + ]); + }); +}); diff --git a/src/shared/skill-menu.ts b/src/shared/skill-menu.ts new file mode 100644 index 000000000..41430e38c --- /dev/null +++ b/src/shared/skill-menu.ts @@ -0,0 +1,90 @@ +/** + * The skill menu: what context-mill publishes as `skill-menu.json` and how the + * wizard reads it. Skill-backed CLI commands, the family picker, the skill + * source screen and the agent's install path all resolve skills through this + * one fetch, so it lives in shared and depends on nothing in the agent. + */ + +import { logToFile } from '@utils/debug'; +import { fetchWithRetry, type RetryOpts } from '@shared/fetch-retry'; + +export type SkillEntry = { + id: string; + name: string; + downloadUrl: string; + /** The hyphenated skill-group prefix of `id` (e.g. `posthog-integration-install`). */ + group?: string; + /** The detection id this variant serves (e.g. `rails`, `react-router`). */ + framework?: string; + /** The variant a bare framework id resolves to when its family has several. */ + default?: boolean; + /** This entry's download is a bundle JSON of every variant, not a single skill's zip. */ + bundle?: boolean; + /** Menu-only: the variants inside a bundle, expanded into entries of their own on fetch. */ + variants?: { id: string; framework?: string; default?: boolean }[]; +}; + +/** + * Entry in the wizard's runtime CLI registry. Mirrors the shape context-mill + * publishes under `cliEntries` inside `skill-menu.json`. The wizard uses these + * to register skill-backed subcommands at runtime instead of from a baked + * build-time snapshot. + */ +export type CliEntry = { + skillId: string; + role: 'command' | 'skill' | 'internal'; + command?: string; + parentCommand?: string; + default?: boolean; + displayName: string; + description: string; +}; + +export interface SkillMenu { + categories: Record; + /** + * Skills exposed as CLI commands. Optional because context-mill releases + * older than the runtime-resolver cutover don't emit this field. + */ + cliEntries?: CliEntry[]; +} + +/** Expand a bundle entry into one entry per variant, so the menu reads the same whether a group ships bundled or as zips. */ +export function expandBundleEntry(entry: SkillEntry): SkillEntry[] { + if (!entry.bundle || !entry.variants) return [entry]; + return entry.variants.map((variant) => ({ + ...variant, + name: entry.name, + group: entry.group, + bundle: true, + downloadUrl: entry.downloadUrl, + })); +} + +/** + * Fetch the skill menu from the skills server. + * Returns parsed data on success, `null` on failure. + */ +export async function fetchSkillMenu( + skillsBaseUrl: string, + opts: RetryOpts = {}, +): Promise { + const menuUrl = `${skillsBaseUrl}/skill-menu.json`; + try { + logToFile(`fetchSkillMenu: fetching from ${menuUrl}`); + const resp = await fetchWithRetry(menuUrl, opts); + const data = (await resp.json()) as SkillMenu; + for (const [category, entries] of Object.entries(data.categories)) { + data.categories[category] = entries.flatMap(expandBundleEntry); + } + logToFile( + `fetchSkillMenu: loaded (${ + Object.keys(data.categories).length + } categories)`, + ); + return data; + } catch (err: any) { + logToFile(`fetchSkillMenu: error: ${err.message}`); + return null; + } +} diff --git a/src/ui/tui/__tests__/frames.test.tsx b/src/ui/tui/__tests__/frames.test.tsx index c24a782af..49bfb6062 100644 --- a/src/ui/tui/__tests__/frames.test.tsx +++ b/src/ui/tui/__tests__/frames.test.tsx @@ -52,9 +52,12 @@ vi.mock('@shared/api', async (actual) => ({ })); vi.mock('@agent/tools', async (actual) => ({ ...(await actual>()), - fetchSkillMenu: vi.fn(pending), downloadSkill: vi.fn(pending), })); +vi.mock('@shared/skill-menu', async (actual) => ({ + ...(await actual>()), + fetchSkillMenu: vi.fn(pending), +})); vi.mock('@ui/tui/hooks/useGithubConnection', () => ({ useGithubConnection: () => undefined, fetchLoginUrl: vi.fn().mockResolvedValue(null), diff --git a/src/ui/tui/__tests__/keyboard-equivalence.test.tsx b/src/ui/tui/__tests__/keyboard-equivalence.test.tsx index e240e36dd..1a5c27db2 100644 --- a/src/ui/tui/__tests__/keyboard-equivalence.test.tsx +++ b/src/ui/tui/__tests__/keyboard-equivalence.test.tsx @@ -66,8 +66,8 @@ vi.mock('@shared/api', async (importOriginal) => ({ fetchSlackConnected: vi.fn().mockResolvedValue(false), fetchUserData: vi.fn(() => new Promise(() => undefined)), })); -vi.mock('@agent/tools', async (importOriginal) => ({ - ...(await importOriginal()), +vi.mock('@shared/skill-menu', async (importOriginal) => ({ + ...(await importOriginal()), fetchSkillMenu: vi.fn(() => new Promise(() => undefined)), })); vi.mock('@utils/setup-utils', async (importOriginal) => ({ diff --git a/src/ui/tui/__tests__/skill-entry.test.ts b/src/ui/tui/__tests__/skill-entry.test.ts index 09f5d43a0..c12071aeb 100644 --- a/src/ui/tui/__tests__/skill-entry.test.ts +++ b/src/ui/tui/__tests__/skill-entry.test.ts @@ -1,5 +1,5 @@ import { resolveSkillEntry } from '@ui/tui/screens/SkillSourceInfo'; -import type { SkillEntry } from '@agent/tools'; +import type { SkillEntry } from '@shared/skill-menu'; const entry = (id: string): SkillEntry => ({ id, downloadUrl: `https://example.com/${id}.tar.gz` } as SkillEntry); diff --git a/src/ui/tui/screens/SkillSourceInfo.tsx b/src/ui/tui/screens/SkillSourceInfo.tsx index 7805f433c..83bf10366 100644 --- a/src/ui/tui/screens/SkillSourceInfo.tsx +++ b/src/ui/tui/screens/SkillSourceInfo.tsx @@ -14,7 +14,7 @@ import { Box, Text } from 'ink'; import { useEffect, useState } from 'react'; -import { fetchSkillMenu, type SkillEntry } from '@agent/tools'; +import { fetchSkillMenu, type SkillEntry } from '@shared/skill-menu'; import { CONTEXT_MILL_RELEASES_URL, getSkillsBaseUrl } from '@shared/constants'; /** diff --git a/src/ui/tui/screens/health/HealthCheckScreen.tsx b/src/ui/tui/screens/health/HealthCheckScreen.tsx index c19e6f5af..672113050 100644 --- a/src/ui/tui/screens/health/HealthCheckScreen.tsx +++ b/src/ui/tui/screens/health/HealthCheckScreen.tsx @@ -24,7 +24,8 @@ import { import { ServiceHealthStatus } from '@shared/health-checks/types'; import { wizardAbort } from '@utils/wizard-abort'; import { ErrorCodes } from '@shared/errors'; -import { fetchSkillMenu, downloadSkill } from '@agent/tools'; +import { downloadSkill } from '@agent/tools'; +import { fetchSkillMenu } from '@shared/skill-menu'; import { GITHUB_SKILLS_BASE_URL } from '@shared/constants'; import { useDismissOnAnyKey } from '@ui/tui/hooks/useDismissOnAnyKey'; From 6484ce11ce7cac2691313ab3ae120e8fefeb2ead Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 13:53:32 -0400 Subject: [PATCH 02/12] build(agent): publish the agent through entry modules `@agent` exports the runtime values callers use and `@agent/types` the types; everything outside `src/agent` imports one of the two. The MCP prompt streaming export loads on first call so the startup chunk does not grow. The architecture scanner enforces the entries on resolved paths and ESLint rejects deep `@agent/*` specifiers in the editor. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- .eslintrc.cjs | 21 ++++++ README.md | 4 +- .../architecture/import-boundaries.test.ts | 72 ++++++++++++++++++- .../architecture/known-violations.json | 37 +--------- src/agent/index.ts | 38 ++++++++++ src/agent/types.ts | 30 ++++++++ src/lib/detection/agentic.ts | 4 +- src/lib/programs/agent-skill/index.ts | 2 +- src/lib/programs/audit/detect.ts | 2 +- src/lib/programs/audit/index.ts | 2 +- src/lib/programs/dispatch-family.ts | 2 +- .../detect.ts | 2 +- .../prompt.ts | 2 +- src/lib/programs/events-audit/index.ts | 2 +- src/lib/programs/mcp-analytics/index.ts | 2 +- src/lib/programs/migration/index.ts | 4 +- src/lib/programs/posthog-doctor/index.ts | 2 +- src/lib/programs/posthog-integration/index.ts | 4 +- src/lib/programs/program-run.ts | 2 +- src/lib/programs/replay-vision/index.ts | 2 +- src/lib/programs/revenue-analytics/detect.ts | 2 +- src/lib/programs/revenue-analytics/index.ts | 2 +- src/lib/programs/run-agent-legacy.ts | 18 ++--- src/lib/programs/self-driving/detect.ts | 2 +- src/lib/programs/self-driving/prompt.ts | 4 +- src/lib/programs/warehouse-source/detect.ts | 2 +- src/lib/programs/warehouse-source/index.ts | 2 +- .../programs/web-analytics-doctor/detect.ts | 2 +- src/lib/runners/run-non-interactive.ts | 4 +- src/lib/wizard-session.ts | 19 ++--- src/lib/wizard-spellbook.ts | 2 +- src/shared/claude-settings.ts | 3 + src/shared/errors/agent-map.ts | 3 + src/shared/errors/skill-map.ts | 2 +- src/ui/agent-progress.ts | 2 +- .../tui/screens/health/HealthCheckScreen.tsx | 2 +- .../mcp-suggested-prompts-services.ts | 4 +- src/ui/wizard-ui.ts | 2 +- tsconfig.build.json | 2 + vitest.config.ts | 2 + 40 files changed, 226 insertions(+), 91 deletions(-) create mode 100644 src/agent/index.ts create mode 100644 src/agent/types.ts diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 849d0ed71..77f26a9b9 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -95,6 +95,27 @@ module.exports = { ], }, }, + { + // Outside the agent, import it through its entry modules only: + // `@agent` for values, `@agent/types` for types. The architecture test + // applies the same rule to resolved paths; this gives editor feedback. + files: ['bin.ts', 'src/**/*.ts', 'src/**/*.tsx'], + excludedFiles: ['src/agent/**', '**/__tests__/**', '**/__mocks__/**'], + rules: { + '@typescript-eslint/no-restricted-imports': [ + 'error', + { + patterns: [ + { + group: ['@agent/**', '!@agent/types'], + message: + 'Import the agent through @agent (values) or @agent/types (types).', + }, + ], + }, + ], + }, + }, { files: [ '*.test.js', diff --git a/README.md b/README.md index 8fbba68e9..b4756f113 100644 --- a/README.md +++ b/README.md @@ -504,7 +504,9 @@ Path aliases defined in `tsconfig.build.json`, resolved by tsdown: |---|---| | `@env` | `src/env.ts` | | `@lib/*` | `src/lib/*` | -| `@agent/*` | `src/agent/*` | +| `@agent` | `src/agent/index.ts`, the agent's runtime entry; the only agent import outside `src/agent` besides types | +| `@agent/types` | `src/agent/types.ts`, type-only | +| `@agent/*` | `src/agent/*`, inside `src/agent` and its tests only | | `@shared/*` | `src/shared/*` | | `@utils/*` | `src/shared/utils/*` | | `@ui/*` | `src/ui/*` | diff --git a/src/__tests__/architecture/import-boundaries.test.ts b/src/__tests__/architecture/import-boundaries.test.ts index dba4024a4..a7ba197ff 100644 --- a/src/__tests__/architecture/import-boundaries.test.ts +++ b/src/__tests__/architecture/import-boundaries.test.ts @@ -48,6 +48,28 @@ export const ALLOWED_IMPORTS: Record = { cli: ['env', 'legacy', 'agent', 'tui', 'cli'], }; +// The agent's public entries. Outside `src/agent`, an import into the agent +// must land on one of these; `types.ts` is type-only, so the TUI may take it. +const AGENT_VALUES_ENTRY = 'src/agent/index.ts'; +const AGENT_TYPES_ENTRY = 'src/agent/types.ts'; + +/** The rule an edge breaks, or null when it is allowed. */ +export function ruleFor(fromFile: string, toFile: string): string | null { + const from = classifySurface(fromFile); + const to = classifySurface(toFile); + if (to === 'agent' && from !== 'agent') { + const target = toFile.split(path.sep).join('/'); + if (target !== AGENT_VALUES_ENTRY && target !== AGENT_TYPES_ENTRY) { + return 'agent-deep-import'; + } + if (from === 'tui' && target !== AGENT_TYPES_ENTRY) { + return `matrix:${from}->${to}`; + } + return null; + } + return ALLOWED_IMPORTS[from].includes(to) ? null : `matrix:${from}->${to}`; +} + const TUI_ONLY_PACKAGES = ['ink', 'react', '@inkjs/ui', 'ink-testing-library']; const SKIP_DIRS = new Set([ @@ -267,7 +289,6 @@ function analyze(): Analysis { fs.readFileSync(path.join(REPO_ROOT, file), 'utf8'), ); const from = classifySurface(file); - const allowed = ALLOWED_IMPORTS[from]; for (const spec of specifiersIn(text)) { const base = spec.startsWith('.') @@ -293,8 +314,8 @@ function analyze(): Analysis { const key = `${file} -> ${target}`; edges.add(key); - const to = classifySurface(target); - if (!allowed.includes(to)) violations.set(key, `matrix:${from}->${to}`); + const broken = ruleFor(file, target); + if (broken !== null) violations.set(key, broken); } } @@ -392,3 +413,48 @@ describe('surface classification', () => { ).toBe('legacy'); }); }); + +describe('agent entry modules', () => { + const rule = (from: string, to: string) => ruleFor(from, to); + + it('lets legacy and cli code reach the agent through its entries only', () => { + expect(rule('src/lib/programs/audit/index.ts', 'src/agent/index.ts')).toBe( + null, + ); + expect(rule('src/lib/programs/audit/index.ts', 'src/agent/types.ts')).toBe( + null, + ); + expect(rule('src/commands/skill.ts', 'src/agent/index.ts')).toBe(null); + expect( + rule('src/lib/programs/audit/index.ts', 'src/agent/agent-runner.ts'), + ).toBe('agent-deep-import'); + expect(rule('src/commands/skill.ts', 'src/agent/runner/index.ts')).toBe( + 'agent-deep-import', + ); + expect(rule('src/shared/errors/agent-map.ts', 'src/agent/signals.ts')).toBe( + 'agent-deep-import', + ); + }); + + it('lets the TUI take agent types but not agent values', () => { + expect(rule('src/ui/tui/App.tsx', 'src/agent/types.ts')).toBe(null); + expect(rule('src/ui/tui/App.tsx', 'src/agent/index.ts')).toBe( + 'matrix:tui->agent', + ); + expect(rule('src/ui/tui/App.tsx', 'src/agent/progress.ts')).toBe( + 'agent-deep-import', + ); + }); + + it('leaves agent-internal and non-agent edges to the matrix', () => { + expect(rule('src/agent/runner/index.ts', 'src/agent/progress.ts')).toBe( + null, + ); + expect(rule('src/lib/programs/audit/index.ts', 'src/ui/tui/store.ts')).toBe( + 'matrix:legacy->tui', + ); + expect(rule('src/agent/runner/index.ts', 'src/ui/tui/store.ts')).toBe( + 'matrix:agent->tui', + ); + }); +}); diff --git a/src/__tests__/architecture/known-violations.json b/src/__tests__/architecture/known-violations.json index 0eecf3f85..ce41efa60 100644 --- a/src/__tests__/architecture/known-violations.json +++ b/src/__tests__/architecture/known-violations.json @@ -2,61 +2,30 @@ "violations": [ "src/commands/factories/family-picker.tsx -> src/commands/command.ts", "src/env.ts -> src/lib/headless-mode.ts", - "src/lib/detection/agentic.ts -> src/agent/agent-interface.ts", - "src/lib/programs/agent-skill/index.ts -> src/agent/agent-runner.ts", "src/lib/programs/agent-skill/index.ts -> src/lib/programs/agent-skill/content/index.tsx", "src/lib/programs/ai-observability/index.ts -> src/lib/programs/agent-skill/content/index.tsx", - "src/lib/programs/audit/detect.ts -> src/agent/agent-runner.ts", - "src/lib/programs/audit/index.ts -> src/agent/tools/index.ts", - "src/lib/programs/dispatch-family.ts -> src/agent/tools/index.ts", "src/lib/programs/dispatch-family.ts -> src/commands/command.ts", "src/lib/programs/dispatch-family.ts -> src/commands/factories/shared.ts", - "src/lib/programs/error-tracking-upload-source-maps/detect.ts -> src/agent/agent-runner.ts", "src/lib/programs/error-tracking-upload-source-maps/index.ts -> src/lib/programs/error-tracking-upload-source-maps/content/index.tsx", - "src/lib/programs/error-tracking-upload-source-maps/prompt.ts -> src/agent/agent-interface.ts", "src/lib/programs/error-tracking/index.ts -> src/lib/programs/error-tracking/content/index.tsx", "src/lib/programs/error-tracking/index.ts -> src/lib/programs/error-tracking/content/tips.ts", - "src/lib/programs/events-audit/index.ts -> src/agent/tools/index.ts", - "src/lib/programs/mcp-analytics/index.ts -> src/agent/agent-runner.ts", "src/lib/programs/metrics/index.ts -> src/lib/programs/agent-skill/content/index.tsx", - "src/lib/programs/migration/index.ts -> src/agent/agent-runner.ts", - "src/lib/programs/migration/index.ts -> src/agent/tools/index.ts", "src/lib/programs/migration/index.ts -> src/lib/programs/migration/content/index.tsx", - "src/lib/programs/posthog-doctor/index.ts -> src/agent/tools/index.ts", - "src/lib/programs/posthog-integration/index.ts -> src/agent/agent-interface.ts", - "src/lib/programs/posthog-integration/index.ts -> src/agent/agent-runner.ts", - "src/lib/programs/posthog-integration/index.ts -> src/agent/tools/index.ts", "src/lib/programs/posthog-integration/index.ts -> src/lib/programs/posthog-integration/content/index.tsx", "src/lib/programs/program-registry.ts -> src/lib/programs/agent-skill/content/index.tsx", - "src/lib/programs/program-run.ts -> src/agent/runner/index.ts", "src/lib/programs/program-step.ts -> src/ui/tui/components/TipsCard.tsx", "src/lib/programs/program-step.ts -> src/ui/tui/primitives/index.ts", "src/lib/programs/program-step.ts -> src/ui/tui/store.ts", - "src/lib/programs/replay-vision/index.ts -> src/agent/agent-runner.ts", - "src/lib/programs/revenue-analytics/detect.ts -> src/agent/agent-runner.ts", - "src/lib/programs/revenue-analytics/index.ts -> src/agent/tools/index.ts", "src/lib/programs/revenue-analytics/index.ts -> src/lib/programs/revenue-analytics/content/index.tsx", - "src/lib/programs/run-agent-legacy.ts -> src/agent/agent-interface.ts", - "src/lib/programs/run-agent-legacy.ts -> src/agent/runner/index.ts", - "src/lib/programs/run-agent-legacy.ts -> src/agent/runner/switchboard/index.ts", - "src/lib/programs/run-agent-legacy.ts -> src/agent/yara-hooks.ts", - "src/lib/programs/self-driving/detect.ts -> src/agent/agent-runner.ts", "src/lib/programs/self-driving/index.ts -> src/lib/programs/self-driving/content/index.tsx", "src/lib/programs/self-driving/index.ts -> src/lib/programs/self-driving/content/pricing.ts", "src/lib/programs/self-driving/index.ts -> src/lib/programs/self-driving/content/tips.ts", - "src/lib/programs/self-driving/prompt.ts -> src/agent/agent-interface.ts", - "src/lib/programs/self-driving/prompt.ts -> src/agent/agent-runner.ts", - "src/lib/programs/warehouse-source/detect.ts -> src/agent/agent-runner.ts", - "src/lib/programs/warehouse-source/index.ts -> src/agent/wizard-ask-bridge.ts", "src/lib/programs/warehouse-source/index.ts -> src/lib/programs/warehouse-source/content/index.tsx", - "src/lib/programs/web-analytics-doctor/detect.ts -> src/agent/agent-runner.ts", "src/lib/task-stream/event-plan-watcher.ts -> src/ui/tui/store.ts", "src/lib/task-stream/task-stream-push.ts -> src/ui/tui/store.ts", "src/lib/wizard-session.ts -> src/agent/progress.ts", - "src/lib/wizard-spellbook.ts -> src/agent/tools/tools.ts", "src/shared/claude-settings.ts -> src/agent/agent-env-isolation.ts", "src/shared/errors/agent-map.ts -> src/agent/signals.ts", - "src/shared/errors/skill-map.ts -> src/agent/tools/index.ts", "src/shared/utils/package-manager.ts -> src/telemetry.ts", "src/shared/utils/setup-utils.ts -> src/telemetry.ts", "src/shared/utils/wizard-abort.ts -> src/ui/logging-ui.ts", @@ -64,10 +33,8 @@ "src/steps/add-or-update-environment-variables.ts -> src/telemetry.ts", "src/steps/run-prettier.ts -> src/telemetry.ts", "src/steps/upload-environment-variables/index.ts -> src/telemetry.ts", - "src/ui/agent-progress.ts -> src/agent/progress.ts", "src/ui/index.ts -> src/ui/logging-ui.ts", - "src/ui/tui/screens/health/HealthCheckScreen.tsx -> src/agent/tools/index.ts", - "src/ui/tui/services/mcp-suggested-prompts-services.ts -> src/agent/mcp-prompt-streaming.ts", - "src/ui/wizard-ui.ts -> src/agent/progress.ts" + "src/ui/tui/screens/health/HealthCheckScreen.tsx -> src/agent/index.ts", + "src/ui/tui/services/mcp-suggested-prompts-services.ts -> src/agent/index.ts" ] } diff --git a/src/agent/index.ts b/src/agent/index.ts new file mode 100644 index 000000000..c014021cc --- /dev/null +++ b/src/agent/index.ts @@ -0,0 +1,38 @@ +/** + * Public entry of the agent. Code outside `src/agent` imports runtime values + * from here and types from `./types`; deeper imports fail lint and the + * architecture test. Keep this list to what callers use, and keep it cheap: + * the startup chunk imports this module, so anything re-exported here loads + * before the wizard does any work. Heavy paths stay behind a lazy import. + */ +export type * from './types'; +export { + resolveBinding, + runAgent, + RunOutcome, + shouldDisableAsk, +} from './runner'; +export { + AgentSignals, + buildRunTags, + initializeAgent, + runAgent as executeAgent, +} from './agent-interface'; +export { configureGatewayFromCIEnvironment } from './gateway-session'; +export { downloadSkill, WIZARD_TOOL_NAMES } from './tools'; +export { LONGER_ASK_TIMEOUT_MS } from './wizard-ask-bridge'; +export { flushScanReport } from './yara-hooks'; + +/** + * Stream an MCP tutorial prompt through the SDK. Loaded on first call: the + * streaming module is the one agent module the startup chunk does not already + * hold, and the TUI reaches it only from the suggested-prompts screen. + */ +export async function* runMcpPromptViaSdk( + args: Parameters< + typeof import('./mcp-prompt-streaming').runMcpPromptViaSdk + >[0], +): AsyncIterable { + const streaming = await import('./mcp-prompt-streaming'); + yield* streaming.runMcpPromptViaSdk(args); +} diff --git a/src/agent/types.ts b/src/agent/types.ts new file mode 100644 index 000000000..6b42c298a --- /dev/null +++ b/src/agent/types.ts @@ -0,0 +1,30 @@ +/** + * Public type surface of the agent. Type-only, so importing it adds no + * runtime dependency. Code outside `src/agent` imports these as + * `@agent/types`; runtime values come from `@agent`. + */ +export type { + AbortCase, + AgentFailure, + AgentRunDefinition, + ProgramBinding, + PromptContext, + RunConfig, + RunInput, + RunResult, + SwitchboardCtx, +} from './runner'; +export type { + AgentInteraction, + AgentProgress, + AskAnswers, + AskQuestion, + AuthErrorDetail, + OutroData, + PendingQuestion, + SpinnerHandle, + TaskNotice, + TokenUsageDelta, +} from './progress'; +export type { AgentChunk } from './mcp-prompt-streaming'; +export type { InstallSkillResult } from './tools'; diff --git a/src/lib/detection/agentic.ts b/src/lib/detection/agentic.ts index c2ce6a279..e3f96aa22 100644 --- a/src/lib/detection/agentic.ts +++ b/src/lib/detection/agentic.ts @@ -15,10 +15,10 @@ import { initializeAgent, - runAgent as executeAgent, + executeAgent, buildRunTags, AgentSignals, -} from '@agent/agent-interface'; +} from '@agent'; import { isAbsolute, resolve, sep } from 'path'; import { detectNodePackageManagers } from './package-manager.js'; import { CallType, getSkillsBaseUrl, HAIKU_MODEL } from '@shared/constants'; diff --git a/src/lib/programs/agent-skill/index.ts b/src/lib/programs/agent-skill/index.ts index 8ab764911..e59f3fdf1 100644 --- a/src/lib/programs/agent-skill/index.ts +++ b/src/lib/programs/agent-skill/index.ts @@ -20,7 +20,7 @@ */ import type { ProgramConfig } from '@lib/programs/program-step'; -import type { AbortCase } from '@agent/agent-runner'; +import type { AbortCase } from '@agent/types'; import type { ProgramRun } from '@lib/programs/program-run'; import { AGENT_SKILL_STEPS } from './steps.js'; import { getContentBlocks } from './content/index.js'; diff --git a/src/lib/programs/audit/detect.ts b/src/lib/programs/audit/detect.ts index 37ee475b2..253c409e6 100644 --- a/src/lib/programs/audit/detect.ts +++ b/src/lib/programs/audit/detect.ts @@ -1,4 +1,4 @@ -import type { AbortCase } from '@agent/agent-runner'; +import type { AbortCase } from '@agent/types'; import { ErrorCodes } from '@shared/errors'; /** `[ABORT] ` cases the audit skill can emit. Reason strings are diff --git a/src/lib/programs/audit/index.ts b/src/lib/programs/audit/index.ts index 110f09816..a5921edbe 100644 --- a/src/lib/programs/audit/index.ts +++ b/src/lib/programs/audit/index.ts @@ -6,7 +6,7 @@ import type { ProgramStep, ProgramConfig } from '@lib/programs/program-step'; import type { ProgramRun } from '@lib/programs/program-run'; import type { WizardSession } from '@lib/wizard-session'; import { OutroKind } from '@lib/wizard-session'; -import { WIZARD_TOOL_NAMES } from '@agent/tools'; +import { WIZARD_TOOL_NAMES } from '@agent'; import { headlessOption, regionOption } from '@lib/headless-mode'; import { AUDIT_ABORT_CASES } from './detect.js'; import { diff --git a/src/lib/programs/dispatch-family.ts b/src/lib/programs/dispatch-family.ts index f2922077b..b9da9d39f 100644 --- a/src/lib/programs/dispatch-family.ts +++ b/src/lib/programs/dispatch-family.ts @@ -2,7 +2,7 @@ import type { Arguments } from 'yargs'; import { auditConfig } from '@lib/programs/audit/index'; import { AUDIT_CHECKS_FILE } from '@lib/programs/audit/types'; -import { WIZARD_TOOL_NAMES } from '@agent/tools'; +import { WIZARD_TOOL_NAMES } from '@agent'; import { agentSkillConfig } from '@lib/programs/program-registry'; import { webAnalyticsDoctorConfig } from '@lib/programs/web-analytics-doctor/index'; import type { ProgramConfig } from '@lib/programs/program-step'; diff --git a/src/lib/programs/error-tracking-upload-source-maps/detect.ts b/src/lib/programs/error-tracking-upload-source-maps/detect.ts index 15ff114af..3574ea1b9 100644 --- a/src/lib/programs/error-tracking-upload-source-maps/detect.ts +++ b/src/lib/programs/error-tracking-upload-source-maps/detect.ts @@ -17,7 +17,7 @@ import { safeReadFile, } from '@utils/bounded-fs'; import type { WizardSession } from '@lib/wizard-session'; -import type { AbortCase } from '@agent/agent-runner'; +import type { AbortCase } from '@agent/types'; import { ErrorCodes } from '@shared/errors'; /** diff --git a/src/lib/programs/error-tracking-upload-source-maps/prompt.ts b/src/lib/programs/error-tracking-upload-source-maps/prompt.ts index df2539095..fbc44f554 100644 --- a/src/lib/programs/error-tracking-upload-source-maps/prompt.ts +++ b/src/lib/programs/error-tracking-upload-source-maps/prompt.ts @@ -1,4 +1,4 @@ -import { AgentSignals } from '@agent/agent-interface'; +import { AgentSignals } from '@agent'; import type { SkillVariant } from './detect.js'; export type SourceMapsUploadPromptParams = { diff --git a/src/lib/programs/events-audit/index.ts b/src/lib/programs/events-audit/index.ts index 850ae62af..d072c7f60 100644 --- a/src/lib/programs/events-audit/index.ts +++ b/src/lib/programs/events-audit/index.ts @@ -4,7 +4,7 @@ import type { WizardSession } from '@lib/wizard-session'; import { OutroKind } from '@lib/wizard-session'; import { SPINNER_MESSAGE } from '@lib/framework-config'; import { isUsingTypeScript } from '@utils/setup-utils'; -import { WIZARD_TOOL_NAMES } from '@agent/tools'; +import { WIZARD_TOOL_NAMES } from '@agent'; import { EVENTS_AUDIT_PROGRAM } from './steps.js'; import { AUDIT_CHECKS_FILE, AUDIT_CHECKS_KEY } from '@lib/programs/audit/types'; import { seedAuditLedger } from '@lib/programs/audit/seed'; diff --git a/src/lib/programs/mcp-analytics/index.ts b/src/lib/programs/mcp-analytics/index.ts index 4b62d407b..458ba18e7 100644 --- a/src/lib/programs/mcp-analytics/index.ts +++ b/src/lib/programs/mcp-analytics/index.ts @@ -1,4 +1,4 @@ -import type { AbortCase } from '@agent/agent-runner'; +import type { AbortCase } from '@agent/types'; import { ErrorCodes } from '@shared/errors'; import { createSkillProgram } from '@lib/programs/agent-skill/index'; diff --git a/src/lib/programs/migration/index.ts b/src/lib/programs/migration/index.ts index 2fc157746..173e007be 100644 --- a/src/lib/programs/migration/index.ts +++ b/src/lib/programs/migration/index.ts @@ -1,6 +1,6 @@ import type { ProgramConfig } from '@lib/programs/program-step'; -import type { AbortCase } from '@agent/agent-runner'; -import { WIZARD_TOOL_NAMES } from '@agent/tools'; +import type { AbortCase } from '@agent/types'; +import { WIZARD_TOOL_NAMES } from '@agent'; import { MIGRATION_PROGRAM } from './steps.js'; import { getContentBlocks } from './content/index.js'; diff --git a/src/lib/programs/posthog-doctor/index.ts b/src/lib/programs/posthog-doctor/index.ts index cf57eb9c7..a6ad9f10e 100644 --- a/src/lib/programs/posthog-doctor/index.ts +++ b/src/lib/programs/posthog-doctor/index.ts @@ -1,5 +1,5 @@ import type { ProgramConfig } from '@lib/programs/program-step'; -import { WIZARD_TOOL_NAMES } from '@agent/tools'; +import { WIZARD_TOOL_NAMES } from '@agent'; import { POSTHOG_DOCTOR_PROGRAM } from './steps.js'; export const posthogDoctorConfig: ProgramConfig = { diff --git a/src/lib/programs/posthog-integration/index.ts b/src/lib/programs/posthog-integration/index.ts index 5a04cc46d..4a6a04a2b 100644 --- a/src/lib/programs/posthog-integration/index.ts +++ b/src/lib/programs/posthog-integration/index.ts @@ -1,10 +1,9 @@ import type { ProgramConfig, ProgramStep } from '@lib/programs/program-step'; import { runProgramAgent } from '@lib/programs/run-agent-legacy'; import type { ProgramRun } from '@lib/programs/program-run'; -import { WIZARD_TOOL_NAMES } from '@agent/tools'; +import { AgentSignals, shouldDisableAsk, WIZARD_TOOL_NAMES } from '@agent'; import type { WizardSession } from '@lib/wizard-session'; import { mayReportScanResults, OutroKind, RunPhase } from '@lib/wizard-session'; -import { AgentSignals } from '@agent/agent-interface'; import { DEFAULT_PACKAGE_INSTALLATION, SPINNER_MESSAGE, @@ -22,7 +21,6 @@ import { requestDeepLink } from '@utils/provisioning'; import { openTrackedLink, withUtm } from '@utils/links'; import type { HostResolution } from '@shared/host-resolution'; import { getDetectedWarehouseSources } from '@lib/programs/warehouse-source/detect'; -import { shouldDisableAsk } from '@agent/agent-runner'; import { POSTHOG_INTEGRATION_PROGRAM } from './steps.js'; import { getContentBlocks } from './content/index.js'; import { buildCodingAgentPrompt } from './handoff.js'; diff --git a/src/lib/programs/program-run.ts b/src/lib/programs/program-run.ts index 523d3bbbe..a3d9a15be 100644 --- a/src/lib/programs/program-run.ts +++ b/src/lib/programs/program-run.ts @@ -5,7 +5,7 @@ * agent `RunConfig.hooks`. */ -import type { AgentRunDefinition } from '@agent/runner'; +import type { AgentRunDefinition } from '@agent/types'; import type { Credentials, WizardSession } from '@lib/wizard-session'; export interface ProgramRun extends AgentRunDefinition { diff --git a/src/lib/programs/replay-vision/index.ts b/src/lib/programs/replay-vision/index.ts index 637f162f5..ef01e6a9d 100644 --- a/src/lib/programs/replay-vision/index.ts +++ b/src/lib/programs/replay-vision/index.ts @@ -1,4 +1,4 @@ -import type { AbortCase } from '@agent/agent-runner'; +import type { AbortCase } from '@agent/types'; import { Integration } from '@shared/constants'; import { detectFramework, gatherFrameworkContext } from '@lib/detection/index'; import { scopeInstallDirToProject } from '@lib/detection/project-scope'; diff --git a/src/lib/programs/revenue-analytics/detect.ts b/src/lib/programs/revenue-analytics/detect.ts index 7089e6e51..d63ef3c6a 100644 --- a/src/lib/programs/revenue-analytics/detect.ts +++ b/src/lib/programs/revenue-analytics/detect.ts @@ -7,7 +7,7 @@ import { existsSync, statSync } from 'fs'; import type { WizardSession } from '@lib/wizard-session'; -import type { AbortCase } from '@agent/agent-runner'; +import type { AbortCase } from '@agent/types'; import { findPackageJsons } from '@lib/programs/shared/package-scanning'; export { diff --git a/src/lib/programs/revenue-analytics/index.ts b/src/lib/programs/revenue-analytics/index.ts index 9194f9e70..fade6e3fc 100644 --- a/src/lib/programs/revenue-analytics/index.ts +++ b/src/lib/programs/revenue-analytics/index.ts @@ -1,5 +1,5 @@ import type { ProgramConfig } from '@lib/programs/program-step'; -import { WIZARD_TOOL_NAMES } from '@agent/tools'; +import { WIZARD_TOOL_NAMES } from '@agent'; import { REVENUE_ANALYTICS_PROGRAM } from './steps.js'; import { REVENUE_ABORT_CASES } from './detect.js'; import { getContentBlocks } from './content/index.js'; diff --git a/src/lib/programs/run-agent-legacy.ts b/src/lib/programs/run-agent-legacy.ts index f259533d6..e0c0733d0 100644 --- a/src/lib/programs/run-agent-legacy.ts +++ b/src/lib/programs/run-agent-legacy.ts @@ -18,23 +18,25 @@ import { analytics } from '@utils/analytics'; import { getUI } from '@ui'; import { createUiReducer, uiInteraction } from '@ui/agent-progress'; import { + buildRunTags, + flushScanReport, + resolveBinding, runAgent, RunOutcome, - resolveBinding, - type RunConfig, - type RunInput, - type SwitchboardCtx, -} from '@agent/runner'; -import type { ProgramBinding } from '@agent/runner/switchboard'; +} from '@agent'; +import type { + ProgramBinding, + RunConfig, + RunInput, + SwitchboardCtx, +} from '@agent/types'; import type { ProgramRun } from './program-run'; -import { buildRunTags } from '@agent/agent-interface'; import { backupAndFixClaudeSettings, checkAllSettingsConflicts, classifySettingsConflicts, restoreClaudeSettings, } from '@shared/claude-settings'; -import { flushScanReport } from '@agent/yara-hooks'; import { evaluateWizardReadiness, WizardReadiness, diff --git a/src/lib/programs/self-driving/detect.ts b/src/lib/programs/self-driving/detect.ts index 080b4ce58..a5dbdb546 100644 --- a/src/lib/programs/self-driving/detect.ts +++ b/src/lib/programs/self-driving/detect.ts @@ -29,7 +29,7 @@ import { import { join } from 'path'; import { analytics } from '@utils/analytics'; import type { WizardSession } from '@lib/wizard-session'; -import type { AbortCase } from '@agent/agent-runner'; +import type { AbortCase } from '@agent/types'; import { ErrorCodes } from '@shared/errors'; import { detectWarehouseSources } from '@lib/warehouse-sources/detect'; import type { DetectedSource } from '@lib/warehouse-sources/types'; diff --git a/src/lib/programs/self-driving/prompt.ts b/src/lib/programs/self-driving/prompt.ts index b79eeaaf9..db629ea28 100644 --- a/src/lib/programs/self-driving/prompt.ts +++ b/src/lib/programs/self-driving/prompt.ts @@ -1,5 +1,5 @@ -import { AgentSignals } from '@agent/agent-interface'; -import type { PromptContext } from '@agent/agent-runner'; +import { AgentSignals } from '@agent'; +import type { PromptContext } from '@agent/types'; import type { DetectedSource } from '@lib/warehouse-sources/types'; /** diff --git a/src/lib/programs/warehouse-source/detect.ts b/src/lib/programs/warehouse-source/detect.ts index c9cfa9673..22a7b79ab 100644 --- a/src/lib/programs/warehouse-source/detect.ts +++ b/src/lib/programs/warehouse-source/detect.ts @@ -9,7 +9,7 @@ import { existsSync, statSync } from 'fs'; import { analytics } from '@utils/analytics'; import type { WizardSession } from '@lib/wizard-session'; -import type { AbortCase } from '@agent/agent-runner'; +import type { AbortCase } from '@agent/types'; import { detectWarehouseSources } from '@lib/warehouse-sources/detect'; import type { DetectedSource } from '@lib/warehouse-sources/types'; diff --git a/src/lib/programs/warehouse-source/index.ts b/src/lib/programs/warehouse-source/index.ts index 7b5e99c23..a2fb7db06 100644 --- a/src/lib/programs/warehouse-source/index.ts +++ b/src/lib/programs/warehouse-source/index.ts @@ -1,7 +1,7 @@ import type { ProgramConfig } from '@lib/programs/program-step'; import type { ProgramRun } from '@lib/programs/program-run'; import type { WizardSession } from '@lib/wizard-session'; -import { LONGER_ASK_TIMEOUT_MS } from '@agent/wizard-ask-bridge'; +import { LONGER_ASK_TIMEOUT_MS } from '@agent'; import { WAREHOUSE_SOURCE_PROGRAM } from './steps.js'; import { WAREHOUSE_ABORT_CASES, diff --git a/src/lib/programs/web-analytics-doctor/detect.ts b/src/lib/programs/web-analytics-doctor/detect.ts index 30ab8ce01..9a05bbc00 100644 --- a/src/lib/programs/web-analytics-doctor/detect.ts +++ b/src/lib/programs/web-analytics-doctor/detect.ts @@ -1,6 +1,6 @@ import { existsSync, statSync } from 'fs'; import type { WizardSession } from '@lib/wizard-session'; -import type { AbortCase } from '@agent/agent-runner'; +import type { AbortCase } from '@agent/types'; import { ErrorCodes } from '@shared/errors'; import { findPackageJsons } from '@lib/programs/shared/package-scanning'; diff --git a/src/lib/runners/run-non-interactive.ts b/src/lib/runners/run-non-interactive.ts index dfa32f9c7..740d102b9 100644 --- a/src/lib/runners/run-non-interactive.ts +++ b/src/lib/runners/run-non-interactive.ts @@ -242,9 +242,7 @@ export function runNonInteractive( try { if (mode === 'ci') { - const { configureGatewayFromCIEnvironment } = await import( - '@agent/gateway-session' - ); + const { configureGatewayFromCIEnvironment } = await import('@agent'); configureGatewayFromCIEnvironment( Number(session.projectId), session.region ?? 'us', diff --git a/src/lib/wizard-session.ts b/src/lib/wizard-session.ts index 18edf4972..e35ba2325 100644 --- a/src/lib/wizard-session.ts +++ b/src/lib/wizard-session.ts @@ -24,14 +24,17 @@ import type { WizardReadinessResult } from '@shared/health-checks/readiness'; import type { SettingsConflict } from '@shared/claude-settings'; import type { ApiUser, ApiProject, Credentials } from '@shared/api'; import type { CloudRegion } from '@utils/types'; -import { - OutroKind, - type AskAnswers, - type AskQuestion, - type OutroData, - type PendingQuestion, - type TaskNotice, -} from '@agent/progress'; +import type { + AskAnswers, + AskQuestion, + OutroData, + PendingQuestion, + TaskNotice, +} from '@agent/types'; +// Leaf module on purpose: shared analytics imports this file, so the agent +// entry would form a module cycle here. +// eslint-disable-next-line @typescript-eslint/no-restricted-imports -- B2: the session becomes a TUI projection +import { OutroKind } from '@agent/progress'; // These shapes moved to their owners; re-exported so every session reader // keeps its import path. `Credentials` sits with the API types, the diff --git a/src/lib/wizard-spellbook.ts b/src/lib/wizard-spellbook.ts index be7105e51..8473d8096 100644 --- a/src/lib/wizard-spellbook.ts +++ b/src/lib/wizard-spellbook.ts @@ -3,7 +3,7 @@ import path from 'path'; import { getSkillsBaseUrl, POSTHOG_DOCS_URL } from '@shared/constants'; import type { ProgramConfig } from './programs/program-step'; import type { WizardSession } from './wizard-session'; -import { downloadSkill } from '@agent/tools/tools'; +import { downloadSkill } from '@agent'; import { fetchSkillMenu, type SkillEntry, diff --git a/src/shared/claude-settings.ts b/src/shared/claude-settings.ts index da4873eec..05d697273 100644 --- a/src/shared/claude-settings.ts +++ b/src/shared/claude-settings.ts @@ -12,6 +12,9 @@ import * as fs from 'fs'; import * as os from 'os'; import { analytics } from '@utils/analytics'; import { registerCleanup } from '@utils/wizard-abort'; +// Leaf module on purpose: shared analytics imports the session, which this +// file feeds, so the agent entry would form a module cycle here. +// eslint-disable-next-line @typescript-eslint/no-restricted-imports -- B1: shared stops feeding the session import { BLOCKED_AGENT_ENV_KEYS, BLOCKED_AGENT_ENV_PATTERNS, diff --git a/src/shared/errors/agent-map.ts b/src/shared/errors/agent-map.ts index c1160d14b..e897b5085 100644 --- a/src/shared/errors/agent-map.ts +++ b/src/shared/errors/agent-map.ts @@ -1,3 +1,6 @@ +// Leaf module on purpose: every module imports the errors, so the agent entry +// would form a module cycle here. +// eslint-disable-next-line @typescript-eslint/no-restricted-imports -- B1: error codes move next to their owners import { AgentErrorType } from '@agent/signals'; import { ErrorCodes, type ErrorCode } from './codes'; diff --git a/src/shared/errors/skill-map.ts b/src/shared/errors/skill-map.ts index 50f9851fb..97d1a1419 100644 --- a/src/shared/errors/skill-map.ts +++ b/src/shared/errors/skill-map.ts @@ -1,5 +1,5 @@ import { ErrorCodes, type ErrorCode } from './codes'; -import type { InstallSkillResult } from '@agent/tools'; +import type { InstallSkillResult } from '@agent/types'; const SKILL_CODES: Record< Exclude, diff --git a/src/ui/agent-progress.ts b/src/ui/agent-progress.ts index a28dcbf27..93e6df292 100644 --- a/src/ui/agent-progress.ts +++ b/src/ui/agent-progress.ts @@ -1,5 +1,5 @@ import type { WizardUI, SpinnerHandle } from './wizard-ui'; -import type { AgentInteraction, AgentProgress } from '@agent/progress'; +import type { AgentInteraction, AgentProgress } from '@agent/types'; // ── Progress → WizardUI, one call per event ─────────────────────────── diff --git a/src/ui/tui/screens/health/HealthCheckScreen.tsx b/src/ui/tui/screens/health/HealthCheckScreen.tsx index 672113050..676276935 100644 --- a/src/ui/tui/screens/health/HealthCheckScreen.tsx +++ b/src/ui/tui/screens/health/HealthCheckScreen.tsx @@ -24,7 +24,7 @@ import { import { ServiceHealthStatus } from '@shared/health-checks/types'; import { wizardAbort } from '@utils/wizard-abort'; import { ErrorCodes } from '@shared/errors'; -import { downloadSkill } from '@agent/tools'; +import { downloadSkill } from '@agent'; import { fetchSkillMenu } from '@shared/skill-menu'; import { GITHUB_SKILLS_BASE_URL } from '@shared/constants'; import { useDismissOnAnyKey } from '@ui/tui/hooks/useDismissOnAnyKey'; diff --git a/src/ui/tui/services/mcp-suggested-prompts-services.ts b/src/ui/tui/services/mcp-suggested-prompts-services.ts index 8651f8dc9..b92ad2d16 100644 --- a/src/ui/tui/services/mcp-suggested-prompts-services.ts +++ b/src/ui/tui/services/mcp-suggested-prompts-services.ts @@ -23,7 +23,7 @@ import { seedDemoEvents as runSeed } from '@lib/mcp-seed-events'; // The streamed event shape is the agent's; re-exported so the screen and the // playground keep their import path. -import type { AgentChunk } from '@agent/mcp-prompt-streaming'; +import type { AgentChunk } from '@agent/types'; export type { AgentChunk }; export interface McpSuggestedPromptsServices { @@ -161,6 +161,6 @@ async function* runProductionPromptStreaming(args: { // Defer the SDK import to call time — the playground never hits // this path (it overrides the whole service object), so demo // sessions don't pay the SDK load cost. - const { runMcpPromptViaSdk } = await import('@agent/mcp-prompt-streaming'); + const { runMcpPromptViaSdk } = await import('@agent'); yield* runMcpPromptViaSdk(args); } diff --git a/src/ui/wizard-ui.ts b/src/ui/wizard-ui.ts index f99802402..e972a5063 100644 --- a/src/ui/wizard-ui.ts +++ b/src/ui/wizard-ui.ts @@ -34,7 +34,7 @@ import type { AuthErrorDetail, SpinnerHandle, TokenUsageDelta, -} from '@agent/progress'; +} from '@agent/types'; export type { AuthErrorDetail, SpinnerHandle, TokenUsageDelta }; export interface WizardUI { diff --git a/tsconfig.build.json b/tsconfig.build.json index f4c4ef2d9..d716e8367 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -20,6 +20,8 @@ "paths": { "@env": ["./src/env.ts"], "@shared/*": ["./src/shared/*"], + "@agent": ["./src/agent/index.ts"], + "@agent/types": ["./src/agent/types.ts"], "@agent/*": ["./src/agent/*"], "@lib/*": ["./src/lib/*"], "@e2e-harness/*": ["./e2e-harness/*"], diff --git a/vitest.config.ts b/vitest.config.ts index 14d4149f8..713b61ac1 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -79,6 +79,8 @@ export default defineConfig({ }, { find: /^ink$/, replacement: r('__mocks__/ink.ts') }, { find: /^@shared\/(.*)$/, replacement: `${r('src/shared')}/$1` }, + { find: /^@agent$/, replacement: r('src/agent/index.ts') }, + { find: /^@agent\/types$/, replacement: r('src/agent/types.ts') }, { find: /^@agent\/(.*)$/, replacement: `${r('src/agent')}/$1` }, // Path aliases — mirror tsconfig `paths`. { find: /^@env$/, replacement: r('src/env.ts') }, From 9f0d4c443bd2eff3940cf6251f419ff7fca899e8 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 13:59:06 -0400 Subject: [PATCH 03/12] refactor(shared): stop shared code from reaching upward `debug()` reports through an injected sink that the UI module installs, so shared no longer looks the UI up. The progress tag helpers move from `src/telemetry.ts` to `@utils/telemetry` and the detect error map moves to programs, which own the detect error kinds. `src/shared` is now its own surface in the architecture test; the remaining upward edges are listed with their owners in the stack plan. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- docs/error-catalog.md | 2 +- .../architecture/import-boundaries.test.ts | 17 +++++---- .../architecture/known-violations.json | 14 ++++---- src/__tests__/cli.test.ts | 1 + src/__tests__/provision-cli.test.ts | 1 + src/agent/__tests__/gateway-session.test.ts | 2 +- .../programs}/__tests__/detect-map.test.ts | 3 +- .../errors => lib/programs}/detect-map.ts | 2 +- src/lib/runners/run-non-interactive.ts | 2 +- src/shared/errors/index.ts | 1 - src/shared/utils/__tests__/debug.test.ts | 35 +++++++++++++++++++ .../utils/__tests__/oauth-refresh.test.ts | 2 +- .../utils/__tests__/oauth-server.test.ts | 2 +- src/shared/utils/debug.ts | 15 ++++++-- src/shared/utils/package-manager.ts | 2 +- src/shared/utils/setup-utils.ts | 2 +- src/{ => shared/utils}/telemetry.ts | 2 +- src/steps/add-mcp-server-to-clients/index.ts | 2 +- .../add-or-update-environment-variables.ts | 2 +- src/steps/run-prettier.ts | 2 +- .../upload-environment-variables/index.ts | 2 +- src/ui/index.ts | 5 +++ 22 files changed, 87 insertions(+), 31 deletions(-) rename src/{shared/errors => lib/programs}/__tests__/detect-map.test.ts (96%) rename src/{shared/errors => lib/programs}/detect-map.ts (97%) rename src/{ => shared/utils}/telemetry.ts (81%) diff --git a/docs/error-catalog.md b/docs/error-catalog.md index 8e8c05b19..c162e2897 100644 --- a/docs/error-catalog.md +++ b/docs/error-catalog.md @@ -117,7 +117,7 @@ guarantee. Program detect steps write `{ kind, ...detail }` into `session.frameworkContext.detectError`. `detectErrorCode()` -([`src/shared/errors/detect-map.ts`](../src/shared/errors/detect-map.ts)) maps `kind` +([`src/lib/programs/detect-map.ts`](../src/lib/programs/detect-map.ts)) maps `kind` → code, and the whole object — `kind` included — rides along as `OutroData.errorDetail`. diff --git a/src/__tests__/architecture/import-boundaries.test.ts b/src/__tests__/architecture/import-boundaries.test.ts index a7ba197ff..9ba7d9236 100644 --- a/src/__tests__/architecture/import-boundaries.test.ts +++ b/src/__tests__/architecture/import-boundaries.test.ts @@ -2,7 +2,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { fileURLToPath } from 'url'; -export type Surface = 'env' | 'legacy' | 'agent' | 'tui' | 'cli'; +export type Surface = 'env' | 'shared' | 'legacy' | 'agent' | 'tui' | 'cli'; const HERE = path.dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = path.resolve(HERE, '../../..'); @@ -10,6 +10,7 @@ const REPO_ROOT = path.resolve(HERE, '../../..'); const SURFACE_RULES: ReadonlyArray boolean]> = [ ['env', (p) => p === 'src/env.ts'], + ['shared', (p) => p.startsWith('src/shared/')], ['agent', (p) => p.startsWith('src/agent/')], [ 'tui', @@ -26,7 +27,6 @@ const SURFACE_RULES: ReadonlyArray boolean]> = (p) => p === 'bin.ts' || p === 'src/wizard.ts' || - p === 'src/telemetry.ts' || p.startsWith('src/commands/') || p.startsWith('src/lib/runners/'), ], @@ -42,10 +42,12 @@ export function classifySurface(relPath: string): Surface { export const ALLOWED_IMPORTS: Record = { env: [], - legacy: ['env', 'legacy'], - agent: ['env', 'legacy', 'agent'], - tui: ['env', 'legacy', 'tui'], - cli: ['env', 'legacy', 'agent', 'tui', 'cli'], + shared: ['env', 'shared'], + legacy: ['env', 'shared', 'legacy'], + // Program types stay importable from the agent until B1 moves the bindings. + agent: ['env', 'shared', 'legacy', 'agent'], + tui: ['env', 'shared', 'legacy', 'tui'], + cli: ['env', 'shared', 'legacy', 'agent', 'tui', 'cli'], }; // The agent's public entries. Outside `src/agent`, an import into the agent @@ -396,7 +398,8 @@ describe('import boundaries', () => { describe('surface classification', () => { it('maps representative paths to their surface', () => { expect(classifySurface('src/env.ts')).toBe('env'); - expect(classifySurface('src/shared/utils/analytics.ts')).toBe('legacy'); + expect(classifySurface('src/shared/utils/analytics.ts')).toBe('shared'); + expect(classifySurface('src/shared/errors/codes.ts')).toBe('shared'); expect(classifySurface('src/agent/agent-runner.ts')).toBe('agent'); expect(classifySurface('src/ui/tui/App.tsx')).toBe('tui'); expect(classifySurface('bin.ts')).toBe('cli'); diff --git a/src/__tests__/architecture/known-violations.json b/src/__tests__/architecture/known-violations.json index ce41efa60..629085454 100644 --- a/src/__tests__/architecture/known-violations.json +++ b/src/__tests__/architecture/known-violations.json @@ -26,13 +26,15 @@ "src/lib/wizard-session.ts -> src/agent/progress.ts", "src/shared/claude-settings.ts -> src/agent/agent-env-isolation.ts", "src/shared/errors/agent-map.ts -> src/agent/signals.ts", - "src/shared/utils/package-manager.ts -> src/telemetry.ts", - "src/shared/utils/setup-utils.ts -> src/telemetry.ts", + "src/shared/utils/analytics.ts -> src/lib/wizard-session.ts", + "src/shared/utils/oauth.ts -> src/ui/index.ts", + "src/shared/utils/setup-utils.ts -> src/lib/oauth/program-scopes.ts", + "src/shared/utils/setup-utils.ts -> src/lib/programs/program-registry.ts", + "src/shared/utils/setup-utils.ts -> src/lib/wizard-session.ts", + "src/shared/utils/setup-utils.ts -> src/ui/index.ts", + "src/shared/utils/wizard-abort.ts -> src/lib/wizard-session.ts", + "src/shared/utils/wizard-abort.ts -> src/ui/index.ts", "src/shared/utils/wizard-abort.ts -> src/ui/logging-ui.ts", - "src/steps/add-mcp-server-to-clients/index.ts -> src/telemetry.ts", - "src/steps/add-or-update-environment-variables.ts -> src/telemetry.ts", - "src/steps/run-prettier.ts -> src/telemetry.ts", - "src/steps/upload-environment-variables/index.ts -> src/telemetry.ts", "src/ui/index.ts -> src/ui/logging-ui.ts", "src/ui/tui/screens/health/HealthCheckScreen.tsx -> src/agent/index.ts", "src/ui/tui/services/mcp-suggested-prompts-services.ts -> src/agent/index.ts" diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index aedc37a40..73de7fa8a 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -99,6 +99,7 @@ vi.mock('@utils/env-api-key', () => ({ vi.mock('@utils/debug', () => ({ configureLogFileFromEnvironment: vi.fn(), logToFile: vi.fn(), + setDebugSink: vi.fn(), })); vi.mock('../lib/registry', () => ({ FRAMEWORK_REGISTRY: {} })); vi.mock('../lib/detection/index', () => ({ diff --git a/src/__tests__/provision-cli.test.ts b/src/__tests__/provision-cli.test.ts index 636f75f72..94b76cb25 100644 --- a/src/__tests__/provision-cli.test.ts +++ b/src/__tests__/provision-cli.test.ts @@ -50,6 +50,7 @@ vi.mock('@utils/env-api-key', () => ({ vi.mock('@utils/debug', () => ({ configureLogFileFromEnvironment: vi.fn(), logToFile: vi.fn(), + setDebugSink: vi.fn(), })); vi.mock('../lib/registry', () => ({ FRAMEWORK_REGISTRY: {} })); vi.mock('../lib/detection/index', () => ({ diff --git a/src/agent/__tests__/gateway-session.test.ts b/src/agent/__tests__/gateway-session.test.ts index 23d5562d8..cf1eda731 100644 --- a/src/agent/__tests__/gateway-session.test.ts +++ b/src/agent/__tests__/gateway-session.test.ts @@ -26,7 +26,7 @@ vi.mock('@utils/analytics', () => ({ analytics: { wizardCapture: vi.fn(), captureException: vi.fn() }, })); -vi.mock('@utils/debug', () => ({ logToFile: vi.fn() })); +vi.mock('@utils/debug', () => ({ logToFile: vi.fn(), setDebugSink: vi.fn() })); // logToFile is variadic, so a leak in any argument is a leak. Rendered every way the // sink might: JSON (which invokes getters and toJSON), an Error's stack, and inspect. diff --git a/src/shared/errors/__tests__/detect-map.test.ts b/src/lib/programs/__tests__/detect-map.test.ts similarity index 96% rename from src/shared/errors/__tests__/detect-map.test.ts rename to src/lib/programs/__tests__/detect-map.test.ts index 28d12cac7..b01368a1a 100644 --- a/src/shared/errors/__tests__/detect-map.test.ts +++ b/src/lib/programs/__tests__/detect-map.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { ErrorCodes } from '../codes'; -import { ERROR_CATALOG } from '../catalog'; +import { ErrorCodes, ERROR_CATALOG } from '@shared/errors'; import { detectErrorCode, type DetectErrorKind } from '../detect-map'; /** diff --git a/src/shared/errors/detect-map.ts b/src/lib/programs/detect-map.ts similarity index 97% rename from src/shared/errors/detect-map.ts rename to src/lib/programs/detect-map.ts index b632966ac..aaf4e25e2 100644 --- a/src/shared/errors/detect-map.ts +++ b/src/lib/programs/detect-map.ts @@ -1,4 +1,4 @@ -import { ErrorCodes, type ErrorCode } from './codes'; +import { ErrorCodes, type ErrorCode } from '@shared/errors'; import type { RevenueDetectError } from '@lib/programs/revenue-analytics/detect'; import type { SelfDrivingDetectError } from '@lib/programs/self-driving/detect'; import type { SourceMapsDetectError } from '@lib/programs/error-tracking-upload-source-maps/detect'; diff --git a/src/lib/runners/run-non-interactive.ts b/src/lib/runners/run-non-interactive.ts index 740d102b9..6977ae5f9 100644 --- a/src/lib/runners/run-non-interactive.ts +++ b/src/lib/runners/run-non-interactive.ts @@ -21,9 +21,9 @@ import { join } from 'node:path'; import { ErrorCodes, classifyRunFailure, - detectErrorCode, emitWizardError, } from '@shared/errors'; +import { detectErrorCode } from '@lib/programs/detect-map'; import type { OutroData, RunPhase as RunPhaseT } from '@lib/wizard-session'; /** diff --git a/src/shared/errors/index.ts b/src/shared/errors/index.ts index 51a0793d7..67178ff72 100644 --- a/src/shared/errors/index.ts +++ b/src/shared/errors/index.ts @@ -9,7 +9,6 @@ export { WizardError } from './wizard-error'; export type { ErrorCatalogEntry, ErrorGroup, RetryAdvice } from './types'; export { classifyAuthFailure, type AuthFailureInput } from './auth'; export { AGENT_ERROR_CODE } from './agent-map'; -export { detectErrorCode, type DetectErrorKind } from './detect-map'; export { skillErrorCode } from './skill-map'; export { PHW_ERROR_PREFIX, diff --git a/src/shared/utils/__tests__/debug.test.ts b/src/shared/utils/__tests__/debug.test.ts index 31ab68581..05cec1309 100644 --- a/src/shared/utils/__tests__/debug.test.ts +++ b/src/shared/utils/__tests__/debug.test.ts @@ -64,3 +64,38 @@ describe('log file writing', () => { expect(content).toContain('second write'); }); }); + +describe('debug console sink', () => { + it('sends enabled debug lines to the injected sink only', async () => { + const { debug, enableDebugLogs, setDebugSink } = await import('../debug'); + const lines: string[] = []; + const previous = setDebugSink((line) => lines.push(line)); + try { + debug('before enable'); + expect(lines).toEqual([]); + enableDebugLogs(); + debug('hello', 'world'); + expect(lines).toEqual(['hello world']); + } finally { + setDebugSink(previous); + } + }); + + it('is wired to the current UI by the UI module', async () => { + const { debug, enableDebugLogs } = await import('../debug'); + const { getUI, setUI } = await import('@ui'); + const seen: string[] = []; + const original = getUI(); + setUI({ + ...original, + log: { ...original.log, info: (line: string) => seen.push(line) }, + } as typeof original); + try { + enableDebugLogs(); + debug('routed'); + expect(seen).toEqual(['routed']); + } finally { + setUI(original); + } + }); +}); diff --git a/src/shared/utils/__tests__/oauth-refresh.test.ts b/src/shared/utils/__tests__/oauth-refresh.test.ts index d3fafdd39..525ea9400 100644 --- a/src/shared/utils/__tests__/oauth-refresh.test.ts +++ b/src/shared/utils/__tests__/oauth-refresh.test.ts @@ -8,7 +8,7 @@ vi.mock('../urls', async (importOriginal) => ({ ...(await importOriginal()), resolveBaseUrl: (baseUrl?: string) => baseUrl, })); -vi.mock('../debug', () => ({ logToFile: vi.fn() })); +vi.mock('../debug', () => ({ logToFile: vi.fn(), setDebugSink: vi.fn() })); const mockedAxios = axios as Mocked; diff --git a/src/shared/utils/__tests__/oauth-server.test.ts b/src/shared/utils/__tests__/oauth-server.test.ts index f5086fa04..87c0dc8df 100644 --- a/src/shared/utils/__tests__/oauth-server.test.ts +++ b/src/shared/utils/__tests__/oauth-server.test.ts @@ -3,7 +3,7 @@ import * as net from 'node:net'; import { startCallbackServer } from '@utils/oauth'; import { logToFile } from '../debug'; -vi.mock('../debug', () => ({ logToFile: vi.fn() })); +vi.mock('../debug', () => ({ logToFile: vi.fn(), setDebugSink: vi.fn() })); const authUrl = 'https://oauth.example.test/authorize'; const signupUrl = 'https://oauth.example.test/signup'; diff --git a/src/shared/utils/debug.ts b/src/shared/utils/debug.ts index 1081ff09a..807d2639a 100644 --- a/src/shared/utils/debug.ts +++ b/src/shared/utils/debug.ts @@ -1,7 +1,6 @@ import { appendFileSync, mkdirSync } from 'fs'; import path from 'path'; import { inspect } from 'node:util'; -import { getUI } from '@ui'; import { IS_DEV, runtimeEnv } from '@env'; import { WIZARD_LOG_FILE } from './paths'; @@ -107,9 +106,21 @@ export function logToFile(...args: unknown[]): void { appendLine(`[${ts}] ${renderLine(args)}\n`); } +/** Where `debug()` lines go. The UI module installs the current UI's info log at load; until then they go to stdout. */ +export type DebugSink = (line: string) => void; + +let debugSink: DebugSink = (line) => process.stdout.write(`${line}\n`); + +/** Replace the console sink; returns the previous one so callers can restore it. */ +export function setDebugSink(sink: DebugSink): DebugSink { + const previous = debugSink; + debugSink = sink; + return previous; +} + export function debug(...args: unknown[]): void { if (!consoleLoggingEnabled) return; - getUI().log.info(renderLine(args)); + debugSink(renderLine(args)); } export function enableDebugLogs(): void { diff --git a/src/shared/utils/package-manager.ts b/src/shared/utils/package-manager.ts index dc846f275..cd95772e1 100644 --- a/src/shared/utils/package-manager.ts +++ b/src/shared/utils/package-manager.ts @@ -1,7 +1,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { readFileHead } from './bounded-fs'; -import { withProgress } from '../../telemetry'; +import { withProgress } from './telemetry'; import { getPackageDotJson, updatePackageDotJson } from './setup-utils'; import type { PackageJson } from './package-json'; import { analytics } from './analytics'; diff --git a/src/shared/utils/setup-utils.ts b/src/shared/utils/setup-utils.ts index eb434f8ef..f8857b555 100644 --- a/src/shared/utils/setup-utils.ts +++ b/src/shared/utils/setup-utils.ts @@ -4,7 +4,7 @@ import * as os from 'node:os'; import { basename, isAbsolute, join, relative } from 'node:path'; import { promisify } from 'node:util'; -import { withProgress } from '../../telemetry'; +import { withProgress } from './telemetry'; import { debug, logToFile } from './debug'; import type { PackageJson } from './package-json'; import { diff --git a/src/telemetry.ts b/src/shared/utils/telemetry.ts similarity index 81% rename from src/telemetry.ts rename to src/shared/utils/telemetry.ts index 5d4bc955a..ba641632c 100644 --- a/src/telemetry.ts +++ b/src/shared/utils/telemetry.ts @@ -1,4 +1,4 @@ -import { analytics } from '@utils/analytics'; +import { analytics } from './analytics'; export function withProgress(step: string, callback: () => T): T { updateProgress(step); diff --git a/src/steps/add-mcp-server-to-clients/index.ts b/src/steps/add-mcp-server-to-clients/index.ts index fcc85a83b..ec014851f 100644 --- a/src/steps/add-mcp-server-to-clients/index.ts +++ b/src/steps/add-mcp-server-to-clients/index.ts @@ -1,6 +1,6 @@ import type { Integration } from '@shared/constants'; import type { CloudRegion } from '@utils/types'; -import { withProgress } from '../../telemetry'; +import { withProgress } from '@utils/telemetry'; import { analytics } from '@utils/analytics'; import { getUI } from '@ui'; import { MCPClient } from './MCPClient'; diff --git a/src/steps/add-or-update-environment-variables.ts b/src/steps/add-or-update-environment-variables.ts index f4d5d20d4..05c8a3579 100644 --- a/src/steps/add-or-update-environment-variables.ts +++ b/src/steps/add-or-update-environment-variables.ts @@ -1,5 +1,5 @@ import type { Integration } from '@shared/constants'; -import { withProgress } from '../telemetry'; +import { withProgress } from '@utils/telemetry'; import { analytics } from '@utils/analytics'; import { getUI } from '@ui'; import { getDotGitignore } from '@utils/bounded-fs'; diff --git a/src/steps/run-prettier.ts b/src/steps/run-prettier.ts index 95078bdd9..33262dd53 100644 --- a/src/steps/run-prettier.ts +++ b/src/steps/run-prettier.ts @@ -1,5 +1,5 @@ import type { Integration } from '@shared/constants'; -import { withProgress } from '../telemetry'; +import { withProgress } from '@utils/telemetry'; import { analytics } from '@utils/analytics'; import { getUI } from '@ui'; import { diff --git a/src/steps/upload-environment-variables/index.ts b/src/steps/upload-environment-variables/index.ts index 239287573..51b61bc22 100644 --- a/src/steps/upload-environment-variables/index.ts +++ b/src/steps/upload-environment-variables/index.ts @@ -1,5 +1,5 @@ import type { Integration } from '@shared/constants'; -import { withProgress } from '../../telemetry'; +import { withProgress } from '@utils/telemetry'; import { analytics } from '@utils/analytics'; import { getUI } from '@ui'; import type { WizardSession } from '@lib/wizard-session'; diff --git a/src/ui/index.ts b/src/ui/index.ts index 9688eaf6f..dc374144a 100644 --- a/src/ui/index.ts +++ b/src/ui/index.ts @@ -5,9 +5,14 @@ import type { WizardUI } from './wizard-ui'; import { LoggingUI } from './logging-ui'; +import { setDebugSink } from '@utils/debug'; let currentUI: WizardUI = new LoggingUI(); +// Shared code never looks the UI up; `debug()` reports through whichever UI +// is current, installed here so the sink follows setUI(). +setDebugSink((line) => currentUI.log.info(line)); + export function getUI(): WizardUI { return currentUI; } From 49614fb372650410ce93fa27fdec02f323447963 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 14:01:49 -0400 Subject: [PATCH 04/12] docs: describe the agent and shared surfaces Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- AGENTS.md | 9 +++ src/agent/README.md | 69 ++++++++++++++++++++ src/agent/runner/README.md | 2 +- src/agent/runner/harness/anthropic/README.md | 6 +- src/agent/runner/sequence/README.md | 2 +- src/shared/README.md | 43 ++++++++++++ 6 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 src/agent/README.md create mode 100644 src/shared/README.md diff --git a/AGENTS.md b/AGENTS.md index bf6620e5c..ad66039db 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,6 +30,10 @@ Each domain has a dedicated boundary: [ai-gateway](https://github.com/PostHog/ai-gateway). To disable scanning in the field without a release, see the kill-switch runbook: `docs/runbooks/warlock-kill-switch.md`. ONLY USE THIS IF ABSOLUTELY NECESSARY. +- **Agent** → `src/agent/`, imported only through `@agent` (values) and + `@agent/types` (types); see [src/agent/README.md](src/agent/README.md) +- **Shared** → `src/shared/`, stateless library code with no upward imports; + see [src/shared/README.md](src/shared/README.md) - **Programs** → step arrays in `src/lib/programs/` - **TUI** → screen components and primitives in `src/ui/tui/` @@ -197,6 +201,11 @@ wizard run points. Full catalog: [`docs/local-dev.md`](docs/local-dev.md). types so they satisfy `Record`. - All UI calls go through `getUI()` (returns `WizardUI` interface). Never import the store directly from business logic. +- Shared helpers never call `getUI()`; they take a sink or return data. `debug()` + reaches the UI through the sink `src/ui/index.ts` installs. +- Outside `src/agent`, import the agent through `@agent` or `@agent/types`. Add + to those entry modules rather than deep-importing; lint and + `pnpm test:arch` reject `@agent/*` paths elsewhere. - Session mutations go through explicit store setters that call `emitChange()`. Never mutate `session` directly — nanostore holds a shallow copy. - The router resolves the active screen from session state. No imperative diff --git a/src/agent/README.md b/src/agent/README.md new file mode 100644 index 000000000..1c86b2e08 --- /dev/null +++ b/src/agent/README.md @@ -0,0 +1,69 @@ +# Agent + +The agent runs one program's AI pipeline against a project directory. It takes resolved data in, reports through progress events, asks through an injected answerer, and returns a result. It never reads a session, a store or a UI. + +## Signatures + +Import runtime values from `@agent` and types from `@agent/types`. Nothing outside `src/agent` imports deeper; lint and the architecture test reject it. + +```ts +import { runAgent, RunOutcome } from '@agent'; +import type { RunConfig, RunInput, RunResult, AgentProgress } from '@agent/types'; + +runAgent(config: RunConfig, input: RunInput, options?: { + onProgress?: (event: AgentProgress) => void; + interaction?: AgentInteraction; +}): Promise +``` + +- `RunConfig`: the program id, its `AgentRunDefinition` (prompt, skill, tools, copy), the resolved `binding` (sequence, harness, model), the switchboard inputs, the skills origin, flag snapshot, trace tags, tool allow and deny lists, seed tasks and bound completion `hooks`. +- `RunInput`: install directory, resolved credentials, project and user payloads, skill id, detected integration, `flags` (`ci`, `signup`, `debug`, `e2eAsk`, `localMcp`, `captureAio`, `benchmark`, `yaraReport`) and the host the CLI was told. +- `RunResult`: `outcome` is `RunOutcome.Success | Aborted | Failed | Crashed`. Success may carry an `outro`; the other three carry a `failure` (`AgentFailure`: message, outro data, error, exit code, error code, detail). Every result carries `skillId` and a `snapshot` of what the run reported: tasks, status lines, stage, token usage totals, final cost, dashboard and notebook URLs, handoff text. +- `AgentProgress`: one event per thing the run reports, in emission order. Kinds: `lifecycle`, `spinner`, `log`, `status`, `tasks`, `stage`, `url`, `usage`, `finalCost`, `authError`, `handoff`, `completion`. Payloads are copies, never live objects. +- `AgentInteraction`: every member optional. `ask(question)` resolves with answers, `cancelAsk()` dismisses the open question, `taskNotice(notice)` resolves with whether to keep an optional task, `cancelTaskNotice()` declines it. +- Errors: the agent does not exit the process and does not throw for a decided failure. An unexpected throw becomes `outcome: Crashed` with the error attached. A gateway 401 emits `authError` and then fails. + +Other runtime exports: `resolveBinding`, `shouldDisableAsk`, `initializeAgent`, `executeAgent`, `buildRunTags`, `AgentSignals`, `configureGatewayFromCIEnvironment`, `downloadSkill`, `WIZARD_TOOL_NAMES`, `LONGER_ASK_TIMEOUT_MS`, `flushScanReport`, and `runMcpPromptViaSdk`, which loads the streaming module on first call. + +Minimal invocation: + +```ts +const result = await runAgent(config, input, { + onProgress: (event) => { + if (event.kind === 'log') console.log(event.message); + }, + interaction: { + ask: async (question) => answersFor(question), + }, +}); +if (result.outcome !== RunOutcome.Success) { + process.exitCode = result.failure.exitCode ?? 1; +} +``` + +`src/agent/__tests__/run-agent-standalone.test.ts` runs this with no UI, no store and no registry. + +## Intent + +Programs call the agent to do the work a skill describes. The TUI and the headless runner observe the run through `onProgress` and answer it through `interaction`; today `src/lib/programs/run-agent-legacy.ts` does both on top of the session. + +Without `onProgress` the run completes and its snapshot still comes back in the result. Without `interaction` the agent installs no ask bridge: `wizard_ask` returns its "not available" error and optional task notices are declined, which is what a `--ci` run does. A throwing observer is logged and the run continues. + +## Architecture + +The agent owns run state for one invocation: the task queue, phase, status, resolved skill, handoff text, usage and the final result. It depends on `src/shared` and on `src/env.ts`, and on program types only until the bindings table moves to programs. + +```text +caller ── RunConfig + RunInput ──▶ runAgent + │ prepareRun: gateway mint, triage provider + ▼ + sequence (linear | orchestrator) + │ + harness (anthropic | pi) ── tools (MCP or pi-native) + │ + onProgress ◀── events ───┤──── questions ──▶ interaction + ▼ + RunResult +``` + +`runner/` holds the dispatcher, sequences, harnesses and the switchboard. `tools/` holds the wizard tools shared by both harnesses. `middleware/` holds the benchmark pipeline. `progress.ts` defines the event and interaction contracts; `yara-hooks.ts` scans what the run installs. diff --git a/src/agent/runner/README.md b/src/agent/runner/README.md index 5f43656cb..79a87bfad 100644 --- a/src/agent/runner/README.md +++ b/src/agent/runner/README.md @@ -32,7 +32,7 @@ New models require Wizard capabilities **and** mint model/effort allowlists, gateway provider/transport support, and compatibility with required Wizard and security-triage prompt policies. Local model constants cannot bypass admission. See the -[development guide](../../../../.claude/skills/wizard-development/SKILL.md#execution-policy-and-model-admission) +[development guide](../../../.claude/skills/wizard-development/SKILL.md#execution-policy-and-model-admission) for the coordinated change checklist. ## The pieces diff --git a/src/agent/runner/harness/anthropic/README.md b/src/agent/runner/harness/anthropic/README.md index 87b39992c..d52a8d3d2 100644 --- a/src/agent/runner/harness/anthropic/README.md +++ b/src/agent/runner/harness/anthropic/README.md @@ -12,7 +12,7 @@ supported: `run()` for linear conversations and `runTask()` for orchestrator seed/task calls. Pi also implements both entry points. The SDK subprocess uses the scoped token minted by -[gateway-session.ts](../../../../gateway-session.ts). Wizard explicitly sets the +[gateway-session.ts](../../../gateway-session.ts). Wizard explicitly sets the gateway URL and authentication environment and isolates stored Claude logins. Model selection must satisfy local routing, the SDK's supported transport, mint model/effort allowlists, and the gateway's required prompt policy. The SDK is @@ -22,6 +22,6 @@ Security is enforced through `wizardCanUseTool`, SDK sandbox configuration, and warlock pre/post tool hooks. Sensitive question answers use vault references; write operations are also guarded while a question overlay is open. Read [agent-interface.ts](../../../agent-interface.ts), -[yara-hooks.ts](../../../../yara-hooks.ts), and -[wizard-tools](../../../../wizard-tools/) for current tool registration and +[yara-hooks.ts](../../../yara-hooks.ts), and +[wizard-tools](../../../tools/) for current tool registration and permission behavior rather than maintaining a second tool inventory here. diff --git a/src/agent/runner/sequence/README.md b/src/agent/runner/sequence/README.md index d50ce96ad..ac4436f9a 100644 --- a/src/agent/runner/sequence/README.md +++ b/src/agent/runner/sequence/README.md @@ -29,7 +29,7 @@ prompt and valid task-skill variants. Metrics already has a Pi/orchestrator binding; other routes can be selected through scoped flags or development CLI overrides. The retired `experiment/orchestrator` branch is not a prerequisite. See -[adding a program](../../../../../.claude/skills/adding-skill-program/SKILL.md). +[adding a program](../../../../.claude/skills/adding-skill-program/SKILL.md). ### Task metadata diff --git a/src/shared/README.md b/src/shared/README.md new file mode 100644 index 000000000..ae2a39c25 --- /dev/null +++ b/src/shared/README.md @@ -0,0 +1,43 @@ +# Shared + +Stateless library code every surface may import: constants, the API and host types, error codes, fetch retry, the skill menu, Claude settings handling, the secret vault, health checks and the utilities under `utils/`. Nothing here depends on the agent, the programs, the TUI or the CLI. + +## Signatures + +Import by path: `@shared/` for the singles and `@utils/` for `src/shared/utils`. There is no barrel; a shared library loads what a caller names and nothing else. + +Modules callers reach most: + +- `@shared/constants`: integrations, harnesses, sequences, URLs and `getSkillsBaseUrl()`. +- `@shared/errors`: `ErrorCodes`, `WizardError`, `emitWizardError`, `classifyRunFailure`, `classifyAuthFailure`, the catalog. +- `@shared/api`: `Credentials`, `ApiUser`, `ApiProject`. +- `@shared/host-resolution`: `HostResolution`, the immutable snapshot of where the wizard talks to. +- `@shared/fetch-retry`: `fetchWithRetry(url, { fetchImpl?, sleepImpl?, maxAttempts? })`, one retry and failover policy for every critical-path fetch. +- `@shared/skill-menu`: `fetchSkillMenu(skillsBaseUrl, retryOpts?)` returns the parsed `SkillMenu` or `null`; `expandBundleEntry`, `SkillEntry`, `CliEntry`. +- `@shared/claude-settings`: settings conflict detection, backup and restore. +- `@shared/secret-vault`: the session-scoped vault the tools resolve secret references through. +- `@shared/health-checks`: `evaluateWizardReadiness`, `checkAllExternalServices` and the gateway and skills-origin endpoint checks. +- `@utils/debug`: `logToFile`, `debug`, `enableDebugLogs`, `setDebugSink`. +- `@utils/analytics`: the `analytics` client (`wizardCapture`, `captureException`, `setTag`, `flush`, `shutdown`). +- `@utils/telemetry`: `withProgress(step, fn)` tags analytics with the current step and runs `fn`. +- `@utils/package-manager`, `@utils/env-scan`, `@utils/bounded-fs`, `@utils/atomic-ledger`, `@utils/semver`, `@utils/urls`, `@utils/links`. + +Callback convention: a shared helper that needs to show something takes a sink or returns data. It never looks the UI up. + +```ts +import { debug, setDebugSink } from '@utils/debug'; + +const restore = setDebugSink((line) => myLog.info(line)); +debug('resolving host', host); // rendered and handed to the sink +setDebugSink(restore); +``` + +`src/ui/index.ts` installs the current UI's info log as the debug sink at load, so `debug()` follows `setUI()` without shared code knowing a UI exists. Until the UI module loads, lines go to stdout. + +## Intent + +Shared exists so the agent, programs, TUI, headless and CLI code can use one implementation of the things they all need without importing each other. A helper belongs here when it holds no run state, needs no surface-specific dependency, and would otherwise be copied. + +## Architecture + +Shared imports `src/env.ts` and itself. The architecture test classifies `src/shared` as its own surface and lists the remaining upward edges in `src/__tests__/architecture/known-violations.json`; each has an owner in the stack plan. `utils/setup-utils.ts`, `utils/oauth.ts` and `utils/wizard-abort.ts` are TUI and CLI flow code that leave in Release C; `utils/analytics.ts` reads the session until Release B; `claude-settings.ts` and `errors/agent-map.ts` import two agent leaf modules until Release B, because the agent entry would form a module cycle through analytics. From 67ece688749ab1d10725ae64b2b464626ed1da4b Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 15:07:27 -0400 Subject: [PATCH 05/12] docs(agent): name what stays in the entry modules and what leaves Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- src/agent/gateway-session.ts | 3 +++ src/agent/index.ts | 30 ++++++++++++++++++++++++++++ src/agent/runner/shared/bootstrap.ts | 5 +++++ src/agent/types.ts | 8 ++++++++ 4 files changed, 46 insertions(+) diff --git a/src/agent/gateway-session.ts b/src/agent/gateway-session.ts index 9932f36ad..e8a1ef1b2 100644 --- a/src/agent/gateway-session.ts +++ b/src/agent/gateway-session.ts @@ -72,6 +72,9 @@ export function configureGatewayCredentialsForCI( }; } +// TODO(B2): CI credential loading belongs to the headless provider, not the +// agent. Leaves with the rest of this module once RunInput carries resolved +// inference auth. export function configureGatewayFromCIEnvironment( projectId: number, region: CloudRegion, diff --git a/src/agent/index.ts b/src/agent/index.ts index c014021cc..42ca8a005 100644 --- a/src/agent/index.ts +++ b/src/agent/index.ts @@ -4,6 +4,35 @@ * architecture test. Keep this list to what callers use, and keep it cheap: * the startup chunk imports this module, so anything re-exported here loads * before the wizard does any work. Heavy paths stay behind a lazy import. + * + * Audited against the stack plan (docs: workbench/wizard-functional-stack-plan.md, + * sections 4.1 to 4.5 and 7). What stays and what leaves: + * + * Final, the agent's contract: + * runAgent, RunOutcome the one way to run the agent (4.1) + * AgentSignals prompt marker strings program prompts embed + * WIZARD_TOOL_NAMES tool ids programs put in allowedTools/disallowedTools + * + * Leaves in B1 (bindings and program data move to programs, 4.5): + * resolveBinding keyed by PROGRAM_BINDINGS; agent keeps only + * "run from an already-resolved binding" + * shouldDisableAsk a flags policy programs decide, then pass in + * LONGER_ASK_TIMEOUT_MS a tuning number; programs own askTimeoutMs + * + * Leaves in B2 (programs own credentials and the legacy adapter dies): + * initializeAgent, executeAgent, buildRunTags + * the pre-runAgent surface detection/agentic.ts + * and run-agent-legacy.ts still call; they go + * through runAgent or leave with detection + * configureGatewayFromCIEnvironment + * CI inference auth; headless provider owns it + * flushScanReport the report becomes a progress event, not a call + * + * Leaves in A4 (scan at load; skill install becomes shared): + * downloadSkill + * + * Leaves in C2 (the TUI receives agent data through program state): + * runMcpPromptViaSdk */ export type * from './types'; export { @@ -18,6 +47,7 @@ export { initializeAgent, runAgent as executeAgent, } from './agent-interface'; +// TODO(B2): leaves the entry when the headless provider owns CI gateway auth. export { configureGatewayFromCIEnvironment } from './gateway-session'; export { downloadSkill, WIZARD_TOOL_NAMES } from './tools'; export { LONGER_ASK_TIMEOUT_MS } from './wizard-ask-bridge'; diff --git a/src/agent/runner/shared/bootstrap.ts b/src/agent/runner/shared/bootstrap.ts index 43d8a570f..1ed76bae0 100644 --- a/src/agent/runner/shared/bootstrap.ts +++ b/src/agent/runner/shared/bootstrap.ts @@ -84,6 +84,11 @@ export async function prepareRun( // readers re-resolve through the cache, which re-mints past the refresh // point. const currentGatewayAuth = () => + // TODO(B2): the agent must not mint inference auth. It receives the + // PostHog token here and derives a gateway token from it, re-minting near + // expiry. Programs own credentials (stack plan 4.5): pass a resolved + // inference-auth provider on RunInput.credentials and move + // gateway-session.ts out of src/agent with it. gatewayAuth(credentials.host, credentials.accessToken, programId); await currentGatewayAuth(); diff --git a/src/agent/types.ts b/src/agent/types.ts index 6b42c298a..549a41b09 100644 --- a/src/agent/types.ts +++ b/src/agent/types.ts @@ -2,6 +2,14 @@ * Public type surface of the agent. Type-only, so importing it adds no * runtime dependency. Code outside `src/agent` imports these as * `@agent/types`; runtime values come from `@agent`. + * + * Audited against the stack plan (sections 4.1 to 4.3): the run contract + * (RunConfig, RunInput, RunResult, AgentRunDefinition, AgentFailure, + * AbortCase, PromptContext) and the progress and interaction contracts + * (AgentProgress, AgentInteraction and their payload shapes) are final. + * ProgramBinding and SwitchboardCtx leave in B1 with the bindings table. + * AgentChunk leaves in C2 with runMcpPromptViaSdk. InstallSkillResult + * leaves in A4 with downloadSkill. */ export type { AbortCase, From 3e2f1be2ce042a2be5a5321d2ad4fead64947070 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 15:09:07 -0400 Subject: [PATCH 06/12] docs(agent): skill install leaves with Release B, not a separate stage Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- src/agent/index.ts | 3 ++- src/agent/types.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/agent/index.ts b/src/agent/index.ts index 42ca8a005..f92999dee 100644 --- a/src/agent/index.ts +++ b/src/agent/index.ts @@ -28,7 +28,8 @@ * CI inference auth; headless provider owns it * flushScanReport the report becomes a progress event, not a call * - * Leaves in A4 (scan at load; skill install becomes shared): + * Leaves with Release B, when the skill scan moves to load time and skill + * install becomes shared: * downloadSkill * * Leaves in C2 (the TUI receives agent data through program state): diff --git a/src/agent/types.ts b/src/agent/types.ts index 549a41b09..0f9bad690 100644 --- a/src/agent/types.ts +++ b/src/agent/types.ts @@ -9,7 +9,7 @@ * (AgentProgress, AgentInteraction and their payload shapes) are final. * ProgramBinding and SwitchboardCtx leave in B1 with the bindings table. * AgentChunk leaves in C2 with runMcpPromptViaSdk. InstallSkillResult - * leaves in A4 with downloadSkill. + * leaves with downloadSkill in Release B. */ export type { AbortCase, From c620d09fe15fe9e7ff9383efefcfa0f86f9dd952 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 15:11:22 -0400 Subject: [PATCH 07/12] docs(agent): group the entry exports by when they leave Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- src/agent/index.ts | 76 +++++++++++++++++++++------------------------- src/agent/types.ts | 24 +++++++-------- 2 files changed, 46 insertions(+), 54 deletions(-) diff --git a/src/agent/index.ts b/src/agent/index.ts index f92999dee..0c2c6d22b 100644 --- a/src/agent/index.ts +++ b/src/agent/index.ts @@ -5,59 +5,51 @@ * the startup chunk imports this module, so anything re-exported here loads * before the wizard does any work. Heavy paths stay behind a lazy import. * - * Audited against the stack plan (docs: workbench/wizard-functional-stack-plan.md, - * sections 4.1 to 4.5 and 7). What stays and what leaves: - * - * Final, the agent's contract: - * runAgent, RunOutcome the one way to run the agent (4.1) - * AgentSignals prompt marker strings program prompts embed - * WIZARD_TOOL_NAMES tool ids programs put in allowedTools/disallowedTools - * - * Leaves in B1 (bindings and program data move to programs, 4.5): - * resolveBinding keyed by PROGRAM_BINDINGS; agent keeps only - * "run from an already-resolved binding" - * shouldDisableAsk a flags policy programs decide, then pass in - * LONGER_ASK_TIMEOUT_MS a tuning number; programs own askTimeoutMs - * - * Leaves in B2 (programs own credentials and the legacy adapter dies): - * initializeAgent, executeAgent, buildRunTags - * the pre-runAgent surface detection/agentic.ts - * and run-agent-legacy.ts still call; they go - * through runAgent or leave with detection - * configureGatewayFromCIEnvironment - * CI inference auth; headless provider owns it - * flushScanReport the report becomes a progress event, not a call - * - * Leaves with Release B, when the skill scan moves to load time and skill - * install becomes shared: - * downloadSkill - * - * Leaves in C2 (the TUI receives agent data through program state): - * runMcpPromptViaSdk + * Grouped by fate, per the stack plan (sections 4.1 to 4.5 and 7). + */ + +/** + * Stays. The agent's contract: the one way to run it, the marker strings + * program prompts embed, and the tool ids programs put in allowedTools and + * disallowedTools. */ export type * from './types'; +export { runAgent, RunOutcome } from './runner'; +export { AgentSignals } from './agent-interface'; +export { WIZARD_TOOL_NAMES } from './tools'; + +/** + * Leaves in B1. Bindings and program data move to programs: resolveBinding + * is keyed by PROGRAM_BINDINGS and the agent keeps only "run from an + * already-resolved binding"; shouldDisableAsk is a flags policy programs + * decide and pass in; LONGER_ASK_TIMEOUT_MS is a tuning number programs own + * as askTimeoutMs. + */ +export { resolveBinding, shouldDisableAsk } from './runner'; +export { LONGER_ASK_TIMEOUT_MS } from './wizard-ask-bridge'; + +/** + * Leaves in B2. Programs own credentials and the legacy adapter dies. + * initializeAgent, executeAgent and buildRunTags are the pre-runAgent surface + * that detection/agentic.ts and run-agent-legacy.ts still call; they go + * through runAgent or leave with detection. configureGatewayFromCIEnvironment + * is CI inference auth the headless provider owns. flushScanReport becomes a + * progress event rather than a call. downloadSkill leaves once the skill scan + * runs at load and skill install becomes shared. + */ export { - resolveBinding, - runAgent, - RunOutcome, - shouldDisableAsk, -} from './runner'; -export { - AgentSignals, buildRunTags, initializeAgent, runAgent as executeAgent, } from './agent-interface'; -// TODO(B2): leaves the entry when the headless provider owns CI gateway auth. export { configureGatewayFromCIEnvironment } from './gateway-session'; -export { downloadSkill, WIZARD_TOOL_NAMES } from './tools'; -export { LONGER_ASK_TIMEOUT_MS } from './wizard-ask-bridge'; export { flushScanReport } from './yara-hooks'; +export { downloadSkill } from './tools'; /** - * Stream an MCP tutorial prompt through the SDK. Loaded on first call: the - * streaming module is the one agent module the startup chunk does not already - * hold, and the TUI reaches it only from the suggested-prompts screen. + * Leaves in C2. The TUI receives agent data through program state. Until + * then the suggested-prompts screen streams through this wrapper, which loads + * the streaming module on first call so the startup chunk does not grow. */ export async function* runMcpPromptViaSdk( args: Parameters< diff --git a/src/agent/types.ts b/src/agent/types.ts index 0f9bad690..d9a4b42a8 100644 --- a/src/agent/types.ts +++ b/src/agent/types.ts @@ -1,26 +1,19 @@ /** * Public type surface of the agent. Type-only, so importing it adds no * runtime dependency. Code outside `src/agent` imports these as - * `@agent/types`; runtime values come from `@agent`. - * - * Audited against the stack plan (sections 4.1 to 4.3): the run contract - * (RunConfig, RunInput, RunResult, AgentRunDefinition, AgentFailure, - * AbortCase, PromptContext) and the progress and interaction contracts - * (AgentProgress, AgentInteraction and their payload shapes) are final. - * ProgramBinding and SwitchboardCtx leave in B1 with the bindings table. - * AgentChunk leaves in C2 with runMcpPromptViaSdk. InstallSkillResult - * leaves with downloadSkill in Release B. + * `@agent/types`; runtime values come from `@agent`. Grouped by fate, per the + * stack plan (sections 4.1 to 4.3). */ + +/** Stays. The run contract and the progress and interaction contracts. */ export type { AbortCase, AgentFailure, AgentRunDefinition, - ProgramBinding, PromptContext, RunConfig, RunInput, RunResult, - SwitchboardCtx, } from './runner'; export type { AgentInteraction, @@ -34,5 +27,12 @@ export type { TaskNotice, TokenUsageDelta, } from './progress'; -export type { AgentChunk } from './mcp-prompt-streaming'; + +/** Leaves in B1 with the bindings table. */ +export type { ProgramBinding, SwitchboardCtx } from './runner'; + +/** Leaves in B2 with downloadSkill. */ export type { InstallSkillResult } from './tools'; + +/** Leaves in C2 with runMcpPromptViaSdk. */ +export type { AgentChunk } from './mcp-prompt-streaming'; From 889af716e6f2a5f99d84162b93466f4a251dcd71 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 16:03:10 -0400 Subject: [PATCH 08/12] test(agent): cover public prompt streaming entry --- src/agent/__tests__/entry-streaming.test.ts | 108 ++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 src/agent/__tests__/entry-streaming.test.ts diff --git a/src/agent/__tests__/entry-streaming.test.ts b/src/agent/__tests__/entry-streaming.test.ts new file mode 100644 index 000000000..9ff311d90 --- /dev/null +++ b/src/agent/__tests__/entry-streaming.test.ts @@ -0,0 +1,108 @@ +import { rmSync } from 'node:fs'; +import { runMcpPromptViaSdk } from '@agent'; +import type { AgentChunk } from '@agent/types'; +import { + configureGatewayCredentialsForCI, + resetGatewaySession, +} from '@agent/gateway-session'; +import { HostResolution } from '@shared/host-resolution'; + +const { query } = vi.hoisted(() => ({ + query: + vi.fn< + (args: { + options: { env: { CLAUDE_CONFIG_DIR: string } }; + }) => Generator + >(), +})); + +vi.mock('@anthropic-ai/claude-agent-sdk', () => ({ query })); + +async function consume( + overrides: Partial[0]> = {}, +): Promise { + const chunks: AgentChunk[] = []; + for await (const chunk of runMcpPromptViaSdk({ + prompt: 'List events', + credentials: { + accessToken: 'test-access-token', + projectApiKey: 'test-project-key', + projectId: 1, + host: HostResolution.fromRegion('us'), + }, + signal: new AbortController().signal, + programId: 'mcp-tutorial', + ...overrides, + })) { + chunks.push(chunk); + } + return chunks; +} + +describe('public agent prompt stream', () => { + beforeEach(() => { + for (const name of [ + 'ANTHROPIC_BASE_URL', + 'ANTHROPIC_AUTH_TOKEN', + 'CLAUDE_CODE_OAUTH_TOKEN', + 'CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS', + ]) { + vi.stubEnv(name, process.env[name]); + } + configureGatewayCredentialsForCI( + 'test-gateway-token', + 1, + 'https://ai-gateway.us.posthog.com', + ); + }); + + afterEach(() => { + for (const [args] of query.mock.calls) { + rmSync(args.options.env.CLAUDE_CONFIG_DIR, { + recursive: true, + force: true, + }); + } + query.mockReset(); + resetGatewaySession(); + vi.unstubAllEnvs(); + }); + + it('forwards text and completion with the resumable session ID', async () => { + query.mockImplementation(function* () { + yield { + type: 'assistant', + message: { content: [{ type: 'text', text: 'Found three events.' }] }, + }; + yield { type: 'result', subtype: 'success', session_id: 'session-123' }; + }); + + await expect(consume()).resolves.toEqual([ + { kind: 'text', text: 'Found three events.' }, + { kind: 'done', sessionId: 'session-123' }, + ]); + }); + + it('forwards an SDK stream failure after partial output', async () => { + query.mockImplementation(function* () { + yield { + type: 'assistant', + message: { content: [{ type: 'text', text: 'Checking events...' }] }, + }; + throw new Error('SDK connection lost'); + }); + + await expect(consume()).resolves.toEqual([ + { kind: 'text', text: 'Checking events...' }, + { kind: 'error', text: 'SDK connection lost' }, + ]); + }); + + it('propagates setup failures instead of silently ending the stream', async () => { + resetGatewaySession(); + + await expect(consume({ programId: undefined })).rejects.toThrow( + 'this run has no program to attribute its spend to', + ); + }); +}); From 2aee2503205354a26bc551d8b91331762873abf1 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 20:33:22 -0400 Subject: [PATCH 09/12] fix(agent): propagate terminal failures and cancel sibling work --- src/agent/README.md | 3 +- src/agent/__tests__/agent-interface.test.ts | 174 ++++++++++-- src/agent/__tests__/aio-capture.test.ts | 28 ++ .../__tests__/run-agent-standalone.test.ts | 103 ++++++- src/agent/__tests__/wizard-ask-bridge.test.ts | 68 +++++ src/agent/agent-interface.ts | 265 ++++++++++++++---- src/agent/aio-capture.ts | 14 +- src/agent/runner/README.md | 49 +++- src/agent/runner/harness/anthropic/index.ts | 4 +- .../harness/pi/__tests__/cancellation.test.ts | 42 +++ .../harness/pi/__tests__/gateway.test.ts | 79 ++++++ .../harness/pi/__tests__/mcp-env.test.ts | 43 +++ src/agent/runner/harness/pi/cancellation.ts | 28 ++ src/agent/runner/harness/pi/completion.ts | 4 +- src/agent/runner/harness/pi/gateway.ts | 65 ++++- src/agent/runner/harness/pi/index.ts | 140 ++++++++- src/agent/runner/harness/pi/mcp.ts | 90 ++++-- src/agent/runner/harness/pi/subagent.ts | 14 + src/agent/runner/harness/pi/task.ts | 129 ++++++++- src/agent/runner/harness/types.ts | 30 +- src/agent/runner/index.ts | 168 ++++++++--- src/agent/runner/sequence/linear.ts | 129 ++++----- .../orchestrator/__tests__/executor.test.ts | 56 +++- .../__tests__/task-notice-timeout.test.ts | 12 + .../runner/sequence/orchestrator/executor.ts | 37 ++- .../orchestrator/orchestrator-runner.ts | 262 ++++++++++++----- src/agent/runner/shared/ask.ts | 2 + src/agent/runner/shared/errors.ts | 25 +- src/agent/runner/shared/progress-collector.ts | 31 +- src/agent/runner/shared/types.ts | 11 +- src/agent/wizard-ask-bridge.ts | 29 +- .../__tests__/agentic-progress.test.ts | 39 ++- src/lib/detection/agentic.ts | 10 +- .../__tests__/run-agent-legacy.test.ts | 34 ++- src/lib/programs/run-agent-legacy.ts | 3 + 35 files changed, 1850 insertions(+), 370 deletions(-) create mode 100644 src/agent/runner/harness/pi/__tests__/cancellation.test.ts create mode 100644 src/agent/runner/harness/pi/__tests__/mcp-env.test.ts create mode 100644 src/agent/runner/harness/pi/cancellation.ts diff --git a/src/agent/README.md b/src/agent/README.md index 1c86b2e08..0d7de1ba7 100644 --- a/src/agent/README.md +++ b/src/agent/README.md @@ -13,6 +13,7 @@ import type { RunConfig, RunInput, RunResult, AgentProgress } from '@agent/types runAgent(config: RunConfig, input: RunInput, options?: { onProgress?: (event: AgentProgress) => void; interaction?: AgentInteraction; + signal?: AbortSignal; }): Promise ``` @@ -21,7 +22,7 @@ runAgent(config: RunConfig, input: RunInput, options?: { - `RunResult`: `outcome` is `RunOutcome.Success | Aborted | Failed | Crashed`. Success may carry an `outro`; the other three carry a `failure` (`AgentFailure`: message, outro data, error, exit code, error code, detail). Every result carries `skillId` and a `snapshot` of what the run reported: tasks, status lines, stage, token usage totals, final cost, dashboard and notebook URLs, handoff text. - `AgentProgress`: one event per thing the run reports, in emission order. Kinds: `lifecycle`, `spinner`, `log`, `status`, `tasks`, `stage`, `url`, `usage`, `finalCost`, `authError`, `handoff`, `completion`. Payloads are copies, never live objects. - `AgentInteraction`: every member optional. `ask(question)` resolves with answers, `cancelAsk()` dismisses the open question, `taskNotice(notice)` resolves with whether to keep an optional task, `cancelTaskNotice()` declines it. -- Errors: the agent does not exit the process and does not throw for a decided failure. An unexpected throw becomes `outcome: Crashed` with the error attached. A gateway 401 emits `authError` and then fails. +- Errors: the agent does not exit the process and returns decided failures. A caught coded error becomes `Failed`. An uncoded throw becomes `Crashed` with the error attached. A gateway 401 returns an auth failure. The host decides whether to show auth UI. Other runtime exports: `resolveBinding`, `shouldDisableAsk`, `initializeAgent`, `executeAgent`, `buildRunTags`, `AgentSignals`, `configureGatewayFromCIEnvironment`, `downloadSkill`, `WIZARD_TOOL_NAMES`, `LONGER_ASK_TIMEOUT_MS`, `flushScanReport`, and `runMcpPromptViaSdk`, which loads the streaming module on first call. diff --git a/src/agent/__tests__/agent-interface.test.ts b/src/agent/__tests__/agent-interface.test.ts index a1bec56b2..2b69ebc11 100644 --- a/src/agent/__tests__/agent-interface.test.ts +++ b/src/agent/__tests__/agent-interface.test.ts @@ -115,6 +115,133 @@ describe('runAgent', () => { }); describe('race condition handling', () => { + it('returns a failure for an SDK error result without an API marker', async () => { + function* failed() { + yield { + type: 'result', + subtype: 'error_max_turns', + is_error: true, + errors: ['maximum turns reached'], + }; + } + mockQuery.mockReturnValue(failed()); + const result = await runAgent( + defaultAgentConfig, + 'test prompt', + defaultOptions, + mockSpinner as unknown as SpinnerHandle, + ); + expect(result).toMatchObject({ + kind: 'failure', + classification: 'WIZARD_API_ERROR', + message: 'maximum turns reached', + }); + expect(mockSpinner.stop).not.toHaveBeenCalledWith('Test success'); + }); + + it('classifies an assistant structured 429 before a generic error result', async () => { + function* rateLimited() { + yield { + type: 'assistant', + error: { status: 429 }, + message: { role: 'assistant', content: [] }, + }; + yield { + type: 'result', + subtype: 'error_during_execution', + is_error: true, + errors: ['request failed'], + }; + } + mockQuery.mockReturnValue(rateLimited()); + const result = await runAgent( + defaultAgentConfig, + 'test prompt', + defaultOptions, + mockSpinner as unknown as SpinnerHandle, + ); + expect(result).toMatchObject({ + kind: 'failure', + classification: 'WIZARD_RATE_LIMIT', + }); + }); + + it('returns an abort when the host cancels and the SDK throws AbortError', async () => { + const controller = new AbortController(); + mockQuery.mockReturnValue({ + [Symbol.asyncIterator]() { + return { + next() { + controller.abort(); + return Promise.reject( + Object.assign(new Error('SDK aborted'), { + name: 'AbortError', + }), + ); + }, + }; + }, + }); + const result = await runAgent( + { ...defaultAgentConfig, signal: controller.signal }, + 'test prompt', + defaultOptions, + mockSpinner as unknown as SpinnerHandle, + ); + expect(result).toMatchObject({ + kind: 'abort', + classification: 'WIZARD_ABORT', + }); + }); + + it('returns a failure when the stream ends without a terminal result', async () => { + function* exhausted() { + yield { type: 'system', subtype: 'init', tools: [], mcp_servers: [] }; + } + mockQuery.mockReturnValue(exhausted()); + const result = await runAgent( + defaultAgentConfig, + 'test prompt', + defaultOptions, + mockSpinner as unknown as SpinnerHandle, + ); + expect(result).toMatchObject({ + kind: 'failure', + classification: 'WIZARD_API_ERROR', + }); + }); + + it('does not treat quoted API error prose as an auth failure', async () => { + function* quoted() { + yield { + type: 'assistant', + message: { + role: 'assistant', + content: [ + { + type: 'text', + text: 'API Error: 401 appears in the user log, but I fixed the issue.', + }, + ], + }, + }; + yield { + type: 'result', + subtype: 'success', + is_error: false, + result: 'done', + }; + } + mockQuery.mockReturnValue(quoted()); + const result = await runAgent( + defaultAgentConfig, + 'test prompt', + defaultOptions, + mockSpinner as unknown as SpinnerHandle, + ); + expect(result).toEqual({ kind: 'success' }); + }); + it('should return success when agent completes successfully then SDK cleanup fails', async () => { // This simulates the race condition: // 1. Agent completes with success result @@ -157,7 +284,7 @@ describe('runAgent', () => { ); // Should return success (empty object), not throw - expect(result).toEqual({}); + expect(result).toEqual({ kind: 'success' }); expect(mockSpinner.stop).toHaveBeenCalledWith('Test success'); }); @@ -232,8 +359,13 @@ describe('runAgent', () => { ); // Should return API error, not success - expect(result.error).toBe('WIZARD_API_ERROR'); - expect(result.message).toContain('API Error'); + expect(result.kind).toBe('failure'); + expect(result.kind === 'failure' && result.classification).toBe( + 'WIZARD_API_ERROR', + ); + expect(result.kind === 'failure' && result.message).toContain( + 'API Error', + ); }); it('should suppress user-facing errors when SDK yields error result after success', async () => { @@ -297,7 +429,7 @@ describe('runAgent', () => { ); // Should return success (empty object), not error - expect(result).toEqual({}); + expect(result).toEqual({ kind: 'success' }); expect(mockSpinner.stop).toHaveBeenCalledWith('Test success'); // ui.log.error should NOT have been called (errors suppressed for user) @@ -354,7 +486,7 @@ describe('runAgent', () => { }, ); - expect(result).toEqual({}); + expect(result).toEqual({ kind: 'success' }); expect(mockSpinner.stop).toHaveBeenCalledWith('Test success'); expect(mockUIInstance.log.error).not.toHaveBeenCalled(); }); @@ -394,7 +526,7 @@ describe('runAgent', () => { }, ); - expect(result).toEqual({}); + expect(result).toEqual({ kind: 'success' }); expect(mockSpinner.stop).toHaveBeenCalledWith('Test success'); }); }); @@ -740,6 +872,7 @@ describe('gateway re-mint on 401', () => { yield { type: 'assistant', session_id: id, + error: { status: 401 }, message: { role: 'assistant', content: [ @@ -791,7 +924,7 @@ describe('gateway re-mint on 401', () => { const result = await run(cfg); - expect(result).toEqual({}); + expect(result).toEqual({ kind: 'success' }); expect(refresh).toHaveBeenCalledTimes(1); expect(authErrors()).toHaveLength(0); expect(mockQuery).toHaveBeenCalledTimes(2); @@ -832,12 +965,16 @@ describe('gateway re-mint on 401', () => { expect(refresh).toHaveBeenCalledTimes(1); expect(mockQuery).toHaveBeenCalledTimes(2); - expect(authErrors()).toHaveLength(1); + expect(authErrors()).toHaveLength(0); // The auth screen is reported, and the decided failure goes back to the // caller, which owns the exit. - expect(result.error).toBeUndefined(); - expect(result.failure?.message).toBe('Authentication failed (401)'); - expect(result.failure?.code).toBeDefined(); + expect(result.kind).toBe('decided_failure'); + expect(result.kind === 'decided_failure' && result.failure.message).toBe( + 'Authentication failed (401)', + ); + expect( + result.kind === 'decided_failure' && result.failure.code, + ).toBeDefined(); }); it('judges a failed resumed session on its own error, not the old 401', async () => { @@ -871,9 +1008,12 @@ describe('gateway re-mint on 401', () => { // The 401 that triggered the re-mint is history; reporting it here would // send the user to the auth screen for a 500. - expect(result.error).toBe('WIZARD_API_ERROR'); - expect(result.message).toContain('500'); - expect(result.message).not.toContain('401'); + expect(result.kind).toBe('failure'); + expect(result.kind === 'failure' && result.classification).toBe( + 'WIZARD_API_ERROR', + ); + expect(result.kind === 'failure' && result.message).toContain('500'); + expect(result.kind === 'failure' && result.message).not.toContain('401'); expect(authErrors()).toHaveLength(0); }); @@ -888,8 +1028,10 @@ describe('gateway re-mint on 401', () => { // A fresh token the gateway rejects is a bad credential, not age. expect(refresh).not.toHaveBeenCalled(); expect(mockQuery).toHaveBeenCalledTimes(1); - expect(authErrors()).toHaveLength(1); - expect(result.failure?.message).toBe('Authentication failed (401)'); + expect(authErrors()).toHaveLength(0); + expect(result.kind === 'decided_failure' && result.failure.message).toBe( + 'Authentication failed (401)', + ); }); }); diff --git a/src/agent/__tests__/aio-capture.test.ts b/src/agent/__tests__/aio-capture.test.ts index 5d0c8d90b..f949a7ec9 100644 --- a/src/agent/__tests__/aio-capture.test.ts +++ b/src/agent/__tests__/aio-capture.test.ts @@ -388,6 +388,7 @@ describe('createAioCapture', () => { type: 'agent_end', willRetry: false, }); + capture.finishPiRun(false); await flushMicrotasks(); const traceBody = JSON.parse(fetchMock.mock.calls[0][1].body); @@ -395,6 +396,33 @@ describe('createAioCapture', () => { expect(traceBody.properties.$ai_span_name).toBe('posthog-integration'); }); + it('marks a failed Pi final turn and trace as errors', async () => { + const capture = createAioCapture(BASE_ARGS); + capture.captureFromPiMessageEndEvent({ + type: 'message_end', + message: { + role: 'assistant', + model: 'claude-sonnet-4-5', + content: [], + stopReason: 'error', + errorMessage: '429 rate limited', + }, + }); + capture.finishPiRun(true); + await flushMicrotasks(); + const bodies = fetchMock.mock.calls.map((call) => + JSON.parse(call[1].body), + ); + expect( + bodies.find((body) => body.event === '$ai_generation').properties + .$ai_is_error, + ).toBe(true); + expect( + bodies.find((body) => body.event === '$ai_trace').properties + .$ai_is_error, + ).toBe(true); + }); + it('does not emit $ai_trace on pi agent_end with willRetry=true', async () => { const capture = createAioCapture(BASE_ARGS); capture.captureFromPiMessageEndEvent({ diff --git a/src/agent/__tests__/run-agent-standalone.test.ts b/src/agent/__tests__/run-agent-standalone.test.ts index d03bd9890..cb26b03d2 100644 --- a/src/agent/__tests__/run-agent-standalone.test.ts +++ b/src/agent/__tests__/run-agent-standalone.test.ts @@ -16,11 +16,12 @@ import { type AskAnswers, type PendingQuestion, } from '@lib/wizard-session'; -import { AGENT_ERROR_CODE, ErrorCodes } from '@shared/errors'; +import { AGENT_ERROR_CODE, ErrorCodes, WizardError } from '@shared/errors'; import { AgentErrorType } from '@agent/signals'; import type { AgentFailure } from '@agent/runner/shared/types'; import type { AgentProgress } from '@agent/progress'; import type { + AgentResult, AgentHarness, BackendRunInputs, TaskRunInputs, @@ -64,12 +65,13 @@ vi.mock('@agent/gateway-session', async (importOriginal) => ({ // The fake harness: reports a little of everything, then returns what the // current test told it to. const harnessState = vi.hoisted(() => ({ - result: {} as { error?: string; message?: string; failure?: unknown }, + result: { kind: 'success' } as AgentResult, throws: undefined as Error | undefined, lastInputs: undefined as unknown, tasks: [] as TaskRunInputs[], selected: [] as Harness[], taskFailure: undefined as AgentFailure | undefined, + taskThrow: undefined as Error | undefined, seedFailure: undefined as AgentFailure | undefined, askQuestions: undefined as PendingQuestion['questions'] | undefined, })); @@ -91,10 +93,12 @@ vi.mock('@agent/runner/switchboard/harness', () => { const { store, currentTaskId } = inputs.orchestrator; if (!currentTaskId) { if (harnessState.seedFailure) - return Promise.resolve({ failure: harnessState.seedFailure }); + return { kind: 'decided_failure', failure: harnessState.seedFailure }; store.enqueue({ type: 'install' }); + } else if (harnessState.taskThrow) { + throw harnessState.taskThrow; } else if (harnessState.taskFailure) { - return Promise.resolve({ failure: harnessState.taskFailure }); + return { kind: 'decided_failure', failure: harnessState.taskFailure }; } else { await askIfRequested(inputs); store.complete(currentTaskId, { @@ -103,7 +107,7 @@ vi.mock('@agent/runner/switchboard/harness', () => { forNextAgent: 'done', }); } - return Promise.resolve({}); + return { kind: 'success' }; }, async run(inputs: BackendRunInputs) { harnessState.lastInputs = inputs; @@ -133,7 +137,7 @@ vi.mock('@agent/runner/switchboard/harness', () => { await askIfRequested(inputs); if (harnessState.throws) throw harnessState.throws; spinner.stop('Done'); - return harnessState.result as never; + return harnessState.result; }, }; return { @@ -243,10 +247,11 @@ const input = (over: Partial = {}): RunInput => ({ beforeEach(() => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'run-agent-standalone-')); - harnessState.result = {}; + harnessState.result = { kind: 'success' }; harnessState.tasks = []; harnessState.selected = []; harnessState.taskFailure = undefined; + harnessState.taskThrow = undefined; harnessState.seedFailure = undefined; harnessState.throws = undefined; harnessState.lastInputs = undefined; @@ -381,7 +386,10 @@ describe('runAgent standalone', () => { ); it('cleans up when the seed fails before the drain starts', async () => { - const failure = { message: 'Authentication failed (401)' }; + const failure = { + code: ErrorCodes.AgentApiError, + message: 'Authentication failed (401)', + }; harnessState.seedFailure = failure; const result = await runAgent( config({ @@ -431,6 +439,38 @@ describe('runAgent standalone', () => { expect(fs.existsSync(path.join(tmp, QUEUE_DIR_NAME))).toBe(false); }); + it('treats a coded task harness rejection as run-fatal without retrying', async () => { + const error = new WizardError( + 'Gateway rejected the request', + {}, + ErrorCodes.AgentApiError, + ); + harnessState.taskThrow = error; + const result = await runAgent( + config({ + binding: { + harness: Harness.pi, + sequence: Sequence.orchestrator, + model: DEFAULT_AGENT_MODEL, + }, + switchboard: { + program: 'test-program', + flags: {}, + cliHarness: Harness.pi, + }, + }), + input(), + ); + expect(result.outcome).toBe('failed'); + expect(result.failure).toMatchObject({ + code: ErrorCodes.AgentApiError, + message: error.message, + }); + expect(result.failure?.error).toBe(error); + expect(harnessState.tasks).toHaveLength(2); + expect(fs.existsSync(path.join(tmp, QUEUE_DIR_NAME))).toBe(false); + }); + it.each([Sequence.linear, Sequence.orchestrator])( 'preserves %s completion, shutdown and scan-flush ordering', async (sequence) => { @@ -571,7 +611,8 @@ describe('runAgent standalone', () => { it('returns an agent abort as a decided failure with the matched case', async () => { harnessState.result = { - error: AgentErrorType.ABORT, + kind: 'abort', + classification: AgentErrorType.ABORT, message: 'No Stripe found', }; const events: AgentProgress[] = []; @@ -588,15 +629,16 @@ describe('runAgent standalone', () => { body: 'Stripe is required.', errorDetail: { reason: 'No Stripe found' }, }); - expect(result.failure?.error?.message).toBe( - 'Agent aborted: No Stripe found', - ); + expect(result.failure?.error).toBeUndefined(); expect(events.some((e) => e.kind === 'completion')).toBe(false); expect(analytics.shutdown).not.toHaveBeenCalled(); }); it('returns a coded failure for a harness error', async () => { - harnessState.result = { error: AgentErrorType.NO_PROGRESS }; + harnessState.result = { + kind: 'failure', + classification: AgentErrorType.NO_PROGRESS, + }; const result = await runAgent(config(), input()); @@ -607,9 +649,23 @@ describe('runAgent standalone', () => { expect(result.failure?.message).toContain('without changing your project'); }); + it('resolves a crash even when the thrown Error has hostile getters', async () => { + const hostile = new Error('hidden'); + Object.defineProperty(hostile, 'message', { + get() { + throw new Error('message getter'); + }, + }); + harnessState.throws = hostile; + const result = await runAgent(config(), input()); + expect(result.outcome).toBe(RunOutcome.Crashed); + expect(result.failure?.error).toBe(hostile); + expect(result.failure?.code).toBe(ErrorCodes.InternalUnhandled); + }); + it('passes a harness-decided failure through untouched', async () => { const failure = { code: ErrorCodes.AgentAbort, message: 'decided' }; - harnessState.result = { failure }; + harnessState.result = { kind: 'decided_failure', failure }; const result = await runAgent(config(), input()); @@ -644,6 +700,25 @@ describe('runAgent standalone', () => { expect(result.snapshot.tasks).toHaveLength(1); }); + it('finishes when an async observer rejects', async () => { + const result = await runAgent(config(), input(), { + onProgress: () => Promise.reject(new Error('observer rejected')), + }); + expect(result.outcome).toBe(RunOutcome.Success); + expect(result.snapshot.tasks).toHaveLength(1); + }); + + it('returns an aborted result before setup for a pre-aborted host signal', async () => { + const controller = new AbortController(); + controller.abort(); + const result = await runAgent(config(), input(), { + signal: controller.signal, + }); + expect(result.outcome).toBe(RunOutcome.Aborted); + expect(result.failure?.code).toBe(ErrorCodes.AgentAbort); + expect(harnessState.lastInputs).toBeUndefined(); + }); + it('calls the bound hooks with the run credentials', async () => { const postRun = vi.fn().mockResolvedValue(undefined); const buildOutroData = vi.fn(() => ({ diff --git a/src/agent/__tests__/wizard-ask-bridge.test.ts b/src/agent/__tests__/wizard-ask-bridge.test.ts index 0f348ab17..d17a33a3d 100644 --- a/src/agent/__tests__/wizard-ask-bridge.test.ts +++ b/src/agent/__tests__/wizard-ask-bridge.test.ts @@ -19,6 +19,49 @@ beforeEach(() => { }); describe('createWizardAskBridge', () => { + it('dismisses and settles an active question on run cancellation', async () => { + const controller = new AbortController(); + const cancelQuestion = vi.fn(); + const bridge = createWizardAskBridge({ + signal: controller.signal, + getSource: () => 'skill', + showQuestion: () => new Promise(() => undefined), + cancelQuestion, + }); + const result = bridge.request({ + questions: [{ id: 'goal', prompt: 'Goal?', kind: 'text' }], + }); + expect(bridge.getPendingQuestion()).not.toBeNull(); + controller.abort(); + await expect(result).resolves.toEqual({ + answers: { goal: CANCELLED_SENTINEL }, + timedOut: false, + }); + expect(cancelQuestion).toHaveBeenCalledTimes(1); + expect(bridge.getPendingQuestion()).toBeNull(); + }); + + it('does not open a question when the run was already cancelled', async () => { + const controller = new AbortController(); + controller.abort(); + const showQuestion = vi.fn(); + const bridge = createWizardAskBridge({ + signal: controller.signal, + getSource: () => 'skill', + showQuestion, + }); + await expect( + bridge.request({ + questions: [{ id: 'goal', prompt: 'Goal?', kind: 'text' }], + }), + ).resolves.toEqual({ + answers: { goal: CANCELLED_SENTINEL }, + timedOut: false, + }); + expect(showQuestion).not.toHaveBeenCalled(); + expect(bridge.getPendingQuestion()).toBeNull(); + }); + it('forwards questions to showQuestion and resolves with the captured answers', async () => { const captured: PendingQuestion[] = []; let resolveAnswers!: (answers: AskAnswers) => void; @@ -255,6 +298,31 @@ describe('createWizardAskBridge', () => { } }); + it('settles a timed-out question when host dismissal throws', async () => { + vi.useFakeTimers(); + try { + const bridge = createWizardAskBridge({ + getSource: () => 'product-tours', + showQuestion: () => new Promise(() => undefined), + cancelQuestion: () => { + throw new Error('overlay broken'); + }, + timeoutMs: 1000, + }); + const result = bridge.request({ + questions: [{ id: 'goal', prompt: 'Goal?', kind: 'text' }], + }); + vi.advanceTimersByTime(1000); + await expect(result).resolves.toEqual({ + answers: { goal: CANCELLED_SENTINEL }, + timedOut: true, + }); + expect(bridge.getPendingQuestion()).toBeNull(); + } finally { + vi.useRealTimers(); + } + }); + it('does not dismiss the overlay when the user answers before the timeout', async () => { vi.useFakeTimers(); try { diff --git a/src/agent/agent-interface.ts b/src/agent/agent-interface.ts index c418e5c19..a118b29ea 100644 --- a/src/agent/agent-interface.ts +++ b/src/agent/agent-interface.ts @@ -29,6 +29,7 @@ import { ADDITIONAL_FEATURE_PROMPTS, } from '@shared/constants'; import type { AgentFailure } from './runner/shared/types'; +import type { AgentResult } from './runner/harness/types'; import { createCustomHeaders } from '@utils/custom-headers'; import type { HostResolution } from '@shared/host-resolution'; import { @@ -55,7 +56,7 @@ import { REMARK_INSTRUCTION, RESUME_INSTRUCTION, } from './signals'; -import { classifyAuthFailure, WizardError } from '@shared/errors'; +import { classifyAuthFailure } from '@shared/errors'; import { isGrantRevoked } from '@shared/auth-session-state'; import { AgentOutputSignals } from './output-signals'; @@ -318,6 +319,7 @@ export function createStopHook( * Internal configuration object returned by initializeAgent */ type AgentRunConfig = { + signal?: AbortSignal; workingDirectory: string; mcpServers: McpServersConfig; model: string; @@ -716,6 +718,52 @@ export async function initializeAgent( * * @returns An object containing any error detected in the agent's output */ +function sdkErrorStatus(value: unknown): number | undefined { + if (typeof value === 'number' && value >= 400 && value < 600) return value; + if (typeof value === 'string') { + const match = value.match(/(?:^|\b)(4\d\d|5\d\d)(?:\b|$)/); + return match ? Number(match[1]) : undefined; + } + if (value && typeof value === 'object') { + const record = value as Record; + return ( + sdkErrorStatus(record.status) ?? + sdkErrorStatus(record.statusCode) ?? + sdkErrorStatus(record.code) ?? + sdkErrorStatus(record.error) + ); + } + return undefined; +} + +function sdkResultFailure( + message: Record, +): Extract | undefined { + if (message.subtype === 'success' && message.is_error !== true) + return undefined; + const errors = Array.isArray(message.errors) ? message.errors : []; + const detail = + errors + .map((error) => + typeof error === 'string' ? error : JSON.stringify(error), + ) + .join('; ') || + (typeof message.result === 'string' + ? message.result + : String(message.subtype ?? 'SDK result failed')); + const status = + sdkErrorStatus(message.api_error_status) ?? + sdkErrorStatus(message.status) ?? + errors.map(sdkErrorStatus).find((code) => code !== undefined) ?? + sdkErrorStatus(message.result); + return { + kind: 'failure', + classification: + status === 429 ? AgentErrorType.RATE_LIMIT : AgentErrorType.API_ERROR, + message: detail, + }; +} + export async function runAgent( agentConfig: AgentRunConfig, prompt: string, @@ -752,11 +800,14 @@ export async function runAgent( onMessage(message: any): void; finalize(resultMessage: any, totalDurationMs: number): any; }, -): Promise<{ - error?: AgentErrorType; - message?: string; - failure?: AgentFailure; -}> { +): Promise { + if (agentConfig.signal?.aborted) { + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; + } const emit = agentConfig.emit ?? NO_PROGRESS; const { spinnerMessage = 'Customizing your PostHog setup...', @@ -783,6 +834,8 @@ export async function runAgent( let receivedSuccessResult = false; let loggedInitialContext = false; let lastResultMessage: any = null; + let terminalFailure: Extract | undefined; + let assistantErrorStatus: number | undefined; // SDK >=0.3.142 replaced TodoWrite (snapshot) with TaskCreate/TaskUpdate (accumulate by id). // The agent's TaskCreate tool_use doesn't know the assigned taskId — the SDK returns it @@ -814,9 +867,7 @@ export async function runAgent( }; // Helper to handle successful completion (used in normal path and race condition recovery) - const completeWithSuccess = ( - suppressedError?: Error, - ): { error?: AgentErrorType; message?: string } => { + const completeWithSuccess = (suppressedError?: Error): AgentResult => { const durationMs = Date.now() - startTime; const durationSeconds = Math.round(durationMs / 1000); @@ -879,13 +930,17 @@ export async function runAgent( logToFile(`${AgentSignals.BENCHMARK} Middleware finalize error:`, e); } spinner.stop(successMessage); - return {}; + return { kind: 'success' }; }; // Abort controller — lets us force-kill the SDK query when we detect an // [ABORT] signal in the agent's output. Also stashes the reason so the // runner can surface it via outroData after we unwind. let abortController = new AbortController(); + const onExternalAbort = () => { + abortController.abort(); + signalDone(); + }; let abortReason: string | null = null; // Set when a YARA hook detects a terminal violation. Returning `stopReason` // from a PostToolUse hook does NOT stop the SDK, so we abort the query and @@ -900,6 +955,10 @@ export async function runAgent( // failure the caller ends the run with. The query is aborted to unwind. let authFailure: AgentFailure | undefined; const agentConfigDir = createIsolatedAgentConfigDir(); + agentConfig.signal?.addEventListener('abort', onExternalAbort, { + once: true, + }); + if (agentConfig.signal?.aborted) onExternalAbort(); try { // Per-program allow/disallow lists tweak BASE_ALLOWED_TOOLS. Skills are @@ -1189,6 +1248,10 @@ export async function runAgent( emitStepEvents, resolveStepKey, ); + if (message.type === 'assistant') { + assistantErrorStatus = + sdkErrorStatus(message.error) ?? assistantErrorStatus; + } // [ABORT] detection: the skill emits "[ABORT] " when it // cannot complete the program. Kill the SDK query immediately — @@ -1220,12 +1283,24 @@ export async function runAgent( // 401 on a bearer past its refresh instant: it aged out, so re-mint // once and resume. Any other 401 is a bad credential: show the auth // error screen and exit. - if (message.type === 'assistant' && signals.hasApiErrorStatus(401)) { + if ( + (message.type === 'assistant' && + sdkErrorStatus(message.error) === 401) || + (message.type === 'result' && + (sdkErrorStatus(message.api_error_status) === 401 || + sdkErrorStatus(message.status) === 401)) || + (message.type === 'result' && + Array.isArray(message.errors) && + message.errors.some( + (error: unknown) => sdkErrorStatus(error) === 401, + )) + ) { signalDone(); if ( agentConfig.refreshGatewayAuth && !reminted && - isPastRefresh(agentConfig.gatewayAuth) + isPastRefresh(agentConfig.gatewayAuth) && + !agentConfig.signal?.aborted ) { logToFile( 'Agent error: 401 on an aged gateway bearer; re-minting', @@ -1261,9 +1336,12 @@ export async function runAgent( ...authError, sessionExpired, }); - emit({ - kind: 'authError', - detail: { + // The caller ends the run with this; the query is abandoned here + // where the process used to exit. + authFailure = { + code: authCode, + message: 'Authentication failed (401)', + authErrorDetail: { hasSettingsConflict: authError.hasSettingsConflict, conflicts: authError.conflicts, usingManagedLogin: authError.usingManagedLogin, @@ -1271,25 +1349,19 @@ export async function runAgent( sessionExpired, logFilePath: getLogFilePath(), }, - }); - // The caller ends the run with this; the query is abandoned here - // where the process used to exit. - authFailure = { - code: authCode, - message: 'Authentication failed (401)', - error: new WizardError( - 'Authentication failed', - { - hasSettingsConflict: authError.hasSettingsConflict, - conflictSources: authError.conflictSources, - conflictKeys: authError.conflictKeys, - gatewayUrl: authError.gatewayUrl, - region: authError.region, - usingManagedLogin: authError.usingManagedLogin, - apiKeySource: authError.apiKeySource, - }, - authCode, - ), + detail: { + hasSettingsConflict: authError.hasSettingsConflict, + conflictSources: authError.conflictSources, + conflictKeys: authError.conflictKeys, + conflicts: authError.conflicts, + credentialPlaces: authError.credentialPlaces, + sessionExpired, + logFilePath: getLogFilePath(), + gatewayUrl: authError.gatewayUrl, + region: authError.region, + usingManagedLogin: authError.usingManagedLogin, + apiKeySource: authError.apiKeySource, + }, }; abortController.abort(); break; @@ -1311,6 +1383,14 @@ export async function runAgent( if (message.subtype === 'success' && !message.is_error) { receivedSuccessResult = true; lastResultMessage = message; + } else if (!receivedSuccessResult) { + terminalFailure = sdkResultFailure(message); + if (terminalFailure && assistantErrorStatus === 429) { + terminalFailure = { + ...terminalFailure, + classification: AgentErrorType.RATE_LIMIT, + }; + } } signalDone(); } @@ -1325,17 +1405,28 @@ export async function runAgent( }; const refreshGatewayAuth = agentConfig.refreshGatewayAuth; - if ((await runQuery()) === 'remint' && refreshGatewayAuth) { + if ( + (await runQuery()) === 'remint' && + refreshGatewayAuth && + !agentConfig.signal?.aborted + ) { // The subprocess froze the dead bearer in its env at spawn, so it cannot // be handed a new one: mint, then resume the session in a new one. reminted = true; remintRequested = false; abortController = new AbortController(); + if (agentConfig.signal?.aborted) abortController.abort(); signals.forgetApiErrors(); spinner.message('Renewing the gateway token...'); const stale = agentConfig.gatewayAuth; // A refusal or failure here ends the run with its own message. agentConfig.gatewayAuth = await refreshGatewayAuth(); + if (agentConfig.signal?.aborted) + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; logToFile( `Gateway token renewed after a 401 (${Math.round( (Date.now() - stale.refreshAtMs) / 1000, @@ -1353,34 +1444,56 @@ export async function runAgent( // A fresh bearer was rejected. The auth screen is already up; hand the // decided failure to the caller, which owns the exit. if (authFailure) { - return { failure: authFailure }; + return { kind: 'decided_failure', failure: authFailure }; + } + if (agentConfig.signal?.aborted) { + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; } // A YARA hook detected a terminal violation and aborted the run. if (yaraViolationReason) { logToFile('Agent error: YARA_VIOLATION'); spinner.stop('Security check stopped the setup'); - return { error: AgentErrorType.YARA_VIOLATION }; + return { kind: 'failure', classification: AgentErrorType.YARA_VIOLATION }; } // If the middleware caught an [ABORT] and aborted the SDK query, surface // it as a structured error before checking other signals. if (abortReason) { spinner.stop('Wizard aborted'); - return { error: AgentErrorType.ABORT, message: abortReason }; + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: abortReason, + }; + } + if (agentConfig.signal?.aborted) { + spinner.stop('Wizard aborted'); + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; } // Check for error markers in the agent's output if (signals.has('MCP_MISSING')) { logToFile('Agent error: MCP_MISSING'); spinner.stop('Agent could not access PostHog MCP'); - return { error: AgentErrorType.MCP_MISSING }; + return { kind: 'failure', classification: AgentErrorType.MCP_MISSING }; } if (signals.has('RESOURCE_MISSING')) { logToFile('Agent error: RESOURCE_MISSING'); spinner.stop('Agent could not access setup resource'); - return { error: AgentErrorType.RESOURCE_MISSING }; + return { + kind: 'failure', + classification: AgentErrorType.RESOURCE_MISSING, + }; } // A clean success result already arrived. The Claude SDK can emit a second @@ -1393,6 +1506,11 @@ export async function runAgent( return completeWithSuccess(); } + if (terminalFailure) { + spinner.stop(errorMessage); + return terminalFailure; + } + // Check for API errors (rate limits, etc.) // Surface just the API error line(s), not the entire output const apiErrorMessage = signals.apiErrorMessage() ?? 'Unknown API error'; @@ -1400,16 +1518,29 @@ export async function runAgent( if (signals.hasApiErrorStatus(429)) { logToFile('Agent error: RATE_LIMIT'); spinner.stop('Rate limit exceeded'); - return { error: AgentErrorType.RATE_LIMIT, message: apiErrorMessage }; + return { + kind: 'failure', + classification: AgentErrorType.RATE_LIMIT, + message: apiErrorMessage, + }; } if (signals.hasApiError()) { logToFile('Agent error: API_ERROR'); spinner.stop('API error occurred'); - return { error: AgentErrorType.API_ERROR, message: apiErrorMessage }; + return { + kind: 'failure', + classification: AgentErrorType.API_ERROR, + message: apiErrorMessage, + }; } - return completeWithSuccess(); + spinner.stop(errorMessage); + return { + kind: 'failure', + classification: AgentErrorType.API_ERROR, + message: 'SDK stream ended without a success result', + }; } catch (error) { // Signal done to unblock the async generator signalDone(); @@ -1420,14 +1551,27 @@ export async function runAgent( if (yaraViolationReason) { logToFile('Agent error: YARA_VIOLATION'); spinner.stop('Security check stopped the setup'); - return { error: AgentErrorType.YARA_VIOLATION }; + return { kind: 'failure', classification: AgentErrorType.YARA_VIOLATION }; } // If the middleware caught an [ABORT] and triggered abortController.abort(), // the SDK will throw an AbortError — surface it as a clean abort result. if (abortReason) { spinner.stop('Wizard aborted'); - return { error: AgentErrorType.ABORT, message: abortReason }; + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: abortReason, + }; + } + + if (agentConfig.signal?.aborted) { + spinner.stop('Wizard aborted'); + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; } // If we already received a successful result, the error is from SDK cleanup @@ -1445,13 +1589,23 @@ export async function runAgent( if (signals.hasApiErrorStatus(429)) { logToFile('Agent error (caught): RATE_LIMIT'); spinner.stop('Rate limit exceeded'); - return { error: AgentErrorType.RATE_LIMIT, message: apiErrorMessage }; + return { + kind: 'failure', + classification: AgentErrorType.RATE_LIMIT, + message: apiErrorMessage, + error: error instanceof Error ? error : undefined, + }; } if (signals.hasApiError()) { logToFile('Agent error (caught): API_ERROR'); spinner.stop('API error occurred'); - return { error: AgentErrorType.API_ERROR, message: apiErrorMessage }; + return { + kind: 'failure', + classification: AgentErrorType.API_ERROR, + message: apiErrorMessage, + error: error instanceof Error ? error : undefined, + }; } // No API error found, re-throw the original exception @@ -1465,17 +1619,22 @@ export async function runAgent( debug('Full error:', error); throw error; } finally { + agentConfig.signal?.removeEventListener('abort', onExternalAbort); // Always capture run duration, even on abort/error, so we can alert on // long runs where the user gave up before completion. A 401 never reached // this block before (the process exited first), so it still does not count. if (!receivedSuccessResult && !authFailure) { const durationMs = Date.now() - startTime; - analytics.wizardCapture('agent aborted', { - duration_ms: durationMs, - duration_seconds: Math.round(durationMs / 1000), - model: agentConfig.model, - ...config?.analyticsProperties, - }); + try { + analytics.wizardCapture('agent aborted', { + duration_ms: durationMs, + duration_seconds: Math.round(durationMs / 1000), + model: agentConfig.model, + ...config?.analyticsProperties, + }); + } catch { + // Analytics must not replace the SDK outcome. + } } } } diff --git a/src/agent/aio-capture.ts b/src/agent/aio-capture.ts index be4182c3c..f9812f849 100644 --- a/src/agent/aio-capture.ts +++ b/src/agent/aio-capture.ts @@ -62,6 +62,7 @@ export type AioCapture = { captureFromAnthropicSDKMessage(msg: unknown): void; /** Called for every subscribe event on the pi harness. */ captureFromPiMessageEndEvent(event: unknown): void; + finishPiRun(isError: boolean): void; }; const noop = (_: unknown): void => undefined; @@ -69,6 +70,7 @@ const NOOP: AioCapture = { setInitialPrompt: noop, captureFromAnthropicSDKMessage: noop, captureFromPiMessageEndEvent: noop, + finishPiRun: noop, }; export function createAioCapture(args: { @@ -216,6 +218,7 @@ export function createAioCapture(args: { role?: string; model?: string; content?: unknown; + stopReason?: string; usage?: { input_tokens?: number; output_tokens?: number; @@ -243,7 +246,8 @@ export function createAioCapture(args: { cacheCreationTokens: e.message.usage?.cache_creation_input_tokens, latencyMs, generationId: e.message.id, - isError: false, + isError: + e.message.stopReason === 'error' || e.message.stopReason === 'aborted', }; }; @@ -328,10 +332,7 @@ export function createAioCapture(args: { }); lastAssistantContent = turn.outputChoices; } - // Terminal: pi emits `agent_end` when the session's prompt() resolves. - if (e?.type === 'agent_end' && !e.willRetry) { - sendTraceEnd({ isError: false }); - } + // The harness decides whether a final turn recovered before ending the trace. } catch (err) { logToFile( `[aio-capture] pi compose failed: ${ @@ -340,5 +341,8 @@ export function createAioCapture(args: { ); } }, + finishPiRun(isError) { + sendTraceEnd({ isError }); + }, }; } diff --git a/src/agent/runner/README.md b/src/agent/runner/README.md index 79a87bfad..7279abc53 100644 --- a/src/agent/runner/README.md +++ b/src/agent/runner/README.md @@ -40,7 +40,7 @@ for the coordinated change checklist. Five layers, each with its own job. Nothing crosses layers unless it has to. **The entry point** (`index.ts`) is the front door: -`runAgent(config, input, {onProgress?, interaction?}) → RunResult`. It takes +`runAgent(config, input, {onProgress?, interaction?, signal?}) → RunResult`. It takes resolved execution data and an invocation snapshot (`shared/types.ts`), reports through `onProgress` and asks through `interaction` (`../progress.ts`), and returns every ending as a result. It never renders, reads a session or exits. @@ -86,6 +86,53 @@ gateway. Each layer is replaceable. +## Ownership map + +```mermaid +%%{init: {"block": {"padding": 20}}}%% +block-beta + columns 11 + hostBand["Host: programs and UI"]:11 + runProgramAgent["runProgramAgent"]:3 space:1 wizardAbort["wizardAbort"]:3 space:4 + space:11 + runnerBand["Agent runner"]:11 + runAgent["runAgent"]:3 space:1 runResult["RunResult"]:3 space:4 + space:11 + sequenceBand["Orchestrator sequence"]:11 + runOrchestrator["runOrchestrator"]:3 space:1 sequenceResult["SequenceResult"]:3 space:4 + space:11 + drainQueue["drainQueue"]:3 space:5 runAbort["AbortController"]:3 + space:11 + harnessBand["Selected harness"]:11 + agentHarness["AgentHarness"]:3 space:1 agentResult["AgentResult"]:3 space:1 signal["TaskRunInputs.signal"]:3 + space:11 + sdkBand["External model SDK"]:11 + sdk["Selected SDK"]:3 space:8 + + runProgramAgent --> runAgent + runAgent --> runOrchestrator + runOrchestrator --> drainQueue + drainQueue --> agentHarness + agentHarness --> sdk + agentHarness --> agentResult + agentResult --> sequenceResult + sequenceResult --> runResult + runResult --> wizardAbort + drainQueue --> runAbort + runAbort --> signal + + classDef owner fill:#9ca3af1f,stroke:#9ca3af,stroke-width:1.5px + classDef changed fill:#3b82f626,stroke:#3b82f6,stroke-width:2px + class hostBand,runnerBand,sequenceBand,harnessBand,sdkBand owner + class runResult,agentResult,runAbort changed +``` + +Calls descend on the left, results return through the middle, and cancellation +moves down the right. Blue marks the result contracts and run-scoped abort. +On the first fatal task result, `drainQueue` stops scheduling, cancels +active work and pending asks, joins siblings, then preserves that failure for +the host to present. + ## Flow 1. The caller runs its gates, authenticates, fetches PostHog flags and resolves diff --git a/src/agent/runner/harness/anthropic/index.ts b/src/agent/runner/harness/anthropic/index.ts index 74a051420..8a75169bf 100644 --- a/src/agent/runner/harness/anthropic/index.ts +++ b/src/agent/runner/harness/anthropic/index.ts @@ -75,7 +75,7 @@ export const anthropicBackend: AgentHarness = { logToFile('[agent-runner] agent initialized'); return executeAgent( - agent, + { ...agent, signal: inputs.signal }, prompt, runOptions(input), spinner, @@ -151,7 +151,7 @@ export const anthropicBackend: AgentHarness = { ); return executeAgent( - { ...agent, model, allowedTools, disallowedTools }, + { ...agent, model, allowedTools, disallowedTools, signal: inputs.signal }, prompt, options, spinner, diff --git a/src/agent/runner/harness/pi/__tests__/cancellation.test.ts b/src/agent/runner/harness/pi/__tests__/cancellation.test.ts new file mode 100644 index 000000000..c4b0d8607 --- /dev/null +++ b/src/agent/runner/harness/pi/__tests__/cancellation.test.ts @@ -0,0 +1,42 @@ +import { bindPiCancellation } from '../cancellation'; + +it('observes session abort and keeps listener failures out of abort dispatch', async () => { + const controller = new AbortController(); + let release: (() => void) | undefined; + const abort = vi.fn( + () => + new Promise((resolve) => { + release = resolve; + }), + ); + const binding = bindPiCancellation(controller.signal, { abort }); + expect(() => controller.abort()).not.toThrow(); + await Promise.resolve(); + expect(abort).toHaveBeenCalledOnce(); + let settled = false; + const waiting = binding.settle().then(() => { + settled = true; + }); + await Promise.resolve(); + expect(settled).toBe(false); + release?.(); + await waiting; + expect(settled).toBe(true); +}); + +it('contains a synchronous abort throw and a diagnostic callback throw', async () => { + const controller = new AbortController(); + const binding = bindPiCancellation( + controller.signal, + { + abort: () => { + throw new Error('abort failed'); + }, + }, + () => { + throw new Error('log failed'); + }, + ); + expect(() => controller.abort()).not.toThrow(); + await expect(binding.settle()).resolves.toBeUndefined(); +}); diff --git a/src/agent/runner/harness/pi/__tests__/gateway.test.ts b/src/agent/runner/harness/pi/__tests__/gateway.test.ts index cfedba108..548e77bb6 100644 --- a/src/agent/runner/harness/pi/__tests__/gateway.test.ts +++ b/src/agent/runner/harness/pi/__tests__/gateway.test.ts @@ -126,6 +126,15 @@ describe('isGatewayAuthRejection', () => { }), ).toBe(false); }); + + it('lets structured non-401 status override quoted 401 prose', () => { + expect( + isGatewayAuthRejection({ + errorMessage: 'Earlier log: API error 401', + diagnostics: [{ error: { code: 403 } }], + }), + ).toBe(false); + }); }); describe('withGatewayRemint', () => { @@ -180,6 +189,76 @@ describe('withGatewayRemint', () => { return { wrapped, registry, refreshAuth, prompts }; } + it.each([ + [ + { + stopReason: 'error', + errorMessage: 'bad request', + diagnostics: [{ error: { code: 400 } }], + }, + 'WIZARD_API_ERROR', + ], + [ + { + stopReason: 'error', + errorMessage: 'rate limited', + diagnostics: [{ error: { code: 429 } }], + }, + 'WIZARD_RATE_LIMIT', + ], + [ + { + stopReason: 'error', + errorMessage: 'service unavailable', + diagnostics: [{ error: { code: 503 } }], + }, + 'WIZARD_API_ERROR', + ], + [{ stopReason: 'aborted', errorMessage: 'cancelled' }, 'WIZARD_ABORT'], + ])('exposes a terminal provider turn %j', async (turn, classification) => { + const { wrapped } = harness(gatewayAuth('phe_fresh', Date.now() + HOUR), [ + turn, + ]); + await wrapped.prompt('do it'); + expect(wrapped.terminalFailure()).toMatchObject({ + classification, + message: turn.errorMessage, + }); + }); + + it('uses the final turn after remint and recovery', async () => { + const { wrapped } = harness(gatewayAuth('phe_old', Date.now() - 1), [ + rejected, + fine, + ]); + await wrapped.prompt('do it'); + expect(wrapped.terminalFailure()).toBeUndefined(); + }); + + it('uses rate-limit prose when diagnostics have no HTTP status', async () => { + const { wrapped } = harness(gatewayAuth('phe_fresh', Date.now() + HOUR), [ + { + stopReason: 'error', + errorMessage: '429 rate limit', + diagnostics: [{ error: { code: 'UNKNOWN' } }], + }, + ]); + await wrapped.prompt('do it'); + expect(wrapped.terminalFailure()?.classification).toBe('WIZARD_RATE_LIMIT'); + }); + + it('recognizes a name-only authentication error as an auth failure', async () => { + const { wrapped } = harness(gatewayAuth('phe_fresh', Date.now() + HOUR), [ + { + stopReason: 'error', + errorMessage: 'rejected', + diagnostics: [{ error: { name: 'AuthenticationError' } }], + }, + ]); + await wrapped.prompt('do it'); + expect(wrapped.terminalFailure()?.status).toBe(401); + }); + it('re-mints once and continues when a turn ends on a 401 from an aged bearer', async () => { const { wrapped, registry, refreshAuth, prompts } = harness( gatewayAuth('phe_old', Date.now() - 1), diff --git a/src/agent/runner/harness/pi/__tests__/mcp-env.test.ts b/src/agent/runner/harness/pi/__tests__/mcp-env.test.ts new file mode 100644 index 000000000..04c9a9c3d --- /dev/null +++ b/src/agent/runner/harness/pi/__tests__/mcp-env.test.ts @@ -0,0 +1,43 @@ +const state = vi.hoisted(() => ({ fail: false })); +vi.mock('jiti', () => ({ + createJiti: () => ({ + import: () => { + if (state.fail) return Promise.reject(new Error('adapter import failed')); + return Promise.resolve({ createMcpAdapter: () => () => undefined }); + }, + }), +})); +vi.mock('@utils/debug'); + +import { setupPostHogMcp } from '../mcp'; + +const envName = 'POSTHOG_MCP_TOKEN'; +const opts = (accessToken: string) => ({ + mcpUrl: 'https://mcp.test', + accessToken, + userAgent: 'test', +}); + +afterEach(() => { + delete process.env[envName]; + state.fail = false; +}); + +it('restores the prior token if adapter setup fails', async () => { + process.env[envName] = 'prior'; + state.fail = true; + await expect(setupPostHogMcp(opts('run-token'))).rejects.toThrow( + 'adapter import failed', + ); + expect(process.env[envName]).toBe('prior'); +}); + +it('keeps a shared token until the final owner releases it', async () => { + process.env[envName] = 'prior'; + const first = await setupPostHogMcp(opts('run-token')); + const second = await setupPostHogMcp(opts('run-token')); + first.cleanup(); + expect(process.env[envName]).toBe('run-token'); + second.cleanup(); + expect(process.env[envName]).toBe('prior'); +}); diff --git a/src/agent/runner/harness/pi/cancellation.ts b/src/agent/runner/harness/pi/cancellation.ts new file mode 100644 index 000000000..2f480f8b6 --- /dev/null +++ b/src/agent/runner/harness/pi/cancellation.ts @@ -0,0 +1,28 @@ +/** Bind a host signal to Pi's abort-and-wait-for-idle operation. */ +export function bindPiCancellation( + signal: AbortSignal | undefined, + session: { abort(): Promise }, + onAbortError?: (error: unknown) => void, +): { settle(): Promise } { + let abortPromise: Promise | undefined; + const onAbort = () => { + abortPromise ??= Promise.resolve() + .then(() => session.abort()) + .catch((error: unknown) => { + try { + onAbortError?.(error); + } catch { + /* Abort diagnostics are best effort. */ + } + }); + }; + signal?.addEventListener('abort', onAbort, { once: true }); + if (signal?.aborted) onAbort(); + + return { + async settle() { + signal?.removeEventListener('abort', onAbort); + await abortPromise; + }, + }; +} diff --git a/src/agent/runner/harness/pi/completion.ts b/src/agent/runner/harness/pi/completion.ts index 8219e5c46..facede847 100644 --- a/src/agent/runner/harness/pi/completion.ts +++ b/src/agent/runner/harness/pi/completion.ts @@ -21,7 +21,9 @@ export function completionFailure(args: { * each entry point decide the type first and hand it to the event, and keeps * the one rule in one place. */ -export function runErrorType(message: string): AgentErrorType { +export function runErrorType( + message: string, +): AgentErrorType.RATE_LIMIT | AgentErrorType.API_ERROR { const lower = message.toLowerCase(); if (lower.includes('rate limit') || lower.includes('429')) { return AgentErrorType.RATE_LIMIT; diff --git a/src/agent/runner/harness/pi/gateway.ts b/src/agent/runner/harness/pi/gateway.ts index 9c6a6d6c7..bb5b7e20a 100644 --- a/src/agent/runner/harness/pi/gateway.ts +++ b/src/agent/runner/harness/pi/gateway.ts @@ -15,6 +15,7 @@ import { modelCapabilities, type ThinkingLevel, } from '../../switchboard/models'; +import { AgentErrorType } from '@agent/signals'; /** Provider registered on the in-memory registry for this run. */ export const GATEWAY_PROVIDER = 'posthog-gateway'; @@ -146,6 +147,49 @@ export interface GatewayTurnError { diagnostics?: { error?: { name?: string; code?: string | number } }[]; } +export type GatewayTerminalFailure = { + classification: + | AgentErrorType.API_ERROR + | AgentErrorType.RATE_LIMIT + | AgentErrorType.ABORT; + message: string; + status?: number; +}; + +export function gatewayTerminalFailure( + turn: ({ stopReason?: string } & GatewayTurnError) | undefined, +): GatewayTerminalFailure | undefined { + if (!turn || (turn.stopReason !== 'error' && turn.stopReason !== 'aborted')) + return undefined; + const message = + turn.errorMessage || + (turn.stopReason === 'aborted' + ? 'Agent turn aborted' + : 'Gateway request failed'); + if (turn.stopReason === 'aborted') { + return { classification: AgentErrorType.ABORT, message }; + } + const codes = + turn.diagnostics + ?.map((d) => d.error?.code) + .filter((code) => code !== undefined) ?? []; + const structuredStatus = codes + .map(Number) + .find((code) => Number.isInteger(code) && code >= 400 && code < 600); + const rateLimited = + structuredStatus !== undefined + ? structuredStatus === 429 + : /\b429\b|rate limit/i.test(message); + return { + classification: rateLimited + ? AgentErrorType.RATE_LIMIT + : AgentErrorType.API_ERROR, + message, + status: + structuredStatus ?? (isGatewayAuthRejection(turn) ? 401 : undefined), + }; +} + /** * Whether a turn's error is the gateway rejecting the bearer. pi attaches the * SDK's own error to `diagnostics`, so its code decides when one is present. @@ -159,6 +203,10 @@ export function isGatewayAuthRejection( typeof turn === 'string' ? { errorMessage: turn, diagnostics: undefined } : turn ?? {}; + const structuredStatus = (diagnostics ?? []) + .map((diagnostic) => Number(diagnostic.error?.code)) + .find((code) => Number.isInteger(code) && code >= 400 && code < 600); + if (structuredStatus !== undefined) return structuredStatus === 401; for (const diagnostic of diagnostics ?? []) { const code = diagnostic.error?.code; if (code === 401 || code === '401') return true; @@ -169,6 +217,7 @@ export function isGatewayAuthRejection( } export interface GatewayRemintOptions { + signal?: AbortSignal; session: { prompt(text: string): Promise }; registry: { registerProvider(providerName: string, config: never): void }; auth: GatewayAuth; @@ -190,30 +239,40 @@ export function withGatewayRemint(opts: GatewayRemintOptions): { prompt(text: string): Promise; /** Feed every assistant `message_end`; the last turn decides. */ noteAssistantTurn(message: unknown): void; + terminalFailure(): GatewayTerminalFailure | undefined; } { let auth = opts.auth; let rejected = false; let reminted = false; + let lastTurn: ({ stopReason?: string } & GatewayTurnError) | undefined; return { noteAssistantTurn(message) { - const turn = message as + lastTurn = message as | ({ stopReason?: string } & GatewayTurnError) | undefined; - rejected = turn?.stopReason === 'error' && isGatewayAuthRejection(turn); + rejected = + lastTurn?.stopReason === 'error' && isGatewayAuthRejection(lastTurn); }, + terminalFailure: () => gatewayTerminalFailure(lastTurn), async prompt(text) { + if (opts.signal?.aborted) return; rejected = false; + lastTurn = undefined; await opts.session.prompt(text); - if (!rejected || reminted || !isPastRefresh(auth)) return; + if (opts.signal?.aborted || !rejected || reminted || !isPastRefresh(auth)) + return; reminted = true; auth = await opts.refreshAuth(); + if (opts.signal?.aborted) return; opts.registry.registerProvider( GATEWAY_PROVIDER, buildGatewayProvider(opts.providerInputs(auth)).provider as never, ); opts.onRemint?.(); rejected = false; + lastTurn = undefined; const next = opts.continueText; + if (opts.signal?.aborted) return; await opts.session.prompt(typeof next === 'function' ? next() : next); }, }; diff --git a/src/agent/runner/harness/pi/index.ts b/src/agent/runner/harness/pi/index.ts index ec903a4e9..90ea3ac54 100644 --- a/src/agent/runner/harness/pi/index.ts +++ b/src/agent/runner/harness/pi/index.ts @@ -44,6 +44,8 @@ import type { ProgressEmitter } from '@agent/progress'; import { createEmitLog } from '@agent/runner/shared/progress-collector'; import type { TaskStore } from './tasks'; import { completionFailure, runErrorType } from './completion'; +import { bindPiCancellation } from './cancellation'; +import { classifyRunFailure, ErrorCodes } from '@shared/errors'; /** Injects the MCP server `instructions` pi-mcp-adapter drops (project env, skill steer, tool domains) into the system prompt, falling back to a bootstrap-derived project block when the warm-connect captured none. */ function piMcpContext( @@ -205,6 +207,13 @@ export const piBackend: AgentHarness = { name: Harness.pi, async run(inputs: BackendRunInputs): Promise { + if (inputs.signal?.aborted) { + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; + } const { config: runConfig, input, boot, emit, prompt, spinner } = inputs; const config = runConfig.run; const modelId = inputs.model; @@ -255,6 +264,9 @@ export const piBackend: AgentHarness = { model: modelId, }); + let mcpCleanup: (() => void) | undefined; + let aioFailed = true; + let cancellation: ReturnType | undefined; try { const { createAgentSession, @@ -298,7 +310,8 @@ export const piBackend: AgentHarness = { const model = registry.find(GATEWAY_PROVIDER, modelId); if (!model) { return { - error: AgentErrorType.API_ERROR, + kind: 'failure', + classification: AgentErrorType.API_ERROR, message: 'pi: gateway model could not be resolved', }; } @@ -338,7 +351,6 @@ export const piBackend: AgentHarness = { const extensionFactories = [security.factory] as Array< (pi: unknown) => void >; - let mcpCleanup: (() => void) | undefined; let mcpInstructions: string | undefined; // Whether the agent really got the tool. The commandments below claim // `posthog_exec` exists when this is true, so it must track the setup and @@ -363,6 +375,12 @@ export const piBackend: AgentHarness = { mcpInstructions = await instructionsPromise; posthogMcp = true; } catch (err) { + try { + mcpCleanup?.(); + } catch { + /* Setup cleanup is best effort. */ + } + mcpCleanup = undefined; logToFile(`[pi] PostHog MCP setup skipped: ${String(err)}`); analytics.wizardCapture('mcp setup failed', { harness: 'pi', @@ -485,12 +503,20 @@ export const piBackend: AgentHarness = { // rebindCurrentSession. createAgentSession builds the session but does not // emit session_start on its own, and the MCP adapter connects on that // event; without this its tools report "MCP not initialized". + cancellation = bindPiCancellation( + inputs.signal, + agentSession, + (error) => { + logToFile(`[pi] abort failed: ${String(error)}`); + }, + ); await agentSession.bindExtensions({}); // A turn that ends on a 401 from an aged bearer re-mints once and // continues; pi resolves the provider's apiKey per request, so // re-registering is enough. const turns = withGatewayRemint({ + signal: inputs.signal, session: agentSession, registry, auth, @@ -573,10 +599,18 @@ export const piBackend: AgentHarness = { // turns that follow. capture.setInitialPrompt(prompt); + let terminal = turns.terminalFailure(); try { + if (inputs.signal?.aborted) + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; // Non-streaming: resolves when the agent run completes. Throws if no // model/api key, or on a transport error. await turns.prompt(prompt); + terminal = turns.terminalFailure(); // Completion guard: pi's prompt() resolves the moment the model returns // a turn with no tool call (e.g. a lone [STATUS] line), even mid-plan. @@ -585,6 +619,8 @@ export const piBackend: AgentHarness = { while ( continueNudges < MAX_CONTINUE_NUDGES && !security.state.criticalViolation && + !inputs.signal?.aborted && + !terminal && hasOpenTasks(wizardTaskTools.store) ) { continueNudges += 1; @@ -592,10 +628,15 @@ export const piBackend: AgentHarness = { `[pi] completion guard: tasks still open, nudge ${continueNudges}/${MAX_CONTINUE_NUDGES}`, ); await turns.prompt(CONTINUE_INSTRUCTION); + terminal = turns.terminalFailure(); } // Best-effort remark ask — a failed turn never fails a successful run. - if (!security.state.criticalViolation) { + if ( + !security.state.criticalViolation && + !terminal && + !inputs.signal?.aborted + ) { try { await agentSession.prompt(REMARK_INSTRUCTION); } catch (err) { @@ -603,8 +644,47 @@ export const piBackend: AgentHarness = { } } } finally { - unsubscribe(); - mcpCleanup?.(); + try { + unsubscribe(); + } catch { + /* Keep the terminal result. */ + } + } + + if (inputs.signal?.aborted) { + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; + } + + if (terminal && !security.state.criticalViolation) { + spinner.stop( + config.errorMessage ?? `${config.integrationLabel} failed`, + ); + captureAborted(terminal.classification); + if (terminal.status === 401) { + return { + kind: 'decided_failure', + failure: { + code: ErrorCodes.AuthInvalidOrExpired, + message: 'Authentication failed (401)', + detail: { providerMessage: terminal.message }, + }, + }; + } + return terminal.classification === AgentErrorType.ABORT + ? { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: terminal.message, + } + : { + kind: 'failure', + classification: terminal.classification, + message: terminal.message, + }; } // A latched post-scan violation terminates the run as a YARA violation, @@ -615,7 +695,10 @@ export const piBackend: AgentHarness = { `[pi] terminated: YARA violation (blocked ${security.state.blockedCount} call(s))`, ); captureAborted(AgentErrorType.YARA_VIOLATION); - return { error: AgentErrorType.YARA_VIOLATION }; + return { + kind: 'failure', + classification: AgentErrorType.YARA_VIOLATION, + }; } // pi ends a run on any tool-call-less turn, so guard against a hollow @@ -631,14 +714,14 @@ export const piBackend: AgentHarness = { assistant_turns: assistantTurns, }); captureAborted(failure); - return { error: failure }; + return { kind: 'failure', classification: failure }; } if (failure === AgentErrorType.INCOMPLETE_TASKS) { spinner.stop('Agent stopped before finishing'); logToFile('[pi] incomplete: tasks left open'); analytics.wizardCapture('agent incomplete tasks', { open_tasks: true }); captureAborted(failure); - return { error: failure }; + return { kind: 'failure', classification: failure }; } const remark = signals.remark(); @@ -678,15 +761,48 @@ export const piBackend: AgentHarness = { cache_read_input_tokens: stats.tokens.cacheRead, }); spinner.stop(config.successMessage ?? 'PostHog integration complete'); - return {}; + aioFailed = false; + return { kind: 'success' }; } catch (err) { + if (inputs.signal?.aborted) { + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; + } const message = err instanceof Error ? err.message : String(err); logToFile(`[pi] run error: ${message}`); spinner.stop(config.errorMessage ?? `${config.integrationLabel} failed`); log.error(`pi backend error: ${message}`); - const error = runErrorType(message); - captureAborted(error); - return { error, message }; + const coded = classifyRunFailure(err); + if (coded.coded && err instanceof Error) { + captureAborted(AgentErrorType.API_ERROR); + return { + kind: 'decided_failure', + failure: { code: coded.code, message: coded.message, error: err }, + }; + } + const classification = runErrorType(message); + captureAborted(classification); + return { + kind: 'failure', + classification, + message, + error: err instanceof Error ? err : undefined, + }; + } finally { + await cancellation?.settle(); + try { + mcpCleanup?.(); + } catch { + /* Keep the terminal result. */ + } + try { + capture.finishPiRun(aioFailed); + } catch { + /* Telemetry is best effort. */ + } } }, diff --git a/src/agent/runner/harness/pi/mcp.ts b/src/agent/runner/harness/pi/mcp.ts index 8f553a983..78ac2d684 100644 --- a/src/agent/runner/harness/pi/mcp.ts +++ b/src/agent/runner/harness/pi/mcp.ts @@ -11,11 +11,37 @@ import { VERSION } from '@shared/version'; import { logToFile } from '@utils/debug'; const MCP_TOKEN_ENV = 'POSTHOG_MCP_TOKEN'; +let mcpTokenOwners = 0; +let activeMcpToken: string | undefined; +let previousMcpToken: string | undefined; + +function acquireMcpToken(token: string): () => void { + if (mcpTokenOwners > 0 && activeMcpToken !== token) { + throw new Error('PostHog MCP token is already owned by another active run'); + } + if (mcpTokenOwners === 0) { + previousMcpToken = process.env[MCP_TOKEN_ENV]; + activeMcpToken = token; + process.env[MCP_TOKEN_ENV] = token; + } + mcpTokenOwners += 1; + let released = false; + return () => { + if (released) return; + released = true; + mcpTokenOwners -= 1; + if (mcpTokenOwners > 0) return; + if (previousMcpToken === undefined) delete process.env[MCP_TOKEN_ENV]; + else process.env[MCP_TOKEN_ENV] = previousMcpToken; + activeMcpToken = undefined; + previousMcpToken = undefined; + }; +} export interface PostHogMcpSetup { /** pi ExtensionFactory to add to the resource loader's `extensionFactories`. */ extensionFactory: (pi: unknown) => void; - /** Drop the token env var. Call after the run. */ + /** Release this setup's token ownership. Call after the run. */ cleanup: () => void; } @@ -55,37 +81,39 @@ export async function setupPostHogMcp(opts: { const { mcpUrl, accessToken, userAgent } = opts; // By env NAME: the token stays in this process, off disk, and never reaches pi's env-scrubbed tool subprocesses. - process.env[MCP_TOKEN_ENV] = accessToken; + const cleanup = acquireMcpToken(accessToken); - // The adapter ships raw TypeScript; loading through jiti is its documented requirement. - const jiti = createJiti(import.meta.url); - const mod = await jiti.import<{ - createMcpAdapter: (options: unknown) => PostHogMcpSetup['extensionFactory']; - }>('pi-mcp-adapter'); - const extensionFactory = mod.createMcpAdapter({ - config: { - mcpServers: { - posthog: { - url: mcpUrl, - auth: 'bearer', - bearerTokenEnv: MCP_TOKEN_ENV, - headers: { 'User-Agent': userAgent }, - // Connect at extension load — direct tools register without a session_start. - lifecycle: 'eager', - // Register only `exec`: `directTools: true` also mints a `posthog_get_` tool per MCP resource, whose sentence-length names overflow Anthropic's 128-char tool-name limit and 400 the whole request. - directTools: ['exec'], - exposeResources: false, + try { + // The adapter ships raw TypeScript; loading through jiti is its documented requirement. + const jiti = createJiti(import.meta.url); + const mod = await jiti.import<{ + createMcpAdapter: ( + options: unknown, + ) => PostHogMcpSetup['extensionFactory']; + }>('pi-mcp-adapter'); + const extensionFactory = mod.createMcpAdapter({ + config: { + mcpServers: { + posthog: { + url: mcpUrl, + auth: 'bearer', + bearerTokenEnv: MCP_TOKEN_ENV, + headers: { 'User-Agent': userAgent }, + // Connect at extension load — direct tools register without a session_start. + lifecycle: 'eager', + // Register only `exec`: `directTools: true` also mints a `posthog_get_` tool per MCP resource, whose sentence-length names overflow Anthropic's 128-char tool-name limit and 400 the whole request. + directTools: ['exec'], + exposeResources: false, + }, }, + // Disable the proxy `mcp` tool (its search indirection pollutes context); the adapter re-enables it only if no direct tools resolve. + settings: { disableProxyTool: true, toolPrefix: 'posthog' }, }, - // Disable the proxy `mcp` tool (its search indirection pollutes context); the adapter re-enables it only if no direct tools resolve. - settings: { disableProxyTool: true, toolPrefix: 'posthog' }, - }, - }); - logToFile(`[pi-mcp] adapter loaded; posthog MCP at ${mcpUrl}`); - - const cleanup = (): void => { - delete process.env[MCP_TOKEN_ENV]; - }; - - return { extensionFactory, cleanup }; + }); + logToFile(`[pi-mcp] adapter loaded; posthog MCP at ${mcpUrl}`); + return { extensionFactory, cleanup }; + } catch (error) { + cleanup(); + throw error; + } } diff --git a/src/agent/runner/harness/pi/subagent.ts b/src/agent/runner/harness/pi/subagent.ts index 1f5e7f7d6..1238ca262 100644 --- a/src/agent/runner/harness/pi/subagent.ts +++ b/src/agent/runner/harness/pi/subagent.ts @@ -17,6 +17,7 @@ import { Type } from 'typebox'; import { defineTool } from '@earendil-works/pi-coding-agent'; import type { ToolDefinition } from '@earendil-works/pi-coding-agent'; import { logToFile } from '@utils/debug'; +import { gatewayTerminalFailure } from './gateway'; /** * Read-only built-ins a subagent may use. bash is supplied separately as the @@ -115,8 +116,19 @@ export function createDispatchAgentTool(ctx: SubagentContext): ToolDefinition { }); let result = ''; + let finalTurn: + | { + stopReason?: string; + errorMessage?: string; + diagnostics?: { + error?: { name?: string; code?: string | number }; + }[]; + } + | undefined; const unsub = child.subscribe((e) => { if (e.type === 'message_end') { + if ((e.message as { role?: string })?.role === 'assistant') + finalTurn = e.message as typeof finalTurn; const t = extractText(e.message).trim(); if (t) result = t; } @@ -124,6 +136,8 @@ export function createDispatchAgentTool(ctx: SubagentContext): ToolDefinition { logToFile(`[pi] subagent dispatch: ${args.description}`); try { await child.prompt(args.prompt); + const failure = gatewayTerminalFailure(finalTurn); + if (failure) throw new Error(`Subagent failed: ${failure.message}`); } finally { unsub(); } diff --git a/src/agent/runner/harness/pi/task.ts b/src/agent/runner/harness/pi/task.ts index 76db7a43a..290950f40 100644 --- a/src/agent/runner/harness/pi/task.ts +++ b/src/agent/runner/harness/pi/task.ts @@ -42,6 +42,8 @@ import { withGatewayRemint, } from './gateway'; import { runErrorType } from './completion'; +import { bindPiCancellation } from './cancellation'; +import { classifyRunFailure, ErrorCodes } from '@shared/errors'; import { assembleCommandments } from '../../switchboard/commandments'; import { applyOutroMarkers, @@ -164,6 +166,13 @@ function isSettled(ctx: OrchestratorToolsContext): boolean { } export async function runPiTask(inputs: TaskRunInputs): Promise { + if (inputs.signal?.aborted) { + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; + } const { config, input, @@ -220,6 +229,9 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { ...analyticsProperties, }); + let mcpCleanup: (() => void) | undefined; + let aioFailed = true; + let cancellation: ReturnType | undefined; try { const sdk = await import('@earendil-works/pi-coding-agent'); const { @@ -262,7 +274,8 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { const model = registry.find(GATEWAY_PROVIDER, modelId); if (!model) { return { - error: AgentErrorType.API_ERROR, + kind: 'failure', + classification: AgentErrorType.API_ERROR, message: 'pi: gateway model could not be resolved', }; } @@ -290,7 +303,6 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { const extensionFactories = [security.factory] as Array< (pi: unknown) => void >; - let mcpCleanup: (() => void) | undefined; let posthogMcp = false; if (allowsPostHogMcp(allowedTools)) { try { @@ -304,6 +316,12 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { mcpCleanup = mcp.cleanup; posthogMcp = true; } catch (err) { + try { + mcpCleanup?.(); + } catch { + /* Setup cleanup is best effort. */ + } + mcpCleanup = undefined; // Silent here reads as a task failure minutes later: a task that asked // for this tool can only skip or fail without it. logToFile(`[pi-task] PostHog MCP setup skipped: ${String(err)}`); @@ -394,11 +412,15 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { noTools: 'builtin', customTools, }); + cancellation = bindPiCancellation(inputs.signal, agentSession, (error) => { + logToFile(`[pi-task] abort failed: ${String(error)}`); + }); await agentSession.bindExtensions({}); // A turn that ends on a 401 from an aged bearer re-mints once and // continues with the nudge the task would get anyway. const turns = withGatewayRemint({ + signal: inputs.signal, session: agentSession, registry, auth, @@ -472,8 +494,16 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { // before it reaches this call site). capture.setInitialPrompt(taskPrompt); + let terminal = turns.terminalFailure(); try { + if (inputs.signal?.aborted) + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; await turns.prompt(taskPrompt); + terminal = turns.terminalFailure(); // pi's prompt() resolves the moment a turn carries no tool call — which // an agent mid-plan does emit. While the work has not reached its @@ -482,6 +512,8 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { while ( nudges < MAX_TASK_NUDGES && !security.state.criticalViolation && + !inputs.signal?.aborted && + !terminal && !isSettled(orchestrator) ) { nudges += 1; @@ -491,9 +523,15 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { await turns.prompt( orchestrator.currentTaskId ? TASK_NUDGE : SEED_NUDGE, ); + terminal = turns.terminalFailure(); } - if (requestRemark && !security.state.criticalViolation) { + if ( + requestRemark && + !security.state.criticalViolation && + !terminal && + !inputs.signal?.aborted + ) { try { await agentSession.prompt(REMARK_INSTRUCTION); } catch (err) { @@ -501,8 +539,46 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { } } } finally { - unsubscribe(); - mcpCleanup?.(); + try { + unsubscribe(); + } catch { + /* Keep the terminal result. */ + } + } + + if (inputs.signal?.aborted) { + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; + } + + if (terminal && !security.state.criticalViolation) { + if (errorMessage || spinnerMessage) + spinner.stop(errorMessage ?? 'Task failed'); + captureAborted(terminal.classification); + if (terminal.status === 401) { + return { + kind: 'decided_failure', + failure: { + code: ErrorCodes.AuthInvalidOrExpired, + message: 'Authentication failed (401)', + detail: { providerMessage: terminal.message }, + }, + }; + } + return terminal.classification === AgentErrorType.ABORT + ? { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: terminal.message, + } + : { + kind: 'failure', + classification: terminal.classification, + message: terminal.message, + }; } if (security.state.criticalViolation) { @@ -511,7 +587,7 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { `[pi-task] terminated: YARA violation (blocked ${security.state.blockedCount} call(s))`, ); captureAborted(AgentErrorType.YARA_VIOLATION); - return { error: AgentErrorType.YARA_VIOLATION }; + return { kind: 'failure', classification: AgentErrorType.YARA_VIOLATION }; } const remark = signals.remark(); @@ -542,15 +618,48 @@ export async function runPiTask(inputs: TaskRunInputs): Promise { `[pi-task] usage task=${taskType} model=${modelId} effort=${caps.thinkingLevel} dur=${durations.duration_seconds}s turns=${assistantTurns} in=${stats.tokens.input} out=${stats.tokens.output} cacheR=${stats.tokens.cacheRead} cacheW=${stats.tokens.cacheWrite}`, ); if (successMessage) spinner.stop(successMessage); - return {}; + aioFailed = false; + return { kind: 'success' }; } catch (err) { + if (inputs.signal?.aborted) { + return { + kind: 'abort', + classification: AgentErrorType.ABORT, + message: 'Agent run cancelled', + }; + } const message = err instanceof Error ? err.message : String(err); logToFile(`[pi-task] run error: ${message}`); if (errorMessage || spinnerMessage) { spinner.stop(errorMessage ?? 'Task failed'); } - const error = runErrorType(message); - captureAborted(error); - return { error, message }; + const coded = classifyRunFailure(err); + if (coded.coded && err instanceof Error) { + captureAborted(AgentErrorType.API_ERROR); + return { + kind: 'decided_failure', + failure: { code: coded.code, message: coded.message, error: err }, + }; + } + const classification = runErrorType(message); + captureAborted(classification); + return { + kind: 'failure', + classification, + message, + error: err instanceof Error ? err : undefined, + }; + } finally { + await cancellation?.settle(); + try { + mcpCleanup?.(); + } catch { + /* Keep the terminal result. */ + } + try { + capture.finishPiRun(aioFailed); + } catch { + /* Telemetry is best effort. */ + } } } diff --git a/src/agent/runner/harness/types.ts b/src/agent/runner/harness/types.ts index 80d4786c8..35da58793 100644 --- a/src/agent/runner/harness/types.ts +++ b/src/agent/runner/harness/types.ts @@ -15,8 +15,7 @@ * rather than silently downgrading. * * A harness reports through `emit` and never reaches for a UI. It returns an - * error classification, or a decided `failure` the sequence must return as - * the run's result. + * tagged success, abort, or failure data for the sequence to decide. */ import type { AdditionalFeature } from '@shared/constants'; @@ -52,6 +51,7 @@ export interface BackendRunInputs { input: RunInput; boot: BootstrapResult; emit: ProgressEmitter; + signal?: AbortSignal; /** The fully assembled prompt. */ prompt: string; /** Installed framework-skill path, when the program installs one. */ @@ -69,15 +69,24 @@ export interface BackendRunInputs { } /** - * What a runner reports back: an error classification, or nothing on success. - * `failure` is a fully decided abort the harness already reported to the host - * (the 401 auth screen); the sequence returns it as the run's result. + * A harness reports one terminal outcome. A decided failure already has the + * caller-visible code and message; the caller alone presents it. */ -export type AgentResult = { - error?: AgentErrorType; - message?: string; - failure?: AgentFailure; -}; +export type AgentResult = + | { kind: 'success' } + | { + kind: 'abort'; + classification: AgentErrorType.ABORT; + message?: string; + error?: Error; + } + | { + kind: 'failure'; + classification: Exclude; + message?: string; + error?: Error; + } + | { kind: 'decided_failure'; failure: AgentFailure }; /** * One orchestrator-mode unit of work — the seed plan, or one drained task. @@ -91,6 +100,7 @@ export interface TaskRunInputs { input: RunInput; boot: BootstrapResult; emit: ProgressEmitter; + signal?: AbortSignal; /** The fully assembled per-task or seed prompt. */ prompt: string; spinner: SpinnerHandle; diff --git a/src/agent/runner/index.ts b/src/agent/runner/index.ts index 9f583e16c..f6b83d31c 100644 --- a/src/agent/runner/index.ts +++ b/src/agent/runner/index.ts @@ -25,7 +25,7 @@ import { Sequence } from '@shared/constants'; import { RunOutcome } from './shared/types'; export { RunOutcome } from './shared/types'; -import { classifyRunFailure } from '@shared/errors'; +import { classifyRunFailure, ErrorCodes } from '@shared/errors'; import { logToFile } from '@utils/debug'; import type { RunAgentOptions, @@ -76,54 +76,152 @@ export async function runAgent( input: RunInput, options: RunAgentOptions = {}, ): Promise { - const collector = createProgressCollector(options.onProgress); - const { emit } = collector; - const log = (message: string) => - emit({ kind: 'log', level: 'info', message }); - - // Flush the warlock scan report once, at this single seam, on every - // termination path and for every harness (linear, orchestrator, or future). - // flushScanReport is idempotent (it zeroes scan state), so a caller that also - // flushes from its own cleanup path sees a harmless no-op. No harness has to - // know reporting exists. + let collector: ReturnType | undefined; + const snapshot = (): RunResult['snapshot'] => { + try { + if (collector) return collector.snapshot(); + } catch { + // A partial snapshot must not replace the run's primary failure. + } + return { + tasks: [], + statusMessages: [], + usage: { + inputTokens: 0, + outputTokens: 0, + cacheReadTokens: 0, + cacheCreationTokens: 0, + }, + }; + }; + let result: RunResult; try { + collector = createProgressCollector(options.onProgress); + const { emit } = collector; + const log = (message: string) => + emit({ kind: 'log', level: 'info', message }); + if (options.signal?.aborted) { + try { + flushScanReport({ yaraReport: input.flags.yaraReport }); + } catch { + // Scan reporting is best effort. + } + return { + outcome: RunOutcome.Aborted, + skillId: input.skillId, + failure: { + code: ErrorCodes.AgentAbort, + message: 'Agent run cancelled', + }, + snapshot: snapshot(), + }; + } const boot = await prepareRun(config, input); if (config.binding.sequence === Sequence.orchestrator) { log('Task-queue orchestrator enabled.'); } - logToFile( - `[agent-runner] run program=${config.programId} sequence=${config.binding.sequence}` + - ` harness=${config.binding.harness} composed=${config.composed}`, - ); - const result = await getSequence(config.binding.sequence).run({ + try { + logToFile( + `[agent-runner] run program=${config.programId} sequence=${config.binding.sequence}` + + ` harness=${config.binding.harness} composed=${config.composed}`, + ); + } catch { + // Logging is best effort. + } + const sequenceResult = await getSequence(config.binding.sequence).run({ config, input, boot, emit, interaction: options.interaction, + signal: options.signal, }); - return { - ...result, + result = { + ...(options.signal?.aborted && + sequenceResult.outcome === RunOutcome.Success + ? { + outcome: RunOutcome.Aborted as const, + failure: { + code: ErrorCodes.AgentAbort, + message: 'Agent run cancelled', + }, + } + : sequenceResult), skillId: input.skillId, - snapshot: collector.snapshot(), + snapshot: snapshot(), }; } catch (error) { - // Not a decision the agent made. Hand it back whole rather than throw, so - // every ending of a run is a result the caller reads the same way. - const failure = classifyRunFailure(error); - logToFile('[agent-runner] run crashed:', error); - return { - outcome: RunOutcome.Crashed, - skillId: input.skillId, - failure: { - code: failure.code, - message: failure.message, - error: error instanceof Error ? error : new Error(String(error)), - }, - snapshot: collector.snapshot(), - }; - } finally { + const original = + error instanceof Error ? error : new Error(safeErrorMessage(error)); + let failure: ReturnType; + try { + failure = classifyRunFailure(original); + } catch { + failure = { + code: ErrorCodes.InternalUnhandled, + message: 'Unexpected agent error', + coded: false, + }; + } + try { + logToFile('[agent-runner] run failed:', original); + } catch { + // Logging is best effort. + } + let abortError = false; + try { + abortError = original.name === 'AbortError'; + } catch { + /* Hostile Error getter. */ + } + if (failure.coded) { + result = { + outcome: RunOutcome.Failed, + skillId: input?.skillId, + failure: { + code: failure.code, + message: failure.message, + error: original, + }, + snapshot: snapshot(), + }; + } else if (options.signal?.aborted && abortError) { + result = { + outcome: RunOutcome.Aborted, + skillId: input?.skillId, + failure: { + code: ErrorCodes.AgentAbort, + message: 'Agent run cancelled', + }, + snapshot: snapshot(), + }; + } else { + result = { + outcome: RunOutcome.Crashed, + skillId: input?.skillId, + failure: { + code: failure.code, + message: failure.message, + error: original, + }, + snapshot: snapshot(), + }; + } + } + try { const report = flushScanReport({ yaraReport: input.flags.yaraReport }); - if (report) log(report); + if (report) + collector?.emit({ kind: 'log', level: 'info', message: report }); + } catch { + // Scan reporting is best effort after the run outcome is decided. + } + return result; +} + +function safeErrorMessage(error: unknown): string { + try { + return String(error); + } catch { + return 'Unknown thrown value'; } } diff --git a/src/agent/runner/sequence/linear.ts b/src/agent/runner/sequence/linear.ts index e86ccbf4d..bbb58955f 100644 --- a/src/agent/runner/sequence/linear.ts +++ b/src/agent/runner/sequence/linear.ts @@ -11,10 +11,10 @@ */ import { OutroKind, type OutroData } from '@agent/progress'; -import { AgentErrorType, AgentSignals } from '../../agent-interface'; +import { AgentErrorType } from '../../agent-interface'; import { logToFile } from '@utils/debug'; import { createBenchmarkPipeline } from '@agent/middleware/benchmark'; -import { AGENT_ERROR_CODE, ErrorCodes, WizardError } from '@shared/errors'; +import { AGENT_ERROR_CODE, ErrorCodes } from '@shared/errors'; import { analytics } from '@utils/analytics'; import { formatYaraAbortMessage } from '@agent/yara-hooks'; import { installSkillById } from '@agent/tools'; @@ -33,10 +33,16 @@ export async function runLinearProgram({ boot, emit, interaction, + signal, }: SequenceContext): Promise { const { run, composed } = config; const { skillsBaseUrl, credentials, project } = boot; const { projectApiKey, host, projectId } = credentials; + const aborted = (): SequenceResult => ({ + outcome: RunOutcome.Aborted, + failure: { code: ErrorCodes.AgentAbort, message: 'Agent run cancelled' }, + }); + if (signal?.aborted) return aborted(); // 5. Skill install (if skillId provided) let skillPath: string | undefined; @@ -48,6 +54,7 @@ export async function runLinearProgram({ skillsBaseUrl, { triage: boot.triageProvider }, ); + if (signal?.aborted) return aborted(); if (installResult.kind !== 'ok') { return failed(installFailure(run.integrationLabel, installResult)); } @@ -72,6 +79,7 @@ export async function runLinearProgram({ getSource: () => input.skillId ?? run.integrationLabel, richLinks: run.richLinks ?? false, timeoutMs: run.askTimeoutMs, + signal, }); const middleware = input.flags.benchmark @@ -95,6 +103,7 @@ export async function runLinearProgram({ : null, }); logToFile(`[agent-runner] prompt assembled (${prompt.length} chars)`); + if (signal?.aborted) return aborted(); // 8. Run the agent through the run-level harness. The harness owns the agent // loop + model transport; everything around it (skill install, prompt, ask @@ -112,14 +121,23 @@ export async function runLinearProgram({ middleware, model, thinkingLevel, + signal, }); + if (signal?.aborted && agentResult.kind === 'success') return aborted(); // 9. Error handling (full set from both harnesses) - if (agentResult.failure) { + if (agentResult.kind === 'decided_failure') { return failed(agentResult.failure); } - if (agentResult.error === AgentErrorType.ABORT) { + if (agentResult.kind === 'success') { + // Success continues through the post-run hooks and outro below. + } else if (agentResult.kind !== 'abort' && agentResult.kind !== 'failure') { + const _exhaustive: never = agentResult; + return _exhaustive; + } + + if (agentResult.kind === 'abort') { const reason = agentResult.message ?? ''; const matched = run.abortCases?.find((c) => c.match.test(reason)); const abortCode = matched?.errorCode ?? ErrorCodes.AgentAbort; @@ -148,22 +166,20 @@ export async function runLinearProgram({ return { outcome: RunOutcome.Aborted, failure: { + message: matched?.message ?? `${run.integrationLabel} aborted`, outroData, code: abortCode, - error: new WizardError( - `Agent aborted: ${reason}`, - { - integration: run.integrationLabel, - error_type: AgentErrorType.ABORT, - reason, - }, - abortCode, - ), + error: agentResult.error, }, }; } - if (agentResult.error === AgentErrorType.MCP_MISSING) { + const classification = + agentResult.kind === 'failure' ? agentResult.classification : undefined; + const failureMessage = + agentResult.kind === 'failure' ? agentResult.message : undefined; + + if (classification === AgentErrorType.MCP_MISSING) { return failed({ code: AGENT_ERROR_CODE[AgentErrorType.MCP_MISSING], message: @@ -171,53 +187,30 @@ export async function runLinearProgram({ 'The wizard was unable to connect to the PostHog MCP server.\n' + 'This could be due to a network issue or a configuration problem.\n\n' + `Please try again, or check the documentation:\n${run.docsUrl}`, - error: new WizardError( - 'Agent could not access PostHog MCP server', - { - integration: run.integrationLabel, - error_type: AgentErrorType.MCP_MISSING, - signal: AgentSignals.ERROR_MCP_MISSING, - }, - AGENT_ERROR_CODE[AgentErrorType.MCP_MISSING], - ), + error: agentResult.kind === 'failure' ? agentResult.error : undefined, }); } - if (agentResult.error === AgentErrorType.RESOURCE_MISSING) { + if (classification === AgentErrorType.RESOURCE_MISSING) { return failed({ code: AGENT_ERROR_CODE[AgentErrorType.RESOURCE_MISSING], message: 'Could not access the setup resource\n\n' + 'This may indicate a version mismatch or a temporary service issue.\n\n' + `Please try again, or check the documentation:\n${run.docsUrl}`, - error: new WizardError( - 'Agent could not access setup resource', - { - integration: run.integrationLabel, - error_type: AgentErrorType.RESOURCE_MISSING, - signal: AgentSignals.ERROR_RESOURCE_MISSING, - }, - AGENT_ERROR_CODE[AgentErrorType.RESOURCE_MISSING], - ), + error: agentResult.kind === 'failure' ? agentResult.error : undefined, }); } - if (agentResult.error === AgentErrorType.YARA_VIOLATION) { + if (classification === AgentErrorType.YARA_VIOLATION) { return failed({ code: AGENT_ERROR_CODE[AgentErrorType.YARA_VIOLATION], message: formatYaraAbortMessage(), - error: new WizardError( - 'YARA scanner terminated session', - { - integration: run.integrationLabel, - error_type: AgentErrorType.YARA_VIOLATION, - }, - AGENT_ERROR_CODE[AgentErrorType.YARA_VIOLATION], - ), + error: agentResult.kind === 'failure' ? agentResult.error : undefined, }); } - if (agentResult.error === AgentErrorType.NO_PROGRESS) { + if (classification === AgentErrorType.NO_PROGRESS) { analytics.wizardCapture('agent no progress', { integration: run.integrationLabel, error_type: AgentErrorType.NO_PROGRESS, @@ -227,18 +220,11 @@ export async function runLinearProgram({ message: 'The Wizard exited without changing your project. Please contact the ' + 'PostHog team with wizard@posthog.com about this error.', - error: new WizardError( - 'Agent made no progress', - { - integration: run.integrationLabel, - error_type: AgentErrorType.NO_PROGRESS, - }, - AGENT_ERROR_CODE[AgentErrorType.NO_PROGRESS], - ), + error: agentResult.kind === 'failure' ? agentResult.error : undefined, }); } - if (agentResult.error === AgentErrorType.INCOMPLETE_TASKS) { + if (classification === AgentErrorType.INCOMPLETE_TASKS) { analytics.wizardCapture('agent incomplete tasks', { integration: run.integrationLabel, error_type: AgentErrorType.INCOMPLETE_TASKS, @@ -248,46 +234,41 @@ export async function runLinearProgram({ message: 'The Wizard exited without completing its planned tasks. Please contact ' + 'the PostHog team with wizard@posthog.com about this error.', - error: new WizardError( - 'Agent left planned tasks incomplete', - { - integration: run.integrationLabel, - error_type: AgentErrorType.INCOMPLETE_TASKS, - }, - AGENT_ERROR_CODE[AgentErrorType.INCOMPLETE_TASKS], - ), + error: agentResult.kind === 'failure' ? agentResult.error : undefined, }); } if ( - agentResult.error === AgentErrorType.RATE_LIMIT || - agentResult.error === AgentErrorType.API_ERROR + classification === AgentErrorType.RATE_LIMIT || + classification === AgentErrorType.API_ERROR ) { analytics.wizardCapture('agent api error', { integration: run.integrationLabel, - error_type: agentResult.error, - error_message: agentResult.message, + error_type: classification, + error_message: failureMessage, }); return failed({ - code: AGENT_ERROR_CODE[agentResult.error], + code: AGENT_ERROR_CODE[classification], message: `API Error\n\n${ - agentResult.message || 'Unknown error' + failureMessage || 'Unknown error' }\n\nPlease report this to: wizard@posthog.com`, - error: new WizardError( - `API error: ${agentResult.message}`, - { - integration: run.integrationLabel, - error_type: agentResult.error, - }, - AGENT_ERROR_CODE[agentResult.error], - ), + error: agentResult.kind === 'failure' ? agentResult.error : undefined, + }); + } + + if (agentResult.kind === 'failure') { + return failed({ + code: AGENT_ERROR_CODE[agentResult.classification], + message: agentResult.message ?? 'Agent failed', + error: agentResult.error, }); } // 10. Post-run hooks if (config.hooks?.postRun) { await config.hooks.postRun(credentials); + if (signal?.aborted) return aborted(); } // A composed sub-run leaves the terminal outro to its host. diff --git a/src/agent/runner/sequence/orchestrator/__tests__/executor.test.ts b/src/agent/runner/sequence/orchestrator/__tests__/executor.test.ts index a5236f1f5..aa4e9647a 100644 --- a/src/agent/runner/sequence/orchestrator/__tests__/executor.test.ts +++ b/src/agent/runner/sequence/orchestrator/__tests__/executor.test.ts @@ -1,6 +1,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; +import { ErrorCodes } from '@shared/errors'; import { QueueStore, TaskStatus, @@ -41,7 +42,10 @@ describe('drainQueue', () => { }; it('waits for live siblings after a fatal error and starts no dependents', async () => { - const fatal = new RunTaskFatal({ message: 'Authentication failed' }); + const fatal = new RunTaskFatal({ + code: ErrorCodes.AgentOrchestratorTasksFailed, + message: 'Authentication failed', + }); let release!: () => void; const blocked = new Promise((resolve) => { release = resolve; @@ -73,7 +77,10 @@ describe('drainQueue', () => { it('preserves a fatal failure when a sibling completes in the same turn', async () => { q.enqueue({ type: 'success' }); q.enqueue({ type: 'fatal' }); - const fatal = new RunTaskFatal({ message: 'Authentication failed' }); + const fatal = new RunTaskFatal({ + code: ErrorCodes.AgentOrchestratorTasksFailed, + message: 'Authentication failed', + }); await expect( drainQueue(q, (task) => { if (task.type === 'fatal') return Promise.reject(fatal); @@ -83,6 +90,51 @@ describe('drainQueue', () => { ).rejects.toBe(fatal); }); + it('cancels and joins an active sibling while retaining the first fatal', async () => { + const controller = new AbortController(); + const fatal = new RunTaskFatal({ + code: ErrorCodes.AgentOrchestratorTasksFailed, + message: 'First failure', + }); + const later = new RunTaskFatal({ + code: ErrorCodes.AgentOrchestratorTasksFailed, + message: 'Abort noise', + }); + const a = q.enqueue({ type: 'fatal' }); + q.enqueue({ type: 'asking' }); + const queued = q.enqueue({ type: 'queued', dependsOn: [a.id] }); + const started: string[] = []; + let siblingSettled = false; + const result = drainQueue( + q, + async (task) => { + started.push(task.type); + if (task.type === 'fatal') throw fatal; + await new Promise((resolve) => + controller.signal.addEventListener('abort', () => resolve(), { + once: true, + }), + ); + siblingSettled = true; + throw later; + }, + { + maxStarts: 50, + signal: controller.signal, + onFatal: () => { + controller.abort(); + throw new Error('Cancellation cleanup failed'); + }, + }, + ).catch((error: unknown) => error); + + expect(await result).toBe(fatal); + expect(controller.signal.aborted).toBe(true); + expect(siblingSettled).toBe(true); + expect(started).toEqual(['fatal', 'asking']); + expect(q.get(queued.id)?.status).toBe(TaskStatus.Pending); + }); + it('runs a single task to done and drains', async () => { const a = q.enqueue({ type: 'install' }); await drainQueue(q, completing, { maxStarts: 50 }); diff --git a/src/agent/runner/sequence/orchestrator/__tests__/task-notice-timeout.test.ts b/src/agent/runner/sequence/orchestrator/__tests__/task-notice-timeout.test.ts index f1227f562..f46dc863d 100644 --- a/src/agent/runner/sequence/orchestrator/__tests__/task-notice-timeout.test.ts +++ b/src/agent/runner/sequence/orchestrator/__tests__/task-notice-timeout.test.ts @@ -81,6 +81,18 @@ describe('task notice timeout', () => { } }); + it('closes an active notice on run cancellation without marking a timeout', async () => { + const controller = new AbortController(); + showTaskNotice.mockReturnValue(new Promise(() => undefined)); + const result = offerSeededTask(NOTICE, { + interaction, + signal: controller.signal, + }); + controller.abort(); + await expect(result).resolves.toEqual({ keep: false, timedOut: false }); + expect(cancelTaskNotice).toHaveBeenCalledTimes(1); + }); + it('keeps the step when the user accepts in time', async () => { vi.useFakeTimers(); try { diff --git a/src/agent/runner/sequence/orchestrator/executor.ts b/src/agent/runner/sequence/orchestrator/executor.ts index e70e5723c..f33baebdc 100644 --- a/src/agent/runner/sequence/orchestrator/executor.ts +++ b/src/agent/runner/sequence/orchestrator/executor.ts @@ -10,7 +10,7 @@ * injected: the real one spins up a fresh agent, the tests use a fake. */ import { analytics } from '@utils/analytics'; -import type { AgentFailure } from '../../shared/types'; +import { RunOutcome, type AgentFailure } from '../../shared/types'; import { logToFile } from '@utils/debug'; import { TaskStatus, type QueueStore, type QueuedTask } from './queue'; @@ -41,7 +41,12 @@ export type RunTask = (task: QueuedTask) => Promise; * the harness used to exit the process. */ export class RunTaskFatal extends Error { - constructor(public readonly failure: AgentFailure) { + constructor( + public readonly failure: AgentFailure, + public readonly outcome: + | RunOutcome.Aborted + | RunOutcome.Failed = RunOutcome.Failed, + ) { super(failure.message ?? 'agent run failed'); this.name = 'RunTaskFatal'; } @@ -50,6 +55,8 @@ export class RunTaskFatal extends Error { export interface DrainOptions { /** Backstop against a pathological always-one-more-pending loop. */ maxStarts: number; + signal?: AbortSignal; + onFatal?: () => void; } export const DEFAULT_DRAIN_OPTIONS: DrainOptions = { @@ -60,11 +67,14 @@ async function runOne( store: QueueStore, runTask: RunTask, task: QueuedTask, + signal?: AbortSignal, ): Promise { + if (signal?.aborted) return; store.start(task.id); try { await runTask(task); } catch (error) { + if (signal?.aborted) return; if (error instanceof RunTaskFatal) throw error; // The task threw rather than reporting. The outcome check below handles // the queue; the exception itself should never be silent. @@ -75,6 +85,8 @@ async function runOne( ); } + if (signal?.aborted) return; + const after = store.get(task.id); if (!after) return; @@ -116,12 +128,28 @@ export async function drainQueue( try { for (;;) { + if (opts.signal?.aborted) break; if (failure) throw failure.error; for (const task of store.nextRunnable()) { + if (opts.signal?.aborted) break; if (++starts > opts.maxStarts) break; - const p = runOne(store, runTask, task) + const p = runOne(store, runTask, task, opts.signal) .catch((error: unknown) => { - failure ??= { error }; + if (!failure) { + failure = { error }; + try { + opts.onFatal?.(); + } catch (abortError) { + try { + logToFile( + '[executor] fatal cancellation failed:', + abortError, + ); + } catch { + // Reporting cancellation failure cannot replace the fatal. + } + } + } }) .finally(() => running.delete(task.id)); running.set(task.id, p); @@ -133,4 +161,5 @@ export async function drainQueue( // No queue or skill cleanup may run while a sibling still uses them. await Promise.allSettled(running.values()); } + if (failure) throw failure.error; } diff --git a/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts b/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts index 2d28b1d61..fbb024137 100644 --- a/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts +++ b/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts @@ -30,7 +30,13 @@ import { analytics } from '@utils/analytics'; import { ciExcludedTaskTypes } from '@utils/ci-flag-overrides'; import { logToFile } from '@utils/debug'; import { ringTerminalBell } from '@utils/terminal-bell'; -import { ErrorCodes, WizardError } from '@shared/errors'; +import { + AGENT_ERROR_CODE, + classifyRunFailure, + ErrorCodes, + WizardError, +} from '@shared/errors'; +import type { AgentResult } from '../../harness/types'; import type { AgentInteraction } from '@agent/progress'; import type { AgentFailure, @@ -55,7 +61,12 @@ import { TaskStatus, type QueuedTask, } from './queue'; -import { drainQueue, RunTaskFatal, type RunTask } from './executor'; +import { + DEFAULT_DRAIN_OPTIONS, + drainQueue, + RunTaskFatal, + type RunTask, +} from './executor'; import { RunMetrics } from './run-metrics'; import { dependencyClosure, uncoveredBySink } from './queue-tools'; import { deferSeededTasks } from './seeded-deps'; @@ -150,6 +161,45 @@ function requireTaskHarness(pick: HarnessPick): AgentHarness & { }; } +function terminalResult(result: AgentResult): + | { + outcome: RunOutcome.Aborted | RunOutcome.Failed; + failure: AgentFailure; + } + | undefined { + switch (result.kind) { + case 'success': + return undefined; + case 'decided_failure': + return { outcome: RunOutcome.Failed, failure: result.failure }; + case 'abort': + return { + outcome: RunOutcome.Aborted, + failure: { + code: AGENT_ERROR_CODE[result.classification], + message: result.message ?? 'Agent aborted', + error: result.error, + }, + }; + case 'failure': + return { + outcome: RunOutcome.Failed, + failure: { + code: AGENT_ERROR_CODE[result.classification], + message: result.message ?? 'Agent failed', + error: result.error, + }, + }; + } +} + +function cancelledRun(): SequenceResult { + return { + outcome: RunOutcome.Aborted, + failure: { code: ErrorCodes.AgentAbort, message: 'Agent run cancelled' }, + }; +} + /** Every skill entry the menu knows, across categories. */ async function fetchSkillMenuEntries( skillsBaseUrl: string, @@ -205,6 +255,7 @@ export const TASK_NOTICE_TIMEOUT_MS = 5 * 60 * 1000; interface SeededTaskOptions { timeoutMs?: number; interaction?: AgentInteraction; + signal?: AbortSignal; } /** @@ -217,8 +268,13 @@ interface SeededTaskOptions { */ export async function offerSeededTask( notice: TaskNotice, - { timeoutMs = TASK_NOTICE_TIMEOUT_MS, interaction }: SeededTaskOptions = {}, + { + timeoutMs = TASK_NOTICE_TIMEOUT_MS, + interaction, + signal, + }: SeededTaskOptions = {}, ): Promise<{ keep: boolean; timedOut: boolean }> { + if (signal?.aborted) return { keep: false, timedOut: false }; // No one to show the notice to: a step nobody can answer for must not run. // The same answer a non-interactive host gives today. if (!interaction?.taskNotice) return { keep: false, timedOut: false }; @@ -226,20 +282,38 @@ export async function offerSeededTask( let timer: ReturnType | undefined; let timedOut = false; + let cancelForAbort: (() => void) | undefined; const timeout = new Promise((resolve) => { timer = setTimeout(() => { timedOut = true; // Dismisses the overlay and settles the showTaskNotice promise too, so // the losing side of the race cannot leave a modal on screen. - cancelTaskNotice?.(); + try { + cancelTaskNotice?.(); + } catch { + // An overlay failure must not leave a pending notice promise. + } resolve(false); }, timeoutMs); }); + const aborted = new Promise((resolve) => { + cancelForAbort = () => { + try { + cancelTaskNotice?.(); + } catch { + // A host overlay failure must not prevent the run from settling. + } + resolve(false); + }; + signal?.addEventListener('abort', cancelForAbort, { once: true }); + if (signal?.aborted) cancelForAbort(); + }); try { - const keep = await Promise.race([taskNotice(notice), timeout]); + const keep = await Promise.race([taskNotice(notice), timeout, aborted]); return { keep, timedOut }; } finally { if (timer) clearTimeout(timer); + if (cancelForAbort) signal?.removeEventListener('abort', cancelForAbort); } } @@ -431,6 +505,10 @@ export function displayOrder( export async function runOrchestrator( context: SequenceContext, ): Promise { + const controller = new AbortController(); + const abortFromHost = () => controller.abort(); + context.signal?.addEventListener('abort', abortFromHost, { once: true }); + if (context.signal?.aborted) abortFromHost(); let cleaned = false; const cleanupQueue = (): void => { if (cleaned) return; @@ -441,23 +519,34 @@ export async function runOrchestrator( force: true, }); } catch (error) { - analytics.captureException( - error instanceof Error ? error : new Error(String(error)), - { step: 'orchestrator_cache_cleanup' }, - ); + try { + analytics.captureException( + error instanceof Error ? error : new Error(String(error)), + { step: 'orchestrator_cache_cleanup' }, + ); + } catch { + // Cleanup reporting must not replace the run result. + } } }; try { - return await executeOrchestrator(context, cleanupQueue); + return await executeOrchestrator( + { ...context, signal: controller.signal }, + cleanupQueue, + controller, + ); } finally { + context.signal?.removeEventListener('abort', abortFromHost); cleanupQueue(); } } async function executeOrchestrator( - { config, input, boot, emit, interaction }: SequenceContext, + { config, input, boot, emit, interaction, signal }: SequenceContext, cleanupQueue: () => void, + controller: AbortController, ): Promise { + if (signal?.aborted) return cancelledRun(); const runId = randomUUID(); const { run } = config; const programId = config.programId; @@ -480,6 +569,7 @@ async function executeOrchestrator( boot.wizardFlagPayloads, ), }); + if (signal?.aborted) return cancelledRun(); const seedPrompt = registry.seed; if (!seedPrompt) { throw new Error( @@ -587,6 +677,7 @@ async function executeOrchestrator( let commandmentsPath: string | undefined; let referenceInstallPath: string | undefined; const menuSkillEntries = await fetchSkillMenuEntries(boot.skillsBaseUrl); + if (signal?.aborted) return cancelledRun(); // The framework key for reference + variant resolution. `input.integration` // is the detected framework and always wins; `input.skillId` is the // fallback for the basic-integration path, where the caller sets it to the @@ -607,6 +698,7 @@ async function executeOrchestrator( triage: boot.triageProvider, }, ); + if (signal?.aborted) return cancelledRun(); if (ref.kind === 'ok') { referenceInstallPath = ref.path; const example = path.join(ref.path, 'references', 'EXAMPLE.md'); @@ -779,8 +871,12 @@ async function executeOrchestrator( // not, which is why the offer lives here and not there. seededConsent.set( task.id, - await askSeededConsent(seeded.type, seeded.notice, { interaction }), + await askSeededConsent(seeded.type, seeded.notice, { + interaction, + signal, + }), ); + if (signal?.aborted) return cancelledRun(); } logToFile(`[orchestrator] runner-seeded task ${seeded.type}`); } @@ -814,6 +910,7 @@ async function executeOrchestrator( const askBridge = shouldDisableAsk(input.flags) ? undefined : createAskBridge(interaction, { + signal, getSource: () => input.skillId ?? programId, beforeShow: () => { // How late the first ask lands is the measure of this run shape: it @@ -845,6 +942,7 @@ async function executeOrchestrator( const seedHarness = requireTaskHarness(seedPick); const seedModel = promptModelFor(seedPrompt, seedPick.harness); const seedResult = await seedHarness.runTask({ + signal, config, input, boot, @@ -861,15 +959,9 @@ async function executeOrchestrator( requestRemark: false, analyticsProperties: { task_type: 'seed', harness: seedPick.harness }, }); - // A decided seed failure ends the run and releases its queue artifacts. - if (seedResult.failure) return failed(seedResult.failure); - if (seedResult.error) { - logToFile( - `[orchestrator] seed error: ${seedResult.error} ${ - seedResult.message ?? '' - }`, - ); - } + if (signal?.aborted) return cancelledRun(); + const seedTerminal = terminalResult(seedResult); + if (seedTerminal) return seedTerminal; analytics.wizardCapture('orchestrator seeded', { task_count: store.list().length, types: store.list().map((t) => t.type), @@ -992,6 +1084,7 @@ async function executeOrchestrator( existsSync(claudeSkillsDir) ? readdirSync(claudeSkillsDir) : [], ); const runTask: RunTask = async (task) => { + if (signal?.aborted) return; renderQueue(); try { @@ -1002,6 +1095,7 @@ async function executeOrchestrator( // The prompt points the agent at them instead. const skillPaths: string[] = []; for (const skillId of resolved.skills) { + if (signal?.aborted) return; // Agent prompts name the bare step-skill (`integration-v2-install`); // SDK-divergent steps ship per-framework variants, so resolve against // the menu with the session's framework before installing. @@ -1024,6 +1118,7 @@ async function executeOrchestrator( boot.skillsBaseUrl, { skillsRoot: taskSkillsRoot, triage: boot.triageProvider }, ); + if (signal?.aborted) return; if (result.kind === 'ok') { skillPaths.push(path.join(result.path, 'SKILL.md')); } else { @@ -1051,33 +1146,49 @@ async function executeOrchestrator( const taskPick = resolveHarness(switchboardCtx, task.type); const taskHarness = requireTaskHarness(taskPick); const taskModel = taskModelSpec(registry, task, taskPick.harness); - const taskResult = await taskHarness.runTask({ - config, - input, - boot, - emit, - prompt: assembleTaskPrompt(promptContext, resolved.prompt, skillPaths), - spinner, - model: requireKnownModel(taskModel.model, taskPick.model), - effort: taskModel.effort, - allowedTools: resolved.allowedTools, - disallowedTools: resolved.disallowedTools, - askBridge: canAsk(registry.get(task.type)) ? askBridge : undefined, - orchestrator: orchestratorCtx(task.id), - spinnerMessage: '', - successMessage: '', - additionalFeatureQueue: [], - requestRemark: false, - analyticsProperties: { - task_type: task.type, - task_id: task.id, - harness: taskPick.harness, - }, - }); - // A decided failure (a 401 the harness already reported) is the run's, - // not the task's: stop the drain and report it, where the harness used - // to exit the process. - if (taskResult.failure) throw new RunTaskFatal(taskResult.failure); + let taskResult: AgentResult; + try { + taskResult = await taskHarness.runTask({ + signal, + config, + input, + boot, + emit, + prompt: assembleTaskPrompt( + promptContext, + resolved.prompt, + skillPaths, + ), + spinner, + model: requireKnownModel(taskModel.model, taskPick.model), + effort: taskModel.effort, + allowedTools: resolved.allowedTools, + disallowedTools: resolved.disallowedTools, + askBridge: canAsk(registry.get(task.type)) ? askBridge : undefined, + orchestrator: orchestratorCtx(task.id), + spinnerMessage: '', + successMessage: '', + additionalFeatureQueue: [], + requestRemark: false, + analyticsProperties: { + task_type: task.type, + task_id: task.id, + harness: taskPick.harness, + }, + }); + } catch (error) { + if (signal?.aborted) return; + if (error instanceof RunTaskFatal) throw error; + const failure = classifyRunFailure(error); + throw new RunTaskFatal({ + code: failure.code, + message: failure.message, + error: error instanceof Error ? error : undefined, + }); + } + if (signal?.aborted) return; + const terminal = terminalResult(taskResult); + if (terminal) throw new RunTaskFatal(terminal.failure, terminal.outcome); } finally { // Durable skills a task installed are irrelevant to later tasks — and // the sdk harness auto-loads .claude/skills into every agent — so sweep @@ -1089,9 +1200,21 @@ async function executeOrchestrator( referenceSkillId, ); } catch (err) { - logToFile(`[orchestrator] per-task skill sweep failed: ${String(err)}`); + try { + logToFile('[orchestrator] per-task skill sweep failed:', err); + } catch { + // Cleanup logging must not replace the task result. + } + } + try { + renderQueue(); + } catch (err) { + try { + logToFile('[orchestrator] per-task queue render failed:', err); + } catch { + // Cleanup logging must not replace the task result. + } } - renderQueue(); } }; // A task that stops for the user is offered, not imposed, and the answer was @@ -1101,15 +1224,19 @@ async function executeOrchestrator( renderQueue(); } - let fatal: AgentFailure | undefined; + let fatal: RunTaskFatal | undefined; try { - await drainQueue(store, runTask); + await drainQueue(store, runTask, { + ...DEFAULT_DRAIN_OPTIONS, + signal, + onFatal: () => controller.abort(), + }); } catch (error) { if (!(error instanceof RunTaskFatal)) throw error; - fatal = error.failure; + fatal = error; } finally { try { - if (referenceSkillId && referenceInstallPath) { + if (!signal?.aborted && referenceSkillId && referenceInstallPath) { promoteReferenceSkill( path.join(input.installDir, referenceInstallPath), claudeSkillsDir, @@ -1117,10 +1244,14 @@ async function executeOrchestrator( ); } } catch (err) { - analytics.captureException( - err instanceof Error ? err : new Error(String(err)), - { step: 'orchestrator_reference_promote' }, - ); + try { + analytics.captureException( + err instanceof Error ? err : new Error(String(err)), + { step: 'orchestrator_reference_promote' }, + ); + } catch { + // Cleanup reporting must not replace the run result. + } } cleanupQueue(); try { @@ -1130,14 +1261,19 @@ async function executeOrchestrator( referenceSkillId, ); } catch (err) { - analytics.captureException( - err instanceof Error ? err : new Error(String(err)), - { step: 'orchestrator_skill_sweep' }, - ); + try { + analytics.captureException( + err instanceof Error ? err : new Error(String(err)), + { step: 'orchestrator_skill_sweep' }, + ); + } catch { + // Cleanup reporting must not replace the run result. + } } } - if (fatal) return failed(fatal); + if (fatal) return { outcome: fatal.outcome, failure: fatal.failure }; + if (signal?.aborted) return cancelledRun(); renderQueue(); diff --git a/src/agent/runner/shared/ask.ts b/src/agent/runner/shared/ask.ts index ffe178940..af1be4729 100644 --- a/src/agent/runner/shared/ask.ts +++ b/src/agent/runner/shared/ask.ts @@ -21,6 +21,7 @@ export function createAskBridge( getSource: () => string; richLinks: boolean; timeoutMs?: number; + signal?: AbortSignal; /** Runs before each question is shown (the orchestrator's bell and metric). */ beforeShow?: () => void; }, @@ -37,5 +38,6 @@ export function createAskBridge( cancelQuestion: interaction?.cancelAsk, richLinks: options.richLinks, timeoutMs: options.timeoutMs, + signal: options.signal, }); } diff --git a/src/agent/runner/shared/errors.ts b/src/agent/runner/shared/errors.ts index d89d3c214..d227a7f0f 100644 --- a/src/agent/runner/shared/errors.ts +++ b/src/agent/runner/shared/errors.ts @@ -3,8 +3,7 @@ */ import type { InstallSkillResult } from '@agent/tools'; -import { skillErrorCode } from '@shared/errors'; -import { WizardError } from '@shared/errors'; +import { ErrorCodes, skillErrorCode } from '@shared/errors'; import { RunOutcome, type AgentFailure, type SequenceResult } from './types'; export const failed = (failure: AgentFailure): SequenceResult => ({ @@ -17,7 +16,7 @@ export function installFailure( integrationLabel: string, result: Exclude, ): AgentFailure { - const code = skillErrorCode(result) ?? undefined; + const code = skillErrorCode(result) ?? ErrorCodes.InternalUnhandled; const message = (() => { switch (result.kind) { @@ -33,17 +32,13 @@ export function installFailure( return { message, code, - error: new WizardError( - `Skill install failed: ${result.kind}`, - { - integration: integrationLabel, - error_type: result.kind, - platform: process.platform, - ...(result.kind === 'download-failed' - ? { error_detail: result.message.slice(0, 500) } - : {}), - }, - code, - ), + detail: { + integration: integrationLabel, + error_type: result.kind, + platform: process.platform, + ...(result.kind === 'download-failed' + ? { error_detail: result.message.slice(0, 500) } + : {}), + }, }; } diff --git a/src/agent/runner/shared/progress-collector.ts b/src/agent/runner/shared/progress-collector.ts index 719603e63..98b6e5154 100644 --- a/src/agent/runner/shared/progress-collector.ts +++ b/src/agent/runner/shared/progress-collector.ts @@ -23,7 +23,7 @@ export interface ProgressCollector { } export function createProgressCollector( - onProgress?: (event: AgentProgress) => void, + onProgress?: (event: AgentProgress) => unknown, ): ProgressCollector { const snapshot: RunSnapshot = { tasks: [], @@ -75,14 +75,33 @@ export function createProgressCollector( apply(event); if (!onProgress) return; try { - onProgress(structuredClone(event)); + const observed = onProgress(structuredClone(event)); + if ( + observed && + typeof (observed as PromiseLike).then === 'function' + ) { + void Promise.resolve(observed).catch((error: unknown) => { + try { + logToFile( + `[agent] progress observer rejected on ${event.kind}:`, + error, + ); + } catch { + // Logging is best effort. + } + }); + } } catch (error) { // A broken projection is the host's problem, not the run's. Say so in // the log and carry on; the snapshot above is the source of truth. - logToFile( - `[agent] progress observer threw on ${event.kind}:`, - error instanceof Error ? error.message : error, - ); + try { + logToFile( + `[agent] progress observer threw on ${event.kind}:`, + error instanceof Error ? error.message : error, + ); + } catch { + // Logging is best effort. + } } }; diff --git a/src/agent/runner/shared/types.ts b/src/agent/runner/shared/types.ts index 3f97d5f5d..2907f2eaf 100644 --- a/src/agent/runner/shared/types.ts +++ b/src/agent/runner/shared/types.ts @@ -12,7 +12,7 @@ import type { AdditionalFeature } from '@shared/constants'; import type { CloudRegion } from '@utils/types'; import type { Credentials } from '@shared/api'; -import type { OutroData, TaskNotice } from '@agent/progress'; +import type { AuthErrorDetail, OutroData, TaskNotice } from '@agent/progress'; import type { PromptContext } from '@agent/agent-prompt'; import type { PackageManagerDetector } from '@utils/package-manager'; import type { ApiProject, ApiUser } from '@shared/api'; @@ -246,13 +246,14 @@ export interface BootstrapResult { * messages stay exactly what they were. */ export interface AgentFailure { - message?: string; + message: string; /** Structured error data. Renders via `outroError` instead of `outro`. */ outroData?: OutroData; error?: Error; exitCode?: number; - code?: ErrorCode; + code: ErrorCode; detail?: Record; + authErrorDetail?: AuthErrorDetail; } export enum RunOutcome { @@ -308,9 +309,10 @@ export type RunResult = ( export interface RunAgentOptions { /** Receives every progress event in emission order. Never awaited. */ - onProgress?: (event: import('@agent/progress').AgentProgress) => void; + onProgress?: (event: import('@agent/progress').AgentProgress) => unknown; /** Answers the agent's questions. Absent → no ask bridge, notices declined. */ interaction?: AgentInteraction; + signal?: AbortSignal; } /** What a sequence receives: the contracts plus the prepared run. */ @@ -320,4 +322,5 @@ export interface SequenceContext { boot: BootstrapResult; emit: ProgressEmitter; interaction: AgentInteraction | undefined; + signal?: AbortSignal; } diff --git a/src/agent/wizard-ask-bridge.ts b/src/agent/wizard-ask-bridge.ts index 68de191e5..505a51cd8 100644 --- a/src/agent/wizard-ask-bridge.ts +++ b/src/agent/wizard-ask-bridge.ts @@ -54,6 +54,7 @@ export interface WizardAskBridge { } export interface WizardAskBridgeOptions { + signal?: AbortSignal; /** Returns the active skill id, used as the analytics `source` on the request. */ getSource: () => string; /** Opens the overlay and resolves once the user submits or cancels. */ @@ -120,6 +121,9 @@ export function createWizardAskBridge( return null; }, async request({ questions, subject }) { + if (opts.signal?.aborted) { + return { answers: buildCancelledAnswers(questions), timedOut: false }; + } const pending: PendingQuestion = { id: randomUUID(), questions, @@ -132,6 +136,7 @@ export function createWizardAskBridge( const startedAt = Date.now(); let timer: ReturnType | undefined; let timedOut = false; + let cancelForAbort: (() => void) | undefined; // Race the user against the timeout. Whichever fires first wins. On // timeout we also cancel the host's overlay: resolving our side alone @@ -140,15 +145,33 @@ export function createWizardAskBridge( const timeoutPromise = new Promise((resolve) => { timer = setTimeout(() => { timedOut = true; - opts.cancelQuestion?.(); - resolve(buildCancelledAnswers(questions)); + try { + opts.cancelQuestion?.(); + } catch { + // A host overlay failure must not leave the ask pending. + } finally { + resolve(buildCancelledAnswers(questions)); + } }, timeoutMs); }); + const aborted = new Promise((resolve) => { + cancelForAbort = () => { + try { + opts.cancelQuestion?.(); + } catch { + // A host overlay failure must not prevent the run from settling. + } + resolve(buildCancelledAnswers(questions)); + }; + opts.signal?.addEventListener('abort', cancelForAbort, { once: true }); + if (opts.signal?.aborted) cancelForAbort(); + }); try { const answers = await Promise.race([ opts.showQuestion(pending), timeoutPromise, + aborted, ]); const durationMs = Date.now() - startedAt; @@ -172,6 +195,8 @@ export function createWizardAskBridge( return { answers, timedOut }; } finally { if (timer) clearTimeout(timer); + if (cancelForAbort) + opts.signal?.removeEventListener('abort', cancelForAbort); pendingQuestions.delete(pending.id); } }, diff --git a/src/lib/detection/__tests__/agentic-progress.test.ts b/src/lib/detection/__tests__/agentic-progress.test.ts index 41e7f0761..424c98414 100644 --- a/src/lib/detection/__tests__/agentic-progress.test.ts +++ b/src/lib/detection/__tests__/agentic-progress.test.ts @@ -6,6 +6,7 @@ import { import { buildSession } from '@lib/wizard-session'; import { HostResolution } from '@shared/host-resolution'; import { getUI } from '@ui'; +import { ErrorCodes } from '@shared/errors'; vi.mock('@utils/debug'); vi.mock('@ui', () => ({ getUI: () => ui })); @@ -13,6 +14,7 @@ const ui = vi.hoisted(() => ({ addTokenUsage: vi.fn(), setStage: vi.fn(), pushStatus: vi.fn(), + showAuthError: vi.fn(), log: { error: vi.fn() }, })); vi.mock('@agent/agent-interface', async (original) => ({ @@ -55,7 +57,7 @@ it('keeps initialization and execution progress visible during detection', async result: '{"projects":[{"path":".","targetId":"node","framework":"Node.js"}]}', }); - return Promise.resolve({}); + return Promise.resolve({ kind: 'success' }); }, ); const session = buildSession({ installDir: '/tmp/detection-test' }); @@ -78,3 +80,38 @@ it('keeps initialization and execution progress visible during detection', async ['Execution diagnostic'], ]); }); + +it('stops optional detection on a data-only 401 before parsing partial JSON', async () => { + vi.mocked(initializeAgent).mockResolvedValue( + {} as Awaited>, + ); + vi.mocked(executeAgent).mockImplementation( + (_config, _prompt, _options, _spinner, _messages, middleware) => { + middleware?.onMessage({ + type: 'result', + result: '{"projects":[{"path":".","targetId":"node"}]}', + }); + return Promise.resolve({ + kind: 'decided_failure', + failure: { + code: ErrorCodes.AuthInvalidOrExpired, + message: 'Authentication failed (401)', + }, + }); + }, + ); + const session = buildSession({ installDir: '/tmp/detection-test' }); + session.credentials = { + accessToken: 'test', + projectApiKey: 'phc_test', + projectId: 1, + host: HostResolution.fromApiHost('https://us.posthog.com'), + }; + await expect( + detectProjectsWithAgent(session, { + programId: 'posthog-integration', + targets: [{ id: 'node', name: 'Node.js' }], + }), + ).rejects.toThrow('Authentication failed (401)'); + expect(ui.showAuthError).not.toHaveBeenCalled(); +}); diff --git a/src/lib/detection/agentic.ts b/src/lib/detection/agentic.ts index e3f96aa22..f32d851fd 100644 --- a/src/lib/detection/agentic.ts +++ b/src/lib/detection/agentic.ts @@ -430,8 +430,14 @@ export async function detectProjectsWithAgent( middleware, ); - if (result.error) { - throw new Error(result.message || `Agent error: ${result.error}`); + if (result.kind !== 'success') { + if (result.kind === 'decided_failure') { + throw result.failure.error ?? new Error(result.failure.message); + } + throw ( + result.error ?? + new Error(result.message || `Agent error: ${result.classification}`) + ); } // Transcript first, final message last — its verdicts win path conflicts. diff --git a/src/lib/programs/__tests__/run-agent-legacy.test.ts b/src/lib/programs/__tests__/run-agent-legacy.test.ts index 1fdce1e41..c916085a5 100644 --- a/src/lib/programs/__tests__/run-agent-legacy.test.ts +++ b/src/lib/programs/__tests__/run-agent-legacy.test.ts @@ -6,10 +6,11 @@ import { Harness, Sequence } from '@shared/constants'; import { buildSession, OutroKind } from '@lib/wizard-session'; import { HostResolution } from '@shared/host-resolution'; import { LoggingUI } from '@ui/logging-ui'; -import { setUI } from '@ui'; +import { getUI, setUI } from '@ui'; import { analytics } from '@utils/analytics'; import { initLogFile } from '@utils/debug'; import { wizardAbort } from '@utils/wizard-abort'; +import { ErrorCodes } from '@shared/errors'; import type { ProgramConfig } from '../program-step'; const streamShutdown = vi.hoisted(() => vi.fn().mockResolvedValue(undefined)); @@ -171,7 +172,11 @@ it('clamps a composed program to linear and keeps host analytics alive', async ( it.each([RunOutcome.Aborted, RunOutcome.Failed] as const)( 'passes a %s result to the existing abort handler', async (outcome) => { - const failure = { message: 'Failed', exitCode: 2 }; + const failure = { + code: ErrorCodes.AgentApiError, + message: 'Failed', + exitCode: 2, + }; vi.mocked(runAgent).mockResolvedValue({ outcome, failure, snapshot }); await runProgramAgent(program(), session()); expect(wizardAbort).toHaveBeenCalledExactlyOnceWith(failure); @@ -179,11 +184,34 @@ it.each([RunOutcome.Aborted, RunOutcome.Failed] as const)( }, ); +it('shows the auth guidance from a decided 401 before the error outro', async () => { + const detail = { hasSettingsConflict: false, logFilePath: '/tmp/wizard.log' }; + const show = vi.spyOn(getUI(), 'showAuthError'); + vi.mocked(runAgent).mockResolvedValue({ + outcome: RunOutcome.Failed, + failure: { + code: ErrorCodes.AuthInvalidOrExpired, + message: 'Authentication failed (401)', + authErrorDetail: detail, + }, + snapshot, + }); + await runProgramAgent(program(), session()); + expect(show).toHaveBeenCalledExactlyOnceWith(detail); + expect(wizardAbort).toHaveBeenCalledWith( + expect.objectContaining({ authErrorDetail: detail }), + ); +}); + it('rethrows the original crash for the outer runner', async () => { const error = new Error('mint refused'); const result: RunResult = { outcome: RunOutcome.Crashed, - failure: { error }, + failure: { + code: ErrorCodes.InternalUnhandled, + message: error.message, + error, + }, snapshot, }; vi.mocked(runAgent).mockResolvedValue(result); diff --git a/src/lib/programs/run-agent-legacy.ts b/src/lib/programs/run-agent-legacy.ts index e0c0733d0..2eb076c2b 100644 --- a/src/lib/programs/run-agent-legacy.ts +++ b/src/lib/programs/run-agent-legacy.ts @@ -282,6 +282,9 @@ async function runProgram( throw result.failure.error; } if (result.outcome !== RunOutcome.Success) { + if (result.failure.authErrorDetail) { + ui.showAuthError(result.failure.authErrorDetail); + } await wizardAbort(result.failure); } } From 1443587cd5e3cc0c5c6d8a824d17cba67be7452d Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Tue, 22 Sep 2026 21:26:52 -0400 Subject: [PATCH 10/12] test(agent): probe terminal SDK fault propagation Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- scripts/a3-fault-probe.no-jest.ts | 119 ++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 scripts/a3-fault-probe.no-jest.ts diff --git a/scripts/a3-fault-probe.no-jest.ts b/scripts/a3-fault-probe.no-jest.ts new file mode 100644 index 000000000..91b83e5ac --- /dev/null +++ b/scripts/a3-fault-probe.no-jest.ts @@ -0,0 +1,119 @@ +import type { RunConfig, RunInput } from '@agent/runner'; + +const gatewayUrl = process.env.WIZARD_FAULT_GATEWAY_URL; +const installDir = process.env.WIZARD_FAULT_INSTALL_DIR; +const harness = process.env.WIZARD_FAULT_HARNESS; +if ( + !gatewayUrl || + !installDir || + !['anthropic', 'pi'].includes(harness ?? '') +) { + throw new Error( + 'Expected WIZARD_FAULT_GATEWAY_URL, WIZARD_FAULT_INSTALL_DIR, and WIZARD_FAULT_HARNESS', + ); +} + +const routedFetch = globalThis.fetch; +globalThis.fetch = (input, init) => { + const url = new URL(input instanceof Request ? input.url : String(input)); + if (url.hostname === 'internal-j.posthog.com') { + return Promise.resolve(new Response('{"status":1}', { status: 200 })); + } + if (url.hostname !== '127.0.0.1' && url.hostname !== 'localhost') { + return Promise.reject( + new Error('Fault probe blocked a non-loopback fetch'), + ); + } + return routedFetch(input, init); +}; + +const { runAgent } = await import('@agent/runner'); +const { configureGatewayCredentialsForCI } = await import( + '@agent/gateway-session' +); +const { DEFAULT_AGENT_MODEL, Harness, Sequence } = await import( + '@shared/constants' +); +const { HostResolution } = await import('@shared/host-resolution'); +const { analytics } = await import('@utils/analytics'); + +// This probe has no telemetry sink and uses only synthetic local credentials. +analytics.capture = () => {}; +analytics.captureException = () => {}; +analytics.wizardCapture = () => {}; +analytics.shutdown = async () => {}; + +configureGatewayCredentialsForCI( + 'phe_synthetic_fault_probe', + 228144, + gatewayUrl, +); + +const config: RunConfig = { + programId: 'fault-probe', + run: { + integrationLabel: 'Fault probe', + spinnerMessage: 'Running fault probe', + successMessage: 'Fault probe completed', + estimatedDurationMinutes: 1, + reportFile: 'fault-probe.md', + docsUrl: 'https://posthog.com/docs', + customPrompt: () => 'Answer briefly without using tools.', + }, + composed: false, + binding: { + sequence: Sequence.linear, + harness: harness as Harness, + model: DEFAULT_AGENT_MODEL, + }, + switchboard: { + program: 'fault-probe', + flags: {}, + cliHarness: harness as Harness, + }, + skillsBaseUrl: 'http://127.0.0.1:1', + wizardFlags: {}, + wizardFlagPayloads: {}, + wizardMetadata: { run_id: 'fault-probe' }, +}; + +const input: RunInput = { + installDir, + credentials: { + accessToken: 'phx_synthetic_fault_probe', + projectApiKey: 'phc_synthetic_fault_probe', + host: HostResolution.fromApiHost('http://127.0.0.1:1', { localMcp: true }), + projectId: 228144, + }, + project: null, + apiUser: null, + flags: { + ci: true, + signup: false, + debug: false, + e2eAsk: false, + localMcp: true, + captureAio: false, + benchmark: false, + yaraReport: false, + }, + host: { projectId: 228144, region: 'us' }, +}; + +const result = await runAgent(config, input); +process.stdout.write( + `WIZARD_FAULT_RESULT ${JSON.stringify({ + harness, + outcome: result.outcome, + code: result.failure?.code, + message: result.failure?.message, + hasError: result.failure?.error instanceof Error, + outroKind: result.outro?.kind, + })}\n`, +); +if (result.outcome === 'failed' || result.outcome === 'aborted') { + const { wizardAbort } = await import('@utils/wizard-abort'); + await wizardAbort(result.failure); +} else { + process.exitCode = 2; +} From 62c4387cbd0d4f89b7956d557d7a146f619bcfa6 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Wed, 23 Sep 2026 12:01:50 -0400 Subject: [PATCH 11/12] fix(agent): label decided failures as errors and cancel open asks when a run ends Terminal analytics. Since A3's tagged results, a decided failure with no Error object (MCP missing, YARA violation, no progress, API errors read from output, an SDK result failure, the agent's own [ABORT]) reached wizardAbort without an Error. So the run was labelled 'cancelled' and skipped error tracking. wizardAbort now takes an explicit status. The legacy host passes 'cancelled' only for a host-cancelled run and 'error' for everything else, and captures a WizardError built from the failure's code and message. The agent's own [ABORT] returns Failed; Aborted now means only that the host's signal cancelled the run. What the user sees is unchanged. Orchestrator funnel. When a sibling's fatal result ended the drain, the accepted steps it stopped sent no 'orchestrator task blocked' event. The fatal path now sends them too, through one helper that catches analytics errors, so a failed capture can't turn a decided result into Crashed. Linear cancellation. With no siblings, an ask left open when the harness ended terminally stayed pending until its own timeout. The linear sequence now owns a run controller, as the orchestrator does: the host signal forwards to it, and it aborts when the run ends. The runner and agent READMEs say terminal analytics are the host's. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- src/__tests__/wizard-abort.test.ts | 29 +++++ src/agent/README.md | 7 +- .../__tests__/run-agent-standalone.test.ts | 118 +++++++++++++++++- src/agent/runner/README.md | 6 +- src/agent/runner/sequence/linear.ts | 36 ++++-- .../runner/sequence/orchestrator/executor.ts | 2 + .../orchestrator/orchestrator-runner.ts | 68 ++++++---- src/agent/wizard-ask-bridge.ts | 1 - .../__tests__/run-agent-legacy.test.ts | 63 +++++++++- src/lib/programs/run-agent-legacy.ts | 10 +- src/shared/utils/wizard-abort.ts | 19 ++- 11 files changed, 307 insertions(+), 52 deletions(-) diff --git a/src/__tests__/wizard-abort.test.ts b/src/__tests__/wizard-abort.test.ts index 66111f8b6..923a53ac6 100644 --- a/src/__tests__/wizard-abort.test.ts +++ b/src/__tests__/wizard-abort.test.ts @@ -187,6 +187,35 @@ describe('wizardAbort', () => { expect(process.exit).toHaveBeenCalledWith(1); }); + it('captures an "error" ending that has no Error from its code and message', async () => { + await expect( + wizardAbort({ + message: 'Could not access MCP', + code: ErrorCodes.AgentMcpMissing, + status: 'error', + }), + ).rejects.toThrow('process.exit called'); + + const [captured, properties] = mockAnalytics.captureException.mock + .calls[0] as [WizardError, Record]; + expect(captured).toBeInstanceOf(WizardError); + expect(captured.message).toBe('Could not access MCP'); + expect(captured.code).toBe(ErrorCodes.AgentMcpMissing); + expect(properties).toEqual({ error_code: ErrorCodes.AgentMcpMissing }); + expect(mockAnalytics.shutdown).toHaveBeenCalledWith('error'); + }); + + it('shuts down as the explicit status even when an Error is provided', async () => { + const error = new Error('stopped'); + + await expect(wizardAbort({ error, status: 'cancelled' })).rejects.toThrow( + 'process.exit called', + ); + + expect(mockAnalytics.captureException).toHaveBeenCalledWith(error, {}); + expect(mockAnalytics.shutdown).toHaveBeenCalledWith('cancelled'); + }); + it('shuts down analytics as "cancelled" when no error is provided', async () => { await expect(wizardAbort({ message: 'Bad input' })).rejects.toThrow( 'process.exit called', diff --git a/src/agent/README.md b/src/agent/README.md index 4998ea053..f73939952 100644 --- a/src/agent/README.md +++ b/src/agent/README.md @@ -47,7 +47,12 @@ runAgent(config: RunConfig, input: RunInput, options?: { - Errors: the agent does not exit the process and returns decided failures. A caught coded error becomes `Failed`. An uncoded throw becomes `Crashed` with the error attached. A gateway 401 returns an auth failure. The host decides - whether to show auth UI. + whether to show auth UI. `Aborted` means the host's signal cancelled the run; + an agent that stops itself with `[ABORT]` returns `Failed` with its abort + code. +- Analytics shutdown is host-owned: the agent never sends the terminal + `setup wizard finished` event. The host sends it from the outcome: `Success` + is `success`, `Aborted` is `cancelled`, `Failed` and `Crashed` are `error`. Other runtime exports: `resolveBinding`, `shouldDisableAsk`, `initializeAgent`, `executeAgent`, `buildRunTags`, `AgentSignals`, diff --git a/src/agent/__tests__/run-agent-standalone.test.ts b/src/agent/__tests__/run-agent-standalone.test.ts index 20fe551ca..82e649679 100644 --- a/src/agent/__tests__/run-agent-standalone.test.ts +++ b/src/agent/__tests__/run-agent-standalone.test.ts @@ -18,7 +18,7 @@ import { } from '@lib/wizard-session'; import { AGENT_ERROR_CODE, ErrorCodes, WizardError } from '@shared/errors'; import { AgentErrorType } from '@agent/signals'; -import { CANCELLED_SENTINEL } from '@agent/wizard-ask-bridge'; +import { CANCELLED_SENTINEL, type AskResponse } from '@agent/wizard-ask-bridge'; import type { AgentFailure } from '@agent/runner/shared/types'; import type { AgentProgress } from '@agent/progress'; import type { @@ -81,6 +81,10 @@ const harnessState = vi.hoisted(() => ({ task: undefined as | ((inputs: TaskRunInputs) => Promise) | undefined, + /** Scripts the linear run in place of the default one. */ + run: undefined as + | ((inputs: BackendRunInputs) => Promise) + | undefined, })); vi.mock('@agent/runner/switchboard/harness', () => { const askIfRequested = async (inputs: BackendRunInputs | TaskRunInputs) => { @@ -121,6 +125,7 @@ vi.mock('@agent/runner/switchboard/harness', () => { }, async run(inputs: BackendRunInputs) { harnessState.lastInputs = inputs; + if (harnessState.run) return harnessState.run(inputs); const { emit, spinner } = inputs; emit({ kind: 'log', level: 'step', message: 'Initializing agent' }); spinner.start('Working'); @@ -268,7 +273,9 @@ beforeEach(() => { harnessState.askQuestions = undefined; harnessState.seedTasks = 1; harnessState.task = undefined; + harnessState.run = undefined; vi.mocked(analytics.shutdown).mockClear(); + vi.mocked(analytics.wizardCapture).mockClear(); vi.mocked(initLogFile).mockClear(); vi.mocked(flushScanReport).mockClear(); }); @@ -542,6 +549,112 @@ describe('runAgent standalone', () => { expect(answers).toEqual([{ q1: CANCELLED_SENTINEL }]); }); + it('reports the steps a fatal task result stopped', async () => { + const failure = { + code: ErrorCodes.AgentApiError, + message: 'Gateway rejected the request', + }; + harnessState.task = ({ orchestrator }) => { + if (!orchestrator.currentTaskId) + throw new Error('a drained task has an id'); + // A step that waits on this one, so the fatal result leaves it pending. + orchestrator.store.enqueue({ + type: 'report', + dependsOn: [orchestrator.currentTaskId], + }); + return Promise.resolve({ kind: 'decided_failure', failure }); + }; + const result = await runAgent( + config({ + binding: { + harness: Harness.pi, + sequence: Sequence.orchestrator, + model: DEFAULT_AGENT_MODEL, + }, + }), + input(), + ); + expect(result.outcome).toBe(RunOutcome.Failed); + expect(result.failure).toBe(failure); + expect(analytics.wizardCapture).toHaveBeenCalledWith( + 'orchestrator task blocked', + { type: 'report', optional: false, failed_types: 'install' }, + ); + }); + + it('keeps a fatal task result when reporting its blocked steps throws', async () => { + const failure = { + code: ErrorCodes.AgentApiError, + message: 'Gateway rejected the request', + }; + harnessState.task = ({ orchestrator }) => { + if (!orchestrator.currentTaskId) + throw new Error('a drained task has an id'); + orchestrator.store.enqueue({ + type: 'report', + dependsOn: [orchestrator.currentTaskId], + }); + return Promise.resolve({ kind: 'decided_failure', failure }); + }; + vi.mocked(analytics.wizardCapture).mockImplementation((event) => { + if (event === 'orchestrator task blocked') { + throw new Error('analytics down'); + } + }); + try { + const result = await runAgent( + config({ + binding: { + harness: Harness.pi, + sequence: Sequence.orchestrator, + model: DEFAULT_AGENT_MODEL, + }, + }), + input(), + ); + expect(result.outcome).toBe(RunOutcome.Failed); + expect(result.failure).toBe(failure); + } finally { + vi.mocked(analytics.wizardCapture).mockReset(); + } + }); + + it('cancels an open question when the linear harness ends the run', async () => { + const signals: AbortSignal[] = []; + let response: Promise | undefined; + harnessState.run = (inputs) => { + // A parallel tool call is still waiting on the user when a violation + // ends the run. + if (!inputs.askBridge) throw new Error('the linear run can ask'); + response = inputs.askBridge.request({ + questions: [{ id: 'q1', prompt: 'Key?', kind: 'text' }], + }); + return Promise.resolve({ + kind: 'failure', + classification: AgentErrorType.YARA_VIOLATION, + }); + }; + const result = await runAgent(config(), input(), { + interaction: { + // Nobody answers: only the run ending can close this question. + ask: (_question, { signal }) => { + signals.push(signal); + return new Promise(() => undefined); + }, + }, + }); + expect(result.outcome).toBe(RunOutcome.Failed); + expect(result.failure?.code).toBe( + AGENT_ERROR_CODE[AgentErrorType.YARA_VIOLATION], + ); + expect(signals).toHaveLength(1); + expect(signals[0].aborted).toBe(true); + await expect(response).resolves.toEqual({ + answers: { q1: CANCELLED_SENTINEL }, + timedOut: false, + }); + }); + it.each([Sequence.linear, Sequence.orchestrator])( 'preserves %s completion and scan-flush ordering and leaves the shutdown to the host', async (sequence) => { @@ -696,7 +809,8 @@ describe('runAgent standalone', () => { onProgress: (e) => events.push(e), }); - expect(result.outcome).toBe('aborted'); + // The agent stopped itself; only the host's signal makes a run aborted. + expect(result.outcome).toBe(RunOutcome.Failed); expect(result.failure?.code).toBe(ErrorCodes.AgentAbort); expect(result.failure?.outroData).toMatchObject({ kind: OutroKind.Error, diff --git a/src/agent/runner/README.md b/src/agent/runner/README.md index 7279abc53..8e8cc4388 100644 --- a/src/agent/runner/README.md +++ b/src/agent/runner/README.md @@ -143,5 +143,7 @@ the host to present. 4. Harness drives each conversation through its SDK, using the bound model, on the PostHog LLM gateway. 5. The scan report flushes; `runAgent` returns a `RunResult`. -6. The caller applies it: a decided failure goes to `wizardAbort`, a crash is - rethrown for the runner's own handling. +6. The caller applies it: a decided failure goes to `wizardAbort` with the + terminal status its outcome names, a crash is rethrown for the runner's own + handling, and a non-composed success sends the terminal success analytics. + The agent sends no terminal analytics. diff --git a/src/agent/runner/sequence/linear.ts b/src/agent/runner/sequence/linear.ts index 8d9c68af7..a1da4d7fd 100644 --- a/src/agent/runner/sequence/linear.ts +++ b/src/agent/runner/sequence/linear.ts @@ -27,14 +27,27 @@ import { createEmitSpinner } from '../shared/progress-collector'; import { createAskBridge } from '../shared/ask'; import { getHarness } from '../switchboard'; -export async function runLinearProgram({ - config, - input, - boot, - emit, - interaction, - signal, -}: SequenceContext): Promise { +export async function runLinearProgram( + context: SequenceContext, +): Promise { + // Aborts on the host's signal or when the run ends, so no ask outlives it. + const controller = new AbortController(); + const abortFromHost = () => controller.abort(); + context.signal?.addEventListener('abort', abortFromHost, { once: true }); + if (context.signal?.aborted) abortFromHost(); + try { + return await executeLinear(context, controller.signal); + } finally { + context.signal?.removeEventListener('abort', abortFromHost); + controller.abort(); + } +} + +/** The host's `signal` decides the outcome; `runSignal` also ends with the run. */ +async function executeLinear( + { config, input, boot, emit, interaction, signal }: SequenceContext, + runSignal: AbortSignal, +): Promise { const { run, composed } = config; const { skillsBaseUrl, credentials, project } = boot; const { projectApiKey, host, projectId } = credentials; @@ -79,7 +92,7 @@ export async function runLinearProgram({ getSource: () => input.skillId ?? run.integrationLabel, richLinks: run.richLinks ?? false, timeoutMs: run.askTimeoutMs, - signal, + signal: runSignal, }); const middleware = input.flags.benchmark @@ -121,7 +134,7 @@ export async function runLinearProgram({ middleware, model, thinkingLevel, - signal, + signal: runSignal, }); if (signal?.aborted && agentResult.kind === 'success') return aborted(); @@ -164,7 +177,8 @@ export async function runLinearProgram({ matched: matched?.message ?? null, }); return { - outcome: RunOutcome.Aborted, + // An agent that stops itself failed the run; only the host's signal cancels it. + outcome: signal?.aborted ? RunOutcome.Aborted : RunOutcome.Failed, failure: { message: matched?.message ?? `${run.integrationLabel} aborted`, outroData, diff --git a/src/agent/runner/sequence/orchestrator/executor.ts b/src/agent/runner/sequence/orchestrator/executor.ts index f33baebdc..f9cf187d1 100644 --- a/src/agent/runner/sequence/orchestrator/executor.ts +++ b/src/agent/runner/sequence/orchestrator/executor.ts @@ -46,6 +46,8 @@ export class RunTaskFatal extends Error { public readonly outcome: | RunOutcome.Aborted | RunOutcome.Failed = RunOutcome.Failed, + /** The type of the task that ended the run, for the steps it stopped. */ + public readonly taskType?: string, ) { super(failure.message ?? 'agent run failed'); this.name = 'RunTaskFatal'; diff --git a/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts b/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts index f44be8aba..147a5d843 100644 --- a/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts +++ b/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts @@ -161,20 +161,18 @@ function requireTaskHarness(pick: HarnessPick): AgentHarness & { }; } -function terminalResult(result: AgentResult): - | { - outcome: RunOutcome.Aborted | RunOutcome.Failed; - failure: AgentFailure; - } - | undefined { +function terminalResult( + result: AgentResult, +): { outcome: RunOutcome.Failed; failure: AgentFailure } | undefined { switch (result.kind) { case 'success': return undefined; case 'decided_failure': return { outcome: RunOutcome.Failed, failure: result.failure }; case 'abort': + // Callers return first on the run's signal, so this abort is the agent's own. return { - outcome: RunOutcome.Aborted, + outcome: RunOutcome.Failed, failure: { code: AGENT_ERROR_CODE[result.classification], message: result.message ?? 'Agent aborted', @@ -301,7 +299,6 @@ export async function offerSeededTask( controller.abort(); }; signal?.addEventListener('abort', cancelForAbort, { once: true }); - if (signal?.aborted) cancelForAbort(); }); try { const keep = await Promise.race([ @@ -486,6 +483,25 @@ export function describeDrainFailure(verdict: { return parts.join(', so '); } +/** One `orchestrator task blocked` event per pending task, best effort once the outcome is decided. */ +function reportBlockedTasks( + tasks: readonly QueuedTask[], + failedTypes: readonly string[], +): void { + for (const task of tasks) { + if (task.status !== TaskStatus.Pending) continue; + try { + analytics.wizardCapture('orchestrator task blocked', { + type: task.type, + optional: task.optional === true, + failed_types: failedTypes.join(',') || 'none', + }); + } catch { + // Reporting must not replace the run result. + } + } +} + /** How many tasks deep in the graph a task sits — 0 when it depends on nothing. */ function graphDepth( task: QueuedTask, @@ -1208,15 +1224,20 @@ async function executeOrchestrator( if (signal?.aborted) return; if (error instanceof RunTaskFatal) throw error; const failure = classifyRunFailure(error); - throw new RunTaskFatal({ - code: failure.code, - message: failure.message, - error: error instanceof Error ? error : undefined, - }); + throw new RunTaskFatal( + { + code: failure.code, + message: failure.message, + error: error instanceof Error ? error : undefined, + }, + RunOutcome.Failed, + task.type, + ); } if (signal?.aborted) return; const terminal = terminalResult(taskResult); - if (terminal) throw new RunTaskFatal(terminal.failure, terminal.outcome); + if (terminal) + throw new RunTaskFatal(terminal.failure, terminal.outcome, task.type); } finally { // Durable skills a task installed are irrelevant to later tasks — and // the sdk harness auto-loads .claude/skills into every agent — so sweep @@ -1300,7 +1321,15 @@ async function executeOrchestrator( } } - if (fatal) return { outcome: fatal.outcome, failure: fatal.failure }; + if (fatal) { + // The steps the fatal task stopped still get their terminal event. + const stoppedBy = drainVerdict(store.list()).requiredFailedTypes; + if (fatal.taskType && !stoppedBy.includes(fatal.taskType)) { + stoppedBy.push(fatal.taskType); + } + reportBlockedTasks(store.list(), stoppedBy); + return { outcome: fatal.outcome, failure: fatal.failure }; + } if (signal?.aborted) return cancelledRun(); renderQueue(); @@ -1345,14 +1374,7 @@ async function executeOrchestrator( // 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', - }); - } + reportBlockedTasks(store.list(), verdict.requiredFailedTypes); if (verdict.requiredFailedTypes.length > 0 || blocked > 0) { const failedTypes = verdict.requiredFailedTypes.join(', '); const whatFailed = describeDrainFailure(verdict); diff --git a/src/agent/wizard-ask-bridge.ts b/src/agent/wizard-ask-bridge.ts index bfc77cd86..5c57a1b79 100644 --- a/src/agent/wizard-ask-bridge.ts +++ b/src/agent/wizard-ask-bridge.ts @@ -163,7 +163,6 @@ export function createWizardAskBridge( controller.abort(); }; opts.signal?.addEventListener('abort', cancelForAbort, { once: true }); - if (opts.signal?.aborted) cancelForAbort(); }); try { diff --git a/src/lib/programs/__tests__/run-agent-legacy.test.ts b/src/lib/programs/__tests__/run-agent-legacy.test.ts index 323771f22..73b40bf90 100644 --- a/src/lib/programs/__tests__/run-agent-legacy.test.ts +++ b/src/lib/programs/__tests__/run-agent-legacy.test.ts @@ -44,6 +44,7 @@ vi.mock('@utils/analytics', () => ({ build: 'test', runId: 'run-1', wizardCapture: vi.fn(), + captureException: vi.fn(), setTag: vi.fn(), getAllFlagsForWizard: vi.fn().mockResolvedValue({}), getWizardFlagPayloads: vi.fn().mockReturnValue({}), @@ -201,9 +202,12 @@ it('clamps a composed program to linear and keeps host analytics alive', async ( expect(analytics.shutdown).toHaveBeenCalledExactlyOnceWith('success'); }); -it.each([RunOutcome.Aborted, RunOutcome.Failed] as const)( - 'passes a %s result to the existing abort handler', - async (outcome) => { +it.each([ + [RunOutcome.Aborted, 'cancelled'], + [RunOutcome.Failed, 'error'], +] as const)( + 'passes a %s result to the existing abort handler as %s', + async (outcome, status) => { const failure = { code: ErrorCodes.AgentApiError, message: 'Failed', @@ -211,11 +215,62 @@ it.each([RunOutcome.Aborted, RunOutcome.Failed] as const)( }; vi.mocked(runAgent).mockResolvedValue({ outcome, failure, snapshot }); await runProgramAgent(program(), session()); - expect(wizardAbort).toHaveBeenCalledExactlyOnceWith(failure); + expect(wizardAbort).toHaveBeenCalledExactlyOnceWith({ ...failure, status }); expect(analytics.shutdown).not.toHaveBeenCalled(); }, ); +it.each([ + [ + RunOutcome.Failed, + 'error', + { code: ErrorCodes.AgentMcpMissing, message: 'Could not access MCP' }, + ], + [ + RunOutcome.Aborted, + 'cancelled', + { code: ErrorCodes.AgentAbort, message: 'Agent run cancelled' }, + ], +] as const)( + 'labels a %s run %s from its outcome when no Error came back', + async (outcome, status, failure) => { + const actual = await vi.importActual( + '@utils/wizard-abort', + ); + vi.mocked(wizardAbort).mockImplementationOnce(actual.wizardAbort); + const exit = vi + .spyOn(process, 'exit') + .mockImplementation(() => undefined as never); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + vi.mocked(runAgent).mockResolvedValue({ + outcome, + failure: { ...failure }, + snapshot, + }); + try { + await runProgramAgent(program(), session()); + } finally { + exit.mockRestore(); + stderr.mockRestore(); + } + expect(analytics.shutdown).toHaveBeenCalledExactlyOnceWith(status); + if (status === 'error') { + // Error tracking still sees the failure, as its code and message. + expect(analytics.captureException).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ + message: failure.message, + code: failure.code, + }), + { error_code: failure.code }, + ); + } else { + expect(analytics.captureException).not.toHaveBeenCalled(); + } + }, +); + it('shows the auth guidance from a decided 401 before the error outro', async () => { const detail = { hasSettingsConflict: false, logFilePath: '/tmp/wizard.log' }; const show = vi.spyOn(getUI(), 'showAuthError'); diff --git a/src/lib/programs/run-agent-legacy.ts b/src/lib/programs/run-agent-legacy.ts index fc2451319..4b74621cc 100644 --- a/src/lib/programs/run-agent-legacy.ts +++ b/src/lib/programs/run-agent-legacy.ts @@ -7,8 +7,8 @@ * authenticates, resolves the program's binding, builds the agent's inputs * from the session, maps every progress event back onto `getUI()` one call * per event, answers the agent's questions through `getUI()`, and applies the - * result — `wizardAbort` for a decided failure, the terminal analytics event - * for a finished top-level run. + * result — `wizardAbort` with the outcome's terminal status for a decided + * failure, the terminal analytics event for a finished top-level run. * * This is the only file that knows about `getUI()`, the session and * `wizardAbort` on the agent's behalf. Programs replace it in Release B. @@ -286,7 +286,11 @@ async function runProgram( if (result.failure.authErrorDetail) { ui.showAuthError(result.failure.authErrorDetail); } - await wizardAbort(result.failure); + // The terminal status follows how the run ended, not whether an Error came back. + await wizardAbort({ + ...result.failure, + status: result.outcome === RunOutcome.Aborted ? 'cancelled' : 'error', + }); } else if (!composed) { // A composed sub-run leaves the terminal event to its host program's run. await analytics.shutdown('success'); diff --git a/src/shared/utils/wizard-abort.ts b/src/shared/utils/wizard-abort.ts index 5b55b6864..0a2b8ae30 100644 --- a/src/shared/utils/wizard-abort.ts +++ b/src/shared/utils/wizard-abort.ts @@ -29,6 +29,8 @@ interface WizardAbortOptions { exitCode?: number; code?: ErrorCode; detail?: Record; + /** Terminal analytics status. Defaults from whether `error` is set. */ + status?: 'error' | 'cancelled'; } const cleanupFns: Array<() => void> = []; @@ -87,16 +89,23 @@ export async function wizardAbort( // 1. Run registered cleanup functions runCleanups(); - // 2. Capture error in analytics (if provided) - if (error) { - analytics.captureException(error, { - ...((error instanceof WizardError && error.context) || {}), + // 2. Capture error in analytics. An 'error' ending with no Error object + // is captured as its code and message. + const status = options?.status ?? (error ? 'error' : 'cancelled'); + const captured = + error ?? + (status === 'error' + ? new WizardError(message, undefined, code) + : undefined); + if (captured) { + analytics.captureException(captured, { + ...((captured instanceof WizardError && captured.context) || {}), ...(code ? { error_code: code } : {}), }); } // 3. Shutdown analytics - await analytics.shutdown(error ? 'error' : 'cancelled'); + await analytics.shutdown(status); // 4. Render the error outro. Synthesize OutroData from `message` // when the caller didn't provide structured data. From 38f57e8024d67d95fae121a5c8984a60e09c0d70 Mon Sep 17 00:00:00 2001 From: "Vincent (Wen Yu) Ge" Date: Wed, 23 Sep 2026 13:18:29 -0400 Subject: [PATCH 12/12] fix(agent): report the scan summary when a run is cancelled before it starts runAgent's pre-aborted return flushed the scan report but dropped the summary line it returned, so the report file could be written with no line in the terminal. Every termination path now flushes through one helper that emits the line as a log event. The scan-summary test table gains the two host-cancel cases, mid-run and before start; the second failed before this change. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589 --- .../__tests__/run-agent-standalone.test.ts | 40 +++++++++++++++++-- src/agent/runner/index.ts | 23 +++++------ 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/agent/__tests__/run-agent-standalone.test.ts b/src/agent/__tests__/run-agent-standalone.test.ts index 160885c51..a6ac6444d 100644 --- a/src/agent/__tests__/run-agent-standalone.test.ts +++ b/src/agent/__tests__/run-agent-standalone.test.ts @@ -208,7 +208,7 @@ vi.mock('@shared/skill-menu', async (original) => ({ })); import { runAgent, RunOutcome } from '@agent/runner'; -import type { RunConfig, RunInput } from '@agent/runner'; +import type { RunAgentOptions, RunConfig, RunInput } from '@agent/runner'; import { analytics } from '@utils/analytics'; import { initLogFile } from '@utils/debug'; import { flushScanReport } from '@agent/yara-hooks'; @@ -1029,8 +1029,10 @@ describe('runAgent standalone', () => { } }); - it.each<[string, () => void, string]>([ - ['completes', () => undefined, 'success'], + it.each< + [string, (host: AbortController) => Partial, string] + >([ + ['completes', () => ({}), 'success'], [ 'stops itself', () => { @@ -1039,6 +1041,7 @@ describe('runAgent standalone', () => { classification: AgentErrorType.ABORT, message: 'No Stripe found', }; + return {}; }, 'failed', ], @@ -1046,21 +1049,50 @@ describe('runAgent standalone', () => { 'crashes', () => { harnessState.throws = new Error('SDK exploded'); + return {}; }, 'crashed', ], + [ + 'is cancelled mid-run', + (host) => { + harnessState.askQuestions = [ + { id: 'q1', prompt: 'Continue?', kind: 'text' }, + ]; + return { + interaction: { + ask: () => { + host.abort(); + return new Promise(() => undefined); + }, + }, + }; + }, + 'aborted', + ], + [ + 'is cancelled before it starts', + (host) => { + host.abort(); + return {}; + }, + 'aborted', + ], ])( 'sends the scan summary to onProgress when the run %s', async (_ending, arrange, outcome) => { const summary = 'YARA scan report: /tmp/yara.json\n— YARA Scanner Summary —'; vi.mocked(flushScanReport).mockReturnValueOnce(summary); - arrange(); + const host = new AbortController(); + const options = arrange(host); const runInput = input(); runInput.flags.yaraReport = true; const events: AgentProgress[] = []; const result = await runAgent(config(), runInput, { + ...options, + signal: host.signal, onProgress: (e) => events.push(e), }); diff --git a/src/agent/runner/index.ts b/src/agent/runner/index.ts index cd98f7725..8f2e92330 100644 --- a/src/agent/runner/index.ts +++ b/src/agent/runner/index.ts @@ -95,6 +95,15 @@ export async function runAgent( }, }; }; + const flushReport = (): void => { + try { + const report = flushScanReport({ yaraReport: input.flags.yaraReport }); + if (report) + collector?.emit({ kind: 'log', level: 'info', message: report }); + } catch { + // Scan reporting is best effort after the run outcome is decided. + } + }; let result: RunResult; try { collector = createProgressCollector(options.onProgress); @@ -102,11 +111,7 @@ export async function runAgent( const log = (message: string) => emit({ kind: 'log', level: 'info', message }); if (options.signal?.aborted) { - try { - flushScanReport({ yaraReport: input.flags.yaraReport }); - } catch { - // Scan reporting is best effort. - } + flushReport(); return { outcome: RunOutcome.Aborted, skillId: input.skillId, @@ -209,13 +214,7 @@ export async function runAgent( }; } } - try { - const report = flushScanReport({ yaraReport: input.flags.yaraReport }); - if (report) - collector?.emit({ kind: 'log', level: 'info', message: report }); - } catch { - // Scan reporting is best effort after the run outcome is decided. - } + flushReport(); return result; }