Tests for the routing policy; honour --backend on route --role reviewer

.github/workflows/ci.yml

@@ -23,6 +23,8 @@ jobs:
         run: node --check skills/delegate-kit/scripts/agent-run
       - name: Writer caps and detach refusal
         run: bash skills/delegate-kit/tests/caps.sh
+      - name: Routing policy
+        run: bash skills/delegate-kit/tests/route.sh
       - name: SKILL.md stays a policy, not a manual
         run: |
           size=$(wc -c < skills/delegate-kit/SKILL.md)

skills/delegate-kit/references/review.md

@@ -4,7 +4,7 @@ How many reviewers a diff deserves, which angle each one takes, and how their fi
 
 ## Independence is the first slot, not the whole review
 
-One reviewer from the other family than the author buys **independence**: two families share fewer blind spots than one. No preset, panel or quota pressure moves slot A off the other family. The one thing that does is availability: when that family's CLI is not installed, `route` places slot A as a fresh read-only worker of the author's family, marks it `independent: false`, and the report names which one ran. A fresh context is still a real review; the other family is the stronger one.
+One reviewer from the other family than the author buys **independence**: two families share fewer blind spots than one. No preset, panel or quota pressure moves slot A off the other family. Two things do. Availability: when that family's CLI is not installed, `route` places slot A as a fresh read-only worker of the author's family, marks it `independent: false`, and the report names which one ran. And the user: an explicit `--backend` on `route --role reviewer` pins slot A (the alternation runs from it), and a same-family pin is reported as `independent: false` too. A fresh context is still a real review; the other family is the stronger one.
 
 A second reviewer with the same brief buys almost nothing: the obvious findings come back twice and the subtle ones stay missed, because both reviewers looked from the same angle. What a second slot should buy is a second **lens**. So a panel is composed as lenses first, families second:
 

skills/delegate-kit/scripts/agent-run

@@ -134,7 +134,8 @@ const die = (msg, code = 1) => {
 const nowIso = () => new Date().toISOString();
 const newId = () => `${new Date().toISOString().replace(/[-:T]/g, "").slice(0, 14)}-${Math.random().toString(36).slice(2, 6)}`;
 const readJson = (p, fallback = null) => { try { return JSON.parse(fs.readFileSync(p, "utf8")); } catch { return fallback; } };
-const writeJson = (p, v) => fs.writeFileSync(p, JSON.stringify(v, null, 2) + "\n");
+// tmp + rename: a concurrent reader sees the old file or the new one, never a truncated one
+const writeJson = (p, v) => { const tmp = `${p}.${process.pid}.tmp`; fs.writeFileSync(tmp, JSON.stringify(v, null, 2) + "\n"); fs.renameSync(tmp, p); };
 const runDir = (id) => path.join(RUNS_DIR, id);
 const metaOf = (id) => readJson(path.join(runDir(id), "meta.json"));
 const saveMeta = (m) => writeJson(path.join(runDir(m.id), "meta.json"), m);
@@ -273,13 +274,37 @@ function acquireCapsMutex(timeoutMs = 15_000) {
   const deadline = Date.now() + timeoutMs;
   for (;;) {
     try { fs.writeFileSync(CAPS_MUTEX, String(process.pid), { flag: "wx" }); capsMutexHeld = true; return; } catch (e) { if (e.code !== "EEXIST") throw e; }
-    const holder = Number(fs.readFileSync(CAPS_MUTEX, "utf8").trim() || 0);
+    let holder; try { holder = Number(fs.readFileSync(CAPS_MUTEX, "utf8").trim() || 0); } catch (e) { if (e.code === "ENOENT") continue; throw e; }
     if (!holder || !alive(holder)) { try { fs.rmSync(CAPS_MUTEX, { force: true }); } catch {} continue; }
     if (Date.now() > deadline) die(`another agent-run (pid ${holder}) has held ${CAPS_MUTEX} for ${timeoutMs / 1000}s; if it is stuck, kill it or remove the file`);
     Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 50);
   }
 }
-function releaseCapsMutex() { if (!capsMutexHeld) return; capsMutexHeld = false; try { fs.rmSync(CAPS_MUTEX, { force: true }); } catch {} }
+/*
+ * Второй мьютекс — репозитория, общий с `agent-wt lock`: каталог
+ * <common .git>/delegate-kit.caps.lock, mkdir атомарен. Без него внешний и
+ * нативный писатель одного репозитория считали слоты под разными замками и
+ * оба проходили при cap 1. agent-run берёт сначала машинный, потом
+ * репозиторный; agent-wt только репозиторный — цикла нет.
+ */
+let repoMutexHeld = null;
+function acquireRepoMutex(cwd, timeoutMs = 15_000) {
+  const r = sh("git", ["rev-parse", "--git-common-dir"], { cwd });
+  if (r.status !== 0) return;
+  const dir = path.join(path.resolve(cwd, r.stdout.trim()), "delegate-kit.caps.lock");
+  const deadline = Date.now() + timeoutMs;
+  for (;;) {
+    try { fs.mkdirSync(dir); fs.writeFileSync(path.join(dir, "pid"), String(process.pid)); repoMutexHeld = dir; return; } catch (e) { if (e.code !== "EEXIST") throw e; }
+    let holder = 0; try { holder = Number(fs.readFileSync(path.join(dir, "pid"), "utf8").trim() || 0); } catch {}
+    if (!holder || !alive(holder)) { try { fs.rmSync(dir, { recursive: true, force: true }); } catch {} continue; }
+    if (Date.now() > deadline) die(`another agent-run or agent-wt (pid ${holder}) has held ${dir} for ${timeoutMs / 1000}s; if it is stuck, kill it or remove the directory`);
+    Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 50);
+  }
+}
+function releaseCapsMutex() {
+  if (repoMutexHeld) { const d = repoMutexHeld; repoMutexHeld = null; try { fs.rmSync(d, { recursive: true, force: true }); } catch {} }
+  if (!capsMutexHeld) return; capsMutexHeld = false; try { fs.rmSync(CAPS_MUTEX, { force: true }); } catch {}
+}
 
 // Refuse before anything is spawned. Native locks (`agent-wt lock`) of the repository of
 // cwd count as workers and as writers; external runs count machine-wide.
@@ -704,12 +729,15 @@ function suggestDepth(stats, kind) {
 // Slot A is always the other family than the author: that is where independence comes from and
 // nothing moves it. Slot B may sit on the author's family — independence is already paid for, and
 // the second slot buys a second lens, not a second opinion from the same angle. Slot C alternates.
-function reviewComposition(depth, author, kind) {
-  const other = otherBackend(author);
+// Slot A is the other family than the author unless the user pins it with --backend: an
+// explicit same-family reviewer is the user's call, reported as independent: false. Before
+// it was honoured, `route --role reviewer --backend X` was silently overridden here.
+function reviewComposition(depth, author, kind, pinned = null) {
+  const first = pinned || otherBackend(author);
   const lenses = kind === "refactor" ? ["correctness", "standards", "spec"] : ["correctness", "spec", "standards"];
   const n = depth === "single" ? 1 : depth === "panel" ? 2 : 3;
-  const families = [other, author, other];
-  return lenses.slice(0, n).map((lens, i) => ({ slot: "ABC"[i], lens, backend: families[i], independent: families[i] !== author }));
+  const families = [first, otherBackend(first), first];
+  return lenses.slice(0, n).map((lens, i) => ({ slot: "ABC"[i], lens, backend: families[i], independent: families[i] !== author, pinned: Boolean(pinned) && i === 0 }));
 }
 
 function cmdRoute(argv) {
@@ -722,10 +750,13 @@ function cmdRoute(argv) {
   let depth = argv.depth && argv.depth !== true ? String(argv.depth) : (suggested ? suggested.depth : "single");
   if (!DEPTHS.includes(depth)) die(`--depth must be ${DEPTHS.join("|")} (got ${depth})`);
   const author = base.author;
-  const reviewers = reviewComposition(depth, author, kind).map((r) => {
-    const rr = resolveRoute({ ...argv, role: "reviewer", backend: r.backend, _composed: true, depth: undefined, diff: undefined });
-    const note = rr.why.find((w) => w.includes("falls back"));
-    return { ...r, backend: rr.backend, independent: rr.backend !== author, model: rr.model, effort: rr.effort, dispatch: rr.dispatch, invoke: rr.invoke, ...(note ? { note } : {}) };
+  const reviewers = reviewComposition(depth, author, kind, argv.backend && argv.backend !== true ? String(argv.backend) : null).map((r) => {
+    // a slot the user pinned with --backend keeps it even when that CLI is missing; composed slots may fall back
+    const rr = resolveRoute({ ...argv, role: "reviewer", backend: r.backend, _composed: !r.pinned, depth: undefined, diff: undefined });
+    // keep what the caller must act on: a fallback that happened, or a pinned CLI that is missing
+    const note = rr.why.find((w) => w.includes("falls back") || w.includes("not installed"));
+    const { pinned: _p, ...slot } = r;
+    return { ...slot, backend: rr.backend, independent: rr.backend !== author, model: rr.model, effort: rr.effort, dispatch: rr.dispatch, invoke: rr.invoke, ...(note ? { note } : {}) };
   });
   const lead = depth === "led" ? (() => { const l = resolveRoute({ ...argv, role: "review-lead", backend: undefined, depth: undefined, diff: undefined }); return { backend: l.backend, model: l.model, effort: l.effort, dispatch: l.dispatch, invoke: l.invoke }; })() : null;
   const sessions = reviewers.length + (lead ? 2 : 0);
@@ -873,7 +904,7 @@ async function cmdRun(opts, resumeOf = null) {
   if (opts._supervise) SUPERVISED = { id: opts.id, role, backend, model, effort, cwd, write };
   // The initial supervised start runs under the mutex its parent still holds; every other
   // start (foreground, detach parent, quota fallback) takes it itself.
-  if (!opts._supervise || opts._fallbackFrom) acquireCapsMutex();
+  if (!opts._supervise || opts._fallbackFrom) { acquireCapsMutex(); if (write) acquireRepoMutex(cwd); }
   checkCaps(caps, write, cwd);
   if (write) { const conflict = writeLockConflict(cwd, null); if (conflict) die(conflict); }
 

skills/delegate-kit/scripts/agent-wt

@@ -39,6 +39,10 @@ base_ref_file() { local p; p=$(wt_path "$1"); [ -d "$p" ] || return 1; echo "$(c
 
 lock_info() {
   local lf; lf=$(lock_file "$1" 2>/dev/null) || { echo "no-lock"; return; }
+  lock_state "$lf"
+}
+lock_state() { # state of one lock file: unlocked | locked(...) | stale-lock(...)
+  local lf=$1
   [ -f "$lf" ] || { echo "unlocked"; return; }
   local kind pid; kind=$(jq -r '.kind // "process"' "$lf"); pid=$(jq -r '.pid // empty' "$lf")
   # A native lock has no process behind it: the parent holds it on behalf of its own
@@ -124,12 +128,15 @@ acquire_caps_mutex() {
   done
   die "another agent-wt (pid $pid) has held $d for 15s; if it is stuck, kill it or remove the directory"
 }
+# Every linked worktree of this checkout, not only the ones under wt_base_dir: an
+# `agent-run --cwd` writer may sit in any worktree, and its lock lives in the same place.
 locked_worktrees() {
-  local base; base=$(wt_base_dir); [ -d "$base" ] || return 0
-  for p in "$base"/*/; do
-    [ -d "$p" ] || continue
-    local n; n=$(basename "$p")
-    [[ "$(lock_info "$n")" == locked* ]] && echo "$n"
+  local common; common="$(git rev-parse --git-common-dir)"; common="$(cd "$common" && pwd)"
+  [ -d "$common/worktrees" ] || return 0
+  local lf
+  for lf in "$common"/worktrees/*/delegate-kit.lock; do
+    [ -f "$lf" ] || continue
+    [[ "$(lock_state "$lf")" == locked* ]] && basename "$(dirname "$lf")"
   done
   return 0
 }

skills/delegate-kit/tests/caps.sh

@@ -133,13 +133,26 @@ for w in w1 w2 w3 w4 w5 w6 w7 w8; do "$WT" lock "$w" --max-writers 1 >/dev/null
 ok "занят один worktree" "$("$WT" list | jq '[.[] | select(.lock | startswith("locked"))] | length')" "1"
 ok "мьютекс отпущен" "$([ -e "$BASE/repo/.git/delegate-kit.caps.lock" ] && echo held || echo free)" "free"
 
-echo "── брошенный мьютекс с мёртвым pid не блокирует"
+echo "── брошенные мьютексы с мёртвым pid не блокируют"
+for w in w1 w2 w3 w4 w5 w6 w7 w8; do "$WT" release "$w" >/dev/null 2>&1; done
 mkdir -p "$BASE/repo/.git/delegate-kit.caps.lock"; echo 999999 > "$BASE/repo/.git/delegate-kit.caps.lock/pid"
-"$WT" lock w8 --max-writers 8 >/dev/null; ok "lock прошёл" "$?" "0"
+"$WT" lock w8 --max-writers 8 >/dev/null; ok "agent-wt снял труп репозиторного мьютекса" "$?" "0"
 echo 999999 > "$DELEGATE_KIT_HOME/caps.lock"
-run "$WTS/w8" --max-writers 1
-ok "agent-run снял труп мьютекса и дошёл до потолка" "$(has "$ERR" "concurrent writers reached")" "yes"
-ok "мьютекс agent-run отпущен" "$([ -e "$DELEGATE_KIT_HOME/caps.lock" ] && echo held || echo free)" "free"
+mkdir -p "$BASE/repo/.git/delegate-kit.caps.lock"; echo 999999 > "$BASE/repo/.git/delegate-kit.caps.lock/pid"
+run "$WTS/w7" --max-writers 1
+ok "agent-run снял оба трупа и дошёл до потолка" "$(has "$ERR" "concurrent writers reached")" "yes"
+ok "машинный мьютекс отпущен" "$([ -e "$DELEGATE_KIT_HOME/caps.lock" ] && echo held || echo free)" "free"
+ok "репозиторный мьютекс отпущен" "$([ -e "$BASE/repo/.git/delegate-kit.caps.lock" ] && echo held || echo free)" "free"
+
+echo "── agent-wt видит process-lock внешнего писателя в любом linked worktree"
+"$WT" release w8 >/dev/null
+git -C "$BASE/repo" worktree add -q -b dk/elsewhere "$BASE/elsewhere" >/dev/null 2>&1
+jq -n --arg pid "$$" '{id:"ext-1",role:"implementer",kind:"process",pid:($pid|tonumber),cwd:"x"}' > "$BASE/repo/.git/worktrees/elsewhere/delegate-kit.lock"
+ERR=$("$WT" lock w1 --max-writers 1 2>&1 >/dev/null); RC=$?
+ok "отказ: чужой worktree занят живым процессом" "$RC" "1"
+ok "он назван" "$(has "$ERR" "(elsewhere)")" "yes"
+jq -n '{id:"ext-2",role:"implementer",kind:"process",pid:999999,cwd:"x"}' > "$BASE/repo/.git/worktrees/elsewhere/delegate-kit.lock"
+"$WT" lock w1 --max-writers 1 >/dev/null; ok "мёртвый process-lock не считается" "$?" "0"
 
 echo; echo "Пройдено: $PASS, провалено: $FAIL"
 exit $((FAIL > 0))