ci: build images once, push to GHCR, reuse in smoke/validate/scan#1695
Merged
Conversation
Previously `build`, `smoke-test`, `image-validate`, and `vulnerability-scan`
each rebuilt the same six service images from scratch in parallel — ~4×
the compute per push for identical artifacts. None of them shared a
build cache.
Now `build` is the single producer: it always builds all six compose-stack
services (db, convex, crawler, rag, platform, proxy), uses GHA layer cache
per service, and pushes to ghcr.io/tale-project/tale/tale-{svc} tagged
`pr-{n}-sha-{shortsha}` (PRs) or `sha-{shortsha}` (main / merge_group).
Downstream jobs depend on `build`, log into GHCR, pull and re-tag locally
as `:latest`, then run the existing test scripts with `SKIP_BUILD=true`
and `PULL_POLICY=never` — no script changes required, both env vars were
already supported. `vulnerability-scan` skips the rebuild and points trivy
straight at the GHCR ref. Web/docs are excluded from the build matrix
(they have their own compose stacks and dedicated test jobs); their
vulnerability coverage continues via security.yml's filesystem scan.
A new cleanup-pr-images workflow runs on `pull_request: closed` and
deletes every GHCR container version tagged `pr-{n}-sha-*` for the closed
PR so per-PR tags don't accumulate.
Contributor
📝 WalkthroughWalkthroughThis PR refactors the container CI pipeline to centralize image building and distribution via GHCR. The Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yml:
- Around line 242-251: Hardcoded registry in the docker tag step causes fork
incompatibility; update the tag to use the existing REGISTRY_PATH variable (TAG
and REGISTRY_PATH are set above and the for svc loop does the pulls) so the
docker tag command tags the pulled IMAGE as
"${REGISTRY_PATH}/tale-${svc}:latest" instead of the hardcoded
ghcr.io/tale-project/tale/tale-${svc}:latest.
- Around line 356-365: The docker tag target is hardcoded to ghcr.io; update the
tag to use the REGISTRY_PATH variable instead. In the loop that builds IMAGE
(variables: TAG, REGISTRY_PATH, IMAGE, svc), replace the hardcoded target
"ghcr.io/tale-project/tale/tale-${svc}:latest" with a tag that uses
REGISTRY_PATH (e.g. "${REGISTRY_PATH}/tale-${svc}:latest") so docker tag uses
the same registry path as docker pull.
In @.github/workflows/cleanup-pr-images.yml:
- Line 63: The file .github/workflows/cleanup-pr-images.yml is missing a
trailing newline after the final line containing "done"; open the workflow file
and add a single newline character after that final line so the file ends with a
newline (ensure the last token "done" remains unchanged).
- Around line 46-50: The current pipeline appends "|| true" after the gh api
call that populates VERSION_IDS which hides all failures; update the logic so
the gh api call's exit code is checked instead of always ignoring errors: run
the gh api to fetch VERSION_IDS, capture its exit status and the HTTP response,
and if the call fails only suppress the error when the HTTP status is 404 (no
versions found); for any other non-zero exit/status (auth, rate limit,
permission, network), fail the job and surface the error via echo/exit or
processLogger equivalent. Ensure you reference VERSION_IDS and the gh api
invocation when making the change so the behavior is limited to this lookup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b03242be-75b3-40c8-8592-baa0819adc65
📒 Files selected for processing (2)
.github/workflows/build.yml.github/workflows/cleanup-pr-images.yml
Comment on lines
+242
to
+251
| - name: Pull images from GHCR | ||
| run: | | ||
| TAG="${{ needs.changes.outputs.image_tag }}" | ||
| REGISTRY_PATH="${{ env.REGISTRY }}/${{ github.repository }}" | ||
| for svc in db convex crawler rag platform proxy; do | ||
| IMAGE="${REGISTRY_PATH}/tale-${svc}:${TAG}" | ||
| echo "Pulling ${IMAGE}..." | ||
| docker pull "${IMAGE}" | ||
| docker tag "${IMAGE}" "ghcr.io/tale-project/tale/tale-${svc}:latest" | ||
| done |
Contributor
There was a problem hiding this comment.
Hardcoded registry path in retag command breaks fork compatibility.
Line 250 hardcodes ghcr.io/tale-project/tale/tale-${svc}:latest while lines 244-249 correctly use REGISTRY_PATH. If this workflow runs on a fork, images will be pulled from the fork's GHCR but retagged with the upstream path, which may cause confusion or failures.
🐛 Proposed fix to use consistent registry path
- name: Pull images from GHCR
run: |
TAG="${{ needs.changes.outputs.image_tag }}"
REGISTRY_PATH="${{ env.REGISTRY }}/${{ github.repository }}"
for svc in db convex crawler rag platform proxy; do
IMAGE="${REGISTRY_PATH}/tale-${svc}:${TAG}"
echo "Pulling ${IMAGE}..."
docker pull "${IMAGE}"
- docker tag "${IMAGE}" "ghcr.io/tale-project/tale/tale-${svc}:latest"
+ docker tag "${IMAGE}" "${REGISTRY_PATH}/tale-${svc}:latest"
done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build.yml around lines 242 - 251, Hardcoded registry in
the docker tag step causes fork incompatibility; update the tag to use the
existing REGISTRY_PATH variable (TAG and REGISTRY_PATH are set above and the for
svc loop does the pulls) so the docker tag command tags the pulled IMAGE as
"${REGISTRY_PATH}/tale-${svc}:latest" instead of the hardcoded
ghcr.io/tale-project/tale/tale-${svc}:latest.
Comment on lines
+356
to
+365
| - name: Pull images from GHCR | ||
| run: | | ||
| TAG="${{ needs.changes.outputs.image_tag }}" | ||
| REGISTRY_PATH="${{ env.REGISTRY }}/${{ github.repository }}" | ||
| for svc in db convex crawler rag platform proxy; do | ||
| IMAGE="${REGISTRY_PATH}/tale-${svc}:${TAG}" | ||
| echo "Pulling ${IMAGE}..." | ||
| docker pull "${IMAGE}" | ||
| docker tag "${IMAGE}" "ghcr.io/tale-project/tale/tale-${svc}:latest" | ||
| done |
Contributor
There was a problem hiding this comment.
Same hardcoded registry path issue as in smoke-test.
Line 364 hardcodes ghcr.io/tale-project/tale/tale-${svc}:latest instead of using the REGISTRY_PATH variable.
🐛 Proposed fix
- name: Pull images from GHCR
run: |
TAG="${{ needs.changes.outputs.image_tag }}"
REGISTRY_PATH="${{ env.REGISTRY }}/${{ github.repository }}"
for svc in db convex crawler rag platform proxy; do
IMAGE="${REGISTRY_PATH}/tale-${svc}:${TAG}"
echo "Pulling ${IMAGE}..."
docker pull "${IMAGE}"
- docker tag "${IMAGE}" "ghcr.io/tale-project/tale/tale-${svc}:latest"
+ docker tag "${IMAGE}" "${REGISTRY_PATH}/tale-${svc}:latest"
done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Pull images from GHCR | |
| run: | | |
| TAG="${{ needs.changes.outputs.image_tag }}" | |
| REGISTRY_PATH="${{ env.REGISTRY }}/${{ github.repository }}" | |
| for svc in db convex crawler rag platform proxy; do | |
| IMAGE="${REGISTRY_PATH}/tale-${svc}:${TAG}" | |
| echo "Pulling ${IMAGE}..." | |
| docker pull "${IMAGE}" | |
| docker tag "${IMAGE}" "ghcr.io/tale-project/tale/tale-${svc}:latest" | |
| done | |
| - name: Pull images from GHCR | |
| run: | | |
| TAG="${{ needs.changes.outputs.image_tag }}" | |
| REGISTRY_PATH="${{ env.REGISTRY }}/${{ github.repository }}" | |
| for svc in db convex crawler rag platform proxy; do | |
| IMAGE="${REGISTRY_PATH}/tale-${svc}:${TAG}" | |
| echo "Pulling ${IMAGE}..." | |
| docker pull "${IMAGE}" | |
| docker tag "${IMAGE}" "${REGISTRY_PATH}/tale-${svc}:latest" | |
| done |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build.yml around lines 356 - 365, The docker tag target is
hardcoded to ghcr.io; update the tag to use the REGISTRY_PATH variable instead.
In the loop that builds IMAGE (variables: TAG, REGISTRY_PATH, IMAGE, svc),
replace the hardcoded target "ghcr.io/tale-project/tale/tale-${svc}:latest" with
a tag that uses REGISTRY_PATH (e.g. "${REGISTRY_PATH}/tale-${svc}:latest") so
docker tag uses the same registry path as docker pull.
Comment on lines
+46
to
+50
| VERSION_IDS=$(gh api -H "Accept: application/vnd.github+json" \ | ||
| "/orgs/${OWNER}/packages/container/${PACKAGE}/versions" \ | ||
| --paginate \ | ||
| --jq "[.[] | select(any(.metadata.container.tags[]?; startswith(\"${PREFIX}\"))) | .id] | .[]" \ | ||
| || true) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
|| true silently swallows API failures, including auth or permission issues.
If the gh api call fails due to authentication problems, rate limiting, or incorrect permissions, the workflow exits cleanly with "No matching versions found" rather than surfacing the error. Consider capturing the exit code and only ignoring "not found" (404) responses.
♻️ Proposed fix to surface real failures
- VERSION_IDS=$(gh api -H "Accept: application/vnd.github+json" \
- "/orgs/${OWNER}/packages/container/${PACKAGE}/versions" \
- --paginate \
- --jq "[.[] | select(any(.metadata.container.tags[]?; startswith(\"${PREFIX}\"))) | .id] | .[]" \
- || true)
+ if ! VERSION_IDS=$(gh api -H "Accept: application/vnd.github+json" \
+ "/orgs/${OWNER}/packages/container/${PACKAGE}/versions" \
+ --paginate \
+ --jq "[.[] | select(any(.metadata.container.tags[]?; startswith(\"${PREFIX}\"))) | .id] | .[]" 2>&1); then
+ # 404 is expected when package has no versions yet; other errors should surface.
+ if echo "${VERSION_IDS}" | grep -q "404"; then
+ echo "No package versions exist for tale-${SVC} (404)."
+ exit 0
+ fi
+ echo "::error::Failed to list versions: ${VERSION_IDS}"
+ exit 1
+ fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/cleanup-pr-images.yml around lines 46 - 50, The current
pipeline appends "|| true" after the gh api call that populates VERSION_IDS
which hides all failures; update the logic so the gh api call's exit code is
checked instead of always ignoring errors: run the gh api to fetch VERSION_IDS,
capture its exit status and the HTTP response, and if the call fails only
suppress the error when the HTTP status is 404 (no versions found); for any
other non-zero exit/status (auth, rate limit, permission, network), fail the job
and surface the error via echo/exit or processLogger equivalent. Ensure you
reference VERSION_IDS and the gh api invocation when making the change so the
behavior is limited to this lookup.
| gh api -X DELETE \ | ||
| "/orgs/${OWNER}/packages/container/${PACKAGE}/versions/${version_id}" \ | ||
| || echo "Failed to delete version ${version_id} (may already be gone)" | ||
| done |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Missing trailing newline at end of file.
POSIX convention expects files to end with a newline. Add a blank line after line 63.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/cleanup-pr-images.yml at line 63, The file
.github/workflows/cleanup-pr-images.yml is missing a trailing newline after the
final line containing "done"; open the workflow file and add a single newline
character after that final line so the file ends with a newline (ensure the last
token "done" remains unchanged).
Critical fixes
- Fork-PR fallback: forks get a read-only GITHUB_TOKEN and cannot push to
GHCR; build now switches to push:false + load:true on forks, and new
smoke-test-fork / image-validate-fork jobs run the local-build pipeline
so external contributor PRs keep CI parity.
- Cleanup workflow surfaces real failures: list call gets a 3-attempt
retry; per-DELETE results are counted (deleted / 404-already-gone /
failures) and reported to the step summary; non-404 failures fail the
job instead of being silently swallowed.
- Failure-summary log redirect: the >> only chained to the echo, so
`docker compose logs` and `ps -a` were writing to stdout instead of
$GITHUB_STEP_SUMMARY. Now grouped in { ... } >> $GITHUB_STEP_SUMMARY
with --tail=300 and collapsible <details> blocks.
Hygiene
- Storybook trigger gate broadened: new `storybook` paths-filter covers
services/platform/**, packages/ui/**, packages/webui/** (where the
bulk of stories actually live).
- TURBO_TOKEN scoped to the storybook job (was workflow-level env,
process-readable by every action in every job).
- SARIF upload guarded with hashFiles(...) so a Trivy crash doesn't
surface as a confusing red cross on the upload step; missing-SARIF
fallback writes a step-summary note.
- Action SHA pin comments tightened: docker/login-action # v4 -> # v4.1.0
(4× sites); github/codeql-action # v4 -> # v4.35.3; bumped
aquasecurity/trivy-action 0.35.0 -> v0.36.0.
- Storybook timeout 15->5 min; web-test/docs-test 15->8 min (per
measured p95).
- Service-list cross-references at all 4 duplication sites.
- ci_tests filter narrowed from blanket services/** to the 6 compose-stack
services explicitly.
- Failure-summary step on build-job matrix legs (was empty on failure).
- provenance:false rationale documented inline; multi-arch caveat added
to cleanup workflow's jq filter.
- fetch-depth note for future Dockerfile authors.
Top-level paths missed several Docker-affecting files (docs/, patches/,
.dockerignore, bunfig.toml, tsconfig.base.json, turbo.json, .env.test);
docs-only PRs in particular never reached the existing docs-test job.
Add VERSION=${image_tag} to mirror release.yml — service Dockerfiles bake
this into TALE_VERSION, used by Sentry release tagging and convex seed
marker namespacing. Without it PR images reported SENTRY_RELEASE=unknown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
build,smoke-test,image-validate, and (on push)vulnerability-scaneach rebuilt the same six service images independently — duplicate compute per run for identical artifacts with no shared cache.buildis now the single producer: full 6-service matrix (db, convex, crawler, rag, platform, proxy), per-service GHA layer cache, pushes toghcr.io/tale-project/tale/tale-{svc}:{tag}where tag ispr-{n}-sha-{shortsha}on PRs andsha-{shortsha}on main/merge_group. Downstream jobsneeds: build, log into GHCR, pull + re-tag locally as:latest, then run the existing test scripts withSKIP_BUILD=true PULL_POLICY=never(env vars already supported — no script changes).vulnerability-scanpoints trivy directly at the GHCR ref.A new
cleanup-pr-images.ymlworkflow deletes everypr-{n}-sha-*tag for the closed PR via the GHCR API onpull_request: closed.Tradeoffs (be aware before merging)
vulnerability-scanruns only onevent_name == 'push', so PRs get no image-level CVE scan today — onlysecurity.yml's filesystem scan, which catches lockfile CVEs and Dockerfile misconfigs but not base-image OS-package CVEs. Web/docs Dockerfiles are also not image-scanned anywhere on CI today. A follow-up PR should either (a) add web/docs to thevulnerability-scanmatrix on push-to-main, or (b) extendsecurity.ymlwith a weekly cron that builds + image-scans both static-site Dockerfiles.Hardening from review feedback
This PR also includes the items surfaced by the multi-agent review:
GITHUB_TOKENand cannot push to GHCR. Build switches topush: false+load: trueon forks; newsmoke-test-fork/image-validate-forkjobs run the local-build pipeline (the pre-refactor shape) so external contributor PRs keep CI parity.>>was only chaining to theecho;docker compose logsandps -awere writing to stdout instead of$GITHUB_STEP_SUMMARY. Now grouped in{ … } >> $GITHUB_STEP_SUMMARYwith--tail=300and collapsible<details>blocks.storybookpaths-filter coversservices/platform/**,packages/ui/**,packages/webui/**(where the bulk of stories actually live).env:(visible to every job/action) to the storybook job only.hashFiles(...)so a Trivy crash doesn't surface as a confusing red cross on the upload step.docker/login-action # v4→# v4.1.0;codeql-action # v4→# v4.35.3);aquasecurity/trivy-actionbumped to v0.36.0.ci_testsfilter narrowed from blanketservices/**to the 6 compose-stack services explicitly.provenance: falserationale documented inline.Test plan
buildmatrix produces 6 successful pushes toghcr.io/tale-project/tale/tale-{svc}:pr-1695-sha-<sha>(visible under the org Packages tab).smoke-testlogs showPulling …lines for all 6 services,Skipping build (SKIP_BUILD=true), and all healthchecks pass (target: 18/18).image-validatelogs showSKIP_BUILD=true — using pre-built imagesand all assertions pass (target: 30/30).Storybookjob triggers on this PR (packages/ui/**orservices/platform/**paths) and builds successfully.smoke-test (fork PR)andValidate images (fork PR)are skipped (this PR is same-repo).Cleanup PR imagesworkflow fires; per-service step summaries showDeleted: N/Already gone: M/Failures: 0; thepr-1695-sha-*tags disappear from GHCR.mainafter merge:vulnerability-scanpulls from GHCR and uploads SARIF without rebuilding.Follow-ups (not in this PR)
tale-webandtale-docs(filesystem scan today doesn't cover base-image OS packages).Summary by CodeRabbit