Skip to content

Commit c6474c2

Browse files
committed
feat(cli): report a delivery that carries no fixtures, and write it anyway
A rule delivered with no `.tests/` works. What it cannot do is demonstrate that it does: `taskless test` reports a pass having executed zero cases, which is indistinguishable from a rule that was proven. Warned rather than refused, and the asymmetry against `describeIncompleteSet` is the point. Everything that refuses a delivery names a rule that cannot work — no rule file, no config to scope it, no capture to invoke it. A fixture-less rule is not broken, and its holder did not author it and cannot add fixtures the service will bless, so refusing would leave them with no action available. Warning first means a gap surfaces from a report rather than from a blocked user, and hardening it into a refusal later can be decided on evidence. Checked against what arrived because the contract cannot express it. The service requires fixtures at construction as of taskless/taskless#163, but `files` is a flat `{path, content}[]` in the OpenAPI document with nothing marking one of them a fixture. Only the top-level `.tests/` counts, the same distinction `PRESERVED_SUBTREES` draws: `captures/.tests/` is a file an engine reads. Depth inside `.tests/` is not policed, since each engine's runner already judges that. Measured: neutering the check fails 3 of the new tests; restored, 1273 pass.
1 parent 8feee4c commit c6474c2

6 files changed

Lines changed: 179 additions & 5 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@taskless/cli": patch
3+
---
4+
5+
A rule delivered without any `.tests/` fixtures is now reported, and still written. A fixture-less rule works; what it cannot do is demonstrate that it does, since `taskless test` reports a pass over zero executed cases and that reads exactly like a rule proven to work. The warning names the rule and says so, and goes to stderr so it does not disturb `--json` on stdout.
6+
7+
Reported rather than refused, deliberately. The holder of a fixture-less rule did not author it and cannot add fixtures the service will bless, so a refusal would leave them with no action available. Warning first means a gap in the seam surfaces from a report rather than from a blocked user, and the decision to harden it into a refusal can be made on evidence.
8+
9+
The service now requires fixtures at construction, so a delivery without them should not occur — but `files` is a flat `{path, content}[]` in the published contract, with nothing that says one of them must be a fixture. The requirement is real and not visible across the seam, which is why this is checked against what actually arrived.

‎packages/cli/src/commands/rules.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,11 @@ const createCommand = defineCommand({
233233
const rules = status.rules ?? [];
234234

235235
for (const rule of rules) {
236-
const ruleFile = await writeRuleFile(cwd, rule);
236+
// To stderr, so a delivery worth remarking on is visible without
237+
// corrupting `--json` on stdout. The rule is written either way.
238+
const ruleFile = await writeRuleFile(cwd, rule, (message) => {
239+
console.error(`Warning: ${message}`);
240+
});
237241
writtenFiles.push(ruleFile);
238242

239243
// A file set carries its fixtures as ordinary files under
@@ -499,7 +503,11 @@ const improveCommand = defineCommand({
499503
const rules = status.rules ?? [];
500504

501505
for (const rule of rules) {
502-
const ruleFile = await writeRuleFile(cwd, rule);
506+
// To stderr, so a delivery worth remarking on is visible without
507+
// corrupting `--json` on stdout. The rule is written either way.
508+
const ruleFile = await writeRuleFile(cwd, rule, (message) => {
509+
console.error(`Warning: ${message}`);
510+
});
503511
writtenFiles.push(ruleFile);
504512

505513
// A file set carries its fixtures as ordinary files under

‎packages/cli/src/rules/deliver.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,48 @@ export function describeIncompleteSet(
165165
return undefined;
166166
}
167167

168+
/**
169+
* Why a delivered file set carries no fixtures, or `undefined` when it does.
170+
*
171+
* **A WARNING, NOT A REFUSAL, AND THE ASYMMETRY IS THE POINT.** Everything
172+
* {@link describeIncompleteSet} names is a rule that cannot work: no rule file,
173+
* no config to scope it, no capture to invoke it. A rule with no fixtures works
174+
* fine. What it cannot do is demonstrate that it does — `taskless test` reports
175+
* a pass having executed nothing, which is indistinguishable from a rule that
176+
* was proven. That is worth saying out loud and not worth refusing a delivery
177+
* over, because refusing would leave the holder of a fixture-less rule with no
178+
* action available to them: they did not author it and cannot add fixtures the
179+
* service will bless.
180+
*
181+
* So this reports and writes. If the service ever delivers without fixtures we
182+
* hear about it from a warning rather than from a blocked user, and the
183+
* decision to harden it into a refusal can be made on evidence.
184+
*
185+
* **The contract cannot express this, which is why it is checked here.** The
186+
* service requires fixtures at construction — a payload without them does not
187+
* compile on their side — but `files` is a flat `{path, content}[]` in the
188+
* OpenAPI document, with nothing that says one of them must be a fixture. The
189+
* requirement is real and simply not visible across the seam, so the only way
190+
* to know a delivery kept it is to look at what arrived.
191+
*
192+
* Asks only whether ANY file lands under `.tests/`. Depth is deliberately not
193+
* policed: what makes a fixture meaningful differs per engine, every engine's
194+
* own runner already judges it, and a second opinion here would be a worse one
195+
* computed from less.
196+
*/
197+
export function describeMissingFixtures(
198+
files: readonly DeliveredFile[]
199+
): string | undefined {
200+
const prefix = `${RULE_TESTS_DIRECTORY}/`;
201+
const hasFixture = files.some((file) => file.path.startsWith(prefix));
202+
if (hasFixture) return undefined;
203+
return (
204+
`was delivered with no ${prefix} fixtures, so nothing exercises it: ` +
205+
"`taskless test` reports a pass over zero cases, which is " +
206+
"indistinguishable from a rule proven to work"
207+
);
208+
}
209+
168210
/** A delivered file set that may be written, or the reason it may not. */
169211
export type DeliveryAssessment =
170212
| { ok: true; files: readonly DeliveredFile[] }

‎packages/cli/src/rules/files.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,24 @@ import { isValidRuleId } from "./validate-id";
1717
import {
1818
assessDelivery,
1919
deliveredFiles,
20+
describeMissingFixtures,
2021
writeDeliveredFileSet,
2122
} from "./deliver";
2223

2324
/**
2425
* Write a generated rule's content into its own rule directory —
2526
* `.taskless/rules/sg/{kebab-id}/{kebab-id}.yml` for the engine-less payloads
2627
* the API delivers today (see {@link resolveIngestEngine}).
28+
*
29+
* `onWarning` receives anything worth saying about a delivery that was still
30+
* written. Optional, and a caller that omits it loses the message rather than
31+
* the write — see {@link describeMissingFixtures} for why a missing fixture is
32+
* reported this way instead of refused.
2733
*/
2834
export async function writeRuleFile(
2935
cwd: string,
30-
rule: GeneratedRule
36+
rule: GeneratedRule,
37+
onWarning?: (message: string) => void
3138
): Promise<string> {
3239
if (!isValidRuleId(rule.id)) {
3340
throw new Error(`Invalid rule ID "${rule.id}"`);
@@ -68,6 +75,12 @@ export async function writeRuleFile(
6875
// repair is not special-cased. The single-content path below has no set to
6976
// be authoritative about and keeps overwriting one file.
7077
await writeDeliveredFileSet(cwd, engine, rule.id, assessment);
78+
// AFTER the write, deliberately. This is an observation about a rule that
79+
// is now on disk, and warning first would read as a reason it was refused.
80+
const missingFixtures = describeMissingFixtures(assessment.files);
81+
if (missingFixtures !== undefined) {
82+
onWarning?.(`Rule "${rule.id}" ${missingFixtures}.`);
83+
}
7184
// The rule file, so the caller's contract ("where did this rule land")
7285
// is unchanged whichever envelope delivered it.
7386
return ruleFilePath(cwd, engine, rule.id);

‎packages/cli/src/rules/runtime/plan.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,9 @@ async function repairWithheldRules(
268268
}
269269

270270
try {
271-
await writeRuleFile(cwd, rule);
271+
// A restored rule missing its fixtures is still the blessed bytes and is
272+
// still worth writing; the notice rides along with the repair's own.
273+
await writeRuleFile(cwd, rule, (message) => notices.push(message));
272274
} catch (error) {
273275
// A failed WRITE and a failed CLEANUP ask the reader for opposite
274276
// things, and saying "could not be written" for both is worse than

‎packages/cli/test/deliver.test.ts‎

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import { dirname, join } from "node:path";
1616
import { afterEach, beforeEach, describe, expect, it } from "vitest";
1717

1818
import { writeRuleFile } from "../src/rules/files";
19-
import { PurgeIncompleteError } from "../src/rules/deliver";
19+
import {
20+
PurgeIncompleteError,
21+
describeMissingFixtures,
22+
} from "../src/rules/deliver";
2023
import { ruleDirectory } from "../src/rules/engines";
2124
import type { GeneratedRule } from "../src/api/rules";
2225

@@ -603,3 +606,100 @@ asUser("a purge that cannot finish", () => {
603606
}
604607
});
605608
});
609+
610+
describe("a delivery that carries no fixtures", () => {
611+
const SG_RULE = "id: no-eval-abc12345\n";
612+
613+
it("warns, and still writes the rule", async () => {
614+
const warnings: string[] = [];
615+
await writeRuleFile(
616+
cwd,
617+
delivered("sg", "no-eval-abc12345", [
618+
{ path: "no-eval-abc12345.yml", content: SG_RULE },
619+
]),
620+
(message) => warnings.push(message)
621+
);
622+
623+
// Both halves matter. Warning without writing would be a refusal wearing a
624+
// warning's clothes, and writing without warning is the silence this
625+
// exists to break.
626+
expect(warnings).toHaveLength(1);
627+
expect(warnings[0]).toContain("no-eval-abc12345");
628+
expect(warnings[0]).toContain(".tests/");
629+
expect(
630+
existsSync(
631+
join(
632+
ruleDirectory(cwd, "sg", "no-eval-abc12345"),
633+
"no-eval-abc12345.yml"
634+
)
635+
)
636+
).toBe(true);
637+
});
638+
639+
it("says why it matters rather than only what is absent", async () => {
640+
const warnings: string[] = [];
641+
await writeRuleFile(
642+
cwd,
643+
delivered("sg", "no-eval-abc12345", [
644+
{ path: "no-eval-abc12345.yml", content: SG_RULE },
645+
]),
646+
(message) => warnings.push(message)
647+
);
648+
649+
// The consequence is the reason this is worth printing: a fixture-less
650+
// rule passes `test` over zero cases and reads as proven.
651+
expect(warnings[0]).toContain("nothing exercises it");
652+
});
653+
654+
it("stays silent when fixtures are present", async () => {
655+
const warnings: string[] = [];
656+
await writeRuleFile(
657+
cwd,
658+
delivered("sg", "no-eval-abc12345", [
659+
{ path: "no-eval-abc12345.yml", content: SG_RULE },
660+
{ path: ".tests/valid/sample.ts", content: "const x = 1;\n" },
661+
]),
662+
(message) => warnings.push(message)
663+
);
664+
665+
expect(warnings).toEqual([]);
666+
});
667+
668+
it("loses the message, not the write, when no handler is passed", async () => {
669+
// `onWarning` is optional, and a caller that omits it must still get the
670+
// rule on disk rather than an unhandled throw.
671+
await expect(
672+
writeRuleFile(
673+
cwd,
674+
delivered("sg", "no-eval-abc12345", [
675+
{ path: "no-eval-abc12345.yml", content: SG_RULE },
676+
])
677+
)
678+
).resolves.toBeTypeOf("string");
679+
});
680+
});
681+
682+
describe("what counts as a fixture", () => {
683+
it("accepts a fixture at any depth under .tests/", () => {
684+
// Depth is deliberately not policed here; each engine's runner judges what
685+
// a meaningful fixture is.
686+
expect(
687+
describeMissingFixtures([
688+
{ path: "check.ts", content: "" },
689+
{ path: ".tests/fail/deeply/nested/case/input.ts", content: "" },
690+
])
691+
).toBeUndefined();
692+
});
693+
694+
it("does not accept a nested .tests/ that an engine would read", () => {
695+
// Only the top-level `.tests/` is the fixture directory. `captures/.tests/`
696+
// is a file an engine reads, which is the same distinction
697+
// `PRESERVED_SUBTREES` draws when it decides what a purge may remove.
698+
expect(
699+
describeMissingFixtures([
700+
{ path: "check.ts", content: "" },
701+
{ path: "captures/.tests/logs.yml", content: "" },
702+
])
703+
).toBeDefined();
704+
});
705+
});

0 commit comments

Comments
 (0)