fix(ci): replace broken --affected check with task validation for integration tests#8078
fix(ci): replace broken --affected check with task validation for integration tests#8078jacekradko wants to merge 7 commits intomainfrom
Conversation
Turbo's --affected flag is a no-op for root-level tasks (//#) — they are always returned as affected regardless of what files changed. The previous check only 'worked' by silently swallowing errors when a task was missing from turbo.json (2>/dev/null + jq returning empty string). Replace with explicit task validation that fails loudly if the turbo task doesn't exist or returns 0 tasks.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: bd651bf The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe CI workflow 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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: 1
🤖 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 375-383: Replace the current assignment + manual exit check with
an if-not capture that keeps stderr out of the JSON stream: run pnpm turbo via
"if ! TURBO_OUTPUT=$(pnpm turbo run \"$TASK_NAME\" --dry=json 2>turbo_err); then
echo \"::error::Turbo task '$TASK_NAME' failed validation\"; cat turbo_err; exit
1; fi" so stdout (TURBO_OUTPUT) remains pure JSON for the subsequent
TASK_COUNT=$(echo "$TURBO_OUTPUT" | jq '.tasks | length') check and stderr is
preserved in turbo_err for logging; reference variables/commands TURBO_OUTPUT,
TASK_NAME, TASK_COUNT, pnpm turbo run and jq when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: bcd6b03b-2a6e-49ca-bc9c-10ca10096e6a
📒 Files selected for processing (1)
.github/workflows/ci.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 370-389: The jq invocation that computes TASK_COUNT (currently:
TASK_COUNT=$(echo "$TURBO_JSON" | jq '.tasks | length')) can mis-handle
missing/null tasks or invalid JSON; change the check to use jq -er -r to force
errors on null/false and emit raw integers, e.g., run jq -er -r '.tasks |
length' against TURBO_JSON (and check jq's exit status) so failures from invalid
JSON or missing .tasks produce a clear error (log TURBO_STDERR/TURBO_JSON
contents and exit non-zero), and only when jq succeeds parse TASK_COUNT as an
integer for the zero-task check; keep references to TASK_NAME, TURBO_STDERR,
TURBO_JSON and TASK_COUNT when adding the error messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23f1e9c4-1d88-43e7-b5a0-9cf534a612fd
📒 Files selected for processing (1)
.github/workflows/ci.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 388-394: The workflow currently sets "affected=1" unconditionally
after verifying TASK_COUNT, which only proves the task exists; change the logic
in the task-status step to set GITHUB_OUTPUT affected=1 only when the task is
actually affected by the PR (not just present in the graph). Concretely, use the
variable or check that indicates whether the task is in the affected set (e.g.,
the Turbo/affected output or a computed AFFECTED_TASKS list) instead of
TASK_COUNT/TASK_NAME alone, and write "affected=1" only when that check is true,
otherwise write "affected=0" so downstream if:
steps.task-status.outputs.affected == '1' filters correctly. Ensure you update
the code that currently writes echo "affected=1" >> $GITHUB_OUTPUT to be
conditional on the actual affected-check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 943615a6-3a83-48fa-aa50-2580017c132b
📒 Files selected for processing (1)
.github/workflows/ci.yml
.github/workflows/ci.yml
Outdated
| if [ "$TASK_COUNT" -eq 0 ]; then | ||
| echo "::error::Turbo task '$TASK_NAME' returned 0 tasks" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "affected=${AFFECTED}" | ||
| echo "affected=${AFFECTED}" >> $GITHUB_OUTPUT | ||
| echo "Task '$TASK_NAME' validated ($TASK_COUNT tasks in graph)" | ||
| echo "affected=1" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
affected is now effectively always true.
TASK_COUNT here only proves that the task exists and has a non-empty dry-run graph. Since Line 394 unconditionally writes affected=1, the later if: steps.task-status.outputs.affected == '1' guards no longer filter anything, so every defined integration suite will run even on unrelated-file PRs. That looks like a regression from the intended skip-on-unrelated-changes behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 388 - 394, The workflow currently sets
"affected=1" unconditionally after verifying TASK_COUNT, which only proves the
task exists; change the logic in the task-status step to set GITHUB_OUTPUT
affected=1 only when the task is actually affected by the PR (not just present
in the graph). Concretely, use the variable or check that indicates whether the
task is in the affected set (e.g., the Turbo/affected output or a computed
AFFECTED_TASKS list) instead of TASK_COUNT/TASK_NAME alone, and write
"affected=1" only when that check is true, otherwise write "affected=0" so
downstream if: steps.task-status.outputs.affected == '1' filters correctly.
Ensure you update the code that currently writes echo "affected=1" >>
$GITHUB_OUTPUT to be conditional on the actual affected-check.
The validation step is now a gate — if it fails, the job stops. No need for an output variable and downstream if-conditions since integration tests always run (root-level turbo tasks bypass --affected filtering).
1 participant
Summary
Replaces the broken
--affectedcheck in the integration tests CI job with explicit task validation that fails loudly on misconfiguration.Problem
The previous check used
turbo run --affected --dry=jsonto decide whether to run each integration test. Investigation revealed this was a no-op://#) bypass--affectedfiltering entirely. Even with zero diff from main,turbo ls --affectedreports 0 packages butturbo run test:integration:X --affected --drystill returns the root task. All integration tests are root tasks.affected=0occurred was when a turbo task was missing fromturbo.json. This is a configuration error that should fail loudly, not silently skip the test.2>/dev/nullswallowed turbo errors,jqreturned empty string (not "0") on empty input, and the|| echo "0"fallback never fired because jq exited 0.This is how the
cache-componentstest was silently skipped for weeks — its turbo task was defined on an unmerged branch, and the CI check silently treated the missing task as "not affected."Fix
Replace the
--affectedcheck with explicit task validation:turbo run <task> --dry=jsonwithout--affected(since it's meaningless for root tasks)::error::annotation if the turbo command fails (missing task, bad config)::error::annotation if jq returns 0 tasksVerification
Tested locally:
Related
--affectedalready always returned root tasks)Summary by CodeRabbit
Chores
New Features
Chores