mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
**Dependency landed:** PR #17129 is merged. This branch is rebased onto master `0f15f82b19` and preserves its provider-owned completion contract. The shared save path calls `notifyLoginSessionChanged(tx)`; the cleanup module invalidates both existing sandbox cleanup workers without selecting a provider. ## Thinking Path > - Paperclip manages work for AI agents. > - Subscription login can allocate a temporary sandbox. > - Existing cleanup workers scan the database every heartbeat tick, even with no login work. > - Empty scans keep idle databases active. > - This PR uses committed session changes and saved expiry times to schedule cleanup. > - Existing rows still drive recovery after failure and restart. ## Linked Issues or Issue Description **What happened?** On master `898144f4b3`, the enabled heartbeat scheduler runs both login reapers every 30 seconds by default. An instance with no pending logins still queries sessions and leases. **What did you expect to happen?** After startup reconciliation, idle login cleanup should make no recurring SQL or provider calls. New sessions, expiry, failed cleanup and restart must still recover. **Steps to reproduce** Start a claimed instance with heartbeat scheduling enabled and no pending login sessions. Observe repeated adapter-login and setup-token cleanup queries at the heartbeat interval. Related sleep migration: #17094. Searched open issues and PRs. Related setup-token cleanup work: #15774, #14236 and #16648. This PR does not change their release-result or null-lease behavior. ## What Changed - Register separate adapter-login and setup-token workers with the existing delivery coordinator and scheduler. - Notify before writes in the owning stores, the lease-tag write, and AI connection save. Shared save uses a provider-neutral notification; each cleanup worker checks its own durable predicates. The shared transaction protocol retains intent until the outer transaction settles. - Reconstruct session expiry, promotion expiry and concrete cleanup retries from existing rows. Preserve configured heartbeat spacing to bound scan bursts. - Recover tagged orphan leases even with terminal or missing sessions. Protect live acquisitions that have not attached their lease yet. - Exclude local-subscription rows from sandbox reapers. Their existing local-login worker owns cleanup. - Preserve deferred work across drain and shutdown. Add no tables, Cloud alarms or host-sleep exemptions. - Update the inventory and document producer coverage, writer scope and recovery boundaries. ## Verification - Updated head: `1d57e13606`. Seven focused suites passed (282 tests): AI connections, login cleanup, device-login service/reaper, login-session runtime, and setup-token session/reaper. `pnpm -r typecheck` and `pnpm build` passed. The duplicate local `pnpm test:run` was interrupted after CI passed all server shards; it is not counted as a complete local pass. Full-suite evidence comes from the exact-head CI run. Greptile reviewed this exact head at 5/5 with no findings or unresolved threads. [Full CI run 39669285577](https://github.com/paperclipai/paperclip/actions/runs/39669285577) passed on attempt 2. Both aggregate gates passed; no checks remain failed or pending. - Extended the real PostgreSQL AI connection tests for both Anthropic and OpenAI. The tests check that neither cleanup observer wakes inside the completion callback, both wake after commit, and an idempotent save does not emit another wake. - Existing cleanup tests cover idle/history states, producer wake, expiry, promotion, cancellation, retry/restart, orphan recovery, nested commit/rollback, drain, concurrency, and shutdown. - Current-head CI attempt 1: the unchanged `sandbox-callback-bridge.test.ts` HTTP/2 preface test failed because its child process could not bind `127.0.0.1:34033` (`EADDRINUSE`). Its single unchanged rerun passed. The exact test also passed locally without changing code or timeouts. This establishes recovery from the bind collision; it does not establish which process owned the port. No login code ran in that package test. - Previous-head failures remain recorded: the login-runtime transaction fixture was corrected; a workspace-runtime readiness port-bind failure passed in isolation and fresh CI, but its first failure cause remains unconfirmed. An earlier combined AI suite timed out in local-login isolation and passed alone; its timing cause remains unconfirmed. An earlier local full suite was interrupted and was not counted as passing. ## Risks - Process-local signals cover one active server with the instrumented shared database handle. Independent SQL writers or replicas need their own signal integration. - Provider resources that never became persisted tagged leases remain the responsibility of the unchanged general lease/spool recovery. Setup-token acquisition retains its required provider expiry. - Cleanup starts are bounded by the configured heartbeat interval. Writes during a sweep can cause a final follow-up scan before quiet. - Existing login and resource blockers still prevent unsafe host sleep. This is worker polling removal, not provider sleep qualification. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools and test execution. The exact serving model ID and context-window size are not exposed in this session. ## 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 references) - [x] My branch name describes the change and contains no internal/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>
193 lines
7.4 KiB
TypeScript
193 lines
7.4 KiB
TypeScript
import { installDatabaseWorkSignals, databaseWorkPending } from "../../../packages/db/src/work-signals.js";
|
|
import { DELIVERY_QUEUES, subscribeDeliveryWork } from "../services/delivery-work-notifications.js";
|
|
import { describe, expect, it, vi } from "vitest";
|
|
import { randomUUID } from "node:crypto";
|
|
import type { AgentAdapterType } from "@paperclipai/shared";
|
|
|
|
// The production runtime resolves the environment through `environmentService`.
|
|
// These tests replace it with a fake, so the runtime runs with no database. The
|
|
// fake returns a fixed environment and records the lease-metadata tag write.
|
|
const mockEnvironmentService = vi.hoisted(() => ({
|
|
getById: vi.fn(),
|
|
updateLeaseMetadata: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("../services/environments.js", () => ({
|
|
environmentService: () => mockEnvironmentService,
|
|
}));
|
|
|
|
import {
|
|
createWorkerBoundLoginPtyOpener,
|
|
createProductionLoginSessionRuntime,
|
|
sessionLoginHomePath,
|
|
DEVICE_LOGIN_PROVIDER_UNSUPPORTED,
|
|
type LoginPtySessionBinding,
|
|
} from "../services/device-login-service.js";
|
|
|
|
// A fake worker pseudo-terminal session. It satisfies the shared transport
|
|
// contract, so the opener can return it.
|
|
function fakeWorkerSession() {
|
|
return {
|
|
onData() {},
|
|
write() {},
|
|
async wait() {
|
|
return { exitCode: 0 };
|
|
},
|
|
kill() {},
|
|
async close() {},
|
|
};
|
|
}
|
|
|
|
// A verified binding for one login session. The runtime builds it after it
|
|
// acquires the lease. The tests set the server-controlled session home.
|
|
function makeBinding(overrides: Partial<LoginPtySessionBinding> = {}): LoginPtySessionBinding {
|
|
return {
|
|
companyId: "company-1",
|
|
environmentId: "env-1",
|
|
adapterType: "codex_local" as AgentAdapterType,
|
|
providerLeaseId: "provider-lease-9",
|
|
sessionHome: sessionLoginHomePath(randomUUID()),
|
|
environment: { id: "env-1", driver: "sandbox" } as never,
|
|
lease: {
|
|
id: "lease-1",
|
|
metadata: { pluginId: "paperclip.daytona", provider: "daytona" },
|
|
} as never,
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
describe("createWorkerBoundLoginPtyOpener", () => {
|
|
it("passes the binding's server-controlled session home to the worker route", async () => {
|
|
const sessionHome = sessionLoginHomePath(randomUUID());
|
|
const session = fakeWorkerSession();
|
|
const openLoginPtySession = vi.fn(
|
|
async (_pluginId: string, _input: Record<string, unknown>) => session,
|
|
);
|
|
const openLivePtySession = createWorkerBoundLoginPtyOpener({
|
|
workerManager: { openLoginPtySession },
|
|
});
|
|
|
|
const opener = await openLivePtySession(makeBinding({ sessionHome }));
|
|
// The opener ignores the incoming command argument. It resolves the closed
|
|
// command key from the trusted adapter type.
|
|
const opened = await opener("caller-supplied-ignored");
|
|
|
|
expect(openLoginPtySession).toHaveBeenCalledTimes(1);
|
|
const [pluginId, input] = openLoginPtySession.mock.calls[0];
|
|
expect(pluginId).toBe("paperclip.daytona");
|
|
expect(input).toMatchObject({
|
|
driverKey: "daytona",
|
|
companyId: "company-1",
|
|
environmentId: "env-1",
|
|
providerLeaseId: "provider-lease-9",
|
|
loginCommandKey: "codex",
|
|
// The opener passes the binding's server-controlled home, not a fresh UUID
|
|
// home. This home is the one the descriptor-bound credential read opens, so
|
|
// the sandbox `CODEX_HOME` and the read target match.
|
|
sessionHome,
|
|
});
|
|
expect(input).not.toHaveProperty("command");
|
|
expect(opened).toBe(session);
|
|
});
|
|
|
|
it("fails closed when the lease carries no sandbox worker binding", async () => {
|
|
const openLoginPtySession = vi.fn();
|
|
const openLivePtySession = createWorkerBoundLoginPtyOpener({
|
|
workerManager: { openLoginPtySession },
|
|
});
|
|
|
|
await expect(
|
|
openLivePtySession(makeBinding({ lease: { id: "lease-1", metadata: {} } as never })),
|
|
).rejects.toThrow();
|
|
expect(openLoginPtySession).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("fails closed when the adapter type has no login command key", async () => {
|
|
const openLoginPtySession = vi.fn();
|
|
const openLivePtySession = createWorkerBoundLoginPtyOpener({
|
|
workerManager: { openLoginPtySession },
|
|
});
|
|
|
|
await expect(
|
|
openLivePtySession(makeBinding({ adapterType: "gemini_local" as AgentAdapterType })),
|
|
).rejects.toThrow();
|
|
expect(openLoginPtySession).not.toHaveBeenCalled();
|
|
});
|
|
});
|
|
|
|
describe("createProductionLoginSessionRuntime lease-acquisition gate", () => {
|
|
const acquireInput = {
|
|
companyId: "company-1",
|
|
environmentId: "env-1",
|
|
adapterType: "codex_local" as AgentAdapterType,
|
|
startedByUserId: "user-a",
|
|
};
|
|
|
|
it("re-checks the provider capability and fails before the provider lease when it is unsupported", async () => {
|
|
mockEnvironmentService.getById.mockResolvedValue({
|
|
id: "env-1",
|
|
driver: "sandbox",
|
|
config: { provider: "daytona" },
|
|
});
|
|
const acquireRunLease = vi.fn();
|
|
// The capability flipped to unsupported after the route gate. The lease gate
|
|
// reads the current capability and fails closed.
|
|
const assertProviderSupportsLoginPty = vi.fn(async () => {
|
|
throw new Error(DEVICE_LOGIN_PROVIDER_UNSUPPORTED);
|
|
});
|
|
const runtime = createProductionLoginSessionRuntime({
|
|
db: {} as never,
|
|
environmentRuntime: { acquireRunLease, getDriver: vi.fn() } as never,
|
|
assertProviderSupportsLoginPty,
|
|
});
|
|
|
|
await expect(
|
|
runtime.acquireLoginLease({ ...acquireInput, sessionId: randomUUID() }),
|
|
).rejects.toThrow(DEVICE_LOGIN_PROVIDER_UNSUPPORTED);
|
|
// The runtime re-checked the current capability from the environment id.
|
|
expect(assertProviderSupportsLoginPty).toHaveBeenCalledWith("env-1");
|
|
// The gate failed before the provider lease, so no lease was acquired.
|
|
expect(acquireRunLease).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it("acquires the provider lease after the capability check passes", async () => {
|
|
mockEnvironmentService.getById.mockResolvedValue({
|
|
id: "env-1",
|
|
driver: "sandbox",
|
|
config: { provider: "daytona" },
|
|
});
|
|
const execute = vi.fn(async () => [{ xid: "42" }]);
|
|
const db = installDatabaseWorkSignals({
|
|
execute,
|
|
async transaction<T>(fn: (tx: object) => Promise<T>) { return fn({ execute }); },
|
|
});
|
|
const settled = vi.fn();
|
|
const unsubscribe = subscribeDeliveryWork(db, DELIVERY_QUEUES.adapterLoginCleanup, settled);
|
|
mockEnvironmentService.updateLeaseMetadata.mockImplementation(async () => {
|
|
expect(databaseWorkPending(db, DELIVERY_QUEUES.adapterLoginCleanup)).toBe(true);
|
|
expect(settled).not.toHaveBeenCalled();
|
|
});
|
|
const lease = { id: "lease-1", providerLeaseId: "provider-lease-1", metadata: {} };
|
|
const acquireRunLease = vi.fn(async () => ({
|
|
lease,
|
|
environment: { id: "env-1", driver: "sandbox" },
|
|
}));
|
|
const assertProviderSupportsLoginPty = vi.fn(async () => {});
|
|
const runtime = createProductionLoginSessionRuntime({
|
|
db: db as never,
|
|
environmentRuntime: { acquireRunLease, getDriver: vi.fn() } as never,
|
|
assertProviderSupportsLoginPty,
|
|
});
|
|
|
|
const acquired = await runtime.acquireLoginLease({ ...acquireInput, sessionId: randomUUID() });
|
|
|
|
expect(assertProviderSupportsLoginPty).toHaveBeenCalledWith("env-1");
|
|
// The gate passed, so the runtime acquired the provider lease after it.
|
|
expect(acquireRunLease).toHaveBeenCalledTimes(1);
|
|
expect(acquired.providerLeaseId).toBe("provider-lease-1");
|
|
expect(settled).toHaveBeenCalledTimes(1);
|
|
expect(databaseWorkPending(db, DELIVERY_QUEUES.adapterLoginCleanup)).toBe(false);
|
|
unsubscribe();
|
|
});
|
|
});
|