Merge every .env candidate instead of stopping at the first - #53
Merge every .env candidate instead of stopping at the first#53kliebensteinchr wants to merge 1 commit into
Conversation
_maybe_load_env_file() picked the first existing candidate of $INKBOX_CLAUDE_ENV_FILE, ./.env, ~/.inkbox-claude/.env and ignored the rest. An unrelated ./.env in the cwd -- a project's own secrets, or a stray one in $HOME -- therefore shadowed the install's config entirely, so `start`/`restart` from that directory reported "INKBOX_API_KEY and INKBOX_IDENTITY are not set" while both sat in the state dir. The systemd/launchd units set INKBOX_CLAUDE_ENV_FILE explicitly, so this only hit manual CLI runs. Read all candidates in the same priority order instead, still via setdefault: real env beats every file, an earlier file beats a later one, and a later file fills only what the earlier ones left out. Resolved paths are deduped so a cwd that is the state dir is not read twice, and an unreadable file is treated as absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| if stripped.startswith("export "): | ||
| stripped = stripped[len("export "):] | ||
| key, value = stripped.split("=", 1) | ||
| os.environ.setdefault(key.strip(), value.strip().strip('"').strip("'")) |
There was a problem hiding this comment.
Because every later file is merged per key, omission in a higher-priority config no longer restores its documented default. A complete explicit config can silently inherit stale optional values from the state-dir file, and a partial cwd config can be combined with credentials or identity from a different install, producing a configuration no single file defines. Please keep the explicit file authoritative and only fall through from cwd when it has zero bridge-owned keys (or otherwise select one Inkbox config atomically) rather than merging independent configs per key.
| if key_path in seen: # cwd == state dir, or a symlink to it | ||
| continue | ||
| seen.add(key_path) | ||
| lines = path.read_text().splitlines() |
There was a problem hiding this comment.
The unreadable-file fallback is incomplete: read_text() can raise UnicodeDecodeError, and malformed assignments such as an empty key raise while updating os.environ outside this try. Because lower-priority files are now always read, an unrelated malformed file can crash startup even when a higher-priority config is complete. Please skip decode and malformed-assignment failures per candidate and add regression coverage for both cases.
The bug
_maybe_load_env_file()picks the first existing candidate and ignores the rest:The candidates are
$INKBOX_CLAUDE_ENV_FILE,./.env,~/.inkbox-claude/.env. So any unrelated./.envin the cwd shadows the install's config entirely.Concretely: a
~/.envholding nothing butSLACK_BOT_TOKENmeansinkbox-claude start/restartrun from$HOMEfail witheven though both are sitting in
~/.inkbox-claude/.env. The failure points at setup, so the natural next step is re-running the wizard — which rewrites a config that was never the problem.The systemd/launchd units set
INKBOX_CLAUDE_ENV_FILEexplicitly, so autostart is unaffected. This only hits manual CLI runs, which is what makes it confusing: the service is up, but the CLI insists you are not configured.The fix
Read every candidate in the same priority order, still via
setdefault:./.envstill wins over the state dir)Also dedupes resolved paths so a cwd that is the state dir is not read twice, and treats an unreadable file as absent rather than raising.
No precedence change for anyone whose
./.envalready carried the Inkbox keys — that file still wins every key it sets.Tests
Two regression tests in
tests/test_daemon.py:test_maybe_load_env_file_merges_state_dir_under_an_unrelated_cwd_env— the reported failure: unrelated./.envpresent, keys still resolve from the state dir, and the cwd file's own var still appliestest_maybe_load_env_file_earlier_candidate_wins_per_key— pins the precedence so a future change cannot silently invert itFull suite locally on 3.12: 346 passed, 22 skipped.
🤖 Generated with Claude Code