fix(rules): ride out transient failures while polling, and resume a request by id

[theCodeDrift]

rule create and rule improve gave up on the first transient failure while polling a submitted request. The only retry was running the command again, which submitted a second generation request while the first could still finish. Fetching the rules after generation had the same problem, and a give-up there threw away a generation that had already succeeded.

What changes

• Transient failures are retried. While polling, and while fetching each produced rule, an unavailable outcome marked retryable (network failure, 408, 429, 5xx) is reported as progress and retried after the 15s poll interval. The command fails only after 8 in a row (about 2 minutes). Any other answer resets the count. A non-retryable unavailable (undocumented 4xx, malformed body), a documented error, a refusal, or a 401 still fails on the first occurrence.
• --resume <requestId> on both commands polls an already-submitted request and fetches, verifies, and writes what it produced, without submitting anything. It refuses --from and any value that is not a UUID with INVALID_INPUT, before calling the service. This also recovers a generated rule whose fetch gave up, without generating it again.
• Give-up messages name the request id, say the request was not cancelled and may still complete, and give the exact --resume command. The fetch give-up says no rules were written, and no longer says "try again", since that would regenerate the rule.
• Agent recipes (create-remote-rule v5, improve-rule v7): on NETWORK_ERROR, the recipe now says to run the named --resume command, and to confirm with the user before re-running with --from, which submits a new request. The requestId notes say --resume is the one command that takes it back.
• Spec: two requirements added to openspec/specs/cli-rules/spec.md by hand, with no OpenSpec change: transient-failure tolerance, and --resume.

Tests

test/rule-poll-transient.test.ts (12 tests) drives the real command against the v2 stub. The stub can now return a queue of HTTP statuses, network failures, and building answers. The tests cover:

• recovering from a mixed transient run;
• giving up at the budget;
• the count resetting after a real answer;
• immediate failure on a non-retryable answer and on 401;
• fetch retry and give-up;
• --resume after a polling give-up and after a fetch give-up (exactly one POST across both runs);
• improve --resume making no iterate call;
• both INVALID_INPUT refusals.

Setting the budget to 1 (the old behavior) fails every retry test.

Fixes #466

[theCodeDrift]

@claude /review

[claude]

Claude finished @theCodeDrift's task in 30m 18s —— View job

---

Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no review threads, review summaries, or top-level comments (only the @claude /review trigger itself), so this is the first review of this PR — the whole diff was assessed fresh, nothing was treated as already addressed.

• Read prior review data (none found — first review)
• Fetch PR diff and changed files (gh pr diff, gh pr view --json files)
• Review packages/cli/src/rules/generate.ts (poll/fetch retry budget, give-up messages)
• Review packages/cli/src/commands/rules.ts (--resume, identity/resume helpers)
• Review schema changes (rules-create.ts, rules-improve.ts)
• Review test coverage (rule-poll-transient.test.ts, v2-server.ts stub)
• Review agent recipes (create-remote-rule.md v5, improve-rule.md v7) and the hand-edited openspec/specs/cli-rules/spec.md
• Post final review

Findings

[New] One finding posted inline on packages/cli/src/commands/rules.ts:198 — resumeCommand (and the give-up messages that embed it, in rules/generate.ts:69 and :345) interpolates the server-authored requestId without the stripControlCharacters treatment this same file already applies to other server-authored text (outcome.reason/details/code, requestErrorText). The new agent recipes explicitly tell an agent to run the named --resume command on NETWORK_ERROR, so an unsanitized, attacker-controllable requestId from a malicious/compromised service is a plausible path to command injection against an agent that executes the suggestion via a shell — the same class of bug the immediately-preceding commit (416a2db) fixed for other fields, not yet extended to requestId.

Everything else checked out

• Retry budget logic (awaitRequest / fetchGenerated in generate.ts): the consecutive-failure counter, its reset on any non-unavailable answer, and the off-by-nothing boundary at MAX_CONSECUTIVE_UNAVAILABLE = 8 all match the spec text and the test assertions (exactly 8 calls to give up, exactly 1 submission across a mixed run, reset verified by the 7+building+7 case).
• --resume validation (resumeRequestId): mutual exclusion with --from and the UUID-shape check both run, and fail, before any identity resolution or network call — matches the "before calling the service" requirement and the two INVALID_INPUT tests.
• z.guid() vs z.uuid(): intentional per the inline comment (loose GUID shape vs. RFC version/variant bits) and consistent with zod ^4.3.6 in packages/cli/package.json.
• Single call site: deliverRevisions's new requestId parameter and servedOrThrow's new context/requestId/attempts parameters have no other callers to update; retryAdvice is still correctly used for submit failures (which are deliberately not retried, to avoid double-submitting a generation).
• Changeset bump: patch is correct — the package is still 0.11.2 (pre-1.0), matching this repo's documented "additions are patch under 0.y.z" rule.
• Docs: agent recipe version bumps (v4→v5, v6→v7) and the NETWORK_ERROR row updates are consistent between create-remote-rule.md and improve-rule.md; no other recipe (check.md, ci.md, onboard.md, rule-meta.md, update.md) references stale requestId/NETWORK_ERROR guidance that this change should have touched.
• Hand-edited spec: the two new requirements in openspec/specs/cli-rules/spec.md are additions (not a MODIFIED Requirements block replacing an existing one), so the "a delta replaces, not patches" hazard from this repo's own OpenSpec guidance doesn't apply here.

Per the review instructions, build/lint/tests were not run locally — CI reports those separately.

[theCodeDrift]

Re: @claude[bot] — "Claude finished @theCodeDrift's task in 30m 18s — Review complete…"
#468 (comment)

Thanks for the review. Its one finding, the unsanitized server requestId in the --resume command, is fixed in 801f918: submit and iterate now refuse any requestId that isn't a UUID, as an invalid response body. Nothing else needed changing.

— AI Coding Agent