feat(rules): generate and improve rules through the v2 API (0.12.0, stack 2/6)

[theCodeDrift]

Stack (root → tip):

• feat(api): propose the v2 rule API migration and add the v2 client (0.12.0, stack 1/6) #411
  • ➡️ feat(rules): generate and improve rules through the v2 API (0.12.0, stack 2/6) #412 (you are here)
    • feat(check): reconcile every rule from a snapshot and fail on an edited static rule (0.12.0, stack 3/6) #413
      • feat(rules): add rule restore and rule rollback (0.12.0, stack 4/6) #414
        • refactor(api): retire the v1 API (0.12.0, stack 5/6) #415
          • feat(check): per-run directories, round-trip fixes, and the archive (0.12.0, stack 6/6) #416

Stack 2/6 of the v2 rule API migration. It merges down into #411, which reaches main once, carrying the whole stack. See #411 for why.

What changes

• rule create / rule improve use v2 generation (rules/generate.ts). The CLI submits, polls by requestId, then fetches each produced rule's head by ruleId. The head is fetched without revision=, so a Free plan is never refused for a rule it just generated. Every rule is verified before any is written, so one bad rule leaves the tree untouched.
• Every served file is checked against its signature before it's written (rules/verify-delivery.ts). The served revisionId must match the one polling reported, and a runtime set's signature must equal its check.ts entry. Anything else refuses the whole rule.
• A served set replaces its rule directory, .tests/ included (the rules team confirmed fixtures always ship). Parent directories are created as files are written.
• BREAKING for --json consumers: rule create --json prints requestId plus rules (the rule ids, which are directory names), and no longer prints ruleId, which always held the request id. rule improve's input ruleId is the directory name. 404 rule_not_found becomes RULE_NOT_FOUND.
• A request that ends failed or unsupported prints the server's error as given, with control characters stripped.
• Recipes: create-remote-rule, improve-rule, rule-meta, and the rule index now say the rule id is the directory name and is never the request id. Each topic version is bumped.
• Removed: the v1 request, poll, and iterate client calls, and their test.

The v1 single-content writers stay until #415, because the v1 repair inside check still calls them until #413 removes it.

Tests

The command-level tests now drive the real command against a stubbed v2 server (test/support/v2-server.ts), which signs served sets with the CLI's own hash. The #280 envelope guard now covers a tampered served rule. New tests cover:

• a revision mismatch
• directory replacement, including stale fixtures
• a sanitized failed error
• RULE_NOT_FOUND on improve
• 12 verifier refusals

pnpm typecheck, pnpm lint, and the full suite (110 files, 1,852 tests) pass.

Review fixes

• request_not_found while polling reports NETWORK_ERROR, with a message to resubmit. That id is the CLI's own request id, never a rule id, so RULE_NOT_FOUND's remedy ("re-check the directory name") was wrong.
• A generated status with no revisions is reported as an invalid response instead of a raw TypeError. It deliberately doesn't fall back to an empty list, which would report a successful delivery of zero rules.
• Kept as is: served rules are verified one at a time, so the first reported failure follows request order.

Refs TSKL-307

[theCodeDrift]

@claude /review

[claude]

Claude finished @theCodeDrift's task in 6m 29s —— View job

---

Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained only the triggering @claude /review comment, no prior review threads, review summaries, or top-level comments — this is the first review pass on this PR, so I assessed the whole diff.

• Read .prior-review.json (no prior threads/reviews found)
• Fetch PR diff and metadata (gh pr diff 412, full 21-file, ~2050-line diff)
• Review correctness, security, performance, tests, clarity
• Post inline comments for concrete issues
• Post final summary

Summary

This slice moves rule create / rule improve onto the v2 generation API: submit → poll by requestId → fetch each produced rule's head by ruleId (no revision=, so Free plans aren't refused) → verify every served file against its signature (rules/verify-delivery.ts) → write only after all rules in the request verify. It also makes a served file set replace the whole rule directory (.tests/ included), and reworks the --json envelope (ruleId → requestId + rules). The refactor is clean: rules/generate.ts cleanly factors the poll/fetch/verify/write pipeline out of commands/rules.ts, the new verifyServedRule checks are thorough (signature↔content, signature↔check.ts, revision match, exactly-one-file-set, fixtures-never-signed), and the test suite (stubbed v2 server, tampered-signature guard test, directory-replacement test, stale-fixture test) matches what the code does.

Three things worth a look, left as inline comments on packages/cli/src/rules/generate.ts:

1. request_not_found during polling maps to RULE_NOT_FOUND (lines 88–96). That code's documented meaning elsewhere (the rule improve error table, describeSubmitFailure's rule_not_found branch) is "the rule id you supplied doesn't exist." Here it's really "the CLI's own requestId expired mid-poll" — and for rule create there's no rule id in play at all yet. An agent following the documented remedy would go "re-check the directory name" for something that isn't a directory-name problem. v1's equivalent 404 surfaced as NETWORK_ERROR.
2. revisions.map(...) has no fallback for a missing/malformed field (line 165). getRequestStatus's acceptObject only checks the body is a record, not that revisions exists — so a generated status without it throws a raw TypeError here. Caught generically as INTERNAL_ERROR by completeRequest's catch, so not fatal, but it's a behavior regression from v1's explicit status.rules ?? [] guard, which treated the same case as "zero rules" rather than an internal error.
3. Minor/low-priority: fetching revisions is parallelized (Promise.all), but verifying them (and hashing each file within a verification) runs sequentially — could reuse the same parallel pattern if rule/file counts ever grow.

Nothing else stood out as a correctness, security, or test-coverage gap: path-safety, case-folding, symlink handling, and directory-replacement semantics in rules/deliver.ts are unchanged by this PR and were already careful; the new .tests/-included purge is deliberate and covered by updated tests in deliver.test.ts and repair-integration.test.ts; the --json schema/recipe doc changes (ruleId → requestId/rules) are consistent across schemas/rules-create.ts, schemas/rules-improve.ts, and the four agent/*.md recipes (each with its topic version bumped). CI is expected to catch pnpm typecheck/pnpm lint/tests; I did not run them here.

[theCodeDrift]

Re: @claude — "Claude finished @theCodeDrift's task in 6m 29s…"
#412 (comment)

All three findings are answered in their threads: the request_not_found code and the missing revisions guard are fixed in 52f441f, and sequential verification is kept on purpose.

— AI Coding Agent