From b8778f702509112fdf43f65c71487a0cfbf00c95 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:56:54 +0800 Subject: [PATCH] chore: organize dirty work from codex/workflow-reliability --- .agents/skills/task/SKILL.md | 2 +- AGENTS.md | 1 + docs/process-hardening.md | 7 + docs/productivity-workflows.md | 10 +- docs/reconciliation-playbook.md | 11 + package.json | 2 + scripts/primary-checkout-guard.mjs | 326 +++++++++++++++++++++++++ scripts/productivity-core.mjs | 10 +- scripts/reconciliation-preflight.mjs | 180 +++++++++++--- tests/primary-checkout-guard.test.ts | 156 ++++++++++++ tests/productivity-workflow.test.ts | 18 ++ tests/reconciliation-preflight.test.ts | 59 ++++- 12 files changed, 727 insertions(+), 55 deletions(-) create mode 100644 scripts/primary-checkout-guard.mjs create mode 100644 tests/primary-checkout-guard.test.ts diff --git a/.agents/skills/task/SKILL.md b/.agents/skills/task/SKILL.md index 48f63bbd7b..c8d03aa0b1 100644 --- a/.agents/skills/task/SKILL.md +++ b/.agents/skills/task/SKILL.md @@ -6,7 +6,7 @@ description: Manage Database task lifecycle transitions safely, including start, # Task 1. Use the required task-start script before repository changes. -2. Run `npm run workflow:lifecycle -- --phase --write-evidence`. +2. Run `npm run primary:status`, then `npm run workflow:lifecycle -- --phase --write-evidence`. 3. Inspect branch, upstream, worktrees, status, and operation markers for the selected phase. 4. Preserve dirty, ambiguous, active, or unmerged work. 5. Keep fetch, push, PR, merge, and provider checks approval-gated. diff --git a/AGENTS.md b/AGENTS.md index 3025ebffed..2295667803 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -466,6 +466,7 @@ Record one `docs/branch-review-ledger.md` row per PR touched, and end with the p - After auth, Supabase, ingestion, answer generation, search/ranking, clinical output, or source-governance changes, run the smallest domain check plus `npm run check:production-readiness`. Run `npm run check:supabase-project` after Supabase env/config changes. - For handoff, archive-safety, or upload-style requests, inspect branch/upstream/status first, run the appropriate verification gate, and only commit or push when the request explicitly asks for that workflow. - For broad chat/worktree reconciliation or cleanup, run `node scripts/reconciliation-preflight.mjs`, use the cheap ownership/PR/ledger/ancestry funnel before patch comparison, and never print raw process command lines. +- Keep feature edits in task-owned worktrees. Before any deliberate write, branch switch, fast-forward, or synchronization in the canonical primary checkout, run it through `npm run primary:mutate -- [args...]`; inspect ownership read-only with `npm run primary:status`. The guard must fail closed on a live writer, dirty state, active Git operation, or unreadable ownership record, while read-only work and independent worktrees remain unblocked. - For codebase appraisal exports, stage outside the repo, include `EXPORT_MANIFEST.md`, exclude secrets/dependencies/build outputs/local state, and verify the archive can be opened before handoff. - When a repeated repo-specific workflow is discovered, update this file or ask the user whether it should be remembered. diff --git a/docs/process-hardening.md b/docs/process-hardening.md index 2466a10d4c..62485b28a1 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -12,6 +12,13 @@ The reusable procedure is [`docs/reconciliation-playbook.md`](reconciliation-pla cleanup. It uses cached Git refs, never fetches, and reports primary/worktree dirty state, detached worktrees, ahead/behind counts, and operation markers. Add `--include-processes` only when ownership could block cleanup; that path emits metadata/counts and never raw command lines. + Its worktree Git checks use bounded asynchronous inspection, per-command fail-closed timeouts, + and duration metadata rather than serial subprocess fan-out tied to the host's worktree count. +- `npm run primary:status` remains read-only. `npm run primary:mutate -- [args...]` is the + cooperative write boundary for the canonical primary checkout: it holds an atomic lease for one + child mutation, refuses live contention, dirt, or Git operation markers, stores no command + arguments, and recovers only owners proven dead (or incomplete records older than the initialization + grace period). Feature worktrees and read-only inspection do not acquire this lease. - `workflow:lifecycle -- --phase reconcile` selects the preflight locally and lists remote fetch as a separate approval-required action. - Candidate filtering is cheap-first: owner/open-PR/review-ledger/ancestry before patch comparison; diff --git a/docs/productivity-workflows.md b/docs/productivity-workflows.md index bdd2635591..29a5b40382 100644 --- a/docs/productivity-workflows.md +++ b/docs/productivity-workflows.md @@ -26,7 +26,15 @@ The repository exposes seven offline-first workflow planners. Each planner inspe - Use lifecycle phase `reconcile` for broad multi-worktree work. It selects the report-only `node scripts/reconciliation-preflight.mjs` locally and keeps `git fetch --prune origin` approval-gated. Add `--include-processes` to the preflight only when process ownership may block cleanup; it never - serializes raw command lines. + serializes raw command lines. Worktree inspection is bounded and reports its duration, so the + command remains useful when many worktrees are registered without making the test suite depend + on that machine-local inventory. +- Use `npm run primary:status` for a read-only view of the canonical checkout's owner, dirty state, + and Git operation markers. Wrap deliberate primary writes with + `npm run primary:mutate -- [args...]`. The wrapper holds an atomic lease for the child + command, refuses a second live owner or pre-existing dirty/operation state, and recovers a lease + whose recorded process is proven dead. It does not serialize read-only work or commands run in + independent feature worktrees. The existing shared `workflow:run`, `workflow:status`, `workflow:verify`, `workflow:deps`, `workflow:clean-state`, `workflow:export`, and `workflow:handoff` commands now resolve their shared implementation through the repository's Git common directory. This keeps them portable in linked and detached Codex worktrees. Set `CODEX_LOCAL_WORKFLOW_ROOT` only when the shared tools live somewhere non-standard. diff --git a/docs/reconciliation-playbook.md b/docs/reconciliation-playbook.md index 8d7c4abafe..ee8c95924f 100644 --- a/docs/reconciliation-playbook.md +++ b/docs/reconciliation-playbook.md @@ -71,6 +71,17 @@ merges, old review refs, and intentionally retained branches. - Start with a red reproducer or exact content proof. - Refresh the base between sequential protected-main PRs. - Reject stale, mixed, duplicated, refuted, or unmeasured behavior rather than forcing it to apply. + +## Canonical checkout synchronization + +- Inspect the primary without claiming it: `npm run primary:status`. +- Keep ordinary edits and verification in task-owned worktrees. +- Wrap each deliberate primary branch switch or fast-forward so ownership is held until Git exits, + for example `npm run primary:mutate -- git merge --ff-only origin/main`. +- Stop when the guard reports a live owner, dirty entry, active Git operation, or unreadable state. + Preserve and classify the checkout; never bypass the lease or discard files to make the guard pass. +- A dead owner is recovered automatically. A fresh incomplete ownership record fails closed until + its 30-second initialization grace period expires. - Protected RAG behavior requires the repository RAG declaration, offline contracts, and approved baseline/post live canary. A failed canary reverts immediately. diff --git a/package.json b/package.json index ce72290335..12c2935708 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,8 @@ "hooks:install": "node scripts/install-git-hooks.mjs", "guard:push": "node scripts/guard-push.mjs", "guard:push:self-test": "node scripts/guard-push.mjs --self-test", + "primary:status": "node scripts/primary-checkout-guard.mjs status", + "primary:mutate": "node scripts/primary-checkout-guard.mjs run --", "check:base-freshness": "node scripts/check-base-freshness.mjs", "ensure": "node scripts/ensure-local-server.mjs", "build": "node scripts/run-heavy.mjs --npm-script build:internal", diff --git a/scripts/primary-checkout-guard.mjs b/scripts/primary-checkout-guard.mjs new file mode 100644 index 0000000000..a9dabc0583 --- /dev/null +++ b/scripts/primary-checkout-guard.mjs @@ -0,0 +1,326 @@ +#!/usr/bin/env node + +import { spawnSync } from "node:child_process"; +import { createHash, randomUUID } from "node:crypto"; +import { existsSync, mkdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { parseWorktreePorcelain, reconciliationPreflightInternals } from "./reconciliation-preflight.mjs"; + +const repositoryRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const incompleteLeaseGraceMs = 30_000; + +/** + * @typedef {{ + * path: string; + * branch: string | null; + * statusEntries: number | null; + * operations: string[]; + * inspectionErrors: string[]; + * }} PrimaryCheckoutState + */ + +function normalizeIdentity(value) { + const resolved = path.resolve(String(value ?? "")); + return process.platform === "win32" ? resolved.toLowerCase() : resolved; +} + +function processIsAlive(pid) { + if (!Number.isInteger(pid) || pid <= 0) return false; + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} + +function git(args, cwd = repositoryRoot) { + const result = spawnSync("git", args, { + cwd, + encoding: "utf8", + env: { ...process.env, GIT_OPTIONAL_LOCKS: "0" }, + stdio: ["ignore", "pipe", "ignore"], + timeout: 15_000, + windowsHide: true, + }); + if (result.status !== 0 || result.error) { + throw new Error(`Could not inspect Git state required by the primary-checkout guard (${args[0] ?? "git"}).`); + } + return String(result.stdout ?? "").trim(); +} + +function resolveRepositoryIdentity() { + return normalizeIdentity(git(["rev-parse", "--path-format=absolute", "--git-common-dir"])); +} + +function resolvePrimaryCheckout() { + const [primary] = parseWorktreePorcelain(git(["worktree", "list", "--porcelain"])); + if (!primary?.path) throw new Error("Could not resolve the canonical primary checkout; mutation is blocked."); + return { ...primary, path: path.resolve(primary.path) }; +} + +function inspectPrimaryCheckout() { + const primary = resolvePrimaryCheckout(); + const status = git(["--no-optional-locks", "status", "--porcelain=v1", "--untracked-files=normal"], primary.path); + const gitDirectory = git(["rev-parse", "--path-format=absolute", "--git-dir"], primary.path); + return { + path: primary.path, + branch: primary.branch ?? null, + statusEntries: status.split(/\r?\n/).filter(Boolean).length, + operations: reconciliationPreflightInternals.operationMarkers.filter((marker) => + existsSync(path.join(gitDirectory, marker)), + ), + inspectionErrors: [], + }; +} + +function lockPathFor(repositoryIdentity, baseDirectory = os.tmpdir()) { + const repositoryId = createHash("sha256").update(normalizeIdentity(repositoryIdentity)).digest("hex").slice(0, 20); + return path.join(baseDirectory, "clinical-kb-primary-checkout-leases", `${repositoryId}.lock`); +} + +function readOwnerRecord(lockPath) { + if (!existsSync(lockPath)) return { state: "absent", owner: null }; + try { + const owner = JSON.parse(readFileSync(path.join(lockPath, "owner.json"), "utf8")); + if (!owner || !Number.isInteger(owner.pid) || owner.pid <= 0 || typeof owner.token !== "string") { + return { state: "unreadable", owner: null }; + } + return { state: "valid", owner }; + } catch { + return { state: "unreadable", owner: null }; + } +} + +function leaseIsOldEnoughToRecover(lockPath, now = Date.now()) { + try { + return now - statSync(lockPath).mtimeMs >= incompleteLeaseGraceMs; + } catch { + return false; + } +} + +function safeOwner(owner, live) { + if (!owner) return null; + return { + live, + pid: owner.pid, + ownerWorktree: owner.ownerWorktree ?? null, + primaryPath: owner.primaryPath ?? null, + startedAt: owner.startedAt ?? null, + }; +} + +function assertPrimaryCanMutate(primary) { + if (!primary || primary.inspectionErrors?.length > 0 || !Number.isInteger(primary.statusEntries)) { + throw new Error("Primary checkout state could not be inspected completely; mutation is blocked."); + } + if (primary.operations?.length > 0) { + throw new Error( + `Primary checkout has an active Git operation (${primary.operations.join(", ")}); mutation is blocked.`, + ); + } + if (primary.statusEntries > 0) { + throw new Error( + `Primary checkout is dirty (${primary.statusEntries} entries); preserve and classify it before mutation.`, + ); + } +} + +/** + * Acquire an atomic lease for one primary-checkout mutation. The caller must + * hold the returned lease until the child mutation exits. + */ +/** + * @param {{ + * repositoryIdentity?: string; + * baseDirectory?: string; + * ownerWorktree?: string; + * processId?: number; + * inspectPrimary?: () => PrimaryCheckoutState; + * processIsAlive?: (pid: number) => boolean; + * }} [options] + */ +export function acquirePrimaryCheckoutLease(options = {}) { + const { + repositoryIdentity = resolveRepositoryIdentity(), + baseDirectory, + ownerWorktree = process.cwd(), + processId = process.pid, + inspectPrimary = inspectPrimaryCheckout, + processIsAlive: ownerIsAlive = processIsAlive, + } = options; + const lockPath = lockPathFor(repositoryIdentity, baseDirectory); + mkdirSync(path.dirname(lockPath), { recursive: true }); + + for (let attempt = 0; attempt < 3; attempt += 1) { + try { + mkdirSync(lockPath); + } catch (error) { + if (error?.code !== "EEXIST") throw error; + const record = readOwnerRecord(lockPath); + if (record.state === "valid" && ownerIsAlive(record.owner.pid)) { + throw new Error( + `Another primary-checkout mutation is active (PID ${record.owner.pid}, ` + + `worktree ${record.owner.ownerWorktree ?? "unknown"}, started ${record.owner.startedAt ?? "unknown"}).`, + ); + } + if (record.state === "unreadable" && !leaseIsOldEnoughToRecover(lockPath)) { + throw new Error(`The primary-checkout ownership record is unreadable at ${lockPath}; mutation is blocked.`); + } + rmSync(lockPath, { recursive: true, force: true }); + continue; + } + + const token = randomUUID(); + let primary; + try { + primary = inspectPrimary(); + const owner = { + pid: processId, + token, + command: "primary checkout mutation", + ownerWorktree: path.resolve(ownerWorktree), + primaryPath: primary?.path ? path.resolve(primary.path) : null, + repositoryIdentity: normalizeIdentity(repositoryIdentity), + startedAt: new Date().toISOString(), + }; + writeFileSync(path.join(lockPath, "owner.json"), `${JSON.stringify(owner, null, 2)}\n`, "utf8"); + assertPrimaryCanMutate(primary); + + let released = false; + return { + path: lockPath, + owner, + primary, + release() { + if (released) return; + released = true; + const current = readOwnerRecord(lockPath); + if (current.state === "valid" && current.owner.token === token) { + rmSync(lockPath, { recursive: true, force: true }); + } + }, + }; + } catch (error) { + const current = readOwnerRecord(lockPath); + if (current.state !== "valid" || current.owner.token === token) { + rmSync(lockPath, { recursive: true, force: true }); + } + throw error; + } + } + + throw new Error(`Could not acquire the primary-checkout mutation lease at ${lockPath}.`); +} + +/** + * @param {{ + * repositoryIdentity?: string; + * baseDirectory?: string; + * inspectPrimary?: () => PrimaryCheckoutState; + * processIsAlive?: (pid: number) => boolean; + * }} [options] + */ +export function inspectPrimaryCheckoutOwnership(options = {}) { + const { + repositoryIdentity = resolveRepositoryIdentity(), + baseDirectory, + inspectPrimary = inspectPrimaryCheckout, + processIsAlive: ownerIsAlive = processIsAlive, + } = options; + const primary = inspectPrimary(); + const lockPath = lockPathFor(repositoryIdentity, baseDirectory); + const record = readOwnerRecord(lockPath); + const live = record.state === "valid" ? ownerIsAlive(record.owner.pid) : null; + const primaryBlocked = + !primary || + primary.inspectionErrors?.length > 0 || + !Number.isInteger(primary.statusEntries) || + primary.statusEntries > 0 || + primary.operations?.length > 0; + + return { + primary, + owner: record.state === "unreadable" ? { live: null, unreadable: true } : safeOwner(record.owner, live), + mutationAllowed: !primaryBlocked && record.state !== "unreadable" && live !== true, + staleOwnerRecoverable: record.state === "valid" && live === false, + readOnlyWorkAllowed: true, + independentWorktreesAllowed: true, + }; +} + +function renderStatus(status) { + console.log("Primary checkout ownership (read-only)\n"); + console.log(`Primary: ${status.primary?.path ?? "(unresolved)"}`); + console.log(`Branch: ${status.primary?.branch ?? "(detached or unknown)"}`); + console.log(`Dirty entries: ${status.primary?.statusEntries ?? "unknown"}`); + console.log(`Active operation markers: ${status.primary?.operations?.join(", ") || "none"}`); + console.log(`Live mutation owner: ${status.owner?.live === true ? "yes" : "no"}`); + console.log(`Primary mutation allowed: ${status.mutationAllowed ? "yes" : "no"}`); + console.log("Read-only work and independent worktrees remain allowed."); +} + +function usage() { + console.log( + "Usage:\n" + + " node scripts/primary-checkout-guard.mjs status [--json]\n" + + " node scripts/primary-checkout-guard.mjs run -- [args...]", + ); +} + +function runCli(argv) { + const [mode, ...rest] = argv; + if (!mode || mode === "--help" || mode === "-h") { + usage(); + return 0; + } + if (mode === "status") { + const unknown = rest.filter((token) => token !== "--json"); + if (unknown.length > 0) throw new Error(`Unknown status option: ${unknown[0]}`); + const status = inspectPrimaryCheckoutOwnership(); + if (rest.includes("--json")) console.log(JSON.stringify(status, null, 2)); + else renderStatus(status); + return 0; + } + if (mode !== "run") throw new Error(`Unknown primary-checkout guard mode: ${mode}`); + + const separator = rest.indexOf("--"); + const commandParts = separator >= 0 ? rest.slice(separator + 1) : rest; + if (commandParts.length === 0) throw new Error("A mutation command is required after `run --`."); + + const lease = acquirePrimaryCheckoutLease(); + try { + const result = spawnSync(commandParts[0], commandParts.slice(1), { + cwd: lease.primary.path, + env: process.env, + stdio: "inherit", + windowsHide: true, + }); + if (result.error) throw new Error(`Could not start the guarded primary-checkout mutation: ${result.error.message}`); + return Number.isInteger(result.status) ? result.status : 1; + } finally { + lease.release(); + } +} + +const invokedDirectly = process.argv[1] && fileURLToPath(import.meta.url) === path.resolve(process.argv[1]); +if (invokedDirectly) { + try { + process.exitCode = runCli(process.argv.slice(2)); + } catch (error) { + console.error(`[primary-checkout-guard] ${error instanceof Error ? error.message : String(error)}`); + process.exitCode = 2; + } +} + +export const primaryCheckoutGuardInternals = { + incompleteLeaseGraceMs, + leaseIsOldEnoughToRecover, + lockPathFor, + normalizeIdentity, + processIsAlive, + readOwnerRecord, +}; diff --git a/scripts/productivity-core.mjs b/scripts/productivity-core.mjs index a036977f5d..53796abc7e 100644 --- a/scripts/productivity-core.mjs +++ b/scripts/productivity-core.mjs @@ -237,17 +237,22 @@ export function buildWorkflowPlan(workflow, files = [], options = {}) { if (!new Set(["status", "start", "reconcile", "handoff", "landed", "cleanup"]).has(phase)) { throw new Error(`Unknown lifecycle phase: ${phase}`); } + const primaryOwnershipCheck = check( + "npm run primary:status", + "Report canonical-checkout ownership and dirty/operation state without acquiring the mutation lease.", + ); localChecks = phase === "handoff" - ? [check("npm run verify:pr-local", "Complete the local handoff gate.")] + ? [primaryOwnershipCheck, check("npm run verify:pr-local", "Complete the local handoff gate.")] : phase === "reconcile" ? [ + primaryOwnershipCheck, check( "node scripts/reconciliation-preflight.mjs", "Inventory cached base, primary checkout, worktrees, dirty state, and Git operations without fetching.", ), ] - : []; + : [primaryOwnershipCheck]; approvalRequired = phase === "handoff" ? [ @@ -267,6 +272,7 @@ export function buildWorkflowPlan(workflow, files = [], options = {}) { proof.push( "Verify branch and worktree state at every transition.", "Use content equality for squash-merge proof before cleanup.", + "Wrap every deliberate canonical primary-checkout mutation with npm run primary:mutate.", ); if (phase === "reconcile") { proof.push( diff --git a/scripts/reconciliation-preflight.mjs b/scripts/reconciliation-preflight.mjs index abc5c8bdfb..6db5fc5397 100644 --- a/scripts/reconciliation-preflight.mjs +++ b/scripts/reconciliation-preflight.mjs @@ -1,16 +1,21 @@ #!/usr/bin/env node -import { execFileSync } from "node:child_process"; -import { existsSync } from "node:fs"; +import { execFile, execFileSync } from "node:child_process"; +import { existsSync, readFileSync, statSync } from "node:fs"; import path from "node:path"; +import { promisify } from "node:util"; import { fileURLToPath } from "node:url"; import { listRepoNodeProcesses } from "./run-eval-safe.mjs"; const repositoryRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const execFileAsync = promisify(execFile); +const defaultWorktreeInspectionConcurrency = 6; +const gitInspectionTimeoutMs = 15_000; const operationMarkers = [ "MERGE_HEAD", "CHERRY_PICK_HEAD", "REVERT_HEAD", + "AM_HEAD", "BISECT_LOG", "rebase-merge", "rebase-apply", @@ -35,9 +40,45 @@ function git(args, cwd = repositoryRoot, { allowFailure = false } = {}) { } } -function tryGit(args, cwd) { +async function gitAsync(args, cwd = repositoryRoot, { allowFailure = false } = {}) { try { - return { ok: true, output: git(args, cwd) }; + const result = await execFileAsync("git", args, { + cwd, + encoding: "utf8", + env: { ...process.env, GIT_OPTIONAL_LOCKS: "0" }, + maxBuffer: 8 * 1024 * 1024, + timeout: gitInspectionTimeoutMs, + windowsHide: true, + }); + return String(result.stdout ?? "").trim(); + } catch (error) { + if (allowFailure) return ""; + throw error; + } +} + +async function tryGitAsync(args, cwd) { + try { + return { ok: true, output: await gitAsync(args, cwd) }; + } catch { + return { ok: false, output: "" }; + } +} + +function tryResolveWorktreeGitDirectory(worktreePath) { + try { + const dotGitPath = path.join(path.resolve(worktreePath), ".git"); + const dotGitStat = statSync(dotGitPath); + if (dotGitStat.isDirectory()) return { ok: true, output: dotGitPath }; + if (!dotGitStat.isFile()) return { ok: false, output: "" }; + + const match = readFileSync(dotGitPath, "utf8") + .trim() + .match(/^gitdir:\s*(.+)$/i); + if (!match?.[1]) return { ok: false, output: "" }; + const gitDirectory = path.resolve(path.dirname(dotGitPath), match[1]); + if (!statSync(gitDirectory).isDirectory()) return { ok: false, output: "" }; + return { ok: true, output: gitDirectory }; } catch { return { ok: false, output: "" }; } @@ -146,12 +187,12 @@ function resolveBaseRef(explicitBaseRef) { return "main"; } -function inspectWorktree(item, baseRef, baseCommit) { - const statusResult = tryGit(["status", "--porcelain=v1", "--untracked-files=normal"], item.path); - const gitDirectoryResult = tryGit(["rev-parse", "--path-format=absolute", "--git-dir"], item.path); - const countsResult = baseCommit - ? tryGit(["rev-list", "--left-right", "--count", `${baseRef}...${item.head}`], item.path) - : { ok: true, output: "" }; +async function inspectWorktree(item, baseCommit, resolveDivergence) { + const gitDirectoryResult = tryResolveWorktreeGitDirectory(item.path); + const [statusResult, countsResult] = await Promise.all([ + tryGitAsync(["--no-optional-locks", "status", "--porcelain=v1", "--untracked-files=normal"], item.path), + baseCommit ? resolveDivergence(item.head) : Promise.resolve({ ok: true, output: "" }), + ]); const inspectionErrors = []; if (!statusResult.ok) inspectionErrors.push("status-unreadable"); if (!gitDirectoryResult.ok || !gitDirectoryResult.output) inspectionErrors.push("git-directory-unresolved"); @@ -179,6 +220,47 @@ function inspectWorktree(item, baseRef, baseCommit) { }; } +/** + * @param {Array>} worktrees + * @param {string} baseRef + * @param {string} baseCommit + * @param {{ + * concurrency?: number; + * inspect?: (worktree: any, baseRef: string, baseCommit: string) => Promise>; + * }} [options] + */ +export async function inspectReconciliationWorktrees(worktrees, baseRef, baseCommit, options = {}) { + const { concurrency = defaultWorktreeInspectionConcurrency, inspect } = options; + const boundedConcurrency = Math.max(1, Math.min(Number.parseInt(String(concurrency), 10) || 1, 16)); + const results = new Array(worktrees.length); + const divergenceByHead = new Map(); + const resolveDivergence = (head) => { + if (!head) return Promise.resolve({ ok: false, output: "" }); + if (!divergenceByHead.has(head)) { + divergenceByHead.set( + head, + tryGitAsync(["rev-list", "--left-right", "--count", `${baseRef}...${head}`], repositoryRoot), + ); + } + return divergenceByHead.get(head); + }; + const inspectOne = inspect ?? ((item) => inspectWorktree(item, baseCommit, resolveDivergence)); + let nextIndex = 0; + + async function worker() { + while (nextIndex < worktrees.length) { + const index = nextIndex; + nextIndex += 1; + results[index] = await inspectOne(worktrees[index], baseRef, baseCommit); + } + } + + await Promise.all( + Array.from({ length: Math.min(boundedConcurrency, Math.max(worktrees.length, 1)) }, () => worker()), + ); + return results; +} + /** * @param {Array<{ path: string }>} worktrees * @param {(roots: string[]) => Array<{ pid: number, parentPid: number, createdAtMs: number | null }>} [listProcesses] @@ -191,11 +273,12 @@ export function collectProcessDiagnostics(worktrees, listProcesses = listRepoNod }; } -export function collectReconciliationState({ baseRef: explicitBaseRef, includeProcesses = false } = {}) { +export async function collectReconciliationState({ baseRef: explicitBaseRef, includeProcesses = false } = {}) { + const inspectionStartedAt = Date.now(); const baseRef = resolveBaseRef(explicitBaseRef); const baseCommit = git(["rev-parse", "--verify", "--quiet", baseRef], repositoryRoot, { allowFailure: true }); const rawWorktrees = parseWorktreePorcelain(git(["worktree", "list", "--porcelain"])); - const worktrees = rawWorktrees.map((item) => inspectWorktree(item, baseRef, baseCommit)); + const worktrees = await inspectReconciliationWorktrees(rawWorktrees, baseRef, baseCommit); const summary = classifyReconciliationState({ baseCommit, baseRef, worktrees }); return { generatedAt: new Date().toISOString(), @@ -204,31 +287,39 @@ export function collectReconciliationState({ baseRef: explicitBaseRef, includePr processDiagnostics: includeProcesses ? collectProcessDiagnostics(worktrees) : { skipped: true, rawCommandLinesSerialized: false }, + inspectionDiagnostics: { + durationMs: Date.now() - inspectionStartedAt, + worktreeConcurrency: defaultWorktreeInspectionConcurrency, + }, ...summary, }; } -function render(result) { - console.log("Reconciliation preflight (read-only, cached refs; no fetch)\n"); - console.log(`Base: ${result.baseRef} ${result.baseCommit ?? "(unresolved)"}`); - console.log(`Primary: ${result.primaryPath ?? "(unknown)"}`); - console.log( +function render(result, writeOutput = console.log) { + writeOutput("Reconciliation preflight (read-only, cached refs; no fetch)\n"); + writeOutput(`Base: ${result.baseRef} ${result.baseCommit ?? "(unresolved)"}`); + writeOutput(`Primary: ${result.primaryPath ?? "(unknown)"}`); + writeOutput( `Worktrees: ${result.totals.worktrees} total, ${result.totals.dirty} dirty, ` + `${result.totals.detached} detached, ${result.totals.activeOperations} with active Git operations`, ); - console.log("Integration base: create a dedicated clean worktree from the freshly fetched remote base."); + writeOutput( + `Inspection: ${result.inspectionDiagnostics.durationMs}ms with bounded concurrency ` + + `${result.inspectionDiagnostics.worktreeConcurrency}.`, + ); + writeOutput("Integration base: create a dedicated clean worktree from the freshly fetched remote base."); if (result.processDiagnostics.skipped) { - console.log("Process check: skipped (add --include-processes when ownership could block cleanup)."); + writeOutput("Process check: skipped (add --include-processes when ownership could block cleanup)."); } else { - console.log( + writeOutput( `Process check: ${result.processDiagnostics.matchingWorktreeNodeProcesses} registered-worktree Node process(es); ` + "raw command lines were not serialized.", ); } - console.log("\nFindings:"); - if (!result.findings.length) console.log("- none"); - for (const finding of result.findings) console.log(`- [${finding.severity}] ${finding.code}: ${finding.detail}`); - console.log( + writeOutput("\nFindings:"); + if (!result.findings.length) writeOutput("- none"); + for (const finding of result.findings) writeOutput(`- [${finding.severity}] ${finding.code}: ${finding.detail}`); + writeOutput( "\nNext: obtain fetch approval, refresh the remote base, classify by ownership/PR/ledger/ancestry, " + "then inspect patch-unique content only for the remaining candidates.", ); @@ -255,28 +346,39 @@ function parseArgs(argv) { return options; } -function main() { - const options = parseArgs(process.argv.slice(2)); +export async function runReconciliationPreflightCli( + argv, + { collectState = collectReconciliationState, writeOutput = console.log } = {}, +) { + const options = parseArgs(argv); if (options.help) { - console.log( + writeOutput( "Usage: node scripts/reconciliation-preflight.mjs [--json] [--strict] [--include-processes] [--base-ref ]", ); - return; + return 0; } - const result = collectReconciliationState(options); - if (options.json) console.log(JSON.stringify(result, null, 2)); - else render(result); - if (options.strict && result.blocking) process.exitCode = 2; + const result = await collectState(options); + if (options.json) writeOutput(JSON.stringify(result, null, 2)); + else render(result, writeOutput); + return options.strict && result.blocking ? 2 : 0; } const invokedDirectly = process.argv[1] && fileURLToPath(import.meta.url) === path.resolve(process.argv[1]); if (invokedDirectly) { - try { - main(); - } catch (error) { - console.error(`[reconciliation-preflight] ${error instanceof Error ? error.message : String(error)}`); - process.exitCode = 1; - } + runReconciliationPreflightCli(process.argv.slice(2)) + .then((exitCode) => { + process.exitCode = exitCode; + }) + .catch((error) => { + console.error(`[reconciliation-preflight] ${error instanceof Error ? error.message : String(error)}`); + process.exitCode = 1; + }); } -export const reconciliationPreflightInternals = { normalizePath, operationMarkers }; +export const reconciliationPreflightInternals = { + defaultWorktreeInspectionConcurrency, + gitInspectionTimeoutMs, + normalizePath, + operationMarkers, + tryResolveWorktreeGitDirectory, +}; diff --git a/tests/primary-checkout-guard.test.ts b/tests/primary-checkout-guard.test.ts new file mode 100644 index 0000000000..dd33da9faa --- /dev/null +++ b/tests/primary-checkout-guard.test.ts @@ -0,0 +1,156 @@ +import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + acquirePrimaryCheckoutLease, + inspectPrimaryCheckoutOwnership, + primaryCheckoutGuardInternals, +} from "../scripts/primary-checkout-guard.mjs"; + +const temporaryDirectories: string[] = []; + +function temporaryDirectory(prefix: string) { + const directory = path.join(os.tmpdir(), `${prefix}${process.pid}-${temporaryDirectories.length}`); + rmSync(directory, { recursive: true, force: true }); + mkdirSync(directory, { recursive: true }); + temporaryDirectories.push(directory); + return directory; +} + +afterEach(() => { + for (const directory of temporaryDirectories.splice(0)) rmSync(directory, { recursive: true, force: true }); +}); + +const cleanPrimary = { + path: "C:/repo", + branch: "main", + statusEntries: 0, + operations: [], + inspectionErrors: [], +}; + +describe("primary checkout ownership guard", () => { + it("blocks a second mutation while a live owner holds the lease", () => { + const baseDirectory = temporaryDirectory("clinical-kb-primary-live-"); + const repositoryIdentity = path.join(baseDirectory, "shared.git"); + const first = acquirePrimaryCheckoutLease({ + repositoryIdentity, + baseDirectory, + ownerWorktree: "C:/repo-task-a", + inspectPrimary: () => cleanPrimary, + processIsAlive: () => true, + }); + + expect(() => + acquirePrimaryCheckoutLease({ + repositoryIdentity, + baseDirectory, + ownerWorktree: "C:/repo-task-b", + inspectPrimary: () => cleanPrimary, + processIsAlive: () => true, + }), + ).toThrow(/primary-checkout mutation is active/i); + + first.release(); + }); + + it("blocks mutation when the primary checkout is dirty and releases its provisional lease", () => { + const baseDirectory = temporaryDirectory("clinical-kb-primary-dirty-"); + const repositoryIdentity = path.join(baseDirectory, "shared.git"); + const lockPath = primaryCheckoutGuardInternals.lockPathFor(repositoryIdentity, baseDirectory); + + expect(() => + acquirePrimaryCheckoutLease({ + repositoryIdentity, + baseDirectory, + ownerWorktree: "C:/repo-task", + inspectPrimary: () => ({ ...cleanPrimary, statusEntries: 2 }), + }), + ).toThrow(/primary checkout is dirty/i); + expect(existsSync(lockPath)).toBe(false); + }); + + it("fails closed on active Git operations or incomplete primary inspection", () => { + const baseDirectory = temporaryDirectory("clinical-kb-primary-operation-"); + const repositoryIdentity = path.join(baseDirectory, "shared.git"); + + expect(() => + acquirePrimaryCheckoutLease({ + repositoryIdentity, + baseDirectory, + inspectPrimary: () => ({ ...cleanPrimary, operations: ["sequencer"] }), + }), + ).toThrow(/active Git operation/i); + expect(() => + acquirePrimaryCheckoutLease({ + repositoryIdentity, + baseDirectory, + inspectPrimary: () => ({ ...cleanPrimary, statusEntries: null, inspectionErrors: ["status-unreadable"] }), + }), + ).toThrow(/could not be inspected completely/i); + }); + + it("recovers a dead owner without letting the stale token release its replacement", () => { + const baseDirectory = temporaryDirectory("clinical-kb-primary-stale-"); + const repositoryIdentity = path.join(baseDirectory, "shared.git"); + const stale = acquirePrimaryCheckoutLease({ + repositoryIdentity, + baseDirectory, + ownerWorktree: "C:/repo-task-a", + processId: 2_147_483_647, + inspectPrimary: () => cleanPrimary, + }); + const replacement = acquirePrimaryCheckoutLease({ + repositoryIdentity, + baseDirectory, + ownerWorktree: "C:/repo-task-b", + inspectPrimary: () => cleanPrimary, + processIsAlive: () => false, + }); + + stale.release(); + expect(existsSync(path.join(replacement.path, "owner.json"))).toBe(true); + replacement.release(); + }); + + it("fails closed for a fresh unreadable ownership record", () => { + const baseDirectory = temporaryDirectory("clinical-kb-primary-invalid-"); + const repositoryIdentity = path.join(baseDirectory, "shared.git"); + const lockPath = primaryCheckoutGuardInternals.lockPathFor(repositoryIdentity, baseDirectory); + mkdirSync(lockPath, { recursive: true }); + writeFileSync(path.join(lockPath, "owner.json"), "not-json", "utf8"); + + expect(() => + acquirePrimaryCheckoutLease({ + repositoryIdentity, + baseDirectory, + ownerWorktree: "C:/repo-task", + inspectPrimary: () => cleanPrimary, + }), + ).toThrow(/ownership record is unreadable/i); + }); + + it("keeps read-only status available when ownership or dirt exists", () => { + const baseDirectory = temporaryDirectory("clinical-kb-primary-status-"); + const repositoryIdentity = path.join(baseDirectory, "shared.git"); + const lease = acquirePrimaryCheckoutLease({ + repositoryIdentity, + baseDirectory, + ownerWorktree: "C:/repo-task-a", + inspectPrimary: () => cleanPrimary, + processIsAlive: () => true, + }); + + const status = inspectPrimaryCheckoutOwnership({ + repositoryIdentity, + baseDirectory, + inspectPrimary: () => ({ ...cleanPrimary, statusEntries: 1 }), + processIsAlive: () => true, + }); + + expect(status).toMatchObject({ mutationAllowed: false, primary: { statusEntries: 1 }, owner: { live: true } }); + expect(existsSync(lease.path)).toBe(true); + lease.release(); + }); +}); diff --git a/tests/productivity-workflow.test.ts b/tests/productivity-workflow.test.ts index 164a49915a..057b8fce7e 100644 --- a/tests/productivity-workflow.test.ts +++ b/tests/productivity-workflow.test.ts @@ -108,12 +108,30 @@ describe("productivity workflow planning", () => { const plan = buildWorkflowPlan("lifecycle", [], { phase: "reconcile" }); expect(plan.localChecks.map((item: { command: string }) => item.command)).toEqual([ + "npm run primary:status", "node scripts/reconciliation-preflight.mjs", ]); expect(plan.approvalRequired.map((item: { command: string }) => item.command)).toEqual([ "git fetch --prune origin", ]); expect(plan.proof.join(" ")).toContain("never print raw process command lines"); + expect(plan.proof.join(" ")).toContain("npm run primary:mutate"); + }); + + it("surfaces primary ownership at task start without blocking independent worktrees", () => { + const plan = buildWorkflowPlan("lifecycle", [], { phase: "start" }); + + expect(plan.localChecks.map((item: { command: string }) => item.command)).toEqual(["npm run primary:status"]); + expect(plan.approvalRequired).toEqual([]); + }); + + it("surfaces primary ownership before the handoff gate", () => { + const plan = buildWorkflowPlan("lifecycle", [], { phase: "handoff" }); + + expect(plan.localChecks.map((item: { command: string }) => item.command)).toEqual([ + "npm run primary:status", + "npm run verify:pr-local", + ]); }); it("classifies common failure signatures", () => { diff --git a/tests/reconciliation-preflight.test.ts b/tests/reconciliation-preflight.test.ts index 1247b5f33b..01e5ebd364 100644 --- a/tests/reconciliation-preflight.test.ts +++ b/tests/reconciliation-preflight.test.ts @@ -1,10 +1,10 @@ -import { spawnSync } from "node:child_process"; -import path from "node:path"; import { describe, expect, it } from "vitest"; import { classifyReconciliationState, collectProcessDiagnostics, + inspectReconciliationWorktrees, parseWorktreePorcelain, + runReconciliationPreflightCli, } from "../scripts/reconciliation-preflight.mjs"; describe("reconciliation preflight", () => { @@ -102,19 +102,54 @@ describe("reconciliation preflight", () => { expect(diagnostics).toEqual({ matchingWorktreeNodeProcesses: 1, rawCommandLinesSerialized: false }); }); - it("emits parseable metadata-only JSON without fetching", () => { - const script = path.resolve(process.cwd(), "scripts", "reconciliation-preflight.mjs"); - const result = spawnSync(process.execPath, [script, "--json"], { - cwd: process.cwd(), - encoding: "utf8", + it("bounds concurrent worktree inspection while preserving registration order", async () => { + let active = 0; + let peak = 0; + const worktrees = Array.from({ length: 24 }, (_, index) => ({ path: `C:/repo-${index}`, head: `${index}` })); + + const result = await inspectReconciliationWorktrees(worktrees, "origin/main", "base123", { + concurrency: 4, + inspect: async (worktree: (typeof worktrees)[number]) => { + active += 1; + peak = Math.max(peak, active); + await new Promise((resolve) => setTimeout(resolve, 1)); + active -= 1; + return { ...worktree, statusEntries: 0, operations: [], inspectionErrors: [] }; + }, }); - expect(result.status).toBe(0); - expect(result.stderr).toBe(""); - const payload = JSON.parse(result.stdout); + expect(peak).toBeGreaterThan(1); + expect(peak).toBeLessThanOrEqual(4); + expect(result.map((item) => item.path)).toEqual(worktrees.map((item) => item.path)); + }); + + it("emits parseable metadata-only JSON from an injected snapshot without scanning real worktrees", async () => { + const output: string[] = []; + const snapshot = { + generatedAt: "2026-07-25T00:00:00.000Z", + cachedRefsOnly: true, + fetched: false, + processDiagnostics: { skipped: true, rawCommandLinesSerialized: false }, + baseRef: "origin/main", + baseCommit: "base123", + primaryPath: "C:/repo", + totals: { worktrees: 1, dirty: 0, detached: 0, activeOperations: 0, inspectionFailures: 0 }, + integrationBase: "dedicated-worktree-required", + blocking: false, + findings: [], + worktrees: [], + inspectionDiagnostics: { durationMs: 4, worktreeConcurrency: 6 }, + }; + const exitCode = await runReconciliationPreflightCli(["--json"], { + collectState: async () => snapshot, + writeOutput: (value) => output.push(value), + }); + + expect(exitCode).toBe(0); + const payload = JSON.parse(output.join("\n")); expect(payload).toMatchObject({ cachedRefsOnly: true, fetched: false }); expect(payload.integrationBase).toBe("dedicated-worktree-required"); expect(payload.processDiagnostics).toMatchObject({ skipped: true, rawCommandLinesSerialized: false }); - expect(result.stdout).not.toContain("commandLine"); - }, 60_000); + expect(output.join("\n")).not.toContain("commandLine"); + }); });