[theCodeDrift]

check --json gets a per-rule summary, and check/verify share one finding shape.

• check --json always includes summary: total findings, distinct files, counts by severity, and rules[] keyed by engine + ruleId (the pair verify groups by), most findings first.
• New --summary flag: with --json it omits results; without it prints one count line per rule. Exit code unchanged.
• One findingSchema (schemas/finding.ts); verify's fixture findings reuse it and add bucket. The two had drifted on nullable note/fix; the shared schema allows null as check already did.
• @taskless/cli/schemas publishes findingSchema and checkOutputSchema. The envelopes stay different on purpose (check lists findings, verify lists rules), documented in the schema exports and recipe (check topic v10) and README.
• OpenSpec check-json-summary, archived here (single-PR shape). The spec still described check --json as JSONL; that requirement is replaced (REMOVED + ADDED) with the real envelope, plus three new requirements.

For review: --summary drops results rather than sending [] (an empty array reads as a clean run); rules[] lists only rules that fired.

Tests: summarizeFindings units, e2e for summary/--summary in JSON and text, and one finding from each of check --json and verify --json parsed by the published findingSchema (the issue's done-when). typecheck, lint, pnpm test (3172 passed) pass. Changeset is patch: additive unless --summary is passed.

Commits are unsigned: the signing key couldn't be unlocked remotely, and the owner chose to ship unsigned.

Closes #678

[theCodeDrift]

@claude /review

[github-actions]

Claude finished @theCodeDrift's task in 2m 59s —— View job

---

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no review threads and no prior reviews — just the triggering @claude /review comment. This is the first full review of this PR; there is nothing previously-raised to carry forward.

Todo

• Read .prior-review.json
• Fetch PR diff via gh pr diff
• Read changed files for context (schemas, summarize-findings, check.ts, format.ts, rule-check.ts)
• Review for correctness, security, performance, test adequacy, clarity
• Post inline comments for concrete issues (none needed — see below)
• Post final summary

Assessment

I read the full diff plus the whole of check.ts, schemas/check.ts, schemas/finding.ts, schemas/rule-check.ts, schemas/index.ts, util/format.ts, and summarize-findings.ts (including the surrounding, unmodified code those files sit in). I did not find a correctness, security, or performance bug worth an inline comment. Specifically checked and found sound:

• summarizeFindings groups by (source, ruleId), normalizes file spellings via relative(cwd, resolve(cwd, file)), and only omits an engine key for a source that isn't mapped — matching the strict ruleSummarySchema/checkSummarySchema exactly, so .strictObject can't reject the constructed objects.
• findingsFields in check.ts is the single place that decides whether results is included and always computes summary; every call site (both early-return branches and the main scan path) goes through it, so the early returns can't disagree with the full-scan path about which keys a response carries — this was clearly a deliberate design goal and it holds up.
• The CI "hide below error" filter intentionally does not apply to --summary output (JSON summary or text formatSummaryText), unlike the plain-text finding list, which does filter in CI. This is explicitly called out in the formatSummaryText doc comment and is consistent between the JSON and text summary paths — not a bug, a deliberate and documented divergence.
• exitCode computation is untouched by summaryOnly, so "exit code unchanged" (as the PR body claims) holds in the code.
• Schema sharing: fixtureFindingSchema = findingSchema.extend({...}) correctly inherits the now-nullable note/fix, which is a strictly looser (non-breaking) change for existing verify/validate consumers. checkOutputSchema's results becomes optional() while errorSchema.results stays required — correct, since the error envelope is unrelated to --summary.
• Sort order: summary.rules sorts by total desc, then ruleId, then engine as a final tiebreak for two engines sharing a rule id — matches the unit test (keeps one id under two engines apart) and is a reasonable, deterministic addition beyond what the openspec requirement text literally states (which only mentions total/ruleId).
• Tests are solid: summarize-findings.test.ts covers empty input, ordering, file-spelling de-duplication, same id under two engines, and an unmapped source; check.test.ts covers --json summary shape, --summary omitting results while preserving exit code, the no-rules-configured case, and the text-mode per-rule output; schemas-export.test.ts proves one finding from each of check --json and verify --json (from the actually-built CLI) parses against the one published findingSchema, which is exactly the issue's done-when.

Two very minor, non-blocking observations, not raised as inline comments since neither is a defect:

• packages/cli/src/rules/check-summary.ts is a pre-existing, unrelated file (rule-reconciliation reporting for verify's snapshot signing) that happens to share a very similar name with the new summarize-findings.ts/summary concept this PR adds. Worth being aware of for future readers skimming src/rules/, but not something this PR needs to rename.
• formatText's inline pluralization and the new formatSummaryText's plural()/describeCounts() helpers duplicate the same "append s unless count is 1" logic in slightly different forms. Harmless duplication, not worth a refactor for this PR.

No findings to flag. Typecheck/lint/tests were not re-run locally per review-mode instructions (CI reports those); the PR description states they pass (3172 tests).

[theCodeDrift]

Re: @github-actions — "Claude finished @theCodeDrift's task in 2m 59s"…
#688 (comment)

Thanks for the review. The pluralization duplication is folded in e81aef1: describeSeverities now delegates to describeCounts, and formatText uses plural(), so the output stays the same. The check-summary.ts name overlap stays as it is. That file predates this PR, and renaming it is out of scope here.

— AI Coding Agent