Skip to content

Biiiiigg deps upgrade & coalesce - #1167

Merged
twitchyliquid64 merged 3 commits into
mainfrom
tom/sftp
Aug 3, 2026
Merged

Biiiiigg deps upgrade & coalesce#1167
twitchyliquid64 merged 3 commits into
mainfrom
tom/sftp

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Aug 3, 2026

Copy link
Copy Markdown
Member

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

    • Improved SFTP error responses with consistent status codes and safer messages that avoid exposing host-path details.
    • Corrected synthetic root directory metadata so unsupported size, ownership, and timestamp fields remain unset.
    • Improved reliability when launching commands and sandbox environments, without changing their behavior.
  • Compatibility

    • Updated core dependency versions and compatibility constraints to improve builds across supported components.

Note

Upgrade dependencies and fix rustc query depth issues in SFTP and sandbox launcher

  • Updates multiple crates in Cargo.toml: bytes, either, google-cloud-*, reqwest, tokio, uuid, russh-sftp (2.1→2.4), and others; switches hakoniwa to a pinned git revision and patches topiary crates to relax wasm-bindgen constraints.
  • Wraps async futures in Box::pin in crates/minimal/src/lib.rs and crates/minimald/src/session_host.rs to prevent rustc query depth overflow during compilation.
  • Adds From<SftpError> for StatusReply in crates/minimald/src/sftp.rs so SFTP errors return only a status code in SSH_FXP_STATUS responses, without leaking host paths in error messages.
  • Updates the synthetic SFTP root to leave size/uid/gid/atime/mtime unset, aligning with russh-sftp >= 2.2 semantics where defaults no longer supply dummy zeros; adds a regression test to enforce this.
  • Behavioral Change: SFTP error responses no longer include human-readable IO error messages.

Macroscope summarized fa6e3bf.

twitchyliquid64 and others added 2 commits August 3, 2026 14:09
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>
@twitchyliquid64
twitchyliquid64 requested a review from a team as a code owner August 3, 2026 21:36
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR updates workspace dependency constraints, pins selected Git revisions, pins two asynchronous futures, and revises SFTP error conversion and synthetic-root metadata behavior.

Changes

Workspace, runtime, and SFTP changes

Layer / File(s) Summary
Dependency resolution constraints
Cargo.toml
Workspace dependencies and selected Git revisions are updated. Topiary crates receive crates.io patches for dependency resolution.
Pinned asynchronous futures
crates/minimal/src/lib.rs, crates/minimald/src/session_host.rs
The command-dispatch and environment-build futures are boxed and pinned before instrumentation or awaiting.
SFTP response and metadata behavior
crates/minimald/src/sftp.rs
SFTP errors convert to StatusReply values without host-path messages. Synthetic-root metadata exposes directory mode 0555 and leaves inode fields unset. Tests verify the metadata fields.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: norrietaylor

Poem

A rabbit pins futures neat,
Updates crates in careful rows,
SFTP replies shed paths,
Root metadata softly shows,
“0555,” the burrow knows.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the main changes but omits the required Testing and Checklist sections, including test evidence and checklist status. Add the required Testing and Checklist sections, document commands and results, and confirm documentation and breaking-change requirements.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title identifies the dependency upgrade and version coalescing, which are the primary changes in the pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tom/sftp

Comment @coderabbitai help to get the list of available commands.

@bryan-minimal bryan-minimal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@twitchyliquid64
twitchyliquid64 enabled auto-merge (rebase) August 3, 2026 21:42

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 90a76b6 and ccb5030.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • Cargo.toml
  • crates/minimal/src/lib.rs
  • crates/minimald/src/session_host.rs
  • crates/minimald/src/sftp.rs

Comment thread Cargo.toml
Comment on lines +204 to +228

# 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" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ 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 --list

Repository: 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
done

Repository: 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
done

Repository: 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.lock

Repository: 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 180

Repository: 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.lock

Repository: 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

@twitchyliquid64
twitchyliquid64 merged commit 3b26ad6 into main Aug 3, 2026
30 checks passed
@twitchyliquid64
twitchyliquid64 deleted the tom/sftp branch August 3, 2026 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants