fix(executors): honour --ep on single-node servers through the argv seal

[zengleixin-amd]

• Description: single-node --ep N never reached sglang or atom. The CLI only exported EP, the only single-node injection (inject_vllm_expert_parallel) handled vLLM, and the Magpie builtin launchers pass --tensor-parallel-size=$TP plus the extra args only. The server ran TP-only with no warning while provenance/KB recorded ep=N. Even for vLLM the flag was lost when a grid variant rebuilt the args with args_mode="replace" (KEEPs with remove_args), because the injection ran only on the materialize path. Pin detection also matched substrings, so --ep-num-redundant-experts counted as an EP pin.

  Fix: the server-argv seal owns the rule, because materialize, grid variants and argv repair all end there and it is the only consumer of EP for the argv.

  • framework_registry.EXPERT_PARALLEL_FLAGS: per framework, the flag to write and its aliases (sglang --ep-size N / --expert-parallel-size / --ep; vLLM --enable-expert-parallel / -ep; atom --enable-expert-parallel).
  • seal_server_argv reads the config's EP:
    • EP <= 1 is a no-op.
    • Pins are matched by whole token (--flag v / --flag=v). A pinned sglang size that contradicts EP raises ValueError; an agreeing pin is kept as is.
    • No flag on multi-node: the multi-node launcher passes EP itself.
    • No flag, with a warning, when EP does not divide the config's TP.
    • Otherwise the flag is appended.
  • Materialize carries EP into benchmark.envs through the existing workload-env loop next to TP. inject_vllm_expert_parallel and its call are deleted.
  • The --ep help text now states what is actually passed.

  Review follow-up (bc1bfe6f8): the multi-node and divisibility guards first lived in materialize, where the later --extra-env / extra_envs merge and grid variants' extra_envs could write EP back past them (e.g. --extra-env EP=3 at TP=8 still launched --ep-size 3). Both guards now sit in the seal, so every source of EP is checked; the contradicting-pin check runs before the divisibility skip so a pinned size cannot launch behind a misleading warning.

• Linked issue(s): none (reported in an internal AgentX single-node defect note, item D2).

• Tests: reproduction tests written first, failing on main (or on the pre-review head) and passing here:

  • sglang EP=4 yields --ep-size 4 exactly once; operator pins via each alias stay single; --ep-num-redundant-experts is not a pin; a recipe pinning --ep-size 4 with the default EP=1 is untouched.
  • a vLLM grid variant in replace mode keeps --enable-expert-parallel; reseal keeps the flag.
  • EP arriving through the operator pin, extra_envs, a grid variant's extra_envs and reseal, at TP=8/EP=3 and on multi-node, adds no flag.
  • a contradicting pin raises, including when EP does not divide TP.
    The deleted vLLM-injector tests' assertions moved into the seal/materialize tests. Related suites run locally on Windows; the failure set is identical to main (Windows-only causes, fcntl / path separators). CI is the authoritative Linux run. No golden snapshot changes.

• Size/complexity triggers crossed: none; 4 commits, 4 non-test files. materialize_config_with_envs stays at cyclomatic complexity 111; new helpers are <= 5.

• If this simplifies or refactors: removed the vLLM-only injector; the contract "vLLM gets --enable-expert-parallel when EP > 1, never duplicated" is preserved in the seal tests.

• Observable effect: single-node sglang/atom runs with --ep > 1 now start with expert parallelism; vLLM keeps it through replace-mode variants; an EP that the launched TP cannot hold, or any EP on multi-node, adds no flag regardless of where it was set; a contradictory --ep-size pin fails at materialize instead of silently winning. benchmark.envs now records EP.

• Breaking changes: yes, narrowly: server args pinning an sglang EP size that contradicts --ep now raise instead of launching with the pin.

• PR addresses single concern: yes

• Root cause is upstream: no (the Magpie launchers only promise TP; the broken promise was our --ep help text). Follow-ups not in this PR: CLI EP/TP validation for single-node, multi-node launchers sharing the registry table, ep_size in sglang launch evidence.

🤖 Generated with Claude Code

[xiaofei-zheng]

PR #1809 -- fix(executors): honour --ep on single-node servers through the argv seal

What it does: --ep N never reached a single-node sglang or atom server -- the only injection was vLLM-specific and ran on the materialize path alone, so a grid variant that rebuilt the argument string dropped it even for vLLM, while provenance and the KB recorded ep=N. The PR moves the rule into seal_server_argv, the last write to EXTRA_<FW>_ARGS that materialize, grid variants and argv repair all pass through, with the per-framework spelling in a new framework_registry.EXPERT_PARALLEL_FLAGS table; materialize writes EP into benchmark.envs on single-node runs and the vLLM injector is deleted.

Blocking issues: 1

1. [free:ep-guard-bypassed-by-a-pin] The new EP/TP guard is undone by a later writer in the same function, so the warning can state the opposite of what launches [verified]
   Problem: _set_single_node_ep refuses to write envs["EP"] and warns "launching without expert parallelism" when EP does not divide the launched TP, and returns early on multi-node because "the multi-node launcher passes EP itself" -- src/hyperloom/orchestrator/actions/executors/_workload_envs.py:1082-1102, called at :1521. The operator/variant extra-env merge at :1910-1911 then writes envs[key] = str(value) unconditionally, and EP passes is_allowed_variant_env_key (it is in BLOCKED_EXTERNAL_ENV_NAMES, not in the variant set the filter uses, src/hyperloom/common/env_safety.py:259 vs :288). seal_server_argv at :2258 then honours that EP.
   Impact: with --extra-env EP=3 and TP=8, materialize logs "EP=3 does not divide the launched TP=8 ... launching without expert parallelism" and still emits EXTRA_SGLANG_ARGS='--watchdog-timeout 1800 --ep-size 3'; with the same pin after the TP clamp, the server is handed an EP size larger than the ranks it has. The multi-node early return is defeated identically: INFERENCE_OPTIMIZER_NODES=2, TP=16, pin EP=16 yields --ep-size 16 in the extra args beside the EP the launcher already passes (launch_multinode.py:133 / launch_infera_node.py:367). Since #1797 an --extra-env pin of a control variable is a supported operator action, so the guard protects only the --ep path.
   Action: validate envs["EP"] where the seal consumes it (or re-run the single-node/divisibility check immediately before seal_server_argv at :2258), and cover an EP arriving through extra_envs in the tests -- every new case in test_workload_envs.py sets EP with monkeypatch.setenv only.

Checked: seal_server_argv / _expert_parallel_arg / _pinned_values and all three callers (materialize, _grid_runner._build_variant_yaml:534, reseal_config_argv:197); expert_parallel_flag against every FRAMEWORKS row (each framework owns a distinct extra_args_env, xdit/custom resolve to no row); _set_single_node_ep and the TP clamp above it; the extra-env merge and filter_untrusted_env_mapping / is_allowed_variant_env_key for EP; the deletion sweep for inject_vllm_expert_parallel (no reference in the head tree); the multi-node EP writers (launch_multinode.py:133, launch_infera_node.py:367, _multi_node_env.py:107); explore.py:352's atom --enable-expert-parallel variant; the argv-preflight drop/reseal interaction in baseline.py:3278-3320; the os.environ["EP"] readers in provenance and the KB (unchanged by this PR); origin/main (#1797, not yet in this PR's base) for the EP ladder and for the extra-env merge this finding turns on.
Ran: pytest src/hyperloom/inference_optimizer/tests/test_server_argv_seal.py src/hyperloom/inference_optimizer/tests/test_workload_envs.py (99 passed, 1 failed: test_every_write_to_the_argument_env_is_one_the_seal_settles, a pre-existing Windows-only path-separator failure in its "/tests/" filter); two materialize repros for the finding above.
Base: 880c167 | Head: 92d8daf

[zengleixin-amd]

Agreed, and confirmed with repros at TP=8: --extra-env EP=3, a extra_envs EP=3, and multi-node + --extra-env EP=16 all reached the argv as --ep-size N, one of them right after the "launching without expert parallelism" warning.

Fixed in bc1bfe6f8 (on top of the reviewed commits, rebased onto current main incl. #1797):

• The seal now owns both rules, since it is the only consumer of EP for the argv and every path (materialize, grid variant, reseal) ends there: no expert-parallel flag on multi-node, and none, with the warning, when EP does not divide the config's TP.
• A pinned size that contradicts EP is checked first, so it still raises when EP does not divide TP (e.g. --ep-size 2 with EP=3, TP=8) instead of launching with the pin behind a misleading warning.
• _set_single_node_ep is deleted; materialize just carries EP in the existing workload-env loop next to TP (no new branch; materialize_config_with_envs stays at 111). envs["EP"] is therefore present even when no flag is emitted; nothing reads it from the YAML except the seal.

New tests, failing before and passing after: EP via the operator pin, via extra_envs, via a grid variant's extra_envs, and via reseal, each at TP=8/EP=3 and on multi-node, plus the contradicting-pin case above.

[xiaofei-zheng]

PR #1809 -- fix(executors): honour --ep on single-node servers through the argv seal (re-review at bc1bfe6)

What it does: --ep N never reached a single-node sglang or atom server -- the only injection was vLLM-specific and ran on the materialize path alone, so a grid variant that rebuilt the argument string dropped it even for vLLM. The PR moves the whole rule into seal_server_argv, the last write to EXTRA_<FW>_ARGS that materialize, grid variants and argv repair all pass through: _expert_parallel_arg reads envs["EP"] and envs["TP"], skips multi-node (the launcher passes EP itself), drops an EP that does not divide TP with a warning, keeps an equal pin, raises on a contradicting one, and otherwise appends the framework's own spelling from the new framework_registry.EXPERT_PARALLEL_FLAGS. EP joins the workload-env copy loop in materialize; the vLLM injector is deleted.

The blocking finding from the previous review is fixed. Re-verified at this head: --extra-env EP=3 with TP=8 now logs the warning and emits EXTRA_SGLANG_ARGS='--watchdog-timeout 1800' with no flag; the same pin on a 2-node run adds nothing; a TP clamp from 8 to 4 with EP=8 drops the flag; --ep 4 at TP=8 still yields --ep-size 4; vLLM at EP=8 still gets --enable-expert-parallel. test_a_grid_variant_ep_that_does_not_divide_tp_adds_no_flag and the nodes/tp/ep parametrization of test_a_resealed_argv_gets_no_expert_parallel_flag_the_launch_cannot_take pin exactly that path.

Blocking issues: 1

1. [X2] The description still describes the previous head [verified]
   Problem: the body says "Materialize writes EP into benchmark.envs on single-node runs (multi-node launchers keep passing EP themselves); an EP that does not divide the launched TP is skipped with a warning", and "3 commits, 4 non-test files". At bc1bfe6 that is no longer what the diff does: materialize only adds "EP" to the workload-env copy loop (src/hyperloom/orchestrator/actions/executors/_workload_envs.py:1453), so benchmark.envs now carries EP on multi-node runs too, and both the multi-node skip and the TP-divisibility skip live in _expert_parallel_arg (src/hyperloom/orchestrator/actions/executors/_server_argv.py:93-137). The branch now has four commits.
   Impact: the release cut aggregates these descriptions, and the one operator-visible change this head added -- EP appearing in a multi-node config's benchmark.envs -- is stated nowhere; a reader looking for the gate in materialize finds no gate there.
   Action: rewrite the two bullets to put the EP rule (multi-node gate, TP divisibility, pin handling) in seal_server_argv, say that EP is now written into benchmark.envs on every run, and fix the commit count.

Checked: _expert_parallel_arg / _pinned_values / seal_server_argv at this head and all three callers (materialize, _grid_runner._build_variant_yaml:534, reseal_config_argv); the "EP" addition to the workload-env copy loop and the TP resolution/clamp above it; _multi_node_env.is_multi_node for an import cycle (none: it does not import _server_argv); the ordering of the contradicting-pin raise against the divisibility skip; a non-numeric --extra-env EP=abc, which raises the same bare int() ValueError every other workload knob already raises in _coerce_workload_int_env, so it is not new here; the new base e5f2be1 (now includes #1797, whose --extra-env pin path this finding turned on).
Ran: pytest src/hyperloom/inference_optimizer/tests/test_server_argv_seal.py src/hyperloom/inference_optimizer/tests/test_workload_envs.py (106 passed, 1 failed: test_every_write_to_the_argument_env_is_one_the_seal_settles, the pre-existing Windows-only "/tests/" path-separator failure); five materialize repros covering pin/multi-node/clamp/plain-EP/vLLM.
Base: e5f2be1 | Head: bc1bfe6

[xiaofei-zheng]

PR #1809 -- fix(executors): honour --ep on single-node servers through the argv seal

No blocking issues at bc1bfe6. The description is still the one written for the previous head (the EP rule now lives in the seal, not in materialize, and EP is written into benchmark.envs on multi-node runs too, over four commits); that is being handled separately, so it no longer holds this review.

Re-verified at this head: _expert_parallel_arg owns the whole rule and is reached by all three writers of the argument string -- materialize, _grid_runner._build_variant_yaml:534 and reseal_config_argv -- so no later writer of envs["EP"] (an --extra-env pin or a grid variant's extra_envs) can get past it. Five materialize repros: --extra-env EP=3 at TP=8 warns and adds no flag; the same pin on a 2-node run adds nothing; a TP clamp from 8 to 4 with EP=8 drops the flag; --ep 4 at TP=8 yields --ep-size 4 exactly once; vLLM at EP=8 keeps --enable-expert-parallel. Also checked: the contradicting-pin raise orders ahead of the divisibility skip; inject_vllm_expert_parallel has no surviving reference; _multi_node_env introduces no import cycle into _server_argv; a non-numeric EP pin raises the same bare int() error every other workload knob already raises in _coerce_workload_int_env; the new base e5f2be1 now carries #1797, whose pin path this interacts with.

Ran: pytest src/hyperloom/inference_optimizer/tests/test_server_argv_seal.py src/hyperloom/inference_optimizer/tests/test_workload_envs.py -- 106 passed, 1 pre-existing Windows-only failure (test_every_write_to_the_argument_env_is_one_the_seal_settles, its "/tests/" path filter). CI is green on all 30 reporting checks at this head.

LGTM