From e301a82948c5058774fee21a951775721fb933a6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 9 Oct 2026 18:31:18 -0700 Subject: [PATCH 1/7] improvement(ci): halve the mobile job, narrow the desktop gate, skip proven checks on main, fix the top flake - e2e (mobile): Chromium and WebKit run as two processes against one app instead of back to back (each seeds its own fixtures and writes its own report), and every e2e group restores its own Turbopack dev cache. The job was the PR critical path at ~19.5 min: Chromium's pass (718 s median) carried the cold compile and WebKit waited it out (310 s). `suite setup` failures are now logged instead of only reaching the uploaded report. - dev-cache action: one mount shared by desktop-live and the e2e groups, keyed by app, event, fork, Next version and the hash of the file that sets the app's environment, so a cache is never read under NEXT_PUBLIC_* values it was not compiled with. http-e2e.sh stops apps with SIGINT so the cache write completes, drops a cache Turbopack reports as corrupt, and retries a startup it aborted once from an empty cache. - desktop-live-changes.sh: run the live desktop suite when a pull request touches a path it exercises instead of skipping only docs-like paths. Every genuine desktop-live failure since the layout change was on a PR the new rule still runs; pushes always run it. - ci.yml: a `proof` job lets a main push skip the checks when it is a merge whose tree is identical to its staging parent and a staging push run of that parent passed `checks / ci`; migrate gates on that proof instead. Any other shape or any error runs the full checks. - codeql, helm, desktop-e2e skip the staging -> main release PR (main's push and schedule still run them); helm never cancels a push mid-publish; trigger-promote on 2 vCPU; attestations via actions/attest with artifact-metadata: write. - stripe-sync-convergence: start each contender only after the parked transaction holds its locks, wait on a deadline rather than 200 polls, and release the transaction before failing, so a lost race no longer hangs the suite's teardown. It was the most frequent flake (9 red runs). - Integration shard weights refreshed from a recent run; /ship runs the affected workspaces' suites instead of the full suite locally. --- .agents/skills/ship/SKILL.md | 2 +- .github/actions/dev-cache/action.yml | 37 +++ .github/scripts/desktop-live-changes.sh | 32 +- .github/scripts/http-e2e.sh | 87 ++++- .github/scripts/staging-proof.sh | 38 +++ .github/workflows/checks.yml | 30 +- .github/workflows/ci.yml | 49 ++- .github/workflows/codeql.yml | 4 +- .github/workflows/desktop-e2e.yml | 6 +- .github/workflows/helm.yml | 12 +- .../stripe-sync-convergence.integration.ts | 18 +- apps/sim/scripts/test-mobile-e2e.ts | 4 +- scripts/desktop-live-changes.test.ts | 28 +- vitest.integration-durations.json | 310 ++++++++++-------- 14 files changed, 454 insertions(+), 203 deletions(-) create mode 100644 .github/actions/dev-cache/action.yml create mode 100755 .github/scripts/staging-proof.sh diff --git a/.agents/skills/ship/SKILL.md b/.agents/skills/ship/SKILL.md index fa6e5433ee5..c9b5b5475c7 100644 --- a/.agents/skills/ship/SKILL.md +++ b/.agents/skills/ship/SKILL.md @@ -42,7 +42,7 @@ When the user runs `/ship`: - If the diff modifies UI code (any non-test `.tsx` file, or anything under `apps/sim/components/`, `apps/sim/hooks/`, or `apps/sim/stores/`), run `/cleanup`. It fans out the React/UI passes (effects, memo, callbacks, state, React Query, emcn, url-state), the comment pass, and the test-audit pass, and applies fixes so they land in this commit. - Otherwise, if the diff adds or changes tests (`*.test.ts(x)`, `*.integration.ts`, `**/e2e/**`, `apps/sim/scripts/test-*-e2e.ts`), run `/test-audit audit ` on its own. Every new or changed test must pass the authoring gate; delete the ones that don't rather than shipping them. - Then run the test files the diff adds or changes, plus the existing tests beside changed source files, with `bun run --cwd test ` (`bun run --cwd apps/sim test ` for the app; `*.integration.ts` needs the setup in `.claude/rules/sim-testing.md`). A failing test aborts ship. - - Then run root `bun run test` from the repo root. It chains `test:scripts` (the `scripts/*.test.ts` suite CI runs) before every workspace suite; workspace-scoped runs skip it, which is how a `scripts/check-*.test.ts` failure has reached CI. A failing test aborts ship. + - Then run `bun run test:scripts` (the `scripts/*.test.ts` suite CI runs; workspace-scoped runs skip it, which is how a `scripts/check-*.test.ts` failure has reached CI) and the suites of the workspaces the change reaches: `bunx turbo run test --filter='...[origin/staging]'` (each changed workspace plus every workspace that depends on it). CI runs every suite, sharded, on every push; the full local suite needs more memory than a laptop reliably has and catches nothing CI does not. When the diff touches root test or build config (`vitest.shared.ts`, `turbo.json`, the root `package.json`, `bun.lock`), the filter selects nothing that config reaches, so run the full root `bun run test` instead. A failing test aborts ship. 5. **Run migration safety** — only if the diff touches `packages/db/migrations/**` or `packages/db/schema.ts`: - Run `/db-migrate` to review the migration for zero-downtime safety (expand/contract phasing, backward-compatibility with the deployed app version). - `(cd packages/db && bunx drizzle-kit generate && git status --porcelain ./migrations)` must print nothing (CI's schema/migration sync step). diff --git a/.github/actions/dev-cache/action.yml b/.github/actions/dev-cache/action.yml new file mode 100644 index 00000000000..bb36cda4ad5 --- /dev/null +++ b/.github/actions/dev-cache/action.yml @@ -0,0 +1,37 @@ +name: dev-cache +description: Mount Turbopack's dev cache at apps/sim/.next/dev for one `next dev` app, keyed by app, event, fork, installed Next version and the app's environment. + +inputs: + provider: + description: The CI_PROVIDER repo variable, forwarded to the cache action. + required: false + default: '' + name: + description: The app the cache belongs to. Each app runs `next dev` under its own fixed NEXT_PUBLIC_* values, so each gets its own cache. + required: true + env-hash: + description: Hash of the file that sets the app's environment. Next inlines NEXT_PUBLIC_* values at compile time, so a changed environment must never restore a cache compiled under the old one. + required: true + +# The cache turns a route's cold compile into a restore. It is content-addressed, so changed modules +# still recompile. The Next version in the key starts an upgrade from an empty cache, and the event +# and fork segments keep untrusted runs off the cache trusted runs read. Needs dependencies installed +# (it reads node_modules/next), and the app must be stopped with SIGINT so the last write completes. +runs: + using: composite + steps: + - name: Resolve Turbopack dev cache key + id: key + shell: bash + env: + APP: ${{ inputs.name }} + FORK_SUFFIX: ${{ github.event.pull_request.head.repo.fork && '-fork' || '' }} + ENV_HASH: ${{ inputs.env-hash }} + run: echo "value=${GITHUB_REPOSITORY}-next-dev-${APP}-${GITHUB_EVENT_NAME}${FORK_SUFFIX}-$(jq -r .version node_modules/next/package.json)-${ENV_HASH:0:12}" >> "$GITHUB_OUTPUT" + + - name: Mount Turbopack dev cache + uses: ./.github/actions/cache + with: + provider: ${{ inputs.provider }} + key: ${{ steps.key.outputs.value }} + path: ./apps/sim/.next/dev diff --git a/.github/scripts/desktop-live-changes.sh b/.github/scripts/desktop-live-changes.sh index d42cc2b2e70..102d52f6784 100755 --- a/.github/scripts/desktop-live-changes.sh +++ b/.github/scripts/desktop-live-changes.sh @@ -1,11 +1,30 @@ #!/usr/bin/env bash -# Prints `changed=false` only when every file a pull request changes is clearly unrelated to the -# live desktop suite, and `changed=true` otherwise, including when the diff cannot be worked out. +# Prints `changed=true` when a pull request touches a path the live desktop suite exercises, or when +# the diff cannot be worked out, and `changed=false` otherwise. +# +# The suite (apps/desktop/e2e/desktop-tools-live-sim.spec.ts) drives the Electron app against a +# local Sim, realtime and Redis through the desktop, mothership, copilot, upload and auth routes and +# the chat page. A change that only breaks Sim boot or an unrelated route fails the build and e2e +# jobs, and every staging and main push runs this suite regardless, so a path missing here can +# delay a failure to the staging push but never lets it deploy. # # Usage: desktop-live-changes.sh set -u -unrelated='^(apps/docs/|apps/pii/|apps/sim/content/|packages/(python-sdk|ts-sdk)/)|\.mdx?$|(^|/)LICENSE$' +relevant='^(apps/desktop/|apps/realtime/'\ +'|packages/(desktop-bridge|browser-protocol|terminal-protocol|realtime-protocol|db|auth|emcn|utils|logger|security|platform-authz|runtime-secrets)/'\ +'|apps/sim/lib/(desktop|mothership|uploads|auth|terminal|browser-agent|api/(client|server))/'\ +'|apps/sim/lib/api/contracts/(chats|copilot|desktop-|mothership-|upload-sessions|workspace-file)'\ +'|apps/sim/app/api/(desktop|mothership|copilot|files|v2/uploads|auth|users/me)/'\ +'|apps/sim/app/api/workspaces/\[id\]/files/'\ +'|apps/sim/app/workspace/\[workspaceId\]/(home/|components/|layout\.tsx)'\ +'|apps/sim/app/(layout\.tsx|desktop/)'\ +'|apps/sim/stores/(chat|chat-panel|panel|mothership-[a-z-]+|tool-permission|terminal|copilot-terminal|browser-session)/'\ +'|apps/sim/hooks/(use-mothership|use-desktop|use-chat|queries/(mothership|desktop|copilot|chats))'\ +'|apps/sim/types/sim-desktop'\ +'|apps/sim/(proxy\.ts|next\.config\.ts|package\.json|instrumentation)'\ +'|bun\.lock$|package\.json$'\ +'|\.github/(workflows/(checks|ci)\.yml|scripts/desktop-live-changes\.sh|actions/))' base=${1:-} run() { @@ -17,8 +36,7 @@ run() { git fetch --quiet --depth=1 origin "$base" || run "could not fetch $base" names=$(git diff --no-renames --name-only "$base" HEAD) || run "could not diff against $base" [ -n "$names" ] || run 'no changed files listed' -if printf '%s\n' "$names" | grep -qvE "$unrelated"; then - run 'a change may affect it' -fi +printf 'Changed files:\n%s\n' "$names" >&2 +match=$(printf '%s\n' "$names" | grep -E -m 1 "$relevant") && run "a change touches a path it exercises: $match" echo "changed=false" -echo 'Skipping the live desktop suite: every change is unrelated to it' >&2 +echo 'Skipping the live desktop suite: no change touches a path it exercises' >&2 diff --git a/.github/scripts/http-e2e.sh b/.github/scripts/http-e2e.sh index ef9c500bd1d..faf797f93e7 100755 --- a/.github/scripts/http-e2e.sh +++ b/.github/scripts/http-e2e.sh @@ -10,11 +10,13 @@ # the readiness deadline only has to catch a hung boot: an exited server fails immediately, and # either way the server log tail lands in the job log. # -# Each app starts from an empty Turbopack dev cache: a cache written under other NEXT_PUBLIC_* -# values, by a server that `next dev` SIGKILLs 100ms after SIGTERM, can panic Turbopack or wedge a -# route compile on restore. It runs in its own session under an E2E_APP tag, and stop-session.sh -# returns only once every process in that session or carrying that tag has exited (Next's -# telemetry flush runs detached and still writes .next/dev). +# Each group restores its own Turbopack dev cache (mounted by the dev-cache action, keyed by group +# and by this file's hash, so a cache is only ever read under the NEXT_PUBLIC_* values it was +# compiled with). The app is stopped with SIGINT so its last cache write completes: `next dev` +# SIGKILLs its server 100ms after SIGTERM. A cache Turbopack reports as corrupt is dropped, and a +# startup it aborted is retried once from an empty cache. The app runs in its own session under an +# E2E_APP tag, and stop-session.sh returns only once every process in that session or carrying +# that tag has exited (Next's telemetry flush runs detached and still writes .next/dev). set -euo pipefail group=${1:?usage: http-e2e.sh } @@ -27,11 +29,35 @@ app_tag='' server_log='' status_log='' +cache_broken() { + grep -qiE 'cache corruption|turbopack.*panic|panicked' "$server_log" 2>/dev/null +} + +# The cache directory is a mount point: empty it rather than remove it. +clear_cache() { + local cache="$GITHUB_WORKSPACE/apps/sim/.next/dev" + [ -d "$cache" ] || return 0 + find "$cache" -mindepth 1 -maxdepth 1 -exec rm -rf {} + +} + +# SIGINT is best-effort; the cleanup always runs, since workers and the detached telemetry flush +# can outlive a server that has already exited. +stop_app() { + if kill -INT "$server_pid" 2>/dev/null; then + for _ in $(seq 1 30); do kill -0 "$server_pid" 2>/dev/null || break; sleep 1; done + fi + bash "$GITHUB_WORKSPACE/.github/scripts/stop-session.sh" "$server_pid" "$app_tag" +} + finish() { local status=$? if [ -n "$server_pid" ]; then - bash "$GITHUB_WORKSPACE/.github/scripts/stop-session.sh" "$server_pid" "$app_tag" || status=1 + stop_app || status=1 wait "$server_pid" 2>/dev/null || true + if cache_broken; then + echo "::warning::Turbopack reported a broken dev cache; clearing it for the next run." + clear_cache + fi if [ -n "$status_log" ]; then awk '/^ (GET|POST|PUT|PATCH|DELETE|HEAD) \/api\// { print }' "$server_log" > "$status_log" fi @@ -43,6 +69,13 @@ finish() { } trap finish EXIT +# launch_app +launch_app() { + E2E_APP="$app_tag" setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 \ + --port "$1" > "$server_log" 2>&1 & + server_pid=$! +} + # start_app