feat(serve+sdk): F4 prereq — daemon protocol completion (serverTimestamp / provenance / errorKind / state_resync_required)

packages/acp-bridge/src/bridgeClient.test.ts

@@ -160,6 +160,192 @@ describe('BridgeClient — BridgeFileSystem injection seam (F1 step 5)', () => {
     });
   });
 
+  describe('FsError preservation over ACP wire (#4175 F4 prereq, Codex #4360 round 2)', () => {
+    // The fix scope: when `BridgeFileSystem.writeText` /
+    // `BridgeFileSystem.readText` throw a structured `FsError`, the
+    // BridgeClient must rethrow as ACP `RequestError` with `data.
+    // errorKind` / `data.hint` / `data.status` preserved. Pre-fix
+    // the ACP SDK serialized only `error.message` so SDK consumers
+    // lost the discriminator and had to regex-match the message.
+    //
+    // FsError lives in `cli/src/serve/fs/errors.ts` — acp-bridge can't
+    // import it (cross-package dep inversion), so we synthesize the
+    // shape directly here. The duck typing in
+    // `preserveFsErrorOverAcp` keys on `err.name === 'FsError'` +
+    // `typeof err.kind === 'string'`.
+
+    function makeFsError(
+      kind: string,
+      message: string,
+      extras: { hint?: string; status?: number } = {},
+    ): Error {
+      const err = new Error(message);
+      err.name = 'FsError';
+      (err as unknown as { kind: string }).kind = kind;
+      if (extras.hint !== undefined) {
+        (err as unknown as { hint: string }).hint = extras.hint;
+      }
+      if (extras.status !== undefined) {
+        (err as unknown as { status: number }).status = extras.status;
+      }
+      return err;
+    }
+
+    it('writeTextFile rethrows FsError as ACP RequestError with errorKind in data', async () => {
+      const writeText = vi.fn(async (): Promise<WriteTextFileResponse> => {
+        throw makeFsError(
+          'untrusted_workspace',
+          'workspace is not trusted; write operations are forbidden',
+          {
+            status: 403,
+            hint: 'enable trust via createWorkspaceFileSystemFactory',
+          },
+        );
+      });
+      const client = makeClient({ writeText, readText: vi.fn() });
+
+      const err = (await client
+        .writeTextFile({
+          path: '/x',
+          content: 'y',
+          sessionId: 'sess:test',
+        })
+        .catch((e) => e)) as Error & { code?: number; data?: unknown };
+
+      // Reshaped as JSON-RPC RequestError (-32603 = internal error)
+      // with structured data field.
+      expect(err.name).toBe('RequestError');
+      expect(err.code).toBe(-32603);
+      expect(err.message).toContain('not trusted');
+      expect(err.data).toMatchObject({
+        errorKind: 'untrusted_workspace',
+        status: 403,
+        hint: expect.any(String),
+      });
+    });
+
+    it('readTextFile rethrows FsError preserving symlink_escape kind', async () => {
+      const readText = vi.fn(async (): Promise<ReadTextFileResponse> => {
+        throw makeFsError(
+          'symlink_escape',
+          'symlink resolves outside workspace',
+          { status: 400 },
+        );
+      });
+      const client = makeClient({ writeText: vi.fn(), readText });
+
+      const err = (await client
+        .readTextFile({ path: '/x', sessionId: 'sess:test' })
+        .catch((e) => e)) as Error & { code?: number; data?: unknown };
+
+      expect(err.name).toBe('RequestError');
+      expect(err.code).toBe(-32603);
+      expect(err.data).toMatchObject({
+        errorKind: 'symlink_escape',
+        status: 400,
+      });
+      // No `hint` field on this FsError → not stamped (spread guard).
+      expect((err.data as { hint?: unknown }).hint).toBeUndefined();
+    });
+
+    it('passes non-FsError errors through unchanged (no RequestError wrap)', async () => {
+      // Plain Error → bridgeClient must NOT wrap it. Only structured
+      // FsError gets the reshape. ACP's default serialization is
+      // adequate for unstructured errors.
+      const writeText = vi.fn(async (): Promise<WriteTextFileResponse> => {
+        throw new Error('boring generic failure');
+      });
+      const client = makeClient({ writeText, readText: vi.fn() });
+
+      const err = (await client
+        .writeTextFile({
+          path: '/x',
+          content: 'y',
+          sessionId: 'sess:test',
+        })
+        .catch((e) => e)) as Error & { code?: number; data?: unknown };
+
+      // Original Error preserved — no JSON-RPC code stamped.
+      expect(err.name).toBe('Error');
+      expect(err.message).toBe('boring generic failure');
+      expect(err.code).toBeUndefined();
+      expect(err.data).toBeUndefined();
+    });
+
+    it('readTextFile passes non-FsError errors through unchanged (wenshao #4360 review)', async () => {
+      // Symmetric guard for the read-side `preserveFsErrorOverAcp`
+      // call. The write- and read-side catch blocks are independent
+      // try/catch wrappers in `bridgeClient.ts`; if a future refactor
+      // diverges them (e.g. adds Error-wrapping to one but not the
+      // other), this test catches the read-side regression.
+      const readText = vi.fn(async (): Promise<ReadTextFileResponse> => {
+        throw new Error('generic read failure');
+      });
+      const client = makeClient({ writeText: vi.fn(), readText });
+
+      const err = (await client
+        .readTextFile({ path: '/x', sessionId: 'sess:test' })
+        .catch((e) => e)) as Error & { code?: number; data?: unknown };
+
+      expect(err.name).toBe('Error');
+      expect(err.message).toBe('generic read failure');
+      expect(err.code).toBeUndefined();
+      expect(err.data).toBeUndefined();
+    });
+
+    it('preserves hint field when present on the FsError', async () => {
+      const writeText = vi.fn(async (): Promise<WriteTextFileResponse> => {
+        throw makeFsError(
+          'file_too_large',
+          'file of 6 MiB exceeds write cap of 5 MiB',
+          { hint: 'split large writes into bounded chunks', status: 413 },
+        );
+      });
+      const client = makeClient({ writeText, readText: vi.fn() });
+
+      const err = (await client
+        .writeTextFile({
+          path: '/x',
+          content: 'y',
+          sessionId: 'sess:test',
+        })
+        .catch((e) => e)) as Error & { code?: number; data?: unknown };
+
+      expect((err.data as { hint?: string }).hint).toBe(
+        'split large writes into bounded chunks',
+      );
+      expect((err.data as { errorKind?: string }).errorKind).toBe(
+        'file_too_large',
+      );
+    });
+
+    it('does not wrap an error that LOOKS like FsError but has wrong name', async () => {
+      // Defensive: an unrelated error class with a `kind` field but
+      // a different `name` should fall through to the unstructured
+      // path. Prevents accidental wrapping of e.g. permission errors
+      // that happen to carry a `kind` discriminator.
+      const writeText = vi.fn(async (): Promise<WriteTextFileResponse> => {
+        const err = new Error('looks-similar');
+        err.name = 'PermissionForbiddenError';
+        (err as unknown as { kind: string }).kind =
+          'designated_originator_mismatch';
+        throw err;
+      });
+      const client = makeClient({ writeText, readText: vi.fn() });
+
+      const err = (await client
+        .writeTextFile({
+          path: '/x',
+          content: 'y',
+          sessionId: 'sess:test',
+        })
+        .catch((e) => e)) as Error & { code?: number };
+
+      expect(err.name).toBe('PermissionForbiddenError');
+      expect(err.code).toBeUndefined();
+    });
+  });
+
   describe('inline fallback when fileSystem is omitted (regression guard)', () => {
     let tmpDir: string;
     beforeEach(async () => {

packages/acp-bridge/src/bridgeClient.ts

@@ -17,6 +17,7 @@ import type {
   WriteTextFileRequest,
   WriteTextFileResponse,
 } from '@agentclientprotocol/sdk';
+import { RequestError } from '@agentclientprotocol/sdk';
 import type { BridgeEvent, EventBus } from './eventBus.js';
 import type { BridgeFileSystem } from './bridgeFileSystem.js';
 import { CANCEL_VOTE_SENTINEL } from './permissionMediator.js';
@@ -35,6 +36,67 @@ import type {
 import { CancelSentinelCollisionError } from './bridgeErrors.js';
 import { writeStderrLine } from './internal/stderrLine.js';
 
+/**
+ * Duck-type check for `FsError` from `cli/src/serve/fs/errors.ts`
+ * (#4175 F4 prereq — Codex review on #4360 round 2). FsError lives
+ * in the `cli` package, but this class lives in `acp-bridge` — a
+ * direct `import { FsError }` would invert the dependency. We use
+ * the same `.name`-based duck typing that `mapDomainErrorToErrorKind`
+ * (status.ts) already applies to `TrustGateError` / `SkillError`
+ * for the same cross-package bundling reason.
+ *
+ * Without this preservation: when the `BridgeFileSystem` adapter
+ * throws an `FsError` (e.g. `kind: 'untrusted_workspace'`, `kind:
+ * 'symlink_escape'`, `kind: 'file_too_large'`), the ACP SDK's
+ * default RPC error path serializes only `error.message` as
+ * "Internal error" — the structured `kind` / `status` / `hint` are
+ * lost on the wire. SDK consumers downstream can no longer dispatch
+ * typed UI (auth retry vs file picker vs proxy hint) without
+ * regex-matching the human-readable message.
+ *
+ * With this preservation: the bridge boundary catches FsError,
+ * rethrows as ACP `RequestError(-32603, message, {errorKind, hint,
+ * status})`. The agent's RPC client receives `data.errorKind` and
+ * can branch on the closed-enum kind. JSON-RPC code stays at
+ * internal-error (-32603) since the bridge can't reliably map
+ * FsError.kind to a JSON-RPC error code shape — the structured
+ * `data` field is what carries semantic information for SDK
+ * consumers.
+ */
+interface FsErrorShape {
+  name: 'FsError';
+  message: string;
+  kind: string;
+  status?: number;
+  hint?: string;
+}
+
+function isFsErrorShape(err: unknown): err is FsErrorShape {
+  return (
+    err instanceof Error &&
+    err.name === 'FsError' &&
+    typeof (err as { kind?: unknown }).kind === 'string'
+  );
+}
+
+/**
+ * Rethrow an FsError as a structured ACP `RequestError` so the
+ * agent's RPC client sees `data.errorKind` / `data.hint` /
+ * `data.status` rather than just the human-readable message.
+ * Non-FsError errors are rethrown unchanged — the default ACP
+ * serialization is fine for unstructured errors.
+ */
+function preserveFsErrorOverAcp(err: unknown): never {
+  if (isFsErrorShape(err)) {
+    throw new RequestError(-32603, err.message, {
+      errorKind: err.kind,
+      ...(err.hint !== undefined ? { hint: err.hint } : {}),
+      ...(err.status !== undefined ? { status: err.status } : {}),
+    });
+  }
+  throw err;
+}
+
 /**
  * #4175 F3 Commit 3 — translate the mediator's internal
  * `PermissionResolution` to the ACP-shaped `RequestPermissionResponse`
@@ -647,7 +709,17 @@ export class BridgeClient implements Client {
     // injection and fall through to the inline path so pre-F1 behavior
     // is preserved verbatim where no adapter has been wired.
     if (this.fileSystem) {
-      return this.fileSystem.writeText(params);
+      // #4175 F4 prereq — preserve FsError structure over ACP wire
+      // (Codex review on #4360 round 2). Without this catch, an
+      // `FsError({kind:'untrusted_workspace'})` from the adapter
+      // would land at the agent as `{code:-32603, message:...}` with
+      // the kind/status/hint stripped. See `preserveFsErrorOverAcp`
+      // for the cross-package duck-typing rationale.
+      try {
+        return await this.fileSystem.writeText(params);
+      } catch (err) {
+        preserveFsErrorOverAcp(err);
+      }
     }
     // Stage 1 known divergence: this raw `fs.writeFile` reimplements file
     // I/O instead of delegating to core's filesystem service. The
@@ -795,7 +867,13 @@ export class BridgeClient implements Client {
     // Mode A + channels + IDE companion fall through to the inline
     // proxy below.
     if (this.fileSystem) {
-      return this.fileSystem.readText(params);
+      // #4175 F4 prereq — preserve FsError structure over ACP wire.
+      // See sibling block in `writeTextFile` for rationale.
+      try {
+        return await this.fileSystem.readText(params);
+      } catch (err) {
+        preserveFsErrorOverAcp(err);
+      }
     }
     // Reject obviously-degenerate `limit` up front. Without this,
     // `sliceLineRange` hits the `end < start` path and returns an