fix(utils): throw AppError from app-log-files and verified-file guards

src/__tests__/test-utils/app-error.ts

@@ -1,43 +1,50 @@
 import assert from 'node:assert/strict';
-import { AppError } from '@agent-device/kernel/errors';
+import { AppError, normalizeError } from '@agent-device/kernel/errors';
+
+/**
+ * What an {@link AppError} assertion may pin. `hint` is checked against the
+ * hint a caller actually reads — `normalizeError`'s, not `details.hint` — so a
+ * throw site that drops its hint and silently inherits `defaultHintForCode`
+ * fails the assertion instead of passing on the default (ADR 0010: a hint is
+ * required wherever the per-code default would mislead).
+ */
+type ExpectedAppError = { code: string; message?: RegExp; hint?: string | RegExp };
+
+function assertAppError(error: unknown, expected: ExpectedAppError): true {
+  assert.ok(
+    error instanceof AppError,
+    `expected AppError, got ${error?.constructor?.name ?? typeof error}: ${String(error)}`,
+  );
+  assert.equal(error.code, expected.code);
+  if (expected.message) assert.match(error.message, expected.message);
+  if (expected.hint !== undefined) {
+    const { hint } = normalizeError(error);
+    assert.ok(typeof hint === 'string', `expected a hint on ${error.code}, got ${String(hint)}`);
+    if (typeof expected.hint === 'string') assert.equal(hint, expected.hint);
+    else assert.match(hint, expected.hint);
+  }
+  return true;
+}
 
 /**
  * Asserts that `run` rejects with an {@link AppError} carrying `code` and,
- * when given, a message matching `message`. Replaces the hand-rolled
+ * when given, a message matching `message` and the exact `hint` a caller
+ * reads. Replaces the hand-rolled
  * `assert.rejects(..., error instanceof AppError + code + match)` validator
  * repeated across platform tests.
  */
 export async function assertRejectsAppError(
   run: () => Promise<unknown>,
-  expected: { code: string; message?: RegExp },
+  expected: ExpectedAppError,
 ): Promise<void> {
-  await assert.rejects(run, (error: unknown) => {
-    assert.ok(
-      error instanceof AppError,
-      `expected AppError, got ${error?.constructor?.name ?? typeof error}: ${String(error)}`,
-    );
-    assert.equal(error.code, expected.code);
-    if (expected.message) assert.match(error.message, expected.message);
-    return true;
-  });
+  await assert.rejects(run, (error: unknown) => assertAppError(error, expected));
 }
 
 /**
  * Synchronous sibling of {@link assertRejectsAppError}: asserts that `fn`
  * throws an {@link AppError} carrying `code` and, when given, a message
- * matching `message`.
+ * matching `message` and the exact `hint`.
  */
-export function assertThrowsAppError(
-  fn: () => unknown,
-  expected: { code: string; message?: RegExp },
-): void {
-  assert.throws(fn, (error: unknown) => {
-    assert.ok(
-      error instanceof AppError,
-      `expected AppError, got ${error?.constructor?.name ?? typeof error}: ${String(error)}`,
-    );
-    assert.equal(error.code, expected.code);
-    if (expected.message) assert.match(error.message, expected.message);
-    return true;
-  });
+export function assertThrowsAppError(fn: () => unknown, expected: ExpectedAppError): void {
+  assert.throws(fn, (error: unknown) => assertAppError(error, expected));
 }

src/daemon/__tests__/app-log.test.ts

@@ -1,9 +1,14 @@
 import fs from 'node:fs';
 import path from 'node:path';
 import { expect, test } from 'vitest';
+import { assertThrowsAppError } from '../../__tests__/test-utils/index.ts';
 import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
 import { appendAppLogMarker, clearAppLogFiles, getAppLogPathMetadata } from '../app-log.ts';
 
+// Pinned as a literal on purpose — see the note in src/utils/__tests__/verified-file.test.ts.
+const NOT_REGULAR_FILE_HINT =
+  'agent-device only reads and writes regular files at this path. Remove the symbolic link or special file there and retry.';
+
 test('marker and clear operations keep app-log file ownership in the daemon', () => {
   const root = mkdtempForTestSync('agent-device-app-log-files-');
   const outPath = path.join(root, 'session', 'app.log');
@@ -37,11 +42,14 @@ test.each(['metadata', 'mark', 'clear'] as const)(
     // whose identity check reports the path as simply not a regular file.
     const expectedMessage =
       operation === 'mark' ? /must not be a symbolic link/ : /must be a regular file/;
-    expect(() => {
-      if (operation === 'metadata') getAppLogPathMetadata(outPath);
-      else if (operation === 'mark') appendAppLogMarker(outPath, 'checkpoint');
-      else clearAppLogFiles(outPath);
-    }).toThrow(expectedMessage);
+    assertThrowsAppError(
+      () => {
+        if (operation === 'metadata') getAppLogPathMetadata(outPath);
+        else if (operation === 'mark') appendAppLogMarker(outPath, 'checkpoint');
+        else clearAppLogFiles(outPath);
+      },
+      { code: 'COMMAND_FAILED', message: expectedMessage, hint: NOT_REGULAR_FILE_HINT },
+    );
     expect(fs.readFileSync(outsidePath, 'utf8')).toBe('outside');
     expect(fs.lstatSync(outPath).isSymbolicLink()).toBe(true);
   },

src/utils/__tests__/app-log-files.test.ts

@@ -1,9 +1,14 @@
 import fs from 'node:fs';
 import path from 'node:path';
 import { expect, test } from 'vitest';
+import { assertThrowsAppError } from '../../__tests__/test-utils/index.ts';
 import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
 import { ensureAppLogPath, rotateAppLogIfNeeded } from '../app-log-files.ts';
 
+// Pinned as a literal on purpose — see the note in verified-file.test.ts.
+const NOT_REGULAR_FILE_HINT =
+  'agent-device only reads and writes regular files at this path. Remove the symbolic link or special file there and retry.';
+
 test('rotateAppLogIfNeeded rotates files and discards the oldest generation', () => {
   const root = mkdtempForTestSync('agent-device-app-log-rotate-');
   const outPath = path.join(root, 'app.log');
@@ -39,9 +44,11 @@ test('rotation rejects a final app.log symlink without touching its target', ()
   fs.writeFileSync(outsidePath, 'outside');
   fs.symlinkSync(outsidePath, outPath);
 
-  expect(() => rotateAppLogIfNeeded(outPath, { maxBytes: 1, maxRotatedFiles: 1 })).toThrow(
-    'symbolic link',
-  );
+  assertThrowsAppError(() => rotateAppLogIfNeeded(outPath, { maxBytes: 1, maxRotatedFiles: 1 }), {
+    code: 'COMMAND_FAILED',
+    message: /must not be a symbolic link/,
+    hint: NOT_REGULAR_FILE_HINT,
+  });
   expect(fs.readFileSync(outsidePath, 'utf8')).toBe('outside');
   expect(fs.lstatSync(outPath).isSymbolicLink()).toBe(true);
 });

src/utils/__tests__/verified-file.test.ts

@@ -1,7 +1,16 @@
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
-import { afterEach, expect, test } from 'vitest';
+import { afterEach, expect, test, vi } from 'vitest';
+import { assertThrowsAppError } from '../../__tests__/test-utils/index.ts';
+
+// The recovery hints are pinned as literals, not imported from the module under test: an
+// assertion that compares the constant to itself stays green when the constant is deleted or
+// reworded, which is the whole behaviour #1792 adds (ADR 0010 — errors say how to recover).
+const NOT_REGULAR_FILE_HINT =
+  'agent-device only reads and writes regular files at this path. Remove the symbolic link or special file there and retry.';
+const CONCURRENT_REPLACEMENT_HINT =
+  'Another process replaced the file at this path while it was being opened. Stop the concurrent writer, then retry.';
 import {
   openVerifiedFileForAppend,
   openVerifiedFileForRead,
@@ -11,6 +20,7 @@ import {
 const roots: string[] = [];
 
 afterEach(() => {
+  vi.restoreAllMocks();
   for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true });
 });
 
@@ -39,19 +49,60 @@ test.each(['read', 'append', 'truncate'] as const)(
     fs.writeFileSync(outside, 'outside');
     fs.symlinkSync(outside, pathname);
 
-    expect(() => {
-      const descriptor =
-        operation === 'read'
-          ? openVerifiedFileForRead(pathname)
-          : operation === 'append'
-            ? openVerifiedFileForAppend(pathname)
-            : openVerifiedFileForTruncate(pathname);
-      if (descriptor !== undefined) fs.closeSync(descriptor);
-    }).toThrow('regular file');
+    assertThrowsAppError(
+      () => {
+        const descriptor =
+          operation === 'read'
+            ? openVerifiedFileForRead(pathname)
+            : operation === 'append'
+              ? openVerifiedFileForAppend(pathname)
+              : openVerifiedFileForTruncate(pathname);
+        if (descriptor !== undefined) fs.closeSync(descriptor);
+      },
+      { code: 'COMMAND_FAILED', message: /must be a regular file/, hint: NOT_REGULAR_FILE_HINT },
+    );
     expect(fs.readFileSync(outside, 'utf8')).toBe('outside');
   },
 );
 
+// The two race guards cannot be reached from the filesystem alone, so the interleaving is
+// planted: the path is swapped between the open and its post-open lstat, or the create keeps
+// losing to a concurrent creator. Both must surface as typed failures with a recovery hint.
+test('reports a typed failure when the file is swapped while it is being opened', () => {
+  const pathname = fixturePath('swapped');
+  const other = `${pathname}.other`;
+  fs.writeFileSync(pathname, 'first');
+  fs.writeFileSync(other, 'second');
+  const realLstat = fs.lstatSync;
+  let lstatCalls = 0;
+  vi.spyOn(fs, 'lstatSync').mockImplementation(((target: fs.PathLike, options?: unknown) => {
+    lstatCalls += 1;
+    // The second lstat is the post-open identity check; answer it with the other file.
+    const resolved = lstatCalls === 2 ? other : target;
+    return (realLstat as (path: fs.PathLike, options?: unknown) => fs.Stats)(resolved, options);
+  }) as typeof fs.lstatSync);
+
+  assertThrowsAppError(() => openVerifiedFileForRead(pathname), {
+    code: 'COMMAND_FAILED',
+    message: /identity changed while it was opened/,
+    hint: CONCURRENT_REPLACEMENT_HINT,
+  });
+});
+
+test('reports a typed failure when a create keeps losing the identity race', () => {
+  const pathname = fixturePath('contended');
+  const eexist = Object.assign(new Error('EEXIST: file already exists'), { code: 'EEXIST' });
+  vi.spyOn(fs, 'openSync').mockImplementation(() => {
+    throw eexist;
+  });
+
+  assertThrowsAppError(() => openVerifiedFileForAppend(pathname), {
+    code: 'COMMAND_FAILED',
+    message: /could not be opened without an identity race/,
+    hint: CONCURRENT_REPLACEMENT_HINT,
+  });
+});
+
 test('returns absent for a missing read without creating the file', () => {
   const pathname = fixturePath('missing');
   expect(openVerifiedFileForRead(pathname)).toBeUndefined();

src/utils/app-log-files.ts

@@ -1,5 +1,7 @@
 import fs from 'node:fs';
 import path from 'node:path';
+import { AppError } from '@agent-device/kernel/errors';
+import { lstatIfPresent, NOT_REGULAR_FILE_HINT } from './verified-file.ts';
 
 const DEFAULT_MAX_APP_LOG_BYTES = 5 * 1024 * 1024;
 const DEFAULT_MAX_ROTATED_FILES = 1;
@@ -37,16 +39,13 @@ function assertAppLogFileIsNotSymbolicLink(outPath: string): void {
 }
 
 function lstatAppLogFile(outPath: string): fs.Stats | undefined {
-  try {
-    const stats = fs.lstatSync(outPath);
-    if (stats.isSymbolicLink()) {
-      throw new Error(`App-log file must not be a symbolic link: ${outPath}`);
-    }
-    return stats;
-  } catch (error) {
-    if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined;
-    throw error;
+  const stats = lstatIfPresent(outPath);
+  if (stats?.isSymbolicLink()) {
+    throw new AppError('COMMAND_FAILED', `App-log file must not be a symbolic link: ${outPath}`, {
+      hint: NOT_REGULAR_FILE_HINT,
+    });
   }
+  return stats;
 }
 
 function positiveIntEnv(raw: string | undefined, fallback: number): number {

src/utils/verified-file.ts

@@ -1,4 +1,14 @@
 import fs from 'node:fs';
+import { AppError } from '@agent-device/kernel/errors';
+
+/**
+ * Both guards are the same failure mode seen from two sides — something other than a plain
+ * regular file sits at the final path — so the recovery is shared (ADR 0010 §3).
+ */
+export const NOT_REGULAR_FILE_HINT =
+  'agent-device only reads and writes regular files at this path. Remove the symbolic link or special file there and retry.';
+const CONCURRENT_REPLACEMENT_HINT =
+  'Another process replaced the file at this path while it was being opened. Stop the concurrent writer, then retry.';
 
 /** Opens a regular final-path file for verified reads, or returns absent. */
 export function openVerifiedFileForRead(pathname: string): number | undefined {
@@ -32,7 +42,11 @@ function openVerifiedFile(
     if (result.status === 'retry') continue;
     return result.status === 'missing' ? undefined : result.descriptor;
   }
-  throw new Error(`Final file could not be opened without an identity race: ${pathname}`);
+  throw new AppError(
+    'COMMAND_FAILED',
+    `Final file could not be opened without an identity race: ${pathname}`,
+    { hint: CONCURRENT_REPLACEMENT_HINT },
+  );
 }
 
 type VerifiedOpenAttempt =
@@ -100,15 +114,28 @@ function assertOpenedIdentity(pathname: string, descriptor: number, before?: fs.
   if (before && !sameFile(before, opened)) throw identityChangedError(pathname);
 }
 
-function identityChangedError(pathname: string): Error {
-  return new Error(`Final file identity changed while it was opened: ${pathname}`);
+function identityChangedError(pathname: string): AppError {
+  return new AppError(
+    'COMMAND_FAILED',
+    `Final file identity changed while it was opened: ${pathname}`,
+    { hint: CONCURRENT_REPLACEMENT_HINT },
+  );
 }
 
 function lstatRegularFile(pathname: string): fs.Stats | undefined {
+  const stats = lstatIfPresent(pathname);
+  if (stats && !stats.isFile()) {
+    throw new AppError('COMMAND_FAILED', `Final path must be a regular file: ${pathname}`, {
+      hint: NOT_REGULAR_FILE_HINT,
+    });
+  }
+  return stats;
+}
+
+/** `lstat` that treats absence as `undefined`; every other errno propagates. */
+export function lstatIfPresent(pathname: string): fs.Stats | undefined {
   try {
-    const stats = fs.lstatSync(pathname);
-    if (!stats.isFile()) throw new Error(`Final path must be a regular file: ${pathname}`);
-    return stats;
+    return fs.lstatSync(pathname);
   } catch (error) {
     if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined;
     throw error;