From 67a9d5cc37a0b134a8a6899ab52bde5525bedbf9 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:01:57 -0500 Subject: [PATCH] perf(runner): bypass skill cache waits for empty selections (#16498) Native runs with no skill files no longer queue behind company skill-cache work. Preserve saved empty selections and the existing exclusion for selected and legacy operational skills. Verified with real PostgreSQL held-lock regressions, 20 distinct affected cases, full repository typecheck/build, fresh full CI/E2E/security and exact-head Apex 5/5. Hosted latency remains to be verified. Co-Authored-By: Paperclip --- doc/DEVELOPING.md | 5 ++ maps/agent-access-map/skills.md | 7 ++ maps/feature-map/skills.md | 6 ++ .../heartbeat-execution-configuration.test.ts | 73 +++++++++++++++++++ .../heartbeat/execution-configuration.ts | 14 +++- 5 files changed, 101 insertions(+), 4 deletions(-) diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index d618afea52c..8a9bfe8d0b5 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -2022,6 +2022,11 @@ to unique managed `.trash-` names. Deletion runs outside the preparation lock and resumes after a crash. Saved version and repository snapshot rows are retained, and normal runtime use restores missing caches without upstream access. +Paperclip Runner preparation skips this lock when the effective selection has no +skills or a saved selection is explicitly empty. It still records the empty +version snapshot. Explicit skill references, selected versions, and the legacy +operational skill keep their existing authorization and cache-lock requirements. + The sweep also handles orphan version directories after explicit skill deletion, preserving captured run pins until they settle. Only recognized staging/trash names are eligible; unknown directories and symlinked paths are retained. diff --git a/maps/agent-access-map/skills.md b/maps/agent-access-map/skills.md index 7f1051511af..1a374200e2c 100644 --- a/maps/agent-access-map/skills.md +++ b/maps/agent-access-map/skills.md @@ -95,6 +95,13 @@ internal company-scoped maintenance operation, with no new agent endpoint. Verif restoration and active-run protection with `server/src/__tests__/skill-version-cache.test.ts` and the assembled heartbeat runtime-skill tests; live acceptance remains separate. +Native runs with an empty effective or saved selection do not read skill files +or wait for the company cache lock. Explicit references still require the existing +skill authorization. Selected skills and legacy operational skills retain cache +exclusion and immutable version pinning. This changes no agent endpoint or grant; +the [configuration tests](../../server/src/__tests__/heartbeat-execution-configuration.test.ts) +cover the held-lock boundary with real PostgreSQL. + ## `policy` Review: `baseline`. diff --git a/maps/feature-map/skills.md b/maps/feature-map/skills.md index 216bc7203e4..184d799560b 100644 --- a/maps/feature-map/skills.md +++ b/maps/feature-map/skills.md @@ -57,6 +57,12 @@ Automated: [policy service](../../server/src/__tests__/company-skill-policy-serv Manual: Enable the skill for one test agent and leave another without access. Run a small task on each, inspect the available skill context, then change the release/policy and verify the next eligible run uses the intended version. +A Paperclip Runner run with no selected skills must proceed while another run +holds the company skill-cache lock. A saved empty selection remains empty after +later assignments. Selected skills and the legacy operational skill retain cache +exclusion through version pinning. The [configuration regression](../../server/src/__tests__/heartbeat-execution-configuration.test.ts) +uses a real PostgreSQL lock; live provider latency remains a separate check. + ## Gotchas - The release picker requires `enableBetaSkills`. Do not invent an Instructions diff --git a/server/src/__tests__/heartbeat-execution-configuration.test.ts b/server/src/__tests__/heartbeat-execution-configuration.test.ts index 42da9043535..c429b0416a3 100644 --- a/server/src/__tests__/heartbeat-execution-configuration.test.ts +++ b/server/src/__tests__/heartbeat-execution-configuration.test.ts @@ -9,6 +9,10 @@ import { eq } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { agents, companies, companyMemberships, companySkills, createDb, heartbeatRunEvents, heartbeatRuns, issues, projects } from "@paperclipai/db"; import { companySkillService } from "../services/company-skills.js"; +import { secretService } from "../services/secrets.js"; +import { instanceSettingsService } from "../services/instance-settings.js"; +import { withSkillVersionCacheLock } from "../services/skill-version-cache.js"; +import { prepareHeartbeatExecutionConfiguration } from "../services/heartbeat/execution-configuration.js"; import { registerServerAdapter, unregisterServerAdapter, type AdapterExecutionContext } from "../adapters/index.js"; import { HttpError } from "../errors.js"; import { heartbeatService } from "../services/heartbeat.js"; @@ -103,6 +107,75 @@ describePostgres("heartbeat execution configuration through adapter dispatch", ( return { finished, input: captured.get(run!.id) }; } + async function configuration(f: Awaited>, config: Record, snapshot: Record = {}, type = "paperclip_runner") { + const [agent] = await db.update(agents).set({ adapterType: type, adapterConfig: config }).where(eq(agents.id, f.agentId)).returning(); + const [run] = await db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, issueId: f.issueId, scopeKind: "issue", status: "running", contextSnapshot: snapshot }).returning(); + const resolvedInstanceSettings = await instanceSettingsService(db).get(); + return { run, prepare: () => prepareHeartbeatExecutionConfiguration(db, { + run, agent, + task: { issueId: f.issueId, context: { ...snapshot, issueId: f.issueId }, responsibleUserId: null, + acceptedPlanContinuationWake: false, acceptedPlanWakeRoutingDecision: null, + taskProjectContext: null, routineEnvContext: { routineId: null, env: null, responsibleUserId: null } }, + config: { sourceWorkspaceConfig: config, aiBinding: undefined }, + policy: { trustPreset: { kind: "standard", preset: "standard", boundary: null, sourcePresets: {} }, useIsolatedTaskDirectory: false, explicitTaskDirectory: false }, + environment: { selectedEnvironmentId: null, selectedEnvironmentForConfig: null, + resolvedInstanceSettings, persistedNativeExecutionInput: null }, + services: { secretsSvc: secretService(db), companySkills: companySkillService(db), appendRunEvent: vi.fn(), finalizeAiConnectionBusyDeferral: vi.fn() }, + onFailureSecretsResolved: () => {}, onManagedAiRuntimeAcquired: () => {}, onIdentityRedactorPrepared: () => {}, + }) }; + } + + it.each(["fresh", "saved empty"])("prepares a %s native skill selection while another run holds the company cache lock", async kind => { + const f = await seed(); + const selected = kind === "saved empty" ? await companySkillService(db).createLocalSkill(f.companyId, { name: "Later assignment" }) : null; + const input = await configuration(f, selected ? { paperclipSkillSync: { desiredSkills: [selected.key] } } : {}, + kind === "saved empty" ? { skillVersions: {}, skillSelectionSnapshot: true } : {}); + let release!: () => void; + let acquired!: () => void; + const ready = new Promise(resolve => { acquired = resolve; }); + const barrier = new Promise(resolve => { release = resolve; }); + const holder = withSkillVersionCacheLock(db, f.companyId, async () => { acquired(); await barrier; }); + await ready; + const started = performance.now(); + const pending = input.prepare(); + let timer: ReturnType | undefined; + try { + const result = await Promise.race([pending, new Promise(resolve => { timer = setTimeout(() => resolve(null), 2000); })]); + expect(result, "An empty native selection must not queue behind skill hydration").not.toBeNull(); + console.info("Empty-skill preparation under a held cache lock", { kind, elapsedMs: performance.now() - started }); + expect(result?.prepared).toBe(true); + if (result?.prepared) expect(result.runtimeSkillEntries).toEqual([]); + const [saved] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, input.run.id)); + expect(saved.contextSnapshot).toMatchObject({ skillVersions: {}, skillSelectionSnapshot: true }); + } finally { + if (timer) clearTimeout(timer); + release(); + await holder; + await pending; + } + }); + + it.each(["selected native", "legacy operational"])("retains the cache exclusion for %s skills", async kind => { + const f = await seed(); + const selected = await companySkillService(db).createLocalSkill(f.companyId, { name: "Selected lock control" }); + const input = await configuration(f, { paperclipSkillSync: { desiredSkills: kind === "selected native" ? [selected.key] : [] } }, {}, kind === "selected native" ? "paperclip_runner" : "codex_local"); + let release!: () => void; + let acquired!: () => void; + const ready = new Promise(resolve => { acquired = resolve; }); + const barrier = new Promise(resolve => { release = resolve; }); + const holder = withSkillVersionCacheLock(db, f.companyId, async () => { acquired(); await barrier; }); + await ready; + let settled = false; + const pending = input.prepare().finally(() => { settled = true; }); + try { + await new Promise(resolve => setTimeout(resolve, 100)); + expect(settled).toBe(false); + } finally { release(); await holder; } + const result = await pending; + expect(result.prepared).toBe(true); + if (result.prepared) expect(result.runtimeSkillEntries.some(entry => entry.key === (kind === "selected native" ? selected.key : "paperclipai/paperclip/paperclip"))).toBe(true); + }); + it("applies project environment and issue overrides and replaces caller-supplied runtime fields", async () => { const f = await seed(); const { finished, input } = await execute(f); diff --git a/server/src/services/heartbeat/execution-configuration.ts b/server/src/services/heartbeat/execution-configuration.ts index f55f8068ab9..b9a830ddf02 100644 --- a/server/src/services/heartbeat/execution-configuration.ts +++ b/server/src/services/heartbeat/execution-configuration.ts @@ -209,10 +209,13 @@ export async function prepareHeartbeatExecutionConfiguration(db: Db, input: Hear const nativeRunnerPreparationSpans: NativeRunHistoricalSpan[] = []; const skillsPrepareStartedAtMs = Date.now(); let skillDeliveryConfig = effectiveResolvedConfig; - const runtimeSkillEntries = await time("configuration_skills", () => withSkillVersionCacheLock(db, agent.companyId, async () => { + const storedVersions = parseObject(run.contextSnapshot?.skillVersions); + const hasStoredVersions = Object.prototype.hasOwnProperty.call(run.contextSnapshot ?? {}, "skillVersions"); + const emptySkillSelection = hasStoredVersions + ? run.contextSnapshot?.skillSelectionSnapshot === true && Object.keys(storedVersions).length === 0 + : agent.adapterType === "paperclip_runner" && runtimeSkillPreference.desiredSkills.length === 0; + const prepareSkills = async () => { try { - const storedVersions = parseObject(run.contextSnapshot?.skillVersions); - const hasStoredVersions = Object.prototype.hasOwnProperty.call(run.contextSnapshot ?? {}, "skillVersions"); const versionSelections = hasStoredVersions ? new Map(Object.entries(storedVersions).filter((entry): entry is [string, string | null] => entry[1] === null || typeof entry[1] === "string")) : skillVersionSelectionMap(runtimeSkillPreference.desiredSkillEntries, { @@ -264,7 +267,10 @@ export async function prepareHeartbeatExecutionConfiguration(db: Db, input: Hear } throw error; } - })); + }; + const runtimeSkillEntries = await time("configuration_skills", () => emptySkillSelection + ? prepareSkills() + : withSkillVersionCacheLock(db, agent.companyId, prepareSkills)); nativeRunnerPreparationSpans.push({ name: "skills.prepare", parentName: "task.prepare",