Skip to content

Refactor Copilot Review Instructions and Skills - #799

Merged
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review
Aug 18, 2026
Merged

Refactor Copilot Review Instructions and Skills#799
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

  • add a provider-independent code-review skill that routes changed files to the applicable language, documentation, and workflow skills
  • reduce the Copilot instruction file to a review bootstrap while preserving its repository-specific disproved-claims ledger
  • require visible findings and a machine-readable coverage marker
  • make pr_review.py parse the marker and block unstated coverage with exit 45

Root Cause

The Copilot instruction file duplicated provider-independent rules and embedded API mechanics already implemented by scripts/pr_review.py. Copilot review output also had no stable machine-readable coverage contract, while an unstated coverage result did not produce a blocking exit code.

Validation

  • uvx ruff@latest check .
  • uvx ruff@latest format --check .
  • uvx mypy@latest
  • full coverage and self-test sequence from OPERATIONS.md (79% total coverage, 97% for scripts/pr_review.py)
  • python3 scripts/build_dist.py --check
  • python3 scripts/repo_gate.py
  • complete prose, JSON, spec, EditorConfig, shellcheck, PSScriptAnalyzer, and Markdown gates
  • 688 script tests and 45 agent-safety install tests

Follow-Up

Closes#793

CopilotAI lite review requested due to automatic review settings August 17, 2026 23:20

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the GitHub Copilot review bootstrap to route through a provider-independent code-review skill, and updates scripts/pr_review.py + tests to accept a stable machine-readable coverage marker and to block “unstated coverage” with a dedicated exit code.

Changes:

  • Introduce a new code-review skill (in both .agents/skills and the Claude plugin distribution) that standardizes review procedure and the structured coverage marker.
  • Slim .github/copilot-instructions.md down to a bootstrap + output contract while preserving the repository’s disproved-claims ledger.
  • Extend scripts/pr_review.py and its tests to parse <!-- fleet-review: ... --> and to return exit 45 when coverage is unstated.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
scripts/tests/test_pr_review.pyUpdates corpus/status tests to cover the new structured marker and new exit-45 behavior.
scripts/pr_review.pyAdds marker parsing and blocks unstated coverage with exit 45.
AGENTS.mdRoutes “reviewing a change set” to the new code-review skill.
.github/copilot-instructions.mdReduces to review bootstrap/runbook + preserves disproved-claims ledger.
.agents/skills/code-review/SKILL.mdAdds provider-independent review skill and defines the structured coverage marker contract.
.agents/skills/pr-review-conduct/SKILL.mdUpdates “mechanics live elsewhere” guidance to point to scripts/pr_review.py + bootstrap.
.agents/skills/copilot-instructions-keeper/SKILL.mdAdjusts description to reflect bootstrap role of Copilot instructions.
.claude-plugin/fleet-skills/skills/code-review/SKILL.mdDistributes the new code-review skill into the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.mdMirrors the same “mechanics live elsewhere” update for the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/copilot-instructions-keeper/SKILL.mdMirrors the bootstrap-description update for the Claude plugin skill set.
.claude-plugin/fleet-skills/.source-digestUpdates the generated skill-set digest.
.claude-plugin/fleet-skills/.claude-plugin/plugin.jsonRegisters the new code-review skill in the plugin manifest.
Suppressed comments (1)

scripts/tests/test_pr_review.py:911

  • The docstring for this test still says that treating a no-coverage body as a failure would “cry wolf”, but scripts/pr_review.py status now blocks on coverage=unstated with exit 45. Update the docstring to reflect that this body shape is recognized as coverage=unstated (not UNVETTED), but it is still a blocking outcome for status because it provides no proof of full diff coverage.
 def test_a_round_stating_no_coverage_at_all_is_unstated(self) -> None:
"""28 of the 332 bodies are an overview and a change list, and that shape is current.
It interleaves with the counted one throughout rather than preceding it, and one pull
request carries both across its two rounds, so failing on it would cry wolf on about one

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadscripts/pr_review.py
CopilotAI review requested due to automatic review settings August 17, 2026 23:28
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Copilot review round 4955381803 reported Suppressed comments (1).

scripts/tests/test_pr_review.py:911: The docstring still says that treating a no-coverage body as a failure would "cry wolf", while unstated coverage now blocks with exit 45.

Fixed in f1c24d7. The test now states that this is a recognized coverage=unstated shape, but it blocks because it cannot prove full diff coverage.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings August 17, 2026 23:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/tests/test_pr_review.py:2844

  • This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md. Reading the generated .github/skills/ file here would better validate the actual Copilot-facing contract (and would fail if the generated tree wasn’t regenerated/committed).
    .github/skills/code-review/SKILL.md:17
  • The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/. In downstream repos (where .agents/skills/ is not carried), this instruction is impossible to follow and will prevent the intended routing to the per-language/workflow skills.
3. Load every applicable skill from `.agents/skills/`:

CopilotAI review requested due to automatic review settings August 18, 2026 00:02
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding responses

Review round on e1396f8 reported Suppressed comments (1).

  • scripts/tests/test_pr_review.py:911: "The docstring for this test still says that treating a no-coverage body as a failure would 'cry wolf', but scripts/pr_review.py status now blocks on coverage=unstated with exit 45."
    • Fixed in f1c24d7 - The fixture and its documentation state that unstated coverage is recognized but blocks the gate. A regression test asserts exit 45 for that state.

Review round on da7249f reported Suppressed comments (2).

  • scripts/tests/test_pr_review.py:2844: "This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md."
    • Fixed in c3738e5 - The contract test reads the generated Copilot-facing skill.
  • .github/skills/code-review/SKILL.md:17: "The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/."
    • Fixed in c3738e5 - The canonical skill now routes to applicable sibling skills in the current distribution. Regeneration applies that provider-neutral instruction to both generated trees.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/build_dist.py:180

  • is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root (for example an accidental hand-added README.md or .DS_Store) would not be detected as drift and build_dist.py --check could still pass. Since the docs and help text describe --check as failing whenever a generated distribution differs from .agents/skills/, it should also treat any non-directory entry under the distribution root as stale (and ideally require the root directory to exist even when there are zero skills).

CopilotAI review requested due to automatic review settings August 18, 2026 00:09
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on c3738e5 reported Suppressed comments (1).

  • scripts/build_dist.py:180: "is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root ... would not be detected as drift."
    • Fixed in 04f4bf7 - Both generated skill roots must exist and contain only the expected skill directories. Three regression tests cover a stray GitHub file, a stray Claude plugin skill file, and a missing empty GitHub distribution.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/pr_review.py:19

  • The status subcommand docstring says exit 0 implies “the round covering the head read the whole diff”, but report_verdict() returns 0 early when there are no head-covering reviews (if not head_reviews(pr): return 0). This makes exit 0 ambiguous (no review yet vs full coverage), so the docstring should describe the no-review case explicitly.

CopilotAI review requested due to automatic review settings August 18, 2026 00:16
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on 04f4bf7 reported Suppressed comments (1).

  • scripts/pr_review.py:19: "The status subcommand docstring says exit 0 implies 'the round covering the head read the whole diff', but report_verdict() returns 0 early when there are no head-covering reviews."
    • Fixed in ced266c - The CLI contract explicitly distinguishes the no-review success case and directs callers that require review presence to wait. A contract test protects the documented behavior.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 05c101e into developAug 18, 2026
8 checks passed
@ptr727
ptr727 deleted the feature/issue-793-copilot-review branch August 18, 2026 00:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Refactor Copilot Review Instructions and Skills by ptr727 · Pull Request #799 · ptr727/ProjectTemplate · GitHub
Skip to content

Refactor Copilot Review Instructions and Skills - #799

Merged
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review
Aug 18, 2026
Merged

Refactor Copilot Review Instructions and Skills#799
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

  • add a provider-independent code-review skill that routes changed files to the applicable language, documentation, and workflow skills
  • reduce the Copilot instruction file to a review bootstrap while preserving its repository-specific disproved-claims ledger
  • require visible findings and a machine-readable coverage marker
  • make pr_review.py parse the marker and block unstated coverage with exit 45

Root Cause

The Copilot instruction file duplicated provider-independent rules and embedded API mechanics already implemented by scripts/pr_review.py. Copilot review output also had no stable machine-readable coverage contract, while an unstated coverage result did not produce a blocking exit code.

Validation

  • uvx ruff@latest check .
  • uvx ruff@latest format --check .
  • uvx mypy@latest
  • full coverage and self-test sequence from OPERATIONS.md (79% total coverage, 97% for scripts/pr_review.py)
  • python3 scripts/build_dist.py --check
  • python3 scripts/repo_gate.py
  • complete prose, JSON, spec, EditorConfig, shellcheck, PSScriptAnalyzer, and Markdown gates
  • 688 script tests and 45 agent-safety install tests

Follow-Up

Closes#793

CopilotAI lite review requested due to automatic review settings August 17, 2026 23:20

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the GitHub Copilot review bootstrap to route through a provider-independent code-review skill, and updates scripts/pr_review.py + tests to accept a stable machine-readable coverage marker and to block “unstated coverage” with a dedicated exit code.

Changes:

  • Introduce a new code-review skill (in both .agents/skills and the Claude plugin distribution) that standardizes review procedure and the structured coverage marker.
  • Slim .github/copilot-instructions.md down to a bootstrap + output contract while preserving the repository’s disproved-claims ledger.
  • Extend scripts/pr_review.py and its tests to parse <!-- fleet-review: ... --> and to return exit 45 when coverage is unstated.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
scripts/tests/test_pr_review.pyUpdates corpus/status tests to cover the new structured marker and new exit-45 behavior.
scripts/pr_review.pyAdds marker parsing and blocks unstated coverage with exit 45.
AGENTS.mdRoutes “reviewing a change set” to the new code-review skill.
.github/copilot-instructions.mdReduces to review bootstrap/runbook + preserves disproved-claims ledger.
.agents/skills/code-review/SKILL.mdAdds provider-independent review skill and defines the structured coverage marker contract.
.agents/skills/pr-review-conduct/SKILL.mdUpdates “mechanics live elsewhere” guidance to point to scripts/pr_review.py + bootstrap.
.agents/skills/copilot-instructions-keeper/SKILL.mdAdjusts description to reflect bootstrap role of Copilot instructions.
.claude-plugin/fleet-skills/skills/code-review/SKILL.mdDistributes the new code-review skill into the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.mdMirrors the same “mechanics live elsewhere” update for the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/copilot-instructions-keeper/SKILL.mdMirrors the bootstrap-description update for the Claude plugin skill set.
.claude-plugin/fleet-skills/.source-digestUpdates the generated skill-set digest.
.claude-plugin/fleet-skills/.claude-plugin/plugin.jsonRegisters the new code-review skill in the plugin manifest.
Suppressed comments (1)

scripts/tests/test_pr_review.py:911

  • The docstring for this test still says that treating a no-coverage body as a failure would “cry wolf”, but scripts/pr_review.py status now blocks on coverage=unstated with exit 45. Update the docstring to reflect that this body shape is recognized as coverage=unstated (not UNVETTED), but it is still a blocking outcome for status because it provides no proof of full diff coverage.
 def test_a_round_stating_no_coverage_at_all_is_unstated(self) -> None:
"""28 of the 332 bodies are an overview and a change list, and that shape is current.
It interleaves with the counted one throughout rather than preceding it, and one pull
request carries both across its two rounds, so failing on it would cry wolf on about one

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadscripts/pr_review.py
CopilotAI review requested due to automatic review settings August 17, 2026 23:28
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Copilot review round 4955381803 reported Suppressed comments (1).

scripts/tests/test_pr_review.py:911: The docstring still says that treating a no-coverage body as a failure would "cry wolf", while unstated coverage now blocks with exit 45.

Fixed in f1c24d7. The test now states that this is a recognized coverage=unstated shape, but it blocks because it cannot prove full diff coverage.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings August 17, 2026 23:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/tests/test_pr_review.py:2844

  • This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md. Reading the generated .github/skills/ file here would better validate the actual Copilot-facing contract (and would fail if the generated tree wasn’t regenerated/committed).
    .github/skills/code-review/SKILL.md:17
  • The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/. In downstream repos (where .agents/skills/ is not carried), this instruction is impossible to follow and will prevent the intended routing to the per-language/workflow skills.
3. Load every applicable skill from `.agents/skills/`:

CopilotAI review requested due to automatic review settings August 18, 2026 00:02
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding responses

Review round on e1396f8 reported Suppressed comments (1).

  • scripts/tests/test_pr_review.py:911: "The docstring for this test still says that treating a no-coverage body as a failure would 'cry wolf', but scripts/pr_review.py status now blocks on coverage=unstated with exit 45."
    • Fixed in f1c24d7 - The fixture and its documentation state that unstated coverage is recognized but blocks the gate. A regression test asserts exit 45 for that state.

Review round on da7249f reported Suppressed comments (2).

  • scripts/tests/test_pr_review.py:2844: "This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md."
    • Fixed in c3738e5 - The contract test reads the generated Copilot-facing skill.
  • .github/skills/code-review/SKILL.md:17: "The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/."
    • Fixed in c3738e5 - The canonical skill now routes to applicable sibling skills in the current distribution. Regeneration applies that provider-neutral instruction to both generated trees.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/build_dist.py:180

  • is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root (for example an accidental hand-added README.md or .DS_Store) would not be detected as drift and build_dist.py --check could still pass. Since the docs and help text describe --check as failing whenever a generated distribution differs from .agents/skills/, it should also treat any non-directory entry under the distribution root as stale (and ideally require the root directory to exist even when there are zero skills).

CopilotAI review requested due to automatic review settings August 18, 2026 00:09
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on c3738e5 reported Suppressed comments (1).

  • scripts/build_dist.py:180: "is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root ... would not be detected as drift."
    • Fixed in 04f4bf7 - Both generated skill roots must exist and contain only the expected skill directories. Three regression tests cover a stray GitHub file, a stray Claude plugin skill file, and a missing empty GitHub distribution.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/pr_review.py:19

  • The status subcommand docstring says exit 0 implies “the round covering the head read the whole diff”, but report_verdict() returns 0 early when there are no head-covering reviews (if not head_reviews(pr): return 0). This makes exit 0 ambiguous (no review yet vs full coverage), so the docstring should describe the no-review case explicitly.

CopilotAI review requested due to automatic review settings August 18, 2026 00:16
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on 04f4bf7 reported Suppressed comments (1).

  • scripts/pr_review.py:19: "The status subcommand docstring says exit 0 implies 'the round covering the head read the whole diff', but report_verdict() returns 0 early when there are no head-covering reviews."
    • Fixed in ced266c - The CLI contract explicitly distinguishes the no-review success case and directs callers that require review presence to wait. A contract test protects the documented behavior.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 05c101e into developAug 18, 2026
8 checks passed
@ptr727
ptr727 deleted the feature/issue-793-copilot-review branch August 18, 2026 00:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Refactor Copilot Review Instructions and Skills by ptr727 · Pull Request #799 · ptr727/ProjectTemplate · GitHub
Skip to content

Refactor Copilot Review Instructions and Skills - #799

Merged
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review
Aug 18, 2026
Merged

Refactor Copilot Review Instructions and Skills#799
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

  • add a provider-independent code-review skill that routes changed files to the applicable language, documentation, and workflow skills
  • reduce the Copilot instruction file to a review bootstrap while preserving its repository-specific disproved-claims ledger
  • require visible findings and a machine-readable coverage marker
  • make pr_review.py parse the marker and block unstated coverage with exit 45

Root Cause

The Copilot instruction file duplicated provider-independent rules and embedded API mechanics already implemented by scripts/pr_review.py. Copilot review output also had no stable machine-readable coverage contract, while an unstated coverage result did not produce a blocking exit code.

Validation

  • uvx ruff@latest check .
  • uvx ruff@latest format --check .
  • uvx mypy@latest
  • full coverage and self-test sequence from OPERATIONS.md (79% total coverage, 97% for scripts/pr_review.py)
  • python3 scripts/build_dist.py --check
  • python3 scripts/repo_gate.py
  • complete prose, JSON, spec, EditorConfig, shellcheck, PSScriptAnalyzer, and Markdown gates
  • 688 script tests and 45 agent-safety install tests

Follow-Up

Closes#793

CopilotAI lite review requested due to automatic review settings August 17, 2026 23:20

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the GitHub Copilot review bootstrap to route through a provider-independent code-review skill, and updates scripts/pr_review.py + tests to accept a stable machine-readable coverage marker and to block “unstated coverage” with a dedicated exit code.

Changes:

  • Introduce a new code-review skill (in both .agents/skills and the Claude plugin distribution) that standardizes review procedure and the structured coverage marker.
  • Slim .github/copilot-instructions.md down to a bootstrap + output contract while preserving the repository’s disproved-claims ledger.
  • Extend scripts/pr_review.py and its tests to parse <!-- fleet-review: ... --> and to return exit 45 when coverage is unstated.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
scripts/tests/test_pr_review.pyUpdates corpus/status tests to cover the new structured marker and new exit-45 behavior.
scripts/pr_review.pyAdds marker parsing and blocks unstated coverage with exit 45.
AGENTS.mdRoutes “reviewing a change set” to the new code-review skill.
.github/copilot-instructions.mdReduces to review bootstrap/runbook + preserves disproved-claims ledger.
.agents/skills/code-review/SKILL.mdAdds provider-independent review skill and defines the structured coverage marker contract.
.agents/skills/pr-review-conduct/SKILL.mdUpdates “mechanics live elsewhere” guidance to point to scripts/pr_review.py + bootstrap.
.agents/skills/copilot-instructions-keeper/SKILL.mdAdjusts description to reflect bootstrap role of Copilot instructions.
.claude-plugin/fleet-skills/skills/code-review/SKILL.mdDistributes the new code-review skill into the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.mdMirrors the same “mechanics live elsewhere” update for the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/copilot-instructions-keeper/SKILL.mdMirrors the bootstrap-description update for the Claude plugin skill set.
.claude-plugin/fleet-skills/.source-digestUpdates the generated skill-set digest.
.claude-plugin/fleet-skills/.claude-plugin/plugin.jsonRegisters the new code-review skill in the plugin manifest.
Suppressed comments (1)

scripts/tests/test_pr_review.py:911

  • The docstring for this test still says that treating a no-coverage body as a failure would “cry wolf”, but scripts/pr_review.py status now blocks on coverage=unstated with exit 45. Update the docstring to reflect that this body shape is recognized as coverage=unstated (not UNVETTED), but it is still a blocking outcome for status because it provides no proof of full diff coverage.
 def test_a_round_stating_no_coverage_at_all_is_unstated(self) -> None:
"""28 of the 332 bodies are an overview and a change list, and that shape is current.
It interleaves with the counted one throughout rather than preceding it, and one pull
request carries both across its two rounds, so failing on it would cry wolf on about one

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadscripts/pr_review.py
CopilotAI review requested due to automatic review settings August 17, 2026 23:28
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Copilot review round 4955381803 reported Suppressed comments (1).

scripts/tests/test_pr_review.py:911: The docstring still says that treating a no-coverage body as a failure would "cry wolf", while unstated coverage now blocks with exit 45.

Fixed in f1c24d7. The test now states that this is a recognized coverage=unstated shape, but it blocks because it cannot prove full diff coverage.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings August 17, 2026 23:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/tests/test_pr_review.py:2844

  • This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md. Reading the generated .github/skills/ file here would better validate the actual Copilot-facing contract (and would fail if the generated tree wasn’t regenerated/committed).
    .github/skills/code-review/SKILL.md:17
  • The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/. In downstream repos (where .agents/skills/ is not carried), this instruction is impossible to follow and will prevent the intended routing to the per-language/workflow skills.
3. Load every applicable skill from `.agents/skills/`:

CopilotAI review requested due to automatic review settings August 18, 2026 00:02
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding responses

Review round on e1396f8 reported Suppressed comments (1).

  • scripts/tests/test_pr_review.py:911: "The docstring for this test still says that treating a no-coverage body as a failure would 'cry wolf', but scripts/pr_review.py status now blocks on coverage=unstated with exit 45."
    • Fixed in f1c24d7 - The fixture and its documentation state that unstated coverage is recognized but blocks the gate. A regression test asserts exit 45 for that state.

Review round on da7249f reported Suppressed comments (2).

  • scripts/tests/test_pr_review.py:2844: "This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md."
    • Fixed in c3738e5 - The contract test reads the generated Copilot-facing skill.
  • .github/skills/code-review/SKILL.md:17: "The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/."
    • Fixed in c3738e5 - The canonical skill now routes to applicable sibling skills in the current distribution. Regeneration applies that provider-neutral instruction to both generated trees.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/build_dist.py:180

  • is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root (for example an accidental hand-added README.md or .DS_Store) would not be detected as drift and build_dist.py --check could still pass. Since the docs and help text describe --check as failing whenever a generated distribution differs from .agents/skills/, it should also treat any non-directory entry under the distribution root as stale (and ideally require the root directory to exist even when there are zero skills).

CopilotAI review requested due to automatic review settings August 18, 2026 00:09
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on c3738e5 reported Suppressed comments (1).

  • scripts/build_dist.py:180: "is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root ... would not be detected as drift."
    • Fixed in 04f4bf7 - Both generated skill roots must exist and contain only the expected skill directories. Three regression tests cover a stray GitHub file, a stray Claude plugin skill file, and a missing empty GitHub distribution.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/pr_review.py:19

  • The status subcommand docstring says exit 0 implies “the round covering the head read the whole diff”, but report_verdict() returns 0 early when there are no head-covering reviews (if not head_reviews(pr): return 0). This makes exit 0 ambiguous (no review yet vs full coverage), so the docstring should describe the no-review case explicitly.

CopilotAI review requested due to automatic review settings August 18, 2026 00:16
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on 04f4bf7 reported Suppressed comments (1).

  • scripts/pr_review.py:19: "The status subcommand docstring says exit 0 implies 'the round covering the head read the whole diff', but report_verdict() returns 0 early when there are no head-covering reviews."
    • Fixed in ced266c - The CLI contract explicitly distinguishes the no-review success case and directs callers that require review presence to wait. A contract test protects the documented behavior.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 05c101e into developAug 18, 2026
8 checks passed
@ptr727
ptr727 deleted the feature/issue-793-copilot-review branch August 18, 2026 00:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Refactor Copilot Review Instructions and Skills by ptr727 · Pull Request #799 · ptr727/ProjectTemplate · GitHub
Skip to content

Refactor Copilot Review Instructions and Skills - #799

Merged
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review
Aug 18, 2026
Merged

Refactor Copilot Review Instructions and Skills#799
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

  • add a provider-independent code-review skill that routes changed files to the applicable language, documentation, and workflow skills
  • reduce the Copilot instruction file to a review bootstrap while preserving its repository-specific disproved-claims ledger
  • require visible findings and a machine-readable coverage marker
  • make pr_review.py parse the marker and block unstated coverage with exit 45

Root Cause

The Copilot instruction file duplicated provider-independent rules and embedded API mechanics already implemented by scripts/pr_review.py. Copilot review output also had no stable machine-readable coverage contract, while an unstated coverage result did not produce a blocking exit code.

Validation

  • uvx ruff@latest check .
  • uvx ruff@latest format --check .
  • uvx mypy@latest
  • full coverage and self-test sequence from OPERATIONS.md (79% total coverage, 97% for scripts/pr_review.py)
  • python3 scripts/build_dist.py --check
  • python3 scripts/repo_gate.py
  • complete prose, JSON, spec, EditorConfig, shellcheck, PSScriptAnalyzer, and Markdown gates
  • 688 script tests and 45 agent-safety install tests

Follow-Up

Closes#793

CopilotAI lite review requested due to automatic review settings August 17, 2026 23:20

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the GitHub Copilot review bootstrap to route through a provider-independent code-review skill, and updates scripts/pr_review.py + tests to accept a stable machine-readable coverage marker and to block “unstated coverage” with a dedicated exit code.

Changes:

  • Introduce a new code-review skill (in both .agents/skills and the Claude plugin distribution) that standardizes review procedure and the structured coverage marker.
  • Slim .github/copilot-instructions.md down to a bootstrap + output contract while preserving the repository’s disproved-claims ledger.
  • Extend scripts/pr_review.py and its tests to parse <!-- fleet-review: ... --> and to return exit 45 when coverage is unstated.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
scripts/tests/test_pr_review.pyUpdates corpus/status tests to cover the new structured marker and new exit-45 behavior.
scripts/pr_review.pyAdds marker parsing and blocks unstated coverage with exit 45.
AGENTS.mdRoutes “reviewing a change set” to the new code-review skill.
.github/copilot-instructions.mdReduces to review bootstrap/runbook + preserves disproved-claims ledger.
.agents/skills/code-review/SKILL.mdAdds provider-independent review skill and defines the structured coverage marker contract.
.agents/skills/pr-review-conduct/SKILL.mdUpdates “mechanics live elsewhere” guidance to point to scripts/pr_review.py + bootstrap.
.agents/skills/copilot-instructions-keeper/SKILL.mdAdjusts description to reflect bootstrap role of Copilot instructions.
.claude-plugin/fleet-skills/skills/code-review/SKILL.mdDistributes the new code-review skill into the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.mdMirrors the same “mechanics live elsewhere” update for the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/copilot-instructions-keeper/SKILL.mdMirrors the bootstrap-description update for the Claude plugin skill set.
.claude-plugin/fleet-skills/.source-digestUpdates the generated skill-set digest.
.claude-plugin/fleet-skills/.claude-plugin/plugin.jsonRegisters the new code-review skill in the plugin manifest.
Suppressed comments (1)

scripts/tests/test_pr_review.py:911

  • The docstring for this test still says that treating a no-coverage body as a failure would “cry wolf”, but scripts/pr_review.py status now blocks on coverage=unstated with exit 45. Update the docstring to reflect that this body shape is recognized as coverage=unstated (not UNVETTED), but it is still a blocking outcome for status because it provides no proof of full diff coverage.
 def test_a_round_stating_no_coverage_at_all_is_unstated(self) -> None:
"""28 of the 332 bodies are an overview and a change list, and that shape is current.
It interleaves with the counted one throughout rather than preceding it, and one pull
request carries both across its two rounds, so failing on it would cry wolf on about one

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadscripts/pr_review.py
CopilotAI review requested due to automatic review settings August 17, 2026 23:28
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Copilot review round 4955381803 reported Suppressed comments (1).

scripts/tests/test_pr_review.py:911: The docstring still says that treating a no-coverage body as a failure would "cry wolf", while unstated coverage now blocks with exit 45.

Fixed in f1c24d7. The test now states that this is a recognized coverage=unstated shape, but it blocks because it cannot prove full diff coverage.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings August 17, 2026 23:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/tests/test_pr_review.py:2844

  • This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md. Reading the generated .github/skills/ file here would better validate the actual Copilot-facing contract (and would fail if the generated tree wasn’t regenerated/committed).
    .github/skills/code-review/SKILL.md:17
  • The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/. In downstream repos (where .agents/skills/ is not carried), this instruction is impossible to follow and will prevent the intended routing to the per-language/workflow skills.
3. Load every applicable skill from `.agents/skills/`:

CopilotAI review requested due to automatic review settings August 18, 2026 00:02
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding responses

Review round on e1396f8 reported Suppressed comments (1).

  • scripts/tests/test_pr_review.py:911: "The docstring for this test still says that treating a no-coverage body as a failure would 'cry wolf', but scripts/pr_review.py status now blocks on coverage=unstated with exit 45."
    • Fixed in f1c24d7 - The fixture and its documentation state that unstated coverage is recognized but blocks the gate. A regression test asserts exit 45 for that state.

Review round on da7249f reported Suppressed comments (2).

  • scripts/tests/test_pr_review.py:2844: "This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md."
    • Fixed in c3738e5 - The contract test reads the generated Copilot-facing skill.
  • .github/skills/code-review/SKILL.md:17: "The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/."
    • Fixed in c3738e5 - The canonical skill now routes to applicable sibling skills in the current distribution. Regeneration applies that provider-neutral instruction to both generated trees.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/build_dist.py:180

  • is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root (for example an accidental hand-added README.md or .DS_Store) would not be detected as drift and build_dist.py --check could still pass. Since the docs and help text describe --check as failing whenever a generated distribution differs from .agents/skills/, it should also treat any non-directory entry under the distribution root as stale (and ideally require the root directory to exist even when there are zero skills).

CopilotAI review requested due to automatic review settings August 18, 2026 00:09
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on c3738e5 reported Suppressed comments (1).

  • scripts/build_dist.py:180: "is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root ... would not be detected as drift."
    • Fixed in 04f4bf7 - Both generated skill roots must exist and contain only the expected skill directories. Three regression tests cover a stray GitHub file, a stray Claude plugin skill file, and a missing empty GitHub distribution.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/pr_review.py:19

  • The status subcommand docstring says exit 0 implies “the round covering the head read the whole diff”, but report_verdict() returns 0 early when there are no head-covering reviews (if not head_reviews(pr): return 0). This makes exit 0 ambiguous (no review yet vs full coverage), so the docstring should describe the no-review case explicitly.

CopilotAI review requested due to automatic review settings August 18, 2026 00:16
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on 04f4bf7 reported Suppressed comments (1).

  • scripts/pr_review.py:19: "The status subcommand docstring says exit 0 implies 'the round covering the head read the whole diff', but report_verdict() returns 0 early when there are no head-covering reviews."
    • Fixed in ced266c - The CLI contract explicitly distinguishes the no-review success case and directs callers that require review presence to wait. A contract test protects the documented behavior.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 05c101e into developAug 18, 2026
8 checks passed
@ptr727
ptr727 deleted the feature/issue-793-copilot-review branch August 18, 2026 00:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Refactor Copilot Review Instructions and Skills by ptr727 · Pull Request #799 · ptr727/ProjectTemplate · GitHub
Skip to content

Refactor Copilot Review Instructions and Skills - #799

Merged
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review
Aug 18, 2026
Merged

Refactor Copilot Review Instructions and Skills#799
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

  • add a provider-independent code-review skill that routes changed files to the applicable language, documentation, and workflow skills
  • reduce the Copilot instruction file to a review bootstrap while preserving its repository-specific disproved-claims ledger
  • require visible findings and a machine-readable coverage marker
  • make pr_review.py parse the marker and block unstated coverage with exit 45

Root Cause

The Copilot instruction file duplicated provider-independent rules and embedded API mechanics already implemented by scripts/pr_review.py. Copilot review output also had no stable machine-readable coverage contract, while an unstated coverage result did not produce a blocking exit code.

Validation

  • uvx ruff@latest check .
  • uvx ruff@latest format --check .
  • uvx mypy@latest
  • full coverage and self-test sequence from OPERATIONS.md (79% total coverage, 97% for scripts/pr_review.py)
  • python3 scripts/build_dist.py --check
  • python3 scripts/repo_gate.py
  • complete prose, JSON, spec, EditorConfig, shellcheck, PSScriptAnalyzer, and Markdown gates
  • 688 script tests and 45 agent-safety install tests

Follow-Up

Closes#793

CopilotAI lite review requested due to automatic review settings August 17, 2026 23:20

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the GitHub Copilot review bootstrap to route through a provider-independent code-review skill, and updates scripts/pr_review.py + tests to accept a stable machine-readable coverage marker and to block “unstated coverage” with a dedicated exit code.

Changes:

  • Introduce a new code-review skill (in both .agents/skills and the Claude plugin distribution) that standardizes review procedure and the structured coverage marker.
  • Slim .github/copilot-instructions.md down to a bootstrap + output contract while preserving the repository’s disproved-claims ledger.
  • Extend scripts/pr_review.py and its tests to parse <!-- fleet-review: ... --> and to return exit 45 when coverage is unstated.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
scripts/tests/test_pr_review.pyUpdates corpus/status tests to cover the new structured marker and new exit-45 behavior.
scripts/pr_review.pyAdds marker parsing and blocks unstated coverage with exit 45.
AGENTS.mdRoutes “reviewing a change set” to the new code-review skill.
.github/copilot-instructions.mdReduces to review bootstrap/runbook + preserves disproved-claims ledger.
.agents/skills/code-review/SKILL.mdAdds provider-independent review skill and defines the structured coverage marker contract.
.agents/skills/pr-review-conduct/SKILL.mdUpdates “mechanics live elsewhere” guidance to point to scripts/pr_review.py + bootstrap.
.agents/skills/copilot-instructions-keeper/SKILL.mdAdjusts description to reflect bootstrap role of Copilot instructions.
.claude-plugin/fleet-skills/skills/code-review/SKILL.mdDistributes the new code-review skill into the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.mdMirrors the same “mechanics live elsewhere” update for the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/copilot-instructions-keeper/SKILL.mdMirrors the bootstrap-description update for the Claude plugin skill set.
.claude-plugin/fleet-skills/.source-digestUpdates the generated skill-set digest.
.claude-plugin/fleet-skills/.claude-plugin/plugin.jsonRegisters the new code-review skill in the plugin manifest.
Suppressed comments (1)

scripts/tests/test_pr_review.py:911

  • The docstring for this test still says that treating a no-coverage body as a failure would “cry wolf”, but scripts/pr_review.py status now blocks on coverage=unstated with exit 45. Update the docstring to reflect that this body shape is recognized as coverage=unstated (not UNVETTED), but it is still a blocking outcome for status because it provides no proof of full diff coverage.
 def test_a_round_stating_no_coverage_at_all_is_unstated(self) -> None:
"""28 of the 332 bodies are an overview and a change list, and that shape is current.
It interleaves with the counted one throughout rather than preceding it, and one pull
request carries both across its two rounds, so failing on it would cry wolf on about one

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadscripts/pr_review.py
CopilotAI review requested due to automatic review settings August 17, 2026 23:28
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Copilot review round 4955381803 reported Suppressed comments (1).

scripts/tests/test_pr_review.py:911: The docstring still says that treating a no-coverage body as a failure would "cry wolf", while unstated coverage now blocks with exit 45.

Fixed in f1c24d7. The test now states that this is a recognized coverage=unstated shape, but it blocks because it cannot prove full diff coverage.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings August 17, 2026 23:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/tests/test_pr_review.py:2844

  • This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md. Reading the generated .github/skills/ file here would better validate the actual Copilot-facing contract (and would fail if the generated tree wasn’t regenerated/committed).
    .github/skills/code-review/SKILL.md:17
  • The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/. In downstream repos (where .agents/skills/ is not carried), this instruction is impossible to follow and will prevent the intended routing to the per-language/workflow skills.
3. Load every applicable skill from `.agents/skills/`:

CopilotAI review requested due to automatic review settings August 18, 2026 00:02
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding responses

Review round on e1396f8 reported Suppressed comments (1).

  • scripts/tests/test_pr_review.py:911: "The docstring for this test still says that treating a no-coverage body as a failure would 'cry wolf', but scripts/pr_review.py status now blocks on coverage=unstated with exit 45."
    • Fixed in f1c24d7 - The fixture and its documentation state that unstated coverage is recognized but blocks the gate. A regression test asserts exit 45 for that state.

Review round on da7249f reported Suppressed comments (2).

  • scripts/tests/test_pr_review.py:2844: "This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md."
    • Fixed in c3738e5 - The contract test reads the generated Copilot-facing skill.
  • .github/skills/code-review/SKILL.md:17: "The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/."
    • Fixed in c3738e5 - The canonical skill now routes to applicable sibling skills in the current distribution. Regeneration applies that provider-neutral instruction to both generated trees.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/build_dist.py:180

  • is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root (for example an accidental hand-added README.md or .DS_Store) would not be detected as drift and build_dist.py --check could still pass. Since the docs and help text describe --check as failing whenever a generated distribution differs from .agents/skills/, it should also treat any non-directory entry under the distribution root as stale (and ideally require the root directory to exist even when there are zero skills).

CopilotAI review requested due to automatic review settings August 18, 2026 00:09
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on c3738e5 reported Suppressed comments (1).

  • scripts/build_dist.py:180: "is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root ... would not be detected as drift."
    • Fixed in 04f4bf7 - Both generated skill roots must exist and contain only the expected skill directories. Three regression tests cover a stray GitHub file, a stray Claude plugin skill file, and a missing empty GitHub distribution.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/pr_review.py:19

  • The status subcommand docstring says exit 0 implies “the round covering the head read the whole diff”, but report_verdict() returns 0 early when there are no head-covering reviews (if not head_reviews(pr): return 0). This makes exit 0 ambiguous (no review yet vs full coverage), so the docstring should describe the no-review case explicitly.

CopilotAI review requested due to automatic review settings August 18, 2026 00:16
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on 04f4bf7 reported Suppressed comments (1).

  • scripts/pr_review.py:19: "The status subcommand docstring says exit 0 implies 'the round covering the head read the whole diff', but report_verdict() returns 0 early when there are no head-covering reviews."
    • Fixed in ced266c - The CLI contract explicitly distinguishes the no-review success case and directs callers that require review presence to wait. A contract test protects the documented behavior.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 05c101e into developAug 18, 2026
8 checks passed
@ptr727
ptr727 deleted the feature/issue-793-copilot-review branch August 18, 2026 00:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Refactor Copilot Review Instructions and Skills by ptr727 · Pull Request #799 · ptr727/ProjectTemplate · GitHub
Skip to content

Refactor Copilot Review Instructions and Skills - #799

Merged
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review
Aug 18, 2026
Merged

Refactor Copilot Review Instructions and Skills#799
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

  • add a provider-independent code-review skill that routes changed files to the applicable language, documentation, and workflow skills
  • reduce the Copilot instruction file to a review bootstrap while preserving its repository-specific disproved-claims ledger
  • require visible findings and a machine-readable coverage marker
  • make pr_review.py parse the marker and block unstated coverage with exit 45

Root Cause

The Copilot instruction file duplicated provider-independent rules and embedded API mechanics already implemented by scripts/pr_review.py. Copilot review output also had no stable machine-readable coverage contract, while an unstated coverage result did not produce a blocking exit code.

Validation

  • uvx ruff@latest check .
  • uvx ruff@latest format --check .
  • uvx mypy@latest
  • full coverage and self-test sequence from OPERATIONS.md (79% total coverage, 97% for scripts/pr_review.py)
  • python3 scripts/build_dist.py --check
  • python3 scripts/repo_gate.py
  • complete prose, JSON, spec, EditorConfig, shellcheck, PSScriptAnalyzer, and Markdown gates
  • 688 script tests and 45 agent-safety install tests

Follow-Up

Closes#793

CopilotAI lite review requested due to automatic review settings August 17, 2026 23:20

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the GitHub Copilot review bootstrap to route through a provider-independent code-review skill, and updates scripts/pr_review.py + tests to accept a stable machine-readable coverage marker and to block “unstated coverage” with a dedicated exit code.

Changes:

  • Introduce a new code-review skill (in both .agents/skills and the Claude plugin distribution) that standardizes review procedure and the structured coverage marker.
  • Slim .github/copilot-instructions.md down to a bootstrap + output contract while preserving the repository’s disproved-claims ledger.
  • Extend scripts/pr_review.py and its tests to parse <!-- fleet-review: ... --> and to return exit 45 when coverage is unstated.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
scripts/tests/test_pr_review.pyUpdates corpus/status tests to cover the new structured marker and new exit-45 behavior.
scripts/pr_review.pyAdds marker parsing and blocks unstated coverage with exit 45.
AGENTS.mdRoutes “reviewing a change set” to the new code-review skill.
.github/copilot-instructions.mdReduces to review bootstrap/runbook + preserves disproved-claims ledger.
.agents/skills/code-review/SKILL.mdAdds provider-independent review skill and defines the structured coverage marker contract.
.agents/skills/pr-review-conduct/SKILL.mdUpdates “mechanics live elsewhere” guidance to point to scripts/pr_review.py + bootstrap.
.agents/skills/copilot-instructions-keeper/SKILL.mdAdjusts description to reflect bootstrap role of Copilot instructions.
.claude-plugin/fleet-skills/skills/code-review/SKILL.mdDistributes the new code-review skill into the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.mdMirrors the same “mechanics live elsewhere” update for the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/copilot-instructions-keeper/SKILL.mdMirrors the bootstrap-description update for the Claude plugin skill set.
.claude-plugin/fleet-skills/.source-digestUpdates the generated skill-set digest.
.claude-plugin/fleet-skills/.claude-plugin/plugin.jsonRegisters the new code-review skill in the plugin manifest.
Suppressed comments (1)

scripts/tests/test_pr_review.py:911

  • The docstring for this test still says that treating a no-coverage body as a failure would “cry wolf”, but scripts/pr_review.py status now blocks on coverage=unstated with exit 45. Update the docstring to reflect that this body shape is recognized as coverage=unstated (not UNVETTED), but it is still a blocking outcome for status because it provides no proof of full diff coverage.
 def test_a_round_stating_no_coverage_at_all_is_unstated(self) -> None:
"""28 of the 332 bodies are an overview and a change list, and that shape is current.
It interleaves with the counted one throughout rather than preceding it, and one pull
request carries both across its two rounds, so failing on it would cry wolf on about one

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadscripts/pr_review.py
CopilotAI review requested due to automatic review settings August 17, 2026 23:28
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Copilot review round 4955381803 reported Suppressed comments (1).

scripts/tests/test_pr_review.py:911: The docstring still says that treating a no-coverage body as a failure would "cry wolf", while unstated coverage now blocks with exit 45.

Fixed in f1c24d7. The test now states that this is a recognized coverage=unstated shape, but it blocks because it cannot prove full diff coverage.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings August 17, 2026 23:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/tests/test_pr_review.py:2844

  • This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md. Reading the generated .github/skills/ file here would better validate the actual Copilot-facing contract (and would fail if the generated tree wasn’t regenerated/committed).
    .github/skills/code-review/SKILL.md:17
  • The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/. In downstream repos (where .agents/skills/ is not carried), this instruction is impossible to follow and will prevent the intended routing to the per-language/workflow skills.
3. Load every applicable skill from `.agents/skills/`:

CopilotAI review requested due to automatic review settings August 18, 2026 00:02
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding responses

Review round on e1396f8 reported Suppressed comments (1).

  • scripts/tests/test_pr_review.py:911: "The docstring for this test still says that treating a no-coverage body as a failure would 'cry wolf', but scripts/pr_review.py status now blocks on coverage=unstated with exit 45."
    • Fixed in f1c24d7 - The fixture and its documentation state that unstated coverage is recognized but blocks the gate. A regression test asserts exit 45 for that state.

Review round on da7249f reported Suppressed comments (2).

  • scripts/tests/test_pr_review.py:2844: "This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md."
    • Fixed in c3738e5 - The contract test reads the generated Copilot-facing skill.
  • .github/skills/code-review/SKILL.md:17: "The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/."
    • Fixed in c3738e5 - The canonical skill now routes to applicable sibling skills in the current distribution. Regeneration applies that provider-neutral instruction to both generated trees.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/build_dist.py:180

  • is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root (for example an accidental hand-added README.md or .DS_Store) would not be detected as drift and build_dist.py --check could still pass. Since the docs and help text describe --check as failing whenever a generated distribution differs from .agents/skills/, it should also treat any non-directory entry under the distribution root as stale (and ideally require the root directory to exist even when there are zero skills).

CopilotAI review requested due to automatic review settings August 18, 2026 00:09
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on c3738e5 reported Suppressed comments (1).

  • scripts/build_dist.py:180: "is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root ... would not be detected as drift."
    • Fixed in 04f4bf7 - Both generated skill roots must exist and contain only the expected skill directories. Three regression tests cover a stray GitHub file, a stray Claude plugin skill file, and a missing empty GitHub distribution.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/pr_review.py:19

  • The status subcommand docstring says exit 0 implies “the round covering the head read the whole diff”, but report_verdict() returns 0 early when there are no head-covering reviews (if not head_reviews(pr): return 0). This makes exit 0 ambiguous (no review yet vs full coverage), so the docstring should describe the no-review case explicitly.

CopilotAI review requested due to automatic review settings August 18, 2026 00:16
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on 04f4bf7 reported Suppressed comments (1).

  • scripts/pr_review.py:19: "The status subcommand docstring says exit 0 implies 'the round covering the head read the whole diff', but report_verdict() returns 0 early when there are no head-covering reviews."
    • Fixed in ced266c - The CLI contract explicitly distinguishes the no-review success case and directs callers that require review presence to wait. A contract test protects the documented behavior.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 05c101e into developAug 18, 2026
8 checks passed
@ptr727
ptr727 deleted the feature/issue-793-copilot-review branch August 18, 2026 00:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Refactor Copilot Review Instructions and Skills by ptr727 · Pull Request #799 · ptr727/ProjectTemplate · GitHub
Skip to content

Refactor Copilot Review Instructions and Skills - #799

Merged
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review
Aug 18, 2026
Merged

Refactor Copilot Review Instructions and Skills#799
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

  • add a provider-independent code-review skill that routes changed files to the applicable language, documentation, and workflow skills
  • reduce the Copilot instruction file to a review bootstrap while preserving its repository-specific disproved-claims ledger
  • require visible findings and a machine-readable coverage marker
  • make pr_review.py parse the marker and block unstated coverage with exit 45

Root Cause

The Copilot instruction file duplicated provider-independent rules and embedded API mechanics already implemented by scripts/pr_review.py. Copilot review output also had no stable machine-readable coverage contract, while an unstated coverage result did not produce a blocking exit code.

Validation

  • uvx ruff@latest check .
  • uvx ruff@latest format --check .
  • uvx mypy@latest
  • full coverage and self-test sequence from OPERATIONS.md (79% total coverage, 97% for scripts/pr_review.py)
  • python3 scripts/build_dist.py --check
  • python3 scripts/repo_gate.py
  • complete prose, JSON, spec, EditorConfig, shellcheck, PSScriptAnalyzer, and Markdown gates
  • 688 script tests and 45 agent-safety install tests

Follow-Up

Closes#793

CopilotAI lite review requested due to automatic review settings August 17, 2026 23:20

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the GitHub Copilot review bootstrap to route through a provider-independent code-review skill, and updates scripts/pr_review.py + tests to accept a stable machine-readable coverage marker and to block “unstated coverage” with a dedicated exit code.

Changes:

  • Introduce a new code-review skill (in both .agents/skills and the Claude plugin distribution) that standardizes review procedure and the structured coverage marker.
  • Slim .github/copilot-instructions.md down to a bootstrap + output contract while preserving the repository’s disproved-claims ledger.
  • Extend scripts/pr_review.py and its tests to parse <!-- fleet-review: ... --> and to return exit 45 when coverage is unstated.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
scripts/tests/test_pr_review.pyUpdates corpus/status tests to cover the new structured marker and new exit-45 behavior.
scripts/pr_review.pyAdds marker parsing and blocks unstated coverage with exit 45.
AGENTS.mdRoutes “reviewing a change set” to the new code-review skill.
.github/copilot-instructions.mdReduces to review bootstrap/runbook + preserves disproved-claims ledger.
.agents/skills/code-review/SKILL.mdAdds provider-independent review skill and defines the structured coverage marker contract.
.agents/skills/pr-review-conduct/SKILL.mdUpdates “mechanics live elsewhere” guidance to point to scripts/pr_review.py + bootstrap.
.agents/skills/copilot-instructions-keeper/SKILL.mdAdjusts description to reflect bootstrap role of Copilot instructions.
.claude-plugin/fleet-skills/skills/code-review/SKILL.mdDistributes the new code-review skill into the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.mdMirrors the same “mechanics live elsewhere” update for the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/copilot-instructions-keeper/SKILL.mdMirrors the bootstrap-description update for the Claude plugin skill set.
.claude-plugin/fleet-skills/.source-digestUpdates the generated skill-set digest.
.claude-plugin/fleet-skills/.claude-plugin/plugin.jsonRegisters the new code-review skill in the plugin manifest.
Suppressed comments (1)

scripts/tests/test_pr_review.py:911

  • The docstring for this test still says that treating a no-coverage body as a failure would “cry wolf”, but scripts/pr_review.py status now blocks on coverage=unstated with exit 45. Update the docstring to reflect that this body shape is recognized as coverage=unstated (not UNVETTED), but it is still a blocking outcome for status because it provides no proof of full diff coverage.
 def test_a_round_stating_no_coverage_at_all_is_unstated(self) -> None:
"""28 of the 332 bodies are an overview and a change list, and that shape is current.
It interleaves with the counted one throughout rather than preceding it, and one pull
request carries both across its two rounds, so failing on it would cry wolf on about one

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadscripts/pr_review.py
CopilotAI review requested due to automatic review settings August 17, 2026 23:28
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Copilot review round 4955381803 reported Suppressed comments (1).

scripts/tests/test_pr_review.py:911: The docstring still says that treating a no-coverage body as a failure would "cry wolf", while unstated coverage now blocks with exit 45.

Fixed in f1c24d7. The test now states that this is a recognized coverage=unstated shape, but it blocks because it cannot prove full diff coverage.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings August 17, 2026 23:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/tests/test_pr_review.py:2844

  • This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md. Reading the generated .github/skills/ file here would better validate the actual Copilot-facing contract (and would fail if the generated tree wasn’t regenerated/committed).
    .github/skills/code-review/SKILL.md:17
  • The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/. In downstream repos (where .agents/skills/ is not carried), this instruction is impossible to follow and will prevent the intended routing to the per-language/workflow skills.
3. Load every applicable skill from `.agents/skills/`:

CopilotAI review requested due to automatic review settings August 18, 2026 00:02
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding responses

Review round on e1396f8 reported Suppressed comments (1).

  • scripts/tests/test_pr_review.py:911: "The docstring for this test still says that treating a no-coverage body as a failure would 'cry wolf', but scripts/pr_review.py status now blocks on coverage=unstated with exit 45."
    • Fixed in f1c24d7 - The fixture and its documentation state that unstated coverage is recognized but blocks the gate. A regression test asserts exit 45 for that state.

Review round on da7249f reported Suppressed comments (2).

  • scripts/tests/test_pr_review.py:2844: "This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md."
    • Fixed in c3738e5 - The contract test reads the generated Copilot-facing skill.
  • .github/skills/code-review/SKILL.md:17: "The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/."
    • Fixed in c3738e5 - The canonical skill now routes to applicable sibling skills in the current distribution. Regeneration applies that provider-neutral instruction to both generated trees.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/build_dist.py:180

  • is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root (for example an accidental hand-added README.md or .DS_Store) would not be detected as drift and build_dist.py --check could still pass. Since the docs and help text describe --check as failing whenever a generated distribution differs from .agents/skills/, it should also treat any non-directory entry under the distribution root as stale (and ideally require the root directory to exist even when there are zero skills).

CopilotAI review requested due to automatic review settings August 18, 2026 00:09
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on c3738e5 reported Suppressed comments (1).

  • scripts/build_dist.py:180: "is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root ... would not be detected as drift."
    • Fixed in 04f4bf7 - Both generated skill roots must exist and contain only the expected skill directories. Three regression tests cover a stray GitHub file, a stray Claude plugin skill file, and a missing empty GitHub distribution.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/pr_review.py:19

  • The status subcommand docstring says exit 0 implies “the round covering the head read the whole diff”, but report_verdict() returns 0 early when there are no head-covering reviews (if not head_reviews(pr): return 0). This makes exit 0 ambiguous (no review yet vs full coverage), so the docstring should describe the no-review case explicitly.

CopilotAI review requested due to automatic review settings August 18, 2026 00:16
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on 04f4bf7 reported Suppressed comments (1).

  • scripts/pr_review.py:19: "The status subcommand docstring says exit 0 implies 'the round covering the head read the whole diff', but report_verdict() returns 0 early when there are no head-covering reviews."
    • Fixed in ced266c - The CLI contract explicitly distinguishes the no-review success case and directs callers that require review presence to wait. A contract test protects the documented behavior.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 05c101e into developAug 18, 2026
8 checks passed
@ptr727
ptr727 deleted the feature/issue-793-copilot-review branch August 18, 2026 00:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Refactor Copilot Review Instructions and Skills by ptr727 · Pull Request #799 · ptr727/ProjectTemplate · GitHub
Skip to content

Refactor Copilot Review Instructions and Skills - #799

Merged
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review
Aug 18, 2026
Merged

Refactor Copilot Review Instructions and Skills#799
ptr727 merged 6 commits into
developfrom
feature/issue-793-copilot-review

Conversation

@ptr727

Copy link
Copy Markdown
Owner

Summary

  • add a provider-independent code-review skill that routes changed files to the applicable language, documentation, and workflow skills
  • reduce the Copilot instruction file to a review bootstrap while preserving its repository-specific disproved-claims ledger
  • require visible findings and a machine-readable coverage marker
  • make pr_review.py parse the marker and block unstated coverage with exit 45

Root Cause

The Copilot instruction file duplicated provider-independent rules and embedded API mechanics already implemented by scripts/pr_review.py. Copilot review output also had no stable machine-readable coverage contract, while an unstated coverage result did not produce a blocking exit code.

Validation

  • uvx ruff@latest check .
  • uvx ruff@latest format --check .
  • uvx mypy@latest
  • full coverage and self-test sequence from OPERATIONS.md (79% total coverage, 97% for scripts/pr_review.py)
  • python3 scripts/build_dist.py --check
  • python3 scripts/repo_gate.py
  • complete prose, JSON, spec, EditorConfig, shellcheck, PSScriptAnalyzer, and Markdown gates
  • 688 script tests and 45 agent-safety install tests

Follow-Up

Closes#793

CopilotAI lite review requested due to automatic review settings August 17, 2026 23:20

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the GitHub Copilot review bootstrap to route through a provider-independent code-review skill, and updates scripts/pr_review.py + tests to accept a stable machine-readable coverage marker and to block “unstated coverage” with a dedicated exit code.

Changes:

  • Introduce a new code-review skill (in both .agents/skills and the Claude plugin distribution) that standardizes review procedure and the structured coverage marker.
  • Slim .github/copilot-instructions.md down to a bootstrap + output contract while preserving the repository’s disproved-claims ledger.
  • Extend scripts/pr_review.py and its tests to parse <!-- fleet-review: ... --> and to return exit 45 when coverage is unstated.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
scripts/tests/test_pr_review.pyUpdates corpus/status tests to cover the new structured marker and new exit-45 behavior.
scripts/pr_review.pyAdds marker parsing and blocks unstated coverage with exit 45.
AGENTS.mdRoutes “reviewing a change set” to the new code-review skill.
.github/copilot-instructions.mdReduces to review bootstrap/runbook + preserves disproved-claims ledger.
.agents/skills/code-review/SKILL.mdAdds provider-independent review skill and defines the structured coverage marker contract.
.agents/skills/pr-review-conduct/SKILL.mdUpdates “mechanics live elsewhere” guidance to point to scripts/pr_review.py + bootstrap.
.agents/skills/copilot-instructions-keeper/SKILL.mdAdjusts description to reflect bootstrap role of Copilot instructions.
.claude-plugin/fleet-skills/skills/code-review/SKILL.mdDistributes the new code-review skill into the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/pr-review-conduct/SKILL.mdMirrors the same “mechanics live elsewhere” update for the Claude plugin skill set.
.claude-plugin/fleet-skills/skills/copilot-instructions-keeper/SKILL.mdMirrors the bootstrap-description update for the Claude plugin skill set.
.claude-plugin/fleet-skills/.source-digestUpdates the generated skill-set digest.
.claude-plugin/fleet-skills/.claude-plugin/plugin.jsonRegisters the new code-review skill in the plugin manifest.
Suppressed comments (1)

scripts/tests/test_pr_review.py:911

  • The docstring for this test still says that treating a no-coverage body as a failure would “cry wolf”, but scripts/pr_review.py status now blocks on coverage=unstated with exit 45. Update the docstring to reflect that this body shape is recognized as coverage=unstated (not UNVETTED), but it is still a blocking outcome for status because it provides no proof of full diff coverage.
 def test_a_round_stating_no_coverage_at_all_is_unstated(self) -> None:
"""28 of the 332 bodies are an overview and a change list, and that shape is current.
It interleaves with the counted one throughout rather than preceding it, and one pull
request carries both across its two rounds, so failing on it would cry wolf on about one

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadscripts/pr_review.py
CopilotAI review requested due to automatic review settings August 17, 2026 23:28
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Copilot review round 4955381803 reported Suppressed comments (1).

scripts/tests/test_pr_review.py:911: The docstring still says that treating a no-coverage body as a failure would "cry wolf", while unstated coverage now blocks with exit 45.

Fixed in f1c24d7. The test now states that this is a recognized coverage=unstated shape, but it blocks because it cannot prove full diff coverage.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

CopilotAI review requested due to automatic review settings August 17, 2026 23:55

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/tests/test_pr_review.py:2844

  • This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md. Reading the generated .github/skills/ file here would better validate the actual Copilot-facing contract (and would fail if the generated tree wasn’t regenerated/committed).
    .github/skills/code-review/SKILL.md:17
  • The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/. In downstream repos (where .agents/skills/ is not carried), this instruction is impossible to follow and will prevent the intended routing to the per-language/workflow skills.
3. Load every applicable skill from `.agents/skills/`:

CopilotAI review requested due to automatic review settings August 18, 2026 00:02
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Suppressed finding responses

Review round on e1396f8 reported Suppressed comments (1).

  • scripts/tests/test_pr_review.py:911: "The docstring for this test still says that treating a no-coverage body as a failure would 'cry wolf', but scripts/pr_review.py status now blocks on coverage=unstated with exit 45."
    • Fixed in f1c24d7 - The fixture and its documentation state that unstated coverage is recognized but blocks the gate. A regression test asserts exit 45 for that state.

Review round on da7249f reported Suppressed comments (2).

  • scripts/tests/test_pr_review.py:2844: "This contract test reads the marker from the .agents/skills/ copy, but the runbook explicitly points Copilot at .github/skills/code-review/SKILL.md."
    • Fixed in c3738e5 - The contract test reads the generated Copilot-facing skill.
  • .github/skills/code-review/SKILL.md:17: "The GitHub Copilot skill lives under .github/skills/, but this step tells the reviewer to load skills from .agents/skills/."
    • Fixed in c3738e5 - The canonical skill now routes to applicable sibling skills in the current distribution. Regeneration applies that provider-neutral instruction to both generated trees.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/build_dist.py:180

  • is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root (for example an accidental hand-added README.md or .DS_Store) would not be detected as drift and build_dist.py --check could still pass. Since the docs and help text describe --check as failing whenever a generated distribution differs from .agents/skills/, it should also treat any non-directory entry under the distribution root as stale (and ideally require the root directory to exist even when there are zero skills).

CopilotAI review requested due to automatic review settings August 18, 2026 00:09
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on c3738e5 reported Suppressed comments (1).

  • scripts/build_dist.py:180: "is_stale() only considers directory entries when validating .github/skills/ (and the plugin's skills/), so a stray file at the distribution root ... would not be detected as drift."
    • Fixed in 04f4bf7 - Both generated skill roots must exist and contain only the expected skill directories. Three regression tests cover a stray GitHub file, a stray Claude plugin skill file, and a missing empty GitHub distribution.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/pr_review.py:19

  • The status subcommand docstring says exit 0 implies “the round covering the head read the whole diff”, but report_verdict() returns 0 early when there are no head-covering reviews (if not head_reviews(pr): return 0). This makes exit 0 ambiguous (no review yet vs full coverage), so the docstring should describe the no-review case explicitly.

CopilotAI review requested due to automatic review settings August 18, 2026 00:16
@ptr727

Copy link
Copy Markdown
OwnerAuthor

Review round on 04f4bf7 reported Suppressed comments (1).

  • scripts/pr_review.py:19: "The status subcommand docstring says exit 0 implies 'the round covering the head read the whole diff', but report_verdict() returns 0 early when there are no head-covering reviews."
    • Fixed in ced266c - The CLI contract explicitly distinguishes the no-review success case and directs callers that require review presence to wait. A contract test protects the documented behavior.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 53 out of 53 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 05c101e into developAug 18, 2026
8 checks passed
@ptr727
ptr727 deleted the feature/issue-793-copilot-review branch August 18, 2026 00:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ptr727