[realtonyyoung]

What

The Codex app-server hosted reviewer never shut down gracefully on a park, so every park teardown 15s-timed-out to SIGKILL (exit 137) — delaying AgentUnregistered ~16s each time. This is the kcap-cli half of the §2.7 B6 park→resume issue (server-side tracked in kurrent-io/kcap-server#1715 / AI-2333).

Root cause

CodexAppServerHostedAgentRuntime.RequestGracefulStopAsync sent turn/interrupt and only when a turn was in flight:

var turnId = _dispatcher.CurrentTurnId;
if (connection is null || threadId is null || turnId is null) return;   // ← between-rounds park: no-op

At park time the reviewer is idle between rounds, so CurrentTurnId is null and the method returned immediately without touching the process. Even mid-turn, turn/interrupt only cancels the turn — it never asks the codex app-server process to exit. So HasExited never flipped, WaitForExitAsync(15s) always exhausted, and teardown fell through to TerminateAsync's SIGKILL on every park.

How

codex app-server exits cleanly (code 0) on stdin EOF — verified empirically against codex-cli 0.147.0 (a bare codex app-server < /dev/null exits 0 with no handshake).

• Add CodexAppServerConnection.CloseInputAsync() — disposes only the write stream (the peer's stdin), leaving the read loop to observe the peer's own exit via stdout EOF. Serializes with an in-flight write via the write gate (bounded), idempotent, best-effort.
• RequestGracefulStopAsync now: (1) interrupts the in-flight turn if one exists (unchanged, best-effort), then (2) unconditionally closes stdin so the process exits inside the graceful window. If the peer ever ignored EOF, the caller still falls through to WaitForExitAsync → TerminateAsync — so this can only shorten teardown, never wedge it.

Verification

• New unit test CloseInputAsync_ends_the_write_side_so_the_peer_reads_eof over the in-memory pipe harness — the peer reads EOF exactly as a real app-server's stdin would; read side stays usable; idempotent.
• Full Codex app-server harness suite green (44/44: CodexAppServerConnectionTests + CodexAppServerHostedAgentRuntimeTests).
• Linear-id lint passes (no ticket tokens in tracked C#).

🤖 Generated with Claude Code

[linear-code]

AI-2333

[qodo-code-review]

PR Summary by Qodo

Gracefully stop Codex app-server by closing stdin

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Close app-server stdin during every graceful stop so idle reviewers exit cleanly.
• Interrupt active turns before signaling process shutdown, with bounded best-effort waits.
• Verify write-side EOF delivery and idempotent closure using the in-memory pipe harness.

Diagram

sequenceDiagram
    actor Park as Park teardown
    participant Runtime as Hosted runtime
    participant Dispatcher as Turn dispatcher
    participant Connection as App connection
    participant Codex as Codex process
    Park->>Runtime: Request graceful stop
    Runtime->>Dispatcher: Check active turn
    alt Turn active
        Runtime->>Connection: Interrupt turn
        Connection->>Codex: turn/interrupt
    end
    Runtime->>Connection: Close input
    Connection-->>Codex: stdin EOF
    Codex-->>Park: Exit code 0

Loading

High-Level Assessment

Closing stdin is the appropriate process-level shutdown mechanism because Codex app-server treats EOF as a clean exit while preserving stdout long enough for the read loop to observe termination. Turn interruption alone cannot stop the process, and relying on termination signals would retain the existing timeout and platform-dependent hard-stop behavior; the current fallback still handles peers that ignore EOF.

Files changed (3) +66 / -9

Bug fix (2) +45 / -9

CodexAppServerConnection.csAdd bounded, idempotent stdin closure +20/-0

Add bounded, idempotent stdin closure

• Adds CloseInputAsync to dispose only the connection's write stream, signaling EOF without stopping the stdout read loop. It coordinates with in-flight writes through a bounded gate and tolerates concurrent full disposal.

src/Capacitor.Cli.Daemon/Harness/Codex/CodexAppServerConnection.cs

CodexAppServerHostedAgentRuntime.csAlways signal app-server process shutdown +25/-9

Always signal app-server process shutdown

• Reworks graceful stop to interrupt only when a turn is active, then unconditionally close app-server stdin. Both steps remain best-effort so existing wait-and-terminate fallback behavior is preserved.

src/Capacitor.Cli.Daemon/Harness/Codex/CodexAppServerHostedAgentRuntime.cs

Tests (1) +21 / -0

CodexAppServerConnectionTests.csTest write-side EOF and idempotent closure +21/-0

Test write-side EOF and idempotent closure

• Adds an in-memory pipe test proving writes work before closure, the peer observes EOF afterward, and repeated CloseInputAsync calls do not throw.

test/Capacitor.Cli.Daemon.Tests.Unit/Harness/Codex/CodexAppServerConnectionTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Shutdown comment records history ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The added shutdown comment narrates the previous implementation, park-time failure, SIGKILL exit
code, and observed delay instead of documenting only the current invariant. This development history
violates the requirement that comments remain current, concise, and nonhistorical.

Code

src/Capacitor.Cli.Daemon/Harness/Codex/CodexAppServerHostedAgentRuntime.cs[R613-616]

+        // exits cleanly (code 0) on stdin EOF. Previously this method sent only `turn/interrupt`, and ONLY
+        // when a turn was in flight, so a between-rounds PARK (the reviewer is idle) made it a total no-op:
+        // the process never exited, the daemon waited out the full 15s graceful window and SIGKILLed
+        // (exit 137), delaying AgentUnregistered ~16s on every park. Closing stdin lands the exit inside

Evidence

PR Compliance ID 26 prohibits comments that narrate how code evolved or retain historical
measurements. The added block explicitly says Previously, describes the old no-op behavior, and
records SIGKILL/exit-137 and ~16s incident details.

CLAUDE.md: Write Only Current, Necessary, Nonhistorical Comments
src/Capacitor.Cli.Daemon/Harness/Codex/CodexAppServerHostedAgentRuntime.cs[612-618]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The graceful-stop comment records the prior implementation and historical failure measurements rather than only the durable behavior required by the current code.

## Issue Context
Retain at most a concise explanation that stdin EOF requests app-server process shutdown and that hard termination remains the fallback; remove the `Previously` narrative, park incident details, exit code, and observed delay.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Harness/Codex/CodexAppServerHostedAgentRuntime.cs[612-618]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: ⚖️ Balanced

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗