Skip to content

code-mode: clean up terminal cell dispatch gates#29310

Closed
cconger wants to merge 1 commit into
cconger/code-mode-runtime-compact-03h-public-runtimefrom
cconger/code-mode-runtime-compact-03i-terminal-cleanup
Closed

code-mode: clean up terminal cell dispatch gates#29310
cconger wants to merge 1 commit into
cconger/code-mode-runtime-compact-03h-public-runtimefrom
cconger/code-mode-runtime-compact-03i-terminal-cleanup

Conversation

@cconger

@cconger cconger commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Why

A yielded cell can complete in the background while its terminal result remains buffered. Waiting for another observation or actor shutdown before cleaning Core's dispatch state leaves one gate allocated per unobserved completion. Terminal notification can also race ahead of Core installing that gate.

What

  • Reintroduce cell_closed(&CellId) -> () on the runtime and session delegate contracts as a synchronous, non-blocking terminal notification.
  • Emit it once after terminal completion commits, even when the ObserveOutcome remains buffered.
  • Deliver an already-attached ObserveOutcome::Completed or ObserveOutcome::Terminated before invoking terminal cleanup.
  • Keep actor shutdown as an idempotent fallback for termination paths.
  • Remove Core's ready dispatch gate when the notification arrives.
  • Linearize close-before-ready by retaining a lightweight closed marker; a later readiness registration consumes the marker instead of reopening a gate.
  • Cover background completion without another observation and the close-before-ready ordering.

Host/Core sequence

sequenceDiagram
    participant Core
    participant Host
    Host->>Host: commit terminal CellEvent
    opt observer is attached
        Host-->>Core: ObserveOutcome::Completed or ::Terminated
    end
    Host-->>Core: cell_closed(&CellId) -> ()
    alt dispatch gate already exists
        Core->>Core: remove gate
    else readiness is not registered yet
        Core->>Core: retain closed marker
        Core->>Core: later readiness consumes marker
    end
    Note over Host,Core: cell_closed has no acknowledgement or retry
Loading

Stack boundary

This notification releases Core dispatch state; it does not acknowledge delivery of the buffered wire outcome. Observation replay storage is addressed separately by #29398 and #29401.

Validation

  • just test -p codex-code-mode
  • just test -p codex-core code_mode
  • Core regressions cover terminal notification, background completion without another observation, and close-before-ready.

Stack parent: #29292.

}

fn cell_closed(&self, cell_id: &CellId) {
self.close_cell(cell_id);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This still has a close-before-ready race...
A fast yield_control(); ...complete... cell can call cell_closed before mark_cell_ready_for_dispatch, so this removes nothing. Then, readiness creates a fresh gate, and the initial observe only returns thebuffered yield

@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch from 18fb5ed to 5ff1f52 Compare June 21, 2026 19:24
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch 2 times, most recently from 2626759 to 64053a7 Compare June 21, 2026 22:44
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch from 5ff1f52 to f3de9a8 Compare June 21, 2026 22:44
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch from 64053a7 to b9578cb Compare June 22, 2026 05:34
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch from f3de9a8 to 7545b94 Compare June 22, 2026 05:34
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch from b9578cb to cb43921 Compare June 22, 2026 06:48
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch from 7545b94 to 2f2de77 Compare June 22, 2026 06:48
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch from cb43921 to 9139181 Compare June 22, 2026 06:53
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch 2 times, most recently from 56dffe9 to df256e8 Compare June 22, 2026 07:02
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch from 9139181 to 86200fe Compare June 22, 2026 07:02
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch from df256e8 to 5baec19 Compare June 22, 2026 07:09
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch from 86200fe to 28b4f57 Compare June 22, 2026 07:09
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch from 5baec19 to b79d2d5 Compare June 22, 2026 07:21
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch from 28b4f57 to 2cb2170 Compare June 22, 2026 07:22
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch from b79d2d5 to f12f4c9 Compare June 22, 2026 07:49
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch from 2cb2170 to 474422b Compare June 22, 2026 07:49
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch from f12f4c9 to fb775b6 Compare June 22, 2026 08:37
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch from 474422b to abbe7df Compare June 22, 2026 08:37
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03h-public-runtime branch from fb775b6 to 1b0c2f7 Compare June 22, 2026 08:56
@cconger cconger force-pushed the cconger/code-mode-runtime-compact-03i-terminal-cleanup branch from abbe7df to 28536e0 Compare June 22, 2026 08:56
DispatchGate::ClosedBeforeReady => {}
},
Entry::Vacant(entry) => {
entry.insert(DispatchGate::ClosedBeforeReady);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Vacant is not necessarily “closed before ready.” InitialObservationGuard already closes the ready gate before asynchronously terminating; the later cell_closed (or the guard if runtime close wins) lands here and installs a marker after the only readiness call has passed.
Nothing consumes it, so cancelled initial observations still leak one entry each.

Can we linearize the ready, terminal, and consumer-finished events and cover this cancellation path?

.map(|observer| (observer.mode, observer.response_tx)),
) {
);
if completion_committed {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A successful commit can still produce CompletionDelivery::Rejected: terminate() may claim CellPhase::Completed between the commit await and deliver_completion. Sound pretty racy to me

@cconger cconger closed this Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants