morluto/rea

Fork 0

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

[GH-ISSUE #5079] [Bug] Uninstall hides an earlier OpenCode registration removal when a later file fails #3115

New issue

Closed

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

bsakaguchi commented

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

Owner

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

Before submitting

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

Area

Setup, packaging, or upgrade

Observed behavior

The production source uninstall workflow removes REA from opencode.json, then fails to back up opencode.jsonc. Its merged OpenCode item reports failed with “Configuration could not be backed up, so no change was made,” omitting the successful removal from the earlier file. The separate stop item says the items report what changed, but no item reports that removal.

Observed once with the real filesystem and no mocked boundaries. Overall failure is correctly reported. This exercised the application workflow called by the CLI; the packaged CLI process and an actual OpenCode process were not executed.

Reproduction

From a source checkout at the revision below with locked dependencies installed, run this probe. It creates only isolated fixtures and never executes their registered command. An empty directory at the second backup path causes a safe backup failure after the first file is edited.

WORK=$(mktemp -d)
cat > "$WORK/probe.mts" <<'EOF'
import { mkdir, readFile, writeFile, rm } from 'node:fs/promises';
import { join } from 'node:path';
import { pathToFileURL } from 'node:url';
import assert from 'node:assert/strict';
const { runUninstall, systemUninstallHost } = await import(pathToFileURL(join(process.cwd(), 'src/application/Uninstall.ts')).href);
const home = process.env.HOME!;
const directory = join(home, '.config/opencode');
await mkdir(directory, { recursive: true });
const first = join(directory, 'opencode.json');
const second = join(directory, 'opencode.jsonc');
const config = JSON.stringify({ mcp: { rea: { type: 'local', command: ['npx', '-y', 'rea-agents@6.7.0', 'mcp'] } }, unrelated: 'keep' });
await writeFile(first, config);
await writeFile(second, config);
await mkdir(`${second}.rea.backup`);
const result = await runUninstall(false, systemUninstallHost(home, undefined, { HOME: home }, process.platform));
const row = result.items.find((item: any) => item.name === 'opencode');
const firstAfter = JSON.parse(await readFile(first, 'utf8'));
const secondAfter = JSON.parse(await readFile(second, 'utf8'));
assert.equal(result.status, 'failed');
assert.equal(row.status, 'failed');
assert.equal(firstAfter.mcp.rea, undefined);
assert.ok(secondAfter.mcp.rea);
assert.equal(firstAfter.unrelated, 'keep');
assert.equal(secondAfter.unrelated, 'keep');
assert.equal(row.detail.includes('Removed registration'), false);
console.log(JSON.stringify({resultStatus: result.status, opencode: row, firstRegistrationRemoved: !firstAfter.mcp.rea, secondRegistrationPresent: !!secondAfter.mcp.rea, stop: result.items.find((item: any) => item.name === 'uninstall')}, null, 2).replaceAll(home, '$HOME'));
await rm(`${second}.rea.backup`, { recursive: true });
const recovery = await runUninstall(false, systemUninstallHost(home, undefined, { HOME: home }, process.platform));
assert.equal(recovery.status, 'complete');
assert.equal(JSON.parse(await readFile(second, 'utf8')).mcp.rea, undefined);
console.log(JSON.stringify({recoveryStatus: recovery.status, recoveryOpencode: recovery.items.find((item: any) => item.name === 'opencode')}).replaceAll(home, '$HOME'));
EOF
env -i PATH="$PATH" HOME="$WORK/home" node --import tsx "$WORK/probe.mts"

The assertions record the current failure and verify recovery after removing the synthetic obstruction. They do not assert a repaired implementation.

Expected behavior

Preserve the earlier successful removal detail alongside the failure, so callers can determine which configuration changed.

The workflow explicitly promises that after a removal failure, “the items above report what changed” (partial-failure result). The documented OpenCode configuration contract supports multiple applicable user files and uninstalling owned entries from each.

Impact and scope

The caller receives an incomplete, misleading description of a partially removed layered registration. One registration was removed while another remained, yet the only client detail said no change was made. Failure status remained correct, and both files retained their unrelated setting. No corruption or unrecoverable loss was observed.

Removing the empty synthetic backup directory and rerunning removed the remaining registration and completed the operation. Natural incidence and other layered-client cases are NOT_ENOUGH_INFO; only this OpenCode fixture was exercised.

REA version

Source main at 3537e58c1ab29309f06eabf50bc52adba9817a0c, package manifest 6.7.0. Relevant source is unchanged from stable tag rea-agents-6.7.0 in Uninstall.ts, SupportedClients.ts, SetupRecoveryBackup.ts and cli/setupCommands.ts. Stable-package runtime was not tested.

Runtime and platform

Node.js v26.12.0; macOS 27.0.1; arm64. Isolated HOME and explicit host environment. No provider or analysis engine involved.

Target and artifact context

Two valid OpenCode user configuration files with the same REA-owned local registration and an unrelated setting; an empty directory occupying the later file's backup path.

Interface

CLI

Earliest failure stage

Transport or result projection

Relevant output or evidence

The probe exited 0 with all assertions passing. Its first result included:

{
  "resultStatus": "failed",
  "opencode": {
    "name": "opencode",
    "status": "failed",
    "detail": "Configuration could not be backed up, so no change was made. Check file permissions, then rerun uninstall."
  },
  "firstRegistrationRemoved": true,
  "secondRegistrationPresent": true,
  "stop": {
    "name": "uninstall",
    "status": "skipped",
    "detail": "Uninstall stopped at this failure; the items above report what changed. Repair the failed configuration, then rerun uninstall."
  }
}

The recovery run printed recoveryStatus: complete and an OpenCode removed item naming the JSONC file. The CLI handler calls this workflow; CLI formatting and exit status were not measured.

Typed outcome or recovery expected

Retain overall and client failure status and the backup diagnostic while reporting the earlier JSON registration removal. Preserve unrelated settings and the later registration when backup fails. Removing the obstruction and rerunning should still complete removal, as observed in the recovery control.

Analysis or suspected cause

Source-derived explanation: OpenCode path discovery orders JSON before JSONC. Result projection collapses items by client name, chooses the failed status, and filters out the earlier removed item's detail. This accounts for the missing observation without requiring a failure in the removal itself.

Additional context

Searched open and closed issues. Related reports cover different contracts: #1164 concerns malformed-config preflight, #2111 concerns skill destination names, #2664 concerns leaving a fallback registration behind, and #3308 concerns retained skill manifest state. This report concerns losing an already completed client-file removal from the failed result.

Originally created by @coygeek on GitHub (Oct 18, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5079 ## 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 Setup, packaging, or upgrade ## Observed behavior The production source uninstall workflow removes REA from `opencode.json`, then fails to back up `opencode.jsonc`. Its merged OpenCode item reports `failed` with “Configuration could not be backed up, so no change was made,” omitting the successful removal from the earlier file. The separate stop item says the items report what changed, but no item reports that removal. Observed once with the real filesystem and no mocked boundaries. Overall failure is correctly reported. This exercised the application workflow called by the CLI; the packaged CLI process and an actual OpenCode process were not executed. ## Reproduction From a source checkout at the revision below with locked dependencies installed, run this probe. It creates only isolated fixtures and never executes their registered command. An empty directory at the second backup path causes a safe backup failure after the first file is edited. ```sh WORK=$(mktemp -d) cat > "$WORK/probe.mts" <<'EOF' import { mkdir, readFile, writeFile, rm } from 'node:fs/promises'; import { join } from 'node:path'; import { pathToFileURL } from 'node:url'; import assert from 'node:assert/strict'; const { runUninstall, systemUninstallHost } = await import(pathToFileURL(join(process.cwd(), 'src/application/Uninstall.ts')).href); const home = process.env.HOME!; const directory = join(home, '.config/opencode'); await mkdir(directory, { recursive: true }); const first = join(directory, 'opencode.json'); const second = join(directory, 'opencode.jsonc'); const config = JSON.stringify({ mcp: { rea: { type: 'local', command: ['npx', '-y', 'rea-agents@6.7.0', 'mcp'] } }, unrelated: 'keep' }); await writeFile(first, config); await writeFile(second, config); await mkdir(`${second}.rea.backup`); const result = await runUninstall(false, systemUninstallHost(home, undefined, { HOME: home }, process.platform)); const row = result.items.find((item: any) => item.name === 'opencode'); const firstAfter = JSON.parse(await readFile(first, 'utf8')); const secondAfter = JSON.parse(await readFile(second, 'utf8')); assert.equal(result.status, 'failed'); assert.equal(row.status, 'failed'); assert.equal(firstAfter.mcp.rea, undefined); assert.ok(secondAfter.mcp.rea); assert.equal(firstAfter.unrelated, 'keep'); assert.equal(secondAfter.unrelated, 'keep'); assert.equal(row.detail.includes('Removed registration'), false); console.log(JSON.stringify({resultStatus: result.status, opencode: row, firstRegistrationRemoved: !firstAfter.mcp.rea, secondRegistrationPresent: !!secondAfter.mcp.rea, stop: result.items.find((item: any) => item.name === 'uninstall')}, null, 2).replaceAll(home, '$HOME')); await rm(`${second}.rea.backup`, { recursive: true }); const recovery = await runUninstall(false, systemUninstallHost(home, undefined, { HOME: home }, process.platform)); assert.equal(recovery.status, 'complete'); assert.equal(JSON.parse(await readFile(second, 'utf8')).mcp.rea, undefined); console.log(JSON.stringify({recoveryStatus: recovery.status, recoveryOpencode: recovery.items.find((item: any) => item.name === 'opencode')}).replaceAll(home, '$HOME')); EOF env -i PATH="$PATH" HOME="$WORK/home" node --import tsx "$WORK/probe.mts" ``` The assertions record the current failure and verify recovery after removing the synthetic obstruction. They do not assert a repaired implementation. ## Expected behavior Preserve the earlier successful removal detail alongside the failure, so callers can determine which configuration changed. The workflow explicitly promises that after a removal failure, “the items above report what changed” ([partial-failure result](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/application/Uninstall.ts#L204-L221)). The [documented OpenCode configuration contract](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/docs/client-configuration.md#L416-L424) supports multiple applicable user files and uninstalling owned entries from each. ## Impact and scope The caller receives an incomplete, misleading description of a partially removed layered registration. One registration was removed while another remained, yet the only client detail said no change was made. Failure status remained correct, and both files retained their unrelated setting. No corruption or unrecoverable loss was observed. Removing the empty synthetic backup directory and rerunning removed the remaining registration and completed the operation. Natural incidence and other layered-client cases are NOT_ENOUGH_INFO; only this OpenCode fixture was exercised. ## REA version Source main at `3537e58c1ab29309f06eabf50bc52adba9817a0c`, package manifest 6.7.0. Relevant source is unchanged from stable tag `rea-agents-6.7.0` in Uninstall.ts, SupportedClients.ts, SetupRecoveryBackup.ts and cli/setupCommands.ts. Stable-package runtime was not tested. ## Runtime and platform Node.js v26.12.0; macOS 27.0.1; arm64. Isolated HOME and explicit host environment. No provider or analysis engine involved. ## Target and artifact context Two valid OpenCode user configuration files with the same REA-owned local registration and an unrelated setting; an empty directory occupying the later file's backup path. ## Interface CLI ## Earliest failure stage Transport or result projection ## Relevant output or evidence The probe exited 0 with all assertions passing. Its first result included: ```json { "resultStatus": "failed", "opencode": { "name": "opencode", "status": "failed", "detail": "Configuration could not be backed up, so no change was made. Check file permissions, then rerun uninstall." }, "firstRegistrationRemoved": true, "secondRegistrationPresent": true, "stop": { "name": "uninstall", "status": "skipped", "detail": "Uninstall stopped at this failure; the items above report what changed. Repair the failed configuration, then rerun uninstall." } } ``` The recovery run printed `recoveryStatus: complete` and an OpenCode `removed` item naming the JSONC file. The [CLI handler](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/cli/setupCommands.ts#L146-L170) calls this workflow; CLI formatting and exit status were not measured. ## Typed outcome or recovery expected Retain overall and client failure status and the backup diagnostic while reporting the earlier JSON registration removal. Preserve unrelated settings and the later registration when backup fails. Removing the obstruction and rerunning should still complete removal, as observed in the recovery control. ## Analysis or suspected cause Source-derived explanation: [OpenCode path discovery](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/application/SupportedClients.ts#L388-L407) orders JSON before JSONC. [Result projection](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/application/Uninstall.ts#L1018-L1050) collapses items by client name, chooses the failed status, and filters out the earlier removed item's detail. This accounts for the missing observation without requiring a failure in the removal itself. ## Additional context Searched open and closed issues. Related reports cover different contracts: [#1164](https://github.com/morluto/rea/issues/1164) concerns malformed-config preflight, [#2111](https://github.com/morluto/rea/issues/2111) concerns skill destination names, [#2664](https://github.com/morluto/rea/issues/2664) concerns leaving a fallback registration behind, and [#3308](https://github.com/morluto/rea/issues/3308) concerns retained skill manifest state. This report concerns losing an already completed client-file removal from the failed result.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

@morluto commented on GitHub (Oct 18, 2026):

Fixed on main in 559af3d3ff.

Root cause: per-file removals were collapsed into one client row by selecting the highest-priority status and discarding other statuses' details. A later failed backup therefore erased an earlier completed removal from the report.

The workflow now preserves each candidate configuration path and typed constituent outcome. A layered client exposes actions containing each path/status/detail, while its aggregate status keeps the existing failed > removed > retained > skipped precedence. The summary derives from all leaf details; a second aggregation after final cleanup flattens existing actions rather than replacing or duplicating their facts. Single-file rows gain the additive configuration_path; those paths identify discovered candidates rather than asserting physical-file identity.

Validation: actual isolated filesystem regression was RED before the fix, confirming JSON removal and unchanged blocked JSONC before asserting the missing report. A compiled public CLI regression also failed before the fix. The final 160 tests across seven files passed, including real CLI failure exit, structured outcomes, preserved unrelated data/backups, and successful retry. The final filesystem rerun passed 96 tests after test grouping and path assertions were updated. The source workflow additionally simulates a final cleanup diagnostic after real resource closure to verify recollapse; it does not claim a real resource-cleanup failure. Full static/architecture checks, docs (5 tests), discovery (1596 files), commit hooks and pre-push checks passed. Existing strict shape tests now assert the additive path rather than dropping their validation.

Cost observations: 12 compiled CLI invocations compared the previous commit against this change: two owned OpenCode layers removed, then idempotent repeat, three samples per variant. File effects, backups, aggregate outcomes and exit codes matched; the candidate additionally retained per-file outcomes. Two warm samples per step had descriptive medians: initial removal 0.345 -> 0.321 s elapsed, 0.534 -> 0.531 s CPU, 355688448 -> 359571456 bytes sampled family RSS, 1148 -> 2509 estimated cl100k JSON tokens. Repeat 0.343 -> 0.344 s elapsed, 0.525 -> 0.537 s CPU, 346284032 -> 357826560 bytes RSS, 1085 -> 2366 tokens. The extra structured paths/actions increase output; these are small local samples, not speedup claims. First processes used empty compile caches, not flushed OS caches. Actual model context, human text rendering and fault/recovery costs were not measured.

Limits and adjacent review: macOS fixtures and controlled final-cleanup diagnostic; no actual OpenCode process, packaged-release or cross-platform runtime qualification. A bounded source scan of setup, update and skill-removal aggregation found no further confirmed failure-side fact loss. Their broader behavior is not certified by this fix. CI is background; latest-head remote success is not claimed.

@morluto commented on GitHub (Oct 18, 2026): Fixed on main in 559af3d3ff4b0236d468e80788c82d5a85b102da. Root cause: per-file removals were collapsed into one client row by selecting the highest-priority status and discarding other statuses' details. A later failed backup therefore erased an earlier completed removal from the report. The workflow now preserves each candidate configuration path and typed constituent outcome. A layered client exposes `actions` containing each path/status/detail, while its aggregate status keeps the existing failed > removed > retained > skipped precedence. The summary derives from all leaf details; a second aggregation after final cleanup flattens existing actions rather than replacing or duplicating their facts. Single-file rows gain the additive `configuration_path`; those paths identify discovered candidates rather than asserting physical-file identity. Validation: actual isolated filesystem regression was RED before the fix, confirming JSON removal and unchanged blocked JSONC before asserting the missing report. A compiled public CLI regression also failed before the fix. The final 160 tests across seven files passed, including real CLI failure exit, structured outcomes, preserved unrelated data/backups, and successful retry. The final filesystem rerun passed 96 tests after test grouping and path assertions were updated. The source workflow additionally simulates a final cleanup diagnostic after real resource closure to verify recollapse; it does not claim a real resource-cleanup failure. Full static/architecture checks, docs (5 tests), discovery (1596 files), commit hooks and pre-push checks passed. Existing strict shape tests now assert the additive path rather than dropping their validation. Cost observations: 12 compiled CLI invocations compared the previous commit against this change: two owned OpenCode layers removed, then idempotent repeat, three samples per variant. File effects, backups, aggregate outcomes and exit codes matched; the candidate additionally retained per-file outcomes. Two warm samples per step had descriptive medians: initial removal 0.345 -> 0.321 s elapsed, 0.534 -> 0.531 s CPU, 355688448 -> 359571456 bytes sampled family RSS, 1148 -> 2509 estimated cl100k JSON tokens. Repeat 0.343 -> 0.344 s elapsed, 0.525 -> 0.537 s CPU, 346284032 -> 357826560 bytes RSS, 1085 -> 2366 tokens. The extra structured paths/actions increase output; these are small local samples, not speedup claims. First processes used empty compile caches, not flushed OS caches. Actual model context, human text rendering and fault/recovery costs were not measured. Limits and adjacent review: macOS fixtures and controlled final-cleanup diagnostic; no actual OpenCode process, packaged-release or cross-platform runtime qualification. A bounded source scan of setup, update and skill-removal aggregation found no further confirmed failure-side fact loss. Their broader behavior is not certified by this fix. CI is background; latest-head remote success is not claimed.

bsakaguchi referenced this issue

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

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

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

No description provided.

Rows
Columns