You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Normalize the gateway compute-driver startup path so driver selection happens before constructing or validating driver-specific configuration. The current path mixes construction responsibilities across CLI parsing, runtime selection, and per-driver constructors.
Context
While reviewing PR #1703, we found that driver config/acquisition is inconsistent across drivers:
Docker and VM configs are constructed in crates/openshell-server/src/cli.rs before the selected runtime is known.
Kubernetes and Podman configs are constructed lazily inside build_compute_runtime after driver selection.
Remote endpoint drivers resolve CLI/env sockets through Config.compute_driver_endpoints, while TOML endpoint sockets are resolved directly from [openshell.drivers.<name>].
This means unrelated invalid Docker or VM config can fail gateway startup even when another driver is selected, and it makes the endpoint-backed driver path harder to reason about.
Definition of Done
build_compute_runtime or an adjacent acquisition layer selects the configured driver before building driver-specific config.
Docker, Podman, Kubernetes, VM, and remote endpoint drivers follow the same high-level acquisition pattern.
Unselected driver tables are not validated during startup.
Local TLS/defaulting behavior remains unchanged for selected local drivers.
Existing driver-selection and gateway-config tests are updated or expanded to cover the normalized behavior.
Issue type:refactor Complexity: Medium Confidence: High — scoped to main; remote endpoint integration is deferred until PR #1703 lands
Summary
Normalize built-in compute driver acquisition so the gateway resolves the selected driver before deserializing driver-specific TOML. The CLI should continue to merge gateway-wide config and compute local runtime defaults, but Docker and VM driver config construction should move behind the selected-driver branch in build_compute_runtime.
Scope
crates/openshell-server/src/cli.rs: remove eager VmComputeConfig and DockerComputeConfig construction from run_from_args; pass the raw ConfigFile and the local TLS snapshot through to server startup; add a focused CLI/helper-level regression test for unselected invalid driver tables.
crates/openshell-server/src/lib.rs: make selected-driver resolution visibly happen first in build_compute_runtime, then call per-driver config helpers inside the matching arm; add a private DriverConfigContext for raw TOML plus runtime defaults.
crates/openshell-server/src/lib.rs: move/convert Docker and VM config helpers to runtime-side helpers returning openshell_core::Result/Error::config, preserving the existing invalid [openshell.drivers.<name>] table: ... diagnostics where practical.
crates/openshell-server/src/lib.rs: update run_server to stop accepting prebuilt Docker/VM config. Since current repo call sites are internal, prefer making run_serverpub(crate) if that compiles cleanly; otherwise keep its public signature free of private helper types.
crates/openshell-server/src/config_file.rs: no schema change expected. Driver tables should remain raw until the selected driver's helper deserializes them.
Driver crates: no changes expected; this issue changes when server startup deserializes driver config, not driver schemas or constructors.
Add or adjust tests that pin the intended behavior: selected Docker/VM invalid config still fails, while one selected non-Docker/VM path ignores invalid unselected Docker/VM tables.
Introduce a private DriverConfigContext<'a> near build_compute_runtime that borrows Option<&'a ConfigFile> and Option<&'a LocalTlsPaths> and carries the runtime TLS mode needed for selected local driver defaults. Derive any VM gateway port default from the merged Config (config.bind_address.port()), not directly from CLI args.
Keep configured_compute_driver(config)? as the first driver-specific step in build_compute_runtime; after that, each match arm calls only the selected driver's config helper.
Add per-driver helpers, with a concise comment that they are only called after driver selection: docker_config_from_context, vm_config_from_context, kubernetes_config_from_context, and podman_config_from_context.
Use the same local TLS snapshot for Docker, VM, and Podman guest TLS defaulting. Do not re-probe defaults::complete_local_tls_paths() inside the Podman path.
Preserve gateway-wide local TLS discovery behavior. A partial local TLS bundle should still fail startup when TLS is enabled because that affects gateway listener defaults, not only driver config.
Keep K8s ServiceAccount bootstrap as an intentional separate read of Kubernetes config when in-cluster sandbox JWT exchange is enabled.
Split commits for review clarity if helpful, but do not require an exact commit sequence in the implementation.
Test Plan
Unit tests: Cover selected Docker and selected VM invalid table failures; cover one selected non-Docker/VM path that ignores invalid unselected Docker/VM tables; keep configured_compute_driver tests focused on selection only; move/rename existing Docker/VM config helper tests with the helpers.
CLI/helper test: Add at least one test at the CLI preparation boundary so a future eager build_docker_config/build_vm_config call in cli.rs would regress.
Integration tests: N/A for the main-scoped refactor.
E2E tests: N/A; this is startup/config routing behavior, not a new sandbox execution path.
Local checks: Run targeted openshell-server tests and mise run pre-commit after implementation.
Risks & Open Questions
Ignoring invalid unselected driver tables is intentional, but selected invalid config must continue to fail with stable, useful diagnostics.
run_server is currently public but only called internally in the repo. The implementation should prefer narrowing visibility to avoid exposing local runtime-default plumbing through the public API.
The refactor should not change VM subprocess launch behavior, Docker/Podman/Kubernetes constructors, gateway TLS listener defaults, or K8s ServiceAccount bootstrap validation.
Documentation Impact
No docs update required. This does not add, remove, rename, or change defaults for gateway TOML keys or driver-specific options.
No LSM compatibility impact expected. The change does not alter process identity, /proc access, binary execution, or inter-process visibility.
Activity
elezar commentedon Jun 17, 2026
Implementation Plan
Issue type:
refactorComplexity: Medium
Confidence: High — scoped to
main; remote endpoint integration is deferred until PR #1703 landsSummary
Normalize built-in compute driver acquisition so the gateway resolves the selected driver before deserializing driver-specific TOML. The CLI should continue to merge gateway-wide config and compute local runtime defaults, but Docker and VM driver config construction should move behind the selected-driver branch in
build_compute_runtime.Scope
crates/openshell-server/src/cli.rs: remove eagerVmComputeConfigandDockerComputeConfigconstruction fromrun_from_args; pass the rawConfigFileand the local TLS snapshot through to server startup; add a focused CLI/helper-level regression test for unselected invalid driver tables.crates/openshell-server/src/lib.rs: make selected-driver resolution visibly happen first inbuild_compute_runtime, then call per-driver config helpers inside the matching arm; add a privateDriverConfigContextfor raw TOML plus runtime defaults.crates/openshell-server/src/lib.rs: move/convert Docker and VM config helpers to runtime-side helpers returningopenshell_core::Result/Error::config, preserving the existinginvalid [openshell.drivers.<name>] table: ...diagnostics where practical.crates/openshell-server/src/lib.rs: updaterun_serverto stop accepting prebuilt Docker/VM config. Since current repo call sites are internal, prefer makingrun_serverpub(crate)if that compiles cleanly; otherwise keep its public signature free of private helper types.crates/openshell-server/src/config_file.rs: no schema change expected. Driver tables should remain raw until the selected driver's helper deserializes them.mainimplementation. When feat(server): support out-of-tree compute drivers via --compute-driver-socket #1703 lands or rebases, apply the same invariant to remote endpoint-backed drivers as a follow-up integration step.Implementation Steps
DriverConfigContext<'a>nearbuild_compute_runtimethat borrowsOption<&'a ConfigFile>andOption<&'a LocalTlsPaths>and carries the runtime TLS mode needed for selected local driver defaults. Derive any VM gateway port default from the mergedConfig(config.bind_address.port()), not directly from CLI args.configured_compute_driver(config)?as the first driver-specific step inbuild_compute_runtime; after that, each match arm calls only the selected driver's config helper.docker_config_from_context,vm_config_from_context,kubernetes_config_from_context, andpodman_config_from_context.defaults::complete_local_tls_paths()inside the Podman path.Test Plan
configured_compute_drivertests focused on selection only; move/rename existing Docker/VM config helper tests with the helpers.build_docker_config/build_vm_configcall incli.rswould regress.main-scoped refactor.openshell-servertests andmise run pre-commitafter implementation.Risks & Open Questions
run_serveris currently public but only called internally in the repo. The implementation should prefer narrowing visibility to avoid exposing local runtime-default plumbing through the public API.main; this plan explicitly defers that normalization until the remote path exists on the target branch.Documentation Impact
/procaccess, binary execution, or inter-process visibility.Revision 2 — updated after plan grilling