Repository navigation
[AI-1761] Codex app-server: hosted-agent runtime, transport & transport wiring - #582
Merged
Merged
Conversation
Adds CodexAppServerConnection — the newline-delimited JSON-RPC 2.0 stdio transport the app-server hosted-agent runtime speaks over. It mirrors the proven AcpConnection (id correlation, single write-gate, one resilient read loop, always-answered server requests) and reuses the shared AcpRpc envelope records, but drops ACP's reconnect-latch machinery: a hosted Codex reviewer is a one-shot unattended run with no reconnect design, so a dead app-server just ends the read loop, faults pending requests, and is reaped through the normal death path. 15 pipe-driven unit tests cover correlation, out-of-order responses, error faulting, notification fan-out, the always-answered server-request guarantee, malformed/wrong-typed frame resilience, and read-loop-end faulting. Part of the AI-1761 app-server hosted-agent runtime.
CodexAppServerHostedAgentRuntime drives a hosted Codex reviewer over the app-server JSON-RPC protocol instead of the interactive PTY. It is control-plane only — transcript recording stays on the hooks + `kcap watch` rollout path, so EmitsTerminalOutput is false and no transcript is aggregated from the protocol stream. Lifecycle (StartAsync, called by the factory): spawn -> initialize (with delta opt-out) -> hooks/list trust preflight (proceed / seed-and-restart / missing -> fail closed) -> thread/start (resolved model + deterministic thread-id identity) -> optional first turn/start. Each review round is a turn/start on the held thread; WaitForTurnIdleAsync resolves from the matching turn/completed notification (and unblocks on transport death rather than hanging). The reaper is fed through AgentActivityClock. Approvals are pinned to `never`, so any server-initiated approval request is answered with a valid on-the-wire decline (never a JSON-RPC error) and logged at Error. Client->server sends are bounded- retried on the -32001 backpressure rejection. Token usage is captured from thread/tokenUsage/updated. All request params and response parsing match the authoritative schema generated from the pinned codex 0.146.0 (initialize/thread-start/turn-start/ turn-completed/hooks-list/tokenUsage). Wire shapes are AOT-safe (JsonNode -> JsonElement, no reflection). 10 lifecycle tests drive the real runtime against an in-process FakeCodexAppServer over pipes: handshake + model/opt-out, round settlement, initial-prompt turn, hook seed-and-restart, missing-hook fail-closed, always-decline approval bridge, usage capture, backpressure retry, failed-status settlement, unsupported raw input. Part of the AI-1761 app-server hosted-agent runtime.
Live validation against the real codex 0.146.0 binary caught a wire-shape bug a
protocol fake could not: thread/start.sandbox is the coarse SandboxMode STRING
(read-only / workspace-write / danger-full-access), a DIFFERENT shape from
turn/start.sandboxPolicy's {type:…} object. The object form is rejected by the
app-server on thread/start ("unknown variant `type`"). The per-turn sandboxPolicy
object stays the load-bearing containment; the thread default is the string.
Adds CodexAppServerPosture.RenderSandboxMode (validated kebab-case string) and
points StartThreadAsync at it. Adds a gated live smoke
(KCAP_CODEX_APPSERVER_SMOKE=1) that drives the no-model handshake surface
(initialize → hooks/list → thread/start) through the production connection
against the installed codex binary, in an isolated CODEX_HOME — it skips loudly
in CI (no codex there) and is the schema-drift tripwire for a Codex version bump.
Part of the AI-1761 app-server hosted-agent runtime.
Wires the app-server runtime into the launch path as the single codex factory. - CodexTransportDecision: the ONE rule (operator selection AND the spike-pinned 0.146.0 version floor, failing toward PTY on anything unparseable), resolved once into DaemonConfig.CodexAppServerActive and read by BOTH the launch router and the certification advertisement, so the advertised policy version and the transport actually used can never diverge. - CodexHostedAgentRuntimeFactory: the only "codex" entry in the vendor->factory dictionary. Routes review-flow launches to codex app-server when active; every interactive launch, and all launches under the PTY default, delegate to the wrapped PTY factory byte-identically. Reuses CodexLauncher.Prepare (fail-closed hooks + TrustWorktree) + BuildAppServerLaunchArgs + the PTY env assembly, and spawns via a seam (real Process / shared AcpChildProcess in prod, a fake peer in tests). Capability advertisement delegates to the PTY factory unchanged — the transport swaps the launch mechanism, not the native-tool-clamp containment. - DaemonConfig.CodexTransport (KCAP_CODEX_TRANSPORT, default pty) + the resolved CodexAppServerActive (probed only when app-server is selected, so a PTY daemon pays no startup probe). DaemonRunner advertises codex-appserver-unattended-v1 vs codex-unattended-v1 from that one field. 22 tests: the decision truth table (floor, tolerance, case/trim), and factory routing — PTY delegation for pty-transport and for interactive-under-active, the app-server route producing a CodexAppServerHostedAgentRuntime via the seam, and fail-closed on missing hooks. Existing capability-advertisement suites stay green (default config advertises codex-unattended-v1 unchanged); AOT clean. Part of the AI-1761 app-server hosted-agent runtime.
PR Summary by QodoAdd opt-in Codex app-server transport for hosted reviewers
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+186
to
+188
| if (root.ValueKind != JsonValueKind.Object) { | ||
| _logger.LogDebug("app-server: skipping non-object frame (kind={Kind})", root.ValueKind); | ||
| return; |
There was a problem hiding this comment.
2. valuekind bypasses json extensions 📘 Rule violation ⚙ Maintainability
The new app-server parsing code performs direct JsonElement.ValueKind checks instead of using the shared JsonElementExtensions. This violates the required AOT-safe JSON inspection pattern in multiple production paths.
Agent Prompt
## Issue description
The new Codex app-server parser directly compares `JsonElement.ValueKind` throughout its production parsing paths.
## Issue Context
Use the shared `JsonElementExtensions` helpers such as `IsObject`, `IsArray`, `IsString`, `Str`, `Num`, `Obj`, and `Arr` instead of direct kind inspection.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Harness/Codex/CodexAppServerConnection.cs[186-197]
- src/Capacitor.Cli.Daemon/Harness/Codex/CodexAppServerConnection.cs[237-256]
- src/Capacitor.Cli.Daemon/Harness/Codex/CodexAppServerHostedAgentRuntime.cs[464-478]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
|
||
| // ── App-server route (spawn seam; isolated HOME so TrustWorktree touches nothing real) ────── | ||
| [Test] | ||
| [NotInParallel("HomeEnvVarMutation")] |
There was a problem hiding this comment.
4. notinparallel uses group key 📘 Rule violation ☼ Reliability
The tests mutating HOME and CODEX_HOME use keyed [NotInParallel("HomeEnvVarMutation")] rather
than the required bare [NotInParallel]. They may therefore overlap with unrelated tests that
access the same process-global environment.
Agent Prompt
## Issue description
Tests that mutate process-global environment variables use a keyed `NotInParallel` group instead of disabling parallel execution globally.
## Issue Context
Replace both keyed annotations with bare `[NotInParallel]` as required for tests manipulating process-global state.
## Fix Focus Areas
- test/Capacitor.Cli.Daemon.Tests.Unit/Harness/Codex/CodexHostedAgentRuntimeFactoryTests.cs[113-115]
- test/Capacitor.Cli.Daemon.Tests.Unit/Harness/Codex/CodexHostedAgentRuntimeFactoryTests.cs[147-149]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Round-1 findings from the codex code-review flow on #582 (borrowed workspace): P1 (correctness / leaks): - ReadOutputAsync no longer completes immediately. The orchestrator treats its completion as the finalize trigger, so an instant yield-break finalized (and killed) every reviewer seconds after launch. It now waits on a whole-runtime terminal signal (or the caller's cancellation), mirroring the ACP runtime. - The terminal signal is scoped correctly across the hook-trust restart. The reused single TCS was permanently completed by child 1's read-loop end, making every WaitForTurnIdle on child 2 return immediately. Introduced _runtimeTerminal (fires only on the LIVE child's death / dispose, gated by a _restarting window), a per-child read-loop CTS so teardown unblocks the loop, and TeardownChildAsync now awaits the retiring loop before installing child 2. - The turn-completion race is closed: the current turn id is unknown during the turn/start request window, so a stray/fast turn/completed could clear the new round's waiter. Early completions are now stashed by id and applied only if they match the started turn; a start response without a turn id is rejected; the turn-in-flight clock is armed before the send and cleared only on the real matching completion. - The factory disposes the runtime if StartAsync throws after spawning a child (fail-closed hook/protocol paths), so a live Codex child is never leaked. P2: - turn/start now carries the requested reasoning effort (max -> xhigh mapping), which was silently dropped on the app-server route. - A wrong-typed inbound server request (non-string method) is answered -32600 with the original id instead of stranding it (the "always one response" invariant now covers the dispatch/validation path). - The version-floor gate rejects prereleases / build-metadata / non-numeric parts (0.146.0-rc.1 is BELOW the verified 0.146.0 release), failing toward PTY rather than normalizing an unverified build upward into the containment- sensitive transport. Adds tests for each fix; existing Codex suites + the live smoke stay green, AOT clean. Part of the AI-1761 app-server hosted-agent runtime.
- Move the Codex transport/version-floor resolution out of DaemonRunner into CodexTransportDecision.ResolveActive (vendor logic stays under Harness/Codex/; DaemonRunner only reads the env var and registers), and trim the startup comment to the why (probe deferred behind the selection so a PTY daemon pays nothing). - Adopt the shared JsonElementExtensions (Obj/Arr/Str/Num) in the app-server runtime parsing, removing the duplicate local Str/Long helpers. (The connection keeps direct ValueKind checks to match its sibling AcpConnection transport.) - Factory tests use the TempDir helpers (CreateFile / PathTo) instead of System.IO.Path.Combine + Directory/File. - Document KCAP_CODEX_TRANSPORT (default pty, app-server opt-in, 0.146.0 floor, review-flow-only, rollback) in the README daemon-config section. No behavior change; 56 Codex unit tests + the live smoke stay green, AOT clean. Part of the AI-1761 app-server hosted-agent runtime.
Collaborator
Author
|
Thanks — addressed the Qodo review (0 bugs, 6 rule violations) in the latest commit: Fixed:
Kept, with rationale:
56 Codex unit tests + the gated live smoke against real codex 0.146 stay green; AOT clean. |
This was referenced Sep 7, 2026
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.
Hosts unattended Codex reviewers over
codex app-server(JSON-RPC 2.0 over stdio) instead of the interactive PTY, behind acodex.transportconfig that defaults topty(so this is fully inert until an operator opts a daemon in). Builds directly on the launch-input layer (#578).Closes #581. Linear AI-1761 (epic AI-1759).
What's here (4 commits)
CodexAppServerConnection: newline-delimited JSON-RPC 2.0 over stdio, a lean mirror of the provenAcpConnection(id correlation, single write-gate, one resilient read loop, always-answered server requests), reusing the shared envelope records and dropping ACP's reconnect-latch machinery a one-shot unattended reviewer has no design for.CodexAppServerHostedAgentRuntime: control-plane only (transcript stays on the hooks +kcap watchrollout path, soEmitsTerminalOutput=false). Lifecycle: spawn →initialize(delta opt-out) →hooks/listtrust preflight (proceed / seed-and-restart / fail-closed) →thread/start(resolved model + deterministic thread-id identity) → per-roundturn/start.WaitForTurnIdleAsyncresolves fromturn/completed(and unblocks on transport death). Approvals pinned tonever, so any server-initiated approval request is answered with a valid on-the-wiredecline(never a JSON-RPC error) and logged at Error. Client→server sends are bounded-retried on the-32001backpressure rejection. Usage captured fromthread/tokenUsage/updated; reaper fed viaAgentActivityClock.thread/start.sandboxis the coarseSandboxModestring, a different shape fromturn/start.sandboxPolicy's object (the object form is rejected there). Fixed and re-verified. Adds a gated (KCAP_CODEX_APPSERVER_SMOKE=1) live smoke that drives the no-model handshake surface through the production connection against the installed binary in an isolatedCODEX_HOME— the schema-drift tripwire for a Codex bump (skips loudly in CI, which has no codex).CodexHostedAgentRuntimeFactoryis the single "codex" factory: routes review-flow launches to app-server whenDaemonConfig.CodexAppServerActive, delegates everything else (all interactive launches, all launches under the PTY default) to the wrapped PTY factory byte-identically.CodexTransportDecisionis the ONE rule (operator selection AND the pinned 0.146.0 floor), resolved once into that field and read by both the router and thecodex-appserver-unattended-v1advertisement, so the advertised policy and the transport used cannot diverge.Testing
codex-unattended-v1unchanged). AOT clean (no IL2026/IL3050).Scope / follow-ups
codex-appserver-unattended-v1classifier (AI-1761 server half / PR 3) are follow-ups; the server already accepts the new policy string via its existing unknown-string handling, so this daemon is safe against an old server.🤖 Generated with Claude Code