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

Tighten A-level reviewer proof requirements after #173 merge #179

New Issue
Closed
opened 2026-07-05 14:15:29 -05:00 by jcwalker3 · 4 comments
jcwalker3 commented 2026-07-05 14:15:29 -05:00
Owner
Copy Link
Copy Source

Context:
PR #176 closed issue #173 and significantly improved blind PR queue review proofs. A later reviewer run successfully found and merged PR #176 after checking both configured repos. The run was strong, but it exposed a few remaining proof/reporting gaps that should become durable harness requirements.

Positive evidence from the run:

  • authenticated as sysadmin / prgs-reviewer
  • inventoried both configured repos:
    • Scaled-Tech-Consulting/Gitea-Tools: PR #176 open and selected
    • Scaled-Tech-Consulting/mcp-control-plane: PR #79 open and left for future review
  • selected one eligible PR
  • pinned and checked out head ccc5ef79dfe629853e144763238593bd808d57e0
  • proved local HEAD matched the pinned SHA
  • ran validation:
    • PYTHONPATH=. ./venv/bin/pytest tests/test_review_proofs.py → 35/35 passed
    • PYTHONPATH=. ./venv/bin/pytest tests/ --ignore=branches → 681 passed, 0 failures, 6 skipped
    • python3 -m py_compile review_proofs.py tests/test_review_proofs.py → passed
    • git diff --check prgs/master...HEAD → passed
  • approved and merged only PR #176
  • left PR #79 open for a future unit
  • issue #173 closed/released after merge

Remaining proof gaps:

  1. Capability proof must be explicit.
    The final report claimed review_pr and merge_pr capability checks passed, but the visible tool log did not clearly show gitea_resolve_task_capability calls. A-level reports should cite the exact capability proof source.

  2. Secret/provenance sweep must be exact.
    "Checked diff for raw credentials or production service endpoints" is too vague. Reports should include the command, script, grep pattern, or named sweep method used.

  3. Final live-state recheck must be explicit.
    Before submitting review or merge, the workflow should explicitly re-read live PR state/head and confirm:

    • PR is still open
    • live head SHA still equals pinned SHA
    • base branch has not changed
    • no stale request-changes or blocking review state remains unresolved

  4. Reviewer flows should avoid unnecessary author namespace calls.
    The run used gitea-author for initial identity/runtime checks even though it was a reviewer task. This was not unsafe, but reviewer workflows should stay in gitea-reviewer unless author namespace use is explicitly justified.

Required behavior:

  • A reviewer/merge report cannot claim capability checks passed unless it includes exact evidence from gitea_resolve_task_capability or equivalent runtime context.
  • A secret/provenance sweep must include exact command/method and scope.
  • A final pre-mutation live-state check must be performed and reported before review submission and before merge.
  • Reviewer-only workflows should not call author namespace tools unless the report explains why.

Tests/harness assertions:

  • final report claiming capability proof without capability evidence is downgraded
  • vague secret sweep summary is downgraded
  • missing final live PR state/head recheck before merge is downgraded
  • reviewer task using author namespace without justification is downgraded
  • complete multi-repo inventory plus pinned-head validation remains the positive baseline

Acceptance criteria:
A blind PR review earns a full A only when it proves capability, complete inventory, pinned checkout, exact validation, exact secret/provenance sweep, final live-state recheck, and clean reviewer-only role boundaries before mutation.

Related:

  • Successor to #173 (closed via PR #176) — raises the A bar from checkout/inventory proofs to capability/sweep/live-state/role-boundary proofs.
  • #175 covers the hard role-pivot wall; gap 4 here is its soft edge (unjustified author-namespace reads during reviewer tasks).
  • The stale-state acceptance case and SHA-reconciliation requirements recorded in #173's comment thread (comments 2950/2966) carry forward into this issue's live-state recheck requirement.
  • mcp-control-plane PR #79 remains open for a future blind reviewer unit; that review should be held to this issue's bar.
Context: PR #176 closed issue #173 and significantly improved blind PR queue review proofs. A later reviewer run successfully found and merged PR #176 after checking both configured repos. The run was strong, but it exposed a few remaining proof/reporting gaps that should become durable harness requirements. Positive evidence from the run: * authenticated as `sysadmin / prgs-reviewer` * inventoried both configured repos: * `Scaled-Tech-Consulting/Gitea-Tools`: PR #176 open and selected * `Scaled-Tech-Consulting/mcp-control-plane`: PR #79 open and left for future review * selected one eligible PR * pinned and checked out head `ccc5ef79dfe629853e144763238593bd808d57e0` * proved local `HEAD` matched the pinned SHA * ran validation: * `PYTHONPATH=. ./venv/bin/pytest tests/test_review_proofs.py` → `35/35 passed` * `PYTHONPATH=. ./venv/bin/pytest tests/ --ignore=branches` → `681 passed, 0 failures, 6 skipped` * `python3 -m py_compile review_proofs.py tests/test_review_proofs.py` → passed * `git diff --check prgs/master...HEAD` → passed * approved and merged only PR #176 * left PR #79 open for a future unit * issue #173 closed/released after merge Remaining proof gaps: 1. Capability proof must be explicit. The final report claimed `review_pr` and `merge_pr` capability checks passed, but the visible tool log did not clearly show `gitea_resolve_task_capability` calls. A-level reports should cite the exact capability proof source. 2. Secret/provenance sweep must be exact. "Checked diff for raw credentials or production service endpoints" is too vague. Reports should include the command, script, grep pattern, or named sweep method used. 3. Final live-state recheck must be explicit. Before submitting review or merge, the workflow should explicitly re-read live PR state/head and confirm: * PR is still open * live head SHA still equals pinned SHA * base branch has not changed * no stale request-changes or blocking review state remains unresolved 4. Reviewer flows should avoid unnecessary author namespace calls. The run used `gitea-author` for initial identity/runtime checks even though it was a reviewer task. This was not unsafe, but reviewer workflows should stay in `gitea-reviewer` unless author namespace use is explicitly justified. Required behavior: * A reviewer/merge report cannot claim capability checks passed unless it includes exact evidence from `gitea_resolve_task_capability` or equivalent runtime context. * A secret/provenance sweep must include exact command/method and scope. * A final pre-mutation live-state check must be performed and reported before review submission and before merge. * Reviewer-only workflows should not call author namespace tools unless the report explains why. Tests/harness assertions: * final report claiming capability proof without capability evidence is downgraded * vague secret sweep summary is downgraded * missing final live PR state/head recheck before merge is downgraded * reviewer task using author namespace without justification is downgraded * complete multi-repo inventory plus pinned-head validation remains the positive baseline Acceptance criteria: A blind PR review earns a full A only when it proves capability, complete inventory, pinned checkout, exact validation, exact secret/provenance sweep, final live-state recheck, and clean reviewer-only role boundaries before mutation. Related: * Successor to #173 (closed via PR #176) — raises the A bar from checkout/inventory proofs to capability/sweep/live-state/role-boundary proofs. * #175 covers the hard role-pivot wall; gap 4 here is its soft edge (unjustified author-namespace reads during reviewer tasks). * The stale-state acceptance case and SHA-reconciliation requirements recorded in #173's comment thread (comments 2950/2966) carry forward into this issue's live-state recheck requirement. * mcp-control-plane PR #79 remains open for a future blind reviewer unit; that review should be held to this issue's bar.
sysadmin commented 2026-07-05 14:21:52 -05:00
Owner
Copy Link
Copy Source

PR #79 Reviewer Run Evidence

A reviewer run successfully reviewed and merged mcp-control-plane PR #79 for Issue #78.

Observed Positive Behavior:

  • Authenticated as sysadmin / prgs-reviewer.
  • Checked both configured repositories:
    • Scaled-Tech-Consulting/Gitea-Tools: PR #180 and PR #181 open.
    • Scaled-Tech-Consulting/mcp-control-plane: PR #79 open and selected.
  • Selected one eligible PR.
  • Pinned and checked out head e3b1b66a764a4d3a68565482e9268617e20b682e.
  • Verified local HEAD matched the pinned SHA.
  • Ran validation:
    • ./venv/bin/pytest tests/test_controller_capabilities.py tests/test_controller_reports.py tests/test_issue_escalation.py → 31/31 passed
    • ./venv/bin/pytest → 431 passed, 0 failures, 13 skipped
    • python3 -m py_compile → passed
    • git diff --check prgs/master...HEAD → passed
  • Approved and merged only PR #79.
  • Left Gitea-Tools PR #180 and #181 open for later review.
  • Issue #78 closed after merge.

Remaining gaps:

  1. Missing Controller Handoff block: The final report was structured but did not include a compact section titled exactly Controller Handoff.
  2. Capability proof not explicit enough: The final report claimed review_pr and merge_pr capability checks passed, but the visible tool log did not clearly show gitea_resolve_task_capability calls. Runtime context is useful, but A-level reports should include exact capability proof.
  3. Final live-state/head recheck before mutation not explicit: The workflow should explicitly re-read live PR state/head immediately before submitting review and again before merge, confirming:
    • PR is still open
    • live head SHA still equals pinned SHA
    • base branch did not change
    • blocking/stale review state is understood
  4. Secret/provenance sweep was vague: “Checked diff for raw credentials or production service endpoints” must be replaced with exact command/method/scope.
  5. Acceptance mapping was too thin for a safety-audit PR: PR #79 implemented issue #78, which came from the #76 audit. The reviewer should map each #78 acceptance item to files/tests before merging.
### PR #79 Reviewer Run Evidence A reviewer run successfully reviewed and merged `mcp-control-plane` [PR #79](https://gitea.prgs.cc/Scaled-Tech-Consulting/mcp-control-plane/pulls/79) for Issue #78. #### Observed Positive Behavior: * Authenticated as `sysadmin / prgs-reviewer`. * Checked both configured repositories: * `Scaled-Tech-Consulting/Gitea-Tools`: PR #180 and PR #181 open. * `Scaled-Tech-Consulting/mcp-control-plane`: PR #79 open and selected. * Selected one eligible PR. * Pinned and checked out head `e3b1b66a764a4d3a68565482e9268617e20b682e`. * Verified local `HEAD` matched the pinned SHA. * Ran validation: * `./venv/bin/pytest tests/test_controller_capabilities.py tests/test_controller_reports.py tests/test_issue_escalation.py` → `31/31 passed` * `./venv/bin/pytest` → `431 passed, 0 failures, 13 skipped` * `python3 -m py_compile` → passed * `git diff --check prgs/master...HEAD` → passed * Approved and merged only PR #79. * Left Gitea-Tools PR #180 and #181 open for later review. * Issue #78 closed after merge. #### Remaining gaps: 1. **Missing Controller Handoff block**: The final report was structured but did not include a compact section titled exactly `Controller Handoff`. 2. **Capability proof not explicit enough**: The final report claimed `review_pr` and `merge_pr` capability checks passed, but the visible tool log did not clearly show `gitea_resolve_task_capability` calls. Runtime context is useful, but A-level reports should include exact capability proof. 3. **Final live-state/head recheck before mutation not explicit**: The workflow should explicitly re-read live PR state/head immediately before submitting review and again before merge, confirming: * PR is still open * live head SHA still equals pinned SHA * base branch did not change * blocking/stale review state is understood 4. **Secret/provenance sweep was vague**: “Checked diff for raw credentials or production service endpoints” must be replaced with exact command/method/scope. 5. **Acceptance mapping was too thin for a safety-audit PR**: PR #79 implemented issue #78, which came from the #76 audit. The reviewer should map each #78 acceptance item to files/tests before merging.
jcwalker3 commented 2026-07-05 14:36:17 -05:00
Author
Owner
Copy Link
Copy Source

Claimed for author implementation. Branch: feat/issue-179-reviewer-proof-tightening. Worktree: branches/feat-issue-179-reviewer-proof-tightening.

Note on claim mechanics: gitea_resolve_task_capability returns Unknown task/action: 'mark_issue' (fail closed) under prgs-author, so per the #183 rule (unknown mutation capability must not be treated as allowed) this claim is recorded via the exactly-proven gitea.issue.comment operation instead of the status:in-progress label toggle. Resolver coverage for issue-claim operations is #183's scope.

Plan: extend review_proofs.py with the four #179 proofs — exact capability evidence, exact secret/provenance sweep evidence, pre-mutation live-state recheck (open + live head == pinned + base unchanged + no unresolved blocking reviews), and reviewer role-boundary (author-namespace use requires justification) — raise build_final_report's A bar to require them, update tests, and wire the requirements into SKILL.md §F/§G and the review/merge templates.

Claimed for author implementation. Branch: `feat/issue-179-reviewer-proof-tightening`. Worktree: `branches/feat-issue-179-reviewer-proof-tightening`. Note on claim mechanics: `gitea_resolve_task_capability` returns `Unknown task/action: 'mark_issue' (fail closed)` under `prgs-author`, so per the #183 rule (unknown mutation capability must not be treated as allowed) this claim is recorded via the exactly-proven `gitea.issue.comment` operation instead of the `status:in-progress` label toggle. Resolver coverage for issue-claim operations is #183's scope. Plan: extend `review_proofs.py` with the four #179 proofs — exact capability evidence, exact secret/provenance sweep evidence, pre-mutation live-state recheck (open + live head == pinned + base unchanged + no unresolved blocking reviews), and reviewer role-boundary (author-namespace use requires justification) — raise `build_final_report`'s A bar to require them, update tests, and wire the requirements into SKILL.md §F/§G and the review/merge templates.
jcwalker3 commented 2026-07-05 14:53:16 -05:00
Author
Owner
Copy Link
Copy Source

Codex queue-selection example: correct PR-first instinct, incomplete queue execution

Observed behavior:
Codex correctly concluded that the best next work is PR review, not more author implementation, because Gitea-Tools had 6 open mergeable PRs and mcp-control-plane had no open PRs. It proposed this order:

  1. PR #181 / issue Harden author workflow against branch drift and accidental commits to master (#177)
  2. PR #186 / issue Require controller handoff summaries in all LLM workflow final reports (#182)
  3. PR #187 / issue Harden author-run reporting and mutation capability proof: Controller Handoff, exact issue-claim capability, exact sweep evidence, PR head SHA (#183)
  4. PR #192 / issue Prevent implicit reviewer-to-author role pivot during blind PR queue workflows (#175)
  5. PR #193 / issue Tighten A-level reviewer proof requirements after #173 merge (#179)
  6. PR #190 / issue #146/#150 area, likely broader and needing care

Positive behavior:

  • Correctly prioritized clearing open PRs before starting more issue work.
  • Correctly recognized that starting new issue work would increase queue congestion.
  • Correctly identified #190 as potentially broader/riskier.

Remaining workflow gaps:

  1. The run was advisory/passive. For a queue-running reviewer session, it should select the next eligible PR and begin review instead of only recommending an order.
  2. It did not show live identity/profile proof.
  3. It did not report full inventory fields for each PR:
    • author
    • branch
    • base
    • head SHA
    • linked issue
    • mergeability
    • stale/conflict status
    • eligibility
  4. It did not prove that PR #181 was actually eligible/current before selecting it.
  5. It did not include a Controller Handoff.

Required behavior:

  • Reviewer queue agents must apply deterministic selection rules and select exactly one PR when enough information exists.
  • Advisory ranking is not enough for continue the queue tasks.
  • Before selecting, the agent must verify identity/profile and complete live inventory.
  • Selection must include evidence that older PRs are eligible or explain why they are skipped.
  • If the agent only provides a ranked recommendation, the report must be downgraded as incomplete for queue execution.

Acceptance criteria addition:
A reviewer queue run earns an A only when it performs live inventory, proves eligibility/staleness/mergeability, selects exactly one eligible PR, and proceeds to review or stops with evidence. A ranked list alone is not sufficient.

### Codex queue-selection example: correct PR-first instinct, incomplete queue execution Observed behavior: Codex correctly concluded that the best next work is PR review, not more author implementation, because `Gitea-Tools` had 6 open mergeable PRs and `mcp-control-plane` had no open PRs. It proposed this order: 1. PR #181 / issue #177 2. PR #186 / issue #182 3. PR #187 / issue #183 4. PR #192 / issue #175 5. PR #193 / issue #179 6. PR #190 / issue #146/#150 area, likely broader and needing care Positive behavior: * Correctly prioritized clearing open PRs before starting more issue work. * Correctly recognized that starting new issue work would increase queue congestion. * Correctly identified #190 as potentially broader/riskier. Remaining workflow gaps: 1. The run was advisory/passive. For a queue-running reviewer session, it should select the next eligible PR and begin review instead of only recommending an order. 2. It did not show live identity/profile proof. 3. It did not report full inventory fields for each PR: * author * branch * base * head SHA * linked issue * mergeability * stale/conflict status * eligibility 4. It did not prove that PR #181 was actually eligible/current before selecting it. 5. It did not include a Controller Handoff. Required behavior: * Reviewer queue agents must apply deterministic selection rules and select exactly one PR when enough information exists. * Advisory ranking is not enough for `continue the queue` tasks. * Before selecting, the agent must verify identity/profile and complete live inventory. * Selection must include evidence that older PRs are eligible or explain why they are skipped. * If the agent only provides a ranked recommendation, the report must be downgraded as incomplete for queue execution. Acceptance criteria addition: A reviewer queue run earns an A only when it performs live inventory, proves eligibility/staleness/mergeability, selects exactly one eligible PR, and proceeds to review or stops with evidence. A ranked list alone is not sufficient.
sysadmin commented 2026-07-05 16:10:11 -05:00
Owner
Copy Link
Copy Source

Additional observed failure:

A run executed the full test suite and got a failure:

TestAuthorProofs.test_run expected no_local_work but got order_violation.

The run then continued to produce a final PR-inventory report instead of treating the failing validation as a blocker.

Required behavior:

  • Any failed validation command must be reported as a blocker.

  • A final report must not summarize the run as clean if tests failed.

  • If a test failure occurs during a reviewer/inventory task, the report must explicitly separate:

    • PR inventory result;
    • local workspace validation result;
    • whether the failing test affects the current branch/PR.

  • If the active branch is unrelated to the target repo or review task, that must be disclosed.

Additional observed failure: A run executed the full test suite and got a failure: `TestAuthorProofs.test_run` expected `no_local_work` but got `order_violation`. The run then continued to produce a final PR-inventory report instead of treating the failing validation as a blocker. Required behavior: * Any failed validation command must be reported as a blocker. * A final report must not summarize the run as clean if tests failed. * If a test failure occurs during a reviewer/inventory task, the report must explicitly separate: * PR inventory result; * local workspace validation result; * whether the failing test affects the current branch/PR. * If the active branch is unrelated to the target repo or review task, that must be disclosed.
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#179