feat(dream-cli): --json flag on list/status and document doctor --json#1000
feat(dream-cli): --json flag on list/status and document doctor --json#1000yasinBursali wants to merge 3 commits intoLight-Heart-Labs:mainfrom
Conversation
|
Audit follow-up: needs a clean-JSON regression before merge. The |
'dream list --json' and 'dream status --json' previously were silently
dropped as unknown args — scripts piping to jq got parse errors. The
main case dispatch forwarded no arguments to cmd_list / cmd_status,
so any flag passed was ignored. Add explicit --json parsing:
- cmd_list --json emits a JSON array of {id, category, status}.
- cmd_status --json delegates to the existing cmd_status_json.
- Unknown flags on these subcommands now error out cleanly (exit 1).
- cmd_help mentions 'doctor --json' which was previously undocumented,
and advertises the new 'list --json' / 'status --json' flags.
'dream status-json' remains as a hyphenated alias for one release.
Platform impact: identical on macOS / Linux / Windows (WSL2) — no
platform branching.
…JSON when registry warnings emit Maintainer audit on PR Light-Heart-Labs#1000 (Lightheartdevs, 2026-04-28) asked for a regression proving `dream list --json` remains parseable JSON when PyYAML/registry loading warnings hit stderr post-Light-Heart-Labs#1006 (which moved log()/warn() to stderr). `tests/test-dream-list-json-clean-stdout.sh` (5 cases, wired into `make test`): 1. Hermetic scaffold — copies dream-cli + minimal lib (service-registry, safe-env, python-cmd) into a tempdir so SCRIPT_DIR resolves there and EXTENSIONS_DIR isolates from the live repo. 2. Plants two manifests: - extensions/services/valid-svc/manifest.yaml (well-formed) - extensions/services/broken-svc/manifest.yaml (missing required `service.id` — triggers `# SKIP: ... missing required "id" field` on stderr from sr_load's Python parser). 3. Runs `NO_COLOR=1 DREAM_HOME=$tempdir $tempdir/dream-cli list --json` with stdout and stderr captured separately. 4. Asserts: a. exit 0 (sr_load tolerates the broken manifest) b. stdout parses as JSON (python3 json.loads) c. JSON contains valid-svc and excludes broken-svc d. stderr contains the `# SKIP:` diagnostic — proving the warning fired during the same invocation e. stdout has no stderr-style content (no SKIP, no ⚠, no [dream], no ANSI escape) — the jq-pipeline contract. Skips gracefully if PyYAML or python3 are unavailable on the host. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ptional cp CG follow-ups on the regression test: 1. The stderr SKIP-diagnostic check now greps for the literal `# SKIP:` prefix lib/service-registry.sh emits (lines 117-151), not the bare substring `SKIP` — matches the assertion message and removes a future false-positive risk if some unrelated code path emits a line containing `SKIP` for a different reason. 2. The "no ANSI escape on stdout" check used `\\033\[` in a `grep -qE` pattern, which matches the literal four-character string `\033[` not real ANSI escape bytes (0x1B). The test happened to pass because `NO_COLOR=1` blanks all colour vars so no ESC bytes ever land on stdout — but the assertion was overstated. Replace with a bash `[[ ... =~ ... ]]` test using `$'\x1b['` (literal ESC byte) for the ANSI portion, keep the `# SKIP:` / `^⚠` / `^[dream]` token greps in `grep -qE`. 3. The optional-helper copies (`safe-env.sh`, `python-cmd.sh`) used `2>/dev/null || true` which violates project CLAUDE.md style. Switch to `[[ -f X ]] && cp X` so a missing-and-needed lib still surfaces instead of being swallowed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
46df764 to
8d2b549
Compare
|
Pushed audit follow-up ( Rebased onto current New
Local CG returned APPROVED WITH WARNINGS. All three labelling-strength warnings folded into CG flagged two coverage gaps as non-blocking follow-ups: a |
The `_docker_cmd_arr: returns sudo docker when DOCKER_CMD is sudo docker` test in `tests/bats-tests/docker-phase.bats` was failing in every CI run on `upstream/main`, blocking the integration-smoke job across all 12 currently-open PRs. Root cause: the test stub at line 94 (mirroring the real function at `installers/phases/05-docker.sh:29`) does `echo "sudo" "docker"`, which emits a single space-joined line `"sudo docker\n"`. The assertion at line 100 expected `$'sudo\ndocker'` — two newline-separated lines. That output never matched; the test had been silently red on main. The real function's consumer at `installers/phases/05-docker.sh:36` correctly word-splits the single-line output via `local -a cmd=($(_docker_cmd_arr))`, so the function works in production. The bug was assertion-only. Fix: change `assert_output $'sudo\ndocker'` to `assert_output 'sudo docker'`. Inline comment captures the single-line + word-splitting contract so a future contributor doesn't "fix" it back to a two-line assertion. Verified: `tests/bats/bats-core/bin/bats tests/bats-tests/docker-phase.bats` — case 2 now passes (was the only one failing in CI; local case 1 fail is environmental — no Docker daemon on dev machine — and unrelated). Unblocks `integration-smoke` for the 12 open PRs (#974, #994, #998, #1000, #1015-1057 etc.) that all base on upstream/main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Audit pass (2026-05-02): |
Lightheartdevs
left a comment
There was a problem hiding this comment.
This is closer than the first audit: the JSON flag is useful, warn() now writes to stderr via current main, and the branch adds a regression for dream list --json keeping stdout parseable while registry diagnostics go to stderr.
I can't merge it yet because the branch is stale/dirty against current main:
- GitHub marks it
DIRTY. - A local merge simulation conflicts in
dream-server/Makefile. - The added regression script syntax-checks, but in this Windows/Git Bash environment it skipped because the
python3command available to the script did not have PyYAML. Please make sure the CI path actually runs the new test rather than skipping it silently.
Please rebase/resolve the Makefile conflict and verify the JSON stdout regression runs in CI. After that, this should be a straightforward re-review.
What
dream list --jsonanddream status --jsonnow work as advertised. Previously the--jsonflag was silently dropped because the main case dispatch forwarded no arguments to the subcommands. Also documents the previously-undocumenteddream doctor --json, keepsdream status-jsonas a back-compat hyphenated alias.Plus an audit follow-up: a regression test proving
dream list --jsonstays valid JSON on stdout even when registry/schema diagnostics fire on stderr.Why
Scripts piping
dream list/dream statusoutput tojqgot parse errors because the subcommands emitted human-readable output regardless of any flag passed. The human-readable hyphenated aliasdream status-jsonwas the only JSON-emitting path;--jsonappeared to work (no error) but emitted the wrong format.Once the
--jsonplumbing is fixed, the next concern (raised by maintainer audit) is that the JSON contract must hold even when registry loading emits diagnostics. After PR #1006 movedlog()/warn()to stderr, the producer/consumer separation is in place — the regression locks it in.How
Functional fix (commit
af9acf0f)"$@"to bothcmd_listandcmd_status.cmd_list --jsonemits a JSON array of{id, category, status}.cmd_status --jsondelegates to the existingcmd_status_json. The delegation is wrapped in a subshell (( cmd_status_json )) to isolate the RETURN trap set insidecmd_status_json— without the subshell, the trap would leak intocmd_status's frame and crash with an unbound-variable error when the sibling nounset PR lands on top.cmd_helpadvertises the new flags and the existingdoctor --json.Audit follow-up (commits
6203c819+8d2b5493)New
dream-server/tests/test-dream-list-json-clean-stdout.sh(5 cases, wired intomake test):dream-cli+ minimallib/(service-registry.shrequired, plussafe-env.sh/python-cmd.shif present) into a tempdir. Becausedream-cliderivesSCRIPT_DIRfromBASH_SOURCE[0], andlib/service-registry.shderivesEXTENSIONS_DIR="${SCRIPT_DIR:-$(pwd)}/extensions/services", the test is fully isolated from the live repo's extensions tree.extensions/services/valid-svc/manifest.yaml(well-formed).extensions/services/broken-svc/manifest.yamlwithschema_version: dream.services.v1but missingservice.id— the canonical trigger forlib/service-registry.sh:127to emit# SKIP: <path>: missing required "id" fieldto stderr.NO_COLOR=1 DREAM_HOME=$tempdir $tempdir/dream-cli list --jsonwith stdout and stderr captured separately.python3 json.loads.valid-svcand excludesbroken-svc.# SKIP:diagnostic — proves the warning fired during the same invocation that produced the JSON.# SKIP:/⚠/[dream]/ ANSI escape (literal0x1B) bytes — the jq-pipeline contract.python3or PyYAML are unavailable on the host.CG-follow-up tightenings in
8d2b5493:# SKIP:literal prefix (was bareSKIP), real$'\x1b['ANSI ESC byte (was\\033\[literal-string match), and[[ -f X ]] && cp X(wascp X 2>/dev/null || true— violated project CLAUDE.md|| trueban).Testing
make lintPASS.make testPASS — new test runs at the end of the suite.shellcheckclean on the new test file.Review
Local CG APPROVED WITH WARNINGS on the regression. All three labelling-strength warnings were folded into commit
8d2b5493before push:# SKIP:literal prefix instead of bareSKIPsubstring.\033[string match.[[ -f ]] && cpinstead ofcp ... 2>/dev/null || true.CG flagged two coverage gaps as non-blocking follow-ups (not addressed here):
cmd_status --jsonregression — same shape but exercises the subshell-isolatedcmd_status_jsonpath. Worth a sibling test in a follow-up.# SKIP: missing id); a malformed-YAML manifest would trigger a different warn shape (warn "Service registry: manifest parser failed"). Assertion (e) would still catch leakage from either.Known Considerations
cmd_status_json's internal RETURN trap (single-quoted,$tmpresolved at fire-time) is the latent root cause and exists pre-PR on upstreammaintoo — dormant there because upstream currently has onlyset -e. The subshell wrap in this PR defensively hardens the new caller path without modifying the pre-existing function (the wider trap fix belongs with the nounset PR that addsset -u).dream status-jsonhyphenated alias is kept for one release for back-compat.Platform Impact
mktemp -d,cp,python3,bash— portable across the supported platforms.