diff --git a/server/src/__tests__/heartbeat-local-environment.test.ts b/server/src/__tests__/heartbeat-local-environment.test.ts index 6375bf874a..cef995d59e 100644 --- a/server/src/__tests__/heartbeat-local-environment.test.ts +++ b/server/src/__tests__/heartbeat-local-environment.test.ts @@ -62,6 +62,7 @@ async function waitForRunLeasesToRelease( describeEmbeddedPostgres("heartbeat local environment lifecycle", () => { let db!: ReturnType; + let heartbeat!: ReturnType; let tempDb: Awaited> | null = null; let previousAgentJwtSecret: string | undefined; @@ -70,9 +71,16 @@ describeEmbeddedPostgres("heartbeat local environment lifecycle", () => { process.env.PAPERCLIP_AGENT_JWT_SECRET = "heartbeat-local-environment-test-secret"; tempDb = await startEmbeddedPostgresTestDatabase("heartbeat-local-environment-"); db = createDb(tempDb.connectionString); + heartbeat = heartbeatService(db); }, 20_000); afterEach(async () => { + // A run reaches its terminal status before finalizeRun finishes writing + // its trailing lifecycle events and side effects (see the comment on + // drainActiveRunExecutions in heartbeat.ts). Drain those in-flight writes + // before the TRUNCATE below, or a write that lands after the company row + // is gone violates heartbeat_run_events' foreign key. + await heartbeat.drainActiveRunExecutions(); await db.execute(sql.raw(` TRUNCATE TABLE "environment_leases", @@ -90,6 +98,10 @@ describeEmbeddedPostgres("heartbeat local environment lifecycle", () => { }); afterAll(async () => { + // Same reasoning as the afterEach drain: closing the embedded database + // while a run's finalize work is still in flight lets a queued write hit + // a socket that cleanup() already tore down. + await heartbeat.drainActiveRunExecutions(); await tempDb?.cleanup(); if (previousAgentJwtSecret === undefined) { delete process.env.PAPERCLIP_AGENT_JWT_SECRET; @@ -126,7 +138,6 @@ describeEmbeddedPostgres("heartbeat local environment lifecycle", () => { permissions: {}, }); - const heartbeat = heartbeatService(db); const queued = await heartbeat.invoke(agentId, "on_demand", {}, "manual"); expect(queued).not.toBeNull(); @@ -198,7 +209,6 @@ describeEmbeddedPostgres("heartbeat local environment lifecycle", () => { permissions: {}, }); - const heartbeat = heartbeatService(db); const queued = await heartbeat.invoke(agentId, "on_demand", {}, "manual"); expect(queued).not.toBeNull(); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 94ab86d2dd..fbdc97b0ae 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -123,6 +123,7 @@ import { type SetupTokenSessionScope, type SetupTokenSessionState, type SetupTokenSessionDescriptor, + SETUP_TOKEN_ADAPTER_TYPE, } from "../services/setup-token-session.js"; import type { DeploymentMode, @@ -4770,9 +4771,6 @@ export function agentRoutes( // Each route writes its full path as a plain string literal, so the static // OpenAPI coverage test can read the path from the source text. - // The company-and-environment login serves only the Claude adapter. - const CLAUDE_SETUP_TOKEN_ADAPTER_TYPE = "claude_local"; - // Maps the internal session state to the public login status. The public union // carries no server-only state, so the route never returns the internal // `submitting` or `stored` state to a client. @@ -4828,7 +4826,7 @@ export function agentRoutes( const companySetupTokenKey = (companyId: string, ownerUserId: string) => ({ companyId, ownerUserId, - adapterType: CLAUDE_SETUP_TOKEN_ADAPTER_TYPE, + adapterType: SETUP_TOKEN_ADAPTER_TYPE, }); // The stored Claude OAuth token status read. It returns @@ -4898,6 +4896,18 @@ export function agentRoutes( res.status(400).json({ error: "This adapter does not support a setup-token login." }); return true; } + // The five follow-up routes and the restart reaper both read only the + // one pinned adapter type. A capability match alone is not enough: an + // adapter that declares `storedSessionId` but is not the served type + // would pass the check above, then create a session that no follow-up + // route and no reaper scan can reach. Reject that case here, before any + // sandbox assertion, lease, durable row, or pseudo-terminal, with the + // same fixed 400 as the capability check above, so the response + // discloses no difference between the two rejection reasons. + if (data.adapterType !== SETUP_TOKEN_ADAPTER_TYPE) { + res.status(400).json({ error: "This adapter does not support a setup-token login." }); + return true; + } if (!SETUP_TOKEN_LOGIN_TRANSPORT_READY) { res.status(503).json({ error: SETUP_TOKEN_START_FAILED }); return true; diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 690c4883e3..d56bbb8fb3 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -232,6 +232,7 @@ import { claudeSetupTokenSessionOwnerResponseSchema, claudeSetupTokenCompletionResponseSchema, claudeOAuthTokenStatusResponseSchema, + startAdapterAuthSessionRequestSchema, } from "@paperclipai/shared"; import { COMPANY_IMPORT_TRANSFERS_API_PATH, @@ -642,11 +643,12 @@ const refreshExternalObjectsBodySchema = z.object({ objectIds: z.array(z.string().guid()).max(50).optional(), }).strict(); -// The start route reads the body directly, so document the accepted fields -// here. A sandbox environment is required. The time-to-live is optional. -const startAdapterLoginSessionSchema = z.object({ - environmentId: z.string().min(1), - ttlSeconds: z.number().optional(), +// The route enforces the shared strict request schema. The route spine +// injects the adapter type from the path, so the client body never carries +// it; derive the documented body from the shared schema and omit that field, +// so the documented body cannot drift from the route again. +const startAdapterLoginSessionSchema = startAdapterAuthSessionRequestSchema.omit({ + adapterType: true, }); const environmentCustomImageCompanyQuerySchema = z.object({ diff --git a/server/src/routes/setup-token-route.test.ts b/server/src/routes/setup-token-route.test.ts index c1075cce77..09c583021f 100644 --- a/server/src/routes/setup-token-route.test.ts +++ b/server/src/routes/setup-token-route.test.ts @@ -693,12 +693,13 @@ describe("company-and-environment setup-token route — object-level authorizati expect(transport.records).toEqual([]); }); - it("starts a setup-token login for a third adapter that declares the capability", async () => { - // A third adapter, not the Claude adapter, declares the pseudo-terminal - // setup-token capability with the stored-session claim. The guard reads the - // capability, not the adapter name, so the adapter passes the guard and - // starts a session. This proves no adapter-name branch remains in the guard - // path. The start response reads the panel mode from the capability. + it("rejects a third adapter that declares the capability but is not the served adapter", async () => { + // A third adapter, not the Claude adapter, declares the same pseudo-terminal + // setup-token capability with the stored-session claim. The five follow-up + // routes and the reaper both read only the one served adapter type, so the + // guard must reject this adapter even though its capability matches. It + // rejects with the same fixed 400 as the capability-mismatch case, before + // any sandbox assertion, lease, durable row, or pseudo-terminal. mockFindActiveServerAdapter.mockImplementation((type: string) => type === "gemini_local" ? { @@ -714,9 +715,15 @@ describe("company-and-environment setup-token route — object-level authorizati environmentId: ENVIRONMENT_ID, adapterType: "gemini_local", }); - expect(res.status, JSON.stringify(res.body)).toBe(201); - expect(res.body.panelMode).toBe("displayed_code"); - expect(res.body.status).toBe("waiting_for_user"); + expect(res.status, JSON.stringify(res.body)).toBe(400); + expect(res.body.error).toBe("This adapter does not support a setup-token login."); + // The route rejected the adapter before any sandbox, lease, or store side + // effect: no cleanup record, no lease acquire, no pseudo-terminal start, and + // no environment or provider guard call. + expect(transport.records).toEqual([]); + expect(transport.factoryInvocations.count).toBe(0); + expect(mockEnvironmentService.getById).not.toHaveBeenCalled(); + expect(mockResolvePluginSandboxProviderDriverByKey).not.toHaveBeenCalled(); }); }); diff --git a/server/src/services/setup-token-session.ts b/server/src/services/setup-token-session.ts index efe18f3dda..813286a168 100644 --- a/server/src/services/setup-token-session.ts +++ b/server/src/services/setup-token-session.ts @@ -41,7 +41,20 @@ import type { AgentAdapterType } from "@paperclipai/shared"; // unified `adapter_auth_sessions` table also holds the Codex device-login rows, // so every store scan filters by this adapter to reach only the setup-token // rows. -const SETUP_TOKEN_ADAPTER_TYPE: AgentAdapterType = "claude_local"; +// +// Three consumers share this one constant, and they must change together: +// 1. The start-route guard in `agents.ts`, which rejects a request for any +// other adapter type before it creates a lease, a durable row, or a +// pseudo-terminal. +// 2. The five follow-up routes in `agents.ts`, which build their session +// lookup key from this constant through `companySetupTokenKey`. +// 3. The restart reaper scan below, which filters on this constant to reach +// only the setup-token rows and to leave every Codex device-login row +// alone. +// A future change that serves a second adapter through this flow must widen +// all three consumers, not just one, or a served adapter will create a row +// that a follow-up route or the reaper cannot find. +export const SETUP_TOKEN_ADAPTER_TYPE: AgentAdapterType = "claude_local"; /** * The session states. The four terminal states end the login. The `stored` diff --git a/ui/src/adapters/use-adapter-capabilities.ts b/ui/src/adapters/use-adapter-capabilities.ts index 9be335be99..c05cf0a143 100644 --- a/ui/src/adapters/use-adapter-capabilities.ts +++ b/ui/src/adapters/use-adapter-capabilities.ts @@ -15,10 +15,14 @@ const ALL_FALSE: AdapterCapabilities = { /** * Synchronous fallback for known built-in adapter types so capability checks * return correct values on first render before the /api/adapters call resolves. + * + * The `login` value for `claude_local` and `codex_local` mirrors the server's + * login capability declaration in `server/src/adapters/registry.ts`. Reconcile + * the two together if either adapter's login flow changes. */ const KNOWN_DEFAULTS: Record = { - claude_local: { supportsInstructionsBundle: true, supportsSkills: true, supportsLocalAgentJwt: true, requiresMaterializedRuntimeSkills: false, supportsModelProfiles: true, supportsAcp: true }, - codex_local: { supportsInstructionsBundle: true, supportsSkills: true, supportsLocalAgentJwt: true, requiresMaterializedRuntimeSkills: false, supportsModelProfiles: true, supportsAcp: true }, + claude_local: { supportsInstructionsBundle: true, supportsSkills: true, supportsLocalAgentJwt: true, requiresMaterializedRuntimeSkills: false, supportsModelProfiles: true, supportsAcp: true, login: { panelMode: "submitted_browser_code", timeoutPolicy: "fixed" } }, + codex_local: { supportsInstructionsBundle: true, supportsSkills: true, supportsLocalAgentJwt: true, requiresMaterializedRuntimeSkills: false, supportsModelProfiles: true, supportsAcp: true, login: { panelMode: "displayed_code", timeoutPolicy: "caller_bounded" } }, paperclip_runner: { supportsInstructionsBundle: false, supportsSkills: true, supportsLocalAgentJwt: false, requiresMaterializedRuntimeSkills: false, supportsModelProfiles: false, supportsAcp: false }, cursor: { supportsInstructionsBundle: true, supportsSkills: true, supportsLocalAgentJwt: true, requiresMaterializedRuntimeSkills: true, supportsModelProfiles: true, supportsAcp: false }, gemini_local: { supportsInstructionsBundle: true, supportsSkills: true, supportsLocalAgentJwt: true, requiresMaterializedRuntimeSkills: true, supportsModelProfiles: true, supportsAcp: true },