feat(api): POST /environment/reload applies the guest metadata environment live - #289
feat(api): POST /environment/reload applies the guest metadata environment live#289devin-ai-integration[bot] wants to merge 4 commits into
Conversation
…nment live The guest init pings this endpoint after applying a new metadata generation. The handler re-reads /bl/metadata and applies its environment to the sandbox-api process (os.Setenv/Unsetenv), so the API process and every process spawned afterwards see the current values without a sandbox restart. VMs on an older initrd simply have no /bl/metadata and get a 404. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
🧪 Testing GuideWhat this PR addressesAdds a new Steps to reproduce the original issuePreviously, updating environment variables on a running sandbox required a full restart of the sandbox-api process. There was no mechanism for the guest to apply environment changes live. How to exercise the new behavior
What to verify (expected behavior)
Note Posted by PR Testing Guide · Tag @mendral-app with feedback. |
🔄 Interaction Flow: Environment ReloadsequenceDiagram
participant VMM as vmm-manager
participant Initrd as initrd (guest)
participant API as sandbox-api<br>/environment/reload
participant Meta as /bl/metadata
participant OS as os (env vars)
VMM->>Initrd: notify env update
Initrd->>Meta: refetch /bl/metadata
Meta-->>Initrd: JSON {generation, environment}
Initrd->>API: POST /environment/reload
API->>API: acquire mutex lock
API->>Meta: read metadata file
Meta-->>API: MetadataDocument
API->>API: diff applied map vs new env
loop removed keys
API->>OS: Unsetenv(key)
end
loop new/updated keys
API->>OS: Setenv(key, value)
end
API->>API: update applied map & generation
API-->>Initrd: 200 {generation, applied, removed}
SummaryThis PR introduces a live environment reload mechanism for running sandboxes:
Thread safety is ensured via Note Posted by PR Sequence Diagram · Tag @mendral-app with feedback. |
|
📋 Created Linear issue ENG-4745 — status: In Progress
Auto-created because no Linear reference was found in the PR title, description, or branch name. Note Posted by Linear Issue Enforcer · Tag @mendral-app with feedback. |
![mendral-app[bot]](https://avatars.githubusercontent.com/in/1975249?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| removed := 0 | ||
| for key := range h.applied { | ||
| if _, ok := doc.Environment[key]; !ok { | ||
| if err := os.Unsetenv(key); err == nil { | ||
| removed++ | ||
| } | ||
| delete(h.applied, key) | ||
| } | ||
| } | ||
| applied := 0 | ||
| for key, value := range doc.Environment { | ||
| if err := os.Setenv(key, value); err != nil { | ||
| logrus.WithError(err).WithField("key", key).Warn("Failed to set environment variable") | ||
| continue | ||
| } | ||
| h.applied[key] = struct{}{} | ||
| applied++ | ||
| } |
There was a problem hiding this comment.
🟡 Variables that also exist in the image configuration are erased instead of restored when dropped from metadata
A variable that the container image already defined is deleted outright (os.Unsetenv(key) at sandbox-api/src/handler/environment.go:97) once it disappears from the metadata, instead of falling back to the value the image originally provided, so settings silently vanish.
Impact: If an environment update temporarily overrides a built-in setting (for example a search path or a service URL) and later drops it, the sandbox and every program it launches afterwards run with that setting completely missing rather than with the original default.
No snapshot of pre-existing values before overwriting
HandleReload tracks only the set of keys it applied (h.applied), not their prior values. First reload: metadata contains KEY=override, so os.Setenv("KEY", "override") replaces the image-provided value and h.applied["KEY"] is recorded (sandbox-api/src/handler/environment.go:104-111). Next reload where metadata no longer carries KEY: the loop at sandbox-api/src/handler/environment.go:95-102 unsets it, so the original image value is lost for this process and, via buildProcessEnv starting from os.Environ() (sandbox-api/src/handler/process/process.go:141), for every process spawned afterwards. Recording the previous value (and whether it existed) when first overriding would allow restoring instead of unsetting.
Prompt for agents
In sandbox-api/src/handler/environment.go, EnvironmentHandler only remembers which keys came from the metadata document (h.applied as a set), not what the process environment held before those keys were first overridden. When a later metadata generation drops a key, the handler unsets it, which erases any value that originally came from the container image ENV or boot-time setup rather than restoring it. Consider changing h.applied to a map from key to the previous value plus a flag indicating whether the key existed before, captured only the first time the handler overrides that key; on removal, restore the previous value if it existed, otherwise unset.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 6b36cc0 — applied now records the value (and existence) the process held before the first override; when a later generation drops a key, that original value is restored instead of the variable being erased. Covered by TestHandleReloadRestoresPreExistingValue.
| func metadataPath() string { | ||
| if path := os.Getenv("BL_METADATA_PATH"); path != "" { | ||
| return path | ||
| } | ||
| return defaultMetadataPath | ||
| } |
There was a problem hiding this comment.
🔍 Metadata can override the very variable that selects the metadata path
metadataPath() reads BL_METADATA_PATH from the process environment, and HandleReload then applies arbitrary key/values from the document via os.Setenv. If a metadata document ever carries BL_METADATA_PATH, subsequent reloads read from a different file (and if a later generation drops the key, the path silently reverts). Worth considering skipping this key when applying, or resolving the path once at handler construction.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 6b36cc0 — the metadata path is now resolved once at handler construction, so a document carrying BL_METADATA_PATH can no longer redirect subsequent reloads.
…ues on removal Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6b36cc0. Configure here.
…time values Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
LGTM
The simplification in 96c39e9 (unset instead of restore) is correct given the protocol design: the metadata document is the authoritative complete set, so a removed key should be fully gone. The code compiles (uses the package-level jsoniter variable from filesystem.go), tests cover the key scenarios, and the mutex serializes concurrent reloads properly.
Tag @mendral-app with feedback or questions. View session
1 participant
Fixes ENG-4745
Summary
Companion to blaxel-ai/executionplane#448 (pull-based guest metadata). An environment update on a running sandbox now reaches the sandbox-api process without a restart:
/bl/metadataand then fires one best-effortPOST http://127.0.0.1:8080/environment/reload.EnvironmentHandler.HandleReloadre-reads/bl/metadataand applies itsenvironmentto this process:Because
buildProcessEnvstarts fromos.Environ(), every process spawned afterwards (process API, terminals, restarts) inherits the updated set too. Only keys that came from the metadata document are ever unset — image ENV and boot-time variables are untouched.Backward compatible: a VM on an older initrd has no
/bl/metadata, the endpoint returns 404, and nothing else changes.Link to Devin session: https://app.devin.ai/sessions/6ab2c5d636a2486684f6cece3d0818e4
Requested by: @drappier-charles
Note
Adds
POST /environment/reloadendpoint that re-reads/bl/metadataand applies its environment map to the sandbox-api process viaos.Setenv/os.Unsetenv. The latest commit simplifies the removal logic to always unset (rather than restore boot-time values), since the metadata document represents the host's complete environment set.Written by Mendral for commit 96c39e9.
Note
Medium Risk
Mutates the API process environment at runtime (including unsetting host-managed keys), which affects all subsequently spawned commands; endpoint is localhost-oriented but unauthenticated like other system routes.
Overview
Adds
POST /environment/reloadso sandbox-api can adopt host environment updates without restarting, paired with pull-based guest metadata from the initrd.EnvironmentHandlerre-reads/bl/metadata(orBL_METADATA_PATH, fixed at handler construction), unmarshalsgenerationandenvironment, thenos.Setenvfor each key andos.Unsetenvonly for keys previously applied from metadata that are absent in the new document. Child processes pick up changes because process spawning usesos.Environ(). Missing metadata returns 404 for older VMs.Router wiring, OpenAPI/Swagger docs, and unit tests cover apply/update/remove and the no-metadata case.
Reviewed by Cursor Bugbot for commit 96c39e9. Bugbot is set up for automated code reviews on this repo. Configure here.