diff --git a/__tests__/initQmScaffold.test.ts b/__tests__/initQmScaffold.test.ts new file mode 100644 index 0000000..77e3cb2 --- /dev/null +++ b/__tests__/initQmScaffold.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, test } from "bun:test"; +import { + customImageBoots, + dropTrailingCommas, + imageSkipReason, + stripJsonComments, +} from "../src/cli/init-qm.js"; + +// A Dockerfile that cannot run is worse than no Dockerfile. It reads as the +// supported install path, so when the binary turns out to be missing the +// operator debugs the file rather than learning the image was never used. + +describe("stripJsonComments", () => { + test("a URL's // survives", () => { + // The classic failure: naive comment-stripping eats the second slash of + // https:// and turns a valid config into a parse error, in a file the + // operator did not know we read. + const src = '{ "publicUrl": "https://example.com/x", "target": "docker" }'; + expect(JSON.parse(stripJsonComments(src))).toEqual({ + publicUrl: "https://example.com/x", + target: "docker", + }); + }); + + test("line and block comments go", () => { + const src = `{ + // which backend + "target": "fly", /* inline */ + "sandbox": { "backend": "sprites" } // trailing + }`; + expect(JSON.parse(stripJsonComments(src))).toEqual({ + target: "fly", + sandbox: { backend: "sprites" }, + }); + }); + + test("an escaped quote inside a string does not end it", () => { + const src = '{ "note": "a \\" then // not a comment", "target": "docker" }'; + expect(JSON.parse(stripJsonComments(src)).target).toBe("docker"); + }); +}); + +describe("dropTrailingCommas", () => { + test("a hand-edited config with trailing commas still parses", () => { + // Without this the config is unreadable, which fails open and writes the + // Dockerfile we were trying not to write — the opposite of the intent. + const src = '{ "target": "docker", "sandbox": { "backend": "sprites", }, }'; + expect(JSON.parse(dropTrailingCommas(src))).toEqual({ + target: "docker", + sandbox: { backend: "sprites" }, + }); + }); + + test("a comma inside a string is left alone", () => { + const src = '{ "note": "a, b, ]", "target": "docker" }'; + expect(JSON.parse(dropTrailingCommas(src)).note).toBe("a, b, ]"); + }); +}); + +describe("the deployment shapes that actually ship", () => { + test("v1: target docker with the sprites backend skips the Dockerfile", () => { + // This is the combination our own guide documents, so it is the one that + // must not regress. `target` says where the control plane runs; `backend` + // says where sandboxes run, and only the latter decides this. + expect(customImageBoots({ target: "docker", backend: "sprites" })).toBe(false); + expect(imageSkipReason({ target: "docker", backend: "sprites" })) + .toBe("sprites-ignores-image"); + }); + + test("an AWS control plane with Sprites sandboxes is NOT MicroVM", () => { + // `backend: "aws"` requires `target: "aws"`, but not the reverse + // (config.js:1106). Reporting this shape as MicroVM would tell the operator + // the CLI can never be installed, which is false — Sprites install it on + // first use. + expect(imageSkipReason({ target: "aws", backend: "sprites" })) + .toBe("sprites-ignores-image"); + }); + + test("Lambda MicroVM is reported as itself", () => { + expect(imageSkipReason({ target: "aws", backend: "aws" })).toBe("aws-microvm"); + }); + + test("a plain local docker deploy still gets a Dockerfile", () => { + expect(imageSkipReason({ target: "docker" })).toBeNull(); + }); +}); + +describe("customImageBoots", () => { + test("Sprites cannot boot a custom image", () => { + // yc-software/qm#272 — the published image is ignored, the stock base boots. + expect(customImageBoots({ backend: "sprites" })).toBe(false); + expect(customImageBoots({ target: "fly" })).toBe(false); + }); + + test("AWS MicroVM cannot install a tool binary", () => { + // yc-software/qm#350 — no install mechanism exists at all. + expect(customImageBoots({ target: "aws" })).toBe(false); + expect(customImageBoots({ backend: "aws" })).toBe(false); + }); + + test("a local docker target still gets the Dockerfile", () => { + expect(customImageBoots({ target: "docker" })).toBe(true); + }); + + test("an unreadable or absent config scaffolds as before", () => { + // Guessing wrong in this direction removes a file someone needs, so an + // unknown shape must keep the previous behaviour rather than assume. + expect(customImageBoots(null)).toBe(true); + expect(customImageBoots({})).toBe(true); + expect(customImageBoots({ target: "something-new" })).toBe(true); + }); +}); diff --git a/qm/README.md b/qm/README.md index 16837b4..875f822 100644 --- a/qm/README.md +++ b/qm/README.md @@ -13,9 +13,26 @@ Three files that teach a QM agent to use PipesHub: | File | Job | | --- | --- | -| `sandbox/Dockerfile` | installs the `pipeshub` command into the agent's sandbox | -| `sandbox/tools/pipeshub/tool.json` | tells QM the command exists, and which uses need approval | +| `sandbox/tools/pipeshub/tool.json` | tells QM the command exists, which hosts it may reach, and which uses need approval | | `sandbox/skills/pipeshub/SKILL.md` | tells the agent when to reach for it and how to read its results | +| `sandbox/Dockerfile` | installs the `pipeshub` command — **only on deployments that can boot a custom image**, see below | + +`init-qm` writes the Dockerfile only where it would actually run. On Fly +Sprites and AWS MicroVM sandboxes it is skipped, because a file that looks +like the install path but never executes sends you debugging the wrong +thing when the command turns up missing. There, `SKILL.md` installs the CLI +on first use. + +That reflects how QM behaves today. If +[qm#272](https://github.com/yc-software/qm/issues/272) is fixed so Sprites +boot a published image, the Dockerfile becomes the install path again and +this changes with it. + +**If you scaffolded with an earlier version**, you already have a +`sandbox/Dockerfile` on a deployment that cannot use it. Delete it — or +remove just the PipesHub install block if the rest of the file is yours. +Upcoming QM validation rejects that file rather than ignoring it, so +leaving it will fail `qm check`. Re-running `init-qm` points this out. This is a **deployment-layer folder** — you copy it into your QM deployment directory and `qm up`. It is *not* a git skill pack. QM supports both, they @@ -90,14 +107,15 @@ Dockerfile is the install path and this first-run step can be dropped. pipeshub init-qm /path/to/your-qm-deployment ``` - Use this rather than copying by hand: it stamps the Dockerfile's version pin - from the package you just installed, so the folder and the CLI it describes - cannot end up on different versions. + Use this rather than copying by hand: it reads your `qm.config.jsonc` to + decide whether a Dockerfile is worth writing, and where it is, stamps the + version pin from the package you just installed — so the folder and the CLI + it describes cannot end up on different versions. Re-running is safe. Files that already exist are kept and listed rather than - overwritten (`--force` overrides). An existing `sandbox/Dockerfile` is - appended to, not replaced — and left alone entirely if it already installs - the CLI. + overwritten (`--force` overrides). Where a Dockerfile does apply, an existing + `sandbox/Dockerfile` is appended to, not replaced — and left alone entirely + if it already installs the CLI. 2. **Deliver the PipesHub origin** as an environment variable that actually reaches the sandbox. An origin with no path — the CLI appends `/mcp` @@ -217,6 +235,12 @@ keychain entry under the wrong service name. ## Air-gapped and locked-down networks +Both install paths reach the public npm registry — `sandbox/Dockerfile` at +build time where one is written, and the skill's first-run step at turn time +on Sprites. On a locked-down network the first-run step is the one to think +about, because it runs inside the sandbox with whatever egress that sandbox +has. + `sandbox/Dockerfile` installs the CLI from the public npm registry at build time. If your network blocks that, you can point it at an internal registry mirror or vendor the tarball into the image. All that matters is that a working diff --git a/qm/docs/use-with-qm.md b/qm/docs/use-with-qm.md index f36eb29..a3de357 100644 --- a/qm/docs/use-with-qm.md +++ b/qm/docs/use-with-qm.md @@ -75,8 +75,14 @@ npm install -g @pipeshub-ai/mcp pipeshub init-qm . ``` -That writes the tool, the skill, and a Dockerfile pinned to the same CLI -version. Re-running is safe: existing files are kept. +That writes the tool and the skill. On Sprites it deliberately does **not** +write a `sandbox/Dockerfile`: Sprites cannot boot a custom image +([qm#272](https://github.com/yc-software/qm/issues/272)), so the file would +look like the install path while never running — the skill's first-run step +is what actually installs the CLI. On a deployment that can build its own +image, the Dockerfile is written and pinned to the CLI version you installed. + +Re-running is safe: existing files are kept. Set `egress` in `sandbox/tools/pipeshub/tool.json` to your PipesHub hostname only — for example `pipeshub.your-company.com` or diff --git a/src/cli/init-qm.ts b/src/cli/init-qm.ts index 8ac4867..cf3e208 100644 --- a/src/cli/init-qm.ts +++ b/src/cli/init-qm.ts @@ -89,10 +89,147 @@ RUN npm install -g "@pipeshub-ai/mcp@${version}" \\ # ---------------------------------------------------------------------------- `; +/** + * Strip JSONC comments without mangling `//` inside string values. + * + * The naive version eats the second slash of `"https://example.com"` and turns + * a valid config into a parse error, which is a maddening failure to debug in + * a file the operator did not know we read. + */ +export function stripJsonComments(src: string): string { + let out = ""; + let inString = false; + let inLine = false; + let inBlock = false; + for (let i = 0; i < src.length; i++) { + const c = src[i]; + const next = src[i + 1]; + if (inLine) { + if (c === "\n") { inLine = false; out += c; } + continue; + } + if (inBlock) { + if (c === "*" && next === "/") { inBlock = false; i++; } + continue; + } + if (inString) { + out += c; + if (c === "\\") { out += next ?? ""; i++; continue; } + if (c === '"') inString = false; + continue; + } + if (c === '"') { inString = true; out += c; continue; } + if (c === "/" && next === "/") { inLine = true; i++; continue; } + if (c === "/" && next === "*") { inBlock = true; i++; continue; } + out += c; + } + return out; +} + +/** + * Drop trailing commas before `}` or `]`. JSONC allows them and hand-edited + * configs collect them; `JSON.parse` does not. Without this a stray comma makes + * the config unreadable, which fails open and writes the Dockerfile we were + * trying not to write. + */ +export function dropTrailingCommas(src: string): string { + let out = ""; + let inString = false; + for (let i = 0; i < src.length; i++) { + const c = src[i]; + if (inString) { + out += c; + if (c === "\\") { out += src[i + 1] ?? ""; i++; continue; } + if (c === '"') inString = false; + continue; + } + if (c === '"') { inString = true; out += c; continue; } + if (c === ",") { + let j = i + 1; + while (j < src.length && /\s/.test(src[j] as string)) j++; + if (src[j] === "}" || src[j] === "]") continue; // drop it + } + out += c; + } + return out; +} + +export interface DeploymentShape { + target?: string | undefined; + backend?: string | undefined; +} + +/** + * Read `target` and `sandbox.backend` from the operator's existing + * qm.config.jsonc. Returns null when there is no readable config — a fresh + * directory, or a file we cannot parse. Never throws: this only decides how + * much to scaffold, and a config we cannot read must not stop the scaffold. + */ +export async function readDeploymentShape( + dest: string, +): Promise { + try { + const raw = await readFile(join(dest, "qm.config.jsonc"), "utf8"); + const cfg = JSON.parse(dropTrailingCommas(stripJsonComments(raw))) as { + target?: unknown; + sandbox?: { backend?: unknown }; + }; + const target = typeof cfg.target === "string" ? cfg.target : undefined; + const backend = typeof cfg.sandbox?.backend === "string" + ? cfg.sandbox.backend + : undefined; + if (!target && !backend) return null; + return { target, backend }; + } catch { + return null; + } +} + +/** + * Why a custom sandbox image cannot boot for this deployment, or null when it + * can. A reason rather than a boolean so the report can never describe a + * deployment as something it is not. + * + * `backend` is what decides where sandboxes run, not `target`: the CLI requires + * `"backend": "aws"` to have `target: "aws"` but not the reverse, so an AWS + * control plane running Sprites sandboxes is a supported and different thing + * from Lambda MicroVMs (`config.js:1106-1112`). + * + * This describes QM as it behaves today. If yc-software/qm#272 is fixed so + * Sprites boot a published image, the Sprites case here stops being true. + */ +export type ImageSkipReason = "sprites-ignores-image" | "aws-microvm" | null; + +export function imageSkipReason(shape: DeploymentShape | null): ImageSkipReason { + if (!shape) return null; + // Lambda MicroVMs have no install mechanism at all (qm#350). + if (shape.backend === "aws") return "aws-microvm"; + // Sprites boot the stock base and ignore a published image (qm#272). + if (shape.backend === "sprites" || shape.target === "fly") { + return "sprites-ignores-image"; + } + // An AWS target with no backend declared cannot run agents yet; skip rather + // than write a file whose fate depends on a choice not made. + if (shape.target === "aws") return "sprites-ignores-image"; + return null; +} + +/** + * Whether to scaffold a Dockerfile. Unknown shapes scaffold as before: + * guessing wrong in that direction removes a file someone needs. + */ +export function customImageBoots(shape: DeploymentShape | null): boolean { + return imageSkipReason(shape) === null; +} + export interface InitResult { written: string[]; skipped: string[]; - dockerfileAction: "created" | "appended" | "manual"; + dockerfileAction: "created" | "appended" | "manual" | "skipped-unusable"; + shape: DeploymentShape | null; + skipReason: ImageSkipReason; + /** A Dockerfile an earlier version already wrote, on a deploy that cannot use it. */ + staleDockerfile: boolean; version: string; configFragment: string; } @@ -112,6 +249,7 @@ export async function initQm( const version = await packageVersion(root); const dest = resolve(targetDir); + const shape = await readDeploymentShape(dest); const written: string[] = []; const skipped: string[] = []; @@ -133,7 +271,17 @@ export async function initQm( // has no PipesHub block yet, and otherwise leave it entirely alone. const dockerfile = join(dest, "sandbox", "Dockerfile"); let dockerfileAction: InitResult["dockerfileAction"]; - if (!await exists(dockerfile)) { + const skipReason = imageSkipReason(shape); + // An earlier version wrote this unconditionally. Leaving it is not neutral: + // upcoming QM validation rejects it outright, so the operator needs telling. + // Not deleted here — it is their file and may carry their own build steps. + let staleDockerfile = false; + if (skipReason !== null) { + // Deliberately not written. The skill installs the CLI on first use, which + // is the path that actually runs on these deployments. + staleDockerfile = await exists(dockerfile); + dockerfileAction = "skipped-unusable"; + } else if (!await exists(dockerfile)) { await copyFile(join(bundle, "sandbox", "Dockerfile"), dockerfile); // Restamp the pin so it matches the version actually installed. const body = await readFile(dockerfile, "utf8"); @@ -170,7 +318,16 @@ export async function initQm( ); } - return { written, skipped, dockerfileAction, version, configFragment }; + return { + written, + skipped, + dockerfileAction, + version, + configFragment, + shape, + skipReason, + staleDockerfile, + }; } export function renderInitReport(dest: string, r: InitResult): string { @@ -182,6 +339,37 @@ export function renderInitReport(dest: string, r: InitResult): string { for (const f of r.skipped) lines.push(` kept ${f} (already existed — use --force to replace)`); lines.push(""); + if (r.dockerfileAction === "skipped-unusable") { + if (r.skipReason === "aws-microvm") { + lines.push("No sandbox/Dockerfile was written: AWS Lambda MicroVM sandboxes"); + lines.push("have no way to install a binary, so the file could never run."); + } else { + lines.push("No sandbox/Dockerfile was written: Fly Sprites boot the stock"); + lines.push("image and ignore a published one, so the file would look like the"); + lines.push("install path while never running."); + } + lines.push("The skill installs the CLI on first use instead — that is the line"); + lines.push("that actually executes, and it needs nothing from you."); + lines.push(""); + } + + if (r.staleDockerfile) { + lines.push("ACTION NEEDED: sandbox/Dockerfile already exists here, written by an"); + lines.push("earlier version. This deployment cannot use it, and upcoming QM"); + lines.push("validation rejects it rather than ignoring it — `qm check` will fail"); + lines.push("with an error naming that file. Delete it, or remove the PipesHub"); + lines.push("install block if the rest of it is yours."); + lines.push(""); + } + + if (r.skipReason === "aws-microvm") { + lines.push("Heads up on AWS: with Lambda MicroVM sandboxes the CLI cannot be"); + lines.push("installed at all (yc-software/qm#350). The tool's guidance, network"); + lines.push("allowlist, and approval rules still apply, but the binary will be"); + lines.push("missing. The sprites backend is what this bundle is tested against."); + lines.push(""); + } + if (r.dockerfileAction === "appended") { lines.push("Appended the install block to your existing sandbox/Dockerfile."); } else if (r.dockerfileAction === "manual") { @@ -206,8 +394,10 @@ export function renderInitReport(dest: string, r: InitResult): string { lines.push(""); lines.push("Then: qm check && qm up"); lines.push(""); - lines.push("On Sprites, `qm sandbox publish` does not put pipeshub on PATH."); - lines.push("The skill installs the CLI on first use."); + if (r.dockerfileAction !== "skipped-unusable") { + lines.push("On Sprites, `qm sandbox publish` does not put pipeshub on PATH."); + lines.push("The skill installs the CLI on first use."); + } return lines.join("\n"); }