fix: scope a workspace credits blocker to the seats it actually blocks - #37
Merged
george-elphick-talieisin merged 2 commits intoSep 20, 2026
Merged
Conversation
A "credits depleted" flag on one seat cooled every seat sharing its account_id, whatever their own usage looked like. On a live two-seat workspace that took the only usable seat out of rotation: backup1 was at 100% on its 5h window and flagged workspace_member_credits_depleted, while main sat at 5h 46% / wk 7% with no flag of its own. main was cooled anyway, and reconcile_snapshots said so in as many words: credits is workspace-wide; cooling main, backup1 until Sun 22:30 seat 'main' is cooling until Sun 22:30 (credits) but reports 5h 46% wk 7%; clear with codex-clean seat status --clear-cooldown main That second notice is the whole bug in one line: the code knew the seat was fine and cooled it regardless, then advised a manual clear that does not stick, because seat status re-reconciles in the same command. It compounds: credits carries no resets_at, so the deadline is a flat default_cooldown_seconds rather than a real reset time; is_lifted_by_reset is false for credits, so a free reset is never offered for it; and the blocked-run probe refetches both seats and re-applies it. A seat with quota to spare stayed blocked in rolling one-hour chunks for as long as its sibling was out. Credits only matter to a seat that has used up its included quota. A seat inside its own windows runs for free and was never going to spend credits. blocker_targets narrows a credits blocker to the members with no headroom left; a member with no reading at all is still cooled, since there is no evidence either way and the next refresh corrects it. spend_control is an admin-set hard stop and still cools every member. Applied at both cooling sites: reconcile_snapshots for snapshot evidence, and runner::affected_seats for a live run failure, where the seat that actually failed is always cooled whatever its stale reading says. Verified by replaying the real recorded state through reconcile_snapshots: before, main and backup1 both cool; after, only backup1 does. Codex code review r1: PASS, no findings.
Copilot started reviewing on behalf of
george-elphick-talieisin
September 20, 2026 21:05
View session
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Clear stale credits cooldowns and add live-run coverage before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
Scopes workspace credits cooldowns to seats lacking quota headroom while preserving workspace-wide spend_control behavior.
Changes:
- Adds per-seat blocker targeting.
- Applies targeting to reconciliation and live run failures.
- Adds credits and spend-control regression tests.
| File | Review |
|---|---|
src/usage.rs |
Implements scoped blocker targeting; fresh healthy readings do not clear previously propagated credits cooldowns. |
src/runner.rs |
Applies scoped targets to live failures; add run-path coverage and update the contradictory helper comment. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…uota Copilot review on PR #37, all three points accepted. - A seat cooled conservatively while its reading was unknown stayed cooled for the full default cooldown even after a refresh proved it had headroom: the fresh reading removed it from the blocker's targets, but cool_seats only ever extends, and the credits-available branch needs has_credits, so nothing lifted it. The blocked-run probe then reached the same dead end on every pass. A fresh Healthy reading now clears a credits cooldown, which is never that seat's own block. Healthy is the whole condition: an OnCredits seat has no included quota left and keeps it, and rate_limit, model_limit and spend_control are left alone. The gap was visible in the test, which cleared the cooldown by hand before asserting. It no longer does, which is what makes it a regression test. - The runner half of the change had no coverage: the existing credits case has no cached readings, so every sibling was a target there either way, and the new usage tests only reach reconcile_snapshots. Added a run-path test where main fails for credits and backup1 holds a cached healthy reading. Against the parent commit it returns 75 with a usable seat idle; it now returns 0, having fallen through to backup1. - affected_seats still documented cooling every same-workspace seat, which its own body had stopped doing.
george-elphick-talieisin
deleted the
fix/credits-cooldown-respects-seat-quota
branch
September 20, 2026 21:46
This was referenced Sep 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
creditsblocker on one seat cooled every seat sharing itsaccount_id, regardless of their own usage. On a live two-seat workspace that took the only usable seat out of rotation:backup1was at 100% on its 5h window and flaggedworkspace_member_credits_depleted;mainwas at 5h 46% / wk 7% with no flag of its own, and was cooled anyway.seat 'main' is cooling until Sun 22:30 (credits) but reports 5h 46% wk 7%; clear with ... --clear-cooldown main— and the clear it advised does not stick, becauseseat statusre-reconciles in the same command.creditscarries noresets_at, so the deadline was a flatdefault_cooldown_secondsrather than a real reset;is_lifted_by_resetis false for it, so no free reset was ever offered; and the blocked-run probe re-applied it. A seat with quota to spare stayed blocked in rolling one-hour chunks for as long as its sibling was out.usage::blocker_targetsnarrows a credits blocker to the members with no headroom left. A member with no reading is still cooled (no evidence either way; the next refresh corrects it), andspend_controlstill cools every member.reconcile_snapshotsfor snapshot evidence, andrunner::affected_seatsfor a live run failure — where the seat that actually failed is always cooled, whatever its possibly stale reading says.Not changed, deliberately: the flag taxonomy in
reason_for_reached_type.workspace_member_usage_limit_reachedis already treated as workspace-wide, so theworkspace_member_prefix does not mean per-member here; scoping from each seat's own quota evidence is the better mechanism and makes the distinction moot.Test plan
cargo test— 155 lib + 93 integration tests passcredits_blocker_spares_a_sibling_that_still_has_included_quotareplays the real-world numbers (46%/7% vs 100%/31% + the flag) and asserts the in-quota seat stays usablecredits_blocker_cools_a_sibling_with_no_reading_of_its_ownpins the conservative case, and that a later healthy reading does not re-cool itspend_control_still_cools_every_member_whatever_their_headroomreconcile_blocker_dominates_credits_in_either_orderupdated: its healthy-with-credits seat was at 50%, so under the fix it is correctly no longer cooled; moved to 100% so the order-independence property still holds on a seat genuinely out of quotastate.jsonandseats.tomlthroughreconcile_snapshots: pre-fix coolsmainandbackup1; post-fix cools onlybackup1codex-clean seat statuswith both seats fetched leavesmainuncooledmainreportsready (active)at 5h 61% / wk 10%;backup1cools on credits, as it should. The "is cooling ... but reports ..." advice line no longer appears.Known adjacent issue, not fixed here
backup1is denied a free reset that would genuinely unblock it: itscreditsreason failsis_lifted_by_reset(src/seat.rs:209) even though it is at 100% on a window a "Weekly + 5 hr" grant would clear. That is a separate defect inverdict's flag-first precedence.