feat: comment-then-stop policy for partial PR reconciliation (Closes #302) #375

2026-07-07T04:47:50-05:00

jcwalker3 commented

2026-07-07 04:47:50 -05:00

Summary

Closes #302.

Already-landed PR reconciliation could prove a PR should be closed but then stop silently when gitea.pr.close was unavailable — even though PR-comment capability was proven. This PR adopts Option B (comment-then-stop) as the durable policy, enforces it in tooling, and documents it in the canonical reconciliation workflow.

Policy decision

Option B chosen over strict all-or-nothing: a reconciliation comment carrying the ancestor proof leaves durable evidence on the PR, prevents the next reviewer run from re-deriving everything, and still performs no close without exact capability. This matches the observed failure, where silence cost the queue a full re-run.

Changes

review_proofs.py
- resolve_partial_reconciliation_plan(ancestor_proven, capabilities) maps proven capabilities to one of four outcomes:
  - FULL_RECONCILE_CLOSE_ALLOWED — close_pr proven; close and report the result.
  - PARTIAL_RECONCILE_COMMENT_THEN_STOP — close_pr missing, comment_pr proven; post exactly one reconciliation comment with PR head SHA, target branch SHA, ancestor proof, linked issue status, and the required missing capability, then stop for an authorized close.
  - RECOVERY_HANDOFF_ONLY — no comment capability either; no Gitea mutation, recovery handoff records the intended comment.
  - GATE_NOT_PROVEN — ancestry unproven; no mutation regardless of capability.
  - capabilities=None or missing keys fail closed.
- assess_partial_reconciliation_report(report_text, plan) requires final reports to explain why the comment was or was not posted, name the missing capability, and state the close result (or that no mutation ran).
- New exported constants PARTIAL_RECONCILIATION_POLICY and PARTIAL_RECONCILIATION_COMMENT_FIELDS.
skills/llm-project-workflow/workflows/reconcile-landed-pr.md — new §12A documenting the policy and outcome states.
tests/test_llm_workflow_split.py — contract test locks the §12A markers.
tests/test_review_proofs.py — TestPartialReconciliationPlan (6 tests) and TestPartialReconciliationReport (3 tests).

Acceptance criteria mapping (#302)

Policy is documented in the PR reconciliation workflow (§12A, contract-tested).
Tooling enforces the chosen policy (resolve_partial_reconciliation_plan, fail-closed).
Final reports explain why comments were or were not posted (assess_partial_reconciliation_report).
Tests cover close capability present, close missing with comment allowed, comment capability missing, and all capabilities missing.

Validation

python -m pytest tests/ in the issue worktree (base f87101c): 1242 passed, 2 failed, 6 skipped, 20 subtests passed.

Both failures are pre-existing on the clean base with this change stashed: test_agent_temp_artifacts.py::TestIssueLockArtifactWarning::test_lock_success_includes_artifact_warning (tracked by #359, fix PR #364 open) and test_final_report_validator.py::TestReviewPrRules::test_clean_reviewer_report_passes (introduced by a recently merged PR on master; fails identically without this diff).

🤖 Generated with Claude Code

## Summary Closes #302. Already-landed PR reconciliation could prove a PR should be closed but then stop silently when `gitea.pr.close` was unavailable — even though PR-comment capability was proven. This PR adopts **Option B (comment-then-stop)** as the durable policy, enforces it in tooling, and documents it in the canonical reconciliation workflow. ## Policy decision Option B chosen over strict all-or-nothing: a reconciliation comment carrying the ancestor proof leaves durable evidence on the PR, prevents the next reviewer run from re-deriving everything, and still performs no close without exact capability. This matches the observed failure, where silence cost the queue a full re-run. ## Changes - `review_proofs.py` - `resolve_partial_reconciliation_plan(ancestor_proven, capabilities)` maps proven capabilities to one of four outcomes: - `FULL_RECONCILE_CLOSE_ALLOWED` — `close_pr` proven; close and report the result. - `PARTIAL_RECONCILE_COMMENT_THEN_STOP` — `close_pr` missing, `comment_pr` proven; post exactly one reconciliation comment with PR head SHA, target branch SHA, ancestor proof, linked issue status, and the required missing capability, then stop for an authorized close. - `RECOVERY_HANDOFF_ONLY` — no comment capability either; no Gitea mutation, recovery handoff records the intended comment. - `GATE_NOT_PROVEN` — ancestry unproven; no mutation regardless of capability. - `capabilities=None` or missing keys fail closed. - `assess_partial_reconciliation_report(report_text, plan)` requires final reports to explain why the comment was or was not posted, name the missing capability, and state the close result (or that no mutation ran). - New exported constants `PARTIAL_RECONCILIATION_POLICY` and `PARTIAL_RECONCILIATION_COMMENT_FIELDS`. - `skills/llm-project-workflow/workflows/reconcile-landed-pr.md` — new §12A documenting the policy and outcome states. - `tests/test_llm_workflow_split.py` — contract test locks the §12A markers. - `tests/test_review_proofs.py` — `TestPartialReconciliationPlan` (6 tests) and `TestPartialReconciliationReport` (3 tests). ## Acceptance criteria mapping (#302) - [x] Policy is documented in the PR reconciliation workflow (§12A, contract-tested). - [x] Tooling enforces the chosen policy (`resolve_partial_reconciliation_plan`, fail-closed). - [x] Final reports explain why comments were or were not posted (`assess_partial_reconciliation_report`). - [x] Tests cover close capability present, close missing with comment allowed, comment capability missing, and all capabilities missing. ## Validation `python -m pytest tests/` in the issue worktree (base f87101c): **1242 passed, 2 failed, 6 skipped, 20 subtests passed**. Both failures are pre-existing on the clean base with this change stashed: `test_agent_temp_artifacts.py::TestIssueLockArtifactWarning::test_lock_success_includes_artifact_warning` (tracked by #359, fix PR #364 open) and `test_final_report_validator.py::TestReviewPrRules::test_clean_reviewer_report_passes` (introduced by a recently merged PR on master; fails identically without this diff). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jcwalker3 added 1 commit 2026-07-07 04:47:51 -05:00

feat: comment-then-stop policy for partial PR reconciliation (Closes #302 ) 852ac1f56b

Already-landed PR reconciliation could prove a PR should be closed but
stop silently when gitea.pr.close was unavailable, even with comment
capability present. Adopt Option B (comment-then-stop) as the durable
policy and enforce it:

- resolve_partial_reconciliation_plan maps proven capabilities to one
  of four outcomes: FULL_RECONCILE_CLOSE_ALLOWED (close_pr proven),
  PARTIAL_RECONCILE_COMMENT_THEN_STOP (comment_pr proven, close_pr
  missing; one reconciliation comment with PR head SHA, target branch
  SHA, ancestor proof, linked issue status, and the missing capability,
  then stop), RECOVERY_HANDOFF_ONLY (no comment capability; no Gitea
  mutation), and GATE_NOT_PROVEN (no ancestry proof; no mutation
  regardless of capability). Missing capability dicts fail closed.
- assess_partial_reconciliation_report requires final reports to
  explain why the comment was or was not posted, name the missing
  capability, and state the close result or that no mutation ran.
- workflows/reconcile-landed-pr.md gains section 12A documenting the
  policy; the workflow contract test locks the new markers.

Tests cover close capability present, close missing with comment
allowed, comment capability missing, all capabilities missing,
unproven ancestry, fail-closed capability dict, and report checks for
each outcome.

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

jcwalker3 referenced this pull request

2026-07-07 04:48:12 -05:00

Define partial reconciliation behavior when PR close capability is missing #302

jcwalker3 referenced this issue from a commit

2026-07-07 08:15:40 -05:00

fix: resolve conflicts for PR #375 against current master

jcwalker3 added 1 commit 2026-07-07 08:15:40 -05:00

fix: resolve conflicts for PR #375 against current master bbf5ee6d59

jcwalker3 referenced this issue from a commit

2026-07-07 08:31:53 -05:00

fix: resolve conflicts for PR #375 against latest master

jcwalker3 added 1 commit 2026-07-07 08:31:53 -05:00

fix: resolve conflicts for PR #375 against latest master ba4cf2e93d

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

jcwalker3 referenced this pull request

2026-07-07 08:33:00 -05:00

feat: enforce canonical reconciliation handoff schema (Closes #307) #387

sysadmin approved these changes 2026-07-07 08:51:01 -05:00

sysadmin left a comment

All tests pass successfully after merging master (1378 passed, 1 failed [pre-existing]). Reviewed the implementation of resolve_partial_reconciliation_plan and assess_partial_reconciliation_report in review_proofs.py, the workflow documentation in skills/llm-project-workflow/workflows/reconcile-landed-pr.md, and the test coverage in tests/test_review_proofs.py. It correctly implements Option B (comment-then-stop) as the durable policy for partial reconciliation. Verified that the remaining failure is pre-existing on master.

All tests pass successfully after merging master (1378 passed, 1 failed [pre-existing]). Reviewed the implementation of `resolve_partial_reconciliation_plan` and `assess_partial_reconciliation_report` in `review_proofs.py`, the workflow documentation in `skills/llm-project-workflow/workflows/reconcile-landed-pr.md`, and the test coverage in `tests/test_review_proofs.py`. It correctly implements Option B (comment-then-stop) as the durable policy for partial reconciliation. Verified that the remaining failure is pre-existing on master.

sysadmin merged commit 1c506fa779 into master

2026-07-07 08:51:13 -05:00

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