From 0ca69472d93cf088c836bed3dc7df252247e2101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Sun, 4 Oct 2026 05:07:49 +0000 Subject: [PATCH] feat(cli): feedback says whether a person or an agent wrote it, with the email an app attaches (#4984) * feat(cli): feedback says whether a person or an agent wrote it, with the email an app attaches * refactor(cli): split the .env loader so each part stays simple * fix(cli): a broken .env loader fails loudly; only a missing .env is ignored --- docs/guides/feedback.mdx | 2 +- docs/packages/cli.mdx | 14 +++++-- packages/cli/src/cli.ts | 27 +------------ packages/cli/src/commands/feedback.test.ts | 39 +++++++++++++++++-- packages/cli/src/commands/feedback.ts | 22 ++++++++++- packages/cli/src/utils/dotEnv.test.ts | 16 ++++++++ packages/cli/src/utils/dotEnv.ts | 31 +++++++++++++++ packages/cli/src/utils/feedbackSource.test.ts | 22 +++++++++++ packages/cli/src/utils/feedbackSource.ts | 17 ++++++++ packages/cli/src/utils/submitFeedback.test.ts | 11 ++++++ packages/cli/src/utils/submitFeedback.ts | 5 +++ 11 files changed, 171 insertions(+), 35 deletions(-) create mode 100644 packages/cli/src/utils/dotEnv.test.ts create mode 100644 packages/cli/src/utils/dotEnv.ts create mode 100644 packages/cli/src/utils/feedbackSource.test.ts create mode 100644 packages/cli/src/utils/feedbackSource.ts diff --git a/docs/guides/feedback.mdx b/docs/guides/feedback.mdx index eeb3d19bf7..2aaead6282 100644 --- a/docs/guides/feedback.mdx +++ b/docs/guides/feedback.mdx @@ -62,7 +62,7 @@ Do not include API keys, access tokens, credentials, private media, or customer HyperFrames may occasionally show a short rating prompt after a render or during a Studio session. You can ignore or dismiss it. -Feedback can include the rating, an optional comment, and basic environment information used to understand the problem. It does not include your composition HTML, video files, project names, file paths, or environment-variable values beyond the short agent and app names listed under [`telemetry`](/packages/cli#telemetry), such as `client=example-app/1.2.3/stable` when another app launched the CLI. +Feedback can include the rating, an optional comment, and basic environment information used to understand the problem. It does not include your composition HTML, video files, project names, file paths, or environment-variable values beyond the short agent and app names listed under [`telemetry`](/packages/cli#telemetry), such as `client=example-app/1.2.3/stable` when another app launched the CLI. When an app launches it for a signed-in person, the report can also carry that person's email, set by the app through `HYPERFRAMES_FEEDBACK_EMAIL`, and whether a person or an agent wrote it. To turn off CLI telemetry: diff --git a/docs/packages/cli.mdx b/docs/packages/cli.mdx index d454219b52..076456127b 100644 --- a/docs/packages/cli.mdx +++ b/docs/packages/cli.mdx @@ -1232,7 +1232,8 @@ Topics: `data-attributes`, `examples`, `rendering`, `gsap`, `troubleshooting`, ### `feedback` -Send anonymous feedback about how it went. +Send feedback about how it went. It is anonymous unless the app that launched +the CLI attaches your email (below). ```bash npx hyperframes feedback --rating 10 @@ -1240,7 +1241,9 @@ npx hyperframes feedback --rating 7 --comment "render succeeded but GSAP timelin npx hyperframes feedback --rating 3 --comment "GSAP timeline froze on seek" --file-issue ``` -`--rating` is required, 0–10. `--comment` adds free text. `--file-issue` +`--rating` is required, 0–10. `--comment` adds free text. `--source person` +or `--source agent` says who wrote a rating report (a `--search-miss` report +carries neither), so the team can tell a person's feedback from an agent's. `--file-issue` also opens a GitHub issue, `--dir` picks the project published as its repro (default: current directory), and `--yes, -y` skips the consent prompt for scripts. @@ -1252,7 +1255,12 @@ submit yourself. No token, no backend. See Agents can call this after a render; [About feedback data](/guides/feedback#about-feedback-data) covers what the -surface includes. With telemetry disabled it prints +surface includes. An app that launches the CLI can attach its signed-in +person's email with the `HYPERFRAMES_FEEDBACK_EMAIL` environment variable. It +is never a flag and never read from a project `.env`, and it goes only into +the feedback report, not telemetry. It is not verified: treat it as a way to +reply, not as proof of who sent the report. With +telemetry disabled it prints `Telemetry is disabled. Feedback not sent.` and exits cleanly. ### `telemetry` diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 3e255afdac..00a6c4bb0b 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -63,35 +63,12 @@ if (rootVersionRequested) { // ── Load .env from CWD ───────────────────────────────────────────────────── // Agents run from the project directory where .env holds API keys (Gemini, // HeyGen, ElevenLabs). Load it automatically so they don't need `source .env`. +const { applyDotEnv } = await import("./utils/dotEnv.js"); try { const { readFileSync } = await import("node:fs"); const { resolve } = await import("node:path"); const envPath = resolve(process.cwd(), ".env"); - const envContent = readFileSync(envPath, "utf-8"); - for (const rawLine of envContent.split("\n")) { - let line = rawLine.trim(); - if (!line || line.startsWith("#")) continue; - // Tolerate `export FOO=bar` (common in dotfile-style .env files). - if (line.startsWith("export ")) line = line.slice(7).trim(); - const eqIdx = line.indexOf("="); - if (eqIdx < 1) continue; - const key = line.slice(0, eqIdx).trim(); - let val = line.slice(eqIdx + 1).trim(); - if (val.startsWith('"') || val.startsWith("'")) { - // Quoted value: take until the matching closing quote; leave the rest. - // Anything after a closing quote (including `# comment`) is dropped. - const quote = val.charAt(0); - const end = val.indexOf(quote, 1); - if (end > 0) val = val.slice(1, end); - else val = val.slice(1); // unterminated quote — best-effort, strip opener - } else { - // Unquoted value: strip inline `# comment` (requires whitespace before # - // to avoid eating `pass#word` style values). - const commentMatch = val.match(/\s+#/); - if (commentMatch?.index !== undefined) val = val.slice(0, commentMatch.index).trim(); - } - if (key && !(key in process.env)) process.env[key] = val; - } + applyDotEnv(readFileSync(envPath, "utf-8"), process.env); } catch { /* .env not present — fine, env vars may be set another way */ } diff --git a/packages/cli/src/commands/feedback.test.ts b/packages/cli/src/commands/feedback.test.ts index 4502f1a617..98688b11c9 100644 --- a/packages/cli/src/commands/feedback.test.ts +++ b/packages/cli/src/commands/feedback.test.ts @@ -1,17 +1,20 @@ -import { expect, it, vi } from "vitest"; +import { afterEach, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ // Never settles: waiting on it would hang the command. flush: vi.fn(() => new Promise(() => {})), - submitFeedback: vi.fn(async () => {}), + submitFeedback: vi.fn(async (_input: { env?: string }) => {}), submitCatalogSearchMiss: vi.fn(async () => {}), })); vi.mock("../telemetry/client.js", () => ({ shouldTrack: () => true, flush: mocks.flush })); +const trackRenderFeedback = vi.hoisted(() => vi.fn()); vi.mock("../telemetry/events.js", () => ({ trackCatalogSearchMiss: vi.fn(), - trackRenderFeedback: vi.fn(), + trackRenderFeedback, })); -vi.mock("../telemetry/feedback.js", () => ({ getDoctorSummary: async () => "" })); +vi.mock("../telemetry/feedback.js", () => ({ getDoctorSummary: async () => "os=test" })); + +afterEach(() => vi.unstubAllEnvs()); vi.mock("../telemetry/config.js", () => ({ readConfig: () => ({ anonymousId: "a" }) })); vi.mock("../utils/submitFeedback.js", () => ({ submitFeedback: mocks.submitFeedback, @@ -27,3 +30,31 @@ it.each([ await feedback.run?.({ args } as never); expect(submit).toHaveBeenCalled(); }); + +it("tells person feedback from agent feedback, with the email only an app's env can attach", async () => { + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.stubEnv("HYPERFRAMES_FEEDBACK_EMAIL", " person@example.com "); + const { default: feedback } = await import("./feedback.js"); + await feedback.run?.({ args: { rating: "6", comment: "hi", source: "person" } } as never); + expect(mocks.submitFeedback).toHaveBeenLastCalledWith( + expect.objectContaining({ source: "person", email: "person@example.com" }), + ); + // The email goes only into the report: never telemetry, never the environment summary. + expect(JSON.stringify(trackRenderFeedback.mock.calls)).not.toContain("person@example.com"); + expect(JSON.stringify(mocks.submitFeedback.mock.lastCall?.[0].env)).not.toContain( + "person@example.com", + ); + vi.unstubAllEnvs(); + await feedback.run?.({ args: { rating: "6", source: "agent" } } as never); + expect(mocks.submitFeedback).toHaveBeenLastCalledWith( + expect.objectContaining({ source: "agent", email: undefined }), + ); +}); + +it("refuses a source that is neither person nor agent, and sends nothing", async () => { + vi.spyOn(console, "error").mockImplementation(() => {}); + mocks.submitFeedback.mockClear(); + const { default: feedback } = await import("./feedback.js"); + await expect(feedback.run?.({ args: { rating: "6", source: "bot" } } as never)).rejects.toThrow(); + expect(mocks.submitFeedback).not.toHaveBeenCalled(); +}); diff --git a/packages/cli/src/commands/feedback.ts b/packages/cli/src/commands/feedback.ts index 99533ab5de..fdedeb1387 100644 --- a/packages/cli/src/commands/feedback.ts +++ b/packages/cli/src/commands/feedback.ts @@ -15,6 +15,7 @@ import { buildIssueUrl, HYPERFRAMES_REPO_URL } from "../utils/feedbackIssue.js"; import { VERSION } from "../version.js"; import { c } from "../ui/colors.js"; import { parseFeedbackRating } from "../utils/feedbackRating.js"; +import { feedbackEmail, parseFeedbackSource } from "../utils/feedbackSource.js"; import { lintFeedbackComment, type FeedbackLintInput } from "../utils/feedbackLint.js"; export const examples: Example[] = [ @@ -155,7 +156,7 @@ async function fileGithubIssue(opts: { } export default defineCommand({ - meta: { name: "feedback", description: "Submit anonymous feedback about your experience" }, + meta: { name: "feedback", description: "Submit feedback about your experience" }, args: { rating: { type: "string", @@ -167,6 +168,10 @@ export default defineCommand({ type: "string", description: "Optional details about your experience", }, + source: { + type: "string", + description: 'Who wrote the report: "person" or "agent"', + }, "search-miss": { type: "string", description: "Report a catalog search that found nothing useful; pass the query you ran", @@ -223,6 +228,12 @@ export default defineCommand({ failCommand(); } + const source = parseFeedbackSource(args.source); + if (source === null) { + console.error(c.error('Source must be "person" or "agent"')); + failCommand(); + } + if (!shouldTrack()) { console.log(c.dim("Telemetry is disabled. Feedback not sent.")); return; @@ -261,7 +272,14 @@ export default defineCommand({ // Ack first so the user isn't kept waiting on the best-effort forward (which // is bounded to a few seconds and never surfaces an error either way). console.log(c.dim("Thanks for the feedback!")); - await submitFeedback({ rating, comment, cliVersion: VERSION, env: envWithJoinKeys }); + await submitFeedback({ + rating, + comment, + cliVersion: VERSION, + env: envWithJoinKeys, + source, + email: feedbackEmail(), + }); if (args["file-issue"] === true) { await fileGithubIssue({ diff --git a/packages/cli/src/utils/dotEnv.test.ts b/packages/cli/src/utils/dotEnv.test.ts new file mode 100644 index 0000000000..478d61ccc9 --- /dev/null +++ b/packages/cli/src/utils/dotEnv.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from "vitest"; +import { applyDotEnv } from "./dotEnv.js"; + +describe("applyDotEnv", () => { + it("copies a project's keys without overwriting what is already set", () => { + const env: NodeJS.ProcessEnv = { KEPT: "outer" }; + applyDotEnv('export GEMINI_KEY="abc" # note\nKEPT=inner\n# comment\nPLAIN=x #c', env); + expect(env).toEqual({ KEPT: "outer", GEMINI_KEY: "abc", PLAIN: "x" }); + }); + + it("never takes the feedback email from a project file", () => { + const env: NodeJS.ProcessEnv = {}; + applyDotEnv("HYPERFRAMES_FEEDBACK_EMAIL=someone@example.com\nOTHER=1", env); + expect(env).toEqual({ OTHER: "1" }); + }); +}); diff --git a/packages/cli/src/utils/dotEnv.ts b/packages/cli/src/utils/dotEnv.ts new file mode 100644 index 0000000000..64818c9f6f --- /dev/null +++ b/packages/cli/src/utils/dotEnv.ts @@ -0,0 +1,31 @@ +import { FEEDBACK_EMAIL_ENV } from "./feedbackSource.js"; + +// A quoted value ends at its closing quote; unquoted, `#` starts a comment only after whitespace, so `pass#word` stays. +function valueOf(raw: string): string { + const quote = raw.charAt(0); + if (quote === '"' || quote === "'") { + const end = raw.indexOf(quote, 1); + return end > 0 ? raw.slice(1, end) : raw.slice(1); + } + const comment = raw.match(/\s+#/); + return comment?.index === undefined ? raw : raw.slice(0, comment.index).trim(); +} + +function entryOf(rawLine: string): [string, string] | null { + let line = rawLine.trim(); + if (!line || line.startsWith("#")) return null; + if (line.startsWith("export ")) line = line.slice(7).trim(); + const eq = line.indexOf("="); + if (eq < 1) return null; + return [line.slice(0, eq).trim(), valueOf(line.slice(eq + 1).trim())]; +} + +// Never over a key already set, and never the feedback email: only the launching app may attach it, and an agent can +// write a project `.env`. +export function applyDotEnv(content: string, env: NodeJS.ProcessEnv): void { + for (const entry of content.split("\n").map(entryOf)) { + if (!entry) continue; + const [key, value] = entry; + if (key && key !== FEEDBACK_EMAIL_ENV && !(key in env)) env[key] = value; + } +} diff --git a/packages/cli/src/utils/feedbackSource.test.ts b/packages/cli/src/utils/feedbackSource.test.ts new file mode 100644 index 0000000000..1fc2efcdfa --- /dev/null +++ b/packages/cli/src/utils/feedbackSource.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from "vitest"; +import { feedbackEmail, parseFeedbackSource } from "./feedbackSource.js"; + +describe("parseFeedbackSource", () => { + it.each([ + [undefined, undefined], + ["person", "person"], + [" agent ", "agent"], + ["Person", null], + ["", null], + ])("%j reads as %j", (raw, expected) => { + expect(parseFeedbackSource(raw)).toBe(expected); + }); +}); + +describe("feedbackEmail", () => { + it("reads only HYPERFRAMES_FEEDBACK_EMAIL, trimmed, and nothing when blank", () => { + expect(feedbackEmail({ HYPERFRAMES_FEEDBACK_EMAIL: " a@b.co " })).toBe("a@b.co"); + expect(feedbackEmail({ HYPERFRAMES_FEEDBACK_EMAIL: " " })).toBeUndefined(); + expect(feedbackEmail({ EMAIL: "a@b.co" })).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/utils/feedbackSource.ts b/packages/cli/src/utils/feedbackSource.ts new file mode 100644 index 0000000000..97829b4cf3 --- /dev/null +++ b/packages/cli/src/utils/feedbackSource.ts @@ -0,0 +1,17 @@ +export type FeedbackSource = "person" | "agent"; + +const SOURCES: readonly string[] = ["person", "agent"]; + +/** The `--source` value, undefined when absent, null when it is neither. */ +export function parseFeedbackSource(raw: string | undefined): FeedbackSource | null | undefined { + if (raw === undefined) return undefined; + const source = raw.trim(); + return SOURCES.includes(source) ? (source as FeedbackSource) : null; +} + +export const FEEDBACK_EMAIL_ENV = "HYPERFRAMES_FEEDBACK_EMAIL"; + +/** Set only by the launching app (never a flag or a project `.env`); unverified, so never proof of identity. */ +export function feedbackEmail(env: NodeJS.ProcessEnv = process.env): string | undefined { + return env[FEEDBACK_EMAIL_ENV]?.trim() || undefined; +} diff --git a/packages/cli/src/utils/submitFeedback.test.ts b/packages/cli/src/utils/submitFeedback.test.ts index 79f19d666e..61d2d5d7b5 100644 --- a/packages/cli/src/utils/submitFeedback.test.ts +++ b/packages/cli/src/utils/submitFeedback.test.ts @@ -48,6 +48,17 @@ describe("submitFeedback", () => { ); }); + it("sends who wrote it and the attached email, the email capped like the backend's", async () => { + const fetchMock = vi.fn(async () => new Response(null, { status: 202 })); + vi.stubGlobal("fetch", fetchMock); + + await submitFeedback({ rating: 5, cliVersion: "1", source: "person", email: "e".repeat(300) }); + + const body = JSON.parse(fetchMock.mock.calls[0]?.[1]?.body as string); + expect(body.source).toBe("person"); + expect(body.email).toHaveLength(254); + }); + it.each([0, 10])("serializes the NPS boundary %i without changing it", async (rating) => { const fetchMock = vi.fn(async () => new Response(null, { status: 202 })); vi.stubGlobal("fetch", fetchMock); diff --git a/packages/cli/src/utils/submitFeedback.ts b/packages/cli/src/utils/submitFeedback.ts index e1d28941f2..f170184ab5 100644 --- a/packages/cli/src/utils/submitFeedback.ts +++ b/packages/cli/src/utils/submitFeedback.ts @@ -7,6 +7,7 @@ import { FEEDBACK_RATING_SCALE } from "./feedbackRating.js"; const MAX_COMMENT = 2000; const MAX_CLI_VERSION = 100; const MAX_ENV = 500; +const MAX_EMAIL = 254; function cap(value: string | undefined, max: number): string | undefined { if (value === undefined) return undefined; @@ -18,6 +19,8 @@ export async function submitFeedback(input: { comment?: string; cliVersion: string; env?: string; + source?: "person" | "agent"; + email?: string; }): Promise { try { const apiBaseUrl = getPublishApiBaseUrl(); @@ -29,6 +32,8 @@ export async function submitFeedback(input: { comment: cap(input.comment, MAX_COMMENT), cli_version: cap(input.cliVersion, MAX_CLI_VERSION), env: cap(input.env, MAX_ENV), + source: input.source, + email: cap(input.email, MAX_EMAIL), }), headers: { "content-type": "application/json",