From 3c66683169b5f2c8da68d7a78c05748c2d8e799e Mon Sep 17 00:00:00 2001 From: dotta Date: Mon, 30 Mar 2026 08:26:14 -0500 Subject: [PATCH] Fix execution workspace reuse and slugify worktrees Co-Authored-By: Paperclip --- .../heartbeat-workspace-session.test.ts | 46 ++++++ .../src/__tests__/workspace-runtime.test.ts | 37 +++++ server/src/services/heartbeat.ts | 131 ++++++++++++------ server/src/services/workspace-runtime.ts | 8 +- 4 files changed, 176 insertions(+), 46 deletions(-) diff --git a/server/src/__tests__/heartbeat-workspace-session.test.ts b/server/src/__tests__/heartbeat-workspace-session.test.ts index ca23d907..47d37c53 100644 --- a/server/src/__tests__/heartbeat-workspace-session.test.ts +++ b/server/src/__tests__/heartbeat-workspace-session.test.ts @@ -4,6 +4,7 @@ import { sessionCodec as codexSessionCodec } from "@paperclipai/adapter-codex-lo import { resolveDefaultAgentWorkspaceDir } from "../home-paths.js"; import { applyPersistedExecutionWorkspaceConfig, + buildRealizedExecutionWorkspaceFromPersisted, buildExplicitResumeSessionOverride, formatRuntimeWorkspaceWarningLog, prioritizeProjectWorkspaceCandidatesForRun, @@ -154,6 +155,51 @@ describe("applyPersistedExecutionWorkspaceConfig", () => { }); }); +describe("buildRealizedExecutionWorkspaceFromPersisted", () => { + it("reuses the persisted execution workspace path instead of deriving a new worktree", () => { + const result = buildRealizedExecutionWorkspaceFromPersisted({ + base: buildResolvedWorkspace({ + cwd: "/tmp/project-primary", + repoRef: "main", + }), + workspace: { + id: "execution-workspace-1", + companyId: "company-1", + projectId: "project-1", + projectWorkspaceId: "workspace-1", + sourceIssueId: "issue-1", + mode: "isolated_workspace", + strategyType: "git_worktree", + name: "PAP-880-thumbs-capture-for-evals-feature", + status: "active", + cwd: "/tmp/reused-worktree", + repoUrl: "https://example.com/paperclip.git", + baseRef: "main", + branchName: "PAP-880-thumbs-capture-for-evals-feature", + providerType: "git_worktree", + providerRef: "/tmp/reused-worktree", + derivedFromExecutionWorkspaceId: null, + lastUsedAt: new Date(), + openedAt: new Date(), + closedAt: null, + cleanupEligibleAt: null, + cleanupReason: null, + config: null, + metadata: null, + createdAt: new Date(), + updatedAt: new Date(), + }, + }); + + expect(result.created).toBe(false); + expect(result.strategy).toBe("git_worktree"); + expect(result.cwd).toBe("/tmp/reused-worktree"); + expect(result.worktreePath).toBe("/tmp/reused-worktree"); + expect(result.branchName).toBe("PAP-880-thumbs-capture-for-evals-feature"); + expect(result.source).toBe("task_session"); + }); +}); + describe("stripWorkspaceRuntimeFromExecutionRunConfig", () => { it("removes workspace runtime before heartbeat execution", () => { const input = { diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index 1af2eea1..573013be 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -247,6 +247,43 @@ describe("realizeExecutionWorkspace", () => { expect(second.branchName).toBe(first.branchName); }); + it("slugifies unsafe issue titles for branch names and worktree folders", async () => { + const repoRoot = await createTempRepo(); + + const realized = await realizeExecutionWorkspace({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + config: { + workspaceStrategy: { + type: "git_worktree", + branchTemplate: "{{issue.identifier}}-{{slug}}", + }, + }, + issue: { + id: "issue-unsafe", + identifier: "PAP-991", + title: "there should be a setting for the allowance of thumbs up / thumbs down data; `rm -rf`", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + }); + + expect(realized.branchName).toBe( + "PAP-991-there-should-be-a-setting-for-the-allowance-of-thumbs-up-thumbs-down-data-rm-rf", + ); + expect(realized.branchName?.includes("/")).toBe(false); + expect(path.basename(realized.cwd)).toBe(realized.branchName); + }); + it("runs a configured provision command inside the derived worktree", async () => { const repoRoot = await createTempRepo(); await fs.mkdir(path.join(repoRoot, "scripts"), { recursive: true }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index bc66f399..4ab87852 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -4,7 +4,7 @@ import { execFile as execFileCallback } from "node:child_process"; import { promisify } from "node:util"; import { and, asc, desc, eq, gt, inArray, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; -import type { BillingType, ExecutionWorkspaceConfig } from "@paperclipai/shared"; +import type { BillingType, ExecutionWorkspace, ExecutionWorkspaceConfig } from "@paperclipai/shared"; import { agents, agentRuntimeState, @@ -37,6 +37,8 @@ import { persistAdapterManagedRuntimeServices, realizeExecutionWorkspace, releaseRuntimeServicesForRun, + type ExecutionWorkspaceInput, + type RealizedExecutionWorkspace, sanitizeRuntimeServiceBaseEnv, } from "./workspace-runtime.js"; import { issueService } from "./issues.js"; @@ -109,6 +111,32 @@ export function stripWorkspaceRuntimeFromExecutionRunConfig(config: Record): Partial | null { const strategy = parseObject(config.workspaceStrategy); const snapshot: Partial = {}; @@ -2085,7 +2113,7 @@ export function heartbeatService(db: Db) { (explicitResumeSessionDisplayId ? { sessionId: explicitResumeSessionDisplayId } : null) ?? normalizeSessionParams(sessionCodec.deserialize(taskSessionForRun?.sessionParamsJson ?? null)); const config = parseObject(agent.adapterConfig); - const executionWorkspaceMode = resolveExecutionWorkspaceMode({ + const requestedExecutionWorkspaceMode = resolveExecutionWorkspaceMode({ projectPolicy: projectExecutionWorkspacePolicy, issueSettings: issueExecutionWorkspaceSettings, legacyUseProjectWorkspace: issueAssigneeOverrides?.useProjectWorkspace ?? null, @@ -2094,15 +2122,8 @@ export function heartbeatService(db: Db) { agent, context, previousSessionParams, - { useProjectWorkspace: executionWorkspaceMode !== "agent_default" }, + { useProjectWorkspace: requestedExecutionWorkspaceMode !== "agent_default" }, ); - const workspaceManagedConfig = buildExecutionWorkspaceAdapterConfig({ - agentConfig: config, - projectPolicy: projectExecutionWorkspacePolicy, - issueSettings: issueExecutionWorkspaceSettings, - mode: executionWorkspaceMode, - legacyUseProjectWorkspace: issueAssigneeOverrides?.useProjectWorkspace ?? null, - }); const issueRef = issueContext ? { id: issueContext.id, @@ -2116,10 +2137,32 @@ export function heartbeatService(db: Db) { : null; const existingExecutionWorkspace = issueRef?.executionWorkspaceId ? await executionWorkspacesSvc.getById(issueRef.executionWorkspaceId) : null; + const shouldReuseExisting = + issueRef?.executionWorkspacePreference === "reuse_existing" && + existingExecutionWorkspace && + existingExecutionWorkspace.status !== "archived"; + const persistedExecutionWorkspaceMode = shouldReuseExisting && existingExecutionWorkspace + ? issueExecutionWorkspaceModeForPersistedWorkspace(existingExecutionWorkspace.mode) + : null; + const effectiveExecutionWorkspaceMode: ReturnType = + persistedExecutionWorkspaceMode === "isolated_workspace" || + persistedExecutionWorkspaceMode === "operator_branch" || + persistedExecutionWorkspaceMode === "agent_default" + ? persistedExecutionWorkspaceMode + : requestedExecutionWorkspaceMode; + const workspaceManagedConfig = shouldReuseExisting + ? { ...config } + : buildExecutionWorkspaceAdapterConfig({ + agentConfig: config, + projectPolicy: projectExecutionWorkspacePolicy, + issueSettings: issueExecutionWorkspaceSettings, + mode: requestedExecutionWorkspaceMode, + legacyUseProjectWorkspace: issueAssigneeOverrides?.useProjectWorkspace ?? null, + }); const persistedWorkspaceManagedConfig = applyPersistedExecutionWorkspaceConfig({ config: workspaceManagedConfig, workspaceConfig: existingExecutionWorkspace?.config ?? null, - mode: executionWorkspaceMode, + mode: effectiveExecutionWorkspaceMode, }); const mergedConfig = issueAssigneeOverrides?.adapterConfig ? { ...persistedWorkspaceManagedConfig, ...issueAssigneeOverrides.adapterConfig } @@ -2140,39 +2183,43 @@ export function heartbeatService(db: Db) { heartbeatRunId: run.id, executionWorkspaceId: existingExecutionWorkspace?.id ?? null, }); - const executionWorkspace = await realizeExecutionWorkspace({ - base: { - baseCwd: resolvedWorkspace.cwd, - source: resolvedWorkspace.source, - projectId: resolvedWorkspace.projectId, - workspaceId: resolvedWorkspace.workspaceId, - repoUrl: resolvedWorkspace.repoUrl, - repoRef: resolvedWorkspace.repoRef, - }, - config: runtimeConfig, - issue: issueRef, - agent: { - id: agent.id, - name: agent.name, - companyId: agent.companyId, - }, - recorder: workspaceOperationRecorder, - }); + const executionWorkspaceBase = { + baseCwd: resolvedWorkspace.cwd, + source: resolvedWorkspace.source, + projectId: resolvedWorkspace.projectId, + workspaceId: resolvedWorkspace.workspaceId, + repoUrl: resolvedWorkspace.repoUrl, + repoRef: resolvedWorkspace.repoRef, + } satisfies ExecutionWorkspaceInput; + const executionWorkspace = shouldReuseExisting && existingExecutionWorkspace + ? buildRealizedExecutionWorkspaceFromPersisted({ + base: executionWorkspaceBase, + workspace: existingExecutionWorkspace, + }) + : await realizeExecutionWorkspace({ + base: executionWorkspaceBase, + config: runtimeConfig, + issue: issueRef, + agent: { + id: agent.id, + name: agent.name, + companyId: agent.companyId, + }, + recorder: workspaceOperationRecorder, + }); const resolvedProjectId = executionWorkspace.projectId ?? issueRef?.projectId ?? executionProjectId ?? null; const resolvedProjectWorkspaceId = issueRef?.projectWorkspaceId ?? resolvedWorkspace.workspaceId ?? null; - const shouldReuseExisting = - issueRef?.executionWorkspacePreference === "reuse_existing" && - existingExecutionWorkspace && - existingExecutionWorkspace.status !== "archived"; let persistedExecutionWorkspace = null; const nextExecutionWorkspaceMetadataBase = { ...(existingExecutionWorkspace?.metadata ?? {}), source: executionWorkspace.source, createdByRuntime: executionWorkspace.created, } as Record; - const nextExecutionWorkspaceMetadata = configSnapshot - ? mergeExecutionWorkspaceConfig(nextExecutionWorkspaceMetadataBase, configSnapshot) - : nextExecutionWorkspaceMetadataBase; + const nextExecutionWorkspaceMetadata = shouldReuseExisting + ? nextExecutionWorkspaceMetadataBase + : configSnapshot + ? mergeExecutionWorkspaceConfig(nextExecutionWorkspaceMetadataBase, configSnapshot) + : nextExecutionWorkspaceMetadataBase; try { persistedExecutionWorkspace = shouldReuseExisting && existingExecutionWorkspace ? await executionWorkspacesSvc.update(existingExecutionWorkspace.id, { @@ -2193,11 +2240,11 @@ export function heartbeatService(db: Db) { projectWorkspaceId: resolvedProjectWorkspaceId, sourceIssueId: issueRef?.id ?? null, mode: - executionWorkspaceMode === "isolated_workspace" + requestedExecutionWorkspaceMode === "isolated_workspace" ? "isolated_workspace" - : executionWorkspaceMode === "operator_branch" + : requestedExecutionWorkspaceMode === "operator_branch" ? "operator_branch" - : executionWorkspaceMode === "agent_default" + : requestedExecutionWorkspaceMode === "agent_default" ? "adapter_managed" : "shared_workspace", strategyType: executionWorkspace.strategy === "git_worktree" ? "git_worktree" : "project_primary", @@ -2272,8 +2319,8 @@ export function heartbeatService(db: Db) { const nextIssueWorkspaceMode = issueExecutionWorkspaceModeForPersistedWorkspace(persistedExecutionWorkspace.mode); const shouldSwitchIssueToExistingWorkspace = issueRef?.executionWorkspacePreference === "reuse_existing" || - executionWorkspaceMode === "isolated_workspace" || - executionWorkspaceMode === "operator_branch"; + requestedExecutionWorkspaceMode === "isolated_workspace" || + requestedExecutionWorkspaceMode === "operator_branch"; const nextIssuePatch: Record = {}; if (issueRef?.executionWorkspaceId !== persistedExecutionWorkspace.id) { nextIssuePatch.executionWorkspaceId = persistedExecutionWorkspace.id; @@ -2326,7 +2373,7 @@ export function heartbeatService(db: Db) { context.paperclipWorkspace = { cwd: executionWorkspace.cwd, source: executionWorkspace.source, - mode: executionWorkspaceMode, + mode: effectiveExecutionWorkspaceMode, strategy: executionWorkspace.strategy, projectId: executionWorkspace.projectId, workspaceId: executionWorkspace.workspaceId, diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index dfece576..7f30d247 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -194,9 +194,9 @@ function toRuntimeServiceRef(record: RuntimeServiceRecord, overrides?: Partial 0 ? normalized : fallback; } @@ -231,9 +231,9 @@ function renderWorkspaceTemplate(template: string, input: { function sanitizeBranchName(value: string): string { return value .trim() - .replace(/[^A-Za-z0-9._/-]+/g, "-") + .replace(/[^A-Za-z0-9_-]+/g, "-") .replace(/-+/g, "-") - .replace(/^[-/.]+|[-/.]+$/g, "") + .replace(/^[-_]+|[-_]+$/g, "") .slice(0, 120) || "paperclip-work"; }