Address Promotion Review Findings

docs/homelab-runner-selection-plan.md

@@ -82,11 +82,11 @@ Ordinary fleet CI and Copilot code review therefore have separate runner policie
 | Fleet validation jobs | Explicitly select GitHub-hosted or homelab |
 | Fleet selector job | Always GitHub-hosted |
 | Copilot code review | Always GitHub-hosted |
-| Copilot cloud agent | Unchanged |
+| Copilot cloud agent | Standard GitHub-hosted runner |
 
 The fleet runner variable never reaches Copilot configuration. Homelab labels never appear in the Copilot workflow.
 
-The organization-level Copilot runner type selects a standard GitHub-hosted runner. Repository customization of the Copilot runner type is disabled.
+The organization-level Copilot runner type applies to both code review and the cloud agent. It selects a standard GitHub-hosted runner for both workloads. Repository customization of the Copilot runner type is disabled. Before rollout, verify these effective organization settings rather than relying on the planned values.
 
 The proposed implementation adds a dedicated `.github/workflows/copilot-code-review.yml`. It pins `copilot-setup-steps` to `ubuntu-24.04` and begins with a step that fails unless `runner.environment` is `github-hosted`.
 
@@ -123,7 +123,7 @@ GitHub-hosted output:
 Homelab output:
 
 ```json
-{"group":"homelab","labels":["self-hosted","linux","x64","homelab","ubuntu-24.04"]}
+{"group":"homelab","labels":["self-hosted","linux","x64","homelab","homelab-ubuntu-24.04"]}
 ```
 
 Dependent jobs pass the output through `fromJSON` in `runs-on`. No caller copies the authorization expression.
@@ -248,6 +248,7 @@ Static workflow verification also covers action pinning, actionlint custom label
 
 Live verification covers both selection branches. Only a live run can prove runner registration and Copilot integration.
 Before the runner-group checkpoint is complete, capture live API evidence for the exact runner group, `visibility: selected`, a selected repository set exactly equal to `ptr727/ProjectTemplate`, `restricted_to_workflows: true`, a `selected_workflows` set containing only the approved workflow path pinned to the protected ref, and `allows_public_repositories` equal to the documented opt-in. Reject extra or missing selected repositories and workflows. Assert the restriction flag and selected-workflow value together because GitHub ignores the value when the flag is false. Run negative canaries that prove direct-label and unapproved-ref requests cannot reach the group. Link provisioning evidence from the owning CloudInit repository rather than inferring it from this plan.
+Run a live negative canary that proves the GitHub-hosted output cannot select a homelab runner.
 
 ## Planned Repository Surfaces
 
@@ -322,6 +323,7 @@ Implementation waits for an explicit decision on each item.
 - [ ] Restricted organization runner group and runner registered.
 - [ ] Live runner-group API evidence asserts `visibility: selected`, a repository set containing only `ptr727/ProjectTemplate`, `restricted_to_workflows: true`, a `selected_workflows` set containing only the approved workflow path pinned to the protected ref, and the documented public-repository setting.
 - [ ] Direct-label and unapproved-ref negative canaries are rejected.
+- [ ] The GitHub-hosted output cannot select a homelab runner in a live negative canary.
 - [ ] CloudInit provisioning evidence is linked from its owning repository.
 - [ ] Protected-ref homelab canary succeeds.
 - [ ] Fork or external path stays GitHub-hosted.

docs/pr-reviewer-evaluation.md

@@ -19,7 +19,7 @@ This document measures whether additional automated reviewers improve the fleet'
 **State:** Active evaluation\
 **Incumbent:** GitHub Copilot\
 **Candidates:** CodeRabbit and Qodo\
-**Samples:** [ProjectTemplate pull request #891][pr-891] and [pull request #892][pr-892]
+**Samples:** [ProjectTemplate pull request #891][pr-891], [pull request #892][pr-892], and [pull request #893][pr-893]
 
 No candidate is a required reviewer. A candidate remains advisory until it meets the first-class support criteria below.
 
@@ -54,8 +54,8 @@ The first sample is too small for an adoption decision. It does show that both c
 | Reviewer | True Positive | Mixed | False Positive | Duplicate Roots | Initial Reading |
 | --- | ---: | ---: | ---: | ---: | --- |
 | GitHub Copilot | Not scored | Not scored | Not scored | Not scored | At least seven review attempts across two pull requests ended in an error, so no review covered any reviewed head |
-| CodeRabbit | 16 | 2 | 0 | 3 | Strong issue discovery, with occasional remedies that overstate configuration scope or contradict another valid control |
-| Qodo | 3 | 1 | 4 | 2 | Finds additional reliability issues, but repeatedly misreads repository title rules and documented tool behavior |
+| CodeRabbit | 25 | 2 | 0 | 3 | Strong issue discovery, with occasional remedies that overstate configuration scope or contradict another valid control |
+| Qodo | 4 | 0 | 4 | 2 | Finds additional reliability issues, but repeatedly misreads repository title rules and documented tool behavior |
 
 The duplicate roots were target-discovery failure, markdownlint filename handling, and process-failure classification. Qodo alone raised command-line length limits. CodeRabbit alone covered the postponed runner design, Docker mount quoting, and the editor-extension identifier.
 
@@ -73,13 +73,14 @@ CodeRabbit posted nine actionable findings:
 
 Qodo posted five findings:
 
-- Two true positives: target-discovery failure and command-line length limits.
-- One mixed finding: markdownlint needs literal filename handling, but the comment did not establish its broader claim for every linter.
+- Three true positives: target-discovery failure, command-line length limits, and option termination for every file-argument linter.
 - Two false positives: `to` is an allowed lowercase title bind word, and the Docker-lint documentation follows the intended skill and runbook ownership model.
 - Two findings duplicated roots CodeRabbit also reported.
 
 Copilot posted three terminal error responses and no findings. This sample therefore measures candidate value during an incumbent outage, not comparative recall over the same completed review.
 
+After `develop` advanced, CodeRabbit added two true positives: the organization Copilot runner policy also governs the cloud agent, and homelab labels must not overlap the hosted selector. Qodo added no finding.
+
 ### 2026-08-21: Pull Request #892
 
 Qodo posted three findings:
@@ -101,8 +102,18 @@ After the corrective push, CodeRabbit completed an explicitly triggered incremen
 
 Successive documentation-only follow-ups found two true positives: a compound-modifier error and inconsistent reviewer-attempt counts. The later comment that identified the resulting log-total mismatch repeated the count-reconciliation root. Qodo added no finding.
 
+Later follow-ups found six more true positives: upstream draft handoff, live runner authorization evidence, duplicate-root bookkeeping, the active workflow-restriction flag, the exact repository set, and the exact workflow set.
+
 Copilot posted terminal error responses on at least four successive pull request heads and supplied no review coverage.
 
+### 2026-08-21: Pull Request #893
+
+All three reviewers reported no findings. Qodo completed in 7 seconds, Copilot completed in 1 minute 55 seconds, and an explicitly triggered CodeRabbit review completed in 4 minutes 36 seconds.
+
+Copilot reviewed 2/2 changed files at Lite effort. Its new `Approval recommended` heading was not recognized by `pr_review.py`, so the otherwise clean review remained blocked. [Issue #894][issue-894] records the shape, and the pull request adds a regression fixture for it.
+
+On the parser-fix head, Copilot reviewed 4/4 files with no findings and the updated script recognized its output. CodeRabbit found one true-positive stale corpus count. Qodo did not reprocess the second push automatically. Its documented `/review` command updated the existing review through the final head in 3 minutes 10 seconds and added no finding.
+
 ## Interaction and Operations
 
 ### GitHub Copilot
@@ -191,6 +202,11 @@ The existing Copilot adapter remains behaviorally unchanged during extraction. P
 
 [pr-891]: https://github.com/ptr727/ProjectTemplate/pull/891
 [pr-892]: https://github.com/ptr727/ProjectTemplate/pull/892
+[pr-893]: https://github.com/ptr727/ProjectTemplate/pull/893
+
+<!-- Issues -->
+
+[issue-894]: https://github.com/ptr727/ProjectTemplate/issues/894
 
 <!-- External -->
 

scripts/docker_lint.py

@@ -240,8 +240,7 @@ def container_command(root: Path, linter: Linter, digest: str, files: Sequence[s
     else:
         command.extend([digest, *linter.arguments])
         if linter.name in {"markdownlint", "cspell", "shellcheck"}:
-            if linter.name in {"markdownlint", "shellcheck"}:
-                command.append("--")
+            command.append("--")
             command.extend(files)
     return command
 

scripts/pr_review.py

@@ -133,7 +133,7 @@
 # This is the third instance of the shape the two patterns above answer.
 # It is also the only one nothing was reading.
 # The line is anchored at its start rather than matched body-wide, both spellings being structural.
-# Over 332 review bodies every coverage statement opens its line and not one sits mid-sentence.
+# Over 333 review bodies every coverage statement opens its line and not one sits mid-sentence.
 # 272 of them open with the reviewer's own name and 32 are the `Review details` bullet.
 # A body-wide match reports the pull request adding this check as a partial round.
 # That is the false positive the suppressed matcher and the refusal matcher have each had once.
@@ -192,17 +192,18 @@
 # The constant carries that name too, so no reader has to map a name here onto another word.
 COVERAGE_FIELD = {UNVETTED: "UNVETTED", PARTIAL: "PARTIAL", FULL: "full", UNSTATED: "unstated"}
 
-# Every structural marker the reviewer's own bodies carry, measured over the same 332.
+# Every structural marker the reviewer's own bodies carry, measured over the same 333.
 # A body is read for these rather than trusted, because every reader below keys on one of them.
 # A heading this script has no spelling for is a section it will not find, reported as absent.
 # That is the shape of all three failures already on record here, each caught after it landed.
-# The lists are small because the output is regular: 7 headings, 6 summaries and 3 labels.
+# The lists are small because the output is regular: 8 headings, 6 summaries and 3 labels.
 # Counts are normalized to `(N)` and non-ASCII is dropped before comparing.
 # The verdict headings carry a colored circle, so the emoji is what would drift most cheaply.
 # Dropping it also keeps this file inside the charset rule that governs the repository.
 VETTED_HEADINGS = {
     "## Pull request overview",
     "### Reviewed changes",
+    "### Approval recommended",
     "### Ready to approve",
     "### Changes recommended",
     "### Not ready to approve",
@@ -652,7 +653,7 @@ def read_coverage(line: str) -> tuple[int, int] | None:
 def coverage_of(node: dict) -> tuple[str, str]:
     """This round's coverage reading, with the line it was read from.
 
-    A round making no statement at all reads as unstated rather than unvetted. 28 of the 332
+    A round making no statement at all reads as unstated rather than unvetted. 28 of the 333
     bodies measured carry an overview and a change list and nothing more. The shape is recognized,
     but it cannot prove full diff coverage and therefore blocks the status gate.
 
@@ -812,7 +813,7 @@ def unrecognized_in(body: str) -> list[str]:
     quoting a heading is not a review carrying one, and this script's own pull requests quote
     these lists in full.
 
-    A body carrying no heading at all is reported rather than passed, since every one of the 332
+    A body carrying no heading at all is reported rather than passed, since every one of the 333
     measured opens on a heading and a body with none is a format nothing here has seen. A refusal
     is the exemption, being a bare paragraph by design and already classified as one.
     """

scripts/tests/test_docker_lint.py

@@ -129,6 +129,13 @@ def test_shellcheck_literal_marker_precedes_option_shaped_filename(self) -> None
         )
         self.assertEqual(["--", "-release.sh"], command[-2:])
 
+    def test_cspell_literal_marker_precedes_option_shaped_filename(self) -> None:
+        linter = next(linter for linter in docker_lint.LINTERS if linter.name == "cspell")
+        command = docker_lint.container_command(
+            self.root, linter, "example@sha256:123", ["-release.md"]
+        )
+        self.assertEqual(["--", "-release.md"], command[-2:])
+
     def test_invalid_git_root_reports_failed_result(self) -> None:
         root = Path(self.enterContext(tempfile.TemporaryDirectory()))
         output = io.StringIO()

scripts/tests/test_pr_review.py

@@ -37,7 +37,7 @@
 LATE = "2026-08-02T11:00:00Z"
 
 
-# The shape 28 of the 332 measured bodies carry: an overview, and no count of what was read.
+# The shape 28 of the 333 measured bodies carry: an overview, and no count of what was read.
 # A body of no text at all is not one of the shapes, and the reader now says so, correctly.
 OVERVIEW = "## Pull request overview\n\nThe change is narrow.\n"
 
@@ -777,7 +777,7 @@ def test_a_human_review_carrying_the_wording_is_not_the_reviewer_refusing(self)
 class TestCoverage(GqlCase):
     """The round that covered the head and read part of the diff, which is a clean pass elsewhere.
 
-    Measured over 332 Copilot review bodies on this repository: five rounds across three pull
+    Measured over 333 Copilot review bodies on this repository: five rounds across three pull
     requests reported reading fewer files than the pull request changed, and all three merged.
     One of them changed three files, left one unread across both its rounds, and reported
     "generated no comments" each time.
@@ -901,7 +901,7 @@ def test_a_coverage_line_that_parses_to_nothing_names_this_script(self) -> None:
         self.assertIn(f"coverage line: {line}", out)
 
     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.
+        """28 of the 333 bodies are an overview and a change list, and that shape is current.
 
         It is a recognized shape rather than unvetted reviewer output, but it cannot prove that
         the reviewer covered the full diff and therefore blocks the status gate.
@@ -986,7 +986,7 @@ def test_prose_mentioning_changed_files_is_not_this_round_stating_its_coverage(s
         self.assertIn("coverage=unstated", out)
 
     def test_a_quoted_line_in_a_fenced_block_is_not_this_round_stating_its_coverage(self) -> None:
-        """131 of the 332 bodies carry a fence, and this change puts both spellings in the diff."""
+        """131 of the 333 bodies carry a fence, and this change puts both spellings in the diff."""
         body = (
             "### Ready to approve\n\nThe vetted spellings read:\n\n```\n"
             "Copilot reviewed 2 out of 3 changed files in this pull request and generated "
@@ -1016,8 +1016,8 @@ class TestUnrecognizedShapes(GqlCase):
     a suppressed heading reworded, a suppressed section moved inside another wrapper, and a
     coverage line nothing parsed. Each was caught after it had already reported a clean pass.
 
-    The inventory is measured rather than imagined. Over the same 332 review bodies, with fenced
-    blocks removed and text reduced to ASCII, the whole corpus is 7 headings, 6 summaries and 3
+    The inventory is measured rather than imagined. Over the same 333 review bodies, with fenced
+    blocks removed and text reduced to ASCII, the whole corpus is 8 headings, 6 summaries and 3
     metadata labels, and every body carries at least one of them.
     """
 
@@ -1038,6 +1038,11 @@ def test_a_heading_that_is_not_in_the_inventory_blocks(self) -> None:
         self.assertIn("shapes=UNRECOGNIZED", out)
         self.assertIn("heading: ### Confidence assessment", out)
 
+    def test_approval_recommended_heading_is_vetted(self) -> None:
+        """The no-findings verdict introduced by the current Copilot review stays readable."""
+        body = "### \U0001f7e2 Approval recommended\n\nDocumentation updates are consistent.\n"
+        self.assertEqual([], pr_review.unrecognized_in(body))
+
     def test_a_details_summary_that_is_not_in_the_inventory_blocks(self) -> None:
         """The suppressed section has already moved between wrappers once."""
         body = OVERVIEW + "\n<details>\n<summary>Withheld findings</summary>\n\nx\n</details>\n"