Skip to content

Scope provider status indicators - #1603

Merged
steipete merged 2 commits into
steipete:mainfrom
Zihao-Qi:codex/scope-provider-status-indicators
Jun 18, 2026
Merged

Scope provider status indicators#1603
steipete merged 2 commits into
steipete:mainfrom
Zihao-Qi:codex/scope-provider-status-indicators

Conversation

@Zihao-Qi

@Zihao-Qi Zihao-Qi commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Scope merged menu bar status indicators to the provider currently rendered in the icon.
  • Stop merged-icon observation from redrawing for status changes on unrelated providers.
  • Add a transparent halo behind status markers so the circle and exclamation remain readable over quota bars.
  • Cover provider scoping, observation, and both indicator glyphs with focused regression tests.

Why

In merged-icon mode, the icon can represent one selected provider while other providers remain enabled. Previously, a background provider could supply the visible status marker, making the icon and marker refer to different providers.

Proof

Exact reviewed head: ce4fc10958b4861affbfa91af688e0a771c47349

  • swift test --filter 'status overlays cut halos|StatusItemAnimationSignatureTests|StatusItemIconObservationSignatureTests' - 19 tests passed.
  • make check - clean.
  • Full branch autoreview - clean, confidence 0.84.
  • CODEXBAR_SIGNING=adhoc ./Scripts/package_app.sh - packaged exact head.
  • codesign --verify --deep --strict --verbose=2 CodexBar.app - valid.
  • Packaged app reports CodexGitCommit=ce4fc109.

Live menu recapture is blocked by the current macOS App Data consent sheet; no permission was granted. The contributor screenshots below show both rendered glyphs, and pixel-level tests verify the halo and glyph pixels deterministically.

Screenshots

Minor provider status marker Major provider status marker

@clawsweeper

clawsweeper Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 17, 2026, 3:24 PM ET / 19:24 UTC.

Summary
The PR scopes merged menu-bar status rendering and icon-observation signatures to the currently rendered provider, adds halo cutouts behind status markers, and adds focused regression tests.

Reproducibility: yes. from source inspection: current main renders one primary provider in merged-icon mode but can select a status indicator by scanning all enabled providers. I did not run live app validation in this read-only review.

Review metrics: 1 noteworthy metric.

  • Diff size: 6 files, +127/-25. The review surface is limited to three icon/controller files and three focused test files.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • [P1] Add redacted real behavior proof showing a merged icon for one selected provider while another provider has a status issue, then switch providers to show the marker follows the rendered provider.
  • [P1] Keep the existing halo screenshots or add a short recording that also shows marker readability over quota bars.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: Screenshots show the status halo, but not the scoped merged-icon behavior; the contributor should add redacted screenshot/recording or live output showing selected provider versus unrelated provider status, then update the PR body to trigger review or ask for @clawsweeper re-review.

Mantis proof suggestion
A short desktop proof would materially show the merged menu-bar icon marker following the rendered provider and remaining readable over quota bars. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify in Merge Icons mode that status markers follow the rendered provider and remain readable over quota bars.

Risk before merge

  • [P1] The posted screenshots show the halo/readability improvement, but not a real merged-icon scenario proving that unrelated provider status changes no longer affect the visible marker.

Maintainer options:

  1. Decide the mitigation before merge
    Merge the scoped-provider implementation with its regression tests after real behavior proof demonstrates the merged-icon provider/status scenario.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The remaining blocker is contributor real behavior proof, not a narrow code repair that automation can supply.

Security
Cleared: The diff touches Swift icon/controller code and tests only; no secrets, dependencies, workflows, or external execution paths changed.

Review details

Best possible solution:

Merge the scoped-provider implementation with its regression tests after real behavior proof demonstrates the merged-icon provider/status scenario.

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

Yes from source inspection: current main renders one primary provider in merged-icon mode but can select a status indicator by scanning all enabled providers. I did not run live app validation in this read-only review.

Is this the best way to solve the issue?

Yes, the proposed code path is the narrow maintainable fix: scope both rendering and observation to primaryProviderForUnifiedIcon and cover the regression. The remaining blocker is proof, not a code-shape concern.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 05545feba362.

Label changes

Label changes:

  • add P2: The PR fixes a misleading provider-status signal in a bounded menu-bar icon path with focused regression coverage.
  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Screenshots show the status halo, but not the scoped merged-icon behavior; the contributor should add redacted screenshot/recording or live output showing selected provider versus unrelated provider status, then update the PR body to trigger review or ask for @clawsweeper re-review.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Screenshots show the status halo, but not the scoped merged-icon behavior; the contributor should add redacted screenshot/recording or live output showing selected provider versus unrelated provider status, then update the PR body to trigger review or ask for @clawsweeper re-review.

Label justifications:

  • P2: The PR fixes a misleading provider-status signal in a bounded menu-bar icon path with focused regression coverage.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Screenshots show the status halo, but not the scoped merged-icon behavior; the contributor should add redacted screenshot/recording or live output showing selected provider versus unrelated provider status, then update the PR body to trigger review or ask for @clawsweeper re-review.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. Screenshots show the status halo, but not the scoped merged-icon behavior; the contributor should add redacted screenshot/recording or live output showing selected provider versus unrelated provider status, then update the PR body to trigger review or ask for @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Repository policy applied: AGENTS.md was read fully; line 45 says provider data should stay siloed when rendering provider-specific information, which supports treating cross-provider status indicators as a real bug boundary. (AGENTS.md:45, 05545feba362)
  • Current main can mix rendered provider and status provider: Current main picks a primary provider for the unified icon at line 246, but the status indicator used by that render scans every enabled provider at lines 294-300 and returns the first issue, so a non-rendered provider can drive the marker. (Sources/CodexBar/StatusItemController+Animation.swift:246, 05545feba362)
  • Current main observes aggregate merged status: In merged-icon mode, current main signs the primary provider plus a separate mergedStatus value, and mergedIconStatusIndicator scans all enabled providers at lines 64-70. (Sources/CodexBar/StatusItemController+IconObservation.swift:14, 05545feba362)
  • PR scopes render and observation paths: The PR replaces the aggregate status scan with store.statusIndicator(for: primaryProvider) and removes the mergedStatus helper from the merged observation signature. (Sources/CodexBar/StatusItemController+Animation.swift:291, b54e9326ef75)
  • PR adds focused regression coverage: The diff adds tests for provider-scoped merged status rendering, observation signatures ignoring non-primary status changes, and halo/glyph pixels for minor and major indicators. (Tests/CodexBarTests/StatusItemAnimationSignatureTests.swift:278, b54e9326ef75)
  • Real behavior proof is partial: I downloaded and inspected both PR screenshots; they show the status marker halo/readability, but they do not show a real merged-icon setup where an unrelated provider status no longer changes the visible marker.

Likely related people:

  • hhh2210: Authored the merged icon-observation reduction merged at Reduce merged icon observation churn #1297, which introduced the current primary-plus-aggregate status signature surface this PR changes. (role: feature-history owner; confidence: high; commits: de55f4850b8d, 7272928e1fae, fa267cb438ec; files: Sources/CodexBar/StatusItemController+IconObservation.swift, Sources/CodexBar/StatusItemController+Animation.swift, Tests/CodexBarTests/StatusItemIconObservationSignatureTests.swift)
  • steipete: Recent main history shows icon projection and test-state work in the same controller/signature area, and the shallow local blame attributes the current release snapshot of these lines to Peter Steinberger. (role: recent area contributor; confidence: medium; commits: 3cd3dd6d5900, c9bf66667417, 06fea2c897cc; files: Sources/CodexBar/StatusItemController+Animation.swift, Sources/CodexBar/StatusItemController+IconObservation.swift, Tests/CodexBarTests/StatusItemAnimationSignatureTests.swift)
  • kiranmagic7: Recently changed merged-icon redraw behavior and related animation-signature tests in Defer merged icon redraws during menu tracking #1409, adjacent to this PR's render-signature path. (role: adjacent merged-icon contributor; confidence: medium; commits: b7772b240a4e, b8b5257ac35a; files: Sources/CodexBar/StatusItemController+Animation.swift, Tests/CodexBarTests/StatusItemAnimationSignatureTests.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.

@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jun 17, 2026
@steipete
steipete force-pushed the codex/scope-provider-status-indicators branch from b54e932 to ce4fc10 Compare June 18, 2026 09:36
@steipete
steipete merged commit b9a3cef into steipete:main Jun 18, 2026
4 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: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants