feat(cli): stable JSON for accept, check, onboard + stdout fix - #119
Merged
Conversation
Add --json support to accept, check, and onboard commands with standard JSON envelope. Fix stdout bleed: suppress cargo subprocess output when --json is set so machine-readable output is clean. Document exit code scheme in error.rs and format.rs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughCLI commands now emit standardized JSON envelopes ChangesStructured JSON output across CLI commands
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant Run as CLI dispatch
participant Check as run_check
participant Accept as run_accept_gate
participant Cmd as run_command
participant Formatter as format_json/format_findings
User->>Run: cairn check --json
Run->>Check: load project / detect blueprint
Check->>Check: compute findings, has_errors?
Check->>Formatter: format_json/findings -> envelope {command:"check",status,data}
Formatter-->>User: stdout JSON + exit code
User->>Run: cairn accept --json
Run->>Accept: run_accept_gate(change_id, json:true)
Accept->>Cmd: run_command("cargo build", quiet:true)
Cmd-->>Accept: GateStep
Accept->>Cmd: run_command("cargo clippy", quiet:true)
Cmd-->>Accept: GateStep
Accept->>Cmd: run_command("cargo fmt --check", quiet:true)
Cmd-->>Accept: GateStep
Accept->>Cmd: run_command("cargo test --workspace --locked", quiet:true)
Cmd-->>Accept: GateStep
Accept->>Formatter: format_json(findings,has_failed,has_blocked)
Formatter-->>User: stdout JSON + exit code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/mod.rs`:
- Around line 100-124: The current "no blueprint found" branch returns plain
text via ok(...) which breaks the --json contract; change it to emit a JSON
envelope when parsed.json is true. Implement a conditional around the final
return: if parsed.json { return ok_json(parsed.json, "No cairn.blueprint found.
...") } else { return ok(...) }, or alternatively construct and return the same
JSON envelope shape used by error_output(parsed.json, ...) so callers always get
machine-readable output when parsed.json is set; update references around ok and
error_output to use parsed.json and the same message text.
In `@tests/phase_7_7_ux_foundation.rs`:
- Around line 55-73: The test currently branches on stdout.starts_with('{') and
allows non-JSON output; change it to always parse and validate JSON by removing
the conditional and calling serde_json::from_str on result.stdout.trim()
unconditionally (e.g., bind stdout = result.stdout.trim(), then let parsed:
serde_json::Value = serde_json::from_str(stdout).expect("cairn check --json must
produce valid JSON");), then keep the assertions against parsed["command"] and
parsed["data"]["findings"] as-is and retain the stderr assert that it does not
contain "cairn lint --json".
🪄 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: CHILL
Plan: Pro Plus
Run ID: 3493c50b-2acb-4d87-9950-a66427d49da6
📒 Files selected for processing (7)
src/cli/accept.rssrc/cli/commands.rssrc/cli/format.rssrc/cli/mod.rssrc/error.rstests/kernel.rstests/phase_7_7_ux_foundation.rs
There was a problem hiding this comment.
Pull request overview
Adds/standardizes --json output for additional CLI commands (accept, check, onboard) and documents the project’s exit-code scheme, with a key fix to prevent subprocess output from corrupting JSON streams.
Changes:
- Implement JSON output for
checkandonboardusing a{command,status,data}envelope. - Add
--jsonsupport toaccept, including suppressing cargo subprocess stdout/stderr to keep JSON output clean. - Update tests and inline documentation for the new JSON behavior and exit-code taxonomy.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/phase_7_7_ux_foundation.rs | Updates the UX test to reflect that check now supports --json. |
| tests/kernel.rs | Adjusts onboard JSON assertions to validate the new envelope (command/status/data). |
| src/error.rs | Documents the CLI exit code scheme in crate-level docs. |
| src/cli/mod.rs | Wires accept JSON flag through and adds JSON envelope output for check. |
| src/cli/format.rs | Documents exit code semantics on err(...). |
| src/cli/commands.rs | Wraps onboard JSON payload in the standard {command,status,data} envelope. |
| src/cli/accept.rs | Adds JSON output for accept and suppresses subprocess output when JSON is enabled. |
Comments suppressed due to low confidence (1)
src/cli/mod.rs:123
- When
--jsonis set, thischeckearly-return still emits a human guidance string viaok(...), socairn check --jsonis not guaranteed to produce machine-readable JSON (violates the stated stable JSON envelope). Consider returning the standard{command,status,data}envelope here as well (e.g.,status:"error"with adata.messageordata.findingspayload) whenparsed.jsonis true.
return ok(
"No cairn.blueprint found. Inspection has nothing to look at.\n\
Run `cairn init` to scaffold a blueprint, then re-run `cairn check`.\n"
.to_owned(),
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+112
to
117
| if parsed.file.ends_with("cairn.blueprint") && root.join("cairn.dsl").exists() { | ||
| return error_output( | ||
| parsed.json, | ||
| "CAIRN_COMMAND_FAILED", | ||
| "no blueprint file was found; rename `cairn.dsl` to `cairn.blueprint`", | ||
| ); |
Comment on lines
+156
to
+163
| fn format_json(findings: &[VerificationFinding], has_failed: bool, has_blocked: bool) -> String { | ||
| let status = if has_failed { | ||
| "failed" | ||
| } else if has_blocked { | ||
| "blocked" | ||
| } else { | ||
| "passed" | ||
| }; |
Comment on lines
55
to
70
| // When a blueprint exists in the CWD (the repo root), check | ||
| // produces a JSON envelope. When it does not, it falls through | ||
| // to a human-friendly guidance message. | ||
| let stdout = result.stdout.trim(); | ||
| if stdout.starts_with('{') { | ||
| let parsed: serde_json::Value = | ||
| serde_json::from_str(stdout).expect("cairn check --json must produce valid JSON"); | ||
| assert_eq!(parsed["command"], "check", "envelope must name the command"); | ||
| assert!( | ||
| parsed["data"]["findings"].is_array(), | ||
| "envelope must contain findings array" | ||
| ); | ||
| } | ||
| // Either way, check --json must not be rejected with an error | ||
| // message pointing at `cairn lint --json`. | ||
| assert!( |
- Wrap "no blueprint found" check path in JSON envelope when --json set - Normalize accept --json status to ok/error with gate_outcome in data - Enforce JSON unconditionally in check --json acceptance test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Wrap DSL migration check path in command envelope when --json set - Remove ad-hoc message field from no-blueprint JSON (empty findings suffice) - Capture subprocess stderr in JSON mode instead of discarding to /dev/null - Add \r, \t, \b, \f escapes to esc() per RFC 8259 - Report blocked gate as status "error" (incomplete verification is not ok) - Strengthen acceptance test: assert exit code and status field shape Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+114
to
+125
| return ok(format!( | ||
| "{{\"command\":\"check\",\"status\":\"error\",\"data\":{{\"findings\":[{}]}}}}\n", | ||
| finding_json(&Finding { | ||
| code: "CAIRN_COMMAND_FAILED".to_owned(), | ||
| severity: FindingSeverity::Error, | ||
| message: | ||
| "no blueprint file was found; rename `cairn.dsl` to `cairn.blueprint`" | ||
| .to_owned(), | ||
| node: None, | ||
| path: None, | ||
| }) | ||
| )); |
There was a problem hiding this comment.
Exit code mismatch with JSON status. This code path returns exit code 0 via ok(), but the JSON envelope contains "status":"error" with an Error-severity finding. According to the documented exit code scheme (line 357-359 in format.rs and lines 7-11 in error.rs), errors should return exit code 1.
if parsed.json {
return CliResult {
code: 1, // Error status requires non-zero exit code
stdout: format!(
"{{\"command\":\"check\",\"status\":\"error\",\"data\":{{\"findings\":[{}]}}}}}\n",
finding_json(&Finding {
code: "CAIRN_COMMAND_FAILED".to_owned(),
severity: FindingSeverity::Error,
message:
"no blueprint file was found; rename `cairn.dsl` to `cairn.blueprint`"
.to_owned(),
node: None,
path: None,
})
),
stderr: String::new(),
};
}This inconsistency will break orchestration tools relying on exit codes to detect failures.
Suggested change
| return ok(format!( | |
| "{{\"command\":\"check\",\"status\":\"error\",\"data\":{{\"findings\":[{}]}}}}\n", | |
| finding_json(&Finding { | |
| code: "CAIRN_COMMAND_FAILED".to_owned(), | |
| severity: FindingSeverity::Error, | |
| message: | |
| "no blueprint file was found; rename `cairn.dsl` to `cairn.blueprint`" | |
| .to_owned(), | |
| node: None, | |
| path: None, | |
| }) | |
| )); | |
| if parsed.json { | |
| return CliResult { | |
| code: 1, | |
| stdout: format!( | |
| "{{\"command\":\"check\",\"status\":\"error\",\"data\":{{\"findings\":[{}]}}}}\n", | |
| finding_json(&Finding { | |
| code: "CAIRN_COMMAND_FAILED".to_owned(), | |
| severity: FindingSeverity::Error, | |
| message: | |
| "no blueprint file was found; rename `cairn.dsl` to `cairn.blueprint`" | |
| .to_owned(), | |
| node: None, | |
| path: None, | |
| }) | |
| ), | |
| stderr: String::new(), | |
| }; | |
| } | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
This was referenced Jun 3, 2026
This was referenced Jul 9, 2026
Closed
Closed
Closed
Closed
This was referenced Aug 8, 2026
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
Replaces PR #114 (rebased onto current dev, review fixes applied).
--jsonsupport toaccept,check, andonboardcommands{"command": "<name>", "status": "ok|error", "data": {...}}--jsonis set (prevents output bleed into machine-readable stream)src/error.rsandsrc/cli/format.rsBead:
cairn-048| GH #98 (partial)Supersedes: #114
Test plan
cargo build(zero warnings)cargo clippy --all-targets --all-features -D warningscargo test(113+ tests pass)🤖 Generated with Claude Code