feat(supervisor): add --json flag to supervisor status - #601
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe supervisor status command now accepts ChangesSupervisor status reporting
Supervisor plumbing cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant StatusCommand
participant IpcClient
participant Supervisor
User->>StatusCommand: Run supervisor status [--json]
StatusCommand->>IpcClient: Request supervisor status
IpcClient->>Supervisor: GetWebUrl
Supervisor-->>IpcClient: WebUrl with optional URL
IpcClient-->>StatusCommand: Status data
StatusCommand-->>User: Human-readable or JSON output
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds a
Confidence Score: 5/5Safe to merge; the changes are additive, backward-compatible, and cleanly scoped to the new --json flag and web URL reporting. The new IPC request is handled gracefully on older supervisors, the URL derivation from the actual bound address is correct, the three-state status logic is sound, and the two dead-code removals are verified safe. Files Needing Attention: No files require special attention. Important Files Changed
Reviews (4): Last reviewed commit: "fix(status): report state file read erro..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/cli/supervisor/status.md (1)
8-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the JSON fields.
Please mention that the output contains
status("up"/"down") and an optionalweb_uiURL, ideally with a short example. “Output in JSON format” does not define the machine-readable contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/cli/supervisor/status.md` around lines 8 - 12, Expand the `--json` section in the Flags documentation to define the JSON contract: include the required `status` field with `"up"` or `"down"` values and the optional `web_ui` URL field. Add a concise example showing the expected JSON output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/cli/supervisor/status.md`:
- Around line 8-12: Expand the `--json` section in the Flags documentation to
define the JSON contract: include the required `status` field with `"up"` or
`"down"` values and the optional `web_ui` URL field. Add a concise example
showing the expected JSON output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 9009c5c9-925a-48aa-a62b-3130c9e24b0d
📒 Files selected for processing (12)
docs/cli/commands.jsondocs/cli/index.mddocs/cli/supervisor.mddocs/cli/supervisor/status.mdpitchfork.usage.kdlsrc/cli/json_output.rssrc/cli/supervisor/status.rssrc/ipc/client.rssrc/ipc/mod.rssrc/supervisor/ipc_handlers.rssrc/web/mod.rssrc/web/server.rs
332fc7f to
0c5856f
Compare
|
This PR currently has merge conflicts. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
|
This PR currently has merge conflicts. If this continues for 7 days, it will be closed automatically. This is warning day 2 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
Adds a --json flag to `pitchfork supervisor status` that prints
{"status": "up"|"down"}, including "web_ui": "<url>" when the web
UI server is running.
The URL is queried from the running supervisor over IPC via a new
GetWebUrl request, so it reflects the server's actual bound address
(port bumping, settings changed since supervisor start) rather than
static config. Older supervisors that don't recognize the request are
treated as having no web UI instead of erroring. The non-JSON output
also gains a "Web UI: <url>" line when the web UI is up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0c5856f to
cff301c
Compare
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/supervisor/status.rs`:
- Around line 41-44: Update the status logic around existing_supervisor_pid() so
lookup errors are preserved and reported as “unknown” rather than converted to
“down.” Distinguish an error from a successful absent PID, and only classify a
confirmed missing or non-running PID as “down”; retain the existing
PROCS.is_running(pid) check for valid PIDs.
- Around line 22-23: Update the status flow around ipc.get_web_url() to
distinguish Ok(None) from Err: keep the older-supervisor compatibility result as
“no Web UI,” but propagate or report get_web_url() errors so JSON status
includes the error and human-readable mode exits unsuccessfully. Remove
unwrap_or_default() and preserve the existing Web UI logging for successful
URLs.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 054e6dfe-19bb-4640-8307-2a4b631d0eaf
📒 Files selected for processing (13)
docs/cli/commands.jsondocs/cli/index.mddocs/cli/supervisor.mddocs/cli/supervisor/status.mdpitchfork.usage.kdlsrc/cli/json_output.rssrc/cli/supervisor/status.rssrc/ipc/client.rssrc/ipc/mod.rssrc/supervisor/ipc_handlers.rssrc/supervisor/mod.rssrc/web/mod.rssrc/web/server.rs
🚧 Files skipped from review as they are similar to previous changes (10)
- docs/cli/index.md
- pitchfork.usage.kdl
- docs/cli/supervisor.md
- docs/cli/commands.json
- src/supervisor/ipc_handlers.rs
- src/ipc/mod.rs
- src/ipc/client.rs
- src/web/mod.rs
- src/cli/json_output.rs
- docs/cli/supervisor/status.md
Dead code that failed clippy with -D warnings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
get_web_url() already maps the older-supervisor compatibility case to Ok(None); a returned Err is a genuine transport or protocol failure. Propagate it in human-readable mode so the command exits unsuccessfully, and report it in the JSON output's error field instead of swallowing it with unwrap_or_default(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
existing_supervisor_pid() fails when the state file cannot be read or parsed; collapsing that error with .ok().flatten() misreported it as a confirmed-down supervisor. Only a successful lookup finding no PID, or a PID that is not running, now classifies as "down" — a lookup error is reported as "unknown" with the failure in the error field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
2 participants
Summary
Adds a
--jsonflag topitchfork supervisor status:{"status": "up"}or{"status": "down"}depending on whether the supervisor is reachable (exit code 0 either way, so it's pipe-friendly forjq). If the IPC connection fails while the supervisor process is still alive (e.g. permission denied on the socket), it reports{"status": "unknown", "error": "..."}instead of a misleadingdown."web_ui": "<url>"only when the web UI server is actually running.GetWebUrlrequest rather than read from static config, so it reflects the server's actual bound address — correct even when the port was auto-bumped or settings changed after the supervisor started. The web server records its real URL (actual port, bind address, base path) at bind time; unspecified addresses like0.0.0.0render aslocalhost.GetWebUrlrespond withError, which is treated as "no web UI" instead of failing (same pattern asSyncMdns/ReloadConfig).Web UI: <url>line when the web UI is up.Testing
mise run ci-devpasses (fmt, clippy, render, 275 e2e tests).http://127.0.0.1:3121).Contributor note
This PR was completely generated by a coding agent. I don't have deep experience with Rust but I also accept if this PR doesn't clear the quality bar and gets closed.
This PR was generated by Claude Code.
🤖 Generated with Claude Code
Summary by CodeRabbit
--jsonflag tosupervisor status, returning JSON withstatus(up,down, orunknown) and, when available, the web interface URL plus any related error details.supervisor statusto still show the web interface URL when it can be determined.supervisor statusto include the optional--jsonflag.