Repository navigation
fix(unskip): Naht nur noch auf devloop-verwaltete Tests (v0.8.1) - #3
Merged
Merged
Conversation
verify-unskip ran on every PR (only `devloop/spec/*` was exempt) and its
TEST_FILE regex claimed EVERY test file in the target repo. Since a new file
is allowed only if it has no active test, nobody could add an ordinary,
active test file outside a spec branch. Live: bsk PR #157 contributes a plain
dev tool (tools/dead-members) with its own node:test suite and collected
5 x "new test file contains an active (non-.skip) test" — a pure false alarm,
while the approval half reported {"ok":true,"tier":"T1"}.
The seam (§11, anti-collusion test<->code) is meant to protect the tests
spec-to-tests derived from a reviewed spec, not every test in the repo.
- core/managed-tests: parse `.devloop/managed-tests.json` (bare array or
{globs:[...]}) and decide per path whether the seam binds it.
- cli/verify-unskip: enforce the seam only on managed paths; report `managed`,
`managedSource`, `checked` and `unmanaged` so a green run is never silently
a disarmed one.
- The list is read from the BASE ref first — a PR cannot widen its own
exemption; only when the file is absent on base does the checkout count
(adoption PR, which necessarily touches the protected set).
- Config file, NOT branch name: an agent picks its own branch name, so a
branch-coupled exemption is self-serve. `.devloop/**` is in the protected
set, and verify-review fails a protected-set touch unconditionally and
BEFORE the unskip step runs.
- Fail-closed & backwards compatible: absent, empty, malformed or
wrong-shaped config => every test file is managed = pre-v0.8.1 behaviour.
No adopter gets looser by upgrading. An EXPLICIT `[]` is honoured.
- init scaffolds the skeleton at its fail-closed default (`globs: ["**"]`)
and notes loudly that it must be narrowed.
- git stderr silenced: `git show <base>:<new file>` legitimately fails for
every added file; the "fatal: path ... does not exist" noise is not a finding.
188 tests green (was 174), tsc clean, dist rebuilt in sync.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01BFcJSagot15i7RfhN9hkwp
Also ships 4a55f73 (per-service stryker config detection in monorepos), which landed on main after the v0.8.0 tag. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01BFcJSagot15i7RfhN9hkwp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Der Fehlalarm
verify-unskiplief auf jedem PR des Ziel-Repos — einzige AusnahmeisSpecBranch(headBranch)(
devloop/spec/*→ No-op).TEST_FILE = /\.(test|spec)\.[jt]sx?$/erfasst jede Testdatei, undfür neue Dateien gilt
isAllowedTestEdit("", neu) === !hasActiveTest(neu).Folge: außerhalb eines
devloop/spec/*-Branches konnte niemand eine gewöhnliche, aktiveTestdatei hinzufügen. Live aufgelaufen an bsk-PR #157, der ein reines Entwickler-Werkzeug
(
tools/dead-members) mit 32 eigenennode:test-Tests beisteuert — mit devloop hat das nichts zutun. Verdikt: 5 ×
"new test file contains an active (non-.skip) test", während der Freigabe-Teilsauber
{"ok":true,"tier":"T1"}meldete.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.
Der Fix
Neu:
.devloop/managed-tests.jsonim Ziel-Repo — eine Glob-Liste der devloop-verwaltetenTestpfade (bare array oder
{ "globs": [...] }). Nur für Treffer gilt die Naht.branchnamen-gekoppelte Ausnahme wäre Selbstbedienung.
.devloop/**steht im geschützten Satz—
verify-reviewfailt bei jeder Berührung, tier-unabhängig und vor dem Unskip-Schritt.<base>:.devloop/managed-tests.json; ein PRkann die Naht nicht für sich selbst lockern. Nur wenn die Datei auf
basefehlt, zählt derCheckout (Adoptions-PR — der berührt zwangsläufig den geschützten Satz).
jede Testdatei ist verwaltet = exakt das Verhalten vor v0.8.1. Kein bestehendes Ziel-Repo
wird durch den Bump still lockerer. Ein explizit leeres
[]wird respektiert.managed,managedSource,checkedundunmanaged— ein grüner Lauf ist nie still ein entwaffneter.initlegt das Skeleton mitglobs: ["**"]an (= heutiges Verhalten) und notiert laut,dass es einzuengen ist.
Das bestehende
isSpecBranch-No-op bleibt unangetastet — dieselbe Schwäche, aber ein eigenerBefund; hier zu ändern hieße, zwei Dinge in einem Diff zu vermischen.
Nachweise
Gegenprobe am echten Fall (bsk-Branch
chore/tools-dead-membersgegenorigin/main, in einemread-only
--shared-Klon; das bsk-Repo wurde nicht angefasst):ok:false, 5 Verstöße, exit 1dist, ohne Konfigok:false, 5 Verstöße, exit 1 — Rückwärtskompatibilität.devloop/managed-tests.jsonauforigin/mainok:true, 0 Verstöße, exit 0,managedSource:"base"services/location-service/api/spec-tests/geschmuggeltok:false, 1 Verstoß, exit 1 — die Naht bleibt scharfMutationsproben (jeweils Implementierung gezielt verfälscht, dann
npm test):return []stattnull)fail closed … never a partial list,a malformed managed-tests.json falls back to managing everything)isManagedTestPathDefault gekippt (null⇒ nichts verwaltet)exit 1 when implement also edited an assertion)basethe BASE list decides,when base has no list …)an ORDINARY new test file … passes — the fehlalarm)init-Skeleton vom Fail-closed-**weg verengtinit scaffolds the unskip-seam scope at its FAIL-CLOSED default)Gates: 188 Tests grün (vorher 174, +14),
tsc --noEmitsauber,npm run check:distgrün(
dist/mitgebaut und mitcommittet).Release
chore(release): v0.8.1bumptpackage.json+.claude-plugin/plugin.json. Tag nach demMerge auf
main:Der Release enthält zusätzlich
4a55f73(per-service Stryker-Config in Monorepos), das nach demv0.8.0-Tag auf
mainlandete.🤖 Generated with Claude Code
https://claude.ai/code/session_01BFcJSagot15i7RfhN9hkwp