fix(parsers): validate Windows FFmpeg discovery candidates

[miguel-heygen]

What

Discover Windows FFmpeg and FFprobe binaries from native current-directory and PATH strings instead of decoding where.exe console output.

Why

where.exe writes bytes in the active Windows console code page. Decoding that output as UTF-8 can corrupt non-ASCII paths and pass a nonexistent mojibake path into process launch.

How

• Enumerate the same current-directory and PATH search space directly in Node on Windows.
• Preserve native Unicode JavaScript path strings end to end.
• Keep PATHEXT handling and prefer the real .exe over command shims.
• Retain which plus executable validation on Unix and explicit path overrides on every platform.

Test plan

• Unicode Windows current-directory regression without console output
• Windows .exe preference regression
• Parser focused suite: 8 passed
• Parser typecheck
• Changed-file lint, format, diff, and commit hooks

[vanceingalls]

Verdict: APPROVE — grade A — rubric CORRECT (on stated scope)

Root-cause fit: Fix is a targeted safety net, not an encoding repair. where.exe's codepage bytes still get decoded as UTF-8 at ffBinaries.ts:112 (unchanged), but the new guard at ffBinaries.ts:111-112 rejects the resulting mojibake candidate via existsSync/accessSync and falls through to scanPath() — which reads process.env.PATH via Node's UTF-16 env APIs, so it's clean. Honest with its PR body ("even when a valid FFmpeg is available through PATH") — does not attempt to recover ffmpeg installed in a cyrillic-only dir absent from PATH.

Claims verified (each with file:line at HEAD):

• "Check that an auto-discovered candidate exists and is executable" — ffBinaries.ts:111 wraps chooseBestPathCandidate in isExecutablePathCandidate (uses existsSync on win32 at line 41, accessSync X_OK elsewhere at line 43).
• "Fall back to the existing PATH scan when validation fails" — ffBinaries.ts:112 candidate && isExecutablePathCandidate(candidate) ? candidate : scanPath(name).
• "Preserve valid Windows .exe selection and explicit path overrides" — chooseBestPathCandidate untouched (lines 27-40); env-override branch untouched (lines 138-142).
• "Add coverage for corrupted Windows discovery output" — new test at ffBinaries.test.ts:73-99 mocks mojibake where output + a /valid-tools/ffmpeg.exe fallback and asserts PATH fallback.
• File-list/diff: 2 files, 36/0 + 2/1 = +38/-1.

Adversarial findings:

• P2 — Doesn't recover ffmpeg in cyrillic/Chinese install paths absent from PATH: still encoding: "utf-8" at ffBinaries.ts:112; no iconv-lite / chcp / codepage-aware decode. Ticket-body language is broader than this PR delivers; expect a follow-up.
• P2 — Only the single "best" candidate is validated: chooseBestPathCandidate returns one path; if a mojibake candidate wins the .exe preference but a clean second-line candidate exists in where's output, the clean one is discarded before validation. Low practical risk on Windows.
• nit — Silent fallback on rejected candidate: no log/warn distinguishes "where errored" from "where succeeded but returned a phantom path".
• nit — Test at ffBinaries.test.ts:96 asserts encoding: "utf-8" in the execFileSync call, locking in current lossy decode. Must move if a follow-up switches to codepage-aware decoding.
• Standards checklist: no new empty catches, no disabled tests, no TODOs, no oxlint breach.

CI state: All required checks green including Tests on windows-latest (12m45s) and Render on windows-latest (8m13s). MERGEABLE.

Suggested next step: Merge — the fix is minimal, correct for its stated scope, tested on Windows CI, and strictly reduces failure modes. Open a follow-up ticket to actually recover non-ASCII paths (spawn where with encoding: "buffer" and decode via iconv-lite using the active codepage from chcp, or scan the registry/Program Files directly). Verify separately whether APPS-984's reporter expects the cyrillic-recovery case; if so, keep the ticket open.

— Review by Via

[james-russo-rames-d-jusso]

Reviewed at 4c3ec29a.

Clean fix. isExecutablePathCandidate gates the where/which output before returning, so a mojibake .exe path decoded from Windows CP output can no longer reach process launch when a valid FFmpeg is on PATH. Windows check is existsSync only (correct — no X_OK model on NTFS); Unix uses accessSync(candidate, X_OK) in a try/catch. Regression test asserts the fallback to scanPath when the decoded candidate doesn't exist, and the existing .exe > .cmd selection still passes because both the .exe mock and existsSync mock agree on the same path. LGTM from my side.

Observations

• If where returns multiple candidates and only the highest-priority one is corrupt, the fix drops the entire where list and re-scans PATH from scratch — a valid .cmd in the where output is discarded even when it would resolve. In practice scanPath re-derives the same valid path from PATH env (which Node reads UTF-8-clean), so this is defensible; documenting the "on validation miss, we prefer PATH scan over iterating remaining where candidates" choice in the comment would help future maintainers.
• The corrupted-output test uses /valid-tools under mocked process.platform="win32". Not a bug — path.delimiter on win32 with a single-entry PATH still resolves — but a real Windows-shaped path (C:\\valid-tools) matches the failure geometry better. Cosmetic.

Nits

• Object.defineProperty(process, "platform", { value: "win32", configurable: true }) pollutes global state across the whole test file. Existing tests already do this, but a fixture that stashes the original and restores in afterEach would prevent surprise ordering hazards if someone later reshuffles the describe blocks.

What I didn't verify

• Cache-poisoning safety: pathLookupCache is module-scoped and populated with the resolved path on first success. If the first call happens with a corrupted where output that falls back to a valid scanPath result, that valid result gets cached — good. But if the first call happens while PATH is unset and where also fails, undefined is cached and all future calls in the process return undefined even after PATH is set. Pre-existing behavior, not touched by this PR — worth flagging separately if it hasn't been.

— Review by Rames D Jusso

[miguel-heygen]

Review follow-up at exact head 59df9d5e2:

• Removed the Windows console-decoding dependency instead of trying to recover mojibake after where.exe.
• Windows discovery now enumerates native current-directory and PATH strings directly, preserving Unicode and covering the clean-candidate sibling case without decoding stdout.
• PATHEXT handling and real-.exe preference remain intact; Unix keeps which plus executable validation.

Verification: focused parser suite 8/8, parser typecheck, changed-file lint/format, diff check, and commit hooks.