diff --git a/openspec/changes/feedback-channels/tasks.md b/openspec/changes/feedback-channels/tasks.md index 0ebc3284..b5e4ee50 100644 --- a/openspec/changes/feedback-channels/tasks.md +++ b/openspec/changes/feedback-channels/tasks.md @@ -1,8 +1,8 @@ ## 1. Survey registry and payload schema -- [ ] 1.1 Restructure `packages/cli/src/survey/constants.ts` into a `SURVEYS` registry keyed by `rule` / `general` / `bug` (ids and question ids per design.md's table), keep a `RULE_SURVEY_ID` export, and point `survey/invite.ts` and the cadence calls at it; verify `pnpm --filter @taskless/cli typecheck` passes and `survey-invite.test.ts` / `survey-cadence.test.ts` still pass unchanged -- [ ] 1.2 Rewrite `packages/cli/src/schemas/feedback.ts` as a `z.discriminatedUnion("kind", …)` of three `z.strictObject` branches, each declaring `kind` as a required `z.literal(…)` with no default, exporting each branch schema; verify with new cases in `feedback-schema.test.ts`: missing `kind` names `kind`, a `general` payload with `ruleKind` names `ruleKind`, a `bug` payload without `expected` names `expected`, each branch's rendered JSON Schema lists `kind` as required with a single `const`, and every existing rule-schema case passes with `kind: "rule"` added -- [ ] 1.3 Make `buildSurveyResponse` map through `SURVEYS[input.kind]`; verify a `feedback-command.test.ts` case per kind asserts the `$survey_id` and the exact set of `$survey_response_` keys +- [x] 1.1 Restructure `packages/cli/src/survey/constants.ts` into a `SURVEYS` registry keyed by `rule` / `general` / `bug` (ids and question ids per design.md's table), keep a `RULE_SURVEY_ID` export, and point `survey/invite.ts` and the cadence calls at it; verify `pnpm --filter @taskless/cli typecheck` passes and `survey-invite.test.ts` / `survey-cadence.test.ts` still pass unchanged +- [x] 1.2 Rewrite `packages/cli/src/schemas/feedback.ts` as a `z.discriminatedUnion("kind", …)` of three `z.strictObject` branches, each declaring `kind` as a required `z.literal(…)` with no default, exporting each branch schema; verify with new cases in `feedback-schema.test.ts`: missing `kind` names `kind`, a `general` payload with `ruleKind` names `ruleKind`, a `bug` payload without `expected` names `expected`, each branch's rendered JSON Schema lists `kind` as required with a single `const`, and every existing rule-schema case passes with `kind: "rule"` added +- [x] 1.3 Make `buildSurveyResponse` map through `SURVEYS[input.kind]`; verify a `feedback-command.test.ts` case per kind asserts the `$survey_id` and the exact set of `$survey_response_` keys ## 2. `feedback send` behavior diff --git a/packages/cli/src/commands/feedback.ts b/packages/cli/src/commands/feedback.ts index ce35c70c..f9d06627 100644 --- a/packages/cli/src/commands/feedback.ts +++ b/packages/cli/src/commands/feedback.ts @@ -7,8 +7,8 @@ import { inputSchema, type FeedbackInput } from "../schemas/feedback"; import { writeNextAsk } from "../survey/cadence"; import { ANSWERED_INTERVAL_MS, - SURVEY_ID, - SURVEY_QUESTIONS, + RULE_SURVEY_ID, + SURVEYS, } from "../survey/constants"; import { getTelemetry, isTelemetryEnabled } from "../telemetry"; import { type CLIErrorCode, writeJsonError } from "../types/errors"; @@ -28,17 +28,23 @@ const NOTHING_SENT = /** * 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. + * Exactly PostHog's contract: `$survey_id` of the survey the payload's `kind` + * selects, 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. A question with no + * payload key is the CLI's to answer, and is skipped here. */ export function buildSurveyResponse( input: FeedbackInput ): Record { - const properties: Record = { $survey_id: SURVEY_ID }; - for (const { key, id } of SURVEY_QUESTIONS) { - const answer = input[key]; + const survey = SURVEYS[input.kind]; + // The branches share no key type, so the payload is read as a plain record; + // the schema has already decided which keys it may hold. + const answers = input as Readonly>; + const properties: Record = { $survey_id: survey.id }; + for (const { key, id } of survey.questions) { + if (key === undefined) continue; + const answer = answers[key]; if (answer !== undefined) properties[`$survey_response_${id}`] = answer; } return properties; @@ -63,8 +69,8 @@ const dismissCommand = defineCommand({ return; } const telemetry = await getTelemetry(cwd); - telemetry.capture("survey dismissed", { $survey_id: SURVEY_ID }); - await writeNextAsk(SURVEY_ID, Date.now() + ANSWERED_INTERVAL_MS); + telemetry.capture("survey dismissed", { $survey_id: RULE_SURVEY_ID }); + await writeNextAsk(RULE_SURVEY_ID, Date.now() + ANSWERED_INTERVAL_MS); console.log("Thanks. Taskless will not ask again for a while."); }, }); @@ -133,7 +139,7 @@ const sendCommand = defineCommand({ const telemetry = await getTelemetry(cwd); telemetry.capture("survey sent", buildSurveyResponse(input)); - await writeNextAsk(SURVEY_ID, Date.now() + ANSWERED_INTERVAL_MS); + await writeNextAsk(RULE_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."); diff --git a/packages/cli/src/prompts/recipes.ts b/packages/cli/src/prompts/recipes.ts index 70143069..16e4be44 100644 --- a/packages/cli/src/prompts/recipes.ts +++ b/packages/cli/src/prompts/recipes.ts @@ -8,7 +8,7 @@ import { } from "../util/invocation"; import { inputSchema as ruleCreateInputSchema } from "../schemas/rules-create"; import { inputSchema as ruleImproveInputSchema } from "../schemas/rules-improve"; -import { inputSchema as feedbackInputSchema } from "../schemas/feedback"; +import { ruleInputSchema } from "../schemas/feedback"; import { AST_GREP_VERSION, VALE_VERSION, @@ -80,7 +80,7 @@ export function canonicalRecipeTopics(): string[] { const TOPIC_INPUT_SCHEMAS: Record = { "create-remote-rule": ruleCreateInputSchema, "improve-rule": ruleImproveInputSchema, - feedback: feedbackInputSchema, + feedback: ruleInputSchema, }; /** Agent-fill marker used when the caller does not supply a real value. */ diff --git a/packages/cli/src/schemas/feedback.ts b/packages/cli/src/schemas/feedback.ts index 1f613a75..093a59dd 100644 --- a/packages/cli/src/schemas/feedback.ts +++ b/packages/cli/src/schemas/feedback.ts @@ -1,21 +1,50 @@ import { z } from "zod"; -import { COMPLETED_CHOICES } from "../survey/constants"; +import { COMPLETED_CHOICES, FEEDBACK_KINDS } 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 + * Human keys only. The map to each survey's question identifiers lives in * `src/survey/constants.ts`, and a payload never carries a `$survey_*` key. + * + * `kind` is the discriminator: a required literal on every branch, never + * defaulted. Each branch is strict, so a key belonging to another kind fails + * naming that key instead of being dropped on the way to the event. Each + * recipe embeds only its own branch, so the agent reading `bug-report` never + * sees the rule survey's keys. */ -export const inputSchema = z.object({ - ruleKind: z + +/** An answer the payload must carry. */ +function requiredAnswer(key: string, description: string) { + return z .string() .trim() - .min(1, "ruleKind must be a non-empty string") - .describe( - "The kind of rule the user was trying to create: the engine and what the rule was for. `none (onboarding)` on the onboarding path" - ), + .min(1, `${key} must be a non-empty string`) + .describe(description); +} + +/** + * An answer the payload may leave out. Blank is not "unanswered": an agent + * that wrote the key meant to answer, and omitting the key is how a question + * is left unanswered. + */ +function optionalAnswer(key: string, description: string) { + return z + .string() + .trim() + .min(1, `${key}, when present, must be non-empty`) + .optional() + .describe(description); +} + +/** The invited survey about rule authoring and onboarding. */ +export const ruleInputSchema = z.strictObject({ + kind: z.literal("rule").describe("Always `rule` for this survey"), + ruleKind: requiredAnswer( + "ruleKind", + "The kind of rule the user was trying to create: the engine and what the rule was for. `none (onboarding)` on the onboarding path" + ), verbatim: z .string() .trim() @@ -32,36 +61,62 @@ export const inputSchema = z.object({ .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" - ), - agents: z - .string() - .trim() - .min(1, "agents, when present, must be non-empty") - .optional() - .describe( - "The open-source, publicly available agent(s) or framework(s) the user is working through, including the one running this recipe" - ), - mostValuableRule: z - .string() - .trim() - .min(1, "mostValuableRule, when present, must be non-empty") - .optional() - .describe( - "Of the rules created so far, the one creating the most value for the team and why. Omit when there are no rules or you cannot tell" - ), + workedWell: optionalAnswer( + "workedWell", + "Steps of the interaction with Taskless that worked well" + ), + needsImprovement: optionalAnswer( + "needsImprovement", + "Steps of the interaction with Taskless that could use improvement" + ), + agents: optionalAnswer( + "agents", + "The open-source, publicly available agent(s) or framework(s) the user is working through, including the one running this recipe" + ), + mostValuableRule: optionalAnswer( + "mostValuableRule", + "Of the rules created so far, the one creating the most value for the team and why. Omit when there are no rules or you cannot tell" + ), }); +/** Feedback the user chose to give about Taskless. */ +export const generalInputSchema = z.strictObject({ + kind: z.literal("general").describe("Always `general` for this survey"), + verbatim: requiredAnswer( + "verbatim", + "The user's feedback, in their own words, unedited" + ), + context: optionalAnswer( + "context", + "What led to the feedback, from what you observed in the session, as the user approved it. Omit when there is nothing to add" + ), +}); + +/** + * A bug report. The survey's version-information question has no key here: + * the CLI answers it itself, so the agent can neither get it wrong nor leave + * it out. + */ +export const bugInputSchema = z.strictObject({ + kind: z.literal("bug").describe("Always `bug` for this survey"), + summary: requiredAnswer("summary", "A one-line summary of the bug"), + trying: requiredAnswer("trying", "What the user was trying to do"), + expected: requiredAnswer("expected", "The result the user expected"), + actual: requiredAnswer("actual", "The result the user actually got"), + context: optionalAnswer( + "context", + "Anything else that would help fix it: the command run, the error text, the steps to reproduce. Omit when there is nothing to add" + ), +}); + +export const inputSchema = z.discriminatedUnion( + "kind", + [ruleInputSchema, generalInputSchema, bugInputSchema], + { + // Zod's own message for a missing or unknown discriminator is "Invalid + // input", which tells the agent nothing about what to write. + error: `kind must be one of ${FEEDBACK_KINDS.map((kind) => `"${kind}"`).join(", ")}`, + } +); + export type FeedbackInput = z.infer; diff --git a/packages/cli/src/survey/constants.ts b/packages/cli/src/survey/constants.ts index d426fc35..b4c71a62 100644 --- a/packages/cli/src/survey/constants.ts +++ b/packages/cli/src/survey/constants.ts @@ -1,11 +1,11 @@ /** - * The PostHog survey the CLI answers on the user's behalf, and the map from - * the payload's human keys to its question identifiers. + * The PostHog surveys the CLI answers on the user's behalf, and the map from + * each payload's human keys to the survey's 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. + * This is the ONLY place the surveys' identifiers live. The agent never sees a + * question UUID: it writes `verbatim`, `summary`, 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 @@ -17,70 +17,141 @@ * 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 is PostHog's and changes whenever the question does. The - * survey below replaced `01a0b1a0-80fb-0000-5dc1-baa4ec44e619` for 0.12.0: - * seven questions, only the first required, every id new. The cadence store - * is keyed by survey id, so every install is invited once more. + * Three surveys, selected by the payload's `kind`: + * + * - `rule` is the invited survey about rule authoring and onboarding. It is + * the only one with an invite, a cadence, and a dismissal. It replaced + * `01a0b1a0-80fb-0000-5dc1-baa4ec44e619` for 0.12.0, and the cadence store + * is keyed by survey id, so that move invited every install once more. + * - `general` is feedback the user chose to give. + * - `bug` is a bug report. It is how a user without a GitHub account reaches + * the team. Its version-information question has no payload key: the CLI + * answers it itself (see `bugVersionInformation` in `commands/feedback.ts`). + * + * A question's id is PostHog's and changes whenever the question does. */ -export const SURVEY_ID = "01a0c7b9-dfe4-0000-d05e-ce253e90a68c"; +export const FEEDBACK_KINDS = ["rule", "general", "bug"] as const; -/** The payload keys, in question order. */ -export type FeedbackKey = - | "ruleKind" - | "verbatim" - | "completed" - | "workedWell" - | "needsImprovement" - | "agents" - | "mostValuableRule"; +export type FeedbackKind = (typeof FEEDBACK_KINDS)[number]; export interface SurveyQuestion { - key: FeedbackKey; + /** The payload key that answers it, or `undefined` when the CLI does. */ + key: string | undefined; id: string; question: string; } -export const SURVEY_QUESTIONS: readonly SurveyQuestion[] = [ - { - key: "ruleKind", - id: "0874591f-c554-4ac3-8930-e11c436d859e", - question: "What kind of rule was the user trying to create?", - }, - { - key: "verbatim", - id: "2c3c80dc-dcda-4e29-b52e-a25ef58b5ca2", - question: "Did the user offer any comments? (leave blank if no comments)", - }, - { - key: "completed", - id: "605e12a8-82b6-480f-93b2-ab8de0fa08bd", - question: "Did the user successfully complete the task in your opinion?", - }, - { - key: "workedWell", - id: "b5375d87-e295-4833-84ed-fca8140ba992", - question: - "What steps of the interaction with the Taskless skills & CLI worked well?", - }, - { - key: "needsImprovement", - id: "a8cf706d-3ff7-4845-bea9-501013be958c", - question: - "What steps of the interaction with Taskless skills & CLI could use improvement?", +export interface Survey { + id: string; + questions: readonly SurveyQuestion[]; +} + +export const SURVEYS: Readonly> = { + rule: { + id: "01a0c7b9-dfe4-0000-d05e-ce253e90a68c", + questions: [ + { + key: "ruleKind", + id: "0874591f-c554-4ac3-8930-e11c436d859e", + question: "What kind of rule was the user trying to create?", + }, + { + key: "verbatim", + id: "2c3c80dc-dcda-4e29-b52e-a25ef58b5ca2", + question: + "Did the user offer any comments? (leave blank if no comments)", + }, + { + key: "completed", + id: "605e12a8-82b6-480f-93b2-ab8de0fa08bd", + question: + "Did the user successfully complete the task in your opinion?", + }, + { + key: "workedWell", + id: "b5375d87-e295-4833-84ed-fca8140ba992", + question: + "What steps of the interaction with the Taskless skills & CLI worked well?", + }, + { + key: "needsImprovement", + id: "a8cf706d-3ff7-4845-bea9-501013be958c", + question: + "What steps of the interaction with Taskless skills & CLI could use improvement?", + }, + { + key: "agents", + id: "f85b22df-8e51-4c9c-8219-261b33b71c90", + question: + "What agent(s) or framework(s) is the user using that are open sourced and publicly available?", + }, + { + key: "mostValuableRule", + id: "4f8e938e-22f6-449c-9b8c-43c51d08e214", + question: + "Of the rules created so far, what rule is creating the most value for the team and why? (leave blank if there are no rules)", + }, + ], }, - { - key: "agents", - id: "f85b22df-8e51-4c9c-8219-261b33b71c90", - question: - "What agent(s) or framework(s) is the user using that are open sourced and publicly available?", + general: { + id: "01a11da4-3948-0000-4ae4-c9da9321801e", + questions: [ + { + key: "verbatim", + id: "c71e52ee-f4c7-469f-815a-af50a9be6d37", + question: "Feedback", + }, + { + key: "context", + id: "ab0ceb25-8084-44c8-9974-1d1a1c7371c2", + question: "Attach any additional context", + }, + ], }, - { - key: "mostValuableRule", - id: "4f8e938e-22f6-449c-9b8c-43c51d08e214", - question: - "Of the rules created so far, what rule is creating the most value for the team and why? (leave blank if there are no rules)", + bug: { + id: "01a11da7-27a2-0000-0f4e-6d3e1f89f385", + questions: [ + { + key: "summary", + id: "2dd63cf3-dac2-4765-ace0-393bc4aa42fe", + question: "One Line Summary", + }, + { + key: undefined, + id: "ba096b81-ae5c-45b5-b15a-707f97129839", + question: "Version Information (taskless info and cli version)", + }, + { + key: "trying", + id: "e59a87e9-9ac7-4a59-a709-a282b16dbf37", + question: "What were you trying to do?", + }, + { + key: "expected", + id: "73415b80-7371-4536-8c50-09c2ecaa5d52", + question: "What was the expected result?", + }, + { + key: "actual", + id: "daa98d8d-113e-4e3e-ac0f-595ecc1fc69f", + question: "What was the actual result?", + }, + { + key: "context", + id: "597381f5-52e9-4937-bf80-dc55739e7435", + question: + "Any additional information or context that can help in fixing this issue", + }, + ], }, -]; +}; + +/** + * The invited survey, by name. The invite, its gate, the cadence, and + * `feedback dismiss` all mean this survey specifically, and spelling it + * `SURVEYS.rule.id` at each of those sites would hide that. + */ +export const RULE_SURVEY_ID = SURVEYS.rule.id; /** The choices PostHog holds for `completed`, in its own casing. */ export const COMPLETED_CHOICES = ["Yes", "No", "Unknown"] as const; diff --git a/packages/cli/src/survey/invite.ts b/packages/cli/src/survey/invite.ts index 49932719..188ca34e 100644 --- a/packages/cli/src/survey/invite.ts +++ b/packages/cli/src/survey/invite.ts @@ -2,7 +2,11 @@ import { getRecipe } from "../prompts/recipes"; import { getTelemetry, isTelemetryEnabled } from "../telemetry"; import { isCiEnvironment } from "../util/interactive"; import { readNextAsk, writeNextAsk } from "./cadence"; -import { SHOWN_INTERVAL_MS, SURVEY_ID, SURVEYED_TOPICS } from "./constants"; +import { + SHOWN_INTERVAL_MS, + RULE_SURVEY_ID, + SURVEYED_TOPICS, +} from "./constants"; /** The fragment appended to a surveyed recipe; see `src/agent/feedback-invite.md`. */ const INVITE_TOPIC = "feedback-invite"; @@ -45,7 +49,7 @@ export async function surveyGateIsOpen( if (!isTelemetryEnabled()) return false; if (isCiEnvironment(context.ci)) return false; if (!SURVEYED_TOPICS.has(context.topic)) return false; - const nextAsk = await readNextAsk(SURVEY_ID); + const nextAsk = await readNextAsk(RULE_SURVEY_ID); const now = (context.now ?? Date.now)(); return nextAsk === undefined || nextAsk <= now; } @@ -85,7 +89,7 @@ export async function withSurveyInvite( // Claim the window first; see the note above on the read-then-write race. const now = (context.now ?? Date.now)(); - await writeNextAsk(SURVEY_ID, now + SHOWN_INTERVAL_MS); + await writeNextAsk(RULE_SURVEY_ID, now + SHOWN_INTERVAL_MS); const invite = getRecipe(INVITE_TOPIC, { invocation: context.invocation, @@ -98,7 +102,7 @@ export async function withSurveyInvite( if (invite === undefined) return recipe; const telemetry = await getTelemetry(context.cwd); - telemetry.capture("survey shown", { $survey_id: SURVEY_ID }); + telemetry.capture("survey shown", { $survey_id: RULE_SURVEY_ID }); return `${recipe}\n\n${invite.trimEnd()}`; } diff --git a/packages/cli/test/feedback-command.test.ts b/packages/cli/test/feedback-command.test.ts index 2a8eb8ed..795d67bb 100644 --- a/packages/cli/test/feedback-command.test.ts +++ b/packages/cli/test/feedback-command.test.ts @@ -7,7 +7,7 @@ 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 { ANSWERED_INTERVAL_MS, RULE_SURVEY_ID } from "../src/survey/constants"; import { CLIError } from "../src/util/cli-error"; import { builtCli } from "./support/built-cli"; @@ -40,6 +40,7 @@ function verb(name: "dismiss" | "send"): RunnableCommand { } const VALID = { + kind: "rule", ruleKind: "ast-grep, forbid eval in TypeScript", verbatim: "The second rule took three tries but the verify loop caught it.", completed: "Yes", @@ -83,14 +84,14 @@ describe("feedback command", () => { await verb("dismiss").run({ args: { dir: cwd }, rawArgs: [] }); expect(capture).toHaveBeenCalledTimes(1); expect(capture).toHaveBeenCalledWith("survey dismissed", { - $survey_id: SURVEY_ID, + $survey_id: RULE_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); + const nextAsk = await readNextAsk(RULE_SURVEY_ID); expect(nextAsk).toBeGreaterThanOrEqual(before + ANSWERED_INTERVAL_MS); expect(nextAsk).toBeLessThanOrEqual(Date.now() + ANSWERED_INTERVAL_MS); }); @@ -99,7 +100,7 @@ describe("feedback command", () => { enabled = false; await verb("dismiss").run({ args: { dir: cwd }, rawArgs: [] }); expect(capture).not.toHaveBeenCalled(); - expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(await readNextAsk(RULE_SURVEY_ID)).toBeUndefined(); expect(process.exitCode).toBeUndefined(); expect(logSpy.mock.calls.flat().join("\n")).toMatch(/disabled/); }); @@ -115,7 +116,7 @@ describe("feedback command", () => { expect(capture).toHaveBeenCalledTimes(1); expect(capture).toHaveBeenCalledWith("survey sent", { - $survey_id: SURVEY_ID, + $survey_id: RULE_SURVEY_ID, "$survey_response_0874591f-c554-4ac3-8930-e11c436d859e": VALID.ruleKind, "$survey_response_2c3c80dc-dcda-4e29-b52e-a25ef58b5ca2": VALID.verbatim, "$survey_response_605e12a8-82b6-480f-93b2-ab8de0fa08bd": "Yes", @@ -130,13 +131,16 @@ describe("feedback command", () => { it("sends the agent's account alone when the user gave no words", async () => { // A `skip` reply is not a dismissal: the payload omits `verbatim` and // the rest still goes, keyed to the survey's required question. - const from = await writePayload({ ruleKind: "none (onboarding)" }); + const from = await writePayload({ + kind: "rule", + ruleKind: "none (onboarding)", + }); await verb("send").run({ args: { dir: cwd, from, json: false }, rawArgs: [], }); expect(capture).toHaveBeenCalledWith("survey sent", { - $survey_id: SURVEY_ID, + $survey_id: RULE_SURVEY_ID, "$survey_response_0874591f-c554-4ac3-8930-e11c436d859e": "none (onboarding)", }); @@ -159,7 +163,7 @@ describe("feedback command", () => { args: { dir: cwd, from, json: false }, rawArgs: [], }); - expect(await readNextAsk(SURVEY_ID)).toBeGreaterThanOrEqual( + expect(await readNextAsk(RULE_SURVEY_ID)).toBeGreaterThanOrEqual( before + ANSWERED_INTERVAL_MS ); }); @@ -174,7 +178,7 @@ describe("feedback command", () => { ); expect(errorSpy.mock.calls.flat().join("\n")).toContain("completed"); expect(capture).not.toHaveBeenCalled(); - expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(await readNextAsk(RULE_SURVEY_ID)).toBeUndefined(); expect(process.exitCode).toBe(1); }); @@ -217,7 +221,7 @@ describe("feedback command", () => { rawArgs: [], }); expect(capture).not.toHaveBeenCalled(); - expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(await readNextAsk(RULE_SURVEY_ID)).toBeUndefined(); expect(process.exitCode).toBeUndefined(); }); }); @@ -225,19 +229,48 @@ describe("feedback command", () => { describe("buildSurveyResponse", () => { it("maps every answered key and no unanswered one", () => { const properties = buildSurveyResponse({ + kind: "rule", ruleKind: "r", verbatim: "v", completed: "Unknown", agents: "Claude Code", }); expect(properties).toEqual({ - $survey_id: SURVEY_ID, + $survey_id: RULE_SURVEY_ID, "$survey_response_0874591f-c554-4ac3-8930-e11c436d859e": "r", "$survey_response_2c3c80dc-dcda-4e29-b52e-a25ef58b5ca2": "v", "$survey_response_605e12a8-82b6-480f-93b2-ab8de0fa08bd": "Unknown", "$survey_response_f85b22df-8e51-4c9c-8219-261b33b71c90": "Claude Code", }); }); + + it("maps a general payload to the general survey", () => { + expect( + buildSurveyResponse({ kind: "general", verbatim: "v", context: "c" }) + ).toEqual({ + $survey_id: "01a11da4-3948-0000-4ae4-c9da9321801e", + "$survey_response_c71e52ee-f4c7-469f-815a-af50a9be6d37": "v", + "$survey_response_ab0ceb25-8084-44c8-9974-1d1a1c7371c2": "c", + }); + }); + + it("maps a bug payload to the bug survey", () => { + expect( + buildSurveyResponse({ + kind: "bug", + summary: "s", + trying: "t", + expected: "e", + actual: "a", + }) + ).toEqual({ + $survey_id: "01a11da7-27a2-0000-0f4e-6d3e1f89f385", + "$survey_response_2dd63cf3-dac2-4765-ace0-393bc4aa42fe": "s", + "$survey_response_e59a87e9-9ac7-4a59-a709-a282b16dbf37": "t", + "$survey_response_73415b80-7371-4536-8c50-09c2ecaa5d52": "e", + "$survey_response_daa98d8d-113e-4e3e-ac0f-595ecc1fc69f": "a", + }); + }); }); }); diff --git a/packages/cli/test/feedback-recipes.test.ts b/packages/cli/test/feedback-recipes.test.ts index e410a541..7876ed2c 100644 --- a/packages/cli/test/feedback-recipes.test.ts +++ b/packages/cli/test/feedback-recipes.test.ts @@ -4,7 +4,7 @@ import { promisify } from "node:util"; import { describe, expect, it } from "vitest"; import { getRecipe } from "../src/prompts/recipes"; -import { inputSchema } from "../src/schemas/feedback"; +import { ruleInputSchema } from "../src/schemas/feedback"; import { COMPLETED_CHOICES } from "../src/survey/constants"; import { builtCli } from "./support/built-cli"; @@ -38,7 +38,7 @@ describe("the feedback recipe", () => { for (const choice of COMPLETED_CHOICES) { expect(stdout).toContain(`"${choice}"`); } - for (const key of Object.keys(inputSchema.shape)) { + for (const key of Object.keys(ruleInputSchema.shape)) { expect(stdout).toContain(`"${key}"`); } }); diff --git a/packages/cli/test/feedback-schema.test.ts b/packages/cli/test/feedback-schema.test.ts index 1f220ed8..571b765c 100644 --- a/packages/cli/test/feedback-schema.test.ts +++ b/packages/cli/test/feedback-schema.test.ts @@ -1,73 +1,160 @@ +import { z } from "zod"; import { describe, expect, it } from "vitest"; -import { inputSchema } from "../src/schemas/feedback"; +import { + bugInputSchema, + generalInputSchema, + inputSchema, + ruleInputSchema, +} from "../src/schemas/feedback"; const valid = { + kind: "rule", ruleKind: "ast-grep, forbid eval in TypeScript", verbatim: "It worked but the second rule took three tries.", completed: "Yes", }; +const validBug = { + kind: "bug", + summary: "check exits 0 when a rule file fails to parse", + trying: "Run taskless check after adding a rule", + expected: "A non-zero exit naming the broken rule", + actual: "Exit 0 with no findings", +}; + +/** The first path segment of each issue, or the unrecognized keys it names. */ +function failingFields(payload: unknown): unknown[] { + const result = inputSchema.safeParse(payload); + expect(result.success).toBe(false); + if (result.success) return []; + return result.error.issues.flatMap((issue) => + issue.code === "unrecognized_keys" ? issue.keys : [issue.path[0]] + ); +} + describe("feedback payload schema", () => { - it("accepts the one required answer alone", () => { - // `skip` at the invite leaves the agent's account and nothing else. - const parsed = inputSchema.parse({ ruleKind: "none (onboarding)" }); - expect(parsed.verbatim).toBeUndefined(); - expect(parsed.completed).toBeUndefined(); - expect(Object.keys(parsed)).toEqual(["ruleKind"]); - }); + describe("kind: rule", () => { + it("accepts the one required answer alone", () => { + // `skip` at the invite leaves the agent's account and nothing else. + const parsed = inputSchema.parse({ + kind: "rule", + ruleKind: "none (onboarding)", + }); + expect(Object.keys(parsed)).toEqual(["kind", "ruleKind"]); + }); + + it("accepts the optional answers when present", () => { + const parsed = ruleInputSchema.parse({ + ...valid, + workedWell: "The verify loop.", + needsImprovement: "The first draft's language field.", + agents: "Claude Code", + mostValuableRule: "no-eval: it caught two uses in the first check.", + }); + expect(parsed.workedWell).toBe("The verify loop."); + expect(parsed.agents).toBe("Claude Code"); + }); - it("accepts the optional answers when present", () => { - const parsed = inputSchema.parse({ - ...valid, - workedWell: "The verify loop.", - needsImprovement: "The first draft's language field.", - agents: "Claude Code", - mostValuableRule: "no-eval: it caught two uses in the first check.", + it.each(["partially", "yes", "true", ""])( + "rejects completed: %j, naming the field", + (completed) => { + expect(failingFields({ ...valid, completed })).toContain("completed"); + } + ); + + it("rejects a missing ruleKind, naming the field", () => { + const { ruleKind: _ruleKind, ...rest } = valid; + expect(failingFields(rest)).toContain("ruleKind"); + }); + + 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. + expect(failingFields({ ...valid, workedWell: " " })).toContain( + "workedWell" + ); }); - expect(parsed.workedWell).toBe("The verify loop."); - expect(parsed.agents).toBe("Claude Code"); }); - 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" + describe("kind: general", () => { + it("accepts the user's words alone", () => { + const parsed = inputSchema.parse({ + kind: "general", + verbatim: "The recipes are long.", + }); + expect(Object.keys(parsed)).toEqual(["kind", "verbatim"]); + }); + + it("requires the user's words", () => { + expect(failingFields({ kind: "general", context: "x" })).toContain( + "verbatim" ); - } - ); + }); - it("rejects a missing ruleKind, naming the field", () => { - const { ruleKind: _ruleKind, ...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( - "ruleKind" - ); + it("rejects a key belonging to another kind, naming it", () => { + expect( + failingFields({ kind: "general", verbatim: "x", ruleKind: "y" }) + ).toContain("ruleKind"); + }); }); - 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); + describe("kind: bug", () => { + it("accepts its four required answers", () => { + expect(inputSchema.parse(validBug)).toEqual(validBug); + }); + + it.each(["summary", "trying", "expected", "actual"])( + "requires %s, naming it", + (key) => { + const payload: Record = { ...validBug }; + delete payload[key]; + expect(failingFields(payload)).toContain(key); + } + ); + + it("takes no version information from the agent", () => { + expect(failingFields({ ...validBug, version: "0.11.3" })).toContain( + "version" + ); + }); }); - 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_2c3c80dc-dcda-4e29-b52e-a25ef58b5ca2": "smuggled", + describe("the kind discriminator", () => { + it("rejects a payload without a kind, naming kind", () => { + expect( + failingFields({ ruleKind: "ast-grep, forbid eval in TypeScript" }) + ).toEqual(["kind"]); + }); + + it("rejects an unknown kind, naming kind", () => { + expect(failingFields({ kind: "praise", verbatim: "x" })).toEqual([ + "kind", + ]); }); - expect(Object.keys(parsed)).not.toContain( - "$survey_response_2c3c80dc-dcda-4e29-b52e-a25ef58b5ca2" + + it.each([ + ["rule", ruleInputSchema], + ["general", generalInputSchema], + ["bug", bugInputSchema], + ] as const)( + "renders %s's kind as a required single const for the recipe", + (kind, schema) => { + const rendered = z.toJSONSchema(schema); + expect(rendered.required).toContain("kind"); + expect(rendered.properties?.kind).toMatchObject({ const: kind }); + } ); }); + + it("never takes a survey key from the agent", () => { + // The map to question ids is the CLI's. Every branch is strict, so a + // payload that tries to carry one is rejected outright. + expect( + failingFields({ + ...valid, + "$survey_response_2c3c80dc-dcda-4e29-b52e-a25ef58b5ca2": "smuggled", + }) + ).toContain("$survey_response_2c3c80dc-dcda-4e29-b52e-a25ef58b5ca2"); + }); }); diff --git a/packages/cli/test/survey-cadence.test.ts b/packages/cli/test/survey-cadence.test.ts index 52d824f4..4285a564 100644 --- a/packages/cli/test/survey-cadence.test.ts +++ b/packages/cli/test/survey-cadence.test.ts @@ -9,18 +9,19 @@ import { ANSWERED_INTERVAL_MS, COMPLETED_CHOICES, SHOWN_INTERVAL_MS, - SURVEY_ID, - SURVEY_QUESTIONS, + RULE_SURVEY_ID, SURVEYED_TOPICS, + SURVEYS, } from "../src/survey/constants"; describe("survey constants", () => { // The identifiers are PostHog's, transcribed once. A question's id changes // whenever the question does, which is exactly the kind of drift this pins: - // the values here are what the 0.12.0 survey holds as of 2026-09-21. - it("carries the live survey's question ids in question order", () => { - expect(SURVEY_ID).toBe("01a0c7b9-dfe4-0000-d05e-ce253e90a68c"); - expect(SURVEY_QUESTIONS.map(({ key, id }) => [key, id])).toEqual([ + // the values here are what each survey holds as of 2026-10-08. + it("carries the rule survey's question ids in question order", () => { + expect(RULE_SURVEY_ID).toBe("01a0c7b9-dfe4-0000-d05e-ce253e90a68c"); + expect(SURVEYS.rule.id).toBe(RULE_SURVEY_ID); + expect(SURVEYS.rule.questions.map(({ key, id }) => [key, id])).toEqual([ ["ruleKind", "0874591f-c554-4ac3-8930-e11c436d859e"], ["verbatim", "2c3c80dc-dcda-4e29-b52e-a25ef58b5ca2"], ["completed", "605e12a8-82b6-480f-93b2-ab8de0fa08bd"], @@ -31,6 +32,26 @@ describe("survey constants", () => { ]); }); + it("carries the general survey's question ids in question order", () => { + expect(SURVEYS.general.id).toBe("01a11da4-3948-0000-4ae4-c9da9321801e"); + expect(SURVEYS.general.questions.map(({ key, id }) => [key, id])).toEqual([ + ["verbatim", "c71e52ee-f4c7-469f-815a-af50a9be6d37"], + ["context", "ab0ceb25-8084-44c8-9974-1d1a1c7371c2"], + ]); + }); + + it("carries the bug survey's question ids, leaving version information to the CLI", () => { + expect(SURVEYS.bug.id).toBe("01a11da7-27a2-0000-0f4e-6d3e1f89f385"); + expect(SURVEYS.bug.questions.map(({ key, id }) => [key, id])).toEqual([ + ["summary", "2dd63cf3-dac2-4765-ace0-393bc4aa42fe"], + [undefined, "ba096b81-ae5c-45b5-b15a-707f97129839"], + ["trying", "e59a87e9-9ac7-4a59-a709-a282b16dbf37"], + ["expected", "73415b80-7371-4536-8c50-09c2ecaa5d52"], + ["actual", "daa98d8d-113e-4e3e-ac0f-595ecc1fc69f"], + ["context", "597381f5-52e9-4937-bf80-dc55739e7435"], + ]); + }); + it("holds PostHog's choices for the single-choice question, in its casing", () => { expect(COMPLETED_CHOICES).toEqual(["Yes", "No", "Unknown"]); }); @@ -64,35 +85,37 @@ describe("survey cadence store", () => { }); it("lives under the survey id in the XDG config directory", () => { - expect(nextAskPath(SURVEY_ID)).toBe( - join(configHome, "taskless", "surveys", SURVEY_ID, "next_ask") + expect(nextAskPath(RULE_SURVEY_ID)).toBe( + join(configHome, "taskless", "surveys", RULE_SURVEY_ID, "next_ask") ); }); it("reads absent as undefined", async () => { - expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(await readNextAsk(RULE_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); + await writeNextAsk(RULE_SURVEY_ID, at + 0.75); + expect(await readNextAsk(RULE_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)); + expect(await readFile(nextAskPath(RULE_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); + const path = nextAskPath(RULE_SURVEY_ID); await mkdir(dirname(path), { recursive: true }); await writeFile(path, "not a number\n", "utf8"); - expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(await readNextAsk(RULE_SURVEY_ID)).toBeUndefined(); - await writeNextAsk(SURVEY_ID, 1234); - expect(await readNextAsk(SURVEY_ID)).toBe(1234); + await writeNextAsk(RULE_SURVEY_ID, 1234); + expect(await readNextAsk(RULE_SURVEY_ID)).toBe(1234); }); it("keeps a different survey's cadence in its own file", async () => { - await writeNextAsk(SURVEY_ID, 1000); + await writeNextAsk(RULE_SURVEY_ID, 1000); expect(await readNextAsk("00000000-0000-4000-8000-000000000000")).toBe( undefined ); diff --git a/packages/cli/test/survey-invite.test.ts b/packages/cli/test/survey-invite.test.ts index 0f7303f7..71917d2a 100644 --- a/packages/cli/test/survey-invite.test.ts +++ b/packages/cli/test/survey-invite.test.ts @@ -6,7 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { getRecipe } from "../src/prompts/recipes"; import { nextAskPath, readNextAsk, writeNextAsk } from "../src/survey/cadence"; -import { SHOWN_INTERVAL_MS, SURVEY_ID } from "../src/survey/constants"; +import { SHOWN_INTERVAL_MS, RULE_SURVEY_ID } from "../src/survey/constants"; import { getTelemetry } from "../src/telemetry"; // Spy on telemetry by mocking the module the gate imports, the same way @@ -88,9 +88,9 @@ describe("the survey gate", () => { expect(served.match(/^# Topic:/gm)).toHaveLength(1); expect(capture).toHaveBeenCalledTimes(1); expect(capture).toHaveBeenCalledWith("survey shown", { - $survey_id: SURVEY_ID, + $survey_id: RULE_SURVEY_ID, }); - expect(await readNextAsk(SURVEY_ID)).toBe(NOW + SHOWN_INTERVAL_MS); + expect(await readNextAsk(RULE_SURVEY_ID)).toBe(NOW + SHOWN_INTERVAL_MS); }); it("claims the cadence window before the telemetry client is initialised", async () => { @@ -98,7 +98,7 @@ describe("the survey gate", () => { // for it, so the assertion is about ordering, not the final state. let seenAtTelemetryInit: number | undefined; vi.mocked(getTelemetry).mockImplementationOnce(async () => { - seenAtTelemetryInit = await readNextAsk(SURVEY_ID); + seenAtTelemetryInit = await readNextAsk(RULE_SURVEY_ID); return { capture, shutdown: () => Promise.resolve() }; }); @@ -117,17 +117,17 @@ describe("the survey gate", () => { }); it("serves the bare recipe within the window, touching nothing", async () => { - await writeNextAsk(SURVEY_ID, NOW + 1); + await writeNextAsk(RULE_SURVEY_ID, NOW + 1); const recipe = getRecipe("create-vale-rule", { invocation, directive: true }) ?? ""; expect(await serve("create-vale-rule")).toBe(recipe.trimEnd()); expect(capture).not.toHaveBeenCalled(); - expect(await readNextAsk(SURVEY_ID)).toBe(NOW + 1); + expect(await readNextAsk(RULE_SURVEY_ID)).toBe(NOW + 1); }); it("opens the moment the window closes", async () => { - await writeNextAsk(SURVEY_ID, NOW); + await writeNextAsk(RULE_SURVEY_ID, NOW); expect(await surveyGateIsOpen({ topic: "onboard", now: () => NOW })).toBe( true ); @@ -136,37 +136,37 @@ describe("the survey gate", () => { it("serves the bare recipe under the telemetry opt-out, without reading the cadence", async () => { enabled = false; // A cadence that says "ask now" would open the gate if it were read. - await writeNextAsk(SURVEY_ID, 0); + await writeNextAsk(RULE_SURVEY_ID, 0); const recipe = getRecipe("create-sg-rule", { invocation, directive: true }) ?? ""; expect(await serve("create-sg-rule")).toBe(recipe.trimEnd()); expect(capture).not.toHaveBeenCalled(); - expect(await readNextAsk(SURVEY_ID)).toBe(0); + expect(await readNextAsk(RULE_SURVEY_ID)).toBe(0); }); it.each(["true", "1"])("serves the bare recipe in CI (CI=%s)", async (ci) => { const recipe = getRecipe("onboard", { invocation, directive: true }) ?? ""; expect(await serve("onboard", { ci })).toBe(recipe.trimEnd()); expect(capture).not.toHaveBeenCalled(); - expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(await readNextAsk(RULE_SURVEY_ID)).toBeUndefined(); }); it("serves an unsurveyed topic bare and leaves the cadence alone", async () => { const recipe = getRecipe("check", { invocation, directive: true }) ?? ""; expect(await serve("check")).toBe(recipe.trimEnd()); expect(capture).not.toHaveBeenCalled(); - expect(await readNextAsk(SURVEY_ID)).toBeUndefined(); + expect(await readNextAsk(RULE_SURVEY_ID)).toBeUndefined(); }); it("repairs a corrupt cadence file by serving and rewriting", async () => { - const path = nextAskPath(SURVEY_ID); + const path = nextAskPath(RULE_SURVEY_ID); await mkdir(join(path, ".."), { recursive: true }); await writeFile(path, "garbage", "utf8"); const served = await serve("create-remote-rule"); expect(served.endsWith(inviteFragment())).toBe(true); - expect(await readNextAsk(SURVEY_ID)).toBe(NOW + SHOWN_INTERVAL_MS); + expect(await readNextAsk(RULE_SURVEY_ID)).toBe(NOW + SHOWN_INTERVAL_MS); }); /** Everything the command wrote to stdout, as one string. */ @@ -210,7 +210,7 @@ describe("the survey gate", () => { expect(printed()).toContain("# Topic: onboard"); expect(printed()).toContain("## Before you finish"); expect(capture).toHaveBeenCalledWith("survey shown", { - $survey_id: SURVEY_ID, + $survey_id: RULE_SURVEY_ID, }); });