fix(detectors): union scopes when duplicate nodes fold - #406
Conversation
A package listed in two dependency groups is reachable at both scopes, but folding the duplicate record kept only whichever group was processed first -- a Pipfile.lock package in both default and develop lost its development scope. Four detectors (node, gomod, gradle, maven) patched this at their own call sites; ten others silently dropped the scope. Move the rule into the shared fold point: detectors.EnsureNode now unions the incoming record's scopes onto the survivor, and the per-detector AddScope merges that duplicated it are deleted. EnsureOccurrence decides distinct-occurrence identity before inserting, so a record that stays its own occurrence never leaks its scope onto the node it collided with. Fixes #400 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 33 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
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 |
Bomly Diff SummaryCompared Overview
Dependency Changes✅ No dependency changes. Vulnerabilities✅ No vulnerability changes. License Changes✅ No license changes. Project Posture✅ No project posture changes ( Policy Findings✅ No policy differences were identified. |
2 participants
Fixes #400.
What
When one package appears twice in a manifest, the duplicate node folds and its scopes went with it — a
Pipfile.lockpackage listed in bothdefaultanddevelopkept only the scope of whichever group was processed first. Four detectors (node, gomod, gradle, maven) hand-merged scopes at their own fold sites; ten others silently dropped them.How
detectors.EnsureNode(the fold point centralized in feat(sbom): detector-asserted package origin in SBOM export #397, calledAddNodeFoldingin the issue) now unions the incoming record's scopes onto the survivor. Scopes answer "where is this package reachable?", so a fold takes the union; everything else on the discarded record is still dropped.EnsureOccurrencenow decides fold-vs-distinct-occurrence before inserting, so a record that stays a distinct occurrence (conflictingResolvedURL) never leaks its scope onto the node it collided with. Same identity semantics as before, restated without the double insert.addOrMergeModuleNodescope parameter, maven's and gradle's post-foldAddScope(PrimaryScope()), gradle's hand-rolled lookup-then-AddScopefor project references, and the four node-family sites (npm list tree, npm/pnpm/yarn lockfile parsers). Every one of those nodes already carries its scope viasdk.ScopesOfat construction. Edge-time scope propagation (composer/cargo/mix/nuget/etc. marking scopes on lookups driven by manifest sections) is a different mechanism and is untouched.Behavior change
Duplicate-node scope semantics change for every detector using the helper: a package reachable at more than one scope now carries all of them, not just the first-seen one. The previously hand-merging detectors are unchanged in effect (their nodes carried a single scope, and union ⊇
PrimaryScopemerge).Tests
TestEnsureNodeUnionsScopesOnFold— fold unions scopes.TestEnsureOccurrenceScopes— distinct occurrence keeps its own scope and leaks nothing; same-resolution fold unions.TestDepGraphFromPipfileLockPackageInBothGroupsKeepsBothScopes— the issue's reproduction:requestsin both groups carries runtime + development,pyteststays development-only.make testpasses with no golden drift. Smoke goldens may move where a real-repo package is reachable at more than one scope — if the smoke run flags drift, dispatch Update Smoke Goldens after review.🤖 Generated with Claude Code