Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions src/commands/__tests__/feedback.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
75 changes: 75 additions & 0 deletions src/commands/support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 <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'];
3 changes: 2 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -198,6 +198,7 @@ registerWorkspaceCommand(program);
// Customers (customer workspaces)
registerCustomersCommand(program);
registerSupportCommand(program);
registerFeedbackCommand(program);
registerNotificationsCommand(program);
registerOrgProfileCommand(program);

Expand Down
8 changes: 5 additions & 3 deletions src/observability/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`',
'',
Expand Down
Loading