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: c3cd3b535f
ℹ️ 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. Keep them coming! 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. Nice work! 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.
Pull request overview
Implements a Linux-only “controlled-session watchdog” launched as a short-lived child process from the same reploy executable. The watchdog receives an immutable cleanup manifest over inherited FDs, isolates itself into a separate process session, and performs verified cleanup on parent loss (and final verification on disarm), while the supervisor is updated to launch/disarm the watchdog as part of the controlled-session lifecycle.
Changes:
- Add a controlled-session watchdog child mode with manifest decoding, boot/label verification, and cleanup enforcement.
- Launch the watchdog from the controlled-session supervisor before starting either container, and disarm it only after verified cleanup succeeds.
- Add unit + integration tests and update controlled-session design documentation and
cmd/reployentrypoint dispatch.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/dockerdeploy/controlled_session_watchdog.go | Watchdog core logic: manifest read/validate, boot/label checks, container/channel cleanup, Docker inspect/remove backend. |
| internal/dockerdeploy/controlled_session_watchdog_linux.go | Linux watchdog process launch, readiness handshake, setsid isolation, disarm + wait. |
| internal/dockerdeploy/controlled_session_watchdog_unsupported.go | Non-Linux stubs returning “requires Linux”. |
| internal/dockerdeploy/controlled_session_watchdog_test.go | Unit tests for watchdog behavior (disarm verification, parent-loss cleanup, boot/label guarding). |
| internal/dockerdeploy/controlled_session_watchdog_integration_linux_test.go | Linux integration test validating process-group isolation and parent-loss cleanup in Docker. |
| internal/dockerdeploy/controlled_session_watchdog_integration_unsupported_test.go | Non-Linux integration test placeholder asserting unsupported behavior. |
| internal/dockerdeploy/controlled_session_supervisor.go | Supervisor now records ownership then starts watchdog before controller/workload, and disarms it after successful cleanup. |
| internal/dockerdeploy/controlled_session_supervisor_test.go | Supervisor unit tests covering watchdog launch ordering, launch failure handling, and disarm gating/error paths. |
| internal/dockerdeploy/controlled_session_supervisor_integration_test.go | Docker integration test builds a reploy binary for the watchdog child path and validates parent-loss cleanup. |
| docs/CONTROLLED_SESSION_DESIGN.md | Update design doc to include watchdog lifecycle, readiness/disarm semantics, and cleanup guarantees. |
| cmd/reploy/main.go | Entrypoint dispatch to handle watchdog child mode before normal CLI execution. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| removeErr := backend.removeContainer(ctx, container.ID) | ||
| _, stillFound, inspectErr := backend.inspectContainer(ctx, container.ID) | ||
| if inspectErr != nil { | ||
| return errors.Join(removeErr, fmt.Errorf("verify controlled-session %s container %q removal: %w", container.Role, container.ID, inspectErr)) | ||
| } | ||
| if stillFound { | ||
| return errors.Join(removeErr, fmt.Errorf("controlled-session %s container %q still exists after removal", container.Role, container.ID)) | ||
| } | ||
| return nil |
| func main() { | ||
| if code, handled := dockerdeploy.RunControlledSessionWatchdogChild(os.Args[1:], os.Stderr); handled { | ||
| os.Exit(code) | ||
| } | ||
| os.Exit(cli.Main(os.Args[1:], os.Stdout, os.Stderr)) |
| return status, controlledsession.CleanupStatusV1{ | ||
| Kind: controlledsession.CleanupStatusFailedV1, Message: "controlled-session delivery-tail cleanup failed", | ||
| }, controlledsession.RecoveryRetryCleanupV1 |
Summary
Validation
go test ./...go test -racefor watchdog and supervisor lifecycle casesgo vet ./internal/dockerdeploy ./cmd/reploy