From 831a62ba9b01becef35769d79ce2c75ea41b6ff8 Mon Sep 17 00:00:00 2001 From: Ben Limmer Date: Tue, 26 May 2026 10:34:53 -0600 Subject: [PATCH] fix: streamline CLI share prompts --- src/cli.test.ts | 3 ++- src/cli.ts | 4 +-- src/index.ts | 2 -- src/interactive/sharePrompt.test.ts | 28 +++++++++++++------- src/interactive/sharePrompt.ts | 38 ++++++++++------------------ src/interactive/targetPrompt.test.ts | 2 ++ src/interactive/targetPrompt.ts | 2 +- src/interactive/testFactories.ts | 1 - src/prompt/Prompter.ts | 3 ++- src/testHelpers/FakePrompter.ts | 5 +++- 10 files changed, 45 insertions(+), 43 deletions(-) diff --git a/src/cli.test.ts b/src/cli.test.ts index 5675c47..cef6792 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -51,7 +51,7 @@ test('rejects more than one positional argument', async () => { }); test('writes a report when the GitHub calls succeed', async () => { - const { ctx, githubClient, fs, analytics } = createFakeContext(); + const { ctx, githubClient, fs, analytics, prompter } = createFakeContext(); githubClient.onPaginate('GET /orgs/{org}/repos', {}).resolves([ { @@ -100,6 +100,7 @@ test('writes a report when the GitHub calls succeed', async () => { // The completed run hands the html back so the caller (index.ts) can drive // the share prompt without re-reading the filesystem. expect(result.run.html).toContain(' 0 ? distinctId : 'anonymous'; await runOpenReportPrompt({ context: ctx, htmlPath: result.run.paths.html }); await runSharePrompt({ context: ctx, target: result.run.target, htmlPath: result.run.paths.html, htmlContent: result.run.html, - identifier, }); } diff --git a/src/interactive/sharePrompt.test.ts b/src/interactive/sharePrompt.test.ts index 9a8a2fc..7d6c13a 100644 --- a/src/interactive/sharePrompt.test.ts +++ b/src/interactive/sharePrompt.test.ts @@ -13,7 +13,10 @@ describe('runSharePrompt', () => { const reportReadyNote = handle.prompter.notes.find((n) => n.title === 'Report ready'); expect(reportReadyNote?.message).toContain('acme'); expect(reportReadyNote?.message).toContain('/tmp/report.html'); - expect(handle.prompter.selects[0]?.message).toContain('share this with us'); + expect(reportReadyNote?.message).not.toContain('Open it to see what would be sent'); + expect(handle.prompter.selects[0]?.message).toContain('waitlist'); + expect(handle.prompter.selects[0]?.message).toContain("We'll upload exactly what's on disk"); + expect(handle.prompter.selects[0]?.message).toContain("won't share your data"); expect(handle.prompter.selects[0]?.choices.map((c) => c.value)).toEqual(['html', 'declined']); expect(handle.prompter.selects[0]?.initialValue).toBe('html'); }); @@ -34,34 +37,41 @@ describe('runSharePrompt', () => { test('html-only: uploads the raw html bytes with kind:html', async () => { const handle = fakeContextHandle.build(); - handle.prompter.scriptSelect('html').scriptText(''); + handle.prompter.scriptSelect('html').scriptText('ben@example.com'); const outcome = await runSharePrompt(sharePromptInputsFor(handle)); - expect(outcome).toMatchObject({ kind: 'shared', identifier: 'anon-uuid' }); + expect(outcome).toMatchObject({ kind: 'shared', identifier: 'ben@example.com' }); expect(handle.uploader.calls).toHaveLength(1); expect(handle.uploader.calls[0]).toMatchObject({ - identifier: 'anon-uuid', + identifier: 'ben@example.com', appVersion: '0.0.1', timestamp: '2026-05-22T12:00:00Z', }); expect(new TextDecoder().decode(handle.uploader.calls[0]?.bytes)).toBe(''); expect(handle.analytics.capturedEvents('upload_succeeded')).toHaveLength(1); + expect(handle.prompter.outros[0]).toContain("you're on the PatchWave waitlist"); + expect(handle.prompter.outros[0]).toContain('ben@example.com'); + expect(handle.prompter.outros[0]?.toLowerCase()).not.toContain('upload id'); + expect(handle.prompter.outros[0]).not.toContain('fake-upload-id'); }); - test('uses a volunteered email as the identifier', async () => { + test('email prompt requires a valid email address', async () => { const handle = fakeContextHandle.build(); handle.prompter.scriptSelect('html').scriptText('ben@example.com'); - const outcome = await runSharePrompt(sharePromptInputsFor(handle)); + await runSharePrompt(sharePromptInputsFor(handle)); - expect(outcome).toMatchObject({ kind: 'shared', identifier: 'ben@example.com' }); - expect(handle.uploader.calls[0]?.identifier).toBe('ben@example.com'); + const validate = handle.prompter.texts[0]?.validate; + expect(handle.prompter.texts[0]?.message).toBe('Email:'); + expect(validate?.('')).toContain('email'); + expect(validate?.('not an email')).toBeDefined(); + expect(validate?.('ben@example.com')).toBeUndefined(); }); test('upload failure surfaces the error and leaves the report in place', async () => { const handle = fakeContextHandle.build(); - handle.prompter.scriptSelect('html').scriptText(''); + handle.prompter.scriptSelect('html').scriptText('ben@example.com'); handle.uploader.fails({ kind: 'presign-bad-status', status: 500, body: 'boom' }); const outcome = await runSharePrompt(sharePromptInputsFor(handle)); diff --git a/src/interactive/sharePrompt.ts b/src/interactive/sharePrompt.ts index 8504550..bc424d8 100644 --- a/src/interactive/sharePrompt.ts +++ b/src/interactive/sharePrompt.ts @@ -11,7 +11,6 @@ export interface SharePromptInputs { readonly target: string; readonly htmlPath: string; readonly htmlContent: string; - readonly identifier: string; } export type ShareOutcome = @@ -23,23 +22,16 @@ export type ShareOutcome = export async function runSharePrompt(inputs: SharePromptInputs): Promise { const { prompter, analytics, uploader } = inputs.context; - prompter.note( - [ - `Scanned: ${inputs.target}`, - `HTML report: ${inputs.htmlPath}`, - '', - "Open it to see what would be sent — we'll upload exactly what's on disk.", - ].join('\n'), - 'Report ready', - ); + prompter.note([`Scanned: ${inputs.target}`, `HTML report: ${inputs.htmlPath}`].join('\n'), 'Report ready'); analytics.capture('share_prompt_shown', {}); const choiceResult = await prompter.select({ - message: "Would you like to share this with us? We won't share your data with anyone.", + message: + "Share this report with PatchWave? Uploading it bumps your spot on the waitlist. We'll upload exactly what's on disk and won't share your data with anyone.", initialValue: 'html', choices: [ - { value: 'html', label: 'Share the HTML report', hint: 'the .html you saw above' }, + { value: 'html', label: 'Share the HTML report', hint: 'boost your waitlist spot' }, { value: 'declined', label: 'No thanks — keep it local', hint: 'nothing leaves your machine' }, ], }); @@ -59,7 +51,7 @@ export async function runSharePrompt(inputs: SharePromptInputs): Promise { +async function askForEmail(prompter: Prompter): Promise<{ kind: 'ok'; identifier: string } | { kind: 'cancelled' }> { const result = await prompter.text({ - message: 'Email (optional, so we can follow up):', - placeholder: 'leave blank to stay anonymous', - defaultValue: '', + message: 'Email:', + placeholder: 'you@example.com', validate: (value) => { const trimmed = value.trim(); - if (trimmed.length === 0) return undefined; + if (trimmed.length === 0) return 'Please enter an email address for the waitlist.'; return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(trimmed) ? undefined : "that doesn't look like an email address"; }, }); @@ -126,9 +116,9 @@ async function maybeAskForEmail( if (result.isErr()) { if (result.error.kind === 'cancelled') return { kind: 'cancelled' }; prompter.warn(formatPromptError(result.error)); - return { kind: 'ok', identifier: fallbackIdentifier }; + return { kind: 'cancelled' }; } const trimmed = result.value.trim(); - return { kind: 'ok', identifier: trimmed.length > 0 ? trimmed : fallbackIdentifier }; + return { kind: 'ok', identifier: trimmed }; } diff --git a/src/interactive/targetPrompt.test.ts b/src/interactive/targetPrompt.test.ts index 5db24dc..8d6d3de 100644 --- a/src/interactive/targetPrompt.test.ts +++ b/src/interactive/targetPrompt.test.ts @@ -27,6 +27,8 @@ describe('promptForTarget', () => { expect(choices[0]?.hint).toContain('personal'); expect(choices[1]?.hint).toContain('organization'); expect(choices.at(-1)?.label).toContain('Other'); + expect(prompter.spinnerEvents).toContainEqual({ type: 'clear' }); + expect(prompter.spinnerEvents).not.toContainEqual({ type: 'stop', message: 'Found 3 options.' }); }); test('"Other" routes to a free-text prompt with login validation', async () => { diff --git a/src/interactive/targetPrompt.ts b/src/interactive/targetPrompt.ts index 70aa9b7..f47072f 100644 --- a/src/interactive/targetPrompt.ts +++ b/src/interactive/targetPrompt.ts @@ -26,7 +26,7 @@ export function promptForTarget(deps: TargetPromptDeps): ResultAsync(() => { target: 'acme', htmlPath: '/tmp/report.html', htmlContent: '', - identifier: 'anon-uuid', }; }); diff --git a/src/prompt/Prompter.ts b/src/prompt/Prompter.ts index e5d721c..fa79144 100644 --- a/src/prompt/Prompter.ts +++ b/src/prompt/Prompter.ts @@ -32,6 +32,7 @@ export interface TextOptions { export interface PromptSpinner { start(msg?: string): void; stop(msg?: string): void; + clear(): void; } export interface Prompter { @@ -105,7 +106,7 @@ export class PrompterImpl implements Prompter { spinner(): PromptSpinner { const s = clack.spinner(); - return { start: (m) => s.start(m), stop: (m) => s.stop(m) }; + return { start: (m) => s.start(m), stop: (m) => s.stop(m), clear: () => s.clear() }; } } diff --git a/src/testHelpers/FakePrompter.ts b/src/testHelpers/FakePrompter.ts index 9819fd6..ff23fd0 100644 --- a/src/testHelpers/FakePrompter.ts +++ b/src/testHelpers/FakePrompter.ts @@ -21,7 +21,7 @@ type TextAnswer = ScriptedAnswer<'text', string>; type Answer = ConfirmAnswer | SelectAnswer | TextAnswer; export interface SpinnerEvent { - readonly type: 'start' | 'stop'; + readonly type: 'start' | 'stop' | 'clear'; readonly message?: string; } @@ -107,6 +107,9 @@ export class FakePrompter implements Prompter { stop: (message?: string) => { this.spinnerEvents.push({ type: 'stop', message }); }, + clear: () => { + this.spinnerEvents.push({ type: 'clear' }); + }, }; }