update rust and node dependencies - #925
Conversation
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds deprecation-warning suppressions in Rust plugin code, updates UI type definitions and test helpers, adjusts PDF.js cleanup and rendering, and bumps package versions in the UI and website manifests. ChangesMaintenance updates across Rust and UI code
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
crates/mesh-llm-host-runtime/src/plugin/mcp.rs (3)
454-454: 📐 Maintainability & Code Quality | 🔵 TrivialAdd inline comment documenting the rmcp deprecation scope.
Same consideration as
handle_requestat Line 382. Document the specific rmcp deprecation being suppressed and the migration/ removal plan.+ // TODO: Remove when rmcp <version> deprecated APIs are migrated (see PR #<number>) #[allow(deprecated)]🤖 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 `@crates/mesh-llm-host-runtime/src/plugin/mcp.rs` at line 454, The #[allow(deprecated)] on the MCP plugin path needs an inline comment explaining the specific rmcp deprecation being suppressed and the expected migration/removal plan, matching the documentation style used in handle_request. Update the surrounding code in mcp.rs near the relevant rmcp usage so the suppression is clearly scoped and future maintainers can see what should replace it and when it can be removed.Source: Coding guidelines
382-382: 📐 Maintainability & Code Quality | 🔵 TrivialAdd inline comment documenting the rmcp deprecation scope.
The
#[allow(deprecated)]suppression is justified for rmcp API compatibility, but future maintainers need context on what is deprecated and when it can be removed. As per coding guidelines, silencing warnings requires a clear reason documented in code. As per coding guidelines,Do not silence Rust warnings with #[allow(...)] unless there is a clear reason and the developer has asked for that tradeoff.The PR explicitly requests this tradeoff, but the code should still document the specific rmcp APIs involved and the migration path.+ // TODO: Remove when rmcp <version> deprecated APIs are migrated (see PR #<number>) #[allow(deprecated)]🤖 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 `@crates/mesh-llm-host-runtime/src/plugin/mcp.rs` at line 382, Add a brief inline comment directly above the `#[allow(deprecated)]` in `mcp.rs` explaining that the suppression is only for rmcp compatibility, naming the deprecated rmcp API surface being used in `McpHostPlugin`/related rmcp calls and noting the condition for removal once the replacement API is adopted. Keep the allow scoped as narrowly as possible and document the migration path so future maintainers know exactly why the warning is silenced and when it can be deleted.Source: Coding guidelines
1335-1335: 📐 Maintainability & Code Quality | 🔵 TrivialAdd inline comment documenting the rmcp
ServerHandlertrait deprecation.This implements a deprecated
rmcp::ServerHandler::get_infotrait method; document the rmcp version and whether this is a temporary backward-compatibility measure or a permanent protocol-level requirement. As per coding guidelines,Do not silence Rust warnings with #[allow(...)] unless there is a clear reason and the developer has asked for that tradeoff.The PR requests this tradeoff, but the code should preserve the rationale for future protocol evolution decisions.+ // TODO: Remove when rmcp ServerHandler no longer requires get_info or we bump minimum rmcp version #[allow(deprecated)]🤖 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 `@crates/mesh-llm-host-runtime/src/plugin/mcp.rs` at line 1335, The deprecated rmcp ServerHandler::get_info usage is currently silenced without explaining the tradeoff. In mcp.rs near the ServerHandler implementation, add an inline comment documenting the rmcp version/deprecation context and whether this is a temporary backward-compatibility shim or a permanent protocol requirement. Keep the rationale close to the #[allow(deprecated)] on get_info so future maintainers can understand why the warning is intentionally suppressed.Source: Coding guidelines
crates/mesh-llm-plugin/src/helpers.rs (1)
188-188: 📐 Maintainability & Code Quality | 🔵 TrivialAdd inline comment documenting the rmcp deprecation scope.
The
#[allow(deprecated)]onplugin_server_info_fullsuppresses warnings from deprecatedrmcpbuilder APIs used in the function body. Document the specific rmcp APIs and the plan for migration or removal. As per coding guidelines,Do not silence Rust warnings with #[allow(...)] unless there is a clear reason and the developer has asked for that tradeoff.The PR requests this tradeoff, but the code should preserve the rationale for future maintainers, especially at this protocol boundary.+ // TODO: Remove when rmcp <version> deprecated builder APIs are migrated (see PR #<number>) #[allow(deprecated)]🤖 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 `@crates/mesh-llm-plugin/src/helpers.rs` at line 188, The #[allow(deprecated)] on plugin_server_info_full is suppressing warnings from deprecated rmcp builder APIs inside this function, so add an inline comment explaining exactly which rmcp calls are affected and why the allowance is temporary. Reference plugin_server_info_full and the rmcp builder usage in its body, and note the intended migration/removal plan so future maintainers understand the scope of the deprecation tradeoff at this protocol boundary.Source: Coding guidelines
🤖 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 `@crates/mesh-llm-ui/src/lib/pdf.ts`:
- Line 34: The teardown path in DestroyablePdfDocument is using the wrong
object, so cleanup is skipped. Update the PDF handling in pdf.ts to stop relying
on doc.destroy?.() and instead keep the loading task returned by the PDF.js load
flow, then call loadingTask.destroy() for teardown. Make sure the relevant
PDFDocumentProxy usage and any helper that wraps document loading are adjusted
so the loading task is available where cleanup happens.
---
Nitpick comments:
In `@crates/mesh-llm-host-runtime/src/plugin/mcp.rs`:
- Line 454: The #[allow(deprecated)] on the MCP plugin path needs an inline
comment explaining the specific rmcp deprecation being suppressed and the
expected migration/removal plan, matching the documentation style used in
handle_request. Update the surrounding code in mcp.rs near the relevant rmcp
usage so the suppression is clearly scoped and future maintainers can see what
should replace it and when it can be removed.
- Line 382: Add a brief inline comment directly above the `#[allow(deprecated)]`
in `mcp.rs` explaining that the suppression is only for rmcp compatibility,
naming the deprecated rmcp API surface being used in `McpHostPlugin`/related
rmcp calls and noting the condition for removal once the replacement API is
adopted. Keep the allow scoped as narrowly as possible and document the
migration path so future maintainers know exactly why the warning is silenced
and when it can be deleted.
- Line 1335: The deprecated rmcp ServerHandler::get_info usage is currently
silenced without explaining the tradeoff. In mcp.rs near the ServerHandler
implementation, add an inline comment documenting the rmcp version/deprecation
context and whether this is a temporary backward-compatibility shim or a
permanent protocol requirement. Keep the rationale close to the
#[allow(deprecated)] on get_info so future maintainers can understand why the
warning is intentionally suppressed.
In `@crates/mesh-llm-plugin/src/helpers.rs`:
- Line 188: The #[allow(deprecated)] on plugin_server_info_full is suppressing
warnings from deprecated rmcp builder APIs inside this function, so add an
inline comment explaining exactly which rmcp calls are affected and why the
allowance is temporary. Reference plugin_server_info_full and the rmcp builder
usage in its body, and note the intended migration/removal plan so future
maintainers understand the scope of the deprecation tradeoff at this protocol
boundary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b0a3983-5ed3-4192-892d-fe3a6d079d45
⛔ Files ignored due to path filters (4)
Cargo.lockis excluded by!**/*.lockcrates/mesh-llm-ui/package-lock.jsonis excluded by!**/package-lock.jsoncrates/mesh-llm-ui/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlwebsite/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
crates/mesh-llm-host-runtime/src/plugin/mcp.rscrates/mesh-llm-plugin/src/helpers.rscrates/mesh-llm-ui/package.jsoncrates/mesh-llm-ui/src/features/app-tabs/app-tabs.test.tsxcrates/mesh-llm-ui/src/features/app-tabs/types.tscrates/mesh-llm-ui/src/features/chat/pages/ChatPage.test.tsxcrates/mesh-llm-ui/src/features/chat/pages/ChatPage.tsxcrates/mesh-llm-ui/src/features/configuration/components/settings/ConfigurationDefaultsControl.tsxcrates/mesh-llm-ui/src/features/dashboard/components/topology/render/line-mesh.tscrates/mesh-llm-ui/src/features/dashboard/components/topology/render/useTopologyCanvas.test.tsxcrates/mesh-llm-ui/src/lib/api/types.tscrates/mesh-llm-ui/src/lib/pdf.tswebsite/package.json
| wordCount: number | ||
| } | ||
|
|
||
| type DestroyablePdfDocument = PDFDocumentProxy & { destroy?: () => Promise<void> } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the installed pdfjs-dist lifecycle API surface without relying on repo node_modules.
set -euo pipefail
tmp="$(mktemp -d)"
npm pack pdfjs-dist@6.1.200 --pack-destination "$tmp" >/dev/null
tar -xOf "$tmp"/pdfjs-dist-6.1.200.tgz package/types/src/display/api.d.ts \
| sed -n '/class PDFDocumentProxy/,/class PDFPageProxy/p' \
| grep -nE 'destroy|loadingTask|cleanup'Repository: Mesh-LLM/mesh-llm
Length of output: 27609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant source and the pdfjs-dist type declarations.
sed -n '1,220p' crates/mesh-llm-ui/src/lib/pdf.ts
echo '--- api.d.ts (PDFDocumentProxy / PDFDocumentLoadingTask) ---'
tmp="$(mktemp -d)"
npm pack pdfjs-dist@6.1.200 --pack-destination "$tmp" >/dev/null
tar -xOf "$tmp"/pdfjs-dist-6.1.200.tgz package/types/src/display/api.d.ts \
| awk '
/interface PDFDocumentLoadingTask|class PDFDocumentLoadingTask|interface PDFDocumentProxy|class PDFDocumentProxy/ {show=1}
show {print}
/interface PDFPageProxy|class PDFPageProxy/ && show {exit}
'Repository: Mesh-LLM/mesh-llm
Length of output: 50373
Destroy the loading task instead of calling doc.destroy?.(). PDFDocumentProxy in pdfjs-dist@6.1.200 exposes loadingTask.destroy() for teardown, while destroy lives on the loading task—not the proxy—so this path currently skips cleanup.
🤖 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 `@crates/mesh-llm-ui/src/lib/pdf.ts` at line 34, The teardown path in
DestroyablePdfDocument is using the wrong object, so cleanup is skipped. Update
the PDF handling in pdf.ts to stop relying on doc.destroy?.() and instead keep
the loading task returned by the PDF.js load flow, then call
loadingTask.destroy() for teardown. Make sure the relevant PDFDocumentProxy
usage and any helper that wraps document loading are adjusted so the loading
task is available where cleanup happens.
…ependencies # Conflicts: # .github/workflows/ci.yml
ndizazzo
left a comment
There was a problem hiding this comment.
I was just thinking about this today, and you read my mind
Summary
Cargo.lock.Impact
This keeps the project current on compatible Rust packages and latest Node packages for the website/UI while preserving existing behavior. The UI lockfile audits are clean for both pnpm and npm lock surfaces.
Validation
just website-buildjust buildcargo fmt --all --checkcargo check -p mesh-llm-plugincargo clippy -p mesh-llm-plugin --all-targets -- -D warningscargo check -p mesh-llm-host-runtimecargo clippy -p mesh-llm-host-runtime --all-targets -- -D warningscargo check -p mesh-llmcargo clippy -p mesh-llm --all-targets -- -D warningspnpm run validateincrates/mesh-llm-uinpm auditinwebsitepnpm auditincrates/mesh-llm-uinpm audit --legacy-peer-depsincrates/mesh-llm-uiNote:
just buildcompleted successfully but macOS local codesign trust setup still printed the existing local identity warning.Summary by CodeRabbit
Bug Fixes
Chores