Skip to content

Fail closed when session watchdog exits - #35

Open
omry wants to merge 1 commit into
pr34from
pr35
Open

Fail closed when session watchdog exits#35
omry wants to merge 1 commit into
pr34from
pr35

Conversation

@omry

@omry omry commented Aug 10, 2026

Copy link
Copy Markdown
Owner

@omry
omry changed the base branch from main to pr34 August 10, 2026 03:46
@omry

omry commented Aug 10, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 4c093f5a08

ℹ️ 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".

@omry
omry changed the base branch from pr34 to main August 10, 2026 04:09
@omry
omry changed the base branch from main to pr34 August 10, 2026 11:17
@omry
omry marked this pull request as ready for review August 10, 2026 11:17
Copilot AI lite review requested due to automatic review settings August 10, 2026 11:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(), and Close() 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.

@omry

omry commented Aug 10, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread internal/dockerdeploy/controlled_session_supervisor.go
@omry
omry changed the base branch from pr34 to main August 10, 2026 11:37
@omry

omry commented Aug 10, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

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".

@omry

omry commented Aug 10, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread internal/dockerdeploy/runtime_host_preflight.go
@omry

omry commented Aug 10, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread internal/dockerdeploy/controlled_session_watchdog.go Outdated
@omry

omry commented Aug 10, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 6bcd565ab0

ℹ️ 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".

@omry
omry changed the base branch from main to pr34 August 10, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants