From 72c6a3f6371a56e882ac97f25d25c67e7733bf31 Mon Sep 17 00:00:00 2001 From: Bhavesh Patel Date: Thu, 3 Sep 2026 11:27:34 +0530 Subject: [PATCH 1/8] Add TrueFoundryAgentStore for SF remote agent sync --- .../20260902194500-truefoundry-agent-store.md | 5 + packages/trueforge/scripts/write-openapi.ts | 3 +- packages/trueforge/src/apis/agents.ts | 23 +- packages/trueforge/src/apis/schedules.ts | 6 +- packages/trueforge/src/apis/sessions.ts | 8 +- packages/trueforge/src/apis/turns.ts | 8 +- packages/trueforge/src/app.ts | 18 +- packages/trueforge/src/auth/middleware.ts | 2 +- packages/trueforge/src/main.ts | 90 +++- packages/trueforge/src/truefoundry/AGENTS.md | 1 + packages/trueforge/src/truefoundry/CLAUDE.md | 1 + .../src/truefoundry/TrueFoundryAgentStore.ts | 128 +++++ .../TrueFoundryServiceFoundryServerClient.ts | 105 +++- .../truefoundry/toPutRemoteAgentPayload.ts | 21 + .../trueforge/tests/unit/apis/agents.test.ts | 2 +- .../unit/apis/deletedSessionCrud.test.ts | 4 +- .../unit/apis/sandboxFileDownload.test.ts | 2 +- .../tests/unit/apis/schedules.test.ts | 2 +- .../tests/unit/apis/sessionHttp.test.ts | 2 +- .../unit/apis/turnHarnessErrorStatus.test.ts | 2 +- .../trueforge/tests/unit/apis/turns.test.ts | 6 +- .../truefoundry/TrueFoundryAgentStore.test.ts | 479 ++++++++++++++++++ 22 files changed, 848 insertions(+), 70 deletions(-) create mode 100644 .changeset/20260902194500-truefoundry-agent-store.md create mode 100644 packages/trueforge/src/truefoundry/AGENTS.md create mode 100644 packages/trueforge/src/truefoundry/CLAUDE.md create mode 100644 packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts create mode 100644 packages/trueforge/src/truefoundry/toPutRemoteAgentPayload.ts create mode 100644 packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts diff --git a/.changeset/20260902194500-truefoundry-agent-store.md b/.changeset/20260902194500-truefoundry-agent-store.md new file mode 100644 index 000000000..6c75fdb0d --- /dev/null +++ b/.changeset/20260902194500-truefoundry-agent-store.md @@ -0,0 +1,5 @@ +--- +"@truefoundry/trueforge": patch +--- + +Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. diff --git a/packages/trueforge/scripts/write-openapi.ts b/packages/trueforge/scripts/write-openapi.ts index 5d227f936..d8d6e71e7 100644 --- a/packages/trueforge/scripts/write-openapi.ts +++ b/packages/trueforge/scripts/write-openapi.ts @@ -59,6 +59,7 @@ function canonicalise(value: unknown): unknown { const sessionStore = new InMemorySessionStore(); const db = createSqliteDb(':memory:'); const tokenStore = new SqliteOAuthTokenStore(db); +const agentStore = new SqliteAgentStore(db); const app = createServerApp({ modelCatalog: ModelCatalog.load(), resolveModelProviderStore: () => new SqliteModelProviderStore(db), @@ -75,7 +76,7 @@ const app = createServerApp({ skillStore: new SqliteSkillStore(db), sandboxCatalog: SandboxCatalog.load(), sandboxProviderStore: new SqliteSandboxProviderStore(db), - agentStore: new SqliteAgentStore(db), + resolveAgentStore: () => agentStore, scheduleStore: new SqliteScheduleStore(db), sessionStore, sessionMetricsStore: new SqliteSessionMetricsStore(db), diff --git a/packages/trueforge/src/apis/agents.ts b/packages/trueforge/src/apis/agents.ts index f9cb16fc0..daee953ba 100644 --- a/packages/trueforge/src/apis/agents.ts +++ b/packages/trueforge/src/apis/agents.ts @@ -4,7 +4,12 @@ import { OpenAPIHono, type RouteHandler } from '@hono/zod-openapi'; import type { AgentSpec } from '@truefoundry/trueforge-core/agent-session'; import type { Context } from 'hono'; -import { AgentNameConflictError, type AgentRecord, type IAgentStore } from '../db/agentStore'; +import { + AgentExternalIdConflictError, + AgentNameConflictError, + type AgentRecord, + type IAgentStore, +} from '../db/agentStore'; import type { IMcpServerStore } from '../db/mcpServerStore'; import type { IModelProviderStore } from '../db/modelProviderStore'; import type { ISandboxProviderStore } from '../db/sandboxProviderStore'; @@ -24,7 +29,7 @@ import { buildAgentCodeSnippets } from './agentCodeSnippets'; import { TENANT_ID } from './sessions'; export interface AgentsRouterDeps { - agentStore: IAgentStore; + resolveAgentStore: (c: Context) => IAgentStore; resolveModelProviderStore: (c: Context) => IModelProviderStore; resolveMcpServerStore: (c: Context) => IMcpServerStore; skillStore: ISkillStore; @@ -65,7 +70,7 @@ async function validateManifest({ export function createAgentsRouter(deps: AgentsRouterDeps) { const listHandler: RouteHandler = async c => { - const records = await deps.agentStore.listAgents(TENANT_ID); + const records = await deps.resolveAgentStore(c).listAgents(TENANT_ID); return c.json({ data: records.map(toWireAgent) }, 200); }; @@ -78,7 +83,7 @@ export function createAgentsRouter(deps: AgentsRouterDeps(deps: AgentsRouterDeps(deps: AgentsRouterDeps = async c => { const { agent_id: agentId } = c.req.valid('param'); - const record = await deps.agentStore.getAgent({ tenant_id: TENANT_ID, id: agentId }); + const record = await deps.resolveAgentStore(c).getAgent({ tenant_id: TENANT_ID, id: agentId }); if (record === undefined) { return c.json({ error: { message: `Agent not found: ${agentId}` } }, 404); } @@ -104,7 +109,7 @@ export function createAgentsRouter(deps: AgentsRouterDeps = async c => { const { agent_id: agentId } = c.req.valid('param'); - const record = await deps.agentStore.getAgent({ tenant_id: TENANT_ID, id: agentId }); + const record = await deps.resolveAgentStore(c).getAgent({ tenant_id: TENANT_ID, id: agentId }); if (record === undefined) { return c.json({ error: { message: `Agent not found: ${agentId}` } }, 404); } @@ -121,7 +126,7 @@ export function createAgentsRouter(deps: AgentsRouterDeps = async c => { const { agent_id: agentId } = c.req.valid('param'); - await deps.agentStore.deleteAgent({ tenant_id: TENANT_ID, id: agentId }); + await deps.resolveAgentStore(c).deleteAgent({ tenant_id: TENANT_ID, id: agentId }); return c.json({}, 200); }; @@ -134,7 +139,7 @@ export function createAgentsRouter(deps: AgentsRouterDeps { scheduleStore: IScheduleStore; - agentStore: IAgentStore; + resolveAgentStore: (c: Context) => IAgentStore; sessions: Sessions; turnDeps: BeginTurnExecutionDeps; withTransaction: WithTransaction; @@ -185,7 +185,7 @@ export function createSchedulesRouter(deps: SchedulesRouterDeps { await startTurnInProcess({ ...turnParams, deps: deps.turnDeps }); }, @@ -217,7 +217,7 @@ export function createSchedulesRouter(deps: SchedulesRouterDeps IModelProviderStore; resolveMcpServerStore: (c: Context) => IMcpServerStore; skillStore: ISkillStore; - agentStore: IAgentStore; + resolveAgentStore: (c: Context) => IAgentStore; sandboxProviderStore: ISandboxProviderStore; redis?: RedisClientType | undefined; requestReplyRouter: RequestReplyRouter; @@ -227,7 +227,7 @@ type InternalSessionsRouterDeps = Pick< | 'resolveModelProviderStore' | 'resolveMcpServerStore' | 'skillStore' - | 'agentStore' + | 'resolveAgentStore' | 'sandboxProviderStore' | 'resolveUserContext' >; @@ -252,7 +252,7 @@ function createGetOrCreateSessionByExternalIdHandler( let agent: SessionRecord['agent']; if (isSessionAgentNameRef(body.agent)) { - const named = await deps.agentStore.getAgent({ tenant_id: TENANT_ID, name: body.agent.name }); + const named = await deps.resolveAgentStore(c).getAgent({ tenant_id: TENANT_ID, name: body.agent.name }); if (named === undefined) { return c.json({ error: { message: `Agent not found: ${body.agent.name}` } }, 404); } @@ -296,7 +296,7 @@ export function createSessionsRouter(deps: SessionsRouterDeps) { const sessionId = newId(); if (isSessionAgentNameRef(body.agent)) { - const agent = await deps.agentStore.getAgent({ tenant_id: TENANT_ID, name: body.agent.name }); + const agent = await deps.resolveAgentStore(c).getAgent({ tenant_id: TENANT_ID, name: body.agent.name }); if (agent === undefined) { return c.json({ error: { message: `Agent not found: ${body.agent.name}` } }, 404); } diff --git a/packages/trueforge/src/apis/turns.ts b/packages/trueforge/src/apis/turns.ts index 420069471..cb001db83 100644 --- a/packages/trueforge/src/apis/turns.ts +++ b/packages/trueforge/src/apis/turns.ts @@ -109,7 +109,7 @@ export interface TurnsRouterDeps { resolveMcpServerStore: (c: Context) => IMcpServerStore; tokenStore: IOAuthTokenStore; skillStore: ISkillStore; - agentStore: IAgentStore; + resolveAgentStore: (c: Context) => IAgentStore; /** Resumable live turn-event transport: create-turn writes, subscribe polls. */ eventSubscriptions: EventSubscriptionRegistry; sandboxProviderStore: ISandboxProviderStore; @@ -119,15 +119,16 @@ export interface TurnsRouterDeps { /** * Deps needed to create a turn and drain events in-process (no HTTP). Unlike the HTTP path, this - * carries already-resolved `modelProviderStore` / `mcpServerStore` (the scheduler has no request + * carries already-resolved `modelProviderStore` / `mcpServerStore` / `agentStore` (the scheduler has no request * context to resolve them). */ export type BeginTurnExecutionDeps = Pick< TurnsRouterDeps, - 'activeTurns' | 'eventSubscriptions' | 'tokenStore' | 'skillStore' | 'agentStore' | 'sandboxProviderStore' | 'logger' + 'activeTurns' | 'eventSubscriptions' | 'tokenStore' | 'skillStore' | 'sandboxProviderStore' | 'logger' > & { modelProviderStore: IModelProviderStore; mcpServerStore: IMcpServerStore; + agentStore: IAgentStore; }; /** @@ -675,6 +676,7 @@ export function createTurnsRouter(deps: TurnsRouterDeps) { ...deps, modelProviderStore: deps.resolveModelProviderStore(c), mcpServerStore: deps.resolveMcpServerStore(c), + agentStore: deps.resolveAgentStore(c), }, }; diff --git a/packages/trueforge/src/app.ts b/packages/trueforge/src/app.ts index 930ae409b..373a8db58 100644 --- a/packages/trueforge/src/app.ts +++ b/packages/trueforge/src/app.ts @@ -175,11 +175,15 @@ export interface ServerDeps { * Called without a context (e.g. the scheduler / OAuth callback) it returns the DB persistence store. */ resolveMcpServerStore: (c?: Context) => IMcpServerWithAuthStore; + /** + * Per-request store: DB singleton, or a token-bound TrueFoundry decorator in TrueFoundry mode. + * Called without a context (e.g. the scheduler) it returns the DB persistence store. + */ + resolveAgentStore: (c?: Context) => IAgentStore; withTransaction: WithTransaction; tokenStore: IOAuthTokenStore; skillStore: ISkillStore; sandboxProviderStore: ISandboxProviderStore; - agentStore: IAgentStore; scheduleStore: IScheduleStore; sessionStore: ISessionStore; sessionMetricsStore: ISessionMetricsStore; @@ -273,7 +277,7 @@ export function createServerApp(deps: ServerDeps) { '/api/v1/agents', withAuth( createAgentsRouter({ - agentStore: deps.agentStore, + resolveAgentStore: deps.resolveAgentStore, resolveModelProviderStore: deps.resolveModelProviderStore, resolveMcpServerStore: deps.resolveMcpServerStore, skillStore: deps.skillStore, @@ -287,7 +291,7 @@ export function createServerApp(deps: ServerDeps) { withAuth( createSchedulesRouter({ scheduleStore: deps.scheduleStore, - agentStore: deps.agentStore, + resolveAgentStore: deps.resolveAgentStore, sessions: deps.sessions, turnDeps: { activeTurns: deps.activeTurns, @@ -296,7 +300,7 @@ export function createServerApp(deps: ServerDeps) { mcpServerStore: deps.resolveMcpServerStore(), tokenStore: deps.tokenStore, skillStore: deps.skillStore, - agentStore: deps.agentStore, + agentStore: deps.resolveAgentStore(), sandboxProviderStore: deps.sandboxProviderStore, logger: deps.logger, }, @@ -328,7 +332,7 @@ export function createServerApp(deps: ServerDeps) { resolveModelProviderStore: deps.resolveModelProviderStore, resolveMcpServerStore: deps.resolveMcpServerStore, skillStore: deps.skillStore, - agentStore: deps.agentStore, + resolveAgentStore: deps.resolveAgentStore, sandboxProviderStore: deps.sandboxProviderStore, resolveUserContext, }), @@ -353,7 +357,7 @@ export function createServerApp(deps: ServerDeps) { resolveModelProviderStore: deps.resolveModelProviderStore, resolveMcpServerStore: deps.resolveMcpServerStore, skillStore: deps.skillStore, - agentStore: deps.agentStore, + resolveAgentStore: deps.resolveAgentStore, sandboxProviderStore: deps.sandboxProviderStore, redis: deps.redis, requestReplyRouter: deps.requestReplyRouter, @@ -373,7 +377,7 @@ export function createServerApp(deps: ServerDeps) { resolveMcpServerStore: deps.resolveMcpServerStore, tokenStore: deps.tokenStore, skillStore: deps.skillStore, - agentStore: deps.agentStore, + resolveAgentStore: deps.resolveAgentStore, eventSubscriptions: deps.eventSubscriptions, sandboxProviderStore: deps.sandboxProviderStore, logger: deps.logger, diff --git a/packages/trueforge/src/auth/middleware.ts b/packages/trueforge/src/auth/middleware.ts index 2d2bf2f94..7b5a75eaf 100644 --- a/packages/trueforge/src/auth/middleware.ts +++ b/packages/trueforge/src/auth/middleware.ts @@ -40,7 +40,7 @@ export function requireAccessToken(c: Context): string { const token = readAccessToken(c); if (!token) { throw new HTTPException(401, { - message: 'Authentication token required to list or call TrueFoundry models and MCP servers', + message: 'Authentication token required to list or call TrueFoundry models, MCP servers, and agents', }); } return token; diff --git a/packages/trueforge/src/main.ts b/packages/trueforge/src/main.ts index b3874a51c..abad9c3ef 100644 --- a/packages/trueforge/src/main.ts +++ b/packages/trueforge/src/main.ts @@ -73,6 +73,7 @@ import { PACKAGE_VERSION } from './packageVersion'; import { ActiveTurnRegistry } from './runtime/activeTurns'; import { EventSubscriptionRegistry } from './runtime/event-subscription'; import { printStandaloneStartupBanner } from './startupBanner'; +import { TrueFoundryAgentStore } from './truefoundry/TrueFoundryAgentStore'; import { TrueFoundryMcpServerStore } from './truefoundry/TrueFoundryMcpServerStore'; import { TrueFoundryModelProviderStore } from './truefoundry/TrueFoundryModelProviderStore'; import { TrueFoundryServiceFoundryServerClient } from './truefoundry/TrueFoundryServiceFoundryServerClient'; @@ -83,38 +84,49 @@ interface ServerPersistence { sessionMetricsStore: ISessionMetricsStore; resolveModelProviderStore: (c?: Context) => IModelProviderStore; resolveMcpServerStore: (c?: Context) => IMcpServerWithAuthStore; + resolveAgentStore: (c?: Context) => IAgentStore; withTransaction: WithTransaction; tokenStore: IOAuthTokenStore; skillStore: ISkillStore; sandboxProviderStore: ISandboxProviderStore; - agentStore: IAgentStore; scheduleStore: IScheduleStore; destroyDb: () => Promise; redis: RedisClientType | undefined; } +/** + * Shared ServiceFoundry HTTP client for TrueFoundry-mode store resolvers (models, MCP, agents). + * Undefined when TrueFoundry mode is off — resolvers then use persistence only. + */ +function createServiceFoundryServerClient(logger: Logger): TrueFoundryServiceFoundryServerClient | undefined { + if (!isTrueFoundryModeEnabled(configuration)) { + return undefined; + } + return new TrueFoundryServiceFoundryServerClient({ + serviceFoundryServerUrl: configuration.TRUEFOUNDRY_SERVICEFOUNDRY_SERVER_URL, + logger, + tls: { enabled: configuration.TRUEFOUNDRY_MTLS_ENABLED, dir: configuration.TRUEFOUNDRY_MTLS_CERTS_DIR }, + }); +} + /** * Per-request model-provider store resolver. In TrueFoundry mode every request gets a token-bound * store over a shared (mTLS) ServiceFoundry client; otherwise the persistence store is reused as-is. */ function buildResolveModelProviderStore(options: { persistenceStore: IModelProviderStore; - logger: Logger; + client: TrueFoundryServiceFoundryServerClient | undefined; }): (c?: Context) => IModelProviderStore { - if (!isTrueFoundryModeEnabled(configuration)) { - return () => options.persistenceStore; + const { persistenceStore, client } = options; + if (!client) { + return () => persistenceStore; } - const client = new TrueFoundryServiceFoundryServerClient({ - serviceFoundryServerUrl: configuration.TRUEFOUNDRY_SERVICEFOUNDRY_SERVER_URL, - logger: options.logger, - tls: { enabled: configuration.TRUEFOUNDRY_MTLS_ENABLED, dir: configuration.TRUEFOUNDRY_MTLS_CERTS_DIR }, - }); // No request context (e.g. the scheduler) means no caller token, so TrueFoundry models are // unavailable there; fall back to the persistence store. return c => c ? new TrueFoundryModelProviderStore({ client, accessToken: requireAccessToken(c) }) - : options.persistenceStore; + : persistenceStore; } /** @@ -126,27 +138,45 @@ function buildResolveModelProviderStore(options: { function buildResolveMcpServerStore(options: { persistenceStore: IMcpServerStore; tokenStore: IOAuthTokenStore; - logger: Logger; + client: TrueFoundryServiceFoundryServerClient | undefined; }): (c?: Context) => IMcpServerWithAuthStore { const withAuthPersistence = new McpServerWithAuthStore({ store: options.persistenceStore, tokenStore: options.tokenStore, clientName: configuration.MCP_DCR_OAUTH_CLIENT_NAME, }); - if (!isTrueFoundryModeEnabled(configuration)) { + const { client } = options; + if (!client) { return () => withAuthPersistence; } - const client = new TrueFoundryServiceFoundryServerClient({ - serviceFoundryServerUrl: configuration.TRUEFOUNDRY_SERVICEFOUNDRY_SERVER_URL, - logger: options.logger, - tls: { enabled: configuration.TRUEFOUNDRY_MTLS_ENABLED, dir: configuration.TRUEFOUNDRY_MTLS_CERTS_DIR }, - }); return c => c ? new TrueFoundryMcpServerStore({ client, accessToken: requireAccessToken(c) }) : withAuthPersistence; } +/** + * Per-request agent store resolver. In TrueFoundry mode every request gets a token-bound + * decorator over DB persistence; otherwise the persistence store is reused as-is. + */ +function buildResolveAgentStore(options: { + persistenceStore: IAgentStore; + client: TrueFoundryServiceFoundryServerClient | undefined; +}): (c?: Context) => IAgentStore { + const { persistenceStore, client } = options; + if (!client) { + return () => persistenceStore; + } + return c => + c + ? new TrueFoundryAgentStore({ + inner: persistenceStore, + client, + accessToken: requireAccessToken(c), + }) + : persistenceStore; +} + /** SQLite stores; Redis unused (executor peering disabled). */ async function createStandalonePersistence(options: { sqlitePath: string; @@ -187,23 +217,28 @@ async function createStandalonePersistence(options: { logger.info('Standalone mode: executor peering disabled and Redis unused'); const tokenStore = new SqliteOAuthTokenStore(db); + const agentStore = new SqliteAgentStore(db); + const serviceFoundryClient = createServiceFoundryServerClient(logger); return { sessionStore: new SqliteSessionStore(db), sessionMetricsStore: new SqliteSessionMetricsStore(db), resolveModelProviderStore: buildResolveModelProviderStore({ persistenceStore: new SqliteModelProviderStore(db), - logger, + client: serviceFoundryClient, }), resolveMcpServerStore: buildResolveMcpServerStore({ persistenceStore: new SqliteMcpServerStore(db), tokenStore, - logger, + client: serviceFoundryClient, + }), + resolveAgentStore: buildResolveAgentStore({ + persistenceStore: agentStore, + client: serviceFoundryClient, }), withTransaction: callback => db.transaction().execute(callback), tokenStore, skillStore: new SqliteSkillStore(db), sandboxProviderStore: new SqliteSandboxProviderStore(db), - agentStore: new SqliteAgentStore(db), scheduleStore: new SqliteScheduleStore(db), destroyDb: () => db.destroy(), redis: undefined, @@ -264,23 +299,28 @@ async function createDistributedPersistence(options: { logger.info(`Executor id: ${executorId}`); const tokenStore = new PostgresOAuthTokenStore(db); + const agentStore = new PostgresAgentStore(db); + const serviceFoundryClient = createServiceFoundryServerClient(logger); return { sessionStore: new PostgresSessionStore(db), sessionMetricsStore: new PostgresSessionMetricsStore(db), resolveModelProviderStore: buildResolveModelProviderStore({ persistenceStore: new PostgresModelProviderStore(db), - logger, + client: serviceFoundryClient, }), resolveMcpServerStore: buildResolveMcpServerStore({ persistenceStore: new PostgresMcpServerStore(db), tokenStore, - logger, + client: serviceFoundryClient, + }), + resolveAgentStore: buildResolveAgentStore({ + persistenceStore: agentStore, + client: serviceFoundryClient, }), withTransaction: callback => db.transaction().execute(callback), tokenStore, skillStore: new PostgresSkillStore(db), sandboxProviderStore: new PostgresSandboxProviderStore(db), - agentStore: new PostgresAgentStore(db), scheduleStore: new PostgresScheduleStore(db), destroyDb: () => db.destroy(), redis: await connectRedis({ url: redisUrl, logger }), @@ -294,11 +334,11 @@ async function createServerRuntime(persistence: ServerPersistence< sessionMetricsStore, resolveModelProviderStore, resolveMcpServerStore, + resolveAgentStore, withTransaction, tokenStore, skillStore, sandboxProviderStore, - agentStore, scheduleStore, destroyDb, redis, @@ -333,11 +373,11 @@ async function createServerRuntime(persistence: ServerPersistence< sandboxCatalog: SandboxCatalog.load(), resolveModelProviderStore, resolveMcpServerStore, + resolveAgentStore, withTransaction, tokenStore, skillStore, sandboxProviderStore, - agentStore, scheduleStore, sessionStore, sessionMetricsStore, diff --git a/packages/trueforge/src/truefoundry/AGENTS.md b/packages/trueforge/src/truefoundry/AGENTS.md new file mode 100644 index 000000000..398cfdbe5 --- /dev/null +++ b/packages/trueforge/src/truefoundry/AGENTS.md @@ -0,0 +1 @@ +- ServiceFoundry shape mappers under this directory MUST use `map*` for SF → domain (read/list parse) and `to*` for domain → SF/wire (request body build). Examples: `mapSfyMcpServers`, `mapEnabledModels`, `toPutRemoteAgentPayload`, `toTrueFoundryMcpManifest`. diff --git a/packages/trueforge/src/truefoundry/CLAUDE.md b/packages/trueforge/src/truefoundry/CLAUDE.md new file mode 100644 index 000000000..43c994c2d --- /dev/null +++ b/packages/trueforge/src/truefoundry/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts new file mode 100644 index 000000000..57744ca32 --- /dev/null +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -0,0 +1,128 @@ +import { + AgentNameConflictError, + type AgentRecord, + type CreateAgentInput, + type DeleteAgentInput, + type GetAgentInput, + type IAgentStore, + type UpdateAgentInput, +} from '../db/agentStore'; +import { toPutRemoteAgentPayload } from './toPutRemoteAgentPayload'; +import { TrueFoundryServiceFoundryServerClient } from './TrueFoundryServiceFoundryServerClient'; + +function asError(value: unknown): Error { + return value instanceof Error ? value : new Error(String(value)); +} + +/** + * create: getByName → putRemote → createDB(external_id) | on non-conflict DB fail → deleteRemote + * update: putRemote(new) → updateDB | on DB fail → putRemote(old) | both fail → AggregateError + * delete: deleteRemote(404 ok) → deleteDB + */ +export class TrueFoundryAgentStore implements IAgentStore { + readonly #inner: IAgentStore; + readonly #client: TrueFoundryServiceFoundryServerClient; + readonly #accessToken: string; + + constructor(input: { + inner: IAgentStore; + client: TrueFoundryServiceFoundryServerClient; + accessToken: string; + }) { + this.#inner = input.inner; + this.#client = input.client; + this.#accessToken = input.accessToken; + } + + listAgents(tenantId: string, transaction?: TTransaction): Promise { + return this.#inner.listAgents(tenantId, transaction); + } + + getAgent(input: GetAgentInput, transaction?: TTransaction): Promise { + return this.#inner.getAgent(input, transaction); + } + + async createAgent(input: CreateAgentInput, transaction?: TTransaction): Promise { + // SF PUT upserts by name — skip if local name exists (avoids overwrite/delete of e.g. research→sf-1). + const existing = await this.#inner.getAgent({ tenant_id: input.tenant_id, name: input.name }, transaction); + if (existing !== undefined) { + throw new AgentNameConflictError({ tenant_id: input.tenant_id, name: input.name }); + } + + const { remoteAgentId } = await this.#client.putRemoteAgent({ + accessToken: this.#accessToken, + ...toPutRemoteAgentPayload({ name: input.name, manifest: input.manifest }), + }); + try { + return await this.#inner.createAgent({ ...input, external_id: remoteAgentId }, transaction); + } catch (error) { + // Race: peer create won the name and owns this remote (1:1) — do not delete it. + if (!(error instanceof AgentNameConflictError)) { + try { + await this.#client.deleteRemoteAgent({ accessToken: this.#accessToken, remoteAgentId }); + } catch (cleanupError) { + throw new AggregateError( + [asError(error), asError(cleanupError)], + 'createAgent failed and ServiceFoundry cleanup also failed', + { cause: cleanupError }, + ); + } + } + throw error; + } + } + + async updateAgent(input: UpdateAgentInput, transaction?: TTransaction): Promise { + if (input.manifest === undefined) { + return this.#inner.updateAgent(input, transaction); + } + + const previous = await this.#inner.getAgent({ tenant_id: input.tenant_id, id: input.id }, transaction); + if (previous === undefined) { + return undefined; + } + + const { remoteAgentId } = await this.#client.putRemoteAgent({ + accessToken: this.#accessToken, + ...toPutRemoteAgentPayload({ name: previous.name, manifest: input.manifest }), + }); + + try { + return await this.#inner.updateAgent( + { + tenant_id: input.tenant_id, + id: input.id, + manifest: input.manifest, + ...(input.metadata === undefined ? {} : { metadata: input.metadata }), + ...(remoteAgentId === previous.external_id ? {} : { external_id: remoteAgentId }), + }, + transaction, + ); + } catch (error) { + try { + await this.#client.putRemoteAgent({ + accessToken: this.#accessToken, + ...toPutRemoteAgentPayload({ name: previous.name, manifest: previous.manifest }), + }); + } catch (restoreError) { + throw new AggregateError( + [asError(error), asError(restoreError)], + 'updateAgent failed and ServiceFoundry restore also failed', + { cause: restoreError }, + ); + } + throw error; + } + } + + async deleteAgent(input: DeleteAgentInput, transaction?: TTransaction): Promise { + const previous = await this.#inner.getAgent({ tenant_id: input.tenant_id, id: input.id }, transaction); + if (previous?.external_id) { + await this.#client.deleteRemoteAgent({ + accessToken: this.#accessToken, + remoteAgentId: previous.external_id, + }); + } + await this.#inner.deleteAgent(input, transaction); + } +} diff --git a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts index 1f40ca014..3297e7709 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts @@ -9,6 +9,7 @@ import { createInternalTlsDispatcher, normalizeInternalTlsUrl, type InternalTlsO const INTEGRATIONS_PATH = 'v1/provider-integrations'; const INSTALLATIONS_PATH = 'v1/llm-gateway/installations'; const MCP_SERVERS_PATH = 'v1/mcp'; +const TFG_AGENTS_PATH = 'internal/tfg/agents'; const INTEGRATIONS_PAGE_SIZE = 1000; const MCP_SERVERS_PAGE_SIZE = 100; @@ -26,6 +27,28 @@ const ServiceFoundryErrorSchema = z.object({ message: z.union([z.string(), z.array(z.string())]).optional(), }); +/** Wire shape from PUT `/internal/tfg/agents` — keep `agentId` only here. */ +const PutRemoteAgentResponseSchema = z.object({ + agentId: z.string().min(1), +}); + +export interface PutRemoteAgentInput { + accessToken: string; + name: string; + description: string; + model: string; + mcp_servers?: string[]; +} + +export interface PutRemoteAgentResult { + remoteAgentId: string; +} + +export interface DeleteRemoteAgentInput { + accessToken: string; + remoteAgentId: string; +} + async function readServiceFoundryErrorMessage( response: Awaited>, ): Promise { @@ -137,6 +160,42 @@ export class TrueFoundryServiceFoundryServerClient { return rows[0]; } + /** PUT `/internal/tfg/agents` — create/reuse remote agent + sync model/MCP grants. */ + async putRemoteAgent(input: PutRemoteAgentInput): Promise { + const payload = await this.#requestJson({ + url: this.#url(TFG_AGENTS_PATH), + accessToken: input.accessToken, + method: 'PUT', + body: { + name: input.name, + description: input.description, + model: input.model, + ...(input.mcp_servers === undefined ? {} : { mcp_servers: input.mcp_servers }), + }, + }); + const parsed = PutRemoteAgentResponseSchema.safeParse(payload); + if (!parsed.success) { + this.#logger?.error('TrueFoundry ServiceFoundry put remote agent returned an unexpected response', { + ...extractErrorLogFields(parsed.error), + }); + throw new HTTPException(424, { + message: 'TrueFoundry ServiceFoundry put remote agent returned an unexpected response', + cause: parsed.error, + }); + } + return { remoteAgentId: parsed.data.agentId }; + } + + /** DELETE `/internal/tfg/agents/:id` — remove remote agent. Missing agent (404) is success. */ + async deleteRemoteAgent(input: DeleteRemoteAgentInput): Promise { + await this.#requestJson({ + url: this.#url(`${TFG_AGENTS_PATH}/${encodeURIComponent(input.remoteAgentId)}`), + accessToken: input.accessToken, + method: 'DELETE', + notFoundOk: true, + }); + } + #parseListResponse(payload: unknown): ListResponse { const parsed = ListResponseSchema.safeParse(payload); if (!parsed.success) { @@ -161,21 +220,35 @@ export class TrueFoundryServiceFoundryServerClient { return url; } - async #getJson(url: URL, accessToken: string): Promise { + #getJson(url: URL, accessToken: string): Promise { + return this.#requestJson({ url, accessToken, method: 'GET' }); + } + + async #requestJson(input: { + url: URL; + accessToken: string; + method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE'; + body?: unknown; + /** Treat HTTP 404 as success (idempotent DELETE). */ + notFoundOk?: boolean; + }): Promise { const startedAt = Date.now(); let response: Awaited>; try { - response = await undiciFetch(url, { - method: 'GET', + response = await undiciFetch(input.url, { + method: input.method, headers: { accept: 'application/json', - authorization: `Bearer ${accessToken}`, + authorization: `Bearer ${input.accessToken}`, + ...(input.body === undefined ? {} : { 'content-type': 'application/json' }), }, + ...(input.body === undefined ? {} : { body: JSON.stringify(input.body) }), ...(this.#dispatcher ? { dispatcher: this.#dispatcher } : {}), }); } catch (error) { this.#logger?.warn('TrueFoundry ServiceFoundry server request failed', { - url: url.href, + url: input.url.href, + method: input.method, durationMs: Date.now() - startedAt, ...extractErrorLogFields(error), }); @@ -185,7 +258,8 @@ export class TrueFoundryServiceFoundryServerClient { }); } this.#logger?.info('TrueFoundry ServiceFoundry server request completed', { - url: url.href, + url: input.url.href, + method: input.method, status: response.status, durationMs: Date.now() - startedAt, }); @@ -194,12 +268,29 @@ export class TrueFoundryServiceFoundryServerClient { message: 'TrueFoundry ServiceFoundry server rejected the request', }); } + if (response.status === 404 && input.notFoundOk) { + return undefined; + } if (!response.ok) { const detail = await readServiceFoundryErrorMessage(response); throw new HTTPException(424, { message: `TrueFoundry ServiceFoundry server request failed: ${detail ?? `HTTP ${String(response.status)}`}`, }); } - return response.json(); + if (response.status === 204) { + return undefined; + } + const text = await response.text(); + if (text.length === 0) { + return undefined; + } + try { + return JSON.parse(text); + } catch (error) { + throw new HTTPException(424, { + message: 'TrueFoundry ServiceFoundry server returned invalid JSON', + cause: error, + }); + } } } diff --git a/packages/trueforge/src/truefoundry/toPutRemoteAgentPayload.ts b/packages/trueforge/src/truefoundry/toPutRemoteAgentPayload.ts new file mode 100644 index 000000000..f27d7380f --- /dev/null +++ b/packages/trueforge/src/truefoundry/toPutRemoteAgentPayload.ts @@ -0,0 +1,21 @@ +/** + * AgentSpec → PUT `/internal/tfg/agents` body fields (domain → SF; inverse of mapSfyMcpServers / mapEnabledModels). + */ +import type { AgentSpec } from '@truefoundry/trueforge-core/agent-session'; + +import type { PutRemoteAgentInput } from './TrueFoundryServiceFoundryServerClient'; + +export function toPutRemoteAgentPayload({ + name, + manifest, +}: { + name: string; + manifest: AgentSpec; +}): Omit { + return { + name, + description: manifest.instructions ?? name, + model: manifest.model.name, + ...(manifest.mcp_servers === undefined ? {} : { mcp_servers: manifest.mcp_servers.map(server => server.name) }), + }; +} diff --git a/packages/trueforge/tests/unit/apis/agents.test.ts b/packages/trueforge/tests/unit/apis/agents.test.ts index 0305c84d7..3412e8eac 100644 --- a/packages/trueforge/tests/unit/apis/agents.test.ts +++ b/packages/trueforge/tests/unit/apis/agents.test.ts @@ -78,7 +78,7 @@ describe('agents router', () => { await modelProviderStore.upsertProvider({ tenant_id: 'default', name: 'anthropic', manifest: modelProvider }); agentStore = new SqliteAgentStore(db); router = createAgentsRouter({ - agentStore, + resolveAgentStore: () => agentStore, resolveModelProviderStore: () => modelProviderStore, resolveMcpServerStore: () => new SqliteMcpServerStore(db), skillStore: new SqliteSkillStore(db), diff --git a/packages/trueforge/tests/unit/apis/deletedSessionCrud.test.ts b/packages/trueforge/tests/unit/apis/deletedSessionCrud.test.ts index d8f7a5d65..4026eb3e4 100644 --- a/packages/trueforge/tests/unit/apis/deletedSessionCrud.test.ts +++ b/packages/trueforge/tests/unit/apis/deletedSessionCrud.test.ts @@ -43,7 +43,7 @@ describe('public CRUD after session deletion', () => { resolveModelProviderStore: () => modelProviderStore, resolveMcpServerStore: () => mcpServerStore, skillStore, - agentStore, + resolveAgentStore: () => agentStore, sandboxProviderStore, redis: createClient(), requestReplyRouter: new RequestReplyRouter(), @@ -61,7 +61,7 @@ describe('public CRUD after session deletion', () => { resolveMcpServerStore: () => mcpServerStore, tokenStore, skillStore, - agentStore, + resolveAgentStore: () => agentStore, eventSubscriptions: new EventSubscriptionRegistry(undefined), sandboxProviderStore, logger: createLogger({ silent: true }), diff --git a/packages/trueforge/tests/unit/apis/sandboxFileDownload.test.ts b/packages/trueforge/tests/unit/apis/sandboxFileDownload.test.ts index a70d6107a..d959be3c5 100644 --- a/packages/trueforge/tests/unit/apis/sandboxFileDownload.test.ts +++ b/packages/trueforge/tests/unit/apis/sandboxFileDownload.test.ts @@ -42,7 +42,7 @@ async function buildApp() { resolveMcpServerStore: () => new SqliteMcpServerStore(db), tokenStore: new SqliteOAuthTokenStore(db), skillStore: new SqliteSkillStore(db), - agentStore: new SqliteAgentStore(db), + resolveAgentStore: () => new SqliteAgentStore(db), eventSubscriptions: new EventSubscriptionRegistry(undefined), sandboxProviderStore: new SqliteSandboxProviderStore(db), logger: createLogger({ silent: true }), diff --git a/packages/trueforge/tests/unit/apis/schedules.test.ts b/packages/trueforge/tests/unit/apis/schedules.test.ts index b98b088ed..da412ad62 100644 --- a/packages/trueforge/tests/unit/apis/schedules.test.ts +++ b/packages/trueforge/tests/unit/apis/schedules.test.ts @@ -51,7 +51,7 @@ async function setup() { '/', createSchedulesRouter({ scheduleStore, - agentStore, + resolveAgentStore: () => agentStore, sessions: { getOrCreateByExternalId: () => Promise.reject(new Error('sessions stub: unexpected call')), } as never, diff --git a/packages/trueforge/tests/unit/apis/sessionHttp.test.ts b/packages/trueforge/tests/unit/apis/sessionHttp.test.ts index 6bebb6c2b..f3a4b8cd0 100644 --- a/packages/trueforge/tests/unit/apis/sessionHttp.test.ts +++ b/packages/trueforge/tests/unit/apis/sessionHttp.test.ts @@ -82,7 +82,7 @@ describe('sessions HTTP agent binding', () => { resolveModelProviderStore: () => modelProviderStore, resolveMcpServerStore: () => mcpServerStore, skillStore, - agentStore, + resolveAgentStore: () => agentStore, sandboxProviderStore, redis: createClient(), requestReplyRouter: new RequestReplyRouter(), diff --git a/packages/trueforge/tests/unit/apis/turnHarnessErrorStatus.test.ts b/packages/trueforge/tests/unit/apis/turnHarnessErrorStatus.test.ts index 2a7b6f76a..460e0be36 100644 --- a/packages/trueforge/tests/unit/apis/turnHarnessErrorStatus.test.ts +++ b/packages/trueforge/tests/unit/apis/turnHarnessErrorStatus.test.ts @@ -59,7 +59,7 @@ async function postTurnRejectingWith(error: AgentHarnessError): Promise new SqliteMcpServerStore(db), tokenStore: new SqliteOAuthTokenStore(db), skillStore: new SqliteSkillStore(db), - agentStore: new SqliteAgentStore(db), + resolveAgentStore: () => new SqliteAgentStore(db), eventSubscriptions: new EventSubscriptionRegistry(undefined), sandboxProviderStore: new SqliteSandboxProviderStore(db), logger: createLogger({ silent: true }), diff --git a/packages/trueforge/tests/unit/apis/turns.test.ts b/packages/trueforge/tests/unit/apis/turns.test.ts index 24cfe0f4e..eeac198ed 100644 --- a/packages/trueforge/tests/unit/apis/turns.test.ts +++ b/packages/trueforge/tests/unit/apis/turns.test.ts @@ -56,7 +56,7 @@ describe('turns', () => { resolveMcpServerStore: () => new SqliteMcpServerStore(db), tokenStore: new SqliteOAuthTokenStore(db), skillStore: new SqliteSkillStore(db), - agentStore: new SqliteAgentStore(db), + resolveAgentStore: () => new SqliteAgentStore(db), eventSubscriptions: new EventSubscriptionRegistry(undefined), sandboxProviderStore: new SqliteSandboxProviderStore(db), logger: createLogger({ silent: true }), @@ -172,7 +172,7 @@ describe('turns', () => { sessionStore: new SqliteSessionStore(db), activeTurns: new ActiveTurnRegistry(), resolveModelProviderStore: () => modelProviderStore, - agentStore: new SqliteAgentStore(db), + resolveAgentStore: () => new SqliteAgentStore(db), resolveMcpServerStore: () => new SqliteMcpServerStore(db), tokenStore: new SqliteOAuthTokenStore(db), skillStore: new SqliteSkillStore(db), @@ -275,7 +275,7 @@ describe('turns', () => { resolveMcpServerStore: () => new SqliteMcpServerStore(db), tokenStore: new SqliteOAuthTokenStore(db), skillStore: new SqliteSkillStore(db), - agentStore: new SqliteAgentStore(db), + resolveAgentStore: () => new SqliteAgentStore(db), eventSubscriptions: new EventSubscriptionRegistry(undefined), sandboxProviderStore: new SqliteSandboxProviderStore(db), logger, diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts new file mode 100644 index 000000000..8e6519cc3 --- /dev/null +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -0,0 +1,479 @@ +import { AgentSpecSchema } from '@truefoundry/trueforge-core/agent-session'; + +import type { AgentRecord, IAgentStore } from '../../../src/db/agentStore'; +import { AgentNameConflictError } from '../../../src/db/agentStore'; +import { TrueFoundryAgentStore } from '../../../src/truefoundry/TrueFoundryAgentStore'; +import { + TrueFoundryServiceFoundryServerClient, + type DeleteRemoteAgentInput, + type PutRemoteAgentInput, + type PutRemoteAgentResult, +} from '../../../src/truefoundry/TrueFoundryServiceFoundryServerClient'; + +const TENANT = 'default'; +const TOKEN = 'test-token'; + +function manifest(overrides: { instructions?: string; mcp_servers?: { name: string }[] } = {}) { + return AgentSpecSchema.parse({ + model: { name: 'openai-gateway/gpt-5' }, + instructions: overrides.instructions ?? 'Be helpful.', + ...(overrides.mcp_servers === undefined ? {} : { mcp_servers: overrides.mcp_servers }), + }); +} + +function record(overrides: Partial = {}): AgentRecord { + const now = '2026-09-02T00:00:00.000Z'; + return { + id: 'agent-1', + tenant_id: TENANT, + name: 'research', + manifest: manifest(), + metadata: {}, + external_id: null, + created_at: now, + updated_at: now, + ...overrides, + }; +} + +function mockInner(overrides: Partial = {}): IAgentStore { + return { + listAgents: jest.fn(), + getAgent: jest.fn(), + createAgent: jest.fn(), + updateAgent: jest.fn(), + deleteAgent: jest.fn(), + ...overrides, + }; +} + +function mockClient( + overrides: { + putRemoteAgent?: TrueFoundryServiceFoundryServerClient['putRemoteAgent']; + deleteRemoteAgent?: TrueFoundryServiceFoundryServerClient['deleteRemoteAgent']; + } = {}, +): TrueFoundryServiceFoundryServerClient { + const client = new TrueFoundryServiceFoundryServerClient({ + serviceFoundryServerUrl: 'http://servicefoundry.test', + }); + client.putRemoteAgent = + overrides.putRemoteAgent ?? (async (): Promise => ({ remoteAgentId: 'sf-1' })); + client.deleteRemoteAgent = overrides.deleteRemoteAgent ?? (async (_input: DeleteRemoteAgentInput) => undefined); + return client; +} + +function firstInvocationOrder(mock: jest.Mock): number { + const order = mock.mock.invocationCallOrder[0]; + if (order === undefined) { + throw new Error('expected mock to have been called'); + } + return order; +} + +describe('TrueFoundryAgentStore', () => { + it('listAgents and getAgent pass through to the inner store', async () => { + const agents = [record()]; + const listAgents = jest.fn(async () => agents); + const getAgent = jest.fn(async () => agents[0]); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ listAgents, getAgent }), + client: mockClient(), + accessToken: TOKEN, + }); + + await expect(store.listAgents(TENANT)).resolves.toBe(agents); + await expect(store.getAgent({ tenant_id: TENANT, id: 'agent-1' })).resolves.toBe(agents[0]); + expect(listAgents).toHaveBeenCalledWith(TENANT, undefined); + expect(getAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'agent-1' }, undefined); + }); + + it('createAgent puts then inserts with external_id', async () => { + const created = record({ external_id: 'sf-1' }); + const putRemoteAgent = jest.fn(async (input: PutRemoteAgentInput) => { + expect(input).toEqual({ + accessToken: TOKEN, + name: 'research', + description: 'Be helpful.', + model: 'openai-gateway/gpt-5', + mcp_servers: ['slack'], + }); + return { remoteAgentId: 'sf-1' }; + }); + const getAgent = jest.fn(async () => undefined); + const createAgent = jest.fn(async () => created); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, createAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.createAgent({ + tenant_id: TENANT, + name: 'research', + manifest: manifest({ mcp_servers: [{ name: 'slack' }] }), + external_id: null, + }), + ).resolves.toBe(created); + expect(getAgent).toHaveBeenCalledWith({ tenant_id: TENANT, name: 'research' }, undefined); + expect(createAgent).toHaveBeenCalledWith( + expect.objectContaining({ + tenant_id: TENANT, + name: 'research', + external_id: 'sf-1', + }), + undefined, + ); + }); + + it('createAgent rejects a duplicate local name before calling ServiceFoundry', async () => { + const getAgent = jest.fn(async () => record({ external_id: 'sf-existing' })); + const createAgent = jest.fn(); + const putRemoteAgent = jest.fn(); + const deleteRemoteAgent = jest.fn(); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, createAgent }), + client: mockClient({ putRemoteAgent, deleteRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), + ).rejects.toMatchObject({ name: 'AgentNameConflictError' }); + expect(putRemoteAgent).not.toHaveBeenCalled(); + expect(createAgent).not.toHaveBeenCalled(); + expect(deleteRemoteAgent).not.toHaveBeenCalled(); + }); + + it('createAgent uses agent name as description when instructions are omitted', async () => { + const putRemoteAgent = jest.fn(async (input: PutRemoteAgentInput) => { + expect(input.description).toBe('research'); + return { remoteAgentId: 'sf-1' }; + }); + const createAgent = jest.fn(async () => record({ external_id: 'sf-1' })); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ createAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await store.createAgent({ + tenant_id: TENANT, + name: 'research', + manifest: AgentSpecSchema.parse({ model: { name: 'openai-gateway/gpt-5' } }), + external_id: null, + }); + expect(putRemoteAgent).toHaveBeenCalled(); + }); + + it('createAgent rolls back SF when DB insert fails', async () => { + const deleteRemoteAgent = jest.fn(async () => undefined); + const createAgent = jest.fn(async () => { + throw new Error('db failed'); + }); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ createAgent }), + client: mockClient({ deleteRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), + ).rejects.toThrow('db failed'); + expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, remoteAgentId: 'sf-1' }); + }); + + it('createAgent does not delete remote on local name conflict after put', async () => { + const deleteRemoteAgent = jest.fn(); + const createAgent = jest.fn(async () => { + throw new AgentNameConflictError({ tenant_id: TENANT, name: 'research' }); + }); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ createAgent }), + client: mockClient({ deleteRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), + ).rejects.toBeInstanceOf(AgentNameConflictError); + expect(deleteRemoteAgent).not.toHaveBeenCalled(); + }); + + it('createAgent still throws when SF rollback fails', async () => { + const deleteRemoteAgent = jest.fn(async () => { + throw new Error('cleanup failed'); + }); + const createAgent = jest.fn(async () => { + throw new Error('db failed'); + }); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ createAgent }), + client: mockClient({ deleteRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), + ).rejects.toMatchObject({ + message: 'createAgent failed and ServiceFoundry cleanup also failed', + errors: [ + expect.objectContaining({ message: 'db failed' }), + expect.objectContaining({ message: 'cleanup failed' }), + ], + }); + expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, remoteAgentId: 'sf-1' }); + }); + + it('createAgent does not insert when putRemoteAgent fails', async () => { + const createAgent = jest.fn(); + const putRemoteAgent = jest.fn(async () => { + throw new Error('sf failed'); + }); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ createAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), + ).rejects.toThrow('sf failed'); + expect(createAgent).not.toHaveBeenCalled(); + }); + + it('updateAgent without manifest passes through to the inner store', async () => { + const updated = record({ metadata: {} }); + const updateAgent = jest.fn(async () => updated); + const putRemoteAgent = jest.fn(); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ updateAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await expect(store.updateAgent({ tenant_id: TENANT, id: 'agent-1', metadata: {} })).resolves.toBe(updated); + expect(putRemoteAgent).not.toHaveBeenCalled(); + expect(updateAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'agent-1', metadata: {} }, undefined); + }); + + it('updateAgent returns undefined for a missing agent without calling putRemoteAgent', async () => { + const getAgent = jest.fn(async () => undefined); + const updateAgent = jest.fn(); + const putRemoteAgent = jest.fn(); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, updateAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.updateAgent({ tenant_id: TENANT, id: 'missing', manifest: manifest({ instructions: 'Updated.' }) }), + ).resolves.toBeUndefined(); + expect(updateAgent).not.toHaveBeenCalled(); + expect(putRemoteAgent).not.toHaveBeenCalled(); + }); + + it('updateAgent puts remote agent then writes manifest when putRemoteAgent returns the same id', async () => { + const previous = record({ external_id: 'sf-1' }); + const updatedManifest = manifest({ instructions: 'Updated.' }); + const updated = record({ manifest: updatedManifest, external_id: 'sf-1' }); + const getAgent = jest.fn(async () => previous); + const updateAgent = jest.fn(async () => updated); + const putRemoteAgent = jest.fn(async () => ({ remoteAgentId: 'sf-1' })); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, updateAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await expect(store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest })).resolves.toBe( + updated, + ); + expect(putRemoteAgent).toHaveBeenCalledTimes(1); + expect(updateAgent).toHaveBeenCalledTimes(1); + expect(updateAgent).toHaveBeenCalledWith( + { + tenant_id: TENANT, + id: previous.id, + manifest: updatedManifest, + }, + undefined, + ); + expect(firstInvocationOrder(putRemoteAgent)).toBeLessThan(firstInvocationOrder(updateAgent)); + }); + + it('updateAgent puts remote agent then writes manifest and external_id when it changes', async () => { + const previous = record({ external_id: 'sf-old' }); + const updatedManifest = manifest({ instructions: 'Updated.' }); + const updated = record({ manifest: updatedManifest, external_id: 'sf-new' }); + const getAgent = jest.fn(async () => previous); + const updateAgent = jest.fn(async () => updated); + const putRemoteAgent = jest.fn(async () => ({ remoteAgentId: 'sf-new' })); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, updateAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + const result = await store.updateAgent({ + tenant_id: TENANT, + id: previous.id, + manifest: updatedManifest, + }); + expect(result?.external_id).toBe('sf-new'); + expect(updateAgent).toHaveBeenCalledTimes(1); + expect(updateAgent).toHaveBeenCalledWith( + { + tenant_id: TENANT, + id: previous.id, + manifest: updatedManifest, + external_id: 'sf-new', + }, + undefined, + ); + }); + + it('updateAgent keeps the DB row when putRemoteAgent fails', async () => { + const previous = record({ external_id: 'sf-old' }); + const getAgent = jest.fn(async () => previous); + const updateAgent = jest.fn(); + const putRemoteAgent = jest.fn(async () => { + throw new Error('sf failed'); + }); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, updateAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: manifest({ instructions: 'Updated.' }) }), + ).rejects.toThrow('sf failed'); + expect(updateAgent).not.toHaveBeenCalled(); + }); + + it('updateAgent restores ServiceFoundry when the DB write fails', async () => { + const previous = record({ external_id: 'sf-old' }); + const updatedManifest = manifest({ instructions: 'Updated.' }); + const getAgent = jest.fn(async () => previous); + const updateAgent = jest.fn(async () => { + throw new Error('db write failed'); + }); + const putRemoteAgent = jest + .fn() + .mockResolvedValueOnce({ remoteAgentId: 'sf-new' }) + .mockResolvedValueOnce({ remoteAgentId: 'sf-old' }); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, updateAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await expect(store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest })).rejects.toThrow( + 'db write failed', + ); + expect(putRemoteAgent).toHaveBeenCalledTimes(2); + expect(putRemoteAgent).toHaveBeenLastCalledWith( + expect.objectContaining({ + accessToken: TOKEN, + name: previous.name, + description: previous.manifest.instructions ?? previous.name, + model: previous.manifest.model.name, + }), + ); + }); + + it('updateAgent still throws when ServiceFoundry restore fails', async () => { + const previous = record({ external_id: 'sf-old' }); + const getAgent = jest.fn(async () => previous); + const updateAgent = jest.fn(async () => { + throw new Error('db write failed'); + }); + const putRemoteAgent = jest + .fn() + .mockResolvedValueOnce({ remoteAgentId: 'sf-new' }) + .mockRejectedValueOnce(new Error('sf restore failed')); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, updateAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: manifest({ instructions: 'Updated.' }) }), + ).rejects.toMatchObject({ + message: 'updateAgent failed and ServiceFoundry restore also failed', + errors: [ + expect.objectContaining({ message: 'db write failed' }), + expect.objectContaining({ message: 'sf restore failed' }), + ], + }); + }); + + it('deleteAgent deletes ServiceFoundry then DB when external_id is set', async () => { + const previous = record({ external_id: 'sf-1' }); + const getAgent = jest.fn(async () => previous); + const deleteAgent = jest.fn(async () => undefined); + const deleteRemoteAgent = jest.fn(async () => undefined); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, deleteAgent }), + client: mockClient({ deleteRemoteAgent }), + accessToken: TOKEN, + }); + + await store.deleteAgent({ tenant_id: TENANT, id: previous.id }); + expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, remoteAgentId: 'sf-1' }); + expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: previous.id }, undefined); + expect(firstInvocationOrder(deleteRemoteAgent)).toBeLessThan(firstInvocationOrder(deleteAgent)); + }); + + it('deleteAgent skips ServiceFoundry when external_id is null', async () => { + const previous = record({ external_id: null }); + const getAgent = jest.fn(async () => previous); + const deleteAgent = jest.fn(async () => undefined); + const deleteRemoteAgent = jest.fn(); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, deleteAgent }), + client: mockClient({ deleteRemoteAgent }), + accessToken: TOKEN, + }); + + await store.deleteAgent({ tenant_id: TENANT, id: previous.id }); + expect(deleteAgent).toHaveBeenCalled(); + expect(deleteRemoteAgent).not.toHaveBeenCalled(); + }); + + it('deleteAgent skips ServiceFoundry when the agent is already missing', async () => { + const getAgent = jest.fn(async () => undefined); + const deleteAgent = jest.fn(async () => undefined); + const deleteRemoteAgent = jest.fn(); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, deleteAgent }), + client: mockClient({ deleteRemoteAgent }), + accessToken: TOKEN, + }); + + await store.deleteAgent({ tenant_id: TENANT, id: 'missing' }); + expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'missing' }, undefined); + expect(deleteRemoteAgent).not.toHaveBeenCalled(); + }); + + it('deleteAgent keeps the DB row when ServiceFoundry delete fails', async () => { + const previous = record({ external_id: 'sf-1' }); + const getAgent = jest.fn(async () => previous); + const deleteAgent = jest.fn(async () => undefined); + const deleteRemoteAgent = jest.fn(async () => { + throw new Error('sf delete failed'); + }); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, deleteAgent }), + client: mockClient({ deleteRemoteAgent }), + accessToken: TOKEN, + }); + + await expect(store.deleteAgent({ tenant_id: TENANT, id: previous.id })).rejects.toThrow('sf delete failed'); + expect(deleteRemoteAgent).toHaveBeenCalled(); + expect(deleteAgent).not.toHaveBeenCalled(); + }); +}); From c9270c5c00602180949a604e7f0f48fedee799e4 Mon Sep 17 00:00:00 2001 From: Bhavesh Patel Date: Thu, 3 Sep 2026 13:52:31 +0530 Subject: [PATCH 2/8] Reserve tfg/trueforge agent names and filter listAgents by external_ids --- .../20260902194500-truefoundry-agent-store.md | 2 +- .../20260903013000-reserve-agent-names.md | 5 ++ packages/trueforge/src/apis/agents.ts | 29 +++++++---- packages/trueforge/src/db/agentStore.ts | 29 ++++++++++- .../agent-store/PostgresAgentStore.ts | 14 +++++- .../db/sqlite/agent-store/SqliteAgentStore.ts | 19 ++++--- packages/trueforge/src/routes/agentRoutes.ts | 4 ++ .../src/truefoundry/TrueFoundryAgentStore.ts | 9 +++- .../tests/db/agentStoreContractSuite.ts | 50 ++++++++++++++++++- .../trueforge/tests/unit/apis/agents.test.ts | 47 +++++++++++++++-- .../truefoundry/TrueFoundryAgentStore.test.ts | 27 ++++++++-- 11 files changed, 202 insertions(+), 33 deletions(-) create mode 100644 .changeset/20260903013000-reserve-agent-names.md diff --git a/.changeset/20260902194500-truefoundry-agent-store.md b/.changeset/20260902194500-truefoundry-agent-store.md index 6c75fdb0d..159ca1e02 100644 --- a/.changeset/20260902194500-truefoundry-agent-store.md +++ b/.changeset/20260902194500-truefoundry-agent-store.md @@ -2,4 +2,4 @@ "@truefoundry/trueforge": patch --- -Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. +Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. Filter `listAgents` by `external_ids`. diff --git a/.changeset/20260903013000-reserve-agent-names.md b/.changeset/20260903013000-reserve-agent-names.md new file mode 100644 index 000000000..94d71770e --- /dev/null +++ b/.changeset/20260903013000-reserve-agent-names.md @@ -0,0 +1,5 @@ +--- +"@truefoundry/trueforge": patch +--- + +Reserve agent names `tfg` and `trueforge` on create in all agent store modes. diff --git a/packages/trueforge/src/apis/agents.ts b/packages/trueforge/src/apis/agents.ts index daee953ba..104f4d6ab 100644 --- a/packages/trueforge/src/apis/agents.ts +++ b/packages/trueforge/src/apis/agents.ts @@ -7,6 +7,7 @@ import type { Context } from 'hono'; import { AgentExternalIdConflictError, AgentNameConflictError, + AgentNameReservedError, type AgentRecord, type IAgentStore, } from '../db/agentStore'; @@ -70,7 +71,7 @@ async function validateManifest({ export function createAgentsRouter(deps: AgentsRouterDeps) { const listHandler: RouteHandler = async c => { - const records = await deps.resolveAgentStore(c).listAgents(TENANT_ID); + const records = await deps.resolveAgentStore(c).listAgents({ tenant_id: TENANT_ID }); return c.json({ data: records.map(toWireAgent) }, 200); }; @@ -91,6 +92,9 @@ export function createAgentsRouter(deps: AgentsRouterDeps(deps: AgentsRouterDeps { - listAgents(tenantId: string, transaction?: TTransaction): Promise; + listAgents(input: ListAgentsInput, transaction?: TTransaction): Promise; getAgent(input: GetAgentInput, transaction?: TTransaction): Promise; - /** Inserts a new agent with a generated ULID. Throws AgentNameConflictError or AgentExternalIdConflictError on unique clash. */ + /** Inserts a new agent with a generated ULID. Throws AgentNameConflictError, AgentExternalIdConflictError, or AgentNameReservedError. */ createAgent(input: CreateAgentInput, transaction?: TTransaction): Promise; /** Patches `manifest` and/or `external_id`. Throws AgentExternalIdConflictError on unique clash. Returns undefined if missing. */ updateAgent(input: UpdateAgentInput, transaction?: TTransaction): Promise; diff --git a/packages/trueforge/src/db/postgres/agent-store/PostgresAgentStore.ts b/packages/trueforge/src/db/postgres/agent-store/PostgresAgentStore.ts index 629c30c79..9d1e59501 100644 --- a/packages/trueforge/src/db/postgres/agent-store/PostgresAgentStore.ts +++ b/packages/trueforge/src/db/postgres/agent-store/PostgresAgentStore.ts @@ -3,12 +3,14 @@ import { newId } from '../../../utils/id'; import { AgentExternalIdConflictError, AgentNameConflictError, + assertAgentNameNotReserved, parseStoredAgentSpec, type AgentRecord, type CreateAgentInput, type DeleteAgentInput, type GetAgentInput, type IAgentStore, + type ListAgentsInput, type UpdateAgentInput, } from '../../agentStore'; import { AGENT_EXTERNAL_ID_UQ } from '../../indexes'; @@ -53,9 +55,16 @@ export class PostgresAgentStore implements IAgentStore> { this.#db = db; } - async listAgents(tenantId: string, transaction?: Transaction): Promise { + async listAgents(input: ListAgentsInput, transaction?: Transaction): Promise { + if (input.external_ids?.length === 0) { + return []; + } const db = transaction ?? this.#db; - const rows = await db.selectFrom('agent').selectAll().where('tenant_id', '=', tenantId).orderBy('name').execute(); + let query = db.selectFrom('agent').selectAll().where('tenant_id', '=', input.tenant_id); + if (input.external_ids !== undefined) { + query = query.where('external_id', 'in', [...input.external_ids]); + } + const rows = await query.orderBy('name').execute(); return rows.map(toRecord); } @@ -72,6 +81,7 @@ export class PostgresAgentStore implements IAgentStore> { } async createAgent(input: CreateAgentInput, transaction?: Transaction): Promise { + assertAgentNameNotReserved(input.name); const db = transaction ?? this.#db; try { const row = await db diff --git a/packages/trueforge/src/db/sqlite/agent-store/SqliteAgentStore.ts b/packages/trueforge/src/db/sqlite/agent-store/SqliteAgentStore.ts index 722735475..9f88825f9 100644 --- a/packages/trueforge/src/db/sqlite/agent-store/SqliteAgentStore.ts +++ b/packages/trueforge/src/db/sqlite/agent-store/SqliteAgentStore.ts @@ -4,12 +4,14 @@ import { newId } from '../../../utils/id'; import { AgentExternalIdConflictError, AgentNameConflictError, + assertAgentNameNotReserved, parseStoredAgentSpec, type AgentRecord, type CreateAgentInput, type DeleteAgentInput, type GetAgentInput, type IAgentStore, + type ListAgentsInput, type UpdateAgentInput, } from '../../agentStore'; import { isUniqueViolation } from '../client'; @@ -48,14 +50,16 @@ export class SqliteAgentStore implements IAgentStore> { this.#db = db; } - async listAgents(tenantId: string, transaction?: Transaction): Promise { + async listAgents(input: ListAgentsInput, transaction?: Transaction): Promise { + if (input.external_ids?.length === 0) { + return []; + } const db = transaction ?? this.#db; - const rows = await db - .selectFrom('agent') - .select(recordColumns) - .where('tenant_id', '=', tenantId) - .orderBy('name') - .execute(); + let query = db.selectFrom('agent').select(recordColumns).where('tenant_id', '=', input.tenant_id); + if (input.external_ids !== undefined) { + query = query.where('external_id', 'in', [...input.external_ids]); + } + const rows = await query.orderBy('name').execute(); return rows.map(toRecord); } @@ -72,6 +76,7 @@ export class SqliteAgentStore implements IAgentStore> { } async createAgent(input: CreateAgentInput, transaction?: Transaction): Promise { + assertAgentNameNotReserved(input.name); const db = transaction ?? this.#db; const timestamp = nowIso(); try { diff --git a/packages/trueforge/src/routes/agentRoutes.ts b/packages/trueforge/src/routes/agentRoutes.ts index 23c0a4f5f..99fcc436e 100644 --- a/packages/trueforge/src/routes/agentRoutes.ts +++ b/packages/trueforge/src/routes/agentRoutes.ts @@ -173,6 +173,10 @@ export const putAgentRoute = createRoute({ content: { 'application/json': { schema: RequestErrorResponseSchema } }, description: 'Agent not found.', }, + 409: { + content: { 'application/json': { schema: RequestErrorResponseSchema } }, + description: 'The update would reuse an external_id already claimed by another agent.', + }, 422: { content: { 'application/json': { schema: RequestErrorResponseSchema } }, description: diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts index a0c16b6a4..d17d9cea2 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -1,10 +1,12 @@ import { AgentNameConflictError, + assertAgentNameNotReserved, type AgentRecord, type CreateAgentInput, type DeleteAgentInput, type GetAgentInput, type IAgentStore, + type ListAgentsInput, type UpdateAgentInput, } from '../db/agentStore'; import { toPutRemoteAgentPayload } from './toPutRemoteAgentPayload'; @@ -34,8 +36,8 @@ export class TrueFoundryAgentStore implements IAgentStore< this.#accessToken = input.accessToken; } - listAgents(tenantId: string, transaction?: TTransaction): Promise { - return this.#inner.listAgents(tenantId, transaction); + listAgents(input: ListAgentsInput, transaction?: TTransaction): Promise { + return this.#inner.listAgents(input, transaction); } getAgent(input: GetAgentInput, transaction?: TTransaction): Promise { @@ -43,6 +45,9 @@ export class TrueFoundryAgentStore implements IAgentStore< } async createAgent(input: CreateAgentInput, transaction?: TTransaction): Promise { + // Reject reserved names here so we never create them in ServiceFoundry first. + assertAgentNameNotReserved(input.name); + // SF PUT upserts by name — skip if local name exists (avoids overwrite/delete of e.g. research→sf-1). const existing = await this.#inner.getAgent({ tenant_id: input.tenant_id, name: input.name }, transaction); if (existing !== undefined) { diff --git a/packages/trueforge/tests/db/agentStoreContractSuite.ts b/packages/trueforge/tests/db/agentStoreContractSuite.ts index 157ec3f8f..7464c375c 100644 --- a/packages/trueforge/tests/db/agentStoreContractSuite.ts +++ b/packages/trueforge/tests/db/agentStoreContractSuite.ts @@ -3,7 +3,12 @@ * Runs under jest against a fresh store per test (see backend test files). */ import { AgentSpecSchema, type AgentSpec } from '@truefoundry/trueforge-core/agent-session'; -import { AgentExternalIdConflictError, AgentNameConflictError, type IAgentStore } from '../../src/db/agentStore'; +import { + AgentExternalIdConflictError, + AgentNameConflictError, + AgentNameReservedError, + type IAgentStore, +} from '../../src/db/agentStore'; const TENANT = 'default'; @@ -111,6 +116,16 @@ export function runAgentStoreContractSuite(getStore: () => IAgentStore): void { ).rejects.toBeInstanceOf(AgentNameConflictError); }); + it('createAgent throws AgentNameReservedError for tfg and trueforge', async () => { + const store = getStore(); + await expect( + store.createAgent({ tenant_id: TENANT, name: 'tfg', manifest: manifest(), external_id: null }), + ).rejects.toBeInstanceOf(AgentNameReservedError); + await expect( + store.createAgent({ tenant_id: TENANT, name: 'trueforge', manifest: manifest(), external_id: null }), + ).rejects.toBeInstanceOf(AgentNameReservedError); + }); + it('listAgents returns only the tenant, ordered by name', async () => { const store = getStore(); await store.createAgent({ @@ -132,11 +147,42 @@ export function runAgentStoreContractSuite(getStore: () => IAgentStore): void { external_id: null, }); - const agents = await store.listAgents(TENANT); + const agents = await store.listAgents({ tenant_id: TENANT }); expect(agents.map(agent => agent.name)).toEqual(['alpha', 'zeta']); expect(agents.every(agent => agent.tenant_id === TENANT)).toBe(true); }); + it('listAgents can filter by external_ids', async () => { + const store = getStore(); + await store.createAgent({ + tenant_id: TENANT, + name: 'local-only', + manifest: manifest(), + external_id: null, + }); + const linked = await store.createAgent({ + tenant_id: TENANT, + name: 'linked', + manifest: manifest(), + external_id: 'sf-agent-1', + }); + await store.createAgent({ + tenant_id: TENANT, + name: 'other-linked', + manifest: manifest(), + external_id: 'sf-agent-2', + }); + + expect(await store.listAgents({ tenant_id: TENANT, external_ids: ['sf-agent-1'] })).toEqual([linked]); + expect( + (await store.listAgents({ tenant_id: TENANT, external_ids: ['sf-agent-1', 'sf-agent-2'] })).map( + agent => agent.name, + ), + ).toEqual(['linked', 'other-linked']); + expect(await store.listAgents({ tenant_id: TENANT, external_ids: ['missing'] })).toEqual([]); + expect(await store.listAgents({ tenant_id: TENANT, external_ids: [] })).toEqual([]); + }); + it('getAgent by id is tenant-scoped', async () => { const store = getStore(); const created = await store.createAgent({ diff --git a/packages/trueforge/tests/unit/apis/agents.test.ts b/packages/trueforge/tests/unit/apis/agents.test.ts index 550bce5dc..7d44cdcbc 100644 --- a/packages/trueforge/tests/unit/apis/agents.test.ts +++ b/packages/trueforge/tests/unit/apis/agents.test.ts @@ -1,4 +1,5 @@ import { createAgentsRouter } from '../../../src/apis/agents'; +import { AgentExternalIdConflictError, type IAgentStore } from '../../../src/db/agentStore'; import { migrateSqliteToLatest } from '../../../src/db/migrateSqlite'; import { SqliteAgentStore } from '../../../src/db/sqlite/agent-store/SqliteAgentStore'; import { createSqliteDb } from '../../../src/db/sqlite/client'; @@ -70,19 +71,27 @@ function jsonInit(method: string, body: unknown): RequestInit { describe('agents router', () => { let router: ReturnType; let agentStore: SqliteAgentStore; + let modelProviderStore: SqliteModelProviderStore; + let mcpServerStore: SqliteMcpServerStore; + let skillStore: SqliteSkillStore; + let sandboxProviderStore: SqliteSandboxProviderStore; + let db: ReturnType; beforeAll(async () => { - const db = createSqliteDb(':memory:'); + db = createSqliteDb(':memory:'); await migrateSqliteToLatest(db); - const modelProviderStore = new SqliteModelProviderStore(db); + modelProviderStore = new SqliteModelProviderStore(db); await modelProviderStore.upsertProvider({ tenant_id: 'default', name: 'anthropic', manifest: modelProvider }); agentStore = new SqliteAgentStore(db); + mcpServerStore = new SqliteMcpServerStore(db); + skillStore = new SqliteSkillStore(db); + sandboxProviderStore = new SqliteSandboxProviderStore(db); router = createAgentsRouter({ resolveAgentStore: () => agentStore, resolveModelProviderStore: () => modelProviderStore, - resolveMcpServerStore: () => new SqliteMcpServerStore(db), - skillStore: new SqliteSkillStore(db), - sandboxProviderStore: new SqliteSandboxProviderStore(db), + resolveMcpServerStore: () => mcpServerStore, + skillStore, + sandboxProviderStore, withTransaction: callback => db.transaction().execute(callback), }); }); @@ -138,6 +147,31 @@ describe('agents router', () => { expect(snippets.status).toBe(404); }); + it('PUT returns 409 when updateAgent hits an external_id conflict', async () => { + const conflictStore: IAgentStore = { + listAgents: async () => [], + getAgent: async () => undefined, + createAgent: async () => { + throw new Error('unused'); + }, + updateAgent: async () => { + throw new AgentExternalIdConflictError({ tenant_id: 'default', external_id: 'sf-taken' }); + }, + deleteAgent: async () => {}, + }; + const conflictRouter = createAgentsRouter({ + resolveAgentStore: () => conflictStore, + resolveModelProviderStore: () => modelProviderStore, + resolveMcpServerStore: () => mcpServerStore, + skillStore, + sandboxProviderStore, + withTransaction: callback => db.transaction().execute(callback), + }); + + const put = await conflictRouter.request('/any-agent-id', jsonInit('PUT', updateBody)); + expect(put.status).toBe(409); + }); + it('GET code-snippets returns snippets for an existing agent', async () => { const created = await router.request('/', jsonInit('POST', { ...writeBody, name: 'snippet-bot' })); expect(created.status).toBe(201); @@ -168,6 +202,9 @@ describe('agents router', () => { const badName = await router.request('/', jsonInit('POST', { ...writeBody, name: 'Not A Name' })); expect(badName.status).toBe(400); + const reserved = await router.request('/', jsonInit('POST', { ...writeBody, name: 'tfg' })); + expect(reserved.status).toBe(400); + const unknownModel = await router.request( '/', jsonInit('POST', { diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts index 8adb73192..225600d8e 100644 --- a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -1,7 +1,7 @@ import { AgentSpecSchema } from '@truefoundry/trueforge-core/agent-session'; import type { AgentRecord, IAgentStore } from '../../../src/db/agentStore'; -import { AgentNameConflictError } from '../../../src/db/agentStore'; +import { AgentNameConflictError, AgentNameReservedError } from '../../../src/db/agentStore'; import { TrueFoundryAgentStore } from '../../../src/truefoundry/TrueFoundryAgentStore'; import { TrueFoundryServiceFoundryServerClient, @@ -80,9 +80,9 @@ describe('TrueFoundryAgentStore', () => { accessToken: TOKEN, }); - await expect(store.listAgents(TENANT)).resolves.toBe(agents); + await expect(store.listAgents({ tenant_id: TENANT })).resolves.toBe(agents); await expect(store.getAgent({ tenant_id: TENANT, id: 'agent-1' })).resolves.toBe(agents[0]); - expect(listAgents).toHaveBeenCalledWith(TENANT, undefined); + expect(listAgents).toHaveBeenCalledWith({ tenant_id: TENANT }, undefined); expect(getAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'agent-1' }, undefined); }); @@ -144,6 +144,27 @@ describe('TrueFoundryAgentStore', () => { expect(deleteRemoteAgent).not.toHaveBeenCalled(); }); + it('createAgent rejects reserved names before calling ServiceFoundry', async () => { + const getAgent = jest.fn(); + const createAgent = jest.fn(); + const putRemoteAgent = jest.fn(); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, createAgent }), + client: mockClient({ putRemoteAgent }), + accessToken: TOKEN, + }); + + await expect( + store.createAgent({ tenant_id: TENANT, name: 'tfg', manifest: manifest(), external_id: null }), + ).rejects.toBeInstanceOf(AgentNameReservedError); + await expect( + store.createAgent({ tenant_id: TENANT, name: 'trueforge', manifest: manifest(), external_id: null }), + ).rejects.toBeInstanceOf(AgentNameReservedError); + expect(getAgent).not.toHaveBeenCalled(); + expect(putRemoteAgent).not.toHaveBeenCalled(); + expect(createAgent).not.toHaveBeenCalled(); + }); + it('createAgent uses agent name as description when instructions are omitted', async () => { const putRemoteAgent = jest.fn(async (input: PutRemoteAgentInput) => { expect(input.description).toBe('research'); From bb53fda9feb219118be5c66beceae14847e48218 Mon Sep 17 00:00:00 2001 From: Bhavesh Patel Date: Thu, 3 Sep 2026 14:41:03 +0530 Subject: [PATCH 3/8] comments addressed --- .../20260902194500-truefoundry-agent-store.md | 2 +- packages/trueforge/src/apis/agents.ts | 23 ++--- packages/trueforge/src/routes/agentRoutes.ts | 4 - packages/trueforge/src/truefoundry/AGENTS.md | 1 - packages/trueforge/src/truefoundry/CLAUDE.md | 1 - .../src/truefoundry/TrueFoundryAgentStore.ts | 25 +++++- .../TrueFoundryServiceFoundryServerClient.ts | 86 +++++++++++-------- .../truefoundry/toPutRemoteAgentPayload.ts | 21 ----- .../trueforge/tests/unit/apis/agents.test.ts | 44 ++-------- 9 files changed, 84 insertions(+), 123 deletions(-) delete mode 100644 packages/trueforge/src/truefoundry/AGENTS.md delete mode 100644 packages/trueforge/src/truefoundry/CLAUDE.md delete mode 100644 packages/trueforge/src/truefoundry/toPutRemoteAgentPayload.ts diff --git a/.changeset/20260902194500-truefoundry-agent-store.md b/.changeset/20260902194500-truefoundry-agent-store.md index 159ca1e02..d83e98d5f 100644 --- a/.changeset/20260902194500-truefoundry-agent-store.md +++ b/.changeset/20260902194500-truefoundry-agent-store.md @@ -2,4 +2,4 @@ "@truefoundry/trueforge": patch --- -Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. Filter `listAgents` by `external_ids`. +Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. Filter `listAgents` by `external_ids`. Time out ServiceFoundry HTTP after 10s. diff --git a/packages/trueforge/src/apis/agents.ts b/packages/trueforge/src/apis/agents.ts index 104f4d6ab..fdf26658d 100644 --- a/packages/trueforge/src/apis/agents.ts +++ b/packages/trueforge/src/apis/agents.ts @@ -143,22 +143,15 @@ export function createAgentsRouter(deps: AgentsRouterDeps { + return { + name, + description: manifest.instructions ?? name, + model: manifest.model.name, + ...(manifest.mcp_servers === undefined ? {} : { mcp_servers: manifest.mcp_servers.map(server => server.name) }), + }; +} + /** * create: getByName → putRemote → createDB(external_id) | on non-conflict DB fail → deleteRemote * update: putRemote(new) → updateDB | on DB fail → putRemote(old) | both fail → AggregateError @@ -61,7 +79,8 @@ export class TrueFoundryAgentStore implements IAgentStore< try { return await this.#inner.createAgent({ ...input, external_id: remoteAgentId }, transaction); } catch (error) { - // Race: peer create won the name and owns this remote (1:1) — do not delete it. + // Skip cleanup on name conflict: a peer may own this remote. Otherwise best-effort delete + // (agents create is not in an outer DB txn today). if (!(error instanceof AgentNameConflictError)) { try { await this.#client.deleteRemoteAgent({ accessToken: this.#accessToken, remoteAgentId }); diff --git a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts index 3297e7709..7826d89ec 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts @@ -13,6 +13,8 @@ const TFG_AGENTS_PATH = 'internal/tfg/agents'; const INTEGRATIONS_PAGE_SIZE = 1000; const MCP_SERVERS_PAGE_SIZE = 100; +export const SERVICE_FOUNDRY_HTTP_TIMEOUT_MS = 10_000; + const ListResponseSchema = z.union([ z.array(z.unknown()), z.object({ @@ -233,9 +235,9 @@ export class TrueFoundryServiceFoundryServerClient { notFoundOk?: boolean; }): Promise { const startedAt = Date.now(); - let response: Awaited>; + const signal = AbortSignal.timeout(SERVICE_FOUNDRY_HTTP_TIMEOUT_MS); try { - response = await undiciFetch(input.url, { + const response = await undiciFetch(input.url, { method: input.method, headers: { accept: 'application/json', @@ -243,52 +245,60 @@ export class TrueFoundryServiceFoundryServerClient { ...(input.body === undefined ? {} : { 'content-type': 'application/json' }), }, ...(input.body === undefined ? {} : { body: JSON.stringify(input.body) }), + signal, ...(this.#dispatcher ? { dispatcher: this.#dispatcher } : {}), }); + this.#logger?.info('TrueFoundry ServiceFoundry server request completed', { + url: input.url.href, + method: input.method, + status: response.status, + durationMs: Date.now() - startedAt, + }); + if (response.status === 401 || response.status === 403) { + throw new HTTPException(response.status, { + message: 'TrueFoundry ServiceFoundry server rejected the request', + }); + } + if (response.status === 404 && input.notFoundOk) { + return undefined; + } + if (!response.ok) { + const detail = await readServiceFoundryErrorMessage(response); + throw new HTTPException(424, { + message: `TrueFoundry ServiceFoundry server request failed: ${detail ?? `HTTP ${String(response.status)}`}`, + }); + } + if (response.status === 204) { + return undefined; + } + const text = await response.text(); + if (text.length === 0) { + return undefined; + } + try { + return JSON.parse(text); + } catch (error) { + throw new HTTPException(424, { + message: 'TrueFoundry ServiceFoundry server returned invalid JSON', + cause: error, + }); + } } catch (error) { + if (error instanceof HTTPException) { + throw error; + } + const timedOut = error instanceof Error && error.name === 'TimeoutError'; this.#logger?.warn('TrueFoundry ServiceFoundry server request failed', { url: input.url.href, method: input.method, durationMs: Date.now() - startedAt, + timedOut, ...extractErrorLogFields(error), }); throw new HTTPException(500, { - message: 'TrueFoundry ServiceFoundry server request failed', - cause: error, - }); - } - this.#logger?.info('TrueFoundry ServiceFoundry server request completed', { - url: input.url.href, - method: input.method, - status: response.status, - durationMs: Date.now() - startedAt, - }); - if (response.status === 401 || response.status === 403) { - throw new HTTPException(response.status, { - message: 'TrueFoundry ServiceFoundry server rejected the request', - }); - } - if (response.status === 404 && input.notFoundOk) { - return undefined; - } - if (!response.ok) { - const detail = await readServiceFoundryErrorMessage(response); - throw new HTTPException(424, { - message: `TrueFoundry ServiceFoundry server request failed: ${detail ?? `HTTP ${String(response.status)}`}`, - }); - } - if (response.status === 204) { - return undefined; - } - const text = await response.text(); - if (text.length === 0) { - return undefined; - } - try { - return JSON.parse(text); - } catch (error) { - throw new HTTPException(424, { - message: 'TrueFoundry ServiceFoundry server returned invalid JSON', + message: timedOut + ? `TrueFoundry ServiceFoundry server request timed out after ${String(SERVICE_FOUNDRY_HTTP_TIMEOUT_MS / 1000)}s` + : 'TrueFoundry ServiceFoundry server request failed', cause: error, }); } diff --git a/packages/trueforge/src/truefoundry/toPutRemoteAgentPayload.ts b/packages/trueforge/src/truefoundry/toPutRemoteAgentPayload.ts deleted file mode 100644 index f27d7380f..000000000 --- a/packages/trueforge/src/truefoundry/toPutRemoteAgentPayload.ts +++ /dev/null @@ -1,21 +0,0 @@ -/** - * AgentSpec → PUT `/internal/tfg/agents` body fields (domain → SF; inverse of mapSfyMcpServers / mapEnabledModels). - */ -import type { AgentSpec } from '@truefoundry/trueforge-core/agent-session'; - -import type { PutRemoteAgentInput } from './TrueFoundryServiceFoundryServerClient'; - -export function toPutRemoteAgentPayload({ - name, - manifest, -}: { - name: string; - manifest: AgentSpec; -}): Omit { - return { - name, - description: manifest.instructions ?? name, - model: manifest.model.name, - ...(manifest.mcp_servers === undefined ? {} : { mcp_servers: manifest.mcp_servers.map(server => server.name) }), - }; -} diff --git a/packages/trueforge/tests/unit/apis/agents.test.ts b/packages/trueforge/tests/unit/apis/agents.test.ts index 7d44cdcbc..87174a5bb 100644 --- a/packages/trueforge/tests/unit/apis/agents.test.ts +++ b/packages/trueforge/tests/unit/apis/agents.test.ts @@ -1,5 +1,4 @@ import { createAgentsRouter } from '../../../src/apis/agents'; -import { AgentExternalIdConflictError, type IAgentStore } from '../../../src/db/agentStore'; import { migrateSqliteToLatest } from '../../../src/db/migrateSqlite'; import { SqliteAgentStore } from '../../../src/db/sqlite/agent-store/SqliteAgentStore'; import { createSqliteDb } from '../../../src/db/sqlite/client'; @@ -71,27 +70,19 @@ function jsonInit(method: string, body: unknown): RequestInit { describe('agents router', () => { let router: ReturnType; let agentStore: SqliteAgentStore; - let modelProviderStore: SqliteModelProviderStore; - let mcpServerStore: SqliteMcpServerStore; - let skillStore: SqliteSkillStore; - let sandboxProviderStore: SqliteSandboxProviderStore; - let db: ReturnType; beforeAll(async () => { - db = createSqliteDb(':memory:'); + const db = createSqliteDb(':memory:'); await migrateSqliteToLatest(db); - modelProviderStore = new SqliteModelProviderStore(db); + const modelProviderStore = new SqliteModelProviderStore(db); await modelProviderStore.upsertProvider({ tenant_id: 'default', name: 'anthropic', manifest: modelProvider }); agentStore = new SqliteAgentStore(db); - mcpServerStore = new SqliteMcpServerStore(db); - skillStore = new SqliteSkillStore(db); - sandboxProviderStore = new SqliteSandboxProviderStore(db); router = createAgentsRouter({ resolveAgentStore: () => agentStore, resolveModelProviderStore: () => modelProviderStore, - resolveMcpServerStore: () => mcpServerStore, - skillStore, - sandboxProviderStore, + resolveMcpServerStore: () => new SqliteMcpServerStore(db), + skillStore: new SqliteSkillStore(db), + sandboxProviderStore: new SqliteSandboxProviderStore(db), withTransaction: callback => db.transaction().execute(callback), }); }); @@ -147,31 +138,6 @@ describe('agents router', () => { expect(snippets.status).toBe(404); }); - it('PUT returns 409 when updateAgent hits an external_id conflict', async () => { - const conflictStore: IAgentStore = { - listAgents: async () => [], - getAgent: async () => undefined, - createAgent: async () => { - throw new Error('unused'); - }, - updateAgent: async () => { - throw new AgentExternalIdConflictError({ tenant_id: 'default', external_id: 'sf-taken' }); - }, - deleteAgent: async () => {}, - }; - const conflictRouter = createAgentsRouter({ - resolveAgentStore: () => conflictStore, - resolveModelProviderStore: () => modelProviderStore, - resolveMcpServerStore: () => mcpServerStore, - skillStore, - sandboxProviderStore, - withTransaction: callback => db.transaction().execute(callback), - }); - - const put = await conflictRouter.request('/any-agent-id', jsonInit('PUT', updateBody)); - expect(put.status).toBe(409); - }); - it('GET code-snippets returns snippets for an existing agent', async () => { const created = await router.request('/', jsonInit('POST', { ...writeBody, name: 'snippet-bot' })); expect(created.status).toBe(201); From e10aaf711f68303b120abc48307aacadb330e6fb Mon Sep 17 00:00:00 2001 From: Bhavesh Patel Date: Thu, 3 Sep 2026 16:08:01 +0530 Subject: [PATCH 4/8] =?UTF-8?q?Approach=20A=20=E2=80=94=20Insert=20first?= =?UTF-8?q?=20in=20create?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../20260902194500-truefoundry-agent-store.md | 2 +- .../src/truefoundry/TrueFoundryAgentStore.ts | 59 +++---- .../TrueFoundryServiceFoundryServerClient.ts | 8 +- .../truefoundry/TrueFoundryAgentStore.test.ts | 148 +++++++++--------- 4 files changed, 112 insertions(+), 105 deletions(-) diff --git a/.changeset/20260902194500-truefoundry-agent-store.md b/.changeset/20260902194500-truefoundry-agent-store.md index d83e98d5f..864fa0659 100644 --- a/.changeset/20260902194500-truefoundry-agent-store.md +++ b/.changeset/20260902194500-truefoundry-agent-store.md @@ -2,4 +2,4 @@ "@truefoundry/trueforge": patch --- -Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. Filter `listAgents` by `external_ids`. Time out ServiceFoundry HTTP after 10s. +Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. Create inserts locally first, then syncs SF (avoids same-name MCP desync). Filter `listAgents` by `external_ids`. Time out ServiceFoundry HTTP after 10s. diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts index c9111c435..dc40b7e6f 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -1,7 +1,5 @@ import type { AgentSpec } from '@truefoundry/trueforge-core/agent-session'; import { - AgentNameConflictError, - assertAgentNameNotReserved, type AgentRecord, type CreateAgentInput, type DeleteAgentInput, @@ -35,7 +33,7 @@ function toPutRemoteAgentPayload({ } /** - * create: getByName → putRemote → createDB(external_id) | on non-conflict DB fail → deleteRemote + * create: createDB(null) → putRemote → updateDB(external_id) | on put/update fail → deleteDB (+ deleteRemote if put ok) * update: putRemote(new) → updateDB | on DB fail → putRemote(old) | both fail → AggregateError * delete: deleteRemote(404 ok) → deleteDB */ @@ -63,35 +61,40 @@ export class TrueFoundryAgentStore implements IAgentStore< } async createAgent(input: CreateAgentInput, transaction?: TTransaction): Promise { - // Reject reserved names here so we never create them in ServiceFoundry first. - assertAgentNameNotReserved(input.name); + // Insert first so unique name picks the winner; only the winner calls SF (avoids MCP desync). + const created = await this.#inner.createAgent({ ...input, external_id: null }, transaction); - // SF PUT upserts by name — skip if local name exists (avoids overwrite/delete of e.g. research→sf-1). - const existing = await this.#inner.getAgent({ tenant_id: input.tenant_id, name: input.name }, transaction); - if (existing !== undefined) { - throw new AgentNameConflictError({ tenant_id: input.tenant_id, name: input.name }); - } - - const { remoteAgentId } = await this.#client.putRemoteAgent({ - accessToken: this.#accessToken, - ...toPutRemoteAgentPayload({ name: input.name, manifest: input.manifest }), - }); + let externalId: string | undefined; try { - return await this.#inner.createAgent({ ...input, external_id: remoteAgentId }, transaction); + ({ externalId } = await this.#client.putRemoteAgent({ + accessToken: this.#accessToken, + ...toPutRemoteAgentPayload({ name: input.name, manifest: input.manifest }), + })); + const updated = await this.#inner.updateAgent( + { tenant_id: input.tenant_id, id: created.id, external_id: externalId }, + transaction, + ); + if (updated === undefined) { + throw new Error(`Internal error: createAgent lost the row after insert: ${created.id}`); + } + return updated; } catch (error) { - // Skip cleanup on name conflict: a peer may own this remote. Otherwise best-effort delete - // (agents create is not in an outer DB txn today). - if (!(error instanceof AgentNameConflictError)) { + const failures = [asError(error)]; + if (externalId !== undefined) { try { - await this.#client.deleteRemoteAgent({ accessToken: this.#accessToken, remoteAgentId }); + await this.#client.deleteRemoteAgent({ accessToken: this.#accessToken, externalId }); } catch (cleanupError) { - throw new AggregateError( - [asError(error), asError(cleanupError)], - 'createAgent failed and ServiceFoundry cleanup also failed', - { cause: cleanupError }, - ); + failures.push(asError(cleanupError)); } } + try { + await this.#inner.deleteAgent({ tenant_id: input.tenant_id, id: created.id }, transaction); + } catch (cleanupError) { + failures.push(asError(cleanupError)); + } + if (failures.length > 1) { + throw new AggregateError(failures, 'createAgent failed and cleanup also failed', { cause: error }); + } throw error; } } @@ -106,7 +109,7 @@ export class TrueFoundryAgentStore implements IAgentStore< return undefined; } - const { remoteAgentId } = await this.#client.putRemoteAgent({ + const { externalId } = await this.#client.putRemoteAgent({ accessToken: this.#accessToken, ...toPutRemoteAgentPayload({ name: previous.name, manifest: input.manifest }), }); @@ -117,7 +120,7 @@ export class TrueFoundryAgentStore implements IAgentStore< tenant_id: input.tenant_id, id: input.id, manifest: input.manifest, - ...(remoteAgentId === previous.external_id ? {} : { external_id: remoteAgentId }), + ...(externalId === previous.external_id ? {} : { external_id: externalId }), }, transaction, ); @@ -143,7 +146,7 @@ export class TrueFoundryAgentStore implements IAgentStore< if (previous?.external_id) { await this.#client.deleteRemoteAgent({ accessToken: this.#accessToken, - remoteAgentId: previous.external_id, + externalId: previous.external_id, }); } await this.#inner.deleteAgent(input, transaction); diff --git a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts index 17182dd42..404a32654 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts @@ -72,12 +72,12 @@ export interface PutRemoteAgentInput { } export interface PutRemoteAgentResult { - remoteAgentId: string; + externalId: string; } export interface DeleteRemoteAgentInput { accessToken: string; - remoteAgentId: string; + externalId: string; } async function readServiceFoundryErrorMessage( @@ -214,13 +214,13 @@ export class TrueFoundryServiceFoundryServerClient { cause: parsed.error, }); } - return { remoteAgentId: parsed.data.agentId }; + return { externalId: parsed.data.agentId }; } /** DELETE `/internal/tfg/agents/:id` — remove remote agent. Missing agent (404) is success. */ async deleteRemoteAgent(input: DeleteRemoteAgentInput): Promise { await this.#requestJson({ - url: this.#url(`${TFG_AGENTS_PATH}/${encodeURIComponent(input.remoteAgentId)}`), + url: this.#url(`${TFG_AGENTS_PATH}/${encodeURIComponent(input.externalId)}`), accessToken: input.accessToken, method: 'DELETE', notFoundOk: true, diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts index 225600d8e..658c4a600 100644 --- a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -1,7 +1,7 @@ import { AgentSpecSchema } from '@truefoundry/trueforge-core/agent-session'; import type { AgentRecord, IAgentStore } from '../../../src/db/agentStore'; -import { AgentNameConflictError, AgentNameReservedError } from '../../../src/db/agentStore'; +import { AgentNameConflictError, AgentNameReservedError, assertAgentNameNotReserved } from '../../../src/db/agentStore'; import { TrueFoundryAgentStore } from '../../../src/truefoundry/TrueFoundryAgentStore'; import { TrueFoundryServiceFoundryServerClient, @@ -56,7 +56,7 @@ function mockClient( serviceFoundryServerUrl: 'http://servicefoundry.test', }); client.putRemoteAgent = - overrides.putRemoteAgent ?? (async (): Promise => ({ remoteAgentId: 'sf-1' })); + overrides.putRemoteAgent ?? (async (): Promise => ({ externalId: 'sf-1' })); client.deleteRemoteAgent = overrides.deleteRemoteAgent ?? (async (_input: DeleteRemoteAgentInput) => undefined); return client; } @@ -86,8 +86,9 @@ describe('TrueFoundryAgentStore', () => { expect(getAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'agent-1' }, undefined); }); - it('createAgent puts then inserts with external_id', async () => { - const created = record({ external_id: 'sf-1' }); + it('createAgent inserts locally, puts remote, then sets external_id', async () => { + const local = record({ external_id: null }); + const linked = record({ external_id: 'sf-1' }); const putRemoteAgent = jest.fn(async (input: PutRemoteAgentInput) => { expect(input).toEqual({ accessToken: TOKEN, @@ -96,12 +97,12 @@ describe('TrueFoundryAgentStore', () => { model: 'openai-gateway/gpt-5', mcp_servers: ['slack'], }); - return { remoteAgentId: 'sf-1' }; + return { externalId: 'sf-1' }; }); - const getAgent = jest.fn(async () => undefined); - const createAgent = jest.fn(async () => created); + const createAgent = jest.fn(async () => local); + const updateAgent = jest.fn(async () => linked); const store = new TrueFoundryAgentStore({ - inner: mockInner({ getAgent, createAgent }), + inner: mockInner({ createAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, }); @@ -113,43 +114,49 @@ describe('TrueFoundryAgentStore', () => { manifest: manifest({ mcp_servers: [{ name: 'slack' }] }), external_id: null, }), - ).resolves.toBe(created); - expect(getAgent).toHaveBeenCalledWith({ tenant_id: TENANT, name: 'research' }, undefined); + ).resolves.toBe(linked); expect(createAgent).toHaveBeenCalledWith( expect.objectContaining({ tenant_id: TENANT, name: 'research', - external_id: 'sf-1', + external_id: null, }), undefined, ); + expect(updateAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: local.id, external_id: 'sf-1' }, undefined); + expect(firstInvocationOrder(createAgent)).toBeLessThan(firstInvocationOrder(putRemoteAgent)); + expect(firstInvocationOrder(putRemoteAgent)).toBeLessThan(firstInvocationOrder(updateAgent)); }); it('createAgent rejects a duplicate local name before calling ServiceFoundry', async () => { - const getAgent = jest.fn(async () => record({ external_id: 'sf-existing' })); - const createAgent = jest.fn(); + const createAgent = jest.fn(async () => { + throw new AgentNameConflictError({ tenant_id: TENANT, name: 'research' }); + }); const putRemoteAgent = jest.fn(); + const updateAgent = jest.fn(); const deleteRemoteAgent = jest.fn(); const store = new TrueFoundryAgentStore({ - inner: mockInner({ getAgent, createAgent }), + inner: mockInner({ createAgent, updateAgent }), client: mockClient({ putRemoteAgent, deleteRemoteAgent }), accessToken: TOKEN, }); await expect( store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), - ).rejects.toMatchObject({ name: 'AgentNameConflictError' }); + ).rejects.toBeInstanceOf(AgentNameConflictError); expect(putRemoteAgent).not.toHaveBeenCalled(); - expect(createAgent).not.toHaveBeenCalled(); + expect(updateAgent).not.toHaveBeenCalled(); expect(deleteRemoteAgent).not.toHaveBeenCalled(); }); it('createAgent rejects reserved names before calling ServiceFoundry', async () => { - const getAgent = jest.fn(); - const createAgent = jest.fn(); + const createAgent = jest.fn(async (input: { name: string }) => { + assertAgentNameNotReserved(input.name); + return record(); + }); const putRemoteAgent = jest.fn(); const store = new TrueFoundryAgentStore({ - inner: mockInner({ getAgent, createAgent }), + inner: mockInner({ createAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, }); @@ -160,19 +167,18 @@ describe('TrueFoundryAgentStore', () => { await expect( store.createAgent({ tenant_id: TENANT, name: 'trueforge', manifest: manifest(), external_id: null }), ).rejects.toBeInstanceOf(AgentNameReservedError); - expect(getAgent).not.toHaveBeenCalled(); expect(putRemoteAgent).not.toHaveBeenCalled(); - expect(createAgent).not.toHaveBeenCalled(); }); it('createAgent uses agent name as description when instructions are omitted', async () => { const putRemoteAgent = jest.fn(async (input: PutRemoteAgentInput) => { expect(input.description).toBe('research'); - return { remoteAgentId: 'sf-1' }; + return { externalId: 'sf-1' }; }); - const createAgent = jest.fn(async () => record({ external_id: 'sf-1' })); + const createAgent = jest.fn(async () => record({ external_id: null })); + const updateAgent = jest.fn(async () => record({ external_id: 'sf-1' })); const store = new TrueFoundryAgentStore({ - inner: mockInner({ createAgent }), + inner: mockInner({ createAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, }); @@ -186,49 +192,64 @@ describe('TrueFoundryAgentStore', () => { expect(putRemoteAgent).toHaveBeenCalled(); }); - it('createAgent rolls back SF when DB insert fails', async () => { - const deleteRemoteAgent = jest.fn(async () => undefined); - const createAgent = jest.fn(async () => { - throw new Error('db failed'); + it('createAgent deletes the local row when putRemoteAgent fails', async () => { + const local = record({ external_id: null }); + const createAgent = jest.fn(async () => local); + const deleteAgent = jest.fn(async () => undefined); + const updateAgent = jest.fn(); + const putRemoteAgent = jest.fn(async () => { + throw new Error('sf failed'); }); + const deleteRemoteAgent = jest.fn(); const store = new TrueFoundryAgentStore({ - inner: mockInner({ createAgent }), - client: mockClient({ deleteRemoteAgent }), + inner: mockInner({ createAgent, updateAgent, deleteAgent }), + client: mockClient({ putRemoteAgent, deleteRemoteAgent }), accessToken: TOKEN, }); await expect( store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), - ).rejects.toThrow('db failed'); - expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, remoteAgentId: 'sf-1' }); + ).rejects.toThrow('sf failed'); + expect(updateAgent).not.toHaveBeenCalled(); + expect(deleteRemoteAgent).not.toHaveBeenCalled(); + expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: local.id }, undefined); }); - it('createAgent does not delete remote on local name conflict after put', async () => { - const deleteRemoteAgent = jest.fn(); - const createAgent = jest.fn(async () => { - throw new AgentNameConflictError({ tenant_id: TENANT, name: 'research' }); + it('createAgent rolls back remote and local when updateAgent fails', async () => { + const local = record({ external_id: null }); + const createAgent = jest.fn(async () => local); + const updateAgent = jest.fn(async () => { + throw new Error('db update failed'); }); + const deleteAgent = jest.fn(async () => undefined); + const deleteRemoteAgent = jest.fn(async () => undefined); const store = new TrueFoundryAgentStore({ - inner: mockInner({ createAgent }), + inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, }); await expect( store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), - ).rejects.toBeInstanceOf(AgentNameConflictError); - expect(deleteRemoteAgent).not.toHaveBeenCalled(); + ).rejects.toThrow('db update failed'); + expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, externalId: 'sf-1' }); + expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: local.id }, undefined); }); - it('createAgent still throws when SF rollback fails', async () => { - const deleteRemoteAgent = jest.fn(async () => { - throw new Error('cleanup failed'); + it('createAgent still throws when cleanup fails', async () => { + const local = record({ external_id: null }); + const createAgent = jest.fn(async () => local); + const updateAgent = jest.fn(async () => { + throw new Error('db update failed'); }); - const createAgent = jest.fn(async () => { - throw new Error('db failed'); + const deleteAgent = jest.fn(async () => { + throw new Error('local cleanup failed'); + }); + const deleteRemoteAgent = jest.fn(async () => { + throw new Error('remote cleanup failed'); }); const store = new TrueFoundryAgentStore({ - inner: mockInner({ createAgent }), + inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, }); @@ -236,30 +257,13 @@ describe('TrueFoundryAgentStore', () => { await expect( store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), ).rejects.toMatchObject({ - message: 'createAgent failed and ServiceFoundry cleanup also failed', + message: 'createAgent failed and cleanup also failed', errors: [ - expect.objectContaining({ message: 'db failed' }), - expect.objectContaining({ message: 'cleanup failed' }), + expect.objectContaining({ message: 'db update failed' }), + expect.objectContaining({ message: 'remote cleanup failed' }), + expect.objectContaining({ message: 'local cleanup failed' }), ], }); - expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, remoteAgentId: 'sf-1' }); - }); - - it('createAgent does not insert when putRemoteAgent fails', async () => { - const createAgent = jest.fn(); - const putRemoteAgent = jest.fn(async () => { - throw new Error('sf failed'); - }); - const store = new TrueFoundryAgentStore({ - inner: mockInner({ createAgent }), - client: mockClient({ putRemoteAgent }), - accessToken: TOKEN, - }); - - await expect( - store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), - ).rejects.toThrow('sf failed'); - expect(createAgent).not.toHaveBeenCalled(); }); it('updateAgent without manifest passes through to the inner store', async () => { @@ -305,7 +309,7 @@ describe('TrueFoundryAgentStore', () => { const updated = record({ manifest: updatedManifest, external_id: 'sf-1' }); const getAgent = jest.fn(async () => previous); const updateAgent = jest.fn(async () => updated); - const putRemoteAgent = jest.fn(async () => ({ remoteAgentId: 'sf-1' })); + const putRemoteAgent = jest.fn(async () => ({ externalId: 'sf-1' })); const store = new TrueFoundryAgentStore({ inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), @@ -334,7 +338,7 @@ describe('TrueFoundryAgentStore', () => { const updated = record({ manifest: updatedManifest, external_id: 'sf-new' }); const getAgent = jest.fn(async () => previous); const updateAgent = jest.fn(async () => updated); - const putRemoteAgent = jest.fn(async () => ({ remoteAgentId: 'sf-new' })); + const putRemoteAgent = jest.fn(async () => ({ externalId: 'sf-new' })); const store = new TrueFoundryAgentStore({ inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), @@ -387,8 +391,8 @@ describe('TrueFoundryAgentStore', () => { }); const putRemoteAgent = jest .fn() - .mockResolvedValueOnce({ remoteAgentId: 'sf-new' }) - .mockResolvedValueOnce({ remoteAgentId: 'sf-old' }); + .mockResolvedValueOnce({ externalId: 'sf-new' }) + .mockResolvedValueOnce({ externalId: 'sf-old' }); const store = new TrueFoundryAgentStore({ inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), @@ -417,7 +421,7 @@ describe('TrueFoundryAgentStore', () => { }); const putRemoteAgent = jest .fn() - .mockResolvedValueOnce({ remoteAgentId: 'sf-new' }) + .mockResolvedValueOnce({ externalId: 'sf-new' }) .mockRejectedValueOnce(new Error('sf restore failed')); const store = new TrueFoundryAgentStore({ inner: mockInner({ getAgent, updateAgent }), @@ -448,7 +452,7 @@ describe('TrueFoundryAgentStore', () => { }); await store.deleteAgent({ tenant_id: TENANT, id: previous.id }); - expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, remoteAgentId: 'sf-1' }); + expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, externalId: 'sf-1' }); expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: previous.id }, undefined); expect(firstInvocationOrder(deleteRemoteAgent)).toBeLessThan(firstInvocationOrder(deleteAgent)); }); From 6845b069a3200b03d264f1d8c80803c53effd02b Mon Sep 17 00:00:00 2001 From: Bhavesh Patel Date: Thu, 3 Sep 2026 16:24:52 +0530 Subject: [PATCH 5/8] =?UTF-8?q?Approach=20B=20=E2=80=94=20Advisory=20lock?= =?UTF-8?q?=20in=20update?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../20260902194500-truefoundry-agent-store.md | 2 +- packages/trueforge/src/db/agentUpdateLock.ts | 12 +++ .../postgres/agent-store/agentUpdateLock.ts | 15 ++++ packages/trueforge/src/main.ts | 9 ++- .../src/truefoundry/TrueFoundryAgentStore.ts | 74 +++++++++++-------- .../truefoundry/TrueFoundryAgentStore.test.ts | 38 ++++++++++ 6 files changed, 116 insertions(+), 34 deletions(-) create mode 100644 packages/trueforge/src/db/agentUpdateLock.ts create mode 100644 packages/trueforge/src/db/postgres/agent-store/agentUpdateLock.ts diff --git a/.changeset/20260902194500-truefoundry-agent-store.md b/.changeset/20260902194500-truefoundry-agent-store.md index 864fa0659..4fc9c7cb5 100644 --- a/.changeset/20260902194500-truefoundry-agent-store.md +++ b/.changeset/20260902194500-truefoundry-agent-store.md @@ -2,4 +2,4 @@ "@truefoundry/trueforge": patch --- -Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. Create inserts locally first, then syncs SF (avoids same-name MCP desync). Filter `listAgents` by `external_ids`. Time out ServiceFoundry HTTP after 10s. +Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. Create inserts locally first, then syncs SF (avoids same-name MCP desync). Update takes a Postgres advisory lock per agent id (no-op on SQLite). Filter `listAgents` by `external_ids`. Time out ServiceFoundry HTTP after 10s. diff --git a/packages/trueforge/src/db/agentUpdateLock.ts b/packages/trueforge/src/db/agentUpdateLock.ts new file mode 100644 index 000000000..ff4711d32 --- /dev/null +++ b/packages/trueforge/src/db/agentUpdateLock.ts @@ -0,0 +1,12 @@ +/** + * Serializes agent updates that also call ServiceFoundry. + * Implementations: Postgres advisory xact lock, or a no-op for SQLite. + */ +export type WithAgentUpdateLock = ( + input: { tenant_id: string; id: string }, + fn: (transaction: TTransaction | undefined) => Promise, +) => Promise; + +export function withoutAgentUpdateLock(): WithAgentUpdateLock { + return async (_input, fn) => fn(undefined); +} diff --git a/packages/trueforge/src/db/postgres/agent-store/agentUpdateLock.ts b/packages/trueforge/src/db/postgres/agent-store/agentUpdateLock.ts new file mode 100644 index 000000000..350efa6bb --- /dev/null +++ b/packages/trueforge/src/db/postgres/agent-store/agentUpdateLock.ts @@ -0,0 +1,15 @@ +import type { Kysely, Transaction } from 'kysely'; +import { sql } from 'kysely'; + +import type { WithAgentUpdateLock } from '../../agentUpdateLock'; +import type { Database } from '../types'; + +/** Transaction-scoped advisory lock held for the whole callback (including SF HTTP). */ +export function createPostgresAgentUpdateLock(db: Kysely): WithAgentUpdateLock> { + return async (input, fn) => + db.transaction().execute(async trx => { + const key = `tf:agent:${input.tenant_id}:${input.id}`; + await sql`SELECT pg_advisory_xact_lock(hashtext(${key}))`.execute(trx); + return fn(trx); + }); +} diff --git a/packages/trueforge/src/main.ts b/packages/trueforge/src/main.ts index 131fe7163..4e7342952 100644 --- a/packages/trueforge/src/main.ts +++ b/packages/trueforge/src/main.ts @@ -66,9 +66,12 @@ import { SkillCatalog } from './catalog/SkillCatalog'; import { type DistributedServerConfiguration } from './config'; import { createController } from './controller'; import type { IAgentStore } from './db/agentStore'; +import type { WithAgentUpdateLock } from './db/agentUpdateLock'; +import { withoutAgentUpdateLock } from './db/agentUpdateLock'; import type { IMcpServerStore, IMcpServerWithAuthStore } from './db/mcpServerStore'; import { McpServerWithAuthStore } from './db/McpServerWithAuthStore'; import type { IModelProviderStore } from './db/modelProviderStore'; +import { createPostgresAgentUpdateLock } from './db/postgres/agent-store/agentUpdateLock'; import type { Database as PostgresDatabase } from './db/postgres/types'; import type { ISandboxProviderStore } from './db/sandboxProviderStore'; import type { IScheduleStore } from './db/scheduleStore'; @@ -182,8 +185,9 @@ function buildResolveMcpServerStore(options: { function buildResolveAgentStore(options: { persistenceStore: IAgentStore; client: TrueFoundryServiceFoundryServerClient | undefined; + withUpdateLock: WithAgentUpdateLock; }): (c?: Context) => IAgentStore { - const { persistenceStore, client } = options; + const { persistenceStore, client, withUpdateLock } = options; if (!client) { return () => persistenceStore; } @@ -193,6 +197,7 @@ function buildResolveAgentStore(options: { inner: persistenceStore, client, accessToken: requireRequestCredentialToken(c), + withUpdateLock, }) : persistenceStore; } @@ -254,6 +259,7 @@ async function createStandalonePersistence(options: { resolveAgentStore: buildResolveAgentStore({ persistenceStore: agentStore, client: serviceFoundryClient, + withUpdateLock: withoutAgentUpdateLock(), }), withTransaction: callback => db.transaction().execute(callback), tokenStore, @@ -336,6 +342,7 @@ async function createDistributedPersistence(options: { resolveAgentStore: buildResolveAgentStore({ persistenceStore: agentStore, client: serviceFoundryClient, + withUpdateLock: createPostgresAgentUpdateLock(db), }), withTransaction: callback => db.transaction().execute(callback), tokenStore, diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts index dc40b7e6f..e50649dcc 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -8,6 +8,7 @@ import { type ListAgentsInput, type UpdateAgentInput, } from '../db/agentStore'; +import type { WithAgentUpdateLock } from '../db/agentUpdateLock'; import { TrueFoundryServiceFoundryServerClient, type PutRemoteAgentInput, @@ -34,22 +35,25 @@ function toPutRemoteAgentPayload({ /** * create: createDB(null) → putRemote → updateDB(external_id) | on put/update fail → deleteDB (+ deleteRemote if put ok) - * update: putRemote(new) → updateDB | on DB fail → putRemote(old) | both fail → AggregateError + * update: lock → get → putRemote(new) → updateDB | on DB fail → putRemote(old) | both fail → AggregateError * delete: deleteRemote(404 ok) → deleteDB */ export class TrueFoundryAgentStore implements IAgentStore { readonly #inner: IAgentStore; readonly #client: TrueFoundryServiceFoundryServerClient; readonly #accessToken: string; + readonly #withUpdateLock: WithAgentUpdateLock; constructor(input: { inner: IAgentStore; client: TrueFoundryServiceFoundryServerClient; accessToken: string; + withUpdateLock: WithAgentUpdateLock; }) { this.#inner = input.inner; this.#client = input.client; this.#accessToken = input.accessToken; + this.#withUpdateLock = input.withUpdateLock; } listAgents(input: ListAgentsInput, transaction?: TTransaction): Promise { @@ -100,45 +104,51 @@ export class TrueFoundryAgentStore implements IAgentStore< } async updateAgent(input: UpdateAgentInput, transaction?: TTransaction): Promise { - if (input.manifest === undefined) { + const nextManifest = input.manifest; + if (nextManifest === undefined) { + // No manifest means only `external_id` changed; pass through to the inner store. return this.#inner.updateAgent(input, transaction); } - const previous = await this.#inner.getAgent({ tenant_id: input.tenant_id, id: input.id }, transaction); - if (previous === undefined) { - return undefined; - } + // Serialize same-agent updates (incl. SF HTTP) so concurrent MCP writes cannot desync. + return this.#withUpdateLock({ tenant_id: input.tenant_id, id: input.id }, async lockedTransaction => { + const txn = lockedTransaction ?? transaction; + const previous = await this.#inner.getAgent({ tenant_id: input.tenant_id, id: input.id }, txn); + if (previous === undefined) { + return undefined; + } - const { externalId } = await this.#client.putRemoteAgent({ - accessToken: this.#accessToken, - ...toPutRemoteAgentPayload({ name: previous.name, manifest: input.manifest }), - }); + const { externalId } = await this.#client.putRemoteAgent({ + accessToken: this.#accessToken, + ...toPutRemoteAgentPayload({ name: previous.name, manifest: nextManifest }), + }); - try { - return await this.#inner.updateAgent( - { - tenant_id: input.tenant_id, - id: input.id, - manifest: input.manifest, - ...(externalId === previous.external_id ? {} : { external_id: externalId }), - }, - transaction, - ); - } catch (error) { try { - await this.#client.putRemoteAgent({ - accessToken: this.#accessToken, - ...toPutRemoteAgentPayload({ name: previous.name, manifest: previous.manifest }), - }); - } catch (restoreError) { - throw new AggregateError( - [asError(error), asError(restoreError)], - 'updateAgent failed and ServiceFoundry restore also failed', - { cause: restoreError }, + return await this.#inner.updateAgent( + { + tenant_id: input.tenant_id, + id: input.id, + manifest: nextManifest, + ...(externalId === previous.external_id ? {} : { external_id: externalId }), + }, + txn, ); + } catch (error) { + try { + await this.#client.putRemoteAgent({ + accessToken: this.#accessToken, + ...toPutRemoteAgentPayload({ name: previous.name, manifest: previous.manifest }), + }); + } catch (restoreError) { + throw new AggregateError( + [asError(error), asError(restoreError)], + 'updateAgent failed and ServiceFoundry restore also failed', + { cause: restoreError }, + ); + } + throw error; } - throw error; - } + }); } async deleteAgent(input: DeleteAgentInput, transaction?: TTransaction): Promise { diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts index 658c4a600..3d12c29a2 100644 --- a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -2,6 +2,7 @@ import { AgentSpecSchema } from '@truefoundry/trueforge-core/agent-session'; import type { AgentRecord, IAgentStore } from '../../../src/db/agentStore'; import { AgentNameConflictError, AgentNameReservedError, assertAgentNameNotReserved } from '../../../src/db/agentStore'; +import { withoutAgentUpdateLock } from '../../../src/db/agentUpdateLock'; import { TrueFoundryAgentStore } from '../../../src/truefoundry/TrueFoundryAgentStore'; import { TrueFoundryServiceFoundryServerClient, @@ -78,6 +79,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ listAgents, getAgent }), client: mockClient(), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect(store.listAgents({ tenant_id: TENANT })).resolves.toBe(agents); @@ -105,6 +107,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect( @@ -139,6 +142,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent }), client: mockClient({ putRemoteAgent, deleteRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect( @@ -159,6 +163,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect( @@ -181,6 +186,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await store.createAgent({ @@ -205,6 +211,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ putRemoteAgent, deleteRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect( @@ -227,6 +234,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect( @@ -252,6 +260,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect( @@ -274,6 +283,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect(store.updateAgent({ tenant_id: TENANT, id: 'agent-1', external_id: 'sf-agent-1' })).resolves.toBe( @@ -294,6 +304,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect( @@ -303,6 +314,24 @@ describe('TrueFoundryAgentStore', () => { expect(putRemoteAgent).not.toHaveBeenCalled(); }); + it('updateAgent runs under withUpdateLock for manifest updates', async () => { + const previous = record({ external_id: 'sf-1' }); + const updatedManifest = manifest({ instructions: 'Updated.' }); + const updated = record({ manifest: updatedManifest, external_id: 'sf-1' }); + const getAgent = jest.fn(async () => previous); + const updateAgent = jest.fn(async () => updated); + const withUpdateLock = jest.fn(async (_input, fn: (txn: undefined) => Promise) => fn(undefined)); + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, updateAgent }), + client: mockClient(), + accessToken: TOKEN, + withUpdateLock, + }); + + await store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest }); + expect(withUpdateLock).toHaveBeenCalledWith({ tenant_id: TENANT, id: previous.id }, expect.any(Function)); + }); + it('updateAgent puts remote agent then writes manifest when putRemoteAgent returns the same id', async () => { const previous = record({ external_id: 'sf-1' }); const updatedManifest = manifest({ instructions: 'Updated.' }); @@ -314,6 +343,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect(store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest })).resolves.toBe( @@ -343,6 +373,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); const result = await store.updateAgent({ @@ -374,6 +405,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect( @@ -397,6 +429,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect(store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest })).rejects.toThrow( @@ -427,6 +460,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect( @@ -449,6 +483,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await store.deleteAgent({ tenant_id: TENANT, id: previous.id }); @@ -466,6 +501,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await store.deleteAgent({ tenant_id: TENANT, id: previous.id }); @@ -481,6 +517,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await store.deleteAgent({ tenant_id: TENANT, id: 'missing' }); @@ -499,6 +536,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, + withUpdateLock: withoutAgentUpdateLock(), }); await expect(store.deleteAgent({ tenant_id: TENANT, id: previous.id })).rejects.toThrow('sf delete failed'); From 2b721e6e2c4dab12484e2747290aea8219f6faff Mon Sep 17 00:00:00 2001 From: Bhavesh Patel Date: Thu, 3 Sep 2026 16:41:03 +0530 Subject: [PATCH 6/8] Hold agent update lock on delete --- .../20260902194500-truefoundry-agent-store.md | 2 +- packages/trueforge/src/db/agentUpdateLock.ts | 2 +- .../src/truefoundry/TrueFoundryAgentStore.ts | 22 +++++++++++-------- .../truefoundry/TrueFoundryAgentStore.test.ts | 20 +++++++++++++++++ 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/.changeset/20260902194500-truefoundry-agent-store.md b/.changeset/20260902194500-truefoundry-agent-store.md index 4fc9c7cb5..d3a1b75b1 100644 --- a/.changeset/20260902194500-truefoundry-agent-store.md +++ b/.changeset/20260902194500-truefoundry-agent-store.md @@ -2,4 +2,4 @@ "@truefoundry/trueforge": patch --- -Sync ServiceFoundry remote agents on create/update/delete via TrueFoundryAgentStore and store the remote id in `external_id`. Create inserts locally first, then syncs SF (avoids same-name MCP desync). Update takes a Postgres advisory lock per agent id (no-op on SQLite). Filter `listAgents` by `external_ids`. Time out ServiceFoundry HTTP after 10s. +Sync ServiceFoundry remote agents on create/update/delete and store the remote id in `external_id`. Filter `listAgents` by `external_ids`. Time out ServiceFoundry HTTP after 10s. diff --git a/packages/trueforge/src/db/agentUpdateLock.ts b/packages/trueforge/src/db/agentUpdateLock.ts index ff4711d32..b24df22cc 100644 --- a/packages/trueforge/src/db/agentUpdateLock.ts +++ b/packages/trueforge/src/db/agentUpdateLock.ts @@ -1,5 +1,5 @@ /** - * Serializes agent updates that also call ServiceFoundry. + * Serializes agent update/delete that also call ServiceFoundry. * Implementations: Postgres advisory xact lock, or a no-op for SQLite. */ export type WithAgentUpdateLock = ( diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts index e50649dcc..5297feae9 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -36,7 +36,7 @@ function toPutRemoteAgentPayload({ /** * create: createDB(null) → putRemote → updateDB(external_id) | on put/update fail → deleteDB (+ deleteRemote if put ok) * update: lock → get → putRemote(new) → updateDB | on DB fail → putRemote(old) | both fail → AggregateError - * delete: deleteRemote(404 ok) → deleteDB + * delete: lock → get → deleteRemote(404 ok) → deleteDB */ export class TrueFoundryAgentStore implements IAgentStore { readonly #inner: IAgentStore; @@ -152,13 +152,17 @@ export class TrueFoundryAgentStore implements IAgentStore< } async deleteAgent(input: DeleteAgentInput, transaction?: TTransaction): Promise { - const previous = await this.#inner.getAgent({ tenant_id: input.tenant_id, id: input.id }, transaction); - if (previous?.external_id) { - await this.#client.deleteRemoteAgent({ - accessToken: this.#accessToken, - externalId: previous.external_id, - }); - } - await this.#inner.deleteAgent(input, transaction); + // Same lock as update so delete cannot remove the remote between update's get and putRemote. + return this.#withUpdateLock({ tenant_id: input.tenant_id, id: input.id }, async lockedTransaction => { + const txn = lockedTransaction ?? transaction; + const previous = await this.#inner.getAgent({ tenant_id: input.tenant_id, id: input.id }, txn); + if (previous?.external_id) { + await this.#client.deleteRemoteAgent({ + accessToken: this.#accessToken, + externalId: previous.external_id, + }); + } + await this.#inner.deleteAgent(input, txn); + }); } } diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts index 5f4181cf0..ccff0a76d 100644 --- a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -478,6 +478,26 @@ describe('TrueFoundryAgentStore', () => { }); }); + it('deleteAgent runs under withUpdateLock', async () => { + const previous = record({ external_id: 'sf-1' }); + const getAgent = jest.fn(async () => previous); + const deleteAgent = jest.fn(async () => undefined); + const lockCalls: Array<{ tenant_id: string; id: string }> = []; + const withUpdateLock: WithAgentUpdateLock = async (input, fn) => { + lockCalls.push(input); + return fn(undefined); + }; + const store = new TrueFoundryAgentStore({ + inner: mockInner({ getAgent, deleteAgent }), + client: mockClient(), + accessToken: TOKEN, + withUpdateLock, + }); + + await store.deleteAgent({ tenant_id: TENANT, id: previous.id }); + expect(lockCalls).toEqual([{ tenant_id: TENANT, id: previous.id }]); + }); + it('deleteAgent deletes ServiceFoundry then DB when external_id is set', async () => { const previous = record({ external_id: 'sf-1' }); const getAgent = jest.fn(async () => previous); From 92733d4ed1df189b52485dc22231d9a8cfa020e1 Mon Sep 17 00:00:00 2001 From: Bhavesh Patel Date: Thu, 3 Sep 2026 21:12:51 +0530 Subject: [PATCH 7/8] comments addressed --- .../20260902194500-truefoundry-agent-store.md | 2 +- .../20260903013000-reserve-agent-names.md | 2 +- packages/trueforge/.env.example | 4 +++ packages/trueforge/src/apis/agents.ts | 4 --- packages/trueforge/src/config.ts | 14 +++++++++ packages/trueforge/src/db/agentStore.ts | 21 +------------ .../agent-store/PostgresAgentStore.ts | 2 -- .../db/sqlite/agent-store/SqliteAgentStore.ts | 2 -- packages/trueforge/src/main.ts | 4 +++ packages/trueforge/src/schemas/agent.ts | 6 +++- .../src/truefoundry/TrueFoundryAgentStore.ts | 3 +- .../TrueFoundryServiceFoundryServerClient.ts | 25 +++++++++++----- .../tests/db/agentStoreContractSuite.ts | 17 +---------- .../trueforge/tests/unit/apis/agents.test.ts | 7 +++-- .../truefoundry/TrueFoundryAgentStore.test.ts | 30 +++++-------------- 15 files changed, 63 insertions(+), 80 deletions(-) diff --git a/.changeset/20260902194500-truefoundry-agent-store.md b/.changeset/20260902194500-truefoundry-agent-store.md index d3a1b75b1..bbadca89b 100644 --- a/.changeset/20260902194500-truefoundry-agent-store.md +++ b/.changeset/20260902194500-truefoundry-agent-store.md @@ -2,4 +2,4 @@ "@truefoundry/trueforge": patch --- -Sync ServiceFoundry remote agents on create/update/delete and store the remote id in `external_id`. Filter `listAgents` by `external_ids`. Time out ServiceFoundry HTTP after 10s. +Sync ServiceFoundry remote agents on create/update/delete and store the remote id in `external_id`. Filter `listAgents` by `external_ids`. Keep general ServiceFoundry HTTP at 10s and agent CRUD calls at 3s. diff --git a/.changeset/20260903013000-reserve-agent-names.md b/.changeset/20260903013000-reserve-agent-names.md index 94d71770e..a5de3d31b 100644 --- a/.changeset/20260903013000-reserve-agent-names.md +++ b/.changeset/20260903013000-reserve-agent-names.md @@ -2,4 +2,4 @@ "@truefoundry/trueforge": patch --- -Reserve agent names `tfg` and `trueforge` on create in all agent store modes. +Reject reserved agent names `tfg` and `trueforge` in create requests. diff --git a/packages/trueforge/.env.example b/packages/trueforge/.env.example index 8aaa335d9..84130726a 100644 --- a/packages/trueforge/.env.example +++ b/packages/trueforge/.env.example @@ -27,6 +27,10 @@ PORT=8790 ## ServiceFoundry server (caller token) and turns call the tenant's default AI Gateway. Unset = local ## API-key model catalog. Mutually exclusive with OIDC. # TRUEFOUNDRY_SERVICEFOUNDRY_SERVER_URL=https://servicefoundry-server.truefoundry.svc.cluster.local +## Max ms for non-agent ServiceFoundry HTTP calls. Default 10000. +# TRUEFOUNDRY_SERVICEFOUNDRY_HTTP_TIMEOUT_MS=10000 +## Max ms for ServiceFoundry agent create/update/delete calls. Default 3000. +# TRUEFOUNDRY_SERVICEFOUNDRY_HTTP_AGENT_TIMEOUT_MS=3000 ## Internal mutual TLS for calls to the ServiceFoundry server. When true, trueforge presents its client ## certificate (and upgrades a mesh-direct http:// peer URL to https://). Only meaningful in-cluster, diff --git a/packages/trueforge/src/apis/agents.ts b/packages/trueforge/src/apis/agents.ts index 837ee0d97..1be276d4c 100644 --- a/packages/trueforge/src/apis/agents.ts +++ b/packages/trueforge/src/apis/agents.ts @@ -8,7 +8,6 @@ import type { ResolveRequestContext } from '../auth/identity'; import { AgentExternalIdConflictError, AgentNameConflictError, - AgentNameReservedError, type AgentRecord, type IAgentStore, } from '../db/agentStore'; @@ -98,9 +97,6 @@ export function createAgentsRouter(deps: AgentsRouterDeps { listAgents(input: ListAgentsInput, transaction?: TTransaction): Promise; getAgent(input: GetAgentInput, transaction?: TTransaction): Promise; - /** Inserts a new agent with a generated ULID. Throws AgentNameConflictError, AgentExternalIdConflictError, or AgentNameReservedError. */ + /** Inserts a new agent with a generated ULID. Throws AgentNameConflictError or AgentExternalIdConflictError on unique clash. */ createAgent(input: CreateAgentInput, transaction?: TTransaction): Promise; /** Patches `manifest` and/or `external_id`. Throws AgentExternalIdConflictError on unique clash. Returns undefined if missing. */ updateAgent(input: UpdateAgentInput, transaction?: TTransaction): Promise; diff --git a/packages/trueforge/src/db/postgres/agent-store/PostgresAgentStore.ts b/packages/trueforge/src/db/postgres/agent-store/PostgresAgentStore.ts index 9d1e59501..32d9206f1 100644 --- a/packages/trueforge/src/db/postgres/agent-store/PostgresAgentStore.ts +++ b/packages/trueforge/src/db/postgres/agent-store/PostgresAgentStore.ts @@ -3,7 +3,6 @@ import { newId } from '../../../utils/id'; import { AgentExternalIdConflictError, AgentNameConflictError, - assertAgentNameNotReserved, parseStoredAgentSpec, type AgentRecord, type CreateAgentInput, @@ -81,7 +80,6 @@ export class PostgresAgentStore implements IAgentStore> { } async createAgent(input: CreateAgentInput, transaction?: Transaction): Promise { - assertAgentNameNotReserved(input.name); const db = transaction ?? this.#db; try { const row = await db diff --git a/packages/trueforge/src/db/sqlite/agent-store/SqliteAgentStore.ts b/packages/trueforge/src/db/sqlite/agent-store/SqliteAgentStore.ts index 9f88825f9..4c4316629 100644 --- a/packages/trueforge/src/db/sqlite/agent-store/SqliteAgentStore.ts +++ b/packages/trueforge/src/db/sqlite/agent-store/SqliteAgentStore.ts @@ -4,7 +4,6 @@ import { newId } from '../../../utils/id'; import { AgentExternalIdConflictError, AgentNameConflictError, - assertAgentNameNotReserved, parseStoredAgentSpec, type AgentRecord, type CreateAgentInput, @@ -76,7 +75,6 @@ export class SqliteAgentStore implements IAgentStore> { } async createAgent(input: CreateAgentInput, transaction?: Transaction): Promise { - assertAgentNameNotReserved(input.name); const db = transaction ?? this.#db; const timestamp = nowIso(); try { diff --git a/packages/trueforge/src/main.ts b/packages/trueforge/src/main.ts index 53ba41909..cff79702c 100644 --- a/packages/trueforge/src/main.ts +++ b/packages/trueforge/src/main.ts @@ -129,6 +129,8 @@ function createServiceFoundryServerClient(logger: Logger): TrueFoundryServiceFou return new TrueFoundryServiceFoundryServerClient({ serviceFoundryServerUrl: configuration.TRUEFOUNDRY_SERVICEFOUNDRY_SERVER_URL, logger, + httpTimeoutMs: configuration.TRUEFOUNDRY_SERVICEFOUNDRY_HTTP_TIMEOUT_MS, + httpAgentTimeoutMs: configuration.TRUEFOUNDRY_SERVICEFOUNDRY_HTTP_AGENT_TIMEOUT_MS, tls: { enabled: configuration.TRUEFOUNDRY_MTLS_ENABLED, dir: configuration.TRUEFOUNDRY_MTLS_CERTS_DIR }, }); } @@ -402,6 +404,8 @@ async function createServerRuntime(persistence: ServerPersistence< trueFoundryClient: new TrueFoundryServiceFoundryServerClient({ serviceFoundryServerUrl: configuration.TRUEFOUNDRY_SERVICEFOUNDRY_SERVER_URL, logger, + httpTimeoutMs: configuration.TRUEFOUNDRY_SERVICEFOUNDRY_HTTP_TIMEOUT_MS, + httpAgentTimeoutMs: configuration.TRUEFOUNDRY_SERVICEFOUNDRY_HTTP_AGENT_TIMEOUT_MS, tls: { enabled: configuration.TRUEFOUNDRY_MTLS_ENABLED, dir: configuration.TRUEFOUNDRY_MTLS_CERTS_DIR }, }), }); diff --git a/packages/trueforge/src/schemas/agent.ts b/packages/trueforge/src/schemas/agent.ts index 0c484c67f..447973c6f 100644 --- a/packages/trueforge/src/schemas/agent.ts +++ b/packages/trueforge/src/schemas/agent.ts @@ -6,10 +6,14 @@ import { z } from '@hono/zod-openapi'; import { AgentSpecSchema } from '@truefoundry/trueforge-core/agent-session'; import { NameSchema } from './common'; +const RESERVED_AGENT_NAMES = new Set(['tfg', 'trueforge']); + /** Create body: unique immutable `name` plus manifest. `id` is never client-supplied. */ export const CreateAgentRequestSchema = z .object({ - name: NameSchema, + name: NameSchema.refine(name => !RESERVED_AGENT_NAMES.has(name), { + message: 'Agent name is reserved, cannot be used', + }), manifest: AgentSpecSchema, }) .strict() diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts index 5297feae9..c9e80b39c 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -65,7 +65,8 @@ export class TrueFoundryAgentStore implements IAgentStore< } async createAgent(input: CreateAgentInput, transaction?: TTransaction): Promise { - // Insert first so unique name picks the winner; only the winner calls SF (avoids MCP desync). + // Lets the DB unique constraint pick one winner for this tenant ID and name. + // Prevents concurrent requests from both creating the same remote agent. const created = await this.#inner.createAgent({ ...input, external_id: null }, transaction); let externalId: string | undefined; diff --git a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts index 404a32654..023eb2a09 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts @@ -14,9 +14,6 @@ const SESSION_PATH = 'v1/session'; const INTEGRATIONS_PAGE_SIZE = 1000; const MCP_SERVERS_PAGE_SIZE = 100; -/** Per-request timeout for ServiceFoundry HTTP (list/get/put/delete/session). */ -export const SERVICE_FOUNDRY_HTTP_TIMEOUT_MS = 10_000; - /** * Fields required to build RequestContext from ServiceFoundry `GET /v1/session`. * Wire shape is camelCase (Nest Session + exposed `subject()`). @@ -101,15 +98,25 @@ export class TrueFoundryServiceFoundryServerClient { readonly #baseUrl: string; readonly #logger: Logger | undefined; readonly #dispatcher: Dispatcher | undefined; + readonly #httpTimeoutMs: number; + readonly #httpAgentTimeoutMs: number; - constructor(input: { serviceFoundryServerUrl: string; logger?: Logger; tls?: InternalTlsOptions }) { - const tls = input.tls ?? { enabled: false, dir: '' }; + constructor(input: { + serviceFoundryServerUrl: string; + logger: Logger; + tls: InternalTlsOptions; + httpTimeoutMs: number; + httpAgentTimeoutMs: number; + }) { + const tls = input.tls; this.#baseUrl = normalizeInternalTlsUrl({ url: input.serviceFoundryServerUrl, enabled: tls.enabled }).replace( /\/+$/, '', ); this.#dispatcher = createInternalTlsDispatcher(tls); this.#logger = input.logger; + this.#httpTimeoutMs = input.httpTimeoutMs; + this.#httpAgentTimeoutMs = input.httpAgentTimeoutMs; } async listProviderIntegrations(accessToken: string): Promise { @@ -197,6 +204,7 @@ export class TrueFoundryServiceFoundryServerClient { url: this.#url(TFG_AGENTS_PATH), accessToken: input.accessToken, method: 'PUT', + timeoutMs: this.#httpAgentTimeoutMs, body: { name: input.name, description: input.description, @@ -223,6 +231,7 @@ export class TrueFoundryServiceFoundryServerClient { url: this.#url(`${TFG_AGENTS_PATH}/${encodeURIComponent(input.externalId)}`), accessToken: input.accessToken, method: 'DELETE', + timeoutMs: this.#httpAgentTimeoutMs, notFoundOk: true, }); } @@ -290,11 +299,13 @@ export class TrueFoundryServiceFoundryServerClient { accessToken: string; method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE'; body?: unknown; + timeoutMs?: number; /** Treat HTTP 404 as success (idempotent DELETE). */ notFoundOk?: boolean; }): Promise { const startedAt = Date.now(); - const signal = AbortSignal.timeout(SERVICE_FOUNDRY_HTTP_TIMEOUT_MS); + const timeoutMs = input.timeoutMs ?? this.#httpTimeoutMs; + const signal = AbortSignal.timeout(timeoutMs); try { const response = await undiciFetch(input.url, { method: input.method, @@ -356,7 +367,7 @@ export class TrueFoundryServiceFoundryServerClient { }); throw new HTTPException(500, { message: timedOut - ? `TrueFoundry ServiceFoundry server request timed out after ${String(SERVICE_FOUNDRY_HTTP_TIMEOUT_MS / 1000)}s` + ? `TrueFoundry ServiceFoundry server request timed out after ${String(timeoutMs / 1000)}s` : 'TrueFoundry ServiceFoundry server request failed', cause: error, }); diff --git a/packages/trueforge/tests/db/agentStoreContractSuite.ts b/packages/trueforge/tests/db/agentStoreContractSuite.ts index 7464c375c..24c90f05c 100644 --- a/packages/trueforge/tests/db/agentStoreContractSuite.ts +++ b/packages/trueforge/tests/db/agentStoreContractSuite.ts @@ -3,12 +3,7 @@ * Runs under jest against a fresh store per test (see backend test files). */ import { AgentSpecSchema, type AgentSpec } from '@truefoundry/trueforge-core/agent-session'; -import { - AgentExternalIdConflictError, - AgentNameConflictError, - AgentNameReservedError, - type IAgentStore, -} from '../../src/db/agentStore'; +import { AgentExternalIdConflictError, AgentNameConflictError, type IAgentStore } from '../../src/db/agentStore'; const TENANT = 'default'; @@ -116,16 +111,6 @@ export function runAgentStoreContractSuite(getStore: () => IAgentStore): void { ).rejects.toBeInstanceOf(AgentNameConflictError); }); - it('createAgent throws AgentNameReservedError for tfg and trueforge', async () => { - const store = getStore(); - await expect( - store.createAgent({ tenant_id: TENANT, name: 'tfg', manifest: manifest(), external_id: null }), - ).rejects.toBeInstanceOf(AgentNameReservedError); - await expect( - store.createAgent({ tenant_id: TENANT, name: 'trueforge', manifest: manifest(), external_id: null }), - ).rejects.toBeInstanceOf(AgentNameReservedError); - }); - it('listAgents returns only the tenant, ordered by name', async () => { const store = getStore(); await store.createAgent({ diff --git a/packages/trueforge/tests/unit/apis/agents.test.ts b/packages/trueforge/tests/unit/apis/agents.test.ts index 9d346eb5d..2f5475632 100644 --- a/packages/trueforge/tests/unit/apis/agents.test.ts +++ b/packages/trueforge/tests/unit/apis/agents.test.ts @@ -170,8 +170,11 @@ describe('agents router', () => { const badName = await router.request('/', jsonInit('POST', { ...writeBody, name: 'Not A Name' })); expect(badName.status).toBe(400); - const reserved = await router.request('/', jsonInit('POST', { ...writeBody, name: 'tfg' })); - expect(reserved.status).toBe(400); + const reservedTfg = await router.request('/', jsonInit('POST', { ...writeBody, name: 'tfg' })); + expect(reservedTfg.status).toBe(400); + + const reservedTrueforge = await router.request('/', jsonInit('POST', { ...writeBody, name: 'trueforge' })); + expect(reservedTrueforge.status).toBe(400); const unknownModel = await router.request( '/', diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts index ccff0a76d..61a721f87 100644 --- a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -1,7 +1,8 @@ import { AgentSpecSchema } from '@truefoundry/trueforge-core/agent-session'; +import { createLogger } from 'winston'; import type { AgentRecord, IAgentStore } from '../../../src/db/agentStore'; -import { AgentNameConflictError, AgentNameReservedError, assertAgentNameNotReserved } from '../../../src/db/agentStore'; +import { AgentNameConflictError } from '../../../src/db/agentStore'; import { withoutAgentUpdateLock, type WithAgentUpdateLock } from '../../../src/db/agentUpdateLock'; import { TrueFoundryAgentStore } from '../../../src/truefoundry/TrueFoundryAgentStore'; import { @@ -13,6 +14,7 @@ import { const TENANT = 'default'; const TOKEN = 'test-token'; +const LOGGER = createLogger({ silent: true }); function manifest(overrides: { instructions?: string; mcp_servers?: { name: string }[] } = {}) { return AgentSpecSchema.parse({ @@ -55,6 +57,10 @@ function mockClient( ): TrueFoundryServiceFoundryServerClient { const client = new TrueFoundryServiceFoundryServerClient({ serviceFoundryServerUrl: 'http://servicefoundry.test', + logger: LOGGER, + tls: { enabled: false, dir: '' }, + httpTimeoutMs: 10_000, + httpAgentTimeoutMs: 3_000, }); client.putRemoteAgent = overrides.putRemoteAgent ?? (async (): Promise => ({ externalId: 'sf-1' })); @@ -153,28 +159,6 @@ describe('TrueFoundryAgentStore', () => { expect(deleteRemoteAgent).not.toHaveBeenCalled(); }); - it('createAgent rejects reserved names before calling ServiceFoundry', async () => { - const createAgent = jest.fn(async (input: { name: string }) => { - assertAgentNameNotReserved(input.name); - return record(); - }); - const putRemoteAgent = jest.fn(); - const store = new TrueFoundryAgentStore({ - inner: mockInner({ createAgent }), - client: mockClient({ putRemoteAgent }), - accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), - }); - - await expect( - store.createAgent({ tenant_id: TENANT, name: 'tfg', manifest: manifest(), external_id: null }), - ).rejects.toBeInstanceOf(AgentNameReservedError); - await expect( - store.createAgent({ tenant_id: TENANT, name: 'trueforge', manifest: manifest(), external_id: null }), - ).rejects.toBeInstanceOf(AgentNameReservedError); - expect(putRemoteAgent).not.toHaveBeenCalled(); - }); - it('createAgent uses agent name as description when instructions are omitted', async () => { const putRemoteAgent = jest.fn(async (input: PutRemoteAgentInput) => { expect(input.description).toBe('research'); From 33e9d6da504798adce9121bb4b1ab2cb792a4075 Mon Sep 17 00:00:00 2001 From: Bhavesh Patel Date: Thu, 3 Sep 2026 22:13:27 +0530 Subject: [PATCH 8/8] Take the agent update lock in TrueFoundryAgentStore. Do not pass it in from main. TrueFoundry mode cannot run with STANDALONE=true. --- .../20260902194500-truefoundry-agent-store.md | 2 +- packages/trueforge/src/config.ts | 4 + packages/trueforge/src/db/agentUpdateLock.ts | 12 -- .../postgres/agent-store/agentUpdateLock.ts | 15 -- packages/trueforge/src/main.ts | 44 ++--- .../src/truefoundry/TrueFoundryAgentStore.ts | 40 ++-- .../truefoundry/TrueFoundryAgentStore.test.ts | 177 +++++++++--------- 7 files changed, 133 insertions(+), 161 deletions(-) delete mode 100644 packages/trueforge/src/db/agentUpdateLock.ts delete mode 100644 packages/trueforge/src/db/postgres/agent-store/agentUpdateLock.ts diff --git a/.changeset/20260902194500-truefoundry-agent-store.md b/.changeset/20260902194500-truefoundry-agent-store.md index bbadca89b..916f127c5 100644 --- a/.changeset/20260902194500-truefoundry-agent-store.md +++ b/.changeset/20260902194500-truefoundry-agent-store.md @@ -2,4 +2,4 @@ "@truefoundry/trueforge": patch --- -Sync ServiceFoundry remote agents on create/update/delete and store the remote id in `external_id`. Filter `listAgents` by `external_ids`. Keep general ServiceFoundry HTTP at 10s and agent CRUD calls at 3s. +Sync ServiceFoundry remote agents on create/update/delete and store the remote id in `external_id`. Filter `listAgents` by `external_ids`. Keep general ServiceFoundry HTTP at 10s and agent CRUD calls at 3s. Require `STANDALONE=false` when `TRUEFOUNDRY_SERVICEFOUNDRY_SERVER_URL` is set. diff --git a/packages/trueforge/src/config.ts b/packages/trueforge/src/config.ts index 7544fedb6..583ecdbfb 100644 --- a/packages/trueforge/src/config.ts +++ b/packages/trueforge/src/config.ts @@ -728,6 +728,10 @@ if (isTrueFoundryModeEnabled(configuration) && isOidcConfigured(configuration)) ); } +if (isTrueFoundryModeEnabled(configuration) && configuration.STANDALONE) { + throw new Error('TRUEFOUNDRY_SERVICEFOUNDRY_SERVER_URL requires STANDALONE=false.'); +} + /** * Public origin for OAuth callbacks. * Standalone (non-development) → `http://localhost:$PORT`; otherwise `PUBLIC_BASE_URL` diff --git a/packages/trueforge/src/db/agentUpdateLock.ts b/packages/trueforge/src/db/agentUpdateLock.ts deleted file mode 100644 index b24df22cc..000000000 --- a/packages/trueforge/src/db/agentUpdateLock.ts +++ /dev/null @@ -1,12 +0,0 @@ -/** - * Serializes agent update/delete that also call ServiceFoundry. - * Implementations: Postgres advisory xact lock, or a no-op for SQLite. - */ -export type WithAgentUpdateLock = ( - input: { tenant_id: string; id: string }, - fn: (transaction: TTransaction | undefined) => Promise, -) => Promise; - -export function withoutAgentUpdateLock(): WithAgentUpdateLock { - return async (_input, fn) => fn(undefined); -} diff --git a/packages/trueforge/src/db/postgres/agent-store/agentUpdateLock.ts b/packages/trueforge/src/db/postgres/agent-store/agentUpdateLock.ts deleted file mode 100644 index 350efa6bb..000000000 --- a/packages/trueforge/src/db/postgres/agent-store/agentUpdateLock.ts +++ /dev/null @@ -1,15 +0,0 @@ -import type { Kysely, Transaction } from 'kysely'; -import { sql } from 'kysely'; - -import type { WithAgentUpdateLock } from '../../agentUpdateLock'; -import type { Database } from '../types'; - -/** Transaction-scoped advisory lock held for the whole callback (including SF HTTP). */ -export function createPostgresAgentUpdateLock(db: Kysely): WithAgentUpdateLock> { - return async (input, fn) => - db.transaction().execute(async trx => { - const key = `tf:agent:${input.tenant_id}:${input.id}`; - await sql`SELECT pg_advisory_xact_lock(hashtext(${key}))`.execute(trx); - return fn(trx); - }); -} diff --git a/packages/trueforge/src/main.ts b/packages/trueforge/src/main.ts index a1233ee1b..b3ae0ae67 100644 --- a/packages/trueforge/src/main.ts +++ b/packages/trueforge/src/main.ts @@ -66,12 +66,10 @@ import { SkillCatalog } from './catalog/SkillCatalog'; import { type DistributedServerConfiguration } from './config'; import { createController } from './controller'; import type { IAgentStore } from './db/agentStore'; -import type { WithAgentUpdateLock } from './db/agentUpdateLock'; -import { withoutAgentUpdateLock } from './db/agentUpdateLock'; import type { IMcpServerStore, IMcpServerWithAuthStore } from './db/mcpServerStore'; import { McpServerWithAuthStore } from './db/McpServerWithAuthStore'; import type { IModelProviderStore } from './db/modelProviderStore'; -import { createPostgresAgentUpdateLock } from './db/postgres/agent-store/agentUpdateLock'; +import type { PostgresAgentStore } from './db/postgres/agent-store/PostgresAgentStore'; import type { Database as PostgresDatabase } from './db/postgres/types'; import type { ISandboxProviderStore } from './db/sandboxProviderStore'; import type { IScheduleStore } from './db/scheduleStore'; @@ -194,22 +192,17 @@ function buildResolveMcpServerStore(options: { * Per-request agent store resolver. In TrueFoundry mode every request gets a token-bound * decorator over DB persistence; otherwise the persistence store is reused as-is. */ -function buildResolveAgentStore(options: { - persistenceStore: IAgentStore; - client: TrueFoundryServiceFoundryServerClient | undefined; - withUpdateLock: WithAgentUpdateLock; -}): (c?: Context) => IAgentStore { - const { persistenceStore, client, withUpdateLock } = options; - if (!client) { - return () => persistenceStore; - } +function buildResolveAgentStore(options: { + persistenceStore: PostgresAgentStore; + client: TrueFoundryServiceFoundryServerClient; +}): (c?: Context) => IAgentStore> { + const { persistenceStore, client } = options; return c => c - ? new TrueFoundryAgentStore({ + ? new TrueFoundryAgentStore({ inner: persistenceStore, client, accessToken: requireRequestCredentialToken(c), - withUpdateLock, }) : persistenceStore; } @@ -255,24 +248,19 @@ async function createStandalonePersistence(options: { const tokenStore = new SqliteOAuthTokenStore(db); const agentStore = new SqliteAgentStore(db); - const serviceFoundryClient = createServiceFoundryServerClient(logger); return { sessionStore: new SqliteSessionStore(db), sessionMetricsStore: new SqliteSessionMetricsStore(db), resolveModelProviderStore: buildResolveModelProviderStore({ persistenceStore: new SqliteModelProviderStore(db), - client: serviceFoundryClient, + client: undefined, }), resolveMcpServerStore: buildResolveMcpServerStore({ persistenceStore: new SqliteMcpServerStore(db), tokenStore, - client: serviceFoundryClient, - }), - resolveAgentStore: buildResolveAgentStore({ - persistenceStore: agentStore, - client: serviceFoundryClient, - withUpdateLock: withoutAgentUpdateLock(), + client: undefined, }), + resolveAgentStore: () => agentStore, withTransaction: callback => db.transaction().execute(callback), tokenStore, skillStore: new SqliteSkillStore(db), @@ -351,11 +339,13 @@ async function createDistributedPersistence(options: { tokenStore, client: serviceFoundryClient, }), - resolveAgentStore: buildResolveAgentStore({ - persistenceStore: agentStore, - client: serviceFoundryClient, - withUpdateLock: createPostgresAgentUpdateLock(db), - }), + resolveAgentStore: + serviceFoundryClient === undefined + ? () => agentStore + : buildResolveAgentStore({ + persistenceStore: agentStore, + client: serviceFoundryClient, + }), withTransaction: callback => db.transaction().execute(callback), tokenStore, skillStore: new PostgresSkillStore(db), diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts index c9e80b39c..6a8084243 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -1,4 +1,5 @@ import type { AgentSpec } from '@truefoundry/trueforge-core/agent-session'; +import { sql, type Transaction } from 'kysely'; import { type AgentRecord, type CreateAgentInput, @@ -8,7 +9,7 @@ import { type ListAgentsInput, type UpdateAgentInput, } from '../db/agentStore'; -import type { WithAgentUpdateLock } from '../db/agentUpdateLock'; +import type { Database } from '../db/postgres/types'; import { TrueFoundryServiceFoundryServerClient, type PutRemoteAgentInput, @@ -38,33 +39,44 @@ function toPutRemoteAgentPayload({ * update: lock → get → putRemote(new) → updateDB | on DB fail → putRemote(old) | both fail → AggregateError * delete: lock → get → deleteRemote(404 ok) → deleteDB */ -export class TrueFoundryAgentStore implements IAgentStore { +export class TrueFoundryAgentStore< + TTransaction extends Transaction = Transaction, +> implements IAgentStore { readonly #inner: IAgentStore; readonly #client: TrueFoundryServiceFoundryServerClient; readonly #accessToken: string; - readonly #withUpdateLock: WithAgentUpdateLock; constructor(input: { inner: IAgentStore; client: TrueFoundryServiceFoundryServerClient; accessToken: string; - withUpdateLock: WithAgentUpdateLock; }) { this.#inner = input.inner; this.#client = input.client; this.#accessToken = input.accessToken; - this.#withUpdateLock = input.withUpdateLock; } - listAgents(input: ListAgentsInput, transaction?: TTransaction): Promise { + listAgents(input: ListAgentsInput, transaction: TTransaction): Promise { return this.#inner.listAgents(input, transaction); } - getAgent(input: GetAgentInput, transaction?: TTransaction): Promise { + getAgent(input: GetAgentInput, transaction: TTransaction): Promise { return this.#inner.getAgent(input, transaction); } - async createAgent(input: CreateAgentInput, transaction?: TTransaction): Promise { + // Serializes updates for this tenant ID and agent id. + // Prevents concurrent MCP writes from desyncing the remote agent. + async #withUpdateLock( + input: { tenant_id: string; id: string }, + transaction: TTransaction, + fn: (transaction: TTransaction) => Promise, + ): Promise { + const key = `tf:agent:${input.tenant_id}:${input.id}`; + await sql`SELECT pg_advisory_xact_lock(hashtext(${key}))`.execute(transaction); + return fn(transaction); + } + + async createAgent(input: CreateAgentInput, transaction: TTransaction): Promise { // Lets the DB unique constraint pick one winner for this tenant ID and name. // Prevents concurrent requests from both creating the same remote agent. const created = await this.#inner.createAgent({ ...input, external_id: null }, transaction); @@ -104,16 +116,14 @@ export class TrueFoundryAgentStore implements IAgentStore< } } - async updateAgent(input: UpdateAgentInput, transaction?: TTransaction): Promise { + async updateAgent(input: UpdateAgentInput, transaction: TTransaction): Promise { const nextManifest = input.manifest; if (nextManifest === undefined) { // No manifest means only `external_id` changed; pass through to the inner store. return this.#inner.updateAgent(input, transaction); } - // Serialize same-agent updates (incl. SF HTTP) so concurrent MCP writes cannot desync. - return this.#withUpdateLock({ tenant_id: input.tenant_id, id: input.id }, async lockedTransaction => { - const txn = lockedTransaction ?? transaction; + return this.#withUpdateLock(input, transaction, async txn => { const previous = await this.#inner.getAgent({ tenant_id: input.tenant_id, id: input.id }, txn); if (previous === undefined) { return undefined; @@ -152,10 +162,8 @@ export class TrueFoundryAgentStore implements IAgentStore< }); } - async deleteAgent(input: DeleteAgentInput, transaction?: TTransaction): Promise { - // Same lock as update so delete cannot remove the remote between update's get and putRemote. - return this.#withUpdateLock({ tenant_id: input.tenant_id, id: input.id }, async lockedTransaction => { - const txn = lockedTransaction ?? transaction; + async deleteAgent(input: DeleteAgentInput, transaction: TTransaction): Promise { + return this.#withUpdateLock(input, transaction, async txn => { const previous = await this.#inner.getAgent({ tenant_id: input.tenant_id, id: input.id }, txn); if (previous?.external_id) { await this.#client.deleteRemoteAgent({ diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts index 61a721f87..78ce5ae60 100644 --- a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -1,9 +1,8 @@ import { AgentSpecSchema } from '@truefoundry/trueforge-core/agent-session'; import { createLogger } from 'winston'; -import type { AgentRecord, IAgentStore } from '../../../src/db/agentStore'; +import type { AgentRecord } from '../../../src/db/agentStore'; import { AgentNameConflictError } from '../../../src/db/agentStore'; -import { withoutAgentUpdateLock, type WithAgentUpdateLock } from '../../../src/db/agentUpdateLock'; import { TrueFoundryAgentStore } from '../../../src/truefoundry/TrueFoundryAgentStore'; import { TrueFoundryServiceFoundryServerClient, @@ -16,6 +15,28 @@ const TENANT = 'default'; const TOKEN = 'test-token'; const LOGGER = createLogger({ silent: true }); +function mockTransaction() { + const executor = { + transformQuery(node: unknown) { + return node; + }, + compileQuery() { + return { sql: 'select 1', parameters: [] }; + }, + executeQuery: jest.fn(async () => ({ rows: [] })), + withPlugins() { + return executor; + }, + }; + return { + getExecutor() { + return executor; + }, + }; +} + +const TXN = mockTransaction(); + function manifest(overrides: { instructions?: string; mcp_servers?: { name: string }[] } = {}) { return AgentSpecSchema.parse({ model: { name: 'openai-gateway/gpt-5' }, @@ -38,7 +59,7 @@ function record(overrides: Partial = {}): AgentRecord { }; } -function mockInner(overrides: Partial = {}): IAgentStore { +function mockInner(overrides = {}) { return { listAgents: jest.fn(), getAgent: jest.fn(), @@ -85,13 +106,12 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ listAgents, getAgent }), client: mockClient(), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - await expect(store.listAgents({ tenant_id: TENANT })).resolves.toBe(agents); - await expect(store.getAgent({ tenant_id: TENANT, id: 'agent-1' })).resolves.toBe(agents[0]); - expect(listAgents).toHaveBeenCalledWith({ tenant_id: TENANT }, undefined); - expect(getAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'agent-1' }, undefined); + await expect(store.listAgents({ tenant_id: TENANT }, TXN)).resolves.toBe(agents); + await expect(store.getAgent({ tenant_id: TENANT, id: 'agent-1' }, TXN)).resolves.toBe(agents[0]); + expect(listAgents).toHaveBeenCalledWith({ tenant_id: TENANT }, TXN); + expect(getAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'agent-1' }, TXN); }); it('createAgent inserts locally, puts remote, then sets external_id', async () => { @@ -113,16 +133,18 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); await expect( - store.createAgent({ - tenant_id: TENANT, - name: 'research', - manifest: manifest({ mcp_servers: [{ name: 'slack' }] }), - external_id: null, - }), + store.createAgent( + { + tenant_id: TENANT, + name: 'research', + manifest: manifest({ mcp_servers: [{ name: 'slack' }] }), + external_id: null, + }, + TXN, + ), ).resolves.toBe(linked); expect(createAgent).toHaveBeenCalledWith( expect.objectContaining({ @@ -130,9 +152,9 @@ describe('TrueFoundryAgentStore', () => { name: 'research', external_id: null, }), - undefined, + TXN, ); - expect(updateAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: local.id, external_id: 'sf-1' }, undefined); + expect(updateAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: local.id, external_id: 'sf-1' }, TXN); expect(firstInvocationOrder(createAgent)).toBeLessThan(firstInvocationOrder(putRemoteAgent)); expect(firstInvocationOrder(putRemoteAgent)).toBeLessThan(firstInvocationOrder(updateAgent)); }); @@ -148,11 +170,10 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent }), client: mockClient({ putRemoteAgent, deleteRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); await expect( - store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), + store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }, TXN), ).rejects.toBeInstanceOf(AgentNameConflictError); expect(putRemoteAgent).not.toHaveBeenCalled(); expect(updateAgent).not.toHaveBeenCalled(); @@ -170,15 +191,17 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - await store.createAgent({ - tenant_id: TENANT, - name: 'research', - manifest: AgentSpecSchema.parse({ model: { name: 'openai-gateway/gpt-5' } }), - external_id: null, - }); + await store.createAgent( + { + tenant_id: TENANT, + name: 'research', + manifest: AgentSpecSchema.parse({ model: { name: 'openai-gateway/gpt-5' } }), + external_id: null, + }, + TXN, + ); expect(putRemoteAgent).toHaveBeenCalled(); }); @@ -195,15 +218,14 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ putRemoteAgent, deleteRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); await expect( - store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), + store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }, TXN), ).rejects.toThrow('sf failed'); expect(updateAgent).not.toHaveBeenCalled(); expect(deleteRemoteAgent).not.toHaveBeenCalled(); - expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: local.id }, undefined); + expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: local.id }, TXN); }); it('createAgent rolls back remote and local when updateAgent fails', async () => { @@ -218,14 +240,13 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); await expect( - store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), + store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }, TXN), ).rejects.toThrow('db update failed'); expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, externalId: 'sf-1' }); - expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: local.id }, undefined); + expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: local.id }, TXN); }); it('createAgent still throws when cleanup fails', async () => { @@ -244,11 +265,10 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); await expect( - store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }), + store.createAgent({ tenant_id: TENANT, name: 'research', manifest: manifest(), external_id: null }, TXN), ).rejects.toMatchObject({ message: 'createAgent failed and cleanup also failed', errors: [ @@ -267,17 +287,13 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - await expect(store.updateAgent({ tenant_id: TENANT, id: 'agent-1', external_id: 'sf-agent-1' })).resolves.toBe( + await expect(store.updateAgent({ tenant_id: TENANT, id: 'agent-1', external_id: 'sf-agent-1' }, TXN)).resolves.toBe( updated, ); expect(putRemoteAgent).not.toHaveBeenCalled(); - expect(updateAgent).toHaveBeenCalledWith( - { tenant_id: TENANT, id: 'agent-1', external_id: 'sf-agent-1' }, - undefined, - ); + expect(updateAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'agent-1', external_id: 'sf-agent-1' }, TXN); }); it('updateAgent returns undefined for a missing agent without calling putRemoteAgent', async () => { @@ -288,36 +304,29 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); await expect( - store.updateAgent({ tenant_id: TENANT, id: 'missing', manifest: manifest({ instructions: 'Updated.' }) }), + store.updateAgent({ tenant_id: TENANT, id: 'missing', manifest: manifest({ instructions: 'Updated.' }) }, TXN), ).resolves.toBeUndefined(); expect(updateAgent).not.toHaveBeenCalled(); expect(putRemoteAgent).not.toHaveBeenCalled(); }); - it('updateAgent runs under withUpdateLock for manifest updates', async () => { + it('updateAgent reads the agent inside the transaction for manifest updates', async () => { const previous = record({ external_id: 'sf-1' }); const updatedManifest = manifest({ instructions: 'Updated.' }); const updated = record({ manifest: updatedManifest, external_id: 'sf-1' }); const getAgent = jest.fn(async () => previous); const updateAgent = jest.fn(async () => updated); - const lockCalls: Array<{ tenant_id: string; id: string }> = []; - const withUpdateLock: WithAgentUpdateLock = async (input, fn) => { - lockCalls.push(input); - return fn(undefined); - }; const store = new TrueFoundryAgentStore({ inner: mockInner({ getAgent, updateAgent }), client: mockClient(), accessToken: TOKEN, - withUpdateLock, }); - await store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest }); - expect(lockCalls).toEqual([{ tenant_id: TENANT, id: previous.id }]); + await store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest }, TXN); + expect(getAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: previous.id }, TXN); }); it('updateAgent puts remote agent then writes manifest when putRemoteAgent returns the same id', async () => { @@ -331,12 +340,11 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - await expect(store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest })).resolves.toBe( - updated, - ); + await expect( + store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest }, TXN), + ).resolves.toBe(updated); expect(putRemoteAgent).toHaveBeenCalledTimes(1); expect(updateAgent).toHaveBeenCalledTimes(1); expect(updateAgent).toHaveBeenCalledWith( @@ -345,7 +353,7 @@ describe('TrueFoundryAgentStore', () => { id: previous.id, manifest: updatedManifest, }, - undefined, + TXN, ); expect(firstInvocationOrder(putRemoteAgent)).toBeLessThan(firstInvocationOrder(updateAgent)); }); @@ -361,14 +369,16 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - const result = await store.updateAgent({ - tenant_id: TENANT, - id: previous.id, - manifest: updatedManifest, - }); + const result = await store.updateAgent( + { + tenant_id: TENANT, + id: previous.id, + manifest: updatedManifest, + }, + TXN, + ); expect(result?.external_id).toBe('sf-new'); expect(updateAgent).toHaveBeenCalledTimes(1); expect(updateAgent).toHaveBeenCalledWith( @@ -378,7 +388,7 @@ describe('TrueFoundryAgentStore', () => { manifest: updatedManifest, external_id: 'sf-new', }, - undefined, + TXN, ); }); @@ -393,11 +403,10 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); await expect( - store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: manifest({ instructions: 'Updated.' }) }), + store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: manifest({ instructions: 'Updated.' }) }, TXN), ).rejects.toThrow('sf failed'); expect(updateAgent).not.toHaveBeenCalled(); }); @@ -417,12 +426,11 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - await expect(store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest })).rejects.toThrow( - 'db write failed', - ); + await expect( + store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: updatedManifest }, TXN), + ).rejects.toThrow('db write failed'); expect(putRemoteAgent).toHaveBeenCalledTimes(2); expect(putRemoteAgent).toHaveBeenLastCalledWith( expect.objectContaining({ @@ -448,11 +456,10 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); await expect( - store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: manifest({ instructions: 'Updated.' }) }), + store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: manifest({ instructions: 'Updated.' }) }, TXN), ).rejects.toMatchObject({ message: 'updateAgent failed and ServiceFoundry restore also failed', errors: [ @@ -462,24 +469,18 @@ describe('TrueFoundryAgentStore', () => { }); }); - it('deleteAgent runs under withUpdateLock', async () => { + it('deleteAgent reads the agent inside the transaction', async () => { const previous = record({ external_id: 'sf-1' }); const getAgent = jest.fn(async () => previous); const deleteAgent = jest.fn(async () => undefined); - const lockCalls: Array<{ tenant_id: string; id: string }> = []; - const withUpdateLock: WithAgentUpdateLock = async (input, fn) => { - lockCalls.push(input); - return fn(undefined); - }; const store = new TrueFoundryAgentStore({ inner: mockInner({ getAgent, deleteAgent }), client: mockClient(), accessToken: TOKEN, - withUpdateLock, }); - await store.deleteAgent({ tenant_id: TENANT, id: previous.id }); - expect(lockCalls).toEqual([{ tenant_id: TENANT, id: previous.id }]); + await store.deleteAgent({ tenant_id: TENANT, id: previous.id }, TXN); + expect(getAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: previous.id }, TXN); }); it('deleteAgent deletes ServiceFoundry then DB when external_id is set', async () => { @@ -491,12 +492,11 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - await store.deleteAgent({ tenant_id: TENANT, id: previous.id }); + await store.deleteAgent({ tenant_id: TENANT, id: previous.id }, TXN); expect(deleteRemoteAgent).toHaveBeenCalledWith({ accessToken: TOKEN, externalId: 'sf-1' }); - expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: previous.id }, undefined); + expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: previous.id }, TXN); expect(firstInvocationOrder(deleteRemoteAgent)).toBeLessThan(firstInvocationOrder(deleteAgent)); }); @@ -509,10 +509,9 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - await store.deleteAgent({ tenant_id: TENANT, id: previous.id }); + await store.deleteAgent({ tenant_id: TENANT, id: previous.id }, TXN); expect(deleteAgent).toHaveBeenCalled(); expect(deleteRemoteAgent).not.toHaveBeenCalled(); }); @@ -525,11 +524,10 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - await store.deleteAgent({ tenant_id: TENANT, id: 'missing' }); - expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'missing' }, undefined); + await store.deleteAgent({ tenant_id: TENANT, id: 'missing' }, TXN); + expect(deleteAgent).toHaveBeenCalledWith({ tenant_id: TENANT, id: 'missing' }, TXN); expect(deleteRemoteAgent).not.toHaveBeenCalled(); }); @@ -544,10 +542,9 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), accessToken: TOKEN, - withUpdateLock: withoutAgentUpdateLock(), }); - await expect(store.deleteAgent({ tenant_id: TENANT, id: previous.id })).rejects.toThrow('sf delete failed'); + await expect(store.deleteAgent({ tenant_id: TENANT, id: previous.id }, TXN)).rejects.toThrow('sf delete failed'); expect(deleteRemoteAgent).toHaveBeenCalled(); expect(deleteAgent).not.toHaveBeenCalled(); });