fix(cli): let an explicit different --port start a sibling (rebase of #3144) - #3188
Conversation
… refusing #3106 made handleStart probe the configured port so a bare `ocx start` cannot shadow a healthy configured-port proxy with an ephemeral-port copy. But the refusal it added fires whenever ANY live proxy is found, ignoring an explicit `--port` that names a DIFFERENT port. That is not the shadow the guard targets: an interactive `ocx start --port X` with X free is an explicit sibling request. The blanket refusal broke two real things. With a proxy on the configured port, no second instance could be started on any other port at all. And every test that spawns the real launcher on a unique free port — shutdown-launcher's three signal cases — timed out its 20 s startup wait on any machine that runs an actual proxy, because the sandboxed home has no config.json, the probe fell through to the default port, and found the developer's real instance. That is how it surfaced: 0 pass / 3 fail on unmodified dev, reproducible only where a live proxy sits on 10100. The gate is deliberately narrow. The sibling path opens only when a live proxy exists AND --port was given explicitly AND it differs from the live proxy's port AND the caller is not the service wrapper. The wrapper always passes the configured port and keeps its exact stay-out-of-the-way semantics (#3106's endless-respawn fix); a bare start and a same-port start still refuse. The sibling start warns that this home's Codex config is re-pointed, which is the one side effect it shares with any start. Verified as a matrix on a machine with a real proxy on the configured port: bare start refuses, --port <same> refuses, --port <different> starts, and shutdown-launcher goes from 0 pass / 3 fail to green; reverting the gate turns it back. A source-level oracle pins the port comparison and the OCX_SERVICE exclusion next to #3106's own oracles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e text Review follow-up on the sibling-port fix. The source oracle pinned implementation text and could keep passing while handleStart refused the request, mis-parsed the port, or ignored OCX_SERVICE — and the integration proof in shutdown-launcher only exercises the sibling path on a machine that actually runs a proxy on the configured port, which CI does not. The decision is now a pure function, `decideStartWithLiveOwner`, exercised across the whole matrix at runtime: bare start refused, same-port refused, different-port sibling, service stay-out on both port shapes, and the exact-"1" sentinel so "0"/"false" cannot reach the stay-out path. The remaining source assertion pins only that handleStart routes through the shared decision. The sibling warning now also names the pid/runtime record takeover, not just the Codex config re-point — the shared-state model for same-home starts is unchanged from pre-#3106 and the message should say all of what that means. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The comparison the old oracle grepped for moved into decideStartWithLiveOwner, where the sentinel semantics run at runtime across the whole matrix. The oracle now pins what stayed in index.ts: the stay-out decision exits 0 and the conflict refusal keeps exit 1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1afbee0a16
ℹ️ 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".
| // Only the exact "1" sentinel is service context — the same check syncCleanup | ||
| // uses — so an env value like "0" or "false" cannot reach the stay-out path. | ||
| && input.ocxService !== "1"; | ||
| if (sibling) return "sibling"; |
There was a problem hiding this comment.
When ocx start --port <different> is used in the same OPENCODEX_HOME, this branch now admits a second process even though both processes retain the single-owner cleanup logic in handleStart: syncCleanup only PID-guards removal of ocx.pid and runtime-port.json, then unconditionally runs revertSystemEnv, restoreNativeCodex, and the Grok cleanup. Consequently, stopping either process removes routing needed by the other; stopping the newer process also leaves the older process undiscoverable because its state records were overwritten. Add multi-owner lifecycle state that restores the surviving owner, or keep refusing siblings until teardown can prove no sibling remains; the regression test should exercise startup followed by shutdown rather than only this pure decision.
AGENTS.md reference: src/AGENTS.md:L24-L25
Useful? React with 👍 / 👎.
| `Proxy already running on port ${owner.live.port}; starting a second instance on requested port ${requestedPort}. ` | ||
| + `The new instance takes over this home's pid/runtime records and Codex config while it runs.`, |
There was a problem hiding this comment.
This newly user-visible behavior directly contradicts docs-site/src/content/docs/reference/cli/lifecycle.md, which still states that ocx start “refuses to start a second live instance.” Update the canonical lifecycle page to document the explicit different-port exception and its shared PID/runtime/Codex-config effects, then keep the translated lifecycle pages consistent so users are not instructed according to the old refusal semantics.
AGENTS.md reference: src/AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
…ort (#3232) #3188 opened the sibling path: `ocx start --port X` beside a live proxy on the configured port starts a second instance instead of refusing. That instance went through chooseListenPort with X as its hard-pinned preference, so shouldPersistSelectedPort(config.port, X, X) was true and config.port was rewritten to X under the still-running configured-port proxy. Observed: a probe session ran `start --port 10198` a few times against the real home, exited, and left config.port=10198 behind. The next `ocx stop` + `ocx service` read config.port, baked `--port 10198` into the launchd plist, and re-pointed Codex's openai_base_url at 10198 -- the service silently moved off 10100 with no start on 10198 in sight. A sibling is a second instance, not a new home for this config, so it must never persist its port. handleStart now records the sibling decision and passes it into both chooseListenPort call sites (initial pick and the EADDRINUSE re-pick); shouldPersistSelectedPort returns false for it. The ordinary first-start persist and the fallback-port non-persist are unchanged. Co-authored-by: jun <jun@lidge.dev>
2 participants
Summary
Maintainer rebase of #3144 by @olddonkey onto current
dev— all three commits cherry-picked with author credit preserved, no conflicts.ocx start --port <n>refused to start when another proxy was already live, even when the requested port was different from the running one. An explicit, different port is an unambiguous request for a sibling instance, so it now starts one instead of refusing. The refusal is preserved where it belongs: same port, or no explicit port.The two follow-up commits are the author's own review response — moving the start-decision test from source-text matching to a runtime evaluation, and updating the
OCX_SERVICEexit oracle to match the routed decision. Both are improvements to how the behavior is proven, not to the behavior.Verification
Exact head
1afbee0a1:bun test ./tests/cli-dispatch.test.ts ./tests/cli-ready.test.ts— 91 pass, 0 fail, 306 expect() calls.Full-suite and typecheck coverage is left to CI on this exact head.
Checklist
dev