fix(service): carry stable launchd launcher ownership (#3554) - #3616
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 72 / 80이 PR은 mise/asdf 같은 버전 관리자로 지금 다만 GitHub base는 테스트는
라인 없음 - base가 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59a1108055
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const state = deps.state === undefined ? readServiceInstallState() : deps.state; | ||
| if (state?.launcherPath) return buildServiceLauncherShellCommand(state.launcherPath, port); |
There was a problem hiding this comment.
Derive the launchd comparison from the current plist
When installation is interrupted or writeServiceInstallState() throws after launchctl load succeeds, installLaunchd() leaves both the live job and on-disk plist using the new launcher while the previous state still names an old launcher or none. expectedLaunchdCommand() then constructs a different command from that stale state, so the common already-loaded path in ocx service start reports an “OLDER plist” and refuses the benign start even though the live job matches the file; detailed status can make the same false claim. Parse the expected command from the current plist, or make the definition/state update atomic with rollback.
Useful? React with 👍 / 👎.
Summary
Carries the unique #3554 layer: launchd install/state/status agree on one recorded stable launcher, preserving file-backed token handling and proof-bound overrides. This repairs which version starts next, not an already-running old proxy.
Refs #3464; the issue remains open for the running-process mismatch portion. No live launchctl round-trip is claimed.
Stack (merge bottom-up): IPv6 → launchd → Codex toggle → canonical discovery. This layer targets codex/260905-residual-ipv6; retarget after its parent lands. These are child-only carries, not wholesale copies of the old parents.
Verification
Checklist