fix: allow concurrent agent runs on one OpenAI or xAI subscription connection (#13452)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Paperclip starts local command-line sessions and stores provider
credentials through managed connections
> - One OpenAI Codex or xAI Grok subscription connection held a
credential lease for the full agent run
> - A second run then waited for the first run, and concurrent runs
could overwrite a newer credential
> - The write-back must compare fresh credentials while it holds the row
lock
> - This pull request removes the run lease, keeps the revocation guard,
and bounds Codex timestamps against the host clock
> - The benefit is safe concurrent use of one subscription connection
with newest-credential selection

## Linked Issues or Issue Description

**What happened?**

A managed OpenAI Codex or xAI Grok subscription connection held a
credential lease for the full agent run. A second run waited for the
first run to finish. The write-back gate also rejected any row change
before it compared credential freshness.

**Expected behavior**

Concurrent runs should start on one subscription connection. The server
should keep the newest valid credential and reject a credential write
after a person revokes the connection.

**Steps to reproduce**

1. Start two runs that use one OpenAI or xAI subscription connection.
2. Let both provider tools refresh the credential.
3. Finish the runs in either order.
4. Confirm that the newest valid credential remains in the connection.

**Paperclip version or commit**

`ec25bf1e4a81d1729a6d7276e6a486587cff4a0b`

**Deployment mode**

Local dev (`pnpm dev`), built from source.

**Agent adapter(s) involved**

Codex. The server path also covers xAI Grok subscription connections.

**Database mode**

Embedded PGlite for local development, and external Postgres for
deployments.

**Access context**

Both board and agent runs can use managed connections.

**Additional context**

The provider command-line tool refreshes credentials inside the sandbox.
The server copies the result back after the run. Two long runs can still
refresh one token hours apart, so the provider can reject the second
refresh. The server cannot observe that provider call.

## What Changed

- Remove the full-run credential lease for OpenAI Codex and xAI Grok
subscription connections.
- Lock and re-read the connection row before credential write-back.
- Accept only a strictly newer credential, while keeping the connection
revocation guard.
- Reject Codex freshness timestamps more than five minutes ahead of the
host clock.
- Add tests for both completion orders, xAI cleanup, revocation, the
Codex time bound, and agent hiring.
- Update the connection and run-log documentation.

## Verification

- `server/src/__tests__/ai-connections.test.ts` passes with 42 tests.
-
`packages/adapters/codex-local/src/server/codex-auth-merge-decision.test.ts`
covers the five-minute boundary and the one-millisecond overflow.
- `packages/adapters/codex-local/src/server/codex-auth-merge.test.ts`
passes.
- `server/src/__tests__/agent-hire-ai-connections.test.ts` covers OpenAI
and Anthropic.
- The project type check reports no new error in changed files.
- GitHub Actions must pass on this pull request.

## Risks

The write-back now permits concurrent runs, so the provider may reject a
later refresh when both long runs use one token. The server keeps the
revocation guard and rejects future-dated Codex timestamps. No schema
change occurs.

## Model Used

OpenAI Codex, GPT-5. Context window and exact deployment build are not
exposed in this run. The model used tool calls, code inspection, and
test verification.

## 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 LeachandPaperclip authored and GitHub committed 2026-09-15 01:10:54 -07:00
1 parent da77a0c28c
commit 9ed55f6931
14 files changed
+554 -514

No files matched your search

+29 -67
View File
@@ -4,7 +4,7 @@ import { mkdtemp, mkdir, writeFile, readFile, rm } from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { and, eq } from "drizzle-orm";
import { type Db, connectionGrants } from "@paperclipai/db";
import { type Db, companySecrets, connectionGrants } from "@paperclipai/db";
import {
AI_CONNECTION_CAPABILITIES,
type AiConnectionBinding,
@@ -166,41 +166,6 @@ done`,
}
}
async function acquireCredentialLease(db: Db, grantId: string) {
const client = await db.$client.reserve();
try {
// A reserved client pins our connection to PgBouncer, not its backend.
// Keep the lease in one transaction so transaction-pooling deployments
// cannot acquire and release it on different PostgreSQL sessions.
await client`begin`;
await client`set local idle_in_transaction_session_timeout = 0`;
const [result] =
await client`select pg_try_advisory_xact_lock(hashtextextended(${`ai-runtime:${grantId}`}, 0)) as acquired`;
if (!result.acquired)
throw unprocessable(
"This subscription is in use. Retry when its current execution finishes.",
{ code: "ai_connection_busy" },
);
} catch (error) {
try {
await client`rollback`;
} finally {
client.release();
}
throw error;
}
let released = false;
return async () => {
if (released) return;
released = true;
try {
await client`rollback`;
} finally {
client.release();
}
};
}
export async function prepareManagedAiRuntime(
db: Db,
input: {
@@ -246,12 +211,6 @@ export async function prepareManagedAiRuntime(
const subscriptionFile =
selection.attribution.method === "subscription" &&
input.binding.provider !== "anthropic";
// A file-backed subscription runs one credential rotation at a time, so it
// needs the lease. Anthropic writes no file back, so two runs share no
// mutable state and can run at the same time without the lease.
const release = subscriptionFile
? await acquireCredentialLease(db, selection.grant.id)
: async () => {};
let home: string | undefined;
try {
const selectedGrantId = selection.grant.id;
@@ -344,18 +303,32 @@ export async function prepareManagedAiRuntime(
),
)
.for("update");
// Reconnect/revocation wins over a process holding an older credential.
if (
!grant ||
grant.status !== "active" ||
grant.updatedAt.getTime() !==
selection.grant.updatedAt.getTime()
)
return;
const current = await service.credential({
...selection,
grant,
});
// A missing or revoked grant blocks the write-back. Among
// active copies, the merge decision below keeps the
// credential with the newest provider freshness field.
if (!grant || grant.status !== "active") return;
const ref = grant.credentialSecretRefs.find(
(r) => r.configPath === "ai.credential",
);
if (!ref) return;
// Lock the referenced secret row for the rest of this
// transaction. The grant-row lock above does not cover it,
// so an authorized rotation of this secret could otherwise
// land between the read and the write below and be
// overwritten by this stale write-back.
await tx
.select({ id: companySecrets.id })
.from(companySecrets)
.where(
and(
eq(companySecrets.id, ref.secretId),
eq(companySecrets.companyId, input.companyId),
),
)
.for("update");
const current = await aiConnectionService(
tx as unknown as Db,
).credential({ ...selection, grant });
const destination = path.join(
providerHome,
"current-auth.json",
@@ -370,9 +343,6 @@ export async function prepareManagedAiRuntime(
errorLabel: "AI account refresh",
});
if (decision !== 10) return;
const ref = grant.credentialSecretRefs.find(
(r) => r.configPath === "ai.credential",
)!;
await secretService(tx).rotate(
ref.secretId,
{ value: refreshed },
@@ -385,20 +355,12 @@ export async function prepareManagedAiRuntime(
});
}
} finally {
try {
if (home) await rm(home, { recursive: true, force: true });
} finally {
await release();
}
if (home) await rm(home, { recursive: true, force: true });
}
},
};
} catch (error) {
try {
if (home) await rm(home, { recursive: true, force: true });
} finally {
await release();
}
if (home) await rm(home, { recursive: true, force: true });
throw error;
}
}
+3 -4
View File
@@ -16037,10 +16037,9 @@ export function heartbeatService(
.then((rows) => rows[0] ?? null);
}
// Subscription homes are serialized so refresh tokens cannot be overwritten
// by another run. Contention is a resource wait, not broken authentication.
// The database lease is released on completion/disconnect; retrying remains
// safe across processes and each attempt revalidates the selected account.
// No code path raises the `ai_connection_busy` error any more. This
// function stays because a stored run row can still carry that error code
// from an earlier release.
async function finalizeAiConnectionBusyDeferral(
run: typeof heartbeatRuns.$inferSelect,
error: HttpError,
+25 -16
View File
@@ -2522,22 +2522,14 @@ export function secretService(db: Db | DbTransaction) {
}
try {
// Lock and update the parent secret row before touching its version
// rows. A credential write-back locks the parent secret row first, then
// calls this function. If this transaction updated the version rows
// first instead, the two transactions would take their two row locks
// in opposite order and could deadlock. Matching the order here removes
// that risk: every caller now locks the parent row before the version
// rows, so a lock cycle between the two tables cannot form.
return await db.transaction(async (tx) => {
await tx
.update(companySecretVersions)
.set({ status: "previous" })
.where(and(
eq(companySecretVersions.secretId, secret.id),
ne(companySecretVersions.version, nextVersion),
));
await tx
.update(companySecretVersions)
.set({ status: "current" })
.where(and(
eq(companySecretVersions.secretId, secret.id),
eq(companySecretVersions.version, nextVersion),
));
const updated = await tx
.update(companySecrets)
.set({
@@ -2561,11 +2553,28 @@ export function secretService(db: Db | DbTransaction) {
if (!updated) {
// The predicate matched no row. A supplied expected version means a
// concurrent rotation won the race; return the stale conflict. An
// unguarded rotation means the secret is gone.
// unguarded rotation means the secret is gone. Neither version row
// has been touched yet, so there is nothing to undo here.
throw input.expectedLatestVersion === undefined
? notFound("Secret not found")
: conflict(SECRET_VERSION_STALE_CONFLICT);
}
await tx
.update(companySecretVersions)
.set({ status: "previous" })
.where(and(
eq(companySecretVersions.secretId, secret.id),
ne(companySecretVersions.version, nextVersion),
));
await tx
.update(companySecretVersions)
.set({ status: "current" })
.where(and(
eq(companySecretVersions.secretId, secret.id),
eq(companySecretVersions.version, nextVersion),
));
return updated;
});
} catch (error) {