feat(adapter): add coven adapter login codex with OAuth port preflight#271
Closed
BunsDev wants to merge 1 commit into
Closed
feat(adapter): add coven adapter login codex with OAuth port preflight#271BunsDev wants to merge 1 commit into
coven adapter login codex with OAuth port preflight#271BunsDev wants to merge 1 commit into
Conversation
When Codex's `codex login` flow gets killed mid-OAuth (esc, SIGINT, browser close, network blip) the local callback server on port 1455 sometimes leaks as an orphan process. The next `codex login` attempt then crashes with: Codex OAuth failed: Failed to bind port 1455: Address already in use (os error 98/48) Upstream Codex has no auto-recover or fallback-port logic. Until that gets fixed in @openai/codex itself, this shim handles it from our side. ## What New subcommand: `coven adapter login <adapter>`. Today the only adapter that needs special handling is `codex`. Pattern: 1. Try to bind 127.0.0.1:1455 ourselves 2. If `AddrInUse`, identify the holder via `lsof -ti tcp:1455` 3. Use sysinfo to fetch the holder's process descriptor 4. **Only kill it if its argv/path clearly contains the token `codex`** (split on whitespace + path separators — won't match substrings like "vscodex" or "not-codex-tool") 5. SIGTERM (1s grace), then SIGKILL if still alive 6. Wait up to 1.5s for the port to free up 7. Exec `codex login` If we can't ID the holder (no lsof, ambiguous), or if it's clearly NOT codex, we **refuse to kill** and print a clear instruction. ## Why this shape and not the alternatives - **Why not auto-run in `coven doctor`?** Because users hit this error mid-frustration trying to log in. They don't want "go run a separate doctor command first" — they want one command that just works. - **Why not change which port Codex uses?** We can't; it's hard-coded upstream. - **Why `coven adapter login codex` and not `coven codex login`?** Reuses the existing `coven adapter ...` namespace (alongside `adapter list`, `adapter doctor`, `adapter install`). ## Tests New module `crates/coven-cli/src/codex_login.rs` with 7 unit tests: - `port_is_free` correctly identifies free vs held ports - `descriptor_looks_like_codex` recognizes codex/codex.js/@openai/codex - `descriptor_looks_like_codex` rejects vscodex, not-codex-tool, etc. - `pid_holding_port` returns None when port is free (also covers lsof-missing case — same observable result) - `preflight_codex_oauth_port` returns PortFree when 1455 is unbound (auto-skips if 1455 is occupied on the test machine — no flake) ## Manual verification - `coven adapter login --help` shows the new subcommand - Preflight with port FREE: correctly reports "free" and exec's codex - Preflight with port HELD by python (non-codex): correctly refuses to kill, prints lsof instructions ## Not in scope - A shim for `claude login` — Anthropic CLI doesn't have this failure mode (uses bearer-token paste, no local callback server) - Surfacing the preflight in cave's UI — separate followup, this PR just ships the CLI handler Closes the recurring "codex OAuth port held" pain we hit whenever a `codex login` flow is killed mid-auth.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new CLI entrypoint to make Codex authentication more resilient by preflighting the fixed OAuth callback port (1455) and optionally clearing stale Codex listeners before running the vendor login flow.
Changes:
- Introduces
coven adapter login <adapter>subcommand, currently implemented forcodex. - Adds a Codex-specific preflight module that detects port 1455 conflicts, identifies the holding PID (via
lsof), and conditionally terminates it using a conservative “looks like codex” descriptor heuristic. - Adds unit tests for the port check and descriptor heuristic behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/coven-cli/src/main.rs | Wires new adapter login subcommand and runs the Codex login wrapper flow. |
| crates/coven-cli/src/codex_login.rs | Implements port-1455 preflight + conservative PID identification/termination logic with unit tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1004
to
+1009
| let status = Command::new("codex") | ||
| .arg("login") | ||
| .status() | ||
| .with_context(|| { | ||
| "failed to exec `codex login`. Install Codex with `npm install -g @openai/codex`." | ||
| })?; |
Comment on lines
+17
to
+18
| //! 3. Wait for the port to actually free up (poll up to ~1.5s), then exec | ||
| //! `codex login`. |
Comment on lines
+60
to
+62
| // Other errors (permission denied, etc) — treat as not free so we | ||
| // surface them later when we re-bind for real. | ||
| Err(_) => false, |
| /// Heuristic: does this descriptor look like a `codex` OAuth helper we can | ||
| /// safely terminate? We require the literal `codex` token to appear as a | ||
| /// path component or argv element. We deliberately do NOT match substrings | ||
| /// like `vscode` or `markdownify` that happen to contain "codex". |
Member
Author
|
Closing — moving the fix to coven-cave's UI instead of the CLI per Val's direction. Will reopen here later if the CLI shim is also wanted. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the recurring "Failed to bind port 1455" error you hit whenever a
codex loginflow gets killed mid-OAuth and leaks an orphan listener on the callback port.The problem
codex loginopens a local OAuth callback server on port 1455. If the flow gets killed mid-auth (esc, SIGINT, browser closed, network blip) the listener sometimes leaks as an orphan process. The nextcodex loginattempt then crashes with:(
os error 48on macOS,os error 98on Linux — same condition.)Upstream Codex has no auto-recover or fallback-port logic, so until that lands in
@openai/codexitself, this shim handles it from our side.The fix
New subcommand:
coven adapter login <adapter>. Today the only adapter that needs preflight iscodex. Behavior:127.0.0.1:1455ourselvesAddrInUse, identify the holding pid vialsof -ti tcp:1455sysinfoto fetch the holder's process descriptor (name + argv)codex— split on whitespace and path separators, sovscodex,not-codex-tool, etc. don't matchexec codex loginIf we can't ID the holder (no lsof, ambiguous results), or if it's clearly NOT codex, we refuse to kill and print a clear instruction:
Why this shape
coven doctor— users hit this error mid-frustration trying to log in. They don't want "go run a separate doctor command first". One command that just works.coven adapter login codexnotcoven codex login— reuses the existingcoven adapter ...namespace (list/doctor/install/login).codexwould have false positives (vscodex,@openai/codex-darwin-arm64). Token-level matching is bulletproof and explicit.Tests
New module
crates/coven-cli/src/codex_login.rswith 7 unit tests, all passing:port_is_freecorrectly identifies free vs held ports (uses ephemeral port to avoid flakes)descriptor_looks_like_codexrecognizescodex,codex.js,@openai/codexdescriptor_looks_like_codexrejectsvscodex,not-codex-tool, empty stringsdescriptor_looks_like_codexhandles path separators (/usr/local/bin/codex)pid_holding_portreturns None when port is free (also covers lsof-missing case)preflight_codex_oauth_portreturnsPortFreewhen 1455 is unbound (auto-skips if 1455 is occupied — no flake)Plus full crate test suite: 856 unit + 4 + 11 = 871 tests, all green. No regressions.
Manual verification
coven adapter login --helpshows the new subcommandcargo clippy -- -D warningscleancargo fmt --checkcleanWhat it looks like in practice
Not in scope (separate follow-ups)
claude login— Anthropic CLI doesn't have this failure mode (uses bearer-token paste, no local callback server)Once this lands, you can rebuild + install with the usual
cargo install --path crates/coven-clior wait for the next coven release stamp.