[drew]

Summary

Add high availability for the shared supervisor introduced by #4702. A hash ring places new sandbox generations, while durable ownership and gateway-signed takeover grants let a replacement supervisor adopt existing runtimes, processes, and memory without restarting workloads. Shared supervisors always use pool coordination, including at one replica.

Compute drivers are unchanged from base 9a85667d0. Coordination remains in the gateway and supervisor control layer.

Related Issue

Closes #4860

Changes

• Persist membership, desired assignments, ownership terms, and term-scoped reports across gateway replicas. Store launch material in the configured credential store.
• Keep launches in an admitting state until one transaction publishes the assignment, creates its owner, and charges member capacity. Refused launches remove only their own publication and material; failed cleanup is fenced and collected. Concurrent successful retries are preserved, and rejected launches cannot be revived by later capacity.
• Advance credential epochs, revoke the previous owner's credentials, and require signed takeover grants. Preserve process identity, binary identity pins, policy, sandbox CA, and SSH host keys during adoption; route runtimes to their current owner even while a stale owner remains reachable.
• Use atomic per-member capacity accounting, keyset pagination with absence confirmation, independently refreshed membership, acknowledged ownership terms, stale-stream rejection, and bounded reconnects. Process restarts can reclaim existing routes even in a full pool.
• Remove the non-pool shared-supervisor path and its enable switch. [openshell.gateway.supervisor_pool] optionally tunes the recovery deadline (60 seconds by default); member identity is required, and configuration errors name the setting to fix. Per-sandbox supervision remains available.
• Render a rolling supervisor Deployment, disruption budget, and preferred node anti-affinity. Changing only supervisor.replicas scales 1 → 2 → 1 without changing gateway configuration or replacing surviving members.
• Add deterministic admission/cleanup/retry, pagination, report-ordering, restart, and reconnect regressions, plus serialized live scaling and abrupt-loss E2Es that require distinct owners and preserved workload identity and memory.

Release Notes

Shared supervisors always run as a pool; set supervisor.replicas to change capacity. One replica has no spare for immediate failover. During takeover, workloads can briefly freeze and existing proxied connections or interactive sessions must reconnect. Workloads terminate if supervision is not restored within the recovery deadline.

Testing

• Affected Rust suites: 2,680 passed, 33 skipped across server (test-support), supervisor-control, and supervisor.
• Disposable PostgreSQL suite: 32/32 passed, including the admission/cleanup races; the container was removed afterward.
• Mutations removing refused-launch cleanup, atomic admission, or stale-admission fencing were caught by the regressions.
• Gateway chart 375/375 and workspace chart 6/6; full helm:test, rendered-TOML loader validation, count-independent 1/2/3 renders, Helm lint/docs, affected Clippy with warnings denied, formatting, and Markdown lint passed.
• Live scaling on gateway/supervisor images built from abffde246: 1 → 2 → 1 passed (85.35 s), plus all seven helper tests. Workload Pod UID/generation/PID/start ticks/in-memory counter and gateway/surviving-supervisor identity stayed unchanged; a new sandbox worked after downscale. The handed-off workload had two transient exec and network probe failures, with a 6.38 s maximum gap between successful probes (including the 2 s sampling interval); the unaffected workload had no failed probes.
• Live abrupt-loss recovery on the rebuilt images passed (36.69 s), plus all seven helper tests: direct supervisor SIGKILL, no graceful Leave, expiry-based takeover, preserved workload identity/memory, restored exec/network, and an unaffected workload.
• Normal full pre-commit hook passed (77.31 s), including workspace/extra-configuration Rust lint, format/license/lockfile, Helm, protobuf, Markdown/Mermaid, Python, and TypeScript checks.

Live Kubernetes validation uses one local k3d node. Physical node loss and multi-gateway partitions are not exercised. Cross-replica wall-clock comparisons and a standalone ownership design record remain follow-ups.

Pool members share the supervisor credential, so declared member identity is within that trust boundary. Identity-bound provider placeholders without stable handles can fail closed after takeover if their provider revision changed. Service discovery adds bounded periodic probes per pool sandbox. A crash between credential creation and candidate staging can leave orphaned material, as in the existing SSH host-key path.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Configuration and operational documentation updated

[copy-pr-bot]

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]

🌿 Preview your docs: https://nvidia-preview-pr-4864.docs.buildwithfern.com/openshell

[matthewgrossman]

Review of head 06cd2e2 from a multi-agent review panel (security, correctness, API/UX, and scope reviewers, plus a verification and triage step). I checked the one blocking finding against the code by hand. The reviewers read the code but didn't build it or run tests.

Summary: a large but coherent HA feature for #4860. The security reviewer found no new problems with the takeover grants, credential-epoch fencing, or the state preserved during adoption. One correctness issue should be fixed before merge. Everything else is polish or follow-up.

Fix before merge

• A refused pool launch leaves its assignment record and launch material behind, and another replica can later give it a takeover owner (supervisor_pool.rs:1731).

Small fixes

• A config error message contains runs of spaces (lib.rs:1876).
• The pool/non-pool mismatch errors don't name the setting to change (controller.rs:1037).
• The HA docs don't say what an upgrade does to running sandboxes (high-availability.mdx).

Follow-ups (fine as separate issues)

• Member freshness and report ordering compare wall-clock times written by different replicas.
• A design record (RFC) for pool ownership, takeover grants, and epoch fencing.
• Decide whether the non-pool shared-supervisor mode stays, since Helm always enables the pool.

Considered and not raised: splitting this ~24k-line PR into smaller PRs. It's one coherent, tested feature, so that's your call.

Details are in the inline comments.

[matthewgrossman]

Follow-up (non-blocking): cross-replica clock comparisons

fresh() compares this replica's now_ms() with an updated_at_ms that another replica wrote from its own clock. write_member and record_reports also order streams by connected_at_ms from different replicas. With a 15 s TTL, a few seconds of skew between gateway replicas could make a live member look expired, which would trigger an unneeded takeover, or could let a stale stream's report win.

Options: use database time (now() in the UPDATE) for freshness and ordering across both the postgres and sqlite stores, or document a clock-sync requirement for multi-replica gateways. Fine as a follow-up issue.

[matthewgrossman]

Follow-up (non-blocking): design record for pool ownership

This PR adds durable membership and ownership with CAS terms, gateway-signed takeover grants, credential-epoch fencing, a Sandbox Protocol minor bump, and a Helm default change. The RFC side is this short note. A short RFC, or an extension of RFC 0012, recording the trust model, the invariants, and the non-goals (multi-gateway partitions, node loss) would make later changes easier to review. It doesn't need to block this PR.

[github-actions]

Compatibility review (advisory)

New or updated breaking changes; see the full report for all findings.

1. cli — No explicit risk acceptance found

The existing openshell-supervisor --mode=shared invocation now additionally requires --pool-member-id, --pool-member-uid, and --pool-advertise-ip, or their environment equivalents.

• Impact: Existing custom launchers exit with a missing-pool-identity error before opening their listeners, although the updated Helm chart supplies the required environment variables.
• Action: Preserve startup compatibility for the old shared-mode invocation in v0.1.6, or defer mandatory pool identity to a permitted minor release with launcher migration guidance.
• PR: feat(supervisor): recover shared instances with hash-based HA #4864

2. rust-sdk — No explicit risk acceptance found

raw::proto::supervisor_host::v1::{HostHello, AssignmentSnapshot, SandboxInstanceAssignment, HostStatus} gain fields that invalidate existing exhaustive Rust literals and patterns.

• Impact: Raw-message consumers, including mock/server fixtures, fail to compile unless updated, while high-level clients and construction using ..Default::default() remain source-compatible.
• Action: Preserve the existing exported message layouts in v0.1.6, or defer their source-breaking expansion to a permitted minor release with explicit field and default-construction migration guidance.
• PR: feat(supervisor): recover shared instances with hash-based HA #4864

Review details

• Execution: complete
• Candidate: PR #4864 (8ab1d50d340b6f47fff0d01ebb8d1cc720ebed79)
• Baseline: 9a85667d0d87e8131b64351ec1fc4eb26e7bb865 (9a85667d0d87e8131b64351ec1fc4eb26e7bb865)
• Reviewed: rust-sdk, python-sdk, go-sdk, typescript-sdk, api-behavior, cli, configuration, policy, helm, state

Advisory review; publication gates are unchanged.

Branch check run

[TaylorMutch]

Reviewed head abffde246 locally. I found two correctness issues, each reproduced with a focused Rust test against this commit; details and reproduction steps are inline.

Validation: 30 supervisor Helm tests, the render-only 1/2/3-replica scaling check, 13 sandbox authentication tests, and the original 12 supervisor-control tests passed. The two additional regression assertions fail at the expected behavior checks. Temporary source changes were restored. These reproductions exercise the Rust controller and SQLite-backed pool; I did not run live Kubernetes recovery E2Es or PostgreSQL validation.

[TaylorMutch]

Confirm the owner's current membership before taking it over

This pass reads members before it lists owners. A healthy member can join and another gateway can admit a sandbox onto it between those reads. The owner is then visible here, but its member is absent from the earlier membership result, so owner_alive returns false and the pass takes over a healthy launch. This unnecessarily advances the credential epoch and revokes the launch owner's credentials, disrupting a sandbox whose supervisor did not fail.

Reproduced on abffde246 using the existing fixture and listing hook: start with m1; pause reconcile_once in the report listing after membership was read (fill one LIST_LIMIT page and use pause_listing_after_first_page(REPORT_OBJECT_TYPE)); register m2; use another replica's place_launch for a sandbox the ring places on m2; resume reconciliation. The assertion that ownership remains (m2, epoch 1) fails: it becomes (m1, epoch 2), with a granted takeover and previous_member_id = m2. The focused test runs to that assertion in 0.17 seconds.

Please confirm the specific owner's member record, including its process, freshness, and leaving state, before recording an intent based on apparent failure, and retain a regression for this ordering.

[TaylorMutch]

Fence local observations by the acknowledged ownership term

When a route returns to the same still-running supervisor process at a later ownership term, this returns the process's earlier status before checking whether it acknowledged the current assignment. member_reports() correctly rejects the earlier term, but local observed() reads still accept it. Consequently a recovering sandbox can be reported ready before its new owner has applied the takeover; an earlier terminal status can instead become Unavailable through SharedSupervision::supervision() and cause the Kubernetes driver to suspend the workload.

Reproduced on abffde246 by adding assert_eq!(controller.observed("rt-a"), ObservedInstance::Pending); to reports_count_only_for_the_term_their_stream_acknowledged, immediately after the term-3 snapshot is sent, the term-1 status is recorded, and terms().is_empty() is asserted. It fails with Reported(SandboxInstanceStatus { phase: Supervising, ready: true, ... }) instead of Pending. The existing test already models the ownership transition from m1/p1 to another member and back before either snapshot was applied.

Please apply the current-term acknowledgement fencing to local observations as well, and extend that regression to assert observed() remains pending until the later term is acknowledged.