fix: surface skipped tracker cleanup on merge read-back failure (#98) #99

2026-07-02T15:16:17-05:00

jcwalker3 commented

2026-07-02 15:16:17 -05:00

Fixes #98. Found by the read-only gitea_merge_pr()/cleanup_in_progress_for_pr() trace.

Summary

gitea_merge_pr ran tracker cleanup inside the same try as the post-merge read-back GET. If the merge POST succeeded but the read-back failed, cleanup was silently skipped: merge_commit: null, no cleanup_status key, no signal — status:in-progress could stay stuck while the merge read as full success.

Behavior before / after

Scenario	Before	After
Merge OK, read-back OK	cleanup runs, `cleanup_status` set	unchanged
Merge OK, read-back fails	cleanup silently skipped; `cleanup_status` absent	`cleanup_status: "skipped (merge read-back failed)"`; merge still `performed: true`
Merge OK, cleanup raises	inner `except` swallowed it and (wrongly) nulled `merge_commit`	`cleanup_status: "skipped (cleanup error: <redacted>)"`; `merge_commit` preserved

Design choices (per issue notes): cleanup stays keyed on the read-back payload — the eligibility result doesn't carry title/body/branch, and pre-fetching before the merge POST would add a happy-path API call and re-sequence every mocked test. Cleanup remains never a merge blocker. _redact kept on all surfaced error text.

Files changed

mcp_server.py — split read-back vs. cleanup handling in gitea_merge_pr (+16/−3 lines, one function)
tests/test_mcp_server.py — 2 regression tests: read-back failure ⇒ explicit skip + performed: true + no tracker DELETE traffic (call-count pinned at 4); cleanup exception ⇒ surfaced + redacted (secret-xyz never appears)

Validation

git diff --check — clean
python3 -m py_compile mcp_server.py manage_labels.py gitea_auth.py — OK
bash -n scripts/clear-provenance — OK
Targeted: 2 new tests pass; -k "TestMergePR or TrackerHygiene" — 26 passed
Full: pytest tests/ -q — 357 passed (355 → +2)
Secret sweep (staged diff; no repo scanner) — clean

Risk notes

Happy-path API call sequence unchanged (no test re-sequencing needed).
Micro behavior change: an empty (None-body) read-back now reports the explicit skip instead of running cleanup against {} (which produced a misleading "no linked issue found") — strictly more accurate.
Audit unchanged: @_audit_pr_result still classifies from the result dict.

Confirmations

No secrets changed or exposed · no release/tag changes (v1.1.0 untouched) · no self-review · no merge performed by this session. #66 (Docker tests, claimed by another worker) and #75 (umbrella) deliberately not bundled.

🤖 Generated with Claude Code

Fixes #98. Found by the read-only `gitea_merge_pr()`/`cleanup_in_progress_for_pr()` trace. ## Summary `gitea_merge_pr` ran tracker cleanup inside the same `try` as the post-merge read-back GET. If the merge POST succeeded but the read-back failed, cleanup was **silently skipped**: `merge_commit: null`, no `cleanup_status` key, no signal — `status:in-progress` could stay stuck while the merge read as full success. ## Behavior before / after | Scenario | Before | After | |---|---|---| | Merge OK, read-back OK | cleanup runs, `cleanup_status` set | unchanged | | Merge OK, read-back **fails** | cleanup silently skipped; `cleanup_status` absent | `cleanup_status: "skipped (merge read-back failed)"`; merge still `performed: true` | | Merge OK, cleanup raises | inner `except` swallowed it and (wrongly) nulled `merge_commit` | `cleanup_status: "skipped (cleanup error: <redacted>)"`; `merge_commit` preserved | Design choices (per issue notes): cleanup stays keyed on the read-back payload — the eligibility result doesn't carry title/body/branch, and pre-fetching before the merge POST would add a happy-path API call and re-sequence every mocked test. Cleanup remains **never** a merge blocker. `_redact` kept on all surfaced error text. ## Files changed - `mcp_server.py` — split read-back vs. cleanup handling in `gitea_merge_pr` (+16/−3 lines, one function) - `tests/test_mcp_server.py` — 2 regression tests: read-back failure ⇒ explicit skip + `performed: true` + no tracker DELETE traffic (call-count pinned at 4); cleanup exception ⇒ surfaced + redacted (`secret-xyz` never appears) ## Validation - `git diff --check` — clean - `python3 -m py_compile mcp_server.py manage_labels.py gitea_auth.py` — OK - `bash -n scripts/clear-provenance` — OK - Targeted: 2 new tests pass; `-k "TestMergePR or TrackerHygiene"` — 26 passed - Full: `pytest tests/ -q` — **357 passed** (355 → +2) - Secret sweep (staged diff; no repo scanner) — clean ## Risk notes - Happy-path API call sequence unchanged (no test re-sequencing needed). - Micro behavior change: an *empty* (None-body) read-back now reports the explicit skip instead of running cleanup against `{}` (which produced a misleading "no linked issue found") — strictly more accurate. - Audit unchanged: `@_audit_pr_result` still classifies from the result dict. ## Confirmations - No secrets changed or exposed · no release/tag changes (v1.1.0 untouched) · no self-review · no merge performed by this session. #66 (Docker tests, claimed by another worker) and #75 (umbrella) deliberately not bundled. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jcwalker3 added 1 commit 2026-07-02 15:16:17 -05:00

fix: surface skipped tracker cleanup on merge read-back failure (#98 ) 8120486109

gitea_merge_pr ran cleanup_in_progress_for_pr inside the same try as the
post-merge read-back GET; a read-back failure silently skipped tracker
cleanup, leaving only merge_commit=null and no cleanup_status at all, so
status:in-progress could stay stuck while the merge read as full success.

Split the block: read-back failure now returns an explicit
cleanup_status='skipped (merge read-back failed)', and an unexpected
cleanup exception returns 'skipped (cleanup error: <redacted>)' instead of
masking merge_commit. Cleanup still never blocks a performed merge, the
happy-path API call sequence is unchanged, and _redact keeps credentials
out of surfaced errors.

Add regression tests: read-back failure => merge still performed, explicit
skip status, no tracker DELETE traffic; cleanup exception => surfaced and
redacted.

Fixes #98.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

sysadmin reviewed 2026-07-02 15:32:35 -05:00

sysadmin left a comment

Independent review from a detached worktree at pinned head 8120486.

Diff scope exact: mcp_server.py (gitea_merge_pr read-back/cleanup split only) + tests/test_mcp_server.py (2 regression tests). No secrets, no release/tag changes, no unrelated refactors.
Behavior verified against source: merge success preserved; cleanup never blocks a performed merge; read-back failure now yields explicit cleanup_status "skipped (merge read-back failed)"; unexpected cleanup exception surfaces redacted via _redact and no longer masks merge_commit; happy-path API call sequence unchanged; audit classification unaffected (skip status lives in cleanup_status, not reasons).
Independent validation: py_compile (mcp_server/manage_labels/gitea_auth) OK; bash -n scripts/clear-provenance OK; new regression tests 2/2; TestMergePR+TrackerHygiene 26/26; full suite 357 passed; git diff --check clean; secret sweep on the PR diff clean (only the intentional fake fixture string in a test).
Branch is 0 behind master (02c0c20), mergeable, head unchanged.

Review Metadata:

LLM-Role: reviewer
Authenticated-Gitea-User: sysadmin
MCP-Profile: prgs-reviewer
Eligibility: passed (author jcwalker3 != reviewer)

Approving.

Independent review from a detached worktree at pinned head 8120486. - Diff scope exact: mcp_server.py (gitea_merge_pr read-back/cleanup split only) + tests/test_mcp_server.py (2 regression tests). No secrets, no release/tag changes, no unrelated refactors. - Behavior verified against source: merge success preserved; cleanup never blocks a performed merge; read-back failure now yields explicit cleanup_status "skipped (merge read-back failed)"; unexpected cleanup exception surfaces redacted via _redact and no longer masks merge_commit; happy-path API call sequence unchanged; audit classification unaffected (skip status lives in cleanup_status, not reasons). - Independent validation: py_compile (mcp_server/manage_labels/gitea_auth) OK; bash -n scripts/clear-provenance OK; new regression tests 2/2; TestMergePR+TrackerHygiene 26/26; full suite 357 passed; git diff --check clean; secret sweep on the PR diff clean (only the intentional fake fixture string in a test). - Branch is 0 behind master (02c0c20), mergeable, head unchanged. Review Metadata: - LLM-Role: reviewer - Authenticated-Gitea-User: sysadmin - MCP-Profile: prgs-reviewer - Eligibility: passed (author jcwalker3 != reviewer) Approving.

sysadmin merged commit cdc32669c7 into master

2026-07-02 15:32:47 -05:00

sysadmin referenced this pull request

2026-07-02 18:47:12 -05:00

feat: profiles.json v2 parser with validation invariants (#103) #114

sysadmin referenced this pull request

2026-07-02 18:48:11 -05:00

docs: compact Controller Handoff as default format (#108) #115

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#99