[GH-ISSUE #6058] [Bug] Dylib root enumeration loses its failed directory cleanup owner #3700

New issue

Closed

opened 2026-10-31 00:43:11 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

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

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

With an injected native directory-close rejection during automatic app-bundle root enumeration, trace_dylib_resolution returned an I/O error with cleanupIncomplete: false. The same production client then returned successful close. Only one close attempt occurred, and the captured real directory handle still accepted a read after provider close.

The unchanged app-bundle control with successful close completed the trace normally. This was an executed production artifact-provider test using real bundle traversal and a synthetic Mach-O; only the root directory’s close was fault-injected.

Reproduction

From the source revision below with locked dependencies installed, save this focused test as tests/process-global/dylibDirectoryLifecycle.test.ts and run:

node node_modules/vitest/vitest.mjs run tests/process-global/dylibDirectoryLifecycle.test.ts --maxWorkers=1

The cases below were executed in one six-case file; unrelated cases and imports are removed here. All six characterization cases passed. These two cases also passed when replayed independently from this excerpt. These assertions pin the observed baseline behavior, not a software repair.

import { mkdir, mkdtemp, readFile, rm, writeFile, access } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { expect, it, vi } from 'vitest';
const fault = vi.hoisted(() => ({ root: '', block: false, closes: 0, nativeClose: undefined as undefined | (()=>Promise<void>), handle: undefined as any }));
vi.mock('node:fs/promises', async (importOriginal) => {
 const actual = await importOriginal<typeof import('node:fs/promises')>();
 return { ...actual, opendir: async (...args: Parameters<typeof actual.opendir>) => {
  const handle = await actual.opendir(...args);
  if (String(args[0]) === fault.root) {
   fault.handle = handle; fault.nativeClose = handle.close.bind(handle);
   handle.close = async () => { fault.closes++; if (fault.block) throw new Error('injected directory close failure'); await fault.nativeClose!(); };
  }
  return handle;
 }};
});
import { ArtifactProvider } from '../../src/artifacts/ArtifactProvider.js';
import { parseBinaryTarget } from '../../src/application/BinaryTargetResolver.js';
import { machoImage } from '../../src/artifacts/apple/MachoImage.fixture.js';
it.each([false,true])('dylib enumeration retains failed directory owner, block=%s',async(block)=>{
 const parent=await mkdtemp(join(tmpdir(),'rea-dyld-owner-'));const root=join(parent,'Fixture.app');
 await mkdir(join(root,'Contents','MacOS'),{recursive:true});
 await writeFile(join(root,'Contents','Info.plist'),'<plist><dict><key>CFBundleExecutable</key><string>Main</string></dict></plist>');
 await writeFile(join(root,'Contents','MacOS','Main'),machoImage({}));
 const target=await parseBinaryTarget(root);if(!target.ok)throw target.error;
 const client=new ArtifactProvider({}).createClient(target.value);
 fault.root=root;fault.block=block;fault.closes=0;
 try {
  const result=await client.execute('trace_dylib_resolution',{});
  const closed=await client.close();let readAfterClose=false;
  if(block){await fault.handle.read();readAfterClose=true;}
  console.log(JSON.stringify({area:'dylib',block,resultOk:result.ok,reason:result.ok?null:result.error.reason,cleanupIncomplete:result.ok?null:result.error.cleanupIncomplete,closeOk:closed.ok,closes:fault.closes,readAfterClose}));
  expect(result.ok).toBe(!block);expect(closed.ok).toBe(true);
  if(block){expect(fault.closes).toBe(1);expect(readAfterClose).toBe(true);}
 }finally{fault.block=false;await client.close();if(block)await fault.nativeClose?.();fault.root='';await rm(parent,{recursive:true,force:true});}
});

The test releases its fixtures and any retained resources in finalization.

Expected behavior

Retain the root-enumeration reader and its unresolved directory ownership, report cleanup uncertainty with the operation failure, and let provider close report the outstanding owner. The ownership contract requires attribution and retention until release is confirmed. The existing directory ownership regression expects reader.close() to report the retained failed directory close.

Impact and scope

P2: a directory-close failure loses the exact cleanup owner, and provider close incorrectly reports completion. The controlled failure left a live directory handle; normal traversal had no leftover handle. Natural close-failure frequency is unknown. Native Node Dir.close can invalidate its public handle before a failed native close completes, so a generally safe retry is not implied. The defect includes lost cleanup attribution even when native release cannot be retried.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e. Fetched origin/main at 1f92c773989585a7c0f3255b7a9264789394908c; the producing code remains unchanged. That newer revision was source-compared, not executed.

Runtime and platform

macOS 27.0.1 arm64; Node.js v26.12.0; locked Vitest 5.0.3. The repository-pinned Node.js 24.18.0 was not exercised.

Target and artifact context

Synthetic Fixture.app containing Contents/Info.plist and a regular Contents/MacOS/Main built with the repository’s machoImage fixture. The app directory is selected with no explicit roots, activating automatic root enumeration.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed characterization results:

successful close control: result.ok=true, provider close.ok=true, close calls=1
injected directory-close failure: result.ok=false, reason=io
cleanupIncomplete=false; provider close.ok=true; close calls=1
captured directory read after provider close succeeded

Production-provider behavior was executed. CLI/MCP reachability is source-derived; their transports were not separately invoked.

Typed outcome or recovery expected

Retain a reader/directory cleanup owner and a cleanup-incomplete marker bound to that owner. Provider close must not claim release merely because enumeration already returned an I/O error. No unsafe retry of a possibly invalidated native Dir is required.

Analysis or suspected cause

Source-derived: executableRoots constructs new DirectoryArtifactReader(root).entries(signal) without retaining the reader or attaching it to the existing operation resource scope. Directory finalization retains its owner after rejected close, but only reader.close can report that retained set. Provider close only knows the resource scope. OwnedDirectoryHandle deliberately preserves a rejected-close state instead of authorizing an unsafe retry.

Additional context

Compared #5907 (post-admission pathname disappearance classification), closed #4495 and PR #4493 (Mach-O file admission/read/digest descriptors), PR #3248 (ordered overlapping magic reads), and PR #1960 (reverse command/session cleanup). These do not own the enumeration directory reader. Later main extends ArtifactResourceScope with close hooks for another owner; executableRoots still registers neither reader nor hook. The early iterator-return/primary-failure combination was not separately executed.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6058 ### 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 With an injected native directory-close rejection during automatic app-bundle root enumeration, `trace_dylib_resolution` returned an I/O error with `cleanupIncomplete: false`. The same production client then returned successful close. Only one close attempt occurred, and the captured real directory handle still accepted a read after provider close. The unchanged app-bundle control with successful close completed the trace normally. This was an executed production artifact-provider test using real bundle traversal and a synthetic Mach-O; only the root directory’s close was fault-injected. ### Reproduction From the source revision below with locked dependencies installed, save this focused test as `tests/process-global/dylibDirectoryLifecycle.test.ts` and run: ```sh node node_modules/vitest/vitest.mjs run tests/process-global/dylibDirectoryLifecycle.test.ts --maxWorkers=1 ``` The cases below were executed in one six-case file; unrelated cases and imports are removed here. All six characterization cases passed. These two cases also passed when replayed independently from this excerpt. These assertions pin the observed baseline behavior, not a software repair. ```ts import { mkdir, mkdtemp, readFile, rm, writeFile, access } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { expect, it, vi } from 'vitest'; const fault = vi.hoisted(() => ({ root: '', block: false, closes: 0, nativeClose: undefined as undefined | (()=>Promise<void>), handle: undefined as any })); vi.mock('node:fs/promises', async (importOriginal) => { const actual = await importOriginal<typeof import('node:fs/promises')>(); return { ...actual, opendir: async (...args: Parameters<typeof actual.opendir>) => { const handle = await actual.opendir(...args); if (String(args[0]) === fault.root) { fault.handle = handle; fault.nativeClose = handle.close.bind(handle); handle.close = async () => { fault.closes++; if (fault.block) throw new Error('injected directory close failure'); await fault.nativeClose!(); }; } return handle; }}; }); import { ArtifactProvider } from '../../src/artifacts/ArtifactProvider.js'; import { parseBinaryTarget } from '../../src/application/BinaryTargetResolver.js'; import { machoImage } from '../../src/artifacts/apple/MachoImage.fixture.js'; it.each([false,true])('dylib enumeration retains failed directory owner, block=%s',async(block)=>{ const parent=await mkdtemp(join(tmpdir(),'rea-dyld-owner-'));const root=join(parent,'Fixture.app'); await mkdir(join(root,'Contents','MacOS'),{recursive:true}); await writeFile(join(root,'Contents','Info.plist'),'<plist><dict><key>CFBundleExecutable</key><string>Main</string></dict></plist>'); await writeFile(join(root,'Contents','MacOS','Main'),machoImage({})); const target=await parseBinaryTarget(root);if(!target.ok)throw target.error; const client=new ArtifactProvider({}).createClient(target.value); fault.root=root;fault.block=block;fault.closes=0; try { const result=await client.execute('trace_dylib_resolution',{}); const closed=await client.close();let readAfterClose=false; if(block){await fault.handle.read();readAfterClose=true;} console.log(JSON.stringify({area:'dylib',block,resultOk:result.ok,reason:result.ok?null:result.error.reason,cleanupIncomplete:result.ok?null:result.error.cleanupIncomplete,closeOk:closed.ok,closes:fault.closes,readAfterClose})); expect(result.ok).toBe(!block);expect(closed.ok).toBe(true); if(block){expect(fault.closes).toBe(1);expect(readAfterClose).toBe(true);} }finally{fault.block=false;await client.close();if(block)await fault.nativeClose?.();fault.root='';await rm(parent,{recursive:true,force:true});} }); ``` The test releases its fixtures and any retained resources in finalization. ### Expected behavior Retain the root-enumeration reader and its unresolved directory ownership, report cleanup uncertainty with the operation failure, and let provider close report the outstanding owner. [The ownership contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L119-L138) requires attribution and retention until release is confirmed. [The existing directory ownership regression](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/tests/boundary/filesystem/directoryArtifactReaderOwnership.test.ts#L102-L141) expects `reader.close()` to report the retained failed directory close. ### Impact and scope P2: a directory-close failure loses the exact cleanup owner, and provider close incorrectly reports completion. The controlled failure left a live directory handle; normal traversal had no leftover handle. Natural close-failure frequency is unknown. Native Node Dir.close can invalidate its public handle before a failed native close completes, so a generally safe retry is not implied. The defect includes lost cleanup attribution even when native release cannot be retried. ### REA version `rea-agents` 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. Fetched `origin/main` at `1f92c773989585a7c0f3255b7a9264789394908c`; the producing code remains unchanged. That newer revision was source-compared, not executed. ### Runtime and platform macOS 27.0.1 arm64; Node.js v26.12.0; locked Vitest 5.0.3. The repository-pinned Node.js 24.18.0 was not exercised. ### Target and artifact context Synthetic Fixture.app containing Contents/Info.plist and a regular Contents/MacOS/Main built with the repository’s machoImage fixture. The app directory is selected with no explicit roots, activating automatic root enumeration. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed characterization results: ```text successful close control: result.ok=true, provider close.ok=true, close calls=1 injected directory-close failure: result.ok=false, reason=io cleanupIncomplete=false; provider close.ok=true; close calls=1 captured directory read after provider close succeeded ``` Production-provider behavior was executed. CLI/MCP reachability is source-derived; their transports were not separately invoked. ### Typed outcome or recovery expected Retain a reader/directory cleanup owner and a cleanup-incomplete marker bound to that owner. Provider close must not claim release merely because enumeration already returned an I/O error. No unsafe retry of a possibly invalidated native Dir is required. ### Analysis or suspected cause Source-derived: [executableRoots](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/apple/DylibResolutionReader.ts#L192-L223) constructs `new DirectoryArtifactReader(root).entries(signal)` without retaining the reader or attaching it to the existing operation resource scope. [Directory finalization](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/DirectoryArtifactReader.ts#L235-L252) retains its owner after rejected close, but only [reader.close](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/DirectoryArtifactReader.ts#L389-L426) can report that retained set. Provider close only knows the resource scope. [OwnedDirectoryHandle](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/filesystem/OwnedDirectoryHandle.ts#L9-L32) deliberately preserves a rejected-close state instead of authorizing an unsafe retry. ### Additional context Compared #5907 (post-admission pathname disappearance classification), closed #4495 and PR #4493 (Mach-O file admission/read/digest descriptors), PR #3248 (ordered overlapping magic reads), and PR #1960 (reverse command/session cleanup). These do not own the enumeration directory reader. Later main extends ArtifactResourceScope with close hooks for another owner; executableRoots still registers neither reader nor hook. The early iterator-return/primary-failure combination was not separately executed.

bsakaguchi closed this issue

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

bsakaguchi referenced this issue

2026-11-01 21:10:57 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471