From 477ef78fedfd6a4f3f1d77085f67cdb3532185d3 Mon Sep 17 00:00:00 2001 From: dotta Date: Mon, 30 Mar 2026 14:55:44 -0500 Subject: [PATCH] Address Greptile feedback on workspace reuse Co-Authored-By: Paperclip --- scripts/ensure-workspace-package-links.ts | 47 ++++++++++++------- .../heartbeat-workspace-session.test.ts | 38 +++++++++++++++ .../src/__tests__/workspace-runtime.test.ts | 34 ++++++++++++++ server/src/adapters/utils.ts | 1 + server/src/services/heartbeat.ts | 9 ++-- server/src/services/workspace-runtime.ts | 4 +- 6 files changed, 109 insertions(+), 24 deletions(-) diff --git a/scripts/ensure-workspace-package-links.ts b/scripts/ensure-workspace-package-links.ts index d2494545..430ba589 100644 --- a/scripts/ensure-workspace-package-links.ts +++ b/scripts/ensure-workspace-package-links.ts @@ -1,6 +1,6 @@ #!/usr/bin/env -S node --import tsx import { spawn } from "node:child_process"; -import { existsSync, readFileSync, realpathSync } from "node:fs"; +import { existsSync, readdirSync, readFileSync, realpathSync } from "node:fs"; import path from "node:path"; import { repoRoot } from "./dev-service-profile.ts"; @@ -14,25 +14,36 @@ function readJsonFile(filePath: string): Record { return JSON.parse(readFileSync(filePath, "utf8")) as Record; } -function resolveWorkspacePackagePath(packageName: string): string | null { - if (packageName === "@paperclipai/adapter-utils") { - return path.join(repoRoot, "packages", "adapter-utils"); +function discoverWorkspacePackagePaths(rootDir: string): Map { + const packagePaths = new Map(); + const ignoredDirNames = new Set([".git", ".paperclip", "dist", "node_modules"]); + + function visit(dirPath: string) { + const packageJsonPath = path.join(dirPath, "package.json"); + if (existsSync(packageJsonPath)) { + const packageJson = readJsonFile(packageJsonPath); + if (typeof packageJson.name === "string" && packageJson.name.length > 0) { + packagePaths.set(packageJson.name, dirPath); + } + } + + for (const entry of readdirSync(dirPath, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + if (ignoredDirNames.has(entry.name)) continue; + visit(path.join(dirPath, entry.name)); + } } - if (packageName === "@paperclipai/db") { - return path.join(repoRoot, "packages", "db"); - } - if (packageName === "@paperclipai/shared") { - return path.join(repoRoot, "packages", "shared"); - } - if (packageName === "@paperclipai/plugin-sdk") { - return path.join(repoRoot, "packages", "plugins", "sdk"); - } - if (packageName.startsWith("@paperclipai/adapter-")) { - return path.join(repoRoot, "packages", "adapters", packageName.slice("@paperclipai/adapter-".length)); - } - return null; + + visit(path.join(rootDir, "packages")); + visit(path.join(rootDir, "server")); + visit(path.join(rootDir, "ui")); + visit(path.join(rootDir, "cli")); + + return packagePaths; } +const workspacePackagePaths = discoverWorkspacePackagePaths(repoRoot); + function findServerWorkspaceLinkMismatches(): WorkspaceLinkMismatch[] { const serverPackageJson = readJsonFile(path.join(repoRoot, "server", "package.json")); const dependencies = { @@ -44,7 +55,7 @@ function findServerWorkspaceLinkMismatches(): WorkspaceLinkMismatch[] { for (const [packageName, version] of Object.entries(dependencies)) { if (typeof version !== "string" || !version.startsWith("workspace:")) continue; - const expectedPath = resolveWorkspacePackagePath(packageName); + const expectedPath = workspacePackagePaths.get(packageName); if (!expectedPath) continue; const linkPath = path.join(repoRoot, "server", "node_modules", ...packageName.split("/")); diff --git a/server/src/__tests__/heartbeat-workspace-session.test.ts b/server/src/__tests__/heartbeat-workspace-session.test.ts index 47d37c53..5a1498f2 100644 --- a/server/src/__tests__/heartbeat-workspace-session.test.ts +++ b/server/src/__tests__/heartbeat-workspace-session.test.ts @@ -198,6 +198,44 @@ describe("buildRealizedExecutionWorkspaceFromPersisted", () => { expect(result.branchName).toBe("PAP-880-thumbs-capture-for-evals-feature"); expect(result.source).toBe("task_session"); }); + + it("falls back to realization when the persisted workspace has no local path yet", () => { + const result = buildRealizedExecutionWorkspaceFromPersisted({ + base: buildResolvedWorkspace({ + cwd: "/tmp/project-primary", + repoRef: "main", + }), + workspace: { + id: "execution-workspace-2", + companyId: "company-1", + projectId: "project-1", + projectWorkspaceId: "workspace-1", + sourceIssueId: "issue-2", + mode: "isolated_workspace", + strategyType: "git_worktree", + name: "PAP-999-missing-provider-ref", + status: "active", + cwd: null, + repoUrl: "https://example.com/paperclip.git", + baseRef: "main", + branchName: "feature/PAP-999-missing-provider-ref", + providerType: "git_worktree", + providerRef: null, + 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).toBeNull(); + }); }); describe("stripWorkspaceRuntimeFromExecutionRunConfig", () => { diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index 573013be..6c98b2cf 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -284,6 +284,40 @@ describe("realizeExecutionWorkspace", () => { expect(path.basename(realized.cwd)).toBe(realized.branchName); }); + it("preserves intentional slashes and dots from the branch template", 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: "release/{{issue.identifier}}.{{slug}}", + }, + }, + issue: { + id: "issue-template-safe", + identifier: "PAP-992", + title: "Hotfix / April.1", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + }); + + expect(realized.branchName).toBe("release/PAP-992.hotfix-april-1"); + expect(path.basename(realized.cwd)).toBe("PAP-992.hotfix-april-1"); + }); + 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/adapters/utils.ts b/server/src/adapters/utils.ts index d9201bb6..6dfccfcb 100644 --- a/server/src/adapters/utils.ts +++ b/server/src/adapters/utils.ts @@ -38,6 +38,7 @@ export function buildInvocationEnvForLogs( env: Record, options: BuildInvocationEnvForLogsOptions = {}, ): Record { + // TODO: Remove this fallback once @paperclipai/adapter-utils exports buildInvocationEnvForLogs everywhere we consume it. const maybeBuildInvocationEnvForLogs = ( serverUtils as typeof serverUtils & { buildInvocationEnvForLogs?: ( diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 4ab87852..168db86b 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -114,10 +114,10 @@ export function stripWorkspaceRuntimeFromExecutionRunConfig(config: Record