* fix(sdk): decouple SDK from build-from-source install path, close #2441 and #2453 Ship sdk/dist prebuilt in the tarball and replace the npm-install-g sub-install with a parent-package bin shim (bin/gsd-sdk.js). npm chmods bin entries from a packed tarball correctly, eliminating the mode-644 failure (#2453) and the full class of NPM_CONFIG_PREFIX/ignore-scripts/ corepack/air-gapped failure modes that caused #2439 and #2441. Changes: - sdk/package.json: prepublishOnly runs `rm -rf dist && tsc && chmod +x dist/cli.js` (stale-build guard + execute-bit fix at publish time) - package.json: add "gsd-sdk": "bin/gsd-sdk.js" bin entry; add sdk/dist to files so the prebuilt CLI ships in the tarball - bin/gsd-sdk.js: new back-compat shim — resolves sdk/dist/cli.js relative to the package root and delegates via `node`, so all existing PATH call sites (slash commands, agents, hooks) continue to work unchanged (S1 shim) - bin/install.js: replace installSdkIfNeeded() build-from-source + global- install dance with a dist-verify + chmod-in-place guard; delete resolveGsdSdk(), detectShellRc(), emitSdkFatal() helpers now unused - .github/workflows/install-smoke.yml: add smoke-unpacked job that strips execute bit from sdk/dist/cli.js before install to reproduce the exact #2453 failure mode - tests/bug-2441-sdk-decouple.test.cjs: new regression tests asserting all invariants (no npm install -g from sdk/, shim exists, sdk/dist in files, prepublishOnly has rm -rf + chmod) - tests/bugs-1656-1657.test.cjs: update stale assertions that required build-from-source behavior (now asserts new prebuilt-dist invariants) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(release): bump to 1.38.2, wire release.yml to build SDK dist - Bump version 1.38.1 -> 1.38.2 for the #2441/#2453 fix shipped in 0f6903d. - Add `build:sdk` script (`cd sdk && npm ci && npm run build`). - `prepublishOnly` now runs hooks + SDK builds as a safety net. - release.yml (rc + finalize): build SDK dist before `npm publish` so the published tarball always ships fresh `sdk/dist/` (kept gitignored). - CHANGELOG: document 1.38.2 entry and `--sdk` flag semantics change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ci: build SDK dist before tests and smoke jobs sdk/dist/ is gitignored (built fresh at publish time via release.yml), but both the test suite and install-smoke jobs run `bin/install.js` or `npm pack` against the checked-out tree where dist doesn't exist yet. - test.yml: `npm run build:sdk` before `npm run test:coverage`, so tests that spawn `bin/install.js` don't hit `installSdkIfNeeded()`'s fatal missing-dist check. - install-smoke.yml (both smoke and smoke-unpacked): build SDK before pack/chmod so the published tarball contains dist and the unpacked install has a file to strip exec-bit from. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sdk): lift SDK runtime deps to parent so tarball install can resolve them The SDK's runtime deps (ws, @anthropic-ai/claude-agent-sdk) live in sdk/package.json, but sdk/node_modules is NOT shipped in the parent tarball — only sdk/dist, sdk/src, sdk/prompts, and sdk/package.json are. When a user runs `npm install -g get-shit-done-cc`, npm installs the parent's node_modules but never runs `npm install` inside the nested sdk/ directory. Result: `node sdk/dist/cli.js` fails with ERR_MODULE_NOT_FOUND for 'ws'. The smoke tarball job caught this; the unpacked variant masked it because `npm install -g <dir>` copies the entire workspace including sdk/node_modules (left over from `npm run build:sdk`). Fix: declare the same deps in the parent package.json so they land in <pkg>/node_modules, which Node's resolution walks up to from <pkg>/sdk/dist/cli.js. Keep them declared in sdk/package.json too so the SDK remains a self-contained package for standalone dev. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(lockfile): regenerate package-lock.json cleanly The previous `npm install` run left the lockfile internally inconsistent (resolved esbuild@0.27.7 referenced but not fully written), causing `npm ci` to fail in CI with "Missing from lock file" errors. Clean regen via rm + npm install fixes all three failed jobs (test, smoke, smoke-unpacked), which were all hitting the same `npm ci` sync check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(deps): remove unused esbuild + vitest from root devDependencies Both were declared but never imported anywhere in the root package (confirmed via grep of bin/, scripts/, tests/). They lived in sdk/ already, which is the only place they're actually used. The transitive tree they pulled in (vitest → vite → esbuild 0.28 → @esbuild/openharmony-arm64) was the root of the CI npm ci failures: the openharmony platform package's `optional: true` flag was not being applied correctly by npm 10 on Linux runners, causing EBADPLATFORM. After removal: 800+ transitive packages → 155. Lockfile regenerated cleanly. All 4170 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sdk): pretest:coverage builds sdk; tighten shim test assertions Add "pretest:coverage": "npm run build:sdk" so npm run test:coverage works in clean checkouts where sdk/dist/ hasn't been built yet. Tighten the two loose shim assertions in bug-2441-sdk-decouple.test.cjs: - forwards-to test now asserts path.resolve() is called with the 'sdk','dist','cli.js' path segments, not just substring presence - node-invocation test now asserts spawnSync(process.execPath, [...]) pattern, ruling out matches in comments or the shebang line Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address PR review — pretest:coverage + tighten shim tests Review feedback from trek-e on PR 2457: 1. pretest:coverage + pretest hooks now run `npm run build:sdk` so `npm run test[:coverage]` in a clean checkout produces the required sdk/dist/ artifacts before running the installer-dependent tests. CI already does this explicitly; local contributors benefit. 2. Shim tests in bug-2441-sdk-decouple.test.cjs tightened from loose substring matches (which would pass on comments/shebangs alone) to regex assertions on the actual path.resolve call, spawnSync with process.execPath, process.argv.slice(2), and process.exit pattern. These now provide real regression protection for #2453-class bugs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: correct CHANGELOG entry and add [1.38.2] reference link Two issues in the 1.38.2 CHANGELOG entry: - installSdkIfNeeded() was described as deleted but it still exists in bin/install.js (repurposed to verify sdk/dist/cli.js and fix execute bit). Corrected the description to say 'repurposes' rather than 'deletes'. - The reference-link block at the bottom of the file was missing a [1.38.2] compare URL and [Unreleased] still pointed to v1.37.1...HEAD. Added the [1.38.2] link and updated [Unreleased] to compare/v1.38.2...HEAD. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sdk): double-cast WorkflowConfig to Record for strict tsc build TypeScript error on main (introduced in #2611) blocks `npm run build` in sdk/, which now runs as part of this PR's tarball build path. Apply the double-cast via `unknown` as the compiler suggests. Same fix as #2622; can be dropped if that lands first. * test: remove bug-2598 test obsoleted by SDK decoupling The bug-2598 test guards the Windows CVE-2024-27980 fix in the old build-from-source path (npm spawnSync with shell:true + formatSpawnFailure diagnostics). This PR removes that entire code path — installSdkIfNeeded no longer spawns npm, it just verifies the prebuilt sdk/dist/cli.js shipped in the tarball. The test asserts `installSdkIfNeeded.toString()` contains a formatSpawnFailure helper. After decoupling, no such helper exists (nothing to format — there's no spawn). Keeping the test would assert invariants of the rejected architecture. The original #2598 defect (silent failure of npm spawn on Windows) is structurally impossible in the shim path: bin/gsd-sdk.js invokes `node sdk/dist/cli.js` directly via child_process.spawn with an explicit argv array. No .cmd wrapper, no shell delegation. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
166 lines
6.4 KiB
JavaScript
166 lines
6.4 KiB
JavaScript
/**
|
|
* Regression tests for fix/2441-sdk-decouple
|
|
*
|
|
* Verifies the architectural invariants introduced by the SDK decouple:
|
|
*
|
|
* (a) bin/install.js does NOT invoke `npm install -g` for the SDK at all.
|
|
* The old `installSdkIfNeeded()` built from source and ran `npm install -g .`
|
|
* in sdk/; the new version only verifies the prebuilt dist.
|
|
*
|
|
* (b) The parent package.json declares a `gsd-sdk` bin entry pointing at
|
|
* bin/gsd-sdk.js (the back-compat shim), so npm chmods it correctly.
|
|
*
|
|
* (c) sdk/dist/ is in the parent package `files` so it ships in the tarball.
|
|
*
|
|
* (d) sdk/package.json `prepublishOnly` runs `rm -rf dist && tsc && chmod +x dist/cli.js`
|
|
* (guards against the mode-644 bug and npm's stale-prepublishOnly issue).
|
|
*/
|
|
|
|
'use strict';
|
|
|
|
const { test, describe } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const fs = require('fs');
|
|
const path = require('path');
|
|
|
|
const INSTALL_JS = path.join(__dirname, '..', 'bin', 'install.js');
|
|
const ROOT_PKG = path.join(__dirname, '..', 'package.json');
|
|
const SDK_PKG = path.join(__dirname, '..', 'sdk', 'package.json');
|
|
const GSD_SDK_SHIM = path.join(__dirname, '..', 'bin', 'gsd-sdk.js');
|
|
|
|
const installContent = fs.readFileSync(INSTALL_JS, 'utf-8');
|
|
const rootPkg = JSON.parse(fs.readFileSync(ROOT_PKG, 'utf-8'));
|
|
const sdkPkg = JSON.parse(fs.readFileSync(SDK_PKG, 'utf-8'));
|
|
|
|
describe('fix #2441: SDK decouple — installer no longer builds from source', () => {
|
|
test('bin/install.js does not call npm install -g in sdk/', () => {
|
|
// The old approach ran `npm install -g .` from sdk/. This must be gone.
|
|
// We check for the specific pattern that installed the SDK globally.
|
|
const hasGlobalInstallFromSdk =
|
|
/spawnSync\(npmCmd,\s*\[['"]install['"],\s*['"](-g|--global)['"]/m.test(installContent) &&
|
|
/cwd:\s*sdkDir/.test(installContent);
|
|
assert.ok(
|
|
!hasGlobalInstallFromSdk,
|
|
'bin/install.js must not run `npm install -g .` from sdk/. ' +
|
|
'The SDK is shipped prebuilt in the tarball (fix #2441).'
|
|
);
|
|
});
|
|
|
|
test('bin/install.js does not run npm run build in sdk/', () => {
|
|
// The old approach ran `npm run build` (tsc) at install time.
|
|
const hasBuildStep =
|
|
/spawnSync\(npmCmd,\s*\[['"]run['"],\s*['"]build['"]\]/m.test(installContent) &&
|
|
/cwd:\s*sdkDir/.test(installContent);
|
|
assert.ok(
|
|
!hasBuildStep,
|
|
'bin/install.js must not run `npm run build` in sdk/ at install time. ' +
|
|
'TypeScript compilation happens at publish time via prepublishOnly.'
|
|
);
|
|
});
|
|
|
|
test('installSdkIfNeeded checks sdk/dist/cli.js exists instead of building', () => {
|
|
assert.ok(
|
|
installContent.includes('sdk/dist/cli.js') || installContent.includes("'dist', 'cli.js'"),
|
|
'installSdkIfNeeded() must reference sdk/dist/cli.js to verify the prebuilt dist.'
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('fix #2441: back-compat shim — parent package bin entry', () => {
|
|
test('root package.json declares gsd-sdk bin entry', () => {
|
|
assert.ok(
|
|
rootPkg.bin && rootPkg.bin['gsd-sdk'],
|
|
'root package.json must have a bin["gsd-sdk"] entry for the back-compat shim.'
|
|
);
|
|
});
|
|
|
|
test('gsd-sdk bin entry points at bin/gsd-sdk.js', () => {
|
|
assert.equal(
|
|
rootPkg.bin['gsd-sdk'],
|
|
'bin/gsd-sdk.js',
|
|
'bin["gsd-sdk"] must point at bin/gsd-sdk.js'
|
|
);
|
|
});
|
|
|
|
test('bin/gsd-sdk.js shim file exists', () => {
|
|
assert.ok(
|
|
fs.existsSync(GSD_SDK_SHIM),
|
|
'bin/gsd-sdk.js must exist as the back-compat PATH shim.'
|
|
);
|
|
});
|
|
|
|
test('bin/gsd-sdk.js resolves sdk/dist/cli.js relative to itself', () => {
|
|
const shimContent = fs.readFileSync(GSD_SDK_SHIM, 'utf-8');
|
|
// Require the actual path.resolve call with the expected segments, not
|
|
// loose substring matches that would pass from comments or shebangs.
|
|
assert.match(
|
|
shimContent,
|
|
/path\.resolve\(\s*__dirname\s*,\s*['"]\.\.['"]\s*,\s*['"]sdk['"]\s*,\s*['"]dist['"]\s*,\s*['"]cli\.js['"]\s*\)/,
|
|
'bin/gsd-sdk.js must call path.resolve(__dirname, "..", "sdk", "dist", "cli.js") to locate the prebuilt CLI.'
|
|
);
|
|
});
|
|
|
|
test('bin/gsd-sdk.js invokes cli.js via spawnSync(process.execPath, ...)', () => {
|
|
const shimContent = fs.readFileSync(GSD_SDK_SHIM, 'utf-8');
|
|
// The shim must invoke via node (not rely on execute bit), which means
|
|
// spawnSync(process.execPath, [cliPath, ...args]).
|
|
assert.match(
|
|
shimContent,
|
|
/spawnSync\(\s*process\.execPath\s*,/,
|
|
'bin/gsd-sdk.js must spawn node via process.execPath so the execute bit on cli.js is irrelevant (#2453).'
|
|
);
|
|
assert.match(
|
|
shimContent,
|
|
/process\.argv\.slice\(\s*2\s*\)/,
|
|
'bin/gsd-sdk.js must forward user args via process.argv.slice(2).'
|
|
);
|
|
assert.match(
|
|
shimContent,
|
|
/process\.exit\(/,
|
|
'bin/gsd-sdk.js must propagate the child exit status via process.exit.'
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('fix #2441: sdk/dist shipped in tarball', () => {
|
|
test('root package.json files includes sdk/dist', () => {
|
|
assert.ok(
|
|
Array.isArray(rootPkg.files) && rootPkg.files.some(f => f === 'sdk/dist' || f.startsWith('sdk/dist')),
|
|
'root package.json files must include "sdk/dist" so the prebuilt CLI ships in the tarball.'
|
|
);
|
|
});
|
|
|
|
test('root package.json files still includes sdk/src (for dev/clone builds)', () => {
|
|
assert.ok(
|
|
Array.isArray(rootPkg.files) && rootPkg.files.some(f => f === 'sdk/src' || f.startsWith('sdk/src')),
|
|
'root package.json files should still include sdk/src for developer builds.'
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('fix #2453: sdk/package.json prepublishOnly guards execute bit', () => {
|
|
test('sdk prepublishOnly deletes old dist before build (npm stale-prepublishOnly guard)', () => {
|
|
const prepub = sdkPkg.scripts && sdkPkg.scripts.prepublishOnly;
|
|
assert.ok(
|
|
prepub && prepub.includes('rm -rf dist'),
|
|
'sdk/package.json prepublishOnly must start with `rm -rf dist` to avoid stale build output.'
|
|
);
|
|
});
|
|
|
|
test('sdk prepublishOnly chmods dist/cli.js after tsc', () => {
|
|
const prepub = sdkPkg.scripts && sdkPkg.scripts.prepublishOnly;
|
|
assert.ok(
|
|
prepub && prepub.includes('chmod +x dist/cli.js'),
|
|
'sdk/package.json prepublishOnly must run `chmod +x dist/cli.js` after tsc to fix mode-644 (#2453).'
|
|
);
|
|
});
|
|
|
|
test('sdk prepublishOnly runs tsc', () => {
|
|
const prepub = sdkPkg.scripts && sdkPkg.scripts.prepublishOnly;
|
|
assert.ok(
|
|
prepub && prepub.includes('tsc'),
|
|
'sdk/package.json prepublishOnly must include tsc to compile TypeScript.'
|
|
);
|
|
});
|
|
});
|