feat(marketplace)!: recalibrate planning and verification skills for tiered-model execution#150
Open
anderskev wants to merge 6 commits into
Open
feat(marketplace)!: recalibrate planning and verification skills for tiered-model execution#150anderskev wants to merge 6 commits into
anderskev wants to merge 6 commits into
Conversation
…ndations Phase 0 of the tiered-model skill modernization. Adds the two internal skills that later phases import instead of restating their own gates. execution-contract defines what an execution-tier model is trusted to decide, and gives it a legal way to stop: failure matching by shape rather than by the planner's guessed string, a baseline-diffed pre-existing-red policy, a two-attempt budget, and a stop-and-report template. Halting with a report is stated explicitly as a success state. verification-budget defines reversibility-based risk tiers, the required loop-budget syntax (max passes, observable stop condition, proceed-and-flag tie-break), the one-echo rule, and a ban on prove-a-negative checks. Also fixes a broken link in the brainstorm to write-plan handoff chain: spec-template.md and resolve-beagle both pointed at docs/specs/, while brainstorm-beagle writes to .beagle/concepts/<slug>/spec.md, so resolve-beagle could not find the specs it exists to resolve. quick-plan's frontmatter description was 1033 chars against a 1024 limit; trimmed to 645 without losing a trigger phrase. Co-Authored-By: Claude Opus 4.8 <[email protected]> Entire-Checkpoint: 54782f8ae050
…execution BREAKING CHANGE: emitted plan.md gains a mandatory Executor Contract section; review/verify skills import shared vocabulary instead of restating their own gates. In-flight plans and specs keep working, but plans emitted before this change carry no attempt budget. These skills assumed one expensive model both planned and executed. The workflow now uses planning-tier models to plan and review while execution-tier models execute, and the old framing burned tokens by over-verifying and getting stuck. Five root causes, all fixed: Unbounded loops. No verification loop declared a cap or stop condition. Every remaining loop now declares max passes, an observable stop condition, and a proceed-and-flag tie-break. The "advance only when every item is honestly yes" gate is gone from write-plan, quick-plan, brainstorm-beagle, and verify-llm-artifacts. Criteria are preserved -- only the delivery mechanism changed. No legal way to stop. Plans now open with an Executor Contract inlining beagle-core:execution-contract, so a blocked executor halts and reports rather than grinding. This closes four stuck-states, including matching against a planner-guessed failure string and an absolute zero-regressions demand with no pre-existing-red policy. Redundant ceremony. Per-verdict echo gates defended against one long-lived context adjudicating code it never read -- a much weaker risk with fresh subagent contexts. Replaced by the one-echo rule, retaining a staleness re-read before irreversible actions. Risk tiering. verify-llm-artifacts collapses five hard gates into one tier-based gate. fix-llm-artifacts keeps every protective control and drops only the duplicate echo and ID-lock. Duplication. write-plan 462 -> 181 lines; review-verification-protocol 1848 -> 926 across seven copies; detection criteria 1046 -> 298 with per-language examples now loaded conditionally. Handoffs preserved: spec.md -> write-plan -> plan.md, quick-plan's sibling tree, subagent-prompt, the spec's eight sections and resolve-beagle hook, and both artifacts JSON schemas field-for-field. Co-Authored-By: Claude Opus 4.8 <[email protected]> Entire-Checkpoint: 068a0c6c4f08
…t per finding
BREAKING CHANGE: code review skills across all seven plugins now apply
review-verification-protocol once at review entry rather than once per
reported finding.
The protocol refactor in the previous commit was only half the win. A
repo-wide grep found ~55 call sites that embedded the per-finding framing
themselves -- "completed for every reported issue", "Run once per
FastAPI-related finding", "For every substantive finding, you can name
which protocol subsection you satisfied". A perfectly budgeted core
protocol still burns the same tokens when 55 call sites independently
demand per-finding application.
Every call site now loads the protocol once and applies it in a single
pass over the assembled finding list. Reporting a finding is
verification-budget tier REVERSIBLE and earns no ceremony; only a finding
whose recommended action is IRREVERSIBLE keeps the full evidence gate.
Eight prove-a-negative demands ("search ALL references", "did you search
all references?") became enumerated pattern lists reported as match
counts, since absence over an open search space has no terminating check.
Every remaining loop declares max passes, an observable stop condition,
and a proceed-and-flag tie-break.
Also resolves the one dangling "Anti-confabulation (gate 0)" link left by
the protocol refactor, and review-structure's local restatement of that
gate, against verification-budget's one-echo rule.
What each review skill looks for is unchanged -- this alters how often
and how expensively the protocol runs, not the review criteria.
review-llm-artifacts keeps its producer-side echo as the pipeline's
single upstream echo; its JSON schema is byte-identical.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Entire-Checkpoint: f4b02dbf0861
anderskev
added
enhancement
…describing terms
The planning skills referenced "tier-3 tests" as though the reader shared a
tiering scheme that beagle never defines anywhere. The term comes from one
project's test taxonomy; a plan-writing model reading "enumerate every
tier-3 trigger" against a repo with no such scheme will either invent one
or stall. This marketplace is installed across many projects, so it cannot
assume a private convention.
The underlying concept is real and stays: some behavior cannot be proven
below the real entrypoint -- launch/bootstrap, CLI arg parsing, env-var
resolution, pty/signal handling, shell scripts whose contract is shell
semantics. That checklist is now named for what it is ("real-entrypoint
coverage") and instructs the planner to use the project's own vocabulary,
read from its AGENTS.md/CLAUDE.md, rather than importing a taxonomy.
Also drops the numeric tier aliases from verification-budget, which had the
same defect: it named its tiers REVERSIBLE and IRREVERSIBLE in the table but
then referred to "tier 1" in prose. Tiers are now always named, never
numbered.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
Entire-Checkpoint: 1193a8c27f14
Entire is personal tooling, not a repo dependency. Committing its harness integrations would opt every contributor who has the CLI installed into session capture to a checkpoint remote, which is a per-developer choice rather than a project one. Untracks .entire/settings.json (added in #148) and ignores the harness integrations Entire generates for Claude Code, Codex, Cursor, opencode, and pi. The Claude Code hooks move to .claude/settings.local.json so local setup keeps working while the tracked .claude/settings.json returns to just enabledPlugins. .codex/INSTALL.md stays tracked -- that is Beagle's own Codex support doc referenced from CLAUDE.md, not an Entire artifact. Only .codex/config.toml and .codex/hooks.json are ignored. Also ignores .claude/settings.local.json, which was previously excluded only by a personal global gitignore and so was unprotected for other contributors. Co-Authored-By: Claude Opus 4.8 <[email protected]> Entire-Checkpoint: c0e61d709d33
Member
Author
|
Two follow-up commits pushed.
The concept is real and stays — some behavior can't be proven below the real entrypoint (launch/bootstrap, CLI arg parsing, env-var resolution, pty/signal handling, shell scripts whose contract is shell semantics). It's now named "real-entrypoint coverage" and instructs the planner to read the project's own vocabulary from its This also caught the same defect in a file added by this PR:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (48)
🚧 Files skipped from review as they are similar to previous changes (28)
WalkthroughThis change introduces execution and verification contracts with bounded attempts, explicit stop conditions, and tiered evidence rules. Planning workflows now use standardized specifications, executor contracts, deterministic task structures, and one-pass reviews. Artifact detection separates language-neutral criteria from Python examples. Core and language-specific review skills now apply verification once per assembled finding list, with enumerated checks for absence claims and specialized guidance for Elixir, Go, iOS, Python, React, and Rust. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/beagle-ios/skills/watchos-code-review/SKILL.md (1)
1-1: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake watchOS and WidgetKit verification review-level, not finding-level.
plugins/beagle-ios/skills/watchos-code-review/SKILL.md#L51-L53: replace the per-finding gates with one pass over the assembled finding list.plugins/beagle-ios/skills/widgetkit-code-review/SKILL.md#L49-L51: apply the same once-per-review rule and stop condition.This is required to match the PR’s bounded verification contract and the other updated review protocols.
🤖 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/beagle-ios/skills/watchos-code-review/SKILL.md` at line 1, Update the watchOS and WidgetKit review protocols to perform verification once per review, after assembling the complete finding list, rather than once for each finding. Replace the per-finding gates in the watchOS and WidgetKit skill instructions with a single review-level pass and preserve the defined stop condition.
🟡 Minor comments (9)
CHANGELOG.md-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a new version section for the release.
Since
marketplace.jsonwas bumped to5.0.0in this PR,CHANGELOG.mdshould reflect this release with a new version section rather than leaving the changes under[Unreleased]. As per coding guidelines, maintainCHANGELOG.mdusing the Keep a Changelog format and add a new version section for releases.📝 Proposed fix to add the version section
-## [Unreleased] +## [Unreleased] + +## [5.0.0] - 2026-07-21🤖 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 `@CHANGELOG.md` at line 7, Update the changelog header around [Unreleased] to add a Keep a Changelog version section for release 5.0.0, moving the release changes out of the unreleased section while preserving the existing changelog entries and format.Sources: Coding guidelines, Learnings
plugins/beagle-core/skills/review-plan/SKILL.md-45-57 (1)
45-57: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the lens-signal count.
The pass condition says “six signals,” but the list contains five lens-selection signals. Either add the missing signal explicitly or change the wording to five; otherwise the gate has an impossible or ambiguous completion condition.
🤖 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/beagle-core/skills/review-plan/SKILL.md` around lines 45 - 57, Correct the inconsistency in the extraction instructions by updating the “six signals” wording to “five signals,” matching the five listed lens-selection signals. Keep the existing signal list and pass-condition behavior unchanged.plugins/beagle-core/skills/execution-contract/SKILL.md-57-73 (1)
57-73: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language identifiers to all fenced Markdown blocks.
Use
textfor report/prompt templates and the appropriate language identifier for code examples so the repository’s Markdown checks pass.
plugins/beagle-core/skills/execution-contract/SKILL.md#L57-L73: tag the stop-and-report template fence.plugins/beagle-core/skills/review-plan/SKILL.md#L95-L200: tag each lens prompt, report-template, and suggested-edit fence.🤖 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/beagle-core/skills/execution-contract/SKILL.md` around lines 57 - 73, Add language identifiers to every fenced Markdown block in plugins/beagle-core/skills/execution-contract/SKILL.md lines 57-73 and plugins/beagle-core/skills/review-plan/SKILL.md lines 95-200: use text for report and prompt templates, and the appropriate language identifier for each code example. Update all lens prompt, report-template, suggested-edit, and stop-and-report fences without changing their contents.Source: Linters/SAST tools
plugins/beagle-analysis/skills/write-plan/references/plan-template.md-9-11 (1)
9-11: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the repository’s bare skill-name reference style consistently.
These Markdown references use the qualified
plugin:skillform. Replace them with bare skill names in prose and tables, reserving qualified forms only for contexts that explicitly require them.
plugins/beagle-analysis/skills/write-plan/references/plan-template.md#L9-L11: referenceexecution-contractwithout the qualified prefix.plugins/beagle-analysis/skills/write-plan/references/planning-disciplines.md#L146-L146: referenceverification-budgetwithout the qualified prefix.plugins/beagle-core/skills/subagent-prompt/SKILL.md#L48-L48: use the repository-standard execution-contract name.plugins/beagle-core/skills/subagent-prompt/SKILL.md#L77-L77: use bare names for the brainstorming, plan-writing, and quick-plan skills.plugins/beagle-core/skills/verify-llm-artifacts/SKILL.md#L11-L11: use the bare verification-budget name.plugins/beagle-core/skills/review-feedback-schema/SKILL.md#L183-L183: use bare names for both referenced protocols.plugins/beagle-core/skills/review-llm-artifacts/SKILL.md#L295-L295: use the bare verification-budget name.plugins/beagle-core/skills/review-plan/SKILL.md#L65-L76: update the skill table entries to the repository-standard bare names.plugins/beagle-core/skills/review-plan/SKILL.md#L77-L77: update the referenced analysis skills consistently.Based on learnings, avoid the outdated
plugin:skillreference format in Markdown and prefer bare skill names.🤖 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/beagle-analysis/skills/write-plan/references/plan-template.md` around lines 9 - 11, Replace outdated qualified plugin:skill references with bare skill names throughout the documentation. Update execution-contract references in plugins/beagle-analysis/skills/write-plan/references/plan-template.md:9-11 and plugins/beagle-core/skills/subagent-prompt/SKILL.md:48; update verification-budget in plugins/beagle-analysis/skills/write-plan/references/planning-disciplines.md:146, plugins/beagle-core/skills/verify-llm-artifacts/SKILL.md:11, and plugins/beagle-core/skills/review-llm-artifacts/SKILL.md:295; update the brainstorming, plan-writing, quick-plan, and review-plan references in plugins/beagle-core/skills/subagent-prompt/SKILL.md:77, plugins/beagle-core/skills/review-feedback-schema/SKILL.md:183, and plugins/beagle-core/skills/review-plan/SKILL.md:65-77. Keep qualified names only where explicitly required.Source: Learnings
plugins/beagle-core/skills/subagent-prompt/SKILL.md-36-36 (1)
36-36: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAvoid unpinned
npxin this example.npx vitest run ...can fall back to a registry download when Vitest isn’t installed locally; use a project-local command ornpx --no-install vitest ..., and pin the version if a download is intentional.🤖 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/beagle-core/skills/subagent-prompt/SKILL.md` at line 36, Update the test-command examples in the skill guidance to avoid unpinned npx execution: replace the npx vitest example with a project-local invocation or npx --no-install, and specify an explicit version only if downloading Vitest is intentional. Preserve the existing scoped, single-command testing guidance.Source: Linters/SAST tools
plugins/beagle-core/skills/llm-artifacts-detection/SKILL.md-45-45 (1)
45-45: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced blocks.
The two fenced prompt blocks omit a language identifier. Use
text(or the appropriate language) after each opening fence.Also applies to: 126-126
🤖 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/beagle-core/skills/llm-artifacts-detection/SKILL.md` at line 45, Add the text language identifier to the opening fences of both fenced prompt blocks in the llm-artifacts-detection skill documentation, including the block referenced near the secondary location, while preserving their contents and closing fences.Sources: Coding guidelines, Linters/SAST tools
plugins/beagle-core/skills/llm-artifacts-detection/references/examples/python/abstraction.md-230-239 (1)
230-239: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClose the final fenced code block.
The
Unused configuration optionsexample opens a```pythonfence but never closes it, leaving the Markdown document structurally invalid.Proposed fix
# config.py class Settings: database_url: str cache_ttl: int = 3600 max_retries: int = 3 enable_metrics: bool = True # Never read! legacy_mode: bool = False # Never read! debug_sql: bool = False # Never read! +```🤖 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/beagle-core/skills/llm-artifacts-detection/references/examples/python/abstraction.md` around lines 230 - 239, Close the unclosed fenced code block in the “Unused configuration options” example after the Settings class, adding the matching Markdown fence so the document structure is valid.Source: Coding guidelines
plugins/beagle-core/skills/llm-artifacts-detection/references/examples/python/dead-code.md-141-141 (1)
141-141: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSpecify a language for the fenced block.
The orphaned-tests directory-tree example uses an untyped fence, violating the repository’s Markdown validation rule.
-``` +```text🤖 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/beagle-core/skills/llm-artifacts-detection/references/examples/python/dead-code.md` at line 141, Update the orphaned-tests directory-tree fenced code block in dead-code.md to specify the text language by changing its untyped fence to a text-labeled fence, while preserving the example content.Sources: Coding guidelines, Linters/SAST tools
plugins/beagle-go/skills/go-code-review/SKILL.md-21-22 (1)
21-22: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGate on the verdict, not the recommendation.
The core risk-tier contract applies the full evidence gate only when the verdict authorizes an irreversible action. These lines trigger it based on the recommended action, which changes the protocol semantics. Use “verdict authorizes” consistently in both Go skills.
🤖 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/beagle-go/skills/go-code-review/SKILL.md` around lines 21 - 22, Update the pre-report verification guidance in both plugins/beagle-go/skills/go-code-review/SKILL.md:21-22 and plugins/beagle-go/skills/go-testing-code-review/SKILL.md:21-22 to apply the full evidence gate only when the verdict authorizes an irreversible action, not when the recommended action is irreversible; use “verdict authorizes” consistently while preserving the existing reversible-finding and one-pass budget rules.
🤖 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/beagle-core/skills/fix-llm-artifacts/SKILL.md`:
- Around line 81-92: Update the findings integrity check in the documented jq
command to fail closed: wrap it in an explicit if ! jq condition and exit 1 on
validation failure instead of using || echo. Validate that findings is
non-empty, IDs are integers forming a contiguous 1..N sequence with no
duplicates, and enforce the upstream summary-count invariants alongside the
existing type checks.
In
`@plugins/beagle-core/skills/llm-artifacts-detection/references/dead-code-criteria.md`:
- Around line 9-26: Update the dead-code criteria in
plugins/beagle-core/skills/llm-artifacts-detection/references/dead-code-criteria.md,
covering the unused-symbol patterns and “Before flagging” guidance, to exempt
framework callbacks, lifecycle hooks, and protocol/interface members; include
checks for framework registration, dynamic dispatch, and string-based invocation
alongside ordinary references. Update the unused-code detection guidance in
plugins/beagle-core/skills/llm-artifacts-detection/SKILL.md at lines 73-79 to
perform the same framework-invocation check before emitting findings.
In `@plugins/beagle-core/skills/review-feedback-schema/SKILL.md`:
- Around line 150-152: Update the verification-budget guidance in the review
feedback schema so unresolved items are never dropped: replace the “or drop it”
tie-break with a deterministic status such as INCONCLUSIVE, and require the
unresolved finding to remain in the output while preserving the one-pass budget.
In `@plugins/beagle-core/skills/review-plan/SKILL.md`:
- Around line 257-269: Update the choice list in the review-plan skill to remove
the “Apply fixes now” option that permits editing the plan; replace it with a
handoff to a separate editing session or explicitly state that editing occurs
outside this skill. Preserve the existing “Save & fix later” and “Proceed
anyway” behavior, and keep the non-editing contract consistent throughout.
In `@plugins/beagle-core/skills/review-verification-protocol/SKILL.md`:
- Around line 25-34: The gate procedure in the review-verification protocol
currently requires a pass once per finding; update the scope to perform exactly
one pass over the complete assembled finding list, consistent with the later
single-pass instruction. Adjust the opening directive and loop-budget wording to
describe shared review-level verification while preserving the existing gate
outputs and stop behavior.
In `@plugins/beagle-core/skills/verification-budget/SKILL.md`:
- Line 9: Update the verification-budget references to use the defined risk-tier
name IRREVERSIBLE instead of the undefined “tier 1,” including the downstream
skill import example, while preserving the existing evidence-gate behavior.
In
`@plugins/beagle-core/skills/verify-llm-artifacts/references/verification-checklist.md`:
- Around line 7-16: Update the “Existence precondition (first check)” workflow
to require git cat-file -e against source_git_head whenever that revision is
provided, and perform all cited-symbol and usage checks against that revision.
Allow test -f only when no source revision is available, preserving the existing
missing-file adjudication and retry budget behavior.
In `@plugins/beagle-elixir/skills/elixir-performance-review/SKILL.md`:
- Line 57: Align the verification tie-break wording with the shared contract
across all listed sites: in
plugins/beagle-elixir/skills/elixir-performance-review/SKILL.md:57-57,
plugins/beagle-elixir/skills/elixir-security-review/SKILL.md:53-53,
plugins/beagle-elixir/skills/exunit-code-review/SKILL.md:70-70,
plugins/beagle-elixir/skills/phoenix-code-review/SKILL.md:65-65,
plugins/beagle-react/skills/react-router-code-review/SKILL.md:71-71,
plugins/beagle-react/skills/review-frontend/SKILL.md:89-89,
plugins/beagle-react/skills/review-remix-v2/SKILL.md:111-111,
plugins/beagle-react/skills/review-verification-protocol/SKILL.md:91-91,
plugins/beagle-react/skills/shadcn-code-review/SKILL.md:34-34, and
plugins/beagle-rust/skills/axum-code-review/SKILL.md:24-24, remove any option to
drop unresolved findings and require proceeding with them flagged or reported as
suspected/questions.
In `@plugins/beagle-elixir/skills/review-elixir/SKILL.md`:
- Line 20: Change beagle-core:review-verification-protocol so its Pre-Report
Verification Checklist runs once over the assembled finding list, not once per
finding, while preserving the existing REVERSIBLE verification-budget semantics.
In plugins/beagle-elixir/skills/review-elixir/SKILL.md lines 20, 71, and
118-123; plugins/beagle-go/skills/bubbletea-code-review/SKILL.md line 16;
plugins/beagle-go/skills/go-code-review/SKILL.md lines 31 and 154;
plugins/beagle-go/skills/go-testing-code-review/SKILL.md line 31;
plugins/beagle-rust/skills/review-rust/SKILL.md lines 20 and 152; and
plugins/beagle-rust/skills/rust-testing-code-review/SKILL.md lines 23 and 202,
update downstream wording only where needed to reference the core list-level
contract; otherwise no direct changes are required at those sites.
In `@plugins/beagle-ios/skills/watchos-code-review/SKILL.md`:
- Around line 51-53: The review-verification gates are incorrectly specified as
per-finding, conflicting with the once-per-review contract. Update the gate
instructions near the review entry protocol to run one pass over the assembled
finding list, with a single review-level stop condition, while retaining the
existing one-time protocol load and evidence-budget behavior.
In `@plugins/beagle-ios/skills/widgetkit-code-review/SKILL.md`:
- Around line 49-51: Update the review-verification instructions around the
three gates to apply them once to the assembled finding list rather than once
per finding. Remove the per-finding pass and budget language, while preserving
the once-at-review-entry protocol loading and the requirement to stop after all
invoked gate artifacts are recorded.
In `@plugins/beagle-react/skills/review-frontend/SKILL.md`:
- Line 92: Preserve the core four-pattern unused-reference check in both
frontend skills, including string-literal or dynamic references, rather than
replacing it with JSX-only wording. In
plugins/beagle-react/skills/review-frontend/SKILL.md at lines 92-92, retain
dynamic-reference coverage and add JSX-only references; in
plugins/beagle-react/skills/review-remix-v2/SKILL.md at lines 114-114, retain
dynamic-reference coverage and add JSX and Remix route-file conventions. Ensure
reports count each core pattern and say “no matches across the 4 enumerated
patterns” when applicable.
In `@plugins/beagle-rust/skills/ffi-code-review/SKILL.md`:
- Line 34: Make the Gate 4 verification requirement list-level and single-pass
rather than per-issue: update
plugins/beagle-rust/skills/ffi-code-review/SKILL.md at line 34 and
plugins/beagle-rust/skills/macros-code-review/SKILL.md at line 33 to remove any
remaining per-issue Gate 4 wording while preserving the one-pass
assembled-finding-list constraint.
---
Outside diff comments:
In `@plugins/beagle-ios/skills/watchos-code-review/SKILL.md`:
- Line 1: Update the watchOS and WidgetKit review protocols to perform
verification once per review, after assembling the complete finding list, rather
than once for each finding. Replace the per-finding gates in the watchOS and
WidgetKit skill instructions with a single review-level pass and preserve the
defined stop condition.
---
Minor comments:
In `@CHANGELOG.md`:
- Line 7: Update the changelog header around [Unreleased] to add a Keep a
Changelog version section for release 5.0.0, moving the release changes out of
the unreleased section while preserving the existing changelog entries and
format.
In `@plugins/beagle-analysis/skills/write-plan/references/plan-template.md`:
- Around line 9-11: Replace outdated qualified plugin:skill references with bare
skill names throughout the documentation. Update execution-contract references
in plugins/beagle-analysis/skills/write-plan/references/plan-template.md:9-11
and plugins/beagle-core/skills/subagent-prompt/SKILL.md:48; update
verification-budget in
plugins/beagle-analysis/skills/write-plan/references/planning-disciplines.md:146,
plugins/beagle-core/skills/verify-llm-artifacts/SKILL.md:11, and
plugins/beagle-core/skills/review-llm-artifacts/SKILL.md:295; update the
brainstorming, plan-writing, quick-plan, and review-plan references in
plugins/beagle-core/skills/subagent-prompt/SKILL.md:77,
plugins/beagle-core/skills/review-feedback-schema/SKILL.md:183, and
plugins/beagle-core/skills/review-plan/SKILL.md:65-77. Keep qualified names only
where explicitly required.
In `@plugins/beagle-core/skills/execution-contract/SKILL.md`:
- Around line 57-73: Add language identifiers to every fenced Markdown block in
plugins/beagle-core/skills/execution-contract/SKILL.md lines 57-73 and
plugins/beagle-core/skills/review-plan/SKILL.md lines 95-200: use text for
report and prompt templates, and the appropriate language identifier for each
code example. Update all lens prompt, report-template, suggested-edit, and
stop-and-report fences without changing their contents.
In
`@plugins/beagle-core/skills/llm-artifacts-detection/references/examples/python/abstraction.md`:
- Around line 230-239: Close the unclosed fenced code block in the “Unused
configuration options” example after the Settings class, adding the matching
Markdown fence so the document structure is valid.
In
`@plugins/beagle-core/skills/llm-artifacts-detection/references/examples/python/dead-code.md`:
- Line 141: Update the orphaned-tests directory-tree fenced code block in
dead-code.md to specify the text language by changing its untyped fence to a
text-labeled fence, while preserving the example content.
In `@plugins/beagle-core/skills/llm-artifacts-detection/SKILL.md`:
- Line 45: Add the text language identifier to the opening fences of both fenced
prompt blocks in the llm-artifacts-detection skill documentation, including the
block referenced near the secondary location, while preserving their contents
and closing fences.
In `@plugins/beagle-core/skills/review-plan/SKILL.md`:
- Around line 45-57: Correct the inconsistency in the extraction instructions by
updating the “six signals” wording to “five signals,” matching the five listed
lens-selection signals. Keep the existing signal list and pass-condition
behavior unchanged.
In `@plugins/beagle-core/skills/subagent-prompt/SKILL.md`:
- Line 36: Update the test-command examples in the skill guidance to avoid
unpinned npx execution: replace the npx vitest example with a project-local
invocation or npx --no-install, and specify an explicit version only if
downloading Vitest is intentional. Preserve the existing scoped, single-command
testing guidance.
In `@plugins/beagle-go/skills/go-code-review/SKILL.md`:
- Around line 21-22: Update the pre-report verification guidance in both
plugins/beagle-go/skills/go-code-review/SKILL.md:21-22 and
plugins/beagle-go/skills/go-testing-code-review/SKILL.md:21-22 to apply the full
evidence gate only when the verdict authorizes an irreversible action, not when
the recommended action is irreversible; use “verdict authorizes” consistently
while preserving the existing reversible-finding and one-pass budget rules.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18d6f8be-1b35-4f1c-badd-ebd616ca3876
📒 Files selected for processing (77)
.claude-plugin/marketplace.jsonCHANGELOG.mdCLAUDE.mdplugins/beagle-analysis/skills/brainstorm-beagle/SKILL.mdplugins/beagle-analysis/skills/brainstorm-beagle/references/spec-reviewer.mdplugins/beagle-analysis/skills/brainstorm-beagle/references/spec-template.mdplugins/beagle-analysis/skills/quick-plan/SKILL.mdplugins/beagle-analysis/skills/quick-plan/references/fanout-brief.mdplugins/beagle-analysis/skills/resolve-beagle/SKILL.mdplugins/beagle-analysis/skills/write-plan/SKILL.mdplugins/beagle-analysis/skills/write-plan/references/plan-reviewer.mdplugins/beagle-analysis/skills/write-plan/references/plan-template.mdplugins/beagle-analysis/skills/write-plan/references/planning-disciplines.mdplugins/beagle-core/skills/execution-contract/SKILL.mdplugins/beagle-core/skills/fix-llm-artifacts/SKILL.mdplugins/beagle-core/skills/llm-artifacts-detection/SKILL.mdplugins/beagle-core/skills/llm-artifacts-detection/references/abstraction-criteria.mdplugins/beagle-core/skills/llm-artifacts-detection/references/dead-code-criteria.mdplugins/beagle-core/skills/llm-artifacts-detection/references/examples/python/abstraction.mdplugins/beagle-core/skills/llm-artifacts-detection/references/examples/python/dead-code.mdplugins/beagle-core/skills/llm-artifacts-detection/references/examples/python/style.mdplugins/beagle-core/skills/llm-artifacts-detection/references/examples/python/tests.mdplugins/beagle-core/skills/llm-artifacts-detection/references/style-criteria.mdplugins/beagle-core/skills/llm-artifacts-detection/references/tests-criteria.mdplugins/beagle-core/skills/review-feedback-schema/SKILL.mdplugins/beagle-core/skills/review-llm-artifacts/SKILL.mdplugins/beagle-core/skills/review-plan/SKILL.mdplugins/beagle-core/skills/review-skill/SKILL.mdplugins/beagle-core/skills/review-structure/SKILL.mdplugins/beagle-core/skills/review-verification-protocol/SKILL.mdplugins/beagle-core/skills/subagent-prompt/SKILL.mdplugins/beagle-core/skills/verification-budget/SKILL.mdplugins/beagle-core/skills/verify-llm-artifacts/SKILL.mdplugins/beagle-core/skills/verify-llm-artifacts/references/verification-checklist.mdplugins/beagle-elixir/skills/elixir-code-review/SKILL.mdplugins/beagle-elixir/skills/elixir-docs-review/SKILL.mdplugins/beagle-elixir/skills/elixir-performance-review/SKILL.mdplugins/beagle-elixir/skills/elixir-security-review/SKILL.mdplugins/beagle-elixir/skills/exunit-code-review/SKILL.mdplugins/beagle-elixir/skills/liveview-code-review/SKILL.mdplugins/beagle-elixir/skills/phoenix-code-review/SKILL.mdplugins/beagle-elixir/skills/review-elixir/SKILL.mdplugins/beagle-elixir/skills/review-verification-protocol/SKILL.mdplugins/beagle-go/skills/bubbletea-code-review/SKILL.mdplugins/beagle-go/skills/go-code-review/SKILL.mdplugins/beagle-go/skills/go-testing-code-review/SKILL.mdplugins/beagle-go/skills/review-go/SKILL.mdplugins/beagle-go/skills/review-tui/SKILL.mdplugins/beagle-go/skills/review-verification-protocol/SKILL.mdplugins/beagle-ios/skills/app-intents-code-review/SKILL.mdplugins/beagle-ios/skills/cloudkit-code-review/SKILL.mdplugins/beagle-ios/skills/review-ios/SKILL.mdplugins/beagle-ios/skills/review-verification-protocol/SKILL.mdplugins/beagle-ios/skills/swift-code-review/SKILL.mdplugins/beagle-ios/skills/swift-testing-code-review/SKILL.mdplugins/beagle-ios/skills/watchos-code-review/SKILL.mdplugins/beagle-ios/skills/widgetkit-code-review/SKILL.mdplugins/beagle-python/skills/fastapi-code-review/SKILL.mdplugins/beagle-python/skills/python-code-review/SKILL.mdplugins/beagle-python/skills/review-python/SKILL.mdplugins/beagle-python/skills/review-verification-protocol/SKILL.mdplugins/beagle-python/skills/sqlalchemy-code-review/SKILL.mdplugins/beagle-react/skills/react-router-code-review/SKILL.mdplugins/beagle-react/skills/review-frontend/SKILL.mdplugins/beagle-react/skills/review-remix-v2/SKILL.mdplugins/beagle-react/skills/review-verification-protocol/SKILL.mdplugins/beagle-react/skills/shadcn-code-review/SKILL.mdplugins/beagle-rust/skills/axum-code-review/SKILL.mdplugins/beagle-rust/skills/ffi-code-review/SKILL.mdplugins/beagle-rust/skills/macros-code-review/SKILL.mdplugins/beagle-rust/skills/review-rust/SKILL.mdplugins/beagle-rust/skills/review-verification-protocol/SKILL.mdplugins/beagle-rust/skills/rust-code-review/SKILL.mdplugins/beagle-rust/skills/rust-testing-code-review/SKILL.mdplugins/beagle-rust/skills/serde-code-review/SKILL.mdplugins/beagle-rust/skills/sqlx-code-review/SKILL.mdplugins/beagle-rust/skills/tokio-async-code-review/SKILL.md
Comment on lines
+81
to
+92
| Run the structural check instead — a scripted integrity test, not an echo: | ||
|
|
||
| ```bash | ||
| python3 - <<'PY' | ||
| import json | ||
| r = json.load(open('.beagle/llm-artifacts-review.json')) | ||
| f = r['findings'] | ||
| if not isinstance(f, list) or not f: | ||
| raise SystemExit("No findings to lock; aborting.") | ||
| ids = [x.get('id') for x in f] | ||
| if any(not isinstance(i, int) for i in ids): | ||
| raise SystemExit("All finding ids must be integers; aborting.") | ||
| if len(set(ids)) != len(ids): | ||
| raise SystemExit("Duplicate finding ids detected; aborting.") | ||
| print("| id | category | file:line | description |") | ||
| print("|----|----------|-----------|-------------|") | ||
| for x in f: | ||
| desc = (x.get('description') or '').replace('|', '\\|')[:80] | ||
| print(f"| {x['id']} | {x.get('category')} | {x.get('file')}:{x.get('line')} | {desc} |") | ||
| print("Locked ids: {" + ", ".join(str(i) for i in sorted(set(ids))) + "}") | ||
| PY | ||
| jq -e '.findings | ||
| | (length > 0) | ||
| and (map(.id) | all(type == "number")) | ||
| and ((map(.id) | unique | length) == length)' \ | ||
| .beagle/llm-artifacts-review.json \ | ||
| || echo "ABORT: findings[] is empty, has non-integer ids, or has duplicate ids." | ||
| ``` | ||
|
|
||
| Adjudicate and fix only the ids above. If your sense of what to fix differs from this table, the table wins. | ||
| Non-zero exit is an **abort**, not a warning — duplicate or non-integer ids break the 1:1 mapping the verification overlay depends on. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Make the findings integrity check fail closed.
jq ... || echo swallows the failure because echo succeeds, so malformed findings continue despite Line 92 saying this is an abort. Additionally, type == "number" accepts decimals and the check does not enforce contiguous 1..N IDs or summary-count invariants required by the upstream contract.
Use an explicit if ! jq ...; then exit 1; fi and validate integer, contiguous IDs plus summary totals.
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 212: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 217: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[error] 36: [P6] Direct Prompt Extraction: Skill contains instructions that could directly expose system prompts, internal rules, or hidden instructions to users or external parties.
Remediation: Remove any instructions that reveal, print, or output system prompts or internal rules. System instructions should never be exposed to end users.
(System Prompt Leakage (P6))
🤖 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/beagle-core/skills/fix-llm-artifacts/SKILL.md` around lines 81 - 92,
Update the findings integrity check in the documented jq command to fail closed:
wrap it in an explicit if ! jq condition and exit 1 on validation failure
instead of using || echo. Validate that findings is non-empty, IDs are integers
forming a contiguous 1..N sequence with no duplicates, and enforce the upstream
summary-count invariants alongside the existing type checks.
Comment on lines
+9
to
+26
| **What to look for:** functions, types, and variables with no references anywhere in the codebase. | ||
|
|
||
| ```python | ||
| # TODO: implement caching <-- Incomplete feature | ||
| def get_user(id): | ||
| return db.query(User).get(id) | ||
| | Sub-pattern | Signal | | ||
| |---|---| | ||
| | Unused function | Defined, exported or not, with zero call sites | | ||
| | Unused type | Class/struct/interface/enum never constructed or referenced in a signature | | ||
| | Unused module-level binding | Constant or global assigned and never read | | ||
| | Unused local | Assigned inside a function, then never used before return | | ||
| | Unreachable code | Statements after an unconditional return, throw, panic, or exit | | ||
|
|
||
| # FIXME: this breaks with unicode <-- Known bug | ||
| def parse_name(name): | ||
| return name.split()[0] | ||
| **How to find:** | ||
|
|
||
| # HACK: temporary workaround for issue #123 <-- Tech debt | ||
| result = data.replace("\x00", "") | ||
| 1. "Find usages" / go-to-references in an editor or language server. | ||
| 2. The language's own dead-code tooling — an unused-symbol linter, an unused-import check, or the compiler's own dead-code warnings where the language has them. | ||
| 3. Direct search for the symbol name across the repo. | ||
| 4. Unused-import detection, which is usually the cheapest true positive in this category. | ||
|
|
||
| # XXX: this needs to be refactored <-- Acknowledged mess | ||
| def complex_function(): | ||
| # 200 lines of spaghetti | ||
| pass | ||
| **Before flagging:** a symbol reachable only through reflection, dynamic dispatch by name, serialization, DI registration, or a framework's string-keyed routing is **not** dead. Search for the name as a string literal, not just as an identifier. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Dead-code detection must preserve framework-invoked contracts. Both the reusable criteria and the detection prompt can classify live callbacks or lifecycle hooks as dead when they have no ordinary call sites.
plugins/beagle-core/skills/llm-artifacts-detection/references/dead-code-criteria.md#L9-L26: add framework callbacks, lifecycle hooks, and protocol/interface members to the non-dead exceptions and search guidance.plugins/beagle-core/skills/llm-artifacts-detection/SKILL.md#L73-L79: apply the same framework-invocation check before emitting an unused-code finding.
📍 Affects 2 files
plugins/beagle-core/skills/llm-artifacts-detection/references/dead-code-criteria.md#L9-L26(this comment)plugins/beagle-core/skills/llm-artifacts-detection/SKILL.md#L73-L79
🤖 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/beagle-core/skills/llm-artifacts-detection/references/dead-code-criteria.md`
around lines 9 - 26, Update the dead-code criteria in
plugins/beagle-core/skills/llm-artifacts-detection/references/dead-code-criteria.md,
covering the unused-symbol patterns and “Before flagging” guidance, to exempt
framework callbacks, lifecycle hooks, and protocol/interface members; include
checks for framework registration, dynamic dispatch, and string-based invocation
alongside ordinary references. Update the unused-code detection guidance in
plugins/beagle-core/skills/llm-artifacts-detection/SKILL.md at lines 73-79 to
perform the same framework-invocation check before emitting findings.
Comment on lines
+150
to
+152
| Reporting a finding is `beagle-core:verification-budget` tier REVERSIBLE. This is an enumerated check list run **once per review**, not a gate applied per finding: record the outcome of each item across the finding list and proceed. | ||
|
|
||
| **Budget:** max **1** pass. Stop when each of the six items below has a recorded outcome. Tie-break: ship an unresolved item's finding as a question, or drop it — do not open another pass. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not drop unresolved findings in the verification tie-break.
The canonical budget requires “proceed and flag.” Replace “or drop it” with a deterministic status such as INCONCLUSIVE and preserve the finding in the output, otherwise a failed verification pass can silently lose review coverage.
🧰 Tools
🪛 SkillSpector (2.3.11)
[error] 189: [RA1] Self-Modification: Skill modifies its own code, configuration, or behavior at runtime. Self-modification enables an agent to escalate privileges, disable safety constraints, or install persistent backdoors.
Remediation: Prevent the skill from modifying its own code, SKILL.md, or configuration files. Treat skill files as read-only at runtime.
(Rogue Agent (RA1))
🤖 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/beagle-core/skills/review-feedback-schema/SKILL.md` around lines 150
- 152, Update the verification-budget guidance in the review feedback schema so
unresolved items are never dropped: replace the “or drop it” tie-break with a
deterministic status such as INCONCLUSIVE, and require the unresolved finding to
remain in the output while preserving the one-pass budget.
Comment on lines
+257
to
+269
| 1. **Apply fixes now** — edit the plan file to address issues | ||
| 2. **Save & fix later** — open a new session to apply fixes | ||
| 3. **Proceed anyway** — execute the plan despite issues (not recommended with Critical open) | ||
|
|
||
| Which option? | ||
| ``` | ||
|
|
||
| ## Rules | ||
|
|
||
| - Satisfy Hard gates 1–2 before Step 3; Hard gate 3 before Step 4; Hard gate 4 before the Step 5 options prompt | ||
| - Load skills BEFORE running the review lenses (Hard gate 2) | ||
| - Run all 5 review lenses — in parallel via subagents when the agent supports them, otherwise sequentially | ||
| - Reference Task:Step for each issue | ||
| - Provide copyable suggested edits for Critical/Major issues | ||
| - Save review before prompting user (Hard gate 4) | ||
| - Never auto-execute plan; require user choice | ||
| - Number issues sequentially (1, 2, 3...) | ||
| - Reference issues by Task:Step; number them sequentially (1, 2, 3…). | ||
| - Provide copyable suggested edits for Critical and Major issues. | ||
| - Never edit the plan and never auto-execute it — the user chooses. | ||
| - A skipped lens is a recorded outcome, not a gap. Do not run a lens "just in case." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the option that contradicts the non-editing contract.
This skill states that it never edits the plan, but “Apply fixes now — edit the plan file” instructs the user to do exactly that. Replace it with a handoff to a separate editing session, or explicitly scope the option outside this skill.
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 268: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
🤖 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/beagle-core/skills/review-plan/SKILL.md` around lines 257 - 269,
Update the choice list in the review-plan skill to remove the “Apply fixes now”
option that permits editing the plan; replace it with a handoff to a separate
editing session or explicitly state that editing occurs outside this skill.
Preserve the existing “Save & fix later” and “Proceed anyway” behavior, and keep
the non-editing contract consistent throughout.
Comment on lines
+25
to
+34
| Apply **once per finding** before it may appear in the review. If a gate fails, **omit** the finding, **downgrade** to Informational (per [Severity Calibration](#severity-calibration)), or **rephrase** as a question — do not ship soft accusations. | ||
|
|
||
| | Step | What you do | Pass condition (objective) | | ||
| |------|----------------|----------------------------| | ||
| | **1. Anchor** | Read the full enclosing symbol or module, not only the diff hunk. | You can state **file path** and **line range** (or symbol name + file) you are judging. | | ||
| | **2. Evidence** | For this finding’s type, run the checks in [Verification by Issue Type](#verification-by-issue-type). | Each required check has an **artifact**: pasted tool output, **file:line** citation, or explicit **"none"** / **"N matches"** after a repo search—not a claim you "looked." | | ||
| | **2. Evidence** | For this finding's type, run the checks in [Verification by Issue Type](#verification-by-issue-type). | Each required check has an **artifact**: pasted tool output, **file:line** citation, or a **"N matches"** count naming the patterns searched — not a claim you "looked." | | ||
| | **3. Severity** | Assign severity using [Severity Calibration](#severity-calibration). | Label matches the table; requests for net-new code that did not exist in scope are **Informational** only. | | ||
| | **4. Format** | Draft the finding for the report. | Matches `[FILE:LINE] ISSUE_TITLE`; Informational items do not add to the actionable count. | | ||
|
|
||
| Style-only or preference items must fail gate 2 or map to **Do NOT Flag At All**—they do not get a severity. | ||
| **Loop budget:** max **1** pass per finding. Stop when all four gate outputs are recorded for that finding. Tie-break: drop or downgrade the finding and proceed — never re-run the gates hoping for a cleaner answer. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Run the gates once per review, not once per finding.
“Once per finding” contradicts the PR objective of consolidated verification and the later instruction at Line 135 to perform exactly one pass over the assembled finding list. This wording reintroduces repeated verification and breaks the shared bounded-review contract. Change the gate scope to one pass over the complete finding list.
🤖 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/beagle-core/skills/review-verification-protocol/SKILL.md` around
lines 25 - 34, The gate procedure in the review-verification protocol currently
requires a pass once per finding; update the scope to perform exactly one pass
over the complete assembled finding list, consistent with the later single-pass
instruction. Adjust the opening directive and loop-budget wording to describe
shared review-level verification while preserving the existing gate outputs and
stop behavior.
Comment on lines
+51
to
+53
| Load `beagle-core:review-verification-protocol` plus the [Swift / iOS delta](../review-verification-protocol/SKILL.md) **once, at review entry** — not once per finding. Reporting a finding is `beagle-core:verification-budget` tier REVERSIBLE: cite the evidence you already have and move on. Only a verdict that authorizes an irreversible action (deleting code, rewriting a file) earns that protocol's full evidence gate. | ||
|
|
||
| The three gates below are per-finding and cheap. Apply them in order. Budget: max **1** pass per finding; stop when each gate has a recorded artifact; tie-break: drop the finding or downgrade it to an open question and proceed — never re-run the gates hoping for a cleaner answer. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
[plugins/beagle-ios/skills/watchos-code-review/SKILL.md:51-53] Apply the gates once per review, not once per finding
The text first requires one review-level protocol load, then explicitly makes the three gates per-finding. That preserves the old verification behavior and contradicts the bounded once-per-review contract.
Change the gates to one pass over the assembled finding list, with a single review-level stop condition.
🤖 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/beagle-ios/skills/watchos-code-review/SKILL.md` around lines 51 - 53,
The review-verification gates are incorrectly specified as per-finding,
conflicting with the once-per-review contract. Update the gate instructions near
the review entry protocol to run one pass over the assembled finding list, with
a single review-level stop condition, while retaining the existing one-time
protocol load and evidence-budget behavior.
Comment on lines
+49
to
+51
| Load `beagle-core:review-verification-protocol` plus the [Swift / iOS delta](../review-verification-protocol/SKILL.md) **once, at review entry** — not once per finding. Reporting a finding is `beagle-core:verification-budget` tier REVERSIBLE: cite the evidence you already have and move on. Only a verdict that authorizes an irreversible action (deleting code, rewriting a file) earns that protocol's full evidence gate. | ||
|
|
||
| The three gates below are per-finding and cheap. Apply them in order. Budget: max **1** pass per finding; stop when each gate has a recorded artifact; tie-break: drop the finding or downgrade it to an open question and proceed — never re-run the gates hoping for a cleaner answer. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
[plugins/beagle-ios/skills/widgetkit-code-review/SKILL.md:49-51] Remove the per-finding verification loop
These lines contradict the PR’s once-per-review verification contract by requiring a separate pass for every finding. Apply the three gates once to the assembled finding list and stop when all invoked gate artifacts are recorded.
🤖 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/beagle-ios/skills/widgetkit-code-review/SKILL.md` around lines 49 -
51, Update the review-verification instructions around the three gates to apply
them once to the assembled finding list rather than once per finding. Remove the
per-finding pass and budget language, while preserving the once-at-review-entry
protocol loading and the requirement to stop after all invoked gate artifacts
are recorded.
|
|
||
| 1. Re-read the actual code (not just diff context) | ||
| 2. For "unused" claims - did you search all references? | ||
| 2. For "unused" claims - did you run the core protocol's four enumerated reference patterns (direct reference, re-export/barrel entry, string-literal or JSX-only reference, framework-invoked contract) and report the count for each? Report "no matches across the 4 enumerated patterns", never "unused anywhere" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not replace the core dynamic-reference check with JSX-only wording.
Both frontend skills omit reflection, decorators, selectors, registries, and configuration references from the core unused-reference verification. Preserve the core four-pattern definition, then add JSX and Remix route-file conventions as frontend-specific extensions:
plugins/beagle-react/skills/review-frontend/SKILL.md#L92-L92: retain “string-literal or dynamic reference” and add JSX-only references.plugins/beagle-react/skills/review-remix-v2/SKILL.md#L114-L114: retain dynamic references and add JSX plus Remix route-file conventions.
📍 Affects 2 files
plugins/beagle-react/skills/review-frontend/SKILL.md#L92-L92(this comment)plugins/beagle-react/skills/review-remix-v2/SKILL.md#L114-L114
🤖 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/beagle-react/skills/review-frontend/SKILL.md` at line 92, Preserve
the core four-pattern unused-reference check in both frontend skills, including
string-literal or dynamic references, rather than replacing it with JSX-only
wording. In plugins/beagle-react/skills/review-frontend/SKILL.md at lines 92-92,
retain dynamic-reference coverage and add JSX-only references; in
plugins/beagle-react/skills/review-remix-v2/SKILL.md at lines 114-114, retain
dynamic-reference coverage and add JSX and Remix route-file conventions. Ensure
reports count each core pattern and say “no matches across the 4 enumerated
patterns” when applicable.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/beagle-core/skills/verification-budget/SKILL.md (1)
17-20: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not let REVERSIBLE bypass data-loss and staleness safeguards.
The Git-checkout criterion is unsafe for dirty worktrees: a mutation can overwrite pre-existing uncommitted changes that checkout cannot recover. Likewise, downstream stages may apply a stale “reversible” patch to the wrong code because only IRREVERSIBLE actions re-read their targets. Classify worktree mutations conservatively, or require a pre-mutation snapshot and immediate target re-read for every mutation.
Also applies to: 48-54
🤖 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/beagle-core/skills/verification-budget/SKILL.md` around lines 17 - 20, Update the REVERSIBLE classification and its safeguards in the verification-budget guidance so worktree mutations cannot bypass dirty-worktree data-loss protection or stale-target checks. Treat mutations conservatively, or require a pre-mutation snapshot and immediate re-read of the target before every mutation; apply the same requirements to the related guidance at the referenced section.
🤖 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.
Outside diff comments:
In `@plugins/beagle-core/skills/verification-budget/SKILL.md`:
- Around line 17-20: Update the REVERSIBLE classification and its safeguards in
the verification-budget guidance so worktree mutations cannot bypass
dirty-worktree data-loss protection or stale-target checks. Treat mutations
conservatively, or require a pre-mutation snapshot and immediate re-read of the
target before every mutation; apply the same requirements to the related
guidance at the referenced section.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 433d93ff-5cdc-4aac-9852-c75c13ce3f94
📒 Files selected for processing (6)
.entire/settings.json.gitignoreplugins/beagle-analysis/skills/write-plan/SKILL.mdplugins/beagle-analysis/skills/write-plan/references/plan-reviewer.mdplugins/beagle-analysis/skills/write-plan/references/planning-disciplines.mdplugins/beagle-core/skills/verification-budget/SKILL.md
💤 Files with no reviewable changes (1)
- .entire/settings.json
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/beagle-analysis/skills/write-plan/references/plan-reviewer.md
- plugins/beagle-analysis/skills/write-plan/references/planning-disciplines.md
BREAKING CHANGE: the six language-specific review-verification-protocol skills are renamed to <language>-verification-protocol. The beagle-core skill keeps the review-verification-protocol name. The Phase 4 refactor split one duplicated protocol into a language-neutral core plus six thin per-language deltas, but left all seven sharing the name review-verification-protocol. Plugin-scoped installs disambiguate by plugin, so this was invisible there. Flat installs -- ~/.agents/skills/<name>/ and ~/.codex/skills/<name>/, which the skills CLI and Codex both use -- key solely on skill name, so six of the seven were silently dropped and every language-specific false-positive pattern became unreachable. Before the refactor each plugin shipped a self-contained copy, so this was a regression the refactor introduced. Renames: python-, go-, rust-, elixir-, react-, ios-verification-protocol. Each delta's frontmatter name and description now identify it as that language's delta extending the shared core. All 40+ intra-plugin relative links retargeted; every beagle-core:review-verification-protocol reference left intact, since those genuinely mean the core. Plugin READMEs and the root SKILLS.md catalog updated. No review criteria or verification content changed -- this is a rename plus reference update. Verified: every skill name in the marketplace is now globally unique, every frontmatter name matches its directory, and no relative markdown link is broken. Co-Authored-By: Claude Opus 4.8 <[email protected]> Entire-Checkpoint: 999ca68cca76
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
These skills were written when one expensive model did planning and execution alike. The workflow now uses planning-tier models to plan and review while execution-tier models execute, and the old framing was burning tokens by over-verifying and getting stuck. This bounds every verification loop, gives executors a legal way to stop, and removes ceremony that defended against a failure mode the current topology already handles.
The headline is not line count — it is that every verification loop now has a cap and every executor has a legal way to stop.
Changes
Added
beagle-core:execution-contract(internal, 93 lines) — capability-role vocabulary (planning vs. execution model, defined by decision authority, never by model name), failure matching by shape rather than the planner's guessed string, a baseline-diffed pre-existing-red policy, a two-attempt-per-step budget, and a stop-and-report template. States explicitly that halting with a report is a success state. Inlined verbatim into every emitted plan.beagle-core:verification-budget(internal, 75 lines) — reversibility-based risk tiers, the required loop-budget syntax (max passes, an observable stop condition, a proceed-and-flag tie-break), the one-echo rule, and a prove-a-negative ban. Review and verify skills import this instead of restating their own gates.Changed
"Advance only when every item is honestly yes"— 17 rows inwrite-plan, 12 inquick-plan, a double layer inbrainstorm-beagle, three uncapped restart loops inverify-llm-artifacts— is gone everywhere. Criteria were preserved; only the delivery mechanism changed.## Executor Contract. Removedsubagent-prompt's"no 'use your judgment' escape hatches"and"do not stop until the final integration check passes", which instructed models to grind."completed for every reported issue","Run once per FastAPI-related finding"). Reporting a finding is tier REVERSIBLE and earns no ceremony; only IRREVERSIBLE-action findings keep the full evidence gate.write-plan462 → 181 lines.review-verification-protocol1,848 → 926 across seven copies (language-neutral core + six thin deltas, every language-specific pattern kept). Detection criteria 1,046 → 298, with per-language examples now loading only on a language match.verify-llm-artifactscollapses five hard gates into one tier-based gate and drops ~1,300 lines/run of cross-skill preloading.fix-llm-artifactskeeps every protective control — stash gate, per-fix code-presence check,git_headstaleness abort, risky-fix prompts, post-fix lint and tests.Fixed
spec-template.mdandresolve-beagleboth pointed atdocs/specs/whilebrainstorm-beaglewrites to.beagle/concepts/<slug>/spec.md, soresolve-beaglecould not find the specs it exists to resolve.quick-planfrontmatter description was 1,033 characters against a 1,024 limit.[WORKSPACE/...]placeholder infanout-brief.mdsent subagent reports to a path no skill established.Anti-confabulation (gate 0)reference left by the protocol refactor, plusreview-structure's local restatement of that gate.Motivation
Agents using these skills spent disproportionate time over-verifying and getting stuck. Five root causes: unbounded loops with no stop condition; emitted plans with no legal halt state; per-verdict anti-confabulation echoes that defended against one long-lived context adjudicating code it never read (a much weaker risk with fresh, narrow subagent contexts); no capability tiering anywhere; and duplication that multiplied every instruction token.
Testing
This repo has no build system, no tests, and no linter — it is a pure markdown plugin marketplace where validation is manual inspection (see
CLAUDE.md). Stating that plainly rather than checking boxes that do not apply.Verification performed
Scripted checks across every changed file:
name:frontmatter value byte-identical toHEADSKILL.mdover 500 linesid, category, file:line, risk,fix_safety,fix_action→confirmed_issue/false_positive/inconclusive, confidence,checks_performed)"honestly yes"/"advance only when"gate phrasing outside the file that documents it as an anti-patternHandoffs confirmed intact
.beagle/concepts/<slug>/spec.md→write-plan→.beagle/concepts/<slug>/plan.md;quick-plan→.beagle/plans/<slug>/plan.md(sibling tree); both →subagent-prompt; the spec's eight sections, slug rules,needs-spike-before-planningmarker, and theresolve-beagleOpen Questions hook.Breaking Changes
Version bumped to 5.0.0.
plan.mdgains a mandatory## Executor Contractsection. Plans generated before this change carry no attempt budget and no pre-existing-red policy. They still execute; they just lack the stop rules.review-verification-protocolno longer has anAnti-confabulation (gate 0)section — replaced byverification-budget's one-echo rule. External references to that heading need repointing.spec.mdstructure is unchanged.Reviewer notes
Worth a careful look at the tradeoff: risk-tiering verification will let some false positives through, and this PR verifies structure, not review outcomes. The counterargument is that a review nobody can afford to run has precision zero, so ceremony is kept on destructive paths and stripped where reversible. I'd suggest an A/B on a known-artifact repo before tagging 5.0.0 — that has not been done.
Note also that
.beagle/is gitignored, so the plan document with the full execution record is not in this diff.Checklist