morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[GH-ISSUE #7608] [Bug] mcp add ignores undeclared flags and applies registration #4539

New issue

Open

opened 2026-10-31 00:50:36 +00:00 by bsakaguchi · 0 comments

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/7608

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 --bogus exits 0, announces successful registration, starts the package-runner registration path, and creates an Amp REA entry. It gives no unknown-option diagnostic. mcp add --agents claude-code, a misspelling of the declared --agent, likewise succeeds while omitting the agent selector from the child arguments and creating Amp settings.

The child executable in this verification was a recording fixture. It did not download or run a package or write client settings. The Amp file was written by REA itself. The correctly spelled mcp add --agent claude-code control passes -a claude-code to the fixture and creates no Amp file.

Reproduction

Use the already-built baseline below, its locked dependencies, and Node.js v24.22.1 on PATH. No dependency installation or rebuild was performed for this verification.
Run this fixture from the checkout root. It puts recording npx and bun executables first on PATH before registration, also intercepts bunx and pnpx, forces the npm runner, and gives each call separate HOME/XDG directories. It deletes all fixtures and settings afterward.

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

node = shutil.which("node")
assert node, "Put Node.js v24.22.1 on PATH first"
with tempfile.TemporaryDirectory() as work:
    root = pathlib.Path(work)
    bindir = root / "bin"
    bindir.mkdir()
    fixture = '''#!/bin/sh
printf 'RUNNER[%s]\\n' "${0##*/}" >> "$HOME/runner.txt"
printf 'ARG[%s]\\n' "$@" >> "$HOME/runner.txt"
printf '│ ✓ Claude Code: fixture │\\n'
exit 0
'''
    for name in ("npx", "bun", "bunx", "pnpx"):
        path = bindir / name
        path.write_text(fixture)
        path.chmod(0o755)
    for case, args in (
        ("unknown", ["--bogus"]),
        ("misspelled", ["--agents", "claude-code"]),
        ("valid", ["--agent", "claude-code"]),
    ):
        env = {
            "PATH": str(bindir) + os.pathsep + str(pathlib.Path(node).parent),
            "npm_config_user_agent": "npm/11 node/v24.22.1",
            "npm_execpath": "", "NO_COLOR": "1", "CI": "1",
        }
        for key, part in {
            "HOME": "home", "XDG_CONFIG_HOME": "config",
            "XDG_CACHE_HOME": "cache", "XDG_DATA_HOME": "data",
            "XDG_STATE_HOME": "state", "TMPDIR": "tmp",
            "npm_config_prefix": "npm-global",
        }.items():
            directory = root / case / part
            directory.mkdir(parents=True, exist_ok=True)
            env[key] = str(directory)
        result = subprocess.run(
            [node, "scripts/rea.mjs", "mcp", "add", *args],
            env=env, text=True, capture_output=True, timeout=30,
        )
        home = pathlib.Path(env["HOME"])
        settings = home / ".config/amp/settings.json"
        log = home / "runner.txt"
        print(case, "exit=" + str(result.returncode), "amp=" + str(settings.exists()))
        print(result.stdout)
        print("stderr=" + repr(result.stderr))
        print(log.read_text() if log.exists() else "no runner invocation")
        if settings.exists():
            print(json.dumps(json.loads(settings.read_text())))
PY

Both malformed requests report exit=0 amp=True, with no -a selector in the runner log. The valid request reports exit=0 amp=False, with ARG[-a] and ARG[claude-code]. These primary cases each reproduced twice in separate homes across the initial verification and the report replay.

Expected behavior

Reject an undeclared flag with a diagnostic naming it and a nonzero exit before starting registration or changing client configuration. Preserve the valid declared options.

The command's executed help declares only --agent <string>, --command, -c <string>, and --no-global. The project contract requires strict inputs and preservation of caller choices. This rejection expectation is also supported by executed doctor --bogus, setup --bogus, update --bogus, and mcp doctor --bogus controls: all return exit 1 and identify the unknown flag.

The CLI guide directs callers to command help for the declared argument shape, but its explicit rejection promise concerns surplus positionals. The installation guide documents this separate registration route and its Windows launcher limitation; neither guide documents silently accepting unknown registration flags. No particular error code is required by this report.

Impact and scope

P2: an undeclared flag receives plausible success and triggers registration effects. A mistyped agent selector loses the requested client selection; verification observed an actual persistent Amp entry and an external-runner invocation without any agent selector. Using the advertised --agent claude-code form is a directly verified workaround.

Only disposable local settings were changed. The real package runner was not executed, so which other client files it would write is unknown. No live client, analysis provider, security boundary, data loss, or credential exposure was involved. Other platforms and existing settings were not tested.

REA version

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

Runtime and platform

Node.js v24.22.1; macOS 27.0.1; arm64. Locked Incur 0.6.1 and the prepared compiled runtime.

Target and artifact context

No analysis target. Artifacts are the fixture runner's argument log and a disposable Amp settings file. The fixture reports one simulated Claude Code success line and makes no registry or package request.

Interface

CLI

Earliest failure stage

Request parsing or validation

Relevant output or evidence

Both malformed cases exit 0 with empty stderr and output containing:

✓ Registered rea as MCP server
  Agents: Claude Code, Amp

Agents can now use rea tools.

Their recorded runner arguments are:

RUNNER[npx]
ARG[add-mcp]
ARG[npx -y rea-agents@6.8.0 mcp]
ARG[--name]
ARG[rea]
ARG[-y]
ARG[-g]

The actual Amp settings contain:

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

The valid control adds ARG[-a] and ARG[claude-code] and does not create that file. Separately executed mcp add --client claude-code also loses the selector. mcp add --agent claude-code --bogus still starts the recording runner, showing acceptance of an unknown flag even when an otherwise valid selector is present.

Contract locators at the baseline: AGENTS.md under Product and Authority; docs/tool-design.md:52-56; docs/cli.md:940-944; docs/installation.md:1100-1109. Source-derived dispatch and cause: scripts/rea.mjs:9-11,31-40; node_modules/incur/src/Cli.ts:1282-1303; node_modules/incur/src/SyncMcp.ts:27-33,51-54. The registered-command surplus guard at src/cli/strictPositionals.ts:56-115 does not intercept this builtin. The ordinary option parser explicitly rejects unknown flags at node_modules/incur/src/Parser.ts:23-42.

Typed outcome or recovery expected

A nonzero validation failure naming the unsupported option, before invoking any runner or creating settings. Keep valid agent-scoped registration working.

Analysis or suspected cause

Source-derived: this builtin manually extracts selected registration flags and does not reject unrecognized tail tokens. The resulting empty agent selection reaches the default registration path, including REA's direct Amp writer. This explains the observed child arguments and settings. No repair was implemented or tested.

Additional context

Duplicate searches covered open/closed issues, all-state PRs including merged fixes, and issue comments. #7375 and PR #7453 address surplus positionals. The pure --bogus reproduction has none, and the inspected PR patch defers malformed-flag parser errors to Incur, leaving this closure signal unmet. #7580 concerns recognized schema discovery, requiring discovery output or refusal. #7535, #7536, and #7538 concern valid Amp scope, failed-write reporting, and preservation of existing settings. #5811 and PR #6347 concern Windows runner startup. Closed #6448 fixes registered-command surplus positionals, a different contract and closure signal. No matching unknown-flag report or prior fix was found; the first affected release is unknown.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Undeclared flags produce successful registration output, launch the registration path and can persist an unintended client entry. The demonstrated effect is bounded to local configuration; severe harm was not observed.
Workaround status: available
Workaround basis: The correctly spelled --agent claude-code control passes the requested selector and creates no Amp settings file in the same isolated fixture.

Originally created by @coygeek on GitHub (Oct 27, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7608 ### 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 --bogus` exits 0, announces successful registration, starts the package-runner registration path, and creates an Amp REA entry. It gives no unknown-option diagnostic. `mcp add --agents claude-code`, a misspelling of the declared `--agent`, likewise succeeds while omitting the agent selector from the child arguments and creating Amp settings. The child executable in this verification was a recording fixture. It did not download or run a package or write client settings. The Amp file was written by REA itself. The correctly spelled `mcp add --agent claude-code` control passes `-a claude-code` to the fixture and creates no Amp file. ### Reproduction 1. Use the already-built baseline below, its locked dependencies, and Node.js v24.22.1 on PATH. No dependency installation or rebuild was performed for this verification. 2. Run this fixture from the checkout root. It puts recording `npx` and `bun` executables first on PATH before registration, also intercepts `bunx` and `pnpx`, forces the npm runner, and gives each call separate HOME/XDG directories. It deletes all fixtures and settings afterward. ```sh python3 -I - <<'PY' import json, os, pathlib, shutil, subprocess, tempfile node = shutil.which("node") assert node, "Put Node.js v24.22.1 on PATH first" with tempfile.TemporaryDirectory() as work: root = pathlib.Path(work) bindir = root / "bin" bindir.mkdir() fixture = '''#!/bin/sh printf 'RUNNER[%s]\\n' "${0##*/}" >> "$HOME/runner.txt" printf 'ARG[%s]\\n' "$@" >> "$HOME/runner.txt" printf '│ ✓ Claude Code: fixture │\\n' exit 0 ''' for name in ("npx", "bun", "bunx", "pnpx"): path = bindir / name path.write_text(fixture) path.chmod(0o755) for case, args in ( ("unknown", ["--bogus"]), ("misspelled", ["--agents", "claude-code"]), ("valid", ["--agent", "claude-code"]), ): env = { "PATH": str(bindir) + os.pathsep + str(pathlib.Path(node).parent), "npm_config_user_agent": "npm/11 node/v24.22.1", "npm_execpath": "", "NO_COLOR": "1", "CI": "1", } for key, part in { "HOME": "home", "XDG_CONFIG_HOME": "config", "XDG_CACHE_HOME": "cache", "XDG_DATA_HOME": "data", "XDG_STATE_HOME": "state", "TMPDIR": "tmp", "npm_config_prefix": "npm-global", }.items(): directory = root / case / part directory.mkdir(parents=True, exist_ok=True) env[key] = str(directory) result = subprocess.run( [node, "scripts/rea.mjs", "mcp", "add", *args], env=env, text=True, capture_output=True, timeout=30, ) home = pathlib.Path(env["HOME"]) settings = home / ".config/amp/settings.json" log = home / "runner.txt" print(case, "exit=" + str(result.returncode), "amp=" + str(settings.exists())) print(result.stdout) print("stderr=" + repr(result.stderr)) print(log.read_text() if log.exists() else "no runner invocation") if settings.exists(): print(json.dumps(json.loads(settings.read_text()))) PY ``` 3. Both malformed requests report `exit=0 amp=True`, with no `-a` selector in the runner log. The valid request reports `exit=0 amp=False`, with `ARG[-a]` and `ARG[claude-code]`. These primary cases each reproduced twice in separate homes across the initial verification and the report replay. ### Expected behavior Reject an undeclared flag with a diagnostic naming it and a nonzero exit before starting registration or changing client configuration. Preserve the valid declared options. The command's executed help declares only `--agent <string>`, `--command, -c <string>`, and `--no-global`. The project contract requires strict inputs and preservation of caller choices. This rejection expectation is also supported by executed `doctor --bogus`, `setup --bogus`, `update --bogus`, and `mcp doctor --bogus` controls: all return exit 1 and identify the unknown flag. The CLI guide directs callers to command help for the declared argument shape, but its explicit rejection promise concerns surplus positionals. The installation guide documents this separate registration route and its Windows launcher limitation; neither guide documents silently accepting unknown registration flags. No particular error code is required by this report. ### Impact and scope P2: an undeclared flag receives plausible success and triggers registration effects. A mistyped agent selector loses the requested client selection; verification observed an actual persistent Amp entry and an external-runner invocation without any agent selector. Using the advertised `--agent claude-code` form is a directly verified workaround. Only disposable local settings were changed. The real package runner was not executed, so which other client files it would write is unknown. No live client, analysis provider, security boundary, data loss, or credential exposure was involved. Other platforms and existing settings were not tested. ### REA version Exact `node scripts/rea.mjs --version` stdout: `6.8.0` followed by a newline. Source baseline: `65f1c447344a423fa2785847dbcea1e9a9e2454a`. ### Runtime and platform Node.js v24.22.1; macOS 27.0.1; arm64. Locked Incur 0.6.1 and the prepared compiled runtime. ### Target and artifact context No analysis target. Artifacts are the fixture runner's argument log and a disposable Amp settings file. The fixture reports one simulated Claude Code success line and makes no registry or package request. ### Interface CLI ### Earliest failure stage Request parsing or validation ### Relevant output or evidence Both malformed cases exit 0 with empty stderr and output containing: ```text ✓ Registered rea as MCP server Agents: Claude Code, Amp Agents can now use rea tools. ``` Their recorded runner arguments are: ```text RUNNER[npx] ARG[add-mcp] ARG[npx -y rea-agents@6.8.0 mcp] ARG[--name] ARG[rea] ARG[-y] ARG[-g] ``` The actual Amp settings contain: ```json {"amp.mcpServers":{"rea":{"command":"npx","args":["-y","rea-agents@6.8.0","mcp"]}}} ``` The valid control adds `ARG[-a]` and `ARG[claude-code]` and does not create that file. Separately executed `mcp add --client claude-code` also loses the selector. `mcp add --agent claude-code --bogus` still starts the recording runner, showing acceptance of an unknown flag even when an otherwise valid selector is present. Contract locators at the baseline: `AGENTS.md` under Product and Authority; `docs/tool-design.md:52-56`; `docs/cli.md:940-944`; `docs/installation.md:1100-1109`. Source-derived dispatch and cause: `scripts/rea.mjs:9-11,31-40`; `node_modules/incur/src/Cli.ts:1282-1303`; `node_modules/incur/src/SyncMcp.ts:27-33,51-54`. The registered-command surplus guard at `src/cli/strictPositionals.ts:56-115` does not intercept this builtin. The ordinary option parser explicitly rejects unknown flags at `node_modules/incur/src/Parser.ts:23-42`. ### Typed outcome or recovery expected A nonzero validation failure naming the unsupported option, before invoking any runner or creating settings. Keep valid agent-scoped registration working. ### Analysis or suspected cause Source-derived: this builtin manually extracts selected registration flags and does not reject unrecognized tail tokens. The resulting empty agent selection reaches the default registration path, including REA's direct Amp writer. This explains the observed child arguments and settings. No repair was implemented or tested. ### Additional context Duplicate searches covered open/closed issues, all-state PRs including merged fixes, and issue comments. [#7375](https://github.com/morluto/rea/issues/7375) and [PR #7453](https://github.com/morluto/rea/pull/7453) address surplus positionals. The pure `--bogus` reproduction has none, and the inspected PR patch defers malformed-flag parser errors to Incur, leaving this closure signal unmet. [#7580](https://github.com/morluto/rea/issues/7580) concerns recognized schema discovery, requiring discovery output or refusal. [#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 valid Amp scope, failed-write reporting, and preservation of existing settings. [#5811](https://github.com/morluto/rea/issues/5811) and [PR #6347](https://github.com/morluto/rea/pull/6347) concern Windows runner startup. Closed [#6448](https://github.com/morluto/rea/issues/6448) fixes registered-command surplus positionals, a different contract and closure signal. No matching unknown-flag report or prior fix was found; the first affected release is unknown. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Undeclared flags produce successful registration output, launch the registration path and can persist an unintended client entry. The demonstrated effect is bounded to local configuration; severe harm was not observed. - Workaround status: available - Workaround basis: The correctly spelled --agent claude-code control passes the requested selector and creates no Amp settings file in the same isolated fixture.

bsakaguchi referenced this issue

2026-11-01 21:10:57 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

bsakaguchi referenced this issue

2026-11-01 21:47:24 +00:00

[PR #6509] [MERGED] feat(pe): inspect import and delay-load tables #7373