Skip to content

Commit ca0a698

Browse files
theCodeDriftclaude
andcommitted
test(cli): derive the guard's inputs instead of restating them
Review feedback on the cross-reference guard, three findings, one root cause each: - The subcommand alternation was a hand-copied duplicate of the registry in src/index.ts. A command added there and forgotten here would drop out of the guard silently. Moved the names to src/commands/names.ts and had index.ts check its record against them with `satisfies`, so the drift is a type error. - The citation checks anchored on a fixed `taskless|@taskless/cli` spelling, which matches nothing a dev/self build renders — under those targets every check found zero citations and passed without measuring anything. The anchor now derives from `buildInvocation()`, and the dangling-citation check fails when it finds no citation at all. - openspec/specs/cli-agent claimed SKILL.md and tskl.md express the CLI as `%(TASKLESS_CLI)s`. They do not, by design: they carry the canonical `npx @taskless/cli` that `applyCliInvocation` rewrites. Reworded the requirement to describe both mechanisms, and scoped the automated-check sentence to recipe sources, which is all it covers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cyga14bww8rmazH2XrF8ms
1 parent c7b2b81 commit ca0a698

4 files changed

Lines changed: 103 additions & 20 deletions

File tree

‎openspec/specs/cli-agent/spec.md‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,11 +269,14 @@ No embedded recipe SHALL contain the string `taskless help`. Recipes cross-refer
269269

270270
### Requirement: Recipes name the CLI by its full invocation
271271

272-
Every instruction in shipped agent-facing content that tells a reader to run the Taskless CLI SHALL express the CLI as `%(TASKLESS_CLI)s`, followed by the subcommand and its arguments. This applies to the recipe sources under `packages/cli/src/agent/`, to `skills/taskless/SKILL.md`, and to `commands/tskl/tskl.md`.
272+
Every instruction in shipped agent-facing content that tells a reader to run the Taskless CLI SHALL name the CLI by the invocation the build resolves for the reader, followed by the subcommand and its arguments — never by a hand-written one. Two mechanisms deliver that, because the content travels by two different paths:
273273

274-
Content SHALL NOT name the CLI as a bare `taskless` binary, because it is not installed on `PATH` for the overwhelming majority of readers, and SHALL NOT hardcode a launcher-and-package string such as `npx @taskless/cli`, because that is the fact `%(TASKLESS_CLI)s` exists to hold in one place. Prose that mentions the product, a config filename, or a directory (`taskless.config`, `.taskless/`) is unaffected — the requirement is about executable instructions.
274+
- Recipe sources under `packages/cli/src/agent/` SHALL express the CLI as `%(TASKLESS_CLI)s`, which the recipe renderer substitutes at render time.
275+
- `skills/taskless/SKILL.md` and `commands/tskl/tskl.md` are not rendered through the recipe renderer, so they SHALL carry the canonical `npx @taskless/cli` string, which `applyCliInvocation` rewrites to the build target's invocation when the content is emitted.
275276

276-
An automated check SHALL fail when a bare `` `taskless <subcommand>` `` invocation appears in a recipe source, so the normalization cannot silently regress as recipes are edited.
277+
Content SHALL NOT name the CLI as a bare `taskless` binary, because it is not installed on `PATH` for the overwhelming majority of readers. A recipe source SHALL NOT hardcode a launcher-and-package string such as `npx @taskless/cli` either, because that is the fact `%(TASKLESS_CLI)s` exists to hold in one place — the string is canonical only in the two files above, where it is the rewrite anchor rather than a hardcoding. Prose that mentions the product, a config filename, or a directory (`taskless.config`, `.taskless/`) is unaffected — the requirement is about executable instructions.
278+
279+
An automated check SHALL fail when a bare `` `taskless <subcommand>` `` invocation, or a hardcoded `npx @taskless/cli`, appears in a recipe source, so the normalization cannot silently regress as recipes are edited. The check SHALL read the subcommand names from the CLI's own registry rather than restating them, so a newly added subcommand is covered without a second edit. It does not extend to `skills/taskless/SKILL.md` or `commands/tskl/tskl.md`, whose rewrite anchor is the very string it flags and which quote invocations as example user utterances.
277280

278281
#### Scenario: A recipe instructs the reader to run a subcommand
279282

@@ -294,3 +297,5 @@ An automated check SHALL fail when a bare `` `taskless <subcommand>` `` invocati
294297

295298
- **WHEN** the check that recipe cross-references cite only topics that resolve is run
296299
- **THEN** it SHALL operate on rendered recipe text, where the invocation is a stable literal, rather than on source text where it is a placeholder
300+
- **AND** it SHALL anchor on the invocation the running build actually renders, so the check does not pass vacuously under a `nightly`, `dev`, or `self` build whose rendered invocation names neither `taskless` nor `@taskless/cli`
301+
- **AND** it SHALL fail when it finds no cross-reference at all, since an empty result is otherwise indistinguishable from every reference resolving

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* Every top-level subcommand the CLI dispatches, as a reader would type it.
3+
*
4+
* This is the single source of truth for the *names*. `src/index.ts` builds its
5+
* `subCommands` record against it with `satisfies`, so a command added or
6+
* removed there without a matching entry here is a type error rather than a
7+
* silent drift — and tests that reason about invocations (see
8+
* `test/recipe-cross-references.test.ts`) read the same list instead of
9+
* restating it. It deliberately imports nothing: a consumer that only needs the
10+
* names must not have to load the whole command tree to get them.
11+
*/
12+
export const SUBCOMMAND_NAMES = [
13+
"agent",
14+
"auth",
15+
"check",
16+
"detect",
17+
"info",
18+
"init",
19+
"onboard",
20+
"rule",
21+
"test",
22+
"update",
23+
"verify",
24+
] as const;
25+
26+
export type SubcommandName = (typeof SUBCOMMAND_NAMES)[number];

‎packages/cli/src/index.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { detectCommand } from "./commands/detect";
77
import { initCommand, updateCommand } from "./commands/init";
88
import { testCommand, verifyCommand } from "./commands/verify";
99
import { infoCommand } from "./commands/info";
10+
import { type SubcommandName } from "./commands/names";
1011
import { onboardCommand } from "./commands/onboard";
1112
import { ruleCommand } from "./commands/rules";
1213
import {
@@ -19,6 +20,11 @@ import { DIR_FLAGS, hasHelpFlag, splitRawArguments } from "./util/argv";
1920
import { showResolvedUsage } from "./util/help";
2021
import { CLIError } from "./util/cli-error";
2122

23+
// `satisfies` against the name list (minus `agent`, which is constructed from
24+
// this record below) is what keeps `SUBCOMMAND_NAMES` honest: adding a command
25+
// here without naming it there — or naming one there that is never registered —
26+
// fails typecheck instead of quietly drifting away from every consumer that
27+
// reads the list.
2228
const subCommands = {
2329
init: initCommand,
2430
update: updateCommand,
@@ -30,7 +36,7 @@ const subCommands = {
3036
rule: ruleCommand,
3137
verify: verifyCommand,
3238
test: testCommand,
33-
};
39+
} satisfies Record<Exclude<SubcommandName, "agent">, unknown>;
3440

3541
const agentCommand = createAgentCommand(subCommands);
3642

@@ -61,7 +67,7 @@ const main = defineCommand({
6167
subCommands: {
6268
...subCommands,
6369
agent: agentCommand,
64-
},
70+
} satisfies Record<SubcommandName, unknown>,
6571
async run({ rawArgs, cmd }) {
6672
// citty always calls the parent's run handler, even after a subcommand.
6773
// Only take action when no positional args (i.e. no subcommand) were

‎packages/cli/test/recipe-cross-references.test.ts‎

Lines changed: 61 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import { readdir, readFile } from "node:fs/promises";
22
import { join, resolve } from "node:path";
33
import { describe, expect, it } from "vitest";
44

5+
import { SUBCOMMAND_NAMES } from "../src/commands/names";
56
import { getRecipe } from "../src/prompts/recipes";
7+
import { buildInvocation } from "../src/util/invocation";
68

79
/**
810
* Recipes cite each other by topic name, in prose. Nothing resolves those
@@ -24,9 +26,41 @@ import { getRecipe } from "../src/prompts/recipes";
2426
*/
2527
const recipeDirectory = resolve(import.meta.dirname, "../src/agent");
2628

27-
/** The CLI's subcommands, as a recipe would name one. */
28-
const SUBCOMMANDS =
29-
"agent|auth|check|detect|info|init|onboard|rule|test|update|verify";
29+
function escapeRegExp(literal: string): string {
30+
return literal.replaceAll(/[$()*+.?[\\\]^{|}]/g, String.raw`\$&`);
31+
}
32+
33+
/**
34+
* The CLI's subcommands, as a recipe would name one — read from the registry
35+
* in `src/commands/names.ts` rather than restated here. A restated list is a
36+
* blind spot with a delay on it: a subcommand added to the CLI but forgotten
37+
* here drops out of the alternation, and the guard below stops flagging a
38+
* hand-written `taskless <that-command>` without anything going red.
39+
*/
40+
const SUBCOMMANDS = SUBCOMMAND_NAMES.map((name) => escapeRegExp(name)).join(
41+
"|"
42+
);
43+
44+
/**
45+
* Every spelling the CLI can carry in RENDERED recipe text, longest first so
46+
* the alternation prefers the most specific.
47+
*
48+
* `<taskless-cli>` is what a prod build with no detected launcher renders, and
49+
* `buildInvocation()` is whatever THIS build bakes in — `npx @taskless/cli` for
50+
* prod, `npx @taskless/cli-nightly@<version>` for a nightly, and a bare
51+
* `node <path>/index.js` for `dev`/`self`. That last one is why the list is
52+
* derived rather than written out: a fixed `taskless|@taskless/cli` anchor
53+
* matches nothing under `TASKLESS_BUILD_TARGET=self`, so every citation check
54+
* below would find zero citations and pass without checking anything. The two
55+
* bare legacy spellings stay in the list so a citation that regresses to one of
56+
* them is still checked rather than silently skipped.
57+
*/
58+
const CLI_NAMES = [
59+
escapeRegExp(buildInvocation()),
60+
"<taskless-cli>",
61+
"@taskless/cli",
62+
"taskless",
63+
].join("|");
3064

3165
/**
3266
* A CLI invocation spelled out in a recipe source instead of deferred to
@@ -74,7 +108,7 @@ describe("shipped recipes name only commands that exist", () => {
74108
const offenders: string[] = [];
75109
for (const [file, rendered] of await renderedRecipes()) {
76110
for (const [index, line] of rendered.split("\n").entries()) {
77-
if (/(?:taskless|@taskless\/cli|<taskless-cli>) help\b/.test(line)) {
111+
if (new RegExp(String.raw`(?:${CLI_NAMES}) help\b`).test(line)) {
78112
offenders.push(`${file}:${String(index + 1)}: ${line.trim()}`);
79113
}
80114
}
@@ -87,26 +121,33 @@ describe("shipped recipes name only commands that exist", () => {
87121
it("cites only topics that resolve to an embedded recipe", async () => {
88122
const topics = await embeddedTopics();
89123
const dangling: string[] = [];
124+
const citation = new RegExp(
125+
String.raw`(?:${CLI_NAMES}) agent ([a-z][a-z-]*)`,
126+
"g"
127+
);
128+
let cited = 0;
90129

91130
for (const [file, rendered] of await renderedRecipes()) {
92131
for (const [index, line] of rendered.split("\n").entries()) {
93-
// `<taskless-cli> agent <topic>` is what the invocation renders to
94-
// under a prod build with no detected launcher, and it is the form
95-
// every recipe now uses. The two older spellings stay matched so a
96-
// citation that regresses to one of them is still checked rather than
97-
// silently skipped.
98-
for (const match of line.matchAll(
99-
/(?:taskless|@taskless\/cli|<taskless-cli>) agent ([a-z][a-z-]*)/g
100-
)) {
132+
for (const match of line.matchAll(citation)) {
101133
const topic = match[1];
102-
if (topic !== undefined && !topics.has(topic)) {
134+
if (topic === undefined) continue;
135+
cited += 1;
136+
if (!topics.has(topic)) {
103137
dangling.push(`${file}:${String(index + 1)} cites '${topic}'`);
104138
}
105139
}
106140
}
107141
}
108142

109143
expect(dangling).toEqual([]);
144+
// An empty `dangling` is the same result whether every citation resolved or
145+
// the pattern matched none of them, and the second reads as a pass. Recipes
146+
// cross-reference each other heavily, so a run that finds no citation at
147+
// all has stopped measuring the thing it reports on.
148+
expect(cited, "found no recipe cross-references to check").toBeGreaterThan(
149+
10
150+
);
110151
});
111152

112153
// Every recipe is reachable by the name its filename implies. A header that
@@ -152,8 +193,13 @@ describe("recipes defer the CLI invocation to the renderer", () => {
152193

153194
it("renders a real invocation into every recipe that names the CLI", async () => {
154195
const rendered = await renderedRecipes();
155-
const naming = rendered.filter(([, text]) =>
156-
text.includes("<taskless-cli>")
196+
// Same build-target reasoning as CLI_NAMES: a prod build with no detected
197+
// launcher renders the `<taskless-cli>` marker, every other target renders
198+
// its own invocation, and hardcoding the marker would make this assert
199+
// nothing under `dev`/`self`.
200+
const naming = rendered.filter(
201+
([, text]) =>
202+
text.includes("<taskless-cli>") || text.includes(buildInvocation())
157203
);
158204
// Nearly every recipe tells the reader to run something; if this drops to
159205
// a handful, the normalization has been undone rather than improved.

0 commit comments

Comments
 (0)