diff --git a/.changeset/skip-hand-written-contracts.md b/.changeset/skip-hand-written-contracts.md new file mode 100644 index 000000000..2802f9eba --- /dev/null +++ b/.changeset/skip-hand-written-contracts.md @@ -0,0 +1,5 @@ +--- +"@qawolf/cli": patch +--- + +Skip public-API command generation for contracts served by hand-written commands, so a future `flow.list` contract does not mint a duplicate of `qawolf flows list --remote`. diff --git a/.claude/rules/domain-structure.md b/.claude/rules/domain-structure.md index d8a206de9..38a5eebb1 100644 --- a/.claude/rules/domain-structure.md +++ b/.claude/rules/domain-structure.md @@ -14,7 +14,7 @@ Import rules are enforced by oxlint: ## Adding a command -There are two registration patterns. Commands that expose a platform public-API endpoint are generated from contracts by `registerPublicApiCommands` (`src/commands/publicApi/`) — adding the contract to `@qawolf/api-contracts` and updating the dependency is all it takes. Commands with local logic or multi-step UX flows are hand-written under `src/commands//` following the steps below. Never register the same endpoint both ways — the generated path throws on command collisions. +There are two registration patterns. Commands that expose a platform public-API endpoint are generated from contracts by `registerPublicApiCommands` (`src/commands/publicApi/`) — adding the contract to `@qawolf/api-contracts` and updating the dependency is all it takes. Commands with local logic or multi-step UX flows are hand-written under `src/commands//` following the steps below. Never register the same endpoint both ways — the generated path throws on command collisions. When a contract's endpoint is served by a hand-written command, add its name to `handWrittenContractNames` in `src/commands/publicApi/index.ts` so the generator skips it. 1. Create the handler directory under `src/commands//` 2. Export a registration function (`registerXCommand`) that takes a Commander `program` instance diff --git a/AGENTS.md b/AGENTS.md index 8282ed979..82a616a22 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,7 +81,7 @@ The codebase is organized into four strict layers. **`core/`** holds pure functi API clients (tRPC for the QA Wolf platform, GitHub REST) live in `src/shell/platform/` and `src/shell/` respectively — one module per auth boundary. -Commands register in one of two ways: platform public-API endpoints are generated from contracts via `registerPublicApiCommands` (`src/commands/publicApi/`), while commands with local logic or multi-step UX flows are hand-written under `src/commands//`. Never both for the same endpoint. See `.claude/rules/domain-structure.md` ("Adding a command") for the full decision rule. +Commands register in one of two ways: platform public-API endpoints are generated from contracts via `registerPublicApiCommands` (`src/commands/publicApi/`), while commands with local logic or multi-step UX flows are hand-written under `src/commands//`. Never both for the same endpoint — contracts served by a hand-written command go in the generator's skip-list (`handWrittenContractNames`). See `.claude/rules/domain-structure.md` ("Adding a command") for the full decision rule. ## Code Style diff --git a/src/commands/publicApi/index.test.ts b/src/commands/publicApi/index.test.ts index 9e162dd60..2a22c9215 100644 --- a/src/commands/publicApi/index.test.ts +++ b/src/commands/publicApi/index.test.ts @@ -134,6 +134,37 @@ describe("registerPublicApiCommands", () => { ).toThrow('Generated command "run create" collides'); }); + it("skips contracts served by hand-written commands", () => { + const listContract = { + description: "List the flows of an environment.", + input: z.object({ environmentId: z.string() }), + kind: "read", + name: "flow.list", + output: z.object({ flows: z.array(z.object({ flowId: z.string() })) }), + } as const; + const getContract = { + description: "Look up a run.", + input: z.object({ runId: z.string() }), + kind: "read", + name: "run.get", + output: z.object({ status: z.string() }), + } as const; + const program = makeProgram(); + + registerPublicApiCommands(program, createSignalRegistry(), { + contracts: { flow: { list: listContract }, run: { get: getContract } }, + }); + + // No `flow` group either: skipped contracts create no empty namespaces. + expect( + program.commands.find((command) => command.name() === "flow"), + ).toBeUndefined(); + const run = program.commands.find((command) => command.name() === "run"); + expect( + run?.commands.find((command) => command.name() === "get"), + ).toBeDefined(); + }); + it("registers nested namespaces from custom contract trees", () => { const contract = { description: "Look up a run attempt.", diff --git a/src/commands/publicApi/index.ts b/src/commands/publicApi/index.ts index cc6fc790c..7573378ee 100644 --- a/src/commands/publicApi/index.ts +++ b/src/commands/publicApi/index.ts @@ -26,6 +26,10 @@ const groupDescriptions: Record = { run: "Trigger and manage QA Wolf runs on the platform", }; +// Contracts already served by hand-written commands; the generator must not +// mint duplicates (flow.list is served by `qawolf flows list --remote`). +const handWrittenContractNames: ReadonlySet = new Set(["flow.list"]); + function resolveGroup(parent: Command, segment: string): Command { const existing = parent.commands.find( (command) => command.name() === segment, @@ -90,7 +94,9 @@ export function registerPublicApiCommands( signals: SignalRegistry, options: Options = {}, ): void { - const specs = buildCommandSpecs(options.contracts ?? publicContractsV1); + const specs = buildCommandSpecs(options.contracts ?? publicContractsV1, { + skipContractNames: handWrittenContractNames, + }); for (const spec of specs) { registerSpec(program, signals, spec, options.authDeps); } diff --git a/src/domains/publicApi/commandSpecs.test.ts b/src/domains/publicApi/commandSpecs.test.ts index 4e8422b66..668445d8a 100644 --- a/src/domains/publicApi/commandSpecs.test.ts +++ b/src/domains/publicApi/commandSpecs.test.ts @@ -26,6 +26,32 @@ describe("buildCommandSpecs", () => { ]); }); + it("skips contracts named in skipContractNames without building their specs", () => { + // Unmappable input proves the skip happens before flag building: a + // hand-written contract never has to be expressible as generated flags. + const skipped = { + description: "Hand-written elsewhere", + input: z.object({ config: z.object({ nested: z.string() }) }), + kind: "read", + name: "flow.list", + output: z.object({}), + } as const; + const kept = { + description: "Generated", + input: z.object({ runId: z.string() }), + kind: "read", + name: "run.get", + output: z.object({}), + } as const; + + const specs = buildCommandSpecs( + { flow: { list: skipped }, run: { get: kept } }, + { skipContractNames: new Set(["flow.list"]) }, + ); + + expect(specs.map((spec) => spec.trpcPath)).toEqual(["public.run.get"]); + }); + it("throws when a contract name does not match its position in the tree", () => { const contract = { description: "Mismatched", diff --git a/src/domains/publicApi/commandSpecs.ts b/src/domains/publicApi/commandSpecs.ts index e43e5a10b..52b6e08cb 100644 --- a/src/domains/publicApi/commandSpecs.ts +++ b/src/domains/publicApi/commandSpecs.ts @@ -56,12 +56,23 @@ function buildSpec( }; } -export function buildCommandSpecs(tree: ContractTree): CommandSpec[] { +type BuildOptions = { + // Contracts served by hand-written commands. Skipped before spec building, + // so a hand-written contract never has to be expressible as generated flags. + skipContractNames?: ReadonlySet; +}; + +export function buildCommandSpecs( + tree: ContractTree, + options: BuildOptions = {}, +): CommandSpec[] { + const skip = options.skipContractNames; const walk = (node: ContractTree, path: string[]): CommandSpec[] => - Object.entries(node).flatMap(([key, value]) => - isContract(value) - ? [buildSpec(value, [...path, key])] - : walk(value, [...path, key]), - ); + Object.entries(node).flatMap(([key, value]) => { + const childPath = [...path, key]; + if (!isContract(value)) return walk(value, childPath); + if (skip?.has(childPath.join("."))) return []; + return [buildSpec(value, childPath)]; + }); return walk(tree, []); }