From 8a9e1a641b81982b587d43054233ab0a6153a233 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 10 Apr 2026 15:24:43 -0400 Subject: [PATCH 1/2] feat(cli): help agents keep Hunk skills in sync --- CHANGELOG.md | 4 + README.md | 3 + bin/hunk.cjs | 16 +- scripts/check-prebuilt-pack.ts | 8 +- scripts/smoke-prebuilt-install.ts | 14 +- scripts/stage-prebuilt-npm.ts | 3 +- src/core/cli.test.ts | 27 ++++ src/core/cli.ts | 51 ++++++ src/core/config.ts | 16 +- src/core/paths.test.ts | 54 +++++++ src/core/paths.ts | 77 +++++++++ src/core/updateNotice.test.ts | 255 +++++++++++++++++++++--------- src/core/updateNotice.ts | 94 ++++++++++- test/cli/entrypoint.test.ts | 41 ++++- test/smoke/tty.test.ts | 2 + 15 files changed, 565 insertions(+), 100 deletions(-) create mode 100644 src/core/paths.test.ts create mode 100644 src/core/paths.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d5d048f..4f41560 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,12 @@ All notable user-visible changes to Hunk are documented in this file. ### Added +- Added `hunk skill path` to print the bundled Hunk review skill path for direct loading or symlinking in coding agents. + ### Changed +- Show a one-time startup notice after version changes that points users with copied agent skills to `hunk skill path`. + ### Fixed ## [0.9.1] - 2026-04-10 diff --git a/README.md b/README.md index 6e5aa41..af34425 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,8 @@ git diff --no-color | hunk patch - # review a patch from stdin Load the [`skills/hunk-review/SKILL.md`](skills/hunk-review/SKILL.md) skill in your coding agent (e.g. Claude, Codex, Opencode, Pi). +Run `hunk skill path` to print the installed bundled skill path. Prefer loading or symlinking that file in your agent instead of copying it so Hunk upgrades stay in sync automatically. + Open Hunk in another window, then ask your agent to leave comments. ## Feature comparison @@ -144,6 +146,7 @@ Hunk supports two agent workflows: #### Steer a live Hunk window Use the Hunk review skill: [`skills/hunk-review/SKILL.md`](skills/hunk-review/SKILL.md). +If you need the installed absolute path for your agent setup, run `hunk skill path`. A good generic prompt is: diff --git a/bin/hunk.cjs b/bin/hunk.cjs index 5454b8c..35e5a8e 100755 --- a/bin/hunk.cjs +++ b/bin/hunk.cjs @@ -5,6 +5,10 @@ const fs = require("node:fs"); const os = require("node:os"); const path = require("node:path"); +function bundledSkillPath() { + return path.join(__dirname, "..", "skills", "hunk-review", "SKILL.md"); +} + function ensureExecutable(target) { if (process.platform === "win32") { return; @@ -94,21 +98,27 @@ function bundledBunRuntime() { } } +const forwardedArgs = process.argv.slice(2); +if (forwardedArgs.length === 2 && forwardedArgs[0] === "skill" && forwardedArgs[1] === "path") { + process.stdout.write(`${bundledSkillPath()}\n`); + process.exit(0); +} + const overrideBinary = process.env.HUNK_BIN_PATH; if (overrideBinary) { - run(overrideBinary, process.argv.slice(2)); + run(overrideBinary, forwardedArgs); } const scriptDir = path.dirname(fs.realpathSync(__filename)); const prebuiltBinary = findInstalledBinary(scriptDir); if (prebuiltBinary) { - run(prebuiltBinary, process.argv.slice(2)); + run(prebuiltBinary, forwardedArgs); } const bunBinary = bundledBunRuntime(); if (bunBinary) { const entrypoint = path.join(__dirname, "..", "dist", "npm", "main.js"); - run(bunBinary, [entrypoint, ...process.argv.slice(2)]); + run(bunBinary, [entrypoint, ...forwardedArgs]); } const printablePackages = hostCandidates() diff --git a/scripts/check-prebuilt-pack.ts b/scripts/check-prebuilt-pack.ts index 64bb5d8..ca1dfc0 100644 --- a/scripts/check-prebuilt-pack.ts +++ b/scripts/check-prebuilt-pack.ts @@ -63,7 +63,13 @@ if (!existsSync(metaDir)) { } const metaPack = runPackDryRun(metaDir); -assertPaths(metaPack, ["bin/hunk.cjs", "README.md", "LICENSE", "package.json"]); +assertPaths(metaPack, [ + "bin/hunk.cjs", + "skills/hunk-review/SKILL.md", + "README.md", + "LICENSE", + "package.json", +]); const packageDirectories = readdirSync(releaseRoot, { withFileTypes: true }) .filter((entry) => entry.isDirectory() && entry.name !== "hunkdiff") diff --git a/scripts/smoke-prebuilt-install.ts b/scripts/smoke-prebuilt-install.ts index e0db0e8..43e24f4 100644 --- a/scripts/smoke-prebuilt-install.ts +++ b/scripts/smoke-prebuilt-install.ts @@ -1,6 +1,6 @@ #!/usr/bin/env bun -import { mkdtempSync, mkdirSync, rmSync } from "node:fs"; +import { existsSync, mkdtempSync, mkdirSync, rmSync } from "node:fs"; import path from "node:path"; import { getHostPlatformPackageSpec, releaseNpmDir } from "./prebuilt-package-helpers"; @@ -83,6 +83,18 @@ try { ); } + const skillPath = run([installedHunk, "skill", "path"], { + env: commandEnv, + }).stdout.trim(); + if ( + !skillPath.endsWith(path.join("skills", "hunk-review", "SKILL.md")) || + !existsSync(skillPath) + ) { + throw new Error( + `Expected installed hunk skill path to resolve to the bundled skill.\n${skillPath}`, + ); + } + const bunCheck = Bun.spawnSync( [ resolvedNode, diff --git a/scripts/stage-prebuilt-npm.ts b/scripts/stage-prebuilt-npm.ts index 9336e8f..5c89bd2 100644 --- a/scripts/stage-prebuilt-npm.ts +++ b/scripts/stage-prebuilt-npm.ts @@ -77,6 +77,7 @@ function stageMetaPackage( const metaDir = path.join(releaseRoot, rootPackage.name); ensureDirectory(path.join(metaDir, "bin")); cpSync(path.join(repoRoot, "bin", "hunk.cjs"), path.join(metaDir, "bin", "hunk.cjs")); + cpSync(path.join(repoRoot, "skills"), path.join(metaDir, "skills"), { recursive: true }); cpSync(path.join(repoRoot, "README.md"), path.join(metaDir, "README.md")); cpSync(path.join(repoRoot, "LICENSE"), path.join(metaDir, "LICENSE")); @@ -87,7 +88,7 @@ function stageMetaPackage( bin: { hunk: "./bin/hunk.cjs", }, - files: ["bin", "README.md", "LICENSE"], + files: ["bin", "skills", "README.md", "LICENSE"], keywords: rootPackage.keywords, repository: rootPackage.repository, homepage: rootPackage.homepage, diff --git a/src/core/cli.test.ts b/src/core/cli.test.ts index 64b4182..46294fe 100644 --- a/src/core/cli.test.ts +++ b/src/core/cli.test.ts @@ -34,6 +34,7 @@ describe("parseCli", () => { expect(parsed.text).toContain("Usage:"); expect(parsed.text).toContain("hunk diff"); expect(parsed.text).toContain("hunk show"); + expect(parsed.text).toContain("hunk skill path"); expect(parsed.text).toContain("Global options:"); expect(parsed.text).toContain("Common review options:"); expect(parsed.text).toContain("auto-reload when the current diff input changes"); @@ -188,6 +189,32 @@ describe("parseCli", () => { }); }); + test("prints the bundled skill path for hunk skill path", async () => { + const parsed = await parseCli(["bun", "hunk", "skill", "path"]); + + expect(parsed.kind).toBe("help"); + if (parsed.kind !== "help") { + throw new Error("Expected bundled skill path output."); + } + + expect(parsed.text).toEndWith(`${join("skills", "hunk-review", "SKILL.md")}\n`); + }); + + test("prints skill help for hunk skill --help", async () => { + const parsed = await parseCli(["bun", "hunk", "skill", "--help"]); + + expect(parsed).toEqual({ + kind: "help", + text: [ + "Usage: hunk skill path", + "", + "Print the bundled Hunk review skill path.", + "Load or symlink that file in your coding agent to keep it in sync across Hunk upgrades.", + "", + ].join("\n"), + }); + }); + test("parses the MCP daemon command", async () => { const parsed = await parseCli(["bun", "hunk", "mcp", "serve"]); diff --git a/src/core/cli.ts b/src/core/cli.ts index dcd3fb5..b306eab 100644 --- a/src/core/cli.ts +++ b/src/core/cli.ts @@ -10,6 +10,7 @@ import type { ParsedCliInput, SessionCommentApplyItemInput, } from "./types"; +import { resolveBundledHunkReviewSkillPath } from "./paths"; import { resolveCliVersion } from "./version"; /** Validate one requested layout mode from CLI input. */ @@ -101,6 +102,22 @@ function renderCliVersion() { return `${resolveCliVersion()}\n`; } +/** Render the bundled Hunk review skill path for shell usage. */ +function renderHunkReviewSkillPath() { + return `${resolveBundledHunkReviewSkillPath()}\n`; +} + +/** Build the `hunk skill` help text. */ +function renderSkillHelp() { + return [ + "Usage: hunk skill path", + "", + "Print the bundled Hunk review skill path.", + "Load or symlink that file in your coding agent to keep it in sync across Hunk upgrades.", + "", + ].join("\n"); +} + /** Build the top-level help text shown by bare `hunk` and `hunk --help`. */ function renderCliHelp() { return [ @@ -118,6 +135,7 @@ function renderCliHelp() { " hunk pager general Git pager wrapper with diff detection", " hunk difftool [path] review Git difftool file pairs", " hunk session inspect or control a live Hunk session", + " hunk skill path print the bundled Hunk review skill path", " hunk mcp serve run the local Hunk session daemon", "", "Global options:", @@ -1141,6 +1159,37 @@ async function parseSessionCommand(tokens: string[]): Promise { throw new Error(`Unknown session command: ${subcommand}`); } +/** Parse `hunk skill ...` for bundled skill discovery commands. */ +async function parseSkillCommand(tokens: string[]): Promise { + const [subcommand, ...rest] = tokens; + if (!subcommand || subcommand === "--help" || subcommand === "-h") { + return { + kind: "help", + text: renderSkillHelp(), + }; + } + + if (subcommand !== "path") { + throw new Error("Only `hunk skill path` is supported."); + } + + if (rest.includes("--help") || rest.includes("-h")) { + return { + kind: "help", + text: renderSkillHelp(), + }; + } + + if (rest.length > 0) { + throw new Error("`hunk skill path` does not accept additional arguments."); + } + + return { + kind: "help", + text: renderHunkReviewSkillPath(), + }; +} + /** Parse `hunk mcp serve` as the local daemon entrypoint. */ async function parseMcpCommand(tokens: string[]): Promise { const [subcommand, ...rest] = tokens; @@ -1258,6 +1307,8 @@ export async function parseCli(argv: string[]): Promise { return parseStashCommand(rest, argv); case "session": return parseSessionCommand(rest); + case "skill": + return parseSkillCommand(rest); case "mcp": return parseMcpCommand(rest); default: diff --git a/src/core/config.ts b/src/core/config.ts index 75e87ea..6e8466c 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import { dirname, join, resolve } from "node:path"; +import { resolveGlobalConfigPath } from "./paths"; import type { CliInput, CommonOptions, LayoutMode, PersistedViewPreferences } from "./types"; const DEFAULT_VIEW_PREFERENCES: PersistedViewPreferences = { @@ -105,19 +106,6 @@ function findRepoRoot(cwd = process.cwd()) { } } -/** Resolve the global XDG-style config path, if the environment provides one. */ -function globalConfigPath(env: NodeJS.ProcessEnv = process.env) { - if (env.XDG_CONFIG_HOME) { - return join(env.XDG_CONFIG_HOME, "hunk", "config.toml"); - } - - if (env.HOME) { - return join(env.HOME, ".config", "hunk", "config.toml"); - } - - return undefined; -} - /** Parse one TOML config file into a plain object. */ function readTomlRecord(path: string) { if (!fs.existsSync(path)) { @@ -139,7 +127,7 @@ export function resolveConfiguredCliInput( ): HunkConfigResolution { const repoRoot = findRepoRoot(cwd); const repoConfigPath = repoRoot ? join(repoRoot, ".hunk", "config.toml") : undefined; - const userConfigPath = globalConfigPath(env); + const userConfigPath = resolveGlobalConfigPath(env); let resolvedOptions: CommonOptions = { mode: DEFAULT_VIEW_PREFERENCES.mode, diff --git a/src/core/paths.test.ts b/src/core/paths.test.ts new file mode 100644 index 0000000..4d0b174 --- /dev/null +++ b/src/core/paths.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { + resolveBundledHunkReviewSkillPath, + resolveGlobalConfigPath, + resolveHunkStatePath, +} from "./paths"; + +function createTempRoot(prefix: string) { + return mkdtempSync(join(tmpdir(), prefix)); +} + +describe("paths", () => { + test("resolves XDG config and state paths", () => { + const env = { XDG_CONFIG_HOME: "/tmp/xdg-home" } as NodeJS.ProcessEnv; + + expect(resolveGlobalConfigPath(env)).toBe("/tmp/xdg-home/hunk/config.toml"); + expect(resolveHunkStatePath(env)).toBe("/tmp/xdg-home/hunk/state.json"); + }); + + test("falls back to HOME for config and state paths", () => { + const env = { HOME: "/tmp/home" } as NodeJS.ProcessEnv; + + expect(resolveGlobalConfigPath(env)).toBe("/tmp/home/.config/hunk/config.toml"); + expect(resolveHunkStatePath(env)).toBe("/tmp/home/.config/hunk/state.json"); + }); + + test("locates the bundled Hunk review skill from source", () => { + const resolvedPath = resolveBundledHunkReviewSkillPath([import.meta.dir]); + + expect(resolvedPath).toEndWith(join("skills", "hunk-review", "SKILL.md")); + }); + + test("locates the bundled Hunk review skill through a nested hunkdiff package", () => { + const tempRoot = createTempRoot("hunk-skill-path-"); + + try { + const nestedPackageRoot = join(tempRoot, "node_modules", "hunkdiff"); + const skillPath = join(nestedPackageRoot, "skills", "hunk-review", "SKILL.md"); + const fakeBinary = join(tempRoot, "node_modules", "hunkdiff-linux-x64", "bin", "hunk"); + + mkdirSync(dirname(skillPath), { recursive: true }); + mkdirSync(dirname(fakeBinary), { recursive: true }); + writeFileSync(skillPath, "# skill\n"); + writeFileSync(fakeBinary, "binary\n"); + + expect(resolveBundledHunkReviewSkillPath([fakeBinary])).toBe(skillPath); + } finally { + rmSync(tempRoot, { recursive: true, force: true }); + } + }); +}); diff --git a/src/core/paths.ts b/src/core/paths.ts new file mode 100644 index 0000000..7ee88bf --- /dev/null +++ b/src/core/paths.ts @@ -0,0 +1,77 @@ +import fs from "node:fs"; +import { dirname, join, resolve } from "node:path"; + +const HUNK_REVIEW_SKILL_RELATIVE_PATH = join("skills", "hunk-review", "SKILL.md"); + +/** Resolve the base config directory Hunk should use for user-scoped files. */ +export function resolveUserConfigDir(env: NodeJS.ProcessEnv = process.env) { + if (env.XDG_CONFIG_HOME) { + return env.XDG_CONFIG_HOME; + } + + if (env.HOME) { + return join(env.HOME, ".config"); + } + + return undefined; +} + +/** Resolve the global Hunk config file path from the current environment. */ +export function resolveGlobalConfigPath(env: NodeJS.ProcessEnv = process.env) { + const configDir = resolveUserConfigDir(env); + return configDir ? join(configDir, "hunk", "config.toml") : undefined; +} + +/** Resolve the persisted Hunk state file path from the current environment. */ +export function resolveHunkStatePath(env: NodeJS.ProcessEnv = process.env) { + const configDir = resolveUserConfigDir(env); + return configDir ? join(configDir, "hunk", "state.json") : undefined; +} + +/** Search one path and its parents for one relative child path. */ +function findRelativePathFromAncestors(startPath: string, relativePath: string) { + let current = resolve(startPath); + + try { + if (fs.statSync(current).isFile()) { + current = dirname(current); + } + } catch { + // Treat non-existent paths as directories so ancestor walking still works in tests. + } + + for (;;) { + const candidate = join(current, relativePath); + if (fs.existsSync(candidate)) { + return candidate; + } + + const parent = dirname(current); + if (parent === current) { + return undefined; + } + + current = parent; + } +} + +/** Resolve the bundled Hunk review skill path from source, npm, or prebuilt package layouts. */ +export function resolveBundledHunkReviewSkillPath(searchRoots?: string[]) { + const roots = searchRoots ?? [import.meta.dir, process.execPath]; + const relativeCandidates = [ + HUNK_REVIEW_SKILL_RELATIVE_PATH, + join("hunkdiff", HUNK_REVIEW_SKILL_RELATIVE_PATH), + join("node_modules", "hunkdiff", HUNK_REVIEW_SKILL_RELATIVE_PATH), + ]; + + for (const root of roots) { + for (const relativePath of relativeCandidates) { + const resolvedPath = findRelativePathFromAncestors(root, relativePath); + if (resolvedPath) { + return resolvedPath; + } + } + } + + throw new Error("Could not locate the bundled Hunk review skill."); +} diff --git a/src/core/updateNotice.test.ts b/src/core/updateNotice.test.ts index 1364f5a..a594d3d 100644 --- a/src/core/updateNotice.test.ts +++ b/src/core/updateNotice.test.ts @@ -1,4 +1,7 @@ import { describe, expect, test } from "bun:test"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { resolveStartupUpdateNotice } from "./updateNotice"; /** Build one JSON response that mimics the npm dist-tags payload. */ @@ -9,94 +12,193 @@ function createDistTagsResponse(tags: Record, status = 200) { }); } +async function withTempStatePath(run: (statePath: string) => Promise) { + const stateDir = mkdtempSync(join(tmpdir(), "hunk-startup-notice-")); + const statePath = join(stateDir, "state.json"); + + try { + await run(statePath); + } finally { + rmSync(stateDir, { recursive: true, force: true }); + } +} + describe("startup update notice", () => { test("prefers latest for stable installs when latest is newer", async () => { - await expect( - resolveStartupUpdateNotice({ - fetchImpl: async () => createDistTagsResponse({ latest: "0.7.1", beta: "0.8.0-beta.1" }), - resolveInstalledVersion: () => "0.7.0", - }), - ).resolves.toEqual({ - key: "latest:0.7.1", - message: "Update available: 0.7.1 (latest) • npm i -g hunkdiff", + await withTempStatePath(async (statePath) => { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => createDistTagsResponse({ latest: "0.7.1", beta: "0.8.0-beta.1" }), + resolveInstalledVersion: () => "0.7.0", + statePath, + }), + ).resolves.toEqual({ + key: "latest:0.7.1", + message: "Update available: 0.7.1 (latest) • npm i -g hunkdiff", + }); }); }); test("falls back to beta for stable installs when latest is not newer", async () => { - await expect( - resolveStartupUpdateNotice({ - fetchImpl: async () => createDistTagsResponse({ latest: "0.7.0", beta: "0.8.0-beta.1" }), - resolveInstalledVersion: () => "0.7.0", - }), - ).resolves.toEqual({ - key: "beta:0.8.0-beta.1", - message: "Update available: 0.8.0-beta.1 (beta) • npm i -g hunkdiff@beta", + await withTempStatePath(async (statePath) => { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => createDistTagsResponse({ latest: "0.7.0", beta: "0.8.0-beta.1" }), + resolveInstalledVersion: () => "0.7.0", + statePath, + }), + ).resolves.toEqual({ + key: "beta:0.8.0-beta.1", + message: "Update available: 0.8.0-beta.1 (beta) • npm i -g hunkdiff@beta", + }); }); }); test("beta installs choose the higher newer version between latest and beta", async () => { - await expect( - resolveStartupUpdateNotice({ - fetchImpl: async () => createDistTagsResponse({ latest: "0.8.0", beta: "0.8.1-beta.1" }), - resolveInstalledVersion: () => "0.8.0-beta.1", - }), - ).resolves.toEqual({ - key: "beta:0.8.1-beta.1", - message: "Update available: 0.8.1-beta.1 (beta) • npm i -g hunkdiff@beta", + await withTempStatePath(async (statePath) => { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => createDistTagsResponse({ latest: "0.8.0", beta: "0.8.1-beta.1" }), + resolveInstalledVersion: () => "0.8.0-beta.1", + statePath, + }), + ).resolves.toEqual({ + key: "beta:0.8.1-beta.1", + message: "Update available: 0.8.1-beta.1 (beta) • npm i -g hunkdiff@beta", + }); }); }); test("returns null when already up to date", async () => { - await expect( - resolveStartupUpdateNotice({ - fetchImpl: async () => createDistTagsResponse({ latest: "0.7.0", beta: "0.7.0-beta.1" }), - resolveInstalledVersion: () => "0.7.0", - }), - ).resolves.toBeNull(); + await withTempStatePath(async (statePath) => { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => createDistTagsResponse({ latest: "0.7.0", beta: "0.7.0-beta.1" }), + resolveInstalledVersion: () => "0.7.0", + statePath, + }), + ).resolves.toBeNull(); + }); }); - test("returns null for unresolved local versions", async () => { - await expect( - resolveStartupUpdateNotice({ - fetchImpl: async () => createDistTagsResponse({ latest: "0.7.0", beta: "0.8.0-beta.1" }), - resolveInstalledVersion: () => "0.0.0-unknown", - }), - ).resolves.toBeNull(); + test("stores the current version on first run without showing a copied-skill notice", async () => { + const stateDir = mkdtempSync(join(tmpdir(), "hunk-startup-notice-")); + const statePath = join(stateDir, "state.json"); + + try { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => createDistTagsResponse({ latest: "0.7.0" }), + resolveInstalledVersion: () => "0.7.0", + statePath, + }), + ).resolves.toBeNull(); + + expect(JSON.parse(readFileSync(statePath, "utf8"))).toEqual({ + version: 1, + lastSeenCliVersion: "0.7.0", + }); + } finally { + rmSync(stateDir, { recursive: true, force: true }); + } }); - test("returns null on non-ok responses", async () => { - await expect( - resolveStartupUpdateNotice({ - fetchImpl: async () => createDistTagsResponse({ latest: "0.7.1" }, 503), - resolveInstalledVersion: () => "0.7.0", - }), - ).resolves.toBeNull(); + test("shows a one-time copied-skill refresh notice after a version change", async () => { + const stateDir = mkdtempSync(join(tmpdir(), "hunk-startup-notice-")); + const statePath = join(stateDir, "state.json"); + let fetchCalled = false; + + try { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => createDistTagsResponse({ latest: "0.7.0" }), + resolveInstalledVersion: () => "0.7.0", + statePath, + }), + ).resolves.toBeNull(); + + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => { + fetchCalled = true; + return createDistTagsResponse({ latest: "0.8.0" }); + }, + resolveInstalledVersion: () => "0.8.0", + statePath, + }), + ).resolves.toEqual({ + key: "skill:0.8.0", + message: "Hunk 0.8.0 installed • If your agent copied Hunk's skill, run hunk skill path", + }); + + expect(fetchCalled).toBe(false); + + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => createDistTagsResponse({ latest: "0.8.0" }), + resolveInstalledVersion: () => "0.8.0", + statePath, + }), + ).resolves.toBeNull(); + } finally { + rmSync(stateDir, { recursive: true, force: true }); + } }); - test("returns null on fetch failure", async () => { - await expect( - resolveStartupUpdateNotice({ - fetchImpl: async () => { - throw new Error("network down"); - }, - resolveInstalledVersion: () => "0.7.0", - }), - ).resolves.toBeNull(); + test("returns null for unresolved local versions", async () => { + await withTempStatePath(async (statePath) => { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => createDistTagsResponse({ latest: "0.7.0", beta: "0.8.0-beta.1" }), + resolveInstalledVersion: () => "0.0.0-unknown", + statePath, + }), + ).resolves.toBeNull(); + }); }); - test("returns null immediately when the CI disable env is set", async () => { - const previous = process.env.HUNK_DISABLE_UPDATE_NOTICE; - process.env.HUNK_DISABLE_UPDATE_NOTICE = "1"; + test("returns null on non-ok responses", async () => { + await withTempStatePath(async (statePath) => { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => createDistTagsResponse({ latest: "0.7.1" }, 503), + resolveInstalledVersion: () => "0.7.0", + statePath, + }), + ).resolves.toBeNull(); + }); + }); - try { + test("returns null on fetch failure", async () => { + await withTempStatePath(async (statePath) => { await expect( resolveStartupUpdateNotice({ fetchImpl: async () => { - throw new Error("should not fetch when disabled"); + throw new Error("network down"); }, resolveInstalledVersion: () => "0.7.0", + statePath, }), ).resolves.toBeNull(); + }); + }); + + test("returns null immediately when the CI disable env is set", async () => { + const previous = process.env.HUNK_DISABLE_UPDATE_NOTICE; + process.env.HUNK_DISABLE_UPDATE_NOTICE = "1"; + + try { + await withTempStatePath(async (statePath) => { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async () => { + throw new Error("should not fetch when disabled"); + }, + resolveInstalledVersion: () => "0.7.0", + statePath, + }), + ).resolves.toBeNull(); + }); } finally { if (previous === undefined) { delete process.env.HUNK_DISABLE_UPDATE_NOTICE; @@ -109,23 +211,26 @@ describe("startup update notice", () => { test("aborts hung fetches after the timeout", async () => { let aborted = false; - await expect( - resolveStartupUpdateNotice({ - fetchImpl: async (_input, init) => - new Promise((_resolve, reject) => { - init?.signal?.addEventListener( - "abort", - () => { - aborted = true; - reject(new Error("aborted")); - }, - { once: true }, - ); - }), - fetchTimeoutMs: 10, - resolveInstalledVersion: () => "0.7.0", - }), - ).resolves.toBeNull(); + await withTempStatePath(async (statePath) => { + await expect( + resolveStartupUpdateNotice({ + fetchImpl: async (_input, init) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener( + "abort", + () => { + aborted = true; + reject(new Error("aborted")); + }, + { once: true }, + ); + }), + fetchTimeoutMs: 10, + resolveInstalledVersion: () => "0.7.0", + statePath, + }), + ).resolves.toBeNull(); + }); expect(aborted).toBe(true); }); diff --git a/src/core/updateNotice.ts b/src/core/updateNotice.ts index 8372341..1c2311e 100644 --- a/src/core/updateNotice.ts +++ b/src/core/updateNotice.ts @@ -1,3 +1,6 @@ +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { dirname } from "node:path"; +import { resolveHunkStatePath } from "./paths"; import { resolveCliVersion, UNKNOWN_CLI_VERSION } from "./version"; const DIST_TAGS_URL = "https://registry.npmjs.org/-/package/hunkdiff/dist-tags"; @@ -5,6 +8,12 @@ const STABLE_SEMVER_PATTERN = /^\d+\.\d+\.\d+$/; const PRERELEASE_SEMVER_PATTERN = /^\d+\.\d+\.\d+-[0-9A-Za-z.-]+$/; const DEFAULT_UPDATE_NOTICE_FETCH_TIMEOUT_MS = 5_000; const DISABLE_STARTUP_UPDATE_NOTICE_ENV = "HUNK_DISABLE_UPDATE_NOTICE"; +const STARTUP_STATE_VERSION = 1; + +interface PersistedStartupState { + version: number; + lastSeenCliVersion?: string; +} export type UpdateChannel = "latest" | "beta"; @@ -21,9 +30,11 @@ interface ParsedDistTags { } export interface UpdateNoticeDeps { + env?: NodeJS.ProcessEnv; fetchImpl?: FetchImpl; fetchTimeoutMs?: number; resolveInstalledVersion?: () => string; + statePath?: string; } /** Return whether one version string is a normalized stable semver. */ @@ -147,19 +158,94 @@ function createFetchTimeoutSignal(timeoutMs: number) { }; } +/** Read the persisted startup state from disk, falling back cleanly on missing or invalid files. */ +function readPersistedStartupState(path: string): PersistedStartupState | null { + try { + const parsed = JSON.parse(readFileSync(path, "utf8")) as Partial; + if (typeof parsed !== "object" || parsed === null) { + return null; + } + + return { + version: typeof parsed.version === "number" ? parsed.version : STARTUP_STATE_VERSION, + lastSeenCliVersion: + typeof parsed.lastSeenCliVersion === "string" ? parsed.lastSeenCliVersion : undefined, + }; + } catch { + return null; + } +} + +/** Persist the current installed CLI version for future upgrade detection. */ +function writePersistedStartupState(path: string, installedVersion: string) { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync( + path, + JSON.stringify( + { + version: STARTUP_STATE_VERSION, + lastSeenCliVersion: installedVersion, + } satisfies PersistedStartupState, + null, + 2, + ), + { + encoding: "utf8", + mode: 0o600, + }, + ); +} + /** Return whether the transient startup notice should stay disabled for deterministic sessions like CI. */ -function startupUpdateNoticeDisabled() { - return process.env[DISABLE_STARTUP_UPDATE_NOTICE_ENV] === "1"; +function startupUpdateNoticeDisabled(env: NodeJS.ProcessEnv = process.env) { + return env[DISABLE_STARTUP_UPDATE_NOTICE_ENV] === "1"; } -/** Resolve the transient startup notice directly from npm dist-tags without persisted state. */ +/** Resolve the one-time copied-skill refresh notice shown after a version change. */ +function resolveStartupSkillRefreshNotice(deps: UpdateNoticeDeps = {}): UpdateNotice | null { + const resolveInstalledVersion = deps.resolveInstalledVersion ?? resolveCliVersion; + const installedVersion = resolveInstalledVersion(); + if (installedVersion === UNKNOWN_CLI_VERSION) { + return null; + } + + const statePath = deps.statePath ?? resolveHunkStatePath(deps.env ?? process.env); + if (!statePath) { + return null; + } + + const previousVersion = readPersistedStartupState(statePath)?.lastSeenCliVersion; + + try { + writePersistedStartupState(statePath, installedVersion); + } catch { + return null; + } + + if (!previousVersion || previousVersion === installedVersion) { + return null; + } + + return { + key: `skill:${installedVersion}`, + message: `Hunk ${installedVersion} installed • If your agent copied Hunk's skill, run hunk skill path`, + }; +} + +/** Resolve the transient startup notice directly from local state or npm dist-tags. */ export async function resolveStartupUpdateNotice( deps: UpdateNoticeDeps = {}, ): Promise { - if (startupUpdateNoticeDisabled()) { + const env = deps.env ?? process.env; + if (startupUpdateNoticeDisabled(env)) { return null; } + const skillRefreshNotice = resolveStartupSkillRefreshNotice(deps); + if (skillRefreshNotice) { + return skillRefreshNotice; + } + const fetchImpl = deps.fetchImpl ?? fetch; const fetchTimeoutMs = deps.fetchTimeoutMs ?? DEFAULT_UPDATE_NOTICE_FETCH_TIMEOUT_MS; const resolveInstalledVersion = deps.resolveInstalledVersion ?? resolveCliVersion; diff --git a/test/cli/entrypoint.test.ts b/test/cli/entrypoint.test.ts index d9f56bb..f1e74e5 100644 --- a/test/cli/entrypoint.test.ts +++ b/test/cli/entrypoint.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -47,6 +47,7 @@ describe("CLI entrypoint contracts", () => { expect(stdout).not.toContain("Examples:"); expect(stdout).toContain("hunk pager"); expect(stdout).toContain("hunk session "); + expect(stdout).toContain("hunk skill path"); expect(stdout).toContain("hunk mcp serve"); expect(stdout).not.toContain("hunk git"); expect(stdout).not.toContain("\u001b[?1049h"); @@ -109,6 +110,44 @@ describe("CLI entrypoint contracts", () => { expect(stdout).not.toContain("\u001b[?1049h"); }); + test("prints the bundled skill path for hunk skill path without terminal takeover sequences", () => { + const proc = Bun.spawnSync(["bun", "run", "src/main.tsx", "skill", "path"], { + cwd: process.cwd(), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + + const stdout = Buffer.from(proc.stdout).toString("utf8"); + const stderr = Buffer.from(proc.stderr).toString("utf8"); + const resolvedPath = stdout.trim(); + + expect(proc.exitCode).toBe(0); + expect(stderr).toBe(""); + expect(resolvedPath).toEndWith(join("skills", "hunk-review", "SKILL.md")); + expect(existsSync(resolvedPath)).toBe(true); + expect(stdout).not.toContain("\u001b[?1049h"); + }); + + test("bin wrapper prints the bundled skill path for hunk skill path", () => { + const proc = Bun.spawnSync(["node", "bin/hunk.cjs", "skill", "path"], { + cwd: process.cwd(), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + env: process.env, + }); + + const stdout = Buffer.from(proc.stdout).toString("utf8"); + const stderr = Buffer.from(proc.stderr).toString("utf8"); + const resolvedPath = stdout.trim(); + + expect(proc.exitCode).toBe(0); + expect(stderr).toBe(""); + expect(resolvedPath).toEndWith(join("skills", "hunk-review", "SKILL.md")); + expect(existsSync(resolvedPath)).toBe(true); + }); + test("general pager mode falls back to plain text for non-diff stdin", () => { const proc = Bun.spawnSync( [ diff --git a/test/smoke/tty.test.ts b/test/smoke/tty.test.ts index d91e53e..f4a0d6f 100644 --- a/test/smoke/tty.test.ts +++ b/test/smoke/tty.test.ts @@ -171,6 +171,7 @@ async function runTtySmoke(options: { ...process.env, TERM: "xterm-256color", HUNK_MCP_DISABLE: "1", + HUNK_DISABLE_UPDATE_NOTICE: "1", }, }); @@ -204,6 +205,7 @@ async function runStdinPagerSmoke(options?: { ...process.env, TERM: "xterm-256color", HUNK_MCP_DISABLE: "1", + HUNK_DISABLE_UPDATE_NOTICE: "1", }, }); From 415e3b0afbfe65507bfd8eecaf549cea29ebbdf5 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 10 Apr 2026 15:44:09 -0400 Subject: [PATCH 2/2] fix(cli): guard missing bundled skill in wrapper --- bin/hunk.cjs | 8 +++++++- test/cli/entrypoint.test.ts | 31 ++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/bin/hunk.cjs b/bin/hunk.cjs index 35e5a8e..8472817 100755 --- a/bin/hunk.cjs +++ b/bin/hunk.cjs @@ -100,7 +100,13 @@ function bundledBunRuntime() { const forwardedArgs = process.argv.slice(2); if (forwardedArgs.length === 2 && forwardedArgs[0] === "skill" && forwardedArgs[1] === "path") { - process.stdout.write(`${bundledSkillPath()}\n`); + const skillPath = bundledSkillPath(); + if (!fs.existsSync(skillPath)) { + console.error(`hunk: could not locate the bundled Hunk review skill at ${skillPath}`); + process.exit(1); + } + + process.stdout.write(`${skillPath}\n`); process.exit(0); } diff --git a/test/cli/entrypoint.test.ts b/test/cli/entrypoint.test.ts index f1e74e5..550efd9 100644 --- a/test/cli/entrypoint.test.ts +++ b/test/cli/entrypoint.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { existsSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { copyFileSync, existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -148,6 +148,35 @@ describe("CLI entrypoint contracts", () => { expect(existsSync(resolvedPath)).toBe(true); }); + test("bin wrapper fails clearly when the bundled skill is missing", () => { + const tempDir = mkdtempSync(join(tmpdir(), "hunk-wrapper-skill-missing-")); + const tempBinDir = join(tempDir, "bin"); + const tempWrapperPath = join(tempBinDir, "hunk.cjs"); + + try { + mkdirSync(tempBinDir, { recursive: true }); + copyFileSync(join(process.cwd(), "bin", "hunk.cjs"), tempWrapperPath); + + const proc = Bun.spawnSync(["node", tempWrapperPath, "skill", "path"], { + cwd: tempDir, + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + env: process.env, + }); + + const stdout = Buffer.from(proc.stdout).toString("utf8"); + const stderr = Buffer.from(proc.stderr).toString("utf8"); + + expect(proc.exitCode).toBe(1); + expect(stdout).toBe(""); + expect(stderr).toContain("hunk: could not locate the bundled Hunk review skill"); + expect(stderr).toContain(join("skills", "hunk-review", "SKILL.md")); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } + }); + test("general pager mode falls back to plain text for non-diff stdin", () => { const proc = Bun.spawnSync( [