Repository navigation
fix(deps): honor source and output overrides - #11896
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDependency providers now support configurable source and output paths. ChangesDependency path overrides
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DepsProviderConfig
participant PnpmDepsProvider
participant ProviderBase
participant DepsEngine
DepsProviderConfig->>PnpmDepsProvider: load source and output overrides
PnpmDepsProvider->>ProviderBase: resolve sources(...) and outputs(...)
ProviderBase-->>PnpmDepsProvider: return tracked paths and output-rule hash
PnpmDepsProvider->>DepsEngine: provide dependency paths
DepsEngine->>DepsEngine: record and validate required and optional outputs
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/deps/providers/mod.rs`:
- Around line 94-115: Update resolve_path_patterns to treat '[' as a glob
metacharacter alongside '*', '{', and '?', allowing bracket character classes
such as inputs/[ab].lock to be expanded through glob; add a regression test
covering matching bracket-pattern files.
🪄 Autofix
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: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e4eade8-8038-46bd-95a8-30d41cdedad6
📒 Files selected for processing (19)
docs/dev-tools/deps.mde2e/cli/test_deps_builtin_path_overridessrc/deps/providers/aube.rssrc/deps/providers/bun.rssrc/deps/providers/bundler.rssrc/deps/providers/composer.rssrc/deps/providers/custom.rssrc/deps/providers/dart.rssrc/deps/providers/deno.rssrc/deps/providers/git_submodule.rssrc/deps/providers/go.rssrc/deps/providers/mod.rssrc/deps/providers/npm.rssrc/deps/providers/pip.rssrc/deps/providers/pnpm.rssrc/deps/providers/poetry.rssrc/deps/providers/uv.rssrc/deps/providers/yarn.rssrc/deps/rule.rs
c863c52 to
ae6cdbc
Compare
Greptile SummaryThis PR adds replacement semantics for dependency-provider source and output overrides and persists expanded output matches to detect deleted glob members.
Confidence Score: 4/5The PR is not yet safe to merge because an incomplete recovery run can permanently forget a deleted required glob output. Persisted output matches correctly trigger the first recovery run, but the successful-run save path replaces that history with only currently existing matches, allowing subsequent checks to report freshness when the command did not recreate every previously tracked output. Files Needing Attention: src/deps/engine.rs Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/deps/engine.rs (1)
719-729: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftBind recorded outputs to the active output configuration.
Line 1027 enforces every historical
seen_outputspath even when the current configuration replaces those outputs or setsoutputs = []. If a prior default output was recorded and then deleted, an explicit empty override still returnsOutputsMissingand reruns the provider. A replacement override has the same problem for its removed old paths.Persist a stable identity for the declared output rules with
seen_outputs. Ignore or replace historical paths when that identity differs. Do not derive the identity from expanded glob matches, because that would hide deletion of a glob match.
src/deps/engine.rs#L719-L729: Store the declared output-rule identity with the observed output paths.src/deps/engine.rs#L1027-L1039: Enforce historical paths only when their stored identity matches the active output rules.This conflicts with the stated requirement that explicit output arrays replace defaults and empty arrays disable tracking.
🤖 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 `@src/deps/engine.rs` around lines 719 - 729, In src/deps/engine.rs lines 719-729, update the DepsState persistence in the provider output-recording flow to store a stable identity of the declared output rules alongside seen_outputs, derived from the configured rules rather than expanded glob matches. In src/deps/engine.rs lines 1027-1039, update the historical-output enforcement to require a matching stored identity before checking prior paths, so changed, replacement, or explicitly empty output configurations do not enforce stale paths; preserve enforcement when the identities match.
🤖 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.
Outside diff comments:
In `@src/deps/engine.rs`:
- Around line 719-729: In src/deps/engine.rs lines 719-729, update the DepsState
persistence in the provider output-recording flow to store a stable identity of
the declared output rules alongside seen_outputs, derived from the configured
rules rather than expanded glob matches. In src/deps/engine.rs lines 1027-1039,
update the historical-output enforcement to require a matching stored identity
before checking prior paths, so changed, replacement, or explicitly empty output
configurations do not enforce stale paths; preserve enforcement when the
identities match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ea914bd-4302-45a5-a348-362e40bdd874
📒 Files selected for processing (3)
e2e/cli/test_deps_builtin_path_overridessrc/deps/engine.rssrc/deps/state.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- e2e/cli/test_deps_builtin_path_overrides
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 `@src/deps/providers/mod.rs`:
- Around line 115-120: Update resolve_path_patterns at the glob invocation to
expand brace alternatives before calling glob, preserving all expanded source
and required-output patterns instead of dropping them on glob errors. Add
regression tests covering brace patterns for both sources and required outputs,
then run mise run test:unit.
🪄 Autofix
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: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ace3f712-4c4b-4afb-b2ee-8d7bcc9e8f81
📒 Files selected for processing (4)
e2e/cli/test_deps_builtin_path_overridessrc/deps/engine.rssrc/deps/providers/mod.rssrc/deps/state.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/deps/engine.rs
Summary
sourcesandoutputswere omitted or explicitly configuredoutputsis explicitly configuredMotivation
Built-in deps providers accepted
sourcesandoutputsin the configuration schema, but their implementations always returned provider-specific defaults. This made custom virtualenv locations and isolated monorepo layouts impossible to track accurately.The new behavior is a full replacement: omitted fields retain the built-in defaults, explicit arrays replace them, and
[]intentionally disables that category of tracking. Applicability still uses each provider's canonical lockfile or manifest.Verification
mise exec -- cargo test --all-features deps::mise run test:e2e e2e/cli/test_deps_builtin_path_overrides e2e/cli/test_deps e2e/config/test_schema_deps_provider_arraysmise exec -- cargo check --all-featuresmise exec -- cargo clippy --workspace --all-features --all-targets -- -D warningsThe E2E coverage verifies pnpm-style isolated outputs, an explicit empty uv output list, and independent relative path resolution for monorepo config roots.
Addresses #8762
AI-assisted — Tool: Codex; model: OpenAI/GPT-5; version: unavailable.
Summary by CodeRabbit
New Features
Documentation
Tests