Upgrade workspace dependencies to latest releases - #615
Merged
Conversation
…hanges Bump every workspace dependency (root, wfl-lsp, wflpkg) to its latest release and fix the breaking changes the major-version jumps introduced. Source-affecting migrations: - rand 0.9 -> 0.10: core `RngCore` trait renamed to `Rng` (crypto.rs) - logos 0.15 -> 0.16: opt the intentional line-comment skip out of the new greedy-repetition lint via `allow_greedy = true` - codespan-reporting 0.11 -> 0.13: `term::emit` -> `emit_to_write_style` - sqlx 0.8 -> 0.9: runtime/TLS feature split, `Database::Arguments` GAT lifetime removal, and `SqlSafeStr` gate (wrap program SQL in `AssertSqlSafe`; all values stay parameter-bound) - reqwest 0.11 -> 0.13, bcrypt 0.15 -> 0.19: no source changes - rcgen 0.14 (key_pair -> signing_key), tokio-tungstenite 0.30 (Message::Text now Utf8Bytes), criterion 0.8 (std::hint::black_box): test/bench-only fixes Held back, with rationale in the Dev Diary: - warp 0.3.7: 0.4 dropped the `tls` feature, which would remove HTTPS support from the web server (backward-compat break) - RustCrypto password-hash wave (sha2/hmac/hkdf/pbkdf2/scrypt/argon2): blocked by argon2 having no password-hash 0.6 release; upgrading piecemeal would split the shared PasswordHasher trait - num-bigint-dig 0.8.6: rsa (via sqlx) still requires 0.8 Verified: cargo fmt, clippy -D warnings (wfl package), cargo test --workspace (1480 passed), and TestPrograms (107 passed) all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Contributor
|
Warning Review limit reached
Next review available in: 51 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. 📝 WalkthroughWalkthroughThe PR refreshes Rust dependencies across workspace manifests, raises the MSRV to Rust 1.94, initializes rustls providers at startup, and updates SQLx, diagnostic, lexer, crypto, benchmark, TLS, and WebSocket code for changed crate APIs. ChangesDependency refresh and compatibility updates
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the WFL workspace to newer dependency major versions and performs the required source migrations across the runtime, LSP, tests, and benchmarks to keep the project building and passing tests.
Changes:
- Bumped core/runtime and tool/workspace dependencies (tokio/reqwest/sqlx/logos/codespan-reporting/rand/bcrypt, etc.) across the workspace.
- Migrated code for breaking API changes (sqlx
SqlSafeStr/AssertSqlSafe, codespan-reporting emit API, logos skip lint opt-out, tokio-tungstenite text frames, rcgen key rename). - Updated benchmarks to use
std::hint::black_boxand added a dev diary entry documenting the upgrade.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Workspace dependency bumps and sqlx feature split; keeps warp pinned for TLS compatibility. |
| Cargo.lock | Lockfile refresh reflecting updated direct/transitive dependencies. |
| crates/wflpkg/Cargo.toml | Updates wflpkg runtime/dev dependencies (tokio/reqwest/rustyline/etc.). |
| wfl-lsp/Cargo.toml | Updates LSP crate dependencies (tokio/dashmap/serde_json/env_logger). |
| src/interpreter/database.rs | sqlx 0.9 migration (AssertSqlSafe, Arguments type change) in DB query/execute paths. |
| src/diagnostics/mod.rs | codespan-reporting migration to emit_to_write_style. |
| src/repl.rs | codespan-reporting migration to emit_to_write_style for REPL diagnostics. |
| src/lexer/token.rs | logos 0.16 migration (allow_greedy opt-out for skip regex lint). |
| src/stdlib/crypto.rs | rand API migration for RNG trait usage in crypto helpers. |
| tests/websocket_test.rs | tokio-tungstenite 0.30 migration for Message::Text payload type change. |
| tests/web_server_tls_test.rs | rcgen 0.14 migration (signing_key rename). |
| benches/lexer_bench.rs | criterion migration to std::hint::black_box. |
| benches/parser_bench.rs | criterion migration to std::hint::black_box. |
| Dev diary/2026-07-13-cargo-crate-updates.md | Documents dependency upgrade rationale, migrations, and verification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+131
to
+134
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+147
to
+150
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+161
to
+164
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+187
to
+190
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+201
to
+204
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+215
to
+218
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9934ff770a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| reqwest = { version = "0.13.4", features = ["json"] } | ||
| # sqlx 0.9 split the old `runtime-tokio-rustls` feature into a separate runtime | ||
| # and TLS backend; `tls-rustls` aliases the ring-backed rustls stack we used before. | ||
| sqlx = { version = "0.9.0", features = ["runtime-tokio", "tls-rustls", "sqlite", "mysql", "postgres", "chrono"] } |
There was a problem hiding this comment.
The sqlx 0.9.0 bump raises the dependency floor to Rust 1.94, but this manifest still advertises rust-version = "1.88" for WFL. Building with the promised 1.88 toolchain (or the documented 1.91 dev toolchain) will fail in Cargo before WFL compiles because the locked dependency requires a newer compiler; either bump WFL's rust-version/docs/toolchain to 1.94 or choose a sqlx release compatible with the stated MSRV.
Useful? React with 👍 / 👎.
sqlx 0.9.0 declares rust-version = "1.94.0", which raises the effective floor for the workspace. WFL previously advertised MSRV 1.88 (dev 1.91.1), so a build on the promised toolchain would fail in cargo. Bump the declared rust-version and align the docs. - Cargo.toml: rust-version 1.88 -> 1.94 (with rationale) - CLAUDE.md, AGENTS.md, Docs/development/building-from-source.md, Docs/reference/supported-platforms.md: MSRV references updated to 1.94 - Dev Diary: record the MSRV decision CI uses dtolnay/rust-toolchain@stable (>= 1.94), so no workflow change. Addresses the Codex P1 review on #615. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Collaborator
Author
|
Addressed the Codex P1 (MSRV alignment) in 4f9b6dd.
Verification remains green: Generated by Claude Code |
rustfmt requires `std::fs` before `std::hint::black_box`; the criterion 0.8 black_box migration left them out of order. Restores `cargo fmt` cleanliness (Check formatting CI job). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Comment on lines
+131
to
+134
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+147
to
+150
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+161
to
+164
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+187
to
+190
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+201
to
+204
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
Comment on lines
+215
to
+218
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. | ||
| let mut query = sqlx::query(sqlx::AssertSqlSafe(sql)); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Cargo.toml (1)
58-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
[workspace.dependencies]to deduplicate shared dependency versions.Multiple crates in the workspace declare their own version pins for core dependencies like
tokio,reqwest, andserde_json. Moving these shared dependencies to a[workspace.dependencies]table in the rootCargo.tomlensures their versions and baseline features stay perfectly synchronized across the project and reduces maintenance overhead during future upgrades.
Cargo.toml#L58-L63: Move shared dependency definitions to a[workspace.dependencies]table and reference them here usingworkspace = true(e.g.,tokio.workspace = true).crates/wflpkg/Cargo.toml#L9-L15: Inherit the shared dependencies usingworkspace = true.wfl-lsp/Cargo.toml#L11-L13: Inherit the shared dependencies usingworkspace = true.🤖 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 `@Cargo.toml` around lines 58 - 63, Centralize shared dependency versions and baseline features in the root Cargo.toml [workspace.dependencies] table, then update Cargo.toml lines 58-63 to use workspace inheritance (including tokio, reqwest, serde_json, and sqlx). Update crates/wflpkg/Cargo.toml lines 9-15 and wfl-lsp/Cargo.toml lines 11-13 to reference those dependencies with workspace = true, preserving any crate-specific features.
🤖 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 `@Cargo.toml`:
- Around line 59-62: Align the rustls crypto provider configuration for reqwest
and sqlx in the dependency declarations, selecting the same provider for both
instead of mixing reqwest’s aws-lc-rs default with sqlx’s ring-backed tls-rustls
stack. Ensure the resulting workspace configuration avoids requiring an implicit
process-level provider before any TLS client or pool is created.
---
Nitpick comments:
In `@Cargo.toml`:
- Around line 58-63: Centralize shared dependency versions and baseline features
in the root Cargo.toml [workspace.dependencies] table, then update Cargo.toml
lines 58-63 to use workspace inheritance (including tokio, reqwest, serde_json,
and sqlx). Update crates/wflpkg/Cargo.toml lines 9-15 and wfl-lsp/Cargo.toml
lines 11-13 to reference those dependencies with workspace = true, preserving
any crate-specific features.
🪄 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: e7061cda-650d-4d50-b747-f95e9949f4db
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomlDev diary/2026-07-13-cargo-crate-updates.mdbenches/lexer_bench.rsbenches/parser_bench.rscrates/wflpkg/Cargo.tomlsrc/diagnostics/mod.rssrc/interpreter/database.rssrc/lexer/token.rssrc/repl.rssrc/stdlib/crypto.rstests/web_server_tls_test.rstests/websocket_test.rswfl-lsp/Cargo.toml
Comment on lines
+131
to
+133
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. |
Comment on lines
+147
to
+149
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. |
Comment on lines
+161
to
+163
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. |
Comment on lines
+187
to
+189
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. |
Comment on lines
+201
to
+203
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. |
Comment on lines
+215
to
+217
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`; the SQL text comes | ||
| // from the WFL program (not concatenated user input) and every value | ||
| // is passed via `.bind()`, so wrapping with `AssertSqlSafe` is sound. |
The fuzz crate is a separate workspace that pins wfl's transitive deps in its own lockfile. The dependency bumps left fuzz/Cargo.lock stale, so the "Fuzz targets compile" job (cargo check --locked) failed. Regenerated the lock so it resolves sqlx 0.9, rand 0.10, reqwest 0.13, etc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
The prior comment implied the SQL text was inherently safe ("not
concatenated user input"). In fact `sql` is a runtime WFL value, and
AssertSqlSafe is an explicit opt-out of sqlx 0.9's SqlSafeStr gate. Reword
to state that only bound parameter values are injection-safe and that WFL
programs must not build SQL text from untrusted input.
Addresses the Copilot review comments on #615.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Comment on lines
+35
to
+39
| - **`codespan-reporting 0.11 → 0.13`** — `term::emit` is deprecated; switched the | ||
| three call sites (`src/diagnostics/mod.rs`, `src/repl.rs`) to | ||
| `term::emit_to_write_style`, the color-preserving replacement (both | ||
| `StandardStream` and `Buffer` satisfy the new `WriteStyle` blanket impl via | ||
| `termcolor::WriteColor`). |
Comment on lines
+131
to
+136
| // sqlx 0.9 gates `query()` behind `SqlSafeStr`. `AssertSqlSafe` is | ||
| // the explicit opt-out for our dynamic SQL: `sql` is a runtime WFL | ||
| // value, not a `&'static str`. It asserts nothing about the query | ||
| // text's own safety — only parameter *values* are injection-safe, | ||
| // via `.bind()`. WFL programs must not build this SQL text from | ||
| // untrusted input. |
reqwest 0.13 (aws-lc-rs) and sqlx 0.9 (tls-rustls-ring) both link rustls 0.23, so a single rustls is compiled with two crypto providers. Each crate configures its own provider explicitly, so TLS works today (verified with a live HTTPS request and the sqlite e2e suite), but rustls 0.23 panics if any code builds a config from the ambient default while multiple providers are present. Install ring once in `main` so the process default is unambiguous. Adds rustls 0.23 (ring feature only) as a direct dependency. Only the `wfl` binary links both providers, so the install lives there. Addresses the CodeRabbit "align rustls providers" finding on #615. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
- database.rs: move the AssertSqlSafe safety contract into the module doc and leave a one-line note per call site instead of repeating the full block six times (Copilot maintainability comment). - Dev Diary: the codespan-reporting migration touched seven call sites (one in diagnostics/mod.rs, six in repl.rs), not three; correct the count and record the rustls CryptoProvider install. Addresses Copilot review comments on #615. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Collaborator
Author
|
Addressed the latest review round (commits CI
CodeRabbit — rustls crypto providers (Major) Copilot — Dev Diary count — corrected "three call sites" → the actual seven ( Deliberately skipped: CodeRabbit's Verification after these changes: Generated by Claude Code |
Commit 4dc9d79 added rustls as a direct dependency of wfl, which changed the dependency graph the fuzz workspace resolves (fuzz depends on wfl by path). The fuzz lock was regenerated before that edge existed, so the "Fuzz targets compile" job (cargo check --locked) still failed. Regenerated fuzz/Cargo.lock so it records the new edge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Comment on lines
+112
to
+118
| // Multiple rustls crypto providers are compiled into this binary: aws-lc-rs | ||
| // (via reqwest) and ring (via sqlx), sharing one rustls 0.23. rustls refuses | ||
| // to auto-select a default when more than one is present, so any code path | ||
| // that builds a TLS config from the ambient default would panic at runtime. | ||
| // Install ring explicitly, once, so the process default is unambiguous; a | ||
| // prior install (the Err case) is fine to ignore. | ||
| let _ = rustls::crypto::ring::default_provider().install_default(); |
Comment on lines
+111
to
+119
| fn main() -> io::Result<()> { | ||
| // Multiple rustls crypto providers are compiled into this binary: aws-lc-rs | ||
| // (via reqwest) and ring (via sqlx), sharing one rustls 0.23. rustls refuses | ||
| // to auto-select a default when more than one is present, so any code path | ||
| // that builds a TLS config from the ambient default would panic at runtime. | ||
| // Install ring explicitly, once, so the process default is unambiguous; a | ||
| // prior install (the Err case) is fine to ignore. | ||
| let _ = rustls::crypto::ring::default_provider().install_default(); | ||
|
|
Move the process-level CryptoProvider install into `wfl::init_rustls_crypto_provider()` and call it from every binary that links both rustls providers, not just `wfl`: - wfl-lsp links the wfl lib (hence reqwest's aws-lc-rs + sqlx's ring), so it could hit the same "multiple providers, no default" panic; it now installs the default too. - In `wfl`, the install moves below the `--help`/`--version` fast path so those trivial invocations skip TLS init. - The helper is public, so embedders of the wfl crate can call it as well. Addresses the Copilot review comments on #615 (centralize the install; keep --help/--version lightweight). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Comment on lines
+83
to
+86
| /// config from the ambient default panics at runtime. Every binary that links | ||
| /// this crate — and any embedder — should call this once at startup, before | ||
| /// creating a TLS client or connection pool. Installing twice is a no-op (the | ||
| /// second call returns `Err`, which is ignored), so it is always safe to call. |
install_default() returns Err whenever a default is already set — by an earlier call OR by any other code — so the helper guarantees only that *some* provider is installed, not ring specifically. Clarify the doc so callers don't rely on ring being selected, and note how to install a specific provider. Addresses the Copilot review comment on #615. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Comment on lines
5
to
8
| # Minimum supported Rust version. The codebase itself only needs 1.88 (it uses | ||
| # `if let … && …` let-chains, stabilized then), but the `sqlx` 0.9 dependency | ||
| # declares `rust-version = "1.94"`, which raises the effective floor. Recorded | ||
| # here so `cargo` fails fast on older toolchains instead of deep in a build. |
| |---|---|---| | ||
| | Rust channel | **stable** | All CI jobs use `dtolnay/rust-toolchain@stable`. | | ||
| | Minimum supported Rust version (MSRV) | **1.88** (declared) | `Cargo.toml` `rust-version = "1.88"`. The codebase uses `let`-chains (stabilized in 1.88), so older toolchains fail fast via `cargo`'s check. Note: CI builds on **stable**, so the 1.88 floor is *declared but not gate-tested* — an MSRV lane is a tracked follow-up. | | ||
| | Minimum supported Rust version (MSRV) | **1.94** (declared) | `Cargo.toml` `rust-version = "1.94"`. The codebase itself needs 1.88 (`let`-chains), but the `sqlx` 0.9 dependency declares `rust-version = "1.94"`, so that is the effective floor; older toolchains fail fast via `cargo`'s check. Note: CI builds on **stable**, so the 1.94 floor is *declared but not gate-tested* — an MSRV lane is a tracked follow-up. | |
The MSRV floor is now driven by sqlx 0.9 (rust-version = 1.94), so tying the rationale to "let-chains stabilized in 1.88" is misleading and no longer load-bearing. Drop that attribution in Cargo.toml, CLAUDE.md, and supported-platforms.md and state the sqlx-driven floor plainly. Addresses the Copilot review comments on #615. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
logbie
commented
Jul 14, 2026
logbie
left a comment
logbie
left a comment
Collaborator
Author
There was a problem hiding this comment.
Reviewed the current head, 37246b3e036ddc1b30b073c901414db6d3d33850.
I found no blocking issues in the latest diff.
What I checked:
- The SQLx 0.9 migration preserves bound parameter handling and now accurately documents that
AssertSqlSafeis an escape hatch for runtime SQL text, not an injection-safety guarantee. - The reqwest/SQLx rustls-provider conflict is handled through a shared initialization helper, invoked by both
wflandwfl-lspafter their help/version fast paths. The helper's contract also correctly accounts for a provider installed earlier by an embedder. - The Rust 1.94 MSRV is synchronized across the package manifest and user/developer documentation.
- The Logos, rand, codespan-reporting, rcgen, tokio-tungstenite, and Criterion migrations are consistent with their updated APIs.
- The current CI run is green for formatting, workspace tests, Linux/Windows integration, PostgreSQL/MariaDB tests, WFL program runs, fuzz-target compilation, and the repository's current Clippy gate.
One known residual risk remains: retaining warp 0.3.7 also retains the older rustls 0.22 / rustls-webpki 0.102.x path and its existing security alert. That is explicitly deferred here to avoid breaking WFL's native TLS support and is not introduced by this PR, so I do not consider it a blocker for this dependency-refresh change.
Verdict: no blockers. GitHub does not allow the PR author to approve their own PR, so I am submitting this as a comment review rather than an approval.
The build environment has a limited disk allowance and the target/ tree grows to tens of GB (debug + release with debug=true), causing "No space left on device" / linker Bus errors mid-build. Document running `cargo clean` before each build in CLAUDE.md so builds start from a clean tree. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Comment on lines
+113
to
+114
| - **Disk space (run `cargo clean` before every build)**: The build environment has a limited disk allowance and the `target/` tree (debug + release, with `debug = true` on release) grows to tens of GB, which causes `No space left on device` / linker `Bus error` failures. Run `cargo clean` before each build so a fresh build never runs out of space. (Trade-off: this forgoes incremental compilation, so every build is a full rebuild.) | ||
| - **Build**: `cargo clean && cargo build` (release: `cargo clean && cargo build --release`). |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
CLAUDE.md (1)
113-114: 📐 Maintainability & Code Quality | 🔵 TrivialConsider alternatives to
cargo cleanto preserve incremental compilation.Requiring
cargo cleanbefore every build completely disables incremental compilation, which significantly degrades the developer experience by forcing full rebuilds every time.To reduce the
target/directory size without losing incremental compilation, consider the following alternatives:
- Limit debug info for dependencies by adding
[profile.dev.package."*"]withdebug = 0toCargo.toml.- Use
cargo-sweepto automatically clean up only old or unused build artifacts.- Configure
split-debuginfo = "unpacked"on Linux/macOS to reduce the linking overhead of large debug binaries.🤖 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 `@CLAUDE.md` around lines 113 - 114, The build guidance in CLAUDE.md should not require cargo clean before every build. Replace the mandatory full-clean workflow with a space-preserving approach that retains incremental compilation, documenting applicable Cargo profile settings such as [profile.dev.package."*"] debug = 0, split-debuginfo = "unpacked", or selective cargo-sweep cleanup while keeping normal build commands incremental.
🤖 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 `@CLAUDE.md`:
- Around line 113-114: The build guidance in CLAUDE.md should not require cargo
clean before every build. Replace the mandatory full-clean workflow with a
space-preserving approach that retains incremental compilation, documenting
applicable Cargo profile settings such as [profile.dev.package."*"] debug = 0,
split-debuginfo = "unpacked", or selective cargo-sweep cleanup while keeping
normal build commands incremental.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2169557a-99a9-4428-85e2-51f3a2f47654
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockfuzz/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
AGENTS.mdCLAUDE.mdCargo.tomlDev diary/2026-07-13-cargo-crate-updates.mdDocs/development/building-from-source.mdDocs/reference/supported-platforms.mdbenches/parser_bench.rssrc/interpreter/database.rssrc/lib.rssrc/main.rswfl-lsp/src/main.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- benches/parser_bench.rs
- src/interpreter/database.rs
Rationale: a build that runs out of disk mid-way forces a cargo clean + full rebuild anyway, so cleaning up front is strictly cheaper than a failed build plus the clean-and-rebuild. But cleaning unconditionally throws away incremental compilation on machines with ample disk. So CLAUDE.md now documents an intelligent, disk-space-gated clean (clean only when there isn't room for a full ~30 GB build) and keeps `cargo build` as the default command. Addresses the CodeRabbit and Copilot review comments on #615 about not disabling incremental builds unconditionally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
62 tasks
This was referenced Jul 26, 2026
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
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.
Summary
Comprehensive dependency refresh across the workspace to move all crates to their latest releases. This includes major-version upgrades for 20+ dependencies, with source code migrations for breaking changes in
sqlx,logos,codespan-reporting,rand,bcrypt,rcgen,tokio-tungstenite, andcriterion.Key Changes
Cargo.toml updates:
tokio 1.35 → 1.52,reqwest 0.11 → 0.13,sqlx 0.8 → 0.9logos 0.15 → 0.16rand 0.9 → 0.10,bcrypt 0.15 → 0.19codespan-reporting 0.11 → 0.13criterion 0.4 → 0.8,rcgen 0.13 → 0.14,tokio-tungstenite 0.21 → 0.30Source code migrations:
src/stdlib/crypto.rs: Updated threeuse rand::RngCore;imports touse rand::Rng;(trait rename in rand 0.10)src/lexer/token.rs: Wrapped logos skip pattern in group form withallow_greedy = trueto opt out of logos 0.16's unbounded-greedy-repetition lintsrc/interpreter/database.rs:sqlx::query()calls to wrap SQL text insqlx::AssertSqlSafe()(0.9 gates behindSqlSafeStrtrait)<$db as sqlx::Database>::Arguments<'q>inbind_param!macro (GAT removal in sqlx 0.9)runtime-tokio-rustlstoruntime-tokio+tls-rustlssrc/repl.rs: Replaced 6 calls to deprecatedterm::emit()withterm::emit_to_write_style()(codespan-reporting 0.13)src/diagnostics/mod.rs: Replacedterm::emit()withterm::emit_to_write_style()(codespan-reporting 0.13)tests/web_server_tls_test.rs: Updatedcertified.key_pairtocertified.signing_key(rcgen 0.14 rename)tests/websocket_test.rs: UpdatedMessage::Texthandling to call.to_string()onUtf8Bytesand use.into()for frame construction (tokio-tungstenite 0.30)benches/lexer_bench.rs&benches/parser_bench.rs: Replaced deprecatedcriterion::black_boxwithstd::hint::black_boxDeliberately held back:
tlsfeature entirely, which would break HTTPS support in WFL's web serverpassword-hashversions in a security-sensitive moduleVerification
cargo fmt --all -- --check— cleancargo clippy -p wfl --all-targets --all-features -- -D warnings— cleancargo test --workspace— 1480 passed, 0 failed, 25 ignoredTestPrograms/*.wfl— 107 passed, 0 failed, 22 skippedhttps://claude.ai/code/session_011KzPjh8Tu7ZZGWL7GLks8M
Summary by CodeRabbit