feat(doctor): add extension diagnostics and health reporting#360
Conversation
5e5b444 to
a1549f0
Compare
Lightheartdevs
left a comment
There was a problem hiding this comment.
Review: REQUEST CHANGES — Two bugs produce incorrect output
Blocking: local used outside a function
The extension diagnostics block uses local container=..., local issues=(), etc. at top-level scope. local outside a function is undefined behavior in Bash — some shells silently treat it as global, others error. Under set -euo pipefail, this is a latent crash.
Fix: Wrap the diagnostics block in a function (e.g., collect_extension_diagnostics). This also fixes the >30 line threshold.
Blocking: printf '%s' missing newline separator
Issues array built with printf '%s' "${issues[@]}" concatenates elements with no separator: "container_not_runningmissing_dependency:searxng". Needs printf '%s\n' for the tr '\n' ',' pipe to work. Multi-issue extensions produce malformed JSON.
Non-blocking:
2>/dev/nullondocker inspect— should be logged per CLAUDE.md- JSON construction via sed pipeline is fragile — consider
jqor building in the Python block - Tests are static grep only, won't catch the JSON bug
for dep in $deps— unquoted variable expansion subject to globbing
🤖 Reviewed with Claude Code
…ove error handling
|
Fixed the bash scoping and JSON formatting bugs Extension diagnostics now work correctly: Critical fixes:
Code improvements:
The JSON output is now properly formatted and the script won't crash on systems with different bash configurations. Kindly review again. Thanks. |
|
Fixed unbound variable error The Split the declaration and assignment: local GPU_BACKEND
GPU_BACKEND="${GPU_BACKEND:-nvidia}"This allows the default value to work correctly even when the outer variable is unset. The integration test should now pass. |
d59b4e5 to
6cdfb72
Compare
6cdfb72 to
d0bde91
Compare
|
Fixed dependency issue with SERVICE_GPU_BACKENDS PR #360 was trying to use Fix: Made the GPU backend compatibility check conditional: if [[ -v SERVICE_GPU_BACKENDS ]]; then
# Check GPU backend compatibility
fiThe extension diagnostics now work correctly whether or not PR #357 has been merged. Once #357 merges, the GPU backend checks will automatically activate. |
Lightheartdevs
left a comment
There was a problem hiding this comment.
Review — Good feature, want to verify scope.
Extends dream-doctor.sh with extension diagnostics — health checks, dependency validation, GPU backend compatibility. This is useful for debugging "why isn't my extension working" issues.
+226/-3, 2 files. Concept is sound. Would like to verify:
- Does the extension scan add significant time to
dream-doctor.sh? - Are the health check timeouts reasonable (won't hang on unresponsive services)?
- Does it handle the case where Docker isn't running?
Lightheartdevs
left a comment
There was a problem hiding this comment.
Approve — Good extension diagnostics addition.
What it adds
Extends dream-doctor.sh with a collect_extension_diagnostics() function that:
- Iterates non-core services from the service registry
- Checks container state via
docker inspect - Runs health endpoint checks with
curl --max-time 5(5s timeout — reasonable) - Validates GPU backend compatibility
- Checks dependency availability
- Builds JSON diagnostic array for each extension
Timeout/Docker-down handling
- Docker not running:
$DOCKER_DAEMON == "true"guard — skips container inspection entirely if Docker daemon isn't available. Extensions getcontainer_state: "unknown"andhealth_status: "unknown". - Health check timeout:
curl --max-time 5— 5 seconds per extension. With ~15 extensions max, worst case is ~75 seconds. Acceptable for a diagnostic tool. - Missing container:
docker inspectfailure →container_state: "not_found". Clean handling.
Report integration
- Adds
"extensions"array to doctor.json output - Summary counters:
extensions_total,extensions_healthy,extensions_issues - Autofix hints for each issue type (container not running, health check failed, GPU incompatible, missing dependency)
Test
98 lines of structural tests (grep-based) — not behavioral tests, but appropriate for verifying the diagnostic function exists and handles expected cases.
Minor nit (non-blocking)
The JSON construction using string concatenation ("{\"id\":\"$sid\"...") is fragile — a service ID with quotes in it would break the JSON. Not a real risk since service IDs are controlled, but jq would be safer.
LGTM.
Summary
Enhances
dream-doctor.shto analyze extension health, dependencies, and compatibility, providing actionable diagnostics for troubleshooting extension issues.Problem
Current doctor report focuses on system capabilities and preflight checks but doesn't analyze extension-specific issues:
Solution
docker inspectfor each extensionReport Schema Additions
{ "extensions": [ { "id": "comfyui", "container_state": "running", "health_status": "healthy", "issues": [] }, { "id": "perplexica", "container_state": "exited", "health_status": "unknown", "issues": ["container_not_running", "missing_dependency:searxng"] } ], "summary": { "extensions_total": 5, "extensions_healthy": 3, "extensions_issues": 2 } }Example Autofix Hints
Implementation Details
Testing
Impact
Related Work
Builds on PR #355 (container state validation) and PR #357 (GPU backend validation). Completes the extension operability diagnostic suite.