mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 13:26:04 -06:00
fix(worktrees): rebind live repository before removal (#126305)
Resolve and persist the exact same-origin repository that owns a live managed checkout before snapshot refs, branch deletion, or worktree removal. Fail closed on different-origin substitution. Closes #126304
This commit is contained in:
committed by
GitHub
parent
81e2992f4b
commit
0538eb969d
@@ -71,12 +71,18 @@ describe("managed worktree registry", () => {
|
||||
expect(getRegistryWorktreeProvisionedPaths(env, "second")).toBeUndefined();
|
||||
|
||||
updateRegistryWorktree(env, "first", {
|
||||
repositoryIdentity: {
|
||||
repoRoot: path.join(root, "rebound-repo"),
|
||||
repoFingerprint: "fedcba9876543210",
|
||||
},
|
||||
lastActiveAt: 30,
|
||||
removedAt: 40,
|
||||
snapshotRef: "refs/openclaw/snapshots/first",
|
||||
provisionedState: [{ path: ".env.local", mode: 0o600, chunks: 1 }],
|
||||
});
|
||||
expect(getRegistryWorktree(env, "first")).toMatchObject({
|
||||
repoRoot: path.join(root, "rebound-repo"),
|
||||
repoFingerprint: "fedcba9876543210",
|
||||
lastActiveAt: 30,
|
||||
removedAt: 40,
|
||||
snapshotRef: "refs/openclaw/snapshots/first",
|
||||
|
||||
@@ -398,6 +398,7 @@ export function updateRegistryWorktree(
|
||||
patch: Partial<
|
||||
Pick<ManagedWorktreeRecord, "lastActiveAt" | "removedAt" | "runEndCleanup" | "snapshotRef">
|
||||
> & {
|
||||
repositoryIdentity?: Pick<ManagedWorktreeRecord, "repoRoot" | "repoFingerprint">;
|
||||
provisionedPaths?: readonly string[];
|
||||
provisionedState?: readonly ProvisionedFileState[];
|
||||
},
|
||||
@@ -418,6 +419,10 @@ export function updateRegistryWorktree(
|
||||
values.run_end_cleanup_json =
|
||||
patch.runEndCleanup === undefined ? null : JSON.stringify(patch.runEndCleanup);
|
||||
}
|
||||
if (patch.repositoryIdentity) {
|
||||
values.repo_root = patch.repositoryIdentity.repoRoot;
|
||||
values.repo_fingerprint = patch.repositoryIdentity.repoFingerprint;
|
||||
}
|
||||
if (patch.provisionedState !== undefined) {
|
||||
values.provisioned_paths_json = JSON.stringify(patch.provisionedState);
|
||||
} else if (patch.provisionedPaths !== undefined) {
|
||||
|
||||
@@ -4,7 +4,11 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { closeOpenClawStateDatabaseForTest } from "../../state/openclaw-state-db.js";
|
||||
import {
|
||||
closeOpenClawStateDatabaseForTest,
|
||||
openOpenClawStateDatabase,
|
||||
} from "../../state/openclaw-state-db.js";
|
||||
import { getRegistryWorktree } from "./registry.js";
|
||||
import { ManagedWorktreeService } from "./service.js";
|
||||
import { initializeManagedWorktreeTestRepository } from "./service.test-support.js";
|
||||
|
||||
@@ -21,8 +25,22 @@ describe("ManagedWorktreeService canonical paths", () => {
|
||||
let root: string;
|
||||
let repo: string;
|
||||
let stateDir: string;
|
||||
let env: NodeJS.ProcessEnv;
|
||||
let service: ManagedWorktreeService;
|
||||
|
||||
async function cloneRepository(name: string, originUrl?: string): Promise<string> {
|
||||
const target = path.join(root, name);
|
||||
await execFileAsync("git", ["clone", "--no-hardlinks", repo, target]);
|
||||
await git(
|
||||
target,
|
||||
"remote",
|
||||
"set-url",
|
||||
"origin",
|
||||
originUrl ?? (await git(repo, "config", "--get", "remote.origin.url")),
|
||||
);
|
||||
return await fs.realpath(target);
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
root = await fs.mkdtemp(
|
||||
path.join(await fs.realpath(os.tmpdir()), "openclaw-worktree-canonical-paths-"),
|
||||
@@ -30,9 +48,8 @@ describe("ManagedWorktreeService canonical paths", () => {
|
||||
repo = await initializeManagedWorktreeTestRepository(root);
|
||||
stateDir = path.join(root, "state");
|
||||
await fs.mkdir(stateDir, { recursive: true });
|
||||
service = new ManagedWorktreeService({
|
||||
env: { ...process.env, OPENCLAW_STATE_DIR: stateDir },
|
||||
});
|
||||
env = { ...process.env, OPENCLAW_STATE_DIR: stateDir };
|
||||
service = new ManagedWorktreeService({ env });
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
@@ -60,6 +77,137 @@ describe("ManagedWorktreeService canonical paths", () => {
|
||||
expect(await fs.readFile(path.join(restored.path, "README.md"), "utf8")).toBe("base\n");
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ label: "normal", allowSnapshotLoss: false },
|
||||
{ label: "forced", allowSnapshotLoss: true },
|
||||
])(
|
||||
"repairs a $label removal to the live checkout repository before snapshotting",
|
||||
async ({ label, allowSnapshotLoss }) => {
|
||||
const canonicalLiveRepo = await cloneRepository(`live-${label}`);
|
||||
const liveIdentity = await service.resolveRepositoryIdentity(canonicalLiveRepo);
|
||||
const staleIdentity = await service.resolveRepositoryIdentity(repo);
|
||||
const created = await service.create({
|
||||
repoRoot: canonicalLiveRepo,
|
||||
name: `repository-rebind-${label}`,
|
||||
baseRef: "HEAD",
|
||||
ownerKind: "session",
|
||||
ownerId: `agent:main:${label}`,
|
||||
});
|
||||
await fs.writeFile(path.join(created.path, "README.md"), `${label} tracked change\n`);
|
||||
await fs.writeFile(path.join(created.path, "untracked.txt"), `${label} untracked change\n`);
|
||||
const staleHead = await git(repo, "rev-parse", "HEAD");
|
||||
const snapshotRef = `refs/openclaw/snapshots/${created.id}`;
|
||||
await git(repo, "branch", created.branch, staleHead);
|
||||
await git(repo, "update-ref", snapshotRef, staleHead);
|
||||
openOpenClawStateDatabase({ env })
|
||||
.db.prepare("UPDATE worktrees SET repo_root = ?, repo_fingerprint = ? WHERE id = ?")
|
||||
.run(staleIdentity.repoRoot, staleIdentity.fingerprint, created.id);
|
||||
|
||||
const removed = await service.remove({
|
||||
id: created.id,
|
||||
reason: "repository-rebind",
|
||||
allowSnapshotLoss,
|
||||
});
|
||||
|
||||
expect(removed).toEqual({ removed: true, snapshotRef });
|
||||
expect(getRegistryWorktree(env, created.id)).toMatchObject({
|
||||
repoRoot: liveIdentity.repoRoot,
|
||||
repoFingerprint: liveIdentity.fingerprint,
|
||||
path: created.path,
|
||||
branch: created.branch,
|
||||
baseRef: created.baseRef,
|
||||
ownerKind: "session",
|
||||
ownerId: `agent:main:${label}`,
|
||||
snapshotRef,
|
||||
});
|
||||
expect(await git(canonicalLiveRepo, "show-ref", "--verify", snapshotRef)).not.toBe("");
|
||||
expect(await git(canonicalLiveRepo, "branch", "--list", created.branch)).toBe("");
|
||||
expect(await git(repo, "rev-parse", snapshotRef)).toBe(staleHead);
|
||||
expect(await git(repo, "rev-parse", created.branch)).toBe(staleHead);
|
||||
|
||||
const restored = await service.restore({ id: created.id });
|
||||
expect(restored.repoRoot).toBe(liveIdentity.repoRoot);
|
||||
expect(restored.path).toBe(created.path);
|
||||
expect(await git(restored.path, "branch", "--show-current")).toBe(created.branch);
|
||||
expect(await fs.readFile(path.join(restored.path, "README.md"), "utf8")).toBe(
|
||||
`${label} tracked change\n`,
|
||||
);
|
||||
expect(await fs.readFile(path.join(restored.path, "untracked.txt"), "utf8")).toBe(
|
||||
`${label} untracked change\n`,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("rejects a live checkout from a different-origin repository before mutation", async () => {
|
||||
const differentOrigin = path.join(root, "different-origin.git");
|
||||
await execFileAsync("git", ["clone", "--bare", repo, differentOrigin]);
|
||||
const liveRepo = await cloneRepository("live-different-origin", differentOrigin);
|
||||
const staleIdentity = await service.resolveRepositoryIdentity(repo);
|
||||
const created = await service.create({
|
||||
repoRoot: liveRepo,
|
||||
name: "repository-rebind-different-origin",
|
||||
baseRef: "HEAD",
|
||||
ownerKind: "session",
|
||||
ownerId: "agent:main:different-origin",
|
||||
});
|
||||
await fs.writeFile(path.join(created.path, "README.md"), "do not snapshot or remove\n");
|
||||
const liveBranch = await git(liveRepo, "rev-parse", created.branch);
|
||||
const staleHead = await git(repo, "rev-parse", "HEAD");
|
||||
const snapshotRef = `refs/openclaw/snapshots/${created.id}`;
|
||||
await git(repo, "branch", created.branch, staleHead);
|
||||
await git(repo, "update-ref", snapshotRef, staleHead);
|
||||
openOpenClawStateDatabase({ env })
|
||||
.db.prepare("UPDATE worktrees SET repo_root = ?, repo_fingerprint = ? WHERE id = ?")
|
||||
.run(staleIdentity.repoRoot, staleIdentity.fingerprint, created.id);
|
||||
const registered = getRegistryWorktree(env, created.id);
|
||||
|
||||
await expect(service.remove({ id: created.id, reason: "different-origin" })).rejects.toThrow(
|
||||
"origin",
|
||||
);
|
||||
|
||||
expect(getRegistryWorktree(env, created.id)).toEqual(registered);
|
||||
expect(registered).toMatchObject({
|
||||
repoRoot: staleIdentity.repoRoot,
|
||||
repoFingerprint: staleIdentity.fingerprint,
|
||||
path: created.path,
|
||||
ownerId: "agent:main:different-origin",
|
||||
});
|
||||
expect(await fs.readFile(path.join(created.path, "README.md"), "utf8")).toBe(
|
||||
"do not snapshot or remove\n",
|
||||
);
|
||||
expect(await git(liveRepo, "rev-parse", created.branch)).toBe(liveBranch);
|
||||
await expect(git(liveRepo, "show-ref", "--verify", snapshotRef)).rejects.toThrow();
|
||||
expect(await git(repo, "rev-parse", created.branch)).toBe(staleHead);
|
||||
expect(await git(repo, "rev-parse", snapshotRef)).toBe(staleHead);
|
||||
});
|
||||
|
||||
it("repairs the live repository before lossless cleanup releases its Git lock", async () => {
|
||||
const liveRepo = await cloneRepository("live-lossless");
|
||||
const liveIdentity = await service.resolveRepositoryIdentity(liveRepo);
|
||||
const staleIdentity = await service.resolveRepositoryIdentity(repo);
|
||||
const created = await service.create({
|
||||
repoRoot: liveIdentity.repoRoot,
|
||||
name: "repository-rebind-lossless",
|
||||
baseRef: "HEAD",
|
||||
ownerKind: "workboard",
|
||||
ownerId: "card-repository-rebind",
|
||||
});
|
||||
await service.acquire(created.id);
|
||||
openOpenClawStateDatabase({ env })
|
||||
.db.prepare("UPDATE worktrees SET repo_root = ?, repo_fingerprint = ? WHERE id = ?")
|
||||
.run(staleIdentity.repoRoot, staleIdentity.fingerprint, created.id);
|
||||
|
||||
await expect(service.removeIfLossless(created.id)).resolves.toBe(true);
|
||||
|
||||
expect(getRegistryWorktree(env, created.id)).toMatchObject({
|
||||
repoRoot: liveIdentity.repoRoot,
|
||||
repoFingerprint: liveIdentity.fingerprint,
|
||||
removedAt: expect.any(Number),
|
||||
runEndCleanup: { outcome: "removed-lossless" },
|
||||
});
|
||||
await expect(fs.stat(created.path)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
});
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"canonicalizes managed paths minted below a symlinked state directory",
|
||||
async () => {
|
||||
|
||||
@@ -186,7 +186,6 @@ async function generateName(
|
||||
type ResolvedRepository = {
|
||||
repoRoot: string;
|
||||
sourceRoot: string;
|
||||
commonDir: string;
|
||||
originUrl: string;
|
||||
fingerprint: string;
|
||||
};
|
||||
@@ -216,7 +215,7 @@ async function resolveRepositoryFromRealPath(
|
||||
.update(`${commonDir}\n${originUrl}`)
|
||||
.digest("hex")
|
||||
.slice(0, 16);
|
||||
return { repoRoot: canonicalRoot, sourceRoot, commonDir, originUrl, fingerprint };
|
||||
return { repoRoot: canonicalRoot, sourceRoot, originUrl, fingerprint };
|
||||
}
|
||||
|
||||
async function resolveRepository(repoRoot: string): Promise<ResolvedRepository> {
|
||||
@@ -929,7 +928,7 @@ export class ManagedWorktreeService {
|
||||
claimToken?: string;
|
||||
runEndCleanup?: ManagedWorktreeRunEndCleanup;
|
||||
}): Promise<RemoveManagedWorktreeResult> {
|
||||
const record = this.requireLiveRecord(params.id);
|
||||
let record = this.requireLiveRecord(params.id);
|
||||
// Claim removal before any cleanliness or snapshot work so a live run lease
|
||||
// rejects it and an admitted run cannot start once the claim is held. The
|
||||
// opaque token makes the claim exclusive against competing removers; a caller
|
||||
@@ -937,6 +936,7 @@ export class ManagedWorktreeService {
|
||||
const claimToken = params.claimToken ?? randomUUID();
|
||||
claimWorktreeRemoval(this.env, { worktreeId: record.id, token: claimToken });
|
||||
try {
|
||||
record = await this.rebindLiveRepository(record);
|
||||
const state = await lockState(record);
|
||||
if (state.kind === "live" || state.kind === "foreign") {
|
||||
throw new Error(
|
||||
@@ -1077,7 +1077,7 @@ export class ManagedWorktreeService {
|
||||
}
|
||||
|
||||
async removeIfLossless(id: string): Promise<boolean> {
|
||||
const record = this.requireLiveRecord(id);
|
||||
let record = this.requireLiveRecord(id);
|
||||
const claimToken = randomUUID();
|
||||
const recordOutcome = (outcome: ManagedWorktreeRunEndCleanupOutcome, error?: unknown) => {
|
||||
// Retained/failed writes happen after this remover released or aborted its
|
||||
@@ -1125,6 +1125,7 @@ export class ManagedWorktreeService {
|
||||
throw error;
|
||||
}
|
||||
try {
|
||||
record = await this.rebindLiveRepository(record);
|
||||
const status = await requireGit(record.path, ["status", "--porcelain"]);
|
||||
const unpushed = await requireGit(record.path, [
|
||||
"log",
|
||||
@@ -1368,6 +1369,27 @@ export class ManagedWorktreeService {
|
||||
return record;
|
||||
}
|
||||
|
||||
private async rebindLiveRepository(
|
||||
record: ManagedWorktreeRecord,
|
||||
): Promise<ManagedWorktreeRecord> {
|
||||
const worktreePath = await fs.realpath(record.path);
|
||||
const repository = await resolveRepositoryFromRealPath(worktreePath, record.path);
|
||||
if (repository.sourceRoot !== worktreePath) {
|
||||
throw new WorktreeRepositoryError(`repository does not own worktree: ${record.path}`);
|
||||
}
|
||||
const registeredRepository = await resolveRepository(record.repoRoot);
|
||||
if (registeredRepository.originUrl !== repository.originUrl) {
|
||||
throw new WorktreeRepositoryError(`repository origin does not match: ${record.path}`);
|
||||
}
|
||||
updateRegistryWorktree(this.env, record.id, {
|
||||
repositoryIdentity: {
|
||||
repoRoot: repository.repoRoot,
|
||||
repoFingerprint: repository.fingerprint,
|
||||
},
|
||||
});
|
||||
return { ...record, repoRoot: repository.repoRoot, repoFingerprint: repository.fingerprint };
|
||||
}
|
||||
|
||||
private async reconcileOrphans(records: ManagedWorktreeRecord[]): Promise<number> {
|
||||
const managedPaths = new Set<string>();
|
||||
for (const record of records) {
|
||||
|
||||
Reference in New Issue
Block a user