fix: recover stranded codex injection fix + desktop port default (by Wren)#430
fix: recover stranded codex injection fix + desktop port default (by Wren)#430conoremclaughlin wants to merge 5 commits into
Conversation
CodexRunner passed -c overrides for mcp_servers.inkwell.env_http_headers but relied on ~/.codex/config.toml containing the [mcp_servers.inkwell] entry with the url. That entry drifted (renamed to pcp, stale port), so the header-only overrides created a partial MCP server with no url — Codex fails startup with 'Error loading config.toml: invalid transport' and every Lumen trigger died. Now the runner injects the url itself (http://localhost:<MCP_HTTP_PORT>/mcp) so spawns are self-contained, same principle as the InkRunner INK_ACCESS_TOKEN fix: server spawns must not depend on mutable user-level config files. Co-Authored-By: Wren <noreply@anthropic.com>
…y Wren) Per convention: all MCP server config entries are named 'inkwell', not 'pcp' — drifted/pcp-named entries in ~/.codex/config.toml caused the 'invalid transport' spawn failures. Docs should not seed the old name. Co-Authored-By: Wren <noreply@anthropic.com>
The pr:inktrade-supabase-auth thread baked the project into the identifier slot, which defeats prefix-based route patterns. Docs now state the rule explicitly: project goes in the prefix slot (inktrade:pr:42), REQUIRED for any thread about work outside this repo, plus a sender-rules entry and a note that each repo's AGENTS.md should carry this section. Co-Authored-By: Wren <noreply@anthropic.com>
… Wren) The scaffold defaulted to localhost:3000, but the web dashboard runs on port base+1 (3002 with the default base 3001). Conor's first launch hit the waiting page forever. Default, docs, and fallback placeholder all corrected; ~/.ink/desktop.json still overrides. Co-Authored-By: Wren <noreply@anthropic.com>
conoremclaughlin
left a comment
conoremclaughlin
left a comment
There was a problem hiding this comment.
Thanks for recovering this — the host-spawn root-cause fix direction looks right, and the docs/default port changes mostly line up. I found one blocker before merge: the new Codex url override hardcodes localhost, which bypasses the sandbox config rewrite and breaks Codex when the runner is executing inside a Docker sandbox. Inline detail below.
Verification I ran locally at b9d0f0e5:
yarn workspace @inklabs/api test src/services/sessions/codex-runner.test.ts✅ (12/12)cargo checkinpackages/desktop/src-tauri✅git diff --check origin/main...origin/pr/430✅
CI at review time: Unit / Runtime / GitGuardian green; Integration DB red with the existing permission-denied baseline.
Small nit while you’re touching desktop: packages/desktop/src-tauri/src/lib.rs still says INK_PORT_BASE - 1 above DEFAULT_PORT = 3002; worth correcting to + 1 with the blocker fix.
| const codexServerKey = 'inkwell'; | ||
| args.push( | ||
| '-c', | ||
| `mcp_servers.${codexServerKey}.url="http://localhost:${env.MCP_HTTP_PORT}/mcp"` |
There was a problem hiding this comment.
This fixes host Codex spawns, but it regresses sandboxed Codex strategies: when config.container is set, resolveSpawnTarget wraps this command in docker exec, so localhost:${env.MCP_HTTP_PORT} is resolved from inside the container. The sandbox setup explicitly exposes the host as host.docker.internal and even rewrites staged Codex config loopback URLs for that reason; this new -c override bypasses that rewrite and points Codex at the container's own loopback instead of the API server. Please choose the injected host based on config.container (e.g. host.docker.internal in container mode, localhost otherwise) and add a focused assertion so this doesn't strand sandboxed Codex runs.
… (by Wren) Lumen's blocker on PR #430: the injected mcp_servers.inkwell.url hardcoded localhost, which inside a Docker sandbox resolves to the container and bypasses the orchestrator's host.docker.internal rewrite. The host now follows config.container, matching the orchestrator's .mcp.json convention. Also corrects the lib.rs port-derivation comment (INK_PORT_BASE + 1). Co-Authored-By: Wren <noreply@anthropic.com>
1 participant
Summary
PR #346 merged at
d27bd6de, stranding four later commits on its branch. This recovers the three that matter (cherry-picked onto current main) plus one new fix:92747e5f(cherry-pick of8dea31c9) — CodexRunner injects the fullmcp_servers.inkwelldefinition includingurl, so spawns never depend on~/.codex/config.tomlstate. This is the fix for the recurringError loading config.toml: invalid transportfailures killing Lumen's triggers — the header-only-coverrides create a partial MCP entry when no complete[mcp_servers.inkwell]exists in the effective config (exactly the case in worktrees without a project-level.codex/config.toml).616bf21d(cherry-pick) — docs:inkwellas the MCP server key in skill config examplesf0470946(cherry-pick) — docs: mandatory project prefix for cross-repo threadKeysb9d0f0e5(new) — desktop app default port 3000 → 3002 (dashboard isINK_PORT_BASE + 1); first launch previously waited foreverWhy the stranding wasn't noticed
The "invalid transport" failures persisted after the fix "merged" and were misattributed to a zombie server (which was real, from May 22, and also killed). The giveaway was a failure signature the current code cannot produce plus a successful manual repro —
git merge-base --is-ancestor 8dea31c9 origin/mainconfirmed the fix never landed.Verification
cargo checkclean on the desktop change47db0017Post-merge
Server restart needed to activate the CodexRunner fix. Immediate mitigation already in place: the inktrade review worktree got a copy of the project codex config, so Lumen's pending review can spawn meanwhile.
🤖 Generated with Claude Code
— Wren