Conversation
|
@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: 771768b755
ℹ️ 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. Can't wait for the next one! 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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f62b8dba27
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Adds durable recording of controlled-session resource ownership (controller/workload container IDs + channel directory + identities) into the existing live-run queue so crash/recovery tooling can reliably reconcile inert/active resources, and removes that record on verified cleanup.
Changes:
- Extend the live-run queue schema to include
controlled_sessionsownership records, with validation and canonical persistence. - Update controlled-session supervision to require an admitted
OperationLock, persist ownership before starting either process, and complete/remove ownership on verified cleanup. - Add unit/integration tests covering persistence ordering, lock-directory mismatch cleanup, and retention on cleanup failure; update design docs.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/dockerdeploy/controlled_session_workload_pty.go | Exposes workload container ID for ownership recording. |
| internal/dockerdeploy/controlled_session_controller.go | Exposes controller container ID for ownership recording. |
| internal/dockerdeploy/controlled_session_supervisor.go | Persists ownership under the admitted operation lock before process start; completes/removes ownership on verified cleanup. |
| internal/dockerdeploy/controlled_session_supervisor_test.go | Adds unit coverage for “persist before start” and retention behavior. |
| internal/dockerdeploy/controlled_session_supervisor_integration_test.go | Updates integration test to admit a live run/lock and verify ownership removal on successful cleanup. |
| internal/deploy/live_run_queue.go | Adds controlled-session ownership types + validation to the live-run queue schema. |
| internal/deploy/live_run_queue_file.go | Adds RecordControlledSessionOwnershipV1 and CompleteControlledSessionV1 lock APIs and updates queue-file lifecycle. |
| internal/deploy/live_run_queue_file_test.go | Adds tests for ownership recording, write-failure preservation, and recovery retention. |
| docs/CONTROLLED_SESSION_DESIGN.md | Updates design doc to describe durable ownership recording. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| controlled-session networking remains a later phase. | ||
| Before either process starts, the exact full controller and workload | ||
| container IDs, their planned ownership identities, the private channel | ||
| directory, and the session, lease, and boot identities are now durably |
0b66a03 to
e5e7027
Compare
b7fd2d2 to
f42c190
Compare
Persist controlled-session ownership in both deployment live-run queues for the full supervisor lifetime. Bind controller and workload resources to one run identity, enforce admission conflicts, and add recovery and cleanup coverage for abandoned or competing sessions.
No description provided.