golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Merge pull request #237 from open-gsd/fix/10-bug-codebase-has-6-divergent-semver-comp

Browse Source 
fix(10): consolidate semver comparison policy
This commit is contained in:
Tom Boucher
2026-05-24 18:45:48 -04:00
committed by GitHub
parent 7170120325 22136ae19d
commit 08f075e91e
10 changed files with 152 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/eager-pumas-dance.md Normal file
View File

@@ -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)

3
docs/INVENTORY-MANIFEST.json
View File

@@ -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",

3
docs/INVENTORY.md
View File

@@ -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 (`****<last-4>`) 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` |

35
get-shit-done/bin/lib/semver-compare.cjs Normal file
View File

@@ -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,
};

17
hooks/gsd-check-update-worker.js
View File

@@ -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),

19
hooks/gsd-statusline.js
View File

@@ -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,
};
/**

37
scripts/changeset/cli.cjs
View File

@@ -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;
});

30
tests/bug-10-semver-policy-consolidation.test.cjs Normal file
View File

@@ -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);
});
});

17
tests/gsd-statusline.test.cjs
View File

@@ -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', () => {

76
tests/semver-compare.test.cjs
View File

@@ -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);
});
});