Skip to content

Fix quota refreshes after provider reordering#2005

Merged
steipete merged 4 commits into
mainfrom
codex/issue-1994-provider-order-refresh
Jul 9, 2026
Merged

Fix quota refreshes after provider reordering#2005
steipete merged 4 commits into
mainfrom
codex/issue-1994-provider-order-refresh

Conversation

@steipete

@steipete steipete commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • separate config revisions used for UI projection from settings revisions that require provider background work
  • keep provider reordering visual-only while preserving conservative refreshes for substantive cross-window config changes
  • cover local, external, notification-relay, and UsageStore refresh behavior

Fixes #1994.

Verification

  • focused settings/UsageStore/MiMo coverage: 189 tests pass
  • cross-instance StatusItemController notification-relay regression: pass
  • make check
  • make test: all 49 shards pass
  • exact-branch autoreview: clean, no actionable findings
  • signed packaged app: macOS 27 build 26A5368g, arm64, Xcode 26.6 / SDK 26.5; Developer ID verification and Gatekeeper assessment pass
  • isolated live endpoint proof with Keychain access disabled and one loopback-only provider:
    • initial quota request count: 1
    • drag Codex from fourth to first in Settings: count remains 1
    • persisted order changes to Codex, OpenAI, Azure OpenAI, Claude, Cursor
    • disable/re-enable provider positive control: count increments from 1 to 2
    • relaunch: one expected startup request and the reordered sidebar persists

Exact packaged commit: 5ce335f9.

Risk

Low. The change narrows refresh observation only for provider ordering; provider credentials, source, enablement, and externally detected substantive config changes still invalidate background work. Dependencies unchanged.

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 6:41 AM ET / 10:41 UTC.

Summary
The branch separates order-only provider config revisions from background-work settings revisions and adds local, external, notification-relay, and UsageStore regression coverage.

Reproducibility: yes. Source inspection on current main shows provider reorder goes through setProviderOrder/updateConfig, bumps configRevision, and UsageStore observes configRevision for background work; I did not run live provider validation due repository policy.

Review metrics: 2 noteworthy metrics.

  • Background observer dependency: 1 configRevision read removed. This is the central behavior change that stops order-only config revisions from entering the provider refresh path.
  • Regression coverage: 4 test files updated. The added tests cover the local, external, notification-relay, and UsageStore refresh boundaries for this fix.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #1994
Summary: This PR is the direct candidate fix for the remaining provider-reorder refresh path in the canonical issue; the earlier merged PR is a partial fix for adjacent visual settings.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] Current-head macOS CI shards were still queued or in progress when inspected; merge should wait for required checks on 5ce335f.

Maintainer options:

  1. Decide the mitigation before merge
    Land this narrow revision split after current-head checks pass, preserving cached UI updates for provider order changes and provider refreshes for data-affecting settings.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No ClawSweeper repair job is needed; the PR is a maintainer-owned fix with sufficient proof and no actionable code findings, so the remaining action is normal merge review and check gating.

Security
Cleared: No security or supply-chain concern found; the diff changes Swift settings/usage observation logic, tests, and changelog text without dependency, script, secret, or external code execution changes.

Review details

Best possible solution:

Land this narrow revision split after current-head checks pass, preserving cached UI updates for provider order changes and provider refreshes for data-affecting settings.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection on current main shows provider reorder goes through setProviderOrder/updateConfig, bumps configRevision, and UsageStore observes configRevision for background work; I did not run live provider validation due repository policy.

Is this the best way to solve the issue?

Yes. The PR uses the narrow maintainable fix: split UI/config projection revisions from background-work revisions while preserving refreshes for provider-affecting config changes.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 36470082e509.

Label changes

Label justifications:

  • P2: This is a normal-priority settings refresh bugfix that reduces avoidable provider calls and rate-limit friction without being a core outage or security issue.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live loopback proof with Keychain access disabled, showing provider reorder did not increment quota requests while a provider-affecting positive control did.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live loopback proof with Keychain access disabled, showing provider reorder did not increment quota requests while a provider-affecting positive control did.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its guidance to prefer focused tests and avoid validation that can trigger provider or Keychain prompts shaped this read-only review. (AGENTS.md:1, 36470082e509)
  • No matching maintainer notes: The repository has .agents skills but no .agents/maintainer-notes files to apply for the touched settings and UsageStore paths. (36470082e509)
  • Current-main source reproduction: On current main, UsageStore observes settings.configRevision as part of the background-work observation token, so config revisions can trigger refreshForSettingsChange through the background settings observer. (Sources/CodexBar/UsageStore.swift:77, a83a83fa4131)
  • Current-main provider reorder path: On current main, setProviderOrder uses the same updateConfig and bumpConfigRevision path as provider-affecting config changes, making provider drag reorder part of the observed background-work input. (Sources/CodexBar/SettingsStore+ConfigPersistence.swift:85, a83a83fa4131)
  • PR implementation boundary: The PR marks provider order changes as affectsBackgroundWork: false, keeps provider config and token-account mutations as true, and infers external order-only changes by comparing provider-order-independent encoded configs. (Sources/CodexBar/SettingsStore+ConfigPersistence.swift:85, 5ce335f96d74)
  • Background observer narrowed: The PR removes the configRevision read from UsageStore.backgroundWorkSettingsObservationToken, leaving background refresh observation tied to backgroundWorkSettingsRevision. (Sources/CodexBar/UsageStore.swift:77, 5ce335f96d74)

Likely related people:

  • steipete: Blame ties the provider reorder/config revision path, StatusItemController config relay, and the PR fix commits to this account in the central files. (role: feature-history contributor and current area owner; confidence: high; commits: a83a83fa4131, b76c34014cc0, 96f29d50a77d; files: Sources/CodexBar/SettingsStore+ConfigPersistence.swift, Sources/CodexBar/UsageStore.swift, Sources/CodexBar/StatusItemController.swift)
  • Zihao-Qi: The merged related PR introduced backgroundWorkSettingsRevision and display-only refresh coverage that this follow-up extends to provider reorder. (role: recent partial-fix contributor; confidence: medium; commits: 5c6e6f99847d; files: Sources/CodexBar/UsageStore.swift, Sources/CodexBar/SettingsStore+Defaults.swift, Tests/CodexBarTests/UsageStoreCoverageTests.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (2 earlier review cycles)
  • reviewed 2026-07-09T10:05:40.803Z sha 97c5ef1 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-09T10:19:50.674Z sha 5ce335f :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. labels Jul 9, 2026
…ider-order-refresh

# Conflicts:
#	CHANGELOG.md
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 9, 2026
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

Maintainer proof for exact head 5ce335f96d74a26eee6f71a030f834bc204cca90:

  • focused settings, UsageStore, and MiMo coverage: 189 tests pass
  • cross-instance notification relay regression: pass
  • make check passes; make test passes all 49 local shards
  • exact-branch autoreview: clean, no actionable findings
  • Developer-ID signed debug app verified by codesign and Gatekeeper on macOS 27 build 26A5368g (arm64, SDK 26.5)
  • isolated loopback live proof with Keychain access disabled:
    • quota request count stays 1 -> 1 after dragging Codex from fourth to first
    • persisted order becomes Codex, OpenAI, Azure OpenAI, Claude, Cursor
    • disabling the provider stays at 1; re-enabling increments 1 -> 2
    • relaunch makes one expected startup request (2 -> 3) and preserves the reordered sidebar
  • independent source-blind behavior validation: PASS 6/6; report SHA-256 ae3d414d2dd26b65b5a94c7524c87d20c5b719872d296d3dab16ab3d2ba728e1
  • dependencies unchanged

Hosted exact-head CI: lint, changes, Linux arm64/x64, security, and all four macOS shards green.

@steipete steipete merged commit 7fdb1b9 into main Jul 9, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Changing visual/display settings refreshes provider quotas and can hit provider refresh rate limits

1 participant