c908ed6050895d50d99a7f5105976d38a8028ad4
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
fde95b9266 |
fix(mcp): chain-scoped newest-wins reviewer lease reads in pr_work_lease (#742)
Second author remediation on PR #743. Fixes the full-ledger defect surfaced by the first remediation. Root cause: pr_work_lease.find_active_reviewer_lease iterated the reviewer markers newest-first and used `continue` on a terminal marker. On a realistic append-only ledger (claimed -> validating -> terminal) it therefore stepped over the terminal marker and returned the older claim of the very chain that marker had just ended. reviewer_pr_lease got strict newest-wins under #577; pr_work_lease never did, so the two modules disagreed for released, blocked, done, and abandoned alike, and merger owner finalization did not leave "no active lease" for pr_work_lease consumers (conflict-fix acquire, PR sync inventory). Fix: a claim is skipped when a later marker terminates its own chain. Chain identity is (repo, pr_number, candidate_head, session_id, reviewer_identity, profile) via the new _reviewer_chain_key; _chain_terminated_after scans only markers appended after the candidate. Consequences: - a valid terminal marker ends its matching earlier claim (no resurrection); - a foreign-session, wrong-repo, wrong-PR, wrong-head, wrong-identity, or wrong-profile terminal marker cannot cancel another session's active lease, which strict newest-wins alone would have allowed; - a malformed terminal marker has no provable chain key, so it cancels nothing and cannot hide a valid active claim; - expiry, freshness, phase sets, ownership and integrity checks, both parsers, and find_active_conflict_fix_lease are untouched; - history stays append-only; no marker is deleted or rewritten. Reviewer lease markers carry no token field, so token-fingerprint validation remains where it already lives (session provenance, merger finalization) and is not weakened here. Tests: new TestFullLedgerNewestWins in tests/test_merger_lease_finalization.py covers claimed->released/blocked/done/abandoned, claimed->validating->terminal, foreign-session and mismatched repo/PR/head/identity/profile terminals, the malformed terminal marker, a newer active chain surviving an older terminated chain, newest-valid-chain selection across multiple histories, all-chains- terminated, single-marker parity between the two modules across eight phases, expired-claim/freshness non-regression, and the real ledger written by gitea_release_merger_pr_lease reading as terminal in both modules with the prior marker preserved. TestCrossModuleTerminalPhaseAgreement's scope-limited placeholder test is replaced by a real both-modules full-ledger assertion. Verified 10 of the new tests fail at the prior head |
||
|
|
7ae5f3a541 |
fix(mcp): mirror abandoned terminal phase into pr_work_lease (#742 review 460)
Addresses REQUEST_CHANGES review 460 on PR #743 @ |
||
|
|
22d0fdd251 |
fix(mcp): accept acquired merger-lease provenance and add owner finalization (Closes #742)
Root cause (confirmed on PR #740, session 33780-7168cbeeba58, marker 12354): merger_lease_adoption.SANCTIONED_PROVENANCE_SOURCES already contained SOURCE_ACQUIRE_MERGER, so the membership check passed, but is_sanctioned_session_lease() branched only for SOURCE_ADOPT and for {SOURCE_ACQUIRE, SOURCE_HEARTBEAT} and fell through to `return False` for a lease minted by gitea_acquire_merger_pr_lease. assess_mutation_lease_gate() therefore appended "in-session lease lacks sanctioned provenance" and gitea_merge_pr fail-closed on the merger's own freshly acquired lease. describe_session_lease_proof() likewise had no branch for that source and reported the lease as lease_proof_kind=unsanctioned. Separately, no merger-role owner-session operation existed to end that lease, so a failed merger lease could only expire. A. Acquired merger provenance is_sanctioned_session_lease() now accepts SOURCE_ACQUIRE_MERGER, but only via the new assess_acquired_merger_lease_integrity(), which fails closed unless the in-session record is complete and self-consistent: comment marker present and matching between provenance and session, non-empty session id, holder identity, merger profile, repository, valid PR number, and a normalized 40-hex candidate_head. describe_session_lease_proof() reports the explicit kind sanctioned_acquire_merger. Manual _SESSION_LEASE seeding, forged or incomplete records, mismatched fields, and unknown provenance all remain rejected; reviewer-acquire, reviewer-heartbeat, and merger-adoption paths are untouched. B. Merger owner-session finalization New native tool gitea_release_merger_pr_lease terminally releases or abandons a merger's own comment-backed lease when the merge does not occur. It is merger-only (gitea.read + gitea.pr.comment + gitea.pr.merge; reviewer profiles lack pr.merge) with an explicit capability-map task release_merger_pr_lease / gitea_release_merger_pr_lease bound to gitea.pr.comment + role merger, and it is listed as role-exclusive in the resolver. assess_merger_lease_finalization() verifies exact session ownership, repository, PR, candidate head vs live head, profile, identity, marker presence on the thread, and the native runtime token fingerprint recorded at acquisition; foreign-session release, reviewer-profile use, and any mismatch fail closed. Finalization appends a terminal lease marker and never edits or deletes ledger history; an already-terminal lease returns already_terminal and posts nothing. The PR, approval, decision lock, branch, and worktree are all preserved. gitea_release_reviewer_pr_lease is unchanged and is not repurposed for merger sessions. "abandoned" is added to reviewer_pr_lease._TERMINAL_PHASES; without it an abandoned marker would fall through the generic non-empty phase branch and keep re-arming the lease as active. Tests: new tests/test_merger_lease_finalization.py (38 tests, 11 subtests) covers all twelve acceptance criteria — provenance sanctioning, explicit proof kind, merge-gate acceptance, manual-seed and unknown-provenance rejection, profile/role/session/head/repository/fingerprint mismatches, owner release, foreign-session refusal, reviewer refusal, append-only idempotent finalization, no surviving lease after finalization, and no regression in adoption or reviewer-lease behavior. The defect was reproduced on clean baseline |