diff --git a/src/lib/agent/runner/sequence/linear.ts b/src/lib/agent/runner/sequence/linear.ts index d851a210..f531a157 100644 --- a/src/lib/agent/runner/sequence/linear.ts +++ b/src/lib/agent/runner/sequence/linear.ts @@ -31,6 +31,7 @@ import { assemblePrompt } from '../../agent-prompt'; import type { ProgramRun, BootstrapResult } from '../shared/types'; import { abortOnInstallFailure } from '../shared/errors'; import { shouldDisableAsk, sessionToOptions } from '../shared/bootstrap'; +import { gateReportOnDisk } from '../shared/outro'; import { resolveHarness, getHarness } from '../switchboard'; export async function runLinearProgram( @@ -279,7 +280,14 @@ export async function runLinearProgram( : undefined, }; if (outroData) { - getUI().setOutroData(outroData); + // Gate the report reference on reality, not intent. `reportFile` is only + // ever written by the agent (an optional, soft step), so on the runs that + // never complete the agent it points at a file that was never created. The + // exit line and outro screens promise "check the report" purely from + // `reportFile` being set, and the handoff prompt tells the user's coding + // agent to read that same missing file. This mirrors the existsSync guard + // the orchestrator runner already applies before building its outro. + getUI().setOutroData(gateReportOnDisk(outroData, session.installDir)); } getUI().outro(config.successMessage); diff --git a/src/lib/agent/runner/shared/__tests__/outro.test.ts b/src/lib/agent/runner/shared/__tests__/outro.test.ts new file mode 100644 index 00000000..4e3d6886 --- /dev/null +++ b/src/lib/agent/runner/shared/__tests__/outro.test.ts @@ -0,0 +1,64 @@ +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { OutroKind } from '@lib/wizard-session'; +import { gateReportOnDisk } from '../outro'; + +describe('gateReportOnDisk', () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'wizard-outro-')); + }); + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it('strips reportFile and handoffPrompt when the report is missing', () => { + const gated = gateReportOnDisk( + { + kind: OutroKind.Success, + message: 'Successfully installed PostHog!', + reportFile: 'posthog-setup-report.md', + handoffPrompt: 'Read `posthog-setup-report.md` and work the checklist.', + }, + dir, + ); + + expect(gated?.reportFile).toBeUndefined(); + expect(gated?.handoffPrompt).toBeUndefined(); + // Other fields are left untouched. + expect(gated?.message).toBe('Successfully installed PostHog!'); + }); + + it('keeps reportFile and handoffPrompt when the report exists on disk', () => { + writeFileSync(join(dir, 'posthog-setup-report.md'), '# report'); + const prompt = 'Read `posthog-setup-report.md` and work the checklist.'; + + const gated = gateReportOnDisk( + { + kind: OutroKind.Success, + reportFile: 'posthog-setup-report.md', + handoffPrompt: prompt, + }, + dir, + ); + + expect(gated?.reportFile).toBe('posthog-setup-report.md'); + expect(gated?.handoffPrompt).toBe(prompt); + }); + + it('is a no-op when no reportFile is set', () => { + const gated = gateReportOnDisk( + { kind: OutroKind.Success, handoffPrompt: 'do the thing' }, + dir, + ); + // Nothing to gate against, so the handoff prompt is preserved as-is. + expect(gated?.handoffPrompt).toBe('do the thing'); + }); + + it('passes undefined through untouched', () => { + expect(gateReportOnDisk(undefined, dir)).toBeUndefined(); + }); +}); diff --git a/src/lib/agent/runner/shared/outro.ts b/src/lib/agent/runner/shared/outro.ts new file mode 100644 index 00000000..1fbbdc06 --- /dev/null +++ b/src/lib/agent/runner/shared/outro.ts @@ -0,0 +1,35 @@ +/** + * Outro post-processing shared across runners. + * + * The setup report (`reportFile`) is written by the agent as an optional, soft + * step — on runs where the agent never finishes (no `agent completed`), no skill + * is installed, or the model just skips the "write a report" instruction, the + * file is never created. Programs still set `reportFile` (and the coding-agent + * `handoffPrompt` that tells the user to read it) unconditionally in their outro + * data, so the outro screen and exit line end up promising a file that isn't on + * disk. Gate the promise on reality: drop both fields when the file is missing. + */ + +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import type { OutroData } from '@lib/wizard-session'; + +/** + * Strip `reportFile`/`handoffPrompt` from outro data when the referenced report + * does not exist under `installDir`. Mutates and returns the same object (or + * passes `undefined` through). The handoff prompt only makes sense alongside the + * report it points at, so the two are gated together. + */ +export function gateReportOnDisk( + outroData: T, + installDir: string, +): T { + if ( + outroData?.reportFile && + !existsSync(join(installDir, outroData.reportFile)) + ) { + outroData.reportFile = undefined; + outroData.handoffPrompt = undefined; + } + return outroData; +} diff --git a/src/ui/tui/__tests__/exit-line.test.ts b/src/ui/tui/__tests__/exit-line.test.ts index 146f74af..61e37877 100644 --- a/src/ui/tui/__tests__/exit-line.test.ts +++ b/src/ui/tui/__tests__/exit-line.test.ts @@ -1,3 +1,4 @@ +import { isAbsolute, join } from 'node:path'; import { getExitLine } from '@ui/tui/exit-line'; import { WizardStore, Program } from '@ui/tui/store'; import { OutroKind } from '@lib/wizard-session'; @@ -95,17 +96,35 @@ describe('getExitLine', () => { expect(line).toContain('• Review findings'); }); - it('appends the report suffix when the message does not already mention it', () => { + it('appends the report suffix with the resolved absolute path (not a bare ./)', () => { + const store = storeWithOutro({ + kind: OutroKind.Success, + message: 'Done!', + reportFile: 'posthog-setup-report.md', + }); + const line = stripAnsi(getExitLine(store)); + const expectedPath = join( + store.session.installDir, + 'posthog-setup-report.md', + ); + expect(line).toContain(`Check ${expectedPath} for details.`); + // Absolute path so users who passed --install-dir look in the right place. + expect(isAbsolute(expectedPath)).toBe(true); + expect(line).not.toContain('./posthog-setup-report.md'); + }); + + it('leaves an already-absolute report path (orchestrator queue fallback) untouched', () => { + const abs = '/tmp/some-project/.posthog-wizard/queue.json'; const line = stripAnsi( getExitLine( storeWithOutro({ kind: OutroKind.Success, message: 'Done!', - reportFile: 'posthog-setup-report.md', + reportFile: abs, }), ), ); - expect(line).toContain('Check ./posthog-setup-report.md for details.'); + expect(line).toContain(`Check ${abs} for details.`); }); it('falls back to a default headline when the outro has no message', () => { diff --git a/src/ui/tui/exit-line.ts b/src/ui/tui/exit-line.ts index 66d70981..24079c36 100644 --- a/src/ui/tui/exit-line.ts +++ b/src/ui/tui/exit-line.ts @@ -13,6 +13,7 @@ * function (start-tui.ts itself pulls in the whole render tree). */ +import { isAbsolute, join } from 'node:path'; import { totalTokenCount, type WizardStore } from './store.js'; import { OutroKind } from '@lib/wizard-session'; import { formatTokenCount, formatCostUsd } from '@lib/agent/token-pricing'; @@ -56,9 +57,20 @@ export function getExitLine(store: WizardStore): string { if (outro?.kind === OutroKind.Success) { const message = outro.message ?? `${label} completed successfully.`; + // `reportFile` reaching here means the file was verified on disk by the + // runner (see linear.ts / orchestrator-runner.ts), so we can promise it + // without over-claiming. Surface the resolved path — a bare `./` is wrong + // when the user passed --install-dir and ran from elsewhere. The + // orchestrator's queue-file fallback is already absolute, so only join + // relative report names against installDir. + const reportPath = outro.reportFile + ? isAbsolute(outro.reportFile) + ? outro.reportFile + : join(store.session.installDir, outro.reportFile) + : undefined; const reportSuffix = - outro.reportFile && !message.includes(outro.reportFile) - ? ` Check ./${outro.reportFile} for details.` + reportPath && outro.reportFile && !message.includes(outro.reportFile) + ? ` Check ${reportPath} for details.` : ''; const headline = `${GREEN}${BOLD}✔${RESET_ATTRS} ${message}${reportSuffix}`; diff --git a/src/ui/tui/screens/OutroScreen.tsx b/src/ui/tui/screens/OutroScreen.tsx index d0f10b1c..011aa61e 100644 --- a/src/ui/tui/screens/OutroScreen.tsx +++ b/src/ui/tui/screens/OutroScreen.tsx @@ -6,6 +6,7 @@ * ship their own screen component (see audit/AuditOutroScreen.tsx). */ +import { isAbsolute, join } from 'node:path'; import { Box, Text } from 'ink'; import { useSyncExternalStore } from 'react'; import type { WizardStore } from '@ui/tui/store'; @@ -97,7 +98,13 @@ export const OutroScreen = ({ store }: OutroScreenProps) => { {outroData.reportFile && ( - Check ./{outroData.reportFile} for details + Check{' '} + + {isAbsolute(outroData.reportFile) + ? outroData.reportFile + : join(store.session.installDir, outroData.reportFile)} + {' '} + for details )}