From 204bce591663562be28439269968a298a3561f5e Mon Sep 17 00:00:00 2001 From: Chisanan232 Date: Sat, 4 Jul 2026 23:46:14 +0800 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=94=92=20(hooks):=20Remove=20fail-ope?= =?UTF-8?q?n=20catch=20in=20ai-sdk/openai-agents=20adapters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ai-sdk and openai-agents adapters wrapped the pre-execution gateway check and approval wait in try { … } catch { return executeOriginal() }, so a caller-supplied gatewayClient that throws on a transport error was silently treated as ALLOW. The with-assembly / wrap-tool wrappers have no such catch: an un-caught check() rejects and blocks the tool. Drop the swallowing catches so all wrappers share one fail-closed posture; faults now propagate (reject) under enforce. Existing tests updated to assert the fault propagates and the original tool does not run. refs AAASM-4137 --- src/hooks/ai-sdk.ts | 37 ++++++++++++++------------------ src/hooks/openai-agents.ts | 37 ++++++++++++++------------------ tests/openai-agents-hook.test.ts | 22 ++++++++++--------- tests/vercel-ai-hook.test.ts | 22 +++++++++++-------- 4 files changed, 57 insertions(+), 61 deletions(-) diff --git a/src/hooks/ai-sdk.ts b/src/hooks/ai-sdk.ts index f1f4ada96..ca8eab7f9 100644 --- a/src/hooks/ai-sdk.ts +++ b/src/hooks/ai-sdk.ts @@ -86,17 +86,17 @@ export function createWrappedExecute( return options.agentId ? runWithAgentId(options.agentId, run) : run(); }; - let decision; - try { - decision = await gatewayClient.check({ - action: "tool_call", - toolName: description, - args, - runId - }); - } catch { - return executeOriginal(); - } + // Let check / approval faults propagate (reject) instead of swallowing them + // into a silent fail-open. A caller-supplied gatewayClient that throws on a + // transport error must NOT be treated as ALLOW — this matches the + // with-assembly / wrap-tool wrappers, whose un-caught check() rejects and + // blocks the tool under enforce (AAASM-4137). + const decision = await gatewayClient.check({ + action: "tool_call", + toolName: description, + args, + runId + }); if (decision.denied) { throw new PolicyViolationError( @@ -105,16 +105,11 @@ export function createWrappedExecute( } if (decision.pending) { - let approval; - try { - approval = await gatewayClient.waitForApproval( - description, - runId, - options.approvalTimeoutMs - ); - } catch { - return executeOriginal(); - } + const approval = await gatewayClient.waitForApproval( + description, + runId, + options.approvalTimeoutMs + ); if (approval.denied) { throw new PolicyViolationError( `Approval rejected: ${approval.reason ?? "Rejected"}` diff --git a/src/hooks/openai-agents.ts b/src/hooks/openai-agents.ts index 48a345c07..a33500e72 100644 --- a/src/hooks/openai-agents.ts +++ b/src/hooks/openai-agents.ts @@ -130,17 +130,17 @@ export function createPatchedRunTool( return result; }; - let decision; - try { - decision = await gatewayClient.check({ - action: "tool_call", - toolName, - args, - runId - }); - } catch { - return executeOriginal(); - } + // Let check / approval faults propagate (reject) instead of swallowing them + // into a silent fail-open. A caller-supplied gatewayClient that throws on a + // transport error must NOT be treated as ALLOW — this matches the + // with-assembly / wrap-tool wrappers, whose un-caught check() rejects and + // blocks the tool under enforce (AAASM-4137). + const decision = await gatewayClient.check({ + action: "tool_call", + toolName, + args, + runId + }); if (decision.denied) { return formatDeniedToolCallOutput( @@ -150,16 +150,11 @@ export function createPatchedRunTool( } if (decision.pending) { - let pendingOutput; - try { - pendingOutput = await handlePendingApproval(gatewayClient, { - toolName, - runId, - timeoutMs: options.approvalTimeoutMs - }); - } catch { - return executeOriginal(); - } + const pendingOutput = await handlePendingApproval(gatewayClient, { + toolName, + runId, + timeoutMs: options.approvalTimeoutMs + }); if (pendingOutput) { return pendingOutput; } diff --git a/tests/openai-agents-hook.test.ts b/tests/openai-agents-hook.test.ts index 170f4cdbb..f3d1f0f2c 100644 --- a/tests/openai-agents-hook.test.ts +++ b/tests/openai-agents-hook.test.ts @@ -271,7 +271,7 @@ describe("openai agents adapter", () => { }); }); - it("fails open and executes original tool when gateway check throws", async () => { + it("propagates the fault and blocks the tool when gateway check throws", async () => { const gateway = createGatewayClientMock(); gateway.check = vi.fn(async () => { throw new Error("gateway unavailable"); @@ -284,7 +284,7 @@ describe("openai agents adapter", () => { approvalTimeoutMs: 4_000 }); - const result = await patchedRunTool( + const error = await patchedRunTool( { function: { name: "critical_tool", @@ -294,13 +294,14 @@ describe("openai agents adapter", () => { { runId: "run-6" } - ); + ).catch((e: Error) => e); - expect(result).toEqual({ ok: "fallback-path" }); - expect(originalRunTool).toHaveBeenCalledTimes(1); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("gateway unavailable"); + expect(originalRunTool).not.toHaveBeenCalled(); }); - it("fails open and executes original tool when approval handling throws on PENDING", async () => { + it("propagates the fault and blocks the tool when approval handling throws on PENDING", async () => { const gateway = createGatewayClientMock(); gateway.check = vi.fn(async () => ({ pending: true, denied: false })); gateway.waitForApproval = vi.fn(async () => { @@ -314,13 +315,14 @@ describe("openai agents adapter", () => { approvalTimeoutMs: 2_000 }); - const result = await patchedRunTool( + const error = await patchedRunTool( { function: { name: "pending_tool", arguments: "{}" } }, { runId: "run-pending-throw" } - ); + ).catch((e: Error) => e); - expect(result).toEqual({ ok: "approval-fail-open" }); - expect(originalRunTool).toHaveBeenCalledTimes(1); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("approval channel unavailable"); + expect(originalRunTool).not.toHaveBeenCalled(); }); it("returns true without re-patching when already patched", async () => { diff --git a/tests/vercel-ai-hook.test.ts b/tests/vercel-ai-hook.test.ts index 8cd9961b9..6e4bdc262 100644 --- a/tests/vercel-ai-hook.test.ts +++ b/tests/vercel-ai-hook.test.ts @@ -159,7 +159,7 @@ describe("vercel ai sdk adapter", () => { ); }); - it("fails open and executes original tool when gateway check throws", async () => { + it("propagates the fault and blocks the tool when gateway check throws", async () => { const gateway = createGatewayClientMock(); gateway.check = vi.fn(async () => { throw new Error("gateway unavailable"); @@ -174,13 +174,14 @@ describe("vercel ai sdk adapter", () => { { approvalTimeoutMs: 4_000, fallbackRunId: "fallback" } ); - const result = await wrappedExecute( + const error = await wrappedExecute( { x: 1 }, { toolCallId: "call-6" } - ); + ).catch((e: Error) => e); - expect(result).toEqual({ ok: "fallback-path" }); - expect(originalExecute).toHaveBeenCalledTimes(1); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("gateway unavailable"); + expect(originalExecute).not.toHaveBeenCalled(); }); it("records results in fire-and-forget mode without surfacing recorder failures", async () => { @@ -242,7 +243,7 @@ describe("vercel ai sdk adapter", () => { expect(fakeModule.tool).toBe(originalTool); }); - it("fails open and executes the original tool when approval-wait throws on PENDING", async () => { + it("propagates the fault and blocks the tool when approval-wait throws on PENDING", async () => { const gateway = createGatewayClientMock(); gateway.check = vi.fn(async () => ({ pending: true, denied: false })); gateway.waitForApproval = vi.fn(async () => { @@ -258,10 +259,13 @@ describe("vercel ai sdk adapter", () => { { approvalTimeoutMs: 2_000, fallbackRunId: "fallback" } ); - const result = await wrappedExecute({ x: 1 }, { toolCallId: "call-pending-throw" }); + const error = await wrappedExecute({ x: 1 }, { toolCallId: "call-pending-throw" }).catch( + (e: Error) => e + ); - expect(result).toEqual({ ok: "approval-fail-open" }); - expect(originalExecute).toHaveBeenCalledTimes(1); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe("approval channel unavailable"); + expect(originalExecute).not.toHaveBeenCalled(); }); it("returns true without re-patching when patchVercelAiSdk is already patched", async () => { From fd5838d65d5265daf61706562612c07cb20dc762 Mon Sep 17 00:00:00 2001 From: Chisanan232 Date: Sat, 4 Jul 2026 23:48:17 +0800 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=90=9B=20(postinstall):=20Resolve=20t?= =?UTF-8?q?he=20actually-published=20native=20package=20layout?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit postinstall resolved @agent-assembly/ binary packages that were never declared nor published (the declared @agent-assembly/runtime-* optionalDeps ship the aasm CLI, not a .node), so runPostinstall always threw, got caught, warned, and no-opped — masking a real binary-provisioning regression. The napi .node binaries actually ship in-package under native/aa-ffi-node/ and are resolved by index.cjs at load time. Verify that bundled binding is present for the current platform (mirroring index.cjs resolution) so a missing binary fails loudly at install rather than only at first native load. All names stay under the org-owned @agent-assembly scope. refs AAASM-4137 --- scripts/postinstall.d.mts | 21 ++---- scripts/postinstall.mjs | 109 ++++++++++++++---------------- tests/scripts/postinstall.test.ts | 88 ++++++++++++++---------- 3 files changed, 108 insertions(+), 110 deletions(-) diff --git a/scripts/postinstall.d.mts b/scripts/postinstall.d.mts index 0e62eaf96..3e94e2c52 100644 --- a/scripts/postinstall.d.mts +++ b/scripts/postinstall.d.mts @@ -1,28 +1,17 @@ export function detectPlatformKey(platform?: string, arch?: string): string | null; -export function resolveBinaryPackageName( - platform?: string, - arch?: string -): string | null; - -export function findFirstNodeBinary(dirPath: string): string | null; - -export function resolveBinaryFromPackage( - packageName: string, - options?: { cwd?: string } -): string; +export function findBundledNativeBinary(nativeDir: string): string | null; export function selectBinaryForCurrentPlatform(options?: { platform?: string; arch?: string; cwd?: string; - targetNativeDir?: string; + nativeDir?: string; logger?: { info: (message: string) => void; warn: (message: string) => void }; }): | { - packageName: string; - sourceBinaryPath: string; - targetBinaryPath: string; + platformKey: string; + binaryPath: string; } | null; @@ -30,7 +19,7 @@ export function runPostinstall(options?: { platform?: string; arch?: string; cwd?: string; - targetNativeDir?: string; + nativeDir?: string; logger?: { info: (message: string) => void; warn: (message: string) => void }; }): boolean; diff --git a/scripts/postinstall.mjs b/scripts/postinstall.mjs index d6a5b6c49..a2b6a4dda 100644 --- a/scripts/postinstall.mjs +++ b/scripts/postinstall.mjs @@ -1,9 +1,24 @@ import fs from "node:fs"; import path from "node:path"; -import { createRequire } from "node:module"; import { pathToFileURL } from "node:url"; -const requireFromHere = createRequire(import.meta.url); +/** + * Native-binding provisioning check run at install time. + * + * The napi-rs `.node` binaries ship *inside* `@agent-assembly/sdk` under + * `native/aa-ffi-node/` (see package.json `files` → `native/aa-ffi-node/*.node`) + * and are resolved at load time by `native/aa-ffi-node/index.cjs`. There is no + * separately published per-platform `@agent-assembly/` binary package + * to install from — the declared `@agent-assembly/runtime-*` optionalDependencies + * ship the `aasm` CLI, not a `.node`. The earlier postinstall resolved phantom + * `@agent-assembly/` names that were never published, so it always + * threw, got caught, warned, and no-opped — masking a genuine missing-binary + * regression until the loud failure at first native load (AAASM-4137). + * + * This script therefore *verifies* the bundled binding for the current platform + * is present (mirroring `index.cjs` resolution) so a provisioning regression + * fails at install time rather than only at first `initAssembly`. + */ const SUPPORTED_PLATFORM_KEYS = { "darwin-arm64": "darwin-arm64", @@ -12,58 +27,33 @@ const SUPPORTED_PLATFORM_KEYS = { "win32-x64": "win32-x64-msvc" }; +const NATIVE_BINDING_DIR = "native/aa-ffi-node"; + export function detectPlatformKey(platform = process.platform, arch = process.arch) { return SUPPORTED_PLATFORM_KEYS[`${platform}-${arch}`] ?? null; } -export function resolveBinaryPackageName(platform = process.platform, arch = process.arch) { - const platformKey = detectPlatformKey(platform, arch); - - if (!platformKey) { - return null; +/** + * Locate the bundled native binding inside `native/aa-ffi-node/`, mirroring the + * runtime loader in `native/aa-ffi-node/index.cjs`: prefer the exact + * `index.node`, otherwise the first platform-suffixed `index..node`. + * Returns the absolute path, or `null` when no binding is present. + */ +export function findBundledNativeBinary(nativeDir) { + const directPath = path.join(nativeDir, "index.node"); + if (fs.existsSync(directPath)) { + return directPath; } - return `@agent-assembly/${platformKey}`; -} - -export function findFirstNodeBinary(dirPath) { - const entries = fs.readdirSync(dirPath, { withFileTypes: true }); - - for (const entry of entries) { - const entryPath = path.join(dirPath, entry.name); - - if (entry.isDirectory()) { - const nested = findFirstNodeBinary(entryPath); - if (nested) { - return nested; - } - } - - if (entry.isFile() && entry.name.endsWith(".node")) { - return entryPath; - } + if (!fs.existsSync(nativeDir)) { + return null; } - return null; -} - -export function resolveBinaryFromPackage( - packageName, - options = {} -) { - const { cwd = process.cwd() } = options; - - const packageJsonPath = requireFromHere.resolve(`${packageName}/package.json`, { - paths: [cwd] - }); - const packageDir = path.dirname(packageJsonPath); - const binaryPath = findFirstNodeBinary(packageDir); - - if (!binaryPath) { - throw new Error(`No .node file found in ${packageName}`); - } + const platformAddon = fs + .readdirSync(nativeDir) + .find((name) => /^index\..+\.node$/.test(name)); - return binaryPath; + return platformAddon ? path.join(nativeDir, platformAddon) : null; } export function selectBinaryForCurrentPlatform(options = {}) { @@ -71,31 +61,34 @@ export function selectBinaryForCurrentPlatform(options = {}) { platform = process.platform, arch = process.arch, cwd = process.cwd(), - targetNativeDir = path.resolve(cwd, "native/aa-ffi-node"), + nativeDir = path.resolve(cwd, NATIVE_BINDING_DIR), logger = console } = options; - const packageName = resolveBinaryPackageName(platform, arch); + const platformKey = detectPlatformKey(platform, arch); - if (!packageName) { + if (!platformKey) { logger.warn( - `[agent-assembly] Unsupported platform: ${platform}-${arch}; skipping binary selection.` + `[agent-assembly] Unsupported platform: ${platform}-${arch}; skipping native binding check.` ); return null; } - const sourceBinaryPath = resolveBinaryFromPackage(packageName, { cwd }); - const targetBinaryPath = path.join(targetNativeDir, "index.node"); + const binaryPath = findBundledNativeBinary(nativeDir); - fs.mkdirSync(targetNativeDir, { recursive: true }); - fs.copyFileSync(sourceBinaryPath, targetBinaryPath); + if (!binaryPath) { + throw new Error( + `No bundled native binding (.node) found in ${NATIVE_BINDING_DIR} for ${platformKey}` + ); + } - logger.info(`[agent-assembly] Selected binary package ${packageName}`); + logger.info( + `[agent-assembly] Native binding present for ${platformKey}: ${path.basename(binaryPath)}` + ); return { - packageName, - sourceBinaryPath, - targetBinaryPath + platformKey, + binaryPath }; } @@ -107,7 +100,7 @@ export function runPostinstall(options = {}) { return true; } catch (error) { logger.warn( - `[agent-assembly] Failed to select native binary: ${error instanceof Error ? error.message : String(error)}` + `[agent-assembly] Failed to verify native binding: ${error instanceof Error ? error.message : String(error)}` ); return false; } diff --git a/tests/scripts/postinstall.test.ts b/tests/scripts/postinstall.test.ts index e8f484cab..024975acf 100644 --- a/tests/scripts/postinstall.test.ts +++ b/tests/scripts/postinstall.test.ts @@ -4,8 +4,8 @@ import { pathToFileURL } from "node:url"; import { afterEach, describe, expect, it, vi } from "vitest"; import { detectPlatformKey, + findBundledNativeBinary, isExecutedDirectly, - resolveBinaryPackageName, runPostinstallEntrypoint, runPostinstall, selectBinaryForCurrentPlatform @@ -19,20 +19,16 @@ function createTempDir() { return dir; } -function seedPlatformPackage( - cwd: string, - packageName: string, - options: { withBinary: boolean } -) { - const packageDir = path.join(cwd, "node_modules", ...packageName.split("/")); - fs.mkdirSync(packageDir, { recursive: true }); - fs.writeFileSync(path.join(packageDir, "package.json"), JSON.stringify({ name: packageName }, null, 2)); - - if (options.withBinary) { - const binaryDir = path.join(packageDir, "prebuilds"); - fs.mkdirSync(binaryDir, { recursive: true }); - fs.writeFileSync(path.join(binaryDir, "index.node"), "fake-native-binary"); - } +/** + * Seed a bundled native binding under `native/aa-ffi-node/` in a temp cwd, + * mirroring the actually-published layout (the `.node` ships in-package and is + * resolved by native/aa-ffi-node/index.cjs — not from a separate npm package). + */ +function seedBundledBinary(cwd: string, fileName: string) { + const nativeDir = path.join(cwd, "native", "aa-ffi-node"); + fs.mkdirSync(nativeDir, { recursive: true }); + fs.writeFileSync(path.join(nativeDir, fileName), "fake-native-binary"); + return nativeDir; } afterEach(() => { @@ -42,16 +38,36 @@ afterEach(() => { }); describe("postinstall script", () => { - it("maps platform and arch to supported package names", () => { + it("maps platform and arch to supported platform keys", () => { expect(detectPlatformKey("linux", "x64")).toBe("linux-x64-gnu"); expect(detectPlatformKey("darwin", "arm64")).toBe("darwin-arm64"); - expect(resolveBinaryPackageName("win32", "x64")).toBe("@agent-assembly/win32-x64-msvc"); - expect(resolveBinaryPackageName("sunos", "x64")).toBeNull(); + expect(detectPlatformKey("win32", "x64")).toBe("win32-x64-msvc"); + expect(detectPlatformKey("sunos", "x64")).toBeNull(); }); - it("copies the selected platform binary into native/aa-ffi-node/index.node", () => { + it("resolves the bundled binary the same way index.cjs does", () => { const cwd = createTempDir(); - seedPlatformPackage(cwd, "@agent-assembly/linux-x64-gnu", { withBinary: true }); + const nativeDir = seedBundledBinary(cwd, "index.node"); + + // Exact index.node is preferred. + expect(findBundledNativeBinary(nativeDir)).toBe(path.join(nativeDir, "index.node")); + + // Falls back to a platform-suffixed index..node. + fs.rmSync(path.join(nativeDir, "index.node")); + fs.writeFileSync(path.join(nativeDir, "index.linux-x64-gnu.node"), "fake"); + expect(findBundledNativeBinary(nativeDir)).toBe( + path.join(nativeDir, "index.linux-x64-gnu.node") + ); + + // Returns null when no binding is present. + fs.rmSync(path.join(nativeDir, "index.linux-x64-gnu.node")); + expect(findBundledNativeBinary(nativeDir)).toBeNull(); + expect(findBundledNativeBinary(path.join(cwd, "does-not-exist"))).toBeNull(); + }); + + it("confirms the bundled binding present for the current platform", () => { + const cwd = createTempDir(); + seedBundledBinary(cwd, "index.node"); const logger = { info: vi.fn(), @@ -65,13 +81,12 @@ describe("postinstall script", () => { logger }); - expect(result?.packageName).toBe("@agent-assembly/linux-x64-gnu"); - expect(result?.targetBinaryPath).toBe( - path.join(cwd, "native/aa-ffi-node", "index.node") + expect(result?.platformKey).toBe("linux-x64-gnu"); + expect(result?.binaryPath).toBe( + path.join(cwd, "native", "aa-ffi-node", "index.node") ); - expect(fs.readFileSync(result!.targetBinaryPath, "utf8")).toBe("fake-native-binary"); expect(logger.info).toHaveBeenCalledWith( - "[agent-assembly] Selected binary package @agent-assembly/linux-x64-gnu" + "[agent-assembly] Native binding present for linux-x64-gnu: index.node" ); expect(logger.warn).not.toHaveBeenCalled(); }); @@ -90,13 +105,13 @@ describe("postinstall script", () => { expect(result).toBeNull(); expect(logger.warn).toHaveBeenCalledWith( - "[agent-assembly] Unsupported platform: freebsd-arm64; skipping binary selection." + "[agent-assembly] Unsupported platform: freebsd-arm64; skipping native binding check." ); }); - it("returns true for successful postinstall selection", () => { + it("returns true when the bundled binding is present", () => { const cwd = createTempDir(); - seedPlatformPackage(cwd, "@agent-assembly/darwin-arm64", { withBinary: true }); + seedBundledBinary(cwd, "index.darwin-arm64.node"); const logger = { info: vi.fn(), @@ -114,9 +129,10 @@ describe("postinstall script", () => { expect(logger.warn).not.toHaveBeenCalled(); }); - it("returns false and warns when package exists but no binary is found", () => { + it("returns false and warns loudly when no bundled binding is found", () => { const cwd = createTempDir(); - seedPlatformPackage(cwd, "@agent-assembly/linux-x64-gnu", { withBinary: false }); + // native/aa-ffi-node exists but ships no .node — a provisioning regression. + fs.mkdirSync(path.join(cwd, "native", "aa-ffi-node"), { recursive: true }); const logger = { info: vi.fn(), @@ -133,20 +149,20 @@ describe("postinstall script", () => { expect(ok).toBe(false); expect(logger.warn).toHaveBeenCalledTimes(1); expect(logger.warn.mock.calls[0]?.[0]).toContain( - "[agent-assembly] Failed to select native binary: No .node file found in @agent-assembly/linux-x64-gnu" + "[agent-assembly] Failed to verify native binding: No bundled native binding (.node) found in native/aa-ffi-node for linux-x64-gnu" ); }); it("stringifies non-Error throwables when logging postinstall failure", () => { const cwd = createTempDir(); - seedPlatformPackage(cwd, "@agent-assembly/linux-x64-gnu", { withBinary: true }); + seedBundledBinary(cwd, "index.node"); const logger = { info: vi.fn(), warn: vi.fn() }; - const copySpy = vi.spyOn(fs, "copyFileSync").mockImplementation(() => { + const readSpy = vi.spyOn(fs, "existsSync").mockImplementation(() => { throw "raw-failure"; }); @@ -154,16 +170,16 @@ describe("postinstall script", () => { platform: "linux", arch: "x64", cwd, - logger, + logger }); expect(ok).toBe(false); expect(logger.warn).toHaveBeenCalledTimes(1); expect(logger.warn).toHaveBeenCalledWith( - "[agent-assembly] Failed to select native binary: raw-failure" + "[agent-assembly] Failed to verify native binding: raw-failure" ); - copySpy.mockRestore(); + readSpy.mockRestore(); }); it("detects direct execution and runs entrypoint only in main mode", () => {