mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-20 08:01:35 +00:00
refactor(cli): wmill sync git-deploy stops committing; caller owns commit+push (#9284)
Single contract for the deployment-callback path: the CLI does branch
checkout + pull, the caller (hub script in production, test in test)
does git add + commit + push. This restores the WIN-1974 invariant —
GPG setup and `git commit` run back-to-back in the same process, so
the agent's pre-warmed passphrase cache is still warm at sign time —
without needing a `--skip-commit` flag for the hub case and a default
"also-commit" for everything else. Same behavior in every call site.
Changes:
- sync.ts: drop the gitSyncDeployPush call from pull()'s deploy path
(both the onlyCreateBranch fast-return and the post-pull commit).
`gitSyncDeployPush` stays exported for any caller that wants the
same commit/push semantics — just not invoked by the CLI subcommand.
- gitsync_promotion.test.ts: e2e test now does its own git add +
commit + push after `wmill sync git-deploy`, mirroring what the
hub script does in production. Same regression coverage
(wm_deploy branch created in Case A, main untouched; main updated
in Case B, no new wm_deploy).
CLI typecheck unchanged (two pre-existing TarAsZip errors at lines
2578/3307, present before this PR). All 743 unit tests still pass.
The accompanying hub script (option-C — CLI for branch+pull, script
for commit+push) lives at /tmp/git-sync-diff/sync-script-to-git-repo-windmill.option-C.ts.
Once published, a follow-up bumps LATEST_GIT_SYNC_SCRIPT_PATH to its id.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
committed by
Diego Imbert
parent
f6d0ad16f5
commit
0257475ebb
@@ -2503,14 +2503,8 @@ export async function pull(
|
||||
}
|
||||
|
||||
if (opts.onlyCreateBranch) {
|
||||
gitSyncDeployPush({
|
||||
items: deployItems,
|
||||
authorName: process.env["WM_USERNAME"] || "windmill",
|
||||
authorEmail: process.env["WM_EMAIL"] || "windmill@windmill.dev",
|
||||
committerName: opts.gitCommitterName,
|
||||
committerEmail: opts.gitCommitterEmail,
|
||||
onlyCreateBranch: true,
|
||||
});
|
||||
// Branch is checked out locally; the caller pushes it. Symmetric with
|
||||
// the non-onlyCreateBranch path: CLI does branch + pull, never push.
|
||||
return;
|
||||
}
|
||||
}
|
||||
@@ -2982,19 +2976,11 @@ export async function pull(
|
||||
log.warn(`Failed to pull shared UI folder: ${e}`);
|
||||
}
|
||||
|
||||
// Git-sync deployment-callback mode: commit the pulled files and push the
|
||||
// current branch (the wm_deploy/fork branch checked out above, or the base
|
||||
// branch in workspace-wide mode).
|
||||
if (opts.gitDeployItems !== undefined && !opts.onlyCreateBranch) {
|
||||
const deployItems: GitSyncDeployItem[] = JSON.parse(opts.gitDeployItems);
|
||||
gitSyncDeployPush({
|
||||
items: deployItems,
|
||||
authorName: process.env["WM_USERNAME"] || "windmill",
|
||||
authorEmail: process.env["WM_EMAIL"] || "windmill@windmill.dev",
|
||||
committerName: opts.gitCommitterName,
|
||||
committerEmail: opts.gitCommitterEmail,
|
||||
});
|
||||
}
|
||||
// Git-sync deployment-callback mode stops here: branch checkout + pull have
|
||||
// happened, but commit + push are the caller's job. The hub script does
|
||||
// them in-process with `set_gpg_signing_secret` so the agent's pre-warmed
|
||||
// passphrase cache is still warm at sign time (WIN-1974). `gitSyncDeployPush`
|
||||
// stays exported for callers that want the same commit/push behavior.
|
||||
}
|
||||
|
||||
// Internal git-sync deployment-callback entrypoint. Invoked only by the
|
||||
|
||||
@@ -6,10 +6,11 @@
|
||||
* `use_individual_branch` is set — NOT straight to the cloned base branch
|
||||
* (e.g. a protected `main`, which fails with GH006).
|
||||
*
|
||||
* The CLI's `wmill sync pull --git-deploy-items ...` now owns that branch
|
||||
* checkout + commit + push (previously hub-script-only, hence untestable).
|
||||
* This drives it against a real local bare repo so the regression is caught
|
||||
* deterministically, with no network and no GitHub.
|
||||
* Contract: `wmill sync git-deploy` does branch checkout + pull only. Commit
|
||||
* + push are the caller's job — the hub script does them in the same process
|
||||
* as `set_gpg_signing_secret` so the GPG agent's passphrase cache is still
|
||||
* warm at sign time (WIN-1974). This test replicates the caller half (git
|
||||
* add + commit + push) inline so the full promotion regression stays caught.
|
||||
*/
|
||||
|
||||
import { expect, test } from "bun:test";
|
||||
@@ -121,6 +122,23 @@ test.skipIf(shouldSkipOnCI())(
|
||||
{ path_type: "script", path: "f/promo/foo", commit_msg: "deploy foo" },
|
||||
]);
|
||||
|
||||
// Caller-half: stage anything the CLI's pull dropped, commit on the
|
||||
// current branch (which the CLI just checked out), and push. Mirrors
|
||||
// what the hub script does in production after `wmill sync git-deploy`.
|
||||
const commitAndPush = (work: string) => {
|
||||
git(work, "config", "user.email", "test@windmill.dev");
|
||||
git(work, "config", "user.name", "test");
|
||||
git(work, "add", "-A");
|
||||
try {
|
||||
git(work, "diff", "--cached", "--quiet");
|
||||
// Exit 0 = nothing staged; nothing to commit. Still push the
|
||||
// (possibly new) branch ref so the assertions see it.
|
||||
} catch {
|
||||
git(work, "commit", "-m", "deploy foo");
|
||||
}
|
||||
git(work, "push", "--porcelain", "-u", "origin", "HEAD");
|
||||
};
|
||||
|
||||
// --- Case A: use_individual_branch=true -> wm_deploy branch, main untouched ---
|
||||
const workA = await mkdtemp(join(tmpdir(), "wmill_promo_a_"));
|
||||
git(workA, "clone", `file://${bareDir}`, ".");
|
||||
@@ -141,6 +159,7 @@ test.skipIf(shouldSkipOnCI())(
|
||||
workA,
|
||||
);
|
||||
expect(resA.code).toBe(0);
|
||||
commitAndPush(workA);
|
||||
|
||||
const branchesA = remoteBranches(bareDir);
|
||||
const expectedBranch = `refs/heads/wm_deploy/${ws}/script/f__promo__foo`;
|
||||
@@ -167,6 +186,8 @@ test.skipIf(shouldSkipOnCI())(
|
||||
workB,
|
||||
);
|
||||
expect(resB.code).toBe(0);
|
||||
commitAndPush(workB);
|
||||
|
||||
expect(remoteHead(bareDir, "main")).not.toBe(seedMain);
|
||||
expect(
|
||||
remoteBranches(bareDir).filter((b) => b.includes("wm_deploy")).length,
|
||||
|
||||
Reference in New Issue
Block a user