Skip to content

Commit a2dbe02

Browse files
committed
fix: don't re-declare forward adapters in host-aspects spawns
A custom class built with `_.forward` fails to evaluate when the `host-aspects` battery is also included: The option `home-manager.users.<u>.den.fwd."<from>/<into>/<path>"' is already declared #603 made a spawned node apply the parent pipeline's subtree routes, so a user-schema route (homeLinux->homeManager) fires against the content the spawn re-emits. Adapter-bearing routes are not safe under that rule: `mkAdapterAspect` / `mkAdapterFunctor` materialize an option DECLARATION, and the parent pipeline already materializes the same route at the same scope. Both folds land in one home-manager evaluation, so the second declaration is a hard conflict — content definitions merge, declarations do not. Exclude declaration-bearing parent routes from the spawn's route merge, leaving the parent's copy as the single owner. That copy is always there to take over: an excluded route is by construction inside the spawned subtree, and the redundant-root suppression only fires on a route at the fold root. Simple routes keep re-applying, preserving #603. Four regression cases (user-defined content, host-defined content, host-defined across two users, and a no-battery control); the three battery cases fail at the parent commit with the reported error. Reported in discussion #642.
1 parent 7f11ba1 commit a2dbe02

2 files changed

Lines changed: 194 additions & 1 deletion

File tree

nix/lib/aspects/fx/spawn-node.nix

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,27 @@ in
161161
allScopeIds = spawnAllScopeIds;
162162
}) (_: true);
163163

164+
# A route that materializes an adapter DECLARES `options.den.fwd.<key>` in
165+
# the target bucket (handlers/forward.nix mkAdapterAspect, edges/route.nix
166+
# mkAdapterFunctor). Content definitions merge; an option DECLARATION does
167+
# not — a second one in the same evalModules is a hard "already declared"
168+
# error. Of buildForwardAspect's three arms only mkAdapterAspect declares:
169+
# the top-level adapter arm evaluates inline and mkDirectAspect places
170+
# content, so both stay.
171+
#
172+
# Testing __complexForward FIRST is load-bearing, not stylistic: every
173+
# complex forward carries an adapterKey (lib/forward.nix always builds
174+
# one), so a bare `adapterKey != null` would also exclude non-declaring
175+
# forwards — among them the home-manager battery's own delivery route.
176+
# The simple-route arm is defensive: adapterKey has one in-tree producer
177+
# (lib/forward.nix), which only builds complex-forward specs.
178+
declaresForwardOption =
179+
r:
180+
if r.__complexForward or false then
181+
(r.needsAdapter or false) && !(r.needsTopLevelAdapter or false)
182+
else
183+
(r.adapterKey or null) != null;
184+
164185
# DELIBERATE: parent-pipeline routes sourced inside the spawned
165186
# subtree MUST re-apply — the spawn re-emits class content at the same scope
166187
# ids but never re-fires schema policies, so without them a user-schema route
@@ -171,6 +192,24 @@ in
171192
# simple route would re-nest content in fresh keyless wrappers and conflict
172193
# at the target). Order/precedence preserved exactly: freshParent (parent
173194
# routes whose key ∉ spawn keys) ++ spawnHere.
195+
#
196+
# Declaration-bearing parent routes are excluded outright, leaving the
197+
# parent's copy as the single owner of that declaration: the parent
198+
# materializes the same route at the same scope and both folds land in one
199+
# target (the user's home-manager evaluation), so re-applying here emits a
200+
# second `den.fwd.<key>` declaration.
201+
#
202+
# The parent's copy is always present to take over — an excluded route is
203+
# by construction inside the spawned subtree, and `suppressionVerdicts`'
204+
# redundant-root rule only fires on a route AT the fold root, which for the
205+
# parent is an ancestor of spawnRoot.
206+
#
207+
# For a forward-only custom class the parent's copy also carries what the
208+
# spawn would have forwarded, because `getCollectedSource` pulls root-scope
209+
# content for it. That is NOT general: `filterRootModules` narrows root
210+
# content to `den.default` modules once fromClass is an entity-owned class,
211+
# and the parent collects the host bucket unprojected where the spawn would
212+
# have re-resolved it per user.
174213
spawnRoutes = result.state.scopedRoutes null;
175214
parentSubtreeRoutes = lib.filterAttrs (sid: _: subtreeSet ? ${sid}) parentState.scopedRoutes;
176215
mergedSpawnRoutes =
@@ -180,7 +219,9 @@ in
180219
let
181220
spawnHere = spawnRoutes.${sid} or [ ];
182221
spawnKeys = lib.genAttrs (map (routeKey sid) spawnHere) (_: true);
183-
freshParent = builtins.filter (r: !(spawnKeys ? ${routeKey sid r})) parentRoutes;
222+
freshParent = builtins.filter (
223+
r: !(spawnKeys ? ${routeKey sid r}) && !(declaresForwardOption r)
224+
) parentRoutes;
184225
in
185226
freshParent ++ spawnHere
186227
) parentSubtreeRoutes;
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
# Discussion #642: a custom class defined via `_.forward` double-emits its
2+
# forwarded content when the `host-aspects` battery is also included, producing
3+
# an "already declared" option conflict on `den.fwd."<aspect>/<class>/<path>"`.
4+
#
5+
# The battery's spawn re-applied the parent pipeline's own forward route, so the
6+
# adapter's `options.den.fwd.<key>` declaration was materialized by two folds
7+
# that both land in the user's home-manager evaluation.
8+
{ denTest, ... }:
9+
let
10+
# One definition, threaded per test — each `denTest` gets its own `den`/`lib`.
11+
obsidianClass =
12+
den: lib:
13+
{ class, aspect-chain }:
14+
den._.forward {
15+
each = lib.singleton class;
16+
fromClass = _: "obsidian";
17+
intoClass = _: "homeManager";
18+
intoPath = _: [
19+
"programs"
20+
"obsidian"
21+
];
22+
fromAspect = _: lib.last aspect-chain;
23+
guard = { options, ... }: options ? programs.obsidian;
24+
};
25+
in
26+
{
27+
flake.tests.deadbugs.issue-642-host-aspects-custom-class = {
28+
29+
# The reported shape: user-defined content through the custom class, with
30+
# the battery on.
31+
test-custom-class-with-host-aspects = denTest (
32+
{
33+
den,
34+
lib,
35+
tuxHm,
36+
...
37+
}:
38+
{
39+
den.default.includes = [
40+
den._.host-aspects
41+
(obsidianClass den lib)
42+
];
43+
44+
den.aspects.obsidian = {
45+
obsidian.enable = true;
46+
};
47+
48+
den.hosts.x86_64-linux.igloo.users.tux = { };
49+
50+
den.aspects.tux.includes = [ den.aspects.obsidian ];
51+
52+
expr = tuxHm.programs.obsidian.enable;
53+
expected = true;
54+
}
55+
);
56+
57+
# The custom class's content lives on the HOST aspect and reaches the user
58+
# only through host-aspects — the projection the duplicate-suppression must
59+
# not strand.
60+
test-host-defined-custom-class-projects = denTest (
61+
{
62+
den,
63+
lib,
64+
tuxHm,
65+
...
66+
}:
67+
{
68+
den.default.includes = [
69+
den._.host-aspects
70+
(obsidianClass den lib)
71+
];
72+
73+
den.aspects.obsidian = {
74+
obsidian.enable = true;
75+
};
76+
77+
den.hosts.x86_64-linux.igloo.users.tux = { };
78+
79+
den.aspects.igloo.includes = [ den.aspects.obsidian ];
80+
81+
expr = tuxHm.programs.obsidian.enable;
82+
expected = true;
83+
}
84+
);
85+
86+
# Suppressing the spawn's copy leaves ONE owner for the declaration, so that
87+
# owner must still reach every user on the host — not just the first.
88+
test-host-defined-custom-class-reaches-every-user = denTest (
89+
{
90+
den,
91+
lib,
92+
igloo,
93+
...
94+
}:
95+
{
96+
den.default.includes = [
97+
den._.host-aspects
98+
(obsidianClass den lib)
99+
];
100+
101+
den.aspects.obsidian = {
102+
obsidian.enable = true;
103+
};
104+
105+
den.hosts.x86_64-linux.igloo.users = {
106+
tux = { };
107+
pingu = { };
108+
};
109+
110+
den.aspects.igloo.includes = [ den.aspects.obsidian ];
111+
112+
expr = {
113+
tux = igloo.home-manager.users.tux.programs.obsidian.enable or "<missing>";
114+
pingu = igloo.home-manager.users.pingu.programs.obsidian.enable or "<missing>";
115+
};
116+
expected = {
117+
tux = true;
118+
pingu = true;
119+
};
120+
}
121+
);
122+
123+
# CONTROL: same custom class without the host-aspects battery — the one
124+
# shape that stayed green while the bug was live, isolating the battery as
125+
# the trigger.
126+
test-custom-class-without-host-aspects = denTest (
127+
{
128+
den,
129+
lib,
130+
tuxHm,
131+
...
132+
}:
133+
{
134+
den.default.includes = [
135+
(obsidianClass den lib)
136+
];
137+
138+
den.aspects.obsidian = {
139+
obsidian.enable = true;
140+
};
141+
142+
den.hosts.x86_64-linux.igloo.users.tux = { };
143+
144+
den.aspects.tux.includes = [ den.aspects.obsidian ];
145+
146+
expr = tuxHm.programs.obsidian.enable;
147+
expected = true;
148+
}
149+
);
150+
151+
};
152+
}

0 commit comments

Comments
 (0)