Skip to content

fix: backport aspect-pipeline fixes from entity-gen-schema-port#589

Merged
sini merged 10 commits into
denful:mainfrom
sini:backport/aspect-pipeline-fixes
Jun 3, 2026
Merged

fix: backport aspect-pipeline fixes from entity-gen-schema-port#589
sini merged 10 commits into
denful:mainfrom
sini:backport/aspect-pipeline-fixes

Conversation

@sini
Copy link
Copy Markdown
Collaborator

@sini sini commented Jun 2, 2026

Backports the standalone aspect-pipeline bug fixes that accumulated on feat/entity-gen-schema-port so they can land on main independently of the gen-schema entity port (which is still in flight) and the den-diagram extraction (tracked in its own PR).

Most commits carry a (cherry picked from commit ...) trailer back to the source branch. One commit (e711eebe) is net-new — authored directly on this branch with no cherry-pick origin (see Net-new below).

Generalized node-spawn + resolve-at-emitting-node (c57ac6b8)

A general primitive — spawnNode — materializes a child resolution node from any parent scope, threaded with the parent pipeline's resolved scope-tree state (parent + siblings), so the node's own assemblePipes re-derives inherited/collected pipe values with full fleet visibility. Paired with resolve-at-emitting-node: a pipeline-parametric pipe emit ({ <arg>, ... }: …) resolves to concrete data at its emitting node on every crossing (local / collected / exposed), never as a function; config-dependent emits stay deferred (__configThunk).

The mechanism is general — den-hoag spawn: a node with one read-only inherited edge, applicable to any parent→child entity relationship (fleet→host, environment→host, host→user, host→home). Home extraction is the first and driving consumer: it replaces the three isolated home-extraction sub-pipelines (host-aspects resolveImports; makeHomeEnv / hm-host resolveEntity) with spawnNode, fixing the originating bug where a host-aspects-projected ssh app saw only the local host, never the fleet peers.

This supersedes the home-env chainCtx workaround (eea3d6b1, under Fixes): that commit threaded ambient resolution-chain context into the isolated extraction as a stopgap; this change removes it — environment and fleet-collected pipe data now arrive structurally via the threaded scope-tree state.

  • assemble-pipes: resolve-at-emitting-node on the collected and exposed crossings; config-dependent emits stay deferred.
  • spawn-node.nix: the spawnNode primitive (re-walk for one class, merge parent state, own assemblePipes, class isolation).
  • policy.spawn effect + register-spawn handler + drain augmentation: a deferred node spawn resolved post-walk.
  • route/apply.nix: forward source resolves via spawnNode (from = parent scope).
  • Bundles the scopeParent-walk pipe inheritance (pure-consumer scopes inherit a pipe's assembled value from the nearest policy-bound ancestor).

Fixes

  • hasAspect — nested refs via __provider chain (5a3bf908): hasAspect resolves nested aspect refs through the provider chain.

  • hasAspect — nested freeform refs at any depth (aa11ce89): supports nested freeform aspect references regardless of depth, with provenance kept distinct.

  • deep-merge nested namespace children across files (ca4569af): colliding nested namespaces declared across multiple files now deep-merge instead of last-write-wins.

  • navigated nested aspects get their own identity (beb2b493): nested includes dedup correctly across scopes instead of sharing identity.

  • home-env resolution-chain ctx (eea3d6b1): threads the resolution-chain context into home-manager extraction so parametric host quirks survive home projection. Superseded by the node-spawn unification above (c57ac6b8), which removes the chainCtx workaround in favor of spawnNode's threaded scope-tree state.

  • policy dispatch — runtime-include policies scoped to their subtree (439c3dc5): the late-sibling dispatch re-fired a sibling's own-include policies at every other sibling (entity-kind filter only), so a user opting into a battery — or any {host,user} policy a user registered — leaked to every other user on the host. Eligibility is now ancestor-or-self: a user's runtime includes stay in its subtree; host-registered provides still fan to all users. Fixes host-aspects projecting onto non-opt-in users (pre-existing leak).

Net-new

  • namespaces: _ provides bundle on namespace roots (e711eebe): a namespace root now exposes _ like an aspect, so [ foo._ ] bundles the namespace's root-level aspects — the container-level analog of den.aspects.foo._. The bundle is a flat map over the container's direct keys (structural keys stages/schema/classes/_module/_ excluded); it does not recurse to enumerate nested aspects. Each root-level aspect is included whole, so its nested children still arrive — via normal include resolution, not via _ descending — and the aspect-leaf _ (e.g. foo.app._) remains a distinct, separately-computed bundle. Previously foo._ threw attribute '_' missing because a namespace root is a container, not an aspect. Reported in discussion Issue regarding usage of namespace aspects with ._ #588. Not a backport (no upstream commit); landed here per maintainer call.
  • namespaces: don't round-trip the synthetic _ on re-import (95b6277e): follow-up fix found in code review — the computed _ was serialized into exported denful and collided with the read-only option when a re-imported namespace forced ._. Now stripped on export and import.

Tests

  • issue-583 mock fix (a2cb8708): stops the issue-583 forwarding-overwrite test mock from colliding with nixpkgs programs.atuin.flags.
  • issue-588 regression suite (e711eebe): 5 tests covering the namespace-root _ bundle, the den.aspects.foo._ baseline, explicit-list equivalence, aspect-leaf ns.app._, and structural-key exclusion.
  • home-extraction suite (c57ac6b8): 8 tests — all-peers (collected, host-aspects projection), resolved-users (exposed), config-thunk deferral, in-tree ≡ threaded equivalency, server-host membership, and a complex-forward source-fallback guard.

Excluded

  • gen-schema entity port (separate PR).
  • den-diagram / den-gram extraction (separate PR).
  • chore: update flake.lock commits (bump gen-schema/den-diagram inputs not present on main).

Verification

  • Full CI: 863/863 passing (was 853/853; +8 from the new home-extraction suite).
  • Affected suites isolated: home-extraction 8/8, host-aspects 10/10, host-aspects-chain-ctx 1/1 (parametric host quirk survives projection via the threaded scope-tree state, with chainCtx removed), pipe-scope 16/16, pipes 9/9, deadbugs 23/23, has-aspect 35/35.
  • just fmt clean.

sini added 2 commits June 2, 2026 11:42
Nested aspects from freeform traversal (e.g., den.aspects.disk.zfs-disk-single)
have __provider set by aspectContentType.merge but lack name/meta. Use __provider
to derive the path key, matching how the pathSet stores these entries.

(cherry picked from commit 5a3bf90)
- has-aspect.nix: accept refs with __provider (set by aspectContentType)
- types.nix: annotate nested attrset children in content merger with
  __provider so deeply nested aspects carry provenance
- Only annotate unregistered keys (skip class/pipe/structural keys)
- Tests: nested present/absent, provenance distinct, deeply nested (3 levels)

(cherry picked from commit aa11ce8)
@sini sini requested a review from vic as a code owner June 2, 2026 18:49
@github-actions github-actions Bot added the allow-ci allow all CI integration tests label Jun 2, 2026
sini added 4 commits June 2, 2026 11:52
aspectContentType's multi-def branch forwarded sub-keys with a shallow
`//`, so when several files each contribute a different child under the
same deeply-nested namespace, all but the last were dropped from
navigation (e.g. services/network/cilium/{cilium,hubble-ui,
cilium-bgp-resources}.nix all defining children of network.cilium).

Deep-merge instead: colliding attrsets recurse and colliding lists
concatenate (matching den's own merge semantics); scalars keep
last-def-wins, with __contentValues remaining the canonical source for
emit/forward collection. Adds a deadbugs regression test.

Full suite 860/861; the one failure (issue-583) is pre-existing
nixpkgs drift (nixpkgs now declares programs.atuin.flags, colliding with
the test's mock module) and fails identically at HEAD.

(cherry picked from commit ca4569a)
wrapChild only injected identity from __provider for content wrappers
carrying __contentValues; a single-def navigated nested aspect carries
__provider (its full path) but no __contentValues, so it fell through
nameless and children.nix renamed it to <parent>/<anon>:<idx>.

That gave the same nested aspect a different identity depending on the
inclusion path — apps.gaming.steam reached via roles.gaming (host scope)
vs. a per-user entity-named aspect's includes (applied by a policy at
user scope) — defeating cross-scope dedup, so its nixos content
(programs.steam.package) was defined twice.

Derive name + meta.provider from __provider whenever a navigated child
has no name. Adds a cross-scope dedup regression test. Full suite
861/862 (only the pre-existing nixpkgs issue-583).

(cherry picked from commit beb2b49)
…tuin.flags

The issue-583 forwarding test mocked options.programs.atuin.flags. nixpkgs
gained that option in rev 64c08a7 (CI lock bumped in 4701e77), so when the
denful#583 fix landed on this branch the mock redeclared an option nixpkgs already
owns -> "option programs.atuin.flags is already declared" -> the test failed
(it passed in the PR's original, older-nixpkgs context).

Forward into a custom `forwardTarget` option instead, so the mock can't
collide with nixpkgs. Behaviour and intent unchanged; full suite now 862/862.

(cherry picked from commit a2cb870)
The host-aspects battery re-resolved the host aspect tree for a user's
classes (homeManager) in an isolated sub-pipeline seeded with only
{ host, user }, dropping the ancestor context the host scope actually
carries (e.g. a parent `environment` entity). A parametric host quirk
emit `{ environment, host, ... }: ...` was then stranded as a raw
function at the {host,user} projection scope, crashing any homeManager
consumer that read the pipe ("expected a set but found a function").

from-host now fires as a policy (receiving the full resolveCtx) and
threads the ambient entity-kind chain bindings into the re-resolution,
so re-fired parametric host aspects bind the same args they would at the
host scope. The same threading is applied to home-env's userForward
extraction path.

Adds deadbugs/host-aspects-chain-ctx regression test.

(cherry picked from commit eea3d6b)
@sini sini force-pushed the backport/aspect-pipeline-fixes branch from 02c444f to bb6482c Compare June 2, 2026 18:52
@sini sini requested review from drupol and theutz June 2, 2026 18:54
`_` (alias for `provides`) lived only on aspect leaves, so
`den.aspects.foo._` worked but `foo._` — where `foo` is a namespace
root — threw `attribute '_' missing`. A namespace root is a container,
not an aspect, so it has no provides of its own.

Add a synthetic, read-only `_` to the namespace container: an aggregate
aspect whose includes are every aspect declared in the namespace, so
`[ ns._ ]` pulls them all in — the container-level analog of an aspect's
provides bundle. Structural keys (stages/schema/classes/_module/_) are
excluded, and aspect-schema's class collection skips `_`.

Reported in denful#588.
Comment thread templates/ci/modules/features/has-aspect.nix
Comment thread templates/ci/modules/features/deadbugs/host-aspects-chain-ctx.nix
sini added 2 commits June 2, 2026 15:32
…ce re-import

The namespace-root `_` provides bundle is a computed, read-only option. It was
being serialized into the exported `flake.denful.<ns>` and fed straight back as
a definition on re-import (`den.namespace name [sources]`), colliding with the
read-only option: forcing a re-imported `<ns>._` threw "The option
'den.ful.<ns>._' is read-only, but it's set multiple times". Existing
namespace-provider tests passed only because they never force `._`.

Drop `_` from the exported namespace and strip it in stripAliases on import —
mirroring how stripAliases already drops the aspect-level `_` aliases — so the
importing side recomputes its own bundle.

Found in code review of denful#589 (the namespace-root `_` feature it introduced).
…tting-node

A general primitive — `spawnNode` — materializes a child resolution node from any
parent scope, threaded with the parent pipeline's resolved scope-tree state
(parent + siblings), so the node's own assemblePipes re-derives inherited/
collected pipe values with full fleet visibility. Paired with resolve-at-emitting-
node: a pipeline-parametric pipe emit resolves to concrete data at its emitting
node on every crossing (local/collected/exposed), never as a function;
config-dependent emits stay deferred (__configThunk).

Home extraction is the first and driving consumer: it replaces three isolated
sub-pipelines (host-aspects resolveImports; makeHomeEnv/hm-host resolveEntity)
with spawnNode, fixing the originating bug where a host-aspects-projected ssh app
saw only the local host, never the fleet peers. The mechanism is general
(den-hoag `spawn` with one read-only inherited edge) and applies to any
parent->child entity relationship; home is just where it is currently exercised.

- assemble-pipes: resolve-at-emitting-node on the collected and exposed crossings.
- spawn-node: spawnNode primitive (re-walk for one class, merge parent state,
  own assemblePipes, class isolation).
- policy.spawn effect + register-spawn handler + drain augmentation: a deferred
  node spawn resolved post-walk.
- route: forward source resolves via spawnNode (from = parent scope); drop the
  chainCtx workaround in home-env.nix.
- Includes the scopeParent-walk pipe inheritance keeper.
- Tests: all-peers, resolved-users, in-tree==threaded equivalency, server-host
  membership. Full CI 870/870.

(cherry picked from commit 7875506)
@sini sini force-pushed the backport/aspect-pipeline-fixes branch from 3c11bb2 to c57ac6b Compare June 2, 2026 23:25
The late-sibling dispatch re-fired every policy registered at the parent OR
any sibling scope at every sibling (entity-kind filter only). So a policy a
user registered via its own includes — opting into the host-aspects battery,
a per-user `to-users` policy, etc. — fanned to every other user on the host,
regardless of opt-in.

Make eligibility ancestor-or-self: at each sibling, only policies registered
at the parent (the host, whose subtree spans every user) plus the sibling's
own fire. A user's runtime includes stay in its own subtree; host-registered
provides still fan to all users (the legacy mutual-provider pattern).

Fixes host-aspects projecting a host's homeManager onto users who never
included the battery (a pre-existing leak, not from the spawnNode work).
host-aspects.nix is unchanged — this is a dispatch fix, not a per-battery guard.

Tests: host-aspects-sibling-leak (regression guard); user-host-mutual-config
re-patterns the user->siblings case to host-level registration and adds
test-user-include-stays-in-subtree.

(cherry picked from commit 4200f37)
@sini sini merged commit dd31602 into denful:main Jun 3, 2026
40 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allow-ci allow all CI integration tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants