Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #126 +/- ##
=======================================
Coverage ? 88.97%
=======================================
Files ? 13
Lines ? 127
Branches ? 44
=======================================
Hits ? 113
Misses ? 14
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.pnp.loader.mjs (1)
1500-1513:⚠️ Potential issue | 🟡 MinorClamp
package.json.typeto known loader formats.Lines 1503 and 1513 now forward any non-nullish
typevalue. A malformed value like""becomesformat, whichload()can't handle.🛠️ Suggested fix
case `.js`: { const pkg = readPackageScope(filepath); if (!pkg) return `commonjs`; - return pkg.data.type ?? `commonjs`; + return pkg.data.type === `module` ? `module` : `commonjs`; } default: { if (entrypointPath !== filepath) return null; const pkg = readPackageScope(filepath); if (!pkg) return `commonjs`; if (pkg.data.type === `module`) return null; - return pkg.data.type ?? `commonjs`; + return `commonjs`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pnp.loader.mjs around lines 1500 - 1513, The code is returning any non-nullish package.type which can be malformed (e.g. empty string) and later break the loader; update both places that return pkg.data.type ?? `commonjs` to explicitly clamp to known formats: check if pkg.data.type === 'module' then return 'module', otherwise return 'commonjs'. Locate the two uses with readPackageScope(filepath) (and the branch that checks pkg.data.type === 'module') and replace the nullish-coalescing return with the explicit conditional clamp so only 'module' or 'commonjs' are ever returned.
🟠 Major comments (20)
.github/workflows/test-commons.yml-30-34 (1)
30-34:⚠️ Potential issue | 🟠 MajorPersist
YARN_GLOBAL_FOLDERusing$GITHUB_ENV, not.env.Line 33 writes to a repository
.envfile, which does not export the variable to subsequent workflow steps. Use$GITHUB_ENVinstead to ensure the variable is available to all following steps.Proposed fix
- name: Set Yarn global folder env: YARN_GLOBAL_FOLDER: ${{ runner.temp }}/.yarn/berry - run: echo "YARN_GLOBAL_FOLDER=$YARN_GLOBAL_FOLDER" >> .env + run: echo "YARN_GLOBAL_FOLDER=$YARN_GLOBAL_FOLDER" >> "$GITHUB_ENV" shell: bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test-commons.yml around lines 30 - 34, The step setting YARN_GLOBAL_FOLDER currently appends to a repo .env file which doesn't export to later workflow steps; change the run command so it writes "YARN_GLOBAL_FOLDER=$YARN_GLOBAL_FOLDER" to the GitHub Actions environment file ($GITHUB_ENV) instead of .env so subsequent steps see the variable (update the step that defines YARN_GLOBAL_FOLDER in the workflow where the run uses echo to .env to instead echo into $GITHUB_ENV)..github/workflows/test-commons.yml-35-47 (1)
35-47:⚠️ Potential issue | 🟠 MajorAlign Node version in CI workflows with
.nvmrcto prevent CI/runtime skew.Lines 39 and 46 pin
node-version: 24.xwhile.nvmrcspecifiesv22. This mismatch can cause inconsistent lint/build results between local and CI environments.Proposed fix
- name: Setup Node.js environment if: matrix.os == 'ubuntu-latest' uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6 with: - node-version: 24.x + node-version-file: .nvmrc cache: yarn cache-dependency-path: yarn.lock - name: Setup Node.js environment (Windows) if: matrix.os == 'windows-latest' uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6 with: - node-version: 24.x + node-version-file: .nvmrc cache: yarnNote: This pattern appears across all workflow files in the repository and should be addressed consistently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test-commons.yml around lines 35 - 47, CI uses Node 24.x in the "Setup Node.js environment" and "Setup Node.js environment (Windows)" steps while .nvmrc specifies v22, causing environment skew; update those steps (the actions/setup-node uses with key `node-version` in the steps named "Setup Node.js environment" and "Setup Node.js environment (Windows)") to match .nvmrc (use 22.x or read the .nvmrc value) and apply the same change across all workflow files to ensure consistent Node versions in CI and local dev..pnp.loader.mjs-183-190 (1)
183-190:⚠️ Potential issue | 🟠 MajorCreate
dirname(indexPath)before touching the shard.Line 183 shards the index under
<indexPath>/<hash[0:2]>, but Lines 214-221 assume that directory already exists. The first file for a new prefix will fail withENOENT.🛠️ Suggested fix
const sourceMode = sourceStat.mode & 511; const indexFileName = `${sourceHash}${sourceMode !== defaultMode ? sourceMode.toString(8) : ``}`; const indexPath = destinationFs.pathUtils.join(linkStrategy.indexPath, sourceHash.slice(0, 2), `${indexFileName}.dat`); + const indexDir = destinationFs.pathUtils.dirname(indexPath); let AtomicBehavior; @@ let tempPathCleaned = false; prelayout.push(async () => { + await destinationFs.mkdirpPromise(indexDir); if (!indexStat) { if (atomicBehavior === 0 /* Lock */) { await destinationFs.lockPromise(indexPath, async () => {Also applies to: 211-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pnp.loader.mjs around lines 183 - 190, The shard directory for indexPath isn't guaranteed to exist before we access the shard file, causing ENOENT; before calling maybeLStat(indexPath) or writing the shard, ensure the parent directory exists by creating dirname(indexPath) via destinationFs.mkdirp or equivalent on destinationFs.pathUtils.dirname(indexPath); place this directory-creation step just before the code referencing indexPath (around where indexPath, AtomicBehavior, atomicBehavior, and indexStat are initialized) so subsequent operations that assume the shard directory exists will succeed..pnp.loader.mjs-191-209 (1)
191-209:⚠️ Potential issue | 🟠 MajorPropagate hardlink destination changes back to the caller.
Lines 234-249 can remove/recreate
destination, but the function still returnsfalse. SincecopyImpl()skips the file postlayout branch forHardlinkFromIndex, parents never learn that the directory entry changed and won't restore their timestamps instableTimemode.🛠️ Suggested fix
if (destinationStat) { const isDestinationHardlinkedFromIndex = indexStat && destinationStat.dev === indexStat.dev && destinationStat.ino === indexStat.ino; const isIndexModified = indexStat?.mtimeMs !== defaultTimeMs; @@ } } } + const willCreateDestination = destinationStat === null; const tempPath = !indexStat && atomicBehavior === 1 /* Rename */ ? `${indexPath}.${Math.floor(Math.random() * 4294967296).toString(16).padStart(8, `0`)}` : null; @@ - if (!destinationStat) { + if (willCreateDestination) { await destinationFs.linkPromise(indexPath, destination); } }); @@ - return false; + return willCreateDestination; }Also applies to: 234-249
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pnp.loader.mjs around lines 191 - 209, The code path that removes/recreates the destination (inside the block using destinationStat, isDestinationHardlinkedFromIndex and the opts.overwrite branch in copyImpl) ends up returning false and not informing callers that the directory entry changed; update this path to propagate the change back to the caller by marking the destination as modified and returning a success/changed indicator instead of false (e.g., set destinationStat appropriately or set a destinationChanged flag and have copyImpl return true/changed), so callers handling HardlinkFromIndex learn the directory entry changed and can restore timestamps in stableTime mode; ensure the change is applied where destination is removed/recreated (the branch that calls destinationFs.removePromise(destination) and sets destinationStat = null) and adjust the function return/contract accordingly..pnp.loader.mjs-131-135 (1)
131-135:⚠️ Potential issue | 🟠 MajorOnly swallow
ENOENTinmaybeLStat().Line 134 currently turns every
lstatfailure into "missing". That masks permission,ENOTDIR, and I/O errors, and can send the copy path down the wrong overwrite/create branch.🛠️ Suggested fix
async function maybeLStat(baseFs, p) { try { return await baseFs.lstatPromise(p); - } catch { - return null; + } catch (err) { + if (err?.code === `ENOENT`) { + return null; + } + throw err; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pnp.loader.mjs around lines 131 - 135, The maybeLStat function currently treats any lstat failure as "missing"; modify its catch to only swallow ENOENT errors: in the catch block inspect the thrown error (from baseFs.lstatPromise) and if error.code === 'ENOENT' return null, otherwise rethrow the error so permission, ENOTDIR, I/O, and other errors are propagated; reference the maybeLStat function and the baseFs.lstatPromise call when making this change..pnp.loader.mjs-169-172 (1)
169-172:⚠️ Potential issue | 🟠 MajorReturn the child status from the parallel folder walk.
Line 170 awaits
copyImpl()but discards its boolean, so Line 172'ssome(status => status)can never flipupdated. Existing directories won't queue their postlayoutlutimesfixup after a child changes.🛠️ Suggested fix
} else { const entriesUpdateStatus = await Promise.all(entries.map(async (entry) => { - await copyImpl(prelayout, postlayout, destinationFs, destinationFs.pathUtils.join(destination, entry), sourceFs, sourceFs.pathUtils.join(source, entry), nextOpts); + return copyImpl(prelayout, postlayout, destinationFs, destinationFs.pathUtils.join(destination, entry), sourceFs, sourceFs.pathUtils.join(source, entry), nextOpts); })); if (entriesUpdateStatus.some((status) => status)) { updated = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pnp.loader.mjs around lines 169 - 172, The parallel folder walk is discarding the boolean result from copyImpl so entriesUpdateStatus will always be undefined; change the entries.map callback in the Promise.all call to return the child status (e.g., replace the current async block that awaits copyImpl without returning with one that returns the boolean result from copyImpl, or simply map to copyImpl(...) directly) so entriesUpdateStatus contains the actual statuses used by the subsequent entriesUpdateStatus.some(status => status) check..prettierignore-5-6 (1)
5-6:⚠️ Potential issue | 🟠 MajorIgnoring
appsandpackagesdisables formatting for most source changes.This conflicts with the staged formatting flow (
packages/commons/lint-staged.base.config.tsLine 1-5), so many staged files won’t actually be formatted.🛠️ Narrow the ignore scope
-apps -packages +# Ignore only generated artifacts under apps/packages instead of entire source roots.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.prettierignore around lines 5 - 6, .prettierignore currently excludes the top-level entries "apps" and "packages", which prevents Prettier from formatting most source changes and conflicts with the staged formatting flow referenced by lint-staged; remove or narrow those entries (e.g., delete "apps" and "packages" or replace them with specific build/output patterns like "apps/*/dist" and "packages/*/dist" or other generated paths) and then ensure the staged formatter configuration (the lint-staged base config referenced as lint-staged.base.config.ts) uses globs that match the now-included source files so staged files are actually formatted..github/workflows/bitbucket.yml-24-24 (1)
24-24:⚠️ Potential issue | 🟠 Major
timeout-minutes: 2is too tight for this workflow's networked operations on a large repository.With a 250MB repository and
fetch-depth: 0(full history), checkout alone can exceed 30–60 seconds. Adding SSH setup, remote fetch, merge, and push creates flaky timeout risks.🛠️ Suggested fix
- timeout-minutes: 2 + timeout-minutes: 10🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bitbucket.yml at line 24, The workflow's hardcoded timeout "timeout-minutes: 2" is too short for networked operations on large repos (especially with "fetch-depth: 0"); increase the timeout to a safer value (e.g., 15–30 minutes) or make it configurable via an input/env var, and update the workflow step that contains "timeout-minutes: 2" to the new value or to reference the configurable variable so checkout, SSH setup, fetch/merge/push steps have sufficient time..github/workflows/bitbucket.yml-61-64 (1)
61-64:⚠️ Potential issue | 🟠 MajorBranch checkout assumes
bitbucket/mainalready exists on Bitbucket.Line 62 will fail on first bootstrap or if the Bitbucket repository lacks a
mainbranch. Aftergit fetch bitbucket(line 60), thebitbucket/mainremote tracking branch will not exist if the upstream doesn't have it, causing the checkout to fail. Add a fallback to useorigin/bitbucketif the remote branch is missing.🛠️ Suggested fix
- - name: Switch to bitbucket/main - run: git checkout -B bitbucket-main bitbucket/main - - name: Merge origin/bitbucket into bitbucket/main - run: git merge origin/bitbucket + - name: Switch to bitbucket/main (with fallback) + run: | + if git show-ref --verify --quiet refs/remotes/bitbucket/main; then + git checkout -B bitbucket-main bitbucket/main + else + git checkout -B bitbucket-main origin/bitbucket + fi + - name: Merge origin/bitbucket into bitbucket/main + run: git merge --no-edit origin/bitbucket🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bitbucket.yml around lines 61 - 64, The checkout step using "git checkout -B bitbucket-main bitbucket/main" can fail if the remote bitbucket/main doesn't exist; update the workflow to detect the remote branch before that checkout and fall back to using origin/bitbucket (or create the local branch from origin/bitbucket) when bitbucket/main is missing. Concretely, replace the unconditional "git checkout -B bitbucket-main bitbucket/main" with logic that checks for the remote ref (e.g., via "git ls-remote" or "git show-ref" for refs/remotes/bitbucket/main) and if present runs "git checkout -B bitbucket-main bitbucket/main", otherwise runs a fallback that creates bitbucket-main from "origin/bitbucket" and then run the existing "git merge origin/bitbucket" step..env.defaults-5-5 (1)
5-5:⚠️ Potential issue | 🟠 Major
YARN_GLOBAL_FOLDERconcatenates two independent fallback expressions.When both
LOCALAPPDATAandHOMEare set, this resolves to an invalid composite path. Use a nested fallback chain instead.🛠️ Suggested fix
-YARN_GLOBAL_FOLDER=${LOCALAPPDATA:-/}${HOME:-/}/.yarn/berry +YARN_GLOBAL_FOLDER=${LOCALAPPDATA:-${HOME:-/}}/.yarn/berry🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.defaults at line 5, The YARN_GLOBAL_FOLDER line builds an invalid path by concatenating two independent fallbacks; change the fallback logic so it uses a nested fallback chain instead of concatenation: when LOCALAPPDATA is set use it, otherwise fall back to HOME, and if neither is set fall back to root, then append /.yarn/berry. Update the YARN_GLOBAL_FOLDER assignment in .env.defaults to use a nested parameter expansion referencing LOCALAPPDATA and HOME (use the nested fallback pattern) so the resolved path is a single valid base plus /.yarn/berry..vscode/settings.default.json-11-12 (1)
11-12:⚠️ Potential issue | 🟠 Major
eslint.execArgvcontains a bare script path instead of Node.js CLI flags.Line 12 passes
["${workspaceFolder}/bin/swc-node.js"], buteslint.execArgvmust contain only Node runtime flags (e.g.,["--max-old-space-size=8192"]). A bare script path is invalid and will prevent the ESLint language server from starting.If the intent is to preload or require this file, use:
["--require", "${workspaceFolder}/bin/swc-node.js"]instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vscode/settings.default.json around lines 11 - 12, The eslint.execArgv setting is incorrectly given a bare script path; update the value of "eslint.execArgv" to use Node CLI flags only — for example replace the current entry ["${workspaceFolder}/bin/swc-node.js"] with ["--require", "${workspaceFolder}/bin/swc-node.js"] (or remove it entirely) so the ESLint language server receives valid Node runtime flags..github/workflows/packages.yml-13-17 (1)
13-17:⚠️ Potential issue | 🟠 MajorDon't advertise
release=trueunconditionally.The
releaseinput at Lines 13-14 is ignored and the reusable workflow output is always'true'. Any caller that keys off this output loses the ability to distinguish a normal CI run from an actual release run.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/packages.yml around lines 13 - 17, The outputs block is hardcoding release:'true' which ignores the reusable workflow input; update the outputs.release.value to propagate the workflow input (use the inputs.release expression) so callers can detect actual release runs (i.e., replace the literal 'true' with the appropriate expression that references the release input), and verify the input name "release" in the workflow definition and any callers match..github/workflows/test-app.yml-33-44 (1)
33-44:⚠️ Potential issue | 🟠 MajorScope the Vercel secrets to the Ubuntu deploy path only.
VERCEL_TOKEN,VERCEL_PROJECT_ID, andVERCEL_ORG_IDare injected into every matrix run here, but only the Ubuntu Vercel steps use them. Keeping deploy credentials at job scope unnecessarily broadens secret exposure to the Windows runner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test-app.yml around lines 33 - 44, The Vercel secrets (VERCEL_TOKEN, VERCEL_PROJECT_ID, VERCEL_ORG_ID) are currently set in the top-level env block and thus exposed to every matrix run; move these three variables out of the top-level env and into the specific Ubuntu deploy job or the deploy step that performs the Vercel publish (where the Vercel runner uses them), leaving the remaining env entries (REF_NAME, TURBO_*, YARN_ENABLE_HARDENED_MODE, TURBO_TELEMETRY_DISABLED, TURBO_GLOBAL_WARNING_DISABLED) at top-level; remove the three VERCEL_* entries from the shared env so only the Ubuntu deploy job/step has access to VERCEL_TOKEN, VERCEL_PROJECT_ID and VERCEL_ORG_ID..github/workflows/sync.yml-66-82 (1)
66-82:⚠️ Potential issue | 🟠 MajorDon't turn merge failures into a green sync.
git merge ... || trueand the final... || truesuppress real errors here. If any conflict remains after the prune step, the job can skip the commit and still continue togit push, leaving the branch unsynchronized while the workflow reports success.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sync.yml around lines 66 - 82, The merge step currently suppresses errors with "git merge ... || true" and the amend commit step ends with "|| true", allowing conflicts or failed commits to be treated as success; change these to surface failures: remove the "|| true" from the "git merge base/main --allow-unrelated-histories --no-commit --ff -X ours" command so the job fails on merge conflicts (or explicitly check the exit code and run "exit 1" on non-zero), and remove the final "|| true" from the "git diff-index --quiet HEAD || (git add . && git commit --no-edit) || true" step so a failed commit fails the job; also ensure the prune step (the "git status | grep 'deleted by us' | awk '{print $4}' | (xargs git rm || true)") correctly fails on errors or is guarded with a clear no-op when there are no matches (avoid swallowing real errors) so the workflow cannot report success while the branch is unsynchronized..github/workflows/package.yml-137-141 (1)
137-141:⚠️ Potential issue | 🟠 MajorMake deployment wait for lint and test.
deployhas noneeds, so Chromatic can run while verification is still in progress or after one of those jobs fails. That removes the normal gate before publishing preview artifacts.Possible change
deploy: name: Deploy + needs: [lint, test] timeout-minutes: 5 runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/package.yml around lines 137 - 141, The deploy job is missing a needs declaration so it can run before lint/test finish; update the deploy job (named "deploy") to include a needs array referencing the verification jobs that must succeed (e.g., add needs: ["lint", "test"] or the actual job IDs used for your linting/tests/verification steps) so Chromatic/publishing waits for those jobs to complete and will not run if they fail..gitignore-20-24 (1)
20-24:⚠️ Potential issue | 🟠 Major
!.env.*re-allows secret-bearing env files.After Line 20 ignores
.env*, Line 23 unignores every.env.<suffix>file again, including common local secret files like.env.localand.env.production. Prefer explicit exceptions for the checked-in templates only.Possible change
.env* .flaskenv* *.secrets -!.env.* +!.env.ci +!.env.defaults .env.keys🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 20 - 24, The current .gitignore has a broad unignore pattern "!.env.*" that re-allows secret-bearing files after ".env*" is ignored; change this to only unignore explicit template filenames (e.g., replace "!.env.*" with explicit safe templates like "!.env.example" and/or "!.env.template") and keep sensitive patterns (".env*", ".env.keys", "*.secrets") ignored; update the .gitignore by removing the general "!.env.*" entry and adding only the known checked-in template names so secrets like ".env.local" and ".env.production" remain ignored..github/workflows/packages.yml-36-64 (1)
36-64:⚠️ Potential issue | 🟠 MajorForward
inputs.releaseinto the package workflow calls.
.github/workflows/package.yml, Lines 175-188, gates cleanup oninputs.release == 'true', but none of these three calls pass that input through. Release invocations from this workflow will silently skip the cleanup job for every package.Possible change
circle-ui-elements: uses: ./.github/workflows/package.yml if: contains(fromJSON(inputs.package_names), '@psychobolt/circle-ui-elements') with: ref: ${{ inputs.ref }} workspace: circle-ui-elements path: packages/circle-ui-elements + release: ${{ inputs.release }} circle-ui-react: uses: ./.github/workflows/package.yml if: contains(fromJSON(inputs.package_names), '@psychobolt/circle-ui-react') with: ref: ${{ inputs.ref }} workspace: circle-ui-react path: packages/circle-ui-react test-runner: vitest + release: ${{ inputs.release }} react-pie-menu: uses: ./.github/workflows/package.yml if: contains(fromJSON(inputs.package_names), 'react-pie-menu') with: ref: ${{ inputs.ref }} workspace: react-pie-menu path: ./ filter: --filter=. test-runner: vitest + release: ${{ inputs.release }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/packages.yml around lines 36 - 64, The package workflow calls for circle-ui-elements, circle-ui-react, and react-pie-menu do not forward the inputs.release value, so the downstream package job's cleanup gating (in package.yml) won't run for releases; update each call (the jobs named circle-ui-elements, circle-ui-react, and react-pie-menu) to pass the release input through by adding a with entry that forwards release: ${{ inputs.release }} so the package workflow can see and act on the release flag..github/workflows/package.yml-118-126 (1)
118-126:⚠️ Potential issue | 🟠 MajorAdd explicit shell declaration to the non-vitest coverage step.
The
testjob matrix includeswindows-latest, and this step uses POSIX shell syntax (tail -n1, command substitution) that will fail on Windows PowerShell. Sincecircle-ui-elementscalls this workflow without specifyingtest-runner, the step executes on both Ubuntu and Windows. Addshell: bashto make the behavior explicit.Suggested change
- name: Run Coverage if: inputs.test-runner != 'vitest' + shell: bash env: # Workaround for https://github.com/facebook/jest/issues/8536 TEMP: ${{ runner.temp }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/package.yml around lines 118 - 126, The "Run Coverage" workflow step uses POSIX shell features (command substitution, tail -n1) but can run on Windows runners; update the step named "Run Coverage" in the workflow to explicitly set shell: bash so the commands run under a POSIX shell on all platforms — modify the step that contains the run block with the concurrently/wait-on/coverage commands to include a top-level shell: bash entry in its step definition..github/workflows/package.yml-170-174 (1)
170-174:⚠️ Potential issue | 🟠 MajorUse
github.shaas the non-PR fallback forCHROMATIC_SHA.
github.refreturns a fully-formed ref likerefs/heads/main, not a commit hash. On non-PR invocations, this sends a branch ref instead of a commit SHA toCHROMATIC_SHA, breaking downstream attribution in Chromatic.Suggested change
- CHROMATIC_SHA: ${{ github.event.pull_request.head.sha || github.ref }} + CHROMATIC_SHA: ${{ github.event.pull_request.head.sha || github.sha }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/package.yml around lines 170 - 174, The CHROMATIC_SHA environment value currently falls back to github.ref which is a ref string, not a commit SHA; update the env assignment for CHROMATIC_SHA so it uses the PR head SHA when present (github.event.pull_request.head.sha) and otherwise uses github.sha as the non-PR fallback (replace the github.ref fallback with github.sha) so downstream Chromatic receives a commit SHA..github/workflows/launch.yml-73-78 (1)
73-78:⚠️ Potential issue | 🟠 MajorMissing npm credentials for publish step.
The release action invocation doesn't pass
npm_registry_urlornpm_auth_tokeninputs, which means the Publish step will run without proper NPM authentication. Compare with.github/workflows/release.yml(lines 77-78) which correctly passes these values.🔧 Suggested fix
- name: 'Create and publish release: ${{ vars.RELEASE_TAG }}' uses: ./.github/actions/release with: strategy: launch tag: ${{ vars.RELEASE_TAG }} dry_run_publish: ${{ vars.DRY_RUN_PUBLISH }} + npm_registry_url: ${{ vars.NPM_REGISTRY_URL }} + npm_auth_token: ${{ secrets.NPM_AUTH_TOKEN || secrets.GITHUB_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/launch.yml around lines 73 - 78, The release workflow invocation for the custom action (the step named "Create and publish release: ${{ vars.RELEASE_TAG }}" that uses ./.github/actions/release) is missing the npm_registry_url and npm_auth_token inputs; update that action call to pass npm_registry_url and npm_auth_token (e.g. wired to the same variables/secrets used in your other release workflow such as vars.NPM_REGISTRY_URL and secrets.NPM_AUTH_TOKEN) so the Publish step has proper NPM authentication.
🟡 Minor comments (2)
.github/workflows/test-commons.yml-48-50 (1)
48-50:⚠️ Potential issue | 🟡 MinorFix cache path typo: use
.yarnrc.yml.Line 50 uses
yarnrc.yml; the correct filename is.yarnrc.yml(with a leading dot). Without this, cache keys won't update when Yarn configuration changes.Proposed fix
cache-dependency-path: | yarn.lock - yarnrc.yml + .yarnrc.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test-commons.yml around lines 48 - 50, The cache key list under the workflow input "cache-dependency-path" uses the wrong filename "yarnrc.yml"; update that entry to ".yarnrc.yml" (with the leading dot) so the cache key reflects changes to the Yarn config, i.e., replace the "yarnrc.yml" entry with ".yarnrc.yml" in the cache-dependency-path block of the workflow..github/workflows/auto-approve.yml-8-9 (1)
8-9:⚠️ Potential issue | 🟡 MinorPermission mismatch:
pull-requests: readinsufficient for approving PRs.The auto-approve action needs write access to approve pull requests, but only
readpermission is declared. The action likely works becauseGH_OWNER_TOKENhas broader permissions, but the declared permissions should reflect actual requirements.🔧 Suggested fix
permissions: - pull-requests: read + pull-requests: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/auto-approve.yml around lines 8 - 9, The workflow's permissions block declares pull-requests: read but the auto-approve action needs write access to approve PRs; update the permissions entry in the workflow to set pull-requests: write (i.e., change the permissions key named "pull-requests" under permissions to "write") so the action has correct declared rights while keeping the scope minimal.
🧹 Nitpick comments (12)
.storybook/vitest.config.ts (1)
6-6: Confirm the intended precedence: currentlycommonConfig(second argument) overridesvitestConfig.In
mergeConfig, the second argument takes precedence on conflicts. If you intendvitestConfigto win instead, reverse the order:If local vitest config should override common config
-export default mergeConfig(vitestConfig, commonConfig); +export default mergeConfig(commonConfig, vitestConfig);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.storybook/vitest.config.ts at line 6, The mergeConfig call currently passes commonConfig as the second argument so commonConfig wins on conflicts; if you intend local vitestConfig to take precedence, swap the arguments to call mergeConfig(commonConfig, vitestConfig) so vitestConfig overrides common; otherwise keep as-is if commonConfig should override. Update the call referencing mergeConfig, vitestConfig, and commonConfig accordingly..husky/pre-commit (1)
1-1: Pre-commit is running a full dependency build on every commit.This is likely to make commits slow and brittle. Consider keeping pre-commit to staged-file checks only, and moving
turbo run buildto pre-push/CI.♻️ Suggested adjustment
-yarn packages/commons turbo run build && yarn g:lint-staged +yarn g:lint-staged🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.husky/pre-commit at line 1, The pre-commit hook currently runs a full build ("yarn packages/commons turbo run build && yarn g:lint-staged"), which slows commits; update the .husky/pre-commit script to only run staged-file checks (keep "yarn g:lint-staged" and remove the "yarn packages/commons turbo run build" part) and add a new pre-push hook (or CI job) that runs "yarn packages/commons turbo run build" so builds occur on push/CI instead of every commit..github/workflows/prepare.yml (1)
52-61: Verify the temporary-branch push has an explicit write-capable token.
create_branchpushes at Line 61 using the default checkout credentials, whilemerge_branchlater switches to an app token for its push path. If the repo or org defaults workflow permissions to read-only, this step becomes the failure point. Reusing the app token here, or granting explicitcontents: write, would make the branch-creation path deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/prepare.yml around lines 52 - 61, The "Push" step currently uses the default checkout credentials which may be read-only; update the workflow so the branch-creation path uses an explicit write-capable token: either add permissions: contents: write to the workflow header or change the "Check out code" step (actions/checkout) to pass a write-enabled token (e.g., token: ${{ secrets.GITHUB_TOKEN }} or the same app token used later), ensuring the "Push" step will be authenticated for pushes when running the create_branch flow..github/workflows/sync.yml (1)
24-25: Give the sync job more than two minutes of headroom.This path does a full fetch/merge plus
yarn installandyarn bootstrap.bin/bootstrap.ts, Lines 28-87, can fall back to extra installs and cache migration, so the current timeout leaves very little margin on a cold runner.Possible change
- timeout-minutes: 2 + timeout-minutes: 10🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sync.yml around lines 24 - 25, The sync job's GitHub Actions timeout is too low; update the workflow step that sets timeout-minutes for the sync job (the timeout-minutes key in .github/workflows/sync.yml) to a larger value (e.g., 15 or 20) so the full fetch/merge plus yarn install and bin/bootstrap.ts fallback work can complete without premature cancellation; adjust the timeout-minutes entry accordingly for the sync job..github/workflows/test-app.yml (1)
105-109: Skip the Vercel prebuild whenturbo-ignoresays to ignore.The ignore flag only gates the deploy step right now, so the Ubuntu leg still runs
vercel pullandvercel buildeven when no deployment will happen. That burns CI time and Vercel quota for no output.Possible change
- - name: Build with Vercel - if: matrix.os == 'ubuntu-latest' + - name: Build with Vercel + if: matrix.os == 'ubuntu-latest' && steps.deploy.outputs.ignore == 'false'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test-app.yml around lines 105 - 109, The "Build with Vercel" step currently always runs `vercel pull` and `vercel build` even when the repo is marked to skip via the turbo-ignore logic; update that step to use the same skip condition as the deploy step so it doesn't run on Ubuntu when turbo-ignore indicates no deployment. Locate the step named "Build with Vercel" and add the same conditional (or guard) the deploy step uses (the turbo-ignore check) to prevent executing `vercel pull` and `vercel build` when turbo-ignore applies, ensuring the `vercel` commands are only invoked when a real deploy is intended..github/workflows/auto-approve.yml (1)
11-13: Consider pinning action to a specific SHA for supply chain security.Other workflows in this PR pin actions to commit SHAs (e.g.,
actions/checkout@de0fac2...). For consistency and supply chain security, consider pinninghmarr/auto-approve-actionto a specific SHA.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/auto-approve.yml around lines 11 - 13, The workflow currently references the third-party action by tag "hmarr/auto-approve-action@v4"; change the uses entry to pin the action to a specific commit SHA (e.g., "hmarr/auto-approve-action@<commit-sha>") to match the project's supply-chain policy, update any other occurrences of this action if present, and commit the updated workflow file so CI uses the immutable SHA instead of the floating tag..github/actions/release/action.yml (1)
30-32: Quote the semver output to prevent word splitting.The
echostatement doesn't quote the semver output, which could cause issues if the JSON contains spaces or shell metacharacters.🛠️ Suggested fix
run: | - echo ${{ steps.versions.outputs.semver }} + echo '${{ steps.versions.outputs.semver }}' yarn turbo run build🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/release/action.yml around lines 30 - 32, The echo invocation in the release action run step uses unquoted interpolation (echo ${{ steps.versions.outputs.semver }}), which can cause word splitting or misinterpretation for semver values containing spaces or shell metacharacters; update the run step to output the semver safely by quoting the expression (e.g., use a quoted echo or printf) so that the semantics of the value from steps.versions.outputs.semver are preserved before running yarn turbo run build..github/workflows/release.yml (1)
39-42: Quote git config values for robustness.Same recommendation as other workflows - quote values to handle special characters.
🛠️ Suggested fix
- name: Configure git run: | - git config --global user.name ${{ steps.get_token.outputs.app-slug }}[bot] - git config --global user.email ${{ steps.get_user_id.outputs.user-id }}+${{ steps.get_token.outputs.app-slug }}[bot]@users.noreply.github.com + git config --global user.name '${{ steps.get_token.outputs.app-slug }}[bot]' + git config --global user.email '${{ steps.get_user_id.outputs.user-id }}+${{ steps.get_token.outputs.app-slug }}[bot]@users.noreply.github.com'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 39 - 42, The git config commands in the Configure git step should quote the interpolated values to handle special characters; update the two commands (git config --global user.name and git config --global user.email) to wrap the expanded expressions in double quotes, e.g. git config --global user.name "${{ steps.get_token.outputs.app-slug }}[bot]" and git config --global user.email "${{ steps.get_user_id.outputs.user-id }}+${{ steps.get_token.outputs.app-slug }}[bot]@users.noreply.github.com"..github/workflows/ci.yml (2)
33-34: Job name "Test" is misleading - it only runs lint.The job is named "Test" but only executes linting (line 72). Consider renaming to "Lint" for clarity, or add actual test execution.
Also applies to: 71-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 33 - 34, The workflow job currently declared as job key "test" with name "Test" only runs lint steps; update the job's human-readable name to "Lint" (or change the job key from "test" to "lint" if you prefer consistency) in the .github/workflows/ci.yml where the job is defined (job key "test" and name "Test") or alternatively add real test steps to that job so the "Test" name is accurate; ensure the step that runs lint remains unchanged if you only rename the job.
95-99: Inconsistent package manager cache configuration.The
diffjob usescache: npm(line 99) while other jobs usecache: yarn. This is intentional since the diff job only installs turbo via npm, but consider adding a comment to clarify this is deliberate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 95 - 99, The Setup Node.js environment step in the diff job currently sets cache: npm while other jobs use cache: yarn; update the diff job's actions/setup-node step (the "Setup Node.js environment" step) to add a clarifying inline comment explaining that cache: npm is intentional because this job only installs turbo via npm, so reviewers don't think it's a misconfiguration—leave the node-version and cache value unchanged and only add the comment near the cache: npm line..github/workflows/version.yml (1)
67-70: Quote git config values.Same recommendation as in
launch.yml- quote the values to handle potential special characters in app-slug.🛠️ Suggested fix
- name: Configure git run: | - git config --global user.name ${{ steps.get_token.outputs.app-slug }}[bot] - git config --global user.email ${{ steps.get_user_id.outputs.user-id }}+${{ steps.get_token.outputs.app-slug }}[bot]@users.noreply.github.com + git config --global user.name '${{ steps.get_token.outputs.app-slug }}[bot]' + git config --global user.email '${{ steps.get_user_id.outputs.user-id }}+${{ steps.get_token.outputs.app-slug }}[bot]@users.noreply.github.com'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/version.yml around lines 67 - 70, The git config lines for user.name and user.email should quote the interpolated values to handle special characters; update the two commands (the git config --global user.name and git config --global user.email invocations that use ${{ steps.get_token.outputs.app-slug }} and ${{ steps.get_user_id.outputs.user-id }}) to wrap the expanded values in quotes so the shell treats them as single arguments..github/workflows/launch.yml (1)
41-43: Quote git config values to handle special characters.If the app-slug contains spaces or special characters, the unquoted values could cause the git config commands to fail.
🛠️ Suggested fix
- name: Configure git run: | - git config --global user.name ${{ steps.get_token.outputs.app-slug }}[bot] - git config --global user.email ${{ steps.get_user_id.outputs.user-id }}+${{ steps.get_token.outputs.app-slug }}[bot]@users.noreply.github.com + git config --global user.name '${{ steps.get_token.outputs.app-slug }}[bot]' + git config --global user.email '${{ steps.get_user_id.outputs.user-id }}+${{ steps.get_token.outputs.app-slug }}[bot]@users.noreply.github.com'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/launch.yml around lines 41 - 43, The git config commands set user.name and user.email using unquoted GitHub Actions outputs which can break if app-slug or user-id contain spaces or special characters; update the commands that call git config --global user.name and git config --global user.email to wrap the interpolated values (${{ steps.get_token.outputs.app-slug }} and ${{ steps.get_user_id.outputs.user-id }} + ${{ steps.get_token.outputs.app-slug }}) in quotes so the entire string (including the trailing [bot] and +...@users.noreply.github.com portion) is treated as a single argument and special characters are handled safely.
| check-release: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 2 | ||
| environment: ${{ github.base_ref }} | ||
| permissions: read-all | ||
| steps: | ||
| - name: Get release tag | ||
| run: echo 'tag=${{ env.RELEASE_TAG }}' >> $GITHUB_OUTPUT | ||
| check-version: | ||
| needs: check-release | ||
| if: needs.check-release.outputs.tag != '' |
There was a problem hiding this comment.
Missing job outputs declaration causes check-release.outputs.tag to be undefined.
The check-release job writes to $GITHUB_OUTPUT but doesn't declare job-level outputs. The static analysis tool correctly flagged that needs.check-release.outputs.tag references an undefined property.
🐛 Proposed fix
check-release:
runs-on: ubuntu-latest
timeout-minutes: 2
environment: ${{ github.base_ref }}
permissions: read-all
+ outputs:
+ tag: ${{ steps.get_tag.outputs.tag }}
steps:
- name: Get release tag
+ id: get_tag
run: echo 'tag=${{ env.RELEASE_TAG }}' >> $GITHUB_OUTPUT📝 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.
| check-release: | |
| runs-on: ubuntu-latest | |
| timeout-minutes: 2 | |
| environment: ${{ github.base_ref }} | |
| permissions: read-all | |
| steps: | |
| - name: Get release tag | |
| run: echo 'tag=${{ env.RELEASE_TAG }}' >> $GITHUB_OUTPUT | |
| check-version: | |
| needs: check-release | |
| if: needs.check-release.outputs.tag != '' | |
| check-release: | |
| runs-on: ubuntu-latest | |
| timeout-minutes: 2 | |
| environment: ${{ github.base_ref }} | |
| permissions: read-all | |
| outputs: | |
| tag: ${{ steps.get_tag.outputs.tag }} | |
| steps: | |
| - name: Get release tag | |
| id: get_tag | |
| run: echo 'tag=${{ env.RELEASE_TAG }}' >> $GITHUB_OUTPUT | |
| check-version: | |
| needs: check-release | |
| if: needs.check-release.outputs.tag != '' |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 25-25: property "tag" is not defined in object type {}
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/version.yml around lines 15 - 25, The check-release job
writes tag to $GITHUB_OUTPUT but never declares a job-level outputs mapping, so
needs.check-release.outputs.tag is undefined; add an outputs section to the
check-release job (outputs: tag: ${{ steps.get-release.outputs.tag }}) and give
the "Get release tag" step an id (e.g., id: get-release) that emits the tag to
$GITHUB_OUTPUT (already done) so the job output can reference
steps.get-release.outputs.tag for downstream use in check-version.
| generate-version: | ||
| if: failure() && startsWith(github.head_ref, 'issue/') || startsWith(github.head_ref, 'feature/') | ||
| needs: check-version |
There was a problem hiding this comment.
Operator precedence issue causes unintended behavior for feature/* branches.
The condition failure() && startsWith(..., 'issue/') || startsWith(..., 'feature/') evaluates as (failure() && issue/) || feature/ due to && binding tighter than ||. This means generate-version will run for all feature/* PRs regardless of whether check-version failed.
🐛 Proposed fix - add parentheses for correct grouping
generate-version:
- if: failure() && startsWith(github.head_ref, 'issue/') || startsWith(github.head_ref, 'feature/')
+ if: failure() && (startsWith(github.head_ref, 'issue/') || startsWith(github.head_ref, 'feature/'))
needs: check-version📝 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.
| generate-version: | |
| if: failure() && startsWith(github.head_ref, 'issue/') || startsWith(github.head_ref, 'feature/') | |
| needs: check-version | |
| generate-version: | |
| if: failure() && (startsWith(github.head_ref, 'issue/') || startsWith(github.head_ref, 'feature/')) | |
| needs: check-version |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/version.yml around lines 42 - 44, The job
"generate-version" has an operator precedence bug in its if condition causing it
to run for all feature/* branches; update the if expression on the
generate-version job so the failure() check applies to both branch tests by
grouping the branch checks in parentheses, i.e. change the condition to
failure() && (startsWith(github.head_ref, 'issue/') ||
startsWith(github.head_ref, 'feature/')), keeping the depends-on reference to
the check-version job as-is.
| const project = setProjectAnnotations([preview.input]); | ||
|
|
||
| beforeAll(project.beforeAll); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Storybook 10.2 with @storybook/react-vite, in .storybook/vitest.setup.tswhenpreview.tsusesdefinePreview, is setProjectAnnotations([preview.input])valid, or should it besetProjectAnnotations([preview])(orbeforeAll(preview.composed.beforeAll))?
💡 Result:
With CSF Next (preview.ts uses definePreview()), don’t call setProjectAnnotations() with either preview, preview.input, or * as previewAnnotations.
Instead, import the default preview and run the preview-level hook directly:
// .storybook/vitest.setup.ts
import { beforeAll } from 'vitest';
import preview from './preview';
beforeAll(preview.composed.beforeAll);That’s the documented migration for portable-stories testing with CSF Next. [1]
If you’re still on the “classic” preview-module shape (i.e. .storybook/preview.ts exports ProjectAnnotations), then setProjectAnnotations([previewAnnotations]) is the correct pattern. [2]
Sources
[1] Storybook “CSF Next” docs (Vitest setup migration) https://storybook.js.org/docs/api/csf/csf-next
[2] Storybook “Portable stories in Vitest” docs (setProjectAnnotations) https://storybook.js.org/docs/api/portable-stories/portable-stories-vitest
🏁 Script executed:
find . -name "preview.ts" -o -name "preview.js" | head -20Repository: psychobolt/react-pie-menu
Length of output: 234
🏁 Script executed:
cat -n ./.storybook/preview.tsRepository: psychobolt/react-pie-menu
Length of output: 1091
🏁 Script executed:
cat -n ./.storybook/vitest.setup.tsRepository: psychobolt/react-pie-menu
Length of output: 565
Use beforeAll(preview.composed.beforeAll) instead of setProjectAnnotations.
The .storybook/preview.ts uses definePreview() (CSF Next), not the classic preview pattern. For CSF Next, setProjectAnnotations() should not be used. The correct setup is:
Fix
import { beforeAll } from 'vitest';
-import { setProjectAnnotations } from '@storybook/react-vite';
import preview from './preview.ts';
// This is an important step to apply the right configuration when testing your stories.
// More info at: https://storybook.js.org/docs/api/portable-stories/portable-stories-vitest#setprojectannotations
-const project = setProjectAnnotations([preview.input]);
-
-beforeAll(project.beforeAll);
+beforeAll(preview.composed.beforeAll);📝 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.
| const project = setProjectAnnotations([preview.input]); | |
| beforeAll(project.beforeAll); | |
| beforeAll(preview.composed.beforeAll); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.storybook/vitest.setup.ts around lines 8 - 10, The test setup incorrectly
uses setProjectAnnotations and project.beforeAll for a CSF Next preview; remove
the setProjectAnnotations call and stop referencing project.beforeAll, and
instead call beforeAll with the composed beforeAll from the preview export (use
preview.composed.beforeAll). Locate the usage of setProjectAnnotations and the
beforeAll(project.beforeAll) line and replace that flow so the test harness
invokes preview.composed.beforeAll directly.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/package.yml (2)
1-1: Consider adding a workflownamefor better discoverability.The workflow is missing a
name:declaration at the top level. While valid, adding a name improves visibility in the GitHub Actions UI.♻️ Suggested addition
+name: Package + on: workflow_call:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/package.yml at line 1, The workflow currently starts with the top-level key "on:" and lacks a human-readable name; add a top-level "name:" key above the existing "on:" (e.g., name: "Package CI" or another descriptive title) to improve discoverability in the GitHub Actions UI, keeping the rest of the file unchanged.
135-136: Conflicting Codecov options may mask missing coverage reports.
fail_ci_if_error: trueandhandle_no_reports_found: truetogether may create ambiguity. Thehandle_no_reports_foundoption typically prevents failure when no reports are found, which could conflict with your intent to fail on errors. Consider settinghandle_no_reports_found: falseto ensure the job fails if coverage reports are missing.♻️ Suggested fix
fail_ci_if_error: true - handle_no_reports_found: true + handle_no_reports_found: false token: ${{ secrets.CODECOV_TOKEN }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/package.yml around lines 135 - 136, The workflow currently sets both fail_ci_if_error: true and handle_no_reports_found: true which can mask missing coverage reports; update the job's Codecov step to set handle_no_reports_found: false (or remove that flag) so that when fail_ci_if_error is true the job actually fails on missing reports—change the handle_no_reports_found value in the Codecov action configuration to false to resolve the conflict.
🤖 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/package.yml:
- Around line 16-17: The workflow input "release" declared in package.yml is
never forwarded by the callers, so cleanup's check of inputs.release will always
be falsy; update each job usage that calls package.yml (the circle-ui-elements,
circle-ui-react, and react-pie-menu invocations in packages.yml) to add
"release: ${{ inputs.release }}" inside their with: blocks so the release input
is passed through to the called workflow and cleanup can run (ensure the
package.yml input name "release" remains unchanged and the cleanup job still
reads inputs.release).
- Line 174: The CHROMATIC_SHA environment variable currently falls back to
github.ref which is a ref string; update the workflow so CHROMATIC_SHA uses
github.event.pull_request.head.sha || github.sha instead of github.ref, i.e.,
change the fallback symbol from github.ref to github.sha where CHROMATIC_SHA is
set.
---
Nitpick comments:
In @.github/workflows/package.yml:
- Line 1: The workflow currently starts with the top-level key "on:" and lacks a
human-readable name; add a top-level "name:" key above the existing "on:" (e.g.,
name: "Package CI" or another descriptive title) to improve discoverability in
the GitHub Actions UI, keeping the rest of the file unchanged.
- Around line 135-136: The workflow currently sets both fail_ci_if_error: true
and handle_no_reports_found: true which can mask missing coverage reports;
update the job's Codecov step to set handle_no_reports_found: false (or remove
that flag) so that when fail_ci_if_error is true the job actually fails on
missing reports—change the handle_no_reports_found value in the Codecov action
configuration to false to resolve the conflict.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd90efd6-1cd2-4ee1-9318-d26604c93838
📒 Files selected for processing (1)
.github/workflows/package.yml
| release: | ||
| type: string |
There was a problem hiding this comment.
release input is never passed by the caller workflow.
Based on context from packages.yml (lines 36-64), none of the three job invocations (circle-ui-elements, circle-ui-react, react-pie-menu) include the release input in their with clause. This means the cleanup job (line 179) will never execute because inputs.release will always be empty/falsy.
Update packages.yml to pass the release input:
circle-ui-elements:
uses: ./.github/workflows/package.yml
with:
ref: ${{ inputs.ref }}
workspace: circle-ui-elements
path: packages/circle-ui-elements
release: ${{ inputs.release }} # Add this🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/package.yml around lines 16 - 17, The workflow input
"release" declared in package.yml is never forwarded by the callers, so
cleanup's check of inputs.release will always be falsy; update each job usage
that calls package.yml (the circle-ui-elements, circle-ui-react, and
react-pie-menu invocations in packages.yml) to add "release: ${{ inputs.release
}}" inside their with: blocks so the release input is passed through to the
called workflow and cleanup can run (ensure the package.yml input name "release"
remains unchanged and the cleanup job still reads inputs.release).
| env: | ||
| DOTENV_PRIVATE_KEY_CI: ${{ secrets.DOTENV_PRIVATE_KEY_CI }} | ||
| CHROMATIC_BRANCH: ${{ inputs.ref || github.ref_name }} | ||
| CHROMATIC_SHA: ${{ github.event.pull_request.head.sha || github.ref }} |
There was a problem hiding this comment.
Incorrect fallback for CHROMATIC_SHA — uses ref string instead of commit SHA.
github.ref is a ref string (e.g., refs/heads/main), not a commit SHA. Chromatic expects a SHA for the CHROMATIC_SHA environment variable. The fallback should use github.sha.
🐛 Proposed fix
- CHROMATIC_SHA: ${{ github.event.pull_request.head.sha || github.ref }}
+ CHROMATIC_SHA: ${{ github.event.pull_request.head.sha || github.sha }}📝 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.
| CHROMATIC_SHA: ${{ github.event.pull_request.head.sha || github.ref }} | |
| CHROMATIC_SHA: ${{ github.event.pull_request.head.sha || github.sha }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/package.yml at line 174, The CHROMATIC_SHA environment
variable currently falls back to github.ref which is a ref string; update the
workflow so CHROMATIC_SHA uses github.event.pull_request.head.sha || github.sha
instead of github.ref, i.e., change the fallback symbol from github.ref to
github.sha where CHROMATIC_SHA is set.
Summary by CodeRabbit
New Features
Updates
Chores