feat: structured error output (closes #47)#81
Merged
Conversation
Adds a typed error surface to every raid failure: stable code string, category that maps to one of five exit codes (1 generic / 2 config / 3 task / 4 network / 5 not-found), optional hint, and code-specific detail fields. Codes are part of raid's CLI contract — new codes ship additively; existing codes never change name or category. Implementation: - New public package src/raid/errs: Error interface, Category enum, Code constants, ExitCode, EmitJSON, AsError, Wrap, and per-code constructors. Pure alias façade over the internal impl. - New internal package src/internal/lib/errs: RaidError concrete type + 25 per-code constructors + Newf escape hatch for preserve-wording cases. Single newRaidError construction point. - src/cmd/raid.go: --json is now a persistent flag on rootCmd (replaces per-command --json on doctor / env / env list / profile list / context). Central error handler in executeRoot walks errs.AsError to set the categorical exit code; emits errs.EmitJSON to stderr when --json is active, plain text + hint otherwise. - Every fmt.Errorf in src/internal/lib (lib.go, repo.go, profile.go, env.go, command.go, task_runner.go, lock.go, config.go) rewritten to call the appropriate constructor. Message text preserved verbatim so existing substring-matching tests keep passing. - src/cmd/install/install.go drops log.Fatalf in favour of returning the error; src/cmd/profile/profile.go drops os.Exit; src/cmd/doctor moves to RunE returning a CategoryConfig error when findings include errors. - MCP handlers (raid_install / raid_env_switch / raid_run_task) emit structured JSON via NewToolResultError so agents can pivot on code / category instead of parsing prose, with captured stdout/stderr as a separate `output` field. Docs: - New /docs/references/errors page with code table, exit-code table, JSON shape, MCP integration shape, and stability promise. - README gains an Exit codes section. - llms.txt adds the new reference page. - whats-new entry under 0.13.0. Version 0.12.0-beta → 0.13.0-beta. Coverage: src/raid/errs 100%, src/internal/lib/errs 100%, src/cmd 95.1%, src/cmd/* ≥89%, src/internal/lib 93.5%. ~25 prior tests that asserted on error substrings continue to pass; tests that exercised os.Exit / log.Fatalf via subprocess have been simplified to direct error assertions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #81 +/- ##
==========================================
+ Coverage 91.20% 92.06% +0.86%
==========================================
Files 33 36 +3
Lines 2978 3303 +325
==========================================
+ Hits 2716 3041 +325
- Misses 170 171 +1
+ Partials 92 91 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a structured error system for Raid, adding stable error codes + categories (mapped to exit codes) and a root-level --json flag so automation can reliably branch on errors without parsing free-form text.
Changes:
- Add a public error façade (
src/raid/errs) backed by an internal implementation (src/internal/lib/errs) with stable codes, categories, optional hints, and JSON emission helpers. - Update CLI root execution to emit structured JSON errors when
--jsonis present and to map structured categories to stable exit codes. - Sweep internal/lib + cmd packages to return structured errors (and update tests/docs accordingly), plus MCP server tool handlers to return JSON-shaped tool errors.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/resources/app.properties | Bump version to 0.13.0-beta. |
| src/raid/errs/errs.go | Public re-export surface for structured errors (types, constants, helpers, constructors). |
| src/raid/errs/errs_test.go | Coverage/contract tests ensuring the public façade routes to the internal implementation. |
| src/internal/lib/task_runner.go | Convert task execution errors to structured errors with stable codes/categories. |
| src/internal/lib/repo.go | Convert repo validation/clone errors to structured errors. |
| src/internal/lib/profile.go | Convert profile CRUD/load/validation errors to structured errors. |
| src/internal/lib/lock.go | Convert mutation lock acquisition errors to structured errors. |
| src/internal/lib/lib.go | Convert install/schema validation paths to structured errors and aggregate clone failures. |
| src/internal/lib/env.go | Convert env selection/execution errors to structured errors. |
| src/internal/lib/config.go | Convert config file creation errors to structured errors. |
| src/internal/lib/command.go | Convert command dispatch errors to structured errors. |
| src/internal/lib/errs/raid_error.go | Core structured error type, category mapping, JSON emission, wrapping, and helpers. |
| src/internal/lib/errs/raid_error_test.go | Unit tests for the structured error core behavior (JSON shape, exit-code mapping, etc.). |
| src/internal/lib/errs/constructors.go | Per-code constructors providing stable codes/categories and optional hints/details. |
| src/cmd/raid.go | Root --json flag + centralized error handling (JSON emission + categorical exit codes). |
| src/cmd/raid_test.go | Tests for --json argument detection logic. |
| src/cmd/profile/profile.go | Convert raid profile <name> to RunE returning structured errors (no os.Exit). |
| src/cmd/profile/profile_test.go | Update tests to assert returned errors instead of subprocess exit behavior. |
| src/cmd/profile/list.go | Replace per-command --json with root persistent --json lookup at runtime. |
| src/cmd/install/install.go | Convert install command to RunE returning structured errors (no log.Fatalf). |
| src/cmd/install/install_test.go | Update tests to assert returned errors instead of subprocess exit behavior. |
| src/cmd/env/list.go | Replace per-command --json with root persistent --json via helper. |
| src/cmd/env/env.go | Replace per-command --json with root persistent --json; return structured errors for invalid usage/not-found. |
| src/cmd/env/env_test.go | Reset cobra flag state between tests; update assertions for structured error returns. |
| src/cmd/doctor/doctor.go | Convert doctor to RunE, use root persistent --json, and return structured config errors on findings. |
| src/cmd/doctor/doctor_test.go | Remove subprocess tests; assert structured errors and exit-code mapping instead. |
| src/cmd/context/serve.go | MCP tool handlers now return structured JSON tool errors (tool, code, category, etc.). |
| src/cmd/context/serve_test.go | Update MCP tests to expect structured JSON tool errors. |
| src/cmd/context/context.go | Replace per-command --json with root persistent --json. |
| src/cmd/context/context_test.go | Update tests to reflect --json being a root persistent flag. |
| site/docs/whats-new.mdx | Add release note entry describing structured errors and --json. |
| site/docs/references/errors.mdx | New error reference page (exit codes, code table, JSON shape, MCP shape, stability promise). |
| README.md | Document exit code categories and --json structured error output. |
| llms.txt | Link to the new errors reference documentation. |
| func installRepo(repo Repo) error { | ||
| if err := CloneRepository(repo); err != nil { | ||
| return fmt.Errorf("failed to clone repository '%s': %w", repo.Name, err) | ||
| return liberrs.Newf(liberrs.CodeCloneFailed, liberrs.CategoryNetwork, "failed to clone repository '%s': %v", repo.Name, err) |
Comment on lines
499
to
505
| err := CloneRepository(repo) | ||
| if semaphore != nil { | ||
| <-semaphore | ||
| } | ||
| if err != nil { | ||
| cloneErrs <- fmt.Errorf("failed to clone repository '%s': %w", repo.Name, err) | ||
| cloneErrs <- liberrs.Newf(liberrs.CodeCloneFailed, liberrs.CategoryNetwork, "failed to clone repository '%s': %v", repo.Name, err) | ||
| } |
Comment on lines
162
to
169
| if err := rootCmd.Execute(); err != nil { | ||
| // Subprocess exits keep their own status — preserve prior behavior | ||
| // for Shell / Script task failures so $? matches what the user's | ||
| // command returned. | ||
| var exitErr *exec.ExitError | ||
| if errors.As(err, &exitErr) { | ||
| return exitErr.ExitCode() | ||
| } |
Comment on lines
+154
to
+160
| func Newf(code string, category Category, format string, args ...any) *RaidError { | ||
| var cause error | ||
| // Last arg is a cause if it's an error and the format ends with %w/%v. | ||
| if n := len(args); n > 0 { | ||
| if e, ok := args[n-1].(error); ok { | ||
| cause = e | ||
| } |
| path := lockPath() | ||
| if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { | ||
| return nil, fmt.Errorf("create raid config dir: %w", err) | ||
| return nil, liberrs.Newf(liberrs.CodeLockFailed, liberrs.CategoryGeneric, "create raid config dir: %v", err) |
…-json + ExitError, lock hint
- lib.installRepo/Install: pass through CloneRepository's structured
error instead of re-wrapping it as CLONE_FAILED/Network, which
misclassified GIT_NOT_INSTALLED / REPO_NOT_CLONED and dropped their
hints/details.
- Add CloneFailedMulti aggregate that uses errors.Join so each per-repo
failure stays unwrap-walkable, and surface per-repo {code, category,
message, repo, url} via details for JSON consumers.
- raid.go: a structured error that wraps exec.ExitError (e.g.
TASK_SHELL_FAILED) no longer silently bypasses the --json / hint
output path. We now emit the structured error first, then return the
subprocess exit code; the bare-ExitError fast-path stays for plain
subprocess failures.
- lock.go: route MkdirAll failure through LockFailed(...) so every lock
acquisition failure carries the standard ~/.raid/.lock hint.
- errs.Newf: documentation now matches the implementation (trailing
error arg is always captured as the wrapped cause).
Co-Authored-By: Copilot <copilot@github.com>
Owner
Author
|
Auto-review by meeseeks Updates pushed: 1 commit
Copilot comments addressed: 5 of 5
Skipped: 0 Codecov patch: 86.05% (project 90.73%) — within 8pt tolerance ✅ |
Adds targeted tests for the new code paths in this PR and trims dead branches that were inflating uncovered-line count: - TestExecuteRoot_structuredErrorRouting drives the central error handler through real subcommands in both text and JSON mode, hitting the categorical exit-code routing, the structured stderr emission, the hint-line emission, and errs.EmitJSON. Restores rootCmd's persistent --json flag after the test so the global cobra state doesn't leak into TestExecute_inProcess_nonInfoCommand. - TestMcpStructuredError_emitsJSON / _plainErrorWrappedAsUnknown / _reservedDetailKeysIgnored / _marshalFallback exercise every branch of the MCP error helper, including the json.Marshal failure path via a test-only unmarshalableError type that plants a channel in Details(). - TestJsonModeFromRoot_handlesDetachedCmd and TestJsonMode_handlesDetachedCmd cover the bare-cmd case (cmd.Root() on a detached command returns itself; GetBool on an unregistered flag returns the zero value). - Drops the dead `if root == nil` branches in jsonModeFromRoot / jsonMode / profile/list / context/context — cmd.Root() never returns nil, and GetBool already does the right thing when the flag isn't registered. Removing the dead checks both trims uncovered lines and matches what the tests actually exercise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov reported 58 missing patch lines on a498688. The biggest gaps were two new functions introduced during the rebase (CloneFailedMulti + joinErrors) plus the LockFailed / Newf wraps around os.MkdirAll in AcquireMutationLock and newVarsWatcher. Adds: - TestJoinErrors: empty / single / multi-element behaviour, plus the errors.Is round-trip that proves errors.Join is reachable. - TestCloneFailedMulti: empty causes (count 0), typed cause that propagates repo+url into the failures[] detail block, plain cause that doesn't, and the aggregate errors.Is round-trip. - TestAcquireMutationLock_mkdirAllFails: drives the LockFailed wrap by pointing LockPathOverride at a path whose parent is a regular file, asserts code == LOCK_FAILED. - TestNewVarsWatcher_mkdirFailureReturnsStructuredError: same trick for the fsnotify watcher's MkdirAll wrap; asserts the structured message contains the new "ensure vars watch dir" prefix. All four target patch lines that codecov flagged on the prior commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #47 — Structured error output. Every raid failure now carries a stable error code, a category that maps to one of five exit codes, and an optional hint. Pass
--json(now a persistent flag on rootCmd, working on every command) and errors come back as a single line of{\"error\": {\"code\": \"...\", \"category\": \"...\", \"message\": \"...\", \"hint\": \"...\"}}on stderr — agents pivot on the code instead of parsing prose.```bash
$ raid profile nonexistent
raid: profile 'nonexistent' not found
hint: Run `raid profile list` to see registered profiles.
$ echo $?
5
$ raid --json profile nonexistent
{"error":{"category":"not-found","code":"PROFILE_NOT_FOUND","hint":"Run `raid profile list` to see registered profiles.","message":"profile 'nonexistent' not found","profile":"nonexistent"}}
$ echo $?
5
```
Exit codes: `1` generic / `2` config / `3` task / `4` network / `5` not-found. Subprocess exits (e.g. a Shell task's `exit 7`) keep their own code.
Package layout
Public-vs-internal split per the project convention:
Future implementations (richer details, plugin-supplied errors) slot into the internal package and satisfy the same public interface — call sites never change.
Scope
Docs
Test plan
Out of scope (deferred)
🤖 Generated with Claude Code