[slice-2] Wire integration tests into PR CI; expand coverage (#2474)

.github-staging/workflows/test-integration.yml

@@ -0,0 +1,181 @@
+name: Integration Tests
+
+on:
+  workflow_call:
+    outputs:
+      passed:
+        description: 'Whether all integration tests passed'
+        value: ${{ jobs.aggregate.outputs.passed }}
+  workflow_dispatch:  # Allow manual testing
+
+jobs:
+  integration:
+    name: Integration Tests
+    runs-on: ubuntu-latest
+    # The 30-min budget set on the caller (.github/workflows/test.yml)
+    # is repeated here as defense in depth so the budget is enforced
+    # whether this workflow is invoked via `uses:` or via
+    # `workflow_dispatch`.
+    timeout-minutes: 30
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.14"
+
+      - name: Install uv
+        uses: astral-sh/setup-uv@v4
+
+      - name: Install dependencies
+        run: uv sync --extra dev
+
+      - name: Build containers
+        run: |
+          docker build -t egg-gateway -f gateway/Dockerfile .
+          docker build -t egg-sandbox -f sandbox/Dockerfile .
+
+      - name: Set up k3s
+        run: |
+          curl -sfL https://get.k3s.io | INSTALL_K3S_EXEC="--flannel-backend=none --disable-network-policy --write-kubeconfig-mode=644" sh -
+          export KUBECONFIG=/etc/rancher/k3s/k3s.yaml
+          echo "KUBECONFIG=/etc/rancher/k3s/k3s.yaml" >> "$GITHUB_ENV"
+          # Install Calico CNI
+          scripts/install-calico.sh
+          # Wait for node to be ready
+          kubectl wait --for=condition=Ready node --all --timeout=120s
+
+      - name: Import images into k3s
+        # Flake guard (HITL Q1 of #2474): retry the image-import step
+        # up to 3 attempts with a short backoff. Transient k3s
+        # containerd import failures (rare but observed in #2556's
+        # early runs) should not flake the entire integration tier.
+        # `set -o pipefail` makes a `docker save` failure on the
+        # left side of the pipe propagate into the `if`'s test
+        # result (rather than being swallowed by an exit-0
+        # `k3s ctr images import` on the right side reading an
+        # empty stream); the surrounding `if … then exit 0 … fi`
+        # then keeps `set -e` suspended for the pipeline so a
+        # failure falls through to the next retry instead of
+        # aborting the script.
+        run: |
+          set -eo pipefail
+          attempt=0
+          max_attempts=3
+          until [ "$attempt" -ge "$max_attempts" ]; do
+            attempt=$((attempt + 1))
+            echo "::group::Image import attempt ${attempt}/${max_attempts}"
+            if docker save egg-gateway:latest | sudo k3s ctr images import - \
+              && docker save egg-sandbox:latest | sudo k3s ctr images import -; then
+              echo "::endgroup::"
+              echo "Image import succeeded on attempt ${attempt}"
+              exit 0
+            fi
+            echo "::endgroup::"
+            echo "Image import failed on attempt ${attempt}"
+            if [ "$attempt" -lt "$max_attempts" ]; then
+              echo "Sleeping 5s before retry..."
+              sleep 5
+            fi
+          done
+          echo "Image import failed after ${max_attempts} attempts" >&2
+          exit 1
+
+      - name: Deploy egg to k3s
+        run: |
+          kubectl apply -k k8s/overlays/local/
+          kubectl -n egg-system wait --for=condition=Available deployment/egg-gateway --timeout=120s
+
+      - name: Run integration and security tests
+        env:
+          EGG_RUNTIME: kubernetes
+          KUBECONFIG: /etc/rancher/k3s/k3s.yaml
+        run: |
+          PYTHONPATH=shared .venv/bin/pytest integration_tests -v \
+            -m "integration or security" \
+            --timeout=300
+
+      - name: Collect k3s debug artifacts on failure
+        # Flake guard (HITL Q1 of #2474): on any prior-step failure,
+        # capture cluster-wide events and per-pod logs so the
+        # debugger doesn't have to reproduce locally to triage a CI
+        # flake. Uploaded as a single `k3s-debug` workflow artifact.
+        # NB: `kubectl logs` requires an explicit pod name or a
+        # non-empty label selector - there is no "all pods in
+        # namespace" primitive, so we enumerate pods per namespace
+        # via `kubectl get pods -o name` and tail each individually.
+        if: failure()
+        run: |
+          set +e
+          echo "Collecting cluster events..."
+          kubectl get events --all-namespaces -o yaml > k3s-debug-events.yaml 2>&1 || true
+          echo "Collecting pod logs..."
+          {
+            for ns in egg-system egg-test-agents; do
+              echo "===== Namespace: ${ns} ====="
+              for pod in $(kubectl get pods -n "${ns}" -o name 2>/dev/null); do
+                echo "----- ${ns}/${pod} (current) -----"
+                kubectl logs -n "${ns}" "${pod}" --all-containers=true --tail=-1 --prefix=true 2>&1 || true
+                # CrashLoopBackOff is the exact failure mode this artifact
+                # is meant to triage, and `kubectl logs` without
+                # `--previous` returns ONLY the current container
+                # instance's logs — so the crashing instance's stderr
+                # (the part the debugger actually needs) is invisible.
+                # Add a second pass for the previous instance; `|| true`
+                # keeps the call safe for pods with no previous instance.
+                echo "----- ${ns}/${pod} (previous) -----"
+                kubectl logs -n "${ns}" "${pod}" --all-containers=true --previous --tail=-1 --prefix=true 2>&1 || true
+              done
+              echo
+              echo "===== Pods in ${ns} ====="
+              kubectl get pods -n "${ns}" -o wide 2>&1 || true
+              echo
+            done
+          } > k3s-debug-pods.log 2>&1 || true
+          ls -la k3s-debug-events.yaml k3s-debug-pods.log || true
+
+      - name: Upload k3s debug artifacts
+        if: failure()
+        uses: actions/upload-artifact@v4
+        with:
+          name: k3s-debug
+          path: |
+            k3s-debug-events.yaml
+            k3s-debug-pods.log
+          if-no-files-found: warn
+          retention-days: 14
+
+      - name: Cleanup
+        if: always()
+        run: |
+          kubectl delete namespace egg-test-agents --ignore-not-found=true 2>/dev/null || true
+          kubectl delete namespace egg-system --ignore-not-found=true 2>/dev/null || true
+          /usr/local/bin/k3s-uninstall.sh 2>/dev/null || true
+
+  aggregate:
+    name: Aggregate Integration Test Results
+    runs-on: ubuntu-latest
+    if: always()
+    needs: [integration]
+    outputs:
+      passed: ${{ steps.check.outputs.passed }}
+    steps:
+      - name: Check all jobs passed
+        id: check
+        run: |
+          if [[ "${{ needs.integration.result }}" != "success" ]]; then
+            echo "passed=false" >> "$GITHUB_OUTPUT"
+            echo "Integration tests failed"
+            echo "  integration: ${{ needs.integration.result }}"
+            # Without `exit 1`, the failure branch falls through with a
+            # zero exit code and this aggregate job reports success even
+            # when the integration tier was red. For the reusable-workflow
+            # `uses:` path the caller's job-failure propagation usually
+            # also fails the parent's `integration` job, but the
+            # standalone `workflow_dispatch` path has no such backstop.
+            exit 1
+          else
+            echo "passed=true" >> "$GITHUB_OUTPUT"
+            echo "All integration tests passed"
+          fi

.github-staging/workflows/test.yml

@@ -0,0 +1,108 @@
+name: Test
+
+on:
+  pull_request:
+    types: [opened, synchronize, reopened]
+  workflow_call:
+    outputs:
+      passed:
+        description: 'Whether all tests passed'
+        value: ${{ jobs.aggregate.outputs.passed }}
+  workflow_dispatch:  # Allow manual testing
+
+concurrency:
+  group: test-${{ github.head_ref || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  unit:
+    name: Unit Tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.14"
+
+      - name: Install uv
+        uses: astral-sh/setup-uv@v4
+
+      - name: Install dependencies
+        run: uv sync --extra dev
+
+      - name: Run unit tests
+        run: |
+          make test-all PYTEST_ARGS="--cov=gateway --cov=shared --cov=sandbox --cov-report=term-missing --cov-fail-under=80"
+
+  security:
+    name: Security Scan
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.14"
+
+      - name: Install uv
+        uses: astral-sh/setup-uv@v4
+
+      - name: Install dependencies
+        run: uv sync --extra dev
+
+      - name: Run security scan
+        run: make security
+
+  integration:
+    name: Integration Tests
+    # Required-from-day-1 per decision-3 of #2474. The reusable
+    # workflow lives at .github/workflows/test-integration.yml after
+    # the human reviewer performs the pre-merge `git mv` from
+    # .github-staging/. Folded into `aggregate` below so the
+    # canonical required-check name stays `Test / aggregate`.
+    #
+    # `timeout-minutes` historically wasn't in the documented
+    # keyword set for `uses:` caller jobs in GitHub Actions, but is
+    # honored in practice. The test-integration.yml reusable
+    # workflow also carries per-job `timeout-minutes` as a defense
+    # in depth so the budget is enforced regardless.
+    uses: ./.github/workflows/test-integration.yml
+    timeout-minutes: 30
+
+  aggregate:
+    # Intentionally NO `name:` override here: the GitHub-rendered
+    # check name must be `Test / aggregate` (lowercase job-id) so it
+    # matches the canonical required-check name documented in
+    # decision-3 / `manual_steps` of #2474 ("repo admin flips
+    # `Test / aggregate` to required-for-merge"). Adding a `name:`
+    # override would render the check as `Test / <override>` and
+    # silently desync from the operator-flipped required-check.
+    runs-on: ubuntu-latest
+    if: always()
+    needs: [unit, security, integration]
+    outputs:
+      passed: ${{ steps.check.outputs.passed }}
+    steps:
+      - name: Check all jobs passed
+        id: check
+        run: |
+          if [[ "${{ needs.unit.result }}" != "success" || \
+                "${{ needs.security.result }}" != "success" || \
+                "${{ needs.integration.result }}" != "success" ]]; then
+            echo "passed=false" >> "$GITHUB_OUTPUT"
+            echo "Some tests failed"
+            echo "  unit: ${{ needs.unit.result }}"
+            echo "  security: ${{ needs.security.result }}"
+            echo "  integration: ${{ needs.integration.result }}"
+            # Without this `exit 1`, the failure branch falls through with
+            # a zero exit code and the aggregate job — and the canonical
+            # required-for-merge `Test / aggregate` check — would report
+            # success regardless of which tier was red.
+            exit 1
+          else
+            echo "passed=true" >> "$GITHUB_OUTPUT"
+            echo "All tests passed"
+          fi

.github/workflows/lint.yml

@@ -156,6 +156,11 @@ jobs:
             echo "  docker: ${{ needs.docker.result }}"
             echo "  actions: ${{ needs.actions.result }}"
             echo "  custom-checks: ${{ needs.custom-checks.result }}"
+            # Without this `exit 1`, the failure branch falls through
+            # with a zero exit code and this aggregate would report
+            # success even when a lint tier was red — defeating any
+            # branch-protection rule that requires `Lint / aggregate`.
+            exit 1
           else
             echo "passed=true" >> "$GITHUB_OUTPUT"
             echo "All lint checks passed"

orchestrator/routes/pipelines.py

@@ -9041,6 +9041,35 @@ def _build_github_staging_manual_step(worktree_repo_path: Path) -> str:
     if not staged_paths:
         return ""
 
+    # Compute concrete move commands per staged file, choosing
+    # ``git mv`` vs ``git rm`` + ``git mv`` based on whether the target
+    # ``.github/<rest>`` already exists.  ``git mv`` refuses to
+    # overwrite an existing destination, so a template that always
+    # emits the plain form breaks for replacement scenarios (e.g.
+    # restaging an existing workflow).
+    staging_prefix = ".github-staging/"
+    target_prefix = ".github/"
+    mkdir_dirs: list[str] = []
+    move_cmds: list[str] = []
+    for rel in staged_paths:
+        if not rel.startswith(staging_prefix):
+            continue
+        rest = rel[len(staging_prefix) :]
+        target_rel = f"{target_prefix}{rest}"
+        target_dir = target_rel.rsplit("/", 1)[0] if "/" in rest else target_prefix.rstrip("/")
+        if target_dir and target_dir not in mkdir_dirs:
+            mkdir_dirs.append(target_dir)
+        target_abs = worktree_repo_path / target_rel
+        # ``Path.exists()`` follows symlinks and returns False for a
+        # broken link, so an existing-but-broken symlink would slip
+        # through the existence check and ``git mv`` would still refuse
+        # to overwrite it. ``Path.is_symlink()`` returns True regardless
+        # of whether the target resolves, so the disjunction catches
+        # regular files, valid symlinks, and broken symlinks.
+        if target_abs.is_symlink() or target_abs.exists():
+            move_cmds.append(f"git rm {target_rel}  # target exists; remove before mv")
+        move_cmds.append(f"git mv {rel} {target_rel}")
+
     lines = [
         "### Move staged `.github/` changes (auto-generated, issue #2508)",
         "",
@@ -9060,12 +9089,21 @@ def _build_github_staging_manual_step(worktree_repo_path: Path) -> str:
             "",
             "1. Review each staged file for correctness — these are proposed "
             "CI / repo-config changes that bypass the agent's normal sandbox.",
-            "2. Move each file from `.github-staging/<path>` to `.github/<path>`. For example:",
+            "2. Run the following to move each staged file into `.github/` "
+            "(commands below are pre-computed for this PR; replacement targets "
+            "are handled via `git rm` + `git mv` since `git mv` refuses to "
+            "overwrite an existing destination):",
             "   ```",
-            "   mkdir -p .github/workflows",
-            "   git mv .github-staging/workflows/test-e2e.yml .github/workflows/test-e2e.yml",
+        ]
+    )
+    for d in mkdir_dirs:
+        lines.append(f"   mkdir -p {d}")
+    for cmd in move_cmds:
+        lines.append(f"   {cmd}")
+    lines.extend(
+        [
             "   ```",
-            "   After the `git mv`, `.github-staging/` is no longer tracked "
+            "   After the moves, `.github-staging/` is no longer tracked "
             "by git (git doesn't track empty directories). Run "
             "`rm -rf .github-staging` locally if you want to clear any "
             "leftover empty subdirectories from your worktree.",