Fix Settings window opening

[Zihao-Qi]

Summary

• replace the selector-based Settings opener with a retained AppKit Settings window controller
• route status-menu and application-menu Settings actions through the same typed presentation path
• coordinate Dock activation and presentation attempts with bounded, generation-safe completion/watchdog handling
• repair localized Settings application-menu commands with bounded convergence after language changes
• end status-menu tracking before presenting Settings so macOS does not restore the previously active app over the new window

Problem

After the Settings path introduced around #3003, opening Settings could fail silently or create the window behind the previously active application. The responder-chain dispatch did not prove that a Settings window was actually created or retained, and promoting the accessory app before the status menu finished tracking allowed AppKit to restore the prior foreground application after presentation.

Language changes could also temporarily leave a stale or duplicate Settings command while SwiftUI rebuilt the application menu.

Root cause

The old path treated selector dispatch as success even though window creation, ownership, activation, and menu-tracking completion are separate AppKit events. Settings and Sparkle presentation also shared an unbounded Dock-promotion path without an explicit completion signal.

Implementation

• retain one identified Settings window and reuse it while switching panes
• report presentation outcomes and resolve only the matching activation generation
• demote back to accessory mode only when no real application window still requires regular activation
• close and forget tracked status menus before requesting Settings presentation
• keep Settings menu repair scoped to the application menu, prefer native commands, and verify fallback repairs for a bounded number of main-loop turns
• resolve Sparkle presentation attempts from its update-cycle completion callback, with a bounded watchdog for hung cycles

Verification

• swift test --filter 'StatusItemControllerShutdownTests|SettingsApplicationMenuTests|DockIconPolicyDecisionTests|SettingsWindowControllerTests|AppDelegateTests' — 29 tests passed
• make check — formatting, repository checks, and strict SwiftLint passed with zero violations
• git diff --check upstream/main...HEAD
• fresh packaged Release app built, signed, passed resource smoke checks, relaunched, and stayed running

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 18, 2026, 12:03 AM ET / 04:03 UTC.

ClawSweeper review

What this changes

The PR adds a retained AppKit Settings window and menu/Dock coordination, but also removes the Overview spend summary, its tests, proof artifact, and unreleased changelog entry.

Merge readiness

⛔ Blocked until real behavior proof is added - 6 items remain

The Settings repair is blocked by an unrelated removal of the existing Overview spend feature and its release entry; the patch needs narrowing before it can merge.

Priority: P2
Reviewed head: 04b64cc4955360ede728e62b9970d083cde6fad5

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The Settings direction has targeted tests, but a blocking unrelated feature deletion and missing real behavior proof leave the PR unready to merge.
Proof confidence	🧂 unranked krab (1/6)	Needs real behavior proof before merge: The PR lists tests and a relaunch claim but supplies no inspectable after-fix native Settings evidence; after narrowing the patch, add redacted screenshots, recording, or runtime output showing the repaired flow. 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.
Patch quality	🦪 silver shellfish (2/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The PR lists tests and a relaunch claim but supplies no inspectable after-fix native Settings evidence; after narrowing the patch, add redacted screenshots, recording, or runtime output showing the repaired flow. 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	3 items	Unrelated feature deletion: The branch deletes the block that adds the Overview spend summary before provider rows, plus its implementation and tests; this is unrelated to opening Settings. Current-main feature record: Current main lists the Overview usage-and-spend feature as an unreleased addition, while this branch deletes that record. Patch scope: The branch affects 23 files and removes 400 Overview-associated lines, beyond the stated Settings-opening purpose.
Findings	1 actionable finding	[P1] Restore the unrelated Overview spend feature
Security	None	None.

How this fits together

CodexBar menu actions open Settings through a shared native-window path that coordinates app activation and Dock policy. Independently, the Overview menu renders usage and spend summaries from provider data.

flowchart LR
A[Status and app menus] --> B[Settings controller]
B --> C[Create or reuse Settings window]
C --> D[Dock activation policy]
D --> E[Visible Settings UI]
F[Provider usage data] --> G[Overview spend summary]
G --> H[Status menu]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The PR lists tests and a relaunch claim but supplies no inspectable after-fix native Settings evidence; after narrowing the patch, add redacted screenshots, recording, or runtime output showing the repaired flow. 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.
• Restore the unrelated Overview spend feature (P1) - This removes the existing spendProviders summary from every Overview menu, and the branch also deletes its implementation, tests, screenshot artifact, and unreleased changelog record. Nothing in the Settings-opening change replaces that UI, so merging would silently regress the Overview spend summary. Restore these unrelated paths or split an explicitly approved removal into its own PR.
• Resolve merge risk (P1) - Merging the branch would remove the existing Overview usage-and-spend summary and its regression coverage, a user-visible compatibility regression unrelated to Settings.
• Complete next step (P2) - The unrelated removal has a narrow mechanical repair, but contributor-provided real behavior proof remains separately required before merge.
• Improve patch quality - Restore the unrelated Overview spend implementation, tests, screenshot artifact, and changelog entry.
• Improve patch quality - Add redacted real-app proof that Settings opens from the status and application menus and remains frontmost; update the PR body to trigger re-review.

Findings

• [P1] Restore the unrelated Overview spend feature — Sources/CodexBar/StatusItemController+Menu.swift:581-582

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Changed surface	23 files; production +573/-293, tests +452/-308	The submitted diff is materially broader than the stated Settings-opening repair.
Unrelated removal	Overview-associated +0/-400	These removals disable an existing user-visible Overview spend summary rather than supporting Settings.

Merge-risk options

Maintainer options:

1. Narrow the branch to Settings (recommended)
   Restore the removed Overview spend feature, tests, proof artifact, and unreleased release-note entry, then re-run focused validation.
2. Accept an intentional removal
   Only merge the deletions if a maintainer explicitly approves removing the Overview spend feature and its release record as a separate product decision.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Restore all unrelated Overview spend and changelog deletions; retain only Settings-opening changes and update focused tests.

Technical review

Best possible solution:

Restore every unrelated Overview spend, screenshot, test, and release-note deletion, then validate the retained Settings flow in a freshly packaged app.

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

Source-reproducible: comparison with current main proves this patch removes the Overview spend summary; the original Settings-opening failure still lacks a live current-head trace.

Is this the best way to solve the issue?

No: a Settings fix should not remove the unrelated Overview spend surface; restore that feature and retain a focused native-window repair.

Full review comments:

• [P1] Restore the unrelated Overview spend feature — Sources/CodexBar/StatusItemController+Menu.swift:581-582
  This removes the existing spendProviders summary from every Overview menu, and the branch also deletes its implementation, tests, screenshot artifact, and unreleased changelog record. Nothing in the Settings-opening change replaces that UI, so merging would silently regress the Overview spend summary. Restore these unrelated paths or split an explicitly approved removal into its own PR.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

• P2: The branch would regress an existing user-visible Overview feature if merged.
• merge-risk: 🚨 compatibility: It changes native Settings activation behavior and removes an established Overview menu capability.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR lists tests and a relaunch claim but supplies no inspectable after-fix native Settings evidence; after narrowing the patch, add redacted screenshots, recording, or runtime output showing the repaired flow. 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

Acceptance criteria:

• [P1] swift test --filter 'OverviewSpendSummaryTests|StatusMenuOverviewSpendTests|SpendDashboardScreenshotRenderTests|SettingsWindowControllerTests|SettingsApplicationMenuTests'.
• [P1] make check.
• [P1] git diff --check 8c113bb...HEAD.

What I checked:

• Unrelated feature deletion: The branch deletes the block that adds the Overview spend summary before provider rows, plus its implementation and tests; this is unrelated to opening Settings. (Sources/CodexBar/StatusItemController+Menu.swift:581, 04b64cc49553)
• Current-main feature record: Current main lists the Overview usage-and-spend feature as an unreleased addition, while this branch deletes that record. (CHANGELOG.md:6, 8c113bb03221)
• Patch scope: The branch affects 23 files and removes 400 Overview-associated lines, beyond the stated Settings-opening purpose. (04b64cc49553)

Likely related people:

• steipete: The provided merged predecessor shows steipete authored the selector-based Settings lifecycle path this PR replaces. (role: related Settings lifecycle author; confidence: medium; commits: 4b4abd9467b5; files: Sources/CodexBar/CodexbarApp.swift, Sources/CodexBar/SettingsWindowOpener.swift)
• Chipagosfinest: Current main’s unreleased changelog attributes the removed Overview spend feature to Chipagosfinest. (role: Overview spend contributor; confidence: medium; files: Sources/CodexBar/StatusItemController+OverviewSpend.swift, Sources/CodexBar/StatusItemController+Menu.swift)

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-08-18T03:09:26.467Z sha ac9540d :: needs real behavior proof before merge. :: none