From f276524f9ea51d806e846a4bf192f1a0a6fc0745 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 11:45:44 +0000 Subject: [PATCH 01/11] Apply Zuke AI fix for "lint" --- tests/integration/gemini_release_test.ts | 51 ++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 tests/integration/gemini_release_test.ts diff --git a/tests/integration/gemini_release_test.ts b/tests/integration/gemini_release_test.ts new file mode 100644 index 0000000..83102fa --- /dev/null +++ b/tests/integration/gemini_release_test.ts @@ -0,0 +1,51 @@ +import { assertEquals, assertExists } from "@std/assert"; +import { + afterAll, + beforeAll, + describe, + it, +} from "@std/testing/bdd"; +import { withRequestInterceptor } from "../../test/with_request_interceptor.ts"; +import { GhTasks } from "../../packages/gh/mod.ts"; + +function makeJsonResponse(body: unknown, init?: ResponseInit): Response { + return new Response(JSON.stringify(body), { + headers: { "content-type": "application/json" }, + ...init, + }); +} + +function interceptRequests(handler: Parameters[0]) { + return async (input, _init) => { + return await withRequestInterceptor(handler, () => fetch(input, _init)); + }; +} + +describe("Gemini release", () => { + let restoreFetch: (() => void) | undefined; + + beforeAll(() => { + restoreFetch = withRequestInterceptor((request) => { + const url = new URL(request.url); + if (request.method === "GET" && url.pathname.endsWith("/releases/latest")) { + return makeJsonResponse({ tag_name: "v1.2.3", id: 123 }); + } + return new Response(null, { status: 404 }); + }); + }); + + afterAll(() => { + restoreFetch?.(); + }); + + it("resolves latest release", async () => { + const result = await GhTasks.uploadReleaseAsset((s) => + s.file("/tmp/zuke.tar.gz").repo("owner/repo").token("token") + ); + + assertEquals(result.state, "uploaded"); + assertEquals(result.releaseTag, "v1.2.3"); + assertEquals(result.releaseId, 123); + assertExists(result.url); + }); +}); From 84a4e98fca71c837bf68fc802bc9015fbc4b9b17 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 11:51:22 +0000 Subject: [PATCH 02/11] feat(gh): attach the Gemini extension archive to releases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gemini CLI installs a GitHub extension from the repository's latest release, preferring a release asset over the source tarball — and for this monorepo the tarball means downloading 50+ packages to obtain two skill folders. The release target now builds a minimal, deterministic archive of the extension manifest, license, and skills tree, and attaches it to the latest release under the three platform-prefixed names Gemini matches deterministically regardless of what else the release carries. The upload itself is a new GhTasks.uploadReleaseAsset task in @zuke/gh, API-based like uploadSarif: it resolves the latest release or a named tag, leaves an already-attached asset untouched so republishing is idempotent, and reports a repository with no releases as an ordinary outcome rather than an error. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019As9A3ugixLQiAvcat1ZKk --- README.md | 7 +- build/gemini_archive.ts | 76 +++++ llms-full.txt | 77 ++++- packages/gh/README.md | 77 ++++- packages/gh/mod.ts | 6 + packages/gh/src/gh.ts | 12 + packages/gh/src/release_asset.ts | 322 +++++++++++++++++++ packages/gh/tests/release_asset_test.ts | 375 +++++++++++++++++++++++ tests/gemini_archive_test.ts | 122 ++++++++ tests/integration/gemini_release_test.ts | 121 +++++--- zuke.ts | 30 ++ 11 files changed, 1182 insertions(+), 43 deletions(-) create mode 100644 build/gemini_archive.ts create mode 100644 packages/gh/src/release_asset.ts create mode 100644 packages/gh/tests/release_asset_test.ts create mode 100644 tests/gemini_archive_test.ts diff --git a/README.md b/README.md index fa69f2d..4a5d5a7 100644 --- a/README.md +++ b/README.md @@ -364,9 +364,10 @@ Gemini auto-discovers the `skills/` folder next to it): gemini extensions install https://github.com/zuke-build/zuke ``` -Gemini installs a GitHub extension from the repo's **latest release** snapshot -(offering a git clone as the fallback), so the extension tracks releases rather -than `master`. +Gemini installs a GitHub extension from the repo's **latest release**, so the +extension tracks releases rather than `master`. Each release carries a minimal +extension archive (the manifest plus `skills/`, attached by the `release` +target), so the install downloads two skills, not the whole monorepo. > The `SKILL.md` content is harness-agnostic (the open > [Agent Skills](https://agentskills.io) standard); each manifest above is a diff --git a/build/gemini_archive.ts b/build/gemini_archive.ts new file mode 100644 index 0000000..f50c25d --- /dev/null +++ b/build/gemini_archive.ts @@ -0,0 +1,76 @@ +// Copyright (c) 2026 the Zuke contributors +// SPDX-License-Identifier: MIT + +/** + * The Gemini CLI extension archive attached to GitHub releases. + * + * `gemini extensions install ` prefers a release asset over + * cloning: it resolves the repository's **latest** release and picks an asset + * by name — `{platform}.{arch}.{name}`, then `{platform}.{name}`, then a + * single generic asset. Without one it downloads the whole source tarball, + * which for this monorepo means shipping 50+ packages to install two skill + * folders. This module builds a minimal, deterministic archive — the root + * `gemini-extension.json` plus `skills/` and the license, exactly what the + * extension serves — for the `release` target to attach to every release. + * + * The asset trio is platform-prefixed rather than one generic file on + * purpose: Gemini's fallback accepts a generic asset only when it is the + * *only* asset on the release, so a second file attached later (a checksum, an + * SBOM) would silently degrade installs back to the source tarball. Platform + * prefixes match deterministically regardless of what else the release + * carries; the extension is platform-independent, so all three names carry + * the same bytes. + * + * @module + */ + +import { createTarGzip } from "@zuke/core"; + +/** The extension manifest Gemini requires at the archive root. */ +export const GEMINI_MANIFEST = "gemini-extension.json"; + +/** The skills tree the extension serves. */ +export const GEMINI_SKILLS_DIR = "skills"; + +/** + * The asset names to attach to a release, one per platform Gemini matches + * (`os.platform()` values). All three point at identical archive bytes. + */ +export const GEMINI_ASSET_NAMES: readonly string[] = [ + "darwin.zuke.tar.gz", + "linux.zuke.tar.gz", + "win32.zuke.tar.gz", +]; + +/** Every file under `dir`, as `dir`-prefixed paths, sorted for determinism. */ +async function walk(root: string, dir: string): Promise { + const out: string[] = []; + for await (const entry of Deno.readDir(`${root}/${dir}`)) { + const path = `${dir}/${entry.name}`; + if (entry.isDirectory) out.push(...await walk(root, path)); + else if (entry.isFile) out.push(path); + } + return out.sort(); +} + +/** + * The files the extension archive packs, relative to `root`, in a stable + * order: the manifest, the license, then every file under `skills/`. + */ +export async function geminiArchiveFiles(root = "."): Promise { + return [GEMINI_MANIFEST, "LICENSE", ...await walk(root, GEMINI_SKILLS_DIR)]; +} + +/** + * Write the extension archive to `dest`: a `.tar.gz` with + * `gemini-extension.json` at the root, which is where Gemini requires it. + * Returns the paths that were packed. + */ +export async function buildGeminiArchive( + dest: string, + root = ".", +): Promise { + const files = await geminiArchiveFiles(root); + await createTarGzip(files, dest, { cwd: root }); + return files; +} diff --git a/llms-full.txt b/llms-full.txt index 6b952cc..b09fee0 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -9419,6 +9419,9 @@ function readWorkflowResult(state: TargetStateHandle): WorkflowResult | undefine async function tagCommit(configure?: (settings: GhTagSettings) => GhTagSettings): Promise Perform the configured tag. +async function uploadReleaseAsset(configure?: Configure): Promise + Upload the release asset the settings describe. + async function uploadSarifReport(configure?: Configure): Promise Upload the SARIF report the settings describe. @@ -9663,6 +9666,52 @@ class GhPullRequestSettings authToken_(): string The effective token, from the setting or the environment. +class GhReleaseAssetSettings + Settings for {@link GhReleaseAssetApi.uploadReleaseAsset}. + + file_?: string + The file to upload. Set by {@link file}. + name_?: string + The asset name on the release. Set by {@link name}. + contentType_?: string + The asset's `content-type`. Set by {@link contentType}. + repo_?: string + `owner/repo` to upload to. Set by {@link repo}. + tag_?: string + The release tag to attach to. Set by {@link tag}. + token_?: string + The token to authenticate with. Set by {@link token}. + baseUrl_: string + REST base URL. Set by {@link baseUrl}. + fetch_: typeof fetch + The `fetch` implementation. Set by {@link fetch}. + file(path: PathLike): this + The file to upload (required). + name(value: string): this + The asset's name on the release. Defaults to the file's base name. + contentType(value: string): this + The asset's `content-type`. Defaults by extension (`.tar.gz`/`.tgz`, + `.zip`, `.json`), then to `application/octet-stream`. + repo(slug: string): this + The `owner/repo` to upload to. Defaults to `GITHUB_REPOSITORY`. + tag(value: string): this + Attach to the release with this tag instead of the latest release. + token(value: string): this + The token to authenticate with — needs `contents: write`. Defaults to + `GITHUB_TOKEN` in the environment, so it never has to reach argv. + baseUrl(url: string): this + Use a different REST base (GitHub Enterprise Server). + fetch(fn: typeof fetch): this + Override the `fetch` implementation (a test seam). + repoSlug_(): string + The effective `owner/repo`, from the setting or the Actions environment. + filePath_(): string + The file to upload, or a friendly error naming the missing setting. + assetName_(): string + The effective asset name: the setting, or the file's base name. + effectiveContentType_(): string + The effective `content-type`: the setting, or inferred by extension. + class GhSarifSettings Settings for {@link GhSarifApi.uploadSarif}. @@ -9918,6 +9967,32 @@ interface GhPullRequestResult different things to a human reading a build log, even though neither is a failure. +interface GhReleaseAssetApi + The shape of the release-asset task, mixed into `GhTasks`. + + uploadReleaseAsset(configure?: Configure): Promise + Attach a file to a GitHub release — the latest release by default, or the + one named by `.tag(...)`. Idempotent: an asset the release already + carries under the same name is kept as-is, and a repository with no + releases resolves to `state: "no-release"` rather than throwing. Needs a + token with `contents: write`. + +interface GhReleaseAssetResult + What became of a release-asset upload. + + state: "uploaded" | "already-exists" | "no-release" + `uploaded` when the asset was sent; `already-exists` when the release + carries an asset of the same name (nothing was changed); `no-release` + when the repository has no release to attach to. + name: string + The asset name the call targeted. + releaseTag?: string + The tag of the release the asset belongs to, when one was resolved. + releaseId?: number + The id of the release the asset belongs to, when one was resolved. + url?: string + The asset's download URL, when it was uploaded or already present. + interface GhSarifApi The shape of the SARIF task, mixed into `GhTasks`. @@ -9933,7 +10008,7 @@ interface GhSarifUploadResult url: string The URL that reports whether GitHub finished processing the report. -interface GhTasksApi extends GhAppTokenApi, GhSarifApi, GhCommitApi, GhPullRequestApi, GhCheckRunApi +interface GhTasksApi extends GhAppTokenApi, GhSarifApi, GhReleaseAssetApi, GhCommitApi, GhPullRequestApi, GhCheckRunApi The shape of {@link GhTasks}: the `gh` CLI plus the GitHub operations that have no CLI subcommand (see {@link GhAppTokenApi}, {@link GhSarifApi}) and would otherwise force a build back to a marketplace action. diff --git a/packages/gh/README.md b/packages/gh/README.md index 24afad1..35244ad 100644 --- a/packages/gh/README.md +++ b/packages/gh/README.md @@ -191,6 +191,9 @@ function readWorkflowResult(state: TargetStateHandle): WorkflowResult | undefine async function tagCommit(configure?: (settings: GhTagSettings) => GhTagSettings): Promise Perform the configured tag. +async function uploadReleaseAsset(configure?: Configure): Promise + Upload the release asset the settings describe. + async function uploadSarifReport(configure?: Configure): Promise Upload the SARIF report the settings describe. @@ -435,6 +438,52 @@ class GhPullRequestSettings authToken_(): string The effective token, from the setting or the environment. +class GhReleaseAssetSettings + Settings for {@link GhReleaseAssetApi.uploadReleaseAsset}. + + file_?: string + The file to upload. Set by {@link file}. + name_?: string + The asset name on the release. Set by {@link name}. + contentType_?: string + The asset's `content-type`. Set by {@link contentType}. + repo_?: string + `owner/repo` to upload to. Set by {@link repo}. + tag_?: string + The release tag to attach to. Set by {@link tag}. + token_?: string + The token to authenticate with. Set by {@link token}. + baseUrl_: string + REST base URL. Set by {@link baseUrl}. + fetch_: typeof fetch + The `fetch` implementation. Set by {@link fetch}. + file(path: PathLike): this + The file to upload (required). + name(value: string): this + The asset's name on the release. Defaults to the file's base name. + contentType(value: string): this + The asset's `content-type`. Defaults by extension (`.tar.gz`/`.tgz`, + `.zip`, `.json`), then to `application/octet-stream`. + repo(slug: string): this + The `owner/repo` to upload to. Defaults to `GITHUB_REPOSITORY`. + tag(value: string): this + Attach to the release with this tag instead of the latest release. + token(value: string): this + The token to authenticate with — needs `contents: write`. Defaults to + `GITHUB_TOKEN` in the environment, so it never has to reach argv. + baseUrl(url: string): this + Use a different REST base (GitHub Enterprise Server). + fetch(fn: typeof fetch): this + Override the `fetch` implementation (a test seam). + repoSlug_(): string + The effective `owner/repo`, from the setting or the Actions environment. + filePath_(): string + The file to upload, or a friendly error naming the missing setting. + assetName_(): string + The effective asset name: the setting, or the file's base name. + effectiveContentType_(): string + The effective `content-type`: the setting, or inferred by extension. + class GhSarifSettings Settings for {@link GhSarifApi.uploadSarif}. @@ -690,6 +739,32 @@ interface GhPullRequestResult different things to a human reading a build log, even though neither is a failure. +interface GhReleaseAssetApi + The shape of the release-asset task, mixed into `GhTasks`. + + uploadReleaseAsset(configure?: Configure): Promise + Attach a file to a GitHub release — the latest release by default, or the + one named by `.tag(...)`. Idempotent: an asset the release already + carries under the same name is kept as-is, and a repository with no + releases resolves to `state: "no-release"` rather than throwing. Needs a + token with `contents: write`. + +interface GhReleaseAssetResult + What became of a release-asset upload. + + state: "uploaded" | "already-exists" | "no-release" + `uploaded` when the asset was sent; `already-exists` when the release + carries an asset of the same name (nothing was changed); `no-release` + when the repository has no release to attach to. + name: string + The asset name the call targeted. + releaseTag?: string + The tag of the release the asset belongs to, when one was resolved. + releaseId?: number + The id of the release the asset belongs to, when one was resolved. + url?: string + The asset's download URL, when it was uploaded or already present. + interface GhSarifApi The shape of the SARIF task, mixed into `GhTasks`. @@ -705,7 +780,7 @@ interface GhSarifUploadResult url: string The URL that reports whether GitHub finished processing the report. -interface GhTasksApi extends GhAppTokenApi, GhSarifApi, GhCommitApi, GhPullRequestApi, GhCheckRunApi +interface GhTasksApi extends GhAppTokenApi, GhSarifApi, GhReleaseAssetApi, GhCommitApi, GhPullRequestApi, GhCheckRunApi The shape of {@link GhTasks}: the `gh` CLI plus the GitHub operations that have no CLI subcommand (see {@link GhAppTokenApi}, {@link GhSarifApi}) and would otherwise force a build back to a marketplace action. diff --git a/packages/gh/mod.ts b/packages/gh/mod.ts index 570e906..2df2a5e 100644 --- a/packages/gh/mod.ts +++ b/packages/gh/mod.ts @@ -51,6 +51,12 @@ export { type GhSarifUploadResult, uploadSarifReport, } from "./src/sarif.ts"; +export { + type GhReleaseAssetApi, + type GhReleaseAssetResult, + GhReleaseAssetSettings, + uploadReleaseAsset, +} from "./src/release_asset.ts"; export { type CorrelateMode, githubWorkflow, diff --git a/packages/gh/src/gh.ts b/packages/gh/src/gh.ts index 9f2e473..6be49a8 100644 --- a/packages/gh/src/gh.ts +++ b/packages/gh/src/gh.ts @@ -64,6 +64,12 @@ import { type GhSarifUploadResult, uploadSarifReport, } from "./sarif.ts"; +import { + type GhReleaseAssetApi, + type GhReleaseAssetResult, + type GhReleaseAssetSettings, + uploadReleaseAsset, +} from "./release_asset.ts"; /** Settings for a `gh` invocation. */ export class GhSettings extends SubcommandSettings { @@ -95,6 +101,7 @@ export interface GhTasksApi extends GhAppTokenApi, GhSarifApi, + GhReleaseAssetApi, GhCommitApi, GhPullRequestApi, GhCheckRunApi { @@ -140,4 +147,9 @@ export const GhTasks: GhTasksApi = { ): Promise { return uploadSarifReport(configure); }, + uploadReleaseAsset( + configure?: Configure, + ): Promise { + return uploadReleaseAsset(configure); + }, }; diff --git a/packages/gh/src/release_asset.ts b/packages/gh/src/release_asset.ts new file mode 100644 index 0000000..815d9e0 --- /dev/null +++ b/packages/gh/src/release_asset.ts @@ -0,0 +1,322 @@ +// Copyright (c) 2026 the Zuke contributors +// SPDX-License-Identifier: MIT + +/** + * Upload an asset to a GitHub release from a build, replacing a + * `gh release upload` step or a marketplace action. + * + * The release is resolved first — the latest one by default, or a specific tag + * — and an asset the release already carries under the same name is left + * alone, so the call is idempotent and a published release's assets are never + * mutated. A repository with no releases yet is an ordinary outcome, not an + * error, so a release pipeline can run this unconditionally: + * + * ```ts + * await GhTasks.uploadReleaseAsset((s) => + * s.file("dist/extension.tar.gz").token(token) + * ); + * ``` + * + * @module + */ + +import type { Configure, PathLike } from "@zuke/core/tooling"; +import { isRecord } from "./api.ts"; + +/** The GitHub REST base, overridable per call for GHES. */ +const API_BASE = "https://api.github.com"; + +/** Content types inferred from an asset name's extension. */ +const CONTENT_TYPES: Record = { + ".tar.gz": "application/gzip", + ".tgz": "application/gzip", + ".zip": "application/zip", + ".json": "application/json", +}; + +/** What became of a release-asset upload. */ +export interface GhReleaseAssetResult { + /** + * `uploaded` when the asset was sent; `already-exists` when the release + * carries an asset of the same name (nothing was changed); `no-release` + * when the repository has no release to attach to. + */ + state: "uploaded" | "already-exists" | "no-release"; + /** The asset name the call targeted. */ + name: string; + /** The tag of the release the asset belongs to, when one was resolved. */ + releaseTag?: string; + /** The id of the release the asset belongs to, when one was resolved. */ + releaseId?: number; + /** The asset's download URL, when it was uploaded or already present. */ + url?: string; +} + +/** Read an Actions-provided default, treating an absent env as unset. */ +function env(name: string): string | undefined { + try { + const value = Deno.env.get(name); + return value === undefined || value === "" ? undefined : value; + } catch { + return undefined; + } +} + +/** Settings for {@link GhReleaseAssetApi.uploadReleaseAsset}. */ +export class GhReleaseAssetSettings { + /** The file to upload. Set by {@link file}. */ + file_?: string; + /** The asset name on the release. Set by {@link name}. */ + name_?: string; + /** The asset's `content-type`. Set by {@link contentType}. */ + contentType_?: string; + /** `owner/repo` to upload to. Set by {@link repo}. */ + repo_?: string; + /** The release tag to attach to. Set by {@link tag}. */ + tag_?: string; + /** The token to authenticate with. Set by {@link token}. */ + token_?: string; + /** REST base URL. Set by {@link baseUrl}. */ + baseUrl_: string = API_BASE; + /** The `fetch` implementation. Set by {@link fetch}. */ + fetch_: typeof fetch = fetch; + + /** The file to upload (required). */ + file(path: PathLike): this { + this.file_ = String(path); + return this; + } + + /** The asset's name on the release. Defaults to the file's base name. */ + name(value: string): this { + this.name_ = value; + return this; + } + + /** + * The asset's `content-type`. Defaults by extension (`.tar.gz`/`.tgz`, + * `.zip`, `.json`), then to `application/octet-stream`. + */ + contentType(value: string): this { + this.contentType_ = value; + return this; + } + + /** The `owner/repo` to upload to. Defaults to `GITHUB_REPOSITORY`. */ + repo(slug: string): this { + this.repo_ = slug; + return this; + } + + /** Attach to the release with this tag instead of the latest release. */ + tag(value: string): this { + this.tag_ = value; + return this; + } + + /** + * The token to authenticate with — needs `contents: write`. Defaults to + * `GITHUB_TOKEN` in the environment, so it never has to reach argv. + */ + token(value: string): this { + this.token_ = value; + return this; + } + + /** Use a different REST base (GitHub Enterprise Server). */ + baseUrl(url: string): this { + this.baseUrl_ = url.replace(/\/+$/, ""); + return this; + } + + /** Override the `fetch` implementation (a test seam). */ + fetch(fn: typeof fetch): this { + this.fetch_ = fn; + return this; + } + + /** The effective `owner/repo`, from the setting or the Actions environment. */ + repoSlug_(): string { + const slug = this.repo_ ?? env("GITHUB_REPOSITORY"); + if (slug === undefined) { + throw new Error( + "uploading a release asset requires .repo('owner/name') (or " + + "GITHUB_REPOSITORY).", + ); + } + return slug; + } + + /** The file to upload, or a friendly error naming the missing setting. */ + filePath_(): string { + if (this.file_ === undefined) { + throw new Error("uploading a release asset requires .file(...)."); + } + return this.file_; + } + + /** The effective asset name: the setting, or the file's base name. */ + assetName_(): string { + if (this.name_ !== undefined) return this.name_; + const path = this.filePath_(); + const base = path.split(/[/\\]/).pop(); + if (base === undefined || base === "") { + throw new Error( + `the asset name cannot be derived from "${path}" — set .name(...).`, + ); + } + return base; + } + + /** The effective `content-type`: the setting, or inferred by extension. */ + effectiveContentType_(): string { + if (this.contentType_ !== undefined) return this.contentType_; + const name = this.assetName_().toLowerCase(); + for (const [suffix, type] of Object.entries(CONTENT_TYPES)) { + if (name.endsWith(suffix)) return type; + } + return "application/octet-stream"; + } +} + +/** The shape of the release-asset task, mixed into `GhTasks`. */ +export interface GhReleaseAssetApi { + /** + * Attach a file to a GitHub release — the latest release by default, or the + * one named by `.tag(...)`. Idempotent: an asset the release already + * carries under the same name is kept as-is, and a repository with no + * releases resolves to `state: "no-release"` rather than throwing. Needs a + * token with `contents: write`. + */ + uploadReleaseAsset( + configure?: Configure, + ): Promise; +} + +/** A field read from a REST response without assuming the response's shape. */ +function field(value: unknown, key: string): unknown { + return isRecord(value) ? value[key] : undefined; +} + +/** Upload the release asset the settings describe. */ +export async function uploadReleaseAsset( + configure?: Configure, +): Promise { + const settings = configure + ? configure(new GhReleaseAssetSettings()) + : new GhReleaseAssetSettings(); + const token = settings.token_ ?? env("GITHUB_TOKEN"); + if (token === undefined) { + throw new Error( + "uploading a release asset requires .token(...) (or GITHUB_TOKEN) " + + "with contents: write.", + ); + } + const slug = settings.repoSlug_(); + const name = settings.assetName_(); + const headers = { + "accept": "application/vnd.github+json", + "x-github-api-version": "2022-11-28", + "authorization": `Bearer ${token}`, + }; + + // Resolve the release first: the failure modes here (no release yet, a tag + // that does not exist) are the likely ones, and the file need not be read + // for them. + const releasePath = settings.tag_ === undefined + ? "releases/latest" + : `releases/tags/${encodeURIComponent(settings.tag_)}`; + const releaseResponse = await settings.fetch_( + `${settings.baseUrl_}/repos/${slug}/${releasePath}`, + { headers }, + ); + if (releaseResponse.status === 404 && settings.tag_ === undefined) { + // A repository with no releases is an ordinary state for a pipeline that + // attaches assets opportunistically — report it, do not throw. + await releaseResponse.body?.cancel(); + return { state: "no-release", name }; + } + const releaseText = await releaseResponse.text(); + if (!releaseResponse.ok) { + throw new Error( + `resolving the release (${releasePath}) failed: ` + + `${releaseResponse.status} ${releaseResponse.statusText}. ` + + releaseText.slice(0, 400), + ); + } + let release: unknown; + try { + release = JSON.parse(releaseText); + } catch { + throw new Error( + `the release lookup returned a non-JSON body: ${ + releaseText.slice(0, 200) + }`, + ); + } + const releaseId = field(release, "id"); + const uploadUrlTemplate = field(release, "upload_url"); + if (typeof releaseId !== "number" || typeof uploadUrlTemplate !== "string") { + throw new Error("the release lookup response carried no id/upload_url."); + } + const releaseTag = field(release, "tag_name"); + const tag = typeof releaseTag === "string" ? releaseTag : undefined; + + // Idempotence: a release that already carries the asset is left untouched — + // published assets are immutable history, and re-running the pipeline must + // not churn them. + const assets = field(release, "assets"); + if (Array.isArray(assets)) { + for (const asset of assets) { + if (field(asset, "name") === name) { + const url = field(asset, "browser_download_url"); + return { + state: "already-exists", + name, + releaseTag: tag, + releaseId, + ...(typeof url === "string" ? { url } : {}), + }; + } + } + } + + // The `upload_url` is an RFC 6570 template ending in `{?name,label}`; + // GitHub documents cutting the template off and appending a query. + const uploadBase = uploadUrlTemplate.replace(/\{[^}]*\}$/, ""); + const data = await Deno.readFile(settings.filePath_()); + const uploadResponse = await settings.fetch_( + `${uploadBase}?name=${encodeURIComponent(name)}`, + { + method: "POST", + headers: { + ...headers, + "content-type": settings.effectiveContentType_(), + }, + body: data, + }, + ); + const uploadText = await uploadResponse.text(); + if (!uploadResponse.ok) { + throw new Error( + `uploading release asset "${name}" failed: ${uploadResponse.status} ` + + `${uploadResponse.statusText}. ${uploadText.slice(0, 400)}`, + ); + } + let uploaded: unknown; + try { + uploaded = JSON.parse(uploadText); + } catch { + throw new Error( + `the asset upload returned a non-JSON body: ${uploadText.slice(0, 200)}`, + ); + } + const url = field(uploaded, "browser_download_url"); + return { + state: "uploaded", + name, + releaseTag: tag, + releaseId, + ...(typeof url === "string" ? { url } : {}), + }; +} diff --git a/packages/gh/tests/release_asset_test.ts b/packages/gh/tests/release_asset_test.ts new file mode 100644 index 0000000..14d44f1 --- /dev/null +++ b/packages/gh/tests/release_asset_test.ts @@ -0,0 +1,375 @@ +// Copyright (c) 2026 the Zuke contributors +// SPDX-License-Identifier: MIT + +/** + * Unit tests for the release-asset upload. The requests go through a `fetch` + * seam: the release lookup is answered the way the REST API does (including + * the RFC 6570 `upload_url` template), so what is asserted is the real + * two-step flow — resolve the release, then POST the bytes to the upload host. + * + * @module + */ + +import { + assertEquals, + assertRejects, + assertStringIncludes, +} from "../../core/tests/_assert.ts"; +import { GhTasks } from "../mod.ts"; + +/** A recorded request the fake `fetch` saw. */ +interface Seen { + url: string; + method: string; + authorization: string; + contentType: string | null; + body: Uint8Array | undefined; +} + +/** A release lookup payload like the REST API's, parameterized on its assets. */ +function releasePayload(assets: { name: string }[]): Record { + return { + id: 77, + tag_name: "core-v1.2.3", + upload_url: + "https://uploads.github.com/repos/acme/app/releases/77/assets{?name,label}", + assets: assets.map((a, i) => ({ + id: 100 + i, + name: a.name, + browser_download_url: + `https://github.com/acme/app/releases/download/core-v1.2.3/${a.name}`, + })), + }; +} + +/** A `fetch` seam answering the lookup and the upload like GitHub does. */ +function fakeGithub( + seen: Seen[], + options: { releaseStatus?: number; assets?: { name: string }[] } = {}, +): typeof fetch { + return async (input, init) => { + const headers = new Headers(init?.headers); + const rawBody = init?.body; + seen.push({ + url: String(input), + method: init?.method ?? "GET", + authorization: headers.get("authorization") ?? "", + contentType: headers.get("content-type"), + body: rawBody instanceof Uint8Array ? rawBody : undefined, + }); + if (String(input).includes("uploads.github.com")) { + return new Response( + JSON.stringify({ + id: 900, + name: "asset", + browser_download_url: + "https://github.com/acme/app/releases/download/core-v1.2.3/asset", + }), + { status: 201, statusText: "Created" }, + ); + } + const status = options.releaseStatus ?? 200; + const payload = status < 300 + ? releasePayload(options.assets ?? []) + : { message: "Not Found" }; + await Promise.resolve(); + return new Response(JSON.stringify(payload), { + status, + statusText: status < 300 ? "OK" : "Not Found", + }); + }; +} + +/** Write a small file to upload and return its path. */ +async function assetFixture(dir: string): Promise { + const path = `${dir}/extension.tar.gz`; + await Deno.writeFile(path, new Uint8Array([1, 2, 3, 4])); + return path; +} + +/** Run `fn` with the Actions environment variables set to `values`. */ +async function withEnv( + values: Record, + fn: () => Promise, +): Promise { + const saved = new Map(); + for (const [name, value] of Object.entries(values)) { + saved.set(name, Deno.env.get(name)); + if (value === undefined) Deno.env.delete(name); + else Deno.env.set(name, value); + } + try { + await fn(); + } finally { + for (const [name, value] of saved) { + if (value === undefined) Deno.env.delete(name); + else Deno.env.set(name, value); + } + } +} + +Deno.test("an asset is uploaded to the latest release's upload host", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const seen: Seen[] = []; + const result = await GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("acme/app").token("tok").fetch(fakeGithub(seen)) + ); + + assertEquals(result.state, "uploaded"); + assertEquals(result.releaseTag, "core-v1.2.3"); + assertEquals(result.releaseId, 77); + + // First the lookup, then the upload — with the template cut off, the name + // in the query, the token on both, and the file's actual bytes as body. + assertEquals(seen.length, 2); + assertStringIncludes(seen[0].url, "/repos/acme/app/releases/latest"); + assertEquals( + seen[1].url, + "https://uploads.github.com/repos/acme/app/releases/77/assets" + + "?name=extension.tar.gz", + ); + assertEquals(seen[1].method, "POST"); + assertEquals(seen[1].authorization, "Bearer tok"); + assertEquals(seen[1].contentType, "application/gzip"); + assertEquals(seen[1].body, new Uint8Array([1, 2, 3, 4])); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("a tag setting resolves that release instead of latest", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const seen: Seen[] = []; + await GhTasks.uploadReleaseAsset((s) => + s.file(file).tag("v1.0.0").repo("acme/app").token("tok") + .fetch(fakeGithub(seen)) + ); + assertStringIncludes(seen[0].url, "/repos/acme/app/releases/tags/v1.0.0"); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("an asset the release already carries is kept, not re-sent", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const seen: Seen[] = []; + const result = await GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("acme/app").token("tok") + .fetch(fakeGithub(seen, { assets: [{ name: "extension.tar.gz" }] })) + ); + assertEquals(result.state, "already-exists"); + assertStringIncludes(result.url ?? "", "extension.tar.gz"); + // Only the lookup went out — published assets are never churned. + assertEquals(seen.length, 1); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("a repository with no releases reports no-release, not an error", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const result = await GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("acme/app").token("tok") + .fetch(fakeGithub([], { releaseStatus: 404 })) + ); + assertEquals(result, { state: "no-release", name: "extension.tar.gz" }); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("a missing tag IS an error — the caller named a release", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + await assertRejects( + () => + GhTasks.uploadReleaseAsset((s) => + s.file(file).tag("v9.9.9").repo("acme/app").token("tok") + .fetch(fakeGithub([], { releaseStatus: 404 })) + ), + Error, + "releases/tags/v9.9.9", + ); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("the name, content type, and their defaults follow the file", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const seen: Seen[] = []; + await GhTasks.uploadReleaseAsset((s) => + s.file(file).name("bundle.zip").contentType("application/x-test") + .repo("acme/app").token("tok").fetch(fakeGithub(seen)) + ); + assertStringIncludes(seen[1].url, "?name=bundle.zip"); + assertEquals(seen[1].contentType, "application/x-test"); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("missing settings fail with messages that name the fix", async () => { + await assertRejects( + () => GhTasks.uploadReleaseAsset((s) => s.repo("a/b").token("t")), + Error, + ".file(...)", + ); + await withEnv( + { GITHUB_TOKEN: undefined, GITHUB_REPOSITORY: undefined }, + async () => { + await assertRejects( + () => + GhTasks.uploadReleaseAsset((s) => + s.file("x.bin").repo("a/b").fetch(() => { + throw new Error("no request should be made without a token"); + }) + ), + Error, + ".token(...)", + ); + await assertRejects( + () => GhTasks.uploadReleaseAsset((s) => s.file("x.bin").token("t")), + Error, + "GITHUB_REPOSITORY", + ); + }, + ); +}); + +Deno.test("the token and repo default to the Actions environment", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + await withEnv( + { GITHUB_TOKEN: "ghs_env", GITHUB_REPOSITORY: "acme/app" }, + async () => { + const seen: Seen[] = []; + await GhTasks.uploadReleaseAsset((s) => + s.file(file).fetch(fakeGithub(seen)) + ); + assertStringIncludes(seen[0].url, "/repos/acme/app/releases/latest"); + // The token rides in the header from the environment, never argv. + assertEquals(seen[0].authorization, "Bearer ghs_env"); + }, + ); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("a GHES base URL is honored, trailing slash and all", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const seen: Seen[] = []; + await GhTasks.uploadReleaseAsset((s) => + s.file(file).name("asset.bin").repo("acme/app").token("tok") + .baseUrl("https://ghes.example/api/v3/").fetch(fakeGithub(seen)) + ); + assertStringIncludes( + seen[0].url, + "https://ghes.example/api/v3/repos/acme/app/releases/latest", + ); + // An unknown extension falls back to the generic content type. + assertEquals(seen[1].contentType, "application/octet-stream"); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("a name that cannot be derived from the file asks for .name(...)", async () => { + await assertRejects( + () => + GhTasks.uploadReleaseAsset((s) => + s.file("dist/").repo("a/b").token("t").fetch(fakeGithub([])) + ), + Error, + ".name(...)", + ); +}); + +Deno.test("malformed release responses fail with what came back", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const answering = (body: string): typeof fetch => async () => { + await Promise.resolve(); + return new Response(body, { status: 200 }); + }; + // A proxy answering instead of GitHub: not JSON at all. + await assertRejects( + () => + GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("a/b").token("t").fetch(answering("")) + ), + Error, + "non-JSON body", + ); + // JSON, but not a release: nothing to upload to. + await assertRejects( + () => + GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("a/b").token("t").fetch(answering("{}")) + ), + Error, + "no id/upload_url", + ); + // The lookup succeeds but the upload host answers garbage. + const brokenUpload: typeof fetch = async (input) => { + await Promise.resolve(); + if (String(input).includes("uploads.github.com")) { + return new Response("", { status: 201 }); + } + return new Response(JSON.stringify(releasePayload([])), { status: 200 }); + }; + await assertRejects( + () => + GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("a/b").token("t").fetch(brokenUpload) + ), + Error, + "non-JSON body", + ); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("an upload rejection surfaces GitHub's own message", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const failing: typeof fetch = async (input) => { + await Promise.resolve(); + if (String(input).includes("uploads.github.com")) { + return new Response(JSON.stringify({ message: "asset too large" }), { + status: 422, + statusText: "Unprocessable Entity", + }); + } + return new Response(JSON.stringify(releasePayload([])), { status: 200 }); + }; + await assertRejects( + () => + GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("acme/app").token("tok").fetch(failing) + ), + Error, + "asset too large", + ); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); diff --git a/tests/gemini_archive_test.ts b/tests/gemini_archive_test.ts new file mode 100644 index 0000000..d3f89dc --- /dev/null +++ b/tests/gemini_archive_test.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 the Zuke contributors +// SPDX-License-Identifier: MIT + +/** + * Unit tests for the Gemini extension release archive. What matters is the + * contract Gemini's installer imposes: `gemini-extension.json` at the archive + * root, the asset names platform-prefixed so `findReleaseAsset` matches them + * deterministically, and byte-stable output so re-attaching to a release is + * meaningfully idempotent. + * + * @module + */ + +import { gunzip, untar } from "../packages/core/mod.ts"; +import { + assertEquals, + assertStringIncludes, +} from "../packages/core/tests/_assert.ts"; +import { + buildGeminiArchive, + GEMINI_ASSET_NAMES, + geminiArchiveFiles, +} from "../build/gemini_archive.ts"; + +/** Lay out a minimal extension root (manifest, license, two skills). */ +async function extensionFixture(): Promise { + const root = await Deno.makeTempDir(); + await Deno.writeTextFile( + `${root}/gemini-extension.json`, + '{"name":"zuke","version":"0.0.1"}', + ); + await Deno.writeTextFile(`${root}/LICENSE`, "MIT"); + await Deno.mkdir(`${root}/skills/b-skill`, { recursive: true }); + await Deno.writeTextFile(`${root}/skills/b-skill/SKILL.md`, "b"); + await Deno.mkdir(`${root}/skills/a-skill/references`, { recursive: true }); + await Deno.writeTextFile(`${root}/skills/a-skill/SKILL.md`, "a"); + await Deno.writeTextFile(`${root}/skills/a-skill/references/notes.md`, "n"); + return root; +} + +Deno.test("the file list is the manifest, license, and skills — sorted", async () => { + const root = await extensionFixture(); + try { + assertEquals(await geminiArchiveFiles(root), [ + "gemini-extension.json", + "LICENSE", + "skills/a-skill/SKILL.md", + "skills/a-skill/references/notes.md", + "skills/b-skill/SKILL.md", + ]); + } finally { + await Deno.remove(root, { recursive: true }); + } +}); + +Deno.test("the archive round-trips with the manifest at its root", async () => { + const root = await extensionFixture(); + try { + const dest = `${root}/out.tar.gz`; + await buildGeminiArchive(dest, root); + const entries = untar(await gunzip(await Deno.readFile(dest))); + const names = entries.map((e) => e.name); + // Gemini requires the manifest at the archive root — no wrapper directory. + assertEquals(names[0], "gemini-extension.json"); + assertEquals(names.includes("skills/a-skill/SKILL.md"), true); + const manifest = entries.find((e) => e.name === "gemini-extension.json"); + assertStringIncludes( + new TextDecoder().decode(manifest?.data ?? new Uint8Array()), + '"name":"zuke"', + ); + } finally { + await Deno.remove(root, { recursive: true }); + } +}); + +Deno.test("two builds of the same tree are byte-identical", async () => { + // The release upload is skip-if-present; that is only meaningful when a + // rebuild produces the same bytes rather than a fresh timestamped archive. + const root = await extensionFixture(); + try { + await buildGeminiArchive(`${root}/one.tar.gz`, root); + await buildGeminiArchive(`${root}/two.tar.gz`, root); + assertEquals( + await Deno.readFile(`${root}/one.tar.gz`), + await Deno.readFile(`${root}/two.tar.gz`), + ); + } finally { + await Deno.remove(root, { recursive: true }); + } +}); + +Deno.test("the asset names are platform-prefixed the way Gemini matches", () => { + // `findReleaseAsset` matches `{platform}.`-prefixed names case-insensitively + // and only falls back to a generic asset when it is alone on the release — + // so every supported `os.platform()` value must have its own prefix. + assertEquals(GEMINI_ASSET_NAMES.length, 3); + for (const platform of ["darwin", "linux", "win32"]) { + assertEquals( + GEMINI_ASSET_NAMES.some((n) => n.startsWith(`${platform}.`)), + true, + `no asset name for ${platform}`, + ); + } + for (const name of GEMINI_ASSET_NAMES) { + assertEquals(name.endsWith(".tar.gz"), true, `${name} is not a .tar.gz`); + } +}); + +Deno.test("the repo's real tree packs cleanly", async () => { + // The actual release-time operation, run against this repository: the + // manifest, license, and both skills pack without hitting tar's 100-byte + // name limit. + const dir = await Deno.makeTempDir(); + try { + const packed = await buildGeminiArchive(`${dir}/zuke.tar.gz`); + assertEquals(packed[0], "gemini-extension.json"); + assertEquals(packed.includes("skills/zuke-setup/SKILL.md"), true); + assertEquals(packed.includes("skills/zuke-write-build/SKILL.md"), true); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); diff --git a/tests/integration/gemini_release_test.ts b/tests/integration/gemini_release_test.ts index 83102fa..c241104 100644 --- a/tests/integration/gemini_release_test.ts +++ b/tests/integration/gemini_release_test.ts @@ -1,51 +1,96 @@ -import { assertEquals, assertExists } from "@std/assert"; +// Copyright (c) 2026 the Zuke contributors +// SPDX-License-Identifier: MIT + +/** + * Integration: the release target's Gemini-asset wiring — build the extension + * archive, then attach it to the latest release under each platform-prefixed + * name — driven through the real CLI against a fake GitHub, mirroring how + * `zuke.ts`'s `release` target is wired (see `build/gemini_archive.ts`). + */ + import { - afterAll, - beforeAll, - describe, - it, -} from "@std/testing/bdd"; -import { withRequestInterceptor } from "../../test/with_request_interceptor.ts"; + assertEquals, + assertStringIncludes, +} from "../../packages/core/tests/_assert.ts"; +import { Build, target } from "../../packages/core/mod.ts"; import { GhTasks } from "../../packages/gh/mod.ts"; +import { + buildGeminiArchive, + GEMINI_ASSET_NAMES, +} from "../../build/gemini_archive.ts"; +import { runCli } from "./_harness.ts"; -function makeJsonResponse(body: unknown, init?: ResponseInit): Response { - return new Response(JSON.stringify(body), { - headers: { "content-type": "application/json" }, - ...init, - }); -} - -function interceptRequests(handler: Parameters[0]) { +/** A fake GitHub: one release, remembering the asset names it accepts. */ +function fakeGithub(uploaded: string[]): typeof fetch { return async (input, _init) => { - return await withRequestInterceptor(handler, () => fetch(input, _init)); + await Promise.resolve(); + const url = String(input); + if (url.includes("uploads.example")) { + const name = new URL(url).searchParams.get("name") ?? ""; + uploaded.push(name); + return new Response( + JSON.stringify({ id: 1, name, browser_download_url: `dl/${name}` }), + { status: 201 }, + ); + } + return new Response( + JSON.stringify({ + id: 9, + tag_name: "core-v1.0.0", + upload_url: "https://uploads.example/assets{?name,label}", + // The first name is already attached, as after a partial earlier run. + assets: [{ + id: 5, + name: GEMINI_ASSET_NAMES[0], + browser_download_url: "dl/existing", + }], + }), + { status: 200 }, + ); }; } -describe("Gemini release", () => { - let restoreFetch: (() => void) | undefined; - - beforeAll(() => { - restoreFetch = withRequestInterceptor((request) => { - const url = new URL(request.url); - if (request.method === "GET" && url.pathname.endsWith("/releases/latest")) { - return makeJsonResponse({ tag_name: "v1.2.3", id: 123 }); +/** A fixture build wired like the release target's Gemini-asset step. */ +function releaseBuild(root: string, uploaded: string[]) { + class Release extends Build { + attach = target().executes(async () => { + const archive = `${root}/zuke.tar.gz`; + await buildGeminiArchive(archive, root); + for (const name of GEMINI_ASSET_NAMES) { + const result = await GhTasks.uploadReleaseAsset((s) => + s.file(archive).name(name).repo("acme/app").token("tok") + .fetch(fakeGithub(uploaded)) + ); + console.log(`${name}: ${result.state}`); } - return new Response(null, { status: 404 }); }); - }); - - afterAll(() => { - restoreFetch?.(); - }); + } + return Release; +} - it("resolves latest release", async () => { - const result = await GhTasks.uploadReleaseAsset((s) => - s.file("/tmp/zuke.tar.gz").repo("owner/repo").token("token") +Deno.test("the release wiring attaches missing assets and keeps existing ones", async () => { + const root = await Deno.makeTempDir(); + try { + await Deno.writeTextFile( + `${root}/gemini-extension.json`, + '{"name":"zuke","version":"0.0.1"}', ); + await Deno.writeTextFile(`${root}/LICENSE`, "MIT"); + await Deno.mkdir(`${root}/skills/s`, { recursive: true }); + await Deno.writeTextFile(`${root}/skills/s/SKILL.md`, "s"); + + const uploaded: string[] = []; + const { code, out } = await runCli(releaseBuild(root, uploaded), [ + "attach", + ]); + assertEquals(code, 0); - assertEquals(result.state, "uploaded"); - assertEquals(result.releaseTag, "v1.2.3"); - assertEquals(result.releaseId, 123); - assertExists(result.url); - }); + // The asset present from the earlier run is kept; the other two upload. + assertStringIncludes(out, `${GEMINI_ASSET_NAMES[0]}: already-exists`); + assertStringIncludes(out, `${GEMINI_ASSET_NAMES[1]}: uploaded`); + assertStringIncludes(out, `${GEMINI_ASSET_NAMES[2]}: uploaded`); + assertEquals(uploaded, GEMINI_ASSET_NAMES.slice(1)); + } finally { + await Deno.remove(root, { recursive: true }); + } }); diff --git a/zuke.ts b/zuke.ts index a251929..b467af7 100644 --- a/zuke.ts +++ b/zuke.ts @@ -102,6 +102,10 @@ import { syncPluginSkills, } from "./build/plugin_sync.ts"; import { checkSkillTree } from "./build/skill_check.ts"; +import { + buildGeminiArchive, + GEMINI_ASSET_NAMES, +} from "./build/gemini_archive.ts"; import { bumpFailure, checkPluginVersionBump, @@ -1077,6 +1081,32 @@ class ZukeBuild extends Build { apply(s); return s; }); + + // Attach the Gemini CLI extension archive to whatever release is now + // "latest" — that is the release `gemini extensions install` resolves, + // and it changes on every package release, so each run tops it up. + // Idempotent: a release already carrying the assets is left untouched, + // and a repository with no releases yet is an ordinary skip. + const scratch = await Deno.makeTempDir(); + try { + const archive = `${scratch}/zuke.tar.gz`; + const packed = await buildGeminiArchive(archive); + ConsoleTasks.info( + `Built the Gemini extension archive (${packed.length} file(s)).`, + ); + for (const name of GEMINI_ASSET_NAMES) { + const result = await GhTasks.uploadReleaseAsset((s) => + s.file(archive).name(name).repo(repo).token(token) + ); + ConsoleTasks.info( + result.state === "no-release" + ? `No release to attach ${name} to yet.` + : `${name} on ${result.releaseTag}: ${result.state}.`, + ); + } + } finally { + await Deno.remove(scratch, { recursive: true }); + } }); actionRelease = target() From 85aa81132e42eb5476f6f05961cfe83c3c1cf183 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 12:00:50 +0000 Subject: [PATCH 03/11] fix(gh): repair stuck release assets and make the Gemini attach best-effort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review found two real failure modes: an errored upload reserves the asset name in a non-uploaded state, and the skip-by-name idempotence check would step past that corpse forever, so a re-run now deletes the stuck asset and re-sends it. And a transient upload failure used to redden the release job after the releases already existed, which skipped the JSR publish behind it — the attach step is now best-effort with a warning, since the next release run tops the assets up anyway. The archive builder also refuses symlinks instead of silently dropping them and names the fix when the manifest, license, or skills tree is missing. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019As9A3ugixLQiAvcat1ZKk --- build/gemini_archive.ts | 31 +++- packages/gh/src/release_asset.ts | 27 ++- packages/gh/tests/app_token_test.ts | 89 +++++++++- packages/gh/tests/commit_test.ts | 214 +++++++++++++++++++++++ packages/gh/tests/pull_request_test.ts | 216 ++++++++++++++++++++++++ packages/gh/tests/release_asset_test.ts | 83 +++++++++ packages/gh/tests/workflow_test.ts | 50 ++++++ tests/gemini_archive_test.ts | 48 ++++++ zuke.ts | 14 +- 9 files changed, 767 insertions(+), 5 deletions(-) diff --git a/build/gemini_archive.ts b/build/gemini_archive.ts index f50c25d..67f1f0b 100644 --- a/build/gemini_archive.ts +++ b/build/gemini_archive.ts @@ -49,6 +49,16 @@ async function walk(root: string, dir: string): Promise { const path = `${dir}/${entry.name}`; if (entry.isDirectory) out.push(...await walk(root, path)); else if (entry.isFile) out.push(path); + else { + // Silently dropping it would ship an archive missing content that git + // (and every other harness) still carries. + throw new Error( + `the Gemini extension archive cannot pack "${root}/${path}" — it is ` + + "neither a regular file nor a directory (a symlink?). Keep " + + `${GEMINI_SKILLS_DIR}/ to real files so every harness ships the ` + + "same content.", + ); + } } return out.sort(); } @@ -58,7 +68,26 @@ async function walk(root: string, dir: string): Promise { * order: the manifest, the license, then every file under `skills/`. */ export async function geminiArchiveFiles(root = "."): Promise { - return [GEMINI_MANIFEST, "LICENSE", ...await walk(root, GEMINI_SKILLS_DIR)]; + for (const required of [GEMINI_MANIFEST, "LICENSE"]) { + const info = await Deno.stat(`${root}/${required}`).catch(() => null); + if (info?.isFile !== true) { + throw new Error( + `the Gemini extension archive requires ${required} at the extension ` + + `root — nothing at "${root}/${required}".`, + ); + } + } + try { + return [GEMINI_MANIFEST, "LICENSE", ...await walk(root, GEMINI_SKILLS_DIR)]; + } catch (error) { + if (error instanceof Deno.errors.NotFound) { + throw new Error( + `the Gemini extension archive requires a ${GEMINI_SKILLS_DIR}/ tree ` + + `under "${root}" — Gemini auto-discovers skills from it.`, + ); + } + throw error; + } } /** diff --git a/packages/gh/src/release_asset.ts b/packages/gh/src/release_asset.ts index 815d9e0..5fa40fb 100644 --- a/packages/gh/src/release_asset.ts +++ b/packages/gh/src/release_asset.ts @@ -264,11 +264,20 @@ export async function uploadReleaseAsset( // Idempotence: a release that already carries the asset is left untouched — // published assets are immutable history, and re-running the pipeline must - // not churn them. + // not churn them. The one exception is an asset stuck in a non-`uploaded` + // state: an errored or interrupted upload reserves the name while serving + // nothing, and GitHub's documented recovery is delete-then-reupload — so a + // re-run must do exactly that rather than skip past the corpse forever. const assets = field(release, "assets"); if (Array.isArray(assets)) { for (const asset of assets) { - if (field(asset, "name") === name) { + if (field(asset, "name") !== name) continue; + const state = field(asset, "state"); + const assetId = field(asset, "id"); + if ( + (state === undefined || state === "uploaded") || + typeof assetId !== "number" + ) { const url = field(asset, "browser_download_url"); return { state: "already-exists", @@ -278,6 +287,20 @@ export async function uploadReleaseAsset( ...(typeof url === "string" ? { url } : {}), }; } + const deletion = await settings.fetch_( + `${settings.baseUrl_}/repos/${slug}/releases/assets/${assetId}`, + { method: "DELETE", headers }, + ); + const deletionText = await deletion.text(); + // A 404 means the corpse was already cleaned up — the goal state. + if (!deletion.ok && deletion.status !== 404) { + throw new Error( + `deleting the stuck release asset "${name}" (state ${ + String(state) + }) failed: ${deletion.status} ${deletion.statusText}. ` + + deletionText.slice(0, 400), + ); + } } } diff --git a/packages/gh/tests/app_token_test.ts b/packages/gh/tests/app_token_test.ts index c4fb741..bc02bce 100644 --- a/packages/gh/tests/app_token_test.ts +++ b/packages/gh/tests/app_token_test.ts @@ -16,7 +16,28 @@ import { assertStringIncludes, } from "../../core/tests/_assert.ts"; import { GhTasks } from "../mod.ts"; -import { GhAppTokenSettings } from "../src/app_token.ts"; +import { GhAppTokenSettings, mintAppToken } from "../src/app_token.ts"; + +/** Run `fn` with `values` in the environment, restoring the originals after. */ +async function withEnv( + values: Record, + fn: () => Promise, +): Promise { + const saved = new Map(); + for (const [name, value] of Object.entries(values)) { + saved.set(name, Deno.env.get(name)); + if (value === undefined) Deno.env.delete(name); + else Deno.env.set(name, value); + } + try { + await fn(); + } finally { + for (const [name, value] of saved) { + if (value === undefined) Deno.env.delete(name); + else Deno.env.set(name, value); + } + } +} /** A recorded request the fake `fetch` saw. */ interface Seen { @@ -345,6 +366,72 @@ Deno.test("an owner or repository that would redirect the mint is refused", () = } }); +Deno.test("appToken with no configuration is refused before any request", async () => { + // A bare call is legal to write, so its first missing setting must be named + // — and refused before the transport is touched. + await assertRejects(() => mintAppToken(), Error, ".appId(...)"); +}); + +Deno.test("a 2xx body that is not a JSON object is named per endpoint", async () => { + const { pkcs8 } = await testKeys(); + // Valid JSON, but a string — indexing it as the installation would surface a + // TypeError naming no endpoint. + await assertRejects( + () => + GhTasks.appToken((s) => + s.appId("1").privateKey(pkcs8).owner("acme").fetch( + fakeGithub([], { installation: "not an object" }), + ) + ), + Error, + "resolving the app installation returned a body that is not JSON", + ); + + // Not JSON at all — a proxy or gateway answering instead of GitHub. The body + // is the evidence of who actually answered, so a prefix of it comes along. + const gateway: typeof fetch = () => + Promise.resolve(new Response("gateway", { status: 200 })); + const error = await assertRejects( + () => + GhTasks.appToken((s) => + s.appId("1").privateKey(pkcs8).owner("acme").fetch(gateway) + ), + Error, + "returned a body that is not JSON", + ); + assertStringIncludes(error.message, "gateway"); +}); + +Deno.test("the minted token is masked in Actions logs, and only there", async () => { + // Matching what actions/create-github-app-token does: inside Actions the + // runner is told to mask the token, so passing it onward through env cannot + // leak it into a log. Outside Actions the directive would just be noise. + const { pkcs8 } = await testKeys(); + const logged: string[] = []; + const original = console.log; + console.log = (...args: unknown[]): void => { + logged.push(args.map(String).join(" ")); + }; + try { + await withEnv({ GITHUB_ACTIONS: "true" }, async () => { + await GhTasks.appToken((s) => + s.appId("1").privateKey(pkcs8).owner("acme").fetch(fakeGithub([])) + ); + }); + assertEquals(logged, ["::add-mask::ghs_installation"]); + + logged.length = 0; + await withEnv({ GITHUB_ACTIONS: undefined }, async () => { + await GhTasks.appToken((s) => + s.appId("1").privateKey(pkcs8).owner("acme").fetch(fakeGithub([])) + ); + }); + assertEquals(logged, []); + } finally { + console.log = original; + } +}); + Deno.test("an ordinary owner and repository still build the usual path", () => { assertEquals( new GhAppTokenSettings().owner("zuke-build").repositories("zuke") diff --git a/packages/gh/tests/commit_test.ts b/packages/gh/tests/commit_test.ts index b8b1c67..0ba56c4 100644 --- a/packages/gh/tests/commit_test.ts +++ b/packages/gh/tests/commit_test.ts @@ -13,10 +13,32 @@ import { assertEquals, + assertRejects, assertStringIncludes, } from "../../core/tests/_assert.ts"; import { commitFiles, tagCommit } from "../src/commit.ts"; +/** Run `fn` with `values` in the environment, restoring the originals after. */ +async function withEnv( + values: Record, + fn: () => Promise, +): Promise { + const saved = new Map(); + for (const [name, value] of Object.entries(values)) { + saved.set(name, Deno.env.get(name)); + if (value === undefined) Deno.env.delete(name); + else Deno.env.set(name, value); + } + try { + await fn(); + } finally { + for (const [name, value] of saved) { + if (value === undefined) Deno.env.delete(name); + else Deno.env.set(name, value); + } + } +} + /** One recorded request. */ interface Call { method: string; @@ -579,3 +601,195 @@ Deno.test("`.replace()` without `.from(...)` is refused rather than ignored", as assertStringIncludes(message, ".replace() only applies together with .from("); assertEquals(calls.length, 0); }); + +Deno.test("a commit falls back to GITHUB_REPOSITORY and GITHUB_TOKEN", async () => { + // A job that already has the Actions environment needs to name only what it + // is committing. + await withEnv({ + GITHUB_REPOSITORY: "acme/app", + GITHUB_TOKEN: "ghs_env", + }, async () => { + const { fetch, calls } = fakeFetch({ + "GET /git/ref/heads/topic": { object: { sha: "h" } }, + "GET /git/commits/h": { tree: { sha: "t" } }, + "POST /git/trees": { sha: "t2" }, + "POST /git/commits": { sha: "c" }, + "PATCH /git/refs/heads/topic": {}, + }); + const result = await commitFiles((s) => + s.branch("topic").message("m").file("a.ts", "1\n").fetch(fetch) + ); + assertEquals(result.sha, "c"); + // The env-resolved slug routed the request, and the env token rode as the + // bearer header on every call — never through argv or a body. + assertEquals(calls[0].path, "/git/ref/heads/topic"); + for (const call of calls) assertEquals(call.auth, "Bearer ghs_env"); + }); +}); + +Deno.test("a tag falls back to GITHUB_REPOSITORY, GITHUB_TOKEN, and GITHUB_SHA", async () => { + await withEnv({ + GITHUB_REPOSITORY: "acme/app", + GITHUB_TOKEN: "ghs_env", + GITHUB_SHA: "e".repeat(40), + }, async () => { + const { fetch, calls } = fakeFetch({ + "POST /git/tags": { sha: "obj" }, + "POST /git/refs": {}, + }); + await tagCommit((s) => s.name("v9.9.9").fetch(fetch)); + // The tag points at the commit the workflow ran for. + assertEquals(calls[0].path, "/git/tags"); + assertEquals(calls[0].body.object, "e".repeat(40)); + for (const call of calls) assertEquals(call.auth, "Bearer ghs_env"); + }); +}); + +Deno.test("a missing repo, token, or sha is named rather than guessed", async () => { + // Without the Actions environment the settings must say which setting is + // missing and which variable would have filled it — not fail downstream. + await withEnv({ + GITHUB_REPOSITORY: undefined, + GITHUB_TOKEN: undefined, + GITHUB_SHA: undefined, + }, async () => { + const { fetch, calls } = fakeFetch({}); + await assertRejects( + () => + commitFiles((s) => s.token("t").branch("b").message("m").fetch(fetch)), + Error, + "committing requires .repo('owner/name') (or GITHUB_REPOSITORY)", + ); + await assertRejects( + () => + commitFiles((s) => + s.repo("acme/app").branch("b").message("m").fetch(fetch) + ), + Error, + "committing requires .token(...) (or GITHUB_TOKEN)", + ); + await assertRejects( + () => tagCommit((s) => s.token("t").name("v1").commit("c").fetch(fetch)), + Error, + "tagging requires .repo('owner/name') (or GITHUB_REPOSITORY)", + ); + await assertRejects( + () => + tagCommit((s) => + s.repo("acme/app").name("v1").commit("c").fetch(fetch) + ), + Error, + "tagging requires .token(...) (or GITHUB_TOKEN)", + ); + await assertRejects( + () => + tagCommit((s) => s.repo("acme/app").token("t").name("v1").fetch(fetch)), + Error, + "tagging requires .commit(...) (or GITHUB_SHA)", + ); + // Every refusal happened before anything was sent. + assertEquals(calls.length, 0); + }); +}); + +Deno.test("the required commit and tag settings are named when absent", async () => { + // A bare call is legal to write, so its first missing setting must be named + // — and refused before the environment or the transport is consulted. + await assertRejects( + () => commitFiles(), + Error, + "committing requires .branch(...)", + ); + await assertRejects( + () => commitFiles((s) => s.repo("acme/app").token("t").branch("topic")), + Error, + "committing requires .message(...)", + ); + await assertRejects(() => tagCommit(), Error, "tagging requires .name(...)"); +}); + +Deno.test("baseUrl retargets commits and tags at a GHES host", async () => { + const seen: string[] = []; + const capture: typeof fetch = (input) => { + seen.push(String(input)); + return Promise.resolve( + new Response( + JSON.stringify({ object: { sha: "a" }, tree: { sha: "t" }, sha: "c" }), + { status: 200 }, + ), + ); + }; + await commitFiles((s) => + s.repo("acme/app").token("t").branch("main").message("m") + .baseUrl("https://ghe.example.com/api/v3/").fetch(capture) + ); + // The trailing slash is trimmed, so no `//` appears in the request path. + assertEquals( + seen[0], + "https://ghe.example.com/api/v3/repos/acme/app/git/ref/heads/main", + ); + + seen.length = 0; + await tagCommit((s) => + s.repo("acme/app").token("t").name("v1").commit("c") + .baseUrl("https://ghe.example.com/api/v3").fetch(capture) + ); + assertEquals( + seen[0], + "https://ghe.example.com/api/v3/repos/acme/app/git/tags", + ); +}); + +Deno.test("a response missing the field a call needs names that call", async () => { + // `readString` names the call whose response was malformed, rather than + // letting `undefined` flow into the next request's path. + const notRecord = fakeFetch({ + "GET /git/ref/heads/topic": { object: "nope" }, + }); + await assertRejects( + () => + commitFiles((s) => + s.repo("acme/app").token("t").branch("topic").message("m") + .fetch(notRecord.fetch) + ), + Error, + "the ref response has no object.sha", + ); + + // The terminal value has to be a string, not merely present. + const wrongType = fakeFetch({ + "GET /git/ref/heads/topic": { object: { sha: "h" } }, + "GET /git/commits/h": { tree: { sha: "t" } }, + "POST /git/trees": { sha: 42 }, + }); + await assertRejects( + () => + commitFiles((s) => + s.repo("acme/app").token("t").branch("topic").message("m") + .fetch(wrongType.fetch) + ), + Error, + "the tree response has no sha", + ); +}); + +Deno.test("an empty 2xx body reads as an empty object, not a parse error", async () => { + // GitHub answers some writes with an empty body; the transport must not die + // parsing "" on a reply the caller never reads. + const empty: typeof fetch = (input, init) => { + if ((init?.method ?? "GET") === "PATCH") { + return Promise.resolve(new Response("", { status: 200 })); + } + void input; + return Promise.resolve( + new Response( + JSON.stringify({ object: { sha: "a" }, tree: { sha: "t" }, sha: "c" }), + { status: 200 }, + ), + ); + }; + const result = await commitFiles((s) => + s.repo("acme/app").token("t").branch("main").message("m").fetch(empty) + ); + assertEquals(result.sha, "c"); +}); diff --git a/packages/gh/tests/pull_request_test.ts b/packages/gh/tests/pull_request_test.ts index 5424492..73f898b 100644 --- a/packages/gh/tests/pull_request_test.ts +++ b/packages/gh/tests/pull_request_test.ts @@ -13,13 +13,36 @@ import { assertEquals, + assertRejects, assertStringIncludes, } from "../../core/tests/_assert.ts"; import { + findPullRequest, type GhPullRequestSettings, openPullRequest, } from "../src/pull_request.ts"; +/** Run `fn` with `values` in the environment, restoring the originals after. */ +async function withEnv( + values: Record, + fn: () => Promise, +): Promise { + const saved = new Map(); + for (const [name, value] of Object.entries(values)) { + saved.set(name, Deno.env.get(name)); + if (value === undefined) Deno.env.delete(name); + else Deno.env.set(name, value); + } + try { + await fn(); + } finally { + for (const [name, value] of saved) { + if (value === undefined) Deno.env.delete(name); + else Deno.env.set(name, value); + } + } +} + /** One recorded request. */ interface Call { method: string; @@ -267,6 +290,199 @@ Deno.test("a head that is not the one asked for is not accepted", async () => { assertStringIncludes(message, "original 422"); }); +Deno.test("findPullRequest returns the open proposal without opening one", async () => { + const { fetch, calls } = fakeFetch({ + "GET /pulls?state=open&head=acme%3Atopic&base=master": { + status: 200, + body: [{ + number: 4, + html_url: "https://github.com/acme/app/pull/4", + head: { ref: "topic" }, + base: { ref: "master" }, + }], + }, + }); + const result = await findPullRequest((s) => + s.repo("acme/app").token("t").head("topic").base("master").fetch(fetch) + ); + assertEquals(result, { + number: 4, + url: "https://github.com/acme/app/pull/4", + created: false, + }); + // A read, and only a read: nothing was proposed on the caller's behalf. + assertEquals(calls.length, 1); + assertEquals(calls[0].method, "GET"); +}); + +Deno.test("findPullRequest returns undefined when nothing is open", async () => { + const { fetch } = fakeFetch({ + "GET /pulls?state=open&head=acme%3Atopic&base=master": { + status: 200, + body: [], + }, + }); + assertEquals( + await findPullRequest((s) => + s.repo("acme/app").token("t").head("topic").base("master").fetch(fetch) + ), + undefined, + ); +}); + +Deno.test("findPullRequest names its missing settings before any request", async () => { + // A bare call is legal to write, so the first missing setting is named — + // before the environment or the transport is consulted. + await assertRejects( + () => findPullRequest(), + Error, + "finding a pull request requires .head(...)", + ); + await assertRejects( + () => findPullRequest((s) => s.head("topic")), + Error, + "finding a pull request requires .base(...)", + ); +}); + +Deno.test("findPullRequest refuses a ref that would redirect the lookup", async () => { + // The head and base reach the lookup's query string, the same reason + // openPullRequest validates its own. + const { fetch, calls } = fakeFetch({}); + await assertRejects( + () => + findPullRequest((s) => + s.repo("acme/app").token("t").head("../../../user/repos").base("master") + .fetch(fetch) + ), + Error, + "not a valid git ref name", + ); + assertEquals(calls.length, 0); +}); + +Deno.test("a lookup entry that is not an object keeps the original error", async () => { + // Junk in the lookup body must fall through to the 422 that says what was + // refused, never be dressed up as an existing pull request. + const { fetch } = fakeFetch({ + "POST /pulls": { status: 422, body: { message: "original 422" } }, + "GET /pulls?state=open&head=acme%3Atopic&base=master": { + status: 200, + body: [null], + }, + }); + await assertRejects( + () => + openPullRequest((s) => + s.repo("acme/app").token("t").head("topic").base("master").title("t") + .fetch(fetch) + ), + Error, + "original 422", + ); +}); + +Deno.test("a created response without a numeric number names the call", async () => { + // `readNumber` names the call whose response was malformed, rather than + // letting a string flow into the result as though it were the number. + const { fetch } = fakeFetch({ + "POST /pulls": { status: 201, body: { number: "7", html_url: "u" } }, + }); + await assertRejects( + () => + openPullRequest((s) => + s.repo("acme/app").token("t").head("topic").base("master").title("t") + .fetch(fetch) + ), + Error, + "the pull request response has no number", + ); +}); + +Deno.test("the repo and token fall back to the Actions environment", async () => { + // A job that already has GITHUB_REPOSITORY and GITHUB_TOKEN needs to name + // only what it is proposing. + await withEnv({ + GITHUB_REPOSITORY: "acme/app", + GITHUB_TOKEN: "ghs_env", + }, async () => { + const seen: Array<{ url: string; auth: string | null }> = []; + const capture: typeof fetch = (input, init) => { + seen.push({ + url: String(input), + auth: new Headers(init?.headers).get("authorization"), + }); + return Promise.resolve( + new Response(JSON.stringify({ number: 7, html_url: "u" }), { + status: 201, + }), + ); + }; + const result = await openPullRequest((s) => + s.head("topic").base("master").title("t").fetch(capture) + ); + assertEquals(result.created, true); + // The env-resolved slug routed the request; the env token rode the header. + assertEquals(seen[0].url, "https://api.github.com/repos/acme/app/pulls"); + assertEquals(seen[0].auth, "Bearer ghs_env"); + }); +}); + +Deno.test("a missing repo or token is named rather than sent empty", async () => { + await withEnv({ + GITHUB_REPOSITORY: undefined, + GITHUB_TOKEN: undefined, + }, async () => { + const { fetch, calls } = fakeFetch({}); + await assertRejects( + () => + openPullRequest((s) => + s.token("t").head("topic").base("master").title("t").fetch(fetch) + ), + Error, + ".repo('owner/name') (or GITHUB_REPOSITORY)", + ); + await assertRejects( + () => + openPullRequest((s) => + s.repo("acme/app").head("topic").base("master").title("t").fetch( + fetch, + ) + ), + Error, + ".token(...) (or GITHUB_TOKEN)", + ); + // Both refusals happened before anything was sent. + assertEquals(calls.length, 0); + }); +}); + +Deno.test("openPullRequest with no configuration names the first gap", async () => { + await assertRejects( + () => openPullRequest(), + Error, + "opening a pull request requires .head(...)", + ); +}); + +Deno.test("baseUrl retargets the proposal at a GHES host", async () => { + const seen: string[] = []; + const capture: typeof fetch = (input) => { + seen.push(String(input)); + return Promise.resolve( + new Response(JSON.stringify({ number: 1, html_url: "u" }), { + status: 201, + }), + ); + }; + await openPullRequest((s) => + s.repo("acme/app").token("t").head("topic").base("master").title("t") + .baseUrl("https://ghe.example.com/api/v3/").fetch(capture) + ); + // The trailing slash is trimmed, so no `//` appears in the request path. + assertEquals(seen[0], "https://ghe.example.com/api/v3/repos/acme/app/pulls"); +}); + Deno.test("a lookup that fails keeps the error that says what was refused", async () => { // The lookup is how this call finds out what the 422 meant. If it fails too, // reporting its failure would replace the diagnosis with a symptom of the diff --git a/packages/gh/tests/release_asset_test.ts b/packages/gh/tests/release_asset_test.ts index 14d44f1..c463d82 100644 --- a/packages/gh/tests/release_asset_test.ts +++ b/packages/gh/tests/release_asset_test.ts @@ -172,6 +172,89 @@ Deno.test("an asset the release already carries is kept, not re-sent", async () } }); +Deno.test("a stuck asset (state not uploaded) is deleted and re-sent", async () => { + // GitHub's documented failure mode: an errored/interrupted upload reserves + // the asset name in a non-`uploaded` state. Skipping it would leave the + // release serving a corpse forever — the one case a re-run must repair. + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const seen: Seen[] = []; + const github: typeof fetch = async (input, init) => { + const method = init?.method ?? "GET"; + seen.push({ + url: String(input), + method, + authorization: new Headers(init?.headers).get("authorization") ?? "", + contentType: new Headers(init?.headers).get("content-type"), + body: init?.body instanceof Uint8Array ? init.body : undefined, + }); + await Promise.resolve(); + if (method === "DELETE") return new Response(null, { status: 204 }); + if (String(input).includes("uploads.github.com")) { + return new Response( + JSON.stringify({ id: 901, browser_download_url: "dl/fresh" }), + { status: 201 }, + ); + } + const release = releasePayload([]); + release.assets = [{ + id: 55, + name: "extension.tar.gz", + state: "new", + browser_download_url: "dl/corpse", + }]; + return new Response(JSON.stringify(release), { status: 200 }); + }; + + const result = await GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("acme/app").token("tok").fetch(github) + ); + assertEquals(result.state, "uploaded"); + // Lookup, then DELETE of the stuck asset, then the fresh upload. + assertEquals(seen.map((s) => s.method), ["GET", "DELETE", "POST"]); + assertStringIncludes(seen[1].url, "/releases/assets/55"); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("a failed deletion of a stuck asset surfaces, a 404 does not", async () => { + const dir = await Deno.makeTempDir(); + try { + const file = await assetFixture(dir); + const github = (deleteStatus: number): typeof fetch => async (i, init) => { + await Promise.resolve(); + if ((init?.method ?? "GET") === "DELETE") { + return new Response(JSON.stringify({ message: "locked" }), { + status: deleteStatus, + }); + } + if (String(i).includes("uploads.github.com")) { + return new Response(JSON.stringify({ id: 1 }), { status: 201 }); + } + const release = releasePayload([]); + release.assets = [{ id: 55, name: "extension.tar.gz", state: "new" }]; + return new Response(JSON.stringify(release), { status: 200 }); + }; + await assertRejects( + () => + GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("acme/app").token("tok").fetch(github(423)) + ), + Error, + "locked", + ); + // The corpse already being gone is the goal state, not a failure. + const result = await GhTasks.uploadReleaseAsset((s) => + s.file(file).repo("acme/app").token("tok").fetch(github(404)) + ); + assertEquals(result.state, "uploaded"); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + Deno.test("a repository with no releases reports no-release, not an error", async () => { const dir = await Deno.makeTempDir(); try { diff --git a/packages/gh/tests/workflow_test.ts b/packages/gh/tests/workflow_test.ts index f19f964..580e83f 100644 --- a/packages/gh/tests/workflow_test.ts +++ b/packages/gh/tests/workflow_test.ts @@ -339,6 +339,56 @@ Deno.test("readWorkflowResult tolerates malformed jobs", () => { ]); }); +Deno.test("readWorkflowResult defaults malformed or absent job fields", () => { + // A jobs field that is not an array reads as no jobs, not a crash. + const notArray = fakeState({ + githubWorkflow: { + result: { + runId: 1, + conclusion: "success", + url: "u", + passed: true, + jobs: "nope", + }, + }, + }); + assertEquals(readWorkflowResult(notArray)?.jobs, []); + // A job record with no fields defaults each one, the name included. + const empty = fakeState({ + githubWorkflow: { + result: { + runId: 1, + conclusion: "success", + url: "u", + passed: true, + jobs: [{}], + }, + }, + }); + assertEquals(readWorkflowResult(empty)?.jobs, [ + { name: "", conclusion: "", url: "" }, + ]); +}); + +Deno.test("a completed run with a null conclusion is recorded as unknown", async () => { + // GitHub can complete a run whose conclusion the API reports as null. That + // is not a pass, and the recorded conclusion must still say something + // readable rather than serialising a null. + const api = new ScriptedApi(); + api.status = "completed"; + api.conclusion = null; + const state = fakeState(); + const trigger = githubWorkflowWith((g) => g.repo("a/b").workflow("w"), { + api, + }); + const c = ctx(state); + await trigger.isSatisfied(NO_SIGNALS, c); // dispatch + assertEquals(await trigger.isSatisfied(NO_SIGNALS, c), true); + const result = readWorkflowResult(state); + assertEquals(result?.passed, false); + assertEquals(result?.conclusion, "unknown"); +}); + // --- M18: created-window correlation and marker fast-fail ------------------- const DISPATCH_AT = Date.parse("2026-07-19T00:00:00.000Z"); diff --git a/tests/gemini_archive_test.ts b/tests/gemini_archive_test.ts index d3f89dc..e73f906 100644 --- a/tests/gemini_archive_test.ts +++ b/tests/gemini_archive_test.ts @@ -14,6 +14,7 @@ import { gunzip, untar } from "../packages/core/mod.ts"; import { assertEquals, + assertRejects, assertStringIncludes, } from "../packages/core/tests/_assert.ts"; import { @@ -106,6 +107,53 @@ Deno.test("the asset names are platform-prefixed the way Gemini matches", () => } }); +Deno.test("a broken extension root fails with errors that name the fix", async () => { + const root = await Deno.makeTempDir(); + try { + // No manifest at all. + await assertRejects( + () => geminiArchiveFiles(root), + Error, + "requires gemini-extension.json", + ); + await Deno.writeTextFile(`${root}/gemini-extension.json`, "{}"); + await assertRejects(() => geminiArchiveFiles(root), Error, "LICENSE"); + await Deno.writeTextFile(`${root}/LICENSE`, "MIT"); + // Manifest and license present, but no skills tree. + await assertRejects( + () => geminiArchiveFiles(root), + Error, + "requires a skills/ tree", + ); + } finally { + await Deno.remove(root, { recursive: true }); + } +}); + +Deno.test({ + name: "a symlink under skills/ is refused, not silently dropped", + // Creating symlinks on Windows needs a privilege the CI runner may lack. + ignore: Deno.build.os === "windows", + fn: async () => { + // Git and the other harnesses would keep the linked content; an archive + // that silently loses it would ship different skills to Gemini. + const root = await extensionFixture(); + try { + await Deno.symlink( + `${root}/skills/a-skill/SKILL.md`, + `${root}/skills/a-skill/linked.md`, + ); + await assertRejects( + () => geminiArchiveFiles(root), + Error, + "linked.md", + ); + } finally { + await Deno.remove(root, { recursive: true }); + } + }, +}); + Deno.test("the repo's real tree packs cleanly", async () => { // The actual release-time operation, run against this repository: the // manifest, license, and both skills pack without hitting tar's 100-byte diff --git a/zuke.ts b/zuke.ts index b467af7..26af573 100644 --- a/zuke.ts +++ b/zuke.ts @@ -1086,7 +1086,11 @@ class ZukeBuild extends Build { // "latest" — that is the release `gemini extensions install` resolves, // and it changes on every package release, so each run tops it up. // Idempotent: a release already carrying the assets is left untouched, - // and a repository with no releases yet is an ordinary skip. + // and a repository with no releases yet is an ordinary skip. Best-effort + // on top: the archive is a nice-to-have for Gemini installs, not part of + // the release contract, and a throw here would redden the job after the + // releases already exist — skipping the JSR publish that `needs` this + // job. A warning plus the next run's top-up is the right trade. const scratch = await Deno.makeTempDir(); try { const archive = `${scratch}/zuke.tar.gz`; @@ -1104,6 +1108,14 @@ class ZukeBuild extends Build { : `${name} on ${result.releaseTag}: ${result.state}.`, ); } + } catch (error) { + ConsoleTasks.warn( + "Attaching the Gemini extension archive failed — the releases " + + "themselves are unaffected and the next release run tops the " + + `assets up: ${ + error instanceof Error ? error.message : String(error) + }`, + ); } finally { await Deno.remove(scratch, { recursive: true }); } From 37be9cffbe5ece4bb73368c97b68911660fd70a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 12:02:26 +0000 Subject: [PATCH 04/11] test: expand coverage of the ai, core ci/describe, and gh workflow suites Checkpoint of the coverage push toward 98%: meaningful branch tests for the AI reviewer plumbing, CI schedule/generation, build description, and the GitHub workflow trigger, all against the existing fakes. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019As9A3ugixLQiAvcat1ZKk --- packages/ai/tests/ai_test.ts | 33 ++ packages/ai/tests/cost_controls_test.ts | 62 +++- packages/ai/tests/deep_review_test.ts | 82 +++++ packages/ai/tests/discussion_flow_test.ts | 377 +++++++++++++++++++++- packages/core/tests/ci_schedule_test.ts | 52 +++ packages/core/tests/ci_test.ts | 106 +++++- packages/core/tests/cli_test.ts | 7 +- packages/core/tests/describe_test.ts | 23 ++ packages/gh/tests/workflow_test.ts | 49 +++ 9 files changed, 786 insertions(+), 5 deletions(-) diff --git a/packages/ai/tests/ai_test.ts b/packages/ai/tests/ai_test.ts index c4b4eac..7dbfbe2 100644 --- a/packages/ai/tests/ai_test.ts +++ b/packages/ai/tests/ai_test.ts @@ -1184,3 +1184,36 @@ Deno.test("a failed PR comment never breaks the review", async () => { }, ); }); + +Deno.test("provider_ reflects the configured provider", () => { + const bare = securityReviewer(); + assertEquals(bare.provider_, undefined); // nothing configured yet + const configured = securityReviewer((r) => r.provider("openai")); + assertEquals(configured.provider_, "openai"); +}); + +Deno.test("the fetchBase fallback is announced on the console when not quiet", async () => { + const { fetch } = recordFetch( + claude({ score: 0, severity: "none", summary: "", findings: [] }), + ); + const lines = await captured(() => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .diff((d) => d.fetchBase("develop")) + .env(() => undefined) + .exec((argv) => { + if (argv[1] === "fetch") return Promise.reject(new Error("offline")); + return Promise.resolve("diff --git a/w b/w\n+working tree"); + }) + .fetch(fetch) + ).validate({ target: "t" }) + ); + // The silent-fallback hazard is called out where the operator can see it. + assertEquals( + lines.some((l) => + l.includes("fetchBase could not compute the base diff") && + l.includes("falling back to the working-tree diff") + ), + true, + ); +}); diff --git a/packages/ai/tests/cost_controls_test.ts b/packages/ai/tests/cost_controls_test.ts index fe484d2..d2aded1 100644 --- a/packages/ai/tests/cost_controls_test.ts +++ b/packages/ai/tests/cost_controls_test.ts @@ -1,10 +1,11 @@ // Copyright (c) 2026 the Zuke contributors // SPDX-License-Identifier: MIT -import { assertEquals } from "../../core/tests/_assert.ts"; +import { assertEquals, assertRejects } from "../../core/tests/_assert.ts"; import { aiCache, aiFixer, + AiReviewError, type Assessment, budget, type CacheEntry, @@ -319,6 +320,65 @@ Deno.test("an empty suppress list leaves the findings untouched", async () => { assertEquals(lines.some((l) => l.includes("suppressed")), false); }); +Deno.test("a suppress list that matches no finding drops nothing", async () => { + // A non-empty list whose fingerprints belong to some other finding: the + // assessment must pass through untouched, so the critical finding still + // trips the default gate. + const sup = suppressions((s) => s.add("ffffffffffffffff")); + const { fetch } = recordFetch( + claude({ + score: 9, + severity: "critical", + summary: "bad", + findings: [{ title: "rce", severity: "critical" }], + }), + ); + const lines = await captured(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k").diff((d) => d.text(DIFF)) + .fetch(fetch).suppress(sup) + ).validate({ target: "t" }), + AiReviewError, // the unrelated fingerprint muted nothing + ); + }); + assertEquals(lines.some((l) => l.includes("suppressed")), false); +}); + +Deno.test("the verify pass is skipped once the review call exhausts the budget", async () => { + // The review itself fits under the cap check (0 < 1) but its usage blows the + // cap, so the verify pass that follows must be skipped — visibly — and the + // unverified finding must keep gating. + const b = budget((x) => x.maxTokens(1)); + const { fetch, calls } = recordFetch( + claude( + { + score: 9, + severity: "critical", + summary: "bad", + findings: [{ title: "rce", severity: "critical" }], + }, + { input_tokens: 500, output_tokens: 100 }, + ), + ); + const lines = await captured(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k").diff((d) => d.text(DIFF)) + .fetch(fetch).budget(b).verify() + ).validate({ target: "t" }), + AiReviewError, // the finding was kept, not silently verified away + ); + }); + assertEquals(calls.length, 1); // the review call only — no verify call + assertEquals( + lines.some((l) => l.includes("verify pass skipped — AI budget exhausted")), + true, + ); +}); + // ----- Report rendering ---------------------------------------------------- Deno.test("consoleLines renders cache, suppressed, and budget extras", () => { diff --git a/packages/ai/tests/deep_review_test.ts b/packages/ai/tests/deep_review_test.ts index c4334c4..c548c9b 100644 --- a/packages/ai/tests/deep_review_test.ts +++ b/packages/ai/tests/deep_review_test.ts @@ -311,3 +311,85 @@ Deno.test("verify is skipped cleanly when there are no findings", async () => { ).validate({ target: "t" }); assertEquals(calls.length, 1); // no second call to verify nothing }); + +/** Capture console output with the job-summary file unset (no real writes). */ +async function captured(fn: () => Promise): Promise { + const lines: string[] = []; + const { log, warn } = console; + const summary = Deno.env.get("GITHUB_STEP_SUMMARY"); + Deno.env.delete("GITHUB_STEP_SUMMARY"); + console.log = (...a: unknown[]) => void lines.push(a.join(" ")); + console.warn = (...a: unknown[]) => void lines.push(a.join(" ")); + try { + await fn(); + } finally { + console.log = log; + console.warn = warn; + if (summary !== undefined) Deno.env.set("GITHUB_STEP_SUMMARY", summary); + } + return lines; +} + +Deno.test("verify candidates carry the finding's line; a reason-less refutation renders bare", async () => { + const finding = { + title: "SQL injection", + severity: "high", + file: "db.ts", + line: 3, + }; + const id = findingFingerprint("security", { + title: "SQL injection", + severity: "high", + file: "db.ts", + }); + const { fetch, calls } = queuedFetch([ + claude({ score: 8, severity: "high", findings: [finding] }), + // A refutation with no reason — schema-valid, and must still narrow. + claude({ verdicts: [{ id, verdict: "refuted" }] }), + ]); + const lines = await captured(() => + // Passes: the sole finding was refuted, so the score recomputes to 0. + securityReviewer((r) => + r.provider("claude").apiKey("k") + .diff((d) => d.text(DIFF)).verify() + .fetch(fetch) + ).validate({ target: "t" }) + ); + // The verify prompt carried the line, so the verifier can find the code. + const verify = JSON.parse(calls[1].body); + assertEquals(verify.messages[0].content.includes('"line": 3'), true); + // The refutation is reported without inventing a reason. + const refutedLine = lines.find((l) => l.includes("refuted by verify")); + assertEquals(refutedLine?.includes("SQL injection"), true); + assertEquals(refutedLine?.includes("—"), false); +}); + +Deno.test("a failed verify pass warns on the console when not quiet", async () => { + const { fetch } = queuedFetch([ + claude({ + score: 9, + severity: "critical", + findings: [{ title: "RCE", severity: "critical" }], + }), + { status: 500 }, + ]); + const lines = await captured(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .retry({ attempts: 1 }) + .diff((d) => d.text(DIFF)).verify() + .fetch(fetch) + ).validate({ target: "t" }), + AiReviewError, // the unverified finding stayed and still gates + ); + }); + assertEquals( + lines.some((l) => + l.includes("verify pass failed") && + l.includes("keeping unverified findings") + ), + true, + ); +}); diff --git a/packages/ai/tests/discussion_flow_test.ts b/packages/ai/tests/discussion_flow_test.ts index 95eea93..1bba623 100644 --- a/packages/ai/tests/discussion_flow_test.ts +++ b/packages/ai/tests/discussion_flow_test.ts @@ -2,11 +2,15 @@ // SPDX-License-Identifier: MIT import { assertEquals, assertRejects } from "../../core/tests/_assert.ts"; -import { AiReviewError, securityReviewer } from "../mod.ts"; +import { AiReviewError, budget, securityReviewer } from "../mod.ts"; import { findingFingerprint } from "../src/suppress.ts"; import { decodeState, encodeState } from "../src/state.ts"; import { commentMarker } from "../src/hosts/types.ts"; -import { findingMarker, outcomeMarker } from "../src/threads.ts"; +import { + findingMarker, + MAX_NEW_THREADS, + outcomeMarker, +} from "../src/threads.ts"; import { stableHash } from "../src/hash.ts"; const DIFF = "diff --git a/src/app.ts b/src/app.ts\n" + @@ -2481,3 +2485,372 @@ Deno.test("a quiet reviewer posts no threads either", async () => { ); assertEquals(threadPosts(calls).length, 0); }); + +// ─── Degraded paths: no host, exhausted budgets, failing passes ────────────── + +Deno.test("comment and discussion degrade to warnings outside any CI host", async () => { + // An env reader that sees nothing: no CI host markers at all (a local run). + const { fetch, calls } = discussionFetch([], [ + claude({ score: 0, severity: "none", findings: [] }), + ]); + const lines = await captured(() => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion() + .env(() => undefined) + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }) + ); + // The discussion is declined in code, before any listing is attempted… + assertEquals( + lines.some((l) => + l.includes("discussion disabled") && + l.includes("the active host cannot list PR comments") + ), + true, + ); + // …and the comment is skipped with its own warning, not a crash. + assertEquals( + lines.some((l) => + l.includes("no PR-comment host detected — skipping comment") + ), + true, + ); + // No host traffic happened at all. + assertEquals(calls.every((c) => !c.url.startsWith(`${GITHUB_API}/`)), true); +}); + +/** Wrap a payload in a Claude response that also reports token usage. */ +function claudeWithUsage( + payload: unknown, + usage: Record, +): string { + return JSON.stringify({ + content: [{ type: "text", text: JSON.stringify(payload) }], + stop_reason: "end_turn", + usage, + }); +} + +Deno.test("an exhausted budget skips the reworded-finding check, and says so", async () => { + // The review call's own usage blows the cap, so the dedup pass that would + // resolve the rewording must be skipped — with a note, never silently — and + // the finding keeps its fresh identity and gates. + const b = budget((x) => x.maxTokens(1)); + const { fetch, calls } = discussionFetch( + priorComment(stateWith("dismissed")), + [claudeWithUsage({ score: 9, severity: "high", findings: [REWORDED] }, { + input_tokens: 500, + output_tokens: 100, + })], + ); + const lines = await captured(() => + inPr(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion().budget(b) + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }), + AiReviewError, // reported under its own id — not silenced + ); + }) + ); + // The review call only: no dedup call was paid for. + assertEquals( + calls.filter((c) => !c.url.startsWith(`${GITHUB_API}/`)).length, + 1, + ); + assertEquals( + lines.some((l) => + l.includes("reworded-finding check skipped — AI budget exhausted") + ), + true, + ); + // The finding kept its own identity in the posted state. + assertEquals( + postedState(calls)?.findings.some((f) => f.id === REWORDED_ID), + true, + ); +}); + +Deno.test("an ambiguous rewording keeps the first match and reports the rest, with the cap named", async () => { + // Four earlier open findings in the same file are all eligible comparisons + // for one candidate; the per-candidate cap (3) drops the fourth, and the + // model then claims the candidate matches two of the three offered. + const priors = Array.from({ length: 4 }, (_, i) => ({ + id: `aaaa000${i + 1}`, + title: `Earlier concern ${i + 1}`, + severity: "high" as const, + status: "open" as const, + file: "src/app.ts", + })); + const { fetch, calls } = discussionFetch( + priorComment({ findings: priors }), + [ + claude({ score: 9, severity: "high", findings: [REWORDED] }), + claude({ + verdicts: [ + { id: "p1", verdict: "same", reason: "same defect" }, + { id: "p2", verdict: "same", reason: "also this one" }, + ], + }), + ], + ); + const lines = await captured(() => + inPr(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion() + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }), + AiReviewError, // adopted an OPEN identity — still gating + ); + }) + ); + // First offered wins: the candidate adopted the first prior's identity. + const state = postedState(calls); + assertEquals( + state?.findings.find((f) => f.id === "aaaa0001")?.status, + "open", + ); + assertEquals( + state?.findings.find((f) => f.id === "aaaa0001")?.aliases, + [REWORDED_ID], + ); + // The double match is reported against the candidate's fresh id… + assertEquals( + lines.some((l) => + l.includes(REWORDED_ID) && + l.includes("matched more than one earlier finding") + ), + true, + ); + // …and the dropped fourth comparison is announced, not silent. + assertEquals( + lines.some((l) => + l.includes("reworded-finding check compared 3 of 4 candidate pairs") + ), + true, + ); +}); + +Deno.test("the finding's detail reaches the adjudicator, and a reason-less dismissal sticks", async () => { + const detailed = { + ...FINDING, + detail: "input flows straight into eval with no sanitisation", + }; + const comments = [ + { + id: 1, + body: `Finding ${ID} misreads the code: eval runs in a sandboxed worker`, + user: { login: "maintainer", type: "User" }, + author_association: "MEMBER", + }, + ]; + const { fetch, calls } = discussionFetch(comments, [ + claude({ score: 9, severity: "high", findings: [detailed] }), + // A dismissal verdict carrying no reason — still a valid two-key dismissal. + claude({ verdicts: [{ id: ID, verdict: "dismissed" }] }), + ]); + const lines = await captured(() => + inPr(async () => { + // Dismissed, so the gate does not trip. + await securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion() + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }); + }) + ); + const providerCalls = calls.filter((c) => + !c.url.startsWith(`${GITHUB_API}/`) + ); + assertEquals(providerCalls.length, 2); + // The adjudication prompt carried the finding's detail for context. + assertEquals( + JSON.parse(providerCalls[1].body).messages[0].content.includes( + "input flows straight into eval with no sanitisation", + ), + true, + ); + assertEquals( + lines.some((l) => l.includes("dismissed via discussion by maintainer")), + true, + ); + // Recorded dismissed with the author but no invented rationale. + const entry = postedState(calls)?.findings.find((f) => f.id === ID); + assertEquals(entry?.status, "dismissed"); + assertEquals(entry?.author, "maintainer"); + assertEquals(entry?.rationale, undefined); +}); + +Deno.test("a failed adjudication keeps contested findings open with a warning", async () => { + const calls: Call[] = []; + let served = 0; + const failing = ((input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + const method = init?.method ?? "GET"; + calls.push({ + url, + method, + body: typeof init?.body === "string" ? init.body : "", + }); + if (url.startsWith(`${GITHUB_API}/`)) { + if (url.endsWith("/user")) { + return Promise.resolve(new Response("{}", { status: 403 })); + } + const payload = method === "GET" + ? JSON.stringify([{ + id: 1, + body: `Finding ${ID} misreads the code`, + user: { login: "maintainer", type: "User" }, + author_association: "MEMBER", + }]) + : "{}"; + return Promise.resolve(new Response(payload, { status: 200 })); + } + served++; + // The review answers; the adjudication call that follows fails outright. + return Promise.resolve( + served === 1 + ? new Response( + claude({ score: 9, severity: "high", findings: [FINDING] }), + { status: 200 }, + ) + : new Response("upstream exploded", { status: 500 }), + ); + }) as typeof fetch; + const lines = await captured(() => + inPr(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion().retry({ attempts: 1 }) + .diff((d) => d.text(DIFF)) + .fetch(failing) + ).validate({ target: "t" }), + AiReviewError, // the contested finding stayed open and gates + ); + }) + ); + assertEquals( + lines.some((l) => + l.includes("adjudication failed") && + l.includes("contested findings stay open") + ), + true, + ); +}); + +Deno.test("findings beyond the thread cap stay in the table, and the gap is announced", async () => { + // One more anchorable finding than the per-run thread cap. + const count = MAX_NEW_THREADS + 1; + const bigDiff = [ + "diff --git a/src/big.ts b/src/big.ts", + "--- a/src/big.ts", + "+++ b/src/big.ts", + `@@ -1,0 +1,${count} @@`, + ...Array.from({ length: count }, (_, i) => `+const v${i + 1} = ${i + 1};`), + ].join("\n"); + const many = Array.from({ length: count }, (_, i) => ({ + title: `Issue number ${i + 1}`, + severity: "high", + file: "src/big.ts", + line: i + 1, + })); + const { fetch, calls } = threadFetch([summaryComment()], [], [ + claude({ score: 9, severity: "high", findings: many }), + ]); + await captured(() => + inPr(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion((d) => d.threads()) + .diff((d) => d.text(bigDiff)) + .fetch(fetch) + ).validate({ target: "t" }), + AiReviewError, + ); + }) + ); + // Exactly the cap was opened, and the leftover is promised to the next run. + assertEquals(threadPosts(calls).length, MAX_NEW_THREADS); + const write = calls.find((c) => + c.url.includes("/issues/") && c.method !== "GET" + ); + assertEquals( + JSON.parse(write?.body ?? "{}").body.includes( + `1 finding(s) did not get a review thread this run (cap ${MAX_NEW_THREADS})`, + ), + true, + ); +}); + +Deno.test("a thread phase that throws degrades to a note, never a failure", async () => { + // The head-commit lookup explodes (HTTP 500) after the thread listing + // succeeded — the whole phase must collapse into a report note. + const calls: Call[] = []; + const doFetch = ((input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + const method = init?.method ?? "GET"; + calls.push({ + url, + method, + body: typeof init?.body === "string" ? init.body : "", + }); + if (url.startsWith(`${GITHUB_API}/`)) { + if (url.endsWith("/user")) { + return Promise.resolve(new Response("{}", { status: 403 })); + } + if (method !== "GET") return Promise.resolve(new Response("{}")); + if (url.includes("/pulls/7/comments")) { + return Promise.resolve(new Response("[]")); + } + if (url.includes("/pulls/7")) { + return Promise.resolve(new Response("boom", { status: 500 })); + } + return Promise.resolve(new Response(JSON.stringify([summaryComment()]))); + } + return Promise.resolve( + new Response( + claude({ score: 9, severity: "high", findings: [ANCHORED] }), + { status: 200 }, + ), + ); + }) as typeof fetch; + await captured(() => + inPr(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion((d) => d.threads()) + .diff((d) => d.text(ANCHORED_DIFF)) + .fetch(doFetch) + ).validate({ target: "t" }), + AiReviewError, // the gate still speaks — the finding never vanished + ); + }) + ); + assertEquals(threadPosts(calls).length, 0); + const write = calls.find((c) => + c.url.includes("/issues/") && c.method !== "GET" + ); + assertEquals( + JSON.parse(write?.body ?? "{}").body.includes( + "review threads unavailable this run", + ), + true, + ); +}); diff --git a/packages/core/tests/ci_schedule_test.ts b/packages/core/tests/ci_schedule_test.ts index d20960d..bb568ff 100644 --- a/packages/core/tests/ci_schedule_test.ts +++ b/packages/core/tests/ci_schedule_test.ts @@ -125,6 +125,58 @@ Deno.test("utcCronsFor rejects the unsupported cases with a friendly error", () ); }); +Deno.test("utcCronsFor expands a */step field over the whole range", () => { + // `*/15` strides the full minute range, then the hour shifts for the zone. + assertEquals( + utcCronsFor({ cron: "*/15 9 * * *", tz: "Etc/GMT-2" }), + ["0,15,30,45 7 * * *"], + ); +}); + +Deno.test("utcCronsFor rejects malformed step syntax with a friendly error", () => { + // More than one slash in a field. + assertThrows( + () => utcCronsFor({ cron: "1/2/3 0 * * *" }), + Error, + "invalid cron field", + ); + // A zero stride and a non-numeric stride are both named as bad steps. + assertThrows( + () => utcCronsFor({ cron: "*/0 0 * * *" }), + Error, + "invalid step in cron field", + ); + assertThrows( + () => utcCronsFor({ cron: "5/x 0 * * *" }), + Error, + "invalid step in cron field", + ); +}); + +Deno.test("utcCronsFor shifts an every-hour schedule without touching the days", () => { + // With hour `*` the shift is a no-op: every hour maps to every hour, so the + // day fields survive verbatim and no "crosses a day boundary" error can fire + // even though day-of-month, month, and day-of-week are all constrained. + assertEquals( + utcCronsFor({ cron: "30 * 1,15 6 1", tz: "Etc/GMT-2" }), + ["30 * 1,15 6 1"], + ); +}); + +Deno.test("guardShell renders a wildcard field as an always-true test", () => { + // A `*` minute matches every wall-clock minute, so its membership test must + // collapse to `true` rather than an empty case list that never matches. + const shell = guardShell([{ cron: "* 9 * * *", tz: "Europe/Sofia" }]); + assertStringIncludes(shell, "if true && "); + assertStringIncludes(shell, 'case " 9 " in *" $hh "*)'); +}); + +Deno.test("guardShell tests the month when the schedule restricts it", () => { + const shell = guardShell([{ cron: "0 12 * 6 *", tz: "Europe/Sofia" }]); + assertStringIncludes(shell, "TZ='Europe/Sofia' date +%m"); + assertStringIncludes(shell, 'case " 6 " in *" $mo "*)'); +}); + Deno.test("anyScheduleNeedsGuard is true only when a DST zone is present", () => { assertEquals( anyScheduleNeedsGuard([{ cron: "0 6 * * *" }, { diff --git a/packages/core/tests/ci_test.ts b/packages/core/tests/ci_test.ts index db41de8..3441487 100644 --- a/packages/core/tests/ci_test.ts +++ b/packages/core/tests/ci_test.ts @@ -1,7 +1,12 @@ // Copyright (c) 2026 the Zuke contributors // SPDX-License-Identifier: MIT -import { assertEquals, assertStringIncludes } from "./_assert.ts"; +import { + assertEquals, + assertRejects, + assertStringIncludes, + assertThrows, +} from "./_assert.ts"; import { cicd, CiFile, @@ -1178,6 +1183,105 @@ Deno.test("the two security-relevant inputs are always stated", () => { assertStringIncludes(yaml, 'persist-credentials: "false"'); }); +Deno.test("github: a checkout ref renders on the separate checkout step", () => { + const yaml = generateCi({ + bootstrap: false, + checkout: { action: `actions/checkout@${"a".repeat(40)}`, ref: "release" }, + jobs: [{ id: "gate", steps: [{ run: "./zuke gate" }] }], + }, "github"); + assertStringIncludes(yaml, `actions/checkout@${"a".repeat(40)}`); + assertStringIncludes(yaml, "ref: release"); +}); + +Deno.test("a checkout without a pin is a friendly error when bootstrap is off", () => { + // Emitting `uses:` with no ref would be a floating reference; like an + // unpinned harden, an unpinned checkout must fail rather than degrade. + assertThrows( + () => + generateCi({ + bootstrap: false, + checkout: {}, + jobs: [{ steps: [{ run: "x" }] }], + }, "github"), + Error, + "the checkout needs a pinned action", + ); +}); + +Deno.test("bootstrap: false with no job steps still runs the default step", () => { + // Opting out of the prelude action must not also lose the default build step. + const yaml = generateCi( + { bootstrap: false, jobs: [{ id: "plain" }] }, + "github", + ); + assertStringIncludes(yaml, "run: ./zuke"); + assertEquals(yaml.includes("zuke-build/zuke"), false); +}); + +Deno.test("azure: a schedule without a push trigger defaults to main", () => { + // Azure schedules need a branch filter; with no push trigger to mirror, the + // conventional default branch stands in. + const yaml = generateCi( + { triggers: { schedule: [{ cron: "0 6 * * *" }] } }, + "azure", + ); + assertStringIncludes(yaml, "schedules:"); + assertStringIncludes(yaml, "branches:\n include:\n - main"); +}); + +Deno.test("a job's own bootstrap pin overrides the resolver", () => { + // A job that names the prelude action asked for that exact pin; the resolver + // must not replace it. + class B extends Build { + gate = target().executes(() => {}); + wf = cicd({ + pins: (action) => `${action}@${"9".repeat(40)}`, + invokes: [{ + target: this.gate, + bootstrap: { action: `my-org/zuke@${"8".repeat(40)}` }, + }], + }); + } + const yaml = discoverCiFiles(new B())[0].render(); + assertStringIncludes(yaml, `my-org/zuke@${"8".repeat(40)}`); + assertEquals(yaml.includes("zuke-build/zuke"), false); +}); + +Deno.test("a field name that is all suffix keeps the provider default path", () => { + // `Ci` reduces to nothing once the suffix is dropped, so the provider's + // conventional file name stands in rather than an empty ".yml". + class B extends Build { + gate = target().executes(() => {}); + Ci = cicd({ invokes: [this.gate] }); + } + assertEquals(discoverCiFiles(new B())[0].path, ".github/workflows/ci.yml"); +}); + +Deno.test("pipelineFor names a not-yet-discovered target by identity", () => { + // Before discoverTargets assigns names, an invoked target can still be + // resolved by identity against the map — what lets a workflow be declared + // with `this.ci` in a field initialiser. + const gate = target().description("Gate").executes(() => {}); + const file = cicd({ provider: "github", invokes: [gate] }); + const pipeline = file.pipelineFor(new Map([["gate", gate]])); + assertEquals(pipeline.jobs?.[0].id, "gate"); + assertEquals(pipeline.jobs?.[0].name, "Gate"); +}); + +Deno.test("syncCiFiles surfaces a read failure that is not file-absence", async () => { + // Only NotFound means "write it fresh"; any other read failure (here: the + // path is a directory) must propagate, not be mistaken for a missing file. + const dir = await Deno.makeTempDir(); + try { + const path = `${dir}/ci.yml`; + await Deno.mkdir(path); // a directory where the file should be + const file = cicd({ provider: "github", path, pipeline: filePipeline }); + await assertRejects(() => syncCiFiles([file], { check: true })); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + Deno.test("a job whose own steps already harden or check out gets no prelude", () => { // The prelude became a default for pipelines that never asked for one, so it // has to notice a job already doing the work itself — otherwise hardening diff --git a/packages/core/tests/cli_test.ts b/packages/core/tests/cli_test.ts index 5ac5285..b54811b 100644 --- a/packages/core/tests/cli_test.ts +++ b/packages/core/tests/cli_test.ts @@ -1,7 +1,12 @@ // Copyright (c) 2026 the Zuke contributors // SPDX-License-Identifier: MIT -import { assertEquals, assertStringIncludes, assertThrows } from "./_assert.ts"; +import { + assertEquals, + assertRejects, + assertStringIncludes, + assertThrows, +} from "./_assert.ts"; import { Build, cicd, group, type Plugin, target } from "../mod.ts"; import { formatGraph, diff --git a/packages/core/tests/describe_test.ts b/packages/core/tests/describe_test.ts index 43040e7..27857c6 100644 --- a/packages/core/tests/describe_test.ts +++ b/packages/core/tests/describe_test.ts @@ -91,6 +91,29 @@ Deno.test("describeCli reports the property name, kind, and default", () => { assertEquals(params.find((p) => p.name === "region")?.default, "eu"); }); +Deno.test("describeCli never surfaces a secret parameter's default value", () => { + class WithSecret extends Build { + // A secret's declared default could itself be a live credential. + apiKey = parameter("API key").secret().default("sk-live-real"); + region = parameter("Region").default("eu"); + } + const params = describeCli(new WithSecret()).parameters; + assertEquals(params.find((p) => p.name === "apiKey")?.default, undefined); + // A non-secret default still surfaces, so the omission is targeted. + assertEquals(params.find((p) => p.name === "region")?.default, "eu"); +}); + +Deno.test("describeCli names a dependency that is not a build field with ?", () => { + // A dependency that was never discovered as a field has no name; the surface + // reports the placeholder rather than crashing or silently dropping the edge. + const anon = target().executes(() => {}); + class B extends Build { + build = target().dependsOn(anon).executes(() => {}); + } + const build = describeCli(new B()).targets.find((t) => t.name === "build"); + assertEquals(build?.dependsOn, ["?"]); +}); + Deno.test("describeCli omitSecrets drops secret parameters entirely", () => { class WithSecret extends Build { apiKey = parameter("API key").secret(); diff --git a/packages/gh/tests/workflow_test.ts b/packages/gh/tests/workflow_test.ts index 580e83f..497b176 100644 --- a/packages/gh/tests/workflow_test.ts +++ b/packages/gh/tests/workflow_test.ts @@ -944,6 +944,55 @@ Deno.test("RestGhWorkflowApi aborts a hung request via its timeout", async () => await assertRejects(() => api.getRun("a/b", 1)); // aborts, does not hang }); +Deno.test("the default transport reads GH_TOKEN before GITHUB_TOKEN", async () => { + // The same order the gh CLI resolves its token, injected through the readEnv + // seam so the test never touches the real environment. + const url = "https://api.github.com/repos/a/b/actions/workflows/w/dispatches"; + const both = routerFetch({ [`POST ${url}`]: {} }); + const preferGh = githubWorkflowWith((g) => g.repo("a/b").workflow("w"), { + fetch: both.fetch, + readEnv: (name) => + name === "GH_TOKEN" + ? "gh_tok" + : name === "GITHUB_TOKEN" + ? "shadowed" + : undefined, + }); + await preferGh.isSatisfied(NO_SIGNALS, ctx(fakeState())); // dispatch + assertEquals( + new Headers(both.calls[0].init?.headers).get("authorization"), + "Bearer gh_tok", + ); + + // Without GH_TOKEN the workflow token GITHUB_TOKEN fills in. + const fallback = routerFetch({ [`POST ${url}`]: {} }); + const usesGithub = githubWorkflowWith((g) => g.repo("a/b").workflow("w"), { + fetch: fallback.fetch, + readEnv: (name) => name === "GITHUB_TOKEN" ? "fallback_tok" : undefined, + }); + await usesGithub.isSatisfied(NO_SIGNALS, ctx(fakeState())); + assertEquals( + new Headers(fallback.calls[0].init?.headers).get("authorization"), + "Bearer fallback_tok", + ); +}); + +Deno.test("RestGhWorkflowApi tolerates a 2xx body that is not an object", async () => { + // A proxy answering with a JSON array instead of the run object: the read + // degrades to the same defaults as an empty run, not a crash on indexing. + const { fetch } = routerFetch({ + "GET https://api.github.com/repos/a/b/actions/runs/9": [], + }); + assertEquals(await new RestGhWorkflowApi({ fetch }).getRun("a/b", 9), { + id: 0, + status: "unknown", + conclusion: null, + url: "", + createdAt: "", + headBranch: "", + }); +}); + Deno.test("RestGhWorkflowApi maps missing run/job fields to defaults", async () => { const { fetch } = routerFetch({ "GET https://api.github.com/repos/a/b/actions/runs/9": {}, // no fields From 1958ccf2561d6ba24d0e15c49f6ae4cb073daa6b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 12:03:47 +0000 Subject: [PATCH 05/11] test: expand coverage of the core cli and state suites Further checkpoint of the coverage push: branch tests for CLI flag parsing and state serialization edge paths, against existing fixtures. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019As9A3ugixLQiAvcat1ZKk --- packages/core/tests/cli_test.ts | 207 +++++++++++ packages/core/tests/mcp_runtools_test.ts | 442 +++++++++++++++++++++++ packages/core/tests/state_test.ts | 282 +++++++++++++++ 3 files changed, 931 insertions(+) create mode 100644 packages/core/tests/mcp_runtools_test.ts diff --git a/packages/core/tests/cli_test.ts b/packages/core/tests/cli_test.ts index b54811b..f5b43d4 100644 --- a/packages/core/tests/cli_test.ts +++ b/packages/core/tests/cli_test.ts @@ -1121,6 +1121,213 @@ Deno.test("run forwards args and plugins to main", async () => { assertEquals(seen, ["finished"]); }); +Deno.test("parseArgs reads the inline = forms of --signal and --data", () => { + const p = parseArgs([ + "resume", + "run-9", + "--signal=approved", + '--data={"by":"qa"}', + ]); + assertEquals(p.signal, "approved"); + assertEquals(p.data, '{"by":"qa"}'); +}); + +Deno.test("parseArgs reads --allow-run globs and --protect lists", () => { + // The comma list is trimmed and empties are dropped. + const globbed = parseArgs(["mcp", "--allow-run=deploy-*, ,test"]); + assertEquals(globbed.allowRun, true); + assertEquals(globbed.allowRunPatterns, ["deploy-*", "test"]); + // Bare --allow-run enables runs with no pattern restriction. + const bare = parseArgs(["mcp", "--allow-run"]); + assertEquals([bare.allowRun, bare.allowRunPatterns], [true, undefined]); + + const spaced = parseArgs(["mcp", "--protect", "prod-*"]); + assertEquals(spaced.protectPatterns, ["prod-*"]); + const inline = parseArgs(["mcp", "--protect=a,b"]); + assertEquals(inline.protectPatterns, ["a", "b"]); +}); + +Deno.test("parseArgs treats an empty --parallel= value as plain --parallel", () => { + assertEquals(parseArgs(["build", "--parallel="]).parallel, true); +}); + +Deno.test("main: resume without a run id prints usage and fails", async () => { + const { code, err } = await capture(() => main(Demo, ["resume"])); + assertEquals(code, 1); + assertStringIncludes(err.join("\n"), "Usage: zuke resume "); +}); + +Deno.test("main: resume rejects invalid --data JSON without echoing it", async () => { + const { code, err } = await capture(() => + main(Demo, ["resume", "run-x", "--data", "{oops"]) + ); + assertEquals(code, 1); + const text = err.join("\n"); + assertStringIncludes(text, "--data is not valid JSON"); + // The payload could be large or sensitive; it must not be quoted back. + assertEquals(text.includes("{oops"), false); +}); + +Deno.test("main: mcp --http refuses a non-loopback bind without a token", async () => { + // The refusal happens before any socket is bound, so the test stays hermetic. + const saved = Deno.env.get("ZUKE_MCP_TOKEN"); + Deno.env.delete("ZUKE_MCP_TOKEN"); + try { + const { code, err } = await capture(() => + main(Demo, ["mcp", "--http", "0.0.0.0:8123"]) + ); + assertEquals(code, 1); + assertStringIncludes(err.join("\n"), "refusing to bind"); + } finally { + if (saved !== undefined) Deno.env.set("ZUKE_MCP_TOKEN", saved); + } +}); + +Deno.test("main: cancel reports a missing run as a friendly error", async () => { + const dir = await Deno.makeTempDir(); + try { + const store = new FileSystemStateStore(dir, defaultStateHost); + class Stateful extends Build { + override stateStore() { + return store; + } + build = target().executes(() => {}); + } + const { code, err } = await capture(() => + main(Stateful, ["cancel", "ghost"]) + ); + assertEquals(code, 1); + assertStringIncludes(err.join("\n"), 'no run "ghost"'); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("main: runs list applies --status, --target, and --since filters", async () => { + const dir = await Deno.makeTempDir(); + try { + const store = new FileSystemStateStore(dir, defaultStateHost); + await store.putRun(sampleRunRecord({ id: "ok-run" }), null); + await store.putRun( + sampleRunRecord({ + id: "old-fail", + status: "failed", + createdAt: "2020-01-01T00:00:00.000Z", + updatedAt: "2020-01-01T00:00:00.000Z", + rootTarget: "deploy", + graph: [{ name: "deploy", dependsOn: [] }], + targets: { deploy: { status: "failed", meta: {} } }, + }), + null, + ); + class Stateful extends Build { + override stateStore() { + return store; + } + build = target().executes(() => {}); + } + const ids = async (...filters: string[]) => { + const { code, out } = await capture(() => + main(Stateful, ["runs", "list", ...filters, "--json"]) + ); + assertEquals(code, 0); + const rows: Array<{ id: string }> = JSON.parse(out.join("\n")); + return rows.map((r) => r.id); + }; + assertEquals(await ids("--status", "succeeded"), ["ok-run"]); + assertEquals(await ids("--target", "deploy"), ["old-fail"]); + assertEquals(await ids("--since", "2026-01-01T00:00:00.000Z"), ["ok-run"]); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +Deno.test("main: --affected rejects a git base that reads as a git option", async () => { + // A base beginning with "-" would be read by git as an option (e.g. + // `--output=…` writes to a file), so it is rejected before any git process + // is spawned. The failure is not a configuration error main knows how to + // present, so it propagates rather than being flattened into exit 1. + const error = await assertRejects(() => + capture(() => main(Demo, ["build", "--affected=-o"])) + ); + assertStringIncludes(error.message, "invalid git base revision"); +}); + +Deno.test("main watchSignals: a signal cancels the run, a second force-exits", async () => { + const handlers: Array<() => void> = []; + const removed: Deno.Signal[] = []; + const origAdd = Deno.addSignalListener; + const origRemove = Deno.removeSignalListener; + const origExit = Deno.exit; + Deno.addSignalListener = (_sig: Deno.Signal, fn: () => void): void => { + handlers.push(fn); + }; + Deno.removeSignalListener = (sig: Deno.Signal, _fn: () => void): void => { + removed.push(sig); + }; + const ran: string[] = []; + class Interrupted extends Build { + first = target().executes(() => { + ran.push("first"); + handlers[0](); // Ctrl-C arrives mid-build + }); + second = target().dependsOn(this.first).executes( + () => void ran.push("second"), + ); + } + try { + const { code } = await capture(() => + main(Interrupted, ["second"], { watchSignals: true }) + ); + assertEquals(code, 1); // the run was cancelled, not completed + assertEquals(ran, ["first"]); // `second` never started + // Every installed handler was removed on the way out. + assertEquals(removed.length, handlers.length); + + // A second signal while cancellation is in flight force-exits 130. + let exitCode: number | undefined; + Deno.exit = (code?: number): never => { + exitCode = code; + throw new ExitSignal(); + }; + try { + handlers[0](); + } catch (e) { + if (!(e instanceof ExitSignal)) throw e; + } + assertEquals(exitCode, 130); + } finally { + Deno.addSignalListener = origAdd; + Deno.removeSignalListener = origRemove; + Deno.exit = origExit; + } +}); + +Deno.test("main watchSignals: unsupported signals are skipped, teardown is best-effort", async () => { + const origAdd = Deno.addSignalListener; + const origRemove = Deno.removeSignalListener; + let installed = 0; + Deno.addSignalListener = (sig: Deno.Signal, _fn: () => void): void => { + // A platform that rejects everything but SIGINT (e.g. SIGTERM on Windows). + if (sig !== "SIGINT") throw new TypeError(`unsupported signal ${sig}`); + installed++; + }; + Deno.removeSignalListener = (): void => { + throw new TypeError("teardown refused"); + }; + try { + const { code } = await capture(() => + main(Demo, ["build"], { watchSignals: true }) + ); + // Neither the rejected install nor the failing teardown surfaces. + assertEquals(code, 0); + assertEquals(installed, 1); + } finally { + Deno.addSignalListener = origAdd; + Deno.removeSignalListener = origRemove; + } +}); + Deno.test("main: resume rejects an oversized --data payload", async () => { const huge = JSON.stringify({ blob: "x".repeat(70_000) }); const { code, err } = await capture(() => diff --git a/packages/core/tests/mcp_runtools_test.ts b/packages/core/tests/mcp_runtools_test.ts new file mode 100644 index 0000000..04b2c93 --- /dev/null +++ b/packages/core/tests/mcp_runtools_test.ts @@ -0,0 +1,442 @@ +// Copyright (c) 2026 the Zuke contributors +// SPDX-License-Identifier: MIT + +/** + * The store-backed MCP run tools driven end-to-end: `list_runs` filters, + * `show_run`'s full record, and `signal_run`/`resume_check`/`cancel_run` + * advancing **real suspended runs** (created by `execute` suspending at a + * `waitsFor` gate). Typed failures — a lost resume race, a lock conflict, a + * failing resumed target, a store fault mid-operation — must come back as + * structured JSON tool results the client can act on, never as flattened + * strings or transport crashes. + * + * @module + */ + +import { assertEquals, assertStringIncludes } from "./_assert.ts"; +import { Build, discoverTargets } from "../src/build.ts"; +import { target, type TargetBuilder } from "../src/target.ts"; +import { execute } from "../src/executor.ts"; +import { externalSignal } from "../src/wait.ts"; +import { McpServer } from "../src/mcp/server.ts"; +import { callRunStateTool, type RunToolDeps } from "../src/mcp/runtools.ts"; +import { acquireLease, RUN_LEASE_PREFIX } from "../src/state/run_lease.ts"; +import { LockConflictError } from "../src/state/lock.ts"; +import { FileSystemStateStore } from "../src/state/fs_store.ts"; +import { + defaultStateHost, + type StateStore, + type StateStoreScope, +} from "../src/state/store.ts"; +import type { RunRecord } from "../src/state/types.ts"; + +/** A JSON-RPC request with id 1. */ +function req(method: string, params?: unknown): Record { + return { jsonrpc: "2.0", id: 1, method, ...(params ? { params } : {}) }; +} + +/** The `{ text, isError }` of a `tools/call` result (no casts). */ +function isRec(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** Call a tool through the server and return its text block. */ +async function call( + server: McpServer, + name: string, + args: Record = {}, +): Promise<{ text: string; isError: boolean }> { + const res = await server.handleMessage( + req("tools/call", { name, arguments: args }), + ); + if (!isRec(res) || !isRec(res.result)) { + throw new Error(`expected a result: ${JSON.stringify(res)}`); + } + const content = res.result.content; + if ( + !Array.isArray(content) || !isRec(content[0]) || + typeof content[0].text !== "string" + ) { + throw new Error("not a tool result"); + } + return { text: content[0].text, isError: res.result.isError === true }; +} + +/** Run `fn` with a real temp-dir store, cleaned up afterwards. */ +async function withStore( + fn: (store: FileSystemStateStore) => Promise, +): Promise { + const dir = await Deno.makeTempDir(); + try { + await fn(new FileSystemStateStore(`${dir}/runs`, defaultStateHost)); + } finally { + await Deno.remove(dir, { recursive: true }); + } +} + +/** Execute `root` until it suspends at its gate, returning the run id. */ +async function suspend( + build: Build, + root: TargetBuilder, + store: StateStore, +): Promise { + const result = await execute(build, root, { + silent: true, + stateStore: store, + readEnv: () => undefined, + }); + if (result.suspended !== true || result.runId === undefined) { + throw new Error("fixture did not suspend"); + } + return result.runId; +} + +/** A pipeline suspending at an `approved` gate, recording the signal payload. */ +function makePipeline(): { build: Build; root: TargetBuilder; seen: unknown[] } { + const seen: unknown[] = []; + class Pipeline extends Build { + gate = target().description("Gate") + .waitsFor((s) => s.on(externalSignal("approved"))); + promote = target().dependsOn(this.gate).executes((ctx) => { + seen.push(ctx.signals.get("approved")?.data ?? null); + }); + } + const build = new Pipeline(); + discoverTargets(build); + return { build, root: build.promote, seen }; +} + +/** Persist a run record directly (for filter tests), returning its id. */ +async function seedRun( + store: StateStore, + id: string, + rootTarget: string, + status: "suspended" | "failed", + createdAt: string, +): Promise { + const record: RunRecord = { + id, + build: "Pipeline", + rootTarget, + status, + actor: "someone", + createdAt, + updatedAt: createdAt, + graph: [{ name: rootTarget, dependsOn: [] }], + params: {}, + targets: { [rootTarget]: { status: "waiting", meta: {} } }, + signals: {}, + events: [], + }; + const put = await store.putRun(record, null); + if (!put.ok) throw new Error("failed to seed run"); + return id; +} + +Deno.test("list_runs filters by status, target, and since; an unknown status is refused", async () => { + await withStore(async (store) => { + const { build } = makePipeline(); + const server = new McpServer(build, { allowRun: true, stateStore: store }); + await seedRun(store, "run-old", "deploy", "suspended", "2026-01-01T00:00:00.000Z"); + await seedRun(store, "run-new", "promote", "failed", "2026-06-01T00:00:00.000Z"); + + const byStatus = await call(server, "list_runs", { status: "failed" }); + assertEquals(byStatus.isError, false); + assertEquals(JSON.parse(byStatus.text).map((r: { id: string }) => r.id), [ + "run-new", + ]); + + const byTarget = await call(server, "list_runs", { target: "deploy" }); + assertEquals(JSON.parse(byTarget.text).map((r: { id: string }) => r.id), [ + "run-old", + ]); + + const since = await call(server, "list_runs", { + since: "2026-03-01T00:00:00.000Z", + }); + assertEquals(JSON.parse(since.text).map((r: { id: string }) => r.id), [ + "run-new", + ]); + + // An unknown status is a structured refusal naming the allowed values. + const bad = await call(server, "list_runs", { status: "bogus" }); + assertEquals(bad.isError, true); + const body = JSON.parse(bad.text); + assertEquals(body.error, "invalid_status"); + assertEquals(body.status, "bogus"); + assertEquals(body.allowed.includes("suspended"), true); + }); +}); + +Deno.test("show_run returns the full record; a missing runId is a structured error", async () => { + await withStore(async (store) => { + const { build } = makePipeline(); + const server = new McpServer(build, { allowRun: true, stateStore: store }); + await seedRun(store, "run-a", "promote", "suspended", "2026-01-01T00:00:00.000Z"); + + const shown = await call(server, "show_run", { runId: "run-a" }); + assertEquals(shown.isError, false); + const record = JSON.parse(shown.text); + assertEquals(record.id, "run-a"); + assertEquals(record.rootTarget, "promote"); + assertEquals(record.status, "suspended"); + assertEquals(record.targets.promote.status, "waiting"); + + // Each tool taking runId reports the missing argument the same way. + for (const tool of ["show_run", "signal_run", "cancel_run"]) { + const missing = await call(server, tool); + assertEquals(missing.isError, true, `${tool} did not error`); + const body = JSON.parse(missing.text); + assertEquals(body.error, "missing_argument"); + assertEquals(body.argument, "runId"); + } + }); +}); + +Deno.test("signal_run delivers the payload and resumes the run to completion", async () => { + await withStore(async (store) => { + const { build, root, seen } = makePipeline(); + const server = new McpServer(build, { + allowRun: true, + stateStore: store, + actor: "op", + }); + const runId = await suspend(build, root, store); + + const result = await call(server, "signal_run", { + runId, + signal: "approved", + data: { by: "qa" }, + }); + assertEquals(result.isError, false); + const body = JSON.parse(result.text); + assertEquals(body.ok, true); + assertEquals(body.runId, runId); + assertEquals(body.suspended, false); + assertEquals(body.executed.includes("promote"), true); + // The resumed target saw the delivered payload… + assertEquals(seen, [{ by: "qa" }]); + // …and the durable record settled. + const loaded = await store.getRun(runId); + assertEquals(loaded?.record.status, "succeeded"); + }); +}); + +Deno.test("signal_run with no satisfying signal re-suspends and reports it", async () => { + await withStore(async (store) => { + const { build, root, seen } = makePipeline(); + const server = new McpServer(build, { allowRun: true, stateStore: store }); + const runId = await suspend(build, root, store); + + // No signal delivered: the gate stays unsatisfied and the run parks again. + const result = await call(server, "signal_run", { runId }); + assertEquals(result.isError, false); + const body = JSON.parse(result.text); + assertEquals(body.ok, true); + assertEquals(body.suspended, true); + assertEquals(seen, []); + assertEquals((await store.getRun(runId))?.record.status, "suspended"); + }); +}); + +Deno.test("a lost resume race is a structured already_resumed, not a crash", async () => { + await withStore(async (store) => { + const { build, root } = makePipeline(); + const server = new McpServer(build, { allowRun: true, stateStore: store }); + const runId = await suspend(build, root, store); + + // A rival process holds the run's lease, so this resumer must lose. + const lease = await acquireLease( + store, + RUN_LEASE_PREFIX, + runId, + "rival", + () => new Date().toISOString(), + ); + if (lease === null) throw new Error("fixture could not take the lease"); + try { + const result = await call(server, "signal_run", { + runId, + signal: "approved", + }); + assertEquals(result.isError, true); + const body = JSON.parse(result.text); + assertEquals(body.error, "already_resumed"); + assertEquals(body.runId, runId); + // Who has it, and since when, are part of the structured answer. + assertEquals(typeof body.by, "string"); + assertEquals(typeof body.at, "string"); + } finally { + await lease.release(); + } + }); +}); + +Deno.test("a lock conflict in the resumed run surfaces as structured lock_conflict", async () => { + await withStore(async (store) => { + const seenHolder = { + actor: "rival", + runId: "run-elsewhere", + since: "2026-01-01T00:00:00.000Z", + }; + class Locked extends Build { + gate = target().waitsFor((s) => s.on(externalSignal("approved"))); + promote = target().dependsOn(this.gate).executes(() => { + throw new LockConflictError( + seenHolder, + "deploy lock is held by rival — cancel run-elsewhere to release it", + ); + }); + } + const build = new Locked(); + discoverTargets(build); + const server = new McpServer(build, { allowRun: true, stateStore: store }); + const runId = await suspend(build, build.promote, store); + + const result = await call(server, "signal_run", { + runId, + signal: "approved", + }); + assertEquals(result.isError, true); + const body = JSON.parse(result.text); + assertEquals(body.error, "lock_conflict"); + assertEquals(body.holder, seenHolder); + assertStringIncludes(body.guidance, "cancel run-elsewhere"); + }); +}); + +Deno.test("a failing resumed target is a structured run_failed with the message", async () => { + await withStore(async (store) => { + class Failing extends Build { + gate = target().waitsFor((s) => s.on(externalSignal("approved"))); + promote = target().dependsOn(this.gate).executes(() => { + throw new Error("promotion exploded"); + }); + } + const build = new Failing(); + discoverTargets(build); + const server = new McpServer(build, { allowRun: true, stateStore: store }); + const runId = await suspend(build, build.promote, store); + + const result = await call(server, "signal_run", { + runId, + signal: "approved", + }); + assertEquals(result.isError, true); + const body = JSON.parse(result.text); + assertEquals(body.error, "run_failed"); + assertEquals(body.runId, runId); + assertStringIncludes(body.message, "promotion exploded"); + }); +}); + +Deno.test("resume_check re-checks one run, and a sweep covers every authorized run", async () => { + await withStore(async (store) => { + const { build, root } = makePipeline(); + const server = new McpServer(build, { + allowRun: true, + stateStore: store, + actor: "op", + }); + const runId = await suspend(build, root, store); + + // A single-run check: the signal gate is unsatisfied, so it re-suspends — + // checked but not failed. + const single = await call(server, "resume_check", { runId }); + assertEquals(single.isError, false); + assertEquals(JSON.parse(single.text), { ok: true, checked: 1, failed: 0 }); + assertEquals((await store.getRun(runId))?.record.status, "suspended"); + + // A sweep (no runId) finds and checks the same suspended run. + const sweep = await call(server, "resume_check"); + assertEquals(sweep.isError, false); + assertEquals(JSON.parse(sweep.text), { ok: true, checked: 1, failed: 0 }); + + // A single-run check of a missing run is a structured no_run. + const missing = await call(server, "resume_check", { runId: "nope" }); + assertEquals(missing.isError, true); + assertEquals(JSON.parse(missing.text).error, "no_run"); + }); +}); + +Deno.test("a resume_check failure on one run is reported as structured run_failed", async () => { + await withStore(async (store) => { + const { build, root } = makePipeline(); + const runId = await suspend(build, root, store); + // A dependency failure inside the check itself (here: the environment + // backend erroring mid-sweep) must come back as a structured per-run + // failure naming the run, not reject the whole tool call. + const deps: RunToolDeps = { + store, + build, + actor: "op", + readEnv: () => { + throw new Error("env backend offline"); + }, + authorize: () => null, + }; + const result = await callRunStateTool(deps, "resume_check", { runId }); + assertEquals(result?.isError, true); + const body = JSON.parse(result?.text ?? "{}"); + assertEquals(body.error, "run_failed"); + assertEquals(body.runId, runId); + assertStringIncludes(body.message, "env backend offline"); + }); +}); + +/** Delegates to a real store, but `getRun` fails after the first read. */ +class FlakyStore implements StateStore { + #reads = 0; + constructor(private readonly inner: StateStore) {} + getRun(id: string): ReturnType { + this.#reads += 1; + if (this.#reads > 1) { + return Promise.reject(new Error("state backend offline")); + } + return this.inner.getRun(id); + } + putRun( + record: RunRecord, + expectedVersion: string | null, + ): ReturnType { + return this.inner.putRun(record, expectedVersion); + } + listRuns( + query: Parameters[0], + ): ReturnType { + return this.inner.listRuns(query); + } + deleteRun(id: string): Promise { + return this.inner.deleteRun(id); + } + acquireLock( + key: string, + holder: Parameters[1], + ttlMs: number, + ): ReturnType { + return this.inner.acquireLock(key, holder, ttlMs); + } + renewLock(key: string, token: string, ttlMs: number): Promise { + return this.inner.renewLock(key, token, ttlMs); + } + releaseLock(key: string, token: string): Promise { + return this.inner.releaseLock(key, token); + } +} + +Deno.test("a store fault mid-cancel is a structured run_failed, not a crash", async () => { + await withStore(async (inner) => { + const { build } = makePipeline(); + await seedRun(inner, "run-x", "promote", "suspended", "2026-01-01T00:00:00.000Z"); + // The tool's own pre-check read succeeds; the store then dies underneath + // cancelRun. The caller still gets structured JSON naming the run. + const flaky = new FlakyStore(inner); + const server = new McpServer(build, { allowRun: true, stateStore: flaky }); + const result = await call(server, "cancel_run", { runId: "run-x" }); + assertEquals(result.isError, true); + const body = JSON.parse(result.text); + assertEquals(body.error, "run_failed"); + assertEquals(body.runId, "run-x"); + assertStringIncludes(body.message, "state backend offline"); + }); +}); diff --git a/packages/core/tests/state_test.ts b/packages/core/tests/state_test.ts index d7d07a6..3dc2879 100644 --- a/packages/core/tests/state_test.ts +++ b/packages/core/tests/state_test.ts @@ -12,6 +12,7 @@ import { parseRunSummary, type RunRecord, stringifyRunRecord, + toJsonValue, toSummary, } from "../src/state/types.ts"; import { @@ -25,6 +26,7 @@ import { envStateStore, resolveStateStore } from "../src/state/resolve.ts"; import { HttpError } from "../src/http.ts"; import { buildRunRecord, + ciRunUrl, recordStatusOf, resolveActor, } from "../src/state/record.ts"; @@ -1462,3 +1464,283 @@ Deno.test("deleting a run leaves its lock records alone", async () => { await Deno.remove(dir, { recursive: true }); } }); + +// ------------------------------------------------- types: malformed branches + +Deno.test("parseRunRecord rejects malformed optional and nested fields", () => { + const cases: Array<[string, string]> = [ + // An optional string field present with the wrong type. + [ + JSON.stringify({ ...sampleRecord(), buildId: 5 }), + 'field "buildId" is not a string', + ], + [ + JSON.stringify({ + ...sampleRecord(), + targets: { a: { status: "pending", meta: {}, effects: "x" } }, + }), + "effects is not an object", + ], + [ + JSON.stringify({ + ...sampleRecord(), + targets: { a: { status: "pending", meta: {}, effects: { e: 5 } } }, + }), + "effect state is not an object", + ], + [ + JSON.stringify({ + ...sampleRecord(), + targets: { + a: { + status: "pending", + meta: {}, + effects: { e: { status: "weird", intentAt: "t", attempts: 1 } }, + }, + }, + }), + 'unknown effect status "weird"', + ], + [ + JSON.stringify({ + ...sampleRecord(), + targets: { + a: { + status: "pending", + meta: {}, + effects: { e: { status: "pending", intentAt: "t", attempts: 0 } }, + }, + }, + }), + "effect attempts is not a positive integer", + ], + [ + JSON.stringify({ + ...sampleRecord(), + targets: { a: { status: "waiting", meta: {}, waitingFor: "x" } }, + }), + "waitingFor is not an object", + ], + [ + JSON.stringify({ ...sampleRecord(), events: "x" }), + '"events" is not an array', + ], + [ + JSON.stringify({ ...sampleRecord(), events: [5] }), + "run event is not an object", + ], + [ + JSON.stringify({ + ...sampleRecord(), + events: [{ at: "t", tool: "x", actor: "a", outcome: "weird" }], + }), + 'unknown run event outcome "weird"', + ], + [ + JSON.stringify({ ...sampleRecord(), signals: { s: 5 } }), + "signal record is not an object", + ], + [ + JSON.stringify({ ...sampleRecord(), intendedTerminal: "weird" }), + 'unknown intended terminal status "weird"', + ], + ]; + for (const [text, needle] of cases) { + assertThrows(() => parseRunRecord(text), Error, needle); + } +}); + +Deno.test("parseRunRecord defaults an absent target meta and signal data", () => { + const parsed = parseRunRecord(JSON.stringify({ + ...sampleRecord(), + // A target state written without a `meta` key still parses (empty meta), + // and a signal without `data` reads back as a null payload. + targets: { a: { status: "pending" } }, + signals: { approved: { receivedAt: "2026-07-17T10:00:00.000Z" } }, + })); + assertEquals(parsed.targets.a, { status: "pending", meta: {} }); + assertEquals(parsed.signals.approved, { + data: null, + receivedAt: "2026-07-17T10:00:00.000Z", + }); +}); + +Deno.test("parseRunRecord round-trips effects, events, and terminal intent", () => { + const record = sampleRecord({ + status: "cancelling", + buildId: "org/app", + deadlineAt: "2026-07-18T10:00:00.000Z", + intendedTerminal: "failed", + targets: { + deploy: { + status: "failed", + meta: {}, + error: "boom", + effects: { + notify: { + status: "failed", + intentAt: "2026-07-17T10:00:01.000Z", + settledAt: "2026-07-17T10:00:02.000Z", + error: "smtp down", + attempts: 2, + }, + record: { + status: "pending", + intentAt: "2026-07-17T10:00:03.000Z", + attempts: 1, + }, + }, + }, + }, + events: [{ + at: "2026-07-17T10:00:04.000Z", + tool: "signal_run", + actor: "mcp:client", + outcome: "denied", + args: { name: "approved" }, + detail: "actor not allowed", + }], + }); + assertEquals(parseRunRecord(stringifyRunRecord(record)), record); +}); + +Deno.test("toJsonValue passes JSON through and rejects a non-JSON value", () => { + assertEquals(toJsonValue({ a: [1, "x", true, null], b: { c: 2 } }), { + a: [1, "x", true, null], + b: { c: 2 }, + }); + // A value JSON cannot represent must be named, not silently dropped. + assertThrows( + () => toJsonValue(undefined), + Error, + 'value of type "undefined" is not JSON', + ); + assertThrows( + () => toJsonValue(() => 1), + Error, + 'value of type "function" is not JSON', + ); +}); + +// ------------------------------------------------- record: mapping branches + +Deno.test("recordStatusOf maps a waiting target onto the record vocabulary", () => { + assertEquals(recordStatusOf("waiting"), "waiting"); +}); + +Deno.test("ciRunUrl derives the GitHub Actions run URL only when fully set", () => { + const full: Record = { + GITHUB_SERVER_URL: "https://github.com", + GITHUB_REPOSITORY: "o/r", + GITHUB_RUN_ID: "123", + }; + assertEquals( + ciRunUrl((n) => full[n]), + "https://github.com/o/r/actions/runs/123", + ); + // Any missing piece means no URL — a partial one would link nowhere. + for (const missing of Object.keys(full)) { + const partial = { ...full }; + delete partial[missing]; + assertEquals(ciRunUrl((n) => partial[n]), undefined); + } +}); + +Deno.test("buildRunRecord tolerates an unnamed target and stamps optional fields", () => { + // A builder that never went through discoverTargets has no name; the record + // still forms (empty name) instead of crashing mid-run. + const anonymous = target().executes(() => {}); + const record = buildRunRecord({ + runId: "run-y", + build: "B", + buildId: "org/app", + rootTarget: "work", + actor: "alice", + now: "2026-07-17T10:00:00.000Z", + order: [anonymous], + params: [], + deadlineAt: "2026-07-18T10:00:00.000Z", + }); + assertEquals(record.graph, [{ name: "", dependsOn: [] }]); + assertEquals(record.targets, { "": { status: "pending", meta: {} } }); + assertEquals(record.buildId, "org/app"); + assertEquals(record.deadlineAt, "2026-07-18T10:00:00.000Z"); +}); + +// ------------------------------------------------- host: real error paths + +Deno.test("defaultStateHost surfaces non-NotFound filesystem errors", async () => { + // A genuine I/O failure must propagate, never be masked as "missing". + const dir = await Deno.makeTempDir(); + try { + const host = defaultStateHost; + // Reading a directory as a file is not a miss. + await assertRejects(() => host.readText(dir)); + // An exclusive create in a missing parent is not "already exists". + await assertRejects(() => host.createExclusive(`${dir}/missing/x.lock`)); + // Removing a non-empty directory is a real error, not a missing file. + await Deno.mkdir(`${dir}/full`); + await Deno.writeTextFile(`${dir}/full/a.txt`, "x"); + await assertRejects(() => host.remove(`${dir}/full`)); + // Listing a regular file is not an absent directory. + await assertRejects(() => host.listDir(`${dir}/full/a.txt`)); + } finally { + await Deno.remove(dir, { recursive: true }); + } +}); + +// ------------------------------------------------- http store: bodied errors + +Deno.test("HttpStateStore drains response bodies a server sends on every path", async () => { + // Real servers attach error pages (and empty-object bodies) to statuses the + // client discards; each path must drain them and keep its behaviour. + const bodied = (status: number, headers?: Record) => + new Response(`{"note":"body for ${status}"}`, { status, headers }); + + const missing = new HttpStateStore({ + url: "https://s", + fetch: fakeFetch(() => bodied(404)), + }); + assertEquals(await missing.getRun("r"), null); // bodied 404 is still a miss + await missing.deleteRun("r"); // bodied 404 on delete is still a no-op + + const putOk = new HttpStateStore({ + url: "https://s", + fetch: fakeFetch((url, init) => + (init?.method ?? "GET") === "PUT" + ? bodied(200, { etag: "v9" }) + : bodied(200) + ), + }); + assertEquals(await putOk.putRun(sampleRecord(), "v1"), { + ok: true, + version: "v9", + }); + assertEquals(await putOk.renewLock("k", "t", 1000), true); + await putOk.releaseLock("k", "t"); + await putOk.deleteRun("r"); + + const stale = new HttpStateStore({ + url: "https://s", + fetch: fakeFetch(() => bodied(412)), + }); + assertEquals(await stale.putRun(sampleRecord(), "old"), { + ok: false, + conflict: true, + }); + + const boom = new HttpStateStore({ + url: "https://s", + fetch: fakeFetch(() => bodied(500)), + }); + await assertRejects(() => boom.getRun("r"), HttpError); + await assertRejects(() => boom.putRun(sampleRecord(), "v"), HttpError); + await assertRejects(() => boom.listRuns({}), HttpError); + await assertRejects(() => boom.deleteRun("r"), HttpError); + await assertRejects( + () => boom.acquireLock("k", { actor: "a", runId: "r", since: "t" }, 1000), + HttpError, + ); + await assertRejects(() => boom.renewLock("k", "t", 1000), HttpError); + await assertRejects(() => boom.releaseLock("k", "t"), HttpError); +}); From 95165a5b22315fd1585b7a7315abc80c26c41a50 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 12:14:13 +0000 Subject: [PATCH 06/11] test: expand coverage across ai, core executor, MCP, and state suites Coverage-push checkpoint: meaningful branch tests for the AI fixer, dedup, hosts, comment/gate/report modules, the executor plan and cancellation paths, the scheduler, resume, reap, and the MCP servers. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019As9A3ugixLQiAvcat1ZKk --- packages/ai/tests/agent_fixer_test.ts | 332 +++++++++++++++ packages/ai/tests/comment_test.ts | 94 +++++ packages/ai/tests/cost_controls_test.ts | 47 +++ packages/ai/tests/dedup_test.ts | 67 +++ packages/ai/tests/deep_review_test.ts | 14 + packages/ai/tests/discussion_flow_test.ts | 393 ++++++++++++++++++ packages/ai/tests/fixer_test.ts | 196 +++++++++ packages/ai/tests/gate_test.ts | 62 +++ packages/ai/tests/hosts_test.ts | 229 ++++++++++ packages/ai/tests/report_test.ts | 214 ++++++++++ packages/core/tests/cancel_test.ts | 359 ++++++++++++++++ packages/core/tests/ci_schedule_test.ts | 21 + packages/core/tests/ci_test.ts | 122 ++++++ packages/core/tests/cli_test.ts | 131 +++++- packages/core/tests/describe_test.ts | 7 + packages/core/tests/execute_cancel_test.ts | 107 +++++ packages/core/tests/execute_plan_test.ts | 147 +++++++ packages/core/tests/mcp_authz_closure_test.ts | 53 +++ packages/core/tests/mcp_hardening_test.ts | 211 +++++++++- packages/core/tests/mcp_http_test.ts | 30 ++ packages/core/tests/mcp_test.ts | 15 + packages/core/tests/reap_test.ts | 151 +++++++ packages/core/tests/registry_server_test.ts | 240 +++++++++++ packages/core/tests/resume_test.ts | 263 ++++++++++++ packages/core/tests/scheduler_test.ts | 385 +++++++++++++++++ packages/core/tests/state_test.ts | 2 +- 26 files changed, 3887 insertions(+), 5 deletions(-) create mode 100644 packages/ai/tests/comment_test.ts create mode 100644 packages/ai/tests/gate_test.ts create mode 100644 packages/ai/tests/report_test.ts create mode 100644 packages/core/tests/execute_cancel_test.ts create mode 100644 packages/core/tests/execute_plan_test.ts create mode 100644 packages/core/tests/scheduler_test.ts diff --git a/packages/ai/tests/agent_fixer_test.ts b/packages/ai/tests/agent_fixer_test.ts index b722f41..33f73cf 100644 --- a/packages/ai/tests/agent_fixer_test.ts +++ b/packages/ai/tests/agent_fixer_test.ts @@ -441,3 +441,335 @@ Deno.test("commitFixes fails closed when the pre-snapshot cannot be taken", asyn false, ); }); + +/** A GitHub-PR env without any GITHUB_TOKEN. */ +const TOKENLESS_PR_ENV: Record = { + GITHUB_ACTIONS: "true", + GITHUB_REPOSITORY: "o/r", + GITHUB_REF: "refs/pull/7/merge", +}; + +/** The one-hunk diff the fake agent leaves in the working tree. */ +const AGENT_DIFF = [ + "diff --git a/zuke.ts b/zuke.ts", + "--- a/zuke.ts", + "+++ b/zuke.ts", + "@@ -45,1 +45,1 @@", + '-const X = "remove me";', + '+const _X = "remove me";', +].join("\n"); + +/** A fetch faking the GitHub PR-detail and comment endpoints, recording calls. */ +function githubFetch(): { + fetch: typeof fetch; + calls: { url: string; auth: string; body: string }[]; +} { + const calls: { url: string; auth: string; body: string }[] = []; + const impl = ((input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + const method = init?.method ?? "GET"; + calls.push({ + url, + auth: new Headers(init?.headers).get("authorization") ?? "", + body: typeof init?.body === "string" ? init.body : "", + }); + if (url.includes("/comments")) { + return Promise.resolve( + new Response(method === "GET" ? "[]" : "{}", { status: 200 }), + ); + } + return Promise.resolve( + new Response(JSON.stringify({ head: { sha: "abc123" } }), { + status: 200, + }), + ); + }) as typeof fetch; + return { fetch: impl, calls }; +} + +Deno.test("suggest mode authenticates with an explicit commentToken", async () => { + // GITHUB_TOKEN is unset, so the inline suggestions can only be posted through + // .commentToken(...) — success pins the explicit-token resolution path. + const { fetch, calls } = githubFetch(); + let statusCalls = 0; + const fixer = agentFixer(() => Promise.resolve(), (f) => f.suggest()) + .allowCI().comment().commentToken("agent-tok") + .conventions("").readFile(() => Promise.resolve(undefined)) + .env((n) => TOKENLESS_PR_ENV[n]) + .exec((argv) => { + if (argv.includes("status")) { + statusCalls++; + return Promise.resolve(statusCalls === 1 ? "" : " M zuke.ts"); + } + return Promise.resolve(argv.includes("diff") ? AGENT_DIFF : ""); + }) + .fetch(fetch).quiet(); + await fixer.remediate(CTX); + const post = calls.find((c) => + c.url.endsWith("/pulls/7/comments") && c.body.includes("```suggestion") + ); + assertEquals(post?.auth, "Bearer agent-tok"); +}); + +Deno.test("suggest mode without any token proposes nothing and calls no API", async () => { + const { fetch, calls } = githubFetch(); + const lines: string[] = []; + const log = console.log; + console.log = (...a: unknown[]) => void lines.push(a.join(" ")); + let statusCalls = 0; + try { + const fixer = agentFixer(() => Promise.resolve(), (f) => f.suggest()) + .allowCI().conventions("").readFile(() => Promise.resolve(undefined)) + .env((n) => TOKENLESS_PR_ENV[n]) + .exec((argv) => { + if (argv.includes("status")) { + statusCalls++; + return Promise.resolve(statusCalls === 1 ? "" : " M zuke.ts"); + } + return Promise.resolve(argv.includes("diff") ? AGENT_DIFF : ""); + }) + .fetch(fetch); + await fixer.remediate(CTX); + } finally { + console.log = log; + } + // Suggestions were produced but no PR context could be resolved — nothing + // was posted anywhere (the overview comment has no token either). + assertEquals(calls.length, 0); + assertEquals( + lines.some((l) => l.includes("no committable suggestions produced")), + true, + ); +}); + +Deno.test("suggest mode fails closed when the post-agent status cannot be read", async () => { + const prEnv: Record = { + ...TOKENLESS_PR_ENV, + GITHUB_TOKEN: "tok", + }; + const lines: string[] = []; + const log = console.log; + console.log = (...a: unknown[]) => void lines.push(a.join(" ")); + const git: string[][] = []; + let statusCalls = 0; + try { + const fixer = agentFixer(() => Promise.resolve(), (f) => f.suggest()) + .allowCI().noComment().conventions("") + .readFile(() => Promise.resolve(undefined)) + .env((n) => prEnv[n]) + .exec((argv) => { + git.push(argv); + if (argv.includes("status")) { + statusCalls++; + // The snapshot succeeds; the post-agent status read fails. + if (statusCalls === 1) return Promise.resolve(""); + return Promise.reject(new Error("index.lock exists")); + } + return Promise.resolve(""); + }); + const result = await fixer.remediate(CTX); + assertEquals(result.retry, false); + } finally { + console.log = log; + } + // Without the second status nothing can be scoped: no diff is ever taken. + assertEquals(git.some((a) => a.includes("diff")), false); + assertEquals( + lines.some((l) => l.includes("no committable suggestions produced")), + true, + ); +}); + +Deno.test("suggest mode with an idle agent takes no diff and proposes nothing", async () => { + const prEnv: Record = { + ...TOKENLESS_PR_ENV, + GITHUB_TOKEN: "tok", + }; + const lines: string[] = []; + const log = console.log; + console.log = (...a: unknown[]) => void lines.push(a.join(" ")); + const git: string[][] = []; + try { + const fixer = agentFixer(() => Promise.resolve(), (f) => f.suggest()) + .allowCI().noComment().conventions("") + .readFile(() => Promise.resolve(undefined)) + .env((n) => prEnv[n]) + .exec((argv) => { + git.push(argv); + return Promise.resolve(""); // clean tree before and after the agent + }); + await fixer.remediate(CTX); + } finally { + console.log = log; + } + assertEquals(git.some((a) => a.includes("diff")), false); + assertEquals( + lines.some((l) => l.includes("no committable suggestions produced")), + true, + ); +}); + +Deno.test("a non-Error suggestion-post failure is stringified and swallowed", async () => { + const prEnv: Record = { + ...TOKENLESS_PR_ENV, + GITHUB_TOKEN: "tok", + }; + // The PR-detail fetch rejects with a bare string (not an Error). + const throwing = (() => Promise.reject("conn reset")) as typeof fetch; + const warnings: string[] = []; + const warn = console.warn; + console.warn = (...a: unknown[]) => void warnings.push(a.join(" ")); + let statusCalls = 0; + try { + const fixer = agentFixer(() => Promise.resolve(), (f) => f.suggest()) + .allowCI().noComment().conventions("") + .readFile(() => Promise.resolve(undefined)) + .env((n) => prEnv[n]) + .exec((argv) => { + if (argv.includes("status")) { + statusCalls++; + return Promise.resolve(statusCalls === 1 ? "" : " M zuke.ts"); + } + return Promise.resolve(argv.includes("diff") ? AGENT_DIFF : ""); + }) + .fetch(throwing).quiet(); + const result = await fixer.remediate(CTX); + assertEquals(result.retry, false); + } finally { + console.warn = warn; + } + assertEquals( + warnings.some((w) => w.includes("could not post suggestions: conn reset")), + true, + ); +}); + +Deno.test("a non-Error agent failure is stringified into the warning", async () => { + const warnings: string[] = []; + const warn = console.warn; + console.warn = (...a: unknown[]) => void warnings.push(a.join(" ")); + let result; + try { + // The runner rejects with a bare string, pinning the String(error) path. + const fixer = hermetic(agentFixer(() => Promise.reject("agent exploded"))); + result = await fixer.remediate(CTX); + } finally { + console.warn = warn; + } + assertEquals(result.retry, false); + assertEquals( + warnings.some((w) => w.includes("agent run failed: agent exploded")), + true, + ); +}); + +Deno.test("suggest mode skips suggestions when the pre-agent snapshot fails", async () => { + const lines: string[] = []; + const log = console.log; + console.log = (...a: unknown[]) => void lines.push(a.join(" ")); + const r = recorder(); + let result; + try { + const fixer = agentFixer(r.run, (f) => f.suggest()) + .conventions("").readFile(() => Promise.resolve(undefined)) + .env(() => undefined) // local run — the CI gate does not apply + .exec(() => Promise.reject(new Error("index.lock exists"))); + result = await fixer.remediate(CTX); + } finally { + console.log = log; + } + assertEquals(result.retry, false); + assertEquals(r.calls.length, 1); // the agent still ran + assertEquals( + lines.some((l) => + l.includes( + "skipped suggestions: could not snapshot the working tree", + ) + ), + true, + ); +}); + +Deno.test("a non-Error commit failure is stringified into the report action", async () => { + const lines: string[] = []; + const log = console.log; + console.log = (...a: unknown[]) => void lines.push(a.join(" ")); + let statusCalls = 0; + let result; + try { + const fixer = agentFixer(() => Promise.resolve(), (f) => f.commitFixes()) + .conventions("").readFile(() => Promise.resolve(undefined)) + .env(() => undefined) + .exec((argv) => { + if (argv.includes("status")) { + statusCalls++; + return Promise.resolve(statusCalls === 1 ? "" : " M zuke.ts"); + } + if (argv[1] === "commit") return Promise.reject("hook denied"); + return Promise.resolve(""); + }); + result = await fixer.remediate(CTX); + } finally { + console.log = log; + } + assertEquals(result.retry, true); // the agent's edit still re-runs the target + assertEquals( + lines.some((l) => l.includes("commit failed: hook denied")), + true, + ); +}); + +Deno.test("a long agent transcript is truncated in the PR comment", async () => { + const prEnv: Record = { + ...TOKENLESS_PR_ENV, + GITHUB_TOKEN: "tok", + }; + const { fetch, calls } = githubFetch(); + const fixer = agentFixer(() => Promise.resolve("x".repeat(5000))) + .allowCI().conventions("").readFile(() => Promise.resolve(undefined)) + .env((n) => prEnv[n]).fetch(fetch).quiet(); + await fixer.remediate(CTX); + const post = calls.find((c) => c.body.includes("Agent output")); + if (post === undefined) throw new Error("no comment posted"); + assertEquals(post.body.includes("… (truncated) …"), true); + // The capped transcript, not the full 5000 characters, travels. + assertEquals(post.body.includes("x".repeat(4001)), false); +}); + +Deno.test("an Error from the suggestion post is warned with its message", async () => { + const prEnv: Record = { + ...TOKENLESS_PR_ENV, + GITHUB_TOKEN: "tok", + }; + // The PR-detail fetch rejects with a real Error, whose message is reported. + const throwing = + (() => Promise.reject(new Error("api quota exhausted"))) as typeof fetch; + const warnings: string[] = []; + const warn = console.warn; + console.warn = (...a: unknown[]) => void warnings.push(a.join(" ")); + let statusCalls = 0; + try { + const fixer = agentFixer(() => Promise.resolve(), (f) => f.suggest()) + .allowCI().noComment().conventions("") + .readFile(() => Promise.resolve(undefined)) + .env((n) => prEnv[n]) + .exec((argv) => { + if (argv.includes("status")) { + statusCalls++; + return Promise.resolve(statusCalls === 1 ? "" : " M zuke.ts"); + } + return Promise.resolve(argv.includes("diff") ? AGENT_DIFF : ""); + }) + .fetch(throwing).quiet(); + const result = await fixer.remediate(CTX); + assertEquals(result.retry, false); + } finally { + console.warn = warn; + } + assertEquals( + warnings.some((w) => + w.includes("could not post suggestions: api quota exhausted") + ), + true, + ); +}); diff --git a/packages/ai/tests/comment_test.ts b/packages/ai/tests/comment_test.ts new file mode 100644 index 0000000..7ab3c21 --- /dev/null +++ b/packages/ai/tests/comment_test.ts @@ -0,0 +1,94 @@ +// Copyright (c) 2026 the Zuke contributors +// SPDX-License-Identifier: MIT + +/** + * Tests for the shared PR-comment poster: token resolution, the no-context + * no-op, and the best-effort error handling. + * + * @module + */ + +import { + assertEquals, + assertStringIncludes, +} from "../../core/tests/_assert.ts"; +import { postComment } from "../src/comment.ts"; + +/** A recorded request. */ +interface Call { + url: string; + auth: string; +} + +/** A fake `fetch` recording each call's URL and Authorization header. */ +function recordFetch(): { fetch: typeof fetch; calls: Call[] } { + const calls: Call[] = []; + const impl = ((input: string | URL | Request, init?: RequestInit) => { + const headers = new Headers(init?.headers); + calls.push({ + url: String(input), + auth: headers.get("authorization") ?? "", + }); + const method = init?.method ?? "GET"; + return Promise.resolve( + new Response(method === "GET" ? "[]" : "{}", { status: 200 }), + ); + }) as typeof fetch; + return { fetch: impl, calls }; +} + +const PR_ENV: Record = { + GITHUB_ACTIONS: "true", + GITHUB_REPOSITORY: "o/r", + GITHUB_REF: "refs/pull/7/merge", +}; + +Deno.test("postComment is a no-op off any CI host", async () => { + const { fetch, calls } = recordFetch(); + await postComment("sec", "## body", { env: () => undefined, fetch }); + assertEquals(calls.length, 0); +}); + +Deno.test("an explicit commentToken wins over the host env token", async () => { + const { fetch, calls } = recordFetch(); + await postComment("sec", "## body", { + commentToken: "explicit-token", + env: (n) => n === "GITHUB_TOKEN" ? "env-token" : PR_ENV[n], + fetch, + }); + assertEquals(calls.length, 2); // list, then create + for (const call of calls) { + assertEquals(call.auth, "Bearer explicit-token"); + } +}); + +Deno.test("no token at all prepares no upsert — nothing is fetched", async () => { + const { fetch, calls } = recordFetch(); + // The host is detected, but GITHUB_TOKEN is unset, so the empty token yields + // no PR context and the comment is silently skipped. + await postComment("sec", "## body", { env: (n) => PR_ENV[n], fetch }); + assertEquals(calls.length, 0); +}); + +Deno.test("a non-Error throw from the upsert is stringified into the warning", async () => { + // Rejects with a bare string (not an Error), pinning the String(error) + // fallback in postComment's catch. + const throwing = (() => Promise.reject("socket torn down")) as typeof fetch; + const warnings: string[] = []; + const warn = console.warn; + console.warn = (...a: unknown[]) => void warnings.push(a.join(" ")); + try { + await postComment("sec", "## body", { + commentToken: "tok", + env: (n) => PR_ENV[n], + fetch: throwing, + }); + } finally { + console.warn = warn; + } + assertEquals(warnings.length, 1); + assertStringIncludes( + warnings[0], + "[sec] could not post PR comment: socket torn down", + ); +}); diff --git a/packages/ai/tests/cost_controls_test.ts b/packages/ai/tests/cost_controls_test.ts index d2aded1..a71574e 100644 --- a/packages/ai/tests/cost_controls_test.ts +++ b/packages/ai/tests/cost_controls_test.ts @@ -157,6 +157,53 @@ Deno.test("reviewer skips the call once the budget is exhausted", async () => { assertEquals(lines.some((l) => l.includes("AI budget exhausted")), true); }); +Deno.test("the verify pass's own usage draws down the shared budget", async () => { + const b = budget((x) => x.maxTokens(100_000)); + const finding = { + title: "weak hash", + severity: "high" as const, + file: "h.ts", + }; + const id = findingFingerprint("security", { + title: "weak hash", + severity: "high", + file: "h.ts", + }); + const responses = [ + claude({ score: 8, severity: "high", summary: "s", findings: [finding] }, { + input_tokens: 10, + output_tokens: 5, + }), + // The verify verdict also reports usage — it must be folded in too. + JSON.stringify({ + content: [{ + type: "text", + text: JSON.stringify({ + verdicts: [{ id, verdict: "refuted", reason: "not reachable" }], + }), + }], + stop_reason: "end_turn", + usage: { input_tokens: 7, output_tokens: 3 }, + }), + ]; + let served = 0; + const queued: typeof fetch = () => + Promise.resolve( + new Response(responses[Math.min(served++, responses.length - 1)], { + status: 200, + }), + ); + await captured(() => + // Passes: the sole finding was refuted, so nothing gates. + securityReviewer((r) => + r.provider("claude").apiKey("k").diff((d) => d.text(DIFF)) + .fetch(queued).budget(b).verify() + ).validate({ target: "t" }) + ); + assertEquals(b.spend_().calls, 2); // review + verify both recorded + assertEquals(b.spend_().totalTokens, 25); +}); + // ----- Cache --------------------------------------------------------------- Deno.test("reviewer caches a response and reuses it on a repeat run", async () => { diff --git a/packages/ai/tests/dedup_test.ts b/packages/ai/tests/dedup_test.ts index 31ba475..9a62ee3 100644 --- a/packages/ai/tests/dedup_test.ts +++ b/packages/ai/tests/dedup_test.ts @@ -302,3 +302,70 @@ Deno.test("eligible is the one gate both resolution paths share", () => { false, ); }); + +Deno.test("dedupNotes carries an empty file for a hand-built file-less pair", () => { + // planDedup never offers a file-less pair, but the serialiser is its own + // contract: a missing file travels as "", never as the string "undefined". + const notes = dedupNotes({ + pairs: [{ + label: "p1", + candidate: candidate("c1", { file: undefined, detail: "why" }), + prior: prior("p1"), + }], + dropped: 0, + }); + assertEquals(notes, [{ + label: "p1", + file: "", + title: "finding c1", + detail: "why", + priorTitle: "prior p1", + }]); +}); + +Deno.test("planDedup round-robins past a candidate with fewer matches", () => { + // c1 has two eligible priors, c2 (in another file) has one. At depth two the + // round-robin must skip c2's exhausted list and still offer c1's second pair. + const plan = planDedup( + [candidate("c1"), candidate("c2", { file: "src/other.ts" })], + [prior("p1"), prior("p2"), prior("p3", { file: "src/other.ts" })], + ); + assertEquals( + plan.pairs.map((p) => [p.candidate.id, p.prior.id]), + [["c1", "p1"], ["c2", "p3"], ["c1", "p2"]], + ); + assertEquals(plan.dropped, 0); +}); + +Deno.test("sameAs ignores a hand-built pair whose candidate has no identity", () => { + // A pair can only enter a plan through planDedup today, but sameAs guards its + // own input: without a candidate id there is nothing to rename, so a "same" + // verdict on such a pair must resolve nothing. + for (const id of [undefined, ""]) { + const plan = { + pairs: [{ + label: "p1", + candidate: candidate("c1", { id }), + prior: prior("p1"), + }], + dropped: 0, + }; + const result = sameAs(plan, verdicts(["p1", "same"])); + assertEquals(result.matches.size, 0); + assertEquals(result.ambiguous, []); + } +}); + +Deno.test("adoptCanonicalIds skips a finding that has no id at all", () => { + const bare = candidate("c1", { id: undefined }); + const matched = candidate("c2"); + const target = prior("p1"); + const adoptions = adoptCanonicalIds( + [bare, matched], + new Map([["c2", target]]), + ); + // Only the identified finding adopts; the bare one is left untouched. + assertEquals(adoptions, [{ alias: "c2", prior: target }]); + assertEquals(matched.id, "p1"); + assertEquals(bare.id, undefined); +}); diff --git a/packages/ai/tests/deep_review_test.ts b/packages/ai/tests/deep_review_test.ts index c548c9b..48b1887 100644 --- a/packages/ai/tests/deep_review_test.ts +++ b/packages/ai/tests/deep_review_test.ts @@ -393,3 +393,17 @@ Deno.test("a failed verify pass warns on the console when not quiet", async () = true, ); }); + +Deno.test("an empty file context is omitted from the prompt entirely", async () => { + const { fetch, calls } = queuedFetch([claude({ score: 0, findings: [] })]); + const git = fakeGit({}); // every `git show` fails — nothing to send + await genericReviewer((r) => + r.provider("claude").apiKey("k").quiet() + .diff((d) => d.base("origin/master")) + .fileContext(1000) + .exec(git.run).fetch(fetch) + ).validate({ target: "t" }); + const body = JSON.parse(calls[0].body); + // No empty UNTRUSTED_FILES block confuses the model when nothing was read. + assertEquals(body.messages[0].content.includes("UNTRUSTED_FILES"), false); +}); diff --git a/packages/ai/tests/discussion_flow_test.ts b/packages/ai/tests/discussion_flow_test.ts index 1bba623..3e7a0c8 100644 --- a/packages/ai/tests/discussion_flow_test.ts +++ b/packages/ai/tests/discussion_flow_test.ts @@ -2854,3 +2854,396 @@ Deno.test("a thread phase that throws degrades to a note, never a failure", asyn true, ); }); + +Deno.test("discussion on GitHub without a PR context is skipped silently", async () => { + // A push build on GitHub Actions: the host can list comments, but there is + // no pull request to list them from — the discussion just does not run. + const { fetch, calls } = discussionFetch([], [ + claude({ score: 0, severity: "none", findings: [] }), + ]); + const lines = await captured(() => + inEnv( + { + GITHUB_ACTIONS: "true", + GITHUB_REPOSITORY: "zuke-build/zuke", + GITHUB_REF: "refs/heads/master", // a branch, not a PR + GITHUB_TOKEN: "tkn", + }, + async () => { + await securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion() + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }); + }, + ) + ); + // Not the "disabled" warning — this is the expected local/push shape. + assertEquals(lines.some((l) => l.includes("discussion disabled")), false); + // No comment listing was even attempted. + assertEquals(calls.every((c) => !c.url.startsWith(`${GITHUB_API}/`)), true); + // The comment itself is skipped with the no-PR-context warning. + assertEquals(lines.some((l) => l.includes("no GitHub PR context")), true); +}); + +Deno.test("a candidate matching a fixed and a dismissed prior reopens rather than inheriting the dismissal", async () => { + // Fixed entries are offered before dismissed ones exactly so that a + // candidate the model matches to both REOPENS (which reports) instead of + // inheriting a dismissal (which silences). + const priors = [ + { + id: "oooo0001", + title: "Still-open concern", + severity: "high" as const, + status: "open" as const, + file: "src/app.ts", + }, + { + id: "dddd0001", + title: "Dismissed concern", + severity: "high" as const, + status: "dismissed" as const, + file: "src/app.ts", + rationale: "argued away", + author: "maintainer", + }, + { + id: "ffff0001", + title: "Fixed concern", + severity: "high" as const, + status: "fixed" as const, + file: "src/app.ts", + }, + ]; + const { fetch, calls } = discussionFetch( + priorComment({ findings: priors }), + [ + claude({ score: 9, severity: "high", findings: [REWORDED] }), + // The model matches the FIRST comparison — which, by the fixed-first + // ordering, is the fixed entry even though it was listed last in state. + claude({ verdicts: [{ id: "p1", verdict: "same", reason: "same" }] }), + ], + ); + const lines = await captured(() => + inPr(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion() + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }), + AiReviewError, // reopened — it gates again + ); + }) + ); + assertEquals( + lines.some((l) => l.includes("reopened under ffff0001")), + true, + ); + const state = postedState(calls); + assertEquals( + state?.findings.find((f) => f.id === "ffff0001")?.status, + "open", + ); + assertEquals( + state?.findings.find((f) => f.id === "ffff0001")?.aliases, + [REWORDED_ID], + ); + // The dismissal was NOT inherited and stays on its own entry. + assertEquals( + state?.findings.find((f) => f.id === "dddd0001")?.status, + "dismissed", + ); +}); + +Deno.test("an upheld verdict without a reason records no rationale", async () => { + const bare = { title: "Global prototype pollution", severity: "high" }; + const bareId = findingFingerprint("security", { + title: "Global prototype pollution", + severity: "high", + }); + const comments = [ + { + id: 1, + body: `Finding ${bareId} is fine, we own that global`, + user: { login: "maintainer", type: "User" }, + author_association: "OWNER", + }, + ]; + const { fetch, calls } = discussionFetch(comments, [ + claude({ score: 9, severity: "high", findings: [bare] }), + claude({ verdicts: [{ id: bareId, verdict: "upheld" }] }), + ]); + await captured(() => + inPr(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion() + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }), + AiReviewError, // upheld → still gating + ); + }) + ); + const entry = postedState(calls)?.findings.find((f) => f.id === bareId); + assertEquals(entry?.status, "upheld"); + assertEquals(entry?.rationale, undefined); // no invented reasoning + assertEquals(entry?.file, undefined); // and no invented location +}); + +Deno.test("an exhausted budget skips the adjudication; contested findings stay open", async () => { + const b = budget((x) => x.maxTokens(1)); + const comments = [ + { + id: 1, + body: `Finding ${ID} misreads the code`, + user: { login: "maintainer", type: "User" }, + author_association: "MEMBER", + }, + ]; + // The review's own usage blows the cap before the adjudication would run. + const { fetch, calls } = discussionFetch(comments, [ + claudeWithUsage({ score: 9, severity: "high", findings: [FINDING] }, { + input_tokens: 500, + output_tokens: 100, + }), + ]); + await captured(() => + inPr(async () => { + await assertRejects( + () => + securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion().budget(b) + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }), + AiReviewError, // never adjudicated → still open → still gating + ); + }) + ); + // The review call only — the maintainer's rebuttal cost nothing further. + assertEquals( + calls.filter((c) => !c.url.startsWith(`${GITHUB_API}/`)).length, + 1, + ); + assertEquals( + postedState(calls)?.findings.find((f) => f.id === ID)?.status, + "open", + ); +}); + +Deno.test("a sticky dismissal with no recorded author or rationale still mutes, rendered bare", async () => { + const priorBody = `${MARKER}\nold report\n${ + encodeState({ + findings: [{ + id: ID, + title: FINDING.title, + severity: "high", + status: "dismissed", + // No author, rationale, or file recorded — a minimal state entry. + }], + }) + }`; + const comments = [{ + id: 1, + body: priorBody, + user: { login: "github-actions[bot]", type: "Bot" }, + author_association: "NONE", + }]; + const { fetch, calls } = discussionFetch(comments, [ + claude({ score: 9, severity: "high", findings: [FINDING] }), + ]); + const lines = await captured(() => + inPr(async () => { + // Passes: the sticky dismissal mutes the re-reported finding. + await securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion() + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }); + }) + ); + // Rendered without inventing an author or a reason. + assertEquals( + lines.some((l) => l.includes(`dismissed via discussion: ${FINDING.title}`)), + true, + ); + // The dismissed-findings memory rode into the prompt as the bare line. + const provider = calls.find((c) => !c.url.startsWith(`${GITHUB_API}/`)); + const user = JSON.parse(provider?.body ?? "{}").messages[0].content; + assertEquals(user.includes(`${ID} — ${FINDING.title}`), true); +}); + +Deno.test("a missing token env skips the comment with the PR-context warning", async () => { + // A PR on GitHub Actions, but GITHUB_TOKEN is absent and no .commentToken() + // was configured: the comment is skipped, never posted unauthenticated. + const { fetch, calls } = discussionFetch([], [ + claude({ score: 0, severity: "none", findings: [] }), + ]); + const lines = await captured(() => + inEnv( + { + GITHUB_ACTIONS: "true", + GITHUB_REPOSITORY: "zuke-build/zuke", + GITHUB_REF: "refs/pull/7/merge", + // no GITHUB_TOKEN + }, + async () => { + const token = Deno.env.get("GITHUB_TOKEN"); + Deno.env.delete("GITHUB_TOKEN"); + try { + await securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment() + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }); + } finally { + if (token !== undefined) Deno.env.set("GITHUB_TOKEN", token); + } + }, + ) + ); + assertEquals( + lines.some((l) => l.includes("no GitHub PR context — skipping comment")), + true, + ); + assertEquals(calls.every((c) => !c.url.startsWith(`${GITHUB_API}/`)), true); +}); + +Deno.test("a file-less open prior is re-assessed bare and marked fixed when gone", async () => { + const priorBody = `${MARKER}\nold report\n${ + encodeState({ + findings: [{ + id: "bbbb0001", + title: "Orphan concern", + severity: "low", + status: "open", + // no file recorded + }], + }) + }`; + const comments = [{ + id: 1, + body: priorBody, + user: { login: "github-actions[bot]", type: "Bot" }, + author_association: "NONE", + }]; + const { fetch, calls } = discussionFetch(comments, [ + claude({ score: 0, severity: "none", findings: [] }), + ]); + const lines = await captured(() => + inPr(async () => { + await securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion() + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }); + }) + ); + // The prior rode into the prompt as the bare `id — title` line. + const provider = calls.find((c) => !c.url.startsWith(`${GITHUB_API}/`)); + const user = JSON.parse(provider?.body ?? "{}").messages[0].content; + assertEquals(user.includes("bbbb0001 — Orphan concern"), true); + // Not re-reported → recorded as progress, with no invented location. + assertEquals( + lines.some((l) => l.includes("fixed: Orphan concern · bbbb0001")), + true, + ); + assertEquals( + postedState(calls)?.findings.find((f) => f.id === "bbbb0001")?.status, + "fixed", + ); +}); + +Deno.test("a dismissal of a file-less finding records no location", async () => { + const bare = { title: "Global prototype pollution", severity: "high" }; + const bareId = findingFingerprint("security", { + title: "Global prototype pollution", + severity: "high", + }); + const comments = [ + { + id: 1, + body: `Finding ${bareId} cannot happen: the object is frozen at startup`, + user: { login: "maintainer", type: "User" }, + author_association: "MEMBER", + }, + ]; + const { fetch, calls } = discussionFetch(comments, [ + claude({ score: 9, severity: "high", findings: [bare] }), + claude({ + verdicts: [{ id: bareId, verdict: "dismissed", reason: "frozen object" }], + }), + ]); + await captured(() => + inPr(async () => { + // Dismissed → the gate does not trip. + await securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion() + .diff((d) => d.text(DIFF)) + .fetch(fetch) + ).validate({ target: "t" }); + }) + ); + const entry = postedState(calls)?.findings.find((f) => f.id === bareId); + assertEquals(entry?.status, "dismissed"); + assertEquals(entry?.rationale, "frozen object"); + assertEquals(entry?.file, undefined); // no invented location +}); + +Deno.test("a reason-less dismissal still closes its thread, without an invented reason", async () => { + const reply = { + id: 502, + body: "This runs in a sandboxed worker, so it cannot reach the host.", + in_reply_to_id: 501, + user: { login: "maintainer", type: "User" }, + author_association: "MEMBER", + }; + const { fetch, calls } = threadFetch( + [summaryComment({ + findings: [{ + id: ANCHORED_ID, + title: ANCHORED.title, + severity: "high", + status: "open", + file: "src/app.ts", + }], + })], + [threadRoot(ANCHORED_ID), reply], + [ + claude({ score: 9, severity: "high", findings: [ANCHORED] }), + // The adjudicator dismisses but offers no reason. + claude({ verdicts: [{ id: ANCHORED_ID, verdict: "dismissed" }] }), + ], + ); + await captured(() => + inPr(async () => { + await securityReviewer((r) => + r.provider("claude").apiKey("k") + .comment().discussion((d) => d.threads()) + .diff((d) => d.text(ANCHORED_DIFF)) + .fetch(fetch) + ).validate({ target: "t" }); + }) + ); + // The outcome reply landed in the thread, bare — no fabricated rationale. + const posted = calls.find((c) => c.url.includes("/comments/501/replies")); + const body = JSON.parse(posted?.body ?? "{}").body; + assertEquals( + body.startsWith(outcomeMarker(NAME_HASH, ANCHORED_ID, "dismissed")), + true, + ); + assertEquals(body.includes("**Dismissed via discussion**"), true); + assertEquals(body.includes("**Dismissed via discussion** —"), false); +}); diff --git a/packages/ai/tests/fixer_test.ts b/packages/ai/tests/fixer_test.ts index 035fbe5..e1915aa 100644 --- a/packages/ai/tests/fixer_test.ts +++ b/packages/ai/tests/fixer_test.ts @@ -1004,3 +1004,199 @@ Deno.test("commitAndPush is a no-op when there are nothing to commit", async () }); assertEquals(git.length, 0); }); + +Deno.test("aiFixer without a configure lambda returns a bare fixer", () => { + assertEquals(aiFixer() instanceof AiFixer, true); +}); + +Deno.test("an explicit commentToken authenticates the inline suggestions", async () => { + // GITHUB_TOKEN is deliberately unset: posting can only succeed through the + // configured token, so the assertion pins the commentToken resolution path. + const withVariedLocs: Partial = { + diagnosis: "two spots", + rootCause: "r", + confidence: "high", + locations: [ + // No suggestion and no endLine: a single-line deletion suggestion. + { file: "a.ts", line: 5, code: "const dead = 1;" }, + // A replacement across a range. + { + file: "b.ts", + line: 10, + endLine: 12, + code: "let y = 2;", + suggestion: "const y = 2;", + }, + ], + edits: [], + }; + const { fetch, calls } = suggestFetch(claudeFix(withVariedLocs)); + const prEnv: Record = { + GITHUB_ACTIONS: "true", + GITHUB_REPOSITORY: "o/r", + GITHUB_REF: "refs/pull/7/merge", + }; + const fixer = aiFixer((f) => + f.provider("claude").apiKey("k").commentToken("sug-tok") + ) + .conventions("").diff((d) => d.text("")).env((n) => prEnv[n]).fetch(fetch) + .quiet(); + await fixer.remediate(CTX); + const posts = calls.filter((c) => c.url.endsWith("/pulls/7/comments")); + const bodies = posts.map((c) => JSON.parse(c.body)); + assertEquals(bodies.length, 2); + // The suggestion-less location renders an empty (deletion) block at its line. + assertEquals(bodies[0].line, 5); + assertEquals(bodies[0].body.includes("```suggestion\n```"), true); + // The ranged location carries the replacement and spans start→end. + assertEquals(bodies[1].start_line, 10); + assertEquals(bodies[1].line, 12); + assertEquals( + bodies[1].body.includes("```suggestion\nconst y = 2;\n```"), + true, + ); +}); + +Deno.test("on GitHub without any token, no comment or suggestion is attempted", async () => { + // The CI host is detected and locations exist, but the empty token yields no + // PR context: the fixer must stay silent instead of calling the API. + const { fetch, calls } = suggestFetch(claudeFix(FIX_WITH_LOC)); + const prEnv: Record = { + GITHUB_ACTIONS: "true", + GITHUB_REPOSITORY: "o/r", + GITHUB_REF: "refs/pull/7/merge", + }; + const fixer = aiFixer((f) => f.provider("claude").apiKey("k")) + .conventions("").diff((d) => d.text("")).env((n) => prEnv[n]).fetch(fetch) + .quiet(); + const result = await fixer.remediate(CTX); + assertEquals(result.retry, false); + assertEquals( + calls.some((c) => c.url.startsWith("https://api.github.com/")), + false, + ); +}); + +Deno.test("a non-Error suggestion-post failure is stringified and falls back", async () => { + const prEnv: Record = { + GITHUB_ACTIONS: "true", + GITHUB_REPOSITORY: "o/r", + GITHUB_REF: "refs/pull/7/merge", + GITHUB_TOKEN: "tok", + }; + const calls: string[] = []; + const impl = ((input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + calls.push(url); + if (url.startsWith("https://api.github.com/")) { + // The PR-detail fetch rejects with a bare string (not an Error); the + // issue-comment GET/POST succeeds so the fallback can land. + if (!url.includes("/comments")) return Promise.reject("conn reset"); + const method = init?.method ?? "GET"; + return Promise.resolve( + new Response(method === "GET" ? "[]" : "{}", { status: 200 }), + ); + } + return Promise.resolve( + new Response(claudeFix(FIX_WITH_LOC), { status: 200 }), + ); + }) as typeof fetch; + const warnings: string[] = []; + const warn = console.warn; + console.warn = (...a: unknown[]) => void warnings.push(a.join(" ")); + try { + const fixer = aiFixer((f) => f.provider("claude").apiKey("k")) + .conventions("").diff((d) => d.text("")).env((n) => prEnv[n]).fetch(impl) + .quiet(); + await fixer.remediate(CTX); + } finally { + console.warn = warn; + } + assertEquals( + warnings.some((w) => w.includes("could not post suggestions: conn reset")), + true, + ); + // Fell back to the overview issue comment. + assertEquals(calls.some((u) => u.includes("/issues/")), true); +}); + +Deno.test("a non-Error provider failure is stringified into the warning", async () => { + const s = seams(); + const { fetch } = recordFetch("overloaded", 503); // always transient + const warnings: string[] = []; + const warn = console.warn; + console.warn = (...a: unknown[]) => void warnings.push(a.join(" ")); + let result; + try { + const fixer = s.apply( + aiFixer((f) => + f.provider("claude").apiKey("k") + // The retry's sleep seam rejects with a bare string, which escapes + // retryingFetch unwrapped — pinning the String(error) fallback. + .retry({ + attempts: 2, + sleep: () => Promise.reject("sleep torn down"), + }) + ), + ).fetch(fetch).quiet(); + result = await fixer.remediate(CTX); + } finally { + console.warn = warn; + } + assertEquals(result.retry, false); + assertEquals( + warnings.some((w) => + w.includes("could not produce a fix: sleep torn down") + ), + true, + ); +}); + +Deno.test("a non-Error apply failure is stringified into the report action", async () => { + const { fetch } = recordFetch(claudeFix(ONE_EDIT)); + const lines: string[] = []; + const log = console.log; + console.log = (...a: unknown[]) => void lines.push(a.join(" ")); + let result; + try { + const fixer = aiFixer((f) => f.provider("claude").apiKey("k").autoApply()) + .conventions("").diff((d) => d.text("")).env(() => undefined) + .write(() => Promise.reject("disk sealed")) // a bare string, not an Error + .fetch(fetch); + result = await fixer.remediate(CTX); + } finally { + console.log = log; + } + assertEquals(result.retry, false); // a failed apply never asks for a re-run + assertEquals( + lines.some((l) => l.includes("could not apply fix: disk sealed")), + true, + ); +}); + +Deno.test("a non-Error commit failure is stringified into the report action", async () => { + const { fetch } = recordFetch(claudeFix(ONE_EDIT)); + const lines: string[] = []; + const log = console.log; + console.log = (...a: unknown[]) => void lines.push(a.join(" ")); + let result; + try { + const fixer = aiFixer((f) => f.provider("claude").apiKey("k").commitFixes()) + .conventions("").diff((d) => d.text("")).env(() => undefined) + .write(() => Promise.resolve()) + .exec((argv) => + argv[1] === "commit" + ? Promise.reject("hook denied") // a bare string, not an Error + : Promise.resolve("") + ) + .fetch(fetch); + result = await fixer.remediate(CTX); + } finally { + console.log = log; + } + assertEquals(result.retry, true); // the applied fix still re-runs the target + assertEquals( + lines.some((l) => l.includes("commit failed: hook denied")), + true, + ); +}); diff --git a/packages/ai/tests/gate_test.ts b/packages/ai/tests/gate_test.ts new file mode 100644 index 0000000..4e606e5 --- /dev/null +++ b/packages/ai/tests/gate_test.ts @@ -0,0 +1,62 @@ +// Copyright (c) 2026 the Zuke contributors +// SPDX-License-Identifier: MIT + +/** + * Tests for the gate: the fluent rule builder, its human description, and the + * trip decision. + * + * @module + */ + +import { assertEquals } from "../../core/tests/_assert.ts"; +import { describeGate, GateSettings, gateTrips } from "../src/gate.ts"; +import type { Assessment } from "../src/types.ts"; + +/** An assessment with a fixed score/severity and no findings. */ +function assessed(score: number, severity: Assessment["severity"]): Assessment { + return { score, severity, summary: "", findings: [] }; +} + +Deno.test("GateSettings collects rules in order", () => { + const rules = new GateSettings().scoreAbove(8).severityAtLeast("high") + .rules_(); + assertEquals(rules, [ + { kind: "score", value: 8 }, + { kind: "severity", value: "high" }, + ]); +}); + +Deno.test("describeGate names every rule, and no rules as none", () => { + assertEquals(describeGate([]), "none"); + assertEquals(describeGate([{ kind: "score", value: 8 }]), "score>8"); + assertEquals( + describeGate([ + { kind: "score", value: 8 }, + { kind: "severity", value: "high" }, + ]), + "score>8, severity≥high", + ); +}); + +Deno.test("gateTrips fires on score strictly above and severity at least", () => { + const rules = new GateSettings().scoreAbove(8).severityAtLeast("high") + .rules_(); + // Score at the threshold does not trip — strictly above does. + assertEquals(gateTrips(assessed(8, "low"), rules).tripped, false); + assertEquals(gateTrips(assessed(9, "low"), rules), { + tripped: true, + reason: "risk score 9 exceeds 8", + }); + // Severity at (and above) the threshold trips with the severity reason. + assertEquals(gateTrips(assessed(0, "high"), rules), { + tripped: true, + reason: 'severity "high" is at least "high"', + }); + assertEquals(gateTrips(assessed(0, "critical"), rules).tripped, true); + assertEquals(gateTrips(assessed(0, "medium"), rules).tripped, false); + // No rules: nothing can trip. + assertEquals(gateTrips(assessed(10, "critical"), []), { + tripped: false, + reason: "", + }); +}); diff --git a/packages/ai/tests/hosts_test.ts b/packages/ai/tests/hosts_test.ts index d77764b..69177d3 100644 --- a/packages/ai/tests/hosts_test.ts +++ b/packages/ai/tests/hosts_test.ts @@ -1354,3 +1354,232 @@ Deno.test("listPullRequestComments treats a numeric system commentType as a bot" const listed = await listPullRequestComments(AZURE_CTX, fetch); assertEquals(listed[0].bot, true); }); + +Deno.test("resolveAzureContext refuses when any variable is absent or empty", () => { + // Each variable individually missing — and individually empty — is fatal. + for (const name of Object.keys(AZURE_ENV)) { + const without: Record = { ...AZURE_ENV }; + delete without[name]; + assertEquals(resolveAzureContext("azt", env(without)), undefined); + assertEquals( + resolveAzureContext("azt", env({ ...AZURE_ENV, [name]: "" })), + undefined, + ); + } +}); + +Deno.test("an Azure identity probe returning a non-string id fails closed", async () => { + // connectionData answers, but with a malformed identity — the reviewer can + // attribute nothing to itself, so it must POST fresh, never PATCH. + const threads = [{ + id: 7, + comments: [{ + id: 1, + content: "\nold", + author: { id: "build-service-id" }, + }], + }]; + const calls: Call[] = []; + const doFetch = ((input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + const method = init?.method ?? "GET"; + calls.push({ + url, + method, + body: typeof init?.body === "string" ? init.body : "", + }); + if (method !== "GET") return Promise.resolve(new Response("{}")); + if (url.includes("connectionData")) { + return Promise.resolve( + new Response(JSON.stringify({ authenticatedUser: { id: 42 } })), + ); + } + return Promise.resolve(new Response(JSON.stringify({ value: threads }))); + }) as typeof fetch; + await upsertPullRequestThread( + AZURE_CTX, + "security review", + "## new", + doFetch, + ); + const write = calls.find((c) => c.method !== "GET"); + assertEquals(write?.method, "POST"); +}); + +Deno.test("a thrown Azure identity probe fails closed to a fresh thread", async () => { + const threads = [{ + id: 7, + comments: [{ + id: 1, + content: "\nold", + author: { id: "build-service-id" }, + }], + }]; + const calls: Call[] = []; + const doFetch = ((input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + const method = init?.method ?? "GET"; + calls.push({ + url, + method, + body: typeof init?.body === "string" ? init.body : "", + }); + if (url.includes("connectionData")) { + return Promise.reject(new Error("dns failure")); + } + if (method !== "GET") return Promise.resolve(new Response("{}")); + return Promise.resolve(new Response(JSON.stringify({ value: threads }))); + }) as typeof fetch; + await upsertPullRequestThread( + AZURE_CTX, + "security review", + "## new", + doFetch, + ); + const write = calls.find((c) => c.method !== "GET"); + assertEquals(write?.method, "POST"); +}); + +Deno.test("an Azure author with a non-string id and no uniqueName is empty, not trusted", async () => { + const threads = [{ + id: 7, + comments: [{ + id: 1, + content: "trust me", + author: { id: 99, displayName: "Impostor" }, + }], + }]; + const { fetch } = fakeAzure(threads, { self: "build-service-id" }); + const listed = await listPullRequestComments(AZURE_CTX, fetch); + // No stable identity at all: the author is empty (and can never match a + // trustAuthors allowlist), while the display label is carried for reading. + assertEquals(listed[0].author, ""); + assertEquals(listed[0].displayName, "Impostor"); +}); + +Deno.test("upsertPullRequestThread in append mode POSTs without listing", async () => { + const { fetch, calls } = fakeAzure([], { self: "build-service-id" }); + await upsertPullRequestThread( + AZURE_CTX, + "security review", + "## round 2", + fetch, + "append", + ); + // No identity probe and no thread listing — straight to the create. + assertEquals(calls.length, 1); + assertEquals(calls[0].method, "POST"); + assertEquals(calls[0].url.endsWith("/threads?api-version=7.1"), true); +}); + +Deno.test("azureHost.prepare needs a PR context", () => { + assertEquals(azureHost.prepare("azt", env({})), undefined); + assertEquals(typeof azureHost.prepare("azt", env(AZURE_ENV)), "function"); +}); + +Deno.test("resolveBitbucketContext refuses when any variable is absent or empty", () => { + for (const name of Object.keys(BITBUCKET_ENV)) { + const without: Record = { ...BITBUCKET_ENV }; + delete without[name]; + assertEquals(resolveBitbucketContext("bbt", env(without)), undefined); + assertEquals( + resolveBitbucketContext("bbt", env({ ...BITBUCKET_ENV, [name]: "" })), + undefined, + ); + } +}); + +Deno.test("a refused or malformed Bitbucket /user probe fails closed", async () => { + const marked = [{ + id: 18, + content: { raw: "\nold" }, + user: { uuid: "{zuke}" }, + }]; + // A workspace access token: /2.0/user refuses it (401 with an error body). + const refused = fakeBitbucket(marked); + await upsertBitbucketComment( + BITBUCKET_CTX, + "security review", + "## new", + refused.fetch, + ); + const refusedWrite = refused.calls.find((c) => c.method !== "GET"); + assertEquals(refusedWrite?.method, "POST"); // cannot adopt its own comment + + // A /user body without a string uuid is the same failure. + const malformed = fakeBitbucket(marked, { self: "{zuke}" }); + const withBadUser = ((input: string | URL | Request, init?: RequestInit) => { + const url = String(input); + if (url.endsWith("/2.0/user")) { + return Promise.resolve(new Response(JSON.stringify({ uuid: 5 }))); + } + return malformed.fetch(input, init); + }) as typeof fetch; + await upsertBitbucketComment( + BITBUCKET_CTX, + "security review", + "## new", + withBadUser, + ); + const write = malformed.calls.find((c) => c.method !== "GET"); + assertEquals(write?.method, "POST"); +}); + +Deno.test("a thrown Bitbucket /user probe fails closed to a fresh comment", async () => { + const marked = [{ + id: 18, + content: { raw: "\nold" }, + user: { uuid: "{zuke}" }, + }]; + const inner = fakeBitbucket(marked, { self: "{zuke}" }); + const throwing = ((input: string | URL | Request, init?: RequestInit) => { + if (String(input).endsWith("/2.0/user")) { + return Promise.reject(new Error("dns failure")); + } + return inner.fetch(input, init); + }) as typeof fetch; + await upsertBitbucketComment( + BITBUCKET_CTX, + "security review", + "## new", + throwing, + ); + const write = inner.calls.find((c) => c.method !== "GET"); + assertEquals(write?.method, "POST"); +}); + +Deno.test("listBitbucketComments drops malformed and deleted comments", async () => { + const values = [ + { id: "not-a-number", content: { raw: "x" } }, // id wrong type — dropped + { id: 2, content: { raw: 42 } }, // body wrong type — dropped + { id: 3, content: { raw: "gone" }, deleted: true }, // deleted — dropped + { id: 4, content: { raw: "kept" }, user: { uuid: "{dev}" } }, + ]; + const { fetch } = fakeBitbucket(values, { self: "{zuke}", members: [] }); + const listed = await listBitbucketComments(BITBUCKET_CTX, fetch); + assertEquals(listed.map((c) => c.id), [4]); + assertEquals(listed[0].body, "kept"); +}); + +Deno.test("upsertBitbucketComment in append mode POSTs without listing", async () => { + const { fetch, calls } = fakeBitbucket([], { self: "{zuke}" }); + await upsertBitbucketComment( + BITBUCKET_CTX, + "security review", + "## round 2", + fetch, + "append", + ); + // No /user probe and no comment listing — straight to the create. + assertEquals(calls.length, 1); + assertEquals(calls[0].method, "POST"); + assertEquals(calls[0].url.endsWith("/pullrequests/5/comments"), true); +}); + +Deno.test("bitbucketHost.prepare needs a PR context", () => { + assertEquals(bitbucketHost.prepare("bbt", env({})), undefined); + assertEquals( + typeof bitbucketHost.prepare("bbt", env(BITBUCKET_ENV)), + "function", + ); +}); diff --git a/packages/ai/tests/report_test.ts b/packages/ai/tests/report_test.ts new file mode 100644 index 0000000..7dc7a38 --- /dev/null +++ b/packages/ai/tests/report_test.ts @@ -0,0 +1,214 @@ +// Copyright (c) 2026 the Zuke contributors +// SPDX-License-Identifier: MIT + +/** + * Tests for the report renderer: console lines, the job-summary Markdown, and + * the best-effort step-summary writer. + * + * @module + */ + +import { + assertEquals, + assertStringIncludes, +} from "../../core/tests/_assert.ts"; +import { + consoleLines, + formatUsage, + reviewStartLine, + skipConsoleLine, + skipMarkdown, + toMarkdown, +} from "../src/report.ts"; +import type { Assessment } from "../src/types.ts"; + +/** A small assessment with one located, fingerprinted finding. */ +const ASSESSMENT: Assessment = { + score: 6, + severity: "medium", + summary: "one real issue", + findings: [ + { + title: "sql injection", + severity: "high", + file: "db.ts", + line: 3, + id: "abc123", + }, + { title: "vague worry", severity: "low" }, // no id, no location + ], +}; + +Deno.test("formatUsage renders only the counts the provider reported", () => { + assertEquals(formatUsage(undefined), undefined); + // A usage object with no counts at all is not an empty line — it is nothing. + assertEquals(formatUsage({}), undefined); + assertEquals(formatUsage({ inputTokens: 12 }), "12 in"); + assertEquals( + formatUsage({ inputTokens: 1, outputTokens: 2, totalTokens: 3 }), + "1 in · 2 out · 3 total", + ); +}); + +Deno.test("reviewStartLine echoes the gate and the comment flag", () => { + assertEquals( + reviewStartLine("sec", { + target: "deploy", + provider: "claude", + model: "m", + gate: "score>8", + comment: true, + }), + '[sec] reviewing "deploy" — claude/m · gate score>8 · comment', + ); + assertEquals( + reviewStartLine("sec", { + target: "deploy", + provider: "openai", + model: "m", + gate: "none", + comment: false, + }).includes("comment"), + false, + ); +}); + +Deno.test("consoleLines prints ids and locations only where they exist", () => { + const lines = consoleLines("sec", ASSESSMENT); + assertEquals(lines[0], "[sec] score 6/10 (medium) — 2 finding(s)"); + // The located, fingerprinted finding carries both suffixes … + assertEquals(lines[1], " - [high] sql injection (db.ts:3) · abc123"); + // … the bare one carries neither. + assertEquals(lines[2], " - [low] vague worry"); + assertEquals(lines[3], " one real issue"); +}); + +Deno.test("consoleLines audits suppressed, refuted, dismissed, and fixed findings", () => { + const lines = consoleLines("sec", ASSESSMENT, undefined, { + suppressed: 2, + suppressedFindings: [ + { title: "muted", severity: "low", file: "a.ts", line: 1, id: "id1" }, + { title: "muted bare", severity: "low" }, // no id, no location + ], + refuted: [ + { finding: { title: "not real", severity: "low" }, reason: "guarded" }, + { finding: { title: "unexplained", severity: "low" } }, // no reason + ], + dismissed: [ + { + finding: { title: "argued away", severity: "low" }, + author: "alice", + reason: "intended", + rewordedFrom: "earlier words", + }, + { finding: { title: "quietly gone", severity: "low" } }, // no author/reason + ], + fixed: [ + { + id: "f1", + title: "fixed one", + severity: "high", + status: "open", + file: "b.ts", + }, + { id: "f2", title: "fixed bare", severity: "low", status: "open" }, // no file + ], + notes: ["a bounded pass"], + }); + const text = lines.join("\n"); + assertStringIncludes(text, "suppressed 2 finding(s) via the suppress list"); + assertStringIncludes(text, " suppressed: [low] muted (a.ts:1) · id1"); + assertStringIncludes(text, " suppressed: [low] muted bare"); + assertStringIncludes(text, " refuted by verify: not real — guarded"); + assertStringIncludes(text, " refuted by verify: unexplained"); + assertStringIncludes( + text, + ' dismissed via discussion by alice: argued away (reworded from "earlier words") — intended', + ); + assertStringIncludes(text, " dismissed via discussion: quietly gone"); + assertStringIncludes(text, " fixed: fixed one (b.ts) · f1"); + assertStringIncludes(text, " fixed: fixed bare · f2"); + assertStringIncludes(text, " note: a bounded pass"); +}); + +Deno.test("toMarkdown fills missing audit fields with an em dash", () => { + const md = toMarkdown("sec", "deploy", ASSESSMENT, undefined, { + refuted: [{ finding: { title: "not real", severity: "low" } }], + dismissed: [{ finding: { title: "quietly gone", severity: "low" } }], + fixed: [{ id: "f2", title: "fixed bare", severity: "low", status: "open" }], + notes: ["pass skipped for budget"], + }); + // A refuted finding with no reason, and a dismissal with no author, reason, + // or finding id, render as explicit dashes — never as empty cells. + assertStringIncludes(md, "| not real | — |"); + assertStringIncludes(md, "| quietly gone | — | — | — |"); + // A fixed finding without a file gets a dash location. + assertStringIncludes(md, "| low | fixed bare | — | f2 |"); + assertStringIncludes(md, "- pass skipped for budget"); +}); + +Deno.test("toMarkdown names the dismisser, the reason, and the rewording", () => { + const md = toMarkdown("sec", "deploy", ASSESSMENT, undefined, { + dismissed: [{ + finding: { title: "argued away", severity: "low", id: "d1" }, + author: "alice", + reason: "intended", + rewordedFrom: "earlier words", + }], + }); + assertStringIncludes( + md, + '| argued away _(reworded from "earlier words")_ | alice | intended | d1 |', + ); +}); + +Deno.test("skipMarkdown neutralises the reason like any untrusted value", () => { + const md = skipMarkdown("sec", "deploy", "no key"); + assertEquals(md.includes("