chore(deps): upgrade pnpm 10.33.0 → 11.13.0 - #413
Merged
Merged
Conversation
pnpm 11 stops reading pnpm-specific config from two places, both silently: 1. package.json's "pnpm" field. That held 22 security overrides pinning tmp, qs, uuid, js-yaml and others — the very packages .github/dependabot.yml ignores *because* they are pinned there. Losing them would have been a silent supply-chain regression. Moved to pnpm-workspace.yaml. 2. .npmrc's pnpm settings. `pnpm config get shamefully-hoist` returns undefined under 11, so hoisting stopped and every module apps/web imports without declaring failed to resolve. Moved to pnpm-workspace.yaml. Two latent defects surfaced and are fixed at the root: - allowBuilds held b/d/e/i/l/s/u — the sorted letters of "esbuild", a string that had been spread character-by-character. It never approved anything; esbuild, sharp and msw silently ran without their postinstall scripts. Rewritten with the real policy from onlyBuiltDependencies. - apps/web imported onnxruntime-web (owned by packages/ai-core), @tauri-apps/api (owned by apps/desktop) and @sentry/browser (transitive of @sentry/react) without declaring any of them. They resolved only through shamefully-hoist. Now declared explicitly. Verified: pnpm install clean, check:lockfile OK, tsc --noEmit 0 errors, all 22 overrides present in the regenerated lockfile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ored The document claimed the .npmrc pnpm settings remain valid under pnpm 11. They do not: pnpm config get shamefully-hoist returns undefined, hoisting stops, and the phantom imports in apps/web break. Replaced the claim with what the migration actually found. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR upgrades the repository to pnpm 11.13.0, relocates pnpm configuration, aligns bootstrap scripts, documents staged toolchain changes, resolves an audit backlog item, adds web dependencies, and centralizes ONNX runtime loading in web workers. ChangesToolchain migration
Web runtime updates
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Docker |  |
Jul 14, 2026 8:06a.m. | Review ↗ |
| JavaScript | |
Jul 14, 2026 8:06a.m. | Review ↗ |
| Python | |
Jul 14, 2026 8:06a.m. | Review ↗ |
| Rust | |
Jul 14, 2026 8:06a.m. | Review ↗ |
| Shell | |
Jul 14, 2026 8:06a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
docs/toolchain-update.md (1)
110-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare languages on fenced code blocks.
Add a language marker such as
textto both fences; markdownlint reports MD040 for these blocks.Proposed fix
-``` +```textAlso applies to: 138-141
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/toolchain-update.md` around lines 110 - 113, Update both fenced code blocks in docs/toolchain-update.md, including the additional block around the referenced later section, to declare an explicit language such as text. Preserve the existing dependency content unchanged.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/toolchain-update.md`:
- Line 229: Update the documentation reference in the toolchain configuration
note to point to strictPeerDependencies in pnpm-workspace.yaml instead of
strict-peer-dependencies=false in .npmrc, while preserving the guidance about
manually checking peer ranges and considering enabling strict peer dependency
enforcement.
- Line 83: Update the quotation in the pnpm.overrides table entry so the phrase
“via pnpm.overrides gepinnt” uses matching German opening and closing quotation
marks, while leaving the surrounding documentation unchanged.
- Line 196: Replace the angle-bracket placeholder in the test command with a
shell-safe filename placeholder such as path/to/test-file.ts or $TEST_FILE,
while preserving the existing pnpm filter and test:run invocation.
- Line 194: Update the documented pnpm install command in the toolchain update
instructions to remove --frozen-lockfile when regenerating the lockfile after
the pnpm 11 bump, while preserving frozen-lockfile usage only for subsequent
verification.
- Around line 34-45: Update the migration plan in docs/toolchain-update.md to
reflect that package.json already pins pnpm@11.13.0, removing the outdated
10.33.0 pending-upgrade state. Revise the affected current-state, file-count,
and plan sections to list package.json, pnpm-workspace.yaml, .npmrc, and
pnpm-lock.yaml, or explicitly label the existing values as the pre-migration
baseline.
- Line 90: Correct the phantom-dependency count in the paragraph describing the
undeclared imports: it currently names only onnxruntime-web, `@tauri-apps/api`,
and `@sentry/browser`, so change “Vier Phantom-Dependencies” to three unless a
fourth package is explicitly added to the list.
- Line 216: Update the pnpm 11 rollback instructions to explicitly move
pnpm-specific settings from pnpm-workspace.yaml back into .npmrc, restore the
previous package.json pnpm block, and regenerate the lockfile after reverting
packageManager to pnpm@10.33.0. Keep the existing Corepack guidance.
In `@package.json`:
- Line 6: Update the hard-coded Corepack pnpm version in
scripts/cursor-cloud-update.sh, scripts/setup-windows.ps1, and
scripts/windows-doctor.mjs from pnpm@10.33.0 to pnpm@11.13.0, matching the
packageManager declaration in package.json.
---
Nitpick comments:
In `@docs/toolchain-update.md`:
- Around line 110-113: Update both fenced code blocks in
docs/toolchain-update.md, including the additional block around the referenced
later section, to declare an explicit language such as text. Preserve the
existing dependency content unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: cd59b5ac-f326-4090-8c33-c7eb9bec1885
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.npmrcapps/web/package.jsondocs/toolchain-update.mdpackage.jsonpnpm-workspace.yaml
…ound - Section 1 is the pre-migration baseline, not the current state (pnpm 11 is now pinned). - The pnpm bump spans four files, not two. - Rollback must restore package.json's pnpm block and .npmrc, not just packageManager — a half rollback leaves the 22 overrides pointing nowhere. - Three phantom deps, not four; strictPeerDependencies now lives in pnpm-workspace.yaml; frozen install cannot regenerate a lockfile. All raised by CodeRabbit on #413. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 6: Update the Corepack activation commands in
scripts/cursor-cloud-update.sh, scripts/setup-windows.ps1, and
scripts/windows-doctor.mjs from pnpm@10.33.0 to pnpm@11.13.0, matching the
packageManager pin while preserving each script’s existing setup flow.
In `@pnpm-workspace.yaml`:
- Around line 55-57: Rename the pnpm audit configuration key from
auditConfig.ignoreCves to auditConfig.ignoreGhsas, preserving the existing
ignored advisory entry under the renamed key.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b471507-78ac-4454-8cee-4449b0fb3515
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
.npmrcapps/web/package.jsondocs/toolchain-update.mdpackage.jsonpnpm-workspace.yaml
…orepack pins Two findings from CodeRabbit on this PR, both real: 1. auditConfig.ignoreCves does not exist in pnpm 11 (it is ignoreGhsas, and it takes GHSA IDs). Carried over unchanged, the suppression would have stopped working with no error. Investigating it showed the suppression is obsolete — and was dangerous. The entry suppressed CVE-2026-41242 = GHSA-xq3m-2v4x-88gg, "Arbitrary code execution in protobufjs", severity CRITICAL — while AUDIT_BACKLOG S-07 recorded it as a low-severity false positive. The overrides pin protobufjs >=8.2.0, which resolves to 8.7.0 (patched), so the advisory no longer applies: both CI audits (--audit-level=critical --prod and --audit-level=high) pass with no auditConfig at all. Removed rather than migrated. Keeping it would have silently swallowed the next genuine protobufjs advisory. S-07 updated to Resolved with the correction. 2. scripts/setup-windows.ps1, scripts/windows-doctor.mjs and scripts/cursor-cloud-update.sh still pinned corepack to pnpm@10.33.0. Synced to 11.13.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/setup-windows.ps1`:
- Line 18: Update the Windows bootstrap around Test-Command 'pnpm' so it
validates the installed pnpm version and enforces 11.13.0 rather than checking
presence alone. Ensure corepack prepare pnpm@11.13.0 --activate runs when pnpm
is missing or has a different version, so the subsequent pnpm install uses the
required release.
- Line 18: Save scripts/setup-windows.ps1 as UTF-8 with BOM, preserving its
existing content and commands so Windows PowerShell correctly handles the
non-ASCII characters.
In `@scripts/windows-doctor.mjs`:
- Line 46: Update the diagnostic string in bad() for the “pnpm not found” case
to replace the non-ASCII em dash with the ASCII sequence "--", keeping the
JavaScript source ASCII-only.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7dd5b274-02ad-4184-a615-45d508ccc991
📒 Files selected for processing (5)
docs/AUDIT_BACKLOG.mdpnpm-workspace.yamlscripts/cursor-cloud-update.shscripts/setup-windows.ps1scripts/windows-doctor.mjs
💤 Files with no reviewable changes (1)
- pnpm-workspace.yaml
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
scripts/windows-doctor.mjs (1)
46-46: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the changed diagnostic ASCII-only.
The em dash in this
.mjsstring violates the repository rule. Replace it with--.Proposed fix
-else bad('pnpm not found — corepack enable && corepack prepare pnpm@11.13.0 --activate') +else bad('pnpm not found -- corepack enable && corepack prepare pnpm@11.13.0 --activate')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/windows-doctor.mjs` at line 46, Update the diagnostic string in bad() for the “pnpm not found” case to replace the non-ASCII em dash with the ASCII sequence “--”, preserving the rest of the message unchanged.Source: Coding guidelines
scripts/setup-windows.ps1 (1)
15-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce pnpm 11.13.0 instead of checking only presence.
When any
pnpmexecutable is already onPATH, lines 15-18 skip Corepack entirely; an older global pnpm can then run the install. Comparepnpm --versionwith11.13.0and activate Corepack when it differs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/setup-windows.ps1` around lines 15 - 18, Update the pnpm setup condition around Test-Command so it also runs when the available pnpm version is not 11.13.0. Check pnpm --version, compare it with the required version, and enable/prepare Corepack whenever pnpm is missing or mismatched.
🧹 Nitpick comments (1)
pnpm-workspace.yaml (1)
9-9: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftAdd a runtime dependency guard before relying on
shamefullyHoist.
shamefullyHoist: trueexposes hoisted packages to application code, allowing undeclared runtime dependencies to resolve accidentally. pnpm documents this as a source of phantom dependencies. (pnpm.io) Add a CI audit for runtime imports or disable hoisting after all consumers are explicitly declared.Based on learnings:
shamefullyHoist: truecan hide undeclared runtime dependencies, so packages used at runtime must be declared or validated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pnpm-workspace.yaml` at line 9, Add a CI dependency audit covering runtime imports before relying on the shamefullyHoist setting, ensuring every package imported by application code is explicitly declared in its consumer package manifest; alternatively, remove shamefullyHoist after completing those declarations, while preserving valid runtime dependency resolution.Sources: Learnings, MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/package.json`:
- Line 55: Remove onnxruntime-web from the apps/web dependencies and add it to
`@cannaguide/ai-core` optionalDependencies. Update web code to consume the runtime
through `@cannaguide/ai-core`, preserving existing behavior and the package’s
optional dependency handling.
In `@docs/toolchain-update.md`:
- Around line 112-117: Add the appropriate language identifier, such as text, to
the opening fences of both code blocks in docs/toolchain-update.md, including
the blocks containing the TypeScript peer dependency examples near lines 112 and
140, while leaving their contents unchanged.
In `@scripts/cursor-cloud-update.sh`:
- Line 68: Update the Corepack activation command in the script so failures are
not suppressed, and verify via pnpm --version that the active version is exactly
11.13.0 before continuing to pnpm install; terminate with a clear failure if
either condition is unmet.
---
Duplicate comments:
In `@scripts/setup-windows.ps1`:
- Around line 15-18: Update the pnpm setup condition around Test-Command so it
also runs when the available pnpm version is not 11.13.0. Check pnpm --version,
compare it with the required version, and enable/prepare Corepack whenever pnpm
is missing or mismatched.
In `@scripts/windows-doctor.mjs`:
- Line 46: Update the diagnostic string in bad() for the “pnpm not found” case
to replace the non-ASCII em dash with the ASCII sequence “--”, preserving the
rest of the message unchanged.
---
Nitpick comments:
In `@pnpm-workspace.yaml`:
- Line 9: Add a CI dependency audit covering runtime imports before relying on
the shamefullyHoist setting, ensuring every package imported by application code
is explicitly declared in its consumer package manifest; alternatively, remove
shamefullyHoist after completing those declarations, while preserving valid
runtime dependency resolution.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5cac2648-03e8-4ee9-b16a-c4d3ceeed286
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.npmrcapps/web/package.jsondocs/AUDIT_BACKLOG.mddocs/toolchain-update.mdpackage.jsonpnpm-workspace.yamlscripts/cursor-cloud-update.shscripts/setup-windows.ps1scripts/windows-doctor.mjs
This was referenced Jul 14, 2026
…i optional apps/web declared onnxruntime-web as a hard dependency while ai-core already owned it as an optional one and re-exports it lazily via @cannaguide/ai-core/ml. The workers imported the package directly, so the runtime was pinned into the web app's dependency graph and could not be skipped in lite installs. Both workers now call loadOnnxRuntime() from ai-core, and OrtModule is derived from that loader's return type instead of a direct typeof import. @tauri-apps/api moves to optionalDependencies: it is only ever reached through guarded dynamic imports and must not break a plain web install. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Test-Command 'pnpm' only proves that *a* pnpm exists. An older global install satisfied it, skipped corepack entirely, and then ran the install with the wrong pnpm. Compare the reported version against the required one and activate via corepack whenever they differ. Also drop a non-ASCII em dash from windows-doctor's output, which renders as mojibake in the default Windows console code page. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/setup-windows.ps1`:
- Around line 18-26: Update the pnpm setup branch around $currentPnpm and the
corepack enable/prepare calls to handle Node 25, which may not provide the
corepack executable. Either reject Node 25 while preserving supported Node 24+
behavior, or bootstrap userland Corepack before invoking it; ensure installation
uses the required pnpm version explicitly, including the install path if
applicable.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e847cbf-1bfe-4220-a422-999b7f15ff6c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/web/package.jsonapps/web/workers/hydroForecastWorker.tsapps/web/workers/visionInferenceWorker.tsscripts/setup-windows.ps1scripts/windows-doctor.mjs
Resolves the apps/web/package.json conflict between main's dependency bumps and this branch's dependency reshuffle. main (#417) dropped @sentry/browser from apps/web while five source files still import it directly (privacyService, workerTelemetryService, cloudTtsService, porcupineWakeWordService, usePredictiveAnalytics). It only kept resolving because pnpm-workspace.yaml sets shamefullyHoist: true, so the package was hoisted in as a transitive dependency of @sentry/react. Re-declare it explicitly at ^10.65.0 to match @sentry/react rather than rely on hoisting. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The assertion loop ran two expect() calls per tensor element. The tensor is 3 x 224 x 224, so that is ~301k matcher invocations for a single check, taking ~37s and blowing past the 30s default timeout on a loaded machine. Reduce to min/max in a plain loop and assert twice. Same guarantee, no timeout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/workers/visionInferenceWorker.test.ts`:
- Around line 100-108: Update the tensor validation loop in the vision inference
test to stop treating missing values as zero and explicitly track any non-finite
or missing tensor entries, including NaN. Assert that no such invalid values
were encountered before performing the existing min/max bound checks.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 885eaa9d-308b-4a7c-875d-ca09cb685a68
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
apps/web/package.jsonapps/web/workers/visionInferenceWorker.test.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/package.json
- vision test: the min/max reduction I introduced let NaN through. NaN loses every comparison, so an all-NaN tensor left min=Infinity/max=-Infinity and both bounds assertions passed. Scan for the first non-finite or out-of-range element instead, and assert on its index. - cursor-cloud-update.sh: `corepack prepare ... || true` opted out of set -e, so a failed activation fell through to pnpm install with whatever pnpm was already on PATH. Fail fast and verify the active version. - setup-windows.ps1: Node 25 no longer bundles Corepack, so `corepack enable` is not guaranteed to exist. Install it from npm when missing, and verify the activation actually took. - toolchain-update.md: tag the two bare fences as `text` (markdownlint MD040). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
✅ Action performedReview finished.
|
DeepSource (JS-0111) flagged the single-letter `v`. Spell it `value`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/setup-windows.ps1 (1)
25-31: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPin the Corepack fallback and avoid the legacy activation command.
corepack@latestmakes this bootstrap non-reproducible, whilecorepack prepareis the older activation spelling. Current Corepack documentation usescorepack install --global ...; pin a tested Corepack version and use that command, or verifyprepareagainst every supported Corepack release. (github.com)Proposed adjustment
- npm install -g corepack@latest + npm install -g corepack@<tested-version> ... - corepack prepare "pnpm@$requiredPnpm" --activate + corepack install --global "pnpm@$requiredPnpm"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/setup-windows.ps1` around lines 25 - 31, Update the Corepack fallback in the setup script to install a tested, pinned Corepack version instead of corepack@latest, and replace the legacy corepack prepare invocation with corepack install --global for the required pnpm version. Keep corepack enable and the existing requiredPnpm value unchanged.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/setup-windows.ps1`:
- Around line 25-31: Update the Corepack fallback in the setup script to install
a tested, pinned Corepack version instead of corepack@latest, and replace the
legacy corepack prepare invocation with corepack install --global for the
required pnpm version. Keep corepack enable and the existing requiredPnpm value
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f514c6b6-60d4-42be-a718-ec317d8efd42
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/web/package.jsonapps/web/workers/visionInferenceWorker.test.tsdocs/toolchain-update.mdpackage.jsonscripts/cursor-cloud-update.shscripts/setup-windows.ps1
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- apps/web/package.json
- docs/toolchain-update.md
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sted packages pnpm 11 turns on a minimumReleaseAge quarantine by default, and enforces it against the lockfile — so it fires even on --frozen-lockfile in CI. This PR is what activates it (main is still on pnpm 10), which is why every job here died in the install step with ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION on 18 entries that main itself had merged hours earlier. Keep the quarantine — it is the window in which a hijacked maintainer account gets caught — but state it explicitly instead of inheriting an invisible default. Exempt the packages Dependabot had just merged. Each is first-party, built by its own repo's CI, and carries a verified npm provenance attestation: turbo, @turbo/* -> vercel/turborepo @typescript-eslint/* -> typescript-eslint/typescript-eslint postcss -> postcss/postcss Provenance rules out precisely the attack the quarantine defends against: a version published with a stolen token from outside the project's CI. The exclusion matches on name, not on provenance, so it is deliberately short. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DeepSource JS-R1005: the test callback reached a cyclomatic complexity of 7 -- four conditions in the scan, the loop, and a ternary in the failure message. Move the validity check into isNormalisedPixel() at module scope and drop the ternary. Same guarantee, including the NaN case, which is why this tests for validity rather than reducing to min/max. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 14, 2026
qnbs
added a commit
that referenced
this pull request
Jul 14, 2026
## What One line: git ignores `.claude/worktrees/`. ## Why `.claude/worktrees/` holds **full checkouts of this repo, inside the repo**. An unqualified `git add -A` in the main checkout therefore stages a worktree as an *embedded git repository* and commits a bare gitlink instead of the actual change: ```text warning: adding embedded git repository: .claude/worktrees/pr413-cc hint: You've added another git repository inside your current repository. ``` This happened while working on #413: the resulting commit contained nothing but `.claude/worktrees/pr413-cc | 1 +` and none of the real changes. It was reverted non-destructively, but the trap is open to anyone using Claude Code on this repo. ## Scope Deliberately limited to the `worktrees/` subdirectory, so checked-in `.claude` configuration (`settings.json`, agents, skills) stays tracked. Verified: ```text $ git check-ignore -v .claude/worktrees/foo .gitignore:55:.claude/worktrees/ .claude/worktrees/foo $ git check-ignore -v .claude/settings.json (no match — correct) ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated project ignore rules to prevent temporary Claude Code worktrees from being accidentally staged or committed. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
qnbs
added a commit
that referenced
this pull request
Jul 14, 2026
This branch was cut from chore/pnpm-11 before #413 merged, so it carried a duplicate of the entire pnpm 11 migration -- the Windows scripts, pnpm-workspace.yaml, docs/toolchain-update.md, .npmrc. All of that is on main now, so every conflicted file takes main's version. What is left is the branch's actual contribution: eslint-config-prettier 9.1.2 -> 10.1.8, one line in the root package.json plus its lockfile entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
qnbs
added a commit
that referenced
this pull request
Jul 14, 2026
## What `e2e-cross-browser` no longer runs on pull requests. It still runs on `main`, via `workflow_dispatch`, and on any PR carrying the new **`e2e-cross-browser`** label. The **Chromium** E2E job (`e2e`) — the one that actually gates — is unchanged and still runs on every PR. ## Why On a PR this job was pure cost with no leverage: - It is **Firefox-only**. WebKit is already excluded in the workflow (too slow/flaky on GitHub runners). - It carries `continue-on-error: true`, so it **cannot block a merge**. Nobody is allowed to gate on its result. - It still occupies **two runners per PR**, with a 90-minute ceiling. Cross-browser regressions are almost always introduced by a browser or Playwright update rather than by a single PR's diff. `main` and `workflow_dispatch` catch them one build later, at no cost to PR turnaround. ## Measurement From the last CI run on #413: `Quality Gates` dominates at **515 s**; everything else is under 80 s. The four Playwright jobs per PR are the largest avoidable block — this halves them. ## Verification - `ci.yml` parses; the `if` expression folds correctly onto one line. A syntax error there would silently *skip* the job rather than fail, so this was checked explicitly. - The `e2e` job still has **no** `if`, so it runs on every PR exactly as before. - The `e2e-cross-browser` label exists in the repo. - This PR is its own proof: `E2E Cross-Browser` must **not** appear in its checks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated cross-browser (Firefox) end-to-end testing to run on pull requests only when explicitly enabled via a label. * Kept automatic execution for non–pull request events after quality checks pass. * Documented that the cross-browser job is advisory and won’t block the release pipeline. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
qnbs
added a commit
that referenced
this pull request
Jul 14, 2026
The base branch now carries main (post-#413), which brought its own minimumReleaseAgeExclude. Git merged both copies in, producing a duplicate `minimumReleaseAgeExclude` key -- a YAML error, and one that silently resolved to the *second* block, dropping the turbo and postcss exemptions and putting CI straight back into the release quarantine. Kept main's list: its `@typescript-eslint/*` glob is a superset of the ten pinned 8.64.0 entries this branch had added, so nothing is lost. snyk: false -> true, per review. snyk's postinstall (`node wrapper_dist/bootstrap.js exec`) is what fetches the actual CLI binary. Denying it leaves `snyk` on PATH but non-functional, silently breaking `pnpm run security:snyk` and therefore `security:scan`. Verified against the installed package's manifest. @google/genai stays denied: its only install script is `preinstall: echo 'preinstall: no-op'`, so allowing it would buy nothing and grant a pure-JS SDK the right to execute code at install time. This does not disable the Gemini integration -- allowBuilds governs install-time scripts, not runtime features. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
qnbs
added a commit
that referenced
this pull request
Jul 14, 2026
Stacked on #413 (both touch `pnpm-lock.yaml`). Retargets to `main` automatically once #413 merges. ## Why this one is safe while ESLint 10 is not | Package | `peerDependencies.eslint` | ESLint 10? | |---|---|---| | **eslint-config-prettier@10.1.8** | `>=7.0.0` | ✅ — and works on the current ESLint 9 | | eslint-plugin-react@7.37.5 | `… \|\| ^9.7` | ❌ caps at 9 | | eslint-plugin-import@2.32.0 | `… \|\| ^9` | ❌ caps at 9 | | eslint-plugin-jsx-a11y@6.10.2 | `… \|\| ^9` | ❌ caps at 9 | So `eslint-config-prettier` can go to 10 **without** waiting for the ESLint 10 major, which stays blocked by three plugins (see `docs/toolchain-update.md` §3). ## Breaking-change exposure: none v10's breaking changes are removed/renamed individual rule entries. `eslint.config.js:194` consumes only the default export (`prettierConfig`, appended last to disable formatting-conflicting rules) — it never references individual rule names from the package. ## Verification ``` pnpm exec eslint apps/web/components/common/Toast.tsx exit 0 pnpm run lint:scopes exit 0 (blocking CI gate) ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated the code formatting configuration dependency to a newer version. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
qnbs
added a commit
that referenced
this pull request
Jul 14, 2026
## What Adds `check:phantom-deps`, a CI gate that fails when a workspace imports a package it does not declare — and fixes every violation it found. ## Why `pnpm-workspace.yaml` sets `shamefullyHoist: true`, which flattens transitive dependencies into the root `node_modules`. An **undeclared import therefore still resolves** — until the package that dragged it in drops it, or hoisting is turned off. Then it breaks at runtime, with nothing in the diff to explain why. This is not hypothetical. In #413, `@sentry/browser` was removed from `apps/web` by a dependency bump while **five source files still imported it directly**. It kept working purely because `@sentry/react` happened to pull it in. CodeRabbit flagged the same class of risk on `shamefullyHoist` in that PR; this is the guard it asked for. Running it against `main` found **five more**. ## What the guard found | Package | Imported by | Declared in | Resolution | |---|---|---|---| | `@xenova/transformers` | `apps/web` (4 files) | `packages/ai-core` (optional) | Routed through `ai-core`'s `loadTransformers()` | | `@mlc-ai/web-llm` | `apps/web` | `packages/ai-core` (optional) | Routed through `ai-core`'s `loadWebLlm()` | | `@tauri-apps/plugin-dialog` | `apps/web` | `apps/desktop` | Declared as an `optionalDependency` of `apps/web` | | `@tauri-apps/plugin-notification` | `apps/web` | `apps/desktop` | Declared as an `optionalDependency` of `apps/web` | | `@tensorflow/tfjs` | `apps/web` | **nowhere** | Dead path removed — see below | ## The TensorFlow finding `predictYield()` dynamically imported `@tensorflow/tfjs` once a grower had **10 or more recorded harvests**. That package is declared in no `package.json` in this repo and is not installed, so the import could only ever reject. **The feature threw for exactly the users who had used the app the longest.** Three layers of defence missed it: - the **typecheck** was blinded by an ambient `declare module '@tensorflow/tfjs'` in `optional-deps.d.ts`, - the **tests** only exercise the zero-sample path, which returns before the import, - the **bundler** never resolved it, because the import is dynamic. The path is removed rather than repaired: making it work meant shipping a multi-megabyte runtime to train a three-layer network on a handful of samples. `predictYield()` is now heuristic throughout — which is what every user was already getting, minus the exception. Recorded harvests still raise confidence. This also deletes the ambient declaration and its **11 `no-explicit-any` suppressions**. ## How the guard works For each workspace it extracts external imports from the sources and checks them against **that workspace's own `package.json`** — never against whatever happens to sit in `node_modules`. Precision measures, since a noisy gate gets disabled: - **comments are stripped first**, so a commented-out import or a JSDoc `@type {import('x')}` is not counted as a dependency; - **specifiers are validated** against a bare-specifier pattern, so a regex that wandered across a line boundary reports nothing rather than a nonsense package name; - **self-references** through a package's own `exports` are ignored; - test, config and setup files may import `devDependencies`; runtime files may not — hoisting hides that just as effectively. It runs in the `quality` job **before** the test suite: it is a sub-second static check and should fail early. ## Verification - `pnpm run check:phantom-deps` → **OK**, zero findings, zero false positives. - `turbo run typecheck` → **5/5 green**. - `yieldPredictionService` tests → green. - ESLint on every changed source file → **0 errors**. - Lockfile diff is **exactly 6 lines** (the two Tauri plugins); no incidental version bumps. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional desktop dialog and notification support. * **Updates** * Yield forecasting now uses heuristic projections only; ML-specific status messaging was removed and the forecast UI/explanations were refreshed across supported languages. * Forecast confidence is now displayed as a percentage (and the “no history” vs heuristic confidence text was clarified). * Tech-stack credits now reference ONNX Runtime Web only. * **Tests** * Updated yield prediction tests to assert heuristic-only behavior. * **Chores** * CI now enforces declared runtime imports. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
qnbs
added a commit
that referenced
this pull request
Jul 14, 2026
This branch predates the pnpm 11 migration (#413) by 37 commits, so it still carried package.json's `pnpm.overrides` block -- which pnpm 11 no longer reads. Its only addition there, `@babel/core: >=7.29.6`, already lives on main in pnpm-workspace.yaml, so package.json and the lockfile take main's version. check-file-budget.mjs also takes main's version. Both branches hit the same problem -- touching a locale file failed the budget gate -- and solved it differently: this branch blacklisted `locales/` and `data/strains/`, main scoped the changed-file path to SCAN_GLOBS. The whitelist subsumes the blacklist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
4 tasks
qnbs
added a commit
that referenced
this pull request
Jul 14, 2026
Stacked on #415 → #413. Retargets automatically as those merge. ## Why 6.0.3 and not 7.0.2 `typescript-eslint@8.64` — the newest there is, no v9 exists — declares: ``` peerDependencies.typescript: ">=4.8.4 <6.1.0" ``` TS 7 is outside that range. Because `.npmrc` sets `strict-peer-dependencies=false`, `pnpm install` would **not** stop it: it would install cleanly and the parser would fail at runtime, taking the blocking `lint:changed` and `lint:scopes` gates with it. TS 6.0.3 is the transitional release and sits inside the supported range. ## The only two source changes **1. `apps/web/tsconfig.json` — removed `baseUrl`** TS 6 reports it as deprecated (`TS5101`, removed in 7.0) and offers `"ignoreDeprecations": "6.0"` to silence the error. Silencing only defers the problem to the TS 7 upgrade. `paths` have resolved relative to `tsconfig.json` since TS 4.1, and the mapping is already relative: ```json "paths": { "@/*": ["./*"] } ``` So `baseUrl: "."` was redundant. Removing it **clears a TS 7 blocker** rather than postponing it. **2. `webGpuService.ts` — removed an `eslint-disable`** TS 6's narrowing made the `no-unsafe-type-assertion` suppression unnecessary; ESLint now reports the directive itself as unused. **One suppression fewer, not one more** — the repo's suppression ratchet stays honest. ## Verification ``` turbo run typecheck 5/5 packages successful pnpm run typecheck exit 0 (root) pnpm run lint:scopes exit 0 (blocking gate) zero-any count 0 ``` The unit suite, coverage thresholds and bundle budget run in CI — a TS major can change emit, so the bundle-budget gate is the one to watch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated the TypeScript development tooling. * Refined workspace configuration and build permissions. * Improved configuration formatting and readability. * Removed an obsolete compiler setting and an unnecessary lint suppression. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Not the one-liner it looked like
pnpm 11 stops reading pnpm config from two locations, both silently — no error, just an easily-missed warning.
pnpm.overrides(22 pins)package.jsontmp,qs,uuid,js-yaml,undici,ws, … vanish. These are the exact packages.github/dependabot.ymlignores because they are pinned here. Result: neither Dependabot nor override — a silent supply-chain regression.pnpm.auditConfig/pnpm.onlyBuiltDependenciespackage.jsononlyBuiltDependenciesis gone; replaced byallowBuildsinpnpm-workspace.yaml.shamefully-hoist,strict-peer-dependencies,auto-install-peers.npmrcpnpm config get shamefully-hoist→undefined)apps/webimports without declaring fails to resolve.All three are migrated to
pnpm-workspace.yaml.Two latent defects surfaced — fixed at the root, not worked around
1.
allowBuildswas corrupt data.b, d, e, i, l, s, uare the sorted letters of "esbuild" — a string that had been spread character-by-character. It never approved anything.esbuild,sharpandmswhave been running without their postinstall scripts. Rewritten with the real policy fromonlyBuiltDependencies, with everything else denied explicitly.2. Four phantom dependencies in
apps/web.apps/webimports packages it never declared; they resolved only throughshamefully-hoist:onnxruntime-webpackages/ai-core@tauri-apps/apiapps/desktop@sentry/browser@sentry/reacttscsurfaced this as 12 ×TS2307: Cannot find modulethe moment hoisting stopped. Now declared explicitly inapps/web/package.json— the imports are real, the declarations were missing.Verification (local, before push)
CI must confirm:
qualityjob (install → typecheck → zero-any → coverage → build → all gates).Docs
docs/toolchain-update.md— full toolchain inventory, and why TypeScript 7 and ESLint 10 are blocked (typescript-eslint@8.64peers:typescript: ">=4.8.4 <6.1.0";eslint-plugin-react/-import/-jsx-a11yall cap at ESLint 9).🤖 Generated with Claude Code
Summary by CodeRabbit