fix: aspect provenance collisions and the nested _ alias (#670)

nix/lib/aspects/fx/aspect/children.nix

@@ -55,7 +55,7 @@ let
   registerPolicy =
     p:
     fx.send "register-aspect-policy" {
-      inherit (p) name fn;
+      inherit (p) fn;
       ownerIdentity = identity.key p;
     };
 

nix/lib/aspects/fx/aspect/provide.nix

@@ -14,7 +14,7 @@ let
   inherit (den.lib.schemaUtil) schemaEntityKinds schemaEntityKindsSet;
 
   mkCrossPolicy =
-    aspectName: nodeIdentity: provides: key:
+    nodeIdentity: provides: key:
     let
       value = provides.${key};
       policyFn =
@@ -52,9 +52,9 @@ let
           );
     in
     fx.send "register-aspect-policy" {
-      name = "${aspectName}/${key}";
       fn = policyFn;
       ownerIdentity = nodeIdentity;
+      label = key;
     };
 
   # Extract the inner function and args from a provider value.
@@ -148,8 +148,13 @@ let
 
       provides = aspect.provides or { };
       crossKeys = builtins.filter (k: k != aspectName) (builtins.attrNames provides);
-      compatKeys = builtins.filter (k: !(schemaEntityKindsSet ? ${k})) crossKeys;
-      allRegistrations = map (mkCrossPolicy aspectName nodeIdentity provides) compatKeys;
+      # Every merged aspect carries a synthetic `__functor` on `provides`; it is
+      # pipeline machinery, not a delivery target. Registering it produced one
+      # inert policy per aspect that re-dispatched on every iteration.
+      compatKeys = builtins.filter (
+        k: !(schemaEntityKindsSet ? ${k}) && !(lib.hasPrefix "__" k)
+      ) crossKeys;
+      allRegistrations = map (mkCrossPolicy nodeIdentity provides) compatKeys;
 
       hasSelfProvide = provides ? ${aspectName};
       selfProvide = mkSelfProvideInclude aspect aspectName;

nix/lib/aspects/fx/handlers/policy.nix

@@ -8,14 +8,21 @@ let
     "register-aspect-policy" =
       { param, state }:
       let
-        entry = {
-          inherit (param) fn ownerIdentity;
-        };
+        entry = { inherit (param) fn; };
+        # The registry is the one scoped field that merges rather than appends,
+        # so a repeated key is a silent drop. Derive it here from the owner's
+        # identity instead of letting each caller name its own: a caller that
+        # reaches for a local name registers `tools/to-users` for every aspect
+        # owning a sub-aspect called `tools`, and all but the last vanish.
+        # `label` separates several registrations by one owner (one per
+        # `provides` key); an owner registering once needs none.
+        label = param.label or null;
+        registryKey = if label == null then param.ownerIdentity else "${param.ownerIdentity}/${label}";
       in
       {
         resume = null;
         state = scopedMerge state "scopedAspectPolicies" state.currentScope {
-          ${param.name} = entry;
+          ${registryKey} = entry;
         };
       };
   };

nix/lib/aspects/types.nix

@@ -553,7 +553,29 @@ let
           # matching mergeWithAspectMeta behavior for root aspects — including
           # the rule that a name the wrapper defines itself keeps its own value
           # and stays classified.
-          providesChildren = builtins.removeAttrs (merged.provides or { }) [ "_module" ];
+          # `_` is the write alias for `provides`. aspectSubmodule wires it with
+          # mkAliasOptionModule, but a nested key never reaches that submodule:
+          # `_` is structural, so an alias write arrives here as a plain key and
+          # is then discarded by the `_` this wrapper publishes below. Fold it
+          # into the provides source so both spellings mean the same thing at
+          # every depth. A `_` read back off another wrapper carries __functor;
+          # that is the read shorthand, not a write, and stays out of provides.
+          writtenUnderscore =
+            let
+              v = merged._ or null;
+            in
+            lib.optionalAttrs (builtins.isAttrs v && !(v ? __functor)) v;
+          # Both spellings arrive as a content wrapper when the key is defined in
+          # more than one file, carrying `__contentValues` / `__provider` / `_`
+          # alongside the real children. Those are wrapper machinery, not
+          # provides children: unfiltered they surface as `provides` keys, enter
+          # `__providesForwarded`, and `_` (not `__`-prefixed) registers an inert
+          # cross-provide policy. Filtering here covers `provides` and `_` at
+          # once, and the single-def path is unaffected because a raw attrset
+          # carries none of these keys.
+          providesChildren = lib.filterAttrs (k: _: !(structuralKeysSet ? ${k}) && !(lib.hasPrefix "__" k)) (
+            (merged.provides or { }) // writtenUnderscore
+          );
           unshadowedProvides = builtins.filter (k: !(merged ? ${k})) (builtins.attrNames providesChildren);
           provider = (typeCfg.providerPrefix or [ ]) ++ [ keyName ];
           # A key names a candidate child aspect when it is neither structural,
@@ -604,7 +626,12 @@ let
           __contentValues = flatDefs;
           __provider = provider;
           __providesForwarded = unshadowedProvides;
-          _ = underscoreAt provider annotatedMerged;
+          # Root aspects publish `provides` and `_` as one value — provides-
+          # children plus the all-children functor (mergeWithAspectMeta's
+          # syntheticProvides). Match that here so the two spellings are
+          # interchangeable for reading as well as writing, at any depth.
+          provides = providesChildren // underscoreAt provider annotatedMerged;
+          _ = providesChildren // underscoreAt provider annotatedMerged;
         }
         // lib.optionalAttrs singleFn {
           __functor = _self: (builtins.head flatDefs).value;
@@ -661,7 +688,30 @@ let
         internal = true;
         visible = false;
         description = "Provider path tracking aspect provenance";
-        type = lib.types.listOf lib.types.str;
+        # NOT listOf: that type accumulates, and a node has exactly one
+        # position. Two files defining one aspect path each inject the same
+        # chain (providerType.merge, wrapperToAspect), and concatenating them
+        # yields ["a" "a"] — a chain every descendant then inherits. Agreeing
+        # definitions collapse; genuinely different ones are an ambiguity den
+        # cannot resolve, so it says so rather than picking one.
+        type = lib.types.mkOptionType {
+          name = "aspectChain";
+          description = "aspect provenance chain";
+          check = v: builtins.isList v && builtins.all builtins.isString v;
+          merge =
+            loc: defs:
+            let
+              distinct = lib.unique (map (d: d.value) defs);
+            in
+            if distinct == [ ] then
+              [ ]
+            else if builtins.length distinct == 1 then
+              builtins.head distinct
+            else
+              throw "den: conflicting provenance for ${locName loc}: ${
+                lib.concatMapStringsSep " vs " (c: "[${lib.concatStringsSep " " c}]") distinct
+              }";
+        };
         default = typeCfg.providerPrefix or [ ];
       };
       options.collisionPolicy = lib.mkOption {
@@ -681,12 +731,21 @@ let
     typeCfg:
     lib.types.submodule (
       { name, config, ... }:
+      let
+        # The chain this aspect's children hang off. `meta.provider` defaults to
+        # `typeCfg.providerPrefix`, but providerType.merge overrides it when it
+        # re-types an included nested aspect (wrapperToAspect injects the chain
+        # from __provider). Reading the static typeCfg there truncates the chain
+        # to the aspect's own name, so `alpha/tools` and `beta/tools` both hand
+        # their children the prefix ["tools"] and the children collide.
+        childProviderPrefix = config.meta.provider ++ [ config.name ];
+      in
       {
         freeformType = lib.types.lazyAttrsOf (
           aspectKeyType (
             typeCfg
             // {
-              providerPrefix = (typeCfg.providerPrefix or [ ]) ++ [ config.name ];
+              providerPrefix = childProviderPrefix;
             }
           )
         );
@@ -733,7 +792,7 @@ let
                 providerType (
                   typeCfg
                   // {
-                    providerPrefix = (typeCfg.providerPrefix or [ ]) ++ [ config.name ];
+                    providerPrefix = childProviderPrefix;
                   }
                 )
               );

templates/ci/modules/features/deadbugs/aspect-chain-doubling.nix

@@ -0,0 +1,40 @@
+# `meta.provider` is a node's position, not an accumulating set. Two files
+# defining one aspect path each inject the same chain, and a `listOf` type
+# concatenated them into ["a" "a"] — which every descendant then inherited as
+# its own prefix, corrupting the whole subtree's identities.
+{ denTest, ... }:
+{
+  flake.tests.deadbugs.aspect-chain-doubling = {
+
+    test-agreeing-definitions-collapse = denTest (
+      { den, ... }:
+      {
+        imports = [
+          { den.aspects.igloo.provides.shared = den.aspects.a.tools; }
+          { den.aspects.igloo.provides.shared = den.aspects.a.tools; }
+        ];
+
+        den.hosts.x86_64-linux.igloo.users.tux = { };
+        den.aspects.a.tools.nixos.environment.etc."t".text = "y";
+
+        expr = den.aspects.igloo.provides.shared.meta.provider or [ ];
+        expected = [ "a" ];
+      }
+    );
+
+    # CONTROL: a single definition was never affected, so a passing multi-def
+    # case above is only meaningful next to this.
+    test-control-single-definition = denTest (
+      { den, ... }:
+      {
+        den.hosts.x86_64-linux.igloo.users.tux = { };
+        den.aspects.a.tools.nixos.environment.etc."t".text = "y";
+        den.aspects.igloo.provides.shared = den.aspects.a.tools;
+
+        expr = den.aspects.igloo.provides.shared.meta.provider or [ ];
+        expected = [ "a" ];
+      }
+    );
+
+  };
+}

templates/ci/modules/features/deadbugs/issue-670-same-named-sub-aspects.nix

@@ -0,0 +1,115 @@
+# Issue #670: two unrelated aspects each declaring a same-named sub-aspect that
+# delivers via `provides.to-users` — only one of the two lands, the other is
+# silently dropped.
+#
+# Two independent collisions on one cause: an identity string built from a chain
+# that had lost its head. `aspectSubmodule` handed its children a provider prefix
+# read from the static `typeCfg`, but `providerType.merge` rewrites that chain on
+# the value when it re-types an included nested aspect, so `alpha/tools` and
+# `beta/tools` both gave their children the prefix ["tools"]. The two delivered
+# aspects then shared one identity and gate dedup dropped one of them; the two
+# cross-provide policies also shared one `scopedAspectPolicies` key and last-win
+# dropped the other.
+{ denTest, ... }:
+{
+  flake.tests.deadbugs.issue-670-same-named-sub-aspects = {
+
+    test-same-named-sub-aspects = denTest (
+      { den, tuxHm, ... }:
+      {
+        den.hosts.x86_64-linux.igloo.users.tux = { };
+
+        den.aspects.igloo.includes = [
+          den.aspects.alpha
+          den.aspects.beta
+        ];
+
+        den.aspects.alpha = {
+          includes = [ den.aspects.alpha.tools ];
+          tools.provides.to-users.homeManager.home.sessionVariables.ALPHA = "yes";
+        };
+
+        den.aspects.beta = {
+          includes = [ den.aspects.beta.tools ];
+          tools.provides.to-users.homeManager.home.sessionVariables.BETA = "yes";
+        };
+
+        expr = {
+          alpha = tuxHm.home.sessionVariables.ALPHA or "<missing>";
+          beta = tuxHm.home.sessionVariables.BETA or "<missing>";
+        };
+        expected = {
+          alpha = "yes";
+          beta = "yes";
+        };
+      }
+    );
+
+    # The delivery assertions above pass for any fix that merely keeps the two
+    # registrations apart. This pins the cause: the two sub-aspects must hold
+    # DISTINCT identities, which is what `hasAspect` reads.
+    test-same-named-sub-aspects-have-distinct-identities = denTest (
+      { den, igloo, ... }:
+      {
+        den.hosts.x86_64-linux.igloo.users.tux = { };
+
+        den.aspects.igloo.includes = [
+          den.aspects.alpha
+          den.aspects.beta
+        ];
+
+        den.aspects.alpha = {
+          includes = [ den.aspects.alpha.tools ];
+          tools.nixos.environment.etc."alpha".text = "yes";
+        };
+
+        den.aspects.beta = {
+          includes = [ den.aspects.beta.tools ];
+          tools.nixos.environment.etc."beta".text = "yes";
+        };
+
+        expr = {
+          alphaTools = igloo.environment.etc ? "alpha";
+          betaTools = igloo.environment.etc ? "beta";
+        };
+        expected = {
+          alphaTools = true;
+          betaTools = true;
+        };
+      }
+    );
+
+    # CONTROL: same shape, distinct sub-aspect names — reporter says this works.
+    test-control-distinct-sub-aspect-names = denTest (
+      { den, tuxHm, ... }:
+      {
+        den.hosts.x86_64-linux.igloo.users.tux = { };
+
+        den.aspects.igloo.includes = [
+          den.aspects.alpha
+          den.aspects.beta
+        ];
+
+        den.aspects.alpha = {
+          includes = [ den.aspects.alpha.atools ];
+          atools.provides.to-users.homeManager.home.sessionVariables.ALPHA = "yes";
+        };
+
+        den.aspects.beta = {
+          includes = [ den.aspects.beta.btools ];
+          btools.provides.to-users.homeManager.home.sessionVariables.BETA = "yes";
+        };
+
+        expr = {
+          alpha = tuxHm.home.sessionVariables.ALPHA or "<missing>";
+          beta = tuxHm.home.sessionVariables.BETA or "<missing>";
+        };
+        expected = {
+          alpha = "yes";
+          beta = "yes";
+        };
+      }
+    );
+
+  };
+}