From 3cd0188288e9df008000122e88ea9c889dac5308 Mon Sep 17 00:00:00 2001 From: Jason Bowman Date: Thu, 6 Aug 2026 10:36:23 -0700 Subject: [PATCH 1/3] fix: ._ shorthand stops synthesizing below the second nesting level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `den.aspects.a.b._` resolved, `den.aspects.a.b.c._` threw `attribute '_' missing`. aspectContentType synthesized `_` only on the wrapper for its own key; children below it were annotated with `__provider` and nothing else. The two annotators had already diverged: the single-def path walked one level and the multi-def path recursed, and neither carried `_`. Collapse them into one recursive `annotateChildren` so every node that gets provenance also gets its own `._`, at any depth. Include lists now map over the annotated children rather than the raw ones, which is what the multi-def path's own comment required — raw children get anon-renamed per inclusion path and double-emit class content. `provides`/`_` stay structural keys, so `._` still never collects provides children at any level. --- nix/lib/aspects/types.nix | 102 +++++++---------- .../modules/public-api/include-children.nix | 108 ++++++++++++++++++ 2 files changed, 151 insertions(+), 59 deletions(-) diff --git a/nix/lib/aspects/types.nix b/nix/lib/aspects/types.nix index d2dd86d09..dad57695b 100644 --- a/nix/lib/aspects/types.nix +++ b/nix/lib/aspects/types.nix @@ -489,45 +489,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 @@ -545,33 +517,47 @@ let # matching mergeWithAspectMeta behavior for root aspects. providesChildren = builtins.removeAttrs (merged.provides or { }) [ "_module" ]; 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 @@ -579,9 +565,7 @@ let __contentValues = flatDefs; __provider = provider; __providesForwarded = builtins.attrNames providesChildren; - _ = { - __functor = _self: _args: syntheticAspect; - }; + _ = underscoreAt provider annotatedMerged; } // lib.optionalAttrs singleFn { __functor = _self: (builtins.head flatDefs).value; diff --git a/templates/ci/modules/public-api/include-children.nix b/templates/ci/modules/public-api/include-children.nix index 30b97a354..52d356100 100644 --- a/templates/ci/modules/public-api/include-children.nix +++ b/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 ( { From 28267c04844dc6cf23947f4d1b9cacc25c7270c3 Mon Sep 17 00:00:00 2001 From: Jason Bowman Date: Thu, 6 Aug 2026 10:43:30 -0700 Subject: [PATCH 2/3] fix: provides child no longer masks a same-named class key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An aspect declaring `provides.user` alongside `user`-class content emitted no user class at all — silently, with no error and no diagnostic. `__providesForwarded` listed every provides child so classification would skip the forwarded copies, but the forwarded value only reaches the top level when the aspect does not define that name itself: the merge is `providesChildren // merged`, so a direct key wins. Masking it anyway left the surviving value classified as neither class nor nested key, and it fell out of the pipeline. Report the forwarded names only for children the aspect does not define directly, at both the root-aspect and nested-wrapper sites. `host-to-users` reads the same marker to decide entity-named sub-aspect fan-out, so it now matches its own stated rule too ("not from provides"). --- nix/lib/aspects/types.nix | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/nix/lib/aspects/types.nix b/nix/lib/aspects/types.nix index dad57695b..fd8f15e57 100644 --- a/nix/lib/aspects/types.nix +++ b/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; }; @@ -514,8 +520,11 @@ 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 ]; # A key names a candidate child aspect when it is neither structural, # internal, class nor pipe. Provides children are reached through @@ -564,7 +573,7 @@ let // { __contentValues = flatDefs; __provider = provider; - __providesForwarded = builtins.attrNames providesChildren; + __providesForwarded = unshadowedProvides; _ = underscoreAt provider annotatedMerged; } // lib.optionalAttrs singleFn { From c268fb49ac3c108dc82134e4ebc06dfb16efe6d5 Mon Sep 17 00:00:00 2001 From: Jason Bowman Date: Thu, 6 Aug 2026 11:04:24 -0700 Subject: [PATCH 3/3] fix: quirk keys defined on the same aspect twice collapse to one empty value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An aspect defined across several files, each contributing the same quirk key, reached consumers as a single entry carrying none of the emitted data — the consumer's pipe list held one attrset whose only attribute was `imports`. `unwrapContentValuesList` folds multiple definitions into `{ imports = vals; }`. That is right for a class key, where the module system merges the imports back into one value, and wrong for a quirk key, where nothing merges: the wrapper IS the value the consumer reads, and the definitions inside it are never seen. Split the two readings. `unwrapContentValuesAll` returns one value per definition; pipe emission uses it, class emission keeps the collapsing form. Both the emit-classes pipe path and the deferred-include drain in resolve.nix now emit one entry per definition, which is what assembly already expected — it concatenates entries per scope and per pipe. --- nix/lib/aspects/fx/content-util.nix | 30 +++++++++++++-- nix/lib/aspects/fx/handlers/emit-classes.nix | 6 ++- nix/lib/aspects/fx/resolve.nix | 6 ++- templates/ci/modules/public-api/pipes.nix | 40 ++++++++++++++++++++ 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/nix/lib/aspects/fx/content-util.nix b/nix/lib/aspects/fx/content-util.nix index 1e07ff3a4..980ed62ff 100644 --- a/nix/lib/aspects/fx/content-util.nix +++ b/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 ; diff --git a/nix/lib/aspects/fx/handlers/emit-classes.nix b/nix/lib/aspects/fx/handlers/emit-classes.nix index d6dca19f4..664be841b 100644 --- a/nix/lib/aspects/fx/handlers/emit-classes.nix +++ b/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: diff --git a/nix/lib/aspects/fx/resolve.nix b/nix/lib/aspects/fx/resolve.nix index 0421a6a65..dfe66dee1 100644 --- a/nix/lib/aspects/fx/resolve.nix +++ b/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: diff --git a/templates/ci/modules/public-api/pipes.nix b/templates/ci/modules/public-api/pipes.nix index bf0449778..dad4fca19 100644 --- a/templates/ci/modules/public-api/pipes.nix +++ b/templates/ci/modules/public-api/pipes.nix @@ -265,5 +265,45 @@ expected = "resolved"; } ); + + # Quirks accumulate: several definitions of one aspect (i.e. the same + # aspect name across several files) each contribute an entry, rather than + # collapsing into a single value the way class content does. + test-pipe-multi-def-same-aspect = denTest ( + { den, igloo, ... }: + { + imports = [ + (_: { + den.aspects.svc.firewall.ports = [ 80 ]; + }) + (_: { + den.aspects.svc.firewall.ports = [ 5432 ]; + }) + ]; + + den.hosts.x86_64-linux.igloo.users.tux = { }; + den.quirks.firewall = { + description = "Firewall port declarations"; + }; + + den.aspects.igloo.includes = [ + den.aspects.svc + den.aspects.consumer + ]; + + den.aspects.consumer.nixos = + { firewall, ... }: + { + networking.firewall.allowedTCPPorts = lib.concatMap (f: f.ports or [ ]) firewall; + }; + + # Merge order is not a contract — compare as a sorted set. + expr = lib.sort (a: b: a < b) igloo.networking.firewall.allowedTCPPorts; + expected = [ + 80 + 5432 + ]; + } + ); }; }