Require explicit recursive workspace lookup (#646) - #672
Draft
leynos wants to merge 2 commits into
Draft
Conversation
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
Contributor
Reviewer's GuideThis PR changes executable lookup to a PATH-only default and introduces an explicit Sequence diagram for explicit recursive workspace executable lookupsequenceDiagram
participant Caller
participant WhichResolver
participant Lookup
participant Workspace
Caller->>WhichResolver: resolve(command, options)
WhichResolver->>Lookup: PATH lookup
alt PATH match found
Lookup-->>WhichResolver: matches
else PATH miss
alt cwd_mode == WorkspaceRecursive
Lookup->>Workspace: search_workspace(env.cwd, command, options.all, workspace_skips)
Workspace-->>Lookup: discovered paths
Lookup-->>WhichResolver: matches or not_found
else other cwd_mode
Lookup-->>WhichResolver: not_found
end
end
WhichResolver-->>Caller: result
Flow diagram for PATH-only default and recursive opt-inflowchart TD
A["which or command_available"] --> B{"cwd_mode"}
B -->|auto| C["Search PATH only"]
B -->|always| D["Search workspace root, then PATH"]
B -->|never| E["Search non-empty PATH entries"]
B -->|workspace-recursive| F["Search PATH first"]
F --> G{"PATH miss?"}
G -->|yes| H["search_workspace"]
G -->|no| I["Return PATH match"]
C --> J["Missing command remains absent"]
D --> J
E --> J
H --> K["Return bounded workspace match or not_found"]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
force-pushed
the
issue-646-require-explicit-opt-in-for-recursive-workspace-executable-resolution
branch
from
September 4, 2026 11:20
9852e7b to
c72a789
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please investigate the cause of the following issue using codegraph exploration and research, identify a fix and provide an AI coding agent prompt for the fix: https://github.com/leynos/netsuke/actions/runs/33867515631/job/101005632437?pr=672 Seek a systemic fix rather than tactical. Ask yourself, can this happen again or happen elsewhere? If so, think about a long term fix of the underlying issue. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== changed files =='
git diff --name-only "$(git merge-base HEAD origin/main)" HEAD 2>/dev/null || git diff --name-only HEAD~1 HEAD
printf '%s\n' '== relevant file discovery =='
fd -t f -e rs . src tests | sort | rg '(which|ambient_sync|file_io|mock|env)'
printf '%s\n' '== outlines =='
for f in $(fd -t f -e rs . src tests | rg '(^|/)(env|mod|tests|ambient_sync)\.rs$|which'); do
ast-grep outline "$f" --items all --type function,impl,struct,enum,trait 2>/dev/null || true
done
printf '%s\n' '== environment snapshot and mock usage =='
rg -n -C 5 -P '\b(EnvSnapshot|capture_with_env|capture_impl|MockEnv|os_string|PATH|Path)\b' src tests
printf '%s\n' '== ambient synchronisation call graph =='
rg -n -C 5 -P '\b(sync_temp_ninja_file|ambient_sync|capture_with_env|EnvSnapshot::capture)\s*\(' src tests
printf '%s\n' '== target test and test serialisation/environment mutation helpers =='
rg -n -C 8 -P '\bworkspace_only_executable_requires_recursive_mode\b|#\[(case|test|serial|tokio::test)' src tests
printf '%s\n' '== PR patch for affected code =='
git diff "$(git merge-base HEAD origin/main)" HEAD -- src tests 2>/dev/null || git diff HEAD~1 HEAD -- src testsLength of output: 151802 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== EnvSnapshot implementation =='
env_file=$(fd -t f '^env\.rs$' src/stdlib/which | head -n1)
nl -ba "$env_file" | sed -n '1,360p'
echo '== failing test region =='
test_file=$(fd -t f '^tests\.rs$' src/stdlib/which/lookup | head -n1)
nl -ba "$test_file" | sed -n '300,440p'
echo '== all capture_with_env callers and MockEnv configuration =='
rg -n -C 10 -P '\b(capture_with_env|capture_impl|MockEnv::new|expect_os_string|expect_raw)\b' src tests
echo '== Env trait and MockEnv methods in lockfile/source metadata =='
rg -n -C 6 -P 'trait Env\b|fn os_string\b|fn raw\b|mockable' Cargo.toml Cargo.lock src tests
echo '== Windows-only which environment tests =='
rg -n -C 8 -P '#\[cfg\(windows\)\]|PATHEXT|capture_with_pathext|capture_with_env' src/stdlib/which
echo '== exact changed lines in EnvSnapshot and lookup test =='
git diff --unified=80 "$(git merge-base HEAD origin/main)" HEAD -- src/stdlib/which/env.rs src/stdlib/which/lookup/tests.rsLength of output: 1367 🏁 Script executed: #!/bin/bash
set -euo pipefail
env_file=$(fd -t f '^env\.rs$' src/stdlib/which | head -n1)
test_file=$(fd -t f '^tests\.rs$' src/stdlib/which/lookup | head -n1)
echo "== $env_file =="
sed -n '1,360p' "$env_file"
echo "== $test_file lines 300-440 =="
sed -n '300,440p' "$test_file"
echo '== all mock environment fixtures and capture calls =='
rg -n -C 10 -P '\b(capture_with_env|capture_impl|MockEnv::new|expect_os_string|expect_raw)\b' src tests
echo '== Windows-specific capture behaviour =='
rg -n -C 10 -P '#\[cfg\(windows\)\]|PATHEXT|capture_with_pathext' src/stdlib/which
echo '== relevant dependency source, if cached =='
fd -t f 'env.rs' "${HOME}/.cargo/registry/src" 2>/dev/null | rg '/mockable-[^/]+/src/env\.rs$' | head -n 3 | while read -r f; do
echo "== $f ==";
sed -n '1,160p' "$f";
sed -n '380,450p' "$f";
done
echo '== exact PR patch for the affected test and environment module =='
git diff --unified=80 "$(git merge-base HEAD origin/main)" HEAD -- src/stdlib/which/env.rs src/stdlib/which/lookup/tests.rsLength of output: 103438
Fix the incomplete
On Windows, env.os_string("PATHEXT")
Apply a systemic fix. Do not add a one-off expectation only in Create a shared test-only environment builder for
For the unset-PATH test, return Add a Windows regression assertion for the unset-PATH snapshot. Verify that it completes with Use this coding-agent prompt: You are interacting with an AI system. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Keep the default `which` and `command_available` search domain limited to PATH, and require `cwd_mode="workspace-recursive"` before scanning checkout-controlled executables. Preserve the existing recursive walk behind that explicit mode, with its cache, skip-list, canonicalization, executable, and platform behaviour. Document the trust boundary and update the complete test and diagnostic coverage.
Add an accessible flow diagram showing the flat and recursive search paths selected by each `cwd_mode` value.
leynos
force-pushed
the
issue-646-require-explicit-opt-in-for-recursive-workspace-executable-resolution
branch
from
September 8, 2026 13:06
8eee9eb to
d2a2b70
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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
1 participant
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
This branch makes recursive workspace executable discovery an explicit
cwd_mode='workspace-recursive'opt-in. Defaultwhichandcommand_availablenow search only PATH, preventing an empty or unset PATHfrom resolving a checkout-controlled helper.
Closes #646.
Review walkthrough
Validation
make check-fmt: passedmake lint: passedmake doc-coverage: passed (99.14%)make test: passed (2,813 nextest tests; all applicable doctests passed)Summary by Sourcery
Make executable discovery PATH-only by default and require an explicit workspace-recursive opt-in for checkout-controlled command resolution.
New Features:
workspace-recursiveexecutable discovery mode for trusted manifests.Bug Fixes:
PATHvalues from implicitly resolving checkout-controlled executables.Enhancements:
auto,always,never, andworkspace-recursivemodes while preserving bounded recursive lookup for deliberate callers.Documentation:
Tests:
Chores:
whichdiagnostics for the breaking behavior change.References