diff --git a/.changeset/streamlined-onboarding-report.md b/.changeset/streamlined-onboarding-report.md index c959c40c..9ac28174 100644 --- a/.changeset/streamlined-onboarding-report.md +++ b/.changeset/streamlined-onboarding-report.md @@ -4,6 +4,6 @@ `taskless detect --json` now reports ast-grep and Vale configurations as linters (Taskless's own configuration under `.taskless/` excluded), the command-line tools on the host's `PATH` (`gh`, `git`, `jq`, and now `glab` for GitLab repositories) under `hostTools`, and an `agent` section of commands an agent may choose to run to check whether a source is usable, such as `gh auth status`. Detection still runs nothing and stays offline. `taskless info` lists `glab` among its tools too. -New `taskless report` publishes an unlisted Taskless report to tskl.es: a page showing the rules in this repository, what they find in the code today, and how often review raised each one's pattern. The CLI counts what the rules find itself, by running the same scan as `check --anonymous`, and an agent cannot supply or change those numbers. When the CLI could not run every rule (a rule written into another linter, a runtime rule, or a scan that failed), it publishes no counts rather than a partial total. With no flags it prints a draft of what the CLI knows, including those counts, and the facts an agent still has to supply; `--from ` checks there are at least three working Taskless rules, withholds any fixture that looks like it holds a secret or contact detail, validates against the tskl.es schema, publishes, and records the report in `.taskless/reports/` (gitignored, since each record holds the URL that deletes it); `--delete ` takes a report down. The new `report` agent topic walks an agent through it. Migration 11 creates `.taskless/reports/`. +New `taskless report` publishes an unlisted Taskless report to tskl.es: a page showing the rules in this repository, what they find in the code today, and how often review raised each one's pattern. The CLI counts what the rules find itself, by running the same scan as `check --anonymous`, and an agent cannot supply or change those numbers. When the CLI could not run every rule (a rule written into another linter, a runtime rule, or a scan that failed), it publishes no counts rather than a partial total. With no flags it prints a draft of what the CLI knows, including those counts, and the facts an agent still has to supply; `--from ` checks there are at least three working Taskless rules, withholds any fixture that looks like it holds a secret or contact detail, validates against the tskl.es schema, publishes, and records the report in `.taskless/reports/` (gitignored, since each record holds the URL that deletes it); `--delete ` takes a report down. Every rule on the page carries the agent's one- or two-sentence explanation of why it is worth holding in this codebase, screened like the titles. The new `report` agent topic walks an agent through it. Migration 11 creates `.taskless/reports/`. Onboarding now asks one question, "Do you want me to set up Taskless for you?", and does the rest: it mines every source it can reach, writes every rule the repository can enforce locally (always logged out), marks onboarding complete, publishes a report, sends Taskless onboarding feedback, and ends by strongly encouraging running the rules in CI. It asks anything else only when it cannot find enough rules for a report. GitLab merge requests are mined through `glab` the way GitHub pull requests are through `gh`. diff --git a/openspec/changes/archive/2026-10-16-report-rule-explain/proposal.md b/openspec/changes/archive/2026-10-16-report-rule-explain/proposal.md new file mode 100644 index 00000000..a65a2821 --- /dev/null +++ b/openspec/changes/archive/2026-10-16-report-rule-explain/proposal.md @@ -0,0 +1,26 @@ +## Why + +A published report lists each rule by its title and numbers, which says what a rule enforces but not why this repository should care. ReportV1 on tskl.es now carries an optional per-rule `explain`: plain prose on how the rule improves the codebase, scrubbed by the CLI like `title`. The agent that wrote or proposed each rule is the one that knows why, so the CLI should ask it every time. + +## What Changes + +- **Vendored ReportV1 regenerated** from `https://tskl.es/api/v1/schema`. It adds `explain` on rules (1 to 1000 characters, at least one non-space) and a non-space `pattern` on `title`. +- **Every rule in the facts requires `explain`.** Local `rules`, `legacy`, and `cloud` entries each take one; a missing, blank, or over-length one is refused under `REPORT_INVALID`. ReportV1 keeps it optional; the CLI is stricter so no report it publishes has a rule without one. `title` also refuses a blank value now, matching the schema. +- **The CLI carries `explain` onto every published rule** and screens it with the other agent-supplied strings, so a match refuses the publish under `REPORT_REDACTION` naming the field. +- **Recipe.** The report recipe (topic v6) asks for an `explain` on every rule and gains `## Explaining a rule`: one or two sentences, what goes wrong without the rule and what holding it buys, grounded in the session, no counts, no names or hosts. + +## Capabilities + +### Modified Capabilities + +- `cli-report`: the facts carry a required `explain` per rule, the redaction screen covers it, and the recipe asks for it. + +## Delivery Shape + +**Single PR.** The change is one reviewable diff. `report` is in no release (0.12.0 predates it), so the bump is `patch`, and the release note extends the existing report changeset rather than adding one. + +## Impact + +- **Code**: `packages/cli/src/schemas/report-facts.ts`, `packages/cli/src/report/{types,assemble}.ts`, `packages/cli/src/generated/report-v1.schema.json`. +- **Recipe**: `packages/cli/src/agent/report.md`. +- **Tests**: `packages/cli/test/report.test.ts`, `report-telemetry.test.ts`. diff --git a/openspec/changes/archive/2026-10-16-report-rule-explain/specs/cli-report/spec.md b/openspec/changes/archive/2026-10-16-report-rule-explain/specs/cli-report/spec.md new file mode 100644 index 00000000..932f3b60 --- /dev/null +++ b/openspec/changes/archive/2026-10-16-report-rule-explain/specs/cli-report/spec.md @@ -0,0 +1,125 @@ +## MODIFIED Requirements + +### Requirement: The agent supplies session facts, not a report + +`taskless report --from ` SHALL read a facts file, not a ReportV1. The facts file SHALL carry only what the agent knows from its session: the history counts and the window they were mined over, per-rule titles, an `explain` for every rule (the agent's take, in one or two sentences, on why the rule is worth holding in this codebase), and `surfaced` counts (the merged pull requests in the window whose review raised the rule's pattern, and the review comments that asked for it), rules authored into an existing linter (`legacy` tier), runtime-shaped candidates the agent did not author (`cloud` tier) each with `estimatedViolations`, the agent's estimate of how many instances of the candidate's pattern are in the code today, the agent's own redaction counts, and the fixtures it elects to withhold. The facts file SHALL NOT accept any value input: no token, call, price, time, or rate estimate. The CLI SHALL merge the facts with what it establishes itself, and SHALL treat its own values as authoritative for every field it can establish. + +Every published rule SHALL carry a `state`. Rules under `.taskless/rules/` and `legacy` rules SHALL be `written`; `cloud` candidates from the facts file SHALL be `candidate`. + +The facts file SHALL be validated against a published facts schema before anything else happens. The `report` recipe SHALL embed that schema. + +#### Scenario: Facts cannot set what the CLI knows + +- **WHEN** the facts file gives a rule under `.taskless/rules/` an engine, a language, a test status, or a state +- **THEN** the command SHALL refuse the field as unknown under `REPORT_INVALID` +- **AND** a report that publishes SHALL carry the engine, language, test status, and state the CLI established + +#### Scenario: Malformed facts are refused before any other work + +- **WHEN** the facts file is not valid JSON or fails the facts schema +- **THEN** the command SHALL exit non-zero with `REPORT_INVALID` naming each failing field +- **AND** SHALL NOT perform any network call + +#### Scenario: Cloud candidates are carried as cloud tier + +- **WHEN** the facts file lists a runtime-shaped candidate +- **THEN** the published report SHALL include it with `tier: "cloud"`, `engine: "runtime"`, and `state: "candidate"` +- **AND** its tests status SHALL be `not-run` +- **AND** it SHALL carry the agent's `estimatedViolations` + +#### Scenario: A candidate without an estimate is refused + +- **WHEN** the facts file lists a `cloud` candidate with no `estimatedViolations` +- **THEN** the command SHALL exit non-zero with `REPORT_INVALID` + +#### Scenario: Every rule carries the agent's explanation + +- **WHEN** the facts give a local rule, a `legacy` rule, and a `cloud` candidate each an `explain` +- **THEN** the published report SHALL carry each one's `explain` on that rule + +#### Scenario: A rule without an explanation is refused + +- **WHEN** the facts give any rule, in `rules`, `legacy`, or `cloud`, no `explain`, or one that is blank or longer than 1000 characters +- **THEN** the command SHALL exit non-zero with `REPORT_INVALID` + +#### Scenario: Value inputs are not accepted + +- **WHEN** the facts file includes `estimates`, `contextTokens`, `callsPerCatch`, or `complexity` +- **THEN** the command SHALL refuse the field as unknown under `REPORT_INVALID` + +#### Scenario: Fix counts are not accepted + +- **WHEN** the facts file gives a rule `fixed`, `fixedBasis`, or `origin` +- **THEN** the command SHALL refuse the field as unknown under `REPORT_INVALID` + +### Requirement: The CLI redacts and withholds before publishing + +tskl.es performs no redaction of its own. Before publishing, the CLI SHALL scan every fixture and every string supplied by the facts file for secrets, email addresses, host names, IP addresses, and phone numbers. A fixture containing a match SHALL be withheld in full, never partially masked: its code SHALL be omitted, its side SHALL be recorded in that rule's `withheld`, and `redaction.withheldFixtures` SHALL count it. A match in any other agent-supplied string SHALL refuse the publish with `REPORT_REDACTION`, naming the field, so the agent can rewrite it. A fixture whose code exceeds the schema's length limit SHALL be withheld the same way. + +`redaction.counts` SHALL be the sum of the agent's declared counts and the CLI's own matches per category. `redaction.policyVersion` SHALL identify the CLI's redaction policy. + +#### Scenario: A secret in a fixture withholds the fixture + +- **WHEN** a rule's failing fixture contains an AWS access key +- **THEN** the published rule SHALL have `tests.fail: null` and `"fail"` in `tests.withheld` +- **AND** `redaction.counts.secret` SHALL include that match +- **AND** no part of the fixture SHALL appear in the published payload + +#### Scenario: An email in a rule title refuses the publish + +- **WHEN** the facts give a rule a title containing an email address +- **THEN** the command SHALL exit non-zero with `REPORT_REDACTION` naming that rule's title +- **AND** SHALL perform no network call + +#### Scenario: Every agent-supplied rule string is screened + +- **WHEN** the facts give any rule an `explain`, a legacy rule a `tool` or `language`, or a cloud candidate a `language` or `id`, containing an email address or a host name +- **THEN** the command SHALL exit non-zero with `REPORT_REDACTION` naming that field + +#### Scenario: The agent's counts are carried + +- **WHEN** the facts declare `person: 2` and the CLI matches no person names +- **THEN** `redaction.counts.person` SHALL be 2 + +### Requirement: The report recipe drives publishing and deletion + +The CLI SHALL embed a `report` recipe following the canonical recipe template. It SHALL instruct the agent to: run `taskless report` and read `missing`; supply each missing fact from its session, writing the facts to `.taskless/.tmp-report.json`; give every rule an `explain` of one or two sentences saying what goes wrong without the rule and what holding it buys, grounded in what the session saw and carrying no counts; count each rule's `surfaced` by the method the recipe states; give each cloud candidate an `estimatedViolations` from what it saw while mining; run `taskless report --from` that path; on `REPORT_INSUFFICIENT`, ask the user only what would let it author the rules or establish the facts the refusal names, then retry; on success, show the CLI's summary as printed and tell the user the report can be deleted after review if they ask; delete the facts file; and on a request to delete, run `taskless report --delete` with the record. The recipe SHALL embed the facts schema and SHALL state that tskl.es does not redact, so anything the agent cannot vouch for is to be withheld. + +The counting method SHALL be a single procedure: a pull request counts toward a rule's `surfaced.prs` when it merged inside the window and at least one review comment on it asked for the change the rule enforces; a comment counts toward `surfaced.reviewComments` when a reader of that comment would say it asks for that change. Only review comments count: the recipe SHALL state that a fix-up commit, a `git blame` date, or a finding in the code today is not a review comment and SHALL NOT be counted toward either field, since the facts have no field that would label it honestly. Every author counts: a comment from a bot account or from the pull request's own author SHALL count the same as a reviewer's, since each one spends cycles, tokens, time, or money. The recipe SHALL direct the agent to judge each comment by reading it, not by matching keywords. + +Whenever the agent restates the report's numbers in its own words, the recipe SHALL require it to keep counts the CLI measured in the code today, counts mined from review history over the window, and its own estimates for cloud candidates apart, and to say which is which. + +The recipe SHALL NOT ask the agent for any value input: tskl.es estimates reviewer time, agent tokens, and dollars itself. + +#### Scenario: The recipe asks questions only on refusal + +- **WHEN** the `report` recipe's steps are read +- **THEN** the only step that permits a question to the user SHALL be the one handling `REPORT_INSUFFICIENT` + +#### Scenario: The recipe embeds the facts schema + +- **WHEN** an agent runs `taskless agent report` +- **THEN** stdout SHALL open with `# Topic: report` and contain the JSON Schema for the facts file + +#### Scenario: The recipe states one counting method + +- **WHEN** an agent runs `taskless agent report` +- **THEN** stdout SHALL state how to count `surfaced.prs` and `surfaced.reviewComments`, including that bot and author comments count +- **AND** SHALL NOT ask for a count of fixes + +#### Scenario: Only review comments count as surfaced + +- **WHEN** an agent runs `taskless agent report` +- **THEN** the `## Counting surfaced` section SHALL state that a fix-up commit, a `git blame` date, and a finding in the code today are not review comments and do not count + +#### Scenario: The recipe asks for an explanation of every rule + +- **WHEN** an agent runs `taskless agent report` +- **THEN** stdout SHALL contain an `## Explaining a rule` section asking for one or two sentences on why each rule, written or candidate, is worth holding in this codebase +- **AND** that section SHALL tell the agent to leave counts out of `explain` + +#### Scenario: The recipe asks for a candidate estimate and no value input + +- **WHEN** an agent runs `taskless agent report` +- **THEN** the step that supplies facts SHALL direct the agent to give each cloud candidate an `estimatedViolations` +- **AND** SHALL NOT mention `contextTokens` or `callsPerCatch` diff --git a/openspec/changes/archive/2026-10-16-report-rule-explain/tasks.md b/openspec/changes/archive/2026-10-16-report-rule-explain/tasks.md new file mode 100644 index 00000000..6048a79c --- /dev/null +++ b/openspec/changes/archive/2026-10-16-report-rule-explain/tasks.md @@ -0,0 +1,14 @@ +## 1. Schema and assembly + +- [x] 1.1 Regenerate `src/generated/report-v1.schema.json` from tskl.es +- [x] 1.2 Require `explain` on every facts rule entry (local, legacy, cloud) and refuse a blank `title` +- [x] 1.3 Carry `explain` onto every published rule and add it to `ReportRule` +- [x] 1.4 Screen each rule's `explain` for redaction matches, naming the field + +## 2. Recipe + +- [x] 2.1 Ask for `explain` in the report recipe's facts step and add `## Explaining a rule`; bump the topic to v6 + +## 3. Tests + +- [x] 3.1 Add `explain` to facts fixtures; test it is carried on every tier, required, bounded, and screened diff --git a/openspec/specs/cli-report/spec.md b/openspec/specs/cli-report/spec.md index 557aea70..4c605db1 100644 --- a/openspec/specs/cli-report/spec.md +++ b/openspec/specs/cli-report/spec.md @@ -51,7 +51,7 @@ The draft SHALL NOT perform any network call. ### Requirement: The agent supplies session facts, not a report -`taskless report --from ` SHALL read a facts file, not a ReportV1. The facts file SHALL carry only what the agent knows from its session: the history counts and the window they were mined over, per-rule titles and `surfaced` counts (the merged pull requests in the window whose review raised the rule's pattern, and the review comments that asked for it), rules authored into an existing linter (`legacy` tier), runtime-shaped candidates the agent did not author (`cloud` tier) each with `estimatedViolations`, the agent's estimate of how many instances of the candidate's pattern are in the code today, the agent's own redaction counts, and the fixtures it elects to withhold. The facts file SHALL NOT accept any value input: no token, call, price, time, or rate estimate. The CLI SHALL merge the facts with what it establishes itself, and SHALL treat its own values as authoritative for every field it can establish. +`taskless report --from ` SHALL read a facts file, not a ReportV1. The facts file SHALL carry only what the agent knows from its session: the history counts and the window they were mined over, per-rule titles, an `explain` for every rule (the agent's take, in one or two sentences, on why the rule is worth holding in this codebase), and `surfaced` counts (the merged pull requests in the window whose review raised the rule's pattern, and the review comments that asked for it), rules authored into an existing linter (`legacy` tier), runtime-shaped candidates the agent did not author (`cloud` tier) each with `estimatedViolations`, the agent's estimate of how many instances of the candidate's pattern are in the code today, the agent's own redaction counts, and the fixtures it elects to withhold. The facts file SHALL NOT accept any value input: no token, call, price, time, or rate estimate. The CLI SHALL merge the facts with what it establishes itself, and SHALL treat its own values as authoritative for every field it can establish. Every published rule SHALL carry a `state`. Rules under `.taskless/rules/` and `legacy` rules SHALL be `written`; `cloud` candidates from the facts file SHALL be `candidate`. @@ -81,6 +81,16 @@ The facts file SHALL be validated against a published facts schema before anythi - **WHEN** the facts file lists a `cloud` candidate with no `estimatedViolations` - **THEN** the command SHALL exit non-zero with `REPORT_INVALID` +#### Scenario: Every rule carries the agent's explanation + +- **WHEN** the facts give a local rule, a `legacy` rule, and a `cloud` candidate each an `explain` +- **THEN** the published report SHALL carry each one's `explain` on that rule + +#### Scenario: A rule without an explanation is refused + +- **WHEN** the facts give any rule, in `rules`, `legacy`, or `cloud`, no `explain`, or one that is blank or longer than 1000 characters +- **THEN** the command SHALL exit non-zero with `REPORT_INVALID` + #### Scenario: Value inputs are not accepted - **WHEN** the facts file includes `estimates`, `contextTokens`, `callsPerCatch`, or `complexity` @@ -112,7 +122,7 @@ tskl.es performs no redaction of its own. Before publishing, the CLI SHALL scan #### Scenario: Every agent-supplied rule string is screened -- **WHEN** the facts give a legacy rule a `tool` or `language`, or a cloud candidate a `language` or `id`, containing an email address or a host name +- **WHEN** the facts give any rule an `explain`, a legacy rule a `tool` or `language`, or a cloud candidate a `language` or `id`, containing an email address or a host name - **THEN** the command SHALL exit non-zero with `REPORT_REDACTION` naming that field #### Scenario: The agent's counts are carried @@ -307,7 +317,7 @@ The counts SHALL be all-or-nothing. The CLI SHALL record them only when every wr ### Requirement: The report recipe drives publishing and deletion -The CLI SHALL embed a `report` recipe following the canonical recipe template. It SHALL instruct the agent to: run `taskless report` and read `missing`; supply each missing fact from its session, writing the facts to `.taskless/.tmp-report.json`; count each rule's `surfaced` by the method the recipe states; give each cloud candidate an `estimatedViolations` from what it saw while mining; run `taskless report --from` that path; on `REPORT_INSUFFICIENT`, ask the user only what would let it author the rules or establish the facts the refusal names, then retry; on success, show the CLI's summary as printed and tell the user the report can be deleted after review if they ask; delete the facts file; and on a request to delete, run `taskless report --delete` with the record. The recipe SHALL embed the facts schema and SHALL state that tskl.es does not redact, so anything the agent cannot vouch for is to be withheld. +The CLI SHALL embed a `report` recipe following the canonical recipe template. It SHALL instruct the agent to: run `taskless report` and read `missing`; supply each missing fact from its session, writing the facts to `.taskless/.tmp-report.json`; give every rule an `explain` of one or two sentences saying what goes wrong without the rule and what holding it buys, grounded in what the session saw and carrying no counts; count each rule's `surfaced` by the method the recipe states; give each cloud candidate an `estimatedViolations` from what it saw while mining; run `taskless report --from` that path; on `REPORT_INSUFFICIENT`, ask the user only what would let it author the rules or establish the facts the refusal names, then retry; on success, show the CLI's summary as printed and tell the user the report can be deleted after review if they ask; delete the facts file; and on a request to delete, run `taskless report --delete` with the record. The recipe SHALL embed the facts schema and SHALL state that tskl.es does not redact, so anything the agent cannot vouch for is to be withheld. The counting method SHALL be a single procedure: a pull request counts toward a rule's `surfaced.prs` when it merged inside the window and at least one review comment on it asked for the change the rule enforces; a comment counts toward `surfaced.reviewComments` when a reader of that comment would say it asks for that change. Only review comments count: the recipe SHALL state that a fix-up commit, a `git blame` date, or a finding in the code today is not a review comment and SHALL NOT be counted toward either field, since the facts have no field that would label it honestly. Every author counts: a comment from a bot account or from the pull request's own author SHALL count the same as a reviewer's, since each one spends cycles, tokens, time, or money. The recipe SHALL direct the agent to judge each comment by reading it, not by matching keywords. @@ -336,6 +346,12 @@ The recipe SHALL NOT ask the agent for any value input: tskl.es estimates review - **WHEN** an agent runs `taskless agent report` - **THEN** the `## Counting surfaced` section SHALL state that a fix-up commit, a `git blame` date, and a finding in the code today are not review comments and do not count +#### Scenario: The recipe asks for an explanation of every rule + +- **WHEN** an agent runs `taskless agent report` +- **THEN** stdout SHALL contain an `## Explaining a rule` section asking for one or two sentences on why each rule, written or candidate, is worth holding in this codebase +- **AND** that section SHALL tell the agent to leave counts out of `explain` + #### Scenario: The recipe asks for a candidate estimate and no value input - **WHEN** an agent runs `taskless agent report` diff --git a/packages/cli/src/agent/report.md b/packages/cli/src/agent/report.md index 021efde9..50e299cc 100644 --- a/packages/cli/src/agent/report.md +++ b/packages/cli/src/agent/report.md @@ -1,4 +1,4 @@ -# Topic: report (CLI v%(CLI_VERSION)s / topic v5) +# Topic: report (CLI v%(CLI_VERSION)s / topic v6) ## Goal Publish a Taskless report: an unlisted page on tskl.es that shows what @@ -49,15 +49,17 @@ later and deletes one on request. A zero is honest only for a source you read and found empty, not for one you did not read. - `rules`: one entry per rule the draft listed, with a title a - reader understands and its `surfaced` counts, counted as described - under `## Counting surfaced`. + reader understands, an `explain` written as described under + `## Explaining a rule`, and its `surfaced` counts, counted as + described under `## Counting surfaced`. - `legacy`: rules you wrote into the repository's own linter, which - the CLI cannot see. + the CLI cannot see. Each one needs an `explain` too. - `cloud`: runtime-shaped candidates you judged better solved by a runtime rule and did not author. Count their `surfaced` the same - way. Give each one `estimatedViolations`: how many instances of - its pattern you estimate are in the code today, made as described - under `## Estimating violations`. No rule has run, so this is an + way, and give each one an `explain`. Give each one + `estimatedViolations`: how many instances of its pattern you + estimate are in the code today, made as described under + `## Estimating violations`. No rule has run, so this is an estimate, and it is published as one. - `redaction`: what you removed yourself, by kind, and any fixture you choose not to publish. Names of people and customers are @@ -111,6 +113,25 @@ later and deletes one on request. confirms the deletion. Tell the user cached copies of the page may be served for up to a day afterwards. +## Explaining a rule + +Every rule in the facts, written or candidate, carries an `explain`: +your take on why the rule is worth holding in **this** codebase, in +one or two sentences. It is published beside the title, so a reader +who never saw the session understands what the rule protects. + +- Say what goes wrong without the rule, and what holding it buys. + "Direct fetch calls skip the retry and auth refresh the API client + adds, so each one fails differently when a session expires." says + both. +- Ground it in what you saw: the review comments, the code, the bug + the pattern caused. Do not restate the title. +- No counts. The report already carries `violations`, `surfaced` and + `estimatedViolations`, labelled apart; a number in prose is a third + copy that can contradict them. +- No names, hosts, or contact details. The CLI screens `explain` like + `title` and refuses the publish if its patterns match. + ## Counting surfaced Every agent counts `surfaced` the same way, so reports from different @@ -175,7 +196,7 @@ The `--from` JSON file conforms to: |--------------------------|-----------------------------------------------------------------|---------------------------------------------------------------------------| | `REPORT_INSUFFICIENT` | fewer than three working Taskless rules | write the missing rules (step 3), then retry | | `REPORT_INVALID` | the facts file, or the assembled report, failed validation | the message names each field; fix the facts file and retry | -| `REPORT_REDACTION` | a title or label you wrote looks like it holds a secret or contact detail | rewrite the field the message names without it | +| `REPORT_REDACTION` | a title, explanation, or label you wrote looks like it holds a secret or contact detail | rewrite the field the message names without it | | `REPORT_TOO_LARGE` | the report is over tskl.es's size limit | withhold long fixtures or drop cloud candidates, then retry | | `REPORT_RATE_LIMITED` | tskl.es is limiting publishes from this address | wait a minute, then retry | | `REPORT_PUBLISH_FAILED` | tskl.es could not store the report; nothing was published | retry later | diff --git a/packages/cli/src/generated/report-v1.schema.json b/packages/cli/src/generated/report-v1.schema.json index fe074108..d3cca1fc 100644 --- a/packages/cli/src/generated/report-v1.schema.json +++ b/packages/cli/src/generated/report-v1.schema.json @@ -218,8 +218,16 @@ "type": "string", "minLength": 1, "maxLength": 200, + "pattern": "\\S", "description": "What the rule enforces, in one line, scrubbed by the CLI." }, + "explain": { + "type": "string", + "minLength": 1, + "maxLength": 1000, + "pattern": "\\S", + "description": "Optional plain prose from the agent on how the rule improves this codebase: what goes wrong without it, and what holding it buys. Scrubbed by the CLI, like `title`." + }, "tier": { "type": "string", "enum": ["cli", "cloud", "legacy"], @@ -428,7 +436,7 @@ "text" ], "additionalProperties": false, - "description": "Values the CLI scrubbed from rule titles and examples, by kind." + "description": "Values the CLI scrubbed from rule titles, explanations and examples, by kind." }, "withheldFixtures": { "type": "integer", @@ -490,6 +498,7 @@ { "id": "use-api-client", "title": "Call the API through the shared apiClient", + "explain": "Direct fetch calls skip the retry, auth refresh and error mapping the client adds, so each one fails differently when the session expires.", "tier": "cli", "engine": "sg", "language": "TypeScript", diff --git a/packages/cli/src/report/assemble.ts b/packages/cli/src/report/assemble.ts index 8a7c21a9..5d6cf07d 100644 --- a/packages/cli/src/report/assemble.ts +++ b/packages/cli/src/report/assemble.ts @@ -97,7 +97,7 @@ function mergeRules( .filter((id) => !factsById.has(id)); if (uncovered.length > 0) { throw invalid( - `facts are missing for these rules: ${uncovered.join(", ")}. Every rule \`taskless report\` lists needs a "rules" entry with its title and surfaced counts.` + `facts are missing for these rules: ${uncovered.join(", ")}. Every rule \`taskless report\` lists needs a "rules" entry with its title, explain, and surfaced counts.` ); } @@ -108,6 +108,7 @@ function mergeRules( rule: { id: local.id, title: ruleFacts.title, + explain: ruleFacts.explain, tier: local.tier, engine: local.engine, language: local.language, @@ -128,6 +129,7 @@ function mergeRules( rule: { id: legacy.id, title: legacy.title, + explain: legacy.explain, tier: "legacy", tool: legacy.tool, language: legacy.language, @@ -147,6 +149,7 @@ function mergeRules( rule: { id: cloud.id, title: cloud.title, + explain: cloud.explain, tier: "cloud", engine: "runtime", language: cloud.language, @@ -204,7 +207,8 @@ function screenStrings(facts: ReportFacts, label: string | undefined): void { for (const rule of facts[group]) { fields.push( [`${group}[${rule.id}].id`, rule.id], - [`${group}[${rule.id}].title`, rule.title] + [`${group}[${rule.id}].title`, rule.title], + [`${group}[${rule.id}].explain`, rule.explain] ); } } diff --git a/packages/cli/src/report/types.ts b/packages/cli/src/report/types.ts index 1dfbbc93..0942183b 100644 --- a/packages/cli/src/report/types.ts +++ b/packages/cli/src/report/types.ts @@ -37,6 +37,7 @@ export interface Findings { export interface ReportRule { id: string; title: string; + explain?: string; tier: Tier; engine?: "sg" | "vale" | "runtime"; tool?: string; diff --git a/packages/cli/src/schemas/report-facts.ts b/packages/cli/src/schemas/report-facts.ts index 5e3c9a86..11fc19e9 100644 --- a/packages/cli/src/schemas/report-facts.ts +++ b/packages/cli/src/schemas/report-facts.ts @@ -43,9 +43,20 @@ const outcome = { .string() .min(1) .max(200) + .regex(/\S/) .describe( "What the rule enforces, in a phrase a reader of the report understands" ), + // Optional in ReportV1, required here: a report whose rules do not say why + // they matter is a list of titles. + explain: z + .string() + .min(1) + .max(1000) + .regex(/\S/) + .describe( + "Why this rule is worth holding in this codebase, in one or two sentences: what goes wrong without it, and what holding it buys" + ), surfaced, }; diff --git a/packages/cli/test/report-telemetry.test.ts b/packages/cli/test/report-telemetry.test.ts index 4cf25f33..1d290a71 100644 --- a/packages/cli/test/report-telemetry.test.ts +++ b/packages/cli/test/report-telemetry.test.ts @@ -108,6 +108,7 @@ describe("report telemetry", () => { rules: ["alpha", "beta", "gamma"].map((id) => ({ id, title: TITLE, + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 1, reviewComments: 1 }, })), }), diff --git a/packages/cli/test/report.test.ts b/packages/cli/test/report.test.ts index c2d508b6..4837a750 100644 --- a/packages/cli/test/report.test.ts +++ b/packages/cli/test/report.test.ts @@ -76,6 +76,7 @@ function factsFor( rules: ids.map((id) => ({ id, title: `Rule ${id}`, + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 6, reviewComments: 9 }, })), ...overrides, @@ -168,6 +169,72 @@ describe("assembleReport", () => { }); }); + it("carries every rule's explanation, on every tier", () => { + const facts = factsFor(["a"], { + legacy: ["l1", "l2"].map((id) => ({ + id, + title: `Legacy ${id}`, + explain: `Why ${id} matters.`, + surfaced: { prs: 1, reviewComments: 1 }, + tool: "eslint", + language: "TypeScript", + })), + cloud: [ + { + id: "c1", + title: "Cloud c1", + explain: "Why c1 matters.", + surfaced: { prs: 1, reviewComments: 1 }, + language: "TypeScript", + estimatedViolations: 2, + }, + ], + }); + const report = assembleReport(draftOf([localRule("a")]), facts); + expect(report.rules.map((rule) => [rule.id, rule.explain])).toEqual([ + ["a", "Holding it keeps review on design instead of style."], + ["l1", "Why l1 matters."], + ["l2", "Why l2 matters."], + ["c1", "Why c1 matters."], + ]); + expect(reportSchemaIssues(report)).toEqual([]); + }); + + it.each([ + ["missing", undefined], + ["empty", ""], + ["blank", " "], + ["over 1000 characters", "x".repeat(1001)], + ])("refuses a rule whose explanation is %s", (_case, explain) => { + const facts = { + ...factsFor(three), + rules: three.map((id) => ({ + id, + title: `Rule ${id}`, + ...(explain === undefined ? {} : { explain }), + surfaced: { prs: 1, reviewComments: 1 }, + })), + }; + expect(reportFactsSchema.safeParse(facts).success).toBe(false); + }); + + it.each([ + ["empty", ""], + ["blank", " "], + ["over 200 characters", "x".repeat(201)], + ])("refuses a rule whose title is %s", (_case, title) => { + const facts = { + ...factsFor(three), + rules: three.map((id) => ({ + id, + title, + explain: "Holding it keeps review on design instead of style.", + surfaced: { prs: 1, reviewComments: 1 }, + })), + }; + expect(reportFactsSchema.safeParse(facts).success).toBe(false); + }); + it("carries the draft's measured findings on every written rule", () => { const report = assembleReport( draftOf( @@ -217,6 +284,7 @@ describe("assembleReport", () => { { id: "eslint-rule", title: "No default exports", + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 1, reviewComments: 1 }, tool: "eslint", language: "TypeScript", @@ -245,6 +313,7 @@ describe("assembleReport", () => { rules: three.map((id) => ({ id, title: `Rule ${id}`, + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 1, reviewComments: 1 }, ...extra, })), @@ -265,6 +334,7 @@ describe("assembleReport", () => { cloud: ["x", "y", "z", "w"].map((id) => ({ id, title: `Cloud ${id}`, + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 1, reviewComments: 1 }, language: "TypeScript", estimatedViolations: 5, @@ -285,6 +355,7 @@ describe("assembleReport", () => { legacy: ["l1", "l2"].map((id) => ({ id, title: `Legacy ${id}`, + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 1, reviewComments: 1 }, tool: "eslint", language: "TypeScript", @@ -314,6 +385,7 @@ describe("assembleReport", () => { const candidate = { id: "tenant-check", title: "Routes call requireTenant()", + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 0, reviewComments: 0 }, language: "TypeScript", }; @@ -335,6 +407,7 @@ describe("assembleReport", () => { { id: "tenant-check", title: "Routes call requireTenant()", + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 3, reviewComments: 4 }, language: "TypeScript", estimatedViolations: 12, @@ -414,6 +487,7 @@ describe("assembleReport", () => { const legacy = { id: "l1", title: "Legacy l1", + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 1, reviewComments: 1 }, tool: "eslint", language: "TypeScript", @@ -421,6 +495,7 @@ describe("assembleReport", () => { const cloud = { id: "c1", title: "Cloud c1", + explain: "Holding it keeps review on design instead of style.", surfaced: { prs: 1, reviewComments: 1 }, language: "TypeScript", estimatedViolations: 3, @@ -438,6 +513,20 @@ describe("assembleReport", () => { "cloud[c1].language", { legacy: [], cloud: [{ ...cloud, language: "jane@acme.io" }] }, ], + [ + "legacy[l1].explain", + { + legacy: [{ ...legacy, explain: "Ask jane@acme.io why it matters." }], + cloud: [], + }, + ], + [ + "cloud[c1].explain", + { + legacy: [], + cloud: [{ ...cloud, explain: "It broke api.acme.io last spring." }], + }, + ], [ "cloud[api.acme.io].id", { legacy: [], cloud: [{ ...cloud, id: "api.acme.io" }] }, @@ -878,7 +967,7 @@ describe("taskless report", () => { it("counts only review comments, and asks for a candidate estimate, not pricing", async () => { const { stdout, exitCode } = await run(["agent", "report"]); expect(exitCode).toBe(0); - expect(stdout).toMatch(/^# Topic: report {5}\(CLI v.+ \/ topic v5\)/m); + expect(stdout).toMatch(/^# Topic: report {5}\(CLI v.+ \/ topic v6\)/m); const flat = stdout.replaceAll(/\s+/g, " "); const counting = flat.slice(flat.indexOf("## Counting surfaced")); expect(counting).toContain("**Only review comments count.**");