From 1c95f49e98814ddf55b05f622f756f7bd22ccbd2 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Fri, 15 May 2026 14:43:35 -0700 Subject: [PATCH 1/2] fix(drone): add codeql suppression for github.com substring check in create_branch_pr (false positive #16) --- .../drone/apps/handlers/git/commit_handler.py | 56 ++++++++++ .../drone/apps/handlers/git/dev_pr_handler.py | 1 + src/aipass/drone/tests/test_git_access.py | 100 +++++++++++++++++- 3 files changed, 156 insertions(+), 1 deletion(-) diff --git a/src/aipass/drone/apps/handlers/git/commit_handler.py b/src/aipass/drone/apps/handlers/git/commit_handler.py index 19d579c2..16fc88ba 100644 --- a/src/aipass/drone/apps/handlers/git/commit_handler.py +++ b/src/aipass/drone/apps/handlers/git/commit_handler.py @@ -18,6 +18,57 @@ from aipass.drone.apps.handlers.json import json_handler from aipass.drone.apps.handlers.git.lock_handler import find_repo_root +def _run_test_gate(repo_root: Path) -> dict | None: + """Run pytest for changed branches. Returns error dict if tests fail, None if all pass.""" + status_result = subprocess.run( + ["git", "status", "--porcelain"], + capture_output=True, + text=True, + cwd=str(repo_root), + ) + changed_branches: set[str] = set() + for line in status_result.stdout.strip().splitlines(): + filepath = line[3:].split(" -> ")[-1] + parts = Path(filepath).parts + if len(parts) >= 3 and parts[0] == "src" and parts[1] == "aipass": + changed_branches.add(parts[2]) + + venv_python = repo_root / ".venv" / "bin" / "python" + python_bin = str(venv_python) if venv_python.exists() else "python3" + + failed_branches: list[tuple[str, str]] = [] + for branch_name in sorted(changed_branches): + test_dir = repo_root / "src" / "aipass" / branch_name / "tests" + if not test_dir.is_dir(): + continue + try: + test_result = subprocess.run( + [python_bin, "-m", "pytest", str(test_dir), "--tb=short", "-q"], + capture_output=True, + text=True, + cwd=str(repo_root), + timeout=120, + ) + except subprocess.TimeoutExpired: + logger.warning("pytest timed out for branch %s (120s limit)", branch_name) + failed_branches.append((branch_name, "pytest timed out (120s)")) + continue + if test_result.returncode != 0: + failed_branches.append((branch_name, test_result.stdout.strip())) + + if not failed_branches: + return None + + msg_parts = ["Test failures — fix before committing:"] + for bname, output in failed_branches: + msg_parts.append(f"\n--- {bname} ---\n{output}") + return { + "stdout": "", + "stderr": "\n".join(msg_parts), + "exit_code": 1, + } + + def stage_branch_dir(branch_dir: Path, repo_root: Path | None = None) -> dict: """Stage all changes under branch_dir. @@ -111,6 +162,11 @@ def commit_changes( "stderr": f"Lint errors — fix before committing:\n{lint_check.stdout.strip()}", "exit_code": 1, } + + test_gate_result = _run_test_gate(repo_root) + if test_gate_result is not None: + return test_gate_result + add_result = subprocess.run( ["git", "add", "-A"], capture_output=True, diff --git a/src/aipass/drone/apps/handlers/git/dev_pr_handler.py b/src/aipass/drone/apps/handlers/git/dev_pr_handler.py index 6d498aba..e1815ae8 100644 --- a/src/aipass/drone/apps/handlers/git/dev_pr_handler.py +++ b/src/aipass/drone/apps/handlers/git/dev_pr_handler.py @@ -90,6 +90,7 @@ def create_branch_pr(description: str, target_branch: str = "main") -> dict: if "already exists" in stderr: existing_url = "" for line in stderr.splitlines(): + # codeql[py/incomplete-url-substring-sanitization] if "github.com" in line: existing_url = line.strip() break diff --git a/src/aipass/drone/tests/test_git_access.py b/src/aipass/drone/tests/test_git_access.py index cf2ec407..9ee63d60 100644 --- a/src/aipass/drone/tests/test_git_access.py +++ b/src/aipass/drone/tests/test_git_access.py @@ -346,6 +346,7 @@ class TestCommitChanges: mock_ruff_fix = MagicMock(returncode=0, stdout="", stderr="") mock_ruff_format = MagicMock(returncode=0, stdout="", stderr="") mock_ruff_gate = MagicMock(returncode=0, stdout="", stderr="") + mock_status = MagicMock(returncode=0, stdout=" M src/aipass/api/app.py\n", stderr="") mock_add = MagicMock(returncode=0, stderr="") mock_diff = MagicMock(returncode=1, stdout="", stderr="") mock_commit = MagicMock(returncode=0, stdout="[main def456] all commit", stderr="") @@ -356,13 +357,110 @@ class TestCommitChanges: patch("shutil.which", return_value="/usr/bin/ruff"), patch( "aipass.drone.apps.handlers.git.commit_handler.subprocess.run", - side_effect=[mock_ruff_fix, mock_ruff_format, mock_ruff_gate, mock_add, mock_diff, mock_commit], + side_effect=[ + mock_ruff_fix, + mock_ruff_format, + mock_ruff_gate, + mock_status, + mock_add, + mock_diff, + mock_commit, + ], ), ): result = commit_changes("all commit", branch_dir=branch_dir, all_files=True) assert result["exit_code"] == 0 + def test_commit_all_blocks_on_test_failure(self, repo_dir: Path) -> None: + mock_ruff_fix = MagicMock(returncode=0, stdout="", stderr="") + mock_ruff_format = MagicMock(returncode=0, stdout="", stderr="") + mock_ruff_gate = MagicMock(returncode=0, stdout="", stderr="") + mock_status = MagicMock( + returncode=0, + stdout=" M src/aipass/drone/apps/handlers/git/commit_handler.py\n", + stderr="", + ) + mock_pytest = MagicMock( + returncode=1, + stdout="FAILED test_foo.py::test_bar - assert 1 == 2\n1 failed", + stderr="", + ) + + test_dir = repo_dir / "src" / "aipass" / "drone" / "tests" + test_dir.mkdir(parents=True) + + with ( + patch("shutil.which", return_value="/usr/bin/ruff"), + patch( + "aipass.drone.apps.handlers.git.commit_handler.subprocess.run", + side_effect=[mock_ruff_fix, mock_ruff_format, mock_ruff_gate, mock_status, mock_pytest], + ), + ): + result = commit_changes("fail commit", all_files=True) + + assert result["exit_code"] == 1 + assert "Test failures" in result["stderr"] + assert "drone" in result["stderr"] + + def test_commit_all_passes_with_green_tests(self, repo_dir: Path) -> None: + mock_ruff_fix = MagicMock(returncode=0, stdout="", stderr="") + mock_ruff_format = MagicMock(returncode=0, stdout="", stderr="") + mock_ruff_gate = MagicMock(returncode=0, stdout="", stderr="") + mock_status = MagicMock( + returncode=0, + stdout=" M src/aipass/drone/apps/handlers/git/commit_handler.py\n", + stderr="", + ) + mock_pytest = MagicMock(returncode=0, stdout="3 passed", stderr="") + mock_add = MagicMock(returncode=0, stderr="") + mock_diff = MagicMock(returncode=1, stdout="", stderr="") + mock_commit = MagicMock(returncode=0, stdout="[main abc999] green commit", stderr="") + + test_dir = repo_dir / "src" / "aipass" / "drone" / "tests" + test_dir.mkdir(parents=True) + + with ( + patch("shutil.which", return_value="/usr/bin/ruff"), + patch( + "aipass.drone.apps.handlers.git.commit_handler.subprocess.run", + side_effect=[ + mock_ruff_fix, mock_ruff_format, mock_ruff_gate, + mock_status, mock_pytest, mock_add, mock_diff, mock_commit, + ], + ), + ): + result = commit_changes("green commit", all_files=True) + + assert result["exit_code"] == 0 + + def test_commit_all_skips_branches_without_tests(self, repo_dir: Path) -> None: + mock_ruff_fix = MagicMock(returncode=0, stdout="", stderr="") + mock_ruff_format = MagicMock(returncode=0, stdout="", stderr="") + mock_ruff_gate = MagicMock(returncode=0, stdout="", stderr="") + mock_status = MagicMock( + returncode=0, + stdout=" M src/aipass/flow/apps/module.py\n", + stderr="", + ) + mock_add = MagicMock(returncode=0, stderr="") + mock_diff = MagicMock(returncode=1, stdout="", stderr="") + mock_commit = MagicMock(returncode=0, stdout="[main skip77] no tests", stderr="") + + with ( + patch("shutil.which", return_value="/usr/bin/ruff"), + patch( + "aipass.drone.apps.handlers.git.commit_handler.subprocess.run", + side_effect=[ + mock_ruff_fix, mock_ruff_format, mock_ruff_gate, + mock_status, mock_add, mock_diff, mock_commit, + ], + ), + ): + result = commit_changes("no tests commit", all_files=True) + + assert result["exit_code"] == 0 + def test_commit_os_error(self, repo_dir: Path) -> None: mock_diff = MagicMock(returncode=1, stdout="", stderr="") From c114d4405f0991f557273d7e730a7c441af658a3 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Fri, 15 May 2026 14:48:07 -0700 Subject: [PATCH 2/2] =?UTF-8?q?feat(drone):=20add=20pytest=20gate=20to=20c?= =?UTF-8?q?ommit=20handler=20=E2=80=94=20blocks=20commit=20on=20test=20fai?= =?UTF-8?q?lures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../drone/apps/handlers/git/commit_handler.py | 4 +++- src/aipass/drone/tests/test_git_access.py | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/aipass/drone/apps/handlers/git/commit_handler.py b/src/aipass/drone/apps/handlers/git/commit_handler.py index 16fc88ba..bd6ec58e 100644 --- a/src/aipass/drone/apps/handlers/git/commit_handler.py +++ b/src/aipass/drone/apps/handlers/git/commit_handler.py @@ -27,7 +27,9 @@ def _run_test_gate(repo_root: Path) -> dict | None: cwd=str(repo_root), ) changed_branches: set[str] = set() - for line in status_result.stdout.strip().splitlines(): + for line in status_result.stdout.splitlines(): + if len(line) < 4: + continue filepath = line[3:].split(" -> ")[-1] parts = Path(filepath).parts if len(parts) >= 3 and parts[0] == "src" and parts[1] == "aipass": diff --git a/src/aipass/drone/tests/test_git_access.py b/src/aipass/drone/tests/test_git_access.py index 9ee63d60..be1e7717 100644 --- a/src/aipass/drone/tests/test_git_access.py +++ b/src/aipass/drone/tests/test_git_access.py @@ -425,8 +425,14 @@ class TestCommitChanges: patch( "aipass.drone.apps.handlers.git.commit_handler.subprocess.run", side_effect=[ - mock_ruff_fix, mock_ruff_format, mock_ruff_gate, - mock_status, mock_pytest, mock_add, mock_diff, mock_commit, + mock_ruff_fix, + mock_ruff_format, + mock_ruff_gate, + mock_status, + mock_pytest, + mock_add, + mock_diff, + mock_commit, ], ), ): @@ -452,8 +458,13 @@ class TestCommitChanges: patch( "aipass.drone.apps.handlers.git.commit_handler.subprocess.run", side_effect=[ - mock_ruff_fix, mock_ruff_format, mock_ruff_gate, - mock_status, mock_add, mock_diff, mock_commit, + mock_ruff_fix, + mock_ruff_format, + mock_ruff_gate, + mock_status, + mock_add, + mock_diff, + mock_commit, ], ), ):