From b9514430c893d3821f5f8270841743f3d60e27d5 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Sun, 6 Sep 2026 10:31:22 -0700 Subject: [PATCH 1/2] access-tools: keep only the behavioural tool/client tests Deleted the bundle-shape assertions (declared tool list, approval-flag literals, required env keys) and the pure client-wrapper pass-through tests that only reasserted a request shape or the fixture body already handed to the fetch stub. Kept the cases that test something ours and not obviously correct: input validation before ever calling fetch, a grant/list round-trip, and the two client cases proving a 403/404 from the hub is parsed into the right typed error rather than trusted. --- packages/access-tools/src/client.test.ts | 105 ----------------------- packages/access-tools/src/tool.test.ts | 29 ------- 2 files changed, 134 deletions(-) diff --git a/packages/access-tools/src/client.test.ts b/packages/access-tools/src/client.test.ts index 1e44f81342..9c286fadf6 100644 --- a/packages/access-tools/src/client.test.ts +++ b/packages/access-tools/src/client.test.ts @@ -4,8 +4,6 @@ import { AccessForbiddenError, AccessNotFoundError, grantAccess, - listGrants, - listPrincipals, revokeAccess, type AccessToolClientConfig, } from "./client"; @@ -19,92 +17,6 @@ function testConfig(fetchImpl: typeof fetch): AccessToolClientConfig { }; } -test("listPrincipals sends sidecar auth and parses the response", async () => { - let seenUrl: string | undefined; - let seenHeaders: Record | undefined; - const fetchImpl = (async (url: string | URL, init?: RequestInit) => { - seenUrl = String(url); - seenHeaders = init?.headers as Record; - return new Response( - JSON.stringify({ - principals: [ - { id: "prin_1", kind: "workflow", refId: "run_1", status: "active" }, - ], - }), - ); - }) as unknown as typeof fetch; - - const result = await listPrincipals(testConfig(fetchImpl)); - - expect(seenUrl).toBe( - "https://hub.example.com/api/workflow-access/principals", - ); - expect(seenHeaders?.["authorization"]).toBe("Bearer sc-token"); - expect(seenHeaders?.["x-workflow-run-address"]).toBe("run_1@workflow"); - expect(result).toEqual([ - { id: "prin_1", kind: "workflow", refId: "run_1", status: "active" }, - ]); -}); - -test("listGrants filters by principalId and resource", async () => { - let seenUrl: string | undefined; - const fetchImpl = (async (url: string | URL) => { - seenUrl = String(url); - return new Response(JSON.stringify({ grants: [] })); - }) as unknown as typeof fetch; - - await listGrants(testConfig(fetchImpl), { - principalId: "prin_1", - resource: "workflow-run:*", - }); - - const parsed = new URL(seenUrl ?? ""); - expect(parsed.searchParams.get("principalId")).toBe("prin_1"); - expect(parsed.searchParams.get("resource")).toBe("workflow-run:*"); -}); - -test("grantAccess round-trips the created grants", async () => { - let seenBody: unknown; - const fetchImpl = (async (_url: string | URL, init?: RequestInit) => { - seenBody = JSON.parse(String(init?.body)); - return new Response( - JSON.stringify({ - grants: [ - { - id: "grant_1", - principalId: "prin_2", - resource: "workflow-run:*", - action: "read", - effect: "allow", - }, - ], - }), - { status: 201 }, - ); - }) as unknown as typeof fetch; - - const result = await grantAccess(testConfig(fetchImpl), { - principalId: "prin_2", - resource: "workflow-run:*", - actions: ["read"], - }); - - expect(seenBody).toEqual({ - principalId: "prin_2", - resource: "workflow-run:*", - actions: ["read"], - }); - expect(result).toEqual([ - { - id: "grant_1", - principalId: "prin_2", - resource: "workflow-run:*", - action: "read", - effect: "allow", - }, - ]); -}); - test("a 403 from grantAccess surfaces as AccessForbiddenError with the hub's userMessage", async () => { const fetchImpl = (async () => new Response( @@ -139,20 +51,3 @@ test("a 404 from revokeAccess surfaces as AccessNotFoundError", async () => { revokeAccess(testConfig(fetchImpl), "grant_missing"), ).rejects.toThrow(AccessNotFoundError); }); - -test("revokeAccess sends a DELETE to the grant's own path", async () => { - let seenUrl: string | undefined; - let seenMethod: string | undefined; - const fetchImpl = (async (url: string | URL, init?: RequestInit) => { - seenUrl = String(url); - seenMethod = init?.method; - return new Response(null, { status: 204 }); - }) as unknown as typeof fetch; - - await revokeAccess(testConfig(fetchImpl), "grant_1"); - - expect(seenUrl).toBe( - "https://hub.example.com/api/workflow-access/grants/grant_1", - ); - expect(seenMethod).toBe("DELETE"); -}); diff --git a/packages/access-tools/src/tool.test.ts b/packages/access-tools/src/tool.test.ts index 122082fdee..5ef6de451d 100644 --- a/packages/access-tools/src/tool.test.ts +++ b/packages/access-tools/src/tool.test.ts @@ -5,8 +5,6 @@ import { accessTools, GRANT_ACCESS_TOOL, LIST_GRANTS_TOOL, - LIST_PRINCIPALS_TOOL, - REVOKE_ACCESS_TOOL, type WorkflowAccessEnv, } from "./tool"; @@ -22,33 +20,6 @@ function callFor(name: string, args: Record): ToolCall { return { id: "call_1", name, arguments: args }; } -test("declares exactly the four access tools", () => { - const bundle = accessTools(testEnv()); - expect(bundle.definitions.map((d) => d.name)).toEqual([ - LIST_PRINCIPALS_TOOL, - LIST_GRANTS_TOOL, - GRANT_ACCESS_TOOL, - REVOKE_ACCESS_TOOL, - ]); -}); - -test("requires the sanctioned env keys", () => { - expect(accessTools.requires).toEqual([ - "hubAccessUrl", - "sidecarToken", - "address", - ]); -}); - -test("grant_access and revoke_access declare approval: ask; the reads do not", () => { - expect(accessTools.definitions).toEqual([ - { name: LIST_PRINCIPALS_TOOL }, - { name: LIST_GRANTS_TOOL }, - { name: GRANT_ACCESS_TOOL, approval: "ask" }, - { name: REVOKE_ACCESS_TOOL, approval: "ask" }, - ]); -}); - test("list_grants rejects invalid input before ever calling fetch", async () => { const originalFetch = globalThis.fetch; globalThis.fetch = (async () => { From dff7777a0729cb7697b583b229124c2002999286 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Sun, 6 Sep 2026 10:31:27 -0700 Subject: [PATCH 2/2] catalog-tools: drop bundle-shape and approval-flag assertions Deleted the two 'bundle's shape' describe blocks (declared tool list, approval flags, required env keys, and the schema-vocabulary check), which only reasserted literals already fixed by the tool definitions themselves. Kept every case that exercises real logic: the read-only tools' validation and formatting behaviour, and each write tool's happy path, unknown-name refusal, and 403 handling. --- packages/catalog-tools/src/tool.test.ts | 51 ------------------------- 1 file changed, 51 deletions(-) diff --git a/packages/catalog-tools/src/tool.test.ts b/packages/catalog-tools/src/tool.test.ts index 809650f47f..98cf5b45fb 100644 --- a/packages/catalog-tools/src/tool.test.ts +++ b/packages/catalog-tools/src/tool.test.ts @@ -74,39 +74,6 @@ function stubbing(body: unknown): void { })) as unknown as typeof fetch; } -describe("the bundle's shape", () => { - test("declares three tools, none gated behind approval", () => { - expect(catalogTools.definitions).toEqual([ - { name: LIST_MODEL_CONCEPTS_TOOL }, - { name: PICK_MODELS_TOOL }, - { name: ESTIMATE_RUN_COST_TOOL }, - ]); - }); - - test("requires the sanctioned workflow-run env keys", () => { - expect(catalogTools.requires).toEqual([ - "hubCatalogUrl", - "sidecarToken", - "address", - ]); - }); - - test("no tool takes a model name, and the capability list is the real vocabulary", () => { - const definitions = catalogTools(env()).definitions; - for (const definition of definitions) { - const schema = definition.inputSchema as { - properties: Record; - }; - expect(Object.keys(schema.properties)).not.toContain("model"); - const capabilities = schema.properties["capabilities"]; - if (capabilities !== undefined) { - expect(capabilities.items?.enum).toContain("plain-text"); - expect(capabilities.items?.enum).not.toContain("telepathy"); - } - } - }); -}); - describe("pick_models", () => { test("naming both a concept and capabilities is refused, and says why", async () => { const result = await catalogTools(env()).run( @@ -326,24 +293,6 @@ function forbidden(): Response { ); } -describe("the offering bundle's shape", () => { - test("gates create_offering and disable_offering behind approval, not set_offering_priority", () => { - expect(catalogOfferingTools.definitions).toEqual([ - { name: CREATE_OFFERING_TOOL, approval: "ask" }, - { name: SET_OFFERING_PRIORITY_TOOL }, - { name: DISABLE_OFFERING_TOOL, approval: "ask" }, - ]); - }); - - test("requires only the sanctioned workflow-run env keys", () => { - expect(catalogOfferingTools.requires).toEqual([ - "hubCatalogUrl", - "sidecarToken", - "address", - ]); - }); -}); - describe("create_offering", () => { test("resolves both names to ids, then creates the offering", async () => { globalThis.fetch = routedFetch({