code-mode: release acknowledged terminal observations#29401
Closed
cconger wants to merge 1 commit into
Closed
Conversation
ed0bc3b to
ba7fa7b
Compare
ea2eb64 to
c3c7e84
Compare
ba7fa7b to
846a04a
Compare
c3c7e84 to
63a5d00
Compare
846a04a to
8ec0e02
Compare
3986a35 to
ec57b61
Compare
12fe361 to
74b9705
Compare
ec57b61 to
4f4da26
Compare
74b9705 to
aaa1d51
Compare
4f4da26 to
735f23f
Compare
This was referenced Jun 22, 2026
aaa1d51 to
ffe72de
Compare
735f23f to
1d6187f
Compare
1d6187f to
82f86d9
Compare
ffe72de to
e7f4fc2
Compare
jif-oai
reviewed
Jun 22, 2026
| request.cell_id | ||
| ) | ||
| })?; | ||
| if !matches!( |
Collaborator
There was a problem hiding this comment.
A yielded generation also has no successor after wait(... terminate: true) no? should we retire this too?
| if is_terminal | ||
| && let Err(error) = self | ||
| .session()? | ||
| .release_observation(codex_code_mode::ReleaseObservationRequest { |
Collaborator
There was a problem hiding this comment.
This destructive ack happens before Core commits N + 1. This is racy
| request: ReleaseObservationRequest, | ||
| ) -> Result<(), String> { | ||
| let mut observations = self.observations.lock().await; | ||
| let Some(record) = observations.get(&request.cell_id) else { |
Collaborator
There was a problem hiding this comment.
None cannot be an idempotent old release here because released generations remain as tombstones. This accepts any generation for an unknown or never-observed cell and hides wire desync. Should we reject this case instead?
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
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.
Why
An
ObserveOutcome::Yieldedvalue is implicitly acknowledged by the next generation.ObserveOutcome::CompletedandObserveOutcome::Terminatedhave no successor, so their replay payload otherwise remains retained until the session ends.What
ReleaseObservationRequest { cell_id, generation }andrelease_observation(ReleaseObservationRequest) -> Result<(), String>to the wire/session contract.ObserveOutcomeisCompletedorTerminated.YieldedorMissingoutcomes; reject them as non-terminal.ObserveOutcome.Host/Core wire sequence
sequenceDiagram participant Core participant Host Core->>Host: ObserveRequest { cell_id, generation, ... } Host-->>Core: ObserveOutcome::Completed or ::Terminated Core->>Core: consume terminal outcome Core->>Host: ReleaseObservationRequest { cell_id, generation } Host->>Host: replace payload with generation tombstone Host-->>Core: Result<(), String> opt release returns Err(String) Core->>Core: warn; consumed outcome remains valid endStack boundary
release_observation(ReleaseObservationRequest)acknowledges transport delivery of terminal output. It is separate fromresume(PendingGeneration), which advances a pausable runtime frontier.Validation
just test -p codex-code-modejust test -p codex-code-mode-protocolStack parent: #29398.