fix(embedded): preserve shell environment variables when constructor defaults are empty - #3401
Open
handnewb wants to merge 2 commits into
Open
fix(embedded): preserve shell environment variables when constructor defaults are empty#3401handnewb wants to merge 2 commits into
handnewb wants to merge 2 commits into
Conversation
handnewb
force-pushed
the
fix/embedded-preserve-shell-env-vars
branch
from
August 11, 2026 21:24
ae1bf8b to
8ce1df0
Compare
…defaults are empty The HindsightEmbedded constructor included every parameter in the config dict even when the caller left them at their default of "". Those empty-string values travelled through the daemon-startup env pipeline and overwrote the caller's actual shell environment variables (os.environ) set via export or .env files, so HINDSIGHT_API_LLM_API_KEY (and every other key) was silently ignored unless passed explicitly to the constructor. Root cause: the HINDSIGHT_* propagation loop in _start_daemon used 'value is not None' as its guard, so an empty string was always propagated and overwrote any prior os.environ value. Changes (defence in depth): - embedded.py: _set_if_truthy helper — only add keys with truthy values to the config dict. An empty default no longer poisons the merged env. - daemon_embed_manager.py: guard propagation on 'value' (truthy) instead of 'value is not None', so even if an empty string reaches this loop it is harmlessly skipped. Closes vectorize-io#3253.
handnewb
force-pushed
the
fix/embedded-preserve-shell-env-vars
branch
from
August 11, 2026 21:28
8ce1df0 to
5da99b8
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
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
Fixes #3253:
HindsightEmbeddedconstructor included every parameter in the config dict even when the caller left them at their default of"". Those empty-string values travelled through the daemon-startup env pipeline and overwrote the caller's actual shell environment variables set viaexportor.envfiles.Root Cause
The
HINDSIGHT_*propagation loop in_start_daemon(line 543) usedvalue is not Noneas its guard:An empty string (
"") is notNone, so it was always propagated and overwrote any prioros.environvalue. The caller'sHINDSIGHT_API_LLM_API_KEYfrom their shell was silently replaced with"".Changes (defence in depth)
1.
embedded.py— source fixNew
_set_if_truthyhelper: only adds keys with truthy values to the config dict. An empty default no longer poisons the merged env. This is the primary fix — the constructor never emits empty values in the first place.2.
daemon_embed_manager.py— defense in depthGuard propagation on
value(truthy) instead ofvalue is not None. Even if an empty string reaches this loop through some other path, it is harmlessly skipped.Impact
Users can now set
HINDSIGHT_API_LLM_API_KEY(and every otherHINDSIGHT_*var) in their shell environment and haveHindsightEmbedded()pick it up without explicitly passing it to the constructor.