Skip to content

fix: do not crash when a fileChange start event omits changes - #775

Open
kevin9327 wants to merge 1 commit into
openai:mainfrom
kevin9327:fix/filechange-started-without-changes
Open

kevin9327 wants to merge 1 commit into
openai:mainfrom
kevin9327:fix/filechange-started-without-changes

Conversation

@kevin9327

Copy link
Copy Markdown

Problem

describeStartedItem reads item.changes.length for fileChange start notifications. Codex can emit item/started before the change list is present. That throws TypeError: Cannot read properties of undefined (reading 'length') inside the broker notification handler and aborts the turn.

Change

Treat a missing or non-array changes field as zero file changes when rendering the progress line.

Test

task survives fileChange started items that omit changes fails on main with the TypeError and passes after this change.

AI-assisted (Grok)

describeStartedItem assumed item.changes was always present on
item/started fileChange notifications. Codex can emit the start event
before the change list is known, which threw TypeError and aborted the
turn. Treat a missing or non-array changes field as zero file changes
so the job can finish.
@kevin9327
kevin9327 requested a review from a team September 20, 2026 15:11
ApexAiOfficial added a commit to ApexAiOfficial/codex-plugin-cc that referenced this pull request Sep 25, 2026
Audited against open upstream reports on openai/codex-plugin-cc; each defect
was reproduced or verified against this fork and has a regression test in
tests/substrate.test.mjs (dedicated fake app-server).

- openai#302 unbounded waits: every RPC has a bounded wall-clock timeout (120s
  default, command/exec aware, CODEX_COMPANION_RPC_TIMEOUT_MS, 0 disables),
  and requests on a dead connection fail immediately. The turn watchdog does
  not use a silence timeout (legitimate turns are silent for long periods):
  it fails fast when the connection closes, and after quiet periods asks the
  app-server via thread/read whether the thread is still active, recovering a
  turn whose completion event was lost and failing only an app-server that
  stops answering.
- openai#453 zombie broker: the broker exits when its app-server child dies, so the
  next caller starts a healthy broker instead of wedging.
- openai#706/openai#707 retained subscriptions: the broker tracks per-socket thread
  ownership (subagents inherit their parent's owners) and sends
  thread/unsubscribe when the last owner disconnects; requests for a thread
  wait (bounded) for its in-flight unsubscribe. Simpler than openai#707 because this
  broker already serializes active requests and streams.
- openai#762 + openai#768 together: broker acquisition is serialized; a broker whose
  process is alive is never torn down or killed because it missed a probe
  (the caller uses a private app-server); metadata is cleared only when the
  process is provably gone; only a just-spawned broker that never became
  ready is killed, identity-checked. SessionEnd's kill is identity-checked.
- openai#574 RC3: non-retryable errors fail a turn even when completion is
  inferred. The upstream fix (any error fails) is wrong: error notifications
  carry willRetry, and retried turns can succeed.
- openai#775: fileChange start events without a change list no longer throw.
- openai#740 reproduced with real Codex 0.144.1: live thread/resume ignores the
  requested sandbox in both directions (write-capable threads stayed writable
  after a read-only resume), and a per-turn override persists to later turns.
  Every turn now sends an explicit sandboxPolicy.

Also fixes a same-process withFileLockAsync bug (a second async holder
treated the first as stale) and makes the test harness stop brokers when a
test process is killed by a signal.

Validation: npm test 156/156, tsc clean, no stray processes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants