fix(ephemerals): don't cache empty ephemerals

[Aphosis]

Closes #2173

Ephemerals were initially exposed to package commands as an ephemerals binding.

Since dcf77a2, they are also exposed to late-bound functions using the same strategy: reusing the existing list of resolved ephemerals.

The issue lies in the difference in timing between late-bound functions and package commands:

• Package commands are executed after the context have been resolved, so we get a properly resolved list of ephemerals
• Late-bound functions are executed during a solve, which means we don't have access to the resolved ephemerals yet

The previous implementation did not crash when accessing ephemerals in late-bound functions: instead, it defaulted to an empty list.

This behavior leads to a second issue: ephemerals were added and cached as part of the list of pre-resolve bindings.

This can lead to the following scenario:

• A package late-bound function accesses the ephemerals binding
• Since the context is not resolve yet, no resolved ephemerals is found and an empty list is returned and cached
• Later, in the package commands, we also access ephemerals
• The cached empty list is returned, meaning that most queries will likely fail

This commit addresses these issues the following way:

• Move ephemerals out of pre-resolve bindings for commands, so that they are always retrieved from the resolved context
• For late-bound functions, use resolved ephemerals only if a context is available

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: Aphosis / name: Aphosis (6707d24)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.42%. Comparing base (5c598c5) to head (52ba3b0).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2174      +/-   ##
==========================================
+ Coverage   61.29%   61.42%   +0.12%     
==========================================
  Files         164      164              
  Lines       20568    20572       +4     
  Branches     3575     3575              
==========================================
+ Hits        12607    12636      +29     
+ Misses       7089     7069      -20     
+ Partials      872      867       -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Aphosis]

I didn't add any test yet because I'm unsure where it should be added in the test suite, and how the test package should be created.

A good candidate seemed to be in the TestContext class:

rez/src/rez/tests/test_context.py

Line 23 in aa10016

class TestContext(TestBase, TempdirMixin):

Also, the test requires a package with specific attributes, and I'm not sure if the package should be created inline in the test or stored somewhere under data/tests.

[maxnbk]

A good candidate seemed to be in the TestContext class:

I would agree. The bug is about ephemerals being empty inside commands() during a ResolvedContext execution, which makes it a context execution test, rather than a rex binding test or a package attribute test. The existing methods in that class already test patterns like "resolve a context, execute commands, check the environ", which is exactly what needs doing to test this.

Also, the test requires a package with specific attributes, and I'm not sure if the package should be created inline in the test or stored somewhere under data/tests.

IMHO ( @JeanChristopheMorinPerso might disagree), an inline-package created into a tempdir, resolving against it, and checking the environ is preferable to a fixture under data/tests because the pkgdef is right in the test, we can easily see what late() / commands() are set to, and what is being checked without digging into data/tests. The existing late_binding fixture is for testing package-attribute-access, not context-execution, so adding it there would probably blur the line between the package resource tests (test_packages.py) and context execution tests (test_context.py). The test also requires the specific ephemeral request, and so separating the pkgdef from the test would make the relationship between the two less clear.

A few specifics to help guide you to a working test that tests what you want it to test:

• Use self.root for the temp directory (TempdirMixin is being used so it's a managed temp dir that gets clean up automatically)
• Use get_environ() and not execute_command() to avoid annoying subprocess plumbing or parent_environ e.g. the way that text_execute_command_testing_environ does it.
• Assert both the truthiness of r.resolved_ephemerals and that the environ flag is "1" to catch both the case where the text fixture is wrong and no ephemeral resolves, and the actual bug itself
• Comment that the late function is there to trigger the bug, not to test the requires behavior.

I would also advise a second test that goes without the late-function as a control, which should also see the ephemeral, but is what proves the fix doesn't break the normal path while demonstrating that the late function was the trigger for the bug.

[maxnbk]

I am just marking this "Request Changes" so that review can be re-requested when updates are made, so that I'll see it again.

[JeanChristopheMorinPerso]

IMHO ( @JeanChristopheMorinPerso might disagree), an inline-package created into a tempdir, resolving against it, and checking the environ is preferable to a fixture under data/tests because the pkgdef is right in the test

FYI @maxnbk I'm usually in the camp of "no test data file and everything must be inline with the test to easily see what's being tested". Though, I know that this can sometimes incur some costs, but I don't think there's any costs involved here.

[Aphosis]

Hi, and thanks for your answers! I'm resuming work on this and just added the test case, is there anything else worth adding (CHANGELOG.md maybe, or is that an automatic process) ?

EDIT: sorry for the multiple force-push, I fixed a formatting issue, updated the tests to take your suggestions into account, and signed-off the commits.

[Copilot]

Pull request overview

This PR fixes a regression where the ephemerals binding could become permanently empty in commands() when a late-bound function caused ephemerals to be evaluated (and cached) before resolution completed. It adjusts how ephemerals is bound so command execution always sees the resolved ephemerals, and adds tests covering the regression scenario described in #2173.

Changes:

• Stop caching ephemerals as part of ResolvedContext pre-resolve bindings; bind it later from resolved_ephemerals.
• Ensure late-bound function evaluation gets an ephemerals object based on the context when available.
• Add regression tests ensuring commands() observes ephemerals both with and without a late function.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/rez/tests/test_context.py	Adds regression tests that validate ephemerals is visible in commands() even when late functions run during solve.
src/rez/resolved_context.py	Moves ephemerals out of cached pre-resolve bindings and binds it later from the resolved context.
src/rez/packages.py	Sets up ephemerals for late-bound function evaluation depending on whether a context is available.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.