@qwen-code /triage

@qwen-code /triage

Thanks for the PR, @tt-a1i!

Template looks good ✓ — all required sections are present and well-filled (What, Why, Reviewer Test Plan, Risk & Scope, Linked Issues, 中文说明). The previous template-flagging review was a false positive; the PR body fully follows the template.

On direction: this is a real bug — MCP prompt named arguments set to an empty string being silently dropped is surprising and incorrect behavior. The fix is squarely within scope/mcp and the CLI's core responsibility. Aligned.

On approach: the scope is minimal and exactly right — two truthy checks replaced with Object.hasOwn presence checks, plus two focused unit tests. No scope creep, no drive-by refactors. This is the simplest possible fix for this bug.

Moving on to code review. 🔍

— Qwen Code · qwen3.7-max

Code Review

The fix is correct and clean. I independently arrived at the same approach: replace the two truthy checks with presence checks so that empty-string values are retained rather than discarded.

Two changes, both correct:

• if (argValues[arg.name]) → if (Object.hasOwn(argValues, arg.name)) — correctly preserves empty-string named args in the parsed output.
• !promptInputs[arg.name] → !Object.hasOwn(promptInputs, arg.name) — correctly prevents required args with empty-string values from being flagged as missing.

Object.hasOwn is the right choice over in (avoids prototype chain). The regex parsing correctly captures empty quoted values: for --trail="", match[2] is "" (not null), and the ?? operator preserves it since "" is not nullish.

No correctness issues, no security concerns, no AGENTS.md violations. The diff is exactly the minimal change needed.

Testing

Unit tests (worktree): 25/25 passed, including both new test cases:

• "should preserve empty optional named arguments" ✓
• "should treat empty required named arguments as provided" ✓

 RUN  v3.2.4 packages/cli
 ✓ src/services/McpPromptLoader.test.ts (25 tests) 15ms

 Test Files  1 passed (1)
      Tests  25 passed (25)

CI (GitHub): All 9 required checks passed (Lint, CodeQL, Test × 3 platforms). 12 skipped (integration tests, coverage, etc.).

Real-scenario (tmux): Not applicable for this fix — the bug is in internal argument parsing logic, not user-visible TUI behavior. Testing would require a configured MCP server with prompt definitions, which is beyond the scope of automated triage. The unit tests cover the fix comprehensively.

— Qwen Code · qwen3.7-max

This is a clean, well-scoped bug fix that does exactly what it says. The independent proposal I formed before reading the diff matched the PR's approach exactly — replace truthy checks with Object.hasOwn — which confirms it's the right solution.

The bug is real (empty-string MCP prompt args silently dropped), the fix is minimal (2 lines of logic), and the tests are focused (2 new cases that exercise both branches of the fix). Every line in the diff is necessary; nothing to cut.

The prior CHANGES_REQUESTED review flagged the template as incomplete, but the PR body actually follows the template fully — that was a false positive that shouldn't have blocked this PR.

Approving. ✅

— Qwen Code · qwen3.7-max

✅ Local verification report — PR #5323 fix(cli): preserve empty MCP prompt args

Verdict: strong LGTM. This is a correct, minimal, well-scoped fix. Empty-string named MCP prompt args (--trail="", --name="") were dropped by truthy checks in McpPromptLoader.parseArgs; switching to Object.hasOwn presence checks fixes it. The new tests genuinely catch the bug, the fix holds across an 7-case adversarial edge sweep, it is even slightly more correct than the old check (no prototype-member leak), and there are no regressions or type/lint/format issues.

Root cause & fix

The named-arg regex already stores argValues['trail'] = '' for --trail=""; the bug was the gate that consumed it:

• copy loop: if (argValues[arg.name]) → '' is falsy → dropped. Now if (Object.hasOwn(argValues, arg.name)).
• unfilled/required check: !promptInputs[arg.name] → '' is falsy → flagged missing. Now !Object.hasOwn(promptInputs, arg.name).

How it was verified

Real vitest under tmux in two isolated worktrees: AFTER = e3f22f0e (PR head), BEFORE = f91054a3 (origin/main), repo node_modules symlinked. Node v22.22.2, vitest 3.2.4. Merges cleanly; target files identical between merge-base and main.

Adversarial edge sweep (AFTER all pass; BASE diverges)

The last two cases are a robustness bonus: Object.hasOwn (unlike the old truthy/in-style access) won't pick up an inherited Object.prototype member when a prompt arg happens to be named toString/constructor/etc. On base, a required toString arg that the user omits silently "passes" with the inherited function as its value — the PR fixes that latent quirk too.

Note on pre-existing behavior (not changed, not a regression)

A single unfilled required arg with no input is filled with '' via positionalArgs.join('') (the "one unfilled arg" branch), rather than reported missing. So a lone empty required arg already happened to "work"; the user-visible bug bit when two or more required args were empty (the issue's --name="" --age="" case). The PR's fix makes all these cases consistent.

Reproduce

git worktree add --detach /tmp/wt-after  $(gh pr view 5323 --json headRefOid -q .headRefOid)
git worktree add --detach /tmp/wt-before origin/main
for w in wt-after wt-before; do ln -s "$PWD/node_modules" /tmp/$w/node_modules; \
  ln -s "$PWD/packages/cli/node_modules" /tmp/$w/packages/cli/node_modules; \
  ln -s "$PWD/packages/core/node_modules" /tmp/$w/packages/core/node_modules; done
(cd /tmp/wt-after/packages/cli && ../../node_modules/.bin/vitest run --coverage.enabled=false src/services/McpPromptLoader.test.ts)  # 27/27
# Catch the bug: copy PR's test onto BASE source -> 2 fail
cp /tmp/wt-after/packages/cli/src/services/McpPromptLoader.test.ts /tmp/wt-before/packages/cli/src/services/McpPromptLoader.test.ts
(cd /tmp/wt-before/packages/cli && ../../node_modules/.bin/vitest run --coverage.enabled=false src/services/McpPromptLoader.test.ts)

_{Verified locally with real vitest under tmux on isolated worktrees of the PR head and origin/main; bug reality proven by running the PR's tests against base source; parseArgs audited with a 7-case adversarial edge sweep (incl. a prototype-member-name robustness case) against base behavior.}