ci: swap Docker trigger-workflow-and-wait for JS workflow_dispatch#64
Conversation
convictional/trigger-workflow-and-wait is a Docker container action; on the mini runner there is no Docker daemon, so building its image fails. Replace it with the-actions-org/workflow-dispatch (node20) in both dispatch composite actions. Same wait/fail semantics; the downstream run id is fed to ecosystem-ci-result unchanged.
…e24) node20 is being deprecated and the-actions-org/workflow-dispatch is node20. Use the Codex- two-step (node24): return-dispatch dispatches and returns the run id, await-remote-run polls it to completion. return-dispatch identifies the run by a distinct_id echoed in a step name, so the downstream workflow gains a distinct_id input plus a tiny ubuntu-latest echo job (wired on rspack-ecosystem-ci-from-pr.yml for verification; roll out to the rest once green).
📝 WalkthroughWalkthroughChangesThis pull request migrates two composite GitHub Actions (ecosystem_ci_dispatch and ecosystem_ci_per_commit) from using convictional/trigger-workflow-and-wait to a two-step dispatch-and-await flow using Codex-/return-dispatch and Codex-/await-remote-run. Output declarations (workflow-id, workflow-url, conclusion) are rewired to reference the new steps, and the workflow-output payload passed to result-collection steps now contains only the dispatched run_id instead of the full previous step outputs. Additionally, the rspack-ecosystem-ci-from-pr workflow gains a new optional distinct_id input and a return-dispatch-id job that echoes it. Sequence Diagram(s)Not applicable — no user-visible diagram required beyond the hidden artifact. Estimated code review effort: 3/5 (Moderate — CI/workflow wiring changes across three files requiring verification of output references and dispatch/await semantics) Related issues: None specified Related PRs: None specified Suggested labels: ci, github-actions Suggested reviewers: None specified 🐰 A rabbit dispatched a run into the night, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/actions/ecosystem_ci_dispatch/action.yaml (1)
117-139: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider guarding the await step on dispatch success.
If
dispatchfails (continue-on-error swallows it),eco_cistill runs with an emptyrun_id, producing a confusing failure inawait-remote-runbefore falling through toeco-ci-result's "cannot determine workflow run id" fallback. Guarding withif: steps.dispatch.outcome == 'success'would make the failure path cleaner.🤖 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/actions/ecosystem_ci_dispatch/action.yaml around lines 117 - 139, The Await Ecosystem CI step still runs even when Dispatch Ecosystem CI fails because continue-on-error hides the failure, leaving eco_ci with an empty run_id. Update the action in ecosystem_ci_dispatch so the await-remote-run step is guarded with the dispatch outcome (for example, only run when steps.dispatch indicates success), using the existing dispatch and eco_ci step ids to keep the failure path clean and avoid the fallback error from eco-ci-result.
🤖 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/actions/ecosystem_ci_dispatch/action.yaml:
- Around line 117-128: The Dispatch Ecosystem CI action is hardcoded to a
temporary branch on the dispatch target ref, which breaks the auto-propagation
bridge. Update the dispatch step in ecosystem_ci_dispatch/action.yaml so the ref
stays on main, matching the sibling ecosystem_ci_per_commit/action.yaml and the
SHA/ref policy, and do not parameterize or override that ref in the
trigger-workflow-and-wait flow.
In @.github/workflows/rspack-ecosystem-ci-from-pr.yml:
- Around line 64-73: Add an explicit permissions block to the return-dispatch-id
job so it does not inherit the workflow’s default token scope. Update the job
definition for return-dispatch-id in rspack-ecosystem-ci-from-pr.yml to set
least-privilege permissions appropriate for a step that only echoes DISTINCT_ID,
and keep the change localized to that job.
---
Nitpick comments:
In @.github/actions/ecosystem_ci_dispatch/action.yaml:
- Around line 117-139: The Await Ecosystem CI step still runs even when Dispatch
Ecosystem CI fails because continue-on-error hides the failure, leaving eco_ci
with an empty run_id. Update the action in ecosystem_ci_dispatch so the
await-remote-run step is guarded with the dispatch outcome (for example, only
run when steps.dispatch indicates success), using the existing dispatch and
eco_ci step ids to keep the failure path clean and avoid the fallback error from
eco-ci-result.
🪄 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: CHILL
Plan: Pro Plus
Run ID: c0b5b8aa-7e5d-48aa-919c-8fb7cf398de3
📒 Files selected for processing (3)
.github/actions/ecosystem_ci_dispatch/action.yaml.github/actions/ecosystem_ci_per_commit/action.yaml.github/workflows/rspack-ecosystem-ci-from-pr.yml
| - id: dispatch | ||
| name: Dispatch Ecosystem CI | ||
| uses: Codex-/return-dispatch@16fa9d14771c4d56ae0196bbda1d3c17f7f3650f # v3.0.2 | ||
| continue-on-error: true | ||
| with: | ||
| token: ${{ inputs.github-token }} | ||
| ref: ci/dockerless-workflow-dispatch | ||
| owner: rstackjs | ||
| repo: rstack-ecosystem-ci | ||
| workflow: ${{ inputs.workflow-file }} | ||
| workflow_inputs: ${{ inputs.client-payload }} | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Temporary branch pin on ref breaks the auto-propagation bridge.
ref: ci/dockerless-workflow-dispatch hardcodes the dispatch target to a dev branch instead of main. Per the SHA/ref policy this field is explicitly meant to remain main — pointing it elsewhere breaks the mechanism that lets the third bucket auto-propagate, and diverges from the sibling ecosystem_ci_per_commit/action.yaml, which correctly keeps ref: 'main' (line 53). This is already called out as WIP in the commit history, but as staged it would ship a broken/diverged config.
🔧 Proposed fix
with:
token: ${{ inputs.github-token }}
- ref: ci/dockerless-workflow-dispatch
+ ref: main
owner: rstackjsAs per coding guidelines, "Do not parameterize ref: main in trigger-workflow-and-wait — it is the bridge that makes the third bucket auto-propagate" and "When fixing a bug or adding a feature to one stack's workflows or shared actions, check whether the same issue or gap exists in the other stacks and apply the fix uniformly."
📝 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.
| - id: dispatch | |
| name: Dispatch Ecosystem CI | |
| uses: Codex-/return-dispatch@16fa9d14771c4d56ae0196bbda1d3c17f7f3650f # v3.0.2 | |
| continue-on-error: true | |
| with: | |
| token: ${{ inputs.github-token }} | |
| ref: ci/dockerless-workflow-dispatch | |
| owner: rstackjs | |
| repo: rstack-ecosystem-ci | |
| workflow: ${{ inputs.workflow-file }} | |
| workflow_inputs: ${{ inputs.client-payload }} | |
| - id: dispatch | |
| name: Dispatch Ecosystem CI | |
| uses: Codex-/return-dispatch@16fa9d14771c4d56ae0196bbda1d3c17f7f3650f # v3.0.2 | |
| continue-on-error: true | |
| with: | |
| token: ${{ inputs.github-token }} | |
| ref: main | |
| owner: rstackjs | |
| repo: rstack-ecosystem-ci | |
| workflow: ${{ inputs.workflow-file }} | |
| workflow_inputs: ${{ inputs.client-payload }} |
🤖 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/actions/ecosystem_ci_dispatch/action.yaml around lines 117 - 128,
The Dispatch Ecosystem CI action is hardcoded to a temporary branch on the
dispatch target ref, which breaks the auto-propagation bridge. Update the
dispatch step in ecosystem_ci_dispatch/action.yaml so the ref stays on main,
matching the sibling ecosystem_ci_per_commit/action.yaml and the SHA/ref policy,
and do not parameterize or override that ref in the trigger-workflow-and-wait
flow.
Source: Coding guidelines
| jobs: | ||
| return-dispatch-id: | ||
| name: Return Dispatch ID | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Return Dispatch ID ${{ inputs.distinct_id }} | ||
| run: echo "${DISTINCT_ID}" | ||
| env: | ||
| DISTINCT_ID: ${{ inputs.distinct_id }} | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add explicit least-privilege permissions to the new job.
zizmor flags this job for using default (overly broad) permissions since no permissions: block is declared. This job only needs to echo a string, so it should not inherit the workflow's default token scope.
🔒 Proposed fix
return-dispatch-id:
name: Return Dispatch ID
runs-on: ubuntu-latest
+ permissions: {}
steps:
- name: Return Dispatch ID ${{ inputs.distinct_id }}📝 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.
| jobs: | |
| return-dispatch-id: | |
| name: Return Dispatch ID | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Return Dispatch ID ${{ inputs.distinct_id }} | |
| run: echo "${DISTINCT_ID}" | |
| env: | |
| DISTINCT_ID: ${{ inputs.distinct_id }} | |
| jobs: | |
| return-dispatch-id: | |
| name: Return Dispatch ID | |
| runs-on: ubuntu-latest | |
| permissions: {} | |
| steps: | |
| - name: Return Dispatch ID ${{ inputs.distinct_id }} | |
| run: echo "${DISTINCT_ID}" | |
| env: | |
| DISTINCT_ID: ${{ inputs.distinct_id }} |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 65-72: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 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/rspack-ecosystem-ci-from-pr.yml around lines 64 - 73, Add
an explicit permissions block to the return-dispatch-id job so it does not
inherit the workflow’s default token scope. Update the job definition for
return-dispatch-id in rspack-ecosystem-ci-from-pr.yml to set least-privilege
permissions appropriate for a step that only echoes DISTINCT_ID, and keep the
change localized to that job.
Source: Linters/SAST tools
|
After removing the dependency on Docker in this PR, what are the next steps? Is simply removing the Docker dependency our ultimate goal? |
|
Later, I will change the runner from a GitHub runner to a self-hosted mini runner to save GH runner resources. |
Why
convictional/trigger-workflow-and-waitis a Docker action, so the dispatch step only runs on Docker-capable Linux runners. Replacing it with the JS-basedCodex-/return-dispatch+Codex-/await-remote-runpair makes the dispatch dockerless (runs on any runner, node24) and returns the downstreamrun_iddirectly.What
ecosystem_ci_dispatch(from-pr) andecosystem_ci_per_commit(from-commit): split the single trigger step into Dispatch (return-dispatch) + Await (await-remote-run), and feed the resultingrun_idintoecosystem-ci-resultand the action outputs.rspack-ecosystem-ci-from-pr.yml: add adistinct_idinput and areturn-dispatch-idjob thatreturn-dispatchneeds to correlate the dispatched run.TODO before ready
ecosystem_ci_dispatch'sreturn-dispatchref:is pinned toci/dockerless-workflow-dispatchfor verification (commitc741cf0) — revert tomainbefore finalize.distinct_idinput +return-dispatch-idjob to the other five stacks' from-pr workflows and all from-commit workflows (cross-stack isomorphism).ecosystem_ci_dispatch/**+ecosystem_ci_per_commit/**→ run the SHA Pin release + consumer pin bump per policy.