fix(code-review): short-circuit exhausted peer routes - #1344
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b25a97a91
ℹ️ 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".
| if candidate.timestamp() <= now + 60: | ||
| candidate += timedelta(days=1) |
There was a problem hiding this comment.
Keep imminent reset times on the current day
When the provider reports an absolute reset less than 60 seconds in the future, this condition advances the reset to tomorrow before adding the intended one-minute buffer. For example, at 15:49:30 a resets 3:50pm response opens the circuit for the full 24-hour cap instead of 90 seconds, unnecessarily disabling Claude peer reviews for the rest of the day; only advance the date when the reset time has actually passed.
Useful? React with 👍 / 👎.
| try: | ||
| zone = ZoneInfo(zone_name) if zone_name else datetime.now().astimezone().tzinfo | ||
| except ZoneInfoNotFoundError: | ||
| zone = datetime.now().astimezone().tzinfo |
There was a problem hiding this comment.
Honor named reset zones on native Windows
On native Windows installations without the optional tzdata package, ZoneInfo("America/New_York") raises ZoneInfoNotFoundError, so this fallback silently interprets the provider's explicitly New York reset time in the machine's local timezone. A user outside that zone can therefore remain blocked for several extra hours—or, when the misinterpreted time has passed, up to the 24-hour cap—despite native Windows being a supported target; use a source that can resolve the named zone or fall back conservatively rather than changing its meaning.
AGENTS.md reference: AGENTS.md:L55-L55
Useful? React with 👍 / 👎.
Preflight Claude authentication and persist a privacy-safe, reset-aware session-quota circuit so later reviews exclude a known-exhausted route before packaging or egress. Keep transient 429, execution-context auth, and transport failures distinct while preserving fixed-recipient behavior. Fixes EveryInc#1343 Co-Authored-By: Codex <codex@openai.com> Agent-Provenance: host=conductor harness=codex model=gpt-5.6-sol
0b25a97 to
2343c6b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2343c6bc54
ℹ️ 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".
|
|
||
| def _classify(text: str) -> str: | ||
| lowered = text.lower() | ||
| if "session limit" in lowered or "usage limit" in lowered: |
There was a problem hiding this comment.
Recognize unqualified Claude limit messages
When Claude reports quota exhaustion as You've hit your limit · resets ..., the text contains neither session limit nor usage limit, so _classify falls through to other. cmd_record then never persists a retry epoch, and every subsequent review sends another doomed API-bearing request instead of short-circuiting the route; recognize the unqualified limit form while tying it to reset or 429 evidence to avoid false positives.
Useful? React with 👍 / 👎.
| MAX_QUOTA_TTL = 24 * 3600 | ||
| SAFE_ROUTE = re.compile(r"^[A-Za-z0-9._-]+$") | ||
| RESET_AT = re.compile( | ||
| r"\bresets?\s+(?:at\s+)?(\d{1,2}):(\d{2})\s*(am|pm)" |
There was a problem hiding this comment.
Parse reset times that omit minutes
For round-hour diagnostics such as resets 4pm, this expression does not match because it requires :MM, so _retry_after uses the one-hour fallback rather than the provider's absolute reset. That can retry an exhausted route hours too early or suppress it after it is available; make the minute group optional and default it to 00.
Useful? React with 👍 / 👎.
Fixes #1343
Problem
The code-review cross-model route treated an installed Claude CLI as available even after the account had exhausted its session quota. It discovered the known-unhealthy route only after packaging and launching the adversarial review, then repeated that late failure on later invocations.
Fix
claude auth status --jsonbefore review payload construction;ce-code-reviewretains its local adversarial reviewer;resets 3:50pm (America/New_York)diagnostic shape.Claude exposes authentication state but no zero-cost quota-remaining endpoint. The first newly exhausted session still requires an API-bearing request; every subsequent review learns the unhealthy route before code leaves the host or a peer job displaces the local fallback.
Validation
git diff --checkpassed.Security Disclosure
Security-relevant state handling changed. The circuit stores no prompt, diff, raw diagnostic, account, or credential data. Its bounded state is owner-private, symlink/ownership checked, lock-serialized, and atomically published. A state-integrity failure fails closed before provider egress.
Agent Disclosure
Codex · GPT-5.6 Sol · via Conductor authored the implementation and tests.
🤖 Generated with Codex · GPT-5.6 Sol · via Conductor