fix: keep unnameable context values out of scope identity - #682
Merged
Merged
Conversation
follow-up to the report behind #681, which asked whether a quirk reached through `den.hosts.<sys>.<name>.includes` is collected differently from one reached through `den.aspects.<name>.includes`. ten cells, each pairing an instance spelling with its aspect control: host scope, user scope, the flat `den.hosts.<name>` spelling, a producer and consumer split across the two entrypoints, a pipe policy over an instance-included producer, and a flat host declaring `system` in one module and content in another. all ten pass on #681 with no further change, so this records parity rather than fixing anything.
Collaborator
|
theutz posted two screenshots on #682 showing `den.hosts.kocaeli.includes` yielding an empty `homebrew.taps` where `den.aspects.kocaeli.includes` yields the tap, a two-byte diff between the two runs. that report is darwin and uses the flat `den.hosts.<name>` spelling with the host declared in one module and the collection added from another. every existing cell is nixos, x86_64-linux and two-level, so none of it was covered. three cells mirroring the reported shape, all green at 20d1e76, so the divergence does not reproduce on this tree.
`mkScopeId` built the scope identity from every ctx key, including ones whose value it cannot name. Those fell through to the type placeholder branch and rendered as `den=<set:den>`, `inputs=<set:inputs>`, `lib=<set:lib>`. a value that can only render as its own type distinguishes nothing, so it adds no identity. all it does is split one logical scope across two ids depending on which module args happened to be bound on the path. pipe emits are bucketed by scope id and a consumer reads its own bucket, so a producer and a consumer that belong to the same logical scope landed in different buckets and the collection came back empty. reported by theutz on #682: a quirk reached through `den.hosts.<name>.includes` arrived empty while the same aspect through `den.aspects.<name>.includes` delivered. traced on his host, where the user scope pushed ctx keys `den|host|inputs|lib|system|user` and the producer emitted at `host=kocaeli,system=aarch64-darwin` while the surviving consumer bound at the polluted id. the keys stay in the context, where binding still reads them. only the identity is narrowed.
sini
changed the title
theutz
approved these changes
Sep 18, 2026
sini
enabled auto-merge (squash)
September 18, 2026 20:55
sini
added a commit
that referenced
this pull request
Sep 19, 2026
Closes #683. ## Summary - Keeps `__providesForwarded` intact when an already-merged aspect is merged a second time, so a `provides` child whose name matches a registered class stays a child instead of being reclassified as the aspect's own class content and emitted into that class. In the reporting configuration a `provides.packages` child reached through an `<angle/bracket>` include arrived as `flake.packages.<system>` carrying fourteen of the child aspect's own fields — `name`, `includes`, `meta`, `provides`, `_`, `__functor`, `homeManager`, `darwin` among them — which a derivation-typed package schema then fails on. - Routes the shadow test through `mkUnderscore`, which already holds both the aspect and the candidate set, rather than leaving a copy at each of the three construction sites. All three derived it from `!(own ? k)`, which cannot tell a name the author wrote at top level from a name an earlier merge forwarded there; the earlier merge published exactly that distinction, so the predicate reads the marker instead of guessing from key presence. - Fixes all three sites, not the reported one. #683 bisects to #678, which is correct for the path it hits (`mergeFunctions`, site B) but not for the defect: `d50f0fc` already carried the identical test at `mergeWithAspectMeta` and `aspectContentType` (lines 132 and 579), so aliasing a merged aspect to a root slot or to a nested freeform key lost the marker before #678. #678 extended the flaw to the include path. - Leaves `packages` registered as a class. The leak is not name-specific — `provides.nixos` reproduces it on stock den with no flake outputs imported — so un-reserving `packages` would address one spelling and leave `nixos`, `darwin`, `homeManager`, `apps`, `checks`, `devShells`, `legacyPackages`, `os`, `user`, `hjem`, `maid` and `wsl` unchanged, while breaking `packages-merge.nix`, the #572 regression asserting `den.aspects.<name>.packages` reaches `flake.packages.<system>`. - Adds two cells: the marker and its classification at the two re-merge sites readable by marker, with the once-merged source as a live control in the same run; and the reporter's shape end to end, since the symptom is the child's own fields arriving as flake output content rather than a marker value. ## Behaviour changes - A provides child sharing a name with a registered class is no longer emitted as that class's content when its aspect is re-merged. A configuration that was receiving the child aspect's fields in a flake output, or its `nixos`/`darwin`/`homeManager` content delivered twice, stops receiving them. That content was never the class content the key named. - A provides child with an ordinary name is no longer reclassified as a nested key on re-merge. Nested keys are not auto-walked, so this was silent. - A name the author genuinely writes at top level alongside a same-named provides child keeps its direct value and stays classified, unchanged. That case is what the shadow test exists for and the predicate still answers it. ## Validation - `nix develop -c just ci`: 1238/1238, exit 0, zero `❌` and zero `☢️`, counted off a capture with stderr merged. The summary numerator agrees with the counted passes, so the `pass + fail` identity is not standing in for a truncated run. - Ran the same suite on the tree with only `types.nix` reverted: 1238 collected, 1236 passing, exit 1, and the two failures are exactly the two new cells. Same population both runs, so nothing else moved in either direction. 1236 is also the count #682 landed at, which is the independent check that the population is right. - Falsified both new cells against the unfixed tree on their actual symptom rather than on a mismatch: `aMarker`/`cMarker` read `[ ]` against `[ "nixos" ]` and `aClassified`/`cClassified` read `[ "nixos" ]` against `[ ]`, while the once-merged control in the same cell stays green — so the cell measures the re-merge rather than a marker that vanished everywhere. - Reproduced #683 as filed before touching anything, against the local checkout rather than the pinned revision: `originalForwarded = [ "packages" ]`, `remergedForwarded = [ ]`, `remergedClasses.classKeys = [ "packages" ]`, and thirteen names in `flake.packages.x86_64-linux`. - Measured the name-independence claim rather than reasoning to it: `provides.nixos` loses the marker and gains `classKeys = [ "nixos" ]` on re-merge with no flake output module imported at all, and a `provides.plainchild` child is reclassified as a nested key in the same run. - Measured the three key positions separately, because they do not share a verdict: a root aspect named `apps` classifies its own content normally (`classKeys = [ "nixos" ]`), a nested key named `apps` is read as class content (`classKeys = [ "apps" ]`) and is correctly reserved, and a `provides` child named `apps` is exempt by marker (`classKeys = [ ]`). Only the third is this bug. - `nix develop -c just fmt`: 577 files processed, 0 changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mkScopeIdto context values it can name, as an unnameable value falls through to the type placeholder branch and renders asden=<set:den>, so it carries no identity and only splits one logical scope across two ids depending on which module args happened to be bound on the path.den.hosts.<name>.includesarriving empty while the same aspect throughden.aspects.<name>.includesdelivers, because pipe emits are bucketed by scope id andassemble-pipesgives a consumer its own bucket, with no plain ancestor inheritance of raw emits outsidepolicyBoundAncestor.darwin.homebrew.casksthrough the same instance list arrived while one delivering acasksquirk did not.den.hosts.<name>spelling that no existing cell reached.Validation
nix develop -c just ci: 1236/1236, exit 0, read off stderr."host=igloo,inputs=<set:inputs>,lib=<set:lib>,user=tux"where it expects"host=igloo,user=tux", and its control cell stays green in that same run, so the cell measures the exclusion rather than a scope id that dropped every key.20d1e76, changing one token in one file:den.aspects.kocaeli.includesyields the tap,den.hosts.kocaeli.includesyields[ ], both at exit 0 with no error.host|systemwhile the user scope pushedden|host|inputs|lib|system|user, which is where the placeholder keys entered the identity.slackandspotifyreachhomebrew.casksthrough that instance list via a class key androotshelldoes not via a quirk key.