fix: three silent drops in aspect-key handling (._ depth, provides mask, quirk multi-def)

nix/lib/aspects/fx/content-util.nix

@@ -1,17 +1,24 @@
 { lib, ... }:
 let
+  # Definition values of a content wrapper, empty definitions dropped.
+  defValues =
+    rawValue:
+    builtins.filter (v: !(builtins.isAttrs v && v == { })) (map (d: d.value) rawValue.__contentValues);
+
   # Unwrap to a list of values, with empty-set fallback to [{}].
   # Used by aspect emitClassModules which needs per-element processing.
   # Matches source order: list check first, then __contentValues, then singleton.
+  #
+  # Several definitions of one CLASS key collapse into a single module: the
+  # module system merges them through `imports`, so the class sees one value.
+  # Quirk data has no such merge — see unwrapContentValuesAll.
   unwrapContentValuesList =
     rawValue:
     if builtins.isList rawValue then
       rawValue
     else if builtins.isAttrs rawValue && rawValue ? __contentValues then
       let
-        vals = builtins.filter (v: !(builtins.isAttrs v && v == { })) (
-          map (d: d.value) rawValue.__contentValues
-        );
+        vals = defValues rawValue;
       in
       if builtins.length vals == 0 then
         [ { } ]
@@ -22,6 +29,22 @@ let
     else
       [ rawValue ];
 
+  # Every definition, uncollapsed. Quirks accumulate rather than merge, so a
+  # pipe key defined by several modules must reach assembly as one entry per
+  # definition. Collapsing them into `{ imports = …; }` would hand consumers a
+  # single value carrying none of the emitted data.
+  unwrapContentValuesAll =
+    rawValue:
+    if builtins.isList rawValue then
+      rawValue
+    else if builtins.isAttrs rawValue && rawValue ? __contentValues then
+      let
+        vals = defValues rawValue;
+      in
+      if vals == [ ] then [ { } ] else vals
+    else
+      [ rawValue ];
+
   # Unwrap for key-classification inspection: merges attrset values
   # for sub-key detection, returns null for non-attrsets.
   unwrapContentValuesForClassification =
@@ -59,6 +82,7 @@ in
 {
   inherit
     unwrapContentValuesList
+    unwrapContentValuesAll
     unwrapContentValuesForClassification
     applyProvide
     ;

nix/lib/aspects/fx/handlers/emit-classes.nix

@@ -8,7 +8,7 @@
 let
   inherit (den.lib) fx;
   inherit (den.lib.aspects.fx) identity;
-  inherit (den.lib.aspects.fx.contentUtil) unwrapContentValuesList;
+  inherit (den.lib.aspects.fx.contentUtil) unwrapContentValuesList unwrapContentValuesAll;
   inherit (den.lib.schemaUtil) schemaEntityKindsSet;
 
   inherit (den.lib.aspects.fx.aspect) ctxFromHandlers;
@@ -105,10 +105,12 @@ let
     in
     fx.seq (lib.imap0 mkEntry modules);
 
+  # One entry per definition: quirk values accumulate at assembly, so several
+  # definitions of the same pipe key on one aspect must stay separate.
   emitPipeKey =
     aspect: ctx: contextDep: nodeIdentity: k:
     let
-      modules = unwrapContentValuesList aspect.${k};
+      modules = unwrapContentValuesAll aspect.${k};
       isMulti = builtins.length modules > 1;
       mkEntry =
         idx: module:

nix/lib/aspects/fx/resolve.nix

@@ -694,7 +694,7 @@ let
         let
           allDeferred = (result.state.scopedDeferredIncludes or (_: { })) null;
           inherit (den.lib.aspects.fx.keyClassification) classifyKeys;
-          inherit (den.lib.aspects.fx.contentUtil) unwrapContentValuesList;
+          inherit (den.lib.aspects.fx.contentUtil) unwrapContentValuesList unwrapContentValuesAll;
           # Build enriched context for a scope by inheriting parent enrichment.
           # Walks up scopeParent to find enrichment keys not present in the
           # scope's own context.
@@ -744,8 +744,10 @@ let
                   lib.concatMap (
                     k:
                     let
-                      modules = unwrapContentValuesList child.${k};
                       isPipe = den.quirks ? ${k};
+                      # Pipe keys keep one entry per definition (quirks accumulate);
+                      # class keys collapse into a single module (the module system merges).
+                      modules = if isPipe then unwrapContentValuesAll child.${k} else unwrapContentValuesList child.${k};
                     in
                     map (
                       module:

nix/lib/aspects/types.nix

@@ -96,8 +96,14 @@ let
       # Forward provides children onto the merged aspect so
       # aspect.docker resolves to aspect.provides.docker.
       # provides-first so direct freeform keys on merged take priority.
-      # __providesForwarded tells the pipeline to skip these during classification.
+      # __providesForwarded tells the pipeline to skip these during
+      # classification — but only for names the aspect does not define
+      # itself. A name held by both loses the forwarded value to the direct
+      # key above, so masking it would classify neither: an aspect declaring
+      # provides.user alongside user-class content would silently emit no
+      # user class at all.
       providesChildren = builtins.removeAttrs (merged.provides or { }) [ "_module" ];
+      unshadowedProvides = builtins.filter (k: !(merged ? ${k})) (builtins.attrNames providesChildren);
       # Child aspect keys for synthetic provides: freeform keys that are
       # not structural, internal, class, pipe, or forwarded-from-provides.
       classReg = den.classes or { };
@@ -132,7 +138,7 @@ let
     // merged
     // {
       __functor = if originalFunctor != null then originalFunctor else resolveAspectWith;
-      __providesForwarded = builtins.attrNames providesChildren;
+      __providesForwarded = unshadowedProvides;
       provides = syntheticProvides;
       _ = syntheticProvides;
     };
@@ -489,45 +495,17 @@ let
                           bv
                       ) b;
                     subForwarded = builtins.foldl' deepMerge { } subAttrVals;
-                    # Multi-def counterpart of annotatedMerged: tag forwarded
-                    # children recursively with __provider, else navigation
-                    # through a multi-def key yields raw children that get
-                    # anon-renamed per inclusion path and double-emit class
-                    # content. Recursive (unlike annotatedMerged) because
-                    # subForwarded never re-enters aspectContentType per
-                    # level. Name-based guards come first — forcing a
-                    # registered class value mid-merge can re-enter the flake
-                    # fixpoint (#580; see isNestedKey); only unregistered
-                    # namespace keys (forced by navigation anyway) get WHNF'd.
-                    annotateDeep =
-                      provPath: attrs:
-                      lib.mapAttrs (
-                        ck: cv:
-                        let
-                          childPath = provPath ++ [ ck ];
-                        in
-                        if
-                          !(lib.hasPrefix "__" ck)
-                          && !(classReg ? ${ck})
-                          && !(pipeReg ? ${ck})
-                          && !(structuralKeysSet ? ${ck})
-                          && builtins.isAttrs cv
-                          && !(cv ? __provider)
-                          && !(cv ? __contentValues)
-                        then
-                          annotateDeep childPath cv // { __provider = childPath; }
-                        else
-                          cv
-                      ) attrs;
                     provBase = (typeCfg.providerPrefix or [ ]) ++ [
                       keyName
                       k
                     ];
+                    annotatedSub = annotateChildren provBase subForwarded;
                   in
-                  annotateDeep provBase subForwarded
+                  annotatedSub
                   // {
                     __contentValues = defsForKey;
                     __provider = provBase;
+                    _ = underscoreAt provBase annotatedSub;
                   }
               );
           # Single-function content wrappers need __functor so the wrapper is
@@ -542,46 +520,61 @@ let
           inherit (den.lib.aspects.fx.keyClassification) structuralKeysSet;
           # Forward provides children onto the wrapper so
           # aspect.child.monitoring resolves to aspect.child.provides.monitoring,
-          # matching mergeWithAspectMeta behavior for root aspects.
+          # 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" ];
+          unshadowedProvides = builtins.filter (k: !(merged ? ${k})) (builtins.attrNames providesChildren);
           provider = (typeCfg.providerPrefix or [ ]) ++ [ keyName ];
-          childKeys = builtins.filter (
-            k: !(structuralKeysSet ? ${k}) && !(lib.hasPrefix "__" k) && !(classReg ? ${k}) && !(pipeReg ? ${k})
-          ) (builtins.attrNames merged);
-          aspectName = lib.concatStringsSep "." provider;
-          syntheticAspect = {
-            name = "${aspectName}._";
-            includes = map (k: merged.${k}) childKeys;
+          # A key names a candidate child aspect when it is neither structural,
+          # internal, class nor pipe. Provides children are reached through
+          # `provides`/`_`, which are structural — ._ never collects them.
+          isChildKey =
+            k:
+            !(structuralKeysSet ? ${k}) && !(lib.hasPrefix "__" k) && !(classReg ? ${k}) && !(pipeReg ? ${k});
+          # The synthetic aspect behind ._ at a given tree position.
+          underscoreAt = provPath: attrs: {
+            __functor = _self: _args: {
+              name = "${lib.concatStringsSep "." provPath}._";
+              includes = map (k: attrs.${k}) (builtins.filter isChildKey (builtins.attrNames attrs));
+            };
           };
           # Annotate nested attrset children with __provider so deeply nested
-          # aspects carry provenance for hasAspect resolution.
-          # Only annotate unregistered keys (potential nested aspects) —
-          # skip class keys, pipe keys, structural keys, and internal keys.
-          annotatedMerged = lib.mapAttrs (
-            k: v:
-            if
-              builtins.isAttrs v
-              && !(v ? __provider)
-              && !(v ? __contentValues)
-              && !(lib.hasPrefix "__" k)
-              && !(classReg ? ${k})
-              && !(pipeReg ? ${k})
-              && !(structuralKeysSet ? ${k})
-            then
-              v // { __provider = provider ++ [ k ]; }
-            else
-              v
-          ) merged;
+          # aspects carry provenance for hasAspect resolution, and give each
+          # one its own ._ so the shorthand holds at every depth rather than
+          # only at this wrapper. Without the recursion, navigation through a
+          # nested key also yields raw children that get anon-renamed per
+          # inclusion path and double-emit class content.
+          # Name-based guards come first — forcing a registered class value
+          # mid-merge can re-enter the flake fixpoint (#580; see isNestedKey);
+          # only unregistered namespace keys (forced by navigation anyway) get
+          # WHNF'd.
+          annotateChildren =
+            provPath: attrs:
+            lib.mapAttrs (
+              k: v:
+              let
+                childPath = provPath ++ [ k ];
+                sub = annotateChildren childPath v;
+              in
+              if isChildKey k && builtins.isAttrs v && !(v ? __provider) && !(v ? __contentValues) then
+                sub
+                // {
+                  __provider = childPath;
+                  _ = underscoreAt childPath sub;
+                }
+              else
+                v
+            ) attrs;
+          annotatedMerged = annotateChildren provider merged;
         in
         providesChildren
         // annotatedMerged
         // {
           __contentValues = flatDefs;
           __provider = provider;
-          __providesForwarded = builtins.attrNames providesChildren;
-          _ = {
-            __functor = _self: _args: syntheticAspect;
-          };
+          __providesForwarded = unshadowedProvides;
+          _ = underscoreAt provider annotatedMerged;
         }
         // lib.optionalAttrs singleFn {
           __functor = _self: (builtins.head flatDefs).value;

templates/ci/modules/public-api/include-children.nix

@@ -348,6 +348,114 @@
       }
     );
 
+    # ._ at depth 3 — synthesis must not stop at the first nesting level
+    test-include-children-depth-three = denTest (
+      {
+        den,
+        igloo,
+        ...
+      }:
+      {
+        den.hosts.x86_64-linux.igloo.users.tux = { };
+
+        den.aspects.a.b.c.x.nixos.services.openssh.enable = true;
+        den.aspects.a.b.c.y.nixos.networking.nameservers = [ "1.1.1.1" ];
+
+        den.aspects.igloo.includes = [ den.aspects.a.b.c._ ];
+
+        expr = {
+          ssh = igloo.services.openssh.enable;
+          dns = igloo.networking.nameservers;
+        };
+        expected = {
+          ssh = true;
+          dns = [ "1.1.1.1" ];
+        };
+      }
+    );
+
+    # ._ at depth 4 — synthesis is depth-unbounded
+    test-include-children-depth-four = denTest (
+      {
+        den,
+        igloo,
+        ...
+      }:
+      {
+        den.hosts.x86_64-linux.igloo.users.tux = { };
+
+        den.aspects.a.b.c.d.x.nixos.services.openssh.enable = true;
+        den.aspects.a.b.c.d.y.nixos.networking.nameservers = [ "1.1.1.1" ];
+
+        den.aspects.igloo.includes = [ den.aspects.a.b.c.d._ ];
+
+        expr = {
+          ssh = igloo.services.openssh.enable;
+          dns = igloo.networking.nameservers;
+        };
+        expected = {
+          ssh = true;
+          dns = [ "1.1.1.1" ];
+        };
+      }
+    );
+
+    # Depth-3 ._ excludes class keys, same as the shallower levels
+    test-include-children-depth-three-skips-class-keys = denTest (
+      {
+        den,
+        igloo,
+        ...
+      }:
+      {
+        den.hosts.x86_64-linux.igloo.users.tux = { };
+
+        den.aspects.a.b.c = {
+          nixos.networking.hostName = "should-not-leak";
+          child.nixos.services.openssh.enable = true;
+        };
+
+        den.aspects.igloo.includes = [ den.aspects.a.b.c._ ];
+
+        expr = {
+          ssh = igloo.services.openssh.enable;
+          hostName = igloo.networking.hostName;
+        };
+        expected = {
+          ssh = true;
+          hostName = "nixos";
+        };
+      }
+    );
+
+    # Depth-3 ._ collects children contributed by several definitions
+    test-include-children-depth-three-multi-def = denTest (
+      {
+        den,
+        igloo,
+        ...
+      }:
+      {
+        den.hosts.x86_64-linux.igloo.users.tux = { };
+
+        den.aspects.a.b.c.x.nixos.services.openssh.enable = true;
+        den.aspects.a.b = {
+          c.y.nixos.networking.nameservers = [ "1.1.1.1" ];
+        };
+
+        den.aspects.igloo.includes = [ den.aspects.a.b.c._ ];
+
+        expr = {
+          ssh = igloo.services.openssh.enable;
+          dns = igloo.networking.nameservers;
+        };
+        expected = {
+          ssh = true;
+          dns = [ "1.1.1.1" ];
+        };
+      }
+    );
+
     # Nested ._ with parametric child aspects
     test-include-children-nested-parametric = denTest (
       {