Changed: Unify dependency versions across workspace#109
Conversation
- Add [workspace.dependencies] to root Cargo.toml with 40+ shared dependencies - Convert all 5 crates to use workspace = true for common dependencies - Unify internal crate paths (llm-coding-tools-*) in workspace manifest - Consolidate dev-dependencies (criterion, rstest, serial_test, wiremock, etc.) Benefits: - Single source of truth for dependency versions - Prevents version drift between crates - Cleaner per-crate Cargo.toml files - Faster dependency resolution
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe pull request centralizes Rust crate dependency management by introducing a new Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #109 +/- ##
=======================================
Coverage 81.04% 81.04%
=======================================
Files 110 110
Lines 4517 4517
=======================================
Hits 3661 3661
Misses 856 856
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Add reference-style link definitions for `ToolError::Io` in fs modules - Fix `CatalogError::Json` link in models-dev schema docs - Fix `ToolError` link in serdesai convert module - Escape private function names (`load_directory_with`, `core_error_to_serdes`) with backticks - Fix models.dev README markdown link syntax - Add missing `Ok(())` in blocking doctest
…onflict Cargo ignores `default-features = false` in crate-level Cargo.toml when the workspace dependency doesn't specify it. This caused llm-coding-tools-core's default features (tokio) to always be enabled, making both tokio and blocking active simultaneously and producing `BufFile defined multiple times`. - Add `default-features = false` to workspace `llm-coding-tools-core` dep - Add explicit `features = ["tokio"]` to agents crate (relied on defaults)
✅ Actions performedReview triggered.
|
|
@coderabbitai full-review |
|
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Cargo.toml (1)
12-89: Centralization looks clean; a couple of optional polish suggestions.Overall the
[workspace.dependencies]block is well-organized. Two minor, optional tweaks:
- Version pins mix broad (
"1.0","2","3") and narrow ("3.1.2","0.5.5","0.4.25"). Either is fine, but pinning only the patch level for a subset creates an inconsistent contract and can mask future MSRV bumps. Consider standardizing (typically to major.minor) unless there's a specific patch requirement.- Comment headers like
# Internal cratesand# Dev dependenciesare helpful; the block# Dev dependenciesis still in[workspace.dependencies]— individual crates must still explicitly place them under[dev-dependencies](you already do this correctly). Just flagging that the section label can confuse future readers if[workspace.dev-dependencies]is ever assumed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Cargo.toml` around lines 12 - 89, Standardize the version pinning in the [workspace.dependencies] block by choosing a scheme (e.g., major.minor like "1.0" -> "1.0" or lock to patch "1.0.3" if needed) and update entries such as serde, tokio, html-to-markdown-rs, shellexpand, soft-canonicalize, ignore, etc., to follow that scheme so the contract is consistent; also clarify the "# Dev dependencies" comment (currently inside [workspace.dependencies]) by either moving true development-only crates into a separate [workspace.dev-dependencies] section or renaming the comment to avoid implying those entries are already dev-dependencies.src/llm-coding-tools-models-dev/README.md (1)
153-154: Remove the orphan[models.dev]link reference.After renaming the in-text reference to
[models.dev_link]on line 11, the[models.dev]: https://models.devdefinition on line 154 is no longer referenced anywhere in the README. Keeping it is harmless but may trigger unused-reference warnings in some markdown linters and creates two definitions for the same URL.🧹 Proposed cleanup
[models.dev_link]: https://models.dev -[models.dev]: https://models.dev🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/llm-coding-tools-models-dev/README.md` around lines 153 - 154, Remove the unused/duplicate markdown reference definition "[models.dev]: https://models.dev" since the in-text link was renamed to "[models.dev_link]"; locate the orphan reference definition (the "[models.dev]" link entry in README) and delete that line so only the active "[models.dev_link]: https://models.dev" reference remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/llm-coding-tools-bubblewrap/Cargo.toml`:
- Around line 19-24: Remove the redundant tempfile entry from [dev-dependencies]
in Cargo.toml: tempfile is already declared under [dependencies] (used by
runtime code such as TempSandboxDirs in profile/factory.rs) and tests inherit
regular dependencies, so delete the tempfile = { workspace = true } line from
the [dev-dependencies] section to avoid duplication.
---
Nitpick comments:
In `@src/Cargo.toml`:
- Around line 12-89: Standardize the version pinning in the
[workspace.dependencies] block by choosing a scheme (e.g., major.minor like
"1.0" -> "1.0" or lock to patch "1.0.3" if needed) and update entries such as
serde, tokio, html-to-markdown-rs, shellexpand, soft-canonicalize, ignore, etc.,
to follow that scheme so the contract is consistent; also clarify the "# Dev
dependencies" comment (currently inside [workspace.dependencies]) by either
moving true development-only crates into a separate [workspace.dev-dependencies]
section or renaming the comment to avoid implying those entries are already
dev-dependencies.
In `@src/llm-coding-tools-models-dev/README.md`:
- Around line 153-154: Remove the unused/duplicate markdown reference definition
"[models.dev]: https://models.dev" since the in-text link was renamed to
"[models.dev_link]"; locate the orphan reference definition (the "[models.dev]"
link entry in README) and delete that line so only the active
"[models.dev_link]: https://models.dev" reference remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f436b151-59a6-48eb-9e08-48772b4bae82
📒 Files selected for processing (13)
src/Cargo.tomlsrc/llm-coding-tools-agents/Cargo.tomlsrc/llm-coding-tools-agents/src/loader.rssrc/llm-coding-tools-bubblewrap/Cargo.tomlsrc/llm-coding-tools-core/Cargo.tomlsrc/llm-coding-tools-core/src/fs/blocking_impl.rssrc/llm-coding-tools-core/src/fs/tokio_impl.rssrc/llm-coding-tools-models-dev/Cargo.tomlsrc/llm-coding-tools-models-dev/README.mdsrc/llm-coding-tools-models-dev/src/api/schema.rssrc/llm-coding-tools-models-dev/src/catalog/mod.rssrc/llm-coding-tools-serdesai/Cargo.tomlsrc/llm-coding-tools-serdesai/src/convert.rs
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-03-28T02:14:04.465Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
Applied to files:
src/Cargo.tomlsrc/llm-coding-tools-serdesai/Cargo.tomlsrc/llm-coding-tools-core/Cargo.toml
📚 Learning: 2026-04-08T21:35:35.581Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-04-08T21:35:35.581Z
Learning: If relevant to your review task, read `.cargo/{general,performance,documentation}.md` for guidance
Applied to files:
src/llm-coding-tools-agents/Cargo.toml
📚 Learning: 2026-03-11T22:12:27.804Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 54
File: src/llm-coding-tools-models-dev/src/catalog/load_cache.rs:23-29
Timestamp: 2026-03-11T22:12:27.804Z
Learning: In `src/llm-coding-tools-models-dev/src/cache/` (Rust, llm-coding-tools-models-dev crate), the on-disk cache (models.dev.catalog.v1.cache) is assumed to be written only by the local user/process. Malicious or externally-crafted cache files are explicitly out of scope for this threat model, so there is no need to add upper-bound validation on `payload_len_decompressed` before calling `zstd::bulk::decompress`.
Applied to files:
src/llm-coding-tools-models-dev/Cargo.toml
🔇 Additional comments (11)
src/llm-coding-tools-serdesai/src/convert.rs (1)
53-55: LGTM — correct rustdoc fix for a private helper.Since
core_error_to_serdesispub(crate), an intra-doc link from public docs would warn/fail. Inline code plus the explicit[ToolError]link def is the right treatment.src/llm-coding-tools-agents/src/loader.rs (1)
85-85: LGTM — private helper referenced as inline code.
load_directory_withis private to the module, so referencing it via inline code (rather than an intra-doc link) is correct and removes the rustdoc warning.Also applies to: 108-108
src/llm-coding-tools-core/Cargo.toml (1)
41-115: LGTM — clean workspace conversion preserving existing feature semantics.Feature gates (
tokio,blocking,linux-bubblewrap) are unchanged. Dev-dep tokio correctly layersrt-multi-thread/macroson the workspace entry (features are additive). Optional deps (reqwest,tokio,llm-coding-tools-bubblewrap) still gate viaoptional = true.src/llm-coding-tools-agents/Cargo.toml (1)
33-33: LGTM — explicitfeatures = ["tokio"]is required here and correctly applied.Matches the fix from commit 3 ("Workspace default-features unification caused tokio+blocking conflict") — this crate previously relied on
llm-coding-tools-core's defaulttokiofeature, so the explicit enablement is necessary now that the workspace entry setsdefault-features = false.src/llm-coding-tools-serdesai/Cargo.toml (1)
54-95: LGTM — workspace redirection preserves provider feature pass-through and async/TLS configuration.The provider feature gates (
serdes-ai-models/openai, etc.) still work becauseserdes-ai-modelsis brought in via the workspace withdefault-features = false, matching the prior local setting.reqwestnow inheritsrustls+rustls-native-certsfrom the workspace, so the HTTP/TLS behavior is unchanged.src/llm-coding-tools-models-dev/src/catalog/mod.rs (1)
78-78: LGTM — doctest now returns a value consistent with its signature.Matches the pattern used in the
load_atdoctest below (line 148) and fixes the blocking-feature doctest compile error called out in commit 2.src/Cargo.toml (1)
78-78: All internal consumers ofllm-coding-tools-corecorrectly enable a backend feature.
- llm-coding-tools-agents (
src/llm-coding-tools-agents/Cargo.toml:33):features = ["tokio"]✓- llm-coding-tools-serdesai (
src/llm-coding-tools-serdesai/Cargo.toml:54):features = ["tokio"]✓- llm-coding-tools-models-dev (
src/llm-coding-tools-models-dev/Cargo.toml:27): Declaresdefault-features = falseon the core dependency, but the package itself specifiesdefault = ["tokio"](line 11), which automatically propagates the tokio backend to consumers. ✓No binary, example, or test crates were found with missing backend feature configuration.
src/llm-coding-tools-core/src/fs/blocking_impl.rs (1)
11-45: LGTM — intra-doc link definitions look correct.Adding
[ToolError::Io]: crate::error::ToolError::Iofor each docblock resolves the previously bare references and will silencerustdoc::broken_intra_doc_linkson this file. Consistent with the mirror fix intokio_impl.rs.src/llm-coding-tools-models-dev/src/api/schema.rs (1)
93-94: LGTM.Reference definition matches the error variant that
serde_json::Errorconverts into via the?operator; fixes the intra-doc link without any API change.src/llm-coding-tools-core/src/fs/tokio_impl.rs (1)
11-45: LGTM.Async counterpart of the
blocking_impl.rsdoc fix; keeps the link resolution consistent between the two runtime variants.src/llm-coding-tools-models-dev/Cargo.toml (1)
25-64: LGTM — workspace migration is consistent and feature wiring preserved.
llm-coding-tools-core = { workspace = true, default-features = false }is the right belt-and-suspenders after the fix in commit 3 (tokio+blocking conflict).- The
reqwestoptional flag andreqwest/blockingfeature activation on line 20 still work because the rustls feature list now lives on the workspace entry.- Declaring
tokioas both an optional prod dep and a dev-dep with["rt", "macros"]is fine — Cargo takes the union across normal and dev dependency graphs.
tempfile already declared in [dependencies]; tests inherit it automatically.
1 participant
Problem
Dependencies were declared independently in each crate's
Cargo.toml, leading to version inconsistencies across the workspace:1.0,1.0.2281.0,1.0.1452.0,2.0.181.51,13.27,39.1,90.5.5,0.56,6.0.0This made it easy for versions to drift apart over time, and required updating multiple files to bump a shared dependency.
Solution
Add
[workspace.dependencies]to the rootsrc/Cargo.tomland convert all 5 crates to reference workspace deps viaworkspace = true.Root Cargo.toml - 40+ shared dependencies defined once:
serde,serde_json,serde_yaml,bitcode,bitflagstokio,maybe-async,async-trait,futuresreqwest,html-to-markdown-rshashbrown,indexmap,tinyvec,lite-strtab,ahash,parking_lotglobset,grep-regex,grep-searcher,memchr,ignoreprocess-wrapllm-coding-tools-core,llm-coding-tools-bubblewrap,llm-coding-tools-agents,llm-coding-tools-models-devcriterion,rstest,serial_test,wiremock,indoc,temp-envPer-crate Cargo.toml - all version strings replaced with
workspace = true; feature overrides preserved locally.Remaining duplicates (unavoidable)
Transitive duplicates from external crates (serdes-ai, AWS SDK) still exist in the lockfile — these are outside our control and would require upstream changes:
thiserrorv1 (serdes-ai) + v2 (our crates)reqwestv0.12 (serdes-ai) + v0.13 (our crates)rustlsv0.21 (AWS SDK) + v0.23 (our crates)hashbrownv0.16 (html-to-markdown-rs) + v0.17 (our crates)Files changed
src/Cargo.toml[workspace.dependencies]src/llm-coding-tools-core/Cargo.tomlsrc/llm-coding-tools-agents/Cargo.tomlsrc/llm-coding-tools-models-dev/Cargo.tomlsrc/llm-coding-tools-serdesai/Cargo.tomlsrc/llm-coding-tools-bubblewrap/Cargo.toml