diff --git a/src/lib/__tests__/wizard-ask-bridge.test.ts b/src/lib/__tests__/wizard-ask-bridge.test.ts index f86e31b66..da936fde2 100644 --- a/src/lib/__tests__/wizard-ask-bridge.test.ts +++ b/src/lib/__tests__/wizard-ask-bridge.test.ts @@ -1,10 +1,16 @@ import { CANCELLED_SENTINEL, + TIMED_OUT_SENTINEL, createWizardAskBridge, isFullyCancelled, + isFullyTimedOut, } from '@lib/wizard-ask-bridge'; import { analytics } from '@utils/analytics'; -import type { AskAnswers, PendingQuestion } from '@lib/wizard-session'; +import type { + AskAnswers, + AskQuestion, + PendingQuestion, +} from '@lib/wizard-session'; vi.mock('../../utils/analytics', () => ({ analytics: { @@ -189,10 +195,31 @@ describe('createWizardAskBridge', () => { it('is false for an empty answer map', () => { expect(isFullyCancelled({})).toBe(false); }); + + it('is true for a timed-out ask, so a timeout also refunds the slot', () => { + expect( + isFullyCancelled({ a: TIMED_OUT_SENTINEL, b: TIMED_OUT_SENTINEL }), + ).toBe(true); + }); + }); + + describe('isFullyTimedOut', () => { + // Gates the timeout guidance the facades return: it must fire for a + // timeout and never for a dismissal, which needs the opposite advice. + it('is true only when every field is the timed-out sentinel', () => { + expect( + isFullyTimedOut({ a: TIMED_OUT_SENTINEL, b: TIMED_OUT_SENTINEL }), + ).toBe(true); + expect( + isFullyTimedOut({ a: CANCELLED_SENTINEL, b: CANCELLED_SENTINEL }), + ).toBe(false); + expect(isFullyTimedOut({ a: TIMED_OUT_SENTINEL, b: 'real' })).toBe(false); + expect(isFullyTimedOut({})).toBe(false); + }); }); describe('timeout', () => { - it('resolves every field with the cancelled sentinel and dismisses the host overlay when the user does not answer in time', async () => { + it('resolves every field with the timed-out sentinel and dismisses the host overlay when the user does not answer in time', async () => { vi.useFakeTimers(); try { // showQuestion intentionally never resolves — the timeout has to win. @@ -213,9 +240,12 @@ describe('createWizardAskBridge', () => { vi.advanceTimersByTime(1000); + // A timeout must not look like a dismissal. The agent reads the + // dismissal sentinel as "the user declined" and unwinds its work, and + // the user who walked off to run a build is still coming back. await expect(promise).resolves.toEqual({ - goal: CANCELLED_SENTINEL, - audience: CANCELLED_SENTINEL, + goal: TIMED_OUT_SENTINEL, + audience: TIMED_OUT_SENTINEL, }); // Without this, the host's pending-question state survives the @@ -232,6 +262,52 @@ describe('createWizardAskBridge', () => { } }); + // The regression this guards: `cancelQuestion` is not a no-op on the real + // TUI path. `WizardStore.cancelPendingQuestion()` fills every field with + // the dismissal sentinel and resolves the pending `showQuestion` promise + // synchronously, so cancelling before resolving let the dismissal settle + // first and win the `Promise.race` — the agent got "__cancelled__" and + // none of the timeout guidance, on exactly the path this all exists for. + it('answers with the timeout sentinel even when cancelQuestion settles the host promise', async () => { + vi.useFakeTimers(); + try { + const questions: AskQuestion[] = [ + { id: 'goal', prompt: 'Goal?', kind: 'text' }, + { id: 'audience', prompt: 'Who?', kind: 'text' }, + ]; + let resolveHost!: (answers: AskAnswers) => void; + + const bridge = createWizardAskBridge({ + getSource: () => 'product-tours', + showQuestion: () => + new Promise((r) => { + resolveHost = r; + }), + cancelQuestion: () => { + const cancelled: AskAnswers = {}; + for (const q of questions) cancelled[q.id] = CANCELLED_SENTINEL; + resolveHost(cancelled); + }, + timeoutMs: 1000, + }); + + const promise = bridge.request({ questions }); + vi.advanceTimersByTime(1000); + + await expect(promise).resolves.toEqual({ + goal: TIMED_OUT_SENTINEL, + audience: TIMED_OUT_SENTINEL, + }); + + const cancelledCall = wizardCaptureMock.mock.calls.find( + ([name]) => name === 'wizard_ask cancelled', + ); + expect(cancelledCall?.[1]).toMatchObject({ timed_out: true }); + } finally { + vi.useRealTimers(); + } + }); + it('does not dismiss the overlay when the user answers before the timeout', async () => { vi.useFakeTimers(); try { diff --git a/src/lib/__tests__/wizard-tools.test.ts b/src/lib/__tests__/wizard-tools.test.ts index 901a1d025..87cf8c281 100644 --- a/src/lib/__tests__/wizard-tools.test.ts +++ b/src/lib/__tests__/wizard-tools.test.ts @@ -6,6 +6,7 @@ import { zipSync } from 'fflate'; import { ASK_BATCH_THRESHOLD, ASK_SUBJECT_UNSPECIFIED, + ASK_TIMED_OUT_NOTE, DEFAULT_ASK_MAX_QUESTIONS, WIZARD_ASK_SUBJECT_DESCRIPTION, WIZARD_ASK_TOOL_DESCRIPTION, @@ -918,12 +919,26 @@ describe('wizard_ask shared descriptions', () => { expect(WIZARD_ASK_TOOL_DESCRIPTION).toMatch(/never blocked/i); }); - it('keeps the cancellation promise the warehouse skill relies on', () => { + it('keeps the free-cancellation promise the warehouse skill relies on', () => { expect(WIZARD_ASK_TOOL_DESCRIPTION).toMatch( - /cancelled or timed-out response does NOT count/, + /Neither a cancelled nor a timed-out response counts/, ); }); + it('separates a dismissal from a timeout, and forbids reverting on a timeout', () => { + expect(WIZARD_ASK_TOOL_DESCRIPTION).toMatch(/__cancelled__/); + expect(WIZARD_ASK_TOOL_DESCRIPTION).toMatch(/__timed_out__/); + expect(WIZARD_ASK_TOOL_DESCRIPTION).toMatch(/ask the same question again/i); + expect(WIZARD_ASK_TOOL_DESCRIPTION).toMatch(/never undo or revert/i); + }); + + it('tells the agent on a timeout to keep waiting and leave its changes in place', () => { + expect(ASK_TIMED_OUT_NOTE).toMatch(/not a decline/i); + expect(ASK_TIMED_OUT_NOTE).toMatch(/Do NOT undo, revert, or delete/); + expect(ASK_TIMED_OUT_NOTE).toMatch(/Ask the same question again/); + expect(ASK_TIMED_OUT_NOTE).toMatch(/costs nothing/); + }); + it('explains what a subject is and what omitting it costs', () => { expect(WIZARD_ASK_SUBJECT_DESCRIPTION).toMatch(/Postgres/); expect(WIZARD_ASK_SUBJECT_DESCRIPTION).toMatch(/consecutive calls/i); diff --git a/src/lib/agent/__tests__/__snapshots__/commandments.test.ts.snap b/src/lib/agent/__tests__/__snapshots__/commandments.test.ts.snap index 9135f7aab..d53dc1694 100644 --- a/src/lib/agent/__tests__/__snapshots__/commandments.test.ts.snap +++ b/src/lib/agent/__tests__/__snapshots__/commandments.test.ts.snap @@ -22,7 +22,7 @@ When a skill provides a numbered or bulleted list of questions, translate the en - For \`single\` and \`multi\`, extract the alternatives from the prose into \`options\` as \`{ label, value }\` pairs. Use the human phrase as \`label\` and a lowercase-hyphenated form as \`value\` (e.g., \`label: "Vanilla JS"\`, \`value: "vanilla-js"\`). - Use a kebab-case slug of the question label as \`id\` (e.g., "Tech stack" → \`tech-stack\`, "Show frequency" → \`show-frequency\`). - Do not invent fields the schema does not define (no \`source\`, \`category\`, \`priority\`, etc.) — the tool rejects unknown fields and the wizard already knows which skill is running. -After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields. +After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields. The one exception is a fully timed-out return, where every answer is \`__timed_out__\`: nothing was answered, so ask the same questions again to keep waiting, and follow the guidance that result carries. Use the Task tools to plan and track the whole run so the user always sees where you are. Create the task list once you understand the work — after you load and skim the skill workflow, not before — in a single tool call, in the order you will run them, with one task per stage covering the whole run through to instrumenting events, creating the dashboard, and writing the setup report. Give each an imperative subject AND an \`activeForm\` (the present-continuous label the panel shows while it runs, e.g. subject "Install SDK" / activeForm "Installing SDK"). Keep the list current: add a task the moment you discover work it is missing. Try to keep exactly ONE task \`in_progress\`. \`TaskUpdate\` it to \`in_progress\` right before you start that stage, and to \`completed\` the instant you finish it — one at a time, never batched at the end. Only mark \`completed\` when the work is genuinely done; if the build fails, a step is partial, or you hit a blocker, keep it \`in_progress\` and add a task for the fix. After you complete a task, take the next one in order (lowest id first — earlier stages set up later ones), mark it \`in_progress\`, and continue. Driving the list in order top to bottom is how you finish every stage. @@ -51,7 +51,7 @@ When a skill provides a numbered or bulleted list of questions, translate the en - For \`single\` and \`multi\`, extract the alternatives from the prose into \`options\` as \`{ label, value }\` pairs. Use the human phrase as \`label\` and a lowercase-hyphenated form as \`value\` (e.g., \`label: "Vanilla JS"\`, \`value: "vanilla-js"\`). - Use a kebab-case slug of the question label as \`id\` (e.g., "Tech stack" → \`tech-stack\`, "Show frequency" → \`show-frequency\`). - Do not invent fields the schema does not define (no \`source\`, \`category\`, \`priority\`, etc.) — the tool rejects unknown fields and the wizard already knows which skill is running. -After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields." +After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields. The one exception is a fully timed-out return, where every answer is \`__timed_out__\`: nothing was answered, so ask the same questions again to keep waiting, and follow the guidance that result carries." `; exports[`commandments by axis > 'pi' + 'linear' > matches the published prompt 1`] = ` @@ -76,7 +76,7 @@ When a skill provides a numbered or bulleted list of questions, translate the en - For \`single\` and \`multi\`, extract the alternatives from the prose into \`options\` as \`{ label, value }\` pairs. Use the human phrase as \`label\` and a lowercase-hyphenated form as \`value\` (e.g., \`label: "Vanilla JS"\`, \`value: "vanilla-js"\`). - Use a kebab-case slug of the question label as \`id\` (e.g., "Tech stack" → \`tech-stack\`, "Show frequency" → \`show-frequency\`). - Do not invent fields the schema does not define (no \`source\`, \`category\`, \`priority\`, etc.) — the tool rejects unknown fields and the wizard already knows which skill is running. -After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields. +After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields. The one exception is a fully timed-out return, where every answer is \`__timed_out__\`: nothing was answered, so ask the same questions again to keep waiting, and follow the guidance that result carries. Use the Task tools to plan and track the whole run so the user always sees where you are. Create the task list once you understand the work — after you load and skim the skill workflow, not before — in a single tool call, in the order you will run them, with one task per stage covering the whole run through to instrumenting events, creating the dashboard, and writing the setup report. Give each an imperative subject AND an \`activeForm\` (the present-continuous label the panel shows while it runs, e.g. subject "Install SDK" / activeForm "Installing SDK"). Keep the list current: add a task the moment you discover work it is missing. Try to keep exactly ONE task \`in_progress\`. \`TaskUpdate\` it to \`in_progress\` right before you start that stage, and to \`completed\` the instant you finish it — one at a time, never batched at the end. Only mark \`completed\` when the work is genuinely done; if the build fails, a step is partial, or you hit a blocker, keep it \`in_progress\` and add a task for the fix. After you complete a task, take the next one in order (lowest id first — earlier stages set up later ones), mark it \`in_progress\`, and continue. Driving the list in order top to bottom is how you finish every stage. @@ -129,7 +129,7 @@ When a skill provides a numbered or bulleted list of questions, translate the en - For \`single\` and \`multi\`, extract the alternatives from the prose into \`options\` as \`{ label, value }\` pairs. Use the human phrase as \`label\` and a lowercase-hyphenated form as \`value\` (e.g., \`label: "Vanilla JS"\`, \`value: "vanilla-js"\`). - Use a kebab-case slug of the question label as \`id\` (e.g., "Tech stack" → \`tech-stack\`, "Show frequency" → \`show-frequency\`). - Do not invent fields the schema does not define (no \`source\`, \`category\`, \`priority\`, etc.) — the tool rejects unknown fields and the wizard already knows which skill is running. -After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields. +After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields. The one exception is a fully timed-out return, where every answer is \`__timed_out__\`: nothing was answered, so ask the same questions again to keep waiting, and follow the guidance that result carries. ## This runtime Below are important guidance on the harness constraints you are bound to. Follow them as commandments. @@ -172,7 +172,7 @@ When a skill provides a numbered or bulleted list of questions, translate the en - For \`single\` and \`multi\`, extract the alternatives from the prose into \`options\` as \`{ label, value }\` pairs. Use the human phrase as \`label\` and a lowercase-hyphenated form as \`value\` (e.g., \`label: "Vanilla JS"\`, \`value: "vanilla-js"\`). - Use a kebab-case slug of the question label as \`id\` (e.g., "Tech stack" → \`tech-stack\`, "Show frequency" → \`show-frequency\`). - Do not invent fields the schema does not define (no \`source\`, \`category\`, \`priority\`, etc.) — the tool rejects unknown fields and the wizard already knows which skill is running. -After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields. +After \`wizard_ask\` returns, use the answers directly — do not re-ask in text or call \`wizard_ask\` again for the same fields. The one exception is a fully timed-out return, where every answer is \`__timed_out__\`: nothing was answered, so ask the same questions again to keep waiting, and follow the guidance that result carries. ALWAYS surface a custom-scout proposal in step 6b: bring the user your one or two strongest candidate scouts even when the built-in troop looks sufficient. The proposal ask leads with a "None — keep the built-in troop" option, so declining costs the user one keystroke — but a proposal you silently skip is coverage they never got to see or judge. Where the skill says to skip the ask when the gap analysis finds no candidate, do NOT skip: pick your best candidates anyway and let the user decide. Rank candidates at the discriminator level, not the category level. "Covered" only means an enabled scout would actually FIRE for that failure mode: a conversion-rate watcher does not catch entry volume collapsing; a Stripe-transaction watcher does not catch a lead form going silent. A surface whose failure mode has no firing condition among the enabled scouts is your strongest candidate. Be honest in the option descriptions: if a candidate overlaps something an enabled scout partially watches, say so in its description rather than dropping the candidate. The user chooses with full information; you do not gatekeep on their behalf. diff --git a/src/lib/agent/__tests__/commandments.test.ts b/src/lib/agent/__tests__/commandments.test.ts index b00a74a4f..d723ab7af 100644 --- a/src/lib/agent/__tests__/commandments.test.ts +++ b/src/lib/agent/__tests__/commandments.test.ts @@ -133,6 +133,15 @@ describe('commandments by axis', () => { it('tells the agent to use answers directly without re-asking', () => { expect(text).toMatch(/do not re-ask/i); }); + + // The no-re-ask rule and the timeout guidance in `WIZARD_ASK_TOOL_DESCRIPTION` + // both reach the agent in one context. Without this carve-out the commandment + // — assembled first, in every run — forbids the retry that is the whole point + // of answering a timeout with its own sentinel. + it('exempts a fully timed-out return from the no-re-ask rule', () => { + expect(text).toMatch(/`__timed_out__`/); + expect(text).toMatch(/ask the same questions again to keep waiting/i); + }); }); }); diff --git a/src/lib/agent/commandments.ts b/src/lib/agent/commandments.ts index b45593cc8..464f03412 100644 --- a/src/lib/agent/commandments.ts +++ b/src/lib/agent/commandments.ts @@ -45,6 +45,6 @@ export const WIZARD_COMMANDMENTS = [ ' - For `single` and `multi`, extract the alternatives from the prose into `options` as `{ label, value }` pairs. Use the human phrase as `label` and a lowercase-hyphenated form as `value` (e.g., `label: "Vanilla JS"`, `value: "vanilla-js"`).', ' - Use a kebab-case slug of the question label as `id` (e.g., "Tech stack" → `tech-stack`, "Show frequency" → `show-frequency`).', ' - Do not invent fields the schema does not define (no `source`, `category`, `priority`, etc.) — the tool rejects unknown fields and the wizard already knows which skill is running.', - 'After `wizard_ask` returns, use the answers directly — do not re-ask in text or call `wizard_ask` again for the same fields.', + 'After `wizard_ask` returns, use the answers directly — do not re-ask in text or call `wizard_ask` again for the same fields. The one exception is a fully timed-out return, where every answer is `__timed_out__`: nothing was answered, so ask the same questions again to keep waiting, and follow the guidance that result carries.', ].join('\n'), ]; diff --git a/src/lib/agent/runner/harness/pi/__tests__/tools.test.ts b/src/lib/agent/runner/harness/pi/__tests__/tools.test.ts index 1b1d88d9f..0529f82df 100644 --- a/src/lib/agent/runner/harness/pi/__tests__/tools.test.ts +++ b/src/lib/agent/runner/harness/pi/__tests__/tools.test.ts @@ -10,6 +10,7 @@ import { join } from 'node:path'; import { describe, it, expect, vi } from 'vitest'; import { CANCELLED_SENTINEL, + TIMED_OUT_SENTINEL, type WizardAskBridge, } from '@lib/wizard-ask-bridge'; import { createWizardPiTools } from '../tools'; @@ -17,6 +18,7 @@ import { evaluateToolCall } from '../security'; import { allowedPiCodingTools, allowedOrchestratorTools } from '../task'; import { ASK_BATCH_THRESHOLD, + ASK_TIMED_OUT_NOTE, WIZARD_ASK_SENSITIVE_DESCRIPTION, WIZARD_ASK_SUBJECT_DESCRIPTION, WIZARD_ASK_TOOL_DESCRIPTION, @@ -92,6 +94,19 @@ describe('pi wizard_ask — sensitive answers are vaulted', () => { expect(answers.token).toBe(CANCELLED_SENTINEL); }); + it('a timed-out sensitive answer is returned as its own sentinel, not vaulted', async () => { + const { wizardAsk } = makeTools({ token: TIMED_OUT_SENTINEL }); + const result = await call(wizardAsk, { + questions: [ + { id: 'token', prompt: 'Zendesk token', kind: 'text', sensitive: true }, + ], + }); + const { answers } = JSON.parse(textOf(result)) as { + answers: { token: string }; + }; + expect(answers.token).toBe(TIMED_OUT_SENTINEL); + }); + it('still rejects sensitive=true on non-text kinds', async () => { const { wizardAsk, request } = makeTools({}); const result = await call(wizardAsk, { @@ -459,3 +474,47 @@ describe('pi task tool grant — the names the inventory shows', () => { ]); }); }); + +describe('pi wizard_ask — a timeout is not a decline', () => { + it('returns the do-not-revert guidance when every field timed out', async () => { + const { wizardAsk } = makeTools({ verified: TIMED_OUT_SENTINEL }); + const result = await call(wizardAsk, { + questions: [ + { id: 'verified', prompt: 'See the error in PostHog?', kind: 'text' }, + ], + }); + const body = JSON.parse(textOf(result)) as { + unanswered_reason?: string; + note?: string; + }; + expect(body.unanswered_reason).toBe('timeout'); + expect(body.note).toBe(ASK_TIMED_OUT_NOTE); + }); + + it('says nothing about a timeout when the user dismissed the prompt', async () => { + const { wizardAsk } = makeTools({ verified: CANCELLED_SENTINEL }); + const result = await call(wizardAsk, { + questions: [ + { id: 'verified', prompt: 'See the error in PostHog?', kind: 'text' }, + ], + }); + const body = JSON.parse(textOf(result)) as Record; + expect(body.unanswered_reason).toBeUndefined(); + expect(body.note).toBeUndefined(); + }); + + it('does not burn a call slot, so the agent can ask again and keep waiting', async () => { + const { wizardAsk, request } = makeTools( + { verified: TIMED_OUT_SENTINEL }, + 1, + ); + const questions = [ + { id: 'verified', prompt: 'See the error in PostHog?', kind: 'text' }, + ]; + await call(wizardAsk, { questions, subject: 'verify' }); + const second = await call(wizardAsk, { questions, subject: 'verify' }); + + expect(request).toHaveBeenCalledTimes(2); + expect(textOf(second)).not.toContain('cap reached'); + }); +}); diff --git a/src/lib/agent/runner/harness/pi/tools.ts b/src/lib/agent/runner/harness/pi/tools.ts index 35b5aa8c5..8c57605ac 100644 --- a/src/lib/agent/runner/harness/pi/tools.ts +++ b/src/lib/agent/runner/harness/pi/tools.ts @@ -27,6 +27,7 @@ import { checkEnvKeys as checkEnvKeysCore, createAskAccounting, fetchSkillMenu, + formatAskResult, installSkillById, mergeEnvValues, normaliseAskSubject, @@ -39,7 +40,11 @@ import { WIZARD_ASK_TOOL_DESCRIPTION, } from '@lib/wizard-tools/tools'; import type { LLMProvider } from '@posthog/warlock'; -import { isFullyCancelled, type WizardAskBridge } from '@lib/wizard-ask-bridge'; +import { + isFullyCancelled, + isFullyTimedOut, + type WizardAskBridge, +} from '@lib/wizard-ask-bridge'; import { PUBLISH_HANDOFF_CONTENT_DESCRIPTION, PUBLISH_HANDOFF_DESCRIPTION, @@ -393,7 +398,7 @@ export function createWizardPiTools(ctx: PiToolsContext): ToolDefinition[] { Object.keys(answers).length } answer(s) for ${args.questions.length} question(s)`, ); - return text(JSON.stringify({ answers: sanitised }, null, 2)); + return text(formatAskResult(sanitised, isFullyTimedOut(answers))); } catch (err) { askAccounting.refund(args.subject); const message = err instanceof Error ? err.message : String(err); diff --git a/src/lib/agent/runner/shared/types.ts b/src/lib/agent/runner/shared/types.ts index 2c9e4b141..40cc23a10 100644 --- a/src/lib/agent/runner/shared/types.ts +++ b/src/lib/agent/runner/shared/types.ts @@ -76,7 +76,9 @@ export interface ProgramRun { * Per-question `wizard_ask` timeout in milliseconds. Defaults to * DEFAULT_ASK_TIMEOUT_MS (5 minutes). Raise it for programs whose * questions send the user off to do slow work (run a build, create a - * key in the browser) before they can answer. + * key in the browser) before they can answer. A timeout is not a decline: + * it answers with TIMED_OUT_SENTINEL and tells the agent to ask again + * rather than unwind its work, so the timer bounds one wait, not the run. */ askTimeoutMs?: number; /** diff --git a/src/lib/programs/error-tracking-upload-source-maps/index.ts b/src/lib/programs/error-tracking-upload-source-maps/index.ts index 80db004f5..c446d53fe 100644 --- a/src/lib/programs/error-tracking-upload-source-maps/index.ts +++ b/src/lib/programs/error-tracking-upload-source-maps/index.ts @@ -94,8 +94,8 @@ export const errorTrackingUploadSourceMapsConfig: ProgramConfig = { // The flow parks on wizard_ask while the user does slow work — create // a personal API key in the browser (STEP 1), or run a production // build, trigger the test error, and check Error Tracking (STEP 8). - // The 5-minute default cancels the question mid-task and the agent - // wraps up to the outro, so give these answers half an hour. + // The 5-minute default closes the question mid-task, so give these + // answers half an hour. askTimeoutMs: 30 * 60 * 1000, customPrompt: (ctx) => { diff --git a/src/lib/programs/self-driving/index.ts b/src/lib/programs/self-driving/index.ts index b507cc18e..0c0ab7970 100644 --- a/src/lib/programs/self-driving/index.ts +++ b/src/lib/programs/self-driving/index.ts @@ -70,8 +70,7 @@ const buildRun = (session: WizardSession): Promise => richLinks: true, // STEP 3 (GitHub App install) and STEP 5 (Linear OAuth) park on wizard_ask // while the user does slow browser work; a first-time GitHub App install - // routinely exceeds the 5-min default, and a timeout is indistinguishable - // from a decline (both resolve to __cancelled__). Match upload-source-maps. + // routinely exceeds the 5-min default. Match upload-source-maps. askTimeoutMs: 30 * 60 * 1000, // Emit a `wizard: step` analytics event on each agent task transition so we diff --git a/src/lib/wizard-ask-bridge.ts b/src/lib/wizard-ask-bridge.ts index b0d4c4a03..2fe314f19 100644 --- a/src/lib/wizard-ask-bridge.ts +++ b/src/lib/wizard-ask-bridge.ts @@ -38,7 +38,9 @@ export interface WizardAskBridge { /** * Open the WizardAsk overlay and resolve with the user's answers. * One answer per question id (string for `single`/`text`, string[] for - * `multi`). Cancelled fields come back as the literal `"__cancelled__"`. + * `multi`). A field the user dismissed comes back as the literal + * `"__cancelled__"`; a field the timeout closed comes back as + * `"__timed_out__"`. */ request(req: WizardAskRequest): Promise; } @@ -51,7 +53,7 @@ export interface WizardAskBridgeOptions { /** * Per-question timeout in milliseconds. When the user takes longer than * this to answer, every unanswered field resolves with the - * {@link CANCELLED_SENTINEL} value. Defaults to {@link DEFAULT_ASK_TIMEOUT_MS}. + * {@link TIMED_OUT_SENTINEL} value. Defaults to {@link DEFAULT_ASK_TIMEOUT_MS}. */ timeoutMs?: number; /** @@ -70,24 +72,56 @@ export interface WizardAskBridgeOptions { cancelQuestion?: () => void; } -/** Sentinel returned for unanswered fields on cancellation or timeout. */ +/** Sentinel returned for unanswered fields when the user dismisses the overlay. */ export const CANCELLED_SENTINEL = '__cancelled__'; +/** + * Sentinel returned for unanswered fields when the timeout wins the race. + * + * A timeout is not a decline. The programs that raise `askTimeoutMs` park on a + * question while the user does slow work away from the terminal — run a + * production build, trigger a test error, check Error Tracking. Before this + * sentinel existed both endings resolved to {@link CANCELLED_SENTINEL}, the + * agent read "the user declined", and it unwound work the user was still in the + * middle of verifying. The two endings need different answers so the agent can + * tell them apart. + */ +export const TIMED_OUT_SENTINEL = '__timed_out__'; + /** Default per-question timeout (5 minutes). */ export const DEFAULT_ASK_TIMEOUT_MS = 5 * 60 * 1000; -function buildCancelledAnswers(questions: AskQuestion[]): AskAnswers { +function buildUnansweredAnswers( + questions: AskQuestion[], + sentinel: string, +): AskAnswers { const out: AskAnswers = {}; for (const q of questions) { - out[q.id] = CANCELLED_SENTINEL; + out[q.id] = sentinel; } return out; } +/** True for either unanswered sentinel — a dismissal or a timeout. */ +export function isUnansweredSentinel(value: unknown): boolean { + return value === CANCELLED_SENTINEL || value === TIMED_OUT_SENTINEL; +} + +/** + * True when no field carries a real answer. Gates the per-run cap refund, so + * it must cover both endings: neither a dismissal nor a timeout may burn a slot. + */ export function isFullyCancelled(answers: AskAnswers): boolean { const values = Object.values(answers); if (values.length === 0) return false; - return values.every((v) => v === CANCELLED_SENTINEL); + return values.every(isUnansweredSentinel); +} + +/** True when every field came back as the timeout sentinel. */ +export function isFullyTimedOut(answers: AskAnswers): boolean { + const values = Object.values(answers); + if (values.length === 0) return false; + return values.every((v) => v === TIMED_OUT_SENTINEL); } export function createWizardAskBridge( @@ -107,6 +141,7 @@ export function createWizardAskBridge( const startedAt = Date.now(); let timer: ReturnType | undefined; + let timedOut = false; // Race the user against the timeout. Whichever fires first wins. On // timeout we also cancel the host's overlay: resolving our side alone @@ -114,8 +149,20 @@ export function createWizardAskBridge( // wizard_ask would be rejected as a duplicate request. const timeoutPromise = new Promise((resolve) => { timer = setTimeout(() => { - opts.cancelQuestion?.(); - resolve(buildCancelledAnswers(questions)); + timedOut = true; + // Answer before cleaning up, or cleanup wins the race. On the real + // TUI path `cancelQuestion` settles the `showQuestion` promise + // synchronously with the dismissal sentinel + // (`WizardStore.cancelPendingQuestion`), so cancelling first let that + // promise settle ahead of this one — and the timeout came back + // indistinguishable from the decline it is not. + resolve(buildUnansweredAnswers(questions, TIMED_OUT_SENTINEL)); + try { + opts.cancelQuestion?.(); + } catch { + // Best-effort: the timeout answer is already settled, and a + // caller-injected callback must not take the run down from a timer. + } }, timeoutMs); }); @@ -132,7 +179,7 @@ export function createWizardAskBridge( subject, question_count: questions.length, duration_ms: durationMs, - timed_out: durationMs >= timeoutMs, + timed_out: timedOut, }); } else { analytics.wizardCapture('wizard_ask answered', { diff --git a/src/lib/wizard-tools/mcp.ts b/src/lib/wizard-tools/mcp.ts index ef7a038ee..832fd5e1d 100644 --- a/src/lib/wizard-tools/mcp.ts +++ b/src/lib/wizard-tools/mcp.ts @@ -20,7 +20,11 @@ import { type AuditCheck, type AuditStatus, } from '../programs/audit/types'; -import { type WizardAskBridge, isFullyCancelled } from '../wizard-ask-bridge'; +import { + type WizardAskBridge, + isFullyCancelled, + isFullyTimedOut, +} from '../wizard-ask-bridge'; import { PUBLISH_HANDOFF_CONTENT_DESCRIPTION, PUBLISH_HANDOFF_DESCRIPTION, @@ -45,6 +49,7 @@ import { ensureGitignoreCoverage, createAskAccounting, fetchSkillMenu, + formatAskResult, checkEnvKeys as checkEnvKeysCore, mergeEnvValues, normaliseAskSubject, @@ -768,7 +773,7 @@ export async function createWizardToolsServer(options: WizardToolsOptions) { content: [ { type: 'text' as const, - text: JSON.stringify({ answers: sanitised }, null, 2), + text: formatAskResult(sanitised, isFullyTimedOut(answers)), }, ], }; diff --git a/src/lib/wizard-tools/tools.ts b/src/lib/wizard-tools/tools.ts index 25b54bc07..5e64a4b42 100644 --- a/src/lib/wizard-tools/tools.ts +++ b/src/lib/wizard-tools/tools.ts @@ -29,7 +29,7 @@ import { type AuditCheck, type AuditStatus, } from '../programs/audit/types'; -import { CANCELLED_SENTINEL } from '../wizard-ask-bridge'; +import { isUnansweredSentinel } from '../wizard-ask-bridge'; import type { SecretVault } from '../secret-vault'; import { fetchWithRetry, type RetryOpts } from '../fetch-retry'; @@ -388,9 +388,46 @@ export const WIZARD_ASK_TOOL_DESCRIPTION = 'than asking one at a time, and tag the call with `subject`. Walking a list — ' + 'one call per data-warehouse source, one call per integration step — is ' + 'expected and is never blocked, because the batching guard counts consecutive ' + - 'calls per subject. A fully cancelled or timed-out response does NOT count ' + - 'against the per-run cap — treat it as "the user declined" and fall back ' + - 'gracefully (e.g. hand over a deep link) without worrying about a wasted call.'; + 'calls per subject. Neither a cancelled nor a timed-out response counts ' + + 'against the per-run cap, so an unanswered call never wastes a slot. The two ' + + 'endings mean different things. Every answer is "__cancelled__" when the user ' + + 'dismissed the prompt: treat that as a decline and fall back gracefully (e.g. ' + + 'hand over a deep link). Every answer is "__timed_out__" when no answer ' + + 'arrived in time: the user is probably still doing the slow work you asked ' + + 'for, so ask the same question again to keep waiting, and never undo or revert ' + + 'a change you already applied.'; + +/** + * Guidance returned beside a fully timed-out `wizard_ask` result, shared by both + * harness facades so the contract cannot drift between them. The sentinel alone + * only says the answer is missing; this says what not to do about it. See + * `TIMED_OUT_SENTINEL` in `wizard-ask-bridge` for why the two endings differ. + */ +export const ASK_TIMED_OUT_NOTE = + 'No answer arrived before the question timed out, so every answer above is ' + + '"__timed_out__". This is a timeout, not a decline. The user is probably still ' + + 'doing the slow work you asked for — a production build, a browser step, a ' + + 'check in PostHog — and is away from the terminal. Do NOT undo, revert, or ' + + 'delete any change you already applied, and do NOT treat the run as declined. ' + + 'Ask the same question again to keep waiting; a timed-out call costs nothing ' + + 'against your call budget. If a later ask comes back as "__cancelled__", the ' + + 'user dismissed it and you may fall back. If two more asks time out, finish the ' + + 'run, leave every change in place, and report the step as unverified.'; + +/** + * Format the `wizard_ask` tool result both harness facades return. A fully + * timed-out ask carries {@link ASK_TIMED_OUT_NOTE} beside the answers; every + * other ending returns the answers alone, exactly as before. + */ +export function formatAskResult( + answers: Record, + timedOut: boolean, +): string { + const payload = timedOut + ? { answers, unanswered_reason: 'timeout', note: ASK_TIMED_OUT_NOTE } + : { answers }; + return JSON.stringify(payload, null, 2); +} export type AskCapDecision = | { kind: 'ok' } @@ -823,8 +860,8 @@ export function mergeEnvValues( /** * Swap sensitive text answers for opaque vault refs before they return to - * the agent — the raw value never enters the LLM conversation. Cancelled - * answers pass through as the sentinel, unvaulted. + * the agent — the raw value never enters the LLM conversation. Unanswered + * answers pass through as their sentinel, unvaulted. */ export function vaultSensitiveAnswers( questions: readonly { id: string; prompt: string; sensitive?: boolean }[], @@ -841,7 +878,7 @@ export function vaultSensitiveAnswers( if ( label !== undefined && typeof answer === 'string' && - answer !== CANCELLED_SENTINEL + !isUnansweredSentinel(answer) ) { const ref = vault.put(answer, { label, source: 'wizard_ask' }); sanitised[id] = { secretRef: ref };