src/utils/validation.ts (2)

6-6: Optional: consider co-locating compareSemverStrings with the validation module.

validation.ts is a general-purpose utility, but it now carries a dependency on setup/migration.ts. If migration.ts is ever split or its exports change, this silent coupling breaks version checks. Moving compareSemverStrings into validation.ts (or a dedicated src/utils/semver.ts) would eliminate the dependency and make migration.ts the consumer rather than the source.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/validation.ts` at line 6, validation.ts currently imports
compareSemverStrings from setup/migration.js creating an unnecessary coupling;
extract the compareSemverStrings implementation into validation.ts (or a new
src/utils/semver.ts) and update all imports (e.g., in migration.ts and any other
modules) to consume it from the new location so migration.ts becomes a consumer,
not the source; ensure exported name stays compareSemverStrings and run/adjust
any unit tests or type exports that referenced the old path.

---

261-264: Consider including bun upgrade in the error message for immediate actionability.

The current URL (https://bun.sh/docs/installation) is an install guide. Users who already have an older Bun installed are better served by the upgrade command directly. bun upgrade is the built-in command for upgrading Bun, and a dedicated upgrade guide is available at https://bun.com/docs/guides/util/upgrade.

💡 Suggested wording

-      `Please upgrade: https://bun.sh/docs/installation`
+      `Please upgrade by running: bun upgrade\n` +
+      `See also: https://bun.com/docs/guides/util/upgrade`

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/validation.ts` around lines 261 - 264, The version-mismatch message
that uses minVersion and currentVersion should be updated to include an
actionable upgrade hint; in the return statement that builds the string using
minVersion and currentVersion (where the code currently returns `ralph-tui
requires Bun >= ${minVersion}... Bun ${currentVersion}`), append a suggestion to
run `bun upgrade` and include the upgrade guide URL
(`https://bun.com/docs/guides/util/upgrade`) alongside or instead of the
installation docs so users have an immediate remediation path.

src/utils/validation.test.ts (1)

49-61: Consider adding a test for a pre-release version that falls below the minimum.

The suite covers 1.3.6-beta (pre-release of the minimum itself) and build metadata, but not 1.3.5-beta — a pre-release of a version that is lower than the minimum. The implementation handles it correctly (metadata stripped → 1.3.5 < 1.3.6 → error), but an explicit test would document the intended behaviour.

+  test('returns error for pre-release version below minimum', () => {
+    const result = checkBunVersion('1.3.5-beta', MIN_VERSION);
+    expect(result).not.toBeNull();
+    expect(result).toContain('requires Bun >= 1.3.6');
+  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/validation.test.ts` around lines 49 - 61, Add a test in
src/utils/validation.test.ts that verifies pre-release versions below the
minimum produce an error: call checkBunVersion('1.3.5-beta', MIN_VERSION) and
assert it is NOT null (or matches the existing error shape). Place it alongside
the other tests (e.g., name it "pre-release below minimum returns error") so it
documents that '1.3.5-beta' is treated as 1.3.5 and thus fails against
MIN_VERSION; reference the checkBunVersion function and MIN_VERSION constant
used in the file.

src/cli.tsx (1)

28-29: MIN_BUN_VERSION is a second source of truth alongside package.json.

engines.bun in package.json already declares >=1.3.6. If the minimum version is bumped in the future, both this constant and the manifest need updating in sync. A brief comment cross-referencing package.json would reduce the risk of the two drifting apart.

💡 Suggested addition

-/** Minimum bun version required to run ralph-tui. */
+/** Minimum bun version required to run ralph-tui. Keep in sync with engines.bun in package.json. */
 const MIN_BUN_VERSION = '1.3.6';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cli.tsx` around lines 28 - 29, The MIN_BUN_VERSION constant is a second
source of truth; update the comment above MIN_BUN_VERSION to explicitly
reference the engines.bun entry in package.json (e.g., "See package.json
engines.bun for the authoritative minimum") and add a short note to keep both
values in sync when bumping versions — alternatively, replace the hardcoded
MIN_BUN_VERSION with code that reads engines.bun from package.json at runtime if
you prefer a single source of truth.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

src/cli.tsx (1)

159-160: The dynamic package.json import inside handleSubcommand is now redundant and shadows the module-level pkg.

The static import added at line 27 (import pkg from '../package.json') already provides the JSON object as pkg. The local const pkg = await import(…) re-imports the same file, shadows the outer pkg with a module-namespace object (requiring .default.version instead of .version), and adds an unnecessary async dependency to the version branch.

♻️ Suggested simplification

-    // Dynamic import to get version from package.json
-    const pkg = await import('../package.json', { with: { type: 'json' } });
-    console.log(`ralph-tui ${pkg.default.version}`);
+    console.log(`ralph-tui ${pkg.version}`);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cli.tsx` around lines 159 - 160, In handleSubcommand, remove the local
dynamic import that shadows the module-level pkg; eliminate "const pkg = await
import(...)" and use the already-imported top-level pkg (import pkg from
'../package.json') so console.log uses pkg.version instead of
pkg.default.version, avoiding the unnecessary await/async dependency and
shadowing; update the console.log call in handleSubcommand accordingly.

src/utils/semver.ts (1)

15-21: v-prefixed strings silently compare as 0.x.y.

parseInt("v1", 10) returns NaN, which || 0 converts to 0, so "v1.3.6" is treated as "0.3.6". Current callers (Bun.version, config version strings) never produce a v prefix, so this is benign today, but as the utility gains callers it could bite. A one-liner guard would future-proof it:

♻️ Optional robustness fix

 export function compareSemverStrings(a: string, b: string): -1 | 0 | 1 {
-  // Strip any pre-release/build metadata (e.g., "2.0-beta" -> "2.0")
-  const cleanA = a.split(/[-+]/)[0];
-  const cleanB = b.split(/[-+]/)[0];
+  // Strip leading 'v' and any pre-release/build metadata (e.g., "v2.0-beta" -> "2.0")
+  const cleanA = a.replace(/^v/i, '').split(/[-+]/)[0];
+  const cleanB = b.replace(/^v/i, '').split(/[-+]/)[0];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/semver.ts` around lines 15 - 21, compareSemverStrings silently
treats "v"-prefixed versions as 0.x.y because parseInt returns NaN; fix by
normalizing inputs to strip a leading "v" (case-insensitive) before removing
pre-release/build metadata so "v1.3.6" becomes "1.3.6"; update the cleanA/cleanB
logic in compareSemverStrings (e.g., apply .replace(/^v/i, '') before
.split(/[-+]/)[0]) so parsing to integers works correctly for v-prefixed
strings.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

src/utils/semver.test.ts (1)

39-43: Consider documenting the intentional SemVer spec deviation in the pre-release test.

Per the SemVer spec, a pre-release tag makes a version lower than the corresponding release (2.0-alpha < 2.0), so compareSemverStrings('2.0-alpha', '2.0') returning 0 rather than -1 is a deliberate non-spec behaviour. The test name describes the mechanism ("strips … metadata") but not the semantic consequence, which can mislead future maintainers into assuming full spec compliance.

A short comment and/or an extra assertion makes the intent explicit:

✏️ Suggested annotation

  test('strips pre-release and build metadata', () => {
+    // NOTE: Intentionally deviates from SemVer §9 — pre-release tags are
+    // stripped rather than being given lower precedence than the release.
+    // This means '2.0-alpha' is treated as equal to '2.0', not less-than.
    expect(compareSemverStrings('2.0-beta', '2.0')).toBe(0);
    expect(compareSemverStrings('2.0+build123', '2.0')).toBe(0);
    expect(compareSemverStrings('2.0-alpha', '2.0-beta')).toBe(0);
+    // Consequence for the Bun version check: '1.3.6-rc1' is treated as '1.3.6' and passes.
+    expect(compareSemverStrings('1.3.6-rc1', '1.3.6')).toBe(0);
  });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/utils/semver.test.ts` around lines 39 - 43, The test in semver.test.ts
uses compareSemverStrings and currently asserts that pre-release/build metadata
are stripped (e.g., compareSemverStrings('2.0-alpha','2.0') === 0) but doesn't
document that this deliberately deviates from SemVer (where '2.0-alpha' <
'2.0'); update the test by adding a short inline comment next to the
compareSemverStrings calls explaining the intentional non‑spec behavior, and
optionally add an extra assertion or note that the SemVer spec would expect -1
for '2.0-alpha' vs '2.0' to make the semantic consequence explicit for future
maintainers while keeping the existing assertions intact.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro