Skip to content

Commit b44136c

Browse files
committed
ci(showcase): harden aimock-e2e workflow; matrix pytest on 3.10+3.12
showcase_aimock-e2e.yml: - workflow_dispatch slug is now a choice-type enum restricted to Python packages that ship aimock_toggle.py; a TS (mastra) or Java (spring-ai) slug would have skipped the Python agent start step and then failed with a misleading Playwright timeout. For comment-trigger paths, the pkg-type step short-circuits with a clear ::error:: when the slug does not ship aimock_toggle.py. - Slug parsing replaced PCRE grep -oP '\\K\\S+' with a POSIX-safe grep -oE + sed pipeline so BSD/Alpine grep works too (future-proof against runner image changes). - aimock pinned to @copilotkit/aimock@^1.14.3 (was @latest) with --ignore-scripts; unpinned @latest let a bad aimock publish silently poison CI for everyone. - pnpm install now runs with --ignore-scripts — trusted commenter triggers /test-aimock on untrusted PR content, so postinstall scripts must not run on the runner. - OPENAI_BASE_URL is no longer pre-set for Python agent start on packages that ship aimock_toggle.py — forcing the toggle itself to do the redirection proves the toggle works rather than masking it with env already set. - Dead AIMOCK_URL export removed from the Next.js dev-server step (the runtime only reads OPENAI_BASE_URL). - Python agent health-check extended 60s -> 90s (45 iter * 2s) for CrewAI cold imports. - actions/setup-python caches pip keyed on requirements.txt. - github-script Post-result step passes slug + job status via env rather than ${{ }} interpolation — even though the slug is already whitelisted, the env pattern is the defensive default for dynamic values. showcase_validate.yml: - python-unit-tests is now a matrix on Python 3.10 + 3.12 (fail-fast disabled) so the typing_extensions fallback branch gets CI coverage; previously only 3.12 ran. - pip install per package is a HARD FAIL (was ::warning:: + continue); hiding broken requirements.txt behind a warning let a package ship green with unresolvable runtime deps. A missing requirements.txt is still handled gracefully via the -f guard. - pip caching added via actions/setup-python.
1 parent 76f0c13 commit b44136c

2 files changed

Lines changed: 224 additions & 17 deletions

File tree

.github/workflows/showcase_aimock-e2e.yml

Lines changed: 146 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,18 @@ on:
66
workflow_dispatch:
77
inputs:
88
slug:
9-
description: "Package slug to test (e.g. langgraph-python), or 'all'"
9+
description: "Package slug to test (must ship aimock_toggle.py)"
1010
required: false
11-
default: "langgraph-python"
11+
# Only crewai-crews currently ships aimock_toggle.py and exercises
12+
# the AIMOCK_URL path end-to-end. Restricting the enum here prevents
13+
# accidental dispatch of a TS-only (mastra) or Java (spring-ai) slug
14+
# that would skip the Python agent startup step and then fail with a
15+
# misleading Playwright timeout instead of a clear "no toggle shipped"
16+
# error. When a new Python slug adds aimock_toggle.py, append it here.
17+
type: choice
18+
options:
19+
- crewai-crews
20+
default: "crewai-crews"
1221

1322
permissions:
1423
contents: read
@@ -17,10 +26,19 @@ permissions:
1726

1827
jobs:
1928
aimock-e2e:
20-
# Only run on PR comments matching /test-aimock, or manual dispatch
29+
# Only run on PR comments matching /test-aimock (from trusted authors), or manual dispatch.
30+
# The author_association gate prevents arbitrary third-party commenters from
31+
# triggering runs with attacker-controlled comment bodies (which the
32+
# 'Determine slug' step then parses — see env-based shell interpolation below).
2133
if: >
2234
github.event_name == 'workflow_dispatch' ||
23-
(github.event.issue.pull_request && contains(github.event.comment.body, '/test-aimock'))
35+
(github.event.issue.pull_request
36+
&& contains(github.event.comment.body, '/test-aimock')
37+
&& contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association))
38+
# Pinned to ubuntu-latest deliberately: the 'Determine slug' step uses
39+
# POSIX-only `grep -oE` + `sed` (no `grep -oP` / PCRE) so a future BSD
40+
# grep would still work, but ubuntu-latest keeps the install/setup matrix
41+
# consistent with every other showcase workflow.
2442
runs-on: ubuntu-latest
2543
timeout-minutes: 15
2644

@@ -55,14 +73,54 @@ jobs:
5573

5674
- name: Determine slug
5775
id: slug
76+
# SECURITY: comment body and dispatch slug are UNTRUSTED. Pass via env
77+
# (NOT via `${{ ... }}` expression interpolation) so shell never parses
78+
# attacker-controlled text. Then validate against a strict whitelist
79+
# before anything downstream uses $SLUG as a path / package name — so
80+
# `../../../etc/shadow` or similar cannot reach `cd`/`pip install`.
81+
env:
82+
EVENT_NAME: ${{ github.event_name }}
83+
COMMENT_BODY: ${{ github.event.comment.body }}
84+
DISPATCH_SLUG: ${{ github.event.inputs.slug }}
5885
run: |
59-
if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then
60-
echo "slug=${{ github.event.inputs.slug }}" >> "$GITHUB_OUTPUT"
86+
set -euo pipefail
87+
if [ "$EVENT_NAME" = "workflow_dispatch" ]; then
88+
SLUG="$DISPATCH_SLUG"
6189
else
62-
COMMENT="${{ github.event.comment.body }}"
63-
SLUG=$(echo "$COMMENT" | grep -oP '/test-aimock\s+\K\S+' || echo "langgraph-python")
64-
echo "slug=$SLUG" >> "$GITHUB_OUTPUT"
90+
# POSIX-safe extraction (no `grep -oP` / PCRE `\K`): pull the
91+
# whole `/test-aimock <slug>` match with `grep -oE`, then strip
92+
# the prefix via `sed`. Works on both GNU grep (ubuntu-latest) and
93+
# BSD grep (macOS / Alpine busybox) in case the runner image
94+
# changes out from under us.
95+
SLUG=$(printf '%s' "$COMMENT_BODY" \
96+
| grep -oE '/test-aimock[[:space:]]+[^[:space:]]+' \
97+
| head -n1 \
98+
| sed 's|^/test-aimock[[:space:]]*||' \
99+
|| true)
100+
# Fallback when the comment is bare `/test-aimock` (no slug).
101+
# crewai-crews is the only package currently shipping aimock_toggle.py
102+
# and exercising the AIMOCK_URL path end-to-end; langgraph-python
103+
# does not, so we fall back to crewai-crews (NOT langgraph-python).
104+
if [ -z "$SLUG" ]; then
105+
SLUG="crewai-crews"
106+
fi
107+
fi
108+
# Strict slug whitelist: lowercase alphanumerics + hyphens only. This
109+
# blocks path traversal (`../`), absolute paths, command substitution,
110+
# and anything else that could escape `showcase/packages/$SLUG`.
111+
case "$SLUG" in
112+
''|*[!a-z0-9-]*)
113+
echo "::error::Invalid slug '$SLUG' — must match ^[a-z0-9-]+$"
114+
exit 1
115+
;;
116+
esac
117+
# Belt-and-suspenders: the slug must correspond to an existing package
118+
# directory. Rejects typos and anything that bypasses the regex.
119+
if [ ! -d "showcase/packages/$SLUG" ]; then
120+
echo "::error::Slug '$SLUG' does not map to showcase/packages/$SLUG"
121+
exit 1
65122
fi
123+
echo "slug=$SLUG" >> "$GITHUB_OUTPUT"
66124
67125
- name: Detect package type
68126
id: pkg-type
@@ -74,10 +132,26 @@ jobs:
74132
else
75133
echo "has_python=false" >> "$GITHUB_OUTPUT"
76134
fi
135+
# The workflow's downstream steps (dev-server + Playwright) assume
136+
# a Python agent listening on :8000. Dispatching a TS (mastra) or
137+
# Java (spring-ai) slug would skip the Python agent start step and
138+
# then fail with a misleading Playwright timeout. Short-circuit
139+
# with a clear error instead — the workflow_dispatch enum narrows
140+
# this at the UI layer, but a comment-trigger slug bypasses that.
141+
if [ -f "$PKG_DIR/src/aimock_toggle.py" ]; then
142+
echo "ships_toggle=true" >> "$GITHUB_OUTPUT"
143+
else
144+
echo "::error::Slug '$SLUG' does not ship src/aimock_toggle.py — this workflow only exercises packages that wire AIMOCK_URL end-to-end. Add aimock_toggle.py (and requirements.txt) to the package first."
145+
exit 1
146+
fi
77147
78148
- name: Start aimock
79149
run: |
80-
npm install -g @copilotkit/aimock@latest
150+
# Pin aimock to a known-good floor (caret = safe patch/minor).
151+
# An unrestricted `@latest` means a bad aimock publish silently
152+
# poisons CI for everyone; pinning makes the upgrade explicit and
153+
# keeps this PR's CI signal reproducible.
154+
npm install -g "@copilotkit/aimock@^1.14.3" --ignore-scripts
81155
npx aimock --port 4010 --host 127.0.0.1 --fixtures showcase/aimock/feature-parity.json --validate-on-load &
82156
# Wait for aimock to be ready
83157
for i in $(seq 1 20); do
@@ -91,31 +165,76 @@ jobs:
91165
uses: actions/setup-python@v5
92166
with:
93167
python-version: "3.12"
168+
# Cache pip to avoid reinstalling CrewAI's heavy transitive dep
169+
# tree on every PR run. Key scopes to the selected slug so each
170+
# package gets its own cache bucket keyed on its requirements.txt.
171+
cache: "pip"
172+
cache-dependency-path: showcase/packages/${{ steps.slug.outputs.slug }}/requirements.txt
94173

95174
- name: Start Python agent
96175
if: steps.pkg-type.outputs.has_python == 'true'
176+
# NOTE: this workflow tests the SOURCE PACKAGE directly
177+
# (showcase/packages/<slug>), whose dev script binds the agent on port
178+
# 8000. Generated STARTERS (showcase/starters/<slug>) instead bind on
179+
# port 8123 — that's the production scaffold users copy. If you're
180+
# debugging a scaffolded starter, the health checks here won't apply.
97181
run: |
98182
SLUG="${{ steps.slug.outputs.slug }}"
99183
cd "showcase/packages/$SLUG"
100184
pip install -r requirements.txt
101-
OPENAI_BASE_URL=http://localhost:4010/v1 OPENAI_API_KEY=test-key python -m uvicorn agent:app --host 127.0.0.1 --port 8000 &
102-
# Wait for agent to be ready
103-
for i in $(seq 1 30); do
185+
# Prefer agent_server:app (FastAPI entrypoint used by non-langgraph
186+
# Python showcases). Legacy `agent:app` path retained for any
187+
# package that does not ship src/agent_server.py; langgraph-fastapi
188+
# uses `langgraph_cli dev` and is NOT exercised by this fallback.
189+
if [ -f "src/agent_server.py" ]; then
190+
export PYTHONPATH="$PWD/src:${PYTHONPATH:-}"
191+
APP_MODULE="agent_server:app"
192+
else
193+
APP_MODULE="agent:app"
194+
fi
195+
# Set AIMOCK_URL ONLY (not OPENAI_BASE_URL). Packages that ship
196+
# aimock_toggle.py MUST prove the toggle itself wires OPENAI_BASE_URL
197+
# + LITELLM_API_BASE + dummy key. Pre-setting OPENAI_BASE_URL here
198+
# would make a green E2E indistinguishable from "toggle worked"
199+
# vs "OPENAI_BASE_URL was already set before the toggle ran" — so
200+
# we deliberately leave the rest for configure_aimock() to inject.
201+
# OPENAI_API_KEY is left unset so the toggle's dummy-key injection
202+
# path is exercised too.
203+
AIMOCK_URL=http://localhost:4010/v1 \
204+
python -m uvicorn "$APP_MODULE" --host 127.0.0.1 --port 8000 &
205+
# Wait for agent to be ready. CrewAI's cold import (litellm + the
206+
# full crew graph) can exceed 60s on a cold runner, so give it 90s
207+
# (45 iterations × 2s) before declaring failure. Mirrors the aimock
208+
# start pattern: loop + hard-fail so a cryptic Playwright timeout
209+
# doesn't mask a bind/startup failure here.
210+
for i in $(seq 1 45); do
104211
curl -sf http://localhost:8000/health > /dev/null 2>&1 && break
105212
curl -sf http://localhost:8000/ > /dev/null 2>&1 && break
106213
sleep 2
107214
done
215+
curl -sf http://localhost:8000/health > /dev/null 2>&1 \
216+
|| curl -sf http://localhost:8000/ > /dev/null 2>&1 \
217+
|| { echo "Python agent failed to start on :8000"; exit 1; }
108218
109219
- name: Install package dependencies
110220
run: |
111221
SLUG="${{ steps.slug.outputs.slug }}"
112222
cd "showcase/packages/$SLUG"
113-
pnpm install
223+
# `--ignore-scripts`: a trusted commenter can run `/test-aimock` on
224+
# a PR whose package.json is untrusted content. Without this flag
225+
# an attacker's postinstall script would execute on the runner with
226+
# the workflow's token. The E2E path (Playwright + Next.js dev) does
227+
# not require install-time scripts to succeed.
228+
pnpm install --ignore-scripts
114229
115230
- name: Start dev server
116231
run: |
117232
SLUG="${{ steps.slug.outputs.slug }}"
118233
cd "showcase/packages/$SLUG"
234+
# The Next.js runtime consumes OPENAI_BASE_URL (not AIMOCK_URL), so
235+
# we only export what the runtime actually reads. A future Node-side
236+
# aimock toggle can be added later; until that lands there's no
237+
# reason to export dead config.
119238
OPENAI_BASE_URL=http://localhost:4010/v1 \
120239
OPENAI_API_KEY=test-key \
121240
AGENT_URL=http://localhost:8000 \
@@ -139,6 +258,8 @@ jobs:
139258
BASE_URL=http://localhost:3000 npx playwright test --reporter=list
140259
env:
141260
CI: "true"
261+
# Only export env the Next.js runtime actually reads (same as the
262+
# dev-server step above). AIMOCK_URL is a Python-side toggle.
142263
OPENAI_BASE_URL: http://localhost:4010/v1
143264
OPENAI_API_KEY: test-key
144265

@@ -154,14 +275,22 @@ jobs:
154275
- name: Post result to PR
155276
if: github.event_name == 'issue_comment' && always()
156277
uses: actions/github-script@v7
278+
# Pass dynamic values through env (NOT `${{ ... }}` interpolation into
279+
# the script body). Even though the slug is whitelisted upstream, the
280+
# env-var pattern is the defensive default: any future additions that
281+
# aren't pre-validated cannot accidentally reach script text.
282+
env:
283+
SLUG: ${{ steps.slug.outputs.slug }}
284+
JOB_STATUS: ${{ job.status }}
157285
with:
158286
script: |
159-
const status = '${{ job.status }}' === 'success' ? '✅' : '❌';
160-
const slug = '${{ steps.slug.outputs.slug }}';
287+
const slug = process.env.SLUG;
288+
const jobStatus = process.env.JOB_STATUS;
289+
const status = jobStatus === 'success' ? '✅' : '❌';
161290
const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
162291
await github.rest.issues.createComment({
163292
owner: context.repo.owner,
164293
repo: context.repo.repo,
165294
issue_number: context.issue.number,
166-
body: `${status} **Aimock E2E Tests** (\`${slug}\`): ${{ job.status }}\n\n[View run](${runUrl})`
295+
body: `${status} **Aimock E2E Tests** (\`${slug}\`): ${jobStatus}\n\n[View run](${runUrl})`
167296
});

.github/workflows/showcase_validate.yml

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,3 +381,81 @@ jobs:
381381
if: failure() && github.event_name == 'push' && env.SLACK_WEBHOOK == ''
382382
run: |
383383
echo "::warning::showcase_validate failed on push but SLACK_WEBHOOK_OSS_ALERTS is not set; no Slack notification sent."
384+
385+
python-unit-tests:
386+
name: Python unit tests (${{ matrix.python-version }})
387+
# Separate job so pre-existing `validate-parity` failures don't mask new
388+
# Python unit-test regressions. pytest runs independently of JS/TS checks.
389+
runs-on: ubuntu-latest
390+
timeout-minutes: 10
391+
strategy:
392+
# Fail-fast disabled so a 3.10-only regression (e.g. typing_extensions
393+
# fallback path breaking) doesn't cancel the 3.12 run and leave us
394+
# guessing which version is the actual problem.
395+
fail-fast: false
396+
matrix:
397+
# 3.10 covers the typing_extensions `NotRequired` fallback path used
398+
# by aimock_toggle.py (stdlib `NotRequired` only landed in 3.11).
399+
# 3.12 is the production/runner default. Pinning both guarantees we
400+
# catch a regression in either branch the first time it lands.
401+
python-version: ["3.10", "3.12"]
402+
steps:
403+
- name: Checkout
404+
uses: actions/checkout@v4
405+
406+
- name: Setup Python
407+
uses: actions/setup-python@v5
408+
with:
409+
python-version: ${{ matrix.python-version }}
410+
# Cache pip so the CrewAI-class heavy transitive dep trees don't
411+
# reinstall from scratch every PR run. Keyed on every package
412+
# requirements.txt in the matrix so a single drift invalidates.
413+
cache: "pip"
414+
cache-dependency-path: showcase/packages/*/requirements.txt
415+
416+
- name: Install pytest
417+
run: python -m pip install --quiet pytest
418+
419+
- name: Run showcase package Python unit tests
420+
# Keep scope narrow: only showcase/packages/*/tests/python/ directories
421+
# (not e2e, not langgraph which has its own runtime). Each package has
422+
# its own conftest.py that wires up import paths; we cd into the pkg
423+
# dir so those apply.
424+
#
425+
# Install each package's requirements.txt (when present) so pytest
426+
# collection does not fail on transitive imports. Today the crewai-crews
427+
# unit tests only touch aimock_toggle.py (stdlib-only), but future tests
428+
# may exercise modules that pull in package deps — paying the install
429+
# cost per-package is cheap and keeps this job future-proof.
430+
#
431+
# pip install is a HARD FAIL (no `|| warn`): hiding a broken
432+
# requirements.txt behind a warning means a package ships a PR green
433+
# while its runtime deps are unresolvable. The only acceptable
434+
# "skipped install" case is when requirements.txt doesn't exist,
435+
# which we handle via the `-f` guard below.
436+
run: |
437+
set -euo pipefail
438+
failed=0
439+
found=0
440+
for pkg_dir in showcase/packages/*/; do
441+
tests_dir="${pkg_dir}tests/python"
442+
[ -d "$tests_dir" ] || continue
443+
found=$((found + 1))
444+
pkg=$(basename "$pkg_dir")
445+
echo "--- pytest: $pkg ---"
446+
if [ -f "${pkg_dir}requirements.txt" ]; then
447+
echo "installing ${pkg}/requirements.txt"
448+
if ! python -m pip install --quiet -r "${pkg_dir}requirements.txt"; then
449+
echo "::error file=${pkg_dir}requirements.txt::pip install failed for $pkg"
450+
failed=1
451+
# Skip running pytest against a package whose deps failed —
452+
# the pytest output would be a confusing collection error.
453+
continue
454+
fi
455+
fi
456+
(cd "$pkg_dir" && python -m pytest tests/python/ -v) || failed=1
457+
done
458+
if [ "$found" -eq 0 ]; then
459+
echo "::warning::No showcase/packages/*/tests/python/ directories found"
460+
fi
461+
exit "$failed"

0 commit comments

Comments
 (0)