diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e6fe6a2..d3288978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,15 @@ PyPI version — not the changelog header. ## [2026-07-21] +**feat(drone)** — joint-decision gate on `drone @git merge` (DPLAN-0256, +Patrick ruling S330: merges are always done together, never accidental). +The gate sits in `_handle_merge` before the plugin import — `merge_pr()` is +unreachable without confirmation. A real terminal gets an interactive y/N +prompt; headless callers (agent Bash) are refused unless `--confirm` is +passed explicitly. Every gate decision (confirm / tty-yes / tty-abort / +headless-refused) is logged via json_handler. 6 new tests (86 green), +live-fired refusal verified, seedgo 31/31. + **feat(skills)** — telegram user_message_relay joins the sound layer: relay events now carry their own sound key so an inbound user message is audible like every other hook event (59/59 + 252 green). diff --git a/src/aipass/drone/apps/modules/git_module.py b/src/aipass/drone/apps/modules/git_module.py index 52b09922..e45bf52c 100644 --- a/src/aipass/drone/apps/modules/git_module.py +++ b/src/aipass/drone/apps/modules/git_module.py @@ -17,6 +17,7 @@ from __future__ import annotations import json import subprocess +import sys from pathlib import Path from aipass.prax import logger @@ -302,10 +303,47 @@ def _handle_close_pr(args: list[str]) -> dict: return {"stdout": "", "stderr": result["message"], "exit_code": 1} +def _confirm_merge(pr_number: str, caller: str, confirmed: bool) -> dict | None: + """Joint-decision gate: merges must never happen accidentally (DPLAN-0256). + + Returns None when the merge may proceed, or a refusal/abort result dict. + Order matters: an explicit --confirm always passes; otherwise a real + terminal gets an interactive y/N prompt; headless callers are refused. + """ + if confirmed: + json_handler.log_operation("merge_gate", {"pr_number": pr_number, "caller": caller, "path": "--confirm"}) + return None + + if sys.stdin.isatty(): + answer = input(f"Merge PR #{pr_number}? Merges are a joint decision. [y/N] ") + if answer.strip().lower() in ("y", "yes"): + json_handler.log_operation("merge_gate", {"pr_number": pr_number, "caller": caller, "path": "tty-yes"}) + return None + json_handler.log_operation("merge_gate", {"pr_number": pr_number, "caller": caller, "path": "tty-abort"}) + return {"stdout": "", "stderr": f"Merge of PR #{pr_number} aborted at prompt.", "exit_code": 1} + + json_handler.log_operation("merge_gate", {"pr_number": pr_number, "caller": caller, "path": "headless-refused"}) + logger.info("merge gate: refused headless merge of PR #%s by %s (no --confirm)", pr_number, caller) + return { + "stdout": "", + "stderr": ( + f"Merge of PR #{pr_number} requires explicit confirmation — merges are a joint decision.\n" + f"Re-run once agreed: drone @git merge {pr_number} --confirm" + ), + "exit_code": 1, + } + + def _handle_merge(args: list[str], caller: str) -> dict: """Handle the merge subcommand (owner-tier, auth pre-checked).""" - if not args: - return {"stdout": "", "stderr": "Usage: drone @git merge ", "exit_code": 1} + confirmed = "--confirm" in args + pr_args = [a for a in args if not a.startswith("--")] + if not pr_args: + return {"stdout": "", "stderr": "Usage: drone @git merge [--confirm]", "exit_code": 1} + + refusal = _confirm_merge(pr_args[0], caller, confirmed) + if refusal is not None: + return refusal try: from aipass.drone.apps.plugins.devpulse_ops.merge_plugin import merge_pr @@ -313,7 +351,7 @@ def _handle_merge(args: list[str], caller: str) -> dict: logger.error("Failed to import devpulse_ops merge plugin: %s", exc) return {"stdout": "", "stderr": f"devpulse_ops plugin not available: {exc}", "exit_code": 1} - result = merge_pr(args[0], caller) + result = merge_pr(pr_args[0], caller) if result["success"]: return {"stdout": result["message"], "stderr": "", "exit_code": 0} return {"stdout": "", "stderr": result["message"], "exit_code": 1} @@ -637,8 +675,10 @@ def get_help(command: str | None = None) -> str: return "git unlock --force — Force-release the PR lock [owner]\n" if command == "merge": return ( - "git merge — Merge a PR and sync local main [owner]\n" + "git merge [--confirm] — Merge a PR and sync local main [owner]\n" " Runs gh pr merge --merge --delete-branch, then git pull --rebase.\n" + " Joint-decision gate: terminal sessions get a y/N prompt; headless\n" + " callers must pass --confirm or the merge is refused.\n" ) if command == "smart-sync": return "git smart-sync — Fetch origin and rebase if behind [owner]\n" @@ -686,7 +726,7 @@ def get_help(command: str | None = None) -> str: " dev-pr Push dev and create PR to main\n" " delete-branch Delete a remote branch\n" " close-pr Close a PR\n" - " merge Merge a PR\n" + " merge [--confirm] Merge a PR (gated: y/N prompt or --confirm)\n" " sync [--autostash] Sync with origin/main (FF on dev)\n" " smart-sync Fetch + rebase if behind\n" " unlock --force Force-release the PR lock\n" diff --git a/src/aipass/drone/tests/test_git_module.py b/src/aipass/drone/tests/test_git_module.py index 21a0a95c..bdaa9cae 100644 --- a/src/aipass/drone/tests/test_git_module.py +++ b/src/aipass/drone/tests/test_git_module.py @@ -1471,3 +1471,91 @@ class TestScopeFooter: result = handle_command("diff", ["--all"]) assert "showing drone scope" not in result["stdout"] + + +# =========================================================================== +# Merge joint-decision gate (DPLAN-0256) +# =========================================================================== + +_MERGE_PR = "aipass.drone.apps.plugins.devpulse_ops.merge_plugin.merge_pr" +_MERGE_OK = { + "success": True, + "pr_number": "123", + "title": "t", + "merge_commit": "abc123", + "message": "PR #123 merged: t (abc123)", +} + + +class TestMergeGate: + """merge must never run without explicit confirmation.""" + + @patch(_AUTH, return_value="devpulse") + def test_headless_without_confirm_refused(self, _mock_auth: MagicMock) -> None: + """Non-TTY caller without --confirm is refused before the plugin loads.""" + with patch(_MERGE_PR) as mock_merge: + with patch(f"{_GIT_MOD}.sys.stdin") as mock_stdin: + mock_stdin.isatty.return_value = False + result = handle_command("merge", ["123"]) + + assert result["exit_code"] == 1 + assert "requires explicit confirmation" in result["stderr"] + assert "--confirm" in result["stderr"] + mock_merge.assert_not_called() + + @patch(_AUTH, return_value="devpulse") + def test_confirm_flag_proceeds(self, _mock_auth: MagicMock) -> None: + """--confirm merges without prompting, even headless.""" + with patch(_MERGE_PR, return_value=dict(_MERGE_OK)) as mock_merge: + with patch(f"{_GIT_MOD}.sys.stdin") as mock_stdin: + mock_stdin.isatty.return_value = False + result = handle_command("merge", ["123", "--confirm"]) + + assert result["exit_code"] == 0 + mock_merge.assert_called_once_with("123", "devpulse") + + @patch(_AUTH, return_value="devpulse") + def test_confirm_flag_position_agnostic(self, _mock_auth: MagicMock) -> None: + """--confirm before the PR number still resolves the right PR.""" + with patch(_MERGE_PR, return_value=dict(_MERGE_OK)) as mock_merge: + with patch(f"{_GIT_MOD}.sys.stdin") as mock_stdin: + mock_stdin.isatty.return_value = False + result = handle_command("merge", ["--confirm", "123"]) + + assert result["exit_code"] == 0 + mock_merge.assert_called_once_with("123", "devpulse") + + @patch(_AUTH, return_value="devpulse") + def test_tty_yes_proceeds(self, _mock_auth: MagicMock) -> None: + """Interactive terminal answering y merges.""" + with patch(_MERGE_PR, return_value=dict(_MERGE_OK)) as mock_merge: + with patch(f"{_GIT_MOD}.sys.stdin") as mock_stdin: + mock_stdin.isatty.return_value = True + with patch("builtins.input", return_value="y"): + result = handle_command("merge", ["123"]) + + assert result["exit_code"] == 0 + mock_merge.assert_called_once_with("123", "devpulse") + + @patch(_AUTH, return_value="devpulse") + def test_tty_default_aborts(self, _mock_auth: MagicMock) -> None: + """Interactive terminal hitting enter (default N) aborts.""" + with patch(_MERGE_PR) as mock_merge: + with patch(f"{_GIT_MOD}.sys.stdin") as mock_stdin: + mock_stdin.isatty.return_value = True + with patch("builtins.input", return_value=""): + result = handle_command("merge", ["123"]) + + assert result["exit_code"] == 1 + assert "aborted" in result["stderr"] + mock_merge.assert_not_called() + + @patch(_AUTH, return_value="devpulse") + def test_confirm_without_pr_number_is_usage_error(self, _mock_auth: MagicMock) -> None: + """--confirm alone (no PR number) is a usage error, not a merge.""" + with patch(_MERGE_PR) as mock_merge: + result = handle_command("merge", ["--confirm"]) + + assert result["exit_code"] == 1 + assert "Usage" in result["stderr"] + mock_merge.assert_not_called()