Enable Environment Specification and/or overriding from offload#43
Merged
DanverImbue merged 13 commits intomainfrom Feb 18, 2026
Merged
Enable Environment Specification and/or overriding from offload#43DanverImbue merged 13 commits intomainfrom
DanverImbue merged 13 commits intomainfrom
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
danverbraganza
force-pushed
the
danver/enable-env-overriding
branch
from
d48b86d to
fb5de33
Compare
There was a problem hiding this comment.
Vet found 2 issues.
[test_coverage] (severity 3/5) (confidence 0.85)
The issues tracker lists code-65 (tests for environment variable expansion) as 'open', meaning tests for the new expand_env_value function with actual environment variable lookups (e.g., ${VAR} with VAR set, ${VAR:-default} with VAR unset) were not added. The existing tests only cover pure string parsing (no env vars, escaped dollars, error cases) but don't test the core functionality of actually expanding environment variables. The --env CLI flag, base_env() trait method, and env propagation through the orchestrator also lack test coverage.
danverbraganza
force-pushed
the
danver/enable-env-overriding
branch
from
2c5e2d1 to
c1692d2
Compare
Add environment variable expansion support to config loading:
- ${VAR} - required variable, errors if not set
- ${VAR:-default} - optional with default value
- $$ - escaped literal dollar sign
Applied automatically when loading provider.env values.
Add env field to DefaultSandbox and inject env vars as shell prefix (KEY=value) into the command string in build_exec_command(). This fixes the issue where provider.env variables were not being passed to remote sandbox execution, unlike LocalProvider which correctly sets env vars on the process. Fixes: code-67
danverbraganza
force-pushed
the
danver/enable-env-overriding
branch
from
4c5be3c to
48e773a
Compare
Tests use reliably existing/non-existing env vars (HOME, _OFFLOAD_TEST_*)
instead of unsafe set_var/remove_var. Covers all expansion cases:
- ${VAR} when set/unset
- ${VAR:-default} when set/unset
- $$ escape sequence
- Mixed expansion
- Empty default
Fixes: code-65
danverbraganza
force-pushed
the
danver/enable-env-overriding
branch
from
48e773a to
c5b68bc
Compare
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
1 participant
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.
Description
Some tests require local environment variables to succeed. Proper CI might inject these variables via secrets managers, but for an ad-hoc testrunner we want to leverage, e.g. Modal's ability to capture environments variables and set them on the remote boxes.