From 6cc734d0b8ed2bcb1cc52c13f1d41cc5e8e9b3fe Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Thu, 10 Sep 2026 15:48:30 -0700 Subject: [PATCH 1/7] feat(supervisor): apply streamed configuration snapshots Signed-off-by: Piotr Mlocek --- architecture/gateway.md | 14 +- architecture/sandbox.md | 54 +- crates/openshell-core/src/grpc_client.rs | 116 +++ crates/openshell-core/src/proto/mod.rs | 6 +- crates/openshell-sandbox/src/lib.rs | 639 +++++++++++++- .../openshell-server/src/config_delivery.rs | 35 +- crates/openshell-server/src/grpc/policy.rs | 105 ++- crates/openshell-server/src/lib.rs | 1 + .../src/supervisor_session.rs | 832 +++++++++++++++--- .../openshell-supervisor-process/src/run.rs | 4 + .../src/supervisor_session.rs | 285 +++++- docs/reference/gateway-config.mdx | 10 +- proto/openshell.proto | 3 +- sdk/go/proto/openshellv1/openshell.pb.go | 3 +- skills/debug-openshell-cluster/SKILL.md | 2 +- 15 files changed, 1900 insertions(+), 209 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index ac47301868..ccbcd24d74 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -600,8 +600,10 @@ still referenced by a sandbox. Policy and runtime settings are delivered together through the effective sandbox config path. A gateway-global policy can override sandbox-scoped policy. The -sandbox supervisor polls for config revisions and hot-reloads dynamic policy -when the policy engine accepts the update. +gateway pushes complete snapshots to active supervisor sessions and periodically +rebuilds them to repair missed delivery. Supervisors hot-reload accepted policy +and acknowledge the exact revision. The legacy poller remains as a mixed-version +compatibility path during this stage. External supervisor middleware registration is operator-owned configuration under `[[openshell.supervisor.middleware]]`. At startup the gateway connects to @@ -666,10 +668,12 @@ process-local supervisor registry. A future HA implementation can resolve the gateway that owns a session and forward the same typed message without changing mutation handlers. -Polling remains authoritative during the first rollout stage. Snapshot build, +Current supervisors apply stream snapshots directly. Previous-revision +supervisors retain polling as a rollout fallback, and owner reconciliation +repairs missed or failed delivery from current database state. Snapshot build, fanout, or enqueue failure cannot fail a mutation that already committed. -Provider snapshots may contain credentials and must not be -persisted or included in logs. +Provider snapshots may contain credentials and must not be persisted or +included in logs. See [sandbox configuration delivery](sandbox.md#supervisor-configuration-delivery) for bootstrap, revision, and supervisor application semantics. diff --git a/architecture/sandbox.md b/architecture/sandbox.md index 00fc709c25..8dfb9008ab 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -475,15 +475,18 @@ quickly. ## Supervisor Configuration Delivery -The gateway and supervisor must implement the same internal supervisor protocol -revision. Peers built before the handshake existed report revision zero and are -accepted for one release with a warning and a counter, because sandboxes keep -their supervisor binary until they are recreated. The gateway includes a configuration bootstrap when it accepts a -`ConnectSupervisor` session and can send complete component replacements on the -same stream after policy, settings, or provider state changes. -While polling remains authoritative, optional bootstrap construction has a -one-second budget. The gateway accepts the session without a bootstrap when -that budget expires, so slow credential backends do not block relay reconnects. +The current gateway and supervisor use internal supervisor protocol revision 2. +The gateway accepts Stage 1 revision 1 supervisors through the polling +compatibility path. Peers built before the handshake report revision zero and +remain accepted for one release with a warning and counter because sandboxes +keep their supervisor binary until they are recreated. The gateway includes a +configuration bootstrap when it accepts a `ConnectSupervisor` session and can +send complete component replacements on the same stream after policy, settings, +or provider state changes. +Revision 2 supervisors require a complete bootstrap. The gateway uses the same +bounded 45-second construction window as other snapshot builds and rejects the +connection when construction fails. Revision 1 compatibility sessions retain +the optional one-second bootstrap budget and use polling when it expires. These payloads describe the latest effective state rather than the mutation that produced it. The gateway assigns ordering sequences within each session and component, while each snapshot retains its own content @@ -503,21 +506,34 @@ without changing publishers. Provider payloads can contain credentials, so the gateway does not persist or render complete stream messages in logs. -The supervisor currently parses and ignores stream-delivered configuration. -Polling remains the only path that changes runtime state and repairs dropped or -unavailable delivery. The gateway serializes construction per sandbox and -component, and coalesces repeated mutations into the latest full snapshot. An -enqueue result means only that the local stream queue accepted the message. A -bounded scope fanout scheduler coalesces repeated workspace and global changes, +The supervisor applies stream-delivered configuration through the same runtime +primitives used by the compatibility poller. It reports the requested and +active revisions plus a component-specific outcome on `ConnectSupervisor`. +Sandbox-scoped policy results update only the matching policy-history row, so a +late result cannot mark a newer revision loaded. Explicit local policy remains +authoritative and produces a retained-local-override result. + +The gateway keeps one update in flight per session and component. It replaces +the pending snapshot when newer desired state arrives, validates the update ID, +component sequence, component, and requested revision on acknowledgement, then +sends the newest pending snapshot. A 30-second owner reconciliation pass +rebuilds current snapshots for active local sessions. Applied equality +revisions suppress unchanged delivery, while failed or timed-out delivery is +retried from current database state. Reconnect discards session delivery state +and starts with a fresh bootstrap. + +Polling remains available during the mixed-version rollout. The gateway +serializes construction per sandbox and component, and coalesces repeated +mutations into the latest full snapshot. An enqueue result means only that the +local stream queue accepted the message. A bounded scope fanout scheduler +coalesces repeated workspace and global changes, and semaphores sized from the database pool bound delivery workers and snapshot builds. Fanout waits for worker capacity before admitting each recipient, so a fleet-wide change cannot create a fleet-sized task backlog or saturate the store and credential backends. Snapshot construction has a deadline that starts once a build holds a permit, and the gateway rejects encoded stream messages that -approach the transport decoder limit. A later migration will apply these -payloads directly and acknowledge their exact revisions before removing -supervisor polling. At that point, the gateway will require a valid bootstrap -before marking a session ready. +approach the transport decoder limit. Durable apply operations and final polling +removal remain separate follow-up work. ## Policy Revision Acknowledgement diff --git a/crates/openshell-core/src/grpc_client.rs b/crates/openshell-core/src/grpc_client.rs index 38b91e8501..115b6d6667 100644 --- a/crates/openshell-core/src/grpc_client.rs +++ b/crates/openshell-core/src/grpc_client.rs @@ -960,6 +960,29 @@ fn settings_poll_result(inner: crate::proto::GetSandboxConfigResponse) -> Settin } } +impl From for SettingsPollResult { + fn from(inner: crate::proto::SandboxConfigSnapshot) -> Self { + Self { + policy: inner.policy, + version: inner.version, + policy_hash: inner.policy_hash, + config_revision: inner.config_revision, + policy_source: PolicySource::try_from(inner.policy_source) + .unwrap_or(PolicySource::Unspecified), + settings: inner.settings, + global_policy_version: inner.global_policy_version, + provider_env_revision: inner.provider_env_revision, + supervisor_middleware_services: inner.supervisor_middleware_services, + workspace: inner.workspace, + policy_validation_failure_mode: inner + .policy_validation_failure_mode + .parse() + .unwrap_or_default(), + extension_authentication_enabled: inner.extension_authentication_enabled, + } + } +} + #[cfg(test)] mod settings_poll_tests { use super::settings_poll_result; @@ -1001,6 +1024,25 @@ mod settings_poll_tests { let legacy = settings_poll_result(GetSandboxConfigResponse::default()); assert!(!legacy.extension_authentication_enabled); } + + #[test] + fn delivered_sandbox_snapshot_uses_the_polling_projection() { + let result = super::SettingsPollResult::from(crate::proto::SandboxConfigSnapshot { + version: 7, + policy_hash: "hash-7".to_string(), + config_revision: 42, + provider_env_revision: 9, + workspace: "workspace-a".to_string(), + extension_authentication_enabled: true, + ..Default::default() + }); + assert_eq!(result.version, 7); + assert_eq!(result.policy_hash, "hash-7"); + assert_eq!(result.config_revision, 42); + assert_eq!(result.provider_env_revision, 9); + assert_eq!(result.workspace, "workspace-a"); + assert!(result.extension_authentication_enabled); + } } pub struct ProviderEnvironmentResult { @@ -1012,6 +1054,80 @@ pub struct ProviderEnvironmentResult { pub non_secret_environment_keys: Vec, } +impl From for ProviderEnvironmentResult { + fn from(snapshot: crate::proto::ProviderEnvironmentSnapshot) -> Self { + use crate::proto::ProviderEnvironmentValueClassification; + + let mut environment = HashMap::with_capacity(snapshot.values.len()); + let mut credential_expires_at_ms = HashMap::new(); + let mut static_credential_bindings = HashMap::new(); + let mut non_secret_environment_keys = Vec::new(); + for value in snapshot.values { + environment.insert(value.name.clone(), value.value); + if let Some(expires_at_ms) = value.expires_at_ms { + credential_expires_at_ms.insert(value.name.clone(), expires_at_ms); + } + match ProviderEnvironmentValueClassification::try_from(value.classification) + .unwrap_or_default() + { + ProviderEnvironmentValueClassification::NonSecret => { + non_secret_environment_keys.push(value.name); + } + ProviderEnvironmentValueClassification::StaticCredential => { + if let Some(binding) = value.static_credential_binding { + static_credential_bindings.insert(value.name, binding); + } + } + ProviderEnvironmentValueClassification::Unspecified => {} + } + } + Self { + environment, + provider_env_revision: snapshot.provider_env_revision, + credential_expires_at_ms, + dynamic_credentials: snapshot.dynamic_credentials, + static_credential_bindings, + non_secret_environment_keys, + } + } +} + +#[cfg(test)] +mod provider_snapshot_tests { + use super::ProviderEnvironmentResult; + use crate::proto::{ + ProviderEnvironmentSnapshot, ProviderEnvironmentValue, + ProviderEnvironmentValueClassification, StaticCredentialBinding, + }; + + #[test] + fn delivered_provider_snapshot_preserves_secret_classification_metadata() { + let result = ProviderEnvironmentResult::from(ProviderEnvironmentSnapshot { + provider_env_revision: 11, + values: vec![ + ProviderEnvironmentValue { + name: "REGION".to_string(), + value: "west".to_string(), + classification: ProviderEnvironmentValueClassification::NonSecret.into(), + ..Default::default() + }, + ProviderEnvironmentValue { + name: "TOKEN".to_string(), + value: "redacted".to_string(), + classification: ProviderEnvironmentValueClassification::StaticCredential.into(), + static_credential_binding: Some(StaticCredentialBinding::default()), + ..Default::default() + }, + ], + ..Default::default() + }); + assert_eq!(result.provider_env_revision, 11); + assert_eq!(result.environment.len(), 2); + assert_eq!(result.non_secret_environment_keys, ["REGION"]); + assert!(result.static_credential_bindings.contains_key("TOKEN")); + } +} + pub struct ProviderSubjectTokenExchangeResult { pub access_token: String, pub expires_in: i64, diff --git a/crates/openshell-core/src/proto/mod.rs b/crates/openshell-core/src/proto/mod.rs index 2ab1e155d1..76bca9eaa1 100644 --- a/crates/openshell-core/src/proto/mod.rs +++ b/crates/openshell-core/src/proto/mod.rs @@ -100,7 +100,11 @@ pub fn all_workspaces_selector() -> WorkspaceSelector { /// The supervisor stream is an internal, version-locked deployment contract. /// Bump this when either peer can no longer honor the previous stream /// semantics. -pub const SUPERVISOR_PROTOCOL_REVISION: u32 = 1; +pub const SUPERVISOR_PROTOCOL_REVISION: u32 = 2; + +/// Stage 1 peers understand snapshot envelopes but do not apply them. They +/// remain compatible while polling is retained for the rollout. +pub const PREVIOUS_SUPERVISOR_PROTOCOL_REVISION: u32 = 1; /// Revision implied by peers built before the handshake existed. Proto3 leaves /// the field unset, so such peers report zero. diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index b5b9358ac0..fdc52d36b2 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -526,6 +526,8 @@ pub async fn run_sandbox( // GetSandboxConfig and broadcasts it. Flush tasks and the policy.local // API read the current value so proposals target the correct workspace. let (workspace_tx, workspace_rx) = tokio::sync::watch::channel(String::new()); + let (config_apply_tx, config_apply_rx) = tokio::sync::mpsc::channel(16); + let mut config_apply_rx = Some(config_apply_rx); let mut networking = if network_enabled { #[cfg(target_os = "linux")] @@ -629,6 +631,7 @@ pub async fn run_sandbox( sandbox_id: sandbox_id.clone(), trusted_ssh_socket_path: std::path::PathBuf::from(trusted_ssh_socket_path), control_publisher: sidecar_control_publisher.clone(), + config_apply_tx: config_apply_tx.clone(), }, ); } @@ -773,6 +776,7 @@ pub async fn run_sandbox( capable: transparent_tcp_capable, substrate_ready: transparent_tcp_substrate_ready, }, + config_apply_rx: config_apply_rx.take(), }; tokio::spawn(async move { @@ -972,6 +976,7 @@ pub async fn run_sandbox( main_env, ca_file_paths, agent_proposals.clone(), + Some(config_apply_tx.clone()), #[cfg(target_os = "linux")] netns.as_ref(), #[cfg(target_os = "linux")] @@ -1327,6 +1332,9 @@ struct SidecarEntrypointHandler { sandbox_id: Option, trusted_ssh_socket_path: std::path::PathBuf, control_publisher: Option, + config_apply_tx: tokio::sync::mpsc::Sender< + openshell_supervisor_process::supervisor_session::ConfigApplyRequest, + >, } #[cfg(target_os = "linux")] @@ -1343,6 +1351,7 @@ fn spawn_sidecar_entrypoint_handler( sandbox_id, trusted_ssh_socket_path, control_publisher, + config_apply_tx, } = handler; let mut session_started = false; let mut session_task: Option> = None; @@ -1454,6 +1463,7 @@ fn spawn_sidecar_entrypoint_handler( Some(supervisor_pid), Arc::clone(&terminating), started.instance_id.clone(), + Some(config_apply_tx.clone()), )); session_started = true; info!("sidecar supervisor session task spawned"); @@ -3352,6 +3362,11 @@ struct PolicyPollLoopContext { middleware_connector: MiddlewareConnector, /// Immutable driver capability and startup substrate state. transparent_tcp: TransparentTcpReloadState, + config_apply_rx: Option< + tokio::sync::mpsc::Receiver< + openshell_supervisor_process::supervisor_session::ConfigApplyRequest, + >, + >, } type MiddlewareConnector = Arc< @@ -3730,6 +3745,420 @@ fn emit_policy_validation_failure( } } +async fn receive_config_apply( + receiver: &mut Option< + tokio::sync::mpsc::Receiver< + openshell_supervisor_process::supervisor_session::ConfigApplyRequest, + >, + >, +) -> Option { + match receiver { + Some(receiver) => receiver.recv().await, + None => std::future::pending().await, + } +} + +fn sandbox_config_revision( + snapshot: &openshell_core::grpc_client::SettingsPollResult, +) -> openshell_core::proto::ConfigSnapshotRevision { + openshell_core::proto::ConfigSnapshotRevision { + component: Some( + openshell_core::proto::config_snapshot_revision::Component::SandboxConfig( + openshell_core::proto::SandboxConfigRevision { + config_revision: snapshot.config_revision, + policy_version: snapshot.version, + policy_source: snapshot.policy_source.into(), + global_policy_version: snapshot.global_policy_version, + }, + ), + ), + } +} + +fn provider_config_revision(revision: u64) -> openshell_core::proto::ConfigSnapshotRevision { + openshell_core::proto::ConfigSnapshotRevision { + component: Some( + openshell_core::proto::config_snapshot_revision::Component::ProviderEnvironment( + revision, + ), + ), + } +} + +fn config_apply_result( + component: openshell_core::proto::ConfigComponent, + requested_revision: openshell_core::proto::ConfigSnapshotRevision, + applied_revision: Option, + outcome: openshell_core::proto::ConfigApplyOutcome, + failure: Option<(&str, String, bool)>, +) -> openshell_core::proto::ConfigComponentApplyResult { + openshell_core::proto::ConfigComponentApplyResult { + component: component.into(), + requested_revision: Some(requested_revision), + applied_revision, + outcome: outcome.into(), + failure: failure.map(|(code, message, retryable)| { + openshell_core::proto::ConfigApplyFailure { + code: code.to_string(), + message: message.chars().take(1024).collect(), + retryable, + } + }), + } +} + +#[allow(clippy::too_many_arguments)] +async fn apply_stream_config_request( + ctx: &PolicyPollLoopContext, + client: &C, + request: openshell_supervisor_process::supervisor_session::ConfigApplyRequest, + current_config_revision: &mut u64, + current_stream_sandbox_revision: &mut Option, + current_provider_env_revision: &mut u64, + current_policy_version: &mut u32, + current_policy_hash: &mut String, + current_middleware_services: &mut Vec, + current_extension_authentication_enabled: &mut bool, + middleware_registry_status: &mut MiddlewareRegistryStatus, + current_settings: &mut std::collections::HashMap< + String, + openshell_core::proto::EffectiveSetting, + >, + reloads_gateway_policy: bool, + has_last_valid_policy: &mut bool, +) { + use openshell_core::proto::{ConfigBootstrapResult, ConfigUpdateResult, config_update}; + use openshell_supervisor_process::supervisor_session::ConfigApplyRequest; + + match request { + ConfigApplyRequest::Bootstrap { + bootstrap, + response, + } => { + let mut results = Vec::with_capacity(2); + if let Some(snapshot) = bootstrap.provider_environment { + results.push(apply_stream_provider_snapshot( + ctx, + snapshot, + current_provider_env_revision, + )); + } + if let Some(snapshot) = bootstrap.sandbox_config { + results.push( + apply_stream_sandbox_snapshot( + ctx, + client, + snapshot.into(), + current_config_revision, + current_stream_sandbox_revision, + current_policy_version, + current_policy_hash, + current_middleware_services, + current_extension_authentication_enabled, + middleware_registry_status, + current_settings, + reloads_gateway_policy, + has_last_valid_policy, + ) + .await, + ); + } + let _ = response.send(ConfigBootstrapResult { results }); + } + ConfigApplyRequest::Update { update, response } => { + let result = match update.component { + Some(config_update::Component::SandboxConfig(snapshot)) => { + apply_stream_sandbox_snapshot( + ctx, + client, + snapshot.into(), + current_config_revision, + current_stream_sandbox_revision, + current_policy_version, + current_policy_hash, + current_middleware_services, + current_extension_authentication_enabled, + middleware_registry_status, + current_settings, + reloads_gateway_policy, + has_last_valid_policy, + ) + .await + } + Some(config_update::Component::ProviderEnvironment(snapshot)) => { + apply_stream_provider_snapshot(ctx, snapshot, current_provider_env_revision) + } + None => config_apply_result( + openshell_core::proto::ConfigComponent::Unspecified, + openshell_core::proto::ConfigSnapshotRevision::default(), + None, + openshell_core::proto::ConfigApplyOutcome::Unsupported, + Some(( + "unsupported_component", + "configuration update has no supported component".to_string(), + false, + )), + ), + }; + let _ = response.send(ConfigUpdateResult { + update_id: update.update_id, + component_sequence: update.component_sequence, + result: Some(result), + }); + } + } +} + +fn apply_stream_provider_snapshot( + ctx: &PolicyPollLoopContext, + snapshot: openshell_core::proto::ProviderEnvironmentSnapshot, + current_revision: &mut u64, +) -> openshell_core::proto::ConfigComponentApplyResult { + use openshell_core::proto::{ConfigApplyOutcome, ConfigComponent}; + + let requested_revision = provider_config_revision(snapshot.provider_env_revision); + if snapshot.provider_env_revision == *current_revision { + return config_apply_result( + ConfigComponent::ProviderEnvironment, + requested_revision, + Some(requested_revision), + ConfigApplyOutcome::IgnoredDuplicate, + None, + ); + } + let result: openshell_core::grpc_client::ProviderEnvironmentResult = snapshot.into(); + let revision = result.provider_env_revision; + match ctx.provider_credentials.install_bound_environment( + revision, + result.environment, + result.credential_expires_at_ms, + result.dynamic_credentials, + result.static_credential_bindings, + result.non_secret_environment_keys, + ) { + Ok(_) => { + let child_env = ctx.provider_credentials.child_env_with_gcp_resolved(); + if let Some(publisher) = ctx.sidecar_control_publisher.as_ref() { + publisher.publish_provider_env(revision, child_env); + } + *current_revision = revision; + config_apply_result( + ConfigComponent::ProviderEnvironment, + requested_revision, + Some(requested_revision), + ConfigApplyOutcome::Applied, + None, + ) + } + Err(error) => { + let child_env = ctx.provider_credentials.child_env_with_gcp_resolved(); + if let Some(publisher) = ctx.sidecar_control_publisher.as_ref() { + publisher.publish_provider_env(revision, child_env); + } + *current_revision = revision; + config_apply_result( + ConfigComponent::ProviderEnvironment, + requested_revision, + Some(requested_revision), + ConfigApplyOutcome::Degraded, + Some(("invalid_provider_environment", error.to_string(), false)), + ) + } + } +} + +#[allow(clippy::too_many_arguments)] +async fn apply_stream_sandbox_snapshot( + ctx: &PolicyPollLoopContext, + client: &C, + snapshot: openshell_core::grpc_client::SettingsPollResult, + current_config_revision: &mut u64, + current_stream_revision: &mut Option, + current_policy_version: &mut u32, + current_policy_hash: &mut String, + current_middleware_services: &mut Vec, + current_extension_authentication_enabled: &mut bool, + middleware_registry_status: &mut MiddlewareRegistryStatus, + current_settings: &mut std::collections::HashMap< + String, + openshell_core::proto::EffectiveSetting, + >, + reloads_gateway_policy: bool, + has_last_valid_policy: &mut bool, +) -> openshell_core::proto::ConfigComponentApplyResult { + use openshell_core::proto::{ConfigApplyOutcome, ConfigComponent, PolicySource}; + use std::sync::atomic::Ordering; + + let requested_revision = sandbox_config_revision(&snapshot); + if snapshot.config_revision == *current_config_revision { + return config_apply_result( + ConfigComponent::SandboxConfig, + requested_revision, + Some(requested_revision), + ConfigApplyOutcome::IgnoredDuplicate, + None, + ); + } + + let _ = ctx.workspace_tx.send(snapshot.workspace.clone()); + let middleware_credentials = if snapshot.extension_authentication_enabled { + client + .extension_credentials_for(&snapshot.supervisor_middleware_services) + .await + .unwrap_or_default() + } else { + std::collections::HashMap::new() + }; + let registry_changed = *current_extension_authentication_enabled + != snapshot.extension_authentication_enabled + || middleware_registry_needs_rebuild( + *middleware_registry_status, + current_middleware_services, + &snapshot.supervisor_middleware_services, + ); + + let outcome = if reloads_gateway_policy { + let runtime_changed = *current_policy_hash != snapshot.policy_hash || registry_changed; + if runtime_changed { + match reload_gateway_policy_runtime( + &ctx.opa_engine, + snapshot.policy.as_ref(), + ctx.entrypoint_pid.load(Ordering::Acquire), + MiddlewareReloadContext { + desired_services: &snapshot.supervisor_middleware_services, + authentication: &MiddlewareAuthentication { + credentials: middleware_credentials, + enabled: snapshot.extension_authentication_enabled, + }, + registry_changed, + connector: &ctx.middleware_connector, + }, + ctx.transparent_tcp, + ) + .await + { + Ok(()) => { + if let Some(policy) = snapshot.policy.as_ref() { + if let Some(policy_local_ctx) = ctx.policy_local_ctx.as_ref() { + policy_local_ctx.set_current_policy(policy.clone()).await; + } + if let Some(publisher) = ctx.sidecar_control_publisher.as_ref() { + publisher.publish_policy( + policy.clone(), + snapshot.policy_hash.clone(), + snapshot.config_revision, + ); + } + } + *has_last_valid_policy = true; + current_policy_hash.clone_from(&snapshot.policy_hash); + current_middleware_services + .clone_from(&snapshot.supervisor_middleware_services); + *current_extension_authentication_enabled = + snapshot.extension_authentication_enabled; + *middleware_registry_status = MiddlewareRegistryStatus::Synchronized; + Ok(ConfigApplyOutcome::Applied) + } + Err(failure) => { + let failure_mode = snapshot.policy_validation_failure_mode; + let error = match apply_gateway_runtime_reload_failure( + &ctx.opa_engine, + failure, + failure_mode, + *has_last_valid_policy, + snapshot.version, + ) { + Ok( + GatewayRuntimeFailureDisposition::PolicyRejected { error, .. } + | GatewayRuntimeFailureDisposition::MiddlewareUnavailable { error } + | GatewayRuntimeFailureDisposition::TransparentTcpExpansionRejected { + error, + .. + }, + ) => error, + Err(error) => error.to_string(), + }; + Err(error) + } + } + } else { + Ok(ConfigApplyOutcome::Applied) + } + } else { + reconcile_middleware_registry( + &ctx.opa_engine, + &ctx.middleware_connector, + MiddlewareRegistryReconciliation { + desired_services: &snapshot.supervisor_middleware_services, + authentication: MiddlewareAuthentication { + credentials: middleware_credentials, + enabled: snapshot.extension_authentication_enabled, + }, + registry_changed, + extension_credentials: &ctx.extension_credentials, + current_services: current_middleware_services, + status: middleware_registry_status, + }, + ) + .await; + Ok(ConfigApplyOutcome::RetainedLocalOverride) + }; + + log_setting_changes(current_settings, &snapshot.settings); + apply_ocsf_json_setting(&ctx.ocsf_enabled, &snapshot.settings); + apply_ocsf_schema_version_setting(&ctx.ocsf_schema_version, &snapshot.settings); + apply_agent_proposals_enabled( + &ctx.agent_proposals, + agent_proposals_enabled_from_settings(&snapshot.settings), + "stream snapshot", + Some(snapshot.config_revision), + ctx.sidecar_control_publisher.as_ref(), + skills::install_static_skills, + ); + *current_settings = snapshot.settings; + + match outcome { + Ok(outcome) => { + *current_config_revision = snapshot.config_revision; + if snapshot.version > 0 && snapshot.policy_source == PolicySource::Sandbox { + *current_policy_version = snapshot.version; + } + let applied_revision = (outcome != ConfigApplyOutcome::RetainedLocalOverride) + .then_some(requested_revision); + if let Some(applied_revision) = applied_revision.as_ref() { + *current_stream_revision = Some(*applied_revision); + } + config_apply_result( + ConfigComponent::SandboxConfig, + requested_revision, + applied_revision, + outcome, + None, + ) + } + Err(error) => { + let outcome = if snapshot.policy_validation_failure_mode + == PolicyValidationFailureMode::RetainLastValid + && *has_last_valid_policy + { + ConfigApplyOutcome::FailedRetainedLastKnownGood + } else { + ConfigApplyOutcome::FailedClosed + }; + let applied_revision = (outcome == ConfigApplyOutcome::FailedRetainedLastKnownGood) + .then_some(*current_stream_revision) + .flatten(); + config_apply_result( + ConfigComponent::SandboxConfig, + requested_revision, + applied_revision, + outcome, + Some(("runtime_apply_failed", error, true)), + ) + } + } +} + async fn run_policy_poll_loop(ctx: PolicyPollLoopContext) -> Result<()> { let client = openshell_core::grpc_client::CachedOpenShellClient::connect_with_credentials( &ctx.endpoint, @@ -3740,12 +4169,13 @@ async fn run_policy_poll_loop(ctx: PolicyPollLoopContext) -> Result<()> { } async fn run_policy_poll_loop_with_client( - ctx: PolicyPollLoopContext, + mut ctx: PolicyPollLoopContext, client: C, ) -> Result<()> { use openshell_core::proto::PolicySource; use std::sync::atomic::Ordering; + let mut config_apply_rx = ctx.config_apply_rx.take(); let (status_sender, status_receiver) = tokio::sync::mpsc::unbounded_channel(); tokio::spawn(run_policy_status_reporter( client.clone(), @@ -3754,6 +4184,7 @@ async fn run_policy_poll_loop_with_client( )); let mut current_config_revision: u64 = 0; + let mut current_stream_sandbox_revision = None; let mut current_provider_env_revision: u64 = ctx.provider_credentials.snapshot().revision; let mut current_policy_version: u32 = 0; let mut current_policy_hash = String::new(); @@ -3782,6 +4213,7 @@ async fn run_policy_poll_loop_with_client( let _ = ctx.workspace_tx.send(client.workspace()); match initial_poll_disposition(&ctx.loaded_policy_origin, &result) { InitialPollDisposition::Acknowledge(candidate) => { + let stream_revision = sandbox_config_revision(&result); apply_ocsf_json_setting(&ctx.ocsf_enabled, &result.settings); apply_ocsf_schema_version_setting(&ctx.ocsf_schema_version, &result.settings); apply_agent_proposals_enabled( @@ -3799,6 +4231,7 @@ async fn run_policy_poll_loop_with_client( current_extension_authentication_enabled = result.extension_authentication_enabled; current_settings = result.settings; + current_stream_sandbox_revision = Some(stream_revision); enqueue_policy_status( &status_sender, PolicyStatusUpdate::initial_loaded(&candidate), @@ -3843,7 +4276,31 @@ async fn run_policy_poll_loop_with_client( let result = if let Some(result) = pending_result.take() { result } else { - tokio::time::sleep(next_poll_delay(&ctx.extension_credentials, interval)).await; + let delay = next_poll_delay(&ctx.extension_credentials, interval); + tokio::select! { + request = receive_config_apply(&mut config_apply_rx) => { + if let Some(request) = request { + apply_stream_config_request( + &ctx, + &client, + request, + &mut current_config_revision, + &mut current_stream_sandbox_revision, + &mut current_provider_env_revision, + &mut current_policy_version, + &mut current_policy_hash, + &mut current_middleware_services, + &mut current_extension_authentication_enabled, + &mut middleware_registry_status, + &mut current_settings, + reloads_gateway_policy, + &mut has_last_valid_policy, + ).await; + continue; + } + } + () = tokio::time::sleep(delay) => {} + } match client.poll_settings(&ctx.sandbox_id).await { Ok(result) => { let _ = ctx.workspace_tx.send(client.workspace()); @@ -4318,6 +4775,9 @@ async fn run_policy_poll_loop_with_client( skills::install_static_skills, ); + if reloads_gateway_policy && (!policy_runtime_changed || policy_runtime_reconciled) { + current_stream_sandbox_revision = Some(sandbox_config_revision(&result)); + } current_config_revision = result.config_revision; if !reloads_gateway_policy { current_policy_hash = result.policy_hash; @@ -5095,9 +5555,184 @@ network_policies: extension_authentication_enabled: false, middleware_connector, transparent_tcp: TransparentTcpReloadState::default(), + config_apply_rx: None, } } + #[tokio::test] + async fn stream_provider_snapshot_applies_without_fetching() { + let engine = + Arc::new(OpaEngine::from_proto(&proto_policy_fixture()).expect("build OPA engine")); + let ctx = policy_poll_test_context( + engine, + LoadedPolicyOrigin::Gateway { + revision: None, + has_last_valid_policy: true, + }, + default_middleware_connector(), + ); + let mut revision = 0; + let result = apply_stream_provider_snapshot( + &ctx, + openshell_core::proto::ProviderEnvironmentSnapshot { + provider_env_revision: 17, + values: vec![openshell_core::proto::ProviderEnvironmentValue { + name: "REGION".to_string(), + value: "west".to_string(), + classification: + openshell_core::proto::ProviderEnvironmentValueClassification::NonSecret + .into(), + ..Default::default() + }], + ..Default::default() + }, + &mut revision, + ); + + assert_eq!(revision, 17); + assert_eq!( + openshell_core::proto::ConfigApplyOutcome::try_from(result.outcome).unwrap(), + openshell_core::proto::ConfigApplyOutcome::Applied + ); + assert!( + ctx.provider_credentials + .snapshot() + .child_env + .contains_key("REGION") + ); + } + + #[tokio::test] + async fn stream_sandbox_snapshot_applies_without_fetching() { + let initial = settings_poll_result( + Some(proto_policy_fixture()), + 1, + openshell_core::proto::PolicySource::Sandbox, + ); + let desired = settings_poll_result( + Some(proto_policy_fixture()), + 2, + openshell_core::proto::PolicySource::Sandbox, + ); + let engine = + Arc::new(OpaEngine::from_proto(&proto_policy_fixture()).expect("build OPA engine")); + let ctx = policy_poll_test_context( + engine, + LoadedPolicyOrigin::Gateway { + revision: Some(LoadedPolicyRevision::from_snapshot(&initial)), + has_last_valid_policy: true, + }, + default_middleware_connector(), + ); + let (client, _polls, _reports) = scripted_policy_gateway(); + let mut config_revision = initial.config_revision; + let mut stream_revision = Some(sandbox_config_revision(&initial)); + let mut policy_version = initial.version; + let mut policy_hash = initial.policy_hash; + let mut middleware_services = Vec::new(); + let mut extension_authentication_enabled = false; + let mut middleware_registry_status = MiddlewareRegistryStatus::Synchronized; + let mut settings = std::collections::HashMap::new(); + let mut has_last_valid_policy = true; + + let result = apply_stream_sandbox_snapshot( + &ctx, + &client, + desired.clone(), + &mut config_revision, + &mut stream_revision, + &mut policy_version, + &mut policy_hash, + &mut middleware_services, + &mut extension_authentication_enabled, + &mut middleware_registry_status, + &mut settings, + true, + &mut has_last_valid_policy, + ) + .await; + + assert_eq!(config_revision, desired.config_revision); + assert_eq!(policy_version, desired.version); + assert_eq!(policy_hash, desired.policy_hash); + assert_eq!( + openshell_core::proto::ConfigApplyOutcome::try_from(result.outcome).unwrap(), + openshell_core::proto::ConfigApplyOutcome::Applied + ); + } + + #[tokio::test] + async fn failed_stream_snapshot_remains_retryable() { + let initial = settings_poll_result( + Some(proto_policy_fixture()), + 1, + openshell_core::proto::PolicySource::Sandbox, + ); + let mut desired = settings_poll_result( + Some(proto_policy_fixture()), + 2, + openshell_core::proto::PolicySource::Sandbox, + ); + desired.policy_validation_failure_mode = PolicyValidationFailureMode::RetainLastValid; + desired.supervisor_middleware_services = + vec![openshell_core::proto::SupervisorMiddlewareService { + name: "unavailable-guard".into(), + grpc_endpoint: "http://127.0.0.1:1".into(), + max_payload_bytes: 1024, + ..Default::default() + }]; + let engine = + Arc::new(OpaEngine::from_proto(&proto_policy_fixture()).expect("build OPA engine")); + install_builtin_middleware_registry(&engine) + .await + .expect("install built-in middleware registry"); + let ctx = policy_poll_test_context( + engine, + LoadedPolicyOrigin::Gateway { + revision: Some(LoadedPolicyRevision::from_snapshot(&initial)), + has_last_valid_policy: true, + }, + default_middleware_connector(), + ); + let (client, _polls, _reports) = scripted_policy_gateway(); + let mut config_revision = initial.config_revision; + let initial_revision = sandbox_config_revision(&initial); + let mut stream_revision = Some(initial_revision); + let mut policy_version = initial.version; + let mut policy_hash = initial.policy_hash.clone(); + let mut middleware_services = Vec::new(); + let mut extension_authentication_enabled = false; + let mut middleware_registry_status = MiddlewareRegistryStatus::Synchronized; + let mut settings = std::collections::HashMap::new(); + let mut has_last_valid_policy = true; + + let result = apply_stream_sandbox_snapshot( + &ctx, + &client, + desired, + &mut config_revision, + &mut stream_revision, + &mut policy_version, + &mut policy_hash, + &mut middleware_services, + &mut extension_authentication_enabled, + &mut middleware_registry_status, + &mut settings, + true, + &mut has_last_valid_policy, + ) + .await; + + assert_eq!(config_revision, initial.config_revision); + assert_eq!(policy_version, initial.version); + assert_eq!(policy_hash, initial.policy_hash); + assert_eq!(result.applied_revision, Some(initial_revision)); + assert_eq!( + openshell_core::proto::ConfigApplyOutcome::try_from(result.outcome).unwrap(), + openshell_core::proto::ConfigApplyOutcome::FailedRetainedLastKnownGood + ); + } + async fn expect_policy_report( reports: &mut tokio::sync::mpsc::UnboundedReceiver<(u32, bool, String)>, version: u32, diff --git a/crates/openshell-server/src/config_delivery.rs b/crates/openshell-server/src/config_delivery.rs index 2437ada722..53ab1d4c34 100644 --- a/crates/openshell-server/src/config_delivery.rs +++ b/crates/openshell-server/src/config_delivery.rs @@ -28,7 +28,10 @@ pub const MAX_SUPERVISOR_CONFIG_MESSAGE_BYTES: usize = 3 * 1024 * 1024; const CONFIG_SNAPSHOT_BUILD_TIMEOUT: Duration = Duration::from_secs(45); // Stage 1 bootstrap is optional. Keep credential backend stalls well below // the 15-second relay session-wait budget while polling remains authoritative. -const CONFIG_BOOTSTRAP_BUILD_TIMEOUT: Duration = Duration::from_secs(1); +pub const OPTIONAL_CONFIG_BOOTSTRAP_BUILD_TIMEOUT: Duration = Duration::from_secs(1); +// Stage 2 supervisors apply the bootstrap directly, so allow the same bounded +// build window as an ordinary complete snapshot before rejecting the session. +pub const REQUIRED_CONFIG_BOOTSTRAP_BUILD_TIMEOUT: Duration = CONFIG_SNAPSHOT_BUILD_TIMEOUT; const MAX_ACTIVE_FANOUT_WORKERS: usize = 64; /// Concurrent snapshot builds allowed per pooled database connection. Builds /// are short bursts of small queries, so a little oversubscription keeps the @@ -65,6 +68,8 @@ impl fmt::Debug for SupervisorConfigMessage { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum DeliveryDisposition { Enqueued, + Coalesced, + SuppressedUnchanged, NoActiveSession, QueueFull, SessionClosed, @@ -354,13 +359,11 @@ enum FanoutEnqueue { pub async fn build_config_bootstrap( state: &Arc, sandbox: &Sandbox, + timeout: Duration, ) -> Result { - tokio::time::timeout( - CONFIG_BOOTSTRAP_BUILD_TIMEOUT, - build_consistent_config_bootstrap(state, sandbox), - ) - .await - .map_err(|_| Status::deadline_exceeded("supervisor configuration bootstrap timed out"))? + tokio::time::timeout(timeout, build_consistent_config_bootstrap(state, sandbox)) + .await + .map_err(|_| Status::deadline_exceeded("supervisor configuration bootstrap timed out"))? } async fn build_consistent_config_bootstrap( @@ -587,6 +590,8 @@ fn record_delivery_worker_full(sandbox_id: &str, component: &'static str) { fn record_delivery(component: &'static str, disposition: DeliveryDisposition) { let outcome = match disposition { DeliveryDisposition::Enqueued => "enqueued", + DeliveryDisposition::Coalesced => "coalesced", + DeliveryDisposition::SuppressedUnchanged => "unchanged", DeliveryDisposition::NoActiveSession => "no_active_session", DeliveryDisposition::QueueFull => "queue_full", DeliveryDisposition::SessionClosed => "session_closed", @@ -600,6 +605,20 @@ fn record_delivery(component: &'static str, disposition: DeliveryDisposition) { .increment(1); } +/// Periodically rebuild current snapshots for every locally routable session. +/// This repairs missed mutation notifications and queue pressure without a +/// supervisor fetch. +pub fn spawn_owner_reconciler(state: Arc, interval: Duration) { + tokio::spawn(async move { + let mut timer = tokio::time::interval(interval); + timer.tick().await; + loop { + timer.tick().await; + publish_all_connected(&state, ConfigComponents::ALL); + } + }); +} + fn record_build_failure(sandbox_id: &str, component: &'static str, error_code: Code) { counter!( "openshell_supervisor_config_snapshot_failures_total", @@ -947,7 +966,7 @@ mod tests { let connect = connect_supervisor_stream( &state, "sandbox", - openshell_core::proto::SUPERVISOR_PROTOCOL_REVISION, + openshell_core::proto::PREVIOUS_SUPERVISOR_PROTOCOL_REVISION, ); let (response, hit) = tokio::join!(connect, resolve_hit); hit.expect("bootstrap must reach the stalled credential driver"); diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 77c997e046..9517d9b04e 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -28,6 +28,7 @@ use crate::provider_profile_sources::ProviderProfileSources; use crate::storage_proto::StoredProviderCredentialRefreshState; #[cfg(test)] use crate::storage_proto::StoredProviderProfile; +use metrics::counter; use openshell_core::net::{is_always_blocked_ip, is_internal_ip}; #[cfg(test)] use openshell_core::proto::StaticCredentialBinding; @@ -4318,70 +4319,96 @@ pub(super) async fn handle_report_policy_status( return Err(Status::invalid_argument("version is required")); } - let version = i64::from(req.version); let status_str = match PolicyStatus::try_from(req.status) { Ok(PolicyStatus::Loaded) => "loaded", Ok(PolicyStatus::Failed) => "failed", _ => return Err(Status::invalid_argument("status must be LOADED or FAILED")), }; - let loaded_at_ms = if status_str == "loaded" { - Some(current_time_ms()) - } else { - None - }; + record_policy_apply_result( + state, + &req.sandbox_id, + req.version, + status_str == "loaded", + (!req.load_error.is_empty()).then_some(req.load_error.as_str()), + "polling", + ) + .await?; - let load_error = if status_str == "failed" && !req.load_error.is_empty() { - Some(req.load_error.as_str()) - } else { - None - }; + info!( + sandbox_id = %req.sandbox_id, + version = req.version, + status = %status_str, + "ReportPolicyStatus: sandbox reported policy load result" + ); + + Ok(Response::new(ReportPolicyStatusResponse {})) +} +/// Persist the exact sandbox policy revision attempted by a supervisor. +/// Unary rollout reports and stream acknowledgements share this path. +pub async fn record_policy_apply_result( + state: &Arc, + sandbox_id: &str, + version: u32, + loaded: bool, + load_error: Option<&str>, + source: &'static str, +) -> Result<(), Status> { + if sandbox_id.is_empty() || version == 0 { + return Err(Status::invalid_argument( + "sandbox_id and policy version are required", + )); + } + let status = if loaded { "loaded" } else { "failed" }; + counter!( + "openshell_supervisor_policy_apply_results_total", + "source" => source, + "outcome" => status, + ) + .increment(1); + let sanitized_error = (!loaded).then(|| { + load_error + .unwrap_or_default() + .chars() + .take(1024) + .collect::() + }); + let version_i64 = i64::from(version); let updated = state .store .update_policy_status( - &req.sandbox_id, - version, - status_str, - load_error, - loaded_at_ms, + sandbox_id, + version_i64, + status, + sanitized_error.as_deref().filter(|error| !error.is_empty()), + loaded.then(current_time_ms), ) .await - .map_err(|e| Status::internal(format!("update policy status failed: {e}")))?; - + .map_err(|error| Status::internal(format!("update policy status failed: {error}")))?; if !updated { return Err(Status::not_found("policy revision not found")); } - - if status_str == "loaded" { + if loaded { let _ = state .store - .supersede_older_policies(&req.sandbox_id, version) + .supersede_older_policies(sandbox_id, version_i64) .await; - - // Update current_policy_version using CAS - // TODO: Accept expected_version from UpdateConfigRequest for proper client-driven CAS let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; - let version_to_set = req.version; state .store - .update_message_cas::(&req.sandbox_id, 0, |sandbox| { - sandbox.set_current_policy_version(version_to_set); + .update_message_cas::(sandbox_id, 0, |sandbox| { + if sandbox.current_policy_version() < version { + sandbox.set_current_policy_version(version); + } }) .await - .map_err(|e| super::persistence_error_to_status(e, "update current_policy_version"))?; - - state.sandbox_watch_bus.notify(&req.sandbox_id); + .map_err(|error| { + super::persistence_error_to_status(error, "update current_policy_version") + })?; + state.sandbox_watch_bus.notify(sandbox_id); } - - info!( - sandbox_id = %req.sandbox_id, - version = req.version, - status = %status_str, - "ReportPolicyStatus: sandbox reported policy load result" - ); - - Ok(Response::new(ReportPolicyStatusResponse {})) + Ok(()) } // --------------------------------------------------------------------------- diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 8c0870d26a..549782f8be 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -729,6 +729,7 @@ pub(crate) async fn run_server( state.compute.spawn_watchers(shutdown_rx.clone()); ssh_sessions::spawn_session_reaper(store.clone(), Duration::from_hours(1)); supervisor_session::spawn_relay_reaper(state.clone(), Duration::from_secs(30)); + config_delivery::spawn_owner_reconciler(state.clone(), Duration::from_secs(30)); provider_refresh::spawn_refresh_worker(state.clone(), Duration::from_mins(1)); // Create the multiplexed service diff --git a/crates/openshell-server/src/supervisor_session.rs b/crates/openshell-server/src/supervisor_session.rs index 5cb66ac16f..72a626566a 100644 --- a/crates/openshell-server/src/supervisor_session.rs +++ b/crates/openshell-server/src/supervisor_session.rs @@ -16,11 +16,16 @@ use tracing::{debug, info, warn}; use uuid::Uuid; use openshell_core::proto::{ - ConfigUpdate, GatewayMessage, RelayFrame, RelayInit, RelayOpen, ReportMainProcessExitRequest, - ReportMainProcessExitResponse, Sandbox, SandboxPhase, SessionAccepted, SshRelayTarget, - SupervisorMessage, config_update, gateway_message, relay_open, supervisor_message, + ConfigApplyOutcome, ConfigBootstrap, ConfigComponent, ConfigComponentApplyResult, + ConfigSnapshotRevision, ConfigUpdate, ConfigUpdateResult, GatewayMessage, PolicySource, + RelayFrame, RelayInit, RelayOpen, ReportMainProcessExitRequest, ReportMainProcessExitResponse, + Sandbox, SandboxPhase, SessionAccepted, SshRelayTarget, SupervisorMessage, + config_snapshot_revision, config_update, gateway_message, relay_open, supervisor_message, +}; +use openshell_core::proto::{ + LEGACY_SUPERVISOR_PROTOCOL_REVISION, PREVIOUS_SUPERVISOR_PROTOCOL_REVISION, + SUPERVISOR_PROTOCOL_REVISION, }; -use openshell_core::proto::{LEGACY_SUPERVISOR_PROTOCOL_REVISION, SUPERVISOR_PROTOCOL_REVISION}; use openshell_core::transport_errors::is_expected_transport_close_status; use crate::ServerState; @@ -76,8 +81,24 @@ struct LiveSession { #[derive(Debug, Default)] struct ConfigSequences { - sandbox_config: u64, - provider_environment: u64, + sandbox_config: ComponentDeliveryState, + provider_environment: ComponentDeliveryState, +} + +#[derive(Debug, Default)] +struct ComponentDeliveryState { + sequence: u64, + in_flight: Option, + pending: Option, + last_acknowledged_revision: Option, +} + +#[derive(Debug)] +struct InFlightConfigUpdate { + update_id: String, + component_sequence: u64, + revision: ConfigSnapshotRevision, + sent_at: Instant, } /// Holds a oneshot sender that will deliver the upgraded relay stream or a @@ -117,6 +138,109 @@ impl std::fmt::Debug for SupervisorSessionRegistry { } } +fn build_config_update( + state: &mut ComponentDeliveryState, + message: SupervisorConfigMessage, +) -> (GatewayMessage, InFlightConfigUpdate) { + state.sequence = state.sequence.saturating_add(1); + let component_sequence = state.sequence; + let (component, revision) = match message { + SupervisorConfigMessage::SandboxConfig(snapshot) => { + let revision = ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::SandboxConfig( + openshell_core::proto::SandboxConfigRevision { + config_revision: snapshot.config_revision, + policy_version: snapshot.version, + policy_source: snapshot.policy_source, + global_policy_version: snapshot.global_policy_version, + }, + )), + }; + (config_update::Component::SandboxConfig(*snapshot), revision) + } + SupervisorConfigMessage::ProviderEnvironment(snapshot) => { + let revision = ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::ProviderEnvironment( + snapshot.provider_env_revision, + )), + }; + ( + config_update::Component::ProviderEnvironment(snapshot), + revision, + ) + } + }; + let update_id = Uuid::new_v4().to_string(); + ( + GatewayMessage { + payload: Some(gateway_message::Payload::ConfigUpdate(ConfigUpdate { + update_id: update_id.clone(), + component_sequence, + component: Some(component), + })), + }, + InFlightConfigUpdate { + update_id, + component_sequence, + revision, + sent_at: Instant::now(), + }, + ) +} + +fn config_message_revision(message: &SupervisorConfigMessage) -> ConfigSnapshotRevision { + match message { + SupervisorConfigMessage::SandboxConfig(snapshot) => ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::SandboxConfig( + openshell_core::proto::SandboxConfigRevision { + config_revision: snapshot.config_revision, + policy_version: snapshot.version, + policy_source: snapshot.policy_source, + global_policy_version: snapshot.global_policy_version, + }, + )), + }, + SupervisorConfigMessage::ProviderEnvironment(snapshot) => ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::ProviderEnvironment( + snapshot.provider_env_revision, + )), + }, + } +} + +fn validate_component_apply_result( + result: &ConfigComponentApplyResult, + requested_revision: &ConfigSnapshotRevision, +) -> Result { + if result.requested_revision.as_ref() != Some(requested_revision) { + return Err(Status::invalid_argument( + "configuration result revision does not match the delivered snapshot", + )); + } + let outcome = ConfigApplyOutcome::try_from(result.outcome).unwrap_or_default(); + let applied_matches_request = result.applied_revision.as_ref() == Some(requested_revision); + let applied_is_absent = result.applied_revision.is_none(); + let valid = match outcome { + ConfigApplyOutcome::Applied + | ConfigApplyOutcome::IgnoredDuplicate + | ConfigApplyOutcome::Degraded => applied_matches_request, + ConfigApplyOutcome::RetainedLocalOverride | ConfigApplyOutcome::FailedClosed => { + applied_is_absent + } + ConfigApplyOutcome::FailedRetainedLastKnownGood => { + result.applied_revision.is_some() && !applied_matches_request + } + ConfigApplyOutcome::IgnoredStale | ConfigApplyOutcome::Unsupported => true, + ConfigApplyOutcome::Unspecified => false, + }; + if !valid { + return Err(Status::invalid_argument( + "configuration result outcome does not match its applied revision", + )); + } + Ok(outcome) +} + impl SupervisorSessionRegistry { pub fn new() -> Self { Self::default() @@ -263,7 +387,7 @@ impl SupervisorSessionRegistry { let Some(session) = sessions.get_mut(sandbox_id) else { return DeliveryDisposition::NoActiveSession; }; - let sequence = match &message { + let delivery_state = match &message { SupervisorConfigMessage::SandboxConfig(_) => { &mut session.config_sequences.sandbox_config } @@ -271,31 +395,34 @@ impl SupervisorSessionRegistry { &mut session.config_sequences.provider_environment } }; - *sequence = sequence.saturating_add(1); - let component_sequence = *sequence; + if delivery_state.in_flight.is_none() + && delivery_state.last_acknowledged_revision.as_ref() + == Some(&config_message_revision(&message)) + { + return DeliveryDisposition::SuppressedUnchanged; + } + if delivery_state + .in_flight + .as_ref() + .is_some_and(|update| update.sent_at.elapsed() < Duration::from_mins(1)) + { + delivery_state.pending = Some(message); + return DeliveryDisposition::Coalesced; + } + delivery_state.in_flight = None; + delivery_state.pending = None; - let component = match message { - SupervisorConfigMessage::SandboxConfig(snapshot) => { - config_update::Component::SandboxConfig(*snapshot) - } - SupervisorConfigMessage::ProviderEnvironment(snapshot) => { - config_update::Component::ProviderEnvironment(snapshot) - } - }; - let gateway_message = GatewayMessage { - payload: Some(gateway_message::Payload::ConfigUpdate(ConfigUpdate { - update_id: Uuid::new_v4().to_string(), - component_sequence, - component: Some(component), - })), - }; + let (gateway_message, in_flight) = build_config_update(delivery_state, message); if gateway_message.encoded_len() > MAX_SUPERVISOR_CONFIG_MESSAGE_BYTES { return DeliveryDisposition::PayloadTooLarge; } match session.tx.try_send(gateway_message) { - Ok(()) => DeliveryDisposition::Enqueued, + Ok(()) => { + delivery_state.in_flight = Some(in_flight); + DeliveryDisposition::Enqueued + } Err(mpsc::error::TrySendError::Full(_)) => { warn!( sandbox_id = %sandbox_id, @@ -308,6 +435,105 @@ impl SupervisorSessionRegistry { } } + fn complete_config_update( + &self, + sandbox_id: &str, + session_id: &str, + result: &ConfigUpdateResult, + ) -> Result<(), Status> { + let component = result + .result + .as_ref() + .and_then(|result| ConfigComponent::try_from(result.component).ok()) + .unwrap_or_default(); + let mut sessions = self.sessions.lock().unwrap(); + let session = sessions + .get_mut(sandbox_id) + .filter(|session| session.session_id == session_id) + .ok_or_else(|| Status::failed_precondition("obsolete supervisor session result"))?; + let delivery_state = match component { + ConfigComponent::SandboxConfig => &mut session.config_sequences.sandbox_config, + ConfigComponent::ProviderEnvironment => { + &mut session.config_sequences.provider_environment + } + ConfigComponent::Unspecified => { + return Err(Status::invalid_argument( + "configuration result component is required", + )); + } + }; + let in_flight = delivery_state + .in_flight + .as_ref() + .ok_or_else(|| Status::failed_precondition("no matching update is in flight"))?; + let component_result = result + .result + .as_ref() + .ok_or_else(|| Status::invalid_argument("configuration result is required"))?; + if in_flight.update_id != result.update_id + || in_flight.component_sequence != result.component_sequence + { + return Err(Status::invalid_argument( + "configuration result does not match the in-flight delivery", + )); + } + let outcome = validate_component_apply_result(component_result, &in_flight.revision)?; + if matches!( + outcome, + ConfigApplyOutcome::Applied + | ConfigApplyOutcome::IgnoredDuplicate + | ConfigApplyOutcome::RetainedLocalOverride + | ConfigApplyOutcome::Degraded + ) { + delivery_state.last_acknowledged_revision = Some(in_flight.revision); + } + delivery_state.in_flight = None; + if let Some(pending) = delivery_state.pending.take() { + if delivery_state.last_acknowledged_revision.as_ref() + == Some(&config_message_revision(&pending)) + { + return Ok(()); + } + let (message, next) = build_config_update(delivery_state, pending); + match session.tx.try_send(message) { + Ok(()) => delivery_state.in_flight = Some(next), + Err(mpsc::error::TrySendError::Full(_) | mpsc::error::TrySendError::Closed(_)) => { + // Owner reconciliation rebuilds the newest snapshot. + } + } + } + Ok(()) + } + + fn retry_config_update_after_persistence_failure( + &self, + sandbox_id: &str, + session_id: &str, + result: &ConfigComponentApplyResult, + ) { + let component = ConfigComponent::try_from(result.component).unwrap_or_default(); + let Some(requested_revision) = result.requested_revision.as_ref() else { + return; + }; + let mut sessions = self.sessions.lock().unwrap(); + let Some(session) = sessions + .get_mut(sandbox_id) + .filter(|session| session.session_id == session_id) + else { + return; + }; + let delivery_state = match component { + ConfigComponent::SandboxConfig => &mut session.config_sequences.sandbox_config, + ConfigComponent::ProviderEnvironment => { + &mut session.config_sequences.provider_environment + } + ConfigComponent::Unspecified => return, + }; + if delivery_state.last_acknowledged_revision.as_ref() == Some(requested_revision) { + delivery_state.last_acknowledged_revision = None; + } + } + pub fn is_current_session(&self, sandbox_id: &str, session_id: &str) -> bool { self.sessions .lock() @@ -807,6 +1033,7 @@ pub async fn handle_connect_supervisor( }; let sandbox_id = hello.sandbox_id.clone(); + let stream_applies_config = hello.protocol_revision == SUPERVISOR_PROTOCOL_REVISION; if sandbox_id.is_empty() { return Err(Status::invalid_argument("sandbox_id is required")); } @@ -816,29 +1043,44 @@ pub async fn handle_connect_supervisor( } let sandbox = require_persisted_sandbox(&state.store, &sandbox_id).await?; - let bootstrap = match crate::config_delivery::build_config_bootstrap(state, &sandbox).await { - Ok(bootstrap) => { - counter!( - "openshell_supervisor_config_bootstrap_total", - "outcome" => "built" - ) - .increment(1); - Some(bootstrap) - } - Err(error) => { - counter!( - "openshell_supervisor_config_bootstrap_total", - "outcome" => "build_failed" - ) - .increment(1); - warn!( - sandbox_id = %sandbox_id, - error_code = ?error.code(), - "failed to build supervisor configuration bootstrap" - ); - None - } + let bootstrap_timeout = if stream_applies_config { + crate::config_delivery::REQUIRED_CONFIG_BOOTSTRAP_BUILD_TIMEOUT + } else { + crate::config_delivery::OPTIONAL_CONFIG_BOOTSTRAP_BUILD_TIMEOUT }; + let bootstrap = + match crate::config_delivery::build_config_bootstrap(state, &sandbox, bootstrap_timeout) + .await + { + Ok(bootstrap) => { + counter!( + "openshell_supervisor_config_bootstrap_total", + "outcome" => "built" + ) + .increment(1); + Some(bootstrap) + } + Err(error) => { + counter!( + "openshell_supervisor_config_bootstrap_total", + "outcome" => "build_failed" + ) + .increment(1); + warn!( + sandbox_id = %sandbox_id, + error_code = ?error.code(), + "failed to build supervisor configuration bootstrap" + ); + if stream_applies_config { + return Err(error); + } + None + } + }; + let expected_bootstrap_revisions = bootstrap + .as_ref() + .map(bootstrap_revision_fence) + .unwrap_or_default(); let session_id = Uuid::new_v4().to_string(); info!( @@ -857,7 +1099,7 @@ pub async fn handle_connect_supervisor( session_id: session_id.clone(), heartbeat_interval_secs: HEARTBEAT_INTERVAL_SECS, bootstrap, - protocol_revision: SUPERVISOR_PROTOCOL_REVISION, + protocol_revision: hello.protocol_revision, })), }; if accepted.encoded_len() > MAX_SUPERVISOR_CONFIG_MESSAGE_BYTES { @@ -866,6 +1108,11 @@ pub async fn handle_connect_supervisor( "outcome" => "payload_too_large" ) .increment(1); + if stream_applies_config { + return Err(Status::resource_exhausted( + "supervisor configuration bootstrap exceeds the stream message limit", + )); + } let Some(gateway_message::Payload::SessionAccepted(accepted_payload)) = accepted.payload.as_mut() else { @@ -898,29 +1145,23 @@ pub async fn handle_connect_supervisor( .await; } - if let Err(err) = state - .compute - .supervisor_session_connected(&sandbox_id, &hello.instance_id) - .await - { - warn!( - sandbox_id = %sandbox_id, - session_id = %session_id, - error = %err, - "supervisor session: failed to mark sandbox ready" - ); - } else { - state.telemetry.sandbox_session_connected(&sandbox_id); + if !stream_applies_config { + let _ = + mark_supervisor_initialized(state, &sandbox_id, &session_id, &hello.instance_id).await; } // Step 4: Spawn the session loop that reads inbound messages. let state_clone = Arc::clone(state); let sandbox_id_clone = sandbox_id.clone(); + let instance_id = hello.instance_id.clone(); tokio::spawn(async move { run_session_loop( &state_clone, &sandbox_id_clone, &session_id, + &instance_id, + stream_applies_config, + &expected_bootstrap_revisions, &tx, &mut inbound, shutdown_rx, @@ -963,6 +1204,14 @@ pub async fn handle_connect_supervisor( fn validate_protocol_revision(sandbox_id: &str, supervisor_revision: u32) -> Result<(), Status> { match supervisor_revision { SUPERVISOR_PROTOCOL_REVISION => Ok(()), + PREVIOUS_SUPERVISOR_PROTOCOL_REVISION => { + counter!("openshell_supervisor_protocol_previous_sessions_total").increment(1); + warn!( + sandbox_id = %sandbox_id, + "supervisor session: Stage 1 supervisor is using polling compatibility" + ); + Ok(()) + } LEGACY_SUPERVISOR_PROTOCOL_REVISION => { counter!("openshell_supervisor_protocol_legacy_sessions_total").increment(1); warn!( @@ -1033,10 +1282,14 @@ pub async fn handle_finalize_main_process_exit( )) } +#[allow(clippy::too_many_arguments)] async fn run_session_loop( state: &Arc, sandbox_id: &str, session_id: &str, + instance_id: &str, + stream_applies_config: bool, + expected_bootstrap_revisions: &[(ConfigComponent, ConfigSnapshotRevision)], tx: &mpsc::Sender, inbound: &mut tonic::Streaming, mut shutdown_rx: oneshot::Receiver<()>, @@ -1045,6 +1298,9 @@ async fn run_session_loop( let mut heartbeat_timer = tokio::time::interval(heartbeat_interval); // Skip the first immediate tick. heartbeat_timer.tick().await; + let bootstrap_timeout = tokio::time::sleep(Duration::from_mins(2)); + tokio::pin!(bootstrap_timeout); + let mut bootstrap_complete = !stream_applies_config; loop { tokio::select! { @@ -1055,7 +1311,55 @@ async fn run_session_loop( msg = inbound.message() => { match msg { Ok(Some(msg)) => { - handle_supervisor_message(state, sandbox_id, session_id, msg); + let bootstrap_succeeded = match msg.payload.as_ref() { + Some(supervisor_message::Payload::ConfigBootstrapResult(result)) + if stream_applies_config => + { + match validate_bootstrap_result( + &result.results, + expected_bootstrap_revisions, + ) { + Ok(succeeded) => Some(succeeded), + Err(error) => { + warn!( + sandbox_id, + session_id, + error = %error, + "supervisor configuration bootstrap result did not match the delivered snapshot" + ); + break; + } + } + } + _ => None, + }; + handle_supervisor_message( + state, + sandbox_id, + session_id, + stream_applies_config, + msg, + ).await; + match bootstrap_succeeded { + Some(true) => { + if !mark_supervisor_initialized( + state, + sandbox_id, + session_id, + instance_id, + ) + .await + { + break; + } + bootstrap_complete = true; + } + Some(false) => { + warn!(sandbox_id, session_id, "supervisor configuration bootstrap failed"); + break; + } + None => {} + } } Ok(None) => { info!(sandbox_id = %sandbox_id, session_id = %session_id, "supervisor session: stream closed by supervisor"); @@ -1094,14 +1398,19 @@ async fn run_session_loop( break; } } + () = &mut bootstrap_timeout, if !bootstrap_complete => { + warn!(sandbox_id, session_id, "supervisor configuration bootstrap timed out"); + break; + } } } } -fn handle_supervisor_message( +async fn handle_supervisor_message( state: &Arc, sandbox_id: &str, session_id: &str, + stream_applies_config: bool, msg: SupervisorMessage, ) { match msg.payload { @@ -1140,20 +1449,60 @@ fn handle_supervisor_message( ); } Some(supervisor_message::Payload::ConfigUpdateResult(result)) => { - debug!( - sandbox_id = %sandbox_id, - session_id = %session_id, - component_sequence = result.component_sequence, - "supervisor session: ignoring configuration result while polling remains authoritative" - ); + if let Err(error) = state + .supervisor_sessions + .complete_config_update(sandbox_id, session_id, &result) + { + debug!( + sandbox_id, + session_id, + error = %error, + "ignored unmatched supervisor configuration result" + ); + return; + } + if let Some(result) = result.result.as_ref() + && let Err(error) = record_component_apply_result(state, sandbox_id, result).await + { + state + .supervisor_sessions + .retry_config_update_after_persistence_failure(sandbox_id, session_id, result); + warn!( + sandbox_id, + session_id, + component = result.component, + error = %error, + "failed to persist supervisor configuration result" + ); + } } Some(supervisor_message::Payload::ConfigBootstrapResult(result)) => { - debug!( - sandbox_id = %sandbox_id, - session_id = %session_id, - result_count = result.results.len(), - "supervisor session: ignoring bootstrap result while polling remains authoritative" - ); + if !stream_applies_config { + debug!( + sandbox_id, + session_id, "ignored bootstrap result from polling-compatibility supervisor" + ); + return; + } + if !state + .supervisor_sessions + .is_current_session(sandbox_id, session_id) + { + return; + } + for component in &result.results { + if let Err(error) = + record_component_apply_result(state, sandbox_id, component).await + { + warn!( + sandbox_id, + session_id, + component = component.component, + error = %error, + "failed to persist supervisor bootstrap result" + ); + } + } } _ => { warn!( @@ -1165,6 +1514,162 @@ fn handle_supervisor_message( } } +fn bootstrap_revision_fence( + bootstrap: &ConfigBootstrap, +) -> Vec<(ConfigComponent, ConfigSnapshotRevision)> { + let mut revisions = Vec::with_capacity(2); + if let Some(snapshot) = bootstrap.sandbox_config.as_ref() { + revisions.push(( + ConfigComponent::SandboxConfig, + ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::SandboxConfig( + openshell_core::proto::SandboxConfigRevision { + config_revision: snapshot.config_revision, + policy_version: snapshot.version, + policy_source: snapshot.policy_source, + global_policy_version: snapshot.global_policy_version, + }, + )), + }, + )); + } + if let Some(snapshot) = bootstrap.provider_environment.as_ref() { + revisions.push(( + ConfigComponent::ProviderEnvironment, + ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::ProviderEnvironment( + snapshot.provider_env_revision, + )), + }, + )); + } + revisions +} + +fn validate_bootstrap_result( + results: &[ConfigComponentApplyResult], + expected: &[(ConfigComponent, ConfigSnapshotRevision)], +) -> Result { + if expected.len() != 2 || results.len() != expected.len() { + return Err(Status::invalid_argument( + "bootstrap result must contain every delivered component exactly once", + )); + } + let mut seen = Vec::with_capacity(results.len()); + let mut all_succeeded = true; + for result in results { + let component = ConfigComponent::try_from(result.component).unwrap_or_default(); + if component == ConfigComponent::Unspecified || seen.contains(&component) { + return Err(Status::invalid_argument( + "bootstrap result contains an invalid or duplicate component", + )); + } + let Some((_, revision)) = expected + .iter() + .find(|(expected_component, _)| *expected_component == component) + else { + return Err(Status::invalid_argument( + "bootstrap result contains an unexpected component", + )); + }; + let outcome = validate_component_apply_result(result, revision)?; + seen.push(component); + all_succeeded &= matches!( + outcome, + ConfigApplyOutcome::Applied + | ConfigApplyOutcome::IgnoredDuplicate + | ConfigApplyOutcome::RetainedLocalOverride + | ConfigApplyOutcome::Degraded + ); + } + Ok(all_succeeded) +} + +async fn mark_supervisor_initialized( + state: &Arc, + sandbox_id: &str, + session_id: &str, + instance_id: &str, +) -> bool { + if !state + .supervisor_sessions + .is_current_session(sandbox_id, session_id) + { + return false; + } + if let Err(err) = state + .compute + .supervisor_session_connected(sandbox_id, instance_id) + .await + { + warn!( + sandbox_id, + session_id, + error = %err, + "supervisor session: failed to mark sandbox initialized" + ); + false + } else { + state.telemetry.sandbox_session_connected(sandbox_id); + true + } +} + +async fn record_component_apply_result( + state: &Arc, + sandbox_id: &str, + result: &ConfigComponentApplyResult, +) -> Result<(), Status> { + let component = ConfigComponent::try_from(result.component).unwrap_or_default(); + let outcome = ConfigApplyOutcome::try_from(result.outcome).unwrap_or_default(); + counter!( + "openshell_supervisor_config_apply_results_total", + "component" => component.as_str_name(), + "outcome" => outcome.as_str_name(), + ) + .increment(1); + if component != ConfigComponent::SandboxConfig { + return Ok(()); + } + let Some(config_snapshot_revision::Component::SandboxConfig(revision)) = result + .requested_revision + .as_ref() + .and_then(|revision| revision.component.as_ref()) + else { + return Err(Status::invalid_argument( + "sandbox configuration result is missing its requested revision", + )); + }; + if PolicySource::try_from(revision.policy_source).unwrap_or_default() != PolicySource::Sandbox + || revision.policy_version == 0 + { + return Ok(()); + } + let loaded = matches!( + outcome, + ConfigApplyOutcome::Applied | ConfigApplyOutcome::IgnoredDuplicate + ); + let failed = matches!( + outcome, + ConfigApplyOutcome::FailedRetainedLastKnownGood | ConfigApplyOutcome::FailedClosed + ); + if !loaded && !failed { + return Ok(()); + } + crate::grpc::policy::record_policy_apply_result( + state, + sandbox_id, + revision.policy_version, + loaded, + result + .failure + .as_ref() + .map(|failure| failure.message.as_str()), + "stream", + ) + .await +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -1187,7 +1692,7 @@ mod tests { payload: Some(gateway_message::Payload::SessionAccepted(SessionAccepted { session_id: "session-1".into(), heartbeat_interval_secs: 15, - bootstrap: Some(openshell_core::proto::ConfigBootstrap { + bootstrap: Some(ConfigBootstrap { sandbox_config: Some(SandboxConfigSnapshot::default()), provider_environment: Some(ProviderEnvironmentSnapshot::default()), }), @@ -1215,34 +1720,30 @@ mod tests { let result = SupervisorMessage { payload: Some(supervisor_message::Payload::ConfigUpdateResult( - openshell_core::proto::ConfigUpdateResult { + ConfigUpdateResult { update_id: "update-1".into(), component_sequence: 4, - result: Some(openshell_core::proto::ConfigComponentApplyResult { - component: openshell_core::proto::ConfigComponent::SandboxConfig.into(), - requested_revision: Some(openshell_core::proto::ConfigSnapshotRevision { - component: Some( - openshell_core::proto::config_snapshot_revision::Component::SandboxConfig( - SandboxConfigRevision { - config_revision: 7, - policy_version: 3, - ..Default::default() - }, - ), - ), + result: Some(ConfigComponentApplyResult { + component: ConfigComponent::SandboxConfig.into(), + requested_revision: Some(ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::SandboxConfig( + SandboxConfigRevision { + config_revision: 7, + policy_version: 3, + ..Default::default() + }, + )), }), - applied_revision: Some(openshell_core::proto::ConfigSnapshotRevision { - component: Some( - openshell_core::proto::config_snapshot_revision::Component::SandboxConfig( - SandboxConfigRevision { - config_revision: 7, - policy_version: 3, - ..Default::default() - }, - ), - ), + applied_revision: Some(ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::SandboxConfig( + SandboxConfigRevision { + config_revision: 7, + policy_version: 3, + ..Default::default() + }, + )), }), - outcome: openshell_core::proto::ConfigApplyOutcome::Applied.into(), + outcome: ConfigApplyOutcome::Applied.into(), failure: None, }), }, @@ -1255,9 +1756,59 @@ mod tests { #[test] fn supervisor_protocol_revision_accepts_current_and_legacy_peers() { assert!(validate_protocol_revision("sb-1", SUPERVISOR_PROTOCOL_REVISION).is_ok()); + assert!(validate_protocol_revision("sb-1", PREVIOUS_SUPERVISOR_PROTOCOL_REVISION).is_ok()); assert!(validate_protocol_revision("sb-1", LEGACY_SUPERVISOR_PROTOCOL_REVISION).is_ok()); } + #[test] + fn bootstrap_requires_every_delivered_component_to_succeed() { + let sandbox_revision = ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::SandboxConfig( + SandboxConfigRevision { + config_revision: 7, + ..Default::default() + }, + )), + }; + let provider_revision = ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::ProviderEnvironment(9)), + }; + let expected = vec![ + (ConfigComponent::SandboxConfig, sandbox_revision), + (ConfigComponent::ProviderEnvironment, provider_revision), + ]; + let result = + |component: ConfigComponent, + revision: ConfigSnapshotRevision, + outcome: ConfigApplyOutcome| ConfigComponentApplyResult { + component: component.into(), + requested_revision: Some(revision), + applied_revision: Some(revision), + outcome: outcome.into(), + ..Default::default() + }; + let mut results = vec![ + result( + ConfigComponent::SandboxConfig, + sandbox_revision, + ConfigApplyOutcome::Applied, + ), + result( + ConfigComponent::ProviderEnvironment, + provider_revision, + ConfigApplyOutcome::Degraded, + ), + ]; + assert!(validate_bootstrap_result(&results, &expected).unwrap()); + + results[1].outcome = ConfigApplyOutcome::FailedClosed.into(); + results[1].applied_revision = None; + assert!(!validate_bootstrap_result(&results, &expected).unwrap()); + assert!(validate_bootstrap_result(&results[..1], &expected).is_err()); + results[1].requested_revision = Some(ConfigSnapshotRevision::default()); + assert!(validate_bootstrap_result(&results, &expected).is_err()); + } + async fn state_with_sandbox(sandbox_id: &str) -> Arc { let state = crate::grpc::test_support::test_server_state().await; state @@ -1294,7 +1845,10 @@ mod tests { else { panic!("expected SessionAccepted"); }; - assert_eq!(accepted.protocol_revision, SUPERVISOR_PROTOCOL_REVISION); + assert_eq!( + accepted.protocol_revision, + LEGACY_SUPERVISOR_PROTOCOL_REVISION + ); assert!( state .supervisor_sessions @@ -1302,6 +1856,33 @@ mod tests { ); } + #[tokio::test] + async fn stage_one_supervisor_uses_polling_compatibility() { + let state = state_with_sandbox("sb-stage-one").await; + let mut harness = crate::grpc::test_support::connect_supervisor_stream( + &state, + "sb-stage-one", + PREVIOUS_SUPERVISOR_PROTOCOL_REVISION, + ) + .await + .expect("Stage 1 supervisor must connect"); + + let Some(gateway_message::Payload::SessionAccepted(accepted)) = + first_gateway_message(&mut harness).await.payload + else { + panic!("expected SessionAccepted"); + }; + assert_eq!( + accepted.protocol_revision, + PREVIOUS_SUPERVISOR_PROTOCOL_REVISION + ); + assert!( + state + .supervisor_sessions + .is_current_session("sb-stage-one", &accepted.session_id) + ); + } + #[tokio::test] async fn unknown_supervisor_protocol_revision_is_rejected() { let state = state_with_sandbox("sb-future").await; @@ -1354,7 +1935,10 @@ mod tests { router .deliver( "sb-1", - SupervisorConfigMessage::SandboxConfig(Box::default()), + SupervisorConfigMessage::SandboxConfig(Box::new(SandboxConfigSnapshot { + config_revision: 2, + ..Default::default() + })), ) .await, DeliveryDisposition::Enqueued @@ -1366,7 +1950,7 @@ mod tests { SupervisorConfigMessage::SandboxConfig(Box::default()), ) .await, - DeliveryDisposition::Enqueued + DeliveryDisposition::Coalesced ); assert_eq!( router @@ -1381,15 +1965,45 @@ mod tests { ); let first = rx.recv().await.expect("first config update"); - let second = rx.recv().await.expect("second config update"); let third = rx.recv().await.expect("provider config update"); - let sequence = |message: GatewayMessage| match message.payload { - Some(gateway_message::Payload::ConfigUpdate(update)) => update.component_sequence, + let update = |message: GatewayMessage| match message.payload { + Some(gateway_message::Payload::ConfigUpdate(update)) => update, other => panic!("expected config update, got {other:?}"), }; - assert_eq!(sequence(first), 1); - assert_eq!(sequence(second), 2); - assert_eq!(sequence(third), 1); + let first = update(first); + assert_eq!(first.component_sequence, 1); + let revision = registry + .sessions + .lock() + .unwrap() + .get("sb-1") + .unwrap() + .config_sequences + .sandbox_config + .in_flight + .as_ref() + .unwrap() + .revision; + registry + .complete_config_update( + "sb-1", + "session-1", + &ConfigUpdateResult { + update_id: first.update_id, + component_sequence: first.component_sequence, + result: Some(ConfigComponentApplyResult { + component: ConfigComponent::SandboxConfig.into(), + requested_revision: Some(revision), + applied_revision: Some(revision), + outcome: ConfigApplyOutcome::Applied.into(), + failure: None, + }), + }, + ) + .unwrap(); + let second = update(rx.recv().await.expect("coalesced config update")); + assert_eq!(second.component_sequence, 2); + assert_eq!(update(third).component_sequence, 1); } #[tokio::test] diff --git a/crates/openshell-supervisor-process/src/run.rs b/crates/openshell-supervisor-process/src/run.rs index ef40fbd760..f44460a44e 100644 --- a/crates/openshell-supervisor-process/src/run.rs +++ b/crates/openshell-supervisor-process/src/run.rs @@ -85,6 +85,9 @@ pub async fn run_process( provider_env: std::collections::HashMap, ca_file_paths: Option<(std::path::PathBuf, std::path::PathBuf)>, agent_proposals: AgentProposals, + config_apply_tx: Option< + tokio::sync::mpsc::Sender, + >, #[cfg(target_os = "linux")] netns: Option<&NetworkNamespace>, #[cfg(target_os = "linux")] bypass_denial_tx: Option< tokio::sync::mpsc::UnboundedSender, @@ -378,6 +381,7 @@ pub async fn run_process( None, Arc::clone(&supervisor_terminating), main_instance_id.clone(), + config_apply_tx, ); info!("supervisor session task spawned"); Some(task) diff --git a/crates/openshell-supervisor-process/src/supervisor_session.rs b/crates/openshell-supervisor-process/src/supervisor_session.rs index 959d7dd0d6..62003abd1d 100644 --- a/crates/openshell-supervisor-process/src/supervisor_session.rs +++ b/crates/openshell-supervisor-process/src/supervisor_session.rs @@ -13,17 +13,23 @@ use std::net::IpAddr; #[cfg(target_os = "linux")] use std::os::fd::RawFd; -use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; use std::time::Duration; use openshell_core::proto::open_shell_client::OpenShellClient; use openshell_core::proto::{ - FinalizeMainProcessExitRequest, GatewayMessage, RelayFrame, RelayInit, RelayOpen, - RelayOpenResult, ReportMainProcessExitRequest, SupervisorHeartbeat, SupervisorHello, - SupervisorMessage, TcpRelayTarget, gateway_message, relay_open, supervisor_message, + ConfigApplyFailure, ConfigApplyOutcome, ConfigBootstrap, ConfigBootstrapResult, + ConfigComponent, ConfigComponentApplyResult, ConfigSnapshotRevision, ConfigUpdate, + ConfigUpdateResult, FinalizeMainProcessExitRequest, GatewayMessage, RelayFrame, RelayInit, + RelayOpen, RelayOpenResult, ReportMainProcessExitRequest, SupervisorHeartbeat, SupervisorHello, + SupervisorMessage, TcpRelayTarget, config_snapshot_revision, config_update, gateway_message, + relay_open, supervisor_message, +}; +use openshell_core::proto::{ + LEGACY_SUPERVISOR_PROTOCOL_REVISION, PREVIOUS_SUPERVISOR_PROTOCOL_REVISION, + SUPERVISOR_PROTOCOL_REVISION, }; -use openshell_core::proto::{LEGACY_SUPERVISOR_PROTOCOL_REVISION, SUPERVISOR_PROTOCOL_REVISION}; use openshell_ocsf::{ ActivityId, ConnectionInfo, Endpoint, EventContext, NetworkActivityBuilder, OcsfEvent, SeverityId, StatusId, ocsf_emit, @@ -39,6 +45,75 @@ use openshell_core::transport_errors::is_expected_transport_close_status; const INITIAL_BACKOFF: Duration = Duration::from_secs(1); const MAX_BACKOFF: Duration = Duration::from_secs(30); +const CONFIG_APPLY_TIMEOUT: Duration = Duration::from_mins(1); + +/// A stream-delivered desired-state payload awaiting application by the +/// sandbox runtime. The response travels back over `ConnectSupervisor`. +pub enum ConfigApplyRequest { + Bootstrap { + bootstrap: ConfigBootstrap, + response: tokio::sync::oneshot::Sender, + }, + Update { + update: ConfigUpdate, + response: tokio::sync::oneshot::Sender, + }, +} + +#[derive(Default)] +struct ConfigSequenceWatermarks { + sandbox_config: u64, + provider_environment: u64, +} + +fn failed_component_result( + component: ConfigComponent, + requested_revision: Option, + outcome: ConfigApplyOutcome, + code: &str, + message: &str, +) -> ConfigComponentApplyResult { + ConfigComponentApplyResult { + component: component.into(), + requested_revision, + applied_revision: None, + outcome: outcome.into(), + failure: Some(ConfigApplyFailure { + code: code.to_string(), + message: message.chars().take(1024).collect(), + retryable: false, + }), + } +} + +fn update_component_and_revision( + update: &ConfigUpdate, +) -> (ConfigComponent, Option) { + match update.component.as_ref() { + Some(config_update::Component::SandboxConfig(snapshot)) => ( + ConfigComponent::SandboxConfig, + Some(ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::SandboxConfig( + openshell_core::proto::SandboxConfigRevision { + config_revision: snapshot.config_revision, + policy_version: snapshot.version, + policy_source: snapshot.policy_source, + global_policy_version: snapshot.global_policy_version, + }, + )), + }), + ), + Some(config_update::Component::ProviderEnvironment(snapshot)) => ( + ConfigComponent::ProviderEnvironment, + Some(ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::ProviderEnvironment( + snapshot.provider_env_revision, + )), + }), + ), + None => (ConfigComponent::Unspecified, None), + } +} /// Parse a gRPC endpoint URI into an OCSF `Endpoint` (host + port). Falls back /// to treating the whole string as a domain if parsing fails. @@ -271,6 +346,7 @@ fn map_session_stream_message( /// /// The task runs for the lifetime of the sandbox process, reconnecting with /// exponential backoff on failures. +#[allow(clippy::too_many_arguments)] pub fn spawn( endpoint: String, sandbox_id: String, @@ -279,6 +355,7 @@ pub fn spawn( expected_ssh_peer_pid: Option, terminating: Arc, instance_id: String, + config_apply_tx: Option>, ) -> tokio::task::JoinHandle<()> { let config = SessionConfig { endpoint, @@ -288,6 +365,7 @@ pub fn spawn( expected_ssh_peer_pid, terminating, instance_id, + config_apply_tx, }; tokio::spawn(run_session_loop(config)) } @@ -300,6 +378,7 @@ struct SessionConfig { expected_ssh_peer_pid: Option, terminating: Arc, instance_id: String, + config_apply_tx: Option>, } async fn run_session_loop(config: SessionConfig) { @@ -392,13 +471,15 @@ async fn run_single_session( ); ocsf_emit!(event); - if accepted.bootstrap.is_some() { - debug!( - sandbox_id = %config.sandbox_id, - session_id = %accepted.session_id, - "supervisor session: ignoring configuration bootstrap while polling remains active" - ); + if let Some(bootstrap) = accepted.bootstrap { + let result = apply_bootstrap(config, bootstrap).await; + tx.send(SupervisorMessage { + payload: Some(supervisor_message::Payload::ConfigBootstrapResult(result)), + }) + .await + .map_err(|_| "failed to queue configuration bootstrap result")?; } + let config_sequences = Arc::new(Mutex::new(ConfigSequenceWatermarks::default())); // Main loop: receive gateway messages + send heartbeats. let mut heartbeat_interval = @@ -424,6 +505,8 @@ async fn run_single_session( channel: &channel, tx: &tx, terminating: &config.terminating, + config_apply_tx: config.config_apply_tx.as_ref(), + config_sequences: &config_sequences, }; handle_gateway_message( &msg, @@ -449,6 +532,12 @@ fn validate_gateway_protocol_revision( ) -> Result<(), Box> { match gateway_revision { SUPERVISOR_PROTOCOL_REVISION => Ok(()), + PREVIOUS_SUPERVISOR_PROTOCOL_REVISION => { + warn!( + "supervisor session: gateway uses Stage 1 stream semantics; polling remains active" + ); + Ok(()) + } LEGACY_SUPERVISOR_PROTOCOL_REVISION => { warn!( "supervisor session: gateway predates the protocol handshake; upgrade the gateway before pinning newer supervisor images" @@ -462,6 +551,79 @@ fn validate_gateway_protocol_revision( } } +async fn apply_bootstrap( + config: &SessionConfig, + bootstrap: ConfigBootstrap, +) -> ConfigBootstrapResult { + let Some(apply_tx) = config.config_apply_tx.as_ref() else { + return ConfigBootstrapResult { + results: bootstrap_components(&bootstrap) + .into_iter() + .map(|(component, revision)| { + failed_component_result( + component, + revision, + ConfigApplyOutcome::Unsupported, + "apply_unavailable", + "configuration apply service is unavailable", + ) + }) + .collect(), + }; + }; + let (response, receiver) = tokio::sync::oneshot::channel(); + if apply_tx + .send(ConfigApplyRequest::Bootstrap { + bootstrap, + response, + }) + .await + .is_err() + { + return ConfigBootstrapResult { + results: Vec::new(), + }; + } + match tokio::time::timeout(CONFIG_APPLY_TIMEOUT, receiver).await { + Ok(Ok(result)) => result, + _ => ConfigBootstrapResult { + results: Vec::new(), + }, + } +} + +fn bootstrap_components( + bootstrap: &ConfigBootstrap, +) -> Vec<(ConfigComponent, Option)> { + let mut components = Vec::with_capacity(2); + if let Some(snapshot) = bootstrap.sandbox_config.as_ref() { + components.push(( + ConfigComponent::SandboxConfig, + Some(ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::SandboxConfig( + openshell_core::proto::SandboxConfigRevision { + config_revision: snapshot.config_revision, + policy_version: snapshot.version, + policy_source: snapshot.policy_source, + global_policy_version: snapshot.global_policy_version, + }, + )), + }), + )); + } + if let Some(snapshot) = bootstrap.provider_environment.as_ref() { + components.push(( + ConfigComponent::ProviderEnvironment, + Some(ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::ProviderEnvironment( + snapshot.provider_env_revision, + )), + }), + )); + } + components +} + /// Report the canonical process result and wait for durable handling. pub async fn report_main_process_exit( endpoint: &str, @@ -510,6 +672,8 @@ struct GatewayMessageContext<'a> { channel: &'a grpc_client::AuthedChannel, tx: &'a mpsc::Sender, terminating: &'a Arc, + config_apply_tx: Option<&'a mpsc::Sender>, + config_sequences: &'a Arc>, } fn handle_gateway_message(msg: &GatewayMessage, context: &GatewayMessageContext<'_>) { @@ -518,13 +682,97 @@ fn handle_gateway_message(msg: &GatewayMessage, context: &GatewayMessageContext< // Gateway heartbeat — nothing to do. } Some(gateway_message::Payload::ConfigUpdate(update)) => { - // Stage 1 accepts pushed configuration but leaves polling as the - // only path that changes runtime state. - debug!( - sandbox_id = %context.sandbox_id, - component_sequence = update.component_sequence, - "supervisor session: ignoring configuration update while polling remains active" - ); + let update = update.clone(); + let tx = context.tx.clone(); + let apply_tx = context.config_apply_tx.cloned(); + let sandbox_id = context.sandbox_id.to_string(); + let (component, _) = update_component_and_revision(&update); + let invalid_update = update.update_id.is_empty() + || update.component_sequence == 0 + || update.component.is_none(); + let stale_sequence = if invalid_update { + false + } else { + let mut watermarks = context.config_sequences.lock().unwrap(); + let watermark = match component { + ConfigComponent::ProviderEnvironment => &mut watermarks.provider_environment, + ConfigComponent::SandboxConfig | ConfigComponent::Unspecified => { + &mut watermarks.sandbox_config + } + }; + if update.component_sequence <= *watermark { + true + } else { + *watermark = update.component_sequence; + false + } + }; + tokio::spawn(async move { + let (component, revision) = update_component_and_revision(&update); + let fallback = |outcome, code: &str, message: &str| ConfigUpdateResult { + update_id: update.update_id.clone(), + component_sequence: update.component_sequence, + result: Some(failed_component_result( + component, revision, outcome, code, message, + )), + }; + let result = if invalid_update { + fallback( + ConfigApplyOutcome::Unsupported, + "invalid_update", + "configuration update identity, sequence, and component are required", + ) + } else if stale_sequence { + fallback( + ConfigApplyOutcome::IgnoredStale, + "stale_sequence", + "configuration update sequence is stale", + ) + } else if let Some(apply_tx) = apply_tx { + let (response, receiver) = tokio::sync::oneshot::channel(); + if apply_tx + .send(ConfigApplyRequest::Update { + update: update.clone(), + response, + }) + .await + .is_err() + { + fallback( + ConfigApplyOutcome::Unsupported, + "apply_unavailable", + "configuration apply service is unavailable", + ) + } else { + match tokio::time::timeout(CONFIG_APPLY_TIMEOUT, receiver).await { + Ok(Ok(result)) => result, + _ => fallback( + ConfigApplyOutcome::Unsupported, + "apply_timeout", + "configuration application timed out", + ), + } + } + } else { + fallback( + ConfigApplyOutcome::Unsupported, + "apply_unavailable", + "configuration apply service is unavailable", + ) + }; + if tx + .send(SupervisorMessage { + payload: Some(supervisor_message::Payload::ConfigUpdateResult(result)), + }) + .await + .is_err() + { + debug!( + sandbox_id, + "configuration result dropped after session close" + ); + } + }); } Some(gateway_message::Payload::RelayOpen(open)) => { let channel_id = open.channel_id.clone(); @@ -870,6 +1118,7 @@ mod target_tests { #[test] fn gateway_protocol_revision_accepts_current_and_legacy_peers() { assert!(validate_gateway_protocol_revision(SUPERVISOR_PROTOCOL_REVISION).is_ok()); + assert!(validate_gateway_protocol_revision(PREVIOUS_SUPERVISOR_PROTOCOL_REVISION).is_ok()); assert!(validate_gateway_protocol_revision(LEGACY_SUPERVISOR_PROTOCOL_REVISION).is_ok()); } diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index bbd8ee72aa..96e60b9c59 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -22,7 +22,7 @@ Gateway CLI flag > gateway OPENSHELL_* env var > TOML file > built-in defa ## Supervisor connections and policy recovery -During supervisor reconnects, the gateway waits at most one second for the optional configuration bootstrap. If a credential backend stalls, the gateway accepts the session without that bootstrap so SSH, exec, and service connections can resume. Supervisor polling continues to supply configuration. +Current supervisors require a complete configuration bootstrap before the gateway accepts the session as initialized. The gateway gives bootstrap construction the same bounded 45-second window as other snapshot builds and rejects the connection when construction fails or times out. Immediately previous supervisors retain the Stage 1 compatibility behavior: the gateway waits at most one second for an optional bootstrap, accepts the session without it when necessary, and relies on supervisor polling for configuration. On startup, the gateway repairs missing legacy policy history and skips invalid stored policies. A valid global policy can still override an invalid local policy; otherwise, that sandbox's configuration reads continue to report the validation failure. @@ -112,10 +112,10 @@ disable_tls = false # Shared driver defaults. These inherit into [openshell.drivers.] tables # when the driver-specific table does not override them. default_image = "ghcr.io/nvidia/openshell-community/sandboxes/base:latest" -# Defaults to the gateway version. Custom builds must match the gateway's -# internal supervisor protocol revision; mismatched peers reject the session. -# Supervisors from releases before the handshake existed still connect, with -# a gateway warning, until the next release. +# Defaults to the gateway version. The gateway accepts its current internal +# supervisor protocol revision, the immediately previous revision through +# polling compatibility, and pre-handshake supervisors for one release. Other +# revisions reject the session. Compatibility sessions emit gateway warnings. # supervisor_image = "ghcr.io/nvidia/openshell/supervisor:" client_tls_secret_name = "openshell-client-tls" service_account_name = "openshell-sandbox" diff --git a/proto/openshell.proto b/proto/openshell.proto index a63d33ac4f..17f6f082ef 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -2530,7 +2530,8 @@ message SessionAccepted { // Complete gateway-owned configuration. During the staged rollout this may // be omitted only when the gateway cannot build the projection. ConfigBootstrap bootstrap = 3; - // Exact internal stream protocol revision implemented by this gateway. + // Stream semantics negotiated from the supervisor's offered revision. The + // gateway echoes a supported compatibility revision during rollout. uint32 protocol_revision = 4; } diff --git a/sdk/go/proto/openshellv1/openshell.pb.go b/sdk/go/proto/openshellv1/openshell.pb.go index cce47f10ec..fc0b2ed1d9 100644 --- a/sdk/go/proto/openshellv1/openshell.pb.go +++ b/sdk/go/proto/openshellv1/openshell.pb.go @@ -11049,7 +11049,8 @@ type SessionAccepted struct { // Complete gateway-owned configuration. During the staged rollout this may // be omitted only when the gateway cannot build the projection. Bootstrap *ConfigBootstrap `protobuf:"bytes,3,opt,name=bootstrap,proto3" json:"bootstrap,omitempty"` - // Exact internal stream protocol revision implemented by this gateway. + // Stream semantics negotiated from the supervisor's offered revision. The + // gateway echoes a supported compatibility revision during rollout. ProtocolRevision uint32 `protobuf:"varint,4,opt,name=protocol_revision,json=protocolRevision,proto3" json:"protocol_revision,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index ec6f3790f0..32580f2f12 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -19,7 +19,7 @@ The target deployment flow is: 4. The CLI registers a reachable gateway endpoint with `openshell gateway add`. 5. The gateway creates sandboxes through the selected compute driver. -If supervisor sessions fail with a protocol revision mismatch, check that custom supervisor images match the gateway release. Gateway and supervisor require the same internal protocol revision; authentication success does not make mismatched versions compatible. Supervisors that predate the handshake still connect for one release. The gateway logs a warning for each such session and counts them in the `openshell_supervisor_protocol_legacy_sessions_total` metric, so recreate those sandboxes before the next gateway upgrade. See the published [gateway configuration reference](https://docs.nvidia.com/openshell/latest/reference/gateway-config.md). +If supervisor sessions fail with a protocol revision mismatch, check that custom supervisor images match the gateway release. The gateway accepts its current internal protocol revision, the immediately previous revision through polling compatibility, and supervisors that predate the handshake for one release. Authentication success does not make other revisions compatible. The gateway logs compatibility sessions and counts them in `openshell_supervisor_protocol_previous_sessions_total` or `openshell_supervisor_protocol_legacy_sessions_total`; recreate those sandboxes before compatibility is removed. See the published [gateway configuration reference](https://docs.nvidia.com/openshell/latest/reference/gateway-config.md). The `openshell-gateway` composition crate explicitly installs its compiled Docker, Podman, Kubernetes, and VM registrations at startup; `openshell-server` From 485da701ca566c20a31fd76c94f0e7a7865a649f Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Thu, 10 Sep 2026 18:14:11 -0700 Subject: [PATCH 2/7] fix(supervisor): complete stream-first configuration startup Signed-off-by: Piotr Mlocek --- architecture/gateway.md | 4 +- architecture/sandbox.md | 81 ++- crates/openshell-sandbox/src/lib.rs | 662 +++++++++++++++--- .../openshell-sandbox/src/sidecar_control.rs | 15 + crates/openshell-server/proto/storage.proto | 15 + crates/openshell-server/src/compute/mod.rs | 34 +- .../openshell-server/src/persistence/mod.rs | 2 + crates/openshell-server/src/storage_proto.rs | 58 +- .../src/supervisor_session.rs | 118 ++++ .../openshell-supervisor-process/src/run.rs | 34 +- .../src/supervisor_session.rs | 148 +++- docs/reference/gateway-config.mdx | 2 +- skills/debug-openshell-cluster/SKILL.md | 14 +- 13 files changed, 1020 insertions(+), 167 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index ccbcd24d74..4615b72cf9 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -668,7 +668,9 @@ process-local supervisor registry. A future HA implementation can resolve the gateway that owns a session and forward the same typed message without changing mutation handlers. -Current supervisors apply stream snapshots directly. Previous-revision +Current supervisors establish the stream before gateway-owned runtime +initialization, apply bootstrap and live snapshots directly, and persist only +compact component observations from their results. Previous-revision supervisors retain polling as a rollout fallback, and owner reconciliation repairs missed or failed delivery from current database state. Snapshot build, fanout, or enqueue failure cannot fail a mutation that already committed. diff --git a/architecture/sandbox.md b/architecture/sandbox.md index 8dfb9008ab..2e96520ccf 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -23,17 +23,21 @@ only when the set is already empty; any other outcome fails the spawn. ## Startup Flow -1. The compute runtime starts the workload with sandbox identity, callback +1. The compute runtime starts the supervisor with sandbox identity, callback endpoint, TLS or secret material, image metadata, and initial command. -2. The supervisor loads policy and runtime settings from local files or the - gateway, depending on mode. -3. It prepares filesystem access, process restrictions, network namespace +2. In gateway-backed mode, the supervisor opens `ConnectSupervisor` and + receives the complete desired-state bootstrap. Standalone mode reads its + explicit local files without opening a gateway session. +3. It installs policy, settings, middleware, and provider state from that + authoritative source. +4. It prepares filesystem access, process restrictions, network namespace routing, trust stores, and provider credential resolution. -4. It launches the persisted canonical main-process argv and retains its PTY +5. It starts the policy proxy and local SSH server, then launches the persisted + canonical main-process argv and retains its PTY or pipes in the main-session multiplexer. -5. It starts the policy proxy and local SSH server. -6. It opens a supervisor session back to the gateway for connect, exec, file - sync, config polling, and log push. +6. After runtime endpoints are ready, it reports bootstrap application and + keeps the session open for desired-state delivery, connect, exec, file sync, + and log push. ## Isolation Layers @@ -201,10 +205,9 @@ the registry. Public custom-CA PEM travels with the stable registration. The slots live in a supervisor-owned `ExtensionCredentialStore` shared by every gateway connection the supervisor opens, so the registry's clients and the -polling loop that rotates them observe the same credentials. Configuration -polling runs far more frequently than credentials expire, so the loop rotates -only when a credential is missing or has passed four fifths of its lifetime, -and bounds its sleep by the soonest rotation deadline. +configuration loop that rotates them observe the same credentials. The loop +rotates only when a credential is missing or has passed four fifths of its +lifetime, and bounds its sleep by the soonest rotation deadline. Middleware cannot observe injected credentials, introduce credential placeholders, or mutate supervisor-owned credential, routing, or framing @@ -487,6 +490,10 @@ Revision 2 supervisors require a complete bootstrap. The gateway uses the same bounded 45-second construction window as other snapshot builds and rejects the connection when construction fails. Revision 1 compatibility sessions retain the optional one-second bootstrap budget and use polling when it expires. +The revision 2 supervisor opens the stream and consumes the bootstrap before it +constructs gateway-owned policy, provider state, networking, or the workload. +It reports bootstrap results after those components, the workload, and relay +endpoints are ready. These payloads describe the latest effective state rather than the mutation that produced it. The gateway assigns ordering sequences within each session and component, while each snapshot retains its own content @@ -496,7 +503,8 @@ Bootstrap components are independent read projections, not one atomic database snapshot. The sandbox configuration carries the provider-environment revision it was built against. The gateway retries bootstrap construction when that revision does not match the provider snapshot. Later component updates and -polling repair changes committed while the other projections were being built. +owner reconciliation repair changes committed while the other projections +were being built. Configuration delivery goes through a gateway-owned routing boundary rather than exposing local supervisor channels to mutation handlers. The current @@ -506,12 +514,15 @@ without changing publishers. Provider payloads can contain credentials, so the gateway does not persist or render complete stream messages in logs. -The supervisor applies stream-delivered configuration through the same runtime -primitives used by the compatibility poller. It reports the requested and +The supervisor applies bootstrap and live stream snapshots through shared +component runtime primitives. It reports the requested and active revisions plus a component-specific outcome on `ConnectSupervisor`. Sandbox-scoped policy results update only the matching policy-history row, so a late result cannot mark a newer revision loaded. Explicit local policy remains -authoritative and produces a retained-local-override result. +authoritative and produces a retained-local-override result. Other component +results persist only compact observed state: requested and active revisions, +outcome, effective source, observation time, and a bounded sanitized error. +Delivered snapshots, including provider credentials, are never persisted. The gateway keeps one update in flight per session and component. It replaces the pending snapshot when newer desired state arrives, validates the update ID, @@ -522,10 +533,11 @@ revisions suppress unchanged delivery, while failed or timed-out delivery is retried from current database state. Reconnect discards session delivery state and starts with a fresh bootstrap. -Polling remains available during the mixed-version rollout. The gateway -serializes construction per sandbox and component, and coalesces repeated -mutations into the latest full snapshot. An enqueue result means only that the -local stream queue accepted the message. A bounded scope fanout scheduler +Revision 2 does not poll configuration fetch APIs. Polling remains available +only to revision 1 and revision 0 supervisors during the mixed-version rollout. +The gateway serializes construction per sandbox and component, and coalesces +repeated mutations into the latest full snapshot. An enqueue result means only +that the local stream queue accepted the message. A bounded scope fanout scheduler coalesces repeated workspace and global changes, and semaphores sized from the database pool bound delivery workers and snapshot builds. Fanout waits for worker capacity before admitting each recipient, so a @@ -546,12 +558,11 @@ If policy construction fails, it reports the captured revision as `FAILED` with the original construction error. It never infers revision identity by comparing policy structure. -This holds even when the initial policy is enriched with baseline paths during -startup: the enriched revision the supervisor synced back to the gateway is the -revision it acknowledges, so a successfully constructed initial policy never -remains `Pending`. If the first poll returns a different revision, the supervisor -processes it through the normal reload path instead of treating it as already -loaded. +Image-specific policy discovery and baseline enrichment can require one initial +gateway synchronization. The supervisor commits that repair before runtime +initialization, discards the mutation response, and reconnects so it installs +only the fresh authoritative stream bootstrap. Compatibility supervisors retain +the earlier enrichment and first-poll reconciliation path. A newer sandbox-scoped revision can carry the same non-empty effective policy hash as the currently loaded revision, for example when provenance changes @@ -562,23 +573,23 @@ reconciliation succeeds. Global policies, local overrides, equal or older versions, and different hashes do not use this shortcut. Success telemetry is emitted only after the gateway accepts the resulting loaded-status report. -Policy status delivery uses a FIFO background worker. Retryable delivery -failures retain the ordered update and retry with capped exponential backoff; -terminal errors are logged and discarded. The outbox is nonblocking and does -not discard updates because of a fixed queue capacity, so status endpoint -outages cannot block policy polling, enforcement, settings, or provider -refreshes and cannot permanently lose the initial acknowledgement. +Revision 2 policy status is recorded from the correlated stream result. The +retained reporting RPC uses the same domain helper for compatibility +supervisors. Retryable legacy status delivery uses a FIFO background worker so +status endpoint outages do not block enforcement. Only sandbox-scoped revisions (`PolicySource::Sandbox`, version greater than zero) are acknowledged. Global policies and local-file development policies do not use the sandbox revision API and produce no acknowledgement. When explicit local Rego and data files are configured, the supervisor continues polling the -gateway for settings and provider refreshes but never replaces the local OPA -engine with a gateway policy revision. +gateway for settings and provider refreshes only on the compatibility path; a +revision 2 supervisor receives those components on the stream and never +replaces the local OPA engine with a gateway policy revision. ## Failure Behavior -- If gateway config polling fails, the sandbox keeps its last-known-good policy. +- If a compatibility configuration poll fails, the sandbox keeps its + last-known-good policy. - If a live policy or middleware-registry update is invalid, the supervisor rejects the combined update and keeps the current runtime pair. - If an operator-run middleware call fails, the selected config's `on_error` diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index fdc52d36b2..ca1209a00f 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -178,6 +178,138 @@ pub async fn run_sandbox( None }; + let main_process_instance_id = sidecar_bootstrap + .as_ref() + .map(|bootstrap| bootstrap.main_process_instance_id.clone()) + .filter(|instance_id| !instance_id.is_empty()) + .unwrap_or_else(|| uuid::Uuid::new_v4().to_string()); + + // Revision-2 gateway-backed startup receives desired state from the + // persistent supervisor stream before constructing policy, networking, or + // the workload. Process-only sidecars receive the same state through the + // authenticated local sidecar bootstrap instead. + let mut prepared_supervisor_session = if process_uses_sidecar_control { + None + } else if let (Some(endpoint), Some(id)) = (&openshell_endpoint, &sandbox_id) { + Some( + openshell_supervisor_process::supervisor_session::prepare( + endpoint.clone(), + id.clone(), + main_process_instance_id.clone(), + ) + .await + .map_err(|error| { + miette::miette!("failed to establish supervisor bootstrap session: {error}") + })?, + ) + } else { + None + }; + let mut stream_bootstrap = prepared_supervisor_session.as_mut().and_then( + openshell_supervisor_process::supervisor_session::PreparedSupervisorSession::take_bootstrap, + ); + + // A sandbox created without an explicit policy historically discovers the + // image's baked-in policy on first boot. The runtime also enriches explicit + // policies with image-specific baseline paths before installing Landlock. + // Commit either startup repair before initialization, then reopen + // ConnectSupervisor: runtime state still comes only from the fresh + // authoritative bootstrap, never from the mutation response. + let uses_stream_configuration = prepared_supervisor_session.as_ref().is_some_and( + openshell_supervisor_process::supervisor_session::PreparedSupervisorSession::uses_stream_configuration, + ); + let initial_policy_repair = + if uses_stream_configuration && policy_rules.is_none() && policy_data.is_none() { + stream_bootstrap + .as_ref() + .and_then(|bootstrap| bootstrap.sandbox_config.as_ref()) + .and_then(|snapshot| { + snapshot.policy.clone().map_or_else( + || { + let mut discovered = discover_policy_from_disk_or_default(); + enrich_proto_baseline_paths(&mut discovered); + strip_proto_provider_policy_entries(&mut discovered); + Some(discovered) + }, + |mut policy| { + let enriched = enrich_proto_baseline_paths(&mut policy); + proto_sync_payload_for_enriched_policy(&policy, enriched) + }, + ) + }) + } else { + None + }; + if let Some(initial_policy_repair) = initial_policy_repair { + let endpoint = openshell_endpoint.as_deref().ok_or_else(|| { + miette::miette!("gateway-backed policy discovery requires an OpenShell endpoint") + })?; + let id = sandbox_id.as_deref().ok_or_else(|| { + miette::miette!("gateway-backed policy discovery requires a sandbox ID") + })?; + let sandbox_name = sandbox.as_deref().ok_or_else(|| { + miette::miette!("gateway-backed policy discovery requires a sandbox name") + })?; + let workspace = stream_bootstrap + .as_ref() + .and_then(|bootstrap| bootstrap.sandbox_config.as_ref()) + .map(|snapshot| snapshot.workspace.clone()) + .ok_or_else(|| { + miette::miette!("supervisor bootstrap omitted required sandbox configuration") + })?; + grpc_retry("Initial policy bootstrap repair", || { + let initial_policy_repair = initial_policy_repair.clone(); + let workspace = workspace.clone(); + async move { + openshell_core::grpc_client::sync_policy_and_fetch_snapshot( + endpoint, + id, + sandbox_name, + &initial_policy_repair, + &workspace, + ) + .await + .map(|_| ()) + } + }) + .await?; + + prepared_supervisor_session = Some( + openshell_supervisor_process::supervisor_session::prepare( + endpoint.to_string(), + id.to_string(), + main_process_instance_id.clone(), + ) + .await + .map_err(|error| { + miette::miette!( + "failed to reestablish supervisor session after policy bootstrap repair: {error}" + ) + })?, + ); + stream_bootstrap = prepared_supervisor_session.as_mut().and_then( + openshell_supervisor_process::supervisor_session::PreparedSupervisorSession::take_bootstrap, + ); + } + if stream_bootstrap + .as_ref() + .is_some_and(|bootstrap| bootstrap.sandbox_config.is_none()) + { + return Err(miette::miette!( + "supervisor bootstrap omitted required sandbox configuration" + )); + } + if uses_stream_configuration + && stream_bootstrap + .as_ref() + .and_then(|bootstrap| bootstrap.sandbox_config.as_ref()) + .is_some_and(|snapshot| snapshot.policy.is_none()) + { + return Err(miette::miette!( + "supervisor bootstrap omitted required sandbox policy" + )); + } + // Extension credentials are owned by this supervisor and shared by every // gateway connection it opens, so the middleware registry's bearer slots // and the policy poll loop that rotates them stay the same objects. @@ -192,8 +324,8 @@ pub async fn run_sandbox( retained_proto, middleware_registry_status, loaded_policy_origin, - initial_agent_proposals_enabled, - initial_extension_authentication_enabled, + mut initial_agent_proposals_enabled, + mut initial_extension_authentication_enabled, ) = if let Some(bootstrap) = sidecar_bootstrap.as_ref() { let (policy, opa_engine, retained_proto, loaded_policy_origin) = load_policy_from_sidecar_bootstrap(bootstrap)?; @@ -214,9 +346,19 @@ pub async fn run_sandbox( policy_rules, policy_data, &extension_credentials, + stream_bootstrap + .as_ref() + .and_then(|bootstrap| bootstrap.sandbox_config.clone()), ) .await? }; + if let Some(snapshot) = stream_bootstrap + .as_ref() + .and_then(|bootstrap| bootstrap.sandbox_config.as_ref()) + { + initial_agent_proposals_enabled = agent_proposals_enabled_from_settings(&snapshot.settings); + initial_extension_authentication_enabled = snapshot.extension_authentication_enabled; + } // Normalize the active driver's identity contract once, while both the // policy and launched image filesystem are available. Kubernetes and @@ -248,14 +390,62 @@ pub async fn run_sandbox( ); #[cfg_attr(not(target_os = "linux"), allow(unused_mut))] - let (provider_credentials, mut provider_env) = if let Some(bootstrap) = + let (provider_credentials, mut provider_env, provider_bootstrap_degraded) = if let Some( + bootstrap, + ) = sidecar_bootstrap.as_ref() { let provider_credentials = ProviderCredentialState::from_child_env_snapshot( bootstrap.provider_env_revision, bootstrap.provider_child_env.clone(), ); - (provider_credentials, bootstrap.provider_child_env.clone()) + ( + provider_credentials, + bootstrap.provider_child_env.clone(), + false, + ) + } else if let Some(snapshot) = stream_bootstrap + .as_ref() + .and_then(|bootstrap| bootstrap.provider_environment.clone()) + { + let result: openshell_core::grpc_client::ProviderEnvironmentResult = snapshot.into(); + let dynamic_credentials_fallback = result.dynamic_credentials.clone(); + let mut degraded = false; + let provider_credentials = ProviderCredentialState::from_bound_environment( + result.provider_env_revision, + result.environment, + result.credential_expires_at_ms, + result.dynamic_credentials, + result.static_credential_bindings, + result.non_secret_environment_keys, + ) + .unwrap_or_else(|error| { + degraded = true; + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::High) + .status(StatusId::Failure) + .state(StateId::Disabled, "fail_closed") + .message(format!( + "Rejected streamed provider environment bindings; static provider credentials were revoked; delivered dynamic token grants remain active: {error}" + )) + .build() + ); + ProviderCredentialState::from_environment( + result.provider_env_revision, + std::collections::HashMap::new(), + std::collections::HashMap::new(), + dynamic_credentials_fallback, + ) + }); + let provider_env = provider_credentials.child_env_with_gcp_resolved(); + (provider_credentials, provider_env, degraded) + } else if stream_bootstrap.is_some() { + ( + ProviderCredentialState::from_child_env_snapshot(0, std::collections::HashMap::new()), + std::collections::HashMap::new(), + true, + ) } else { // Fetch provider environment variables from the server. // This is done after loading the policy so the sandbox can still start @@ -352,9 +542,48 @@ pub async fn run_sandbox( } }; let provider_env = provider_credentials.child_env_with_gcp_resolved(); - (provider_credentials, provider_env) + (provider_credentials, provider_env, false) }; + let mut prepared_bootstrap_result = stream_bootstrap.as_ref().map(|bootstrap| { + use openshell_core::proto::{ConfigApplyOutcome, ConfigBootstrapResult, ConfigComponent}; + let mut results = Vec::with_capacity(2); + if let Some(snapshot) = bootstrap.provider_environment.as_ref() { + let revision = provider_config_revision(snapshot.provider_env_revision); + let outcome = if provider_bootstrap_degraded { + ConfigApplyOutcome::Degraded + } else { + ConfigApplyOutcome::Applied + }; + results.push(config_apply_result( + ConfigComponent::ProviderEnvironment, + revision, + Some(revision), + outcome, + None, + )); + } + if let Some(snapshot) = bootstrap.sandbox_config.as_ref() { + let settings: openshell_core::grpc_client::SettingsPollResult = snapshot.clone().into(); + let revision = sandbox_config_revision(&settings); + let outcome = if loaded_policy_origin.allows_gateway_policy_reload() { + ConfigApplyOutcome::Applied + } else { + ConfigApplyOutcome::RetainedLocalOverride + }; + let applied_revision = + (outcome != ConfigApplyOutcome::RetainedLocalOverride).then_some(revision); + results.push(config_apply_result( + ConfigComponent::SandboxConfig, + revision, + applied_revision, + outcome, + None, + )); + } + ConfigBootstrapResult { results } + }); + if credential_gating_unavailable( &loaded_policy_origin, provider_credentials.resolver().is_some(), @@ -585,6 +814,7 @@ pub async fn run_sandbox( Some(sidecar_control::spawn_server( &socket, sidecar_control::BootstrapData { + main_process_instance_id: main_process_instance_id.clone(), policy_proto: proto.clone(), provider_env_revision: provider_credentials.snapshot().revision, provider_env_generation: 0, @@ -632,6 +862,8 @@ pub async fn run_sandbox( trusted_ssh_socket_path: std::path::PathBuf::from(trusted_ssh_socket_path), control_publisher: sidecar_control_publisher.clone(), config_apply_tx: config_apply_tx.clone(), + prepared_supervisor_session: prepared_supervisor_session.take(), + prepared_bootstrap_result: prepared_bootstrap_result.take(), }, ); } @@ -777,6 +1009,10 @@ pub async fn run_sandbox( substrate_ready: transparent_tcp_substrate_ready, }, config_apply_rx: config_apply_rx.take(), + initial_stream_snapshot: stream_bootstrap + .as_ref() + .and_then(|bootstrap| bootstrap.sandbox_config.clone()) + .map(Into::into), }; tokio::spawn(async move { @@ -976,6 +1212,9 @@ pub async fn run_sandbox( main_env, ca_file_paths, agent_proposals.clone(), + main_process_instance_id, + prepared_supervisor_session.take(), + prepared_bootstrap_result.take(), Some(config_apply_tx.clone()), #[cfg(target_os = "linux")] netns.as_ref(), @@ -1335,6 +1574,9 @@ struct SidecarEntrypointHandler { config_apply_tx: tokio::sync::mpsc::Sender< openshell_supervisor_process::supervisor_session::ConfigApplyRequest, >, + prepared_supervisor_session: + Option, + prepared_bootstrap_result: Option, } #[cfg(target_os = "linux")] @@ -1352,6 +1594,8 @@ fn spawn_sidecar_entrypoint_handler( trusted_ssh_socket_path, control_publisher, config_apply_tx, + mut prepared_supervisor_session, + mut prepared_bootstrap_result, } = handler; let mut session_started = false; let mut session_task: Option> = None; @@ -1455,16 +1699,30 @@ fn spawn_sidecar_entrypoint_handler( ); continue; }; - session_task = Some(openshell_supervisor_process::supervisor_session::spawn( - endpoint.clone(), - id.clone(), - trusted_ssh_socket_path.clone(), - None, - Some(supervisor_pid), - Arc::clone(&terminating), - started.instance_id.clone(), - Some(config_apply_tx.clone()), - )); + session_task = if let Some(prepared) = prepared_supervisor_session.take() { + Some( + openshell_supervisor_process::supervisor_session::spawn_prepared( + prepared, + prepared_bootstrap_result.take(), + trusted_ssh_socket_path.clone(), + None, + Some(supervisor_pid), + Arc::clone(&terminating), + config_apply_tx.clone(), + ), + ) + } else { + Some(openshell_supervisor_process::supervisor_session::spawn( + endpoint.clone(), + id.clone(), + trusted_ssh_socket_path.clone(), + None, + Some(supervisor_pid), + Arc::clone(&terminating), + started.instance_id.clone(), + Some(config_apply_tx.clone()), + )) + }; session_started = true; info!("sidecar supervisor session task spawned"); } @@ -2316,6 +2574,7 @@ async fn load_policy( policy_rules: Option, policy_data: Option, extension_credentials: &openshell_extension_core::ExtensionCredentialStore, + initial_snapshot: Option, ) -> Result<( SandboxPolicy, Option>, @@ -2346,13 +2605,54 @@ async fn load_policy( std::path::Path::new(data_file), Some(&validate_middleware_config), )?; - let middleware_registry = - openshell_supervisor_middleware::MiddlewareRegistry::connect_services( - openshell_supervisor_middleware_builtins::services(), - Vec::new(), - ) - .await?; - engine.replace_middleware_registry(middleware_registry)?; + let initial_services = initial_snapshot.as_ref().map_or_else(Vec::new, |snapshot| { + snapshot.supervisor_middleware_services.clone() + }); + let initial_extension_authentication_enabled = initial_snapshot + .as_ref() + .is_some_and(|snapshot| snapshot.extension_authentication_enabled); + let middleware_authentication = if initial_extension_authentication_enabled { + if let Some(endpoint) = openshell_endpoint.as_deref() { + let credentials = + openshell_core::grpc_client::CachedOpenShellClient::connect_with_credentials( + endpoint, + extension_credentials.clone(), + ) + .await? + .refresh_extension_credentials(&initial_services) + .await?; + MiddlewareAuthentication { + credentials, + enabled: true, + } + } else { + MiddlewareAuthentication::default() + } + } else { + MiddlewareAuthentication::default() + }; + let middleware_registry_status = match connect_middleware_registry( + &initial_services, + &middleware_authentication, + ) + .await + { + Ok(registry) => { + engine.replace_middleware_registry(registry)?; + MiddlewareRegistryStatus::Synchronized + } + Err(error) => { + warn!(error = %error, "Local policy middleware registry is degraded at startup"); + let middleware_registry = + openshell_supervisor_middleware::MiddlewareRegistry::connect_services( + openshell_supervisor_middleware_builtins::services(), + Vec::new(), + ) + .await?; + engine.replace_middleware_registry(middleware_registry)?; + MiddlewareRegistryStatus::NeedsReconciliation + } + }; let config = engine.query_sandbox_config()?; let mut policy = SandboxPolicy { version: 1, @@ -2370,27 +2670,43 @@ async fn load_policy( policy, Some(Arc::new(engine)), None, - MiddlewareRegistryStatus::Synchronized, + middleware_registry_status, LoadedPolicyOrigin::LocalOverride, - false, - false, + initial_snapshot + .as_ref() + .is_some_and(|snapshot| agent_proposals_enabled_from_settings(&snapshot.settings)), + initial_extension_authentication_enabled, )); } - // gRPC mode: fetch typed proto policy, construct OPA engine from baked rules + proto data + // Gateway mode: consume a stream snapshot for the current protocol or + // fetch one for compatibility supervisors, then construct the OPA engine. if let (Some(id), Some(endpoint)) = (&sandbox_id, &openshell_endpoint) { - info!( - sandbox_id = %id, - endpoint = %endpoint, - "Fetching sandbox policy via gRPC" - ); - let mut snapshot = grpc_retry("Policy fetch", || { - openshell_core::grpc_client::fetch_settings_snapshot(endpoint, id) - }) - .await?; + let stream_bootstrap = initial_snapshot.is_some(); + if stream_bootstrap { + info!(sandbox_id = %id, "Loading sandbox policy from supervisor bootstrap"); + } else { + info!( + sandbox_id = %id, + endpoint = %endpoint, + "Fetching sandbox policy via compatibility RPC" + ); + } + let mut snapshot = if let Some(snapshot) = initial_snapshot { + snapshot.into() + } else { + grpc_retry("Policy fetch", || { + openshell_core::grpc_client::fetch_settings_snapshot(endpoint, id) + }) + .await? + }; let mut proto_policy = if let Some(p) = snapshot.policy.clone() { p + } else if stream_bootstrap { + return Err(miette::miette!( + "supervisor bootstrap omitted required sandbox policy" + )); } else { // No policy configured on the server. Discover from disk or // fall back to the restrictive default, then sync to the @@ -2444,6 +2760,11 @@ async fn load_policy( // back to the gateway so users can see the effective policy. let enriched = enrich_proto_baseline_paths(&mut proto_policy); let sync_policy = proto_sync_payload_for_enriched_policy(&proto_policy, enriched); + if stream_bootstrap && sync_policy.is_some() { + return Err(miette::miette!( + "supervisor bootstrap policy omitted required baseline paths" + )); + } if let Some(sync_policy) = sync_policy { if let Some(sandbox_name) = sandbox.as_deref() { match openshell_core::grpc_client::sync_policy_and_fetch_snapshot( @@ -2493,6 +2814,11 @@ async fn load_policy( let engine = match OpaEngine::from_proto(&proto_policy) { Ok(engine) => Arc::new(engine), Err(e) => { + if stream_bootstrap { + return Err(e).wrap_err( + "failed to install required sandbox policy from supervisor bootstrap", + ); + } report_initial_policy_failure(endpoint, id, loaded_policy_revision.as_ref(), &e) .await; let validation_error = e.to_string(); @@ -2566,6 +2892,11 @@ async fn load_policy( .await .and_then(|registry| engine.replace_middleware_registry(registry)) { + if stream_bootstrap { + return Err(error).wrap_err( + "failed to install required middleware runtime from supervisor bootstrap", + ); + } ocsf_emit!( ConfigStateChangeBuilder::new(ocsf_ctx()) .severity(SeverityId::Medium) @@ -3367,6 +3698,10 @@ struct PolicyPollLoopContext { openshell_supervisor_process::supervisor_session::ConfigApplyRequest, >, >, + /// Present for revision-2 sessions. The bootstrap already initialized + /// runtime state, so this seeds equality tracking and disables fetch-based + /// reconciliation for the current protocol. + initial_stream_snapshot: Option, } type MiddlewareConnector = Arc< @@ -4183,18 +4518,41 @@ async fn run_policy_poll_loop_with_client( status_receiver, )); - let mut current_config_revision: u64 = 0; - let mut current_stream_sandbox_revision = None; + let initial_stream_snapshot = ctx.initial_stream_snapshot.take(); + let stream_authoritative = initial_stream_snapshot.is_some(); + let mut current_config_revision: u64 = initial_stream_snapshot + .as_ref() + .map_or(0, |snapshot| snapshot.config_revision); + let mut current_stream_sandbox_revision = initial_stream_snapshot + .as_ref() + .map(sandbox_config_revision) + .filter(|_| ctx.loaded_policy_origin.allows_gateway_policy_reload()); let mut current_provider_env_revision: u64 = ctx.provider_credentials.snapshot().revision; - let mut current_policy_version: u32 = 0; - let mut current_policy_hash = String::new(); - let mut current_middleware_services = Vec::new(); - let mut current_extension_authentication_enabled = ctx.extension_authentication_enabled; + let mut current_policy_version: u32 = initial_stream_snapshot + .as_ref() + .map_or(0, |snapshot| snapshot.version); + let mut current_policy_hash = initial_stream_snapshot + .as_ref() + .map_or_else(String::new, |snapshot| snapshot.policy_hash.clone()); + let mut current_middleware_services = initial_stream_snapshot + .as_ref() + .map_or_else(Vec::new, |snapshot| { + snapshot.supervisor_middleware_services.clone() + }); + let mut current_extension_authentication_enabled = initial_stream_snapshot + .as_ref() + .map_or(ctx.extension_authentication_enabled, |snapshot| { + snapshot.extension_authentication_enabled + }); let mut middleware_registry_status = ctx.middleware_registry_status; let mut current_settings: std::collections::HashMap< String, openshell_core::proto::EffectiveSetting, - > = std::collections::HashMap::new(); + > = initial_stream_snapshot + .as_ref() + .map_or_else(std::collections::HashMap::new, |snapshot| { + snapshot.settings.clone() + }); let reloads_gateway_policy = ctx.loaded_policy_origin.allows_gateway_policy_reload(); let mut last_failed_runtime_revision: Option = None; let mut rejected_policy_generation: Option = None; @@ -4208,71 +4566,117 @@ async fn run_policy_poll_loop_with_client( // Initialize revision from the first poll and acknowledge the initial // policy revision the supervisor actually loaded. A mismatched result is // reconciled below instead of being recorded as already applied. - match client.poll_settings(&ctx.sandbox_id).await { - Ok(result) => { - let _ = ctx.workspace_tx.send(client.workspace()); - match initial_poll_disposition(&ctx.loaded_policy_origin, &result) { - InitialPollDisposition::Acknowledge(candidate) => { - let stream_revision = sandbox_config_revision(&result); - apply_ocsf_json_setting(&ctx.ocsf_enabled, &result.settings); - apply_ocsf_schema_version_setting(&ctx.ocsf_schema_version, &result.settings); - apply_agent_proposals_enabled( - &ctx.agent_proposals, - agent_proposals_enabled_from_settings(&result.settings), - "initial settings poll", - Some(candidate.config_revision), - ctx.sidecar_control_publisher.as_ref(), - skills::install_static_skills, - ); - current_config_revision = candidate.config_revision; - current_policy_version = candidate.version; - current_policy_hash.clone_from(&candidate.policy_hash); - current_middleware_services = result.supervisor_middleware_services; - current_extension_authentication_enabled = - result.extension_authentication_enabled; - current_settings = result.settings; - current_stream_sandbox_revision = Some(stream_revision); - enqueue_policy_status( - &status_sender, - PolicyStatusUpdate::initial_loaded(&candidate), - ); - debug!( - config_revision = current_config_revision, - "Settings poll: initial policy matches loaded revision" - ); - } - InitialPollDisposition::Reconcile => pending_result = Some(result), - InitialPollDisposition::TrackOnly => { - apply_ocsf_json_setting(&ctx.ocsf_enabled, &result.settings); - apply_ocsf_schema_version_setting(&ctx.ocsf_schema_version, &result.settings); - apply_agent_proposals_enabled( - &ctx.agent_proposals, - agent_proposals_enabled_from_settings(&result.settings), - "initial settings poll", - Some(result.config_revision), - ctx.sidecar_control_publisher.as_ref(), - skills::install_static_skills, - ); - current_config_revision = result.config_revision; - current_policy_hash = result.policy_hash.clone(); - current_middleware_services = result.supervisor_middleware_services; - current_extension_authentication_enabled = - result.extension_authentication_enabled; - current_settings = result.settings; - debug!( - config_revision = current_config_revision, - "Settings poll: tracking gateway config while preserving local policy override" - ); + if let Some(snapshot) = initial_stream_snapshot.as_ref() { + let _ = ctx.workspace_tx.send(snapshot.workspace.clone()); + apply_ocsf_json_setting(&ctx.ocsf_enabled, &snapshot.settings); + apply_ocsf_schema_version_setting(&ctx.ocsf_schema_version, &snapshot.settings); + } else { + match client.poll_settings(&ctx.sandbox_id).await { + Ok(result) => { + let _ = ctx.workspace_tx.send(client.workspace()); + match initial_poll_disposition(&ctx.loaded_policy_origin, &result) { + InitialPollDisposition::Acknowledge(candidate) => { + let stream_revision = sandbox_config_revision(&result); + apply_ocsf_json_setting(&ctx.ocsf_enabled, &result.settings); + apply_ocsf_schema_version_setting( + &ctx.ocsf_schema_version, + &result.settings, + ); + apply_agent_proposals_enabled( + &ctx.agent_proposals, + agent_proposals_enabled_from_settings(&result.settings), + "initial settings poll", + Some(candidate.config_revision), + ctx.sidecar_control_publisher.as_ref(), + skills::install_static_skills, + ); + current_config_revision = candidate.config_revision; + current_policy_version = candidate.version; + current_policy_hash.clone_from(&candidate.policy_hash); + current_middleware_services = result.supervisor_middleware_services; + current_extension_authentication_enabled = + result.extension_authentication_enabled; + current_settings = result.settings; + current_stream_sandbox_revision = Some(stream_revision); + enqueue_policy_status( + &status_sender, + PolicyStatusUpdate::initial_loaded(&candidate), + ); + debug!( + config_revision = current_config_revision, + "Settings poll: initial policy matches loaded revision" + ); + } + InitialPollDisposition::Reconcile => pending_result = Some(result), + InitialPollDisposition::TrackOnly => { + apply_ocsf_json_setting(&ctx.ocsf_enabled, &result.settings); + apply_ocsf_schema_version_setting( + &ctx.ocsf_schema_version, + &result.settings, + ); + apply_agent_proposals_enabled( + &ctx.agent_proposals, + agent_proposals_enabled_from_settings(&result.settings), + "initial settings poll", + Some(result.config_revision), + ctx.sidecar_control_publisher.as_ref(), + skills::install_static_skills, + ); + current_config_revision = result.config_revision; + current_policy_hash = result.policy_hash.clone(); + current_middleware_services = result.supervisor_middleware_services; + current_extension_authentication_enabled = + result.extension_authentication_enabled; + current_settings = result.settings; + debug!( + config_revision = current_config_revision, + "Settings poll: tracking gateway config while preserving local policy override" + ); + } } } - } - Err(e) => { - warn!(error = %e, "Settings poll: failed to fetch initial version, will retry"); + Err(e) => { + warn!(error = %e, "Settings poll: failed to fetch initial version, will retry"); + } } } let interval = Duration::from_secs(ctx.interval_secs); loop { + if stream_authoritative { + let delay = next_poll_delay(&ctx.extension_credentials, interval); + tokio::select! { + request = receive_config_apply(&mut config_apply_rx) => { + let Some(request) = request else { + return Err(miette::miette!("stream configuration apply channel closed")); + }; + apply_stream_config_request( + &ctx, + &client, + request, + &mut current_config_revision, + &mut current_stream_sandbox_revision, + &mut current_provider_env_revision, + &mut current_policy_version, + &mut current_policy_hash, + &mut current_middleware_services, + &mut current_extension_authentication_enabled, + &mut middleware_registry_status, + &mut current_settings, + reloads_gateway_policy, + &mut has_last_valid_policy, + ).await; + } + () = tokio::time::sleep(delay) => { + if current_extension_authentication_enabled + && let Err(error) = client.refresh_installed_extension_credentials().await + { + warn!(error = %error, "Extension credential refresh failed"); + } + } + } + continue; + } let result = if let Some(result) = pending_result.take() { result } else { @@ -5434,6 +5838,7 @@ network_policies: >, >, reports: UnboundedSender<(u32, bool, String)>, + poll_calls: Arc, } #[tonic::async_trait] @@ -5442,6 +5847,7 @@ network_policies: &self, _sandbox_id: &str, ) -> Result { + self.poll_calls.fetch_add(1, Ordering::SeqCst); self.polls .lock() .await @@ -5521,6 +5927,7 @@ network_policies: ScriptedPolicyGateway { polls: Arc::new(tokio::sync::Mutex::new(poll_rx)), reports: report_tx, + poll_calls: Arc::new(AtomicUsize::new(0)), }, poll_tx, report_rx, @@ -5556,6 +5963,7 @@ network_policies: middleware_connector, transparent_tcp: TransparentTcpReloadState::default(), config_apply_rx: None, + initial_stream_snapshot: None, } } @@ -5661,6 +6069,60 @@ network_policies: ); } + #[tokio::test] + async fn revision_two_stream_never_polls_gateway_settings() { + let initial = settings_poll_result( + Some(proto_policy_fixture()), + 1, + openshell_core::proto::PolicySource::Sandbox, + ); + let engine = + Arc::new(OpaEngine::from_proto(&proto_policy_fixture()).expect("build OPA engine")); + let mut ctx = policy_poll_test_context( + engine, + LoadedPolicyOrigin::Gateway { + revision: Some(LoadedPolicyRevision::from_snapshot(&initial)), + has_last_valid_policy: true, + }, + default_middleware_connector(), + ); + ctx.initial_stream_snapshot = Some(initial); + let (config_apply_tx, config_apply_rx) = tokio::sync::mpsc::channel(1); + ctx.config_apply_rx = Some(config_apply_rx); + let (client, _polls, _reports) = scripted_policy_gateway(); + let poll_calls = Arc::clone(&client.poll_calls); + + let handle = tokio::spawn(run_policy_poll_loop_with_client(ctx, client)); + let (response_tx, response_rx) = tokio::sync::oneshot::channel(); + config_apply_tx + .send( + openshell_supervisor_process::supervisor_session::ConfigApplyRequest::Update { + update: openshell_core::proto::ConfigUpdate { + update_id: "provider-2".to_string(), + component_sequence: 1, + component: Some( + openshell_core::proto::config_update::Component::ProviderEnvironment( + openshell_core::proto::ProviderEnvironmentSnapshot { + provider_env_revision: 2, + ..Default::default() + }, + ), + ), + }, + response: response_tx, + }, + ) + .await + .unwrap(); + timeout(Duration::from_secs(1), response_rx) + .await + .expect("stream update timed out") + .expect("stream update responder stopped"); + + assert_eq!(poll_calls.load(Ordering::SeqCst), 0); + handle.abort(); + } + #[tokio::test] async fn failed_stream_snapshot_remains_retryable() { let initial = settings_poll_result( diff --git a/crates/openshell-sandbox/src/sidecar_control.rs b/crates/openshell-sandbox/src/sidecar_control.rs index 11f3e68e23..bef52b02ae 100644 --- a/crates/openshell-sandbox/src/sidecar_control.rs +++ b/crates/openshell-sandbox/src/sidecar_control.rs @@ -22,6 +22,7 @@ use tracing::{debug, info, warn}; #[derive(Debug, Clone)] pub struct BootstrapData { + pub main_process_instance_id: String, pub policy_proto: openshell_core::proto::SandboxPolicy, pub provider_env_revision: u64, pub provider_env_generation: u64, @@ -184,6 +185,7 @@ enum WireClientMessage { #[serde(tag = "type", rename_all = "snake_case")] enum WireServerMessage { BootstrapResponse { + main_process_instance_id: String, policy_proto: Vec, provider_env_revision: u64, provider_env_generation: u64, @@ -215,6 +217,7 @@ impl BootstrapData { #[cfg_attr(not(target_os = "linux"), allow(dead_code))] fn to_wire(&self) -> WireServerMessage { WireServerMessage::BootstrapResponse { + main_process_instance_id: self.main_process_instance_id.clone(), policy_proto: self.policy_proto.encode_to_vec(), provider_env_revision: self.provider_env_revision, provider_env_generation: self.provider_env_generation, @@ -237,6 +240,7 @@ impl TryFrom for BootstrapData { fn try_from(message: WireServerMessage) -> Result { let WireServerMessage::BootstrapResponse { + main_process_instance_id, policy_proto, provider_env_revision, provider_env_generation, @@ -260,6 +264,7 @@ impl TryFrom for BootstrapData { )?; Ok(Self { + main_process_instance_id, policy_proto, provider_env_revision, provider_env_generation, @@ -752,6 +757,7 @@ mod tests { fn bootstrap_message(policy: &SandboxPolicy) -> WireServerMessage { WireServerMessage::BootstrapResponse { + main_process_instance_id: "instance-1".to_string(), policy_proto: policy.encode_to_vec(), provider_env_revision: 0, provider_env_generation: 0, @@ -826,6 +832,7 @@ mod tests { let mut env = HashMap::new(); env.insert("GITHUB_TOKEN".to_string(), "secret".to_string()); let bootstrap = BootstrapData { + main_process_instance_id: "instance-1".to_string(), policy_proto: SandboxPolicy { version: 7, ..SandboxPolicy::default() @@ -843,6 +850,7 @@ mod tests { .await .unwrap(); + assert_eq!(received.main_process_instance_id, "instance-1"); assert_eq!(received.policy_proto.version, 7); assert_eq!(received.provider_env_revision, 3); assert_eq!(received.provider_env_generation, 0); @@ -865,6 +873,7 @@ mod tests { let server = spawn_server( &socket, BootstrapData { + main_process_instance_id: "instance-1".to_string(), policy_proto: SandboxPolicy::default(), provider_env_revision: u64::MAX, provider_env_generation: 7, @@ -949,6 +958,7 @@ mod tests { let server = spawn_server( &socket, BootstrapData { + main_process_instance_id: "instance-1".to_string(), policy_proto: SandboxPolicy::default(), provider_env_revision: 0, provider_env_generation: 0, @@ -990,6 +1000,7 @@ mod tests { let server = spawn_server( &socket, BootstrapData { + main_process_instance_id: "instance-1".to_string(), policy_proto: SandboxPolicy::default(), provider_env_revision: 0, provider_env_generation: 0, @@ -1071,6 +1082,7 @@ mod tests { let _server = spawn_server( &socket, BootstrapData { + main_process_instance_id: "instance-1".to_string(), policy_proto: SandboxPolicy::default(), provider_env_revision: 0, provider_env_generation: 0, @@ -1106,6 +1118,7 @@ mod tests { let server = spawn_server( &socket, BootstrapData { + main_process_instance_id: "instance-1".to_string(), policy_proto: SandboxPolicy::default(), provider_env_revision: 0, provider_env_generation: 0, @@ -1136,6 +1149,7 @@ mod tests { let server = spawn_server( &socket, BootstrapData { + main_process_instance_id: "instance-1".to_string(), policy_proto: SandboxPolicy::default(), provider_env_revision: 0, provider_env_generation: 0, @@ -1167,6 +1181,7 @@ mod tests { let server = spawn_server( &socket, BootstrapData { + main_process_instance_id: "instance-1".to_string(), policy_proto: SandboxPolicy::default(), provider_env_revision: 0, provider_env_generation: 0, diff --git a/crates/openshell-server/proto/storage.proto b/crates/openshell-server/proto/storage.proto index 2ca7946f83..851f260032 100644 --- a/crates/openshell-server/proto/storage.proto +++ b/crates/openshell-server/proto/storage.proto @@ -74,6 +74,21 @@ message StoredProviderProfile { openshell.v1.ProviderProfile profile = 2; } +// Compact, non-secret record of the latest supervisor-observed state for one +// gateway-owned configuration component. Full delivered snapshots never enter +// durable storage. +message StoredConfigComponentObservation { + openshell.datamodel.v1.ObjectMeta metadata = 1; + string sandbox_id = 2; + openshell.v1.ConfigComponent component = 3; + openshell.v1.ConfigSnapshotRevision requested_revision = 4; + openshell.v1.ConfigSnapshotRevision applied_revision = 5; + openshell.v1.ConfigApplyOutcome outcome = 6; + string effective_source = 7; + int64 observed_at_ms = 8; + string sanitized_error = 9; +} + // Stored payload for a policy revision row in the generic objects table. message PolicyRevisionPayload { // Serialized policy contents. diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index de221f67db..7f1d072995 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -10,8 +10,9 @@ pub mod rootfs_tar; use crate::grpc::policy::SANDBOX_SETTINGS_OBJECT_TYPE; use crate::otel_tracing::TraceContextInterceptor; use crate::persistence::{ - DRAFT_CHUNK_OBJECT_TYPE, ObjectCursor, ObjectId, ObjectListQuery, ObjectName, ObjectRecord, - ObjectType, POLICY_OBJECT_TYPE, Store, WriteCondition, + CONFIG_COMPONENT_OBSERVATION_OBJECT_TYPE, DRAFT_CHUNK_OBJECT_TYPE, ObjectCursor, ObjectId, + ObjectListQuery, ObjectName, ObjectRecord, ObjectType, POLICY_OBJECT_TYPE, Store, + WriteCondition, }; use crate::sandbox_index::SandboxIndex; use crate::sandbox_watch::SandboxWatchBus; @@ -3350,6 +3351,10 @@ impl ComputeRuntime { for (object_type, label) in [ (POLICY_OBJECT_TYPE, "policy revisions"), (DRAFT_CHUNK_OBJECT_TYPE, "draft policy chunks"), + ( + CONFIG_COMPONENT_OBSERVATION_OBJECT_TYPE, + "configuration component observations", + ), ] { self.store .delete_by_scope(object_type, sandbox.object_id()) @@ -6304,6 +6309,19 @@ mod tests { ) .await .unwrap(); + runtime + .store + .put_scoped( + CONFIG_COMPONENT_OBSERVATION_OBJECT_TYPE, + "observation-owned", + "observation-owned", + sandbox.object_workspace(), + sandbox.object_id(), + br#"{"outcome":"applied"}"#, + None, + ) + .await + .unwrap(); session } @@ -6374,6 +6392,18 @@ mod tests { .is_some(), expected ); + assert_eq!( + runtime + .store + .get( + CONFIG_COMPONENT_OBSERVATION_OBJECT_TYPE, + "observation-owned", + ) + .await + .unwrap() + .is_some(), + expected + ); } fn make_driver_condition(reason: &str, message: &str) -> DriverCondition { diff --git a/crates/openshell-server/src/persistence/mod.rs b/crates/openshell-server/src/persistence/mod.rs index ac55c4db8e..bc0e290079 100644 --- a/crates/openshell-server/src/persistence/mod.rs +++ b/crates/openshell-server/src/persistence/mod.rs @@ -23,6 +23,8 @@ pub use sqlite::SqliteStore; pub const POLICY_OBJECT_TYPE: &str = "sandbox_policy"; /// Object type string for draft policy chunk records. pub const DRAFT_CHUNK_OBJECT_TYPE: &str = "draft_policy_chunk"; +/// Object type string for compact supervisor component observations. +pub const CONFIG_COMPONENT_OBSERVATION_OBJECT_TYPE: &str = "config_component_observation"; pub type PersistenceResult = Result; diff --git a/crates/openshell-server/src/storage_proto.rs b/crates/openshell-server/src/storage_proto.rs index 8f7c55914f..f00972918b 100644 --- a/crates/openshell-server/src/storage_proto.rs +++ b/crates/openshell-server/src/storage_proto.rs @@ -108,6 +108,48 @@ impl ObjectWorkspace for StoredProviderCredentialRefreshState { } } +impl ObjectId for StoredConfigComponentObservation { + fn object_id(&self) -> &str { + self.metadata.as_ref().map_or("", |m| m.id.as_str()) + } +} + +impl ObjectName for StoredConfigComponentObservation { + fn object_name(&self) -> &str { + self.metadata.as_ref().map_or("", |m| m.name.as_str()) + } +} + +impl ObjectLabels for StoredConfigComponentObservation { + fn object_labels(&self) -> Option> { + self.metadata.as_ref().map(|m| m.labels.clone()) + } +} + +impl SetResourceVersion for StoredConfigComponentObservation { + fn set_resource_version(&mut self, version: u64) { + if let Some(meta) = self.metadata.as_mut() { + meta.resource_version = version; + } + } +} + +impl GetResourceVersion for StoredConfigComponentObservation { + fn get_resource_version(&self) -> u64 { + self.metadata.as_ref().map_or(0, |m| m.resource_version) + } +} + +impl ObjectWorkspace for StoredConfigComponentObservation { + fn object_workspace(&self) -> &str { + self.metadata.as_ref().map_or("", |m| m.workspace.as_str()) + } + + fn requires_workspace() -> bool { + true + } +} + #[cfg(test)] mod tests { use super::*; @@ -117,13 +159,13 @@ mod tests { use std::collections::{BTreeMap, BTreeSet, VecDeque}; const STORAGE_V1_SCHEMA_SHA256: &str = - "79c72615d957fc0653c672f61998bf7d8d21b757bc05d07b3fff92bd70fc8f52"; + "a623124c961f3a56af58a4ab148c12985e5efaec1ad23441031f5cd5b073fe22"; const PUBLIC_RPC_SCHEMA_SHA256: &str = "0f14943574349d02bdc61076c8c5a59a98b627325564ef1a6d21d7941825dc46"; const DURABLE_SCHEMA_SHA256: &str = - "920a5243dfb37ce709f0f562a47d17791a5ede90fd7f662ed01542abd60a0dfb"; + "60f2f912d45bb8bdc6bbf9a62a27b03aad2e65d8564b3f5832d19803a239ac79"; const PUBLIC_DURABLE_OVERLAP_SHA256: &str = - "05add438ba041defc98d791038ae593d3f09352677cae43f2276d494205ce415"; + "0d1b24b78becb0025512f35fe320b0d91d0728d3239eabf78cdf99e599eb6af2"; // Synthetic payloads generated with the public declarations at v0.0.116, // before their relocation into openshell.storage.v1. Values are deliberately // non-secret and the ordinary protobuf bytes contain no package names. @@ -136,21 +178,23 @@ mod tests { "0a0472756c651a07666978747572652d0000403f3a0b6578616d706c652e636f6d40bb035002"; const V0_0_116_POLICY_RECORD: &str = "0a09706f6c6963792d6964120a73616e64626f782d6964180222030102032a0673686132353632066c6f616465643a046e6f6e6540fa0148ac0252110a06736f75726365120766697874757265"; const V0_0_116_DRAFT_RECORD: &str = "0a086368756e6b2d6964120a73616e64626f782d69641802220770656e64696e672a0472756c65320204053a076669787475726549000000000000e83f50de02589003620b6578616d706c652e636f6d68bb037801"; - const STORAGE_MESSAGE_NAMES: [&str; 7] = [ + const STORAGE_MESSAGE_NAMES: [&str; 8] = [ "DraftChunkPayload", "PolicyRevisionPayload", + "StoredConfigComponentObservation", "StoredDraftChunk", "StoredPolicyRevision", "StoredProviderCredentialRefreshState", "StoredProviderProfile", "StoredRefreshMaterialDeletion", ]; - const DURABLE_ROOTS: [&str; 12] = [ + const DURABLE_ROOTS: [&str; 13] = [ ".openshell.datamodel.v1.Provider", ".openshell.datamodel.v1.Workspace", ".openshell.sandbox.v1.SandboxPolicy", ".openshell.storage.v1.DraftChunkPayload", ".openshell.storage.v1.PolicyRevisionPayload", + ".openshell.storage.v1.StoredConfigComponentObservation", ".openshell.storage.v1.StoredProviderCredentialRefreshState", ".openshell.storage.v1.StoredProviderProfile", ".openshell.v1.Sandbox", @@ -491,9 +535,9 @@ mod tests { ); assert_eq!( (durable_closure.messages.len(), durable_closure.enums.len()), - (81, 8) + (84, 11) ); - assert_eq!((overlap_messages.len(), overlap_enums.len()), (71, 8)); + assert_eq!((overlap_messages.len(), overlap_enums.len()), (73, 11)); assert_eq!( public_inventory_hash, PUBLIC_RPC_SCHEMA_SHA256, diff --git a/crates/openshell-server/src/supervisor_session.rs b/crates/openshell-server/src/supervisor_session.rs index 72a626566a..808a2333ad 100644 --- a/crates/openshell-server/src/supervisor_session.rs +++ b/crates/openshell-server/src/supervisor_session.rs @@ -27,6 +27,7 @@ use openshell_core::proto::{ SUPERVISOR_PROTOCOL_REVISION, }; use openshell_core::transport_errors::is_expected_transport_close_status; +use openshell_core::{ObjectId, ObjectWorkspace}; use crate::ServerState; use crate::auth::principal::Principal; @@ -35,6 +36,8 @@ use crate::config_delivery::{ }; #[cfg(test)] use crate::config_delivery::{LocalSupervisorConfigRouter, SupervisorConfigRouter}; +use crate::persistence::{CONFIG_COMPONENT_OBSERVATION_OBJECT_TYPE, ObjectType, current_time_ms}; +use crate::storage_proto::StoredConfigComponentObservation; const HEARTBEAT_INTERVAL_SECS: u32 = 15; const RELAY_PENDING_TIMEOUT: Duration = Duration::from_secs(10); @@ -53,6 +56,12 @@ const MAX_PENDING_RELAYS: usize = 256; /// cap (20) so tunnel-specific limits still fire first for that caller. const MAX_PENDING_RELAYS_PER_SANDBOX: usize = 32; +impl ObjectType for StoredConfigComponentObservation { + fn object_type() -> &'static str { + CONFIG_COMPONENT_OBSERVATION_OBJECT_TYPE + } +} + // --------------------------------------------------------------------------- // Session registry // --------------------------------------------------------------------------- @@ -1628,6 +1637,7 @@ async fn record_component_apply_result( "outcome" => outcome.as_str_name(), ) .increment(1); + record_config_component_observation(state, sandbox_id, component, outcome, result).await?; if component != ConfigComponent::SandboxConfig { return Ok(()); } @@ -1670,6 +1680,68 @@ async fn record_component_apply_result( .await } +async fn record_config_component_observation( + state: &Arc, + sandbox_id: &str, + component: ConfigComponent, + outcome: ConfigApplyOutcome, + result: &ConfigComponentApplyResult, +) -> Result<(), Status> { + let sandbox = state + .store + .get_message::(sandbox_id) + .await + .map_err(|error| { + Status::internal(format!("fetch sandbox for observation failed: {error}")) + })? + .ok_or_else(|| Status::not_found("sandbox not found while recording observation"))?; + let component_name = match component { + ConfigComponent::SandboxConfig => "sandbox_config", + ConfigComponent::ProviderEnvironment => "provider_environment", + ConfigComponent::Unspecified => "unspecified", + }; + let observation_id = format!("{sandbox_id}:{component_name}"); + let now_ms = current_time_ms(); + let sanitized_error = result + .failure + .as_ref() + .map(|failure| failure.message.chars().take(1024).collect()) + .unwrap_or_default(); + let observation = StoredConfigComponentObservation { + metadata: Some(openshell_core::proto::ObjectMeta { + id: observation_id.clone(), + name: observation_id, + workspace: sandbox.object_workspace().to_string(), + created_at_ms: now_ms, + ..Default::default() + }), + sandbox_id: sandbox.object_id().to_string(), + component: component.into(), + requested_revision: result.requested_revision, + applied_revision: result.applied_revision, + outcome: outcome.into(), + effective_source: match outcome { + ConfigApplyOutcome::RetainedLocalOverride => "local_override", + ConfigApplyOutcome::FailedRetainedLastKnownGood => "last_known_good", + ConfigApplyOutcome::FailedClosed => "fail_closed", + ConfigApplyOutcome::Degraded => "gateway_degraded", + ConfigApplyOutcome::Unspecified + | ConfigApplyOutcome::Applied + | ConfigApplyOutcome::IgnoredDuplicate + | ConfigApplyOutcome::IgnoredStale + | ConfigApplyOutcome::Unsupported => "gateway", + } + .to_string(), + observed_at_ms: now_ms, + sanitized_error, + }; + state + .store + .put_scoped_message(&observation, sandbox_id) + .await + .map_err(|error| Status::internal(format!("persist component observation failed: {error}"))) +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -1819,6 +1891,52 @@ mod tests { state } + #[tokio::test] + async fn component_apply_result_persists_compact_observed_state() { + let state = state_with_sandbox("sb-observed").await; + let requested_revision = ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::ProviderEnvironment(11)), + }; + let applied_revision = ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::ProviderEnvironment(9)), + }; + record_component_apply_result( + &state, + "sb-observed", + &ConfigComponentApplyResult { + component: ConfigComponent::ProviderEnvironment.into(), + requested_revision: Some(requested_revision), + applied_revision: Some(applied_revision), + outcome: ConfigApplyOutcome::FailedRetainedLastKnownGood.into(), + failure: Some(openshell_core::proto::ConfigApplyFailure { + message: "x".repeat(2_000), + ..Default::default() + }), + }, + ) + .await + .unwrap(); + + let observation = state + .store + .get_message::("sb-observed:provider_environment") + .await + .unwrap() + .expect("component observation"); + assert_eq!(observation.sandbox_id, "sb-observed"); + assert_eq!( + observation.component, + ConfigComponent::ProviderEnvironment as i32 + ); + assert_eq!( + observation.outcome, + ConfigApplyOutcome::FailedRetainedLastKnownGood as i32 + ); + assert_eq!(observation.effective_source, "last_known_good"); + assert_eq!(observation.sanitized_error.len(), 1_024); + assert!(observation.observed_at_ms > 0); + } + async fn first_gateway_message( harness: &mut crate::grpc::test_support::SupervisorStreamHarness, ) -> GatewayMessage { diff --git a/crates/openshell-supervisor-process/src/run.rs b/crates/openshell-supervisor-process/src/run.rs index f44460a44e..936e46b2b9 100644 --- a/crates/openshell-supervisor-process/src/run.rs +++ b/crates/openshell-supervisor-process/src/run.rs @@ -85,6 +85,9 @@ pub async fn run_process( provider_env: std::collections::HashMap, ca_file_paths: Option<(std::path::PathBuf, std::path::PathBuf)>, agent_proposals: AgentProposals, + main_instance_id: String, + prepared_supervisor_session: Option, + prepared_bootstrap_result: Option, config_apply_tx: Option< tokio::sync::mpsc::Sender, >, @@ -126,7 +129,13 @@ pub async fn run_process( // proposals flag is on at startup, rather than waiting for the policy // poll loop's first tick. In offline/file-mode there is no gateway, so // the flag stays at its default (false) and no skill is installed. - install_initial_agent_skill(sandbox_id, openshell_endpoint, &agent_proposals).await; + install_initial_agent_skill( + sandbox_id, + openshell_endpoint, + &agent_proposals, + prepared_supervisor_session.is_none(), + ) + .await; // Provider token grants may mount supervisor-only identity sockets such as // the SPIFFE Workload API. Prepare the child mount namespace that hides @@ -274,7 +283,6 @@ pub async fn run_process( let main_pid = handle.pid(); let main_session = crate::main_session::MainSession::new(handle.take_io(), main_pid); - let main_instance_id = uuid::Uuid::new_v4().to_string(); // SSH-spawned shells get http_proxy=http://: exported into // their env so cooperative tools (curl, npm, Node) route through the @@ -370,13 +378,25 @@ pub async fn run_process( // Spawn the persistent supervisor session if we have a gateway endpoint // and sandbox identity. The session provides relay channels for SSH // connect and ExecSandbox through the gateway. - let supervisor_session_task = if let (Some(endpoint), Some(id), Some(socket)) = - (openshell_endpoint, sandbox_id, ssh_socket_path.as_ref()) + let supervisor_session_task = if let (Some(prepared), Some(config_apply_tx)) = + (prepared_supervisor_session, config_apply_tx.clone()) { + let task = crate::supervisor_session::spawn_prepared( + prepared, + prepared_bootstrap_result, + ssh_socket_path.clone().unwrap_or_default(), + ssh_netns_fd, + None, + Arc::clone(&supervisor_terminating), + config_apply_tx, + ); + info!("prepared supervisor session task resumed"); + Some(task) + } else if let (Some(endpoint), Some(id)) = (openshell_endpoint, sandbox_id) { let task = crate::supervisor_session::spawn( endpoint.to_string(), id.to_string(), - socket.clone(), + ssh_socket_path.clone().unwrap_or_default(), ssh_netns_fd, None, Arc::clone(&supervisor_terminating), @@ -726,10 +746,12 @@ async fn install_initial_agent_skill( sandbox_id: Option<&str>, openshell_endpoint: Option<&str>, agent_proposals: &AgentProposals, + fetch_settings: bool, ) { use openshell_core::proto::setting_value; - if let (Some(id), Some(endpoint)) = (sandbox_id, openshell_endpoint) + if fetch_settings + && let (Some(id), Some(endpoint)) = (sandbox_id, openshell_endpoint) && let Ok(client) = openshell_core::grpc_client::CachedOpenShellClient::connect(endpoint).await && let Ok(result) = client.poll_settings(id).await diff --git a/crates/openshell-supervisor-process/src/supervisor_session.rs b/crates/openshell-supervisor-process/src/supervisor_session.rs index 62003abd1d..c1892eca2c 100644 --- a/crates/openshell-supervisor-process/src/supervisor_session.rs +++ b/crates/openshell-supervisor-process/src/supervisor_session.rs @@ -46,6 +46,7 @@ use openshell_core::transport_errors::is_expected_transport_close_status; const INITIAL_BACKOFF: Duration = Duration::from_secs(1); const MAX_BACKOFF: Duration = Duration::from_secs(30); const CONFIG_APPLY_TIMEOUT: Duration = Duration::from_mins(1); +const SESSION_PREPARE_TIMEOUT: Duration = Duration::from_mins(2); /// A stream-delivered desired-state payload awaiting application by the /// sandbox runtime. The response travels back over `ConnectSupervisor`. @@ -60,6 +61,33 @@ pub enum ConfigApplyRequest { }, } +/// A revision-2 supervisor session that has received its required bootstrap. +/// +/// It has not yet reported runtime initialization. Holding the stream open +/// across sandbox construction makes the bootstrap the source of initial +/// gateway-owned state rather than a later reconciliation input. +pub struct PreparedSupervisorSession { + endpoint: String, + sandbox_id: String, + instance_id: String, + channel: grpc_client::AuthedChannel, + tx: mpsc::Sender, + inbound: tonic::Streaming, + heartbeat_secs: u32, + protocol_revision: u32, + bootstrap: Option, +} + +impl PreparedSupervisorSession { + pub fn take_bootstrap(&mut self) -> Option { + self.bootstrap.take() + } + + pub fn uses_stream_configuration(&self) -> bool { + self.protocol_revision == SUPERVISOR_PROTOCOL_REVISION + } +} + #[derive(Default)] struct ConfigSequenceWatermarks { sandbox_config: u64, @@ -367,7 +395,51 @@ pub fn spawn( instance_id, config_apply_tx, }; - tokio::spawn(run_session_loop(config)) + tokio::spawn(run_session_loop(config, None)) +} + +/// Establish the revision-2 control stream and receive its required bootstrap +/// before gateway-owned runtime initialization begins. +pub async fn prepare( + endpoint: String, + sandbox_id: String, + instance_id: String, +) -> Result> { + let prepared = tokio::time::timeout( + SESSION_PREPARE_TIMEOUT, + open_session(endpoint, sandbox_id, instance_id), + ) + .await + .map_err(|_| "timed out waiting for supervisor session bootstrap")??; + if prepared.protocol_revision == SUPERVISOR_PROTOCOL_REVISION && prepared.bootstrap.is_none() { + return Err("revision-2 gateway omitted required configuration bootstrap".into()); + } + Ok(prepared) +} + +/// Resume a prepared startup session after the sandbox has installed the +/// bootstrap and made its runtime endpoints ready. +#[allow(clippy::too_many_arguments)] +pub fn spawn_prepared( + prepared: PreparedSupervisorSession, + bootstrap_result: Option, + ssh_socket_path: std::path::PathBuf, + netns_fd: Option, + expected_ssh_peer_pid: Option, + terminating: Arc, + config_apply_tx: mpsc::Sender, +) -> tokio::task::JoinHandle<()> { + let config = SessionConfig { + endpoint: prepared.endpoint.clone(), + sandbox_id: prepared.sandbox_id.clone(), + ssh_socket_path, + netns_fd, + expected_ssh_peer_pid, + terminating, + instance_id: prepared.instance_id.clone(), + config_apply_tx: Some(config_apply_tx), + }; + tokio::spawn(run_session_loop(config, Some((prepared, bootstrap_result)))) } struct SessionConfig { @@ -381,14 +453,22 @@ struct SessionConfig { config_apply_tx: Option>, } -async fn run_session_loop(config: SessionConfig) { +async fn run_session_loop( + config: SessionConfig, + mut prepared: Option<(PreparedSupervisorSession, Option)>, +) { let mut backoff = INITIAL_BACKOFF; let mut attempt: u64 = 0; loop { attempt += 1; - match run_single_session(&config).await { + let result = if let Some((session, bootstrap_result)) = prepared.take() { + run_prepared_session(&config, session, bootstrap_result).await + } else { + run_single_session(&config).await + }; + match result { Ok(()) => { let event = session_closed_event( openshell_ocsf::ctx::ctx(), @@ -416,11 +496,23 @@ async fn run_session_loop(config: SessionConfig) { async fn run_single_session( config: &SessionConfig, ) -> Result<(), Box> { - // Connect to the gateway. The same `Channel` is used for both the - // long-lived control stream and all data-plane `RelayStream` calls, so - // every relay rides the same TCP+TLS+HTTP/2 connection — no new TLS - // handshake per relay. - let channel = grpc_client::connect_channel_pub(&config.endpoint) + let prepared = open_session( + config.endpoint.clone(), + config.sandbox_id.clone(), + config.instance_id.clone(), + ) + .await?; + run_prepared_session(config, prepared, None).await +} + +async fn open_session( + endpoint: String, + sandbox_id: String, + instance_id: String, +) -> Result> { + // The same authenticated channel carries the long-lived control stream + // and all data-plane RelayStream calls. + let channel = grpc_client::connect_channel_pub(&endpoint) .await .map_err(|e| format!("connect failed: {e}"))?; let mut client = OpenShellClient::new(channel.clone()); @@ -432,8 +524,8 @@ async fn run_single_session( // Send hello as the first message. tx.send(SupervisorMessage { payload: Some(supervisor_message::Payload::Hello(SupervisorHello { - sandbox_id: config.sandbox_id.clone(), - instance_id: config.instance_id.clone(), + sandbox_id: sandbox_id.clone(), + instance_id: instance_id.clone(), protocol_revision: SUPERVISOR_PROTOCOL_REVISION, })), }) @@ -465,13 +557,45 @@ async fn run_single_session( validate_gateway_protocol_revision(accepted.protocol_revision)?; let event = session_established_event( openshell_ocsf::ctx::ctx(), - &config.endpoint, + &endpoint, &accepted.session_id, heartbeat_secs, ); ocsf_emit!(event); - if let Some(bootstrap) = accepted.bootstrap { + let protocol_revision = accepted.protocol_revision; + Ok(PreparedSupervisorSession { + endpoint, + sandbox_id, + instance_id, + channel, + tx, + inbound, + heartbeat_secs, + protocol_revision, + bootstrap: (protocol_revision == SUPERVISOR_PROTOCOL_REVISION) + .then_some(accepted.bootstrap) + .flatten(), + }) +} + +async fn run_prepared_session( + config: &SessionConfig, + mut prepared: PreparedSupervisorSession, + startup_result: Option, +) -> Result<(), Box> { + let heartbeat_secs = prepared.heartbeat_secs; + let channel = prepared.channel; + let tx = prepared.tx; + let mut inbound = prepared.inbound; + + if let Some(result) = startup_result { + tx.send(SupervisorMessage { + payload: Some(supervisor_message::Payload::ConfigBootstrapResult(result)), + }) + .await + .map_err(|_| "failed to queue configuration bootstrap result")?; + } else if let Some(bootstrap) = prepared.bootstrap.take() { let result = apply_bootstrap(config, bootstrap).await; tx.send(SupervisorMessage { payload: Some(supervisor_message::Payload::ConfigBootstrapResult(result)), diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 96e60b9c59..ee84603ff8 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -22,7 +22,7 @@ Gateway CLI flag > gateway OPENSHELL_* env var > TOML file > built-in defa ## Supervisor connections and policy recovery -Current supervisors require a complete configuration bootstrap before the gateway accepts the session as initialized. The gateway gives bootstrap construction the same bounded 45-second window as other snapshot builds and rejects the connection when construction fails or times out. Immediately previous supervisors retain the Stage 1 compatibility behavior: the gateway waits at most one second for an optional bootstrap, accepts the session without it when necessary, and relies on supervisor polling for configuration. +Current supervisors open `ConnectSupervisor` before initializing gateway-owned runtime state and require a complete configuration bootstrap before the gateway accepts the session as initialized. They apply later policy, settings, middleware, and provider snapshots from that stream without configuration fetch polling. If first-start image policy discovery or image-specific baseline enrichment changes policy, the supervisor commits that repair and reconnects before installing the fresh stream bootstrap; it does not initialize from the mutation response. The gateway gives bootstrap construction the same bounded 45-second window as other snapshot builds and rejects the connection when construction fails or times out. Immediately previous supervisors retain the Stage 1 compatibility behavior: the gateway waits at most one second for an optional bootstrap, accepts the session without it when necessary, and relies on supervisor polling for configuration. On startup, the gateway repairs missing legacy policy history and skips invalid stored policies. A valid global policy can still override an invalid local policy; otherwise, that sandbox's configuration reads continue to report the validation failure. diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 32580f2f12..15e30f93a4 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -21,6 +21,13 @@ The target deployment flow is: If supervisor sessions fail with a protocol revision mismatch, check that custom supervisor images match the gateway release. The gateway accepts its current internal protocol revision, the immediately previous revision through polling compatibility, and supervisors that predate the handshake for one release. Authentication success does not make other revisions compatible. The gateway logs compatibility sessions and counts them in `openshell_supervisor_protocol_previous_sessions_total` or `openshell_supervisor_protocol_legacy_sessions_total`; recreate those sandboxes before compatibility is removed. See the published [gateway configuration reference](https://docs.nvidia.com/openshell/latest/reference/gateway-config.md). +Current supervisors must receive their complete desired-state bootstrap before +gateway-owned policy, provider state, networking, or the workload is initialized. +If startup stalls or fails, inspect gateway snapshot-build and bootstrap-result +logs together with the supervisor session logs. Current supervisors do not use +settings or provider fetch polling; polling messages indicate a revision 1 or +revision 0 compatibility session. + The `openshell-gateway` composition crate explicitly installs its compiled Docker, Podman, Kubernetes, and VM registrations at startup; `openshell-server` does not link compute-driver crates. Custom gateway binaries may include a @@ -542,9 +549,10 @@ restart it with a fresh listener. If the process supervisor fails before launching the workload, inspect both containers for control-socket bind, connect, bootstrap, or update errors. If new SSH/exec sessions do not pick up refreshed provider environment, -inspect the network sidecar settings-poll logs and the process container logs -for provider environment update handling; the process container should consume -newer provider-env revisions without receiving gateway credentials. +inspect the network sidecar configuration-delivery logs and the process +container logs for provider environment update handling; the process container +should consume newer provider-env revisions without receiving gateway +credentials. The process container reports the workload entrypoint PID over the same control socket, and the network sidecar uses that PID for binary-scoped policy From 797c36ebafad2e8b7a4eb59b971e4d27aff6745f Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Thu, 10 Sep 2026 19:07:24 -0700 Subject: [PATCH 3/7] fix(server): update rebased schema inventory Signed-off-by: Piotr Mlocek --- architecture/gateway.md | 4 ++-- crates/openshell-server/src/storage_proto.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index 4615b72cf9..619c8696fc 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -328,8 +328,8 @@ public descriptor set generated by `openshell-core`; a fingerprint test in Compute-driver, credential-driver, gateway-interceptor, and supervisor-middleware services are compiled contracts for internal extension boundaries, not public gateway RPCs. The current public inventory has 74 -methods, 278 messages, and 12 enums -(`0f14943574349d02bdc61076c8c5a59a98b627325564ef1a6d21d7941825dc46`). +methods, 291 messages, and 15 enums +(`e173ad118e822557efcfc92ace7042bbc7bdd35e709c503abb123de378e9e7cb`). Storage-only messages live in the private, versioned `openshell.storage.v1` package under `crates/openshell-server/proto`. The server diff --git a/crates/openshell-server/src/storage_proto.rs b/crates/openshell-server/src/storage_proto.rs index f00972918b..f590f53599 100644 --- a/crates/openshell-server/src/storage_proto.rs +++ b/crates/openshell-server/src/storage_proto.rs @@ -161,7 +161,7 @@ mod tests { const STORAGE_V1_SCHEMA_SHA256: &str = "a623124c961f3a56af58a4ab148c12985e5efaec1ad23441031f5cd5b073fe22"; const PUBLIC_RPC_SCHEMA_SHA256: &str = - "0f14943574349d02bdc61076c8c5a59a98b627325564ef1a6d21d7941825dc46"; + "e173ad118e822557efcfc92ace7042bbc7bdd35e709c503abb123de378e9e7cb"; const DURABLE_SCHEMA_SHA256: &str = "60f2f912d45bb8bdc6bbf9a62a27b03aad2e65d8564b3f5832d19803a239ac79"; const PUBLIC_DURABLE_OVERLAP_SHA256: &str = @@ -531,7 +531,7 @@ mod tests { assert_eq!( (public_closure.messages.len(), public_closure.enums.len()), - (278, 12) + (291, 15) ); assert_eq!( (durable_closure.messages.len(), durable_closure.enums.len()), From 408f20f26210a0b1f50d32af78a738fb5621d071 Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Fri, 11 Sep 2026 12:28:02 -0700 Subject: [PATCH 4/7] perf(server): suppress bootstrap config redelivery Signed-off-by: Piotr Mlocek --- .../src/supervisor_session.rs | 277 ++++++++++++++++-- 1 file changed, 253 insertions(+), 24 deletions(-) diff --git a/crates/openshell-server/src/supervisor_session.rs b/crates/openshell-server/src/supervisor_session.rs index 808a2333ad..da3c93248c 100644 --- a/crates/openshell-server/src/supervisor_session.rs +++ b/crates/openshell-server/src/supervisor_session.rs @@ -15,6 +15,8 @@ use tonic::{Request, Response, Status}; use tracing::{debug, info, warn}; use uuid::Uuid; +#[cfg(test)] +use openshell_core::proto::ConfigBootstrapResult; use openshell_core::proto::{ ConfigApplyOutcome, ConfigBootstrap, ConfigComponent, ConfigComponentApplyResult, ConfigSnapshotRevision, ConfigUpdate, ConfigUpdateResult, GatewayMessage, PolicySource, @@ -250,6 +252,16 @@ fn validate_component_apply_result( Ok(outcome) } +fn outcome_acknowledges_revision(outcome: ConfigApplyOutcome) -> bool { + matches!( + outcome, + ConfigApplyOutcome::Applied + | ConfigApplyOutcome::IgnoredDuplicate + | ConfigApplyOutcome::RetainedLocalOverride + | ConfigApplyOutcome::Degraded + ) +} + impl SupervisorSessionRegistry { pub fn new() -> Self { Self::default() @@ -487,13 +499,7 @@ impl SupervisorSessionRegistry { )); } let outcome = validate_component_apply_result(component_result, &in_flight.revision)?; - if matches!( - outcome, - ConfigApplyOutcome::Applied - | ConfigApplyOutcome::IgnoredDuplicate - | ConfigApplyOutcome::RetainedLocalOverride - | ConfigApplyOutcome::Degraded - ) { + if outcome_acknowledges_revision(outcome) { delivery_state.last_acknowledged_revision = Some(in_flight.revision); } delivery_state.in_flight = None; @@ -514,6 +520,48 @@ impl SupervisorSessionRegistry { Ok(()) } + /// Record a successfully persisted bootstrap result in the live session's + /// delivery state so reconciliation does not immediately redeliver it. + /// + /// A streamed update may be delivered while the bootstrap result is being + /// persisted. In that case, or if this session has already acknowledged a + /// revision, leave the newer delivery state untouched. + fn acknowledge_bootstrap_component( + &self, + sandbox_id: &str, + session_id: &str, + result: &ConfigComponentApplyResult, + ) -> bool { + let component = ConfigComponent::try_from(result.component).unwrap_or_default(); + let outcome = ConfigApplyOutcome::try_from(result.outcome).unwrap_or_default(); + if !outcome_acknowledges_revision(outcome) { + return false; + } + let Some(revision) = result.requested_revision.as_ref() else { + return false; + }; + let mut sessions = self.sessions.lock().unwrap(); + let Some(session) = sessions + .get_mut(sandbox_id) + .filter(|session| session.session_id == session_id) + else { + return false; + }; + let delivery_state = match component { + ConfigComponent::SandboxConfig => &mut session.config_sequences.sandbox_config, + ConfigComponent::ProviderEnvironment => { + &mut session.config_sequences.provider_environment + } + ConfigComponent::Unspecified => return false, + }; + if delivery_state.in_flight.is_some() || delivery_state.last_acknowledged_revision.is_some() + { + return false; + } + delivery_state.last_acknowledged_revision = Some(*revision); + true + } + fn retry_config_update_after_persistence_failure( &self, sandbox_id: &str, @@ -1500,16 +1548,21 @@ async fn handle_supervisor_message( return; } for component in &result.results { - if let Err(error) = - record_component_apply_result(state, sandbox_id, component).await - { - warn!( - sandbox_id, - session_id, - component = component.component, - error = %error, - "failed to persist supervisor bootstrap result" - ); + match record_component_apply_result(state, sandbox_id, component).await { + Ok(()) => { + state + .supervisor_sessions + .acknowledge_bootstrap_component(sandbox_id, session_id, component); + } + Err(error) => { + warn!( + sandbox_id, + session_id, + component = component.component, + error = %error, + "failed to persist supervisor bootstrap result" + ); + } } } } @@ -1583,13 +1636,7 @@ fn validate_bootstrap_result( }; let outcome = validate_component_apply_result(result, revision)?; seen.push(component); - all_succeeded &= matches!( - outcome, - ConfigApplyOutcome::Applied - | ConfigApplyOutcome::IgnoredDuplicate - | ConfigApplyOutcome::RetainedLocalOverride - | ConfigApplyOutcome::Degraded - ); + all_succeeded &= outcome_acknowledges_revision(outcome); } Ok(all_succeeded) } @@ -1891,6 +1938,66 @@ mod tests { state } + #[tokio::test] + async fn persisted_bootstrap_result_suppresses_unchanged_reconciliation() { + let state = state_with_sandbox("sb-bootstrap-ack").await; + let (tx, mut rx) = mpsc::channel(1); + let (shutdown_tx, _shutdown_rx) = oneshot::channel(); + state.supervisor_sessions.register( + "sb-bootstrap-ack".into(), + "session-1".into(), + tx, + shutdown_tx, + ); + let snapshot = ProviderEnvironmentSnapshot { + provider_env_revision: 11, + ..Default::default() + }; + let revision = config_message_revision(&SupervisorConfigMessage::ProviderEnvironment( + snapshot.clone(), + )); + let result = ConfigComponentApplyResult { + component: ConfigComponent::ProviderEnvironment.into(), + requested_revision: Some(revision), + applied_revision: Some(revision), + outcome: ConfigApplyOutcome::Applied.into(), + ..Default::default() + }; + + handle_supervisor_message( + &state, + "sb-bootstrap-ack", + "session-1", + true, + SupervisorMessage { + payload: Some(supervisor_message::Payload::ConfigBootstrapResult( + ConfigBootstrapResult { + results: vec![result], + }, + )), + }, + ) + .await; + + let observation = state + .store + .get_message::( + "sb-bootstrap-ack:provider_environment", + ) + .await + .unwrap() + .expect("bootstrap observation"); + assert_eq!(observation.requested_revision, Some(revision)); + assert_eq!( + state.supervisor_sessions.deliver_config( + "sb-bootstrap-ack", + SupervisorConfigMessage::ProviderEnvironment(snapshot), + ), + DeliveryDisposition::SuppressedUnchanged + ); + assert!(rx.try_recv().is_err()); + } + #[tokio::test] async fn component_apply_result_persists_compact_observed_state() { let state = state_with_sandbox("sb-observed").await; @@ -2041,6 +2148,128 @@ mod tests { ); } + #[test] + fn bootstrap_acknowledgement_suppresses_unchanged_reconciliation() { + let registry = SupervisorSessionRegistry::new(); + let (tx, mut rx) = mpsc::channel(1); + let (shutdown_tx, _shutdown_rx) = oneshot::channel(); + registry.register("sb-1".into(), "session-1".into(), tx, shutdown_tx); + let snapshot = SandboxConfigSnapshot { + config_revision: 7, + version: 11, + ..Default::default() + }; + let revision = config_message_revision(&SupervisorConfigMessage::SandboxConfig(Box::new( + snapshot.clone(), + ))); + + assert!(!registry.acknowledge_bootstrap_component( + "sb-1", + "session-1", + &ConfigComponentApplyResult { + component: ConfigComponent::SandboxConfig.into(), + requested_revision: Some(revision), + applied_revision: None, + outcome: ConfigApplyOutcome::FailedClosed.into(), + ..Default::default() + }, + )); + assert!(registry.acknowledge_bootstrap_component( + "sb-1", + "session-1", + &ConfigComponentApplyResult { + component: ConfigComponent::SandboxConfig.into(), + requested_revision: Some(revision), + applied_revision: Some(revision), + outcome: ConfigApplyOutcome::Applied.into(), + ..Default::default() + }, + )); + assert_eq!( + registry.deliver_config( + "sb-1", + SupervisorConfigMessage::SandboxConfig(Box::new(snapshot)), + ), + DeliveryDisposition::SuppressedUnchanged + ); + assert!(rx.try_recv().is_err()); + } + + #[test] + fn bootstrap_acknowledgement_does_not_replace_newer_delivery_state() { + let registry = SupervisorSessionRegistry::new(); + let (tx, mut rx) = mpsc::channel(1); + let (shutdown_tx, _shutdown_rx) = oneshot::channel(); + registry.register("sb-1".into(), "session-1".into(), tx, shutdown_tx); + let bootstrap_revision = ConfigSnapshotRevision { + component: Some(config_snapshot_revision::Component::ProviderEnvironment(7)), + }; + + assert_eq!( + registry.deliver_config( + "sb-1", + SupervisorConfigMessage::ProviderEnvironment(ProviderEnvironmentSnapshot { + provider_env_revision: 8, + ..Default::default() + }), + ), + DeliveryDisposition::Enqueued + ); + assert!(!registry.acknowledge_bootstrap_component( + "sb-1", + "session-1", + &ConfigComponentApplyResult { + component: ConfigComponent::ProviderEnvironment.into(), + requested_revision: Some(bootstrap_revision), + applied_revision: Some(bootstrap_revision), + outcome: ConfigApplyOutcome::Applied.into(), + ..Default::default() + }, + )); + + let message = rx.try_recv().expect("newer update"); + let Some(gateway_message::Payload::ConfigUpdate(update)) = message.payload else { + panic!("expected config update"); + }; + assert_eq!(update.component_sequence, 1); + let in_flight_revision = registry + .sessions + .lock() + .unwrap() + .get("sb-1") + .unwrap() + .config_sequences + .provider_environment + .in_flight + .as_ref() + .unwrap() + .revision; + assert_eq!( + in_flight_revision.component, + Some(config_snapshot_revision::Component::ProviderEnvironment(8)) + ); + + let (replacement_tx, _replacement_rx) = mpsc::channel(1); + let (replacement_shutdown_tx, _replacement_shutdown_rx) = oneshot::channel(); + registry.register( + "sb-1".into(), + "session-2".into(), + replacement_tx, + replacement_shutdown_tx, + ); + assert!(!registry.acknowledge_bootstrap_component( + "sb-1", + "session-1", + &ConfigComponentApplyResult { + component: ConfigComponent::ProviderEnvironment.into(), + requested_revision: Some(bootstrap_revision), + applied_revision: Some(bootstrap_revision), + outcome: ConfigApplyOutcome::Applied.into(), + ..Default::default() + }, + )); + } + #[tokio::test] async fn config_router_assigns_sequences_per_component() { let registry = Arc::new(SupervisorSessionRegistry::new()); From 1903910ac7b97747e1134e59289e8a2821815c59 Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Thu, 10 Sep 2026 19:56:03 -0700 Subject: [PATCH 5/7] fix(sandbox): preserve streamed policy outcomes Signed-off-by: Piotr Mlocek --- crates/openshell-sandbox/src/lib.rs | 184 ++++++++++++++++++++++++---- 1 file changed, 163 insertions(+), 21 deletions(-) diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index ca1209a00f..5ac5fa4dd2 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -4325,7 +4325,7 @@ async fn apply_stream_sandbox_snapshot( use std::sync::atomic::Ordering; let requested_revision = sandbox_config_revision(&snapshot); - if snapshot.config_revision == *current_config_revision { + if reloads_gateway_policy && snapshot.config_revision == *current_config_revision { return config_apply_result( ConfigComponent::SandboxConfig, requested_revision, @@ -4396,24 +4396,60 @@ async fn apply_stream_sandbox_snapshot( } Err(failure) => { let failure_mode = snapshot.policy_validation_failure_mode; - let error = match apply_gateway_runtime_reload_failure( + let (outcome, error) = match apply_gateway_runtime_reload_failure( &ctx.opa_engine, failure, failure_mode, *has_last_valid_policy, snapshot.version, ) { - Ok( - GatewayRuntimeFailureDisposition::PolicyRejected { error, .. } - | GatewayRuntimeFailureDisposition::MiddlewareUnavailable { error } - | GatewayRuntimeFailureDisposition::TransparentTcpExpansionRejected { - error, - .. - }, - ) => error, - Err(error) => error.to_string(), + Ok(GatewayRuntimeFailureDisposition::PolicyRejected { + error, + disposition, + }) => { + emit_policy_validation_failure( + &disposition, + snapshot.version, + &snapshot.policy_hash, + &error, + ); + let outcome = if disposition.previous_policy_active { + ConfigApplyOutcome::FailedRetainedLastKnownGood + } else { + ConfigApplyOutcome::FailedClosed + }; + (outcome, error) + } + Ok(GatewayRuntimeFailureDisposition::MiddlewareUnavailable { error }) => { + ocsf_emit!(ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Medium) + .status(StatusId::Failure) + .state(StateId::Other, "failed") + .unmapped("version", serde_json::json!(snapshot.version)) + .unmapped("error", serde_json::json!(&error)) + .unmapped("previous_policy_active", serde_json::json!(true)) + .message(format!( + "Supervisor middleware registry unavailable, keeping last-known-good policy runtime active [version:{} error:{error}]", + snapshot.version + )) + .build()); + (ConfigApplyOutcome::FailedRetainedLastKnownGood, error) + } + Ok(GatewayRuntimeFailureDisposition::TransparentTcpExpansionRejected { + error, + active_generation, + }) => { + emit_transparent_tcp_expansion_rejection( + snapshot.version, + &snapshot.policy_hash, + active_generation, + &error, + ); + (ConfigApplyOutcome::FailedRetainedLastKnownGood, error) + } + Err(error) => (ConfigApplyOutcome::FailedClosed, error.to_string()), }; - Err(error) + Err((outcome, error)) } } } else { @@ -4471,15 +4507,7 @@ async fn apply_stream_sandbox_snapshot( None, ) } - Err(error) => { - let outcome = if snapshot.policy_validation_failure_mode - == PolicyValidationFailureMode::RetainLastValid - && *has_last_valid_policy - { - ConfigApplyOutcome::FailedRetainedLastKnownGood - } else { - ConfigApplyOutcome::FailedClosed - }; + Err((outcome, error)) => { let applied_revision = (outcome == ConfigApplyOutcome::FailedRetainedLastKnownGood) .then_some(*current_stream_revision) .flatten(); @@ -5967,6 +5995,120 @@ network_policies: } } + #[tokio::test] + async fn streamed_tcp_expansion_reports_retained_active_policy() { + use openshell_core::proto::{ConfigApplyOutcome, PolicySource}; + + let engine = Arc::new( + OpaEngine::from_proto(&proto_policy_fixture()).expect("build initial OPA engine"), + ); + let mut ctx = policy_poll_test_context( + Arc::clone(&engine), + LoadedPolicyOrigin::Gateway { + revision: None, + has_last_valid_policy: true, + }, + default_middleware_connector(), + ); + ctx.transparent_tcp = TransparentTcpReloadState { + capable: true, + substrate_ready: false, + }; + let initial = settings_poll_result(Some(proto_policy_fixture()), 1, PolicySource::Sandbox); + let mut candidate = + settings_poll_result(Some(proto_tcp_policy_fixture()), 2, PolicySource::Sandbox); + candidate.policy_validation_failure_mode = PolicyValidationFailureMode::FailClosed; + let (client, _polls, _reports) = scripted_policy_gateway(); + let initial_revision = sandbox_config_revision(&initial); + let initial_generation = engine.current_generation(); + let result = apply_stream_sandbox_snapshot( + &ctx, + &client, + candidate, + &mut initial.config_revision.clone(), + &mut Some(initial_revision), + &mut initial.version.clone(), + &mut initial.policy_hash.clone(), + &mut Vec::new(), + &mut false, + &mut MiddlewareRegistryStatus::Synchronized, + &mut std::collections::HashMap::new(), + true, + &mut true, + ) + .await; + + assert_eq!(engine.current_generation(), initial_generation); + assert!(engine.fail_closed_reason().is_none()); + assert_eq!( + ConfigApplyOutcome::try_from(result.outcome).unwrap(), + ConfigApplyOutcome::FailedRetainedLastKnownGood + ); + assert_eq!(result.applied_revision, Some(initial_revision)); + assert!( + result + .failure + .as_ref() + .unwrap() + .message + .contains("without the transparent TCP substrate") + ); + } + + #[tokio::test] + async fn duplicate_stream_snapshot_preserves_local_policy_override() { + use openshell_core::proto::{ConfigApplyOutcome, PolicySource}; + + let engine = Arc::new( + OpaEngine::from_proto(&proto_policy_fixture()).expect("build local OPA engine"), + ); + let ctx = policy_poll_test_context( + Arc::clone(&engine), + LoadedPolicyOrigin::LocalOverride, + default_middleware_connector(), + ); + let (client, _polls, _reports) = scripted_policy_gateway(); + let mut current_config_revision = 0; + let mut current_revision = None; + let mut version = 0; + let mut hash = String::new(); + let mut services = Vec::new(); + let mut auth_enabled = false; + let mut registry_status = MiddlewareRegistryStatus::Synchronized; + let mut settings = std::collections::HashMap::new(); + let mut has_last_valid_policy = true; + let initial_generation = engine.current_generation(); + + for _ in 0..2 { + let candidate = + settings_poll_result(Some(proto_tcp_policy_fixture()), 2, PolicySource::Sandbox); + let result = apply_stream_sandbox_snapshot( + &ctx, + &client, + candidate, + &mut current_config_revision, + &mut current_revision, + &mut version, + &mut hash, + &mut services, + &mut auth_enabled, + &mut registry_status, + &mut settings, + false, + &mut has_last_valid_policy, + ) + .await; + + assert_eq!(engine.current_generation(), initial_generation); + assert_eq!( + ConfigApplyOutcome::try_from(result.outcome).unwrap(), + ConfigApplyOutcome::RetainedLocalOverride + ); + assert!(result.applied_revision.is_none()); + assert!(current_revision.is_none()); + } + } + #[tokio::test] async fn stream_provider_snapshot_applies_without_fetching() { let engine = From 12d2dc49432c59eede4b3fddedee2fe060fdeaf2 Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Thu, 10 Sep 2026 21:04:29 -0700 Subject: [PATCH 6/7] perf(server): increase configuration delivery admission capacity Signed-off-by: Piotr Mlocek --- architecture/gateway.md | 5 ++++- .../openshell-server/src/config_delivery.rs | 21 +++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index f541397b86..f08bab7e3c 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -680,7 +680,10 @@ refresh bootstrap material; sandboxes receive minted access tokens instead. Committed configuration mutations publish component and scope identifiers, never configuration payloads, to a bounded coalescing scheduler. The scheduler admits a fixed number of delivery workers and builds the latest full snapshot -for each affected active sandbox. Fleet fanout waits for worker capacity before +for each affected active sandbox. Delivery admission allows at least 64 workers, +while snapshot build concurrency remains tied to database pool capacity. This +absorbs scoped bursts without increasing concurrent database-backed builds. +Fleet fanout waits for worker capacity before admitting another recipient. An async router owns session lookup, message sizing, sequence allocation, and enqueue. Its local implementation uses the process-local supervisor registry. A future HA implementation can resolve the diff --git a/crates/openshell-server/src/config_delivery.rs b/crates/openshell-server/src/config_delivery.rs index 53ab1d4c34..6cf27cfe2c 100644 --- a/crates/openshell-server/src/config_delivery.rs +++ b/crates/openshell-server/src/config_delivery.rs @@ -38,6 +38,9 @@ const MAX_ACTIVE_FANOUT_WORKERS: usize = 64; /// pool busy without stacking every waiter on the acquire timeout. const SNAPSHOT_BUILDS_PER_DB_CONNECTION: usize = 2; const MIN_CONCURRENT_SNAPSHOT_BUILDS: usize = 4; +/// Admit scoped bursts independently of the database build bound. Workers +/// waiting to build still count toward this limit. +const MIN_CONCURRENT_DELIVERY_WORKERS: usize = 64; /// One complete configuration component awaiting delivery to a supervisor. #[derive(Clone)] @@ -210,11 +213,15 @@ impl Default for ConfigDeliveryQueue { impl ConfigDeliveryQueue { #[must_use] pub fn new(max_concurrent_builds: usize) -> Self { + Self::with_limits(max_concurrent_builds, max_concurrent_builds) + } + + fn with_limits(max_concurrent_builds: usize, max_delivery_workers: usize) -> Self { let max_concurrent_builds = max_concurrent_builds.max(1); Self { pending: Mutex::default(), fanout_pending: Mutex::default(), - delivery_permits: Arc::new(Semaphore::new(max_concurrent_builds)), + delivery_permits: Arc::new(Semaphore::new(max_delivery_workers.max(1))), build_permits: Semaphore::new(max_concurrent_builds), } } @@ -223,11 +230,10 @@ impl ConfigDeliveryQueue { #[must_use] pub fn for_db_connections(max_connections: u32) -> Self { let max_connections = usize::try_from(max_connections).unwrap_or(usize::MAX); - Self::new( - max_connections - .saturating_mul(SNAPSHOT_BUILDS_PER_DB_CONNECTION) - .max(MIN_CONCURRENT_SNAPSHOT_BUILDS), - ) + let builds = max_connections + .saturating_mul(SNAPSHOT_BUILDS_PER_DB_CONNECTION) + .max(MIN_CONCURRENT_SNAPSHOT_BUILDS); + Self::with_limits(builds, builds.max(MIN_CONCURRENT_DELIVERY_WORKERS)) } #[cfg(test)] @@ -672,6 +678,9 @@ mod tests { #[test] fn build_bound_is_sized_from_the_database_pool() { + let local = ConfigDeliveryQueue::for_db_connections(5); + assert_eq!(local.max_concurrent_builds(), 10); + assert_eq!(local.delivery_permits.available_permits(), 64); assert_eq!( ConfigDeliveryQueue::for_db_connections(10).max_concurrent_builds(), 20 From 13f7f06b3ea1845ce07f288d9bec8401f83f2307 Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Thu, 10 Sep 2026 21:17:25 -0700 Subject: [PATCH 7/7] fix(server): retain pending supervisor configuration deliveries Signed-off-by: Piotr Mlocek --- architecture/gateway.md | 10 +- .../openshell-server/src/config_delivery.rs | 529 +++++++++++++++--- 2 files changed, 461 insertions(+), 78 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index f08bab7e3c..c92c9d9780 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -683,8 +683,14 @@ admits a fixed number of delivery workers and builds the latest full snapshot for each affected active sandbox. Delivery admission allows at least 64 workers, while snapshot build concurrency remains tied to database pool capacity. This absorbs scoped bursts without increasing concurrent database-backed builds. -Fleet fanout waits for worker capacity before -admitting another recipient. An async router owns session lookup, message +Up to 1024 running or queued component keys retain pending work without storing +configuration payloads. A FIFO dispatcher builds current state when capacity +is available. Repeated mutations coalesce; mutations during delivery return the +key to the tail for another pass. Fanout waits for pending capacity through fair +admission, while direct overflow requests one coalesced all-connected repair +pass. Two reserved fanout scopes keep that repair available when workspace +fanout is full. Periodic reconciliation remains the fallback for build, route, +or session failures. An async router owns session lookup, message sizing, sequence allocation, and enqueue. Its local implementation uses the process-local supervisor registry. A future HA implementation can resolve the gateway that owns a session and forward the same typed message without changing diff --git a/crates/openshell-server/src/config_delivery.rs b/crates/openshell-server/src/config_delivery.rs index 6cf27cfe2c..f38e2dd6b3 100644 --- a/crates/openshell-server/src/config_delivery.rs +++ b/crates/openshell-server/src/config_delivery.rs @@ -3,8 +3,7 @@ //! Build and route complete supervisor configuration snapshots. -use std::collections::HashMap; -use std::collections::hash_map::Entry; +use std::collections::{HashMap, VecDeque}; use std::fmt; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -13,7 +12,8 @@ use metrics::counter; use openshell_core::proto::{ ConfigBootstrap, ProviderEnvironmentSnapshot, Sandbox, SandboxConfigSnapshot, }; -use tokio::sync::{OwnedSemaphorePermit, Semaphore}; +use tokio::sync::{Notify, OwnedSemaphorePermit, Semaphore}; +use tokio::task::JoinSet; use tonic::{Code, Status}; use tracing::warn; @@ -41,6 +41,8 @@ const MIN_CONCURRENT_SNAPSHOT_BUILDS: usize = 4; /// Admit scoped bursts independently of the database build bound. Workers /// waiting to build still count toward this limit. const MIN_CONCURRENT_DELIVERY_WORKERS: usize = 64; +/// Includes running and queued component keys; payloads are built on dispatch. +const MAX_PENDING_DELIVERIES: usize = 1024; /// One complete configuration component awaiting delivery to a supervisor. #[derive(Clone)] @@ -188,22 +190,32 @@ struct FanoutKey { /// Coalesces publications and bounds workers per sandbox and component. /// -/// The map entry is also the worker lease. Its boolean is set when another -/// mutation arrives during a build or route operation. The worker then rebuilds -/// the current full snapshot once, regardless of how many mutations arrived. -/// -/// Each pending map entry owns a delivery permit for its worker. Direct -/// publications fail fast when all permits are held. Fanout workers wait for a -/// permit before admitting the next recipient, which bounds both spawned tasks -/// and pending keys while still walking the full recipient list. +/// Queued keys do not own tasks or build permits. Repeated publications mark +/// the existing key dirty. A mutation during delivery schedules another pass +/// at the FIFO tail so a busy sandbox cannot monopolize a worker. #[derive(Debug)] pub struct ConfigDeliveryQueue { - pending: Mutex>, + pending: Mutex, fanout_pending: Mutex>, - delivery_permits: Arc, + max_delivery_workers: usize, + pending_slots: Arc, + ready: Notify, build_permits: Semaphore, } +#[derive(Debug, Default)] +struct PendingDeliveries { + changed: HashMap, + ready: VecDeque, + dispatcher_running: bool, +} + +#[derive(Debug)] +struct PendingDelivery { + changed: bool, + _slot: OwnedSemaphorePermit, +} + impl Default for ConfigDeliveryQueue { fn default() -> Self { Self::new(MIN_CONCURRENT_SNAPSHOT_BUILDS) @@ -217,11 +229,25 @@ impl ConfigDeliveryQueue { } fn with_limits(max_concurrent_builds: usize, max_delivery_workers: usize) -> Self { + Self::with_pending_capacity( + max_concurrent_builds, + max_delivery_workers, + MAX_PENDING_DELIVERIES, + ) + } + + fn with_pending_capacity( + max_concurrent_builds: usize, + max_delivery_workers: usize, + max_pending: usize, + ) -> Self { let max_concurrent_builds = max_concurrent_builds.max(1); Self { pending: Mutex::default(), fanout_pending: Mutex::default(), - delivery_permits: Arc::new(Semaphore::new(max_delivery_workers.max(1))), + max_delivery_workers: max_delivery_workers.max(1), + pending_slots: Arc::new(Semaphore::new(max_pending.max(1))), + ready: Notify::new(), build_permits: Semaphore::new(max_concurrent_builds), } } @@ -258,61 +284,85 @@ impl ConfigDeliveryQueue { fn enqueue(&self, key: DeliveryKey) -> DeliveryEnqueue { let mut pending = self.pending.lock().unwrap(); - match pending.entry(key) { - Entry::Occupied(mut entry) => { - *entry.get_mut() = true; - DeliveryEnqueue::Coalesced - } - Entry::Vacant(entry) => { - let Ok(permit) = Arc::clone(&self.delivery_permits).try_acquire_owned() else { - return DeliveryEnqueue::Full; - }; - entry.insert(true); - DeliveryEnqueue::StartWorker(permit) - } + if let Some(entry) = pending.changed.get_mut(&key) { + entry.changed = true; + return DeliveryEnqueue::Coalesced; + } + let Ok(slot) = Arc::clone(&self.pending_slots).try_acquire_owned() else { + return DeliveryEnqueue::Full; + }; + self.admit(&mut pending, key, slot) + } + + fn admit( + &self, + pending: &mut PendingDeliveries, + key: DeliveryKey, + slot: OwnedSemaphorePermit, + ) -> DeliveryEnqueue { + pending.changed.insert( + key.clone(), + PendingDelivery { + changed: true, + _slot: slot, + }, + ); + pending.ready.push_back(key); + self.ready.notify_one(); + if pending.dispatcher_running { + DeliveryEnqueue::Queued + } else { + pending.dispatcher_running = true; + DeliveryEnqueue::StartDispatcher } } async fn enqueue_from_fanout(&self, key: DeliveryKey) -> DeliveryEnqueue { { let mut pending = self.pending.lock().unwrap(); - if let Entry::Occupied(mut entry) = pending.entry(key.clone()) { - *entry.get_mut() = true; + if let Some(entry) = pending.changed.get_mut(&key) { + entry.changed = true; return DeliveryEnqueue::Coalesced; } } - - let permit = Arc::clone(&self.delivery_permits) + // The fair semaphore reserves released slots for waiting fanouts, so + // a stream of new direct publications cannot repeatedly bypass repair. + let slot = Arc::clone(&self.pending_slots) .acquire_owned() .await - .expect("delivery worker semaphore is never closed"); + .expect("pending delivery semaphore is never closed"); let mut pending = self.pending.lock().unwrap(); - match pending.entry(key) { - Entry::Occupied(mut entry) => { - *entry.get_mut() = true; - DeliveryEnqueue::Coalesced - } - Entry::Vacant(entry) => { - entry.insert(true); - DeliveryEnqueue::StartWorker(permit) - } + if let Some(entry) = pending.changed.get_mut(&key) { + entry.changed = true; + DeliveryEnqueue::Coalesced + } else { + self.admit(&mut pending, key, slot) } } - fn take(&self, key: &DeliveryKey) { + fn take_ready(&self) -> Option { let mut pending = self.pending.lock().unwrap(); - if let Some(changed) = pending.get_mut(key) { - *changed = false; + let key = pending.ready.pop_front()?; + pending.changed.get_mut(&key).unwrap().changed = false; + Some(key) + } + + fn finish_pass(&self, key: &DeliveryKey) { + let mut pending = self.pending.lock().unwrap(); + if pending.changed.get(key).is_some_and(|entry| entry.changed) { + pending.ready.push_back(key.clone()); + } else { + pending.changed.remove(key); } } - fn finish_pass(&self, key: &DeliveryKey) -> bool { + fn stop_if_idle(&self) -> bool { let mut pending = self.pending.lock().unwrap(); - if pending.get(key).is_some_and(|changed| !changed) { - pending.remove(key); - false + if pending.ready.is_empty() { + pending.dispatcher_running = false; + true } else { - pending.contains_key(key) + false } } @@ -322,7 +372,10 @@ impl ConfigDeliveryQueue { *changed = true; return FanoutEnqueue::Coalesced; } - if pending.len() >= MAX_ACTIVE_FANOUT_WORKERS { + // Reserve the two all-connected component scopes for overflow repair, + // even when workspace fanout admission is full. + let reserved_repair = matches!(key.scope, FanoutScope::AllConnected); + if !reserved_repair && pending.len() >= MAX_ACTIVE_FANOUT_WORKERS { FanoutEnqueue::Full } else { pending.insert(key, true); @@ -350,7 +403,8 @@ impl ConfigDeliveryQueue { #[derive(Debug)] enum DeliveryEnqueue { - StartWorker(OwnedSemaphorePermit), + StartDispatcher, + Queued, Coalesced, Full, } @@ -424,27 +478,71 @@ fn enqueue_sandbox(state: &Arc, sandbox_id: &str, components: Confi component, }; match state.config_delivery_queue.enqueue(key.clone()) { - DeliveryEnqueue::StartWorker(permit) => { - spawn_delivery_worker(state, key, permit); + DeliveryEnqueue::StartDispatcher => { + spawn_delivery_dispatcher(state); } - DeliveryEnqueue::Coalesced => {} + DeliveryEnqueue::Coalesced | DeliveryEnqueue::Queued => {} DeliveryEnqueue::Full => { record_delivery_worker_full(sandbox_id, component.name()); + // Retain the recovery obligation as one coalesced fleet pass, + // rather than a task or retry timer for every rejected key. + enqueue_fanout( + state, + FanoutScope::AllConnected, + ConfigComponents { + sandbox_config: component == ConfigComponentKind::SandboxConfig, + provider_environment: component == ConfigComponentKind::ProviderEnvironment, + }, + ); } } } } -fn spawn_delivery_worker(state: &Arc, key: DeliveryKey, permit: OwnedSemaphorePermit) { +fn spawn_delivery_dispatcher(state: &Arc) { let state = Arc::clone(state); tokio::spawn(async move { - let _permit = permit; + let queue = &state.config_delivery_queue; + let mut workers = JoinSet::new(); + let mut in_flight = HashMap::new(); loop { - state.config_delivery_queue.take(&key); - publish_sandbox_component_now(&state, &key).await; - if !state.config_delivery_queue.finish_pass(&key) { + while workers.len() < queue.max_delivery_workers { + let Some(key) = queue.take_ready() else { + break; + }; + let worker_state = Arc::clone(&state); + let worker_key = key.clone(); + let handle = workers.spawn(async move { + publish_sandbox_component_now(&worker_state, &worker_key).await; + }); + in_flight.insert(handle.id(), key); + } + // Changing the running flag under the admission lock prevents a + // publication racing dispatcher exit from losing its wakeup. + if workers.is_empty() && queue.stop_if_idle() { break; } + tokio::select! { + completed = workers.join_next_with_id(), if !workers.is_empty() => { + let id = match completed.expect("nonempty delivery workers") { + Ok((id, ())) => id, + Err(error) => { + // Panic payloads may contain credential backend data. + warn!( + cancelled = error.is_cancelled(), + panicked = error.is_panic(), + "supervisor configuration delivery worker failed" + ); + error.id() + } + }; + let key = in_flight.remove(&id).expect("delivery task has a key"); + // Failed builds/routes rely on reconciliation as before. + // A concurrent mutation still gets its own subsequent pass. + queue.finish_pass(&key); + } + () = queue.ready.notified() => {} + } } }); } @@ -463,8 +561,8 @@ async fn enqueue_sandbox_from_fanout( .enqueue_from_fanout(key.clone()) .await { - DeliveryEnqueue::StartWorker(permit) => spawn_delivery_worker(state, key, permit), - DeliveryEnqueue::Coalesced => {} + DeliveryEnqueue::StartDispatcher => spawn_delivery_dispatcher(state), + DeliveryEnqueue::Coalesced | DeliveryEnqueue::Queued => {} DeliveryEnqueue::Full => unreachable!("fanout waits for delivery worker capacity"), } } @@ -589,7 +687,7 @@ fn record_delivery_worker_full(sandbox_id: &str, component: &'static str) { .increment(1); warn!( sandbox_id, - component, "supervisor configuration delivery worker queue is full" + component, "supervisor configuration pending queue is full; scheduling reconciliation" ); } @@ -658,10 +756,10 @@ mod tests { fn queue_coalesces_repeated_component_changes_while_worker_is_active() { let queue = ConfigDeliveryQueue::default(); let key = key("sb-1", ConfigComponentKind::SandboxConfig); - let DeliveryEnqueue::StartWorker(permit) = queue.enqueue(key.clone()) else { - panic!("first publication must start a worker"); + let DeliveryEnqueue::StartDispatcher = queue.enqueue(key.clone()) else { + panic!("first publication must start the dispatcher"); }; - queue.take(&key); + assert_eq!(queue.take_ready(), Some(key.clone())); assert!(matches!( queue.enqueue(key.clone()), DeliveryEnqueue::Coalesced @@ -670,17 +768,281 @@ mod tests { queue.enqueue(key.clone()), DeliveryEnqueue::Coalesced )); - assert!(queue.finish_pass(&key)); - queue.take(&key); - assert!(!queue.finish_pass(&key)); - drop(permit); + queue.finish_pass(&key); + assert_eq!(queue.take_ready(), Some(key.clone())); + queue.finish_pass(&key); + assert!(queue.take_ready().is_none()); + assert!(queue.stop_if_idle()); + } + + #[test] + fn pending_work_is_bounded_coalesced_and_fair_to_other_keys() { + let queue = ConfigDeliveryQueue::with_pending_capacity(1, 1, 3); + let a = key("a", ConfigComponentKind::SandboxConfig); + let b = key("b", ConfigComponentKind::SandboxConfig); + let c = key("c", ConfigComponentKind::SandboxConfig); + assert!(matches!( + queue.enqueue(a.clone()), + DeliveryEnqueue::StartDispatcher + )); + assert_eq!(queue.take_ready(), Some(a.clone())); + assert!(matches!(queue.enqueue(b.clone()), DeliveryEnqueue::Queued)); + assert!(matches!(queue.enqueue(c.clone()), DeliveryEnqueue::Queued)); + for _ in 0..100 { + assert!(matches!( + queue.enqueue(a.clone()), + DeliveryEnqueue::Coalesced + )); + assert!(matches!( + queue.enqueue(b.clone()), + DeliveryEnqueue::Coalesced + )); + } + for i in 0..10_000 { + assert!(matches!( + queue.enqueue(key( + &format!("overflow-{i}"), + ConfigComponentKind::SandboxConfig + )), + DeliveryEnqueue::Full + )); + } + assert_eq!(queue.pending.lock().unwrap().changed.len(), 3); + queue.finish_pass(&a); + // The running key's new state goes behind both previously queued keys. + for expected in [b, c, a] { + assert_eq!(queue.take_ready(), Some(expected.clone())); + queue.finish_pass(&expected); + } + assert!(queue.take_ready().is_none()); + assert_eq!(queue.pending_slots.available_permits(), 3); + assert!(queue.stop_if_idle()); + assert!(matches!( + queue.enqueue(key("new", ConfigComponentKind::SandboxConfig)), + DeliveryEnqueue::StartDispatcher + )); + } + + #[tokio::test] + async fn waiting_fanout_reserves_capacity_ahead_of_new_direct_work() { + let queue = ConfigDeliveryQueue::with_pending_capacity(1, 1, 1); + let a = key("a", ConfigComponentKind::SandboxConfig); + let b = key("b", ConfigComponentKind::SandboxConfig); + queue.enqueue(a.clone()); + assert_eq!(queue.take_ready(), Some(a.clone())); + let waiting = queue.enqueue_from_fanout(b.clone()); + tokio::pin!(waiting); + tokio::select! { + biased; + _ = &mut waiting => panic!("queue must be full"), + () = tokio::task::yield_now() => {} + } + queue.finish_pass(&a); + assert!(matches!( + queue.enqueue(key("new", ConfigComponentKind::SandboxConfig)), + DeliveryEnqueue::Full + )); + assert!(matches!(waiting.await, DeliveryEnqueue::Queued)); + assert_eq!(queue.take_ready(), Some(b)); + } + + #[test] + fn idle_dispatcher_exit_does_not_lose_new_publications() { + let queue = ConfigDeliveryQueue::new(1); + let a = key("a", ConfigComponentKind::SandboxConfig); + queue.enqueue(a.clone()); + queue.take_ready(); + queue.finish_pass(&a); + // A publication before the exit check keeps this dispatcher alive. + assert!(matches!(queue.enqueue(a.clone()), DeliveryEnqueue::Queued)); + assert!(!queue.stop_if_idle()); + queue.take_ready(); + queue.finish_pass(&a); + assert!(queue.stop_if_idle()); + // A publication after the exit check starts a replacement dispatcher. + assert!(matches!(queue.enqueue(a), DeliveryEnqueue::StartDispatcher)); + } + + #[derive(Debug)] + struct GatedRouter { + visits: tokio::sync::mpsc::UnboundedSender, + release: Semaphore, + panic_next: std::sync::atomic::AtomicBool, + } + + #[tonic::async_trait] + impl SupervisorConfigRouter for GatedRouter { + async fn deliver( + &self, + sandbox_id: &str, + _message: SupervisorConfigMessage, + ) -> DeliveryDisposition { + self.visits.send(sandbox_id.to_string()).unwrap(); + assert!( + !self.panic_next.swap(false, Ordering::SeqCst), + "injected worker failure" + ); + self.release.acquire().await.unwrap().forget(); + DeliveryDisposition::Enqueued + } + + async fn routable_sandbox_ids(&self) -> Vec { + vec!["a".into(), "b".into(), "c".into()] + } + } + + async fn gated_delivery_state( + capacity: usize, + ) -> ( + Arc, + Arc, + tokio::sync::mpsc::UnboundedReceiver, + ) { + let mut state = test_server_state().await; + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + let router = Arc::new(GatedRouter { + visits: tx, + release: Semaphore::new(0), + panic_next: std::sync::atomic::AtomicBool::new(false), + }); + let mutable = Arc::get_mut(&mut state).unwrap(); + mutable.config_delivery_queue = ConfigDeliveryQueue::with_pending_capacity(1, 1, capacity); + mutable.supervisor_config_router = router.clone(); + for id in ["a", "b", "c"] { + state + .store + .put_message(&Sandbox { + metadata: Some(ObjectMeta { + id: id.into(), + name: id.into(), + workspace: "default".into(), + ..Default::default() + }), + spec: Some(SandboxSpec::default()), + ..Default::default() + }) + .await + .unwrap(); + } + (state, router, rx) + } + + async fn next_visit(rx: &mut tokio::sync::mpsc::UnboundedReceiver) -> String { + tokio::time::timeout(Duration::from_secs(5), rx.recv()) + .await + .unwrap() + .unwrap() + } + + async fn wait_until_drained(state: &ServerState) { + tokio::time::timeout(Duration::from_secs(5), async { + loop { + if !state + .config_delivery_queue + .pending + .lock() + .unwrap() + .dispatcher_running + && state + .config_delivery_queue + .fanout_pending + .lock() + .unwrap() + .is_empty() + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .unwrap(); + } + + #[tokio::test] + async fn queued_changes_and_mutations_during_delivery_drain_without_reconciliation() { + let (state, router, mut rx) = gated_delivery_state(3).await; + publish_sandbox_components(&state, "a", ConfigComponents::SANDBOX_CONFIG); + assert_eq!(next_visit(&mut rx).await, "a"); + for id in ["b", "b", "c", "a", "a"] { + publish_sandbox_components(&state, id, ConfigComponents::SANDBOX_CONFIG); + } + assert!(rx.try_recv().is_err(), "only one worker may run"); + for expected in ["b", "c", "a"] { + router.release.add_permits(1); + assert_eq!(next_visit(&mut rx).await, expected); + } + router.release.add_permits(1); + wait_until_drained(&state).await; + assert!(rx.try_recv().is_err()); + assert_eq!( + state + .config_delivery_queue + .pending_slots + .available_permits(), + 3 + ); + } + + #[tokio::test] + async fn worker_panic_does_not_strand_pending_work() { + let (state, router, mut rx) = gated_delivery_state(3).await; + router.panic_next.store(true, Ordering::SeqCst); + router.release.add_permits(1); + publish_sandbox_components(&state, "a", ConfigComponents::SANDBOX_CONFIG); + publish_sandbox_components(&state, "b", ConfigComponents::SANDBOX_CONFIG); + assert_eq!(next_visit(&mut rx).await, "a"); + assert_eq!(next_visit(&mut rx).await, "b"); + wait_until_drained(&state).await; + assert_eq!( + state + .config_delivery_queue + .pending_slots + .available_permits(), + 3 + ); + } + + #[tokio::test] + async fn overflow_repairs_rejected_keys_without_periodic_reconciliation() { + let (state, router, mut rx) = gated_delivery_state(1).await; + publish_sandbox_components(&state, "a", ConfigComponents::SANDBOX_CONFIG); + assert_eq!(next_visit(&mut rx).await, "a"); + publish_sandbox_components(&state, "b", ConfigComponents::SANDBOX_CONFIG); + publish_sandbox_components(&state, "c", ConfigComponents::SANDBOX_CONFIG); + assert_eq!( + state + .config_delivery_queue + .pending + .lock() + .unwrap() + .changed + .len(), + 1 + ); + assert!( + state + .config_delivery_queue + .fanout_pending + .lock() + .unwrap() + .len() + <= 1 + ); + router.release.add_permits(16); + let mut observed = std::collections::HashSet::new(); + while !observed.contains("b") || !observed.contains("c") { + observed.insert(next_visit(&mut rx).await); + } + wait_until_drained(&state).await; + // No owner reconciler was started in this fixture. } #[test] fn build_bound_is_sized_from_the_database_pool() { let local = ConfigDeliveryQueue::for_db_connections(5); assert_eq!(local.max_concurrent_builds(), 10); - assert_eq!(local.delivery_permits.available_permits(), 64); + assert_eq!(local.max_delivery_workers, 64); assert_eq!( ConfigDeliveryQueue::for_db_connections(10).max_concurrent_builds(), 20 @@ -766,25 +1128,25 @@ mod tests { let queue = ConfigDeliveryQueue::new(3); assert!(matches!( queue.enqueue(key("sb-1", ConfigComponentKind::SandboxConfig)), - DeliveryEnqueue::StartWorker(_) + DeliveryEnqueue::StartDispatcher )); assert!(matches!( queue.enqueue(key("sb-1", ConfigComponentKind::ProviderEnvironment)), - DeliveryEnqueue::StartWorker(_) + DeliveryEnqueue::Queued )); assert!(matches!( queue.enqueue(key("sb-2", ConfigComponentKind::SandboxConfig)), - DeliveryEnqueue::StartWorker(_) + DeliveryEnqueue::Queued )); } #[tokio::test] async fn fleet_fanout_waits_without_creating_unbounded_delivery_workers() { const ROUTED_SANDBOXES: usize = 10_000; - let queue = Arc::new(ConfigDeliveryQueue::new(1)); + let queue = Arc::new(ConfigDeliveryQueue::with_pending_capacity(1, 1, 2)); let first = key("sandbox-0", ConfigComponentKind::SandboxConfig); - let DeliveryEnqueue::StartWorker(_blocked_worker) = queue.enqueue(first) else { - panic!("first publication must start a worker"); + let DeliveryEnqueue::StartDispatcher = queue.enqueue(first) else { + panic!("first publication must start the dispatcher"); }; let sandbox_ids = (1..ROUTED_SANDBOXES) @@ -803,7 +1165,7 @@ mod tests { () = tokio::task::yield_now() => {} } - assert_eq!(queue.pending.lock().unwrap().len(), 1); + assert_eq!(queue.pending.lock().unwrap().changed.len(), 2); } #[test] @@ -840,6 +1202,21 @@ mod tests { }), FanoutEnqueue::Full ); + for component in ConfigComponents::ALL.selected() { + let repair = FanoutKey { + scope: FanoutScope::AllConnected, + component, + }; + assert_eq!( + queue.enqueue_fanout(repair.clone()), + FanoutEnqueue::StartWorker + ); + assert_eq!(queue.enqueue_fanout(repair), FanoutEnqueue::Coalesced); + } + assert_eq!( + queue.fanout_pending.lock().unwrap().len(), + MAX_ACTIVE_FANOUT_WORKERS + 2 + ); } #[test]