Guide chargeable goal resumptions to change strategy - #1156
Conversation
|
Warning Your free Security trial is over. An organization admin can activate billing to continue. |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8cd238450
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| u32::try_from(spent).unwrap_or(u32::MAX) | ||
| } | ||
|
|
||
| fn automatic_resume_guidance(unchargeable: bool) -> Result<Option<GoalGuidance>, GoalTextError> { |
There was a problem hiding this comment.
At the added call sites, false means chargeable and true means unchargeable, so reversing either literal silently changes whether strategy guidance is injected. This is the boolean-blindness case prohibited by docs/style.md; represent the classification with a named two-variant enum so the polarity is visible and checked at every call.
AGENTS.md reference: AGENTS.md:L14-L15
Useful? React with 👍 / 👎.
| let guidance = automatic_resume_guidance(false) | ||
| .expect("the static guidance is admitted") | ||
| .expect("a chargeable failure carries guidance"); | ||
|
|
||
| assert_eq!(guidance.as_str(), CHARGEABLE_FAILURE_RESUME_GUIDANCE); |
There was a problem hiding this comment.
If attempt_automatic_resume flips the classification, always constructs Resume(None), or fails to persist the guidance as the next turn's input, both new tests still pass because they invoke only automatic_resume_guidance and compare its wrapper against the same constant used to construct it. Exercise the classification-to-command-to-accepted-input path so these tests classify the behavior their names claim.
AGENTS.md reference: AGENTS.md:L231-L237
Useful? React with 👍 / 👎.
| answered them, and every other event ends it. Below the required configured | ||
| chargeable-attempt budget, the appended need text states that automatic |
There was a problem hiding this comment.
When automatic_resume_attempt_budget is configured to anything other than 20, this changed paragraph correctly describes a configured budget, but the same section still calls it a “twenty-attempt goal budget” on line 224 and the page header still promises a “twenty-attempt ceiling” on line 24. Because this page owns the implemented cross-crate behavior, those remaining claims now contradict the changed contract and should also refer to the configured budget.
AGENTS.md reference: AGENTS.md:L46-L49
Useful? React with 👍 / 👎.
Rust coverage (report only)Report only. This measurement has no threshold, gates no merge, and
What this number does not measure
Per crate, least-covered first
25 files with the most uncovered lines
Measured at |
Outcome
Chargeable automatic goal resumptions now receive typed strategy guidance instead of replaying the original goal statement unchanged. Unchargeable infrastructure, provider, rate-limit, reconciliation-timeout, and restart failures continue to reuse the original input without consuming the configured attempt budget.
The runtime classifies the exact failed turn through the existing durable failure classifier. If that read fails, the bounded automatic-resume loop retries infrastructure failure and leaves the goal visibly blocked on exhaustion rather than guessing.
123 meaningfully changed lines (excluding lockfiles).
Validation
signalboxdbuild passedUS/Easternzoneinfo alias. The affected process test suite passed 25 tests with 2 ignored.Deployment
Deployed head:
f8cd23845. Startup and the public runtime health checks passed.Numeric bounds added: none. Existing required dogfood values are base backoff 120s, cap 1800s, attempt budget 20, and startup retry 1s.