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 <noreply@paperclip.ing>
This commit is contained in:
Nicky Leach 2026-08-25 19:48:44 -07:00 committed by GitHub
parent ca02d2463a
commit dc30dc4f34
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 69 additions and 23 deletions

View File

@ -62,6 +62,7 @@ async function waitForRunLeasesToRelease(
describeEmbeddedPostgres("heartbeat local environment lifecycle", () => {
let db!: ReturnType<typeof createDb>;
let heartbeat!: ReturnType<typeof heartbeatService>;
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | 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();

View File

@ -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;

View File

@ -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({

View File

@ -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();
});
});

View File

@ -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`

View File

@ -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<string, AdapterCapabilities> = {
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 },