morluto/rea

Fork 0

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

[GH-ISSUE #7610] [Bug] MCP registration mixes human messages into --json stdout #4540

New issue

Open

opened 2026-10-31 00:50:37 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

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

Owner

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

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 amp --json successfully saves a registration and exits 0, but its complete stdout cannot be parsed as JSON. Human progress, a terminal clear-line sequence, and a success message precede the registration object. Stderr is empty. This also occurs with stdout redirected to a regular file.

Reproduction

Start in the already-built source checkout at the revision below, with its locked dependencies and Node.js v24.22.1 selected on PATH. No dependency install or rebuild was performed during verification.
Run this procedure. It creates recording npx and bun executables first on PATH before invoking REA, supplies a separate empty HOME and XDG directories for each command, and removes the fixtures automatically. The Amp-only command writes local settings without invoking either runner. No real package or registry is contacted.

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

with tempfile.TemporaryDirectory() as work:
    root = pathlib.Path(work)
    runners = root / 'bin'
    runners.mkdir()
    log = root / 'runner.log'
    for name in ('npx', 'bun'):
        stub = runners / name
        stub.write_text('#!/bin/sh\nprintf "%s\\n" "$(basename "$0") $*" >> "$RUNNER_LOG"\nexit 0\n')
        stub.chmod(0o755)
    for label, args in (
        ('add-1', ['mcp', 'add', '--agent', 'amp', '--json']),
        ('add-2', ['mcp', 'add', '--agent', 'amp', '--json']),
        ('control', ['setup', '--client', 'codex', '--skill=false', '--dry-run', '--json']),
    ):
        env = dict(os.environ)
        env['PATH'] = str(runners) + os.pathsep + env['PATH']
        env['RUNNER_LOG'] = str(log)
        for key, folder in (('HOME', 'home'), ('XDG_CONFIG_HOME', 'config'),
                            ('XDG_DATA_HOME', 'data'), ('XDG_CACHE_HOME', 'cache'),
                            ('XDG_STATE_HOME', 'state'), ('TMPDIR', 'temp')):
            path = root / label / folder
            path.mkdir(parents=True)
            env[key] = str(path)
        output = root / (label + '.stdout')
        with output.open('wb') as stream:
            result = subprocess.run(['node', 'scripts/rea.mjs', *args], env=env,
                                    stdout=stream, stderr=subprocess.PIPE, timeout=30)
        raw = output.read_bytes()
        print(label, 'exit=' + str(result.returncode), 'bytes=' + str(len(raw)),
              'stderr=' + str(len(result.stderr)))
        try:
            json.loads(raw)
            print('parse=ok')
        except json.JSONDecodeError as error:
            print('parse=' + str(error))
            print('prefix=' + repr(raw[:28]))
            tail = json.loads(raw[raw.index(b'{'):])
            print('suffix parses; agents=' + repr(tail['agents']))
            settings = pathlib.Path(env['HOME']) / '.config/amp/settings.json'
            saved = json.loads(settings.read_text())
            print('registration saved=' + str('rea' in saved['amp.mcpServers']))
    print('runner calls=' + (log.read_text() if log.exists() else 'none'))
PYREPRO

Both add-1 and add-2 report exit 0, 200 stdout bytes, zero stderr bytes, a JSON parse failure at the first character, and a saved registration. The setup control reports parse=ok and exit 0. No runner calls occur.

Both registration captures in this exact procedure reproduced in fresh Amp homes. Three earlier independent Amp runs also reproduced, including one fully consumed capture pipe, for five failures in five invocations.

Expected behavior

Selecting --json should let a consumer parse the complete stdout as the registration result. Human progress and success prose should be suppressed in that mode or sent to stderr. Registration content and exit status should remain intact.

The general CLI output guidance recommends --json for JSON consumers and specifies compact JSON when piped or redirected. It gives no exception for the documented MCP-registration builtin. There is no separate command-specific sentence explicitly promising one JSON document for mcp add; this expectation follows from the global output contract and selected format. The installation guide's explicit progress-to-stderr statement applies to setup, so it is not used here as a separate MCP-add requirement.

Impact and scope

P2, supported: a JSON consumer fails to read the result of a registration that actually succeeded. The demonstrated effect is a parse failure; no downstream application failure or data loss is claimed. Five isolated Amp runs failed in this way on the tested platform.

A bounded workaround was verified: in these captures, parsing only the suffix beginning at the first opening brace recovers the complete registration object. This depends on the human prefix's contents and is unsuitable as a stable output contract. Other operating systems and real package-runner client registrations remain untested.

REA version

Exact node scripts/rea.mjs --version output:

6.8.0

Source commit: 65f1c447344a423fa2785847dbcea1e9a9e2454a.

Runtime and platform

Node.js v24.22.1; macOS 27.0.1; Darwin arm64; locked Incur 0.6.1. No Amp client, native provider, or GUI was started.

Target and artifact context

No analysis target. The only registration artifact is a disposable Amp settings file in an isolated home. Both package-runner executables are recording fixtures.

Interface

CLI

Earliest failure stage

Transport or result projection

Relevant output or evidence

The inline procedure prints, for each registration:

add-1 exit=0 bytes=200 stderr=0
parse=Expecting value: line 1 column 1 (char 0)
prefix=b'Registering MCP server...\r\x1b['
suffix parses; agents=['Amp']
registration saved=True

add-2 has the same result. The prefix preview stops partway through the clear-line escape; the complete capture contains carriage return followed by ESC [K. The final 93 bytes form a complete, independently parseable registration object with name equal to rea, command equal to npx -y rea-agents@6.8.0 mcp, and agents equal to ["Amp"]. The full capture is 200 bytes, so this is additional non-JSON output rather than missing JSON bytes.

Contract and reachability locators at the stated baseline:

docs/cli.md:954-963: global JSON-consumer and redirected-output guidance.
docs/installation.md:1100-1109: documented builtin registration route; docs/installation.md:132-133 contains the setup-specific progress convention.
node_modules/incur/src/Cli.ts:1282-1324: selected-agent registration, unconditional progress and success writes, then the selected-format object.
node_modules/incur/src/SyncMcp.ts:22-27,51-84: Amp-only runner bypass and local settings writer.
scripts/rea.mjs:139-150 and src/cli/cliResultOutput.ts:233-238: forwarding of SDK stdout; SDK capture applies to JSONL rather than ordinary JSON.

Typed outcome or recovery expected

The same successful invocation should emit a single parseable registration document on stdout. Parsing the entire capture should succeed without knowing any progress wording, while the saved registration and successful exit status remain unchanged.

Analysis or suspected cause

Source-derived: the locked dependency's builtin writes its progress and human success block before checking whether an explicit output format was selected. It then appends the JSON object to the same stream. REA forwards those writes, so the builtin bypasses the clean structured-result behavior used by the setup control. The report concerns REA's shipped CLI behavior; the producing handler belongs to Incur.

Additional context

Open and closed issue searches, PR searches including merged work, and relevant issue comments found no same-defect report. Related reports were read and excluded:

Closed #3894 and open PR #4038 concern pipe completion and exit status. Here the JSON suffix is complete and regular-file captures fail too; this is not a demonstrated regression of that fix.
Open #7372 concerns an SDK debug prefix from scoped MCP doctor prompt discovery, a different producer and closure condition.
Closed #5435, merged PR #5514, and open #7441 concern indentation of otherwise parseable JSON.
Open #7375, #7580, #7535, #7536, and #7538 concern argument validation, discovery, scope, failed persistence, and configuration preservation respectively.

No last-known-good release was established.

Originally created by @coygeek on GitHub (Oct 27, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7610 ### 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 amp --json` successfully saves a registration and exits 0, but its complete stdout cannot be parsed as JSON. Human progress, a terminal clear-line sequence, and a success message precede the registration object. Stderr is empty. This also occurs with stdout redirected to a regular file. ### Reproduction 1. Start in the already-built source checkout at the revision below, with its locked dependencies and Node.js v24.22.1 selected on PATH. No dependency install or rebuild was performed during verification. 2. Run this procedure. It creates recording `npx` and `bun` executables first on PATH before invoking REA, supplies a separate empty HOME and XDG directories for each command, and removes the fixtures automatically. The Amp-only command writes local settings without invoking either runner. No real package or registry is contacted. ```sh python3 -I - <<'PYREPRO' import json, os, pathlib, subprocess, tempfile with tempfile.TemporaryDirectory() as work: root = pathlib.Path(work) runners = root / 'bin' runners.mkdir() log = root / 'runner.log' for name in ('npx', 'bun'): stub = runners / name stub.write_text('#!/bin/sh\nprintf "%s\\n" "$(basename "$0") $*" >> "$RUNNER_LOG"\nexit 0\n') stub.chmod(0o755) for label, args in ( ('add-1', ['mcp', 'add', '--agent', 'amp', '--json']), ('add-2', ['mcp', 'add', '--agent', 'amp', '--json']), ('control', ['setup', '--client', 'codex', '--skill=false', '--dry-run', '--json']), ): env = dict(os.environ) env['PATH'] = str(runners) + os.pathsep + env['PATH'] env['RUNNER_LOG'] = str(log) for key, folder in (('HOME', 'home'), ('XDG_CONFIG_HOME', 'config'), ('XDG_DATA_HOME', 'data'), ('XDG_CACHE_HOME', 'cache'), ('XDG_STATE_HOME', 'state'), ('TMPDIR', 'temp')): path = root / label / folder path.mkdir(parents=True) env[key] = str(path) output = root / (label + '.stdout') with output.open('wb') as stream: result = subprocess.run(['node', 'scripts/rea.mjs', *args], env=env, stdout=stream, stderr=subprocess.PIPE, timeout=30) raw = output.read_bytes() print(label, 'exit=' + str(result.returncode), 'bytes=' + str(len(raw)), 'stderr=' + str(len(result.stderr))) try: json.loads(raw) print('parse=ok') except json.JSONDecodeError as error: print('parse=' + str(error)) print('prefix=' + repr(raw[:28])) tail = json.loads(raw[raw.index(b'{'):]) print('suffix parses; agents=' + repr(tail['agents'])) settings = pathlib.Path(env['HOME']) / '.config/amp/settings.json' saved = json.loads(settings.read_text()) print('registration saved=' + str('rea' in saved['amp.mcpServers'])) print('runner calls=' + (log.read_text() if log.exists() else 'none')) PYREPRO ``` 3. Both `add-1` and `add-2` report exit 0, 200 stdout bytes, zero stderr bytes, a JSON parse failure at the first character, and a saved registration. The setup control reports `parse=ok` and exit 0. No runner calls occur. Both registration captures in this exact procedure reproduced in fresh Amp homes. Three earlier independent Amp runs also reproduced, including one fully consumed capture pipe, for five failures in five invocations. ### Expected behavior Selecting `--json` should let a consumer parse the complete stdout as the registration result. Human progress and success prose should be suppressed in that mode or sent to stderr. Registration content and exit status should remain intact. The general CLI output guidance recommends `--json` for JSON consumers and specifies compact JSON when piped or redirected. It gives no exception for the documented MCP-registration builtin. There is no separate command-specific sentence explicitly promising one JSON document for `mcp add`; this expectation follows from the global output contract and selected format. The installation guide's explicit progress-to-stderr statement applies to setup, so it is not used here as a separate MCP-add requirement. ### Impact and scope P2, supported: a JSON consumer fails to read the result of a registration that actually succeeded. The demonstrated effect is a parse failure; no downstream application failure or data loss is claimed. Five isolated Amp runs failed in this way on the tested platform. A bounded workaround was verified: in these captures, parsing only the suffix beginning at the first opening brace recovers the complete registration object. This depends on the human prefix's contents and is unsuitable as a stable output contract. Other operating systems and real package-runner client registrations remain untested. ### REA version Exact `node scripts/rea.mjs --version` output: ```text 6.8.0 ``` Source commit: `65f1c447344a423fa2785847dbcea1e9a9e2454a`. ### Runtime and platform Node.js v24.22.1; macOS 27.0.1; Darwin arm64; locked Incur 0.6.1. No Amp client, native provider, or GUI was started. ### Target and artifact context No analysis target. The only registration artifact is a disposable Amp settings file in an isolated home. Both package-runner executables are recording fixtures. ### Interface CLI ### Earliest failure stage Transport or result projection ### Relevant output or evidence The inline procedure prints, for each registration: ```text add-1 exit=0 bytes=200 stderr=0 parse=Expecting value: line 1 column 1 (char 0) prefix=b'Registering MCP server...\r\x1b[' suffix parses; agents=['Amp'] registration saved=True ``` `add-2` has the same result. The prefix preview stops partway through the clear-line escape; the complete capture contains carriage return followed by ESC `[K`. The final 93 bytes form a complete, independently parseable registration object with `name` equal to `rea`, `command` equal to `npx -y rea-agents@6.8.0 mcp`, and `agents` equal to `["Amp"]`. The full capture is 200 bytes, so this is additional non-JSON output rather than missing JSON bytes. Contract and reachability locators at the stated baseline: - `docs/cli.md:954-963`: global JSON-consumer and redirected-output guidance. - `docs/installation.md:1100-1109`: documented builtin registration route; `docs/installation.md:132-133` contains the setup-specific progress convention. - `node_modules/incur/src/Cli.ts:1282-1324`: selected-agent registration, unconditional progress and success writes, then the selected-format object. - `node_modules/incur/src/SyncMcp.ts:22-27,51-84`: Amp-only runner bypass and local settings writer. - `scripts/rea.mjs:139-150` and `src/cli/cliResultOutput.ts:233-238`: forwarding of SDK stdout; SDK capture applies to JSONL rather than ordinary JSON. ### Typed outcome or recovery expected The same successful invocation should emit a single parseable registration document on stdout. Parsing the entire capture should succeed without knowing any progress wording, while the saved registration and successful exit status remain unchanged. ### Analysis or suspected cause Source-derived: the locked dependency's builtin writes its progress and human success block before checking whether an explicit output format was selected. It then appends the JSON object to the same stream. REA forwards those writes, so the builtin bypasses the clean structured-result behavior used by the setup control. The report concerns REA's shipped CLI behavior; the producing handler belongs to Incur. ### Additional context Open and closed issue searches, PR searches including merged work, and relevant issue comments found no same-defect report. Related reports were read and excluded: - Closed [#3894](https://github.com/morluto/rea/issues/3894) and open [PR #4038](https://github.com/morluto/rea/pull/4038) concern pipe completion and exit status. Here the JSON suffix is complete and regular-file captures fail too; this is not a demonstrated regression of that fix. - Open [#7372](https://github.com/morluto/rea/issues/7372) concerns an SDK debug prefix from scoped MCP doctor prompt discovery, a different producer and closure condition. - Closed [#5435](https://github.com/morluto/rea/issues/5435), merged [PR #5514](https://github.com/morluto/rea/pull/5514), and open [#7441](https://github.com/morluto/rea/issues/7441) concern indentation of otherwise parseable JSON. - Open [#7375](https://github.com/morluto/rea/issues/7375), [#7580](https://github.com/morluto/rea/issues/7580), [#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 argument validation, discovery, scope, failed persistence, and configuration preservation respectively. No last-known-good release was established.

bsakaguchi commented

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

Author

Owner

@z2Ace0107 commented on GitHub (Oct 28, 2026):

I would like to take this one.

Reading the report, the defect is that mcp add --agent amp --json emits human
progress output, a terminal clear-line sequence and a success message on stdout
ahead of the registration object, with stderr empty. The repository's own
logging contract says the opposite: REA_LOG_LEVEL selects the level, logs go
to stderr, MCP defaults to info, one-shot CLI logging is opt-in, and stdout
carries only protocol messages and command results. So a --json run should
write nothing but the JSON object to stdout.

My plan is to keep the human-facing progress and success reporting where it is
for interactive runs, and route it to stderr (or suppress it) when --json is
selected, so the --json contract holds for a pipe or a regular file as well as
a terminal. I will also check the terminal clear-line sequence specifically,
since that is a separate emission from the progress text.

Scope: this is limited to the registration command's output routing. I am not
changing what the registration writes, only where its human-facing messages go.

I have not started implementation yet; I will post the reproduction output from
this issue alongside my own before/after evidence in the pull request.

@z2Ace0107 commented on GitHub (Oct 28, 2026): I would like to take this one. Reading the report, the defect is that `mcp add --agent amp --json` emits human progress output, a terminal clear-line sequence and a success message on stdout ahead of the registration object, with stderr empty. The repository's own logging contract says the opposite: `REA_LOG_LEVEL` selects the level, logs go to stderr, MCP defaults to `info`, one-shot CLI logging is opt-in, and stdout carries only protocol messages and command results. So a `--json` run should write nothing but the JSON object to stdout. My plan is to keep the human-facing progress and success reporting where it is for interactive runs, and route it to stderr (or suppress it) when `--json` is selected, so the `--json` contract holds for a pipe or a regular file as well as a terminal. I will also check the terminal clear-line sequence specifically, since that is a separate emission from the progress text. Scope: this is limited to the registration command's output routing. I am not changing what the registration writes, only where its human-facing messages go. I have not started implementation yet; I will post the reproduction output from this issue alongside my own before/after evidence in the pull request.

bsakaguchi commented

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

Author

Owner

@z2Ace0107 commented on GitHub (Oct 28, 2026):

Follow-up with the actual root cause, which is not in this repository.

mcp add is a builtin of the incur CLI library, not REA code. In
incur@0.6.1, dist/Cli.js implements the add subcommand and writes, all to
stdout: Registering MCP server..., then \r\x1b[K, then the
✓ Registered <name> as MCP server block with the agent list and the
"Agents can now use ... tools" line, and only then — guarded by
if (fullOutput || formatExplicit) — the machine-readable object. So the
human-facing output is unconditional and only the final object honours the
requested format, which is exactly the mismatch reported here.

That means the fix belongs in incur: the progress, clear-line and success
emissions should go to stderr (or be suppressed) when a machine-readable format
is selected, the same way REA's own one-shot commands keep stdout to protocol
messages and command results. I cannot correct it from this repository without
overriding a library builtin, which would fight the library rather than fix the
defect.

I am withdrawing my earlier claim on this issue so nobody waits on a pull
request that cannot land here. The reproduction in the report is precise enough
to file upstream, and the fullOutput || formatExplicit guard is the line to
point at.

@z2Ace0107 commented on GitHub (Oct 28, 2026): Follow-up with the actual root cause, which is not in this repository. `mcp add` is a builtin of the `incur` CLI library, not REA code. In `incur@0.6.1`, `dist/Cli.js` implements the `add` subcommand and writes, all to stdout: `Registering MCP server...`, then `\r\x1b[K`, then the `✓ Registered <name> as MCP server` block with the agent list and the "Agents can now use ... tools" line, and only then — guarded by `if (fullOutput || formatExplicit)` — the machine-readable object. So the human-facing output is unconditional and only the final object honours the requested format, which is exactly the mismatch reported here. That means the fix belongs in `incur`: the progress, clear-line and success emissions should go to stderr (or be suppressed) when a machine-readable format is selected, the same way REA's own one-shot commands keep stdout to protocol messages and command results. I cannot correct it from this repository without overriding a library builtin, which would fight the library rather than fix the defect. I am withdrawing my earlier claim on this issue so nobody waits on a pull request that cannot land here. The reproduction in the report is precise enough to file upstream, and the `fullOutput || formatExplicit` guard is the line to point at.

bsakaguchi referenced this issue

2026-10-31 01:13:25 +00:00

[PR #4549] [MERGED] fix(objc): isolate empty protocol selectors and category class names #6301

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#4540

No description provided.

Rows
Columns