From b482f12d571f0b11713239dc2d114f5964cc812d Mon Sep 17 00:00:00 2001 From: Huanqi Cao Date: Mon, 3 Aug 2026 00:21:17 +0800 Subject: [PATCH] fix(coverage): probe the pwsh exemption with the executor's own resolution The review caught the pwshCoverageExclusions comment claiming the inverse of the code's behavior: a bare-PATH probe FAILING grants the exemption, so a win32 host where only install-location pwsh (or 5.1) resolves kept the exemption while its suites ran - over-exempting, never tightening. resolvePwshPath/candidatePwshPaths move to the dependency-free pwsh-local/src/resolve.ts (index.ts re-exports; public API unchanged) and vitest.config.ts probes with that shared definition, so the exemption is active exactly when the suites skip. The two spec headers stop saying 'on PATH' for a probe that is deliberately not PATH-only, and the parity note records the abort backport as the one both-ways parity change (both languages, pairing re-recorded); catalogs regenerated for the line shift. --- ...2026-08-02-pwsh-tool-bash-parity.i18n.yaml | 4 +- .../2026-08-02-pwsh-tool-bash-parity.md | 1 + .../2026-08-02-pwsh-tool-bash-parity.zh.md | 3 +- docs/config-catalog.md | 2 +- packages/bash/pwsh-local/src/index.ts | 53 ++-------------- packages/bash/pwsh-local/src/resolve.ts | 60 +++++++++++++++++++ .../bash/pwsh-local/tests/executor.spec.ts | 4 +- .../bash/tool-pwsh/tests/integration.spec.ts | 2 +- vitest.config.ts | 10 ++-- 9 files changed, 78 insertions(+), 61 deletions(-) create mode 100644 packages/bash/pwsh-local/src/resolve.ts diff --git a/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.i18n.yaml b/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.i18n.yaml index 913cf0cc06..6cbc24d8aa 100644 --- a/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.md -2026-08-02-pwsh-tool-bash-parity.md: fb0f3fff1ed00dfde286781733881facfb1e6c7b -2026-08-02-pwsh-tool-bash-parity.zh.md: 9dc84bcd5e53c52d5d68b2db61cf79fe7fb97bb9 +2026-08-02-pwsh-tool-bash-parity.md: 417c6d6bc91eb3afaa38976e0013e4fbe72854ca +2026-08-02-pwsh-tool-bash-parity.zh.md: 926433526b3820f7b4770acb3ee172448970b601 diff --git a/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.md b/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.md index fb0f3fff1e..417c6d6bc9 100644 --- a/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.md +++ b/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.md @@ -27,6 +27,7 @@ The first Windows-native foundation shipped `dsh-tool-pwsh` as a deliberately mi ## Consequences - The bash and pwsh tools are now behaviorally interchangeable for foreground and background shell work (minus sandbox), and the pwsh prompt/description sentences are each backed by the renderer — the reviewer's grep-against-code check passes. +- Parity ran BOTH ways once: the pwsh tool's structured foreground abort (`HarnessError('tool call aborted', TOOL_ABORTED)` with name `AbortError`) was backported to the bash tool, replacing its uncoded `Error('command aborted')` — a model-visible/logged change pinned by exact-shape tests on both sides and by the cancel-tool-calls fixture. - `@deepseek-ai/dsh-bash-env` is a new shipped package; `dsh-tool-bash`'s `dshHome` config moved there, so compositions mounting the shell tools must also mount `bash-env` (the spine bundles do). - Windows-only semantics (CRLF normalization, forced-termination exit-1/signal-null, POSIX-only self-signal) remain pinned by tests as before. - The pwsh tool's per-file coverage gate rides on the scriptable fake-executor suite (`tests/tools.spec.ts`); the real-pwsh integration and Loader-composition suites self-skip where `pwsh` is absent, mirroring the bash suites' division of labor. diff --git a/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.zh.md b/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.zh.md index 9dc84bcd5e..926433526b 100644 --- a/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.zh.md +++ b/.agents/notes/implemented/feature/2026-08-02-pwsh-tool-bash-parity.zh.md @@ -26,7 +26,8 @@ Status: implemented ## 后果 -- bash 与 pwsh 工具在前台与后台 shell 工作（减 sandbox）上行为可互换，pwsh 的 prompt/描述句每句都有渲染器背书——reviewer 的"拿代码 grep 对证"检查通过。 +- bash 与 pwsh 工具在前台与后台 shell 工作（减 sandbox）上行为可互换，pwsh 的 prompt/描述句每句都有渲染器背书——reviewer 的“拿代码 grep 对证”检查通过。 +- 对齐也反向发生过一次：pwsh 工具的结构化前台中止（`HarnessError('tool call aborted', TOOL_ABORTED)`，name 为 `AbortError`）被回移到 bash 工具，取代其无码的 `Error('command aborted')`——这是模型可见/入日志的变更，由两侧的精确形状测试与 cancel-tool-calls fixture 钉住。 - `@deepseek-ai/dsh-bash-env` 成为新的交付包；`dsh-tool-bash` 的 `dshHome` 配置迁往那里，因此挂载 shell 工具的组合也必须挂载 `bash-env`（spine bundle 已如此）。 - Windows 专属语义（CRLF 归一化、强制终止 exit-1/signal-null、仅 POSIX 的自信号）一如既往由测试钉住。 - pwsh 工具的 per-file 覆盖门禁由可脚本化的 fake-executor 套件（`tests/tools.spec.ts`）承担；真实 pwsh 的集成与 Loader 组合套件在无 `pwsh` 的宿主自跳过，与 bash 套件的分工一致。 diff --git a/docs/config-catalog.md b/docs/config-catalog.md index f14a7a5900..9a2eb8f0be 100644 --- a/docs/config-catalog.md +++ b/docs/config-catalog.md @@ -1004,7 +1004,7 @@ export interface Config { } ``` -Source: [`packages/bash/pwsh-local/src/index.ts:55`](../packages/bash/pwsh-local/src/index.ts) +Source: [`packages/bash/pwsh-local/src/index.ts:54`](../packages/bash/pwsh-local/src/index.ts) ## `@deepseek-ai/dsh-repeat-tool-guard` diff --git a/packages/bash/pwsh-local/src/index.ts b/packages/bash/pwsh-local/src/index.ts index b966a579af..316d2c8651 100644 --- a/packages/bash/pwsh-local/src/index.ts +++ b/packages/bash/pwsh-local/src/index.ts @@ -13,14 +13,13 @@ * @module @deepseek-ai/dsh-pwsh-local */ -import { existsSync } from 'node:fs' -import { join } from 'node:path' import { Context } from 'cordis' import z from 'schemastery' import { BashExecutor } from '@deepseek-ai/dsh-bash' import type { BashExecRequest, BashExecSpec, BashProcess, BashProcessRead, BashRunResult, CollectedOutput } from '@deepseek-ai/dsh-bash' import type { SubprocessCollect, SubprocessHandle, SubprocessOutputReader, SubprocessSpawnSpec } from '@deepseek-ai/dsh-subprocess' import { clampTimeout, deadline, timeoutOf } from '@deepseek-ai/dsh-timeout' +import { resolvePwshPath } from './resolve.ts' /* jscpd:ignore-start -- deliberate call-for-call mirror of dsh-bash-local (Agent Note: pwsh-tool-and-executor). */ /** @@ -77,53 +76,9 @@ export interface Config { /** The shape after schemastery applied the defaults (cwd/pwshPath have none). */ type ResolvedConfig = Required> & Pick -/** - * Well-known Windows PowerShell install locations plus PATH entries, newest - * first. Explicitly parameterized (env) so resolution is a pure function of - * its inputs on every platform. - * @param env - the environment to probe; defaults to the process environment. - * @returns candidate `pwsh` executable paths in resolution order. - */ -export function candidatePwshPaths(env: NodeJS.ProcessEnv = process.env): string[] { - const programFiles = env.ProgramFiles ?? 'C:\\Program Files' - const systemRoot = env.SystemRoot ?? 'C:\\Windows' - const candidates = [ - join(programFiles, 'PowerShell', '7', 'pwsh.exe'), - ] - // Microsoft Store installs (and any user-added location) live on PATH; - // entries may carry surrounding quotes from `setx`-style definitions. - for (const entry of (env.PATH ?? '').split(';')) { - const trimmed = entry.trim().replace(/^"|"$/g, '') - if (trimmed.length === 0) continue - candidates.push(join(trimmed, 'pwsh.exe')) - } - // Windows PowerShell 5.1 remains the last-resort fallback on legacy hosts. - candidates.push(join(systemRoot, 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe')) - return candidates -} - -/** - * Resolve the pwsh executable this executor spawns. - * @param configured - an explicit `pwshPath` config value, trusted as-is. - * @param env - the environment to probe on Windows; defaults to the process environment. - * @param platform - the platform to resolve for; defaults to the process platform. - * @returns the first existing well-known location on Windows (PowerShell 7 - * install, a PATH entry such as the Microsoft Store install, then Windows - * PowerShell 5.1), else `pwsh` for PATH resolution. - */ -export function resolvePwshPath( - configured?: string, - env: NodeJS.ProcessEnv = process.env, - platform: NodeJS.Platform = process.platform, -): string { - if (configured !== undefined && configured.length > 0) return configured - if (platform === 'win32') { - for (const candidate of candidatePwshPaths(env)) { - if (existsSync(candidate)) return candidate - } - } - return 'pwsh' -} +// Resolution lives in its own dependency-free module so the repository's +// coverage-gate probe shares the exact definition the suites use. +export { candidatePwshPaths, resolvePwshPath } from './resolve.ts' /** Project a settled collect-mode reader into the final CollectedOutput shape. */ function finalOutput(reader: SubprocessOutputReader): CollectedOutput { diff --git a/packages/bash/pwsh-local/src/resolve.ts b/packages/bash/pwsh-local/src/resolve.ts new file mode 100644 index 0000000000..c6ded2f883 --- /dev/null +++ b/packages/bash/pwsh-local/src/resolve.ts @@ -0,0 +1,60 @@ +/** + * PowerShell executable resolution, dependency-free so non-package consumers + * (the repository's coverage-gate probe in `vitest.config.ts`) can share the + * ONE resolution definition with the executor and its suites — a probe that + * resolved differently from the code under test could exempt a file whose + * suites actually run. + * + * @module @deepseek-ai/dsh-pwsh-local/resolve + */ + +import { existsSync } from 'node:fs' +import { join } from 'node:path' + +/** + * Well-known Windows PowerShell install locations plus PATH entries, newest + * first. Explicitly parameterized (env) so resolution is a pure function of + * its inputs on every platform. + * @param env - the environment to probe; defaults to the process environment. + * @returns candidate `pwsh` executable paths in resolution order. + */ +export function candidatePwshPaths(env: NodeJS.ProcessEnv = process.env): string[] { + const programFiles = env.ProgramFiles ?? 'C:\\Program Files' + const systemRoot = env.SystemRoot ?? 'C:\\Windows' + const candidates = [ + join(programFiles, 'PowerShell', '7', 'pwsh.exe'), + ] + // Microsoft Store installs (and any user-added location) live on PATH; + // entries may carry surrounding quotes from `setx`-style definitions. + for (const entry of (env.PATH ?? '').split(';')) { + const trimmed = entry.trim().replace(/^"|"$/g, '') + if (trimmed.length === 0) continue + candidates.push(join(trimmed, 'pwsh.exe')) + } + // Windows PowerShell 5.1 remains the last-resort fallback on legacy hosts. + candidates.push(join(systemRoot, 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe')) + return candidates +} + +/** + * Resolve the pwsh executable this executor spawns. + * @param configured - an explicit `pwshPath` config value, trusted as-is. + * @param env - the environment to probe on Windows; defaults to the process environment. + * @param platform - the platform to resolve for; defaults to the process platform. + * @returns the first existing well-known location on Windows (PowerShell 7 + * install, a PATH entry such as the Microsoft Store install, then Windows + * PowerShell 5.1), else `pwsh` for PATH resolution. + */ +export function resolvePwshPath( + configured?: string, + env: NodeJS.ProcessEnv = process.env, + platform: NodeJS.Platform = process.platform, +): string { + if (configured !== undefined && configured.length > 0) return configured + if (platform === 'win32') { + for (const candidate of candidatePwshPaths(env)) { + if (existsSync(candidate)) return candidate + } + } + return 'pwsh' +} diff --git a/packages/bash/pwsh-local/tests/executor.spec.ts b/packages/bash/pwsh-local/tests/executor.spec.ts index 5113f6a988..4552f2eeec 100644 --- a/packages/bash/pwsh-local/tests/executor.spec.ts +++ b/packages/bash/pwsh-local/tests/executor.spec.ts @@ -3,8 +3,8 @@ * service plus a REAL pwsh executable, exercised through the executor seam * (`resolve` → `run`/`start`). These verify the world — actual PowerShell * runs, output capture, truncation and spill, deadlines, kill escalation, and - * the background-handle contract. The suite self-skips when no `pwsh` is on - * PATH (a CI accommodation for hosts without PowerShell); the pure unit tests + * the background-handle contract. The suite self-skips when no usable `pwsh` + * resolves (a CI accommodation for hosts without PowerShell); the pure unit tests * (config validation, executable resolution) run on every platform. PowerShell * writes CRLF on Windows, so exact text assertions normalize line endings. */ diff --git a/packages/bash/tool-pwsh/tests/integration.spec.ts b/packages/bash/tool-pwsh/tests/integration.spec.ts index 711f7663b1..c347866f50 100644 --- a/packages/bash/tool-pwsh/tests/integration.spec.ts +++ b/packages/bash/tool-pwsh/tests/integration.spec.ts @@ -4,7 +4,7 @@ * process. These verify the world — actual commands run, stdout/stderr come * back, exit codes render, timeouts abort, background tasks settle through the * generic task runtime, and per-session cwd resolution works. The suite - * self-skips when no `pwsh` is on PATH (a CI accommodation for hosts without + * self-skips when no usable `pwsh` resolves (a CI accommodation for hosts without * PowerShell); the fake-executor suite (tools.spec.ts) carries the coverage * gate. */ diff --git a/vitest.config.ts b/vitest.config.ts index 458b1134b1..c922cb8a32 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,5 +1,6 @@ import { spawnSync } from 'node:child_process' import tsconfigPaths from 'vite-tsconfig-paths' +import { resolvePwshPath } from './packages/bash/pwsh-local/src/resolve.ts' import { defineConfig } from 'vitest/config' import { vitestExecArgv } from './vitest.shared.ts' import { COVERAGE_EXEMPT_ENV, coverageExemptHeavySuites } from './scripts/coverage-exempt.ts' @@ -44,11 +45,10 @@ const windowsCoverageExclusions = process.platform === 'win32' // self-skip without a real pwsh (executor.spec.ts hasPwsh), leaving this file // far below per-file 100% on pwsh-less hosts; the exemption keeps those hosts // green while CI runners ship pwsh and still enforce the full bar. The probe -// is deliberately PATH-only (narrower than the suites' resolvePwshPath): a -// win32 host where only install-location pwsh or 5.1 resolves forfeits the -// exemption while the suites still run, so the gate can only get stricter, -// never falsely green. -const pwshCoverageExclusions = spawnSync('pwsh', ['-NoLogo', '-NoProfile', '-NonInteractive', '-Command', '$true'], { encoding: 'utf8' }).status === 0 +// runs the suites' own resolution (the dependency-free resolve.ts module), +// so the exemption is active exactly when the suites skip — a mismatched +// narrower probe could exempt the file on hosts whose suites actually run. +const pwshCoverageExclusions = spawnSync(resolvePwshPath(), ['-NoLogo', '-NoProfile', '-NonInteractive', '-Command', '$true'], { encoding: 'utf8' }).status === 0 ? [] : ['packages/bash/pwsh-local/src/index.ts']