Skip to content

fix: report a clear cli error when no herdr server is running - #1963

Merged
ogulcancelik merged 3 commits into
herdrdev:masterfrom
season179:issue/1941-cli-server-not-running
Aug 1, 2026
Merged

fix: report a clear cli error when no herdr server is running#1963
ogulcancelik merged 3 commits into
herdrdev:masterfrom
season179:issue/1941-cli-server-not-running

Conversation

@season179

Copy link
Copy Markdown
Contributor

Current behavior

With no herdr server running, every socket CLI command fails with a raw debug string:

$ herdr workspace create --cwd /tmp --label test
Error: Os { code: 2, kind: NotFound, message: "No such file or directory" }

It reads like the --cwd path is invalid, which is exactly how #1941 was reported. A stale socket file (ConnectionRefused) and every other workspace/tab/pane/agent/worktree command produce the same raw output.

What this does

Dead-socket connect failures from the guarded CLI request path (ErrorKind::NotFound | ConnectionRefused — kind-based so Windows named pipes classify the same) map to a marker error carrying a server_not_running ErrorResponse, mirroring the existing protocol_guard pattern. The edge that surfaces the error prints it exactly once:

$ herdr workspace create --cwd /tmp --label test
{"id":"cli:workspace:create","error":{"code":"server_not_running","message":"no herdr server is running at /Users/me/.config/herdr/herdr.sock; run `herdr` to start one"}}

Exit codes are unchanged (server errors 1, syntax errors 2). Printing is deferred — the marker carries the response — so callers that intentionally recover from a dead server keep their behavior with nothing printed:

  • plugin commands still fall back to the offline registry (plugin list --json → offline result, exit 0, empty stderr)
  • herdr status still reports status: not running
  • agent start transport failures print exactly one error line
  • session stop messaging unchanged

The shared classifier replaces the private copy in cli/status.rs. No wire protocol change (CLI-emitted error, like protocol_mismatch) and no server auto-start behavior.

Validation

  • just check green on macOS (zig 0.15.2 via homebrew) except one pre-existing failure unrelated to this change: live_handoff_keeps_unmanaged_agent_name_bound_to_saved_session fails identically on clean master @ 1491b7d
  • two focused regression tests: dead-server mapping (request id, code, socket path in message, marker recognizable without string matching) and a classifier negative case
  • manually verified: dead socket → single JSON error + exit 1 across command groups; status graceful; positive path against a live server unchanged; plugin offline fallback behavior identical to v0.7.5

refs #1941

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

The CLI now converts missing or refused server sockets into structured server_not_running errors. Responses carry request and socket details, propagate through agent and plugin paths, and print once as JSON. Session tests cover missing and stale sockets.

Server-not-running error flow

Layer / File(s) Summary
Error contract and request mapping
src/cli.rs, src/cli/server_not_running.rs, src/cli/status.rs
The CLI creates socket-aware ErrorResponse values, wraps them in marker errors, and maps NotFound and ConnectionRefused failures.
Error propagation and output handling
src/cli/agent.rs, src/cli/plugin.rs, src/main.rs
Agent, plugin, and main CLI paths detect reported errors, support fallback behavior, and print the response once.
Session error integration tests
src/cli.rs, tests/cli/sessions.rs
Tests verify markers, request IDs, socket paths, JSON output, exit status, and session attach guidance.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant ServerSocket
  participant ErrorMapper
  participant Main
  participant ResponsePrinter
  CLI->>ServerSocket: send API request
  ServerSocket-->>CLI: return NotFound or ConnectionRefused
  CLI->>ErrorMapper: map socket failure
  ErrorMapper-->>CLI: return structured server_not_running error
  CLI->>Main: propagate reported response
  Main->>ResponsePrinter: serialize and print JSON once
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: reporting a clear CLI error when no herdr server is running.
Description check ✅ Passed The description directly explains the dead-socket error handling, preserved behavior, tests, and validation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@kangal-bot kangal-bot added coderabbit-review greptile-review Trigger Greptile review for contributor-approved pull requests labels Jul 28, 2026
@season179
season179 marked this pull request as ready for review July 28, 2026 04:50
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

The PR converts missing or stale local server socket connection failures into a deferred, structured server_not_running CLI response while preserving recovery behavior.

  • Adds a marker error carrying the structured response until the command-surfacing edge.
  • Applies the shared mapping to guarded and unchecked request paths.
  • Preserves plugin offline fallback, status reporting, and agent-specific error handling.
  • Adds session-aware dead-server CLI regression coverage.

Confidence Score: 5/5

The PR appears safe to merge with no actionable changed-code defects identified.

The new marker is consistently propagated or intentionally recovered, structured errors are emitted once at the CLI boundary, and socket overrides and named sessions receive appropriate startup guidance.

Important Files Changed

Filename Overview
src/cli.rs Centralizes dead-server classification and maps both guarded and unchecked local API requests to the deferred marker.
src/cli/server_not_running.rs Defines the marker, structured error response, and session-aware startup guidance.
src/main.rs Surfaces deferred server-not-running responses once at the top-level CLI boundary.
src/cli/plugin.rs Recognizes the marker as a recoverable connection failure so offline registry fallback remains silent.
src/cli/agent.rs Preserves structured single-line output for agent transport failures carrying the new marker.
src/cli/status.rs Reuses the shared dead-server classifier while retaining graceful not-running status output.
tests/cli/sessions.rs Adds end-to-end coverage for one-line JSON output and session-aware startup guidance on missing and stale sockets.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CLI socket request] --> B{Connect result}
    B -->|Success| C[Process server response]
    B -->|NotFound or ConnectionRefused| D[Create server_not_running marker]
    B -->|Other error| E[Propagate original I/O error]
    D --> F{Caller recovers?}
    F -->|Plugin fallback| G[Use offline registry silently]
    F -->|Status path| H[Report not running]
    F -->|No recovery| I[Print carried JSON once and exit 1]
Loading

Reviews (1): Last reviewed commit: "fix: make server-not-running guidance se..." | Re-trigger Greptile

Comment thread src/cli.rs
@ogulcancelik

Copy link
Copy Markdown
Collaborator

@season179 thanks for working on this—the clearer error and preserved offline fallbacks look good. before we merge, could you make the startup guidance session-aware? for example, herdr --session foo workspace create should tell the user to start or attach foo, rather than run the default herdr session. please also add a process-level cli test confirming a missing or stale socket produces exactly one server_not_running json line and exits with code 1. after that, please rebase onto the latest master and rerun the checks. thanks!

socket cli commands surfaced a raw io::Error debug string
(`Error: Os { code: 2, ... }`) when nothing was listening on the
api socket, which read like a bad --cwd path. map dead-socket
connect failures to a `server_not_running` json error carrying the
resolved socket path, printed once at the edge that surfaces the
error; recovering callers (plugin offline registry fallback, agent
start polling) recognize the marker and keep their existing
behavior.

refs herdrdev#1941
@season179
season179 force-pushed the issue/1941-cli-server-not-running branch from a9825d0 to a98bead Compare August 1, 2026 07:00
@kangal-bot kangal-bot added the ai-review Trigger automated AI reviews for pull requests admitted by the PR gate label Aug 1, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/cli.rs (1)

808-826: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consolidate api_client_error_to_io

Define it once as pub(super) in src/cli.rs, then call super::api_client_error_to_io(err) from src/cli/status.rs.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: aca24bc3-4535-40f9-aa1a-bca4f9ef89b5

📥 Commits

Reviewing files that changed from the base of the PR and between 26a7bc8 and a98bead.

📒 Files selected for processing (7)
  • src/cli.rs
  • src/cli/agent.rs
  • src/cli/plugin.rs
  • src/cli/server_not_running.rs
  • src/cli/status.rs
  • src/main.rs
  • tests/cli/sessions.rs

@ogulcancelik
ogulcancelik merged commit f99647b into herdrdev:master Aug 1, 2026
8 checks passed
@ogulcancelik

Copy link
Copy Markdown
Collaborator

ty!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Trigger automated AI reviews for pull requests admitted by the PR gate coderabbit-review greptile-review Trigger Greptile review for contributor-approved pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants