Repository navigation
fix(codex): read the pane's Codex home as Codex does - #1750
Merged
Merged
Conversation
Use the first CODEX_HOME and HOME entries, matching getenv. Resolve relative CODEX_HOME against the Codex process cwd on Linux, returning null when it cannot be read. Cover the Linux environment and cwd cases with regression tests. Refs #1493
Traverse CODEX_HOME through the process cwd or root without lexical normalization. Open the directory before canonicalizing its proc fd to avoid Bun realpath collapsing symlink parent components. Match Codex's existing-directory requirement for absolute and relative values, with deterministic regressions. Refs #1493
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description follows the required Change and Validation sections. It explains the behavior change, references the related issue, identifies affected files, and reports validation results. The repor… |
| Title check | |
The title clearly and concisely identifies the primary change: matching Codex home discovery behavior for a pane's Codex process. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 … |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Description check
-
Explanation
The description follows the required Change and Validation sections. It explains the behavior change, references the related issue, identifies affected files, and reports validation results. The reported test count differs from the objective summary, but this is non-critical.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Owner
Author
|
Merge-pass review (head 52b43ad) by a gate reviewer on a different model than the author's. Not merging yet, because one case breaks the PR's own rule. Passing:
Please fix before merge:
When that's in and the checks are green, this goes back in the merge queue. |
Owner
Author
|
Fixed the merge-pass blocker in commit Added a Linux regression using a live Verification:
Only @coderabbitai review |
|
Owner
Author
|
Merge pass: reviewed instead of a CodeRabbit review (rate-limited). The first gate review (on a different model from the author's) found one blocker: a live Codex whose cwd had been removed returned
|
This was referenced Oct 26, 2026
devswha
added a commit
that referenced
this pull request
Oct 26, 2026
## Problem Typing `codex` in a Herdr terminal and opening Chat before the first message said "Conversation unavailable", while a Codex started from New workspace, New tab or New worktree showed its greeting (#990). Codex writes no session before the first message (0.163 makes no `state_5.sqlite` at all until then), and herdr reports none, so nothing tied the pane to a transcript. Fixes #1191 ## Fix #990 left terminal launches out because they may resume a conversation. Chat now greets one only when every fact says it has begun none, and otherwise keeps the terminal fallback as before (`codexFreshSession` in `server/codex.ts`, called from the `no_session_path` fallback in `paneConversation`): - **Screen**: Codex's start screen with nothing said (`codexStartScreenUnused`, `server/start-screen.ts`, sharing the footer rule the model is read by). Under the last banner line (`>_ OpenAI Codex (v…)`, boxed or not, never an answer that only mentions it) the composer is the first prompt line, and no prompt, answer, warning or error cell follows it. None may sit above the banner either: that would be a command's card or a second session in the same terminal. - **Command line**: no `resume` or `fork` on the argv of the pane's Codex processes. - **Store**: no interactive thread with a first message, for any folder the pane names (the shell's and the foreground process's, as given and as they resolve), begun or written since those processes started (a lower bound of their start, less a second), in the store the pane's Codex writes to. A missing store holds none. That store must be placed for certain on every platform (`certainProcessCodexHome`): the `CODEX_HOME` directory each Codex's environment names, or `.codex` in the `HOME` its environment names when it names none (never the server's own home). An environment that cannot be read (another user's process, a gone one, Windows), a `CODEX_HOME` that is relative or not a directory, and on macOS a command line that spells `CODEX_HOME=` or `HOME=`, no `HOME` or a relative one, or a `ps -E` line with no environment all say no, as do a store or a start time that cannot be read. - **Status**: the pane is not working or blocked. The answer is a conversation not begun, as for an app launch, with the model and effort the status line states. Its history id names the pane and its Codex processes, with no time in it (a start time read on macOS moves between reads), so the first message's transcript replaces it whole. A resumed session still shows its saved messages through the existing lookup. Anything this cannot identify keeps "Conversation unavailable". A launcher that picks another `CODEX_HOME` works because the pane's own Codex home is the one checked. ## Verification - **Unit** (`server/conversation.test.ts`, `server/start-screen.test.ts`): a Codex at its start screen gets the greeting with no store, and with a store whose threads of the folder all ended before it started. Each of these keeps "unavailable": a `resume` or `fork` command line, something said on screen, a pane at work, a thread of the folder (or of its resolved path, or of the foreground folder) with a first message begun or written since, and an unreadable store. A thread in another folder, a subagent's, a `codex exec` run's, or one with no first message does not count. The screen cases cover a draft in the composer, an answer quoting the banner, a boxed banner and a conversation above the banner. A second test starts real child processes with the `CODEX_HOME` under test (server default store isolated): an absolute home greets; a relative one whose store holds a conversation begun now, a non-directory, and a second Codex with a relative home do not. With no `CODEX_HOME`, the store follows the child's own `HOME`; with no `HOME` there is no greeting. Each child says it runs before it is read, so the test does not race its exec (0 failures in 30 runs). 163/163 pass on Linux and on macOS 27 (the test MacBook Air, fresh clone). Restoring the default-store fallback, or the server's `homedir()`, fails the test on both. - **Real surface**: real `codex` 0.163.0 typed in a pane of the isolated test herdr session (throwaway `CODEX_HOME`, closed provider), with Chat opened in headless Chrome: - At 1440 px and 390 px, the greeting "What should Codex do in proj?" shows with the GPT-6-Sol / Medium chip, and the frame shows no "Conversation unavailable". `GET /api/pane/conversation` answers 200 with `history_id: unwritten:codex:…`, no turns, and model `gpt-6-sol` / `medium`. - After a first message the greeting is gone, and Chat shows the terminal fallback as on main while the turn runs. - `bun run check fast` on d12ff6d: green (unit 7191 pass, 0 fail, 24 skip; typecheck, build, generated types and themes, workflow syntax ok). ## Reviews - **omo-native-code-reviewer**: approve. Its one MEDIUM (the symlinked-folder assertion could not fail, because the foreground folder still matched) is fixed: the case now names the link as both folders, and dropping the `realpathSync` in `codexFreshSession` fails it. LOWs left as follow-ups: - A fresh Codex that draws a start-up warning cell (an MCP server failing to start) keeps "Conversation unavailable", as on main: a warning counts as something said. - Windows never greets a terminal-started Codex: there is no process start time or environment to read. - Its other two LOWs (a relative `CODEX_HOME` guard on Linux only, and the fallback listing the pane's Codex processes twice) went away with the merge-pass fix below, which places the store itself. - **Codex (gpt-6.1-sol)**: no HIGH, MEDIUM or LOW on the rebased commit; it checked the greeting against main's newer lookups (#1941 loaded threads, #1790 title candidates, #1750 the pane's Codex home), which all run before it. Two earlier rounds on the pre-rebase version found a HIGH (answer text taken for the banner) and MEDIUMs (symlinked folder, relative `CODEX_HOME`, macOS start time, foreground folder), all fixed. - **Merge pass (Astra gate)** on 9cc35fa: one blocker. On macOS a relative `CODEX_HOME` fell back to the server's default store, so a false greeting was possible. Fixed in c1b4e8d: the store is now placed for certain on every platform, as described under **Store**. Its re-review of c1b4e8d found a HIGH (with no `CODEX_HOME`, the store came from the server's home rather than the Codex's own `HOME`) and a MEDIUM (the new test raced its child's exec), both fixed in d12ff6d and 11d7b71. --------- Co-authored-by: mp <mp@local>
devswha
added a commit
that referenced
this pull request
Oct 26, 2026
… the agent or skill (#1991) ## Change Follow-ups from #1489's review (#1493): sending an `@skill` from Chat to an OpenCode v2 pane. - **The check after Tab** (`server/opencode-skill-attach.ts`): a skill that did attach was reported as not attached when the agent row redrew, when a line with `┃ … ┃` borders was on screen, or when the theme had no bold. The check now accepts the skill's styling by comparing the token with the input's own colour (read before Tab), not with the panel padding, and it tolerates a redrawn model row and bordered content. A plain token is still never taken for an attached one. - **When attaching is not confirmed**, the rest of the message is still sent as plain text with the not-attached note, as before, and the recorded prompt now matches what was typed: a space that Tab already put into the input is recorded once and never typed again. This holds for queued sends too. - **When Tab changes something else**: if the screen after Tab shows another skill attached (also when the longer token wraps onto the next row, or its own name wraps, as `@effect` + `-two`), or OpenCode's agent switched (Build to Plan, seen on two consistent reads), nothing more is typed and no Enter is sent. An immediate send keeps the text in Chat with a note naming the switch; a queued one stays as an uncertain message with the same note. A switch that first shows on the last read before the 1 s acceptance limit, or on a first read that used the whole limit, is refused the same way. Both are in every language, as is the note for another skill. A model-only status row on a narrow terminal never counts as a switch; there the agent name is not on screen, so a switch cannot be seen and the send goes on as an unconfirmed skill (as on main). - **Cold sends** (`server/opencode-native.ts`): a send whose catalog the browser had not fetched ran the synchronous skill scan after the 1.5 s lookup budget. It no longer scans on the send path. It uses the catalog the bridge remembers for the pane; without one, `@` mentions go as plain text and no skill is invented. So a first send while OpenCode's service hangs no longer leaves a "did not attach @effect" record. - `PendingMessage.outcome_id` is removed: nothing sets or reads it (item M4 of #1444). **Not fixed here: Tab landing on a screen that changes during the one round trip after the fresh read.** herdr has no conditional send-keys (`pane.send_keys` takes only the pane and the keys), and another read before the key would leave the same window open. That needs herdr to send a key only if the screen is still at a given revision. What this PR adds is that a changed result is now detected after the key and the send stops there. **Already fixed on main:** the Codex home edge cases (#1750); macOS without a configured home is intentionally unchanged. Completion is already tied to this send's inbox message and execution, not to the session (`server/opencode-native.ts`, with contract coverage). The wire contract only loses the unused optional `PendingMessage.outcome_id`; refusals reuse the existing `submit_changed` code. An earlier draft added `CommandOutcome.agent_change`; it was dropped because a switch now stops the send, so no outcome ever carries it. Fixes #1493 ## Verification - Unit tests, each file on its own: `server/opencode-skill-attach.test.ts` 59 pass, `server/opencode-native.test.ts` 17, `server/dialogs/opencode.test.ts` 15, `src/lib/compose.test.ts` 61, `src/components/PendingMessages.test.tsx` 10, `src/components/pendingStatus.test.ts` 2, `src/lib/outcomeChip.test.ts` 12, `src/lib/i18n.test.ts` 22. Each finding from the earlier review rounds has a test that failed on the reviewed version. - Contract tests on an isolated herdr session (`bun run check run bun test server/opencode.contract.test.ts server/opencode-first-message.contract.test.ts`): 29 pass, 0 fail, including the immediate and queued agent-switch refusals (text kept, no Enter, no outcome) and an unconfirmed queued Tab that still delivers the rest as plain text. - Browser, demo app over its fixture transport: a refused immediate send shows the agent-switch note in the composer and keeps the text; an uncertain queued message shows it on its row. No horizontal overflow at 390 and 1440 px, light and dark, plus Korean at 390 px. - Reviews: omo-native-code-reviewer approved; its notes are handled above (the changelog no longer claims a switch is seen on a narrow model-only row, the another-skill note is translated, an unread value is gone). Codex (gpt-6.1-sol, xhigh) confirmed the earlier rounds' findings resolved and found one MEDIUM: a replacement token split inside its own name was read as the typed skill attached. It is fixed, with a test that fails without the fix; Codex's own reproducer now passes. Its other probes (a resize during Tab, a narrow redraw that drops the agent label) fail the same way on main and are tracked separately. - The merge pass's gate review of 2063ca2 found one HIGH: an agent switch first showing on the deadline read was not refused, so the rest and Enter went to the other agent. Fixed in 6c56eaf with two regression cases that fail without it (contract tests 29/0, check fast 7230/0 on that commit). - Not a contract change beyond removing an unused optional field, so no extra review gate; the merge pass reviews as usual. - `bun run check fast` on the pushed head: passed (7228 unit tests, 0 failures). --------- Co-authored-by: mp <mp@local>
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.
Change
The Codex home bullet of #1493.
paneDaemonHome(server/codex-daemon.ts) reads the pane's Codex process'sCODEX_HOMEandHOMEfrom/proc/<pid>/environon Linux, to find the daemon whose native skills it lists. Two cases picked a different home from the one Codex itself uses:getenv, which Codex uses, returns the first. It now takes the first, forCODEX_HOMEandHOME.CODEX_HOMEwas resolved against the server's own directory. Codex itself requiresCODEX_HOME(relative or absolute) to exist and be a directory, and canonicalizes it on the filesystem (codex-rsutils/home-dir). The value is now resolved the same way: a relative one from the Codex process's working directory (/proc/<pid>/cwd), following symlinks before..as the filesystem does, and any value that does not resolve to a directory gives no home, never the server's directory.The Linux branch had no unit test; it has now. macOS (no configured home) and the issue's other bullets are unchanged.
Refs #1493
Validation
server/codex-daemon.test.ts: new cases for a duplicatedCODEX_HOME, a duplicatedHOMEand a relativeCODEX_HOMEfail on main and pass now, plus a symlink-before-parent case (43 pass).bun run check fast: pass.