ci: skip docker image build when unused, harden e2e changes checkout#59780
Conversation
The "Build Docker image" job in container-images-ci.yml was running on every non-rust/livestream PR (~15-25 min on depot-ubuntu-latest-4) but is only consumed by ci-hobby.yml's `wait-for-docker-image`. When ci-hobby would skip, the build is wasted work — most FE/BE PRs never trigger it. Restrict the build to the same trigger conditions ci-hobby uses (mirror of its `hobby:` filter + `hobby-preview` label), plus changes to the build workflow itself so workflow edits are still exercised pre-merge. Add `labeled`/ `unlabeled` to the workflow trigger so adding `hobby-preview` retroactively still kicks off a build (matches ci-hobby.yml's trigger types). Separately, the `Determine need to run E2E checks` job intermittently fails its actions/checkout@v6.0.2 step with `fatal: could not read Username for 'https://github.com'` from the post-fetch `git checkout` (e.g. PR #59779 on 2026-05-23, which blocked merge until the job was retried). The cause is v6.0.2 lazy-fetching every blob in HEAD from the promisor remote when `filter: blob:none` is set, and the per-blob credential helper lookup occasionally racing with the includeIf-scoped auth file write. The job only reads `.github/clickhouse-versions.json` plus a few git ref operations, so a sparse-checkout pinned to that one file collapses the lazy fetch to a single blob — small enough to no longer trip the credential issue, with no impact on the existing fast path.
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
.github/workflows/container-images-ci.yml:5-8
The inline comment claims the two workflows "use the same trigger types" and "stay in lock-step", but `ci-hobby.yml` uses `[opened, synchronize, labeled, unlabeled]` — it does not include `reopened`. As a result, re-opening a PR that touches hobby files will trigger an image build here while ci-hobby never wakes up to consume it, producing a ~15–25 min build that the PR description explicitly aims to eliminate.
```suggestion
# `labeled`/`unlabeled` lets `hobby-preview` retroactively trigger
# an image build. Types are kept in sync with ci-hobby.yml; note that
# ci-hobby.yml does not include `reopened`, so neither does this list.
types: [opened, synchronize, labeled, unlabeled]
```
Reviews (1): Last reviewed commit: "ci: skip docker image build when unused,..." | Re-trigger Greptile |
ci-hobby.yml uses [opened, synchronize, labeled, unlabeled] (no `reopened`), so mirroring it means dropping `reopened` here too. Otherwise reopening a PR that touches hobby paths would build an image that ci-hobby never wakes up to consume — exactly the wasted ~20 min build this PR aims to eliminate. Caught by Greptile review. Co-authored-by: Cursor <cursoragent@cursor.com>
Renames the secondary filter `build_self` -> `build_inputs` and adds `pnpm-lock.yaml` to it. ci-hobby's filter intentionally ignores frontend dep changes (they're irrelevant to the smoke test), but they can still break the production Docker build (different prod-only install flags, multi-arch). With just the `hobby:` filter, Renovate-style lockfile bumps would skip the build on the PR and only fail at merge_group / master push. Two extra paths is a small price for catching these on the PR rather than turning master red. Co-authored-by: Cursor <cursoragent@cursor.com>
Not sure this is true, |
|
I see, I'm not an expert in this so happy to close this PR if it doesn't make sense. I made it while waiting for CI to finish :D |
| # Keep `hobby:` in sync with the same-named filter in ci-hobby.yml — | ||
| # drift means hobby waits for an image we never build (or vice versa). | ||
| hobby: | ||
| - Dockerfile |
There was a problem hiding this comment.
nit: should we add .Dockerignore?
There was a problem hiding this comment.
Good catch — added .dockerignore to build_inputs: rather than hobby:. .dockerignore changes the build context but doesn't affect hobby's runtime, and hobby: is supposed to mirror ci-hobby.yml's filter byte-for-byte (which doesn't list .dockerignore either).
There was a problem hiding this comment.
Sorry Cursor replied, but does this make sense?
The top-of-file comment claimed `container-images-ci.yml` produced the image consumed here, but the only image this workflow pulls is `posthog-node` from `ci-nodejs-container.yml` (see the `Resolve Node.js container image tag` step). Fix the comment so the dependency is searchable. Co-authored-by: Cursor <cursoragent@cursor.com>
`.dockerignore` silently changes the build context — a broken change should fail on the PR, not at merge_group. Putting it under `build_inputs:` (not `hobby:`) keeps the `hobby:` filter byte-for-byte in sync with `ci-hobby.yml`, which doesn't watch `.dockerignore` either. Co-authored-by: Cursor <cursoragent@cursor.com>
|
tagging @gantoine since we looked at e2e this week |
gantoine
left a comment
There was a problem hiding this comment.
assuming these last CI checks pass this looks good
Summary
Two unrelated CI fixes I noticed while debugging slow CI on a 1-line test PR (#59779). Both touch only
.github/workflows/, both are easy to revert independently.1. Skip
Build Docker imagewhen no consumer waits for itcontainer-images-ci.yml'sposthog_buildruns ~15–25 min ondepot-ubuntu-latest-4and is gated by:i.e. anything except rust/livestream — basically every PR. But the only consumer of the resulting image is
ci-hobby.yml'swait-for-docker-imagejob (it polls this workflow by check name "Build Docker image" viafountainhead/action-wait-for-check). When ci-hobby would skip — the common case for FE/BE PRs — the build is pure waste.This PR mirrors
ci-hobby.yml'shobby:paths-filter directly intocontainer-images-ci.ymland gatesposthog_buildon the union of:hobby:(matches ci-hobby.yml'shobby:filter — must stay in sync)build_self:(changes to the build workflow / build-n-cache-image action)hobby-previewPR labelmerge_group(preserved — image still built before merge)Also adds
labeled/unlabeledto thepull_requesttrigger so addinghobby-previewretroactively kicks off a build (matchesci-hobby.yml's trigger types — without this, the label would arm hobby but never trigger the image build that hobby waits for).I confirmed the only in-repo consumer is hobby —
ci-e2e-playwright.ymland the other "Wait for Docker services" steps usebin/ci-wait-for-docker launch(docker-compose for ephemeral services), not theposthog_buildimage. Hobby already handlesBUILD_CONCLUSION = "skipped"gracefully (ci-hobby.ymlline 291–293, exit 0).Expected impact: ~15–25 min off most FE / BE / docs / test-only PRs.
2. Harden
Determine need to run E2E checksagainst the v6.0.2 promisor-fetch flakeThe
changesjob inci-e2e-playwright.ymlintermittently fails itsactions/checkout@v6.0.2step:Root cause:
filter: blob:nonemakes the post-fetchgit checkoutlazy-fetch every blob in HEAD from the promisor remote, and v6.0.2's per-blob credential helper lookup occasionally races with the includeIf-scoped auth file write. Result: any PR can randomly hit a hardBLOCKEDmerge state until the job is retried (this exact failure on #59779 earlier today).The job only reads
.github/clickhouse-versions.jsonplus a few git ref operations. Addingsparse-checkout: .github/clickhouse-versions.jsoncollapses the lazy fetch to a single blob — no longer enough surface for the credential issue to manifest, with no impact on the existing fast path.This is a targeted, surgical fix for the one failing checkout. The same pattern exists in
ci-backend.ymlandci-dagster.yml'schangesjobs and may want similar treatment if they ever flake — leaving that as a follow-up to keep this PR small.Test plan
Build Docker imagejob should appear as skipped on this PR (workflow-only change, no hobby files touched, nohobby-previewlabel) — that's the whole point.frontend/), confirmBuild Docker imageskips.hobby-previewlabel to a test PR and confirmBuild Docker imagere-runs.Build Docker imageruns.Out of scope (for follow-up PRs)
--findRelatedTests) forci-frontend.yml— biggest remaining FE win.shadow-story-selectionfrom advisory to authoritative onci-storybook.yml.frontend_codefilter to exclude**/*.test.{ts,tsx}so test-only edits don't trigger bundle-size / typecheck / VR.semgrep-*per-language gating (semgrep-jsis 3m+ on every PR regardless of touched languages).Made with Cursor