From a8245be48cca067eee2823f55fa4b2f8027a46e2 Mon Sep 17 00:00:00 2001 From: Luong NGUYEN Date: Sun, 19 Apr 2026 08:44:04 +0200 Subject: [PATCH] fix(install): bundle skillgrade via bundledDependencies (#172) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skillgrade was declared as a regular `dependencies` entry, so in the normal case npm would fetch it from the registry on install. That leaves a reinstall silently broken whenever the registry, the local npm cache, or the network path is unhealthy — the exact scenario the reporter hit: `asm eval --runtime init` keeps failing with "skillgrade not installed" after `npm install -g agent-skill-manager`. Adding `bundledDependencies: ["skillgrade"]` ships skillgrade inside the packed tarball (package size grows from ~960kB to ~5.3MB), so `npm install -g` always has a local copy to unpack — offline, air- gapped, or registry-flaky installs all succeed. Also adds three e2e smoke tests to `npm-install-e2e.test.ts`: - `skillgrade/ bin is reachable after install` — guards post-install reachability of the bundled skillgrade entry point. - `tarball embeds skillgrade via bundledDependencies` — the discriminating test: inspects the actual tarball listing so a future drop of `bundledDependencies` is caught immediately. - `eval --runtime init scaffolds eval.yaml via bundled skillgrade` — exercises the end-to-end `asm eval --runtime init` flow, covering acceptance criterion 4 (CI-visible smoke test of the runtime-eval path after a clean global install). Tarball growth is a deliberate trade-off: skillgrade drives the `asm eval --runtime` contract, so it must ship with asm by default. Users who want a leaner install can still override via `ASM_SKILLGRADE_BIN` to point at a system skillgrade. --- package.json | 3 + tests/e2e/npm-install-e2e.test.ts | 126 +++++++++++++++++++++++++++++- 2 files changed, 127 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 8a9cdc7..868367d 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,9 @@ "skillgrade": "^0.1.3", "yaml": "^2.8.3" }, + "bundledDependencies": [ + "skillgrade" + ], "engines": { "node": ">=18", "bun": ">=1.0.0" diff --git a/tests/e2e/npm-install-e2e.test.ts b/tests/e2e/npm-install-e2e.test.ts index ef4139e..084b287 100644 --- a/tests/e2e/npm-install-e2e.test.ts +++ b/tests/e2e/npm-install-e2e.test.ts @@ -11,8 +11,9 @@ import { mkdtemp, rm, readFile, access } from "fs/promises"; import { existsSync, readdirSync } from "fs"; import { tmpdir } from "os"; -// npm pack + install can take a while -setDefaultTimeout(60_000); +// npm pack + install can take a while, and the runtime eval smoke +// test spawns skillgrade (also slow on the first run). +setDefaultTimeout(120_000); const ROOT = resolve(import.meta.dir, "..", ".."); @@ -166,6 +167,73 @@ describe("npm install: package structure", () => { const jsons = readdirSync(dataDir).filter((f) => f.endsWith(".json")); expect(jsons.length).toBeGreaterThanOrEqual(1); }); + + // Regression: issue #172 — reinstall must restore bundled skillgrade so + // `asm eval --runtime` works without a separate install step. The package + // declares `bundledDependencies: ["skillgrade"]`, so the tarball ships the + // full skillgrade tree under node_modules/ and npm preserves it on install. + // + // Note: this only asserts post-install reachability. A regular + // `dependencies` entry would also satisfy this — the discriminating test + // for `bundledDependencies` is the offline-install assertion below. + test("skillgrade/ bin is reachable after install (issue #172)", () => { + if (setupError) throw new Error(setupError); + const skillgradeBin = join( + installDir, + "lib", + "node_modules", + "agent-skill-manager", + "node_modules", + "skillgrade", + "bin", + "skillgrade.js", + ); + expect(existsSync(skillgradeBin)).toBe(true); + }); + + // Discriminating test for `bundledDependencies` (issue #172): the packed + // tarball itself must contain skillgrade's files, so the package can be + // installed offline / into an air-gapped CI runner / onto a failing + // registry mirror. Without `bundledDependencies`, npm pack would omit + // node_modules/ and this check would fail. + test("tarball embeds skillgrade via bundledDependencies (issue #172)", async () => { + if (setupError) throw new Error(setupError); + + // Re-pack to a throwaway directory so we can inspect the tarball + // contents without coupling to the shared install tarball's lifecycle. + const inspectDir = await mkdtemp(join(tmpdir(), "asm-npm-e2e-pack-")); + try { + const packProc = Bun.spawn( + ["npm", "pack", "--pack-destination", inspectDir, ROOT], + { + stdout: "pipe", + stderr: "pipe", + }, + ); + const packExit = await packProc.exited; + expect(packExit).toBe(0); + + const [packed] = readdirSync(inspectDir).filter((f) => + f.match(/^agent-skill-manager-.*\.tgz$/), + ); + expect(packed).toBeTruthy(); + + const listProc = Bun.spawn(["tar", "-tzf", join(inspectDir, packed)], { + stdout: "pipe", + stderr: "pipe", + }); + const listing = await new Response(listProc.stdout).text(); + + // Both the entry point and package metadata must ship inside the + // tarball — simply having one could be a partial-match false positive. + expect(listing).toContain( + "package/node_modules/skillgrade/bin/skillgrade.js", + ); + expect(listing).toContain("package/node_modules/skillgrade/package.json"); + } finally { + await rm(inspectDir, { recursive: true, force: true }); + } + }); }); // ─── Command tests via installed binary ───────────────────────────────────── @@ -265,6 +333,60 @@ describe("npm install: asm init", () => { }); }); +// ─── Runtime eval flow (bundled skillgrade, issue #172) ───────────────────── +// +// Regression smoke test for issue #172 acceptance criterion 4: verifies that +// a clean global install produces a working `asm eval --runtime` flow — no +// separate skillgrade install required. `asm eval --runtime init` +// is the cheapest exercise of the bundled binary: it scaffolds eval.yaml +// without making LLM calls, so it's deterministic and CI-safe. + +describe("npm install: asm eval --runtime (bundled skillgrade)", () => { + let evalWorkspace: string; + + beforeAll(async () => { + evalWorkspace = await mkdtemp(join(tmpdir(), "asm-npm-e2e-eval-")); + }); + + afterAll(async () => { + if (evalWorkspace) { + await rm(evalWorkspace, { recursive: true, force: true }); + } + }); + + test("eval --runtime init scaffolds eval.yaml via bundled skillgrade", async () => { + if (setupError) throw new Error(setupError); + + // Minimal SKILL.md fixture — skillgrade init reads this and drafts + // eval.yaml without invoking any LLM. + const skillDir = join(evalWorkspace, "bundled-skillgrade-skill"); + const { exitCode: mkExit } = await runAsm( + "init", + "bundled-skillgrade-skill", + "--path", + skillDir, + ); + expect(mkExit).toBe(0); + + const { exitCode, stdout, stderr } = await runAsm( + "eval", + skillDir, + "--runtime", + "init", + ); + + // Assertion #1: the bundled skillgrade binary was reachable — the + // "skillgrade not installed" error path means bundling regressed. + const combined = `${stdout}\n${stderr}`; + expect(combined).not.toContain("skillgrade not installed"); + + // Assertion #2: scaffold succeeded. + expect(exitCode).toBe(0); + const evalYaml = join(skillDir, "eval.yaml"); + expect(existsSync(evalYaml)).toBe(true); + }); +}); + // ─── Regression: no protocol errors ───────────────────────────────────────── describe("npm install: no Node.js protocol errors", () => {