Build: Add NX check #33291
Build: Add NX check #33291
Conversation
|
View your CI Pipeline Execution ↗ for commit d4be2f8
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Pull request overview
This PR adds Nx Cloud status checks to the GitHub Actions workflow for pull requests. The changes create a pending status when a PR workflow starts and update it to success/failure/error when the workflow completes, with links to the Nx Cloud dashboard.
Key Changes
- Adds a pending commit status at the start of PR workflows linking to the Nx Cloud dashboard
- Adds a final commit status update that reflects the job outcome (success/failure/cancelled/skipped)
| await github.rest.repos.createCommitStatus({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| sha: context.payload.pull_request.head.sha, | ||
| state: 'pending', | ||
| avatar_url: 'https://avatars.githubusercontent.com/in/80458', | ||
| target_url: nxCloudUrl, | ||
| description: 'View Nx Cloud dashboard for this PR', | ||
| context: 'Nx Cloud' | ||
| }); |
There was a problem hiding this comment.
The workflow needs statuses: write permission to create commit statuses via the GitHub API. Currently, the permissions only include actions: read and contents: read, which will cause the createCommitStatus API calls to fail with a 403 error.
Add statuses: write to the permissions section:
permissions:
actions: read
contents: read
statuses: write
.github/workflows/nx.yml
Outdated
| repo: context.repo.repo, | ||
| sha: context.payload.pull_request.head.sha, | ||
| state: 'pending', | ||
| avatar_url: 'https://avatars.githubusercontent.com/in/80458', |
There was a problem hiding this comment.
The avatar_url parameter is not a valid parameter for the createCommitStatus API. According to the GitHub API documentation, the valid parameters are: state, target_url, description, and context. The avatar_url parameter will be ignored.
If you want to associate an avatar with the status, consider using a GitHub App instead of the commit status API, or remove this parameter as it has no effect.
| avatar_url: 'https://avatars.githubusercontent.com/in/80458', |
.github/workflows/nx.yml
Outdated
| repo: context.repo.repo, | ||
| sha: context.payload.pull_request.head.sha, | ||
| state, | ||
| avatar_url: 'https://avatars.githubusercontent.com/in/80458', |
There was a problem hiding this comment.
The avatar_url parameter is not a valid parameter for the createCommitStatus API. According to the GitHub API documentation, the valid parameters are: state, target_url, description, and context. The avatar_url parameter will be ignored.
If you want to associate an avatar with the status, consider using a GitHub App instead of the commit status API, or remove this parameter as it has no effect.
| avatar_url: 'https://avatars.githubusercontent.com/in/80458', |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds scheduled GitHub Actions NX orchestration that computes a tag, posts pending Nx Cloud status, runs nx once, emits per-task commit statuses and a final Nx Cloud status; removes CircleCI compile step, prepends yarn in check, adjusts CI task CPU/cache flags, and updates tags in 50+ project.json manifests. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant WF as nx.yml Workflow
participant Tag as Tag Logic
participant NXC as Nx Cloud API
participant NX as Nx Executor
participant CS as Commit Status API
GH->>WF: Trigger (push / PR / schedule)
WF->>Tag: Compute tag (normal / merged / daily)
Tag-->>WF: tag
WF->>NXC: Create Nx Cloud status (pending) with tag
NXC-->>WF: ack
WF->>NX: Run `nx` (capture stdout)
NX-->>WF: task results (+ optional log URLs)
WF->>WF: Parse nx output into per-task outcomes
loop per task
WF->>CS: Create per-task commit status "nx run <task>" (success/failure + log URL)
CS-->>WF: ack
end
alt any failures
WF->>NXC: Create final Nx Cloud status (failure + summary URL/description)
else all success
WF->>NXC: Create final Nx Cloud status (success + summary URL/description)
end
NXC-->>WF: final ack
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/nx.yml (1)
10-12: Missingstatuses: writepermission for commit status API calls.Both new GitHub Script steps (lines 41–59 and 77–107) call
github.rest.repos.createCommitStatus(), which requires thestatuses: writepermission. The current permissions block (lines 10–12) only grantsactions: readandcontents: read, so both steps will fail with a permission error when executed.Add
statuses: writeto the permissions block:permissions: actions: read contents: read + statuses: writeAlso applies to: 41-59, 77-107
🧹 Nitpick comments (1)
.github/workflows/nx.yml (1)
41-59: Reduce duplication of hardcoded Nx Cloud org/workspace IDs.The organization and workspace IDs are duplicated across both status update steps (lines 48 and 84). Extract these as workflow-level environment variables to improve maintainability and reduce the risk of inconsistency if they need to be updated.
Add env variables at the workflow or job level and reference them:
env: NX_CLOUD_ACCESS_TOKEN: ${{ secrets.NX_CLOUD_ACCESS_TOKEN }} + NX_CLOUD_ORG_ID: 606dcb5cdc2a2b00059cc0e9 + NX_CLOUD_WORKSPACE_ID: 6929fbef73e98d8094d2a343Then update both status update steps to use these variables:
- const nxCloudUrl = `https://cloud.nx.app/orgs/606dcb5cdc2a2b00059cc0e9/workspaces/6929fbef73e98d8094d2a343/overview?branch=${prNumber}`; + const nxCloudUrl = `https://cloud.nx.app/orgs/${process.env.NX_CLOUD_ORG_ID}/workspaces/${process.env.NX_CLOUD_WORKSPACE_ID}/overview?branch=${prNumber}`;Also applies to: 77-107
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nx.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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
scripts/tasks/compile.ts (1)
44-51: Logic is correct; consider adding parentheses for readability of cache conditionThe ternary currently relies on operator precedence (
||vs?:), which works but is a bit subtle. You could make the intent clearer (and avoid any future refactor mistakes) by explicitly grouping the condition:- `${command} ${skipCache || process.env.CI ? '--skip-nx-cache' : ''}`, + `${command} ${(skipCache || process.env.CI) ? '--skip-nx-cache' : ''}`,This keeps behavior identical (skip Nx cache when
skipCacheis truthy or when running in CI) but is easier to read at a glance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.circleci/config.yml(0 hunks)scripts/tasks/check.ts(1 hunks)scripts/tasks/compile.ts(1 hunks)
💤 Files with no reviewable changes (1)
- .circleci/config.yml
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
scripts/tasks/check.tsscripts/tasks/compile.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
scripts/tasks/check.tsscripts/tasks/compile.ts
🧬 Code graph analysis (1)
scripts/tasks/compile.ts (1)
scripts/utils/tools.ts (1)
process(115-115)
⏰ 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). (3)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
scripts/tasks/check.ts (2)
22-22: LGTM! Skip cache logic is correct.The conditional logic properly applies the
--skip-nx-cacheflag when either theskipCacheparameter is truthy or when running in a CI environment. This aligns with the PR objective to skip NX cache in CI.
19-20: The refactoring looks good. TheskipCacheparameter is properly typed inPassedOptionValues, which theTask.runfunction signature receives. Command selection extraction into a local variable improves readability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/nx.yml (1)
76-105: Extract hardcoded Nx Cloud workspace URL to environment variable.The finalize status step correctly maps job.status to GitHub commit states and uses the
always()condition to ensure execution. However, the hardcoded Nx Cloud workspace URL is duplicated between the pending and finalize steps (lines 46 and 83). Extracting this to an environment variable or repository secret would improve maintainability and reduce duplication.Define an environment variable at the workflow level and reference it in both steps:
env: NX_CLOUD_ACCESS_TOKEN: ${{ secrets.NX_CLOUD_ACCESS_TOKEN }} + NX_CLOUD_WORKSPACE_URL: https://cloud.nx.app/orgs/606dcb5cdc2a2b00059cc0e9/workspaces/6929fbef73e98d8094d2a343/overviewThen update both scripts to use this variable:
- const nxCloudUrl = `https://cloud.nx.app/orgs/606dcb5cdc2a2b00059cc0e9/workspaces/6929fbef73e98d8094d2a343/overview?branch=${prNumber}`; + const nxCloudUrl = `${{ env.NX_CLOUD_WORKSPACE_URL }}?branch=${prNumber}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nx.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). (3)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (2)
.github/workflows/nx.yml (2)
10-13: Correct permission fix for commit status creation.The addition of
statuses: writepermission directly resolves the previous blocker, enabling thecreateCommitStatusAPI calls to succeed without 403 errors.
39-56: No action needed on workspace ID configuration.The hardcoded Nx Cloud workspace IDs are public identifiers and appropriate to include in version control. The workspace ID (
6929fbef73e98d8094d2a343) matches thenxCloudIdalready declared innx.json, and the organization ID is a public URL component. The sensitive credential (NX_CLOUD_ACCESS_TOKEN) is correctly stored as a secret. This configuration follows standard practice and requires no changes.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
code/core/src/manager/globals/exports.ts (1)
658-667: New'a'entry instorybook/internal/typeslooks like an unintended globalThe addition:
'storybook/internal/types': [ 'Addon_TypesEnum', 'CoreWebpackCompiler', 'Feature', 'SupportedBuilder', 'SupportedFramework', 'SupportedLanguage', 'SupportedRenderer', 'a', ],combined with
export const aincode/core/src/types/index.tsintroduces a new globalized symbol with a non‑descriptive name that doesn’t match the rest of this list.Given this file is generated (“this file is generated by sourcefiles.ts”), and the paired TS export is also suspicious, this feels like a temporary/testing artifact rather than a deliberate public API addition.
I’d recommend:
- Removing
'a'from this list, and- If this change was intentional, instead adding a well‑named, documented symbol and updating
sourcefiles.ts(or the relevant generator) so this file stays in sync.Example revert here:
- 'SupportedRenderer', - 'a', + 'SupportedRenderer',and drop the corresponding export in
code/core/src/types/index.ts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/nx.yml(3 hunks)code/core/src/manager/globals/exports.ts(1 hunks)code/core/src/types/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/core/src/types/index.tscode/core/src/manager/globals/exports.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode
Files:
code/core/src/types/index.tscode/core/src/manager/globals/exports.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/types/index.tscode/core/src/manager/globals/exports.ts
code/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions that need to be tested from their modules
Files:
code/core/src/types/index.tscode/core/src/manager/globals/exports.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/core/src/types/index.tscode/core/src/manager/globals/exports.ts
🧠 Learnings (4)
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{ts,tsx,js,jsx} : Export functions that need to be tested from their modules
Applied to files:
code/core/src/types/index.ts
📚 Learning: 2025-11-28T14:50:24.889Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Ensure all builds and tests pass before submitting pull requests
Applied to files:
.github/workflows/nx.yml
📚 Learning: 2025-09-24T09:39:39.233Z
Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.
Applied to files:
code/core/src/manager/globals/exports.ts
📚 Learning: 2025-09-18T20:51:06.618Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/viewport/components/Tool.tsx:38-39
Timestamp: 2025-09-18T20:51:06.618Z
Learning: The useGlobals hook from storybook/manager-api returns a tuple where the third element (storyGlobals) is typed as Globals, not Globals | undefined. This means TypeScript guarantees it's always defined, making the `in` operator safe to use without additional null checks.
Applied to files:
code/core/src/manager/globals/exports.ts
🪛 actionlint (1.7.9)
.github/workflows/nx.yml
88-88: property "nx-run" is not defined in object type {}
(expression)
⏰ 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: Core Unit Tests, windows-latest
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/nx.yml (1)
87-92: Use${{ github.token }}instead of${{ secrets.GITHUB_TOKEN }}on line 91.Unless
GITHUB_TOKENis explicitly defined as a repository secret,${{ secrets.GITHUB_TOKEN }}will be empty and the API calls will fail with a 403 error. The built-in workflow token should be passed explicitly via the context:with: - github-token: ${{ secrets.GITHUB_TOKEN }} + github-token: ${{ github.token }}Alternatively, omit the
github-tokeninput entirely;actions/github-scriptdefaults to the workflow token automatically.Based on learnings from past review feedback, this was already flagged and should be corrected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nx.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/nx.yml
66-66: step ID "tag" duplicates. previously defined at line:61,col:13. step ID must be unique within a job. note that step ID is case insensitive
(id)
71-71: step ID "tag" duplicates. previously defined at line:61,col:13. step ID must be unique within a job. note that step ID is case insensitive
(id)
75-75: step ID "tag" duplicates. previously defined at line:61,col:13. step ID must be unique within a job. note that step ID is case insensitive
(id)
⏰ 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). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
.github/workflows/nx.yml (1)
10-13:statuses: writepermission addition is correct.This enables the subsequent
createCommitStatusAPI calls in the workflow.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/nx.yml (1)
85-89: ** Use${{ github.token }}instead of${{ secrets.GITHUB_TOKEN }}.**This issue was flagged in the previous review but has reappeared. Unless
GITHUB_TOKENis explicitly defined as a repository secret (which it is not by default), this reference will be empty. The built-in workflow token should be used instead:- name: Create per-task Nx statuses if: always() uses: actions/github-script@v7 with: - github-token: ${{ secrets.GITHUB_TOKEN }} + github-token: ${{ github.token }}Alternatively, omit the
github-tokeninput entirely;actions/github-scriptdefaults to using the workflow token automatically.
🧹 Nitpick comments (1)
.github/workflows/nx.yml (1)
62-75: Remove dead/commented-out code.The four commented-out tag-setting steps should be removed entirely or, if they represent planned future work, documented via an inline comment or tracked via an issue. Leaving commented code in the repository creates maintenance burden and confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nx.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). (3)
- GitHub Check: normal
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
.github/workflows/nx.yml (3)
10-13: Permissions correctly updated for commit status creation.The addition of
statuses: writeis necessary for thecreateCommitStatusAPI calls in the pending and per-task status steps.
35-49: Pending status step correctly handles PR and push events.The optional chaining and fallback logic properly gracefully handle both PR and non-PR workflows.
76-83: Nx run step correctly captures output and exit status.The
id: nxis now present and unique, enabling downstream reference tosteps.nx.outputs.nx_output. The use of${PIPESTATUS[0]}correctly preserves the yarn command's exit code.
.github/workflows/nx.yml
Outdated
| await github.rest.repos.createCommitStatus({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| sha, | ||
| state: failedCount ? 'failure' : 'success', | ||
| target_url: nxCloudUrl, | ||
| description: failedCount | ||
| ? `Nx Cloud run failed (${failedCount} tasks failed)` | ||
| : 'Nx Cloud run finished successfully', | ||
| context: 'Nx Cloud', |
There was a problem hiding this comment.
Undefined variable failedCount causes runtime error.
Line 132 references failedCount in a ternary operator, but this variable is never defined in the script. The script populates a failures array (line 93) but does not create a failedCount variable. This will crash the step with a ReferenceError when it executes.
Replace failedCount with failures.length:
await github.rest.repos.createCommitStatus({
owner: context.repo.owner,
repo: context.repo.repo,
sha,
- state: failedCount ? 'failure' : 'success',
+ state: failures.length ? 'failure' : 'success',
target_url: nxCloudUrl,
- description: failedCount
- ? `Nx Cloud run failed (${failedCount} tasks failed)`
+ description: failures.length
+ ? `Nx Cloud run failed (${failures.length} tasks failed)`
: 'Nx Cloud run finished successfully',
context: 'Nx Cloud',
});📝 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.
| await github.rest.repos.createCommitStatus({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| sha, | |
| state: failedCount ? 'failure' : 'success', | |
| target_url: nxCloudUrl, | |
| description: failedCount | |
| ? `Nx Cloud run failed (${failedCount} tasks failed)` | |
| : 'Nx Cloud run finished successfully', | |
| context: 'Nx Cloud', | |
| await github.rest.repos.createCommitStatus({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| sha, | |
| state: failures.length ? 'failure' : 'success', | |
| target_url: nxCloudUrl, | |
| description: failures.length | |
| ? `Nx Cloud run failed (${failures.length} tasks failed)` | |
| : 'Nx Cloud run finished successfully', | |
| context: 'Nx Cloud', |
🤖 Prompt for AI Agents
In .github/workflows/nx.yml around lines 128 to 137 the script references an
undefined variable `failedCount`, causing a ReferenceError; replace uses of
`failedCount` with `failures.length` (e.g., for the `state` ternary and the
description string) so the status uses the actual count of failures collected
earlier.
kasperpeulen
added
ci:daily
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/nx.yml (1)
97-97: Use${{ github.token }}instead of${{ secrets.GITHUB_TOKEN }}.The step passes
secrets.GITHUB_TOKEN, which will be undefined unless explicitly added as a repository secret. The built-in workflow token should be passed explicitly:- name: Create per-task Nx statuses if: always() uses: actions/github-script@v7 with: - github-token: ${{ secrets.GITHUB_TOKEN }} + github-token: ${{ github.token }}Alternatively, omit the
github-tokeninput entirely;actions/github-scriptdefaults to the workflow token automatically.
🧹 Nitpick comments (1)
.github/workflows/nx.yml (1)
104-117: Nx output parsing is fragile to format changes.The regex patterns used to extract task names (line 108:
✖\s+([^│]+?)\s{2,}) and log URLs (line 114:Task logs:\s*(https:\/\/...)) are tightly coupled to nx's current output format. If nx changes its output structure, these patterns will silently fail to match, resulting in missed failures or lost log links.Consider adding:
- Explicit error handling / logging if critical patterns don't match
- Fallback display of raw output line when parsing fails
- Documentation of the expected format
This is a low-priority observation but worth noting for long-term maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/nx.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). (3)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
- GitHub Check: nx
🔇 Additional comments (3)
.github/workflows/nx.yml (3)
94-98: Restrict per-task status step to pull_request events.The step runs unconditionally with
if: always()on all event types (PR, push, schedule) and creates commit statuses, which are primarily meaningful for PRs. While the script uses optional chaining to avoid crashes on non-PR events, it still attempts to create statuses on push/schedule runs, which may produce unintended check entries.Add a PR-only condition:
- name: Create per-task Nx statuses - if: always() + if: always() && github.event_name == 'pull_request' uses: actions/github-script@v7Verify whether commit statuses should be created for push-to-next and scheduled runs, or if this step should run only on PRs.
38-59: Well-structured tag consolidation.The consolidated "Set Nx tag(s)" step cleanly handles all event types (PR labels, push, schedule) in one place, eliminating the past duplicate-ID issue. The logic is clear and maintainable.
84-91: Correct step ID and output handling.The
id: nxenables downstream step references, and the heredoc output format withtee -acorrectly preserves both the exit status and captured output. This resolves past concerns around missing step IDs.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/nx.yml (1)
97-97: Replacesecrets.GITHUB_TOKENwithgithub.tokencontext reference.Line 97 passes
${{ secrets.GITHUB_TOKEN }}, which is undefined unless explicitly set as a repository secret. The built-in workflow token should be referenced via the GitHub Actions context instead. Past reviews flagged this same issue; ensure the fix is applied.Apply this diff:
- name: Create per-task Nx statuses if: always() uses: actions/github-script@v7 with: - github-token: ${{ secrets.GITHUB_TOKEN }} + github-token: ${{ github.token }} script: |Alternatively, omit the
github-tokeninput entirely;actions/github-script@v7defaults to using the workflow token automatically.
🧹 Nitpick comments (1)
.github/workflows/nx.yml (1)
99-117: Validate regex patterns for robustness against malformed output.The regex patterns on lines 108 and 114 parse nx CLI output to extract task names and task log URLs. These patterns assume consistent formatting; if nx output format changes or diverges, extraction may fail silently or produce incorrect results.
Consider adding defensive logic:
- Log extracted values for debugging
- Provide fallback descriptions if URL extraction fails
- Test against actual nx output to confirm pattern robustness
Example defensive enhancement:
const task = line.match(/✖\s+([^│]+?)\s{2,}/)?.[1]?.trim() || 'Unknown Nx task'; const url = lines .slice(i + 1, i + 6) .find(l => l.includes('Task logs:')) ?.match(/Task logs:\s*(https:\/\/cloud\.nx\.app\/logs\/\S+)/)?.[1]; if (!url) { console.warn(`No task log URL found for task: ${task}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
.github/workflows/nx.yml(3 hunks)code/addons/a11y/project.json(1 hunks)code/addons/docs/project.json(1 hunks)code/addons/links/project.json(1 hunks)code/addons/onboarding/project.json(1 hunks)code/addons/pseudo-states/project.json(1 hunks)code/addons/themes/project.json(1 hunks)code/addons/vitest/project.json(1 hunks)code/builders/builder-vite/project.json(1 hunks)code/builders/builder-webpack5/project.json(1 hunks)code/core/project.json(1 hunks)code/frameworks/angular/project.json(1 hunks)code/frameworks/ember/project.json(1 hunks)code/frameworks/html-vite/project.json(1 hunks)code/frameworks/nextjs-vite/project.json(1 hunks)code/frameworks/nextjs/project.json(1 hunks)code/frameworks/preact-vite/project.json(1 hunks)code/frameworks/react-native-web-vite/project.json(1 hunks)code/frameworks/react-vite/project.json(1 hunks)code/frameworks/react-webpack5/project.json(1 hunks)code/frameworks/server-webpack5/project.json(1 hunks)code/frameworks/svelte-vite/project.json(1 hunks)code/frameworks/sveltekit/project.json(1 hunks)code/frameworks/vue3-vite/project.json(1 hunks)code/frameworks/web-components-vite/project.json(1 hunks)code/lib/cli-storybook/project.json(1 hunks)code/lib/codemod/project.json(1 hunks)code/lib/core-webpack/project.json(1 hunks)code/lib/create-storybook/project.json(1 hunks)code/lib/csf-plugin/project.json(1 hunks)code/lib/eslint-plugin/project.json(1 hunks)code/lib/react-dom-shim/project.json(1 hunks)code/presets/create-react-app/project.json(1 hunks)code/presets/react-webpack/project.json(1 hunks)code/presets/server-webpack/project.json(1 hunks)code/project.json(1 hunks)code/renderers/html/project.json(1 hunks)code/renderers/preact/project.json(1 hunks)code/renderers/react/project.json(1 hunks)code/renderers/server/project.json(1 hunks)code/renderers/svelte/project.json(1 hunks)code/renderers/vue3/project.json(1 hunks)code/renderers/web-components/project.json(1 hunks)scripts/project.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- code/frameworks/svelte-vite/project.json
- code/renderers/server/project.json
🚧 Files skipped from review as they are similar to previous changes (19)
- code/frameworks/react-webpack5/project.json
- code/presets/create-react-app/project.json
- code/frameworks/nextjs/project.json
- code/frameworks/ember/project.json
- scripts/project.json
- code/lib/csf-plugin/project.json
- code/frameworks/nextjs-vite/project.json
- code/frameworks/sveltekit/project.json
- code/frameworks/react-vite/project.json
- code/renderers/web-components/project.json
- code/frameworks/react-native-web-vite/project.json
- code/renderers/react/project.json
- code/builders/builder-webpack5/project.json
- code/lib/codemod/project.json
- code/addons/a11y/project.json
- code/lib/core-webpack/project.json
- code/addons/onboarding/project.json
- code/frameworks/server-webpack5/project.json
- code/project.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use ESLint and Prettier configurations that are enforced in the codebase
Files:
code/frameworks/html-vite/project.jsoncode/lib/create-storybook/project.jsoncode/lib/react-dom-shim/project.jsoncode/presets/server-webpack/project.jsoncode/frameworks/angular/project.jsoncode/addons/themes/project.jsoncode/frameworks/preact-vite/project.jsoncode/addons/docs/project.jsoncode/addons/links/project.jsoncode/frameworks/vue3-vite/project.jsoncode/addons/pseudo-states/project.jsoncode/frameworks/web-components-vite/project.jsoncode/renderers/preact/project.jsoncode/renderers/html/project.jsoncode/lib/cli-storybook/project.jsoncode/builders/builder-vite/project.jsoncode/lib/eslint-plugin/project.jsoncode/renderers/vue3/project.jsoncode/renderers/svelte/project.jsoncode/addons/vitest/project.jsoncode/presets/react-webpack/project.jsoncode/core/project.json
code/**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing
Files:
code/frameworks/html-vite/project.jsoncode/lib/create-storybook/project.jsoncode/lib/react-dom-shim/project.jsoncode/presets/server-webpack/project.jsoncode/frameworks/angular/project.jsoncode/addons/themes/project.jsoncode/frameworks/preact-vite/project.jsoncode/addons/docs/project.jsoncode/addons/links/project.jsoncode/frameworks/vue3-vite/project.jsoncode/addons/pseudo-states/project.jsoncode/frameworks/web-components-vite/project.jsoncode/renderers/preact/project.jsoncode/renderers/html/project.jsoncode/lib/cli-storybook/project.jsoncode/builders/builder-vite/project.jsoncode/lib/eslint-plugin/project.jsoncode/renderers/vue3/project.jsoncode/renderers/svelte/project.jsoncode/addons/vitest/project.jsoncode/presets/react-webpack/project.jsoncode/core/project.json
⏰ 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). (3)
- GitHub Check: daily
- GitHub Check: nx
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (19)
code/addons/links/project.json (1)
8-8: Tag metadata update aligns with project classification.The change from
["ci:normal"]to["library"]standardizes the tag to match the declaredprojectTypeand supports the new Nx orchestration workflow. This is consistent across the 50+ project.json files being updated in this PR.code/frameworks/angular/project.json (1)
8-8: Tag update consistent across frameworks.This aligns with the systematic
["ci:normal"]→["library"]migration across all project.json files in this PR.code/lib/eslint-plugin/project.json (1)
8-8: Metadata tag update—consistent with PR-wide migration.code/lib/cli-storybook/project.json (1)
8-8: Metadata tag update—consistent with PR-wide migration.code/addons/themes/project.json (1)
7-7: Metadata tag update—consistent with PR-wide migration.code/addons/vitest/project.json (1)
8-8: Metadata tag update—consistent with PR-wide migration.code/presets/react-webpack/project.json (1)
8-8: Metadata tag update—consistent with PR-wide migration.code/frameworks/vue3-vite/project.json (1)
8-8: Metadata tag update—consistent with PR-wide migration.Verify that the Nx workflow (.github/workflows/nx.yml) correctly interprets and filters projects using the new
["library"]tag, especially for the diverse module types (addons, frameworks, presets, lib) represented in this PR. Please confirm that this tag migration doesn't break any existing CI orchestration that may depend on the["ci:normal"]tag.code/lib/create-storybook/project.json (1)
8-8: Metadata tag alignment.The tag update aligns with the repository-wide shift from
ci:normaltolibraryclassification for NX orchestration.code/addons/docs/project.json (1)
8-8: Metadata tag alignment.The tag update is consistent with the repository-wide migration to
libraryclassification.code/renderers/vue3/project.json (1)
11-11: Metadata tag alignment.The tag update aligns with the repository-wide NX orchestration metadata migration.
code/renderers/svelte/project.json (1)
11-11: Metadata tag alignment.The tag update maintains consistency with the repository-wide shift.
code/renderers/preact/project.json (1)
8-8: Metadata tag alignment.The tag update aligns with the NX orchestration metadata migration across the repository.
code/presets/server-webpack/project.json (1)
8-8: Metadata tag alignment.The tag update is consistent with the repository-wide metadata migration for NX orchestration.
.github/workflows/nx.yml (5)
38-59: Tag consolidation design is sound.The unified tag-setting step uses proper bash conditionals to derive tags based on event type and labels, eliminating the duplicate step ID issue from prior iterations. Outputs are correctly captured to
$GITHUB_OUTPUTand referenced downstream.
84-91: Step ID and output capture are correct.The
id: nxlabel on the Run nx step ensures thatsteps.nx.outputs.nx_outputis properly defined for downstream steps. The heredoc-style output capture withtee -a "$GITHUB_OUTPUT"correctly appends to GitHub's output file, andPIPESTATUS[0]properly preserves the exit code.
119-149: Per-task and final status logic is sound.The step properly uses optional chaining (
context.payload.pull_request?.head?.sha) to gracefully handle non-PR events (e.g., scheduled runs). Theif: always()condition ensures statuses are created regardless of prior step outcomes. The final commit status correctly reflects overall pass/fail based onfailures.length.
61-77: Pending status creation uses correct optional chaining.The "Create Nx Cloud Status (pending)" step safely accesses PR context with optional chaining and provides sensible fallbacks for non-PR events (line 70:
context.shafallback; line 73:'next'branch fallback).
9-10: Schedule trigger and job condition properly integrated.The cron-based schedule trigger (
0 23 * * *) is correctly added to theonsection, and the job condition (line 27–28) extends the trigger logic to includegithub.event_name == 'schedule', ensuring the workflow runs as intended on the daily cadence.Also applies to: 27-28
Closes #
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.