fix(runner): make RunnerContext.get_env() read live from os.environ#885
fix(runner): make RunnerContext.get_env() read live from os.environ#885mergify[bot] merged 1 commit intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughRunnerContext now stores constructor-provided environment overrides in an internal Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/runners/ambient-runner/ambient_runner/platform/context.py`:
- Around line 31-33: The bridge is still reading stale merged environment via
direct access to self._context.environment; update any reads in the LangGraph
bridge (where it reads LANGGRAPH_URL, LANGGRAPH_GRAPH_ID, LANGSMITH_API_KEY from
self._context.environment) to use the context's get_env() method (or the helper
in ambient_runner.platform.utils) instead so runtime-updated env vars are
respected; search for occurrences of self._context.environment in the bridge (or
the symbols LANGGRAPH_URL/LANGGRAPH_GRAPH_ID/LANGSMITH_API_KEY) and replace
those direct dict reads with self._context.get_env().get(...) or the utils
helper.
- Around line 32-39: get_env currently assumes self._overrides exists which
fails for objects created via object.__new__; update get_env (in the
RunnerContext / ambient_runner.platform.context module) to first check for the
presence of _overrides (e.g. via hasattr(self, "_overrides")) and if absent fall
back to using self.environment for override lookup, or ensure _overrides is
initialized (empty dict) before use; specifically modify get_env to return
self._overrides[key] when _overrides exists, otherwise check self.environment
for the key and then return os.environ.get(key, default).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26a13632-9ea2-4df9-9a7c-ae0e5bd01483
📒 Files selected for processing (2)
components/runners/ambient-runner/ambient_runner/platform/context.pycomponents/runners/ambient-runner/tests/test_context.py
|
Can you fix unit tests |
RunnerContext.__post_init__ snapshots os.environ once at construction. After that, get_env() reads from the frozen dict. Multiple endpoints mutate os.environ at runtime (POST /workflow, POST /repos/add, auth refresh), but the context is never recreated — mark_dirty() resets the bridge while reusing the same stale context. Store explicit constructor overrides separately in _overrides and make get_env() check overrides first, then fall through to live os.environ. self.environment remains populated for backward compat. Made-with: Cursor
9ca461a to
935dd40
Compare
Pushed! Problem was: "The issue was that test_claude_auth.py has a _make_context() helper that uses object.new(RunnerContext) to bypass post_init, so _overrides was never set. The fix adds a getattr guard: when _overrides is absent (bypassed init), get_env() falls back to the old behavior of reading only from self.environment. When _overrides is present (normal construction), it checks overrides first, then reads live from os.environ." |
Merge Queue Status
This pull request spent 19 seconds in the queue, including 5 seconds running CI. Required conditions to merge
|
Summary
This is a followup PR to ensure that #726 data is pushed to Langfuse for product instrumentation
RunnerContext:get_env()was reading from a frozen dict captured once at construction. Runtime mutations toos.environ(byPOST /workflow,POST /repos/add, auth refresh) were invisible to any code usingcontext.get_env().__post_init__mergedos.environintoself.environmentandget_env()read only from that dict. The context is never recreated —mark_dirty()resets the bridge but reuses the same stale context._overrides.get_env()now checks overrides first, then falls through to liveos.environ.self.environmentremains populated for backward compatibility.Changes
ambient_runner/platform/context.py_overridesin__post_init__;get_env()reads live fromos.environwith overrides winningtests/test_context.pyTest plan
test_get_env_sees_os_environ_mutations_after_creation— creates context, mutatesos.environafter, assertsget_env()returns mutated valuetest_explicit_overrides_win_over_os_environ— constructor overrides take precedence even afteros.environmutationMade with Cursor