From c54ddfd07f364f9de41ffbaca23c3bf007c76f9f Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 19 Jul 2026 00:48:41 +0200 Subject: [PATCH] fix(migrate): preserve checkout and shell semantics --- .../app/src/desktop/migrations/conductor.tsx | 4 +- packages/client/src/node.ts | 51 ++++++++++++-- .../sources/conductor/project-config.test.ts | 22 ++++++ .../src/sources/conductor/project-config.ts | 15 +++- .../migration-host-automation.e2e.test.ts | 69 +++++++++++++++++++ packages/server/src/server/worktree-core.ts | 17 +++++ packages/server/src/utils/worktree.ts | 11 --- 7 files changed, 167 insertions(+), 22 deletions(-) diff --git a/packages/app/src/desktop/migrations/conductor.tsx b/packages/app/src/desktop/migrations/conductor.tsx index 8f8ca340f..6a2f31ab5 100644 --- a/packages/app/src/desktop/migrations/conductor.tsx +++ b/packages/app/src/desktop/migrations/conductor.tsx @@ -33,7 +33,7 @@ export function ConductorMigration() { return ( <> - + @@ -60,5 +60,3 @@ const styles = StyleSheet.create((theme) => ({ titleRow: { flexDirection: "row", alignItems: "center", gap: theme.spacing[2] }, icon: { width: theme.iconSize.md, height: theme.iconSize.md }, })); - -const conductorRowStyle = [settingsStyles.row, settingsStyles.rowBorder]; diff --git a/packages/client/src/node.ts b/packages/client/src/node.ts index 0d4a1c104..f977726e2 100644 --- a/packages/client/src/node.ts +++ b/packages/client/src/node.ts @@ -1,7 +1,10 @@ +import { execFile } from "node:child_process"; import { randomUUID } from "node:crypto"; import { existsSync, readFileSync } from "node:fs"; import os from "node:os"; import path from "node:path"; +import { promisify } from "node:util"; +import { slugify } from "@getpaseo/protocol/branch-slug"; import { buildDaemonWebSocketUrl, buildRelayWebSocketUrl, @@ -19,6 +22,7 @@ import { DaemonClient, type WebSocketLike } from "./daemon-client.js"; const DEFAULT_HOST = "localhost:6767"; const DEFAULT_TIMEOUT_MS = 15_000; +const execFileAsync = promisify(execFile); export interface NodeHostConnectionOptions { appVersion: string; @@ -325,15 +329,21 @@ class DaemonHostAutomation implements HostAutomation { refName: string; directoryName: string; }): Promise<{ path: string; created: boolean }> { + const directorySlug = slugify(input.directoryName); + if (!directorySlug) + throw new Error(`Unable to derive a checkout name from ${input.directoryName}`); const listed = await this.client.getPaseoWorktreeList({ repoRoot: input.rootPath }); if (listed.error) throw new Error(`Unable to inspect existing checkouts: ${listed.error.message}`); const existing = listed.worktrees.find( - (worktree) => - worktree.branchName === input.refName && - path.basename(worktree.worktreePath) === input.directoryName, + (worktree) => path.basename(worktree.worktreePath) === directorySlug, ); - if (existing) { + if (existing && existing.branchName !== input.refName) { + throw new Error( + `Checkout name ${directorySlug} is already used by branch ${existing.branchName ?? "unknown"}.`, + ); + } + if (existing && existsSync(existing.worktreePath)) { const opened = await this.client.openProject(existing.worktreePath); if (!opened.workspace) { throw new Error(opened.error ?? `Unable to open ${existing.worktreePath}`); @@ -341,9 +351,11 @@ class DaemonHostAutomation implements HostAutomation { return { path: existing.worktreePath, created: false }; } + await removeMissingCheckoutRegistration(input.rootPath, input.refName); + const result = await this.client.createPaseoWorktree({ cwd: input.rootPath, - worktreeSlug: input.directoryName, + worktreeSlug: directorySlug, action: "checkout", refName: input.refName, }); @@ -357,6 +369,35 @@ class DaemonHostAutomation implements HostAutomation { } } +async function removeMissingCheckoutRegistration(rootPath: string, refName: string): Promise { + const { stdout } = await execFileAsync("git", ["worktree", "list", "--porcelain"], { + cwd: rootPath, + encoding: "utf8", + }); + const expectedBranch = `refs/heads/${refName.replace(/^refs\/heads\//, "")}`; + const stale = parseGitWorktreeList(stdout).find( + (worktree) => worktree.branch === expectedBranch && !existsSync(worktree.path), + ); + if (!stale) return; + + // ensureCheckout is the explicit repair boundary: remove only the missing + // registration that blocks the requested branch, never unrelated worktrees. + await execFileAsync("git", ["worktree", "remove", "--force", stale.path], { + cwd: rootPath, + }); +} + +function parseGitWorktreeList(stdout: string): Array<{ path: string; branch: string | null }> { + return stdout + .trim() + .split(/\n\s*\n/) + .flatMap((entry) => { + const worktreePath = entry.match(/^worktree (.+)$/m)?.[1]; + if (!worktreePath) return []; + return [{ path: worktreePath, branch: entry.match(/^branch (.+)$/m)?.[1] ?? null }]; + }); +} + export async function connectHostAutomation( options: NodeHostConnectionOptions, ): Promise { diff --git a/packages/migrate/src/sources/conductor/project-config.test.ts b/packages/migrate/src/sources/conductor/project-config.test.ts index 727a940cd..ca134ad9d 100644 --- a/packages/migrate/src/sources/conductor/project-config.test.ts +++ b/packages/migrate/src/sources/conductor/project-config.test.ts @@ -118,6 +118,28 @@ run = 42 }); }); +test("skips cwd scripts on Windows instead of emitting POSIX shell syntax", () => { + const repo = emptyRepo("windows-cwd"); + writeSettings( + repo, + ` +[scripts.run.dev] +command = "npm run dev" +[scripts.run.dev.options] +cwd = "apps/web" +`, + ); + + const inspected = inspectConductorProjectConfig(repo, {}, "win32"); + + expect(inspected.config).toBeNull(); + expect(inspected.notices).toContainEqual({ + code: "conductor-setting-unsupported", + level: "warning", + message: "scripts.dev.cwd: Working-directory scripts are not imported on Windows.", + }); +}); + function fixtureRepo(name: "current" | "legacy"): string { const directory = mkdtempSync(path.join(os.tmpdir(), `paseo-migrate-${name}-`)); cleanup.push(directory); diff --git a/packages/migrate/src/sources/conductor/project-config.ts b/packages/migrate/src/sources/conductor/project-config.ts index e94404b97..a3e463409 100644 --- a/packages/migrate/src/sources/conductor/project-config.ts +++ b/packages/migrate/src/sources/conductor/project-config.ts @@ -51,6 +51,7 @@ type RewriteContext = "lifecycle" | "run"; export function inspectConductorProjectConfig( repoRoot: string, databaseSettings: ConductorSettings = {}, + platform: NodeJS.Platform = process.platform, ): { config: PaseoConfigRaw | null; notices: MigrationNotice[] } { const settings = mergeSettings( databaseSettings, @@ -61,7 +62,7 @@ export function inspectConductorProjectConfig( mapLifecycle(settings.scripts?.setup, "worktree.setup", "setup", config, notices); mapLifecycle(settings.scripts?.archive, "worktree.teardown", "teardown", config, notices); - mapRunScripts(settings.scripts?.run, config, notices); + mapRunScripts(settings.scripts?.run, config, notices, platform); mapMetadataPrompts(settings.prompts, config, notices); reportUnsupported(repoRoot, settings, notices); @@ -168,11 +169,12 @@ function mapRunScripts( runConfig: unknown, config: PaseoConfigRaw, notices: MigrationNotice[], + platform: NodeJS.Platform, ): void { if (runConfig === undefined) return; const services = new Map(); if (typeof runConfig === "string") { - mapRunScript("run", { command: runConfig }, config, notices, services); + mapRunScript("run", { command: runConfig }, config, notices, services, platform); return; } if (!isRecord(runConfig)) { @@ -186,7 +188,7 @@ function mapRunScripts( continue; } const script = normalizeRunScript(scriptId, value, notices); - if (script) mapRunScript(scriptId, script, config, notices, services); + if (script) mapRunScript(scriptId, script, config, notices, services, platform); } } @@ -268,6 +270,7 @@ function mapRunScript( config: PaseoConfigRaw, notices: MigrationNotice[], serviceNames: Map, + platform: NodeJS.Platform, ): void { const key = `scripts.${scriptId}`; if (script.hide) { @@ -287,6 +290,12 @@ function mapRunScript( let command = appendArgs(script.command, script.args ?? []); if (script.cwd) { + if (platform === "win32") { + notices.push( + unsupportedSetting(`${key}.cwd`, "Working-directory scripts are not imported on Windows."), + ); + return; + } const cwdPrefix = safeCwdPrefix(script.cwd); if (!cwdPrefix) { notices.push( diff --git a/packages/server/src/server/daemon-e2e/migration-host-automation.e2e.test.ts b/packages/server/src/server/daemon-e2e/migration-host-automation.e2e.test.ts index 6e2b61dc8..c95e4dfb6 100644 --- a/packages/server/src/server/daemon-e2e/migration-host-automation.e2e.test.ts +++ b/packages/server/src/server/daemon-e2e/migration-host-automation.e2e.test.ts @@ -94,6 +94,75 @@ test("a different live checkout continues to protect its branch", async () => { expect(realpathSync(existingPath)).toBe(existingPath); }); +test("ensureCheckout repairs only the requested missing registration", async () => { + const repoRoot = createRepository(); + execFileSync("git", ["branch", "unrelated"], { cwd: repoRoot }); + const staleFeature = path.join(path.dirname(repoRoot), "stale-feature"); + const staleUnrelated = path.join(path.dirname(repoRoot), "stale-unrelated"); + execFileSync("git", ["worktree", "add", staleFeature, "feature"], { cwd: repoRoot }); + execFileSync("git", ["worktree", "add", staleUnrelated, "unrelated"], { cwd: repoRoot }); + rmSync(staleFeature, { recursive: true, force: true }); + rmSync(staleUnrelated, { recursive: true, force: true }); + + const daemon = await createTestPaseoDaemon(); + cleanupDaemons.add(daemon); + const paseo = await connectHostAutomation({ + appVersion: "0.1.110", + clientId: "migration-stale-registration-e2e", + env: {}, + host: `127.0.0.1:${daemon.port}`, + }); + cleanupConnections.add(paseo); + + const ensured = await paseo.ensureCheckout({ + rootPath: repoRoot, + refName: "feature", + directoryName: "imported-feature", + }); + + expect(path.basename(ensured.path)).toBe("imported-feature"); + const registrations = execFileSync("git", ["worktree", "list", "--porcelain"], { + cwd: repoRoot, + encoding: "utf8", + }); + expect(registrations).not.toContain(staleFeature); + expect(registrations).toContain(staleUnrelated); +}); + +test("normalized checkout-name collisions cannot reuse another branch", async () => { + const repoRoot = createRepository(); + execFileSync("git", ["branch", "other-feature"], { cwd: repoRoot }); + const daemon = await createTestPaseoDaemon(); + cleanupDaemons.add(daemon); + const paseo = await connectHostAutomation({ + appVersion: "0.1.110", + clientId: "migration-slug-collision-e2e", + env: {}, + host: `127.0.0.1:${daemon.port}`, + }); + cleanupConnections.add(paseo); + + const first = await paseo.ensureCheckout({ + rootPath: repoRoot, + refName: "feature", + directoryName: "Foo.Bar", + }); + await expect( + paseo.ensureCheckout({ + rootPath: repoRoot, + refName: "other-feature", + directoryName: "foo-bar", + }), + ).rejects.toThrow(/already used by branch feature/); + + expect( + execFileSync("git", ["branch", "--show-current"], { + cwd: first.path, + encoding: "utf8", + }).trim(), + ).toBe("feature"); +}); + test("the public connector authenticates to a real password-protected daemon and closes cleanly", async () => { const daemon = await createTestPaseoDaemon({ auth: { password: hashDaemonPassword("connector-secret") }, diff --git a/packages/server/src/server/worktree-core.ts b/packages/server/src/server/worktree-core.ts index 90d078890..dca8109f7 100644 --- a/packages/server/src/server/worktree-core.ts +++ b/packages/server/src/server/worktree-core.ts @@ -117,6 +117,7 @@ export async function createWorktreeCore( worktreesRoot: input.worktreesRoot, }); if (existingWorktree) { + assertExistingWorktreeMatchesIntent(existingWorktree, intent, normalizedSlug); return { worktree: existingWorktree, intent, repoRoot, created: false }; } @@ -135,6 +136,22 @@ export async function createWorktreeCore( }; } +function assertExistingWorktreeMatchesIntent( + existingWorktree: WorktreeConfig, + intent: WorktreeCreationIntent, + slug: string, +): void { + const intendedBranchName = + intent.kind === "checkout-change-request" || intent.kind === "checkout-github-pr" + ? (intent.localBranchName ?? intent.headRef) + : intent.branchName; + if (existingWorktree.branchName !== intendedBranchName) { + throw new Error( + `Worktree name ${slug} is already used by branch ${existingWorktree.branchName}.`, + ); + } +} + async function resolveForge( repoRoot: string, deps: CreateWorktreeCoreDeps, diff --git a/packages/server/src/utils/worktree.ts b/packages/server/src/utils/worktree.ts index 993a056a1..64d5ae2c7 100644 --- a/packages/server/src/utils/worktree.ts +++ b/packages/server/src/utils/worktree.ts @@ -1226,7 +1226,6 @@ export const createWorktree = async ({ paseoHome, worktreesRoot, }: CreateWorktreeOptions): Promise => { - await pruneVerifiedStaleWorktrees(cwd); const sourcePlan = await resolveWorktreeSourcePlan({ cwd, source, desiredSlug: worktreeSlug }); let worktreePath = join(await getPaseoWorktreesRoot(cwd, paseoHome, worktreesRoot), worktreeSlug); mkdirSync(dirname(worktreePath), { recursive: true }); @@ -1284,16 +1283,6 @@ export const createWorktree = async ({ }; }; -async function pruneVerifiedStaleWorktrees(cwd: string): Promise { - const { stdout } = await runGitCommand(["worktree", "list", "--porcelain"], { - cwd, - envOverlay: READ_ONLY_GIT_ENV, - }); - const hasStaleRegistration = parseWorktreeList(stdout).some((entry) => !existsSync(entry.path)); - if (!hasStaleRegistration) return; - await runGitCommand(["worktree", "prune"], { cwd, timeout: 30_000 }); -} - interface ResolveWorktreeSourcePlanOptions { cwd: string; source: WorktreeSource;