-
Notifications
You must be signed in to change notification settings - Fork 886
Description
Summary
The SDK ships 10 e2e test files in nodejs/test/e2e/ that spawn the CLI via CopilotClient.start(). These tests would have caught the --headless removal in CLI v0.0.410 before it broke every SDK consumer. They did not catch it because CI never installs or runs the CLI binary.
Evidence
1. CI installs dependencies with --ignore-scripts
The nodejs-sdk-tests.yml workflow runs:
npm ci --ignore-scriptsThe --ignore-scripts flag suppresses the postinstall hook that @github/copilot uses to install its platform-specific binary. Without it, the CLI binary does not exist on disk. Any e2e test that calls client.start() either silently skips or fails — either way, it tests nothing.
2. No COPILOT_CLI_PATH is set
The e2e test harness respects COPILOT_CLI_PATH for locating the CLI. The CI workflow never sets this variable and never installs the CLI through any other mechanism.
3. Scenario tests compile but never execute
The scenario-builds.yml workflow compiles scenarios across four languages (TypeScript, Python, Go, C#) but never runs them. The test/scenarios/verify.sh orchestrator exists and accepts COPILOT_CLI_PATH, but no workflow invokes it.
4. Unit tests mock away CLI spawning entirely
nodejs/test/client.test.ts uses vi.spyOn() to mock internal methods. It tests option validation (URL parsing, port conflicts) but never exercises startCLIServer(), --headless, or --stdio.
Impact
CLI v0.0.410 removed --headless --stdio on February 13. Nine days later, SDK v0.1.26-preview.0 still passes --headless unconditionally (lines 693-704 of dist/client.js). A single CI job that installed the CLI and ran client.start() would have caught this before the CLI shipped.
The --acp flag was added in CLI v0.0.397 — a 13-version window where both flags coexisted. The SDK had ample runway to detect the deprecation and prepare.
Suggested fix
Add a CI job to nodejs-sdk-tests.yml that:
- Installs
@github/copilotwith postinstall scripts (drop--ignore-scriptsor add a separate step) - Sets
COPILOT_CLI_PATHto the installed binary - Runs the existing e2e tests against it
The tests already exist. The infrastructure to run them does not.
Related
- CLI v0.0.410+ auto-update breaks all SDK versions — --headless --stdio no longer accepted #530 — CLI v0.0.410+ breaks all SDK versions (
--headlessremoved) - Node SDK: getBundledCliPath() breaks in CJS bundles (VS Code extensions) #528 —
getBundledCliPathuses ESM-onlyimport.meta.resolve, breaks CJS consumers