Fix Launch at Login disable for pending approval

[AmrMohamad]

Summary

• Fix disabling Launch at Login when ServiceManagement reports .requiresApproval by unregistering every status except .notRegistered.
• Add focused tests around LaunchAtLoginManager status handling.
• Extracted from Add cold-start validation follow-ups #1234 per maintainer feedback after Fix cold-start Codex menu readiness #1231 already handled the original UI readiness issue.

Out of scope

• No duplicate-launch guard.
• No validation harness.
• No keychain prompt changes.
• No package or run script changes.

Verification

• swift test --filter LaunchAtLoginManagerTests
• git diff --check
• make check

Runtime proof

Packaged the PR build for local macOS validation and verified the staged app bundle:

• App bundle: .build/package/CodexBar.app
• Bundle identifier: com.steipete.codexbar
• Embedded commit: 644b4f12
• codesign --verify --deep --strict --verbose=2 .build/package/CodexBar.app passed

ServiceManagement status was measured inside the running packaged app with LLDB:

• Enum mapping on this host:
  • .notRegistered = SMAppServiceStatus(rawValue: 0)
  • .enabled = SMAppServiceStatus(rawValue: 1)
  • .requiresApproval = SMAppServiceStatus(rawValue: 2)
  • .notFound = SMAppServiceStatus(rawValue: 3)
• With launchAtLogin=true, launching .build/package/CodexBar.app registered the app and reported .enabled / rawValue: 1.
• System Settings > Login Items showed CodexBar.app under Open at Login.
• Removing CodexBar.app from Open at Login through System Settings changed the live status to .notFound / rawValue: 3 on this machine, not .requiresApproval.
• Re-launching the same packaged build with launchAtLogin=true registered it again and returned .enabled / rawValue: 1.
• Re-launching with launchAtLogin=false after UI removal did not produce a login-item error in the checked log slice.

I could not reproduce a real .requiresApproval state locally through the normal System Settings Open at Login flow; this host transitions through .enabled and .notFound for the available UI operations. The unit test still covers the .requiresApproval decision path directly, and the packaged run confirms the build/register/remove surface is wired to the real ServiceManagement API.

Notes from packaging:

• The normal release package command could not use the maintainer Developer ID identity on this machine: Developer ID Application: Peter Steinberger (Y5PE65HELJ): no identity found.
• Re-running with CODEXBAR_SIGNING=adhoc produced a signed staged bundle at .build/package/CodexBar.app; the final root CodexBar.app replacement failed because the existing root bundle has permission drift, so runtime proof used the verified staged bundle directly.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed June 12, 2026, 5:38 PM ET / 21:38 UTC.

Summary
This PR makes Launch at Login registration status-aware, unregisters pending-approval entries when disabled, and adds eight focused state-direction tests.

Reproducibility: Partly: source inspection and injected tests provide a high-confidence reproduction of the incorrect decision path, but the contributor could not create a real .requiresApproval state to observe removal in the packaged app.

Review metrics: 3 noteworthy metrics.

• Status coverage: 8 state-direction cases. The tests cover both preference directions across every known SMAppService.Status value.
• Patch surface: 3 files, 159 additions, 2 deletions. The behavioral change remains isolated to one manager and one focused test suite, plus release-note text.
• Prior-PR reduction: 3 files versus 12 files. The PR isolates the login-item behavior from the broader, closed validation follow-up at Add cold-start validation follow-ups #1234.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🦞 diamond lobster
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 terminal output, logs, or a recording showing a real .requiresApproval entry being disabled, or obtain an explicit maintainer proof override.
• Redact private identifiers, paths, endpoints, credentials, and other sensitive details from any posted proof.

Proof guidance:

• [P1] Needs stronger real behavior proof before merge: The packaged-app LLDB evidence demonstrates real ServiceManagement wiring for .enabled and .notFound, but it does not demonstrate the changed .requiresApproval disable path; add redacted terminal, log, or recording evidence, or obtain an explicit maintainer override. Updating the PR body should trigger a new review; otherwise ask a maintainer to comment @clawsweeper re-review. 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] The exact .requiresApproval disable operation remains unobserved against a real ServiceManagement entry, so the focused tests do not prove that macOS accepts and removes that pending registration as expected.
• [P1] The status matrix changes when existing login-item registrations are registered or removed; an incorrect OS-semantic assumption could leave an existing user's Launch at Login preference out of sync.

Maintainer options:

1. Accept a proof override (recommended)
   A maintainer can explicitly accept the focused state tests and packaged ServiceManagement wiring because the target status could not be induced on the available host.
2. Wait for pending-state evidence
   Keep the PR open until a packaged build shows that disabling Launch at Login successfully removes a real .requiresApproval registration.
3. Pause if OS semantics remain uncertain
   Pause the PR if maintainers cannot confirm that unregister() is supported for .requiresApproval, rather than shipping an unverified compatibility change.

Next step before merge

• [P1] The current head has no concrete code or test defect for an automated repair; the maintainer next action is to accept a proof override or require direct pending-approval runtime evidence.

Security
Cleared: The diff does not change dependencies, executable scripts, entitlements, permissions, credentials, downloaded artifacts, or other supply-chain surfaces.

Review details

Best possible solution:

Keep the focused status matrix and tests, then merge after a maintainer either verifies a real pending-approval removal with redacted runtime evidence or explicitly accepts the unavailable-state proof limitation.

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

Partly: source inspection and injected tests provide a high-confidence reproduction of the incorrect decision path, but the contributor could not create a real .requiresApproval state to observe removal in the packaged app.

Is this the best way to solve the issue?

Yes at the implementation level: a small status-aware decision seam is the narrowest maintainable correction, and the latest head avoids unnecessary .notFound unregister attempts. The remaining question is proof acceptance, not an identified code defect.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

• P2: This is a bounded user-facing settings bug with limited blast radius and no evidence of data loss, security impact, or application unavailability.
• merge-risk: 🚨 compatibility: The new state matrix changes registration and removal behavior for existing Launch at Login setups, while the target pending-approval operation lacks direct OS-level proof.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The packaged-app LLDB evidence demonstrates real ServiceManagement wiring for .enabled and .notFound, but it does not demonstrate the changed .requiresApproval disable path; add redacted terminal, log, or recording evidence, or obtain an explicit maintainer override. Updating the PR body should trigger a new review; otherwise ask a maintainer to comment @clawsweeper re-review. 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:

• Current-main behavior: Current main calls register() whenever enabled and unregister() whenever disabled without consulting SMAppService.status, so it does not distinguish a pending-approval registration. (Sources/CodexBar/LaunchAtLoginManager.swift:15, 1dfd7d90f3f1)
• Status-aware implementation: The current PR head treats .enabled and .requiresApproval as already registered when enabling, and unregisters both when disabling while leaving .notRegistered and .notFound alone. (Sources/CodexBar/LaunchAtLoginManager.swift:22, 358f617e5535)
• Focused regression coverage: Eight injected-action tests cover enabling and disabling across all four known SMAppService.Status values, including unregistering .requiresApproval and skipping .notFound during disable. (Tests/CodexBarTests/LaunchAtLoginManagerTests.swift:6, 358f617e5535)
• Maintainer refinement: The latest head commit by steipete completed the four-status state matrix after the earlier review, including removing the earlier attempt to unregister .notFound. (Sources/CodexBar/LaunchAtLoginManager.swift:39, 358f617e5535)
• Runtime-proof boundary: The PR body documents a packaged, ad-hoc-signed build exercising real registration and removal states, but the host produced .enabled and .notFound, not .requiresApproval; therefore the changed pending-approval removal path remains unobserved. (358f617e5535)
• Feature provenance: Git history attributes the current LaunchAtLoginManager implementation on main to the v0.34.0 release commit by Peter Steinberger, and the latest PR head refinement is also authored by steipete. (Sources/CodexBar/LaunchAtLoginManager.swift:4, 7717813b9eed)

Likely related people:

• steipete: Introduced the current-main Launch at Login manager in the v0.34.0 release history and authored the PR head’s complete status-handling refinement. (role: feature owner and recent area contributor; confidence: high; commits: 7717813b9eed, 358f617e5535; files: Sources/CodexBar/LaunchAtLoginManager.swift, Tests/CodexBarTests/LaunchAtLoginManagerTests.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.

[AmrMohamad]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/27434414578
• Updated: 2026-06-12T18:20:32.996Z

[steipete]

Maintainer fixup applied at exact head 358f617e553531959f158e1a3f6c798a966851f7.

The state handling now follows the ServiceManagement contract:

• Enabling registers .notRegistered and .notFound.
• Enabling leaves .enabled and .requiresApproval unchanged because both are already registered.
• Disabling unregisters .enabled and .requiresApproval.
• Disabling leaves .notRegistered and .notFound unchanged.

Verification:

• swiftformat Sources/CodexBar/LaunchAtLoginManager.swift Tests/CodexBarTests/LaunchAtLoginManagerTests.swift
• swift test --filter LaunchAtLoginManagerTests — 8 tests passed
• make check — SwiftFormat and SwiftLint clean
• swift test — 3,882 tests passed
• autoreview --mode branch --base origin/main — clean, no accepted/actionable findings

The branch was rebased onto current main; the changelog credits @AmrMohamad.