fix: guard PR/issue comment listing against non-list API payloads (Closes #485) #487

2026-07-08T01:17:58-05:00

jcwalker3 commented

2026-07-08 01:17:58 -05:00

Summary

Closes #485.

_list_pr_lease_comments and gitea_list_issue_comments now fail safe to an empty list when the comments API returns a non-list payload (e.g. an HTTP 401 error object), preventing KeyError: slice on the reviewer/conflict-fix lease path and the issue comment listing path.

Changes

gitea_mcp_server.py — non-list guard on _list_pr_lease_comments and gitea_list_issue_comments
tests/test_pr_lease_comments_non_list_guard.py — regression tests for dict/None payloads and unchanged list behavior

Acceptance criteria

_list_pr_lease_comments returns [] on non-list payload without raising
gitea_list_issue_comments site likewise guarded
Normal list payloads behave exactly as before
Regression tests cover non-list and valid list cases
No relaxation of #399 lease/stale-head gates

Verification

venv/bin/python -m pytest tests/test_pr_lease_comments_non_list_guard.py tests/test_mcp_server.py::TestIssueCommentTools -q
# 19 passed

Worktree

branches/issue-485-lease-comments-non-list-guard @ 08202f7

## Summary Closes #485. `_list_pr_lease_comments` and `gitea_list_issue_comments` now fail safe to an empty list when the comments API returns a non-list payload (e.g. an HTTP 401 error object), preventing `KeyError: slice` on the reviewer/conflict-fix lease path and the issue comment listing path. ## Changes - `gitea_mcp_server.py` — non-list guard on `_list_pr_lease_comments` and `gitea_list_issue_comments` - `tests/test_pr_lease_comments_non_list_guard.py` — regression tests for dict/None payloads and unchanged list behavior ## Acceptance criteria - [x] `_list_pr_lease_comments` returns `[]` on non-list payload without raising - [x] `gitea_list_issue_comments` site likewise guarded - [x] Normal list payloads behave exactly as before - [x] Regression tests cover non-list and valid list cases - [x] No relaxation of #399 lease/stale-head gates ## Verification ```bash venv/bin/python -m pytest tests/test_pr_lease_comments_non_list_guard.py tests/test_mcp_server.py::TestIssueCommentTools -q # 19 passed ``` ## Worktree `branches/issue-485-lease-comments-non-list-guard` @ `08202f7`

jcwalker3 added 1 commit 2026-07-08 01:17:59 -05:00

fix: guard PR/issue comment listing against non-list API payloads (Closes #485 ) 08202f7eaa

_list_pr_lease_comments and gitea_list_issue_comments now fail safe to an
empty list when the comments API returns a non-list payload (e.g. an error
object), preventing KeyError: slice on the lease and listing paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

sysadmin commented

2026-07-08 02:17:20 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #487
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 97274-676d20a825c4
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr487
phase: claimed
candidate_head: 08202f7eaa
target_branch: master
target_branch_sha: 974463163d
last_activity: 2026-07-08T07:17:18Z
expires_at: 2026-07-08T09:17:18Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #487 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 97274-676d20a825c4 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr487 phase: claimed candidate_head: 08202f7eaa6d09b2eca8f2960126994a4b22646b target_branch: master target_branch_sha: 974463163dff0c77022efda40d86f19ffd09fee9 last_activity: 2026-07-08T07:17:18Z expires_at: 2026-07-08T09:17:18Z blocker: none

sysadmin approved these changes 2026-07-08 02:23:28 -05:00

sysadmin left a comment

Approve — #485 follow-up source guard for PR/issue comment listing.

Reviewer: sysadmin / prgs-reviewer (not PR author jcwalker3). Reviewed head: 08202f7eaa.

Scope: clean, #485-only — gitea_mcp_server.py (two guard hunks) + tests/test_pr_lease_comments_non_list_guard.py. No conflict markers; no unrelated files.

Correctness:

_list_pr_lease_comments and gitea_list_issue_comments fail safe to empty on non-list API payloads (dict/None), fixing the KeyError: slice crash. Valid list payloads behave exactly as before; valid comments are not hidden.
Lease/head safety not weakened: an error payload yields no lease comments (no fabricated lease, no crash); #399/#407 head-SHA/stale gates untouched.
Regression test patches mcp_server.api_request (the real source path), not the helper — it exercises the actual guard, no patching-around.

Validation (isolated review worktree @ 08202f7):

pytest tests/test_pr_lease_comments_non_list_guard.py -q -> 5 passed
full suite -> 1 failed, 1736 passed; the sole failure test_mcp_server.py::TestMergePR::test_merge_blocked_on_stale_approval_head reproduces on clean prgs/master (9744631) -> pre-existing, not introduced by this PR.
git diff --check clean; py_compile clean.

Zero new failures. Approve for merge on explicit operator command via a separate merger session.

Approve — #485 follow-up source guard for PR/issue comment listing. Reviewer: sysadmin / prgs-reviewer (not PR author jcwalker3). Reviewed head: 08202f7eaa6d09b2eca8f2960126994a4b22646b. Scope: clean, #485-only — `gitea_mcp_server.py` (two guard hunks) + `tests/test_pr_lease_comments_non_list_guard.py`. No conflict markers; no unrelated files. Correctness: - `_list_pr_lease_comments` and `gitea_list_issue_comments` fail safe to empty on non-list API payloads (dict/None), fixing the `KeyError: slice` crash. Valid list payloads behave exactly as before; valid comments are not hidden. - Lease/head safety not weakened: an error payload yields no lease comments (no fabricated lease, no crash); #399/#407 head-SHA/stale gates untouched. - Regression test patches `mcp_server.api_request` (the real source path), not the helper — it exercises the actual guard, no patching-around. Validation (isolated review worktree @ 08202f7): - `pytest tests/test_pr_lease_comments_non_list_guard.py -q` -> 5 passed - full suite -> 1 failed, 1736 passed; the sole failure `test_mcp_server.py::TestMergePR::test_merge_blocked_on_stale_approval_head` reproduces on clean prgs/master (9744631) -> pre-existing, not introduced by this PR. - `git diff --check` clean; py_compile clean. Zero new failures. Approve for merge on explicit operator command via a separate merger session.

jcwalker3 referenced this pull request

2026-07-08 02:25:58 -05:00

Reviewer mutation tools must use the active branches worktree, not stale MCP server process root #503

jcwalker3 referenced this pull request

2026-07-08 02:25:58 -05:00

Reviewer mutation tools must use the active branches worktree, not stale MCP server process root #503

sysadmin commented

2026-07-08 02:39:59 -05:00

repo: Scaled-Tech-Consulting/Gitea-Tools
pr: #487
issue: none
reviewer_identity: sysadmin
profile: prgs-reviewer
session_id: 97274-676d20a825c4
worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr487
phase: claimed
candidate_head: 08202f7eaa
target_branch: master
target_branch_sha: none
last_activity: 2026-07-08T07:39:58Z
expires_at: 2026-07-08T09:39:58Z
blocker: none

repo: Scaled-Tech-Consulting/Gitea-Tools pr: #487 issue: none reviewer_identity: sysadmin profile: prgs-reviewer session_id: 97274-676d20a825c4 worktree: /Users/jasonwalker/Development/Gitea-Tools/branches/review-pr487 phase: claimed candidate_head: 08202f7eaa6d09b2eca8f2960126994a4b22646b target_branch: master target_branch_sha: none last_activity: 2026-07-08T07:39:58Z expires_at: 2026-07-08T09:39:58Z blocker: none

jcwalker3 referenced this pull request

2026-07-08 02:45:48 -05:00

Isolate MCP workspace binding across author/reviewer/merger namespaces #510

sysadmin merged commit b3edd53185 into master

2026-07-08 02:46:04 -05:00

jcwalker3 referenced this pull request

2026-07-08 03:07:35 -05:00

Block raw git branch-delete bypass outside authorized cleanup roles #514

jcwalker3 referenced this pull request

2026-07-08 03:07:45 -05:00

Add safe post-merge moot lease cleanup for already-merged PRs #515

sysadmin referenced this pull request