Skip to content

fix(startup): require RunAtLoad before Desktop relaunch - #7277

Open
luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:codex/cold-start-runatload-followup-20261017
Open

luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:codex/cold-start-runatload-followup-20261017

Conversation

@luvs01

@luvs01 luvs01 commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The cold-start caller could accept a matching macOS LaunchAgent without the required Start-at-Login RunAtLoad: true field, as described in the upstream discussion. Require that boolean before the lease-owning caller attempts relaunch; false or missing values return manual-open guidance without launch, readiness waiting or a competing CLI proxy, while releasing the lease. Both true modern [program, "--autostart"] and legacy [program] registrations remain accepted, and an already-running Desktop or a lease held by another caller retains its existing wait path.

Verification

GitHub's update-branch operation merged current dev into the PR branch. Signed publication HEAD: 031917f996a9a624ff68becaddf7f0bd1d362849; first parent: 6ee0dd7150448864f0834540caeae3e07f8ae3af; second parent (dev): 8640c2f30edbd2fbb6947ed5c25e2793f5ae72a6; tree: 0b87cfff23a39e1754b793f823755a25652b5a2e. GitHub reports signature verification valid, author luvs01, and GitHub as committer. The server tree exactly matches the locally validated merge candidate. Its delta against dev is 3 files, 25 insertions, 5 deletions, frozen patch SHA256 b71ea9c19a24997d62e081585091946b45f7a92238ab18f1680f33cea7da2c45. Source and test blobs are unchanged from the original PR; the runtime documentation also retains the current dev prose.

Fresh local validation of that exact merge tree on 2026-10-18:

  • Repository-pinned Bun 1.4.3: bun test tests/cli/cli-ensure-desktop-cold-start.test.ts --timeout 30000 — 30 passed, 7 skipped, 0 failed, 186 assertions, 42.33 seconds. Covers rejected false/missing RunAtLoad, accepted modern/legacy registrations, running Desktop waits, manual-open guidance and refusal of CLI fallback.
  • Node 24.18.0 / TypeScript 7.0.2: direct node node_modules/typescript/bin/tsc --noEmit — PASS, exit 0. The compiler was invoked directly because the reused dependency Bun shim previously failed; no helper was modified.
  • Direct pinned Bun 1.4.3 structure SSOT, privacy scan and file-size ratchet — PASS. Latest-dev-relative staged diff check — PASS.

Exact-head Cross-platform CI run 39097190022 on 031917f996a9a624ff68becaddf7f0bd1d362849 completed successfully at 2026-10-18 04:38:26 UTC: 14 successful jobs, 9 skipped, 0 failed, 0 cancelled, all 23 jobs completed. Skipped platform/native jobs are not passing coverage. The new-head React Doctor, Catalog async contracts, target-branch and label checks also completed successfully. PR hygiene run 39097188514 succeeded; the separate body-edit-triggered hygiene run 39097233018 was cancelled, not passed. These results do not claim every merge prerequisite or approval is complete.

The original-head Cross-platform CI run 39041846086 completed with 14 successful and 9 skipped jobs on 6ee0dd7150448864f0834540caeae3e07f8ae3af; that remains historical evidence only, separate from the refreshed-head CI above.

The full suite and changed-test wrapper were NOT RUN locally for this refreshed tree, under the focused-validation/shared-resource scope. Seven native/macOS/Linux-specific tests skipped on Windows remain UNVERIFIED; no real Desktop launch, native plutil, LaunchServices or service registration was exercised. Maintainer/code-owner approval and explicit security review remain pending before merge. Author validation is complete and this PR is ready for review. Maintainer/code-owner approval and explicit security review remain separate merge prerequisites; no approval, merge, release or deployment is claimed.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.
  • The signed publication HEAD/tree and exact-head hosted checks are recorded.
  • Maintainer/code-owner review is complete.
  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • macOS Desktop cold-start now proceeds only when the app is registered to launch at login. If that registration is missing or disabled, the app provides guidance to open Desktop manually instead of waiting for readiness or trying a command-line fallback.
    • Behavior for an already-running Desktop app is unchanged.

Require a valid Start-at-Login RunAtLoad registration before the cold-start caller relaunches Desktop. Preserve modern and legacy argument forms, running-process waits and manual-open guidance, with focused regression coverage and runtime documentation.
@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: a8ba01f5-5c5d-4397-81b6-d377b43843a6
📥 Commits

Reviewing files that changed from the base of the PR and between 2a10a2b and 6ee0dd7.

📒 Files selected for processing: 3
  • src/cli/ensure-desktop-cold-start.ts
  • structure/runtime.md
  • tests/cli/cli-ensure-desktop-cold-start.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The Desktop cold-start check now requires the recorded macOS LaunchAgent registration to set RunAtLoad to true. Tests cover registrations where the value is false or missing, and the runtime documentation describes the requirement.

Changes: Desktop cold start

Layer / File(s) Summary
Validate LaunchAgent registration
src/cli/ensure-desktop-cold-start.ts, tests/cli/cli-ensure-desktop-cold-start.test.ts, structure/runtime.md
The registration check rejects LaunchAgents unless RunAtLoad is exactly true. Tests verify the refusal behavior when the value is false or missing. The runtime documentation records the requirement.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6ee0d

No merge-blocking issue is established in the reviewed change. Native macOS and live LaunchServices behavior were not exercised, but that validation gap alone does not indicate a current defect.

Pre-merge checks: Passed 5
Check name Status Explanation
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title accurately and concisely describes the main change: requiring RunAtLoad before relaunching Desktop during startup.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 17, 2026
@luvs01
luvs01 marked this pull request as ready for review October 17, 2026 15:04
@Ingwannu

Copy link
Copy Markdown
Owner

Code/test review at 031917f996: the additional strict RunAtLoad:true predicate matches the relaunch caller's existing consent contract, with false/missing refusal before open/wait/fallback. Both legacy and modern accepted argv shapes and already-running Desktop behavior remain covered. I verified the exact-head Ubuntu CI receipt; the author explicitly distinguishes mocked registration/probe evidence from actual native macOS launch behavior. No correctness blocker found in this narrow static pass. The description still records explicit boundary/code-owner review as pending; I have not supplied a new scan or that separate clearance, so I am not converting this comment into merge approval.

This branch has not been deployed

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants