[xsvm]

Summary

docx upgrade refused on Windows because a running .exe can't be overwritten,
so Windows users had to download and swap the binary by hand. Windows does let
you rename a running .exe, and install.sh already supports Git Bash/MSYS
(it picks docx-windows-x64.exe and writes docx.exe). So:

• On Windows, install.sh gets PREFIX=<dir>/.docx-upgrade and installs there.
  It's the same download and the same checksum gate.
• Only after it exits 0, swapIn (src/cli/upgrade/swap-in.ts) renames the
  running binary to docx.exe.old and the staged one into its place. The
  staging folder is removed either way.
• The .old file can't delete itself while it runs, so the next upgrade
  removes it.
• Nothing moves until the new binary is downloaded and verified. A failed or
  interrupted upgrade therefore leaves docx.exe exactly as it was, so there's
  no restore path and no signal handling. I tried stepping aside first and
  restoring on failure, but Ctrl+C can't be trapped reliably on Windows: MSYS
  sh on piped stdio ignores it, so a Ctrl+C mid-download could leave no
  docx at all.
• The renamed-binary guard compares against docx.exe on Windows (the same
  name rule install.sh uses).
• If sh isn't on PATH (cmd/PowerShell without Git Bash), it refuses up front
  and points to Git Bash or a manual swap.
• There's still one install implementation, with no new download or checksum
  logic in TypeScript.

Docs: --help, README, SECURITY.md, troubleshooting.md, src/cli/CLAUDE.md.

Test plan

• bun run check
• tests/cli/upgrade.test.ts: a new swap-in case (staged → in place, old
  → .old, previous .old swept). It's plain file moves, so it runs on
  every OS.
• The compiled-binary test now also works on Windows (.exe suffix,
  platform path separator). It used to fail there.
• Real Windows 11 + Git Bash, binary built from this branch:
  - upgrade --to v0.0.1: install fails on the missing SHA256SUMS,
  docx.exe is untouched, and no staging folder is left behind
  - upgrade --to v0.27.0 with the download held open (the proxy points
  at a socket that never answers), then the terminal is killed:
  docx.exe is untouched and still runs, and the next upgrade clears the
  staging leftovers
  - I didn't run a full successful download. That path is install.sh's
  normal Git Bash install into the staging folder, followed by the two
  renames the unit test covers.

[kklimuk]

🤖 docx-cli triage

Thanks @xsvm, this is a careful PR. The "nothing moves until install.sh exits 0" ordering is the right design, and the reasoning in the src/cli/CLAUDE.md note is solid.

What the PR claims

docx upgrade would work on Windows (from Git Bash). It points the embedded install.sh at a staging folder <bindir>/.docx-upgrade. Once install.sh has downloaded and SHA-256-verified the new binary, it renames the running docx.exe to docx.exe.old and moves the staged binary into its place. A failed or interrupted install should leave docx.exe untouched.

Supply-chain check (installer invariant)

• skills/docx-cli/scripts/install.sh is unchanged. It is still the single implementation, still embedded at build time, and still piped to sh -s. Nothing new is fetched or executed.
• REQUIRE_CHECKSUM=1 and the tag validation are unchanged. The only env difference is PREFIX on Windows.
• No changes to package.json, lockfile, CI, or test runner config. No new network calls or obfuscated code.
• Linux/macOS behavior is unchanged: installPrefix === prefix, and the windows branch is skipped. The only new step there is the Bun.which("sh") guard, which always passes on POSIX.

What I verified (Linux, at bdbc6db)

bun run check                 → biome + knip + tsc clean
bun test tests/cli/upgrade.test.ts tests/cli/installer.test.ts → 21 pass, 0 fail
                                 (includes the compiled-binary standalone/renamed-binary smoke)
bun run test:unit             → 1910 pass, 0 fail

I had no real Windows machine, so I reviewed the Windows path by reading the code only. Renaming a running .exe is a well-established Windows self-update technique, so the core idea is sound. I did not run docx upgrade against the network.

Findings

1. No rollback if the swap fails halfway (can leave the user without docx). swapIn renames docx.exe to .old and then renames the staged file in. If the second rename fails, the user is left with only docx.exe.old. On Windows this is plausible: Defender or another AV often holds a newly written .exe open for a moment. The error escapes to the catch-all, and the staged binary sits in .docx-upgrade. Repro on Linux with a missing staged file:

   threw: ENOENT: no such file or directory, rename '…/.docx-upgrade/docx.exe' -> '…/docx.exe'
   docx.exe exists: false | docx.exe.old exists: true
   

   This contradicts the docs' "A failed or interrupted upgrade leaves docx.exe as it was". Proposed fix: put the old binary back, and report the failure through fail() instead of throwing:

   export async function swapIn(staged: string, binaryPath: string): Promise<void> {
   	const aside = `${binaryPath}.old`;
   	await rm(aside, { force: true });
   	await rename(binaryPath, aside);
   	try {
   		await rename(staged, binaryPath);
   	} catch (error) {
   		await rename(aside, binaryPath); // put the running binary back
   		throw error;
   	}
   }

   In index.ts, wrap the call in try { … } catch { return fail("UPGRADE_FAILED", could not swap the new binary into ${binaryPath} — it is unchanged, "…"); } finally { await rm(installPrefix, …) } so the staging folder is always removed and a weak agent sees {code,error,hint} instead of a stack trace. A unit test that passes a missing staged path and asserts that docx.exe still holds "current" would cover this.

2. Misleading PATH note on every Windows upgrade. install.sh ends by checking whether $PREFIX is on PATH. It never will be, because $PREFIX is now .docx-upgrade. So every successful Windows upgrade prints Note: …\.docx-upgrade is not on your PATH … export PATH="…\.docx-upgrade:$PATH", which tells the user (or agent) to put a folder on PATH that gets deleted a moment later. Changing install.sh is not the right fix (it is the shared installer). Possible fixes: filter that block out of the forwarded stdout on Windows, or add a documented install.sh knob such as QUIET_PATH_HINT=1. Adding a knob is a posture decision (see questions below).

3. SECURITY.md is now contradictory. Line 110 was updated, but line 21 (the "keeping it current" section) still says "On a macOS/Linux standalone-binary install, run docx upgrade … On Windows, download and verify the replacement binary from Releases." Line 59 also says only "replaces it", so it could mention the Windows staging step.

4. Minor:

   • The renamed-binary guard compares basename(binaryPath) !== "docx.exe" case-sensitively. Windows filenames are case-insensitive, so a Docx.exe would be refused. A .toLowerCase() on Windows would fix this.
   • The Not upgraded … the binary is left untouched line in HELP holds only after fix 1.
   • Windows PREFIX is a native path (C:\…\.docx-upgrade) handed to MSYS sh. Git Bash accepts it, but nothing tests it on a real Windows runner.

Questions for @kklimuk

• Do you want Windows self-upgrade at all? No issue asked for it. Until now the project position was "refuse on Windows; download and verify by hand". This PR makes docx upgrade a supported Windows path and adds Git Bash as a de facto dependency.
• Supply-chain posture: are you OK with the integrity guarantee on Windows coming from Git Bash's sha256sum/curl (same REQUIRE_CHECKSUM=1 as POSIX), and with a docx.exe.old left in the user's bin dir until the next upgrade?
• Coverage: the Windows branch has no test on a real Windows runner. Do you want a windows-latest CI job (at least the compiled --dry-run smoke) before merging, or is reading the code enough?
• For finding 2: suppress the PATH note in upgrade, or add an install.sh knob? (An install.sh change ships to all three install routes.)

Action needed: answer the questions above — whether Windows self-upgrade is wanted (and the Windows CI coverage expectation); if yes, ask the author for fixes 1–3 before merging.

---

Generated by Claude Code

[xsvm]

Thanks for the review. I've pushed fixes 1–3 and the case-sensitivity point. I also have a real Windows machine (Windows 11, Git Bash), so I ran docx upgrade end to end. That turned up a blocker that reading the code couldn't have caught. Both commits are mine, so the last-push rule doesn't get in the way.

e98d08b: findings 1–4

• Rollback: swapIn is now your version. If the staged binary can't be moved in, the running one is renamed back before the error is rethrown. index.ts wraps the call in try/catch/finally, returns fail("UPGRADE_FAILED", "could not swap the new binary into … — it is unchanged", …), and always removes the staging folder. There's a new unit test with a missing staged file: docx.exe still holds "current". It failed with ENOENT before the change.
• PATH note: I took the option that leaves install.sh alone. On Windows, withoutStagingPathHint (src/cli/upgrade/staging-path-hint.ts) drops the three PATH-note lines from the forwarded stdout. Everything else passes through, held only until its line ends, so the "→ Downloading" progress line still shows immediately. It's tested with chunk boundaries landing mid-line. If you'd rather add a documented install.sh knob instead, I can switch.
• SECURITY.md: line 21 no longer says Windows must upgrade by hand. The docx upgrade entry now describes the staging swap.
• Minor: on Windows the renamed-binary guard compares case-insensitively. src/cli/CLAUDE.md now covers the rollback and the PATH filter.

541ca5c: every Windows upgrade failed its checksum

The first real run downloaded v0.28.0 and then refused to install it:

Error: SHA-256 mismatch for docx-windows-x64.exe — refusing to install.
  expected: 48a652079f6838ff3dc3fc613481e8a545fd9f0acd4d620f2cc03a80872378d3
  actual:   \48a652079f6838ff3dc3fc613481e8a545fd9f0acd4d620f2cc03a80872378d3

This is your third minor point (a native PREFIX handed to MSYS sh). With PREFIX=C:\…\.docx-upgrade, the temp file's path contains backslashes. GNU sha256sum escapes such a name and prefixes its output line with \, and install.sh's sha256sum "$1" | awk '{print $1}' keeps that backslash. The failure was safe: the binary was unchanged and the staging folder was removed. But on Windows the upgrade could never succeed. upgrade now passes install.sh PREFIX with forward slashes (C:/…, which Git Bash accepts) and keeps the native path for its own rename and cleanup. install.sh is untouched.

With that fix, the second real run:

→ Downloading docx-windows-x64.exe from …/v0.28.0/docx-windows-x64.exe
✓ Verified SHA-256 (docx-windows-x64.exe)
✓ Installed: C:/…/up26/.docx-upgrade/docx.exe
docx 0.28.0
upgrade C:\…\up26\docx.exe          (exit 0)

Afterwards docx.exe hashes to 48a65207… (the release asset, matching SHA256SUMS), the previously running binary is docx.exe.old (the earlier .old was swept), .docx-upgrade is gone, and no PATH note was printed. In an earlier run I killed the process mid-download with timeout. docx.exe was byte-for-byte unchanged, and only the staging folder was left behind, since a killed process never reaches finally. The next upgrade reuses and removes it.

Separately, install.sh would be more robust if it hashed from stdin (sha256sum < "$1") so that no file name ever reaches the output. That change would ship to all three install routes, though, so it's your call. I didn't touch it.

biome, knip, tsc and upgrade/installer tests pass locally (23 tests). On this branch the compiled-binary test also passes on Windows. The questions about whether you want Windows self-upgrade at all, and whether to add a windows-latest CI job, are still yours to decide.

[kklimuk]

🤖 docx-cli triage

Thanks @xsvm. The follow-ups do what you said, and the checksum bug you found on Windows is a good catch. I re-reviewed at 541ca5c.

Prior findings, re-checked

• 1. Rollback: fixed. swapIn now renames the running binary back if the second rename fails. index.ts reports UPGRADE_FAILED as {code,error,hint} and removes the staging folder in finally. The new unit test (staged file missing → docx.exe still holds "current") passes.
• 2. PATH note: fixed without touching install.sh. I matched withoutStagingPathHint against install.sh's PATH-note block (lines 177–185). The filter keys on the same forward-slash shellPrefix that install.sh sees as $PREFIX, so it matches. I also tried a prefix with spaces (C:/Program Files/my bin/.docx-upgrade): only the three note lines are dropped. The two echo blank lines stay, which is cosmetic.
• 3. SECURITY.md: fixed. Line 21 and the docx upgrade network entry now agree with line 110. README, troubleshooting.md and src/cli/CLAUDE.md (read as text) match the code.
• 4. Minor: the case-insensitive docx.exe guard is in. The HELP "binary is left untouched" line now holds.
• New 541ca5c (forward-slash PREFIX): I confirmed the cause on Linux. sha256sum on a file whose name contains a backslash prints \2d71…, and sha256sum < file prints 2d71…. The fix only changes what install.sh is handed on Windows. POSIX still gets PREFIX = the binary's own directory.

Supply-chain check (installer invariant)

• skills/docx-cli/scripts/install.sh and the root install.sh shim are byte-identical to main. It is still the one implementation, embedded at build time and piped to sh -s, with REQUIRE_CHECKSUM=1. Nothing is fetched and then executed. There are no new network calls in TypeScript and no download or checksum logic outside install.sh.
• The PR doesn't touch package.json, the lockfile, CI, scripts/, or the test runner config.

What I ran (Linux, at 541ca5c)

bun run check                                             → biome + knip + tsc clean
bun test tests/cli/upgrade.test.ts tests/cli/installer.test.ts → 23 pass, 0 fail (incl. compiled-binary smoke)
bun run test:unit                                         → 1911 pass, 1 fail

The one failure is output-contract.test.ts › "large CLI Markdown and AST match … through a slow pipe", which hit its 5 s timeout while the full suite was running. Run alone, it passes 91/91. This PR doesn't touch it, so it's load flakiness. I can't run Windows here, so I reviewed the Windows branch by reading the code, plus your Windows 11 run.

Optional nit (not blocking)

In the antivirus case that the rollback exists for, await rm(installPrefix, { recursive: true, force: true }) in finally can itself throw (EBUSY/EPERM on the staged .exe that is still held open). A throw from finally replaces the fail(...) return, so the user would see the catch-all error instead of {code,error,hint}. The binary would still be restored, so it's safe either way. Adding .catch(() => {}) would keep the clean error, and leftovers get cleared on the next run anyway. Similarly, if the restoring rename ever failed too, the message "it is unchanged" would be wrong. That is very unlikely and I don't think it's worth more code.

Decisions for @kklimuk

• Do you want Windows self-upgrade at all? No issue asked for it. It moves the project from "refuse on Windows, swap by hand" to a supported Git Bash path, and leaves a docx.exe.old in the bin dir until the next upgrade.
• Windows CI: add a windows-latest job (at least the compiled-binary --dry-run smoke) before merging, or rely on the contributor's manual Windows 11 run?
• install.sh hashing from stdin (sha256sum < "$1", as @xsvm suggested): this would harden all three install routes against escaped file names. It's out of scope here and would be a separate install.sh change (and SECURITY.md check), so it's your call.
• Optional: ask for the finally nit above.

I pushed nothing. The contributor made the last push, so your own approval satisfies the last-push rule.

Action needed: answer question — decide whether Windows self-upgrade is wanted and whether a Windows CI job is required. If yes, approve and merge PR #26 (your approval alone satisfies the last-push rule, since I pushed nothing). Separately, decide on the install.sh stdin-hashing follow-up.

---

Generated by Claude Code

[kklimuk]

@xsvm hold off from making further changes till i've had a bit of time to read through and do my own reviews, it's gonna take me a few days.

[kklimuk]

🤖 docx-cli triage

Three new commits landed on this PR since my last review (cc0f4ed0, 875523fa, 3f90a6f1, authored and pushed as kklimuk with no routine trailer, so I'm reading them as the maintainer's own review work). The PR head is now 3f90a6f, and its diff touches files this routine never checks out or runs automatically:

• .github/workflows/ci.yml
• .gitattributes (repo-root config)
• install.sh
• skills/docx-cli/scripts/install.sh

So I haven't fetched or executed this head and I'm leaving the PR to the maintainer to review by hand (labelled needs-human). That fits the earlier note asking for changes to stop until the maintainer has reviewed it. I haven't changed anything on the branch.

Action needed: inspect a suspicious PR — confirm cc0f4ed0/875523fa/3f90a6f1 are your own commits (they're unsigned) and review the CI/installer changes by hand; the routine won't touch #26 again unless you leave an unmarked comment telling it to.

---

Generated by Claude Code