Skip to content

fix(server): bound Codex subagent progress events - #5682

Open
Andrey170170 wants to merge 4 commits into
pingdotgg:mainfrom
Andrey170170:fix/codex-subagent-progress-backpressure
Open

fix(server): bound Codex subagent progress events#5682
Andrey170170 wants to merge 4 commits into
pingdotgg:mainfrom
Andrey170170:fix/codex-subagent-progress-backpressure

Conversation

@Andrey170170

@Andrey170170 Andrey170170 commented Aug 8, 2026

Copy link
Copy Markdown

Closes #5681

What Changed

  • Coalesce normal Codex child item and cumulative token-usage progress at the provider runtime boundary.
  • Preserve independent latest-item and latest-usage lanes per child thread.
  • Throttle normal progress through the shared keyed worker, while allowing a terminal child lifecycle event to flush that child's latest progress immediately.
  • Add priority-flush and scope-ownership semantics to KeyedCoalescingWorker without changing existing drainKey behavior.
  • Bind the progress worker to the Codex runtime scope so explicit close cancels pending best-effort progress before event queues shut down.
  • Add focused coverage for burst coalescing, latest-value preservation, progress-before-terminal ordering, cross-key priority flush, and scope-close interruption.

Why

Native Codex subagent observability currently emits every child item/started, item/completed, and cumulative token-usage snapshot as a separate provider event. The adapter maps each event to task.progress, and provider ingestion persists every update as a durable activity append.

Stable task activity IDs keep projected row count small, but do not reduce event-store or command-processing work. A busy subagent fleet can therefore fill the serial ingestion path with redundant progress snapshots and delay unrelated threads.

Coalescing belongs at the provider boundary because these events are latest-state snapshots: the UI needs the newest item summary and cumulative usage, not every intermediate value. Terminal lifecycle events still flush the latest pending values first, so task state cannot overtake its final progress.

Normal progress is throttled per Codex session. Terminal flushes intentionally bypass the debounce and may emit up to the latest item and usage snapshots immediately; this avoids cross-child head-of-line blocking without claiming an absolute rate cap over lifecycle flushes.

Validation

  • node_modules/vite-plus/bin/vp test run packages/shared/src/KeyedCoalescingWorker.test.ts apps/server/src/provider/Layers/CodexCollabRuntime.integration.test.ts
    • 2 test files passed
    • 6 tests passed
  • Targeted vp lint --type-aware --type-check --report-unused-disable-directives on all four changed files passed.
  • Targeted vp fmt --check on all four changed files passed.
  • git diff --check passed.

UI Changes

None.

Scope

This PR does not change raw assistant streaming, global orchestration scheduling, projectors, SQLite cleanup, or UI behavior. It addresses only Codex child progress amplification and the worker semantics required for correct terminal flushing.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I added focused regression coverage
  • No UI screenshots or interaction video are needed

Related

Prepared with Codex CLI (GPT-5.6-sol).


Note

Medium Risk
Touches provider event ordering and ingestion volume for collab children; terminal flush ordering is correctness-sensitive but scoped to Codex runtime and covered by integration tests.

Overview
Codex collab child progress (collabAgent/item, collabAgent/tokenUsage) is no longer emitted on every wire notification. Updates are merged per child thread into separate latest-item and latest-usage lanes, debounced (~250ms) through a session-scoped KeyedCoalescingWorker, then emitted as provider events.

Terminal lifecycle (turn/completed, non-active thread/status/changed, thread/closed, terminal child error) calls flushKey so the newest pending progress is delivered before idle/complete/close events, without waiting behind other children. Normal flushes skip the debounce sleep.

KeyedCoalescingWorker gains flushKey (priority processing for one key, context.flush on process) and documents that drainKey behavior is unchanged. Closing the worker’s scope interrupts in-flight work.

Session close now Scope.close(runtimeScope) before session/closed and queue shutdown so coalesced progress cannot enqueue after teardown.

Integration and unit tests cover burst coalescing, latest-value retention, progress-before-terminal ordering, cross-key flush, and scope-close interruption.

Reviewed by Cursor Bugbot for commit c74ae21. Configure here.

Note

Bound Codex subagent progress events with coalescing and flush-before-terminal semantics

  • Collab child progress notifications (collabAgent/item and collabAgent/tokenUsage) are now coalesced via a keyed worker with a 250ms cooldown, emitting only the latest item and token usage per child within each window.
  • Before emitting terminal child lifecycle events (turn completed, status changed to non-active, thread closed, terminal errors), the latest coalesced progress is flushed synchronously to preserve ordering.
  • Extends KeyedCoalescingWorker in KeyedCoalescingWorker.ts with a flushKey method that bypasses the queue and cooldown for a specific key, passing a flush-context flag to the process callback.
  • Session close now shuts down the runtime scope before emitting session/closed, cancelling any pending best-effort progress emissions.
  • Behavioral Change: child progress events may be delayed up to ~250ms and burst updates are collapsed to a single emit; consumers will see fewer, coalesced events rather than every individual update.

Macroscope summarized d765754.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5cbe578b-4cd0-4c2a-9d64-5d3be26673c7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 8, 2026
Comment thread packages/shared/src/KeyedCoalescingWorker.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c74ae21515

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Effect.flatMap((nextValue) =>
nextValue === null ? Effect.void : processKey(key, nextValue),
Effect.flatMap((next) =>
next === null ? Effect.void : processKey(key, next.value, next.flush),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Yield hot keys before processing their next value

When one Codex child produces progress at least once per 250 ms debounce window, latestByKey remains populated and this recursive call immediately processes that same key again instead of returning it to the queue. Normal progress for every other queued child can therefore remain stale indefinitely until the hot child pauses or a terminal event invokes flushKey; requeue the next value so the worker can service keys round-robin.

AGENTS.md reference: AGENTS.md:L142-L142

Useful? React with 👍 / 👎.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c74ae21. Configure here.

Effect.catchCause(() => cleanupFailedKey(key)),
),
),
Effect.andThen(drainKey(key)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flush swallows interrupt on close

High Severity

flushKey wraps processKey in Effect.catchCause that recovers from all causes, including fiber interrupts. Codex runs flushKey on the notification consumer fiber and now closes runtimeScope before queue shutdown, so a close that lands mid-flush can be swallowed. The consumer then keeps waiting on serverNotifications while Scope.close waits for that fiber, deadlocking session teardown.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c74ae21. Configure here.


return [
{ key, value } as const,
{ key, value, flush: state.flushRequestedKeys.has(key) } as const,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flush races with stale queue take

Medium Severity

flushKey can steal a queued key and run processKey on the caller fiber while leaving a stale TxQueue entry. The queue consumer only checks whether latestByKey still has a value, not whether the key is already in activeKeys. A concurrent enqueue during that flush can repopulate latestByKey, so the stale take starts a second processor for the same key and can clear activeKeys early, letting drainKey finish before progress emission completes.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c74ae21. Configure here.

@macroscopeapp

macroscopeapp Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

1 blocking correctness issue found. This PR introduces a new concurrent KeyedCoalescingWorker pattern to debounce subagent progress events with flush/drain semantics, and modifies session lifecycle timing. Multiple unresolved review comments flag potential deadlocks and race conditions in the new concurrency logic. The substantial new runtime behavior and open correctness concerns warrant human review.

You can customize Macroscope's approvability policy. Learn more.

Comment thread apps/server/src/provider/Layers/CodexSessionRuntime.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Codex subagent progress floods orchestration ingestion and delays unrelated threads

1 participant