Keep helper behavior single-sourced before duplicate parsers drift again

openspec/changes/agent-codex-replace-custom-semver-jsonc-stdin-helper-2026-04-22-18-06/proposal.md

@@ -0,0 +1,24 @@
+## Why
+
+- `src/toolchain/index.js` and `src/cli/main.js` still carry duplicate custom version-comparison and stdin line-reader helpers.
+- The custom semver logic ignores prerelease ordering, and the single-byte stdin reader splits multi-byte characters.
+- `src/scaffold/index.js` still hand-rolls JSONC stripping even though the repo only needs robust JSONC parsing for settings-style files.
+
+## What Changes
+
+- Add shared core helpers for semver comparison and stdin line reading, then route both `src/toolchain/index.js` and `src/cli/main.js` through them.
+- Replace the custom JSONC stripping/parsing path in `src/scaffold/index.js` with `jsonc-parser`.
+- Add focused regression coverage for prerelease version ordering, multi-byte stdin reads, and JSONC parsing with comments/trailing commas.
+
+## Acceptance Criteria
+
+- Prerelease ordering comes from shared semver helpers under `src/core`, so `1.2.3` sorts after `1.2.3-alpha.4` and `1.2.3-alpha.10` sorts after `1.2.3-alpha.2`.
+- Interactive yes/no prompts keep reading a single logical line without corrupting multi-byte UTF-8 input.
+- JSONC parsing for scaffold-owned settings files uses `jsonc-parser` and preserves string literals that contain comment-like text.
+- CLI commands keep their existing names, prompts, and output wording on `status`, `release`, and `setup` flows.
+
+## Impact
+
+- Primary files: `package.json`, `package-lock.json`, `src/core/**`, `src/toolchain/index.js`, `src/cli/main.js`, `src/scaffold/index.js`, and targeted tests.
+- Main risk is behavior drift in `gx status`, self-update prompts, README-driven release selection, and VS Code settings repair, so verification stays focused on those paths.
+- This is an internal cleanup/correctness pass only; command names, output wording, and managed-file behavior must stay stable.

openspec/changes/agent-codex-replace-custom-semver-jsonc-stdin-helper-2026-04-22-18-06/specs/cli-modularization/spec.md

@@ -0,0 +1,14 @@
+## MODIFIED Requirements
+
+### Requirement: Module seams mirror operational responsibility
+The CLI SHALL keep version comparison, interactive stdin reading, and JSONC parsing in single-sourced shared helpers instead of redefining custom parser logic in command modules.
+
+#### Scenario: Toolchain and release flows reuse the same version/stdin helpers
+- **WHEN** maintainers inspect `src/toolchain/index.js` and `src/cli/main.js`
+- **THEN** semver comparison and interactive stdin line reading come from shared helpers under `src/core`
+- **AND** `src/toolchain/index.js` and `src/cli/main.js` do not reintroduce local copies of those helpers.
+
+#### Scenario: Scaffold JSONC parsing uses a standards-based parser
+- **WHEN** Guardex reads repo-owned JSONC-style files such as shared VS Code settings
+- **THEN** comments and trailing commas are parsed through `jsonc-parser`
+- **AND** escaped string content is preserved without custom comment-stripping logic.

openspec/changes/agent-codex-replace-custom-semver-jsonc-stdin-helper-2026-04-22-18-06/tasks.md

@@ -0,0 +1,37 @@
+## Definition of Done
+
+This change is complete only when all of the following are true:
+
+- Every checkbox below is checked.
+- The agent branch reaches `MERGED` state on `origin` and the PR URL + state are recorded in the completion handoff.
+- If any step blocks, add a `BLOCKED:` line under section 4 and stop.
+
+## Handoff
+
+- Handoff: change=`agent-codex-replace-custom-semver-jsonc-stdin-helper-2026-04-22-18-06`; branch=`agent/codex/replace-custom-semver-jsonc-stdin-helper-2026-04-22-18-06`; scope=`package.json`, `package-lock.json`, `src/core/**`, `src/toolchain/index.js`, `src/cli/main.js`, `src/scaffold/index.js`, and targeted tests; action=`replace the remaining custom semver/jsonc/stdin helpers with shared, standards-based implementations without changing CLI behavior`.
+
+## 1. Specification
+
+- [x] 1.1 Capture the bounded cleanup scope and acceptance criteria for the semver/jsonc/stdin helper replacement.
+- [x] 1.2 Add a `cli-modularization` spec delta that keeps version/stdin helper ownership single-sourced and JSONC parsing standards-based.
+
+## 2. Implementation
+
+- [x] 2.1 Add focused regression coverage for prerelease version ordering, multi-byte stdin reads, and JSONC parsing before deleting the custom helper code.
+- [x] 2.2 Add shared core helpers for semver comparison and stdin line reading, then route `src/toolchain/index.js` and `src/cli/main.js` through them.
+- [x] 2.3 Replace the custom JSONC stripping/parsing code in `src/scaffold/index.js` with `jsonc-parser`.
+- [x] 2.4 Remove the now-duplicated local helper implementations from `src/cli/main.js` and `src/toolchain/index.js`.
+
+## 3. Verification
+
+- [x] 3.1 Run `node --check src/core/versions.js src/core/stdin.js src/toolchain/index.js src/scaffold/index.js src/cli/main.js`.
+- [x] 3.2 Run targeted tests for the touched surfaces (`node --test test/status.test.js test/release.test.js test/setup.test.js test/core-version.test.js test/core-stdin.test.js test/scaffold-jsonc.test.js`).
+- [x] 3.3 Run `npm test`.
+- [x] 3.4 Run `openspec validate agent-codex-replace-custom-semver-jsonc-stdin-helper-2026-04-22-18-06 --type change --strict`.
+- [x] 3.5 Run `openspec validate --specs`.
+
+## 4. Cleanup
+
+- [ ] 4.1 Run `gx branch finish --branch agent/codex/replace-custom-semver-jsonc-stdin-helper-2026-04-22-18-06 --base main --via-pr --wait-for-merge --cleanup`.
+- [ ] 4.2 Record PR URL and final merge state (`MERGED`) in the completion handoff.
+- [ ] 4.3 Confirm the sandbox worktree is removed and no local/remote refs remain for the branch.

package.json

@@ -69,5 +69,9 @@
   },
   "devDependencies": {
     "fast-check": "^3.23.2"
+  },
+  "dependencies": {
+    "jsonc-parser": "^3.3.1",
+    "semver": "^7.7.4"
   }
 }

src/cli/main.js

@@ -113,6 +113,12 @@ const {
   runReviewBotCommand,
   invokePackageAsset,
 } = require('../core/runtime');
+const {
+  parseVersionString,
+  compareParsedVersions,
+  isNewerVersion,
+} = require('../core/versions');
+const { readSingleLineFromStdin } = require('../core/stdin');
 const {
   normalizeManagedForcePath,
   parseCommonArgs,
@@ -169,9 +175,6 @@ const {
   removeLegacyManagedRepoFile,
   ensureAgentsSnippet,
   ensureManagedGitignore,
-  stripJsonComments,
-  stripJsonTrailingCommas,
-  parseJsonObjectLikeFile,
   buildRepoVscodeSettings,
   ensureRepoVscodeSettings,
   configureHooks,
@@ -867,44 +870,6 @@ function isInteractiveTerminal() {
   return Boolean(process.stdin.isTTY && process.stdout.isTTY);
 }
 
-const stdinWaitArray = new Int32Array(new SharedArrayBuffer(4));
-
-function sleepSyncMs(milliseconds) {
-  Atomics.wait(stdinWaitArray, 0, 0, milliseconds);
-}
-
-function readSingleLineFromStdin() {
-  let input = '';
-  const buffer = Buffer.alloc(1);
-
-  while (true) {
-    let bytesRead = 0;
-    try {
-      bytesRead = fs.readSync(process.stdin.fd, buffer, 0, 1);
-    } catch (error) {
-      if (error && ['EAGAIN', 'EWOULDBLOCK', 'EINTR'].includes(error.code)) {
-        sleepSyncMs(15);
-        continue;
-      }
-      return input;
-    }
-
-    if (bytesRead === 0) {
-      if (process.stdin.isTTY) {
-        sleepSyncMs(15);
-        continue;
-      }
-      return input;
-    }
-
-    const char = buffer.toString('utf8', 0, bytesRead);
-    if (char === '\n' || char === '\r') {
-      return input;
-    }
-    input += char;
-  }
-}
-
 function parseAutoApproval(name) {
   const raw = process.env[name];
   if (raw == null) return null;
@@ -983,38 +948,6 @@ function describeGuardexRepoToggle(toggle) {
   return `${toggle.source} (${GUARDEX_REPO_TOGGLE_ENV}=${toggle.raw})`;
 }
 
-function parseVersionString(version) {
-  const match = String(version || '').trim().match(/^v?(\d+)\.(\d+)\.(\d+)/);
-  if (!match) return null;
-  return [
-    Number.parseInt(match[1], 10),
-    Number.parseInt(match[2], 10),
-    Number.parseInt(match[3], 10),
-  ];
-}
-
-function compareParsedVersions(left, right) {
-  if (!left || !right) return 0;
-  for (let index = 0; index < Math.max(left.length, right.length); index += 1) {
-    const leftValue = left[index] || 0;
-    const rightValue = right[index] || 0;
-    if (leftValue > rightValue) return 1;
-    if (leftValue < rightValue) return -1;
-  }
-  return 0;
-}
-
-function isNewerVersion(latest, current) {
-  const latestParts = parseVersionString(latest);
-  const currentParts = parseVersionString(current);
-
-  if (!latestParts || !currentParts) {
-    return String(latest || '').trim() !== String(current || '').trim();
-  }
-
-  return compareParsedVersions(latestParts, currentParts) > 0;
-}
-
 function parseNpmVersionOutput(stdout) {
   const trimmed = String(stdout || '').trim();
   if (!trimmed) return '';

src/core/stdin.js

@@ -0,0 +1,52 @@
+const fs = require('node:fs');
+const { StringDecoder } = require('node:string_decoder');
+
+const stdinWaitArray = new Int32Array(new SharedArrayBuffer(4));
+
+function sleepSyncMs(milliseconds) {
+  Atomics.wait(stdinWaitArray, 0, 0, milliseconds);
+}
+
+function readSingleLineFromStdin(options = {}) {
+  const fsModule = options.fsModule || fs;
+  const input = options.input || process.stdin;
+  const sleepSync = options.sleepSync || sleepSyncMs;
+  const retryDelayMs = options.retryDelayMs == null ? 15 : options.retryDelayMs;
+  const buffer = Buffer.alloc(1);
+  const decoder = new StringDecoder('utf8');
+  let text = '';
+
+  while (true) {
+    let bytesRead = 0;
+    try {
+      bytesRead = fsModule.readSync(input.fd, buffer, 0, 1);
+    } catch (error) {
+      if (error && ['EAGAIN', 'EWOULDBLOCK', 'EINTR'].includes(error.code)) {
+        sleepSync(retryDelayMs);
+        continue;
+      }
+      return text + decoder.end();
+    }
+
+    if (bytesRead === 0) {
+      if (input.isTTY) {
+        sleepSync(retryDelayMs);
+        continue;
+      }
+      return text + decoder.end();
+    }
+
+    const char = decoder.write(buffer.subarray(0, bytesRead));
+    if (!char) {
+      continue;
+    }
+    if (char === '\n' || char === '\r') {
+      return text;
+    }
+    text += char;
+  }
+}
+
+module.exports = {
+  readSingleLineFromStdin,
+};

src/core/versions.js

@@ -0,0 +1,33 @@
+const semver = require('semver');
+
+function parseVersionString(version) {
+  const trimmed = String(version || '').trim();
+  if (!trimmed) {
+    return null;
+  }
+  return semver.valid(trimmed) || null;
+}
+
+function compareParsedVersions(left, right) {
+  if (!left || !right) {
+    return 0;
+  }
+  return semver.compare(left, right);
+}
+
+function isNewerVersion(latest, current) {
+  const latestParts = parseVersionString(latest);
+  const currentParts = parseVersionString(current);
+
+  if (!latestParts || !currentParts) {
+    return String(latest || '').trim() !== String(current || '').trim();
+  }
+
+  return semver.gt(latestParts, currentParts);
+}
+
+module.exports = {
+  parseVersionString,
+  compareParsedVersions,
+  isNewerVersion,
+};