Skip to content

Commit d8a5894

Browse files
pluskymimi1vx
authored andcommitted
fix(mcp): keep REMAINDER flags from swallowing later flags in kwargs_to_argv
The MCP server re-encodes tool-call kwargs into an argv list that is fed to the same argparse parser the REPL uses, so the two paths must parse to the same Namespace. Two argv-marshalling bugs broke that parity: * _argv.py: an append action with nargs=REMAINDER (commit -m, lock -c) was emitted into the flag list. A flag declared *after* it -- notably -T/--template added by _add_template_arg for commit/lock -- was then emitted behind it and swallowed by the REMAINDER, corrupting the message/comment and leaving template=None (a wrong fan-out across every loaded template). Route append+REMAINDER into the positional tail, exactly as the optional-REMAINDER path already does, so later flags stay safe regardless of parser._actions declaration order. * session.py start_jobs: a fanned-out job was scoped with [*argv, "-T", rrid]. For a positional nargs=REMAINDER command (run) the trailing -T rrid was swallowed into command, so template stayed None and -T <RRID> leaked into the remote shell command. Prepend the scope flag (["-T", rrid, *argv]) so it is always parsed as a real flag. Add tests/test_mcp_repl_parity.py: a reusable parity assertion parametrised across the command matrix (both paths must parse to equal Namespaces), dedicated regressions for both fixes (commit/lock -m/-c with a template, and start_jobs scoping for a REMAINDER run plus a nargs='+' slow command), and a safety check that a smuggled force kwarg cannot reach approve/reject.
1 parent b545d98 commit d8a5894

4 files changed

Lines changed: 476 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,12 @@ and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.ht
213213

214214
### Fixed
215215

216+
- `mtui-mcp` no longer corrupts a `commit`/`lock` call that also carries a
217+
`template` argument, nor a backgrounded `run` that fans out across several
218+
loaded templates. A `-m`/`-c` message or a `run` command line no longer
219+
swallows the template-scoping flag, so the message stays intact, the call
220+
is scoped to the intended template (instead of silently fanning out to
221+
every one), and no stray `-T <RRID>` leaks into the remote command.
216222
- An unscoped multi-template fan-out of a host-phase command (e.g. `lock`,
217223
`run`) no longer fails the whole fan-out — or, for `lock`, pretends to
218224
succeed — when a loaded template has no connected host. A host-less template

mtui/mcp/_argv.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
when the action's ``nargs`` consumes a remainder/multi
1313
(``REMAINDER``/``*``/``+``/integer ``N`` — e.g. ``commit -m``, ``lock -c``):
1414
there the flag is emitted once followed by every token, so argparse rebuilds
15-
``[[v1, v2, ...]]`` instead of swallowing a repeated flag as a value.
15+
``[[v1, v2, ...]]`` instead of swallowing a repeated flag as a value — and,
16+
like the optional-REMAINDER case below, **into the positional tail** so a
17+
later flag (e.g. ``-T/--template``) declared after it cannot be swallowed.
1618
* Positional ``nargs=REMAINDER``/``*``/``+`` → append elements verbatim
1719
at the end (after every flag-shaped argument), preserving order.
1820
* Optional ``nargs=REMAINDER`` (e.g. ``reject --message``) → emit the
@@ -194,8 +196,20 @@ def kwargs_to_argv(
194196
# argparse rebuilds ``[[v1, v2, ...]]``; ``--flag v1 --flag v2``
195197
# would let REMAINDER swallow the second ``--flag`` as a value,
196198
# producing ``[[v1, '--flag', v2]]`` and a corrupted message.
197-
flag_tokens.append(flag)
198-
flag_tokens.extend(str(item) for item in value)
199+
#
200+
# Route into the positional tail (not ``flag_tokens``) for the
201+
# same reason as the optional-REMAINDER branch below: a
202+
# REMAINDER/multi append consumes every token after its flag, so
203+
# any flag declared *after* it (e.g. ``-T/--template`` added by
204+
# ``_add_template_arg`` for ``commit``/``lock``) would be
205+
# swallowed into the message/comment and lost. Emitting after
206+
# every other flag keeps later flags safe regardless of
207+
# ``parser._actions`` declaration order. (Assumes at most one
208+
# REMAINDER-shaped tail contributor per command: no current
209+
# command declares a real positional *after* an append+REMAINDER
210+
# flag, which would be emitted ahead of it here and swallowed.)
211+
positional_tail.append(flag)
212+
positional_tail.extend(str(item) for item in value)
199213
else:
200214
for item in value:
201215
flag_tokens.extend([flag, str(item)])

mtui/mcp/session.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1012,7 +1012,12 @@ async def start_jobs(
10121012
self._job_counter += 1
10131013
token = rrid.replace(":", "_")
10141014
job_id = f"{cmd_cls.command}-{token}-{self._job_counter}"
1015-
scoped_argv = [*argv, "-T", rrid]
1015+
# Prepend the scope flag: a positional ``nargs=REMAINDER`` command
1016+
# (``run``) or an append-REMAINDER flag at the tail of ``argv``
1017+
# would swallow a trailing ``-T <rrid>`` into its own value,
1018+
# leaving ``template=None`` (wrong fan-out) and leaking ``-T`` into
1019+
# the remote command. Emitting it first keeps it a real flag.
1020+
scoped_argv = ["-T", rrid, *argv]
10161021
job_ids.append(self._mint_job(cmd_cls, scoped_argv, job_id))
10171022
return job_ids
10181023

0 commit comments

Comments
 (0)