From 4847b8b7e91d111a692cb60e708c583c75a064a8 Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Sat, 22 Aug 2026 12:40:06 +0300 Subject: [PATCH 1/5] wip: AIT-458 feedback tool From 1e0213dbd1ebc4b9cce6e0cc3caa613377f97d90 Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Sat, 22 Aug 2026 12:46:01 +0300 Subject: [PATCH 2/5] =?UTF-8?q?AIT-458:=20hookmyapp=20feedback=20=E2=80=94?= =?UTF-8?q?=20one-way=20friction=20report?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Separate command rather than a flag on `support new`: agents route on descriptions, and nothing in a support description fires when the human is merely confused. The description carries the negative clause (broken thing or waiting human means open a ticket instead). Governed by the existing telemetry switch, so there is no second consent surface to explain: telemetry off means nothing leaves the machine. --- src/commands/__tests__/feedback.test.ts | 63 +++++++++++++++++++++++++ src/commands/support.ts | 55 +++++++++++++++++++++ src/index.ts | 3 +- 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 src/commands/__tests__/feedback.test.ts diff --git a/src/commands/__tests__/feedback.test.ts b/src/commands/__tests__/feedback.test.ts new file mode 100644 index 0000000..fa3ce9b --- /dev/null +++ b/src/commands/__tests__/feedback.test.ts @@ -0,0 +1,63 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { Command } from 'commander'; + +vi.mock('../../api/client.js', () => ({ apiClient: vi.fn() })); +vi.mock('../../observability/telemetry.js', () => ({ isTelemetryEnabled: vi.fn().mockReturnValue(true) })); + +const mockConsoleLog = vi.spyOn(console, 'log').mockImplementation(() => {}); + +import { apiClient } from '../../api/client.js'; +import { isTelemetryEnabled } from '../../observability/telemetry.js'; +import { registerFeedbackCommand } from '../support.js'; + +const mockedApi = vi.mocked(apiClient); +const mockedTelemetry = vi.mocked(isTelemetryEnabled); + +function makeProgram(): Command { + const program = new Command(); + program.exitOverride(); + program.option('--json'); + registerFeedbackCommand(program); + return program; +} + +beforeEach(() => { + mockedApi.mockReset(); + mockedTelemetry.mockReturnValue(true); + mockConsoleLog.mockClear(); +}); + +/** AIT-458 — one-way friction report, gated by the telemetry switch. */ +describe('hookmyapp feedback', () => { + it('posts the message plus surface and echoes the no-reply note', async () => { + mockedApi.mockResolvedValue({ ticketId: 'sup_7', note: 'no reply is coming' }); + await makeProgram().parseAsync(['feedback', 'gave up on the connect flow', '--surface', 'docs'], { from: 'user' }); + + expect(mockedApi).toHaveBeenCalledWith('/support/feedback', { + method: 'POST', + body: JSON.stringify({ message: 'gave up on the connect flow', surface: 'docs' }), + }); + expect(mockConsoleLog.mock.calls[0][0]).toContain('sup_7'); + expect(mockConsoleLog.mock.calls[0][0]).toContain('no reply is coming'); + }); + + it('defaults the surface to cli', async () => { + mockedApi.mockResolvedValue({ ticketId: 'sup_8', note: 'n' }); + await makeProgram().parseAsync(['feedback', 'confusing error'], { from: 'user' }); + expect(JSON.parse((mockedApi.mock.calls[0][1] as { body: string }).body).surface).toBe('cli'); + }); + + it('sends nothing when telemetry is off', async () => { + mockedTelemetry.mockReturnValue(false); + await makeProgram().parseAsync(['feedback', 'confusing error'], { from: 'user' }); + expect(mockedApi).not.toHaveBeenCalled(); + expect(mockConsoleLog.mock.calls[0][0]).toContain('config set telemetry on'); + }); + + it('rejects an unknown surface before calling the API', async () => { + await expect( + makeProgram().parseAsync(['feedback', 'x', '--surface', 'carrier-pigeon'], { from: 'user' }), + ).rejects.toThrow(/--surface must be one of/); + expect(mockedApi).not.toHaveBeenCalled(); + }); +}); diff --git a/src/commands/support.ts b/src/commands/support.ts index ec0e14d..e8b94eb 100644 --- a/src/commands/support.ts +++ b/src/commands/support.ts @@ -3,6 +3,7 @@ import { apiClient } from '../api/client.js'; import { output } from '../output/format.js'; import { NetworkError, ValidationError } from '../output/error.js'; import { addExamples } from '../output/help.js'; +import { isTelemetryEnabled } from '../observability/telemetry.js'; /** * AIT-337 — `hookmyapp support`: open and follow support tickets from the CLI @@ -358,3 +359,57 @@ EXAMPLES: `, ); } + +/** + * AIT-458 — `hookmyapp feedback`: one-way friction report for the agent driving + * this CLI. Separate command, not `support new --kind`, because agents route on + * descriptions: nothing in a support description fires when the human is merely + * confused. Governed by the existing telemetry switch — no second consent + * surface, and telemetry off means nothing leaves the machine. + */ +export function registerFeedbackCommand(program: Command): void { + const feedback = program + .command('feedback') + .description( + 'Report friction you observed: the human got confused, repeated themselves, misread an error, ' + + 'abandoned a flow, or declined an upgrade after hitting a plan limit. One-way — nobody replies. ' + + 'If something is broken or they need an answer, use `hookmyapp support new` instead.', + ) + .argument('[message]', 'What they were trying to do and what confused them (a summary, never a transcript)') + .option('--surface ', 'Where it happened: cli, mcp, docs, dashboard, api', 'cli') + .option('--json', 'Output machine-readable JSON') + .action(async (message: string | undefined, opts: { surface: string; json?: boolean }) => { + const body = message ?? (await readStdinBody()); + if (!body) throw new ValidationError('Provide the feedback as an argument or pipe it on stdin.'); + if (!SURFACES.includes(opts.surface)) { + throw new ValidationError(`--surface must be one of: ${SURFACES.join(', ')}.`); + } + if (!isTelemetryEnabled()) { + // Same switch as crash reporting: off means nothing leaves the machine. + const note = 'Telemetry is off, so nothing was sent. Turn it on: hookmyapp config set telemetry on'; + console.log(opts.json || program.opts().json ? JSON.stringify({ sent: false, note }, null, 2) : note); + return; + } + const res = (await apiClient('/support/feedback', { + method: 'POST', + body: JSON.stringify({ message: body, surface: opts.surface }), + })) as { ticketId: string; note: string }; + if (opts.json || program.opts().json) { + console.log(JSON.stringify({ sent: true, ...res }, null, 2)); + return; + } + console.log(`Thanks — recorded as ${res.ticketId}. ${res.note}`); + }); + + addExamples( + feedback, + ` +EXAMPLES: + $ hookmyapp feedback "Spent 20 minutes on the connect flow; read 'pending' as an error and nearly gave up." + $ hookmyapp feedback "Hit the plan limit and decided not to upgrade — said the price is too high for their volume." + $ hookmyapp feedback --surface docs "The webhook signature page never says which header carries the timestamp." +`, + ); +} + +const SURFACES = ['cli', 'mcp', 'docs', 'dashboard', 'api']; diff --git a/src/index.ts b/src/index.ts index 51e4ff9..36f2917 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,7 +15,7 @@ import { registerAlertsCommand } from './commands/alerts.js'; import { registerBillingCommand } from './commands/billing.js'; import { registerWorkspaceCommand } from './commands/workspace.js'; import { registerCustomersCommand } from './commands/customers.js'; -import { registerSupportCommand } from './commands/support.js'; +import { registerSupportCommand, registerFeedbackCommand } from './commands/support.js'; import { registerNotificationsCommand } from './commands/notifications.js'; import { registerOrgProfileCommand } from './commands/org-profile.js'; import { registerSandboxCommand } from './commands/sandbox/index.js'; @@ -196,6 +196,7 @@ registerWorkspaceCommand(program); // Customers (customer workspaces) registerCustomersCommand(program); registerSupportCommand(program); +registerFeedbackCommand(program); registerNotificationsCommand(program); registerOrgProfileCommand(program); From 6f9a9728d79d13e2963414dd76447f9bb8e3190b Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Sat, 22 Aug 2026 12:47:11 +0300 Subject: [PATCH 3/5] AIT-458: name feedback in the telemetry disclosure Feedback rides the telemetry switch, so the disclosure has to say so. Bumps the disclosure version to 3 so existing installs see the changed text once. --- src/observability/telemetry.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/observability/telemetry.ts b/src/observability/telemetry.ts index efeae37..a1e1c6a 100644 --- a/src/observability/telemetry.ts +++ b/src/observability/telemetry.ts @@ -21,8 +21,9 @@ import { getConfigFile, safeWriteFileSync } from '../storage/path.js'; type TelemetryFlag = 'on' | 'off'; // Bump when the disclosure text materially changes what is collected (v2: -// account email + user id, AIT-278) so existing installs see it again. -const DISCLOSURE_VERSION = 2; +// account email + user id, AIT-278; v3: agent-filed friction reports, +// AIT-458) so existing installs see it again. +const DISCLOSURE_VERSION = 3; interface Config { telemetry?: TelemetryFlag; @@ -100,6 +101,7 @@ export function maybePrintFirstRunDisclosure(): void { 'ℹ Telemetry: HookMyApp CLI reports crashes + usage analytics to help us fix bugs and improve UX.', ' No command arguments, file contents, or env var values are sent.', ' When logged in, your account email + user id accompany crash reports.', + ' Also covers friction your coding agent reports with `hookmyapp feedback`.', ' Disable: `hookmyapp config set telemetry off` or `HOOKMYAPP_TELEMETRY=off`', '', ].join('\n'), From f87965867b2712ba6912c43885918deb50b58b2a Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Mon, 24 Aug 2026 14:18:08 +0300 Subject: [PATCH 4/5] AIT-458: address review on the feedback command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The disclosure claimed no command arguments are ever sent, two lines above a new line about a command whose whole argument is sent. Say the exception. - The disclosure only printed from the Sentry init path, so a CLI built without a DSN would upload the message having never shown it. Print it at the moment the data actually leaves. - Bare `hookmyapp feedback` blocked forever on stdin — the command has no required option, so that is the likely typo. Error on a TTY instead. - Validate --surface and the telemetry switch before reading stdin, so a typo reports itself instead of buffering a pipe. - Guard the optional note, matching how printDetail already treats it. --- src/commands/__tests__/feedback.test.ts | 32 +++++++++++++++++++++++-- src/commands/support.ts | 21 ++++++++++++---- src/observability/telemetry.ts | 4 ++-- 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/commands/__tests__/feedback.test.ts b/src/commands/__tests__/feedback.test.ts index fa3ce9b..9d4645f 100644 --- a/src/commands/__tests__/feedback.test.ts +++ b/src/commands/__tests__/feedback.test.ts @@ -2,12 +2,15 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { Command } from 'commander'; vi.mock('../../api/client.js', () => ({ apiClient: vi.fn() })); -vi.mock('../../observability/telemetry.js', () => ({ isTelemetryEnabled: vi.fn().mockReturnValue(true) })); +vi.mock('../../observability/telemetry.js', () => ({ + isTelemetryEnabled: vi.fn().mockReturnValue(true), + maybePrintFirstRunDisclosure: vi.fn(), +})); const mockConsoleLog = vi.spyOn(console, 'log').mockImplementation(() => {}); import { apiClient } from '../../api/client.js'; -import { isTelemetryEnabled } from '../../observability/telemetry.js'; +import { isTelemetryEnabled, maybePrintFirstRunDisclosure } from '../../observability/telemetry.js'; import { registerFeedbackCommand } from '../support.js'; const mockedApi = vi.mocked(apiClient); @@ -54,6 +57,31 @@ describe('hookmyapp feedback', () => { expect(mockConsoleLog.mock.calls[0][0]).toContain('config set telemetry on'); }); + it('shows the telemetry disclosure before the message leaves the machine', async () => { + mockedApi.mockResolvedValue({ ticketId: 'sup_9', note: 'n' }); + await makeProgram().parseAsync(['feedback', 'confusing'], { from: 'user' }); + // Must not depend on Sentry having initialized — a CLI built without a DSN + // would otherwise upload the message with no disclosure ever shown. + expect(maybePrintFirstRunDisclosure).toHaveBeenCalled(); + }); + + it('errors instead of blocking on stdin when run bare in a terminal', async () => { + const wasTty = process.stdin.isTTY; + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + try { + await expect(makeProgram().parseAsync(['feedback'], { from: 'user' })).rejects.toThrow(/argument or pipe/); + expect(mockedApi).not.toHaveBeenCalled(); + } finally { + Object.defineProperty(process.stdin, 'isTTY', { value: wasTty, configurable: true }); + } + }); + + it('prints cleanly when the backend omits the note', async () => { + mockedApi.mockResolvedValue({ ticketId: 'sup_10' }); + await makeProgram().parseAsync(['feedback', 'confusing'], { from: 'user' }); + expect(mockConsoleLog.mock.calls[0][0]).toBe('Thanks — recorded as sup_10.'); + }); + it('rejects an unknown surface before calling the API', async () => { await expect( makeProgram().parseAsync(['feedback', 'x', '--surface', 'carrier-pigeon'], { from: 'user' }), diff --git a/src/commands/support.ts b/src/commands/support.ts index e8b94eb..2ee4f43 100644 --- a/src/commands/support.ts +++ b/src/commands/support.ts @@ -3,7 +3,7 @@ import { apiClient } from '../api/client.js'; import { output } from '../output/format.js'; import { NetworkError, ValidationError } from '../output/error.js'; import { addExamples } from '../output/help.js'; -import { isTelemetryEnabled } from '../observability/telemetry.js'; +import { isTelemetryEnabled, maybePrintFirstRunDisclosure } from '../observability/telemetry.js'; /** * AIT-337 — `hookmyapp support`: open and follow support tickets from the CLI @@ -379,8 +379,8 @@ export function registerFeedbackCommand(program: Command): void { .option('--surface ', 'Where it happened: cli, mcp, docs, dashboard, api', 'cli') .option('--json', 'Output machine-readable JSON') .action(async (message: string | undefined, opts: { surface: string; json?: boolean }) => { - const body = message ?? (await readStdinBody()); - if (!body) throw new ValidationError('Provide the feedback as an argument or pipe it on stdin.'); + // Validate and check the switch BEFORE touching stdin: otherwise a typo'd + // --surface blocks on a pipe instead of reporting itself. if (!SURFACES.includes(opts.surface)) { throw new ValidationError(`--surface must be one of: ${SURFACES.join(', ')}.`); } @@ -390,15 +390,26 @@ export function registerFeedbackCommand(program: Command): void { console.log(opts.json || program.opts().json ? JSON.stringify({ sent: false, note }, null, 2) : note); return; } + // `feedback` has no required option, so a bare invocation is the likely + // typo — never silently block forever on an interactive terminal. + if (message === undefined && process.stdin.isTTY) { + throw new ValidationError('Provide the feedback as an argument or pipe it on stdin.'); + } + const body = message ?? (await readStdinBody()); + if (!body) throw new ValidationError('Provide the feedback as an argument or pipe it on stdin.'); + // This is the moment data leaves the machine, so it is the moment the + // disclosure has to have been shown — it cannot depend on Sentry having + // initialized (no DSN in a local or self-built CLI means no banner). + maybePrintFirstRunDisclosure(); const res = (await apiClient('/support/feedback', { method: 'POST', body: JSON.stringify({ message: body, surface: opts.surface }), - })) as { ticketId: string; note: string }; + })) as { ticketId: string; note?: string }; if (opts.json || program.opts().json) { console.log(JSON.stringify({ sent: true, ...res }, null, 2)); return; } - console.log(`Thanks — recorded as ${res.ticketId}. ${res.note}`); + console.log(`Thanks — recorded as ${res.ticketId}.${res.note ? ` ${res.note}` : ''}`); }); addExamples( diff --git a/src/observability/telemetry.ts b/src/observability/telemetry.ts index a1e1c6a..14284a9 100644 --- a/src/observability/telemetry.ts +++ b/src/observability/telemetry.ts @@ -99,9 +99,9 @@ export function maybePrintFirstRunDisclosure(): void { [ '', 'ℹ Telemetry: HookMyApp CLI reports crashes + usage analytics to help us fix bugs and improve UX.', - ' No command arguments, file contents, or env var values are sent.', + ' No command arguments, file contents, or env var values are sent — except the', + ' message you pass to `hookmyapp feedback`, which is sent on purpose.', ' When logged in, your account email + user id accompany crash reports.', - ' Also covers friction your coding agent reports with `hookmyapp feedback`.', ' Disable: `hookmyapp config set telemetry off` or `HOOKMYAPP_TELEMETRY=off`', '', ].join('\n'), From afb676257f600ed79fa6958335f5ee3fe37fa03a Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Mon, 24 Aug 2026 14:46:38 +0300 Subject: [PATCH 5/5] AIT-458: address the Codex review of the CLI - --surface defaulted to 'cli' and was always sent, so friction the human hit in the docs or the dashboard was recorded as CLI friction. Omit when unknown. - maybePrintFirstRunDisclosure() throws on a read-only config dir, which would have blocked the submission entirely. Fail open, as the Sentry call site does. - The disclosure named only the message; --surface is uploaded too. Say both. - The help said 'never a transcript', which does not exclude tokens, PII, or customer message content. Carry the same prohibition support new has. --- src/commands/__tests__/feedback.test.ts | 15 +++++++++++++-- src/commands/support.ts | 21 +++++++++++++++------ src/observability/telemetry.ts | 2 +- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/commands/__tests__/feedback.test.ts b/src/commands/__tests__/feedback.test.ts index 9d4645f..493a476 100644 --- a/src/commands/__tests__/feedback.test.ts +++ b/src/commands/__tests__/feedback.test.ts @@ -44,10 +44,21 @@ describe('hookmyapp feedback', () => { expect(mockConsoleLog.mock.calls[0][0]).toContain('no reply is coming'); }); - it('defaults the surface to cli', async () => { + it('omits the surface when not given — the calling CLI is not where the friction happened', async () => { mockedApi.mockResolvedValue({ ticketId: 'sup_8', note: 'n' }); await makeProgram().parseAsync(['feedback', 'confusing error'], { from: 'user' }); - expect(JSON.parse((mockedApi.mock.calls[0][1] as { body: string }).body).surface).toBe('cli'); + expect(JSON.parse((mockedApi.mock.calls[0][1] as { body: string }).body)).toEqual({ + message: 'confusing error', + }); + }); + + it('still sends when the disclosure cannot persist its flag', async () => { + vi.mocked(maybePrintFirstRunDisclosure).mockImplementationOnce(() => { + throw new Error('EROFS: read-only file system'); + }); + mockedApi.mockResolvedValue({ ticketId: 'sup_11', note: 'n' }); + await makeProgram().parseAsync(['feedback', 'confusing'], { from: 'user' }); + expect(mockedApi).toHaveBeenCalled(); }); it('sends nothing when telemetry is off', async () => { diff --git a/src/commands/support.ts b/src/commands/support.ts index 2ee4f43..5e92e45 100644 --- a/src/commands/support.ts +++ b/src/commands/support.ts @@ -373,15 +373,16 @@ export function registerFeedbackCommand(program: Command): void { .description( 'Report friction you observed: the human got confused, repeated themselves, misread an error, ' + 'abandoned a flow, or declined an upgrade after hitting a plan limit. One-way — nobody replies. ' + - 'If something is broken or they need an answer, use `hookmyapp support new` instead.', + 'Summarize what happened; do not include secrets, tokens, personal data, or your customers’ ' + + 'message content. If something is broken or they need an answer, use `hookmyapp support new` instead.', ) .argument('[message]', 'What they were trying to do and what confused them (a summary, never a transcript)') - .option('--surface ', 'Where it happened: cli, mcp, docs, dashboard, api', 'cli') + .option('--surface ', 'Where it happened: cli, mcp, docs, dashboard, api') .option('--json', 'Output machine-readable JSON') - .action(async (message: string | undefined, opts: { surface: string; json?: boolean }) => { + .action(async (message: string | undefined, opts: { surface?: string; json?: boolean }) => { // Validate and check the switch BEFORE touching stdin: otherwise a typo'd // --surface blocks on a pipe instead of reporting itself. - if (!SURFACES.includes(opts.surface)) { + if (opts.surface !== undefined && !SURFACES.includes(opts.surface)) { throw new ValidationError(`--surface must be one of: ${SURFACES.join(', ')}.`); } if (!isTelemetryEnabled()) { @@ -400,10 +401,18 @@ export function registerFeedbackCommand(program: Command): void { // This is the moment data leaves the machine, so it is the moment the // disclosure has to have been shown — it cannot depend on Sentry having // initialized (no DSN in a local or self-built CLI means no banner). - maybePrintFirstRunDisclosure(); + // Fail-open like the Sentry call site: on a read-only config dir the + // write throws, and that must not be what stops feedback being sent. + try { + maybePrintFirstRunDisclosure(); + } catch { + // banner already printed; the persisted flag is the only casualty + } const res = (await apiClient('/support/feedback', { method: 'POST', - body: JSON.stringify({ message: body, surface: opts.surface }), + // Omit when unknown: the CLI is where the call came FROM, not + // necessarily where the human hit the friction (docs, dashboard, …). + body: JSON.stringify({ message: body, ...(opts.surface ? { surface: opts.surface } : {}) }), })) as { ticketId: string; note?: string }; if (opts.json || program.opts().json) { console.log(JSON.stringify({ sent: true, ...res }, null, 2)); diff --git a/src/observability/telemetry.ts b/src/observability/telemetry.ts index 14284a9..a203ef2 100644 --- a/src/observability/telemetry.ts +++ b/src/observability/telemetry.ts @@ -100,7 +100,7 @@ export function maybePrintFirstRunDisclosure(): void { '', 'ℹ Telemetry: HookMyApp CLI reports crashes + usage analytics to help us fix bugs and improve UX.', ' No command arguments, file contents, or env var values are sent — except the', - ' message you pass to `hookmyapp feedback`, which is sent on purpose.', + ' message and --surface you pass to `hookmyapp feedback`, sent on purpose.', ' When logged in, your account email + user id accompany crash reports.', ' Disable: `hookmyapp config set telemetry off` or `HOOKMYAPP_TELEMETRY=off`', '',