diff --git a/src/commands/__tests__/feedback.test.ts b/src/commands/__tests__/feedback.test.ts new file mode 100644 index 0000000..493a476 --- /dev/null +++ b/src/commands/__tests__/feedback.test.ts @@ -0,0 +1,102 @@ +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), + maybePrintFirstRunDisclosure: vi.fn(), +})); + +const mockConsoleLog = vi.spyOn(console, 'log').mockImplementation(() => {}); + +import { apiClient } from '../../api/client.js'; +import { isTelemetryEnabled, maybePrintFirstRunDisclosure } 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('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)).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 () => { + 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('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' }), + ).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..5e92e45 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, maybePrintFirstRunDisclosure } from '../observability/telemetry.js'; /** * AIT-337 — `hookmyapp support`: open and follow support tickets from the CLI @@ -358,3 +359,77 @@ 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. ' + + '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') + .option('--json', 'Output machine-readable JSON') + .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 (opts.surface !== undefined && !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; + } + // `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). + // 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', + // 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)); + return; + } + console.log(`Thanks — recorded as ${res.ticketId}.${res.note ? ` ${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 6d823d8..fb3e1d7 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'; @@ -198,6 +198,7 @@ registerWorkspaceCommand(program); // Customers (customer workspaces) registerCustomersCommand(program); registerSupportCommand(program); +registerFeedbackCommand(program); registerNotificationsCommand(program); registerOrgProfileCommand(program); diff --git a/src/observability/telemetry.ts b/src/observability/telemetry.ts index efeae37..a203ef2 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; @@ -98,7 +99,8 @@ 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 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`', '',