Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes cancellation at the queued admission boundary deterministic by introducing an internal ready reservation state and requiring the owning caller to explicitly “claim” that reservation as the admission linearization point.
Changes:
- Introduce/extend
readysemantics in the live-run queue and add an explicitActivateReadyLiveRunV1transition. - Update admission loops (live runs and control markers) to claim
readyunder the operation lock, ensuring cancellation before/after the claim has deterministic outcomes. - Hide internal
readyfrom user-facing reporting (runs list, stop results) and expand unit tests + docs to match the new behavior.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/dockerdeploy/live_runs.go | Remaps internal ready to public waiting for list/stop results. |
| internal/dockerdeploy/live_runs_test.go | Updates expectations and adds coverage for stopping a ready reservation. |
| internal/dockerdeploy/live_run_container_test.go | Adjusts promotion assertions to reflect ready reservation behavior. |
| internal/dockerdeploy/live_run_admission.go | Claims ready as the admission point and updates wait notices to describe queued blockers. |
| internal/dockerdeploy/live_run_admission_test.go | Adds deterministic cancellation race tests and wait-notice coverage for ready predecessors. |
| internal/dockerdeploy/current_workload_run_test.go | Updates queue status expectations to ready after lifecycle phases. |
| internal/dockerdeploy/control_admission.go | Claims ready for control marker admission and makes cancellation deterministic around the claim. |
| internal/dockerdeploy/control_admission_test.go | Adds deterministic cancellation race tests for control admissions and updates expectations. |
| internal/dockerdeploy/control_admission_modes.go | Counts ready alongside waiting for disruption accounting. |
| internal/deploy/live_run_queue.go | Reworks queue validation/promotion around a single ready reservation and adds activation API. |
| internal/deploy/live_run_queue_test.go | Updates FIFO/promotion tests and adds coverage for canceling unclaimed ready runs. |
| internal/deploy/live_run_queue_file.go | Adds OperationLock.ActivateReadyLiveRunV1 to persistently claim reservations. |
| internal/deploy/live_run_queue_file_test.go | Updates atomic transition tests to account for ready then explicit activation. |
| docs/CONTROLLED_SESSION_DESIGN.md | Documents ready as the authoritative admission point and its cancellation semantics. |
| docs/BLUEPRINT_ENVIRONMENT_MODEL.md | Documents internal ready reservation while keeping public waiting/active model. |
| docs/BACKLOG.md | Removes the backlog item now implemented by this PR. |
| docs/APT_PROVIDER_DETAIL_DESIGN.md | Updates queue model documentation to include internal ready reservation. |
| .changes/unreleased/+deterministic-admission-cancellation.yaml | Adds a release-note fragment describing the behavioral fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ahead := queue.Runs[:candidateIndex] | ||
| active := deploy.LiveRunV1{} | ||
| blocker := ahead[0] | ||
| for _, entry := range ahead { | ||
| if entry.Status == deploy.LiveRunStatusActiveV1 { | ||
| active = entry | ||
| if entry.Status == deploy.LiveRunStatusReadyV1 { | ||
| blocker = entry | ||
| break |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
d0f5b68 to
c23abb1
Compare
29d2c85 to
ca3020a
Compare
7d49f06 to
a7c45ba
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb91025497
ℹ️ 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".
Stack created with Sapling. Best reviewed with ReviewStack.