Skip to content

fix(dream-cli): double-quote tmpdir in gpu_reassign RETURN trap#1007

Merged
Lightheartdevs merged 1 commit intoLight-Heart-Labs:mainfrom
yasinBursali:fix/gpu-reassign-return-trap
Apr 27, 2026
Merged

fix(dream-cli): double-quote tmpdir in gpu_reassign RETURN trap#1007
Lightheartdevs merged 1 commit intoLight-Heart-Labs:mainfrom
yasinBursali:fix/gpu-reassign-return-trap

Conversation

@yasinBursali
Copy link
Copy Markdown
Contributor

@yasinBursali yasinBursali commented Apr 23, 2026

What

_gpu_reassign registered its cleanup trap with single-quoted outer
syntax:

trap 'rm -rf "$tmpdir"' RETURN

Single-quoted outer defers $tmpdir expansion until trap fire time.
RETURN traps are process-level (not function-scoped), so after
_gpu_reassign returns to cmd_gpu the trap fires again in
cmd_gpu's scope where $tmpdir is unbound. Under set -u that
triggers tmpdir: unbound variable and exit 1 even on a successful
reassign.

How

Switched to double-quoted outer so $tmpdir is baked in at
trap-set time:

trap "rm -rf '$tmpdir'" RETURN

Single-quoted inner continues to protect paths containing spaces.
Matches the already-landed pattern at dream-cli:542.

Platform Impact

  • macOS Apple Silicon / AMD / CPU: unaffected — the nvidia-smi
    early-return at line 2825 precedes the trap registration.
  • Linux NVIDIA: fixed.
  • Windows WSL2 + NVIDIA passthrough: fixed (same Bash semantic).
  • Windows WSL2 + AMD / Linux AMD Lemonade: unaffected (no
    nvidia-smi, same early-return).

Testing

  • bash -n passes.
  • shellcheck shows the same SC2064 on line 2869 as on the
    precedent line 542 — intentional (we want expansion at set time).
  • Bash repro: bash -c 'set -u; f(){ local t; t=$(mktemp -d); trap "rm -rf \"$t\"" RETURN; }; g(){ f; return 0; }; g' → exit 0.

Merge order (important)

This fix is preventive on upstream/main today: dream-cli is
on set -e only, so the unbound-variable crash does not manifest
yet. It becomes load-bearing the moment #1002
(refactor/dream-cli-nounset-audit, enables set -u) merges —
without this fix in place, dream gpu reassign on NVIDIA Linux
(and Windows WSL2 + NVIDIA passthrough) exits 1 even on a
successful reassign.

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

Known follow-up

The sibling trap at dream-cli:651 has the same wrong quoting
style but does not leak (it is dispatched from the main case
block, not from a function). Tracked as a minor consistency
follow-up outside this PR.

Manual test (requires NVIDIA Linux host)

dream gpu reassign --dry-run && echo okok, exit 0.

_gpu_reassign registered a cleanup trap with single-quoted
outer syntax, so $tmpdir expansion was deferred until fire
time. RETURN traps are process-level (not function-scoped),
so when the function returns to cmd_gpu the trap fires again
in cmd_gpu's scope where $tmpdir is unbound. Under set -u
that triggers 'tmpdir: unbound variable' and exit 1 even
on successful reassign.

Switched to double-quoted outer to bake $tmpdir in at
trap-set time (single-quoted inner keeps paths with spaces
safe). Matches the already-landed pattern at dream-cli:542.

macOS Apple Silicon and AMD paths are unaffected (the
nvidia-smi early-return at line 2825 precedes the trap
registration). Linux NVIDIA and Windows WSL2 + NVIDIA
passthrough are the affected surface.
@Lightheartdevs Lightheartdevs merged commit 3351c56 into Light-Heart-Labs:main Apr 27, 2026
26 of 27 checks passed
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