Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,29 @@ describe("claude execute auth failure classification", () => {
expect(result.errorFamily ?? null).toBeNull();
});

// The CLI's own remedy for "not logged in" is an interactive `/login`, which
// is not something a Paperclip agent can ever do: the run is headless, and on
// a managed sandbox the pod is destroyed with the lease. Echoing it sent a
// paying hosted user chasing a command they could not run, with no hint that
// the fix is to reconnect the credential. Same rule codex-local already
// applies: lead with the remedy that works here, mention host login second.
it("does not tell the user to run an interactive login it cannot run", async () => {
const result = await runExecuteWithProcResult({
exitCode: 1,
stdout: claudeFailureStdout("Not logged in \u00b7 Please run /login"),
stderr: "",
});

expect(result.errorCode).toBe("claude_auth_required");
expect(result.errorMessage).toBe(
"Claude reported no usable credential. Connect a Claude credential for this agent, "
+ "then resume. If you already connected one, it was rejected (an OAuth token that "
+ "has expired or been revoked does this) so mint a fresh one with `claude setup-token` "
+ "and reconnect it. On a self-hosted install, signing in to Claude on the host also works.",
);
expect(result.errorMessage ?? "").not.toContain("/login");
});

it("classifies a 401 invalid OAuth access token failure as claude_auth_required", async () => {
const result = await runExecuteWithProcResult({
exitCode: 1,
Expand Down
23 changes: 21 additions & 2 deletions packages/adapters/claude-local/src/server/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,17 @@ const executeClaudeAcp = createClaudeAcpExecutor();

const CLAUDE_INVALID_CREDENTIAL_MESSAGE =
"Claude rejected the connected credential. Reconnect a valid Claude credential, then resume.";
// The CLI answers "not logged in" with an interactive `/login`, which a
// Paperclip run can never perform: it is headless, and on a managed sandbox the
// pod is destroyed with the lease. Surfacing that verbatim points the user at a
// command they cannot run and hides the actual remedy. Lead with what works
// here and keep the host-login path as the self-hosted footnote, the same shape
// codex-local uses for its managed-home credential error.
const CLAUDE_LOGIN_REQUIRED_MESSAGE =
"Claude reported no usable credential. Connect a Claude credential for this agent, "
+ "then resume. If you already connected one, it was rejected (an OAuth token that "
+ "has expired or been revoked does this) so mint a fresh one with `claude setup-token` "
+ "and reconnect it. On a self-hosted install, signing in to Claude on the host also works.";

interface ClaudeExecutionInput {
runId: string;
Expand Down Expand Up @@ -1026,7 +1037,11 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
exitCode: proc.exitCode,
signal: proc.signal,
timedOut: false,
errorMessage: invalidCredential ? CLAUDE_INVALID_CREDENTIAL_MESSAGE : fallbackErrorMessage,
errorMessage: invalidCredential
? CLAUDE_INVALID_CREDENTIAL_MESSAGE
: loginMeta.requiresLogin
? CLAUDE_LOGIN_REQUIRED_MESSAGE
: fallbackErrorMessage,
errorCode,
errorFamily,
retryNotBefore: transientRetryNotBefore ? transientRetryNotBefore.toISOString() : null,
Expand Down Expand Up @@ -1122,7 +1137,11 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
});
// The raw CLI text stays in resultJson; the surfaced message must tell the
// user what to do, not echo the provider's 401.
const errorMessage = invalidCredential ? CLAUDE_INVALID_CREDENTIAL_MESSAGE : rawErrorMessage;
const errorMessage = invalidCredential
? CLAUDE_INVALID_CREDENTIAL_MESSAGE
: failed && loginMeta.requiresLogin
? CLAUDE_LOGIN_REQUIRED_MESSAGE
: rawErrorMessage;
const providerQuota =
failed &&
!loginMeta.requiresLogin &&
Expand Down
75 changes: 74 additions & 1 deletion server/src/__tests__/agent-credential-inheritance.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { mergeInheritedCredentialEnv } from "../services/agent-credential-inheritance.js";
import { mergeInheritedCredentialEnv, planCompanyCredentialBackfill } from "../services/agent-credential-inheritance.js";

describe("mergeInheritedCredentialEnv (new agents inherit the company credential)", () => {
const donorCred = { type: "secret_ref", secretId: "sec-1", version: "latest" };
Expand Down Expand Up @@ -64,3 +64,76 @@ describe("mergeInheritedCredentialEnv (new agents inherit the company credential
});
});
});

// Inheritance used to run only when an agent was CREATED. The normal order of
// events is the opposite: a company (and its built-in agents) exist from
// signup, and the user connects a provider key afterwards. Every built-in agent
// therefore stayed credential-less forever, and a factory-config built-in agent
// could never run. A real hosted company hit exactly this: an active
// company-scoped Claude token bound only to the one agent the user made by
// hand, while the built-in Summarizer sat paused on "Connect a model key to run
// this agent."
describe("planCompanyCredentialBackfill (existing agents pick up a later-connected key)", () => {
const cred = { type: "secret_ref", secretId: "sec-1", version: "latest" };
const withCred = { env: { CLAUDE_CODE_OAUTH_TOKEN: cred } };

it("gives the donor's credential to an agent that has none", () => {
const plan = planCompanyCredentialBackfill([
{ id: "donor", role: "ceo", adapterConfig: withCred },
{ id: "summarizer", role: "worker", adapterConfig: { model: "x" } },
]);
expect(plan).toEqual([
{ agentId: "summarizer", adapterConfig: { model: "x", env: { CLAUDE_CODE_OAUTH_TOKEN: cred } } },
]);
});

it("covers a built-in agent whose adapterConfig has no env at all", () => {
const plan = planCompanyCredentialBackfill([
{ id: "donor", role: "worker", adapterConfig: withCred },
{ id: "builtin", role: "worker", adapterConfig: {} },
]);
expect(plan.map((entry) => entry.agentId)).toEqual(["builtin"]);
expect(plan[0]?.adapterConfig).toEqual({ env: { CLAUDE_CODE_OAUTH_TOKEN: cred } });
});

it("never touches an agent that already has its own credential", () => {
const own = { type: "secret_ref", secretId: "sec-own", version: "latest" };
const plan = planCompanyCredentialBackfill([
{ id: "donor", role: "ceo", adapterConfig: withCred },
{ id: "other", role: "worker", adapterConfig: { env: { CLAUDE_CODE_OAUTH_TOKEN: own } } },
]);
expect(plan).toEqual([]);
});

it("does nothing when no agent in the company has a credential yet", () => {
expect(planCompanyCredentialBackfill([
{ id: "a", role: "ceo", adapterConfig: {} },
{ id: "b", role: "worker", adapterConfig: { env: { SOME_FLAG: "plain" } } },
])).toEqual([]);
});

it("prefers the ceo as donor, like the create-time path", () => {
const ceoCred = { type: "secret_ref", secretId: "sec-ceo", version: "latest" };
const plan = planCompanyCredentialBackfill([
{ id: "worker-with-key", role: "worker", adapterConfig: withCred },
{ id: "ceo", role: "ceo", adapterConfig: { env: { CLAUDE_CODE_OAUTH_TOKEN: ceoCred } } },
{ id: "needs-key", role: "worker", adapterConfig: {} },
]);
expect(plan).toEqual([
{ agentId: "needs-key", adapterConfig: { env: { CLAUDE_CODE_OAUTH_TOKEN: ceoCred } } },
]);
});

it("is a no-op on a second pass, so repeated connects do not rewrite agents", () => {
const rows = [
{ id: "donor", role: "ceo", adapterConfig: withCred },
{ id: "needs-key", role: "worker", adapterConfig: {} },
];
const first = planCompanyCredentialBackfill(rows);
const settled = rows.map((row) => {
const write = first.find((entry) => entry.agentId === row.id);
return write ? { ...row, adapterConfig: write.adapterConfig } : row;
});
expect(planCompanyCredentialBackfill(settled)).toEqual([]);
});
});
26 changes: 25 additions & 1 deletion server/src/routes/agents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ import {
} from "@paperclipai/adapter-utils/server-utils";
import { trackAgentCreated } from "@paperclipai/shared/telemetry";
import { validate } from "../middleware/validate.js";
import { inheritCompanyCredentialEnv } from "../services/agent-credential-inheritance.js";
import {
adapterConfigHasSecretRef,
backfillCompanyCredentialEnv,
inheritCompanyCredentialEnv,
} from "../services/agent-credential-inheritance.js";
import {
agentService,
agentInstructionsService,
Expand Down Expand Up @@ -3146,6 +3150,26 @@ export function agentRoutes(
details: summarizeAgentUpdateDetails(patchData),
});

// Connecting a credential is an update to ONE agent, but the key is a
// company credential and the user reasonably expects their company to be
// able to run. Without this, every other agent that predates the connect
// (in particular the built-in agents provisioned at signup) stays
// credential-less with no way for the user to tell why. Only agents that
// have no credential of their own are touched, so this cannot overwrite a
// deliberate per-agent key, and it is a no-op on repeat.
if (hasOwn(patchData, "adapterConfig") && adapterConfigHasSecretRef(agent.adapterConfig)) {
await backfillCompanyCredentialEnv(
db,
agent.companyId,
agent.adapterType,
async (agentId, adapterConfig) => {
await svc.update(agentId, { adapterConfig }, {
recordRevision: { source: "credential-backfill" },
});
},
);
}

res.json(agent);
});

Expand Down
80 changes: 80 additions & 0 deletions server/src/services/agent-credential-inheritance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,86 @@ export function mergeInheritedCredentialEnv(
return merged;
}

/** True when an agent's adapterConfig already carries a credential binding. */
export function adapterConfigHasSecretRef(adapterConfig: unknown): boolean {
const env = asRecord(asRecord(adapterConfig)?.env);
if (!env) return false;
return Object.values(env).some((binding) => {
const parsed = asRecord(binding);
return parsed?.type === "secret_ref" && typeof parsed.secretId === "string";
});
}

/**
* Decide which of a company's same-adapter agents should receive the company
* credential, given every such agent's current row. Pure, so the rule is
* testable without a database.
*
* The create-time path ({@link inheritCompanyCredentialEnv}) only ever fires
* while an agent is being made, which is the wrong moment for the common case:
* a company and its built-in agents exist from signup and the user connects a
* provider key afterwards. Nothing re-ran inheritance, so those agents stayed
* credential-less permanently and a factory-config built-in agent could never
* run. This closes that gap by re-running the same donor rule over agents that
* already exist.
*
* Same donor preference as create time (a CEO with a credential, else any agent
* with one) and the same merge semantics, so an agent that already has its own
* credential is never touched and a second pass is a no-op.
*/
export function planCompanyCredentialBackfill(
rows: Array<{ id: string; role: string | null; adapterConfig: unknown }>,
): Array<{ agentId: string; adapterConfig: Record<string, unknown> }> {
const donors = rows.filter((row) => adapterConfigHasSecretRef(row.adapterConfig));
const donor = donors.find((row) => row.role === "ceo") ?? donors[0];
if (!donor) return [];
const donorEnv = asRecord(asRecord(donor.adapterConfig)?.env) ?? {};
const writes: Array<{ agentId: string; adapterConfig: Record<string, unknown> }> = [];
for (const row of rows) {
if (adapterConfigHasSecretRef(row.adapterConfig)) continue;
const config = asRecord(row.adapterConfig) ?? {};
const env = mergeInheritedCredentialEnv(donorEnv, asRecord(config.env) ?? {});
if (Object.keys(env).length === 0) continue;
writes.push({ agentId: row.id, adapterConfig: { ...config, env } });
}
return writes;
}

/**
* Apply {@link planCompanyCredentialBackfill} to every agent of one adapter
* type in a company. `update` is injected rather than imported so this stays
* free of the agent service (which owns secret-binding sync) and free of a
* dependency cycle. Returns the ids actually written.
*/
export async function backfillCompanyCredentialEnv(
db: Db,
companyId: string,
adapterType: string,
update: (agentId: string, adapterConfig: Record<string, unknown>) => Promise<unknown>,
): Promise<string[]> {
let rows: Array<{ id: string; role: string | null; adapterConfig: unknown }>;
try {
rows = await db
.select({ id: agents.id, role: agents.role, adapterConfig: agents.adapterConfig })
.from(agents)
.where(and(eq(agents.companyId, companyId), eq(agents.adapterType, adapterType)));
} catch {
return [];
}
const written: string[] = [];
for (const write of planCompanyCredentialBackfill(rows)) {
// One agent failing to update must not strand the rest: this runs as a
// side effect of a request that has already succeeded.
try {
await update(write.agentId, write.adapterConfig);
written.push(write.agentId);
} catch {
// keep going
}
}
return written;
}

// Inherit the company's credential env bindings onto a newly created/provisioned
// agent so it can authenticate without the user re-connecting a key for every
// agent. Copies each `secret_ref` env binding from an existing same-adapter
Expand Down
25 changes: 24 additions & 1 deletion server/src/services/built-in-agents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { syncRoutineVariablesWithTemplate } from "@paperclipai/shared";
import type { Agent, Approval, CompanySkill, PermissionKey, Routine, RoutineTrigger, RoutineVariable } from "@paperclipai/shared";
import { conflict, HttpError, notFound, unprocessable } from "../errors.js";
import { logActivity } from "./activity-log.js";
import { inheritCompanyCredentialEnv } from "./agent-credential-inheritance.js";
import { adapterConfigHasSecretRef, inheritCompanyCredentialEnv } from "./agent-credential-inheritance.js";
import { agentInstructionsService } from "./agent-instructions.js";
import { agentService } from "./agents.js";
import { approvalService } from "./approvals.js";
Expand Down Expand Up @@ -1602,6 +1602,29 @@ export function builtInAgentService(
patch.adapterType = adapterType;
patch.adapterConfig = resolvedInput.adapterConfig ?? existing.adapterConfig;
}
// Inheritance at create time is not enough for a built-in agent: it is
// provisioned with the company, before the user has connected any
// provider key, so it inherits nothing and nothing ever re-runs. Left
// there it can never run, and the only signal is a paused agent saying
// "Connect a model key to run this agent." Re-inherit here, on the path
// every built-in agent goes through, but only while it still has no
// credential of its own, so an explicit choice is never overwritten.
if (!existingPendingApproval) {
const currentAdapterType = patch.adapterType ?? existing.adapterType;
const currentAdapterConfig = patch.adapterConfig ?? existing.adapterConfig;
if (!adapterConfigHasSecretRef(currentAdapterConfig)) {
const inherited = await inheritCompanyCredentialEnv(
db,
companyId,
currentAdapterType,
(currentAdapterConfig ?? {}) as Record<string, unknown>,
);
if (adapterConfigHasSecretRef(inherited)) {
patch.adapterType = currentAdapterType;
patch.adapterConfig = inherited;
}
}
}
if (!existingPendingApproval && resolvedInput.budgetMonthlyCents !== undefined) {
patch.budgetMonthlyCents = resolvedInput.budgetMonthlyCents;
}
Expand Down
Loading