From bfc0244e322a89cfdfd024db511a7d3a18b777a9 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 24 Feb 2026 12:59:05 +0100 Subject: [PATCH 1/3] ci: auto-approve integration tests for internal PRs Fork PRs still require manual environment approval to prevent untrusted artifacts from accessing secrets. Internal PRs (same repo) skip the gate since contributors already have write access. This eliminates manual approval overhead for the common case while preserving the security gate for external contributions. --- .github/workflows/ci-integration.yml | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci-integration.yml b/.github/workflows/ci-integration.yml index f063e523..1950704f 100644 --- a/.github/workflows/ci-integration.yml +++ b/.github/workflows/ci-integration.yml @@ -4,7 +4,8 @@ # - ci.yml builds the PR's code in an unprivileged context and uploads binary artifacts # - This workflow downloads those artifacts and runs tests from the DEFAULT BRANCH (main) # - PR code is never checked out in a privileged context -# - Environment approval gate ensures a maintainer reviews the PR before tests run +# - Fork PRs require manual environment approval before tests run (security gate) +# - Internal PRs (same repo) skip the approval gate — contributors already have write access name: Integration Tests (PR) env: @@ -21,19 +22,32 @@ permissions: statuses: write jobs: - # Single approval gate — maintainer reviews PR code, then approves this environment. - # All downstream jobs chain off this, so approval happens once. - approve: + # Fork PRs: require manual approval via environment protection rules. + # A maintainer must review the fork's code before secrets are exposed to its artifacts. + approve-fork: if: > github.event.workflow_run.conclusion == 'success' && - github.event.workflow_run.event == 'pull_request' + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.head_repository.full_name != github.repository runs-on: ubuntu-latest environment: integration-tests steps: - - run: echo "Integration tests approved for ${{ github.event.workflow_run.head_branch }}" + - run: echo "Fork PR approved for ${{ github.event.workflow_run.head_branch }} from ${{ github.event.workflow_run.head_repository.full_name }}" + + # Internal PRs: skip approval gate — contributors already have write access to the repo. + approve-internal: + if: > + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.head_repository.full_name == github.repository + runs-on: ubuntu-latest + steps: + - run: echo "Internal PR auto-approved for ${{ github.event.workflow_run.head_branch }}" smoke-test: - needs: [approve] + needs: [approve-fork, approve-internal] + # Run if either approval job succeeded (the other will be skipped) + if: always() && (needs.approve-fork.result == 'success' || needs.approve-internal.result == 'success') runs-on: ${{ matrix.os }} permissions: contents: read From 4073f8a2279c4ec647892582da0c31a43fb3eeb5 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 24 Feb 2026 14:40:36 +0100 Subject: [PATCH 2/3] ci: upgrade CodeQL Action from v3 to v4 --- .github/workflows/codeql.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 787de037..f8717eee 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -27,9 +27,9 @@ jobs: uses: actions/checkout@v4 - name: Initialize CodeQL - uses: github/codeql-action/init@v3 + uses: github/codeql-action/init@v4 with: languages: ${{ matrix.language }} - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v3 + uses: github/codeql-action/analyze@v4 From 84d6673b1e84ac294d1dffca5a0db222ece823c0 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 24 Feb 2026 16:52:47 +0100 Subject: [PATCH 3/3] fix: integration tests prefer PATH binary over .venv source install The test harness in 4 integration test files hardcoded .venv/bin/apm as the primary binary, falling back to PATH only when the venv didn't exist. In CI (ci-integration.yml), the PR binary artifact is placed on PATH via symlink, but uv sync also creates .venv/bin/apm from main's source. The tests always found the venv binary first, effectively testing main's code instead of the PR's. Invert the priority: prefer shutil.which('apm') (finds PATH binary in CI, or venv binary locally since venv activates onto PATH), falling back to the hardcoded .venv path only when not on PATH. Affected files: - test_ado_e2e.py - test_mixed_deps.py - test_skill_compile.py - test_skill_install.py --- tests/integration/test_ado_e2e.py | 13 ++++++++----- tests/integration/test_mixed_deps.py | 6 ++++++ tests/integration/test_skill_compile.py | 6 ++++++ tests/integration/test_skill_install.py | 8 ++++++-- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/tests/integration/test_ado_e2e.py b/tests/integration/test_ado_e2e.py index b2f3f272..9b1f0334 100644 --- a/tests/integration/test_ado_e2e.py +++ b/tests/integration/test_ado_e2e.py @@ -8,6 +8,7 @@ """ import os +import shutil import subprocess import tempfile from pathlib import Path @@ -24,11 +25,13 @@ def run_apm_command(cmd: str, cwd: Path, timeout: int = 60) -> subprocess.CompletedProcess: """Run an APM CLI command and return the result.""" - # Use the development version of APM - apm_path = Path(__file__).parent.parent.parent / ".venv" / "bin" / "apm" - if not apm_path.exists(): - # Fallback to system apm - apm_path = "apm" + # Prefer binary on PATH (CI uses the PR artifact there) + apm_on_path = shutil.which("apm") + if apm_on_path: + apm_path = apm_on_path + else: + # Fallback to local dev venv + apm_path = Path(__file__).parent.parent.parent / ".venv" / "bin" / "apm" full_cmd = f"{apm_path} {cmd}" result = subprocess.run( diff --git a/tests/integration/test_mixed_deps.py b/tests/integration/test_mixed_deps.py index 01bfe68a..735b8975 100644 --- a/tests/integration/test_mixed_deps.py +++ b/tests/integration/test_mixed_deps.py @@ -7,6 +7,7 @@ """ import os +import shutil import subprocess import pytest from pathlib import Path @@ -45,6 +46,11 @@ def temp_project(tmp_path): @pytest.fixture def apm_command(): """Get the path to the APM CLI executable.""" + # Prefer binary on PATH (CI uses the PR artifact there) + apm_on_path = shutil.which("apm") + if apm_on_path: + return apm_on_path + # Fallback to local dev venv venv_apm = Path(__file__).parent.parent.parent / ".venv" / "bin" / "apm" if venv_apm.exists(): return str(venv_apm) diff --git a/tests/integration/test_skill_compile.py b/tests/integration/test_skill_compile.py index 0d3c1bc9..743edb8e 100644 --- a/tests/integration/test_skill_compile.py +++ b/tests/integration/test_skill_compile.py @@ -7,6 +7,7 @@ """ import os +import shutil import subprocess import pytest from pathlib import Path @@ -45,6 +46,11 @@ def temp_project(tmp_path): @pytest.fixture def apm_command(): """Get the path to the APM CLI executable.""" + # Prefer binary on PATH (CI uses the PR artifact there) + apm_on_path = shutil.which("apm") + if apm_on_path: + return apm_on_path + # Fallback to local dev venv venv_apm = Path(__file__).parent.parent.parent / ".venv" / "bin" / "apm" if venv_apm.exists(): return str(venv_apm) diff --git a/tests/integration/test_skill_install.py b/tests/integration/test_skill_install.py index ebfcc0fa..6a2ae925 100644 --- a/tests/integration/test_skill_install.py +++ b/tests/integration/test_skill_install.py @@ -7,6 +7,7 @@ """ import os +import shutil import subprocess import pytest from pathlib import Path @@ -45,11 +46,14 @@ def temp_project(tmp_path): @pytest.fixture def apm_command(): """Get the path to the APM CLI executable.""" - # Use the development version from source + # Prefer binary on PATH (CI uses the PR artifact there) + apm_on_path = shutil.which("apm") + if apm_on_path: + return apm_on_path + # Fallback to local dev venv venv_apm = Path(__file__).parent.parent.parent / ".venv" / "bin" / "apm" if venv_apm.exists(): return str(venv_apm) - # Fallback to system apm return "apm"