mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-22 00:30:19 +02:00
Keep Muse maintenance idle on unclaimed warm instances (#16974)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Cloud keeps unclaimed applications warm so new users can enter quickly. > - Empty warm applications use an existing standby predicate to skip recurring database work. > - Muse maintenance starts without that predicate and scans its tables every five seconds. > - These scans keep an empty database active even when no Muse binding exists. > - This pull request applies the existing standby predicate to Muse startup and recurring maintenance. > - The benefit is idle database compute while the application stays warm, with normal maintenance after claim. ## Linked Issues or Issue Description **What happened?** `createApp` starts Muse maintenance without the warm standby guard used by other background services. The worker updates qualification evidence at startup and reads binding tables every five seconds, including on an empty unclaimed instance. **Expected behavior** An unclaimed warm application must not run recurring database work. Muse maintenance must resume after the signed claim, without a process restart. **Steps to reproduce** 1. Start the maintenance service with background work disabled. 2. Advance the timer beyond several five-second sweeps. 3. Observe database calls from the original implementation despite the disabled state. The new regression test uses real PostgreSQL. The original source fails its zero-query assertion; the patched source passes. **Paperclip version or commit** Reproduced on `813e808af46` before this fix. Related: Refs #16448. ## What Changed - Pass the Cloud standby predicate to Muse maintenance. - Admit both the startup evidence update and recurring sweeps only when background work is enabled. - Mark startup evidence once, before reconciliation. Retry that write after a database failure. - Add PostgreSQL tests for standby, claim, later ticks, default deadline enforcement, and startup-write failure. - Document the maintenance boundary. ## Verification - Five focused test files passed: 278 tests. They cover Muse maintenance, Muse control and qualification routes, and Cloud standby behavior. - Controlled regression: the new standby test fails against the original maintenance source. The patched source was restored after the comparison. - The new PostgreSQL test advances ten minutes while admission is closed and observes zero database operations. It opens admission and verifies the persisted qualification evidence update and reconciliation. Later sweeps retain a single successful startup update. - `pnpm check:module-boundaries` passed. - `pnpm -r typecheck` passed. - `pnpm build` passed. - All 54 CI checks passed; two conditional jobs were skipped. - Apex review: 5/5, no actionable comments or unresolved threads. - The full local test command is still running. The focused tests, workspace typecheck, build, and complete CI matrix above are the finished checks. ## Risks - Claimed workspaces retain existing Muse maintenance. The app gate depends only on warm standby. It does not suppress deadline enforcement or terminal cleanup during idle task drain. - Work resumes on the next five-second maintenance tick after claim. Request-driven work remains available through the existing claim path. - The receiver keeps its existing lifecycle. Its empty pending-contact map causes no database writes in standby. - No database migration, provider configuration, or authority rule changes. - Broker and receiver effects are mocked in the new maintenance tests. The database operations are real. Existing Muse control tests cover the broker boundary; no live Muse provider qualification is claimed. ## Model Used - OpenAI GPT-6 through Codex, with tool-assisted source inspection, implementation, and test execution. The exact model variant and context window 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 `#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:
1 parent
4b05795712
commit
ba3bdd0675
4 files changed
+151
-8
No files matched your search
+6
-1
@@ -400,7 +400,7 @@ keep the application fully active. A failed database check fails startup.
|
||||
|
||||
An empty unclaimed application keeps its HTTP server and sandbox plugins ready,
|
||||
but skips recurring database work: chat/email delivery, plugin jobs, browser
|
||||
cleanup, feedback export, import cleanup, execution reconciliation, heartbeat
|
||||
cleanup, feedback export, import cleanup, execution reconciliation, Muse maintenance, heartbeat
|
||||
schedules, and automatic backups. Startup migrations, plugin installation, and
|
||||
other one-time preparation still run. Normal anonymous health probes return
|
||||
`warmStandby: true` without SQL or session lookups. This is application liveness,
|
||||
@@ -408,6 +408,11 @@ not a current database connectivity check. Other API requests and WebSocket upgr
|
||||
the signed claim succeeds. Page and asset requests serve only the static UI
|
||||
router, bypassing session, bearer-key, tenant, and other dynamic handlers.
|
||||
|
||||
Muse defers its startup qualification-evidence update and recurring database
|
||||
sweeps until standby ends. The first enabled sweep marks interrupted evidence
|
||||
before it reconciles qualification deadlines or terminal assignments. Claimed
|
||||
workspaces retain ordinary Muse maintenance even when idle task drain is active.
|
||||
|
||||
The existing signed claim on `GET /api/health` writes the identity durably before
|
||||
normal requests and polling resume. No polling discovers claims and no process
|
||||
restart is required. Timers resume on their next normal tick; request-driven
|
||||
|
||||
+3
-1
@@ -624,7 +624,9 @@ export async function createApp(
|
||||
app.use(cloudWarmStandbyMiddleware(isWarmStandby, health, staticUi));
|
||||
app.use("/api/customer-success/v1", customerSuccessRoutes(db, opts.storageService));
|
||||
app.use(museTransportRoutes(db, opts.authPublicBaseUrl));
|
||||
const stopMuseMaintenance = startMuseMaintenance(db);
|
||||
const stopMuseMaintenance = startMuseMaintenance(db, {
|
||||
isBackgroundWorkEnabled: () => !isWarmStandby(),
|
||||
});
|
||||
app.use(publicMcpIngress);
|
||||
// Connection-intent tools carry their own short-lived, run-bound bearer and
|
||||
// must be reachable by remote adapters that intentionally do not receive an
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { eq, sql } from "drizzle-orm";
|
||||
import { agents, companies, createDb, museAgentBindings } from "@paperclipai/db";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js";
|
||||
import { startMuseMaintenance } from "./muse-maintenance.js";
|
||||
|
||||
const effects = vi.hoisted(() => ({
|
||||
endQualification: vi.fn().mockResolvedValue(undefined),
|
||||
revokeOAuthGrant: vi.fn().mockResolvedValue(undefined),
|
||||
reconcileTerminalAssignments: vi.fn().mockResolvedValue(undefined),
|
||||
start: vi.fn(),
|
||||
stop: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("./muse-runner-broker.js", () => ({ museRunnerBroker: () => effects }));
|
||||
vi.mock("./muse-receiver.js", () => ({ museReceiver: () => effects }));
|
||||
|
||||
const support = await getEmbeddedPostgresTestSupport();
|
||||
const describeDatabase = support.supported ? describe : describe.skip;
|
||||
|
||||
describeDatabase("Muse maintenance background admission", () => {
|
||||
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
||||
let db: ReturnType<typeof createDb>;
|
||||
beforeAll(async () => {
|
||||
database = await startEmbeddedPostgresTestDatabase("muse-maintenance-");
|
||||
db = createDb(database.connectionString, { maxConnections: 1 });
|
||||
});
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.useFakeTimers({ toFake: ["setInterval", "clearInterval"] });
|
||||
});
|
||||
afterEach(async () => {
|
||||
vi.useRealTimers();
|
||||
await db.execute(sql`TRUNCATE TABLE companies CASCADE`);
|
||||
});
|
||||
afterAll(async () => {
|
||||
await db?.$client.end({ timeout: 1 });
|
||||
await database?.cleanup();
|
||||
});
|
||||
|
||||
async function fixture(expired = false) {
|
||||
const [company] = await db.insert(companies).values({ name: "Maintenance", issuePrefix: `MT${randomUUID().slice(0, 6)}` }).returning();
|
||||
const [agent] = await db.insert(agents).values({ companyId: company.id, name: "Muse agent" }).returning();
|
||||
const [binding] = await db.insert(museAgentBindings).values({
|
||||
companyId: company.id,
|
||||
agentId: agent.id,
|
||||
operatorId: "synthetic-operator",
|
||||
qualificationId: randomUUID(),
|
||||
qualificationExpiresAt: new Date(Date.now() + (expired ? -1 : 1) * 3_600_000),
|
||||
}).returning();
|
||||
return binding;
|
||||
}
|
||||
|
||||
function observeDatabase(failFirstUpdate = false) {
|
||||
const calls: string[] = [];
|
||||
const observed = new Proxy(db, { get(target, key) {
|
||||
if (!["select", "update", "insert", "delete", "execute", "transaction"].includes(String(key))) {
|
||||
return Reflect.get(target, key);
|
||||
}
|
||||
return (...args: unknown[]) => {
|
||||
calls.push(String(key));
|
||||
if (key === "update" && failFirstUpdate) {
|
||||
failFirstUpdate = false;
|
||||
throw new Error("temporary database failure");
|
||||
}
|
||||
return Reflect.apply(Reflect.get(target, key), target, args);
|
||||
};
|
||||
} });
|
||||
return { observed, calls };
|
||||
}
|
||||
|
||||
it("leaves PostgreSQL untouched in standby and resumes maintenance after claim", async () => {
|
||||
const binding = await fixture();
|
||||
const { observed, calls } = observeDatabase();
|
||||
let enabled = false;
|
||||
const stop = startMuseMaintenance(observed, { isBackgroundWorkEnabled: () => enabled });
|
||||
try {
|
||||
await vi.advanceTimersByTimeAsync(10 * 60_000);
|
||||
expect(calls).toEqual([]);
|
||||
expect(effects.reconcileTerminalAssignments).not.toHaveBeenCalled();
|
||||
enabled = true;
|
||||
await vi.advanceTimersByTimeAsync(5000);
|
||||
await vi.waitFor(() => expect(effects.reconcileTerminalAssignments).toHaveBeenCalledWith(binding.companyId, binding.agentId, binding.id));
|
||||
const [saved] = await db.select().from(museAgentBindings).where(eq(museAgentBindings.id, binding.id));
|
||||
expect(saved.cadenceEvidenceIncompleteAt).toBeInstanceOf(Date);
|
||||
expect(calls.filter(call => call === "update")).toHaveLength(1);
|
||||
enabled = false;
|
||||
const settledCalls = [...calls];
|
||||
await vi.advanceTimersByTimeAsync(10 * 60_000);
|
||||
expect(calls).toEqual(settledCalls);
|
||||
enabled = true;
|
||||
await vi.advanceTimersByTimeAsync(5000);
|
||||
await vi.waitFor(() => expect(effects.reconcileTerminalAssignments).toHaveBeenCalledTimes(2));
|
||||
expect(calls.filter(call => call === "update")).toHaveLength(1);
|
||||
} finally {
|
||||
await stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps ordinary startup and qualification deadline enforcement enabled by default", async () => {
|
||||
const binding = await fixture(true);
|
||||
const stop = startMuseMaintenance(db);
|
||||
try {
|
||||
await vi.waitFor(() => expect(effects.endQualification).toHaveBeenCalledWith({
|
||||
companyId: binding.companyId,
|
||||
agentId: binding.agentId,
|
||||
bindingId: binding.id,
|
||||
qualificationId: binding.qualificationId,
|
||||
operatorId: binding.operatorId,
|
||||
}));
|
||||
await vi.waitFor(() => expect(effects.reconcileTerminalAssignments).toHaveBeenCalled());
|
||||
} finally {
|
||||
await stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("retries startup evidence marking before reconciliation after a database failure", async () => {
|
||||
const binding = await fixture();
|
||||
const { observed, calls } = observeDatabase(true);
|
||||
const stop = startMuseMaintenance(observed);
|
||||
try {
|
||||
await Promise.resolve();
|
||||
expect(effects.reconcileTerminalAssignments).not.toHaveBeenCalled();
|
||||
await vi.advanceTimersByTimeAsync(5000);
|
||||
await vi.waitFor(() => expect(effects.reconcileTerminalAssignments).toHaveBeenCalled());
|
||||
const [saved] = await db.select().from(museAgentBindings).where(eq(museAgentBindings.id, binding.id));
|
||||
expect(saved.cadenceEvidenceIncompleteAt).toBeInstanceOf(Date);
|
||||
expect(calls.filter(call => call === "update")).toHaveLength(2);
|
||||
} finally {
|
||||
await stop();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -4,12 +4,17 @@ import { museAgentBindings as bindings, mcpOauthGrants, type Db } from "@papercl
|
||||
import { museRunnerBroker } from "./muse-runner-broker.js";
|
||||
import { museReceiver } from "./muse-receiver.js";
|
||||
/** Deadline authority lives in the database; every fresh process recovers it. */
|
||||
export function startMuseMaintenance(db:Db) {
|
||||
export function startMuseMaintenance(db:Db, options: { isBackgroundWorkEnabled?: () => boolean } = {}) {
|
||||
const broker=museRunnerBroker(db),receiver=museReceiver(db);
|
||||
let running=false;
|
||||
const isBackgroundWorkEnabled=options.isBackgroundWorkEnabled??(()=>true);
|
||||
let running=false, startupMarked=false;
|
||||
const sweep=async()=>{
|
||||
if(running)return;running=true;
|
||||
if(running||!isBackgroundWorkEnabled())return;running=true;
|
||||
try{
|
||||
if(!startupMarked){
|
||||
await db.update(bindings).set({cadenceEvidenceIncompleteAt:new Date()}).where(and(isNull(bindings.revokedAt),gt(bindings.qualificationExpiresAt,new Date())));
|
||||
startupMarked=true;
|
||||
}
|
||||
let after:string|undefined;
|
||||
while(true) {
|
||||
const expired=await db.select().from(bindings).where(and(isNull(bindings.revokedAt),lte(bindings.qualificationExpiresAt,new Date()),after?gt(bindings.id,after):undefined)).orderBy(asc(bindings.id)).limit(100);
|
||||
@@ -30,9 +35,7 @@ export function startMuseMaintenance(db:Db) {
|
||||
}
|
||||
}finally{running=false;}
|
||||
};
|
||||
// A process restart loses any not-yet-persisted ingress tail. It is explicit
|
||||
// incomplete qualification evidence even when subsequent traffic is healthy.
|
||||
void db.update(bindings).set({cadenceEvidenceIncompleteAt:new Date()}).where(and(isNull(bindings.revokedAt),gt(bindings.qualificationExpiresAt,new Date()))).then(()=>sweep()).catch(()=>{logger.warn("Muse startup qualification evidence marking failed.");});
|
||||
void sweep().catch(()=>{logger.warn("Muse startup qualification evidence marking failed.");});
|
||||
const timer=setInterval(()=>void sweep().catch(()=>{logger.warn("Muse maintenance sweep failed; persisted authority checks remain active.");}),5000);timer.unref();receiver.start();
|
||||
return async()=>{clearInterval(timer);await receiver.stop();};
|
||||
}
|
||||
Reference in new issue
Block a user