fix: Deferred resolution of Puppet-language functions (e.g. mocks in tests) failing due to lack of :global_scope

[griggi-ws]

Hey folks, found an issue with Deferred resolution of Puppet-language functions while testing a control-repo with Onceover that makes heavy use of some custom functions that are mocked in Onceover's config, and did some digging.

The error was:

NoMethodError:
       undefined method `with_global_scope' for {}:Hash

Puppet-language functions use the Named Closure, which explicitly looks up :global_scope and returns an empty Hash as a fallback.

The DeferredResolver creates a new compiler instance which includes :global_scope in its context_overrides, but the resolver's call to resolve_futures does not use them.

This means when a Puppet-lang Deferred function is resolved, enclosing_scope is {}, and when the Closure's invoke is called (expecting a Scope object with the method with_global_scope), it errors out since we have an empty Hash instead of a Scope.

By wrapping the resolve_futures call in a compiler.with_context_overrides block, :global_scope is a valid instance of Scope, and the function can be resolved.

I added a spec test for Deferred processing of Puppet-lang functions, validated the failure, and applied the fix.

[binford2k]

I went back through the original PR to add deferred functions and I believe this is the correct fix.

@griggi-ws apologies for the delay. Can you rebase this to current please?

[binford2k]

@hlindberg could you give this a 👍 before we merge it?

@griggi-ws would be great to get your commits signed. Apologies for not noticing it the other day. https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification

[hlindberg]

@binford2k I think fix is correct. Think this issue was missed since puppet language functions aren't plugin synced. Good catch!

[griggi-ws]

@griggi-ws would be great to get your commits signed. Apologies for not noticing it the other day.

Ah yeah can do, signature was there when you last looked, just lost em because I used the interface rebase button

[silug]

I asked Claude to review this PR. Here is its feedback:

Verdict: the fix is correct, minimal, and well-diagnosed — I'd approve it, with one gap worth raising: the same bug still exists on the lazy (preprocess_deferred = false) path, which I reproduced against the PR branch.

What the PR does

When DeferredResolver.resolve_and_replace resolves Deferred values, it builds a ScriptCompiler but never activated that compiler's context overrides. Puppet-language functions are Named closures, whose enclosing_scope does Puppet.lookup(:global_scope) { {} } — so without the override they get an empty Hash instead of a Scope, and Closure#invoke crashes with undefined method 'with_global_scope' for an instance of Hash. The fix wraps resolver.resolve_futures(catalog) in compiler.with_context_overrides, which pushes :global_scope, :current_environment, and :loaders for the duration of resolution. A regression test compiles a catalog against a real on-disk module with a .pp function and resolves it.

What I verified locally

I checked out the PR into a temporary worktree and ran the suite:

• spec/unit/pops/evaluator/deferred_resolver_spec.rb — 8 examples, 0 failures on the PR branch.
• With the lib change reverted, the new test fails with exactly the reported NoMethodError, so the regression test genuinely covers the bug.
• The author's root-cause analysis in the PR description matches the code: ScriptCompiler#context_overrides does include :global_scope, and this mirrors how ScriptCompiler#compile and Compiler#compile already wrap their own work in Puppet.override(@context_overrides, ...). The fix is idiomatic for this codebase.

The gap: lazy resolution is still broken

The fix only covers eager resolution (preprocess_deferred = true, the default). When preprocess_deferred = false, resolve_future doesn't call the function inside the new block — it wraps the call in a DeferredValue proc that runs later, at catalog-application time (lib/puppet/transaction.rb:476, lib/puppet/type/exec.rb:603), long after with_context_overrides has popped. I wrote a quick spec on the PR branch simulating that (resolve with preprocess = false, then call .resolve on the resulting DeferredValue with no compiler context active), and it fails with the identical undefined method 'with_global_scope' for an instance of Hash.

The natural extension is to capture the override inside the proc in resolve_future:

DeferredValue.new(
  proc {
    resolved_arguments = mapped_arguments.map { |arg| resolve_lazy_args(arg) }
    @compiler.with_context_overrides do
      @scope.call_function(func_name, resolved_arguments)
    end
  }
)

@compiler is already an instance variable on the resolver and both compiler classes define with_context_overrides, so this is a two-line change plus a preprocess = false variant of the new test. This is arguably a pre-existing sibling bug rather than a defect introduced by the PR, so it'd be reasonable to either fold it in here or file it as a follow-up issue — but since the setting is documented and the failure mode is identical, it's worth deciding explicitly rather than leaving it latent.

Minor notes (none blocking)

• The rewritten environment let now builds a temp module directory for every test in the file, not just the new one. Harmless (the suite runs in ~0.15s), but scoping the module-backed environment to the new example would be slightly cleaner.
• with_context_overrides is called with the default empty description; passing something like "For resolving deferred values" would help anyone debugging context stacks. Cosmetic.
• Side effects of the override are benign: it also pushes :loaders, :current_environment, and :rich_data => true during resolution, but those come from the same ScriptCompiler the resolver already uses for scope and function lookup, so this makes resolution more consistent, not less. The other entry point (DeferredResolver.resolve, used by functions/call.rb) runs inside an active compilation context already and is unaffected.
• No security or performance concerns — it's one Puppet.override push/pop per catalog resolution.