Repository navigation
Conversation
zibi-pplx
added a commit
that referenced
this pull request
Oct 26, 2026
## Summary Add an opt-in typed Windows scenario that creates its own sandbox, checks RDP NLA negotiation, stops and starts with the persisted contract, and deletes its resources. Poll asynchronous lifecycle state so the RPC wait budget does not truncate Windows shutdown. The fixture refuses manager restarts and device fault injection under the firmware-only opt-in. Document firmware staging, local opaque-image import, required Windows drivers and clock settings, deployment order, native SSH and fenced serial/RDP access. Evaluation disks and credentials stay outside the repository. ## Validation All 31 smoke framework tests passed. The standalone hardware run passed create, RDP negotiation, stop/start with a new execution identity, deletion and node-lease cleanup, with no cleanup failures. RDP negotiation alone is not authenticated user access; native SSH was checked separately. Agent refusal before wake is covered by the sandbox-manager guest-image tests. ## Severity and risk Severity does not apply to this explicit development test. The scenario requires an operator-supplied host and machine identity, template digest and environment reference. It cleans only its own sandbox and does not restart shared services or allocate GPUs. ## Stack Windows guests. #954 is a separate reader-guard deployment before #955–#959 enable opaque images. Keep that guard as the rollback floor. #955 supplies the SQLite schema before its readers/writers; deploy the matching supervisor before node/build/template/sandbox managers. Firmware guests remain opt-in, standalone and locally imported; export, pull and cross-node distribution stay refused. See #961 for the complete rollout groups. AI assistance: Codex implemented the scenario, documentation and this description.
This was referenced Oct 26, 2026
zibi-pplx
marked this pull request as ready for review
October 26, 2026 03:14
zibi-pplx
force-pushed
the
pr919
branch
4 times, most recently
from
October 26, 2026 15:41
130b4b5 to
d891df2
Compare
Collaborator
Review — 2 findingsLow
Verified against
I didn't run a live mixed-version deployment. |
the-gigi-pplx
approved these changes
Oct 26, 2026
the-gigi-pplx
left a comment
the-gigi-pplx
left a comment
Collaborator
There was a problem hiding this comment.
Approved. 2 non-blocking findings in the review comment above.
## Summary Environment descriptions and node control RPCs currently encode Firecracker-specific release, boot and snapshot assumptions. Add shared runtime-neutral contracts for managed releases, role-tagged boot artifacts, compatibility and optional memory state, plus generic launch, VM-state, capture and restore RPCs. Runtime catalogs and adapters still decide which contracts they support; the new representation does not enable another runtime. Keep the existing Firecracker wire representation, stored documents and environment references intact during migration. Schema-2 readers reject mixed profiles, incorrect tags, duplicate roles, non-Linux KVM hosts and cross-architecture KVM guests. A contract may omit separate boot files or memory state when its runtime supports that shape. The historical E0 restore exception explicitly requires a state format, alongside the existing catalog and inventory checks. Generic VM routes authorize once under the requested RPC name, then use shared internal operations. Snapshot handlers retain waiter metrics, spans and cancellation outcomes. Implemented runtime operations reserve Unimplemented for missing transport methods so internal failures cannot trigger legacy replay. Legacy capture conversion rejects dual sandbox identities rather than silently choosing one. Generic artifacts retain typed roles and formats when constructing requirements; diagnostic CLI labels identify the runtime rather than naming Firecracker. Protected generic launches require matching top-level and verified sandbox identities; legacy conversions preserve the old-peer refusal guards and reject dual legacy identities before transport. Status remapping preserves metadata and details, and memory-state operations require both catalog qualification and a declared format. Regenerate shared Go execution bindings. No SQL or execution-policy changes. Keep the original public sandbox health field and enum names during this bridge; internal health types and diagnostic labels may be runtime-neutral. ## Validation - Rebased on main at 718105d. The integrated Linux arm64 all-target checks passed; CI checks the bridge revision independently. - Documented the two existing snapshot storage locator exceptions in docs/execution-environments.md; checked the field names and retained validators against the implementation. This amendment is documentation-only. - Runtime-core: 109 tests passed. The dual-launch-identity, status-evidence and missing-state-format regressions each failed before their fix and passed afterward. - Linux arm64 all-target Clippy passed for runtime-core, supervisor, node-manager, sandbox-manager, API (with Kubernetes enabled), and the node simulator. - Regenerated Go bindings with CI-pinned protoc 35.0 and plugins; gofmt, go vet and all shim tests passed. The existing public health-response snapshot test passed with the bridge names. - Earlier validation covered stable environment identities, generic snapshot conversion and authorization. No new live KVM or mixed-version deployment was performed for these review fixes; full CI remains required. ## Severity and risk Architectural extension. The main risks are changing persisted identities or accepting unsupported contracts. Golden identities, structural rejection and capability checks cover these boundaries. Keep existing Firecracker/E0 selection during deployment. ## Rollout Land #961 followed by #920 as one bridge release: this PR introduces the contracts; #920 routes Firecracker through them. Deploy every participating service, CLI and rollback image, then verify template builds, create, command, stop/start, pause/resume and suspend/restore with both RPC generations available. Only after that deployment and after old calls are drained, land #921 to remove the legacy gRPC representation and routes. Persisted environment identity compatibility remains. That cleanup also renames the public health field and enum to runtime-neutral names without changing their numeric tags. JSON/string consumers and regenerated source clients must migrate at that boundary; no permanent legacy aliases remain. ## Snapshot storage boundary CaptureVmState and RestoreVmState deliberately retain the existing node-local snapshot storage locators (`directory` and `encrypted_volume_mount`). This is a narrow exception to the domain-identifiers rule for migration of existing snapshot workflows, not precedent for new capabilities. The same peer authorization, execution fences, sandbox-scoped canonical path validation and trusted filesystem handling apply. These fields survive the legacy-RPC cleanup and grant no broader host-path authority. The node-manager API rule is unchanged. AI assistance: Codex drafted the code, tests, documentation and description.
zibi-pplx
enabled stacked auto-merge
October 26, 2026 19:53
GeekLord
pushed a commit
to GeekLord/SPACE
that referenced
this pull request
Oct 26, 2026
…RPCs (perplexityai#920) ## Summary Move existing Firecracker callers onto the runtime-neutral contracts introduced by perplexityai#961. Catalog definitions, node resolution and template build inputs use the generic environment shape while retaining their original canonical identity, stored JSON and environment references. The decoder rejects conversions that would discard or change requirements. Resolution negotiates representation: new nodes return generic definitions to new clients and the equivalent legacy representation to old clients; new clients normalize responses from old nodes. Supervisor launch, VM-state probes, and scheduled, manual and base snapshot capture/restore use the generic RPCs. Sandbox reconciliation consumes the shared VM-state type. Snapshot capture derives the fallback runtime from recorded execution metadata, and restore uses the acknowledged consumer runtime. Historical unbound snapshots retain Firecracker fallback. VM-state probes remain Firecracker-only during this bridge; fallback is removed before other runtime adapters are enabled. Hoist the suspend capture request so its binding and storage fields remain reviewable. The Firecracker-only rolling-upgrade bridge falls back only when a method is missing. Runtime refusals, cancellation and ambiguous transport failures do not replay through the old route. Fallback conversion failures retain the admission-refusal marker, giving old and new supervisors the same error classification without sending a legacy launch. Legacy launch inputs are converted fallibly before transport: combining a verified identity with any top-level legacy sandbox ID is refused locally, even when the IDs match. Existing timeouts, execution fences, source authentication and ownership rules remain. No SQL, executable, selection-policy or newly enabled runtime changes. ## Validation - Rebased on main at 718105d. The integrated Linux arm64 all-target checks passed; mixed-version capture and fallback regressions passed before the rebase. - The metadata-derived capture path passed the mixed-version refusal test and client fallback matrix. Linux arm64 all-target sandbox-manager Clippy and Rust formatting passed. - The old/new-supervisor launch matrix failed before the fallback marker fix (Internal instead of FailedPrecondition) and passed afterward, covering valid launches, mismatched identities and absent/empty containment executables. Invalid requests invoke no launch handler. - The launch integration matrix passed: dual legacy identities cause zero launch or stop RPCs, valid acknowledgement succeeds, and missing or wrong acknowledgements stop the execution. - The client fallback matrix passed for missing methods, success, cancellation, deadlines, transport failures and precondition refusals across Firecracker and other runtime requests. - Linux arm64 all-target Clippy for node-manager and supervisor passed with warnings denied. Earlier bridge validation covered stable canonical identities, historical metadata, both environment representations and builder bindings; inherited perplexityai#961 validation includes 109 runtime-core tests and Go binding checks. - No live mixed-version or KVM deployment performed. ## Severity and risk Architectural migration. The primary risks are identity drift and mixed-version failures. Representation negotiation cannot select a different environment, and both RPC generations remain available during rollout. ## Rollout Land perplexityai#961 then perplexityai#920 and deploy them together to every participating service, CLI and rollback image. Verify Firecracker template builds, create, command, stop/start, pause/resume and suspend/restore through the new routes, including mixed-version fallback. During the canary, measure end-to-end VM-state probe latency and timeout rate on not-yet-upgraded nodes before considering a bounded capability cache. The runbook documents the cleanup-phase lifecycle metric-label transition while preserving the LOAD_SNAPSHOT configuration key. After deployment is complete and old calls are drained, perplexityai#921 removes the obsolete gRPC representation and routes. It retains compatibility with persisted environment identities. Keep the original public sandbox health field and enum names in this bridge; perplexityai#921 changes them to runtime-neutral names at the JSON/string and generated-client migration boundary, with no permanent aliases. AI assistance: Codex drafted the code, tests, documentation and description.
This was referenced Oct 26, 2026
meefs
pushed a commit
to meefs/SPACE
that referenced
this pull request
Oct 27, 2026
…xityai#921) ## Summary Remove the Firecracker-specific environment and runtime-control gRPC left behind by perplexityai#961 and perplexityai#920. All callers now use generic environment definitions and LaunchRuntime, GetVmState, CaptureVmState and RestoreVmState. Delete the old routes, generated types, fallbacks and internal empty-ID encoding. Keep authorization, execution fencing, snapshot observability and source authentication intact. Firecracker adapter inputs retain parsed sandbox IDs, execution fences and paths. Snapshot requests without a fence are rejected before taking ownership locks; current-execution validation remains under those locks. Plain and encrypted storage are distinct parsed variants. Encrypted snapshots ignore unused plaintext locations, and validated mount paths stay typed through preparation. The shared Firecracker contract helper recognizes both historical and runtime-neutral definitions. Retain schema-1 canonical/JSON decoding to verify persisted environment references, but remove its gRPC representation and old-response decoder. Require schema-2-capable callers. Retain the capability request so this release can obtain generic definitions from transitional perplexityai#920 nodes. Remove the retired reservations without renumbering live fields, as agreed. No SQL or execution-policy changes. Rename public sandbox health names to runtime_alive and SANDBOX_HEALTH_COMPONENT_RUNTIME at their existing numeric tags, with no old-name aliases. Shared public sandbox snapshot and host-storage APIs remain. The snapshot path-field exception recorded in perplexityai#961 stays bounded by existing authorization, execution fences and sandbox-scoped path validation. The limiter now names RestoreVmState internally while retaining the deployed LOAD_SNAPSHOT configuration suffix. Bridge-canary fallback guidance is explicitly scoped to the preceding release. ## Validation Rebased directly on main at dbc0fd5, which includes merged perplexityai#961 and perplexityai#920. - Extended the existing resolver handler test across schemas 0, 1 and 2: authorization comes first, legacy schemas receive the schema-specific refusal, and schema 2 reaches host/inventory validation. Disabling the schema gate makes the test fail. All 11 node-artifact tests, 9 limiter tests and the restore-refusal/permit-release regression passed; Linux arm64 all-target node-manager Clippy and formatting passed. - Reproduced the historical-contract helper omission and missing-fence boundary acceptance before fixing them; both regressions pass afterward. - 22 execution-common tests and 48 snapshot tests passed. Nine ownership-contention, seven supervisor-client, one startup-admission and seven builder-binding tests passed. - Linux arm64 all-target Clippy covers node-manager, build-manager and execution-common; formatting is checked. - Approved proto comment/whitespace corrections preserve every non-comment token. Regenerated the Go binding with pinned protoc 35.0 and verified a second generation is unchanged; gofmt, go vet and all shim Go tests passed. CI on the preceding head passed Rust tests, Clippy and isolated-crate checks; its stale generated-comment failure is corrected here. No live deployment or KVM lifecycle test was performed for this amendment. CI runs the complete suite. ## Severity and risk Severity: protocol retirement. Pre-migration clients and rollback images lose the removed routes. Persisted references and snapshot metadata remain readable. JSON health field names, enum strings and generated source names change even though binary field tags stay fixed. ## Rollout perplexityai#961 and perplexityai#920 are merged; deployment remains a separate gate. DO NOT MERGE OR DEPLOY this cleanup until perplexityai#920 is deployed to all participating control-plane services, node-manager, runtime-supervisor, builder, sandbox-manager, node-ctl, shim and operator CLI readers, including rollback images, and old in-flight requests have completed. After that gate, transitional and removal versions can roll in either order because both advertise and understand generic definitions. Restore the transitional server before rolling a client back further. This cleanup must remain a separate release after the bridge deployment. Update JSON/string and generated-source consumers at this boundary. Lifecycle-admission metrics change from method:LoadSnapshot to method:RestoreVmState; monitoring queries must include both during rollout. Keep the LOAD_SNAPSHOT configuration suffix unchanged. AI assistance: Codex drafted code, tests, documentation and this description.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Environment descriptions and node control RPCs currently encode Firecracker-specific release, boot and snapshot assumptions. Add shared runtime-neutral contracts for managed releases, role-tagged boot artifacts, compatibility and optional memory state, plus generic launch, VM-state, capture and restore RPCs. Runtime catalogs and adapters still decide which contracts they support; the new representation does not enable another runtime.
Keep the existing Firecracker wire representation, stored documents and environment references intact during migration. Schema-2 readers reject mixed profiles, incorrect tags, duplicate roles, non-Linux KVM hosts and cross-architecture KVM guests. A contract may omit separate boot files or memory state when its runtime supports that shape. The historical E0 restore exception explicitly requires a state format, alongside the existing catalog and inventory checks.
Generic VM routes authorize once under the requested RPC name, then use shared internal operations. Snapshot handlers retain waiter metrics, spans and cancellation outcomes. Implemented runtime operations reserve Unimplemented for missing transport methods so internal failures cannot trigger legacy replay. Legacy capture conversion rejects dual sandbox identities rather than silently choosing one. Generic artifacts retain typed roles and formats when constructing requirements; diagnostic CLI labels identify the runtime rather than naming Firecracker. Protected generic launches require matching top-level and verified sandbox identities; legacy conversions preserve the old-peer refusal guards and reject dual legacy identities before transport. Status remapping preserves metadata and details, and memory-state operations require both catalog qualification and a declared format. Regenerate shared Go execution bindings. No SQL or execution-policy changes. Keep the original public sandbox health field and enum names during this bridge; internal health types and diagnostic labels may be runtime-neutral.
Validation
Rebased on main at 718105d. The integrated Linux arm64 all-target checks passed; CI checks the bridge revision independently.
Documented the two existing snapshot storage locator exceptions in docs/execution-environments.md; checked the field names and retained validators against the implementation. This amendment is documentation-only.
Runtime-core: 109 tests passed. The dual-launch-identity, status-evidence and missing-state-format regressions each failed before their fix and passed afterward.
Linux arm64 all-target Clippy passed for runtime-core, supervisor, node-manager, sandbox-manager, API (with Kubernetes enabled), and the node simulator.
Regenerated Go bindings with CI-pinned protoc 35.0 and plugins; gofmt, go vet and all shim tests passed. The existing public health-response snapshot test passed with the bridge names.
Earlier validation covered stable environment identities, generic snapshot conversion and authorization. No new live KVM or mixed-version deployment was performed for these review fixes; full CI remains required.
Severity and risk
Architectural extension. The main risks are changing persisted identities or accepting unsupported contracts. Golden identities, structural rejection and capability checks cover these boundaries. Keep existing Firecracker/E0 selection during deployment.
Rollout
Land #961 followed by #920 as one bridge release: this PR introduces the contracts; #920 routes Firecracker through them. Deploy every participating service, CLI and rollback image, then verify template builds, create, command, stop/start, pause/resume and suspend/restore with both RPC generations available.
Only after that deployment and after old calls are drained, land #921 to remove the legacy gRPC representation and routes. Persisted environment identity compatibility remains. That cleanup also renames the public health field and enum to runtime-neutral names without changing their numeric tags. JSON/string consumers and regenerated source clients must migrate at that boundary; no permanent legacy aliases remain.
Snapshot storage boundary
CaptureVmState and RestoreVmState deliberately retain the existing node-local snapshot storage locators (
directoryandencrypted_volume_mount). This is a narrow exception to the domain-identifiers rule for migration of existing snapshot workflows, not precedent for new capabilities. The same peer authorization, execution fences, sandbox-scoped canonical path validation and trusted filesystem handling apply. These fields survive the legacy-RPC cleanup and grant no broader host-path authority. The node-manager API rule is unchanged.AI assistance: Codex drafted the code, tests, documentation and description.
Stack created with Sapling. Best reviewed with ReviewStack.