From f9bd9283c9cb2bf53db9917601b6394d0a0ad289 Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Sat, 12 Sep 2026 11:01:26 -0700 Subject: [PATCH 1/2] docs(adr): ADR-0033 names the SDK helper and what the fold does merge The insertion-helper sentence in ADR-0033 was stale twice over: it named `detectors.EnsureNode`, which was deleted when bomly-sdk v0.9.0 shipped, and it said the helper "deliberately merges nothing", which stopped being true when the SDK fold started unioning scopes, locations, relationship and origins. The same paragraph already said usage facts union, so it contradicted itself. Annotated in place with a dated amendment rather than superseded: the decision the ADR records -- origins are detector-asserted, contradicting records stay distinct, no tiebreak picks a winner -- still holds. Only the helper sentence drifted. The cargo origin test carried the same stale spelling in a comment. Closes #454. Co-Authored-By: Claude Fable 5.1 --- dev-docs/adr/0033-package-origin-is-detector-asserted.md | 2 ++ internal/detectors/cargo/origin_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/dev-docs/adr/0033-package-origin-is-detector-asserted.md b/dev-docs/adr/0033-package-origin-is-detector-asserted.md index f199a534..6b3a36ff 100644 --- a/dev-docs/adr/0033-package-origin-is-detector-asserted.md +++ b/dev-docs/adr/0033-package-origin-is-detector-asserted.md @@ -17,4 +17,6 @@ The invariant runs when a detector records a value, again at the JSON boundary i Origins are never merged, reconciled, or disputed. Two records of one package are either witnesses of one resolution or distinct occurrences, and the rule is uniform: **identical records fold, a gap fills from whichever record has an origin, and contradicting records stay distinct nodes — no tiebreak ever picks a winner over a contradiction.** One-node-per-PURL is the registry's constraint, not the graph's: many graph nodes may share a PURL, all linked to one registry package through `PackageRef`. Where a lockfile gives records positional identity (cargo's source-qualified package IDs, bun's keys), the detector keeps distinct nodes and `normalizeGraphPackageIdentity` preserves them through the canonical-PURL rewrite instead of collapsing them; across manifests, `preserveContradictingOccurrences` re-IDs a contradicting record before the SDK graph merge, so the merge folds only witnesses and fills only gaps. Where a lockfile references by bare name (uv, poetry, pipenv groups), one graph position exists and the deterministic first/last record wins as a whole — no field-level mixing; scope, relationship, and locations still union everywhere, being usage facts rather than assertions. Node insertion itself goes through one helper, `detectors.EnsureNode`, which deliberately merges nothing — records that differ deserve distinct nodes under distinct IDs, and records that are the same need nothing merged. `TestNodeInsertionGoesThroughTheSharedHelper` fails if a hand-written lookup-then-insert reappears; that guard found four sites nobody had reported when it was introduced. Generalizing positional identity beyond origin contradictions is #399. +> **Amended 2026-09-12 (issue #454):** the helper is `detectorkit.EnsureNode` in bomly-sdk; the CLI-local `detectors.EnsureNode` was deleted when bomly-sdk v0.9.0 shipped. Since bomly-sdk v0.10.0 it is a thin wrapper over `Graph.InsertNode`, which folds two records of one identity and unions their usage facts — scope, relationship, locations, origins — so "merges nothing" is no longer literally true. What the fold still never does is reconcile contradicting *assertions*: a gap fills, identical claims fold, and a contradiction stays two nodes with no tiebreak, which is the rule this decision actually rests on. `TestNodeInsertionGoesThroughTheSharedHelper` now names the SDK helper. + One consequence worth stating: origin does not appear in `scan`/`diff`/`explain` payloads, because those documents are built from explicit projections rather than from the SDK types. It is provenance for the SBOM, which is where users read it. (While origin rode on metadata, keeping it out took an explicit prefix filter in `output.cloneRefMetadata`; the typed field made that unnecessary.) diff --git a/internal/detectors/cargo/origin_test.go b/internal/detectors/cargo/origin_test.go index 9f1ae061..85129a76 100644 --- a/internal/detectors/cargo/origin_test.go +++ b/internal/detectors/cargo/origin_test.go @@ -71,7 +71,7 @@ func TestSetCargoOriginBySourcePrefix(t *testing.T) { // "pkg:cargo/helper@1.0.0" and keeping them apart would produce two components // with byte-identical identity. Nothing is lost -- the folded node carries both // repositories as origins, which says more than two indistinguishable nodes -// did (ADR-0041; the reasoning is recorded in detectors.EnsureNode). +// did (ADR-0041; the reasoning is recorded in the SDK's `Graph.InsertNode`, which `detectorkit.EnsureNode` wraps). func TestCargoDuplicateCrateSourcesFoldWithBothOrigins(t *testing.T) { metadata := []byte(`{ "packages": [ From cf2ffcd99b606dd53a17e3111bc75b649965fc8a Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Sat, 12 Sep 2026 22:51:15 -0700 Subject: [PATCH 2/2] docs(adr): the ADR-0033 amendment states the one-node, two-origins fold ADR-0041 decided Codex review on #456: the amendment said a contradiction stays two nodes, which is the occurrence model ADR-0041 superseded. Two records of one identity that assert different origins are one node carrying both origins; what this ADR still owns is that no tiebreak ever picks one. Co-Authored-By: Claude Fable 5.1 --- dev-docs/adr/0033-package-origin-is-detector-asserted.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-docs/adr/0033-package-origin-is-detector-asserted.md b/dev-docs/adr/0033-package-origin-is-detector-asserted.md index 6b3a36ff..ef77e340 100644 --- a/dev-docs/adr/0033-package-origin-is-detector-asserted.md +++ b/dev-docs/adr/0033-package-origin-is-detector-asserted.md @@ -17,6 +17,6 @@ The invariant runs when a detector records a value, again at the JSON boundary i Origins are never merged, reconciled, or disputed. Two records of one package are either witnesses of one resolution or distinct occurrences, and the rule is uniform: **identical records fold, a gap fills from whichever record has an origin, and contradicting records stay distinct nodes — no tiebreak ever picks a winner over a contradiction.** One-node-per-PURL is the registry's constraint, not the graph's: many graph nodes may share a PURL, all linked to one registry package through `PackageRef`. Where a lockfile gives records positional identity (cargo's source-qualified package IDs, bun's keys), the detector keeps distinct nodes and `normalizeGraphPackageIdentity` preserves them through the canonical-PURL rewrite instead of collapsing them; across manifests, `preserveContradictingOccurrences` re-IDs a contradicting record before the SDK graph merge, so the merge folds only witnesses and fills only gaps. Where a lockfile references by bare name (uv, poetry, pipenv groups), one graph position exists and the deterministic first/last record wins as a whole — no field-level mixing; scope, relationship, and locations still union everywhere, being usage facts rather than assertions. Node insertion itself goes through one helper, `detectors.EnsureNode`, which deliberately merges nothing — records that differ deserve distinct nodes under distinct IDs, and records that are the same need nothing merged. `TestNodeInsertionGoesThroughTheSharedHelper` fails if a hand-written lookup-then-insert reappears; that guard found four sites nobody had reported when it was introduced. Generalizing positional identity beyond origin contradictions is #399. -> **Amended 2026-09-12 (issue #454):** the helper is `detectorkit.EnsureNode` in bomly-sdk; the CLI-local `detectors.EnsureNode` was deleted when bomly-sdk v0.9.0 shipped. Since bomly-sdk v0.10.0 it is a thin wrapper over `Graph.InsertNode`, which folds two records of one identity and unions their usage facts — scope, relationship, locations, origins — so "merges nothing" is no longer literally true. What the fold still never does is reconcile contradicting *assertions*: a gap fills, identical claims fold, and a contradiction stays two nodes with no tiebreak, which is the rule this decision actually rests on. `TestNodeInsertionGoesThroughTheSharedHelper` now names the SDK helper. +> **Amended 2026-09-12 (issue #454):** the helper is `detectorkit.EnsureNode` in bomly-sdk; the CLI-local `detectors.EnsureNode` was deleted when bomly-sdk v0.9.0 shipped. Since bomly-sdk v0.10.0 it is a thin wrapper over `Graph.InsertNode`, which folds two records of one identity into one node and unions their usage facts — scope, relationship, locations — and their origins, so "merges nothing" is no longer literally true. That fold also supersedes the sentence above about contradicting records staying distinct nodes: under [ADR-0041](0041-identity-is-the-canonical-purl-on-typed-nodes.md) identity is the canonical package URL, so two records of one package that assert different origins are one node whose origins list has two elements (`TestConsolidateGraphsFoldsContradictingResolutionsKeepingBoth`), a fact to display rather than a reason to split identity. What survives unchanged, and is the rule this decision actually rests on, is that no tiebreak ever picks a winner: both origins are kept, neither is reconciled into the other, and export projects what the detectors asserted. `TestNodeInsertionGoesThroughTheSharedHelper` now names the SDK helper. One consequence worth stating: origin does not appear in `scan`/`diff`/`explain` payloads, because those documents are built from explicit projections rather than from the SDK types. It is provenance for the SBOM, which is where users read it. (While origin rode on metadata, keeping it out took an explicit prefix filter in `output.cloneRefMetadata`; the typed field made that unnecessary.)