Conversation
Route persistent workloads and transient application commands through one validated sandbox plan. Rename the system-install identity field to account and cover renderer parity and schema migration.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
Reject runtime host sources that resolve to the filesystem root or protected kernel system trees. Use platform-specific filesystem and mount topology identity to catch direct, aliased, and nested kernel-control filesystems while preserving explicit ordinary directory binds with nested application objects. Reject unsupported remote Docker endpoints before runtime and recovered-container operations, including admission cleanup paths. Document the narrowed direct-bind security contract, close the corresponding backlog item, and add a security changelog fragment.
Add portable public and local runtime-network controls that default to deny, compile into the locked runtime policy, and grant declared inbound ports only to the persistent workload shape. Classify translation and tunneling ranges conservatively, require both ordinary grants by default, and provide a documented temporary escape hatch for environments that explicitly need the ambiguous class. Install IPv4 and IPv6 nftables policy through the trusted startup helper, retain declared-only inbound admission even when egress is unrestricted, then irreversibly drop setup capabilities and assume the final application identity for workloads, commands, shells, lifecycle actions, and private-environment execution. Preserve declared endpoint traffic, constrain Docker DNS, admit only established responses plus related ICMP network errors, and surface setup failures with their backend diagnostics. Add unit and live Docker coverage for all policy combinations, public exceptions, ambiguous ranges, declared and undeclared inbound ports, root and non-root execution, DNS, endpoint persistence, and transient isolation. Run the live policy matrix in CI, document the security boundary and temporary escape hatch, update the deferred gateway backlog, and revise the security changelog fragment.
Add immutable session authorization records that bind opaque handles to exact deployments, generations, builds, admitted runs, container plans, runtime identities, controller operations, and declared endpoints. Extract the canonical container runtime identity contract so blueprint resolution, application sandbox planning, and controlled sessions share the same portable user, numeric ID, root-group, and supplementary-group validation. Reuse the deployment live-run ID validator instead of defining another controlled-session grammar. Keep connection ownership and lifetime as non-transferable host lease state: one controller connection owns one admitted live run until disconnect or host cancellation. Document that boundary and prevent authorization digests from implying reconnect or lease extension. Derive canonical authorization digests and add deterministic handle, shared identity, and adversarial validation coverage.
Define the versioned typed frame codec and shared lifecycle result types used between the host supervisor and controller. Give every V1 frame and event an explicit stable wire assignment while preserving terminal bytes exactly and validating structured payloads, direction, dimensions, endpoint identifiers, protocol version, duplicate JSON fields, and bounded payload length. Treat controller completion as finalization rather than a termination cause, and model bounded workload-output finalization explicitly in the opened contract, events, and final result. Align the controlled-session design and follow-up networking backlog with the temporary coarse lease-network grant and the planned gateway-based direct path. Add stable-wire, round-trip, malformed-frame, direction-confusion, duplicate-field, binary-data, output-finalization, and fuzz-seed coverage.
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 a backend-neutral bridge that dispatches typed controller requests through lifecycle authorization, applies accepted PTY input and resize operations, and forwards exact ordered output with bounded backpressure. Give validated lifecycle events a separate prioritized write admission path without allowing callers to forge opened or output events. Preserve explicit cancellation, disconnect, and output-finalization diagnostics while leaving lifecycle and resource ownership to the host supervisor. Cover authorization rejection, binary output, lifecycle priority and cancellation, slow consumers, disconnects, resize, ordinary Ctrl-C, and exact Docker exit behavior with race-enabled unit and live integration tests. Update the controlled-session implementation status.
There was a problem hiding this comment.
Pull request overview
This PR makes controlled sessions “fail closed” when the cleanup watchdog process exits unexpectedly, distinguishing this from Docker runtime-observation loss and surfacing a dedicated lifecycle cause/observation plus a safe diagnostic.
Changes:
- Extend the watchdog runtime interface to expose
Done(),ExitError(), andClose()so the supervisor can detect and react to premature watchdog exit. - Add a new lifecycle observation/cause (
cleanup-containment-lost) and update the Docker supervisor to observe watchdog exit both during startup and while active, emitting a bounded diagnostic event when possible. - Add/extend unit + integration tests, update design docs, and include a Changie fragment for the behavior change.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/dockerdeploy/controlled_session_watchdog.go | Extends watchdog runtime interface for exit observability and explicit closure. |
| internal/dockerdeploy/controlled_session_watchdog_linux.go | Implements watchdog exit tracking (Done/ExitError) and idempotent Close; refactors readiness/teardown coordination. |
| internal/dockerdeploy/controlled_session_watchdog_integration_linux_test.go | Updates integration test to use the new watchdog exit channel/error surface. |
| internal/dockerdeploy/controlled_session_supervisor.go | Observes watchdog exit (startup + active), latches containment-loss observation, emits diagnostic, and adjusts teardown behavior accordingly. |
| internal/dockerdeploy/controlled_session_supervisor_test.go | Adds regression tests for failing closed after watchdog exit (startup + active) and extends fakes to support new watchdog/channel behaviors. |
| internal/controlledsession/model.go | Adds new termination cause constant and validation support. |
| internal/controlledsession/lifecycle.go | Adds new observation kind that latches the cleanup-containment-lost cause. |
| internal/controlledsession/lifecycle_test.go | Adds coverage ensuring the new cause participates correctly in lifecycle behavior and racing. |
| docs/CONTROLLED_SESSION_DESIGN.md | Documents the new containment-loss semantics and diagnostic behavior. |
| .changes/unreleased/+controlled-session-cleanup-containment-loss.yaml | Release note fragment for the user-visible behavior change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@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: d6c0fb6361
ℹ️ 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. 👍 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 492fcbe498
ℹ️ 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: 492fcbe498
ℹ️ 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. Delightful! 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". |
Stack created with Sapling. Best reviewed with ReviewStack.