feat(feedback): send each kind to its survey, fill a bug's version info, and point to issues when telemetry is off

openspec/changes/feedback-channels/tasks.md

@@ -6,9 +6,9 @@
 
 ## 2. `feedback send` behavior
 
-- [ ] 2.1 Add the CLI-built bug version-information answer (CLI version, `install.cliVersion`, `rules.reconciledTo`, platform/arch, Node version; no network, no identity) and attach it to `bug` sends only; verify tests for an initialised project, a directory with no `.taskless/`, and a logged-in token whose login/email/org/repository URL do not appear in the answer
-- [ ] 2.2 Advance `next_ask` only for `kind: "rule"`; verify a test that sends `general` and `bug` payloads and asserts `next_ask` is unchanged and no cadence file exists for either survey
-- [ ] 2.3 Change the telemetry-off message for `feedback send` to say telemetry is disabled and name `https://github.com/taskless/cli/issues`, keeping validation first; verify tests under `DO_NOT_TRACK=1` for a valid payload of each kind (exit 0, URL printed, nothing captured) and an invalid one (exit 1, `INVALID_INPUT`)
+- [x] 2.1 Add the CLI-built bug version-information answer (CLI version, `install.cliVersion`, `rules.reconciledTo`, platform/arch, Node version; no network, no identity) and attach it to `bug` sends only; verify tests for an initialised project, a directory with no `.taskless/`, and a logged-in token whose login/email/org/repository URL do not appear in the answer
+- [x] 2.2 Advance `next_ask` only for `kind: "rule"`; verify a test that sends `general` and `bug` payloads and asserts `next_ask` is unchanged and no cadence file exists for either survey
+- [x] 2.3 Change the telemetry-off message for `feedback send` to say telemetry is disabled and name `https://github.com/taskless/cli/issues`, keeping validation first; verify tests under `DO_NOT_TRACK=1` for a valid payload of each kind (exit 0, URL printed, nothing captured) and an invalid one (exit 1, `INVALID_INPUT`)
 
 ## 3. Recipes and the agent index
 

packages/cli/src/commands/feedback.ts

@@ -1,8 +1,10 @@
-import { resolve } from "node:path";
+import { join, resolve } from "node:path";
 import process from "node:process";
 
 import { defineCommand } from "citty";
 
+import { readManifest } from "../filesystem/manifest";
+import { TASKLESS_DIRECTORY } from "../rules/vale/formats";
 import { inputSchema, type FeedbackInput } from "../schemas/feedback";
 import { writeNextAsk } from "../survey/cadence";
 import {
@@ -17,34 +19,75 @@ import { readJsonInput } from "../util/json-input";
 import { getCliPrefix } from "../util/package-manager";
 
 /**
- * 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.
+ * What `dismiss` says under the telemetry opt-out. An agent should never reach
+ * it 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.";
 
+/** Where to report instead when telemetry is off. */
+export const ISSUES_URL = "https://github.com/taskless/cli/issues";
+
+/**
+ * What `send` says under the telemetry opt-out. Unlike the invite, general
+ * feedback and bug reports are things the user asked to send, so the opt-out
+ * gets a way forward rather than a shrug. Exit zero all the same: the opt-out
+ * is honoured, not an error.
+ */
+const SEND_DISABLED = `Telemetry is disabled, so nothing was sent. To reach the Taskless team anyway, open an issue at ${ISSUES_URL}`;
+
+/**
+ * The bug survey's version-information answer, built by the CLI so the agent
+ * can neither get it wrong nor leave it out.
+ *
+ * Local state only, and nothing that identifies the user: no login, email,
+ * organization, repository URL, or path, and no network call. That rules out
+ * reusing `info`, which probes `whoami` and reports all of those. The event
+ * carries the usual telemetry identity regardless; this answer is the text a
+ * person reads in the responses view, and it says only what build and project
+ * layout the bug was seen on. A missing or unreadable `.taskless/` drops the
+ * project lines rather than failing the report.
+ */
+export async function bugVersionInformation(cwd: string): Promise<string> {
+  const manifest = await readManifest(join(cwd, TASKLESS_DIRECTORY)).then(
+    (read) => read.manifest,
+    () => null
+  );
+  const lines = [`cli: ${__VERSION__}`];
+  const installed = manifest?.install?.cliVersion;
+  if (installed) lines.push(`installed scaffold: ${installed}`);
+  const reconciledTo = manifest?.rules?.reconciledTo;
+  if (reconciledTo) lines.push(`rules reconciled to: ${reconciledTo}`);
+  lines.push(
+    `platform: ${process.platform} ${process.arch}`,
+    `node: ${process.version}`
+  );
+  return lines.join("\n");
+}
+
 /**
  * The `survey sent` properties for a validated payload.
  *
  * Exactly PostHog's contract: `$survey_id` of the survey the payload's `kind`
  * selects, and one `$survey_response_<id>` 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.
+ * payload key is the CLI's to answer, from `cliAnswer`; the bug survey's
+ * version information is the only one.
  */
 export function buildSurveyResponse(
-  input: FeedbackInput
+  input: FeedbackInput,
+  cliAnswer?: string
 ): Record<string, string> {
   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<Record<string, string | undefined>>;
   const properties: Record<string, string> = { $survey_id: survey.id };
   for (const { key, id } of survey.questions) {
-    if (key === undefined) continue;
-    const answer = answers[key];
+    const answer = key === undefined ? cliAnswer : answers[key];
     if (answer !== undefined) properties[`$survey_response_${id}`] = answer;
   }
   return properties;
@@ -133,13 +176,20 @@ const sendCommand = defineCommand({
     }
 
     if (!isTelemetryEnabled()) {
-      console.log(NOTHING_SENT);
+      console.log(SEND_DISABLED);
       return;
     }
 
+    const cliAnswer =
+      input.kind === "bug" ? await bugVersionInformation(cwd) : undefined;
     const telemetry = await getTelemetry(cwd);
-    telemetry.capture("survey sent", buildSurveyResponse(input));
-    await writeNextAsk(RULE_SURVEY_ID, Date.now() + ANSWERED_INTERVAL_MS);
+    telemetry.capture("survey sent", buildSurveyResponse(input, cliAnswer));
+    // Only the invited survey has a cadence. General feedback and bug reports
+    // are the user's own initiative, and answering one is not an answer to
+    // the invite: it neither earns nor costs the user a quiet spell.
+    if (input.kind === "rule") {
+      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.");

packages/cli/test/feedback-command.test.ts

@@ -1,13 +1,25 @@
 import { execFile } from "node:child_process";
-import { mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises";
+import {
+  mkdir,
+  mkdtemp,
+  readdir,
+  readFile,
+  rm,
+  writeFile,
+} from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } 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, RULE_SURVEY_ID } from "../src/survey/constants";
+import { LATEST_SCHEMA_VERSION } from "../src/filesystem/migrate";
+import { readNextAsk, writeNextAsk } from "../src/survey/cadence";
+import {
+  ANSWERED_INTERVAL_MS,
+  RULE_SURVEY_ID,
+  SURVEYS,
+} from "../src/survey/constants";
 import { CLIError } from "../src/util/cli-error";
 import { builtCli } from "./support/built-cli";
 
@@ -24,7 +36,7 @@ vi.mock("../src/telemetry", () => ({
   shutdownTelemetry: () => Promise.resolve(),
 }));
 
-const { feedbackCommand, buildSurveyResponse } =
+const { feedbackCommand, buildSurveyResponse, ISSUES_URL } =
   await import("../src/commands/feedback");
 
 interface RunnableCommand {
@@ -79,6 +91,16 @@ describe("feedback command", () => {
     return path;
   }
 
+  async function send(payload: unknown): Promise<Record<string, string>> {
+    const from = await writePayload(payload);
+    await verb("send").run({
+      args: { dir: cwd, from, json: false },
+      rawArgs: [],
+    });
+    expect(capture).toHaveBeenCalledTimes(1);
+    return capture.mock.calls[0]![1] as Record<string, string>;
+  }
+
   describe("dismiss", () => {
     it("captures survey dismissed with the survey id and nothing else", async () => {
       await verb("dismiss").run({ args: { dir: cwd }, rawArgs: [] });
@@ -226,6 +248,109 @@ describe("feedback command", () => {
     });
   });
 
+  describe("send, by kind", () => {
+    const GENERAL = { kind: "general", verbatim: "The recipes are long." };
+    const BUG = {
+      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",
+    };
+    const VERSION_QUESTION = `$survey_response_${
+      SURVEYS.bug.questions.find(({ key }) => key === undefined)!.id
+    }`;
+
+    it.each([
+      ["rule", VALID, "01a0c7b9-dfe4-0000-d05e-ce253e90a68c"],
+      ["general", GENERAL, "01a11da4-3948-0000-4ae4-c9da9321801e"],
+      ["bug", BUG, "01a11da7-27a2-0000-0f4e-6d3e1f89f385"],
+    ])("sends a %s payload to its own survey", async (_kind, payload, id) => {
+      const properties = await send(payload);
+      expect(properties.$survey_id).toBe(id);
+    });
+
+    it.each([
+      ["general", GENERAL],
+      ["bug", BUG],
+    ])("leaves the cadence alone for a %s payload", async (kind, payload) => {
+      await writeNextAsk(RULE_SURVEY_ID, 1234);
+      await send(payload);
+      expect(await readNextAsk(RULE_SURVEY_ID)).toBe(1234);
+      expect(
+        await readNextAsk(SURVEYS[kind as "general" | "bug"].id)
+      ).toBeUndefined();
+    });
+
+    it("answers a bug report's version information itself", async () => {
+      await mkdir(join(cwd, ".taskless"), { recursive: true });
+      await writeFile(
+        join(cwd, ".taskless", "taskless.json"),
+        JSON.stringify({
+          version: LATEST_SCHEMA_VERSION,
+          install: { cliVersion: "0.11.3" },
+          rules: { reconciledTo: "0.11.0" },
+        }),
+        "utf8"
+      );
+      const properties = await send(BUG);
+      const answer = properties[VERSION_QUESTION]!;
+      expect(answer).toMatch(/^cli: \S+/);
+      expect(answer).toContain("installed scaffold: 0.11.3");
+      expect(answer).toContain("rules reconciled to: 0.11.0");
+      expect(answer).toContain(`platform: ${process.platform} ${process.arch}`);
+      expect(answer).toContain(`node: ${process.version}`);
+    });
+
+    it("still answers it with no .taskless/, and says nothing identifying", async () => {
+      const properties = await send(BUG);
+      const answer = properties[VERSION_QUESTION]!;
+      // Every line is one of the four local facts and nothing else: no
+      // login, email, organization, repository URL, or path.
+      expect(
+        answer.split("\n").map((line) => line.slice(0, line.indexOf(":")))
+      ).toEqual(["cli", "platform", "node"]);
+      expect(answer).not.toContain(cwd);
+    });
+
+    it("gives no other kind a version-information answer", async () => {
+      const properties = await send(GENERAL);
+      expect(Object.keys(properties)).not.toContain(VERSION_QUESTION);
+    });
+
+    it.each([
+      ["rule", VALID],
+      ["general", GENERAL],
+      ["bug", BUG],
+    ])(
+      "under the opt-out sends no %s payload and names the issues page",
+      async (_kind, payload) => {
+        enabled = false;
+        const from = await writePayload(payload);
+        await verb("send").run({
+          args: { dir: cwd, from, json: false },
+          rawArgs: [],
+        });
+        expect(capture).not.toHaveBeenCalled();
+        expect(process.exitCode).toBeUndefined();
+        const printed = logSpy.mock.calls.flat().join("\n");
+        expect(printed).toMatch(/disabled/);
+        expect(printed).toContain(ISSUES_URL);
+        expect(ISSUES_URL).toBe("https://github.com/taskless/cli/issues");
+      }
+    );
+
+    it("rejects a payload with no kind, naming kind", async () => {
+      const { kind: _kind, ...rest } = VALID;
+      const from = await writePayload(rest);
+      await expect(
+        verb("send").run({ args: { dir: cwd, from, json: false }, rawArgs: [] })
+      ).rejects.toBeInstanceOf(CLIError);
+      expect(errorSpy.mock.calls.flat().join("\n")).toContain("kind");
+      expect(capture).not.toHaveBeenCalled();
+    });
+  });
+
   describe("buildSurveyResponse", () => {
     it("maps every answered key and no unanswered one", () => {
       const properties = buildSurveyResponse({