From aaf7d651adcbc3e3c9e686b9b3b35b29251550af Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 24 May 2026 20:00:09 +0000 Subject: [PATCH 1/3] ci: restrict abi-drift + zig-test to cartridges changed in the PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both workflows currently sweep every cartridge under cartridges/* and fail the PR check if any single one fails. With ~66 cartridges in the abi-drift allowlist and ~73 in zig-test, a PR touching one cartridge gets gated by unrelated breakage in others (e.g. browser-mcp and orchestrator-lsp-mcp Zig failures, container-mcp/git-mcp/queues-mcp/ vordr-mcp ABI drift) that has been sitting on main for weeks. This makes every cartridge-touching PR look "blocked" when it isn't. Constrain the per-cartridge loops to the intersection of: (1) the existing allowlist / discovery, and (2) cartridges with any file changed under cartridges//** in this PR's diff (origin/...HEAD). On `push: main` events the full sweep is preserved, so cross-cutting drift is still caught at trunk — this is purely a PR-time false- positive fix, not a relaxation of the gate. Both workflows now also `actions/checkout` with `fetch-depth: 0` so the `git diff origin/...HEAD` can resolve. An empty changed-set on a PR (which the `paths:` filter shouldn't allow anyway, but be defensive) emits a `::notice::` and exits 0 instead of going red on nothing. abi-drift: new "Restrict to cartridges changed in this PR" step filters steps.discover.outputs.carts; the verify loop picks either steps.filter.outputs.carts (PR) or steps.discover.outputs.carts (push). zig-test: new "Determine cartridges to test" step writes the in-scope set once; the FFI-tests and shared-library-build loops both read it. Catalogue tests (cd ffi/zig && zig build test) and the readiness step still run on every invocation — those test the cross-cartridge plumbing and must always go. This is a follow-up to PR #146's CI investigation; unblocks future single-cartridge PRs from inheriting unrelated breakage. Aspect tests (tests/aspect_tests.sh) use a different shape — they assert global invariants across the whole tree — so a "changed-files" filter doesn't fit them cleanly. Leaving aspect alone in this PR; can revisit with a baseline-aware ratchet if needed. --- .github/workflows/abi-drift.yml | 48 +++++++++++++++++++++++- .github/workflows/zig-test.yml | 65 ++++++++++++++++++++++++++++++--- 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/.github/workflows/abi-drift.yml b/.github/workflows/abi-drift.yml index db826d4f..c49523fc 100644 --- a/.github/workflows/abi-drift.yml +++ b/.github/workflows/abi-drift.yml @@ -43,6 +43,11 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + # Need at least two commits so `git diff origin/...HEAD` + # can compute the changed-cartridge set on pull_request events. + # `0` = full history (cheap on this repo). + fetch-depth: 0 - name: Install Rust toolchain (stable) uses: dtolnay/rust-toolchain@b3b07ba8b418998c39fb20f53e8b695cdcc8de1b # stable @@ -165,11 +170,52 @@ jobs: echo 'EOF' } >> "$GITHUB_OUTPUT" - - name: Emit + verify each cartridge + # On pull_request, restrict the verify loop to cartridges that + # actually changed in this PR — unrelated cartridge drift is the + # main branch's problem, not this PR's. On push to main, scan the + # full allowlist so cross-cutting regressions are still caught at + # trunk. Empty changed-set on a PR ⇒ skip the loop (the workflow + # `paths:` filter shouldn't have triggered us anyway, but stay + # defensive). + - name: Restrict to cartridges changed in this PR + id: filter + if: github.event_name == 'pull_request' env: + BASE_REF: ${{ github.base_ref }} CARTS: ${{ steps.discover.outputs.carts }} run: | set -euo pipefail + git fetch --no-tags --depth=50 origin "$BASE_REF" || true + changed=$(git diff --name-only "origin/${BASE_REF}...HEAD" -- 'cartridges/**' \ + | awk -F/ '{print $2}' | sort -u) + { + echo 'carts<> "$GITHUB_OUTPUT" + echo "Changed cartridges intersected with allowlist:" + while IFS= read -r cart; do + [ -z "$cart" ] && continue + if printf '%s\n' "$changed" | grep -qx "$cart"; then + echo " • $cart" + fi + done <<< "$CARTS" + + - name: Emit + verify each cartridge + env: + # On pull_request: filtered set. On push/main: full allowlist. + CARTS: ${{ github.event_name == 'pull_request' && steps.filter.outputs.carts || steps.discover.outputs.carts }} + run: | + set -euo pipefail + if [ -z "$(printf '%s' "$CARTS" | tr -d '[:space:]')" ]; then + echo "::notice::No allowlisted cartridges changed in this PR — skipping drift verify." + exit 0 + fi failed="" while IFS= read -r cart; do [ -z "$cart" ] && continue diff --git a/.github/workflows/zig-test.yml b/.github/workflows/zig-test.yml index 8cbbf822..cacd7b52 100644 --- a/.github/workflows/zig-test.yml +++ b/.github/workflows/zig-test.yml @@ -26,22 +26,68 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + # Needed so `git diff origin/...HEAD` can identify the + # cartridges actually changed in this PR. Full history is cheap + # on this repo; cuts down on cross-cartridge false-positive + # gating where a PR is blocked by pre-existing breakage in + # unrelated cartridges (browser-mcp, orchestrator-lsp-mcp, + # etc.). On push/main we keep the full per-cartridge sweep. + fetch-depth: 0 - name: Install Zig uses: mlugg/setup-zig@d1434d08867e3ee9daa34448df10607b98908d29 # v2 with: version: 0.15.2 + - name: Determine cartridges to test + id: scope + env: + EVENT: ${{ github.event_name }} + BASE_REF: ${{ github.base_ref }} + run: | + set -euo pipefail + all="$(ls -d cartridges/*/ffi/ 2>/dev/null | sed 's|cartridges/||;s|/ffi/||' | sort)" + if [ "$EVENT" = "pull_request" ]; then + git fetch --no-tags --depth=50 origin "$BASE_REF" || true + changed="$(git diff --name-only "origin/${BASE_REF}...HEAD" -- 'cartridges/**' \ + | awk -F/ '{print $2}' | sort -u)" + scope="" + while IFS= read -r cart; do + [ -z "$cart" ] && continue + if printf '%s\n' "$changed" | grep -qx "$cart"; then + scope="$scope$cart"$'\n' + fi + done <<< "$all" + else + scope="$all" + fi + { + echo 'carts<> "$GITHUB_OUTPUT" + echo "Cartridges in scope for this run:" + printf ' • %s\n' $(printf '%s\n' "$scope" | grep -v '^$' || true) + - name: Run catalogue tests run: cd ffi/zig && zig build test --summary all - name: Run readiness tests run: cd ffi/zig && zig build readiness --summary all - - name: Run cartridge FFI tests (all 73 cartridges) + - name: Run cartridge FFI tests + env: + CARTS: ${{ steps.scope.outputs.carts }} run: | + if [ -z "$(printf '%s' "$CARTS" | tr -d '[:space:]')" ]; then + echo "::notice::No cartridges changed in this PR — skipping per-cartridge FFI tests." + exit 0 + fi failed="" - for cart in $(ls -d cartridges/*/ffi/ 2>/dev/null | sed 's|cartridges/||;s|/ffi/||' | sort); do + while IFS= read -r cart; do + [ -z "$cart" ] && continue echo "::group::Testing $cart..." if cd "cartridges/$cart/ffi" && zig build test --summary all 2>&1; then echo " ✓ $cart passed" @@ -51,19 +97,26 @@ jobs: fi cd "$GITHUB_WORKSPACE" echo "::endgroup::" - done + done <<< "$CARTS" if [ -n "$failed" ]; then echo "::error::Failed cartridges:$failed" exit 1 fi - - name: Build cartridge shared libraries (all 73 cartridges) + - name: Build cartridge shared libraries + env: + CARTS: ${{ steps.scope.outputs.carts }} run: | - for cart in $(ls -d cartridges/*/ffi/ 2>/dev/null | sed 's|cartridges/||;s|/ffi/||' | sort); do + if [ -z "$(printf '%s' "$CARTS" | tr -d '[:space:]')" ]; then + echo "::notice::No cartridges changed in this PR — skipping per-cartridge .so build." + exit 0 + fi + while IFS= read -r cart; do + [ -z "$cart" ] && continue echo "Building $cart .so..." cd "cartridges/$cart/ffi" && zig build cd "$GITHUB_WORKSPACE" - done + done <<< "$CARTS" - name: Build static library run: cd ffi/zig && zig build lib From 151b3d49e8b6a9ff9412456be8d54be18a49e8d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 24 May 2026 20:00:58 +0000 Subject: [PATCH 2/3] ci: trigger zig-test on workflow self-edits (mirror abi-drift) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So this PR's own workflow change actually runs the workflow. abi-drift.yml already includes itself in `paths:` — apply the same convention to zig-test.yml. --- .github/workflows/zig-test.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/zig-test.yml b/.github/workflows/zig-test.yml index cacd7b52..3809517e 100644 --- a/.github/workflows/zig-test.yml +++ b/.github/workflows/zig-test.yml @@ -11,11 +11,13 @@ on: paths: - 'ffi/**' - 'cartridges/**/ffi/**' + - '.github/workflows/zig-test.yml' pull_request: branches: [main] paths: - 'ffi/**' - 'cartridges/**/ffi/**' + - '.github/workflows/zig-test.yml' permissions: contents: read From b8b4b2aa4dc1ac5cf445d52f0eb25e13448c5ead Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 24 May 2026 20:02:49 +0000 Subject: [PATCH 3/3] ci(abi-drift): drop the ${{}} ternary fallback that re-expanded to full sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous refactor used: CARTS: ${{ github.event_name == 'pull_request' && steps.filter.outputs.carts || steps.discover.outputs.carts }} GitHub Actions expressions short-circuit JavaScript-style: when steps.filter.outputs.carts is an EMPTY STRING (which is exactly the intended PR-with-no-cartridge-changes case), the expression treats it as falsy and falls through to steps.discover.outputs.carts — the full 66-cartridge allowlist. That's why PR #147 (touches only .github/workflows/*) still reported drift in container-mcp / git-mcp / queues-mcp / vordr-mcp: the filter worked correctly but its empty output was discarded. Replace the two-step filter+ternary pattern with one always-correct step (id: scope) that handles both PR and push internally and writes a single output. Mirrors what zig-test.yml already does. --- .github/workflows/abi-drift.yml | 49 +++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/.github/workflows/abi-drift.yml b/.github/workflows/abi-drift.yml index c49523fc..3cf24685 100644 --- a/.github/workflows/abi-drift.yml +++ b/.github/workflows/abi-drift.yml @@ -174,42 +174,49 @@ jobs: # actually changed in this PR — unrelated cartridge drift is the # main branch's problem, not this PR's. On push to main, scan the # full allowlist so cross-cutting regressions are still caught at - # trunk. Empty changed-set on a PR ⇒ skip the loop (the workflow - # `paths:` filter shouldn't have triggered us anyway, but stay - # defensive). - - name: Restrict to cartridges changed in this PR - id: filter - if: github.event_name == 'pull_request' + # trunk. Empty changed-set on a PR ⇒ skip the loop (handled in + # the verify step's defensive empty check below). + # + # Single output, always-correct: scope.outputs.carts is the + # PR-filtered allowlist on pull_request, the full allowlist on + # push. Avoids the `${{ X && Y || Z }}` ternary footgun where Y + # being an empty string short-circuits to Z (which would silently + # re-expand to the full sweep — exactly the bug we're trying to + # fix). + - name: Scope cartridges to verify + id: scope env: + EVENT: ${{ github.event_name }} BASE_REF: ${{ github.base_ref }} CARTS: ${{ steps.discover.outputs.carts }} run: | set -euo pipefail - git fetch --no-tags --depth=50 origin "$BASE_REF" || true - changed=$(git diff --name-only "origin/${BASE_REF}...HEAD" -- 'cartridges/**' \ - | awk -F/ '{print $2}' | sort -u) - { - echo 'carts<> "$GITHUB_OUTPUT" - echo "Changed cartridges intersected with allowlist:" - while IFS= read -r cart; do - [ -z "$cart" ] && continue - if printf '%s\n' "$changed" | grep -qx "$cart"; then - echo " • $cart" - fi - done <<< "$CARTS" + echo "Cartridges in scope for this run:" + printf ' • %s\n' $(printf '%s\n' "$scope" | grep -v '^$' || true) - name: Emit + verify each cartridge env: - # On pull_request: filtered set. On push/main: full allowlist. - CARTS: ${{ github.event_name == 'pull_request' && steps.filter.outputs.carts || steps.discover.outputs.carts }} + CARTS: ${{ steps.scope.outputs.carts }} run: | set -euo pipefail if [ -z "$(printf '%s' "$CARTS" | tr -d '[:space:]')" ]; then