From dc30dc4f341057a94c0c6c0acde69324a9ce3d18 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Tue, 25 Aug 2026 19:48:44 -0700 Subject: [PATCH] fix(setup-token): pin the start guard to the served adapter (#12179) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Sandbox agents use adapter login routes to start authenticated sessions > - The setup-token start route accepted adapter types that later routes and cleanup did not serve > - This mismatch could create sessions that no route or reaper could reach > - The OpenAPI body schema and synchronous login capability defaults also differed from the enforced behavior > - This pull request pins the start guard to the served adapter, shares the adapter constant, aligns the schema, and exposes login capabilities early > - The benefit is consistent session access, cleanup, API documentation, and login UI behavior ## Linked Issues or Issue Description Refs: #11730 Refs: #11286 **Subsystem affected** Cross-cutting server and UI login behavior. **Problem or motivation** The setup-token start route accepted a non-served adapter type. Follow-up routes and the reaper only handled the served adapter. This could create an unreachable session that held its slot. The OpenAPI schema and early capability defaults also did not match the route behavior. **Proposed solution** Pin the start guard, follow-up key, and reaper filter to one exported served-adapter constant. Derive the OpenAPI body from the strict shared schema. Add the login capability projection to synchronous defaults. **Alternatives considered** Keep separate adapter constants and add another guard at each follow-up route. This would preserve duplicate sources of truth and leave future drift possible. **Roadmap alignment** This change supports the Cloud / Sandbox agents milestone in `ROADMAP.md`. ## What Changed - Reject a setup-token start request when its adapter type is not the served adapter. - Reuse one exported adapter constant for the start guard, follow-up key, and reaper filter. - Derive the company adapter login-sessions start body from the strict shared schema. - Add the `login` capability projection to the synchronous Claude and Codex adapter defaults. - Add regression coverage for the rejected non-served adapter request. ## Verification - The setup-token route suite passes, including the non-served adapter regression test. - The setup-token session-service suite passes. - The setup-token reaper suite passes. - The OpenAPI suite passes. - The server TypeScript check passes. - The UI TypeScript check passes. - GitHub Actions must confirm all required checks after pull request creation. ## Risks The start route now rejects adapter types that follow-up routes cannot serve. No database migration exists. Revert the one commit to roll back the change. ## Model Used OpenAI Codex, GPT-5, exact runtime model ID not exposed, large context window, reasoning, tool use, and code execution. The implementing engineer used AI assistance. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../heartbeat-local-environment.test.ts | 14 +++++++++-- server/src/routes/agents.ts | 18 ++++++++++--- server/src/routes/openapi.ts | 12 +++++---- server/src/routes/setup-token-route.test.ts | 25 ++++++++++++------- server/src/services/setup-token-session.ts | 15 ++++++++++- ui/src/adapters/use-adapter-capabilities.ts | 8 ++++-- 6 files changed, 69 insertions(+), 23 deletions(-) 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 },