Skip to content

Commit ce38772

Browse files
committed
feat: warn when a raw-ref exclude matches no claim anywhere in a resolution
An exclude naming a policy record den never found resolved to null, was filtered out, and suppressed nothing with no diagnostic. The check is global, not per-scope: resolveRawRefIdentity returning null for one scope is correct behaviour — a host-scope exclude reaches every descendant scope and the policy it names is typically claimed in only some of them — so warning from isPolicyExcluded would fire on every legitimate multi-scope exclude. It reads the terminal state at fxResolveFull's post-assembly position instead, where both the constraint registry and policyClaimsByName are complete.
1 parent 8fd5c6b commit ce38772

3 files changed

Lines changed: 138 additions & 1 deletion

File tree

nix/lib/aspects/fx/handlers/constraint.nix

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,43 @@ let
198198
in
199199
builtins.any directApplies directEntries || rawRefExcluded ? ${name};
200200

201+
# The `den:` diagnostics for raw-ref excludes that resolved to no claim
202+
# ANYWHERE in a finished resolution — a user naming a record den never found,
203+
# suppressing nothing, previously in silence.
204+
#
205+
# DELIBERATELY NOT ON THE PER-SCOPE PATH. resolveRawRefIdentity returning null
206+
# for one scope is ordinary correct behaviour: an exclude registered at host
207+
# scope reaches every descendant scope, and the policy it names is typically
208+
# claimed in only some of them. Warning from isPolicyExcluded would fire on
209+
# every non-matching scope of every legitimate exclude. The warnable condition
210+
# is global — no claim in the whole run carries this reference — so it takes
211+
# the TERMINAL state, and is read once at post-assembly (resolve.nix's
212+
# fxResolveFull), never during dispatch.
213+
#
214+
# Whole-value `==` against the claim bucket, matching resolveClaim's rule
215+
# exactly, minus its scope walk: the question here is whether the referenced
216+
# record registered AT ALL, not whether it registered somewhere a given scope
217+
# can see. Deliberately the weaker test — a claim in an unreachable sibling
218+
# scope stays silent rather than risk a false alarm.
219+
unmatchedRawRefExcludes =
220+
state:
221+
let
222+
registry = (state.scopedConstraintRegistry or (_: { })) null;
223+
claims = (state.policyClaimsByName or (_: { })) null;
224+
rawRefEntries = builtins.filter (e: e.type == "exclude" && (e.rawRef or null) != null) (
225+
builtins.concatMap (scopeData: builtins.concatLists (builtins.attrValues scopeData)) (
226+
builtins.attrValues registry
227+
)
228+
);
229+
isUnmatched = e: !(builtins.any (c: c.value == e.rawRef) (claims."name:${e.rawRef.name}" or [ ]));
230+
in
231+
lib.unique (
232+
map (
233+
e:
234+
"den: exclude in aspect '${e.owner}' names policy '${e.rawRef.name}', which never registered in this resolution — the exclude suppresses nothing"
235+
) (builtins.filter isUnmatched rawRefEntries)
236+
);
237+
201238
entryToResume =
202239
entry:
203240
if entry.type == "exclude" then
@@ -319,6 +356,7 @@ in
319356
foldScopeAncestors
320357
resolveClaim
321358
isPolicyExcluded
359+
unmatchedRawRefExcludes
322360
collectScopedConstraints
323361
scopedConstraintsFor
324362
scopedConstraintsForScope

nix/lib/aspects/fx/resolve.nix

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,15 @@ let
10681068
);
10691069
in
10701070
{
1071-
imports = phase4.${class} or [ ];
1071+
# Terminal position for the unmatched-raw-ref-exclude diagnostic: both
1072+
# the constraint registry and policyClaimsByName are complete on
1073+
# result.state here, and nothing per-scope can decide the question (see
1074+
# unmatchedRawRefExcludes in handlers/constraint.nix). Attached to
1075+
# `imports` so it surfaces exactly when the resolved module set is
1076+
# consumed, not when a path-set or edge-trace reader touches the bundle.
1077+
imports = lib.foldl' (v: msg: lib.warn msg v) (phase4.${class} or [ ]) (
1078+
handlers.unmatchedRawRefExcludes result.state
1079+
);
10721080
# Surfaced from the SAME result.state — this is thunked onto state.
10731081
pathSetByScope = result.state.pathSetByScope null;
10741082
# Per-scope ctx + entity-kind, so the entity surface can re-key the path
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
# A raw-ref exclude naming a policy den never found must say so.
2+
#
3+
# The two cells disagree on purpose. The diagnostic is GLOBAL — "this reference
4+
# matched no claim anywhere in the run" — because an exclude that resolves in
5+
# one scope and not another is CORRECT behaviour, not a user error. The control
6+
# below is the whole reason the check cannot sit on the per-scope dispatch path:
7+
# it holds a host-scope exclude whose policy is claimed only under one of two
8+
# users, so isPolicyExcluded genuinely answers true at one scope and false at
9+
# the other, and nothing may warn about it.
10+
{ denTest, ... }:
11+
let
12+
# The finished pipeline state for igloo — the terminal registries the
13+
# diagnostic reads, same shape resolve.nix's post-assembly sees.
14+
hostState =
15+
den:
16+
let
17+
fxLib = den.lib.aspects.fx;
18+
hostRoot = den.lib.resolveEntity "host" { host = den.hosts.x86_64-linux.igloo; };
19+
in
20+
(fxLib.pipeline.fxFullResolve {
21+
class = "nixos";
22+
ctx = fxLib.aspect.ctxFromHandlers (hostRoot.__scopeHandlers or { });
23+
self = den.lib.aspects.normalizeRoot hostRoot;
24+
}).state;
25+
in
26+
{
27+
flake.tests.unmatched-policy-exclude = {
28+
29+
test-exclude-naming-unregistered-policy-warns = denTest (
30+
{ den, ... }:
31+
let
32+
hostRoot = den.lib.resolveEntity "host" { host = den.hosts.x86_64-linux.igloo; };
33+
in
34+
{
35+
den.hosts.x86_64-linux.igloo.users.tux = { };
36+
den.policies.never-registered = _: [
37+
(den.lib.policy.include { nixos.environment.variables.MARKER = "yes"; })
38+
];
39+
den.aspects.igloo.excludes = [ den.policies.never-registered ];
40+
41+
expr = {
42+
messages = den.lib.aspects.fx.handlers.unmatchedRawRefExcludes (hostState den);
43+
# Forces the wired production path so the warning actually reaches
44+
# stderr; the message text itself is asserted above.
45+
resolves = (den.lib.aspects.resolve "nixos" hostRoot).imports != [ ];
46+
};
47+
expected = {
48+
messages = [
49+
"den: exclude in aspect 'igloo' names policy 'never-registered', which never registered in this resolution — the exclude suppresses nothing"
50+
];
51+
resolves = true;
52+
};
53+
}
54+
);
55+
56+
# The live control. tux claims the policy, pingu does not; the exclude is
57+
# declared once at host scope and reaches both. Excluded at tux, not at
58+
# pingu — and silent.
59+
test-exclude-matching-only-one-scope-is-silent = denTest (
60+
{ den, ... }:
61+
let
62+
handlers = den.lib.aspects.fx.handlers;
63+
st = hostState den;
64+
scopes = builtins.attrNames (st.scopeContexts null);
65+
scopeMatching = pat: builtins.head (builtins.filter (s: builtins.match pat s != null) scopes);
66+
excludedAt =
67+
scope:
68+
handlers.isPolicyExcluded st scope (handlers.scopedConstraintsForScope st scope) "add-marker";
69+
in
70+
{
71+
den.hosts.x86_64-linux.igloo.users.pingu = { };
72+
den.hosts.x86_64-linux.igloo.users.tux.aspect.includes = [ den.policies.add-marker ];
73+
den.policies.add-marker = _: [
74+
(den.lib.policy.include { homeManager.programs.git.enable = true; })
75+
];
76+
den.aspects.igloo.excludes = [ den.policies.add-marker ];
77+
78+
expr = {
79+
atTux = excludedAt (scopeMatching ".*user=tux.*");
80+
atPingu = excludedAt (scopeMatching ".*user=pingu.*");
81+
messages = handlers.unmatchedRawRefExcludes st;
82+
};
83+
expected = {
84+
atTux = true;
85+
atPingu = false;
86+
messages = [ ];
87+
};
88+
}
89+
);
90+
};
91+
}

0 commit comments

Comments
 (0)