Address Greptile feedback on workspace reuse

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
dotta 2026-03-30 14:55:44 -05:00
parent 2b18fc4007
commit 477ef78fed
6 changed files with 109 additions and 24 deletions

View file

@ -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<string, unknown> {
return JSON.parse(readFileSync(filePath, "utf8")) as Record<string, unknown>;
}
function resolveWorkspacePackagePath(packageName: string): string | null {
if (packageName === "@paperclipai/adapter-utils") {
return path.join(repoRoot, "packages", "adapter-utils");
function discoverWorkspacePackagePaths(rootDir: string): Map<string, string> {
const packagePaths = new Map<string, string>();
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("/"));

View file

@ -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", () => {

View file

@ -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 });

View file

@ -38,6 +38,7 @@ export function buildInvocationEnvForLogs(
env: Record<string, string>,
options: BuildInvocationEnvForLogsOptions = {},
): Record<string, string> {
// TODO: Remove this fallback once @paperclipai/adapter-utils exports buildInvocationEnvForLogs everywhere we consume it.
const maybeBuildInvocationEnvForLogs = (
serverUtils as typeof serverUtils & {
buildInvocationEnvForLogs?: (

View file

@ -114,10 +114,10 @@ export function stripWorkspaceRuntimeFromExecutionRunConfig(config: Record<strin
export function buildRealizedExecutionWorkspaceFromPersisted(input: {
base: ExecutionWorkspaceInput;
workspace: ExecutionWorkspace;
}): RealizedExecutionWorkspace {
}): RealizedExecutionWorkspace | null {
const cwd = readNonEmptyString(input.workspace.cwd) ?? readNonEmptyString(input.workspace.providerRef);
if (!cwd) {
throw new Error(`Execution workspace ${input.workspace.id} has no local path to reuse.`);
return null;
}
const strategy = input.workspace.strategyType === "git_worktree" ? "git_worktree" : "project_primary";
@ -2191,12 +2191,13 @@ export function heartbeatService(db: Db) {
repoUrl: resolvedWorkspace.repoUrl,
repoRef: resolvedWorkspace.repoRef,
} satisfies ExecutionWorkspaceInput;
const executionWorkspace = shouldReuseExisting && existingExecutionWorkspace
const reusedExecutionWorkspace = shouldReuseExisting && existingExecutionWorkspace
? buildRealizedExecutionWorkspaceFromPersisted({
base: executionWorkspaceBase,
workspace: existingExecutionWorkspace,
})
: await realizeExecutionWorkspace({
: null;
const executionWorkspace = reusedExecutionWorkspace ?? await realizeExecutionWorkspace({
base: executionWorkspaceBase,
config: runtimeConfig,
issue: issueRef,

View file

@ -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";
}