From 75ee497011dca076de0923ded9da8e23e08bfb84 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Mon, 14 Sep 2026 23:16:37 +0200 Subject: [PATCH] fix(cli): stage a rewritten shared lockfile on git-sync deploy push (#11126) * fix(cli): stage a rewritten shared lockfile on git-sync deploy push Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GGciSSE5EFMiDf1dFWQq5M * test(cli): pin that a swept shared lockfile is committed as a deletion Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GGciSSE5EFMiDf1dFWQq5M * chore: bump the git sync hub script to 28969 Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01GGciSSE5EFMiDf1dFWQq5M --------- Co-authored-by: Claude Fable 5.1 --- backend/windmill-common/src/workspaces.rs | 2 +- cli/src/utils/git.ts | 6 ++ cli/test/gitsync_deploy_push_unit.test.ts | 98 +++++++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 cli/test/gitsync_deploy_push_unit.test.ts diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index ad040c5905..3b00a87920 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -183,7 +183,7 @@ pub enum ObjectType { DatatableMigration, } -pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/28958/sync-script-to-git-repo-windmill"; +pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/28969/sync-script-to-git-repo-windmill"; /// Hub script that applies a repository's state back into a workspace /// (the repo → Windmill / "pull" direction). Same script the UI runs from diff --git a/cli/src/utils/git.ts b/cli/src/utils/git.ts index 5ca06f427a..354e15c36e 100644 --- a/cli/src/utils/git.ts +++ b/cli/src/utils/git.ts @@ -1,6 +1,7 @@ import * as log from "../core/log.ts"; import { execSync, spawnSync } from "node:child_process"; import { WM_FORK_PREFIX } from "../core/constants.ts"; +import { SHARED_LOCK_DIR } from "./script_common.ts"; // Fork *workspace id* prefix ("wm-fork-"). WM_FORK_PREFIX is the *branch* // prefix ("wm-fork") used inside the wm-fork// branch name. @@ -584,6 +585,11 @@ export function gitSyncDeployPush(params: { git(["add", "wmill-lock.yaml", `${parent_path}**`], { allowFail: true }); } } + // A shared lockfile (`dedupeLockfiles`) lives under `locks/`, outside every + // item's path glob, and the pull rewrites it when a deployed script's lock + // changed. `-A` also stages the deletion of a swept one; the add fails only + // when nothing under `locks/` exists or is tracked. + git(["add", "-A", "--", SHARED_LOCK_DIR], { allowFail: true }); // `git diff --cached --quiet` exits 1 iff there is something staged. const staged = git(["diff", "--cached", "--quiet"], { allowFail: true }); diff --git a/cli/test/gitsync_deploy_push_unit.test.ts b/cli/test/gitsync_deploy_push_unit.test.ts new file mode 100644 index 0000000000..f87e86bcc3 --- /dev/null +++ b/cli/test/gitsync_deploy_push_unit.test.ts @@ -0,0 +1,98 @@ +import { expect, test } from "bun:test"; +import { execFileSync } from "node:child_process"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { gitSyncDeployPush } from "../src/utils/git.ts"; + +function git(cwd: string, ...args: string[]): string { + return execFileSync("git", args, { cwd, encoding: "utf8" }).trim(); +} + +// A seeded clone of a bare remote, with `files` committed on main. +async function seededClone( + files: Record, +): Promise<{ bare: string; work: string }> { + const bare = await mkdtemp(join(tmpdir(), "wmill_deploy_push_bare_")); + execFileSync("git", ["init", "--quiet", "--bare", "--initial-branch=main", bare]); + const work = await mkdtemp(join(tmpdir(), "wmill_deploy_push_work_")); + git(work, "init", "--quiet", "--initial-branch=main"); + git(work, "config", "user.email", "seed@windmill.dev"); + git(work, "config", "user.name", "seed"); + for (const [path, content] of Object.entries(files)) { + await mkdir(join(work, path, ".."), { recursive: true }); + await writeFile(join(work, path), content); + } + git(work, "add", "-A"); + git(work, "commit", "--quiet", "-m", "seed"); + git(work, "remote", "add", "origin", `file://${bare}`); + git(work, "push", "--quiet", "-u", "origin", "main"); + return { bare, work }; +} + +function deployPushIn(work: string, path: string) { + const cwd = process.cwd(); + process.chdir(work); + try { + return gitSyncDeployPush({ + items: [{ path_type: "script", path, commit_msg: `deploy ${path}` }], + authorName: "windmill", + authorEmail: "windmill@windmill.dev", + }); + } finally { + process.chdir(cwd); + } +} + +test("a rewritten shared lockfile is committed with the deployed item", async () => { + const { bare, work } = await seededClone({ + "wmill-lock.yaml": "locks: {}\n", + "f/dd/a.script.yaml": "lock: '!inline locks/requirements.in.lock'\n", + "locks/requirements.in.lock": "requests==2.31.0\n", + }); + // What the deploy callback's pull leaves behind after `f/dd/a` was relocked: + // the item's own files are unchanged, only the shared file moved. + await writeFile(join(work, "locks/requirements.in.lock"), "requests==2.32.3\n"); + + expect(deployPushIn(work, "f/dd/a").pushed).toBe(true); + expect(git(work, "show", "--name-only", "--format=", "HEAD")).toBe( + "locks/requirements.in.lock", + ); + expect(git(bare, "cat-file", "-p", "main:locks/requirements.in.lock")).toBe( + "requests==2.32.3", + ); + + await rm(bare, { recursive: true, force: true }); + await rm(work, { recursive: true, force: true }); +}); + +test("a swept shared lockfile is committed as a deletion", async () => { + const { bare, work } = await seededClone({ + "wmill-lock.yaml": "locks: {}\n", + "f/dd/a.script.yaml": "lock: '!inline f/dd/a.script.lock'\n", + "f/dd/a.script.lock": "requests==2.31.0\n", + "locks/requirements.in.lock": "requests==2.31.0\n", + }); + // The pull removed the last shared lockfile, and `locks/` with it. + await rm(join(work, "locks"), { recursive: true, force: true }); + + expect(deployPushIn(work, "f/dd/a").pushed).toBe(true); + expect(git(bare, "ls-tree", "--name-only", "main", "locks/")).toBe(""); + + await rm(bare, { recursive: true, force: true }); + await rm(work, { recursive: true, force: true }); +}); + +test("a repository without shared lockfiles is left alone", async () => { + const { bare, work } = await seededClone({ + "wmill-lock.yaml": "locks: {}\n", + "f/dd/a.script.yaml": "lock: '!inline f/dd/a.script.lock'\n", + "f/dd/a.script.lock": "requests==2.31.0\n", + }); + + expect(deployPushIn(work, "f/dd/a").pushed).toBe(false); + expect(git(bare, "rev-parse", "main")).toBe(git(work, "rev-parse", "HEAD")); + + await rm(bare, { recursive: true, force: true }); + await rm(work, { recursive: true, force: true }); +});