From ac1b6d679f3d2cdbf25c04ee56fe4c6c7a821fbc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 18 Aug 2026 15:32:57 -0400 Subject: [PATCH] enhance(#3618): fold fallow-runner onto the canonical binary resolver (epic #3411 Phase 2) (#3633) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(#3618): fold fallow-runner onto the canonical binary resolver Epic #3411 Phase 2. src/fallow-runner.cts was the fourth divergent implementation of Windows binary resolution the epic enumerated — candidateNames, isExecutableFile, findInPath, findInNodeModules, 40 lines. All four are deleted; resolveFallowBinary is one seam call. Two OPT-IN options were added to resolveExecutableBinary to make the fold behavior-preserving, both defaulting off so Phase 1's callers are byte-identical: prependPaths dirs searched before env.PATH, in order, through the identical per-directory candidate logic. This expresses node_modules/.bin-first precedence without env surgery — the rejected alternative re-introduced the spread-loses-the-proxy hazard the Windows lane caught in Phase 1, at every future call site instead of once. requireExecutable POSIX-only accessSync(X_OK); a no-op on win32 where mode bits do not mean execute. Opt-in rather than default because unconditional X_OK breaks #3445's suite, which stages candidates with plain writeFileSync and never sets an exec bit — the repo bans chmod in tests — so every one would resolve to null on POSIX. Deliberate behavior change on Windows: fallow's prior candidate list ended in a BARE fallow. The seam never tries a bare name there, so an extensionless file beside fallow.cmd is no longer resolved. That is the fix, not a regression — the extensionless file is npm's POSIX sh shim, which CreateProcess cannot run (#3275). Rows 7 and 8 of the design record it. Defect found while working, fixed inline: the resolution order was documented BACKWARDS as PATH-then-.bin in structural-pre-pass.md, docs/INVENTORY.md and four INVENTORY translations. The code has always been .bin first, and .bin first is correct — a project-local tool should beat a global one. The archived changeset is left alone as a historical record. fallow-runner had no test file at all. tests/fallow-runner.test.cjs is new (F1-F15) and the seam options are pinned by S1-S12 folded into the existing dispatch suite. RED proven by execution: with both source files stashed and build:lib re-run, 7 of 27 probe cases failed. Refs #3411 * chore(#3618): backfill changeset pr number 3633 * fix(#3618): assert both platform contracts in F4 instead of a POSIX-only premise Windows CI on #3633 failed F4. The test monkeypatched accessSync to throw and asserted resolveFallowBinary returned null — but that premise, that the X_OK check is consulted at all, is POSIX-only by design. requireExecutable is a deliberate no-op on win32 because Windows mode bits do not mean execute, so the staged fixture correctly resolved there. 40-design.md's negative-space section already states this carve-out verbatim. The test contradicted the design it was written from: fixtures were made platform-adaptive in the previous commit, and this assertion was left platform-blind. F4 now asserts BOTH contracts — null on POSIX, resolves on win32 — rather than skipping either. A t.skip on one lane would have been green and would have left the win32 carve-out unpinned by fallow's own entry point. Audited every other row for the same class. F1-F3, F5, F6, F11-F15 hold on both platforms; F7-F10 and S1-S12 inject platform explicitly and are unaffected. F4 was the only row with a single-platform premise. The local probe runs on one platform and structurally cannot catch this, which is why it was green — that limitation is now stated at the top of the probe so a green probe is not mistaken for platform coverage. The win32 branch was proven by injecting platform:'win32' with accessSync throwing and asserting it still resolves. Refs #3411 --------- Co-authored-by: sim --- .changeset/eager-pandas-glide.md | 5 + CONTEXT.md | 2 +- docs/INVENTORY.md | 2 +- docs/ja-JP/INVENTORY.md | 2 +- docs/ko-KR/INVENTORY.md | 2 +- docs/pt-BR/INVENTORY.md | 2 +- docs/zh-CN/INVENTORY.md | 2 +- .../code-review/steps/structural-pre-pass.md | 2 +- src/fallow-runner.cts | 60 +--- src/shell-command-projection.cts | 43 ++- tests/fallow-runner.test.cjs | 329 ++++++++++++++++++ ...shell-command-projection-dispatch.test.cjs | 222 ++++++++++++ 12 files changed, 614 insertions(+), 59 deletions(-) create mode 100644 .changeset/eager-pandas-glide.md create mode 100644 tests/fallow-runner.test.cjs diff --git a/.changeset/eager-pandas-glide.md b/.changeset/eager-pandas-glide.md new file mode 100644 index 000000000..8ccd7a027 --- /dev/null +++ b/.changeset/eager-pandas-glide.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3633 +--- +**Fallow binary resolution now shares the platform seam** — resolving the fallow binary uses the same PATH/PATHEXT logic as every other spawn, so on Windows a `fallow.cmd` shim resolves correctly and an extensionless npm shim is no longer picked up in its place. `node_modules/.bin` is still searched before `PATH`, and the POSIX executable-bit check is unchanged. (#3618) diff --git a/CONTEXT.md b/CONTEXT.md index b4364ab6d..6211f4a60 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -825,7 +825,7 @@ The prompt-level data/instruction isolation seam for untrusted web/document ingr ## Shell Command Projection Module (expanded glossary entry, 2026-05-13) -Module owning all OS-facing I/O for the tool: runtime-aware command-text rendering (hook commands, PATH action lines, shim scripts), subprocess dispatch (execGit, execNpm, execTool, probeTty, isSpawnTimeout), Windows binary resolution (resolveExecutableBinary, projectSpawnInvocation), and platform file I/O (platformWriteSync, platformReadSync, platformEnsureDir). Single seam for platform-conditional logic — one place to fix any shell or file write regression across Windows, macOS, and Linux. WINDOWS BINARY RESOLUTION — which file a declared command name actually names, and what must be handed to `spawnSync` to start it — is owned here as of #3411 (epic #3411 Phase 1), which found the seam declaration untrue for this axis: four divergent implementations had grown outside it (`execNpm`'s `shell:true`, `execTool`'s absence of any handling, a private PATH+PATHEXT scan in `gsd-core/bin/gsd-tools.cjs`, and a fourth candidate-extension array in `fallow-runner.cts`), and #3275's fix to one provably never reached the others. `resolveExecutableBinary(name, {platform, env})` returns the RESOLVED PATH (the prior `hasBinary` computed it and threw it away): on win32 it tries PATHEXT entries ONLY and never the bare name, because npm global installs drop an extensionless POSIX `sh` shim beside `foo.CMD` and a bare-name-first scan resolves to it, leaving the ENOENT unchanged (#3275); a name already carrying a PATHEXT-listed extension is tried as-is BEFORE the append loop, and a suffix outside PATHEXT is not an extension. On POSIX it answers existence only, and `execTool` does not consult it there at all — the bare name goes to `spawnSync` unchanged and Node's own PATH search does the work, keeping macOS/Linux byte-identical for a symbol whose blast radius is CRITICAL (167 affected symbols, 53 files). `projectSpawnInvocation(command, args, {platform, env})` is the inseparable second half: `CreateProcess` cannot execute a `.cmd`/`.bat` at all, so resolution alone does not fix Windows, and exporting only the resolver would leave every caller to re-derive the mediation — which is how the four copies accumulated. It mediates through `ComSpec` with an EXPLICIT argv array (`/d /s /c`), never `shell: true`: that is CVE-2024-27980's argument-injection vector and Node 26's DEP0190. Mediation keys on the target — the resolved path, or the declared name when resolution found nothing — so a `.cmd` resident in the current directory (which PATH-only resolution misses but `cmd.exe /c` still finds) keeps working, while a BARE unresolved name is passed through untouched so the spawn fails with ENOENT rather than cmd.exe's exit 9009, preserving `_spawnResult`'s `': not found'` contract. `gsd-core/bin/gsd-tools.cjs`'s `resolveSpawnBinary` is the `bin/` entry point onto this and holds no copy of the logic. CALLERS CHOOSE HOW MUCH OF THE PROJECTION TO ADOPT, and the asymmetry is deliberate: `windowsVerbatimArguments` marks the cases where mediation was REQUIRED (the caller must take `command` and `args` together, or the batch file cannot start at all), whereas a merely-RESOLVED path is an offer a caller may decline. `execTool` declines it and keeps spawning the DECLARED name — libuv's `CreateProcess` path already performs PATH+PATHEXT search, so resolving a `.exe` there buys nothing while changing what 167 dependents observe being spawned; `tests/graphify.test.cjs` pins that contract by spying on `spawnSync`'s first argument, and the Windows CI lane caught the violation when `execTool` briefly adopted the resolved path (`python3` arriving as `C:\…\python3.EXE`). `deps.spawn` accepts it, because that lane's `hasBinary` probe answers from the same resolver and probe and spawn must agree on the exact file (#3445). Env lookups go through a case-insensitive read: Windows names the variable `Path`, `process.env` is a case-insensitive proxy that hides this, and `execTool`'s `{...process.env, ...opts.env}` spread produces a PLAIN object that keeps the OS casing and loses the proxy — an exact-case `env['PATH']` there returns undefined and the scan silently sees nothing. Known gap at Phase 1: `src/fallow-runner.cts` still carries its own resolver (Phase 2), and no lint ratchet yet rejects a bare-binary spawn outside this seam (Phase 3). `isSpawnTimeout` is the single shared "did this subprocess time out" predicate (error.code==='ETIMEDOUT' only — cross-platform-safe; does not require signal==='SIGTERM'), consumed by worktree-safety.cts, worktree-base-ref.cts, commands.cts, and this module's own dispatchGsdCommand (#3050 — "Generative Fix Divergence"). `projectPathExportLine(targetDir)` is the single source of the `export PATH=":$PATH"` line for all three PATH-persistence lanes (repair, persist, win32 Git Bash) — it double-quote-escapes for the line's final rc-file context before any lane single-quotes it for its own `echo` transport, closing the #3118 command-substitution injection where a lane re-escaped the line itself and let a `$(…)` in the target dir execute on every new shell; the win32 cmd.exe lane fails closed (empty actions) whenever the target dir contains `"`, since that character is reserved on Windows and would otherwise close cmd's quoted region, and now tags that empty result with a typed `PATH_ACTION_REASON` (`win32_reserved_quote`) so it stays distinguishable from the unrelated "no target directory given" empty result (`no_target_dir`, #3118); the fish lane emits `fish_add_path -- ''` (`--` end-of-options separator, verified empirically against fish 4.8.1), since a leading-dash target dir is otherwise misparsed by fish's argparse-based option scanning regardless of quoting; `escapeTomlDoubleQuotedString` now escapes TOML's required control characters (U+0000-U+0008, U+000A-U+001F, U+007F), not just backslash and quote, closing a raw-newline/CR/NUL config.toml parse failure (#3118). Lives in `gsd-core/bin/lib/shell-command-projection.cjs`. See ADR-0009 (superseded "does not execute" constraint) and ADR-0010 (superseded File Operation Engine). +Module owning all OS-facing I/O for the tool: runtime-aware command-text rendering (hook commands, PATH action lines, shim scripts), subprocess dispatch (execGit, execNpm, execTool, probeTty, isSpawnTimeout), Windows binary resolution (resolveExecutableBinary, projectSpawnInvocation), and platform file I/O (platformWriteSync, platformReadSync, platformEnsureDir). Single seam for platform-conditional logic — one place to fix any shell or file write regression across Windows, macOS, and Linux. WINDOWS BINARY RESOLUTION — which file a declared command name actually names, and what must be handed to `spawnSync` to start it — is owned here as of #3411 (epic #3411 Phase 1), which found the seam declaration untrue for this axis: four divergent implementations had grown outside it (`execNpm`'s `shell:true`, `execTool`'s absence of any handling, a private PATH+PATHEXT scan in `gsd-core/bin/gsd-tools.cjs`, and a fourth candidate-extension array in `fallow-runner.cts`), and #3275's fix to one provably never reached the others. `resolveExecutableBinary(name, {platform, env})` returns the RESOLVED PATH (the prior `hasBinary` computed it and threw it away): on win32 it tries PATHEXT entries ONLY and never the bare name, because npm global installs drop an extensionless POSIX `sh` shim beside `foo.CMD` and a bare-name-first scan resolves to it, leaving the ENOENT unchanged (#3275); a name already carrying a PATHEXT-listed extension is tried as-is BEFORE the append loop, and a suffix outside PATHEXT is not an extension. On POSIX it answers existence only, and `execTool` does not consult it there at all — the bare name goes to `spawnSync` unchanged and Node's own PATH search does the work, keeping macOS/Linux byte-identical for a symbol whose blast radius is CRITICAL (167 affected symbols, 53 files). `projectSpawnInvocation(command, args, {platform, env})` is the inseparable second half: `CreateProcess` cannot execute a `.cmd`/`.bat` at all, so resolution alone does not fix Windows, and exporting only the resolver would leave every caller to re-derive the mediation — which is how the four copies accumulated. It mediates through `ComSpec` with an EXPLICIT argv array (`/d /s /c`), never `shell: true`: that is CVE-2024-27980's argument-injection vector and Node 26's DEP0190. Mediation keys on the target — the resolved path, or the declared name when resolution found nothing — so a `.cmd` resident in the current directory (which PATH-only resolution misses but `cmd.exe /c` still finds) keeps working, while a BARE unresolved name is passed through untouched so the spawn fails with ENOENT rather than cmd.exe's exit 9009, preserving `_spawnResult`'s `': not found'` contract. `gsd-core/bin/gsd-tools.cjs`'s `resolveSpawnBinary` is the `bin/` entry point onto this and holds no copy of the logic. CALLERS CHOOSE HOW MUCH OF THE PROJECTION TO ADOPT, and the asymmetry is deliberate: `windowsVerbatimArguments` marks the cases where mediation was REQUIRED (the caller must take `command` and `args` together, or the batch file cannot start at all), whereas a merely-RESOLVED path is an offer a caller may decline. `execTool` declines it and keeps spawning the DECLARED name — libuv's `CreateProcess` path already performs PATH+PATHEXT search, so resolving a `.exe` there buys nothing while changing what 167 dependents observe being spawned; `tests/graphify.test.cjs` pins that contract by spying on `spawnSync`'s first argument, and the Windows CI lane caught the violation when `execTool` briefly adopted the resolved path (`python3` arriving as `C:\…\python3.EXE`). `deps.spawn` accepts it, because that lane's `hasBinary` probe answers from the same resolver and probe and spawn must agree on the exact file (#3445). Env lookups go through a case-insensitive read: Windows names the variable `Path`, `process.env` is a case-insensitive proxy that hides this, and `execTool`'s `{...process.env, ...opts.env}` spread produces a PLAIN object that keeps the OS casing and loses the proxy — an exact-case `env['PATH']` there returns undefined and the scan silently sees nothing. `resolveExecutableBinary` carries two OPT-IN options, both defaulting off so Phase 1's callers are byte-identical: `prependPaths` (directories searched before `env.PATH`, in order, with the identical per-directory candidate logic — this is how `node_modules/.bin`-first precedence is expressed without env surgery) and `requireExecutable` (POSIX-only additional `accessSync(X_OK)`; a no-op on win32, where mode bits do not mean execute). `requireExecutable` is opt-in rather than default because making it unconditional would break #3445's own suite, which stages candidates with plain `writeFileSync` and never sets an exec bit — the repo bans `chmod` in tests — so every one of those would resolve to null on POSIX. As of #3618 (epic #3411 Phase 2) `src/fallow-runner.cts` holds no resolver of its own: `resolveFallowBinary` is one seam call passing `prependPaths: [/node_modules/.bin]` and `requireExecutable: true`, preserving `.bin`-before-PATH precedence and the POSIX executability check. Its prior win32 candidate list ended in a BARE `fallow`; the seam does not, and dropping it is the fix — an extensionless file beside `fallow.cmd` is npm's POSIX `sh` shim that `CreateProcess` cannot run (#3275). Note the precedence was documented BACKWARDS (`PATH` then `.bin`) in `structural-pre-pass.md` and four INVENTORY translations until #3618 corrected them; the code was always `.bin` first. Known gap after Phase 2: no lint ratchet yet rejects a bare-binary spawn outside this seam (Phase 3, #3619). `isSpawnTimeout` is the single shared "did this subprocess time out" predicate (error.code==='ETIMEDOUT' only — cross-platform-safe; does not require signal==='SIGTERM'), consumed by worktree-safety.cts, worktree-base-ref.cts, commands.cts, and this module's own dispatchGsdCommand (#3050 — "Generative Fix Divergence"). `projectPathExportLine(targetDir)` is the single source of the `export PATH=":$PATH"` line for all three PATH-persistence lanes (repair, persist, win32 Git Bash) — it double-quote-escapes for the line's final rc-file context before any lane single-quotes it for its own `echo` transport, closing the #3118 command-substitution injection where a lane re-escaped the line itself and let a `$(…)` in the target dir execute on every new shell; the win32 cmd.exe lane fails closed (empty actions) whenever the target dir contains `"`, since that character is reserved on Windows and would otherwise close cmd's quoted region, and now tags that empty result with a typed `PATH_ACTION_REASON` (`win32_reserved_quote`) so it stays distinguishable from the unrelated "no target directory given" empty result (`no_target_dir`, #3118); the fish lane emits `fish_add_path -- ''` (`--` end-of-options separator, verified empirically against fish 4.8.1), since a leading-dash target dir is otherwise misparsed by fish's argparse-based option scanning regardless of quoting; `escapeTomlDoubleQuotedString` now escapes TOML's required control characters (U+0000-U+0008, U+000A-U+001F, U+007F), not just backslash and quote, closing a raw-newline/CR/NUL config.toml parse failure (#3118). Lives in `gsd-core/bin/lib/shell-command-projection.cjs`. See ADR-0009 (superseded "does not execute" constraint) and ADR-0010 (superseded File Operation Engine). Invariants: - Result shape: all exec* functions return `{ exitCode, stdout, stderr }`; never throw on non-zero exit code. diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 3200de86d..a45d0f7bb 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -499,7 +499,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `eval.cjs` | Deterministic eval scoring (compiled from `src/eval.cts`, gitignored) — `computeEvalScore` (coverage*0.6 + infra*0.4, bands 80/60/40) + `cmdEvalScore` CLI domain guard; moves the gsd-eval-auditor's weighted arithmetic out of the prompt into code (#10 / #1579) | | `estimate-cli.cjs` | I/O seam over `phase-estimation.cjs` — the `estimate-check` and `estimate-calibration` query verbs; reads the `workflow.smart_zone_tokens` budget and `.planning/estimation-calibration.json`, both degrading to defaults rather than failing planning (#2630) | | `observability/event.cjs` | DispatchEvent shape factory for every Hub dispatch — traceId/parentTraceId/command/result/timestamp record consumed by DispatchLogger (#177, ADR-0174 P1.3/P1.4) | -| `fallow-runner.cjs` | Fallow audit adapter for `/gsd-code-review`: binary resolution (`PATH` then `node_modules/.bin`), actionable missing-binary errors, and structural findings normalization | +| `fallow-runner.cjs` | Fallow audit adapter for `/gsd-code-review`: binary resolution (`node_modules/.bin` then `PATH`), actionable missing-binary errors, and structural findings normalization | | `federated-config.cjs` | Defensive merge of capability-declared config slices into the loadConfig return value — ADR-857 phase 3b; exports `mergeFederatedConfig({ configSchema, isCentralKey, userConfig })` → `{ values, validKeys, warnings }`; live for migrated Capability keys that are atomically removed from the central config schema | | `frontmatter.cjs` | YAML frontmatter CRUD operations | | `gap-checker.cjs` | Post-planning gap analysis (#2493): unified REQUIREMENTS.md + CONTEXT.md decisions vs PLAN.md coverage report (`gsd-tools gap-analysis`) | diff --git a/docs/ja-JP/INVENTORY.md b/docs/ja-JP/INVENTORY.md index 6ea9cfc38..75561b862 100644 --- a/docs/ja-JP/INVENTORY.md +++ b/docs/ja-JP/INVENTORY.md @@ -395,7 +395,7 @@ | `decisions.cjs` | CONTEXT.md の `` ブロックを解析。数値（D-42）と英数字（D-INFRA-01）の ID を受け付け。`{id, text, category, tags, trackable}` を返す | | `docs.cjs` | docs-update ワークフロー初期化、Markdown スキャン、モノリポ検出 | | `drift.cjs` | 実行後のコードベース構造ドリフト検出器（#2003）: ファイル変更を new-dir/barrel/migration/route カテゴリに分類し、`last_mapped_commit` フロントマターをラウンドトリップ | -| `fallow-runner.cjs` | `/gsd-code-review` 向けのファロー監査アダプター: バイナリ解決（`PATH` 次に `node_modules/.bin`）、アクション可能なバイナリ欠落エラー、構造的な調査結果の正規化 | +| `fallow-runner.cjs` | `/gsd-code-review` 向けのファロー監査アダプター: バイナリ解決（`node_modules/.bin` 次に `PATH`）、アクション可能なバイナリ欠落エラー、構造的な調査結果の正規化 | | `frontmatter.cjs` | YAML フロントマター CRUD 操作 | | `gap-checker.cjs` | 計画後のギャップ分析（#2493）: REQUIREMENTS.md + CONTEXT.md 決定事項 vs PLAN.md カバレッジレポート（`gsd-tools gap-analysis`）の統合 | | `graphify.cjs` | `/gsd-graphify` 向けのナレッジグラフビルド/クエリ/ステータス/差分 | diff --git a/docs/ko-KR/INVENTORY.md b/docs/ko-KR/INVENTORY.md index 11bf8651a..80369d828 100644 --- a/docs/ko-KR/INVENTORY.md +++ b/docs/ko-KR/INVENTORY.md @@ -395,7 +395,7 @@ | `decisions.cjs` | CONTEXT.md `` 블록 파싱; 숫자형(D-42) 및 영숫자형(D-INFRA-01) ID 허용; `{id, text, category, tags, trackable}` 반환 | | `docs.cjs` | 문서 업데이트 워크플로우 초기화, 마크다운 스캔, 모노레포 감지 | | `drift.cjs` | 실행 후 코드베이스 구조 드리프트 감지기(#2003): 파일 변경을 new-dir/barrel/migration/route 카테고리로 분류하고 `last_mapped_commit` 프론트매터를 왕복 처리 | -| `fallow-runner.cjs` | `/gsd-code-review`를 위한 Fallow 감사 어댑터: 바이너리 해석(`PATH` 이후 `node_modules/.bin`), 실행 가능한 누락 바이너리 오류, 구조적 결과 정규화 | +| `fallow-runner.cjs` | `/gsd-code-review`를 위한 Fallow 감사 어댑터: 바이너리 해석(`node_modules/.bin` 이후 `PATH`), 실행 가능한 누락 바이너리 오류, 구조적 결과 정규화 | | `frontmatter.cjs` | YAML 프론트매터 CRUD 작업 | | `gap-checker.cjs` | 계획 후 공백 분석(#2493): REQUIREMENTS.md + CONTEXT.md 결정 대 PLAN.md 커버리지 보고서(`gsd-tools gap-analysis`) | | `graphify.cjs` | `/gsd-graphify`를 위한 지식 그래프 빌드/쿼리/상태/비교 | diff --git a/docs/pt-BR/INVENTORY.md b/docs/pt-BR/INVENTORY.md index 3baefd60d..00e3e5bbd 100644 --- a/docs/pt-BR/INVENTORY.md +++ b/docs/pt-BR/INVENTORY.md @@ -395,7 +395,7 @@ Listagem completa: `gsd-core/bin/lib/*.cjs`. | `decisions.cjs` | Analisa blocos `` do CONTEXT.md; aceita IDs numéricos (D-42) e alfanuméricos (D-INFRA-01); retorna `{id, text, category, tags, trackable}` | | `docs.cjs` | Inicialização do workflow docs-update, varredura de Markdown, detecção de monorepo | | `drift.cjs` | Detector de drift estrutural pós-execução da base de código (#2003): classifica alterações de arquivo em categorias new-dir/barrel/migration/route e faz round-trip do frontmatter `last_mapped_commit` | -| `fallow-runner.cjs` | Adaptador de auditoria fallow para `/gsd-code-review`: resolução binária (`PATH` depois `node_modules/.bin`), erros acionáveis de binário ausente e normalização de descobertas estruturais | +| `fallow-runner.cjs` | Adaptador de auditoria fallow para `/gsd-code-review`: resolução binária (`node_modules/.bin` depois `PATH`), erros acionáveis de binário ausente e normalização de descobertas estruturais | | `frontmatter.cjs` | Operações CRUD de frontmatter YAML | | `gap-checker.cjs` | Análise de lacunas pós-planejamento (#2493): relatório unificado de cobertura de decisões do REQUIREMENTS.md + CONTEXT.md vs PLAN.md (`gsd-tools gap-analysis`) | | `graphify.cjs` | Build/consulta/status/diff do grafo de conhecimento para `/gsd-graphify` | diff --git a/docs/zh-CN/INVENTORY.md b/docs/zh-CN/INVENTORY.md index 1c020396a..f76c490fe 100644 --- a/docs/zh-CN/INVENTORY.md +++ b/docs/zh-CN/INVENTORY.md @@ -395,7 +395,7 @@ | `decisions.cjs` | 解析 CONTEXT.md `` 块；接受数字（D-42）和字母数字（D-INFRA-01）ID；返回 `{id, text, category, tags, trackable}` | | `docs.cjs` | 文档更新工作流初始化、Markdown 扫描、单体仓库检测 | | `drift.cjs` | 执行后代码库结构漂移检测器（#2003）：将文件更改分类为新目录/桶/迁移/路由类别，并循环处理 `last_mapped_commit` frontmatter | -| `fallow-runner.cjs` | `/gsd-code-review` 的 fallow 审计适配器：二进制解析（`PATH` 然后 `node_modules/.bin`）、可操作的缺少二进制错误和结构性发现规范化 | +| `fallow-runner.cjs` | `/gsd-code-review` 的 fallow 审计适配器：二进制解析（`node_modules/.bin` 然后 `PATH`）、可操作的缺少二进制错误和结构性发现规范化 | | `frontmatter.cjs` | YAML frontmatter 增删改查操作 | | `gap-checker.cjs` | 规划后间隙分析（#2493）：REQUIREMENTS.md + CONTEXT.md 决策 vs PLAN.md 覆盖率报告（`gsd-tools gap-analysis`） | | `graphify.cjs` | `/gsd-graphify` 的知识图谱构建/查询/状态/差异 | diff --git a/gsd-core/workflows/code-review/steps/structural-pre-pass.md b/gsd-core/workflows/code-review/steps/structural-pre-pass.md index b3aa2b9e0..87f3e6cf1 100644 --- a/gsd-core/workflows/code-review/steps/structural-pre-pass.md +++ b/gsd-core/workflows/code-review/steps/structural-pre-pass.md @@ -1,6 +1,6 @@ When `FALLOW_ENABLED=true`: -1) Resolve binary via PATH first, then `node_modules/.bin/fallow`. +1) Resolve binary via `node_modules/.bin/fallow` first, then PATH. ```bash FALLOW_BIN=$(FALLOW_CWD="$(pwd)" node -e " const { resolveFallowBinary } = require('./gsd-core/bin/lib/fallow-runner.cjs'); diff --git a/src/fallow-runner.cts b/src/fallow-runner.cts index 42833141e..da8bfea45 100644 --- a/src/fallow-runner.cts +++ b/src/fallow-runner.cts @@ -7,51 +7,18 @@ * * Parses the real fallow `audit --format json` schema (schema_version 3 * envelope, nested dead_code/duplication sections). See fallow 2.70.0+. + * + * #3411 Phase 2 (#3618): binary resolution no longer lives here — it delegates + * to the platform seam's `resolveExecutableBinary` (shell-command-projection.cts). + * This file's prior private resolver deliberately included an extensionless + * `fallow` as a win32 candidate; the seam does not, and that is a fix, not a + * regression — an extensionless file sitting beside `fallow.cmd` is npm's POSIX + * `sh` shim, which `CreateProcess` cannot run (#3275). */ import fs from 'node:fs'; import path from 'node:path'; - -function candidateNames(): string[] { - return process.platform === 'win32' - ? ['fallow.exe', 'fallow.cmd', 'fallow.bat', 'fallow'] - : ['fallow']; -} - -function isExecutableFile(filePath: string): boolean { - try { - const stat = fs.statSync(filePath); - if (!stat.isFile()) return false; - if (process.platform === 'win32') return true; - fs.accessSync(filePath, fs.constants.X_OK); - return true; - } catch { - return false; - } -} - -function findInPath(envPath: string | undefined): string | null { - if (!envPath) return null; - const names = candidateNames(); - const segments = envPath.split(path.delimiter).filter(Boolean); - for (const segment of segments) { - for (const name of names) { - const candidate = path.join(segment, name); - if (isExecutableFile(candidate)) return candidate; - } - } - return null; -} - -function findInNodeModules(cwd: string): string | null { - const names = candidateNames(); - const binDir = path.join(cwd, 'node_modules', '.bin'); - for (const name of names) { - const candidate = path.join(binDir, name); - if (isExecutableFile(candidate)) return candidate; - } - return null; -} +import { resolveExecutableBinary } from './shell-command-projection.cjs'; export interface ResolveFallowOpts { cwd: string; @@ -59,7 +26,16 @@ export interface ResolveFallowOpts { } export function resolveFallowBinary({ cwd, envPath = process.env['PATH'] ?? '' }: ResolveFallowOpts): string | null { - return findInNodeModules(cwd) || findInPath(envPath) || null; + return resolveExecutableBinary('fallow', { + prependPaths: [path.join(cwd, 'node_modules', '.bin')], + // PATHEXT is passed through (not spread from the rest of process.env) so + // win32 resolution still works, while avoiding the spread-loses-the- + // case-insensitive-proxy hazard the Windows lane caught in Phase 1. `?? ''` + // is intentional: the seam falls back to its own DEFAULT_PATHEXT when this + // is empty/absent, so an empty string here is a safe "let the seam decide". + env: { PATH: envPath, PATHEXT: process.env['PATHEXT'] ?? '' }, + requireExecutable: true, + }); } export function requireFallowBinary({ cwd, envPath = process.env['PATH'] ?? '' }: ResolveFallowOpts): string { diff --git a/src/shell-command-projection.cts b/src/shell-command-projection.cts index 3dd1feb87..b7bba2ccb 100644 --- a/src/shell-command-projection.cts +++ b/src/shell-command-projection.cts @@ -653,11 +653,14 @@ const DEFAULT_PATHEXT = '.EXE;.CMD;.BAT;.COM'; /** Windows extensions that must be mediated through cmd.exe rather than spawned. */ const CMD_MEDIATED_EXT = /\.(cmd|bat)$/i; -function _isFile(candidate: string): boolean { +function _isFile(candidate: string, requireExecutable = false, platform: string = process.platform): boolean { try { - return fs.statSync(candidate).isFile(); + if (!fs.statSync(candidate).isFile()) return false; + if (requireExecutable && platform !== 'win32') fs.accessSync(candidate, fs.constants.X_OK); + return true; } catch { - // Missing, unreadable (EACCES), or a broken link — all mean "not this one". + // Missing, unreadable (EACCES), a broken link, or (when requireExecutable + // is set) not executable — all mean "not this one". return false; } } @@ -733,25 +736,45 @@ function _buildVerbatimCmdLine(target: string, args: string[]): string { * Path-like names (any `/` or `\`) bypass the PATH scan: the name is already an * address, so it passes through when it names an existing file. * + * **`opts.prependPaths`** — directories searched BEFORE `env.PATH`, in array + * order (e.g. a project-local `node_modules/.bin`). Defaults to `[]`, so + * Phase 1's callers (`execTool`, and `gsd-tools.cjs`'s `resolveSpawnBinary` / + * `deps.spawn` / `hasBinary`), which set neither new option, are byte-identical + * to today. The existing per-directory candidate logic (win32 as-is-then-append- + * PATHEXT; POSIX bare name) applies to prepended directories exactly as it does + * to `PATH` segments — there is no special-casing. + * + * **`opts.requireExecutable`** — when `true` and the platform is not `win32`, + * a candidate must additionally pass `fs.accessSync(candidate, fs.constants.X_OK)` + * to count as a match. On `win32` this is a no-op (mode bits do not mean + * execute on Windows — the same carve-out `fallow-runner`'s prior private + * resolver already had). Defaults to `false`, so `accessSync` is never called + * unless a caller opts in. It is opt-in rather than the default because making + * `X_OK` unconditional would break #3445's suite: those tests stage candidates + * with plain `fs.writeFileSync` and never set an exec bit (the repo bans + * `chmod` in tests), so every one of them would resolve to `null` on POSIX. + * * @returns the resolved path, or `null` when nothing matched. Callers fall back to * the declared name on `null` so a genuine ENOENT still surfaces (#3086). */ export function resolveExecutableBinary( name: string | null | undefined, - opts: { platform?: string; env?: NodeJS.ProcessEnv } = {}, + opts: { platform?: string; env?: NodeJS.ProcessEnv; prependPaths?: string[]; requireExecutable?: boolean } = {}, ): string | null { if (!name) return null; + const requireExecutable = opts.requireExecutable ?? false; + const platform = opts.platform ?? process.platform; if (name.includes('/') || name.includes('\\')) { - return _isFile(name) ? name : null; + return _isFile(name, requireExecutable, platform) ? name : null; } const env = opts.env ?? process.env; - const platform = opts.platform ?? process.platform; - const segments = String(_envGet(env, 'PATH') || '').split(path.delimiter).filter(Boolean); + const pathSegments = String(_envGet(env, 'PATH') || '').split(path.delimiter).filter(Boolean); + const segments = [...(opts.prependPaths ?? []), ...pathSegments]; if (platform !== 'win32') { for (const dir of segments) { const candidate = path.join(dir, name); - if (_isFile(candidate)) return candidate; + if (_isFile(candidate, requireExecutable, platform)) return candidate; } return null; } @@ -766,11 +789,11 @@ export function resolveExecutableBinary( for (const dir of segments) { if (carriesKnownExt) { const asIs = path.join(dir, name); - if (_isFile(asIs)) return asIs; + if (_isFile(asIs, requireExecutable, platform)) return asIs; } for (const ext of exts) { const candidate = path.join(dir, name + ext); - if (_isFile(candidate)) return candidate; + if (_isFile(candidate, requireExecutable, platform)) return candidate; } } return null; diff --git a/tests/fallow-runner.test.cjs b/tests/fallow-runner.test.cjs new file mode 100644 index 000000000..0f2823bd5 --- /dev/null +++ b/tests/fallow-runner.test.cjs @@ -0,0 +1,329 @@ +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); +const fs = require('node:fs'); + +const { + resolveFallowBinary, + requireFallowBinary, +} = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'fallow-runner.cjs')); +const { + resolveExecutableBinary, +} = require(path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'shell-command-projection.cjs')); + +const { createTempDir, cleanup } = require('./helpers.cjs'); + +// `resolveFallowBinary` resolves through the seam using the REAL process.platform — +// it has no platform injection. So the fixture, not the assertion, is what varies: +// on win32 the seam only accepts a PATHEXT-listed extension (never a bare name, by +// design — the #3275 npm sh-shim trap), so the staged file must carry one there. +// This keeps every row genuinely executing on all three CI lanes instead of +// silently no-opping on one. +// +// The extension's CASE must come from the same source the seam itself falls +// back to (fallow-runner.cjs passes `process.env.PATHEXT ?? ''`, and the seam +// falls back to its own DEFAULT_PATHEXT `.EXE;.CMD;.BAT;.COM` when that's +// empty) — resolveExecutableBinary returns `name + `, +// never the on-disk file's own casing, so a hardcoded 'fallow.cmd' would +// silently mismatch a real Windows PATHEXT of '.CMD' with a strict +// assert.equal (case-insensitive filesystem, case-SENSITIVE string compare). +const WIN32_CMD_EXT = (process.env.PATHEXT || '.EXE;.CMD;.BAT;.COM') + .split(';') + .find((ext) => ext.toLowerCase() === '.cmd') ?? '.CMD'; +const FALLOW_FIXTURE = process.platform === 'win32' ? `fallow${WIN32_CMD_EXT}` : 'fallow'; + +// ─── fallow-runner (#3618 Phase 2) ────────────────────────────────────────── +// Fold of the prior private resolver onto the seam's resolveExecutableBinary. +// See .gsd/phase/chore-3618-fallow-binary-resolution/50-test-matrix.md (F1-F15). +// +// Executability (X_OK) cannot be forced via chmod — root Docker bypasses mode +// bits, which would silently zero out coverage. Every case that needs a POSIX +// "is executable" or "is not executable" outcome monkeypatches fs.accessSync +// instead, restored in a finally. +// +// F7-F10 (the win32 rows) cannot inject a platform through +// resolveFallowBinary's public signature, so they are asserted directly +// against resolveExecutableBinary with platform:'win32' plus the same +// prependPaths/requireExecutable shape resolveFallowBinary passes — a +// projection of fallow's call, not fallow's own entry point (approach (a) +// from the phase brief). +function resolveFallowBinaryWin32({ cwd, envPath = '' }) { + return resolveExecutableBinary('fallow', { + platform: 'win32', + prependPaths: [path.join(cwd, 'node_modules', '.bin')], + env: { PATH: envPath, PATHEXT: process.env['PATHEXT'] ?? '' }, + requireExecutable: true, + }); +} + +describe('resolveFallowBinary (#3618)', () => { + test('F1: POSIX, executable fallow in /node_modules/.bin only — the .bin path', () => { + const cwd = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + const binDir = path.join(cwd, 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + const fallowPath = path.join(binDir, FALLOW_FIXTURE); + fs.writeFileSync(fallowPath, ''); + fs.accessSync = () => {}; + const resolved = resolveFallowBinary({ cwd, envPath: '' }); + assert.equal(resolved, fallowPath); + } finally { + fs.accessSync = originalAccessSync; + cleanup(cwd); + } + }); + + test('F2: POSIX, fallow on envPath only — the PATH copy', () => { + const cwd = createTempDir(); + const pathDir = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + const fallowPath = path.join(pathDir, FALLOW_FIXTURE); + fs.writeFileSync(fallowPath, ''); + fs.accessSync = () => {}; + const resolved = resolveFallowBinary({ cwd, envPath: pathDir }); + assert.equal(resolved, fallowPath); + } finally { + fs.accessSync = originalAccessSync; + cleanup(cwd); + cleanup(pathDir); + } + }); + + test('F3: POSIX, executable fallow in BOTH .bin and PATH — .bin wins (documented-backwards precedence)', () => { + const cwd = createTempDir(); + const pathDir = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + const binDir = path.join(cwd, 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + const binFallow = path.join(binDir, FALLOW_FIXTURE); + const pathFallow = path.join(pathDir, FALLOW_FIXTURE); + fs.writeFileSync(binFallow, ''); + fs.writeFileSync(pathFallow, ''); + fs.accessSync = () => {}; + const resolved = resolveFallowBinary({ cwd, envPath: pathDir }); + assert.equal(resolved, binFallow); + assert.notEqual(resolved, pathFallow); + } finally { + fs.accessSync = originalAccessSync; + cleanup(cwd); + cleanup(pathDir); + } + }); + + test('F4: accessSync failure yields null on POSIX; on win32 requireExecutable is a documented no-op', () => { + const cwd = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + const binDir = path.join(cwd, 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + const fallowPath = path.join(binDir, FALLOW_FIXTURE); + fs.writeFileSync(fallowPath, ''); + fs.accessSync = () => { throw Object.assign(new Error('EACCES'), { code: 'EACCES' }); }; + const resolved = resolveFallowBinary({ cwd, envPath: '' }); + if (process.platform === 'win32') { + // requireExecutable is a documented no-op on win32 (mode bits don't + // mean execute there), so accessSync throwing must NOT block resolution. + assert.equal(resolved, fallowPath); + } else { + // POSIX: the X_OK check is consulted, so a throwing accessSync must null out. + assert.equal(resolved, null); + } + } finally { + fs.accessSync = originalAccessSync; + cleanup(cwd); + } + }); + + test('F5: absent everywhere — null', () => { + const cwd = createTempDir(); + try { + const resolved = resolveFallowBinary({ cwd, envPath: '' }); + assert.equal(resolved, null); + } finally { + cleanup(cwd); + } + }); + + test('F6: envPath omitted (default from process.env.PATH), binary in .bin — resolves from .bin', () => { + const cwd = createTempDir(); + const originalAccessSync = fs.accessSync; + const originalPath = process.env.PATH; + try { + const binDir = path.join(cwd, 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + const fallowPath = path.join(binDir, FALLOW_FIXTURE); + fs.writeFileSync(fallowPath, ''); + fs.accessSync = () => {}; + process.env.PATH = ''; + // Call shape production uses (structural-pre-pass.md): no envPath. + const resolved = resolveFallowBinary({ cwd }); + assert.equal(resolved, fallowPath); + } finally { + fs.accessSync = originalAccessSync; + process.env.PATH = originalPath; + cleanup(cwd); + } + }); + + test('F7: win32, fallow.CMD in .bin — resolves .bin/fallow.CMD', () => { + const cwd = createTempDir(); + try { + const binDir = path.join(cwd, 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + const fallowCmd = path.join(binDir, 'fallow.CMD'); + fs.writeFileSync(fallowCmd, ''); + const resolved = resolveFallowBinaryWin32({ cwd, envPath: '' }); + assert.equal(resolved, fallowCmd); + } finally { + cleanup(cwd); + } + }); + + test('F8: win32, fallow.EXE and fallow.CMD both on PATH — .EXE wins (PATHEXT default order)', () => { + const cwd = createTempDir(); + const pathDir = createTempDir(); + try { + const exePath = path.join(pathDir, 'fallow.EXE'); + const cmdPath = path.join(pathDir, 'fallow.CMD'); + fs.writeFileSync(exePath, ''); + fs.writeFileSync(cmdPath, ''); + const resolved = resolveFallowBinaryWin32({ cwd, envPath: pathDir }); + assert.equal(resolved, exePath); + assert.notEqual(resolved, cmdPath); + } finally { + cleanup(cwd); + cleanup(pathDir); + } + }); + + test('F9: win32, extensionless fallow beside fallow.CMD — resolves fallow.CMD, never the shim (#3275 trap)', () => { + const cwd = createTempDir(); + const pathDir = createTempDir(); + try { + const bareFallow = path.join(pathDir, 'fallow'); + const cmdFallow = path.join(pathDir, 'fallow.CMD'); + fs.writeFileSync(bareFallow, ''); + fs.writeFileSync(cmdFallow, ''); + const resolved = resolveFallowBinaryWin32({ cwd, envPath: pathDir }); + assert.equal(resolved, cmdFallow); + assert.notEqual(resolved, bareFallow); + } finally { + cleanup(cwd); + cleanup(pathDir); + } + }); + + test('F10: win32, only an extensionless fallow — null (intentional change)', () => { + const cwd = createTempDir(); + const pathDir = createTempDir(); + try { + fs.writeFileSync(path.join(pathDir, 'fallow'), ''); + const resolved = resolveFallowBinaryWin32({ cwd, envPath: pathDir }); + assert.equal(resolved, null); + } finally { + cleanup(cwd); + cleanup(pathDir); + } + }); + + test('F11: requireFallowBinary with a binary present — returns the path, does not throw', () => { + const cwd = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + const binDir = path.join(cwd, 'node_modules', '.bin'); + fs.mkdirSync(binDir, { recursive: true }); + const fallowPath = path.join(binDir, FALLOW_FIXTURE); + fs.writeFileSync(fallowPath, ''); + fs.accessSync = () => {}; + const resolved = requireFallowBinary({ cwd, envPath: '' }); + assert.equal(resolved, fallowPath); + } finally { + fs.accessSync = originalAccessSync; + cleanup(cwd); + } + }); + + test('F12: requireFallowBinary with nothing found — throws the byte-identical install-instructions message', () => { + const cwd = createTempDir(); + try { + let caught = null; + try { + requireFallowBinary({ cwd, envPath: '' }); + } catch (err) { + caught = err; + } + assert.ok(caught instanceof Error, 'requireFallowBinary must throw when nothing is found'); + assert.equal( + caught.message, + 'Fallow is enabled but no binary was found. Please install fallow via `npm install -D fallow` or `cargo install fallow`.', + ); + } finally { + cleanup(cwd); + } + }); + + test('F13: has no node_modules at all — no throw; falls through to PATH', () => { + const cwd = createTempDir(); + const pathDir = createTempDir(); + const originalAccessSync = fs.accessSync; + const originalPath = process.env.PATH; + try { + const fallowPath = path.join(pathDir, FALLOW_FIXTURE); + fs.writeFileSync(fallowPath, ''); + fs.accessSync = () => {}; + process.env.PATH = pathDir; + // Call shape production uses (structural-pre-pass.md): no envPath. + assert.doesNotThrow(() => { + const resolved = resolveFallowBinary({ cwd }); + assert.equal(resolved, fallowPath); + }); + } finally { + fs.accessSync = originalAccessSync; + process.env.PATH = originalPath; + cleanup(cwd); + cleanup(pathDir); + } + }); + + test('F14: /node_modules/.bin exists but is a file, not a directory — no throw; falls through', () => { + const cwd = createTempDir(); + const pathDir = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + const nodeModulesDir = path.join(cwd, 'node_modules'); + fs.mkdirSync(nodeModulesDir, { recursive: true }); + fs.writeFileSync(path.join(nodeModulesDir, '.bin'), ''); // .bin is a FILE, not a dir + const fallowPath = path.join(pathDir, FALLOW_FIXTURE); + fs.writeFileSync(fallowPath, ''); + fs.accessSync = () => {}; + assert.doesNotThrow(() => { + const resolved = resolveFallowBinary({ cwd, envPath: pathDir }); + assert.equal(resolved, fallowPath); + }); + } finally { + fs.accessSync = originalAccessSync; + cleanup(cwd); + cleanup(pathDir); + } + }); + + test('F15: a directory named fallow in .bin — not resolved (isFile() is the predicate)', () => { + const cwd = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + const binDir = path.join(cwd, 'node_modules', '.bin'); + fs.mkdirSync(path.join(binDir, FALLOW_FIXTURE), { recursive: true }); // fallow is a DIRECTORY + fs.accessSync = () => {}; + const resolved = resolveFallowBinary({ cwd, envPath: '' }); + assert.equal(resolved, null); + } finally { + fs.accessSync = originalAccessSync; + cleanup(cwd); + } + }); +}); diff --git a/tests/shell-command-projection-dispatch.test.cjs b/tests/shell-command-projection-dispatch.test.cjs index c68a45fa9..f251b6855 100644 --- a/tests/shell-command-projection-dispatch.test.cjs +++ b/tests/shell-command-projection-dispatch.test.cjs @@ -413,6 +413,228 @@ describe('resolveExecutableBinary (#3411)', () => { }); }); +// ─── resolveExecutableBinary seam options (#3618) ─────────────────────────── +// prependPaths / requireExecutable — both opt-in, both default off, so +// Phase 1 callers (R1-R25 above, P1-P16 below) stay byte-identical. See +// .gsd/phase/chore-3618-fallow-binary-resolution/50-test-matrix.md (S1-S12). + +describe('resolveExecutableBinary seam options (#3618)', () => { + test('S1: prependPaths with binary only in dirA resolves in dirA', () => { + const dirA = createTempDir(); + try { + fs.writeFileSync(path.join(dirA, 'foo'), ''); + const resolved = resolveExecutableBinary('foo', { + platform: 'linux', + env: { PATH: '' }, + prependPaths: [dirA], + }); + assert.equal(resolved, path.join(dirA, 'foo')); + } finally { + cleanup(dirA); + } + }); + + test('S2: binary in both prependPaths dir and env.PATH — the prependPaths copy wins (precedence)', () => { + const dirA = createTempDir(); + const dirB = createTempDir(); + try { + fs.writeFileSync(path.join(dirA, 'foo'), ''); + fs.writeFileSync(path.join(dirB, 'foo'), ''); + const resolved = resolveExecutableBinary('foo', { + platform: 'linux', + env: { PATH: dirB }, + prependPaths: [dirA], + }); + assert.equal(resolved, path.join(dirA, 'foo')); + assert.notEqual(resolved, path.join(dirB, 'foo')); + } finally { + cleanup(dirA); + cleanup(dirB); + } + }); + + test('S3: prependPaths: [] is identical to omitting it', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo'), ''); + const withEmpty = resolveExecutableBinary('foo', { platform: 'linux', env: { PATH: dir }, prependPaths: [] }); + const withOmitted = resolveExecutableBinary('foo', { platform: 'linux', env: { PATH: dir } }); + assert.equal(withEmpty, path.join(dir, 'foo')); + assert.equal(withEmpty, withOmitted); + } finally { + cleanup(dir); + } + }); + + test('S4: prependPaths given, binary only on env.PATH — falls through to PATH', () => { + const dirA = createTempDir(); + const dirB = createTempDir(); + try { + fs.writeFileSync(path.join(dirB, 'foo'), ''); + const resolved = resolveExecutableBinary('foo', { + platform: 'linux', + env: { PATH: dirB }, + prependPaths: [dirA], + }); + assert.equal(resolved, path.join(dirB, 'foo')); + } finally { + cleanup(dirA); + cleanup(dirB); + } + }); + + test('S5: prependPaths with two dirs, binary in the second — first-match wins, in array order', () => { + const dir1 = createTempDir(); + const dir2 = createTempDir(); + try { + fs.writeFileSync(path.join(dir2, 'foo'), ''); + const resolved = resolveExecutableBinary('foo', { + platform: 'linux', + env: { PATH: '' }, + prependPaths: [dir1, dir2], + }); + assert.equal(resolved, path.join(dir2, 'foo')); + // dir1 is first in the array but has no match, so dir2 wins — confirms + // the search follows array order, not the reverse. + assert.notEqual(resolved, path.join(dir1, 'foo')); + } finally { + cleanup(dir1); + cleanup(dir2); + } + }); + + test('S6: win32 + prependPaths — PATHEXT applies inside the prepended dir too', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const resolved = resolveExecutableBinary('foo', { + platform: 'win32', + env: { PATH: '', PATHEXT: '.EXE;.CMD' }, + prependPaths: [dir], + }); + assert.equal(resolved, path.join(dir, 'foo.CMD')); + } finally { + cleanup(dir); + } + }); + + test('S7: requireExecutable true, POSIX, file exists and is executable — resolves', () => { + const dir = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + fs.writeFileSync(path.join(dir, 'foo'), ''); + // Executability is forced deterministically via accessSync monkeypatch + // rather than chmod — root Docker bypasses mode bits (see file header). + fs.accessSync = () => {}; + const resolved = resolveExecutableBinary('foo', { + platform: 'linux', + env: { PATH: dir }, + requireExecutable: true, + }); + assert.equal(resolved, path.join(dir, 'foo')); + } finally { + fs.accessSync = originalAccessSync; + cleanup(dir); + } + }); + + test('S8: requireExecutable true, POSIX, accessSync throws EACCES — null', () => { + const dir = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + fs.writeFileSync(path.join(dir, 'foo'), ''); + fs.accessSync = () => { throw Object.assign(new Error('EACCES'), { code: 'EACCES' }); }; + const resolved = resolveExecutableBinary('foo', { + platform: 'linux', + env: { PATH: dir }, + requireExecutable: true, + }); + assert.equal(resolved, null); + } finally { + fs.accessSync = originalAccessSync; + cleanup(dir); + } + }); + + test('S9: requireExecutable omitted — resolves anyway and accessSync is never consulted', () => { + const dir = createTempDir(); + const originalAccessSync = fs.accessSync; + let called = false; + try { + fs.writeFileSync(path.join(dir, 'foo'), ''); + fs.accessSync = () => { + called = true; + throw Object.assign(new Error('EACCES'), { code: 'EACCES' }); + }; + const resolved = resolveExecutableBinary('foo', { platform: 'linux', env: { PATH: dir } }); + assert.equal(resolved, path.join(dir, 'foo')); + assert.equal(called, false, 'accessSync must not be consulted when requireExecutable is not set'); + } finally { + fs.accessSync = originalAccessSync; + cleanup(dir); + } + }); + + test('S10: requireExecutable true + win32 is a no-op — resolves without consulting accessSync', () => { + const dir = createTempDir(); + const originalAccessSync = fs.accessSync; + let called = false; + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + fs.accessSync = () => { + called = true; + throw Object.assign(new Error('EACCES'), { code: 'EACCES' }); + }; + const resolved = resolveExecutableBinary('foo', { + platform: 'win32', + env: { PATH: dir, PATHEXT: '.CMD' }, + requireExecutable: true, + }); + assert.equal(resolved, path.join(dir, 'foo.CMD')); + assert.equal(called, false, 'accessSync must never be consulted on win32'); + } finally { + fs.accessSync = originalAccessSync; + cleanup(dir); + } + }); + + test('S11: requireExecutable true + path-like name — executability still enforced on the direct path', () => { + const dir = createTempDir(); + const originalAccessSync = fs.accessSync; + try { + const staged = path.join(dir, 'foo'); + fs.writeFileSync(staged, ''); + fs.accessSync = () => { throw Object.assign(new Error('EACCES'), { code: 'EACCES' }); }; + const resolved = resolveExecutableBinary(staged, { + platform: 'linux', + env: { PATH: '' }, + requireExecutable: true, + }); + assert.equal(resolved, null); + } finally { + fs.accessSync = originalAccessSync; + cleanup(dir); + } + }); + + test('S12: neither seam option set behaves exactly as before (win32 and posix)', () => { + const dirWin = createTempDir(); + const dirPosix = createTempDir(); + try { + fs.writeFileSync(path.join(dirWin, 'foo.CMD'), ''); + const win = resolveExecutableBinary('foo', { platform: 'win32', env: { PATH: dirWin, PATHEXT: '.CMD' } }); + assert.equal(win, path.join(dirWin, 'foo.CMD')); + + fs.writeFileSync(path.join(dirPosix, 'foo'), ''); + const posix = resolveExecutableBinary('foo', { platform: 'linux', env: { PATH: dirPosix } }); + assert.equal(posix, path.join(dirPosix, 'foo')); + } finally { + cleanup(dirWin); + cleanup(dirPosix); + } + }); +}); + // ─── projectSpawnInvocation (#3411) ───────────────────────────────────────── // Mirrors the seam's own `_cmdQuoteToken` (a literal `"` is doubled) so