feat(cli): add guard as first-class resource type with CRUD subcommands#4
Merged
Conversation
📝 WalkthroughWalkthroughAdds a new "guard" resource with full govctl CLI support (new/list/get/set/add/remove/delete/show), RFC and schema updates, TOML adapters and I/O, edit-op/runtime support, deletion safety checks, tests, and documentation including a guard-writer skill. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Client CLI
participant Router as Command Router
participant Cmd as Guard Command Impl
participant FS as Filesystem (gov/guard/)
participant CFG as Config (gov/config.toml)
participant WI as Work Items (gov/work/...)
CLI->>Router: parse `govctl guard new|delete|show ...`
Router->>Cmd: dispatch to new_guard / delete_guard / show_guard
alt new_guard
Cmd->>FS: read existing guard files
FS-->>Cmd: file list / existence
Cmd->>FS: write guard TOML
FS-->>Cmd: write result
Cmd->>CLI: print created path & hint
else delete_guard
Cmd->>CFG: read verification.default_guards
Cmd->>WI: scan work items for required_guards / waivers
CFG-->>Cmd: default_guards
WI-->>Cmd: referencing work items (if any)
alt references found
Cmd->>CLI: error listing blockers
else
Cmd->>FS: delete guard TOML
FS-->>Cmd: deletion result
Cmd->>CLI: confirmation
end
else show_guard
Cmd->>FS: read guard TOML
FS-->>Cmd: guard spec
Cmd->>CLI: render TOML/JSON output
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/parse.rs (1)
313-346: Consider extracting a generic TOML write helper.The
write_guard,write_adr, andwrite_work_itemfunctions share identical structure. A generic helper could reduce duplication:♻️ Optional refactor
fn write_toml_artifact<T: Serialize>( path: &Path, spec: &T, schema: ArtifactSchema, op: WriteOp, display_path: Option<&Path>, ) -> Result<(), Diagnostic> { let body = toml::to_string_pretty(spec).map_err(|e| { Diagnostic::new( DiagnosticCode::E0901IoError, format!("Failed to serialize TOML: {e}"), path.display().to_string(), ) })?; let content = with_schema_header(schema, &body); match op { WriteOp::Execute => { std::fs::write(path, &content).map_err(|e| { Diagnostic::new(DiagnosticCode::E0901IoError, e.to_string(), path.display().to_string()) })?; } WriteOp::Preview => { ui::dry_run_file_preview(display_path.unwrap_or(path), &content); } } Ok(()) }Then
write_guardbecomes:pub fn write_guard(path: &Path, spec: &GuardSpec, op: WriteOp, display_path: Option<&Path>) -> Result<(), Diagnostic> { write_toml_artifact(path, spec, ArtifactSchema::Guard, op, display_path) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/parse.rs` around lines 313 - 346, The three functions write_guard, write_adr, and write_work_item duplicate TOML serialization and file/write/preview logic; extract a generic helper fn write_toml_artifact<T: Serialize>(path: &Path, spec: &T, schema: ArtifactSchema, op: WriteOp, display_path: Option<&Path>) -> Result<(), Diagnostic> that performs toml::to_string_pretty, wraps with with_schema_header(schema, &body), maps serialization and IO errors to Diagnostic::new(DiagnosticCode::E0901IoError, ... , path.display().to_string()), and handles WriteOp::Execute with std::fs::write and WriteOp::Preview with ui::dry_run_file_preview(display_path.unwrap_or(path), &content); then simplify write_guard (and likewise write_adr/write_work_item) to delegate to write_toml_artifact with the appropriate specific spec type (e.g., GuardSpec) and ArtifactSchema::Guard.tests/test_guard.rs (1)
68-97: Add one round-trip test fortimeout_secs.Current guard field coverage only hits string fields (
title/command), so the new numeric edit path can regress unnoticed. Aguard set ... timeout_secs 30followed byguard get ... timeout_secswould cover the non-string runtime mapping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_guard.rs` around lines 68 - 97, Add a new round-trip test in tests/test_guard.rs that mirrors the string-field tests but exercises the numeric path: create a temp project with init_project(), write a guard via write_guard(...) (e.g., "GUARD-ECHO" with some initial timeout), call run_commands(...) to run ["guard","set","GUARD-ECHO","timeout_secs","30"] followed by ["guard","get","GUARD-ECHO","timeout_secs"], and assert the output contains "30"; name the test function something like test_guard_set_timeout_secs and reuse the existing helpers (init_project, write_guard, run_commands).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 10-16: Add the missing "govctl guard get retrieves a specific
guard field value (WI-2026-03-27-001)" entry under the "### Added" section in
CHANGELOG.md so it matches the other resource commands; ensure the text and WI
identifier format follow the existing lines for "govctl guard
new/list/show/set/delete" to keep the changelog consistent with the tests that
use guard get in test_guard.rs.
In `@gov/schema/edit-ops.json`:
- Around line 559-566: The runtime schema for the "guard" artifact exposes
"timeout_secs" with mode type "string", but GuardCheck.timeout_secs (in
src/model.rs / GuardSpec) is a u64, so set_toml_field will fail deserializing
string values; change the runtime field mode for the entry with name
"timeout_secs" (path ["check","timeout_secs"]) from a string type to a
numeric/integer mode (use an integer/number mode appropriate for u64) so the
runtime writes numeric values that round-trip into GuardSpec.
In `@src/cli.rs`:
- Around line 1086-1092: The Delete command's force flag (Delete { id: String,
force: bool }) is currently skipping mandatory reference checks; modify the
delete handler in src/cmd/guard.rs (the guard deletion function handling lines
~76–113) so that reference/safety checks always run regardless of force, and
only the interactive confirmation prompt is bypassed when force is true; if
references (e.g., entries in verification.default_guards or required_guards) are
found, abort deletion and return an error/log, otherwise proceed to delete, but
do not tie the check logic to the force flag—keep force solely for skipping the
confirmation prompt.
In `@src/cmd/guard.rs`:
- Around line 76-116: The current blocker scanning is gated by if !force &&
!op.is_preview(), which causes --force (and preview) to skip reference checks
and allow deletion with dangling references; change the logic so blockers are
always computed (use the existing checks: config.verification.default_guards,
crate::parse::load_work_items and scanning wi.spec.verification.required_guards
and waivers) regardless of force/preview, and only use force to bypass the
interactive confirmation step before calling crate::write::delete_file (use
guard.path and op as before); mirror the delete_clause/delete_work_item flow by
moving the blocker collection out of the force/preview guard and keeping force
solely for skipping the prompt.
- Around line 41-57: The scaffolded GuardCheck uses a successful echo command
allowing a no-op guard to always pass; change the default check.command in the
GuardCheck construction (the field set where GuardCheck is instantiated before
write_guard) to a failing placeholder command (e.g., a command that prints an
explanatory message and exits non‑zero) so newly generated guards fail until
user replaces it; keep the rest of the flow (write_guard, ui hints) but ensure
the preview/creation path still shows the file while the default command is
non‑passing.
In `@src/cmd/list.rs`:
- Around line 57-61: The guard branch in list() converts
load_guards_with_warnings errors into an anyhow instead of the standard
Vec<Diagnostic> used by other targets; change the error handling so
load_guards_with_warnings returns/propagates the same Vec<Diagnostic> path
(i.e., remove the map_err(|d| anyhow::anyhow!("{}", d.message)) conversion) and
return the diagnostics directly on error, while keeping
list_guards(&result.items, filter, limit, output) and returning
Ok(result.warnings) on success; reference: ListTarget::Guard,
load_guards_with_warnings, list_guards, and list().
- Around line 360-378: The current mapping truncates guard commands with a
byte-slice (&cmd[..50]) which can panic on non-ASCII and also loses the full
command for JSON/plain outputs; change the mapping in the creation of summaries
(where GuardSummary is constructed from items and cmd is read from
g.spec.check.command) to store the full cmd (e.g., cmd.clone()) without any
slicing, and move any display truncation into the table/plain renderer used by
output_list (or the renderer callback passed to output_list) where you apply a
char-boundary-safe truncation helper (e.g., truncate_str_by_chars) so table
output shows a shortened preview while JSON/plain output contains the full
command.
---
Nitpick comments:
In `@src/parse.rs`:
- Around line 313-346: The three functions write_guard, write_adr, and
write_work_item duplicate TOML serialization and file/write/preview logic;
extract a generic helper fn write_toml_artifact<T: Serialize>(path: &Path, spec:
&T, schema: ArtifactSchema, op: WriteOp, display_path: Option<&Path>) ->
Result<(), Diagnostic> that performs toml::to_string_pretty, wraps with
with_schema_header(schema, &body), maps serialization and IO errors to
Diagnostic::new(DiagnosticCode::E0901IoError, ... , path.display().to_string()),
and handles WriteOp::Execute with std::fs::write and WriteOp::Preview with
ui::dry_run_file_preview(display_path.unwrap_or(path), &content); then simplify
write_guard (and likewise write_adr/write_work_item) to delegate to
write_toml_artifact with the appropriate specific spec type (e.g., GuardSpec)
and ArtifactSchema::Guard.
In `@tests/test_guard.rs`:
- Around line 68-97: Add a new round-trip test in tests/test_guard.rs that
mirrors the string-field tests but exercises the numeric path: create a temp
project with init_project(), write a guard via write_guard(...) (e.g.,
"GUARD-ECHO" with some initial timeout), call run_commands(...) to run
["guard","set","GUARD-ECHO","timeout_secs","30"] followed by
["guard","get","GUARD-ECHO","timeout_secs"], and assert the output contains
"30"; name the test function something like test_guard_set_timeout_secs and
reuse the existing helpers (init_project, write_guard, run_commands).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63367a76-ce87-472b-9d8e-b7b69151ec46
📒 Files selected for processing (22)
.claude/skills/guard-writer/SKILL.mdCHANGELOG.mdCLAUDE.mdbuild.rsdocs/rfc/RFC-0002.mdgov/rfc/RFC-0002/clauses/C-CRUD-VERBS.tomlgov/rfc/RFC-0002/clauses/C-RESOURCE-MODEL.tomlgov/rfc/RFC-0002/clauses/C-RESOURCES.tomlgov/rfc/RFC-0002/rfc.tomlgov/schema/edit-ops.jsongov/schema/edit-ops.schema.jsongov/work/2026-03-27-add-guard-resource-subcommands-new-list-show-set-delete.tomlsrc/cli.rssrc/cmd/edit/adapter.rssrc/cmd/edit/mod.rssrc/cmd/edit/runtime.rssrc/cmd/guard.rssrc/cmd/list.rssrc/cmd/mod.rssrc/command_router.rssrc/parse.rstests/test_guard.rs
Comment on lines
+10
to
+16
| ### Added | ||
|
|
||
| - govctl guard new scaffolds guard TOML with hint about default_guards (WI-2026-03-27-001) | ||
| - govctl guard list shows all guards in tabular format (WI-2026-03-27-001) | ||
| - govctl guard show displays a guard (WI-2026-03-27-001) | ||
| - govctl guard set edits guard fields (WI-2026-03-27-001) | ||
| - govctl guard delete removes a guard with safety checks (WI-2026-03-27-001) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if guard get/add/remove commands are implemented
rg -n "guard.*get|guard.*add|guard.*remove" src/cli.rs src/command_router.rs 2>/dev/null || echo "No matches in CLI files"
ast-grep --pattern 'fn $FUNC($_) { $$$ }' | rg -i "guard" | head -20Repository: govctl-org/govctl
Length of output: 84
🏁 Script executed:
# Find all Rust source files and explore structure
git ls-files | grep -E '\.(rs|toml)$' | head -20Repository: govctl-org/govctl
Length of output: 1288
🏁 Script executed:
# Search for where CLI commands are registered/defined
rg -n "guard" --type rust | grep -i "command\|subcommand\|arg\|cmd" | head -30Repository: govctl-org/govctl
Length of output: 2950
🏁 Script executed:
# Look for the actual command implementations
rg -n "\"get\"|\"add\"|\"remove\"" --type rust | grep -i "guard\|sub" | head -20Repository: govctl-org/govctl
Length of output: 295
Add changelog entry for guard get command.
Tests in test_guard.rs show that guard get is implemented and used to retrieve individual guard field values (lines 76–77, 93), but this command is missing from the changelog. The other standard resource model commands (new, list, show, set, delete) are documented, so get should be added for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` around lines 10 - 16, Add the missing "govctl guard get
retrieves a specific guard field value (WI-2026-03-27-001)" entry under the "###
Added" section in CHANGELOG.md so it matches the other resource commands; ensure
the text and WI identifier format follow the existing lines for "govctl guard
new/list/show/set/delete" to keep the changelog consistent with the tests that
use guard get in test_guard.rs.
Comment on lines
+57
to
+61
| if target == ListTarget::Guard { | ||
| let result = | ||
| load_guards_with_warnings(config).map_err(|d| anyhow::anyhow!("{}", d.message))?; | ||
| list_guards(&result.items, filter, limit, output); | ||
| return Ok(result.warnings); |
There was a problem hiding this comment.
Keep guard list on the same diagnostic path as the other list targets.
This branch converts the top-level guard load failure into anyhow, while the rest of list() returns Vec<Diagnostic>. A read/permission error under gov/guard will therefore bypass the normal diagnostic renderer just for guards.
Suggested fix
- let result =
- load_guards_with_warnings(config).map_err(|d| anyhow::anyhow!("{}", d.message))?;
+ let result = match load_guards_with_warnings(config) {
+ Ok(result) => result,
+ Err(d) => return Ok(vec![d]),
+ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cmd/list.rs` around lines 57 - 61, The guard branch in list() converts
load_guards_with_warnings errors into an anyhow instead of the standard
Vec<Diagnostic> used by other targets; change the error handling so
load_guards_with_warnings returns/propagates the same Vec<Diagnostic> path
(i.e., remove the map_err(|d| anyhow::anyhow!("{}", d.message)) conversion) and
return the diagnostics directly on error, while keeping
list_guards(&result.items, filter, limit, output) and returning
Ok(result.warnings) on success; reference: ListTarget::Guard,
load_guards_with_warnings, list_guards, and list().
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/cmd/guard.rs (2)
41-45:⚠️ Potential issue | 🟠 MajorScaffolded guards should fail until they're configured.
The default
check.commandis anechothat exits 0, so a brand-new guard can be added toverification.default_guardsand immediately report success without doing any verification. Use a failing placeholder until the command is replaced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/guard.rs` around lines 41 - 45, The scaffolded GuardCheck uses a success-returning placeholder command; change the default check.command in the GuardCheck initializer to a failing placeholder (e.g., "false" or "sh -c 'exit 1'") so new guards fail until configured; update the initializer where GuardCheck is constructed (the check field in the Guard struct) to use the failing command string and leave timeout_secs/pattern as-is.
76-113:⚠️ Potential issue | 🔴 CriticalKeep blocker checks mandatory for
guard delete.Blocker scanning is gated behind
!force && !op.is_preview(), soguard delete --forceand preview mode both skip the very checks RFC-0002 requires.--forceshould only skip confirmation after blockers are known, never bypass config/work-item reference detection. Downstream, the CLI help text andtests/test_guard.rs::test_guard_delete_force_bypasses_checkswill need to be updated with the same contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/guard.rs` around lines 76 - 113, The blocker detection (building blockers via config.verification.default_guards and scanning work items with crate::parse::load_work_items into the blockers Vec) must run unconditionally; remove the surrounding gate "if !force && !op.is_preview() { ... }" so those checks always execute, and only use force/preview to skip the subsequent confirmation step (not the detection). Keep the existing code that returns an error when blockers is_non_empty, and adjust caller logic or add a separate confirmation branch that is conditional on force or op.is_preview(); also update the CLI help text and the test tests/test_guard.rs::test_guard_delete_force_bypasses_checks to reflect that --force does not bypass blocker checks but only skips confirmation.src/cmd/list.rs (2)
57-61:⚠️ Potential issue | 🟡 MinorKeep
guard liston the standard diagnostic path.This branch turns a
Diagnosticintoanyhow, so guard-specific load/read failures skip the normal renderer used by every other list target. Return the diagnostic directly here instead of erroring out.Suggested fix
if target == ListTarget::Guard { - let result = - load_guards_with_warnings(config).map_err(|d| anyhow::anyhow!("{}", d.message))?; + let result = match load_guards_with_warnings(config) { + Ok(result) => result, + Err(d) => return Ok(vec![d]), + }; list_guards(&result.items, filter, limit, output); return Ok(result.warnings); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/list.rs` around lines 57 - 61, The branch for ListTarget::Guard converts a Diagnostic into an anyhow error, bypassing the standard diagnostic renderer; instead propagate the Diagnostic directly: call load_guards_with_warnings(config) without map_err (e.g. let result = load_guards_with_warnings(config)?), keep calling list_guards(&result.items, filter, limit, output), and return result.warnings; ensure the surrounding function's Result type propagates the Diagnostic so diagnostics follow the normal rendering path.
360-378:⚠️ Potential issue | 🟠 MajorDon't truncate guard commands before serialization.
&cmd[..50]can panic on non-ASCII commands, and truncating here also means JSON/plain output loses the real command. Keep the full value inGuardSummaryand only shorten it in the table renderer with a char-boundary-safe helper.Suggested fix
struct GuardSummary { id: String, title: String, command: String, } +fn truncate_for_display(value: &str, max_chars: usize) -> String { + if value.chars().count() <= max_chars { + return value.to_string(); + } + let end = value + .char_indices() + .nth(max_chars) + .map(|(idx, _)| idx) + .unwrap_or(value.len()); + format!("{}…", &value[..end]) +} + fn list_guards( guards: &[crate::model::GuardEntry], filter: Option<&str>, limit: Option<usize>, @@ let summaries: Vec<GuardSummary> = items .iter() - .map(|g| { - let cmd = &g.spec.check.command; - let truncated = if cmd.len() > 50 { - format!("{}…", &cmd[..50]) - } else { - cmd.clone() - }; - GuardSummary { - id: g.meta().id.clone(), - title: g.meta().title.clone(), - command: truncated, - } - }) + .map(|g| GuardSummary { + id: g.meta().id.clone(), + title: g.meta().title.clone(), + command: g.spec.check.command.clone(), + }) .collect(); output_list(&summaries, &["Guard", "Title", "Command"], output, |s| { - vec![s.id.clone(), s.title.clone(), s.command.clone()] + vec![ + s.id.clone(), + s.title.clone(), + match output { + OutputFormat::Table => truncate_for_display(&s.command, 50), + OutputFormat::Json | OutputFormat::Plain => s.command.clone(), + }, + ] }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/list.rs` around lines 360 - 378, The code currently truncates command strings inside the map that builds GuardSummary (in the items.iter() -> GuardSummary creation), using &cmd[..50] which can panic on non-ASCII and also loses the full command for JSON/plain output; instead store the full command in GuardSummary.command (remove the truncation logic in the iterator) and perform any visual shortening only in the table renderer path passed to output_list (implement a char-boundary-safe truncation helper, e.g. using chars().take(n).collect::<String>() or a unicode-segmentation helper) so JSON/plain serializers still receive the complete command while the table view shows a shortened version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/rfc/RFC-0002.md`:
- Around line 588-590: The v0.5.0 changelog line incorrectly calls guard
“resource type 6”; update the changelog so it matches C-RESOURCES (which inserts
guard as item 5 and renumbers release to 6) by changing any mention of guard
being type 6 to guard being type 5 and ensuring release is documented as type 6;
verify related references in the v0.5.0 entry that mention C-CRUD-VERBS or
C-RESOURCE-MODEL to reflect the corrected resource numbering.
- Around line 214-217: The RFC's guard-delete contract must also require
checking work-item waivers for references: update the normative text to include
verification.waivers[*].guard alongside verification.required_guards and
verification.default_guards so deletion MUST verify no work items reference the
guard ID in verification.required_guards or in verification.waivers[*].guard and
MUST verify the guard is not listed in verification.default_guards in
gov/config.toml, and MUST error with a list of referencing artifacts/config if
any references exist; reference these exact symbols
(verification.waivers[*].guard, verification.required_guards,
verification.default_guards, gov/config.toml) when editing the prose.
In `@src/cmd/guard.rs`:
- Around line 19-21: Slug produced by slugify(title) can be empty or start with
a digit, which yields invalid guard IDs; after computing let slug =
slugify(title).to_uppercase().replace('_', "-"), validate slug and reject the
input if slug.is_empty() or slug.chars().next().map_or(false, |c|
c.is_ascii_digit()); in that case return an explicit error/result indicating the
title is invalid instead of proceeding to let id = format!("GUARD-{slug}"), so
the invalid title fails fast (refer to the slug and id variables and the slugify
call to locate the change).
---
Duplicate comments:
In `@src/cmd/guard.rs`:
- Around line 41-45: The scaffolded GuardCheck uses a success-returning
placeholder command; change the default check.command in the GuardCheck
initializer to a failing placeholder (e.g., "false" or "sh -c 'exit 1'") so new
guards fail until configured; update the initializer where GuardCheck is
constructed (the check field in the Guard struct) to use the failing command
string and leave timeout_secs/pattern as-is.
- Around line 76-113: The blocker detection (building blockers via
config.verification.default_guards and scanning work items with
crate::parse::load_work_items into the blockers Vec) must run unconditionally;
remove the surrounding gate "if !force && !op.is_preview() { ... }" so those
checks always execute, and only use force/preview to skip the subsequent
confirmation step (not the detection). Keep the existing code that returns an
error when blockers is_non_empty, and adjust caller logic or add a separate
confirmation branch that is conditional on force or op.is_preview(); also update
the CLI help text and the test
tests/test_guard.rs::test_guard_delete_force_bypasses_checks to reflect that
--force does not bypass blocker checks but only skips confirmation.
In `@src/cmd/list.rs`:
- Around line 57-61: The branch for ListTarget::Guard converts a Diagnostic into
an anyhow error, bypassing the standard diagnostic renderer; instead propagate
the Diagnostic directly: call load_guards_with_warnings(config) without map_err
(e.g. let result = load_guards_with_warnings(config)?), keep calling
list_guards(&result.items, filter, limit, output), and return result.warnings;
ensure the surrounding function's Result type propagates the Diagnostic so
diagnostics follow the normal rendering path.
- Around line 360-378: The code currently truncates command strings inside the
map that builds GuardSummary (in the items.iter() -> GuardSummary creation),
using &cmd[..50] which can panic on non-ASCII and also loses the full command
for JSON/plain output; instead store the full command in GuardSummary.command
(remove the truncation logic in the iterator) and perform any visual shortening
only in the table renderer path passed to output_list (implement a
char-boundary-safe truncation helper, e.g. using
chars().take(n).collect::<String>() or a unicode-segmentation helper) so
JSON/plain serializers still receive the complete command while the table view
shows a shortened version.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: af61318b-73b5-4b13-afeb-bf244b214f70
📒 Files selected for processing (23)
.claude/skills/guard-writer/SKILL.mdCHANGELOG.mdCLAUDE.mdbuild.rsdocs/rfc/RFC-0002.mdgov/rfc/RFC-0002/clauses/C-CRUD-VERBS.tomlgov/rfc/RFC-0002/clauses/C-RESOURCE-MODEL.tomlgov/rfc/RFC-0002/clauses/C-RESOURCES.tomlgov/rfc/RFC-0002/rfc.tomlgov/schema/edit-ops.jsongov/schema/edit-ops.schema.jsongov/work/2026-03-27-add-guard-resource-subcommands-new-list-show-set-delete.tomlsrc/cli.rssrc/cmd/edit/adapter.rssrc/cmd/edit/mod.rssrc/cmd/edit/runtime.rssrc/cmd/guard.rssrc/cmd/list.rssrc/cmd/mod.rssrc/cmd/new.rssrc/command_router.rssrc/parse.rstests/test_guard.rs
✅ Files skipped from review due to trivial changes (7)
- src/cmd/edit/runtime.rs
- gov/rfc/RFC-0002/clauses/C-RESOURCE-MODEL.toml
- CHANGELOG.md
- .claude/skills/guard-writer/SKILL.md
- gov/schema/edit-ops.json
- CLAUDE.md
- gov/work/2026-03-27-add-guard-resource-subcommands-new-list-show-set-delete.toml
🚧 Files skipped from review as they are similar to previous changes (9)
- src/cmd/mod.rs
- build.rs
- gov/schema/edit-ops.schema.json
- src/parse.rs
- src/cmd/edit/adapter.rs
- gov/rfc/RFC-0002/clauses/C-RESOURCES.toml
- src/cmd/edit/mod.rs
- gov/rfc/RFC-0002/clauses/C-CRUD-VERBS.toml
- gov/rfc/RFC-0002/rfc.toml
Amend RFC-0002 to v0.5.0 adding guard to C-RESOURCES, C-CRUD-VERBS, and C-RESOURCE-MODEL. Implement govctl guard new/list/show/get/set/ add/remove/delete following the standard resource model pattern. Includes guard-writer skill, edit-ops rules, GuardTomlAdapter, safety checks on delete (blocks when referenced by config or work items), and 10 integration tests. Closes WI-2026-03-27-001.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/cmd/list.rs (1)
67-72:⚠️ Potential issue | 🟡 MinorGuard error path diverges from the established diagnostic pattern.
Other list targets propagate load failures as
Ok(vec![d]), but the guard branch converts them toanyhow. This inconsistency means guard-related I/O errors will bypass the normal diagnostic renderer.Suggested fix
if target == ListTarget::Guard { - let result = - load_guards_with_warnings(config).map_err(|d| anyhow::anyhow!("{}", d.message))?; + let result = match load_guards_with_warnings(config) { + Ok(r) => r, + Err(d) => return Ok(vec![d]), + }; list_guards(&result.items, filter, limit, output); return Ok(result.warnings); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/list.rs` around lines 67 - 72, The guard branch diverges by turning load_guards_with_warnings errors into anyhow errors; instead change the error path to return the diagnostics Vec like the other targets. In the ListTarget::Guard case, call load_guards_with_warnings(config) and, on Err(d), return Ok(vec![d]) (or otherwise propagate the diagnostics container the same way as the other branches) before calling list_guards(&result.items, filter, limit, output) so guard I/O failures are rendered by the normal diagnostic renderer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/cmd/list.rs`:
- Around line 67-72: The guard branch diverges by turning
load_guards_with_warnings errors into anyhow errors; instead change the error
path to return the diagnostics Vec like the other targets. In the
ListTarget::Guard case, call load_guards_with_warnings(config) and, on Err(d),
return Ok(vec![d]) (or otherwise propagate the diagnostics container the same
way as the other branches) before calling list_guards(&result.items, filter,
limit, output) so guard I/O failures are rendered by the normal diagnostic
renderer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10ebb892-9d14-47a6-a9f0-57ba9a14013d
📒 Files selected for processing (23)
.claude/skills/guard-writer/SKILL.mdCHANGELOG.mdCLAUDE.mdbuild.rsdocs/rfc/RFC-0002.mdgov/rfc/RFC-0002/clauses/C-CRUD-VERBS.tomlgov/rfc/RFC-0002/clauses/C-RESOURCE-MODEL.tomlgov/rfc/RFC-0002/clauses/C-RESOURCES.tomlgov/rfc/RFC-0002/rfc.tomlgov/schema/edit-ops.jsongov/schema/edit-ops.schema.jsongov/work/2026-03-27-add-guard-resource-subcommands-new-list-show-set-delete.tomlsrc/cli.rssrc/cmd/edit/adapter.rssrc/cmd/edit/mod.rssrc/cmd/edit/runtime.rssrc/cmd/guard.rssrc/cmd/list.rssrc/cmd/mod.rssrc/cmd/new.rssrc/command_router.rssrc/parse.rstests/test_guard.rs
✅ Files skipped from review due to trivial changes (6)
- gov/rfc/RFC-0002/clauses/C-RESOURCE-MODEL.toml
- src/cmd/mod.rs
- CHANGELOG.md
- .claude/skills/guard-writer/SKILL.md
- src/cmd/edit/runtime.rs
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (9)
- src/cmd/new.rs
- gov/work/2026-03-27-add-guard-resource-subcommands-new-list-show-set-delete.toml
- gov/schema/edit-ops.schema.json
- gov/rfc/RFC-0002/rfc.toml
- src/cmd/guard.rs
- gov/rfc/RFC-0002/clauses/C-CRUD-VERBS.toml
- src/cli.rs
- docs/rfc/RFC-0002.md
- gov/schema/edit-ops.json
This was referenced Apr 6, 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
1 participant
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.
Amend RFC-0002 to v0.5.0 adding guard to C-RESOURCES, C-CRUD-VERBS, and C-RESOURCE-MODEL. Implement govctl guard new/list/show/get/set/ add/remove/delete following the standard resource model pattern.
Includes guard-writer skill, edit-ops rules, GuardTomlAdapter, safety checks on delete (blocks when referenced by config or work items), and 10 integration tests.
Closes WI-2026-03-27-001.
Summary by CodeRabbit
New Features
Behavior
Documentation
Tests