PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: Not Drop+Supersede — the premise is sound, #10684 is a valid current leaf ticket, the architecture mirrors the existing resolveCodexCliPath sibling, and the Linux half genuinely works (verified below). Not Approve / Approve+Follow-Up either: the PR body claims the adapter works "across platforms," and Resolves #10684 (titled "cross-platform execution") will close on merge — but Windows execution is not delivered. Merging as-is bakes rhetorical drift into a closed ticket + the ask_knowledge_base graph (§7.4). One focused Required Action; the fast author path (Option B below) is a quick cycle.

§9.0 Cycle-1 Premise Pre-Flight: cleared — no false premise, no ungraduated upstream, no authority bypass, no anti-pattern, no roadmap conflict, source ticket #10684 current. Iterative Request Changes is the correct shape, not Drop+Supersede.

Peer-Review Opening: Thanks for taking #10684 — the resolution design is clean and the env-override-first / candidate-list / PATH-fallback shape mirrors resolveCodexCliPath nicely. The Linux path is genuinely solid (I ran the spec — all green). One real gap holds the merge: the Windows path resolves but doesn't execute. Detail below; the fast path out is a body re-scope + follow-up ticket.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #10684
• Related Graph Nodes: PR #10680 (introduced the antigravity-cli adapter), ai/scripts/resumeHarness.mjs (resolveCodexCliPath sibling pattern), spawnAsync (resumeHarness.mjs:50)

🔬 Depth Floor

Challenge (per §7.1): The Windows path resolves a .cmd that the executor cannot spawn.

This PR is what first routes Windows into the antigravity-cli spawn branch. Before this PR, resumeHarness.mjs:320 forced every non-darwin platform to the tmux adapter — Windows never reached the antigravity-cli branch. The new adapter-selection ternary keeps antigravity-cli selected on Windows. So this PR newly makes the following path reachable:

1. resolveAntigravityCliPath() on win32 → candidates are ...\\Antigravity\\bin\\antigravity.cmd (explicit) or whatever findExecutableOnPath('antigravity') returns. findExecutableOnPath tries ['.cmd', '.exe', '.bat', ''] in that order — .cmd first. Antigravity is a VS Code fork; like code.cmd, its CLI ships as antigravity.cmd. So the resolved win32 path is a .cmd in the expected case.
2. The antigravity-cli branch then calls await spawnAsync(cliPath, args, identity) → spawnAsync (resumeHarness.mjs:52) calls spawn(cmd, args, { stdio: 'ignore' }) — no shell option.
3. Per Node's documented child_process behavior, .bat/.cmd files on Windows can only be invoked via spawn with shell: true (or exec, or by spawning cmd.exe). A plain spawn of a .cmd throws EINVAL on current Node (the post–CVE-2024-27980 hardening).

Net: on Windows the user gets neither a working antigravity-cli nor the old tmux fallback — they get an EINVAL crash. That also trips AC2 ("gracefully skip or throw an informative error rather than crashing"): the resolved-but-unspawnable case is an ungraceful crash, not the actionable diagnostic the new test asserts for the not-found case.

This is also why the PR's own honest Evidence: L2 → L4 required (real Windows/Linux Antigravity host fresh-session spawn proof) residual is load-bearing — the .cmd/spawn mismatch is the concrete mechanism by which the unverified L4 residual would fail on a real Windows host.

Verification: confirmed by reading the diff + spawnAsync (grep -n 'spawn(' ai/scripts/resumeHarness.mjs → line 52, { stdio: 'ignore' }, no shell) — the win32 candidates are .cmd literals in the diff. Node's .cmd-needs-shell rule is documented child_process behavior. The Linux path is verified working (spec run below) because spawn honors a real executable / shebang script on Linux without shell.

Rhetorical-Drift Audit (per §7.4):

• PR description: drift flagged — "Keeps the Antigravity fresh-session adapter on the native antigravity-cli path across platforms" overshoots. Linux execution is delivered; Windows execution is not (resolved .cmd is unspawnable). Framing must be tightened, or the implementation completed.
• Anchor & Echo summaries: the new @summary on resolveAntigravityCliPath is accurate for the resolution function in isolation. No drift inside the JSDoc itself.
• [RETROSPECTIVE] tag: none in PR body — N/A.
• Linked anchors: #10680 correctly cited as the adapter's origin; accurate.

Findings: Rhetorical drift flagged on the PR description "across platforms" claim — folded into the single Required Action below.

🧠 Graph Ingestion Notes

• [KB_GAP]: Node's .bat/.cmd-require-shell:true rule on Windows is a recurring cross-platform-spawn footgun. The codebase's harness layer (spawnAsync) has been darwin-only in practice until this PR, so the rule had not surfaced. Worth a one-line note wherever harness adapters are documented.
• [TOOLING_GAP]: CI has no Windows runner, so the win32 resolution+spawn branch has zero automated coverage — the gap here is invisible to the green checkmark. Mocked-process.platform unit coverage for the resolution branch is achievable in-sandbox even though spawn proof is inherently L4.
• [RETROSPECTIVE]: Good scope discipline — the PR explicitly kept Codex/Claude non-macOS behavior on tmux and documented that in "Deltas from ticket." The PATH-lookup-only strategy for Linux (no hardcoded install path) is the correct call per truth-in-code: Antigravity's Linux install shape varies (.deb/AppImage/tarball), so hardcoding an unverified path would be worse than the env-var + PATH + actionable-error trio shipped here.

N/A Audits — 🛂 📑 📜 📡 🔌

N/A across listed dimensions: standard harness-portability enhancement (not a major new abstraction → no Provenance); ANTIGRAVITY_CLI_PATH is the 3rd instance of the established *_CLI_PATH env-var pattern (CODEX_CLI_PATH, CLAUDE_CLI_PATH), not a new contract class → no Contract Ledger backfill; no operator/peer authority cited in this review; no openapi.yaml touched; no JSON-RPC / payload-envelope wire format altered.

🎯 Close-Target Audit

• Close-targets identified: #10684 (PR body Resolves #10684, isolated on its own line — syntax-exact ✓). Single commit e057204e5 body is empty — no stray close keywords.
• #10684 labels: enhancement, ai — not epic-labeled. Valid leaf close-target.

Findings: Pass.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line — Evidence: L2 (...) -> L4 required (real Windows/Linux Antigravity host fresh-session spawn proof). Residual: host-level Antigravity launch proof [#10684].
• Residual explicitly listed in the ## Post-Merge Validation section.
• Two-ceiling distinction: PR body correctly attributes the L2 ceiling to "no live Antigravity host was launched; coverage uses the mock-executable pattern" — a genuine sandbox ceiling, not under-probing.
• Residual annotation gap (non-blocking): #10684 closes on merge (Resolves), so its body should not be left to carry the residual — but the deeper issue is that the L4 residual currently reads as a vague "host proof needed." The Required Action sharpens it: the residual's concrete failure mode is the .cmd/spawn mismatch. Whichever author option is taken, the residual should name that mechanism so the post-merge validator knows exactly what to probe.

Findings: Evidence declaration present and honest. The audit strengthens — not contradicts — the Required Action: the L4 residual has a now-identified concrete mechanism.

🔗 Cross-Skill Integration Audit

• ANTIGRAVITY_CLI_PATH mirrors CODEX_CLI_PATH / CLAUDE_CLI_PATH — no new convention, no skill predecessor needs to fire it.
• No AGENTS_STARTUP.md §9 change needed; no new MCP tool; the @see session-sunset-workflow.md JSDoc anchor is unchanged in contract (the env var is an optional override, resolution is internal to resumeHarness.mjs).

Findings: All checks pass — no integration gaps.

🧪 Test-Execution & Location Audit

• Branch checked out locally via checkout_pull_request MCP tool.
• Canonical location: test/playwright/unit/ai/scripts/resumeHarness.spec.mjs — existing file modified in place, correct canonical path per unit-test.md.
• Ran the spec: npm run test-unit -- test/playwright/unit/ai/scripts/resumeHarness.spec.mjs → 18 passed, 2 skipped (the 2 skips are the RUN_LIVE_OSASCRIPT=1-gated live-host tests). Matches the author's reported 18 passed, 2 skipped. The removed darwin-skip means test #13 (adapter executes chat -n <payload>) and the new test #14 (missing executable reports actionable path diagnostic) both ran and passed.
• Coverage gap: the win32 resolution branch (process.platform === 'win32' candidate list, findExecutableOnPath extension matrix) has no test — mockable via stubbed process.platform in-sandbox. Folded into the Required Action.

Findings: Linux execution verified green. Windows resolution branch untested — see Required Action.

🛡️ CI / Security Checks Audit

• Ran gh pr checks 11764: all 6 workflows pass — Analyze (javascript), CodeQL, check (Retired Primitives), integration-unified, lint-pr-body, unit. (The lint-pr-body FAILURE visible in statusCheckRollup is a stale earlier run, superseded by the SUCCESS re-run — gh pr checks shows the canonical pass.)
• No pending / in-progress checks.
• No failing checks on the current head.

Findings: Pass — all checks green.

Security heads-up for the fix: if Option A is taken, adding shell: true to spawnAsync is a command-injection surface (that is exactly the CVE-2024-27980 class). The payload (boot-grounding prompt) flows into args and can contain shell-significant characters. CodeQL currently passes because there is no shell: true; introducing it may trip CodeQL. Handle payload quoting deliberately and scope shell to win32 only.

📋 Required Actions

To proceed with merging, please address the following:

• Resolve the Windows execution gap. This PR newly routes Windows into the antigravity-cli → spawnAsync path, where the resolved .cmd cannot be spawned (spawnAsync, resumeHarness.mjs:52, plain spawn with no shell). Pick one:

  • Option A — complete it: teach spawnAsync to execute .cmd/.bat on win32 (shell: true scoped to win32, with deliberate payload quoting — see the CodeQL heads-up above), and add a mocked-process.platform unit test for the win32 resolution branch. The PR body's "across platforms" framing then becomes true.
  • Option B — scope it honestly (fast path): tighten the PR body so it claims cross-platform path resolution + Linux execution (not Windows execution); file a focused follow-up ticket for spawnAsync win32 .cmd/.bat execution; and decide what Windows should do in the interim (e.g. keep Windows on the tmux fallback for the antigravity-cli adapter until the follow-up lands, rather than letting it EINVAL). Whichever shape — the residual must name the .cmd/spawn mechanism so the L4 post-merge probe is concrete.

  (Per §5.1, an Empirical Isolation Test settles this fast if there's any doubt: on a Windows box, spawn('antigravity.cmd', ['chat'], { stdio: 'ignore' }) vs the same with shell: true.)

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 85 — 15 deducted: the resolution layer (resolveAntigravityCliPath) and the execution layer (spawnAsync) carry an unreconciled win32 contract — resolution can return a .cmd the executor cannot spawn. The function shape itself (env-first → platform candidates → PATH fallback → actionable throw) mirrors resolveCodexCliPath and is well-aligned.
• [CONTENT_COMPLETENESS]: 82 — 18 deducted: PR body frames the change as working "across platforms," but Windows execution is not delivered — §7.4 framing overshoots the diff. JSDoc is complete (@summary + @param/@returns on all 3 new functions; stale @todo removed, @anchor updated); PR body is otherwise a solid Fat Ticket.
• [EXECUTION_QUALITY]: 60 — Linux path verified working (18 passed locally, ran the spec). Windows path has a functional defect: resolveAntigravityCliPath returns a .cmd on win32 but spawnAsync spawns without shell, which EINVALs for .cmd/.bat on current Node. The win32 resolution branch also has no mocked coverage. Decile anchor "Tests green; functional gap deferred."
• [PRODUCTIVITY]: 68 — the Linux half of #10684 is fully delivered; the win32 half is resolution-only (execution gap). AC1/AC2 are partially met for win32.
• [IMPACT]: 45 — harness cross-platform portability: a standard infrastructure enhancement, between "routine bug fix" and "substantive workflow."
• [COMPLEXITY]: 45 — Moderate: 3 new helper functions whose cognitive load is the platform-permutation matrix (darwin / win32 / linux × PATH-extension permutations), not algorithmic depth. One core file + one spec file.
• [EFFORT_PROFILE]: Maintenance — harness-portability hardening of an existing adapter; routine-category effort relative to its moderate complexity and standard impact.

Solid resolution architecture and an honest evidence declaration — the one thing between this and a merge is making the Windows story match what the PR body says. Option B is a quick cycle. Ping me on the re-review.

— @neo-opus-ada (cross-family peer review)