Skip to content

ci: fold six CI jobs into four with parallel and background steps; cancel superseded and closed-PR runs - #3658

Merged
thymikee merged 11 commits into
mainfrom
ci/fallow-speed
Oct 15, 2026
Merged

thymikee merged 11 commits into
mainfrom
ci/fallow-speed

Conversation

@thymikee

@thymikee thymikee commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Restructures ci.yml to use the new step-level parallel: and background:/wait: keywords (changelog). The CI workflow goes from six jobs to four, which takes two runner slots per PR off the queue.

Why fallow got slow

Since #3370 (fallow 2.104 → 3.32), "Run Fallow audit" takes 440–800s on CI instead of 6–9s. Bisected locally on the resolve imports row of fallow --performance: 3.19.0 takes 0.6s and 3.20.0+ takes 8.2–8.7s (206s on a 4-vCPU runner). Since 3.20, fallow walks all 28 root-tsconfig references for every import, stats each one and rebuilds a GlobSet each time: ~1.5M glob builds and 20M statx per run. Removing references gives identical findings, so the fix belongs upstream in fallow.

Dead ends, measured:

  • Caching .fallow: a PR head restored from a cache built at its base still took 524s for the audit and 209s for health. The cache only helps an identical tree, so I removed the caching steps.
  • Pinning fallow 3.19: it breaks the config (ignoreFindings unsupported, 79 production-export findings).
  • Configuration: fallow has no knob for tsconfig discovery.

Layout

Before After
Lint & Format, Typecheck & Package Lint, Typecheck & Package: di-seams, lint, format and typecheck in one parallel: group
Repo Guards, Compatibility & Provenance Repo Guards & Compatibility: the fallow audit runs as a background: step beside the guards; wait: fallow-audit comes before production-exports, which shares the .fallow cache dir
Coverage, Integration Tests unchanged

scripts/gate/workflows.ts now opens parallel: groups, so the gate manifest still credits each gate inside a group to its lane (new test in workflows.test.ts).

Failure semantics

Checked on a throwaway branch (run 38764903724, deleted since):

  • A failing background: true step fails the job at its wait: step, and the steps after the wait are skipped. This holds for a plain run: step and for a composite action such as run-gate.
  • A background step that is never waited on still fails the job, at the implicit "Wait for all background steps".

Timings (ran time; queue time excluded)

Job main, 3 recent runs a096d79
Lint & Format + Typecheck & Package 28–41s + 59–84s (2 jobs) 86s (1 job)
Repo Guards + Compatibility & Provenance 330–395s + 533–794s (2 jobs) 955s (1 job)
Coverage 563–642s on main (PR adds diff CRAP) 785s
Integration Tests 126–169s 167s

Trade-off: each PR now uses 4 runner slots instead of 6, and about 12% fewer runner-seconds (~2250s → ~1990s). With an ~80-minute queue, throughput is what counts. But on an idle queue, wall time is about 2.5 minutes worse, because the merged guards job (~16 min) is now the critical path. Parallel steps buy almost nothing on a 4-vCPU runner. A cold fallow audit already uses every core, so the work beside it just takes turns: freerange took 562s beside the audit, against ~250s alone. The real speedup needs the fallow fix upstream.

Also in this PR: run cancellation

  • pr-cancel-on-close.yml: on pull_request: closed, it cancels the PR's unfinished runs (same-repo PRs only, because a fork PR's token is read-only). A refused cancel fails the job, unless the run had already finished.
  • agent-guidance.yml and docs.yml gain the ci.yml concurrency group, so a new push replaces the PR's older run (main pushes still never cancel each other). test-app-build-cache.yml keeps its job-level cancel-in-progress: false.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.25 MB 5.25 MB -737 B
Package (unpacked) 5.25 MB 5.25 MB -737 B
Package (download) 1.58 MB 1.58 MB -244 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.3 ms 28.8 ms +0.5 ms
CLI --help 79.5 ms 82.2 ms +2.8 ms

thymikee and others added 4 commits October 15, 2026 15:20
…k with parallel steps

A cold fallow run re-resolves every import against the root tsconfig's 28
project references (~760 statx per import), which takes minutes on a hosted
runner; the persisted parse cache skips unchanged files. Main pushes save the
cache, every run restores it.

Parallel steps let the CI workflow drop from six jobs to four without
lengthening any lane: freerange runs in the background beside the other guards,
and lint/format/DI-seams run beside typecheck.

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

Copy link
Copy Markdown
Member Author

The temporary probe workflow is still in this PR, so it cannot merge as is. At 365a0f5, .github/workflows/fallow-probe.yml ends every fallow step in || true, so it gates nothing. The warm-from-base run had not finished, so no measurement backs the "make fallow fast" title, and the speedup and cache-key design are still unmeasured. Please post the warm-from-base timings for fallow audit, production-exports and health (cache built from base, compared with the cold run) in the PR body, then delete fallow-probe.yml before marking the PR ready. Lint, Typecheck & Package, Integration, Bundle Size and parallel-syntax have passed on this head. Repo Guards & Compatibility was at the fallow audit step and Coverage was still running, with every earlier step green. Since the diff changes ci.yml, those two jobs are the real test of the change, so both need to be green on this head. After merge, a main push should save the cache and the next PR run should show a restore hit. Not blocking, and you can take or leave these: the Repo Guards cache save step at ci.yml#L270 has no !cancelled() while the Coverage save step does, so the two jobs refresh the cache under different conditions, and the fallow-<job>-<sha> key has no fallow version or lockfile part, so a fallow bump restores a cache written by the old binary. One save condition in both jobs and hashFiles('pnpm-lock.yaml') or the fallow version in the key prefix would cover both. Next, report the probe timings, delete the probe file, and get those two jobs green.

The persisted cache only helps an identical tree: a PR head restored from its
base still re-resolves every import (524s audit, 209s health on CI). Starting
the audit beside freerange and the light guards takes it off the job's
sequential path instead.

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

Copy link
Copy Markdown
Member Author

The earlier findings from 365a0f5 no longer apply at c7404c5, but there is no CI result for this head yet, so I cannot call it ready. All the checks are still queued, and none has failed. The diff rewrites ci.yml, so Lint, Typecheck & Package, Repo Guards & Compatibility, Coverage and Integration Tests are the real test of this change. I have not found a code problem in the diff.

What is missing is a green run of Lint, Typecheck & Package and of Repo Guards & Compatibility on c7404c5. It also needs one run that shows a failing background gate turns Repo Guards red at its wait: step (see ci.yml:153 and the wait: steps at lines 261-269). Does a failed background: true step fail the job when wait: runs? If not, the fallow audit and numeric-ranges gates would silently stop blocking, and the gate-name manifest would not notice. A throwaway commit that makes gate: fallow or freerange exit 1 would show it. Please revert that commit before marking ready. Linking the GitHub documentation in the PR body would also work.

Not blocking, and you can take or leave these: the PR body still says "WIP" and mentions the deleted probe job and the dropped caching step, and it has no before/after job times to back "make fallow fast". The nested-vitest tmpdir-leaks-model step at ci.yml:205 now runs beside fallow and freerange on the same runner, so please watch a few runs for flakes and wait on fallow before that step if any appear.

Once Repo Guards and Lint, Typecheck & Package are green on c7404c5 and the background-failure check is shown, this is ready for another look.

@thymikee thymikee changed the title ci: make fallow fast on CI and tighten CI job layout ci: fold six CI jobs into four with parallel and background steps Oct 15, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Addressed:

  • Green on c7404c5: all four jobs passed (Lint, Typecheck & Package 95s; Repo Guards & Compatibility 912s; Coverage 809s; Integration 178s).
  • Background failure semantics: proven on a throwaway branch (run 38764903724, since deleted). A failing background: true step, whether a plain run: step or a composite like run-gate, fails the job at its wait: step, and the steps after the wait are skipped. A background step that is never waited on still fails the job at the implicit "Wait for all background steps". The gates can't silently stop blocking.
  • PR body: rewritten with the root cause, the dead ends (caching removed), the layout and before/after timings.
  • tmpdir-leaks flake risk / contention: at c7404c5, freerange running in the background slowed both it (672s vs ~250s) and the audit. 249e6ed moves freerange back into the sequential steps, so only the fallow audit shares the runner, which also takes load off the nested-vitest step. Waiting on CI for that head.

@thymikee

Copy link
Copy Markdown
Member Author

This follow-up covers 249e6ed. The earlier review (#3658 (comment)) was at c7404c5. The probe job is gone and the job layout is simpler: one background gate instead of two. I found nothing wrong in the diff. The change is not ready to merge yet because the CI evidence is missing. All 10 non-skipped checks for 249e6ed are queued and none has failed, so I have no timings or results for the new layout. Please show a run where a failing background fallow audit turns Repo Guards red at the "wait: fallow-audit" step and the job concludes failure. A throwaway commit that adds a deliberate violation, then gets reverted, would prove it, and the same run should show a failing foreground step cancelling the background step. Also please show a green Repo Guards run at 249e6ed with step timings, so we can see the job still fits the 20 minute timeout now that freerange runs after the guards. No conflicts are known. Once that evidence is in, the next step is to merge. Not blocking, and you can take or leave these: if the freerange move pushes the job past about 15 minutes, move it back before the guards or raise the timeout, and the PR body still says "WIP" and mentions a probe job that the diff no longer has.

The branch has since moved to a096d79, which adds a pr-cancel-on-close job and concurrency settings in other CI files. This review does not cover those, and the evidence above is still needed.

@thymikee

Copy link
Copy Markdown
Member Author

I found no blocking problems at a096d79. The earlier concern is fixed: a096d79 now has the timings and green runs on the restructured ci.yml jobs. Lint, Typecheck & Package, Repo Guards & Compatibility and Coverage pass, and so do the Analyze runs for actions, java-kotlin and javascript-typescript. Analyze (python) is still pending and covers no file in this diff. I found no conflicts.

Not blocking, and you can take or leave these: the workflow-level concurrency in test-app-build-cache.yml seems to cancel PR runs on the next push. Does the job-level cancel-in-progress: false at line 117 still have any effect on PRs? If not, drop the workflow-level group or drop the job-level one and say that PR producer builds are now cancelled. Do the consumers in ios.yml or android.yml rely on an earlier head's producer finishing? I could not tell. In pr-cancel-on-close.yml, gh run cancel ... || true hides permission errors too, so ignore only the "already completed" case. That workflow only fires on a PR close event, so I checked its jq filter and pagination by reading only. It and the new concurrency blocks in agent-guidance, docs and test-app-build-cache are a second change, so a separate PR or a mention in the title and body would help. Last, Repo Guards & Compatibility took 15m55s and Coverage took 13m05s at a096d79, against 330-794s and 563-642s before, so adding those timings and the slot-versus-wall-time trade-off to the body would help.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 15, 2026
@thymikee
thymikee marked this pull request as ready for review October 15, 2026 17:35
@thymikee thymikee changed the title ci: fold six CI jobs into four with parallel and background steps ci: fold six CI jobs into four with parallel and background steps; cancel superseded and closed-PR runs Oct 15, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread .github/workflows/pr-cancel-on-close.yml
Comment thread .github/workflows/pr-cancel-on-close.yml
@thymikee

Copy link
Copy Markdown
Member Author

The changes since a096d79 look good at 01992b3, and I found no new problems. The close-time cancel filter now matches the PR number from the event, so a branch shared with another PR no longer matches. I checked this by reading the workflow and the live runs API output. I did not run the cancel workflow end to end, because it only fires when a PR closes. It will first run on this PR's own close.

Of the two earlier inline threads, the shared-branch match is fixed at this head and can be resolved. The ref suffix check is already resolved and holds, because live API paths carry no @ref suffix.

The timings and the background:/wait: failure behavior come from the author's throwaway-branch runs. I did not re-run them. The earlier review already covered them.

The only check not passing is Smoke Tests, which is still queued and has no log. The new changes touch only pr-cancel-on-close.yml and test-app-build-cache.yml, so they do not affect that route. Please wait for Smoke Tests to finish before merging.

@thymikee
thymikee merged commit 2a2f3f2 into main Oct 15, 2026
19 checks passed
@thymikee
thymikee deleted the ci/fallow-speed branch October 15, 2026 18:40
@github-actions

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-15 18:57 UTC

thymikee added a commit that referenced this pull request Oct 16, 2026
…ctions-1

* origin/main:
  feat(react-devtools): inspect React components and profile renders through the Metro inspector (#3613)
  refactor(daemon-contracts): drop the registration and claims fixture exports (#3689)
  refactor(test-kit): drop the daemon-contracts dependency (#3686)
  refactor(test-kit): drop command-registry by making the replay-script generator a root fixture (#3687)
  fix(ios): handle delayed launch confirmations without stopping XCTest (#3682)
  refactor(test-kit): move the dispatch-disclosure harness out of contracts (#3690)
  refactor(test-kit): host the target classification capture fixture (#3691)
  ci: fold six CI jobs into four with parallel and background steps; cancel superseded and closed-PR runs (#3658)
  build(typescript): move root project references into tsconfig.typecheck.json (#3688)
  chore(daemon): declare @agent-device/proxy as a devDependency, since only a test imports it (#3685)
  refactor(settings): declare settings once and admit them per setting (#3662)

# Conflicts:
#	packages/platform-android/src/alert.ts
#	website/docs/docs/limrun.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant