Repository navigation
feat(skills): make validation proportional across repositories - #2725
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughThe change defines proportional validation rules for Genie and updates canonical and plugin ChangesProportional validation policy
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Apply risk-proportional validation across target repositories while preserving aggregate integration and release gates.
ac8722a to
e09b066
Compare
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e09b06660e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - **Validation:** `bun run wishes:lint`, `bun run skills:lint`, `bun scripts/sync-plugin-skills.ts --check`, | ||
| `bun test scripts/sync-plugin-skills.test.ts` (8 pass, 0 fail), and `git diff --check` all passed. |
There was a problem hiding this comment.
For this Genie-repo skill/policy change, the recorded validation lists only focused lints/parity checks and the mirror test, but the repository contract still requires the full gate (bun run check) for changes under this tree. Because this wish is also establishing that repository-required aggregate gates remain intact, shipping it with evidence that omits the documented full gate lets future reviews cite an under-validated precedent and can miss type/dead-code/test failures covered only by the full check.
AGENTS.md reference: AGENTS.md:L5-L13
Useful? React with 👍 / 👎.
| - **Finding:** The amended criterion consistently distinguishes generated executable/runtime artifacts from | ||
| deterministic documentation/plugin mirrors and preserves the aggregate-gate boundary without adding machinery. | ||
|
|
||
| ### Group 1 execution review — 2026-07-28 |
There was a problem hiding this comment.
This execution-review evidence records a SHIP verdict without naming who performed the review, so the wish cannot demonstrate that the reviewer was distinct from the engineer for the group. That makes the review evidence non-auditable and allows self-review to be accepted by omission, which the repo contract explicitly forbids.
AGENTS.md reference: AGENTS.md:L36-L36
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@plugins/genie/skills/work/SKILL.md`:
- Around line 31-38: Update the “Validate” guidance in the shared skill to
remove the hardcoded “Bash” client reference and require running the validation
command through the active runtime’s native command surface. Preserve the
existing evidence, scope-rationale, proportional-check, and nonzero-validation
requirements.
In `@skills/work/SKILL.md`:
- Line 31: Remove the hardcoded “Bash” reference from the Validate step in the
shared skill instructions, replacing it with runtime-neutral wording that
directs the agent to use the active runtime’s native command-execution surface
while preserving the requirement to run and record the validation command and
scope rationale.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7bc8fab6-d372-4941-bead-32163d9492bb
📒 Files selected for processing (7)
.genie/wishes/proportional-validation-policy/WISH.mdplugins/genie/skills/review/SKILL.mdplugins/genie/skills/wish/SKILL.mdplugins/genie/skills/work/SKILL.mdskills/review/SKILL.mdskills/wish/SKILL.mdskills/work/SKILL.md
| 7. **Validate:** run the group's validation command yourself (Bash); record the output and the scope rationale as | ||
| evidence. Confirm it remains proportional to the actual diff, widening it when implementation reached beyond the | ||
| plan: documentation-only changes, including deterministic generated documentation or plugin skill mirrors, use | ||
| relevant format, link, example, generator, parity, or content-contract checks; runtime changes use focused behavior | ||
| tests and add type, lint, or build checks for boundaries reached. Shared runtime/core behavior, dependency or lockfile, | ||
| generated executable or runtime artifact, configuration or schema, CI or release, broad-refactor, or uncertain-impact | ||
| changes require the repository full gate plus affected build or end-to-end checks. Validation may never be zero, and | ||
| an unexplained full-suite run is not acceptable evidence. Preserve required aggregate integration and release gates. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the hardcoded Bash client reference.
This shared skill should require direct execution through the active runtime’s native command surface, not mandate Bash by name.
Proposed wording
-7. **Validate:** run the group's validation command yourself (Bash); record the output and the scope rationale as
+7. **Validate:** run the group's validation command directly through the active runtime's native command surface; record the output and the scope rationale as📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 7. **Validate:** run the group's validation command yourself (Bash); record the output and the scope rationale as | |
| evidence. Confirm it remains proportional to the actual diff, widening it when implementation reached beyond the | |
| plan: documentation-only changes, including deterministic generated documentation or plugin skill mirrors, use | |
| relevant format, link, example, generator, parity, or content-contract checks; runtime changes use focused behavior | |
| tests and add type, lint, or build checks for boundaries reached. Shared runtime/core behavior, dependency or lockfile, | |
| generated executable or runtime artifact, configuration or schema, CI or release, broad-refactor, or uncertain-impact | |
| changes require the repository full gate plus affected build or end-to-end checks. Validation may never be zero, and | |
| an unexplained full-suite run is not acceptable evidence. Preserve required aggregate integration and release gates. | |
| 7. **Validate:** run the group's validation command directly through the active runtime's native command surface; record the output and the scope rationale as | |
| evidence. Confirm it remains proportional to the actual diff, widening it when implementation reached beyond the | |
| plan: documentation-only changes, including deterministic generated documentation or plugin skill mirrors, use | |
| relevant format, link, example, generator, parity, or content-contract checks; runtime changes use focused behavior | |
| tests and add type, lint, or build checks for boundaries reached. Shared runtime/core behavior, dependency or lockfile, | |
| generated executable or runtime artifact, configuration or schema, CI or release, broad-refactor, or uncertain-impact | |
| changes require the repository full gate plus affected build or end-to-end checks. Validation may never be zero, and | |
| an unexplained full-suite run is not acceptable evidence. Preserve required aggregate integration and release gates. |
🤖 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 `@plugins/genie/skills/work/SKILL.md` around lines 31 - 38, Update the
“Validate” guidance in the shared skill to remove the hardcoded “Bash” client
reference and require running the validation command through the active
runtime’s native command surface. Preserve the existing evidence,
scope-rationale, proportional-check, and nonzero-validation requirements.
Source: Coding guidelines
| 5. **Local review:** per finished group, dispatch a reviewer subagent (reviewer ≠ engineer) to run `review` against that group's acceptance criteria. The orchestrator appends each returned evidence block under `## Review Results`; the reviewer never edits it. Diagnose before fixing: `overdesigned-plan` returns to wish/design review without consuming a fix attempt; other FIX-FIRST gaps may use at most 2 fix loops. | ||
| 6. **Quality review:** dispatch a reviewer for a quality pass (security, maintainability, perf). On FIX-FIRST, one fix loop. | ||
| 7. **Validate:** run the group's validation command yourself (Bash); record the output as evidence. | ||
| 7. **Validate:** run the group's validation command yourself (Bash); record the output and the scope rationale as |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the hardcoded Bash client name.
This shared skill must remain runtime-neutral. Replace (Bash) with wording such as “using the active runtime’s native command-execution surface”; otherwise non-Bash clients receive an invalid execution instruction.
Proposed fix
-7. **Validate:** run the group's validation command yourself (Bash); record the output and the scope rationale as
+7. **Validate:** run the group's validation command using the active runtime's native command-execution surface; record the output and the scope rationale as📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 7. **Validate:** run the group's validation command yourself (Bash); record the output and the scope rationale as | |
| 7. **Validate:** run the group's validation command using the active runtime's native command-execution surface; record the output and the scope rationale as |
🤖 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 `@skills/work/SKILL.md` at line 31, Remove the hardcoded “Bash” reference from
the Validate step in the shared skill instructions, replacing it with
runtime-neutral wording that directs the agent to use the active runtime’s
native command-execution surface while preserving the requirement to run and
record the validation command and scope rationale.
Source: Coding guidelines
…lways valid evidence - work/review: a passing full-suite run counts as evidence; a missing scope rationale is a write-up gap (at most MEDIUM), never by itself grounds for FIX-FIRST - wish/work/review: a repository-documented gate is by itself sufficient justification for its scope - align dispatch-brief validation example and wish acceptance criterion - mirrors regenerated via sync-plugin-skills --write
There was a problem hiding this comment.
♻️ Duplicate comments (1)
skills/work/SKILL.md (1)
31-31:⚠️ Potential issue | 🟠 MajorUse runtime-neutral command execution in both work skills.
The canonical skill and its plugin mirror both require
Bashby name, which conflicts with their multi-runtime native delegation model.
skills/work/SKILL.md#L31-L31: replace(Bash)with the active runtime’s native command-execution surface.plugins/genie/skills/work/SKILL.md#L31-L31: apply the identical runtime-neutral wording.🤖 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 `@skills/work/SKILL.md` at line 31, Update the validation instruction at skills/work/SKILL.md:31 to replace the Bash-specific reference with wording that directs execution through the active runtime’s native command-execution surface. Apply the identical change at plugins/genie/skills/work/SKILL.md:31 so both mirrored work skills remain consistent.Source: Coding guidelines
🤖 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.
Duplicate comments:
In `@skills/work/SKILL.md`:
- Line 31: Update the validation instruction at skills/work/SKILL.md:31 to
replace the Bash-specific reference with wording that directs execution through
the active runtime’s native command-execution surface. Apply the identical
change at plugins/genie/skills/work/SKILL.md:31 so both mirrored work skills
remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6acd1916-d1f8-460f-8220-47c974532b2e
📒 Files selected for processing (7)
.genie/wishes/proportional-validation-policy/WISH.mdplugins/genie/skills/review/SKILL.mdplugins/genie/skills/wish/SKILL.mdplugins/genie/skills/work/SKILL.mdskills/review/SKILL.mdskills/wish/SKILL.mdskills/work/SKILL.md
Summary
Why
A narrow change such as a README edit should use focused documentation checks instead of automatically running an entire application test suite. Broader runtime, dependency, schema, CI, release, and uncertain-impact changes still escalate to the target repository full gate.
Validation
The aggregate suite was intentionally not run because this change only modifies workflow policy documents and deterministic plugin mirrors.
Closes #2724
Summary by CodeRabbit