mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The `codex_local` adapter isolates local Codex runs by assigning managed `CODEX_HOME` state per company and per agent > - PR #8272 tightened that isolation, but it left a gap: once `CODEX_HOME` became explicit, the adapter treated it like a user-managed override and skipped auth seeding > - That meant newly isolated agents could launch with no usable `auth.json`, hit OpenAI unauthenticated, and fail with `401 Missing bearer` > - Users who had already persisted one of those broken managed homes could remain stranded even after config changes unless Paperclip repaired the home itself > - This pull request teaches Paperclip to seed managed homes correctly, backfill already-stranded managed homes on startup, and reject credential-less managed homes before they reach the provider > - The benefit is that affected managed `codex_local` agents recover automatically after upgrade and restart, without manual `CODEX_HOME` surgery ## Linked Issues or Issue Description - Fixes #497 - Refs #5028 - Related PR: #8272 - Related PR: #8399 ## What Changed - Distinguished Paperclip-managed `CODEX_HOME` paths from genuine external overrides and always seeded auth into managed homes, even when `CODEX_HOME` is explicit in config. - Wrote API-key-backed `auth.json` files for managed homes when `OPENAI_API_KEY` is configured, otherwise symlinked the shared Codex auth for subscription/OAuth flows. - Added a startup reconciliation pass that backfills already-isolated managed homes created by the broken release so upgrade plus restart repairs stranded agents automatically. - Preserved previously resolved API-key auth when the stored `OPENAI_API_KEY` binding is secret-backed and startup cannot resolve the secret value directly. - Hardened the managed-home preflight to require a credential-bearing `auth.json`, not just file presence, and documented the recovery behavior. - Added regression tests covering managed-home seeding, fail-fast behavior, and server-side startup reconciliation. ## Verification ```bash pnpm exec vitest run packages/adapters/codex-local/src/server/codex-home.test.ts packages/adapters/codex-local/src/server/execute.auth.test.ts server/src/__tests__/codex-auth-reconciliation.test.ts server/src/__tests__/codex-local-execute.test.ts server/src/__tests__/server-startup-feedback-export.test.ts pnpm --filter @paperclipai/adapter-codex-local typecheck pnpm --filter @paperclipai/server typecheck pnpm check:tokens pnpm exec vitest run server/src/__tests__/server-startup-feedback-export.test.ts ``` GitHub Actions `PR` workflow is green on latest head `e5b1e08d4`, including policy, typecheck, test shards, build, e2e, serialized server suites, and canary dry run. Greptile Review is green on latest head with 0 comments added. No UI changes. ## Risks - Startup reconciliation now mutates persisted managed Codex homes at boot. Risk is low because it only touches Paperclip-managed company/agent home paths and no-ops when a home already has usable auth. - Genuine external `CODEX_HOME` overrides remain intentionally self-managed, so those users still own repair steps inside their custom home. - Hosts with neither shared Codex auth nor an explicit per-agent API key now fail earlier with a clearer adapter error instead of surfacing a downstream `401`, which changes timing but not capability. ## Model Used - OpenAI Codex, GPT-5-based coding agent in a local Codex session; exact served model ID/context window were not exposed to the session. Tool use and code execution were enabled. ## 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 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] If this change affects the UI, I have included before/after screenshots - [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>
405 lines
14 KiB
TypeScript
405 lines
14 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
|
|
const ORIGINAL_PAPERCLIP_API_URL = process.env.PAPERCLIP_API_URL;
|
|
const ORIGINAL_PAPERCLIP_RUNTIME_API_URL = process.env.PAPERCLIP_RUNTIME_API_URL;
|
|
const ORIGINAL_PAPERCLIP_RUNTIME_API_CANDIDATES_JSON = process.env.PAPERCLIP_RUNTIME_API_CANDIDATES_JSON;
|
|
const ORIGINAL_PAPERCLIP_LISTEN_HOST = process.env.PAPERCLIP_LISTEN_HOST;
|
|
const ORIGINAL_PAPERCLIP_LISTEN_PORT = process.env.PAPERCLIP_LISTEN_PORT;
|
|
|
|
const {
|
|
createAppMock,
|
|
createBetterAuthInstanceMock,
|
|
createDbMock,
|
|
detectPortMock,
|
|
deriveAuthTrustedOriginsMock,
|
|
feedbackExportServiceMock,
|
|
feedbackServiceFactoryMock,
|
|
fakeServer,
|
|
loadConfigMock,
|
|
} = vi.hoisted(() => {
|
|
const createAppMock = vi.fn(async () => ((_: unknown, __: unknown) => {}) as never);
|
|
const createBetterAuthInstanceMock = vi.fn(() => ({}));
|
|
const createDbMock = vi.fn(() => ({}) as never);
|
|
const detectPortMock = vi.fn(async (port: number) => port);
|
|
const deriveAuthTrustedOriginsMock = vi.fn(() => []);
|
|
const feedbackExportServiceMock = {
|
|
flushPendingFeedbackTraces: vi.fn(async () => ({ attempted: 0, sent: 0, failed: 0 })),
|
|
};
|
|
const feedbackServiceFactoryMock = vi.fn(() => feedbackExportServiceMock);
|
|
const fakeServer = {
|
|
once: vi.fn().mockReturnThis(),
|
|
off: vi.fn().mockReturnThis(),
|
|
listen: vi.fn((_port: number, _host: string, callback?: () => void) => {
|
|
callback?.();
|
|
return fakeServer;
|
|
}),
|
|
close: vi.fn(),
|
|
};
|
|
const loadConfigMock = vi.fn();
|
|
|
|
return {
|
|
createAppMock,
|
|
createBetterAuthInstanceMock,
|
|
createDbMock,
|
|
detectPortMock,
|
|
deriveAuthTrustedOriginsMock,
|
|
feedbackExportServiceMock,
|
|
feedbackServiceFactoryMock,
|
|
fakeServer,
|
|
loadConfigMock,
|
|
};
|
|
});
|
|
|
|
function buildTestConfig(overrides: Record<string, unknown> = {}) {
|
|
return {
|
|
deploymentMode: "authenticated",
|
|
deploymentExposure: "private",
|
|
bind: "loopback",
|
|
customBindHost: undefined,
|
|
host: "127.0.0.1",
|
|
port: 3210,
|
|
allowedHostnames: [],
|
|
authBaseUrlMode: "auto",
|
|
authPublicBaseUrl: undefined,
|
|
authDisableSignUp: false,
|
|
databaseMode: "postgres",
|
|
databaseUrl: "postgres://paperclip:paperclip@127.0.0.1:5432/paperclip",
|
|
embeddedPostgresDataDir: "/tmp/paperclip-test-db",
|
|
embeddedPostgresPort: 54329,
|
|
databaseBackupEnabled: false,
|
|
databaseBackupIntervalMinutes: 60,
|
|
databaseBackupRetentionDays: 30,
|
|
databaseBackupDir: "/tmp/paperclip-test-backups",
|
|
serveUi: false,
|
|
uiDevMiddleware: false,
|
|
secretsProvider: "local_encrypted",
|
|
secretsStrictMode: false,
|
|
secretsMasterKeyFilePath: "/tmp/paperclip-master.key",
|
|
storageProvider: "local_disk",
|
|
storageLocalDiskBaseDir: "/tmp/paperclip-storage",
|
|
storageS3Bucket: "paperclip-test",
|
|
storageS3Region: "us-east-1",
|
|
storageS3Endpoint: undefined,
|
|
storageS3Prefix: "",
|
|
storageS3ForcePathStyle: false,
|
|
feedbackExportBackendUrl: "https://telemetry.example.com",
|
|
feedbackExportBackendToken: "telemetry-token",
|
|
heartbeatSchedulerEnabled: false,
|
|
heartbeatSchedulerIntervalMs: 30000,
|
|
companyDeletionEnabled: false,
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
vi.mock("node:http", () => ({
|
|
createServer: vi.fn(() => fakeServer),
|
|
}));
|
|
|
|
vi.mock("detect-port", () => ({
|
|
default: detectPortMock,
|
|
}));
|
|
|
|
vi.mock("@paperclipai/db", () => ({
|
|
createDb: createDbMock,
|
|
ensurePostgresDatabase: vi.fn(),
|
|
getPostgresDataDirectory: vi.fn(),
|
|
inspectMigrations: vi.fn(async () => ({ status: "upToDate" })),
|
|
applyPendingMigrations: vi.fn(),
|
|
reconcilePendingMigrationHistory: vi.fn(async () => ({ repairedMigrations: [] })),
|
|
formatDatabaseBackupResult: vi.fn(() => "ok"),
|
|
runDatabaseBackup: vi.fn(),
|
|
authUsers: {},
|
|
companies: {},
|
|
companyMemberships: {},
|
|
instanceUserRoles: {},
|
|
}));
|
|
|
|
vi.mock("../app.js", () => ({
|
|
createApp: createAppMock,
|
|
}));
|
|
|
|
vi.mock("../config.js", () => ({
|
|
loadConfig: loadConfigMock,
|
|
}));
|
|
|
|
vi.mock("../middleware/logger.js", () => ({
|
|
logger: {
|
|
child: vi.fn(function child() {
|
|
return this;
|
|
}),
|
|
info: vi.fn(),
|
|
warn: vi.fn(),
|
|
error: vi.fn(),
|
|
},
|
|
}));
|
|
|
|
vi.mock("../realtime/live-events-ws.js", () => ({
|
|
setupLiveEventsWebSocketServer: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("../services/index.js", () => ({
|
|
backfillPrincipalAccessCompatibility: vi.fn(async () => ({
|
|
agentMembershipsInserted: 0,
|
|
humanGrantsInserted: 0,
|
|
})),
|
|
feedbackService: feedbackServiceFactoryMock,
|
|
bootstrapExecutionPolicyFromEnv: vi.fn(async () => null),
|
|
heartbeatService: vi.fn(() => ({
|
|
reapOrphanedRuns: vi.fn(async () => undefined),
|
|
promoteDueScheduledRetries: vi.fn(async () => ({ promoted: 0, runIds: [] })),
|
|
resumeQueuedRuns: vi.fn(async () => undefined),
|
|
reconcileStrandedAssignedIssues: vi.fn(async () => ({
|
|
dispatchRequeued: 0,
|
|
continuationRequeued: 0,
|
|
successfulRunHandoffEscalated: 0,
|
|
escalated: 0,
|
|
skipped: 0,
|
|
issueIds: [],
|
|
})),
|
|
tickTimers: vi.fn(async () => ({ enqueued: 0 })),
|
|
})),
|
|
instanceSettingsService: vi.fn(() => ({
|
|
getGeneral: vi.fn(async () => ({
|
|
backupRetention: {
|
|
dailyDays: 7,
|
|
weeklyWeeks: 4,
|
|
monthlyMonths: 1,
|
|
},
|
|
})),
|
|
})),
|
|
reconcileCloudUpstreamRunsOnStartup: vi.fn(async () => ({ reconciled: 0 })),
|
|
reconcileCodexLocalManagedHomesOnStartup: vi.fn(async () => ({
|
|
scanned: 0,
|
|
seeded: 0,
|
|
alreadySeeded: 0,
|
|
externalOverride: 0,
|
|
noManagedHome: 0,
|
|
sourceAuthMissing: 0,
|
|
failed: 0,
|
|
seededAgentIds: [],
|
|
})),
|
|
reconcilePersistedRuntimeServicesOnStartup: vi.fn(async () => ({ reconciled: 0 })),
|
|
routineService: vi.fn(() => ({
|
|
tickScheduledTriggers: vi.fn(async () => ({ triggered: 0 })),
|
|
})),
|
|
}));
|
|
|
|
vi.mock("../storage/index.js", () => ({
|
|
createStorageServiceFromConfig: vi.fn(() => ({ id: "storage-service" })),
|
|
}));
|
|
|
|
vi.mock("../services/feedback-share-client.js", () => ({
|
|
createFeedbackTraceShareClientFromConfig: vi.fn(() => ({ id: "feedback-share-client" })),
|
|
}));
|
|
|
|
vi.mock("../startup-banner.js", () => ({
|
|
printStartupBanner: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("../board-claim.js", () => ({
|
|
getBoardClaimWarningUrl: vi.fn(() => null),
|
|
initializeBoardClaimChallenge: vi.fn(async () => undefined),
|
|
}));
|
|
|
|
vi.mock("../auth/better-auth.js", () => ({
|
|
createBetterAuthHandler: vi.fn(() => undefined),
|
|
createBetterAuthInstance: createBetterAuthInstanceMock,
|
|
deriveAuthTrustedOrigins: deriveAuthTrustedOriginsMock,
|
|
resolveBetterAuthSession: vi.fn(async () => null),
|
|
resolveBetterAuthSessionFromHeaders: vi.fn(async () => null),
|
|
}));
|
|
|
|
import { startServer } from "../index.ts";
|
|
|
|
describe("startServer feedback export wiring", () => {
|
|
beforeEach(() => {
|
|
vi.clearAllMocks();
|
|
loadConfigMock.mockReturnValue(buildTestConfig());
|
|
createBetterAuthInstanceMock.mockReturnValue({});
|
|
deriveAuthTrustedOriginsMock.mockReturnValue([]);
|
|
process.env.BETTER_AUTH_SECRET = "test-secret";
|
|
});
|
|
|
|
it("passes the feedback export service into createApp so pending traces flush in runtime", async () => {
|
|
const started = await startServer();
|
|
|
|
expect(started.server).toBe(fakeServer);
|
|
expect(feedbackServiceFactoryMock).toHaveBeenCalledTimes(1);
|
|
expect(createAppMock).toHaveBeenCalledTimes(1);
|
|
expect(createAppMock.mock.calls[0]?.[1]).toMatchObject({
|
|
feedbackExportService: feedbackExportServiceMock,
|
|
storageService: { id: "storage-service" },
|
|
serverPort: 3210,
|
|
});
|
|
});
|
|
|
|
it("refuses authenticated public startup without an external database URL", async () => {
|
|
loadConfigMock.mockReturnValue(buildTestConfig({
|
|
deploymentExposure: "public",
|
|
authBaseUrlMode: "explicit",
|
|
authPublicBaseUrl: "https://tenant.example.com",
|
|
databaseMode: "embedded-postgres",
|
|
databaseUrl: undefined,
|
|
}));
|
|
|
|
await expect(startServer()).rejects.toThrow(
|
|
"authenticated public deployments require DATABASE_URL or config.database.connectionString",
|
|
);
|
|
expect(createDbMock).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("refuses authenticated public startup when DATABASE_URL is not a postgres URL", async () => {
|
|
loadConfigMock.mockReturnValue(buildTestConfig({
|
|
deploymentExposure: "public",
|
|
authBaseUrlMode: "explicit",
|
|
authPublicBaseUrl: "https://tenant.example.com",
|
|
databaseUrl: "secret://paperclip-cloud/stacks/alpha/database/runtime-url",
|
|
}));
|
|
|
|
await expect(startServer()).rejects.toThrow(
|
|
"authenticated public deployments require DATABASE_URL to be a postgres/postgresql connection string",
|
|
);
|
|
expect(createDbMock).not.toHaveBeenCalled();
|
|
});
|
|
});
|
|
|
|
describe("startServer authenticated auth origin setup", () => {
|
|
beforeEach(() => {
|
|
vi.clearAllMocks();
|
|
loadConfigMock.mockReturnValue(buildTestConfig());
|
|
createBetterAuthInstanceMock.mockReturnValue({});
|
|
deriveAuthTrustedOriginsMock.mockReturnValue([]);
|
|
process.env.BETTER_AUTH_SECRET = "test-secret";
|
|
});
|
|
|
|
it("derives trusted origins from the detected listen port before auth initializes", async () => {
|
|
loadConfigMock.mockReturnValue(buildTestConfig({
|
|
port: 3210,
|
|
allowedHostnames: ["board.example.test"],
|
|
authBaseUrlMode: "explicit",
|
|
authPublicBaseUrl: "http://127.0.0.1:3210",
|
|
}));
|
|
detectPortMock.mockResolvedValueOnce(3211);
|
|
deriveAuthTrustedOriginsMock.mockImplementation(
|
|
(_config: { port: number; authPublicBaseUrl?: string }, opts?: { listenPort?: number }) => [
|
|
`http://board.example.test:${opts?.listenPort ?? 0}`,
|
|
],
|
|
);
|
|
|
|
await startServer();
|
|
|
|
expect(deriveAuthTrustedOriginsMock).toHaveBeenCalledWith(
|
|
expect.objectContaining({
|
|
port: 3210,
|
|
authPublicBaseUrl: "http://127.0.0.1:3211/",
|
|
}),
|
|
{ listenPort: 3211 },
|
|
);
|
|
expect(createBetterAuthInstanceMock).toHaveBeenCalledWith(
|
|
expect.anything(),
|
|
expect.objectContaining({
|
|
port: 3210,
|
|
authPublicBaseUrl: "http://127.0.0.1:3211/",
|
|
}),
|
|
["http://board.example.test:3211"],
|
|
);
|
|
expect(createAppMock.mock.calls[0]?.[1]).toMatchObject({
|
|
serverPort: 3211,
|
|
});
|
|
});
|
|
});
|
|
|
|
describe("startServer PAPERCLIP_API_URL handling", () => {
|
|
beforeEach(() => {
|
|
vi.clearAllMocks();
|
|
loadConfigMock.mockReturnValue(buildTestConfig());
|
|
process.env.BETTER_AUTH_SECRET = "test-secret";
|
|
delete process.env.PAPERCLIP_API_URL;
|
|
});
|
|
|
|
afterEach(() => {
|
|
if (ORIGINAL_PAPERCLIP_API_URL === undefined) delete process.env.PAPERCLIP_API_URL;
|
|
else process.env.PAPERCLIP_API_URL = ORIGINAL_PAPERCLIP_API_URL;
|
|
|
|
if (ORIGINAL_PAPERCLIP_RUNTIME_API_URL === undefined) delete process.env.PAPERCLIP_RUNTIME_API_URL;
|
|
else process.env.PAPERCLIP_RUNTIME_API_URL = ORIGINAL_PAPERCLIP_RUNTIME_API_URL;
|
|
|
|
if (ORIGINAL_PAPERCLIP_RUNTIME_API_CANDIDATES_JSON === undefined) {
|
|
delete process.env.PAPERCLIP_RUNTIME_API_CANDIDATES_JSON;
|
|
} else {
|
|
process.env.PAPERCLIP_RUNTIME_API_CANDIDATES_JSON = ORIGINAL_PAPERCLIP_RUNTIME_API_CANDIDATES_JSON;
|
|
}
|
|
|
|
if (ORIGINAL_PAPERCLIP_LISTEN_HOST === undefined) delete process.env.PAPERCLIP_LISTEN_HOST;
|
|
else process.env.PAPERCLIP_LISTEN_HOST = ORIGINAL_PAPERCLIP_LISTEN_HOST;
|
|
|
|
if (ORIGINAL_PAPERCLIP_LISTEN_PORT === undefined) delete process.env.PAPERCLIP_LISTEN_PORT;
|
|
else process.env.PAPERCLIP_LISTEN_PORT = ORIGINAL_PAPERCLIP_LISTEN_PORT;
|
|
});
|
|
|
|
it("uses the externally set PAPERCLIP_API_URL when provided", async () => {
|
|
process.env.PAPERCLIP_API_URL = "http://custom-api:3100";
|
|
|
|
const started = await startServer();
|
|
|
|
expect(started.apiUrl).toBe("http://custom-api:3100");
|
|
expect(process.env.PAPERCLIP_API_URL).toBe("http://custom-api:3100");
|
|
expect(JSON.parse(process.env.PAPERCLIP_RUNTIME_API_CANDIDATES_JSON ?? "[]")).toEqual(
|
|
expect.arrayContaining(["http://custom-api:3100"]),
|
|
);
|
|
expect(JSON.parse(process.env.PAPERCLIP_RUNTIME_API_CANDIDATES_JSON ?? "[]")[0]).toBe("http://custom-api:3100");
|
|
});
|
|
|
|
it("falls back to host-based URL when PAPERCLIP_API_URL is not set", async () => {
|
|
const started = await startServer();
|
|
|
|
expect(started.apiUrl).toBe("http://127.0.0.1:3210");
|
|
expect(process.env.PAPERCLIP_API_URL).toBe("http://127.0.0.1:3210");
|
|
});
|
|
|
|
it("keeps loopback as the runtime API URL when allowed hostnames are present", async () => {
|
|
loadConfigMock.mockReturnValueOnce(buildTestConfig({
|
|
allowedHostnames: ["192.168.1.50"],
|
|
}));
|
|
|
|
const started = await startServer();
|
|
|
|
expect(started.apiUrl).toBe("http://127.0.0.1:3210");
|
|
expect(process.env.PAPERCLIP_RUNTIME_API_URL).toBe("http://127.0.0.1:3210");
|
|
expect(process.env.PAPERCLIP_API_URL).toBe("http://127.0.0.1:3210");
|
|
expect(JSON.parse(process.env.PAPERCLIP_RUNTIME_API_CANDIDATES_JSON ?? "[]")).toEqual(
|
|
expect.arrayContaining(["http://127.0.0.1:3210", "http://192.168.1.50:3210"]),
|
|
);
|
|
});
|
|
|
|
it("rewrites explicit-port auth public URLs when detect-port selects a new port", async () => {
|
|
loadConfigMock.mockReturnValueOnce(buildTestConfig({
|
|
port: 3100,
|
|
authBaseUrlMode: "explicit",
|
|
authPublicBaseUrl: "http://my-host.ts.net:3100",
|
|
}));
|
|
detectPortMock.mockResolvedValueOnce(3110);
|
|
|
|
const started = await startServer();
|
|
|
|
expect(started.listenPort).toBe(3110);
|
|
expect(started.apiUrl).toBe("http://my-host.ts.net:3110");
|
|
expect(process.env.PAPERCLIP_RUNTIME_API_URL).toBe("http://my-host.ts.net:3110");
|
|
});
|
|
|
|
it("keeps no-port auth public URLs stable when detect-port selects a new port", async () => {
|
|
loadConfigMock.mockReturnValueOnce(buildTestConfig({
|
|
port: 3100,
|
|
authBaseUrlMode: "explicit",
|
|
authPublicBaseUrl: "https://paperclip.example",
|
|
}));
|
|
detectPortMock.mockResolvedValueOnce(3110);
|
|
|
|
const started = await startServer();
|
|
|
|
expect(started.listenPort).toBe(3110);
|
|
expect(started.apiUrl).toBe("https://paperclip.example");
|
|
expect(process.env.PAPERCLIP_RUNTIME_API_URL).toBe("https://paperclip.example");
|
|
});
|
|
});
|