fix(workspaces): reopen archived git worktree for managed_checkout projects (#11395)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Execution workspaces give agent tasks isolated Git worktrees > - Archived isolated workspaces must reopen against a live project checkout > - A managed_checkout project has no project workspace directory in its row > - The reopen path used the removed archived worktree as the Git working directory > - This pull request resolves the live managed checkout and reports a clear error when it is unavailable > - The benefit is reliable workspace reopen behavior after archive cleanup ## Linked Issues or Issue Description Related public pull request: [#6164](https://github.com/paperclipai/paperclip/pull/6164) clears archive state during un-archive. This pull request fixes the separate reopen failure that occurs after archive cleanup. **What happened?** An archived isolated `git_worktree` workspace under a `managed_checkout` project failed to reopen after cleanup. The route attempted to run Git in the removed archived worktree and returned a generic service error. **Expected behavior** The reopen path should use the live managed checkout as the Git base directory and should return a clear error when that directory is unavailable. **Steps to reproduce** 1. Create a project with `managed_checkout` source control. 2. Create and archive an isolated `git_worktree` execution workspace. 3. Let archive cleanup remove the worktree. 4. Reopen the workspace for an issue. **Paperclip version or commit** `cab0c31dc61310106caef42ca244e9f7b0f19460` **Deployment mode** Local dev with the default embedded database. **Agent adapter(s) involved** Not adapter-specific. This issue affects core workspace handling. ## What Changed - Resolve the live managed checkout when a managed project reopens an archived Git worktree. - Keep local-folder projects on their project workspace directory. - Validate the Git base directory before `git rev-parse` and return a scrubbed error. - Add nine regression tests for workspace reopen behavior. ## Verification - `server` TypeScript check passes with `tsc --noEmit`. - `server/src/__tests__/execution-workspace-reopen.test.ts` passes with 9 tests. - GitHub Actions must pass all required PR checks. ## Risks Low risk. The change affects only archived isolated workspace reopen behavior. It reuses the existing managed checkout and Git authentication helpers. It adds no new credential path, endpoint, or telemetry. ## Model Used OpenAI GPT-5 assisted with review and GitHub operations. The implementation author supplied the code and test results. ## 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:
Nicky Leach
GitHub
parent
35a9b98733
commit
69027cbaae
|
|
@ -1,4 +1,6 @@
|
|||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { mkdirSync, writeFileSync } from "node:fs";
|
||||
import { mkdtemp, rm, stat } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { randomUUID } from "node:crypto";
|
||||
|
|
@ -27,6 +29,7 @@ import {
|
|||
readExecutionWorkspaceLifecycleGeneration,
|
||||
readMetadataReopenPendingConsumptionSince,
|
||||
} from "../services/execution-workspaces.js";
|
||||
import { resolveManagedProjectWorkspaceDir } from "../home-paths.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
|
@ -128,6 +131,96 @@ describeEmbeddedPostgres("reopen archived isolated execution workspace", () => {
|
|||
return workspaceId;
|
||||
}
|
||||
|
||||
// Seed a company and project whose primary project workspace has a null cwd.
|
||||
// This models a managed_checkout project: the base is not a local folder, so
|
||||
// the live managed checkout supplies the base path at rebuild time.
|
||||
async function seedManagedCheckoutProject() {
|
||||
const companyId = randomUUID();
|
||||
const projectId = randomUUID();
|
||||
const projectWorkspaceId = randomUUID();
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `PAP-${companyId.slice(0, 8)}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await db.insert(projects).values({
|
||||
id: projectId,
|
||||
companyId,
|
||||
name: "Managed checkout project",
|
||||
status: "in_progress",
|
||||
});
|
||||
await db.insert(projectWorkspaces).values({
|
||||
id: projectWorkspaceId,
|
||||
companyId,
|
||||
projectId,
|
||||
name: "Primary",
|
||||
sourceType: "managed_checkout",
|
||||
cwd: null,
|
||||
isPrimary: true,
|
||||
});
|
||||
return { companyId, projectId, projectWorkspaceId };
|
||||
}
|
||||
|
||||
// Seed one closed isolated git_worktree row. The reaper already removed the
|
||||
// worktree directory, so cwd points at a path that is not on disk.
|
||||
async function seedClosedGitWorktreeWorkspace(input: {
|
||||
companyId: string;
|
||||
projectId: string;
|
||||
projectWorkspaceId: string;
|
||||
cwd: string;
|
||||
repoUrl: string;
|
||||
branchName: string;
|
||||
}) {
|
||||
const workspaceId = randomUUID();
|
||||
await db.insert(executionWorkspaces).values({
|
||||
id: workspaceId,
|
||||
companyId: input.companyId,
|
||||
projectId: input.projectId,
|
||||
projectWorkspaceId: input.projectWorkspaceId,
|
||||
mode: "isolated_workspace",
|
||||
strategyType: "git_worktree",
|
||||
name: "reopen-git-worktree",
|
||||
status: "archived",
|
||||
providerType: "local_fs",
|
||||
cwd: input.cwd,
|
||||
repoUrl: input.repoUrl,
|
||||
baseRef: null,
|
||||
branchName: input.branchName,
|
||||
closedAt: new Date(),
|
||||
cleanupReason: "issue_terminal",
|
||||
cleanupEligibleAt: new Date(),
|
||||
metadata: {
|
||||
[EXECUTION_WORKSPACE_LIFECYCLE_GENERATION_METADATA_KEY]: 1,
|
||||
},
|
||||
});
|
||||
return workspaceId;
|
||||
}
|
||||
|
||||
// Build a real git repository at the managed checkout path so
|
||||
// ensureManagedProjectWorkspace adopts it without a network clone. The repo
|
||||
// holds the branch that the archived worktree row references.
|
||||
function initManagedGitRepo(dir: string, worktreeBranch: string) {
|
||||
mkdirSync(dir, { recursive: true });
|
||||
const git = (...args: string[]) =>
|
||||
execFileSync("git", args, {
|
||||
cwd: dir,
|
||||
env: {
|
||||
...process.env,
|
||||
GIT_AUTHOR_NAME: "Test",
|
||||
GIT_AUTHOR_EMAIL: "test@example.com",
|
||||
GIT_COMMITTER_NAME: "Test",
|
||||
GIT_COMMITTER_EMAIL: "test@example.com",
|
||||
},
|
||||
stdio: "ignore",
|
||||
});
|
||||
git("init");
|
||||
writeFileSync(join(dir, "README.md"), "seed\n");
|
||||
git("add", "README.md");
|
||||
git("commit", "-m", "seed");
|
||||
git("branch", worktreeBranch);
|
||||
}
|
||||
|
||||
async function seedIssue(input: {
|
||||
companyId: string;
|
||||
projectId: string;
|
||||
|
|
@ -246,6 +339,56 @@ describeEmbeddedPostgres("reopen archived isolated execution workspace", () => {
|
|||
expect(readExecutionWorkspaceLifecycleGeneration(row?.metadata as Record<string, unknown> | null)).toBe(3);
|
||||
});
|
||||
|
||||
it("resolves the managed base checkout for a git_worktree row when the project workspace cwd is null", async () => {
|
||||
const previousHome = process.env.PAPERCLIP_HOME;
|
||||
const tempHome = await mkdtemp(join(tmpdir(), "paperclip-reopen-home-"));
|
||||
tempDirs.push(tempHome);
|
||||
process.env.PAPERCLIP_HOME = tempHome;
|
||||
try {
|
||||
const { companyId, projectId, projectWorkspaceId } = await seedManagedCheckoutProject();
|
||||
const repoUrl = "https://example.test/acme/widget.git";
|
||||
const branchName = "reopen-feature";
|
||||
// Build the live managed checkout that the rebuild must spawn git in.
|
||||
const managedDir = resolveManagedProjectWorkspaceDir({ companyId, projectId, repoName: "widget" });
|
||||
initManagedGitRepo(managedDir, branchName);
|
||||
|
||||
// The archived worktree path. The reaper already removed it from disk.
|
||||
const deletedWorktree = join(tempHome, "worktrees", "reopen-worktree");
|
||||
const workspaceId = await seedClosedGitWorktreeWorkspace({
|
||||
companyId,
|
||||
projectId,
|
||||
projectWorkspaceId,
|
||||
cwd: deletedWorktree,
|
||||
repoUrl,
|
||||
branchName,
|
||||
});
|
||||
const issueId = await seedIssue({ companyId, projectId, workspaceId, issueNumber: 4305 });
|
||||
|
||||
const svc = executionWorkspaceService(db);
|
||||
const result = await svc.reopenClosedIsolatedExecutionWorkspaceForIssue({
|
||||
workspaceId,
|
||||
issue: { id: issueId, companyId, projectId },
|
||||
actor: { agentId: null, actorType: "user" },
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
if (!result.ok) return;
|
||||
expect(result.reopened).toBe(true);
|
||||
|
||||
const row = await readWorkspace(workspaceId);
|
||||
expect(row?.status).toBe("active");
|
||||
expect(row?.closedAt).toBeNull();
|
||||
expect(row?.cleanupReason).toBeNull();
|
||||
// The rebuild recreated the worktree at the archived path from the managed
|
||||
// base checkout.
|
||||
const worktreeStat = await stat(deletedWorktree).catch(() => null);
|
||||
expect(worktreeStat?.isDirectory()).toBe(true);
|
||||
} finally {
|
||||
if (previousHome === undefined) delete process.env.PAPERCLIP_HOME;
|
||||
else process.env.PAPERCLIP_HOME = previousHome;
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses to reopen a workspace in another company", async () => {
|
||||
const first = await seedCompanyProject();
|
||||
const second = await seedCompanyProject();
|
||||
|
|
|
|||
|
|
@ -3127,6 +3127,100 @@ describe("realizeExecutionWorkspace", () => {
|
|||
await expect(fs.readFile(path.join(initial.cwd, ".paperclip-restored-state"), "utf8")).resolves.toBe("reprovisioned\n");
|
||||
}, 15_000);
|
||||
|
||||
it("rejects an empty base checkout path with a clear cause", async () => {
|
||||
// An empty base path makes the later "git" spawn fail with a raw ENOENT.
|
||||
// The reopen must throw a clear cause first that names the empty checkout.
|
||||
let error: unknown = null;
|
||||
try {
|
||||
await ensurePersistedExecutionWorkspaceAvailable({
|
||||
base: {
|
||||
baseCwd: "",
|
||||
source: "project_primary",
|
||||
projectId: "project-1",
|
||||
workspaceId: "workspace-1",
|
||||
repoUrl: null,
|
||||
repoRef: "HEAD",
|
||||
},
|
||||
workspace: {
|
||||
mode: "isolated_workspace",
|
||||
strategyType: "git_worktree",
|
||||
cwd: "/does-not-exist/worktree",
|
||||
providerRef: "/does-not-exist/worktree",
|
||||
projectId: "project-1",
|
||||
projectWorkspaceId: "workspace-1",
|
||||
repoUrl: null,
|
||||
baseRef: "HEAD",
|
||||
branchName: "feature-branch",
|
||||
},
|
||||
issue: {
|
||||
id: "issue-1",
|
||||
identifier: "PAP-461",
|
||||
title: "Empty base checkout path",
|
||||
},
|
||||
agent: {
|
||||
id: "agent-1",
|
||||
name: "Codex Coder",
|
||||
companyId: "company-1",
|
||||
},
|
||||
});
|
||||
} catch (err) {
|
||||
error = err;
|
||||
}
|
||||
|
||||
expect(error).toBeInstanceOf(Error);
|
||||
expect((error as Error).message).toBe(
|
||||
"Cannot rebuild the git worktree: the base project checkout path is empty.",
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves a whitespace-only base checkout path and reports it as missing", async () => {
|
||||
// The reopen must use the persisted base path exactly. A trim would change
|
||||
// a whitespace-only path into an empty path and hide the real cause.
|
||||
// A directory name can consist of spaces, so keep the path unchanged and
|
||||
// let the directory-exists check report the missing checkout.
|
||||
let error: unknown = null;
|
||||
try {
|
||||
await ensurePersistedExecutionWorkspaceAvailable({
|
||||
base: {
|
||||
baseCwd: " ",
|
||||
source: "project_primary",
|
||||
projectId: "project-1",
|
||||
workspaceId: "workspace-1",
|
||||
repoUrl: null,
|
||||
repoRef: "HEAD",
|
||||
},
|
||||
workspace: {
|
||||
mode: "isolated_workspace",
|
||||
strategyType: "git_worktree",
|
||||
cwd: "/does-not-exist/worktree",
|
||||
providerRef: "/does-not-exist/worktree",
|
||||
projectId: "project-1",
|
||||
projectWorkspaceId: "workspace-1",
|
||||
repoUrl: null,
|
||||
baseRef: "HEAD",
|
||||
branchName: "feature-branch",
|
||||
},
|
||||
issue: {
|
||||
id: "issue-1",
|
||||
identifier: "PAP-461",
|
||||
title: "Whitespace base checkout path",
|
||||
},
|
||||
agent: {
|
||||
id: "agent-1",
|
||||
name: "Codex Coder",
|
||||
companyId: "company-1",
|
||||
},
|
||||
});
|
||||
} catch (err) {
|
||||
error = err;
|
||||
}
|
||||
|
||||
expect(error).toBeInstanceOf(Error);
|
||||
expect((error as Error).message).toBe(
|
||||
"Cannot rebuild the git worktree: the base project checkout directory does not exist.",
|
||||
);
|
||||
});
|
||||
|
||||
it("auto-detects the default branch when baseRef is not configured", async () => {
|
||||
// Create a repo with "master" as default branch (not "main")
|
||||
const repoRoot = await createTempRepo("master");
|
||||
|
|
|
|||
|
|
@ -52,6 +52,7 @@ import {
|
|||
type PullRequestMergeDetailsResolver,
|
||||
} from "./github-pull-request-merge.js";
|
||||
import { visibleIssueCondition } from "./issue-visibility.js";
|
||||
import { createGitRemoteAuthProvider } from "./git-credentials.js";
|
||||
import { readProjectWorkspaceRuntimeConfig } from "./project-workspace-runtime-config.js";
|
||||
import {
|
||||
listCurrentRuntimeServicesForExecutionWorkspaces,
|
||||
|
|
@ -2764,11 +2765,17 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
|||
};
|
||||
}
|
||||
|
||||
const [{ ensurePersistedExecutionWorkspaceAvailable }, { workspaceOperationService }] =
|
||||
await Promise.all([
|
||||
import("./workspace-runtime.js"),
|
||||
import("./workspace-operations.js"),
|
||||
]);
|
||||
const [
|
||||
{ ensurePersistedExecutionWorkspaceAvailable },
|
||||
{ workspaceOperationService },
|
||||
{ ensureManagedProjectWorkspace },
|
||||
] = await Promise.all([
|
||||
import("./workspace-runtime.js"),
|
||||
import("./workspace-operations.js"),
|
||||
// heartbeat.js imports this module, so a static import creates a
|
||||
// cycle. Load ensureManagedProjectWorkspace dynamically instead.
|
||||
import("./heartbeat.js"),
|
||||
]);
|
||||
const [projectWorkspace, projectPolicy] = await Promise.all([
|
||||
row.projectWorkspaceId
|
||||
? db
|
||||
|
|
@ -2786,6 +2793,25 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
|||
.where(and(eq(projects.companyId, row.companyId), eq(projects.id, row.projectId)))
|
||||
.then((rows) => parseProjectExecutionWorkspacePolicy(rows[0]?.executionWorkspacePolicy)),
|
||||
]);
|
||||
// Resolve the base checkout that the rebuild spawns git in. A
|
||||
// local-folder project stores its base path in projectWorkspaces.cwd.
|
||||
// A managed_checkout project stores null there, so resolve its live
|
||||
// managed checkout instead. Never use row.cwd for a git_worktree
|
||||
// rebuild: row.cwd is the archived worktree path, which the reaper
|
||||
// already removed from disk. A spawn in that missing directory fails
|
||||
// with "spawn git ENOENT" and hides the real cause.
|
||||
let resolvedBaseCwd = projectWorkspace?.cwd ?? null;
|
||||
if (resolvedBaseCwd == null && row.strategyType === "git_worktree" && row.projectId) {
|
||||
const managedWorkspace = await ensureManagedProjectWorkspace({
|
||||
companyId: row.companyId,
|
||||
projectId: row.projectId,
|
||||
repoUrl: row.repoUrl,
|
||||
resolveGitAuth: createGitRemoteAuthProvider(db, row.companyId, {
|
||||
issueId: row.sourceIssueId ?? issue.id,
|
||||
}),
|
||||
});
|
||||
resolvedBaseCwd = managedWorkspace.cwd;
|
||||
}
|
||||
const config = readExecutionWorkspaceConfig(row.metadata as Record<string, unknown> | null);
|
||||
const nextGeneration = readExecutionWorkspaceLifecycleGeneration(
|
||||
row.metadata as Record<string, unknown> | null,
|
||||
|
|
@ -2803,7 +2829,7 @@ export function executionWorkspaceService(db: Db, opts: ExecutionWorkspaceServic
|
|||
const realized = await ensurePersistedExecutionWorkspaceAvailable({
|
||||
db: tx as unknown as Db,
|
||||
base: {
|
||||
baseCwd: projectWorkspace?.cwd ?? row.cwd ?? "",
|
||||
baseCwd: resolvedBaseCwd ?? row.cwd ?? "",
|
||||
source: "task_session",
|
||||
projectId: row.projectId,
|
||||
workspaceId: row.projectWorkspaceId,
|
||||
|
|
|
|||
|
|
@ -3031,7 +3031,24 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
|
|||
}
|
||||
return realized;
|
||||
}
|
||||
const repoRoot = await runGit(["rev-parse", "--show-toplevel"], input.base.baseCwd);
|
||||
// Validate the base checkout before the git spawn. A missing or empty base
|
||||
// path makes the "git" spawn fail with a raw "spawn git ENOENT" error. That
|
||||
// error hides the real cause: the base project checkout is not on disk.
|
||||
// Throw a clear cause first so a future failure names the missing checkout.
|
||||
// Keep the persisted path exact. A directory name can start or end with a
|
||||
// space, so a trim would change a valid checkout path.
|
||||
const baseCwd = asString(input.base.baseCwd, "");
|
||||
if (!baseCwd) {
|
||||
throw new Error(
|
||||
"Cannot rebuild the git worktree: the base project checkout path is empty.",
|
||||
);
|
||||
}
|
||||
if (!await directoryExists(baseCwd)) {
|
||||
throw new Error(
|
||||
"Cannot rebuild the git worktree: the base project checkout directory does not exist.",
|
||||
);
|
||||
}
|
||||
const repoRoot = await runGit(["rev-parse", "--show-toplevel"], baseCwd);
|
||||
const recordedBaseRefSha = readRecordedBaseRefSha(input.workspace.metadata);
|
||||
if (await directoryExists(cwd)) {
|
||||
const reuseBaseRef = input.workspace.baseRef ?? input.base.repoRef ?? null;
|
||||
|
|
|
|||
Loading…
Reference in New Issue