morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #7609] [Bug] MCP registration ignores missing values for --agent and --command #4537

New issue

Closed

opened 2026-10-31 00:50:36 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:50:36 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 27, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/7609

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

CLI

Observed behavior

node scripts/rea.mjs mcp add --agent exits 0, announces successful registration, invokes the registration runner without an agent selector, and creates an Amp registration using the default launcher. mcp add --command and its -c alias also succeed with the default launcher when the required command value is absent.

Verification used fresh homes and recording npx and bun fixtures first on PATH. The recording runner returned no simulated client output; the Amp settings write came from REA itself. No package was fetched, and no real client configuration was used.

Reproduction

Use the already-built baseline below with its locked dependencies and Node.js v24.22.1 on PATH. The prepared runtime was used without rebuilding or reinstalling dependencies.
From the checkout root, run this isolated reproduction. It constructs each child environment explicitly, installs both recording executables before any registration request, prints the result and saved settings, and deletes all fixtures on exit.

python3 -I - <<'PY'
import json, pathlib, shutil, subprocess, tempfile

node = shutil.which('node')
assert node, 'Put Node.js v24.22.1 on PATH first'
assert subprocess.check_output([node, '--version'], text=True).strip() == 'v24.22.1'
with tempfile.TemporaryDirectory() as work:
    root = pathlib.Path(work)
    bin_dir = root / 'bin'
    bin_dir.mkdir()
    for name in ['npx', 'bun']:
        fixture = bin_dir / name
        fixture.write_text('#!/bin/sh\nprintf "%s\\n" "$@" >> "$HOME/runner-argv.txt"\nexit 0\n')
        fixture.chmod(0o755)
    cases = [
        ('help', ['mcp', 'add', '--help']),
        ('agent-1', ['mcp', 'add', '--agent']),
        ('agent-2', ['mcp', 'add', '--agent']),
        ('command-1', ['mcp', 'add', '--command']),
        ('command-2', ['mcp', 'add', '--command']),
        ('short-command', ['mcp', 'add', '-c']),
        ('setup-control', ['setup', '--client']),
        ('valid-agent', ['mcp', 'add', '--agent', 'claude-code']),
        ('valid-command', ['mcp', 'add', '--command', 'fixture-server --mcp']),
    ]
    for case, args in cases:
        home = root / case
        home.mkdir()
        env = {'PATH': str(bin_dir) + ':' + str(pathlib.Path(node).parent) + ':/usr/bin:/bin',
               'LANG': 'en_US.UTF-8', 'TERM': 'dumb', 'NO_COLOR': '1',
               'HOME': str(home)}
        for key, part in [('XDG_CONFIG_HOME', '.config'), ('XDG_CACHE_HOME', '.cache'),
                          ('XDG_DATA_HOME', '.local/share'), ('XDG_STATE_HOME', '.local/state'),
                          ('TMPDIR', 'tmp'), ('npm_config_prefix', '.npm-global')]:
            directory = home / part
            directory.mkdir(parents=True, exist_ok=True)
            env[key] = str(directory)
        result = subprocess.run([node, 'scripts/rea.mjs', *args], env=env,
                                text=True, capture_output=True, timeout=35)
        log = home / 'runner-argv.txt'
        settings = home / '.config/amp/settings.json'
        print(case, 'exit=' + str(result.returncode))
        print('stdout=' + repr(result.stdout), 'stderr=' + repr(result.stderr))
        print('runner=' + json.dumps(log.read_text().splitlines() if log.exists() else []))
        print('amp=' + (settings.read_text().strip() if settings.exists() else 'absent'))
PY

Both agent runs and both command runs report exit 0, call the runner without a supplied selector/override, and create Amp settings with the default launcher. The short-command case does the same. setup-control exits 1 with a missing-value diagnostic and makes no runner call or Amp write. The value-supplied controls show the selector and override reaching the recording runner correctly.

Each long-option case reproduced 2/2 times in this replay and 2/2 times in an earlier independent probe; the alias reproduced once in each replay. No other platform or real add-mcp client integration was tested.

Expected behavior

The command's public help advertises --agent <string> to target a specific agent and --command, -c <string> to override its launcher. These are valued options. Supplying one without its value should produce a nonzero validation failure identifying the incomplete option before calling the registration runner or writing client settings.

The executed node scripts/rea.mjs setup --client comparison already rejects an analogous missing client value. No particular new error code is required here; the essential result is rejection before registration side effects.

Impact and scope

P2: an incomplete registration request produces misleading success and a persistent default registration. In the isolated fixture, the missing --agent removed selection from the runner request and caused an Amp settings write; the missing --command retained the default launcher. The real external runner's client selection was not exercised, so additional client writes are unknown.

Supplying the intended value is an observed practical workaround: --agent claude-code forwarded -a claude-code and did not create Amp settings, while --command 'fixture-server --mcp' forwarded and saved that override. Existing settings, backups, and running client behavior were not tested.

REA version

Exact node scripts/rea.mjs --version output: 6.8.0 followed by a newline.
Source baseline: 65f1c447344a423fa2785847dbcea1e9a9e2454a.

Runtime and platform

Node.js v24.22.1; macOS 27.0.1; Darwin arm64. Repository-pinned Incur 0.6.1 and the prepared compiled runtime.

Target and artifact context

No analysis target. Each case uses an empty home, isolated XDG directories, and recording package-runner fixtures. The affected artifact is a disposable Amp MCP launcher entry. Registration stores the fixture command as data; it never executes that command.

Interface

CLI

Earliest failure stage

Request parsing or validation

Relevant output or evidence

Public mcp add --help returned these option declarations:

--agent <string>        Target a specific agent (e.g. claude-code, cursor)
--command, -c <string>  Override the command agents will run (e.g. "pnpm my-cli --mcp")

For each trailing --agent, --command, and -c case, the recording runner received:

["add-mcp", "npx -y rea-agents@6.8.0 mcp", "--name", "rea", "-y", "-g"]

The CLI exited 0 with empty stderr and printed ✓ Registered rea as MCP server and Agents: Amp. The created settings contained:

{"amp.mcpServers":{"rea":{"command":"npx","args":["-y","rea-agents@6.8.0","mcp"]}}}

The sibling setup --client comparison exited 1, recorded no child arguments, created no Amp settings, and returned:

code: UNKNOWN
message: "Missing value for flag: --client"

Source-derived contract and reachability: node_modules/incur/src/internal/command.ts:408-426 declares optional string options and the c alias; node_modules/incur/src/Parser.ts:49-54 rejects a missing value in the normal parser. scripts/rea.mjs:9-11,31-40 preserves the supported registration-helper route; src/cli.ts:95-103,169-181 configures and serves it. docs/installation.md:1100-1109 documents this separate registration route, and the Shell completion section in docs/cli.md directs callers to command help for accepted options. No documentation found gives these trailing valued options a defaulting meaning.

Typed outcome or recovery expected

Reject the incomplete option with a nonzero diagnostic and leave registration runners and client settings untouched. Preserve value-supplied selector and command behavior.

Analysis or suspected cause

Source-derived: the builtin registration handler at node_modules/incur/src/Cli.ts:1282-1304 consumes an agent or command only when the next token is truthy, then continues into registration with the defaults. It does not run the string-option validation used by normal commands. The empty selector list reaches the external runner at node_modules/incur/src/SyncMcp.ts:21-33 and enables the direct Amp writer at :51-82.

A repair would need to enforce the valued-option contract before either side effect. No repair was implemented or evaluated in this verification.

Additional context

Open #7375 and PR #7453 address surplus positional arguments. The PR's current parser guard catches malformed-flag errors and leaves them to Incur, so it does not cover these missing values. #7580 concerns schema discovery; #7535, #7536, and #7538 concern scope, failed persistence, and existing-field preservation. #5811 and PR #6347 concern Windows runner startup. Closed #6455 concerns missing output-format diagnostics on mcp doctor; closed #6448 concerns surplus positionals on registered commands.

Open/closed issue, all-state PR, and comment searches found no exact missing-option-value duplicate. The first affected release is unknown; this report makes no regression claim.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Repeated incomplete option requests succeeded and persisted a default Amp registration, with the requested agent selection or command override absent. The observed impact was bounded to disposable local settings.
Workaround status: available
Workaround basis: Value-supplied agent and command controls forwarded the intended values; the agent control also avoided the unintended Amp write.

Originally created by @coygeek on GitHub (Oct 27, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7609 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area CLI ### Observed behavior `node scripts/rea.mjs mcp add --agent` exits 0, announces successful registration, invokes the registration runner without an agent selector, and creates an Amp registration using the default launcher. `mcp add --command` and its `-c` alias also succeed with the default launcher when the required command value is absent. Verification used fresh homes and recording `npx` and `bun` fixtures first on PATH. The recording runner returned no simulated client output; the Amp settings write came from REA itself. No package was fetched, and no real client configuration was used. ### Reproduction 1. Use the already-built baseline below with its locked dependencies and Node.js v24.22.1 on PATH. The prepared runtime was used without rebuilding or reinstalling dependencies. 2. From the checkout root, run this isolated reproduction. It constructs each child environment explicitly, installs both recording executables before any registration request, prints the result and saved settings, and deletes all fixtures on exit. ```sh python3 -I - <<'PY' import json, pathlib, shutil, subprocess, tempfile node = shutil.which('node') assert node, 'Put Node.js v24.22.1 on PATH first' assert subprocess.check_output([node, '--version'], text=True).strip() == 'v24.22.1' with tempfile.TemporaryDirectory() as work: root = pathlib.Path(work) bin_dir = root / 'bin' bin_dir.mkdir() for name in ['npx', 'bun']: fixture = bin_dir / name fixture.write_text('#!/bin/sh\nprintf "%s\\n" "$@" >> "$HOME/runner-argv.txt"\nexit 0\n') fixture.chmod(0o755) cases = [ ('help', ['mcp', 'add', '--help']), ('agent-1', ['mcp', 'add', '--agent']), ('agent-2', ['mcp', 'add', '--agent']), ('command-1', ['mcp', 'add', '--command']), ('command-2', ['mcp', 'add', '--command']), ('short-command', ['mcp', 'add', '-c']), ('setup-control', ['setup', '--client']), ('valid-agent', ['mcp', 'add', '--agent', 'claude-code']), ('valid-command', ['mcp', 'add', '--command', 'fixture-server --mcp']), ] for case, args in cases: home = root / case home.mkdir() env = {'PATH': str(bin_dir) + ':' + str(pathlib.Path(node).parent) + ':/usr/bin:/bin', 'LANG': 'en_US.UTF-8', 'TERM': 'dumb', 'NO_COLOR': '1', 'HOME': str(home)} for key, part in [('XDG_CONFIG_HOME', '.config'), ('XDG_CACHE_HOME', '.cache'), ('XDG_DATA_HOME', '.local/share'), ('XDG_STATE_HOME', '.local/state'), ('TMPDIR', 'tmp'), ('npm_config_prefix', '.npm-global')]: directory = home / part directory.mkdir(parents=True, exist_ok=True) env[key] = str(directory) result = subprocess.run([node, 'scripts/rea.mjs', *args], env=env, text=True, capture_output=True, timeout=35) log = home / 'runner-argv.txt' settings = home / '.config/amp/settings.json' print(case, 'exit=' + str(result.returncode)) print('stdout=' + repr(result.stdout), 'stderr=' + repr(result.stderr)) print('runner=' + json.dumps(log.read_text().splitlines() if log.exists() else [])) print('amp=' + (settings.read_text().strip() if settings.exists() else 'absent')) PY ``` 3. Both `agent` runs and both `command` runs report exit 0, call the runner without a supplied selector/override, and create Amp settings with the default launcher. The `short-command` case does the same. `setup-control` exits 1 with a missing-value diagnostic and makes no runner call or Amp write. The value-supplied controls show the selector and override reaching the recording runner correctly. Each long-option case reproduced 2/2 times in this replay and 2/2 times in an earlier independent probe; the alias reproduced once in each replay. No other platform or real add-mcp client integration was tested. ### Expected behavior The command's public help advertises `--agent <string>` to target a specific agent and `--command, -c <string>` to override its launcher. These are valued options. Supplying one without its value should produce a nonzero validation failure identifying the incomplete option before calling the registration runner or writing client settings. The executed `node scripts/rea.mjs setup --client` comparison already rejects an analogous missing client value. No particular new error code is required here; the essential result is rejection before registration side effects. ### Impact and scope P2: an incomplete registration request produces misleading success and a persistent default registration. In the isolated fixture, the missing `--agent` removed selection from the runner request and caused an Amp settings write; the missing `--command` retained the default launcher. The real external runner's client selection was not exercised, so additional client writes are unknown. Supplying the intended value is an observed practical workaround: `--agent claude-code` forwarded `-a claude-code` and did not create Amp settings, while `--command 'fixture-server --mcp'` forwarded and saved that override. Existing settings, backups, and running client behavior were not tested. ### REA version Exact `node scripts/rea.mjs --version` output: `6.8.0` followed by a newline. Source baseline: `65f1c447344a423fa2785847dbcea1e9a9e2454a`. ### Runtime and platform Node.js v24.22.1; macOS 27.0.1; Darwin arm64. Repository-pinned Incur 0.6.1 and the prepared compiled runtime. ### Target and artifact context No analysis target. Each case uses an empty home, isolated XDG directories, and recording package-runner fixtures. The affected artifact is a disposable Amp MCP launcher entry. Registration stores the fixture command as data; it never executes that command. ### Interface CLI ### Earliest failure stage Request parsing or validation ### Relevant output or evidence Public `mcp add --help` returned these option declarations: ```text --agent <string> Target a specific agent (e.g. claude-code, cursor) --command, -c <string> Override the command agents will run (e.g. "pnpm my-cli --mcp") ``` For each trailing `--agent`, `--command`, and `-c` case, the recording runner received: ```json ["add-mcp", "npx -y rea-agents@6.8.0 mcp", "--name", "rea", "-y", "-g"] ``` The CLI exited 0 with empty stderr and printed `✓ Registered rea as MCP server` and `Agents: Amp`. The created settings contained: ```json {"amp.mcpServers":{"rea":{"command":"npx","args":["-y","rea-agents@6.8.0","mcp"]}}} ``` The sibling `setup --client` comparison exited 1, recorded no child arguments, created no Amp settings, and returned: ```text code: UNKNOWN message: "Missing value for flag: --client" ``` Source-derived contract and reachability: `node_modules/incur/src/internal/command.ts:408-426` declares optional string options and the `c` alias; `node_modules/incur/src/Parser.ts:49-54` rejects a missing value in the normal parser. `scripts/rea.mjs:9-11,31-40` preserves the supported registration-helper route; `src/cli.ts:95-103,169-181` configures and serves it. `docs/installation.md:1100-1109` documents this separate registration route, and the Shell completion section in `docs/cli.md` directs callers to command help for accepted options. No documentation found gives these trailing valued options a defaulting meaning. ### Typed outcome or recovery expected Reject the incomplete option with a nonzero diagnostic and leave registration runners and client settings untouched. Preserve value-supplied selector and command behavior. ### Analysis or suspected cause Source-derived: the builtin registration handler at `node_modules/incur/src/Cli.ts:1282-1304` consumes an agent or command only when the next token is truthy, then continues into registration with the defaults. It does not run the string-option validation used by normal commands. The empty selector list reaches the external runner at `node_modules/incur/src/SyncMcp.ts:21-33` and enables the direct Amp writer at `:51-82`. A repair would need to enforce the valued-option contract before either side effect. No repair was implemented or evaluated in this verification. ### Additional context Open [#7375](https://github.com/morluto/rea/issues/7375) and [PR #7453](https://github.com/morluto/rea/pull/7453) address surplus positional arguments. The PR's current parser guard catches malformed-flag errors and leaves them to Incur, so it does not cover these missing values. [#7580](https://github.com/morluto/rea/issues/7580) concerns schema discovery; [#7535](https://github.com/morluto/rea/issues/7535), [#7536](https://github.com/morluto/rea/issues/7536), and [#7538](https://github.com/morluto/rea/issues/7538) concern scope, failed persistence, and existing-field preservation. [#5811](https://github.com/morluto/rea/issues/5811) and [PR #6347](https://github.com/morluto/rea/pull/6347) concern Windows runner startup. Closed [#6455](https://github.com/morluto/rea/issues/6455) concerns missing output-format diagnostics on `mcp doctor`; closed [#6448](https://github.com/morluto/rea/issues/6448) concerns surplus positionals on registered commands. Open/closed issue, all-state PR, and comment searches found no exact missing-option-value duplicate. The first affected release is unknown; this report makes no regression claim. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Repeated incomplete option requests succeeded and persisted a default Amp registration, with the requested agent selection or command override absent. The observed impact was bounded to disposable local settings. - Workaround status: available - Workaround basis: Value-supplied agent and command controls forwarded the intended values; the agent control also avoided the unintended Amp write.

bsakaguchi commented

2026-10-31 00:50:36 +00:00

Author

Owner

@mirzasnowy commented on GitHub (Oct 29, 2026):

Hi, I'd like to pick this up. Plan is pretty simple: reject the incomplete mcp add request in REA's own entry point before Incur runs, so a trailing --agent, --command, or -c without a value fails with a missing-value diagnostic instead of silently registering with defaults. I'll add unit tests for the check plus keep the valid-value and help paths untouched.

@mirzasnowy commented on GitHub (Oct 29, 2026): Hi, I'd like to pick this up. Plan is pretty simple: reject the incomplete mcp add request in REA's own entry point before Incur runs, so a trailing --agent, --command, or -c without a value fails with a missing-value diagnostic instead of silently registering with defaults. I'll add unit tests for the check plus keep the valid-value and help paths untouched.

bsakaguchi referenced this issue

2026-10-31 01:11:50 +00:00

[PR #4158] [MERGED] feat(apk): inspect signing declarations with retained fact queries #6097

bsakaguchi closed this issue

2026-11-04 12:09:24 +00:00