From 32f49a493742f05326b341f86c9fb071ca4bde01 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 25 Aug 2026 03:39:25 -0700 Subject: [PATCH] fix(workers): preserve executable workspace files on Windows nodes (#129226) * fix(workers): preserve executable modes on Windows nodes * fix(ci): route worker workspace transfers to Windows --- scripts/ci-changed-scope.mjs | 8 +- .../node-worker-tunnel.test.ts | 22 ++- .../worker-environments/node-worker-tunnel.ts | 16 +- .../workspace-sync-manifest.test.ts | 54 ++++++ .../workspace-sync-scripts.ts | 33 ++++ .../node-worker-transfer-client.test.ts | 162 ++++++++++++++++++ src/node-host/node-worker-transfer-client.ts | 38 +++- src/scripts/ci-changed-scope.windows.test.ts | 16 ++ 8 files changed, 335 insertions(+), 14 deletions(-) diff --git a/scripts/ci-changed-scope.mjs b/scripts/ci-changed-scope.mjs index d9cd4431ac17..25cb3c524513 100644 --- a/scripts/ci-changed-scope.mjs +++ b/scripts/ci-changed-scope.mjs @@ -89,6 +89,8 @@ const WINDOWS_WORKSPACE_QUIESCENCE_SCOPE_RE = /^src\/gateway\/worker-environments\/workspace-quiescence(?:-scripts|(?:\.windows)?\.test)?\.ts$/; const WINDOWS_WORKER_BUNDLE_SCOPE_RE = /^src\/(?:shared\/worker-bundle-(?:archive|hash)(?:\.test)?|gateway\/worker-environments\/bundle(?:-staging)?(?:\.test)?|node-host\/node-worker-bundle-installer(?:\.test)?)\.ts$/; +const WINDOWS_WORKER_WORKSPACE_SCOPE_RE = + /^src\/(?:node-host\/node-worker-transfer-client(?:\.test)?|gateway\/worker-environments\/(?:node-worker-tunnel(?:\.test)?|workspace-sync-(?:scripts|manifest\.test)))\.ts$/; const CONTROL_UI_I18N_SCOPE_RE = /^(ui\/src\/i18n\/|ui\/config\/control-ui-locales\.ts$|scripts\/(?:control-ui-i18n(?:-verify)?\.ts|lib\/control-ui-i18n-(?:catalog|config|raw-copy|sync-plan)\.ts)$|\.github\/workflows\/control-ui-locale-refresh\.yml$)/; const CONTROL_UI_RAW_COPY_SOURCE_RE = /^ui\/src\/(?:app|components|lib|pages)\/.*\.tsx?$/; @@ -207,7 +209,8 @@ export function detectChangedScope(changedPaths) { WINDOWS_NODE_HOST_EXECUTABLE_SCOPE_RE.test(path) || WINDOWS_MEMORY_EXTRA_FILE_SCOPE_RE.test(path) || WINDOWS_WORKSPACE_QUIESCENCE_SCOPE_RE.test(path) || - WINDOWS_WORKER_BUNDLE_SCOPE_RE.test(path)) && + WINDOWS_WORKER_BUNDLE_SCOPE_RE.test(path) || + WINDOWS_WORKER_WORKSPACE_SCOPE_RE.test(path)) && (!facts.isTestOnly || WINDOWS_TEST_SCOPE_RE.test(path) || WINDOWS_FILE_URL_SCOPE_RE.test(path) || @@ -221,7 +224,8 @@ export function detectChangedScope(changedPaths) { WINDOWS_NODE_HOST_EXECUTABLE_SCOPE_RE.test(path) || WINDOWS_MEMORY_EXTRA_FILE_SCOPE_RE.test(path) || WINDOWS_WORKSPACE_QUIESCENCE_SCOPE_RE.test(path) || - WINDOWS_WORKER_BUNDLE_SCOPE_RE.test(path)) + WINDOWS_WORKER_BUNDLE_SCOPE_RE.test(path) || + WINDOWS_WORKER_WORKSPACE_SCOPE_RE.test(path)) ) { runWindows = true; } diff --git a/src/gateway/worker-environments/node-worker-tunnel.test.ts b/src/gateway/worker-environments/node-worker-tunnel.test.ts index ddc9b11ca054..0e0f3095c1a9 100644 --- a/src/gateway/worker-environments/node-worker-tunnel.test.ts +++ b/src/gateway/worker-environments/node-worker-tunnel.test.ts @@ -349,7 +349,7 @@ describe("node worker tunnel manager", () => { const manifestRef = `sha256:${createHash("sha256").update(rawManifest).digest("hex")}`; const outputs = [`quiesced ${"c".repeat(32)}`, manifestRef, ""]; const nodeTransport = transport(); - nodeTransport.invoke = vi.fn(async () => ({ + const invoke = vi.fn(async () => ({ ok: true, payloadJSON: JSON.stringify({ workspaceDir: "/node/workspace", @@ -361,6 +361,7 @@ describe("node worker tunnel manager", () => { termination: "exit", }), })); + nodeTransport.invoke = invoke; const prepareSync = vi.fn(async () => { await validation.promise; if (outcome === "failure") { @@ -401,6 +402,15 @@ describe("node worker tunnel manager", () => { const expectedStatus = outcome === "success" ? "fulfilled" : "rejected"; expect(results.map((result) => result.status)).toEqual([expectedStatus, expectedStatus]); expect(manager.status("environment-1")).toBe(outcome === "success" ? "connected" : "stopped"); + if (outcome === "success") { + expect(invoke).toHaveBeenCalledWith( + expect.objectContaining({ + params: expect.objectContaining({ + argv: expect.arrayContaining(["all", manifestRef.slice("sha256:".length)]), + }), + }), + ); + } }, ); @@ -853,13 +863,14 @@ describe("node worker tunnel manager", () => { }); const transferDirections: string[] = []; const nodeTransport = transport(); - nodeTransport.invoke = vi.fn(async ({ params }) => { + const invoke = vi.fn(async ({ params }) => { const input = params as { transfer?: { direction?: string } }; if (input.transfer?.direction) { transferDirections.push(input.transfer.direction); } return { ok: true, payloadJSON: spawnResult(`${baseManifestRef}\n`) }; }); + nodeTransport.invoke = invoke; const publishSnapshot = vi.fn(() => "accepted-download-token"); const transfer = { prepareSync: vi.fn(async () => ({ @@ -902,5 +913,12 @@ describe("node worker tunnel manager", () => { expect(reconciliation.manifestRef).toBe(baseManifestRef); expect(transferDirections).toEqual(["download", "upload"]); expect(publishSnapshot).not.toHaveBeenCalled(); + expect(invoke).toHaveBeenCalledWith( + expect.objectContaining({ + params: expect.objectContaining({ + argv: expect.arrayContaining(["all", baseManifestRef.slice("sha256:".length)]), + }), + }), + ); }); }); diff --git a/src/gateway/worker-environments/node-worker-tunnel.ts b/src/gateway/worker-environments/node-worker-tunnel.ts index b881889aac2f..97283d1aede3 100644 --- a/src/gateway/worker-environments/node-worker-tunnel.ts +++ b/src/gateway/worker-environments/node-worker-tunnel.ts @@ -326,23 +326,21 @@ export function createNodeWorkerTunnelManager(options: NodeWorkerTunnelManagerOp sharedHost: true, runWorkspaceCommand: async (command) => await exec(command), }); - const captureManifest = async (remoteWorkspaceDir: string, baseCommit: string | null) => { + const captureManifest = async (dir: string, base: string | null, reference: string) => { const captured = await exec({ argv: [ "node", "-e", REMOTE_WORKSPACE_MANIFEST_JS, - remoteWorkspaceDir, - ...(baseCommit ? [baseCommit, "eligible"] : []), + dir, + ...(base ? [base, "eligible"] : ["", "all"]), + reference.slice("sha256:".length), ], transportRetry: "idempotent", }); const manifestRef = captured.stdout.trim(); - if ( - captured.termination !== "exit" || - captured.code !== 0 || - !/^sha256:[a-f0-9]{64}$/u.test(manifestRef) - ) { + const validRef = /^sha256:[a-f0-9]{64}$/u.test(manifestRef); + if (captured.termination !== "exit" || captured.code !== 0 || !validRef) { throw new Error("Node workspace manifest capture failed"); } return manifestRef; @@ -371,6 +369,7 @@ export function createNodeWorkerTunnelManager(options: NodeWorkerTunnelManagerOp const remoteManifestRef = await captureManifest( restoredWorkspace.remoteWorkspaceDir, prepared.snapshot.manifest.baseCommit, + restoredWorkspace.manifestRef, ); if (remoteManifestRef !== restoredWorkspace.manifestRef) { throw new Error("Node workspace changed before tunnel recovery"); @@ -420,6 +419,7 @@ export function createNodeWorkerTunnelManager(options: NodeWorkerTunnelManagerOp const observed = await captureManifest( request.remoteWorkspaceDir, uploaded.base.baseCommit, + expectedRemoteRef, ); if (observed !== expectedRemoteRef) { throw new Error("Cloud workspace changed during final reconciliation"); diff --git a/src/gateway/worker-environments/workspace-sync-manifest.test.ts b/src/gateway/worker-environments/workspace-sync-manifest.test.ts index 69cfdbdfc90a..59dbc1597307 100644 --- a/src/gateway/worker-environments/workspace-sync-manifest.test.ts +++ b/src/gateway/worker-environments/workspace-sync-manifest.test.ts @@ -34,6 +34,60 @@ function spawnTransaction(argv: string[], env: NodeJS.ProcessEnv) { } describe("remote workspace manifest script", () => { + it("preserves authenticated executable modes when Windows cannot represent them", async () => { + const root = tempDirs.make("openclaw-windows-manifest-modes-"); + const home = path.join(root, "home"); + const workspace = path.join(root, "workspace"); + await Promise.all([fs.mkdir(home), fs.mkdir(workspace)]); + const file = path.join(workspace, "script.sh"); + const original = Buffer.from("#!/bin/sh\necho before\n"); + await fs.writeFile(file, original, { mode: 0o644 }); + const rawManifest = serializeWorkerWorkspaceManifest({ + version: 1, + baseCommit: null, + entries: [ + { + path: "script.sh", + type: "file", + mode: 0o755, + size: original.byteLength, + sha256: createHash("sha256").update(original).digest("hex"), + }, + ], + }); + const digest = createHash("sha256").update(rawManifest).digest("hex"); + const windowsScript = `Object.defineProperty(process, "platform", { value: "win32" });\n${REMOTE_WORKSPACE_MANIFEST_JS}`; + const env = { ...process.env, HOME: home }; + + const published = await runCommandWithTimeout( + [process.execPath, "-e", windowsScript, workspace, "", "publish", digest], + { timeoutMs: 10_000, baseEnv: env, input: rawManifest }, + ); + expect(published).toMatchObject({ code: 0, stdout: `sha256:${digest}\n` }); + + const capture = async () => + await runCommandWithTimeout( + [process.execPath, "-e", windowsScript, workspace, "", "all", digest], + { timeoutMs: 10_000, baseEnv: env }, + ); + expect(await capture()).toMatchObject({ code: 0, stdout: `sha256:${digest}\n` }); + + await fs.writeFile(file, "#!/bin/sh\necho changed\n"); + await fs.writeFile(path.join(workspace, "new.txt"), "new\n", { mode: 0o644 }); + const changed = await capture(); + expect(changed.code, changed.stderr).toBe(0); + const changedDigest = changed.stdout.trim().slice("sha256:".length); + const changedRaw = await fs.readFile( + path.join(home, ".openclaw-worker", "manifests", `${changedDigest}.json`), + "utf8", + ); + const manifest = parseWorkerWorkspaceManifest(changedRaw, changed.stdout.trim()); + expect(manifest.entries).toEqual([ + expect.objectContaining({ path: "new.txt", mode: 0o644 }), + expect.objectContaining({ path: "script.sh", mode: 0o755 }), + ]); + }); + it("atomically applies and rolls back accepted workspace paths", async () => { const root = tempDirs.make("openclaw-accepted-paths-test-"); const home = path.join(root, "home"); diff --git a/src/gateway/worker-environments/workspace-sync-scripts.ts b/src/gateway/worker-environments/workspace-sync-scripts.ts index 30760dc82b2b..d33d7b940018 100644 --- a/src/gateway/worker-environments/workspace-sync-scripts.ts +++ b/src/gateway/worker-environments/workspace-sync-scripts.ts @@ -416,6 +416,38 @@ async function readPublishedManifest() { } return Buffer.concat(chunks).toString("utf8"); } +function preserveWindowsFileModes(entries, manifestRoot) { + if (process.platform !== "win32" || priorManifestDigests.length === 0) return; + const modes = new Map(); + for (const digest of priorManifestDigests) { + if (!/^[a-f0-9]{64}$/.test(digest)) fail("invalid prior workspace manifest digest"); + const raw = readManifestFile(path.join(manifestRoot, digest + ".json")); + if (crypto.createHash("sha256").update(raw).digest("hex") !== digest) { + fail("prior workspace manifest digest mismatch"); + } + const prior = JSON.parse(raw); + if ( + !prior || + prior.version !== 1 || + !Array.isArray(prior.entries) || + prior.entries.length > MAX_WORKSPACE_INVENTORY_ENTRIES + ) { + fail("invalid prior workspace manifest"); + } + for (const entry of prior.entries) { + if (entry.type === "file" && !modes.has(entry.path)) { + if (entry.mode !== 0o644 && entry.mode !== 0o755) { + fail("invalid prior workspace file mode"); + } + modes.set(entry.path, entry.mode); + } + } + } + // Windows cannot persist POSIX execute bits; the authenticated prior manifest owns them. + for (const entry of entries) { + if (entry.type === "file" && modes.has(entry.path)) entry.mode = modes.get(entry.path); + } +} async function main() { const workerRoot = path.join(process.env.HOME, ".openclaw-worker"); const manifestRoot = path.join(workerRoot, "manifests"); @@ -444,6 +476,7 @@ async function main() { const entries = [...entriesByPath.values()]; assertSerializedManifestBudget(requestedBaseCommit, entries); await hashFiles(entries); + preserveWindowsFileModes(entries, manifestRoot); const baseCommit = requestedBaseCommit; const manifest = serializeManifest(baseCommit, entries); const digest = publishManifest(manifestRoot, manifest); diff --git a/src/node-host/node-worker-transfer-client.test.ts b/src/node-host/node-worker-transfer-client.test.ts index c94f46cd5e31..6e2273cad00f 100644 --- a/src/node-host/node-worker-transfer-client.test.ts +++ b/src/node-host/node-worker-transfer-client.test.ts @@ -86,6 +86,168 @@ async function git(root: string, args: string[]): Promise { } describe("node worker transfer client", () => { + it.runIf(process.platform === "win32")( + "preserves foreign executable modes through Windows workspace downloads and uploads", + async () => { + const root = tempDirs.make("node-worker-transfer-windows-executable-"); + const workspaceDir = path.join(root, "workspace"); + const original = Buffer.from("#!/bin/sh\necho before\n"); + const sha256 = createHash("sha256").update(original).digest("hex"); + const rawManifest = serializeWorkerWorkspaceManifest({ + version: 1, + baseCommit: null, + entries: [ + { path: "script.sh", type: "file", mode: 0o755, size: original.byteLength, sha256 }, + ], + }); + const manifestRef = `sha256:${createHash("sha256").update(rawManifest).digest("hex")}`; + let uploadedRaw: string | undefined; + const server = createHttpServer((req, res) => { + void (async () => { + if (req.url?.endsWith("/manifest")) { + res.writeHead(200).end(rawManifest); + return; + } + if (req.url?.endsWith(`/blobs/${sha256}`)) { + res.writeHead(200).end(original); + return; + } + if (req.method === "POST" && req.url?.includes("/reconciliations/")) { + const chunks: Buffer[] = []; + for await (const chunk of req) { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + } + const body = Buffer.concat(chunks); + const baseBytes = body.readUInt32BE(0); + const currentOffset = 4 + baseBytes; + const currentBytes = body.readUInt32BE(currentOffset); + uploadedRaw = body + .subarray(currentOffset + 4, currentOffset + 4 + currentBytes) + .toString("utf8"); + const currentRef = `sha256:${createHash("sha256").update(uploadedRaw).digest("hex")}`; + res.writeHead(200).end(JSON.stringify({ manifestRef: currentRef })); + return; + } + res.writeHead(404).end(); + })().catch((error: unknown) => { + res.destroy(error instanceof Error ? error : new Error(String(error))); + }); + }); + const gatewayUrl = await listen(server); + try { + await expect( + runNodeWorkerWorkspaceTransfer({ + gatewayUrl, + environmentId: "environment-windows-executable", + workspaceDir, + manifestHome: root, + transfer: { direction: "download", token: "download-token", manifestRef }, + }), + ).resolves.toBe(manifestRef); + await expect( + fs.readFile( + path.join( + root, + ".openclaw-worker", + "manifests", + `${manifestRef.slice("sha256:".length)}.json`, + ), + "utf8", + ), + ).resolves.toBe(rawManifest); + + await fs.writeFile(path.join(workspaceDir, "script.sh"), "#!/bin/sh\necho changed\n"); + await fs.writeFile(path.join(workspaceDir, "new.txt"), "new\n"); + const currentRef = await runNodeWorkerWorkspaceTransfer({ + gatewayUrl, + environmentId: "environment-windows-executable", + workspaceDir, + manifestHome: root, + transfer: { direction: "upload", token: "upload-token", baseManifestRef: manifestRef }, + }); + expect(currentRef).toMatch(/^sha256:[a-f0-9]{64}$/u); + expect(JSON.parse(uploadedRaw!)).toMatchObject({ + entries: [ + expect.objectContaining({ path: "new.txt", mode: 0o644 }), + expect.objectContaining({ path: "script.sh", mode: 0o755 }), + ], + }); + } finally { + server.closeAllConnections(); + await new Promise((resolve) => { + server.close(() => resolve()); + }); + } + }, + ); + + it.runIf(process.platform === "win32")( + "reuses foreign executable Git-base files without requesting an unavailable blob", + async () => { + const root = tempDirs.make("node-worker-transfer-windows-git-executable-"); + const source = path.join(root, "source"); + const workspaceDir = path.join(root, "workspace"); + const content = Buffer.from("#!/bin/sh\necho tracked\n"); + await fs.mkdir(source); + await git(source, ["init", "--quiet", "--object-format=sha1"]); + await git(source, ["config", "core.filemode", "false"]); + await fs.writeFile(path.join(source, "script.sh"), content); + const object = await git(source, ["hash-object", "-w", "script.sh"]); + await git(source, ["update-index", "--add", "--cacheinfo", `100755,${object},script.sh`]); + await git(source, ["commit", "--quiet", "-m", "POSIX executable base"]); + const commit = await git(source, ["rev-parse", "HEAD"]); + const rawManifest = serializeWorkerWorkspaceManifest({ + version: 1, + baseCommit: commit, + entries: [ + { + path: "script.sh", + type: "file", + mode: 0o755, + size: content.byteLength, + sha256: createHash("sha256").update(content).digest("hex"), + }, + ], + }); + const manifestRef = `sha256:${createHash("sha256").update(rawManifest).digest("hex")}`; + const packed = await runCommandBuffered( + ["git", "-C", source, "pack-objects", "--stdout", "--revs"], + { input: `${commit}\n`, maxOutputBytes: 4 * 1024 * 1024 }, + ); + expect(packed.code).toBe(0); + let requestedBlobs = 0; + const server = createHttpServer((req, res) => { + if (req.url?.endsWith("/manifest")) { + res.writeHead(200).end(rawManifest); + } else if (req.url?.endsWith("/pack")) { + res.writeHead(200).end(packed.stdout); + } else { + requestedBlobs += 1; + res.writeHead(404).end(); + } + }); + const gatewayUrl = await listen(server); + try { + await expect( + runNodeWorkerWorkspaceTransfer({ + gatewayUrl, + environmentId: "environment-windows-git-executable", + workspaceDir, + manifestHome: root, + transfer: { direction: "download", token: "download-token", manifestRef }, + }), + ).resolves.toBe(manifestRef); + expect(requestedBlobs).toBe(0); + await expect(fs.readFile(path.join(workspaceDir, "script.sh"))).resolves.toEqual(content); + } finally { + server.closeAllConnections(); + await new Promise((resolve) => { + server.close(() => resolve()); + }); + } + }, + ); + it("keeps the prior workspace intact when a pack transfer is cut short", async () => { const root = tempDirs.make("node-worker-transfer-cut-"); const workspaceDir = path.join(root, "workspace"); diff --git a/src/node-host/node-worker-transfer-client.ts b/src/node-host/node-worker-transfer-client.ts index d36d70b70d30..eec1a14fa8a6 100644 --- a/src/node-host/node-worker-transfer-client.ts +++ b/src/node-host/node-worker-transfer-client.ts @@ -173,6 +173,7 @@ async function captureManifest(params: { workspaceDir: string; manifestHome: string; baseCommit: string | null; + referenceManifestRef: string; signal?: AbortSignal; }): Promise { return ( @@ -185,7 +186,14 @@ async function captureManifest(params: { REMOTE_WORKSPACE_MANIFEST_JS, params.workspaceDir, params.baseCommit ?? "", - ...(params.baseCommit ? ["eligible"] : []), + ...(process.platform === "win32" + ? [ + params.baseCommit ? "eligible" : "all", + params.referenceManifestRef.slice("sha256:".length), + ] + : params.baseCommit + ? ["eligible"] + : []), ], signal: params.signal, }) @@ -417,6 +425,26 @@ async function downloadWorkspace(params: { }); const staging = stagingWorkspace.dir; try { + if (process.platform === "win32") { + const published = await runWorkspaceCommand({ + workspaceDir: staging, + homeDir: params.manifestHome, + argv: [ + "node", + "-e", + REMOTE_WORKSPACE_MANIFEST_JS, + staging, + manifest.baseCommit ?? "", + "publish", + params.transfer.manifestRef.slice("sha256:".length), + ], + input: raw, + signal: params.signal, + }); + if (published.trim() !== params.transfer.manifestRef) { + throw new Error("workspace transfer manifest publication acknowledgement is invalid"); + } + } if (manifest.baseCommit) { const packPath = path.join(staging, ".openclaw-base.pack"); const packStartedAt = performance.now(); @@ -451,7 +479,11 @@ async function downloadWorkspace(params: { } for (const entry of manifest.entries) { const destination = workspacePath(staging, entry.path); - if (manifest.baseCommit && (await absoluteEntryMatches(destination, entry))) { + const materializedEntry = + process.platform === "win32" && entry.type === "file" && entry.mode === 0o755 + ? { ...entry, mode: 0o644 } + : entry; + if (manifest.baseCommit && (await absoluteEntryMatches(destination, materializedEntry))) { continue; } await fsp.mkdir(path.dirname(destination), { recursive: true, mode: 0o700 }); @@ -481,6 +513,7 @@ async function downloadWorkspace(params: { workspaceDir: staging, manifestHome: params.manifestHome, baseCommit: manifest.baseCommit, + referenceManifestRef: params.transfer.manifestRef, signal: params.signal, }); if (observed !== params.transfer.manifestRef) { @@ -540,6 +573,7 @@ async function uploadWorkspace(params: { workspaceDir: params.workspaceDir, manifestHome: params.manifestHome, baseCommit: base.baseCommit, + referenceManifestRef: params.transfer.baseManifestRef, signal: params.signal, }); const currentRaw = await fsp.readFile( diff --git a/src/scripts/ci-changed-scope.windows.test.ts b/src/scripts/ci-changed-scope.windows.test.ts index c8de737b36e4..0fde5fac94d6 100644 --- a/src/scripts/ci-changed-scope.windows.test.ts +++ b/src/scripts/ci-changed-scope.windows.test.ts @@ -22,6 +22,22 @@ describe("detectChangedScope Windows routing", () => { } }); + it("routes paired-worker workspace transfer owners and native regression coverage to Windows", () => { + for (const workspacePath of [ + "src/node-host/node-worker-transfer-client.ts", + "src/node-host/node-worker-transfer-client.test.ts", + "src/gateway/worker-environments/node-worker-tunnel.ts", + "src/gateway/worker-environments/node-worker-tunnel.test.ts", + "src/gateway/worker-environments/workspace-sync-scripts.ts", + "src/gateway/worker-environments/workspace-sync-manifest.test.ts", + ]) { + expect(detectChangedScope([workspacePath]), workspacePath).toMatchObject({ + runNode: true, + runWindows: true, + }); + } + }); + it("routes SQLite transcript archive changes to Windows", () => { for (const archivePath of [ "src/config/sessions/session-accessor.sqlite-archive.ts",