Skip to content

fix: provider sub-aspect functions receive parametric context#419

Merged
vic merged 2 commits intovic:mainfrom
sini:fix/provider-sub-aspect-function
Apr 9, 2026
Merged

fix: provider sub-aspect functions receive parametric context#419
vic merged 2 commits intovic:mainfrom
sini:fix/provider-sub-aspect-function

Conversation

@sini
Copy link
Copy Markdown
Collaborator

@sini sini commented Apr 9, 2026

Fixes #413

Problem

Provider sub-aspects defined as bare context functions don't receive { host } during resolution:

den.aspects.foo._.sub = { host, ... }: {
  nixos = lib.optionalAttrs (host.hostName != "whatever") {
    networking.networkmanager.enable = true;
  };
};

When a parent function returns { includes = [foo._.sub] }, the sub-aspect is resolved by the adapter system which only passes { class, aspect-chain } — the { host } context from the pipeline never reaches nested includes of function results.

Fix

applyDeep in parametric.applyIncludes — when takeFn succeeds and returns a bare result with sub-includes (no meta, no __functor), also apply takeFn to those sub-includes. This propagates context to provider sub-aspects nested inside function results without double-applying to parametric wrappers or deepRecurse outputs.

The key discriminator: bare provider results carry only includes (+ name from carryAttrs). Results from withOwn/withIdentity have meta; deferred deepRecurse wrappers have __functor. Neither should be re-resolved.

Test coverage

  • deadbugs/issue-413-provider-bare-function.nix — reproduces the original report
  • deadbugs/issue-413-provider-sub-aspect-function.nix — variant with lib.optionalAttrs guard
  • provides-parametric.nix — provider sub-aspects with parametric context in various configurations

cc: @kalyanoliveira for the report

@sini sini requested a review from vic April 9, 2026 20:51
kalyanoliveira and others added 2 commits April 9, 2026 13:58
When a parametric aspect includes a provider sub-aspect that is also
a bare function (e.g. den.aspects.foo._.sub = { host, ... }: { ... }),
the sub-aspect's function was never called with the parametric context.

Root cause: providerFnType.merge wraps bare functions as
{ __functor = _: fn; }, placing them in the aspect's __functor rather
than includes. When the parent aspect's function returns
{ includes = [foo._.sub]; }, the sub-aspect appears as a leaf in
deepRecurse's mapIncludes and reaches resolve uncalled.

Fix: applyDeep in parametric.applyIncludes resolves sub-includes of
successful takeFn results one level deep. When take.atLeast calls
a provider function that returns { includes = [sub-aspect]; }, the
nested sub-aspect also receives parametric context before entering
the static pipeline.

Guards prevent double-resolution: results carrying meta (from
withIdentity/withOwn) or __functor (deferred deepRecurse wrappers)
are returned as-is since their includes are already processed.

Fixes vic#413
@sini sini force-pushed the fix/provider-sub-aspect-function branch from c66d7c8 to 0851ca1 Compare April 9, 2026 21:01
@vic vic added allow-ci allow all CI integration tests merge-approved Approved, merge yourself when ready labels Apr 9, 2026
@vic vic merged commit afbe47f into vic:main Apr 9, 2026
30 of 48 checks passed
@vic
Copy link
Copy Markdown
Owner

vic commented Apr 9, 2026

Thanks @kalyanoliveira and @sini

sini added a commit to sini/den that referenced this pull request Apr 10, 2026
PR vic#419 introduced applyDeep to propagate parametric context into
bare-result sub-includes. But it called takeFn unconditionally on every
sub, including full static aspects whose default functor (withOwn
parametric.atLeast) discards owned class configs in a non-static branch.
Result: `den.aspects.role._.sub.nixos.x = true` was silently dropped
when role was a parametric parent that included its sub-aspect.

Gate the inner re-application on canTake.upTo: a static aspect's
default functor has empty functionArgs, so upTo is false and the sub is
left alone for the static resolve pass. User-provided provider fns
(e.g. `{ host, ... }: { nixos = ...; }`) have host in functionArgs,
so upTo fires and their config is materialized. This preserves the
vic#419 fix while restoring pre-vic#419 behavior for static sub-aspects.

Fixes vic#423.
sini added a commit to sini/den that referenced this pull request Apr 10, 2026
PR vic#419 introduced applyDeep to propagate parametric context into
bare-result sub-includes. But it called takeFn unconditionally on every
sub, including full static aspects whose default functor (withOwn
parametric.atLeast) discards owned class configs in a non-static branch.
Result: `den.aspects.role._.sub.nixos.x = true` was silently dropped
when role was a parametric parent that included its sub-aspect.

Gate the inner re-application on canTake.upTo: a static aspect's
default functor has empty functionArgs, so upTo is false and the sub is
left alone for the static resolve pass. User-provided provider fns
(e.g. `{ host, ... }: { nixos = ...; }`) have host in functionArgs,
so upTo fires and their config is materialized. This preserves the

Fixes vic#423.
sini added a commit that referenced this pull request Apr 10, 2026
)

PR #419 introduced applyDeep to propagate parametric context into
bare-result sub-includes. But it called takeFn unconditionally on every
sub, including full static aspects whose default functor (withOwn
parametric.atLeast) discards owned class configs in a non-static branch.
Result: `den.aspects.role._.sub.nixos.x = true` was silently dropped
when role was a parametric parent that included its sub-aspect.

Gate the inner re-application on canTake.upTo: a static aspect's default
functor has empty functionArgs, so upTo is false and the sub is left
alone for the static resolve pass. User-provided provider fns (e.g. `{
host, ... }: { nixos = ...; }`) have host in functionArgs, so upTo fires
and their config is materialized. This preserves the #419 fix while
restoring pre-#419 behavior for static sub-aspects.

Fixes #423.
@sini sini deleted the fix/provider-sub-aspect-function branch April 14, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted allow-ci allow all CI integration tests merge-approved Approved, merge yourself when ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants