archgate · rhuanbarreto · May 3, 2026 · May 3, 2026
@@ -46,6 +46,7 @@ Skipping steps 2 or 3 is a workflow violation. The user should NEVER have to inv
 - **`Bun.Glob.scan({ dot: false })` silently drops dot-prefixed segments — even on explicit paths** — `dot: false` (the default) skips matches whose path contains a `.`-prefixed segment, including patterns that explicitly name the dir (e.g. `.github/workflows/release.yml`). Behavior also varies across platforms — Windows reliably drops the match while Linux can match the same pattern, so a rule appears to "work in CI" but no-ops locally. For code repos where `.github/`, `.husky/`, `.vscode/` are first-class source dirs, ALWAYS pass `dot: true` to `Bun.Glob.scan()`. Applied in `src/engine/runner.ts` (`ctx.glob`, `ctx.grepFiles`) and `src/engine/git-files.ts` (`resolveScopedFiles`). See archgate/cli#222.
 - **PowerShell 5.1 reads BOM-less `.ps1` files as ANSI — never use non-ASCII chars in `install.ps1`** — `install.ps1` has no UTF-8 BOM (first bytes are `# A...`). On Windows PowerShell 5.1, this means the file is decoded as the system codepage (Windows-1252), so multi-byte UTF-8 characters like em-dash (`—`, `\xE2\x80\x94`) get split into garbage bytes that break later string parsing — the parser reports cryptic errors like "string is missing the terminator" on lines that look fine. ALWAYS stick to ASCII (`-`, `--`, straight quotes) in comments and string literals in `install.ps1`. To verify after editing: `[System.Management.Automation.Language.Parser]::ParseFile((Resolve-Path .\install.ps1).Path, [ref]$null, [ref]$errs)`. Same caveat applies to any unsigned `.ps1` file the project distributes.
 - **SLSA reusable workflow MUST be tag-pinned, not SHA-pinned** — `slsa-framework/slsa-github-generator/.github/workflows/*` looks like it should follow CI-001 (SHA pin), but the SLSA generator's `generate-builder.sh` reads the workflow ref to download the prebuilt builder from a GitHub release and rejects non-tag refs (`Invalid ref: ... Expected ref of the form refs/tags/vX.Y.Z`, exit 2). Pinning by SHA broke the v0.31.0 release ([run 25107195589](https://github.com/archgate/cli/actions/runs/25107195589)). The CI-001 rule allowlists this path so it does NOT block a SHA repin — meaning a future agent could "fix" the tag pin and the rule would be silent until the next release fails. ALWAYS keep `@v2.x.y` for `release-binaries.yml:165` and read the inline comment + CI-001 "Carved-out exceptions" before changing. Upstream issue: [slsa-framework/slsa-github-generator#150](https://github.com/slsa-framework/slsa-github-generator/issues/150).
+- **Windows binary upgrade: never use detached child processes for `.old` cleanup** — On Windows, `replaceBinary()` renames the running exe to `.old` because the OS file-locks it. Cleaning up the `.old` via a detached `cmd /c ping -n 2 ... & del` process is unreliable (process may not spawn, `del` may fail silently, timing races). Instead, `cleanupStaleBinary()` runs at the next CLI startup as a fire-and-forget `unlink()` — the file is guaranteed unlocked by then. The cleanup is platform-agnostic (uses `getArtifactInfo()` to resolve the binary name), so it works on any supported platform even though only Windows currently creates `.old` files. The sync `unlinkSync` in `replaceBinary()` is kept as defense-in-depth for leftover `.old` files from previous upgrades. Do NOT reintroduce detached cleanup processes.
 - **`GITHUB_TOKEN`-authored pushes do NOT trigger downstream workflows — release.yml MUST use the GH App token** — When an Actions workflow pushes commits or opens PRs using `${{ github.token }}` / `secrets.GITHUB_TOKEN`, GitHub intentionally suppresses the resulting `push` / `pull_request` events to prevent recursion. Symptom on release PRs: the head SHA has no `pull_request`-event check runs, so `Validate Code` / `Lint, Test & Check` / `DCO Sign-off Check` are missing from the PR rollup and branch protection treats the PR as missing required checks. PR [#131](https://github.com/archgate/cli/pull/131) papered over this by manually `gh workflow run` + posting commit statuses, but `workflow_dispatch` runs land on `head_branch: release` with `pull_requests: []` — they are not associated with the PR ref, so `Lint, Test & Check` stayed orphaned and the bug recurred on PR [#251](https://github.com/archgate/cli/pull/251). Root-cause fix: in `release.yml` the `pull-request` job MUST generate a GitHub App installation token via `actions/create-github-app-token` (using `secrets.GH_APP_APP_ID` / `secrets.GH_APP_PRIVATE_KEY`) and pass it to BOTH `actions/checkout` and `simple-release-action`. App-token-authored pushes DO trigger `pull_request` events naturally. Apply the same pattern to any future workflow that pushes to a branch whose downstream CI must run.
 
 ## Validation Pipeline

@@ -14,6 +14,7 @@ import { registerReviewContextCommand } from "./commands/review-context";
 import { registerSessionContextCommand } from "./commands/session-context/index";
 import { registerTelemetryCommand } from "./commands/telemetry";
 import { registerUpgradeCommand } from "./commands/upgrade";
+import { cleanupStaleBinary } from "./helpers/binary-upgrade";
 import { beginCommand, exitWith, finalizeCommand } from "./helpers/exit";
 import { installGit } from "./helpers/git";
 import { type LogLevel, logError, setLogLevel } from "./helpers/log";
@@ -59,6 +60,10 @@ if (!isSupportedPlatform()) {
 
 createPathIfNotExists(paths.cacheFolder);
 
+// Fire-and-forget: remove leftover .old binary from a previous Windows upgrade.
+// No await — this must never block startup or affect the user's command.
+cleanupStaleBinary();
+
 async function main() {
   await installGit();
 

@@ -1,9 +1,11 @@
 import { createHash } from "node:crypto";
 import { chmodSync, mkdtempSync, renameSync, unlinkSync } from "node:fs";
+import { unlink } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 
 import { logDebug } from "./log";
+import { internalPath } from "./paths";
 import { isWindows } from "./platform";
 
 // ---------------------------------------------------------------------------
@@ -213,8 +215,9 @@ export async function downloadReleaseBinary(
  * Replace the running binary with the new one.
  *
  * Unix: directly renames the new binary over the old one (OS handles inode unlinking).
- * Windows: renames the running exe to .old (allowed by the OS), moves the new one
- * into place, and spawns a detached cleanup process for the old file.
+ * Windows: renames the running exe to .old (allowed by the OS), then moves the
+ * new one into place.  The .old file is cleaned up on the next CLI startup via
+ * {@link cleanupStaleBinary}.
  */
 export function replaceBinary(
   currentPath: string,
@@ -234,17 +237,46 @@ export function replaceBinary(
     renameSync(currentPath, oldPath);
     renameSync(newBinaryPath, currentPath);
 
-    // Spawn detached cleanup — waits for this process to exit, then deletes the old file
-    Bun.spawn(["cmd", "/c", `ping -n 2 127.0.0.1 >nul & del "${oldPath}"`], {
-      stdout: "ignore",
-      stderr: "ignore",
-    });
+    // The .old file is still locked by the running process so it cannot be
+    // deleted right now.  cleanupStaleBinary() will remove it on the next
+    // CLI invocation when the file is guaranteed to be unlocked.
   } else {
     renameSync(newBinaryPath, currentPath);
     chmodSync(currentPath, 0o755);
   }
 }
 
+// ---------------------------------------------------------------------------
+// Stale binary cleanup
+// ---------------------------------------------------------------------------
+
+/**
+ * Attempt to delete the leftover `.old` binary from a previous upgrade.
+ *
+ * On Windows, `replaceBinary()` renames the running exe to `.old` because the
+ * OS file-locks the running binary.  The `.old` file cannot be deleted during
+ * that same process — but it is guaranteed to be unlocked by the time the
+ * *next* CLI invocation starts.
+ *
+ * The cleanup is platform-agnostic: it resolves the correct binary name for
+ * the current platform and attempts to remove `<binary>.old` from the install
+ * directory.  On Unix the `.old` file is unlikely to exist (rename is atomic),
+ * but running the check everywhere keeps the logic unified.
+ *
+ * Call this once at CLI startup (fire-and-forget, no `await`).  Errors are
+ * silently swallowed — cleanup is best-effort and must never affect the
+ * user's command.
+ */
+export function cleanupStaleBinary(): Promise<void> {
+  const artifact = getArtifactInfo();
+  if (!artifact) return Promise.resolve();
+
+  const oldPath = internalPath("bin", `${artifact.binaryName}.old`);
+  return unlink(oldPath).catch(() => {
+    // File absent or still locked — nothing to do.
+  });
+}
+
 /**
  * Returns the manual install hint for the current platform.
  */

@@ -1,9 +1,16 @@
-import { describe, expect, test, mock, afterEach } from "bun:test";
-import { existsSync, mkdtempSync, statSync, writeFileSync } from "node:fs";
+import { describe, expect, test, mock, beforeEach, afterEach } from "bun:test";
+import {
+  existsSync,
+  mkdirSync,
+  mkdtempSync,
+  statSync,
+  writeFileSync,
+} from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 
 import {
+  cleanupStaleBinary,
   getArtifactInfo,
   getManualInstallHint,
   fetchLatestGitHubVersion,
@@ -142,4 +149,62 @@ describe("replaceBinary", () => {
     const mode = statSync(currentPath).mode & 0o777;
     expect(mode).toBe(0o755);
   });
+
+  test("creates .old file on Windows", () => {
+    if (process.platform !== "win32") return;
+
+    const tmpDir = mkdtempSync(join(tmpdir(), "archgate-replace-test-"));
+    const currentPath = join(tmpDir, "archgate.exe");
+    const newBinaryPath = join(tmpDir, "archgate.exe.new");
+
+    writeFileSync(currentPath, "old binary content");
+    writeFileSync(newBinaryPath, "new binary content");
+
+    replaceBinary(currentPath, newBinaryPath);
+
+    expect(existsSync(currentPath)).toBe(true);
+    expect(existsSync(currentPath + ".old")).toBe(true);
+    expect(existsSync(newBinaryPath)).toBe(false);
+  });
+});
+
+describe("cleanupStaleBinary", () => {
+  let savedHome: string | undefined;
+
+  beforeEach(() => {
+    savedHome = Bun.env.HOME;
+  });
+
+  afterEach(() => {
+    Bun.env.HOME = savedHome;
+  });
+
+  test("deletes the .old binary when present", async () => {
+    const artifact = getArtifactInfo();
+    if (!artifact) return; // unsupported platform
+
+    const tmpDir = mkdtempSync(join(tmpdir(), "archgate-cleanup-test-"));
+    Bun.env.HOME = tmpDir;
+
+    // Recreate the ~/.archgate/bin/ structure
+    const binDir = join(tmpDir, ".archgate", "bin");
+    mkdirSync(binDir, { recursive: true });
+    const oldPath = join(binDir, `${artifact.binaryName}.old`);
+    writeFileSync(oldPath, "stale binary");
+
+    await cleanupStaleBinary();
+
+    expect(existsSync(oldPath)).toBe(false);
+  });
+
+  test("resolves silently when no .old file exists", async () => {
+    const artifact = getArtifactInfo();
+    if (!artifact) return; // unsupported platform
+
+    const tmpDir = mkdtempSync(join(tmpdir(), "archgate-cleanup-test-"));
+    Bun.env.HOME = tmpDir;
+
+    // No .old file — should not throw
+    await expect(cleanupStaleBinary()).resolves.toBeUndefined();
+  });
 });