Skip to content

Commit f5069f6

Browse files
committed
Harden feedback honesty and CLI help exit path
Ship blockers from the OSS panel: --help exits 0 via CliHelpError, feedback says queued not sent with truncation notice, hide /feedback from the slash menu until survey env ids are set, and drop a bare feedback arm when another slash command runs.
1 parent 014aa60 commit f5069f6

10 files changed

Lines changed: 98 additions & 19 deletions

File tree

docs/TELEMETRY.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,10 @@ still win — `DO_NOT_TRACK=1` or `CORBITS_TELEMETRY=0/false/off/no` block
160160

161161
Survey id / question id come from env (`CORBITS_FEEDBACK_SURVEY_ID`,
162162
`CORBITS_FEEDBACK_QUESTION_ID`). Both must be set or capture fails closed with
163-
an “not configured” message — no default survey is baked into the client.
163+
an “not configured” message — no default survey is baked into the client. When
164+
ids are missing the command is also hidden from the slash menu (still callable
165+
if typed). Success copy says “queued,” not delivered; free text over 2000
166+
characters is truncated with an explicit notice.
164167

165168
## Opting out
166169

src/config.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
55

6-
import { buildBifrostSource, buildOpenAISource, buildProviderCatalog, catalogEntryAsProviderSettings, KEYLESS_API_KEY, loadConfig, providerCatalogToSettings, runtimeSettingsWithCatalog, SOURCE_MAX_TOKENS } from "./config/index.js";
6+
import { buildBifrostSource, buildOpenAISource, buildProviderCatalog, catalogEntryAsProviderSettings, CliHelpError, CLI_HELP_TEXT, KEYLESS_API_KEY, loadConfig, providerCatalogToSettings, runtimeSettingsWithCatalog, SOURCE_MAX_TOKENS } from "./config/index.js";
77
import type { Config, UnconfiguredConfig } from "./config/index.js";
88
import { mergeProviderIntoSettings, type ResolvedProvider, type Settings } from "./config/settings.js";
99
import { OPENCODE_GO_BASE_URL } from "../packages/opencode-go/src/index.js";
@@ -362,6 +362,22 @@ describe("loadConfig", () => {
362362
}
363363
});
364364

365+
test("--help throws CliHelpError with exitCode 0 and full help text", async () => {
366+
await expect(loadConfig(["--help"], { globalSettingsPath: NO_SETTINGS })).rejects.toBeInstanceOf(
367+
CliHelpError,
368+
);
369+
try {
370+
await loadConfig(["-h"], { globalSettingsPath: NO_SETTINGS });
371+
expect.unreachable("expected CliHelpError");
372+
} catch (err) {
373+
expect(err).toBeInstanceOf(CliHelpError);
374+
const help = err as CliHelpError;
375+
expect(help.exitCode).toBe(0);
376+
expect(help.message).toBe(CLI_HELP_TEXT);
377+
expect(help.message).toContain("resume");
378+
}
379+
});
380+
365381
test("rejects unknown flags", async () => {
366382
await expect(
367383
loadConfig(["--unknown"], { globalSettingsPath: NO_SETTINGS }),

src/config/index.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,19 @@ Flags:
379379
--help, -h show this help
380380
`;
381381

382+
/**
383+
* Thrown when the operator asked for CLI help. Entry points must print
384+
* `message` to stdout and exit 0 — not treat this as a crash.
385+
*/
386+
export class CliHelpError extends Error {
387+
readonly exitCode = 0 as const;
388+
389+
constructor(text: string = CLI_HELP_TEXT) {
390+
super(text);
391+
this.name = "CliHelpError";
392+
}
393+
}
394+
382395
export type LoadConfigOptions = {
383396
// Override the global settings file location (for tests / non-standard homes).
384397
globalSettingsPath?: string;
@@ -444,7 +457,7 @@ export async function loadConfig(
444457
}
445458

446459
if (args[0] === "--help" || args[0] === "-h") {
447-
throw new Error(CLI_HELP_TEXT);
460+
throw new CliHelpError();
448461
}
449462

450463

src/index.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { primeCrashReporting, writeCrashReport, type CrashKind } from "./crash/r
44
import { getActiveRun, markCrashed } from "./session/active-run.js";
55
import { getActiveDisposeHost } from "./session/active-host.js";
66
import { saveCrashState } from "./session/state.js";
7-
import { loadConfig } from "./config/index.js";
7+
import { loadConfig, CliHelpError } from "./config/index.js";
88
import { ensureTelemetrySettings, globalSettingsPath } from "./config/settings.js";
99
import { installFileLogSink } from "./logging/sink.js";
1010
import { flushPerfToOtel } from "./perf/index.js";
@@ -297,8 +297,14 @@ if (import.meta.main) {
297297
try {
298298
code = await main(process.argv.slice(2));
299299
} catch (err: unknown) {
300-
process.stderr.write(`${err instanceof Error ? (err.stack ?? err.message) : String(err)}\n`);
301-
code = 1;
300+
// Help is an intentional early exit, not a crash — stdout + 0.
301+
if (err instanceof CliHelpError) {
302+
process.stdout.write(`${err.message}\n`);
303+
code = err.exitCode;
304+
} else {
305+
process.stderr.write(`${err instanceof Error ? (err.stack ?? err.message) : String(err)}\n`);
306+
code = 1;
307+
}
302308
}
303309
process.exit(code);
304310
}

src/telemetry/feedback.test.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,20 @@ describe("captureFeedback", () => {
148148
expect(events).toHaveLength(0);
149149
});
150150

151+
test("reports truncation when free text exceeds the cap", () => {
152+
const { telemetry, events } = captureSpy();
153+
const long = "x".repeat(FEEDBACK_MAX_CHARS + 50);
154+
const status = captureFeedback(telemetry, long, {
155+
env: {
156+
CORBITS_FEEDBACK_SURVEY_ID: "s1",
157+
CORBITS_FEEDBACK_QUESTION_ID: "q1",
158+
},
159+
});
160+
expect(status).toBe("sent_truncated");
161+
expect(events).toHaveLength(1);
162+
expect(String(events[0]?.properties.$survey_response).length).toBe(FEEDBACK_MAX_CHARS);
163+
});
164+
151165
test("rejects non-survey events on the intentional door", () => {
152166
const { telemetry, events } = captureSpy();
153167
expect(telemetry.captureIntentional("cli_start")).toBe(false);
@@ -157,7 +171,8 @@ describe("captureFeedback", () => {
157171

158172
describe("feedbackResultMessage", () => {
159173
test("maps statuses to operator-facing lines", () => {
160-
expect(feedbackResultMessage("sent")).toContain("Thanks");
174+
expect(feedbackResultMessage("sent")).toBe("Thanks — feedback queued.");
175+
expect(feedbackResultMessage("sent_truncated")).toContain("truncated");
161176
expect(feedbackResultMessage("blocked")).toContain("could not be sent");
162177
expect(feedbackResultMessage("unconfigured")).toContain("not configured");
163178
expect(feedbackResultMessage("empty")).toContain("No feedback");

src/telemetry/feedback.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ export const FEEDBACK_MAX_CHARS = 2000;
1212
export const FEEDBACK_PROMPT =
1313
"Please share your feedback. When done please hit enter. (Empty Enter cancels.)";
1414

15-
export const FEEDBACK_THANKS = "Thanks — feedback sent.";
15+
export const FEEDBACK_THANKS = "Thanks — feedback queued.";
16+
17+
export const FEEDBACK_THANKS_TRUNCATED =
18+
"Thanks — feedback queued (truncated to 2000 characters).";
1619

1720
export const FEEDBACK_EMPTY = "No feedback text provided.";
1821

@@ -37,6 +40,11 @@ export function feedbackQuestionId(env: NodeJS.ProcessEnv = process.env): string
3740
return env.CORBITS_FEEDBACK_QUESTION_ID?.trim() ?? "";
3841
}
3942

43+
/** True when both survey env ids are present (command is useful to show). */
44+
export function isFeedbackConfigured(env: NodeJS.ProcessEnv = process.env): boolean {
45+
return feedbackSurveyId(env).length > 0 && feedbackQuestionId(env).length > 0;
46+
}
47+
4048
export function capFeedbackMessage(message: string): string {
4149
if (message.length <= FEEDBACK_MAX_CHARS) return message;
4250
return message.slice(0, FEEDBACK_MAX_CHARS);
@@ -74,6 +82,8 @@ export function buildSurveyProperties(
7482
/**
7583
* Capture intentional survey response. Returns whether the event was queued.
7684
* Empty/whitespace-only text is not sent. Missing survey/question ids fail closed.
85+
* "sent" means queued for flush (not a network delivery ack); "sent_truncated"
86+
* is the same after the free-text cap was applied.
7787
*/
7888
export function captureFeedback(
7989
telemetry: Telemetry,
@@ -82,26 +92,30 @@ export function captureFeedback(
8292
turnTraceId?: string | undefined;
8393
env?: NodeJS.ProcessEnv;
8494
} = {},
85-
): "empty" | "blocked" | "unconfigured" | "sent" {
95+
): "empty" | "blocked" | "unconfigured" | "sent" | "sent_truncated" {
8696
const trimmed = message.trim();
8797
if (trimmed.length === 0) return "empty";
8898
const env = options.env ?? process.env;
89-
if (feedbackSurveyId(env).length === 0 || feedbackQuestionId(env).length === 0) {
99+
if (!isFeedbackConfigured(env)) {
90100
return "unconfigured";
91101
}
102+
const truncated = trimmed.length > FEEDBACK_MAX_CHARS;
92103
const ok = telemetry.captureIntentional(
93104
"survey sent",
94105
buildSurveyProperties(trimmed, options),
95106
);
96-
return ok ? "sent" : "blocked";
107+
if (!ok) return "blocked";
108+
return truncated ? "sent_truncated" : "sent";
97109
}
98110

99111
export function feedbackResultMessage(
100-
status: "empty" | "blocked" | "unconfigured" | "sent",
112+
status: "empty" | "blocked" | "unconfigured" | "sent" | "sent_truncated",
101113
): string {
102114
switch (status) {
103115
case "sent":
104116
return FEEDBACK_THANKS;
117+
case "sent_truncated":
118+
return FEEDBACK_THANKS_TRUNCATED;
105119
case "blocked":
106120
return FEEDBACK_BLOCKED;
107121
case "unconfigured":

src/tui/commands/built-in.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,12 @@ describe("/feedback command", () => {
177177
signalClear: () => {},
178178
submitFeedback: (text) => {
179179
sent.push(text);
180-
return "Thanks — feedback sent.";
180+
return "Thanks — feedback queued.";
181181
},
182182
};
183183
expect(getCommand("feedback")!.handler("love the TUI", ctx)).toEqual({
184184
type: "message",
185-
text: "Thanks — feedback sent.",
185+
text: "Thanks — feedback queued.",
186186
});
187187
expect(sent).toEqual(["love the TUI"]);
188188
});

src/tui/commands/built-in.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
parseChangelog,
66
resolveChangelogPath,
77
} from "../../changelog/index.js";
8-
import { FEEDBACK_PROMPT } from "../../telemetry/feedback.js";
8+
import { FEEDBACK_PROMPT, isFeedbackConfigured } from "../../telemetry/feedback.js";
99

1010
/**
1111
* Register every built-in slash command.
@@ -166,10 +166,12 @@ export function registerBuiltInCommands(): void {
166166

167167
// Intentional product feedback → PostHog survey (headless). Can ship when
168168
// ambient telemetry is off; env kill switches still block. Free text 2000 cap.
169+
// Hidden from the slash menu until survey env ids are set (still callable).
169170
registerCommand({
170171
name: "feedback",
171172
description: "Send product feedback (env kill switches still apply)",
172173
argumentHint: "[your feedback]",
174+
available: () => isFeedbackConfigured(),
173175
handler: (args, ctx) => {
174176
const text = args.trim();
175177
if (text.length === 0) {

src/tui/runner.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,11 @@ export function createSubmitHandler(
392392
return;
393393
}
394394
if (route.kind === "command") {
395+
// Any other slash command drops a bare-/feedback arm so the next
396+
// free-text line is not mis-routed as survey text.
397+
if (feedbackPending && route.name !== "feedback") {
398+
deps.cancelFeedbackCapture?.();
399+
}
395400
deps.dispatchCommand(route.name, route.args);
396401
return;
397402
}

src/tui/submit-handler.test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,29 +103,34 @@ describe("composer submit handler", () => {
103103
isFeedbackCapturePending: () => isFeedbackCapturePending(),
104104
onFeedbackText: (text) => {
105105
feedbackTexts.push(text);
106-
return "Thanks — feedback sent.";
106+
return "Thanks — feedback queued.";
107+
107108
},
108109
});
109110
armFeedbackCapture();
110111
h.submit("the UI is snappy");
111112
expect(feedbackTexts).toEqual(["the UI is snappy"]);
112113
expect(h.prompts).toEqual([]);
113-
expect(h.notices).toEqual(["Thanks — feedback sent."]);
114+
expect(h.notices).toEqual(["Thanks — feedback queued."]);
115+
114116
expect(h.telemetry()).toBe(0);
115117
});
116118

117119
test("slash commands still dispatch while feedback capture is pending", () => {
118120
const h = harness({
119121
isFeedbackCapturePending: () => isFeedbackCapturePending(),
122+
cancelFeedbackCapture: () => {
123+
cancelFeedbackCapture();
124+
},
120125
onFeedbackText: () => "should not run",
121126
});
122127
armFeedbackCapture();
123128
h.submit("/help");
124129
expect(h.dispatched).toEqual([{ name: "help", args: "" }]);
125130
expect(h.prompts).toEqual([]);
126131
expect(h.notices).toEqual([]);
127-
// Still pending — only a non-command line consumes it.
128-
expect(isFeedbackCapturePending()).toBe(true);
132+
// Other slash commands drop the arm so the next free-text line is a prompt.
133+
expect(isFeedbackCapturePending()).toBe(false);
129134
});
130135

131136
test("empty Enter while armed cancels instead of trapping the operator", () => {

0 commit comments

Comments
 (0)