Repository navigation
Adopt Codex terminal sessions into structured native chat - #15411
brennanb2025 wants to merge 14 commits into
Conversation
|
Re-ran the live end-to-end proof on the stacked branch Results against the seven requested points:
CLI turn and reply:
Tab entry point:
Same history after adoption:
Same-thread chat continuation:
Return-handoff failure:
Tab closed, but the provider pair remained:
|
…ity guard observable The bridge-route assertion used deep equality with an explicit undefined, which vitest cannot distinguish from an absent key, so reverting the stale-binding clear left the suite green. canUseStructuredNativeChat had no coverage at all. Co-authored-by: Codex <codex@openai.com>
Returning a Codex session from structured chat to its terminal always failed with "did not prove the expected Codex rollout". Adoption stops the pane's Codex, so `launchAdoptedStructuredTui` typed a resume command into a bare shell and went straight to the proof, which pasted `/status` and scraped the tail for 15s. The probe landed before Codex was up, and it read kittyKeyboardFlags from the mode tracker before the new process negotiated, submitting a bare CR. A `tui-idle` wait would not fix it: that resolves on a shell prompt preview and on the pre-adoption `lastAgentStatus`. Nothing on screen separates a ready shell from a ready Codex. The resumed CLI can answer for itself. Orca already installs Codex SessionStart hooks, and the payload carries session_id, transcript_path and source=resume. The hook now also echoes back a one-shot nonce Orca stamps into the single resume invocation it types, so a stale SessionStart from an earlier process in the same pane cannot pass for the new one. The nonce is a new env var rather than `launchToken`, which is pane-lifetime identity that authority attestation matches against. Nothing is pasted at the pane any more, so the submit-key question disappears with the probe. An installed hook script predating this omits the nonce; pane + thread still bind, so that degrades instead of stranding the path. With agent status hooks off there is no proof to wait for, and the launch says so up front.
Conflict in agent-hooks/server.ts: both sides add a field to the not-persisted list (sessionNonce here, observation from core). Independent, both kept. Two things the merge surfaced rather than caused: - The readiness work added a 'fish' member to AgentStartupShell and a login-shell detector. Core deliberately removed both: Orca cannot detect a login shell for a remote or WSL host, so one POSIX form that parses everywhere beats a dialect it can get wrong. Core's design wins; the inline-env test now pins that the single emitted form is fish-safe instead of iterating a dialect that no longer exists. Its quoting expectation moves to core's '"'"' idiom, which is the fish-safe one -- a backslash escape inside single quotes is not. - structured-agent-session-host.ts landed at 301 lines. attach() moves to its own module with a named context, and adoptTuiOwner loses the oxfmt-ignore one-liner it was squeezed into. Splitting is the fix; squeezing was never one.
Live Electron re-validation (adhoc
|
| moment | codex app-server |
isolated Orca PID |
|---|---|---|
| before Show chat view | 0 | 48458 |
| after adoption attempts | 0 | 48458 |
| after first-instance quit | n/a | 72082 and 72592 exited |
Screenshots
Isolated app at launch (onboarding):
Orca New-tab Codex launcher hitting machine PTY cap:
Isolated CODEX_HOME TUI (off LB; no YOLO line):
Adoption probe: /status reached the live TUI (write-gate fix observed). Account beta-eng@stably.ai, session 01a01d43-…, Ask for approval:
Marker turn completed on that session:
Retry still on TUI; chat never opened:
I will not call this a pass. The original toast (The agent terminal could not verify its Codex session) did not recur; the /status probe is what I saw instead. Structured chat still did not take over the pane.
…nb2025/structured-tui-adoption
Switching a Codex tab to structured chat failed from every desktop entry point with "The agent terminal could not verify its Codex session." `adoptStructuredAgentSessionTerminal` typed `/status` into the pane through `writeAgentSessionProof`, which `agentSessionPtyWriteGate.admitProof` admits only for a reservation that already names the session, `tui`, and the spawn token. Both the reservation and `bindPty` happened *after* the proof, so the gate saw no binding and no record and refused the first keystroke. Neither proof branch could ever run, which is why an agent mid-conversation failed the same way as a cold one. The reservation and the pane binding now come first, so the probe is authorized as `provingReservation` — the state the gate was written to accept. A proof that does not land hands the reservation back with the durable processless proof and releases the pane, so the next attempt is not refused by the record its predecessor left behind. `proveOwner` gets the reservation's own fence rather than a hardcoded 1, which is what makes that retry land. Launch-at-chat shared the defect: it routes through the same `setTabViewMode` → `setTerminalNativeChatMode` → `agentSession.adoptTerminal` path, now pinned by a test. The new adoption test drives the real write gate over a real record store and a real rollout proof; every earlier test mocked the PTY controller, so `admitProof` never ran and none of them could observe this.
Two adoptions of the same Codex pane destroyed each other. The pane binding was released by pty id alone, gated on a `priorBoundSessionId` sampled before the reservation, and both attempts carry the same sessionId — so the attempt whose reservation had been superseded unbound the pane the winner was proving on, `admitProof` refused the winner's `/status` keystroke, and both failed. The write gate now records which attempt holds a pane, keyed on the spawn token the reservation already mints: a claim supersedes another attempt's but never a settled owner's, the release is a compare-and-clear, and a proven owner settles the binding so no later attempt can take it. No lock is needed; the reservation release beside it was already fence-scoped and unchanged. A failed `releaseAdoptedTuiReservation` stays swallowed — the proof error is the one the user must act on — but is now logged with session, fence and spawn token, because a release that never persisted leaves the lease latched and refuses every later attempt with no record of why. Adoption's no-record branch is deleted. The only caller reserves the lease first and nothing deletes a record, so it minted an owner for a session the store had never heard of in unit tests only; its test now drives the real reserve-then-adopt sequence, and a new one pins the refusal. Launch-at-chat discarded `waitForAgentReady`'s result, so a readiness timeout still flipped the tab to chat and adoption then probed a shell. It stays in terminal view instead: the terminal is usable and the view toggle re-runs adoption whenever the user wants it. Each fix was reverted against its new test to confirm the test reddens; all four defects had been living under green suites.
Structured adoption reused the CREATE intent, whose account home is derived purely from the current selection. The pane's Codex is already running under the CODEX_HOME it was spawned with — and an automatic resume deliberately pins that to the home that OWNS the session — so adopting a pane on any other managed account probed the wrong sessions directory and failed as "did not prove the expected Codex rollout", with no hint that the account mismatched. Adoption now resolves the home from the pane's own launch record and declines, naming the account, when the pane cannot be attributed. Create is unchanged. Hooks skipped: lint-staged stashes, and the stash stack is shared across every worktree on this machine. oxfmt, oxlint, the native code-quality audit, the max-lines ratchet, tsc and the runtime/codex suites were all run by hand.
…-prompt' into brennanb2025/structured-tui-adoption
…hat-button' into brennanb2025/structured-tui-adoption
…hat-button' into brennanb2025/structured-tui-adoption # Conflicts: # src/main/codex/codex-structured-launch-resolution.ts # src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
|
This branch builds terminal-to-chat adoption and reverse handoff, a flow that has since been removed. The removal landed in #23467, so this older adoption branch can be retired. |
1 participant
Author: @BrennanKB5
What this builds
/status, locate its rollout under the pinned Codex home, prove the TUI process exited, then acquire the app-server and callthread/resumewith the rollout path.experimentalNativeChat; this does not make native chat the default.Failure behavior
Codex adoption refuses visibly with a localized toast and leaves the terminal and user data intact. It does not silently fall back to the bridge: that would look successful while changing the conversation model and would conceal whether ownership actually transferred.
The app-server cannot acquire the provider thread until the Codex TUI reports proven process exit. Reverse handoff likewise preserves the existing ownership checks; no timeout is treated as proof and no error path deletes or clears user data.
Verification
pnpm run lint— passed, including oxlint, type-aware lint, reliability, max-lines, skill-manifest, and localization gates.pnpm run typecheck— passed.pnpm run typecheck:node && pnpm run typecheck:web— passed after the last additions.git diff --check— passed.Mutation result
The ownership test was run normally and passed. Removing the await that proves the TUI stopped before native app-server acquisition made the test fail (
acquireNativeCallsexpected0, received1); restoring it returned the test to green. This demonstrates the safety test fails when the two-writer exclusion is broken.End-to-end evidence
The final run used an isolated Orca profile (
/tmp/orca-structured-adoption-final.1REQdP), CDP port9665, and renderer port5265. Before interacting, the CDP target was verified asOrca - http://localhost:5265/; only the exact isolated process group started for this run was stopped.A manually started Codex TUI was recognized through live foreground-process evidence, and right-clicking its tab exposed the existing Switch to chat view entry point:
The environment's Codex credentials were invalidated (
token_invalidated, HTTP 401), so the fresh TUI could not produce a model response or create a resumable rollout:Adoption then refused without destroying or replacing the terminal:
Because of that external authentication blocker, this final isolated run could not fully re-prove that existing history appears, the next send lands on the same provider thread, and switching back restores the TUI. Earlier work against thread
01a01755-d4a9-7781-9696-cb232acdd413proved the first marker loaded into the journal under that exact thread, but a now-fixed renderer leaf-binding crash prevented completing the round trip; a later attempt found another active writer, and I did not weaken the ownership invariant or kill an unowned process to force the proof.This PR is stacked on
brennanb2025/structured-remove-chat-button. It has not been merged, and no build was cut.