feat(cli): expose soar to frontends with JSON output and a plugin manifest - #194
Conversation
Deploying soar-docs with
|
| Latest commit: |
60ccc15
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7a28ed06.soar-docs.pages.dev |
| Branch Preview URL: | https://json-output.soar-docs.pages.dev |
|
Warning Review limit reached
Next review available in: 1 minute You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe CLI adds structured JSON output, JSON event streams, update checks, apply dry-run responses, and plugin-manifest generation. SQLite connections share timeout and WAL setup. Package search enriches results with maintainer data. ChangesCLI JSON and plugin integration
Database connection preparation
Package metadata enrichment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant SoarContext
participant JsonLinesSink
participant JSONOutput
CLI->>SoarContext: classify command and configure output
SoarContext->>JsonLinesSink: route event-stream output
JsonLinesSink->>CLI: write JSON event lines
CLI->>JSONOutput: convert command result
JSONOutput->>CLI: emit one JSON document
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@crates/soar-cli/src/apply.rs`:
- Around line 42-44: Update the JSON output path around ApplyDiffJson::new so it
receives the prune setting, and make ApplyDiffJson include diff.to_remove only
when prune is enabled. Keep removal entries excluded when --prune is absent,
matching the existing human-output behavior and apply semantics.
In `@crates/soar-cli/src/list.rs`:
- Around line 413-418: Update the count-only handling in the
ListInstalledPackages flow before the json_enabled branch so count requests emit
a defined JSON count document to stdout via json_output::emit before returning.
Preserve the existing info! behavior for non-JSON output and leave the full
Listing::new JSON response unchanged for non-count requests.
In `@crates/soar-cli/src/main.rs`:
- Around line 77-82: Keep JSON document event output isolated from tracing: in
crates/soar-cli/src/main.rs lines 77-82, reserve stderr for JsonLinesSink events
when stdout carries a JSON document; in crates/soar-cli/src/logging.rs lines
128-132, prevent tracing records from being routed through the event stream; and
in crates/soar-cli/src/logging.rs line 155, configure logging from the command
output mode rather than only args.json, suppressing tracing for document
commands or representing diagnostics as typed events.
In `@crates/soar-cli/src/plugin_manifest.rs`:
- Around line 211-226: Update the args for ops.default_config, ops.add_repo,
ops.remove_repo, and ops.set_repo_enabled to include --json so their output
matches the declared ndjson format; only change the format declarations instead
if those commands are intentionally non-JSON.
In `@crates/soar-operations/src/search.rs`:
- Around line 352-355: Replace the per-package
MetadataRepository::get_maintainers call in the package-processing loop with
batched lookups grouped by repo_name: collect package IDs per repository, query
each repository once, then map the returned maintainers by package ID and attach
them to the corresponding packages. Preserve the existing maintainer results and
package ordering while eliminating sequential queries for each package.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: adad5e46-ff26-4d78-8a97-ba256b197ec8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
crates/soar-cli/src/apply.rscrates/soar-cli/src/cli.rscrates/soar-cli/src/json_output.rscrates/soar-cli/src/list.rscrates/soar-cli/src/logging.rscrates/soar-cli/src/main.rscrates/soar-cli/src/plugin_manifest.rscrates/soar-cli/src/repo.rscrates/soar-cli/src/update.rscrates/soar-cli/src/utils.rscrates/soar-db/src/connection.rscrates/soar-events/Cargo.tomlcrates/soar-events/src/event.rscrates/soar-events/src/sink.rscrates/soar-operations/src/apply.rscrates/soar-operations/src/search.rscrates/soar-registry/src/package.rs
💤 Files with no reviewable changes (1)
- crates/soar-registry/src/package.rs
| if answers_with_diff { | ||
| json_output::emit(&ApplyDiffJson::new(&diff)); | ||
| return Ok(()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Filter removal entries when --prune is absent.
ApplyDiffJson::new(&diff) always includes diff.to_remove. The human output filters these entries with prune, but JSON output does not. Therefore, soar --json apply --dry-run can report removals that soar apply will not perform.
Pass prune into ApplyDiffJson and emit to_remove only when it is true.
Proposed fix
- json_output::emit(&ApplyDiffJson::new(&diff));
+ json_output::emit(&ApplyDiffJson::new(&diff, prune));- pub fn new(diff: &ApplyDiff) -> Self {
+ pub fn new(diff: &ApplyDiff, prune: bool) -> Self {
// ...
- to_remove: diff.to_remove.iter().map(/* ... */).collect(),
+ to_remove: if prune {
+ diff.to_remove.iter().map(/* ... */).collect()
+ } else {
+ Vec::new()
+ },🤖 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 `@crates/soar-cli/src/apply.rs` around lines 42 - 44, Update the JSON output
path around ApplyDiffJson::new so it receives the prune setting, and make
ApplyDiffJson include diff.to_remove only when prune is enabled. Keep removal
entries excluded when --prune is absent, matching the existing human-output
behavior and apply semantics.
| if json_enabled() { | ||
| let items: Vec<InstalledJson> = result.packages.iter().map(Into::into).collect(); | ||
| json_output::emit(&Listing::new(items, result.total_count)); | ||
| return Ok(()); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Emit a JSON count document for count-only requests.
When ListInstalledPackages { count: true } runs, Lines 405-408 return before this JSON branch. The info! call writes a log record to stderr in JSON mode. Stdout remains empty.
Emit a defined JSON count document from the count-only branch before returning.
🤖 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 `@crates/soar-cli/src/list.rs` around lines 413 - 418, Update the count-only
handling in the ListInstalledPackages flow before the json_enabled branch so
count requests emit a defined JSON count document to stdout via
json_output::emit before returning. Preserve the existing info! behavior for
non-JSON output and leave the full Listing::new JSON response unchanged for
non-count requests.
| if utils::json_enabled() { | ||
| let events: EventSinkHandle = if answers_with_document { | ||
| Arc::new(soar_events::JsonLinesSink::stderr()) | ||
| } else { | ||
| Arc::new(soar_events::JsonLinesSink::stdout()) | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep JSON event output separate from tracing output.
For JSON document commands, JsonLinesSink::stderr() emits events on stderr. JSON-mode tracing also writes records to stderr. This mixes tracing records with the event stream and bypasses the event sink as the single stream owner.
Keep stderr limited to one protocol. Suppress tracing output for document commands, or convert diagnostics into typed event records.
crates/soar-cli/src/main.rs#L77-L82: reserve stderr for typed JSON events when stdout contains a JSON document.crates/soar-cli/src/logging.rs#L128-L132: do not route tracing records to the event stream.crates/soar-cli/src/logging.rs#L155-L155: configure logging from the command output mode, not only fromargs.json.
#!/bin/bash
set -euo pipefail
ast-grep outline crates/soar-cli/src/main.rs --items all
ast-grep outline crates/soar-cli/src/logging.rs --items all
rg -n -C 3 'JsonLinesSink::(stdout|stderr)|WriterBuilder::new|logs_to_stderr' \
crates/soar-cli/src/main.rs crates/soar-cli/src/logging.rs
rg -n -C 3 '\b(info|warn|error|debug|trace)!\(' \
crates/soar-cli/src crates/soar-operations/src
rg -n -C 3 'EventSink|JsonLinesSink|\.emit\(|Event::' \
crates/soar-cli/src crates/soar-events/src crates/soar-operations/src📍 Affects 2 files
crates/soar-cli/src/main.rs#L77-L82(this comment)crates/soar-cli/src/logging.rs#L128-L132crates/soar-cli/src/logging.rs#L155-L155
🤖 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 `@crates/soar-cli/src/main.rs` around lines 77 - 82, Keep JSON document event
output isolated from tracing: in crates/soar-cli/src/main.rs lines 77-82,
reserve stderr for JsonLinesSink events when stdout carries a JSON document; in
crates/soar-cli/src/logging.rs lines 128-132, prevent tracing records from being
routed through the event stream; and in crates/soar-cli/src/logging.rs line 155,
configure logging from the command output mode rather than only args.json,
suppressing tracing for document commands or representing diagnostics as typed
events.
| [ops.default_config] | ||
| args = ["defconfig"] | ||
| output = { format = "ndjson" } | ||
|
|
||
| # Repository commands report nothing on success, so there is no shape to read. | ||
| [ops.add_repo] | ||
| args = ["repo", "add", "{name}", "{url}"] | ||
| output = { format = "ndjson" } | ||
|
|
||
| [ops.remove_repo] | ||
| args = ["repo", "remove", "{name}"] | ||
| output = { format = "ndjson" } | ||
|
|
||
| [ops.set_repo_enabled] | ||
| args = ["repo", "update", "{name}", "--enabled", "{enabled}"] | ||
| output = { format = "ndjson" } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Enable JSON mode for every operation declared as NDJSON.
These manifest entries declare format = "ndjson" but omit --json. They can therefore produce human-readable output instead of the declared event stream.
crates/soar-cli/src/plugin_manifest.rs#L211-L213: add--jsontoops.default_config, or declare its actual output format.crates/soar-cli/src/plugin_manifest.rs#L216-L218: add--jsontoops.add_repo, or declare its actual output format.crates/soar-cli/src/plugin_manifest.rs#L220-L222: add--jsontoops.remove_repo, or declare its actual output format.crates/soar-cli/src/plugin_manifest.rs#L224-L226: add--jsontoops.set_repo_enabled, or declare its actual output format.
🤖 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 `@crates/soar-cli/src/plugin_manifest.rs` around lines 211 - 226, Update the
args for ops.default_config, ops.add_repo, ops.remove_repo, and
ops.set_repo_enabled to include --json so their output matches the declared
ndjson format; only change the format declarations instead if those commands are
intentionally non-JSON.
| for package in &mut packages { | ||
| let found = metadata_mgr.query_repo(&package.repo_name, |conn| { | ||
| MetadataRepository::get_maintainers(conn, package.id as i32) | ||
| }); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Batch the maintainer lookup.
This loop performs one MetadataRepository::get_maintainers query per returned package. Queries that match multiple versions or repositories therefore create an N+1 pattern and add one sequential SQLite round trip per result. Group package IDs by repo_name, fetch maintainers once per repository, and attach them by package ID. (raw.githubusercontent.com)
🤖 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 `@crates/soar-operations/src/search.rs` around lines 352 - 355, Replace the
per-package MetadataRepository::get_maintainers call in the package-processing
loop with batched lookups grouped by repo_name: collect package IDs per
repository, query each repository once, then map the returned maintainers by
package ID and attach them to the corresponding packages. Preserve the existing
maintainer results and package ordering while eliminating sequential queries for
each package.
…tput and a plugin manifest (pkgforge#194) ⌚
Summary by CodeRabbit
New Features
update --checkoption to preview available updates without applying them.Bug Fixes