fix(hooks): create real git worktrees and clean them up in worktree hooks

scripts/worktree-create.js

@@ -2,13 +2,75 @@
 const fs = require('fs');
 const path = require('path');
 const os = require('os');
+const crypto = require('crypto');
+const { execFileSync } = require('child_process');
 
 process.stdin.setEncoding('utf8');
 let data = '';
 process.stdin.on('data', chunk => { data += chunk; });
 process.stdin.on('end', () => {
+  // WorktreeCreate contract: a registered hook owns worktree creation outright.
+  // The harness does no git setup itself. The hook must echo ONLY the path of a
+  // directory that already exists (empty stdout is an error too), and the agent
+  // works directly in that directory, so it has to be a real git worktree
+  // whenever the session is inside a repo.
+  const baseDir = path.join(os.tmpdir(), 'pro-workflow', 'worktrees');
+
+  // Git ref names can't start with '.', contain '..', or end in '.lock'. Strip
+  // disallowed characters first, then collapse anything left that would still
+  // fail `git check-ref-format`.
+  const sanitizeRefComponent = (rawName) => {
+    let name = String(rawName || 'worktree').replace(/[^A-Za-z0-9._-]/g, '-');
+    name = name.replace(/\.+/g, '.').replace(/^\.+/, '').replace(/\.lock$/, '-lock');
+    return name || 'worktree';
+  };
+
+  const createWorktree = (rawName, cwdHint) => {
+    const ts = Date.now();
+    const unique = `${ts}-${crypto.randomBytes(3).toString('hex')}`;
+    const name = sanitizeRefComponent(rawName);
+    const target = path.join(baseDir, `${name}-${unique}`);
+    const branch = `agents/${name}-${unique}`;
+    const cwd = cwdHint && fs.existsSync(cwdHint) ? cwdHint : process.cwd();
+
+    let inRepo = false;
+    try {
+      execFileSync('git', ['-C', cwd, 'rev-parse', '--git-dir'], { stdio: 'ignore' });
+      inRepo = true;
+    } catch (err) {
+      // Genuinely not a git repo: a plain directory is the correct outcome.
+    }
+
+    if (inRepo) {
+      try {
+        fs.mkdirSync(baseDir, { recursive: true });
+        execFileSync('git', ['-C', cwd, 'worktree', 'add', '-b', branch, target], { stdio: 'pipe' });
+        return { target, branch, repo: cwd };
+      } catch (err) {
+        // We ARE in a repo but `worktree add` still failed (ref collision, disk,
+        // git version, etc.). Never block the spawn over this, but make the
+        // failure visible instead of silently handing back an empty directory
+        // that looks identical to the "not a repo" case.
+        console.error(`[ProWorkflow] git worktree add failed, falling back to a plain directory: ${(err.stderr || err.message || err).toString().trim()}`);
+      }
+    }
+
+    fs.mkdirSync(target, { recursive: true });
+    return { target, branch: null, repo: null };
+  };
+
+  let input = {};
+  try { input = JSON.parse(data); } catch (e) { input = {}; }
+
+  let created;
+  try {
+    created = createWorktree(input.name, input.cwd);
+  } catch (err) {
+    process.exit(1);
+  }
+  const worktreePath = created.target;
+
   try {
-    const input = JSON.parse(data);
     const tempDir = path.join(os.tmpdir(), 'pro-workflow');
     if (!fs.existsSync(tempDir)) fs.mkdirSync(tempDir, { recursive: true });
 
@@ -21,18 +83,19 @@ process.stdin.on('end', () => {
     worktrees.push({
       timestamp: new Date().toISOString(),
       session_id: input.session_id || 'unknown',
-      worktree_path: input.worktree_path || 'unknown',
-      branch: input.branch || 'unknown'
+      worktree_path: worktreePath,
+      branch: created.branch,
+      repo: created.repo
     });
 
     if (worktrees.length > 100) worktrees = worktrees.slice(-100);
     fs.writeFileSync(worktreeLog, JSON.stringify(worktrees, null, 2));
 
-    console.error(`[ProWorkflow] Worktree created: ${input.branch || 'unknown'}`);
+    console.error(`[ProWorkflow] Worktree created: ${worktreePath}`);
     console.error('[ProWorkflow] Isolated workspace ready for parallel work');
-
-    console.log(data);
   } catch (err) {
-    console.log(data || '{}');
+    // Logging failures must never block the spawn.
   }
+
+  console.log(worktreePath);
 });

scripts/worktree-remove.js

@@ -1,29 +1,100 @@
 #!/usr/bin/env node
+// WorktreeRemove hook: cleanup counterpart to worktree-create.js.
+// Input (stdin JSON): { session_id, transcript_path, cwd, hook_event_name,
+// worktree_path } where worktree_path is the path worktree-create.js echoed.
+// Best-effort: remove the git worktree, delete the matching agents/* branch,
+// prune stale admin entries, drop the ledger row. Never blocks the harness:
+// always exits 0 and echoes stdin, matching the original script's contract.
 const fs = require('fs');
 const path = require('path');
 const os = require('os');
+const { execFileSync } = require('child_process');
+
+const tempDir = path.join(os.tmpdir(), 'pro-workflow');
+const baseDir = path.join(tempDir, 'worktrees');
+const worktreeLog = path.join(tempDir, 'worktrees.json');
+
+const git = (args, cwd) => execFileSync('git', args, {
+  cwd: cwd || process.cwd(),
+  stdio: ['ignore', 'pipe', 'ignore'],
+  timeout: 8000
+}).toString().trim();
+
+const tryGit = (args, cwd) => { try { return git(args, cwd); } catch (e) { return null; } };
+
+function cleanup(input, raw) {
+  const wt = input.worktree_path || input.worktreePath || input.path || '';
+
+  // Capture the raw payload so future harness schema changes are diagnosable.
+  try {
+    fs.mkdirSync(tempDir, { recursive: true });
+    fs.appendFileSync(path.join(tempDir, 'worktree-remove.log'),
+      `${new Date().toISOString()} ${String(raw).replace(/\s+/g, ' ').slice(0, 2000)}\n`);
+  } catch (e) { /* ignore */ }
+
+  let ledger = [];
+  try { ledger = JSON.parse(fs.readFileSync(worktreeLog, 'utf8')); } catch (e) { ledger = []; }
+  if (!Array.isArray(ledger)) ledger = [];
+  const entry = ledger.find(w => w && w.worktree_path === wt) || null;
+
+  const resolved = wt ? path.resolve(wt) : '';
+  // Only paths under our own baseDir are ever deleted from disk.
+  const ours = !!resolved && resolved.startsWith(baseDir + path.sep);
+
+  // Identify the branch before the directory disappears. Fallbacks cover the
+  // case where the harness (or a previous attempt) already removed the dir:
+  // the ledger row, then the create hook's own naming scheme.
+  let branch = null;
+  if (wt && fs.existsSync(wt)) branch = tryGit(['branch', '--show-current'], wt);
+  if (!branch && entry && typeof entry.branch === 'string') branch = entry.branch;
+  if (!branch && ours) branch = 'agents/' + path.basename(resolved);
+
+  // Locate the main repo: from the worktree's git-common-dir, the ledger,
+  // or the session cwd the payload carries.
+  let repo = null;
+  if (wt && fs.existsSync(wt)) {
+    const common = tryGit(['rev-parse', '--git-common-dir'], wt);
+    if (common) repo = path.dirname(path.resolve(wt, common));
+  }
+  if (!repo && entry && typeof entry.repo === 'string' && fs.existsSync(entry.repo)) repo = entry.repo;
+  if (!repo && input.cwd && fs.existsSync(input.cwd)) repo = input.cwd;
+
+  // Only touch git state when we've confirmed ownership: either the path
+  // resolves under our own baseDir, or the ledger has a matching row for it.
+  // `repo` alone isn't enough to gate on — it can resolve via input.cwd (the
+  // session's cwd), which may be the user's main repo, not a worktree we made.
+  const owned = ours || !!entry;
+
+  if (owned) {
+    if (repo) tryGit(['worktree', 'remove', '--force', resolved], repo);
+    try {
+      if (fs.existsSync(resolved)) fs.rmSync(resolved, { recursive: true, force: true });
+    } catch (e) { /* ignore */ }
+  }
+  if (owned && repo) {
+    tryGit(['worktree', 'prune'], repo);
+    // agents/* is the namespace worktree-create.js owns; never touch others.
+    if (branch && /^agents\//.test(branch)) tryGit(['branch', '-D', branch], repo);
+  }
+
+  try {
+    if (entry) {
+      fs.writeFileSync(worktreeLog, JSON.stringify(ledger.filter(w => w !== entry), null, 2));
+    }
+  } catch (e) { /* ignore */ }
+
+  console.error(`[ProWorkflow] Worktree removed: ${wt || 'unknown'}${branch ? ` (branch ${branch})` : ''}`);
+}
 
 process.stdin.setEncoding('utf8');
 let data = '';
 process.stdin.on('data', chunk => { data += chunk; });
 process.stdin.on('end', () => {
-  try {
-    const input = JSON.parse(data);
-
-    console.error(`[ProWorkflow] Worktree removed: ${input.worktree_path || 'unknown'}`);
-
-    const tempDir = path.join(os.tmpdir(), 'pro-workflow');
-    const worktreeLog = path.join(tempDir, 'worktrees.json');
-    if (fs.existsSync(worktreeLog)) {
-      try {
-        let worktrees = JSON.parse(fs.readFileSync(worktreeLog, 'utf8'));
-        worktrees = worktrees.filter(w => w.worktree_path !== input.worktree_path);
-        fs.writeFileSync(worktreeLog, JSON.stringify(worktrees, null, 2));
-      } catch (e) { /* ignore */ }
-    }
-
-    console.log(data);
-  } catch (err) {
-    console.log(data || '{}');
-  }
+  let input = {};
+  try { input = JSON.parse(data); } catch (e) { input = {}; }
+  try { cleanup(input, data); } catch (e) { /* cleanup is best-effort, never block */ }
+  // No process.exit() here: stdout to a pipe is written asynchronously by
+  // Node, and exit() doesn't wait for the flush. Letting the event loop drain
+  // naturally (no open handles remain) guarantees the payload isn't truncated.
+  console.log(data || '{}');
 });