Dev#22
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughAdds loop plan freshness detection (compute/display/validate, block open-round until replanned), generates skill assets at build time and uses them in sync/init, and hard-codes cargo-binstall archive URLs; includes RFC/docs/work-item updates and tests. ChangesLoop Plan Freshness Detection
Build-time Skill Asset Generation
Cargo-binstall URL Configuration
Governance and Documentation Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/cmd/self_update_tests.rs (1)
131-140: ⚡ Quick winConsider validating Windows override
pkg-fmtfor completeness.The test validates the Windows override
pkg-urlbut notpkg-fmt. SinceCargo.tomlline 109 specifiespkg-fmt = "zip"for the Windows override, the test should verify this field to ensure the override is fully validated.🧪 Suggested validation addition
windows_pkg_url, "{ repo }/releases/download/v{ version }/govctl-v{ version }-{ target }.zip" ); +let windows_pkg_fmt = binstall + .get("overrides") + .and_then(|overrides| overrides.get("x86_64-pc-windows-msvc")) + .and_then(|windows| windows.get("pkg-fmt")) + .and_then(toml::Value::as_str) + .ok_or("missing package.metadata.binstall.overrides.x86_64-pc-windows-msvc.pkg-fmt")?; +assert_eq!(windows_pkg_fmt, "zip");🤖 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 `@src/cmd/self_update_tests.rs` around lines 131 - 140, The test currently extracts and asserts windows_pkg_url (variable windows_pkg_url) but doesn't validate the Windows override's "pkg-fmt"; update the test to also read the "pkg-fmt" value from the same override chain (use the same binstall.get(...).and_then(...).and_then(...).and_then(toml::Value::as_str).ok_or(...) pattern targeting "pkg-fmt") and add an assertion (similar to assert_eq!) that its value equals "zip" to match the Cargo.toml override.
🤖 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 `@build.rs`:
- Around line 16-17: Update the top-of-file comment that currently reads
"Project-local assets installed by `govctl init-skills`. Plugin/global-only
skills such as `init` are intentionally omitted." to include an inline RFC
clause citation (for example `[[RFC-0002:C-GLOBAL-COMMANDS]]`) immediately after
the invariant statement so the policy is traceable; locate that exact comment in
build.rs and append the RFC citation to the sentence describing omitted
plugin/global-only skills.
In `@Cargo.toml`:
- Around line 102-109: Add a one-line ADR citation comment above the
package.metadata.binstall section to record that the release asset naming
convention (pkg-url and pkg-fmt) implements ADR-0041; reference the section and
the Windows override by name (package.metadata.binstall and
package.metadata.binstall.overrides.x86_64-pc-windows-msvc) so reviewers can
trace this config back to ADR-0041.
In `@tests/test_agent_dir.rs`:
- Around line 34-38: The test currently only asserts the SKILL.md file absence
(variable init_skill); also assert the containing directory is not created by
constructing a path for the directory (e.g., let init_dir =
temp_dir.path().join(".claude/skills/init");) and add
assert!(!init_dir.exists(), "init is a plugin/global onboarding skill, not a
project-local init-skills asset") to ensure the directory itself is excluded;
update tests/test_agent_dir.rs accordingly (use the existing temp_dir and same
error message).
---
Nitpick comments:
In `@src/cmd/self_update_tests.rs`:
- Around line 131-140: The test currently extracts and asserts windows_pkg_url
(variable windows_pkg_url) but doesn't validate the Windows override's
"pkg-fmt"; update the test to also read the "pkg-fmt" value from the same
override chain (use the same
binstall.get(...).and_then(...).and_then(...).and_then(toml::Value::as_str).ok_or(...)
pattern targeting "pkg-fmt") and add an assertion (similar to assert_eq!) that
its value equals "zip" to match the Cargo.toml override.
🪄 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: 58464255-3031-4b97-85e1-8935d4e32102
📒 Files selected for processing (21)
.claude/skills/gov/SKILL.md.claude/skills/wi-writer/SKILL.mdCHANGELOG.mdCargo.tomlbuild.rsdocs/guide/recommended-workflows.mddocs/rfc/RFC-0002.mdgov/rfc/RFC-0002/clauses/C-GLOBAL-COMMANDS.tomlgov/rfc/RFC-0002/rfc.tomlgov/work/2026-06-08-fix-cargo-binstall-unix-asset-suffix.tomlgov/work/2026-06-08-support-bundled-skill-installation.tomlgov/work/2026-06-09-detect-stale-loop-plans.tomlsrc/cmd/loop_cmd/execution/mod.rssrc/cmd/loop_cmd/mod.rssrc/cmd/loop_cmd/output.rssrc/cmd/loop_cmd/state.rssrc/cmd/new/skills.rssrc/cmd/self_update_tests.rstests/loop_tests/scope.rstests/loop_tests/surface_cases/listing.rstests/test_agent_dir.rs
Reject stale cached loop dependency closures before opening new rounds, and expose fresh/stale plan status in loop show/list.
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.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests