From 277eb4ea8927751a2080d652ec3554d3cb428088 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 17:36:44 +0000 Subject: [PATCH] fix(tui): release the pending ask when a screen crashes, and mask secrets A render throw on the wizard_ask overlay routed the run to the outro but left the ask pending: the agent stayed parked on a promise nobody resolved, and the store rejected every later wizard_ask as a duplicate. Cancel the ask first, so the agent gets the same sentinel as an Esc and can fall back. Mask sensitive text answers with PasswordInput, so a pasted personal API key does not land in the terminal scrollback. Turn on richLinks for upload-source-maps, so the key-creation URL in its first prompt is a click target and gets copied to the clipboard. Generated-By: PostHog Desktop Task-Id: 724efebf-7b36-4d03-8841-d207f0d40fb4 --- .../index.ts | 4 ++ src/ui/tui/__tests__/WizardAskScreen.test.ts | 30 ++++++++++ .../__tests__/screen-error-boundary.test.ts | 57 +++++++++++++++++++ src/ui/tui/primitives/ScreenErrorBoundary.tsx | 30 +++++++--- src/ui/tui/screens/WizardAskScreen.tsx | 26 +++++++-- 5 files changed, 133 insertions(+), 14 deletions(-) create mode 100644 src/ui/tui/__tests__/screen-error-boundary.test.ts 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..0c8223928 100644 --- a/src/lib/programs/error-tracking-upload-source-maps/index.ts +++ b/src/lib/programs/error-tracking-upload-source-maps/index.ts @@ -97,6 +97,10 @@ export const errorTrackingUploadSourceMapsConfig: ProgramConfig = { // The 5-minute default cancels the question mid-task and the agent // wraps up to the outro, so give these answers half an hour. askTimeoutMs: 30 * 60 * 1000, + // STEP 1 hands the user the personal-API-key settings URL inside the ask + // prompt. Render it as an OSC 8 hyperlink and copy it to the clipboard, + // so the user can reach the page the answer depends on. + richLinks: true, customPrompt: (ctx) => { const { variant, displayName, projectPath, skillId } = readSelection(); diff --git a/src/ui/tui/__tests__/WizardAskScreen.test.ts b/src/ui/tui/__tests__/WizardAskScreen.test.ts index f210f3886..7a2900e68 100644 --- a/src/ui/tui/__tests__/WizardAskScreen.test.ts +++ b/src/ui/tui/__tests__/WizardAskScreen.test.ts @@ -19,10 +19,13 @@ vi.mock('../../../utils/analytics.js', () => ({ sessionProperties: vi.fn(() => ({})), })); +import { PasswordInput, TextInput } from '@inkjs/ui'; +import { isValidElement, type ReactNode } from 'react'; import { WizardStore } from '@ui/tui/store'; import { handleAskKey, isRequiredButEmpty, + QuestionInput, } from '@ui/tui/screens/WizardAskScreen'; const pending = { @@ -84,3 +87,30 @@ describe('isRequiredButEmpty', () => { expect(isRequiredButEmpty({}, ['events'])).toBe(false); }); }); + +/** Every component type in a rendered element tree, in no particular order. */ +function componentTypes(node: ReactNode): unknown[] { + if (Array.isArray(node)) return node.flatMap(componentTypes); + if (!isValidElement(node)) return []; + const children = (node.props as { children?: ReactNode }).children; + return [node.type, ...componentTypes(children)]; +} + +describe('QuestionInput', () => { + it('masks a sensitive answer so the credential stays out of the scrollback', () => { + const types = componentTypes( + QuestionInput({ + question: { + id: 'api-key', + prompt: 'Paste your personal API key', + kind: 'text', + sensitive: true, + }, + onSubmit: vi.fn(), + }), + ); + + expect(types).toContain(PasswordInput); + expect(types).not.toContain(TextInput); + }); +}); diff --git a/src/ui/tui/__tests__/screen-error-boundary.test.ts b/src/ui/tui/__tests__/screen-error-boundary.test.ts new file mode 100644 index 000000000..1dcaa150c --- /dev/null +++ b/src/ui/tui/__tests__/screen-error-boundary.test.ts @@ -0,0 +1,57 @@ +/** + * ScreenErrorBoundary — a crashed screen must release the run. + * + * The `wizard_ask` overlay is a screen, so a render throw can land while the + * agent is parked on an ask. The boundary routes to the outro; if it leaves the + * ask pending, the agent waits on a promise nobody resolves and every later ask + * is rejected as a duplicate. + */ + +import { vi } from 'vitest'; + +vi.mock('../../../utils/analytics.js', () => ({ + analytics: { + capture: vi.fn(), + wizardCapture: vi.fn(), + setTag: vi.fn(), + shutdown: vi.fn().mockResolvedValue(undefined), + }, + sessionProperties: vi.fn(() => ({})), +})); + +import { WizardStore } from '@ui/tui/store'; +import { handleScreenCrash } from '@ui/tui/primitives/ScreenErrorBoundary'; +import { OutroKind, RunPhase } from '@lib/wizard-session'; + +const pending = { + id: 'req-1', + source: 'error-tracking-upload-source-maps', + questions: [ + { id: 'api-key', prompt: 'Paste your key', kind: 'text' as const }, + ], +}; + +describe('handleScreenCrash', () => { + it('releases an in-flight ask and routes to the outro', async () => { + const store = new WizardStore(); + const answers = store.requestQuestion(pending); + + handleScreenCrash(store, new Error('boom')); + + await expect(answers).resolves.toEqual({ 'api-key': '__cancelled__' }); + expect(store.session.pendingQuestion).toBeNull(); + expect(store.session.runPhase).toBe(RunPhase.Error); + expect(store.session.outroData?.kind).toBe(OutroKind.Error); + }); + + it('leaves the next ask free to open', () => { + const store = new WizardStore(); + void store.requestQuestion(pending); + + handleScreenCrash(store, new Error('boom')); + + expect(() => + store.requestQuestion({ ...pending, id: 'req-2' }), + ).not.toThrow(); + }); +}); diff --git a/src/ui/tui/primitives/ScreenErrorBoundary.tsx b/src/ui/tui/primitives/ScreenErrorBoundary.tsx index 6320b7a37..d60efbd94 100644 --- a/src/ui/tui/primitives/ScreenErrorBoundary.tsx +++ b/src/ui/tui/primitives/ScreenErrorBoundary.tsx @@ -20,6 +20,27 @@ interface State { error: Error | null; } +/** + * Route a crashed screen to the outro. + * + * The ask overlay is a screen like any other, so a render throw can land while + * a `wizard_ask` request is in flight. Release it first: the agent is parked on + * that promise, and the store rejects the next `wizard_ask` while one is still + * pending, so a crash that leaves it set wedges the rest of the run. Cancelling + * hands the agent the same sentinel as an Esc, which every task already treats + * as "the user declined" and falls back on. No-op when nothing is pending. + * + * Extracted from the boundary so it is testable without a live Ink render. + */ +export function handleScreenCrash(store: WizardStore, error: Error): void { + store.cancelPendingQuestion(); + store.setOutroData({ + kind: OutroKind.Error, + message: `A screen crashed: ${error.message}`, + }); + store.setRunPhase(RunPhase.Error); +} + export class ScreenErrorBoundary extends Component { state: State = { error: null }; @@ -28,19 +49,12 @@ export class ScreenErrorBoundary extends Component { } componentDidCatch(error: Error): void { - const { store } = this.props; - // The console.error below is wiped with the alt screen; this survives. logToFile('[screen-error-boundary]', error); // eslint-disable-next-line no-console console.error('[ScreenErrorBoundary]', error.message, error.stack); - // Set error state — the router will resolve to outro - store.setOutroData({ - kind: OutroKind.Error, - message: `A screen crashed: ${error.message}`, - }); - store.setRunPhase(RunPhase.Error); + handleScreenCrash(this.props.store, error); } render(): ReactNode { diff --git a/src/ui/tui/screens/WizardAskScreen.tsx b/src/ui/tui/screens/WizardAskScreen.tsx index e47ba6435..95932565e 100644 --- a/src/ui/tui/screens/WizardAskScreen.tsx +++ b/src/ui/tui/screens/WizardAskScreen.tsx @@ -7,7 +7,7 @@ */ import { Box, Text, useInput } from 'ink'; -import { TextInput } from '@inkjs/ui'; +import { PasswordInput, TextInput } from '@inkjs/ui'; import { useEffect, useState, useSyncExternalStore } from 'react'; import type { WizardStore } from '@ui/tui/store'; import { @@ -264,7 +264,11 @@ interface QuestionInputProps { onSubmit: (value: string | string[]) => void; } -const QuestionInput = ({ question, onSubmit }: QuestionInputProps) => { +/** + * Renders the input for one question. Exported so the sensitive-answer path can + * be asserted without a live Ink render. + */ +export const QuestionInput = ({ question, onSubmit }: QuestionInputProps) => { switch (question.kind) { case 'single': return ( @@ -310,10 +314,20 @@ const QuestionInput = ({ question, onSubmit }: QuestionInputProps) => { // to fit its widest child, so the right-aligned hint walks left/right // as the typed text changes width. - onSubmit(value)} - /> + {/* A sensitive answer is a live credential the agent never sees in + the clear (it is vaulted as a secretRef). Echoing it would still + leave it in the terminal scrollback, so mask the input too. */} + {question.sensitive ? ( + onSubmit(value)} + /> + ) : ( + onSubmit(value)} + /> + )} ENTER