Updated Tinybird CI to trigger & follow infra workflow#27058
Updated Tinybird CI to trigger & follow infra workflow#27058
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughCI changes remove Tinybird cloud 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 761-779: The dispatch currently uses a moving ref and picks the
newest run by timestamp which causes race conditions; change the dispatch
payload to pass an immutable commit SHA and a unique caller token (e.g., set
ghost_ref='${{ github.sha }}' and add a field like caller_run_id='${{
github.run_id }}'), then change the run-selection logic (the gh workflow run
invocation / the for-loop that sets RUN_ID using the --jq 'map(select(.createdAt
>= ...)) | sort_by(.createdAt) | last | .databaseId // empty' expression) to
instead select the exact run by matching the caller token (or headSha) returned
by the workflow (e.g., query the run JSON for the token/input or headSha and
select where that equals the dispatched value) so RUN_ID and RUN_URL map to the
precise dispatched run.
- Around line 751-755: The job "Trigger and watch traffic analytics infra
Tinybird workflow" currently uses if: github.repository == 'TryGhost/Ghost' but
that still runs for forked pull_request events where secrets are unavailable;
update the if to also ensure the PR head repo is the same repo or the event is
not a pull_request, e.g. change the condition to: github.repository ==
'TryGhost/Ghost' && (github.event.pull_request == null ||
github.event.pull_request.head.repo.full_name == github.repository) so the job
is skipped for forked PRs that lack secrets.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 94467c45-d693-4672-87f5-ac3b6e910854
📒 Files selected for processing (1)
.github/workflows/ci.yml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #27058 +/- ##
==========================================
- Coverage 73.26% 73.21% -0.06%
==========================================
Files 1530 1531 +1
Lines 121767 121796 +29
Branches 14710 14698 -12
==========================================
- Hits 89218 89168 -50
- Misses 31555 31634 +79
Partials 994 994
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JoeeGrigg
requested review from
9larsons,
EvanHahn,
cmraible and
troyciesco
as code owners
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
763-779:⚠️ Potential issue | 🔴 CriticalCorrelate the exact dispatched run and pin the Ghost revision.
github.refis a PR merge ref onpull_requestand a moving branch ref onpush, whilegithub.shais the triggering commit SHA andgithub.run_attemptincrements on reruns. Combined with thecreatedAt/lastlookup, that makes this correlation ambiguous enough that a later push, rerun, or unrelatedworkflow_dispatchcan be the run this job ends up watching. (docs.github.com)Suggested direction
gh workflow run "$WORKFLOW" \ --repo "$REPO" \ --ref "$BRANCH" \ - -f ghost_ref='${{ github.ref }}' \ + -f ghost_ref='${{ github.sha }}' \ + -f caller_token='${{ github.run_id }}-${{ github.run_attempt }}' \ -f run_local_tests='false'Then have the infra workflow surface
caller_tokenin its run name, or another field you can query here, and filter this lookup on that token instead ofsort_by(.createdAt) | last.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 763 - 779, The current run lookup using ghost_ref='${{ github.ref }}' and sorting by createdAt/last is ambiguous; instead pass and pin the exact triggering SHA (use github.sha) or a unique caller_token when dispatching (e.g., include ghost_revision=github.sha or caller_token) and then change the gh run list query to filter on that token/SHA (match the run name/field you surface) rather than using sort_by(.createdAt) | last; update the dispatch arguments where ghost_ref is set and the lookup that builds RUN_ID/RUN_URL (the gh workflow run invocation and the jq/select in the RUN_ID assignment) to use the new pinned field for deterministic correlation.
751-756:⚠️ Potential issue | 🟠 MajorSkip the remote dispatch when the auth token is unavailable.
pull_requestevents from forks are sent to the base repository, sogithub.repositorystill matchesTryGhost/Ghosthere, but repository secrets other thanGITHUB_TOKENare withheld and unresolved secrets evaluate to an empty string. This step therefore reaches the remote-dispatch path with an emptyGH_TOKENon fork PRs. (docs.github.com)Suggested fix
- name: Trigger and watch traffic analytics infra Tinybird workflow if: github.repository == 'TryGhost/Ghost' env: GH_TOKEN: ${{ secrets.CANARY_DOCKER_BUILD }} run: | set -euo pipefail + + if [ -z "${GH_TOKEN:-}" ]; then + echo "Skipping remote infra Tinybird workflow because GH_TOKEN is unavailable for this event." + exit 0 + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 751 - 756, The step "Trigger and watch traffic analytics infra Tinybird workflow" currently assumes GH_TOKEN is available and will attempt the remote dispatch even when secrets.CANARY_DOCKER_BUILD is unset (empty) on fork PRs; update the step to check GH_TOKEN before doing the dispatch and skip/exit 0 when it's empty (e.g., add an early guard in the run script that tests GH_TOKEN/env.GH_TOKEN or otherwise change the step's condition to require a non-empty GH_TOKEN), referencing the GH_TOKEN env var to avoid invoking the remote-dispatch path with an empty token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 763-779: The current run lookup using ghost_ref='${{ github.ref
}}' and sorting by createdAt/last is ambiguous; instead pass and pin the exact
triggering SHA (use github.sha) or a unique caller_token when dispatching (e.g.,
include ghost_revision=github.sha or caller_token) and then change the gh run
list query to filter on that token/SHA (match the run name/field you surface)
rather than using sort_by(.createdAt) | last; update the dispatch arguments
where ghost_ref is set and the lookup that builds RUN_ID/RUN_URL (the gh
workflow run invocation and the jq/select in the RUN_ID assignment) to use the
new pinned field for deterministic correlation.
- Around line 751-756: The step "Trigger and watch traffic analytics infra
Tinybird workflow" currently assumes GH_TOKEN is available and will attempt the
remote dispatch even when secrets.CANARY_DOCKER_BUILD is unset (empty) on fork
PRs; update the step to check GH_TOKEN before doing the dispatch and skip/exit 0
when it's empty (e.g., add an early guard in the run script that tests
GH_TOKEN/env.GH_TOKEN or otherwise change the step's condition to require a
non-empty GH_TOKEN), referencing the GH_TOKEN env var to avoid invoking the
remote-dispatch path with an empty token.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 962900e1-e52b-4282-aa5b-91a1c16b71d1
📒 Files selected for processing (2)
.github/workflows/ci.ymlghost/core/core/server/data/tinybird/README.md
✅ Files skipped from review due to trivial changes (1)
- ghost/core/core/server/data/tinybird/README.md
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
761-779:⚠️ Potential issue | 🔴 CriticalCorrelate the exact remote run instead of “latest since timestamp”.
github.refis a moving branch/PR ref, whilegithub.shais the immutable revision that triggered this workflow; onpull_requestruns that SHA is the merge commit this job is already testing. Using the moving ref here and then selectinglast(createdAt)makes it easy to watch the wrong infra run when another dispatch lands nearby. Usegithub.shaplus a per-attempt caller token such as${{ github.run_id }}-${{ github.run_attempt }}, and match the remote run on that token instead of creation time. (docs.github.com)🧭 Minimal change on this side
+ CALLER_TOKEN='${{ github.run_id }}-${{ github.run_attempt }}' + gh workflow run "$WORKFLOW" \ --repo "$REPO" \ --ref "$BRANCH" \ - -f ghost_ref='${{ github.ref }}' \ + -f ghost_ref='${{ github.sha }}' \ + -f caller_token="$CALLER_TOKEN" \ -f run_local_tests='false'The infra workflow then needs to surface
caller_tokenin a queryable field, and thegh run listfilter should match that value instead ofsort_by(.createdAt) | last.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 761 - 779, Replace the fragile timestamp-based correlation with an explicit caller token: when invoking gh workflow run (the block using DISPATCHED_AT and gh workflow run), pass a caller_token built from the immutable trigger SHA and per-attempt identity (e.g., github.sha + github.run_id/github.run_attempt) via -f caller_token='...'; update the infra workflow to expose that caller_token on the dispatched run so it is queryable, and change the gh run list invocation (the command assigning RUN_ID via gh run list and its --jq filter) to select the run by the caller_token field rather than using createdAt/sort_by(... ) so you always match the exact remote run for the requested SHA/attempt.
751-754:⚠️ Potential issue | 🟠 MajorSkip this dispatch on fork PRs.
On
pull_requestruns from forks, GitHub sends the event to the base repository, sogithub.repositorystill resolves toTryGhost/Ghost, but repository secrets other thanGITHUB_TOKENare not passed to the runner. That means this step still runs without a usableCANARY_DOCKER_BUILDtoken and will fail for external PRs that touch Tinybird. (docs.github.com)🔒 Suggested guard
- if: github.repository == 'TryGhost/Ghost' + if: > + github.repository == 'TryGhost/Ghost' && + ( + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 751 - 754, The step "Trigger and watch traffic analytics infra Tinybird workflow" runs on fork PRs because github.repository stays 'TryGhost/Ghost' but secrets are unavailable; update the step's if condition to also guard against forked PRs by requiring either the event is not a pull_request or the PR head repo is not a fork. Concretely, replace the current if with a compound check such as: github.repository == 'TryGhost/Ghost' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false) so the step is skipped for forked pull requests while still running for native PRs and other events.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 761-779: Replace the fragile timestamp-based correlation with an
explicit caller token: when invoking gh workflow run (the block using
DISPATCHED_AT and gh workflow run), pass a caller_token built from the immutable
trigger SHA and per-attempt identity (e.g., github.sha +
github.run_id/github.run_attempt) via -f caller_token='...'; update the infra
workflow to expose that caller_token on the dispatched run so it is queryable,
and change the gh run list invocation (the command assigning RUN_ID via gh run
list and its --jq filter) to select the run by the caller_token field rather
than using createdAt/sort_by(... ) so you always match the exact remote run for
the requested SHA/attempt.
- Around line 751-754: The step "Trigger and watch traffic analytics infra
Tinybird workflow" runs on fork PRs because github.repository stays
'TryGhost/Ghost' but secrets are unavailable; update the step's if condition to
also guard against forked PRs by requiring either the event is not a
pull_request or the PR head repo is not a fork. Concretely, replace the current
if with a compound check such as: github.repository == 'TryGhost/Ghost' &&
(github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork
== false) so the step is skipped for forked pull requests while still running
for native PRs and other events.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 982df633-0995-4bbb-9021-17162c69ca6c
📒 Files selected for processing (2)
.github/workflows/ci.ymlghost/core/core/server/data/tinybird/tests/api_kpis.yaml
✅ Files skipped from review due to trivial changes (1)
- ghost/core/core/server/data/tinybird/tests/api_kpis.yaml
JoeeGrigg
force-pushed
the
tinybird-ci
branch
3 times, most recently
from
6bc0e43 to
cec32ea
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
751-755:⚠️ Potential issue | 🟠 MajorSkip this dispatch on fork PRs.
pull_requestevents from forks are sent to the base repository, and Actions secrets are not passed to those runs. Thisgithub.repository == 'TryGhost/Ghost'check therefore still matches external PRs whileTRAFFIC_ANALYTICS_GITHUB_TOKENis unavailable. (docs.github.com)Suggested guard
- if: github.repository == 'TryGhost/Ghost' + if: > + github.repository == 'TryGhost/Ghost' && + ( + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 751 - 755, The current step "Trigger and watch traffic analytics infra Tinybird workflow" relies on TRAFFIC_ANALYTICS_GITHUB_TOKEN but only checks github.repository, so forked PRs still match and run without secrets; update the step's if condition to also ensure the PR originates from the same repository (e.g. add a guard like github.event.pull_request.head.repo.full_name == github.repository) so the run is skipped for forked PRs where TRAFFIC_ANALYTICS_GITHUB_TOKEN is not available; keep the existing env usage (TRAFFIC_ANALYTICS_GITHUB_TOKEN) and ensure the combined conditional prevents external PRs from entering this step.
761-779:⚠️ Potential issue | 🔴 CriticalCorrelate this to the exact dispatched infra run.
This still sends a moving Ghost ref and then selects
last(createdAt)from allworkflow_dispatchruns onmain. Under concurrent pushes or PR updates, that can mirror the wrong remote run, and the runner-localDISPATCHED_ATcutoff can also miss the intended run because it depends on clock alignment.github.refis a branch/tag ref onpushand the syntheticrefs/pull/<n>/mergeref onpull_request, whilegithub.shais the commit SHA for the current workflow run andgithub.run_idis unique per workflow run. Please dispatch the immutable SHA plus a caller token, then match on that token once the infra workflow exposes it. (docs.github.com)Possible starting point
gh workflow run "$WORKFLOW" \ --repo "$REPO" \ --ref "$BRANCH" \ - -f ghost_ref='${{ github.ref }}' \ + -f ghost_ref='${{ github.sha }}' \ + -f caller_run_id='${{ github.run_id }}' \ -f run_local_tests='false'The polling side still needs a companion change in
TryGhost/traffic-analytics-infraso it can filter oncaller_run_idinstead oflast(createdAt).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 761 - 779, Replace the moving ghost ref / time-based matching with an immutable caller token: when invoking gh workflow run, pass the commit SHA and the caller run id as explicit inputs (e.g., add -f caller_sha='${{ github.sha }}' and -f caller_run_id='${{ github.run_id }}' to the gh workflow run invocation), and then change the polling logic that sets RUN_ID / RUN_URL (the gh run list call inside the for loop that currently uses DISPATCHED_AT and createdAt sorting) to filter for the returned infra run whose exposed input caller_run_id equals the caller token (match .inputs.caller_run_id or the workflow's exposed field) and take that run's databaseId as RUN_ID; keep DISPATCHED_AT only if you need it for logging but do not rely on it to select the run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 751-755: The current step "Trigger and watch traffic analytics
infra Tinybird workflow" relies on TRAFFIC_ANALYTICS_GITHUB_TOKEN but only
checks github.repository, so forked PRs still match and run without secrets;
update the step's if condition to also ensure the PR originates from the same
repository (e.g. add a guard like github.event.pull_request.head.repo.full_name
== github.repository) so the run is skipped for forked PRs where
TRAFFIC_ANALYTICS_GITHUB_TOKEN is not available; keep the existing env usage
(TRAFFIC_ANALYTICS_GITHUB_TOKEN) and ensure the combined conditional prevents
external PRs from entering this step.
- Around line 761-779: Replace the moving ghost ref / time-based matching with
an immutable caller token: when invoking gh workflow run, pass the commit SHA
and the caller run id as explicit inputs (e.g., add -f caller_sha='${{
github.sha }}' and -f caller_run_id='${{ github.run_id }}' to the gh workflow
run invocation), and then change the polling logic that sets RUN_ID / RUN_URL
(the gh run list call inside the for loop that currently uses DISPATCHED_AT and
createdAt sorting) to filter for the returned infra run whose exposed input
caller_run_id equals the caller token (match .inputs.caller_run_id or the
workflow's exposed field) and take that run's databaseId as RUN_ID; keep
DISPATCHED_AT only if you need it for logging but do not rely on it to select
the run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6614fb5b-b988-4d5b-9ba1-072e4abe809c
📒 Files selected for processing (3)
.github/workflows/ci.ymlghost/core/core/server/data/tinybird/connections/test.connectionghost/core/core/server/data/tinybird/tests/api_kpis.yaml
✅ Files skipped from review due to trivial changes (2)
- ghost/core/core/server/data/tinybird/tests/api_kpis.yaml
- ghost/core/core/server/data/tinybird/connections/test.connection
Ghost CI now triggers & waits for the dispatched traffic analytics workflow and mirrors its final result.
JoeeGrigg
force-pushed
the
tinybird-ci
branch
3 times, most recently
from
b8d582f to
1602391
Compare
- Fixed moving git ref given to external checks - Swicthed to gaining external workflow run ID direct from API - Started passing this repos workflow run ID to the external run for tracking purposes
|
cmraible
left a comment
cmraible
left a comment
There was a problem hiding this comment.
PR looks good! Have you seen these checks fail at all? I just want to make sure if they do fail, that it's reported properly here and prevents merging.
Should be able to test this by adding some gibberish to the other repo and triggering this workflow again.
Yep I tested it last week, before making the tests pass I first tested it failing and seems to work well. I'll get this merged 🎉 |
2 participants
Ghost CI now triggers & waits for the dispatched traffic analytics workflow and mirrors its final result.
This moves the Tinybird deploy check out of this repo, over to an internally managed repo.