diff --git a/server/file-chat.js b/server/file-chat.js index cf029a4..1a3da6a 100644 --- a/server/file-chat.js +++ b/server/file-chat.js @@ -175,7 +175,12 @@ function appendMessage(projectId, { sender, channel = "general", text, type = "m console.error(`[file-chat] Append failure for project ${projectId}: ${err.message}`); throw err; } - Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 100); + // #975: retry immediately — no sleep. The previous + // Atomics.wait(...,100) was a SYNCHRONOUS 100ms block on the main + // thread (up to 300ms per append) on every chat + system message; in + // this single-process server that froze every agent's WS/HTTP/timers. + // appendFileSync transient failures are rare and not time-dependent, so + // the busy-wait bought nothing. Attempt count is unchanged. } } diff --git a/server/index.js b/server/index.js index 3c5cfd4..35ed3e1 100644 --- a/server/index.js +++ b/server/index.js @@ -1006,12 +1006,12 @@ function spawnButlerPty() { args.push(...flags); } - // Pre-trust the Butler working directory to avoid blocking trust prompt - if (command === "claude") { - try { - execFileSync("claude", ["-p", "echo ok"], { cwd: docsDir, timeout: 15000, stdio: "pipe" }); - } catch {} - } + // #975: the old pre-trust ran `claude -p "echo ok"` synchronously with a + // 15s timeout on the event loop — a hung claude blackholed every agent's + // WS/HTTP/timers for up to 15s. Dropped: the interactive trust prompt is + // already auto-answered by the onData trust-listener installed below (the + // same mechanism the agent PTYs rely on), so no synchronous pre-trust is + // needed to reach a trusted session. const seedPath = path.join(__dirname, "..", "templates", "seeds", "butler.CLAUDE.md"); const claudePath = path.join(docsDir, "CLAUDE.md"); diff --git a/server/routes.js b/server/routes.js index bb256d6..0c6f334 100644 --- a/server/routes.js +++ b/server/routes.js @@ -3683,10 +3683,15 @@ router.get("/api/batch-progress", async (req, res) => { // ─── Setup ───────────────────────────────────────────────────────────────── -function exec(cmd, args, opts) { +// #975: setup shell-outs run OFF the event loop via the promisified execFile, +// so a slow/hung `gh clone` / `git fetch` / `git worktree add` / `claude -p` no +// longer blocks every agent's WS/HTTP/timers in this single-process server. +// Returns { ok, output } (the old synchronous exec()'s contract) so call sites +// only needed an `await`; setup fns take this as an injectable execFn for tests. +async function execAsync(cmd, args, opts) { try { - const out = execFileSync(cmd, args, { encoding: "utf-8", timeout: 30000, ...opts }); - return { ok: true, output: out.trim() }; + const { stdout } = await _execFileAsync(cmd, args, { encoding: "utf-8", timeout: 30000, ...opts }); + return { ok: true, output: (stdout || "").trim() }; } catch (err) { return { ok: false, output: String(err.stderr || err.stdout || err.message || "").trim() }; } @@ -3700,22 +3705,25 @@ function repoSlugFromRemote(url) { return m ? m[1].toLowerCase() : ""; } -function ensureGitHeadForSetup(workingDir, repo) { +// #975: async — awaits each git/gh shell-out via the injectable execFn +// (default execAsync) so the setup route never blocks the event loop. execFn +// keeps the exec() { ok, output } contract; tests inject a fake. +async function ensureGitHeadForSetup(workingDir, repo, execFn = execAsync) { const gitDir = path.join(workingDir, ".git"); if (!fs.existsSync(gitDir)) { if (!fs.existsSync(workingDir)) ensureSecureDir(workingDir); if (!REPO_RE.test(repo)) return { ok: false, error: "Invalid repo" }; - const clone = exec("gh", ["repo", "clone", repo, workingDir]); + const clone = await execFn("gh", ["repo", "clone", repo, workingDir]); if (!clone.ok) return { ok: false, error: `Clone failed: ${clone.output}` }; } else { - const fetch = exec("git", ["fetch", "origin", "--prune"], { cwd: workingDir }); + const fetch = await execFn("git", ["fetch", "origin", "--prune"], { cwd: workingDir }); if (!fetch.ok) return { ok: false, error: `Fetch failed: ${fetch.output}` }; } // #974: fail clearly if the working dir is a clone of a DIFFERENT repo than // the slug entered — otherwise setup silently seeds worktrees into the wrong // project. (Same guard as the CLI wizard.) - const originUrl = exec("git", ["remote", "get-url", "origin"], { cwd: workingDir }); + const originUrl = await execFn("git", ["remote", "get-url", "origin"], { cwd: workingDir }); if (originUrl.ok) { const actual = repoSlugFromRemote(originUrl.output); const expected = String(repo || "").toLowerCase().replace(/\.git$/, ""); @@ -3724,25 +3732,25 @@ function ensureGitHeadForSetup(workingDir, repo) { } } - let headCheck = exec("git", ["rev-parse", "--verify", "HEAD"], { cwd: workingDir }); + let headCheck = await execFn("git", ["rev-parse", "--verify", "HEAD"], { cwd: workingDir }); if (!headCheck.ok) { - const remoteHead = exec("git", ["symbolic-ref", "--quiet", "--short", "refs/remotes/origin/HEAD"], { cwd: workingDir }); + const remoteHead = await execFn("git", ["symbolic-ref", "--quiet", "--short", "refs/remotes/origin/HEAD"], { cwd: workingDir }); let remoteBranch = remoteHead.ok ? remoteHead.output.replace(/^origin\//, "") : ""; if (!remoteBranch) { for (const candidate of ["main", "master"]) { - const hasBranch = exec("git", ["rev-parse", "--verify", `origin/${candidate}`], { cwd: workingDir }); + const hasBranch = await execFn("git", ["rev-parse", "--verify", `origin/${candidate}`], { cwd: workingDir }); if (hasBranch.ok) { remoteBranch = candidate; break; } } } if (remoteBranch) { - const checkout = exec("git", ["checkout", "-B", remoteBranch, `origin/${remoteBranch}`], { cwd: workingDir }); + const checkout = await execFn("git", ["checkout", "-B", remoteBranch, `origin/${remoteBranch}`], { cwd: workingDir }); if (!checkout.ok) return { ok: false, error: `Checkout failed after fetch: ${checkout.output}` }; - headCheck = exec("git", ["rev-parse", "--verify", "HEAD"], { cwd: workingDir }); + headCheck = await execFn("git", ["rev-parse", "--verify", "HEAD"], { cwd: workingDir }); } } if (headCheck.ok) return { ok: true }; - const seed = exec("git", [ + const seed = await execFn("git", [ "-c", "user.name=QuadWork", "-c", "user.email=quadwork@localhost", "commit", "--allow-empty", "-m", "Initial commit (created by QuadWork setup)", @@ -3750,9 +3758,9 @@ function ensureGitHeadForSetup(workingDir, repo) { if (!seed.ok) { return { ok: false, error: `Repository is empty and could not be initialized: ${seed.output}` }; } - const branchResult = exec("git", ["symbolic-ref", "--short", "HEAD"], { cwd: workingDir }); + const branchResult = await execFn("git", ["symbolic-ref", "--short", "HEAD"], { cwd: workingDir }); const defaultBranch = branchResult.ok ? branchResult.output : "main"; - const push = exec("git", ["push", "origin", defaultBranch], { cwd: workingDir }); + const push = await execFn("git", ["push", "origin", defaultBranch], { cwd: workingDir }); if (!push.ok) return { ok: false, error: `Initial commit created but push failed: ${push.output}` }; return { ok: true }; } @@ -3763,18 +3771,21 @@ function ensureGitHeadForSetup(workingDir, repo) { // exists" and bricked a re-run with no rollback. Reuse an existing branch // instead — prune any stale worktree registration still holding it, then // `worktree add` re-attaches. `execFn` is injectable for tests. -function createAgentWorktree(workingDir, wtDir, branchName, execFn = exec) { - const branchExists = execFn("git", ["rev-parse", "--verify", "--quiet", `refs/heads/${branchName}`], { cwd: workingDir }).ok; +// #975: async (default execFn = execAsync) so `git worktree add` runs off the +// event loop. `await execFn(...)` works whether execFn is async (production) or +// returns a plain object (test fakes). +async function createAgentWorktree(workingDir, wtDir, branchName, execFn = execAsync) { + const branchExists = (await execFn("git", ["rev-parse", "--verify", "--quiet", `refs/heads/${branchName}`], { cwd: workingDir })).ok; if (branchExists) { - execFn("git", ["worktree", "prune"], { cwd: workingDir }); + await execFn("git", ["worktree", "prune"], { cwd: workingDir }); } else { - const branch = execFn("git", ["branch", branchName, "HEAD"], { cwd: workingDir }); + const branch = await execFn("git", ["branch", branchName, "HEAD"], { cwd: workingDir }); if (!branch.ok) return { ok: false, error: `branch failed: ${branch.output}` }; } - const result = execFn("git", ["worktree", "add", wtDir, branchName], { cwd: workingDir }); + const result = await execFn("git", ["worktree", "add", wtDir, branchName], { cwd: workingDir }); if (result.ok) return { ok: true, detached: false }; // Fallback: detached worktree (branch may be checked out in a live worktree). - const result2 = execFn("git", ["worktree", "add", "--detach", wtDir, "HEAD"], { cwd: workingDir }); + const result2 = await execFn("git", ["worktree", "add", "--detach", wtDir, "HEAD"], { cwd: workingDir }); if (result2.ok) return { ok: true, detached: true }; return { ok: false, error: result.output }; } @@ -3866,7 +3877,11 @@ router.get("/api/setup/reviewer-token-status", (_req, res) => { // ─── Setup Wizard ───────────────────────────────────────────────────────── -router.post("/api/setup", (req, res) => { +// #975: async handler — setup shell-outs (gh/git/claude) now await execAsync +// so they run off the event loop instead of freezing every agent's WS/HTTP/ +// timers in this single-process server. Express 5 forwards a rejected handler +// to the default error handler (same 500 the prior sync throws produced). +router.post("/api/setup", async (req, res) => { const step = req.query.step; const body = req.body || {}; @@ -3874,7 +3889,7 @@ router.post("/api/setup", (req, res) => { case "verify-repo": { const repo = body.repo; if (!repo || !REPO_RE.test(repo)) return res.json({ ok: false, error: "Invalid repo format (use owner/repo)" }); - const result = exec("gh", ["repo", "view", repo, "--json", "name,owner,viewerPermission"]); + const result = await execAsync("gh", ["repo", "view", repo, "--json", "name,owner,viewerPermission"]); if (!result.ok) return res.json({ ok: false, error: "Cannot access repo. Check gh auth and repo permissions." }); try { const info = JSON.parse(result.output); @@ -3888,7 +3903,7 @@ router.post("/api/setup", (req, res) => { case "create-worktrees": { const workingDir = body.workingDir; if (!workingDir) return res.json({ ok: false, error: "Missing working directory" }); - const ready = ensureGitHeadForSetup(workingDir, body.repo); + const ready = await ensureGitHeadForSetup(workingDir, body.repo); if (!ready.ok) return res.json({ ok: false, error: ready.error }); // Sibling dirs: ../projectName-head/, ../projectName-re1/, etc. (matches CLI wizard) const projectName = path.basename(workingDir); @@ -3902,7 +3917,7 @@ router.post("/api/setup", (req, res) => { const branchName = `worktree-${agent}`; // #974: reuse an existing branch (see createAgentWorktree) so a re-run // after a deleted worktree dir no longer aborts on "branch exists". - const wt = createAgentWorktree(workingDir, wtDir, branchName); + const wt = await createAgentWorktree(workingDir, wtDir, branchName); if (!wt.ok) { errors.push(`${agent}: ${wt.error}`); continue; } created.push(wt.detached ? `${agent} (detached)` : agent); } @@ -3912,12 +3927,14 @@ router.post("/api/setup", (req, res) => { const agentBackends = body.backends || {}; const claudeAgents = agents.filter((a) => (agentBackends[a] || "claude") === "claude"); if (claudeAgents.length > 0) { - const claudePath = exec("which", ["claude"]); + const claudePath = await execAsync("which", ["claude"]); if (claudePath.ok) { for (const agent of claudeAgents) { const wtDir = path.join(parentDir, `${projectName}-${agent}`); if (!fs.existsSync(wtDir)) continue; - exec("claude", ["-p", "echo ok"], { cwd: wtDir, timeout: 15000, stdio: "pipe" }); + // #975: await off the event loop. A hung `claude -p` (15s timeout) + // per agent used to block the whole server; now it yields. + await execAsync("claude", ["-p", "echo ok"], { cwd: wtDir, timeout: 15000, stdio: "pipe" }); } } } diff --git a/server/routes.setupHardening.test.js b/server/routes.setupHardening.test.js index 8bf3e2a..45bcb0e 100644 --- a/server/routes.setupHardening.test.js +++ b/server/routes.setupHardening.test.js @@ -1,5 +1,7 @@ // #974: setup hardening — origin-mismatch guard, repo-slug normalization, and // re-runnable worktree creation (reuse an existing branch instead of aborting). +// #975: the setup functions are now async (execFn injected) so the setup route +// never blocks the event loop; tests inject a fake async exec and await. // // Plain node:assert script — run with // `node server/routes.setupHardening.test.js`. @@ -7,30 +9,12 @@ const assert = require("node:assert/strict"); const fs = require("fs"); const path = require("path"); -const cp = require("child_process"); -const realExecFileSync = cp.execFileSync; const realExistsSync = fs.existsSync; const realMkdirSync = fs.mkdirSync; const realChmodSync = fs.chmodSync; let existing = new Set(); -let commands = []; -let originUrl = "https://github.com/owner/repo.git"; - -// Stub only git/gh; everything else falls through to the real impl. -cp.execFileSync = function stubExecFileSync(cmd, args, opts = {}) { - commands.push({ cmd, args: args.slice(), cwd: opts.cwd || null }); - if (cmd !== "git" && cmd !== "gh") return realExecFileSync.apply(this, arguments); - const joined = args.join(" "); - if (cmd === "git" && joined === "fetch origin --prune") return ""; - if (cmd === "git" && joined === "remote get-url origin") return `${originUrl}\n`; - if (cmd === "git" && joined === "rev-parse --verify HEAD") return "abc123\n"; - const err = new Error(`unexpected command: ${cmd} ${joined}`); - err.stderr = Buffer.from(err.message); - throw err; -}; - fs.existsSync = (p) => existing.has(path.normalize(p)); fs.mkdirSync = () => {}; fs.chmodSync = () => {}; @@ -38,100 +22,104 @@ fs.chmodSync = () => {}; const routes = require("./routes"); const { ensureGitHeadForSetup, createAgentWorktree, repoSlugFromRemote } = routes; -function reset() { - commands = []; - existing = new Set(); - originUrl = "https://github.com/owner/repo.git"; +// A fake async exec: records calls, returns { ok, output } from a lookup keyed +// on the joined args (default { ok: true, output: "" }). Mirrors the exec() +// contract without touching a real shell. +function recordingExec(results, calls) { + return async (cmd, args) => { + calls.push(args.join(" ")); + const key = args.join(" "); + return results[key] !== undefined ? results[key] : { ok: true, output: "" }; + }; } let passed = 0; -try { - // ── repoSlugFromRemote normalization ── - reset(); - assert.equal(repoSlugFromRemote("https://github.com/owner/repo.git"), "owner/repo", "https + .git"); - assert.equal(repoSlugFromRemote("https://github.com/Owner/Repo"), "owner/repo", "https, no .git, lowercased"); - assert.equal(repoSlugFromRemote("git@github.com:owner/repo.git"), "owner/repo", "ssh scp-form"); - assert.equal(repoSlugFromRemote("ssh://git@github.com/owner/repo.git"), "owner/repo", "ssh url"); - assert.equal(repoSlugFromRemote("https://github.com/owner/repo/"), "owner/repo", "trailing slash"); - assert.equal(repoSlugFromRemote(""), "", "empty"); - console.log(" PASS: repoSlugFromRemote normalizes common remote URL forms"); - passed++; - // ── ensureGitHeadForSetup: origin matches → proceeds to HEAD verify ── - reset(); - existing.add(path.normalize("/tmp/proj/.git")); - originUrl = "https://github.com/owner/repo.git"; - let result = ensureGitHeadForSetup("/tmp/proj", "owner/repo"); - assert.equal(result.ok, true, "matching origin proceeds"); - assert.ok(commands.some((c) => c.args.join(" ") === "remote get-url origin"), "origin is checked"); - assert.ok(commands.some((c) => c.args.join(" ") === "rev-parse --verify HEAD"), "HEAD verified after origin match"); - console.log(" PASS: matching origin passes the guard and verifies HEAD"); - passed++; +(async () => { + try { + // ── repoSlugFromRemote normalization ── + assert.equal(repoSlugFromRemote("https://github.com/owner/repo.git"), "owner/repo", "https + .git"); + assert.equal(repoSlugFromRemote("https://github.com/Owner/Repo"), "owner/repo", "https, no .git, lowercased"); + assert.equal(repoSlugFromRemote("git@github.com:owner/repo.git"), "owner/repo", "ssh scp-form"); + assert.equal(repoSlugFromRemote("ssh://git@github.com/owner/repo.git"), "owner/repo", "ssh url"); + assert.equal(repoSlugFromRemote("https://github.com/owner/repo/"), "owner/repo", "trailing slash"); + assert.equal(repoSlugFromRemote(""), "", "empty"); + console.log(" PASS: repoSlugFromRemote normalizes common remote URL forms"); + passed++; - // ── ensureGitHeadForSetup: origin mismatch → clear error, no HEAD work ── - reset(); - existing.add(path.normalize("/tmp/proj/.git")); - originUrl = "git@github.com:someone-else/other.git"; - result = ensureGitHeadForSetup("/tmp/proj", "owner/repo"); - assert.equal(result.ok, false, "mismatched origin fails"); - assert.match(result.error, /Origin mismatch/, "error names the mismatch"); - assert.match(result.error, /someone-else\/other/, "error reports the actual slug"); - assert.ok(!commands.some((c) => c.args.join(" ") === "rev-parse --verify HEAD"), "aborts before HEAD work"); - console.log(" PASS: mismatched origin fails clearly before seeding anything"); - passed++; + // ── ensureGitHeadForSetup: origin matches → proceeds to HEAD verify ── + existing = new Set([path.normalize("/tmp/proj/.git")]); + let calls = []; + let result = await ensureGitHeadForSetup("/tmp/proj", "owner/repo", recordingExec({ + "fetch origin --prune": { ok: true, output: "" }, + "remote get-url origin": { ok: true, output: "https://github.com/owner/repo.git" }, + "rev-parse --verify HEAD": { ok: true, output: "abc123" }, + }, calls)); + assert.equal(result.ok, true, "matching origin proceeds"); + assert.ok(calls.includes("remote get-url origin"), "origin is checked"); + assert.ok(calls.includes("rev-parse --verify HEAD"), "HEAD verified after origin match"); + console.log(" PASS: matching origin passes the guard and verifies HEAD"); + passed++; - // ── createAgentWorktree: fresh branch (none exists) ── - const calls = []; - const fakeExec = (results) => (cmd, args) => { - calls.push(args.join(" ")); - const key = args.join(" "); - return results[key] !== undefined ? results[key] : { ok: true, output: "" }; - }; + // ── ensureGitHeadForSetup: origin mismatch → clear error, no HEAD work ── + existing = new Set([path.normalize("/tmp/proj/.git")]); + calls = []; + result = await ensureGitHeadForSetup("/tmp/proj", "owner/repo", recordingExec({ + "fetch origin --prune": { ok: true, output: "" }, + "remote get-url origin": { ok: true, output: "git@github.com:someone-else/other.git" }, + }, calls)); + assert.equal(result.ok, false, "mismatched origin fails"); + assert.match(result.error, /Origin mismatch/, "error names the mismatch"); + assert.match(result.error, /someone-else\/other/, "error reports the actual slug"); + assert.ok(!calls.includes("rev-parse --verify HEAD"), "aborts before HEAD work"); + console.log(" PASS: mismatched origin fails clearly before seeding anything"); + passed++; - calls.length = 0; - let wt = createAgentWorktree("/wd", "/wd-dev", "worktree-dev", fakeExec({ - "rev-parse --verify --quiet refs/heads/worktree-dev": { ok: false, output: "" }, - "branch worktree-dev HEAD": { ok: true, output: "" }, - "worktree add /wd-dev worktree-dev": { ok: true, output: "" }, - })); - assert.deepEqual(wt, { ok: true, detached: false }, "fresh branch created + attached"); - assert.ok(calls.includes("branch worktree-dev HEAD"), "creates the branch when absent"); - assert.ok(!calls.includes("worktree prune"), "no prune when branch is new"); - console.log(" PASS: createAgentWorktree creates a fresh branch when none exists"); - passed++; + // ── createAgentWorktree: fresh branch (none exists) ── + calls = []; + let wt = await createAgentWorktree("/wd", "/wd-dev", "worktree-dev", recordingExec({ + "rev-parse --verify --quiet refs/heads/worktree-dev": { ok: false, output: "" }, + "branch worktree-dev HEAD": { ok: true, output: "" }, + "worktree add /wd-dev worktree-dev": { ok: true, output: "" }, + }, calls)); + assert.deepEqual(wt, { ok: true, detached: false }, "fresh branch created + attached"); + assert.ok(calls.includes("branch worktree-dev HEAD"), "creates the branch when absent"); + assert.ok(!calls.includes("worktree prune"), "no prune when branch is new"); + console.log(" PASS: createAgentWorktree creates a fresh branch when none exists"); + passed++; - // ── createAgentWorktree: stale branch reuse (branch exists, dir gone) ── - calls.length = 0; - wt = createAgentWorktree("/wd", "/wd-dev", "worktree-dev", fakeExec({ - "rev-parse --verify --quiet refs/heads/worktree-dev": { ok: true, output: "abc\n" }, - "worktree prune": { ok: true, output: "" }, - "worktree add /wd-dev worktree-dev": { ok: true, output: "" }, - })); - assert.deepEqual(wt, { ok: true, detached: false }, "existing branch reused"); - assert.ok(calls.includes("worktree prune"), "prunes stale registration before re-attach"); - assert.ok(!calls.includes("branch worktree-dev HEAD"), "does NOT recreate an existing branch (the old brick)"); - console.log(" PASS: createAgentWorktree reuses an existing branch (re-runnable setup)"); - passed++; + // ── createAgentWorktree: stale branch reuse (branch exists, dir gone) ── + calls = []; + wt = await createAgentWorktree("/wd", "/wd-dev", "worktree-dev", recordingExec({ + "rev-parse --verify --quiet refs/heads/worktree-dev": { ok: true, output: "abc" }, + "worktree prune": { ok: true, output: "" }, + "worktree add /wd-dev worktree-dev": { ok: true, output: "" }, + }, calls)); + assert.deepEqual(wt, { ok: true, detached: false }, "existing branch reused"); + assert.ok(calls.includes("worktree prune"), "prunes stale registration before re-attach"); + assert.ok(!calls.includes("branch worktree-dev HEAD"), "does NOT recreate an existing branch (the old brick)"); + console.log(" PASS: createAgentWorktree reuses an existing branch (re-runnable setup)"); + passed++; - // ── createAgentWorktree: detached fallback when add fails ── - calls.length = 0; - wt = createAgentWorktree("/wd", "/wd-dev", "worktree-dev", fakeExec({ - "rev-parse --verify --quiet refs/heads/worktree-dev": { ok: false, output: "" }, - "branch worktree-dev HEAD": { ok: true, output: "" }, - "worktree add /wd-dev worktree-dev": { ok: false, output: "already checked out" }, - "worktree add --detach /wd-dev HEAD": { ok: true, output: "" }, - })); - assert.deepEqual(wt, { ok: true, detached: true }, "falls back to detached worktree"); - console.log(" PASS: createAgentWorktree falls back to a detached worktree"); - passed++; + // ── createAgentWorktree: detached fallback when add fails ── + calls = []; + wt = await createAgentWorktree("/wd", "/wd-dev", "worktree-dev", recordingExec({ + "rev-parse --verify --quiet refs/heads/worktree-dev": { ok: false, output: "" }, + "branch worktree-dev HEAD": { ok: true, output: "" }, + "worktree add /wd-dev worktree-dev": { ok: false, output: "already checked out" }, + "worktree add --detach /wd-dev HEAD": { ok: true, output: "" }, + }, calls)); + assert.deepEqual(wt, { ok: true, detached: true }, "falls back to detached worktree"); + console.log(" PASS: createAgentWorktree falls back to a detached worktree"); + passed++; - console.log(`\n${passed} passed, 0 failed\n`); -} catch (err) { - console.error("test failed:", err); - process.exitCode = 1; -} finally { - cp.execFileSync = realExecFileSync; - fs.existsSync = realExistsSync; - fs.mkdirSync = realMkdirSync; - fs.chmodSync = realChmodSync; -} + console.log(`\n${passed} passed, 0 failed\n`); + } catch (err) { + console.error("test failed:", err); + process.exitCode = 1; + } finally { + fs.existsSync = realExistsSync; + fs.mkdirSync = realMkdirSync; + fs.chmodSync = realChmodSync; + } +})(); diff --git a/server/routes.setupWorktrees.test.js b/server/routes.setupWorktrees.test.js index b5f4f48..6ac3674 100644 --- a/server/routes.setupWorktrees.test.js +++ b/server/routes.setupWorktrees.test.js @@ -1,5 +1,8 @@ // #947: setup wizard worktree creation must refresh stale existing clones // before HEAD checks and initialize genuinely empty repos with inline identity. +// #975: ensureGitHeadForSetup is now async with an injectable execFn (so setup +// shell-outs run off the event loop); this test injects a fake async exec that +// returns the exec() { ok, output } contract and awaits the function. // // Plain node:assert script — run with // `node server/routes.setupWorktrees.test.js`. @@ -7,55 +10,12 @@ const assert = require("node:assert/strict"); const fs = require("fs"); const path = require("path"); -const cp = require("child_process"); -const realExecFileSync = cp.execFileSync; const realExistsSync = fs.existsSync; const realMkdirSync = fs.mkdirSync; const realChmodSync = fs.chmodSync; let existing = new Set(); -let commands = []; -let mode = "stale"; - -cp.execFileSync = function stubExecFileSync(cmd, args, opts = {}) { - commands.push({ cmd, args: args.slice(), cwd: opts.cwd || null }); - if (cmd !== "git" && cmd !== "gh") return realExecFileSync.apply(this, arguments); - const joined = args.join(" "); - if (cmd === "gh" && joined.startsWith("repo clone")) return ""; - if (cmd === "git" && joined === "fetch origin --prune") return ""; - if (cmd === "git" && joined === "rev-parse --verify HEAD") { - if ((mode === "stale" || mode === "stale-no-origin-head") && commands.some((c) => c.args.join(" ") === "checkout -B main origin/main")) return "abc123\n"; - if (mode === "ready") return "abc123\n"; - const err = new Error("fatal: ambiguous argument 'HEAD'"); - err.stderr = Buffer.from("fatal: ambiguous argument 'HEAD'\n"); - throw err; - } - if (cmd === "git" && joined === "symbolic-ref --quiet --short refs/remotes/origin/HEAD") { - if (mode === "stale") return "origin/main\n"; - const err = new Error("fatal: ref not found"); - err.stderr = Buffer.from("fatal: ref not found\n"); - throw err; - } - if (cmd === "git" && joined === "rev-parse --verify origin/main") { - if (mode === "stale-no-origin-head") return "abc123\n"; - const err = new Error("fatal: bad revision"); - err.stderr = Buffer.from("fatal: bad revision\n"); - throw err; - } - if (cmd === "git" && joined === "rev-parse --verify origin/master") { - const err = new Error("fatal: bad revision"); - err.stderr = Buffer.from("fatal: bad revision\n"); - throw err; - } - if (cmd === "git" && joined === "checkout -B main origin/main") return ""; - if (cmd === "git" && joined.includes("-c user.name=QuadWork -c user.email=quadwork@localhost commit --allow-empty")) return ""; - if (cmd === "git" && joined === "symbolic-ref --short HEAD") return "main\n"; - if (cmd === "git" && joined === "push origin main") return ""; - const err = new Error(`unexpected command: ${cmd} ${joined}`); - err.stderr = Buffer.from(err.message); - throw err; -}; fs.existsSync = function stubExistsSync(p) { return existing.has(path.normalize(p)); @@ -65,57 +25,95 @@ fs.chmodSync = () => {}; const { ensureGitHeadForSetup } = require("./routes"); -function reset() { - commands = []; - existing = new Set(); - mode = "stale"; +// Fake async exec mirroring the real git/gh behavior for each mode. Records the +// joined args into `calls` and returns { ok, output } (never touches a shell). +function makeExec(mode, calls) { + return async (cmd, args) => { + calls.push(args.join(" ")); + const joined = args.join(" "); + if (cmd === "gh" && joined.startsWith("repo clone")) return { ok: true, output: "" }; + if (cmd === "git" && joined === "fetch origin --prune") return { ok: true, output: "" }; + // #974: origin-match guard — return a URL matching "owner/repo" so the + // guard passes and the HEAD-handling flow (the subject of this test) runs. + if (cmd === "git" && joined === "remote get-url origin") { + return { ok: true, output: "https://github.com/owner/repo.git" }; + } + if (cmd === "git" && joined === "rev-parse --verify HEAD") { + if ((mode === "stale" || mode === "stale-no-origin-head") && calls.includes("checkout -B main origin/main")) { + return { ok: true, output: "abc123" }; + } + if (mode === "ready") return { ok: true, output: "abc123" }; + return { ok: false, output: "fatal: ambiguous argument 'HEAD'" }; + } + if (cmd === "git" && joined === "symbolic-ref --quiet --short refs/remotes/origin/HEAD") { + if (mode === "stale") return { ok: true, output: "origin/main" }; + return { ok: false, output: "fatal: ref not found" }; + } + if (cmd === "git" && joined === "rev-parse --verify origin/main") { + if (mode === "stale-no-origin-head") return { ok: true, output: "abc123" }; + return { ok: false, output: "fatal: bad revision" }; + } + if (cmd === "git" && joined === "rev-parse --verify origin/master") { + return { ok: false, output: "fatal: bad revision" }; + } + if (cmd === "git" && joined === "checkout -B main origin/main") return { ok: true, output: "" }; + if (cmd === "git" && joined.includes("-c user.name=QuadWork -c user.email=quadwork@localhost commit --allow-empty")) { + return { ok: true, output: "" }; + } + if (cmd === "git" && joined === "symbolic-ref --short HEAD") return { ok: true, output: "main" }; + if (cmd === "git" && joined === "push origin main") return { ok: true, output: "" }; + return { ok: false, output: `unexpected command: ${cmd} ${joined}` }; + }; } -try { - reset(); - existing.add(path.normalize("/tmp/proj/.git")); - let result = ensureGitHeadForSetup("/tmp/proj", "owner/repo"); - assert.equal(result.ok, true, "stale existing clone with remote commits becomes ready"); - assert.ok(commands.some((c) => c.args.join(" ") === "fetch origin --prune"), "existing clone is fetched before HEAD handling"); - assert.ok(commands.some((c) => c.args.join(" ") === "checkout -B main origin/main"), "remote default branch is checked out when local HEAD is unborn"); - assert.ok(!commands.some((c) => c.args.includes("commit")), "stale clone with remote commits does not seed an empty commit"); - console.log(" PASS: stale empty local clone fetches/checks out remote default branch"); +(async () => { + try { + // stale empty local clone → fetch + checkout remote default branch + existing = new Set([path.normalize("/tmp/proj/.git")]); + let calls = []; + let result = await ensureGitHeadForSetup("/tmp/proj", "owner/repo", makeExec("stale", calls)); + assert.equal(result.ok, true, "stale existing clone with remote commits becomes ready"); + assert.ok(calls.includes("fetch origin --prune"), "existing clone is fetched before HEAD handling"); + assert.ok(calls.includes("checkout -B main origin/main"), "remote default branch is checked out when local HEAD is unborn"); + assert.ok(!calls.some((c) => c.includes("commit")), "stale clone with remote commits does not seed an empty commit"); + console.log(" PASS: stale empty local clone fetches/checks out remote default branch"); - reset(); - existing.add(path.normalize("/tmp/proj/.git")); - mode = "stale-no-origin-head"; - result = ensureGitHeadForSetup("/tmp/proj", "owner/repo"); - assert.equal(result.ok, true, "stale clone without origin/HEAD checks out origin/main"); - assert.ok(commands.some((c) => c.args.join(" ") === "rev-parse --verify origin/main"), "origin/main fallback is checked"); - assert.ok(commands.some((c) => c.args.join(" ") === "checkout -B main origin/main"), "origin/main fallback is checked out"); - console.log(" PASS: stale clone falls back to origin/main when origin/HEAD is missing"); + // stale clone without origin/HEAD → origin/main fallback + existing = new Set([path.normalize("/tmp/proj/.git")]); + calls = []; + result = await ensureGitHeadForSetup("/tmp/proj", "owner/repo", makeExec("stale-no-origin-head", calls)); + assert.equal(result.ok, true, "stale clone without origin/HEAD checks out origin/main"); + assert.ok(calls.includes("rev-parse --verify origin/main"), "origin/main fallback is checked"); + assert.ok(calls.includes("checkout -B main origin/main"), "origin/main fallback is checked out"); + console.log(" PASS: stale clone falls back to origin/main when origin/HEAD is missing"); - reset(); - existing.add(path.normalize("/tmp/empty/.git")); - mode = "empty"; - result = ensureGitHeadForSetup("/tmp/empty", "owner/repo"); - assert.equal(result.ok, true, "genuinely empty repo is initialized"); - const commit = commands.find((c) => c.args.includes("commit") && c.args.includes("--allow-empty")); - assert.ok(commit, "empty repo path creates seed commit"); - assert.ok(commit.args.includes("user.name=QuadWork"), "seed commit uses inline user.name"); - assert.ok(commit.args.includes("user.email=quadwork@localhost"), "seed commit uses inline user.email"); - assert.ok(commands.some((c) => c.args.join(" ") === "push origin main"), "seed commit is pushed to default branch"); - console.log(" PASS: empty repo seeds with inline identity and pushes"); + // genuinely empty repo → seed inline-identity commit + push + existing = new Set([path.normalize("/tmp/empty/.git")]); + calls = []; + result = await ensureGitHeadForSetup("/tmp/empty", "owner/repo", makeExec("empty", calls)); + assert.equal(result.ok, true, "genuinely empty repo is initialized"); + const commit = calls.find((c) => c.includes("commit") && c.includes("--allow-empty")); + assert.ok(commit, "empty repo path creates seed commit"); + assert.ok(commit.includes("user.name=QuadWork"), "seed commit uses inline user.name"); + assert.ok(commit.includes("user.email=quadwork@localhost"), "seed commit uses inline user.email"); + assert.ok(calls.includes("push origin main"), "seed commit is pushed to default branch"); + console.log(" PASS: empty repo seeds with inline identity and pushes"); - reset(); - mode = "ready"; - result = ensureGitHeadForSetup("/tmp/new", "owner/repo"); - assert.equal(result.ok, true, "missing local clone is cloned and verified"); - assert.ok(commands.some((c) => c.cmd === "gh" && c.args.join(" ") === "repo clone owner/repo /tmp/new"), "missing clone is cloned via gh"); - console.log(" PASS: missing clone path still clones via gh"); + // missing local clone → cloned via gh + existing = new Set(); + calls = []; + result = await ensureGitHeadForSetup("/tmp/new", "owner/repo", makeExec("ready", calls)); + assert.equal(result.ok, true, "missing local clone is cloned and verified"); + assert.ok(calls.includes("repo clone owner/repo /tmp/new"), "missing clone is cloned via gh"); + console.log(" PASS: missing clone path still clones via gh"); - console.log("\n4 passed, 0 failed\n"); -} catch (err) { - console.error("test failed:", err); - process.exitCode = 1; -} finally { - cp.execFileSync = realExecFileSync; - fs.existsSync = realExistsSync; - fs.mkdirSync = realMkdirSync; - fs.chmodSync = realChmodSync; -} + console.log("\n4 passed, 0 failed\n"); + } catch (err) { + console.error("test failed:", err); + process.exitCode = 1; + } finally { + fs.existsSync = realExistsSync; + fs.mkdirSync = realMkdirSync; + fs.chmodSync = realChmodSync; + } +})();