From a750dc71920ae6f4e59d7d6bf096f85d9a4a2eac Mon Sep 17 00:00:00 2001 From: Guy Ben Aharon Date: Sun, 14 Jun 2026 19:16:03 +0300 Subject: [PATCH] feat: add listAgents and make ensureAgent idempotent at the agent cap ensureAgent is documented as idempotent ("returns normally if it already exists"), but it broke at the project's agent cap. The server evaluates the agent quota before the identifier-uniqueness check, so re-creating an existing identifier while at the cap returned 403 (quota) instead of the 409 (conflict) that ensureAgent treats as success -- so ensureAgent threw even though the agent already existed. On a 403, ensureAgent now confirms whether the agent already exists (via a new listAgents() call) and, if so, resolves with created: false; a genuine quota error (agent does not exist) still surfaces. The happy path is unchanged -- the extra lookup only runs on a 403. Also adds a public listAgents() method and Agent type. Fixes #40 Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 16 ++++ src/agents/index.ts | 64 ++++++++++++++ src/agents/types.ts | 9 ++ src/client.ts | 8 ++ src/index.ts | 1 + test/agents/client.test.ts | 168 +++++++++++++++++++++++++++++++++++++ 6 files changed, 266 insertions(+) diff --git a/README.md b/README.md index ed00899..2112d43 100644 --- a/README.md +++ b/README.md @@ -215,6 +215,20 @@ console.log(agent.createdAt); // ISO 8601 timestamp **Returns** `{ id, name, identifier, createdAt }` +#### `onecli.listAgents(options?)` + +List all agents in the project. + +```typescript +const agents = await onecli.listAgents(); + +for (const agent of agents) { + console.log(agent.identifier, agent.isDefault); +} +``` + +**Returns** `Array<{ id, name, identifier, isDefault, createdAt }>` + #### `onecli.ensureAgent(input, options?)` Ensure an agent exists. Creates it if missing, returns normally if it already exists. @@ -228,6 +242,8 @@ const result = await onecli.ensureAgent({ console.log(result.created); // true if newly created, false if already existed ``` +Idempotent even at the agent cap: if the project is at its plan's agent limit but the target identifier already exists, the call still resolves with `created: false` instead of throwing a quota error. + **Returns** `{ name, identifier, created }` --- diff --git a/src/agents/index.ts b/src/agents/index.ts index 6075d62..8b0860c 100644 --- a/src/agents/index.ts +++ b/src/agents/index.ts @@ -4,6 +4,7 @@ import { toOneCLIError, } from "../errors.js"; import type { + Agent, CreateAgentInput, CreateAgentResponse, EnsureAgentResponse, @@ -78,6 +79,55 @@ export class AgentsClient { } }; + /** + * List all agents in the project. + */ + listAgents = async (options?: RequestOptions): Promise => { + const url = `${this.baseUrl}/v1/agents`; + + try { + const res = await fetch(url, { + method: "GET", + headers: this.buildHeaders(options), + signal: AbortSignal.timeout(this.timeout), + }); + + if (!res.ok) { + throw new OneCLIRequestError( + `OneCLI returned ${res.status} ${res.statusText}`, + { url, statusCode: res.status }, + ); + } + + return (await res.json()) as Agent[]; + } catch (error) { + if ( + error instanceof OneCLIError || + error instanceof OneCLIRequestError + ) { + throw error; + } + throw toOneCLIError(error); + } + }; + + /** + * Whether an agent with the given identifier already exists in the project. + * Swallows lookup failures and returns `false` so callers can fall back to + * surfacing their original error when existence can't be confirmed. + */ + private agentExists = async ( + identifier: string, + options?: RequestOptions, + ): Promise => { + try { + const agents = await this.listAgents(options); + return agents.some((a) => a.identifier === identifier); + } catch { + return false; + } + }; + /** * Ensure an agent exists. Creates it if missing, returns normally if it already exists. * Unlike `createAgent`, this method treats a 409 conflict as success. @@ -97,6 +147,20 @@ export class AgentsClient { created: false, }; } + // At the agent cap the server may evaluate the quota before the + // identifier-uniqueness check and return 403 where it would otherwise + // return 409 for an existing identifier. Re-creating an existing agent is + // a no-op, so confirm existence and treat it as success; only surface the + // quota error when the agent genuinely doesn't exist. See issue #40. + if (error instanceof OneCLIRequestError && error.statusCode === 403) { + if (await this.agentExists(input.identifier, options)) { + return { + name: input.name, + identifier: input.identifier, + created: false, + }; + } + } throw error; } }; diff --git a/src/agents/types.ts b/src/agents/types.ts index ae331f8..4d28740 100644 --- a/src/agents/types.ts +++ b/src/agents/types.ts @@ -16,6 +16,15 @@ export interface CreateAgentResponse { createdAt: string; } +export interface Agent { + id: string; + name: string; + identifier: string; + /** Whether this is the project's default agent. */ + isDefault: boolean; + createdAt: string; +} + export interface EnsureAgentResponse { name: string; identifier: string; diff --git a/src/client.ts b/src/client.ts index ee280dc..6fa9bde 100644 --- a/src/client.ts +++ b/src/client.ts @@ -10,6 +10,7 @@ import type { GetContainerConfigOptions, } from "./container/types.js"; import type { + Agent, CreateAgentInput, CreateAgentResponse, EnsureAgentResponse, @@ -92,6 +93,13 @@ export class OneCLI { return this.containerClient.applyContainerConfig(args, options); }; + /** + * List all agents in the project. + */ + listAgents = (options?: RequestOptions): Promise => { + return this.agentsClient.listAgents(options); + }; + /** * Create a new agent. */ diff --git a/src/index.ts b/src/index.ts index b8b0bf6..cb031b9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,6 +14,7 @@ export type { ApplyContainerConfigOptions, } from "./container/types.js"; export type { + Agent, CreateAgentInput, CreateAgentResponse, EnsureAgentResponse, diff --git a/test/agents/client.test.ts b/test/agents/client.test.ts index 58e3014..c0ad614 100644 --- a/test/agents/client.test.ts +++ b/test/agents/client.test.ts @@ -9,6 +9,14 @@ const MOCK_AGENT = { createdAt: "2025-01-01T00:00:00.000Z", }; +const MOCK_LIST_AGENT = { + id: "clxyz123abc", + name: "My Agent", + identifier: "my-agent", + isDefault: false, + createdAt: "2025-01-01T00:00:00.000Z", +}; + describe("AgentsClient", () => { let fetchSpy: ReturnType; @@ -181,6 +189,76 @@ describe("AgentsClient", () => { }); }); + describe("listAgents", () => { + it("sends GET with correct URL and auth header", async () => { + fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify([MOCK_LIST_AGENT]), { status: 200 }), + ); + + const client = new AgentsClient( + "http://localhost:3000", + "oc_mykey", + 5000, + ); + await client.listAgents(); + + expect(fetchSpy).toHaveBeenCalledWith( + "http://localhost:3000/v1/agents", + expect.objectContaining({ + method: "GET", + headers: { + "Content-Type": "application/json", + Authorization: "Bearer oc_mykey", + }, + }), + ); + }); + + it("returns parsed array on success", async () => { + fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify([MOCK_LIST_AGENT]), { status: 200 }), + ); + + const client = new AgentsClient( + "http://localhost:3000", + "oc_test", + 5000, + ); + const agents = await client.listAgents(); + + expect(agents).toEqual([MOCK_LIST_AGENT]); + }); + + it("throws OneCLIRequestError on 401", async () => { + fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({ error: "Unauthorized" }), { + status: 401, + statusText: "Unauthorized", + }), + ); + + const client = new AgentsClient("http://localhost:3000", "oc_bad", 5000); + + const err = await client.listAgents().catch((e: unknown) => e); + expect(err).toBeInstanceOf(OneCLIRequestError); + expect((err as OneCLIRequestError).statusCode).toBe(401); + }); + + it("wraps network errors into OneCLIError", async () => { + fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockRejectedValue(new TypeError("fetch failed")); + + const client = new AgentsClient( + "http://localhost:3000", + "oc_test", + 5000, + ); + + await expect(client.listAgents()).rejects.toThrow(OneCLIError); + }); + }); + describe("ensureAgent", () => { it("returns created: true when agent is newly created", async () => { fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( @@ -229,6 +307,96 @@ describe("AgentsClient", () => { }); }); + it("returns created: false on 403 when the agent already exists", async () => { + // At the agent cap the server may return 403 (quota) instead of 409 for + // an existing identifier. ensureAgent confirms existence via GET /agents. + fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + new Response(JSON.stringify({ error: "agents limit reached" }), { + status: 403, + statusText: "Forbidden", + }), + ) + .mockResolvedValueOnce( + new Response(JSON.stringify([MOCK_LIST_AGENT]), { status: 200 }), + ); + + const client = new AgentsClient( + "http://localhost:3000", + "oc_test", + 5000, + ); + const result = await client.ensureAgent({ + name: "My Agent", + identifier: "my-agent", + }); + + expect(result).toEqual({ + name: "My Agent", + identifier: "my-agent", + created: false, + }); + expect(fetchSpy).toHaveBeenCalledTimes(2); + }); + + it("re-throws the 403 when the agent does not exist (genuine quota cap)", async () => { + fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + new Response(JSON.stringify({ error: "agents limit reached" }), { + status: 403, + statusText: "Forbidden", + }), + ) + .mockResolvedValueOnce( + // Listing exists but does not include the requested identifier. + new Response( + JSON.stringify([{ ...MOCK_LIST_AGENT, identifier: "other" }]), + { status: 200 }, + ), + ); + + const client = new AgentsClient( + "http://localhost:3000", + "oc_test", + 5000, + ); + + const err = await client + .ensureAgent({ name: "My Agent", identifier: "my-agent" }) + .catch((e: unknown) => e); + expect(err).toBeInstanceOf(OneCLIRequestError); + expect((err as OneCLIRequestError).statusCode).toBe(403); + }); + + it("re-throws the original 403 when the existence check itself fails", async () => { + fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce( + new Response(JSON.stringify({ error: "agents limit reached" }), { + status: 403, + statusText: "Forbidden", + }), + ) + .mockResolvedValueOnce( + new Response("", { status: 500, statusText: "Internal Server Error" }), + ); + + const client = new AgentsClient( + "http://localhost:3000", + "oc_test", + 5000, + ); + + const err = await client + .ensureAgent({ name: "My Agent", identifier: "my-agent" }) + .catch((e: unknown) => e); + // The original 403 surfaces, not the 500 from the existence check. + expect(err).toBeInstanceOf(OneCLIRequestError); + expect((err as OneCLIRequestError).statusCode).toBe(403); + }); + it("throws on non-409 errors", async () => { fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue( new Response(JSON.stringify({ error: "Unauthorized" }), {