Skip to content

fix(dream-cli): Apple GPU output polish + compose wrapper SIGINT/zero-match#1016

Open
yasinBursali wants to merge 3 commits intoLight-Heart-Labs:mainfrom
yasinBursali:fix/dream-cli-polish
Open

fix(dream-cli): Apple GPU output polish + compose wrapper SIGINT/zero-match#1016
yasinBursali wants to merge 3 commits intoLight-Heart-Labs:mainfrom
yasinBursali:fix/dream-cli-polish

Conversation

@yasinBursali
Copy link
Copy Markdown
Contributor

⚠️ Draft — depends on #999 AND #1001 merging first

All four fixes live in code introduced by unmerged upstream PRs:

Once #999 and #1001 both merge, I'll rebase and the PR diff will reduce to just the delta shown below.

What

Four small polish fixes on top of #999 + #1001:

Our delta (post-deps-rebase view)

One file, two commits:

  • dream-server/dream-cli: +21/-9 lines across the 4 touched regions

Testing

  • bash -n dream-server/dream-cli — clean
  • shellcheck dream-server/dream-cli — zero NEW warnings in the delta
  • Pre-commit hooks clean
  • Verified Apple-branch header logic doesn't accidentally affect the non-apple gpu_count path ([[ GPU_BACKEND == "apple" ]] short-circuits before $gpu_count is evaluated)

Platform Impact

Known Considerations

Follow-up fork issue worth filing: the SIGINT trap currently only cleans up the log file without exiting. A cleaner idiom (trap 'rm -f "$_compose_log"; exit 130' INT or EXIT trap) would avoid a downstream log-path-to-deleted-file edge case. Not a regression vs. current behavior (previously no cleanup at all on INT) so out-of-scope for this PR.

@yasinBursali
Copy link
Copy Markdown
Contributor Author

Re-marking as draft as a mechanical merge-order safeguard.

This branch contains commits f60df964 and 98b05249 from #999 (Apple Silicon coverage). Merging this PR before #999 lands would bring #999's content in as a side-effect rather than on its own merit.

Merge order: #999 first, then this PR rebases clean and gets re-marked ready.

@Lightheartdevs
Copy link
Copy Markdown
Collaborator

Audit follow-up: keep draft / rescope after the split fixes.

The focused foundations this overlapped with are now merged (#1006, #1007, #1008, #1023). Please rebase on current main and reduce this branch to the remaining unique Apple GPU output polish / compose-wrapper behavior, with the strict-mode fixes preserved from main rather than duplicated.

yasinBursali and others added 3 commits April 29, 2026 04:16
…date

The four commands that shell out to `docker compose` (restart, start, stop,
update) previously let the raw per-container-state-transition output pass
through with no summary and no visible error banner. A failure like
"dream-llama-ready is unhealthy" was one line buried in 40+ lines of state
transitions.

Introduce `_compose_run_with_summary` that runs compose with `--progress
quiet`, captures stdout+stderr to a mktemp log, and either prints a
"<verb> — done" success line or a red "<verb> failed:" banner that
surfaces up to 20 lines matching error|unhealthy|failed|dependency and
points the operator at the preserved full log.

Wrap all four callers. For `cmd_update`, keep the existing `if ! …; error`
guard on both the pull and up-recreate calls so the actionable rollback
hint is preserved on top of the surfaced error context.

Use `cmd || rc=\$?` to capture the compose exit code (safe under `set -e`
and not reliant on `\$?` after `if…fi`, which is spec'd to 0 on the
not-taken branch). The grep | sed | head pipeline is safe under `set -e`
without pipefail — this branch does not have pipefail, so no SIGPIPE /
no-match exit concerns.

Platform impact:
- macOS / Linux / Windows (WSL2): identical — no platform branching.
  `docker compose --progress quiet`, `grep -iE`, `sed`, `head`, `mktemp`
  are all portable.
…ware header

cmd_status_json emitted gpu_cores as a JSON string via --arg. Switch
to --argjson with integer-or-null detection: numeric gpu_cores
become JSON integers, the literal "?" from system_profiler fallback
becomes JSON null.

dream gpu status header hardcoded "(0 GPUs)" derived from nvidia-smi.
Branch on GPU_BACKEND=apple to show "(1 integrated GPU)" instead.
Non-apple paths keep the existing behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… fallback

Ctrl-C during a compose operation leaked the mktemp log because there
was no INT/TERM trap. Add trap 'rm -f "$_compose_log"' INT TERM after
mktemp and restore with trap - INT TERM before each return.

The grep | sed | head pipeline exit code is 0 even when grep finds no
matches (head exits 0 on empty input), so the || warn fallback never
fired. Capture the pipeline output to _surfaced and branch on non-empty
to decide between printing matches or emitting the "no error keywords
matched" hint. Correct under set -e today, pipefail-safe going forward.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@yasinBursali yasinBursali force-pushed the fix/dream-cli-polish branch from 4cd57c9 to 0fc56ca Compare April 29, 2026 01:17
@yasinBursali
Copy link
Copy Markdown
Contributor Author

Pushed audit follow-up — rebased onto current upstream/main.

The audit said "rescope after the split fixes (#1006, #1007, #1008, #1023)" and "reduce this branch to the remaining unique Apple GPU output polish / compose-wrapper behavior, with the strict-mode fixes preserved from main rather than duplicated."

Rebase outcome: the 5 pre-rebase commits collapsed to 3 clean commits on top of current upstream/main. The two staging-merge commits (57899eac merge: fix/dream-cli-apple-silicon-coverage as staging base and 4916453b merge: fix/dream-cli-compose-summary-wrapper as staging base) became no-ops because fix/dream-cli-apple-silicon-coverage (#999) is now on main and fix/dream-cli-compose-summary-wrapper (#1001) was closed unmerged.

Final 3 commits:

Strict-mode preservation: dream-cli on current upstream/main already has set -eo pipefail from #1008. The 3 commits above don't touch that line; they add new code that operates correctly under it (the trap/return ordering in _compose_run_with_summary is pipefail-safe by construction, and the zero-match pattern in commit 3 was specifically updated to be safe under pipefail).

make lint green, bash -n clean, shellcheck reports no NEW warnings in the delta.

Re-marking ready — the audit ask is addressed and the PR no longer depends on #999/#1001 (one merged, one closed).

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