diff --git a/.eslintrc.cjs b/.eslintrc.cjs index a73cf66eb..e6eb9c9e1 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -100,6 +100,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/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/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/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/e2e-harness/__tests__/e2e-result.test.ts b/e2e-harness/__tests__/e2e-result.test.ts index 89506dc87..961c1dda5 100644 --- a/e2e-harness/__tests__/e2e-result.test.ts +++ b/e2e-harness/__tests__/e2e-result.test.ts @@ -15,7 +15,7 @@ import { OutroKind, RunPhase } from '@lib/wizard-session'; import type { AskQuestion, WizardSession } from '@lib/wizard-session'; import { DETECTED_WAREHOUSE_SOURCES_KEY } from '@lib/programs/warehouse-source/detect'; import { Overlay } from '@ui/tui/router'; -import { TASK_OUTCOMES_KEY } from '@agent/runner/sequence/orchestrator/queue'; +import { TASK_OUTCOMES_KEY } from '@agent'; import { E2eRunRecorder, abortReasonFrom, diff --git a/e2e-harness/e2e-result.ts b/e2e-harness/e2e-result.ts index 6fc6a668a..1ba4b9d88 100644 --- a/e2e-harness/e2e-result.ts +++ b/e2e-harness/e2e-result.ts @@ -22,10 +22,8 @@ import fs from 'fs'; import path from 'path'; import { OutroKind, type WizardSession } from '@lib/wizard-session'; -import { - TASK_OUTCOMES_KEY, - type TaskOutcome, -} from '@agent/runner/sequence/orchestrator/queue'; +import { TASK_OUTCOMES_KEY } from '@agent'; +import type { TaskOutcome } from '@agent/types'; import { DETECTED_WAREHOUSE_SOURCES_KEY } from '@lib/programs/warehouse-source/detect'; import type { DetectedSource } from '@lib/warehouse-sources/types'; import type { E2eDecisionReport } from './e2e-profile.js'; 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; +} diff --git a/src/__tests__/architecture/import-boundaries.test.ts b/src/__tests__/architecture/import-boundaries.test.ts index dba4024a4..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,12 +42,36 @@ 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 +// 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 +291,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 +316,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); } } @@ -375,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'); @@ -392,3 +416,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 1f0f7bd21..629085454 100644 --- a/src/__tests__/architecture/known-violations.json +++ b/src/__tests__/architecture/known-violations.json @@ -2,73 +2,41 @@ "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/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/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" + "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__/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__/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/__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/__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 new file mode 100644 index 000000000..f73939952 --- /dev/null +++ b/src/agent/README.md @@ -0,0 +1,117 @@ +# 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; + signal?: AbortSignal; +}): 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, { signal })` + resolves with answers, and `taskNotice(notice, { signal })` resolves with + whether to keep an optional task. Each request has its own signal, which + aborts when that request times out, the host aborts the run, or another task + fails the run; on abort the host dismisses that request alone, without + throwing. +- 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. `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`, +`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/__tests__/agent-interface.test.ts b/src/agent/__tests__/agent-interface.test.ts index c14f03c77..c096578e6 100644 --- a/src/agent/__tests__/agent-interface.test.ts +++ b/src/agent/__tests__/agent-interface.test.ts @@ -147,7 +147,8 @@ describe('runAgent', () => { expect(controller?.signal.aborted).toBe(true); expect(await pending).toEqual({ - error: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, + kind: 'failure', + classification: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, message: 'Agent run timed out after 60s', }); expect(mockSpinner.stop).toHaveBeenCalledWith('Agent run timed out'); @@ -159,6 +160,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 @@ -201,7 +329,7 @@ describe('runAgent', () => { ); // Should return success (empty object), not throw - expect(result).toEqual({}); + expect(result).toEqual({ kind: 'success' }); expect(mockSpinner.stop).toHaveBeenCalledWith('Test success'); }); @@ -276,8 +404,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 () => { @@ -341,7 +474,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) @@ -398,7 +531,7 @@ describe('runAgent', () => { }, ); - expect(result).toEqual({}); + expect(result).toEqual({ kind: 'success' }); expect(mockSpinner.stop).toHaveBeenCalledWith('Test success'); expect(mockUIInstance.log.error).not.toHaveBeenCalled(); }); @@ -438,7 +571,7 @@ describe('runAgent', () => { }, ); - expect(result).toEqual({}); + expect(result).toEqual({ kind: 'success' }); expect(mockSpinner.stop).toHaveBeenCalledWith('Test success'); }); }); @@ -784,6 +917,7 @@ describe('gateway re-mint on 401', () => { yield { type: 'assistant', session_id: id, + error: { status: 401 }, message: { role: 'assistant', content: [ @@ -835,7 +969,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); @@ -876,12 +1010,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 () => { @@ -915,9 +1053,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); }); @@ -932,8 +1073,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__/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', + ); + }); +}); 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/agent/__tests__/run-agent-standalone.test.ts b/src/agent/__tests__/run-agent-standalone.test.ts index cf4a8f058..a6ac6444d 100644 --- a/src/agent/__tests__/run-agent-standalone.test.ts +++ b/src/agent/__tests__/run-agent-standalone.test.ts @@ -16,11 +16,13 @@ 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 { 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 { + AgentResult, AgentHarness, BackendRunInputs, TaskRunInputs, @@ -64,14 +66,25 @@ 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, + /** How many install tasks the seed plans. */ + seedTasks: 1, + /** Scripts each drained task in place of the default install. */ + 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) => { @@ -91,10 +104,15 @@ vi.mock('@agent/runner/switchboard/harness', () => { const { store, currentTaskId } = inputs.orchestrator; if (!currentTaskId) { if (harnessState.seedFailure) - return Promise.resolve({ failure: harnessState.seedFailure }); - store.enqueue({ type: 'install' }); + return { kind: 'decided_failure', failure: harnessState.seedFailure }; + for (let i = 0; i < harnessState.seedTasks; i++) + store.enqueue({ type: 'install' }); + } else if (harnessState.task) { + return harnessState.task(inputs); + } 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,10 +121,11 @@ vi.mock('@agent/runner/switchboard/harness', () => { forNextAgent: 'done', }); } - return Promise.resolve({}); + return { kind: 'success' }; }, 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'); @@ -142,7 +161,7 @@ vi.mock('@agent/runner/switchboard/harness', () => { ); if (harnessState.throws) throw harnessState.throws; spinner.stop('Done'); - return harnessState.result as never; + return harnessState.result; }, }; return { @@ -183,13 +202,13 @@ 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: {} }), })); 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'; @@ -252,15 +271,20 @@ 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; 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(); }); @@ -390,7 +414,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({ @@ -440,6 +467,203 @@ 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('cancels a sibling task’s open question when another task fails the run', async () => { + const failure = { + code: ErrorCodes.AgentApiError, + message: 'Gateway rejected the request', + }; + const signals: AbortSignal[] = []; + const answers: AskAnswers[] = []; + let questionOpened!: () => void; + const opened = new Promise((resolve) => { + questionOpened = resolve; + }); + harnessState.seedTasks = 2; + let started = 0; + harnessState.task = async (inputs) => { + if (started++ === 0) { + if (!inputs.askBridge) throw new Error('the install task can ask'); + const response = await inputs.askBridge.request({ + questions: [{ id: 'q1', prompt: 'Key?', kind: 'text' }], + }); + answers.push(response.answers); + return { kind: 'success' }; + } + await opened; + return { kind: 'decided_failure', failure }; + }; + 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(), + { + interaction: { + // Nobody answers: only the sibling's failure can end this question. + ask: (_question, { signal }) => { + signals.push(signal); + questionOpened(); + return new Promise(() => undefined); + }, + }, + }, + ); + expect(result.outcome).toBe(RunOutcome.Failed); + expect(result.failure).toMatchObject(failure); + // The question's own signal aborted, so the host dismisses its overlay, + // and the ask settled as cancelled, so its task joined the drain. + expect(signals).toHaveLength(1); + expect(signals[0].aborted).toBe(true); + 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) => { @@ -584,7 +808,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[] = []; @@ -593,7 +818,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, @@ -601,15 +827,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()); @@ -620,9 +847,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()); @@ -657,6 +898,67 @@ 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.each([Sequence.linear, Sequence.orchestrator])( + 'cancels an open question when the host aborts the %s run', + async (sequence) => { + harnessState.askQuestions = [ + { id: 'q1', prompt: 'Continue?', kind: 'text' }, + ]; + const host = new AbortController(); + const signals: AbortSignal[] = []; + const running = runAgent( + config({ + binding: { + harness: Harness.pi, + sequence, + model: DEFAULT_AGENT_MODEL, + }, + switchboard: { + program: 'test-program', + flags: {}, + cliHarness: Harness.pi, + }, + }), + input(), + { + signal: host.signal, + interaction: { + ask: (_question, { signal }) => { + signals.push(signal); + return new Promise(() => undefined); + }, + }, + }, + ); + await vi.waitFor(() => expect(signals).toHaveLength(1)); + expect(signals[0].aborted).toBe(false); + host.abort(); + const result = await running; + expect(result.outcome).toBe(RunOutcome.Aborted); + // The host's abort reached the open question as its own abort. + expect(signals[0].aborted).toBe(true); + }, + ); + + 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(() => ({ @@ -727,37 +1029,70 @@ describe('runAgent standalone', () => { } }); - it.each<[string, () => void, string]>([ - ['completes', () => undefined, 'success'], + it.each< + [string, (host: AbortController) => Partial, string] + >([ + ['completes', () => ({}), 'success'], [ - 'aborts', + 'stops itself', () => { harnessState.result = { - error: AgentErrorType.ABORT, + kind: 'abort', + classification: AgentErrorType.ABORT, message: 'No Stripe found', }; + return {}; }, - 'aborted', + 'failed', ], [ '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/__tests__/wizard-ask-bridge.test.ts b/src/agent/__tests__/wizard-ask-bridge.test.ts index bd99ad0c4..4f636fb7e 100644 --- a/src/agent/__tests__/wizard-ask-bridge.test.ts +++ b/src/agent/__tests__/wizard-ask-bridge.test.ts @@ -19,6 +19,122 @@ beforeEach(() => { }); describe('createWizardAskBridge', () => { + it('dismisses and settles an active question on run cancellation', async () => { + const controller = new AbortController(); + const signals: AbortSignal[] = []; + const bridge = createWizardAskBridge({ + signal: controller.signal, + getSource: () => 'skill', + showQuestion: (_question, { signal }) => { + signals.push(signal); + return new Promise(() => undefined); + }, + }); + const result = bridge.request({ + questions: [{ id: 'goal', prompt: 'Goal?', kind: 'text' }], + }); + expect(bridge.getPendingQuestion()).not.toBeNull(); + expect(signals[0].aborted).toBe(false); + controller.abort(); + await expect(result).resolves.toEqual({ + answers: { goal: CANCELLED_SENTINEL }, + timedOut: false, + }); + // The run's abort reaches the host as this question's own abort, so the + // host dismisses the overlay it opened. + expect(signals[0].aborted).toBe(true); + expect(bridge.getPendingQuestion()).toBeNull(); + }); + + it('aborts every open question when the run is cancelled', async () => { + const controller = new AbortController(); + const signals: AbortSignal[] = []; + const bridge = createWizardAskBridge({ + signal: controller.signal, + getSource: () => 'skill', + showQuestion: (_question, { signal }) => { + signals.push(signal); + return new Promise(() => undefined); + }, + }); + const questions = [{ id: 'goal', prompt: 'Goal?', kind: 'text' as const }]; + const first = bridge.request({ questions }); + const second = bridge.request({ questions }); + controller.abort(); + await expect(Promise.all([first, second])).resolves.toEqual([ + { answers: { goal: CANCELLED_SENTINEL }, timedOut: false }, + { answers: { goal: CANCELLED_SENTINEL }, timedOut: false }, + ]); + expect(signals.map((signal) => signal.aborted)).toEqual([true, true]); + }); + + it('leaves an answered question alone when the run is cancelled later', async () => { + const controller = new AbortController(); + const signals: AbortSignal[] = []; + const bridge = createWizardAskBridge({ + signal: controller.signal, + getSource: () => 'skill', + showQuestion: (_question, { signal }) => { + signals.push(signal); + return Promise.resolve({ goal: 'ship it' }); + }, + }); + await expect( + bridge.request({ + questions: [{ id: 'goal', prompt: 'Goal?', kind: 'text' }], + }), + ).resolves.toEqual({ answers: { goal: 'ship it' }, timedOut: false }); + controller.abort(); + // A late abort must not dismiss whatever the host shows next. + expect(signals[0].aborted).toBe(false); + }); + + it('settles a cancelled question when the host rejects on dismissal', async () => { + const controller = new AbortController(); + const bridge = createWizardAskBridge({ + signal: controller.signal, + getSource: () => 'skill', + showQuestion: (_question, { signal }) => + new Promise((_resolve, reject) => { + signal.addEventListener( + 'abort', + () => reject(new Error('overlay broken')), + { once: true }, + ); + }), + }); + const result = bridge.request({ + questions: [{ id: 'goal', prompt: 'Goal?', kind: 'text' }], + }); + controller.abort(); + await expect(result).resolves.toEqual({ + answers: { goal: CANCELLED_SENTINEL }, + timedOut: false, + }); + 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; @@ -258,6 +374,38 @@ describe('createWizardAskBridge', () => { } }); + it('settles a timed-out question when the host rejects on dismissal', async () => { + vi.useFakeTimers(); + try { + // The host's dismissal is its own: an abort listener's throw reaches + // no caller. What reaches the bridge is the host's promise, which may + // reject once its overlay is torn down; the timeout has already won. + const bridge = createWizardAskBridge({ + getSource: () => 'product-tours', + showQuestion: (_question, { signal }) => + new Promise((_resolve, reject) => { + signal.addEventListener( + 'abort', + () => reject(new Error('overlay broken')), + { once: true }, + ); + }), + 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 abort the question when the user answers before the timeout', async () => { vi.useFakeTimers(); try { diff --git a/src/agent/__tests__/wizard-tools.test.ts b/src/agent/__tests__/wizard-tools.test.ts index d087cabbc..1ce282533 100644 --- a/src/agent/__tests__/wizard-tools.test.ts +++ b/src/agent/__tests__/wizard-tools.test.ts @@ -24,7 +24,6 @@ import { ensureGitignoreCoverage, describeAskCancellation, evaluateAskCap, - fetchSkillMenu, mergeEnvValues, normaliseAskSubject, parseEnvKeys, @@ -1463,47 +1462,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/agent-interface.ts b/src/agent/agent-interface.ts index 610608d34..5e6641fc0 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, @@ -754,11 +802,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...', @@ -785,6 +836,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 @@ -816,9 +869,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); @@ -881,13 +932,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 timedOut = false; let abortReason: string | null = null; // Set when a YARA hook detects a terminal violation. Returning `stopReason` @@ -903,6 +958,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(); const timeoutMs = config?.timeoutMs; const timeoutId = timeoutMs ? setTimeout(() => { @@ -1201,6 +1260,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 — @@ -1232,12 +1295,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', @@ -1273,9 +1348,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, @@ -1283,25 +1361,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; @@ -1323,6 +1395,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(); } @@ -1337,17 +1417,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, @@ -1365,27 +1456,47 @@ 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', + }; } if (timedOut) { spinner.stop('Agent run timed out'); return { - error: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, + kind: 'failure', + classification: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, message: `Agent run timed out after ${timeoutMs! / 1000}s`, }; } @@ -1394,13 +1505,16 @@ export async function runAgent( 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 @@ -1413,6 +1527,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'; @@ -1420,16 +1539,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(); @@ -1440,14 +1572,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 @@ -1461,7 +1606,8 @@ export async function runAgent( if (timedOut) { spinner.stop('Agent run timed out'); return { - error: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, + kind: 'failure', + classification: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, message: `Agent run timed out after ${timeoutMs! / 1000}s`, }; } @@ -1473,13 +1619,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 @@ -1493,18 +1649,23 @@ export async function runAgent( debug('Full error:', error); throw error; } finally { + agentConfig.signal?.removeEventListener('abort', onExternalAbort); if (timeoutId) clearTimeout(timeoutId); // 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/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 new file mode 100644 index 000000000..a6446cd3e --- /dev/null +++ b/src/agent/index.ts @@ -0,0 +1,66 @@ +/** + * 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. + * + * 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, and AgentErrorType, which + * classifies executeAgent's failures, goes with them. + * 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 { + AgentErrorType, + buildRunTags, + initializeAgent, + runAgent as executeAgent, +} from './agent-interface'; +export { configureGatewayFromCIEnvironment } from './gateway-session'; +export { flushScanReport } from './yara-hooks'; +export { downloadSkill } from './tools'; +/** The frameworkContext slot the legacy adapter fills for the e2e harness. */ +export { TASK_OUTCOMES_KEY } from './runner'; + +/** + * 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< + typeof import('./mcp-prompt-streaming').runMcpPromptViaSdk + >[0], +): AsyncIterable { + const streaming = await import('./mcp-prompt-streaming'); + yield* streaming.runMcpPromptViaSdk(args); +} diff --git a/src/agent/progress.ts b/src/agent/progress.ts index 7fc3e027c..83e4c4551 100644 --- a/src/agent/progress.ts +++ b/src/agent/progress.ts @@ -241,9 +241,11 @@ export type ProgressEmitter = (event: AgentProgress) => void; * capability is optional. With none supplied the agent installs no ask bridge, * so `wizard_ask` returns its existing "not available" error, and an optional * task notice is declined — the same path a `--ci` run takes today. - * On a request's `signal` abort, the host dismisses that request alone, and - * that dismissal must not throw: abort listeners run where the agent cannot - * catch them, so Node would rethrow the error as an uncaught exception. + * Each request's `signal` aborts when that request times out, the run's + * signal aborts, or another task fails the run. On that abort the host + * dismisses that request alone, and that dismissal must not throw: abort + * listeners run where the agent cannot catch them, so Node would rethrow the + * error as an uncaught exception. */ export interface AgentInteraction { /** diff --git a/src/agent/runner/README.md b/src/agent/runner/README.md index 5f43656cb..8e8cc4388 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 @@ -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 @@ -96,5 +143,7 @@ Each layer is replaceable. 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/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/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/pi/tools.ts b/src/agent/runner/harness/pi/tools.ts index 420f7e96f..bab2dafd3 100644 --- a/src/agent/runner/harness/pi/tools.ts +++ b/src/agent/runner/harness/pi/tools.ts @@ -39,7 +39,6 @@ import { createAskAccounting, describeAskCancellation, ensureGitignoreCoverage, - fetchSkillMenu, installSkillById, mergeEnvValues, normaliseAskSubject, @@ -54,6 +53,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/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 93de08427..666d94a19 100644 --- a/src/agent/runner/index.ts +++ b/src/agent/runner/index.ts @@ -26,7 +26,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, @@ -65,6 +65,7 @@ export { shouldDisableAsk } from './shared/bootstrap'; export { resolveBinding } from './switchboard'; export type { ProgramBinding, SwitchboardCtx } from './switchboard'; export { TASK_OUTCOMES_KEY } from './sequence/orchestrator/queue'; +export type { TaskOutcome } from './sequence/orchestrator/queue'; /** * Run a program's agent pipeline. @@ -78,54 +79,151 @@ 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, + }, + }; + }; + 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); + const { emit } = collector; + const log = (message: string) => + emit({ kind: 'log', level: 'info', message }); + if (options.signal?.aborted) { + flushReport(); + 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 report = flushScanReport({ yaraReport: input.flags.yaraReport }); - if (report) log(report); + 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(), + }; + } + } + flushReport(); + return result; +} + +function safeErrorMessage(error: unknown): string { + try { + return String(error); + } catch { + return 'Unknown thrown value'; } } 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/agent/runner/sequence/linear.ts b/src/agent/runner/sequence/linear.ts index 7af34d37f..a1da4d7fd 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'; @@ -27,16 +27,35 @@ import { createEmitSpinner } from '../shared/progress-collector'; import { createAskBridge } from '../shared/ask'; import { getHarness } from '../switchboard'; -export async function runLinearProgram({ - config, - input, - boot, - emit, - interaction, -}: 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; + 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 +67,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 +92,7 @@ export async function runLinearProgram({ getSource: () => input.skillId ?? run.integrationLabel, richLinks: run.richLinks ?? false, timeoutMs: run.askTimeoutMs, + signal: runSignal, }); const middleware = input.flags.benchmark @@ -95,6 +116,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 +134,23 @@ export async function runLinearProgram({ middleware, model, thinkingLevel, + signal: runSignal, }); + 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; @@ -146,24 +177,23 @@ 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, 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 +201,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 +234,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 +248,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 0f36fb4ad..695991b02 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 @@ -85,6 +85,71 @@ 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, + }); + expect(noticeSignal().aborted).toBe(false); + controller.abort(); + await expect(result).resolves.toEqual({ keep: false, timedOut: false }); + // The run's abort reaches the host as this notice's own abort. + expect(noticeSignal().aborted).toBe(true); + }); + + it('declines without showing the notice when the run was already cancelled', async () => { + const controller = new AbortController(); + controller.abort(); + await expect( + offerSeededTask(NOTICE, { interaction, signal: controller.signal }), + ).resolves.toEqual({ keep: false, timedOut: false }); + expect(showTaskNotice).not.toHaveBeenCalled(); + }); + + it('declines a timed-out notice when the host rejects on dismissal', async () => { + vi.useFakeTimers(); + try { + showTaskNotice.mockImplementation( + (_notice, { signal }) => + new Promise((_resolve, reject) => { + signal.addEventListener( + 'abort', + () => reject(new Error('overlay broken')), + { once: true }, + ); + }), + ); + const promise = offerSeededTask(NOTICE, { timeoutMs: 1000, interaction }); + vi.advanceTimersByTime(1000); + // The timeout won; the host's rejection must not turn it into an error. + await expect(promise).resolves.toEqual({ keep: false, timedOut: true }); + } finally { + vi.useRealTimers(); + } + }); + + it('declines a cancelled notice when the host rejects on dismissal', async () => { + const controller = new AbortController(); + showTaskNotice.mockImplementation( + (_notice, { signal }) => + new Promise((_resolve, reject) => { + signal.addEventListener( + 'abort', + () => reject(new Error('overlay broken')), + { once: true }, + ); + }), + ); + const result = offerSeededTask(NOTICE, { + interaction, + signal: controller.signal, + }); + controller.abort(); + await expect(result).resolves.toEqual({ keep: false, timedOut: false }); + }); + it('keeps the step when the user accepts in time', async () => { vi.useFakeTimers(); try { 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/executor.ts b/src/agent/runner/sequence/orchestrator/executor.ts index e70e5723c..f9cf187d1 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,14 @@ 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, + /** 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'; } @@ -50,6 +57,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 +69,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 +87,8 @@ async function runOne( ); } + if (signal?.aborted) return; + const after = store.get(task.id); if (!after) return; @@ -116,12 +130,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 +163,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 fb3efca83..e3cfd6f83 100644 --- a/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts +++ b/src/agent/runner/sequence/orchestrator/orchestrator-runner.ts @@ -24,16 +24,19 @@ 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'; 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, @@ -60,7 +63,12 @@ import { type QueuedTask, type TaskOutcome, } 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'; @@ -155,6 +163,43 @@ function requireTaskHarness(pick: HarnessPick): AgentHarness & { }; } +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.Failed, + 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, @@ -210,6 +255,7 @@ export const TASK_NOTICE_TIMEOUT_MS = 5 * 60 * 1000; interface SeededTaskOptions { timeoutMs?: number; interaction?: AgentInteraction; + signal?: AbortSignal; } /** @@ -222,33 +268,50 @@ 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 }; const { taskNotice } = interaction; + // This notice's own signal: its timeout or the run's cancellation aborts it. const controller = new AbortController(); let timer: ReturnType | undefined; let timedOut = false; + let cancelForAbort: (() => void) | undefined; const timeout = new Promise((resolve) => { timer = setTimeout(() => { timedOut = true; + // Settle first: a host that rejects once dismissed must not win. + resolve(false); // The host dismisses this notice's overlay and settles its promise too, // so the losing side of the race cannot leave a modal on screen. controller.abort(); - resolve(false); }, timeoutMs); }); + const aborted = new Promise((resolve) => { + cancelForAbort = () => { + resolve(false); + controller.abort(); + }; + signal?.addEventListener('abort', cancelForAbort, { once: true }); + }); try { const keep = await Promise.race([ taskNotice(notice, { signal: controller.signal }), timeout, + aborted, ]); return { keep, timedOut }; } finally { if (timer) clearTimeout(timer); + if (cancelForAbort) signal?.removeEventListener('abort', cancelForAbort); } } @@ -422,6 +485,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, @@ -484,6 +566,10 @@ export function effectiveExcludedTaskTypes( 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; @@ -494,23 +580,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; @@ -533,6 +630,7 @@ async function executeOrchestrator( boot.wizardFlagPayloads, ), }); + if (signal?.aborted) return cancelledRun(); const seedPrompt = registry.seed; if (!seedPrompt) { throw new Error( @@ -640,6 +738,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 @@ -660,6 +759,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'); @@ -834,8 +934,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}`); } @@ -869,6 +973,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 @@ -900,6 +1005,7 @@ async function executeOrchestrator( const seedHarness = requireTaskHarness(seedPick); const seedModel = promptModelFor(seedPrompt, seedPick.harness); const seedResult = await seedHarness.runTask({ + signal, config, input, boot, @@ -924,15 +1030,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), @@ -1055,6 +1155,7 @@ async function executeOrchestrator( existsSync(claudeSkillsDir) ? readdirSync(claudeSkillsDir) : [], ); const runTask: RunTask = async (task) => { + if (signal?.aborted) return; renderQueue(); try { @@ -1065,6 +1166,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. @@ -1087,6 +1189,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 { @@ -1114,33 +1217,54 @@ 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, + }, + RunOutcome.Failed, + task.type, + ); + } + if (signal?.aborted) return; + const terminal = terminalResult(taskResult); + 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 @@ -1152,9 +1276,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 @@ -1164,12 +1300,16 @@ 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 { // The queue file is wiped below; the e2e harness reads outcomes from here. config.hooks?.recordTaskOutcomes?.( @@ -1180,7 +1320,7 @@ async function executeOrchestrator( })) satisfies TaskOutcome[], ); try { - if (referenceSkillId && referenceInstallPath) { + if (!signal?.aborted && referenceSkillId && referenceInstallPath) { promoteReferenceSkill( path.join(input.installDir, referenceInstallPath), claudeSkillsDir, @@ -1188,10 +1328,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 { @@ -1201,14 +1345,27 @@ 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) { + // 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(); @@ -1252,14 +1409,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/runner/shared/ask.ts b/src/agent/runner/shared/ask.ts index 7acdfcb68..5ead4cf9c 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; }, @@ -36,5 +37,6 @@ export function createAskBridge( }, richLinks: options.richLinks, timeoutMs: options.timeoutMs, + signal: options.signal, }); } 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/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 52c284e94..4d94d9ade 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'; @@ -252,13 +252,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 { @@ -314,9 +315,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. */ @@ -326,4 +328,5 @@ export interface SequenceContext { boot: BootstrapResult; emit: ProgressEmitter; interaction: AgentInteraction | undefined; + signal?: AbortSignal; } diff --git a/src/agent/tools/mcp.ts b/src/agent/tools/mcp.ts index 8c0940ef0..ccbf21e84 100644 --- a/src/agent/tools/mcp.ts +++ b/src/agent/tools/mcp.ts @@ -46,7 +46,6 @@ import { ensureGitignoreCoverage, createAskAccounting, describeAskCancellation, - fetchSkillMenu, checkEnvKeys as checkEnvKeysCore, mergeEnvValues, normaliseAskSubject, @@ -58,7 +57,6 @@ import { templateEnvWriteRefusal, legacyKeyNameRefusal, vaultSensitiveAnswers, - type SkillEntry, AUDIT_ADD_CHECKS_DESCRIPTION, AUDIT_ADD_CHECKS_PARAM_DESCRIPTION, AUDIT_RESOLVE_CHECKS_DESCRIPTION, @@ -71,6 +69,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 e59b6cb51..6751232a7 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/agent/types.ts b/src/agent/types.ts new file mode 100644 index 000000000..c1367885f --- /dev/null +++ b/src/agent/types.ts @@ -0,0 +1,41 @@ +/** + * 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`. 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, + PromptContext, + RunConfig, + RunInput, + RunResult, +} from './runner'; +export type { + AgentInteraction, + AgentProgress, + AskAnswers, + AskQuestion, + AuthErrorDetail, + OutroData, + PendingQuestion, + SpinnerHandle, + TaskNotice, + TokenUsageDelta, +} from './progress'; + +/** 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 B2 with the legacy adapter that records it. */ +export type { TaskOutcome } from './runner'; + +/** Leaves in C2 with runMcpPromptViaSdk. */ +export type { AgentChunk } from './mcp-prompt-streaming'; diff --git a/src/agent/wizard-ask-bridge.ts b/src/agent/wizard-ask-bridge.ts index f76f1d1cd..5c57a1b79 100644 --- a/src/agent/wizard-ask-bridge.ts +++ b/src/agent/wizard-ask-bridge.ts @@ -54,17 +54,19 @@ export interface WizardAskBridge { } export interface WizardAskBridgeOptions { + /** Run cancellation: settles open questions as cancelled and aborts their signals. */ + 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. `signal` - * is this question's own: it aborts when the timeout wins the race, and the - * host dismisses this question's overlay. Without that the host keeps its - * pending-question state, and every later `wizard_ask` in the run fails with - * "another request is pending" — one unanswered prompt would block - * credential collection for all remaining sources. The host's abort - * handling must not throw: the bridge cannot catch an abort listener's - * error, and Node rethrows it as an uncaught exception. + * is this question's own: it aborts when the timeout wins the race or the + * run is cancelled, and the host dismisses this question's overlay. Without + * that the host keeps its pending-question state, and every later + * `wizard_ask` in the run fails with "another request is pending" — one + * unanswered prompt would block credential collection for all remaining + * sources. The host's abort handling must not throw: the bridge cannot catch + * an abort listener's error, and Node rethrows it as an uncaught exception. */ showQuestion: ( question: PendingQuestion, @@ -124,6 +126,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, @@ -137,24 +142,34 @@ export function createWizardAskBridge( const controller = new AbortController(); 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 abort this question's signal so the host dismisses its - // overlay: resolving our side alone would leave the host's - // pending-question state set, and the next wizard_ask would be rejected - // as a duplicate request. + // Race the user against the timeout and the run. Whichever fires first + // wins. When the timeout or the run wins we also abort this question's + // signal so the host dismisses its overlay: resolving our side alone + // would leave the host's pending-question state set, and the next + // wizard_ask would be rejected as a duplicate request. const timeoutPromise = new Promise((resolve) => { timer = setTimeout(() => { timedOut = true; - controller.abort(); + // Settle first: a host that rejects once dismissed must not win. resolve(buildCancelledAnswers(questions)); + controller.abort(); }, timeoutMs); }); + const aborted = new Promise((resolve) => { + cancelForAbort = () => { + resolve(buildCancelledAnswers(questions)); + controller.abort(); + }; + opts.signal?.addEventListener('abort', cancelForAbort, { once: true }); + }); try { const answers = await Promise.race([ opts.showQuestion(pending, { signal: controller.signal }), timeoutPromise, + aborted, ]); const durationMs = Date.now() - startedAt; @@ -178,6 +193,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/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/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/__tests__/agentic-retry.test.ts b/src/lib/detection/__tests__/agentic-retry.test.ts index f1df7e6b3..82ab1c161 100644 --- a/src/lib/detection/__tests__/agentic-retry.test.ts +++ b/src/lib/detection/__tests__/agentic-retry.test.ts @@ -37,7 +37,7 @@ function session() { function emitResult(text: string) { return execute.mockImplementationOnce((...args) => { args[5]?.onMessage({ type: 'result', result: text }); - return Promise.resolve({}); + return Promise.resolve({ kind: 'success' }); }); } @@ -92,7 +92,8 @@ describe('agentic detection retry', () => { it('retries a timed-out first run with a fresh Haiku session', async () => { const events: string[] = []; execute.mockResolvedValueOnce({ - error: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, + kind: 'failure', + classification: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, }); emitResult( '{"path":".","framework":"Next.js","targetId":"nextjs","hasPostHog":false}', @@ -116,7 +117,8 @@ describe('agentic detection retry', () => { it('reports a typed timeout when the retry also times out', async () => { execute.mockResolvedValue({ - error: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, + kind: 'failure', + classification: AgentErrorType.AGENTIC_DETECTION_TIMEOUT, }); await expect(detectProjectsWithAgent(session(), options)).rejects.toThrow( @@ -141,7 +143,7 @@ describe('agentic detection retry', () => { }, }); args[5]?.onMessage({ type: 'result', result: 'Done.' }); - return Promise.resolve({}); + return Promise.resolve({ kind: 'success' }); }); const report = await detectProjectsWithAgent(session(), { diff --git a/src/lib/detection/agentic.ts b/src/lib/detection/agentic.ts index 075298f4a..5be6704fa 100644 --- a/src/lib/detection/agentic.ts +++ b/src/lib/detection/agentic.ts @@ -15,11 +15,11 @@ import { initializeAgent, - runAgent as executeAgent, + executeAgent, buildRunTags, AgentSignals, AgentErrorType, -} from '@agent/agent-interface'; +} from '@agent'; import { isAbsolute, resolve, sep } from 'path'; import { detectNodePackageManagers } from './package-manager.js'; import { @@ -454,15 +454,24 @@ export async function detectProjectsWithAgent( middleware, ); - if (result.error === AgentErrorType.AGENTIC_DETECTION_TIMEOUT) { + if ( + result.kind === 'failure' && + result.classification === AgentErrorType.AGENTIC_DETECTION_TIMEOUT + ) { if (attempt === 0) { onEvent?.('Project scan timed out; retrying...'); continue; } throw new AgenticDetectionTimeoutError(attempt + 1, timeoutMs); } - 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/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/lib/programs/__tests__/run-agent-legacy.test.ts b/src/lib/programs/__tests__/run-agent-legacy.test.ts index 5f03cfd46..1e2703d16 100644 --- a/src/lib/programs/__tests__/run-agent-legacy.test.ts +++ b/src/lib/programs/__tests__/run-agent-legacy.test.ts @@ -10,10 +10,11 @@ import { LoggingUI } from '@ui/logging-ui'; import { InkUI } from '@ui/tui/ink-ui'; import { startTUI } from '@ui/tui/start-tui'; import { WizardStore } from '@ui/tui/store'; -import { setUI } from '@ui'; +import { getUI, setUI } from '@ui'; import { analytics } from '@utils/analytics'; import { initLogFile, logToFile } 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)); @@ -206,22 +207,105 @@ 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) => { - const failure = { message: 'Failed', exitCode: 2 }; +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', + exitCode: 2, + }; 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'); + 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 }), + ); + // wizardAbort sends the one terminal event for a failed run. + expect(analytics.shutdown).not.toHaveBeenCalled(); +}); + 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/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/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/programs/dispatch-family.ts b/src/lib/programs/dispatch-family.ts index e36831674..b9da9d39f 100644 --- a/src/lib/programs/dispatch-family.ts +++ b/src/lib/programs/dispatch-family.ts @@ -2,12 +2,12 @@ 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'; 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/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 b9f4b9800..5bfed9021 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, @@ -25,7 +24,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 2e0998789..6eddcf5b6 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. @@ -19,24 +19,26 @@ import { analytics } from '@utils/analytics'; import { getUI } from '@ui'; import { createUiReducer, uiInteraction } from '@ui/agent-progress'; import { + buildRunTags, + flushScanReport, + resolveBinding, runAgent, RunOutcome, - resolveBinding, TASK_OUTCOMES_KEY, - 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, @@ -286,7 +288,14 @@ async function runProgram( throw result.failure.error; } if (result.outcome !== RunOutcome.Success) { - await wizardAbort(result.failure); + if (result.failure.authErrorDetail) { + ui.showAuthError(result.failure.authErrorDetail); + } + // 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. // The run already succeeded: a failed flush is logged, never the outcome. 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..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'; /** @@ -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 894ae1f6e..8473d8096 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'; 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/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. 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/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 d55cb517d..f9c982420 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/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/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/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/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/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. 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/__tests__/agent-progress.test.ts b/src/ui/__tests__/agent-progress.test.ts index 237efb9ed..32b1d13dc 100644 --- a/src/ui/__tests__/agent-progress.test.ts +++ b/src/ui/__tests__/agent-progress.test.ts @@ -168,6 +168,20 @@ it('dismisses an open question or notice when its signal aborts', () => { expect(cancelNotice).toHaveBeenCalledOnce(); }); +it('dismisses at once when the request signal aborted before it opened', () => { + const ui = new LoggingUI(); + vi.spyOn(ui, 'requestQuestion').mockReturnValue(new Promise(() => undefined)); + const cancelAsk = vi.spyOn(ui, 'cancelPendingQuestion'); + vi.spyOn(ui, 'showTaskNotice').mockReturnValue(new Promise(() => undefined)); + const cancelNotice = vi.spyOn(ui, 'cancelTaskNotice'); + const interaction = uiInteraction(ui); + // An abort listener added to an aborted signal never fires. + void interaction.ask?.(question, { signal: AbortSignal.abort() }); + void interaction.taskNotice?.(notice, { signal: AbortSignal.abort() }); + expect(cancelAsk).toHaveBeenCalledOnce(); + expect(cancelNotice).toHaveBeenCalledOnce(); +}); + it('settles a timed-out question when the host dismissal throws', async () => { vi.useFakeTimers(); try { diff --git a/src/ui/agent-progress.ts b/src/ui/agent-progress.ts index 4594927a1..c3525af7a 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'; import { logToFile } from '@utils/debug'; // ── Progress → WizardUI, one call per event ─────────────────────────── @@ -95,6 +95,8 @@ function dismissOnAbort( logToFile('[agent-progress] dismissing an aborted request failed', error); } }; - signal.addEventListener('abort', onAbort, { once: true }); + // An abort listener added to an already aborted signal never fires. + if (signal.aborted) onAbort(); + else signal.addEventListener('abort', onAbort, { once: true }); return open.finally(() => signal.removeEventListener('abort', onAbort)); } 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; } 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..676276935 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'; +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 5d0af5eb4..3a5cbda40 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') },