feat(cli): prettify status output with table, links, and --json flag#490
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the user experience of the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
- Parse docker compose ps JSON into a formatted table with service name, state icon, health, image tag, and status columns - Add --json flag to status for raw JSON output (default is table) - Parse health check response into a one-line summary instead of raw JSON - Add dashboard, API docs, and health check links to status output - Add dashboard and API docs links to doctor output - Add release notes link to update output after successful CLI update - Add dashboard link after successful container restart in update - Improve doctor bug report instructions (attach diagnostic file) - Remove raw container JSON dump from doctor FormatText (summary replaces it) - Add UI.Table(), UI.Link(), UI.Writer() methods to the ui package
- Split printContainerStates and printHealthStatus under 50 lines - Warn on NDJSON parse failures instead of silent skip - Only show Persistence on healthy (2xx) responses - Thread --json flag as parameter instead of global var - Guard against zero-value health data - Refactor doctor.go to use *ui.UI for consistent styled output - Add tests for imageTag, healthIcon, parseContainerJSON, Table, Link - Remove dual raw/out writer params (use out.Writer()) - Preallocate rows slice
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces direct CLI stdout with a UI abstraction across commands (doctor, status, update), adds UI rendering APIs (Section, Link, Table, Writer), introduces a doctor report subcommand, improves status health/container parsing with optional JSON output, adjusts diagnostics formatting, updates Docker healthchecks, and adds tests and deps. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI
participant UI
participant Backend
participant FS
CLI->>UI: start command (status / doctor / doctor report)
CLI->>Backend: GET /containers (NDJSON) and GET /health
Backend-->>CLI: container lines, health JSON
CLI->>CLI: parseContainerJSON(), fetchHealth(), renderHealthSummary()
alt doctor_report
CLI->>FS: save diagnostics archive
FS-->>CLI: saved path
end
CLI->>UI: render Section, Table, Link, Key/Value, Hints (or raw JSON)
UI-->>User: formatted output / links / JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly refactors the CLI's output and diagnostic commands by introducing a new ui package for standardized user interface elements. The doctor command now uses the new UI for collecting and presenting diagnostics, including generating GitHub issue URLs and displaying relevant links. The status command has been enhanced with a --json flag for raw output, and its display of Docker container states and health checks has been revamped to use structured tables and more detailed summaries, supported by new helper functions and unit tests. Additionally, the update command now provides release notes and dashboard links post-update. Review comments suggest improving consistency in doctor.go by reusing an existing UI instance for error output, refining the default icon for unknown container health states in status.go, enhancing health check error messages for better debugging, and simplifying a conditional check in the health status rendering logic.
| errOut := ui.NewUI(cmd.ErrOrStderr()) | ||
| errOut.Warn(fmt.Sprintf("Could not save diagnostic file: %v", err)) |
There was a problem hiding this comment.
It would be more readable to use the out variable instead of creating a new ui.UI instance. This aligns with the existing pattern in this function and promotes consistency.
| errOut := ui.NewUI(cmd.ErrOrStderr()) | |
| errOut.Warn(fmt.Sprintf("Could not save diagnostic file: %v", err)) | |
| errOut := out | |
| errOut.Warn(fmt.Sprintf("Could not save diagnostic file: %v", err)) |
| if state == "restarting" { | ||
| return ui.IconWarning | ||
| } | ||
| return ui.IconError |
There was a problem hiding this comment.
Returning IconError as a default case might be misleading. Consider returning an empty string or a more descriptive icon to indicate an unknown state.
| return ui.IconError | |
| return "" // Or a more appropriate icon for unknown state |
| body, statusCode, err := fetchHealth(ctx, state.BackendPort) | ||
| if err != nil { | ||
| _, _ = fmt.Fprintf(out, " Backend: error creating request (%v)\n", err) | ||
| out.Error(err.Error()) |
There was a problem hiding this comment.
Returning after printing the error makes sense, but the error message itself could be improved. Consider including the specific error from err.Error() in the output for better debugging.
| out.Error(err.Error()) | |
| out.Error(fmt.Sprintf("Health check failed: %v", err)) |
| if json.Unmarshal(body, &envelope) != nil || envelope.Data.Status == "" { | ||
| out.Warn(fmt.Sprintf("Health: unparseable response (HTTP %d)", statusCode)) | ||
| return |
There was a problem hiding this comment.
The condition envelope.Data.Status == "" seems redundant after checking json.Unmarshal(body, &envelope) != nil. If unmarshalling fails, envelope.Data.Status will likely be an empty string anyway. Consider simplifying the condition.
| if json.Unmarshal(body, &envelope) != nil || envelope.Data.Status == "" { | |
| out.Warn(fmt.Sprintf("Health: unparseable response (HTTP %d)", statusCode)) | |
| return | |
| if json.Unmarshal(body, &envelope) != nil { | |
| out.Warn(fmt.Sprintf("Health: unparseable response (HTTP %d)", statusCode)) | |
| return | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/internal/ui/ui.go`:
- Around line 121-161: The table alignment uses len() (byte length) to compute
column widths in UI.Table and then formats with fmt.Fprintf, which mismeasures
multibyte Unicode (e.g., ✓). Replace byte-length math with terminal display
width: import github.com/mattn/go-runewidth and use runewidth.StringWidth(...)
when computing widths (in the headers loop and when scanning rows into widths)
and use runewidth.FillRight(cell, w) (or runewidth.FillLeft/Fill) when emitting
each cell inside printRow so the printed cell occupies the desired display
width; also build the separator using strings.Repeat("─", w) after widths are
computed. Update references: widths slice, UI.Table, printRow, sep.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ebf4b6c-268f-4a31-abad-ddb01daa57ac
📒 Files selected for processing (8)
cli/cmd/doctor.gocli/cmd/status.gocli/cmd/status_test.gocli/cmd/update.gocli/internal/diagnostics/collect.gocli/internal/diagnostics/collect_test.gocli/internal/ui/ui.gocli/internal/ui/ui_test.go
💤 Files with no reviewable changes (1)
- cli/internal/diagnostics/collect.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Lint CLI Go code with golangci-lint and go vet; test with go test -race; check vulnerabilities with govulncheck
Files:
cli/internal/ui/ui.gocli/internal/diagnostics/collect_test.gocli/internal/ui/ui_test.gocli/cmd/doctor.gocli/cmd/status_test.gocli/cmd/update.gocli/cmd/status.go
🧠 Learnings (1)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/doctor.gocli/cmd/update.gocli/cmd/status.go
🧬 Code graph analysis (5)
cli/internal/diagnostics/collect_test.go (1)
cli/internal/diagnostics/collect.go (1)
ContainerDetail(24-29)
cli/internal/ui/ui_test.go (1)
cli/internal/ui/ui.go (1)
NewUI(57-69)
cli/cmd/doctor.go (2)
cli/internal/ui/ui.go (1)
NewUI(57-69)cli/internal/diagnostics/collect.go (2)
Collect(56-80)Report(32-53)
cli/cmd/status_test.go (1)
cli/internal/ui/ui.go (4)
IconSuccess(26-26)IconError(35-35)IconInProgress(29-29)IconWarning(32-32)
cli/cmd/status.go (1)
cli/internal/ui/ui.go (6)
NewUI(57-69)UI(43-52)IconSuccess(26-26)IconError(35-35)IconInProgress(29-29)IconWarning(32-32)
🔇 Additional comments (18)
cli/internal/ui/ui.go (2)
71-72: LGTM!Simple accessor exposing the underlying writer for direct output when needed.
116-119: LGTM!The
Linkmethod properly sanitizes both label and URL usingstripControlto prevent terminal escape sequence injection.cli/internal/ui/ui_test.go (1)
58-105: LGTM!Good test coverage for the new UI methods. Tests verify:
Linkrenders label and URL correctlyTableproduces header, rows, and separator- Empty headers produce no output
Writer()returns the underlying buffercli/cmd/update.go (2)
83-85: LGTM!Release notes URL is correctly constructed. The
TrimPrefixensures consistent tag formatting regardless of whether the version string includes a "v" prefix.
291-291: LGTM!Dashboard link provides a convenient clickable URL after successful restart.
cli/internal/diagnostics/collect_test.go (1)
43-45: LGTM!Test correctly updated to use the new
ContainerSummaryfield withContainerDetailentries, and validates the renamed "Container Summary" section in the formatted output.Also applies to: 53-53
cli/cmd/status.go (8)
17-17: LGTM!Clean integration of the UI abstraction and
--jsonflag. The ignored error fromGetBoolis acceptable since the flag was registered in the sameinit()function.Also applies to: 29-29, 36-36, 43-43
91-97: LGTM!
imageTagcorrectly extracts the tag portion after the last colon, with a sensible fallback returning the full string if no colon is present.
99-114: LGTM!
healthIconprovides clear visual indicators for container states with appropriate icon mappings. The fallback toIconErrorfor unknown states is a safe default.
116-133: LGTM!Good implementation with failure counting for parse warnings. Empty lines are properly skipped, and the caller receives both parsed containers and failure count to decide whether to warn the user.
135-151: LGTM!Row slice is preallocated with correct capacity. Health label defaults to "-" when empty for consistent table rendering.
219-236: LGTM!
fetchHealthimplements proper context propagation, reasonable timeout (5s), and response body limiting (64KB) to prevent memory exhaustion from large/malicious responses.
238-257: LGTM!
renderHealthSummarycorrectly guards against zero-value health data by checking both unmarshal success and non-empty status. Persistence is only displayed for healthy 2xx responses as intended.
259-264: LGTM!Clean link output for dashboard, API docs, and health endpoints.
cli/cmd/status_test.go (1)
9-70: LGTM!Comprehensive test coverage for the new helper functions:
TestImageTagcovers various image string formats including edge casesTestHealthIconvalidates all state/health combinationsTestParseContainerJSONverifies both successful parsing and failure countingcli/cmd/doctor.go (3)
28-31: LGTM!Clean refactor to use the UI abstraction. The unused args parameter is properly marked with
_.
48-53: LGTM!Proper separation of stdout/stderr using distinct UI instances. Success messages go to stdout, warnings to stderr.
71-99: LGTM!Well-implemented URL builder with:
- Proper URL encoding via
url.QueryEscape- Conservative URL length guard (3500 chars) with graceful fallback to shorter body
- Correct label encoding (
%%3Aproduces%3Awhich decodes to:)
- Fix web Dockerfile healthcheck: localhost → 127.0.0.1 (localhost doesn't resolve in Alpine containers, causing permanent "unhealthy") - Pretty-print health JSON in doctor diagnostic report instead of dumping raw single-line JSON - Format uptime as human-readable "3h 36m" instead of raw "12991s" - Add formatUptime test (6 cases)
- Use runewidth.StringWidth for Table column alignment (fixes Unicode icon misalignment with multi-byte chars like ✓, ✗, ●) - Handle both JSON array (Compose v2.21+) and NDJSON in container parsing — prevents "No containers running" on newer Compose - Fix imageTag for registry:port/image (no tag) — guard against port colon being mistaken for tag separator - Fix backend Dockerfile healthcheck: localhost → 127.0.0.1 (same IPv6 resolution issue as web, now all 3 Dockerfiles consistent) - Remove unnecessary ui.NewUI allocation in doctor error path - Restore docker stats error warning (was silently swallowed) - Add test case for tagless registry image
- Fix DepartmentFactory flaky test: set reporting_lines=() to prevent polyfactory from generating random ReportingLine with subordinate == supervisor (confirmed stable over 50 runs) - Move Recent Logs section to top of doctor diagnostic report so raw logs appear before the structured summary sections
Aureliolo
force-pushed
the
feat/cli-status-ux
branch
from
586c93f to
840f994
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #490 +/- ##
=======================================
Coverage 93.36% 93.36%
=======================================
Files 480 480
Lines 23202 23202
Branches 2215 2215
=======================================
Hits 21662 21662
Misses 1216 1216
Partials 324 324 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Replace plain-text doctor dump with styled *ui.UI rendering: colored section headers, green/red per-container health status, clean config key-value display, no raw JSON - Remove logs from doctor terminal output (use 'synthorg logs') - Add UI.Section() method for bold section headers - Add 'synthorg doctor report' subcommand for bug reporting: collects diagnostics, saves archive, generates full GitHub issue URL with environment table, container summary, persistence/memory backend, image tag, and sandbox config - Fix flaky DepartmentFactory test (polyfactory generating subordinate == supervisor collision)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/cmd/status.go (1)
43-68:⚠️ Potential issue | 🔴 Critical
--jsonmode still writes human-formatted output.This path always emits version info, Docker/Compose key-values, resource usage, health text, and links through the UI layer. That makes stdout invalid JSON and breaks the new flag’s machine-readable contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/cmd/status.go` around lines 43 - 68, The status command always emits human-readable UI output which breaks --json; update cmd/status.go to detect jsonOut and short-circuit to JSON-only output: after determining safeDir and detecting docker (functions/symbols: safeStateDir, docker.Detect), if jsonOut is true, do not call ui.NewUI or the human printers (printVersionInfo, out.KeyValue prints, printContainerStates, printResourceUsage, printHealthStatus, printLinks); instead gather the same data those functions produce and marshal a single JSON object to stdout (or invoke existing JSON-capable helpers if available) and return, ensuring no other human-formatted text is written to stdout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/doctor_report.go`:
- Line 20: The command description in the Cobra command's Long field currently
states it "opens a pre-filled GitHub issue URL in the browser" but the
implementation only prints the URL; update the Long string in
cli/cmd/doctor_report.go (the Cobra command's Long field for this command) to
accurately say it prints or outputs a pre-filled GitHub issue URL (e.g., "prints
a pre-filled GitHub issue URL to the console") so the description matches
behavior.
In `@cli/cmd/doctor.go`:
- Around line 109-119: The code copies c.State into health when c.Health is
empty, causing outputs like "running (running)"; change the logic so health
remains empty if c.Health is empty (do not assign c.State into health), and
update the rendering: keep the existing cases for health == "healthy" and for
unhealthy/exited, but in the default branch when health == "" print only c.State
(e.g., out.Warn(fmt.Sprintf("%-24s %s", c.Name, c.State))) and when health is
non-empty keep the current "%-24s %s (%s)" format; adjust the switch/conditions
around the health variable and the out.Warn/out.Error/out.Success calls
(references: variable health, fields c.Health and c.State, and the
out.Warn/out.Error/out.Success calls).
In `@cli/cmd/status_test.go`:
- Around line 61-69: The test currently indexes containers[0] after a non-fatal
length check; change the length assertion to a fatal assertion (use
t.Fatalf/t.Fatal) or otherwise guard the access so that parseContainerJSON
failure cannot cause a panic when reading containers[0]; update the test around
parseContainerJSON, the len(containers) check and the subsequent access to
containers[0].Service to ensure the test aborts immediately on zero-length
results.
In `@cli/cmd/status.go`:
- Around line 172-175: The jsonOut branch currently prints psOut verbatim which
can be NDJSON on older Compose versions; instead call parseContainerJSON(psOut)
to parse NDJSON (or single JSON) into a canonical Go structure and then
re-encode that structure as a single JSON value before writing to w (replace the
fmt.Fprintln(w, psOut) path). Ensure you handle parse errors from
parseContainerJSON and return or log them appropriately, and keep the
surrounding output logic (the "Containers:" header) consistent with the JSON
output path.
- Around line 255-267: The current logic treats any 2xx HTTP response as healthy
regardless of the health payload; change the success condition to require both a
2xx statusCode and a healthy payload by checking envelope.Data.Status
(hr.Status) instead of only HTTP code. Update the conditional around out.Success
and the Persistence display so it only runs when statusCode is 2xx AND hr.Status
indicates healthy (e.g., strings.EqualFold(hr.Status, "healthy") or the expected
success token used by the backend); otherwise fall through to the existing
else/unhealthy branch. Ensure the earlier unmarshal/presence check still runs
(json.Unmarshal into envelope and verify hr.Status exists) and that Persistence
is only printed from inside the new combined success block (reference variables:
envelope, hr, hr.Status, statusCode, out.Success, out.KeyValue).
In `@cli/internal/ui/ui.go`:
- Around line 127-174: The Table method writes raw header and cell strings
allowing terminal control injection; sanitize all header and cell values with
the existing stripControl function before measuring widths and before printing.
Specifically, inside UI.Table create local sanitized copies (e.g., sanHeaders
and sanRows) by calling stripControl on each header and each cell in rows, use
those sanitized values when computing widths (the widths slice) and in the
printRow closure (and when building sep), and leave the original input slices
unmodified; ensure every place that currently reads headers or row cells uses
the sanitized versions so no control characters are emitted.
---
Outside diff comments:
In `@cli/cmd/status.go`:
- Around line 43-68: The status command always emits human-readable UI output
which breaks --json; update cmd/status.go to detect jsonOut and short-circuit to
JSON-only output: after determining safeDir and detecting docker
(functions/symbols: safeStateDir, docker.Detect), if jsonOut is true, do not
call ui.NewUI or the human printers (printVersionInfo, out.KeyValue prints,
printContainerStates, printResourceUsage, printHealthStatus, printLinks);
instead gather the same data those functions produce and marshal a single JSON
object to stdout (or invoke existing JSON-capable helpers if available) and
return, ensuring no other human-formatted text is written to stdout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d7e65c9d-e8f4-4f4e-8873-c2a0373e9e27
📒 Files selected for processing (13)
cli/cmd/doctor.gocli/cmd/doctor_report.gocli/cmd/status.gocli/cmd/status_test.gocli/cmd/update.gocli/go.modcli/internal/diagnostics/collect.gocli/internal/diagnostics/collect_test.gocli/internal/ui/ui.gocli/internal/ui/ui_test.godocker/backend/Dockerfiledocker/web/Dockerfiletests/unit/core/conftest.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (Python 3.14)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Lint CLI Go code with golangci-lint and go vet; test with go test -race; check vulnerabilities with govulncheck
Files:
cli/internal/diagnostics/collect_test.gocli/internal/diagnostics/collect.gocli/internal/ui/ui.gocli/cmd/doctor.gocli/cmd/status.gocli/cmd/update.gocli/internal/ui/ui_test.gocli/cmd/doctor_report.gocli/cmd/status_test.go
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do not usefrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:syntax (no parentheses) per PEP 758 — ruff enforces this on Python 3.14
Add type hints to all public functions and classes; enforce mypy strict mode
Use Google-style docstrings on all public classes and functions — required and enforced by ruff D rules
Keep line length to 88 characters (ruff enforced)
Files:
tests/unit/core/conftest.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark unit tests with@pytest.mark.unit, integration tests with@pytest.mark.integration, e2e tests with@pytest.mark.e2e, and slow tests with@pytest.mark.slow
Useasyncio_mode = "auto"in pytest configuration — no manual@pytest.mark.asyncioneeded on async tests
Set 30-second timeout per test
Prefer@pytest.mark.parametrizefor testing similar cases
Use Hypothesis for property-based testing in Python (@given+@settings); useciprofile (200 examples, default) anddevprofile (1000 examples) controlled viaHYPOTHESIS_PROFILEenv var
Never skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; for timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic
Files:
tests/unit/core/conftest.py
cli/go.mod
📄 CodeRabbit inference engine (CLAUDE.md)
CLI requires Go 1.26+; dependencies in
cli/go.mod(Cobra, charmbracelet/huh, charmbracelet/lipgloss)
Files:
cli/go.mod
🧠 Learnings (5)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/internal/ui/ui.gocli/cmd/doctor.gocli/cmd/status.gocli/cmd/update.gocli/go.mod
📚 Learning: 2026-03-16T07:36:16.026Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:36:16.026Z
Learning: Applies to cli/go.mod : CLI requires Go 1.26+; dependencies in `cli/go.mod` (Cobra, charmbracelet/huh, charmbracelet/lipgloss)
Applied to files:
cli/cmd/update.gocli/go.mod
📚 Learning: 2026-03-16T07:36:16.026Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:36:16.026Z
Learning: Applies to cli/**/*.go : Lint CLI Go code with golangci-lint and go vet; test with go test -race; check vulnerabilities with govulncheck
Applied to files:
cli/cmd/update.gocli/go.mod
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
Applied to files:
docker/backend/Dockerfiledocker/web/Dockerfile
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
docker/backend/Dockerfiledocker/web/Dockerfile
🧬 Code graph analysis (4)
cli/internal/diagnostics/collect_test.go (1)
cli/internal/diagnostics/collect.go (1)
ContainerDetail(24-29)
cli/cmd/update.go (1)
cli/internal/version/version.go (1)
RepoURL(5-5)
cli/internal/ui/ui_test.go (1)
cli/internal/ui/ui.go (1)
NewUI(58-70)
cli/cmd/doctor_report.go (4)
cli/internal/ui/ui.go (1)
NewUI(58-70)cli/internal/config/state.go (1)
State(16-27)cli/internal/diagnostics/collect.go (2)
Collect(56-80)Report(32-53)cli/internal/version/version.go (1)
RepoURL(5-5)
🪛 GitHub Check: CodeQL
cli/cmd/doctor_report.go
[failure] 51-51: Uncontrolled data used in path expression
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
🔇 Additional comments (10)
docker/web/Dockerfile (1)
38-39: Healthcheck host normalization looks good.Using
127.0.0.1here is deterministic and avoidslocalhostresolver/IPv6 ambiguity in container environments.docker/backend/Dockerfile (1)
112-113: Backend healthcheck host change is correct.Switching the probe to
127.0.0.1keeps the check explicit and consistent with container-local health probing.cli/go.mod (1)
8-12: Direct dependency promotion looks correct.These additions are consistent with the new UI/table and update functionality and are cleanly declared as direct requirements.
tests/unit/core/conftest.py (1)
132-132: Factory update is aligned with the Department schema.Good addition to keep generated test objects structurally complete.
cli/cmd/update.go (1)
83-84: Release-notes and dashboard link output is implemented cleanly.The new post-update/post-restart guidance is clear and correctly formatted.
Also applies to: 291-291
cli/internal/diagnostics/collect.go (1)
127-137: Health body rendering fallback is robust.Nice improvement: pretty JSON when possible, with safe truncation fallback when it isn’t.
cli/internal/diagnostics/collect_test.go (1)
43-45: Test expectations correctly track the new report layout.The updated fixture data and section checks are aligned with the current FormatText output.
Also applies to: 53-53
cli/internal/ui/ui_test.go (1)
58-105: New UI helper tests add good coverage.This gives solid regression protection for the newly introduced
Link,Table, andWriterbehavior.cli/internal/ui/ui.go (1)
72-125: New UI helper methods are clean and cohesive.
Writer,Section, andLinkfit the UI abstraction well and keep output routing consistent.cli/cmd/doctor_report.go (1)
41-51: No action required. The code already implements proper path hardening—safeStateDir()callsconfig.SecurePath()which enforces canonicalization and absolute-path validation before the file write.
- Fix doctor_report Long field: says "prints" not "opens in browser" - Fix doctor container display: no more "running (running)" duplication when health is empty — show state only without repeating - Use t.Fatalf for container count assertion to prevent index panic - Canonicalize --json container output: parse NDJSON/array then re-encode as proper JSON array instead of raw dump - Check both HTTP status code AND hr.Status == "ok" for healthy - Sanitize table headers and cells with stripControl before measuring and printing to prevent terminal control injection
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/doctor.go`:
- Around line 108-118: The branch handling exited containers prints c.Health
which may be empty, causing a blank status; change the logic in the switch so
the branch that matches exited containers uses a non-empty fallback (e.g., use
c.State when c.Health == "") instead of printing c.Health directly: update the
case handling (the case testing c.State == "exited" / c.Health == "unhealthy")
or compute a status variable from c.Health || c.State and pass that to out.Error
(references: c.Health, c.State, out.Error, out.Warn, out.Step).
In `@cli/cmd/status_test.go`:
- Around line 56-71: Add a new unit test to cover JSON array input for
parseContainerJSON by creating a TestParseContainerJSON_Array (or
TestParseContainerJSON_Array) in cli/cmd/status_test.go that passes a JSON array
string like `[{"Name":"a",...},{"Name":"b",...}]` into parseContainerJSON,
asserts len(containers)==2, failures==0, and verifies containers[0].Service ==
"backend"; this mirrors the existing NDJSON test (TestParseContainerJSON) to
ensure array-format output from Compose v2.21+ is parsed correctly.
In `@cli/cmd/status.go`:
- Around line 276-288: The formatUptime function doesn't handle negative
seconds; detect when seconds < 0, record the sign (neg := seconds < 0), use
math.Abs(seconds) to compute a positive duration for d/h/m calculations, then
format the result the same way and prefix a "-" when neg is true; update
references inside formatUptime (seconds, d, h, m) accordingly and add the math
import if missing.
In `@cli/internal/ui/ui.go`:
- Around line 127-187: The Table function still allows newline and tab
characters through stripControl which can break alignment; add a small sanitizer
(e.g., sanitizeCell) that replaces "\n" and "\t" with a single space and apply
it to every header and cell after stripControl in the Table method (update where
sanHeaders and sanRows are populated) so headers and cells are both stripped of
control chars and normalized for stable column widths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31268206-3b22-4e80-9bc5-c8c612122761
📒 Files selected for processing (5)
cli/cmd/doctor.gocli/cmd/doctor_report.gocli/cmd/status.gocli/cmd/status_test.gocli/internal/ui/ui.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: CLI Test (macos-latest)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Lint CLI Go code with golangci-lint and go vet; test with go test -race; check vulnerabilities with govulncheck
Files:
cli/cmd/status_test.gocli/internal/ui/ui.gocli/cmd/doctor.gocli/cmd/doctor_report.gocli/cmd/status.go
🧠 Learnings (2)
📚 Learning: 2026-03-16T07:36:16.026Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:36:16.026Z
Learning: Applies to cli/**/*.go : Lint CLI Go code with golangci-lint and go vet; test with go test -race; check vulnerabilities with govulncheck
Applied to files:
cli/cmd/status_test.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/internal/ui/ui.gocli/cmd/doctor.gocli/cmd/status.go
🧬 Code graph analysis (2)
cli/cmd/doctor.go (3)
cli/internal/ui/ui.go (2)
NewUI(58-70)UI(44-53)cli/internal/config/state.go (2)
Load(52-90)State(16-27)cli/internal/diagnostics/collect.go (2)
Collect(56-80)Report(32-53)
cli/cmd/doctor_report.go (4)
cli/internal/ui/ui.go (1)
NewUI(58-70)cli/internal/config/state.go (2)
Load(52-90)State(16-27)cli/internal/diagnostics/collect.go (2)
Collect(56-80)Report(32-53)cli/internal/version/version.go (1)
RepoURL(5-5)
🔇 Additional comments (12)
cli/internal/ui/ui.go (1)
72-125: LGTM!The
Writer(),Section(), andLink()methods are well-implemented with proper sanitization of user inputs viastripControl().cli/cmd/doctor.go (2)
160-174: LGTM!Config rendering properly masks the JWT secret and exposes only non-sensitive configuration values. The helper functions are well-structured and use the UI abstraction consistently.
27-75: LGTM!The
runDoctorfunction is well-structured with proper error handling, safe file permissions (0o600), and consistent use of the UI wrapper for output.cli/cmd/doctor_report.go (2)
69-133: LGTM!The URL builder properly escapes all dynamic values with
url.QueryEscapeand includes sensible truncation logic for browser URL limits. The generated bug report template is well-structured.
41-53: Path validation is properly implemented throughconfig.SecurePath.The static analysis flag at line 51 is a false positive.
safeDiris secured byconfig.SecurePath(), which validates that paths are absolute and cleans them.filenameis generated fromtime.Now().Format(), not user input. The code is safe.cli/cmd/status.go (4)
172-181: LGTM!The JSON output is now properly canonicalized by parsing NDJSON/array inputs with
parseContainerJSONand re-encoding as a proper JSON array withjson.MarshalIndent. This addresses the previous review concern about NDJSON on older Compose versions.
255-274: LGTM!The health check now properly requires both HTTP 2xx status code AND
hr.Status == "ok"for healthy determination (Line 264). This addresses the previous review concern about degraded payloads being incorrectly marked as healthy.
91-99: LGTM!The
imageTagfunction correctly handles registry port edge cases (e.g.,registry:5000/imagewith no tag) by checking if the last colon appears before the last slash.
236-253: LGTM!The
fetchHealthfunction has good defensive practices: context-aware request, 5-second timeout, andio.LimitReaderto cap response body at 64KB preventing memory exhaustion from malicious responses.cli/cmd/status_test.go (3)
56-71: LGTM!The test now correctly uses
t.Fatalf(Line 63) for the container count assertion, preventing potential panics when accessingcontainers[0]. This addresses the previous review concern.
9-54: LGTM!The
TestImageTagandTestHealthIcontests have excellent coverage of edge cases including empty inputs, registry ports, and various state/health combinations.
73-92: LGTM!The
TestFormatUptimetest covers all formatting branches (seconds-only, minutes+seconds, hours+minutes) with well-chosen values including edge cases like 0 and 24 hours.
- Fix doctor exited container with empty health showing blank status - Add JSON array test for parseContainerJSON (Compose v2.21+) - Handle negative uptime in formatUptime (clock skew guard) - Sanitize table cells: collapse \n and \t to spaces for stable alignment
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/doctor.go`:
- Around line 131-136: The code currently derives availability by substring
matching strings.Contains(img, "available") which false-positives on
"unavailable" or "not available"; update the logic that processes r.ImageStatus
to use a structured availability flag from diagnostics (e.g., an Available bool
alongside the message) and call out.Success(message) or out.Error(message) based
on that flag, or if changing diagnostics is not possible, match an exact
token/prefix instead of a substring (e.g., parse img into a status token and
message or check for a leading "available" token) to determine success vs error
rather than using strings.Contains; update references to r.ImageStatus,
out.Success and out.Error accordingly.
In `@cli/cmd/status_test.go`:
- Around line 87-105: Add a regression case for negative uptime to
TestFormatUptime: update the tests table in TestFormatUptime to include an entry
with seconds set to a negative value (e.g., -1) and the expected string matching
the function's clamp behavior (e.g., "0s"); locate the table inside
TestFormatUptime and add the new test row so the subtest loop exercises
formatUptime(seconds < 0) and validates the explicit negative-value branch.
In `@cli/cmd/status.go`:
- Around line 43-45: The current status command always writes human-readable
output (via ui.NewUI, printVersionInfo and subsequent UI/print calls) even when
jsonOut is true; change the control flow so every human-readable section
(printVersionInfo, Docker/Compose metadata, resource usage, health output,
container links/arrays and any early-warning/progress branches) is only executed
when !jsonOut, and route any diagnostics or warnings to stderr in JSON mode so
stdout remains a single canonical JSON value; locate uses of jsonOut,
printVersionInfo, ui.NewUI and the early-warning branches in status.go and wrap
or redirect them accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1a2b10c0-4c16-4364-bca9-12e59c9d57ad
📒 Files selected for processing (4)
cli/cmd/doctor.gocli/cmd/status.gocli/cmd/status_test.gocli/internal/ui/ui.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Lint CLI Go code with golangci-lint and go vet; test with go test -race; check vulnerabilities with govulncheck
Files:
cli/cmd/status.gocli/internal/ui/ui.gocli/cmd/status_test.gocli/cmd/doctor.go
🧠 Learnings (1)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/status.gocli/internal/ui/ui.gocli/cmd/doctor.go
🧬 Code graph analysis (2)
cli/cmd/status.go (2)
cli/internal/config/state.go (2)
Load(52-90)State(16-27)cli/internal/ui/ui.go (6)
NewUI(58-70)UI(44-53)IconSuccess(27-27)IconError(36-36)IconInProgress(30-30)IconWarning(33-33)
cli/cmd/status_test.go (1)
cli/internal/ui/ui.go (4)
IconSuccess(27-27)IconError(36-36)IconInProgress(30-30)IconWarning(33-33)
🔇 Additional comments (1)
cli/internal/ui/ui.go (1)
127-194:UI.Tablelooks solid.Stripping control chars, collapsing
\n/\t, and sizing withrunewidthis the right combination here; it keeps wide glyphs aligned and prevents cell content from breaking the table layout.
| func TestFormatUptime(t *testing.T) { | ||
| tests := []struct { | ||
| seconds float64 | ||
| want string | ||
| }{ | ||
| {0, "0s"}, | ||
| {45, "45s"}, | ||
| {90, "1m 30s"}, | ||
| {3600, "1h 0m"}, | ||
| {12991, "3h 36m"}, | ||
| {86400, "24h 0m"}, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.want, func(t *testing.T) { | ||
| if got := formatUptime(tt.seconds); got != tt.want { | ||
| t.Errorf("formatUptime(%v) = %q, want %q", tt.seconds, got, tt.want) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add a regression case for negative uptime values.
formatUptime now has explicit handling for seconds < 0, but this table only exercises non-negative inputs. Adding one clock-skew case here would lock down the new branch and prevent silent regressions.
💚 Suggested test case
}{
{0, "0s"},
{45, "45s"},
+ {-90, "-1m 30s"},
{90, "1m 30s"},
{3600, "1h 0m"},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/cmd/status_test.go` around lines 87 - 105, Add a regression case for
negative uptime to TestFormatUptime: update the tests table in TestFormatUptime
to include an entry with seconds set to a negative value (e.g., -1) and the
expected string matching the function's clamp behavior (e.g., "0s"); locate the
table inside TestFormatUptime and add the new test row so the subtest loop
exercises formatUptime(seconds < 0) and validates the explicit negative-value
branch.
| out := ui.NewUI(cmd.OutOrStdout()) | ||
| printVersionInfo(out, state) | ||
|
|
There was a problem hiding this comment.
--json mode still writes human-readable output to stdout.
When jsonOut is true, this path still emits version info, Docker/Compose metadata, resource usage, health output, and links around the container array. That means synthorg status --json cannot be parsed as the single canonical JSON value promised by the flag. Please gate every human-readable section behind !jsonOut, and keep diagnostics off stdout in JSON mode as well.
📦 Suggested fix
out := ui.NewUI(cmd.OutOrStdout())
- printVersionInfo(out, state)
+ if !jsonOut {
+ printVersionInfo(out, state)
+ }
@@
- out.KeyValue("Docker", info.DockerVersion)
- out.KeyValue("Compose", info.ComposeVersion)
- _, _ = fmt.Fprintln(out.Writer())
-
- printContainerStates(ctx, out, info, safeDir, jsonOut)
- printResourceUsage(ctx, out, info, safeDir)
- printHealthStatus(ctx, out, state, jsonOut)
- printLinks(out, state)
+ if jsonOut {
+ printContainerStates(ctx, out, info, safeDir, true)
+ return nil
+ }
+
+ out.KeyValue("Docker", info.DockerVersion)
+ out.KeyValue("Compose", info.ComposeVersion)
+ _, _ = fmt.Fprintln(out.Writer())
+
+ printContainerStates(ctx, out, info, safeDir, false)
+ printResourceUsage(ctx, out, info, safeDir)
+ printHealthStatus(ctx, out, state, false)
+ printLinks(out, state)The same rule should also apply to the early warning branches so JSON mode never prints prose to stdout.
Also applies to: 52-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/cmd/status.go` around lines 43 - 45, The current status command always
writes human-readable output (via ui.NewUI, printVersionInfo and subsequent
UI/print calls) even when jsonOut is true; change the control flow so every
human-readable section (printVersionInfo, Docker/Compose metadata, resource
usage, health output, container links/arrays and any early-warning/progress
branches) is only executed when !jsonOut, and route any diagnostics or warnings
to stderr in JSON mode so stdout remains a single canonical JSON value; locate
uses of jsonOut, printVersionInfo, ui.NewUI and the early-warning branches in
status.go and wrap or redirect them accordingly.
- Use strings.HasSuffix(": available") instead of Contains("available")
to prevent false-positive on "unavailable" strings
- Add negative uptime regression test case (-90 → "-1m 30s")
🤖 I have created a release *beep* *boop* --- ## [0.3.0](v0.2.9...v0.3.0) (2026-03-16) ### Features * **cli:** prettify status output with table, links, and --json flag ([#490](#490)) ([61fa8af](61fa8af)) ### Bug Fixes * **api:** don't require password change after self-service setup ([#488](#488)) ([ba13e04](ba13e04)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
2 participants
Summary
docker compose psJSON into a formatted table with service, state icon, health, image tag, and status columns--jsonflag tosynthorg statusfor raw JSON output (default is human-readable table)statusoutputdoctoroutput with colored sections, per-container health status (green/red), clean config displaysynthorg doctor reportsubcommand — generates diagnostic archive + pre-filled GitHub issue URL with environment table and container summaryupdateafter successful CLI update*ui.UIfor consistent styled outputUI.Table(),UI.Link(),UI.Writer(),UI.Section()methodslocalhost→127.0.0.1for IPv6 compatibility)DepartmentFactorytest (polyfactorysubordinate == supervisorcollision)3h 36minstead of12991s)stripControlto prevent terminal injectionimageTagfor registry:port/image (no tag) edge caseReview coverage
Pre-reviewed by 4 agents + 3 external reviewers across multiple rounds:
Test plan
go vet/golangci-lint— cleango test ./...— all passingpytest— 8214+ passed (flaky test fixed)imageTag(6 cases),healthIcon(6 cases),parseContainerJSON,formatUptime(6 cases)Table,Link,Writer,TableEmpty,Sectionsynthorg status— formatted table with iconssynthorg doctor— styled colored outputsynthorg doctor report— generates valid GitHub issue URLrelease-as: 0.3.0