Skip to content

⚑ Cut CI wall clock and fix the red develop build - #646

Open
jrosseel wants to merge 2 commits into
developfrom
feat/ci-speed
Open

⚑ Cut CI wall clock and fix the red develop build#646
jrosseel wants to merge 2 commits into
developfrom
feat/ci-speed

Conversation

@jrosseel

@jrosseel jrosseel commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Develop had been red for 60+ runs, and a red develop silently disables the Nx cache, nx-set-shas, and the smoke's baseline-image reuse β€” so the build got slower as it stayed broken. This fixes the break and the slowness together.

Rebased onto develop after #645 merged.

The red build

agent-style-check reported 9 errors on develop:

  • 8 were in libs/frontend/state/conversation/render/.upstream/ui/ β€” a pinned copy of someone else's source (it has a sibling VERSION file). The script's own comment says exclusions cover "vendored code"; .upstream/ was simply never added to the list.
  • 1 was real: a double ternary in runtime-bootstrap.router.test.ts:70. Split via a helper β€” ?? would have been wrong there, since identity is both optional and nullable and the two cases mean different things.

Also moved the three MCP wire interfaces into their existing paired mcp-gateway.types.ts (a TYPES-IN-IMPL violation a wider diff base surfaces).

Where the time went

Measured on run 31781214616. The critical path was the k3d smoke, not the test job.

phase before
5 smoke image builds, strictly serial 7m29s
agent-style-check (775 files) 5m06s
nx affected -t build test lint 4m34s
Storybook block, serial in the test job 3m55s

Changes

Style gate β€” ~7min β†’ 36s at 1101 files. It spawned a Node process per file for the inline-conditional check (loading the TypeScript compiler each time) plus ~8 grep/awk subprocesses per file. Now one pass per rule over the whole list. Verified byte-identical findings against the old script on a fixed 300-file sample (221 files checked, 2 errors, 47 warnings both ways); 2m02s β†’ 1.9s on that sample.

Nx cache now survives a run. actions/cache only saves when the job succeeded, so a repo this red never populated it β€” the only nx- entry on develop was days stale against an old lockfile hash. Split into cache/restore + cache/save with if: always(), and moved the restore above the SQL suites so those replay too.

Storybook targets are cacheable. test-storybook and test-storybook-visual were "cache": false; build-storybook declared outputs but never cache: true, and targetDefaults only covers build/test/lint, so it was uncached as well.

Their inputs also hard-coded a package list that had already drifted β€” 9 packages listed against 10 globbed by .storybook/main.ts (conversation-workspace was missing). Once caching is on, that drift is a false green, not a slow run. Replaced with one storybookCatalogue named input covering the frontend tree, deliberately broad so it cannot silently miss a package.

Verified both directions: a story change in conversation-workspace (the previously-missing package) correctly rebuilds; a backend-only change correctly replays from cache (0.9s vs a 44s build).

Storybook moved to its own job, parallel to test. Both publish jobs now gate on it, so splitting it out cannot let a broken component contract publish β€” and the guard test in affected-deployables.test.mjs was extended to hold that gate in place. Playwright's browser download is cached. --parallel=1 is kept and commented: it is required, not conservative, because both test targets bind port 4400.

Smoke images build a few at a time (SMOKE_BUILD_CONCURRENCY, default 3) instead of strictly one after another. Capped rather than fanned out to five, because the existing comment correctly noted the runner's Docker daemon is small. Each build's output is captured and replayed whole so concurrent progress lines stay readable.

Dockerfiles copy install manifests before npm ci and sources after. Every image copied COPY libs libs first, so editing any library invalidated the dependency layer and each of the 5+ images paid a full npm ci (~60s each). libs/ is not an npm workspace at all β€” the root workspaces field is apps/* and website, and @opencrane/* resolves through 96 tsconfig path aliases β€” so the install never needed it.

Freed ~2GB of Actions cache budget. The repo sits at 10.33GB against a 10GB ceiling, so entries were being LRU-evicted continuously. cache: npm on setup-node was pure waste in docker.yml: those jobs cache the resolved node_modules directly and both key off the same lockfile, so the npm download cache only ever hit when node_modules already hit. Left in place in docs/nightly/release, which do not cache node_modules.

Deliberately not switching the buildkit cache to mode=min β€” that caches only final-image layers and would defeat the npm ci layer caching above, since npm ci lives in an intermediate build stage.

Release stamping

Changing an app's Dockerfile adapts its release contract, so the seven touched apps move to the current root train (0.8.1): adaptedVersion, the mirrored package.json version, the chart appVersion that names the image, and the matching releases/0.8.1.json entries. Chart versions stay at 0.8.0 β€” no chart template changed, so no helm migration is owed β€” and the vendored subchart archives are repackaged so they match their source Chart.yaml.

Verification

All run locally against origin/develop:

  • Guard suites pass: agent-style, prisma-boundaries, affected-deployables, config-docs-coverage, pr-stack-integrity, module-growth, release-versioning, workload-ownership-app-composition, agent-domain-boundary.
  • Diff-scoped gates pass: agent-style-check --diff, check:module-growth, check:prisma-boundaries, check:release-versioning, check:agent-domain-boundary, check:workload-ownership-app-composition.
  • Style checker produces byte-identical findings vs the old script.
  • backend-server-authorization:test and frontend-mcp-adapter:lint pass.
  • Nx caching verified in both directions.
  • Workflow YAML parses; job graph confirmed.
  • npm ci proven to resolve with no libs/ present (2241 packages, dry run), and a real docker build of the UI image got through npm ci successfully.

Not verified locally: the UI image build past npm ci, and test:database-migrations β€” the machine ran out of disk and the local Docker VM was stopped. CI exercises both.

The style gate scanned files one process at a time, the Nx cache never survived
a run, Storybook sat serially inside the test job, and every smoke image rebuilt
its dependencies from scratch.

- agent-style-check: one node/grep/awk pass over the whole file list instead of
  ~9 processes per file (1101 files: ~7min -> 36s, byte-identical findings).
- Exclude vendored .upstream/ trees from the style rules, and split the one real
  double-ternary in runtime-bootstrap.router.test.ts.
- Move the MCP wire shapes into their paired mcp-gateway.types.ts.
- Nx cache: restore before the first Nx task and save with if: always(), so a run
  that fails late no longer leaves the next one cold.
- Storybook: build-storybook/test-storybook/test-storybook-visual are cacheable,
  and their inputs come from one storybookCatalogue named input rather than a
  hand-kept package list that had already drifted by one package.
- Storybook moves to its own job, parallel to test; both publish jobs still gate
  on it.
- Drop the redundant setup-node npm cache (node_modules is cached directly), which
  frees ~2GB against the repo's 10GB cache ceiling.
- Smoke images build a few at a time instead of strictly one after another.
- Dockerfiles copy the install manifests before npm ci and the sources after, so
  editing a library no longer invalidates the dependency layer.
Changing an app's Dockerfile adapts that app's release contract, so each touched
app moves to the current root train (0.8.1): metadata.release.adaptedVersion, the
mirrored package.json version, the chart appVersion that names the image, and the
matching entries in releases/0.8.1.json. Chart versions stay at 0.8.0 β€” no chart
template changed, so no helm migration is owed. The vendored subchart archives are
repackaged so they match their source Chart.yaml again.
@jrosseel

Copy link
Copy Markdown
Collaborator Author

CI status: the speedups landed; all three failures are pre-existing

Run 31798766239. Develop's own tip (eef7f283, this PR's base) fails on the same things β€” run 31798237803.

Measured effect of this PR

step before now
Enforce mechanical TypeScript agent style 5m06s 1s
smoke image preparation 7m29s 1m56s
Storybook 3m55s serial inside test 3m18s, parallel job

The k3d log shows three builds starting together at 12:09:16 and the rest at 12:11:12 β€” the capped fan-out behaving as intended. Save the Nx computation cache also ran despite the job failing, which is exactly the if: always() fix working; the next run gets a warm cache.

The three failures

  1. Run the current-silo smoke β€” fails identically on develop (eef7f283). CNPG postgres comes up after the pooler, so obot/litellm hit connection refused.
  2. Build and test affected Storybook component contracts β€” fails identically on develop. These are genuine addon-a11y colour-contrast violations in the conversation-workspace stories (e.g. #64748b on #e9fbfe at 4.46:1, needs 4.5:1), not a plumbing problem.
  3. Enforce monorepo dependency boundaries β€” 6 @nx/enforce-module-boundaries violations, none in a file this PR touches (zero overlap with the diff). This one is newly visible rather than newly broken: on develop the Storybook steps sit earlier in the same job, so the job died before ever reaching lint:boundaries. Moving Storybook out let the test job get further and surface it.

None of the three is caused by this change, and none is fixed by it either β€” they need separate work: a11y contrast fixes, boundary tag/dependency decisions, and the smoke's postgres ordering.

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.

1 participant