diff --git a/.github/workflows/release-drift.yml b/.github/workflows/release-drift.yml new file mode 100644 index 0000000..5d5b84a --- /dev/null +++ b/.github/workflows/release-drift.yml @@ -0,0 +1,38 @@ +name: Release Drift + +# Guards against source drift: code merged to main that never gets released, so the +# published @agent-ix/quoin silently lags its source. Fails while main's src/ is ahead +# of the latest release tag. Self-healing — it re-evaluates on tag push (goes green once +# you tag the release) and daily, so it never gets stuck red on an old commit. + +on: + push: + branches: [main] + tags: ["*.*.*"] + schedule: + - cron: "0 13 * * *" + workflow_dispatch: + +jobs: + drift: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 # full history + tags so `git describe` works + - name: Fail if src/ changed since the latest release tag + run: | + set -euo pipefail + LAST_TAG=$(git describe --tags --abbrev=0 2>/dev/null || echo "") + if [ -z "$LAST_TAG" ]; then + echo "No release tags yet; nothing to compare against." + exit 0 + fi + if git diff --quiet "$LAST_TAG"..HEAD -- src; then + echo "✅ No src/ changes since $LAST_TAG — main is released." + else + echo "::error::src/ has changed since $LAST_TAG. Tag a release (vX.Y.Z) so the published package matches main." + echo "Changed since $LAST_TAG:" + git diff --name-only "$LAST_TAG"..HEAD -- src + exit 1 + fi diff --git a/reviews/26-06-26-gap-analysis-pr21.md b/reviews/26-06-26-gap-analysis-pr21.md new file mode 100644 index 0000000..85ee798 --- /dev/null +++ b/reviews/26-06-26-gap-analysis-pr21.md @@ -0,0 +1,69 @@ +--- +id: SR-001 +title: "Gap-analysis review of the update-registry + versioning change (Task-010)" +type: SpecReview +analysis: gap-analysis +scope: "spec/plan.md PL-001 Task-010; FR-002; FR-022; spec/matrix.md TM-001" +review_set: subset +relationships: + - target: "ix://agent-ix/quoin/PL-001" + type: "reviews" + - target: "ix://agent-ix/quoin/TM-001" + type: "references" +--- + +## Summary + +Post-implementation gate over the FR-022 self-update registry default plus the +truthful-versioning and release-drift work (PL-001 Task-010, PR #21). The plan is +complete and FR-022 is faithfully implemented and matrix-backed. The gate found one +`high` spec-faithfulness gap (version resolution changed without updating FR-002) and +one `medium` untracked test — **both remediated within PR #21** (see Remediation). One +`low` note (the release-drift CI guard) is accepted as out-of-scope process tooling. + +## Verdict + +**FAIL at review time → remediated to PASS in PR #21.** FND-001 (high) and FND-002 +(medium) are fixed in the same PR; FND-003 (low) is accepted as out-of-scope. The +Findings table below records the gate's original findings for the audit trail. + +## Remediation (applied in PR #21) + +- **FND-001:** FR-002 Description, Behavior, and a new **FR-002-AC-4** now specify + "prefer the build-time baked `git describe` version; fall back to `package.json`." +- **FND-002:** `tests/version.test.ts` added to the FR-002 Test Matrix row. +- **FND-003:** accepted — the release-drift guard is CI/release tooling, intentionally + not modeled as a product requirement. + +## Findings + +| ID | Severity | Summary | Refs | +| ------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | +| FND-001 | high | `packageVersion()` now prefers a build-time baked `git describe` value; FR-002 mandates reading from `package.json` and never describes baked/drift versioning — spec-faithfulness gap + underspecified feature | FR-002 | +| FND-002 | medium | `tests/version.test.ts` ("resolveVersion") is not registered in the Test Matrix FR-002 row — untracked test | TM-001 | +| FND-003 | low | `.github/workflows/release-drift.yml` CI guard has no owning StR/US/FR/NFR (release-process tooling) | - | + +## Coverage + +- **Plan completion (Step 1): PASS.** PL-001 Task-001…Task-010 are all `Done`; no + blocked/incomplete tasks or stale checkboxes. +- **Matrix verification (Step 2): partial.** FR-022 is backed by `update.test.ts` + (4 cases; names match the matrix row, incl. the new public-npm default case). quoin + maps requirement→test by `file :: "test name"` rather than `FR-xxx-AC-x` code tags, so + "backing" = a named test exists. Gap: `tests/version.test.ts` exists but is absent from + the matrix (FND-002). +- **Underspecified code (Step 3):** the baked-version path (`resolveVersion`, + `__QUOIN_VERSION__`, `vite.config.ts` `gitVersion`) changes FR-002 behavior with no + owning/updated requirement (FND-001); the release-drift guard is unowned process tooling + (FND-003). The FR-022 registry default and `DEFAULT_UPDATE_REGISTRY` are correctly owned + by FR-022. +- **Semantic review (Step 4): not run** (optional; pending user opt-in). + +### Remediation to clear FAIL → PASS + +1. Update **FR-002** (Description/Behavior + an AC) to specify: resolve the version from the + build-time baked `git describe` value when present, falling back to `package.json`; + include the new test in `update`/`version` AC verification. +2. Add `tests/version.test.ts` to the **FR-002** matrix row. +3. Optionally introduce an NFR (or note) covering the release-drift guard, or record it as + intentionally out of spec scope. diff --git a/spec/functional/FR-002-print-package-version.md b/spec/functional/FR-002-print-package-version.md index 27ffce4..2b1936a 100644 --- a/spec/functional/FR-002-print-package-version.md +++ b/spec/functional/FR-002-print-package-version.md @@ -11,10 +11,13 @@ relationships: ## Description -The CLI SHALL print its own package version — read from the package's -`package.json` — in response to the `version` command, the `--version` flag, or -the `-v` flag, and SHALL raise an error when the package version is missing or -not a string. +The CLI SHALL print its own version in response to the `version` command, the +`--version` flag, or the `-v` flag. The version SHALL be the value baked at build +time from `git describe` — a bare tag for a clean release (e.g. `0.5.2`), a +drift-revealing suffix otherwise (e.g. `0.5.1-3-gabc123-dirty`) — and SHALL fall +back to the package's `package.json` `version` field when no build-time value is +present (dev, test, or no-git builds), raising an error when that fallback version +is missing or not a string. ## Inputs @@ -27,19 +30,22 @@ not a string. ## Behavior -- The CLI SHALL resolve its `package.json` relative to the installed package and - read the `version` field. -- The CLI SHALL print that version and return before any other dispatch. -- The CLI SHALL raise an explicit error when the `version` field is absent or is - not a string. +- The CLI SHALL prefer the build-time baked version (`git describe --tags --dirty`, + leading `v` stripped) when it is a non-empty string. +- The CLI SHALL otherwise resolve its `package.json` relative to the installed + package and read the `version` field. +- The CLI SHALL print the resolved version and return before any other dispatch. +- The CLI SHALL raise an explicit error when, on the fallback path, the `version` + field is absent or is not a string. ## Acceptance Criteria -| ID | Criteria | Verification | -| ----------- | --------------------------------------------------------------- | ----------------------------------- | -| FR-002-AC-1 | `version`, `--version`, and `-v` each print the package version | Test (cli.test.ts, scripts.test.ts) | -| FR-002-AC-2 | The reported value is a non-empty string | Test (cli.test.ts) | -| FR-002-AC-3 | A missing or non-string version field raises an error | Test (cli-version-missing.test.ts) | +| ID | Criteria | Verification | +| ----------- | ----------------------------------------------------------------------------------------------------- | ----------------------------------- | +| FR-002-AC-1 | `version`, `--version`, and `-v` each print the package version | Test (cli.test.ts, scripts.test.ts) | +| FR-002-AC-2 | The reported value is a non-empty string | Test (cli.test.ts) | +| FR-002-AC-3 | A missing or non-string version field raises an error | Test (cli-version-missing.test.ts) | +| FR-002-AC-4 | A non-empty baked version is reported verbatim; an empty one falls back to the `package.json` version | Test (version.test.ts) | ## Dependencies diff --git a/spec/functional/FR-022-self-update.md b/spec/functional/FR-022-self-update.md index b986cf0..54e31af 100644 --- a/spec/functional/FR-022-self-update.md +++ b/spec/functional/FR-022-self-update.md @@ -18,7 +18,8 @@ published `@agent-ix/quoin` by delegating to `ix-cli-core`'s self-update (querying the registry, comparing against the running version, and installing globally), supporting a `--check` mode that reports availability without installing and a `--registry ` option that selects the registry; with no -`--registry`, the ambient npm configuration resolves the package. +`--registry`, the command defaults to the public npm registry +(`https://registry.npmjs.org/`), where `@agent-ix/quoin` is published. ## Inputs @@ -34,16 +35,16 @@ installing and a `--registry ` option that selects the registry; with no package coordinates and the running version (see [FR-002](./FR-002-print-package-version.md)). - The CLI SHALL report availability without installing when `--check` is given. -- The CLI SHALL pass a `--registry` value through, and SHALL otherwise let the - ambient npm configuration resolve the package. +- The CLI SHALL pass a `--registry` value through, and SHALL otherwise default to + the public npm registry (`https://registry.npmjs.org/`). ## Acceptance Criteria -| ID | Criteria | Verification | -| ----------- | ---------------------------------------------------------------------------------------------------------------- | --------------------- | -| FR-022-AC-1 | `update` delegates to the self-update with `@agent-ix/quoin`, the running version, and the `quoin update` header | Test (update.test.ts) | -| FR-022-AC-2 | `--check` is passed through to report availability without installing | Test (update.test.ts) | -| FR-022-AC-3 | `--registry ` is passed through, and its absence leaves resolution to the ambient npm config | Test (update.test.ts) | +| ID | Criteria | Verification | +| ----------- | --------------------------------------------------------------------------------------------------------------------- | --------------------- | +| FR-022-AC-1 | `update` delegates to the self-update with `@agent-ix/quoin`, the running version, and the `quoin update` header | Test (update.test.ts) | +| FR-022-AC-2 | `--check` is passed through to report availability without installing | Test (update.test.ts) | +| FR-022-AC-3 | `--registry ` is passed through; its absence defaults to the public npm registry (`https://registry.npmjs.org/`) | Test (update.test.ts) | ## Dependencies diff --git a/spec/matrix.md b/spec/matrix.md index 6c24d4f..403459d 100644 --- a/spec/matrix.md +++ b/spec/matrix.md @@ -34,7 +34,7 @@ Coverage is mapped requirement → test as `file :: "test name"`: | Requirement | Coverage | Test (file :: name) | | ----------- | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | FR-001 | ✅ Covered | `cli.test.ts` :: "long flag with = and following-value form both work"; :: "boolean long flag (no value) is honored"; :: "subcommand is only captured for catalog/plugin; others become positionals" | -| FR-002 | ✅ Covered | `cli.test.ts` :: "--version, -v, and the version command all print the package version"; :: "returns a non-empty version string"; `cli-version-missing.test.ts` :: "throws when package.json has no string version" | +| FR-002 | ✅ Covered | `cli.test.ts` :: "--version, -v, and the version command all print the package version"; :: "returns a non-empty version string"; `cli-version-missing.test.ts` :: "throws when package.json has no string version"; `version.test.ts` :: "returns the baked version verbatim when present"; :: "falls back to the package.json version when nothing is baked in" | | FR-003 | ✅ Covered | `cli.test.ts` :: "no command prints the top-level usage"; :: "--help and -h print command help"; :: "--help on an unrecognized command falls back to the top-level usage"; :: "help for a spec-flow command prints the workflow usage" | | FR-004 | ✅ Covered | `cli.test.ts` :: "falls back to IX_HOME when --config-root is omitted, and prints 'unknown' for a versionless module"; :: "--no-project-config short-circuits project config root" | | FR-005 | ✅ Covered | `cli.test.ts` :: "throws on an unknown command"; :: "unknown catalog subcommand throws"; :: "unknown plugin subcommand throws" | @@ -54,7 +54,7 @@ Coverage is mapped requirement → test as `file :: "test name"`: | FR-019 | ✅ Covered | `plugins.test.ts` :: "install adds a plugin from a path source"; :: "installs, lists, then removes a plugin and its target dir + registry entry"; readModuleName suite ("reads the name from a top-level manifest.yaml"…); `cli.test.ts` :: "install without a source throws"; :: "remove without a name throws"; :: "remove deletes a plugin and prints confirmation" | | FR-020 | ✅ Covered | `flows.test.ts` :: "lists the bundled spec flows"; :: "throws for an unknown flow name"; `flows-notfound.test.ts` :: "throws when no candidate root contains the skill" | | FR-021 | ✅ Covered | `flows.test.ts` :: "resolves when ix-flow exits 0; builds id/json/target args"; :: "matrix runs the flow and propagates a non-zero exit code"; :: "defaults exit code to 1 when ix-flow is killed by a signal (null code)"; :: "rejects when ix-flow cannot be spawned (PATH has no ix-flow)"; `cli.test.ts` :: "review with --target/--json/--id runs the flow (ix-flow exit 0)" | -| FR-022 | ✅ Covered | `update.test.ts` :: "delegates to runSelfUpdate with quoin's package coordinates" (asserts packageName `@agent-ix/quoin`, currentVersion, header "quoin update", registry undefined → ambient config, check false); :: "passes --check through"; :: "passes a custom --registry through" | +| FR-022 | ✅ Covered | `update.test.ts` :: "delegates to runSelfUpdate with quoin's package coordinates" (asserts packageName `@agent-ix/quoin`, currentVersion, header "quoin update", registry defaults to `https://registry.npmjs.org/`, check false); :: "passes --check through"; :: "passes a custom --registry through"; :: "defaults the registry to public npm when no --registry is given" | | FR-023 | ✅ Covered | `cli.test.ts` :: "falls back to IX_HOME when --config-root is omitted, and prints 'unknown' for a versionless module"; `catalog.test.ts` :: "includes QUOIN_MODULE_PATHS entries and installed module dirs"; `flows.test.ts` :: "resolves when ix-flow exits 0; builds id/json/target args" (IX_SPEC_WORKFLOWS_ROOT) | diff --git a/spec/plan.md b/spec/plan.md index 120a7ff..6a06758 100644 --- a/spec/plan.md +++ b/spec/plan.md @@ -19,3 +19,4 @@ type: Plan | Task-007 | Add use cases for authoring, validation, plugins, and workflows. | Done | | Task-008 | Add an eval matrix with latency/token/tool-call metrics. | Done | | Task-009 | Build executable eval harness and fixture repos. | Done | +| Task-010 | Default `update` to public npm (FR-022) when no `--registry`. | Done | diff --git a/src/cli.ts b/src/cli.ts index 3847229..fa7a1d2 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -21,6 +21,12 @@ interface ParsedArgs { flags: Record; } +// quoin is published to the public npm registry (see package.json +// publishConfig.registry). `update` defaults here so it resolves the package +// from public npm regardless of the ambient npm config; override with +// --registry for local-dev snapshots (e.g. http://npm.ix/). +const DEFAULT_UPDATE_REGISTRY = "https://registry.npmjs.org/"; + const USAGE = `quoin Spec workflow and catalog CLI for Agent IX. @@ -156,10 +162,9 @@ Usage: Flags: --check Report whether an update is available; do not install. --registry Force an npm registry to query/install from. Defaults - to your npm config (the registry @agent-ix resolves to, - i.e. however quoin was installed). Pass - https://registry.npmjs.org/ for public npm, or - http://npm.ix/ for local dev snapshots. + to the public npm registry + (https://registry.npmjs.org/), where @agent-ix/quoin is + published. Pass http://npm.ix/ for local dev snapshots. Examples: quoin update @@ -225,7 +230,13 @@ export async function main(argv: string[]): Promise { throw new Error(`unknown command ${parsed.command}\n\n${USAGE}`); } -export function packageVersion(): string { +// Baked at build time from `git describe` (see vite.config.ts). A bare semver +// means a clean tagged release; a `--g` / `-dirty` suffix means the build +// is ahead of / diverges from its tag. Empty for dev/test/no-git builds, which +// fall back to package.json. +declare const __QUOIN_VERSION__: string; + +function readPackageJsonVersion(): string { const packageRoot = dirname(dirname(fileURLToPath(import.meta.url))); const packageJson = JSON.parse( readFileSync(join(packageRoot, "package.json"), "utf8"), @@ -236,6 +247,17 @@ export function packageVersion(): string { return packageJson.version; } +// Prefer the build-time baked version (truthful about drift); fall back to +// package.json when it is absent (dev/test builds, or a no-git build). +export function resolveVersion(baked: string): string { + if (baked) return baked; + return readPackageJsonVersion(); +} + +export function packageVersion(): string { + return resolveVersion(__QUOIN_VERSION__); +} + function helpFor(parsed: ParsedArgs): string { if (parsed.command === "update") return UPDATE_USAGE; if (parsed.command === "write") return WRITE_USAGE; @@ -270,9 +292,10 @@ async function runUpdate(parsed: ParsedArgs): Promise { packageName: "@agent-ix/quoin", currentVersion: packageVersion(), header: "quoin update", - // undefined → ambient npm config (the registry @agent-ix resolves to). - // Override for local dev with --registry http://npm.ix/. - registry: stringFlag(parsed, "registry"), + // Default to public npm (where @agent-ix/quoin is published) so update + // works regardless of the ambient npm config. Override for local dev with + // --registry http://npm.ix/. + registry: stringFlag(parsed, "registry") ?? DEFAULT_UPDATE_REGISTRY, check: parsed.flags.check === true, }); } diff --git a/tests/update.test.ts b/tests/update.test.ts index 5aa5bb7..81548a3 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -24,11 +24,19 @@ describe("quoin update", () => { packageName: "@agent-ix/quoin", currentVersion: packageVersion(), header: "quoin update", - registry: undefined, + registry: "https://registry.npmjs.org/", check: false, }); }); + it("defaults the registry to public npm when no --registry is given", async () => { + runSelfUpdate.mockClear(); + await main(["update"]); + expect(runSelfUpdate).toHaveBeenCalledWith( + expect.objectContaining({ registry: "https://registry.npmjs.org/" }), + ); + }); + it("passes --check through", async () => { runSelfUpdate.mockClear(); await main(["update", "--check"]); diff --git a/tests/version.test.ts b/tests/version.test.ts new file mode 100644 index 0000000..fad543e --- /dev/null +++ b/tests/version.test.ts @@ -0,0 +1,25 @@ +import { readFileSync } from "node:fs"; + +import { describe, expect, it } from "vitest"; + +import { packageVersion, resolveVersion } from "../src/cli"; + +describe("resolveVersion", () => { + it("returns the baked version verbatim when present", () => { + // A clean tagged release bakes a bare semver... + expect(resolveVersion("0.5.2")).toBe("0.5.2"); + // ...and a drifted build bakes a describe string with a suffix. + expect(resolveVersion("0.5.1-3-gabc123-dirty")).toBe( + "0.5.1-3-gabc123-dirty", + ); + }); + + it("falls back to the package.json version when nothing is baked in", () => { + const pkg = JSON.parse( + readFileSync(new URL("../package.json", import.meta.url), "utf8"), + ) as { version: string }; + // Under vitest __QUOIN_VERSION__ is "", so the fallback reads package.json. + expect(resolveVersion("")).toBe(pkg.version); + expect(packageVersion()).toBe(pkg.version); + }); +}); diff --git a/vite.config.ts b/vite.config.ts index 1ceaa8d..9854ad9 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -1,9 +1,27 @@ /// +import { execSync } from "node:child_process"; import { builtinModules } from "node:module"; import { defineConfig } from "vite"; import dts from "vite-plugin-dts"; +// Truthful version baked into the bundle at build time. `git describe` yields the +// bare tag on a clean release commit (e.g. v0.5.2) and a drift-revealing string +// otherwise (e.g. v0.5.1-3-gabc123-dirty); we strip the leading `v`. Only computed +// for `vite build` — under vitest (serve) it stays "" so packageVersion() exercises +// its package.json fallback. Empty on a no-git build (e.g. tarball) too. +function gitVersion(): string { + try { + return execSync("git describe --tags --dirty --always", { + encoding: "utf8", + }) + .trim() + .replace(/^v/, ""); + } catch { + return ""; + } +} + const external = [ ...builtinModules, ...builtinModules.map((name) => `node:${name}`), @@ -17,7 +35,10 @@ const external = [ "zod", ]; -export default defineConfig({ +export default defineConfig(({ command }) => ({ + define: { + __QUOIN_VERSION__: JSON.stringify(command === "build" ? gitVersion() : ""), + }, plugins: [dts({ rollupTypes: true, include: ["src"] })], build: { lib: { @@ -48,4 +69,4 @@ export default defineConfig({ }, }, }, -}); +}));