Biiiiigg deps upgrade & coalesce - #1167
Conversation
Runs `cargo update` (17 in-range bumps) and raises the manifest floors that had drifted below what already resolves: bytes 1.11->1.12, either 1.16->1.17, rustc-hash 2.1.2->2.1.3, tokio 1.52->1.53, uuid 1.23->1.24. Rewrites the google-cloud comment to name the actual root cause. It is not reqwest: nickel-lang-core's `format` feature reaches topiary-web-tree-sitter-sys, which pins an exact `wasm-bindgen = "=0.2.100"` *untargeted*, so it constrains resolution even though we never build wasm32. That one pin is what holds back google-cloud-auth/storage, reqwest and russh-sftp, and what costs us the duplicate RustCrypto 0.10 stack. object stays at 0.39: 0.40 turns `Object::exports()`/`imports()` from `Result<Vec<_>>` into lazy `Result<_>`-yielding iterators, so crates/check needs a policy for a per-symbol error mid-scan. That is a deliberate change, not a version bump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nickel-lang-core's `format` feature reaches topiary-web-tree-sitter-sys, whose released 0.7.3 pins an exact `wasm-bindgen = "=0.2.100"` *untargeted*. That constrained resolution for the whole graph even though we never build wasm32, capping js-sys/web-sys at 0.3.77 and holding google-cloud-auth, google-cloud-storage, reqwest and russh-sftp well below their releases. Upstream already fixed it — master moved to a caret `wasm-bindgen = "0.2"` and target-gates the crate behind `cfg(target_arch = "wasm32")` — but has not published it. Master's workspace version is still 0.7.3, so a `[patch.crates-io]` satisfies nickel's `topiary-core = "^0.7"` unchanged. Only the two wasm crates are patched; topiary-core stays on crates.io, so .ncl formatting output is untouched. Drop the patch once topiary ships >0.7.3. Unblocks google-cloud-auth 1.5->1.15, google-cloud-storage 1.7->1.17, reqwest 0.13.1->0.13.4 and russh-sftp 2.1.2->2.4.0, and collapses the duplicate RustCrypto 0.10 stack: 65 -> 54 crates carrying more than one version. BREAKING CHANGE: russh-sftp 2.3 changed `Handler::Error` to require `Into<StatusReply>` rather than `Into<StatusCode>`. `SftpError` gains that impl; the reply carries no message, so the wire output is byte-identical and `Io` error text (which can embed host paths) still never crosses the export boundary. russh-sftp 2.2 also changed `FileAttributes::default()` from dummy zeros to absent fields, which compiles silently. `root_attrs()` takes the new meaning deliberately: the synthetic root has no inode, so it now states only its mode instead of claiming uid 0, size 0 and an epoch mtime. A regression test pins that contract. reqwest 0.13.4's deeper types push two futures past rustc's default query depth (128) when computing their layout, and the overflow surfaces in every crate root that monomorphizes them rather than where it originates. Both are boxed at the bottom of the chain instead — `Env::build`, which reaches the cache fetchers and the client stack beneath them, and the CLI's `run_command` dispatch. One allocation per session launch and per process invocation respectively, versus a `recursion_limit` attribute on six crate roots and every future integration test that touches a launch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR updates workspace dependency constraints, pins selected Git revisions, pins two asynchronous futures, and revises SFTP error conversion and synthetic-root metadata behavior. ChangesWorkspace, runtime, and SFTP changes
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 `@Cargo.toml`:
- Around line 204-228: Add a just-based regression recipe for .ncl formatting
that runs mip check --fix against representative files and verifies the
resulting contents are unchanged. Integrate it with the existing just test
workflow and ensure the fixture files cover the formatting path affected by the
topiary patch.
🪄 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: 0ae3d69d-5af2-4ae6-bcf4-2f7188a54ac6
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.tomlcrates/minimal/src/lib.rscrates/minimald/src/session_host.rscrates/minimald/src/sftp.rs
|
|
||
| # Lifts the wasm-bindgen ceiling. | ||
| # | ||
| # nickel-lang-core's `format` feature (what `mip check --fix` formats .ncl | ||
| # files with, crates/check/src/lib.rs) reaches topiary-core -> | ||
| # topiary-tree-sitter-facade -> topiary-web-tree-sitter-sys. Released 0.7.3 | ||
| # pins an exact `wasm-bindgen = "=0.2.100"` *untargeted*, so it constrains | ||
| # resolution even though we never build wasm32 — capping wasm-bindgen at | ||
| # 0.2.100 and js-sys/web-sys at 0.3.77 for the entire graph. That in turn | ||
| # pinned google-cloud-auth/storage, reqwest and russh-sftp well below their | ||
| # current releases, and forced duplicate copies of the RustCrypto 0.10 stack | ||
| # (sha2/digest/block-buffer/crypto-common) alongside our 0.11. | ||
| # | ||
| # Upstream already fixed it — master moved to a caret `wasm-bindgen = "0.2"` | ||
| # and target-gates topiary-web-tree-sitter-sys behind | ||
| # `cfg(target_arch = "wasm32")` — but has not cut a release carrying it. | ||
| # Master's workspace version is still 0.7.3, so this patch satisfies | ||
| # nickel's `topiary-core = "^0.7"` unchanged. | ||
| # | ||
| # Only the two wasm crates are patched: topiary-core has no wasm-bindgen | ||
| # dependency of its own and stays on crates.io 0.7.3, so .ncl formatting | ||
| # output is untouched. Drop this section once topiary publishes >0.7.3. | ||
| [patch.crates-io] | ||
| topiary-tree-sitter-facade = { git = "https://github.com/tweag/topiary.git", rev = "838c74ab4058f82bc3c6a12380f22c10f22ac814" } | ||
| topiary-web-tree-sitter-sys = { git = "https://github.com/tweag/topiary.git", rev = "838c74ab4058f82bc3c6a12380f22c10f22ac814" } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rev='838c74ab4058f82bc3c6a12380f22c10f22ac814'
test -f Cargo.lock
rg -n -F "$rev" Cargo.lock
rg -n -A6 -B2 \
'^name = "(topiary-tree-sitter-facade|topiary-web-tree-sitter-sys|topiary-core)"$' \
Cargo.lock
rg -n 'topiary-(tree-sitter-facade|web-tree-sitter-sys)|topiary-core' Cargo.toml
just --listRepository: gominimal/minimal
Length of output: 2032
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- repository recipes ---'
fd -i 'justfile|just\.|cargo\.toml|cargo\.lock' -t f | sort
for f in $(fd -i 'justfile|just\.' -t f | sort); do
printf '\n--- %s ---\n' "$f"
sed -n '1,240p' "$f"
done
printf '%s\n' '--- relevant manifest and lockfile sections ---'
sed -n '190,235p' Cargo.toml
sed -n '6918,6985p' Cargo.lock
printf '%s\n' '--- formatting-related files and recipes ---'
rg -n -i 'format|topiary|\.ncl|check --fix' --glob '!Cargo.lock' --glob '!target/**' .
printf '%s\n' '--- exact Topiary revision package manifests ---'
rev='838c74ab4058f82bc3c6a12380f22c10f22ac814'
for path in \
Cargo.toml \
topiary-core/Cargo.toml \
topiary-tree-sitter-facade/Cargo.toml \
topiary-web-tree-sitter-sys/Cargo.toml \
crates/topiary-core/Cargo.toml \
crates/topiary-tree-sitter-facade/Cargo.toml \
crates/topiary-web-tree-sitter-sys/Cargo.toml
do
url="https://raw.githubusercontent.com/tweag/topiary/${rev}/${path}"
body=$(curl -fsSL "$url" 2>/dev/null || true)
if [ -n "$body" ]; then
printf '\n--- %s ---\n' "$path"
printf '%s\n' "$body" | rg -n '^(name|version)\s*=|wasm-bindgen|target_arch|topiary-web-tree-sitter-sys|topiary-tree-sitter-facade' || true
fi
doneRepository: gominimal/minimal
Length of output: 50374
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- just recipes related to checks and tests ---'
rg -n -B4 -A8 '^(test|check|.*format.*|.*ncl.*):|cargo test|cargo nextest|mip check|check --fix' justfile | head -n 240
printf '%s\n' '--- check crate manifest and formatter call sites ---'
cat -n crates/check/Cargo.toml
rg -n -A12 -B8 'topiary|format|check.*fix|fix' crates/check crates/mip | head -n 240
printf '%s\n' '--- exact Topiary revision tree and package manifests ---'
rev='838c74ab4058f82bc3c6a12380f22c10f22ac814'
tree=$(curl -fsSL "https://api.github.com/repos/tweag/topiary/git/trees/${rev}?recursive=1")
printf '%s\n' "$tree" | jq -r '.tree[] | select(.path | endswith("Cargo.toml")) | .path'
for path in $(printf '%s\n' "$tree" | jq -r '.tree[] | select(.path | endswith("Cargo.toml")) | .path' | rg '(^|/)(topiary-(core|tree-sitter-facade|web-tree-sitter-sys))/Cargo.toml$|^Cargo.toml$'); do
printf '\n--- %s ---\n' "$path"
curl -fsSL "https://raw.githubusercontent.com/tweag/topiary/${rev}/${path}" |
rg -n '^(name|version)\s*=|wasm-bindgen|target_arch|topiary-web-tree-sitter-sys|topiary-tree-sitter-facade' || true
doneRepository: gominimal/minimal
Length of output: 147
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- justfile test and check recipes ---'
rg -n -B3 -A10 '^(test|check|.*fmt.*|.*format.*):|cargo (test|nextest)|mip check' justfile | head -n 220
printf '%s\n' '--- formatter integration points ---'
rg -n -B6 -A10 'topiary|mip check|check.*fix|format' crates/check crates/mip --glob '*.rs' --glob 'Cargo.toml' | head -n 260
printf '%s\n' '--- all tracked Nickel files and test fixtures ---'
git ls-files | rg '(^|/).*\.ncl$|test|fixture' | head -n 220
printf '%s\n' '--- lockfile package records ---'
awk '
/^name = "(topiary-core|topiary-tree-sitter-facade|topiary-web-tree-sitter-sys)"$/ { show=1; n=0 }
show { print; n++ }
show && n >= 16 { show=0 }
' Cargo.lockRepository: gominimal/minimal
Length of output: 147
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- justfile recipe names ---'
awk '/^[A-Za-z0-9_.-]+(\s+\*args)?(\s*:.*)?$/{print NR ":" $0}' justfile | head -n 180
printf '%s\n' '--- formatter call sites ---'
grep -R -n -E 'topiary|format|check.*fix' crates/check/src crates/mip/src --include='*.rs' --include='Cargo.toml' | head -n 180
printf '%s\n' '--- tracked Nickel files ---'
git ls-files '*.ncl' | head -n 180Repository: gominimal/minimal
Length of output: 11605
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- just test recipes ---'
sed -n '260,290p' justfile
printf '%s\n' '--- Nickel formatting implementation ---'
sed -n '960,1010p' crates/check/src/lib.rs
printf '%s\n' '--- formatter-related tests in check crate ---'
grep -n -E '^[[:space:]]*#\[test\]|format|formatter|\.ncl' crates/check/src/lib.rs crates/check/src/*.rs | tail -n 120
printf '%s\n' '--- patched package records ---'
sed -n '6924,6980p' Cargo.lockRepository: gominimal/minimal
Length of output: 15839
Add a just-based .ncl formatting regression test. Run mip check --fix on representative files and assert that the formatted output remains unchanged.
🤖 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 204 - 228, Add a just-based regression recipe for
.ncl formatting that runs mip check --fix against representative files and
verifies the resulting contents are unchanged. Integrate it with the existing
just test workflow and ensure the fixture files cover the formatting path
affected by the topiary patch.
Source: Coding guidelines
Biggest lift is all the crates that we've been carrying two versions for are now coalesced. We can stop carrying the topiary patch once they release a new version.
Summary by CodeRabbit
Bug Fixes
Compatibility
Note
Upgrade dependencies and fix rustc query depth issues in SFTP and sandbox launcher
bytes,either,google-cloud-*,reqwest,tokio,uuid,russh-sftp(2.1→2.4), and others; switcheshakoniwato a pinned git revision and patches topiary crates to relaxwasm-bindgenconstraints.Box::pinin crates/minimal/src/lib.rs and crates/minimald/src/session_host.rs to prevent rustc query depth overflow during compilation.From<SftpError> for StatusReplyin crates/minimald/src/sftp.rs so SFTP errors return only a status code inSSH_FXP_STATUSresponses, without leaking host paths in error messages.russh-sftp >= 2.2semantics where defaults no longer supply dummy zeros; adds a regression test to enforce this.Macroscope summarized fa6e3bf.