diff --git a/openspec/changes/feedback-channels/tasks.md b/openspec/changes/feedback-channels/tasks.md index b5e4ee50..7b9e7bc0 100644 --- a/openspec/changes/feedback-channels/tasks.md +++ b/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 diff --git a/packages/cli/src/commands/feedback.ts b/packages/cli/src/commands/feedback.ts index f9d06627..3fbeed76 100644 --- a/packages/cli/src/commands/feedback.ts +++ b/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,14 +19,54 @@ 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 { + 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. * @@ -32,10 +74,12 @@ const NOTHING_SENT = * 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. + * 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 { const survey = SURVEYS[input.kind]; // The branches share no key type, so the payload is read as a plain record; @@ -43,8 +87,7 @@ export function buildSurveyResponse( 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]; + 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."); diff --git a/packages/cli/test/feedback-command.test.ts b/packages/cli/test/feedback-command.test.ts index 795d67bb..0c5cd197 100644 --- a/packages/cli/test/feedback-command.test.ts +++ b/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> { + 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; + } + 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({