Repository navigation
feat(usage): declare what each command does to the world - #240
Conversation
usage 3.6 added `effect=`, which lets a spec say whether a command only inspects state, changes it, or destroys something (jdx/usage#739). Declare it for all 4 communique commands: 2 read, 2 write. `generate` is write rather than read because, while a bare invocation only prints, `--changelog` rewrites CHANGELOG.md and `--github-release` replaces the body of a published release. Write is the honest floor: it keeps the command out of any read-only allowlist, which is the distinction that matters. Three tests keep the table honest: every command must be classified, no entry may refer to a command that no longer exists, and nothing may be classified twice. To get `effect=` at all, generate the spec through usage-lib 3 rather than clap_usage, which is still published against usage-lib 2.x and so cannot carry the field. Spec generation moves into a shared generate_to_string so `communique usage` and the sync test cannot drift apart, and communique.usage.kdl is regenerated. Also emit `min_usage_version "3.6"`, since older `usage` CLIs reject the spec with "unsupported cmd prop effect". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #240 +/- ##
==========================================
+ Coverage 94.84% 95.01% +0.17%
==========================================
Files 26 27 +1
Lines 5377 5501 +124
Branches 5377 5501 +124
==========================================
+ Hits 5100 5227 +127
+ Misses 177 172 -5
- Partials 100 102 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Greptile SummaryUpdates usage metadata and generated CLI documentation:
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (6): Last reviewed commit: "test: evaluate assert messages so the li..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR updates Communiqué’s communique usage spec generation to support usage v3.6’s new effect= command annotation, enabling downstream tooling to distinguish read-only vs mutating commands.
Changes:
- Replace
clap_usage-based generation with a directusage-libv3 Spec conversion and a sharedgenerate_to_string()code path. - Introduce
src/command_effects.rsto declare and applyread/writeeffects to all CLI subcommands, with tests enforcing full coverage. - Regenerate
communique.usage.kdlto includemin_usage_version "3.6"and per-commandeffect=fields.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/usage.rs | Inlines spec generation to string, adds min_usage_version, and applies command effects. |
| src/main.rs | Wires in the new command_effects module. |
| src/command_effects.rs | New module defining per-command effects and tests to keep the table in sync with the CLI tree. |
| communique.usage.kdl | Regenerated usage spec including effect= and min_usage_version "3.6". |
| Cargo.toml | Drops clap_usage and adds usage-lib v3 with clap feature. |
| Cargo.lock | Lockfile updates reflecting dependency changes (including usage-lib 3.6.0). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //! **An unlisted command means "unknown", not "safe".** Consumers treat the | ||
| //! absence of a value as "ask", so leaving a command out is the conservative | ||
| //! choice and mislabeling one `read` is the dangerous one. |
| // 3.6 added `effect=`; older `usage` CLIs reject the spec outright with | ||
| // "unsupported cmd prop effect". | ||
| format!("// @generated by usage-cli from clap metadata\nmin_usage_version \"3.6\"\n{spec}") | ||
| } |
usage 4 lets a flag raise a command's effect (jdx/usage#742), so `generate` no longer has to carry its worst case. It is `read` — a bare invocation only prints — with the danger declared where it actually lives: - --changelog write rewrites CHANGELOG.md - --github-release write replaces a published release body - --output write writes to a file - init --force destructive overwrites an existing communique.toml The effect of an invocation is the maximum of the command's and those of the flags supplied, so `communique generate` is now correctly usable from a read-only allowlist while `--github-release` still prompts. Most flags declare nothing, which is the intent. A test asserts every FLAG_EFFECTS entry matches a real flag, so a renamed flag fails the build rather than silently losing its annotation. Review feedback: - the module docs called omitting a command the "conservative choice" while the tests hard-fail on it. Both are true for different reasons, but the wording was confusing: an unset effect is never dangerous, yet for a CLI this small "unknown" should be deliberate rather than forgotten. - the generated header credited usage-cli; the spec is produced by `communique usage`. Corrected. Also pins `usage = "4"` instead of "latest" in mise.toml. There is no lockfile here, so `latest` resolved to a pre-3.6 usage at CI time and the render failed with "unsupported cmd prop effect" — the spec's declared minimum was not reflected in the toolchain it is rendered with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both findings addressed, and usage 4 landed in the meantime so "Conservative choice" vs tests that hard-fail — fair, the wording was confusing. Both things were true for different reasons and I ran them together. An unset effect genuinely is never dangerous the way a wrong
generate is now read, with the danger on its flagsjdx/usage#742 (in usage 4) lets a flag raise its command's effect, so I no longer have to give the whole command its worst case:
The effect of an invocation is the maximum of the command's and the flags supplied, so A test asserts every The CI failure
The spec declared a minimum that the toolchain rendering it did not have to meet. Now pinned to Verified
This comment was generated by an AI coding assistant. |
mise resolved `usage = "4"` as a literal version and tried to fetch a `v4` release tag, which does not exist: Failed to install aqua:jdx/usage@4: 404 for releases/tags/v4 Pin 4.0.0. With no lockfile in this repo an exact pin is also what makes the toolchain reproducible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
codecov flagged the patch, and it was right about something real: every test here checked the tables against the CLI — complete, no stale entries, no duplicates — but none checked that `apply` actually transfers them onto the spec. The tables could have been perfect and silently never applied. `apply_annotates_commands_and_flags` covers that path directly, including that a flag with no entry is left alone rather than inheriting one. Also switches to clap_usage::spec(), now that clap_usage 4.0.0 is published (jdx/usage#743) with the accessor that was missing when this branch started. The direct usage-lib dependency goes away — clap_usage re-exports usage. `communique usage` emits a byte-identical spec. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
On the two codecov checks — I dug into the actual numbers rather than guessing, and I don't think there's anything real to fix here. Your call, though.
There is no Worth noting the review did surface a real gap, and I fixed that one: none of the tests called If you'd rather these checks not gate PRs, a This comment was generated by an AI coding assistant. |
Measured the coverage rather than guessing at it, and usage.rs was the real gap: 78.95%, with `run` entirely uncovered. It is the only path that actually emits the spec, so nothing checked that `communique usage` succeeds at all — `generate_to_string` was covered by the sync test, but the command wrapping it was not. usage.rs is now 100%. The four lines still uncovered in command_effects.rs are the format arguments of assert messages and the `_ =>` arm that records a missing flag. Rust only evaluates those when the assertion fails, so covering them would mean asserting that the tests fail. Left alone deliberately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`assert!(cond, "..{}", x)` only formats when the assertion fails, so the
message expressions were dead code while the tests passed — which is what
codecov was flagging on the patch. Building the detail string up front costs
nothing at these list sizes and makes the code genuinely execute, rather
than covering it by asserting that a test fails.
Same reason for `then_some` over `then` in the flag check: the label is now
built for every entry instead of only on the failing path.
command_effects.rs goes from 95.37% to 99.13%. The one line left is the
`is_some_and` short-circuit for a command that does not exist, which cannot
happen while the completeness test passes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both codecov checks pass now. I measured rather than kept guessing — installed That turned up something I had wrong. I said the uncovered lines were all test-helper The remaining lines in One line is still uncovered: the Worth noting the three This comment was generated by an AI coding assistant. |
This release teaches communique's usage spec what each command does to the world — `read`, `write`, or `destructive` — so tools built on the spec can tell a safe inspection from a state-changing one. It also picks up progress-display fixes from `clx` v3. ## Added - *(usage)* Every communique command now carries an `effect` annotation in its usage spec, distinguishing read-only commands from those that modify or destroy state. `generate` and `sponsors` are `read`; `init` is `write`; and the flags that actually change things are raised accordingly — `--changelog`, `--github-release`, and `--output` on `generate` are `write`, and `init --force` is `destructive`. Because `effect` is per-command, `generate` is classified as `read` at its floor while its state-changing flags carry the higher effect (the effect of an invocation is the maximum of the command and its supplied flags). The regenerated `communique.usage.kdl` and `docs/cli` reflect these labels. ([#240](#240)) (@jdx) ## Changed - Updated `clx` to v3 for progress-display fixes: cursor escape sequences are no longer written in text or quiet mode, and a reflowed frame is now cleared after a terminal resize. ([#248](#248)) **Full Changelog**: https://github.com/jdx/communique/commits/v1.3.0 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Version and documentation-only changes with no runtime logic in this diff. > > **Overview** > **Cuts release v1.3.0** by bumping the crate and generated CLI metadata from `1.2.4` to `1.3.0` in `Cargo.toml`, `Cargo.lock`, `communique.usage.kdl`, and `docs/cli`. > > **Updates `CHANGELOG.md`** with a dated `[1.3.0]` section documenting the shipped work (usage `read`/`write`/`destructive` effects and `clx` v3 progress fixes), while `[Unreleased]` still only lists the upcoming default model bump to `claude-opus-4-8`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 169db15. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * CLI usage specifications now identify each command’s world effects: read, write, or destructive. * **Changed** * Updated the CLI to version 1.3.0. * Text and quiet modes no longer emit cursor escape sequences. * Reflowed frames are cleared after terminal resizing. * Updated displayed version information across CLI documentation and metadata. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
usage 3.6 shipped
effect=(jdx/usage#739). This declares it for all 4 communique commands: 2 read, 2 write.Last of the set, after jdx/mise#11306, jdx/hk#1121 and jdx/pitchfork#666.
The one judgement call
generateis write, not read. A bare invocation only prints, but--changelogrewritesCHANGELOG.mdand--github-releasereplaces the body of a published release. Sinceeffectis per-command rather than per-flag,writeis the honest floor — it keepsgenerateout of any read-only allowlist, which is the distinction that actually matters. The comment in the table spells out why, including that--github-releaseoverwrites remote content.Why the clap_usage dependency goes away
Same as the sibling PRs:
clap_usageis still published againstusage-lib2.x (latest 2.0.3, requiringusage-lib ^2.0.3), so a spec generated through it physically cannot carryeffect=.clap_usage::generateis four lines, so this inlines it and depends onusage-lib = "3"directly.Publishing
clap_usage3.x from the usage repo is the alternative — its copy in-repo is still versioned 2.0.3 while building against workspaceusage-lib3.6.0, so it's due a release regardless.One cleanup worth noting
src/usage.rshad two separate spec-generation paths — the real one inrun()and a second copy inside thetest_usage_kdl_in_synctest. They could drift, and the test would still pass while checking the wrong thing. Both now go through onegenerate_to_string(), so the sync test verifies what the command actually emits.communique.usage.kdlis regenerated.Verified
cargo build,cargo clippy --all-targets,cargo fmt --checkclean. All four new/affected tests pass, includingtest_usage_kdl_in_sync.cargo testhas 3 pre-existing failures intools::grep(test_grep_basic,test_grep_no_matches,test_grep_with_glob) — confirmed present on a clean checkout ofmain; they shell out to ripgrep, which isn't installed in my environment.This PR was generated by an AI coding assistant.
Note
Low Risk
Metadata and dependency bumps only; runtime CLI behavior is unchanged aside from richer usage/docs output.
Overview
Upgrades
clap_usageto 4 and pins theusageCLI to 4.0.0 so the checked-in spec can useeffect=andmin_usage_version "4.0".Adds
src/command_effects.rs, which classifies each subcommand and a few state-changing flags (clap cannot express this), thenapplymerges those annotations into the spec before emission.generateis read by default;--changelog,--github-release, and--outputare write;init --forceis destructive. Tests require every command to be classified and that flag entries still match real flags.communique usagenow builds the KDL through a singlegenerate_to_string()(shared with the sync test), andcommunique.usage.kdlplusdocs/cliJSON/Markdown are regenerated with effect labels.Reviewed by Cursor Bugbot for commit b6b45e1. Bugbot is set up for automated code reviews on this repo. Configure here.