diff --git a/.changeset/qualitative-survey-feedback.md b/.changeset/qualitative-survey-feedback.md index e1b398c0..d16368f9 100644 --- a/.changeset/qualitative-survey-feedback.md +++ b/.changeset/qualitative-survey-feedback.md @@ -3,3 +3,5 @@ --- `.taskless/.gitignore` now ignores `/.tmp-*`, the scratch request files the agent recipes write (`.tmp-rule-request.json`, `.tmp-improve-request.json`), so a file an agent forgot to clean up is a stray rather than a commit. This is scaffold migration 7; the scaffold's own `version` field carries the compatibility signal, and a project at 6 gains one ignore line the next time it is bootstrapped. + +A `feedback` subcommand joins the CLI, reached only through the survey invite a served recipe carries and so absent from the `taskless agent` index: `feedback send --from ` validates a human-keyed payload (`verbatim`, `goal`, `completed` as `Yes`/`No`/`Unknown`, optional `workedWell` and `needsImprovement`), maps it to the PostHog survey's question ids, and captures `survey sent`; `feedback dismiss` captures `survey dismissed`. Both hold the next invite off for 20 days and, under the telemetry opt-out, say nothing was sent and exit 0. diff --git a/packages/cli/src/commands/agent.ts b/packages/cli/src/commands/agent.ts index 585a7d94..bfbe2d8d 100644 --- a/packages/cli/src/commands/agent.ts +++ b/packages/cli/src/commands/agent.ts @@ -45,8 +45,8 @@ async function resolveDescription( /** * Commands this index deliberately does not advertise. * - * A declared list rather than a condition, because the two entries are absent - * for unrelated reasons and a bare `name === "agent"` recorded neither. + * A declared list rather than a condition, because the entries are absent for + * unrelated reasons and a bare `name === "agent"` recorded none of them. * * - `agent` is the index itself, so listing it would be circular. * - `demo` writes a fixed example rule for someone learning what a rule is. It @@ -59,10 +59,16 @@ async function resolveDescription( * verb gave it discoverability it does not want yet, and this is the cost of * that choice, paid here. * + * - `feedback` is reached only through the survey invite a served recipe + * carries. Listed, it would invite an agent to run it unprompted, and a + * `survey sent` with no invite behind it is noise in the funnel. When a + * general feedback channel exists this surface folds into it, and that is + * the point to reconsider listing. + * * Absence from this list is what puts a command in the index, so adding one is * a decision someone made rather than a step they forgot. */ -const UNLISTED_COMMANDS = new Set(["agent", "demo"]); +const UNLISTED_COMMANDS = new Set(["agent", "demo", "feedback"]); export function createAgentCommand(subCommands: SubCommandsDef) { return defineCommand({ diff --git a/packages/cli/src/commands/feedback.ts b/packages/cli/src/commands/feedback.ts new file mode 100644 index 00000000..abf92843 --- /dev/null +++ b/packages/cli/src/commands/feedback.ts @@ -0,0 +1,183 @@ +import { readFile } from "node:fs/promises"; +import { resolve } from "node:path"; + +import { defineCommand } from "citty"; +import { ZodError } from "zod"; + +import { inputSchema, type FeedbackInput } from "../schemas/feedback"; +import { writeNextAsk } from "../survey/cadence"; +import { + ANSWERED_INTERVAL_MS, + SURVEY_ID, + SURVEY_QUESTIONS, +} from "../survey/constants"; +import { getTelemetry, isTelemetryEnabled } from "../telemetry"; +import { type CLIErrorCode, writeJsonError } from "../types/errors"; +import { CLIError } from "../util/cli-error"; + +/** + * What both verbs say under the telemetry opt-out. An agent should never + * reach them in that state, because the invite is not served in it, so this + * is a defensive line rather than a path the recipe describes. Exit zero: the + * user asked for nothing to be sent, and nothing was. + */ +const NOTHING_SENT = + "Telemetry is disabled, so no feedback was sent. Nothing else to do."; + +/** + * The `survey sent` properties for a validated payload. + * + * Exactly PostHog's contract: `$survey_id` and one `$survey_response_` + * per answered question. An optional question left blank is absent rather + * than sent as an empty string, so the responses view shows a gap and not an + * empty answer. + */ +export function buildSurveyResponse( + input: FeedbackInput +): Record { + const properties: Record = { $survey_id: SURVEY_ID }; + for (const { key, id } of SURVEY_QUESTIONS) { + const answer = input[key]; + if (answer !== undefined) properties[`$survey_response_${id}`] = answer; + } + return properties; +} + +const dismissCommand = defineCommand({ + meta: { + name: "dismiss", + description: "Record that the user declined the feedback survey", + }, + args: { + dir: { + type: "string", + alias: "d", + description: "Working directory", + }, + }, + async run({ args }) { + const cwd = resolve(args.dir ?? process.cwd()); + if (!isTelemetryEnabled()) { + console.log(NOTHING_SENT); + return; + } + const telemetry = await getTelemetry(cwd); + telemetry.capture("survey dismissed", { $survey_id: SURVEY_ID }); + await writeNextAsk(SURVEY_ID, Date.now() + ANSWERED_INTERVAL_MS); + console.log("Thanks. Taskless will not ask again for a while."); + }, +}); + +const sendCommand = defineCommand({ + meta: { + name: "send", + description: + "Send a completed feedback survey (use --from to specify the input file)", + }, + args: { + dir: { + type: "string", + alias: "d", + description: "Working directory", + }, + from: { + type: "string", + description: + "Path to a JSON file containing the feedback payload (required). Example: --from .taskless/.tmp-feedback.json", + }, + json: { + type: "boolean", + description: + "On error, write the standardized { ok:false, code, message } envelope to stdout instead of human text on stderr", + default: false, + }, + }, + async run({ args }) { + const cwd = resolve(args.dir ?? process.cwd()); + + /** Emit an error and exit, respecting --json mode */ + function fail(message: string, code: CLIErrorCode): never { + if (args.json) { + writeJsonError(code, message); + } else { + console.error(`Error: ${message}`); + } + process.exitCode = 1; + throw new CLIError(message, code, { reported: true }); + } + + // Validate before checking the opt-out: a malformed payload is wrong + // whether or not anything would be sent, and the agent should hear so. + if (!args.from) { + fail( + "--from is required. Provide a path to a JSON file.\n Example: taskless feedback send --from .taskless/.tmp-feedback.json", + "INVALID_INPUT" + ); + } + + const filePath = resolve(cwd, args.from); + let fileContent: string; + try { + fileContent = await readFile(filePath, "utf8"); + } catch { + fail(`Could not read file "${args.from}".`, "INVALID_INPUT"); + } + + let rawJson: unknown; + try { + rawJson = JSON.parse(fileContent) as unknown; + } catch { + fail(`"${args.from}" is not valid JSON.`, "INVALID_INPUT"); + } + + let input: FeedbackInput; + try { + input = inputSchema.parse(rawJson); + } catch (error) { + if (error instanceof ZodError) { + fail( + `Invalid input: ${error.issues + .map( + (issue) => + `${issue.path.join(".") || "payload"}: ${issue.message}` + ) + .join(", ")}`, + "INVALID_INPUT" + ); + } + fail( + error instanceof Error ? error.message : String(error), + "INVALID_INPUT" + ); + } + + if (!isTelemetryEnabled()) { + console.log(NOTHING_SENT); + return; + } + + const telemetry = await getTelemetry(cwd); + telemetry.capture("survey sent", buildSurveyResponse(input)); + await writeNextAsk(SURVEY_ID, Date.now() + ANSWERED_INTERVAL_MS); + // The input file is left where it is, like `rule create --from`; the + // recipe's clean-up step deletes it, and `/.tmp-*` is ignored regardless. + console.log("Feedback sent. Thank you."); + }, +}); + +/** + * Reached only through the survey invite. Deliberately absent from the + * `taskless agent` index (see `UNLISTED_COMMANDS` there): listing it would + * invite an agent to run it unprompted. When a general feedback channel + * exists, this surface folds into it. + */ +export const feedbackCommand = defineCommand({ + meta: { + name: "feedback", + description: "Send or dismiss the Taskless feedback survey", + }, + subCommands: { + dismiss: dismissCommand, + send: sendCommand, + }, +}); diff --git a/packages/cli/src/commands/names.ts b/packages/cli/src/commands/names.ts index c003c5af..f90956e7 100644 --- a/packages/cli/src/commands/names.ts +++ b/packages/cli/src/commands/names.ts @@ -15,6 +15,7 @@ export const SUBCOMMAND_NAMES = [ "check", "demo", "detect", + "feedback", "info", "init", "onboard", diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index b928280f..d7658efa 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -5,6 +5,7 @@ import { authCommand } from "./commands/auth"; import { checkCommand } from "./commands/check"; import { demoCommand } from "./commands/demo"; import { detectCommand } from "./commands/detect"; +import { feedbackCommand } from "./commands/feedback"; import { initCommand, updateCommand } from "./commands/init"; import { testCommand, verifyCommand } from "./commands/verify"; import { infoCommand } from "./commands/info"; @@ -35,6 +36,7 @@ const subCommands = { detect: detectCommand, check: checkCommand, demo: demoCommand, + feedback: feedbackCommand, auth: authCommand, onboard: onboardCommand, rule: ruleCommand, diff --git a/packages/cli/src/schemas/feedback.ts b/packages/cli/src/schemas/feedback.ts new file mode 100644 index 00000000..7e44ec12 --- /dev/null +++ b/packages/cli/src/schemas/feedback.ts @@ -0,0 +1,45 @@ +import { z } from "zod"; + +import { COMPLETED_CHOICES } from "../survey/constants"; + +/** + * Input schema for `taskless feedback send --from` JSON file. + * + * Human keys only. The map to the survey's question identifiers lives in + * `src/survey/constants.ts`, and a payload never carries a `$survey_*` key. + */ +export const inputSchema = z.object({ + verbatim: z + .string() + .trim() + .min(1, "verbatim must be the user's own words, non-empty") + .describe("The user's reply, in their own words, unedited"), + goal: z + .string() + .trim() + .min(1, "goal must be a non-empty string") + .describe("What the user was trying to accomplish, in your words"), + completed: z + .enum(COMPLETED_CHOICES, { + error: `completed must be one of ${COMPLETED_CHOICES.map((choice) => `"${choice}"`).join(", ")}`, + }) + .describe( + "Whether the user completed the task, in your opinion. Success is binary; use Unknown when you cannot tell" + ), + workedWell: z + .string() + .trim() + .min(1, "workedWell, when present, must be non-empty") + .optional() + .describe("Steps of the interaction with Taskless that worked well"), + needsImprovement: z + .string() + .trim() + .min(1, "needsImprovement, when present, must be non-empty") + .optional() + .describe( + "Steps of the interaction with Taskless that could use improvement" + ), +}); + +export type FeedbackInput = z.infer; diff --git a/packages/cli/src/survey/cadence.ts b/packages/cli/src/survey/cadence.ts new file mode 100644 index 00000000..189f9c32 --- /dev/null +++ b/packages/cli/src/survey/cadence.ts @@ -0,0 +1,53 @@ +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { getConfigDirectory } from "../auth/token"; + +const NEXT_ASK_FILE = "next_ask"; + +/** + * Where a survey's cadence lives: one file per survey under the same XDG + * config directory that holds the anonymous telemetry id. + * + * Keyed by survey rather than by CLI version. A directory per release would + * grow without bound, and an upgrade should not reset the cadence: a newer + * CLI reads the same file and may not ask right away, which is fine. A new + * survey is a new id and therefore a new ask. Per-release segmentation still + * works because `cliVersion` rides on every capture. + */ +export function nextAskPath(surveyId: string): string { + return join(getConfigDirectory(), "surveys", surveyId, NEXT_ASK_FILE); +} + +/** + * The earliest time the next invite may be served, as epoch milliseconds, or + * `undefined` when the file is absent or not a number. Both read as "ask + * now"; the write that follows repairs a corrupt file. + */ +export async function readNextAsk( + surveyId: string +): Promise { + let content: string; + try { + content = await readFile(nextAskPath(surveyId), "utf8"); + } catch { + return undefined; + } + const value = Number(content.trim()); + return Number.isFinite(value) ? value : undefined; +} + +/** Record when the next invite may be served. Best-effort, like the anonymous id. */ +export async function writeNextAsk( + surveyId: string, + at: number +): Promise { + const path = nextAskPath(surveyId); + try { + await mkdir(join(path, ".."), { recursive: true }); + await writeFile(path, String(Math.trunc(at)), "utf8"); + } catch { + // A cadence that could not be written means the invite may be served + // again sooner than intended, which is the cheaper failure. + } +} diff --git a/packages/cli/src/survey/constants.ts b/packages/cli/src/survey/constants.ts new file mode 100644 index 00000000..5a93451e --- /dev/null +++ b/packages/cli/src/survey/constants.ts @@ -0,0 +1,96 @@ +/** + * The PostHog survey the CLI answers on the user's behalf, and the map from + * the payload's human keys to its question identifiers. + * + * This is the ONLY place the survey's identifiers live. The agent never sees a + * question UUID: it writes `verbatim`, `goal`, and so on, and `feedback send` + * translates. A mangled UUID would be a silently missing answer; a mangled + * human key is a validation error with a message. + * + * The identifiers are PostHog's. `survey shown`, `survey dismissed`, and + * `survey sent` are its event literals for a custom survey, `$survey_id` and + * `$survey_response_` are its property contract, and the CLI + * adds no survey-specific property of its own. Everything else on the event + * is the standard set the telemetry client stamps on every capture. + * + * `survey shown` means SERVED. The CLI knows it appended the invite to a + * recipe; it cannot know the agent put the question to a person. The funnel + * reads shown ≫ sent by design, and nothing here pretends otherwise. + * + * A question's id changes when its type changes in PostHog. Q3 became single + * choice on 2026-09-17 and took a new id; the one below is current. + */ +export const SURVEY_ID = "01a0b1a0-80fb-0000-5dc1-baa4ec44e619"; + +/** The payload keys, in question order. */ +export type FeedbackKey = + | "verbatim" + | "goal" + | "completed" + | "workedWell" + | "needsImprovement"; + +export interface SurveyQuestion { + key: FeedbackKey; + id: string; + question: string; +} + +export const SURVEY_QUESTIONS: readonly SurveyQuestion[] = [ + { + key: "verbatim", + id: "5feff6a3-6768-4817-92d7-5ae3975c6baa", + question: "What was the user's comments verbatim?", + }, + { + key: "goal", + id: "561e87f4-a1b7-4855-b728-29d19421f7e7", + question: "What was the user trying to accomplish?", + }, + { + key: "completed", + id: "6ebdfabb-3575-49aa-857c-47b6bbfdebc8", + question: "Did the user successfully complete the task in your opinion?", + }, + { + key: "workedWell", + id: "2316428e-dc3e-4c96-ae67-a6e8c66d7db5", + question: "What steps of the interaction with Taskless worked well?", + }, + { + key: "needsImprovement", + id: "67bedbd9-ca70-4c1c-b1a6-6df830a453dd", + question: + "What steps of the interaction with Taskless could use improvement?", + }, +]; + +/** The choices PostHog holds for `completed`, in its own casing. */ +export const COMPLETED_CHOICES = ["Yes", "No", "Unknown"] as const; + +/** + * The recipes that carry the invite. Used by the gate alone; the events do not + * name the recipe, because the survey contract has no slot for it. + */ +export const SURVEYED_TOPICS: ReadonlySet = new Set([ + "onboard", + "create-sg-rule", + "create-vale-rule", + "create-remote-rule", +]); + +const DAY_MS = 24 * 60 * 60 * 1000; + +/** + * How long serving an invite holds the next one off. Ten days, because the + * surveyed recipes are rule generation, which is the touchiest part of the + * agent experience, and a long quiet gap is a gap in feedback. + */ +export const SHOWN_INTERVAL_MS = 10 * DAY_MS; + +/** + * How long an explicit answer holds the next invite off. Twenty days for both + * a dismissal and a sent response: either is a terminal action the user took, + * and only silence earns the shorter gap. + */ +export const ANSWERED_INTERVAL_MS = 20 * DAY_MS; diff --git a/packages/cli/src/telemetry-run.ts b/packages/cli/src/telemetry-run.ts index 58ebf11a..4404040c 100644 --- a/packages/cli/src/telemetry-run.ts +++ b/packages/cli/src/telemetry-run.ts @@ -4,19 +4,29 @@ import type { TelemetryClient } from "./telemetry"; import { splitRawArguments } from "./util/argv"; import { CLIError } from "./util/cli-error"; +/** + * Top-level commands whose second positional is a verb worth keeping in the + * cli_run `command` property: `rule create` and `rule improve` are different + * actions, and so are `feedback send` and `feedback dismiss` (send can fail + * with INVALID_INPUT, dismiss cannot). + */ +const VERB_COMMANDS = new Set(["rule", "feedback"]); + /** * Derive the cli_run `command` property from the raw argv. Flags (and the * value after `-d`/`--dir`) are skipped; the first positional is the command, - * and `rule` keeps its subcommand (e.g. `rule create`) since that distinction - * is meaningful. `agent`'s topic is recorded separately on cli_agent, so the - * command for an agent invocation is just `agent`. + * and `rule` and `feedback` keep their subcommand (e.g. `rule create`, + * `feedback send`) since that distinction is meaningful. `agent`'s topic is + * recorded separately on cli_agent, so the command for an agent invocation is + * just `agent`. */ export function resolveCommandName(rawArguments: string[]): string { const { positionals } = splitRawArguments(rawArguments); if (positionals.length === 0) return "(default)"; const top = positionals[0]!; - if (top === "rule" && positionals[1]) return `rule ${positionals[1]}`; + if (VERB_COMMANDS.has(top) && positionals[1]) + return `${top} ${positionals[1]}`; return top; } diff --git a/packages/cli/src/telemetry.ts b/packages/cli/src/telemetry.ts index 1a164562..55af0a1f 100644 --- a/packages/cli/src/telemetry.ts +++ b/packages/cli/src/telemetry.ts @@ -49,6 +49,16 @@ function isTelemetryDisabled(): boolean { ); } +/** + * Whether captures reach PostHog at all. The same predicate that hands out + * the no-op client, exposed so a caller that must NOT do work under the + * opt-out (the survey invite, which has nowhere to send an answer) asks this + * rather than restating the two variables. + */ +export function isTelemetryEnabled(): boolean { + return !isTelemetryDisabled(); +} + const noopClient: TelemetryClient = { capture() {}, async shutdown() {}, diff --git a/packages/cli/test/cli-run.test.ts b/packages/cli/test/cli-run.test.ts index c6a1ebe2..5be1932a 100644 --- a/packages/cli/test/cli-run.test.ts +++ b/packages/cli/test/cli-run.test.ts @@ -9,6 +9,9 @@ describe("resolveCommandName", () => { [["check", "--json"], "check"], [["rule", "create"], "rule create"], [["rule"], "rule"], + [["feedback", "send", "--from", "req.json"], "feedback send"], + [["feedback", "dismiss"], "feedback dismiss"], + [["feedback"], "feedback"], [["agent", "route"], "agent"], [["-d", "/tmp", "check"], "check"], [["--dir", "/tmp", "info"], "info"], diff --git a/packages/cli/test/feedback-command.test.ts b/packages/cli/test/feedback-command.test.ts new file mode 100644 index 00000000..f27bce96 --- /dev/null +++ b/packages/cli/test/feedback-command.test.ts @@ -0,0 +1,263 @@ +import { execFile } from "node:child_process"; +import { mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { promisify } from "node:util"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { readNextAsk } from "../src/survey/cadence"; +import { ANSWERED_INTERVAL_MS, SURVEY_ID } from "../src/survey/constants"; +import { CLIError } from "../src/util/cli-error"; + +// Spy on telemetry by mocking the module the command imports, the same way +// agent-telemetry.test.ts does. `enabled` is flipped per test to exercise the +// opt-out path without touching the environment the client reads. +const capture = vi.fn(); +let enabled = true; +vi.mock("../src/telemetry", () => ({ + getTelemetry: vi.fn(() => + Promise.resolve({ capture, shutdown: () => Promise.resolve() }) + ), + isTelemetryEnabled: () => enabled, + shutdownTelemetry: () => Promise.resolve(), +})); + +const { feedbackCommand, buildSurveyResponse } = + await import("../src/commands/feedback"); + +interface RunnableCommand { + run: (context: { + args: Record; + rawArgs: string[]; + }) => Promise; +} + +function verb(name: "dismiss" | "send"): RunnableCommand { + const subCommands = feedbackCommand.subCommands as Record; + return subCommands[name] as RunnableCommand; +} + +const VALID = { + verbatim: "The second rule took three tries but the verify loop caught it.", + goal: "Forbid eval in TypeScript", + completed: "Yes", + needsImprovement: "The first draft used a language name ast-grep rejects.", +}; + +describe("feedback command", () => { + let cwd: string; + let configHome: string; + let logSpy: ReturnType; + let errorSpy: ReturnType; + + beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "tskl-feedback-cwd-")); + configHome = await mkdtemp(join(tmpdir(), "tskl-feedback-config-")); + vi.stubEnv("XDG_CONFIG_HOME", configHome); + enabled = true; + capture.mockClear(); + process.exitCode = undefined; + logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(async () => { + logSpy.mockRestore(); + errorSpy.mockRestore(); + vi.unstubAllEnvs(); + process.exitCode = undefined; + await rm(cwd, { recursive: true, force: true }); + await rm(configHome, { recursive: true, force: true }); + }); + + async function writePayload(payload: unknown): Promise { + const path = join(cwd, ".tmp-feedback.json"); + await writeFile(path, JSON.stringify(payload), "utf8"); + return path; + } + + describe("dismiss", () => { + it("captures survey dismissed with the survey id and nothing else", async () => { + await verb("dismiss").run({ args: { dir: cwd }, rawArgs: [] }); + expect(capture).toHaveBeenCalledTimes(1); + expect(capture).toHaveBeenCalledWith("survey dismissed", { + $survey_id: SURVEY_ID, + }); + }); + + it("holds the next invite off by the answered interval", async () => { + const before = Date.now(); + await verb("dismiss").run({ args: { dir: cwd }, rawArgs: [] }); + const nextAsk = await readNextAsk(SURVEY_ID); + expect(nextAsk).toBeGreaterThanOrEqual(before + ANSWERED_INTERVAL_MS); + expect(nextAsk).toBeLessThanOrEqual(Date.now() + ANSWERED_INTERVAL_MS); + }); + + it("sends nothing under the opt-out, and says so with exit 0", async () => { + enabled = false; + await verb("dismiss").run({ args: { dir: cwd }, rawArgs: [] }); + expect(capture).not.toHaveBeenCalled(); + expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(process.exitCode).toBeUndefined(); + expect(logSpy.mock.calls.flat().join("\n")).toMatch(/disabled/); + }); + }); + + describe("send", () => { + it("captures survey sent as exactly PostHog's keys", async () => { + const from = await writePayload(VALID); + await verb("send").run({ + args: { dir: cwd, from, json: false }, + rawArgs: [], + }); + + expect(capture).toHaveBeenCalledTimes(1); + expect(capture).toHaveBeenCalledWith("survey sent", { + $survey_id: SURVEY_ID, + "$survey_response_5feff6a3-6768-4817-92d7-5ae3975c6baa": VALID.verbatim, + "$survey_response_561e87f4-a1b7-4855-b728-29d19421f7e7": VALID.goal, + "$survey_response_6ebdfabb-3575-49aa-857c-47b6bbfdebc8": "Yes", + "$survey_response_67bedbd9-ca70-4c1c-b1a6-6df830a453dd": + VALID.needsImprovement, + }); + // The unanswered optional (`workedWell`) has no key at all. + const properties = capture.mock.calls[0]![1] as Record; + expect(Object.keys(properties)).toHaveLength(5); + }); + + it("leaves the input file in place and creates no .taskless/", async () => { + const from = await writePayload(VALID); + await verb("send").run({ + args: { dir: cwd, from, json: false }, + rawArgs: [], + }); + expect(await readFile(from, "utf8")).toBe(JSON.stringify(VALID)); + expect(await readdir(cwd)).toEqual([".tmp-feedback.json"]); + }); + + it("holds the next invite off by the answered interval", async () => { + const from = await writePayload(VALID); + const before = Date.now(); + await verb("send").run({ + args: { dir: cwd, from, json: false }, + rawArgs: [], + }); + expect(await readNextAsk(SURVEY_ID)).toBeGreaterThanOrEqual( + before + ANSWERED_INTERVAL_MS + ); + }); + + it("rejects an invalid payload with INVALID_INPUT naming the field, sending nothing", async () => { + const from = await writePayload({ ...VALID, completed: "partially" }); + await expect( + verb("send").run({ args: { dir: cwd, from, json: false }, rawArgs: [] }) + ).rejects.toSatisfy( + (error: unknown) => + error instanceof CLIError && error.code === "INVALID_INPUT" + ); + expect(errorSpy.mock.calls.flat().join("\n")).toContain("completed"); + expect(capture).not.toHaveBeenCalled(); + expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(process.exitCode).toBe(1); + }); + + it("rejects a missing --from", async () => { + await expect( + verb("send").run({ args: { dir: cwd, json: false }, rawArgs: [] }) + ).rejects.toBeInstanceOf(CLIError); + expect(capture).not.toHaveBeenCalled(); + }); + + it("rejects an unreadable file and invalid JSON", async () => { + await expect( + verb("send").run({ + args: { dir: cwd, from: "missing.json", json: false }, + rawArgs: [], + }) + ).rejects.toBeInstanceOf(CLIError); + const from = join(cwd, "bad.json"); + await writeFile(from, "{", "utf8"); + await expect( + verb("send").run({ args: { dir: cwd, from, json: false }, rawArgs: [] }) + ).rejects.toBeInstanceOf(CLIError); + expect(capture).not.toHaveBeenCalled(); + }); + + it("validates before honoring the opt-out, then sends nothing", async () => { + enabled = false; + const bad = await writePayload({ goal: "x" }); + await expect( + verb("send").run({ + args: { dir: cwd, from: bad, json: false }, + rawArgs: [], + }) + ).rejects.toBeInstanceOf(CLIError); + + process.exitCode = undefined; + const good = await writePayload(VALID); + await verb("send").run({ + args: { dir: cwd, from: good, json: false }, + rawArgs: [], + }); + expect(capture).not.toHaveBeenCalled(); + expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(process.exitCode).toBeUndefined(); + }); + }); + + describe("buildSurveyResponse", () => { + it("maps every answered key and no unanswered one", () => { + const properties = buildSurveyResponse({ + verbatim: "v", + goal: "g", + completed: "Unknown", + }); + expect(properties).toEqual({ + $survey_id: SURVEY_ID, + "$survey_response_5feff6a3-6768-4817-92d7-5ae3975c6baa": "v", + "$survey_response_561e87f4-a1b7-4855-b728-29d19421f7e7": "g", + "$survey_response_6ebdfabb-3575-49aa-857c-47b6bbfdebc8": "Unknown", + }); + }); + }); +}); + +describe("feedback in the built CLI", () => { + const execFileAsync = promisify(execFile); + const binPath = resolve(import.meta.dirname, "../dist/index.js"); + + it("is absent from the agent index", async () => { + const { stdout } = await execFileAsync("node", [binPath, "agent"]); + expect(stdout).toContain("Topics:"); + expect(stdout).not.toMatch(/^\s*feedback\b/m); + }); + + it("still serves --help with both verbs", async () => { + const { stdout } = await execFileAsync("node", [ + binPath, + "feedback", + "--help", + ]); + expect(stdout).toContain("dismiss"); + expect(stdout).toContain("send"); + }); + + it("emits the JSON error envelope for a missing --from", async () => { + const failure = await execFileAsync("node", [ + binPath, + "feedback", + "send", + "--json", + ]).then( + () => {}, + (error: { stdout: string; code: number }) => error + ); + expect(failure?.code).toBe(1); + const envelope = JSON.parse(failure?.stdout ?? "") as { + ok: boolean; + code: string; + }; + expect(envelope).toMatchObject({ ok: false, code: "INVALID_INPUT" }); + }); +}); diff --git a/packages/cli/test/feedback-schema.test.ts b/packages/cli/test/feedback-schema.test.ts new file mode 100644 index 00000000..ca549138 --- /dev/null +++ b/packages/cli/test/feedback-schema.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from "vitest"; + +import { inputSchema } from "../src/schemas/feedback"; + +const valid = { + verbatim: "It worked but the second rule took three tries.", + goal: "Add an ast-grep rule that forbids eval", + completed: "Yes", +}; + +describe("feedback payload schema", () => { + it("accepts the three required answers alone", () => { + const parsed = inputSchema.parse(valid); + expect(parsed.workedWell).toBeUndefined(); + expect(parsed.needsImprovement).toBeUndefined(); + }); + + it("accepts the optional answers when present", () => { + const parsed = inputSchema.parse({ + ...valid, + workedWell: "The verify loop.", + needsImprovement: "The first draft's language field.", + }); + expect(parsed.workedWell).toBe("The verify loop."); + }); + + it.each(["partially", "yes", "true", ""])( + "rejects completed: %j, naming the field", + (completed) => { + const result = inputSchema.safeParse({ ...valid, completed }); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues.map((issue) => issue.path[0])).toContain( + "completed" + ); + } + ); + + it("rejects a missing verbatim, naming the field", () => { + const { verbatim: _verbatim, ...rest } = valid; + const result = inputSchema.safeParse(rest); + expect(result.success).toBe(false); + if (result.success) return; + expect(result.error.issues.map((issue) => issue.path[0])).toContain( + "verbatim" + ); + }); + + it("rejects an optional answer that is present but blank", () => { + // Blank is not "unanswered": an agent that wrote the key meant to answer. + // Omitting the key is how a question is left unanswered. + const result = inputSchema.safeParse({ ...valid, workedWell: " " }); + expect(result.success).toBe(false); + }); + + it("never takes a survey key from the agent", () => { + // The map to question ids is the CLI's. A payload that tries to carry one + // is not rejected (zod strips unknown keys), and the stripped key never + // reaches the event; feedback-command.test.ts asserts the event shape. + const parsed = inputSchema.parse({ + ...valid, + "$survey_response_5feff6a3-6768-4817-92d7-5ae3975c6baa": "smuggled", + }); + expect(Object.keys(parsed)).not.toContain( + "$survey_response_5feff6a3-6768-4817-92d7-5ae3975c6baa" + ); + }); +}); diff --git a/packages/cli/test/survey-cadence.test.ts b/packages/cli/test/survey-cadence.test.ts new file mode 100644 index 00000000..ff810ccd --- /dev/null +++ b/packages/cli/test/survey-cadence.test.ts @@ -0,0 +1,98 @@ +import { mkdtemp, readFile, rm, writeFile, mkdir } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { nextAskPath, readNextAsk, writeNextAsk } from "../src/survey/cadence"; +import { + ANSWERED_INTERVAL_MS, + COMPLETED_CHOICES, + SHOWN_INTERVAL_MS, + SURVEY_ID, + SURVEY_QUESTIONS, + SURVEYED_TOPICS, +} from "../src/survey/constants"; + +describe("survey constants", () => { + // The identifiers are PostHog's, transcribed once. Q3's id changed when the + // question became single choice, which is exactly the kind of drift this + // pins: the value here is what the live survey holds as of 2026-09-17. + it("carries the live survey's question ids in question order", () => { + expect(SURVEY_ID).toBe("01a0b1a0-80fb-0000-5dc1-baa4ec44e619"); + expect(SURVEY_QUESTIONS.map(({ key, id }) => [key, id])).toEqual([ + ["verbatim", "5feff6a3-6768-4817-92d7-5ae3975c6baa"], + ["goal", "561e87f4-a1b7-4855-b728-29d19421f7e7"], + ["completed", "6ebdfabb-3575-49aa-857c-47b6bbfdebc8"], + ["workedWell", "2316428e-dc3e-4c96-ae67-a6e8c66d7db5"], + ["needsImprovement", "67bedbd9-ca70-4c1c-b1a6-6df830a453dd"], + ]); + }); + + it("holds PostHog's choices for the single-choice question, in its casing", () => { + expect(COMPLETED_CHOICES).toEqual(["Yes", "No", "Unknown"]); + }); + + it("surveys the four authoring and onboarding recipes only", () => { + expect([...SURVEYED_TOPICS].toSorted()).toEqual([ + "create-remote-rule", + "create-sg-rule", + "create-vale-rule", + "onboard", + ]); + }); + + it("holds an explicit answer off longer than a served invite", () => { + expect(SHOWN_INTERVAL_MS).toBe(10 * 24 * 60 * 60 * 1000); + expect(ANSWERED_INTERVAL_MS).toBe(20 * 24 * 60 * 60 * 1000); + }); +}); + +describe("survey cadence store", () => { + let configHome: string; + + beforeEach(async () => { + configHome = await mkdtemp(join(tmpdir(), "tskl-cadence-")); + vi.stubEnv("XDG_CONFIG_HOME", configHome); + }); + + afterEach(async () => { + vi.unstubAllEnvs(); + await rm(configHome, { recursive: true, force: true }); + }); + + it("lives under the survey id in the XDG config directory", () => { + expect(nextAskPath(SURVEY_ID)).toBe( + join(configHome, "taskless", "surveys", SURVEY_ID, "next_ask") + ); + }); + + it("reads absent as undefined", async () => { + expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + }); + + it("round-trips an epoch, truncated to whole milliseconds", async () => { + const at = Date.now() + SHOWN_INTERVAL_MS; + await writeNextAsk(SURVEY_ID, at + 0.75); + expect(await readNextAsk(SURVEY_ID)).toBe(at); + // A bare decimal string, nothing else, so a human can read it. + expect(await readFile(nextAskPath(SURVEY_ID), "utf8")).toBe(String(at)); + }); + + it("reads a corrupt file as undefined, and the next write repairs it", async () => { + const path = nextAskPath(SURVEY_ID); + await mkdir(dirname(path), { recursive: true }); + await writeFile(path, "not a number\n", "utf8"); + expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + + await writeNextAsk(SURVEY_ID, 1234); + expect(await readNextAsk(SURVEY_ID)).toBe(1234); + }); + + it("keeps a different survey's cadence in its own file", async () => { + await writeNextAsk(SURVEY_ID, 1000); + expect(await readNextAsk("00000000-0000-4000-8000-000000000000")).toBe( + undefined + ); + }); +});