From a379ea90d70f0969ec12d49975999358ad2c0645 Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Tue, 28 Jul 2026 00:51:24 -0700 Subject: [PATCH 1/4] Classify dependency metadata transitions Add canonical SDK comparison results for relationship, source, and registry eligibility changes while preserving version-change semantics. Project lean before-and-after occurrence evidence into diff JSON, keep head-side enrichment authoritative, and regenerate the diff schema documentation. --- dev-docs/ARCHITECTURE.md | 22 ++++ dev-docs/MODELS.md | 11 +- docs/schemas/diff.md | 24 ++++ docs/schemas/diff.schema.json | 182 ++++++++++++++++++++++++++++ internal/output/view.go | 166 ++++++++++++++++++++++---- internal/output/view_test.go | 170 +++++++++++++++++++++++++- sdk/graph.go | 151 ++++++++++++++++++++++-- sdk/graph_test.go | 216 ++++++++++++++++++++++++++++++++++ 8 files changed, 904 insertions(+), 38 deletions(-) diff --git a/dev-docs/ARCHITECTURE.md b/dev-docs/ARCHITECTURE.md index a9ebb912..5f9e892f 100644 --- a/dev-docs/ARCHITECTURE.md +++ b/dev-docs/ARCHITECTURE.md @@ -271,6 +271,28 @@ the CLI output schema identifier remains `1.0` and the compact MCP schema remains `mcp/1`. Protocol-v1 decoding still accepts the earlier wire field from existing external auditor plugins. +### Decision: dependency metadata transitions are canonical diff results + +`sdk.Compare` classifies package version changes separately from changes to an +occurrence's dependency relationship, source, or registry-matching +eligibility. The same occurrence may appear in both lists when both kinds of +change happened. Keeping these as parallel results avoids treating a move from +direct to transitive, registry to Git, or eligible to ineligible as a package +addition or removal. + +Each transition keeps before and after evidence and an ordered list of changed +fields. Explicit detector relationships win. For older protocol-v1 graphs that +omit the relationship, the classifier derives direct or transitive from graph +edges and uses unknown when the graph cannot prove either. Exact and trusted +fuzzy identity matches call the same SDK classifier. Output code only projects +that result; it does not repeat the policy. + +Manifest results preserve duplicate occurrences. The global JSON and MCP +views deduplicate only identical evidence and use stable ordering and bounded +MCP truncation. Diff package enrichment still uses the head-side registry, so +reporting a metadata transition does not replace current vulnerability or +remediation data. + ### Decision: registry matching eligibility is an occurrence-level engine boundary Detection keeps every dependency occurrence and every PURL-backed package artifact, including application roots, workspace members, local sources, and unknown relationships. Immediately before matcher selection and execution, `engine.registryMatchRequest` clones only occurrences for which `Dependency.RegistryMatchEligible()` is true and preserves edges whose endpoints are both eligible. Every built-in and external matcher therefore receives the same filtered graph, while the full `PackageRegistry` remains shared so enrichment is still deduplicated by PURL. Analysis and auditors continue with the complete original graph. diff --git a/dev-docs/MODELS.md b/dev-docs/MODELS.md index d8e4c1d4..526df9f8 100644 --- a/dev-docs/MODELS.md +++ b/dev-docs/MODELS.md @@ -123,6 +123,7 @@ Key helpers: - `dep.PrimaryScope()`, `dep.HasScope(s)`, `dep.AddScope(s)` — scope helpers. - `sdk.DetectionLicenses(dep)` / `sdk.SetDetectionLicenses(dep, licenses)` — read/write detection-time license facts stashed in `dep.Metadata`. - `sdk.NormalizeDependencyIdentity(dep)` — canonical identity for diff matching. +- `sdk.CompareDependencyMetadata(baseGraph, headGraph, before, after)` — classify occurrence-level relationship, source, and registry-matching eligibility transitions. - `sdk.CanonicalPackageURLFromDependency(dep)` — derive the canonical PURL when the detector didn't supply one. - `sdk.RelationshipForPath(path)` — preserve an explicit relationship or derive direct/transitive from a root-to-target path. - `dep.RegistryMatchEligible()` — classify whether this occurrence may be sent to external registry enrichment. @@ -415,7 +416,15 @@ SARIF projects the same registry-resolved findings; SBOM (SPDX/CycloneDX) projects the `packages` enrichment onto components (licenses, vulnerabilities, CPEs, checksums, EOL). -`bomly diff` and `bomly explain` use the same vocabulary. SARIF and SBOM output are projected from the same registry-aware helpers; see [`../docs/OUTPUT_FORMATS.md`](../docs/OUTPUT_FORMATS.md) and [`../docs/SBOM.md`](../docs/SBOM.md) for format-specific details. +`bomly diff` and `bomly explain` use the same vocabulary. Diff reports version +changes separately from occurrence metadata transitions. A transition carries +the before and after dependency relationship, source, and registry-matching +eligibility plus an ordered list of the fields that changed. This preserves +changes that do not alter package identity or version, including changes on +duplicate occurrences in different manifests. SARIF and SBOM output are +projected from the same registry-aware helpers; see +[`../docs/OUTPUT_FORMATS.md`](../docs/OUTPUT_FORMATS.md) and +[`../docs/SBOM.md`](../docs/SBOM.md) for format-specific details. ## Common patterns diff --git a/docs/schemas/diff.md b/docs/schemas/diff.md index 02aabe58..7aded733 100644 --- a/docs/schemas/diff.md +++ b/docs/schemas/diff.md @@ -132,6 +132,28 @@ Complete reference for the `bomly diff` JSON output. | `added` | Array<[`DiffPackageChange`](#diffpackagechange)> | | | `removed` | Array<[`DiffPackageChange`](#diffpackagechange)> | | | `changed` | Array<[`DiffChangedPackage`](#diffchangedpackage)> | | +| `transitions` | Array<[`DiffDependencyTransition`](#diffdependencytransition)> | | + +### `DiffDependencyTransition` + +| Field | Type | Description | +|-------|------|-------------| +| `before` | [`DiffDependencyTransitionState`](#diffdependencytransitionstate) | | +| `after` | [`DiffDependencyTransitionState`](#diffdependencytransitionstate) | | +| `changed_fields` | Array<`string`> | | + +### `DiffDependencyTransitionState` + +| Field | Type | Description | +|-------|------|-------------| +| `id` | `string` | | +| `name` | `string` | | +| `version` | `string` | | +| `purl` | `string` | | +| `scope` | `string` | | +| `relationship` | `string` | | +| `source` | `string` | | +| `registry_eligible` | `boolean` | | ### `DiffLicenseChange` @@ -169,6 +191,7 @@ Complete reference for the `bomly diff` JSON output. | `added` | Array<[`DiffPackageChange`](#diffpackagechange)> | | | `removed` | Array<[`DiffPackageChange`](#diffpackagechange)> | | | `changed` | Array<[`DiffChangedPackage`](#diffchangedpackage)> | | +| `transitions` | Array<[`DiffDependencyTransition`](#diffdependencytransition)> | | ### `DiffPackageChange` @@ -195,6 +218,7 @@ Complete reference for the `bomly diff` JSON output. | `unchanged_manifest_count` | `integer` | | | `added_package_count` | `integer` | | | `changed_package_count` | `integer` | | +| `transitioned_package_count` | `integer` | | | `removed_package_count` | `integer` | | | `exact_match_count` | `integer` | | | `fuzzy_match_count` | `integer` | | diff --git a/docs/schemas/diff.schema.json b/docs/schemas/diff.schema.json index fc29465c..c15348c9 100644 --- a/docs/schemas/diff.schema.json +++ b/docs/schemas/diff.schema.json @@ -3116,6 +3116,95 @@ "type": "object" }, "type": "array" + }, + "transitions": { + "items": { + "properties": { + "after": { + "properties": { + "id": { + "type": "string" + }, + "name": { + "type": "string" + }, + "purl": { + "type": "string" + }, + "registry_eligible": { + "type": "boolean" + }, + "relationship": { + "type": "string" + }, + "scope": { + "type": "string" + }, + "source": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "required": [ + "id", + "name", + "relationship", + "registry_eligible" + ], + "type": "object" + }, + "before": { + "properties": { + "id": { + "type": "string" + }, + "name": { + "type": "string" + }, + "purl": { + "type": "string" + }, + "registry_eligible": { + "type": "boolean" + }, + "relationship": { + "type": "string" + }, + "scope": { + "type": "string" + }, + "source": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "required": [ + "id", + "name", + "relationship", + "registry_eligible" + ], + "type": "object" + }, + "changed_fields": { + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "before", + "after", + "changed_fields" + ], + "type": "object" + }, + "type": "array" } }, "type": "object" @@ -7008,6 +7097,95 @@ }, "subproject": { "type": "string" + }, + "transitions": { + "items": { + "properties": { + "after": { + "properties": { + "id": { + "type": "string" + }, + "name": { + "type": "string" + }, + "purl": { + "type": "string" + }, + "registry_eligible": { + "type": "boolean" + }, + "relationship": { + "type": "string" + }, + "scope": { + "type": "string" + }, + "source": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "required": [ + "id", + "name", + "relationship", + "registry_eligible" + ], + "type": "object" + }, + "before": { + "properties": { + "id": { + "type": "string" + }, + "name": { + "type": "string" + }, + "purl": { + "type": "string" + }, + "registry_eligible": { + "type": "boolean" + }, + "relationship": { + "type": "string" + }, + "scope": { + "type": "string" + }, + "source": { + "type": "string" + }, + "version": { + "type": "string" + } + }, + "required": [ + "id", + "name", + "relationship", + "registry_eligible" + ], + "type": "object" + }, + "changed_fields": { + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "before", + "after", + "changed_fields" + ], + "type": "object" + }, + "type": "array" } }, "required": [ @@ -9877,6 +10055,9 @@ "removed_package_count": { "type": "integer" }, + "transitioned_package_count": { + "type": "integer" + }, "unchanged_manifest_count": { "type": "integer" }, @@ -9891,6 +10072,7 @@ "unchanged_manifest_count", "added_package_count", "changed_package_count", + "transitioned_package_count", "removed_package_count", "exact_match_count", "fuzzy_match_count", diff --git a/internal/output/view.go b/internal/output/view.go index eefde4a9..3ddc1f3c 100644 --- a/internal/output/view.go +++ b/internal/output/view.go @@ -83,9 +83,10 @@ type DiffResults struct { // DiffDependencyResults aggregates package changes across all manifests. type DiffDependencyResults struct { - Added []DiffPackageChange `json:"added,omitempty"` - Removed []DiffPackageChange `json:"removed,omitempty"` - Changed []DiffChangedPackage `json:"changed,omitempty"` + Added []DiffPackageChange `json:"added,omitempty"` + Removed []DiffPackageChange `json:"removed,omitempty"` + Changed []DiffChangedPackage `json:"changed,omitempty"` + Transitions []DiffDependencyTransition `json:"transitions,omitempty"` } // DiffLicenseResults aggregates license changes across all manifests. @@ -134,31 +135,54 @@ type DiffChangedPackage struct { Before PackageRef `json:"before"` } +// DiffDependencyTransition is one same-identity occurrence whose +// relationship, source, or registry-matching eligibility changed. +type DiffDependencyTransition struct { + Before DiffDependencyTransitionState `json:"before"` + After DiffDependencyTransitionState `json:"after"` + ChangedFields []sdk.DependencyMetadataField `json:"changed_fields"` +} + +// DiffDependencyTransitionState preserves the identity and metadata of one +// occurrence before or after a transition. +type DiffDependencyTransitionState struct { + ID string `json:"id"` + Name string `json:"name"` + Version string `json:"version,omitempty"` + Purl string `json:"purl,omitempty"` + Scope string `json:"scope,omitempty"` + Relationship string `json:"relationship"` + Source sdk.DependencySource `json:"source,omitempty"` + RegistryEligible bool `json:"registry_eligible"` +} + // DiffManifestResult describes changes for one manifest. type DiffManifestResult struct { - Status string `json:"status"` - Path string `json:"path,omitempty"` - Kind sdk.ManifestKind `json:"kind,omitempty"` - Subproject string `json:"subproject,omitempty"` - Ecosystem sdk.Ecosystem `json:"ecosystem,omitempty"` - PackageManager sdk.PackageManager `json:"package_manager,omitempty"` - Added []DiffPackageChange `json:"added,omitempty"` - Removed []DiffPackageChange `json:"removed,omitempty"` - Changed []DiffChangedPackage `json:"changed,omitempty"` + Status string `json:"status"` + Path string `json:"path,omitempty"` + Kind sdk.ManifestKind `json:"kind,omitempty"` + Subproject string `json:"subproject,omitempty"` + Ecosystem sdk.Ecosystem `json:"ecosystem,omitempty"` + PackageManager sdk.PackageManager `json:"package_manager,omitempty"` + Added []DiffPackageChange `json:"added,omitempty"` + Removed []DiffPackageChange `json:"removed,omitempty"` + Changed []DiffChangedPackage `json:"changed,omitempty"` + Transitions []DiffDependencyTransition `json:"transitions,omitempty"` } // DiffSummary aggregates manifest and package counts for a diff. type DiffSummary struct { - AddedManifestCount int `json:"added_manifest_count"` - ChangedManifestCount int `json:"changed_manifest_count"` - RemovedManifestCount int `json:"removed_manifest_count"` - UnchangedManifestCount int `json:"unchanged_manifest_count"` - AddedPackageCount int `json:"added_package_count"` - ChangedPackageCount int `json:"changed_package_count"` - RemovedPackageCount int `json:"removed_package_count"` - ExactMatchCount int `json:"exact_match_count"` - FuzzyMatchCount int `json:"fuzzy_match_count"` - UnmatchedPackageCount int `json:"unmatched_package_count"` + AddedManifestCount int `json:"added_manifest_count"` + ChangedManifestCount int `json:"changed_manifest_count"` + RemovedManifestCount int `json:"removed_manifest_count"` + UnchangedManifestCount int `json:"unchanged_manifest_count"` + AddedPackageCount int `json:"added_package_count"` + ChangedPackageCount int `json:"changed_package_count"` + TransitionedPackageCount int `json:"transitioned_package_count"` + RemovedPackageCount int `json:"removed_package_count"` + ExactMatchCount int `json:"exact_match_count"` + FuzzyMatchCount int `json:"fuzzy_match_count"` + UnmatchedPackageCount int `json:"unmatched_package_count"` } // ExplainResponse is the structured payload for the explain command. @@ -615,7 +639,7 @@ func diffResultsFromConsolidated(baseConsolidated, headConsolidated sdk.Consolid filterSBOMPseudoPackageDiff(&manifestDiff, baseManifest.Graph, headManifest.Graph) } exactMatches := len(manifestDiff.Updated) - reconcileDiffWithFuzzyMatches(&manifestDiff) + reconcileDiffWithFuzzyMatches(&manifestDiff, baseManifest.Graph, headManifest.Graph) summary.ExactMatchCount += exactMatches summary.FuzzyMatchCount += len(manifestDiff.Updated) - exactMatches summary.UnmatchedPackageCount += len(manifestDiff.Added) + len(manifestDiff.Removed) @@ -631,14 +655,16 @@ func diffResultsFromConsolidated(baseConsolidated, headConsolidated sdk.Consolid Added: diffPackageChangesFromPackages(manifestDiff.Added, headRegistry, headDirect), Removed: diffPackageChangesFromPackages(manifestDiff.Removed, baseRegistry, baseDirect), Changed: diffChangedPackagesFromDiff(manifestDiff.Updated, baseRegistry, headRegistry, baseDirect, headDirect), + Transitions: diffDependencyTransitionsFromDiff(manifestDiff.Transitions), } - if len(result.Added) == 0 && len(result.Removed) == 0 && len(result.Changed) == 0 { + if len(result.Added) == 0 && len(result.Removed) == 0 && len(result.Changed) == 0 && len(result.Transitions) == 0 { summary.UnchangedManifestCount++ } else { result.Status = "changed" summary.ChangedManifestCount++ summary.AddedPackageCount += len(result.Added) summary.ChangedPackageCount += len(result.Changed) + summary.TransitionedPackageCount += len(result.Transitions) summary.RemovedPackageCount += len(result.Removed) } results.Manifests = append(results.Manifests, result) @@ -669,6 +695,7 @@ func aggregateDependencyChanges(manifests []DiffManifestResult) DiffDependencyRe added := make(map[string]DiffPackageChange) removed := make(map[string]DiffPackageChange) changed := make(map[string]DiffChangedPackage) + transitions := make(map[string]DiffDependencyTransition) for _, manifest := range manifests { for _, change := range manifest.Added { added[change.Package.ID] = change @@ -680,6 +707,9 @@ func aggregateDependencyChanges(manifests []DiffManifestResult) DiffDependencyRe key := change.Before.ID + "->" + change.After.ID changed[key] = change } + for _, transition := range manifest.Transitions { + transitions[diffDependencyTransitionKey(transition)] = transition + } } out := DiffDependencyResults{} for _, change := range added { @@ -691,12 +721,38 @@ func aggregateDependencyChanges(manifests []DiffManifestResult) DiffDependencyRe for _, change := range changed { out.Changed = append(out.Changed, change) } + for _, transition := range transitions { + out.Transitions = append(out.Transitions, transition) + } sort.Slice(out.Added, func(i, j int) bool { return out.Added[i].Package.ID < out.Added[j].Package.ID }) sort.Slice(out.Removed, func(i, j int) bool { return out.Removed[i].Package.ID < out.Removed[j].Package.ID }) sort.Slice(out.Changed, func(i, j int) bool { return out.Changed[i].After.ID < out.Changed[j].After.ID }) + sort.Slice(out.Transitions, func(i, j int) bool { + return diffDependencyTransitionKey(out.Transitions[i]) < diffDependencyTransitionKey(out.Transitions[j]) + }) return out } +func diffDependencyTransitionKey(transition DiffDependencyTransition) string { + fields := make([]string, 0, len(transition.ChangedFields)) + for _, field := range transition.ChangedFields { + fields = append(fields, string(field)) + } + return strings.Join([]string{ + transition.Before.ID, + transition.After.ID, + transition.Before.Scope, + transition.After.Scope, + transition.Before.Relationship, + transition.After.Relationship, + string(transition.Before.Source), + string(transition.After.Source), + strconv.FormatBool(transition.Before.RegistryEligible), + strconv.FormatBool(transition.After.RegistryEligible), + strings.Join(fields, ","), + }, "\x00") +} + func aggregateLicenseChanges(dependencies DiffDependencyResults) DiffLicenseResults { out := DiffLicenseResults{} for _, change := range dependencies.Added { @@ -970,6 +1026,18 @@ func filterSBOMPseudoPackageDiff(diff *sdk.Diff, baseGraph, headGraph *sdk.Graph } diff.Added = filterSBOMPseudoPackages(diff.Added, headGraph) diff.Removed = filterSBOMPseudoPackages(diff.Removed, baseGraph) + if len(diff.Transitions) > 0 { + baseRoots := graphRootIDs(baseGraph) + headRoots := graphRootIDs(headGraph) + filtered := make([]sdk.DependencyMetadataTransition, 0, len(diff.Transitions)) + for _, transition := range diff.Transitions { + if isSBOMPseudoPackage(transition.Before, baseRoots) || isSBOMPseudoPackage(transition.After, headRoots) { + continue + } + filtered = append(filtered, transition) + } + diff.Transitions = filtered + } } func filterSBOMPseudoPackages(packages []*sdk.Dependency, graph *sdk.Graph) []*sdk.Dependency { @@ -1041,6 +1109,37 @@ func diffChangedPackagesFromDiff(changes []sdk.VersionChange, baseRegistry, head return out } +func diffDependencyTransitionsFromDiff(transitions []sdk.DependencyMetadataTransition) []DiffDependencyTransition { + out := make([]DiffDependencyTransition, 0, len(transitions)) + for _, transition := range transitions { + out = append(out, DiffDependencyTransition{ + Before: diffDependencyTransitionState(transition.Before, transition.BeforeRelationship, transition.BeforeRegistryEligible), + After: diffDependencyTransitionState(transition.After, transition.AfterRelationship, transition.AfterRegistryEligible), + ChangedFields: append([]sdk.DependencyMetadataField(nil), transition.ChangedFields...), + }) + } + sort.Slice(out, func(i, j int) bool { + return diffDependencyTransitionKey(out[i]) < diffDependencyTransitionKey(out[j]) + }) + return out +} + +func diffDependencyTransitionState(dependency *sdk.Dependency, relationship sdk.DependencyRelationship, eligible bool) DiffDependencyTransitionState { + if dependency == nil { + return DiffDependencyTransitionState{Relationship: string(relationship), RegistryEligible: eligible} + } + return DiffDependencyTransitionState{ + ID: dependency.ID, + Name: dependency.Name, + Version: dependency.Version, + Purl: dependency.PURL, + Scope: string(dependency.PrimaryScope()), + Relationship: string(relationship), + Source: dependency.Source, + RegistryEligible: eligible, + } +} + // directMembership captures, for one graph, the set of direct-dependency node // IDs and whether the graph carries enough hierarchy to know directness at all. type directMembership struct { @@ -1103,7 +1202,7 @@ const ( diffFuzzyTierKey = "bomly.diff.fuzzy_tier" ) -func reconcileDiffWithFuzzyMatches(diff *sdk.Diff) { +func reconcileDiffWithFuzzyMatches(diff *sdk.Diff, baseGraph, headGraph *sdk.Graph) { if diff == nil || len(diff.Added) == 0 || len(diff.Removed) == 0 { return } @@ -1154,6 +1253,9 @@ func reconcileDiffWithFuzzyMatches(diff *sdk.Diff) { before := diff.Removed[match.removedIdx] applyFuzzyMetadata(before, after, match.score, match.tier) diff.Updated = append(diff.Updated, sdk.VersionChange{Before: before, After: after}) + if transition, changed := sdk.CompareDependencyMetadata(baseGraph, headGraph, before, after); changed { + diff.Transitions = append(diff.Transitions, transition) + } matchedAdded[match.addedIdx] = struct{}{} matchedRemoved[match.removedIdx] = struct{}{} } @@ -1193,6 +1295,20 @@ func reconcileDiffWithFuzzyMatches(diff *sdk.Diff) { } return left.Before.ID < right.Before.ID }) + sort.Slice(diff.Transitions, func(i, j int) bool { + left := diff.Transitions[i] + right := diff.Transitions[j] + if left.Before.IdentityKey() != right.Before.IdentityKey() { + return left.Before.IdentityKey() < right.Before.IdentityKey() + } + if left.Before.Version != right.Before.Version { + return left.Before.Version < right.Before.Version + } + if left.After.Version != right.After.Version { + return left.After.Version < right.After.Version + } + return left.Before.ID < right.Before.ID + }) } func fuzzyReconcileScore(before, after *sdk.Dependency) (float64, string) { diff --git a/internal/output/view_test.go b/internal/output/view_test.go index 6deee94e..421c1c9a 100644 --- a/internal/output/view_test.go +++ b/internal/output/view_test.go @@ -416,6 +416,98 @@ func TestBuildDiffResponseAggregatesManifestChanges(t *testing.T) { } } +func TestBuildDiffResponseReportsDependencyMetadataTransitions(t *testing.T) { + baseGraph := sdk.New() + headGraph := sdk.New() + baseRoot := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{ + Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, + Type: sdk.PackageTypeApplication, Name: "app", FirstParty: true, + }}) + headRoot := baseRoot.Clone() + baseDependency := sdk.NewDependency(sdk.Dependency{ + Coordinates: sdk.Coordinates{ + Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, + Name: "example", Version: "1.0.0", PURL: "pkg:npm/example@1.0.0", + }, + Relationship: sdk.DependencyRelationshipDirect, + Source: sdk.DependencySourceRegistry, + PackageRef: "pkg:npm/example@1.0.0", + }) + headDependency := baseDependency.Clone() + headDependency.Relationship = sdk.DependencyRelationshipUnknown + headDependency.Source = sdk.DependencySourceGit + + for _, pair := range []struct { + graph *sdk.Graph + root *sdk.Dependency + dep *sdk.Dependency + }{ + {graph: baseGraph, root: baseRoot, dep: baseDependency}, + {graph: headGraph, root: headRoot, dep: headDependency}, + } { + if err := pair.graph.AddNode(pair.root); err != nil { + t.Fatal(err) + } + if err := pair.graph.AddNode(pair.dep); err != nil { + t.Fatal(err) + } + if err := pair.graph.AddEdge(pair.root.ID, pair.dep.ID); err != nil { + t.Fatal(err) + } + } + + baseConsolidated, err := consolidation.ConsolidateGraphs(singleManifestDiffResults(baseGraph)) + if err != nil { + t.Fatalf("ConsolidateGraphs(base) error = %v", err) + } + headConsolidated, err := consolidation.ConsolidateGraphs(singleManifestDiffResults(headGraph)) + if err != nil { + t.Fatalf("ConsolidateGraphs(head) error = %v", err) + } + headRegistry := sdk.NewPackageRegistry() + headRegistry.Ensure(headDependency.PURL).Remediation = &sdk.PackageRemediation{ + Status: sdk.PackageRemediationComplete, + RecommendedVersion: "1.1.0", + } + response := output.BuildDiffResponse( + "/tmp/demo", "base", "head", baseConsolidated, headConsolidated, nil, + time.Now().Add(-time.Second), + output.ReportOptions{HeadRegistry: headRegistry}, + ) + + if response.Summary.TransitionedPackageCount != 1 || response.Summary.ChangedPackageCount != 0 { + t.Fatalf("unexpected diff summary: %#v", response.Summary) + } + if len(response.Results.Dependencies.Transitions) != 1 || len(response.Results.Manifests[0].Transitions) != 1 { + t.Fatalf("transition projection missing: %#v", response.Results) + } + transition := response.Results.Dependencies.Transitions[0] + if transition.Before.Relationship != "direct" || transition.After.Relationship != "unknown" { + t.Fatalf("relationship transition = %#v", transition) + } + if transition.Before.Source != sdk.DependencySourceRegistry || transition.After.Source != sdk.DependencySourceGit { + t.Fatalf("source transition = %#v", transition) + } + if !transition.Before.RegistryEligible || transition.After.RegistryEligible { + t.Fatalf("registry eligibility transition = %#v", transition) + } + if len(transition.ChangedFields) != 3 { + t.Fatalf("ChangedFields = %#v", transition.ChangedFields) + } + if len(response.Packages) != 1 || response.Packages[0].Remediation == nil || response.Packages[0].Remediation.RecommendedVersion != "1.1.0" { + t.Fatalf("head-side remediation was not preserved: %#v", response.Packages) + } + encoded, err := json.Marshal(response) + if err != nil { + t.Fatalf("marshal diff response: %v", err) + } + if response.SchemaVersion != "1.0" || + !strings.Contains(string(encoded), `"transitioned_package_count":1`) || + !strings.Contains(string(encoded), `"changed_fields":["relationship","source","registry_eligibility"]`) { + t.Fatalf("dependency transition JSON contract is incomplete: %s", encoded) + } +} + func TestBuildDiffResponseEnrichesPackageDeltasFromRegistries(t *testing.T) { baseGraph := sdk.New() headGraph := sdk.New() @@ -909,9 +1001,17 @@ func TestBuildDiffResponseFuzzyReconcilesRenamedPackage(t *testing.T) { headGraph := sdk.New() baseApp := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, Name: "app", Version: "1.0.0"}}) - baseDep := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, Name: "left-pad", Version: "1.0.0"}}) + baseDep := sdk.NewDependency(sdk.Dependency{ + Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, Name: "left-pad", Version: "1.0.0"}, + Relationship: sdk.DependencyRelationshipDirect, + Source: sdk.DependencySourceRegistry, + }) headApp := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, Name: "app", Version: "1.0.0"}}) - headDep := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, Name: "leftpad", Version: "1.1.0"}}) + headDep := sdk.NewDependency(sdk.Dependency{ + Coordinates: sdk.Coordinates{Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, Name: "leftpad", Version: "1.1.0"}, + Relationship: sdk.DependencyRelationshipTransitive, + Source: sdk.DependencySourceGit, + }) for _, pkg := range []*sdk.Dependency{baseApp, baseDep} { if err := baseGraph.AddNode(pkg); err != nil { @@ -979,6 +1079,14 @@ func TestBuildDiffResponseFuzzyReconcilesRenamedPackage(t *testing.T) { if len(response.Results.Manifests) != 1 || len(response.Results.Manifests[0].Changed) != 1 { t.Fatalf("expected one changed package in manifest, got %#v", response.Results.Manifests) } + if response.Summary.TransitionedPackageCount != 1 || len(response.Results.Manifests[0].Transitions) != 1 { + t.Fatalf("expected fuzzy reconciliation to preserve the metadata transition, got %#v", response.Results) + } + transition := response.Results.Manifests[0].Transitions[0] + if transition.Before.Relationship != "direct" || transition.After.Relationship != "transitive" || + transition.Before.Source != sdk.DependencySourceRegistry || transition.After.Source != sdk.DependencySourceGit { + t.Fatalf("unexpected fuzzy metadata transition: %#v", transition) + } changed := response.Results.Manifests[0].Changed[0] if changed.After.Metadata == nil { t.Fatalf("expected fuzzy metadata on reconciled package: %#v", changed.After) @@ -988,6 +1096,64 @@ func TestBuildDiffResponseFuzzyReconcilesRenamedPackage(t *testing.T) { } } +func TestBuildDiffResponseKeepsDuplicateOccurrenceTransitionsPerManifest(t *testing.T) { + newOccurrenceGraph := func(t *testing.T, relationship sdk.DependencyRelationship, source sdk.DependencySource) *sdk.Graph { + t.Helper() + graph := sdk.New() + root := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{ + Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, + Type: sdk.PackageTypeApplication, Name: "app", FirstParty: true, + }}) + dependency := sdk.NewDependency(sdk.Dependency{ + Coordinates: sdk.Coordinates{ + Ecosystem: sdk.EcosystemNPM, PackageManager: sdk.PackageManagerNPM, + Name: "shared", Version: "1.0.0", PURL: "pkg:npm/shared@1.0.0", + }, + Relationship: relationship, + Source: source, + PackageRef: "pkg:npm/shared@1.0.0", + }) + if err := graph.AddNode(root); err != nil { + t.Fatal(err) + } + if err := graph.AddNode(dependency); err != nil { + t.Fatal(err) + } + if err := graph.AddEdge(root.ID, dependency.ID); err != nil { + t.Fatal(err) + } + return graph + } + consolidated := func(first, second *sdk.Graph) sdk.ConsolidatedGraph { + return sdk.ConsolidatedGraph{Manifests: []sdk.ConsolidatedManifest{ + {Entry: sdk.GraphEntry{Graph: first, Manifest: sdk.ManifestMetadata{Path: "apps/a/package-lock.json", Kind: "package-lock.json"}}}, + {Entry: sdk.GraphEntry{Graph: second, Manifest: sdk.ManifestMetadata{Path: "apps/b/package-lock.json", Kind: "package-lock.json"}}}, + }} + } + + base := consolidated( + newOccurrenceGraph(t, sdk.DependencyRelationshipDirect, sdk.DependencySourceRegistry), + newOccurrenceGraph(t, sdk.DependencyRelationshipDirect, sdk.DependencySourceRegistry), + ) + head := consolidated( + newOccurrenceGraph(t, sdk.DependencyRelationshipTransitive, sdk.DependencySourceRegistry), + newOccurrenceGraph(t, sdk.DependencyRelationshipDirect, sdk.DependencySourceGit), + ) + + response := output.BuildDiffResponse("/tmp/demo", "base", "head", base, head, nil, time.Now()) + if response.Summary.TransitionedPackageCount != 2 { + t.Fatalf("TransitionedPackageCount = %d, want 2", response.Summary.TransitionedPackageCount) + } + if len(response.Results.Manifests) != 2 || + len(response.Results.Manifests[0].Transitions) != 1 || + len(response.Results.Manifests[1].Transitions) != 1 { + t.Fatalf("duplicate occurrence transitions were not preserved per manifest: %#v", response.Results.Manifests) + } + if len(response.Results.Dependencies.Transitions) != 2 { + t.Fatalf("aggregate should retain distinct occurrence evidence: %#v", response.Results.Dependencies.Transitions) + } +} + func newViewTestGraph(t *testing.T) *sdk.Graph { t.Helper() g := sdk.New() diff --git a/sdk/graph.go b/sdk/graph.go index 93656e49..86414f61 100644 --- a/sdk/graph.go +++ b/sdk/graph.go @@ -26,9 +26,10 @@ type Path struct { // Diff summarizes the dependency changes between two graphs. type Diff struct { - Added []*Dependency - Removed []*Dependency - Updated []VersionChange + Added []*Dependency + Removed []*Dependency + Updated []VersionChange + Transitions []DependencyMetadataTransition } // VersionChange captures a dependency identity that changed versions. @@ -37,6 +38,34 @@ type VersionChange struct { After *Dependency } +// DependencyMetadataField identifies one occurrence property that changed +// independently of package identity or version. +type DependencyMetadataField string + +const ( + // DependencyMetadataRelationship is a direct, transitive, or unknown + // relationship change. + DependencyMetadataRelationship DependencyMetadataField = "relationship" + // DependencyMetadataSource is a registry, workspace, file, Git, URL, or + // project source change. + DependencyMetadataSource DependencyMetadataField = "source" + // DependencyMetadataRegistryEligibility indicates that external registry + // matching eligibility changed. + DependencyMetadataRegistryEligibility DependencyMetadataField = "registry_eligibility" +) + +// DependencyMetadataTransition captures same-identity occurrence metadata +// changes. Version changes remain represented separately by VersionChange. +type DependencyMetadataTransition struct { + Before *Dependency + After *Dependency + ChangedFields []DependencyMetadataField + BeforeRelationship DependencyRelationship + AfterRelationship DependencyRelationship + BeforeRegistryEligible bool + AfterRegistryEligible bool +} + // Graph stores dependency nodes as a directed graph. type Graph struct { indexByID map[string]int @@ -356,15 +385,20 @@ func (g *Graph) PrettyTree() string { return strings.TrimSuffix(b.String(), "\n") } -// Compare returns the added, removed, and updated dependencies between base and -// head. Synthetic consolidated subproject nodes are ignored. +// Compare returns added, removed, version-changed, and metadata-transitioned +// dependencies between base and head. Synthetic consolidated subproject nodes +// are ignored. func Compare(base, head *Graph) Diff { baseExact, headExact := indexDiffableNodes(base), indexDiffableNodes(head) baseRemainder := make(map[string]*Dependency) headRemainder := make(map[string]*Dependency) + transitions := make([]DependencyMetadataTransition, 0) for id, node := range baseExact { - if _, ok := headExact[id]; ok { + if headNode, ok := headExact[id]; ok { + if transition, changed := CompareDependencyMetadata(base, head, node, headNode); changed { + transitions = append(transitions, transition) + } continue } baseRemainder[id] = node @@ -387,9 +421,10 @@ func Compare(base, head *Graph) Diff { } diff := Diff{ - Added: make([]*Dependency, 0), - Removed: make([]*Dependency, 0), - Updated: make([]VersionChange, 0), + Added: make([]*Dependency, 0), + Removed: make([]*Dependency, 0), + Updated: make([]VersionChange, 0), + Transitions: transitions, } for key := range identities { baseNodes := baseByIdentity[key] @@ -402,7 +437,12 @@ func Compare(base, head *Graph) Diff { pairs = len(headNodes) } for i := 0; i < pairs; i++ { - diff.Updated = append(diff.Updated, VersionChange{Before: baseNodes[i], After: headNodes[i]}) + before := baseNodes[i] + after := headNodes[i] + diff.Updated = append(diff.Updated, VersionChange{Before: before, After: after}) + if transition, changed := CompareDependencyMetadata(base, head, before, after); changed { + diff.Transitions = append(diff.Transitions, transition) + } } if pairs < len(baseNodes) { diff.Removed = append(diff.Removed, baseNodes[pairs:]...) @@ -428,9 +468,100 @@ func Compare(base, head *Graph) Diff { } return left.Before.ID < right.Before.ID }) + sortDependencyMetadataTransitions(diff.Transitions) return diff } +// CompareDependencyMetadata returns a transition when relationship, source, or +// registry-matching eligibility differs between two occurrences. It is +// exported so trusted fuzzy identity reconciliation can use the same canonical +// classifier as Compare. +func CompareDependencyMetadata(baseGraph, headGraph *Graph, before, after *Dependency) (DependencyMetadataTransition, bool) { + if before == nil || after == nil { + return DependencyMetadataTransition{}, false + } + beforeRelationship := dependencyRelationshipForDiff(baseGraph, before) + afterRelationship := dependencyRelationshipForDiff(headGraph, after) + beforeEligible := before.RegistryMatchEligible() + afterEligible := after.RegistryMatchEligible() + changedFields := make([]DependencyMetadataField, 0, 3) + if beforeRelationship != afterRelationship { + changedFields = append(changedFields, DependencyMetadataRelationship) + } + if before.Source != after.Source { + changedFields = append(changedFields, DependencyMetadataSource) + } + if beforeEligible != afterEligible { + changedFields = append(changedFields, DependencyMetadataRegistryEligibility) + } + if len(changedFields) == 0 { + return DependencyMetadataTransition{}, false + } + return DependencyMetadataTransition{ + Before: before, + After: after, + ChangedFields: changedFields, + BeforeRelationship: beforeRelationship, + AfterRelationship: afterRelationship, + BeforeRegistryEligible: beforeEligible, + AfterRegistryEligible: afterEligible, + }, true +} + +func dependencyRelationshipForDiff(graph *Graph, node *Dependency) DependencyRelationship { + if node == nil { + return DependencyRelationshipUnknown + } + if node.Relationship != "" { + return node.Relationship + } + if graph == nil || graph.Size() == 0 { + return DependencyRelationshipUnknown + } + roots := graph.Roots() + if len(roots) == 0 || len(roots) == graph.Size() { + return DependencyRelationshipUnknown + } + for _, root := range roots { + if root == nil || root.ID == node.ID { + continue + } + children, err := graph.DirectDependencies(root.ID) + if err != nil { + continue + } + for _, child := range children { + if child != nil && child.ID == node.ID { + return DependencyRelationshipDirect + } + } + } + if parents, err := graph.Dependents(node.ID); err == nil && len(parents) > 0 { + return DependencyRelationshipTransitive + } + return DependencyRelationshipUnknown +} + +func sortDependencyMetadataTransitions(transitions []DependencyMetadataTransition) { + sort.Slice(transitions, func(i, j int) bool { + left := transitions[i] + right := transitions[j] + if left.Before.IdentityKey() != right.Before.IdentityKey() { + return left.Before.IdentityKey() < right.Before.IdentityKey() + } + if left.Before.Version != right.Before.Version { + return left.Before.Version < right.Before.Version + } + if left.After.Version != right.After.Version { + return left.After.Version < right.After.Version + } + if left.Before.ID != right.Before.ID { + return left.Before.ID < right.Before.ID + } + return left.After.ID < right.After.ID + }) +} + func indexDiffableNodes(g *Graph) map[string]*Dependency { indexed := make(map[string]*Dependency) if g == nil { diff --git a/sdk/graph_test.go b/sdk/graph_test.go index b3ab75b8..6de94e87 100644 --- a/sdk/graph_test.go +++ b/sdk/graph_test.go @@ -466,6 +466,222 @@ func TestCompare_ClassifiesAddedRemovedAndUpdated(t *testing.T) { if diff.Updated[0].Before.ID != "update@1.0.0" || diff.Updated[0].After.ID != "update@2.0.0" { t.Fatalf("unexpected updated node: %#v", diff.Updated[0]) } + if len(diff.Transitions) != 0 { + t.Fatalf("unexpected metadata transitions: %#v", diff.Transitions) + } +} + +func TestCompare_ClassifiesDependencyMetadataTransitions(t *testing.T) { + base := New() + head := New() + baseRoot := NewDependency(Dependency{Coordinates: Coordinates{Type: PackageTypeApplication, Name: "app", FirstParty: true}}) + headRoot := baseRoot.Clone() + baseDependency := NewDependency(Dependency{ + Coordinates: Coordinates{Ecosystem: EcosystemNPM, Name: "example", Version: "1.0.0"}, + Relationship: DependencyRelationshipDirect, + Source: DependencySourceRegistry, + }) + headDependency := baseDependency.Clone() + headDependency.Relationship = DependencyRelationshipUnknown + headDependency.Source = DependencySourceGit + + for _, pair := range []struct { + graph *Graph + root *Dependency + dep *Dependency + }{ + {graph: base, root: baseRoot, dep: baseDependency}, + {graph: head, root: headRoot, dep: headDependency}, + } { + if err := pair.graph.AddNode(pair.root); err != nil { + t.Fatal(err) + } + if err := pair.graph.AddNode(pair.dep); err != nil { + t.Fatal(err) + } + if err := pair.graph.AddEdge(pair.root.ID, pair.dep.ID); err != nil { + t.Fatal(err) + } + } + + diff := Compare(base, head) + if len(diff.Added) != 0 || len(diff.Removed) != 0 || len(diff.Updated) != 0 { + t.Fatalf("metadata-only diff changed package identity/version buckets: %#v", diff) + } + if len(diff.Transitions) != 1 { + t.Fatalf("Transitions = %#v, want one", diff.Transitions) + } + transition := diff.Transitions[0] + wantFields := []DependencyMetadataField{ + DependencyMetadataRelationship, + DependencyMetadataSource, + DependencyMetadataRegistryEligibility, + } + if !slices.Equal(transition.ChangedFields, wantFields) { + t.Fatalf("ChangedFields = %#v, want %#v", transition.ChangedFields, wantFields) + } + if transition.BeforeRelationship != DependencyRelationshipDirect || transition.AfterRelationship != DependencyRelationshipUnknown { + t.Fatalf("relationship transition = %q -> %q", transition.BeforeRelationship, transition.AfterRelationship) + } + if !transition.BeforeRegistryEligible || transition.AfterRegistryEligible { + t.Fatalf("registry eligibility transition = %t -> %t", transition.BeforeRegistryEligible, transition.AfterRegistryEligible) + } +} + +func TestCompareDependencyMetadataClassifiesEachAxisIndependently(t *testing.T) { + base := NewDependency(Dependency{ + Coordinates: Coordinates{ + Ecosystem: EcosystemNPM, + Name: "example", + Version: "1.0.0", + PURL: "pkg:npm/example@1.0.0", + }, + Relationship: DependencyRelationshipDirect, + Source: DependencySourceRegistry, + }) + tests := []struct { + name string + after func() *Dependency + want DependencyMetadataField + }{ + { + name: "relationship only", + after: func() *Dependency { + after := base.Clone() + after.Relationship = DependencyRelationshipTransitive + return after + }, + want: DependencyMetadataRelationship, + }, + { + name: "source only", + after: func() *Dependency { + after := base.Clone() + after.Source = "" + return after + }, + want: DependencyMetadataSource, + }, + { + name: "registry eligibility only", + after: func() *Dependency { + after := base.Clone() + after.FirstParty = true + return after + }, + want: DependencyMetadataRegistryEligibility, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + transition, changed := CompareDependencyMetadata(nil, nil, base, tt.after()) + if !changed { + t.Fatal("CompareDependencyMetadata() did not report a transition") + } + if len(transition.ChangedFields) != 1 || transition.ChangedFields[0] != tt.want { + t.Fatalf("ChangedFields = %#v, want [%s]", transition.ChangedFields, tt.want) + } + }) + } +} + +func TestCompareSortsDependencyMetadataTransitions(t *testing.T) { + base := New() + head := New() + for _, name := range []string{"zeta", "alpha"} { + before := NewDependency(Dependency{ + Coordinates: Coordinates{Ecosystem: EcosystemNPM, Name: name, Version: "1.0.0"}, + Source: DependencySourceRegistry, + }) + after := before.Clone() + after.Source = DependencySourceGit + if err := base.AddNode(before); err != nil { + t.Fatal(err) + } + if err := head.AddNode(after); err != nil { + t.Fatal(err) + } + } + + diff := Compare(base, head) + if len(diff.Transitions) != 2 { + t.Fatalf("Transitions = %#v, want two", diff.Transitions) + } + if diff.Transitions[0].Before.Name != "alpha" || diff.Transitions[1].Before.Name != "zeta" { + t.Fatalf("transitions are not stable: %#v", diff.Transitions) + } +} + +func TestCompare_DerivesRelationshipTransitionFromGraphEdges(t *testing.T) { + base := New() + head := New() + for _, graph := range []*Graph{base, head} { + for _, node := range []*Dependency{ + NewDependency(Dependency{Coordinates: Coordinates{Type: PackageTypeApplication, Name: "app", FirstParty: true}}), + NewDependency(Dependency{Coordinates: Coordinates{Ecosystem: EcosystemNPM, Name: "parent", Version: "1.0.0"}}), + NewDependency(Dependency{Coordinates: Coordinates{Ecosystem: EcosystemNPM, Name: "child", Version: "1.0.0"}}), + } { + if err := graph.AddNode(node); err != nil { + t.Fatal(err) + } + } + } + appID := NewDependencyRef("app", "").ID + parentID := NewDependency(Dependency{Coordinates: Coordinates{Ecosystem: EcosystemNPM, Name: "parent", Version: "1.0.0"}}).ID + childID := NewDependency(Dependency{Coordinates: Coordinates{Ecosystem: EcosystemNPM, Name: "child", Version: "1.0.0"}}).ID + if err := base.AddEdge(appID, childID); err != nil { + t.Fatal(err) + } + if err := base.AddEdge(appID, parentID); err != nil { + t.Fatal(err) + } + if err := head.AddEdge(appID, parentID); err != nil { + t.Fatal(err) + } + if err := head.AddEdge(parentID, childID); err != nil { + t.Fatal(err) + } + + diff := Compare(base, head) + if len(diff.Transitions) != 1 { + t.Fatalf("Transitions = %#v, want one", diff.Transitions) + } + transition := diff.Transitions[0] + if !slices.Equal(transition.ChangedFields, []DependencyMetadataField{DependencyMetadataRelationship}) { + t.Fatalf("ChangedFields = %#v", transition.ChangedFields) + } + if transition.BeforeRelationship != DependencyRelationshipDirect || transition.AfterRelationship != DependencyRelationshipTransitive { + t.Fatalf("relationship transition = %q -> %q", transition.BeforeRelationship, transition.AfterRelationship) + } +} + +func TestCompare_ReportsVersionAndMetadataChangesSeparately(t *testing.T) { + base := New() + head := New() + before := NewDependency(Dependency{ + Coordinates: Coordinates{Ecosystem: EcosystemNPM, Name: "example", Version: "1.0.0"}, + Relationship: DependencyRelationshipDirect, + Source: DependencySourceRegistry, + }) + after := NewDependency(Dependency{ + Coordinates: Coordinates{Ecosystem: EcosystemNPM, Name: "example", Version: "2.0.0"}, + Relationship: DependencyRelationshipTransitive, + Source: DependencySourceRegistry, + }) + if err := base.AddNode(before); err != nil { + t.Fatal(err) + } + if err := head.AddNode(after); err != nil { + t.Fatal(err) + } + + diff := Compare(base, head) + if len(diff.Updated) != 1 || len(diff.Transitions) != 1 { + t.Fatalf("Compare() = %#v, want one version change and one metadata transition", diff) + } + if !slices.Equal(diff.Transitions[0].ChangedFields, []DependencyMetadataField{DependencyMetadataRelationship}) { + t.Fatalf("ChangedFields = %#v", diff.Transitions[0].ChangedFields) + } } func TestCompare_IgnoresSyntheticSubprojectRoots(t *testing.T) { From 19749891d3439a26fba239d99f7f57a5e79b35e3 Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Tue, 28 Jul 2026 00:51:36 -0700 Subject: [PATCH 2/4] Show dependency detail changes in diff outputs Expose canonical transition evidence through text, Markdown, the interactive diff view, and compact MCP responses. Use plain user-facing labels, cap compact transition records with explicit truncation counts, and document how version and dependency-detail changes coexist. --- docs/ARCHITECTURE.md | 2 +- docs/OUTPUT_FORMATS.md | 13 ++ internal/cli/diff_cmd_test.go | 2 +- internal/cli/render/diff.go | 55 +++++++- internal/cli/render/diff_markdown.go | 43 +++++- internal/cli/render/diff_markdown_test.go | 58 +++++++++ internal/cli/root_cmd_test.go | 2 +- internal/mcp/compact_diff.go | 152 +++++++++++++++++----- internal/mcp/compact_diff_test.go | 63 ++++++++- internal/mcp/types_compact.go | 28 ++-- internal/tui/diff.go | 113 ++++++++++++++-- internal/tui/diff_aggregations_test.go | 57 ++++++++ internal/tui/utils.go | 4 + 13 files changed, 524 insertions(+), 68 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d8702cb7..b8288995 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -38,7 +38,7 @@ flowchart TD 5. **Audit** — When you pass `--audit`, [auditors](AUDITORS.md) evaluate policy (severity thresholds, license rules, denied packages) against the enriched data and produce findings. As part of this same step, configured policy-status rules may mark a finding non-gating without removing it. Combine `--enrich --audit` to gate on fresh external data in one run. 6. **Render** — Bomly emits the result as text, JSON, SARIF, or an SBOM. See [Output formats](OUTPUT_FORMATS.md) and [SBOM formats](SBOM.md). -`bomly explain` reuses the detect and match stages, then traces the dependency paths that pull in a given package. `bomly diff` runs the pipeline against two states and reports what changed. +`bomly explain` reuses the detect and match stages, then traces the dependency paths that pull in a given package. `bomly diff` runs the pipeline against two states and reports package additions, removals, version changes, and changes to dependency relationship, source, or registry-matching eligibility. ## Configuration trust diff --git a/docs/OUTPUT_FORMATS.md b/docs/OUTPUT_FORMATS.md index 0a0d090f..1ff9e0f5 100644 --- a/docs/OUTPUT_FORMATS.md +++ b/docs/OUTPUT_FORMATS.md @@ -95,6 +95,19 @@ independent of a package version bump. A finding present on both sides is `persisted`; it is not reported as one resolved finding plus one introduced finding merely because the affected package version changed. +Dependency changes are split into separate kinds. A version change says that +the package release changed. A metadata transition says that the same package +occurrence changed in one of these ways: + +- its relationship changed between direct, transitive, and unknown; +- its source changed, such as registry to Git or workspace; +- its eligibility for registry matching changed. + +A dependency can have both a version change and a metadata transition in the +same diff. JSON keeps the before and after evidence under +`results.dependencies.transitions` and under the matching manifest. Text, +Markdown, the interactive view, and MCP show the same classification. + ## `sarif` — CI security tools SARIF 2.1.0. Findings only. One result per (rule × package) pair. Includes: diff --git a/internal/cli/diff_cmd_test.go b/internal/cli/diff_cmd_test.go index 02483a9c..6bd76e88 100644 --- a/internal/cli/diff_cmd_test.go +++ b/internal/cli/diff_cmd_test.go @@ -231,7 +231,7 @@ func TestRenderDiffMarkdownIncludesPatchedVersionsByDefault(t *testing.T) { for _, want := range []string{ "# Bomly Diff Summary", "Compared `main` to `feature`.", - "**Summary:** 1 added, 1 changed, 0 removed.", + "**Summary:** 1 added, 1 version changed, 0 with detail changes, 0 removed.", "| added | react@18.2.0 | 18.2.0 | - | unknown | - |", "| changed | zod | 3.22.0 → 3.23.0 | - | unknown | - |", "## Vulnerabilities", diff --git a/internal/cli/render/diff.go b/internal/cli/render/diff.go index 2e9129e0..18410ba9 100644 --- a/internal/cli/render/diff.go +++ b/internal/cli/render/diff.go @@ -55,7 +55,7 @@ func dependencyTextSections(results output.DiffDependencyResults) []string { if len(results.Changed) == 0 { return } - lines = append(lines, Style(fmt.Sprintf("Changed (%d)", len(results.Changed)), Bold)) + lines = append(lines, Style(fmt.Sprintf("Version changed (%d)", len(results.Changed)), Bold)) for _, change := range results.Changed { name := change.After.Name if strings.TrimSpace(name) == "" { @@ -65,15 +65,68 @@ func dependencyTextSections(results output.DiffDependencyResults) []string { lines = append(lines, Wrap(line, Yellow)) } } + appendTransitions := func() { + if len(results.Transitions) == 0 { + return + } + lines = append(lines, Style(fmt.Sprintf("Dependency detail changes (%d)", len(results.Transitions)), Bold)) + for _, transition := range results.Transitions { + name := dependencyTransitionDisplayName(transition.After) + line := fmt.Sprintf(" ↔ %s %s", name, dependencyTransitionDescription(transition)) + lines = append(lines, Wrap(line, Yellow)) + } + } appendAdded() appendRemoved() appendChanged() + appendTransitions() if len(lines) == 0 { lines = append(lines, Style("No dependency changes.", Dim)) } return lines } +func dependencyTransitionDescription(transition output.DiffDependencyTransition) string { + parts := make([]string, 0, len(transition.ChangedFields)) + for _, field := range transition.ChangedFields { + switch field { + case sdk.DependencyMetadataRelationship: + parts = append(parts, fmt.Sprintf( + "relationship: %s → %s", + valueOrDash(transition.Before.Relationship), + valueOrDash(transition.After.Relationship), + )) + case sdk.DependencyMetadataSource: + parts = append(parts, fmt.Sprintf( + "source: %s → %s", + valueOrDash(string(transition.Before.Source)), + valueOrDash(string(transition.After.Source)), + )) + case sdk.DependencyMetadataRegistryEligibility: + parts = append(parts, fmt.Sprintf( + "registry matching: %s → %s", + registryEligibilityLabel(transition.Before.RegistryEligible), + registryEligibilityLabel(transition.After.RegistryEligible), + )) + } + } + return strings.Join(parts, "; ") +} + +func dependencyTransitionDisplayName(state output.DiffDependencyTransitionState) string { + if name := strings.TrimSpace(state.Name); name != "" { + return name + } + return state.ID +} + +func registryEligibilityLabel(eligible bool) string { + if eligible { + return "eligible" + } + return "not eligible" +} + // findingsSummaryLine produces a summary line plus a list of each introduced or // persisted finding when audit data is present. N/A-severity findings (e.g. unknown // license) are omitted — they belong in the policy section, not the compact diff --git a/internal/cli/render/diff_markdown.go b/internal/cli/render/diff_markdown.go index 502c1f7a..a50e9e31 100644 --- a/internal/cli/render/diff_markdown.go +++ b/internal/cli/render/diff_markdown.go @@ -56,7 +56,13 @@ func diffOverviewMarkdown(payload output.DiffResponse) []string { [][]string{{ status, fmt.Sprintf("+%d / ~%d / -%d", payload.Summary.AddedManifestCount, payload.Summary.ChangedManifestCount, payload.Summary.RemovedManifestCount), - fmt.Sprintf("+%d / ~%d / -%d", payload.Summary.AddedPackageCount, payload.Summary.ChangedPackageCount, payload.Summary.RemovedPackageCount), + fmt.Sprintf( + "+%d / ~%d version / ~%d details / -%d", + payload.Summary.AddedPackageCount, + payload.Summary.ChangedPackageCount, + payload.Summary.TransitionedPackageCount, + payload.Summary.RemovedPackageCount, + ), fmt.Sprintf("%d introduced / %d persisted / %d resolved", introduced, persisted, resolved), humanizeDurationMS(payload.Metadata.DurationMS), }}, @@ -79,18 +85,49 @@ func humanizeDurationMS(ms int64) string { func diffDependencyMarkdown(payload output.DiffResponse) []string { results := payload.Results.Dependencies lines := []string{ - fmt.Sprintf("**Summary:** %d added, %d changed, %d removed.", len(results.Added), len(results.Changed), len(results.Removed)), + fmt.Sprintf( + "**Summary:** %d added, %d version changed, %d with detail changes, %d removed.", + len(results.Added), len(results.Changed), len(results.Transitions), len(results.Removed), + ), "", } lines = append(lines, diffAddedRemovedDependencyTable("Added Dependencies", "added", results.Added)...) lines = append(lines, diffChangedDependencyTable(results.Changed)...) + lines = append(lines, diffDependencyTransitionTable(results.Transitions)...) lines = append(lines, diffAddedRemovedDependencyTable("Removed Dependencies", "removed", results.Removed)...) - if len(results.Added) == 0 && len(results.Changed) == 0 && len(results.Removed) == 0 { + if len(results.Added) == 0 && len(results.Changed) == 0 && len(results.Transitions) == 0 && len(results.Removed) == 0 { return []string{"✅ No dependency changes."} } return trimTrailingMarkdownBlanks(lines) } +func diffDependencyTransitionTable(transitions []output.DiffDependencyTransition) []string { + if len(transitions) == 0 { + return nil + } + sorted := append([]output.DiffDependencyTransition(nil), transitions...) + sort.Slice(sorted, func(i, j int) bool { + left := sorted[i].After + right := sorted[j].After + if dependencyTransitionDisplayName(left) != dependencyTransitionDisplayName(right) { + return dependencyTransitionDisplayName(left) < dependencyTransitionDisplayName(right) + } + return left.ID < right.ID + }) + rows := make([][]string, 0, len(sorted)) + for _, transition := range sorted { + rows = append(rows, []string{ + dependencyTransitionDisplayName(transition.After), + valueOrDash(transition.After.Version), + dependencyTransitionDescription(transition), + }) + } + return append( + []string{"### Dependency Detail Changes", ""}, + append(markdownTable([]string{"Package", "Version", "Changes"}, rows), "")..., + ) +} + func diffAddedRemovedDependencyTable(title, status string, changes []output.DiffPackageChange) []string { if len(changes) == 0 { return nil diff --git a/internal/cli/render/diff_markdown_test.go b/internal/cli/render/diff_markdown_test.go index dc447d3b..6d6d9fa2 100644 --- a/internal/cli/render/diff_markdown_test.go +++ b/internal/cli/render/diff_markdown_test.go @@ -38,6 +38,64 @@ func TestDiffOverviewMarkdownPersistedWarningsAreWarnings(t *testing.T) { } } +func TestDiffTextAndMarkdownRenderDependencyMetadataTransitions(t *testing.T) { + payload := output.DiffResponse{ + Summary: output.DiffSummary{TransitionedPackageCount: 1}, + Results: output.DiffResults{Dependencies: output.DiffDependencyResults{ + Transitions: []output.DiffDependencyTransition{{ + Before: output.DiffDependencyTransitionState{ + Name: "example", + Version: "1.0.0", + Relationship: "direct", + Source: sdk.DependencySourceRegistry, + RegistryEligible: true, + }, + After: output.DiffDependencyTransitionState{ + Name: "example", + Version: "1.0.0", + Relationship: "transitive", + Source: sdk.DependencySourceGit, + RegistryEligible: false, + }, + ChangedFields: []sdk.DependencyMetadataField{ + sdk.DependencyMetadataRelationship, + sdk.DependencyMetadataSource, + sdk.DependencyMetadataRegistryEligibility, + }, + }}, + }}, + } + + var text bytes.Buffer + if err := Diff(&text, payload); err != nil { + t.Fatalf("Diff() error = %v", err) + } + for _, want := range []string{ + "Dependency detail changes (1)", + "relationship: direct → transitive", + "source: registry → git", + "registry matching: eligible → not eligible", + } { + if !strings.Contains(text.String(), want) { + t.Fatalf("text output missing %q:\n%s", want, text.String()) + } + } + + var markdown bytes.Buffer + if err := DiffMarkdown(&markdown, payload); err != nil { + t.Fatalf("DiffMarkdown() error = %v", err) + } + for _, want := range []string{ + "1 with detail changes", + "### Dependency Detail Changes", + "relationship: direct → transitive", + } { + if !strings.Contains(markdown.String(), want) { + t.Fatalf("Markdown output missing %q:\n%s", want, markdown.String()) + } + } +} + func TestHumanizeDurationMS(t *testing.T) { cases := []struct { ms int64 diff --git a/internal/cli/root_cmd_test.go b/internal/cli/root_cmd_test.go index 200fff8f..6290ed36 100644 --- a/internal/cli/root_cmd_test.go +++ b/internal/cli/root_cmd_test.go @@ -692,7 +692,7 @@ func TestRoot_DiffCommand_JSONOutputWithMarkdownOutputFile(t *testing.T) { if err != nil { t.Fatalf("read summary file: %v", err) } - for _, want := range []string{"# Bomly Diff Summary", "Compared `" + baseSHA + "` to `" + headSHA + "`.", "**Summary:** 1 added, 1 changed, 0 removed."} { + for _, want := range []string{"# Bomly Diff Summary", "Compared `" + baseSHA + "` to `" + headSHA + "`.", "**Summary:** 1 added, 1 version changed, 0 with detail changes, 0 removed."} { if !strings.Contains(string(summary), want) { t.Fatalf("expected summary to contain %q, got:\n%s", want, summary) } diff --git a/internal/mcp/compact_diff.go b/internal/mcp/compact_diff.go index 16693060..f6de258e 100644 --- a/internal/mcp/compact_diff.go +++ b/internal/mcp/compact_diff.go @@ -1,12 +1,16 @@ package mcp import ( + "sort" + "strconv" + "strings" + "github.com/bomly-dev/bomly-cli/internal/output" "github.com/bomly-dev/bomly-cli/sdk" ) // diffHint tells agents how to drill into the delta. -const diffHint = "Introduced findings are new on head; resolved close when this ref merges; persisted remain after merge. Use bomly_explain (on the head checkout) for full advisory detail of one package." +const diffHint = "Introduced findings are new on head; resolved close when this ref merges; persisted remain after merge. Dependency transitions describe relationship, source, or registry-matching changes without treating them as version changes. Use bomly_explain (on the head checkout) for full advisory detail of one package." // SecurityDelta buckets advisory findings by what merging head changes. type SecurityDelta struct { @@ -17,33 +21,53 @@ type SecurityDelta struct { // CompactDiffSummary counts manifest/package/finding deltas. type CompactDiffSummary struct { - ManifestsAdded int `json:"manifests_added,omitempty"` - ManifestsChanged int `json:"manifests_changed,omitempty"` - ManifestsRemoved int `json:"manifests_removed,omitempty"` - PackagesAdded int `json:"packages_added,omitempty"` - PackagesChanged int `json:"packages_changed,omitempty"` - PackagesRemoved int `json:"packages_removed,omitempty"` - Introduced int `json:"introduced,omitempty"` - Resolved int `json:"resolved,omitempty"` - Persisted int `json:"persisted,omitempty"` - EnrichRan bool `json:"enrich_ran"` - AuditRan bool `json:"audit_ran"` + ManifestsAdded int `json:"manifests_added,omitempty"` + ManifestsChanged int `json:"manifests_changed,omitempty"` + ManifestsRemoved int `json:"manifests_removed,omitempty"` + PackagesAdded int `json:"packages_added,omitempty"` + PackagesChanged int `json:"packages_changed,omitempty"` + PackagesTransitioned int `json:"packages_transitioned,omitempty"` + PackagesRemoved int `json:"packages_removed,omitempty"` + Introduced int `json:"introduced,omitempty"` + Resolved int `json:"resolved,omitempty"` + Persisted int `json:"persisted,omitempty"` + EnrichRan bool `json:"enrich_ran"` + AuditRan bool `json:"audit_ran"` +} + +// CompactDependencyTransitionState is one side of a dependency occurrence +// metadata transition. +type CompactDependencyTransitionState struct { + Relationship string `json:"relationship,omitempty"` + Source sdk.DependencySource `json:"source,omitempty"` + RegistryEligible bool `json:"registry_eligible"` +} + +// CompactDependencyTransition reports one same-identity metadata change +// without repeating licenses, vulnerabilities, locations, or other large +// package detail. +type CompactDependencyTransition struct { + Package PackageIdentity `json:"package"` + ChangedFields []sdk.DependencyMetadataField `json:"changed_fields"` + Before CompactDependencyTransitionState `json:"before"` + After CompactDependencyTransitionState `json:"after"` } // CompactDiffResponse is the bomly_diff tool result: the branch-aware // security delta ("what does this ref fix vs base, what remains after // merge") with integrated remediation context for what is still open. type CompactDiffResponse struct { - SchemaVersion string `json:"schema_version"` - Command string `json:"command"` - Comparison output.DiffComparison `json:"comparison"` - Summary CompactDiffSummary `json:"summary"` - SecurityDelta SecurityDelta `json:"security_delta"` - Remediations []RemediationGroup `json:"remediations,omitempty"` - Informational []CompactFinding `json:"informational,omitempty"` - Diagnostics []Diagnostic `json:"diagnostics,omitempty"` - Truncation *TruncationInfo `json:"truncation,omitempty"` - Hint string `json:"hint,omitempty"` + SchemaVersion string `json:"schema_version"` + Command string `json:"command"` + Comparison output.DiffComparison `json:"comparison"` + Summary CompactDiffSummary `json:"summary"` + SecurityDelta SecurityDelta `json:"security_delta"` + Transitions []CompactDependencyTransition `json:"dependency_transitions,omitempty"` + Remediations []RemediationGroup `json:"remediations,omitempty"` + Informational []CompactFinding `json:"informational,omitempty"` + Diagnostics []Diagnostic `json:"diagnostics,omitempty"` + Truncation *TruncationInfo `json:"truncation,omitempty"` + Hint string `json:"hint,omitempty"` } // BuildCompactDiff projects a diff run into the agent-facing compact @@ -57,17 +81,18 @@ func BuildCompactDiff(run DiffRunResult) CompactDiffResponse { Command: "diff", Comparison: run.Response.Comparison, Summary: CompactDiffSummary{ - ManifestsAdded: run.Response.Summary.AddedManifestCount, - ManifestsChanged: run.Response.Summary.ChangedManifestCount, - ManifestsRemoved: run.Response.Summary.RemovedManifestCount, - PackagesAdded: run.Response.Summary.AddedPackageCount, - PackagesChanged: run.Response.Summary.ChangedPackageCount, - PackagesRemoved: run.Response.Summary.RemovedPackageCount, - Introduced: len(run.Introduced), - Resolved: len(run.Resolved), - Persisted: len(run.Persisted), - AuditRan: run.AuditRan, - EnrichRan: run.EnrichRan, + ManifestsAdded: run.Response.Summary.AddedManifestCount, + ManifestsChanged: run.Response.Summary.ChangedManifestCount, + ManifestsRemoved: run.Response.Summary.RemovedManifestCount, + PackagesAdded: run.Response.Summary.AddedPackageCount, + PackagesChanged: run.Response.Summary.ChangedPackageCount, + PackagesTransitioned: run.Response.Summary.TransitionedPackageCount, + PackagesRemoved: run.Response.Summary.RemovedPackageCount, + Introduced: len(run.Introduced), + Resolved: len(run.Resolved), + Persisted: len(run.Persisted), + AuditRan: run.AuditRan, + EnrichRan: run.EnrichRan, }, Diagnostics: capDiagnostics(run.Diagnostics), Hint: diffHint, @@ -86,6 +111,7 @@ func BuildCompactDiff(run DiffRunResult) CompactDiffResponse { } trunc := &TruncationInfo{} + response.Transitions = compactDependencyTransitions(run.Response.Results.Dependencies.Transitions, trunc) response.SecurityDelta.Introduced = compactFindingList(run.Introduced, headInput, trunc) response.SecurityDelta.Persisted = compactFindingList(run.Persisted, headInput, trunc) response.SecurityDelta.Resolved = compactFindingList(run.Resolved, baseInput, trunc) @@ -99,7 +125,7 @@ func BuildCompactDiff(run DiffRunResult) CompactDiffResponse { response.Informational = remediation.Informational mergeTruncation(trunc, remediation.Truncation) - if trunc.OmittedFindings > 0 || trunc.OmittedGroups > 0 || trunc.OmittedPackages > 0 || trunc.OmittedPaths > 0 { + if trunc.OmittedFindings > 0 || trunc.OmittedGroups > 0 || trunc.OmittedPackages > 0 || trunc.OmittedPaths > 0 || trunc.OmittedTransitions > 0 { trunc.Truncated = true trunc.Note = "response was capped; diff a narrower path or use bomly_explain per package for the rest" response.Truncation = trunc @@ -135,4 +161,62 @@ func mergeTruncation(target, source *TruncationInfo) { target.OmittedGroups += source.OmittedGroups target.OmittedPackages += source.OmittedPackages target.OmittedPaths += source.OmittedPaths + target.OmittedTransitions += source.OmittedTransitions +} + +func compactDependencyTransitions(transitions []output.DiffDependencyTransition, trunc *TruncationInfo) []CompactDependencyTransition { + if len(transitions) == 0 { + return nil + } + sorted := append([]output.DiffDependencyTransition(nil), transitions...) + sort.Slice(sorted, func(i, j int) bool { + return compactDependencyTransitionSortKey(sorted[i]) < compactDependencyTransitionSortKey(sorted[j]) + }) + limit := len(sorted) + if limit > maxDependencyTransitions { + trunc.OmittedTransitions += limit - maxDependencyTransitions + limit = maxDependencyTransitions + } + out := make([]CompactDependencyTransition, 0, limit) + for _, transition := range sorted[:limit] { + pkg := transition.After + out = append(out, CompactDependencyTransition{ + Package: PackageIdentity{ + Name: pkg.Name, + Version: pkg.Version, + Purl: pkg.Purl, + }, + ChangedFields: append([]sdk.DependencyMetadataField(nil), transition.ChangedFields...), + Before: CompactDependencyTransitionState{ + Relationship: transition.Before.Relationship, + Source: transition.Before.Source, + RegistryEligible: transition.Before.RegistryEligible, + }, + After: CompactDependencyTransitionState{ + Relationship: transition.After.Relationship, + Source: transition.After.Source, + RegistryEligible: transition.After.RegistryEligible, + }, + }) + } + return out +} + +func compactDependencyTransitionSortKey(transition output.DiffDependencyTransition) string { + fields := make([]string, 0, len(transition.ChangedFields)) + for _, field := range transition.ChangedFields { + fields = append(fields, string(field)) + } + return strings.Join([]string{ + transition.After.Purl, + transition.After.Name, + transition.After.ID, + transition.Before.Relationship, + transition.After.Relationship, + string(transition.Before.Source), + string(transition.After.Source), + strconv.FormatBool(transition.Before.RegistryEligible), + strconv.FormatBool(transition.After.RegistryEligible), + strings.Join(fields, ","), + }, "\x00") } diff --git a/internal/mcp/compact_diff_test.go b/internal/mcp/compact_diff_test.go index 10bf1782..bcddc31f 100644 --- a/internal/mcp/compact_diff_test.go +++ b/internal/mcp/compact_diff_test.go @@ -1,6 +1,7 @@ package mcp import ( + "fmt" "testing" "github.com/bomly-dev/bomly-cli/internal/output" @@ -21,8 +22,33 @@ func TestBuildCompactDiffBucketsAndRemediation(t *testing.T) { run := DiffRunResult{ Response: output.DiffResponse{ Comparison: output.DiffComparison{Base: "main", Head: "feature"}, - Summary: output.DiffSummary{ChangedManifestCount: 1, ChangedPackageCount: 2}, - Metadata: output.Metadata{}, + Summary: output.DiffSummary{ChangedManifestCount: 1, ChangedPackageCount: 2, TransitionedPackageCount: 1}, + Results: output.DiffResults{Dependencies: output.DiffDependencyResults{ + Transitions: []output.DiffDependencyTransition{{ + Before: output.DiffDependencyTransitionState{ + Name: "lib-a", + Version: "1.0.0", + Purl: "pkg:npm/lib-a@1.0.0", + Relationship: "direct", + Source: sdk.DependencySourceRegistry, + RegistryEligible: true, + }, + After: output.DiffDependencyTransitionState{ + Name: "lib-a", + Version: "1.0.0", + Purl: "pkg:npm/lib-a@1.0.0", + Relationship: "transitive", + Source: sdk.DependencySourceGit, + RegistryEligible: false, + }, + ChangedFields: []sdk.DependencyMetadataField{ + sdk.DependencyMetadataRelationship, + sdk.DependencyMetadataSource, + sdk.DependencyMetadataRegistryEligibility, + }, + }}, + }}, + Metadata: output.Metadata{}, }, // lib-a's finding is introduced by head; deep's persists; the // old-lib finding only existed on base (resolved by this ref). @@ -74,6 +100,39 @@ func TestBuildCompactDiffBucketsAndRemediation(t *testing.T) { if compact.SchemaVersion != CompactSchemaVersion || compact.Command != "diff" { t.Fatalf("header wrong: %#v", compact) } + if compact.Summary.PackagesTransitioned != 1 || len(compact.Transitions) != 1 { + t.Fatalf("dependency transition missing: %#v", compact) + } + if compact.Transitions[0].Before.Relationship != "direct" || + compact.Transitions[0].After.Source != sdk.DependencySourceGit || + compact.Transitions[0].After.RegistryEligible { + t.Fatalf("dependency transition evidence wrong: %#v", compact.Transitions[0]) + } +} + +func TestBuildCompactDiffCapsDependencyTransitions(t *testing.T) { + transitions := make([]output.DiffDependencyTransition, 0, maxDependencyTransitions+2) + for index := 0; index < maxDependencyTransitions+2; index++ { + name := fmt.Sprintf("package-%03d", index) + transitions = append(transitions, output.DiffDependencyTransition{ + Before: output.DiffDependencyTransitionState{ + Name: name, Purl: "pkg:npm/" + name + "@1.0.0", Relationship: "direct", + }, + After: output.DiffDependencyTransitionState{ + Name: name, Purl: "pkg:npm/" + name + "@1.0.0", Relationship: "transitive", + }, + ChangedFields: []sdk.DependencyMetadataField{sdk.DependencyMetadataRelationship}, + }) + } + compact := BuildCompactDiff(DiffRunResult{Response: output.DiffResponse{ + Results: output.DiffResults{Dependencies: output.DiffDependencyResults{Transitions: transitions}}, + }}) + if len(compact.Transitions) != maxDependencyTransitions { + t.Fatalf("transition count = %d, want %d", len(compact.Transitions), maxDependencyTransitions) + } + if compact.Truncation == nil || compact.Truncation.OmittedTransitions != 2 { + t.Fatalf("transition truncation = %#v", compact.Truncation) + } } func TestBuildCompactDiffEnrichedWithoutAuditReturnsHeadRemediation(t *testing.T) { diff --git a/internal/mcp/types_compact.go b/internal/mcp/types_compact.go index 50239874..d9e13cf8 100644 --- a/internal/mcp/types_compact.go +++ b/internal/mcp/types_compact.go @@ -10,13 +10,14 @@ const CompactSchemaVersion = "mcp/1" // Compact response caps. Anything cut by a cap is counted in TruncationInfo — // never silently dropped. const ( - maxFindingsPerGroup = 15 - maxRemediationGroups = 40 - maxInformational = 60 - maxPathNodes = 6 - maxAliases = 3 - maxInventoryEntries = 200 - maxDiagnosticsReported = 20 + maxFindingsPerGroup = 15 + maxRemediationGroups = 40 + maxInformational = 60 + maxPathNodes = 6 + maxAliases = 3 + maxInventoryEntries = 200 + maxDiagnosticsReported = 20 + maxDependencyTransitions = 60 ) // Diagnostic surfaces one pipeline warning (detector fallback, matcher @@ -130,12 +131,13 @@ type CompactSummary struct { // TruncationInfo reports exactly what the caps cut so the agent knows the // response is partial and how to drill down. type TruncationInfo struct { - Truncated bool `json:"truncated"` - OmittedFindings int `json:"omitted_findings,omitempty"` - OmittedGroups int `json:"omitted_groups,omitempty"` - OmittedPackages int `json:"omitted_packages,omitempty"` - OmittedPaths int `json:"omitted_paths,omitempty"` - Note string `json:"note,omitempty"` + Truncated bool `json:"truncated"` + OmittedFindings int `json:"omitted_findings,omitempty"` + OmittedGroups int `json:"omitted_groups,omitempty"` + OmittedPackages int `json:"omitted_packages,omitempty"` + OmittedPaths int `json:"omitted_paths,omitempty"` + OmittedTransitions int `json:"omitted_transitions,omitempty"` + Note string `json:"note,omitempty"` } // CompactScanResponse is the bomly_scan tool result: remediation-grouped diff --git a/internal/tui/diff.go b/internal/tui/diff.go index 5e85e647..b29a56d5 100644 --- a/internal/tui/diff.go +++ b/internal/tui/diff.go @@ -382,7 +382,7 @@ func (m *DiffModel) diffAggregateCounts() diffAggregateCounts { s := m.payload.Summary out := diffAggregateCounts{ ManifestDeltas: s.AddedManifestCount + s.ChangedManifestCount + s.RemovedManifestCount, - PackageDeltas: s.AddedPackageCount + s.ChangedPackageCount + s.RemovedPackageCount, + PackageDeltas: s.AddedPackageCount + s.ChangedPackageCount + s.TransitionedPackageCount + s.RemovedPackageCount, } if m.payload.Audit != nil { for _, bucket := range [][]output.AuditFinding{m.payload.Audit.Introduced, m.payload.Audit.Persisted, m.payload.Audit.Resolved} { @@ -482,7 +482,8 @@ func (m *DiffModel) buildOverviewTab() *listModel { render.Style("Package Changes", render.Bold, render.Cyan), "", render.Style(" Added: ", render.Dim) + fmt.Sprintf("%d", summary.AddedPackageCount), - render.Style(" Changed: ", render.Dim) + fmt.Sprintf("%d", summary.ChangedPackageCount), + render.Style(" Version changed: ", render.Dim) + fmt.Sprintf("%d", summary.ChangedPackageCount), + render.Style(" Details changed: ", render.Dim) + fmt.Sprintf("%d", summary.TransitionedPackageCount), render.Style(" Removed: ", render.Dim) + fmt.Sprintf("%d", summary.RemovedPackageCount), }, }, @@ -585,7 +586,8 @@ type diffOverviewStats struct { // unmatchedRemoved: total Removed packages — see unmatchedAdded. unmatchedRemoved int - // changedTotal: total package-level change events (Added+Changed+Removed). + // changedTotal: total package-level change events + // (Added+VersionChanged+MetadataTransitioned+Removed). changedTotal int // auditSummaryTotal: the raw AuditSummary.Total field. Per @@ -653,7 +655,7 @@ func (m *DiffModel) computeOverviewStats() diffOverviewStats { if eco == "" { eco = "unknown" } - n := len(mf.Added) + len(mf.Changed) + len(mf.Removed) + n := len(mf.Added) + len(mf.Changed) + len(mf.Transitions) + len(mf.Removed) if n > 0 { out.ecosystems[eco] += n out.changedTotal += n @@ -672,6 +674,9 @@ func (m *DiffModel) computeOverviewStats() diffOverviewStats { out.matchedExact++ } } + for _, transition := range mf.Transitions { + addScopeRel("transitioned", transition.After.Scope, transition.After.ID) + } out.unmatchedAdded += len(mf.Added) out.unmatchedRemoved += len(mf.Removed) } @@ -989,9 +994,10 @@ func (m *DiffModel) overviewDashboardView(width, height int) string { render.Style(fmt.Sprintf("%d changed", s.ChangedManifestCount), render.Yellow), render.Style(fmt.Sprintf("%d removed", s.RemovedManifestCount), render.Red), ), cardWidth, cardHeight, render.Cyan), - boxView("Packages", summaryCountCardLines(s.AddedPackageCount+s.ChangedPackageCount+s.RemovedPackageCount, "Package Changes", cardWidth-2, render.Magenta, + boxView("Packages", summaryCountCardLines(s.AddedPackageCount+s.ChangedPackageCount+s.TransitionedPackageCount+s.RemovedPackageCount, "Package Changes", cardWidth-2, render.Magenta, render.Style(fmt.Sprintf("%d added", s.AddedPackageCount), render.Green), - render.Style(fmt.Sprintf("%d changed", s.ChangedPackageCount), render.Yellow), + render.Style(fmt.Sprintf("%d version changed", s.ChangedPackageCount), render.Yellow), + render.Style(fmt.Sprintf("%d details changed", s.TransitionedPackageCount), render.Cyan), render.Style(fmt.Sprintf("%d removed", s.RemovedPackageCount), render.Red), ), cardWidth, cardHeight, render.Magenta), boxView("Vulnerabilities", summaryCountCardLines(diffVulnTotal(m.payload), "Vuln Deltas", cardWidth-2, render.Red, @@ -1221,7 +1227,8 @@ func (m *DiffModel) overviewPanels() []listPanel { }, color: render.Cyan, weight: 1}, {title: "Packages", lines: []string{ render.Style(fmt.Sprintf("%d Added", summary.AddedPackageCount), render.Green, render.Bold), - render.Style(fmt.Sprintf("%d Changed", summary.ChangedPackageCount), render.Yellow, render.Bold), + render.Style(fmt.Sprintf("%d Version Changed", summary.ChangedPackageCount), render.Yellow, render.Bold), + render.Style(fmt.Sprintf("%d Details Changed", summary.TransitionedPackageCount), render.Cyan, render.Bold), render.Style(fmt.Sprintf("%d Removed", summary.RemovedPackageCount), render.Red, render.Bold), }, color: render.Magenta, weight: 1}, } @@ -1281,7 +1288,7 @@ func (m *DiffModel) overviewTopChangedManifests() []string { for _, mf := range m.payload.Results.Manifests { rows = append(rows, ranked{ name: render.DiffManifestDisplayLabel(mf), - count: len(mf.Added) + len(mf.Changed) + len(mf.Removed), + count: len(mf.Added) + len(mf.Changed) + len(mf.Transitions) + len(mf.Removed), }) } sort.Slice(rows, func(i, j int) bool { return rows[i].count > rows[j].count }) @@ -1312,7 +1319,7 @@ type flatComponentChange struct { manifestKey string manifestName string ecosystem string - status string // "added" / "changed" / "removed" + status string // "added" / "changed" / "transitioned" / "removed" pkgName string pkgRef output.PackageRef // After for changed, Package for added/removed beforePkg output.PackageRef // populated for changed entries @@ -1321,6 +1328,7 @@ type flatComponentChange struct { maxSeverity string // worst severity across pkgRef.Vulnerabilities relationship string // root/direct/transitive — looked up in head/base graph remediation *sdk.PackageRemediation + transition *output.DiffDependencyTransition } func (m *DiffModel) collectComponentChanges() []flatComponentChange { @@ -1375,6 +1383,20 @@ func (m *DiffModel) collectComponentChanges() []flatComponentChange { remediation: remediationForPURL(m.headRegistry, change.After.Purl), }) } + for idx := range mf.Transitions { + transition := mf.Transitions[idx] + after := packageRefFromTransitionState(transition.After) + before := packageRefFromTransitionState(transition.Before) + out = append(out, flatComponentChange{ + manifest: mf, manifestKey: mfKey, manifestName: mfName, ecosystem: eco, + status: "transitioned", pkgName: render.DiffPackageDisplayName(after), + pkgRef: after, beforePkg: before, + beforeVer: transition.Before.Version, afterVer: transition.After.Version, + relationship: transition.After.Relationship, + remediation: remediationForPURL(m.headRegistry, transition.After.Purl), + transition: &transition, + }) + } for _, change := range mf.Removed { out = append(out, flatComponentChange{ manifest: mf, manifestKey: mfKey, manifestName: mfName, ecosystem: eco, @@ -1388,6 +1410,28 @@ func (m *DiffModel) collectComponentChanges() []flatComponentChange { return out } +func packageRefFromTransitionState(state output.DiffDependencyTransitionState) output.PackageRef { + ref := output.PackageRef{ + ID: state.ID, + Name: state.Name, + Version: state.Version, + Purl: state.Purl, + Scope: state.Scope, + Relationship: state.Relationship, + Licenses: []output.LicenseRef{}, + Vulnerabilities: []output.VulnerabilityRef{}, + } + switch state.Relationship { + case string(sdk.DependencyRelationshipDirect): + direct := true + ref.Direct = &direct + case string(sdk.DependencyRelationshipTransitive): + direct := false + ref.Direct = &direct + } + return ref +} + func maxSeverity(vulns []output.VulnerabilityRef) string { best := "" bestRank := severityRank("zzz") @@ -1471,13 +1515,15 @@ func (m *DiffModel) buildComponentsTab() *listModel { } } - added, changed, removed := 0, 0, 0 + added, changed, transitioned, removed := 0, 0, 0, 0 for _, c := range filtered { switch c.status { case "added": added++ case "changed": changed++ + case "transitioned": + transitioned++ case "removed": removed++ } @@ -1492,6 +1538,7 @@ func (m *DiffModel) buildComponentsTab() *listModel { render.Style(" | Ecosystem: ", render.Dim) + render.Style(valueOrDefault(m.componentsEcosystem, "All"), render.BgYellow, render.Bold) + render.Style(" | Added: ", render.Dim) + render.Style(fmt.Sprintf("%d", added), render.Green, render.Bold) + render.Style(" | Changed: ", render.Dim) + render.Style(fmt.Sprintf("%d", changed), render.Yellow, render.Bold) + + render.Style(" | Details changed: ", render.Dim) + render.Style(fmt.Sprintf("%d", transitioned), render.Cyan, render.Bold) + render.Style(" | Removed: ", render.Dim) + render.Style(fmt.Sprintf("%d", removed), render.Red, render.Bold), } @@ -1555,7 +1602,7 @@ func sortedComponentsGroupKeys(groups map[string][]flatComponentChange, group co } sort.Slice(keys, func(i, j int) bool { if group == componentsGroupStatus { - order := map[string]int{"added": 0, "changed": 1, "removed": 2} + order := map[string]int{"added": 0, "changed": 1, "transitioned": 2, "removed": 3} return order[keys[i]] < order[keys[j]] } return keys[i] < keys[j] @@ -1570,13 +1617,14 @@ func (m *DiffModel) componentsGroupDetails(key string, group componentsGroup, it render.Style(" Group axis: ", render.Dim) + componentsGroupName(group), render.Style(" Items: ", render.Dim) + fmt.Sprintf("%d", len(items)), } - counts := map[string]int{"added": 0, "changed": 0, "removed": 0} + counts := map[string]int{"added": 0, "changed": 0, "transitioned": 0, "removed": 0} for _, c := range items { counts[c.status]++ } lines = append(lines, render.Style(" Added: ", render.Dim)+fmt.Sprintf("%d", counts["added"]), render.Style(" Changed: ", render.Dim)+fmt.Sprintf("%d", counts["changed"]), + render.Style(" Details changed: ", render.Dim)+fmt.Sprintf("%d", counts["transitioned"]), render.Style(" Removed: ", render.Dim)+fmt.Sprintf("%d", counts["removed"]), ) // When grouping by subproject, list the manifests the group spans so the @@ -1625,6 +1673,9 @@ func componentChangeRowTitle(c flatComponentChange) string { if c.status == "changed" { return fmt.Sprintf("%s (%s → %s)", c.pkgName, valueOrDash(c.beforeVer), valueOrDash(c.afterVer)) } + if c.status == "transitioned" { + return c.pkgName + " (details)" + } return c.pkgName } @@ -1634,6 +1685,8 @@ func componentChangeDetailTitle(c flatComponentChange) string { return "Added package" case "removed": return "Removed package" + case "transitioned": + return "Dependency details changed" default: return "Changed package" } @@ -1724,6 +1777,8 @@ func componentChangeDetails(c flatComponentChange) []string { } lines = append(lines, renderLicenseDelta(c.beforePkg.Licenses, c.pkgRef.Licenses)...) lines = append(lines, renderVulnDelta(c.beforePkg.Vulnerabilities, c.pkgRef.Vulnerabilities)...) + } else if c.status == "transitioned" && c.transition != nil { + lines = append(lines, renderDependencyMetadataTransition(*c.transition)...) } else { lines = append(lines, renderLicenseList(c.pkgRef.Licenses)...) lines = append(lines, renderVulnList(c.pkgRef.Vulnerabilities)...) @@ -1735,6 +1790,40 @@ func componentChangeDetails(c flatComponentChange) []string { return lines } +func renderDependencyMetadataTransition(transition output.DiffDependencyTransition) []string { + lines := []string{"", render.Style("Metadata changes", render.Bold, render.Cyan)} + for _, field := range transition.ChangedFields { + var label, before, after string + switch field { + case sdk.DependencyMetadataRelationship: + label = "Relationship" + before = valueOrDash(transition.Before.Relationship) + after = valueOrDash(transition.After.Relationship) + case sdk.DependencyMetadataSource: + label = "Source" + before = valueOrDash(string(transition.Before.Source)) + after = valueOrDash(string(transition.After.Source)) + case sdk.DependencyMetadataRegistryEligibility: + label = "Registry matching" + before = tuiRegistryEligibilityLabel(transition.Before.RegistryEligible) + after = tuiRegistryEligibilityLabel(transition.After.RegistryEligible) + default: + continue + } + lines = append(lines, + render.Style(" "+label+": ", render.Dim)+before+" → "+after, + ) + } + return lines +} + +func tuiRegistryEligibilityLabel(eligible bool) string { + if eligible { + return "eligible" + } + return "not eligible" +} + // renderLicenseList renders one package's license inventory. func renderLicenseList(licenses []output.LicenseRef) []string { out := []string{"", render.Style("Licenses", render.Bold, render.Yellow)} diff --git a/internal/tui/diff_aggregations_test.go b/internal/tui/diff_aggregations_test.go index baa65778..dd70d489 100644 --- a/internal/tui/diff_aggregations_test.go +++ b/internal/tui/diff_aggregations_test.go @@ -119,6 +119,63 @@ func TestDiffAggregateCounts_EmptyPayload(t *testing.T) { } } +func TestDiffComponentsProjectDependencyMetadataTransition(t *testing.T) { + transition := output.DiffDependencyTransition{ + Before: output.DiffDependencyTransitionState{ + ID: "example@1.0.0", + Name: "example", + Version: "1.0.0", + Relationship: "direct", + Source: sdk.DependencySourceRegistry, + RegistryEligible: true, + }, + After: output.DiffDependencyTransitionState{ + ID: "example@1.0.0", + Name: "example", + Version: "1.0.0", + Relationship: "transitive", + Source: sdk.DependencySourceGit, + RegistryEligible: false, + }, + ChangedFields: []sdk.DependencyMetadataField{ + sdk.DependencyMetadataRelationship, + sdk.DependencyMetadataSource, + sdk.DependencyMetadataRegistryEligibility, + }, + } + payload := output.DiffResponse{ + Summary: output.DiffSummary{TransitionedPackageCount: 1}, + Results: output.DiffResults{Manifests: []output.DiffManifestResult{{ + Status: "changed", + Path: "package-lock.json", + Ecosystem: sdk.EcosystemNPM, + Transitions: []output.DiffDependencyTransition{transition}, + }}}, + } + model := NewDiff(payload, sdk.ConsolidatedGraph{}, sdk.ConsolidatedGraph{}) + changes := model.collectComponentChanges() + if len(changes) != 1 || changes[0].status != "transitioned" { + t.Fatalf("component changes = %#v", changes) + } + details := strings.Join(componentChangeDetails(changes[0]), "\n") + for _, want := range []string{ + "Dependency details changed", + "Relationship:", + "direct → transitive", + "Source:", + "registry → git", + "Registry matching:", + "eligible → not eligible", + } { + if !strings.Contains(render.StripANSI(details), want) { + t.Fatalf("transition details missing %q:\n%s", want, render.StripANSI(details)) + } + } + if counts := model.diffAggregateCounts(); counts.PackageDeltas != 1 { + t.Fatalf("PackageDeltas = %d, want 1", counts.PackageDeltas) + } +} + func TestDiffAggregateCounts_LicenseDedup(t *testing.T) { // Five packages all introduce MIT — unique count should be 1, not 5. payload := output.DiffResponse{Results: output.DiffResults{Manifests: []output.DiffManifestResult{{ diff --git a/internal/tui/utils.go b/internal/tui/utils.go index 41fa437d..43edc94f 100644 --- a/internal/tui/utils.go +++ b/internal/tui/utils.go @@ -158,6 +158,8 @@ func statusBadge(status string) string { return terminalSafeBadge(label, render.BgRed, render.White) case "changed": return terminalSafeBadge(label, render.BgYellow, render.Black) + case "transitioned": + return terminalSafeBadge(label, render.BgNeutral, render.White) case "unchanged": return terminalSafeBadge(label, render.BgNeutral, render.White) case "new": // audit-delta "introduced" (display-side label) @@ -259,6 +261,8 @@ func statusText(status string) string { return render.Style(status, render.Red, render.Bold) case "changed": return render.Style(status, render.Yellow, render.Bold) + case "transitioned": + return render.Style(status, render.Cyan, render.Bold) case "unchanged": return render.Style(status, render.Cyan, render.Bold) case "new": From b34b4ccd2c7e18daaff0306cebfbcf7a78143633 Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Tue, 28 Jul 2026 01:41:06 -0700 Subject: [PATCH 3/4] Address dependency detail change review feedback Share relationship classification across path and diff comparisons, index graph relationships once, and suppress changes that rely on missing source or relationship evidence. Use consistent detail-change language across human and MCP output, fold version-plus-detail changes into one TUI row and occurrence count, and clarify vulnerability-check coverage. Add an end-to-end SBOM smoke fixture for direct-to-transitive changes and refresh all affected diff goldens. --- dev-docs/ARCHITECTURE.md | 4 +- dev-docs/MODELS.md | 4 +- docs/OUTPUT_FORMATS.md | 7 +- internal/cli/diff_cmd_test.go | 2 +- internal/cli/render/diff.go | 31 ++-- internal/cli/render/diff_markdown.go | 10 +- internal/cli/render/diff_markdown_test.go | 23 ++- internal/cli/root_cmd_test.go | 2 +- internal/mcp/compact_diff.go | 87 ++++++----- internal/mcp/compact_diff_test.go | 28 +++- internal/output/view.go | 51 +++--- internal/output/view_test.go | 12 +- internal/tui/diff.go | 144 +++++++++++++---- internal/tui/diff_aggregations_test.go | 54 ++++++- internal/tui/utils.go | 4 +- sdk/graph.go | 131 ++++++++++------ sdk/graph_test.go | 55 ++++--- sdk/relationship.go | 19 ++- test/smoke/smoke_test.go | 41 ++++- .../golden/container-diff-alpine.golden.json | 1 + .../testdata/golden/diff-go-audit.golden.json | 1 + .../smoke/testdata/golden/diff-go.golden.json | 1 + .../testdata/golden/diff-npm.golden.json | 1 + .../diff-sbom-detail-change.golden.json | 147 ++++++++++++++++++ .../testdata/golden/diff-sbom.golden.json | 1 + .../testdata/golden/lite-diff-go.golden.json | 1 + .../sboms/detail-change-base.spdx.json | 55 +++++++ .../sboms/detail-change-head.spdx.json | 74 +++++++++ 28 files changed, 758 insertions(+), 233 deletions(-) create mode 100644 test/smoke/testdata/golden/diff-sbom-detail-change.golden.json create mode 100644 test/smoke/testdata/sboms/detail-change-base.spdx.json create mode 100644 test/smoke/testdata/sboms/detail-change-head.spdx.json diff --git a/dev-docs/ARCHITECTURE.md b/dev-docs/ARCHITECTURE.md index 5f9e892f..fdb96865 100644 --- a/dev-docs/ARCHITECTURE.md +++ b/dev-docs/ARCHITECTURE.md @@ -271,7 +271,7 @@ the CLI output schema identifier remains `1.0` and the compact MCP schema remains `mcp/1`. Protocol-v1 decoding still accepts the earlier wire field from existing external auditor plugins. -### Decision: dependency metadata transitions are canonical diff results +### Decision: dependency detail changes are canonical diff results `sdk.Compare` classifies package version changes separately from changes to an occurrence's dependency relationship, source, or registry-matching @@ -290,7 +290,7 @@ that result; it does not repeat the policy. Manifest results preserve duplicate occurrences. The global JSON and MCP views deduplicate only identical evidence and use stable ordering and bounded MCP truncation. Diff package enrichment still uses the head-side registry, so -reporting a metadata transition does not replace current vulnerability or +reporting a detail change does not replace current vulnerability or remediation data. ### Decision: registry matching eligibility is an occurrence-level engine boundary diff --git a/dev-docs/MODELS.md b/dev-docs/MODELS.md index 526df9f8..c6b02bfc 100644 --- a/dev-docs/MODELS.md +++ b/dev-docs/MODELS.md @@ -123,7 +123,7 @@ Key helpers: - `dep.PrimaryScope()`, `dep.HasScope(s)`, `dep.AddScope(s)` — scope helpers. - `sdk.DetectionLicenses(dep)` / `sdk.SetDetectionLicenses(dep, licenses)` — read/write detection-time license facts stashed in `dep.Metadata`. - `sdk.NormalizeDependencyIdentity(dep)` — canonical identity for diff matching. -- `sdk.CompareDependencyMetadata(baseGraph, headGraph, before, after)` — classify occurrence-level relationship, source, and registry-matching eligibility transitions. +- `sdk.CompareDependencyDetails(baseGraph, headGraph, before, after)` — classify occurrence-level relationship, source, and registry-matching eligibility transitions. - `sdk.CanonicalPackageURLFromDependency(dep)` — derive the canonical PURL when the detector didn't supply one. - `sdk.RelationshipForPath(path)` — preserve an explicit relationship or derive direct/transitive from a root-to-target path. - `dep.RegistryMatchEligible()` — classify whether this occurrence may be sent to external registry enrichment. @@ -417,7 +417,7 @@ projects the `packages` enrichment onto components (licenses, vulnerabilities, CPEs, checksums, EOL). `bomly diff` and `bomly explain` use the same vocabulary. Diff reports version -changes separately from occurrence metadata transitions. A transition carries +changes separately from occurrence detail changes. A transition carries the before and after dependency relationship, source, and registry-matching eligibility plus an ordered list of the fields that changed. This preserves changes that do not alter package identity or version, including changes on diff --git a/docs/OUTPUT_FORMATS.md b/docs/OUTPUT_FORMATS.md index 1ff9e0f5..83652ae1 100644 --- a/docs/OUTPUT_FORMATS.md +++ b/docs/OUTPUT_FORMATS.md @@ -96,15 +96,16 @@ independent of a package version bump. A finding present on both sides is finding merely because the affected package version changed. Dependency changes are split into separate kinds. A version change says that -the package release changed. A metadata transition says that the same package +the package release changed. A detail change says that the same package occurrence changed in one of these ways: - its relationship changed between direct, transitive, and unknown; - its source changed, such as registry to Git or workspace; - its eligibility for registry matching changed. -A dependency can have both a version change and a metadata transition in the -same diff. JSON keeps the before and after evidence under +A dependency can have both a version change and a detail change in the same +diff. Structured output calls each detail-change record a `transition`. JSON +keeps the before and after evidence under `results.dependencies.transitions` and under the matching manifest. Text, Markdown, the interactive view, and MCP show the same classification. diff --git a/internal/cli/diff_cmd_test.go b/internal/cli/diff_cmd_test.go index 6bd76e88..2165ef99 100644 --- a/internal/cli/diff_cmd_test.go +++ b/internal/cli/diff_cmd_test.go @@ -231,7 +231,7 @@ func TestRenderDiffMarkdownIncludesPatchedVersionsByDefault(t *testing.T) { for _, want := range []string{ "# Bomly Diff Summary", "Compared `main` to `feature`.", - "**Summary:** 1 added, 1 version changed, 0 with detail changes, 0 removed.", + "**Summary:** 1 added, 1 version changed, 0 detail changes, 0 removed.", "| added | react@18.2.0 | 18.2.0 | - | unknown | - |", "| changed | zod | 3.22.0 → 3.23.0 | - | unknown | - |", "## Vulnerabilities", diff --git a/internal/cli/render/diff.go b/internal/cli/render/diff.go index 18410ba9..000a4f87 100644 --- a/internal/cli/render/diff.go +++ b/internal/cli/render/diff.go @@ -69,7 +69,7 @@ func dependencyTextSections(results output.DiffDependencyResults) []string { if len(results.Transitions) == 0 { return } - lines = append(lines, Style(fmt.Sprintf("Dependency detail changes (%d)", len(results.Transitions)), Bold)) + lines = append(lines, Style(fmt.Sprintf("Detail changes (%d)", len(results.Transitions)), Bold)) for _, transition := range results.Transitions { name := dependencyTransitionDisplayName(transition.After) line := fmt.Sprintf(" ↔ %s %s", name, dependencyTransitionDescription(transition)) @@ -88,23 +88,27 @@ func dependencyTextSections(results output.DiffDependencyResults) []string { func dependencyTransitionDescription(transition output.DiffDependencyTransition) string { parts := make([]string, 0, len(transition.ChangedFields)) + sourceChanged := dependencyDetailFieldChanged(transition, sdk.DependencyDetailSource) for _, field := range transition.ChangedFields { switch field { - case sdk.DependencyMetadataRelationship: + case sdk.DependencyDetailRelationship: parts = append(parts, fmt.Sprintf( "relationship: %s → %s", - valueOrDash(transition.Before.Relationship), - valueOrDash(transition.After.Relationship), + valueOrDash(string(transition.Before.Relationship)), + valueOrDash(string(transition.After.Relationship)), )) - case sdk.DependencyMetadataSource: + case sdk.DependencyDetailSource: parts = append(parts, fmt.Sprintf( "source: %s → %s", valueOrDash(string(transition.Before.Source)), valueOrDash(string(transition.After.Source)), )) - case sdk.DependencyMetadataRegistryEligibility: + case sdk.DependencyDetailRegistryEligibility: + if sourceChanged { + continue + } parts = append(parts, fmt.Sprintf( - "registry matching: %s → %s", + "vulnerability checks: %s → %s", registryEligibilityLabel(transition.Before.RegistryEligible), registryEligibilityLabel(transition.After.RegistryEligible), )) @@ -113,6 +117,15 @@ func dependencyTransitionDescription(transition output.DiffDependencyTransition) return strings.Join(parts, "; ") } +func dependencyDetailFieldChanged(transition output.DiffDependencyTransition, wanted sdk.DependencyDetailField) bool { + for _, field := range transition.ChangedFields { + if field == wanted { + return true + } + } + return false +} + func dependencyTransitionDisplayName(state output.DiffDependencyTransitionState) string { if name := strings.TrimSpace(state.Name); name != "" { return name @@ -122,9 +135,9 @@ func dependencyTransitionDisplayName(state output.DiffDependencyTransitionState) func registryEligibilityLabel(eligible bool) string { if eligible { - return "eligible" + return "covered" } - return "not eligible" + return "not covered" } // findingsSummaryLine produces a summary line plus a list of each introduced or diff --git a/internal/cli/render/diff_markdown.go b/internal/cli/render/diff_markdown.go index a50e9e31..736a7336 100644 --- a/internal/cli/render/diff_markdown.go +++ b/internal/cli/render/diff_markdown.go @@ -57,7 +57,7 @@ func diffOverviewMarkdown(payload output.DiffResponse) []string { status, fmt.Sprintf("+%d / ~%d / -%d", payload.Summary.AddedManifestCount, payload.Summary.ChangedManifestCount, payload.Summary.RemovedManifestCount), fmt.Sprintf( - "+%d / ~%d version / ~%d details / -%d", + "%d added / %d version changed / %d detail changes / %d removed", payload.Summary.AddedPackageCount, payload.Summary.ChangedPackageCount, payload.Summary.TransitionedPackageCount, @@ -86,8 +86,12 @@ func diffDependencyMarkdown(payload output.DiffResponse) []string { results := payload.Results.Dependencies lines := []string{ fmt.Sprintf( - "**Summary:** %d added, %d version changed, %d with detail changes, %d removed.", - len(results.Added), len(results.Changed), len(results.Transitions), len(results.Removed), + "**Summary:** %d added, %d version changed, %d %s, %d removed.", + len(results.Added), + len(results.Changed), + len(results.Transitions), + pluralWord(len(results.Transitions), "detail change", "detail changes"), + len(results.Removed), ), "", } diff --git a/internal/cli/render/diff_markdown_test.go b/internal/cli/render/diff_markdown_test.go index 6d6d9fa2..1fb919d9 100644 --- a/internal/cli/render/diff_markdown_test.go +++ b/internal/cli/render/diff_markdown_test.go @@ -38,7 +38,7 @@ func TestDiffOverviewMarkdownPersistedWarningsAreWarnings(t *testing.T) { } } -func TestDiffTextAndMarkdownRenderDependencyMetadataTransitions(t *testing.T) { +func TestDiffTextAndMarkdownRenderDependencyDetailTransitions(t *testing.T) { payload := output.DiffResponse{ Summary: output.DiffSummary{TransitionedPackageCount: 1}, Results: output.DiffResults{Dependencies: output.DiffDependencyResults{ @@ -57,10 +57,10 @@ func TestDiffTextAndMarkdownRenderDependencyMetadataTransitions(t *testing.T) { Source: sdk.DependencySourceGit, RegistryEligible: false, }, - ChangedFields: []sdk.DependencyMetadataField{ - sdk.DependencyMetadataRelationship, - sdk.DependencyMetadataSource, - sdk.DependencyMetadataRegistryEligibility, + ChangedFields: []sdk.DependencyDetailField{ + sdk.DependencyDetailRelationship, + sdk.DependencyDetailSource, + sdk.DependencyDetailRegistryEligibility, }, }}, }}, @@ -71,22 +71,29 @@ func TestDiffTextAndMarkdownRenderDependencyMetadataTransitions(t *testing.T) { t.Fatalf("Diff() error = %v", err) } for _, want := range []string{ - "Dependency detail changes (1)", + "Detail changes (1)", "relationship: direct → transitive", "source: registry → git", - "registry matching: eligible → not eligible", } { if !strings.Contains(text.String(), want) { t.Fatalf("text output missing %q:\n%s", want, text.String()) } } + if strings.Contains(text.String(), "vulnerability checks:") { + t.Fatalf("source changes must not repeat their implied coverage change:\n%s", text.String()) + } + coverageOnly := payload.Results.Dependencies.Transitions[0] + coverageOnly.ChangedFields = []sdk.DependencyDetailField{sdk.DependencyDetailRegistryEligibility} + if description := dependencyTransitionDescription(coverageOnly); description != "vulnerability checks: covered → not covered" { + t.Fatalf("independent eligibility wording = %q", description) + } var markdown bytes.Buffer if err := DiffMarkdown(&markdown, payload); err != nil { t.Fatalf("DiffMarkdown() error = %v", err) } for _, want := range []string{ - "1 with detail changes", + "1 detail change", "### Dependency Detail Changes", "relationship: direct → transitive", } { diff --git a/internal/cli/root_cmd_test.go b/internal/cli/root_cmd_test.go index 6290ed36..d1d4464c 100644 --- a/internal/cli/root_cmd_test.go +++ b/internal/cli/root_cmd_test.go @@ -692,7 +692,7 @@ func TestRoot_DiffCommand_JSONOutputWithMarkdownOutputFile(t *testing.T) { if err != nil { t.Fatalf("read summary file: %v", err) } - for _, want := range []string{"# Bomly Diff Summary", "Compared `" + baseSHA + "` to `" + headSHA + "`.", "**Summary:** 1 added, 1 version changed, 0 with detail changes, 0 removed."} { + for _, want := range []string{"# Bomly Diff Summary", "Compared `" + baseSHA + "` to `" + headSHA + "`.", "**Summary:** 1 added, 1 version changed, 0 detail changes, 0 removed."} { if !strings.Contains(string(summary), want) { t.Fatalf("expected summary to contain %q, got:\n%s", want, summary) } diff --git a/internal/mcp/compact_diff.go b/internal/mcp/compact_diff.go index f6de258e..3b494ae3 100644 --- a/internal/mcp/compact_diff.go +++ b/internal/mcp/compact_diff.go @@ -10,7 +10,7 @@ import ( ) // diffHint tells agents how to drill into the delta. -const diffHint = "Introduced findings are new on head; resolved close when this ref merges; persisted remain after merge. Dependency transitions describe relationship, source, or registry-matching changes without treating them as version changes. Use bomly_explain (on the head checkout) for full advisory detail of one package." +const diffHint = "Introduced findings are new on head; resolved close when this ref merges; persisted remain after merge. Detail changes describe relationship, source, or vulnerability-check coverage changes without treating them as version changes. Use bomly_explain (on the head checkout) for full advisory detail of one package." // SecurityDelta buckets advisory findings by what merging head changes. type SecurityDelta struct { @@ -21,34 +21,34 @@ type SecurityDelta struct { // CompactDiffSummary counts manifest/package/finding deltas. type CompactDiffSummary struct { - ManifestsAdded int `json:"manifests_added,omitempty"` - ManifestsChanged int `json:"manifests_changed,omitempty"` - ManifestsRemoved int `json:"manifests_removed,omitempty"` - PackagesAdded int `json:"packages_added,omitempty"` - PackagesChanged int `json:"packages_changed,omitempty"` - PackagesTransitioned int `json:"packages_transitioned,omitempty"` - PackagesRemoved int `json:"packages_removed,omitempty"` - Introduced int `json:"introduced,omitempty"` - Resolved int `json:"resolved,omitempty"` - Persisted int `json:"persisted,omitempty"` - EnrichRan bool `json:"enrich_ran"` - AuditRan bool `json:"audit_ran"` + ManifestsAdded int `json:"manifests_added,omitempty"` + ManifestsChanged int `json:"manifests_changed,omitempty"` + ManifestsRemoved int `json:"manifests_removed,omitempty"` + PackagesAdded int `json:"packages_added,omitempty"` + PackagesChanged int `json:"packages_changed,omitempty"` + PackagesWithDetailChanges int `json:"packages_with_detail_changes,omitempty"` + PackagesRemoved int `json:"packages_removed,omitempty"` + Introduced int `json:"introduced,omitempty"` + Resolved int `json:"resolved,omitempty"` + Persisted int `json:"persisted,omitempty"` + EnrichRan bool `json:"enrich_ran"` + AuditRan bool `json:"audit_ran"` } // CompactDependencyTransitionState is one side of a dependency occurrence -// metadata transition. +// detail change. type CompactDependencyTransitionState struct { - Relationship string `json:"relationship,omitempty"` - Source sdk.DependencySource `json:"source,omitempty"` - RegistryEligible bool `json:"registry_eligible"` + Relationship sdk.DependencyRelationship `json:"relationship,omitempty"` + Source sdk.DependencySource `json:"source,omitempty"` + RegistryEligible bool `json:"registry_eligible"` } -// CompactDependencyTransition reports one same-identity metadata change +// CompactDependencyTransition reports one same-identity detail change // without repeating licenses, vulnerabilities, locations, or other large // package detail. type CompactDependencyTransition struct { Package PackageIdentity `json:"package"` - ChangedFields []sdk.DependencyMetadataField `json:"changed_fields"` + ChangedFields []sdk.DependencyDetailField `json:"changed_fields"` Before CompactDependencyTransitionState `json:"before"` After CompactDependencyTransitionState `json:"after"` } @@ -62,7 +62,7 @@ type CompactDiffResponse struct { Comparison output.DiffComparison `json:"comparison"` Summary CompactDiffSummary `json:"summary"` SecurityDelta SecurityDelta `json:"security_delta"` - Transitions []CompactDependencyTransition `json:"dependency_transitions,omitempty"` + Transitions []CompactDependencyTransition `json:"transitions,omitempty"` Remediations []RemediationGroup `json:"remediations,omitempty"` Informational []CompactFinding `json:"informational,omitempty"` Diagnostics []Diagnostic `json:"diagnostics,omitempty"` @@ -81,18 +81,18 @@ func BuildCompactDiff(run DiffRunResult) CompactDiffResponse { Command: "diff", Comparison: run.Response.Comparison, Summary: CompactDiffSummary{ - ManifestsAdded: run.Response.Summary.AddedManifestCount, - ManifestsChanged: run.Response.Summary.ChangedManifestCount, - ManifestsRemoved: run.Response.Summary.RemovedManifestCount, - PackagesAdded: run.Response.Summary.AddedPackageCount, - PackagesChanged: run.Response.Summary.ChangedPackageCount, - PackagesTransitioned: run.Response.Summary.TransitionedPackageCount, - PackagesRemoved: run.Response.Summary.RemovedPackageCount, - Introduced: len(run.Introduced), - Resolved: len(run.Resolved), - Persisted: len(run.Persisted), - AuditRan: run.AuditRan, - EnrichRan: run.EnrichRan, + ManifestsAdded: run.Response.Summary.AddedManifestCount, + ManifestsChanged: run.Response.Summary.ChangedManifestCount, + ManifestsRemoved: run.Response.Summary.RemovedManifestCount, + PackagesAdded: run.Response.Summary.AddedPackageCount, + PackagesChanged: run.Response.Summary.ChangedPackageCount, + PackagesWithDetailChanges: run.Response.Summary.TransitionedPackageCount, + PackagesRemoved: run.Response.Summary.RemovedPackageCount, + Introduced: len(run.Introduced), + Resolved: len(run.Resolved), + Persisted: len(run.Persisted), + AuditRan: run.AuditRan, + EnrichRan: run.EnrichRan, }, Diagnostics: capDiagnostics(run.Diagnostics), Hint: diffHint, @@ -168,9 +168,19 @@ func compactDependencyTransitions(transitions []output.DiffDependencyTransition, if len(transitions) == 0 { return nil } - sorted := append([]output.DiffDependencyTransition(nil), transitions...) + type keyedTransition struct { + key string + transition output.DiffDependencyTransition + } + sorted := make([]keyedTransition, len(transitions)) + for index, transition := range transitions { + sorted[index] = keyedTransition{ + key: compactDependencyTransitionSortKey(transition), + transition: transition, + } + } sort.Slice(sorted, func(i, j int) bool { - return compactDependencyTransitionSortKey(sorted[i]) < compactDependencyTransitionSortKey(sorted[j]) + return sorted[i].key < sorted[j].key }) limit := len(sorted) if limit > maxDependencyTransitions { @@ -178,7 +188,8 @@ func compactDependencyTransitions(transitions []output.DiffDependencyTransition, limit = maxDependencyTransitions } out := make([]CompactDependencyTransition, 0, limit) - for _, transition := range sorted[:limit] { + for _, item := range sorted[:limit] { + transition := item.transition pkg := transition.After out = append(out, CompactDependencyTransition{ Package: PackageIdentity{ @@ -186,7 +197,7 @@ func compactDependencyTransitions(transitions []output.DiffDependencyTransition, Version: pkg.Version, Purl: pkg.Purl, }, - ChangedFields: append([]sdk.DependencyMetadataField(nil), transition.ChangedFields...), + ChangedFields: append([]sdk.DependencyDetailField(nil), transition.ChangedFields...), Before: CompactDependencyTransitionState{ Relationship: transition.Before.Relationship, Source: transition.Before.Source, @@ -211,8 +222,8 @@ func compactDependencyTransitionSortKey(transition output.DiffDependencyTransiti transition.After.Purl, transition.After.Name, transition.After.ID, - transition.Before.Relationship, - transition.After.Relationship, + string(transition.Before.Relationship), + string(transition.After.Relationship), string(transition.Before.Source), string(transition.After.Source), strconv.FormatBool(transition.Before.RegistryEligible), diff --git a/internal/mcp/compact_diff_test.go b/internal/mcp/compact_diff_test.go index bcddc31f..28d0feb4 100644 --- a/internal/mcp/compact_diff_test.go +++ b/internal/mcp/compact_diff_test.go @@ -1,7 +1,9 @@ package mcp import ( + "encoding/json" "fmt" + "strings" "testing" "github.com/bomly-dev/bomly-cli/internal/output" @@ -41,10 +43,10 @@ func TestBuildCompactDiffBucketsAndRemediation(t *testing.T) { Source: sdk.DependencySourceGit, RegistryEligible: false, }, - ChangedFields: []sdk.DependencyMetadataField{ - sdk.DependencyMetadataRelationship, - sdk.DependencyMetadataSource, - sdk.DependencyMetadataRegistryEligibility, + ChangedFields: []sdk.DependencyDetailField{ + sdk.DependencyDetailRelationship, + sdk.DependencyDetailSource, + sdk.DependencyDetailRegistryEligibility, }, }}, }}, @@ -100,13 +102,23 @@ func TestBuildCompactDiffBucketsAndRemediation(t *testing.T) { if compact.SchemaVersion != CompactSchemaVersion || compact.Command != "diff" { t.Fatalf("header wrong: %#v", compact) } - if compact.Summary.PackagesTransitioned != 1 || len(compact.Transitions) != 1 { - t.Fatalf("dependency transition missing: %#v", compact) + if compact.Summary.PackagesWithDetailChanges != 1 || len(compact.Transitions) != 1 { + t.Fatalf("dependency detail change missing: %#v", compact) + } + encoded, err := json.Marshal(compact) + if err != nil { + t.Fatalf("marshal compact diff: %v", err) + } + if !strings.Contains(string(encoded), `"packages_with_detail_changes":1`) || + !strings.Contains(string(encoded), `"transitions":[`) || + strings.Contains(string(encoded), `"packages_transitioned"`) || + strings.Contains(string(encoded), `"dependency_transitions"`) { + t.Fatalf("compact detail-change names are inconsistent: %s", encoded) } if compact.Transitions[0].Before.Relationship != "direct" || compact.Transitions[0].After.Source != sdk.DependencySourceGit || compact.Transitions[0].After.RegistryEligible { - t.Fatalf("dependency transition evidence wrong: %#v", compact.Transitions[0]) + t.Fatalf("dependency detail-change evidence wrong: %#v", compact.Transitions[0]) } } @@ -121,7 +133,7 @@ func TestBuildCompactDiffCapsDependencyTransitions(t *testing.T) { After: output.DiffDependencyTransitionState{ Name: name, Purl: "pkg:npm/" + name + "@1.0.0", Relationship: "transitive", }, - ChangedFields: []sdk.DependencyMetadataField{sdk.DependencyMetadataRelationship}, + ChangedFields: []sdk.DependencyDetailField{sdk.DependencyDetailRelationship}, }) } compact := BuildCompactDiff(DiffRunResult{Response: output.DiffResponse{ diff --git a/internal/output/view.go b/internal/output/view.go index 3ddc1f3c..d4ca6dcc 100644 --- a/internal/output/view.go +++ b/internal/output/view.go @@ -140,20 +140,20 @@ type DiffChangedPackage struct { type DiffDependencyTransition struct { Before DiffDependencyTransitionState `json:"before"` After DiffDependencyTransitionState `json:"after"` - ChangedFields []sdk.DependencyMetadataField `json:"changed_fields"` + ChangedFields []sdk.DependencyDetailField `json:"changed_fields"` } -// DiffDependencyTransitionState preserves the identity and metadata of one +// DiffDependencyTransitionState preserves the identity and details of one // occurrence before or after a transition. type DiffDependencyTransitionState struct { - ID string `json:"id"` - Name string `json:"name"` - Version string `json:"version,omitempty"` - Purl string `json:"purl,omitempty"` - Scope string `json:"scope,omitempty"` - Relationship string `json:"relationship"` - Source sdk.DependencySource `json:"source,omitempty"` - RegistryEligible bool `json:"registry_eligible"` + ID string `json:"id"` + Name string `json:"name"` + Version string `json:"version,omitempty"` + Purl string `json:"purl,omitempty"` + Scope string `json:"scope,omitempty"` + Relationship sdk.DependencyRelationship `json:"relationship"` + Source sdk.DependencySource `json:"source,omitempty"` + RegistryEligible bool `json:"registry_eligible"` } // DiffManifestResult describes changes for one manifest. @@ -743,8 +743,8 @@ func diffDependencyTransitionKey(transition DiffDependencyTransition) string { transition.After.ID, transition.Before.Scope, transition.After.Scope, - transition.Before.Relationship, - transition.After.Relationship, + string(transition.Before.Relationship), + string(transition.After.Relationship), string(transition.Before.Source), string(transition.After.Source), strconv.FormatBool(transition.Before.RegistryEligible), @@ -1029,7 +1029,7 @@ func filterSBOMPseudoPackageDiff(diff *sdk.Diff, baseGraph, headGraph *sdk.Graph if len(diff.Transitions) > 0 { baseRoots := graphRootIDs(baseGraph) headRoots := graphRootIDs(headGraph) - filtered := make([]sdk.DependencyMetadataTransition, 0, len(diff.Transitions)) + filtered := make([]sdk.DependencyDetailTransition, 0, len(diff.Transitions)) for _, transition := range diff.Transitions { if isSBOMPseudoPackage(transition.Before, baseRoots) || isSBOMPseudoPackage(transition.After, headRoots) { continue @@ -1109,13 +1109,13 @@ func diffChangedPackagesFromDiff(changes []sdk.VersionChange, baseRegistry, head return out } -func diffDependencyTransitionsFromDiff(transitions []sdk.DependencyMetadataTransition) []DiffDependencyTransition { +func diffDependencyTransitionsFromDiff(transitions []sdk.DependencyDetailTransition) []DiffDependencyTransition { out := make([]DiffDependencyTransition, 0, len(transitions)) for _, transition := range transitions { out = append(out, DiffDependencyTransition{ Before: diffDependencyTransitionState(transition.Before, transition.BeforeRelationship, transition.BeforeRegistryEligible), After: diffDependencyTransitionState(transition.After, transition.AfterRelationship, transition.AfterRegistryEligible), - ChangedFields: append([]sdk.DependencyMetadataField(nil), transition.ChangedFields...), + ChangedFields: append([]sdk.DependencyDetailField(nil), transition.ChangedFields...), }) } sort.Slice(out, func(i, j int) bool { @@ -1126,7 +1126,7 @@ func diffDependencyTransitionsFromDiff(transitions []sdk.DependencyMetadataTrans func diffDependencyTransitionState(dependency *sdk.Dependency, relationship sdk.DependencyRelationship, eligible bool) DiffDependencyTransitionState { if dependency == nil { - return DiffDependencyTransitionState{Relationship: string(relationship), RegistryEligible: eligible} + return DiffDependencyTransitionState{Relationship: relationship, RegistryEligible: eligible} } return DiffDependencyTransitionState{ ID: dependency.ID, @@ -1134,7 +1134,7 @@ func diffDependencyTransitionState(dependency *sdk.Dependency, relationship sdk. Version: dependency.Version, Purl: dependency.PURL, Scope: string(dependency.PrimaryScope()), - Relationship: string(relationship), + Relationship: relationship, Source: dependency.Source, RegistryEligible: eligible, } @@ -1253,7 +1253,7 @@ func reconcileDiffWithFuzzyMatches(diff *sdk.Diff, baseGraph, headGraph *sdk.Gra before := diff.Removed[match.removedIdx] applyFuzzyMetadata(before, after, match.score, match.tier) diff.Updated = append(diff.Updated, sdk.VersionChange{Before: before, After: after}) - if transition, changed := sdk.CompareDependencyMetadata(baseGraph, headGraph, before, after); changed { + if transition, changed := sdk.CompareDependencyDetails(baseGraph, headGraph, before, after); changed { diff.Transitions = append(diff.Transitions, transition) } matchedAdded[match.addedIdx] = struct{}{} @@ -1295,20 +1295,7 @@ func reconcileDiffWithFuzzyMatches(diff *sdk.Diff, baseGraph, headGraph *sdk.Gra } return left.Before.ID < right.Before.ID }) - sort.Slice(diff.Transitions, func(i, j int) bool { - left := diff.Transitions[i] - right := diff.Transitions[j] - if left.Before.IdentityKey() != right.Before.IdentityKey() { - return left.Before.IdentityKey() < right.Before.IdentityKey() - } - if left.Before.Version != right.Before.Version { - return left.Before.Version < right.Before.Version - } - if left.After.Version != right.After.Version { - return left.After.Version < right.After.Version - } - return left.Before.ID < right.Before.ID - }) + sdk.SortDependencyDetailTransitions(diff.Transitions) } func fuzzyReconcileScore(before, after *sdk.Dependency) (float64, string) { diff --git a/internal/output/view_test.go b/internal/output/view_test.go index 421c1c9a..ec244289 100644 --- a/internal/output/view_test.go +++ b/internal/output/view_test.go @@ -416,7 +416,7 @@ func TestBuildDiffResponseAggregatesManifestChanges(t *testing.T) { } } -func TestBuildDiffResponseReportsDependencyMetadataTransitions(t *testing.T) { +func TestBuildDiffResponseReportsDependencyDetailTransitions(t *testing.T) { baseGraph := sdk.New() headGraph := sdk.New() baseRoot := sdk.NewDependency(sdk.Dependency{Coordinates: sdk.Coordinates{ @@ -434,7 +434,7 @@ func TestBuildDiffResponseReportsDependencyMetadataTransitions(t *testing.T) { PackageRef: "pkg:npm/example@1.0.0", }) headDependency := baseDependency.Clone() - headDependency.Relationship = sdk.DependencyRelationshipUnknown + headDependency.Relationship = sdk.DependencyRelationshipTransitive headDependency.Source = sdk.DependencySourceGit for _, pair := range []struct { @@ -482,7 +482,7 @@ func TestBuildDiffResponseReportsDependencyMetadataTransitions(t *testing.T) { t.Fatalf("transition projection missing: %#v", response.Results) } transition := response.Results.Dependencies.Transitions[0] - if transition.Before.Relationship != "direct" || transition.After.Relationship != "unknown" { + if transition.Before.Relationship != "direct" || transition.After.Relationship != "transitive" { t.Fatalf("relationship transition = %#v", transition) } if transition.Before.Source != sdk.DependencySourceRegistry || transition.After.Source != sdk.DependencySourceGit { @@ -504,7 +504,7 @@ func TestBuildDiffResponseReportsDependencyMetadataTransitions(t *testing.T) { if response.SchemaVersion != "1.0" || !strings.Contains(string(encoded), `"transitioned_package_count":1`) || !strings.Contains(string(encoded), `"changed_fields":["relationship","source","registry_eligibility"]`) { - t.Fatalf("dependency transition JSON contract is incomplete: %s", encoded) + t.Fatalf("dependency detail-change JSON contract is incomplete: %s", encoded) } } @@ -1080,12 +1080,12 @@ func TestBuildDiffResponseFuzzyReconcilesRenamedPackage(t *testing.T) { t.Fatalf("expected one changed package in manifest, got %#v", response.Results.Manifests) } if response.Summary.TransitionedPackageCount != 1 || len(response.Results.Manifests[0].Transitions) != 1 { - t.Fatalf("expected fuzzy reconciliation to preserve the metadata transition, got %#v", response.Results) + t.Fatalf("expected fuzzy reconciliation to preserve the detail change, got %#v", response.Results) } transition := response.Results.Manifests[0].Transitions[0] if transition.Before.Relationship != "direct" || transition.After.Relationship != "transitive" || transition.Before.Source != sdk.DependencySourceRegistry || transition.After.Source != sdk.DependencySourceGit { - t.Fatalf("unexpected fuzzy metadata transition: %#v", transition) + t.Fatalf("unexpected fuzzy detail change: %#v", transition) } changed := response.Results.Manifests[0].Changed[0] if changed.After.Metadata == nil { diff --git a/internal/tui/diff.go b/internal/tui/diff.go index b29a56d5..1029d18d 100644 --- a/internal/tui/diff.go +++ b/internal/tui/diff.go @@ -355,7 +355,9 @@ func (m *DiffModel) diffFooterSummary() string { // Semantics: // // ManifestDeltas: Added + Changed + Removed manifests. -// PackageDeltas: Added + Changed + Removed packages across all manifests. +// PackageDeltas: Distinct added, changed, detail-changed, and removed +// package occurrences across all manifests. A version +// and detail change for the same occurrence counts once. // VulnDeltas: Introduced + Persisted + Resolved audit findings whose // Kind classifies them as a vulnerability (matches // isVulnerabilityFinding). Persisted is included because @@ -382,7 +384,7 @@ func (m *DiffModel) diffAggregateCounts() diffAggregateCounts { s := m.payload.Summary out := diffAggregateCounts{ ManifestDeltas: s.AddedManifestCount + s.ChangedManifestCount + s.RemovedManifestCount, - PackageDeltas: s.AddedPackageCount + s.ChangedPackageCount + s.TransitionedPackageCount + s.RemovedPackageCount, + PackageDeltas: diffPackageChangeCount(m.payload.Results.Manifests), } if m.payload.Audit != nil { for _, bucket := range [][]output.AuditFinding{m.payload.Audit.Introduced, m.payload.Audit.Persisted, m.payload.Audit.Resolved} { @@ -403,6 +405,33 @@ func (m *DiffModel) diffAggregateCounts() diffAggregateCounts { return out } +func diffPackageChangeCount(manifests []output.DiffManifestResult) int { + total := 0 + for _, manifest := range manifests { + total += manifestPackageChangeCount(manifest) + } + return total +} + +func manifestPackageChangeCount(manifest output.DiffManifestResult) int { + total := len(manifest.Added) + len(manifest.Removed) + len(manifest.Changed) + versionChanged := make(map[string]struct{}, len(manifest.Changed)) + for _, change := range manifest.Changed { + versionChanged[dependencyChangePairKey(change.Before.ID, change.After.ID)] = struct{}{} + } + for _, transition := range manifest.Transitions { + if _, combined := versionChanged[dependencyChangePairKey(transition.Before.ID, transition.After.ID)]; combined { + continue + } + total++ + } + return total +} + +func dependencyChangePairKey(beforeID, afterID string) string { + return beforeID + "\x00" + afterID +} + // diffLegend mirrors scan's legend so the keyboard help row reads identical // across commands. func (m *DiffModel) diffLegend() string { @@ -483,7 +512,7 @@ func (m *DiffModel) buildOverviewTab() *listModel { "", render.Style(" Added: ", render.Dim) + fmt.Sprintf("%d", summary.AddedPackageCount), render.Style(" Version changed: ", render.Dim) + fmt.Sprintf("%d", summary.ChangedPackageCount), - render.Style(" Details changed: ", render.Dim) + fmt.Sprintf("%d", summary.TransitionedPackageCount), + render.Style(" Detail changes: ", render.Dim) + fmt.Sprintf("%d", summary.TransitionedPackageCount), render.Style(" Removed: ", render.Dim) + fmt.Sprintf("%d", summary.RemovedPackageCount), }, }, @@ -587,7 +616,7 @@ type diffOverviewStats struct { unmatchedRemoved int // changedTotal: total package-level change events - // (Added+VersionChanged+MetadataTransitioned+Removed). + // (Added + version changed + detail changed + removed). changedTotal int // auditSummaryTotal: the raw AuditSummary.Total field. Per @@ -655,7 +684,7 @@ func (m *DiffModel) computeOverviewStats() diffOverviewStats { if eco == "" { eco = "unknown" } - n := len(mf.Added) + len(mf.Changed) + len(mf.Transitions) + len(mf.Removed) + n := manifestPackageChangeCount(mf) if n > 0 { out.ecosystems[eco] += n out.changedTotal += n @@ -674,7 +703,14 @@ func (m *DiffModel) computeOverviewStats() diffOverviewStats { out.matchedExact++ } } + versionChanged := make(map[string]struct{}, len(mf.Changed)) + for _, change := range mf.Changed { + versionChanged[dependencyChangePairKey(change.Before.ID, change.After.ID)] = struct{}{} + } for _, transition := range mf.Transitions { + if _, combined := versionChanged[dependencyChangePairKey(transition.Before.ID, transition.After.ID)]; combined { + continue + } addScopeRel("transitioned", transition.After.Scope, transition.After.ID) } out.unmatchedAdded += len(mf.Added) @@ -994,10 +1030,10 @@ func (m *DiffModel) overviewDashboardView(width, height int) string { render.Style(fmt.Sprintf("%d changed", s.ChangedManifestCount), render.Yellow), render.Style(fmt.Sprintf("%d removed", s.RemovedManifestCount), render.Red), ), cardWidth, cardHeight, render.Cyan), - boxView("Packages", summaryCountCardLines(s.AddedPackageCount+s.ChangedPackageCount+s.TransitionedPackageCount+s.RemovedPackageCount, "Package Changes", cardWidth-2, render.Magenta, + boxView("Packages", summaryCountCardLines(diffPackageChangeCount(m.payload.Results.Manifests), "Package Changes", cardWidth-2, render.Magenta, render.Style(fmt.Sprintf("%d added", s.AddedPackageCount), render.Green), render.Style(fmt.Sprintf("%d version changed", s.ChangedPackageCount), render.Yellow), - render.Style(fmt.Sprintf("%d details changed", s.TransitionedPackageCount), render.Cyan), + render.Style(fmt.Sprintf("%d detail changes", s.TransitionedPackageCount), render.Cyan), render.Style(fmt.Sprintf("%d removed", s.RemovedPackageCount), render.Red), ), cardWidth, cardHeight, render.Magenta), boxView("Vulnerabilities", summaryCountCardLines(diffVulnTotal(m.payload), "Vuln Deltas", cardWidth-2, render.Red, @@ -1228,7 +1264,7 @@ func (m *DiffModel) overviewPanels() []listPanel { {title: "Packages", lines: []string{ render.Style(fmt.Sprintf("%d Added", summary.AddedPackageCount), render.Green, render.Bold), render.Style(fmt.Sprintf("%d Version Changed", summary.ChangedPackageCount), render.Yellow, render.Bold), - render.Style(fmt.Sprintf("%d Details Changed", summary.TransitionedPackageCount), render.Cyan, render.Bold), + render.Style(fmt.Sprintf("%d Detail Changes", summary.TransitionedPackageCount), render.Cyan, render.Bold), render.Style(fmt.Sprintf("%d Removed", summary.RemovedPackageCount), render.Red, render.Bold), }, color: render.Magenta, weight: 1}, } @@ -1288,7 +1324,7 @@ func (m *DiffModel) overviewTopChangedManifests() []string { for _, mf := range m.payload.Results.Manifests { rows = append(rows, ranked{ name: render.DiffManifestDisplayLabel(mf), - count: len(mf.Added) + len(mf.Changed) + len(mf.Transitions) + len(mf.Removed), + count: manifestPackageChangeCount(mf), }) } sort.Slice(rows, func(i, j int) bool { return rows[i].count > rows[j].count }) @@ -1363,6 +1399,11 @@ func (m *DiffModel) collectComponentChanges() []flatComponentChange { mfKey := diffManifestKey(mf) mfName := render.DiffManifestDisplayLabel(mf) eco := valueOrDefault(string(mf.Ecosystem), "unknown") + transitionsByPair := make(map[string]int, len(mf.Transitions)) + for index, transition := range mf.Transitions { + transitionsByPair[dependencyChangePairKey(transition.Before.ID, transition.After.ID)] = index + } + usedTransitions := make(map[int]struct{}, len(mf.Transitions)) for _, change := range mf.Added { out = append(out, flatComponentChange{ manifest: mf, manifestKey: mfKey, manifestName: mfName, ecosystem: eco, @@ -1373,7 +1414,7 @@ func (m *DiffModel) collectComponentChanges() []flatComponentChange { }) } for _, change := range mf.Changed { - out = append(out, flatComponentChange{ + component := flatComponentChange{ manifest: mf, manifestKey: mfKey, manifestName: mfName, ecosystem: eco, status: "changed", pkgName: render.DiffPackageDisplayName(change.After), pkgRef: change.After, beforePkg: change.Before, @@ -1381,9 +1422,18 @@ func (m *DiffModel) collectComponentChanges() []flatComponentChange { maxSeverity: maxSeverity(change.After.Vulnerabilities), relationship: pickRel("changed", change.After.ID), remediation: remediationForPURL(m.headRegistry, change.After.Purl), - }) + } + if index, ok := transitionsByPair[dependencyChangePairKey(change.Before.ID, change.After.ID)]; ok { + transition := mf.Transitions[index] + component.transition = &transition + usedTransitions[index] = struct{}{} + } + out = append(out, component) } for idx := range mf.Transitions { + if _, used := usedTransitions[idx]; used { + continue + } transition := mf.Transitions[idx] after := packageRefFromTransitionState(transition.After) before := packageRefFromTransitionState(transition.Before) @@ -1392,7 +1442,7 @@ func (m *DiffModel) collectComponentChanges() []flatComponentChange { status: "transitioned", pkgName: render.DiffPackageDisplayName(after), pkgRef: after, beforePkg: before, beforeVer: transition.Before.Version, afterVer: transition.After.Version, - relationship: transition.After.Relationship, + relationship: string(transition.After.Relationship), remediation: remediationForPURL(m.headRegistry, transition.After.Purl), transition: &transition, }) @@ -1417,15 +1467,15 @@ func packageRefFromTransitionState(state output.DiffDependencyTransitionState) o Version: state.Version, Purl: state.Purl, Scope: state.Scope, - Relationship: state.Relationship, + Relationship: string(state.Relationship), Licenses: []output.LicenseRef{}, Vulnerabilities: []output.VulnerabilityRef{}, } switch state.Relationship { - case string(sdk.DependencyRelationshipDirect): + case sdk.DependencyRelationshipDirect: direct := true ref.Direct = &direct - case string(sdk.DependencyRelationshipTransitive): + case sdk.DependencyRelationshipTransitive: direct := false ref.Direct = &direct } @@ -1515,15 +1565,16 @@ func (m *DiffModel) buildComponentsTab() *listModel { } } - added, changed, transitioned, removed := 0, 0, 0, 0 + added, changed, detailsChanged, removed := 0, 0, 0, 0 for _, c := range filtered { + if c.transition != nil { + detailsChanged++ + } switch c.status { case "added": added++ case "changed": changed++ - case "transitioned": - transitioned++ case "removed": removed++ } @@ -1538,7 +1589,7 @@ func (m *DiffModel) buildComponentsTab() *listModel { render.Style(" | Ecosystem: ", render.Dim) + render.Style(valueOrDefault(m.componentsEcosystem, "All"), render.BgYellow, render.Bold) + render.Style(" | Added: ", render.Dim) + render.Style(fmt.Sprintf("%d", added), render.Green, render.Bold) + render.Style(" | Changed: ", render.Dim) + render.Style(fmt.Sprintf("%d", changed), render.Yellow, render.Bold) + - render.Style(" | Details changed: ", render.Dim) + render.Style(fmt.Sprintf("%d", transitioned), render.Cyan, render.Bold) + + render.Style(" | Detail changes: ", render.Dim) + render.Style(fmt.Sprintf("%d", detailsChanged), render.Cyan, render.Bold) + render.Style(" | Removed: ", render.Dim) + render.Style(fmt.Sprintf("%d", removed), render.Red, render.Bold), } @@ -1576,6 +1627,9 @@ func componentsGroupKey(c flatComponentChange, group componentsGroup) string { func componentsGroupLabel(key string, group componentsGroup) string { switch group { case componentsGroupStatus: + if key == "transitioned" { + return "Detail changes" + } return titleCase(key) default: return key @@ -1617,14 +1671,17 @@ func (m *DiffModel) componentsGroupDetails(key string, group componentsGroup, it render.Style(" Group axis: ", render.Dim) + componentsGroupName(group), render.Style(" Items: ", render.Dim) + fmt.Sprintf("%d", len(items)), } - counts := map[string]int{"added": 0, "changed": 0, "transitioned": 0, "removed": 0} + counts := map[string]int{"added": 0, "changed": 0, "details": 0, "removed": 0} for _, c := range items { counts[c.status]++ + if c.transition != nil { + counts["details"]++ + } } lines = append(lines, render.Style(" Added: ", render.Dim)+fmt.Sprintf("%d", counts["added"]), render.Style(" Changed: ", render.Dim)+fmt.Sprintf("%d", counts["changed"]), - render.Style(" Details changed: ", render.Dim)+fmt.Sprintf("%d", counts["transitioned"]), + render.Style(" Detail changes: ", render.Dim)+fmt.Sprintf("%d", counts["details"]), render.Style(" Removed: ", render.Dim)+fmt.Sprintf("%d", counts["removed"]), ) // When grouping by subproject, list the manifests the group spans so the @@ -1674,7 +1731,7 @@ func componentChangeRowTitle(c flatComponentChange) string { return fmt.Sprintf("%s (%s → %s)", c.pkgName, valueOrDash(c.beforeVer), valueOrDash(c.afterVer)) } if c.status == "transitioned" { - return c.pkgName + " (details)" + return c.pkgName + " (detail changes)" } return c.pkgName } @@ -1686,8 +1743,11 @@ func componentChangeDetailTitle(c flatComponentChange) string { case "removed": return "Removed package" case "transitioned": - return "Dependency details changed" + return "Dependency detail changes" default: + if c.transition != nil { + return "Version and dependency detail changes" + } return "Changed package" } } @@ -1775,10 +1835,13 @@ func componentChangeDetails(c flatComponentChange) []string { render.Style(" After: ", render.Dim)+valueOrDash(c.pkgRef.Scope), ) } + if c.transition != nil { + lines = append(lines, renderDependencyDetailTransition(*c.transition)...) + } lines = append(lines, renderLicenseDelta(c.beforePkg.Licenses, c.pkgRef.Licenses)...) lines = append(lines, renderVulnDelta(c.beforePkg.Vulnerabilities, c.pkgRef.Vulnerabilities)...) } else if c.status == "transitioned" && c.transition != nil { - lines = append(lines, renderDependencyMetadataTransition(*c.transition)...) + lines = append(lines, renderDependencyDetailTransition(*c.transition)...) } else { lines = append(lines, renderLicenseList(c.pkgRef.Licenses)...) lines = append(lines, renderVulnList(c.pkgRef.Vulnerabilities)...) @@ -1790,21 +1853,25 @@ func componentChangeDetails(c flatComponentChange) []string { return lines } -func renderDependencyMetadataTransition(transition output.DiffDependencyTransition) []string { - lines := []string{"", render.Style("Metadata changes", render.Bold, render.Cyan)} +func renderDependencyDetailTransition(transition output.DiffDependencyTransition) []string { + lines := []string{"", render.Style("Detail changes", render.Bold, render.Cyan)} + sourceChanged := dependencyDetailFieldChangedForTUI(transition, sdk.DependencyDetailSource) for _, field := range transition.ChangedFields { var label, before, after string switch field { - case sdk.DependencyMetadataRelationship: + case sdk.DependencyDetailRelationship: label = "Relationship" - before = valueOrDash(transition.Before.Relationship) - after = valueOrDash(transition.After.Relationship) - case sdk.DependencyMetadataSource: + before = valueOrDash(string(transition.Before.Relationship)) + after = valueOrDash(string(transition.After.Relationship)) + case sdk.DependencyDetailSource: label = "Source" before = valueOrDash(string(transition.Before.Source)) after = valueOrDash(string(transition.After.Source)) - case sdk.DependencyMetadataRegistryEligibility: - label = "Registry matching" + case sdk.DependencyDetailRegistryEligibility: + if sourceChanged { + continue + } + label = "Vulnerability checks" before = tuiRegistryEligibilityLabel(transition.Before.RegistryEligible) after = tuiRegistryEligibilityLabel(transition.After.RegistryEligible) default: @@ -1817,11 +1884,20 @@ func renderDependencyMetadataTransition(transition output.DiffDependencyTransiti return lines } +func dependencyDetailFieldChangedForTUI(transition output.DiffDependencyTransition, wanted sdk.DependencyDetailField) bool { + for _, field := range transition.ChangedFields { + if field == wanted { + return true + } + } + return false +} + func tuiRegistryEligibilityLabel(eligible bool) string { if eligible { - return "eligible" + return "covered" } - return "not eligible" + return "not covered" } // renderLicenseList renders one package's license inventory. diff --git a/internal/tui/diff_aggregations_test.go b/internal/tui/diff_aggregations_test.go index dd70d489..14dc6f77 100644 --- a/internal/tui/diff_aggregations_test.go +++ b/internal/tui/diff_aggregations_test.go @@ -119,7 +119,7 @@ func TestDiffAggregateCounts_EmptyPayload(t *testing.T) { } } -func TestDiffComponentsProjectDependencyMetadataTransition(t *testing.T) { +func TestDiffComponentsProjectDependencyDetailTransition(t *testing.T) { transition := output.DiffDependencyTransition{ Before: output.DiffDependencyTransitionState{ ID: "example@1.0.0", @@ -137,10 +137,10 @@ func TestDiffComponentsProjectDependencyMetadataTransition(t *testing.T) { Source: sdk.DependencySourceGit, RegistryEligible: false, }, - ChangedFields: []sdk.DependencyMetadataField{ - sdk.DependencyMetadataRelationship, - sdk.DependencyMetadataSource, - sdk.DependencyMetadataRegistryEligibility, + ChangedFields: []sdk.DependencyDetailField{ + sdk.DependencyDetailRelationship, + sdk.DependencyDetailSource, + sdk.DependencyDetailRegistryEligibility, }, } payload := output.DiffResponse{ @@ -159,13 +159,12 @@ func TestDiffComponentsProjectDependencyMetadataTransition(t *testing.T) { } details := strings.Join(componentChangeDetails(changes[0]), "\n") for _, want := range []string{ - "Dependency details changed", + "Dependency detail changes", + "Detail changes", "Relationship:", "direct → transitive", "Source:", "registry → git", - "Registry matching:", - "eligible → not eligible", } { if !strings.Contains(render.StripANSI(details), want) { t.Fatalf("transition details missing %q:\n%s", want, render.StripANSI(details)) @@ -176,6 +175,42 @@ func TestDiffComponentsProjectDependencyMetadataTransition(t *testing.T) { } } +func TestDiffComponentsFoldVersionAndDetailChangesIntoOneRow(t *testing.T) { + transition := output.DiffDependencyTransition{ + Before: output.DiffDependencyTransitionState{ID: "example@1.0.0", Name: "example", Version: "1.0.0", Relationship: "direct"}, + After: output.DiffDependencyTransitionState{ID: "example@2.0.0", Name: "example", Version: "2.0.0", Relationship: "transitive"}, + ChangedFields: []sdk.DependencyDetailField{ + sdk.DependencyDetailRelationship, + }, + } + payload := output.DiffResponse{ + Summary: output.DiffSummary{ChangedPackageCount: 1, TransitionedPackageCount: 1}, + Results: output.DiffResults{Manifests: []output.DiffManifestResult{{ + Status: "changed", + Path: "package-lock.json", + Changed: []output.DiffChangedPackage{{ + Before: output.PackageRef{ID: "example@1.0.0", Name: "example", Version: "1.0.0"}, + After: output.PackageRef{ID: "example@2.0.0", Name: "example", Version: "2.0.0"}, + }}, + Transitions: []output.DiffDependencyTransition{transition}, + }}}, + } + model := NewDiff(payload, sdk.ConsolidatedGraph{}, sdk.ConsolidatedGraph{}) + changes := model.collectComponentChanges() + if len(changes) != 1 || changes[0].status != "changed" || changes[0].transition == nil { + t.Fatalf("combined component change was not folded into one row: %#v", changes) + } + if counts := model.diffAggregateCounts(); counts.PackageDeltas != 1 { + t.Fatalf("combined PackageDeltas = %d, want 1", counts.PackageDeltas) + } + details := render.StripANSI(strings.Join(componentChangeDetails(changes[0]), "\n")) + for _, want := range []string{"Version and dependency detail changes", "1.0.0", "2.0.0", "direct → transitive"} { + if !strings.Contains(details, want) { + t.Fatalf("combined details missing %q:\n%s", want, details) + } + } +} + func TestDiffAggregateCounts_LicenseDedup(t *testing.T) { // Five packages all introduce MIT — unique count should be 1, not 5. payload := output.DiffResponse{Results: output.DiffResults{Manifests: []output.DiffManifestResult{{ @@ -1114,6 +1149,9 @@ func TestStatusBadge_NewOldFixed(t *testing.T) { t.Errorf("statusBadge(%q) = %q, want it to contain %q", status, got, want) } } + if got := render.StripANSI(statusBadge("transitioned")); !strings.Contains(got, "DETAILS") { + t.Fatalf("detail-change badge = %q, want a clear DETAILS label", got) + } } // TestDistributionLine_PreservesLongCompositeLabels guards against the diff --git a/internal/tui/utils.go b/internal/tui/utils.go index 43edc94f..11eaaf09 100644 --- a/internal/tui/utils.go +++ b/internal/tui/utils.go @@ -159,7 +159,7 @@ func statusBadge(status string) string { case "changed": return terminalSafeBadge(label, render.BgYellow, render.Black) case "transitioned": - return terminalSafeBadge(label, render.BgNeutral, render.White) + return terminalSafeBadge(" DETAILS ", render.BgYellow, render.Black) case "unchanged": return terminalSafeBadge(label, render.BgNeutral, render.White) case "new": // audit-delta "introduced" (display-side label) @@ -262,7 +262,7 @@ func statusText(status string) string { case "changed": return render.Style(status, render.Yellow, render.Bold) case "transitioned": - return render.Style(status, render.Cyan, render.Bold) + return render.Style("Detail changes", render.Yellow, render.Bold) case "unchanged": return render.Style(status, render.Cyan, render.Bold) case "new": diff --git a/sdk/graph.go b/sdk/graph.go index 86414f61..282224d5 100644 --- a/sdk/graph.go +++ b/sdk/graph.go @@ -29,7 +29,7 @@ type Diff struct { Added []*Dependency Removed []*Dependency Updated []VersionChange - Transitions []DependencyMetadataTransition + Transitions []DependencyDetailTransition } // VersionChange captures a dependency identity that changed versions. @@ -38,28 +38,28 @@ type VersionChange struct { After *Dependency } -// DependencyMetadataField identifies one occurrence property that changed +// DependencyDetailField identifies one occurrence property that changed // independently of package identity or version. -type DependencyMetadataField string +type DependencyDetailField string const ( - // DependencyMetadataRelationship is a direct, transitive, or unknown + // DependencyDetailRelationship is a direct, transitive, or unknown // relationship change. - DependencyMetadataRelationship DependencyMetadataField = "relationship" - // DependencyMetadataSource is a registry, workspace, file, Git, URL, or + DependencyDetailRelationship DependencyDetailField = "relationship" + // DependencyDetailSource is a registry, workspace, file, Git, URL, or // project source change. - DependencyMetadataSource DependencyMetadataField = "source" - // DependencyMetadataRegistryEligibility indicates that external registry + DependencyDetailSource DependencyDetailField = "source" + // DependencyDetailRegistryEligibility indicates that external registry // matching eligibility changed. - DependencyMetadataRegistryEligibility DependencyMetadataField = "registry_eligibility" + DependencyDetailRegistryEligibility DependencyDetailField = "registry_eligibility" ) -// DependencyMetadataTransition captures same-identity occurrence metadata -// changes. Version changes remain represented separately by VersionChange. -type DependencyMetadataTransition struct { +// DependencyDetailTransition captures same-identity occurrence detail changes. +// Version changes remain represented separately by VersionChange. +type DependencyDetailTransition struct { Before *Dependency After *Dependency - ChangedFields []DependencyMetadataField + ChangedFields []DependencyDetailField BeforeRelationship DependencyRelationship AfterRelationship DependencyRelationship BeforeRegistryEligible bool @@ -385,18 +385,20 @@ func (g *Graph) PrettyTree() string { return strings.TrimSuffix(b.String(), "\n") } -// Compare returns added, removed, version-changed, and metadata-transitioned +// Compare returns added, removed, version-changed, and detail-changed // dependencies between base and head. Synthetic consolidated subproject nodes // are ignored. func Compare(base, head *Graph) Diff { baseExact, headExact := indexDiffableNodes(base), indexDiffableNodes(head) + baseRelationships := dependencyRelationshipsForGraph(base) + headRelationships := dependencyRelationshipsForGraph(head) baseRemainder := make(map[string]*Dependency) headRemainder := make(map[string]*Dependency) - transitions := make([]DependencyMetadataTransition, 0) + transitions := make([]DependencyDetailTransition, 0) for id, node := range baseExact { if headNode, ok := headExact[id]; ok { - if transition, changed := CompareDependencyMetadata(base, head, node, headNode); changed { + if transition, changed := compareDependencyDetails(node, headNode, baseRelationships, headRelationships); changed { transitions = append(transitions, transition) } continue @@ -440,7 +442,7 @@ func Compare(base, head *Graph) Diff { before := baseNodes[i] after := headNodes[i] diff.Updated = append(diff.Updated, VersionChange{Before: before, After: after}) - if transition, changed := CompareDependencyMetadata(base, head, before, after); changed { + if transition, changed := compareDependencyDetails(before, after, baseRelationships, headRelationships); changed { diff.Transitions = append(diff.Transitions, transition) } } @@ -468,36 +470,48 @@ func Compare(base, head *Graph) Diff { } return left.Before.ID < right.Before.ID }) - sortDependencyMetadataTransitions(diff.Transitions) + SortDependencyDetailTransitions(diff.Transitions) return diff } -// CompareDependencyMetadata returns a transition when relationship, source, or +// CompareDependencyDetails returns a transition when relationship, source, or // registry-matching eligibility differs between two occurrences. It is // exported so trusted fuzzy identity reconciliation can use the same canonical // classifier as Compare. -func CompareDependencyMetadata(baseGraph, headGraph *Graph, before, after *Dependency) (DependencyMetadataTransition, bool) { +func CompareDependencyDetails(baseGraph, headGraph *Graph, before, after *Dependency) (DependencyDetailTransition, bool) { + return compareDependencyDetails( + before, + after, + dependencyRelationshipsForGraph(baseGraph), + dependencyRelationshipsForGraph(headGraph), + ) +} + +func compareDependencyDetails(before, after *Dependency, beforeRelationships, afterRelationships map[string]DependencyRelationship) (DependencyDetailTransition, bool) { if before == nil || after == nil { - return DependencyMetadataTransition{}, false + return DependencyDetailTransition{}, false } - beforeRelationship := dependencyRelationshipForDiff(baseGraph, before) - afterRelationship := dependencyRelationshipForDiff(headGraph, after) + beforeRelationship := dependencyRelationshipFromMap(beforeRelationships, before) + afterRelationship := dependencyRelationshipFromMap(afterRelationships, after) beforeEligible := before.RegistryMatchEligible() afterEligible := after.RegistryMatchEligible() - changedFields := make([]DependencyMetadataField, 0, 3) - if beforeRelationship != afterRelationship { - changedFields = append(changedFields, DependencyMetadataRelationship) + changedFields := make([]DependencyDetailField, 0, 3) + if beforeRelationship != DependencyRelationshipUnknown && + afterRelationship != DependencyRelationshipUnknown && + beforeRelationship != afterRelationship { + changedFields = append(changedFields, DependencyDetailRelationship) } - if before.Source != after.Source { - changedFields = append(changedFields, DependencyMetadataSource) + if before.Source != "" && after.Source != "" && before.Source != after.Source { + changedFields = append(changedFields, DependencyDetailSource) } - if beforeEligible != afterEligible { - changedFields = append(changedFields, DependencyMetadataRegistryEligibility) + eligibilityEvidenceComparable := before.Source == after.Source || (before.Source != "" && after.Source != "") + if eligibilityEvidenceComparable && beforeEligible != afterEligible { + changedFields = append(changedFields, DependencyDetailRegistryEligibility) } if len(changedFields) == 0 { - return DependencyMetadataTransition{}, false + return DependencyDetailTransition{}, false } - return DependencyMetadataTransition{ + return DependencyDetailTransition{ Before: before, After: after, ChangedFields: changedFields, @@ -508,41 +522,58 @@ func CompareDependencyMetadata(baseGraph, headGraph *Graph, before, after *Depen }, true } -func dependencyRelationshipForDiff(graph *Graph, node *Dependency) DependencyRelationship { +func dependencyRelationshipFromMap(relationships map[string]DependencyRelationship, node *Dependency) DependencyRelationship { if node == nil { return DependencyRelationshipUnknown } - if node.Relationship != "" { - return node.Relationship + if relationship := relationships[node.ID]; relationship != "" { + return relationship } + return relationshipForDepth(node, 0) +} + +func dependencyRelationshipsForGraph(graph *Graph) map[string]DependencyRelationship { + relationships := make(map[string]DependencyRelationship) if graph == nil || graph.Size() == 0 { - return DependencyRelationshipUnknown + return relationships } roots := graph.Roots() - if len(roots) == 0 || len(roots) == graph.Size() { - return DependencyRelationshipUnknown + hasUsableEdges := len(roots) > 0 && len(roots) != graph.Size() + direct := make(map[int]struct{}) + if hasUsableEdges { + for _, root := range roots { + rootIndex, ok := graph.indexByID[root.ID] + if !ok { + continue + } + for childIndex := range graph.outgoing[rootIndex] { + direct[childIndex] = struct{}{} + } + } } - for _, root := range roots { - if root == nil || root.ID == node.ID { + + for index, node := range graph.nodes { + if node == nil || !graph.alive[index] { continue } - children, err := graph.DirectDependencies(root.ID) - if err != nil { + if node.Relationship != "" { + relationships[node.ID] = relationshipForDepth(node, 0) continue } - for _, child := range children { - if child != nil && child.ID == node.ID { - return DependencyRelationshipDirect + depth := 0 + if hasUsableEdges && len(graph.incoming[index]) > 0 { + depth = 2 + if _, ok := direct[index]; ok { + depth = 1 } } + relationships[node.ID] = relationshipForDepth(node, depth) } - if parents, err := graph.Dependents(node.ID); err == nil && len(parents) > 0 { - return DependencyRelationshipTransitive - } - return DependencyRelationshipUnknown + return relationships } -func sortDependencyMetadataTransitions(transitions []DependencyMetadataTransition) { +// SortDependencyDetailTransitions orders detail changes deterministically. +func SortDependencyDetailTransitions(transitions []DependencyDetailTransition) { sort.Slice(transitions, func(i, j int) bool { left := transitions[i] right := transitions[j] diff --git a/sdk/graph_test.go b/sdk/graph_test.go index 6de94e87..4d6d27fb 100644 --- a/sdk/graph_test.go +++ b/sdk/graph_test.go @@ -467,11 +467,11 @@ func TestCompare_ClassifiesAddedRemovedAndUpdated(t *testing.T) { t.Fatalf("unexpected updated node: %#v", diff.Updated[0]) } if len(diff.Transitions) != 0 { - t.Fatalf("unexpected metadata transitions: %#v", diff.Transitions) + t.Fatalf("unexpected detail changes: %#v", diff.Transitions) } } -func TestCompare_ClassifiesDependencyMetadataTransitions(t *testing.T) { +func TestCompare_ClassifiesDependencyDetailTransitions(t *testing.T) { base := New() head := New() baseRoot := NewDependency(Dependency{Coordinates: Coordinates{Type: PackageTypeApplication, Name: "app", FirstParty: true}}) @@ -506,16 +506,15 @@ func TestCompare_ClassifiesDependencyMetadataTransitions(t *testing.T) { diff := Compare(base, head) if len(diff.Added) != 0 || len(diff.Removed) != 0 || len(diff.Updated) != 0 { - t.Fatalf("metadata-only diff changed package identity/version buckets: %#v", diff) + t.Fatalf("detail-only diff changed package identity/version buckets: %#v", diff) } if len(diff.Transitions) != 1 { t.Fatalf("Transitions = %#v, want one", diff.Transitions) } transition := diff.Transitions[0] - wantFields := []DependencyMetadataField{ - DependencyMetadataRelationship, - DependencyMetadataSource, - DependencyMetadataRegistryEligibility, + wantFields := []DependencyDetailField{ + DependencyDetailSource, + DependencyDetailRegistryEligibility, } if !slices.Equal(transition.ChangedFields, wantFields) { t.Fatalf("ChangedFields = %#v, want %#v", transition.ChangedFields, wantFields) @@ -528,7 +527,7 @@ func TestCompare_ClassifiesDependencyMetadataTransitions(t *testing.T) { } } -func TestCompareDependencyMetadataClassifiesEachAxisIndependently(t *testing.T) { +func TestCompareDependencyDetailsClassifiesEachAxisIndependently(t *testing.T) { base := NewDependency(Dependency{ Coordinates: Coordinates{ Ecosystem: EcosystemNPM, @@ -542,7 +541,7 @@ func TestCompareDependencyMetadataClassifiesEachAxisIndependently(t *testing.T) tests := []struct { name string after func() *Dependency - want DependencyMetadataField + want DependencyDetailField }{ { name: "relationship only", @@ -551,16 +550,16 @@ func TestCompareDependencyMetadataClassifiesEachAxisIndependently(t *testing.T) after.Relationship = DependencyRelationshipTransitive return after }, - want: DependencyMetadataRelationship, + want: DependencyDetailRelationship, }, { - name: "source only", + name: "known source only", after: func() *Dependency { after := base.Clone() - after.Source = "" + after.Source = DependencySource("custom-registry") return after }, - want: DependencyMetadataSource, + want: DependencyDetailSource, }, { name: "registry eligibility only", @@ -569,14 +568,14 @@ func TestCompareDependencyMetadataClassifiesEachAxisIndependently(t *testing.T) after.FirstParty = true return after }, - want: DependencyMetadataRegistryEligibility, + want: DependencyDetailRegistryEligibility, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - transition, changed := CompareDependencyMetadata(nil, nil, base, tt.after()) + transition, changed := CompareDependencyDetails(nil, nil, base, tt.after()) if !changed { - t.Fatal("CompareDependencyMetadata() did not report a transition") + t.Fatal("CompareDependencyDetails() did not report a transition") } if len(transition.ChangedFields) != 1 || transition.ChangedFields[0] != tt.want { t.Fatalf("ChangedFields = %#v, want [%s]", transition.ChangedFields, tt.want) @@ -585,7 +584,21 @@ func TestCompareDependencyMetadataClassifiesEachAxisIndependently(t *testing.T) } } -func TestCompareSortsDependencyMetadataTransitions(t *testing.T) { +func TestCompareDependencyDetailsIgnoresMissingEvidence(t *testing.T) { + before := NewDependency(Dependency{ + Coordinates: Coordinates{Ecosystem: EcosystemNPM, Name: "example", Version: "1.0.0"}, + Relationship: DependencyRelationshipUnknown, + }) + after := before.Clone() + after.Relationship = DependencyRelationshipDirect + after.Source = DependencySourceRegistry + + if transition, changed := CompareDependencyDetails(nil, nil, before, after); changed { + t.Fatalf("missing relationship/source evidence must not create a detail change: %#v", transition) + } +} + +func TestCompareSortsDependencyDetailTransitions(t *testing.T) { base := New() head := New() for _, name := range []string{"zeta", "alpha"} { @@ -647,7 +660,7 @@ func TestCompare_DerivesRelationshipTransitionFromGraphEdges(t *testing.T) { t.Fatalf("Transitions = %#v, want one", diff.Transitions) } transition := diff.Transitions[0] - if !slices.Equal(transition.ChangedFields, []DependencyMetadataField{DependencyMetadataRelationship}) { + if !slices.Equal(transition.ChangedFields, []DependencyDetailField{DependencyDetailRelationship}) { t.Fatalf("ChangedFields = %#v", transition.ChangedFields) } if transition.BeforeRelationship != DependencyRelationshipDirect || transition.AfterRelationship != DependencyRelationshipTransitive { @@ -655,7 +668,7 @@ func TestCompare_DerivesRelationshipTransitionFromGraphEdges(t *testing.T) { } } -func TestCompare_ReportsVersionAndMetadataChangesSeparately(t *testing.T) { +func TestCompare_ReportsVersionAndDetailChangesSeparately(t *testing.T) { base := New() head := New() before := NewDependency(Dependency{ @@ -677,9 +690,9 @@ func TestCompare_ReportsVersionAndMetadataChangesSeparately(t *testing.T) { diff := Compare(base, head) if len(diff.Updated) != 1 || len(diff.Transitions) != 1 { - t.Fatalf("Compare() = %#v, want one version change and one metadata transition", diff) + t.Fatalf("Compare() = %#v, want one version change and one detail change", diff) } - if !slices.Equal(diff.Transitions[0].ChangedFields, []DependencyMetadataField{DependencyMetadataRelationship}) { + if !slices.Equal(diff.Transitions[0].ChangedFields, []DependencyDetailField{DependencyDetailRelationship}) { t.Fatalf("ChangedFields = %#v", diff.Transitions[0].ChangedFields) } } diff --git a/sdk/relationship.go b/sdk/relationship.go index 3d07baf2..4c4e3e4f 100644 --- a/sdk/relationship.go +++ b/sdk/relationship.go @@ -36,13 +36,28 @@ func RelationshipForPath(path []*Dependency) DependencyRelationship { return "" } target := path[len(path)-1] + depth := len(path) - 1 + if depth == 0 { + // Preserve the historical interpretation of a one-node path as a + // direct target. Graph-wide classification uses unknown for a root + // occurrence because no owning edge is available there. + depth = 1 + } + return relationshipForDepth(target, depth) +} + +func relationshipForDepth(target *Dependency, depth int) DependencyRelationship { if target != nil && target.Relationship != "" { return target.Relationship } - if len(path) <= 2 { + switch { + case depth == 1: return DependencyRelationshipDirect + case depth > 1: + return DependencyRelationshipTransitive + default: + return DependencyRelationshipUnknown } - return DependencyRelationshipTransitive } // MergeDependencyRelationship combines occurrence relationships for a merged diff --git a/test/smoke/smoke_test.go b/test/smoke/smoke_test.go index 75194e68..9be689cf 100644 --- a/test/smoke/smoke_test.go +++ b/test/smoke/smoke_test.go @@ -12,6 +12,7 @@ package smoke import ( + "encoding/json" "fmt" "os" "os/exec" @@ -366,9 +367,10 @@ func TestScanRecursiveDepthLimitFindsNothing(t *testing.T) { func TestDiff(t *testing.T) { cases := []struct { - name string - args []string - tools []string + name string + args []string + tools []string + requireDetailChange bool }{ { name: "diff-go", @@ -384,6 +386,11 @@ func TestDiff(t *testing.T) { name: "diff-sbom", args: []string{"diff", "--sbom", "--base", sbomFixture("go.spdx.json"), "--head", sbomFixture("js.spdx.json"), "--format", "json"}, }, + { + name: "diff-sbom-detail-change", + args: []string{"diff", "--sbom", "--base", sbomFixture("detail-change-base.spdx.json"), "--head", sbomFixture("detail-change-head.spdx.json"), "--format", "json"}, + requireDetailChange: true, + }, } for _, tc := range cases { @@ -400,6 +407,34 @@ func TestDiff(t *testing.T) { if len(stdout) == 0 { t.Fatal("bomly produced no stdout output") } + if tc.requireDetailChange { + var response struct { + Summary struct { + TransitionedPackageCount int `json:"transitioned_package_count"` + } `json:"summary"` + Results struct { + Dependencies struct { + Transitions []json.RawMessage `json:"transitions"` + } `json:"dependencies"` + } `json:"results"` + } + if err := json.Unmarshal([]byte(stdout), &response); err != nil { + t.Fatalf("decode detail-change diff: %v", err) + } + if response.Summary.TransitionedPackageCount == 0 || len(response.Results.Dependencies.Transitions) == 0 { + t.Fatalf("expected an end-to-end dependency detail change, got summary=%+v", response.Summary) + } + textArgs := append([]string(nil), tc.args...) + textArgs[len(textArgs)-1] = "text" + textStdout, textStderr, textCode := runBomly(t, textArgs...) + if textCode != 0 { + t.Fatalf("bomly text diff exited %d\nstderr:\n%s", textCode, textStderr) + } + if !strings.Contains(textStdout, "Detail changes (1)") || + !strings.Contains(textStdout, "relationship: direct → transitive") { + t.Fatalf("expected text output to explain the dependency detail change:\n%s", textStdout) + } + } got := normalizeJSON(t, []byte(stdout)) assertGolden(t, tc.name, got) diff --git a/test/smoke/testdata/golden/container-diff-alpine.golden.json b/test/smoke/testdata/golden/container-diff-alpine.golden.json index 954a72db..b2eb3c7d 100644 --- a/test/smoke/testdata/golden/container-diff-alpine.golden.json +++ b/test/smoke/testdata/golden/container-diff-alpine.golden.json @@ -2364,6 +2364,7 @@ "removed_manifest_count": 0, "removed_package_count": 1, "unchanged_manifest_count": 0, + "transitioned_package_count": 0, "unmatched_package_count": 1 } } diff --git a/test/smoke/testdata/golden/diff-go-audit.golden.json b/test/smoke/testdata/golden/diff-go-audit.golden.json index 0c61b8dc..3a3f3e95 100644 --- a/test/smoke/testdata/golden/diff-go-audit.golden.json +++ b/test/smoke/testdata/golden/diff-go-audit.golden.json @@ -1005,6 +1005,7 @@ "fuzzy_match_count": 0, "removed_manifest_count": 0, "removed_package_count": 18, + "transitioned_package_count": 0, "unchanged_manifest_count": 0, "unmatched_package_count": 21 } diff --git a/test/smoke/testdata/golden/diff-go.golden.json b/test/smoke/testdata/golden/diff-go.golden.json index 8015be95..139ba00e 100644 --- a/test/smoke/testdata/golden/diff-go.golden.json +++ b/test/smoke/testdata/golden/diff-go.golden.json @@ -995,6 +995,7 @@ "fuzzy_match_count": 0, "removed_manifest_count": 0, "removed_package_count": 18, + "transitioned_package_count": 0, "unchanged_manifest_count": 0, "unmatched_package_count": 21 } diff --git a/test/smoke/testdata/golden/diff-npm.golden.json b/test/smoke/testdata/golden/diff-npm.golden.json index 2f68848b..6c2982ec 100644 --- a/test/smoke/testdata/golden/diff-npm.golden.json +++ b/test/smoke/testdata/golden/diff-npm.golden.json @@ -1238,6 +1238,7 @@ "fuzzy_match_count": 0, "removed_manifest_count": 0, "removed_package_count": 0, + "transitioned_package_count": 0, "unchanged_manifest_count": 0, "unmatched_package_count": 0 } diff --git a/test/smoke/testdata/golden/diff-sbom-detail-change.golden.json b/test/smoke/testdata/golden/diff-sbom-detail-change.golden.json new file mode 100644 index 00000000..65e9b708 --- /dev/null +++ b/test/smoke/testdata/golden/diff-sbom-detail-change.golden.json @@ -0,0 +1,147 @@ +{ + "command": "diff", + "comparison": { + "base": "detail-change-base.spdx.json", + "head": "detail-change-head.spdx.json" + }, + "metadata": { + "duration_ms": 0 + }, + "packages": [ + { + "ecosystem": "go", + "licenses": [], + "name": "example.com/app", + "purl": "pkg:golang/example.com/app@1.0.0", + "version": "1.0.0", + "vulnerabilities": [] + }, + { + "ecosystem": "go", + "licenses": [], + "name": "example.com/parent", + "purl": "pkg:golang/example.com/parent@1.0.0", + "version": "1.0.0", + "vulnerabilities": [] + }, + { + "ecosystem": "go", + "licenses": [], + "name": "example.com/shared", + "purl": "pkg:golang/example.com/shared@1.0.0", + "version": "1.0.0", + "vulnerabilities": [] + } + ], + "project": { + "ecosystem": "other", + "name": "\u003cnormalized\u003e", + "package_manager": "multiple", + "path": "\u003cnormalized\u003e", + "target_type": "dependency diff" + }, + "results": { + "dependencies": { + "added": [ + { + "package": { + "direct": true, + "id": "pkg:golang/example.com/parent@1.0.0", + "licenses": [], + "name": "example.com/parent", + "purl": "pkg:golang/example.com/parent@1.0.0", + "relationship": "direct", + "version": "1.0.0", + "vulnerabilities": [] + } + } + ], + "transitions": [ + { + "after": { + "id": "pkg:golang/example.com/shared@1.0.0", + "name": "example.com/shared", + "purl": "pkg:golang/example.com/shared@1.0.0", + "registry_eligible": true, + "relationship": "transitive", + "version": "1.0.0" + }, + "before": { + "id": "pkg:golang/example.com/shared@1.0.0", + "name": "example.com/shared", + "purl": "pkg:golang/example.com/shared@1.0.0", + "registry_eligible": true, + "relationship": "direct", + "version": "1.0.0" + }, + "changed_fields": [ + "relationship" + ] + } + ] + }, + "licenses": {}, + "manifests": [ + { + "added": [ + { + "package": { + "direct": true, + "id": "pkg:golang/example.com/parent@1.0.0", + "licenses": [], + "name": "example.com/parent", + "purl": "pkg:golang/example.com/parent@1.0.0", + "relationship": "direct", + "version": "1.0.0", + "vulnerabilities": [] + } + } + ], + "ecosystem": "sbom", + "kind": "detail-change-head.spdx.json", + "package_manager": "sbom", + "path": "detail-change-head.spdx.json", + "status": "changed", + "subproject": ".", + "transitions": [ + { + "after": { + "id": "pkg:golang/example.com/shared@1.0.0", + "name": "example.com/shared", + "purl": "pkg:golang/example.com/shared@1.0.0", + "registry_eligible": true, + "relationship": "transitive", + "version": "1.0.0" + }, + "before": { + "id": "pkg:golang/example.com/shared@1.0.0", + "name": "example.com/shared", + "purl": "pkg:golang/example.com/shared@1.0.0", + "registry_eligible": true, + "relationship": "direct", + "version": "1.0.0" + }, + "changed_fields": [ + "relationship" + ] + } + ] + } + ], + "vulnerabilities": {} + }, + "schema_version": "1.0", + "summary": { + "added_manifest_count": 0, + "added_package_count": 1, + "changed_manifest_count": 1, + "changed_package_count": 0, + "exact_match_count": 0, + "fuzzy_match_count": 0, + "removed_manifest_count": 0, + "removed_package_count": 0, + "transitioned_package_count": 1, + "unchanged_manifest_count": 0, + "unmatched_package_count": 1 + } +} diff --git a/test/smoke/testdata/golden/diff-sbom.golden.json b/test/smoke/testdata/golden/diff-sbom.golden.json index e695b69e..bdab3dea 100644 --- a/test/smoke/testdata/golden/diff-sbom.golden.json +++ b/test/smoke/testdata/golden/diff-sbom.golden.json @@ -242,6 +242,7 @@ "fuzzy_match_count": 0, "removed_manifest_count": 0, "removed_package_count": 3, + "transitioned_package_count": 0, "unchanged_manifest_count": 0, "unmatched_package_count": 6 } diff --git a/test/smoke/testdata/golden/lite-diff-go.golden.json b/test/smoke/testdata/golden/lite-diff-go.golden.json index 8015be95..139ba00e 100644 --- a/test/smoke/testdata/golden/lite-diff-go.golden.json +++ b/test/smoke/testdata/golden/lite-diff-go.golden.json @@ -995,6 +995,7 @@ "fuzzy_match_count": 0, "removed_manifest_count": 0, "removed_package_count": 18, + "transitioned_package_count": 0, "unchanged_manifest_count": 0, "unmatched_package_count": 21 } diff --git a/test/smoke/testdata/sboms/detail-change-base.spdx.json b/test/smoke/testdata/sboms/detail-change-base.spdx.json new file mode 100644 index 00000000..1afb8b3e --- /dev/null +++ b/test/smoke/testdata/sboms/detail-change-base.spdx.json @@ -0,0 +1,55 @@ +{ + "spdxVersion": "SPDX-2.3", + "dataLicense": "CC0-1.0", + "SPDXID": "SPDXRef-DOCUMENT", + "name": "detail-change-base", + "documentNamespace": "https://example.com/spdx/detail-change-base", + "creationInfo": { + "created": "2026-07-28T00:00:00Z", + "creators": [ + "Tool: bomly-cli-test" + ] + }, + "packages": [ + { + "SPDXID": "SPDXRef-app", + "name": "example.com/app", + "versionInfo": "1.0.0", + "downloadLocation": "NOASSERTION", + "filesAnalyzed": false, + "externalRefs": [ + { + "referenceCategory": "PACKAGE-MANAGER", + "referenceType": "purl", + "referenceLocator": "pkg:golang/example.com/app@1.0.0" + } + ] + }, + { + "SPDXID": "SPDXRef-shared", + "name": "example.com/shared", + "versionInfo": "1.0.0", + "downloadLocation": "NOASSERTION", + "filesAnalyzed": false, + "externalRefs": [ + { + "referenceCategory": "PACKAGE-MANAGER", + "referenceType": "purl", + "referenceLocator": "pkg:golang/example.com/shared@1.0.0" + } + ] + } + ], + "relationships": [ + { + "spdxElementId": "SPDXRef-DOCUMENT", + "relatedSpdxElement": "SPDXRef-app", + "relationshipType": "DESCRIBES" + }, + { + "spdxElementId": "SPDXRef-app", + "relatedSpdxElement": "SPDXRef-shared", + "relationshipType": "DEPENDS_ON" + } + ] +} diff --git a/test/smoke/testdata/sboms/detail-change-head.spdx.json b/test/smoke/testdata/sboms/detail-change-head.spdx.json new file mode 100644 index 00000000..addfbb92 --- /dev/null +++ b/test/smoke/testdata/sboms/detail-change-head.spdx.json @@ -0,0 +1,74 @@ +{ + "spdxVersion": "SPDX-2.3", + "dataLicense": "CC0-1.0", + "SPDXID": "SPDXRef-DOCUMENT", + "name": "detail-change-head", + "documentNamespace": "https://example.com/spdx/detail-change-head", + "creationInfo": { + "created": "2026-07-28T00:00:00Z", + "creators": [ + "Tool: bomly-cli-test" + ] + }, + "packages": [ + { + "SPDXID": "SPDXRef-app", + "name": "example.com/app", + "versionInfo": "1.0.0", + "downloadLocation": "NOASSERTION", + "filesAnalyzed": false, + "externalRefs": [ + { + "referenceCategory": "PACKAGE-MANAGER", + "referenceType": "purl", + "referenceLocator": "pkg:golang/example.com/app@1.0.0" + } + ] + }, + { + "SPDXID": "SPDXRef-parent", + "name": "example.com/parent", + "versionInfo": "1.0.0", + "downloadLocation": "NOASSERTION", + "filesAnalyzed": false, + "externalRefs": [ + { + "referenceCategory": "PACKAGE-MANAGER", + "referenceType": "purl", + "referenceLocator": "pkg:golang/example.com/parent@1.0.0" + } + ] + }, + { + "SPDXID": "SPDXRef-shared", + "name": "example.com/shared", + "versionInfo": "1.0.0", + "downloadLocation": "NOASSERTION", + "filesAnalyzed": false, + "externalRefs": [ + { + "referenceCategory": "PACKAGE-MANAGER", + "referenceType": "purl", + "referenceLocator": "pkg:golang/example.com/shared@1.0.0" + } + ] + } + ], + "relationships": [ + { + "spdxElementId": "SPDXRef-DOCUMENT", + "relatedSpdxElement": "SPDXRef-app", + "relationshipType": "DESCRIBES" + }, + { + "spdxElementId": "SPDXRef-app", + "relatedSpdxElement": "SPDXRef-parent", + "relationshipType": "DEPENDS_ON" + }, + { + "spdxElementId": "SPDXRef-parent", + "relatedSpdxElement": "SPDXRef-shared", + "relationshipType": "DEPENDS_ON" + } + ] +} From ce6ddbd4473b885a89ea4ca9d52f0bdd7f99787c Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Tue, 28 Jul 2026 01:54:40 -0700 Subject: [PATCH 4/4] Make SBOM smoke slice ownership explicit Anchor both SBOM diff cases in the nightly and golden-update workflow matrices so future regex tightening cannot silently drop the dependency detail-change fixture. --- .github/workflows/smoke.yml | 2 +- .github/workflows/update-smoke-goldens.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 51e80d5f..362880eb 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -94,7 +94,7 @@ jobs: - name: ruby run: 'TestScan$/scan-bundler' - name: sbom - run: 'TestScan$/scan-sbom|TestDiff/diff-sbom|TestLiteScan/lite-scan-sbom' + run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom' - name: dotnet run: 'TestScan$/scan-nuget' dotnet: true diff --git a/.github/workflows/update-smoke-goldens.yml b/.github/workflows/update-smoke-goldens.yml index 149fa4f6..7de93d20 100644 --- a/.github/workflows/update-smoke-goldens.yml +++ b/.github/workflows/update-smoke-goldens.yml @@ -118,7 +118,7 @@ jobs: - name: cpp run: 'TestScan$/scan-cpp-conan' - name: sbom - run: 'TestScan$/scan-sbom|TestDiff/diff-sbom|TestLiteScan/lite-scan-sbom' + run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom' - name: plugin run: 'TestPluginWorkflows' - name: container