Repository navigation
Conversation
|
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. |
drew
added this pull request to stack #4698
October 19, 2026 02:17
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
from
October 20, 2026 16:24
0e074fb to
e2b27bd
Compare
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
3 times, most recently
from
October 20, 2026 22:10
bec05a1 to
d823dac
Compare
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
6 times, most recently
from
October 21, 2026 01:05
4a7e9d5 to
00d9769
Compare
Base automatically changed from
feat/4694-experimental-supervisor-topology/drew
to
main
October 21, 2026 01:48
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
from
October 21, 2026 01:48
00d9769 to
9ca201c
Compare
drew
changed the title
drew
marked this pull request as ready for review
October 22, 2026 00:14
A supervisor process that hosts several sandboxes cannot use one process-wide OCSF context: every event would carry the wrong sandbox identity, and middleware requests read their sandbox identity from the same context. Builders now accept a ContextRef, either a borrowed context or a context shared by a hosted session, so per-session contexts need not be leaked. ctx() returns the context bound to the calling thread when present and falls back to the process-wide context otherwise. OCSF JSON output settings can likewise be bound per thread, and OpenTelemetry can parent a span from an explicit W3C trace context instead of process environment. A dedicated supervisor keeps using the process-wide context. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
…a session The supervisor kept its gateway bearer, Sandbox Protocol bearer, and renewal task in process-wide slots, and the network supervisor read provider token grants, proxy CA files, and the gateway identity from process environment. A second sandbox in the same process would have replaced or rotated the first sandbox's credentials, reused its provider access tokens, and exposed its denials through policy.local. GatewayCredentials now holds one session's gateway and Sandbox Protocol credentials and their renewal; every gateway client receives a GatewayEndpoint that pairs the URL with them, including log push, ConnectSupervisor and its redirects, and extension-credential rotation. run_networking takes a NetworkSession with the sandbox identity, the session's gateway access, its Workload API socket, its policy.local denial source, and its proxy CA files. Provider token grants use a cache and HTTP client owned by the session and fail closed without a Workload API socket. run_session supervises one generation from an explicit SessionConfig and owner-provided SessionControls. run_sandbox_with_backend remains the dedicated entry point: it keeps the process-wide OCSF context, reads the same environment, binds readiness endpoints to session readiness, and stops on SIGTERM or SIGINT. The binary shares one set of credentials between log push and supervision, as before. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
SessionHost runs several supervisor sessions in one process. Each session gets its own Tokio runtime, and every thread that runtime starts, plus the thread driving it, is bound to the session's OCSF context, OCSF JSON settings, log sink, and denial log. Tasks, blocking work, reconnects, and credential renewal spawned by a session therefore keep its identity, while threads outside any session carry none and push no logs. Gateway credentials stay explicit per session. Removing a session stops its workload gracefully, or cancels it at once while it is still starting, then shuts down only its runtime and revokes its gateway credentials. The key stays registered until the session thread has finished, so the same generation cannot gain a second owner while one is stopping. A panicking session reports a failure, clears its readiness, and is revoked the same way. Hosted sessions get a private proxy trust directory and serve policy.local denials from their own log. A focused test runs two sessions concurrently and checks that nested and blocking tasks see their own identity, gateway requests carry their own bearer, log lines reach only their own stream, and removing one leaves the other supervising. Closes #4699 Signed-off-by: Drew Newberry <anewberry@nvidia.com>
A sandbox instance is one running generation of a stable Sandbox resource. Gateway transport connections and the gateway supervisor sessions they carry can reconnect within one instance, so the hosted object, its configuration, controls, host, and per-instance network, denial, and token-grant state now use the sandbox instance name. Rename session_host to sandbox_instance_host, run_session to run_sandbox_instance, and SessionConfig, SessionControls, SessionHost, and related hosted types to their SandboxInstance equivalents. Gateway supervisor sessions, SSH sessions, and authentication session types keep their names. No wire, configuration, or lifecycle behavior changes. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
…epared bindings Replace SandboxRunConfig and its field-by-field copy into SandboxInstanceConfig with a single launch configuration. Live per-instance state (gateway credentials, OCSF context and JSON settings, log sink, and denial log) is now an explicit SandboxInstanceBindings value that the owner prepares once, instead of placeholders the runner or host overwrote. The dedicated binary loads launch inputs from its arguments and environment and shares one GatewayCredentials with log push; run_sandbox_with_backend keeps the process-wide OCSF context, health endpoints, and signal handling. The instance host binds the same SandboxInstanceBindings to its runtime threads and revokes their credentials when the instance ends. Signed-off-by: Drew Newberry <anewberry@nvidia.com>
TaylorMutch
requested changes
Oct 22, 2026
TaylorMutch
left a comment
TaylorMutch
left a comment
Collaborator
There was a problem hiding this comment.
Found one Windows test compilation regression. The remaining credential, policy/provider state, logging, and lifecycle changes have no other actionable findings from this static review. Tests and a Windows build were not run.
| pub sandbox_name: Option<String>, | ||
| pub openshell_endpoint: Option<String>, | ||
| /// Gateway access with this sandbox's credentials. | ||
| pub gateway: Option<openshell_core::grpc_client::GatewayEndpoint>, |
Collaborator
There was a problem hiding this comment.
Please update the Windows test initializer alongside this field rename. Before this PR, test_config matched HostProxyConfig; it still initializes openshell_endpoint: None at line 261, so renaming the field to gateway makes Windows test builds fail with E0560 (no field named openshell_endpoint). Change that initializer to gateway: None. This module is Windows-only, so Linux/macOS checks do not catch it. Verified statically against this commit; no Windows build was run.
drew
force-pushed
the
feat/4699-supervisor-session-state/drew
branch
from
October 22, 2026 00:57
fc7caf2 to
e1be7e9
Compare
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
TaylorMutch
approved these changes
Oct 22, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Make supervisor state belong to a sandbox instance (one running generation), so a single process can host several sandboxes without sharing gateway credentials, provider grants, policy decisions, or log identity. Dedicated supervision continues to run one instance; Kubernetes shared hosting is added in #4702.
Related Issue
Closes #4699
Changes
SandboxInstanceConfigfor dedicated and hosted launches, with explicitSandboxInstanceBindingsfor prepared credentials and logging state. Remove the duplicate launch config and long argument-copying wrapper; keep process health and signals in the dedicated wrapper.A sandbox is the stable resource; an instance is one running generation. Transport and authentication sessions may reconnect within that instance. Existing wire/storage names, environment variables, CLI flags, and defaults remain compatible.
Release Notes
Testing
1d880c9ac, preserving all 38 commits and the merge-only changes. Current full-stack Rust tests: 176 supervisor, 429 server authorization/token/expiry, and 2 runtime-routing tests passed; 2 server tests ignored.The focused shared-supervisor, SPIFFE, and dedicated lifecycle scenarios compile. A fresh-cluster live run was blocked before OpenShell deployment: containerd could not create an inotify watcher (
too many open files; hostfs.inotify.max_user_instances=128), so the node never registered. No live scenarios ran; the temporary cluster was removed. Windows MSVC compilation was not run locally.Checklist