142b8e9d5b
Supersedes #2321 + #2322. Applies the same shape uniformly across every required check that uses a path filter: Canvas (Next.js), Platform (Go), Python Lint & Test, Shellcheck (E2E scripts). The bug + fix in one paragraph: GitHub registers a check run for every job whose `name:` matches the required-check context, regardless of whether the job actually executed. A job-level `if:` that evaluates false produces a SKIPPED check run. Branch protection's "required check" rule looks at the SET of check runs with the matching context name on the latest commit and treats any conclusion other than SUCCESS as not-passed — including SKIPPED. Adding a sibling no-op job under the same `name:` (PR #2321 / #2322 attempt) doesn't help: branch protection still sees the SKIPPED sibling and stays BLOCKED. The shape that works: ONE job per required check name, no job-level `if:`, all real work gated per-step. The job always runs and reports SUCCESS regardless of which paths changed. This patch: * Canvas (Next.js): drops the `canvas-build-noop` shadow added in #2321 (which didn't actually clear merge state — verified live on PR #2314). Refactors `canvas-build` to always run; gates checkout/ setup-node/install/build/test on `if: needs.changes.outputs.canvas == 'true'`. Coverage upload step also gated. * Platform (Go): drops job-level `if:`. Gates checkout/setup-go/ download/build/vet/lint/test/coverage-report/threshold-check on per-step `if:`. * Python Lint & Test: drops job-level `if:`. Gates checkout/setup- python/install/pytest on per-step `if:`. * Shellcheck (E2E scripts): drops job-level `if:`. Gates checkout/ shellcheck-run on per-step `if:`. Each refactored job adds a leading no-op echo step with `working-directory: .` override so the always-running spin-up doesn't fail when the path- filter-true working-directory (workspace, workspace-server, canvas) doesn't exist after no-op checkout. Why all four in one PR: the bug shape is identical across all four, and a future PR that only touches workspace-server (passing platform filter, missing canvas/python/scripts) would hit the same BLOCKED state on whichever filter it missed. PR-A and PR-2321 merged because their diffs happened to trigger every filter; PR-B (#2314) only missed canvas. Fixing one at a time means re-living this debugging cycle three more times. Cost: ~10s of always-on CI runtime per PR per job (the ubuntu-latest spin-up + the no-op echo). 40s aggregate, negligible vs. the manual- merge cost when BLOCKED catches us. Memory `feedback_branch_protection_check_name_parity` already updated (2026-04-29) to mark the original two-jobs-sharing-name pattern as DO NOT FOLLOW and document the working shape this PR uses. Refs PR #2321 (the misguided fix-attempt that this supersedes).
352 lines
17 KiB
YAML
352 lines
17 KiB
YAML
name: CI
|
||
|
||
on:
|
||
push:
|
||
branches: [main, staging]
|
||
pull_request:
|
||
branches: [main, staging]
|
||
# GitHub merge queue fires `merge_group` for the queue's pre-merge CI run.
|
||
# Required so the queue gets a real check result instead of a false-green
|
||
# from the absence of a triggered workflow. Safe to add unconditionally —
|
||
# the event simply doesn't fire until the queue is enabled on the branch.
|
||
merge_group:
|
||
types: [checks_requested]
|
||
|
||
# Cancel in-progress CI runs when a new commit arrives on the same ref.
|
||
# This prevents stale runs from queuing behind each other. The merge_group
|
||
# refs (refs/heads/gh-readonly-queue/...) get their own concurrency group
|
||
# automatically because github.ref differs from the PR ref.
|
||
concurrency:
|
||
group: ci-${{ github.ref }}
|
||
cancel-in-progress: true
|
||
|
||
jobs:
|
||
# Detect which paths changed so downstream jobs can skip when only
|
||
# docs/markdown files were modified.
|
||
changes:
|
||
name: Detect changes
|
||
runs-on: ubuntu-latest
|
||
outputs:
|
||
platform: ${{ steps.check.outputs.platform }}
|
||
canvas: ${{ steps.check.outputs.canvas }}
|
||
python: ${{ steps.check.outputs.python }}
|
||
scripts: ${{ steps.check.outputs.scripts }}
|
||
steps:
|
||
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
|
||
with:
|
||
fetch-depth: 0
|
||
- id: check
|
||
run: |
|
||
# For PR events: diff against the base branch (not HEAD~1 of the branch,
|
||
# which may be unrelated after force-pushes). When a push updates a PR,
|
||
# both pull_request and push events fire — prefer the PR base so that
|
||
# the diff is always computed against the actual merge base, not the
|
||
# previous SHA on the branch which may be on a different history line.
|
||
BASE="${GITHUB_BASE_REF:-${{ github.event.before }}}"
|
||
# GITHUB_BASE_REF is set by GitHub for PR events (the base branch name).
|
||
# For pull_request events we use the stored base.sha; for push events
|
||
# (or when base.sha is unavailable) fall back to github.event.before.
|
||
if [ "${{ github.event_name }}" = "pull_request" ] && [ -n "${{ github.event.pull_request.base.sha }}" ]; then
|
||
BASE="${{ github.event.pull_request.base.sha }}"
|
||
fi
|
||
# Fallback: if BASE is empty or all zeros (new branch), run everything
|
||
if [ -z "$BASE" ] || echo "$BASE" | grep -qE '^0+$'; then
|
||
echo "platform=true" >> "$GITHUB_OUTPUT"
|
||
echo "canvas=true" >> "$GITHUB_OUTPUT"
|
||
echo "python=true" >> "$GITHUB_OUTPUT"
|
||
echo "scripts=true" >> "$GITHUB_OUTPUT"
|
||
exit 0
|
||
fi
|
||
DIFF=$(git diff --name-only "$BASE" HEAD 2>/dev/null || echo ".github/workflows/ci.yml")
|
||
echo "platform=$(echo "$DIFF" | grep -qE '^workspace-server/|^\.github/workflows/ci\.yml$' && echo true || echo false)" >> "$GITHUB_OUTPUT"
|
||
echo "canvas=$(echo "$DIFF" | grep -qE '^canvas/|^\.github/workflows/ci\.yml$' && echo true || echo false)" >> "$GITHUB_OUTPUT"
|
||
echo "python=$(echo "$DIFF" | grep -qE '^workspace/|^\.github/workflows/ci\.yml$' && echo true || echo false)" >> "$GITHUB_OUTPUT"
|
||
echo "scripts=$(echo "$DIFF" | grep -qE '^tests/e2e/|^scripts/|^infra/scripts/|^\.github/workflows/ci\.yml$' && echo true || echo false)" >> "$GITHUB_OUTPUT"
|
||
|
||
# Platform (Go) is a required check on staging. Always-run + per-step
|
||
# gating (see Canvas (Next.js) for the rationale and the failure mode
|
||
# this avoids).
|
||
platform-build:
|
||
name: Platform (Go)
|
||
needs: changes
|
||
runs-on: ubuntu-latest
|
||
defaults:
|
||
run:
|
||
working-directory: workspace-server
|
||
steps:
|
||
- if: needs.changes.outputs.platform != 'true'
|
||
working-directory: .
|
||
run: echo "No platform/** changes — skipping real build steps; this job always runs to satisfy the required-check name on branch protection."
|
||
- if: needs.changes.outputs.platform == 'true'
|
||
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
|
||
- if: needs.changes.outputs.platform == 'true'
|
||
uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5
|
||
with:
|
||
go-version: 'stable'
|
||
- if: needs.changes.outputs.platform == 'true'
|
||
run: go mod download
|
||
- if: needs.changes.outputs.platform == 'true'
|
||
run: go build ./cmd/server
|
||
# CLI (molecli) moved to standalone repo: github.com/Molecule-AI/molecule-cli
|
||
- if: needs.changes.outputs.platform == 'true'
|
||
run: go vet ./... || true
|
||
- if: needs.changes.outputs.platform == 'true'
|
||
name: Run golangci-lint
|
||
run: golangci-lint run --timeout 3m ./... || true
|
||
- if: needs.changes.outputs.platform == 'true'
|
||
name: Run tests with race detection and coverage
|
||
run: go test -race -coverprofile=coverage.out ./...
|
||
|
||
- if: needs.changes.outputs.platform == 'true'
|
||
name: Per-file coverage report
|
||
# Advisory — lists every source file with its coverage so reviewers
|
||
# can see at-a-glance where gaps are. Sorted ascending so the worst
|
||
# offenders float to the top. Does NOT fail the build; the hard
|
||
# gate is the threshold check below. (#1823)
|
||
run: |
|
||
echo "=== Per-file coverage (worst first) ==="
|
||
go tool cover -func=coverage.out \
|
||
| grep -v '^total:' \
|
||
| awk '{file=$1; sub(/:[0-9][0-9.]*:.*/, "", file); pct=$NF; gsub(/%/,"",pct); s[file]+=pct; c[file]++}
|
||
END {for (f in s) printf "%6.1f%% %s\n", s[f]/c[f], f}' \
|
||
| sort -n
|
||
|
||
- if: needs.changes.outputs.platform == 'true'
|
||
name: Check coverage thresholds
|
||
# Enforces two gates from #1823 Layer 1:
|
||
# 1. Total floor (25% — ratchet plan in COVERAGE_FLOOR.md).
|
||
# 2. Per-file floor — non-test .go files in security-critical
|
||
# paths with coverage <10% fail the build, UNLESS the file
|
||
# path is listed in .coverage-allowlist.txt (acknowledged
|
||
# historical debt with a tracking issue + expiry).
|
||
run: |
|
||
set -e
|
||
TOTAL_FLOOR=25
|
||
# Security-critical paths where a 0%-coverage file is a real risk.
|
||
CRITICAL_PATHS=(
|
||
"internal/handlers/tokens"
|
||
"internal/handlers/workspace_provision"
|
||
"internal/handlers/a2a_proxy"
|
||
"internal/handlers/registry"
|
||
"internal/handlers/secrets"
|
||
"internal/middleware/wsauth"
|
||
"internal/crypto"
|
||
)
|
||
|
||
TOTAL=$(go tool cover -func=coverage.out | grep '^total:' | awk '{print $3}' | sed 's/%//')
|
||
echo "Total coverage: ${TOTAL}%"
|
||
if awk "BEGIN{exit !($TOTAL < $TOTAL_FLOOR)}"; then
|
||
echo "::error::Total coverage ${TOTAL}% is below the ${TOTAL_FLOOR}% floor. See COVERAGE_FLOOR.md for ratchet plan."
|
||
exit 1
|
||
fi
|
||
|
||
# Aggregate per-file coverage → /tmp/perfile.txt: "<fullpath> <pct>"
|
||
go tool cover -func=coverage.out \
|
||
| grep -v '^total:' \
|
||
| awk '{file=$1; sub(/:[0-9][0-9.]*:.*/, "", file); pct=$NF; gsub(/%/,"",pct); s[file]+=pct; c[file]++}
|
||
END {for (f in s) printf "%s %.1f\n", f, s[f]/c[f]}' \
|
||
> /tmp/perfile.txt
|
||
|
||
# Build allowlist — paths relative to workspace-server, one per line.
|
||
# Lines starting with # are comments.
|
||
ALLOWLIST=""
|
||
if [ -f ../.coverage-allowlist.txt ]; then
|
||
ALLOWLIST=$(grep -vE '^(#|[[:space:]]*$)' ../.coverage-allowlist.txt || true)
|
||
fi
|
||
|
||
FAILED=0
|
||
WARNED=0
|
||
for path in "${CRITICAL_PATHS[@]}"; do
|
||
while read -r file pct; do
|
||
[[ "$file" == *_test.go ]] && continue
|
||
[[ "$file" == *"$path"* ]] || continue
|
||
awk "BEGIN{exit !($pct < 10)}" || continue
|
||
|
||
# Strip the package-import prefix so we can match .coverage-allowlist.txt
|
||
# entries written as paths relative to workspace-server/.
|
||
# Handle both module paths: platform/workspace-server/... and platform/...
|
||
rel=$(echo "$file" | sed 's|^github.com/Molecule-AI/molecule-monorepo/platform/workspace-server/||; s|^github.com/Molecule-AI/molecule-monorepo/platform/||')
|
||
|
||
if echo "$ALLOWLIST" | grep -qxF "$rel"; then
|
||
echo "::warning file=workspace-server/$rel::Critical file at ${pct}% coverage (allowlisted, #1823) — fix before expiry."
|
||
WARNED=$((WARNED+1))
|
||
else
|
||
echo "::error file=workspace-server/$rel::Critical file at ${pct}% coverage — must be >=10% (target 80%). See #1823. To acknowledge as known debt, add this path to .coverage-allowlist.txt."
|
||
FAILED=$((FAILED+1))
|
||
fi
|
||
done < /tmp/perfile.txt
|
||
done
|
||
|
||
echo ""
|
||
echo "Critical-path check: $FAILED new failures, $WARNED allowlisted warnings."
|
||
|
||
if [ "$FAILED" -gt 0 ]; then
|
||
echo ""
|
||
echo "$FAILED security-critical file(s) have <10% test coverage and are"
|
||
echo "NOT in the allowlist. These paths handle auth, tokens, secrets, or"
|
||
echo "workspace provisioning — a 0% file here is the exact gap that let"
|
||
echo "CWE-22, CWE-78, KI-005 slip through in past incidents. Either:"
|
||
echo " (a) add tests to raise coverage above 10%, or"
|
||
echo " (b) add the path to .coverage-allowlist.txt with an expiry date"
|
||
echo " and a tracking issue reference."
|
||
exit 1
|
||
fi
|
||
|
||
# Canvas (Next.js) — required check, always runs. See platform-build
|
||
# comment above for the rationale.
|
||
#
|
||
# Supersedes the canvas-build-noop pattern attempted in PR #2321: two
|
||
# jobs sharing `name:` doesn't actually satisfy branch protection
|
||
# because the SKIPPED check run sibling is treated as not-passed
|
||
# regardless of how many SUCCESS siblings it has. Verified empirically
|
||
# on PR #2314 — mergeStateStatus stayed BLOCKED until I collapsed to
|
||
# a single-job-with-conditional-steps shape.
|
||
canvas-build:
|
||
name: Canvas (Next.js)
|
||
needs: changes
|
||
runs-on: ubuntu-latest
|
||
defaults:
|
||
run:
|
||
working-directory: canvas
|
||
steps:
|
||
- if: needs.changes.outputs.canvas != 'true'
|
||
working-directory: .
|
||
run: echo "No canvas/** changes — skipping real build steps; this job always runs to satisfy the required-check name on branch protection."
|
||
- if: needs.changes.outputs.canvas == 'true'
|
||
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
|
||
- if: needs.changes.outputs.canvas == 'true'
|
||
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
|
||
with:
|
||
node-version: '22'
|
||
- if: needs.changes.outputs.canvas == 'true'
|
||
run: rm -f package-lock.json && npm install
|
||
- if: needs.changes.outputs.canvas == 'true'
|
||
run: npm run build
|
||
- if: needs.changes.outputs.canvas == 'true'
|
||
name: Run tests with coverage
|
||
# Coverage instrumentation is configured in canvas/vitest.config.ts
|
||
# (provider: v8, reporters: text + html + json-summary). Step 2 of
|
||
# #1815 — wires coverage into CI so we get a baseline visible on
|
||
# every PR. No threshold gate yet; thresholds dial in (Step 3, also
|
||
# tracked in #1815) after the team sees what current coverage is.
|
||
# Per the inline comment in vitest.config.ts: "first land
|
||
# observability so we can see the baseline, then dial in
|
||
# thresholds + a hard gate" — this PR ships the observability half.
|
||
run: npx vitest run --coverage
|
||
- name: Upload coverage summary as artifact
|
||
if: needs.changes.outputs.canvas == 'true' && always()
|
||
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
|
||
with:
|
||
name: canvas-coverage-${{ github.run_id }}
|
||
path: canvas/coverage/
|
||
retention-days: 7
|
||
if-no-files-found: warn
|
||
|
||
# MCP Server + SDK removed from CI — now in standalone repos:
|
||
# - github.com/Molecule-AI/molecule-mcp-server (npm CI)
|
||
# - github.com/Molecule-AI/molecule-sdk-python (PyPI CI)
|
||
|
||
# e2e-api job moved to .github/workflows/e2e-api.yml (issue #458).
|
||
# It now has workflow-level concurrency (cancel-in-progress: false) so
|
||
# new pushes queue the E2E run rather than cancelling it at the run level.
|
||
|
||
# Shellcheck (E2E scripts) — required check, always runs. See
|
||
# platform-build for the rationale.
|
||
shellcheck:
|
||
name: Shellcheck (E2E scripts)
|
||
needs: changes
|
||
runs-on: ubuntu-latest
|
||
steps:
|
||
- if: needs.changes.outputs.scripts != 'true'
|
||
run: echo "No tests/e2e/ or infra/scripts/ changes — skipping real shellcheck; this job always runs to satisfy the required-check name on branch protection."
|
||
- if: needs.changes.outputs.scripts == 'true'
|
||
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
|
||
- if: needs.changes.outputs.scripts == 'true'
|
||
name: Run shellcheck on tests/e2e/*.sh and infra/scripts/*.sh
|
||
# shellcheck is pre-installed on ubuntu-latest runners (via apt).
|
||
# infra/scripts/ is included because setup.sh + nuke.sh gate the
|
||
# README quickstart — a shellcheck regression there silently breaks
|
||
# new-user onboarding. scripts/ is intentionally excluded until its
|
||
# pre-existing SC3040/SC3043 warnings are cleaned up.
|
||
run: |
|
||
find tests/e2e infra/scripts -type f -name '*.sh' -print0 \
|
||
| xargs -0 shellcheck --severity=warning
|
||
|
||
canvas-deploy-reminder:
|
||
name: Canvas Deploy Reminder
|
||
runs-on: ubuntu-latest
|
||
needs: [changes, canvas-build]
|
||
# Only fires on direct pushes to main (i.e. after staging→main promotion).
|
||
if: needs.changes.outputs.canvas == 'true' && github.event_name == 'push' && github.ref == 'refs/heads/main'
|
||
permissions:
|
||
# Required to post commit comments via the GitHub API.
|
||
contents: write
|
||
steps:
|
||
- name: Post deploy reminder as commit comment
|
||
env:
|
||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||
COMMIT_SHA: ${{ github.sha }}
|
||
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
|
||
run: |
|
||
# Write body to a temp file — avoids backtick escaping in shell.
|
||
cat > /tmp/deploy-reminder.md << 'BODY'
|
||
## Canvas build passed ✅ — deploy required
|
||
|
||
The `publish-canvas-image` workflow is now building a fresh Docker image
|
||
(`ghcr.io/molecule-ai/canvas:latest`) in the background.
|
||
|
||
Once it completes (~3–5 min), apply on the host machine with:
|
||
```bash
|
||
cd <runner-workspace>
|
||
git pull origin main
|
||
docker compose pull canvas && docker compose up -d canvas
|
||
```
|
||
|
||
If you need to rebuild from local source instead (e.g. testing unreleased
|
||
changes or a new `NEXT_PUBLIC_*` URL), use:
|
||
```bash
|
||
docker compose build canvas && docker compose up -d canvas
|
||
```
|
||
BODY
|
||
printf '\n> Posted automatically by CI · commit `%s` · [build log](%s)\n' \
|
||
"$COMMIT_SHA" "$RUN_URL" >> /tmp/deploy-reminder.md
|
||
|
||
gh api \
|
||
--method POST \
|
||
"repos/${{ github.repository }}/commits/${{ github.sha }}/comments" \
|
||
--field "body=@/tmp/deploy-reminder.md"
|
||
|
||
# Python Lint & Test — required check, always runs. See platform-build
|
||
# for the rationale.
|
||
python-lint:
|
||
name: Python Lint & Test
|
||
needs: changes
|
||
runs-on: ubuntu-latest
|
||
env:
|
||
WORKSPACE_ID: test
|
||
defaults:
|
||
run:
|
||
working-directory: workspace
|
||
steps:
|
||
- if: needs.changes.outputs.python != 'true'
|
||
working-directory: .
|
||
run: echo "No workspace/** changes — skipping real lint+test; this job always runs to satisfy the required-check name on branch protection."
|
||
- if: needs.changes.outputs.python == 'true'
|
||
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
|
||
- if: needs.changes.outputs.python == 'true'
|
||
uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0
|
||
with:
|
||
python-version: '3.11'
|
||
cache: pip
|
||
cache-dependency-path: workspace/requirements.txt
|
||
- if: needs.changes.outputs.python == 'true'
|
||
run: pip install -r requirements.txt pytest pytest-asyncio pytest-cov
|
||
# Coverage flags + fail-under floor moved into workspace/pytest.ini
|
||
# (issue #1817) so local `pytest` and CI use identical config.
|
||
- if: needs.changes.outputs.python == 'true'
|
||
run: python -m pytest --tb=short
|
||
|
||
# SDK + plugin validation moved to standalone repo:
|
||
# github.com/Molecule-AI/molecule-sdk-python
|
||
|