From 6b7f7562f85e6612528f96bdb0ac960449ddcbc2 Mon Sep 17 00:00:00 2001 From: michael-wojcik <5386199+michael-wojcik@users.noreply.github.com> Date: Sun, 5 Jul 2026 02:24:06 -0400 Subject: [PATCH 1/4] fix(merge-guard): raise Layer-1 retirement granularity to op+target+flags (#1100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _retire_token_for_command now discriminates on bound_flags in addition to op+target+session: a successful bare same-op/same-target command no longer retires an operator's approved escalated (flag-carrying) token — the #1100 over-retirement friction. Target and the flag set are derived from ONE leg-isolated extract_command_context call, mirroring the read arm, so mint, read, and retirement agree on (op, target, bound_flags) identity by construction (same _target_value + same extract_privileged_flags SSOT). The discriminator is conjunctive and monotonic (retires fewer tokens, never more), so the only residual is a tolerated under-retire backstopped by TTL + MAX_USES; as a PostToolUse observer it can never over-block a faithful click. Tests: new TestFlagAwareRetirement (self-consume incl. flagless, friction-cured survival both directions, flag-normalization equivalence, seam-independent and SSOT-neuter non-vacuity, both-modes session matrix) plus a post->pre seam test. Update the pre-existing target-axis non-vacuity probe to a flagless token so it isolates the target predicate under the new flag axis. --- pact-plugin/hooks/merge_guard_post.py | 53 +++- pact-plugin/tests/test_merge_guard.py | 262 +++++++++++++++++- .../test_merge_guard_seam_integration.py | 32 ++- 3 files changed, 336 insertions(+), 11 deletions(-) diff --git a/pact-plugin/hooks/merge_guard_post.py b/pact-plugin/hooks/merge_guard_post.py index eb1feaae..bdddf718 100644 --- a/pact-plugin/hooks/merge_guard_post.py +++ b/pact-plugin/hooks/merge_guard_post.py @@ -657,6 +657,20 @@ def _retire_token_for_command( token's stored `operation_type` for symmetric retirement across merge/close/branch-delete/force-push. + Match granularity (#1097 target-aware + #1100 flag-aware): a token is + retired ONLY when it agrees with the executed command on op_type AND + target AND bound_flags AND session. Target and flags are derived from ONE + leg-isolated extract_command_context call, so retirement, mint, and read + agree on identity BY CONSTRUCTION (the SAME _target_value + the SAME + extract_privileged_flags SSOT the mint/read use). This finer match cures + the over-retirement friction where a bare same-op/same-target command + burned an operator's approved ESCALATED (flag-carrying) token, forcing a + re-approval. The discriminator is conjunctive and MONOTONIC — it retires + FEWER tokens, never more — so the only residual is a tolerated under-retire + (a genuine self-consume still retires; an unrelated command's leftover + token is backstopped by TTL + MAX_USES). As a PostToolUse observer it can + never over-block a faithful click. + Emits a path-annotated stderr forensic log when retirement is observed (BC-NIT addressed: log line distinguishes "direct" rename-by-this-session from "race-recover" observed-by-this-session @@ -682,13 +696,24 @@ def _retire_token_for_command( token_dir = TOKEN_DIR pattern = str(token_dir / f"{TOKEN_PREFIX}*") current_session = get_session_id() - # #1097: target-aware retirement. Bring the match up to the granularity the mint - # records — retire ONLY the token for the SPECIFIC operation (same op AND same - # target), not any same-op token in the session. Uses the SAME _target_value the - # mint/read derive, so retirement, mint, and read agree on target identity by - # construction. (The API-merge form reuses pr_number per #1096, so this covers it - # uniformly with zero extra code.) - cmd_target = _target_value(extract_command_context(command)) + # #1097 + #1100: op+target+bound_flags retirement. Bring the match up to the + # granularity the mint records — retire ONLY the token for the SPECIFIC operation + # (same op AND same target AND same flag set), not any same-op token in the session. + # Both target and the flag set are derived from ONE leg-isolated + # extract_command_context call, mirroring the read arm (merge_guard_pre.py:536-540), + # so retirement, mint, and read agree on (op, target, bound_flags) identity by + # construction — the SAME _target_value + the SAME extract_privileged_flags SSOT. + # Compounds are refused at mint, so the single-leg selection collapses to the whole + # command for every real token; the `or command` tail preserves the pre-fix whole- + # command behavior when neither leg selector binds. (The API-merge form reuses + # pr_number per #1096, so target coverage stays uniform with zero extra code.) + cmd_ctx = extract_command_context( + _single_destructive_leg(command) + or _single_detectable_leg(command) + or command + ) + cmd_target = _target_value(cmd_ctx) + cmd_flags = set(cmd_ctx.get("bound_flags", [])) for path in glob.glob(pattern): basename = os.path.basename(path) # Skip terminal-rename siblings and per-use markers (mirrors the @@ -724,6 +749,20 @@ def _retire_token_for_command( token_target = _target_value(ctx) if token_target is not None and cmd_target != token_target: continue + # #1100: bound_flags axis — retire ONLY when the executed command's binding- + # relevant flag set EXACTLY equals the token's approved set. Byte-mirrors the + # read arm's #1042 set-equality (merge_guard_pre.py:559). UNCONDITIONAL — unlike + # the target skip above (which falls back to op+session for a target-LESS legacy + # token), the empty set is a first-class flag universe here (a flagless base + # command), so [] == [] correctly retires a flagless self-consume while a bare + # `gh pr close 5` (cmd_flags == set()) no longer burns an approved ESCALATED + # {close,5,[--delete-branch]} token. Conjunctive + MONOTONIC: this can only + # retire FEWER tokens, so the sole residual is a tolerated under-retire + # (backstopped by TTL + MAX_USES) — never an over-block (PostToolUse observer). + # A token-has-flags guard would be WRONG: it would let a flagged command retire + # a flagless token, breaking the symmetric identity match. + if set(ctx.get("bound_flags", [])) != cmd_flags: + continue # Session scoping (SEC-S1 cycle-2 revised asymmetric predicate). token_session = token_data.get("session_id", "") if current_session: diff --git a/pact-plugin/tests/test_merge_guard.py b/pact-plugin/tests/test_merge_guard.py index 00e5f554..13836e88 100644 --- a/pact-plugin/tests/test_merge_guard.py +++ b/pact-plugin/tests/test_merge_guard.py @@ -13160,13 +13160,21 @@ def test_target_predicate_non_vacuous_under_target_value_neuter( worktree.""" import merge_guard_post as mgp - # direction 1 — fix present: the unrelated command does not retire + # direction 1 — fix present: the unrelated command does not retire. + # FLAGLESS token + bare command: with the #1100 flag axis now also live, + # the token and command flag-sets must be EQUAL here (both empty) so this + # test isolates the TARGET predicate — otherwise a flag mismatch would be + # a second, independent blocker and the target-neuter alone could not + # restore coarse retirement (the #1100/#1097 interaction). Survival in + # direction 1 is therefore attributable solely to the target mismatch. mgp.write_token({"operation_type": "close", "pr_number": "42", - "bound_flags": ["--delete-branch"]}, token_dir=tmp_path) + "bound_flags": []}, token_dir=tmp_path) assert mgp._retire_token_for_command( "gh pr close 43", "close", token_dir=tmp_path) is False assert len(self._live(tmp_path)) == 1 - # direction 2 — pre-fix surface restored: the coarse retirement returns + # direction 2 — pre-fix surface restored: the coarse retirement returns. + # Flags stay equal ([] == []) so neutering _target_value is the ONLY + # change, and it flips survive -> retire. monkeypatch.setattr(mgp, "_target_value", lambda ctx: None) assert mgp._retire_token_for_command( "gh pr close 43", "close", token_dir=tmp_path) is True, ( @@ -13176,6 +13184,254 @@ def test_target_predicate_non_vacuous_under_target_value_neuter( assert len(self._live(tmp_path)) == 0 +class TestFlagAwareRetirement: + """#1100 flag-aware Layer-1 token retirement — the bidirectional cert. + + #1097 raised the retirement match key to op+target; #1100 raises it once + more to op+target+bound_flags so a successful BARE same-op/same-target + command no longer retires an operator's approved ESCALATED (flag-carrying) + token — the bounded-friction OVER-BLOCK. `_retire_token_for_command` + reuses the mint-stored `bound_flags` and the SAME `extract_privileged_flags` + SSOT the read arm (merge_guard_pre) set-compares, adding an unconditional + `set(ctx.bound_flags) != cmd_flags` skip AFTER the existing target check. + + Governing principle (task-level SACROSANCT): retirement is a PostToolUse + OBSERVER — it can only fail-to-retire, never over-block a faithful click. + The flag axis is a pure ADDITIONAL conjunctive skip, so: + - PRIMARY (inviolable): an escalated token survives a bare same-target + command (the #1100 cure) — RED on pre-#1100 main, GREEN after. + - SECONDARY (no-new-under-block): a genuine self-consume (matching + flags, INCLUDING the flagless case) STILL retires. + + Tokens are minted THROUGH `extract_command_context` (the production SSOT) + so their stored `bound_flags` are scanner-derived, not hand-picked + literals — this is what lets the row-4 neuter collapse BOTH the command- + side and token-side flag sets symmetrically. In-memory only (write_token / + _retire_token_for_command with token_dir=tmp_path); NO git mutation in the + shared worktree (#1097 precedent).""" + + def _live(self, tmp_path): + return [t for t in tmp_path.glob("merge-authorized-*") + if not t.name.endswith(".consumed") and ".use-" not in t.name] + + @staticmethod + def _mint_from(cmd, tmp_path): + """Mint a token whose context (incl. bound_flags) is derived by the + production SSOT extract_command_context — faithful to what a real + approval of `cmd` would store, so the flag set is scanner-derived + rather than a hand-picked literal.""" + from merge_guard_post import write_token, extract_command_context + + path = write_token(extract_command_context(cmd), token_dir=tmp_path) + assert path is not None, f"mint failed for {cmd!r}" + return path + + # --- SECONDARY (no-new-under-block): matching-flag self-consume STILL retires --- + + @pytest.mark.parametrize( + "mint_cmd,retire_cmd,op", + [ + ("gh pr merge 42 --admin", "gh pr merge 42 --admin", "merge"), + ("gh pr close 42 --delete-branch", + "gh pr close 42 --delete-branch", "close"), + ("git push --no-verify origin main --force", + "git push --no-verify origin main --force", "force-push"), + ("git push --force-with-lease origin main", + "git push --force-with-lease origin main", "push-to-main"), + # FLAGLESS self-consume — guards an over-strict "token must have + # flags" implementation: a bare-approved bare-executed op MUST + # still retire (empty set == empty set). + ("gh pr close 5", "gh pr close 5", "close"), + ], + ids=["merge_admin", "close_delete_branch", "force_push_no_verify", + "push_to_main_lease", "flagless_close"], + ) + def test_matching_flag_self_consume_still_retires( + self, tmp_path, mint_cmd, retire_cmd, op + ): + """No-new-under-block: an execution whose flag-set EQUALS the approved + token's (incl. the empty set) retires its own token — the + one-approval-one-operation discipline holds across every op-class.""" + from merge_guard_post import _retire_token_for_command + + self._mint_from(mint_cmd, tmp_path) + retired = _retire_token_for_command(retire_cmd, op, token_dir=tmp_path) + assert retired is True, ( + f"NEW UNDER-BLOCK: self-consume stopped retiring for {op} ({retire_cmd!r})" + ) + assert len(self._live(tmp_path)) == 0 + + # --- PRIMARY (inviolable): flag mismatch at same op+target SURVIVES (#1100 cure) --- + + @pytest.mark.parametrize( + "mint_cmd,retire_cmd,op", + [ + # Escalated token, bare execution — the operator's approved + # --delete-branch token must NOT burn on a bare `gh pr close 5`. + ("gh pr close 5 --delete-branch", "gh pr close 5", "close"), + # Reciprocal: bare token, escalated execution — the bare token + # is a distinct identity from the --delete-branch variant. + ("gh pr close 5", "gh pr close 5 --delete-branch", "close"), + # merge/--admin mirror, both directions. + ("gh pr merge 42 --admin", "gh pr merge 42", "merge"), + ("gh pr merge 42", "gh pr merge 42 --admin", "merge"), + ], + ids=["escalated_survives_bare_close", "bare_survives_escalated_close", + "escalated_survives_bare_merge", "bare_survives_escalated_merge"], + ) + def test_flag_mismatch_same_target_survives( + self, tmp_path, mint_cmd, retire_cmd, op + ): + """#1100 CURED (RED on pre-#1100 main): same op + same target but a + DIFFERENT flag-set must NOT retire — the bare and escalated forms are + distinct token identities. Pre-#1100 retirement ignored flags, so a + bare command over-retired the escalated token (the bounded friction).""" + from merge_guard_post import _retire_token_for_command + + self._mint_from(mint_cmd, tmp_path) + retired = _retire_token_for_command(retire_cmd, op, token_dir=tmp_path) + assert retired is False, ( + f"#1100 OVER-BLOCK: {retire_cmd!r} retired the differently-flagged " + f"{mint_cmd!r} token" + ) + assert len(self._live(tmp_path)) == 1, "the escalated/bare token must survive" + + # --- flag-normalization symmetry: equal NORMALIZED sets retire (reuse of + # extract_privileged_flags, not a literal-key compare) --- + + @pytest.mark.parametrize( + "mint_cmd,retire_cmd,op", + [ + # -d is the short alias of --delete-branch: same canonical set. + ("gh pr close 42 --delete-branch", "gh pr close 42 -d", "close"), + # --repo value across =-joined / space / attached-short spellings. + ("gh pr close 7 --repo=o/r", "gh pr close 7 --repo o/r", "close"), + ("gh pr close 7 --repo=o/r", "gh pr close 7 -Ro/r", "close"), + # Reordered multi-flag: sets are order-insensitive. + ("gh pr merge 9 --repo=o/r --admin", + "gh pr merge 9 --admin --repo=o/r", "merge"), + # Duplicate flag collapses to the same singleton set. + ("gh pr merge 9 --admin", "gh pr merge 9 --admin --admin", "merge"), + # A benign non-privileged flag (--json) is not part of the bound set. + ("gh pr merge 9 --admin", "gh pr merge 9 --admin --json number", "merge"), + # Value-bearing round-trip: identical --repo value retires. + ("gh pr close 7 --repo=o/r", "gh pr close 7 --repo=o/r", "close"), + ], + ids=["delete_branch_long_vs_short", "repo_eq_vs_space", + "repo_eq_vs_attached_short", "multi_flag_reordered", "duplicate_flag", + "benign_json_alongside", "repo_value_round_trip"], + ) + def test_normalized_flag_equivalence_retires( + self, tmp_path, mint_cmd, retire_cmd, op + ): + """The approved and executed forms differ TEXTUALLY but normalize to the + SAME privileged-flag set, so the self-consume still retires. A literal + string compare (not reuse of extract_privileged_flags) would MISMATCH + `-d` vs `--delete-branch` and wrongly leave the token live.""" + from merge_guard_post import _retire_token_for_command + + self._mint_from(mint_cmd, tmp_path) + retired = _retire_token_for_command(retire_cmd, op, token_dir=tmp_path) + assert retired is True, ( + f"normalization broke: {retire_cmd!r} did not retire the equivalent " + f"{mint_cmd!r} token — flags compared as literals, not normalized sets" + ) + assert len(self._live(tmp_path)) == 0 + + def test_repo_value_mismatch_survives(self, tmp_path): + """The --repo VALUE is part of the bound-flag identity (a value-carrying + flag): approve `--repo=o/r`, execute `--repo=x/y` at the same target -> + distinct sets -> survives. RED on pre-#1100 main (flags ignored).""" + from merge_guard_post import _retire_token_for_command + + self._mint_from("gh pr close 7 --repo=o/r", tmp_path) + retired = _retire_token_for_command( + "gh pr close 7 --repo=x/y", "close", token_dir=tmp_path) + assert retired is False, "cross-repo value redirect retired the o/r token" + assert len(self._live(tmp_path)) == 1 + + # --- non-vacuity: the survival rows are coupled to the flag predicate --- + + def test_flag_value_is_load_bearing_black_box(self, tmp_path): + """SEAM-INDEPENDENT primary non-vacuity: holding op, target, and session + fixed, the retire/survive outcome flips on the flag-set ALONE — matching + flags retire, mismatched flags survive. Robust regardless of the hook's + internal extraction seam.""" + from merge_guard_post import _retire_token_for_command + + # matching flags -> retire + self._mint_from("gh pr close 5 --delete-branch", tmp_path) + assert _retire_token_for_command( + "gh pr close 5 --delete-branch", "close", token_dir=tmp_path) is True + assert len(self._live(tmp_path)) == 0 + # same op+target, mismatched flags -> survive (RED on pre-#1100 main) + self._mint_from("gh pr close 5 --delete-branch", tmp_path) + assert _retire_token_for_command( + "gh pr close 5", "close", token_dir=tmp_path) is False + assert len(self._live(tmp_path)) == 1 + + def test_flag_predicate_non_vacuous_under_extract_neuter( + self, tmp_path, monkeypatch + ): + """Counter-mutation mirroring the #1097 target-neuter: with the shared + `extract_privileged_flags` SSOT neutered to always-[], BOTH the + command's cmd_flags AND a freshly-minted token's stored bound_flags + collapse to the empty set, so the escalated-vs-bare distinction + disappears and the coarse op+target retirement returns — proving the + survival rows are coupled to the flag predicate, not vacuously green. + In-memory (monkeypatch) by design — no git mutation in the worktree.""" + from merge_guard_post import _retire_token_for_command + + # direction 1 — predicate present: escalated token survives bare command + self._mint_from("gh pr close 5 --delete-branch", tmp_path) + assert _retire_token_for_command( + "gh pr close 5", "close", token_dir=tmp_path) is False + assert len(self._live(tmp_path)) == 1 + # direction 2 — neuter the SSOT both arms derive flags from; re-mint so + # the token's stored bound_flags also collapse to [] (symmetric neuter) + monkeypatch.setattr( + "shared.merge_guard_common.extract_privileged_flags", + lambda *a, **k: []) + self._mint_from("gh pr close 5 --delete-branch", tmp_path) + assert _retire_token_for_command( + "gh pr close 5", "close", token_dir=tmp_path) is True, ( + "flag-extraction neuter did not restore coarse retirement — the " + "survival assertions above would be vacuous" + ) + assert len(self._live(tmp_path)) == 0 + + # --- both-modes: identical flag-axis behavior across session states --- + + @pytest.mark.parametrize( + "session_id", ["", "sess-A"], + ids=["empty_session", "populated_matching_session"], + ) + def test_flag_axis_identical_across_session_states( + self, tmp_path, monkeypatch, session_id + ): + """Dual-mode contract: the flag axis behaves identically whether + retirement runs with no PACT session (get_session_id()=='' -> the + session gate is skipped, graceful degradation) or a populated session + matching the token's own session_id. (This surface's session axis is + empty-vs-populated-matching — NOT leadSessionId topology, which this + hook does not key on.) Both the matching-flag self-consume (retire) and + the flag-mismatch friction (survive) are asserted under each state.""" + import merge_guard_post as mgp + from merge_guard_post import _retire_token_for_command + + monkeypatch.setattr(mgp, "get_session_id", lambda: session_id) + # matching flags -> retire, in both session states + self._mint_from("gh pr close 5 --delete-branch", tmp_path) + assert _retire_token_for_command( + "gh pr close 5 --delete-branch", "close", token_dir=tmp_path) is True + assert len(self._live(tmp_path)) == 0 + # flag mismatch -> survive, in both session states (RED on pre-#1100 main) + self._mint_from("gh pr close 5 --delete-branch", tmp_path) + assert _retire_token_for_command( + "gh pr close 5", "close", token_dir=tmp_path) is False + assert len(self._live(tmp_path)) == 1 + + class TestD1MatcherQuoteNormalization: """D1-matcher (#933): _token_matches_command normalizes surrounding quotes on the captured branch name at compare time, so a token branch `feat/x` diff --git a/pact-plugin/tests/test_merge_guard_seam_integration.py b/pact-plugin/tests/test_merge_guard_seam_integration.py index e98586a0..41cdd1ac 100644 --- a/pact-plugin/tests/test_merge_guard_seam_integration.py +++ b/pact-plugin/tests/test_merge_guard_seam_integration.py @@ -53,7 +53,11 @@ import pytest -from merge_guard_post import main as _post_main, write_token +from merge_guard_post import ( + main as _post_main, + write_token, + _retire_token_for_command, +) from merge_guard_pre import check_merge_authorization, main as _pre_main from shared.merge_guard_common import extract_command_context @@ -170,6 +174,32 @@ def test_no_token_holds_the_merge(self, tmp_path): merge is held.""" assert check_merge_authorization("gh pr merge 252", token_dir=tmp_path) is not None + def test_escalated_token_survives_bare_retire_and_still_authorizes(self, tmp_path): + """#1100 end-to-end over the real on-disk seam: an operator's approved + ESCALATED close token, minted to disk, survives a successful BARE + `gh pr close` (a distinct, non-branch-deleting op that drives the + PostToolUse retirement observer but mismatches on the flag-set) and then + STILL authorizes the operator's approved escalated re-execution. Nothing + stubbed: real write_token -> real _retire_token_for_command -> real + check_merge_authorization on one disk token. RED before the #1100 flag + axis — the bare retire would have burned the token, so the escalated + re-execution would then be held.""" + escalated = "gh pr close 5 --delete-branch" + context = extract_command_context(escalated) + assert context.get("bound_flags") == ["--delete-branch"] + write_token(context, token_dir=tmp_path) + + # A successful bare close must NOT retire the differently-flagged token. + retired = _retire_token_for_command( + "gh pr close 5", "close", token_dir=tmp_path) + assert retired is False + live = [t for t in tmp_path.glob("merge-authorized-*") + if not t.name.endswith(".consumed") and ".use-" not in t.name] + assert len(live) == 1, "the escalated token must survive the bare retire" + + # The surviving token still authorizes the approved escalated command. + assert check_merge_authorization(escalated, token_dir=tmp_path) is None + # ───────────────────────── L-B — in-process main()→main() ───────────────────── From 5d9e5a561bdd5582e634e8f1f4c0e5fffd86698c Mon Sep 17 00:00:00 2001 From: michael-wojcik <5386199+michael-wojcik@users.noreply.github.com> Date: Sun, 5 Jul 2026 02:54:50 -0400 Subject: [PATCH 2/4] docs(merge-guard): precise two-move framing in #1100 retirement docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the independent security review and an adversarial break-sweep flagged that the prior "conjunctive and MONOTONIC — retires fewer, never more" wording understated the change. It composes two independent, both-safe moves: a purely subtractive flag-inequality skip, plus a leg-isolated cmd_target derivation that purifies the compare toward the executed op's own identity (mirroring the read arm) and can retire a self-consume a prior whole-command mis-scan would have missed — the safe direction, since over-retire only consumes a token, never authorizes one. Docstring-only; behavior unchanged. --- pact-plugin/hooks/merge_guard_post.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/pact-plugin/hooks/merge_guard_post.py b/pact-plugin/hooks/merge_guard_post.py index bdddf718..876d6669 100644 --- a/pact-plugin/hooks/merge_guard_post.py +++ b/pact-plugin/hooks/merge_guard_post.py @@ -665,11 +665,19 @@ def _retire_token_for_command( extract_privileged_flags SSOT the mint/read use). This finer match cures the over-retirement friction where a bare same-op/same-target command burned an operator's approved ESCALATED (flag-carrying) token, forcing a - re-approval. The discriminator is conjunctive and MONOTONIC — it retires - FEWER tokens, never more — so the only residual is a tolerated under-retire - (a genuine self-consume still retires; an unrelated command's leftover - token is backstopped by TTL + MAX_USES). As a PostToolUse observer it can - never over-block a faithful click. + re-approval. Two independent, both-SAFE moves compose here, precisely: + the flag-inequality skip is purely SUBTRACTIVE (it only ever declines to + retire on a flag mismatch — fewer tokens on the flag axis); the + leg-isolated cmd_target derivation PURIFIES the compare toward the + executed op's OWN (op, target, flags) identity (mirroring the read arm, + merge_guard_pre.py:536-540) and so can retire a self-consume a prior + whole-command mis-scan would have MISSED — still the SAFE direction, since + over-retire only CONSUMES a token, never authorizes one. Net: laundering- + safe by construction — retirement only ever REMOVES tokens, never grants + authorization; the sole residual is a tolerated under-retire (a genuine + self-consume still retires; an unrelated command's leftover token is + backstopped by TTL + MAX_USES), and over-block stays structurally + IMPOSSIBLE (a PostToolUse observer renames after success, never gates). Emits a path-annotated stderr forensic log when retirement is observed (BC-NIT addressed: log line distinguishes "direct" From ab473873b75717345cfcd3efb20fc3fa6087e702 Mon Sep 17 00:00:00 2001 From: michael-wojcik <5386199+michael-wojcik@users.noreply.github.com> Date: Sun, 5 Jul 2026 02:57:14 -0400 Subject: [PATCH 3/4] chore(release): bump plugin version to 4.4.55 (#1100 retirement flag-granularity) --- .claude-plugin/marketplace.json | 2 +- README.md | 2 +- pact-plugin/.claude-plugin/plugin.json | 2 +- pact-plugin/README.md | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 128923a6..c196c943 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -12,7 +12,7 @@ "name": "PACT", "source": "./pact-plugin", "description": "Orchestration harness that turns Claude Code into a coordinated team of specialist AI agents", - "version": "4.4.54", + "version": "4.4.55", "author": { "name": "Synaptic-Labs-AI" }, diff --git a/README.md b/README.md index a3dd8ea4..ae7ac47a 100644 --- a/README.md +++ b/README.md @@ -605,7 +605,7 @@ When installed as a plugin, PACT lives in your plugin cache: │ └── cache/ │ └── pact-plugin/ │ └── PACT/ -│ └── 4.4.54/ # Plugin version +│ └── 4.4.55/ # Plugin version │ ├── agents/ │ ├── commands/ │ ├── skills/ diff --git a/pact-plugin/.claude-plugin/plugin.json b/pact-plugin/.claude-plugin/plugin.json index b6ebc4d7..e9aa659d 100644 --- a/pact-plugin/.claude-plugin/plugin.json +++ b/pact-plugin/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "PACT", - "version": "4.4.54", + "version": "4.4.55", "description": "Orchestration harness that turns Claude Code into a coordinated team of specialist AI agents", "author": { "name": "Synaptic-Labs-AI", diff --git a/pact-plugin/README.md b/pact-plugin/README.md index 3fb0123a..3960efff 100644 --- a/pact-plugin/README.md +++ b/pact-plugin/README.md @@ -1,6 +1,6 @@ # PACT — Orchestration Harness for Claude Code -> **Version**: 4.4.54 +> **Version**: 4.4.55 Turn a single Claude Code session into a managed team of specialist AI agents that prepare, design, build, and test your code systematically. From 55d4507ed144a8753efa52f2e2926f2f3a7bce56 Mon Sep 17 00:00:00 2001 From: michael-wojcik <5386199+michael-wojcik@users.noreply.github.com> Date: Sun, 5 Jul 2026 03:26:32 -0400 Subject: [PATCH 4/4] fix(merge-guard): fail-safe retirement observer against malformed bound_flags (Copilot #1110 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #1100 flag-axis read `set(ctx.get("bound_flags", []))` could crash the PostToolUse retirement observer on a malformed/legacy token: a JSON-null bound_flags yields None (dict.get returns the present key's value, not the default) so set(None) raises TypeError; a list containing a non-string/unhashable element makes set() raise on the unhashable. An unguarded crash aborts the whole scan, skipping retirement for the rest of the run. Add an isinstance-list + all-string-element guard mirroring the function's existing malformed-ctx skip: a malformed token is skipped gracefully (the safe under-retire direction) and never poisons the loop — a valid token in the same run still retires. Skip, not delete (deletion is the read arm's reaper job). Valid-token semantics unchanged; empty [] still passes. Tests: TestMalformedTokenRetirementRobustness — malformed bound_flags (null/int/float/bool/string and list-with-unhashable-element) never raises and is skipped; a malformed token forced first does not poison a valid retirement. --- pact-plugin/hooks/merge_guard_post.py | 20 +++- pact-plugin/tests/test_merge_guard.py | 126 ++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) diff --git a/pact-plugin/hooks/merge_guard_post.py b/pact-plugin/hooks/merge_guard_post.py index 876d6669..26fc27dc 100644 --- a/pact-plugin/hooks/merge_guard_post.py +++ b/pact-plugin/hooks/merge_guard_post.py @@ -769,7 +769,25 @@ def _retire_token_for_command( # (backstopped by TTL + MAX_USES) — never an over-block (PostToolUse observer). # A token-has-flags guard would be WRONG: it would let a flagged command retire # a flagless token, breaking the symmetric identity match. - if set(ctx.get("bound_flags", [])) != cmd_flags: + token_flags = ctx.get("bound_flags", []) + if not isinstance(token_flags, list) or not all(isinstance(x, str) for x in token_flags): + # Robustness (Copilot #1110): a malformed/legacy token whose + # bound_flags is not a list of strings would crash the whole scan + # and skip retirement for every later token in the run. Two shapes: + # (1) NON-LIST — e.g. JSON null -> Python None, since .get's [] + # default applies ONLY when the KEY is ABSENT, not when it is present + # and null — makes set(token_flags) raise TypeError; (2) a list + # CONTAINING an unhashable element (e.g. [{}] or [["x"]]) passes the + # list check but still makes set(token_flags) raise (unhashable + # type). The all(isinstance(x, str)) clause rejects BOTH: the mint + # only ever writes a list of flag STRINGS (extract_privileged_flags + # SSOT), so any non-str element is malformed. Skip it gracefully — + # mirrors the isinstance(ctx, dict) guard above; a safe under-retire, + # never a crash of the PostToolUse observer. A valid token's + # bound_flags is always a list of strings (incl. []; all() over an + # empty list is True), so valid-token semantics are unchanged. + continue + if set(token_flags) != cmd_flags: continue # Session scoping (SEC-S1 cycle-2 revised asymmetric predicate). token_session = token_data.get("session_id", "") diff --git a/pact-plugin/tests/test_merge_guard.py b/pact-plugin/tests/test_merge_guard.py index 13836e88..05cdd7c2 100644 --- a/pact-plugin/tests/test_merge_guard.py +++ b/pact-plugin/tests/test_merge_guard.py @@ -13432,6 +13432,132 @@ def test_flag_axis_identical_across_session_states( assert len(self._live(tmp_path)) == 1 +class TestMalformedTokenRetirementRobustness: + """#1100 follow-up (Copilot PR review): a token whose stored `bound_flags` + is malformed must never crash `_retire_token_for_command` — the retirement + observer is fail-safe (never raises, never blocks the caller). Two malformed + classes both made `set(token_flags)` raise: (1) a NON-list value (JSON null -> + None, or int/float/bool/str) -> `set(None)` TypeError; (2) a LIST containing a + non-string / unhashable element (`[{}]`, `[["x"]]`) -> `unhashable type`. The + token-side read is hardened with `isinstance(token_flags, list) and + all(isinstance(x, str) for x in token_flags)`; a malformed token is SKIPPED, + and — critically — a malformed token must not poison the rest of the + retirement loop, so a VALID token in the same directory still retires. + + Tokens are written to disk DIRECTLY (bypassing write_token) so a malformed + `bound_flags` the production mint path would never emit can be injected, and + so two tokens can coexist (write_token's Layer-5 cleanup retires priors). + In-memory only; no git mutation in the shared worktree.""" + + def _live(self, tmp_path): + return [t for t in tmp_path.glob("merge-authorized-*") + if not t.name.endswith(".consumed") and ".use-" not in t.name] + + @staticmethod + def _write_token_json(tmp_path, context, name="merge-authorized-1000"): + """Write a token file directly with an arbitrary context (so a malformed + `bound_flags` can be injected). Mirrors write_token's on-disk shape.""" + now = time.time() + data = {"created_at": now, "expires_at": now + 300, + "context": context, "max_uses": 2, "uses_remaining": 2} + path = tmp_path / name + path.write_text(json.dumps(data)) + return path + + @pytest.mark.parametrize( + "bad_flags", + [ + # non-list shapes — closed by the `isinstance(token_flags, list)` clause + None, 7, 1.5, True, "notalist", + # list WITH a non-string / unhashable element — PASSES isinstance-list + # but `set([{}])` / `set([["x"]])` raises `unhashable type`; closed by + # the `all(isinstance(x, str) for x in token_flags)` element clause. + # Without these rows that element clause would ship UNTESTED. + [{}], [["x"]], + ], + ids=["none", "int", "float", "bool", "string", + "list_with_dict", "list_with_list"], + ) + def test_malformed_bound_flags_does_not_crash_and_is_skipped( + self, tmp_path, bad_flags + ): + """A token whose bound_flags is malformed — either NOT a list (None/int/ + float/bool/string) OR a list containing a non-string/unhashable element + (`[{}]`, `[["x"]]`) — must NOT crash the retirement observer and must NOT + be retired. Driven with a NON-empty command flag-set (`--delete-branch`) + so the malformed token cannot match whether the guard SKIPS it or coerces + bound_flags to [] — robust to the guard's exact shape. Without the guard + the non-list crashers AND the unhashable-element lists raise `set(...)` + TypeError (closed by the isinstance-list clause and the + all(isinstance(x, str)) element clause respectively); the string case + never crashes (char-set) but is still skipped.""" + from merge_guard_post import _retire_token_for_command + + self._write_token_json( + tmp_path, + {"operation_type": "close", "pr_number": "5", "bound_flags": bad_flags}, + ) + try: + retired = _retire_token_for_command( + "gh pr close 5 --delete-branch", "close", token_dir=tmp_path) + except Exception as exc: # noqa: BLE001 — the observer contract is never-raise + pytest.fail( + f"malformed bound_flags ({bad_flags!r}) crashed the retirement " + f"observer: {type(exc).__name__}: {exc}" + ) + assert retired is False, "a malformed-flags token must not be retired" + assert len(self._live(tmp_path)) == 1, ( + "the malformed token is skipped (survives), not consumed" + ) + + @pytest.mark.parametrize( + "bad_flags", [None, [{}]], ids=["non_list", "list_with_unhashable"] + ) + def test_malformed_token_does_not_poison_valid_retirement( + self, tmp_path, monkeypatch, bad_flags + ): + """RESILIENCE (the core of the finding): a malformed token encountered + BEFORE a valid matching token must not abort the loop — the valid token + still retires. `_retire_token_for_command` returns on the FIRST match and + glob order is filesystem-arbitrary, so we FORCE the malformed token first + via a monkeypatched glob to make the hazard deterministic (otherwise the + test could pass by luck of iteration order). Covers BOTH crash surfaces + (non-list and unhashable-element list). RED before the guard: the + malformed token raises and the valid token is never reached.""" + import merge_guard_post as mgp + from merge_guard_post import _retire_token_for_command + + malformed = self._write_token_json( + tmp_path, + {"operation_type": "close", "pr_number": "5", "bound_flags": bad_flags}, + name="merge-authorized-1000", + ) + valid = self._write_token_json( + tmp_path, + {"operation_type": "close", "pr_number": "5", + "bound_flags": ["--delete-branch"]}, + name="merge-authorized-2000", + ) + # Visit the malformed token FIRST (deterministic hazard ordering). + monkeypatch.setattr( + mgp.glob, "glob", lambda pattern: [str(malformed), str(valid)]) + try: + retired = _retire_token_for_command( + "gh pr close 5 --delete-branch", "close", token_dir=tmp_path) + except Exception as exc: # noqa: BLE001 — the observer contract is never-raise + pytest.fail( + f"a malformed token poisoned the retirement loop: " + f"{type(exc).__name__}: {exc}" + ) + assert retired is True, ( + "the valid token must still retire after the malformed one is skipped" + ) + live = self._live(tmp_path) + assert len(live) == 1 and live[0].name == "merge-authorized-1000", ( + "the malformed token is skipped (survives); the valid token is consumed" + ) + + class TestD1MatcherQuoteNormalization: """D1-matcher (#933): _token_matches_command normalizes surrounding quotes on the captured branch name at compare time, so a token branch `feat/x`