morluto/rea

Fork 0

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

[GH-ISSUE #6085] [Bug] Source-map cleanup lists a codec run after verified process release #3723

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/6085

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

When trace-web-source-location successfully releases its codec process but cannot remove the runtime directory, the CLI error lists both the released logical codec run ID and the still-existing directory as unresolved resources. The actual process supervisor returns verified-cleanup, an OS check reports the PID absent, and codec_pid is null in the error. Both the original failure and the failed retry repeat the released run ID.

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 lists a released codec run as unresolved", async () => {
  const {workspace,fixture}=await makeWebFixture(); const observed=observeStops(); let root=""; let attempts=0;
  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++; throw new Error("controlled persistent 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(result.code).toBe("cleanup_incomplete"); expect(attempts).toBe(2);
    expect(result.details.resources.some((s:string)=>s.startsWith("rea-source-map-"))).toBe(true);
    expect(JSON.stringify(result)).toContain('"codec_pid":null'); expect(observed.stops).toEqual([{pid:expect.any(Number),status:"verified-cleanup",absent:true}]);
    console.log("SOURCE_MAP_RESOURCES",JSON.stringify({attempts,stops:observed.stops,result}));
    await writeFile(join(scratch,"source-map-resources-output.json"),JSON.stringify(result,null,2));
  } finally {spy.mockRestore(); observed.spy.mockRestore(); if(root) await rm(root,{recursive:true,force:true});}
});

Expected behavior

When process cleanup is verified and only the private directory remains, cleanupResources should contain only the directory. Retain the run ID while process cleanup is actually unresolved. The lifecycle contract requires only unresolved resources, and OwnedWorkerRuntime uses live owners to derive that list.

Impact and scope

P3: the unresolved-resource diagnostic overstates a genuine directory-cleanup failure by including an already-released process ownership token. The correct directory is still listed, and the overall cleanup_incomplete classification is appropriate while that directory remains. No wrong process cleanup action or blocked recovery was observed. The directory denial was controlled; natural frequency is unknown.

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, replacing local resources with placeholders:

{"runtime_close_attempts":2,"process_stop":"verified-cleanup","pid_absent":true,"final_code":"cleanup_incomplete","resources":["<released-codec-run>","<remaining-runtime-root>"],"original_codec_pid":null,"retry_codec_pid":null,"original_previous_error":null}

The released run ID appears in the final aggregate and both original/retry errors. After inspection, the injected failure was removed and the remaining directory was physically deleted. The run ID is a logical ownership token, not the numeric PID.

Typed outcome or recovery expected

Report only the private runtime root while that is the sole unresolved owner.

Analysis or suspected cause

OwnedCodec clears its supervisor after verified stop but constructs every cleanup resource list with runId unconditionally. Derive the remaining resources from live owners. This correction is independent of resolving earlier error objects after a successful root retry: the reproduced root remains unresolved at the moment the false run ID is emitted.

Additional context

Checked #5973, #5983, #5987, #5991, #5995, #6061, #6063, #3671, #5388, #4932, #4628 and #1653 / merged PR #1654. Related #6084 observes a stale source-map cleanup marker after a successful root retry, which concerns a different state transition. Here the root still exists; only the released process token should be absent. 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/6085 ### 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 When trace-web-source-location successfully releases its codec process but cannot remove the runtime directory, the CLI error lists both the released logical codec run ID and the still-existing directory as unresolved resources. The actual process supervisor returns verified-cleanup, an OS check reports the PID absent, and codec_pid is null in the error. Both the original failure and the failed retry repeat the released run ID. ### 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 lists a released codec run as unresolved", async () => { const {workspace,fixture}=await makeWebFixture(); const observed=observeStops(); let root=""; let attempts=0; 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++; throw new Error("controlled persistent 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(result.code).toBe("cleanup_incomplete"); expect(attempts).toBe(2); expect(result.details.resources.some((s:string)=>s.startsWith("rea-source-map-"))).toBe(true); expect(JSON.stringify(result)).toContain('"codec_pid":null'); expect(observed.stops).toEqual([{pid:expect.any(Number),status:"verified-cleanup",absent:true}]); console.log("SOURCE_MAP_RESOURCES",JSON.stringify({attempts,stops:observed.stops,result})); await writeFile(join(scratch,"source-map-resources-output.json"),JSON.stringify(result,null,2)); } finally {spy.mockRestore(); observed.spy.mockRestore(); if(root) await rm(root,{recursive:true,force:true});} }); ``` ### Expected behavior When process cleanup is verified and only the private directory remains, cleanupResources should contain only the directory. Retain the run ID while process cleanup is actually unresolved. [The lifecycle contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L123-L131) requires only unresolved resources, and [OwnedWorkerRuntime uses live owners to derive that list](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/process/OwnedWorkerRuntime.ts#L127-L135). ### Impact and scope P3: the unresolved-resource diagnostic overstates a genuine directory-cleanup failure by including an already-released process ownership token. The correct directory is still listed, and the overall cleanup_incomplete classification is appropriate while that directory remains. No wrong process cleanup action or blocked recovery was observed. The directory denial was controlled; natural frequency is unknown. ### 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, replacing local resources with placeholders: ```json {"runtime_close_attempts":2,"process_stop":"verified-cleanup","pid_absent":true,"final_code":"cleanup_incomplete","resources":["<released-codec-run>","<remaining-runtime-root>"],"original_codec_pid":null,"retry_codec_pid":null,"original_previous_error":null} ``` The released run ID appears in the final aggregate and both original/retry errors. After inspection, the injected failure was removed and the remaining directory was physically deleted. The run ID is a logical ownership token, not the numeric PID. ### Typed outcome or recovery expected Report only the private runtime root while that is the sole unresolved owner. ### Analysis or suspected cause [OwnedCodec clears its supervisor after verified stop](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/javascript/sourceMaps/SourceMapCommand.ts#L453-L465) but [constructs every cleanup resource list with runId unconditionally](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/javascript/sourceMaps/SourceMapCommand.ts#L473-L489). Derive the remaining resources from live owners. This correction is independent of resolving earlier error objects after a successful root retry: the reproduced root remains unresolved at the moment the false run ID is emitted. ### Additional context Checked #5973, #5983, #5987, #5991, #5995, #6061, #6063, #3671, #5388, #4932, #4628 and #1653 / merged PR #1654. Related [#6084](https://github.com/morluto/rea/issues/6084) observes a stale source-map cleanup marker after a successful root retry, which concerns a different state transition. Here the root still exists; only the released process token should be absent. 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:23 +00:00

bsakaguchi commented

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

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

The cleanup resource getter now includes the run ID only while the supervisor remains live. Verified stop clears that owner before root cleanup; a failing root therefore leaves only the directory in resources. Regressions explicitly inspect the root-only list and process+root to root-only to empty transitions.

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. The cleanup resource getter now includes the run ID only while the supervisor remains live. Verified stop clears that owner before root cleanup; a failing root therefore leaves only the directory in resources. Regressions explicitly inspect the root-only list and process+root to root-only to empty transitions. Implementation: [SourceMapCommand.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/javascript/sourceMaps/SourceMapCommand.ts#L467), [SourceMapCommand.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/javascript/sourceMaps/SourceMapCommand.ts#L484). 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#3723

No description provided.

Rows
Columns