From 15d9ffd1c98d03f3cfe808c404898da90f39fe3b Mon Sep 17 00:00:00 2001
From: haelyra <49814733+haelyra@users.noreply.github.com>
Date: Sun, 11 Oct 2026 06:49:37 -0400
Subject: [PATCH] fix: validate rollback allocation before changing live skill

---
 scripts/lib/skill-evolution/versioning.js | 15 ++++++---
 tests/lib/skill-evolution-storage.test.js | 38 +++++++++++++++++++++++
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/skill-evolution/versioning.js b/scripts/lib/skill-evolution/versioning.js
index 32bc83858..a57178ac3 100644
--- a/scripts/lib/skill-evolution/versioning.js
+++ b/scripts/lib/skill-evolution/versioning.js
@@ -169,9 +169,7 @@ function getEvolutionLog(skillPath, logType) {
   return readJsonl(getEvolutionLogPath(skillPath, logType));
 }
 
-function createVersion(skillPath, options = {}) {
-  const skillFilePath = ensureSkillExists(skillPath);
-
+function allocateSnapshotVersion(skillPath) {
   const versions = listVersions(skillPath);
   const versionsDir = getVersionsDir(skillPath);
   const occupiedPaths = new Set(fs.existsSync(versionsDir) ? fs.readdirSync(versionsDir) : []);
@@ -183,6 +181,13 @@ function createVersion(skillPath, options = {}) {
   if (!Number.isSafeInteger(nextVersion)) {
     throw new Error('Skill snapshot version range is exhausted; existing snapshots were preserved');
   }
+  return nextVersion;
+}
+
+function createVersion(skillPath, options = {}) {
+  const skillFilePath = ensureSkillExists(skillPath);
+  const nextVersion = allocateSnapshotVersion(skillPath);
+  const versionsDir = getVersionsDir(skillPath);
   ensureSkillVersioning(skillPath);
   const snapshotPath = path.join(versionsDir, `v${nextVersion}.md`);
   const skillContent = fs.readFileSync(skillFilePath, 'utf8');
@@ -207,12 +212,11 @@ function createVersion(skillPath, options = {}) {
 
 function rollbackTo(skillPath, targetVersion, options = {}) {
   const normalizedTargetVersion = Number(targetVersion);
-  if (!Number.isInteger(normalizedTargetVersion) || normalizedTargetVersion <= 0) {
+  if (!Number.isSafeInteger(normalizedTargetVersion) || normalizedTargetVersion <= 0) {
     throw new Error(`Invalid target version: ${targetVersion}`);
   }
 
   ensureSkillExists(skillPath);
-  ensureSkillVersioning(skillPath);
 
   const targetPath = path.join(getVersionsDir(skillPath), `v${normalizedTargetVersion}.md`);
   if (!fs.existsSync(targetPath)) {
@@ -221,6 +225,7 @@ function rollbackTo(skillPath, targetVersion, options = {}) {
 
   const currentVersion = getCurrentVersion(skillPath);
   const targetContent = fs.readFileSync(targetPath, 'utf8');
+  allocateSnapshotVersion(skillPath);
   fs.writeFileSync(getSkillFilePath(skillPath), targetContent, 'utf8');
 
   const createdVersion = createVersion(skillPath, {
diff --git a/tests/lib/skill-evolution-storage.test.js b/tests/lib/skill-evolution-storage.test.js
index 8cc7a8a1b..50283f2d1 100644
--- a/tests/lib/skill-evolution-storage.test.js
+++ b/tests/lib/skill-evolution-storage.test.js
@@ -102,5 +102,43 @@ test('an occupied final safe snapshot path refuses creation while preserving the
   assert.equal(fs.existsSync(versioning.getEvolutionDir(skillDir)), false);
 });
 
+test('exhausted rollback preserves unsnapshotted content, snapshot files and evolution state', fixture => {
+  const { skillDir, versionsDir } = fixture;
+  fs.mkdirSync(versionsDir);
+  const previous = path.join(versionsDir, 'v1.md');
+  const last = path.join(versionsDir, `v${Number.MAX_SAFE_INTEGER}.md`);
+  fs.writeFileSync(previous, '# Previous\n');
+  fs.writeFileSync(last, '# Latest snapshot\n');
+  const assertPreserved = () => {
+    assert.equal(fs.readFileSync(path.join(skillDir, 'SKILL.md'), 'utf8'), '# Original\n');
+    assert.equal(fs.readFileSync(previous, 'utf8'), '# Previous\n');
+    assert.equal(fs.readFileSync(last, 'utf8'), '# Latest snapshot\n');
+    assert.equal(fs.readdirSync(versionsDir).length, 2);
+  };
+  assert.throws(() => versioning.rollbackTo(skillDir, 1), /snapshot version range is exhausted/);
+  assertPreserved();
+  assert.equal(fs.existsSync(versioning.getEvolutionDir(skillDir)), false);
+  versioning.ensureSkillVersioning(skillDir);
+  const logs = versioning.EVOLUTION_LOG_TYPES.map(type => versioning.getEvolutionLogPath(skillDir, type));
+  for (const file of logs) fs.writeFileSync(file, '{"event":"preserve"}\n');
+  assert.throws(() => versioning.rollbackTo(skillDir, 1), /snapshot version range is exhausted/);
+  assertPreserved();
+  for (const file of logs) assert.equal(fs.readFileSync(file, 'utf8'), '{"event":"preserve"}\n');
+});
+
+test('unsafe rollback targets preserve the live skill and excluded snapshots', fixture => {
+  const { skillDir, versionsDir } = fixture;
+  fs.mkdirSync(versionsDir);
+  const invalid = path.join(versionsDir, 'v9007199254740992.md');
+  fs.writeFileSync(invalid, '# Excluded snapshot\n');
+  for (const target of [9007199254740992, '9007199254740992']) {
+    assert.throws(() => versioning.rollbackTo(skillDir, target), /Invalid target version/);
+    assert.equal(fs.readFileSync(path.join(skillDir, 'SKILL.md'), 'utf8'), '# Original\n');
+    assert.equal(fs.readFileSync(invalid, 'utf8'), '# Excluded snapshot\n');
+    assert.equal(fs.readdirSync(versionsDir).length, 1);
+    assert.equal(fs.existsSync(versioning.getEvolutionDir(skillDir)), false);
+  }
+});
+
 console.log(`Results: Passed: ${passed}, Failed: ${failed}`);
 process.exitCode = failed > 0 ? 1 : 0;