morluto/rea

Fork 0

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

[GH-ISSUE #5842] [Bug] Evidence export reports incomplete cleanup after its staging retry succeeds #3539

New issue

Closed

opened 2026-10-31 00:41:38 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:41:38 +00:00

Owner

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

Originally assigned to: @kaoru0822-kitauji on GitHub.

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 source-workflow test of the implementation used by rea evidence-export SOURCE OUTPUT --json returned cleanup_incomplete after the staging directory had been removed successfully. The test used a valid empty Evidence bundle and a new output path, injected EACCES at the publication hard link, and injected EBUSY on only the first staging-directory removal. The workflow retried removal successfully, but its error still reported cleanup: incomplete, the first removal failure, and one retained resource.

This was an application-workflow and error-projection reproduction with filesystem fault injection, not a spawned CLI run or an observation of spontaneous host filesystem faults. The real bundle parser, staged-file writes, identity checks, unlink, resource scope, retry, and final directory removal ran unchanged.

Reproduction

From the baseline checkout with installed dependencies and the repository-pinned Node.js on PATH, save the following temporary test as src/application/repro.test.ts. Use an isolated disposable directory for HOME, XDG_CONFIG_HOME and TMPDIR. Run npm run test:local -- src/application/repro.test.ts, then delete the temporary test.

The three cases ran once each. They characterize the observed behavior; the middle case demonstrates the defect by asserting that the returned cleanup flag remains stale despite an empty staging inventory.

import { readdir, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { expect, it, vi } from "vitest";
import { exportEvidenceBundleCommand } from "./EvidenceBundleCommands.js";
import { projectAnalysisError } from "../domain/analysisErrorProjection.js";
import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js";

const fault = vi.hoisted(() => ({ publication: false, cleanup: false, attempts: 0 }));
vi.mock("node:fs/promises", async (importOriginal) => {
  const fs = await importOriginal<typeof import("node:fs/promises")>();
  return {
    ...fs,
    link: async (...args: Parameters<typeof fs.link>) => {
      if (fault.publication)
        throw Object.assign(new Error("injected publication EACCES"), { code: "EACCES" });
      return fs.link(...args);
    },
    rmdir: async (...args: Parameters<typeof fs.rmdir>) => {
      if (String(args[0]).includes(".rea-write-")) {
        fault.attempts += 1;
        if (fault.cleanup && fault.attempts === 1)
          throw Object.assign(new Error("injected first cleanup EBUSY"), { code: "EBUSY" });
      }
      return fs.rmdir(...args);
    },
  };
});

it.each([
  { publicationFails: true, cleanupFailsOnce: false, expected: "execution_failure", attempts: 1 },
  { publicationFails: true, cleanupFailsOnce: true, expected: "cleanup_incomplete", attempts: 2 },
  { publicationFails: false, cleanupFailsOnce: true, expected: "success", attempts: 2 },
])("export after publication failure=$publicationFails cleanup failure=$cleanupFailsOnce", async (scenario) => {
  const root = await createTestTempDirectory("rea-export-retry-");
  const source = join(root, "source.json");
  const output = join(root, "out.json");
  await writeFile(source, JSON.stringify({ artifacts: [], providers: [], environments: [], scenarios: [], captures: [], unknowns: [], records: [] }));
  fault.publication = scenario.publicationFails;
  fault.cleanup = scenario.cleanupFailsOnce;
  fault.attempts = 0;
  try {
    const result = await exportEvidenceBundleCommand(source, output, false);
    const projected = result.ok ? undefined : projectAnalysisError(result.error);
    const entries = await readdir(root);
    const stages = entries.filter((name) => name.startsWith(".rea-write-"));
    console.log(JSON.stringify({
      publicationFails: scenario.publicationFails,
      cleanupFailsOnce: scenario.cleanupFailsOnce,
      rmdirAttempts: fault.attempts,
      stagesRemaining: stages.length,
      outputExists: entries.includes("out.json"),
      code: projected?.code ?? "success",
      cleanup: projected?.details?.cleanup ?? null,
      cleanupReason: projected?.details?.cleanup_reason ?? null,
      retainedResourceCount: Array.isArray(projected?.details?.resources) ? projected.details.resources.length : 0,
      remediation: projected?.remediation.action ?? null,
    }));
    expect(projected?.code ?? "success").toBe(scenario.expected);
    expect(fault.attempts).toBe(scenario.attempts);
    expect(stages).toEqual([]);
    expect(entries.includes("out.json")).toBe(!scenario.publicationFails);
    if (scenario.expected === "cleanup_incomplete") {
      expect(projected?.details).toMatchObject({ cleanup: "incomplete", cleanup_reason: "injected first cleanup EBUSY" });
      expect(projected?.details?.resources).toHaveLength(1);
    }
  } finally {
    fault.publication = false;
    fault.cleanup = false;
  }
});

Expected behavior

When the exact staging owner confirms release, return the original publication failure without current cleanup_incomplete state or retained-resource claims. The export must still fail because publication did not happen.

The cleanup contract requires restoration of the original typed outcome after confirmed cleanup. The lifecycle ownership invariant requires reporting only resources whose cleanup remains unresolved. If publication already completed, the existing success recovery should continue working, as it did in the third control.

Impact and scope

P2: callers can receive a false cleanup-failure code and stale retained-resource state after a failed export. That changes the error classification a caller uses for recovery even though no cleanup remains. The observed scope is the standalone Evidence-export workflow with an unpublished write failure followed by an initial cleanup failure and a successful cleanup retry. Normal primary-failure cleanup and published-output recovery passed the controls.

The output remained absent in both publication-failure cases; no data loss or evidence corruption was observed. Frequency of real transient filesystem failures, behavior on other operating systems, and MCP export behavior were not tested. The observed remediation remained the generic instruction to check file permissions; no manual-cleanup instruction was observed.

REA version

rea-agents 6.7.0; source baseline a5f982de8d.

Runtime and platform

Node.js v24.18.0; macOS 27.0.1 arm64; Vitest 5.0.3; no analysis provider required.

Target and artifact context

Synthetic valid Evidence JSON with seven empty manifest/record arrays; a new local output file; injected publication EACCES and one cleanup EBUSY.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Actual focused test result: one test file passed, three tests passed. These are the test's exact JSON observations, excluding runner paths and timing:

{"publicationFails":true,"cleanupFailsOnce":false,"rmdirAttempts":1,"stagesRemaining":0,"outputExists":false,"code":"execution_failure","cleanup":null,"cleanupReason":null,"retainedResourceCount":0,"remediation":"Evidence file could not be accessed. Check file permissions and try again."}
{"publicationFails":true,"cleanupFailsOnce":true,"rmdirAttempts":2,"stagesRemaining":0,"outputExists":false,"code":"cleanup_incomplete","cleanup":"incomplete","cleanupReason":"injected first cleanup EBUSY","retainedResourceCount":1,"remediation":"Evidence file could not be accessed. Check file permissions and try again."}
{"publicationFails":false,"cleanupFailsOnce":true,"rmdirAttempts":2,"stagesRemaining":0,"outputExists":true,"code":"success","cleanup":null,"cleanupReason":null,"retainedResourceCount":0,"remediation":null}

Reachability is source-derived: the CLI handler calls the tested workflow with the accepted source/output/overwrite inputs, and its error adapter uses the tested projection. The writer's admission checks accept the new destination; the publication and cleanup sequence then reaches both injected filesystem operations.

Typed outcome or recovery expected

For the middle case, return the primary EvidenceFileError projection with code: execution_failure, matching the first control, and no current cleanup marker or retained staging resource. Preserve the original write failure and any useful historical diagnostics.

Analysis or suspected cause

Source-derived: the writer wraps the initial cleanup failure in a new EvidenceFileError with a cleanup observation. The export recovery branch uses successful scope closure only for an already-published write; otherwise it returns the same error. That error has no binding to the retained lease that could clear its cleanup flag. The projection therefore emits the stale current state.

Additional context

No response

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5842 Originally assigned to: @kaoru0822-kitauji on GitHub. ### 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 source-workflow test of the implementation used by `rea evidence-export SOURCE OUTPUT --json` returned `cleanup_incomplete` after the staging directory had been removed successfully. The test used a valid empty Evidence bundle and a new output path, injected `EACCES` at the publication hard link, and injected `EBUSY` on only the first staging-directory removal. The workflow retried removal successfully, but its error still reported `cleanup: incomplete`, the first removal failure, and one retained resource. This was an application-workflow and error-projection reproduction with filesystem fault injection, not a spawned CLI run or an observation of spontaneous host filesystem faults. The real bundle parser, staged-file writes, identity checks, unlink, resource scope, retry, and final directory removal ran unchanged. ### Reproduction From the baseline checkout with installed dependencies and the repository-pinned Node.js on PATH, save the following temporary test as `src/application/repro.test.ts`. Use an isolated disposable directory for HOME, XDG_CONFIG_HOME and TMPDIR. Run `npm run test:local -- src/application/repro.test.ts`, then delete the temporary test. The three cases ran once each. They characterize the observed behavior; the middle case demonstrates the defect by asserting that the returned cleanup flag remains stale despite an empty staging inventory. ```ts import { readdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, it, vi } from "vitest"; import { exportEvidenceBundleCommand } from "./EvidenceBundleCommands.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; const fault = vi.hoisted(() => ({ publication: false, cleanup: false, attempts: 0 })); vi.mock("node:fs/promises", async (importOriginal) => { const fs = await importOriginal<typeof import("node:fs/promises")>(); return { ...fs, link: async (...args: Parameters<typeof fs.link>) => { if (fault.publication) throw Object.assign(new Error("injected publication EACCES"), { code: "EACCES" }); return fs.link(...args); }, rmdir: async (...args: Parameters<typeof fs.rmdir>) => { if (String(args[0]).includes(".rea-write-")) { fault.attempts += 1; if (fault.cleanup && fault.attempts === 1) throw Object.assign(new Error("injected first cleanup EBUSY"), { code: "EBUSY" }); } return fs.rmdir(...args); }, }; }); it.each([ { publicationFails: true, cleanupFailsOnce: false, expected: "execution_failure", attempts: 1 }, { publicationFails: true, cleanupFailsOnce: true, expected: "cleanup_incomplete", attempts: 2 }, { publicationFails: false, cleanupFailsOnce: true, expected: "success", attempts: 2 }, ])("export after publication failure=$publicationFails cleanup failure=$cleanupFailsOnce", async (scenario) => { const root = await createTestTempDirectory("rea-export-retry-"); const source = join(root, "source.json"); const output = join(root, "out.json"); await writeFile(source, JSON.stringify({ artifacts: [], providers: [], environments: [], scenarios: [], captures: [], unknowns: [], records: [] })); fault.publication = scenario.publicationFails; fault.cleanup = scenario.cleanupFailsOnce; fault.attempts = 0; try { const result = await exportEvidenceBundleCommand(source, output, false); const projected = result.ok ? undefined : projectAnalysisError(result.error); const entries = await readdir(root); const stages = entries.filter((name) => name.startsWith(".rea-write-")); console.log(JSON.stringify({ publicationFails: scenario.publicationFails, cleanupFailsOnce: scenario.cleanupFailsOnce, rmdirAttempts: fault.attempts, stagesRemaining: stages.length, outputExists: entries.includes("out.json"), code: projected?.code ?? "success", cleanup: projected?.details?.cleanup ?? null, cleanupReason: projected?.details?.cleanup_reason ?? null, retainedResourceCount: Array.isArray(projected?.details?.resources) ? projected.details.resources.length : 0, remediation: projected?.remediation.action ?? null, })); expect(projected?.code ?? "success").toBe(scenario.expected); expect(fault.attempts).toBe(scenario.attempts); expect(stages).toEqual([]); expect(entries.includes("out.json")).toBe(!scenario.publicationFails); if (scenario.expected === "cleanup_incomplete") { expect(projected?.details).toMatchObject({ cleanup: "incomplete", cleanup_reason: "injected first cleanup EBUSY" }); expect(projected?.details?.resources).toHaveLength(1); } } finally { fault.publication = false; fault.cleanup = false; } }); ``` ### Expected behavior When the exact staging owner confirms release, return the original publication failure without current `cleanup_incomplete` state or retained-resource claims. The export must still fail because publication did not happen. The [cleanup contract](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/mcp-contracts.md#L672-L681) requires restoration of the original typed outcome after confirmed cleanup. The [lifecycle ownership invariant](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/docs/tool-design.md#L123-L133) requires reporting only resources whose cleanup remains unresolved. If publication already completed, the existing success recovery should continue working, as it did in the third control. ### Impact and scope P2: callers can receive a false cleanup-failure code and stale retained-resource state after a failed export. That changes the error classification a caller uses for recovery even though no cleanup remains. The observed scope is the standalone Evidence-export workflow with an unpublished write failure followed by an initial cleanup failure and a successful cleanup retry. Normal primary-failure cleanup and published-output recovery passed the controls. The output remained absent in both publication-failure cases; no data loss or evidence corruption was observed. Frequency of real transient filesystem failures, behavior on other operating systems, and MCP export behavior were not tested. The observed remediation remained the generic instruction to check file permissions; no manual-cleanup instruction was observed. ### REA version rea-agents 6.7.0; source baseline a5f982de8dbe4b092cc20ab89d492b7e7028b758. ### Runtime and platform Node.js v24.18.0; macOS 27.0.1 arm64; Vitest 5.0.3; no analysis provider required. ### Target and artifact context Synthetic valid Evidence JSON with seven empty manifest/record arrays; a new local output file; injected publication EACCES and one cleanup EBUSY. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Actual focused test result: one test file passed, three tests passed. These are the test's exact JSON observations, excluding runner paths and timing: ```json {"publicationFails":true,"cleanupFailsOnce":false,"rmdirAttempts":1,"stagesRemaining":0,"outputExists":false,"code":"execution_failure","cleanup":null,"cleanupReason":null,"retainedResourceCount":0,"remediation":"Evidence file could not be accessed. Check file permissions and try again."} {"publicationFails":true,"cleanupFailsOnce":true,"rmdirAttempts":2,"stagesRemaining":0,"outputExists":false,"code":"cleanup_incomplete","cleanup":"incomplete","cleanupReason":"injected first cleanup EBUSY","retainedResourceCount":1,"remediation":"Evidence file could not be accessed. Check file permissions and try again."} {"publicationFails":false,"cleanupFailsOnce":true,"rmdirAttempts":2,"stagesRemaining":0,"outputExists":true,"code":"success","cleanup":null,"cleanupReason":null,"retainedResourceCount":0,"remediation":null} ``` Reachability is source-derived: the [CLI handler](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/cliEvidenceCommands.ts#L31-L48) calls the tested workflow with the accepted source/output/overwrite inputs, and its [error adapter](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/cliEvidenceCommands.ts#L68-L71) uses the tested projection. The [writer's admission checks](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/JsonFiles.ts#L239-L263) accept the new destination; the [publication and cleanup sequence](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/JsonFiles.ts#L353-L378) then reaches both injected filesystem operations. ### Typed outcome or recovery expected For the middle case, return the primary `EvidenceFileError` projection with `code: execution_failure`, matching the first control, and no current cleanup marker or retained staging resource. Preserve the original write failure and any useful historical diagnostics. ### Analysis or suspected cause Source-derived: the [writer wraps the initial cleanup failure](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/JsonFiles.ts#L264-L293) in a new `EvidenceFileError` with a cleanup observation. The [export recovery branch](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/application/EvidenceBundleCommands.ts#L49-L66) uses successful scope closure only for an already-published write; otherwise it returns the same error. That error has no binding to the retained lease that could clear its cleanup flag. The [projection](https://github.com/morluto/rea/blob/a5f982de8dbe4b092cc20ab89d492b7e7028b758/src/domain/analysisErrorProjection.ts#L68-L86) therefore emits the stale current state. ### Additional context _No response_

bsakaguchi closed this issue

2026-10-31 00:41:39 +00:00

bsakaguchi commented

2026-10-31 00:41:39 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 21, 2026):

Confirmed — the middle case clearly shows the projection keeps the stale cleanup_incomplete state (plus a retained resource) even though the staging inventory is empty after the retry. Since the lifecycle contract is to report only unresolved cleanup, the export recovery branch should restore the primary publication error (execution_failure) once the staging owner confirms release, exactly as the first control does. The third control already proves the retry path itself works; the fix is purely in the error projection ordering.

@XinwangTao commented on GitHub (Oct 21, 2026): Confirmed — the middle case clearly shows the projection keeps the stale cleanup_incomplete state (plus a retained resource) even though the staging inventory is empty after the retry. Since the lifecycle contract is to report only unresolved cleanup, the export recovery branch should restore the primary publication error (execution_failure) once the staging owner confirms release, exactly as the first control does. The third control already proves the retry path itself works; the fix is purely in the error projection ordering.

bsakaguchi referenced this issue

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

[GH-ISSUE #7740] [Feature] Keep literal call arguments on call sites, such as the name in client.rpc("name") #4631

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

No description provided.

Rows
Columns