Files
software-mansion__argent/.github/workflows/windows-e2e.yml
Ignacy Łątka 86a2668fb0 test: stop the suite and the process environment leaking into each other (#1058)
Test env leakage, both directions.

**Out** — six `argent-tools-client` suites pointed `HOME`/`USERPROFILE`
at a temp dir, deleted the dir, and never restored, so a `--no-isolate`
run handed every later file a home that no longer exists. Lifted the
redirect into a shared `test/helpers/home-redirect.ts`; the three suites
that already hand-rolled the save/restore moved onto it too.

**In** — `packages/telemetry` had no `setupFiles` at all, so its unit
tests read the developer's shell. Added
`test/setup/clear-telemetry-env.ts` (ARGENT_ prefix sweep +
`DO_NOT_TRACK` + the cloud-agent detector's markers), wired via
`setupFiles`, following the `@argent/tool-server` precedent — and
dropped the per-file guards it replaces.

**Pinned, not just fixed** — a `test/setup/assert-env-restored.ts` in
each of the three packages fails any suite that leaves a watched
variable changed, so dropping a restore can no longer pass. Each watches
`HOME` and `USERPROFILE`, plus `PATH` (`argent-tools-client`,
`tool-server`) or `CI` and the five `OTEL_EXPORTER_OTLP_*` (`telemetry`)
— the ones the sweep deliberately leaves ambient and individual tests
then set by hand. Each of the three `clear-*-env` setup files has a pin
test that re-imports it against planted sentinels;
`packages/tool-server`'s existing copy gains the mixed-case one it was
missing.

`packages/tool-server` is in the diff because it has the same two gaps:
it originated the prefix sweep (both pre-existing copies were
case-sensitive, so they are fixed together) and it has the largest
redirect surface of the three — 12 files hand-rolling the redirect plus
17 going through `scopeTempHome` — with no guard. It is already green
under one; deleting `scopeTempHome`'s restore turns 17 files red. Two of
its suites restored `HOME` by assigning back a saved value that can be
`undefined`, writing the string `"undefined"`; the guard caught both on
a host with no ambient `HOME`, and they are fixed here.

`windows-e2e.yml` runs seven `tool-server` files through the config this
branch edits, but neither that config nor `test/setup/**` was in its
`paths:` filter — so the new setup file would have reached
windows-latest unverified. Added both.

No docs change: test infrastructure only, no user-facing behaviour.

Fixes #1028
Fixes #839

Same-class problems found while reviewing this and filed rather than
folded in: #1068 (`argent-cli` fails under `DO_NOT_TRACK=1`), #1069 (the
`ios-device-runner` suites pin only `HOME`, so the redirect is inert on
Windows — its second half, the restore that wrote the string
`"undefined"`, is fixed here because the new guard caught it), #1070
(`useTempHome` shares one saved-`HOME` slot, so a nested `scopeHome()`
restores a deleted dir), #1071 (`tool-server`'s sweep is prefix-only, so
an exported `ADB_SERVER_SOCKET` reddens its suite), #1072
(`external-devices.test.ts` reads the developer's real
`~/.argent/providers`) and #1073 (`flow-script-environment.test.ts`
fails with no ambient `HOME`).

<details>
<summary>#1028 — probe results, six files</summary>

`test/zz-home-probe.test.ts` from the issue body, run per suite.
Vitest's sequencer orders by cached duration and falls back to file
size, so the cache is cleared before each run to keep the tiny probe
last:

```
rm -rf node_modules/.vite/vitest ../../node_modules/.vite/vitest
npx vitest run test/<suite>.test.ts test/zz-home-probe.test.ts --no-isolate --no-file-parallelism
```

| suite | before | after |
| --- | --- | --- |
| `launcher-state` | 1 failed \| 23 passed — `argent-state-test-…
exists=false` | 24 passed |
| `launcher-spawn` | 1 failed \| 8 passed — `argent-spawn-test-…
exists=false` | 9 passed |
| `launcher-version-gate` | 1 failed \| 7 passed —
`argent-version-gate-test-… exists=false` | 8 passed |
| `launcher-duplicate-spawn` | 1 failed \| 10 passed —
`argent-dup-test-… exists=false` | 11 passed |
| `launcher-exit` | 1 failed \| 1 passed — `argent-exit-test-…
exists=false` | 2 passed |
| `link-config` | 1 failed \| 43 passed — `argent-link-config-test-…
exists=false` | 44 passed |
| `launcher-sweep` (control, fixed in #713) | passed | passed |
| `launcher-ps-guard` (control, fixed in #713) | passed | passed |

The probe itself is a diagnostic and is not part of the diff —
`test/setup/assert-env-restored.ts` is the durable guard, and it needs
no `--no-isolate` to fire.
</details>

<details>
<summary>#839 — the stated repro is stale; here is what is actually
left</summary>

The issue was filed 2026-08-18. #714 landed 2026-08-19 and
hand-enumerated the variables inside `index.test.ts` (a
`CONSENT_ENV_KEYS` `beforeEach`) and `base-props.test.ts` (eight
`delete`s). On `aa90873b5` all three named repros are green:

```
$ DO_NOT_TRACK=1            npx vitest run --root packages/telemetry   → 319 passed
$ ARGENT_TELEMETRY=0        npx vitest run --root packages/telemetry   → 319 passed
$ CURSOR_AGENT_WORKER_ID=x  npx vitest run --root packages/telemetry   → 319 passed
```

What remains is the structural gap the issue names: the protection is
per-file and per-variable, nothing pins it, and `vitest.config.ts` still
has no `setupFiles`. This PR moves the property to the setup file and
deletes the two hand-rolled guards, so the setup file is what holds it —
see the mutation section, where removing the `setupFiles` line
reproduces the issue's 17 and 1 exactly.

Post-change sweep, one hostile value per run, 329 tests each:
`DO_NOT_TRACK=1`, `ARGENT_TELEMETRY=0`, `ARGENT_TELEMETRY_DEBUG=1`,
`CLAUDE_CODE_ENVIRONMENT_KIND=byoc`, `CLAUDE_CODE_ENTRYPOINT=remote`,
`CLAUDE_CODE_REMOTE_SESSION_ID=x`, `CURSOR_AGENT_WORKER_ID=x`,
`CURSOR_WORKER_POOL_NAME=x`, `GITHUB_ACTIONS=true`,
`GITHUB_ACTOR=copilot`,
`GITHUB_WORKFLOW_REF=…copilot-swe-agent.yml@refs/heads/main`,
`REPLIT_AGENT=1`, `ARGENT_HOST`/`ARGENT_PORT`/`ARGENT_AUTH_TOKEN`,
`CI=1`, `CI=false`, `OTEL_EXPORTER_OTLP_ENDPOINT`,
`OTEL_EXPORTER_OTLP_HEADERS`, the real CI combination, and all eleven at
once — 329 passed every time.

`GITHUB_ACTIONS` is a ci-info vendor input as well as a copilot-agent
marker, so clearing it does move `isCi()` — but only its GitHub Actions
branch, and an Actions run also sets `CI`, which `isCi()` answers ahead
of any vendor check. `GITHUB_ACTOR` and `GITHUB_WORKFLOW_REF` appear in
neither `ci-info/vendors.json` nor `ci-detect.ts`'s generic list, so
they cost nothing. The rest of the ci-info surface and `OTEL_*` are
deliberately left alone; the setup file says so.
</details>

<details>
<summary>Mutation testing — every guard, applied by hand, verified to
fail, reverted</summary>

| mutation | result |
| --- | --- |
| drop `setupFiles` from `packages/telemetry/vitest.config.ts` |
`DO_NOT_TRACK=1` → 19 failed (**17 in `index.test.ts`**, matching the
issue, + the two registration pins) |
| drop `CURSOR_WORKER_POOL_NAME` from `CLEARED_ENV_VARS` | drift test
red |
| move an `env.` read into `src/detect/vendor.ts` | drift test red,
naming `detect/vendor.ts` |
| rewrite a read as `env["X"]`, `const { X } = env`, `const { X }:
NodeJS.ProcessEnv = env` or `const { X = "d" } = env` | drift test red |
| add `cloud-agent-detect.ts` to `UNSWEPT_SRC_FILES` | drift test red |
| revert `toUpperCase()` in any of the three sweeps | that package's
mixed-case case red |
| drop `restoreHome()` from one suite / from all nine | 1 file / 9 files
red, each naming its deleted dir |
| drop `scopeTempHome()`'s restore in `tool-server` | 17 files red |
| drop the hand-rolled `CI` restore in `base-props.test.ts` | red,
naming that file (green before; under `--no-isolate`, 7 unrelated
failures in `otel-endpoint.test.ts`) |
| narrow either guard's filter to variables that were already set |
`assert-env-restored.test.ts` red |
| replace the leaked half of the guard's message with a constant |
`assert-env-restored.test.ts` red |
| rename one function's `env` parameter in `cloud-agent-detect.ts` |
drift test red — the read is found under the new binding |
| narrow any guard's filter to a subset of `SCOPED_ENV_VARS` |
`assert-env-restored.test.ts` red — the pin plants every entry |
| drop the hand-rolled OTLP restore in `otel-endpoint-live.test.ts` |
red, naming all four leaked variables |
| move the guard's `AMBIENT` snapshot inside `assertEnvRestored` |
`assert-env-restored.test.ts` red in all three packages |
| delete the guard's `afterAll(...)` registration |
`assert-env-restored.test.ts` red in all three packages |
| drop `restoreHome()` from `artifacts.test.ts`'s two `afterEach` | red
— the file-level guard catches a per-test omission |
| drop ps-guard's `PATH` restore | red, naming the file |
| empty `leakedEnvVars`, or drop a name from `SCOPED_ENV_VARS` |
`assert-env-restored.test.ts` red |
| replace the helper's delete branch with a plain assignment |
`home-redirect.test.ts` red |
| drop `USERPROFILE` from the helper | `home-redirect.test.ts` red |

</details>

<details>
<summary>Local CI parity</summary>

```
npx vitest run --root packages/argent-tools-client               213 passed | 1 skipped
npx vitest run --root packages/argent-tools-client --no-isolate  213 passed | 1 skipped
npx vitest run --root packages/telemetry                         329 passed
npx vitest run --root packages/telemetry --no-isolate            329 passed
npx vitest run --root packages/tool-server                       6166 passed | 1 skipped
npx tsc --noEmit -p packages/{argent-tools-client,telemetry,tool-server}/tsconfig.test.json   clean
npm run format / npm run lint / npm run knip                                                 clean
```
</details>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Tests**
- Improved test isolation by consistently restoring temporary
environment settings after test runs.
- Added safeguards that detect leaked environment settings and flag
incomplete cleanup.
- Added coverage to ensure telemetry-related environment variables are
cleared before tests and do not affect results.
- Added cross-platform checks for environment variable handling,
including mixed-case names.
- Expanded validation for home-directory redirection and restoration
behavior.
- Added checks confirming environment cleanup runs in the correct order
across test suites.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-09-09 12:38:51 +02:00

194 lines
9.1 KiB
YAML

name: Windows E2E
# Proves Argent runs on Windows. iOS Simulator is macOS-only (Apple), so
# "Argent on Windows" means the Android + Chromium host control plane.
#
# This job, windows-chromium, has the tool-server drive a real headless Chrome
# over CDP (discover → screenshot → describe → tap → observe the DOM change).
# Chromium control is pure host-side TypeScript, so a hosted Windows runner (no
# virtualization) verifies it fully. It also runs the cross-platform resolver
# unit tests natively on Windows and confirms adb.exe resolution.
#
# The simulator-server.exe itself (the binary the Android path spawns) lives in
# the private radon repo, so it can't be built here without a cross-repo token.
# Its Windows build + smoke-run is verified in radon CI instead — see the
# build_argent_windows job in radon's simulator-server-ci workflow (companion
# PR). WHPX is available on windows-latest (Server 2025), so an Android emulator
# does boot on a hosted runner — that is where the boot-device stdio fix was
# verified. The full Android emulator E2E still lives in radon CI alongside that
# binary rather than here, because this repo can't build simulator-server.exe
# without a cross-repo token; that path is therefore out of scope for this job.
on:
workflow_dispatch:
pull_request:
branches:
- main
paths:
- "packages/tool-server/src/utils/command-on-path.ts"
- "packages/tool-server/src/utils/android-binary.ts"
- "packages/tool-server/src/utils/adb.ts"
- "packages/tool-server/src/utils/check-deps.ts"
- "packages/tool-server/src/utils/vega-cli.ts"
- "packages/tool-server/src/utils/chromium-discovery.ts"
- "packages/tool-server/src/tools/simulator/stop-metro.ts"
- "packages/tool-server/src/tools/devices/boot-device.ts"
- "packages/tool-server/src/utils/device-shutdown.ts"
# The unit tests this job runs — editing one must re-run it here, and so
# must editing the config or a setup file every one of them loads.
- "packages/tool-server/vitest.config.ts"
- "packages/tool-server/test/setup/**"
- "packages/tool-server/test/command-on-path.test.ts"
- "packages/tool-server/test/android-binary-windows.test.ts"
- "packages/tool-server/test/adb-resolve-avd-path.test.ts"
- "packages/tool-server/test/check-deps.test.ts"
- "packages/tool-server/test/http-dep-gate.test.ts"
- "packages/tool-server/test/stop-metro-netstat.test.ts"
- "packages/tool-server/test/device-shutdown.test.ts"
- "packages/native-devtools-ios/src/index.ts"
- "packages/native-devtools-ios/test/index.test.ts"
- "packages/argent/scripts/bundle-tools.cjs"
- "packages/argent/scripts/argent-simulator-server.cjs"
- "scripts/dev.cjs"
- "scripts/download-simulator-server.sh"
- "scripts/ci/windows-chromium-e2e.mjs"
- "scripts/ci/e2e-chromium-page.html"
- ".github/workflows/windows-e2e.yml"
jobs:
windows-chromium:
name: Chromium control plane + resolver unit tests (Windows)
runs-on: windows-latest
timeout-minutes: 20
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: "20"
- name: Install dependencies
run: npm ci
- name: Fix rollup native bindings (win32)
# vitest 4 transforms via rollup, whose platform binary is an optional
# dep npm ci sometimes skips; install the win32 one explicitly.
run: npm install @rollup/rollup-win32-x64-msvc --no-save
- name: Build workspace
run: npm run build
- name: Resolver unit tests on Windows (tool-server)
# The win32 branches added for Windows support, exercised on a real
# Windows host (real `where.exe`, real `.exe` access semantics) rather
# than a mocked platform. The broader suite has POSIX-only fixtures, so
# this is the curated cross-platform subset.
working-directory: packages/tool-server
run: >
npx vitest run
test/command-on-path.test.ts
test/android-binary-windows.test.ts
test/adb-resolve-avd-path.test.ts
test/check-deps.test.ts
test/http-dep-gate.test.ts
test/stop-metro-netstat.test.ts
test/device-shutdown.test.ts
- name: Resolver unit tests on Windows (native-devtools-ios)
working-directory: packages/native-devtools-ios
run: npx vitest run
- name: adb.exe resolves on Windows
# windows-latest ships the Android SDK with ANDROID_HOME/ANDROID_SDK_ROOT
# set but does NOT put platform-tools on PATH — exactly the gap the
# Windows resolver closes (`.exe` suffix + SDK-root fallback). Resolve
# adb the way the tool-server does, then run the *resolved* path (a bare
# `adb` wouldn't be found, which is the whole point).
shell: pwsh
run: |
$adb = node -e "require('./packages/tool-server/dist/utils/android-binary.js').resolveAndroidBinary('adb').then(p => process.stdout.write(p || ''))"
Write-Host "resolved adb -> $adb"
if ([string]::IsNullOrWhiteSpace($adb) -or -not $adb.ToLower().EndsWith('adb.exe')) {
Write-Error "expected resolveAndroidBinary('adb') to return an adb.exe path"
exit 1
}
if (-not (Test-Path $adb)) { Write-Error "resolved adb path does not exist: $adb"; exit 1 }
& $adb version
- name: Locate Chrome
id: chrome
run: |
$candidates = @(
"$env:ProgramFiles\Google\Chrome\Application\chrome.exe",
"${env:ProgramFiles(x86)}\Google\Chrome\Application\chrome.exe",
"$env:LOCALAPPDATA\Google\Chrome\Application\chrome.exe"
)
$chrome = $candidates | Where-Object { Test-Path $_ } | Select-Object -First 1
if (-not $chrome) { Write-Error "Google Chrome not found on this runner"; exit 1 }
Write-Host "Chrome: $chrome"
"path=$chrome" >> $env:GITHUB_OUTPUT
# Launch Chrome + the tool-server AND run the E2E in one step. On Windows
# runners, `Start-Process` background processes don't reliably survive into
# a later step (unlike `nohup` on Linux), so both must stay in scope for
# the duration of the E2E — hence a single step rather than three.
- name: Launch Chrome + tool-server and run Chromium E2E
env:
ARGENT_E2E_URL: http://127.0.0.1:3033
run: |
$page = "file:///" + ((Resolve-Path scripts/ci/e2e-chromium-page.html).Path -replace '\\','/')
$udd = Join-Path $env:RUNNER_TEMP "chrome-e2e-profile"
$chromeLog = Join-Path $env:RUNNER_TEMP "chrome.log"
Start-Process -FilePath "${{ steps.chrome.outputs.path }}" -ArgumentList @(
"--headless=new","--remote-debugging-port=9222","--user-data-dir=$udd",
"--no-first-run","--no-default-browser-check","--disable-gpu",$page
) -RedirectStandardOutput $chromeLog -RedirectStandardError "$chromeLog.err" | Out-Null
$cdpUp = $false
for ($i = 0; $i -lt 30; $i++) {
try {
$r = Invoke-WebRequest -UseBasicParsing "http://127.0.0.1:9222/json/version" -TimeoutSec 2
if ($r.StatusCode -eq 200) { Write-Host "CDP up at t+${i}s"; $cdpUp = $true; break }
} catch {}
Start-Sleep -Seconds 1
}
if (-not $cdpUp) { Get-Content "$chromeLog.err" -ErrorAction SilentlyContinue; Write-Error "Chrome CDP never came up on :9222"; exit 1 }
$env:ARGENT_PORT = "3033"
New-Item -ItemType Directory -Force -Path "$env:USERPROFILE\.argent" | Out-Null
$tsLog = Join-Path $env:RUNNER_TEMP "tool-server.log"
Start-Process -FilePath "node" -ArgumentList @("packages/tool-server/dist/index.js","start") `
-RedirectStandardOutput $tsLog -RedirectStandardError "$tsLog.err" | Out-Null
$tsUp = $false
for ($i = 0; $i -lt 40; $i++) {
try {
$r = Invoke-WebRequest -UseBasicParsing "http://127.0.0.1:3033/tools" -TimeoutSec 2
if ($r.StatusCode -eq 200) { Write-Host "tool-server up at t+${i}s"; $tsUp = $true; break }
} catch {}
Start-Sleep -Seconds 1
}
if (-not $tsUp) {
Write-Host "--- tool-server.log ---"; Get-Content $tsLog -ErrorAction SilentlyContinue
Write-Host "--- tool-server.err ---"; Get-Content "$tsLog.err" -ErrorAction SilentlyContinue
Write-Error "tool-server never came up on :3033"; exit 1
}
$env:ARGENT_E2E_OUT = Join-Path $env:RUNNER_TEMP "e2e"
node scripts/ci/windows-chromium-e2e.mjs
- name: Upload artifacts
if: always()
uses: actions/upload-artifact@v4
with:
name: windows-chromium-e2e
path: |
${{ runner.temp }}/e2e/*.png
${{ runner.temp }}/tool-server.log
${{ runner.temp }}/tool-server.log.err
${{ runner.temp }}/chrome.log
${{ runner.temp }}/chrome.log.err
if-no-files-found: warn
retention-days: 7