code-mode: make session shutdown authoritative#29287
Conversation
2eb9c28 to
82722d4
Compare
0118f65 to
442d204
Compare
| Err(error_text) => RuntimeCommand::ToolError { id, error_text }, | ||
| let command = tokio::select! { | ||
| biased; | ||
| _ = cancellation_token.cancelled() => { |
There was a problem hiding this comment.
Can we avoid dropping the delegate future as soon as this token fires? The delegate API currently promises cooperative token cancellation, not cancellation-safe Drop, so this can skip async cleanup and let provider-owned work survive after shutdown returns
There was a problem hiding this comment.
Yeah, I like your proposed structure better. Co-operative cancelation will work better in the split env.
I'm a little stressed that a wedged delegate call can hold open a terminating session indefinitely, but we can perhaps enforce this with timeouts or other forms of exceptional ungraceful termination.
Updated
| DelegateEvent::NotificationStarted | ||
| ); | ||
|
|
||
| let shutdown_service = Arc::clone(&service); |
There was a problem hiding this comment.
Could these 3 100ms timeouts use the existing 1s/2s test bound? They include V8 termination and actor scheduling, so 100ms will be flaky
82722d4 to
728e1ad
Compare
442d204 to
ebc9240
Compare
2 participants
Summary
Why
A best-effort registry scan can miss cells admitted concurrently or blocked behind the registry lock.
Impact
Session shutdown reliably stops every admitted cell and rejects new work once shutdown begins.
Validation
just test -p codex-code-mode -p codex-code-mode-protocol(70 passed).cconger/code-mode-runtime-compact-03c-terminal-state.