feat(codex): drive, resume & answer Codex over the app-server JSON-RPC transport#6
Merged
Merged
Conversation
Add a live-driver path for Codex built on `codex app-server` (JSON-RPC 2.0 over stdio) — the interface the Codex VS Code extension uses — as a third acquisition transport beside the rollout-file scraper and the PTY TUI launch. It lands in the same `ExtractedRows` + pending-input signal as the existing provider, never a parallel pipeline; codex protocol types stay inside these files. Three composable layers, all verified: - transport.ts — generic newline-delimited JSON-RPC client over a child process's stdio. Scoped spawn (killed on scope close); raw readline/exit/error callbacks only offer onto an inbound queue; one scoped fiber resolves id-correlated Deferreds and fans notifications / server-requests to Streams. On process death every pending request fails and both streams end. - protocol.ts — Effect Schema for the item variants a turn produces (userMessage, agentMessage, reasoning, commandExecution), token usage, and the driver-facing thread/start + approval payloads. Mirrors codex.ts's Schema.Union + decode*Option idiom (unknown shape -> skipped, never throws). - normalize.ts — folds a turn's item/completed payloads into the shared SessionRowBuilder / ExtractedRows. Structured commandExecution fields (command, exitCode, aggregatedOutput) drop the EXEC_WRAPPER telemetry-regex the rollout provider needs. - driver.ts — handshake (initialize -> thread/start), notification fold, and the approval state machine: each item/*/requestApproval becomes an addressed PendingApproval in a SubscriptionRef (with server-supplied availableDecisions), cleared on serverRequest/resolved — the deterministic replacement for the racy outputText-flash inference. Tests: hermetic node-peer coverage for the transport and the full driver flow (handshake, approval surface+answer, turn->rows), plus an env-gated live smoke (CODEX_LIVE=1) that drives the real codex binary end to end.
Add an optional `appServer` capability to ProviderSpec — a third launch mode beside `batch` (non-interactive runs) and `interactive` (PTY TUI): a long-lived JSON-RPC process on stdio the codex-appserver driver spawns and drives. Populate it for codex (`codex app-server`). Optional field, so the ListProviders RPC Schema stays backward-compatible. The capability is additive — a provider that declares it can still be scraped post-hoc and launched as a TUI. Its reader is the driver-launch wiring (next step); this just makes the capability discoverable via ProviderRegistry.
Wire the driver into IngestStore so a live-driven codex session is indistinguishable from a scraped one downstream (one store, one projection). - driver: accumulate the thread cumulatively rather than per-turn. The store's replaceSession replaces a session with its whole row set (the file scraper re-parses the entire rollout each time), so each completed turn now yields the session's rows *so far*, not just that turn's delta. - launch.ts: launchCodexAppServerSession reads a ProviderSpec app-server capability, spawns the driver, and persists each completed turn's cumulative rows via IngestStore.replaceSession — the reader of the appServer capability. pendingApprovals / answerApproval pass through unchanged. Tests: hermetic node-peer coverage that a completed turn lands in an in-memory IngestStore; the env-gated live smoke now drives two real turns through launch -> driver -> normalizer -> store and asserts cumulative rows persist.
App-server approvals can't be answered by focusing a PTY (there is none), so Arc must own the answer surface. First main-side pieces: - PendingApproval / ApprovalRequestParams gain approvalId — codex's own approval handle (present on commandExecution approvals, absent on fileChange). It is display/correlation detail; the reliable routing key stays the always-present JSON-RPC request id the request arrived with. - CodexDriverRegistry: owns the live drivers keyed by targetSessionId, mirrors each driver's pendingApprovals into one reactive aggregate (SessionApprovals), and routes AnswerApproval back to the right driver. register is scoped to the caller (the session launch): the mirror fiber + deregistration finalizer live in that scope, so a session that ends drops out of the aggregate. Hermetic test registers a peer-backed driver, answers the approval *through the registry* (proving per-session routing), and asserts the aggregate clears on serverRequest/resolved.
The answer surface app-server needs, reaching the renderer:
- shared AppServerApproval schema — the inline card's view of an approval
(requestId, approvalId, itemId, command, decisions). Ephemeral, never
persisted. Each decision is {label, payload} where payload is the raw
server decision re-encoded as JSON, so acceptWithExecpolicyAmendment and
friends round-trip verbatim — the decision model is never collapsed to
approve/deny.
- codex-approval-view: projectApprovals flattens the registry's per-session
aggregate into that view; parseDecisionPayload decodes a chosen decision back
to the raw value to answer the driver.
- three RPCs: ListAppServerApprovals (one-shot), WatchAppServerApprovals
(stream), AnswerAppServerApproval (echo a decision payload back). Handlers
read/route through CodexDriverRegistry, now provided in the runtime layer graph.
Pure test covers the projection + verbatim decision round-trip (string and
rule-carrying object decisions).
The renderer half of the pty-less answer surface. Unlike the Question card (deliberately read-only — a PTY provider's picker owns the answer), AppServerApproval *is* the answer surface: it renders the server-supplied decisions verbatim as buttons and calls onAnswer(payload) with the raw decision JSON, so acceptWithExecpolicyAmendment and friends round-trip unchanged. Presentational only (approval + onAnswer + answering props), styled with the shared Button/Badge primitives. Verified in Storybook across command-exec, rule-carrying (execpolicy amendment), fileChange (no command), and answering (disabled) states.
The structured-process sibling of the PTY TargetSessionManager, and the piece that makes app-server sessions live. TargetSession identity is unchanged; only the runtime owner differs (per the pty/rpc runtime split). - launch(req): spawns the driver (persisting turns to the shared store via launchCodexAppServerSession) and registers its approvals with CodexDriverRegistry. Idempotent per targetSessionId. Each session gets a scope forked off the layer scope, so stop() closes just that session (kills the driver, deregisters its approvals) and app quit closes them all. - submit(req): runs a user turn (driver.runTurn); accepted:false for an unknown session. - stop / list round out the lifecycle. Provider-agnostic (command/args come from the app-server capability, mapped in by the caller); codex is the only RPC driver today, pi's rpc mode is the same family. Wired into the runtime layer graph. Hermetic test drives a node peer through the whole cycle: launch, answer an approval *through the registry* (proving launch registered the driver), submit a turn, assert it persisted to the shared IngestStore, then stop and assert both the manager and the registry aggregate clear.
…urns The one door onto both session runtimes, and the wiring that makes an app-server session usable from the composer. - SessionRuntimeRouter: launch dispatches by *intent* (LaunchRequest.runtime: "pty" | "rpc") — never by provider identity, since codex declares both interactive and appServer. submit/stop dispatch by *ownership* (which manager holds the id), so no persisted kind is needed. rpc-launch resolves cwd, creates + persists a TargetSession row, spawns the driver, and bindNatives the thread id (so the timeline projection can find the target). No migration. - The router does NOT project rpc turns itself — that would depend on ChatMessageService, which depends back on the router. Instead submit returns the turn's rows and sendPrompt projects them via ingestArtifactSession, keeping the graph acyclic. RpcSessionManager.launch now returns the thread id; submit returns the cumulative rows. - sendPrompt routes through the router: it skips the PTY attached-check for rpc sessions (no byte stream), submits via the router, and projects the returned rows into the chat timeline (the driver wrote IngestStore; the renderer reads the ArcStore projection). - LaunchTarget / StopTarget RPCs route through the router; LaunchTarget gains a runtime field. SubmitPrompt stays PTY-raw. Tests: router dispatch (submit/stop/ownsRpc by ownership, rpc turn carries rows, unknown id falls through to pty). Full suite 506 passed / 1 skipped.
The pty-less answer surface: an app-server session has no terminal to defer approvals to, so Arc owns the interaction. - appServerApprovalsAtom (live WatchAppServerApprovals stream) + answerAppServerApprovalAtom mutation. - ChatApprovals: chat-scoped container filtering the live approvals to the current chat, rendering AppServerApproval cards, echoing a decision's payload back verbatim, with optimistic per-request in-flight disabling. Mounted above the composer in UnifiedChatPane. - Launch entry: LaunchableProvider carries runtime + label; a provider declaring both interactive and appServer surfaces two options (codex / codex . app-server). onLaunch branches — rpc fires LaunchTarget directly (no pane, no xterm measure), pty keeps the pane/measure path. No terminal pane for rpc falls out for free. - ChatApprovals story (Pending filters an other-chat approval out; Empty renders null) — verified in Storybook.
A launched app-server session was persisted and resident in RpcSessionManager but never appeared in the renderer: WatchSessions/ListSessions were backed only by the PTY TargetSessionManager's in-memory store. The router also leaned on the PTY manager's bindNative to record the thread id — a no-op for an rpc target (WARN 'hook binding for unknown target'), so the DB row's nativeSessionId stayed null and ingestArtifactSession could not resolve the target by (provider, native id) to project a turn either. - RpcSessionManager owns observable TargetSession state (SubscriptionRef), builds the session with nativeSessionId = threadId, and exposes sessions/changes; launch returns the full TargetSession. - SessionRuntimeRouter drops bindNative, persists the DB row with the bound thread id, and exposes a unified view over both runtimes — Effect.zipWith for the list, Stream.zipLatestWith (rechunk-1 per side) for the change stream. - ListSessions/WatchSessions handlers repointed at the router. Tests: router asserts the rpc session surfaces in the unified list with its thread id bound; RpcSessionManager asserts sessions populate/clear; rpc-streaming swapped to a router stub.
…top lifecycle, launch cleanup
Three should-fix findings from a Codex review of the branch (work_01kwqe11np):
1. Transport hang after child exit. route({kind:"exit"}) failed pending requests
but recorded no closed state, so a request made *after* the process died
registered a Deferred that no future exit event could fail — an indefinite
hang. Now the exit branch sets a closed error; request/notify/respond fail
fast, with a register-then-recheck in request to close the interleaving
window. Also close the readline interface in a scope finalizer.
2. rpc stop didn't mark the persisted row exited. The router now persists
'exited' after a successful rpc.stop (best-effort + logged, mirroring the PTY
manager's exit handler), so a restart's restorePersistedSessions can't
resurrect a stopped app-server session as a stale non-attached PTY target.
3. launchRpc leaked the live driver if the DB upsert failed. The persist now
pipes tapError -> rpc.stop(id), so a failed LaunchTarget doesn't leave a
running driver + registered approvals + a phantom entry in the unified
WatchSessions that the DB knows nothing about.
Tests: transport 'fails fast after child exit' (proves no hang); router 'marks
the persisted target row exited on stop'. Finding 3's cleanup is verified by
typecheck + inspection (a fault-injecting ArcStore stub isn't worth a one-line
tapError). +2 tests, suite 509 (508 pass / 1 skipped).
A codex session is transport-agnostic — the app-server thread id is a codex
session id, so the same session resumes over either transport. Until now resume
was PTY-only (codex resume <id> in a terminal); this adds resume back into the
app-server runtime, chosen as a resume-time intent (not a persisted session
property), mirroring the launch-by-intent design.
- Driver: makeCodexAppServerDriver takes an optional resumeThreadId and branches
the handshake to thread/resume { threadId } vs thread/start. Both answer with
{ thread: { id } }, so the same decoder extracts the id and the session rejoins
under its original native id.
- RpcSessionManager / launch: thread resumeThreadId through.
- SessionRuntimeRouter: resume(req) dispatches by req.runtime — rpc reads the
persisted row, rpc.launch({ resumeThreadId: nativeSessionId, ... }), and flips
the durable row back to running. ResumeTarget now routes through the router;
ResumeRequest / the ResumeTarget RPC gain a runtime intent.
- Renderer: the detached-session overlay offers 'resume <provider> · app-server'
when the provider declares an appServer capability; it fires ResumeTarget with
runtime:rpc directly (no pane). The resumed session comes back attached, which
auto-clears the detached overlay (it keys on !attached) and surfaces it in the
chat pane.
Tests: driver rejoins by id via thread/resume; router rpc-resume rejoins under
the same native id + re-marks the row running (provider registry stubbed to a
scripted peer so the path is drivable without the real codex binary). +2 tests,
suite 511 (510 pass / 1 skipped).
Resuming a codex session into the rpc runtime showed it twice ('codex 1
detached' ×2) and never cleared the detached overlay. Every persisted row is
boot-restored into the PTY TargetSessionManager as a detached shell (it can't
tell rpc from pty — no runtime column). rpc-resume then made the same id live in
RpcSessionManager too, so one identity was held by both managers and the unified
list (pty ++ rpc) carried it twice. Worse, detachedSession keys on
'!attached' (shellSelectors), so it kept finding the stale PTY copy and the
overlay stuck.
The fix is ownership transfer, not view dedup: a single id is owned by at most
one manager. TargetSessionManager gains release(id) — drops an id it holds only
as a boot-restored detached shell (refuses if a live PTY child exists, which
would orphan a process). The router calls pty.release(row.id) after the rpc
session is live, so the id leaves the PTY store. The merge stays a plain concat;
no precedence rule needed.
Test: the router resume test now runs against a PTY stub that holds the id as a
detached shell and honors release, and asserts the unified list carries it
exactly once (attached) afterward — it'd be two without the release.
…turns Two UX gaps for app-server (rpc) chat turns: 1. The composer held the sent text (greyed, disabled) until the turn finished, because SendChatPrompt awaited the whole rpc turn. The composer is now optimistic: it clears the draft immediately and lets the turn run in the background (the reply lands via the chat-changes stream), matching how a PTY submit returns instantly. Restores the prompt if the send errors. 2. No 'codex generating' indicator. LiveTargetStateService only iterated the PTY TargetSessionManager, so rpc sessions (which live under RpcSessionManager) had no live state at all, and had no turn signal even if listed. It now reads the unified SessionRuntimeRouter session list and unions in a new RpcSessionManager.generating marker (an rpc session has no hook stream, so the manager sets the marker around each turn) — so the composer/sidebar paint 'generating' the same way they do for a PTY turn's hooks. RpcSessionManager also serializes turns per session with a semaphore: the freed composer makes concurrent submits possible, and the driver folds one turn at a time (unkeyed outcomes) — closing the concurrency hazard the review flagged. Test: manager marks the session generating while a turn is in flight (observed mid-turn while it blocks on an approval) and clears it after. +1 test, suite 512 (511 pass / 1 skipped).
The composer's optimistic user row was persisted immediately, but the chat-update publish only fired at the end of sendPrompt. An rpc turn holds that handler open for the whole turn, so the user's own message didn't surface until codex replied. Publish right after the optimistic upsert so the bubble shows at once; the end-of-send publish still delivers the reply, and reconcileComposerOptimisticUser dedups the row when the turn projects.
… rpc An rpc (app-server) target has no PTY, so focusing/adopting one used to mount an empty xterm (a stray cursor). Add a live `runtime: "pty" | "rpc"` field to TargetSession (set by RpcSessionManager; not persisted — it reflects which manager currently owns the session), thread it into the shell's ShellSessionRef, and use it to keep rpc sessions out of the terminal machinery: adoption skips them, and SESSION_FOCUSED gives them a chat-only focus with no pane. Foundation for the focus split (work_01kwqktppt): the composer-target vs PTY-pane-focus decoupling lands next and replaces the chat-only SESSION_FOCUSED branch here (which currently can't steer the composer to an rpc target when a chat holds multiple targets).
Session focus conflated two notions that only coincide for PTY targets: the "active session" was derived from the active terminal pane, and drove both the composer addressee and the sidebar highlight. A paneless target (rpc app-server; Cursor SDK and other non-PTY runtimes next) could never become current — so focusing the app-server target chat-only (the cursor fix) left the composer still addressing a PTY target when a chat held multiple. Split them into two first-class notions: - terminalPaneId (unchanged) — the focused PTY pane (xterm keyboard/surface). - activeTargetId (new, on ArcShellContext, not persisted) — the current composer target, any runtime. Focus always sets activeTargetId; a PTY focus/bind also activates its pane, a non-PTY focus sets only activeTargetId (no pane, terminal untouched). It's set everywhere a target becomes current — SESSION_FOCUSED, TARGET_BOUND, and the direct rpc launch/resume path (new focusTarget action, focuses by the returned session ref so it doesn't race the live list). On PTY_EXITED it follows to the newly-active pane's session only if the exiting target was active; chat switch clears it. The selector resolves it against the live sessions (attached + in-chat) so a stale id never leaks, and the addressableTarget first-attached fallback is demoted to a cold-start-only default. Design + Codex review on work_01kwqktppt. +6 shell-machine tests (pty focus sets both; rpc focus sets only the target + no terminal; target follows last focus across runtimes; TARGET_BOUND sets it; PTY exit clears only when the exited target was active; chat switch clears; detached never becomes composer target). Suite 519 (518 pass / 1 skipped).
…c-resume ownership transfer)
A boot-restored session isn't live in any runtime — it's just a persisted row.
But boot-restore seeded EVERY persisted row into the PTY manager's live store as
a detached shell (it can't tell rpc from pty), so a former rpc session came back
masquerading as a PTY session. Resuming it as rpc then had to wrench it out of
the PTY manager (pty.release) into the rpc manager — an 'ownership transfer' with
a one-tick window where both managers held the id, which the id-keyed composer/
sidebar chips reconciled into a ghost row ('codex' + 'codex 1').
Model it directly instead. The PTY manager's store holds only sessions with a
live PTY; boot-restore just re-arms hook sockets (rearmPersistedSessionHooks) and
no longer seeds the store. The router's unified list is now
live PTY ∪ live rpc ∪ (persisted rows not live and not exited)
— the last set is 'detached', read from the DB and runtime-neutral. An id is
either live in one manager or detached, never both: no duplicate, no ghost, no
handoff. Resuming a detached session launches it into a runtime; it becomes live
and drops out of the detached set because its id is now live. rpc stop marks the
row exited before removing it from the store so the detached recompute (driven by
that store tick) doesn't resurrect it. PTY resume reads the row from the DB
(restoredSessionFromRow) since the store no longer holds detached sessions.
Removes TargetSessionManager.release and SessionRuntimeRouter.resumeRpc's
pty.release. Supersedes the release band-aid (99656fe). Design + review on
work_01kwqpx98s.
Known edge: a codex CLI that survives an app restart and fires a *delayed*
SessionStart won't re-bind its native id (the restored session isn't in the PTY
store) — the id is already persisted, and a normal resume re-adds the session to
the store before its hooks fire, so the common flow is unaffected.
Tests: router surfaces a persisted not-live not-exited row as detached (and
excludes exited); rpc resume rejoins with no duplicate; boot-restore split into
restoredSessionFromRow (pure) + rearmPersistedSessionHooks. Suite 520 (519 pass /
1 skipped).
The detached-set refactor excluded exited rows, so a stopped rpc session vanished from the sidebar entirely — and its transcript messages lost their source label (the renderer resolves a message's target label from the live session list, which no longer held the session). That was a regression from prior behavior, where a PTY exit stays in its store as exited and every persisted row surfaced. Include every persisted row not currently live, carrying its state — exited rows show as exited (stopped sessions stay visible + their messages resolve their source), everything else as unknown/detached. rpc stop still marks the row exited before removing it from the store, so it shows as exited rather than a spurious resumable-looking 'unknown' for a tick. Test updated: the unified list keeps an exited row (as exited) rather than dropping it.
The detached overlay reads TargetSession.resumable to show the resume affordance. The PTY manager's list sets it (resumable: canResume(s)), but the router's DB-derived detached set didn't — so a restored/stopped codex session showed 'not resumable' even though it has a native session id. Apply canResume in the router's unify when projecting a persisted row into a detached session.
The renderer-facing derived fields (attached, resumable) were computed independently in TargetSessionManager.asList and the router's detached-set projection, so the two paths drifted on which fields they set — a divergence that shipped as three separate bugfixes on this branch (missing resumable, dropped exited rows, lost source label). Consolidate onto presentTargetSession(s, attached) in provider-args, called by both asList and the router's unify. A field added in one is now structurally impossible to miss in the other. Structural fix from the thermo-nuclear review (work_01kwqs3bmq).
Greptile SummaryThis PR adds OpenAI Codex's
Confidence Score: 4/5The change is safe to merge; the one concrete bug is confined to the approval UI and only activates when two app-server sessions in the same chat both have a pending approval at the same time. The approval-answering key in ChatApprovals.tsx uses only the JSON-RPC requestId without the targetSessionId. Because each session's transport numbers its requests from 1, two concurrent sessions in the same chat will inevitably share requestId values. When this happens, answering one session's approval also disables the other session's approval button, leaving the user unable to click it. All other code paths — the transport lifecycle, turn serialization, launch-lock idempotency, ghost-message rollback, and the shell focus split — appear correct. src/renderer/src/chat/ChatApprovals.tsx — the approval key and answering-state key both need to incorporate targetSessionId.
|
| Filename | Overview |
|---|---|
| src/renderer/src/chat/ChatApprovals.tsx | New inline approval card container; the answering-state key and React key both use only requestId without targetSessionId, causing collisions when two sessions in the same chat have pending approvals with the same sequential requestId. |
| src/main/ingest/providers/codex-appserver/transport.ts | New generic ndjson JSON-RPC transport over a child process; scope finalizer correctly fails all pending deferreds on both intentional stop and process crash. |
| src/main/ingest/providers/codex-appserver/driver.ts | Codex app-server protocol driver; folds item/turn notifications and runs the approval state machine; aborted-signal path ensures runTurn doesn't hang on process exit. |
| src/main/services/RpcSessionManager.ts | Runtime owner for RPC-backed sessions; serializes launches via launchLock semaphore and turns via per-session semaphore; mirrors TargetSessionManager patterns. |
| src/main/services/SessionRuntimeRouter.ts | Unified dispatch layer for both session runtimes; routes by launch intent and submit/stop ownership; DB read in unify on every session-change tick is a known follow-up. |
| src/main/services/ChatMessageService.ts | Adds rpc submit path; optimistic publish + rollback publish; duck-typed 'rows' in delivery dispatch flagged as known follow-up. |
| src/renderer/src/shell/arcShellMachine.ts | Adds activeTargetId decoupled from terminalPaneId; rpc session focus skips terminal pane mounting; pane-close correctly follows activeTargetId only when it was the exiting target. |
| src/renderer/src/App.tsx | Adds rpc launch/resume paths that bypass the shell pane machinery; focusRpcTarget sets activeTargetId by ref to avoid race with session list update. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant R as Renderer
participant Router as SessionRuntimeRouter
participant RpcMgr as RpcSessionManager
participant Driver as CodexAppServerDriver
participant Transport as AppServerTransport
participant Codex as codex app-server
R->>Router: "LaunchTarget {runtime:"rpc"}"
Router->>RpcMgr: launch(req)
RpcMgr->>Driver: makeCodexAppServerDriver()
Driver->>Transport: makeAppServerTransport()
Transport->>Codex: spawn()
Driver->>Codex: initialize → initialized → thread/start
Codex-->>Driver: "{thread:{id}}"
RpcMgr-->>Router: "TargetSession {nativeSessionId}"
Router->>DB: upsertTargetSession
Router-->>R: TargetSession
R->>Router: "SendChatPrompt {text}"
Router->>RpcMgr: "submit {text}"
RpcMgr->>Driver: runTurn(text)
Driver->>Codex: turn/start
Codex-->>Driver: item/completed notifications
Codex-->>Driver: requestApproval (server to client)
Driver->>R: pendingApprovals update via CodexDriverRegistry
R->>Router: "AnswerAppServerApproval {decision}"
Router->>Driver: answerApproval(id, decision)
Driver->>Codex: "respond {decision}"
Codex-->>Driver: turn/completed
Driver-->>RpcMgr: "TurnResult {rows}"
RpcMgr-->>Router: "{accepted:true, rows}"
Router-->>R: rows projected into chat timeline
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant R as Renderer
participant Router as SessionRuntimeRouter
participant RpcMgr as RpcSessionManager
participant Driver as CodexAppServerDriver
participant Transport as AppServerTransport
participant Codex as codex app-server
R->>Router: "LaunchTarget {runtime:"rpc"}"
Router->>RpcMgr: launch(req)
RpcMgr->>Driver: makeCodexAppServerDriver()
Driver->>Transport: makeAppServerTransport()
Transport->>Codex: spawn()
Driver->>Codex: initialize → initialized → thread/start
Codex-->>Driver: "{thread:{id}}"
RpcMgr-->>Router: "TargetSession {nativeSessionId}"
Router->>DB: upsertTargetSession
Router-->>R: TargetSession
R->>Router: "SendChatPrompt {text}"
Router->>RpcMgr: "submit {text}"
RpcMgr->>Driver: runTurn(text)
Driver->>Codex: turn/start
Codex-->>Driver: item/completed notifications
Codex-->>Driver: requestApproval (server to client)
Driver->>R: pendingApprovals update via CodexDriverRegistry
R->>Router: "AnswerAppServerApproval {decision}"
Router->>Driver: answerApproval(id, decision)
Driver->>Codex: "respond {decision}"
Codex-->>Driver: turn/completed
Driver-->>RpcMgr: "TurnResult {rows}"
RpcMgr-->>Router: "{accepted:true, rows}"
Router-->>R: rows projected into chat timeline
Reviews (6): Last reviewed commit: "fix(codex): serialize rpc launches (atom..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…solved id)
Two P2 findings from the PR review:
- normalize: agentMessage stored the untrimmed item.text while guarding on the
trimmed value — inconsistent with userMessage. Store the trimmed text.
- driver: serverRequest/resolved extracted requestId as unknown; an absent field
left it undefined, and approval.id !== undefined matches every pending
approval, so the filter cleared nothing and the card stuck until scope close.
Guard on a real number|string id.
The other two findings ('rows' in delivery duck-typing; unify's per-tick DB read)
are the already-tracked non-blocking observations on work_01kwqs3bmq.
…bble) Greptile P1: the optimistic echo publishes immediately to show the user's bubble. When the rpc turn then fails (CodexDriverError -> accepted:false), the rollback deletes the row but didn't publish, so the renderer kept showing a bubble whose DB row was gone until the next message in the chat forced a refetch. Publish the updates after the rollback delete so the ghost clears.
Greptile P1: runTurn does turn/start then Queue.take(turnOutcomes). If the process crashes after the turn/start ack but before turn/completed, the transport shuts its notification queue, the fold fiber ends, but turnOutcomes never gets a completion and is never closed — so the take blocks forever, hanging the SendChatPrompt handler. The fold now offers an 'aborted' signal (a discriminated TurnSignal) when it finishes, via ensuring (not andThen: the queue shutdown *interrupts* the fold, so andThen would be skipped). runTurn takes the signal and fails with a CodexDriverError on abort instead of blocking. Test: driver fails (doesn't hang) when the peer acks turn/start then exits before turn/completed.
…n stop) Greptile P1 (symmetric to the mid-turn fix): pending request Deferreds were only failed in route's exit branch, which needs the drain fiber alive. On an intentional stop, closing the session scope interrupts the drain fiber *before* the finalizers kill the child, so the exit is never routed and a request blocked on Deferred.await (e.g. turn/start) hangs forever, holding SendChatPrompt open. Extract failAllPending() and also run it as a scope finalizer, so pending requests fail on scope close regardless of the drain fiber. Idempotent with the exit route. Test: an in-flight request fails (doesn't hang) when the transport scope is closed while it's pending.
Greptile P1: RpcSessionManager.launch had no lock, so two concurrent launches of the same target id (a double-clicked resume) both passed the idempotency guard, both spawned a driver + thread/resume, and the second sessions.set orphaned the first scope — a leaked codex process and two clients on one thread. Wrap launch in a per-manager Semaphore.make(1), mirroring the PTY manager's launchLock, so the check + spawn + set are atomic and the second launch returns the first's session. Swept the rest of the rpc lifecycle for the same families while here: both teardown-hang points (transport Deferred.await, driver Queue.take) are already guarded; stop is idempotent (Scope.close is safe to repeat). Test: concurrent launches of one id spawn a single driver (peer reports its pid as the thread id).
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
1 participant
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.
Adds OpenAI Codex's
app-server(a long-lived JSON-RPC 2.0 process over stdio — the interface the Codex VS Code extension is built on) as a third live driver transport, alongside the existing rollout-file scraper and PTY TUI launch. You can now launch, message, approve, resume, and stop a Codex session driven directly over the typed protocol — with its turns landing in the same chat timeline as every other provider.Why
The rollout-file scraper is post-hoc and lossy (it reverse-parses a telemetry preamble out of tool-result text). app-server gives a typed, bidirectional, streaming control protocol: first-class
commandExecutionfields, real server→client approval requests, and token usage — no scraping. It's additive, not a replacement: live driving and post-hoc ingest stay complementary, writing to one store and one projection.Architecture
Three quarantined layers under
src/main/ingest/providers/codex-appserver/:initialize → thread/start(orthread/resume), foldsitem/*/turn/*notifications, runs the approval state machine, normalizes to the sharedExtractedRows. Every payload isSchema-decoded.IngestStore, so a live-driven turn and a scraped turn render identically.Session runtime split —
TargetSessiongained a second live runtime:RpcSessionManager— runtime owner for app-server sessions (sibling of the PTYTargetSessionManager): serializes turns, tracks ageneratingmarker.SessionRuntimeRouter— the one door: dispatches launch by intent (runtime: pty | rpc), submit/stop by ownership. Assembles the unified session list.live-pty ∪ live-rpc ∪ (persisted rows not live), so an id is live in exactly one manager or detached — never both. Resuming launches it into a runtime; no cross-manager ownership handoff. A Codex session is transport-agnostic: the same detached row resumes over either PTY (codex resume) or rpc (thread/resume).Approvals — app-server sends real approval requests (an rpc session has no PTY to defer to), so Arc owns the interaction:
CodexDriverRegistryaggregates each driver's pending approvals; an inline card answers by echoing the server-offered decision back verbatim (the decision model —acceptForSession,acceptWithExecpolicyAmendment— is never collapsed to a boolean).Renderer
interactiveandappServersurfaces two options (codex/codex · app-server); rpc launch/resume bypass the terminal-pane machinery.terminalPaneId(PTY-pane focus) andactiveTargetId(the current composer target, any runtime), so a paneless (rpc / future SDK) target can be the composer's addressee and the sidebar's active highlight.Testing & review
codexbinary throughout.presentTargetSession.Known edge / follow-ups
"rows" in delivery, scopeunify's per-tick read.