Guard reused execution workspaces against stale bases

This commit is contained in:
Dotta committed 2026-05-26 13:04:29 +00:00
1 parent cdb8b90c8b
commit 773ced1bdb
5 files changed
+292 -5

No files matched your search

@@ -170,6 +170,8 @@ describe("mergeExecutionWorkspaceMetadataForPersistence", () => {
provisionCommand: "bash ./scripts/provision.sh",
},
shouldReuseExisting: false,
baseRef: null,
baseRefSha: null,
})).toEqual({
source: "task_session",
createdByRuntime: true,
@@ -200,6 +202,8 @@ describe("mergeExecutionWorkspaceMetadataForPersistence", () => {
provisionCommand: "bash ./scripts/new-provision.sh",
},
shouldReuseExisting: true,
baseRef: null,
baseRefSha: null,
})).toEqual({
config: {
environmentId: "env-old",
@@ -209,6 +213,25 @@ describe("mergeExecutionWorkspaceMetadataForPersistence", () => {
createdByRuntime: false,
});
});
it("records the resolved base ref SHA for newly realized workspaces", () => {
expect(mergeExecutionWorkspaceMetadataForPersistence({
existingMetadata: null,
source: "task_session",
createdByRuntime: true,
configSnapshot: null,
shouldReuseExisting: false,
baseRef: "origin/main",
baseRefSha: "abc1234567890",
})).toEqual({
source: "task_session",
createdByRuntime: true,
baseRefSnapshot: {
baseRef: "origin/main",
resolvedSha: "abc1234567890",
},
});
});
});
describe("buildRealizedExecutionWorkspaceFromPersisted", () => {
@@ -372,6 +372,75 @@ describe("realizeExecutionWorkspace", () => {
expect(second.branchName).toBe(first.branchName);
});
it("warns when reusing a git worktree whose base ref has advanced", async () => {
const repoRoot = await createTempRepo();
const initial = await realizeExecutionWorkspace({
base: {
baseCwd: repoRoot,
source: "project_primary",
projectId: "project-1",
workspaceId: "workspace-1",
repoUrl: null,
repoRef: "main",
},
config: {
workspaceStrategy: {
type: "git_worktree",
branchTemplate: "{{issue.identifier}}-{{slug}}",
},
},
issue: {
id: "issue-1",
identifier: "PAP-447",
title: "Add Worktree Support",
},
agent: {
id: "agent-1",
name: "Codex Coder",
companyId: "company-1",
},
});
expect(initial.baseRefSha).toMatch(/^[0-9a-f]{40}$/);
await fs.writeFile(path.join(repoRoot, "server-auth-fix.txt"), "cookie fix\n", "utf8");
await runGit(repoRoot, ["add", "server-auth-fix.txt"]);
await runGit(repoRoot, ["commit", "-m", "Add auth runtime fix"]);
const reused = await realizeExecutionWorkspace({
base: {
baseCwd: repoRoot,
source: "project_primary",
projectId: "project-1",
workspaceId: "workspace-1",
repoUrl: null,
repoRef: "main",
},
config: {
workspaceStrategy: {
type: "git_worktree",
branchTemplate: "{{issue.identifier}}-{{slug}}",
},
},
issue: {
id: "issue-1",
identifier: "PAP-447",
title: "Add Worktree Support",
},
agent: {
id: "agent-1",
name: "Codex Coder",
companyId: "company-1",
},
});
expect(reused.created).toBe(false);
expect(reused.cwd).toBe(initial.cwd);
expect(reused.warnings).toEqual([
expect.stringContaining("is behind main by 1 commit"),
]);
});
it("rejects reusing an empty directory that only looks like a worktree because it sits inside the repo", async () => {
const repoRoot = await createTempRepo();
const branchName = "PAP-447-add-worktree-support";
@@ -250,6 +250,7 @@ export function executionWorkspaceRoutes(db: Db) {
repoUrl: existing.repoUrl,
baseRef: existing.baseRef,
branchName: existing.branchName,
metadata: existing.metadata as Record<string, unknown> | null,
config: {
...existing.config,
provisionCommand:
+47 -1
View File
@@ -87,6 +87,7 @@ import { logActivity, publishPluginDomainEvent, type LogActivityInput } from "./
import {
buildWorkspaceReadyComment,
cleanupExecutionWorkspaceArtifacts,
ensurePersistedExecutionWorkspaceAvailable,
ensureRuntimeServicesForRun,
persistAdapterManagedRuntimeServices,
realizeExecutionWorkspace,
@@ -594,6 +595,8 @@ export function mergeExecutionWorkspaceMetadataForPersistence(input: {
createdByRuntime: boolean;
configSnapshot: Record<string, unknown> | null;
shouldReuseExisting: boolean;
baseRef: string | null | undefined;
baseRefSha: string | null | undefined;
}) {
const base = {
...(input.existingMetadata ?? {}),
@@ -601,6 +604,17 @@ export function mergeExecutionWorkspaceMetadataForPersistence(input: {
createdByRuntime: input.createdByRuntime,
} as Record<string, unknown>;
const existingSnapshot = parseObject(base.baseRefSnapshot);
if (
typeof existingSnapshot.resolvedSha !== "string"
&& input.baseRefSha
) {
base.baseRefSnapshot = {
baseRef: input.baseRef ?? null,
resolvedSha: input.baseRefSha,
};
}
if (input.shouldReuseExisting || !input.configSnapshot) {
return base;
}
@@ -624,6 +638,8 @@ export function buildRealizedExecutionWorkspaceFromPersisted(input: {
}
const strategy = input.workspace.strategyType === "git_worktree" ? "git_worktree" : "project_primary";
const baseRefSnapshot = parseObject(input.workspace.metadata?.baseRefSnapshot);
const baseRefSha = typeof baseRefSnapshot.resolvedSha === "string" ? baseRefSnapshot.resolvedSha : null;
return {
baseCwd: input.base.baseCwd,
source: input.workspace.mode === "shared_workspace" ? "project_primary" : "task_session",
@@ -637,6 +653,7 @@ export function buildRealizedExecutionWorkspaceFromPersisted(input: {
worktreePath: strategy === "git_worktree" ? (readNonEmptyString(input.workspace.providerRef) ?? cwd) : null,
warnings: [],
created: false,
baseRefSha,
};
}
@@ -7229,7 +7246,34 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
repoRef: resolvedWorkspace.repoRef,
} satisfies ExecutionWorkspaceInput;
const reusedExecutionWorkspace = shouldReuseExisting && existingExecutionWorkspace
? buildRealizedExecutionWorkspaceFromPersisted({
? await ensurePersistedExecutionWorkspaceAvailable({
base: executionWorkspaceBase,
workspace: {
mode: existingExecutionWorkspace.mode,
strategyType: existingExecutionWorkspace.strategyType,
cwd: existingExecutionWorkspace.cwd,
providerRef: existingExecutionWorkspace.providerRef,
projectId: existingExecutionWorkspace.projectId,
projectWorkspaceId: existingExecutionWorkspace.projectWorkspaceId,
repoUrl: existingExecutionWorkspace.repoUrl,
baseRef: existingExecutionWorkspace.baseRef,
branchName: existingExecutionWorkspace.branchName,
metadata: existingExecutionWorkspace.metadata as Record<string, unknown> | null,
config: {
provisionCommand:
existingExecutionWorkspace.config?.provisionCommand
?? projectExecutionWorkspacePolicy?.workspaceStrategy?.provisionCommand
?? null,
},
},
issue: issueRef,
agent: {
id: agent.id,
name: agent.name,
companyId: agent.companyId,
},
recorder: workspaceOperationRecorder,
}) ?? buildRealizedExecutionWorkspaceFromPersisted({
base: executionWorkspaceBase,
workspace: existingExecutionWorkspace,
})
@@ -7254,6 +7298,8 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
createdByRuntime: executionWorkspace.created,
configSnapshot,
shouldReuseExisting,
baseRef: executionWorkspace.repoRef,
baseRefSha: executionWorkspace.baseRefSha ?? null,
});
try {
persistedExecutionWorkspace = shouldReuseExisting && existingExecutionWorkspace
+152 -4
View File
@@ -67,6 +67,7 @@ export interface RealizedExecutionWorkspace extends ExecutionWorkspaceInput {
worktreePath: string | null;
warnings: string[];
created: boolean;
baseRefSha?: string | null;
}
export interface RuntimeServiceRef {
@@ -524,11 +525,110 @@ async function runGit(args: string[], cwd: string): Promise<string> {
return proc.stdout.trim();
}
function formatShortSha(value: string | null | undefined) {
return value ? value.slice(0, 12) : "unknown";
}
function gitErrorIncludes(error: unknown, needle: string) {
const message = error instanceof Error ? error.message : String(error);
return message.toLowerCase().includes(needle.toLowerCase());
}
function parseRemoteTrackingRef(ref: string): { remote: string; branch: string } | null {
const trimmed = ref.trim();
const refsRemotesPrefix = "refs/remotes/";
const normalized = trimmed.startsWith(refsRemotesPrefix)
? trimmed.slice(refsRemotesPrefix.length)
: trimmed;
const slashIndex = normalized.indexOf("/");
if (slashIndex <= 0 || slashIndex === normalized.length - 1) return null;
const remote = normalized.slice(0, slashIndex);
const branch = normalized.slice(slashIndex + 1);
if (!/^[A-Za-z0-9._-]+$/.test(remote)) return null;
return { remote, branch };
}
async function refreshRemoteTrackingBaseRef(repoRoot: string, baseRef: string): Promise<string[]> {
const remoteTracking = parseRemoteTrackingRef(baseRef);
if (!remoteTracking) return [];
const remoteExists = await runGit(["remote", "get-url", remoteTracking.remote], repoRoot)
.then(() => true)
.catch(() => false);
if (!remoteExists) return [];
try {
await runGit([
"fetch",
"--prune",
remoteTracking.remote,
`+refs/heads/${remoteTracking.branch}:refs/remotes/${remoteTracking.remote}/${remoteTracking.branch}`,
], repoRoot);
return [];
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
return [`Could not refresh base ref ${baseRef} before preparing the execution workspace: ${message}`];
}
}
async function resolveBaseRefSha(repoRoot: string, baseRef: string): Promise<string | null> {
return await runGit(["rev-parse", "--verify", `${baseRef}^{commit}`], repoRoot).catch(() => null);
}
function readRecordedBaseRefSha(metadata: Record<string, unknown> | null | undefined): string | null {
const snapshot = parseObject(metadata?.baseRefSnapshot);
const resolvedSha = snapshot.resolvedSha;
return typeof resolvedSha === "string" && resolvedSha.trim().length > 0 ? resolvedSha.trim() : null;
}
export async function inspectExecutionWorkspaceBaseDrift(input: {
repoRoot: string;
worktreePath: string;
branchName: string | null;
baseRef: string | null;
recordedBaseRefSha?: string | null;
skipRefresh?: boolean;
}): Promise<{
warnings: string[];
currentBaseRefSha: string | null;
branchBaseRefSha: string | null;
}> {
const baseRef = input.baseRef?.trim();
if (!baseRef) {
return { warnings: [], currentBaseRefSha: null, branchBaseRefSha: null };
}
const warnings = input.skipRefresh ? [] : await refreshRemoteTrackingBaseRef(input.repoRoot, baseRef);
const currentBaseRefSha = await resolveBaseRefSha(input.repoRoot, baseRef);
if (!currentBaseRefSha) {
warnings.push(`Could not resolve base ref ${baseRef} while checking execution workspace freshness.`);
return { warnings, currentBaseRefSha: null, branchBaseRefSha: null };
}
const branchBaseRefSha = await runGit(["merge-base", "HEAD", baseRef], input.worktreePath).catch(() => null);
if (!branchBaseRefSha) {
warnings.push(`Could not compare execution workspace ${input.branchName ?? "branch"} against base ref ${baseRef}.`);
return { warnings, currentBaseRefSha, branchBaseRefSha: null };
}
if (branchBaseRefSha !== currentBaseRefSha) {
const behindCountRaw = await runGit(["rev-list", "--count", `HEAD..${baseRef}`], input.worktreePath).catch(() => "");
const behindCount = Number.parseInt(behindCountRaw, 10);
const behindText = Number.isFinite(behindCount) && behindCount > 0
? `${behindCount} commit${behindCount === 1 ? "" : "s"}`
: "newer commits";
const recordedText = input.recordedBaseRefSha
? `recorded base ${formatShortSha(input.recordedBaseRefSha)}`
: `merge-base ${formatShortSha(branchBaseRefSha)}`;
warnings.push(
`Execution workspace branch ${input.branchName ? `"${input.branchName}"` : "HEAD"} is behind ${baseRef} by ${behindText}: ${recordedText}, current base ${formatShortSha(currentBaseRefSha)}. Refresh or rebase the workspace before relying on recent base-branch fixes.`,
);
}
return { warnings, currentBaseRefSha, branchBaseRefSha };
}
type GitWorktreeListEntry = {
worktree: string;
branch: string | null;
@@ -1003,6 +1103,7 @@ export async function realizeExecutionWorkspace(input: {
worktreePath: null,
warnings: [],
created: false,
baseRefSha: null,
};
}
@@ -1026,10 +1127,20 @@ export async function realizeExecutionWorkspace(input: {
const baseRef = configuredBaseRef
?? await detectDefaultBranch(repoRoot)
?? "HEAD";
const baseRefreshWarnings = await refreshRemoteTrackingBaseRef(repoRoot, baseRef);
const currentBaseRefSha = await resolveBaseRefSha(repoRoot, baseRef);
await fs.mkdir(worktreeParentDir, { recursive: true });
async function reuseExistingWorktree(reusablePath: string) {
const baseDrift = await inspectExecutionWorkspaceBaseDrift({
repoRoot,
worktreePath: reusablePath,
branchName,
baseRef,
recordedBaseRefSha: null,
skipRefresh: true,
});
if (input.recorder) {
await input.recorder.recordOperation({
phase: "worktree_prepare",
@@ -1039,6 +1150,8 @@ export async function realizeExecutionWorkspace(input: {
worktreePath: reusablePath,
branchName,
baseRef,
currentBaseRefSha: baseDrift.currentBaseRefSha,
branchBaseRefSha: baseDrift.branchBaseRefSha,
created: false,
reused: true,
},
@@ -1066,8 +1179,9 @@ export async function realizeExecutionWorkspace(input: {
cwd: reusablePath,
branchName,
worktreePath: reusablePath,
warnings: [],
warnings: [...baseRefreshWarnings, ...baseDrift.warnings],
created: false,
baseRefSha: baseDrift.branchBaseRefSha ?? baseDrift.currentBaseRefSha,
};
}
@@ -1109,6 +1223,7 @@ export async function realizeExecutionWorkspace(input: {
worktreePath,
branchName,
baseRef,
baseRefSha: currentBaseRefSha,
created: true,
},
successMessage: `Created git worktree at ${worktreePath}\n`,
@@ -1128,6 +1243,7 @@ export async function realizeExecutionWorkspace(input: {
worktreePath,
branchName,
baseRef,
baseRefSha: currentBaseRefSha,
created: false,
reusedExistingBranch: true,
},
@@ -1163,8 +1279,9 @@ export async function realizeExecutionWorkspace(input: {
cwd: worktreePath,
branchName,
worktreePath,
warnings: [],
warnings: baseRefreshWarnings,
created: true,
baseRefSha: currentBaseRefSha,
};
}
@@ -1180,6 +1297,7 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
repoUrl: string | null | undefined;
baseRef: string | null | undefined;
branchName: string | null | undefined;
metadata?: Record<string, unknown> | null;
config?: {
provisionCommand?: string | null;
} | null;
@@ -1205,15 +1323,26 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
worktreePath: strategy === "git_worktree" ? (input.workspace.providerRef ?? cwd) : null,
warnings: [],
created: false,
baseRefSha: readRecordedBaseRefSha(input.workspace.metadata),
};
const provisionCommand = asString(input.workspace.config?.provisionCommand, "").trim();
if (strategy !== "git_worktree") {
return realized;
}
const repoRoot = await runGit(["rev-parse", "--show-toplevel"], input.base.baseCwd);
const recordedBaseRefSha = readRecordedBaseRefSha(input.workspace.metadata);
if (await directoryExists(cwd)) {
const baseDrift = await inspectExecutionWorkspaceBaseDrift({
repoRoot,
worktreePath: realized.worktreePath ?? cwd,
branchName: realized.branchName,
baseRef: input.workspace.baseRef ?? input.base.repoRef ?? null,
recordedBaseRefSha,
});
realized.warnings = baseDrift.warnings;
realized.baseRefSha = recordedBaseRefSha ?? baseDrift.branchBaseRefSha ?? baseDrift.currentBaseRefSha;
if (provisionCommand) {
const repoRoot = await runGit(["rev-parse", "--show-toplevel"], input.base.baseCwd);
await provisionExecutionWorktree({
strategy: {
type: "git_worktree",
@@ -1232,7 +1361,6 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
return realized;
}
const repoRoot = await runGit(["rev-parse", "--show-toplevel"], input.base.baseCwd);
const worktreePath = realized.worktreePath ?? cwd;
const branchName = asString(input.workspace.branchName, "").trim();
if (!branchName) {
@@ -1241,6 +1369,9 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
await fs.mkdir(path.dirname(worktreePath), { recursive: true });
await runGit(["worktree", "prune"], repoRoot).catch(() => {});
const restoreBaseRef = input.workspace.baseRef ?? input.base.repoRef ?? null;
const restoreRefreshWarnings = restoreBaseRef ? await refreshRemoteTrackingBaseRef(repoRoot, restoreBaseRef) : [];
const restoreCurrentBaseRefSha = restoreBaseRef ? await resolveBaseRefSha(repoRoot, restoreBaseRef) : null;
let created = false;
try {
@@ -1253,6 +1384,7 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
worktreePath,
branchName,
baseRef: input.workspace.baseRef ?? input.base.repoRef ?? null,
currentBaseRefSha: restoreCurrentBaseRefSha,
created: false,
restored: true,
},
@@ -1268,6 +1400,7 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
throw error;
}
const baseRef = input.workspace.baseRef ?? await detectDefaultBranch(repoRoot) ?? "HEAD";
const recreatedBaseRefSha = await resolveBaseRefSha(repoRoot, baseRef);
await recordGitOperation(input.recorder, {
phase: "worktree_prepare",
args: ["worktree", "add", "-b", branchName, worktreePath, baseRef],
@@ -1277,6 +1410,7 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
worktreePath,
branchName,
baseRef,
baseRefSha: recreatedBaseRefSha,
created: true,
restored: true,
},
@@ -1286,6 +1420,15 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
created = true;
}
const baseDrift = await inspectExecutionWorkspaceBaseDrift({
repoRoot,
worktreePath,
branchName,
baseRef: input.workspace.baseRef ?? input.base.repoRef ?? null,
recordedBaseRefSha,
skipRefresh: true,
});
await provisionExecutionWorktree({
strategy: {
type: "git_worktree",
@@ -1305,7 +1448,12 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: {
...realized,
cwd: worktreePath,
worktreePath,
warnings: [...restoreRefreshWarnings, ...baseDrift.warnings],
created,
baseRefSha:
recordedBaseRefSha
?? (created ? restoreCurrentBaseRefSha : baseDrift.branchBaseRefSha)
?? baseDrift.currentBaseRefSha,
};
}