Skip to content

Remove lifecycle keepalive window - #3003

Merged
steipete merged 1 commit into
mainfrom
fix/remove-keepalive-window
Aug 17, 2026
Merged

Remove lifecycle keepalive window#3003
steipete merged 1 commit into
mainfrom
fix/remove-keepalive-window

Conversation

@steipete

Copy link
Copy Markdown
Owner

Summary

  • remove the hidden CodexBarLifecycleKeepalive SwiftUI scene and its inert WindowServer footprint
  • replace the notification relay with AppKit Settings dispatch: showSettingsWindow: first, then legacy showPreferencesWindow:
  • remove keepalive-specific Dock policy and tests, and add focused selector-order/failure coverage

Bundle proof

  • ./Scripts/compile_and_run.sh built the production bundle, signed it with Peter's Developer ID, validated the nested signatures and packaged resources, relaunched it, and reported OK: CodexBar is running.
  • CGWindowListCopyWindowInfo showed the 880×620 Settings window and status-bar windows, with no window named CodexBarLifecycleKeepalive.
  • Peekaboo first captured the target display and found the visible codexbar-merged item at (1746, 0), size 34×30, inside the 2424×1476 display. Clicking CodexBar's Settings row made the 880×620 About Settings window key/frontmost, with the rebuilt sidebar and content intact.
  • The Settings opener tests verify modern-selector-first dispatch, legacy fallback, and explicit failure when neither action is handled.

CodexBar Settings without lifecycle keepalive

Validation

  • swift test --filter SettingsWindowOpeningTests — 3 passed
  • swift test --filter DockIconPolicyDecisionTests — 6 passed
  • make check — passed, including format/lint and architecture/package gates
  • autoreview — clean, no accepted/actionable findings
  • source-blind behavior contract — passed launch, menu-to-Settings, complete sidebar/content, process liveness, and no-keepalive-window clauses

make test was also run. The repository wrapper reached group 13 and stopped on reproducible, unrelated fake-Claude subprocess failures: app runtime auto prefers CLI before web when OAuth unavailable, auto cli usage retries loading panel before stale web fallback, and auto cli usage retries timeout when cli is final source. Their fake claude auth status --json children time out after five seconds on this host; this branch changes no Claude or subprocess code. A supplemental full built-product run skipped the first exact known failure, exercised the remaining repository through completion, and isolated the latter two as the only residual failures.

@clawsweeper

clawsweeper Bot commented Aug 17, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. 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. labels Aug 17, 2026
@clawsweeper

clawsweeper Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 17, 2026, 2:59 AM ET / 06:59 UTC.

ClawSweeper review

What this changes

The PR removes CodexBar’s invisible lifecycle keepalive window and opens Settings through a modern AppKit action with a legacy fallback.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open for normal owner review: the focused lifecycle cleanup has strong real-app evidence and no actionable correctness finding.

Priority: P2
Reviewed head: 76a07af8f5f3f1516977e4cf933ee7956157656a

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) The focused patch has direct native-app proof, selector-order coverage, and no identified correctness defect.
Proof confidence 🦞 diamond lobster (5/6) ✨ media proof bonus Sufficient (screenshot): The PR body supplies a signed-bundle launch, WindowServer verification that the keepalive window is gone, and a menu-to-Settings interaction with a screenshot.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (screenshot): The PR body supplies a signed-bundle launch, WindowServer verification that the keepalive window is gone, and a menu-to-Settings interaction with a screenshot.
Evidence reviewed 5 items PR implementation: The diff removes the hidden lifecycle scene and notification relay, adds ordered AppKit selector dispatch, and replaces keepalive tests with selector-order and failure coverage.
Current integration path: Current main registers the real Settings window with Dock policy, so the visible Settings window remains the lifecycle owner after the hidden window is removed.
Feature history: Recent commits cover Dock activation, minimized Settings, and lifecycle-window behavior, showing an actively maintained compatibility path.
Findings None None.
Security None None.

How this fits together

CodexBar’s status-menu actions select a settings pane, activate the app, and present its SwiftUI Settings scene. This change removes the hidden relay window and directly dispatches to the visible Settings window while preserving Dock policy preparation.

flowchart LR
A[Status menu] --> B[Select settings pane]
B --> C[Activate CodexBar]
C --> D[Prepare Dock policy]
D --> E[Modern Settings action]
E --> F[Legacy action fallback]
F --> G[Visible Settings window]
Loading

Before merge

  • Complete next step (P2) - This owner-authored PR has adequate real-behavior proof and no discrete repair finding; it remains for normal merge review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Lifecycle cleanup 89 added, 205 removed across 9 files The patch removes a hidden window and notification relay while adding narrowly scoped selector coverage.

Technical review

Best possible solution:

Keep one selector-based Settings-opening path with legacy fallback and retain focused coverage for menu opening, minimized Settings, and Dock activation.

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

Not applicable: this PR addresses an implementation-level lifecycle artifact and provides direct after-fix bundle and menu interaction evidence.

Is this the best way to solve the issue?

Yes: the visible Settings window already owns Dock registration, and direct selector dispatch removes the separate hidden relay without adding configuration or state.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P2: The patch repairs bounded but user-visible native Settings and window-lifecycle behavior.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies a signed-bundle launch, WindowServer verification that the keepalive window is gone, and a menu-to-Settings interaction with a screenshot.
  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body supplies a signed-bundle launch, WindowServer verification that the keepalive window is gone, and a menu-to-Settings interaction with a screenshot.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR body supplies a signed-bundle launch, WindowServer verification that the keepalive window is gone, and a menu-to-Settings interaction with a screenshot.

Label justifications:

  • P2: The patch repairs bounded but user-visible native Settings and window-lifecycle behavior.
  • 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 (screenshot): The PR body supplies a signed-bundle launch, WindowServer verification that the keepalive window is gone, and a menu-to-Settings interaction with a screenshot.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies a signed-bundle launch, WindowServer verification that the keepalive window is gone, and a menu-to-Settings interaction with a screenshot.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body supplies a signed-bundle launch, WindowServer verification that the keepalive window is gone, and a menu-to-Settings interaction with a screenshot.

Evidence

What I checked:

  • PR implementation: The diff removes the hidden lifecycle scene and notification relay, adds ordered AppKit selector dispatch, and replaces keepalive tests with selector-order and failure coverage. (Sources/CodexBar/SettingsWindowOpener.swift:1, 76a07af8f5f3)
  • Current integration path: Current main registers the real Settings window with Dock policy, so the visible Settings window remains the lifecycle owner after the hidden window is removed. (Sources/CodexBar/PreferencesView.swift:370, 09d2d0140d61)
  • Feature history: Recent commits cover Dock activation, minimized Settings, and lifecycle-window behavior, showing an actively maintained compatibility path. (Sources/CodexBar/DockIconController.swift:111, 26c612e935ba)
  • Release/main provenance: The prior lifecycle-window adjustment is on current main but is not contained by a local release tag; this PR is therefore evaluated as an unmerged current branch change. (Sources/CodexBar/CodexbarApp.swift:110, 26c612e935ba)
  • Real behavior proof: The PR body records an after-fix signed-bundle launch, WindowServer check with no lifecycle window, and a successful menu-to-Settings interaction with a screenshot. (76a07af8f5f3)

Likely related people:

  • steipete: Authored the recent lifecycle-window, minimized-Settings, and Dock-policy work that this PR consolidates. (role: feature owner; confidence: high; commits: 26c612e935ba, 6fdc0ac0308f, b20dc2374085; files: Sources/CodexBar/CodexbarApp.swift, Sources/CodexBar/DockIconController.swift, Sources/CodexBar/PreferencesView.swift)
  • KGBos: Recent history includes a focused Settings-window focus repair near the Dock-policy behavior. (role: recent adjacent contributor; confidence: medium; commits: 6697bf399325; files: Sources/CodexBar/DockIconController.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.

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

1 participant