Harden LLM role/task alignment and PR review feedback discovery #169

2026-07-05T12:00:25-05:00

jcwalker3 commented

2026-07-05 12:00:25 -05:00

Summary

Review feedback discovery (MCP-native): new read-only tool gitea_get_pr_review_feedback reads Gitea's formal review endpoints (GET /pulls/{n} + GET /pulls/{n}/reviews) so LLMs never infer review state from issue comments. Reports each submitted review (reviewer, verdict, redacted body, timestamp, reviewed head SHA, dismissed, stale), latest_review_state_by_reviewer (PENDING drafts and COMMENT reviews never drive verdict state), has_blocking_change_requests (dismissed reviews do not block), approval_visible, current_head_sha vs latest_reviewed_head_sha, review_feedback_stale, and author_pushed_after_request_changes. COMMENT reviews never advance the reviewed-head marker, so a drive-by comment on the current head cannot mask a stale REQUEST_CHANGES.
Fail-closed gating: the tool requires gitea.read; a permission block returns feedback_not_attempted: true with reasons and a structured #142 permission_report and makes zero API calls — deliberately distinct from a successful "no reviews yet" result.
Task/role guards: gitea_resolve_task_capability now returns stop_required and task_role_guidance, and gains a distinct author task address_pr_change_requests (requires gitea.branch.push). A review/merge task under an author profile gets explicit STOP guidance (no author-side commits/pushes/edits/comments unless the operator re-tasks); author-side remediation is explicitly barred from review verdicts and merges. All changes additive; no permission broadened; gitea.pr.comment still does not imply gitea.issue.comment.
Validation reporting: new build_validation_report helper distinguishes passed / failed / skipped / not-run, requires the exact output for failures, rejects vague statuses (fail closed), and reports full_suite_passed as a strict tri-state so full-suite success is never implied.
Docs: docs/llm-workflow-runbooks.md gains three sections — a task/role alignment table (review PR / address change requests / merge / comment on issue / comment on PR / author implementation, each with required identity, allowed/forbidden actions, and stop conditions), MCP-native review feedback discovery, and validation reporting discipline.
Tests: tests/test_review_feedback.py (24 tests) covers discovery, per-reviewer latest state, staleness after new commits, COMMENT-masking prevention, dismissed REQUEST_CHANGES/APPROVED handling, PENDING exclusion, body redaction, URL hiding without the reveal opt-in, fail-closed gating with zero API calls, role guards for author and reviewer profiles, and validation report semantics.

Safety

No review/merge gate weakened; self-review/self-merge blocks untouched.
New tool is strictly read-only; discovery never mutates.
No secrets, tokens, keychain IDs, raw service URLs, or config-sensitive values in code, tests, or output paths.

Validation

python3 -m py_compile mcp_server.py tests/*.py — passed.
venv/bin/python3 -m unittest tests.test_review_feedback — passed (24 tests).
venv/bin/python3 -m unittest discover -s tests — passed (626 tests, OK).
git diff --check — passed (clean).
Secret/provenance sweep over the diff — passed (synthetic fixtures only).
Read-only subagent review — no blockers, no should-fix; nits addressed (COMMENT staleness-masking fixed with a regression test, dismissed-APPROVED coverage added, docstring verdict name corrected).

Closes #167

## Summary - **Review feedback discovery (MCP-native):** new read-only tool `gitea_get_pr_review_feedback` reads Gitea's formal review endpoints (`GET /pulls/{n}` + `GET /pulls/{n}/reviews`) so LLMs never infer review state from issue comments. Reports each submitted review (reviewer, verdict, redacted body, timestamp, reviewed head SHA, dismissed, stale), `latest_review_state_by_reviewer` (PENDING drafts and COMMENT reviews never drive verdict state), `has_blocking_change_requests` (dismissed reviews do not block), `approval_visible`, `current_head_sha` vs `latest_reviewed_head_sha`, `review_feedback_stale`, and `author_pushed_after_request_changes`. COMMENT reviews never advance the reviewed-head marker, so a drive-by comment on the current head cannot mask a stale REQUEST_CHANGES. - **Fail-closed gating:** the tool requires `gitea.read`; a permission block returns `feedback_not_attempted: true` with reasons and a structured #142 `permission_report` and makes **zero** API calls — deliberately distinct from a successful "no reviews yet" result. - **Task/role guards:** `gitea_resolve_task_capability` now returns `stop_required` and `task_role_guidance`, and gains a distinct author task `address_pr_change_requests` (requires `gitea.branch.push`). A review/merge task under an author profile gets explicit STOP guidance (no author-side commits/pushes/edits/comments unless the operator re-tasks); author-side remediation is explicitly barred from review verdicts and merges. All changes additive; no permission broadened; `gitea.pr.comment` still does not imply `gitea.issue.comment`. - **Validation reporting:** new `build_validation_report` helper distinguishes `passed` / `failed` / `skipped` / `not-run`, requires the exact output for failures, rejects vague statuses (fail closed), and reports `full_suite_passed` as a strict tri-state so full-suite success is never implied. - **Docs:** `docs/llm-workflow-runbooks.md` gains three sections — a task/role alignment table (review PR / address change requests / merge / comment on issue / comment on PR / author implementation, each with required identity, allowed/forbidden actions, and stop conditions), MCP-native review feedback discovery, and validation reporting discipline. - **Tests:** `tests/test_review_feedback.py` (24 tests) covers discovery, per-reviewer latest state, staleness after new commits, COMMENT-masking prevention, dismissed REQUEST_CHANGES/APPROVED handling, PENDING exclusion, body redaction, URL hiding without the reveal opt-in, fail-closed gating with zero API calls, role guards for author and reviewer profiles, and validation report semantics. ## Safety - No review/merge gate weakened; self-review/self-merge blocks untouched. - New tool is strictly read-only; discovery never mutates. - No secrets, tokens, keychain IDs, raw service URLs, or config-sensitive values in code, tests, or output paths. ## Validation - `python3 -m py_compile mcp_server.py tests/*.py` — passed. - `venv/bin/python3 -m unittest tests.test_review_feedback` — passed (24 tests). - `venv/bin/python3 -m unittest discover -s tests` — passed (626 tests, OK). - `git diff --check` — passed (clean). - Secret/provenance sweep over the diff — passed (synthetic fixtures only). - Read-only subagent review — no blockers, no should-fix; nits addressed (COMMENT staleness-masking fixed with a regression test, dismissed-APPROVED coverage added, docstring verdict name corrected). Closes #167

jcwalker3 added 1 commit 2026-07-05 12:00:25 -05:00

feat: harden LLM role/task alignment and PR review feedback discovery (#167 ) a4060c59cc

- Add gitea_get_pr_review_feedback: read-only MCP-native discovery of
  formal PR review verdicts (APPROVED / REQUEST_CHANGES / COMMENT) with
  latest-state-per-reviewer, blocking change-request and approval
  summaries, reviewed-head vs current-head staleness, dismissed/PENDING
  handling, credential redaction of review bodies, no endpoint URLs
  without the reveal opt-in, and a fail-closed gitea.read gate that
  returns feedback_not_attempted with a structured #142 permission
  report and zero API calls.
- Add task/role guards to gitea_resolve_task_capability: new
  stop_required and task_role_guidance outputs, and a distinct
  address_pr_change_requests author task, so a review/merge request
  under an author profile returns explicit STOP guidance instead of
  silently degrading into author-side mutations, and author-side
  remediation is explicitly barred from review verdicts and merges.
- Add build_validation_report: validation reporting helper that
  distinguishes passed / failed / skipped / not-run, requires the exact
  output for failures, rejects vague statuses, and never implies
  full-suite success unless the full-suite command passed.
- Document task/role alignment (review vs author-fix vs merge vs
  comment workflows with required identity, allowed/forbidden actions,
  and stop conditions), MCP-native review feedback discovery, and
  validation reporting discipline in docs/llm-workflow-runbooks.md.
- Add tests/test_review_feedback.py (22 tests) covering discovery,
  staleness, redaction, URL hiding, fail-closed gating, role guards for
  author/reviewer profiles, and validation report semantics.

Closes #167

Co-Authored-By: Claude Fable 5 <[email protected]>

sysadmin reviewed 2026-07-05 12:05:46 -05:00

sysadmin left a comment

APPROVE at head a4060c59cc.

Reviewed against issue #167 requirements (all six covered):

Task/role guards: gitea_resolve_task_capability returns stop_required + task_role_guidance; review/merge tasks under an author profile get explicit STOP guidance; new address_pr_change_requests author task requires gitea.branch.push and is barred from review verdicts/merges.
MCP-native review feedback discovery: read-only gitea_get_pr_review_feedback over formal review endpoints; no inference from issue comments; PENDING/COMMENT never drive verdict state; COMMENT reviews cannot mask a stale REQUEST_CHANGES.
PR state summary: has_blocking_change_requests, approval_visible, current vs latest-reviewed head SHA, review_feedback_stale, author_pushed_after_request_changes.
Validation reporting: build_validation_report enforces passed/failed/skipped/not-run with exact failure output; full_suite_passed strict tri-state; rejects vague statuses.
Docs: llm-workflow-runbooks.md task/role table with required identity, allowed/forbidden actions, stop conditions per workflow.
Tests: tests/test_review_feedback.py, 24 passed.

Gate integrity: diff is purely additive (0 deletions in mcp_server.py); existing review/merge/comment gates untouched; new tool requires gitea.read and fails closed with zero API calls on a permission block; bodies redacted; endpoint URLs only behind GITEA_MCP_REVEAL_ENDPOINTS.

Validation at pinned head: local checkout == a4060c5 exactly; tests/test_review_feedback.py 24 passed; full suite 633 passed / 6 skipped; python3 -m py_compile mcp_server.py clean; git diff --check clean; secret sweep clean (matches are synthetic test fixtures only).

Scope note for the record: PR #169 implements issue #167. It does NOT address the mcp-control-plane #66 comment-2817 blocker (PR #168 missing tests for the five issue-#166 Requirement-5 conflict/staleness scenarios); no tests matching those scenarios exist on this head. That remains an open follow-up against merged #168, tracked separately.

Reviewed by sysadmin/prgs-reviewer; author jcwalker3.

APPROVE at head a4060c59cc7ac2033cf77764677c81d067e29316. Reviewed against issue #167 requirements (all six covered): 1. Task/role guards: gitea_resolve_task_capability returns stop_required + task_role_guidance; review/merge tasks under an author profile get explicit STOP guidance; new address_pr_change_requests author task requires gitea.branch.push and is barred from review verdicts/merges. 2. MCP-native review feedback discovery: read-only gitea_get_pr_review_feedback over formal review endpoints; no inference from issue comments; PENDING/COMMENT never drive verdict state; COMMENT reviews cannot mask a stale REQUEST_CHANGES. 3. PR state summary: has_blocking_change_requests, approval_visible, current vs latest-reviewed head SHA, review_feedback_stale, author_pushed_after_request_changes. 4. Validation reporting: build_validation_report enforces passed/failed/skipped/not-run with exact failure output; full_suite_passed strict tri-state; rejects vague statuses. 5. Docs: llm-workflow-runbooks.md task/role table with required identity, allowed/forbidden actions, stop conditions per workflow. 6. Tests: tests/test_review_feedback.py, 24 passed. Gate integrity: diff is purely additive (0 deletions in mcp_server.py); existing review/merge/comment gates untouched; new tool requires gitea.read and fails closed with zero API calls on a permission block; bodies redacted; endpoint URLs only behind GITEA_MCP_REVEAL_ENDPOINTS. Validation at pinned head: local checkout == a4060c5 exactly; tests/test_review_feedback.py 24 passed; full suite 633 passed / 6 skipped; python3 -m py_compile mcp_server.py clean; git diff --check clean; secret sweep clean (matches are synthetic test fixtures only). Scope note for the record: PR #169 implements issue #167. It does NOT address the mcp-control-plane #66 comment-2817 blocker (PR #168 missing tests for the five issue-#166 Requirement-5 conflict/staleness scenarios); no tests matching those scenarios exist on this head. That remains an open follow-up against merged #168, tracked separately. Reviewed by sysadmin/prgs-reviewer; author jcwalker3.

sysadmin merged commit 29f04d003a into master

2026-07-05 12:17:43 -05:00

jcwalker3 referenced this pull request

2026-07-05 12:29:23 -05:00

Add missing tests for issue #166 Requirement-5 conflict/staleness scenarios #170

jcwalker3 referenced this pull request

2026-07-05 12:29:40 -05:00

Audit and harden redaction for raw URL exposure in PR inventory/review failure output #171

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

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