Skip to content

refactor(dream-cli): guard .env grep pipelines against pipefail kill on missing keys#1008

Merged
Lightheartdevs merged 1 commit intoLight-Heart-Labs:mainfrom
yasinBursali:refactor/dream-cli-post-pipefail-hygiene
Apr 27, 2026
Merged

refactor(dream-cli): guard .env grep pipelines against pipefail kill on missing keys#1008
Lightheartdevs merged 1 commit intoLight-Heart-Labs:mainfrom
yasinBursali:refactor/dream-cli-post-pipefail-hygiene

Conversation

@yasinBursali
Copy link
Copy Markdown
Contributor

@yasinBursali yasinBursali commented Apr 23, 2026

What

Seven grep "^KEY=" .env | cut | tr pipelines in dream-cli would
exit 1 when the key is absent. Under set -eo pipefail those
pipelines would kill the script before the downstream defensive
checks ([[ -n ... ]], ${var:-(not set)}) can handle the
empty-on-miss case the surrounding code was written for.

Why

The surrounding code treats missing keys as benign (empty string,
"not set" default, fallthrough to defaults). That contract held
pre-pipefail because a grep miss yielded an empty pipeline output
with exit 0. Post-pipefail, a grep miss propagates exit 1, and
because these pipelines are assigned with bare VAR=$(...) (not
local VAR=$(...), which would mask the exit status), set -e
kills the script mid-flow.

How

Appended || true to each of the seven pipelines:

line function key read
254 _check_version_compat DREAM_VERSION
836 cmd_dry_run DREAM_VERSION
849 cmd_dry_run DASHBOARD_API_KEY
938 cmd_dry_run loop model-related keys
953 cmd_dry_run loop (2) model-related keys
1434 cmd_enable GPU_BACKEND
1781 cmd_preset DREAM_MODE

Line 254 additionally swaps | head -1 for | sed -n '1p'
SIGPIPE-safe under pipefail, portable across BSD and GNU sed.

Line 1434 additionally picks up a missing 2>/dev/null for
consistency with the other six sites.

Six other grep "^KEY=" pipelines in the file (around lines 2082,
2084–2086, 2163–2164) use the single-line local VAR=$(...) form,
which masks the pipeline exit status (a local command always
exits 0 regardless of RHS) — those sites are already safe and are
intentionally out of scope.

Testing

  • bash -n passes.
  • shellcheck diff against base: zero new warnings.
  • Behavior confirmed under set -eo pipefail:
    • grep '^NOPE=' file | cut -d= -f2 | tr -d '[:space:]' || truerc=0, empty value (missing key).
    • grep '^FOO=' file | cut -d= -f2 | tr -d '[:space:]' || truerc=0, value returned (present key).
  • sed -n '1p' works identically on BSD (macOS) and GNU (Linux)
    sed; no platform hazard.

Merge order (important)

This refactor is preventive on upstream/main today: dream-cli
is on set -e only, so a grep miss in any of these pipelines is
silently absorbed. It becomes load-bearing the moment #998
(fix/dream-cli-pipefail-exit-codes, enables set -o pipefail)
merges — without this fix in place, dream update, dream rollback, dream enable, dream preset, and dream dry-run can
abort mid-flow on installs that predate (or manually-edited-out) a
given .env key.

Recommended ordering: merge this PR in the same batch as
#998, or land this first. If #998 merges alone, users on upstream
hit the regression immediately.

Platform Impact

  • macOS / Linux / Windows WSL2: POSIX grep, cut, tr, sed -n '1p' all behave identically. Zero platform risk.

📋 Chain status (operator's reminder — apr-25)

This PR is the head of a dream-cli strict-mode chain:

#1008 (this) ──READY──▶ #998 ──DRAFT─▶ #1002 ──DRAFT
                              └──────▶ #1018 ──DRAFT

After this PR merges, the next operator step is:

  1. Promote fix(dream-cli): pipefail + surface LLM failures + exit-code contract #998 from DRAFT → ready (gh pr ready 998).
  2. Maintainer merges fix(dream-cli): pipefail + surface LLM failures + exit-code contract #998.
  3. Operator rebases refactor(dream-cli): enable set -u and add guards for conditional variables #1002 onto post-fix(dream-cli): pipefail + surface LLM failures + exit-code contract #998 main (shared hunks drop out automatically) → promote refactor(dream-cli): enable set -u and add guards for conditional variables #1002.
  4. Operator does the same for test(dream-cli): BATS regression shield for 5 dream-cli / supporting behaviors #1018 (after its other 4 deps land — fix(dream-cli): schema-driven secret masking + macOS Bash 4 validation #994, fix(dashboard,dashboard-api): sentinel-based setup wizard success detection #1003, fix(dream-cli): Apple GPU output polish + compose wrapper SIGINT/zero-match #1016, plus fix(dream-cli): pipefail + surface LLM failures + exit-code contract #998 itself).

#998 / #1002 / #1018 are kept in DRAFT as a mechanical safeguard so they cannot merge before this one.

Seven 'grep "^KEY=" .env | cut | tr' pipelines in dream-cli
currently emit exit 1 when the key is absent. Under set -eo
pipefail these pipelines would kill the script before the
downstream defensive checks ([[ -n ... ]], :-(not set)) can
handle the empty-on-miss case the surrounding code was
written for.

Appended '|| true' to each pipeline at:
  - line 254  _check_version_compat   (DREAM_VERSION)
  - line 836  cmd_dry_run             (DREAM_VERSION)
  - line 849  cmd_dry_run             (DASHBOARD_API_KEY)
  - line 938  cmd_dry_run loop        (model-related keys)
  - line 953  cmd_dry_run loop (2)    (model-related keys)
  - line 1434 cmd_enable              (GPU_BACKEND)
  - line 1781 cmd_preset              (DREAM_MODE)

Line 254 additionally swaps '| head -1' for
'| sed -n '1p'' (BSD/GNU-portable, SIGPIPE-safe under
pipefail). Line 1434 additionally picks up a missing
2>/dev/null for consistency with the other six sites.

Preventive hardening — becomes load-bearing once set -o
pipefail lands in the CLI.
@Lightheartdevs Lightheartdevs merged commit e86d78c into Light-Heart-Labs:main Apr 27, 2026
26 of 27 checks passed
yasinBursali added a commit to yasinBursali/DreamServer that referenced this pull request Apr 29, 2026
Maintainer audit on PR Light-Heart-Labs#998 (Lightheartdevs, 2026-04-28) flagged that
under `set -euo pipefail`, `_check_version_compat` must tolerate a
`.env` lacking `DREAM_VERSION` and fall back to `.version` /
`manifest.json`. The audit-required form is the trailing `|| true`
on the DREAM_VERSION grep pipeline:

    _COMPAT_INSTALLED_VER=$(grep '^DREAM_VERSION=' "$INSTALL_DIR/.env" 2>/dev/null \
        | sed -n '1p' | cut -d= -f2 | tr -d '[:space:]' || true)

The rebase onto current main (which carries Light-Heart-Labs#1008's tolerance fix)
preserved this form via conflict resolution; this test locks it in.

`tests/test-dream-cli-version-compat-pipefail.sh` (4 cases, wired
into `make test`):

1. dream-cli runs under strict mode (set -e or stricter).
2. The DREAM_VERSION grep pipeline ends with `|| true` (matched against
   active code; comment lines stripped before grep so the rationale
   block can't satisfy the assertion on its own).
3. `.version` and `manifest.json` fallback branches still exist (they
   are the destinations the `|| true` enables reaching).
4. Anti-regression: the bare-close form (no trailing `|| true`)
   does NOT appear — catches a future "cleanup" PR removing the guard.

Behavioural test was attempted but rejected: `_check_version_compat`
depends on `warn`/`log`/`_manifest_field`/`_semver_*` and several
file-scope color vars, so isolating it requires too much harness
setup. The dream-cli BATS suite (Light-Heart-Labs#1018) carries the behavioural
coverage at the right layer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants