Skip to content

Commit 3932adf

Browse files
authored
fix: derive class-content emit ctx from authoritative scope state (#624)
## Problem A **static** (constant-name) aspect included via `den.schema.<kind>.includes` whose host-class (`nixos`/`darwin`) content **names an entity kind** — e.g. `nixos = { user, ... }: …` — collapsed **N sibling entities → 1** at the shared host merge. With three users on a host, only one user's content survived; the other two were dropped *before evaluation* by `dedupByKey` on a sid-free aspect identity. ### Root cause `emit-classes` derived its emit `ctx` from the per-aspect `aspect.__scopeHandlers` attribute. That attribute is only populated for **parametric** aspects (bind augmentation) or for includes **propagated** from a parent that already has it — and is **absent** for a static aspect reached through a static include chain. So such content was emitted with an **empty ctx**, collapsing to a base identity. The same content delivered from multiple scopes (e.g. a host aspect that also has `homeManager` content, reaching both the host scope and the user scopes) keyed **inconsistently** (base vs context-qualified), surfacing as duplicate unique-option definitions (`programs.steam.package defined multiple times`) the moment any per-context keying was applied. The authoritative scope context was always present in pipeline state right next to the drop point — `state.scopeContexts.${currentScope}` — and is exactly what `bind.nix` already reads. ## Fix (one handler, a unification) 1. **Authoritative ctx.** `emit-classes` now reads the scope's context from pipeline state (the same source/pattern as `bind.nix`), layering the aspect's own `__scopeHandlers` on top so fan-out child bindings still win. Gated to child scopes; the root scope keeps the historic handler-only path. 2. **Key by named args.** Each class-content entry is keyed by the entity kinds its function **names** and that are present in ctx: - `nixos = { user, ... }:` → `{user=<u>}` → **fans per user** - `nixos = { host, ... }:` → `{host=<h>}` → **dedups** across every scope it's delivered from - `nixos = { persist, ... }:` / `_:` → no entity kind → **singular** (shared infra aspects keep deduping; no double option declaration) ## Validation - `just ci`: **1044/1044** (adds `user-scoped-host-class-fanout` regression suite — the missing coverage that let this through). - **Byte-identical** existing output: real nixos + k8s hosts produce the same toplevel derivation pre/post fix (drvPath diff). Only previously-miskeyed per-user content changes.
1 parent 11866c1 commit 3932adf

2 files changed

Lines changed: 145 additions & 3 deletions

File tree

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

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ let
99
inherit (den.lib) fx;
1010
inherit (den.lib.aspects.fx) identity;
1111
inherit (den.lib.aspects.fx.contentUtil) unwrapContentValuesList;
12+
inherit (den.lib.schemaUtil) schemaEntityKindsSet;
1213

1314
inherit (den.lib.aspects.fx.aspect) ctxFromHandlers;
1415

@@ -20,6 +21,40 @@ let
2021
(resolvedArgs != [ ] && builtins.any (ak: ctx ? ${ak}) resolvedArgs)
2122
|| (aspect.meta.contextDependent or false);
2223

24+
# Entity kinds a class-content module NAMES as function args AND that are
25+
# present in the emitting scope's context. Such content is per-instance of
26+
# those entities: key its identity by them so it neither over-collapses nor
27+
# over-fans.
28+
# - `nixos = { user, ... }:` → keyed {user=<u>} → fans per user (the bug
29+
# this fixes: a static user-scoped aspect's host-class content used to
30+
# collapse N users → 1 at the shared host merge).
31+
# - `nixos = { host, ... }:` → keyed {host=<h>} → dedups across every scope
32+
# the aspect is delivered from (host scope + the user scopes it reaches via
33+
# home content), so host-targeted content stays one module.
34+
# - `nixos = { persist, ... }:` / `_:` (no entity kind) → no suffix → singular
35+
# (shared infra aspects like impermanence keep deduping; no double option
36+
# declaration).
37+
# Relies on the emit ctx being the authoritative scope context (see handler) —
38+
# otherwise `ctx ? <kind>` is unreliable and the keying is path-dependent.
39+
namedEntityArgs =
40+
ctx: module:
41+
if builtins.isFunction module then
42+
builtins.filter (a: (schemaEntityKindsSet ? ${a}) && (ctx ? ${a})) (
43+
builtins.attrNames (builtins.functionArgs module)
44+
)
45+
else
46+
[ ];
47+
48+
# Per-instance identity suffix for the named entity kinds, e.g.
49+
# "/{host=cortex,user=sini}". Empty when the content names no entity kind.
50+
# attrNames is already sorted, so the suffix is deterministic.
51+
entityIdSuffix =
52+
ctx: args:
53+
if args == [ ] then
54+
""
55+
else
56+
"/{" + lib.concatStringsSep "," (map (a: "${a}=${ctx.${a}.name or "?"}") args) + "}";
57+
2358
emitClassEntry =
2459
{
2560
class,
@@ -50,16 +85,22 @@ let
5085
isMulti = builtins.length modules > 1;
5186
mkEntry =
5287
idx: module:
88+
let
89+
entityArgs = namedEntityArgs ctx module;
90+
baseId = if isMulti then "${nodeIdentity}[${toString idx}]" else nodeIdentity;
91+
in
5392
emitClassEntry {
5493
class = k;
55-
identity = if isMulti then "${nodeIdentity}[${toString idx}]" else nodeIdentity;
94+
identity = baseId + entityIdSuffix ctx entityArgs;
5695
inherit
5796
module
5897
ctx
5998
aspectPolicy
6099
globalPolicy
61100
;
62-
isContextDependent = contextDep;
101+
# Content naming an entity kind is per-instance ⇒ keep the {…} suffix
102+
# through identity computation (wrap-classes.nix finalIdentity).
103+
isContextDependent = contextDep || entityArgs != [ ];
63104
};
64105
in
65106
fx.seq (lib.imap0 mkEntry modules);
@@ -93,7 +134,27 @@ in
93134
classKeys = param.classKeys;
94135
pipeKeys = param.pipeKeys or [ ];
95136
nodeIdentity = param.identity;
96-
ctx = ctxFromHandlers (aspect.__scopeHandlers or { });
137+
# Authoritative emit context: the scope's own context from pipeline state
138+
# (host + any descendant entity bindings), the SAME source bind.nix reads
139+
# — not the per-aspect `__scopeHandlers`, which is only populated for
140+
# parametric aspects / propagated includes and is ABSENT for a static
141+
# aspect on a static include chain (→ empty ctx → path-dependent identity
142+
# and the N→1 host-class collapse). The aspect's own handlers (fan-out
143+
# child bindings) layer on top so they still win. Child scopes only; the
144+
# root scope keeps the historic handler-only path (byte-stable).
145+
currentScope = state.currentScope or null;
146+
rootScopeId = state.rootScopeId or null;
147+
isChildScope = currentScope != null && rootScopeId != null && currentScope != rootScopeId;
148+
scopeCtx =
149+
if isChildScope then
150+
let
151+
ctxs = (state.scopeContexts or (_: { })) null;
152+
entityCls = ((state.scopeEntityClass or (_: { })) null).${currentScope} or null;
153+
in
154+
(ctxs.${currentScope} or { }) // lib.optionalAttrs (entityCls != null) { class = entityCls; }
155+
else
156+
{ };
157+
ctx = scopeCtx // ctxFromHandlers (aspect.__scopeHandlers or { });
97158
aspectPolicy = aspect.meta.collisionPolicy or null;
98159
globalPolicy = den.config.classModuleCollisionPolicy or "error";
99160
contextDep = isContextDep aspect ctx;
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Regression: a STATIC (constant-name) aspect included via den.schema.user.includes
2+
# whose host-class (nixos) content NAMES `user` must fan PER-USER. Its content
3+
# merges into the single shared host config, so without per-content keying it
4+
# collapses N users → 1 (dedupByKey on a sid-free aspect identity) and all but
5+
# one user's content is dropped before evaluation. The fix keys class content by
6+
# the entity kinds its function NAMES (emit-classes.nix): naming `user` ⇒ per-user
7+
# identity. Content that names NO entity kind stays singular (shared infra aspects
8+
# like impermanence keep deduping) — exercised throughout the rest of the suite.
9+
{ denTest, ... }:
10+
{
11+
flake.tests.user-scoped-host-class-fanout = {
12+
13+
# Two users on one host. A static user-include sets per-user SYSTEM config
14+
# (environment.etc keyed by user.name). BOTH must materialize on the host.
15+
test-static-user-include-nixos-fans-per-user = denTest (
16+
{
17+
den,
18+
igloo,
19+
lib,
20+
...
21+
}:
22+
{
23+
den.hosts.x86_64-linux.igloo.users = {
24+
tux = { };
25+
pingu = { };
26+
};
27+
28+
# Static aspect: same identity for every user. Its nixos content names
29+
# `user`, so it must key per-user rather than collapse to one identity.
30+
den.aspects.per-user-probe.nixos =
31+
{ user, ... }:
32+
{
33+
environment.etc."probe-${user.name}".text = "user=${user.name}";
34+
};
35+
den.schema.user.includes = [ den.aspects.per-user-probe ];
36+
37+
expr = lib.sort (a: b: a < b) (
38+
builtins.filter (n: lib.hasPrefix "probe-" n) (builtins.attrNames igloo.environment.etc)
39+
);
40+
# Pre-fix this was a single entry (one arbitrary user won the dedup).
41+
expected = [
42+
"probe-pingu"
43+
"probe-tux"
44+
];
45+
}
46+
);
47+
48+
# The per-user content closes over the RIGHT user (no cross-user leakage):
49+
# each probe's text reflects its own user, not the survivor's.
50+
test-per-user-content-binds-own-user = denTest (
51+
{
52+
den,
53+
igloo,
54+
...
55+
}:
56+
{
57+
den.hosts.x86_64-linux.igloo.users = {
58+
tux = { };
59+
pingu = { };
60+
};
61+
62+
den.aspects.per-user-probe.nixos =
63+
{ user, ... }:
64+
{
65+
environment.etc."probe-${user.name}".text = "user=${user.name}";
66+
};
67+
den.schema.user.includes = [ den.aspects.per-user-probe ];
68+
69+
expr = {
70+
tux = igloo.environment.etc."probe-tux".text;
71+
pingu = igloo.environment.etc."probe-pingu".text;
72+
};
73+
expected = {
74+
tux = "user=tux";
75+
pingu = "user=pingu";
76+
};
77+
}
78+
);
79+
80+
};
81+
}

0 commit comments

Comments
 (0)