Skip to content

Remove ambient fs - #448

Open
leynos wants to merge 8 commits into
mainfrom
remove-ambient-fs
Open

Remove ambient fs#448
leynos wants to merge 8 commits into
mainfrom
remove-ambient-fs

Conversation

@leynos

@leynos leynos commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary by Sourcery

Remove the ambient_fs crate and adopt direct standard-library filesystem operations while updating documentation and tooling to reflect the new ambient filesystem strategy and installation flow.

New Features:

  • Document and support installing Netsuke from crates.io via cargo install.
  • Describe the experimental AST-based clone detection substrate and its feature extraction model.

Enhancements:

  • Replace ambient_fs-based executable and path handling with direct std::fs metadata checks and canonicalization in the which resolver and workspace search.
  • Clarify the rationale for Netsuke’s ambient filesystem usage and capability-scoped operations in the developer and lint configuration guides.
  • Update wording and examples in the Whitaker user guide to better describe installer behaviour, rolling releases, and experimental lints.

Build:

  • Simplify the Cargo workspace by removing the ambient_fs member and dependency, keeping Netsuke as the single publishable crate.

CI:

  • Adjust Dependabot cargo configuration to drop the ambient_fs directory from automated update coverage.

Documentation:

  • Expand README installation instructions to cover crates.io installation alongside source-based installation.
  • Add detailed documentation for the clone detection AST feature extraction pipeline and parser pinning responsibilities.

Tests:

  • Extend documentation example tests to validate the new crates.io installation snippet in the README.

Chores:

  • Update dylint configuration to treat Netsuke as the sanctioned ambient filesystem user instead of the removed ambient_fs crate.

leynos added 4 commits July 27, 2026 23:03
Remove the unpublished `ambient_fs` workspace dependency and keep the
necessary ambient operations at their application call sites. Exclude
`netsuke` from `no_std_fs_operations` through `dylint.toml`, as Whitaker
requires.

Keep dependency automation, documentation, and lockfiles aligned with the
single-crate layout. Register the crates.io installation fence so the README
continues to satisfy the executable-documentation contract.
@sourcery-ai

sourcery-ai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Remove the ambient_fs crate and inline its remaining functionality into Netsuke, updating filesystem-related code paths and documentation to reflect ambient filesystem access at application boundaries while adding new clone-detection and install docs for Whitaker/Netsuke.

Flow diagram for clone-detection AST feature extraction

flowchart TD
    A["Input: source file + ByteSpan"] --> B["lower_span validates span"]
    B --> C{"parser feature enabled?"}
    C -- yes --> D["parse file with ra_ap_syntax"]
    D --> E["find smallest covering syntax subtree"]
    E --> F["normalize subtree to NormalizedTree"]
    F --> G["extract_features from NormalizedTree"]
    G --> H["count syntax kinds"]
    G --> I["apply depth weighting"]
    G --> J["collect bigrams/trigrams"]
    G --> K["compute canonical hash with PARSER_SCHEMA_VERSION"]

    C -- no --> L["AstError::ParserUnavailable"]
Loading

File-Level Changes

Change Details Files
Inline ambient executable and path canonicalization logic into the which lookup implementation using std::fs instead of the ambient_fs crate.
  • Switch is_executable to use std::fs::metadata and a local is_executable_metadata helper with Unix-specific permission checks and non-Unix file-only checks.
  • Replace ambient_fs::canonicalize with std::fs::canonicalize over Utf8Path conversions.
  • Update module-level documentation to explain deliberate ambient PATH-based lookup without capability-scoped handles.
src/stdlib/which/lookup/mod.rs
Remove ambient_fs workspace membership and dependency wiring and adjust workspace and lint configuration to treat Netsuke as the ambient boundary instead.
  • Drop ambient_fs from Cargo.toml dependencies and workspace members/default-members.
  • Update dylint.toml no_std_fs_operations exclusions from ambient_fs to netsuke, with revised rationale.
  • Adjust developers guide text to describe Netsuke as the ambient filesystem user and remove ambient_fs-specific workflow notes.
  • Simplify workspace description in Cargo.toml to reflect single publishable crate.
  • Stop Dependabot from tracking the removed ambient_fs directory.
Cargo.toml
dylint.toml
docs/developers-guide.md
.github/dependabot.yml
Replace ambient_fs-based temp file syncing and WalkDir helpers with direct std and walkdir APIs.
  • Change sync_temp_ninja_file to call sync_all on the NamedTempFile handle instead of ambient_fs::sync_file.
  • Use walkdir::DirEntry::file_type().is_dir()/is_file() checks instead of ambient_fs entry helpers in workspace search implementations for POSIX and Windows.
src/runner/process/file_io.rs
src/stdlib/which/lookup/workspace/mod.rs
src/stdlib/which/lookup/workspace/posix.rs
src/stdlib/which/lookup/workspace/windows.rs
Remove the ambient_fs crate entirely from the repository.
  • Delete ambient_fs/Cargo.toml and ambient_fs/src/lib.rs from the tree.
  • Remove ambient_fs from .gitignore, Cargo.lock-related tracking, and tooling configuration by virtue of file deletions.
ambient_fs/Cargo.toml
ambient_fs/src/lib.rs
.gitignore
Cargo.lock
test_support/Cargo.lock
Update documentation to reflect new install instructions, rolling-release guidance, lint behaviour, and experimental clone-detection AST substrate.
  • Clarify verification behaviour for cargo-dylint and dylint-link in Whitaker user guide, including Windows PATH handling and rolling-release archive expectations.
  • Refine wording for cranelift option and other guide sections, and add MD024 suppression before Ancestor context propagation.
  • Expand experimental lint scope/behaviour to describe passive helper-call collection and fingerprinting with roadmap references.
  • Add a new section describing the clone-detection AST feature extraction, its feature vectors, parser schema versioning, and parser feature gating.
  • Add crates.io install snippet to README and register the new tested example id in documentation_examples_tests.
docs/whitaker-users-guide.md
docs/developers-guide.md
README.md
tests/documentation_examples_tests.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The ambient_fs crate and its workspace wiring are removed. Netsuke uses standard-library filesystem operations, lint and tooling configuration is aligned, crates.io installation is documented, and Whitaker documentation is updated.

Changes

Ambient filesystem removal

Layer / File(s) Summary
Filesystem helper replacement
src/stdlib/which/..., src/runner/process/file_io.rs
Replace ambient filesystem helpers with standard-library metadata, canonicalisation, walkdir entry checks, and direct temporary-file syncing.
Crate and workspace cleanup
Cargo.toml, .github/dependabot.yml, ambient_fs/...
Remove the ambient_fs dependency, workspace wiring, and crate contents; update Dependabot to track test_support.
Policy and toolchain alignment
dylint.toml, docs/developers-guide.md, rust-toolchain.toml, .gitignore
Document Netsuke’s sanctioned filesystem operations, update mutation scope, add rust-analyzer, and ignore Vim swap files.
Installation and documentation updates
README.md, docs/whitaker-users-guide.md, tests/documentation_examples_tests.rs
Document crates.io installation, revise Whitaker guidance, add clone-detection documentation, and register the new README example.

Possibly related PRs

Suggested reviewers: codescene-access

Poem

Remove the crate, let standard paths flow,
Sync with the handle already in tow.
Guide crates.io installers to the shore,
Let Whitaker’s handbook reveal more.
Keep lint and workspace settings bright.


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 2 warnings)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error Only the README example ID is asserted; the crates.io install body isn’t, and the temp Ninja sync change is covered only by same-process reopen/read tests. Assert the README crates.io fence body, and add an end-to-end temp-file hand-off test that fails if sync_all is removed.
User-Facing Documentation ⚠️ Warning docs/users-guide.md only documents source install (cargo install --path .); it omits the new crates.io install flow and other user-facing changes. Add the new crates.io installation path and any changed user-facing behaviour to docs/users-guide.md, then keep it in sync with the README.
Developer Documentation ⚠️ Warning Fail: the new clone-detection section points to a missing ra_ap_syntax runbook, and roadmap item 7.3.2 is not recorded outside the user guide. Add the missing developer-guide runbook/anchor, record the clone-detection decision in a design doc or ADR, and add or check off the matching roadmap item.
✅ Passed checks (17 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: removal of the ambient_fs crate and related filesystem refactor.
Description check ✅ Passed The description directly matches the changeset, covering the crate removal, std::fs migration, documentation updates, and build changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Module-Level Documentation ✅ Passed Checked 284 Rust files; none lacked leading module docs, and the changed modules carry //! summaries explaining purpose and relationships.
Testing (Unit And Behavioural) ✅ Passed PASS: Added unit and integration tests cover workspace search, PATHEXT/canonicalisation, direct-path errors, temp-file writes, and live doc examples.
Testing (Property / Proof) ✅ Passed Only implementation swaps and doc updates landed; the changed invariants already have targeted unit coverage, and no new proof lemma or broad state invariant was introduced.
Testing (Compile-Time / Ui) ✅ Passed PASS — the PR adds unit coverage for the runtime fs changes and wires the new README install fence into the documented-example harness; no compile-time/UI contract needs trybuild or snapshots.
Unit Architecture ✅ Passed Filesystem and env access stay at explicit boundaries; lookups and writes expose fallibility via Result, and tests cover the command-like file helpers.
Domain Architecture ✅ Passed PASS: stdlib/which and runner/process own the fs calls, while core domain modules stay value-centric and no ambient_fs reference remains.
Observability ✅ Passed The changed paths already log key outcomes and failure boundaries (file writes, which resolution cache/error categories), so no observability gap appears.
Security And Privacy ✅ Passed PASS: The patch only swaps ambient_fs for std/capability-scoped file ops and updates docs/configs; no secrets, auth gaps, injection sinks, or privacy leaks appeared.
Performance And Resource Use ✅ Passed PASS: The resolver and temp-file paths keep the same bounded per-candidate metadata/canonicalisation work; no new unbounded loops, batching misses, or extra collection growth appears.
Concurrency And State ✅ Passed No shared mutable state, async tasks, locks, or global caches were added; the changes stay local to file sync and workspace path resolution, with focused tests.
Architectural Complexity And Maintainability ✅ Passed PASS: The PR removes ambient_fs and replaces it with direct std::fs calls; it adds no new layers, traits, registries, or other complexity-heavy abstractions.
Rust Compiler Lint Integrity ✅ Passed No new dead_code/unused suppressions or anchor helpers appear in the PR range; the Rust edits use direct std::fs APIs and keep imports used.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove-ambient-fs

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

codescene-access[bot]

This comment was marked as outdated.

sourcery-ai[bot]

This comment was marked as resolved.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 193baff2da

ℹ️ 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".

Comment thread dylint.toml
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@pandalump

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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 @.gitignore:
- Line 4: Update the swap-file ignore pattern in .gitignore from “.*.swp” to
“*.swp” so both hidden and non-hidden Vim swap files are ignored.

In `@docs/whitaker-users-guide.md`:
- Around line 694-695: Rename the heading “Clone Detection: AST Feature
Extraction” to “Clone detection: AST feature extraction” to follow sentence
case.
- Around line 43-46: Update the documentation sentence describing dylint-link
verification to say it is “not invoked directly for verification” rather than
“never executed,” while preserving the explanation that it operates as a linker
wrapper and forwards arguments to the underlying linker.

In `@dylint.toml`:
- Around line 19-22: Narrow the no_std_fs_operations exemption in dylint.toml so
it applies only to sanctioned call sites or the smallest targeted scope, rather
than excluding the entire netsuke crate; update docs/developers-guide.md to
accurately describe the resulting enforcement scope.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: e04a4ee2-b378-4d3b-a787-5ffc1bca9968

📥 Commits

Reviewing files that changed from the base of the PR and between 6ecbda0 and c7bb33b.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • test_support/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (16)
  • .github/dependabot.yml
  • .gitignore
  • Cargo.toml
  • README.md
  • ambient_fs/Cargo.toml
  • ambient_fs/src/lib.rs
  • docs/developers-guide.md
  • docs/whitaker-users-guide.md
  • dylint.toml
  • rust-toolchain.toml
  • src/runner/process/file_io.rs
  • src/stdlib/which/lookup/mod.rs
  • src/stdlib/which/lookup/workspace/mod.rs
  • src/stdlib/which/lookup/workspace/posix.rs
  • src/stdlib/which/lookup/workspace/windows.rs
  • tests/documentation_examples_tests.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • leynos/rstest-bdd (auto-detected)
  • leynos/ortho-config (auto-detected)
  • leynos/shared-actions (auto-detected)
💤 Files with no reviewable changes (4)
  • ambient_fs/Cargo.toml
  • ambient_fs/src/lib.rs
  • Cargo.toml
  • .github/dependabot.yml

Comment thread .gitignore Outdated
Comment thread docs/whitaker-users-guide.md
Comment thread docs/whitaker-users-guide.md
Comment thread dylint.toml
leynos added 3 commits July 28, 2026 09:45
Add the registry installation command alongside the source checkout flow.
Extend the executable documentation contract so the user guide and README
cannot silently diverge.
codescene-access[bot]

This comment was marked as outdated.

@codescene-access codescene-access 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.

No quality gates enabled for this code.

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