feat(spec): add effect= to declare what a command does to the world

[jdx]

Lets a spec declare what running a command does:

cmd "ls"        effect="read"        help="List installed tools"
cmd "use"       effect="write"       help="Install a tool and add it to the config"
cmd "uninstall" effect="destructive" help="Remove a tool"

Effect	Meaning
read	Only inspects state. Running it twice is the same as running it once.
write	Creates or modifies state, but removes nothing the user cannot recreate.
destructive	May delete or irreversibly overwrite something. Deserves a confirmation prompt.

Why in the spec

Several consumers keep reinventing this same distinction, and each one hand-maintains its own list:

• docs and --help want to mark destructive commands
• wrapper scripts want to confirm before running one
• AI coding agents want an allowlist of read-only commands rather than prompting on every invocation — the Agent Skills spec has an allowed-tools field that is exactly this list, currently written by hand

That is the usual argument for putting something in a spec: one annotation, many renderings, same as help or hide today. It's structured data, so there's nothing to keep in sync and no prose to go stale.

Design notes

Not inherited by subcommands. git remote and git remote remove do different things. Inheriting would make the least safe reading of a spec the default one, which is the wrong direction for a field whose whole purpose is flagging danger.

Unset means unknown, not safe. Consumers should treat a missing effect as "ask". This keeps the field additive — existing specs are unaffected, and a partially annotated spec degrades to today's behavior.

Coarse on purpose. Three values, not a capability model. Anything finer becomes a policy language, and that belongs in the consumer.

Changes

• SpecCommandEffect enum in lib/src/spec/effect.rs, exported as usage::SpecCommandEffect
• SpecCommand::effect: Option<SpecCommandEffect>, plus a builder method
• parsed as a prop (effect="read") or a child node (effect "read") — the latter so framework integrations generating KDL don't have to emit props; unknown values are a parse error pointing at the span
• round-trips through KDL serialization
• generated markdown renders an - **Effect**: line next to Usage and Aliases

Not in this PR

• usage generate skill consuming it to emit allowed-tools. That's the payoff, but it's a separate discussion about output format and shouldn't ride along with the spec primitive.
• manpage rendering.
• framework integrations. clap/cobra/etc. can't infer this — it needs an explicit annotation from the author, same as hide.

Verified

cargo test -p usage-lib passes (275 + doctests). Four new tests cover prop parsing, child-node parsing, non-inheritance, KDL round-trip, and the error on an unknown value. Markdown output checked by hand against a sample spec.

cargo test --workspace has two pre-existing failures in usage-cli --test examples (test_usage_double_slash_execution, ..._old) — confirmed present on a clean checkout of main, unrelated to this change.

This PR was generated by an AI coding assistant.

---

Note

Low Risk
Additive spec field with optional semantics; no changes to parsing or runtime CLI behavior beyond new metadata and doc generation.

Overview
Adds an optional effect on commands so specs can classify side effects as read, write, or destructive (via effect="…" or a child effect node). Missing effect stays unknown; values are not inherited by subcommands.

The library introduces SpecCommandEffect, wires it through SpecCommand (parse, KDL serialize, merge), SpecCommandBuilder::effect, and the public API. Generated markdown adds a - **Effect**: line when set. Docs in docs/spec/index.md describe semantics and consumers (docs, wrappers, agents).

Tests cover parsing, non-inheritance, KDL round-trip, merge behavior, invalid values, and markdown rendering. .gitignore ignores *.pending-snap from insta.

^{Reviewed by Cursor Bugbot for commit e0e5192. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

Warning

Review limit reached

@jdx, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 33 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 99a0187e-1da1-4bcb-88cb-86536b72e932

📥 Commits

Reviewing files that changed from the base of the PR and between 5f731b2 and e0e5192.

📒 Files selected for processing (10)

• .gitignore
• docs/spec/index.md
• lib/src/docs/markdown/cmd.rs
• lib/src/docs/markdown/templates/cmd_template.md.tera
• lib/src/docs/models.rs
• lib/src/lib.rs
• lib/src/spec/builder.rs
• lib/src/spec/cmd.rs
• lib/src/spec/effect.rs
• lib/src/spec/mod.rs

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

Adds command-effect metadata throughout the spec pipeline.

• Defines and publicly exports the read, write, and destructive effect enum.
• Parses, serializes, builds, and merges optional command effects without inheritance.
• Renders effects in generated Markdown and documents the new spec field.
• Adds coverage for parsing, round-tripping, merge behavior, non-inheritance, validation, and Markdown rendering.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported merge omission and Markdown coverage gap are both addressed at the current head.

Important Files Changed

Filename	Overview
lib/src/spec/cmd.rs	Integrates command effects into parsing, serialization, defaults, and merge composition; the prior merge-loss issue is fixed and covered by tests.
lib/src/spec/effect.rs	Defines the public three-value command-effect enum with consistent parsing, display, serialization, labels, and unit tests.
lib/src/docs/markdown/templates/cmd_template.md.tera	Adds conditional human-readable effect labels to generated Markdown, now exercised for every effect and the unset case.
lib/src/docs/markdown/cmd.rs	Adds focused snapshot coverage for all effect-rendering branches and omission when no effect is declared.
lib/src/docs/models.rs	Propagates command effects into the shared documentation model used by the Markdown template.
lib/src/spec/builder.rs	Adds a typed builder method for assigning command effects.
docs/spec/index.md	Documents effect meanings, supported syntax, non-inheritance, and unknown-by-default semantics.

_{Reviews (3): Last reviewed commit: "fix(spec): drop dead ParseEffectError, u..." | Re-trigger Greptile}

[greptile-apps]

Effect rendering lacks snapshot coverage

The new template branches for read, write, and destructive are not exercised by the existing Markdown snapshots because the shared test spec declares no effects. Add cases for all three values so changes to enum serialization or template comparisons cannot silently produce incorrect labels.

Knowledge Base Used: Docs generation: rendering a Spec into markdown, manpages, and CLI help

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[jdx]

Both findings were real. Fixed in f41891d.

effect dropped by SpecCommand::merge — correct, and it matters: merge is how included and mounted specs are composed, so an overlay that said nothing about effects erased one already declared. Now handled like every other optional field.

Worth noting what the test turned up while I was writing it: at the root level merge replaces subcommands wholesale rather than merging them, so an overlay's cmd "uninstall" discards the base's entirely — every field, not just effect. That's pre-existing behavior and I've left it alone; the test asserts the semantics that actually exist.

Template branches unexercised — also correct, the kitchen-sink spec declares no effects. Added a snapshot covering all three labels plus a command with none, so a change to enum serialization or the template comparisons can't silently produce a wrong label.

---

While fixing the merge gap I audited every SpecCommand field against the three places that have to know about it. Three fields already fall through, unrelated to this PR:

field	gap
deprecated	not carried by merge — an included spec silently un-deprecates a command
restart_token	never reaches docs::models::SpecCommand, so docs can't see it
examples	not written by From<&SpecCommand> for KdlNode — spec.to_string() loses them, so KDL isn't round-trip stable

Happy to open a follow-up. Making merge and both From impls destructure their source exhaustively (no ..) would turn each of these into a compile error instead of silent data loss, at no runtime cost.

This comment was generated by an AI coding assistant.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit f41891d. Configure here.}

[jdx]

Both correct, fixed:

Unused ParseEffectError — dead on arrival. I wrote it, then used bail_parse! for the actual error so it'd carry a source span like every other parse failure, and never removed the type. Gone.

Insta pending snapshot committed — worse than noise: it isn't on main, I added it, and the test run rewrites it, which is what turned the render produces no diff CI check red. Untracked, and *.pending-snap added to .gitignore so it can't recur.

Also ran prettier -w on the docs table, which was the other half of the CI failure.

This comment was generated by an AI coding assistant.