feat(sdk/typescript): add harness provider preflight - #1069
santoshkumarradha merged 3 commits into
Conversation
…inary harnessDoctor passed the bare provider name to the version probe while findExecutable had already resolved a PATHEXT-qualified path. On Windows the bare name misses npm shims (ENOENT) and spawning a resolved .cmd/.bat without a shell is rejected by Node (EINVAL, CVE-2024-27980), so shim-based providers like codex always reported version_probe_failed and usable=false. Probe the resolved absolute path instead, and route Windows batch shims through cmd.exe /d /s /c with the outer-quote form so paths with spaces survive. POSIX behavior is unchanged: the same file that findExecutable verified is executed directly. The VersionProbe contract now documents that command[0] is the resolved path.
|
|
santoshkumarradha
left a comment
santoshkumarradha
left a comment
There was a problem hiding this comment.
The focused TypeScript tests, lint, build, and diff checks pass locally. I found one API-contract issue in the new doctor options that needs a small fix before this is ready.
Performance
⚠ Regression detected:
|
📊 Coverage gateThresholds from
✅ Gate passedNo surface regressed past the allowed threshold and the aggregate stayed above the floor. |
📐 Patch coverage gateThreshold: 80% on lines this PR touches vs
✅ Patch gate passedEvery surface whose lines were touched by this PR has patch coverage at or above the threshold. |
…tion harnessDoctor resolved binaries against process.env even when callers supplied HarnessDoctorOptions.env, so a report could mix installation state from the process environment with authentication state from the custom one. Binary discovery (PATH lookup and Windows PATHEXT matching) now uses the same env as the offline auth signal. Addresses PR Agent-Field#1069 review feedback.
|
Fixed in 67b5bee: |
santoshkumarradha
left a comment
santoshkumarradha
left a comment
There was a problem hiding this comment.
Thanks for fixing the environment propagation. The custom PATH and PATHEXT coverage addresses the issue, and the focused tests, lint, build, TypeScript CI, CLA, and required coverage checks are all green.
Summary
Add an offline TypeScript harness-provider health API through
harnessDoctor()andAgent.harnessDoctor(), with structured version, installation, auth-signal, and issue data for every supported provider. Provider execution now fails before launch with the actionable typedHarnessProviderUnavailableerror, including when a CLI disappears between preflight and spawn.This is the TypeScript follow-up to the Python implementation in #756. The Go SDK remains a separate follow-up under #685.
Design notes
HarnessProviderUnavailable.missingAuthEnvis part of the typed error contract for parity with the merged Python implementation (feat(harness): add Python provider preflight #756), but is intentionally never populated: the doctor's auth signal is advisory (auth: "unknown") because a CLI may hold a local login with no environment variable set, so no preflight path fails on missing auth configuration. The field keeps the error forward-compatible with a future opt-in strict-auth check without changing current behavior..cmd/.batshims throughcmd.exe /d /s /cbecause Node refuses to spawn batch files without a shell (CVE-2024-27980); POSIX probes execute the resolved path directly. A failed probe still marks the provider unusable — the batch routing removes a false negative for npm shim installs rather than weakening the health signal. Note the provider spawn path itself still launches the configured bare binary name (a pre-existing Windows shim gap this PR does not claim to fix; tracked as a follow-up).Type of change
Test plan
cd sdk/typescript && npm run lintcd sdk/typescript && npm run buildcd sdk/typescript && npx vitest run --config vitest.config.ts tests/harness_doctor.test.ts tests/harness_cli.test.ts tests/harness_agent_wiring.test.ts(34 passed)harnessDoctor(['codex'])resolvedcodex.cmd, probed the real versioncodex-cli 0.153.4throughcmd.exe, and reportedusable: truewith no issuesOn this Windows host, the unfiltered
npm testresult was 1002 passed and 1 failed (1003 total). The failure is the pre-existing path-separator assertiontests/harness_runner.test.ts > run with schema injects suffix and parses output, which expects/while Windows returns\\; neither that test norsrc/harness/runner.tsis changed by this PR, and the failure reproduces in isolation. No paid provider/liveness calls were run.Test coverage
coverage-baseline.jsonin this PR only if the removal caused a legitimate regression and I called it out in the summary above. (No baseline change was needed.)Checklist
CONTRIBUTING.md(if present) anddocs/DEVELOPMENT.md.Related issues / PRs
Part of #685.
Follows #756.