Skip to content

feat(computer): ship the Windows helper exe as a release asset with on-demand download#849

Merged
muqsitnawaz merged 5 commits into
mainfrom
agents/win-exe
Jul 10, 2026
Merged

feat(computer): ship the Windows helper exe as a release asset with on-demand download#849
muqsitnawaz merged 5 commits into
mainfrom
agents/win-exe

Conversation

@muqsitnawaz

Copy link
Copy Markdown
Contributor

Closes #547.

agents computer setup --host failed for any npm i -g user with Windows helper exe not built. Run: bash scripts/build-win.sh — the ~157MB computer-helper-win.exe never ships in the npm tarball and resolveWinHelperExe() only checked local build-output paths.

Half 1 — release plumbing (.github/workflows/computer-helper-win.yml)

  • on.push now also matches tags: ['v*'] (the tag apps/cli/scripts/release.sh pushes at the merge commit — no release.sh change needed). Per GitHub docs, "Path filters are not evaluated for pushes of tags", so the existing paths filter does not gate tag runs.
  • New release-exe job (if: github.ref_type == 'tag', contents: write): publishes the self-contained single-file exe with the exact build-win.sh flags, smoke-tests that artifact (smoke/smoke.mjs), writes a sha256sum-format .sha256, creates the GitHub release for the tag if missing (--verify-tag), and gh release upload --clobbers both assets.
  • The two branch/PR smoke jobs get if: github.ref_type != 'tag' — release-exe re-smokes the artifact it ships, so tag runs don't double-build.

Half 2 — download fallback (apps/cli/src/lib/ssh-tunnel.ts)

  • resolveWinHelperExe() keeps local-path resolution first, unchanged.
  • setupRemoteHelper() now calls ensureWinHelperExe(): local build if present, else downloadWinHelperExe(getCliVersion()) — fetches releases/download/v<version>/computer-helper-win.exe, verifies its sha256 against the published .sha256 asset, streams to a .download partial and renames only after the checksum passes, caches under ~/.agents/.cache/computer/win-helper/v<version>/.
  • Only the exact v<version> tag is consulted. A missing asset is a hard error naming that tag (no computer-helper-win.exe release asset for tag v<X> (HTTP 404 on <url>)) — no fallback to other tags.

Verification (real flow, quoted)

Temporary prerelease v0.0.0-win-exe-test with a locally cross-published exe + .sha256 as assets; CLI version temporarily pointed at it; local exe paths absent; empty cache. Note: the issue plan said draft release, but draft assets are not served at the public releases/download/<tag>/ URL (probed: 404) — they are API-only until published — so the test used a short-lived published prerelease, deleted immediately after along with its tag (git ls-remote --tags origin | grep win-exe-test → no match).

$ node dist/index.js computer setup --host win-mini
Downloading computer-helper-win.exe v0.0.0-win-exe-test from GitHub releases (~160 MB)...
pushed computer-helper-win.exe to muqsit@win-mini.tail1a85a1.ts.net
registered LOGON scheduled task "AgentsComputerHelper" (interactive session, started now)
# rc=0, 27.9s total

$ shasum -a 256 ~/.agents/.cache/computer/win-helper/v0.0.0-win-exe-test/computer-helper-win.exe
766ed2f218a995e67137ae55db4ceb10c0ea4ca6dc190a3b68f183f180f32c7e  (matches the published .sha256 asset exactly)

$ node dist/index.js computer start --host win-mini
tunnel: 127.0.0.1:58614 -> muqsit@win-mini.tail1a85a1.ts.net (127.0.0.1:8765)
daemon: answering (ssh pid 70039)
  • vitest: 17/17 pass in ssh-tunnel.test.ts (5 new: URL pinning, .sha256 parsing, streaming sha256 vector, cache hit without network, and a real GitHub 404 asserting the error names the exact tag).
  • actionlint on the workflow: clean. tsc --noEmit: clean.

Final CI proof of the release job completes on the next real v* tag — the job only fires on tag pushes, which this PR cannot trigger.

Docs: native/computer-win/README.md corrected (it claimed release scripts stage the exe into the npm tarball — they never did) + CHANGELOG entry under Unreleased.

…n-demand download

setup --host failed for npm-installed CLIs with 'Windows helper exe not
built' because the ~157MB exe never ships in the tarball and the only
resolution paths were local build outputs (GitHub #547).

Two halves:
- computer-helper-win.yml now also triggers on v* tags (pushed by
  scripts/release.sh) and a new release-exe job publishes the
  self-contained single-file exe, smoke-tests that exact artifact,
  and uploads it + a sha256sum-format .sha256 as GitHub release assets
  (creating the release for the tag when missing; --clobber for re-runs).
  The branch/PR smoke jobs are skipped on tag runs since release-exe
  re-smokes the artifact it ships.
- resolveWinHelperExe stays local-paths-first; setupRemoteHelper now
  falls through to ensureWinHelperExe, which downloads
  releases/download/v<cli-version>/computer-helper-win.exe, verifies
  its sha256 against the published .sha256 asset, and caches it under
  ~/.agents/.cache/computer/win-helper/v<version>/. Only the exact tag
  is consulted; a missing asset is a hard error naming that tag.

Verified end-to-end against a temporary prerelease (deleted after):
with no local exe and an empty cache, computer setup --host win-mini
downloaded the asset, checksum-verified it, pushed it to win-mini, and
start --host reported the daemon answering.
@prix-cloud

prix-cloud Bot commented Jul 10, 2026

Copy link
Copy Markdown

Code Reviewer

Verdict: Ready to merge

Build: Clean — tsc (via bun run build) exits 0, no errors.
Tests: Green — ssh-tunnel.test.ts 17/17 pass locally (including a real, unmocked GitHub 404 against phnx-labs/agents-cli releases, no network mocking). CI's sharded full suite (test-shard (1)/(2)/(3)) and typecheck are all green on the PR, along with build-and-smoke / publish-and-smoke (Windows) and gitleaks.

Repo instructions read: root AGENTS.md/CLAUDE.md, apps/cli/AGENTS.md, native/computer-win/AGENTS.md + README.md. No AGENTS.md/CLAUDE.md exists under .github/.

Changes that work well

  • downloadWinHelperExe's stream-to-.partial-then-rename-after-checksum pattern is correct. An interrupted download or a checksum mismatch never leaves cached populated — the finally block removes the partial file, and renameSync only runs after the hash compares equal (apps/cli/src/lib/ssh-tunnel.ts:213-230). A 157MB exe is never buffered in memory (Readable.fromWeb streamed straight to disk, sha256File streams too).
  • Hard-fails naming the exact tag, no silent fallback to another release — matches the PR's stated design and is covered by a real (non-mocked) 404 test hitting GitHub (ssh-tunnel.test.ts:189-194).
  • CI plumbing is careful about not double-building the shipped artifact: build-and-smoke/publish-and-smoke gate off on tag pushes (if: github.ref_type != 'tag') and the new release-exe job re-smokes the exact self-contained single-file exe it uploads, using the same dotnet publish flags as scripts/build-win.sh and the same smoke/smoke.mjs --exe/--port contract the other two jobs use.
  • CHANGELOG entry format matches the existing convention in this file exactly (bold summary line, source file/function pointers, issue reference).
  • Verified the parseSha256Asset regex and sha256File test vector independently: printf 'hello' | shasum -a 2562cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824, matching the test's expected value exactly.

Things to verify manually

  • The actual release-exe job (tag-triggered gh release create/upload path) can't be exercised by this PR — it only fires on v* tag pushes, as the author notes. First real signal will be the next v* release; worth eyeballing that run once it fires.
  • actionlint isn't available in this sandbox, so I couldn't independently re-verify the "actionlint clean" claim on computer-helper-win.yml — the YAML reads correctly by inspection (permissions scoped to contents: write only on the tag job, --clobber/--verify-tag used as described).

Reviewed by Code Reviewer — actually ran the build and tests on this branch.

@muqsitnawaz muqsitnawaz merged commit 4e861c7 into main Jul 10, 2026
11 checks passed
@muqsitnawaz muqsitnawaz deleted the agents/win-exe branch July 10, 2026 02:52
@prix-cloud

prix-cloud Bot commented Jul 10, 2026

Copy link
Copy Markdown

Code Reviewer

Verdict: Ready to merge (two non-blocking design/doc notes below)

Build: Clean — bun run build (tsc) exits 0, no errors.
Tests: apps/cli — scoped ssh-tunnel.test.ts: 17/17 pass (via both bun test and the repo's actual vitest runner), matching the PR's claimed count exactly, including the real-network 404 test (downloadWinHelperExe('0.0.0-ssh-tunnel-no-such-tag') against the live GitHub API). Full-repo vitest run was still executing when this review's time budget closed — scoping to the changed file is sufficient signal here since nothing outside ssh-tunnel.ts/.test.ts was touched on the TS side.

Read for repo conventions before reviewing: root AGENTS.md, apps/cli/AGENTS.md, native/computer-win/AGENTS.md (all three touched-directory maps), plus .github/rush.yml (confirms prix/code-reviewer is the configured non-author reviewer for this repo — noted for context, this review was explicitly requested).

Changes that work well

  • downloadWinHelperExe fetches the small .sha256 asset before the 157MB exe, so a missing release fails fast (~sub-second) instead of after a 15-minute download attempt.
  • Download safety is correct: streams to a <file>.download partial, hashes it, and only renameSyncs into the cache path after the checksum matches — an interrupted or corrupted download can never be picked up as a valid cache hit on the next run.
  • ensureWinHelperExe() preserves resolveWinHelperExe()'s local-build-first precedence unchanged, so repo-checkout devs are unaffected.
  • The workflow's if: github.ref_type == 'tag' / != 'tag' split cleanly separates the branch/PR smoke jobs from the release job, and release-exe re-smokes the exact artifact it uploads rather than trusting the publish-and-smoke leg's build.
  • Verified the "path filters aren't evaluated for tag pushes" GitHub behavior the PR's design leans on — that's correct as stated, so tags: ['v*'] will fire release-exe regardless of the paths: filter.

Issues that need attention

1. Publish/tag-push ordering leaves a window where npm i -g installs a version with no matching release asset yet (design gap, not a code bug) — apps/cli/scripts/release.sh:580,586,597:

git tag "v$TARGET" "$MERGED_SHA"      # 580 — local only
...
npm publish --access=public ...        # 586 — package now installable
...
git push origin "v$TARGET"             # 597 — only now does the tag (and the release-exe trigger) exist on GitHub

release-exe only starts after the tag lands on GitHub (line 597), and then still has to run dotnet publish + the UIA smoke test on a Windows runner before the asset is uploaded — realistically a few minutes. Any Windows user who runs npm i -g @phnx-labs/agents-cli@latest and agents computer setup --host in that window hits downloadWinHelperExe's hard error: no computer-helper-win.exe release asset for tag v<X> (HTTP 404 ...). That message reads as "this version isn't supported," not "try again in a few minutes" — worth a friendlier retry hint, or at minimum a known-limitation note, since this is a new dependency the old resolveWinHelperExe()-only code never had (it never touched the network). Not introduced by a bug in this diff's own files, but this PR is what makes setupRemoteHelper() depend on that timing for the first time.

2. native/computer-win/AGENTS.md:28 still carries the exact stale claim this PR just fixed in the sibling READMEnative/computer-win/AGENTS.md:28:

Output: `dist/computer-helper-win.exe` (gitignored; staged into the npm tarball at
release). Needs the **.NET 10 SDK**; ...

This is the same incorrect "staged into the npm tarball at release" claim that native/computer-win/README.md:58-59 had before this PR and that the PR explicitly corrected there (the exe never shipped in the tarball). apps/cli/AGENTS.md:143's computer-win row (No (staged at release)) has the same drift — it's no longer "staged at release" at all, it's built + uploaded as a GitHub release asset and downloaded on demand. Since this PR's whole premise is fixing exactly this misconception, leaving it standing in the two AGENTS.md files that map this same directory will mislead the next reader.

Things to verify manually

  • The release-exe job itself can only be exercised on a real v* tag push — the author's manual verification (temporary published prerelease v0.0.0-win-exe-test, deleted after) is a reasonable substitute, but the actual workflow trigger path is still unproven until the next real release. Worth watching the first post-merge release closely.
  • Minor, low-likelihood race: downloadWinHelperExe writes to a fixed <cache>/<version>/computer-helper-win.exe.download path with no per-process suffix. Two concurrent first-time agents computer setup --host <A> / --host <B> invocations (same CLI version, empty cache) would both stream into that same partial file. Worst case is a checksum mismatch causing one or both invocations to fail loudly (the verify-then-rename design means it can't corrupt a cache hit) — not a correctness risk, but a confusing error if it ever happens; a retry would succeed.

Reviewed by Code Reviewer — actually ran the build and tests on this branch.

@prix-cloud

prix-cloud Bot commented Jul 10, 2026

Copy link
Copy Markdown

Code Reviewer

Verdict: Changes requested (one real concurrency bug; rest is solid)

Build: Clean — bun install && bun run build (tsc) passes with no errors.
Tests: bun test src/lib/ssh-tunnel.test.ts (the file this PR touches) — 17/17 pass, including the real-network 404 case against GitHub. The repo's full bun test suite was still running after several minutes when this review was written; scoping to the changed file is the pragmatic call here per the touched-package signal, so treat the full-suite result as unverified rather than green.

Read for context: root AGENTS.md/CLAUDE.md, apps/cli/AGENTS.md, native/computer-win/AGENTS.md (all three touched-directory memory files).

I also verified the PR's central technical claim independently: GitHub's paths filter is documented to not apply to tag pushes, so on.push.tags: ['v*'] combined with the existing paths: block will indeed fire release-exe on every v* tag regardless of whether native/computer-win/** changed — matches the PR description.

Changes that work well

  • The download path (downloadWinHelperExe) does the right things in the right order: cache-hit short-circuit, checksum fetched before the ~157MB asset (fails fast on a missing tag), stream-to-temp + hash-verify + rename-only-on-success so a killed download can never be picked up as a valid cache entry later, and a hard, tag-naming error with zero silent fallback to another release. parseSha256Asset and sha256File are both covered with real test vectors (a real sha256("hello") vector, a real 404 against GitHub), not mocks.
  • Workflow gating (if: github.ref_type != 'tag' on the two existing jobs, == 'tag' on the new one) is correct and the "re-smokes what it ships" reasoning for skipping a double build on tag runs holds up.
  • Docs/CHANGELOG entry is accurate and matches the code.

Issues that need attention

1. Race condition: concurrent downloads for the same version corrupt/clobber each otherapps/cli/src/lib/ssh-tunnel.ts:216-229

const partial = `${cached}.download`;
try {
    await pipeline(Readable.fromWeb(exeRes.body ...), fs.createWriteStream(partial));
    const actual = await sha256File(partial);
    if (actual !== expected) throw new Error(...);
    fs.renameSync(partial, cached);
} finally {
    fs.rmSync(partial, { force: true });
}

partial is derived only from version (winHelperCacheDir(version)), not from the process. Two separate agents computer setup --host <A> and agents computer setup --host <B> invocations (two OS processes — normal when a user provisions more than one Windows box, and --host only ever takes one device per invocation, so this is the only way to do two at once) racing against a cold cache for the same CLI version both compute the identical partial path. Both fs.createWriteStream calls truncate-open the same inode and write concurrently, and whichever process's finally fires first unconditionally rmSyncs the file out from under the other's still-in-flight stream/rename — the loser's fs.renameSync(partial, cached) throws ENOENT, and even the "winner" isn't guaranteed clean interleaved bytes weren't written by both writers before that point.

This isn't a hypothetical the codebase hasn't already solved: apps/cli/src/lib/fs-atomic.ts exists for exactly this class of problem — atomicWriteFileSync names its temp file ${filePath}.tmp-${process.pid}-${randomBytes(8)} specifically so concurrent writers never collide, and withFileLock wraps a proper-lockfile-backed critical section for cross-process mutual exclusion (already used by devices/registry.ts, teams/registry.ts, audit/log.ts). downloadWinHelperExe reinvents a similar-looking temp+rename mechanism but drops the uniqueness, reintroducing the race those helpers exist to prevent.

Fix: suffix partial with process.pid/randomBytes, or wrap the download in withFileLock keyed on the cache dir.

Nit — documentation left in the exact state this PR calls "wrong"

The PR body says native/computer-win/README.md was fixed because it "claimed release scripts stage the exe into the npm tarball — they never did." Two other docs in the same touched scope still make that identical, now-disproven claim and weren't updated:

  • native/computer-win/AGENTS.md:28 (same directory as the README that was fixed): "Output: dist/computer-helper-win.exe (gitignored; staged into the npm tarball at release)."
  • apps/cli/AGENTS.md:143: "| computer-win | ... | No (staged at release) | src/lib/ssh-tunnel.ts |"

AGENTS.md is called out repo-wide as the canonical memory file agents read before touching code — worth a one-line fix in the same PR so it doesn't reintroduce the confusion the README fix just resolved.

Things to verify manually

  • The workflow has no workflow_dispatch trigger, so if release-exe fails transiently on a real tag push (flaky Windows runner), the only recovery paths are GitHub's "re-run failed jobs" UI/gh run rerun --failed, or re-tagging. Worth confirming the team's runbook covers this, since a failed release-exe run means that exact CLI version has no Windows asset and downloadWinHelperExe hard-errors for any Windows user on it (by design, per the PR — no fallback to another tag).
  • I couldn't exercise the actual GitHub Actions release-exe job (tag pushes can't be triggered from this review), so the author's own real-tag verification (quoted in the PR description) is the only end-to-end evidence for that half — I did independently confirm the paths-filter-ignored-on-tags behavior it depends on.

Reviewed by Code Reviewer — actually ran the build and tests on this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

computer setup --host: ship the Windows helper exe (on-demand release-asset download)

1 participant