fix(unskip): Naht nur noch auf devloop-verwaltete Tests (v0.8.1)

.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "devloop",
-  "version": "0.8.0",
+  "version": "0.8.1",
   "description": "Generische agentische Dev-Loop-Kette: /devloop:specify|spec-to-tests|implement|critic|loop + Bindungs-Anker (Wächter-Vorbedingung, content-gebundene Stopps, CI-Required-Check). Projektübergreifend.",
   "author": { "name": "Tobias Mogdans" }
 }

USAGE.md

@@ -23,7 +23,7 @@ CI + Branch-Protection.
 ```
 
 Das legt an: den CI-Workflow `devloop-precondition-check.yml` und das Config-Skeleton
-`.devloop/{tier-map,protected-globs,bot-logins}.json`.
+`.devloop/{tier-map,protected-globs,bot-logins,managed-tests}.json`.
 
 Danach **von Hand** (das ist der Anker, der Selbst-Freigabe verhindert):
 
@@ -43,6 +43,11 @@ Danach **von Hand** (das ist der Anker, der Selbst-Freigabe verhindert):
      Admin-Override statt eines normalen Reviews.
    - `.devloop/bot-logins.json` — die GitHub-Login(s) **deines Agenten** (damit seine
      „Approvals" nie als Mensch zählen).
+   - `.devloop/managed-tests.json` — **welche Testpfade devloop verwaltet** (Glob-Liste). Nur für
+     die gilt die Unskip-Naht (§11). Das Skeleton ist `["**"]` = *jede* Testdatei; **eng ziehen**
+     auf die Pfade, die `spec-to-tests` wirklich autort (deine Käfig-Services / Twin-Areas),
+     sonst blockiert `verify-unskip` jeden gewöhnlichen PR, der eine normale aktive Testdatei
+     hinzufügt. Fehlt die Datei (oder ist sie kaputt), gilt fail-closed „alles verwaltet".
 5. **CODEOWNERS = das §9-Merge-Tor (wichtig).** CODEOWNERS muss decken:
    - das **Spec-Verzeichnis** (z.B. `/.specify/specs/`) → erzwingt den Spec-Review-Stopp, tier-unabhängig;
    - **alle T2/T3-Pfade aus deiner `tier-map`** (auth, migrations, contracts, `src/**`, …).
@@ -67,6 +72,12 @@ Danach **von Hand** (das ist der Anker, der Selbst-Freigabe verhindert):
    > nicht Folklore (`describe.skip` um aktive `it` trifft **beide** Wächter und ist tabu).
 8. **`verify-unskip`** als Required Check auf dem Implementierungs-PR wiren (im CI-Template enthalten) —
    erzwingt, dass `implement` an Tests nur `.skip` entfernt.
+   > **Umfang setzen (sonst Fehlalarm):** Die Naht gilt für die Pfade in
+   > `.devloop/managed-tests.json`. Steht dort das Skeleton `["**"]`, gilt sie für **jede**
+   > Testdatei im Repo — dann kann auch ein PR, der mit devloop nichts zu tun hat (ein
+   > Dev-Werkzeug mit eigenen Tests), keine aktive Testdatei mehr hinzufügen. Zieh die Liste auf
+   > deine Käfig-Services eng. Maßgeblich ist die Liste auf dem **Basis-Branch**, nicht die im
+   > PR — ein PR kann sich die Ausnahme nicht selbst ausstellen.
 
 > Prüfen, ob alle Wächter stehen: `devloop check-guardians <repo>` (exit 0 = bereit).
 

dist/cli/verify-unskip.js

@@ -3,9 +3,18 @@
 //   - an EXISTING test file may change ONLY by removing `.skip` (implement PR).
 // implement can thus neither author active tests nor edit existing ones. Fail-closed (exit 1).
 //
-// Usage: verify-unskip <repoPath> <baseRef>   (baseRef e.g. origin/main)
+// SCOPE: the seam applies to the tests devloop MANAGES (those `spec-to-tests` derives from a
+// reviewed spec), not to every test file in the repo. The target repo declares them in
+// `.devloop/managed-tests.json` (glob list) — a file inside the protected set, so an agent
+// cannot widen it. No such file => every test file is managed (today's behaviour, fail-closed).
+// Deliberately NOT branch-name-driven: an agent chooses its own branch name.
+//
+// Usage: verify-unskip <repoPath> <baseRef> [headBranch]   (baseRef e.g. origin/main)
 import { execFileSync } from "node:child_process";
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
 import { isAllowedTestEdit, isSpecBranch } from "../core/unskip.js";
+import { MANAGED_TESTS_PATH, isManagedTestPath, parseManagedTestGlobs, } from "../core/managed-tests.js";
 const repo = process.argv[2] ?? ".";
 const base = process.argv[3] ?? "origin/main";
 const headBranch = process.argv[4] ?? "";
@@ -17,16 +26,40 @@ if (isSpecBranch(headBranch)) {
 }
 const gitSafe = (args) => {
     try {
-        return execFileSync("git", ["-C", repo, ...args], { encoding: "utf8" });
+        // stderr ignored: `git show <ref>:<new file>` legitimately fails for every file the PR adds
+        // (and for an absent config) — the "fatal: path ... does not exist" noise is not a finding.
+        return execFileSync("git", ["-C", repo, ...args], {
+            encoding: "utf8",
+            stdio: ["ignore", "pipe", "ignore"],
+        });
     }
     catch {
         return "";
     }
 };
 const TEST_FILE = /\.(test|spec)\.[jt]sx?$/;
-const changed = gitSafe(["diff", "--name-only", `${base}...HEAD`])
+// The glob list is read from the BASE ref first: the scope that is already on the protected
+// branch decides, so a PR cannot loosen the seam for itself. Only if the file does not exist on
+// base do we fall back to the checkout (a repo adopting the file — that PR necessarily touches
+// `.devloop/**` = the protected set and is already flagged by verify-review).
+const readWorktree = (rel) => {
+    try {
+        return readFileSync(join(repo, rel), "utf8");
+    }
+    catch {
+        return "";
+    }
+};
+const fromBase = gitSafe(["show", `${base}:${MANAGED_TESTS_PATH}`]);
+const fromWorktree = fromBase === "" ? readWorktree(MANAGED_TESTS_PATH) : "";
+const managedRaw = fromBase !== "" ? fromBase : fromWorktree;
+const managedSource = fromBase !== "" ? "base" : fromWorktree !== "" ? "worktree" : "default";
+const managedGlobs = parseManagedTestGlobs(managedRaw);
+const testFiles = gitSafe(["diff", "--name-only", `${base}...HEAD`])
     .split("\n")
     .filter((f) => TEST_FILE.test(f));
+const changed = testFiles.filter((f) => isManagedTestPath(f, managedGlobs));
+const unmanaged = testFiles.filter((f) => !isManagedTestPath(f, managedGlobs));
 const violations = [];
 for (const file of changed) {
     const oldContent = gitSafe(["show", `${base}:${file}`]); // "" if the file is new
@@ -41,5 +74,14 @@ for (const file of changed) {
     }
 }
 const ok = violations.length === 0;
-process.stdout.write(JSON.stringify({ ok, base, checked: changed, violations }, null, 2) + "\n");
+process.stdout.write(JSON.stringify({
+    ok,
+    base,
+    // Which scope was in force — printed so a green run is never silently a disarmed one.
+    managed: managedGlobs === null ? "all-test-files (no .devloop/managed-tests.json)" : managedGlobs,
+    managedSource,
+    checked: changed,
+    unmanaged,
+    violations,
+}, null, 2) + "\n");
 process.exit(ok ? 0 : 1);

dist/core/init.js

@@ -18,6 +18,17 @@ function tierGlobToCodeownersPath(glob) {
 }
 // Project-agnostic skeletons — the adopter tunes them in their constitution/CI.
 const PROTECTED_GLOBS_SKELETON = JSON.stringify(["**/stryker.conf.*", "**/.semgrep/**", "**/constitution.md", "**/CODEOWNERS", "**/*.eslintrc*"], null, 2);
+// Which test paths the unskip seam (§11) binds. The skeleton is the FAIL-CLOSED default —
+// `**` = every test file, i.e. exactly the pre-v0.8.1 behaviour. Narrow it to the paths
+// spec-to-tests actually authors (the caged services / twin areas), otherwise a plain PR that
+// adds an ordinary active test file cannot pass the guard. Lives in `.devloop/` = the protected
+// set, so an agent cannot widen it to smuggle its own tests past the seam.
+const MANAGED_TESTS_SKELETON = JSON.stringify({
+    _comment: "Globs of the DEVLOOP-MANAGED test files (written by spec-to-tests, only un-skippable by implement). " +
+        "Narrow these to your caged areas; ordinary tests outside them are not bound by the unskip seam. " +
+        "Delete this file to fall back to 'every test file is managed'.",
+    globs: ["**"],
+}, null, 2);
 // Anchor (b): identities that are NOT human (the agent's bot/app account). A review by any
 // of these never counts as a human approval. The adopter fills in their agent's login(s).
 const BOT_LOGINS_SKELETON = JSON.stringify(["devloop-agent[bot]"], null, 2);
@@ -55,6 +66,10 @@ export function initRepo(targetRepo, ciTemplate, opts = {}) {
     }
     writeIfAbsent(".devloop/protected-globs.json", PROTECTED_GLOBS_SKELETON + "\n");
     writeIfAbsent(".devloop/bot-logins.json", BOT_LOGINS_SKELETON + "\n");
+    writeIfAbsent(".devloop/managed-tests.json", MANAGED_TESTS_SKELETON + "\n");
+    result.notes.push(".devloop/managed-tests.json scopes the unskip seam (§11). The skeleton (`**`) binds EVERY " +
+        "test file — the fail-closed default. Narrow it to the paths spec-to-tests authors, else a " +
+        "normal PR adding an ordinary active test file is rejected by verify-unskip.");
     // Anchor (b) is the default: CI is authoritative. Recorded explicitly so the local merge
     // hook defers to CI instead of demanding the (anchor-a) local token.
     // anchor: CI is authoritative (b). twin: the optional spec-to-twin station, disabled by default

dist/core/managed-tests.js

@@ -0,0 +1,55 @@
+// WHICH test files does devloop actually manage? (the §11 test<->code seam, scoped)
+//
+// The unskip seam exists to protect the tests that `spec-to-tests` derived from a reviewed
+// spec — the ones `implement` may neither author nor edit. It was never meant to claim EVERY
+// test file in the target repo: a repo also holds ordinary, hand-written tests (a dev tool, a
+// script, a legacy suite) that have nothing to do with a devloop run. Enforcing the seam on
+// those makes it impossible to ADD a normal, active test outside a `devloop/spec/*` branch.
+//
+// The distinction comes from a CONFIG FILE IN THE TARGET REPO — `.devloop/managed-tests.json`,
+// a glob list — deliberately NOT from the branch name: an agent picks its own branch name, so a
+// branch-coupled exemption is self-serve. `.devloop/**` is part of the protected set
+// (protected-globs.json), so an agent cannot widen the exemption list to smuggle its own tests
+// past the seam — `verify-review` fails a diff that touches the protected set, unconditionally
+// and independent of tier, BEFORE the unskip step ever runs.
+//
+// Fail-closed & backwards compatible: NO config file (or an unreadable/malformed one) means
+// EVERY test file is managed — exactly today's behaviour. No existing adopter gets looser by
+// upgrading; a repo opts into the narrower scope explicitly, in a file only a human can land.
+import { matchesAnyGlob } from "./glob.js";
+/** Repo-relative path of the (optional) managed-test glob list. */
+export const MANAGED_TESTS_PATH = ".devloop/managed-tests.json";
+/**
+ * Parse `.devloop/managed-tests.json`. Accepts a bare array (like protected-globs.json /
+ * bot-logins.json) or `{ "globs": [...] }`. Anything else — absent, empty, invalid JSON, wrong
+ * shape, non-string entries — yields `null` (= manage everything), never a partial list: a
+ * half-understood config must not silently disarm the seam.
+ *
+ * An EXPLICIT empty array is honoured (`[]` = "no devloop-managed tests here"); it is a
+ * deliberate statement in a protected file, not an accident.
+ */
+export function parseManagedTestGlobs(raw) {
+    if (raw === null || raw === undefined || raw.trim() === "")
+        return null;
+    let parsed;
+    try {
+        parsed = JSON.parse(raw);
+    }
+    catch {
+        return null; // malformed -> fail closed
+    }
+    const globs = Array.isArray(parsed)
+        ? parsed
+        : typeof parsed === "object" && parsed !== null && Array.isArray(parsed.globs)
+            ? (parsed.globs)
+            : null;
+    if (globs === null)
+        return null; // wrong shape -> fail closed
+    if (!globs.every((g) => typeof g === "string"))
+        return null; // non-string entry -> fail closed
+    return globs;
+}
+/** Is this repo-relative test path devloop-managed (i.e. does the unskip seam apply to it)? */
+export function isManagedTestPath(path, globs) {
+    return globs === null ? true : matchesAnyGlob(path, globs);
+}

docs/2026-08-28-befund-verify-unskip-fehlalarm-managed-tests.md

@@ -0,0 +1,56 @@
+# Befund: `verify-unskip` blockiert jede gewöhnliche neue Testdatei (v0.8.0) — Fix in v0.8.1
+
+## Der Fehlalarm, live aufgelaufen
+
+`src/cli/verify-unskip.ts` lief auf **jedem** PR des Ziel-Repos; die einzige Ausnahme war
+`isSpecBranch(headBranch)` (`devloop/spec/*` → No-op). `TEST_FILE = /\.(test|spec)\.[jt]sx?$/`
+erfasste **jede** Testdatei im Repo, und für neue Dateien gilt
+`isAllowedTestEdit("", neu) === !hasActiveTest(neu)`.
+
+Folge: **außerhalb eines `devloop/spec/*`-Branches konnte niemand eine gewöhnliche, aktive
+Testdatei hinzufügen.** Aufgelaufen am bsk-PR #157, der ein reines Entwickler-Werkzeug
+(`tools/dead-members`) mit 32 eigenen `node:test`-Tests beisteuert — mit devloop hat das nichts
+zu tun. Verdikt: 5 × `"new test file contains an active (non-.skip) test"`, während der
+Freigabe-Teil sauber `{"ok":true,"tier":"T1"}` meldete.
+
+Die Ursache ist eine **Annahme**, keine Regel: der Wächter unterstellte, dass *alle* Tests im
+Repo devloop-verwaltet sind. Die Naht (§11, Anti-Kollusion Test↔Code) soll aber nur die aus der
+Spec abgeleiteten Tests schützen — die, die `spec-to-tests` geschrieben hat und `implement`
+weder autoren noch ändern darf.
+
+## Der Fix: verwaltete vs. gewöhnliche Testpfade
+
+Neu: `.devloop/managed-tests.json` im Ziel-Repo — eine **Glob-Liste** der devloop-verwalteten
+Testpfade. Nur für Treffer gilt die Naht; alles andere ist eine gewöhnliche Testdatei und wird
+durchgelassen.
+
+```jsonc
+// .devloop/managed-tests.json — bare array oder { "globs": [...] }
+["services/*/src/**/*.test.ts", "services/*/twin/**/*.test.ts"]
+```
+
+Drei Eigenschaften, die den Fix tragen:
+
+1. **Konfigurationsdatei, nicht Branch-Name.** Ein Agent wählt seinen Branch-Namen selbst; eine
+   branchnamen-gekoppelte Ausnahme wäre Selbstbedienung (dieselbe Schwäche hat die
+   Semgrep-Fluchttür des Pilot-Repos an `devloop/spec/*`). `.devloop/**` steht dagegen im
+   **geschützten Satz** (`protected-globs.json`) — `verify-review` failt bei jeder Berührung,
+   **tier-unabhängig und vor** dem Unskip-Schritt.
+2. **Basis-Ref entscheidet.** Die Liste wird zuerst aus `<base>:.devloop/managed-tests.json`
+   gelesen: der auf dem geschützten Branch bereits gelandete Umfang gilt, ein PR kann die Naht
+   also nicht für sich selbst lockern. Nur wenn die Datei auf `base` **nicht existiert**, zählt
+   der Checkout (Adoptions-PR — und der berührt zwangsläufig den geschützten Satz).
+3. **Fail-closed & rückwärtskompatibel.** Datei fehlt, ist leer, kein gültiges JSON, falsch
+   geformt oder enthält Nicht-Strings → **jede** Testdatei ist verwaltet, also exakt das
+   Verhalten vor v0.8.1. Kein bestehendes Ziel-Repo wird durch den Bump still lockerer. Eine
+   **explizit leere** Liste (`[]`) wird respektiert — das ist eine bewusste Aussage in einer
+   geschützten Datei.
+
+`init` legt das Skeleton mit `globs: ["**"]` an (= heutiges Verhalten) und **notiert laut**,
+dass es einzuengen ist. Der Ausgabe-JSON von `verify-unskip` nennt jetzt `managed` und
+`managedSource` — ein grüner Lauf ist damit nie still ein entwaffneter.
+
+## Was NICHT geändert wurde
+
+Das bestehende `isSpecBranch`-No-op bleibt. Es ist dieselbe Schwäche (branchnamen-gekoppelt),
+aber ein eigener Befund — hier zu ändern hieße, zwei Dinge in einem Diff zu vermischen.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "devloop",
-  "version": "0.8.0",
+  "version": "0.8.1",
   "private": true,
   "type": "module",
   "license": "MIT",

src/cli/verify-unskip.ts

@@ -3,10 +3,23 @@
 //   - an EXISTING test file may change ONLY by removing `.skip` (implement PR).
 // implement can thus neither author active tests nor edit existing ones. Fail-closed (exit 1).
 //
-// Usage: verify-unskip <repoPath> <baseRef>   (baseRef e.g. origin/main)
+// SCOPE: the seam applies to the tests devloop MANAGES (those `spec-to-tests` derives from a
+// reviewed spec), not to every test file in the repo. The target repo declares them in
+// `.devloop/managed-tests.json` (glob list) — a file inside the protected set, so an agent
+// cannot widen it. No such file => every test file is managed (today's behaviour, fail-closed).
+// Deliberately NOT branch-name-driven: an agent chooses its own branch name.
+//
+// Usage: verify-unskip <repoPath> <baseRef> [headBranch]   (baseRef e.g. origin/main)
 
 import { execFileSync } from "node:child_process";
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
 import { isAllowedTestEdit, isSpecBranch } from "../core/unskip.js";
+import {
+  MANAGED_TESTS_PATH,
+  isManagedTestPath,
+  parseManagedTestGlobs,
+} from "../core/managed-tests.js";
 
 const repo = process.argv[2] ?? ".";
 const base = process.argv[3] ?? "origin/main";
@@ -21,18 +34,43 @@ if (isSpecBranch(headBranch)) {
 
 const gitSafe = (args: string[]): string => {
   try {
-    return execFileSync("git", ["-C", repo, ...args], { encoding: "utf8" });
+    // stderr ignored: `git show <ref>:<new file>` legitimately fails for every file the PR adds
+    // (and for an absent config) — the "fatal: path ... does not exist" noise is not a finding.
+    return execFileSync("git", ["-C", repo, ...args], {
+      encoding: "utf8",
+      stdio: ["ignore", "pipe", "ignore"],
+    });
   } catch {
     return "";
   }
 };
 
 const TEST_FILE = /\.(test|spec)\.[jt]sx?$/;
 
-const changed = gitSafe(["diff", "--name-only", `${base}...HEAD`])
+// The glob list is read from the BASE ref first: the scope that is already on the protected
+// branch decides, so a PR cannot loosen the seam for itself. Only if the file does not exist on
+// base do we fall back to the checkout (a repo adopting the file — that PR necessarily touches
+// `.devloop/**` = the protected set and is already flagged by verify-review).
+const readWorktree = (rel: string): string => {
+  try {
+    return readFileSync(join(repo, rel), "utf8");
+  } catch {
+    return "";
+  }
+};
+const fromBase = gitSafe(["show", `${base}:${MANAGED_TESTS_PATH}`]);
+const fromWorktree = fromBase === "" ? readWorktree(MANAGED_TESTS_PATH) : "";
+const managedRaw = fromBase !== "" ? fromBase : fromWorktree;
+const managedSource = fromBase !== "" ? "base" : fromWorktree !== "" ? "worktree" : "default";
+const managedGlobs = parseManagedTestGlobs(managedRaw);
+
+const testFiles = gitSafe(["diff", "--name-only", `${base}...HEAD`])
   .split("\n")
   .filter((f) => TEST_FILE.test(f));
 
+const changed = testFiles.filter((f) => isManagedTestPath(f, managedGlobs));
+const unmanaged = testFiles.filter((f) => !isManagedTestPath(f, managedGlobs));
+
 const violations: { file: string; reason: string }[] = [];
 for (const file of changed) {
   const oldContent = gitSafe(["show", `${base}:${file}`]); // "" if the file is new
@@ -49,5 +87,20 @@ for (const file of changed) {
 }
 
 const ok = violations.length === 0;
-process.stdout.write(JSON.stringify({ ok, base, checked: changed, violations }, null, 2) + "\n");
+process.stdout.write(
+  JSON.stringify(
+    {
+      ok,
+      base,
+      // Which scope was in force — printed so a green run is never silently a disarmed one.
+      managed: managedGlobs === null ? "all-test-files (no .devloop/managed-tests.json)" : managedGlobs,
+      managedSource,
+      checked: changed,
+      unmanaged,
+      violations,
+    },
+    null,
+    2,
+  ) + "\n",
+);
 process.exit(ok ? 0 : 1);