Skip to content

fix(service): bake the stable ocx launcher into the launchd plist - #3554

Closed
lidge-jun wants to merge 5 commits into
codex/3462-mihomo-ipv6-fakeipfrom
codex/3464-launchd-stable-launcher
Closed

fix(service): bake the stable ocx launcher into the launchd plist#3554
lidge-jun wants to merge 5 commits into
codex/3462-mihomo-ipv6-fakeipfrom
codex/3464-launchd-stable-launcher

Conversation

@lidge-jun

Copy link
Copy Markdown
Owner

Summary

After a mise upgrade the installed CLI was 2.42.0 while the launchd proxy kept serving 2.10.1-preview: buildPlist baked the package-local Bun + CLI pair, so launchd never saw the new package until someone restarted the service by hand (#3464). systemd already moved to stableLauncherEntry() for exactly this failure (#2898 / #2909, which explicitly left launchd untouched); this is the macOS counterpart.

  • buildPlist takes the same { launcher, runtime } deps as buildUnit. With a launcher the job execs that absolute path and omits the OCX_BUN_RUNTIME_* provenance pair, keeping only a proof-bound OPENCODEX_BUN_PATH override (same rule as systemd). Without one the plist is byte-identical to today, so source checkouts and existing tests are unaffected.
  • installLaunchd resolves the launcher once and passes the same value to the plist and to install state (launcherPath), so the existing launcher-aware stale-path diagnostic judges what launchd actually runs.
  • startLaunchd and serviceStatusReport compared the live job against the Bun + CLI command string; a launcher-backed job would have read as "launchd is running an OLDER plist" on every healthy ocx service start. New expectedLaunchdCommand(port) follows the recorded launcherPath (never re-walks PATH) and both callers now use the installed port.
  • Docs: lifecycle.md launcher paragraph generalised to macOS + Linux; structure/04 section renamed and extended.

Scope note — this does not close #3464. Launcher parity fixes which build the next launchd start runs. It cannot replace an already-running proxy after an external upgrade; that stays a restart / ocx service repair step (now stated in the docs) and the auto-repair-vs-refuse decision stays open on the issue.

Security review (MAINTAINERS.md): changes durable executable selection for the service. stableLauncherEntry is unchanged: absolute PATH entries only, regular file + X_OK, lexical (no symlink resolution), shell-quoted into the plist so /bin/sh -lc does not re-resolve it. The launcher's own #!/usr/bin/env node shebang still resolves node through the baked PATH — the same assumption systemd already makes. No credential or token handling change; the API token stays file-backed and read only by the service shell at start.

Stack (260905 bug triage, position 3 of 5; base is #3551's head — retarget to dev after the parents land):

# PR Layer Base
1 #3547 #3467 Google location classification dev
2 #3551 #3462 Mihomo IPv6 fake-ip discovery #3547
3 this #3464 launchd stable launcher #3551
4 #3542 #3522 spill write health
5 next #3406 truthful Codex toggle this head

Unit: devlog/_plan/260905_bug_triage_stack/ (000, 005, 030).

Verification

  • bun run typecheck — exit 0.
  • bun test tests/service/service.test.ts — 199 pass / 0 fail. RED-before: with src/service.ts stashed the new assertions fail (expectedLaunchdCommand missing, launcher plist tests fail); GREEN-after.
  • New coverage: launcher plist names the shim and bakes no versioned Bun/CLI/provenance; launcher: null keeps the legacy shape; proof-bound override preserved vs ambient dropped; metacharacter launcher path stays quoted and launchdListenPort still parses; expectedLaunchdCommand for recorded launcher / v1 legacy state / missing state (Bun + CLI fallback, no PATH walk).
  • bun run privacy:scan — passed.
  • Hosted CI on the exact head is the acceptance gate (no repository-wide local suite was run by maintainer instruction). Not exercised: a live launchctl round-trip on this machine.

Refs #3464

Checklist

jun added 4 commits September 5, 2026 08:31
After a mise/asdf upgrade the installed CLI was 2.42.0 while the launchd proxy kept
serving 2.10.1-preview: buildPlist baked the package-local Bun + CLI pair, so launchd
never saw the new package until someone restarted the service by hand (#3464). systemd
had already moved to stableLauncherEntry() for exactly this failure (#2898 / #2909);
launchd is the macOS counterpart.

- buildPlist takes the same { launcher, runtime } deps as buildUnit; with a launcher the
  job execs that path and omits the OCX_BUN_RUNTIME_* provenance pair, keeping only a
  proof-bound OPENCODEX_BUN_PATH override. Without one the plist is byte-identical.
- installLaunchd resolves the launcher once, passes the same value to the plist and to
  install state (launcherPath), so the stale-path diagnostic judges what launchd runs.
- startLaunchd and serviceStatusReport compared the live job against the Bun + CLI
  command; a launcher-backed job would have read as "an OLDER plist" on every healthy
  start. expectedLaunchdCommand(port) follows the recorded launcherPath (never re-walks
  PATH) and both callers use the installed port.

This fixes which build the next launchd start runs. It does not replace an already
running proxy after an external upgrade — that remains a restart / repair step, noted in
the docs — so the issue stays open for that half.

Refs #3464
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 4, 2026 23:36
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • ^dev$
  • ^preview$

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: a295e6c1-d016-48de-a04e-355ac0a6ce99

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 4, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 70 / 80

이 PR은 Linux systemd가 이미 쓰는 stableLauncherEntry() 경로를 macOS launchd plist에도 맞춰, mise/asdf 업그레이드 뒤에도 launchd가 패키지 트리 안의 옛 Bun+CLI 쌍이 아니라 PATH에 잡힌 안정 ocx 런처를 실행하게 합니다. #3464에서 재현된 증상(설치된 CLI는 2.42.0인데 launchd 프록시는 2.10.1-preview를 계속 씀)의 “다음에 뜨는 빌드” 절반을 고칩니다. buildPlistbuildUnit과 같이 { launcher, runtime }를 받고, 런처가 있으면 절대 경로만 exec하고 버전드 Bun/CLI·OCX_BUN_RUNTIME_* 출처 쌍은 빼며 proof-bound OPENCODEX_BUN_PATH만 남깁니다. installLaunchd는 같은 런처 값을 plist와 install state(launcherPath)에 같이 넣고, startLaunchd/serviceStatusReport가 예전 Bun+CLI 문자열과 비교하다 건강한 런처 잡을 “OLDER plist”로 오진하던 구멍을 expectedLaunchdCommand(port)로 막습니다. 문서(lifecycle.md, structure/04)도 macOS+Linux로 일반화했습니다. 다만 베이스가 dev가 아니라 #3551 헤드(codex/3462-mihomo-ipv6-fakeip)이고, 이미 떠 있는 프록시를 외부 업그레이드 직후 자동으로 갈아끼우지는 않아서 #3464를 Closes 하지 않습니다. 지금 dev 입장에서는 스택 부모(#3547#3551)가 먼저 들어와야 머지 가능한 서비스 수명주기 수정입니다.

라인 expectedLaunchdCommand / launchdStart - 기대 명령을 기록된 launcherPath에서만 읽고 PATH를 다시 걷지 않는 설계가 맞습니다. 테스트도 런처/레거시/state 없음 세 갈래를 고정했습니다. 머지 전에 hosted CI에서 tests/service/service.test.ts가 초록인지만 보면 됩니다.
경로 buildPlist launcher 모드 - 셸·XML 메타문자 경로 인용/이스케이프 테스트가 들어 있어 plist 주입 쪽 회귀가 있습니다. stableLauncherEntry 자체(절대 PATH, 일반 파일+X_OK, 심볼릭 미해석)는 이 PR에서 안 건드린다고 명시되어 있어, 보안 경계 변경은 “어디에 그 절대 경로를 굽느냐” 쪽에 가깝습니다.
경로 ServiceInstallState.launcherPath 주석 - “Linux only”를 launchd+systemd로 고친다고 본문에 적혀 있습니다. 실제 패치에 주석/타입 문서가 빠지지 않았는지 한 번 확인하세요.
경로 #3464 범위 - 이 PR은 “다음 start가 새 빌드를 고른다”까지만 닫고, 이미 로드된 잡의 자동 repair/거절은 이슈에 남깁니다. Refs #3464가 맞고 Closes는 이르습니다.
경로 스택 베이스 - base가 #3551이라 지금 dev에 단독 머지할 수 없습니다. #3547#3551 착지 후 dev로 리타겟이 전제입니다.

메인테이너의 판단이 필요한 지점

너의 추천
스택 부모(#3547, #3551)가 먼저 dev에 들어간 뒤 이 PR을 dev로 리타겟해 머지하세요. #3464는 닫지 말고, 이미 떠 있는 잡의 repair 정책은 별도 이슈/후속으로 두세요. 보안 리뷰는 “절대 런처 경로를 plist에 굽는 것”이 systemd와 동형인지 한 줄만 확인하면 됩니다.

이 댓글은 grok-bot이 작성했습니다

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The launchd design is consistent with the existing systemd launcher boundary on this stacked head. installLaunchd resolves one stable launcher and records the same path in both the plist and install state; start/status derive the expected command from that recorded path instead of re-walking PATH. Launcher mode drops stale package-local Bun provenance, preserves only a proof-bound Bun override, keeps the API token file-backed, and shell/XML quoting is covered. The scope also correctly leaves already-running proxy replacement open in #3464.

I am requesting changes because this executable-selection/security-boundary PR currently targets codex/3462-mihomo-ipv6-fakeip, whose parent chain includes blocked #3551 and #3547. It cannot receive a final security approval or be merged on a temporary stack base.

After the parents land, retarget/rebase this layer onto the then-current dev, ensure its final diff contains only the launchd/service/docs changes, and run exact-head CI including macOS. I will then re-review the final launcher command, install-state provenance, and live-status comparison. Do not merge it into the temporary parent branch.

lidge-jun added a commit that referenced this pull request Sep 5, 2026
Owner-authorized admin merge of the child-only stable launchd launcher carry. Final dev HEAD CI is the gate; no local tests and no live launchd mutation.
@lidge-jun

Copy link
Copy Markdown
Owner Author

Carried into dev by #3616 at 4e2246c, with original attribution retained and the concrete follow-up corrections described there. Closing the source PR as superseded. Final dev HEAD CI is still pending under the owner-authorized admin-merge workflow; this closure does not claim CI success.

@lidge-jun lidge-jun closed this Sep 5, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

Landed via #3616 at 4e2246c

@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 5, 2026
@lidge-jun
lidge-jun deleted the codex/3464-launchd-stable-launcher branch September 5, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants