Skip to content

Refresh custom Account labels after account changes - #2362

Merged
steipete merged 1 commit into
steipete:mainfrom
kiranmagic7:kiran/refresh-custom-account-label-20260721
Jul 28, 2026
Merged

Refresh custom Account labels after account changes#2362
steipete merged 1 commit into
steipete:mainfrom
kiranmagic7:kiran/refresh-custom-account-label-20260721

Conversation

@kiranmagic7

@kiranmagic7 kiranmagic7 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • refresh stored custom layouts when the rendered Account value changes
  • keep account-only snapshot churn ignored when Account is absent or personal info is hidden
  • hash the displayed label in the observation signature so it does not retain account text

Root cause

Custom Account tokens render from the snapshot account email, but icon observation intentionally ignored email-only snapshot churn. A stored layout containing Account could therefore keep showing the previous label until another visual field changed.

Verification

  • regression test failed before the fix because the before/after signatures were identical
  • swift test --filter StatusItemIconObservationSignatureTests — 20 tests passed
  • swift test --skip-build --no-parallel --filter '(MenuBarLayoutRendererTests|StatusItemIconObservationSignatureTests)' — 29 tests passed
  • make check — passed, including SwiftFormat and SwiftLint with zero violations
  • git diff --check — passed

Exact-head runtime proof

I packaged and ran a fresh debug bundle from 5f9cffffbaf40388b90300d9ba1250b6d662cb78 (CodexGitCommit=5f9cffff). The app used a stored custom layout containing only Account, with personal-info hiding off. A local fixture exercised CodexBar's supported Amp CLI provider path and kept usage constant at 20% used while changing only the synthetic account address. The same app process remained running, and I selected Refresh from its status menu after the identity change.

Redacted terminal/live-capture transcript:

bundle_head=5f9cffffbaf40388b90300d9ba1250b6d662cb78
layout_tokens=Account
process_restart_between_captures=no
usage_before=20.0% used
usage_after=20.0% used

before_account=proof-alpha@example.invalid
before_status_item_frame={x:889, y:4, width:201, height:24}
before_capture_ocr="proof-alpha@example.invalid D"
before_capture_sha256=d5aff685e9210f88fda4e98fe1c4fd4bf350fd62f123b667b06c76deb51f20ad

after_refresh_account=proof-beta@example.invalid
after_status_item_frame={x:857, y:4, width:195, height:24}
after_capture_ocr="proof-beta@example.invalid D"
after_capture_sha256=649dca42978002869181260ec4a71bf09f4fdb493827fd39e14fd54eab315ce0

The D suffix is CodexBar's debug-build marker. OCR was run locally against tight status-item captures with macOS Vision. No production account data or credentials were used.

make test was also run. It stopped in group 4 after both the initial run and automatic retry because two untouched AdaptiveRefreshTimerTests hit their 30-second wait deadline:

  • menu open advances a long idle timer during refresh without postponing an earlier tick
  • coding activity advances a long idle timer without postponing an earlier tick

No adaptive-refresh source or test files are changed here; hosted CI can provide clean-environment confirmation.

Risk

Low. Only stored custom layouts containing Account gain an additional render trigger. Other layouts and privacy-hidden account labels keep the previous observation behavior.

Review focus

Please sanity-check the render-aware observation boundary and the account-text hashing.

@clawsweeper clawsweeper Bot added 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 Jul 21, 2026
@clawsweeper

clawsweeper Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 25, 2026, 11:07 AM ET / 15:07 UTC.

ClawSweeper review

What this changes

This PR updates the menu-bar observation signature so stored custom layouts refresh when their displayed Account label changes, while keeping raw account text out of that signature.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

This PR remains necessary: it targets a bounded custom-layout refresh bug, and the contributor now provides focused tests, green hosted checks, and redacted same-process runtime proof. The requested read-only repository inspection was blocked by the sandbox before I could independently compare the exact head with current main, so this should remain open for normal source review rather than be auto-closed.

Priority: P2
Reviewed head: 5f9cffffbaf40388b90300d9ba1250b6d662cb78

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The PR has targeted scope, focused regression coverage, green hosted checks, and directly relevant redacted runtime proof; independent source review remains outstanding because the review sandbox could not inspect the checkout.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The PR body includes redacted exact-head live output demonstrating the Account-only menu-bar label changes after Refresh in the same app process; it directly shows the requested behavior with synthetic account values.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body includes redacted exact-head live output demonstrating the Account-only menu-bar label changes after Refresh in the same app process; it directly shows the requested behavior with synthetic account values.
Evidence reviewed 5 items Focused implementation surface: The supplied PR diff changes the icon observation signature, centralizes account-label resolution used by layout rendering, and adds focused observation-signature coverage.
Real runtime proof: The PR body records an exact-head fresh debug bundle with an Account-only stored layout changing from one synthetic address to another after Refresh in the same process, while usage remains constant; the transcript is redacted and uses .invalid fixture addresses.
Validation status: The supplied GitHub context reports nine hosted checks passed, including lint, build, and two macOS Swift-test shards; the contributor also reports focused signature/layout tests and make check passing.
Findings None None.
Security None None.

How this fits together

CodexBar receives provider usage snapshots and derives an observation signature to decide whether its menu-bar status item needs redrawing. Stored custom layouts can render an Account token, so visible account-label changes must participate in redraw detection without retaining personal information in the signature.

flowchart LR
    A[Provider usage snapshot] --> B[Resolve visible account label]
    C[Stored custom layout] --> D[Check for Account token]
    B --> E[Privacy-preserving observation signature]
    D --> E
    E --> F[Status-item redraw decision]
    F --> G[Updated menu-bar label]
Loading

Before merge

  • Complete next step (P2) - No repair lane is needed: the branch already contains a bounded implementation and the remaining action is ordinary current-main review, not a separate mechanical fix.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 3 files affected; 75 added, 5 removed The change is limited to custom layout rendering, redraw observation, and focused regression tests.
Hosted validation 9 checks passed The supplied PR context reports green lint, build, and macOS test coverage for the exact PR head.

Technical review

Best possible solution:

Merge a narrowly reviewed version that keeps account-label resolution shared between rendering and observation, verifies the signature remains process-local and privacy-preserving, and preserves the focused hidden-label and Account-token regression coverage.

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

Yes, with medium confidence from the supplied focused tests and exact-head runtime transcript: configure an Account-only stored menu-bar layout, change only the provider account address, and select Refresh without restarting the app. I could not independently execute or inspect that path because the read-only sandbox failed before repository access.

Is this the best way to solve the issue?

Yes, provisionally. Including the rendered Account value as a privacy-preserving redraw input only when the stored layout displays Account is the narrowest apparent fix; normal source review should confirm that the hash is used only for process-local observation.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted exact-head live output demonstrating the Account-only menu-bar label changes after Refresh in the same app process; it directly shows the requested behavior with synthetic account values.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes redacted exact-head live output demonstrating the Account-only menu-bar label changes after Refresh in the same app process; it directly shows the requested behavior with synthetic account values.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: The PR repairs a visible menu-bar refresh defect with limited blast radius and no demonstrated data, security, or availability impact.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes redacted exact-head live output demonstrating the Account-only menu-bar label changes after Refresh in the same app process; it directly shows the requested behavior with synthetic account values.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted exact-head live output demonstrating the Account-only menu-bar label changes after Refresh in the same app process; it directly shows the requested behavior with synthetic account values.

Evidence

What I checked:

  • Focused implementation surface: The supplied PR diff changes the icon observation signature, centralizes account-label resolution used by layout rendering, and adds focused observation-signature coverage. (Sources/CodexBar/StatusItemController+IconObservation.swift:47, 5f9cffffbaf4)
  • Real runtime proof: The PR body records an exact-head fresh debug bundle with an Account-only stored layout changing from one synthetic address to another after Refresh in the same process, while usage remains constant; the transcript is redacted and uses .invalid fixture addresses. (5f9cffffbaf4)
  • Validation status: The supplied GitHub context reports nine hosted checks passed, including lint, build, and two macOS Swift-test shards; the contributor also reports focused signature/layout tests and make check passing. (Tests/CodexBarTests/StatusItemIconObservationSignatureTests.swift:86, 5f9cffffbaf4)
  • Inspection limitation: The attempted read-only local inspection failed before yielding repository output with bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted; therefore current-main source, history, and maintainer-note provenance could not be independently audited.
  • Repository policy applied: The supplied full repository policy directs focused Swift tests for this behavior, avoids live credential/Keychain probes without an explicit request, and expects fresh-bundle proof for UI/runtime changes; the contributor’s proof and focused tests align with that guidance. (AGENTS.md:1)

Likely related people:

  • unknown: The sandbox prevented the required git blame/log history pass; no contributor should be assigned as a current-main owner based solely on the proposed branch. (role: current-main ownership unresolved; confidence: low; files: Sources/CodexBar/StatusItemController+IconObservation.swift, Sources/CodexBar/StatusItemController+MenuBarLayout.swift, Tests/CodexBarTests/StatusItemIconObservationSignatureTests.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Perform the normal current-main source review of the observation-signature lifecycle before merge.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (1 earlier review cycle)
  • reviewed 2026-07-21T01:14:59.824Z sha 5f9cfff :: needs real behavior proof before merge. :: none

@kiranmagic7

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 22, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@kiranmagic7

Copy link
Copy Markdown
Contributor Author

Current state on head 5f9cffffbaf40388b90300d9ba1250b6d662cb78: all nine hosted checks are green, and the PR body includes the redacted runtime proof showing the Account-only menu bar layout refreshing from proof-alpha@example.invalid to proof-beta@example.invalid in the same app process.

The previous re-review request appears to have stopped at the review-start placeholder from July 22, so requesting a fresh pass on the current proof.

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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. labels Jul 25, 2026
@steipete
steipete merged commit 23b2a1c into steipete:main Jul 28, 2026
9 checks passed
@steipete

Copy link
Copy Markdown
Owner

Final maintainer review and isolated Codex autoreview were clean on exact head 5f9cffffbaf40388b90300d9ba1250b6d662cb78. The render-aware observation boundary is appropriately narrow: account churn invalidates a stored non-legacy layout only when it contains Account, while absent labels and personal-info hiding remain inert. Rendering and observation share the same privacy-aware label helper, and the internal signature retains only a process-local hash rather than raw account text.

Focused local proof passed all 20 StatusItemIconObservationSignatureTests, including visible custom Account changes, hidden-account stability, unrelated snapshot churn, merged-provider isolation, and existing cost/status invalidation behavior. The PR's exact-head same-process runtime fixture additionally demonstrated the rendered synthetic Account label updating after Refresh without changing usage.

Exact-head hosted CI was fully green, including lint, both macOS test shards, Linux x64/arm64, musl, security, and the aggregate gate: https://github.com/steipete/CodexBar/actions/runs/29792483862

Merged as 23b2a1c14e852131f4d3d42cb607dc12174fcb20. Thank you for tracing the stale-label boundary, keeping the privacy behavior intact, and providing unusually concrete runtime proof, @kiranmagic7!

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: 🐚 platinum hermit Good normal PR readiness with ordinary 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.

2 participants