Scaled-Tech-Consulting/Gitea-Tools
2
0
Fork 0
Code Issues 73 Pull Requests 9 Actions Packages Projects Releases Wiki Activity

Harden blind PR queue review workflow: require checked-out pinned head, complete inventory, and evidence-backed eligibility #173

New Issue
Closed
opened 2026-07-05 13:14:12 -05:00 by jcwalker3 · 6 comments
jcwalker3 commented 2026-07-05 13:14:12 -05:00
Owner
Copy Link
Copy Source

Problem:
A blind PR queue review run successfully used the reviewer namespace and merged Gitea-Tools PR #172, but the process only deserves a C+/B- because several review-quality gaps remain.

Observed gaps:

  1. Pinned SHA vs local HEAD was not proven.
    The agent fetched PR #172 and pinned FETCH_HEAD as 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9, but the command log does not clearly show that the local checkout HEAD was switched to that SHA before diff review and validation. It later ran diffs/tests against HEAD, which may have been the wrong branch.

  2. Open PR inventory may have been incomplete.
    The report listed only:

    But recent operator context expected additional open PRs unless already merged/closed. The report did not prove pagination, filters, repo coverage, or that only one open PR existed per repo.

  3. Test reporting was not strong enough.
    The default pytest collection failed due to duplicate import/worktree issues under branches/. The agent then used venv/bin/pytest tests/ --ignore=branches, but did not provide a clear final pass count or document why that command is the canonical/acceptable validation command.

  4. Self-review contamination claim was not evidence-backed.
    The agent marked mcp-control-plane PR #73 ineligible because it was supposedly authored in the same session under prgs-author. That may be true, but the report did not show enough evidence to distinguish:

    • PR authored by jcwalker3, which is normally eligible for sysadmin / prgs-reviewer
    • PR touched/authored by the same agent/session, which is self-review contaminated

  5. Background/wait behavior was messy.
    The run used scheduled/background-style waiting while trying to complete validation. The final report should only claim validation after the command has completed and the output has been read.

Required behavior:

  1. Before reviewing or validating a selected PR, the workflow must explicitly prove:

    • selected PR head SHA from Gitea
    • local checkout SHA
    • HEAD == selected PR head SHA
    • diff base is the PR base branch

  2. If HEAD does not match the pinned PR head, stop before review/merge.

  3. Queue inventory must prove completeness:

    • both configured repositories checked
    • open PR filters stated
    • pagination handled or explicitly not needed
    • total open PR count per repo reported
    • no "only PRs found" claim unless listing was actually complete

  4. Validation reporting must include exact commands and exact results:

    • pass/fail
    • count of tests passed/skipped/failed, where available
    • any ignored paths and why they are safe to ignore
    • whether the command differs from the repository's canonical validation command

  5. Self-review contamination must be evidence-backed:

    • current authenticated reviewer identity
    • PR author identity
    • whether this current session authored/touched the PR branch
    • evidence source for any "same-session author" claim
    • if evidence is missing, do not declare self-review contamination; report it as unknown and choose another PR or stop.

  6. Final reports must distinguish:

    • identity eligible
    • PR author different from reviewer
    • session contamination absent
    • validation performed on pinned head
    • merge performed
    • issue status verified

  7. Add tests or harness assertions for:

    • fetched SHA but unchecked-out HEAD must block merge
    • complete queue inventory with multiple PRs across repos
    • pagination or multi-page PR listing
    • validation result missing test counts is flagged as weak/incomplete
    • self-review contamination requires evidence, not assumption
    • final report cannot say validation passed unless command output was read
    • ignored paths in validation must be explicitly justified

Acceptance criteria:

  • A blind queue review earns an "A" only if it proves complete inventory, selects an eligible PR, checks out the exact pinned head, validates that exact head, gives evidence-backed self-review reasoning, and reports exact validation results.
  • No review/merge safety gate is weakened.
  • If any proof is missing, the workflow must stop or downgrade the result instead of merging confidently.

Related:

  • mcp-control-plane #68 (pinned-head proof) and #76 walls 4/8 (pinned-head/stale-state, final-report validation) specify the MCP-level gates; this issue covers the Gitea-Tools-side workflow, tests, and harness assertions.
  • mcp-control-plane #77 covers the author-side analogues (exact capability, complete inventory, claim semantics); this issue is the reviewer-side counterpart with checkout-proof and eligibility-evidence requirements.
Problem: A blind PR queue review run successfully used the reviewer namespace and merged Gitea-Tools PR #172, but the process only deserves a C+/B- because several review-quality gaps remain. Observed gaps: 1. Pinned SHA vs local HEAD was not proven. The agent fetched PR #172 and pinned `FETCH_HEAD` as `0fdc8f582026b72a229d59a172c0a63ac4aaeaf9`, but the command log does not clearly show that the local checkout `HEAD` was switched to that SHA before diff review and validation. It later ran diffs/tests against `HEAD`, which may have been the wrong branch. 2. Open PR inventory may have been incomplete. The report listed only: * Gitea-Tools #172 * mcp-control-plane #73 But recent operator context expected additional open PRs unless already merged/closed. The report did not prove pagination, filters, repo coverage, or that only one open PR existed per repo. 3. Test reporting was not strong enough. The default pytest collection failed due to duplicate import/worktree issues under `branches/`. The agent then used `venv/bin/pytest tests/ --ignore=branches`, but did not provide a clear final pass count or document why that command is the canonical/acceptable validation command. 4. Self-review contamination claim was not evidence-backed. The agent marked mcp-control-plane PR #73 ineligible because it was supposedly authored in the same session under `prgs-author`. That may be true, but the report did not show enough evidence to distinguish: * PR authored by `jcwalker3`, which is normally eligible for `sysadmin / prgs-reviewer` * PR touched/authored by the same agent/session, which is self-review contaminated 5. Background/wait behavior was messy. The run used scheduled/background-style waiting while trying to complete validation. The final report should only claim validation after the command has completed and the output has been read. Required behavior: 1. Before reviewing or validating a selected PR, the workflow must explicitly prove: * selected PR head SHA from Gitea * local checkout SHA * `HEAD == selected PR head SHA` * diff base is the PR base branch 2. If `HEAD` does not match the pinned PR head, stop before review/merge. 3. Queue inventory must prove completeness: * both configured repositories checked * open PR filters stated * pagination handled or explicitly not needed * total open PR count per repo reported * no "only PRs found" claim unless listing was actually complete 4. Validation reporting must include exact commands and exact results: * pass/fail * count of tests passed/skipped/failed, where available * any ignored paths and why they are safe to ignore * whether the command differs from the repository's canonical validation command 5. Self-review contamination must be evidence-backed: * current authenticated reviewer identity * PR author identity * whether this current session authored/touched the PR branch * evidence source for any "same-session author" claim * if evidence is missing, do not declare self-review contamination; report it as unknown and choose another PR or stop. 6. Final reports must distinguish: * identity eligible * PR author different from reviewer * session contamination absent * validation performed on pinned head * merge performed * issue status verified 7. Add tests or harness assertions for: * fetched SHA but unchecked-out HEAD must block merge * complete queue inventory with multiple PRs across repos * pagination or multi-page PR listing * validation result missing test counts is flagged as weak/incomplete * self-review contamination requires evidence, not assumption * final report cannot say validation passed unless command output was read * ignored paths in validation must be explicitly justified Acceptance criteria: * A blind queue review earns an "A" only if it proves complete inventory, selects an eligible PR, checks out the exact pinned head, validates that exact head, gives evidence-backed self-review reasoning, and reports exact validation results. * No review/merge safety gate is weakened. * If any proof is missing, the workflow must stop or downgrade the result instead of merging confidently. Related: * mcp-control-plane #68 (pinned-head proof) and #76 walls 4/8 (pinned-head/stale-state, final-report validation) specify the MCP-level gates; this issue covers the Gitea-Tools-side workflow, tests, and harness assertions. * mcp-control-plane #77 covers the author-side analogues (exact capability, complete inventory, claim semantics); this issue is the reviewer-side counterpart with checkout-proof and eligibility-evidence requirements.
jcwalker3 added the status:in-progress label 2026-07-05 13:19:50 -05:00
jcwalker3 commented 2026-07-05 13:30:42 -05:00
Author
Owner
Copy Link
Copy Source

Additional evidence: Gemini review/merge of PR #174 (improved, still short of A)

Gemini reviewed and merged PR #174 as sysadmin / prgs-reviewer (merged 2026-07-05T13:28:03-05:00, merge commit ae7a39049553f219e466e6e08e38bed9894b9295). Improvement over the PR #172 run documented in this issue's body: it checked out the pinned PR head and claimed local HEAD matched the reviewed SHA before validation — addressing observed gap 1.

Remaining gaps in this run:

  1. Mixed gitea-author and gitea-reviewer namespaces during a reviewer task (same pattern as #175).
  2. Attempted gitea_activate_profile even though static-profile workflows should reconnect to the correct namespace instead of switching in-place.
  3. Queue inventory did not explicitly prove pagination/completeness (gap 2 of this issue, still unaddressed).
  4. Validation reporting did not provide one exact canonical full-suite aggregate count (gap 3, still unaddressed).
  5. Reviewed SHA 3ff3affa6435199c7d35aec836bce357fa72d4bf differed from the author-run-reported SHA 3ff3aff8a4020c6a51d95ee2be979c6b659c25f4; the reviewer did not explicitly reconcile handoff SHA vs live head SHA.

Operator-side reconciliation of item 5 (live check, read-only): git ls-remote shows refs/heads/feat/issue-171-redact-raw-urls = 3ff3affa6435199c7d35aec836bce357fa72d4bf — Gemini reviewed the true live head. The author-reported full SHA 3ff3aff8a4… does not exist on the remote; both share only the 7-char prefix 3ff3aff, so the author run appears to have misreported an expanded full SHA from a short one. Verdict: reviewer validated the right code, but only by luck of reviewing live state — a handoff-vs-live SHA reconciliation step would have caught and documented the author-side misreport instead of leaving it ambiguous. This incident is now the canonical motivating example for the reconciliation requirement below.

Scope addenda for this issue:

  • Reviewer reports must reconcile any mismatch between author/handoff-reported head SHA and live PR head SHA before merge; unexplained full-SHA discrepancies must be flagged even when prefixes match.
  • Static profile mode should discourage or block gitea_activate_profile, recommending reconnection to the correct namespace instead.
  • Reviewer tasks stay in gitea-reviewer unless author-namespace usage is explicitly justified in the report (role-boundary wall proper is #175).

Updated acceptance bar: a blind PR review earns an A only when it proves complete queue inventory, uses clean reviewer-only role boundaries, validates the exact pinned live PR head, reports exact aggregate test counts, and reconciles any handoff-vs-live SHA mismatch before merge.

## Additional evidence: Gemini review/merge of PR #174 (improved, still short of A) Gemini reviewed and merged PR #174 as `sysadmin / prgs-reviewer` (merged 2026-07-05T13:28:03-05:00, merge commit `ae7a39049553f219e466e6e08e38bed9894b9295`). Improvement over the PR #172 run documented in this issue's body: it checked out the pinned PR head and claimed local `HEAD` matched the reviewed SHA before validation — addressing observed gap 1. **Remaining gaps in this run:** 1. Mixed `gitea-author` and `gitea-reviewer` namespaces during a reviewer task (same pattern as #175). 2. Attempted `gitea_activate_profile` even though static-profile workflows should reconnect to the correct namespace instead of switching in-place. 3. Queue inventory did not explicitly prove pagination/completeness (gap 2 of this issue, still unaddressed). 4. Validation reporting did not provide one exact canonical full-suite aggregate count (gap 3, still unaddressed). 5. Reviewed SHA `3ff3affa6435199c7d35aec836bce357fa72d4bf` differed from the author-run-reported SHA `3ff3aff8a4020c6a51d95ee2be979c6b659c25f4`; the reviewer did not explicitly reconcile handoff SHA vs live head SHA. **Operator-side reconciliation of item 5 (live check, read-only):** `git ls-remote` shows `refs/heads/feat/issue-171-redact-raw-urls` = `3ff3affa6435199c7d35aec836bce357fa72d4bf` — Gemini reviewed the true live head. The author-reported full SHA `3ff3aff8a4…` does not exist on the remote; both share only the 7-char prefix `3ff3aff`, so the author run appears to have misreported an expanded full SHA from a short one. Verdict: reviewer validated the right code, but only by luck of reviewing live state — a handoff-vs-live SHA reconciliation step would have caught and documented the author-side misreport instead of leaving it ambiguous. This incident is now the canonical motivating example for the reconciliation requirement below. **Scope addenda for this issue:** * Reviewer reports must reconcile any mismatch between author/handoff-reported head SHA and live PR head SHA before merge; unexplained full-SHA discrepancies must be flagged even when prefixes match. * Static profile mode should discourage or block `gitea_activate_profile`, recommending reconnection to the correct namespace instead. * Reviewer tasks stay in `gitea-reviewer` unless author-namespace usage is explicitly justified in the report (role-boundary wall proper is #175). **Updated acceptance bar:** a blind PR review earns an A only when it proves complete queue inventory, uses clean reviewer-only role boundaries, validates the exact pinned live PR head, reports exact aggregate test counts, and reconciles any handoff-vs-live SHA mismatch before merge.
jcwalker3 commented 2026-07-05 13:37:34 -05:00
Author
Owner
Copy Link
Copy Source

Additional evidence: A- review/merge run (mcp-control-plane PR #73) — best run so far, four gaps from A

Third graded blind-review run in this record; grade trend C+/B- (PR #172) → B (PR #174) → A- (PR #73). Merge verified live: PR #73 merged 2026-07-05T13:32:50-05:00, merge commit 5e486652e192a6caa7e874353eae549f6ed865cc.

What went well (A- baseline behaviors, now the expected floor):

  • Correct reviewer identity: sysadmin / prgs-reviewer; no author-role pivot (contrast with the #175 evidence runs).
  • PR head pinned and local HEAD verified to match b5caab87ad7154748431729be4f4020dc08e843f — required behavior 1 of this issue, satisfied.
  • Full suite with exact aggregate count: 421 passed, 13 skipped — required behavior 4, satisfied.
  • Approve and merge through reviewer tools only.

Remaining gaps for a full A:

  1. Report claimed both repositories caught up but showed only mcp-control-plane inventory. Reports must list every configured repo with explicit open-PR counts, including 0 open (required behavior 3 — still the most common miss across all graded runs).
  2. Large/high-risk PR reviewed without per-area findings. Reports must summarize concrete review findings for controller safety walls, merge gates, cleanup gates, author/reviewer separation, and prior REQUEST_CHANGES fixes — "suite green" is not a review.
  3. Secret/provenance sweep reported as a summary without the exact command or method used.
  4. Prior REQUEST_CHANGES existed (at 6717c33e…) but the report did not list the old blockers and map each to its fix in the new diff.

Scope addendum for this issue: add to required reporting — when prior request-changes reviews exist, enumerate each prior blocker and the exact diff element resolving it; high-risk PRs require per-gate findings, not just aggregate test results.

Cross-reference: PR #73 merging activates the mcp-control-plane #76 decision rule — each acceptance-matrix wall must now be verified as implemented-with-tests or explicitly tracked out-of-scope. The PR body claims walls for identity, capability, checkout-proof, merge authorization, duplicate search, and report validation; #76 remains the open tracker for verifying those claims against the matrix.

## Additional evidence: A- review/merge run (mcp-control-plane PR #73) — best run so far, four gaps from A Third graded blind-review run in this record; grade trend C+/B- (PR #172) → B (PR #174) → **A- (PR #73)**. Merge verified live: PR #73 merged 2026-07-05T13:32:50-05:00, merge commit `5e486652e192a6caa7e874353eae549f6ed865cc`. **What went well (A- baseline behaviors, now the expected floor):** * Correct reviewer identity: `sysadmin / prgs-reviewer`; no author-role pivot (contrast with the #175 evidence runs). * PR head pinned and local `HEAD` verified to match `b5caab87ad7154748431729be4f4020dc08e843f` — required behavior 1 of this issue, satisfied. * Full suite with exact aggregate count: `421 passed, 13 skipped` — required behavior 4, satisfied. * Approve and merge through reviewer tools only. **Remaining gaps for a full A:** 1. Report claimed both repositories caught up but showed only mcp-control-plane inventory. Reports must list every configured repo with explicit open-PR counts, including `0 open` (required behavior 3 — still the most common miss across all graded runs). 2. Large/high-risk PR reviewed without per-area findings. Reports must summarize concrete review findings for controller safety walls, merge gates, cleanup gates, author/reviewer separation, and prior `REQUEST_CHANGES` fixes — "suite green" is not a review. 3. Secret/provenance sweep reported as a summary without the exact command or method used. 4. Prior `REQUEST_CHANGES` existed (at `6717c33e…`) but the report did not list the old blockers and map each to its fix in the new diff. **Scope addendum for this issue:** add to required reporting — when prior request-changes reviews exist, enumerate each prior blocker and the exact diff element resolving it; high-risk PRs require per-gate findings, not just aggregate test results. **Cross-reference:** PR #73 merging activates the mcp-control-plane #76 decision rule — each acceptance-matrix wall must now be verified as implemented-with-tests or explicitly tracked out-of-scope. The PR body claims walls for identity, capability, checkout-proof, merge authorization, duplicate search, and report validation; #76 remains the open tracker for verifying those claims against the matrix.
jcwalker3 commented 2026-07-05 13:38:36 -05:00
Author
Owner
Copy Link
Copy Source

Required acceptance case: stop cleanly when live PR state changes during review (positive example)

First fully-positive stale-state evidence in this record — a concurrent reviewer race handled correctly. Add as a required acceptance case.

Evidence:
A reviewer session for mcp-control-plane PR #73 authenticated correctly as sysadmin / prgs-reviewer, pinned PR head b5caab87ad7154748431729be4f4020dc08e843f, checked out that exact SHA locally, and ran validation successfully:

  • targeted tests: 173 passed
  • full suite: 421 passed, 13 skipped
  • py_compile: passed
  • git diff --check: passed
  • secret/provenance grep: clean / benign matches only

Before submitting a review or merge, the agent rechecked live PR state and found the PR had changed:

  • PR #73 state: closed
  • merged_at: 2026-07-05T13:32:50-05:00
  • merge_commit_sha: 5e486652e192a6caa7e874353eae549f6ed865cc

(Consistent with this tracker's prior comment: PR #73 was merged by the A- graded reviewer run at exactly that timestamp — this session lost the race and detected it.)

Correct behavior observed:

  • stopped immediately
  • did not submit approve/request-changes/comment
  • did not merge
  • did not close or mutate issues
  • reported stale/live-state change clearly

Required behavior (add to this issue's requirements):

  1. Reviewer workflows must re-check live PR state before posting review or merge actions.
  2. If the PR changed from open to closed/merged during review, stop immediately.
  3. If the head SHA changed during review, stop immediately.
  4. If the base branch changed during review, stop or explicitly rebase/revalidate according to repo policy.
  5. A local validation result must not be used to approve or merge after live PR state changes.
  6. Final report must include:
    • pinned local SHA
    • live PR state at final check
    • whether review/merge was skipped due to stale state
    • confirmation that no mutation was performed

Tests/harness assertions to add:

  • PR closes during review → no review submitted, no merge attempted
  • PR head changes during review → no review submitted, no merge attempted
  • PR already merged by another reviewer → report stale state and stop
  • final report distinguishes "validation passed locally" from "not safe to act because live PR changed"

Acceptance criterion: a blind PR review earns an A when it detects live PR state changes before mutation and stops without posting stale approval/merge actions.

Cross-reference: this is the workflow-level counterpart of mcp-control-plane #76 wall 4 ("live PR state must be re-read immediately before approval/merge") — this run proves the behavior is achievable and should be the tested norm. Note the merged #73 implementation itself claims this gate; this evidence predates/validates it at the workflow level.

## Required acceptance case: stop cleanly when live PR state changes during review (positive example) First fully-positive stale-state evidence in this record — a concurrent reviewer race handled correctly. Add as a required acceptance case. **Evidence:** A reviewer session for mcp-control-plane PR #73 authenticated correctly as `sysadmin / prgs-reviewer`, pinned PR head `b5caab87ad7154748431729be4f4020dc08e843f`, checked out that exact SHA locally, and ran validation successfully: * targeted tests: `173 passed` * full suite: `421 passed, 13 skipped` * `py_compile`: passed * `git diff --check`: passed * secret/provenance grep: clean / benign matches only Before submitting a review or merge, the agent rechecked live PR state and found the PR had changed: * PR #73 state: `closed` * `merged_at`: `2026-07-05T13:32:50-05:00` * `merge_commit_sha`: `5e486652e192a6caa7e874353eae549f6ed865cc` (Consistent with this tracker's prior comment: PR #73 was merged by the A- graded reviewer run at exactly that timestamp — this session lost the race and detected it.) **Correct behavior observed:** * stopped immediately * did not submit approve/request-changes/comment * did not merge * did not close or mutate issues * reported stale/live-state change clearly **Required behavior (add to this issue's requirements):** 1. Reviewer workflows must re-check live PR state before posting review or merge actions. 2. If the PR changed from open to closed/merged during review, stop immediately. 3. If the head SHA changed during review, stop immediately. 4. If the base branch changed during review, stop or explicitly rebase/revalidate according to repo policy. 5. A local validation result must not be used to approve or merge after live PR state changes. 6. Final report must include: * pinned local SHA * live PR state at final check * whether review/merge was skipped due to stale state * confirmation that no mutation was performed **Tests/harness assertions to add:** * PR closes during review → no review submitted, no merge attempted * PR head changes during review → no review submitted, no merge attempted * PR already merged by another reviewer → report stale state and stop * final report distinguishes "validation passed locally" from "not safe to act because live PR changed" **Acceptance criterion:** a blind PR review earns an A when it detects live PR state changes before mutation and stops without posting stale approval/merge actions. **Cross-reference:** this is the workflow-level counterpart of mcp-control-plane #76 wall 4 ("live PR state must be re-read immediately before approval/merge") — this run proves the behavior is achievable and should be the tested norm. Note the merged #73 implementation itself claims this gate; this evidence predates/validates it at the workflow level.
jcwalker3 commented 2026-07-05 14:00:04 -05:00
Author
Owner
Copy Link
Copy Source

Scope addendum: identity-first execution and complete configured-repo inventory

Title for this addendum: Require identity-first execution and complete configured-repo inventory in blind PR queue reviews

Problem:
A reviewer run safely stopped after finding no open PRs in Scaled-Tech-Consulting/mcp-control-plane, but the blind PR review harness/prompt still showed quality gaps that should be made explicit and testable.

Observed behavior:

  • The run called gitea_list_prs before gitea_whoami.
  • It authenticated as sysadmin / prgs-reviewer.
  • It checked only Scaled-Tech-Consulting/mcp-control-plane.
  • It found 0 open PRs there.
  • It did not check Scaled-Tech-Consulting/Gitea-Tools.
  • It concluded too broadly that the mcp-control-plane queue was fully reviewed/merged or that no queue work remained.
  • It also implied no blockers/follow-ups, even though issue backlog state was not checked.

Problems to harden:

  1. Identity/profile verification must happen before any PR inventory or repo action.
  2. A blind queue review must inventory all configured repositories, not only one target repo, unless the operator explicitly scoped the request to one repo.
  3. 0 open PRs in one repo must not be reported as the whole queue being fully reviewed and merged.
  4. 0 open PRs must not be reported as no follow-up, because open issues may still remain.
  5. Final reports must distinguish:
    • no open PRs in a specific repo
    • no open PRs across all configured repos
    • no eligible PRs for this reviewer
    • no follow-up work
    • open issues still present but no PR ready

Required behavior:

  1. Always run identity/profile checks first:
    • whoami
    • runtime context
    • exact capabilities, if mutation could be attempted later
  2. For blind PR queue review, inventory all configured repositories:
    • Scaled-Tech-Consulting/Gitea-Tools
    • Scaled-Tech-Consulting/mcp-control-plane
    • any future configured repos included in the workflow
  3. Report open PR counts per repo, even when zero.
  4. Do not claim queue fully reviewed and merged unless:
    • all configured repos were inventoried
    • all open PRs were either reviewed/merged or explicitly categorized
    • no open PRs remain across the configured repos
  5. Do not claim no follow-up unless issue inventory was also requested and checked. Otherwise say:
    • No open PRs found in the checked repo.
    • Issue backlog was not checked.

Tests / harness assertions to add:

  • gitea_whoami must precede gitea_list_prs.
  • Single-repo inventory cannot claim full queue completion.
  • Blind queue inventory must include all configured repos.
  • Zero open PRs in one repo is reported narrowly.
  • Final report distinguishes PR queue status from issue backlog status.
  • no follow-up claim is blocked unless issue backlog was checked.

Acceptance criteria addendum:

  • A blind PR queue review earns an A only when identity is verified first, all configured repos are inventoried, open PR counts are reported per repo, and final conclusions are scoped to what was actually checked.
  • The workflow must create precise, durable queue evidence instead of broad unsupported conclusions.
## Scope addendum: identity-first execution and complete configured-repo inventory Title for this addendum: **Require identity-first execution and complete configured-repo inventory in blind PR queue reviews** Problem: A reviewer run safely stopped after finding no open PRs in `Scaled-Tech-Consulting/mcp-control-plane`, but the blind PR review harness/prompt still showed quality gaps that should be made explicit and testable. Observed behavior: * The run called `gitea_list_prs` before `gitea_whoami`. * It authenticated as `sysadmin / prgs-reviewer`. * It checked only `Scaled-Tech-Consulting/mcp-control-plane`. * It found `0` open PRs there. * It did not check `Scaled-Tech-Consulting/Gitea-Tools`. * It concluded too broadly that the mcp-control-plane queue was fully reviewed/merged or that no queue work remained. * It also implied no blockers/follow-ups, even though issue backlog state was not checked. Problems to harden: 1. Identity/profile verification must happen before any PR inventory or repo action. 2. A blind queue review must inventory all configured repositories, not only one target repo, unless the operator explicitly scoped the request to one repo. 3. `0` open PRs in one repo must not be reported as the whole queue being fully reviewed and merged. 4. `0` open PRs must not be reported as `no follow-up`, because open issues may still remain. 5. Final reports must distinguish: * no open PRs in a specific repo * no open PRs across all configured repos * no eligible PRs for this reviewer * no follow-up work * open issues still present but no PR ready Required behavior: 1. Always run identity/profile checks first: * `whoami` * runtime context * exact capabilities, if mutation could be attempted later 2. For blind PR queue review, inventory all configured repositories: * `Scaled-Tech-Consulting/Gitea-Tools` * `Scaled-Tech-Consulting/mcp-control-plane` * any future configured repos included in the workflow 3. Report open PR counts per repo, even when zero. 4. Do not claim `queue fully reviewed and merged` unless: * all configured repos were inventoried * all open PRs were either reviewed/merged or explicitly categorized * no open PRs remain across the configured repos 5. Do not claim `no follow-up` unless issue inventory was also requested and checked. Otherwise say: * `No open PRs found in the checked repo.` * `Issue backlog was not checked.` Tests / harness assertions to add: * `gitea_whoami` must precede `gitea_list_prs`. * Single-repo inventory cannot claim full queue completion. * Blind queue inventory must include all configured repos. * Zero open PRs in one repo is reported narrowly. * Final report distinguishes PR queue status from issue backlog status. * `no follow-up` claim is blocked unless issue backlog was checked. Acceptance criteria addendum: * A blind PR queue review earns an A only when identity is verified first, all configured repos are inventoried, open PR counts are reported per repo, and final conclusions are scoped to what was actually checked. * The workflow must create precise, durable queue evidence instead of broad unsupported conclusions.
jcwalker3 commented 2026-07-05 14:01:06 -05:00
Author
Owner
Copy Link
Copy Source

Required acceptance case: all configured repositories before claiming no open PRs

Section title: Blind PR queue review must check all configured repositories before claiming no open PRs

Evidence:
A reviewer run authenticated correctly as sysadmin / prgs-reviewer and checked Scaled-Tech-Consulting/mcp-control-plane. It found zero open PRs there and stopped.

However, at the same time, Gitea-Tools PR #176 was open:

Observed incorrect behavior:

  • The run reported no selected PR because mcp-control-plane had zero open PRs.
  • It did not inventory Scaled-Tech-Consulting/Gitea-Tools.
  • It did not find PR #176.
  • The conclusion was only valid for one repo, but the broader blind queue workflow requires all configured repos.

Required behavior:

  1. A blind PR queue review must inventory all configured repositories before selecting or stopping:
    • Scaled-Tech-Consulting/Gitea-Tools
    • Scaled-Tech-Consulting/mcp-control-plane
  2. If only one repository is checked, the final report must explicitly say:
    • Only <repo> was checked.
    • Other configured repositories were not inventoried.
    • This is not a complete blind queue review.
  3. The workflow must not conclude any of the following unless all configured repositories were checked:
    • no eligible open PRs
    • queue empty
    • queue fully reviewed
    • no follow-up
  4. Add a test/harness assertion:
    • Given mcp-control-plane has zero open PRs and Gitea-Tools has PR #176 open, the blind queue workflow must find PR #176.
    • A single-repo zero-result inventory must be downgraded to incomplete, not treated as a queue stop.

Acceptance criteria addendum:

  • A blind PR queue review earns an A only if it reports open PR counts per configured repo.
  • A zero-open result from one repo cannot hide open PRs in another repo.
  • PR #176-style cases are discovered and categorized before the workflow stops.
## Required acceptance case: all configured repositories before claiming no open PRs Section title: **Blind PR queue review must check all configured repositories before claiming no open PRs** Evidence: A reviewer run authenticated correctly as `sysadmin / prgs-reviewer` and checked `Scaled-Tech-Consulting/mcp-control-plane`. It found zero open PRs there and stopped. However, at the same time, Gitea-Tools PR #176 was open: * Repo: `Scaled-Tech-Consulting/Gitea-Tools` * PR: #176 * Title: `Add fail-closed proofs for blind PR queue review workflow (Issue #173)` * Author: `jcwalker3` * Branch: `feat/issue-173-reviewer-workflow-proofs` * Base: `master` * State: open * Files changed: 4 * Diff: +880 / -8 Observed incorrect behavior: * The run reported no selected PR because `mcp-control-plane` had zero open PRs. * It did not inventory `Scaled-Tech-Consulting/Gitea-Tools`. * It did not find PR #176. * The conclusion was only valid for one repo, but the broader blind queue workflow requires all configured repos. Required behavior: 1. A blind PR queue review must inventory all configured repositories before selecting or stopping: * `Scaled-Tech-Consulting/Gitea-Tools` * `Scaled-Tech-Consulting/mcp-control-plane` 2. If only one repository is checked, the final report must explicitly say: * `Only <repo> was checked.` * `Other configured repositories were not inventoried.` * `This is not a complete blind queue review.` 3. The workflow must not conclude any of the following unless all configured repositories were checked: * `no eligible open PRs` * `queue empty` * `queue fully reviewed` * `no follow-up` 4. Add a test/harness assertion: * Given `mcp-control-plane` has zero open PRs and `Gitea-Tools` has PR #176 open, the blind queue workflow must find PR #176. * A single-repo zero-result inventory must be downgraded to incomplete, not treated as a queue stop. Acceptance criteria addendum: * A blind PR queue review earns an A only if it reports open PR counts per configured repo. * A zero-open result from one repo cannot hide open PRs in another repo. * PR #176-style cases are discovered and categorized before the workflow stops.
sysadmin commented 2026-07-05 14:03:56 -05:00
Owner
Copy Link
Copy Source

Scoped single-repo inventory is safe only when the operator explicitly scopes the task

Evidence:

Codex authenticated correctly as sysadmin / prgs-reviewer and checked only:

  • Scaled-Tech-Consulting/mcp-control-plane

It found:

  • gitea_list_prs(state=open) returned []
  • open PR count in that repo: 0
  • no review or merge performed
  • no mutations performed

At the same time, Gitea-Tools PR #176 remained open:

Positive behavior:

Codex correctly scoped its conclusion by saying:
“This conclusion applies only to Scaled-Tech-Consulting/mcp-control-plane.”

Remaining workflow gap:

If the operator asked for a blind/open-PR queue review, the workflow still failed to check all configured repositories and missed open PR #176.

Required behavior:

  1. If the task is explicitly scoped to one repo, a single-repo inventory is acceptable, but the final report must clearly say it is scoped.

  2. If the task is blind or general, such as:

    • “review open PRs”
    • “review the queue”
    • “pick the next eligible PR”
    • “what PRs are open”

    then the workflow must inventory all configured repositories before stopping:

    • Scaled-Tech-Consulting/Gitea-Tools
    • Scaled-Tech-Consulting/mcp-control-plane

  3. The workflow must classify the task scope before inventory:

    • explicit single-repo task
    • configured multi-repo queue task
    • ambiguous task requiring default multi-repo inventory

  4. Add tests/harness assertions:

    • explicit mcp-control-plane task may stop after checking only that repo
    • blind queue task must check both configured repos
    • general “review open PRs” task must find Gitea-Tools PR #176 even when mcp-control-plane has zero open PRs
    • final report must state whether inventory was scoped or complete
    • a scoped result cannot be used as evidence that the whole queue is empty

Acceptance criteria:

  • Scoped repo checks are allowed only when the operator explicitly names the repo or PR.
  • Blind queue checks must report counts per configured repo.
  • Open PR #176-style cases cannot be missed by checking only an empty repo.
### Scoped single-repo inventory is safe only when the operator explicitly scopes the task #### Evidence: Codex authenticated correctly as `sysadmin / prgs-reviewer` and checked only: * `Scaled-Tech-Consulting/mcp-control-plane` It found: * `gitea_list_prs(state=open)` returned `[]` * open PR count in that repo: `0` * no review or merge performed * no mutations performed At the same time, Gitea-Tools PR #176 remained open: * Repo: `Scaled-Tech-Consulting/Gitea-Tools` * PR: #176 * Title: `Add fail-closed proofs for blind PR queue review workflow (Issue #173)` * Author: `jcwalker3` * Branch: `feat/issue-173-reviewer-workflow-proofs` * Base: `master` * State: open #### Positive behavior: Codex correctly scoped its conclusion by saying: “This conclusion applies only to `Scaled-Tech-Consulting/mcp-control-plane`.” #### Remaining workflow gap: If the operator asked for a blind/open-PR queue review, the workflow still failed to check all configured repositories and missed open PR #176. #### Required behavior: 1. If the task is explicitly scoped to one repo, a single-repo inventory is acceptable, but the final report must clearly say it is scoped. 2. If the task is blind or general, such as: * “review open PRs” * “review the queue” * “pick the next eligible PR” * “what PRs are open” then the workflow must inventory all configured repositories before stopping: * `Scaled-Tech-Consulting/Gitea-Tools` * `Scaled-Tech-Consulting/mcp-control-plane` 3. The workflow must classify the task scope before inventory: * explicit single-repo task * configured multi-repo queue task * ambiguous task requiring default multi-repo inventory 4. Add tests/harness assertions: * explicit `mcp-control-plane` task may stop after checking only that repo * blind queue task must check both configured repos * general “review open PRs” task must find Gitea-Tools PR #176 even when mcp-control-plane has zero open PRs * final report must state whether inventory was scoped or complete * a scoped result cannot be used as evidence that the whole queue is empty #### Acceptance criteria: * Scoped repo checks are allowed only when the operator explicitly names the repo or PR. * Blind queue checks must report counts per configured repo. * Open PR #176-style cases cannot be missed by checking only an empty repo.
sysadmin removed the status:in-progress label 2026-07-05 14:09:00 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
allocator
Controller-owned work allocator
 anti-stomp
Prevent concurrent LLM session stomping
 architecture
Architecture / structural design
 bug
chore
codex
OpenAI Codex client / workflow session surface
 concurrency
Concurrent session safety
 contamination
Workflow or session contamination incident
 control-plane
MCP control-plane coordination and allocation authority
 dashboard
MCP operational dashboard/queue view
 database
Internal coordination storage (SQLite/Postgres)
 design
Design / investigation, no implementation
 documentation
Docs / runbooks
 enhancement
New feature or improvement
 gitea
Gitea MCP workflow
 glitchtip
GlitchTip integration
 important
incident
Operational or process incident requiring durable audit trail
 incident-bridge
Sentry-to-Gitea incident bridging
 integration
Integration testing
 jenkins
Jenkins integration
 labels
Label taxonomy management
 leases
Lease adopt/release/expire lifecycle
 mcp
MCP server / tooling
 mcp-health
MCP namespace and runtime health
 mcp-menu
MCP menu surface
 multi-project
Work spanning multiple monitoring projects or Gitea repos
 mutating
Mutating action; requires gating
 nice-to-have
observability
Observability, metrics, traces, error reporting
 portability
Cross-platform / portability
 preflight
Shared preflight gates before mutation
 protected-branch
Protected branch / stable-branch policy concern
 queue
Work queue visibility and allocation
 read-only
Read-only, no mutation
 reconnect
MCP client reconnect/reload recovery path
 recovery
Recovery paths for stale/foreign leases
 refactor
Code refactor / restructure
 release
Release / versioning
 reliability
Reliability / failure handling
 resumable-review
Persist and resume prepared review verdicts across sessions
 reviewer
Reviewer workflow tooling
 roadmap
Roadmap / umbrella issue
 safety
Safety rails and fail-closed mutation guards
 security
Security / trust boundary
 self-hosted
Self-hosted infrastructure integration
 sentry
Sentry error monitoring integration
 stale-runtime
Stale backend daemon / runtime-vs-master parity failures
 status:blocked
Issue is blocked
 status:in-progress
Issue is being worked on
 status:pr-open
Issue has an open pull request
 status:ready
Issue is ready for work
 terminal-lock
Terminal review lock (#332) path
 testing
Tests / test coverage
 tracker
Issue tracker hygiene / meta
 type:bug
Bug or defect
 type:feature
Feature or enhancement
 type:feature
Feature or enhancement
 type:guardrail
Safety gate or guardrail
 visibility
Workflow state visibility for LLMs/operators
 workflow
Cross-tool workflow
 workflow-hardening
LLM workflow coordination hardening
 workflow-hardening
LLM workflow coordination hardening
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jcwalker3 sysadmin
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#173