Skip to content

Repair hidden status item defaults repeatedly#1306

Open
Thinkscape wants to merge 1 commit into
steipete:mainfrom
Thinkscape:fix/repeat-status-item-defaults-repair
Open

Repair hidden status item defaults repeatedly#1306
Thinkscape wants to merge 1 commit into
steipete:mainfrom
Thinkscape:fix/repeat-status-item-defaults-repair

Conversation

@Thinkscape
Copy link
Copy Markdown

@Thinkscape Thinkscape commented Jun 5, 2026

Why?

CodexBar randomly disappearing after updates, external screen disconnections, or when interacting with Bartender, is a major pain. Restarting the app would be a logical recovery method, but it currently doesn't work b/c the repair doesn't happen on repair. This PR fixes it.

Summary

  • remove the one-shot gate from hidden macOS status-item visibility defaults repair
  • keep the existing repair marker as historical state, but continue scanning current defaults on each startup
  • preserve the CodexBar-owned key filter so unrelated apps' NSStatusItem VisibleCC ... keys and true values are left intact

Related

Tests

  • swift test --filter StatusItemControllerSplitLifecycleTests

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented Jun 5, 2026

Codex review: needs real behavior proof before merge. Reviewed June 4, 2026, 10:07 PM ET / 02:07 UTC.

Summary
The PR removes the one-shot gate from macOS status-item visibility-defaults repair and updates focused status-item lifecycle tests to expect repeated cleanup.

Reproducibility: yes. From source inspection on the PR branch, set didRepairKey = true and NSStatusItem VisibleCC codexbar-codex = false, then call the repair helper; the codexbar- filter still matches and removes the key.

Review metrics: 1 noteworthy metric.

  • Patch surface: 2 files changed, 49 additions, 10 deletions. The diff is small, but it changes startup defaults repair behavior that affects persisted macOS menu-bar visibility settings.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until 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] Preserve intentional post-repair false NSStatusItem VisibleCC codexbar-* settings while still recovering stale legacy/default keys.
  • [P1] Add focused coverage for marker-existing false CodexBar-owned keys and repeated legacy/default-key repair.
  • [P1] Add redacted real launch/defaults-repair proof, such as terminal output, logs, or a short recording, after the patch is corrected.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The external PR lists a focused XCTest command but does not include redacted after-fix proof from a real macOS app startup/defaults-repair path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging this as written can delete a user's intentional false NSStatusItem VisibleCC codexbar-* setting after the one-time repair already ran, so CodexBar may reappear when the user expects it to stay hidden.
  • [P1] The PR has no redacted real behavior proof from a macOS startup/defaults-repair scenario; tests alone do not prove the app recovers in the reported real setup.

Maintainer options:

  1. Preserve post-repair hidden choices (recommended)
    Keep recovery for stale legacy/default keys, but avoid deleting post-marker codexbar-* = false values unless a narrower explicit recovery trigger is added and tested.
  2. Document an override policy
    Maintainers could intentionally allow repeated CodexBar-owned cleanup only if they decide visibility recovery should override macOS hide settings and add upgrade-facing tests/proof for that policy.
  3. Pause for a narrower repair
    If the compatibility distinction is too uncertain, pause this branch and replace it with a smaller repair focused on the concrete hidden-after-update defaults state.

Next step before merge

  • [P1] Human review is needed because the PR has a P1 user-settings compatibility bug and missing external real behavior proof that automation cannot supply for the contributor.

Security
Cleared: The diff only changes app defaults-repair logic and focused tests; I found no concrete security or supply-chain concern.

Review findings

  • [P1] Preserve post-repair hidden status-item choices — Sources/CodexBar/MenuBarStatusItemDefaultsRepair.swift:8
Review details

Best possible solution:

Keep startup recovery, but make repeated repair compatible by preserving deliberate post-migration CodexBar-owned hidden visibility defaults unless maintainers choose and prove an explicit recovery path.

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

Yes. From source inspection on the PR branch, set didRepairKey = true and NSStatusItem VisibleCC codexbar-codex = false, then call the repair helper; the codexbar- filter still matches and removes the key.

Is this the best way to solve the issue?

No. Removing the one-shot gate is a plausible recovery direction, but the maintainable fix must preserve deliberate post-repair hidden settings or use a narrower explicit recovery trigger.

Full review comments:

  • [P1] Preserve post-repair hidden status-item choices — Sources/CodexBar/MenuBarStatusItemDefaultsRepair.swift:8
    Because the repair marker no longer gates the scan, a user who hides CodexBar after the first repair will have NSStatusItem VisibleCC codexbar-* = false deleted on the next launch. Keep repeated repair away from post-marker CodexBar-owned hidden keys, or gate that override behind an explicit recovery path.
    Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 65e39f4dcb3a.

Label changes

Label changes:

  • add P1: The PR targets a real disappearing menu-bar status item problem, but the current patch can break existing user visibility preferences.
  • add merge-risk: 🚨 compatibility: The diff changes persisted macOS defaults repair semantics and can override an existing user's hidden CodexBar status-item setting after upgrade.

Label justifications:

  • P1: The PR targets a real disappearing menu-bar status item problem, but the current patch can break existing user visibility preferences.
  • merge-risk: 🚨 compatibility: The diff changes persisted macOS defaults repair semantics and can override an existing user's hidden CodexBar status-item setting after upgrade.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The external PR lists a focused XCTest command but does not include redacted after-fix proof from a real macOS app startup/defaults-repair path. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: The one-time hidden visibility defaults repair and adjacent status-item placement fixes were introduced in recent commits on the affected files. (role: introduced behavior and recent area owner; confidence: high; commits: 6665d028b2c7, 35cd513b07e2, 55c0a105002f; files: Sources/CodexBar/MenuBarStatusItemDefaultsRepair.swift, Sources/CodexBar/StatusItemController.swift, Tests/CodexBarTests/StatusItemControllerSplitLifecycleTests.swift)
  • hhh2210: A recent current-main commit touched StatusItemController.swift in the broader merged-menu/status-item area, though not the defaults repair helper itself. (role: recent adjacent contributor; confidence: low; commits: 65e39f4dcb3a; files: Sources/CodexBar/StatusItemController.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.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a8f23c4fc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +18 to +20
if !defaults.bool(forKey: self.didRepairKey) {
defaults.set(true, forKey: self.didRepairKey)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve post-repair hidden status-item choices

Because the repair marker is now only historical state and no longer gates the scan, any later false NSStatusItem VisibleCC codexbar-* value is deleted on the next startup. In the scenario where a user intentionally turns CodexBar off in macOS Menu Bar settings after the first repair, that hidden default matches the same filter and is undone, so the icon cannot stay hidden. Consider limiting repeated repair to legacy/default Item-* keys or to a narrower recovery trigger.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jun 5, 2026
@hhh2210
Copy link
Copy Markdown
Contributor

hhh2210 commented Jun 6, 2026

Opened a small stacked follow-up for the ClawSweeper P1 blocker here: Thinkscape#1

It keeps repeated cleanup for stale NSStatusItem VisibleCC Item-* = false keys, but preserves user-created codexbar-* = false visibility defaults after the repair marker already exists.

Validation run locally:

  • make check
  • swift test --filter StatusItemControllerSplitLifecycleTests
  • swift test
  • independent reviewer pass: no findings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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