fix(executors): keep the inherited launch paths at every prepend site

[zengleixin-amd]

• Description: Magpie and the bypass launcher start the benchmark server with os.environ overlaid by the config's benchmark.envs, so a config path-list value replaces the inherited one. Every site that prepended to PYTHONPATH, PATH or LD_LIBRARY_PATH built the new value from the config key alone, so a config without the key handed the server only the new entry and dropped the operator's and the image's entries.

  The profile flow is where this actually happens today: in 1,835 recent session configs only roofline configs set PYTHONPATH (89 of 93: 71 shim only, 16 shape tool + shim), and a GLM-5.3 profile server imported the shared /sgl-workspace sglang instead of the operator's private sglang/aiter trees used by baseline and candidates. The grid and reference sites have the same defect but are latent (not exercised by current sessions).

  Fix: one helper, prepend_launch_path(envs, var, *entries) in _workload_envs.py, owns the rule: build on the config value when the key is present, otherwise on the inherited value; put the new entries first; de-duplicate; drop empty parts; join with os.pathsep. Every prepend site uses it:

  • profile flow: the TraceLens kernel_shape_tool (SGLang) and the profile start-up shim (ProfileExecutor._inject_host_probe); the shim now moves to the front even when already present, since Python imports only the first sitecustomize.
  • reference overlay (--reference-script).
  • grid: apply_runtime_override (pythonpath_prefix, pythonpath_prefixes, path_prefix, entrypoint_bin_dir, ld_library_path_prefix) and the variant overlay_pythonpath.

  _prepend_path_entry is deleted, so PATH no longer collapses to a single directory and LD_LIBRARY_PATH keeps /opt/rocm/lib when the config lacks the key. Each site's validation and warn-and-drop behaviour is unchanged; only the composition moved.

• Linked issue(s): none (reported in an internal AgentX single-node defect note, item D3; grid/reference sites added at review request).

• Tests: reproduction tests written first, failing on main and passing here, one per site: the inherited value survives when the config lacks the key (shape tool <tool>:/a:/b, full profile chain <shim>:<tool>:/a:/b, reference overlay, each runtime-override field, variant overlay); a config value stays authoritative (<shim>:/c); a present entry moves to the front; re-arming is idempotent. Tests that asserted the old replace behaviour carry their contract across with the environment pinned explicitly. Related suites run locally on Windows; the failure set is identical to main (Windows path separator / fcntl). CI is the authoritative Linux run.

• Size/complexity triggers crossed: none; 3 commits, 4 non-test files. No touched unit's cyclomatic complexity rises (materialize_config_with_envs 111, apply_runtime_override 17, _build_variant_yaml 19).

• If this simplifies or refactors: removes _prepend_path_entry and the hand-written joins at each site; one helper now owns "prepend to a launch path-list env".

• Observable effect: profile/roofline servers import the same trees as baseline and candidates; attempt runtimes, overlays and reference overlays keep the operator's and the image's PYTHONPATH, PATH and LD_LIBRARY_PATH entries instead of silently replacing them. A prefix entry already present on the path now moves to the front instead of staying in place.

• Breaking changes: no

• PR addresses single concern: yes

• Root cause is upstream: no. Not touched: the xdit PATH guard in _build_variant_yaml uses the same config-only read, but the shipped xdit YAMLs pin PATH, so it is latent.

🤖 Generated with Claude Code

[xiaofei-zheng]

PR #1810 -- fix(profile): keep the inherited PYTHONPATH on profile servers

What it does: Magpie launches the benchmark server with os.environ overlaid by benchmark.envs, so a
config PYTHONPATH replaces the inherited one rather than extending it. Two profile-flow sites built the
new value from the config key alone -- the SGLang kernel_shape_tool branch in
materialize_config_with_envs and ProfileExecutor._inject_host_probe -- so a config without the key
dropped the operator's and the image's import paths from the profile server. The PR adds
prepend_launch_pythonpath() in _workload_envs.py, which builds on the config value when the key is
present and on the inherited value otherwise, de-duplicates and drops empty parts, and routes both sites
through it; the start-up shim now also moves to the front when already present, since Python imports only
the first sitecustomize.

Blocking issues: none

Checked: _workload_envs.prepend_launch_pythonpath and its two call sites
(_workload_envs.py:1812, profile.py:874); the overlay premise at common/env_safety.py:436-441
(build_benchmark_env) and _grid_runner.py:654 (_run_magpie env = scrub_benchmark_process_env(os.environ.copy()),
PYTHONPATH not in BLOCKED_CHILD_ENV_NAMES); assets/host_probe/sitecustomize.py
(_chain_preexisting_sitecustomize chains to the next sitecustomize on sys.path, so moving the shim
first does not shadow kernel_shape_tool); ordering in profile.py:_after_materialize_config, which runs
the injection strictly after materialization, giving shim:tool:inherited; the sibling prepend sites left
untouched (_apply_vllm_source_runtime:759, _workload_envs.py:1878, _grid_runner.py:353/362/498,
multi_node/scripts/sglang_shape_gate.py:48), all named as follow-up in the description and none on the
profile route.
Ran: prepend_launch_pythonpath called directly over 7 shapes (key absent, /c, "", None, a path
already containing the entry plus an empty part, whitespace-padded parts, two entries) plus a second call to
check re-arming; pytest -k "keeps_the_inherited or does_not_append_the_inherited or moves_a_present_shim"
on test_framework_rewrite_evidence.py and -k shape_tool on test_workload_envs_branches_unit.py at both
head and merge base -- test_probe_injection_keeps_the_inherited_pythonpath,
test_probe_injection_moves_a_present_shim_to_the_front and
test_profile_shape_tool_keeps_the_inherited_pythonpath fail at the base and pass at the head. The one
unrelated failure in that file (test_probe_injection_prepends_to_an_existing_pythonpath, split(":"))
reproduces at the merge base and is a pre-existing Windows-only path-separator assumption.
CI: green at the current head across the full 6-shard x py3.10/3.11 matrix, coverage, ruff, pylint, mypy,
bandit, CodeQL and docs; mergeable: MERGEABLE.
Base: 880c167 | Head: 5bef0d5

LGTM

[xiaofei-zheng]

Follow-up request: please fold the remaining launch-PYTHONPATH prepend sites into this PR rather than leaving them for later. They are the same shape the PR just fixed -- the config key absent means envs[...] is written with the new entry alone, and Magpie's overlay then drops the inherited value -- and prepend_launch_pythonpath already exists in this branch, so each is a one-line call:

• _workload_envs.py:1878 -- reference overlay: envs["PYTHONPATH"] = overlay + (f":{envs['PYTHONPATH']}" if envs.get("PYTHONPATH") else "")
• _grid_runner.py:353 -- apply_runtime_override, pythonpath_prefix
• _grid_runner.py:362 -- apply_runtime_override, pythonpath_prefixes
• _grid_runner.py:498 -- variant overlay_pythonpath

Three of these also hardcode ":" instead of os.pathsep, which the helper handles.

Same defect class, one level up: _grid_runner._prepend_path_entry (:325) reads only envs.get(var), so an apply_runtime_override carrying path_prefix or entrypoint_bin_dir against a config with no PATH key writes PATH as that single directory, and ld_library_path_prefix does the same to LD_LIBRARY_PATH. That is a harder failure than the PYTHONPATH one -- it is consistent with these paths not being exercised by current sessions, but it should not stay latent. Either give it the same inherited-value fallback or state in the body why PATH is guaranteed present there.

Scope stays at the two non-test files this PR already touches. Each site wants a test that fails without the change, the way the two fixed sites now have.

[zengleixin-amd]

Thanks, folded in as two commits on top of the reviewed one (rebased onto current main):

• 1b85d7451 generalizes the helper to prepend_launch_path(envs, var, *entries) (same semantics, any path-list var) and uses it for the reference overlay; the shape tool and the profile shim call it too, and prepend_launch_pythonpath is gone.
• af9d90994 routes every grid prepend site through it: apply_runtime_override (pythonpath_prefix, pythonpath_prefixes, path_prefix, entrypoint_bin_dir, ld_library_path_prefix) and the variant overlay_pythonpath. _prepend_path_entry is deleted, so PATH / LD_LIBRARY_PATH now build on the inherited value when the config lacks the key (no more single-directory PATH, /opt/rocm/lib kept). All sites join with os.pathsep.

Validation and warn-and-drop behaviour at each site are unchanged; only the composition moved. One visible change, stated in the commit body: a prefix entry already present on the path now moves to the front instead of staying in place.

Each site has a test that fails without the change (inherited value lost) and passes with it, plus a test that a config value stays authoritative. Cyclomatic complexity of every touched unit is unchanged (materialize_config_with_envs 111, apply_runtime_override 17, _build_variant_yaml 19).

Not touched, as it was not in the list: the xdit PATH guard in _build_variant_yaml has the same config-only read; the shipped xdit YAMLs pin PATH, so it is latent. Happy to fold it in too if you prefer.

[xiaofei-zheng]

Re-review at head af9d9099477043a0a99872f68e601ccbbb2244cd (two commits added since my LGTM, which is superseded).

What it does: the launch env overlay means a config value for PATH, PYTHONPATH or LD_LIBRARY_PATH replaces the inherited one, so every site that prepended using only the config value dropped the operator's and the image's entries. _workload_envs.prepend_launch_path(envs, var, *entries) now builds on the config value when the key is present and on the inherited value otherwise, de-duplicates, drops empty parts and moves a present entry to the front. Seven sites route through it: the SGLang kernel_shape_tool branch, the reference overlay, the profile start-up shim, the four prepends in apply_runtime_override, and the variant overlay_pythonpath. _grid_runner._prepend_path_entry is deleted.

The new code is correct as far as I can check it: the helper's generalisation to var is behaviour-preserving for PYTHONPATH, the call sites are mechanical substitutions, validate_overlay_pythonpath returns a single directory so passing it as one entry is right, and the added tests pin both crossing cases (key absent -> inherited survives; key present -> inherited not appended) for all three variables.

Blocking issues: 1

1. [X2] The description still describes the first commit; at the current head it contradicts the diff in four places.
   Problem: body.txt at head af9d9099
   • "Size/complexity triggers crossed: none; 1 commit, 2 non-test files" -- the head has 3 commits, 3 non-test files and 4 test files.
   • "Other PYTHONPATH prepend sites (grid runtime overrides, variant/reference overlays, vLLM source) share the pattern but are not exercised by current sessions; left for a follow-up" -- grid runtime overrides and the variant/reference overlays are fixed in this PR now. Only _apply_vllm_source_runtime is left, and that one already reads os.environ as its fallback, so it does not share the defect.
   • The Fix paragraph names prepend_launch_pythonpath, which does not exist at this head; the symbol is prepend_launch_path(envs, var, *entries) and it now covers PATH and LD_LIBRARY_PATH, not only PYTHONPATH.
   • "Observable effect" names only the profile/roofline server's PYTHONPATH. Two further operator-visible effects are unstated: an attempt runtime_override now keeps the inherited PATH, PYTHONPATH and LD_LIBRARY_PATH instead of replacing them with the override entries alone; and a prefix already present on PATH is now moved to the front rather than left in place (_prepend_path_entry kept it where it was -- test_path_prefix_moves_a_present_entry_to_the_front pins the new behaviour).
     Impact: the release cut aggregates these descriptions, so the PATH/LD_LIBRARY_PATH half of the fix and the reordering are lost for good; a reader checking scope is told the grid sites are still broken when they are not.
     Action: rewrite the description against the head diff -- scope, the helper's real name and signature, the three variables it covers, the grid/overlay sites now included, the PATH reordering, and the added grid tests. Update the commit/file counts.

Checked: prepend_launch_path (_workload_envs.py:759) and all seven call sites; the removal of _grid_runner._prepend_path_entry with no surviving reference to it or to prepend_launch_pythonpath in the head tree; common/overlay.validate_overlay_pythonpath (rejects :, returns one directory); the launch overlay premise at common/env_safety.py:436-441 and _grid_runner.py:_run_magpie; assets/host_probe/sitecustomize.py chaining, which keeps the shim-first move safe.
Ran: prepend_launch_path called directly over the 7 input shapes the call sites produce, plus the reversed() multi-entry loop and the double-PATH-prepend sequence -- output matches the added tests exactly.
Not run: the suite at this head. My sandbox refused the worktree update after the first review, so the earlier base-vs-head run covers only the first commit (where 3 of the 5 new tests failed at the base). CI is green at af9d9099 across the full 6-shard x py3.10/3.11 matrix.
Base: e5f2be1 | Head: af9d909

[xiaofei-zheng]

Approving the code at head af9d9099477043a0a99872f68e601ccbbb2244cd. The description desync raised in my previous comment is being handled separately by the reviewer and is not a reason to hold the code.

Code conclusion: no blocking issues. prepend_launch_path has one definition and seven production call sites; no reference to the deleted _prepend_path_entry or to the earlier prepend_launch_pythonpath survives in the head tree; validate_overlay_pythonpath returns a single directory, so passing it as one entry is correct; the edits to the ten existing runtime-override test lines strengthen rather than weaken them (the multi-entry test now also pins that a config value is not appended to). Helper exercised directly over the seven input shapes the call sites produce, plus the reversed() multi-entry loop and the double-PATH-prepend sequence; output matches the added tests.

Not run: the suite at this head, locally. CI is green at af9d9099 across the full 6-shard x py3.10/3.11 matrix.