From 6913dbcdb179829edfbc42e0ed22c53534e9875e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 24 May 2026 18:14:56 -0400 Subject: [PATCH 1/3] fix(10): centralize semver comparison policy across hooks and changeset --- get-shit-done/bin/lib/semver-compare.cjs | 35 +++++++++ hooks/gsd-check-update-worker.js | 17 +---- hooks/gsd-statusline.js | 19 +++-- scripts/changeset/cli.cjs | 37 +++------ ...ug-10-semver-policy-consolidation.test.cjs | 30 ++++++++ tests/gsd-statusline.test.cjs | 17 ++++- tests/semver-compare.test.cjs | 76 +++++++++---------- 7 files changed, 143 insertions(+), 88 deletions(-) create mode 100644 get-shit-done/bin/lib/semver-compare.cjs create mode 100644 tests/bug-10-semver-policy-consolidation.test.cjs diff --git a/get-shit-done/bin/lib/semver-compare.cjs b/get-shit-done/bin/lib/semver-compare.cjs new file mode 100644 index 000000000..16d6c66bd --- /dev/null +++ b/get-shit-done/bin/lib/semver-compare.cjs @@ -0,0 +1,35 @@ +'use strict'; + +function toNumericTuple(input) { + const cleaned = String(input == null ? '' : input).trim().replace(/^v/, ''); + const base = cleaned.replace(/[-+].*$/, ''); + const parts = base.split('.'); + const major = Number.parseInt(parts[0], 10) || 0; + const minor = Number.parseInt(parts[1], 10) || 0; + const patch = Number.parseInt(parts[2], 10) || 0; + return [major, minor, patch]; +} + +function compareSemverCore(a, b) { + const [a0, a1, a2] = toNumericTuple(a); + const [b0, b1, b2] = toNumericTuple(b); + if (a0 !== b0) return a0 > b0 ? 1 : -1; + if (a1 !== b1) return a1 > b1 ? 1 : -1; + if (a2 !== b2) return a2 > b2 ? 1 : -1; + return 0; +} + +function isSemverNewer(a, b) { + return compareSemverCore(a, b) > 0; +} + +function isStableTripletSemver(v) { + return /^\d+\.\d+\.\d+$/.test(String(v || '').replace(/^v/, '')); +} + +module.exports = { + compareSemverCore, + isSemverNewer, + isStableTripletSemver, + toNumericTuple, +}; diff --git a/hooks/gsd-check-update-worker.js b/hooks/gsd-check-update-worker.js index 6ba3f3b25..fe9769195 100644 --- a/hooks/gsd-check-update-worker.js +++ b/hooks/gsd-check-update-worker.js @@ -12,23 +12,12 @@ const fs = require('fs'); const path = require('path'); const { execFileSync } = require('child_process'); +const { isSemverNewer } = require('../get-shit-done/bin/lib/semver-compare.cjs'); const cacheFile = process.env.GSD_CACHE_FILE; const projectVersionFile = process.env.GSD_PROJECT_VERSION_FILE; const globalVersionFile = process.env.GSD_GLOBAL_VERSION_FILE; -// Compare semver: true if a > b (a is strictly newer than b) -// Strips pre-release suffixes (e.g. '3-beta.1' → '3') to avoid NaN from Number() -function isNewer(a, b) { - const pa = (a || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0); - const pb = (b || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0); - for (let i = 0; i < 3; i++) { - if (pa[i] > pb[i]) return true; - if (pa[i] < pb[i]) return false; - } - return false; -} - // Check project directory first (local install), then global let installed = '0.0.0'; let configDir = ''; @@ -75,7 +64,7 @@ if (configDir) { const versionMatch = content.match(/(?:\/\/|#) gsd-hook-version:\s*(.+)/); if (versionMatch) { const hookVersion = versionMatch[1].trim(); - if (isNewer(installed, hookVersion) && !hookVersion.includes('{{')) { + if (isSemverNewer(installed, hookVersion) && !hookVersion.includes('{{')) { staleHooks.push({ file: hookFile, hookVersion, installedVersion: installed }); } } else { @@ -105,7 +94,7 @@ try { } catch (e) {} const result = { - update_available: latest && isNewer(latest, installed), + update_available: latest && isSemverNewer(latest, installed), installed, latest: latest || 'unknown', checked: Math.floor(Date.now() / 1000), diff --git a/hooks/gsd-statusline.js b/hooks/gsd-statusline.js index 250ca2e29..dde1b3b6c 100755 --- a/hooks/gsd-statusline.js +++ b/hooks/gsd-statusline.js @@ -6,6 +6,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); +const { isSemverNewer } = require('../get-shit-done/bin/lib/semver-compare.cjs'); // --- Config + last-command readers ------------------------------------------ @@ -403,13 +404,12 @@ function runStatusline() { if (cache.stale_hooks && cache.stale_hooks.length > 0) { // If installed version is ahead of npm latest, this is a dev install. // Running /gsd:update would downgrade — show a contextual warning instead. - const isDevInstall = (() => { - if (!cache.installed || !cache.latest || cache.latest === 'unknown') return false; - const parseV = v => v.replace(/^v/, '').split('.').map(Number); - const [ai, bi, ci] = parseV(cache.installed); - const [an, bn, cn] = parseV(cache.latest); - return ai > an || (ai === an && bi > bn) || (ai === an && bi === bn && ci > cn); - })(); + const isDevInstall = ( + cache.installed && + cache.latest && + cache.latest !== 'unknown' && + isInstalledAheadOfLatest(cache.installed, cache.latest) + ); if (isDevInstall) { gsdUpdate += '\x1b[33m⚠ dev install — re-run installer to sync hooks\x1b[0m │ '; } else { @@ -497,11 +497,16 @@ function composeStatusline({ return `${gsdUpdate}${modelSeg} │ ${dirSeg}${ctx}${lastCmdSuffix}`; } +function isInstalledAheadOfLatest(installed, latest) { + return isSemverNewer(installed, latest); +} + // Export helpers for unit tests. Harmless when run as a script. module.exports = { readGsdState, parseStateMd, formatGsdState, readGsdConfig, getConfigValue, readLastSlashCommand, composeStatusline, + isInstalledAheadOfLatest, }; /** diff --git a/scripts/changeset/cli.cjs b/scripts/changeset/cli.cjs index 8d67d8f47..b208a54ec 100755 --- a/scripts/changeset/cli.cjs +++ b/scripts/changeset/cli.cjs @@ -21,6 +21,10 @@ const { parseFragment, FRAGMENT_ERROR } = require('./parse.cjs'); const { renderChangelog } = require('./render.cjs'); const { serializeChangelog, parseChangelog } = require('./serialize.cjs'); const { renderGithubReleaseNotes } = require('./github-release-notes.cjs'); +const { + compareSemverCore, + isStableTripletSemver, +} = require('../../get-shit-done/bin/lib/semver-compare.cjs'); function parseArgs(argv) { const opts = { @@ -205,15 +209,14 @@ function cmdExtract(opts) { // Validate that both bounds are strict semver (N.N.N, digits only). // Coercing a malformed bound like "1.41.x" to "1.41.0" makes range // selection silently wrong; reject early with a structured error. - const SEMVER_RE = /^\d+\.\d+\.\d+$/; - if (!SEMVER_RE.test(from)) { + if (!isStableTripletSemver(from)) { return { exitCode: 1, report: { error: `invalid semver for --from: "${from}" (expected N.N.N)`, releases: [] }, textOutput: null, }; } - if (!SEMVER_RE.test(to)) { + if (!isStableTripletSemver(to)) { return { exitCode: 1, report: { error: `invalid semver for --to: "${to}" (expected N.N.N)`, releases: [] }, @@ -236,37 +239,17 @@ function cmdExtract(opts) { const text = fs.readFileSync(changelogPath, 'utf8'); const { releases } = parseChangelog(text); - // Walk the releases in document order (newest-first in a standard - // Keep-a-Changelog file). Collect every release whose version is - // strictly after `from` and up to and including `to`. - // - // The comparison is semver-aware via the standard numeric-tuple approach - // so that "1.5.9" < "1.5.10" (string comparison would fail here). - function parseSemver(v) { - const parts = String(v).split('.').map(Number); - return [parts[0] || 0, parts[1] || 0, parts[2] || 0]; - } - - function semverCmp(a, b) { - const [a0, a1, a2] = parseSemver(a); - const [b0, b1, b2] = parseSemver(b); - return a0 !== b0 ? a0 - b0 : a1 !== b1 ? a1 - b1 : a2 - b2; - } - const matched = releases.filter((rel) => { if (rel.version === 'Unreleased') return false; - // Skip pre-release entries (e.g. 1.0.0-rc.1, 1.0.0-beta.2) — they cannot - // be range-compared via numeric tuples without implementing full pre-release - // ordering (semver §11). Exclude them silently for now; a consolidation - // issue (#F8) will address pre-release ordering across all comparators. - if (!SEMVER_RE.test(rel.version)) { + // Extract mode intentionally operates on stable releases only. + if (!isStableTripletSemver(rel.version)) { process.stderr.write(`[extract] skipping pre-release/non-semver entry: ${rel.version}\n`); return false; } // from is exclusive: cmp > 0 means rel.version > from - const afterFrom = semverCmp(rel.version, from) > 0; + const afterFrom = compareSemverCore(rel.version, from) > 0; // to is inclusive: cmp <= 0 means rel.version <= to - const upToTo = semverCmp(rel.version, to) <= 0; + const upToTo = compareSemverCore(rel.version, to) <= 0; return afterFrom && upToTo; }); diff --git a/tests/bug-10-semver-policy-consolidation.test.cjs b/tests/bug-10-semver-policy-consolidation.test.cjs new file mode 100644 index 000000000..4d5ed184a --- /dev/null +++ b/tests/bug-10-semver-policy-consolidation.test.cjs @@ -0,0 +1,30 @@ +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); + +const { + compareSemverCore, + isStableTripletSemver, +} = require('../get-shit-done/bin/lib/semver-compare.cjs'); +const { isInstalledAheadOfLatest } = require('../hooks/gsd-statusline.js'); + +describe('bug #10: semver policy consolidation', () => { + test('shared comparator treats prerelease patch increment as newer than previous stable', () => { + assert.ok(compareSemverCore('1.2.1-beta.1', '1.2.0') > 0); + }); + + test('shared comparator ignores prerelease suffix for equal base versions', () => { + assert.equal(compareSemverCore('1.2.0-rc.1', '1.2.0'), 0); + }); + + test('stable-triplet validator excludes prerelease tags', () => { + assert.equal(isStableTripletSemver('1.2.0-rc.1'), false); + assert.equal(isStableTripletSemver('1.2.0'), true); + }); + + test('statusline dev-install detection uses shared comparator semantics', () => { + assert.equal(isInstalledAheadOfLatest('1.2.1-beta.1', '1.2.0'), true); + assert.equal(isInstalledAheadOfLatest('1.2.0-rc.1', '1.2.0'), false); + }); +}); diff --git a/tests/gsd-statusline.test.cjs b/tests/gsd-statusline.test.cjs index 2335a6f51..d996855ea 100644 --- a/tests/gsd-statusline.test.cjs +++ b/tests/gsd-statusline.test.cjs @@ -15,7 +15,12 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); -const { parseStateMd, formatGsdState, readGsdState } = require('../hooks/gsd-statusline.js'); +const { + parseStateMd, + formatGsdState, + readGsdState, + isInstalledAheadOfLatest, +} = require('../hooks/gsd-statusline.js'); // ─── parseStateMd ─────────────────────────────────────────────────────────── @@ -217,6 +222,16 @@ describe('formatGsdState', () => { }); }); +describe('isInstalledAheadOfLatest', () => { + test('treats prerelease patch increment as ahead of prior stable', () => { + assert.equal(isInstalledAheadOfLatest('1.2.1-beta.1', '1.2.0'), true); + }); + + test('treats equal base version prerelease as not ahead', () => { + assert.equal(isInstalledAheadOfLatest('1.2.0-rc.1', '1.2.0'), false); + }); +}); + // ─── readGsdState ─────────────────────────────────────────────────────────── describe('readGsdState', () => { diff --git a/tests/semver-compare.test.cjs b/tests/semver-compare.test.cjs index da8806c43..f8b8ba1ba 100644 --- a/tests/semver-compare.test.cjs +++ b/tests/semver-compare.test.cjs @@ -1,81 +1,79 @@ /** - * Tests for the isNewer() semver comparison function used in gsd-check-update.js. + * Shared semver compare utility tests. * - * WHY DUPLICATED: isNewer() lives inside a template literal string passed to - * spawn(process.execPath, ['-e', `...`]) — it runs in a detached child process - * that has no access to the parent module scope. This means it cannot be - * require()'d or imported from a shared module. The function is intentionally - * inlined in the spawn string so it works in the child process context. - * - * We mirror the implementation here so the logic is testable. If the hook's - * implementation diverges from this copy, the fix is to update this mirror — - * not to restructure the hook (which would require changing the spawn pattern - * across the entire hook architecture). + * These assertions lock the normalization policy used by update-check, + * statusline dev-install detection, and changeset extraction range compare. */ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); +const { + compareSemverCore, + isSemverNewer, + toNumericTuple, +} = require('../get-shit-done/bin/lib/semver-compare.cjs'); -// Mirror of isNewer() from hooks/gsd-check-update.js (inside spawn template) -function isNewer(a, b) { - const pa = (a || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0); - const pb = (b || '').split('.').map(s => Number(s.replace(/-.*/, '')) || 0); - for (let i = 0; i < 3; i++) { - if (pa[i] > pb[i]) return true; - if (pa[i] < pb[i]) return false; - } - return false; -} - -describe('isNewer (semver comparison)', () => { +describe('isSemverNewer (shared semver comparison)', () => { test('newer major version', () => { - assert.strictEqual(isNewer('2.0.0', '1.0.0'), true); + assert.strictEqual(isSemverNewer('2.0.0', '1.0.0'), true); }); test('newer minor version', () => { - assert.strictEqual(isNewer('1.1.0', '1.0.0'), true); + assert.strictEqual(isSemverNewer('1.1.0', '1.0.0'), true); }); test('newer patch version', () => { - assert.strictEqual(isNewer('1.0.1', '1.0.0'), true); + assert.strictEqual(isSemverNewer('1.0.1', '1.0.0'), true); }); test('equal versions', () => { - assert.strictEqual(isNewer('1.0.0', '1.0.0'), false); + assert.strictEqual(isSemverNewer('1.0.0', '1.0.0'), false); }); test('older version returns false', () => { - assert.strictEqual(isNewer('1.0.0', '2.0.0'), false); + assert.strictEqual(isSemverNewer('1.0.0', '2.0.0'), false); }); test('installed ahead of npm (git install scenario)', () => { - assert.strictEqual(isNewer('1.30.0', '1.31.0'), false); + assert.strictEqual(isSemverNewer('1.30.0', '1.31.0'), false); }); test('npm ahead of installed (real update available)', () => { - assert.strictEqual(isNewer('1.31.0', '1.30.0'), true); + assert.strictEqual(isSemverNewer('1.31.0', '1.30.0'), true); }); test('pre-release suffix stripped', () => { - assert.strictEqual(isNewer('1.0.1-beta.1', '1.0.0'), true); + assert.strictEqual(isSemverNewer('1.0.1-beta.1', '1.0.0'), true); }); test('pre-release on both sides', () => { - assert.strictEqual(isNewer('2.0.0-rc.1', '1.9.0-beta.2'), true); + assert.strictEqual(isSemverNewer('2.0.0-rc.1', '1.9.0-beta.2'), true); }); test('null/undefined handled', () => { - assert.strictEqual(isNewer(null, '1.0.0'), false); - assert.strictEqual(isNewer('1.0.0', null), true); - assert.strictEqual(isNewer(null, null), false); + assert.strictEqual(isSemverNewer(null, '1.0.0'), false); + assert.strictEqual(isSemverNewer('1.0.0', null), true); + assert.strictEqual(isSemverNewer(null, null), false); }); test('empty string handled', () => { - assert.strictEqual(isNewer('', '1.0.0'), false); - assert.strictEqual(isNewer('1.0.0', ''), true); + assert.strictEqual(isSemverNewer('', '1.0.0'), false); + assert.strictEqual(isSemverNewer('1.0.0', ''), true); }); test('two-segment version (missing patch)', () => { - assert.strictEqual(isNewer('1.1', '1.0'), true); - assert.strictEqual(isNewer('1.0', '1.1'), false); + assert.strictEqual(isSemverNewer('1.1', '1.0'), true); + assert.strictEqual(isSemverNewer('1.0', '1.1'), false); + }); + + test('v-prefixed versions normalize consistently', () => { + assert.strictEqual(isSemverNewer('v1.2.1', '1.2.0'), true); + assert.strictEqual(isSemverNewer('1.2.0', 'v1.2.0'), false); + assert.deepStrictEqual(toNumericTuple('v1.2.3-rc.1'), [1, 2, 3]); + }); + + test('core comparator uses three-way ordering', () => { + assert.strictEqual(compareSemverCore('1.2.0', '1.2.0'), 0); + assert.strictEqual(compareSemverCore('1.2.1', '1.2.0'), 1); + assert.strictEqual(compareSemverCore('1.2.0', '1.2.1'), -1); }); }); From 5434c8c4cb69e0a8ba3e18edd405e6a7b7975b53 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 24 May 2026 18:25:21 -0400 Subject: [PATCH 2/3] docs: update inventory surfaces for semver compare module --- docs/INVENTORY-MANIFEST.json | 3 ++- docs/INVENTORY.md | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 80c9b9225..41893a1c3 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -1,5 +1,5 @@ { - "generated": "2026-05-23", + "generated": "2026-05-24", "families": { "agents": [ "gsd-advisor-researcher", @@ -321,6 +321,7 @@ "secrets.cjs", "secrets.generated.cjs", "security.cjs", + "semver-compare.cjs", "shell-command-projection.cjs", "state-command-router.cjs", "state-document.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 956b1792a..75c72aac9 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -362,7 +362,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (79 shipped) +## CLI Modules (80 shipped) Full listing: `get-shit-done/bin/lib/*.cjs`. @@ -428,6 +428,7 @@ Full listing: `get-shit-done/bin/lib/*.cjs`. | `schema-detect.generated.cjs` | GENERATED — CJS artifact emitted from `sdk/src/query/schema-detect.ts` via `sdk/scripts/gen-schema-detect.mjs`; schema-drift detection for ORM patterns (Prisma, Drizzle, Supabase, TypeORM, Payload); exports `detectSchemaFiles`, `detectSchemaOrm`, `checkSchemaDrift`, `SCHEMA_PATTERNS`, `ORM_INFO`; do not edit directly | | `secrets.cjs` | CJS shim adapter — re-exports from `secrets.generated.cjs` (Phase 6/#3575 Shared Module migration) | | `secrets.generated.cjs` | GENERATED — CJS artifact emitted from `sdk/src/query/secrets.ts` via `sdk/scripts/gen-secrets.mjs`; secret-config masking convention (`****`) for integration keys; exports `SECRET_CONFIG_KEYS`, `isSecretKey`, `maskSecret`, `maskIfSecret`; do not edit directly | +| `semver-compare.cjs` | Shared semver comparison policy helpers (`compareSemverCore`, stable-triplet validation, normalized tuple parsing) consumed by update-check hooks, statusline dev-install detection, and changeset extract range logic (#10) | | `security.cjs` | Path traversal prevention, prompt injection detection, safe JSON/shell helpers | | `shell-command-projection.cjs` | Runtime-aware shell command projection for managed hook serialization: decides PowerShell call-operator usage by runtime/platform and normalizes Windows script path tokens | | `state-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools state` | From 22136ae19d6cdb83f5fd709770ea9300c79b1367 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 24 May 2026 18:32:50 -0400 Subject: [PATCH 3/3] chore(changeset): add fragment for issue 10 semver consolidation --- .changeset/eager-pumas-dance.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/eager-pumas-dance.md diff --git a/.changeset/eager-pumas-dance.md b/.changeset/eager-pumas-dance.md new file mode 100644 index 000000000..42b6c195e --- /dev/null +++ b/.changeset/eager-pumas-dance.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 10 +--- +**Semver policy is now centralized for update/status/changeset flows** — semver parsing and core comparison were duplicated in six places with drift risk. This change introduces a shared comparator utility and migrates statusline, update worker, and changeset extraction to use one implementation, keeping dev-install and release gating behavior consistent across runtimes. (#10)