Fix /codex:stream Monitor-kill safety + sync upstream + CODEX_HOME-aware broker - #1
Merged
Merged
Conversation
Host plugin runtimes (e.g. Claude Code) inject CLAUDE_PLUGIN_DATA and
CODEX_COMPANION_SESSION_ID into the plugin process. When these leak into
`npm test`, two tests fail with non-bug assertions:
- tests/state.test.mjs: "resolveStateDir uses a temp-backed per-workspace
directory" — CLAUDE_PLUGIN_DATA redirects state to ~/.claude/plugins/data
instead of os.tmpdir().
- tests/runtime.test.mjs: "result without a job id ..." — fixture writer
and the spawned `result` subprocess look at different state dirs.
CI passes (no host-runtime env), so this only bites contributors running
`npm test` from inside a plugin host. Fix: delete these env vars at the top
of tests/helpers.mjs (the shared import) so every test gets a clean slate.
Tests that exercise the env var explicitly ("resolveStateDir uses
CLAUDE_PLUGIN_DATA when it is provided") already use try/finally to set and
restore it, so they continue to pass.
Verified: 86/86 tests pass with the polluting env vars still set in the
parent shell.
Add resolveJobEventsFile / appendJobEvent / readJobEvents for the upcoming
Claude-main-loop observability work (see DESIGN doc at user planning area).
Contract:
- Per-job events file at {stateDir}/jobs/{jobId}.events.ndjson.
- appendJobEvent caps each line at 4KB. POSIX `write(fd, buf, n)` with
O_APPEND is atomic when n < PIPE_BUF (~4096 on Linux/macOS), so single-
write append is safe across concurrent readers. Oversized lines first
truncate the `raw` field; if still too big, the whole event is replaced
with a minimal {type: "oversize-event-elided", seq, ts, method, phase}.
- readJobEvents returns [] for missing files. Partial last line (writer
mid-write before the trailing \n) is tolerated — JSON.parse failure on
the trailing line is skipped, next read picks it up. afterSeq takes
precedence over since when both are passed.
This commit is consumer-less; the producer side (captureTurn onNotification
hook) lands in the next commit so this slice can ship green independently.
Tests: 10 new unit tests covering path resolution, append+read, missing
file, afterSeq filter, since filter, afterSeq precedence over since, limit,
partial-line tolerance, raw truncation, and oversize elision. 96/96 pass.
Three changes to codex.mjs, building on the per-job NDJSON event API:
1. New exported `normalizeNotification(state, message)` function — pure
transform from app-server notifications to the flat event shape that
gets appended to {jobId}.events.ndjson. Covers thread/started,
thread/name/updated, turn/started, item/started, item/completed,
error, turn/completed, and an "unknown" fallback for forward-compat
methods (e.g. thread/compact/started). Phase inference reuses the
existing describeStartedItem / describeCompletedItem maps so the
vocabulary stays consistent with on-screen progress text.
2. `captureTurn` accepts an `options.onNotification` callback. A small
`dispatch(message)` wrapper applies state mutation first, then emits
the normalized event. Order is load-bearing: normalize reads
state.threadTurnIds populated by applyTurnNotification. onNotification
errors are swallowed so a broken consumer can never crash the worker.
3. `runAppServerTurn` forwards `options.onNotification` to captureTurn
and surfaces `usage` (from turnState.finalTurn?.usage) as a top-level
field on the result, so /codex:status and the events stream can read
token usage without traversing the nested `turn` payload.
Tests: 13 new unit tests covering each method branch, phase inference
edge cases (commandExecution -> running vs verifying), unknown-method
fallback, and malformed-input resilience. 109/109 pass.
No behavioral change for callers that don't pass onNotification — the
hook is fully opt-in. The companion-side wiring lands separately.
End-to-end wiring of the observability path landed in the prior two commits
(state.mjs events API + codex.mjs notification hook).
In codex-companion.mjs:
- executeTaskRun: forward `onNotification` through to runAppServerTurn.
Sync foreground callers still don't pass one; only the background
task-worker path opts in.
- handleTaskWorker: build a same-process closure that
(a) appends every normalized notification to {jobId}.events.ndjson
with a monotonic per-job seq number;
(b) reflects phase transitions into state.json via upsertJob (only on
real phase change, to keep the unlocked single-flight writer rare);
(c) runs a 5s stall watchdog — when the gap since the last event
exceeds CODEX_COMPANION_STALL_SECONDS (default 60s), it emits one
{type:"watchdog", phase:"stuck"} record and flips job phase to
stuck. It does NOT cancel. Main-loop Claude decides what to do
next (continue / compact / cancel).
(d) on terminal exit (success OR failure), emits a single
{type:"job/exited", phase:"completed"|"failed", exitCode, errorMessage}
record so a polling reader can distinguish "still slow" from
"already finished". The job.status state.json field is no longer
the sole source of truth — main-loop Claude must look for the
job/exited event.
Observability errors are swallowed; they must never crash the worker.
Watchdog interval is unref'd so it can't keep the event loop alive.
- New `events <job-id> [--since|--after-seq|--limit] [--json]` subcommand
delegating to readJobEvents. Default output is human-readable lines;
--json returns {jobId, eventsFile, count, events}. After-seq + since
semantics match readJobEvents (after-seq takes precedence). printUsage
updated.
Tests: 7 integration tests spawning the real CLI to verify the events
command's empty-result, append-and-read, after-seq filtering, limit
capping, human-readable rendering, and missing-jobId usage paths.
116/116 pass.
The stall threshold is configurable via env var for now
(CODEX_COMPANION_STALL_SECONDS). A `task --max-stall-seconds <N>` CLI
flag is deferred to a later commit so handleTask's option parser stays
untouched in this slice.
E2E against real codex CLI 0.131.0-alpha.9 uncovered three issues mock
tests could not surface. Fixed all three; rerun confirms zero `unknown`
phase events in a successful turn.
1. handleTaskWorker job/exited bug (correctness)
runTrackedJob does NOT throw when codex returns a failed turn — it
resolves with execution.exitStatus != 0 and writes state.status="failed"
out-of-band. Previous code keyed `completed` off the absence of an
exception, so a failed turn was misreported as `phase:"completed",
exitCode:0` in the terminal job/exited event. This is the single
record main-loop Claude reads to decide "did this work?" — getting
it wrong led to silent false positives. Fix: inspect
execution.exitStatus directly. Bug repro: dispatch with --effort
minimal (which conflicts with web_search) — codex returns an
invalid_request_error, runTrackedJob resolves with exitStatus=1,
stored state.json shows status="failed", but the prior code emitted
{phase:"completed", exitCode:0}. After fix: {phase:"failed",
exitCode:1, errorMessage:"Task did not complete successfully ..."}.
2. normalize coverage gaps (forward-compat / readability)
codex 0.131 emits three notification methods + three item types the
prior switch did not recognize, leaving them as phase:"unknown" with
generic fallback messages:
- method `thread/status/changed` with status.type in {"idle"} →
phase:"idle", message "Thread idle.". Post-turn quiescence signal.
- method `thread/tokenUsage/updated` → phase:"metering" (new word in
the phase vocab). Streaming token-usage source. This is the real
event main-loop Claude should poll to detect context-budget pressure
before turn/completed surfaces final usage. NOTE: codex 0.131 payload
shape for the usage object is not documented; normalize tries
{inputTokens,outputTokens,cachedInputTokens} and several aliases;
when no recognized keys are found, falls back to a stable label and
preserves raw. Concrete schema discovery is a Phase 3 follow-up.
- item.type `agentMessage` / `assistantMessage` / `reasoning` →
phase:"thinking". `agentMessage` is codex's final-reply item;
describeStartedItem/describeCompletedItem now use a new
extractItemText helper to surface a content preview ("Codex
replied: pong") so main-loop Claude can recognize the answer
from the event stream without fetching /codex:result.
3. Single E2E rerun verification
Same prompt with --effort low (avoids the minimal+web_search
constraint): 13 events emitted, phase distribution {thinking:8,
completed:2, idle:1, metering:1, warning:1, unknown:0}. The prior
normalize generated 5 unknown out of 9 events (56%); this commit
brings it to 0/13 on the happy path.
Tests: +6 new normalize unit tests (status idle, tokenUsage with usage,
tokenUsage without usage, agentMessage with text, agentMessage with
content[].text, schema fallback). 128/128 pass total. The job/exited
fix is exercised in the existing fake-codex fixture indirectly; a
direct unit test would need a fake runTrackedJob that simulates
"resolved with non-zero exitStatus" which is non-trivial to mock — for
now the E2E repro and the inline reasoning in the comment are the
documented evidence.
Closes the "operate codex like a Claude subagent" half of the design
contract. Phase 1+2 gave the main loop an event stream; Phase 3 gives
it the slash-command surface and the protocol-native recovery path.
In lib/codex.mjs:
- New exported compactAppServerThread(cwd, {threadId}). Wraps codex
app-server's thread/compact/start RPC. This is the protocol-native
recovery for "prompt too long" — main-loop Claude calls it after a
turn fails with context-overflow, then resumes via /codex:rescue
--resume <amended prompt>. Fire-and-return: the call awaits the
app-server's ack but does not consume the streaming response. The
broker recognizes thread/compact/start as STREAMING_METHOD but
routes the stream to whoever owns it at that moment; compaction
itself completes on the codex side regardless of consumer presence.
Uses reuseExistingBroker: true so it can punch through to an
already-running broker if one exists. The exact success payload
shape is undocumented in codex CLI 0.131; result is preserved
verbatim under .result for forward-compat.
In codex-companion.mjs:
- New `compact <thread-id> [--json]` subcommand wrapping
compactAppServerThread. Plain output prints the operation result and
hints the resume flow; --json returns the full structured report.
Smoke-tested against real codex 0.131: bogus thread id correctly
returns attempted:true, compacted:false, with codex's own error
("invalid thread id") preserved under .detail.
In agents/codex-rescue.md + commands/rescue.md:
- Default execution mode flipped from foreground to background. The
prior heuristic ("small bounded => foreground; complex => background")
was the deadlock root cause: a small task that stalls is still a
deadlock, and "small" is unknowable in advance. Background is the
safe default — the main loop polls /codex:events <job-id> for
progress instead of blocking on the synchronous Bash call. --wait is
honored when the user explicitly asks for foreground.
In commands/:
- New events.md: slash command surfacing the per-job event stream.
Documents the {type:"job/exited"} terminal-state contract, the
phase:"stuck" watchdog signal, the phase:"metering" token-usage
source, and the --after-seq incremental polling pattern.
- New compact.md: slash command for the recovery sequence. Documents
the typical "cancel → compact → resume with amended prompt" idiom
main-loop Claude should follow when codex hits context overflow.
In tests/commands.test.mjs:
- Updated assertions to reflect the new background default and to
include events.md + compact.md in the commands/ file-list invariant.
The prior assertions hard-coded the prose "default to foreground" and
the file list of 7 commands; both needed to track this change.
Tests: 128/128 pass. No new tests added — the compact path is exercised
indirectly via a smoke check (bogus thread id returns the expected
structured error against the real app-server). A full E2E for compact
needs a real turn first to obtain a valid thread id; that lands in
Phase 4 alongside version bump + CHANGELOG.
Cuts the minor-version release for the observability rework (feat/event-stream-foundation). Adds /codex:events, /codex:compact, rescue defaults to --background, stall watchdog, terminal job/exited events, top-level token usage, and broader codex CLI 0.131 notification coverage. No breaking changes — all existing commands keep the same signatures and outputs; the new event stream is additive.
test: isolate from host-runtime plugin env vars
…ation feat: per-job event stream + watchdog + compact recovery (1.1.0)
…/codex:status Criterion openai#4 from the 1.1.0 design doc was only partially implemented. runAppServerTurn returned `usage: turnState.finalTurn?.usage` but codex CLI 0.131 does not put usage in the turn payload — only in separate `thread/tokenUsage/updated` notifications. So the field was always null, /codex:status had no token display, and executeTaskRun dropped usage from the stored payload entirely. Wire-through: - captureTurn state grows a `tokenUsage` field (null by default). applyTurnNotification handles `thread/tokenUsage/updated` by storing `params.usage` (or `params.tokenUsage`, or the whole `params` as a last resort) as the latest value. Codex emits cumulative totals, so each notification replaces the previous value rather than summing. - runAppServerTurn returns `usage: state.tokenUsage ?? finalTurn?.usage ?? null` so the top-level surface picks up the accumulator (and keeps the old fallback for future codex versions that put usage on turn). - executeTaskRun's stored payload now includes `usage`, and runTrackedJob mirrors it onto the upserted state record so /codex:result and /codex:status can read it without re-running the turn. - renderJobStatusReport surfaces a "Tokens: in=N out=N [cached=N]" line when the job carries usage. Real codex 0.131 nests counts under `total` (cumulative) and `last` (this-turn); the renderer prefers `total`, falls back to `last`, then to a flat shape. Field-name detection covers inputTokens/input, outputTokens/output, and cachedInputTokens/cached/cachedTokens so the row survives schema shifts. The normalize-notification message helper got the same nested-decode treatment so events-stream messages match. E2E verified against codex-cli 0.131.0-alpha.9: the real wire payload is `params.tokenUsage = { total, last, modelContextWindow }`. After the fix, /codex:status renders e.g. `Tokens: in=33227 out=130 cached=2432` end-to-end. Codex emits the event opportunistically — short turns sometimes skip it — so the renderer just omits the line when usage is null or empty. Tests: new tests/token-usage-accumulator.test.mjs covers the accumulator across single + repeated updates, missing-usage default, schema-alias fallback, and the real codex nested wrapper. tests/render.test.mjs gains six cases for the new Tokens line (present, absent, partial, alias names, nested total/last shapes). tests/normalize-notification.test.mjs gets one more case for the real nested schema's message-string output. 128 baseline → 145 total, all passing.
…sage in /codex:status)
The 1.1.0 design doc's "可测验证 openai#2" calls for an integration test that dispatches a slow codex task and asserts the events stream surfaces `phase:"stuck"`. The behavior was previously verified only by manual E2E (stallMs=8540 ms in a real codex run); this commit adds CI-gated coverage against regressions. Approach: spawn task-worker against a fake codex that responds to thread/start + turn/start then deliberately stops sending notifications. With CODEX_COMPANION_STALL_SECONDS=1 the worker should emit a watchdog event within ~5-7 seconds (setInterval ticks every 5s). The test polls events.ndjson via readJobEvents (not the CLI, to bypass any rendering layer), and asserts the stuck record's stallMs meets the configured threshold. The job is explicitly cancelled at teardown and the broker is torn down via SessionEnd. Extends tests/fake-codex-fixture.mjs with a hang-after-turn-start behavior so future watchdog / timeout tests can reuse the same hang pattern.
…ck-phase event integration test)
Long codex sessions produce events.ndjson files where the `raw` field dominates the line size (each item has the codex notification's full payload). Users tailing the stream in real time may prefer a leaner event log without raw — this opt-out adds that knob. - appendJobEvent (lib/state.mjs) strips `event.raw` to null when CODEX_EVENTS_RAW=0 is set. Default behavior unchanged (raw kept for debuggability). Only the literal string "0" disables; any other value (including "1", "false", empty) keeps raw, mirroring the existing CODEX_COMPANION_STALL_SECONDS env-var convention in this plugin. - Tests cover both the strip-on-zero path and the keep-by-default path including the alias-friendly "any non-zero value keeps raw" semantic. This is a state-layer policy, not a normalize-layer one — normalizeNotification remains a pure transform that always produces raw. Downstream consumers choose what to store; the worker's appendJobEvent is the policy enforcement point.
Patch release closing gaps identified during 1.1.0 post-merge verification: - Criterion openai#4 (token usage in /codex:status) finally end-to-end. Three independent gaps fixed across captureTurn / executeTaskRun / renderJobStatusReport. Real codex 0.131 schema (`total / last / modelContextWindow`) supported via E2E spike. - Stall watchdog regression coverage in tests/stuck-watchdog.test.mjs (extends fake-codex-fixture with hang-after-turn-start behavior). - CODEX_EVENTS_RAW=0 env var to strip raw payloads at appendJobEvent. - thread/compact/start success payload confirmed as empty object {} (compaction is async; status flows via subsequent notifications). Tests: 128 → 148 (all pass).
…2.0)
The main Claude loop now has a push contract complementing the existing
pull contract:
- `/codex:stream <prompt>` runs codex in foreground push mode. Each
notification is emitted as one NDJSON line to stdout
({"jobId","seq","ts","method","phase","message",...}). The main loop
wraps it with `Monitor({command: "node ... task-stream ..."})` — each
NDJSON line becomes a push notification, stream ends when
{type:"job/exited"} fires and the process exits.
- task-stream shares runTrackedJob + executeTaskRun + appendJobEvent
with the background path. Events are still persisted to
{stateDir}/jobs/{jobId}.events.ndjson, so /codex:events <id> and
/codex:result <id> keep working after the stream ends or a consumer
reconnects from another Claude Code session.
- Stall watchdog (default 60s, CODEX_COMPANION_STALL_SECONDS override)
and {type:"job/exited"} terminal-event semantics identical to
background path.
- /codex:stream and /codex:rescue + /codex:events produce the same
events.ndjson bytes — pick by delivery ergonomics, not capability.
Integration test: tests/task-stream.test.mjs spawns task-stream against
fake codex, asserts every stdout line is valid NDJSON with a single
jobId across all lines, monotonic seq starting at 0, terminal
{type:"job/exited"} as the last line, child exit code 0, and matching
events.ndjson on disk. 148 -> 149 tests pass.
Version bump 1.1.1 -> 1.2.0 (new feature, additive, no breaking
changes; all prior slash commands keep their signatures).
Long write-tasks under 1.2.0 (56s+ runs touching files and commands)
exposed three notification methods that normalizeNotification was
returning phase="unknown" for. raw payloads were already preserved
untouched, so the events stream stayed correct — but main-loop Claude
was missing useful phase signal that this patch restores.
- turn/plan/updated -> phase: "thinking" (plan/todo list mutated)
- turn/diff/updated -> phase: "editing" (running unified-diff
refresh, precursor to
fileChange items)
- item/commandExecution/outputDelta
-> phase: "running" (stdout chunk while
command still running)
All three normalizers prefer params.turnId but fall back to
state.threadTurnIds.get(threadId) for forward-compat with future
codex builds that may omit turnId on these specific notifications.
raw payloads are preserved verbatim — consumers that already
join raw.delta / render raw.diff / read raw.plan keep working.
Tests: 7 new cases in tests/normalize-notification.test.mjs (happy
path + fallback path per method), wire-observed payloads copied from
a real 56s codex 0.131 task trace. 156/156 pass (was 149/149).
Schema-coverage patch for the 1.2.0 push-mode E2E findings. See plugins/codex/CHANGELOG.md "1.2.1" section for the per-method detail (turn/plan/updated, turn/diff/updated, item/commandExecution/outputDelta).
1.2.1: normalize 3 codex 0.131 schema gap methods
…urn with a timeout (openai#4) codex 0.131 inline review can stream notifications on a distinct reviewThreadId without emitting turn/completed on the source thread, and review never produces an agentMessage final_answer. captureTurn would wait forever on a signal that never arrives, so /codex:review hangs both in --wait (foreground blocks Claude) and --background (/codex:status shows running plus a stuck watchdog event but the task never ends). - exitedReviewMode lifecycle=completed now triggers completeTurn, treating it as the authoritative review-finished marker regardless of which thread turn/completed lands on. - captureTurn accepts options.timeoutMs and on expiry sends a best-effort turn/interrupt and resolves with a failed turn so the caller fails cleanly with a TURN_TIMEOUT error instead of hanging. - runAppServerReview wires the timeout via CODEX_COMPANION_REVIEW_TIMEOUT_SECONDS (default 600s, 0 disables) so review always has an upper bound. Tests: two new fake-codex behaviors + runtime tests cover both paths: review-on-separate-thread (would previously hang; now completes via exitedReviewMode) and review-hang-after-start (must time out cleanly within the configured window). Full suite: 158/158 pass. Co-authored-by: bit-star <bit@gie.edu.kg>
…n the diff (openai#5) Companion to PR openai#4. Even with the captureTurn timeout, review turns were hanging in the wild because the shared codex app-server inherited the user's developer_instructions, persistent_instructions, and feature flags (memories, goals). Those nudge codex into governance routing, memory recall, and unsolicited MCP calls (e.g. gbrain/recall) that have nothing to do with the diff — turning a 60s review into a 600s timeout (or worse, a silent hang before this PR's timeout existed). - Plumb `codexArgs` through `SpawnedCodexAppServerClient.initialize` so callers can append `-c key=value` overrides to the spawned codex CLI. - `withAppServer(cwd, fn, options)` now forwards `disableBroker` and `codexArgs` to `CodexAppServerClient.connect`, and skips the broker-busy retry when `disableBroker` is explicit. - `runAppServerReview` opts in: `disableBroker: true` (a dedicated short-lived codex process for each review, so it can't reuse a polluted broker), plus `codexArgs: getReviewCodexArgs()` which silences developer_instructions, persistent_instructions, features.memories, and features.goals at the codex CLI layer. - `runAppServerTurn` is unchanged: tasks still share the broker and honor the user's global codex instructions. Tests: two new runtime tests + a fake-codex extension that records every codex app-server spawn's argv. One asserts the review path injects the four `-c` overrides; the other asserts the task path leaves them alone. Full suite: 161/161 pass. In tomtom's real environment this brings the failing /codex:review run from a 180s TURN_TIMEOUT to a clean 53s completion with a correct verdict, and codex no longer touches gbrain MCP or scans ~/.ai-operating-system. Co-authored-by: bit-star <bit@gie.edu.kg>
…penai#398, openai#447) # Conflicts: # .claude-plugin/marketplace.json # package-lock.json # package.json # plugins/codex/.claude-plugin/plugin.json # plugins/codex/scripts/codex-companion.mjs # plugins/codex/scripts/lib/codex.mjs # tests/commands.test.mjs
…ts instead of running in-process Monitor's documented contract is 'Timeout -> killed'. The previous implementation ran the Codex task directly in the process Monitor supervises, so a Monitor-side kill (timeout or flood-stop) killed the task, not just the live view. Now task-stream enqueues the same detached background worker /codex:rescue uses and only tails its events.ndjson file — killing the tail can only lose the live view. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Terminals can inject escape sequences like bracketed paste mode (\x1b[?2004h) into the Codex app-server stdout stream. These were causing JSON.parse failures that killed the entire client connection. Now strips CSI/OSC sequences before parsing, and silently skips lines that become empty after cleanup. Closes openai#23
…ake effect The broker (and the codex app-server it manages) inherits CODEX_HOME once at spawn time, and ensureBrokerSession reused any live broker for the workspace unconditionally — so a caller switching CODEX_HOME (e.g. account fallback after a rate limit) silently kept running on the original account. Store the spawning CODEX_HOME in broker.json and, when a caller arrives with a different one, gracefully stop the old broker via the existing broker/shutdown RPC and spawn a fresh one with the caller's env. Same-account callers keep reusing the warm broker; legacy sessions without the field are treated as default-account. Adds unit tests (fake broker fixture) and a README section documenting multi-account usage and its shell gotchas.
…i-account setup Addresses review: an account switch could broker/shutdown a broker that was mid-turn for another job in the same workspace. ensureBrokerSession now probes the new broker/status RPC (answered before the busy gate; older brokers degrade gracefully: their busy gate rejects the probe with BROKER_BUSY_RPC_CODE when busy, and forward it to an unknown-method error when idle). If the broker is busy, the call returns null and falls back to a directly spawned app server with the caller's env — in-flight work is never interrupted; rotation happens on the next idle call. Timeouts count as busy. README: full alias setup example (add -> source -> per-account login -> use) and the busy-aware rotation semantics.
… (ifIdle)
Closes the probe->shutdown race: broker/shutdown now accepts { ifIdle: true }
and refuses (shutdown: false, busy: true) when a request/stream is in flight —
the check and the shutdown happen in the same event-loop message handling, so
a turn that started after the caller's idle probe is never dropped. Account
rotation sends ifIdle and treats a refusal like the busy-probe result: return
null and fall back to a directly spawned app server with the caller's env.
Legacy brokers ignore the param (pre-existing behavior, no worse). Fixture can
simulate becoming busy between status and shutdown; new test covers the race.
…g entry /codex:stream's own command doc still described the pre-fix foreground behavior and omitted the one fact users most need: killing/timing out the stream no longer kills the underlying Codex task. Also adds a changelog entry for this branch's fixes (stream detached-worker safety, ANSI-strip cherry-pick, CODEX_HOME-aware broker cherry-pick) since none existed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lta floods, propagate task exit code Follow-up to the detached-worker fix per advisor review: three gaps meant the original goal (real-time visibility that survives Monitor's own limits) still wasn't reliably met for non-trivial tasks: - Every pushed NDJSON line carried the full `raw` payload (up to 4KB/event per MAX_EVENT_LINE_BYTES) — a 500-event task could push hundreds of KB into the main loop's context for no reason it needs. Stripped `raw` at the tail's stdout-write site only; the on-disk events.ndjson (written by the worker, untouched here) keeps it intact for /codex:events/-result. - item/commandExecution/outputDelta can fire many times per second for a verbose command — exactly the flood pattern that gets a Monitor watch auto-stopped (task now survives that, per the prior fix, but the live view still died, which is the actual feature). Added coalesceOutputDeltas() (plugins/codex/scripts/lib/job-event-tail.mjs) to merge consecutive same-item deltas observed in one poll batch into a single line. - handleTaskStream never set process.exitCode from the task's actual outcome — it returned as soon as the tail saw job/exited regardless of that record's exitCode, so a failed Codex turn could still exit 0. tailJobEventsToStdout now returns the terminal event's exitCode and handleTaskStream propagates it, mirroring runForegroundCommand's existing pattern for the foreground task/review commands. Also: stream.md now recommends persistent: true (default Monitor timeout caps every stream at 5 minutes otherwise) and --after-seq for reconnects instead of --since (integer comparison on a value already in every pushed line, vs. string timestamp matching). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Code review nitpick: the comment said the on-disk events.ndjson 'keeps raw intact', but appendJobEvent already truncates/elides oversized raw payloads and honors CODEX_EVENTS_RAW=0 -- neither new to this branch, but the comment shouldn't imply a stronger guarantee than exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
5 participants
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.
Summary
/codex:stream,/codex:events,/codex:compactfork (22 commits, history preserved via merge, not squash) and syncs 3 upstream commits this fork's base lacked (feat: add /codex:transfer for Claude Code session handoff openai/codex-plugin-cc#374 Claude session transfer, Update plugin version to 1.0.5 openai/codex-plugin-cc#398 version bump, Remove shell expansion for git commands openai/codex-plugin-cc#447 remove shell expansion for git commands)./codex:streamused to run the Codex task directly in the process Claude Code'sMonitortool supervises. Since Monitor's contract is "Timeout -> killed", killing the monitored process killed the underlying task, not just the live view. Rewiredtask-streamto enqueue the same detached background worker/codex:rescuealready uses, and tail itsevents.ndjsonfile instead — a Monitor-side kill now only loses the live view, never the task. (Two follow-up fixes during review: O(n²) full-file reread per poll tick → byte-offset tracking; a resulting UTF-8 multi-byte-boundary bug → persistentStringDecoder, extracted intoplugins/codex/scripts/lib/job-event-tail.mjs.)CODEX_HOME-aware broker + busy-safeifIdleshutdown, so account rotation never kills an in-flight turn).commands/stream.mdand added a CHANGELOG entry to reflect the new detached-worker architecture.Design spec:
docs/superpowers/specs/2026-08-06-stream-detached-worker-design.md. Implementation plan:docs/superpowers/plans/2026-08-06-stream-detached-worker.md.Test Plan
npm test), including a clean-install run (rm -rf node_modules && npm install && npm test)🤖 Generated with Claude Code