[SebTardif]

What Problem This Solves

splitCommandLine drops a quoted empty argument. mcporter list --stdio '"" node' therefore launches node with no arguments. The same string in a config command field is already rejected: parseCommandString keeps the empty token, and an empty executable is not promoted. --stdio and generate-cli --command now follow that rule.

A quoted empty argument between other tokens is kept. An unquoted C:\Program Files\... path still splits the same way. That path behavior was rejected in #385 and is unchanged here. Config command strings were already fixed in #484 (the change proposed in #486). This pull covers the separate --stdio and generate-cli --command splitter.

Evidence

Before this patch, importing splitCommandLine and resolveEphemeralServer from src/cli/adhoc-server.ts on main (21624e8) printed:

split ["node"]
executable "node"
args []
middle ["node","--label","after"]

"" node became executable node. node --label "" after lost the empty argument.

After this patch, the CLI rejects the empty executable and does not start a server. macOS, Node v26.12.1, pnpm 10.34.6, pnpm exec tsx src/cli.ts from b26deb8:

pnpm exec tsx src/cli.ts list --stdio '"" node'

[mcporter] --stdio requires a non-empty command.
Error: --stdio requires a non-empty command.

pnpm exec tsx src/cli.ts generate-cli --command '"" node'

[mcporter] --command requires a non-empty value.
Error: --command requires a non-empty value.

Both commands exit 1. The same parser keeps the empty argument and leaves the unquoted Program Files split unchanged:

split ["","node"]
middle ["node","--label","","after"]
program-files ["C:Program","Filesnode.exe","script.js"]
quoted-program-files ["C:\\Program Files\\node.exe","script.js"]

Real behavior proof

• Behavior or issue addressed: Quoted empty argv from --stdio and generate-cli --command was discarded, so "" node ran node.
• Real environment tested: macOS, Node v26.12.1, pnpm 10.34.6, pnpm exec tsx src/cli.ts from this branch.
• Exact steps or command run after this patch: pnpm exec tsx src/cli.ts list --stdio '"" node' and pnpm exec tsx src/cli.ts generate-cli --command '"" node'.
• Evidence after fix: Both commands exit 1 with --stdio requires a non-empty command. and --command requires a non-empty value. No server process is started. A middle empty argument stays in the parsed argv: ["node","--label","","after"].
• Observed result after fix: The empty executable is rejected. A quoted empty argument between real tokens is preserved. The unquoted Program Files split is unchanged.
• What was not tested: A Windows host launching an unquoted Program Files stdio command. This patch does not change that split.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-05T23:20:36.268609Z	b26deb8	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

Preserve quoted empty arguments in ad-hoc stdio command strings and reject an empty executable.

Example: Run mcporter list --stdio '"" node'.

• Before: The empty token disappears and node becomes the executable.
• After: The CLI rejects the command with '--stdio requires a non-empty command.'

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, correct repair with direct CLI rejection proof and useful regression coverage; tests were inspected rather than executed during this read-only review.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): Recorded macOS runs through src/cli.ts at b26deb8 exercise both changed executable-validation consumers: list --stdio and generate-cli --command reject '"" node' with exit 1; parser output and regression coverage support preservation of middle empty arguments.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Quoted empty values disappear from stdio commands, shifting arguments and potentially selecting a later token as the executable.
Reason: Preserving explicit argument boundaries matches the existing quoting contract and config-parser behavior with a small, focused repair.

Merge readiness

✅ Ready for maintainer review

Keep open: this PR fixes a distinct argument-parsing bug still present on main, with a focused patch and recorded CLI proof; no actionable defects were found.

Priority: P2
Reviewed head: b26deb8c65c7bc9361674a0ad0aaeae3553ec621

Before merge

None.

Findings

None.

Agent review details

How this fits together

The CLI tokenizes ad-hoc command strings into executable and argument arrays used by runtime server definitions and generated CLIs.

flowchart TD
 A[CLI command string] --> B[Shared command splitter]
 B --> C[Ad-hoc server resolver]
 B --> D[Generated CLI command parser]
 C --> E[Executable and argument array]
 D --> E
 E --> F[Stdio transport]

Loading

Technical review

Best possible solution:

Land the focused splitter repair and executable validation while retaining explicit quoting and existing argument-array formats.

Do we have a high-confidence way to reproduce the issue?

Current-main source directly establishes that '"" node' becomes ['node'] and that middle empty arguments disappear; the contributor also supplies before output and after-fix CLI traces.

Is this the best way to solve the issue?

The patch repairs token presence in the existing shared splitter and validates its executable consumers without changing quoting rules or adding another parsing path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 21624e8cc823.

Provenance checked

• Ad-hoc command tokenization changes intended behavior with a stated reason (63f8dd6: The original ad-hoc implementation added shell-style quote handling; its available source discarded empty accumulated tokens, with no documented intent to discard explicitly quoted arguments.)
• Generated CLI executable validation keeps the original intent (af38e3b: The generator extraction reused the ad-hoc splitter and required a nonempty command; the patch extends that existing requirement to the newly retained empty first token.)
• Explicit Windows path quoting keeps the original intent (fix(cli): keep Windows Program Files paths as one stdio command #385: The recorded collaborator decision retained explicit quoting because extension-based guessing changes valid executable boundaries.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

• Verified focused patch: The pinned introduction changes the existing splitter to retain quoted empty tokens and updates both executable consumers to reject an empty first token. (src/cli/adhoc-server.ts:216, b26deb8c65c7)
• Current-main bug remains: Main emits tokens only when their accumulated text is nonempty, dropping quoted empty arguments before selecting the executable. (src/cli/adhoc-server.ts:228, 21624e8cc823)
• Related config fix has a separate owner: fix(cli): preserve arguments and safe generated command behavior #484 merged the config-command repair proposed in fix(config): preserve empty quoted command arguments #486; current config normalization preserves empty quoted tokens, but the ad-hoc splitter remains separate. (src/config-normalize.ts:309, 21624e8cc823)
• Prior quoting decision respected: In fix(cli): keep Windows Program Files paths as one stdio command #385 (comment), steipete rejected executable-extension guessing and required explicit quoting; this patch preserves that boundary. (src/cli/adhoc-server.ts:239, b26deb8c65c7)
• Recorded changed-path CLI proof: The PR body records macOS CLI runs at the reviewed head for list --stdio and generate-cli --command with '"" node'; both exit 1 with the expected empty-command errors. (b26deb8c65c7)
• Feature history and inspection limit: Available original-file diffs establish the ad-hoc parser and later generator extraction; history and blame identify their continuing owners. Some unrelated historical blobs are unavailable locally, so broad historical stat and follow operations encountered missing-object errors. (src/cli/adhoc-server.ts:211, 63f8dd66463a)

Likely related people:

• Sebastien Tardif: Raw commit 5236b19 adds src/cli/adhoc-server.ts:119 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: 5236b1914f6b; files: src/cli/adhoc-server.ts)

Preflight source

Source unknown. Source comparison at the observed PR snapshot only. Completion is self-reported; body, policy, tests, execution, and report bytes are not verified.

Review metrics

Metric	Value	Why it matters
Patch distribution	Production +9/-4; tests +48/-0; changelog +2/-0	The small production change reuses the existing parser; regression cases cover empty-token positions, executable rejection, and inline npx inference.

Labels

Label changes:

• add P2: Quoted empty arguments can shift server inputs or promote the wrong executable in an advanced CLI workflow, with limited blast radius.
• add proof: sufficient: Contributor real behavior proof is sufficient.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P2: Quoted empty arguments can shift server inputs or promote the wrong executable in an advanced CLI workflow, with limited blast radius.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
• proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed November 5, 2026, 6:31 PM ET / 23:31 UTC.