mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
fix(projects): preserve metadata across Git read failures
Refresh archived workspace facts before persistence and treat only confirmed non-repositories as non-Git so transient failures cannot rewrite stored project metadata.
This commit is contained in:
@@ -1392,7 +1392,6 @@ function SessionProviderInternal({ children, serverId, client }: SessionProvider
|
||||
});
|
||||
|
||||
const unsubProjectUpdate = client.on("project.update", (message) => {
|
||||
if (message.type !== "project.update") return;
|
||||
const update = message.payload;
|
||||
if (update.kind === "remove") {
|
||||
useSessionStore.getState().applyProjectUpdate(serverId, update);
|
||||
|
||||
@@ -9,6 +9,7 @@ import { createNoopWorkspaceGitService } from "../../test-utils/workspace-git-se
|
||||
import {
|
||||
FileBackedProjectRegistry,
|
||||
FileBackedWorkspaceRegistry,
|
||||
type WorkspaceRegistry,
|
||||
} from "../../workspace-registry.js";
|
||||
import type { CreatePaseoWorktreeWorkflowResult } from "../../worktree-session.js";
|
||||
import {
|
||||
@@ -26,6 +27,8 @@ const ARCHIVED_AT = "2026-01-01T00:00:00.000Z";
|
||||
|
||||
let tmpDir: string;
|
||||
let gitRoots: Set<string>;
|
||||
let gitBranches: Map<string, string | null>;
|
||||
let checkoutFailure: Error | null;
|
||||
let workspaceRegistry: FileBackedWorkspaceRegistry;
|
||||
let projectRegistry: FileBackedProjectRegistry;
|
||||
let provisioning: WorkspaceProvisioningService;
|
||||
@@ -34,6 +37,7 @@ function gitService() {
|
||||
return createNoopWorkspaceGitService({
|
||||
peekSnapshot: () => null,
|
||||
getCheckout: async (cwd: string) => {
|
||||
if (checkoutFailure) throw checkoutFailure;
|
||||
let worktreeRoot: string | null = null;
|
||||
for (const root of gitRoots) {
|
||||
if (
|
||||
@@ -46,7 +50,7 @@ function gitService() {
|
||||
return {
|
||||
cwd,
|
||||
isGit: worktreeRoot !== null,
|
||||
currentBranch: worktreeRoot ? "main" : null,
|
||||
currentBranch: worktreeRoot ? (gitBranches.get(worktreeRoot) ?? "main") : null,
|
||||
remoteUrl: null,
|
||||
worktreeRoot,
|
||||
isPaseoOwnedWorktree: false,
|
||||
@@ -59,6 +63,8 @@ function gitService() {
|
||||
beforeEach(async () => {
|
||||
tmpDir = mkdtempSync(path.join(os.tmpdir(), "workspace-provisioning-"));
|
||||
gitRoots = new Set();
|
||||
gitBranches = new Map();
|
||||
checkoutFailure = null;
|
||||
workspaceRegistry = new FileBackedWorkspaceRegistry(
|
||||
path.join(tmpDir, "projects", "workspaces.json"),
|
||||
logger,
|
||||
@@ -122,6 +128,56 @@ test("re-opening an archived workspace by its exact path unarchives it and keeps
|
||||
expect(reopened.archivedAt).toBeNull();
|
||||
});
|
||||
|
||||
test("uses one workspace snapshot when reopening an archived workspace", async () => {
|
||||
const repo = path.join(tmpDir, "repo");
|
||||
gitRoots.add(repo);
|
||||
const created = await provisioning.findOrCreateWorkspaceForDirectory(repo);
|
||||
await workspaceRegistry.archive(created.workspaceId, ARCHIVED_AT);
|
||||
|
||||
const archived = (await workspaceRegistry.list()).filter(
|
||||
(workspace) => workspace.workspaceId === created.workspaceId,
|
||||
);
|
||||
let reads = 0;
|
||||
const snapshotRegistry: WorkspaceRegistry = {
|
||||
initialize: () => workspaceRegistry.initialize(),
|
||||
existsOnDisk: () => workspaceRegistry.existsOnDisk(),
|
||||
list: async () => (reads++ === 0 ? archived : []),
|
||||
get: (workspaceId) => workspaceRegistry.get(workspaceId),
|
||||
upsert: (workspace) => workspaceRegistry.upsert(workspace),
|
||||
archive: (workspaceId, archivedAt) => workspaceRegistry.archive(workspaceId, archivedAt),
|
||||
remove: (workspaceId) => workspaceRegistry.remove(workspaceId),
|
||||
};
|
||||
const snapshotProvisioning = createWorkspaceProvisioningService({
|
||||
workspaceRegistry: snapshotRegistry,
|
||||
projectRegistry,
|
||||
workspaceGitService: gitService(),
|
||||
});
|
||||
|
||||
const reopened = await snapshotProvisioning.findOrCreateWorkspaceForDirectory(repo);
|
||||
|
||||
expect(reopened).toMatchObject({ workspaceId: created.workspaceId, archivedAt: null });
|
||||
expect(await workspaceRegistry.list()).toHaveLength(1);
|
||||
});
|
||||
|
||||
test("reopening an archived workspace refreshes git-derived kind and branch", async () => {
|
||||
const repo = path.join(tmpDir, "repo");
|
||||
gitRoots.add(repo);
|
||||
const created = await provisioning.findOrCreateWorkspaceForDirectory(repo);
|
||||
await workspaceRegistry.archive(created.workspaceId, ARCHIVED_AT);
|
||||
gitRoots.delete(repo);
|
||||
|
||||
const reopened = await provisioning.findOrCreateWorkspaceForDirectory(repo);
|
||||
|
||||
expect(reopened).toEqual({
|
||||
...created,
|
||||
kind: "directory",
|
||||
branch: null,
|
||||
archivedAt: null,
|
||||
updatedAt: expect.any(String),
|
||||
});
|
||||
expect(await workspaceRegistry.get(created.workspaceId)).toEqual(reopened);
|
||||
});
|
||||
|
||||
test("opening a subpath of an archived git workspace mints a fresh workspace at the exact subpath", async () => {
|
||||
const repo = path.join(tmpDir, "repo");
|
||||
gitRoots.add(repo);
|
||||
@@ -152,6 +208,24 @@ test("ensureWorkspaceRecordUnarchived restores the owning archived project with
|
||||
expect((await projectRegistry.get(created.projectId))?.archivedAt).toBeNull();
|
||||
});
|
||||
|
||||
test("does not unarchive either record when checkout refresh fails", async () => {
|
||||
const repo = path.join(tmpDir, "repo");
|
||||
gitRoots.add(repo);
|
||||
const created = await provisioning.findOrCreateWorkspaceForDirectory(repo);
|
||||
await projectRegistry.archive(created.projectId, ARCHIVED_AT);
|
||||
await workspaceRegistry.archive(created.workspaceId, ARCHIVED_AT);
|
||||
const archivedProject = await projectRegistry.get(created.projectId);
|
||||
const archivedWorkspace = await workspaceRegistry.get(created.workspaceId);
|
||||
checkoutFailure = new Error("Git read failed");
|
||||
|
||||
await expect(provisioning.ensureWorkspaceRecordUnarchived(archivedWorkspace!)).rejects.toThrow(
|
||||
"Git read failed",
|
||||
);
|
||||
|
||||
expect(await projectRegistry.get(created.projectId)).toEqual(archivedProject);
|
||||
expect(await workspaceRegistry.get(created.workspaceId)).toEqual(archivedWorkspace);
|
||||
});
|
||||
|
||||
test("resolveOrCreateWorkspaceIdForCreateAgent returns a created worktree's id without touching the registry", async () => {
|
||||
// The branch only reads workspace.workspaceId off the worktree result.
|
||||
const createdWorktree = {
|
||||
|
||||
@@ -110,7 +110,8 @@ export function createWorkspaceProvisioningService(deps: {
|
||||
|
||||
async function findOrCreateWorkspaceForDirectory(cwd: string): Promise<PersistedWorkspaceRecord> {
|
||||
const normalizedCwd = resolve(cwd);
|
||||
const active = (await workspaceRegistry.list())
|
||||
const workspaces = await workspaceRegistry.list();
|
||||
const active = workspaces
|
||||
.filter((workspace) => !workspace.archivedAt && workspace.cwd === normalizedCwd)
|
||||
.sort(
|
||||
(left, right) =>
|
||||
@@ -118,7 +119,7 @@ export function createWorkspaceProvisioningService(deps: {
|
||||
left.workspaceId.localeCompare(right.workspaceId),
|
||||
)[0];
|
||||
if (active) return active;
|
||||
const archived = (await workspaceRegistry.list())
|
||||
const archived = workspaces
|
||||
.filter((workspace) => workspace.archivedAt && workspace.cwd === normalizedCwd)
|
||||
.sort(
|
||||
(left, right) =>
|
||||
@@ -146,11 +147,24 @@ export function createWorkspaceProvisioningService(deps: {
|
||||
const project = await projectRegistry.get(workspace.projectId);
|
||||
if (!project) throw new Error(`Unknown project: ${workspace.projectId}`);
|
||||
const timestamp = new Date().toISOString();
|
||||
let next: PersistedWorkspaceRecord | null = null;
|
||||
if (workspace.archivedAt) {
|
||||
const checkout = await workspaceGitService.getCheckout(workspace.cwd);
|
||||
next = {
|
||||
...workspace,
|
||||
kind: deriveWorkspaceKind(checkout),
|
||||
branch:
|
||||
checkout.currentBranch && checkout.currentBranch.toUpperCase() !== "HEAD"
|
||||
? checkout.currentBranch
|
||||
: null,
|
||||
archivedAt: null,
|
||||
updatedAt: timestamp,
|
||||
};
|
||||
}
|
||||
if (project.archivedAt) {
|
||||
await projectRegistry.upsert({ ...project, archivedAt: null, updatedAt: timestamp });
|
||||
}
|
||||
if (!workspace.archivedAt) return workspace;
|
||||
const next = { ...workspace, archivedAt: null, updatedAt: timestamp };
|
||||
if (!next) return workspace;
|
||||
await workspaceRegistry.upsert(next);
|
||||
return next;
|
||||
}
|
||||
|
||||
@@ -297,6 +297,18 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
test("getCheckout surfaces an unexpected Git read failure", async () => {
|
||||
const service = createService({
|
||||
getCheckoutStatus: vi.fn(async () => {
|
||||
throw new Error("Git read failed");
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(service.getCheckout(REPO_CWD)).rejects.toThrow("Git read failed");
|
||||
|
||||
service.dispose();
|
||||
});
|
||||
|
||||
test("getSnapshot returns the current snapshot without shelling out", async () => {
|
||||
let nowMs = Date.parse("2026-04-12T00:00:00.000Z");
|
||||
const getCheckoutStatus = vi.fn(async (cwd: string) => createCheckoutStatus(cwd));
|
||||
|
||||
@@ -438,31 +438,12 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService {
|
||||
|
||||
async getCheckout(cwd: string): Promise<ProjectCheckoutLitePayload> {
|
||||
const normalizedCwd = resolve(cwd);
|
||||
try {
|
||||
const status = await this.deps.getCheckoutStatus(normalizedCwd, {
|
||||
paseoHome: this.paseoHome,
|
||||
worktreesRoot: this.worktreesRoot,
|
||||
logger: this.logger,
|
||||
});
|
||||
if (!status.isGit) {
|
||||
return checkoutLiteFromGitSnapshot(normalizedCwd, {
|
||||
isGit: false,
|
||||
currentBranch: null,
|
||||
remoteUrl: null,
|
||||
repoRoot: null,
|
||||
isPaseoOwnedWorktree: false,
|
||||
mainRepoRoot: null,
|
||||
});
|
||||
}
|
||||
return checkoutLiteFromGitSnapshot(normalizedCwd, {
|
||||
isGit: true,
|
||||
currentBranch: status.currentBranch,
|
||||
remoteUrl: status.remoteUrl,
|
||||
repoRoot: status.repoRoot,
|
||||
isPaseoOwnedWorktree: status.isPaseoOwnedWorktree,
|
||||
mainRepoRoot: status.mainRepoRoot,
|
||||
});
|
||||
} catch {
|
||||
const status = await this.deps.getCheckoutStatus(normalizedCwd, {
|
||||
paseoHome: this.paseoHome,
|
||||
worktreesRoot: this.worktreesRoot,
|
||||
logger: this.logger,
|
||||
});
|
||||
if (!status.isGit) {
|
||||
return checkoutLiteFromGitSnapshot(normalizedCwd, {
|
||||
isGit: false,
|
||||
currentBranch: null,
|
||||
@@ -472,6 +453,14 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService {
|
||||
mainRepoRoot: null,
|
||||
});
|
||||
}
|
||||
return checkoutLiteFromGitSnapshot(normalizedCwd, {
|
||||
isGit: true,
|
||||
currentBranch: status.currentBranch,
|
||||
remoteUrl: status.remoteUrl,
|
||||
repoRoot: status.repoRoot,
|
||||
isPaseoOwnedWorktree: status.isPaseoOwnedWorktree,
|
||||
mainRepoRoot: status.mainRepoRoot,
|
||||
});
|
||||
}
|
||||
|
||||
peekSnapshot(cwd: string): WorkspaceGitRuntimeSnapshot | null {
|
||||
|
||||
@@ -899,6 +899,51 @@ describe("WorkspaceReconciliationService", () => {
|
||||
expect(projects.get("p1")!.customName).toBe("My Fork");
|
||||
});
|
||||
|
||||
test("keeps persisted Git metadata when a workspace checkout read fails", async () => {
|
||||
const projectRoot = mkdtempSync(path.join(tmpdir(), "reconcile-checkout-read-project-"));
|
||||
const workspaceRoot = path.join(projectRoot, "workspace");
|
||||
mkdirSync(workspaceRoot);
|
||||
tempDirs.push(projectRoot);
|
||||
const { projects, workspaces, projectRegistry, workspaceRegistry } = createTestRegistries();
|
||||
const project = createPersistedProjectRecord({
|
||||
projectId: "p1",
|
||||
rootPath: projectRoot,
|
||||
kind: "non_git",
|
||||
displayName: "project",
|
||||
createdAt: timestamp,
|
||||
updatedAt: timestamp,
|
||||
});
|
||||
const workspace = createPersistedWorkspaceRecord({
|
||||
workspaceId: "w1",
|
||||
projectId: project.projectId,
|
||||
cwd: workspaceRoot,
|
||||
kind: "local_checkout",
|
||||
displayName: "workspace",
|
||||
branch: "feature",
|
||||
createdAt: timestamp,
|
||||
updatedAt: timestamp,
|
||||
});
|
||||
projects.set(project.projectId, project);
|
||||
workspaces.set(workspace.workspaceId, workspace);
|
||||
const service = new WorkspaceReconciliationService({
|
||||
projectRegistry,
|
||||
workspaceRegistry,
|
||||
logger: createTestLogger(),
|
||||
workspaceGitService: {
|
||||
getCheckout: async (cwd) => {
|
||||
if (cwd === workspaceRoot) throw new Error("Git read failed");
|
||||
return createCheckout(cwd, { isGit: true, currentBranch: "main", worktreeRoot: cwd });
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = await service.reconcileGitMetadata();
|
||||
|
||||
expect(result.changesApplied).toEqual([]);
|
||||
expect(projects.get(project.projectId)).toEqual(project);
|
||||
expect(workspaces.get(workspace.workspaceId)).toEqual(workspace);
|
||||
});
|
||||
|
||||
test("updates workspace branch metadata without clobbering the workspace name", async () => {
|
||||
const dir = createTempGitRepo("reconcile-branch-");
|
||||
tempDirs.push(dir);
|
||||
|
||||
@@ -176,26 +176,39 @@ export class WorkspaceReconciliationService {
|
||||
}
|
||||
await Promise.all(
|
||||
roots.map(async ({ rootPath, projects }) => {
|
||||
const rootGit = await readCheckout(rootPath);
|
||||
await Promise.all(
|
||||
projects.map((project) =>
|
||||
this.reconcileProject({
|
||||
project,
|
||||
siblings: workspacesByProject.get(project.projectId) ?? [],
|
||||
currentGit: rootGit,
|
||||
readCheckout,
|
||||
changes,
|
||||
}),
|
||||
),
|
||||
);
|
||||
try {
|
||||
const rootGit = await readCheckout(rootPath);
|
||||
await Promise.all(
|
||||
projects.map((project) =>
|
||||
this.reconcileProject({
|
||||
project,
|
||||
siblings: workspacesByProject.get(project.projectId) ?? [],
|
||||
currentGit: rootGit,
|
||||
readCheckout,
|
||||
changes,
|
||||
}),
|
||||
),
|
||||
);
|
||||
} catch (error) {
|
||||
this.logger.warn(
|
||||
{ err: error, rootPath },
|
||||
"Skipped workspace reconciliation after Git read failed",
|
||||
);
|
||||
}
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
private async reconcileProject(input: ProjectReconciliationInput): Promise<void> {
|
||||
const { project, siblings, currentGit, readCheckout, changes } = input;
|
||||
const existingSiblings = siblings.filter((workspace) => existsSync(workspace.cwd));
|
||||
const workspaceCheckouts = await Promise.all(
|
||||
existingSiblings.map(async (workspace) => ({
|
||||
workspace,
|
||||
checkout: await readCheckout(workspace.cwd),
|
||||
})),
|
||||
);
|
||||
const projectUpdates: Partial<Pick<PersistedProjectRecord, "kind">> = {};
|
||||
|
||||
const mappedKind = deriveProjectKind(currentGit);
|
||||
|
||||
if (project.kind !== mappedKind) {
|
||||
@@ -217,10 +230,8 @@ export class WorkspaceReconciliationService {
|
||||
});
|
||||
}
|
||||
|
||||
const existingSiblings = siblings.filter((workspace) => existsSync(workspace.cwd));
|
||||
await Promise.all(
|
||||
existingSiblings.map(async (workspace) => {
|
||||
const wsGit = await readCheckout(workspace.cwd);
|
||||
workspaceCheckouts.map(async ({ workspace, checkout: wsGit }) => {
|
||||
const expectedKind = deriveWorkspaceKind(wsGit);
|
||||
|
||||
const workspaceUpdates: Partial<Pick<PersistedWorkspaceRecord, "branch" | "kind">> = {};
|
||||
|
||||
@@ -245,6 +245,13 @@ describe("checkout git utilities", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("reports a real non-git directory as non-git", async () => {
|
||||
const nonGitDir = join(tempDir, "not-git-status");
|
||||
mkdirSync(nonGitDir, { recursive: true });
|
||||
|
||||
await expect(getCheckoutStatus(nonGitDir)).resolves.toEqual({ isGit: false });
|
||||
});
|
||||
|
||||
it("returns null for getCurrentBranch in a repo with no commits", async () => {
|
||||
const emptyRepo = join(tempDir, "empty-repo");
|
||||
mkdirSync(emptyRepo, { recursive: true });
|
||||
|
||||
@@ -24,6 +24,7 @@ import { isPaseoOwnedWorktreeCwd, resolvePaseoWorktreesBaseRoot } from "./worktr
|
||||
import { readPaseoWorktreeMetadata } from "./worktree-metadata.js";
|
||||
const READ_ONLY_GIT_ENV = {
|
||||
GIT_OPTIONAL_LOCKS: "0",
|
||||
LC_ALL: "C",
|
||||
} as const;
|
||||
|
||||
/**
|
||||
@@ -806,11 +807,11 @@ export type CheckoutSnapshotFacts =
|
||||
pullRequestLookupTarget: PullRequestStatusLookupTarget | null;
|
||||
};
|
||||
|
||||
function isGitError(error: unknown): boolean {
|
||||
function isNotGitRepositoryError(error: unknown): boolean {
|
||||
if (!(error instanceof Error)) {
|
||||
return false;
|
||||
}
|
||||
return /not a git repository/i.test(error.message) || /git repository/i.test(error.message);
|
||||
return /not a git repository \(or any of the parent directories\): \.git/i.test(error.message);
|
||||
}
|
||||
|
||||
async function requireGitRepo(cwd: string): Promise<void> {
|
||||
@@ -867,8 +868,11 @@ async function getWorktreeRoot(cwd: string, context?: CheckoutContext): Promise<
|
||||
logger: context?.logger,
|
||||
});
|
||||
return parseGitRevParsePath(stdout);
|
||||
} catch {
|
||||
return null;
|
||||
} catch (error) {
|
||||
if (isNotGitRepositoryError(error)) {
|
||||
return null;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1535,35 +1539,29 @@ async function inspectCheckoutContext(
|
||||
cwd: string,
|
||||
context?: CheckoutContext,
|
||||
): Promise<CheckoutInspectionContext | null> {
|
||||
try {
|
||||
const root = await getWorktreeRoot(cwd, context);
|
||||
if (!root) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const [currentBranch, remoteUrl, absoluteGitDir, gitCommonDir, paseoWorktree] =
|
||||
await Promise.all([
|
||||
getCurrentBranch(cwd),
|
||||
getOriginRemoteUrl(cwd),
|
||||
resolveAbsoluteGitDir(cwd),
|
||||
resolveGitCommonDir(cwd),
|
||||
getPaseoWorktreeForCwd(cwd, context, root),
|
||||
]);
|
||||
|
||||
return {
|
||||
worktreeRoot: root,
|
||||
currentBranch,
|
||||
remoteUrl,
|
||||
absoluteGitDir,
|
||||
gitCommonDir,
|
||||
paseoWorktree,
|
||||
};
|
||||
} catch (error) {
|
||||
if (isGitError(error)) {
|
||||
return null;
|
||||
}
|
||||
throw error;
|
||||
const root = await getWorktreeRoot(cwd, context);
|
||||
if (!root) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const [currentBranch, remoteUrl, absoluteGitDir, gitCommonDir, paseoWorktree] = await Promise.all(
|
||||
[
|
||||
getCurrentBranch(cwd),
|
||||
getOriginRemoteUrl(cwd),
|
||||
resolveAbsoluteGitDir(cwd),
|
||||
resolveGitCommonDir(cwd),
|
||||
getPaseoWorktreeForCwd(cwd, context, root),
|
||||
],
|
||||
);
|
||||
|
||||
return {
|
||||
worktreeRoot: root,
|
||||
currentBranch,
|
||||
remoteUrl,
|
||||
absoluteGitDir,
|
||||
gitCommonDir,
|
||||
paseoWorktree,
|
||||
};
|
||||
}
|
||||
|
||||
function buildPullRequestLookupTargetFromBranchConfig(
|
||||
|
||||
Reference in New Issue
Block a user