Repository navigation
fix(rules): stop reporting undelivered requests as success, and poll to a deadline - #660
Conversation
A request that ended in pr, merged, closed, an unknown status or no status printed the success envelope with rules: [] and exited 0. It now fails with RULE_GENERATION_FAILED and a message naming the state.
The request poll treated every status but accepted, classifying and building as final, so the server's pushing step, or any status added later, ended the wait early. Only explicitly terminal statuses now end the poll; pushing gets its own progress line and unknown statuses keep polling. awaitRequest also had no overall timeout. It now gives up after REQUEST_DEADLINE_MS (30 minutes, twice the generator's 15-minute stale-build reset) with NETWORK_ERROR and the --resume command. The deadline and clock are injectable through GenerationContext.
|
@claude /review |
|
Claude finished @theCodeDrift's task in 3m 58s —— View job Review: fix(rules): stop reporting undelivered requests as success, and poll to a deadlineReview mode: incremental — read 0 prior review thread(s) before reviewing.
Correctness. I traced Tests. Good coverage of the new behavior: New findings:
Nothing else stood out — the |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
awaitRequest's deadlineMs and now were documented as test seams but no test used them. Add one that calls awaitRequest with both, and fails if the injected clock is ignored.
Two fixes to how
rule create/rule improvewait on a generation request.#648: an end state that delivers nothing no longer exits 0
completeRequest'sdefaultbranch printed the success envelope withrules: []and returned 0 forpr,merged,closed, an unknown status, or a missing one. A--jsoncaller could not tell "nothing was delivered" from "it worked and produced nothing".Those states now go through
options.failwithRULE_GENERATION_FAILEDand a message naming the state (packages/cli/src/commands/rules.ts,undeliverableMessage).pr/merged/closedget their own wording: the rules went to a pull request, not to this CLI, which is reachable through--resumeon an older request. The code is reused rather than added, since a request that ends without delivering rules is what it already covers.completeRequestnow prints the success envelope only forgenerated.#638:
pushing, unknown statuses, and an overall deadlinegenerated,failed,unsupported,pr,merged,closed) end the poll.pushingis in progress with its own line ("Status: pushing — pushing the rule to a pull request..."), and any status the CLI does not know keeps polling.pushingis handled as a plain string ingenerate.ts; the generated schema/types are untouched, since they come from the live schema.NETWORK_ERROR, rather than being waited on.awaitRequestwaswhile (true), ended only byMAX_CONSECUTIVE_UNAVAILABLE. It now gives up afterREQUEST_DEADLINE_MS= 30 minutes withNETWORK_ERROR, the last status seen, and the--resumecommand (the existing give-up wording and code, which the recipes already route to--resume). The CLI had no poll timeout to reuse, so the value comes from the service: the generator resets a request stuck inbuilding/pushingfor over 15 minutes tofailed(taskless/tasklessopenspec/specs/ticket-pipeline/spec.md). Twice that leaves room for queue time and for that reset to be reported, so the deadline only catches what the service never resolves. The deadline and clock are injectable throughGenerationContext(deadlineMs,now), followingauth/login-step.ts.Tests
packages/cli/test/rule-poll-transient.test.ts, driving the real command:pr/merged/closedand a missing status fail non-zero with an error envelope; an unknown status that never ends fails at the deadline after 120 polls;pushingand an unknown status are waited through and then delivered; a request stuck inbuildinggives up at the deadline. The tests advance a fake clock in the stubbedsetTimeout, so nothing waits in real time. All of them fail before the fix.Fixes #648
Fixes #638