Skip to content

feat(sandbox): add main restart policy - #2798

Open
drew wants to merge 2 commits into
mainfrom
codex/2710-main-restart-policy
Open

feat(sandbox): add main restart policy#2798
drew wants to merge 2 commits into
mainfrom
codex/2710-main-restart-policy

Conversation

@drew

@drewdrew commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Add a gateway-owned restart policy for the canonical sandbox main process introduced by #2726. Sandboxes default to Never and may opt into Kubernetes-style OnFailure or Always replacement with durable exponential backoff.

This is the second PR in the stack and must be reviewed after #2726.

Related Issue

Relates to #2710.

Depends on #2726.

Changes

  • Add Never, OnFailure, and Always restart policies plus the Restarting sandbox phase.
  • Persist restart count, scheduling deadline, and main-process start time on sandbox status.
  • Implement gateway-owned replacement, exponential backoff, HA-safe reconciliation, readiness recovery, and manual stop cancellation.
  • Clear VM stop and terminal tombstones on explicit replacement start so a prior main exit cannot poison later driver recovery.
  • Preserve the original main-process start time when the same supervisor instance reconnects; reset it only for a new instance.
  • Keep native runtime restart disabled across Docker, Podman, Kubernetes, and VM drivers.
  • Surface policy and restart status through the CLI, TUI, SDKs, published docs, and agent guidance.

Testing

  • mise run pre-commit
  • mise run test with an isolated system-gateway directory
  • Focused VM tombstone and gateway reconnect stability-clock tests
  • Docker sandbox lifecycle E2E: 6 passed, covering retained reconnect/history and Ready to Restarting to replacement Ready with workspace preservation
  • Agent infrastructure inventory and cross-reference consistency checks

The unmodified host environment contains /etc/openshell/gateways/default, so the no-config completer test requires an isolated OPENSHELL_SYSTEM_GATEWAY_DIR; the full suite passes with that isolation.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture and user documentation updated
  • Related CLI and TUI agent guidance updated

@copy-pr-bot

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

@drew
drewforce-pushed the codex/2710-main-restart-policy branch from 5040f5c to bab5935CompareAugust 19, 2026 00:28
@drew
drew marked this pull request as ready for review August 19, 2026 00:30
@drew
drewforce-pushed the codex/2710-main-restart-policy branch 5 times, most recently from 1a75200 to e881c9cCompareAugust 19, 2026 15:43
@drew
drewforce-pushed the codex/2710-main-restart-policy branch from e881c9c to 5ce1324CompareAugust 19, 2026 17:40
@drew
drewforce-pushed the codex/2710-main-restart-policy branch from 5ce1324 to 4846676CompareAugust 20, 2026 01:46
@drew
drewforce-pushed the codex/2710-main-restart-policy branch 2 times, most recently from 2de9ac5 to d8d2e54CompareAugust 20, 2026 15:39
@drew
drewforce-pushed the codex/2710-main-restart-policy branch 2 times, most recently from 31bc2c1 to 59b2879CompareAugust 20, 2026 20:56
@drewdrew mentioned this pull request Aug 20, 2026
9 tasks
@drew
drewforce-pushed the codex/2710-main-restart-policy branch from 59b2879 to 71936efCompareAugust 20, 2026 21:05
@drew
drewforce-pushed the codex/2710-main-restart-policy branch from 71936ef to bb5859aCompareAugust 20, 2026 22:01
Base automatically changed from codex/2710-canonical-main-process to mainAugust 20, 2026 22:32
drew added 2 commits August 20, 2026 17:32
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
@sjenning
sjenningforce-pushed the codex/2710-main-restart-policy branch from bb5859a to a5f39e2CompareAugust 20, 2026 22:32
@copy-pr-bot

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@drew

drew commented Aug 21, 2026

Copy link
Copy Markdown
CollaboratorAuthor

/ok to test a5f39e2

Comment threadproto/openshell.proto
SANDBOX_PHASE_STOPPING = 6;
SANDBOX_PHASE_STOPPED = 7;
SANDBOX_PHASE_STARTING = 8;
SANDBOX_PHASE_RESTARTING = 9;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we want the distinction between starting and restarting?

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

no, i think sticking with starting makes more sense.

@sjenning

Copy link
Copy Markdown
Collaborator

review-github-pr skill with 5.6-sol

Potential Concerns

  • P1 — Early replacement exits are discarded. After starting a replacement, status retains the previous instance ID and exit code. If the replacement exits before connecting its supervisor session, its new UUID is rejected as stale (compute/mod.rs:2747, :3180). This can incorrectly restart an OnFailure workload that exited successfully and prevents crash-loop accounting.
  • P1 — HA stop/delete race. The controller releases its process-local guard before driver operations and starts unconditionally afterward (compute/mod.rs:2722-2761). Another gateway can concurrently stop or delete the sandbox, leaving persisted state Stopped while compute is running—or creating an orphan runtime.
  • P2 — Readiness deadline starts too early. The 120-second watchdog is recorded before both driver stop and start (compute/mod.rs:2701). A slow Kubernetes termination can consume the deadline and trigger an immediate second replacement.
  • Documentation drift. The CLI skills incorrectly imply OnFailure restarts after every exit; several existing docs still say all main-process exits are terminal. The architecture phase pseudocode also omits the new gateway Restarting override.
  • Agent-infrastructure drift. The maintenance map indicates debug-openshell-cluster should cover the new distinct Restarting state and gateway-versus-native restart behavior, but it was not updated.
  • TUI scope mismatch. The TUI styles the Restarting phase but does not display restart policy, count, deadline, or exit status, despite the PR description claiming restart status is surfaced there.

Sign up for freeto 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

@drew@sjenning