morluto/rea

Fork 0

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

[GH-ISSUE #5969] [Bug] JavaScript recovery reports a changed selected input as unreadable output #3638

New issue

Closed

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

bsakaguchi commented

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

Owner

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

Before submitting

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

Area

Evidence, artifacts, or reconstruction

Observed behavior

An executed recover_javascript_sources MCP call returned unreadable_output, category execution_failure, retryable: false, and rea doctor guidance after the selected original JavaScript file changed during recovery. The retained reason correctly says Original input changed during recovery. The simulated producer supplied valid output; the failure came from the original-input identity check.

The ordinary control succeeded. The failed recovery rolled back its output and removed its owned process groups and workspaces.

Reproduction

In a prepared source checkout with locked dependencies and existing build artifacts, place the following test at tests/boundary/javascript-recovery/recoveryErrorClassification.repro.test.ts, then run it with Node.js 24.18.0:

node node_modules/vitest/vitest.mjs run tests/boundary/javascript-recovery/recoveryErrorClassification.repro.test.ts --maxWorkers=1

This uses the repository's simulated producer and real owned Node subprocesses. Wakaru was not installed. The test temporarily simulates Linux x64 only while the real command-admission function runs, restoring the host descriptors before subprocess creation. A local executable substitutes for limiter admission; no util-linux resource containment or real Wakaru behavior is claimed. The normal recovery control succeeds through the same MCP registration and SDK transports. The complete observation run contained seven passing assertions; the relevant subset is below. The assertions record the current incorrect result so a passing test here means the behavior reproduced.

import { Client, InMemoryTransport } from '@modelcontextprotocol/client';
import { access, chmod, readFile } from 'node:fs/promises';
import { join } from 'node:path';
import { expect, it, onTestFinished, vi } from 'vitest';
import * as commandModule from '../../../src/javascript/recovery/WakaruCommand.js';
import { WakaruProvider } from '../../../src/javascript/recovery/WakaruProvider.js';
import { JavaScriptRecoveryService } from '../../../src/application/javascript/JavaScriptRecoveryService.js';
import { projectAnalysisError } from '../../../src/domain/analysisErrorProjection.js';
import { createServer } from '../../../src/server/createServer.js';
import { createTestBinarySession } from '../../fixtures/binarySession.js';
import { recoveryFixture, assertRecoveryCleanup } from '../../fixtures/javascript-recovery/provider.js';

// Only platform admission is simulated; real filesystem and owned Node processes follow.
const originalResolve = commandModule.resolveWakaruCommand;
const setup = async (mode = 'normal', limiter?: string) => {
  const fixture = await recoveryFixture(mode);
  const provider = fixture.provider as WakaruProvider;
  Object.assign(provider.environment, {
    REA_JAVASCRIPT_PRLIMIT_COMMAND: limiter ?? join(fixture.root, 'fixture-wakaru'),
  });
  const spy = vi.spyOn(commandModule, 'resolveWakaruCommand').mockImplementation(async env => {
    const platform = Object.getOwnPropertyDescriptor(process, 'platform')!;
    const arch = Object.getOwnPropertyDescriptor(process, 'arch')!;
    Object.defineProperty(process, 'platform', { ...platform, value: 'linux' });
    Object.defineProperty(process, 'arch', { ...arch, value: 'x64' });
    try { return await originalResolve(env); }
    finally {
      Object.defineProperty(process, 'platform', platform);
      Object.defineProperty(process, 'arch', arch);
    }
  });
  onTestFinished(async () => { spy.mockRestore(); await provider.close(); });
  return { ...fixture, provider };
};
const callMcp = async (fixture: Awaited<ReturnType<typeof setup>>) => {
  const session = createTestBinarySession(() => { throw new Error('No native provider expected'); });
  const server = createServer({ kind: 'session', session }, { javascriptRecovery: fixture.provider });
  const client = new Client({ name: 'recovery-error-test', version: '1' });
  const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
  try {
    await server.connect(serverTransport); await client.connect(clientTransport);
    return await client.callTool({ name: 'recover_javascript_sources', arguments: {
      path: fixture.path, output_directory: fixture.output,
    }});
  } finally { await client.close(); await server.close(); await session.close(); }
};
it('control: normal simulated recovery succeeds through MCP', async () => {
  const fixture = await setup();
  const response = await callMcp(fixture);
  console.log('control', JSON.stringify({ isError: response.isError ?? false }));
  expect(response.isError).not.toBe(true);
  await assertRecoveryCleanup(fixture.launches);
});
it('changed input is projected as unreadable output through MCP', async () => {
  const fixture = await setup('source-change');
  const response = await callMcp(fixture);
  console.log('source-change', JSON.stringify(response));
  expect(response.isError).toBe(true);
  expect(JSON.stringify(response)).toContain('unreadable_output');
  expect(JSON.stringify(response)).toContain('Original input changed during recovery');
  await expect(access(fixture.output)).rejects.toMatchObject({ code: 'ENOENT' });
  await assertRecoveryCleanup(fixture.launches);
});

Expected behavior

The recovery contract requires rejecting original-source changes before commit. That refusal should retain the observed selected-artifact identity change using the existing AnalysisArtifactChangedError semantics: artifact_changed, integrity_mismatch, a structured selected path/reason, and retry guidance after the file stabilizes. The shared remediation already distinguishes that condition from unreadable provider output.

Impact and scope

P2: clients branching on typed outcomes receive a permanent-looking provider-output failure for a selected-file identity change. The response directs them to provider diagnostics rather than waiting for stable input. The reason text preserves enough information for a caller to identify the actual condition manually. No stale successful Evidence or source corruption was observed; rollback worked. This run covered one primary input only, with a simulated producer rather than real Wakaru.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

macOS 27.0.1 arm64; Node.js v24.18.0 (repository pin), Vitest 5.0.3. An earlier observation run also completed on Node.js v24.22.0. No real Wakaru binary was used.

Target and artifact context

One regular UTF-8 JavaScript file. The repository producer's source-change mode rewrites the original selected file after REA has snapshotted it, while producing a structurally valid recovery report.

Interface

MCP

Earliest failure stage

Native analysis or reconstruction

Relevant output or evidence

Relevant fields extracted from the actual MCP text error, with the generated fixture location replaced by <fixture>:

{
  "code": "unreadable_output",
  "category": "execution_failure",
  "retryable": false,
  "details": {
    "operation": "recover_javascript_sources",
    "reason": "Original input changed during recovery: <fixture>/selected.js"
  }
}

remediation.action said: Analysis returned an unreadable result. Run followed by rea doctor and guidance that repeating the unchanged request is not expected to succeed. The same test verified absent output and successful owned cleanup.

Typed outcome or recovery expected

Report the selected input change with AnalysisArtifactChangedError, code: "artifact_changed", category: "integrity_mismatch", retryable: true, selected path and reason, and stable-file recovery guidance. Keep rejection and rollback intact.

Analysis or suspected cause

The public MCP registration forwards inputs and execution options to the shared recovery service. The final original-source re-fingerprint recognizes a digest/size mismatch but throws AnalysisOutputError. The provider preserves this typed error after cleanup, so projection cannot recover the selected-artifact classification. This is source-derived explanation of the executed result.

Additional context

#4629 and PR #4757 concern input-size/UTF-8 admission; #3332 concerns FIFO blocking; #4948 and PR #5324 concern host availability. Their repairs differ. The configured-engine change check and additional-input races were not exercised and are outside this report's observed scope. Upstream main 25e1a24a3ed3671a0148757555f18d16c29d936a was fetched; the relevant recovery source and classification mappings remain unchanged. That newer revision was not executed. No fix was applied.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5969 ### 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 Evidence, artifacts, or reconstruction ### Observed behavior An executed `recover_javascript_sources` MCP call returned `unreadable_output`, category `execution_failure`, `retryable: false`, and `rea doctor` guidance after the selected original JavaScript file changed during recovery. The retained reason correctly says `Original input changed during recovery`. The simulated producer supplied valid output; the failure came from the original-input identity check. The ordinary control succeeded. The failed recovery rolled back its output and removed its owned process groups and workspaces. ### Reproduction In a prepared source checkout with locked dependencies and existing build artifacts, place the following test at `tests/boundary/javascript-recovery/recoveryErrorClassification.repro.test.ts`, then run it with Node.js 24.18.0: ```sh node node_modules/vitest/vitest.mjs run tests/boundary/javascript-recovery/recoveryErrorClassification.repro.test.ts --maxWorkers=1 ``` This uses the repository's simulated producer and real owned Node subprocesses. Wakaru was not installed. The test temporarily simulates Linux x64 only while the real command-admission function runs, restoring the host descriptors before subprocess creation. A local executable substitutes for limiter admission; no util-linux resource containment or real Wakaru behavior is claimed. The normal recovery control succeeds through the same MCP registration and SDK transports. The complete observation run contained seven passing assertions; the relevant subset is below. The assertions record the current incorrect result so a passing test here means the behavior reproduced. ```ts import { Client, InMemoryTransport } from '@modelcontextprotocol/client'; import { access, chmod, readFile } from 'node:fs/promises'; import { join } from 'node:path'; import { expect, it, onTestFinished, vi } from 'vitest'; import * as commandModule from '../../../src/javascript/recovery/WakaruCommand.js'; import { WakaruProvider } from '../../../src/javascript/recovery/WakaruProvider.js'; import { JavaScriptRecoveryService } from '../../../src/application/javascript/JavaScriptRecoveryService.js'; import { projectAnalysisError } from '../../../src/domain/analysisErrorProjection.js'; import { createServer } from '../../../src/server/createServer.js'; import { createTestBinarySession } from '../../fixtures/binarySession.js'; import { recoveryFixture, assertRecoveryCleanup } from '../../fixtures/javascript-recovery/provider.js'; // Only platform admission is simulated; real filesystem and owned Node processes follow. const originalResolve = commandModule.resolveWakaruCommand; const setup = async (mode = 'normal', limiter?: string) => { const fixture = await recoveryFixture(mode); const provider = fixture.provider as WakaruProvider; Object.assign(provider.environment, { REA_JAVASCRIPT_PRLIMIT_COMMAND: limiter ?? join(fixture.root, 'fixture-wakaru'), }); const spy = vi.spyOn(commandModule, 'resolveWakaruCommand').mockImplementation(async env => { const platform = Object.getOwnPropertyDescriptor(process, 'platform')!; const arch = Object.getOwnPropertyDescriptor(process, 'arch')!; Object.defineProperty(process, 'platform', { ...platform, value: 'linux' }); Object.defineProperty(process, 'arch', { ...arch, value: 'x64' }); try { return await originalResolve(env); } finally { Object.defineProperty(process, 'platform', platform); Object.defineProperty(process, 'arch', arch); } }); onTestFinished(async () => { spy.mockRestore(); await provider.close(); }); return { ...fixture, provider }; }; const callMcp = async (fixture: Awaited<ReturnType<typeof setup>>) => { const session = createTestBinarySession(() => { throw new Error('No native provider expected'); }); const server = createServer({ kind: 'session', session }, { javascriptRecovery: fixture.provider }); const client = new Client({ name: 'recovery-error-test', version: '1' }); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); try { await server.connect(serverTransport); await client.connect(clientTransport); return await client.callTool({ name: 'recover_javascript_sources', arguments: { path: fixture.path, output_directory: fixture.output, }}); } finally { await client.close(); await server.close(); await session.close(); } }; it('control: normal simulated recovery succeeds through MCP', async () => { const fixture = await setup(); const response = await callMcp(fixture); console.log('control', JSON.stringify({ isError: response.isError ?? false })); expect(response.isError).not.toBe(true); await assertRecoveryCleanup(fixture.launches); }); it('changed input is projected as unreadable output through MCP', async () => { const fixture = await setup('source-change'); const response = await callMcp(fixture); console.log('source-change', JSON.stringify(response)); expect(response.isError).toBe(true); expect(JSON.stringify(response)).toContain('unreadable_output'); expect(JSON.stringify(response)).toContain('Original input changed during recovery'); await expect(access(fixture.output)).rejects.toMatchObject({ code: 'ENOENT' }); await assertRecoveryCleanup(fixture.launches); }); ``` ### Expected behavior The [recovery contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/javascript-recovery.md#L79-L86) requires rejecting original-source changes before commit. That refusal should retain the observed selected-artifact identity change using the existing [AnalysisArtifactChangedError](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorCore.ts#L204-L218) semantics: `artifact_changed`, `integrity_mismatch`, a structured selected path/reason, and retry guidance after the file stabilizes. The [shared remediation](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorPresentation.ts#L125-L130) already distinguishes that condition from unreadable provider output. ### Impact and scope P2: clients branching on typed outcomes receive a permanent-looking provider-output failure for a selected-file identity change. The response directs them to provider diagnostics rather than waiting for stable input. The reason text preserves enough information for a caller to identify the actual condition manually. No stale successful Evidence or source corruption was observed; rollback worked. This run covered one primary input only, with a simulated producer rather than real Wakaru. ### REA version rea-agents 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform macOS 27.0.1 arm64; Node.js v24.18.0 (repository pin), Vitest 5.0.3. An earlier observation run also completed on Node.js v24.22.0. No real Wakaru binary was used. ### Target and artifact context One regular UTF-8 JavaScript file. The repository producer's source-change mode rewrites the original selected file after REA has snapshotted it, while producing a structurally valid recovery report. ### Interface MCP ### Earliest failure stage Native analysis or reconstruction ### Relevant output or evidence Relevant fields extracted from the actual MCP text error, with the generated fixture location replaced by `<fixture>`: ```json { "code": "unreadable_output", "category": "execution_failure", "retryable": false, "details": { "operation": "recover_javascript_sources", "reason": "Original input changed during recovery: <fixture>/selected.js" } } ``` `remediation.action` said: `Analysis returned an unreadable result. Run` followed by `rea doctor` and guidance that repeating the unchanged request is not expected to succeed. The same test verified absent output and successful owned cleanup. ### Typed outcome or recovery expected Report the selected input change with `AnalysisArtifactChangedError`, `code: "artifact_changed"`, `category: "integrity_mismatch"`, `retryable: true`, selected path and reason, and stable-file recovery guidance. Keep rejection and rollback intact. ### Analysis or suspected cause The public [MCP registration](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerJavaScriptRecoveryTool.ts#L19-L33) forwards inputs and execution options to the shared recovery service. The final [original-source re-fingerprint](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/javascript/recovery/WakaruExecution.ts#L131-L142) recognizes a digest/size mismatch but throws `AnalysisOutputError`. The provider preserves this typed error after cleanup, so projection cannot recover the selected-artifact classification. This is source-derived explanation of the executed result. ### Additional context #4629 and PR #4757 concern input-size/UTF-8 admission; #3332 concerns FIFO blocking; #4948 and PR #5324 concern host availability. Their repairs differ. The configured-engine change check and additional-input races were not exercised and are outside this report's observed scope. Upstream main `25e1a24a3ed3671a0148757555f18d16c29d936a` was fetched; the relevant recovery source and classification mappings remain unchanged. That newer revision was not executed. No fix was applied.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

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

Checked resolved on main at d87e73f2db.

I ran the focused recovery cases on Linux x64 with pinned Node 24.22.1. The normal producer control succeeds; source-change, secondary-change and redistribution cases now reject as artifact_changed, roll back publication, and confirm their owned processes and workspaces are gone. The producing boundary retains the changed selected path and identity facts. These checks use the repository's simulated producer and real owned subprocesses, rather than a real Wakaru installation.

Relevant implementation: WakaruExecution.ts, analysisErrorProjection.ts.

Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked resolved on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. I ran the focused recovery cases on Linux x64 with pinned Node 24.22.1. The normal producer control succeeds; source-change, secondary-change and redistribution cases now reject as `artifact_changed`, roll back publication, and confirm their owned processes and workspaces are gone. The producing boundary retains the changed selected path and identity facts. These checks use the repository's simulated producer and real owned subprocesses, rather than a real Wakaru installation. Relevant implementation: [WakaruExecution.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/javascript/recovery/WakaruExecution.ts#L150), [analysisErrorProjection.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/domain/analysisErrorProjection.ts#L277). Closing as completed on main.

bsakaguchi referenced this issue

2026-10-31 00:44:27 +00:00

[GH-ISSUE #6380] [Bug] TypeScript member keys, enum members, meta properties, and import attribute keys create false resolved lexical reads #3831

bsakaguchi referenced this issue

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

[PR #3638] [MERGED] fix(javascript): exclude class member keys from lexical reads #5860

bsakaguchi referenced this issue

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

[PR #6384] [CLOSED] fix(javascript): exclude type, enum and meta-property names from lexical reads #7325