Skip to content

fix(startup): launch Desktop by its registered app bundle path - #7121

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:codex/upstream-desktop-coldstart-20261015
Closed

luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:codex/upstream-desktop-coldstart-20261015

Conversation

@luvs01

@luvs01 luvs01 commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • ocx ensure now opens the Desktop bundle path derived from the installation's Start-at-Login registration, rather than asking LaunchServices to select a bundle by com.opencodex.desktop.
  • The durable Desktop ownership claim must match the app-local install ID. The registration's label, supported argument shape and optional Program must agree; its real executable must be opencodex-desktop under a .app/Contents/MacOS path, executable, and the bundle must declare the expected identifier.
  • Missing, malformed, stale or mismatched evidence refuses the launch and keeps the existing manual-open guidance. A launch failure or readiness timeout does not silently start a competing CLI proxy. Desktop-owned installs without a valid Start-at-Login registration also require manual opening.
  • The startup contract is documented. The test fixture explicitly accepts the existing ownership-resolution union without loosening production types.

The verified property is selection of the recorded bundle path. This change does not compare CFBundleExecutable with the canonical registered executable and does not claim exact executable identity or complete native-launch qualification.

Verification

Prepared tree: 905efa9b55b4cd1a97c81b1a354840e9979ee6c9, with 7640a942b35044c974e534b84d5a8049f4a8ae9a as the sole intended parent. Server-signed publication HEAD: fc7e1b0c4ccc2dd70020afefba0ad49a077dcea4.

Final code tree, after the type-only fixture correction:

  • bun --no-install test --isolate --timeout 30000 --preload "$GUARD" tests/cli/cli-ensure-desktop-cold-start.test.ts: 30 passed, 0 failed, 140 assertions.
  • Scoped strict test typing: the new fixture diagnostics are gone. The command still exits 1 for the unchanged scripts/test-temp-lock.ts:78 TS2339 (SyncSubprocess.signal); this is not a clean strict test-typing pass.

Earlier integration evidence, before that type-only fixture correction, with identical production and documentation files:

  • tests/cli/cli-ensure-readiness.test.ts and tests/codex-integration/codex-shim-ensure-failure.test.ts: 7 passed, 24 assertions.
  • tests/test-layout.test.ts: 2 passed, 3 assertions. tests/test-layout-tooling.test.ts: 25 passed, 565 assertions.
  • Root source typecheck, structure SSOT, ordinary privacy hygiene and file-size ratchet passed. The stricter CLI/Codex migration scan retains an unchanged stale-path comment in tests/codex-integration/multi-agent-keep-native-v1.test.ts:110; no real files were migrated.
  • The final base-to-tree whitespace check passes. Unchanged passing checks were not rerun merely for confidence. The 30 current-tree cases and 34 earlier adjacent/layout cases are reported separately, not as a newly run 64-case final-HEAD suite.

These were Linux/Bun 1.4.3 runs with matching existing dependencies, a scrubbed environment, isolated HOME/TMPDIR, repository test-home guard and a preload rejecting unmocked fetch. No unmocked fetch was attempted. This is fixture-level guarding, not an OS-level network isolation claim.

The full suite was deferred while multiple worktrees shared finite validation resources. Per the documented repository exception, focused regressions were run and broader coverage is left to exact-head hosted CI. The tests inject platform and process execution; no live macOS app or LaunchServices operation was performed. Native acceptance remains: the registered .app is selected, --autostart and background launch behavior work, the app-owned proxy becomes ready, another copy with the same bundle ID is not selected, and stale/mismatched registration refuses. Platform jobs skipped by ordinary PR CI do not establish those behaviors.

The existing review found no confirmed P1/P2 production defect for this tree, but did not rerun regressions. Explicit security-boundary review, maintainer/code-owner approval, hosted checks on this new signed HEAD and agreement on the remaining native validation scope are still pending. Keep this PR in Draft.

Contribution credit

Desktop path binding and POSIX fixture correction: Devin. App-bundle suffix refusal: luvs01.

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

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. Explicit maintainer security-boundary review remains pending.

Launch Desktop from the bundle path recorded in its login registration, with matching installation ownership and fail-closed manual-open guidance. Preserve the app-bundle suffix refusal and explicit ownership fixture types.

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • 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 15, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Carried this implementation into #7137 because maintainer edits are disabled on this branch. The carry retains source commit fc7e1b0 and credits both Epinephrine (@luvs01, numeric user ID 27862058) and Devin with Co-authored-by trailers.

Independent review confirmed the registered-bundle approach, and the carry completes its executable selection checks: the registration must be absolute, its target a regular executable, and Info.plist's main executable must match the canonical registered Desktop binary. Added regressions fail against this original head and pass with the follow-up; real macOS plist conversion is exercised without opening any app. Public recovery guidance is updated.

The carry's fresh focused/adjacent tests, typecheck, structure/privacy checks, layout/size guards, and docs build pass. All-platform CI is dispatched on its exact head: https://github.com/lidge-jun/opencodex/actions/runs/38698829593 . It stays draft pending broader CI/final review. Closing this original only as superseded by that open, credited carry; neither PR was merged.

@lidge-jun lidge-jun closed this Oct 15, 2026
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