Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds artifact upload to the Canvas WASM build workflow, a new publish workflow that downloads and publishes Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
softmarshmallow
had a problem deploying
to
npm-publish
GitHub Actions
Failure
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/grida-canvas-sdk-render-figma/scripts/prepack-publish.cjs (1)
33-47: Consider adding error handling for workspace resolution failures.The
resolveWorkspaceDepsfunction will throw an uncaught exception if:
require.resolvefails (package not found in workspace)- The linked package.json is malformed
Since this runs during
prepack(blocking publish), a cryptic error could be confusing. Consider wrapping in try/catch with a descriptive error message.🛡️ Add error handling
function resolveWorkspaceDeps(obj) { if (!obj || typeof obj !== "object") return; for (const key of Object.keys(obj)) { const val = obj[key]; if (typeof val === "string" && val.startsWith("workspace:")) { - const linkedPkgJson = require.resolve(`${key}/package.json`, { - paths: [pkgDir], - }); - const linkedPkg = JSON.parse(fs.readFileSync(linkedPkgJson, "utf8")); - obj[key] = linkedPkg.version; + try { + const linkedPkgJson = require.resolve(`${key}/package.json`, { + paths: [pkgDir], + }); + const linkedPkg = JSON.parse(fs.readFileSync(linkedPkgJson, "utf8")); + obj[key] = linkedPkg.version; + } catch (err) { + throw new Error(`Failed to resolve workspace dependency "${key}": ${err.message}`); + } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/grida-canvas-sdk-render-figma/scripts/prepack-publish.cjs` around lines 33 - 47, The resolveWorkspaceDeps function currently lets require.resolve and JSON parsing errors bubble up; wrap the workspace resolution block (the branch where val.startsWith("workspace:")) in a try/catch inside resolveWorkspaceDeps and handle failures by throwing or logging a clearer, actionable error that includes the package key and pkgDir and the original error message (e.g., "Failed to resolve workspace dependency for '<key>' from '<pkgDir>': <err>"). Ensure you catch both require.resolve and JSON.parse/fs.readFileSync failures, and either leave obj[key] unchanged or rethrow a new Error with that descriptive message so prepack fails with a readable cause..github/workflows/test.yml (1)
20-29: Third-party action should be pinned to commit SHA for supply chain security.
dawidd6/action-download-artifact@v6is a third-party action. For security hardening, consider pinning to a specific commit SHA rather than a mutable tag to prevent potential supply chain attacks.Also note: downloading artifacts only from
mainbranch (line 27) means PRs that modify WASM code won't have their changes tested until merged. This is acceptable ifbuild-canvas.ymlruns on PRs and produces artifacts, but currently PRs would use stalemainartifacts.🔒 Pin action to commit SHA
- name: Download WASM artifacts id: wasm-download - uses: dawidd6/action-download-artifact@v6 + uses: dawidd6/action-download-artifact@<commit-sha> # v6 with:You can find the latest commit SHA at: https://github.com/dawidd6/action-download-artifact/releases
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 20 - 29, Replace the mutable action tag with a pinned commit SHA for dawidd6/action-download-artifact (change the uses entry for the wasm-download step from dawidd6/action-download-artifact@v6 to dawidd6/action-download-artifact@<commit-sha>) and update the artifact source logic so PRs are tested against their branch instead of always using branch: main (e.g., derive the branch from the PR head or allow build-canvas.yml to run on PRs); modify the wasm-download step inputs (workflow: build-canvas.yml and branch setting) accordingly to fetch artifacts produced for the current PR rather than the main branch..github/workflows/publish-canvas-wasm.yml (2)
67-96: Version bump logic may cause conflicts on concurrent runs.The version is calculated by incrementing the patch version in
package.json. If multiple workflow runs execute concurrently (e.g., rapid merges to main), they could compute the same version and cause npm publish failures.The
concurrencygroup (lines 25-27) withcancel-in-progress: falsehelps, but doesn't fully serialize if the first run completes before the second checks. Consider adding a check for existing npm versions before publishing.🛡️ Add version existence check
+ - name: Check if version exists on npm + working-directory: crates/grida-canvas-wasm + run: | + if npm view "@grida/canvas-wasm@${{ env.VERSION }}" version 2>/dev/null; then + echo "::error::Version ${{ env.VERSION }} already exists on npm" + exit 1 + fi + - name: Publish to npm🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-canvas-wasm.yml around lines 67 - 96, The current version bump (variables BRANCH, CURRENT, BASE, MAJOR, MINOR, PATCH, NEXT_PATCH and the npm version "$VERSION" call) can collide between concurrent runs; modify the bump step to check the npm registry for an existing package before publishing and loop to a new patch if it already exists: after computing VERSION use a registry check (e.g., npm view or npm info for `@grida/canvas-wasm`@$VERSION) and if it exists increment PATCH/NEXT_PATCH and recompute VERSION until a non-existent version is found, then run npm version "$VERSION" --no-git-tag-version and set GITHUB_ENV; keep the branch-based canary/tag logic and the manual INPUT_TAG override intact.
121-128: Consider improving push resilience for future branch protection scenarios.The workflow successfully pushes version bumps to
mainusing the defaultGITHUB_TOKEN. While this currently works, if branch protection rules requiring PRs or status checks are added later, direct pushes will fail. Consider:
- Creating a PR instead of pushing directly, allowing review/checks to run
- Using a service account PAT with elevated permissions (if bypassing protection is intentional)
- Documenting that branch protection must allow
github-actions[bot]to push directly🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-canvas-wasm.yml around lines 121 - 128, The "Commit version bump (stable only)" step currently pushes directly to main using GITHUB_TOKEN which will break if branch protection is enabled; update this step to create a pull request instead of a direct push (or switch to using a service account PAT if direct push is intentional) — specifically, after committing the version bump to the working branch, push that branch and open a PR via the GitHub CLI or API (refer to the step conditional using env.TAG and inputs.dry_run to preserve behavior), or replace GITHUB_TOKEN usage with a preconfigured PAT and document this choice so future branch-protection rules are accommodated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/publish-canvas-wasm.yml:
- Around line 109-112: The "Publish to npm" step currently runs "npm publish"
without supplying the NODE_AUTH_TOKEN; add an env entry to that job step so
NODE_AUTH_TOKEN is set from the repository secret (e.g. NODE_AUTH_TOKEN: ${{
secrets.NPM_TOKEN }}) and do the same for the dry-run publish step (the step
guarded by inputs.dry_run == 'true') so both the working-directory:
crates/grida-canvas-wasm publish steps have the NODE_AUTH_TOKEN environment
variable available to authenticate npm publish.
---
Nitpick comments:
In @.github/workflows/publish-canvas-wasm.yml:
- Around line 67-96: The current version bump (variables BRANCH, CURRENT, BASE,
MAJOR, MINOR, PATCH, NEXT_PATCH and the npm version "$VERSION" call) can collide
between concurrent runs; modify the bump step to check the npm registry for an
existing package before publishing and loop to a new patch if it already exists:
after computing VERSION use a registry check (e.g., npm view or npm info for
`@grida/canvas-wasm`@$VERSION) and if it exists increment PATCH/NEXT_PATCH and
recompute VERSION until a non-existent version is found, then run npm version
"$VERSION" --no-git-tag-version and set GITHUB_ENV; keep the branch-based
canary/tag logic and the manual INPUT_TAG override intact.
- Around line 121-128: The "Commit version bump (stable only)" step currently
pushes directly to main using GITHUB_TOKEN which will break if branch protection
is enabled; update this step to create a pull request instead of a direct push
(or switch to using a service account PAT if direct push is intentional) —
specifically, after committing the version bump to the working branch, push that
branch and open a PR via the GitHub CLI or API (refer to the step conditional
using env.TAG and inputs.dry_run to preserve behavior), or replace GITHUB_TOKEN
usage with a preconfigured PAT and document this choice so future
branch-protection rules are accommodated.
In @.github/workflows/test.yml:
- Around line 20-29: Replace the mutable action tag with a pinned commit SHA for
dawidd6/action-download-artifact (change the uses entry for the wasm-download
step from dawidd6/action-download-artifact@v6 to
dawidd6/action-download-artifact@<commit-sha>) and update the artifact source
logic so PRs are tested against their branch instead of always using branch:
main (e.g., derive the branch from the PR head or allow build-canvas.yml to run
on PRs); modify the wasm-download step inputs (workflow: build-canvas.yml and
branch setting) accordingly to fetch artifacts produced for the current PR
rather than the main branch.
In `@packages/grida-canvas-sdk-render-figma/scripts/prepack-publish.cjs`:
- Around line 33-47: The resolveWorkspaceDeps function currently lets
require.resolve and JSON parsing errors bubble up; wrap the workspace resolution
block (the branch where val.startsWith("workspace:")) in a try/catch inside
resolveWorkspaceDeps and handle failures by throwing or logging a clearer,
actionable error that includes the package key and pkgDir and the original error
message (e.g., "Failed to resolve workspace dependency for '<key>' from
'<pkgDir>': <err>"). Ensure you catch both require.resolve and
JSON.parse/fs.readFileSync failures, and either leave obj[key] unchanged or
rethrow a new Error with that descriptive message so prepack fails with a
readable cause.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e32bbde9-0b70-44c4-9ca7-0a23d65c37b4
⛔ Files ignored due to path filters (2)
crates/grida-canvas-wasm/lib/bin/grida_canvas_wasm.wasmis excluded by!**/*.wasmpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.github/workflows/build-canvas.yml.github/workflows/publish-canvas-wasm.yml.github/workflows/publish-packages.yml.github/workflows/test.ymlcrates/grida-canvas-wasm/lib/bin/.gitattributescrates/grida-canvas-wasm/lib/bin/.gitignorecrates/grida-canvas-wasm/lib/bin/grida-canvas-wasm.jscrates/grida-canvas-wasm/package.jsonpackages/grida-canvas-sdk-render-figma/package.jsonpackages/grida-canvas-sdk-render-figma/scripts/prepack-publish.cjs
💤 Files with no reviewable changes (2)
- crates/grida-canvas-wasm/lib/bin/.gitattributes
- .github/workflows/publish-packages.yml
| - name: Publish to npm | ||
| if: inputs.dry_run != 'true' | ||
| working-directory: crates/grida-canvas-wasm | ||
| run: npm publish --tag ${{ env.TAG }} --access public --provenance |
There was a problem hiding this comment.
Missing NODE_AUTH_TOKEN environment variable for npm publish.
The npm publish command requires authentication. While actions/setup-node with registry-url sets up the .npmrc, you still need to provide NODE_AUTH_TOKEN as an environment variable for the publish step.
🔧 Add NODE_AUTH_TOKEN
- name: Publish to npm
if: inputs.dry_run != 'true'
working-directory: crates/grida-canvas-wasm
run: npm publish --tag ${{ env.TAG }} --access public --provenance
+ env:
+ NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}Apply the same fix to the dry-run publish step (lines 114-119).
📝 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.
| - name: Publish to npm | |
| if: inputs.dry_run != 'true' | |
| working-directory: crates/grida-canvas-wasm | |
| run: npm publish --tag ${{ env.TAG }} --access public --provenance | |
| - name: Publish to npm | |
| if: inputs.dry_run != 'true' | |
| working-directory: crates/grida-canvas-wasm | |
| run: npm publish --tag ${{ env.TAG }} --access public --provenance | |
| env: | |
| NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/publish-canvas-wasm.yml around lines 109 - 112, The
"Publish to npm" step currently runs "npm publish" without supplying the
NODE_AUTH_TOKEN; add an env entry to that job step so NODE_AUTH_TOKEN is set
from the repository secret (e.g. NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}) and
do the same for the dry-run publish step (the step guarded by inputs.dry_run ==
'true') so both the working-directory: crates/grida-canvas-wasm publish steps
have the NODE_AUTH_TOKEN environment variable available to authenticate npm
publish.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/publish-canvas-wasm.yml (1)
29-35: Consider adding a job timeout.The job has no
timeout-minutesset. If the build or publish hangs, the job will run until GitHub's default timeout (6 hours). A reasonable timeout (e.g., 15-20 minutes) prevents resource waste.publish: if: >- github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success' runs-on: ubuntu-latest + timeout-minutes: 15 environment: npm-publish🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-canvas-wasm.yml around lines 29 - 35, The publish job currently lacks a timeout and can hang for GitHub's default period; add a timeout-minutes key to the publish job (the top-level job named "publish") with a reasonable value (e.g., 15 or 20) to bound execution time; update the jobs.publish block to include timeout-minutes: 20 so the workflow cancels long-running build/publish steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/publish-canvas-wasm.yml:
- Around line 29-35: The publish job currently lacks a timeout and can hang for
GitHub's default period; add a timeout-minutes key to the publish job (the
top-level job named "publish") with a reasonable value (e.g., 15 or 20) to bound
execution time; update the jobs.publish block to include timeout-minutes: 20 so
the workflow cancels long-running build/publish steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2e56a690-373a-48d9-8d9f-b38f596a3c23
📒 Files selected for processing (3)
.github/workflows/publish-canvas-wasm.yml.gitignorecrates/grida-canvas-wasm/package.json
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/grida-canvas-wasm/package.json
softmarshmallow
had a problem deploying
to
npm-publish
GitHub Actions
Failure
softmarshmallow
temporarily deployed
to
npm-publish
GitHub Actions
Inactive
1 participant
Summary by CodeRabbit