diff --git a/.changeset/skills-check-command.md b/.changeset/skills-check-command.md new file mode 100644 index 0000000000..786ae8bc3a --- /dev/null +++ b/.changeset/skills-check-command.md @@ -0,0 +1,5 @@ +--- +"@shopify/hydrogen": minor +--- + +Add `hydrogen skills check`, which exits non-zero when the project's synced skills do not match the installed `@shopify/hydrogen`, or were never synced. It prints the summary `skills sync` would act on and writes nothing. The default `--mode=error` gates CI; `--mode=warn` prints the same message and exits zero for dev scripts. diff --git a/.docs/dependencies.md b/.docs/dependencies.md index fd0aa4a68b..4b13878b56 100644 --- a/.docs/dependencies.md +++ b/.docs/dependencies.md @@ -32,7 +32,7 @@ When changing `@shopify/hydrogen/vite` local HTTPS behaviour, update these toget - `templates/react-router/vite.config.ts` - framework example `dev:https` scripts and configs under `examples/*` -`scripts/preview-template-dist.ts` syncs `packages/hydrogen/skills` into template `.agents/skills` and `.claude/skills` when preparing the dist branch, so template source directories should not duplicate those generated skill copies. +`scripts/preview-template-dist.ts` syncs `packages/hydrogen/skills` into each template's harness skill directories (`.claude/skills` and `.agents/skills`) when preparing the dist branch, so template source directories should not duplicate those generated skill copies. ## Hydrogen Skills Sync @@ -40,5 +40,6 @@ When changing how synced skills are stamped or verified (the frontmatter `metada - `packages/hydrogen/src/cli/skills.ts` (single writer and reader of the metadata block) - `scripts/preview-template-dist.ts` (imports `syncSkills` so template copies carry the same metadata) +- `hydrogen skills check` in the same file (reads `getSkillsSyncStatus`, prints `describeSkillsSyncStatus`) - `packages/hydrogen/README.md` ("Keeping skills in sync") - root `README.md` ("Set up in your own project") diff --git a/README.md b/README.md index ec36efa0d9..34fcfea195 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ Then add Hydrogen from your project directory: npx @shopify/hydrogen@preview setup ``` -`setup` installs `@shopify/hydrogen` with your project's package manager and copies Hydrogen's agent skills into your project's skills directory — matched to the version you just installed. After upgrading Hydrogen later, run `npx @shopify/hydrogen skills sync` to bring the skills back in line (see the [package README](packages/hydrogen/README.md#keeping-skills-in-sync)). +`setup` installs `@shopify/hydrogen` with your project's package manager and copies Hydrogen's agent skills into your project's skills directory — matched to the version you just installed. After upgrading Hydrogen later, run `npx @shopify/hydrogen skills sync` to bring the skills back in line, and `npx @shopify/hydrogen skills check` to verify they are (it exits non-zero when they are not, so it can run in CI). See the [package README](packages/hydrogen/README.md#keeping-skills-in-sync). Now ask your coding agent to build the storefront: diff --git a/packages/hydrogen/README.md b/packages/hydrogen/README.md index b5cc1ac933..d0fa7d622f 100644 --- a/packages/hydrogen/README.md +++ b/packages/hydrogen/README.md @@ -31,6 +31,18 @@ Each synced `SKILL.md` records the package version and a content hash in its fro `--force` discards local state in favour of the package: it overwrites edited and colliding skills and removes edited stale ones. +Gate CI on the same comparison. The command exits non-zero when a sync would change anything, or when skills were never synced: + +```bash +npx @shopify/hydrogen skills check +``` + +For dev scripts, `--mode=warn` prints the same message and exits zero so the next command still runs, and prints nothing at all when skills are current: + +```json +{ "scripts": { "dev": "hydrogen skills check --mode=warn && vite dev" } } +``` + ## GraphQL Tooling Enable Storefront and Customer Account API editor diagnostics with Hydrogen's packed TypeScript plugin: diff --git a/packages/hydrogen/skills/hydrogen-setup/steps/11-verify.md b/packages/hydrogen/skills/hydrogen-setup/steps/11-verify.md index 74f5e292d1..c6438330dd 100644 --- a/packages/hydrogen/skills/hydrogen-setup/steps/11-verify.md +++ b/packages/hydrogen/skills/hydrogen-setup/steps/11-verify.md @@ -20,6 +20,7 @@ If Playwright is present, run it headless. - [ ] Every applicable script was executed in this session, after the last code change, and passed - [ ] `hydrogen gql check` ran as part of static checks (not skipped) +- [ ] `dev` still starts with `hydrogen skills check --mode=warn` and prints no skills warning ## Run Runtime Smoke Tests diff --git a/packages/hydrogen/skills/hydrogen-setup/steps/2-scaffold.md b/packages/hydrogen/skills/hydrogen-setup/steps/2-scaffold.md index 6c9db39520..d64beff729 100644 --- a/packages/hydrogen/skills/hydrogen-setup/steps/2-scaffold.md +++ b/packages/hydrogen/skills/hydrogen-setup/steps/2-scaffold.md @@ -18,6 +18,25 @@ Invoke the `hydrogen-routing` skill and create the shared route template manifes - [ ] Route templates are set up +## Keep Skills In Sync + +The packaged skills describe the installed Hydrogen version. Chain `hydrogen skills check --mode=warn` in front of the app's `dev` script so a later `@shopify/hydrogen` bump prints the resync command the next time the dev server starts, without blocking it: + +```json +{ + "scripts": { + "dev": "hydrogen skills check --mode=warn && " + } +} +``` + +Keep the framework's own dev command exactly as it was; only prefix it. Apply the same prefix to any sibling dev script such as `dev:https`. Do not add the plain `hydrogen skills check` (error mode) to `dev`; that belongs in CI, where a stale skill copy should fail the build. + +### Continue when + +- [ ] `dev` runs `hydrogen skills check --mode=warn` before the framework dev command +- [ ] Running the `dev` script prints no skills warning (skills were synced by `hydrogen setup`) + ## Use Standard Environment Names Use these canonical environment variable names throughout the app (kept in sync with `hydrogen-storefront-client`): diff --git a/packages/hydrogen/src/cli/__tests__/setup.test.ts b/packages/hydrogen/src/cli/__tests__/setup.test.ts index 634695591f..f6fbdf9672 100644 --- a/packages/hydrogen/src/cli/__tests__/setup.test.ts +++ b/packages/hydrogen/src/cli/__tests__/setup.test.ts @@ -222,7 +222,7 @@ describe("setupHydrogen", () => { }); await setupHydrogen({ - args: ["--force"], + force: true, cwd: appRoot, packageRoot, runCommand: createRunCommandSpy(), diff --git a/packages/hydrogen/src/cli/__tests__/skills.test.ts b/packages/hydrogen/src/cli/__tests__/skills.test.ts index 3acc4c71e9..a80cdf5dd2 100644 --- a/packages/hydrogen/src/cli/__tests__/skills.test.ts +++ b/packages/hydrogen/src/cli/__tests__/skills.test.ts @@ -15,7 +15,17 @@ import { describe, expect, it, vi } from "vitest"; import { assert } from "../../core/test-utils"; import { isObjectRecord } from "../../core/utils/record"; -import { syncSkills, type SyncSkillsResult, type SyncSkillsRootResult } from "../skills"; +import { + checkSkills, + describeSkillsSyncStatus, + parseSkillsCheckArgs, + getSkillsSyncStatus, + parseSkillsSyncArgs, + syncSkills, + type SkillsSyncStatus, + type SyncSkillsResult, + type SyncSkillsRootResult, +} from "../skills"; const REAL_SKILLS_ROOT = join(import.meta.dirname, "../../../skills"); @@ -93,12 +103,21 @@ function readMetadata(content: string): { version: string; hash: string } { return { version, hash }; } +function statusFixture(overrides: Partial = {}): SkillsSyncStatus { + return { + version: "2026.2.0", + pending: { add: 0, update: 0, remove: 0, modified: 0 }, + conflicts: [], + ...overrides, + }; +} + /** Tests that expect a prompt pass their own confirm; everything else must never ask. */ const rejectPrompt = (question: string): Promise => Promise.reject(new Error(`Unexpected prompt: ${question}`)); -function sync(appRoot: string, packageRoot: string, args: string[] = []) { - return syncSkills({ cwd: appRoot, packageRoot, args, log: vi.fn(), confirm: rejectPrompt }); +function sync(appRoot: string, packageRoot: string, force = false) { + return syncSkills({ cwd: appRoot, packageRoot, force, log: vi.fn(), confirm: rejectPrompt }); } function rootResult(result: SyncSkillsResult, harness: string): SyncSkillsRootResult { @@ -181,7 +200,7 @@ describe("syncSkills", () => { }); expect(readSkill(appRoot, "hydrogen-cart-ui")).toContain("My local note."); - const forced = await sync(appRoot, newPackageRoot, ["--force"]); + const forced = await sync(appRoot, newPackageRoot, true); expect(agents(forced)).toMatchObject({ updated: 1, skipped: [] }); expect(readSkill(appRoot, "hydrogen-cart-ui")).toContain("New."); expect(readSkill(appRoot, "hydrogen-cart-ui")).not.toContain("My local note."); @@ -209,7 +228,7 @@ describe("syncSkills", () => { const skillFile = join(appRoot, ".agents/skills/hydrogen-cart-ui/SKILL.md"); writeFileSync(skillFile, readFileSync(skillFile, "utf8") + "My local note.\n"); - const result = await sync(appRoot, packageRoot, ["--force"]); + const result = await sync(appRoot, packageRoot, true); expect(agents(result)).toMatchObject({ updated: 1, unchanged: 0, skipped: [] }); expect(readSkill(appRoot, "hydrogen-cart-ui")).not.toContain("My local note."); @@ -394,7 +413,7 @@ describe("syncSkills", () => { it("removes without asking when forced", async () => { const { appRoot, skillFile, newPackageRoot } = await createEditedStaleSkill(); - const result = await sync(appRoot, newPackageRoot, ["--force"]); + const result = await sync(appRoot, newPackageRoot, true); expect(agents(result)).toMatchObject({ removed: 1, kept: [], skipped: [] }); expect(existsSync(skillFile)).toBe(false); @@ -425,7 +444,7 @@ describe("syncSkills", () => { expect(existsSync(join(appRoot, ".agents/skills/hydrogen-setup"))).toBe(false); expect(readSkill(appRoot, "hydrogen-cart-ui")).toContain("Handwritten."); - expect(agents(await sync(appRoot, packageRoot, ["--force"]))).toMatchObject({ + expect(agents(await sync(appRoot, packageRoot, true))).toMatchObject({ added: 1, updated: 1, }); @@ -472,11 +491,10 @@ describe("syncSkills", () => { expect(existsSync(join(appRoot, ".claude/skills"))).toBe(false); }); - it("rejects unknown arguments", async () => { - const appRoot = createAppRoot(); - const packageRoot = createPackageRoot("2026.1.0"); - - await expect(sync(appRoot, packageRoot, ["--yolo"])).rejects.toThrow(); + it("parses --force and rejects unknown flags", () => { + expect(parseSkillsSyncArgs([])).toEqual({ force: false }); + expect(parseSkillsSyncArgs(["--force"])).toEqual({ force: true }); + expect(() => parseSkillsSyncArgs(["--yolo"])).toThrow("--yolo"); }); describe("package resolution", () => { @@ -565,4 +583,204 @@ describe("syncSkills", () => { readMetadata(readSkill(appRoot, skillName)); } }); + + describe("getSkillsSyncStatus", () => { + it("reports everything pending for a project that never synced", () => { + const appRoot = createAppRoot(); + const packageRoot = createPackageRoot("2026.1.0", { "hydrogen-cart-ui": "Cart.\n" }); + + const status = getSkillsSyncStatus({ cwd: appRoot, packageRoot }); + + expect(status).toEqual({ + version: "2026.1.0", + pending: { add: 1, update: 0, remove: 0, modified: 0 }, + conflicts: [], + }); + }); + + it("reports nothing pending right after a sync", async () => { + const appRoot = createAppRoot(); + const packageRoot = createPackageRoot("2026.1.0", { "hydrogen-cart-ui": "Cart.\n" }); + await sync(appRoot, packageRoot); + + const status = getSkillsSyncStatus({ cwd: appRoot, packageRoot }); + + expect(status.pending).toEqual({ add: 0, update: 0, remove: 0, modified: 0 }); + }); + + it("counts updates, additions, removals, and local modifications after an upgrade", async () => { + const appRoot = createAppRoot(); + await sync( + appRoot, + createPackageRoot("2026.1.0", { + "hydrogen-cart-ui": "Cart.\n", + "hydrogen-legacy": "Legacy.\n", + "hydrogen-money": "Money.\n", + }), + ); + for (const harness of [".claude", ".agents"]) { + const moneyFile = join(appRoot, harness, "skills/hydrogen-money/SKILL.md"); + writeFileSync(moneyFile, readFileSync(moneyFile, "utf8") + "Mine.\n"); + } + const newPackageRoot = createPackageRoot("2026.2.0", { + "hydrogen-cart-ui": "Cart v2.\n", + "hydrogen-money": "Money v2.\n", + "hydrogen-image": "Image.\n", + }); + + const status = getSkillsSyncStatus({ cwd: appRoot, packageRoot: newPackageRoot }); + + expect(status).toEqual({ + version: "2026.2.0", + pending: { add: 1, update: 1, remove: 1, modified: 1 }, + conflicts: [], + }); + expect(readSkill(appRoot, "hydrogen-cart-ui")).toContain("Cart.\n"); + }); + + it("counts an edited skill the package no longer ships as locally modified", async () => { + const appRoot = createAppRoot(); + await sync(appRoot, createPackageRoot("2026.1.0", { "hydrogen-legacy": "Legacy.\n" })); + const skillFile = join(appRoot, ".agents/skills/hydrogen-legacy/SKILL.md"); + writeFileSync(skillFile, readFileSync(skillFile, "utf8") + "Mine.\n"); + + const status = getSkillsSyncStatus({ + cwd: appRoot, + packageRoot: createPackageRoot("2026.2.0"), + }); + + // The .claude copy is untouched and would be removed; the edited .agents copy waits for consent. + expect(status.pending).toEqual({ add: 0, update: 0, remove: 1, modified: 1 }); + }); + + it("surfaces name collisions without throwing", () => { + const appRoot = createAppRoot(); + writeSkill(join(appRoot, ".agents/skills"), "hydrogen-cart-ui", "Handwritten.\n"); + const packageRoot = createPackageRoot("2026.1.0", { "hydrogen-cart-ui": "Cart.\n" }); + + const status = getSkillsSyncStatus({ cwd: appRoot, packageRoot }); + + expect(status.conflicts).toEqual([join(appRoot, ".agents/skills/hydrogen-cart-ui")]); + }); + }); + + describe("describeSkillsSyncStatus", () => { + it("returns nothing when everything is current", () => { + expect(describeSkillsSyncStatus(statusFixture())).toBeUndefined(); + }); + + it("lists pending updates, additions, and removals with the sync command", () => { + expect( + describeSkillsSyncStatus( + statusFixture({ pending: { add: 2, update: 5, remove: 1, modified: 0 } }), + ), + ).toBe( + "Hydrogen skills are out of date with @shopify/hydrogen 2026.2.0 (5 to update, 2 new, 1 removed upstream). Run `npx @shopify/hydrogen skills sync`.", + ); + }); + + it("points at --force when only locally modified skills are behind", () => { + expect( + describeSkillsSyncStatus( + statusFixture({ pending: { add: 0, update: 0, remove: 0, modified: 2 } }), + ), + ).toBe( + "Hydrogen skills are out of date with @shopify/hydrogen 2026.2.0 (2 locally modified). Run `npx @shopify/hydrogen skills sync --force` to reset them.", + ); + }); + + it("reports locally modified skills alongside other pending changes", () => { + expect( + describeSkillsSyncStatus( + statusFixture({ pending: { add: 1, update: 3, remove: 0, modified: 2 } }), + ), + ).toBe( + "Hydrogen skills are out of date with @shopify/hydrogen 2026.2.0 (3 to update, 1 new, 2 locally modified). Run `npx @shopify/hydrogen skills sync`, or `npx @shopify/hydrogen skills sync --force` to also reset the locally modified ones.", + ); + }); + + it("explains collisions", () => { + const message = describeSkillsSyncStatus( + statusFixture({ conflicts: ["/app/.agents/skills/x"] }), + ); + + expect(message).toContain("/app/.agents/skills/x"); + expect(message).toContain("--force"); + }); + }); + + describe("checkSkills", () => { + it("passes and reports the version when skills are current", async () => { + const appRoot = createAppRoot(); + const packageRoot = createPackageRoot("2026.1.0", { "hydrogen-cart-ui": "Cart.\n" }); + await sync(appRoot, packageRoot); + const log = vi.fn(); + + checkSkills({ cwd: appRoot, packageRoot, log }); + + expect(log).toHaveBeenCalledWith( + "Hydrogen skills are up to date with @shopify/hydrogen 2026.1.0.", + ); + }); + + it("fails when the package moved on", async () => { + const appRoot = createAppRoot(); + await sync(appRoot, createPackageRoot("2026.1.0", { "hydrogen-cart-ui": "Cart.\n" })); + const newPackageRoot = createPackageRoot("2026.2.0", { "hydrogen-cart-ui": "Cart v2.\n" }); + + expect(() => + checkSkills({ cwd: appRoot, packageRoot: newPackageRoot, log: vi.fn() }), + ).toThrow("out of date with @shopify/hydrogen 2026.2.0 (1 to update)"); + }); + + it("fails when skills were never synced", () => { + const appRoot = createAppRoot(); + const packageRoot = createPackageRoot("2026.1.0", { "hydrogen-cart-ui": "Cart.\n" }); + + expect(() => checkSkills({ cwd: appRoot, packageRoot, log: vi.fn() })).toThrow("1 new"); + }); + + it("warns and returns instead of throwing in warn mode", () => { + const appRoot = createAppRoot(); + const packageRoot = createPackageRoot("2026.1.0", { "hydrogen-cart-ui": "Cart.\n" }); + const log = vi.fn(); + const warn = vi.fn(); + + checkSkills({ cwd: appRoot, packageRoot, mode: "warn", log, warn }); + + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0]?.[0]).toContain("1 new"); + expect(log).not.toHaveBeenCalled(); + }); + + it("stays silent in warn mode when skills are current", async () => { + const appRoot = createAppRoot(); + const packageRoot = createPackageRoot("2026.1.0", { "hydrogen-cart-ui": "Cart.\n" }); + await sync(appRoot, packageRoot); + const log = vi.fn(); + const warn = vi.fn(); + + checkSkills({ cwd: appRoot, packageRoot, mode: "warn", log, warn }); + + expect(log).not.toHaveBeenCalled(); + expect(warn).not.toHaveBeenCalled(); + }); + + it("parses --mode and rejects unknown values", () => { + expect(parseSkillsCheckArgs([])).toEqual({ mode: "error" }); + expect(parseSkillsCheckArgs(["--mode=warn"])).toEqual({ mode: "warn" }); + expect(parseSkillsCheckArgs(["--mode", "error"])).toEqual({ mode: "error" }); + expect(() => parseSkillsCheckArgs(["--mode=loud"])).toThrow("Unknown --mode 'loud'"); + expect(() => parseSkillsCheckArgs(["--yolo"])).toThrow("--yolo"); + }); + + it("never writes", () => { + const appRoot = createAppRoot(); + const packageRoot = createPackageRoot("2026.1.0", { "hydrogen-cart-ui": "Cart.\n" }); + + expect(() => checkSkills({ cwd: appRoot, packageRoot, log: vi.fn() })).toThrow(); + expect(existsSync(join(appRoot, ".agents/skills"))).toBe(false); + expect(existsSync(join(appRoot, ".claude"))).toBe(false); + }); + }); }); diff --git a/packages/hydrogen/src/cli/index.ts b/packages/hydrogen/src/cli/index.ts index 61c93f1a21..469d82733f 100644 --- a/packages/hydrogen/src/cli/index.ts +++ b/packages/hydrogen/src/cli/index.ts @@ -6,7 +6,7 @@ import { fileURLToPath } from "node:url"; import { installLocalHttpsCertificates, uninstallLocalHttpsCertificates } from "./certs"; import { checkGraphQL } from "./gql"; import { setupHydrogen } from "./setup"; -import { syncSkills } from "./skills"; +import { checkSkills, parseSkillsCheckArgs, parseSkillsSyncArgs, syncSkills } from "./skills"; const CLI_ARGUMENTS_INDEX = 2; const FAILURE_EXIT_CODE = 1; @@ -17,11 +17,17 @@ const COMMANDS = [ path: ["certs", "uninstall"], run: async (args: string[]) => uninstallLocalHttpsCertificates(args), }, - { path: ["setup"], run: async (args: string[]) => setupHydrogen({ args }) }, + { path: ["setup"], run: async (args: string[]) => setupHydrogen(parseSkillsSyncArgs(args)) }, + { + path: ["skills", "check"], + run: async (args: string[]) => { + checkSkills(parseSkillsCheckArgs(args)); + }, + }, { path: ["skills", "sync"], run: async (args: string[]) => { - await syncSkills({ args }); + await syncSkills(parseSkillsSyncArgs(args)); }, }, { path: ["gql", "check"], run: async (args: string[]) => checkGraphQL({ args }) }, diff --git a/packages/hydrogen/src/cli/setup.ts b/packages/hydrogen/src/cli/setup.ts index f115819691..0e663f9b6b 100644 --- a/packages/hydrogen/src/cli/setup.ts +++ b/packages/hydrogen/src/cli/setup.ts @@ -41,7 +41,7 @@ export type RunCommand = ( interface SetupHydrogenOptions { /** Forwarded to `skills sync`, so `hydrogen setup --force` overwrites like `hydrogen skills sync --force`. */ - args?: string[]; + force?: boolean; cwd?: string; packageRoot?: string; env?: Record; @@ -149,5 +149,5 @@ export async function setupHydrogen(options: SetupHydrogenOptions = {}): Promise await installHydrogen(appRoot, packageManager, runCommand); } - await syncSkills({ args: options.args, cwd: appRoot, packageRoot: options.packageRoot, log }); + await syncSkills({ force: options.force, cwd: appRoot, packageRoot: options.packageRoot, log }); } diff --git a/packages/hydrogen/src/cli/skills.ts b/packages/hydrogen/src/cli/skills.ts index ccbad4ecb9..45adc67c9f 100644 --- a/packages/hydrogen/src/cli/skills.ts +++ b/packages/hydrogen/src/cli/skills.ts @@ -109,7 +109,8 @@ export interface SyncSkillsResult { } export interface SyncSkillsOptions { - args?: string[]; + /** Overwrite locally modified or unmanaged colliding skills and remove modified stale ones. */ + force?: boolean; cwd?: string; packageRoot?: string; log?: (message: string) => void; @@ -456,7 +457,7 @@ function executePlan(destinationRoot: string, planned: PlannedSkill[]): SyncSkil return result; } -function parseSyncArgs(args: string[]): { force: boolean } { +export function parseSkillsSyncArgs(args: string[]): { force: boolean } { const { values } = parseArgs({ args, options: { force: { type: "boolean", default: false } }, @@ -466,28 +467,169 @@ function parseSyncArgs(args: string[]): { force: boolean } { return { force: values.force }; } -export async function syncSkills(options: SyncSkillsOptions = {}): Promise { - const appRoot = options.cwd ?? process.cwd(); - const log = options.log ?? console.log; - const confirm = options.confirm ?? confirmByDefault; - const { force } = parseSyncArgs(options.args ?? []); - const packageRoot = options.packageRoot ?? getInstalledPackageRoot(appRoot) ?? getPackageRoot(); - const sourceSkillsRoot = join(packageRoot, SKILLS_DIRECTORY_NAME); +interface SyncPlan { + version: string; + destinations: DestinationPlan[]; +} + +function planSkillsSync( + appRoot: string, + packageRoot: string | undefined, + force: boolean, +): SyncPlan { + const resolvedPackageRoot = packageRoot ?? getInstalledPackageRoot(appRoot) ?? getPackageRoot(); + const sourceSkillsRoot = join(resolvedPackageRoot, SKILLS_DIRECTORY_NAME); assertDirectory(sourceSkillsRoot, `No packaged skills found at ${sourceSkillsRoot}.`); - const version = readPackageVersion(packageRoot); + const version = readPackageVersion(resolvedPackageRoot); const shippedSkills = new Map( listDirectoryNames(sourceSkillsRoot).map((skillName) => [ skillName, readShippedSkill(sourceSkillsRoot, skillName, version), ]), ); - const destinationRoots = getSkillsDestinationRoots(appRoot); - const plans = destinationRoots.map((destinationRoot) => + const destinations = getSkillsDestinationRoots(appRoot).map((destinationRoot) => planDestination(destinationRoot, shippedSkills, force), ); - const conflicts = plans.flatMap((plan) => plan.conflicts); + return { version, destinations }; +} + +export interface SkillsSyncStatus { + /** Version of the installed `@shopify/hydrogen` the skills are compared against. */ + version: string; + /** Distinct skills a `hydrogen skills sync` run would touch right now. */ + pending: { add: number; update: number; remove: number; modified: number }; + /** Directories Hydrogen did not create that collide with shipped skill names. */ + conflicts: string[]; +} + +/** Reports what a sync would change without touching the filesystem. */ +export function getSkillsSyncStatus( + options: Pick = {}, +): SkillsSyncStatus { + const plan = planSkillsSync(options.cwd ?? process.cwd(), options.packageRoot, false); + // Count skills, not directories: the same skill sits in every harness root. + const names = { + add: new Set(), + update: new Set(), + remove: new Set(), + modified: new Set(), + }; + for (const destination of plan.destinations) { + for (const skill of destination.planned) { + if (skill.action === "add" || skill.action === "update" || skill.action === "remove") { + names[skill.action].add(skill.skillName); + } else if (skill.action === "skip") { + names.modified.add(skill.skillName); + } + } + // Edited skills the package no longer ships wait for consent in `sync`; + // read-only they are simply local edits that a sync would not clear. + for (const skillName of destination.orphans) names.modified.add(skillName); + } + + return { + version: plan.version, + pending: { + add: names.add.size, + update: names.update.size, + remove: names.remove.size, + modified: names.modified.size, + }, + conflicts: plan.destinations.flatMap((destination) => destination.conflicts), + }; +} + +const SYNC_COMMAND = "npx @shopify/hydrogen skills sync"; + +/** A plain sync clears everything except local edits, which only `--force` resets. */ +function describeNextStep(pending: SkillsSyncStatus["pending"]): string { + const syncClears = pending.add + pending.update + pending.remove > 0; + if (pending.modified === 0) return `Run \`${SYNC_COMMAND}\`.`; + if (!syncClears) return `Run \`${SYNC_COMMAND} --force\` to reset them.`; + return `Run \`${SYNC_COMMAND}\`, or \`${SYNC_COMMAND} --force\` to also reset the locally modified ones.`; +} + +/** Explains why a status is not up to date, or returns undefined when nothing needs doing. */ +export function describeSkillsSyncStatus(status: SkillsSyncStatus): string | undefined { + if (status.conflicts.length > 0) { + return `Hydrogen skills cannot be synced: ${status.conflicts.join(", ")} were not created by Hydrogen. Remove them or run \`${SYNC_COMMAND} --force\`.`; + } + + const { pending } = status; + const parts = [ + pending.update > 0 && `${pending.update} to update`, + pending.add > 0 && `${pending.add} new`, + pending.remove > 0 && `${pending.remove} removed upstream`, + pending.modified > 0 && `${pending.modified} locally modified`, + ].filter((part): part is string => typeof part === "string"); + if (parts.length === 0) return undefined; + + return `Hydrogen skills are out of date with @shopify/hydrogen ${status.version} (${parts.join(", ")}). ${describeNextStep(pending)}`; +} + +const CHECK_MODES = ["error", "warn"] as const; + +export type CheckSkillsMode = (typeof CHECK_MODES)[number]; + +export interface CheckSkillsOptions extends Pick { + /** `error` throws on drift for CI; `warn` reports it and continues for dev scripts. */ + mode?: CheckSkillsMode; + warn?: (message: string) => void; +} + +function isCheckSkillsMode(value: string): value is CheckSkillsMode { + return CHECK_MODES.some((mode) => mode === value); +} + +export function parseSkillsCheckArgs(args: string[]): { mode: CheckSkillsMode } { + const { values } = parseArgs({ + args, + options: { mode: { type: "string", default: "error" } }, + strict: true, + }); + + if (!isCheckSkillsMode(values.mode)) { + throw new Error(`Unknown --mode '${values.mode}'. Expected one of: ${CHECK_MODES.join(", ")}.`); + } + + return { mode: values.mode }; +} + +/** + * Reports whether the synced skills match the installed package without writing. + * A project with no synced skills counts as drift; running this command is the opt-in. + */ +export function checkSkills(options: CheckSkillsOptions = {}): void { + const mode = options.mode ?? "error"; + const log = options.log ?? console.log; + const warn = options.warn ?? console.warn; + const status = getSkillsSyncStatus(options); + const problem = describeSkillsSyncStatus(status); + + if (problem) { + if (mode === "error") throw new Error(problem); + warn(problem); + return; + } + + // Warn mode prefixes dev scripts, so it only speaks when something needs doing. + if (mode === "error") { + log(`Hydrogen skills are up to date with @shopify/hydrogen ${status.version}.`); + } +} + +export async function syncSkills(options: SyncSkillsOptions = {}): Promise { + const log = options.log ?? console.log; + const confirm = options.confirm ?? confirmByDefault; + const plan = planSkillsSync( + options.cwd ?? process.cwd(), + options.packageRoot, + options.force ?? false, + ); + + const conflicts = plan.destinations.flatMap((destination) => destination.conflicts); if (conflicts.length > 0) { throw new Error( `Skill directories exist that Hydrogen did not create: ${conflicts.join(", ")}. Remove them or rerun with --force to overwrite.`, @@ -496,12 +638,14 @@ export async function syncSkills(options: SyncSkillsOptions = {}): Promise executePlan(plan.destinationRoot, plan.planned)); + const roots = plan.destinations.map((destination) => + executePlan(destination.destinationRoot, destination.planned), + ); for (const root of roots) { log( - `Synced Hydrogen ${version} skills to ${root.root}: ${root.added} added, ${root.updated} updated, ${root.unchanged} unchanged, ${root.removed} removed.`, + `Synced Hydrogen ${plan.version} skills to ${root.root}: ${root.added} added, ${root.updated} updated, ${root.unchanged} unchanged, ${root.removed} removed.`, ); } @@ -517,7 +661,7 @@ export async function syncSkills(options: SyncSkillsOptions = {}): Promise ` - ${skillRoot}`), "Delete them yourself, or rerun with --force to remove them.", "", @@ -525,5 +669,5 @@ export async function syncSkills(options: SyncSkillsOptions = {}): Promise