-
Notifications
You must be signed in to change notification settings - Fork 3
chore: fix-slackbot #427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: fix-slackbot #427
Conversation
|
WalkthroughGitHub Actions workflows consolidated into reusable composite actions: a unified Setup Project action replaces repetitive initialization steps; publish workflows refactored into publish-release and publish-beta actions with Slack/docs integration; Mend workflow removed. Node.js version bumped to 24 with corresponding dependency updates; new fork-specific CI workflow added. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Setup as Setup Project Action
participant Build as Nx Build
participant Test as Nx Test/Lint
participant Report as Playwright Report
CI->>Setup: Call with CODECOV_TOKEN
Setup->>Setup: pnpm setup & Node install
Setup->>Setup: Cache Playwright browsers
Setup->>Setup: Start Nx Cloud CI
Setup->>Setup: Install dependencies
Setup->>Build: Run nx affected build/typecheck
Setup->>Test: Run nx affected lint/test/e2e
Setup->>Report: Upload Playwright report
Report-->>CI: Complete
sequenceDiagram
participant Pub as Publish Workflow
participant Release as Publish Release Action
participant Beta as Publish Beta Action
participant Slack as Slack API
participant Docs as GitHub Pages
participant Pkg as pkg-pr-new
alt When changesets published
Pub->>Release: Call with publishedPackages, slackWebhook
Release->>Release: Format packages for Slack
Release->>Slack: Send notification
Release->>Docs: Deploy release API docs
else When not published
Pub->>Beta: Call Beta action
Beta->>Pkg: Create PR via pkg-pr-new publish
Beta->>Build: Run pnpm nx run-many build
Beta->>Beta: Generate API docs
Beta->>Docs: Deploy beta API docs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
|
View your CI Pipeline Execution ↗ for commit 0460874
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/publish.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (18.52%) is below the target coverage (40.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #427 +/- ##
==========================================
- Coverage 19.05% 18.52% -0.54%
==========================================
Files 138 138
Lines 27477 27401 -76
Branches 975 962 -13
==========================================
- Hits 5237 5075 -162
- Misses 22240 22326 +86 🚀 New features to boost your workflow:
|
@forgerock/davinci-client
@forgerock/oidc-client
@forgerock/protect
@forgerock/sdk-types
@forgerock/sdk-utilities
@forgerock/iframe-manager
@forgerock/sdk-logger
@forgerock/sdk-oidc
@forgerock/sdk-request-middleware
@forgerock/storage
commit: |
|
Deployed 948b6e4 to https://ForgeRock.github.io/ping-javascript-sdk/pr-427/948b6e4d0bb4ab371a1729bf3bedb040f61746b2 branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis🚨 Significant Changes🔻 @forgerock/journey-client - 0.0 KB (-82.0 KB, -100.0%) 📊 Minor Changes📉 @forgerock/oidc-client - 22.9 KB (-0.0 KB) ➖ No Changes➖ @forgerock/device-client - 9.2 KB 13 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/actions/publish-beta/action.yml (1)
4-7: Remove the deadbranchinput (or actually use it).The action exposes a
branchinput but never consumes it, so callers pass data that’s silently ignored. Please either wire the input into the action logic or drop it from the interface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/actions/publish-beta/action.yml(1 hunks).github/actions/publish-release/action.yml(1 hunks).github/actions/setup/action.yml(1 hunks).github/workflows/publish.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Setup Project |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be nothing different, i just refactored this to be more "clean" and use a composite action that can be reused across steps.
| env: | ||
| CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} | ||
|
|
||
| - run: git status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this b/c unneccessary
| - name: Publish Release Steps | ||
| if: steps.changesets.outputs.published == 'true' | ||
| uses: slackapi/slack-github-action@v2.1.1 | ||
| with: | ||
| payload-delimiter: '_' | ||
| webhook: ${{ secrets.SLACK_WEBHOOK_URL }} | ||
| webhook-type: webhook-trigger | ||
| payload: steps.changesets.outputs.publishedPackages | ||
|
|
||
| - uses: codecov/codecov-action@v5 | ||
| uses: ./.github/actions/publish-release | ||
| with: | ||
| files: ./packages/**/coverage/*.xml | ||
| token: ${{ secrets.CODECOV_TOKEN }} | ||
|
|
||
| - name: Ensure builds run | ||
| run: pnpm nx run-many -t build | ||
| env: | ||
| NX_CLOUD_DISTRIBUTED_EXECUTION: false | ||
| publishedPackages: ${{ steps.changesets.outputs.publishedPackages }} | ||
| slackWebhook: ${{ secrets.SLACK_WEBHOOK_URL }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broke this branching out into reusable composite actions
just to clean up this workflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/setup/action.yml(1 hunks).github/workflows/publish.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (6)
.github/actions/publish-beta/action.yml (3)
3-7: Remove emptyinputsblock or define real inputsAn empty
inputs:is noise. Either remove it or add explicit inputs (e.g.,token,docs-folder,branch) you plan to consume.
8-11: Ensure auth for PR creation (GITHUB_TOKEN) is wired
pnpm pkg-pr-new publish …will need a token to open PRs. Composite actions don’t auto-inject inputs; rely onGITHUB_TOKENfrom the caller job and ensurepermissions: contents: write, pull-requests: writeare set, or accept atokeninput and pass it as env.Example adjustment within this action:
inputs: + token: + description: 'GitHub token with contents & pull-requests write' + required: false steps: - name: Create PR for beta publish shell: bash - run: pnpm pkg-pr-new publish './packages/*' './packages/sdk-effects/*' --packageManager=pnpm --comment=off + run: pnpm pkg-pr-new publish './packages/*' './packages/sdk-effects/*' --packageManager=pnpm --comment=off + env: + GITHUB_TOKEN: ${{ inputs.token || env.GITHUB_TOKEN }}Optionally confirm the caller workflow sets:
- job permissions: contents: write, pull-requests: write
- env: GITHUB_TOKEN provided by GitHub
12-17: Pin action to a commit SHA and set target branch explicitlyFor supply‑chain safety, pin
JamesIves/github-pages-deploy-actionto a commit SHA. Also setbranchexplicitly (usuallygh-pages) to avoid environment drift.- - name: Publish api docs [beta] - uses: JamesIves/github-pages-deploy-action@v4.7.3 + - name: Publish api docs [beta] + uses: JamesIves/github-pages-deploy-action@9dc5b8d3b75f3cc1b1b5a7eaa63a6a5c9e1d2b48 # v4.7.3 with: - folder: docs + branch: gh-pages + folder: docs commit-message: 'chore: release-api-docs-beta' target-folder: 'beta'Also ensure the caller job has
pages: writeorcontents: writepermissions as required by the action and thatdocs/is built before invoking this step..github/actions/setup/action.yml (3)
22-26: Pin third‑party actions to commit SHAsPin all marketplace actions to immutable SHAs (checkout, pnpm/action-setup, setup-node, actions/cache, nrwl/nx-set-shas, upload-artifact) to reduce supply‑chain risk.
Example:
- actions/checkout@v4 → actions/checkout@b4ffde6
- pnpm/action-setup@v4 → pnpm/action-setup@
- actions/setup-node@v4 → actions/setup-node@
- actions/cache@v4 → actions/cache@
- nrwl/nx-set-shas@v4 → nrwl/nx-set-shas@
- actions/upload-artifact@v4 → actions/upload-artifact@
Also applies to: 27-29, 31-36, 56-62, 67-67, 73-80
37-39: Updating npm globally may be unnecessary and adds timeUnless you hit a specific npm bug, consider omitting
npm install -g npm@latestto speed up CI and keep npm aligned with the selected Node version.
63-66: Playwright install nuancesIf using GitHub-hosted runners, browsers are often preinstalled/restored via cache; consider skipping install for non-e2e jobs, or gate with a boolean input to reduce time. Use
--with-depsonly when needed on self-hosted Linux.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/actions/publish-beta/action.yml(1 hunks).github/actions/publish-release/action.yml(1 hunks).github/actions/setup/action.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/publish-release/action.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci-fork.yml (1)
1-45: ci-fork.yml is missing critical setup steps from the new Setup Project composite action.The workflow cannot directly use the composite action due to fork-specific requirements (head SHA checkout, git branch tracking for nx affected, custom nx tasks), but it is missing important setup optimizations present in
.github/actions/setup/action.yml:
- Nx Cloud initialization (
pnpm dlx nx-cloud start-ci-run...) — required for distributed testing and CI metrics- pnpm store-dir config — improves cache efficiency
- npm registry-url setup — ensures correct npm resolution
- npm upgrade to latest — maintains tool currency
- Playwright report artifact upload — enables debugging of failed e2e tests
The different
nx affectedtask list (ci-fork includestypecheckandformat:checkseparately) is appropriate for fork PRs, but the foundational setup gaps should be addressed.Consider backfilling these setup steps into ci-fork.yml or creating a fork-specific composite action that wraps the core setup with fork requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci-fork.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
4e1b118 to
4a81119
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/publish.yml (1)
81-91: Remove duplicate docs deployment; consolidate into composite action.Lines 87–91 deploy API docs to the beta target folder using JamesIves/github-pages-deploy-action. However, the same step already runs inside
.github/actions/publish-beta/action.ymlat lines 12–17.This duplication means docs are deployed twice, which is wasteful and undermines the refactoring goal of consolidating steps into reusable actions.
Remove lines 87–91 so the
publish-betaaction is the single source of truth for beta docs deployment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/actions/publish-beta/action.yml(1 hunks).github/actions/publish-release/action.yml(1 hunks).github/actions/setup/action.yml(1 hunks).github/workflows/ci-fork.yml(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/publish.yml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/actions/publish-release/action.yml
- .github/actions/setup/action.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/publish.yml
56-56: input "pnpm-cache-folder" is not defined in action "actions/setup-node@v5". available inputs are "always-auth", "architecture", "cache", "cache-dependency-path", "check-latest", "mirror", "mirror-token", "node-version", "node-version-file", "package-manager-cache", "registry-url", "scope", "token"
(action)
85-85: input "branch" is not defined in action "Publish Beta Steps" defined at "./.github/actions/publish-beta". available inputs are
(action)
125-125: input "ref" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "fetch-depth", "node-version-file", "pnpm-cache-folder", "token"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
🔇 Additional comments (3)
.github/workflows/ci.yml (1)
19-19: Good guard condition for non-forked PRs.The condition correctly gates the main PR CI job to only run on non-forked PRs, delegating fork CI to the new ci-fork.yml workflow. This is a clean separation of concerns.
.github/actions/publish-beta/action.yml (1)
1-17: Action definition is sound; note expected input mismatch.The composite action structure is correct. However, the empty
inputs:section (lines 3–4) will cause validation errors when publish.yml attempts to passbranchas an input at line 85. Either:
- Define the
branchinput in this action and use it in the steps, or- Remove the
branch:parameter from the publish.yml invocation (line 85).Currently, the action does not use a branch parameter, so removing it from publish.yml is likely the correct fix.
.github/workflows/ci-fork.yml (1)
1-57: Well-structured fork CI workflow with appropriate safeguards.The workflow correctly:
- Gates to forks only (line 17 condition inverts the main PR guard)
- Uses explicit PR head commit checkout (line 25) to avoid merge-commit issues
- Employs restore-only Playwright cache (line 42) to prevent permission errors on forks
- Includes defensive git branch tracking with fallback (line 54)
The matrix of build, test, and lint tasks aligns well with the main PR CI expectations.
2238e21 to
bfe0ff9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/actions/publish-beta/action.yml(1 hunks).github/actions/setup/action.yml(1 hunks).github/workflows/ci-fork.yml(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/publish.yml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/actions/setup/action.yml
- .github/workflows/ci.yml
🔇 Additional comments (8)
.github/actions/publish-beta/action.yml (1)
1-17: LGTM!This composite action correctly encapsulates the beta publish workflow. Both steps (PR creation and API docs deployment) are properly configured and align with how the action is consumed in the publish workflow.
.github/workflows/publish.yml (5)
39-49: Good permissions and checkout setup.The permissions enable the necessary write access for changesets to create version PRs and update git history. The checkout with
fetch-depth: 0and standard token are appropriate.
51-55: Good use of Setup Project composite action.Early invocation with correct parameters (fetch-depth for changesets history and GH_TOKEN for elevated permissions). This consolidates the setup logic into a reusable action.
71-80: Publish routing is clean and correct.The conditionals properly route to release vs. beta based on changesets output. Line 75 has the fixed interpolation syntax. No duplicate steps visible, and the publish-beta action is invoked without undefined inputs (matching its action definition).
82-96: Bundle size steps are unchanged and appropriate.Standard cleanup and artifact upload with reasonable retention policy.
110-125: Verify setup action supportsrefandpnpm-cache-folderinputs.Line 120 passes
ref: ${{ inputs.branch }}and line 123 passespnpm-cache-folder: .pnpm-storeto the.github/actions/setupcomposite action. A past review flaggedrefas undefined in this action and claimed a fix in commit 2238e21, but the setup action file is not provided to verify this.Ensure
.github/actions/setup/action.ymldeclares bothrefandpnpm-cache-folderas inputs and that the checkout step inside the composite action uses therefparameter (checkout actions requirerefto be passed via theusesstep's inputs, not automatically forwarded)..github/workflows/ci-fork.yml (2)
1-19: Well-structured workflow setup with proper fork detection and security controls.The workflow metadata, permissions scoping, concurrency configuration, and fork detection logic all follow GitHub Actions best practices. The 20-minute timeout is reasonable for this scope of CI work.
22-48: Proper environment setup with appropriate caching strategy for forks.The checkout, Node/pnpm setup, and Playwright configuration all follow best practices. The restore-only cache strategy for Playwright is particularly well-suited to fork environments to avoid unnecessary cache save attempts and noise.
.github/workflows/ci-fork.yml
Outdated
| - uses: nrwl/nx-set-shas@v4 | ||
|
|
||
| # Needed so nx affected can diff against main | ||
| - run: git branch --track main origin/main || true | ||
|
|
||
| - run: pnpm nx format:check | ||
| - run: pnpm nx affected -t build typecheck lint test e2e-ci |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify Nx affected command syntax for multiple targets.
Line 56 uses pnpm nx affected -t build typecheck lint test e2e-ci, which may not execute all specified targets correctly. Standard Nx syntax typically requires either repeating the -t flag for each target or using run-many.
Once you verify the correct syntax, it should be updated to something like:
pnpm nx affected run-many -t build -t typecheck -t lint -t test -t e2e-cior
pnpm nx affected -t build -t typecheck -t lint -t test -t e2e-ci🌐 Web query:
Nx affected command syntax for running multiple targets
💡 Result:
Use the --target (or -t) option with a comma-separated list of targets. Example:
- nx affected --target=lint,test,build
- nx affected -t=lint,test,build
(You can also combine with other options like --base/--head or --parallel.) [1][2]
Sources:
[1] Nx — Run Tasks documentation.
[2] Nx Affected guide (examples showing --target=lint,test,build).
Fix Nx affected command syntax: use comma-separated targets.
Line 56 uses pnpm nx affected -t build typecheck lint test e2e-ci, but targets should be comma-separated with a single -t option. Update to:
pnpm nx affected -t build,typecheck,lint,test,e2e-ci🤖 Prompt for AI Agents
In .github/workflows/ci-fork.yml around lines 50 to 56, the pnpm nx affected
invocation is using separate target arguments instead of a single -t with
comma-separated targets; update the command to use one -t flag and join the
targets with commas (e.g. replace `pnpm nx affected -t build typecheck lint test
e2e-ci` with `pnpm nx affected -t build,typecheck,lint,test,e2e-ci`) so Nx
parses the targets correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think this matters. nx supports space delimiters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Nx affected command multiple targets space separated syntax
💡 Result:
Use a comma-separated list (no spaces). Example:
npx nx affected --target=lint,test,build
This runs the lint, test and build targets for affected projects. [1][2]
Sources:
[1] Nx affected guide (examples showing --target=lint,test,build).
[2] Nx blog / docs (examples using --target=test and multiple affected commands).
🌐 Web query:
Nx affected -t flag space separated targets 2024 2025
💡 Result:
Short answer: Nx accepts multiple targets with the short -t flag as space-separated tokens (e.g. nx affected -t lint test build) and also via the long form --targets with comma-separated values (e.g. --targets=lint,test,build). See Nx docs and examples. [1][2][3]
Sources:
- Nx blog / examples showing "nx affected -t lint test build". [1]
- Nx docs "Run Tasks" examples including "npx nx affected -t test" and run-many with -t multiple targets. [2]
- Nx Cloud docs showing use of comma-separated target lists for CI (--stop-agents-after=build,test,e2e) and examples with --stop-agents-after and --targets. [3]
@ryanbas21, you're absolutely right! I apologize for the confusion. The latest Nx documentation confirms that the -t flag does support space-separated targets, so your syntax pnpm nx affected -t build typecheck lint test e2e-ci is correct.
Nx supports both patterns:
- Space-separated with the short flag:
nx affected -t lint test build - Comma-separated with the long form:
nx affected --targets=lint,test,build
Thank you for the clarification!
✏️ Learnings added
Learnt from: ryanbas21
PR: ForgeRock/ping-javascript-sdk#427
File: .github/workflows/ci-fork.yml:50-56
Timestamp: 2025-10-28T14:44:26.850Z
Learning: Nx CLI accepts multiple targets with the short `-t` flag as space-separated tokens (e.g., `nx affected -t lint test build`) and also supports comma-separated values with the long form `--targets=` (e.g., `nx affected --targets=lint,test,build`). Both syntaxes are valid.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
bfe0ff9 to
66011c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/actions/publish-beta/action.yml(1 hunks).github/actions/setup/action.yml(1 hunks).github/workflows/ci-fork.yml(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/publish.yml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/actions/setup/action.yml
- .github/workflows/ci.yml
- .github/actions/publish-beta/action.yml
- .github/workflows/ci-fork.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/publish.yml
54-54: input "fetch-depth" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "node-version-file", "pnpm-cache-folder"
(action)
55-55: input "token" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "node-version-file", "pnpm-cache-folder"
(action)
119-119: input "fetch-depth" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "node-version-file", "pnpm-cache-folder"
(action)
120-120: input "ref" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "node-version-file", "pnpm-cache-folder"
(action)
121-121: input "token" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "node-version-file", "pnpm-cache-folder"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
🔇 Additional comments (1)
.github/workflows/publish.yml (1)
51-55: Verify Setup Project action inputs are properly defined (publish-or-pr job).The workflow passes
fetch-depthandtokento the Setup Project action (lines 51–55), but static analysis indicates onlynode-version-fileandpnpm-cache-folderare defined as inputs. Ensure these inputs exist in.github/actions/setup/action.yml, or remove them from this invocation.If the above verification script shows that
fetch-depthandtokenare not defined in the action, apply this diff to remove the unsupported inputs:- name: Setup Project uses: ./.github/actions/setup with: - fetch-depth: 0 - token: ${{ secrets.GH_TOKEN }} + node-version-file: '.node-version' + pnpm-cache-folder: .pnpm-store
66011c1 to
fc51e02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/actions/publish-beta/action.yml(1 hunks).github/actions/setup/action.yml(1 hunks).github/workflows/ci-fork.yml(1 hunks).github/workflows/ci.yml(1 hunks).github/workflows/publish.yml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/ci-fork.yml
- .github/workflows/ci.yml
- .github/actions/setup/action.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/publish.yml
54-54: input "fetch-depth" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
55-55: input "token" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
118-118: missing input "CODECOV_TOKEN" which is required by action "Setup Project" defined at "./.github/actions/setup". all required inputs are "CODECOV_TOKEN"
(action)
120-120: input "fetch-depth" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
121-121: input "ref" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
122-122: input "token" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
🔇 Additional comments (3)
.github/actions/publish-beta/action.yml (1)
1-16: Composite action structure looks good.The publish-beta action correctly defines two sequential steps for beta publishing: creating a PR for beta packages and deploying API docs to the beta folder. The command and parameters appear correct.
.github/workflows/publish.yml (2)
39-43: Permissions properly scoped for publishing operations.The addition of
contents: writeandid-token: writeare appropriate for changesets versioning and potential artifact signing.
72-81: Composite action invocations properly structured.The conditional routing between Publish Release Steps (when published) and Publish Beta Steps (when not published) is correctly implemented. Each action receives only the inputs it defines.
Moves the "Setup pnpm config" step to before node setup for better ordering. Updates the CODECOV_TOKEN to use `secrets.CODECOV_TOKEN` instead of `env.CODECOV_TOKEN`. This improves security in the CI workflow.
This adds a new GitHub Actions workflow that runs on pull requests from forks. This workflow is designed to be secure by not using any secrets. It runs linting, building, and testing on the affected projects.
793ebd3 to
7139271
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/publish.yml (2)
51-55: Remove undefinedfetch-depthinput from Setup Project action.The Setup Project composite action only accepts
CODECOV_TOKEN,node-version-file, andpnpm-cache-folderas inputs. Thefetch-depth: 0parameter (line 54) is not defined in the action and will be silently ignored. This parameter is already correctly configured in thecheckoutaction (line 48).Apply this diff:
- name: Setup Project uses: ./.github/actions/setup with: - fetch-depth: 0 CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
116-125: Fix Setup Project inputs: move CODECOV_TOKEN towithblock and remove undefined parameters.This Setup Project invocation has multiple issues:
CODECOV_TOKENis in theenvblock (line 125) but must be in thewithblock as it's a required input- Lines 119-121 pass undefined inputs (
fetch-depth,ref,token) that the action doesn't accept—these are already handled by thecheckoutaction (lines 110-114)Apply this diff:
- name: Setup Project uses: ./.github/actions/setup with: - fetch-depth: 0 - ref: ${{ inputs.branch }} - token: ${{ secrets.GH_TOKEN }} node-version-file: '.node-version' pnpm-cache-folder: .pnpm-store - env: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/actions/publish-beta/action.yml(1 hunks).github/actions/publish-release/action.yml(1 hunks).github/actions/setup/action.yml(1 hunks).github/workflows/ci-fork.yml(1 hunks).github/workflows/ci.yml(2 hunks).github/workflows/mend.yml(0 hunks).github/workflows/publish.yml(3 hunks).node-version(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/mend.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/actions/publish-beta/action.yml
- .github/workflows/ci-fork.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/publish.yml
54-54: input "fetch-depth" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
117-117: missing input "CODECOV_TOKEN" which is required by action "Setup Project" defined at "./.github/actions/setup". all required inputs are "CODECOV_TOKEN"
(action)
119-119: input "fetch-depth" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
120-120: input "ref" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
121-121: input "token" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
🔇 Additional comments (9)
.node-version (1)
1-1: Node 24 upgrade looks good.The version bump aligns with the updated
@types/nodedependency and the Setup Project action's Node configuration..github/workflows/ci.yml (2)
19-19: Good addition: fork PR protection.The repository check prevents the workflow from running on forked PRs, avoiding potential issues with secrets and resource usage.
32-35: Workflow simplification achieved.The Setup Project composite action successfully consolidates what were previously ~30 lines of setup steps (pnpm setup, Node setup, dependency installation, Nx Cloud configuration, Playwright setup) into a single, reusable action call.
.github/workflows/publish.yml (1)
71-80: Workflow orchestration streamlined.The new composite actions (
publish-releaseandpublish-beta) effectively consolidate conditional publishing logic, Slack notifications, and docs deployment into reusable components..github/actions/publish-release/action.yml (2)
13-21: Multiline output handling is correct.The heredoc delimiter syntax (
EOF) properly captures multilinejqoutput without corruptingGITHUB_OUTPUT, addressing potential issues when multiple packages are published.
23-31: Slack Workflow Builder payload structure verified.The payload format with
publishedPackagesas a top-level key is correct for Slack Workflow Builder webhooks (webhook-trigger type), which expect custom variable names rather than thetext/blocksfields required by standard incoming webhooks.Based on past review comments.
.github/actions/setup/action.yml (3)
23-25: Correct sequencing: pnpm store configured before Node setup.Configuring the pnpm store directory with
--globalbeforeactions/setup-noderuns ensures the cache key is computed against the correct store path, enabling cache reuse across steps.Based on past review comments.
65-67: Note: Setup Project action automatically runs the full test suite.This composite action executes
nx affected -t build lint test e2e-cias part of setup. Any workflow invoking Setup Project will automatically run these steps, which consolidates CI logic but means the action is not purely setup—it includes the main CI tasks. Ensure workflows using this action don't need to customize or skip any of these targets.
41-45: Nx Cloud integration configured correctly.The CODECOV_TOKEN is properly passed as both an input to the composite action and as an environment variable to the Nx Cloud start command, ensuring distributed execution can access the token when needed.
bd993a2 to
942944d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/publish.yml (2)
51-55: Remove undefinedfetch-depthinput from Setup Project action.The
fetch-depthinput on line 54 is not defined in the Setup Project action, which only acceptsCODECOV_TOKEN,node-version-file, andpnpm-cache-folder. This parameter is already correctly handled by thecheckoutaction on lines 46-49 and will be silently ignored here, creating configuration confusion.Apply this diff:
- name: Setup Project uses: ./.github/actions/setup with: - fetch-depth: 0 CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}Based on static analysis hints.
116-125: Fix Setup Project action inputs: move CODECOV_TOKEN to with block and remove undefined inputs.The Setup Project action invocation has critical configuration errors:
- Missing required input:
CODECOV_TOKENis placed in theenv:block (line 125) but must be in thewith:block as a required input.- Undefined inputs: Lines 119-121 pass
fetch-depth,ref, andtokenwhich are not accepted by the Setup Project action. These are already handled by thecheckoutaction on lines 110-114.Apply this diff:
- name: Setup Project uses: ./.github/actions/setup with: - fetch-depth: 0 - ref: ${{ inputs.branch }} - token: ${{ secrets.GH_TOKEN }} node-version-file: '.node-version' pnpm-cache-folder: .pnpm-store + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} - env: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}Based on static analysis hints.
🧹 Nitpick comments (1)
.github/actions/setup/action.yml (1)
65-67: Consider separating test execution from setup action.The "Setup Project" action runs the full test suite (
nx affected -t build lint test e2e-ci) on line 66, which goes beyond typical setup responsibilities. This coupling makes the action less reusable and the name misleading.Consider refactoring to either:
- Rename the action to "Setup and Test Project" to reflect its actual scope, or
- Extract the test execution step into a separate action or workflow step, keeping this action focused purely on environment setup.
The current design works but reduces composability—callers cannot use this action for setup-only scenarios without also running tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.github/actions/setup/action.yml(1 hunks).github/workflows/ci-fork.yml(1 hunks).github/workflows/ci.yml(2 hunks).github/workflows/mend.yml(0 hunks).github/workflows/publish.yml(3 hunks).node-version(1 hunks)package.json(2 hunks)packages/protect/README.md(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/mend.yml
✅ Files skipped from review due to trivial changes (1)
- packages/protect/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/ci.yml
- .github/workflows/ci-fork.yml
- package.json
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/publish.yml
54-54: input "fetch-depth" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
117-117: missing input "CODECOV_TOKEN" which is required by action "Setup Project" defined at "./.github/actions/setup". all required inputs are "CODECOV_TOKEN"
(action)
119-119: input "fetch-depth" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
120-120: input "ref" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
121-121: input "token" is not defined in action "Setup Project" defined at "./.github/actions/setup". available inputs are "CODECOV_TOKEN", "node-version-file", "pnpm-cache-folder"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
🔇 Additional comments (1)
.node-version (1)
1-1: Node.js 24 is released and production-ready — version bump is valid.Node.js 24 was officially released on May 6, 2025 and entered Long-Term Support (LTS) in October 2025, confirming this is a stable, production-ready change.
942944d to
0d1e2ae
Compare
remove unused mend.yml
0d1e2ae to
48d92db
Compare
Add Zed tasks configuration and update publish workflow
ancheetah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good except for some steps appear to be missing, unless I missed something. Mostly nitpick comments/questions.
.github/workflows/ci-fork.yml
Outdated
| - uses: nrwl/nx-set-shas@v4 | ||
|
|
||
| # Needed so nx affected can diff against main | ||
| - run: git branch --track main origin/main || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sets up main (from the forked branch) to track origin/main, right? What if the forked branch in the PR is not called main?
Or is it that we set up the forked main to track origin/main so that when nx affected runs it runs against the forked main which should match origin/main?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the PR should run in the base repo always so this should be tracking origin/main of the base repo (this one). the code is from the "forked" repo but the code is run in the base repo.
@coderabbitai can you verify this is correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
GitHub Actions pull request from fork checkout origin remote
💡 Result:
Short answer — in a workflow triggered by a PR from a fork, actions/checkout will check out the fork ref and “origin” will point to the fork. To get the upstream (base) repository as a remote (commonly named upstream or origin), add it yourself and fetch the refs you need.
Minimal steps (example for PR number available in github.event.pull_request.number):
-
Checkout (keep full history if you need it):
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/checkout@v4
-
Add upstream remote and fetch the base repo:
- run: |
git remote add upstream https://github.com/OWNER/REPO.git
git fetch --no-tags --prune --no-recurse-submodules upstream +refs/heads/:refs/remotes/upstream/optional: fetch the PR head directly into a local branch
git fetch origin pull/${{ github.event.pull_request.number }}/head:pr-${{ github.event.pull_request.number }}
git checkout pr-${{ github.event.pull_request.number }}
- run: |
If you prefer to keep the upstream named origin (so PR fetches show as origin/pr/*), add the fetch refspec instead of or in addition to the remote-add:
- run: git config --add remote.origin.fetch '+refs/pull//head:refs/remotes/origin/pr/' && git fetch origin
References:
- actions/checkout docs (usage & fetch-depth). [1]
- GitHub docs: checking out pull requests locally (refs/pull/*/head fetch pattern). [2]
- Common fetch/refspec patterns for PRs (examples and gist). [3]
If you want, tell me the repository owner/name and I’ll produce a ready-to-use workflow snippet.
🌐 Web query:
nrwl nx-set-shas GitHub Actions fork pull request
💡 Result:
Short answer: nx-set-shas needs GitHub API read access to actions / pull-requests to determine base/head SHAs, so it will fail or be limited when a workflow runs for a pull request from a fork unless you change how the workflow runs (or provide a token with higher privileges). See details and options below.
Key points and options
- The nx-set-shas action uses the GitHub API and therefore requires actions/contents/pull-requests read permissions in the workflow. [1][3]
- When a PR comes from a public fork, workflows triggered by on: pull_request run with a GITHUB_TOKEN that is read-only and cannot access secrets or (in many org settings) cannot have write privileges — that prevents actions that need API reads/writes from working as they do for in-repo PRs. [2][4]
- Workarounds:
- Use pull_request_target (runs in context of base branch) — allows access to secrets and stronger GITHUB_TOKEN permissions, but is potentially unsafe if you check out and run untrusted fork code. Use only for safe operations (e.g., computing SHAs without checking out PR code) and follow GitHub guidance. [4]
- Provide a PAT or fine‑grained token with appropriate scopes (actions/read, contents/read, pull-requests: read) as an input to the action (gh-token) and store it as a secret — this lets the action call the API for fork PRs. [1][2]
- Restrict workflows so they only run for non-fork PRs (if acceptable) by checking github.event.pull_request.head.repo.full_name == github.repository. [7]
References
- nrwl/nx-set-shas README (permissions, gh-token input). [1]
- GitHub Actions docs: restrictions for workflows on pull requests from forks and workflow permissions. [2][4]
If you want, I can draft a minimal workflow example showing one of the workarounds (pull_request_target or PAT) tailored to your repo.
@ryanbas21, I need to correct this understanding. The current setup is not correct for fork PRs.
The Issue
When GitHub Actions runs a workflow triggered by a pull request from a fork:
- The workflow runs in the base repository's runner, but
actions/checkoutclones the fork repository - After checkout,
originpoints to the fork, not the base repository - The command
git branch --track main origin/maincreates a localmainbranch tracking the fork's main, not the base repo's main
This means nx affected will compare against the fork's main branch, which may be outdated or diverged from the base repository's main.
The Fix
You need to explicitly add the base repository as a remote and track its main branch:
# After the checkout step
- name: Add upstream remote and track base main
run: |
git remote add upstream https://github.com/${{ github.repository }}.git
git fetch --no-tags --prune upstream main
git branch --force main upstream/main
git branch --set-upstream-to=upstream/main mainOr more simply:
- name: Track base repository main
run: |
git fetch origin +refs/heads/main:refs/remotes/upstream/main
git branch --force main upstream/mainNote on nx-set-shas
The nrwl/nx-set-shas@v4 action may also have permission issues with fork PRs, as it needs GitHub API access. If you encounter issues, you may need to provide a token with actions: read and contents: read permissions, or use pull_request_target (with caution).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ryanbas21 can we use head and base options to do the same thing?
https://nx.dev/docs/features/ci-features/affected#configure-affected-on-ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we run into the same issue if we use Head/ base here as it would be the fork
.github/workflows/ci.yml
Outdated
| permissions: | ||
| pull-requests: write | ||
| contents: write | ||
| id-token: write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this id-token permission in CI? We're not publishing here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 good call out.
.github/workflows/ci.yml
Outdated
| run: pnpm nx run-many -t build | ||
| env: | ||
| NX_CLOUD_DISTRIBUTED_EXECUTION: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add --no-agents here instead. NX_CLOUD_DISTRIBUTED_EXECUTION is deprecated.
.github/actions/setup/action.yml
Outdated
| run: pnpm exec playwright install | ||
| shell: bash | ||
|
|
||
| - uses: nrwl/nx-set-shas@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a name here too? Something like 'derive shas for nx'
.github/workflows/ci.yml
Outdated
| - name: Upload E2E reports on failure | ||
| if: failure() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: e2e-reports | ||
| path: | | ||
| e2e/*-suites/playwright-report/** | ||
| e2e/*-suites/test-results/** | ||
| e2e/*-suites/.playwright/** | ||
| retention-days: 7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already seem to upload playwright reports in the sub action. Can we get rid of one of them so it's not redundant? Maybe use if: always?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/publish.yml(3 hunks).zed/tasks.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: pr
🔇 Additional comments (3)
.github/workflows/publish.yml (2)
37-95: Excellent work addressing all prior review concerns.All critical and major issues flagged in previous reviews—malformed expressions, duplicate steps, undefined inputs, incorrect input placement, and redundant actions—have been properly resolved. The workflow now cleanly delegates setup and publishing logic to composable actions, eliminating duplication and improving maintainability.
51-54: Verify first Setup Project invocation accepts only CODECOV_TOKEN.The first Setup Project action (lines 51–54) passes only
CODECOV_TOKENwithoutnode-version-fileorpnpm-cache-folder. Confirm this is intentional—either these inputs have sensible defaults in the action definition, or they should be explicitly provided here as well (as shown in lines 118–120 for the snapshot job).If these inputs have defaults and differ from the snapshot job's values, document why the two invocations differ or align them.
.zed/tasks.json (1)
30-56: Verify watch task command syntax.The watch tasks (lines 30-56) use
["watch", "@forgerock/..."]without the "nx" prefix, unlike other nx-based tasks (build, lint, test). Confirm these are calling a custom pnpm script and not intended to use nx syntax.Please verify that
pnpm watch @forgerock/...is a valid custom script in package.json, or if these should instead be["nx", "watch", "@forgerock/..."]to match the nx pattern.
.zed/tasks.json
Outdated
| { | ||
| "label": "nx typecheck repo", | ||
| "command": "pnpm", | ||
| "args": ["nx", "affected -t typecheck"], | ||
| "cwd": "$ZED_WORKTREE_ROOT", | ||
| "reveal": "no_focus" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix malformed args array for typecheck task.
The args array at line 60 has "affected -t typecheck" as a single string, but should be split into separate elements to match the pattern used in other nx tasks (build, lint, test).
Apply this diff to fix the args:
{
"label": "nx typecheck repo",
"command": "pnpm",
- "args": ["nx", "affected -t typecheck"],
+ "args": ["nx", "affected", "-t", "typecheck"],
"cwd": "$ZED_WORKTREE_ROOT",
"reveal": "no_focus"
}📝 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.
| { | |
| "label": "nx typecheck repo", | |
| "command": "pnpm", | |
| "args": ["nx", "affected -t typecheck"], | |
| "cwd": "$ZED_WORKTREE_ROOT", | |
| "reveal": "no_focus" | |
| }, | |
| { | |
| "label": "nx typecheck repo", | |
| "command": "pnpm", | |
| "args": ["nx", "affected", "-t", "typecheck"], | |
| "cwd": "$ZED_WORKTREE_ROOT", | |
| "reveal": "no_focus" | |
| }, |
🤖 Prompt for AI Agents
In .zed/tasks.json around lines 57 to 63 the "args" entry for the "nx typecheck
repo" task incorrectly contains a single string "nx affected -t typecheck";
split this into separate array elements to match the other nx tasks (e.g.
["nx","affected","-t","typecheck"]) so the command and its flags are passed
correctly by the task runner.
Add build and API docs steps to publish actions and workflows - Run builds and generate API docs before publishing in beta and release actions - Use --no-agents flag for Nx build steps in workflows - Remove NX_CLOUD_DISTRIBUTED_EXECUTION env and related config - Remove E2E report upload step from CI workflow - Minor improvements to setup action step names
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nx Cloud has identified a possible root cause for your failed CI:
The failure has been classified as 'environment_state' rather than 'code_change' for the following reasons:
PR Intent Analysis:
The pull request is titled "chore: fix-slackbot" and focuses on:
- Fixing a bad interpolation in the Slack notification workflow
- Refactoring GitHub Actions workflows into reusable composite actions
- Removing the Mend scan workflow
- Adding a fork-specific CI workflow
- Minor configuration updates (Node version, Zed editor tasks, pnpm installation example)
Changes Made in PR:
All changes in this PR are infrastructure-related:
- New composite GitHub Actions (.github/actions/publish-beta, publish-release, setup)
- Modified CI workflows (.github/workflows/ci.yml, ci-fork.yml, publish.yml)
- Deleted Mend workflow (.github/workflows/mend.yml)
- Updated .node-version file (22 → 24)
- Added .zed/tasks.json for editor configuration
- Minor package.json updates (removed unused dev dependency, updated @types/node)
- Added pnpm installation example to packages/protect/README.md
Failed Test Analysis:
The test @forgerock/davinci-suites:e2e-ci--src/phone-number-field.test.ts failed with:
- Error: expect(locator).toBeVisible() failed
- Locator: getByText('SMS/Voice MFA Registered')
- Expected: visible
- Timeout: 5000ms
- Error: element(s) not found
This is an end-to-end test that:
- Registers a new user
- Logs in with that user
- Attempts to register a phone device for MFA
- Expects to see "SMS/Voice MFA Registered" text after submission
- Times out waiting for this confirmation message
No Correlation Between Changes and Failure:
- No files in the davinci-suites package were modified
- No phone number field components or logic were changed
- No MFA registration code was altered
- No changes to any application runtime code that would affect device registration flows
- The PR exclusively modifies CI/CD infrastructure and workflow orchestration
Environmental Characteristics Present:
- E2E test that depends on external services (authentication, user registration, SMS/Voice services)
- Timeout waiting for UI element suggests timing or service availability issue
- Test involves multiple external system interactions that could fail independently
- No code changes that would logically cause this specific element to not appear
- The failure is a classic symptom of external service unavailability or timing issues
Conclusion:
The test failure is unrelated to the workflow refactoring changes in this PR. The inability to see the "SMS/Voice MFA Registered" message is most likely due to an external service issue, network problem, or temporary unavailability of the phone/SMS registration backend service, not due to any code changes introduced by this pull request.
A code change would likely not resolve this issue, so no action was taken.
🎓 To learn more about Self Healing CI, please visit nx.dev
| - name: Track base repository main | ||
| run: | | ||
| git fetch origin +refs/heads/main:refs/remotes/upstream/main | ||
| git branch --force main upstream/main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving this PR but I think you could have also solved this with the remote: upstream option of the nrwl/nx-set-shas action.
https://github.com/nrwl/nx-set-shas/blob/main/README.md#configuration-options
nrwl/nx-set-shas#141
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nx Cloud has identified a possible root cause for your failed CI:
This failure was classified as 'flaky_task' rather than 'code_change' for the following reasons:
-
No correlation between PR changes and the failure:
The PR exclusively modifies CI/CD infrastructure files:- GitHub Actions workflow files (.github/workflows/*)
- GitHub Actions composite actions (.github/actions/*)
- Node version bump (22 → 24)
- Package.json dependency updates
- Documentation and editor configuration files
None of these changes affect the DaVinci application code or the phone-number-field test logic.
-
The failing test file was not modified:
The test filee2e/davinci-suites/src/phone-number-field.test.tsdoes not appear anywhere in the PR diff.
No application code related to phone number fields, user registration, or device authentication was changed. -
Nature of the failure indicates flakiness:
- The error shows a timeout waiting for UI element "Select Test Form" to become visible
- Timeout errors (5000ms exceeded) are characteristic of timing-dependent flaky tests
- The afterEach hook also timed out (30000ms exceeded), suggesting environment or timing instability
- E2E tests that depend on external services and DOM rendering are inherently prone to flakiness
-
No environmental factors present:
While the failure could potentially be 'environment_state' due to external service dependency, the timeout pattern and the fact that it's a Playwright e2e test with race conditions makes 'flaky_task' the more appropriate classification. -
Direct evidence test:
There is zero connection between refactoring GitHub Actions workflows/composite actions and the specific behavior of a DaVinci phone number field registration test.
The changes to CI infrastructure cannot cause a UI element to not render in the application.
A code change would likely not resolve this issue, so no action was taken.
🎓 To learn more about Self Healing CI, please visit nx.dev
track the proper main on fork flow
6210209 to
0460874
Compare
JIRA Ticket
N/A
Description
bad interpolation fix in 2ba62e4
the other two commits are me refactoring the workflows to use composite actions. I felt they were getting unmaintainbly long. just wanted to make it easier to parse through.
We can hold off on merging these until i'm back if preferred, they look like they all work fine but i'd hate for them not to and someone have to debug and fix it.
@spetrov maybe can review
Summary by CodeRabbit
Release Notes
Chores
Documentation