morluto/rea

Fork 0

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

[GH-ISSUE #6084] [Bug] Source-map cleanup remains incomplete after the command successfully retries removal #3722

New issue

Closed

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

bsakaguchi commented

2026-10-31 00:43:22 +00:00

Owner

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

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

The trace-web-source-location CLI callback returns cleanup_incomplete even after its own final service.close successfully removes the codec's private directory. The real built codec succeeds. Its process supervisor confirms verified-cleanup, and the PID is absent. Only the first runtime-directory removal is made to fail; the second succeeds during the same command's finalization. The final response still lists the deleted directory as incomplete.

Reproduction

I ran a temporary focused Vitest test from the built repository using its locked dependencies and one worker. It invokes the actual registered CLI command callback with valid command arguments; it does not launch a second CLI argument-parser process or an MCP client. Production services and error projection run unchanged. The test captures command registration to call that callback, and controls only resource-boundary failures and, for JavaScript, the reported parent heap headroom. No analyzed application JavaScript executes.

For replay, place the following test in the repository's tests/boundary directory, set REPRO_DIR to an existing disposable absolute directory, and run node node_modules/vitest/vitest.mjs run tests/boundary/cleanupProbe.test.ts --maxWorkers 1. The source-runner fileURLToPath adaptation selects the already-built trusted codec process, rather than a nonexistent emitted .js file beside the TypeScript source. Imports and helper setup are shared with the executed three-case probe; this block retains just the relevant case and substitutes REPRO_DIR for the local evidence directory.

import { access, mkdir, writeFile, rm } from "node:fs/promises";
import { join } from "node:path";
import { expect, it, vi } from "vitest";
import pino from "pino";
const state = vi.hoisted(() => ({ budget: 0, fileFault: false, removals: 0 }));
vi.mock("node:v8", async (original) => {
  const real = await original<typeof import("node:v8")>();
  return { ...real, getHeapStatistics: () => ({...real.getHeapStatistics(),
    ...(state.budget ? {heap_size_limit: process.memoryUsage().heapUsed + 192*1024*1024 + state.budget*2} : {}) }) };
});
vi.mock("node:fs/promises", async (original) => {
  const real = await original<typeof import("node:fs/promises")>();
  return {...real, rm: async (...args: Parameters<typeof real.rm>) => {
    if (state.fileFault && String(args[0]).includes("rea-javascript-analysis-") && String(args[0]).endsWith(".source")) {
      state.removals++; throw Object.assign(new Error("controlled source-file removal failure"), {code:"EACCES"});
    }
    return real.rm(...args);
  }};
});
vi.mock("node:url", async (original) => {
  const real = await original<typeof import("node:url")>();
  return {...real, fileURLToPath: (...args: Parameters<typeof real.fileURLToPath>) => real.fileURLToPath(...args).replace("/src/javascript/sourceMaps/SourceMapCodecProcess.js", "/dist/javascript/sourceMaps/SourceMapCodecProcess.js")};
});
import { ProviderProcessSupervisor } from "../../src/process/ProviderProcess.js";
import { PrivateRuntimeRoot } from "../../src/process/PrivateRuntimeRoot.js";
import { registerWebSourceLocationCommands } from "../../src/cli/webSourceLocationCommands.js";
import { registerElectronCommands } from "../../src/cli/electronCommands.js";
import { webSourceLocationFixture } from "../fixtures/webSourceLocation.js";
const scratch = process.env.REPRO_DIR!;
const observeStops = () => {
  const stops:any[]=[]; const original=ProviderProcessSupervisor.prototype.stop;
  const spy=vi.spyOn(ProviderProcessSupervisor.prototype,"stop").mockImplementation(async function(this:ProviderProcessSupervisor,...args) {
    const pid=this.launch.process.pid; const result=await original.apply(this,args);
    let absent=false; try {if(pid) process.kill(pid,0);} catch(cause:any) {absent=cause.code==="ESRCH";}
    stops.push({pid,status:result.status,absent}); return result;
  }); return {stops,spy};
};
const logger = pino({level:"silent"});
const registry = () => { const commands = new Map<string,any>(); return {commands, cli: { command(name:string, spec:any) {commands.set(name,spec); return this;} } as any}; };

const makeWebFixture = async () => {
  const workspace = join(scratch,"source-map-fixture");
  await mkdir(join(workspace,"files","modules"),{recursive:true});
  const fixture = webSourceLocationFixture();
  await writeFile(join(workspace,"files","modules","main.js"),fixture.source);
  await writeFile(join(workspace,"manifest.json"),JSON.stringify({...fixture.manifest,output_directory:workspace,analysis_input:{input_path:workspace,format:"directory"}}));
  await writeFile(join(workspace,"app.map"),fixture.sourceMap.text);
  return {workspace,fixture};
};

it("source-map command reports stale cleanup after a successful physical retry", async () => {
  const {workspace,fixture}=await makeWebFixture();
  const observed=observeStops(); let attempts=0; let root="";
  const original = PrivateRuntimeRoot.prototype.close;
  const spy=vi.spyOn(PrivateRuntimeRoot.prototype,"close").mockImplementation(async function(this:PrivateRuntimeRoot) {
    if(this.path.includes("rea-source-map-")) {root=this.path; attempts++; if(attempts===1) throw new Error("controlled runtime removal failure");}
    return original.call(this);
  });
  try {
    const {commands,cli}=registry(); registerWebSourceLocationCommands(cli,logger,process.env);
    const result=await commands.get("trace-web-source-location").run({args:{manifestPath:join(workspace,"manifest.json"),scriptIndex:0,sourceMapPath:join(workspace,"app.map"),sourceMapUrl:fixture.sourceMap.url,line:1,column:0}});
    expect(attempts).toBe(2);
    await expect(access(root)).rejects.toMatchObject({code:"ENOENT"});
    expect(result.code).toBe("cleanup_incomplete");
    expect(result.details.resources).toContain(root); expect(result.details.diagnostics.previous_error).toBeNull(); expect(observed.stops).toEqual([{pid:expect.any(Number),status:"verified-cleanup",absent:true}]);
    console.log("SOURCE_MAP_RETRY",JSON.stringify({attempts,rootGone:true,stops:observed.stops,result}));
    await writeFile(join(scratch,"source-map-retry-output.json"),JSON.stringify(result,null,2));
  } finally {spy.mockRestore(); observed.spy.mockRestore();}
});

Expected behavior

Once the exact codec process and runtime directory have been confirmed released, every error bound to that owner should clear its current cleanupIncomplete flag and unresolved resources while retaining historical diagnostics. The owner/retry contract requires this, and the CLI finalizer explicitly waits for owners to resolve errors before projection. This report does not require the discarded successful codec result to be reconstructed after recovery.

Impact and scope

P2: machine-readable output claims a current cleanup failure after confirmed release and directs the caller toward resources that no longer exist. The same one-shot close completed successfully, so another cleanup attempt cannot resolve the already-returned marker. The trigger was a controlled transient directory-removal failure; workload frequency and downstream admission decisions were not measured. No live process or directory leak remains in this reproduction.

REA version

rea-agents 6.7.0 source build; revision 5241597594.

Runtime and platform

macOS 27.0.1 arm64, Node.js v26.12.0, existing locked dependencies and built runtime. The development Node.js 24.18.0 pin was not separately exercised.

Target and artifact context

Inert local JavaScript sources; the repository webSourceLocationFixture: retained generated script and a version-3 map containing mappings AAAA, sources original.ts and embedded source text original. No browser required.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed result, with the runtime path and run token replaced by placeholders:

{"runtime_close_attempts":2,"process_stop":"verified-cleanup","pid_absent":true,"root_exists":false,"final_code":"cleanup_incomplete","final_cleanup":"incomplete","resources":["<released-codec-run>","<deleted-runtime-root>"],"diagnostics":{"previous_error":null,"codec_pid":null}}

The second close called the original physical removal method; access(root) then failed with ENOENT before the projected error was inspected. previous_error null establishes that codec execution itself succeeded.

Typed outcome or recovery expected

After confirmed owner release, report no current incomplete-cleanup marker or unresolved resources; historical diagnostics may remain.

Analysis or suspected cause

SourceMapDecoder retains only cleanupOwner and successful retry clears only pending ownership. The SourceMapCleanupFailure is not bound to a release epoch; error projection consequently reads its stale cleanupIncomplete state. The exact owner needs to update every originating/retry error when release is confirmed.

Additional context

Related #6085 concerns a different state: the runtime directory still needs cleanup, but the resource list includes an already-released codec run. This report concerns stale cleanup status after both owners have been released.

Checked #5973, #5983, #5987, #5991, #5995, #6061, #6063, #3671, #5388, #4932, #4628 and #1653 / merged PR #1654. Related “cleanup stays incomplete” report: #6063 requires a different managed-descriptor producer to bind its cleanup errors; #4932 concerns the browser-backed module resolver, and #4628 concerns projection after cleanup has already resolved. This source-map codec owner never resolves the error in the first place. Fetched upstream main f86206535f; the source-map cleanup implementation is unchanged, and the JavaScript per-file early return still bypasses projection-limit composition. Newer worker reuse/CLI-scope changes do not alter that branch. Only the pinned revision was executed. No last-known-good version or external MCP execution was established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6084 ### 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 The trace-web-source-location CLI callback returns cleanup_incomplete even after its own final service.close successfully removes the codec's private directory. The real built codec succeeds. Its process supervisor confirms verified-cleanup, and the PID is absent. Only the first runtime-directory removal is made to fail; the second succeeds during the same command's finalization. The final response still lists the deleted directory as incomplete. ### Reproduction I ran a temporary focused Vitest test from the built repository using its locked dependencies and one worker. It invokes the actual registered CLI command callback with valid command arguments; it does not launch a second CLI argument-parser process or an MCP client. Production services and error projection run unchanged. The test captures command registration to call that callback, and controls only resource-boundary failures and, for JavaScript, the reported parent heap headroom. No analyzed application JavaScript executes. For replay, place the following test in the repository's tests/boundary directory, set REPRO_DIR to an existing disposable absolute directory, and run node node_modules/vitest/vitest.mjs run tests/boundary/cleanupProbe.test.ts --maxWorkers 1. The source-runner fileURLToPath adaptation selects the already-built trusted codec process, rather than a nonexistent emitted .js file beside the TypeScript source. Imports and helper setup are shared with the executed three-case probe; this block retains just the relevant case and substitutes REPRO_DIR for the local evidence directory. ```ts import { access, mkdir, writeFile, rm } from "node:fs/promises"; import { join } from "node:path"; import { expect, it, vi } from "vitest"; import pino from "pino"; const state = vi.hoisted(() => ({ budget: 0, fileFault: false, removals: 0 })); vi.mock("node:v8", async (original) => { const real = await original<typeof import("node:v8")>(); return { ...real, getHeapStatistics: () => ({...real.getHeapStatistics(), ...(state.budget ? {heap_size_limit: process.memoryUsage().heapUsed + 192*1024*1024 + state.budget*2} : {}) }) }; }); vi.mock("node:fs/promises", async (original) => { const real = await original<typeof import("node:fs/promises")>(); return {...real, rm: async (...args: Parameters<typeof real.rm>) => { if (state.fileFault && String(args[0]).includes("rea-javascript-analysis-") && String(args[0]).endsWith(".source")) { state.removals++; throw Object.assign(new Error("controlled source-file removal failure"), {code:"EACCES"}); } return real.rm(...args); }}; }); vi.mock("node:url", async (original) => { const real = await original<typeof import("node:url")>(); return {...real, fileURLToPath: (...args: Parameters<typeof real.fileURLToPath>) => real.fileURLToPath(...args).replace("/src/javascript/sourceMaps/SourceMapCodecProcess.js", "/dist/javascript/sourceMaps/SourceMapCodecProcess.js")}; }); import { ProviderProcessSupervisor } from "../../src/process/ProviderProcess.js"; import { PrivateRuntimeRoot } from "../../src/process/PrivateRuntimeRoot.js"; import { registerWebSourceLocationCommands } from "../../src/cli/webSourceLocationCommands.js"; import { registerElectronCommands } from "../../src/cli/electronCommands.js"; import { webSourceLocationFixture } from "../fixtures/webSourceLocation.js"; const scratch = process.env.REPRO_DIR!; const observeStops = () => { const stops:any[]=[]; const original=ProviderProcessSupervisor.prototype.stop; const spy=vi.spyOn(ProviderProcessSupervisor.prototype,"stop").mockImplementation(async function(this:ProviderProcessSupervisor,...args) { const pid=this.launch.process.pid; const result=await original.apply(this,args); let absent=false; try {if(pid) process.kill(pid,0);} catch(cause:any) {absent=cause.code==="ESRCH";} stops.push({pid,status:result.status,absent}); return result; }); return {stops,spy}; }; const logger = pino({level:"silent"}); const registry = () => { const commands = new Map<string,any>(); return {commands, cli: { command(name:string, spec:any) {commands.set(name,spec); return this;} } as any}; }; const makeWebFixture = async () => { const workspace = join(scratch,"source-map-fixture"); await mkdir(join(workspace,"files","modules"),{recursive:true}); const fixture = webSourceLocationFixture(); await writeFile(join(workspace,"files","modules","main.js"),fixture.source); await writeFile(join(workspace,"manifest.json"),JSON.stringify({...fixture.manifest,output_directory:workspace,analysis_input:{input_path:workspace,format:"directory"}})); await writeFile(join(workspace,"app.map"),fixture.sourceMap.text); return {workspace,fixture}; }; it("source-map command reports stale cleanup after a successful physical retry", async () => { const {workspace,fixture}=await makeWebFixture(); const observed=observeStops(); let attempts=0; let root=""; const original = PrivateRuntimeRoot.prototype.close; const spy=vi.spyOn(PrivateRuntimeRoot.prototype,"close").mockImplementation(async function(this:PrivateRuntimeRoot) { if(this.path.includes("rea-source-map-")) {root=this.path; attempts++; if(attempts===1) throw new Error("controlled runtime removal failure");} return original.call(this); }); try { const {commands,cli}=registry(); registerWebSourceLocationCommands(cli,logger,process.env); const result=await commands.get("trace-web-source-location").run({args:{manifestPath:join(workspace,"manifest.json"),scriptIndex:0,sourceMapPath:join(workspace,"app.map"),sourceMapUrl:fixture.sourceMap.url,line:1,column:0}}); expect(attempts).toBe(2); await expect(access(root)).rejects.toMatchObject({code:"ENOENT"}); expect(result.code).toBe("cleanup_incomplete"); expect(result.details.resources).toContain(root); expect(result.details.diagnostics.previous_error).toBeNull(); expect(observed.stops).toEqual([{pid:expect.any(Number),status:"verified-cleanup",absent:true}]); console.log("SOURCE_MAP_RETRY",JSON.stringify({attempts,rootGone:true,stops:observed.stops,result})); await writeFile(join(scratch,"source-map-retry-output.json"),JSON.stringify(result,null,2)); } finally {spy.mockRestore(); observed.spy.mockRestore();} }); ``` ### Expected behavior Once the exact codec process and runtime directory have been confirmed released, every error bound to that owner should clear its current cleanupIncomplete flag and unresolved resources while retaining historical diagnostics. [The owner/retry contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L123-L131) requires this, and [the CLI finalizer explicitly waits for owners to resolve errors before projection](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/finishAnalysisCommand.ts#L36-L39). This report does not require the discarded successful codec result to be reconstructed after recovery. ### Impact and scope P2: machine-readable output claims a current cleanup failure after confirmed release and directs the caller toward resources that no longer exist. The same one-shot close completed successfully, so another cleanup attempt cannot resolve the already-returned marker. The trigger was a controlled transient directory-removal failure; workload frequency and downstream admission decisions were not measured. No live process or directory leak remains in this reproduction. ### REA version rea-agents 6.7.0 source build; revision 52415975942a4b9c53a817915b313ccc404f280e. ### Runtime and platform macOS 27.0.1 arm64, Node.js v26.12.0, existing locked dependencies and built runtime. The development Node.js 24.18.0 pin was not separately exercised. ### Target and artifact context Inert local JavaScript sources; the repository webSourceLocationFixture: retained generated script and a version-3 map containing mappings AAAA, sources original.ts and embedded source text original. No browser required. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed result, with the runtime path and run token replaced by placeholders: ```json {"runtime_close_attempts":2,"process_stop":"verified-cleanup","pid_absent":true,"root_exists":false,"final_code":"cleanup_incomplete","final_cleanup":"incomplete","resources":["<released-codec-run>","<deleted-runtime-root>"],"diagnostics":{"previous_error":null,"codec_pid":null}} ``` The second close called the original physical removal method; access(root) then failed with ENOENT before the projected error was inspected. previous_error null establishes that codec execution itself succeeded. ### Typed outcome or recovery expected After confirmed owner release, report no current incomplete-cleanup marker or unresolved resources; historical diagnostics may remain. ### Analysis or suspected cause [SourceMapDecoder retains only cleanupOwner](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/javascript/sourceMaps/SourceMapDecoder.ts#L121-L124) and [successful retry clears only pending ownership](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/javascript/sourceMaps/SourceMapDecoder.ts#L171-L178). The SourceMapCleanupFailure is not bound to a release epoch; [error projection consequently reads its stale cleanupIncomplete state](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorProjection.ts#L69-L90). The exact owner needs to update every originating/retry error when release is confirmed. ### Additional context Related [#6085](https://github.com/morluto/rea/issues/6085) concerns a different state: the runtime directory still needs cleanup, but the resource list includes an already-released codec run. This report concerns stale cleanup status after both owners have been released. Checked #5973, #5983, #5987, #5991, #5995, #6061, #6063, #3671, #5388, #4932, #4628 and #1653 / merged PR #1654. Related “cleanup stays incomplete” report: [#6063](https://github.com/morluto/rea/issues/6063) requires a different managed-descriptor producer to bind its cleanup errors; #4932 concerns the browser-backed module resolver, and #4628 concerns projection after cleanup has already resolved. This source-map codec owner never resolves the error in the first place. Fetched upstream main f86206535f8ec71014781f9d432a84b01e183e5d; the source-map cleanup implementation is unchanged, and the JavaScript per-file early return still bypasses projection-limit composition. Newer worker reuse/CLI-scope changes do not alter that branch. Only the pinned revision was executed. No last-known-good version or external MCP execution was established.

bsakaguchi closed this issue

2026-10-31 00:43:22 +00:00

bsakaguchi commented

2026-10-31 00:43:23 +00:00

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

Cleanup settlement moved to the codec resource owner. SourceMapCommand retains an anchor error, binds subsequent failures to that owner/anchor, and resolves the anchor only after the exact supervisor and runtime root have both released. Thus SourceMapDecoder retaining only cleanupOwner no longer leaves stale originating errors.

Implementation: SourceMapCommand.ts, SourceMapCommand.ts.

The relevant regression checks in SourceMapDecoder.test.ts passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked the reported fix on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. Cleanup settlement moved to the codec resource owner. SourceMapCommand retains an anchor error, binds subsequent failures to that owner/anchor, and resolves the anchor only after the exact supervisor and runtime root have both released. Thus SourceMapDecoder retaining only cleanupOwner no longer leaves stale originating errors. Implementation: [SourceMapCommand.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/javascript/sourceMaps/SourceMapCommand.ts#L473), [SourceMapCommand.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/javascript/sourceMaps/SourceMapCommand.ts#L496). The relevant regression checks in [SourceMapDecoder.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/javascript/sourceMaps/SourceMapDecoder.test.ts) passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

bsakaguchi referenced this issue

2026-11-01 21:10:58 +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#3722

No description provided.

Rows
Columns