fix(vllm): reject same launch flag in both args and extraArgs

`buildVLLMArgs` renders `spec.engine.args` first, then appends
`spec.engine.extraArgs` verbatim. When the same flag key was set in both
(e.g. `tensor-parallel-size: "4"` in args and `--tensor-parallel-size=2`
in extraArgs), the rendered Deployment carried two conflicting copies.
vLLM's argparse is last-wins, so it silently honored the extraArgs value
and defeated the `engine.args` one.

- Add `validateDuplicateVLLMArgKeys`, called at the top of
  `buildVLLMArgs` (covers both aggregated and disaggregated paths),
  which fails with a clear error naming the flag and both fields instead
  of guessing a winner. Flags that legitimately repeat live only in
  `extraArgs` and are untouched.
- Add regression tests: reject the cross-source conflict in both inline
  (`--flag=v`) and two-token (`--flag v`) forms, and confirm an
  `extraArgs`-only flag is still allowed.

Signed-off-by: Suraj Deshmukh <suraj.deshmukh@microsoft.com>

Author: surajssd

providers/vllm/transformer.go

@@ -397,6 +397,10 @@ func (t *Transformer) buildVLLMArgs(md *airunwayv1alpha1.ModelDeployment, kvTran
 		return nil, err
 	}
 
+	if err := validateDuplicateVLLMArgKeys(md.Spec.Engine.Args, md.Spec.Engine.ExtraArgs); err != nil {
+		return nil, err
+	}
+
 	// A flag we derive from structured spec fields (e.g. --tensor-parallel-size
 	// from the GPU count) must yield to an explicit override of the same key,
 	// whether it is provided via spec.engine.args (map) or spec.engine.extraArgs
@@ -509,6 +513,33 @@ func validateReservedVLLMServerArgs(engineArgs map[string]string, extraArgs []st
 	return nil
 }
 
+// validateDuplicateVLLMArgKeys rejects a launch flag that is set in BOTH
+// spec.engine.args (the structured map) and spec.engine.extraArgs (raw tokens).
+// engine.args is a map, so a key can appear there at most once; finding the same
+// key again in extraArgs is an unambiguous contradiction. We render engine.args
+// first and then append extraArgs verbatim, so without this guard the rendered
+// command would carry two conflicting copies of the flag (e.g.
+// "--tensor-parallel-size 4 … --tensor-parallel-size=2"). vLLM's argparse is
+// last-wins, so it would silently honor the extraArgs value and defeat the
+// engine.args one. Surface the conflict as a clear error instead of guessing a
+// winner; the user removes one of the two settings. Flags that legitimately
+// repeat live only in extraArgs and are untouched by this check.
+func validateDuplicateVLLMArgKeys(engineArgs map[string]string, extraArgs []string) error {
+	if len(engineArgs) == 0 {
+		return nil
+	}
+	for _, arg := range extraArgs {
+		key, ok := extraArgKey(arg)
+		if !ok {
+			continue
+		}
+		if _, dup := engineArgs[key]; dup {
+			return fmt.Errorf("launch flag %q is set in both spec.engine.args and spec.engine.extraArgs (%q); set it in exactly one place so vLLM does not receive conflicting values", key, arg)
+		}
+	}
+	return nil
+}
+
 func isReservedVLLMServerArg(key string) bool {
 	// Normalize the key the same way for both the engineArgs map form ("port")
 	// and a user that writes the flag form as a map key ("--port") or with an

providers/vllm/transformer_test.go

@@ -1251,6 +1251,54 @@ func TestBuildVLLMArgsTensorParallelDedupViaExtraArgs(t *testing.T) {
 	}
 }
 
+// Setting the SAME launch flag in both spec.engine.args (the structured map) and
+// spec.engine.extraArgs (raw tokens) is a contradiction: engine.args renders
+// first and extraArgs is appended verbatim, so both would reach vLLM and its
+// last-wins argparse would silently honor the extraArgs value — defeating the
+// engine.args one. The transformer must reject this instead of emitting two
+// conflicting copies. Regression for the --tensor-parallel-size 4-vs-2 bug.
+func TestBuildVLLMArgsRejectsArgsExtraArgsConflict(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		extraArg string
+	}{
+		{name: "inline value form", extraArg: "--tensor-parallel-size=2"},
+		{name: "two-token form", extraArg: "--tensor-parallel-size"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			tr := NewTransformer()
+			md := newTestMD("test-model", "default")
+			md.Spec.Engine.Args = map[string]string{
+				"tensor-parallel-size": "4",
+			}
+			md.Spec.Engine.ExtraArgs = []string{tc.extraArg}
+
+			_, err := tr.buildVLLMArgs(md, "", 0)
+			if err == nil {
+				t.Fatalf("expected an error for a flag set in both engine.args and extraArgs")
+			}
+			for _, want := range []string{"tensor-parallel-size", "spec.engine.args", "spec.engine.extraArgs"} {
+				if !strings.Contains(err.Error(), want) {
+					t.Errorf("error %q should mention %q", err.Error(), want)
+				}
+			}
+		})
+	}
+}
+
+// A flag that is present ONLY in extraArgs (not also in engine.args) is a valid,
+// legitimately-repeatable override and must NOT be rejected by the duplicate
+// guard — only the cross-source contradiction is an error.
+func TestBuildVLLMArgsAllowsExtraArgsOnlyFlag(t *testing.T) {
+	tr := NewTransformer()
+	md := newTestMD("test-model", "default")
+	md.Spec.Engine.ExtraArgs = []string{"--tensor-parallel-size=2"}
+
+	if _, err := tr.buildVLLMArgs(md, "", 0); err != nil {
+		t.Fatalf("unexpected error for an extraArgs-only flag: %v", err)
+	}
+}
+
 // The same dedup must hold for the other derived flags so an explicit engine.args
 // entry is never duplicated by the structured-field renderer.
 func TestBuildVLLMArgsDerivedFlagDedup(t *testing.T) {