Skip to content

fork: fast CI on GitHub-hosted runners (fork-tests.yml) - #6

Merged
scuttletoby merged 10 commits into
mainfrom
fork/ci
Nov 12, 2026
Merged

scuttletoby merged 10 commits into
mainfrom
fork/ci

Conversation

@scuttletoby

@scuttletoby scuttletoby commented Nov 12, 2026 •

Copy link
Copy Markdown
Owner

Why

Upstream's workflows can't run in this fork: they use runner labels and secrets the fork doesn't have. So fork pull requests get no lint, typecheck or tests at all. This PR adds the fork's own CI on GitHub-hosted runners, which are free on a public repository.

What

.github/workflows/fork-tests.yml runs upstream ci.yml's checks step for step, on pull_request and push: main. It also runs as the release test gate through workflow_call / workflow_dispatch with ref. The changes job resolves ref to one commit, and every job checks out that commit, so a branch that moves mid-run can't split a run over two commits.

Job What it runs
Changes upstream's change detector (.github/actions/ci-changes)
Workflow lint and upstream drift actionlint on the fork workflows, every fork script test, and the upstream drift check
Secret scan gitleaks over every commit the PR or push adds (base..head, merges included)
Lint PR-asset guard, knip, generated GraphQL checks, vp check
Build and Typecheck desktop build, preload check, desktop launch (Xvfb), built web boot, vpr typecheck
Test 1-2 every package except server and web, plus upstream's .github/scripts tests
Test Web 1-2 web unit and browser (Chromium) tests, a vitest slice each
Test Server 1-8 test:cached shards and the transfer-budget report
Rust cargo fmt --check and cargo test for the three crates
Mobile Native Static Analysis macOS lint (only for native mobile or workflow changes)
Release Smoke scripts/release-smoke.ts
fork-ci-result the one check to require

fork-ci-result is the required check

Require only fork-ci-result in a ruleset. It needs every other job and fails when any of them fails or is cancelled. It also fails when a job is skipped without a reason. The two valid reasons are:

  • changes said the job's gates aren't needed.
  • It is the secret scan in a release run, which tests a commit that was already scanned on its way to main.

The summary lists every job with its result, the reason for each skip, and each job's queue and run time. A new job is covered automatically once it is added to needs.

Every test file must run somewhere

.github/scripts/fork-ci-tests.cjs checks every *.test.* file under .github/scripts, subdirectories included, against .github/fork-ci-tests.txt. The list has two kinds of entry:

  • <test>: the workflows job runs it by extension: .sh with bash, .py with python3, .cjs/.mjs/.js with node --test. A listed test with any other extension fails. PyYAML is installed only when a fork .py test exists and import yaml fails.
  • <test> run by <file>: another job or script runs it. That file must name the test's path, and it can't be one of upstream's workflows, which are disabled here. Upstream's .github/scripts tests are listed this way. thread-transfer-report.test.cjs was run nowhere, because upstream runs it from a workflow_run of its disabled CI. Test 1 now runs it with upstream's other release-check tests.

The rule is the same on every event:

So a test file that ever reached main can't disappear without failing the run, and the open PRs' files don't block this one. Once #3 and #4 merge, every listed file is present and required. Removing a test on purpose means also removing its line from the list, which shows up in the diff. "Named by its runner" is checked by text: the check can't tell whether that line actually executes.

Secret scan

.github/scripts/fork-secret-scan.sh runs gitleaks on a full clone without file contents. gitleaks then fetches the contents of only the commits it scans.

  • Pull request: base.sha..head.sha, which is every commit the PR adds, including merges of main, whatever their dates. A squash merge never brings the PR's own commits to main, but they stay public under refs/pull/N/head, so they are scanned here.
  • Push to main: before..after. After a force push, that is every commit the new tip has and the old tip didn't. Commits both tips share were scanned when they were first pushed.
  • New branch, or an old tip that can't be fetched: only the pushed commit, with a warning in the log and the summary. There's no older tip to tell new commits from old ones, and everything before the pushed commit is history this fork already had.
  • Merge commits are diffed with --remerge-diff, which shows only what the merge itself changed beyond an automatic merge (a conflict resolution, or a file added in the merge). Plain git log -p shows a merge with no diff at all.

fork-secret-scan.test.sh builds these cases in local repositories. They include a key added and then deleted before a merge of main, and a key that only a merge commit adds. The Secret scan job runs it against the pinned gitleaks before it scans, and the workflows job runs it with a stand-in.

Upstream drift

.github/scripts/fork-tests-drift.cjs digests every job and step of upstream's ci.yml and the four test jobs of release.yml, plus both files' top-level env, defaults, concurrency and permissions. It compares them with the digest in .github/fork-tests-upstream.json. It fails with lines like ci.yml: job test_server: step "Restore Vitest module cache" added, and ignores changes that touch only comments. It caught the newer upstream on main when this branch merged main: test-delta tests, Vitest's module cache on the server shards, and web split in two. All three are ported. After porting a change, run node .github/scripts/fork-tests-drift.cjs --record. It refuses while a tracked job is missing upstream. Otherwise it saves the record and prints every upstream change the record now accepts.

Limit: the check compares upstream with its own record, not with fork-tests.yml. Recording without porting anything still passes. So a commit that changes the record must say, in its message and in the PR body, what was ported for each printed line or why that line doesn't apply. Reviewers check the record diff against that statement. The fix round 1 commit accepts the newly digested top-level keys this way. Upstream's ci.yml permissions and concurrency already have equivalents in fork-tests.yml. release.yml's permissions and concurrency apply to upstream's release run, not its test jobs.

Runner image: ubuntu-24.04, not ubuntu-latest

ubuntu-latest moved to 26.04 while this was being built. The pin means an OS upgrade happens in a PR that says so, not silently in the middle of a run. A Chromium crash in the web job was first seen on the 26.04 image.

Tuning

  • Test is split by .github/scripts/fork-test-shards.cjs. A package with at least 100 test files and a plain vp test run script gets a vitest --shard slice on every shard (desktop, mobile, client-runtime, shared). The rest are placed whole by test-file count, heaviest first onto the lightest shard. This is deterministic, so both shards compute the same split.
  • Test Web runs a vitest slice per runner. Before the split, one 4-CPU runner took 7.5 min for 548 files and crashed partway.
  • vp task cache. Upstream reads a private remote task cache, which needs a secret. Here vp's local task cache is kept with actions/cache for typecheck and the server shards. The key is vp-tasks-v1-<job>-<os>-<lockfile hash>-<sha>, restored by prefix. vp fingerprints each task's inputs, so a restored entry only replays a task whose inputs haven't changed.

Lessons carried over from another project's CI

Lesson Here
Cancel superseded runs concurrency per PR; main and release runs always finish
Caches keyed on the lockfile pnpm store (only Lint saves it), Vite+, Electron, Chromium, vp tasks, Vitest modules
Path filters that report what they skip upstream's detector, which fails open; every skip is named with its reason in fork-ci-result
Parallel jobs about 19 at peak; the server shards don't wait for changes
A timeout on every job 5–20 min
Pinned actions every action pinned to a full commit SHA; actionlint and gitleaks pinned by version and checksum
Least permissions contents: read; only fork-ci-result adds actions: read (job list); pull_request, never pull_request_target; no secrets
One required result check fork-ci-result
No silent retries none; a Chromium crash in Test Web fails the job instead of being retried away
Per-job timings in the fork-ci-result summary
Secret scan gitleaks over every added commit, history and merge commits included
Drift check vs upstream fork-tests-drift.cjs against ci.yml and release.yml

Coverage

Covered: every job of upstream ci.yml, plus release.yml's quality, test, test_web and test_server. Given a ref, every Linux job runs on that commit with no path filter, which is a superset of those four.

Not covered, and why:

  • windows-tests.yml: upstream runs it only by workflow_dispatch.
  • agent-context-budget: a label-gated budget comparison of a PR against main.
  • mobile-fingerprint-check, web-preview, mobile-eas: they need secrets or deploy somewhere.
  • pr-size, pr-vouch: repository management through pull_request_target.
  • thread-transfer-report: triggered by workflow_run.

Timings

Wall time from the first job starting to the last job finishing, on GitHub-hosted 4-CPU ubuntu-24.04 runners:

Run Wall time Slowest jobs
Before (one Test job, one Web job; v0.1.9) 7.5 min Test 438 s; Web 288 s, but it crashed and 53 of 548 files never ran
This PR on current main, cold server caches (43211320633) 6.4 min Server shards 233–375 s, Test 1 335 s, Web 1 330 s
This PR on current main, warm caches (43213388747, green) 6.7 min Test 1 384 s, Web 1 269 s, Test 2 227 s, Build 171 s; server shards 51–96 s

With warm caches, Test 1 sets the wall time: the desktop, mobile, client-runtime and shared slices plus the whole packages placed on shard 1. A third Test shard is the next step if this needs to be faster. Lint takes 93 s, Rust 54 s, Release Smoke 58 s, macOS lint 64 s, Secret scan 23 s and Workflows 12 s.

No test was flaky across the measurement runs. The one Chromium crash (connection closed) was on the 26.04 image, before the split.

Cost

All jobs run on GitHub-hosted runners. These are free for a public repository, including the one macOS job (macos-26, gated to native mobile or workflow changes). No paid or larger runners are used. Peak concurrency is about 19 jobs, against the free plan's limit of 20.

One fix to main in this PR

scripts: move release-repository.test.ts out of scripts/lib (two commits). #1 added that test under scripts/lib, which apps/desktop's typecheck includes, and it imports scripts/build-desktop-artifact.ts, which that project doesn't list (TS6307). Main's Build and Typecheck therefore fails. In scripts/ the test is covered by the scripts project and still runs in the scripts package.

Replaces #2's copy

#2 merged with its own fork-tests.yml. This PR replaces that file and keeps its workflow_call / workflow_dispatch input ref, so fork-release.yml calls it unchanged. fork-release.yml already grants the actions: read that fork-ci-result uses.

General vs project

Fully general: nothing here is specific to one deployment. Repository facts live in the workflow and its two data files (.github/fork-ci-tests.txt and .github/fork-tests-upstream.json).

🤖 Generated with Claude Code

Upstream's workflows are disabled in the fork, so fork pull requests ran
no checks. fork-tests.yml now runs upstream ci.yml's checks on
pull_request and push to main: lint (knip, generated GraphQL, vp check),
build + typecheck + desktop launch + built web boot, package tests, web
tests, 8 server shards, Rust, macOS mobile lint and release smoke.

It keeps the workflow_call / workflow_dispatch `ref` interface, so a
release run tests one commit with every Linux job and no path filter.

Upstream's composite actions and change detector are used as they are.
A new `drift` job fails, naming each job and step, when upstream's
ci.yml or release.yml test jobs change and the copy did not follow.
The Check job lists every job and the reason for every skip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- fork-ci-result needs every job and fails on a skip without a reason;
  its summary lists each job, why it skipped, and job wall times.
- Secret scan (gitleaks, checksum-pinned) of the commits a pull request
  or push adds; workflow lint (actionlint) next to the drift check.
- Ubuntu 24.04 runners as upstream: ubuntu-latest (26.04) fails an
  apps/server test with its newer git.
- Test is split across two runners by test file count; vp's task cache
  is kept between runs for the typecheck and the server shards.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
noreply and others added 3 commits November 12, 2026 11:24
- Test and Test Web run on two runners each. A package with 100+ test
  files (and Test Web's 548) runs a vitest --shard slice on every
  runner; the rest are placed whole by test file count.
- Script header comments fit upstream's 4-line comment rule; the jq
  program in fork-ci-result passes shellcheck.
- The Ubuntu 24.04 pin is about choosing OS upgrades, not git: the
  24.04 image now ships git 2.56 as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The workflows job runs each .github/scripts/fork-*.test.{sh,py,cjs} and
checks them against .github/fork-ci-tests.txt. A present test that is not
listed fails. A listed test that is missing fails once it has been in the
tested commit's history; before that it is a note in the job summary, so a
test whose pull request has not merged yet does not fail the run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The job list API needs it; a failed call now warns in the log instead of
failing silently.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
noreply and others added 3 commits November 12, 2026 11:41
main now carries #1, #2 and a newer upstream. fork-tests.yml keeps this
branch's version (it replaces #2's copy) and ports what the drift check
listed: test-delta.test.cjs in the release checks, Vitest's module cache
on the server shards (saved only by pushes), and no test-cache report
upload. Web was already split in two; the built-client boot stays in the
Build job.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
apps/desktop's typecheck includes scripts/lib, and this test imports
scripts/build-desktop-artifact.ts, which that project does not list
(TS6307). In scripts/ the scripts project covers both files, and the
scripts package still runs the test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@scuttletoby

Copy link
Copy Markdown
Owner Author

lens: correctness+tests (scripts) · reviewer: opus high (t3lens-6-scripts) · read at: f956059

covered: .github/scripts/fork-ci-tests.cjs, fork-test-shards.cjs, fork-tests-drift.cjs and their .test.cjs files, .github/fork-ci-tests.txt and .github/fork-tests-upstream.json, read line by line. Also traced how fork-tests.yml calls them (the workflows job with its sparse /.github/ checkout, the Test matrix's "Pick this shard's packages" and Test steps) and ci-changes.cjs readWorkspace/decide, which the shard planner depends on. Run 43213388747: job 131166404449 ran the unshallow path, because fork-follow.test.sh and fork-schema-check.test.cjs are listed but not present yet, and it passed.

checks:

  • The tests pass at head with node 24: fork-ci-tests 5/5, fork-test-shards 4/4, fork-tests-drift 3/3. Drift against the recorded JSON: exit 0.
  • fork-ci-tests guard, broken by hand in a scratch clone: an unlisted failing fork-new.test.sh gives exit 1 with two ::error lines (failed + not listed). A listed test that fails gives exit 1. A listed test deleted (both uncommitted and committed) gives exit 1 "deleted while listed". A trailing space in the list is ignored (exit 0, correct). A failing fork-new.test.mjs, fork-new.test.js or .github/scripts/fork/fork-sub.test.sh gives exit 0 (see the minor finding).
  • Shallow/merge-commit state: I simulated the PR run in a local repo. Main has the file. The PR branch deletes it and keeps the list entry. Main moves on, there is a --no-ff merge, and a --depth 1 clone is checked out at the merge commit. The script unshallows (afterwards is-shallow is false), finds the deletion through the second parent and exits 1. With origin unreachable the fetch throws: exit 1 (fails closed). A test added and deleted inside one PR without reaching main stays "pending" forever. Harmless: it was never on main.
  • Shards: a randomized property check of assign() with 20,000 runs (0-30 packages, 1-5 shards, random weights and splittable flags, input order reversed). It found 0 violations. Every non-split package is on exactly one shard, every split package is on all shards and in no whole list, and the result does not depend on input order. Every shard computes the plan from the same commit (git ls-files counts and the manifests), so the shards agree. An empty shard gives any=false and skips. TEST_PACKAGES naming a package the planner does not know throws (fails closed). That would happen if ci-changes OWN_TEST_JOB and the planner's OWN_JOB ever drift apart. The planner's set (hasTest, minus t3/web/monorepo) equals upstream's vp run --filter '!t3' --filter '!@t3tools/monorepo' --filter '!@t3tools/web' test. A split package gets one vitest --shard=i/N on every shard, so vitest covers each of its files exactly once. Real plan at head: the 4 split packages plus 19 and 19 whole packages.
  • Drift, fail-closed: release.yml missing gives ENOENT, exit 1. A blank record gives a JSON SyntaxError, exit 1. An empty record {} lists every job as "added", exit 1. A renamed tracked release job (test_web) gives "removed upstream", exit 1. A job key with a trailing comment is absorbed into the previous job and reported as changed, exit 1 (loud, not silent).
  • State question (rerun / half-applied / old / empty): all three scripts are stateless and read only the checked-out commit (plus the trees-only history fetch), so re-running one job gives the same answer. No caches are involved. An old record after an upstream follow is red until someone runs --record. Half-applied: running --record without porting anything is green. The check compares upstream with its own record, never with fork-tests.yml, so "fork-tests.yml did not follow" really means "the record was not refreshed". The review of the JSON diff is the only guard there. That is acceptable, but the script header overstates it. First run on a new fork: the history is inherited from upstream, entries that were never present are notes, and the unshallow works on a depth-1 checkout.
  • Not tested by the .test.cjs files (I checked these by hand above): everInHistory and the shallow fetch, the TEST_FILE pattern, main()'s TEST_PACKAGES guard and the splittable regex, and drift main()/--record.

finding: minor .github/scripts/fork-tests-drift.cjs:29 Only jobs: is digested. A new top-level env: (before or after jobs) or defaults: run: shell: in upstream ci.yml gives exit 0, no drift. These change how every upstream job runs, so a test-relevant upstream change can pass silently. Fix: also digest the top-level keys other than on/concurrency (env, defaults, permissions) as a pseudo-job. (high)
finding: minor .github/scripts/fork-tests-drift.cjs:96 With a tracked release job renamed, --record warns "tracked jobs missing" but still writes "test_web": null and exits 0. Every later check then crashes at :120 with TypeError "Cannot read properties of null (reading 'header')". It is red, but with no usable message. Fix: make --record exit 1 when a tracked job is missing, and have compare() treat null/null as "missing upstream". (high)
finding: minor .github/scripts/fork-ci-tests.cjs:21 TEST_FILE only matches top-level fork-.test.{sh,py,cjs}. A failing .github/scripts/fork-x.test.mjs or .test.js, or one in a subdirectory, is neither run nor flagged as unlisted (exit 0). The PR description says every fork-.test.* is run. Fix: match fork-.test. and fail on any unknown extension (or any subdirectory hit). (high)

@scuttletoby

Copy link
Copy Markdown
Owner Author

lens: security+correctness (workflow) · reviewer: opus high (t3lens-6-wf) · read at: f956059

covered: .github/workflows/fork-tests.yml line by line (all 12 jobs, triggers, concurrency, permissions, caches, the fork-ci-result gate) and scripts/release-repository.test.ts (the move). Traced into fork-test-shards.cjs, fork-ci-tests.cjs, fork-tests-drift.cjs, ci-changes.cjs, setup-ci-deps, scripts/test-cached.ts, fork-release.yml (the workflow_call caller) and upstream ci.yml/release.yml for the job-by-job comparison. Read the logs of run 43213388747 (Secret scan, Test 1).

checks:

  • Tests at head: node --test of the 3 fork script tests, 12/12 pass; drift check green on the recorded digest.
  • Guards broken by hand, all red as they should be: fork-ci-result jq with build skipped while desktop_launch=true (exit 1), secrets skipped on a PR run (1), workflows skipped (1), changes failed and a job skipped (1), a server shard cancelled (1); allowed skips pass (rust+test with gates false, secrets on a release run: 0). An unlisted .github/scripts/fork-zz.test.cjs fails, and so does deleting a listed fork-test-shards.test.cjs. Drift: upstream rust job runs-on/step changed (1), a new upstream job added (1), release.yml unparsable (1, fails closed), ci.yml missing (1, fails closed).
  • Shards: printed 1/2 and 2/2 at head. The 4 split packages (desktop, mobile, client-runtime, shared) get --shard on both shards; every whole package lands on exactly one shard. All split scripts are a single vp test run ... command, so the appended --shard reaches vitest. TEST_PACKAGES is the same on both shards, so the split is deterministic. Every test file outside .repos is in a package with a test script, or is a .github/scripts test (see the minor below); kde-snap-shot is run through desktop's include.
  • release-repository.test.ts: fork-only file (from fork: the server and CLI update from the desktop app's repository #1), not an upstream file, so the move creates no rebase conflict. The imports are right for scripts/ and it ran in Test 1 (6 tests) through @t3tools/scripts. apps/desktop's tsconfig includes only ../../scripts/lib, so the TS6307 cause is gone.
  • Untrusted input: run: lines interpolate only matrix.shard, strategy.job-total and runner.temp. PR, branch, input and file values go through env (EVENT_NAME/COMMITS/BEFORE/TEST_PACKAGES/WHOLE/SPLIT/SHARD). inputs.ref is used only in checkout with:. No pull_request_target. eval of the package.json test script runs PR code, which the PR runs anyway.
  • Permissions: top-level contents: read. fork-ci-result replaces it with actions: read only (no checkout there; download-artifact and the jobs API need it). The caller fork-release grants actions+contents read, which caps workflow_call. No self-hosted runner.
  • State question (rerun / half-way / old / empty state):
    • Empty cache, or first run on a new fork: this very run had no main-scoped caches (all 66 entries are refs/pull/6 and /7) and was green. test-cached --verify-hits copes with an empty task cache.
    • Cache from an old lockfile: the vp-tasks and vitest restore-keys include the lockfile hash, so an old lockfile never matches.
    • PR cache poisoning: Vitest modules are saved only on push. vp-tasks, Electron and Chromium use actions/cache and do save on PR runs, but under refs/pull/N/merge, which only that PR's own runs can read. A PR can't write anything main or another PR restores. A PR restoring main's vp task cache replays only tasks with unchanged fingerprints, the same trust model as upstream's remote cache.
    • A failed attempt saves nothing (actions/cache saves only on success), so a rerun of a flaky shard doesn't restore a half-written cache. Rerunning only fork-ci-result reuses the earlier attempts' needs results and artifacts.
    • A branch where a listed test was deleted: red (shown above). A listed test that has never existed in history is a note, as documented.
    • Release runs get a SHA from fork-release preflight (rev-parse HEAD), so every job tests the same commit.
  • Concurrency: in a reusable workflow github.event_name is the caller's event (workflow_dispatch), not workflow_call, so the comment at :50 is inaccurate. It does no harm: the caller's group is fork-release, and the only overlap is a manual fork-tests dispatch of the same SHA, which would queue behind the release run.

finding: major .github/workflows/fork-tests.yml:199 (with :175) — the secret scan uses --max-count=$COMMITS from the PR head. Once a PR has merged main, git log walks both parents by date, so main's newer commits fill the count and the PR's oldest commits are never scanned. Shown on this very PR: commits=8, and the job's range lists f956059, 993ccc8, 6edd250 (merge), 831e888, ee69d98 (main, #2), 1cfd0a7 (main, #1), 95935ce, 9510d2f. The PR's own 918e22b and 65243f1 are skipped, and the log says "6 commits scanned". Reproduced with gitleaks 8.30.1 on a synthetic repo: PR commit A adds an AWS key, B deletes it, then merge main (3 newer commits), then C. The workflow's range gives "no leaks found", exit 0. The range base..head on the same clone gives "leaks found: 1", exit 1. A squash-merged PR's intermediate commits never reach main, so the push scan doesn't catch it either, yet they stay public under refs/pull/N/head. Fix: fetch github.event.pull_request.base.sha (or deepen until the merge-base) and scan base.sha..head.sha, and add a test with a merge commit. (high confidence)
finding: minor .github/workflows/fork-tests.yml:486 — .github/scripts/thread-transfer-report.test.cjs is run nowhere on the fork. Upstream runs it only in thread-transfer-report.yml, which triggers on workflow_run of "CI", and CI is disabled here. No guard covers non-fork .github/scripts/.test.: fork-ci-tests.cjs only checks fork-.test., and the drift check only watches ci.yml/release.yml. So this test file is present and silently not run, against the requirement that "a test file that exists but isn't run fails CI". Either add it to "Test release checks" (with a reason listed), or extend fork-ci-tests.cjs to fail on any .github/scripts/.test. that no fork job names. (high confidence)
finding: minor .github/workflows/fork-tests.yml:39-44 (and every ref: ${{ inputs.ref }} checkout) — a direct workflow_dispatch with a branch name, which the input allows ("Commit or branch"), resolves the branch separately in each job, and jobs start minutes apart (test/build wait for changes, the shards don't). So one run can test two commits if the branch moves mid-run, while fork-ci-result reports a single green. Releases are unaffected (fork-release passes a SHA). Resolve the ref to a SHA once in changes and hand it to the other jobs as an output, or accept only a SHA. (medium confidence)

…t per run, drift)

Secret scan (.github/scripts/fork-secret-scan.sh, tests: fork-secret-scan.test.sh)
- Pull requests scan base.sha..head.sha on a full blob-less clone instead of the
  newest N commits by date. After a merge of main, the PR's oldest commits fell
  outside --max-count and were never scanned; a squash merge never brings them to
  main, yet they stay public under refs/pull/N/head.
- Merge commits are diffed with --remerge-diff. Plain `git log -p` shows merges
  with no diff, so a key added only in a merge commit was never read.
- Push: before..after; a force push scans what the new tip has and the old one
  had not. A new branch, or an old tip that cannot be fetched, scans only the
  pushed commit and says so in a warning.
- The Secret scan job runs the test against the pinned gitleaks before scanning.

Every test file under .github/scripts runs somewhere (fork-ci-tests.cjs)
- Finds *.test.* in subdirectories too. Runs .sh, .py, .cjs, .mjs and .js; a
  listed test with another extension fails instead of being skipped.
- Upstream's tests are listed as "<test> run by <file>"; the file must name the
  test and cannot be a disabled upstream workflow. thread-transfer-report.test.cjs
  was run nowhere (upstream runs it from a workflow_run of its disabled CI); Test 1
  runs it now with upstream's other release-check tests.

One commit per release run
- `changes` resolves a workflow_dispatch/workflow_call ref to a SHA once; every
  job checks that out, so a branch that moves mid-run cannot split a run over two
  commits. workflows, lint and the server shards now wait for `changes`.

Upstream drift (fork-tests-drift.cjs)
- Also digests the top-level env, defaults, concurrency and permissions.
- --record refuses (exit 1) while a tracked job is missing instead of saving
  null, and an old null entry is reported instead of crashing.
- --record prints the upstream changes it accepts; the commit and PR must say
  what was ported for each.

Re-recorded .github/fork-tests-upstream.json. It accepts:
- ci.yml top-level permissions (contents: read): fork-tests.yml has the same.
- ci.yml top-level concurrency (group per PR/sha, cancel on pull_request):
  fork-tests.yml has the equivalent, with the event name in the group.
- release.yml top-level concurrency and permissions: they apply to upstream's
  release run, not its test jobs. fork-release.yml has its own.
No job or step changed upstream.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@scuttletoby

Copy link
Copy Markdown
Owner Author

fix round 1 · head 6d6e0b5 · fixes f956059..6d6e0b5 (one commit) · lens comments 6823256526 (workflow) and 6823238680 (scripts)

CI: not run on the new head. fork-tests.yml (workflow 415816119) was switched to disabled_manually at 12:14:50Z, ten minutes before this push, so the pull_request event started no run. This round didn't disable it, and re-enabling it is a repository setting, which is the owner's call. What fits: something ran a workflow sweep that didn't list fork-tests.yml among the fork's own workflows (#4's fork-workflows.sh disables every workflow that isn't in OWN_WORKFLOWS). That's unconfirmed. Once it's re-enabled, a re-run or empty push gives the CI result.

Each fix was checked red on f956059's code and green on the fix, locally (node 24, git 2.43, gitleaks 8.30.1 and actionlint 1.7.12 pinned by checksum as in the workflow):

Finding Fix Red on f956059 → green
major :199 secret scan misses the PR's oldest commits after a merge of main .github/scripts/fork-secret-scan.sh: PR scans base.sha..head.sha on a full blob-less clone; push scans before..after (force push: what the new tip has and the old one had not); a new branch or unfetchable old tip scans only the pushed commit, with a warning. Also found: gitleaks never reads merge commits (git log -p shows them with no diff), so a key added only in a merge was missed. Merges now go through --remerge-diff. fork-secret-scan.test.sh with real gitleaks: the f956059 step logic fails 8 of 11 checks, including the add-then-delete-before-merge key (exit 0) and the merge-only key. The fix passes 11 of 11. On this PR itself the old range scanned 6 commits; the new one covers all 8 (ee69d98b..f9560594, the clean merge has no diff). The Secret scan job now runs the test against the pinned gitleaks before it scans.
minor :486 thread-transfer-report.test.cjs runs nowhere Test 1 runs it with upstream's other release-check tests. fork-ci-tests.cjs now covers every *.test.* under .github/scripts (subdirectories too). Upstream tests are listed as <test> run by <file>; the file must name the test and can't be a disabled upstream workflow. New guard, without the workflow line: exit 1 "not run: fork-tests.yml does not name …". With it: 0.
minor :39-44 dispatch with a branch name can test two commits changes resolves ref to a SHA once (outputs.sha); every job checks that out. workflows, lint and the server shards now needs: changes. Not reproduced red: it needs a branch that moves mid-run. actionlint is green. A release-mode run hasn't been made.
minor drift :29 only jobs: digested Top-level env, defaults, concurrency, permissions of both tracked files are digested too. Upstream ci.yml with an added top-level env:: old exit 0, new exit 1. New unit tests cover add/change/remove, keys after jobs:, and on: ignored.
minor drift :96 --record saves null for a missing tracked job --record exits 1 and writes nothing. An old null entry is reported ("missing upstream") instead of crashing. test_web renamed: old --record exit 0, then TypeError at :120. New exit 1 with the message, and the check reports "removed upstream".
minor fork-ci-tests.cjs:21 only top-level .sh/.py/.cjs .mjs/.js run with node --test; any other extension of a listed test fails; subdirectories are searched. A failing unlisted fork-x.test.mjs and sub/fork-y.test.sh: old exit 0, new exit 1. A listed .ts: exit 1 "no runner".
design limit: re-recording without porting passes --record prints every upstream change it accepts. The commit and PR body must say what was ported for each line. That's stated in the script header and the PR body ("Limit"). Not enforced by a check (on purpose, not over-built).

The record is re-recorded. It accepts only the four newly digested top-level keys; no upstream job or step changed. ci.yml permissions/concurrency already have equivalents in fork-tests.yml. release.yml's apply to upstream's release run, not its test jobs. The PR body is updated (test-file rule, secret scan ranges, drift limit, one commit per release run).

Unproven: CI on 6d6e0b5 (workflow disabled). The Secret scan on a real GitHub PR event (lazy blob fetch, base.sha fetch), a release-mode run with a branch ref, and whether --remerge-diff is slow on a large merge of an upstream release (blobs fetched on demand).

@scuttletoby scuttletoby reopened this Nov 12, 2026
@scuttletoby

Copy link
Copy Markdown
Owner Author

lens: quick check r2 · reviewer: opus high (t3lens-6-r2) · read at: 6d6e0b5

covered: the whole fix diff f956059..6d6e0b5 (9 files): fork-secret-scan.sh and its test, the fork-tests.yml changes (secrets job, resolved sha for release runs, needs: changes, thread-transfer-report in Test release checks), fork-ci-tests.cjs/.txt and test, fork-tests-drift.cjs/.json and test. Ran every script test at the head with the stand-in and with the pinned gitleaks 8.30.1 (checksum checked): all pass, drift check passes. Put the f956059 inline scan logic into a script and ran fork-secret-scan.test.sh against it with real gitleaks: 8 of 11 red, including "key added then deleted before a merge of main". Broke the fix by hand: no --remerge-diff -> both merge-commit cases red; PR range back to --max-count= HEAD -> 2 red; push range newest-only -> 2 red. Read CI run 43218151901.

checks:

  • Round-1 MAJOR fixed at the root: PR range is BASE_SHA..HEAD_SHA with fetch-depth 0 + blob:none, so every PR commit is in range whatever its date. A main merge in the PR: main's commits are excluded, the PR's older ones stay (tested). Base moved: base.sha is the event's base tip, and the PR's own commits are still not reachable from it. Force-pushed PR: the new head's range is scanned again in full. 1-commit PR: fine (checked by hand). Hundreds of commits: no depth limit and no API listing, so no paging. Merge commits: --remerge-diff shows conflict resolutions and files added in the merge (tested with real gitleaks). The CI run shows "9 commit(s) ... 8 commits scanned": the 9th is the pure rename 993ccc8, which has no diff to scan. That's expected.
  • The scan stops with an error when BASE/HEAD cannot be fetched or is empty (exit 2, checked by hand). But no test covers it: if I make the fetch failure continue, every test still passes. It still fails, with exit 128 from rev-list under set -e, so it is only a missing test.
  • Minors: thread-transfer-report now runs in Test release checks, and every .github/scripts test (subdirectories too) must be listed with a runner, so the 'only top-level fork-*' gap is closed. Release runs resolve the ref once in changes, and every job checks out needs.changes.outputs.sha (empty on PR/push, so they keep the event commit). Drift now digests top-level env/defaults/concurrency/permissions. --record refuses while a tracked job is missing, and compare() reads an old record's null entries without a TypeError. All of these are tested.
  • No pull_request_target. Untrusted input reaches run: only through env (REF, BASE_SHA, HEAD_SHA, BEFORE). The ${{ }} left in run: lines are matrix/strategy/runner.temp only.
  • Push path: a new branch or an old tip that cannot be fetched scans only the newest commit, with a warning. That is fail-open, but push runs only on main, so it is acceptable. An upstream sync PR puts every upstream commit in range: slower, and a timeout fails closed. Same as before the fix.
  • State question (rerun / half-way / old or empty state): the scan keeps no state. A rerun uses the same event payload, so it gets the same range. An empty history (a new branch) scans the newest commit with a warning. drift --record handles a missing record file ({}) and old records with null entries. It refuses before writing when a job is missing. Half-way stop: the problem is the scan stopping half-way, see the finding below. gitleaks fetches blobs on demand, and if that stops half-way it still passes.

finding: major .github/scripts/fork-secret-scan.sh:79 the checkout is blob:none, so gitleaks makes git fetch file contents on demand. When that fetch fails (network error or GitHub 5xx mid-scan), gitleaks 8.30.1 logs "ERR [git] fatal: could not fetch ... from promisor remote", reports "0 commits scanned", and exits 0. The step summary then says "No secrets found". I reproduced it: blob-less clone, origin made unreachable after the clone, a PR range with a key -> exit 0. In the same state git log -p --remerge-diff "$range" >/dev/null exits 128. Fix: fetch the contents first under set -e (that git log -p, or a full clone for this job) so gitleaks runs offline, or compare gitleaks' scanned count with rev-list's count. Add a test with an unreachable origin after the clone. (high)
finding: major (CI) .github/scripts/fork-ci-tests.cjs, fork-ci-tests.test.cjs, fork-tests-drift.test.cjs: in run 43218151901, Lint fails at vp check: "Formatting issues found" in these 3 files from the fix commit. fork-ci-result cannot go green until vp check --fix is run. (high)

CI 43218151901 at read time: in progress. Lint failed (formatting, above). Secret scan, Changes, Workflow lint and upstream drift, Build and Typecheck, Test 2, Test Server 1-8, Rust, Release Smoke and Mobile Native passed. Test 1, Test Web 1 and Test Web 2 were still running.

Secret scan (.github/scripts/fork-secret-scan.sh, tests: fork-secret-scan.test.sh)
- The checkout is blob-less, so gitleaks fetched file contents on demand. gitleaks
  8.30.1 stops at the first git error (a failed content fetch), logs "0 commits
  scanned" and exits 0, and the summary said "No secrets found". The script now
  runs gitleaks' own `git log -p -U0 <opts>` first: it fetches every blob the scan
  reads and fails the job (exit 2) when a fetch fails. gitleaks then runs with
  GIT_NO_LAZY_FETCH=1.
- The job also fails when gitleaks scanned fewer commits than that git log has
  commits with a hunk in a file they do not delete (what gitleaks counts; a pure
  rename or a clean merge has none, so it can be below rev-list --count).
- New tests: contents unreachable after the clone, gitleaks stopping early, and a
  base or head commit that cannot be fetched.

Lint (vp check)
- Formatting of fork-ci-tests.cjs, fork-ci-tests.test.cjs, fork-tests-drift.test.cjs.
- The header comments of fork-ci-tests.cjs and fork-tests-drift.cjs cut to 4 lines
  (t3code/no-low-value-comments).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@scuttletoby

Copy link
Copy Markdown
Owner Author

fix round 2 · head 675886a · fix range 6d6e0b5..675886a (1 commit)

MAJOR: secret scan passed when it could not read the files. Fixed in .github/scripts/fork-secret-scan.sh:

  • Before gitleaks runs, the script runs gitleaks' own command (git log -p -U0 <log-opts>). That fetches every blob the scan reads, and under pipefail a failed fetch fails the job (exit 2, "cannot read the commits", noted in the step summary).
  • gitleaks then runs with GIT_NO_LAZY_FETCH=1. The runner has git 2.56, so it can't fetch on demand.
  • The job fails (exit 2) when gitleaks reports fewer commits scanned than that git log holds, or reports no count. The expected count uses gitleaks 8.30.1's own rule, read from its source: a commit counts when a file it doesn't delete and that isn't binary has a hunk. A plain rev-list count would always fail: pure renames and clean merges have no hunks (the 9-vs-8 the lens saw). On this PR's range the counts match with real gitleaks: locally 8 = 8 at 6d6e0b5, and in CI 9 = 9 at the new head.
  • New tests in fork-secret-scan.test.sh: a fresh blob-less clone whose origin is gone after the clone; gitleaks stopping early (only the newest commit scanned, exit 0); a base commit that can't be fetched; a head commit that can't be fetched. The stand-in gitleaks now behaves like 8.30.1 on a git error: it logs "0 commits scanned" and exits 0.
  • Red/green, all with real gitleaks 8.30.1 and with the stand-in: the 6d6e0b5 script fails 2 of 15 (the unreachable-origin case and the early stop, both exit 0). The fix passes 15/15. Mutations on the fix: if a base/head fetch failure continues, both fetch tests go red (exit 128 ≠ 2). With no count check, the early-stop test goes red. With no prefetch (and lazy fetch allowed), the early-stop and unreachable-origin tests go red.

MAJOR: Lint. vp check reproduced the formatting failure on 6d6e0b5. vp fmt fixed the 3 files. Behind it, vp check also had 2 errors that CI hadn't reached yet: t3code/no-low-value-comments on the header comments of fork-ci-tests.cjs and fork-tests-drift.cjs (over 4 lines). Both are now 4 lines. Local vp check: 0 errors; the 7 warnings are in upstream files. The workflow comment on the secrets checkout was updated to match.

CI: run 43220629206 (pull_request, 675886a) passed: all 21 jobs, Lint, Secret scan and fork-ci-result included. Its scan-range test ran 15/15 with the pinned gitleaks.

Unproven:

  • No real mid-scan network failure on GitHub. That case is covered by the local unreachable-origin test and the early-stop test.
  • GIT_NO_LAZY_FETCH isn't exercised locally (local git 2.43 ignores it). The count check covers the same failure either way.
  • gitleaks stops at any git stderr line, warnings included (for example a rename-limit warning on a huge upstream sync). That case now fails the job instead of passing it silently. It fails closed, but it could cause a red on a very large range.
  • A commit that only adds an archive binary counts for gitleaks but not in the expected count. That can only push gitleaks' count higher, so only fewer-than-expected fails.

@scuttletoby

Copy link
Copy Markdown
Owner Author

lens: quick check r3 · reviewer: opus high (t3lens-6-r3) · read at: 675886a

covered: the fix diff 6d6e0b5..675886a in full (fork-secret-scan.sh and its test, fork-tests.yml checkout comment, formatting-only changes in fork-ci-tests.cjs / fork-tests-drift.cjs and their tests), plus what it calls into: gitleaks v8.30.1 source (detect/detect.go DetectSource and addCommit, sources/git.go NewGitLogCmd, listenForStdErr and Fragments, sources/file.go archive handling, cmd/root.go exit codes). Local runs used gitleaks 8.30.1 checked against the workflow's SHA256 (OK), with git 2.43 locally; CI runs git 2.56.0. gh was used read-only, with GH_TOKEN and GITHUB_TOKEN unset.

checks:

  • Tests at both commits. fork-secret-scan.test.sh is red on 6d6e0b5 (2 FAIL: "gitleaks scanning fewer commits than the range has fails" and "contents that cannot be fetched after the clone fail", both exit 0 where 2 was expected) and green on 675886a (15/15). Same result with the stand-in and with the real gitleaks. The node tests pass at both commits (8/8 and 6/6); those changes are formatting only.
  • Breaking each fix by hand: dropping the -lt "$expected" check, or making awk print 0, turns the early-stop test red. Ignoring the pre-fetch failure (|| true) turns the offline-clone test red; it also shows the pre-fetch guard is needed, because the truncated git log then gives expected=0 and the count check alone would pass. A changed count format (sed finds no number) makes 4 passing cases exit 2 and the early-stop test red, so a missing count fails. Removing GIT_NO_LAZY_FETCH=1 turns no test red (see the note below).
  • Is the expected count right? It matches gitleaks 8.30.1's own rule: addCommit runs for each fragment, a fragment is a TextFragment of a file that is neither deleted nor binary, and archives yield nothing at the default max-archive-depth 0. Path allowlists are applied after addCommit, so they do not lower gitleaks' count. Measured against the real binary:
    • Edge-case repo with 16 commits: pure rename, mode change, empty new file, empty commit, random binary, delete-only, no-newline file, content lines that look like "commit"/"@@", a 3,000-file commit, 3,000 exact renames, a clean merge, a resolved add/add conflict merge. expected 8 = gitleaks 8.
    • The t3code range HEAD~400..HEAD (one merge): expected 871 = gitleaks 871, in 13 s.
    • Rename limit forced on (diff.renameLimit=1, inexact renames): gitleaks prints the warning as WRN, which is on its allow list; 10 = 10 and the job passes, with every commit read.
    • Empty range (BEFORE=AFTER) and a range of only empty commits: 0 = 0 and the job passes, which is correct because there is nothing to read.
  • "Fewer only, not differ" is sound for the pinned 8.30.1, because the two rules are equal. Strict equality would also catch a future gitleaks bump that changes the counting rule. No test pins the count against the real binary on rename, mode, empty or binary commits, so a bump would only show up if the counts then differed.
  • A merge with a binary conflict: remerge-diff writes "remerge warning: Cannot merge binary files" into the diff header, not to stderr, so it does not trigger the gap below.
  • gitleaks is pinned (8.30.1, sha256sum -c before tar). No ${{ }} appears in any run: line of fork-tests.yml (event values only reach env:). No pull_request_target. permissions: contents: read.
  • CI: run 43220629206 is completed / success, 21/21 jobs, headSha 675886a. The Secret scan log shows the 15 checks passing and "gitleaks scanned 9 commit(s); 9 commit(s) in range have changes it reads".
  • State (rerun, half-way stop, existing/old/empty state):
    • Rerun: the script keeps no state. Each run makes a fresh blob-less checkout and computes the same range and the same counts again. The mktemp log is removed by the EXIT trap.
    • Stop half-way: a blob fetch that fails part-way makes git log exit non-zero, pipefail ends the pre-fetch, and the job exits 2 with "cannot read the commits" (tested). A gitleaks crash exits non-zero. A gitleaks run that stops early with exit 0 is caught by the count, except in the case in the finding below.
    • Old or empty state: an empty range or empty commits give 0 = 0 and pass. A new branch scans only the tip, with a warning. An old tip that cannot be fetched also warns and scans only the tip. Missing count output fails.

finding: minor .github/scripts/fork-secret-scan.sh:113 <git writes any stderr line that is not on gitleaks' allow list (rename-limit and auto-gc are allowed) during the gitleaks run, after a pre-fetch that exited 0 -> gitleaks logs ERR [git] ... and ERR error="stderr is not empty", stops taking diff files once git exits (Fragments returns on errCh without wg.Wait), and still exits 0. The count check is per commit: when the files it drops all belong to the oldest commit with hunks, that commit has already been counted, so scanned == expected and the job reports "No secrets found". Reproduced with a git wrapper that prints one warning line: base + one commit adding 3,000 files with an AWS-style key in zzz.txt gives exit 1 normally, but exit 0 and "gitleaks scanned 1 commit(s); 1 commit(s) in range" in 3 of 3 runs with the stray line. In the 18-commit repo the same line was caught (7 of 10, exit 2). I found no real trigger: remerge conflicts go into the diff, missing objects end git with fatal, and the pre-fetch already fetches every blob, so this is a hole in the backstop, not a hole you can reach today. Fix suggestion: also fail when the gitleaks log contains stderr is not empty (or any ERR line) while status is 0, and add the stray-stderr wrapper as a test case. (confidence: high that the mechanism is real, low that it can happen)

Note, not a finding: GIT_NO_LAZY_FETCH=1 has no test and is ignored by git < 2.44 (CI has 2.56.0, so it works there). It is defense in depth behind the pre-fetch.

Round-2 majors: (1) fixed at the root (pre-fetch with the same git log, exit 2 on failure, an offline gitleaks run, and a count that matches gitleaks' rule); (2) Lint is green in run 43220629206.

@scuttletoby
scuttletoby merged commit 82bebcb into main Nov 12, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants