Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a concurrency-safe, host-owned lifecycle state machine for controlled sessions, formalizing transitions from preparing → active → terminating → terminated while ensuring the first accepted termination cause is latched and preserved across later observations/requests.
Changes:
- Added
MachineV1lifecycle state machine with validated observations/requests and deterministic cause latching. - Added unit tests covering concurrency races, authorization immutability, finalization timeout behavior, and idempotence guarantees.
- Updated controlled session design documentation to reflect the expanded set of termination causes (including controller-requested termination).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/controlledsession/lifecycle.go | Adds the V1 lifecycle state machine, observation handling, request handling, and snapshot/transition helpers. |
| internal/controlledsession/lifecycle_test.go | Adds coverage for lifecycle races, termination cause latching, authorization enforcement, and finalization/timeout behaviors. |
| docs/CONTROLLED_SESSION_DESIGN.md | Updates lifecycle/termination-cause documentation to match the new design semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| transition.After = machine.state | ||
| transition.Cause = machine.cause | ||
| transition.AwaitingControllerFinalization = machine.waitingFinalize | ||
| transition.RequestAccepted = true | ||
| return transition, nil |
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
|
@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: 93b736e2d4
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b862992fe3
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e8814155a
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 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". |
Add the concurrency-safe host-owned lifecycle machine for preparing, active, terminating, and terminated sessions. Latch the first accepted termination cause, enforce granted controller requests, preserve explicit completion across later disconnects, permit bounded controller finalization after application exit, and prevent terminal status reports from rewriting observed outcomes. Reject startup failures after controller activation. Ensure host cancellation, runtime-observation loss, and controller-requested termination end a pending controller-finalization wait without rewriting the already-latched cause. Add lifecycle race, authorization immutability, disconnect, timeout, idempotence, abortive-finalization, startup-boundary, and hostile-result coverage. Clarify controller-requested termination as a distinct design cause.
Add the concurrency-safe host-owned lifecycle machine for preparing, active, terminating, and terminated sessions. Latch the first accepted termination cause, enforce granted controller requests, preserve explicit completion across later disconnects, permit bounded controller finalization after application exit, and prevent terminal status reports from rewriting observed outcomes.
Reject startup failures after controller activation. Preserve an active bounded controller-finalization wait across repeated controller termination, host cancellation, and runtime-observation-loss signals without rewriting the already-latched cause. Skip that wait when the controller was not granted the
completeoperation, allowing the terminal result to recordnot-completedimmediately.When runtime-observation loss occurs before workload output finalization, track it independently from the first latched termination cause, reject a later
drainedclaim, and require explicit failed finalization because complete delivery is no longer verifiable.Add lifecycle race, authorization immutability, disconnect, timeout, idempotence, bounded-finalization, runtime-loss output integrity, startup-boundary, no-complete authorization, and hostile-result coverage. Clarify controller-requested termination as a distinct design cause.
Stack created with Sapling. Best reviewed with ReviewStack.