morluto/rea

Fork 0

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

[GH-ISSUE #7538] [Bug] mcp add --agent amp drops existing environment overrides without a backup #4482

New issue

Closed

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

bsakaguchi commented

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

Owner

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

Before submitting

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

Area

MCP or agent integration

Observed behavior

node scripts/rea.mjs mcp add --agent amp --json removes the environment map from an existing Amp REA registration, even when its launcher already matches REA's current default. With env: {"KEEP_ME": "1"} on that entry, the command exits 0 and reports successful registration, but the rewritten entry contains only command and args.

The peer MCP server and top-level theme survive. No recovery file is created in the isolated configuration directories. No command override or malformed input is required.

Reproduction

From the built source checkout at the revision below, use Node.js 24.22.1 and create an isolated fixture:

WORK=$(mktemp -d)
export HOME="$WORK/home" XDG_CONFIG_HOME="$WORK/config"
export XDG_DATA_HOME="$WORK/data" XDG_CACHE_HOME="$WORK/cache"
export XDG_STATE_HOME="$WORK/state" TMPDIR="$WORK/tmp"
mkdir -p "$HOME/.config/amp" "$TMPDIR"
cat > "$HOME/.config/amp/settings.json" <<'JSON'
{
  "amp.mcpServers": {
    "rea": {
      "command": "npx",
      "args": ["-y", "rea-agents@6.8.0", "mcp"],
      "env": {"KEEP_ME": "1"}
    },
    "other": {"command": "other"}
  },
  "theme": "dark"
}
JSON

node scripts/rea.mjs mcp add --agent amp --json
printf 'exit=%s\n' "$?"
cat "$HOME/.config/amp/settings.json"
node --input-type=module - <<'JS'
import { readdirSync } from 'node:fs';
console.log('Amp directory:', JSON.stringify(readdirSync(`${process.env.HOME}/.config/amp`)));
JS

The default-launcher fixture reproduced in three separate isolated homes, including the manual-backup control described below. A custom-launcher variant with --command 'node custom-rea.mjs' also reproduced twice. These runs used already-built artifacts and locked dependencies; no dependency installation or rebuild was performed during verification.

Expected behavior

Refreshing a registration must preserve existing caller configuration and retain a recoverable pre-change copy before a destructive update. The repository's configuration invariant requires additive, idempotent changes with backups. An entry whose launcher is already current should not silently lose its environment map.

The installation guide explicitly recognizes this builtin as a separate REA registration route implemented through Incur. Its setup/uninstall sections specify additional recovery conventions for those commands; this report does not assume those exact backup filenames also apply to the builtin.

Impact and scope

P2, supported: valid local registration data is persistently deleted during a successful refresh, with no automatic recovery artifact in the tested isolated directories. The demonstrated loss is the synthetic environment map; downstream client failures and real configuration losses were not tested.

A preemptive manual workaround was verified in a separate home: copy the settings before registration, then restore that copy. The restored file retained the environment map and the same current launcher. This does not recover values after a run for which no prior copy exists. Other agents, operating systems, and the first affected release remain unverified.

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 or native analysis provider was started.

Target and artifact context

Synthetic JSON client settings with one existing REA server, a peer server, and a theme. The stored REA launcher is already the default produced by this revision. No analysis target is involved, and registration does not execute the stored launcher.

Interface

CLI

Earliest failure stage

Setup or provider admission

Relevant output or evidence

The public CLI returned exit 0, empty stderr, and:

✓ Registered rea as MCP server
  Agents: Amp

The resulting entry, read directly from the settings file, was:

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

KEEP_ME was absent, while the peer and theme retained their values. The directory inspection printed:

Amp directory: ["settings.json"]

Source evidence at the stated baseline:

AGENTS.md:11: the configuration-change invariant.
docs/installation.md:1100-1109: REA's documented builtin and its distinction from setup.
docs/installation.md:73-89,94-105: setup/uninstall recovery and preservation context, whose command-specific details are not extended here.
scripts/rea.mjs:9-11,31-40,80-91 and src/cli.ts:95-100: public dispatch and REA's supplied default launcher.
node_modules/incur/src/Cli.ts:1282-1304: builtin invocation of the registration writer.
node_modules/incur/src/SyncMcp.ts:22-27,51-55,61-84: Amp-only branch, existing-file parse, entry replacement, and direct write without recovery.

Typed outcome or recovery expected

After the same valid refresh, amp.mcpServers.rea.env.KEEP_ME should still equal "1", the launcher and unrelated settings should remain intact, and a recoverable pre-change copy should be available. If the command cannot preserve the existing entry safely, it should refuse before changing it and explain why.

Analysis or suspected cause

Source-derived: the pinned Incur writer constructs a replacement object containing only the selected command and arguments. It does not merge existing entry fields or create a backup before serializing the settings. The public CLI reproductions confirm this path is reachable through REA.

The implementation cause is in a dependency, but REA ships and documents the command and supplies its launcher. The failure therefore also belongs at REA's integration boundary under its configuration invariant; this report does not attribute authorship of the writer to REA.

Additional context

Open and closed issue searches and PR searches found no same-contract duplicate. GitHub CLI search attempts failed with HTTP 401; searches were completed with the connected GitHub tools using explicit open/closed queries for amp.mcpServers, Amp titles, and MCP-add environment handling, plus backup, overwrite, preservation, and PR queries.

Related reports were read and excluded: #7375 and PR #7453 concern surplus positional rejection; #5811 and PR #6347 concern Windows package-runner spawning; closed #1163 concerns Codex TOML comments and setup/uninstall backups. None has preservation of an existing Amp entry's environment map as its closure signal.

Originally created by @coygeek on GitHub (Oct 27, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7538 ### 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 MCP or agent integration ### Observed behavior `node scripts/rea.mjs mcp add --agent amp --json` removes the environment map from an existing Amp REA registration, even when its launcher already matches REA's current default. With `env: {"KEEP_ME": "1"}` on that entry, the command exits 0 and reports successful registration, but the rewritten entry contains only `command` and `args`. The peer MCP server and top-level theme survive. No recovery file is created in the isolated configuration directories. No command override or malformed input is required. ### Reproduction 1. From the built source checkout at the revision below, use Node.js 24.22.1 and create an isolated fixture: ```sh WORK=$(mktemp -d) export HOME="$WORK/home" XDG_CONFIG_HOME="$WORK/config" export XDG_DATA_HOME="$WORK/data" XDG_CACHE_HOME="$WORK/cache" export XDG_STATE_HOME="$WORK/state" TMPDIR="$WORK/tmp" mkdir -p "$HOME/.config/amp" "$TMPDIR" cat > "$HOME/.config/amp/settings.json" <<'JSON' { "amp.mcpServers": { "rea": { "command": "npx", "args": ["-y", "rea-agents@6.8.0", "mcp"], "env": {"KEEP_ME": "1"} }, "other": {"command": "other"} }, "theme": "dark" } JSON ``` 2. Register and inspect the resulting file and directory: ```sh node scripts/rea.mjs mcp add --agent amp --json printf 'exit=%s\n' "$?" cat "$HOME/.config/amp/settings.json" node --input-type=module - <<'JS' import { readdirSync } from 'node:fs'; console.log('Amp directory:', JSON.stringify(readdirSync(`${process.env.HOME}/.config/amp`))); JS ``` The default-launcher fixture reproduced in three separate isolated homes, including the manual-backup control described below. A custom-launcher variant with `--command 'node custom-rea.mjs'` also reproduced twice. These runs used already-built artifacts and locked dependencies; no dependency installation or rebuild was performed during verification. ### Expected behavior Refreshing a registration must preserve existing caller configuration and retain a recoverable pre-change copy before a destructive update. The repository's configuration invariant requires additive, idempotent changes with backups. An entry whose launcher is already current should not silently lose its environment map. The installation guide explicitly recognizes this builtin as a separate REA registration route implemented through Incur. Its setup/uninstall sections specify additional recovery conventions for those commands; this report does not assume those exact backup filenames also apply to the builtin. ### Impact and scope P2, supported: valid local registration data is persistently deleted during a successful refresh, with no automatic recovery artifact in the tested isolated directories. The demonstrated loss is the synthetic environment map; downstream client failures and real configuration losses were not tested. A preemptive manual workaround was verified in a separate home: copy the settings before registration, then restore that copy. The restored file retained the environment map and the same current launcher. This does not recover values after a run for which no prior copy exists. Other agents, operating systems, and the first affected release remain unverified. ### 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 or native analysis provider was started. ### Target and artifact context Synthetic JSON client settings with one existing REA server, a peer server, and a theme. The stored REA launcher is already the default produced by this revision. No analysis target is involved, and registration does not execute the stored launcher. ### Interface CLI ### Earliest failure stage Setup or provider admission ### Relevant output or evidence The public CLI returned exit 0, empty stderr, and: ```text ✓ Registered rea as MCP server Agents: Amp ``` The resulting entry, read directly from the settings file, was: ```json {"command":"npx","args":["-y","rea-agents@6.8.0","mcp"]} ``` `KEEP_ME` was absent, while the peer and theme retained their values. The directory inspection printed: ```text Amp directory: ["settings.json"] ``` Source evidence at the stated baseline: - `AGENTS.md:11`: the configuration-change invariant. - `docs/installation.md:1100-1109`: REA's documented builtin and its distinction from setup. - `docs/installation.md:73-89,94-105`: setup/uninstall recovery and preservation context, whose command-specific details are not extended here. - `scripts/rea.mjs:9-11,31-40,80-91` and `src/cli.ts:95-100`: public dispatch and REA's supplied default launcher. - `node_modules/incur/src/Cli.ts:1282-1304`: builtin invocation of the registration writer. - `node_modules/incur/src/SyncMcp.ts:22-27,51-55,61-84`: Amp-only branch, existing-file parse, entry replacement, and direct write without recovery. ### Typed outcome or recovery expected After the same valid refresh, `amp.mcpServers.rea.env.KEEP_ME` should still equal `"1"`, the launcher and unrelated settings should remain intact, and a recoverable pre-change copy should be available. If the command cannot preserve the existing entry safely, it should refuse before changing it and explain why. ### Analysis or suspected cause Source-derived: the pinned Incur writer constructs a replacement object containing only the selected command and arguments. It does not merge existing entry fields or create a backup before serializing the settings. The public CLI reproductions confirm this path is reachable through REA. The implementation cause is in a dependency, but REA ships and documents the command and supplies its launcher. The failure therefore also belongs at REA's integration boundary under its configuration invariant; this report does not attribute authorship of the writer to REA. ### Additional context Open and closed issue searches and PR searches found no same-contract duplicate. GitHub CLI search attempts failed with HTTP 401; searches were completed with the connected GitHub tools using explicit open/closed queries for `amp.mcpServers`, Amp titles, and MCP-add environment handling, plus backup, overwrite, preservation, and PR queries. Related reports were read and excluded: [#7375](https://github.com/morluto/rea/issues/7375) and [PR #7453](https://github.com/morluto/rea/pull/7453) concern surplus positional rejection; [#5811](https://github.com/morluto/rea/issues/5811) and [PR #6347](https://github.com/morluto/rea/pull/6347) concern Windows package-runner spawning; closed [#1163](https://github.com/morluto/rea/issues/1163) concerns Codex TOML comments and setup/uninstall backups. None has preservation of an existing Amp entry's environment map as its closure signal.

bsakaguchi referenced this issue

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

[GH-ISSUE #7604] [Bug] evidence-export omits the residual-unknown revision count from its receipt #4534

bsakaguchi referenced this issue

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

[PR #4482] feat(evidence): add evidence bundle audit command and MCP tool #6267

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:11:07 +00:00

[GH-ISSUE #5654] [Tracker] Open PRs that can't merge yet: conflicts and red CI (October 2026) #3425

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

@coygeek commented on GitHub (Oct 31, 2026):

I checked this against latest main. This issue appears resolved. Running mcp add --agent amp no longer silently drops the existing Amp registration's environment map: in src/cli.ts, the CLI now reads ~/.config/amp/settings.json before invoking Incur's registration, captures the existing amp.mcpServers.rea.env, and after registration writes that environment back into the entry if Incur omitted it — so an entry like mine, with env: {"KEEP_ME": "1"} and the already-current default launcher, keeps KEEP_ME after a refresh. The same change addresses the missing recovery copy: src/cli/ampRegistrationBackup.ts now creates a pre-change backup before the update (retained at settings.json.rea.backup, with later changed settings kept as content-addressed recovery copies at owner-only permissions), and the backup path is reported on stderr. Malformed or non-object settings, and backup failures, are now rejected with a validation error before the registration proceeds instead of reporting success after data loss. This was delivered by PR #7995 (merge commit f25d1e3e9, merged into main on 2026-10-29), with boundary test coverage in tests/boundary/cli/ampRegistrationPreservation.test.ts reproducing this exact fixture and asserting both the preserved environment and the retained backup. Closing as resolved.

@coygeek commented on GitHub (Oct 31, 2026): I checked this against latest main. This issue appears resolved. Running `mcp add --agent amp` no longer silently drops the existing Amp registration's environment map: in `src/cli.ts`, the CLI now reads `~/.config/amp/settings.json` before invoking Incur's registration, captures the existing `amp.mcpServers.rea.env`, and after registration writes that environment back into the entry if Incur omitted it — so an entry like mine, with `env: {"KEEP_ME": "1"}` and the already-current default launcher, keeps `KEEP_ME` after a refresh. The same change addresses the missing recovery copy: `src/cli/ampRegistrationBackup.ts` now creates a pre-change backup before the update (retained at `settings.json.rea.backup`, with later changed settings kept as content-addressed recovery copies at owner-only permissions), and the backup path is reported on stderr. Malformed or non-object settings, and backup failures, are now rejected with a validation error before the registration proceeds instead of reporting success after data loss. This was delivered by [PR #7995](https://github.com/morluto/rea/pull/7995) (merge commit [`f25d1e3e9`](https://github.com/morluto/rea/commit/f25d1e3e9fbda7971c312caa3da757dc525a73f6), merged into main on 2026-10-29), with boundary test coverage in `tests/boundary/cli/ampRegistrationPreservation.test.ts` reproducing this exact fixture and asserting both the preserved environment and the retained backup. Closing as resolved.

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#4482

No description provided.

Rows
Columns