morluto/rea

Fork 0

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

[GH-ISSUE #6083] [Bug] JavaScript temporary-file cleanup failure drops the application expansion guard #3718

New issue

Closed

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

bsakaguchi commented

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

Owner

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

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

A controlled analyze-javascript-application CLI callback run loses a recorded application-expansion limit when removal of a worker's per-source file fails. The real child analyzer runs on ten ordinary fetch calls. A low parent result budget triggers the real expansion guard in the no-cleanup-fault control.

Control: the file observation includes application_projection_failure; graph contains zero endpoint nodes and four reported limits. Changing only per-source removal to throw EACCES yields an error with retained partial Evidence, but that Evidence has no projection marker, no graph limits, and ten expanded endpoint nodes. Thus cleanup failure bypasses the already-observed parent expansion guard. The aggregate graph remains partial because a separate source-analysis failure is retained.

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("JavaScript public command loses the projection limitation only on per-file deletion failure", async () => {
  const app=join(scratch,"js-fixture"); await mkdir(app,{recursive:true});
  await writeFile(join(app,"main.js"),Array.from({length:10},(_,i)=>`fetch("https://example.test/${i}");`).join("\n"));
  state.budget=256*1024;
  const {commands,cli}=registry(); registerElectronCommands(cli,logger,process.env);
  const outputs:any[]=[];
  try {
    for(const fault of [false,true]) {
      state.fileFault=fault;
      const result=await commands.get("analyze-javascript-application").run({args:{path:app},options:{maxHeapMb:128,analysisTimeoutMs:30000}});
      outputs.push(result); await writeFile(join(scratch,fault?"js-cleanup-output.json":"js-control-output.json"),JSON.stringify(result,null,2));
      const text=JSON.stringify(result);
      console.log("JS_CASE",JSON.stringify({fault,removals:state.removals,code:result.code,projectionMarkers:(text.match(/application_projection_failure/g)||[]).length,expansionReason:text.includes("Expanded application relationships"),fetchNodes:(text.match(/https:\/\/example.test/g)||[]).length}));
    }
    expect(JSON.stringify(outputs[0])).toContain("application_projection_failure");
    expect(state.removals).toBe(1);
    expect(JSON.stringify(outputs[1])).not.toContain("application_projection_failure");
  } finally {state.fileFault=false;state.budget=0;}
});

Expected behavior

Per-source cleanup failure must preserve applicationProjectionFailure alongside retained static/module facts and cleanup diagnostics. Graph construction must continue to honor the parent expansion guard and retain an explicit project-javascript-application unknown scope. The documented retained-fact and unknown-relationship contract applies independently of cleanup success.

Impact and scope

P2: a cleanup error drops resource-limit metadata and permits graph expansion that the parent had explicitly declined. This affects the combined parent-capacity and temporary-file-removal failure case. The demonstrated expansion is only ten endpoints; no process OOM, complete aggregate graph coverage, total Evidence loss, or natural OS denial was observed. The low-headroom observation and removal failure were injected. No equivalent-output workaround was tested.

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; ten literal fetch call sites in one main.js file.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed comparison:

control: projection markers 1; endpoint nodes 0; graph limit entries 4
file-removal fault: rm faults 1; projection markers 0; endpoint nodes 10; graph limit entries 0
control projection requirement: 1395392 bytes
control available parent budget: 247892 bytes (total 260408)
fault result: execution_failure with details.partial_observation
both aggregate graph coverage statuses: partial

The temporary source consists of ten lines fetch("https://example.test/N"); for N from 0 through 9. The assertions describe the current incorrect behavior, rather than a repaired result.

Typed outcome or recovery expected

Retain the projection-limit field and parent-limit coverage in the partial Evidence while preserving the cleanup diagnostic.

Analysis or suspected cause

The cleanup catch returns early before the shared applicationProjectionFailure composition. The consumer reads that field from either result branch, and graph construction uses it to retain unexpanded facts and the explicit unavailable scope. This is a per-file worker cleanup branch reached before the service returns, not an assumed final scope.close failure after otherwise successful analysis.

Additional context

Checked #5973, #5983, #5987, #5991, #5995, #6061, #6063, #3671, #5388, #4932, #4628 and #1653 / merged PR #1654. #3671 concerns accounting and reporting of a retained projection failure; this case deletes that failure during per-file removal. The previous successful-analysis/final-scope-close premise does not describe this branch. 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/6083 ### 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 A controlled analyze-javascript-application CLI callback run loses a recorded application-expansion limit when removal of a worker's per-source file fails. The real child analyzer runs on ten ordinary fetch calls. A low parent result budget triggers the real expansion guard in the no-cleanup-fault control. Control: the file observation includes application_projection_failure; graph contains zero endpoint nodes and four reported limits. Changing only per-source removal to throw EACCES yields an error with retained partial Evidence, but that Evidence has no projection marker, no graph limits, and ten expanded endpoint nodes. Thus cleanup failure bypasses the already-observed parent expansion guard. The aggregate graph remains partial because a separate source-analysis failure is retained. ### 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("JavaScript public command loses the projection limitation only on per-file deletion failure", async () => { const app=join(scratch,"js-fixture"); await mkdir(app,{recursive:true}); await writeFile(join(app,"main.js"),Array.from({length:10},(_,i)=>`fetch("https://example.test/${i}");`).join("\n")); state.budget=256*1024; const {commands,cli}=registry(); registerElectronCommands(cli,logger,process.env); const outputs:any[]=[]; try { for(const fault of [false,true]) { state.fileFault=fault; const result=await commands.get("analyze-javascript-application").run({args:{path:app},options:{maxHeapMb:128,analysisTimeoutMs:30000}}); outputs.push(result); await writeFile(join(scratch,fault?"js-cleanup-output.json":"js-control-output.json"),JSON.stringify(result,null,2)); const text=JSON.stringify(result); console.log("JS_CASE",JSON.stringify({fault,removals:state.removals,code:result.code,projectionMarkers:(text.match(/application_projection_failure/g)||[]).length,expansionReason:text.includes("Expanded application relationships"),fetchNodes:(text.match(/https:\/\/example.test/g)||[]).length})); } expect(JSON.stringify(outputs[0])).toContain("application_projection_failure"); expect(state.removals).toBe(1); expect(JSON.stringify(outputs[1])).not.toContain("application_projection_failure"); } finally {state.fileFault=false;state.budget=0;} }); ``` ### Expected behavior Per-source cleanup failure must preserve applicationProjectionFailure alongside retained static/module facts and cleanup diagnostics. Graph construction must continue to honor the parent expansion guard and retain an explicit project-javascript-application unknown scope. [The documented retained-fact and unknown-relationship contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/javascript-artifact-reconstruction.md#L629-L635) applies independently of [cleanup success](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L123-L133). ### Impact and scope P2: a cleanup error drops resource-limit metadata and permits graph expansion that the parent had explicitly declined. This affects the combined parent-capacity and temporary-file-removal failure case. The demonstrated expansion is only ten endpoints; no process OOM, complete aggregate graph coverage, total Evidence loss, or natural OS denial was observed. The low-headroom observation and removal failure were injected. No equivalent-output workaround was tested. ### 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; ten literal fetch call sites in one main.js file. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed comparison: ```text control: projection markers 1; endpoint nodes 0; graph limit entries 4 file-removal fault: rm faults 1; projection markers 0; endpoint nodes 10; graph limit entries 0 control projection requirement: 1395392 bytes control available parent budget: 247892 bytes (total 260408) fault result: execution_failure with details.partial_observation both aggregate graph coverage statuses: partial ``` The temporary source consists of ten lines fetch("https://example.test/N"); for N from 0 through 9. The assertions describe the current incorrect behavior, rather than a repaired result. ### Typed outcome or recovery expected Retain the projection-limit field and parent-limit coverage in the partial Evidence while preserving the cleanup diagnostic. ### Analysis or suspected cause [The cleanup catch returns early](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/javascript/analysis/JavaScriptAnalysisWorker.ts#L467-L493) before the shared applicationProjectionFailure composition. [The consumer reads that field from either result branch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/javascript/JavaScriptArtifactAnalysis.ts#L181-L215), and [graph construction uses it to retain unexpanded facts](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/javascript/JavaScriptArtifactGraphStructure.ts#L157-L187) and [the explicit unavailable scope](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/javascript/JavaScriptArtifactGraphStructure.ts#L262-L273). This is a per-file worker cleanup branch reached before the service returns, not an assumed final scope.close failure after otherwise successful analysis. ### Additional context Checked #5973, #5983, #5987, #5991, #5995, #6061, #6063, #3671, #5388, #4932, #4628 and #1653 / merged PR #1654. #3671 concerns accounting and reporting of a retained projection failure; this case deletes that failure during per-file removal. The previous successful-analysis/final-scope-close premise does not describe this branch. 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:21 +00:00

bsakaguchi commented

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

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

Per-source removal failure now assigns an error result and continues through the shared applicationProjectionFailure composition instead of returning early. The regression forces low parent headroom and independently toggles EACCES source removal, then requires the projection resource marker on both result branches and cleanupIncomplete on the fault branch. This restores the producing field that downstream graph guards consume.

Implementation: JavaScriptAnalysisWorker.ts, JavaScriptAnalysisWorker.ts.

The relevant regression checks in workerFailureComposition.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. Per-source removal failure now assigns an error result and continues through the shared applicationProjectionFailure composition instead of returning early. The regression forces low parent headroom and independently toggles EACCES source removal, then requires the projection resource marker on both result branches and cleanupIncomplete on the fault branch. This restores the producing field that downstream graph guards consume. Implementation: [JavaScriptAnalysisWorker.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/javascript/analysis/JavaScriptAnalysisWorker.ts#L539), [JavaScriptAnalysisWorker.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/javascript/analysis/JavaScriptAnalysisWorker.ts#L550). The relevant regression checks in [workerFailureComposition.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/javascript/workerFailureComposition.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: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#3718

No description provided.

Rows
Columns