Skip to content

[AAASM-3544] 🐛 (test): Guard gateway-resolver auto-start tests when native binary is absent#202

Merged
Chisanan232 merged 2 commits into
masterfrom
v0.0.1/AAASM-3544/guard_windows_gateway_resolver_tests
Jun 26, 2026
Merged

[AAASM-3544] 🐛 (test): Guard gateway-resolver auto-start tests when native binary is absent#202
Chisanan232 merged 2 commits into
masterfrom
v0.0.1/AAASM-3544/guard_windows_gateway_resolver_tests

Conversation

@Chisanan232

Copy link
Copy Markdown
Contributor

Target

Unblock the node-sdk master CI Success gate, which is red because the test-matrix workflow fails on every windows-latest cell (Node 18/20/22/24) while ubuntu-latest / macos-latest pass.

  • Task summary:

    Guard the gateway-resolver auto-start test suites so they skip cleanly on platforms where the native aasm runtime binary is absent (Windows today), keyed on actual binary availability rather than process.platform. Linux/macOS keep full coverage; the guarded cases re-enable automatically once the Windows runtime package is published.

  • Task tickets:

    • Task ID: AAASM-3544.
    • Relative task IDs:
      • AAASM-3809 (Windows runtime packaging — see approach (b) below).
    • Relative PRs:
      • N/A.
  • Key point change (optional):

    Root cause (refined during investigation). The Windows failures are not literally caused by the postinstall warning — the failing tests mock the auto-start seams and never load the native binary. They fail because the auto-start code path + its test fixtures encode POSIX-only assumptions that break on Windows once no native runtime is present:

    Failing test Why it fails on Windows
    autoStartGateway > "spawns aasm and resolves..." assertAllowedAasmPath("/usr/local/bin/aasm")path.win32.resolve rewrites it to C:\usr\local\bin\aasm, which is not under the POSIX allow-list -> ConfigurationError thrown where the test expects resolve
    autoStartGateway > "throws GatewayError when... never ready" same allow-list rejection -> ConfigurationError raised where GatewayError is expected
    assertAllowedAasmPath > "accepts an absolute path inside an allow-listed install dir" same POSIX allow-list (/usr/local/bin, /opt/homebrew/bin, ...) does not match a Windows-resolved path
    default PATH lookup... > "findAasmOnPath returns the first matching binary" fixture joins $PATH with : and writes an extension-less aasm; on Windows the separator is ; (and drive letters contain :) so the lookup finds nothing

    These are exactly the "~4 failures" in the ticket.

    Fix. A small test helper tests/helpers/native-runtime.ts exposes nativeRuntimeAvailable(), which returns true only when @agent-assembly/runtime-<platform>-<arch> (the bundled native aasm package) resolves. The three affected suites/cases are guarded with describe.skipIf(...) / it.skipIf(...) on that flag:

    • describe.skipIf(!NATIVE) -> autoStartGateway
    • describe.skipIf(!NATIVE) -> default PATH lookup and spawn seams
    • it.skipIf(!NATIVE) -> the single POSIX-absolute assertAllowedAasmPath case (its two rejection cases stay platform-independent)

    On Linux/macOS the runtime package is present -> nothing is skipped (verified: 37/37 in this file, 338 passing locally). On Windows the runtime is absent -> the four POSIX-bound cases skip and test-matrix goes green. Keyed on package resolvability (not process.platform), the guard self-heals: once the Windows runtime package ships, the cases run again.

    Assessment of approach (b) — proper long-term fix (NOT implemented here). Building & publishing the Windows native runtime package (mirroring the linux/macOS @agent-assembly/runtime-* packaging) is the durable fix so postinstall selects a real Windows binary. This is release/distribution work that lives in release-node.yml / the build-addon pipeline and overlaps AAASM-3809; it should be tracked there rather than bundled into this test-only CI unblock. Note also that even after publishing, the auto-start allow-list (allowedInstallDirs() in src/core/gateway-resolver.ts) and these fixtures would still need Windows-aware paths (%ProgramFiles%, path.delimiter, aasm.exe) to pass on Windows — so approach (b) is a package and a small production-allow-list change, not just a publish. Recommendation: extend AAASM-3809 (or open a focused follow-up under it) to cover Windows runtime packaging + Windows-aware allow-list/fixtures; no brand-new top-level ticket is warranted.

    Related correctness nit (observed, out of scope). scripts/postinstall.mjs resolves @agent-assembly/<key> (e.g. win32-x64-msvc) while the optionalDependencies are named @agent-assembly/runtime-<platform>-<arch> (e.g. runtime-linux-x64) — the two naming schemes don't line up, and there is no win32 optional dep at all. This guard intentionally keys on the runtime-* packages (the ones actually declared/installed), so it is unaffected; the postinstall naming mismatch is worth folding into the approach-(b) follow-up.

Effecting Scope

  • Action Types:
    • 🔧 Fixing bug
      • 🟢 No breaking change
  • Scopes:
    • 🧪 Testing
      • 🧪 Unit testing
    • 🚀 Building
      • 🤖 CI/CD
  • Additional description:
    Test-only change. No production source, public API, or runtime behavior is modified.

Description

  • Add tests/helpers/native-runtime.ts with nativeRuntimeAvailable() / runtimePackageName() — detects whether the platform's bundled native runtime package resolves.
  • Guard tests/gateway-resolver.test.ts auto-start suites with describe.skipIf / it.skipIf on nativeRuntimeAvailable(), with explaining comments.

How to verify

  • Locally (macOS/Linux — runtime present): pnpm install && pnpm test -> nothing skipped, full coverage (337+ passing; gateway-resolver.test.ts runs all 37). pnpm typecheck and pnpm lint clean.
  • Windows: the four POSIX-bound cases skip; the rest of gateway-resolver.test.ts and the suite run. Note: test-matrix only runs the windows-latest cells on push to master/tags, not on PRs (PRs are ubuntu-only for fast/free minutes — see test-matrix.yml), so this PR's own CI cannot directly exercise Windows; the Windows cells turn green after merge to master.

Closes AAASM-3544

🤖 Generated with Claude Code

Chisanan232 and others added 2 commits June 26, 2026 15:19
Detects whether the platform-specific `@agent-assembly/runtime-<platform>-<arch>`
bundled-runtime package (the native `aasm` binary) is resolvable. Returns false
on platforms with no published runtime package (Windows today). Keyed on actual
package availability, not `process.platform`, so guards self-heal once the
Windows runtime package is published (AAASM-3544 / AAASM-3809).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019mSz31RysZF6DYToUoBWLf
…absent

On windows-latest the auto-start suites fail because they assume POSIX install
dirs and a POSIX `$PATH` shape, while `@agent-assembly/runtime-win32-x64` is not
yet published. Skip the `autoStartGateway`, `default PATH lookup and spawn seams`
suites and the POSIX-absolute `assertAllowedAasmPath` case when the platform
native runtime is unavailable, via describe.skipIf/it.skipIf keyed on
nativeRuntimeAvailable(). Linux/macOS keep full coverage (runtime present);
the cases re-enable automatically once the Windows runtime is published.

Closes AAASM-3544

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019mSz31RysZF6DYToUoBWLf
@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sonarqubecloud

Copy link
Copy Markdown

@Chisanan232

Copy link
Copy Markdown
Contributor Author

🤖 Claude Code — PR Review (AAASM-3544)

Independent, read-only review of this test-only CI unblock.

✅ CI status — green

All 17 PR checks pass (Analyze, CodeQL, SonarCloud, audit, codecov/patch, coverage-and-analysis, module-smoke 18/20/22, napi-build ubuntu 20/22, quality, test ubuntu 18/20/22/24). No fixes were needed.

Caveat (acknowledged in the PR/ticket): PR CI is ubuntu-onlytest-matrix.yml runs the windows-latest cells only on push to master/tags. So this PR's own CI cannot prove the Windows fix. The assessment below is from reading the code, not from CI.

Scope checklist

  • Matches ticket. AAASM-3544 cites 4 windows-latest failures in gateway-resolver.test.ts. The guard covers exactly those: autoStartGateway "spawns aasm…" + "GatewayError when never ready" (both under describe.skipIf), assertAllowedAasmPath "accepts an absolute path…" (it.skipIf), and default PATH lookup "findAasmOnPath returns the first matching binary" (describe.skipIf).
  • Test-only. git diff --stat touches only tests/gateway-resolver.test.ts (+23/-5) and new tests/helpers/native-runtime.ts (+40). No src/, no shipped/runtime change, no public API.
  • Keyed on binary availability, not process.platform — self-heals once the Windows runtime ships.

Side-effect assessment

  • Helper key is correct. nativeRuntimeAvailable() resolves @agent-assembly/runtime-${platform()}-${arch()}/package.json, which exactly matches RUNTIME_SUBPACKAGE in src/runtime.ts and the optionalDependencies (runtime-linux-x64, runtime-darwin-arm64, …). It correctly keys on the declared runtime-* packages, not the divergent postinstall.mjs win32-x64-msvc/linux-x64-gnu scheme (that naming mismatch is a real, separately-noted nit, out of scope here).
  • No silent coverage loss on Linux/macOS. Verified locally: helper resolves @agent-assembly/runtime-darwin-arm64NATIVE_RUNTIME_AVAILABLE=true. Ran vitest run tests/gateway-resolver.test.ts37 passed, 0 skipped. The guarded cases still execute on this platform.
  • Guards target only the genuinely platform-dependent cases. The two assertAllowedAasmPath rejection cases stay unguarded and remain correct on Windows — "aasm"/"./aasm" are non-absolute (platform-independent), and "/tmp/evil/aasm" resolves under win32 to C:\tmp\evil\aasm, still outside the allow-list → still throws. Good call leaving them in.
  • Minor (non-blocking) over-skip. Using describe.skipIf on the whole autoStartGateway and default PATH lookup blocks also sweeps in a few platform-independent neighbors (e.g. "throws ConfigurationError when aasm not on PATH", "findAasmOnPath returns null") on Windows only. This costs nothing on Linux/macOS (all run) and only trims marginal Windows coverage that wasn't reliably passing anyway. Acceptable for a CI unblock.

Will the post-merge win32 cells actually go green? — High confidence

  • Install won't fail the job. runPostinstall() in scripts/postinstall.mjs catches the missing-win32-x64-msvc error and only logger.warns (returns false) — pnpm install exits 0 on Windows despite the warning. The job proceeds to the test step rather than dying at install. This is the linchpin, and it holds.
  • The 4 skipped cases are the only Windows failures per the ticket (it states all 4 failures are in gateway-resolver.test.ts; ubuntu/macOS already pass). Other test files mock the auto-start seams and don't hard-require the native binary at import, consistent with the ticket showing only these 4 red. Skipping them therefore removes the only Windows red.
  • Residual (low) risk: any new Windows-only failure introduced since the last red run isn't provable until the post-merge master run — inherent to the ubuntu-only PR matrix, not a defect of this PR.

Verdict: ✅ APPROVE-READY

Correct, minimal, well-commented, test-only fix that addresses exactly the cited failures and self-heals when @agent-assembly/runtime-win32-x64 ships. The follow-up split (Windows runtime packaging + Windows-aware allow-list/fixtures → AAASM-3809) is the right call. Recommend merge; confirm the windows-latest cells turn green on the post-merge master run.

Read-only review — not approving via gh pr review and not merging, per reviewer mandate.

@Chisanan232 Chisanan232 marked this pull request as ready for review June 26, 2026 07:28
@Chisanan232 Chisanan232 merged commit 39c4287 into master Jun 26, 2026
17 checks passed
@Chisanan232 Chisanan232 deleted the v0.0.1/AAASM-3544/guard_windows_gateway_resolver_tests branch June 26, 2026 07:31
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.

1 participant