fix(windows): retain failed native inspection ownership (#1826)

.github/workflows/ci.yml

@@ -305,6 +305,12 @@ jobs:
           sudo apt-get install --no-install-recommends -y g++-mingw-w64-x86-64
       - run: npm run build:windows-native
       - run: npm run verify:windows-native:artifact
+      - run: node scripts/build-windows-native.mjs --inspection-fixture
+      - uses: actions/upload-artifact@v4
+        with:
+          name: windows-native-inspection-fixture
+          path: .cache/windows-native-inspection-fixture/
+          if-no-files-found: error
       - uses: actions/upload-artifact@v4
         with:
           name: windows-native-controls
@@ -505,6 +511,12 @@ jobs:
           tests/process-global/updateMaintenance.test.ts
           tests/conformance/providers/ghidra/realGhidraWindowsWorkflow.test.ts
       - run: npm run verify:windows-native
+      - uses: actions/download-artifact@v4
+        with:
+          name: windows-native-inspection-fixture
+          path: .cache/windows-native-inspection-fixture/
+      - name: Verify native inspection failure ownership and recovery
+        run: node scripts/verify-windows-native-inspection.mjs
       - name: Verify real Node Inspector discovery and observation
         timeout-minutes: 10
         env:

docs/testing.md

@@ -482,6 +482,13 @@ not require Ghidra or Java. It includes Job Object commit-limit controls (a
 limited child that reaches the limit, a control that does not, and invalid
 limit rejection). An optional independently compiled Windows fixture
 adds in-place reparse, breakaway, environment, and token observations.
+The separate `scripts/verify-windows-native-inspection.mjs` lane uses the
+`.cache/windows-native-inspection-fixture` artifact built on Linux with
+`node scripts/build-windows-native.mjs --inspection-fixture`. Its controlled
+first deletion failure checks retained NTFS ownership, later-query recovery,
+successful-admission caching, and environment-teardown cleanup. It neither
+establishes the cause of the intermittent #1701 host failure nor replaces
+verification with the shipped production artifact.
 `npm run verify:ghidra:windows:package` additionally packs and installs REA into
 an isolated prefix and runs ordinary-user CLI/MCP operations against their
 canonical schemas. Run the controlled fixture generator before that lane, or

native/windows/README.md

@@ -106,6 +106,30 @@ npm run verify:windows-native
 npm run verify:ghidra:windows
 ```
 
+The native inspection probe retains its directory owner if deletion fails. A
+later availability query retries that cleanup before creating another probe;
+successful admission and static artifact failures remain cached. Environment
+teardown makes one final cleanup attempt and reports a persistent failure to
+stderr rather than claiming the directory was removed.
+
+CI separately builds and verifies a test-only artifact:
+
+```sh
+node scripts/build-windows-native.mjs --inspection-fixture
+```
+
+On Windows, after compiling the runtime:
+
+```powershell
+node scripts/verify-windows-native-inspection.mjs
+```
+
+This artifact injects one access-denied disposition result while using a real
+NTFS directory. The verifier checks retained ownership, recovery on a later
+query, cached successful admission, and cleanup on environment teardown. It
+does not reproduce or identify the host-dependent cause reported in #1701.
+The fixture is stored under `.cache/` and is excluded from the shipped addon.
+
 The native lane checks DACL readback independently, source write/replacement
 rejection, unrelated sibling renames, reparse rejection, immutable snapshots,
 asynchronous cancellation, cleanup containment,

native/windows/src/addon.cc

@@ -1,6 +1,7 @@
 #include "authority.hpp"
 
 #include <delayimp.h>
+#include <cstdio>
 
 namespace rea {
 
@@ -89,17 +90,45 @@ napi_value failureValue(napi_env env, const Failure& failure) {
     return error;
 }
 
+struct InspectionContext { std::unique_ptr<Runtime> probe; };
+
+static void cleanupInspection(void* data) noexcept {
+  std::unique_ptr<InspectionContext> context(static_cast<InspectionContext*>(data));
+  if (!context->probe) return;
+  try { closeRuntime(*context->probe); }
+  catch (const Failure& failure) {
+    // Environment teardown cannot retain an owner or return an error. Report
+    // terminal cleanup failure instead of claiming the directory was removed.
+    std::fprintf(stderr, "REA Windows probe cleanup failed (Win32 %lu) path=\"",
+                 static_cast<unsigned long>(failure.win32));
+    // Emit reversible UTF-16 escapes without allocation or CRT locale conversion.
+    for (wchar_t unit : failure.path) {
+      if (unit == L'"' || unit == L'\\') std::fputc('\\', stderr);
+      if (unit >= 0x20 && unit <= 0x7e) std::fputc(static_cast<int>(unit), stderr);
+      else std::fprintf(stderr, "\\u%04x", static_cast<unsigned>(unit));
+    }
+    std::fprintf(stderr, "\"\n");
+  }
+  catch (const std::exception& failure) {
+    std::fprintf(stderr, "REA Windows probe cleanup failed: %s\n", failure.what());
+  }
+  catch (...) {
+    std::fprintf(stderr, "REA Windows probe cleanup failed: unknown native exception\n");
+  }
+}
+
 static napi_value call(napi_env env, napi_callback_info info) {
   try {
     std::array<napi_value, 2> args;
     size_t count = args.size();
-    check(napi_get_cb_info(env, info, &count, args.data(), nullptr, nullptr));
+    void* data = nullptr;
+    check(napi_get_cb_info(env, info, &count, args.data(), nullptr, &data));
     require(count == 2, "Native call requires operation and arguments", L"", ERROR_INVALID_PARAMETER);
     const auto operation = wide(env, args[0]);
     const auto values = array(env, args[1]);
     if (operation == L"inspect") {
       require(values.empty(), "Inspect takes no arguments", L"", ERROR_INVALID_PARAMETER);
-      return inspect(env);
+      return inspect(env, static_cast<InspectionContext*>(data)->probe);
     }
     if (operation.rfind(L"process_", 0) == 0) return processCall(env, operation, values);
     return filesystemCall(env, operation, values);
@@ -114,8 +143,13 @@ static napi_value call(napi_env env, napi_callback_info info) {
 }
 
 static napi_value initialize(napi_env env, napi_value exports) {
+  auto context = std::make_unique<InspectionContext>();
+  if (napi_add_env_cleanup_hook(env, cleanupInspection, context.get()) != napi_ok) return nullptr;
+  // The environment owns callback data before a function can expose it. Failed
+  // initialization still leaves a valid context for the registered cleanup hook.
+  auto* data = context.release();
   napi_value function;
-  if (napi_create_function(env, "call", NAPI_AUTO_LENGTH, call, nullptr, &function) != napi_ok) return nullptr;
+  if (napi_create_function(env, "call", NAPI_AUTO_LENGTH, call, data, &function) != napi_ok) return nullptr;
   if (napi_set_named_property(env, exports, "call", function) != napi_ok) return nullptr;
   return exports;
 }

native/windows/src/authority.hpp

@@ -132,6 +132,6 @@ std::unique_ptr<Process> spawnProcess(const std::wstring& command, const std::ws
                                     bool protocolStdin = false, ULONGLONG memoryLimit = 0);
 napi_value processCall(napi_env env, const std::wstring& operation,
                        const std::vector<napi_value>& args);
-napi_value inspect(napi_env env);
+napi_value inspect(napi_env env, std::unique_ptr<Runtime>& probe);
 
 } // namespace rea

native/windows/src/process.cc

@@ -412,13 +412,17 @@ napi_value processCall(napi_env env, const std::wstring& operation, const std::v
   throw Failure("Unknown native process operation", L"", ERROR_INVALID_PARAMETER);
 }
 
-napi_value inspect(napi_env env) {
+napi_value inspect(napi_env env, std::unique_ptr<Runtime>& probe) {
+  // Settle the previous owner before admitting another probe. Failed cleanup
+  // retains its handles, including when identity construction itself failed.
+  if (probe) { closeRuntime(*probe); probe.reset(); }
   wchar_t temp[32768];
   const auto length = GetTempPathW(32768, temp);
   require(length > 0 && length < 32768, "Read temporary directory coordinate failed");
-  auto root = createRuntime(std::wstring(temp, length), L"rea-native-probe-");
-  auto filesystem = identity(env, root->directory.get(), root->path);
-  closeRuntime(*root);
+  probe = createRuntime(std::wstring(temp, length), L"rea-native-probe-");
+  auto filesystem = identity(env, probe->directory.get(), probe->path);
+  closeRuntime(*probe);
+  probe.reset();
   auto job = createJob();
   Attributes attributes(1);
   HANDLE raw = job.get();

scripts/build-windows-native.mjs

@@ -11,7 +11,12 @@ const root = join(dirname(fileURLToPath(import.meta.url)), "..");
 const nodeVersion = "22.19.0";
 const compiler = "x86_64-w64-mingw32-g++";
 const cache = join(homedir(), ".cache", "rea-windows-native");
-const output = join(root, "native", "windows", "build");
+const inspectionFixture = process.argv[2] === "--inspection-fixture";
+if (process.argv.length > 3 || (process.argv.length > 2 && !inspectionFixture))
+  throw new Error("Unknown Windows native build argument");
+const output = inspectionFixture
+  ? join(root, ".cache", "windows-native-inspection-fixture")
+  : join(root, "native", "windows", "build");
 const run = (command, args) => {
   const result = spawnSync(command, args, {
     cwd: root,
@@ -120,7 +125,9 @@ run(compiler, [
   "-Wl,--no-insert-timestamp",
   `-I${headers}`,
   "native/windows/src/addon.cc",
-  "native/windows/src/filesystem.cc",
+  inspectionFixture
+    ? "tests/fixtures/windows-native-inspection-failure.cc"
+    : "native/windows/src/filesystem.cc",
   "native/windows/src/process.cc",
   library,
   "-ladvapi32",

scripts/verify-windows-native-inspection.mjs

@@ -0,0 +1,95 @@
+import assert from "node:assert/strict";
+import { execFileSync } from "node:child_process";
+import {
+  copyFile,
+  mkdir,
+  mkdtemp,
+  readdir,
+  rm,
+  symlink,
+  writeFile,
+} from "node:fs/promises";
+import { createRequire } from "node:module";
+import { tmpdir } from "node:os";
+import { dirname, join, resolve } from "node:path";
+import { pathToFileURL } from "node:url";
+
+if (process.platform !== "win32")
+  throw new Error("Native inspection ownership requires real Windows NTFS");
+const root = resolve(process.argv[2] ?? ".");
+const artifact = resolve(
+  process.argv[3] ?? ".cache/windows-native-inspection-fixture",
+);
+const workspace = await mkdtemp(join(tmpdir(), "rea-inspection-conformance-"));
+try {
+  // Isolate the fixture from the packaged production artifact and loader cache.
+  await mkdir(join(workspace, "dist", "windows"), { recursive: true });
+  await mkdir(join(workspace, "native", "windows", "build"), {
+    recursive: true,
+  });
+  await writeFile(join(workspace, "package.json"), '{"type":"module"}');
+  await copyFile(
+    join(root, "dist", "windows", "WindowsNativeLoader.js"),
+    join(workspace, "dist", "windows", "WindowsNativeLoader.js"),
+  );
+  await copyFile(
+    join(root, "dist", "generatedPackageMetadata.js"),
+    join(workspace, "dist", "generatedPackageMetadata.js"),
+  );
+  for (const name of ["manifest.json", "rea-windows-x64.node"])
+    await copyFile(
+      join(artifact, name),
+      join(workspace, "native", "windows", "build", name),
+    );
+  const require = createRequire(join(root, "package.json"));
+  await mkdir(join(workspace, "node_modules"));
+  await symlink(
+    dirname(require.resolve("zod/package.json")),
+    join(workspace, "node_modules", "zod"),
+    "junction",
+  );
+  const loader = pathToFileURL(
+    join(workspace, "dist", "windows", "WindowsNativeLoader.js"),
+  ).href;
+  const source = `import assert from 'node:assert/strict';
+import {readdirSync,renameSync,statSync} from 'node:fs';
+import {join} from 'node:path';
+import {systemWindowsNativeAuthority} from ${JSON.stringify(loader)};
+const temp=process.env.TEMP;
+const first=systemWindowsNativeAuthority();
+assert.equal(first.available,false);
+assert.equal(first.retryable,true);
+assert.match(first.reason,/Delete owned object by handle failed.*Win32 5/s);
+const [name,...extra]=readdirSync(temp);assert.ok(name);assert.equal(extra.length,0);
+const path=join(temp,name);const identity=statSync(path,{bigint:true});
+assert.throws(()=>renameSync(path,path+'-replacement'));
+assert.equal(statSync(path,{bigint:true}).ino,identity.ino);
+if(process.argv[2]!=='teardown'){
+ const second=systemWindowsNativeAuthority();assert.equal(second.available,true);
+ assert.equal(readdirSync(temp).length,0);
+ assert.equal(systemWindowsNativeAuthority(),second);
+}
+console.log(JSON.stringify({mode:process.argv[2],firstFailed:true,retainedOwner:true,inode:identity.ino.toString()}));
+`;
+  const fixture = join(workspace, "probe.mjs");
+  await writeFile(fixture, source);
+  for (const mode of ["retry", "teardown"]) {
+    const temp = join(workspace, mode);
+    await mkdir(temp);
+    const observed = JSON.parse(
+      execFileSync(process.execPath, [fixture, mode], {
+        encoding: "utf8",
+        timeout: 20_000,
+        windowsHide: true,
+        env: { ...process.env, TEMP: temp, TMP: temp },
+      }),
+    );
+    assert.equal(observed.retainedOwner, true);
+    assert.deepEqual(await readdir(temp), []);
+  }
+  process.stdout.write(
+    `${JSON.stringify({ ok: true, controlledDispositionFailure: true, retainedOwner: true, retryRecovers: true, successfulAdmissionCached: true, environmentTeardownCleansProbe: true, issue1701HostCauseReproduced: false })}\n`,
+  );
+} finally {
+  await rm(workspace, { recursive: true, force: true });
+}

src/windows/WindowsNativeLoader.test.ts

@@ -169,17 +169,18 @@ describe("packaged Windows native admission", () => {
       }),
     ).toMatchObject({
       available: false,
+      retryable: true,
       reason: expect.stringContaining("private DACL: access denied"),
     });
   });
 
   it("requires every inspected native fact", () => {
     const value = fixture();
-    expect(
-      loadWindowsNativeAuthority({
-        ...value.host,
-        loadArtifact: () => ({ call: () => ({ privateDacl: true }) }),
-      }).available,
-    ).toBe(false);
+    const loaded = loadWindowsNativeAuthority({
+      ...value.host,
+      loadArtifact: () => ({ call: () => ({ privateDacl: true }) }),
+    });
+    expect(loaded.available).toBe(false);
+    expect(loaded).not.toHaveProperty("retryable");
   });
 });

src/windows/WindowsNativeLoader.ts

@@ -67,7 +67,12 @@ export interface WindowsNativeAuthority {
 /** Availability of the bundled native implementation, including load failures. */
 export type WindowsNativeLoadResult =
   | { readonly available: true; readonly authority: WindowsNativeAuthority }
-  | { readonly available: false; readonly reason: string };
+  | {
+      readonly available: false;
+      readonly reason: string;
+      /** Only a failed native invocation can recover on a later query. */
+      readonly retryable?: true;
+    };
 
 /** Trusted package reads and module loading, replaceable in loader tests. */
 export interface WindowsNativeLoadHost {
@@ -106,7 +111,17 @@ export const loadWindowsNativeAuthority = (
       );
     assertWindowsX64Artifact(artifact);
     const binding = bindingSchema.parse(host.loadArtifact());
-    const inspection = inspectionSchema.parse(binding.call("inspect", []));
+    let observed: unknown;
+    try {
+      observed = binding.call("inspect", []);
+    } catch (cause: unknown) {
+      return {
+        available: false,
+        retryable: true,
+        reason: `REA Windows native self-test failed: ${cause instanceof Error ? cause.message : String(cause)}`,
+      };
+    }
+    const inspection = inspectionSchema.parse(observed);
     return { available: true, authority: { call: binding.call, inspection } };
   } catch (cause: unknown) {
     return {
@@ -131,8 +146,10 @@ const assertWindowsX64Artifact = (bytes: Buffer): void => {
 
 let cached: WindowsNativeLoadResult | undefined;
 
-/** Load only the package-owned artifact, lazily and once per process. */
+/** Cache stable admission; let a later query retry failed owned inspection. */
 export const systemWindowsNativeAuthority = (): WindowsNativeLoadResult => {
+  if (cached?.available === false && cached.retryable === true)
+    cached = undefined;
   cached ??= loadWindowsNativeAuthority({
     platform: process.platform,
     architecture: process.arch,

tests/fixtures/windows-native-inspection-failure.cc

@@ -0,0 +1,20 @@
+// Only the separate conformance artifact uses this translation unit. The
+// shipped addon always compiles filesystem.cc directly, without interposition.
+#include "../../native/windows/src/authority.hpp"
+#include <atomic>
+
+static BOOL inspectionDisposition(HANDLE handle, FILE_INFO_BY_HANDLE_CLASS kind,
+                                  LPVOID value, DWORD size) {
+  // A real NTFS probe is created normally. Fail its first disposition call so
+  // the verifier can observe ownership after failure, independently of host
+  // filter drivers or the intermittent error reported in issue #1701.
+  static std::atomic_flag failed = ATOMIC_FLAG_INIT;
+  if (kind == FileDispositionInfo && !failed.test_and_set()) {
+    SetLastError(ERROR_ACCESS_DENIED);
+    return FALSE;
+  }
+  return SetFileInformationByHandle(handle, kind, value, size);
+}
+
+#define SetFileInformationByHandle inspectionDisposition
+#include "../../native/windows/src/filesystem.cc"