Skip to content

feat(env): load .env once in bin/px-env (#345) - #437

Merged
adrianwedd merged 1 commit into
masterfrom
fix/345-dotenv-in-px-env
Sep 21, 2026
Merged

adrianwedd merged 1 commit into
masterfrom
fix/345-dotenv-in-px-env

Conversation

@adrianwedd

Copy link
Copy Markdown
Owner

Option 1, as decided. bin/px-env — sourced by every bin/px-* launcher — loads .env once with set -a, so a manual launcher behaves like a systemd unit.

The reported failure: bin/px-spark started, greeted, accepted input, then answered every turn with the unavailable acknowledgement because PX_M5_SPARK_MODEL was unset.

  • caller-set values win (systemd's precedence);
  • only names .env assigns are touched;
  • PX_ENV_SKIP_DOTENV=1 opts out;
  • a missing .env is not an error.

Verified by driving the real script in a clean environment against a fixture project; 6 tests including a guard that .env stays git-ignored.

… like a unit (#345)

The filed failure, live on the robot: `bin/px-spark` started, greeted, accepted
input, and then answered every turn with the unavailable acknowledgement, because
`PX_M5_SPARK_MODEL` was unset — `EnvironmentFile=` supplies it to the systemd
units and nothing supplied it to a human's shell:

    [voice-loop] cognition tier bad_response: PX_M5_SPARK_MODEL must name a
    model explicitly (not auto)

Option 1 (the recommended one): `bin/px-env` — sourced by every `bin/px-*`
launcher — loads `.env` once, `set -a` so every assignment is exported, exactly
as `EnvironmentFile=` means to systemd. One source of truth for environment
construction instead of reproducing it per launcher.

Precedence is systemd's: a variable the caller already exported wins over the
file. Only the names `.env` assigns are saved and restored, so nothing else in
the environment is read or rewritten. `PX_ENV_SKIP_DOTENV=1` opts out for a
caller constructing its own environment. A missing `.env` (fresh clone, CI) is
not an error.

Verified by driving the real script in a clean environment against a fixture
project — an environment fact needs an environment to observe:

    fresh shell          PX_M5_SPARK_MODEL=deepseek-v4.1-flash:cloud
    caller sets it       caller's value preserved
    PX_ENV_SKIP_DOTENV=1 unset, as before
    no .env              unset, exit 0

6 tests in tests/test_px_env_dotenv.py, including a guard that `.env` stays
git-ignored (px-env will happily source it, so that is the guard that matters).
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-21T07:08:02.677669Z ac2c3c6 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@adrianwedd
adrianwedd merged commit d37ae8f into master Sep 21, 2026
4 checks passed
@adrianwedd
adrianwedd deleted the fix/345-dotenv-in-px-env branch September 21, 2026 07:07

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac2c3c6799

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread bin/px-env
Comment on lines +72 to +75
set -a
# shellcheck disable=SC1091
source "$PROJECT_ROOT/.env"
set +a

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove the legacy per-launcher dotenv reloads

When launching bin/px-mind, bin/px-api-server, bin/px-wake-listen, bin/px-blog, or bin/px-evolve, each script sources px-env and then sources .env again in its existing lines 8–23. That second load undoes the caller-value restoration here and ignores PX_ENV_SKIP_DOTENV=1; for example, CALLER_VALUE=from-caller bin/px-mind receives the value from .env instead. This can silently replace test or operational overrides, so the legacy per-launcher loads need to be removed or made to honor the centralized loader's precedence and opt-out behavior.

Useful? React with 👍 / 👎.

Comment thread bin/px-env
# its own environment (test seams, the deploy gate). Existing values always win:
# a variable already exported by the caller is not overwritten, so systemd,
# `env FOO=bar bin/px-x` and the test suite all keep behaving as before.
if [[ "${PX_ENV_SKIP_DOTENV:-0}" != "1" && -f "$PROJECT_ROOT/.env" ]]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Load dotenv before deriving px-env defaults

When .env sets LOG_DIR or PYTHONPATH, this block runs only after px-env has already assigned and exported defaults for those names. The save loop consequently mistakes the generated defaults for caller-supplied values and restores them after sourcing, so a manual launcher ignores these .env settings even though a systemd unit receives them before ExecStart. Snapshot the entry environment or load .env before deriving these defaults so only genuine caller values take precedence.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant