Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions .claude/skills/contributing-to-loopover/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ for maintainer approval (CI shows unverified → the engine **holds**, never clo

## 1. Every CI check → local command → what fails it

The single **required** status check is **`validate`** (it aggregates `changes, lint, test, workers,
mcp, ui, security`; a path-skipped job counts as success). **Codecov** posts `codecov/patch` (the real
coverage gate) and `codecov/project` (informational) independently. The review engine also posts its
own check run named **`LoopOver Orb Review Agent`** (`src/github/app.ts` `LOOPOVER_GATE_CHECK_NAME`) — the gate
verdict (§3), separate from CI. On a PR, jobs run only if their
path filter matched; on push to `main`, everything runs.
The **required** status checks on `main` are **`validate`** (it aggregates `changes, lint, test,
workers, mcp, ui, security`; a path-skipped job counts as success) and **`Superagent Security Scan`**
(a separate third-party GitHub App check, not part of this repo's own workflow files — confirmed via
`gh api repos/JSONbored/loopover/branches/main/protection/required_status_checks`). **Codecov** posts
`codecov/patch` (the real coverage gate) and `codecov/project` (informational) independently. The
review engine also posts its own check run named **`LoopOver Orb Review Agent`**
(`src/github/app.ts` `LOOPOVER_GATE_CHECK_NAME`) — the gate verdict (§3), separate from CI. On a PR,
jobs run only if their path filter matched; on push to `main`, everything runs.

| Check | Runs | Local command | Fails when |
|---|---|---|---|
Expand All @@ -32,7 +34,7 @@ path filter matched; on push to `main`, everything runs.
| lint → miner-env-reference | miner/AMS env-var doc drift | `npm run miner:env-reference:check` | committed `packages/loopover-miner/docs/env-reference.md` / `apps/loopover-ui/src/lib/ams-env-reference.ts` is stale (run `npm run miner:env-reference`) — miner/AMS twin of the selfhost check above |
| lint → observability | Grafana/Prometheus/alert config validation | `npm run selfhost:validate-observability` | a self-host observability config (dashboard/rule/datasource) is malformed |
| lint → typecheck | `tsc --noEmit` | `npm run typecheck` | any backend type error |
| test (1/2) | sharded vitest + coverage | `npm run test:coverage` (unsharded) | any failing `test/**/*.test.ts` (excl. `test/workers/**`) |
| test (1/6..6/6) | sharded vitest + coverage | `npm run test:coverage` (unsharded) | any failing `test/**/*.test.ts` (excl. `test/workers/**`) |
| workers | workers-pool vitest | `npm run test:workers` | any failing `test/workers/**` |
| mcp → build | MCP pkg build | `npm run build:mcp` | MCP package build error |
| mcp → pack | tarball hygiene | `npm run test:mcp-pack` | unexpected/forbidden file or stale README in the npm tarball |
Expand Down Expand Up @@ -71,7 +73,7 @@ these for a normal PR:**

| Local command | Why it's not in the table above |
|---|---|
| `npm run test:engine-parity`, `npm run test:live-gate-parity`, `npm run test:driver-parity` | Plain `test/contract/*.test.ts` files — no dedicated CI job, but they DO run in CI as part of whichever `test (1/2)` shard happens to contain them (sharded `vitest run`). |
| `npm run test:engine-parity`, `npm run test:live-gate-parity`, `npm run test:driver-parity` | Plain `test/contract/*.test.ts` files — no dedicated CI job, but they DO run in CI as part of whichever `test (1/6..6/6)` shard happens to contain them (sharded `vitest run`). |
| `npm run test --workspace @loopover/engine` | The engine package's own `node --test` suite. **Not run by `ci.yml` on a PR at all** — only by `.github/workflows/publish-engine.yml` at release time. A regression here is invisible to Codecov and to every PR-gating CI check; `test:ci` locally is the only pre-merge signal. |

This is a real, previously-hit gap, not a hypothetical: a past PR shipped a genuine, undetected
Expand All @@ -92,8 +94,11 @@ checks go green) is the only way to know you didn't break it.
- **Ignored paths** (no coverage obligation): `apps/**`, `test/**`, `scripts/**`, `src/env.d.ts`.
Coverage `include` is `src/**/*.ts` only. → A UI-only / test-only / script-only change owes **no**
patch coverage; a backend `src/**` change owes coverage on **every changed line + branch**.
- **Measure unsharded locally:** `npm run test:coverage`. CI shards into 2 and Codecov merges them,
- **Measure unsharded locally:** `npm run test:coverage`. CI shards into 6 and Codecov merges them,
so a single local shard under-reports — never trust it.
- **Flaky tests are already tracked.** Every shard uploads a JUnit report (`report_type: test_results`),
which auto-enables Codecov Test Analytics with no extra config — check a PR's "Tests" tab or its
Codecov bot comment if a test needed a retry, rather than assuming it's pure infra noise.

---

Expand Down
85 changes: 85 additions & 0 deletions .github/actions/setup-workspace/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
name: Setup workspace
description: >-
Strip untrusted npm config, set up Node, and restore/install/save the fork/trusted-scoped
node_modules cache. Extracted from ci.yml's validate-code, validate-tests, and validate-tests-merge
jobs, which had this exact sequence copy-pasted three times -- the same kind of drift that caused a
real cache-key mismatch bug this repo already hit once (two jobs' Turborepo cache pair silently
diverged when one was edited and the other wasn't). This doesn't fix that specific bug on its own,
but a future change to this sequence now only needs to happen here, not be remembered at every call
site. Does NOT check out the repo itself -- a local `uses: ./path` action reference needs the repo
already on disk to even find this file, so actions/checkout must run in the calling job BEFORE this
action is invoked, not inside it.

inputs:
save-cache:
description: >-
Whether to save the node_modules cache after a successful install. validate-tests-merge sets
this to "false" -- it only ever reads the cache validate-code/validate-tests already populate,
never writes to it, so a save here would just be redundant work.
required: false
default: "true"

outputs:
cache-hit:
description: Whether the node_modules cache was restored (passed through from actions/cache/restore).
value: ${{ steps.node-modules-cache.outputs.cache-hit }}

runs:
using: composite
steps:
- name: Neutralize untrusted npm config
shell: bash
run: rm -f .npmrc

- name: Setup Node
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
with:
node-version-file: .nvmrc
cache: npm

# actions/checkout wipes node_modules (git clean -ffdx) on every run regardless of the self-hosted
# runner's own persistence, and npm ci always deletes+reinstalls node_modules by design -- so
# neither the runner nor npm ci gives node_modules any real cross-run reuse on its own. This
# explicit restore/save pair (via GitHub's own cache service, not local disk) fills that gap: an
# exact manifest+lockfile match skips npm ci entirely. Keep fork/trusted keys separate even though
# both run on ubuntu-latest: fork PRs get read-only cache tokens, so a fork-keyed entry can never
# actually be written; trusted PRs keep their reusable cache without crossing trust boundaries.
- name: Restore node_modules cache
id: node-modules-cache
uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: |
node_modules
apps/loopover-ui/node_modules
# hashFiles('.nvmrc') matters as much as the package manifests and lockfile: a Node bump
# with no lockfile change would otherwise still hit and silently reuse node_modules whose
# native addons (sharp, workerd, fsevents) were compiled against the OLD Node's ABI. The
# manifests matter too because npm ci validates package.json/package-lock.json consistency
# and runs lifecycle scripts from package.json; a package.json-only change must not skip it.
key: npm-${{ (github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == true) && 'fork' || 'trusted' }}-${{ hashFiles('.nvmrc') }}-${{ hashFiles('package.json', 'apps/*/package.json', 'packages/*/package.json', 'package-lock.json') }}

- name: Install dependencies (retry on transient failures)
if: ${{ steps.node-modules-cache.outputs.cache-hit != 'true' }}
shell: bash
run: |
for attempt in 1 2 3; do
if npm ci --prefer-offline --no-audit --no-fund; then
exit 0
fi
echo "::warning::npm ci failed (attempt ${attempt}/3); retrying in 10s"
sleep 10
done
echo "::error::npm ci failed after 3 attempts"
exit 1

# Placed immediately after install (not as an automatic post-job hook) so a cache is only ever
# saved once npm ci has actually succeeded -- a job that fails here never reaches this step, so a
# broken/partial node_modules can never get written to the cache for a future run to inherit.
- name: Save node_modules cache
if: ${{ inputs.save-cache == 'true' && steps.node-modules-cache.outputs.cache-hit != 'true' }}
uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
with:
path: |
node_modules
apps/loopover-ui/node_modules
key: ${{ steps.node-modules-cache.outputs.cache-primary-key }}
56 changes: 56 additions & 0 deletions .github/workflows/cache-cleanup.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
name: Clean up closed-PR caches

# Every PR-triggered run in ci.yml (node_modules, Turborepo, tsbuildinfo) saves its cache scoped to
# that PR's refs/pull/N/merge ref -- GitHub's own cache isolation, separate from the key string itself
# (two different PRs can share an identical key but still get two separate physical cache entries,
# because they're scoped to different refs). Once a PR closes, nothing can ever restore a cache scoped
# to its ref again -- and this repo auto-closes a red contributor PR one-shot (no reopen, no retry on
# the same PR), so most of those entries are written once and then permanently unreachable, just
# waiting on GitHub's passive 7-day-unused eviction. Confirmed live before adding this workflow: repo
# cache usage sat at ~10.7GB of the 10GB budget, with closed-PR-scoped node_modules caches alone
# accounting for the large majority of that -- crowding out the much smaller, much more useful
# Turborepo/tsbuildinfo caches for LRU survival. This deletes a PR's own cache entries the moment it
# closes (merged or not) instead of waiting on eviction.
#
# pull_request_target, not pull_request: this job only ever calls the GitHub API using
# github.event.pull_request.number -- a trusted value GitHub itself populates, never anything read
# from the PR's own code or checked out from it -- so the classic pull_request_target risk (running
# fork-controlled code with base-repo credentials) doesn't apply here. It has to be
# pull_request_target specifically because a fork-triggered plain `pull_request` run is always capped
# to a read-only token regardless of the permissions block below, and deleting a cache needs
# actions: write.

on:
pull_request_target:
types: [closed]

permissions:
actions: write

concurrency:
group: cache-cleanup-${{ github.event.pull_request.number }}
cancel-in-progress: true

jobs:
cleanup:
name: Delete this PR's caches
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
# Scoped by ref, not by key prefix (e.g. "npm-fork-") -- ANY cache entry scoped to this PR's ref
# (node_modules, Turborepo, tsbuildinfo, trusted or fork) is equally unreachable dead weight the
# moment the PR closes, regardless of which of ci.yml's cache families wrote it.
- name: Delete caches scoped to this PR
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_REF: refs/pull/${{ github.event.pull_request.number }}/merge
run: |
ids=$(gh api "repos/${{ github.repository }}/actions/caches?per_page=100" --paginate --jq ".actions_caches[] | select(.ref == \"$PR_REF\") | .id")
if [ -z "$ids" ]; then
echo "No caches found for $PR_REF"
exit 0
fi
for id in $ids; do
echo "Deleting cache $id ($PR_REF)"
gh api -X DELETE "repos/${{ github.repository }}/actions/caches/$id" || echo "::warning::Failed to delete cache $id (may already be gone)"
done
55 changes: 55 additions & 0 deletions .github/workflows/ci-duration-report.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
name: CI duration report

# Nothing else in this repo tracks whether ci.yml is trending slower or flakier over time -- catching
# that currently requires a manual audit (this workflow exists because one just happened). Purely
# additive and read-only: it doesn't touch ci.yml, doesn't gate anything, and can't fail a PR. Mirrors
# audit.yml's shape (scheduled + workflow_dispatch, single job).

on:
schedule:
- cron: "0 15 * * 1" # Mondays 15:00 UTC, an hour ahead of audit.yml so they don't contend
workflow_dispatch:

permissions:
contents: read
actions: read

concurrency:
group: ci-duration-report
cancel-in-progress: true

jobs:
report:
name: report
runs-on: ubuntu-latest
timeout-minutes: 5
steps:
- name: Checkout
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
- name: Setup Node
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
with:
node-version-file: .nvmrc
- name: Pull ci.yml run stats
env:
GITHUB_TOKEN: ${{ github.token }}
run: node scripts/ci-duration-report.mjs --days=7 --output=ci-duration-report.json
- name: Write summary
run: |
node <<'NODE'
const fs = require("node:fs");
const report = JSON.parse(fs.readFileSync("ci-duration-report.json", "utf8"));
const fmt = (s) => (s === null ? "n/a" : `${Math.round(s / 60)}m ${Math.round(s % 60)}s`);
const pct = (r) => (r === null ? "n/a" : `${Math.round(r * 100)}%`);
const lines = [
`## CI duration report (trailing ${report.windowDays} days)`,
"",
"| Trigger | Runs | p50 | p95 | Failure rate | Cancelled (excluded) |",
"|---|---|---|---|---|---|",
`| push | ${report.push.count} | ${fmt(report.push.p50Seconds)} | ${fmt(report.push.p95Seconds)} | ${pct(report.push.failureRate)} | ${report.push.excludedCancelled} |`,
`| pull_request | ${report.pullRequest.count} | ${fmt(report.pullRequest.p50Seconds)} | ${fmt(report.pullRequest.p95Seconds)} | ${pct(report.pullRequest.failureRate)} | ${report.pullRequest.excludedCancelled} |`,
"",
"Duration is wall-clock (queue time included), not summed job time. \"Failure rate\" excludes cancelled runs (almost always a rapid re-push superseding its predecessor, not CI breaking) from both the count and the denominator.",
];
fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, `${lines.join("\n")}\n`);
NODE
Loading