Simplify manual Apply rollback flow

Centralize repeated rollback result construction, repair context, and entry verification without changing the live Apply contract.

AI-assisted: prepared by Codex under pbakaus's scheduled architecture-refactor authorization.

skill/scripts/lib/live-path-globs.mjs

@@ -1,37 +0,0 @@
-/**
- * Convert a live-config glob pattern to a RegExp.
- *
- * Supports `**` across path segments, `*` within one segment, and `?` for one
- * character. Callers normalize project-relative paths to forward slashes.
- */
-export function livePathGlobToRegex(pattern) {
-  let re = '';
-  let i = 0;
-  while (i < pattern.length) {
-    const c = pattern[i];
-    if (c === '*') {
-      if (pattern[i + 1] === '*') {
-        if (pattern[i + 2] === '/') {
-          re += '(?:.*/)?';
-          i += 3;
-        } else {
-          re += '.*';
-          i += 2;
-        }
-      } else {
-        re += '[^/]*';
-        i += 1;
-      }
-    } else if (c === '?') {
-      re += '[^/]';
-      i += 1;
-    } else if (/[.+^${}()|[\]\\]/.test(c)) {
-      re += `\\${c}`;
-      i += 1;
-    } else {
-      re += c;
-      i += 1;
-    }
-  }
-  return new RegExp(`^${re}$`);
-}

skill/scripts/live-commit-manual-edits.mjs

@@ -944,8 +944,42 @@ export async function commitManualEdits({
     };
   }
 
+  const repairContext = {
+    batch,
+    cwd,
+    pageUrl,
+    count,
+    provider,
+    env,
+    timeoutMs,
+    applyBatchToSource,
+    chatAvailable,
+    transactionId,
+  };
+
   const baseRollbackScope = collectApplyOwnedFiles(batch, cwd);
   const rollbackSnapshot = snapshotRollbackFiles(cwd, baseRollbackScope);
+  const failWithRollback = ({
+    scope = baseRollbackScope,
+    extraFiles = [],
+    failed,
+    files = [],
+    details = {},
+  }) => {
+    const rollback = rollbackChangedFiles(cwd, rollbackSnapshot, extraFiles, scope);
+    return {
+      applied: [],
+      failed,
+      files,
+      cleared: 0,
+      count,
+      pageUrl,
+      ...details,
+      rolledBackFiles: rollback.rolledBackFiles,
+      rollbackFailures: rollback.rollbackFailures,
+      ...countByPage(cwd),
+    };
+  };
   let result;
   try {
     result = repairOnly
@@ -965,42 +999,27 @@ export async function commitManualEdits({
           chatAvailable,
         });
   } catch (err) {
-    const rollback = rollbackChangedFiles(cwd, rollbackSnapshot, [], baseRollbackScope);
-    return {
-      applied: [],
+    return failWithRollback({
       failed: batch.entries.map((entry) => ({
         id: entry.id,
         reason: err.message || String(err),
         candidates: candidatesForEntry(batch, entry.id),
       })),
-      files: [],
-      cleared: 0,
-      count,
-      pageUrl,
-      rolledBackFiles: rollback.rolledBackFiles,
-      rollbackFailures: rollback.rollbackFailures,
-      ...countByPage(cwd),
-    };
+    });
   }
 
   if (result.status === 'error') {
     const rollbackScope = collectApplyOwnedFiles(batch, cwd, result.files || []);
-    const rollback = rollbackChangedFiles(cwd, rollbackSnapshot, result.files || [], rollbackScope);
     const failed = normalizeFailedEntries(batch, result, result.message || 'AI copy edit failed');
-    return {
-      applied: [],
+    return failWithRollback({
+      scope: rollbackScope,
+      extraFiles: result.files || [],
       failed: failed.length > 0
         ? failed
         : verificationFailuresForEntries(batch, batch.entries, result.message || 'AI copy edit failed'),
       files: result.files || [],
-      cleared: 0,
-      count,
-      pageUrl,
-      notes: result.notes || [],
-      rolledBackFiles: rollback.rolledBackFiles,
-      rollbackFailures: rollback.rollbackFailures,
-      ...countByPage(cwd),
-    };
+      details: { notes: result.notes || [] },
+    });
   }
 
   const reportedAppliedIds = uniqueStrings(result.appliedEntryIds || []);
@@ -1013,72 +1032,44 @@ export async function commitManualEdits({
   const conflictingAppliedIds = reportedAppliedIds.filter((id) => failedIds.has(id));
 
   if (conflictingAppliedIds.length > 0) {
-    const rollback = rollbackChangedFiles(cwd, rollbackSnapshot, result.files || [], rollbackScope);
     const conflictingEntries = batch.entries.filter((entry) => conflictingAppliedIds.includes(entry.id));
-    return {
-      applied: [],
+    return failWithRollback({
+      scope: rollbackScope,
+      extraFiles: result.files || [],
       failed: [
         ...verificationFailuresForEntries(batch, conflictingEntries, 'conflicting_apply_result'),
         ...aiFailed.filter((item) => !conflictingAppliedIds.includes(item.id)),
       ],
       files: result.files || [],
-      cleared: 0,
-      count,
-      pageUrl,
-      notes: result.notes || [],
-      rolledBackFiles: rollback.rolledBackFiles,
-      rollbackFailures: rollback.rollbackFailures,
-      ...countByPage(cwd),
-    };
+      details: { notes: result.notes || [] },
+    });
   }
 
   const unreportedFiles = unreportedChangedFiles(cwd, rollbackSnapshot, result.files || [], rollbackScope);
   if (unreportedFiles.length > 0) {
-    const rollback = rollbackChangedFiles(cwd, rollbackSnapshot, result.files || [], [...rollbackScope, ...unreportedFiles]);
-    return {
-      applied: [],
+    return failWithRollback({
+      scope: [...rollbackScope, ...unreportedFiles],
+      extraFiles: result.files || [],
       failed: verificationFailuresForEntries(batch, batch.entries, 'unreported_source_changes', { files: unreportedFiles }),
       files: result.files || [],
-      unreportedFiles,
-      cleared: 0,
-      count,
-      pageUrl,
-      notes: result.notes || [],
-      rolledBackFiles: rollback.rolledBackFiles,
-      rollbackFailures: rollback.rollbackFailures,
-      ...countByPage(cwd),
-    };
+      details: { unreportedFiles, notes: result.notes || [] },
+    });
   }
 
   if (result.status === 'done' && reportedAppliedIds.length === 0) {
-    const rollback = rollbackChangedFiles(cwd, rollbackSnapshot, result.files || [], rollbackScope);
-    return {
-      applied: [],
+    return failWithRollback({
+      scope: rollbackScope,
+      extraFiles: result.files || [],
       failed: verificationFailuresForEntries(batch, batch.entries, 'missing_applied_entry_ids'),
       files: result.files || [],
-      cleared: 0,
-      count,
-      pageUrl,
-      notes: result.notes || [],
-      rolledBackFiles: rollback.rolledBackFiles,
-      rollbackFailures: rollback.rollbackFailures,
-      ...countByPage(cwd),
-    };
+      details: { notes: result.notes || [] },
+    });
   }
 
   const reportedAppliedEntries = batch.entries.filter((entry) => reportedAppliedIds.includes(entry.id));
   if (reportedAppliedIds.length > 0 && reportedFiles.length === 0) {
     return repairPostApplyValidation({
-      batch,
-      cwd,
-      pageUrl,
-      count,
-      provider,
-      env,
-      timeoutMs,
-      applyBatchToSource,
-      chatAvailable,
-      transactionId,
+      ...repairContext,
       appliedEntryIds: reportedAppliedIds,
       files: result.files || [],
       failed: aiFailed,
@@ -1089,21 +1080,10 @@ export async function commitManualEdits({
     });
   }
 
-  const verifiedAppliedIds = [];
-  const verificationFailed = [];
-  for (const entry of reportedAppliedEntries) {
-    const failures = verifyAppliedEntry({ batch, entry, reportedFiles, cwd });
-    if (failures.length === 0) {
-      verifiedAppliedIds.push(entry.id);
-    } else {
-      verificationFailed.push({
-        id: entry.id,
-        reason: 'source_verification_failed',
-        failures,
-        candidates: candidatesForEntry(batch, entry.id),
-      });
-    }
-  }
+  const {
+    verifiedIds: verifiedAppliedIds,
+    failed: verificationFailed,
+  } = verifyEntriesAfterRepair({ batch, appliedEntryIds: reportedAppliedIds, files: reportedFiles, cwd });
   const unreportedEntries = result.status === 'done' || result.status === 'partial'
     ? batch.entries.filter((entry) => !reportedAppliedIds.includes(entry.id) && !aiFailed.some((item) => item.id === entry.id))
     : [];
@@ -1133,37 +1113,22 @@ export async function commitManualEdits({
         reason: 'rolled_back_due_to_failed_entry_source_changed',
         candidates: candidatesForEntry(batch, entry.id),
       }));
-    const rollback = rollbackChangedFiles(cwd, rollbackSnapshot, result.files || [], rollbackScope);
-    return {
-      applied: [],
+    return failWithRollback({
+      scope: rollbackScope,
+      extraFiles: result.files || [],
       failed: [
         ...leakedUnapplied,
         ...failed.filter((item) => !leakedIds.has(item.id)),
         ...rolledBackVerified,
       ],
       files: result.files || [],
-      cleared: 0,
-      count,
-      pageUrl,
-      rolledBackFiles: rollback.rolledBackFiles,
-      rollbackFailures: rollback.rollbackFailures,
-      notes: result.notes || [],
-      ...countByPage(cwd),
-    };
+      details: { notes: result.notes || [] },
+    });
   }
 
   if (verificationFailed.length > 0) {
     return repairPostApplyValidation({
-      batch,
-      cwd,
-      pageUrl,
-      count,
-      provider,
-      env,
-      timeoutMs,
-      applyBatchToSource,
-      chatAvailable,
-      transactionId,
+      ...repairContext,
       appliedEntryIds: reportedAppliedIds,
       files: result.files || [],
       failed: nonRepairFailed,
@@ -1180,16 +1145,7 @@ export async function commitManualEdits({
       ? reportedAppliedEntries.filter((entry) => verifiedAppliedIds.includes(entry.id))
       : batch.entries;
     return repairPostApplyValidation({
-      batch,
-      cwd,
-      pageUrl,
-      count,
-      provider,
-      env,
-      timeoutMs,
-      applyBatchToSource,
-      chatAvailable,
-      transactionId,
+      ...repairContext,
       appliedEntryIds: verifiedAppliedIds.length > 0
         ? verifiedAppliedIds
         : postCheckEntries.map((entry) => entry.id).filter(Boolean),

skill/scripts/live-inject.mjs

@@ -27,7 +27,6 @@ import fs from 'node:fs';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { resolveLiveConfigPath } from './lib/impeccable-paths.mjs';
-import { livePathGlobToRegex } from './lib/live-path-globs.mjs';
 import {
   describeInjectArtifacts,
   frameworkIgnorePatterns,
@@ -365,7 +364,7 @@ export function resolveFiles(rootDir, config) {
   const patterns = config.files;
   const userExcludes = Array.isArray(config.exclude) ? config.exclude : [];
   const allExcludes = [...HARD_EXCLUDES, ...userExcludes];
-  const excludeRegexes = allExcludes.map(livePathGlobToRegex);
+  const excludeRegexes = allExcludes.map(globToRegex);
 
   const isExcluded = (relPath) => excludeRegexes.some((re) => re.test(relPath));
   const isGlob = (s) => /[*?[]/.test(s);
@@ -402,6 +401,47 @@ export function resolveFiles(rootDir, config) {
   return out;
 }
 
+/**
+ * Convert a glob pattern to a RegExp. Supports:
+ *   **  → any number of path segments (including zero)
+ *   *   → any chars except `/`
+ *   ?   → any single char except `/`
+ * Paths are normalized to forward slashes before matching.
+ */
+function globToRegex(pattern) {
+  let re = '';
+  let i = 0;
+  while (i < pattern.length) {
+    const c = pattern[i];
+    if (c === '*') {
+      if (pattern[i + 1] === '*') {
+        // ** — any number of segments, including zero. Handle the common
+        // **/ and /** forms so `a/**/b` matches `a/b` as well as `a/x/y/b`.
+        if (pattern[i + 2] === '/') {
+          re += '(?:.*/)?';
+          i += 3;
+        } else {
+          re += '.*';
+          i += 2;
+        }
+      } else {
+        re += '[^/]*';
+        i += 1;
+      }
+    } else if (c === '?') {
+      re += '[^/]';
+      i += 1;
+    } else if (/[.+^${}()|[\]\\]/.test(c)) {
+      re += '\\' + c;
+      i += 1;
+    } else {
+      re += c;
+      i += 1;
+    }
+  }
+  return new RegExp('^' + re + '$');
+}
+
 // ---------------------------------------------------------------------------
 // Core operations
 // ---------------------------------------------------------------------------

skill/scripts/live.mjs

@@ -24,7 +24,6 @@ import { fileURLToPath } from 'node:url';
 import { resolveTargetSelection } from './context.mjs';
 import { resolveFiles } from './live-inject.mjs';
 import { readLiveServerInfo } from './lib/impeccable-paths.mjs';
-import { livePathGlobToRegex } from './lib/live-path-globs.mjs';
 import { resolveSurfaceBrief } from './lib/surface-briefs.mjs';
 import { resolveLiveTarget } from './live-target.mjs';
 import { bootInstructions } from './live/instructions.mjs';
@@ -241,7 +240,7 @@ function scanForDrift(rootDir, resolvedFiles, config) {
   // Files matching the user's `exclude` globs are intentional omissions,
   // not drift. Compile them to regexes so the orphan list stays signal.
   const userExcludeRegexes = (Array.isArray(config.exclude) ? config.exclude : [])
-    .map(livePathGlobToRegex);
+    .map((p) => globToRegex(p));
   const isUserExcluded = (rel) => userExcludeRegexes.some((re) => re.test(rel));
 
   const orphans = [];
@@ -279,6 +278,38 @@ function scanForDrift(rootDir, resolvedFiles, config) {
   };
 }
 
+/**
+ * Same glob-to-regex mapping used by live-inject.mjs. Kept inline here
+ * to avoid a circular import (live-inject.mjs already imports nothing
+ * from live.mjs). The two must stay in sync.
+ */
+function globToRegex(pattern) {
+  let re = '';
+  let i = 0;
+  while (i < pattern.length) {
+    const c = pattern[i];
+    if (c === '*') {
+      if (pattern[i + 1] === '*') {
+        if (pattern[i + 2] === '/') { re += '(?:.*/)?'; i += 3; }
+        else { re += '.*'; i += 2; }
+      } else {
+        re += '[^/]*';
+        i += 1;
+      }
+    } else if (c === '?') {
+      re += '[^/]';
+      i += 1;
+    } else if (/[.+^${}()|[\]\\]/.test(c)) {
+      re += '\\' + c;
+      i += 1;
+    } else {
+      re += c;
+      i += 1;
+    }
+  }
+  return new RegExp('^' + re + '$');
+}
+
 // ---------------------------------------------------------------------------
 // Helpers
 // ---------------------------------------------------------------------------

tests/live-inject.test.mjs

@@ -10,38 +10,10 @@ import { dirname, join, relative, resolve } from 'node:path';
 import { tmpdir } from 'node:os';
 import { fileURLToPath } from 'node:url';
 import { execFileSync } from 'node:child_process';
-import { livePathGlobToRegex } from '../skill/scripts/lib/live-path-globs.mjs';
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 const INJECT = resolve(__dirname, '..', 'skill/scripts/live-inject.mjs');
 
-describe('live path globs', () => {
-  it('matches recursive segments, including zero segments', () => {
-    const anywhere = livePathGlobToRegex('**/index.html');
-    assert.equal(anywhere.test('index.html'), true);
-    assert.equal(anywhere.test('public/index.html'), true);
-    assert.equal(anywhere.test('apps/web/public/index.html'), true);
-
-    const underPublic = livePathGlobToRegex('public/**/*.html');
-    assert.equal(underPublic.test('public/index.html'), true);
-    assert.equal(underPublic.test('public/docs/index.html'), true);
-    assert.equal(underPublic.test('src/index.html'), false);
-  });
-
-  it('keeps single-star and question-mark matches inside one segment', () => {
-    const pattern = livePathGlobToRegex('pages/*/item?.html');
-    assert.equal(pattern.test('pages/docs/item1.html'), true);
-    assert.equal(pattern.test('pages/docs/deep/item1.html'), false);
-    assert.equal(pattern.test('pages/docs/item12.html'), false);
-  });
-
-  it('treats regular-expression punctuation as literal path text', () => {
-    const pattern = livePathGlobToRegex('pages/[draft]/item+.html');
-    assert.equal(pattern.test('pages/[draft]/item+.html'), true);
-    assert.equal(pattern.test('pages/d/itemm.html'), false);
-  });
-});
-
 function runInject(cwd, configPath, args) {
   try {
     const out = execFileSync('node', [INJECT, ...args], {