docs(code-review): tighten weak language and add trigger phrase

[ralphbean]

Ran skillsaw (evaluating it per #188) against the repo's skills.
Adds a "Use when..." trigger phrase to the description, and rewrites
two negative-only questions ("does the caller degrade gracefully or
silently proceed?", "is auth correctly enforced on all code paths?")
as positive, checkable prompts.

Related to #188

Assisted-by: Claude Opus 4.6 noreply@anthropic.com
Signed-off-by: Ralph Bean rbean@redhat.com

---

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[qodo-code-review]

PR Summary by Qodo

Docs: strengthen code-review skill wording and add “Use when…” trigger

📝 Documentation 🕐 Less than 5 minutes

AI Description

• Add a clear “Use when…” trigger phrase to the code-review skill description.
• Rewrite two negatively framed checklist prompts into positive, checkable questions.
• Clarify failure-path expectations and auth enforcement coverage (including edge cases).

High-Level Assessment

The approach is appropriate for this PR’s goal: small copy edits that improve clarity and auditability of review prompts. Alternatives (larger restructuring of the skill or adding new dimensions) would be out of scope for the stated intent.

Files changed (1) +6 / -4

Documentation (1) +6 / -4

SKILL.mdClarify usage trigger and tighten review/security prompts +6/-4

Clarify usage trigger and tighten review/security prompts

• Updates the skill’s top-level description to include an explicit “Use when…” trigger phrase. Rewords two checklist items to be more directly testable: failure-path behavior is framed as explicit error/fallback vs silent continuation, and authentication is framed as enforced on every path including error/edge branches.

skills/code-review/SKILL.md

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:00 PM UTC · Completed 5:13 PM UTC

Commit: 4aa6856 · View workflow run →

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (1)

1. Protected path skills/ modified 📜 Skill insight § Compliance

Description

This PR modifies a protected governance/infrastructure path under skills/, so it must not be
auto-approved and requires explicit human review per the checklist.

Code

skills/code-review/SKILL.md[R6-7]

+  dimensions, and compiles structured findings. Use when reviewing a
+  code change directly, or when delegated to by the pr-review skill.

Relevance

●●● Strong

Repo enforces protected-path review controls; changes under protected paths are treated as requiring
explicit human review.

PR-#569

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The checklist flags any modifications under protected paths (including skills/) as requiring a
finding and human approval; this PR changes skills/code-review/SKILL.md within that protected
directory.

skills/code-review/SKILL.md[1-8]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR modifies a protected governance/infrastructure path (`skills/`), which must not be auto-approved and requires explicit human review.

## Issue Context
Protected-path changes require heightened scrutiny even when the change is documentation-only.

## Fix Focus Areas
- skills/code-review/SKILL.md[6-7]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context

✅ Compliance rules (platform): 55 rules

✅ Skills: 4 invoked

  code-review

  code-implementation

  pr-review

  docs-review

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] skills/code-review/SKILL.md — This PR modifies a file under the protected path skills/. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale (applying skillsaw findings to tighten language). Human approval is always required for protected-path changes, regardless of context.

Low

• [stale-wording] skills/pr-review/sub-agents/correctness.md:38 — The 'Runtime mechanism checklist' uses failure-handling wording ('does the caller handle it or silently proceed as if it succeeded?') that differs from the tightened wording in SKILL.md ('does the caller return an error or fall back to a default, or does it silently proceed as if nothing failed?'). The PR's more specific phrasing widens the divergence between the skill definition and this sub-agent instruction file.

---

Labels: PR modifies the code-review skill definition, which is part of the review agent system.

Previous run

Review

Findings

Medium

• [protected-path] skills/code-review/SKILL.md — This PR modifies a file under the protected path skills/. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale (applying skillsaw findings to tighten language). Human approval is always required for protected-path changes, regardless of context.

Low

• [stale-wording] skills/pr-review/sub-agents/correctness.md:38 — The 'Runtime mechanism checklist' uses failure-handling wording ('does the caller handle it or silently proceed as if it succeeded?') that differs from the tightened wording in SKILL.md ('does the caller return an error or fall back to a default, or does it silently proceed as if nothing failed?'). The PR's more specific phrasing widens the divergence between the skill definition and this sub-agent instruction file.

Previous run (2)

Review

Findings

Medium

• [logic-error] skills/code-review/SKILL.md:4 — The updated description states "with no PR in context," but the body still references PR-based invocation: line 26 says "If invoked by another skill (e.g., pr-review), use the diff and context that skill provides," and the severity anchoring section references "prior review context (passed from the pr-review skill)." The description now scopes the skill away from PR contexts while the body still accommodates them, which could cause an LLM skill router to skip code-review in a PR-adjacent context where it should be used.
  Remediation: Either (a) remove the "with no PR in context" qualifier from the description, or (b) also update the body to remove pr-review references, aligning it with the new scoping.

• [protected-path] skills/code-review/SKILL.md — This PR modifies a file under the protected path skills/. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale (applying skillsaw findings to tighten language). Human approval is always required for protected-path changes, regardless of context.

Low

• [stale-wording] skills/pr-review/sub-agents/correctness.md:38 — The 'Runtime mechanism checklist' uses failure-handling wording ('does the caller handle it or silently proceed as if it succeeded?') that differs from the tightened wording in SKILL.md ('does the caller return an error or fall back to a default, or does it silently proceed as if nothing failed?'). This divergence predates this PR but the gap widens with the new, more specific phrasing. Consider aligning in a follow-up.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (3)

Review

Findings

Medium

• [protected-path] skills/code-review/SKILL.md — This PR modifies a file under the protected path skills/. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale (applying skillsaw findings to tighten language). Human approval is always required for protected-path changes, regardless of context.

Low

• [stale-wording] skills/pr-review/sub-agents/correctness.md:38 — The 'Runtime mechanism checklist' section uses older failure-handling wording ('does the caller handle it or silently proceed as if it succeeded?') while this PR tightens the same guidance in SKILL.md to be more explicit ('does the caller return an error or fall back to a default, or does it silently proceed as if nothing failed?'). Consider aligning the sub-agent's wording in a follow-up.

Previous run (4)

Review

Findings

Medium

• [protected-path] skills/code-review/SKILL.md — This PR modifies a file under the protected path skills/. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale (applying skillsaw findings to improve documentation language). Human approval is always required for protected-path changes, regardless of context.

Previous run (5)

Review

Findings

Medium

• [protected-path] skills/code-review/SKILL.md — This PR modifies a file under the protected path skills/. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale (applying skillsaw findings to tighten language). Human approval is always required for protected-path changes, regardless of context.

Low

• [inconsistent-documentation-pattern] skills/code-review/SKILL.md:6 — The "Use when" trigger phrase follows a pattern used by 5 of 10 skills in the repo, while the other 5 use a different style. The pattern aligns with AGENTS.md §7 guidance that descriptions should explain "when and how to use the skill," but creates a stylistic split across the skill library.
• [awkward-parallel-structure] skills/code-review/SKILL.md:6 — The phrase "Use when reviewing a code change directly, or when delegated to by the pr-review skill" mixes active voice ("reviewing") with passive voice ("delegated to"). Consider separating into distinct sentences for cleaner parallel structure.
• [stale-doc] skills/pr-review/sub-agents/correctness.md:38 — The failure path check uses the older formulation "does the caller handle it or silently proceed as if it succeeded?" while this PR improves the same guidance in SKILL.md to be more explicit. Consider updating the correctness sub-agent's language in a follow-up.

---

Labels: PR is a documentation-only change to a skill definition file

[ralphbean]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 9:30 PM UTC · Completed 9:36 PM UTC

Commit: 4aa6856 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Fixed 1 of 4 review findings. Rewrote the skill description to fix mixed active/passive voice (awkward-parallel-structure). Disagreed with 3: protected-path is informational (requires human approval, not a code fix), inconsistent-documentation-pattern is an observation about other skills (out of scope), and stale-doc was explicitly suggested as a follow-up by the reviewer.

Fixed (1):

1. awkward-parallel-structure in description: Rewrote 'Use when reviewing a code change directly, or when delegated to by the pr-review skill' to 'Use when reviewing a code change, whether invoked directly or by the pr-review skill' — eliminates mixed active/passive voice and creates cleaner parallel structure.

Disagreed (3):

1. protected-path: skills/code-review/SKILL.md under protected path: This is an informational finding noting that human approval is required for protected-path changes. It is not actionable by the fix agent — the PR already has the requires-manual-review label and will require human approval.
2. inconsistent-documentation-pattern: 'Use when' trigger phrase creates stylistic split: The reviewer notes this pattern aligns with AGENTS.md §7 guidance that descriptions should explain 'when and how to use the skill.' Standardizing other skills' descriptions is out of scope for this PR.
3. stale-doc: correctness sub-agent uses older failure-path formulation: The reviewer explicitly suggested updating the correctness sub-agent's language 'in a follow-up.' This is out of scope for this PR, which targets code-review SKILL.md only.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 9:37 PM UTC · Ended 9:38 PM UTC

Commit: 859290b · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:39 PM UTC · Completed 9:51 PM UTC

Commit: 7acaba0 · View workflow run →

[ralphbean]

/fs-fix fix the trigger phrase placement. put the "use when .." at the start of the description.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 12:52 AM UTC · Completed 12:56 AM UTC

Commit: 7acaba0 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (human-triggered)

Moved the 'Use when...' trigger phrase from the end to the start of the code-review skill description, per human instruction.

Fixed (1):

1. trigger phrase placement: move 'Use when...' to start of description: Reordered the skill description so the 'Use when reviewing a code change...' trigger phrase appears at the start, followed by the procedural summary. Adjusted wording ('Standalone procedure that identifies...' instead of 'Identifies the...') for natural flow.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:57 AM UTC · Completed 1:11 AM UTC

Commit: 4497e46 · View workflow run →

[waynesun09]

/fs-fix

The trigger phrase still contains a factually inaccurate delegation claim, unaddressed by the two prior fix commits (which only reworded voice/ordering, not the substance):

skills/code-review/SKILL.md frontmatter description currently reads:

Use when reviewing a code change, whether invoked directly or by the pr-review skill. Standalone procedure that identifies the change, reads surrounding source, evaluates across six review dimensions, and compiles structured findings.

The clause "or by the pr-review skill" is false. Verified against the repo at PR head:

• None of the files under skills/pr-review/sub-agents/ (challenger, correctness, cross-repo-contracts, docs-currency, intent-coherence, security-triage, security, style-conventions) reference or invoke the code-review skill.
• agents/review.md's "Skill routing" section confirms code-review and pr-review are alternates selected by invocation context, not a delegation chain: "When invoked via --print for pre-push review, use code-review. When invoked for a GitHub PR, use pr-review."

Fix: remove the delegation claim and describe code-review's actual trigger condition — local/pre-push diff review with no PR in context. Suggested rewording:

Use when reviewing a code change directly (local diff, pre-push, or standalone) with no PR in context. Identifies the change, reads surrounding source, evaluates across six review dimensions, and compiles structured findings.

Verify the final wording against agents/review.md's actual routing logic before committing.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 1:40 PM UTC · Completed 1:46 PM UTC

Commit: 4497e46 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 3 (human-triggered)

Addressed the human instruction: removed the factually inaccurate 'or by the pr-review skill' delegation claim from the code-review skill frontmatter description. Verified against agents/review.md that code-review and pr-review are alternate skills selected by invocation context, not a delegation chain. Rewording matches the actual routing logic: code-review is for local diff, pre-push, or standalone review with no PR in context.

Fixed (1):

1. frontmatter description falsely claims code-review is invoked by pr-review skill (skills/code-review/SKILL.md): Removed the false delegation claim ('or by the pr-review skill') and replaced with accurate trigger condition ('local diff, pre-push, or standalone with no PR in context'). Verified against agents/review.md routing logic (lines 89-108) and confirmed no pr-review sub-agents reference code-review.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:48 PM UTC · Completed 2:03 PM UTC

Commit: 3e0c9a8 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:12 PM UTC · Completed 2:24 PM UTC

Commit: 36706f6 · View workflow run →

Superseded by updated review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:38 PM UTC · Completed 3:51 PM UTC

Commit: b8703f1 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 4:09 PM UTC · Completed 4:25 PM UTC

Commit: b8703f1 · View workflow run →

[fullsend-ai-retro]

Retro: PR #749 — docs(code-review): tighten weak language and add trigger phrase

What happened

A small docs PR (8 additions, 6 deletions, single file skills/code-review/SKILL.md) went through 4 fix iterations, 6 successful review runs, and took ~3 days to merge. The original change added a "Use when..." trigger phrase to the skill description and rewrote two negative-only review prompts as positive, checkable ones.

The review agent found low-severity style issues but missed a factual error in the existing description (carried forward by the PR) claiming pr-review delegates to code-review. Human reviewer waynesun09 caught this by cross-referencing agents/review.md and skills/pr-review/sub-agents/. The fix agent's first bare /fs-fix run addressed only agent-generated style findings and did not see the human comment. Two additional human-directed /fs-fix commands were needed before the factual error was corrected.

Notably, the review agent's own suggested remediation for the style finding perpetuated the false delegation claim. The fix agent implemented a variation of this suggestion without verifying the factual accuracy of the replacement text — despite its zero-trust policy.

On the positive side, after fix iteration 3 introduced wording ("with no PR in context") that contradicted the skill body's references to PR-based invocation, the review agent correctly caught this internal consistency issue, and fix iteration 4 resolved it.

Evidence for existing issues

• #643 (Fix agent should prioritize human /fs-fix instructions over review bot findings): The first bare /fs-fix ignored waynesun09's detailed MEDIUM-severity human review comment and addressed only agent-generated LOW findings. This added ~25 hours of delay and 2 extra iterations.
• #265 / #420: The review agent's correctness sub-agent has explicit guidance for cross-file verification and technical documentation accuracy, yet did not verify the delegation claim against the actual skill routing code. The sub-agent definition includes "Cross-file verification" and "Technical documentation with correctness surface area" sections that should have caught this.
• #537: The review agent's suggested remediation text itself contained the same factual error it should have been catching — the suggestion perpetuated the false pr-review delegation claim.
• #636 / #543: 4 iterations for an 8-line docs change is disproportionate. The cascade pattern (fix 3 introduced a contradiction that required fix 4) matches the cascading-fix anti-pattern.
• #373: 6 full review runs (each dispatching 8 sub-agents on opus) for a single-file docs change suggests review dimension pruning for docs-only PRs would save significant tokens. However, this PR also demonstrates that SKILL.md frontmatter changes have functional routing impact, so any dimension-skipping implementation should exempt skill metadata files.

Proposals filed

• Fix agent should verify factual claims in review-suggested remediation text (in fullsend-ai/agents)