[theCodeDrift]

Stack (root → tip):

• ➡️ fix(rules): check every generated rule before writing the first #632 (you are here)
  • fix(rules): roll back a rule delivery that fails mid-write #635

A rule-generation request can produce several rules. deliverRevisions verified every rule's signatures up front, but the other pre-write checks (rule id, known engine, and assessDelivery: unsafe paths, case-folded duplicates, incomplete sets) ran inside the write loop. So when rule 2 was refused, rule 1 was already in .taskless/rules/ while the command failed and reported no rules. Under --json those refusals also surfaced as INTERNAL_ERROR.

What changed

• writeServedRule is split into prepareServedRule (every check, no filesystem writes) and writePreparedRule (ensureTasklessDirectory, the write, the missing-fixtures and id-collision warnings). writeServedRule stays as prepare + write, so rule restore / rollback behave exactly as before, error types included.
• deliverRevisions now verifies and prepares every rule before writing the first. A prepare refusal that is a plain Error is wrapped as RULE_GENERATION_FAILED there (not at the source, so restore's reporting is unchanged); RULE_UNSUPPORTED keeps its code. Per-rule notices are held and released in request order, so output ordering is unchanged.
• The doc comment on deliverRevisions now states what it guarantees, and names the remaining gap: an IO failure part-way through the writes (out of scope, per the issue).
• OpenSpec: an ADDED requirement in cli-generated-rule-delivery, archived in this PR (21 scenarios before, 23 after).
• Patch changeset.

Testing

• New packages/cli/test/rule-create-multi-rule.test.ts drives the real rule create --json against the v2 stub with two rules: second rule with an unknown engine, an unsafe ../ path, or an incomplete set. Each asserts the error code and that .taskless/rules/ (pre-seeded with an older version of rule 1) is byte-identical afterwards. A fourth case checks both rules are written when both pass. The three refusal cases fail against the previous generate.ts.
• pnpm typecheck, pnpm lint, pnpm --filter @taskless/cli test (2970 passed).

Fixes #623

[theCodeDrift]

@claude /review

[github-actions]

Claude finished @theCodeDrift's task in 1m 53s —— View job

---

Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contains no review threads, reviews, or inline comments for this PR (only the trigger comment itself), so this is the first review of this PR — the whole diff was assessed fresh.

• Read prior review data — none found
• Fetch PR diff and changed files
• Review packages/cli/src/rules/files.ts
• Review packages/cli/src/rules/generate.ts
• Review new test packages/cli/test/rule-create-multi-rule.test.ts
• Review OpenSpec changes
• Post inline comments for findings — none warranted
• Post final summary

Assessment: looks correct. I traced the bug this closes — before this PR, writeServedRule ran the id/engine/assessDelivery checks inside the write loop in deliverRevisions, so a second rule's refusal could follow a first rule already written to .taskless/rules/. The fix verifies this:

• prepareServedRule (files.ts:52-90) does the id/engine/migration/assessDelivery checks with no filesystem writes — confirmed by reading assessDelivery and convertLegacyFileSet, both pure. writePreparedRule (files.ts:99-117) does only ensureTasklessDirectory/mkdir/writeDeliveredFileSet/warnings. writeServedRule composes the two, so recover.ts (restore/rollback, still calling writeServedRule unchanged) keeps its exact prior behavior, error types included.
• deliverRevisions (generate.ts:238-299) now runs fetch→verify→prepare for every rule in the first loop (via prepareOrThrow, which wraps a plain Error as RULE_GENERATION_FAILED while passing a CLIError like RULE_UNSUPPORTED through unchanged), and only then runs the write loop. Nothing is written until every rule has passed every check.
• Per-rule notices from the prepare step are held in a local array and flushed into delivered.notices immediately before that rule's write-step notices in the second loop — so output ordering across rules is preserved exactly as before the split, despite prepare and write now being separate passes.
• The new test (rule-create-multi-rule.test.ts) drives the real rule create --json command against a stub server with a pre-seeded rule directory, and asserts byte-identical .taskless/rules/ plus the correct error code for: unknown engine (RULE_UNSUPPORTED), unsafe ../ path, and incomplete file set (both RULE_GENERATION_FAILED), plus a positive case where both rules write. This matches the three failure modes the PR claims to fix and exercises the --json envelope, not just the thrown error.
• OpenSpec: the archived change's scenarios and the synced openspec/specs/cli-generated-rule-delivery/spec.md addition are consistent with the code and the changeset description.

Nothing here warrants an inline comment — I didn't find a correctness, security, or test-coverage gap worth flagging. One thing I'd call out as not a finding but worth confirming matches your intent: the stated out-of-scope gap (an IO failure mid-write after all rules pass their checks can still leave earlier rules written, since the write loop itself isn't transactional) is accurately documented in both the deliverRevisions doc comment and the spec, so it reads as a deliberate boundary rather than an oversight.

[theCodeDrift]

Re: @github-actions — "Claude finished @theCodeDrift's task in 1m 53s"
#632 (comment)

Thanks, no findings to act on. The mid-write IO gap is a deliberate boundary, out of scope per #623, and documented in the deliverRevisions comment and the spec.

— AI Coding Agent