diff --git a/.changeset/check-hides-warnings-in-ci.md b/.changeset/check-hides-warnings-in-ci.md new file mode 100644 index 00000000..c1fb015b --- /dev/null +++ b/.changeset/check-hides-warnings-in-ci.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +`taskless check` prints only error-severity findings when `CI` is `true` or `1`, and ends with a count of the warnings, infos, and hints it left out. Pass `--include-warnings` to print them. `--json` output and the exit code are unchanged. diff --git a/openspec/specs/cli-check/spec.md b/openspec/specs/cli-check/spec.md index e07deb1a..32911d95 100644 --- a/openspec/specs/cli-check/spec.md +++ b/openspec/specs/cli-check/spec.md @@ -97,6 +97,33 @@ When the `--json` flag is not set, the CLI SHALL format `CheckResult` items as h - **WHEN** the scanner completes with zero results - **THEN** stdout SHALL indicate that no issues were found +### Requirement: Check hides findings below error from human output in CI + +When `CI` is `true` or `1` and `--include-warnings` is not set, the human output SHALL print only error-severity findings, and SHALL state how many findings it left out, by severity, and that `--include-warnings` shows them. When every finding was left out, the output SHALL say that no errors were found rather than that no issues were found. Any other value of `CI`, including an empty string, SHALL NOT hide anything. `--json` output SHALL carry every finding whatever `CI` is. The exit code SHALL be unaffected. + +#### Scenario: Warnings are counted, not printed, in CI + +- **WHEN** `CI=true` and the scan produces one error and one warning +- **THEN** stdout SHALL include the error and SHALL NOT include the warning +- **AND** stdout SHALL include `1 warning hidden in CI. Pass --include-warnings to show them.` +- **AND** the process SHALL exit with code 1 + +#### Scenario: Only warnings matched in CI + +- **WHEN** `CI=1` and the scan produces only warnings +- **THEN** stdout SHALL begin with `No errors found.` followed by the hidden count +- **AND** the process SHALL exit with code 0 + +#### Scenario: --include-warnings shows them + +- **WHEN** `CI=true` and `--include-warnings` is set +- **THEN** stdout SHALL include every finding and no hidden count + +#### Scenario: JSON is unaffected + +- **WHEN** `CI=true` and `--json` is set +- **THEN** `results` SHALL include findings of every severity + ### Requirement: Check subcommand formats JSON output when --json is set When the `--json` flag is set, the CLI SHALL output each `CheckResult` as a JSON object per line (JSONL format) to stdout. diff --git a/packages/cli/README.md b/packages/cli/README.md index 73caf220..cd9c007b 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -61,6 +61,12 @@ calls, so CI needs no secrets. Runtime rules (which execute code) only run once the server has verified their signature; otherwise they are reported as skipped and never change the exit code. +When `CI` is `true` or `1`, the human output prints only error-severity +findings and ends with a count of the warnings, infos, and hints it left out. +They never affect the exit code, and nobody reads them in a job log. Pass +`--include-warnings` to print them anyway. `--json` always carries every +finding. + Ask your agent to "add taskless to CI" and it will wire this into the CI system you already use rather than replacing it. diff --git a/packages/cli/src/agent/check.md b/packages/cli/src/agent/check.md index 83772342..c0a01a87 100644 --- a/packages/cli/src/agent/check.md +++ b/packages/cli/src/agent/check.md @@ -269,6 +269,8 @@ delete the rules to make `check` pass, and do not suggest never reported via the `{ ok: false, code, message }` envelope: the envelope only appears when the scan itself fails (e.g. `SCAN_FAILED`) and the normal results payload is absent. + `results` is whole even when `CI` is set: only the human output + hides findings below error there, and `--include-warnings` shows them. ## Exit codes diff --git a/packages/cli/src/commands/check.ts b/packages/cli/src/commands/check.ts index 3530500e..7c1e5b71 100644 --- a/packages/cli/src/commands/check.ts +++ b/packages/cli/src/commands/check.ts @@ -23,6 +23,7 @@ import { requireReconciledEngines } from "../rules/engine-gate"; import { discoverRuntimeRules } from "../rules/runtime/discover"; import { resolveRuleSelection, type RuleSelection } from "../rules/rule-filter"; import { scanProject } from "../rules/scan-project"; +import { isCiEnvironment } from "../util/interactive"; import { markNotice } from "../util/notices"; import { getCliPrefix } from "../util/package-manager"; @@ -230,6 +231,12 @@ export const checkCommand = defineCommand({ type: "string", description: "Run only the named rule; repeatable (--rule a --rule b)", }, + "include-warnings": { + type: "boolean", + description: + "Show findings below error severity in CI, where they are hidden by default", + default: false, + }, "preserve-logs": { type: "boolean", alias: "l", @@ -555,7 +562,21 @@ export const checkCommand = defineCommand({ }); console.log(JSON.stringify(output)); } else { - console.log(formatText(results)); + // In CI, findings below error are counted rather than printed: they + // never fail the run, and nobody reads them in a job log. Human + // output only. `--json` is read by a program, often an agent + // checking that a rule fires, and a sandbox that sets `CI` would + // otherwise hand it an empty `results` for a warning rule that + // matched. + const hideBelowError = + isCiEnvironment(process.env.CI) && !args["include-warnings"]; + const shown = hideBelowError + ? results.filter((result) => result.severity === "error") + : results; + const hidden = hideBelowError + ? results.filter((result) => result.severity !== "error") + : []; + console.log(formatText(shown, hidden)); if (preserve) { console.error(`Run directory kept: ${runDirectory}`); } diff --git a/packages/cli/src/util/format.ts b/packages/cli/src/util/format.ts index ce17c4ff..35315a58 100644 --- a/packages/cli/src/util/format.ts +++ b/packages/cli/src/util/format.ts @@ -1,9 +1,42 @@ import type { CheckResult } from "../types/check"; -/** Format results as human-readable diagnostic output */ -export function formatText(results: CheckResult[]): string { +/** `2 errors, 1 warning`, in severity order, naming only the ones present. */ +function describeSeverities(results: CheckResult[]): string { + const counts: Record = {}; + for (const result of results) { + counts[result.severity] = (counts[result.severity] ?? 0) + 1; + } + + const parts: string[] = []; + for (const severity of ["error", "warning", "info", "hint"] as const) { + const count = counts[severity]; + if (count !== undefined) { + parts.push(`${String(count)} ${severity}${count === 1 ? "" : "s"}`); + } + } + return parts.join(", "); +} + +/** + * Format results as human-readable diagnostic output. + * + * `hidden` is what the caller filtered out (findings below error, in CI). They + * are never printed, only counted, so a run that hid them cannot read as one + * that found nothing to hide. + */ +export function formatText( + results: CheckResult[], + hidden: CheckResult[] = [] +): string { + const hiddenLine = + hidden.length === 0 + ? undefined + : `${describeSeverities(hidden)} hidden in CI. Pass --include-warnings to show them.`; + if (results.length === 0) { - return "No issues found."; + return hiddenLine === undefined + ? "No issues found." + : `No errors found. ${hiddenLine}`; } const lines: string[] = []; @@ -21,24 +54,11 @@ export function formatText(results: CheckResult[]): string { lines.push(""); } - // Summary - const counts: Record = {}; - for (const result of results) { - counts[result.severity] = (counts[result.severity] ?? 0) + 1; - } - - const parts: string[] = []; - for (const severity of ["error", "warning", "info", "hint"] as const) { - const count = counts[severity]; - if (count !== undefined) { - parts.push(`${String(count)} ${severity}${count === 1 ? "" : "s"}`); - } - } - const uniqueFiles = new Set(results.map((r) => r.file)).size; lines.push( - `${String(results.length)} issue${results.length === 1 ? "" : "s"} (${parts.join(", ")}) across ${String(uniqueFiles)} file${uniqueFiles === 1 ? "" : "s"}` + `${String(results.length)} issue${results.length === 1 ? "" : "s"} (${describeSeverities(results)}) across ${String(uniqueFiles)} file${uniqueFiles === 1 ? "" : "s"}` ); + if (hiddenLine !== undefined) lines.push(hiddenLine); return lines.join("\n"); } diff --git a/packages/cli/test/check.test.ts b/packages/cli/test/check.test.ts index 08ab4507..00c95250 100644 --- a/packages/cli/test/check.test.ts +++ b/packages/cli/test/check.test.ts @@ -24,12 +24,15 @@ const fixturesDirectory = resolve( /** Run the CLI and capture output, allowing non-zero exit codes */ async function runCli( - args: string[] + args: string[], + env: Record = {} ): Promise<{ stdout: string; stderr: string; exitCode: number }> { await migrateFixture(args); try { - const { stdout, stderr } = await execFileAsync("node", [binPath, ...args]); + const { stdout, stderr } = await execFileAsync("node", [binPath, ...args], { + env: { ...process.env, ...env }, + }); return { stdout, stderr, exitCode: 0 }; } catch (error) { const execError = error as { @@ -45,6 +48,32 @@ async function runCli( } } +/** One error rule and one warning rule, each matching once. */ +async function writeMixedSeverityProject(directory: string): Promise { + const rules = join(directory, ".taskless", "sg", "rules"); + await mkdir(rules, { recursive: true }); + for (const [id, severity, callee] of [ + ["no-console-error", "error", "console.error"], + ["no-console-warn", "warning", "console.warn"], + ] as const) { + await writeFile( + join(rules, `${id}.yml`), + [ + `id: ${id}`, + "language: javascript", + `severity: ${severity}`, + "rule:", + ` pattern: ${callee}($$$)`, + `message: Avoid ${callee}`, + ].join("\n") + ); + } + await writeFile( + join(directory, "test.js"), + 'console.error("a");\nconsole.warn("b");\n' + ); +} + describe("check", () => { let temporaryDirectory: string; @@ -196,6 +225,75 @@ describe("check", () => { expect(exitCode).toBe(0); // warnings only → exit 0 }); + describe("in CI", () => { + it("hides warnings from human output and counts them", async () => { + await writeMixedSeverityProject(temporaryDirectory); + + const { stdout, exitCode } = await runCli( + ["check", "-d", temporaryDirectory], + { CI: "true" } + ); + expect(exitCode).toBe(1); + expect(stdout).toContain("no-console-error"); + expect(stdout).not.toContain("no-console-warn"); + expect(stdout).toContain( + "1 warning hidden in CI. Pass --include-warnings to show them." + ); + }); + + it("says no errors were found, not no issues, when only warnings matched", async () => { + await writeMixedSeverityProject(temporaryDirectory); + await writeFile( + join(temporaryDirectory, "test.js"), + 'console.warn("b");\n' + ); + + const { stdout, exitCode } = await runCli( + ["check", "-d", temporaryDirectory], + { CI: "1" } + ); + expect(exitCode).toBe(0); + expect(stdout).not.toContain("No issues found."); + expect(stdout).toContain("No errors found. 1 warning hidden in CI."); + }); + + it("shows warnings again with --include-warnings", async () => { + await writeMixedSeverityProject(temporaryDirectory); + + const { stdout } = await runCli( + ["check", "-d", temporaryDirectory, "--include-warnings"], + { CI: "true" } + ); + expect(stdout).toContain("no-console-warn"); + expect(stdout).not.toContain("hidden in CI"); + }); + + it("leaves --json results whole", async () => { + await writeMixedSeverityProject(temporaryDirectory); + + const { stdout } = await runCli( + ["check", "-d", temporaryDirectory, "--json"], + { CI: "true" } + ); + const output = JSON.parse(stdout) as { + results: Array<{ ruleId: string }>; + }; + expect(output.results.map((result) => result.ruleId).toSorted()).toEqual([ + "no-console-error", + "no-console-warn", + ]); + }); + + it("hides nothing when CI is not a claim of CI", async () => { + await writeMixedSeverityProject(temporaryDirectory); + + const { stdout } = await runCli(["check", "-d", temporaryDirectory], { + CI: "false", + }); + expect(stdout).toContain("no-console-warn"); + }); + }); + it("does not require taskless.json to run", async () => { await cp(fixturesDirectory, temporaryDirectory, { recursive: true }); // Remove taskless.json if it exists — check should still work with just rules diff --git a/packages/cli/vite.config.ts b/packages/cli/vite.config.ts index 94ad62f0..ee4e3d8e 100644 --- a/packages/cli/vite.config.ts +++ b/packages/cli/vite.config.ts @@ -553,9 +553,16 @@ export default defineConfig({ // never launches `init` — and the daily shape tracked the Validate run // count. Tests that exercise the opt-out itself clear these with // `vi.stubEnv` first, so the default costs them nothing. + // + // `CI` is cleared for the same reason in the other direction: a runner sets + // it, every spawned `check` inherits it, and human output then hides + // findings below error. Suites asserting on a warning rule's text output + // would pass locally and fail in Validate. Tests of the CI behavior pass + // `CI` to the child explicitly. env: { TASKLESS_TELEMETRY_DISABLED: "1", DO_NOT_TRACK: "1", + CI: "", }, // Two projects, because `__TASKLESS_CLI__` is a compile-time define rather // than a value a test can stub: whatever this config resolves is what every