From 44da3cdf058a8a90493f48623af3c293ddf36ec6 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 19 Jul 2026 01:33:48 +0200 Subject: [PATCH] fix(migrate): preserve consistent source state --- .../desktop/migrations/migration-sheet.tsx | 9 +- .../src/sources/conductor/database.test.ts | 12 +- .../migrate/src/sources/conductor/database.ts | 8 +- .../sources/conductor/project-config.test.ts | 19 +-- .../src/sources/conductor/project-config.ts | 111 ++++++++++-------- 5 files changed, 94 insertions(+), 65 deletions(-) diff --git a/packages/app/src/desktop/migrations/migration-sheet.tsx b/packages/app/src/desktop/migrations/migration-sheet.tsx index dd169c59b..036e8765e 100644 --- a/packages/app/src/desktop/migrations/migration-sheet.tsx +++ b/packages/app/src/desktop/migrations/migration-sheet.tsx @@ -70,6 +70,9 @@ export function MigrationSheet({ const { t } = useTranslation(); const [state, setState] = useState({ status: "confirm" }); const header = useMemo(() => ({ title: source.sheetTitle }), [source.sheetTitle]); + const close = useCallback(() => { + if (state.status !== "running") onClose(); + }, [onClose, state.status]); useEffect(() => { if (!visible) setState({ status: "confirm" }); @@ -120,17 +123,17 @@ export function MigrationSheet({ return ; } return ( - ); - }, [onClose, start, state.status, t]); + }, [close, start, state.status, t]); return ( { + const databasePath = temporaryDatabase(); + cpSync(fixturePath, databasePath); + writeFileSync(`${databasePath}-wal`, "pending"); + + await expect(readConductorCatalog(databasePath)).rejects.toThrow( + "Quit Conductor before migrating", + ); +}); + test("reports malformed database JSON and script columns per project", async () => { const databasePath = temporaryDatabase(); cpSync(fixturePath, databasePath); diff --git a/packages/migrate/src/sources/conductor/database.ts b/packages/migrate/src/sources/conductor/database.ts index 8b458f554..087c28f73 100644 --- a/packages/migrate/src/sources/conductor/database.ts +++ b/packages/migrate/src/sources/conductor/database.ts @@ -1,4 +1,4 @@ -import { readFileSync } from "node:fs"; +import { existsSync, readFileSync, statSync } from "node:fs"; import { createRequire } from "node:module"; import initSqlJs, { type Database } from "sql.js"; import type { MigrationNotice } from "../../types.js"; @@ -36,6 +36,12 @@ export interface ConductorCatalog { export class UnsupportedConductorDatabaseError extends Error {} export async function readConductorCatalog(databasePath: string): Promise { + const walPath = `${databasePath}-wal`; + if (existsSync(walPath) && statSync(walPath).size > 0) { + throw new UnsupportedConductorDatabaseError( + "Conductor has pending database changes. Quit Conductor before migrating.", + ); + } const SQL = await sqlite; const database = new SQL.Database(readFileSync(databasePath)); try { diff --git a/packages/migrate/src/sources/conductor/project-config.test.ts b/packages/migrate/src/sources/conductor/project-config.test.ts index 1c2dce44d..fde83c8bc 100644 --- a/packages/migrate/src/sources/conductor/project-config.test.ts +++ b/packages/migrate/src/sources/conductor/project-config.test.ts @@ -132,28 +132,29 @@ test("reports a malformed top-level scripts value", () => { }); }); -test("does not rewrite literal or escaped Conductor variables", () => { +test("preserves literal or escaped Conductor names and rewrites active variables", () => { const inspected = inspectConductorProjectConfig(emptyRepo("literal-variables"), { scripts: { setup: "printf '%s\\n' '$CONDUCTOR_WORKSPACE_PATH'", run: { escaped: { command: "printf \\$CONDUCTOR_PORT" }, escapedDouble: { command: 'printf "\\$CONDUCTOR_PORT"' }, + bare: { command: "printf CONDUCTOR_PORT" }, active: { command: "printf \"'$CONDUCTOR_PORT'\"" }, }, }, }); expect(inspected.config).toEqual({ - scripts: { active: { command: "printf \"'$PASEO_PORT'\"", type: "service" } }, + worktree: { setup: "printf '%s\\n' '$CONDUCTOR_WORKSPACE_PATH'" }, + scripts: { + active: { command: "printf \"'$PASEO_PORT'\"", type: "service" }, + bare: { command: "printf CONDUCTOR_PORT" }, + escaped: { command: "printf \\$CONDUCTOR_PORT" }, + escapedDouble: { command: 'printf "\\$CONDUCTOR_PORT"' }, + }, }); - expect(inspected.notices.map((notice) => notice.message)).toEqual( - expect.arrayContaining([ - "worktree.setup: Literal or escaped Conductor variables: CONDUCTOR_WORKSPACE_PATH. Command was not imported.", - "scripts.escaped: Literal or escaped Conductor variables: CONDUCTOR_PORT. Command was not imported.", - "scripts.escapedDouble: Literal or escaped Conductor variables: CONDUCTOR_PORT. Command was not imported.", - ]), - ); + expect(inspected.notices).toEqual([]); }); test("skips cwd scripts on Windows instead of emitting POSIX shell syntax", () => { diff --git a/packages/migrate/src/sources/conductor/project-config.ts b/packages/migrate/src/sources/conductor/project-config.ts index ac51b7b41..c48988b0c 100644 --- a/packages/migrate/src/sources/conductor/project-config.ts +++ b/packages/migrate/src/sources/conductor/project-config.ts @@ -354,16 +354,6 @@ function rewriteExactCommand( key: string, notices: MigrationNotice[], ): string | null { - const literalVariables = collectLiteralConductorVariables(command); - if (literalVariables.length > 0) { - notices.push( - unsupportedSetting( - key, - `Literal or escaped Conductor variables: ${literalVariables.join(", ")}. Command was not imported.`, - ), - ); - return null; - } let rewritten = command; for (const [from, to] of [ ["CONDUCTOR_WORKSPACE_PATH", "PASEO_WORKTREE_PATH"], @@ -578,9 +568,13 @@ function malformedSetting(key: string, detail: string): MigrationNotice { } function replaceShellVariable(command: string, from: string, to: string): string { + const mask = activeShellMask(command); const pattern = new RegExp(`\\$\\{${from}(?=[}:#%+\\-=?])|\\$${from}(?![A-Za-z0-9_])`, "g"); return replaceArithmeticVariable( - command.replace(pattern, (match) => (match.startsWith("${") ? `\${${to}` : `$${to}`)), + command.replace(pattern, (match, offset: number) => { + if (mask[offset] !== "$") return match; + return match.startsWith("${") ? `\${${to}` : `$${to}`; + }), from, to, ); @@ -588,63 +582,69 @@ function replaceShellVariable(command: string, from: string, to: string): string function collectConductorVariables(command: string): string[] { const names = new Set(); - for (const match of command.matchAll(/(?:\$\{|\$|[^A-Za-z0-9_])(CONDUCTOR_[A-Za-z0-9_]+)/g)) { - if (match[1]) names.add(match[1]); + const mask = activeShellMask(command); + const direct = + /\$\{(CONDUCTOR_[A-Za-z0-9_]+)(?=[}:#%+\-=?])|\$(CONDUCTOR_[A-Za-z0-9_]+)(?![A-Za-z0-9_])/g; + for (const match of command.matchAll(direct)) { + if (mask[match.index] !== "$") continue; + const name = match[1] ?? match[2]; + if (name) names.add(name); } + forEachActiveArithmetic(command, (body) => { + for (const match of body.matchAll(/(?:^|[^A-Za-z0-9_])(CONDUCTOR_[A-Za-z0-9_]+)/g)) { + if (match[1]) names.add(match[1]); + } + }); return [...names].sort(); } -function collectLiteralConductorVariables(command: string): string[] { - const names = new Set(); +function activeShellMask(command: string): string { + const mask = [...command]; let quote: "single" | "double" | null = null; for (let index = 0; index < command.length; index += 1) { const character = command[index]; if (quote === "single") { - if (character === "'") { - quote = null; - continue; - } - if (!command.startsWith("CONDUCTOR_", index)) continue; - const previous = command[index - 1]; - if (previous && /[A-Za-z0-9_]/.test(previous)) continue; - const name = command.slice(index).match(/^CONDUCTOR_[A-Za-z0-9_]+/)?.[0]; - if (name) names.add(name); + mask[index] = " "; + if (character === "'") quote = null; continue; } if (character === "\\") { - if (command[index + 1] === "$") { - const name = readConductorVariable(command, index + 1); - if (name) names.add(name); - } + mask[index] = " "; + if (index + 1 < mask.length) mask[index + 1] = " "; index += 1; continue; } if (quote === "double") { - if (character === '"') quote = null; + if (character === '"') { + mask[index] = " "; + quote = null; + } continue; } - if (character === "'") quote = "single"; - if (character === '"') quote = "double"; + if (character === "'") { + mask[index] = " "; + quote = "single"; + } else if (character === '"') { + mask[index] = " "; + quote = "double"; + } } - return [...names].sort(); -} - -function readConductorVariable(command: string, dollarIndex: number): string | null { - const suffix = command.slice(dollarIndex + 1); - return ( - suffix.match(/^\{(CONDUCTOR_[A-Za-z0-9_]+)/)?.[1] ?? - suffix.match(/^(CONDUCTOR_[A-Za-z0-9_]+)/)?.[1] ?? - null - ); + return mask.join(""); } function containsShellVariable(command: string, name: string): boolean { - const pattern = new RegExp(`\\$\\{${name}(?=[}:#%+\\-=?])|\\$${name}(?![A-Za-z0-9_])`); - return pattern.test(command) || containsArithmeticVariable(command, name); + const mask = activeShellMask(command); + const pattern = new RegExp(`\\$\\{${name}(?=[}:#%+\\-=?])|\\$${name}(?![A-Za-z0-9_])`, "g"); + for (const match of command.matchAll(pattern)) { + if (mask[match.index] === "$") return true; + } + return containsArithmeticVariable(command, name); } function replaceArithmeticVariable(command: string, from: string, to: string): string { - return command.replace(/\$\(\(([\s\S]*?)\)\)/g, (expression, body: string) => { + const mask = activeShellMask(command); + return command.replace(/\$\(\(([\s\S]*?)\)\)/g, (expression, body: string, offset: number) => { + if (mask.slice(offset, offset + 3) !== "$((") return expression; const identifier = new RegExp(`(^|[^A-Za-z0-9_])${from}(?![A-Za-z0-9_])`, "g"); const rewritten = body.replace(identifier, (_match, prefix: string) => `${prefix}${to}`); return rewritten === body ? expression : `$((` + rewritten + `))`; @@ -653,19 +653,28 @@ function replaceArithmeticVariable(command: string, from: string, to: string): s function containsArithmeticVariable(command: string, name: string): boolean { const identifier = new RegExp(`(^|[^A-Za-z0-9_])${name}(?![A-Za-z0-9_])`); - for (const match of command.matchAll(/\$\(\(([\s\S]*?)\)\)/g)) { - if (identifier.test(match[1])) return true; - } - return false; + let found = false; + forEachActiveArithmetic(command, (body) => { + if (identifier.test(body)) found = true; + }); + return found; } function containsArithmeticVariableOperation(command: string, name: string): boolean { const identifier = new RegExp(`(^|[^A-Za-z0-9_])${name}(?![A-Za-z0-9_])`); + let found = false; + forEachActiveArithmetic(command, (value) => { + const body = value.trim(); + if (identifier.test(body) && body !== name) found = true; + }); + return found; +} + +function forEachActiveArithmetic(command: string, visit: (body: string) => void): void { + const mask = activeShellMask(command); for (const match of command.matchAll(/\$\(\(([\s\S]*?)\)\)/g)) { - const body = match[1].trim(); - if (identifier.test(body) && body !== name) return true; + if (mask.slice(match.index, match.index + 3) === "$((") visit(match[1]); } - return false; } function appendArgs(command: string, args: string[]): string {