diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index db4bee8..dae9efa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,4 +23,10 @@ jobs: timeout-minutes: 15 steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - run: /bin/bash --version && /bin/bash scripts/test.sh + # Hard premise of this job: stock /bin/bash must be 3.2. A print alone + # would let the suite silently run under a different bash if the runner + # image ever ships something else. + - run: | + /bin/bash --version + /bin/bash -c '[[ ${BASH_VERSINFO[0]} -eq 3 && ${BASH_VERSINFO[1]} -eq 2 ]]' + /bin/bash scripts/test.sh diff --git a/README.en.md b/README.en.md index 00a0a37..220549b 100644 --- a/README.en.md +++ b/README.en.md @@ -23,11 +23,11 @@ answer. - `git` — all work happens on a `cleanup/YYYYMMDD` branch, never on main. With no git repository the skill only diagnoses: its rollback depends on having a good commit to go back to. -- For JS/TS projects: Node with `npx` (knip runs via `npx knip`, no prior - installation). +- For JS/TS projects: Node with `npx` (knip runs via `npx knip@6.32.0`, + pinned — never bare `npx knip`). - Other stacks use the tools of each ecosystem (vulture, deadcode, cargo-udeps, ReferenceTrimmer). Whatever is missing, the skill reports - instead of installing on its own. + instead of installing on its own; `pip install` only with confirmation. - The gate (`scripts/gate.sh`) detects the stack from the manifest and runs typecheck + tests for JS/TS, Go, Rust, Python, JVM, Ruby and .NET. The toolchain has to be reachable: on PATH for most stacks and, for Python, also @@ -111,7 +111,7 @@ exercises the real GNU `timeout` instead of the perl backend: ```bash docker run --rm -v "$PWD":/repo:ro node:22-bookworm bash -c \ 'apt-get update -qq && apt-get install -y -qq procps && cd /repo && bash scripts/test.sh' -# validated 2026-08: 86/86 cases, 5/5 properties, 77/77 invariants +# validated 2026-08: 127/127 cases, 5/5 properties, 151/151 invariants ``` The .NET heuristic was validated against the real SDK @@ -155,9 +155,13 @@ into one of three levels: | Level | Condition | What it does | |---|---|---| -| GREEN | typecheck and tests pass | runs the phases without asking; phase 2 stops at the checkpoint | -| YELLOW | partial net, or no test file in the stack | only deps and orphan files, no touching exports | -| RED | no tests and no typecheck, or a baseline already failing | diagnoses only; nothing is deleted | +| GREEN | typecheck and tests pass | runs phase 1 without asking; phase 2 and phase 3 stop at the human checkpoint | +| YELLOW | partial net, or no test file in the stack | only deps and orphan files, no touching exports; does not run phase 2 or phase 3 | +| RED | no tests and no typecheck, or a baseline already failing | diagnoses only; nothing is deleted; no `CLEANUP_PROGRESS` commit | + +Stack caps in `references/other-stacks.md` override the GREEN column (Python +always confirms before deleting; JVM/Ruby/.NET code stay YELLOW or diagnosis +by default). A project that arrives with a red suite falls into RED, not YELLOW: with a broken baseline there is no telling what the cleanup broke from what was @@ -166,18 +170,26 @@ happen. The skill names the failing check and stops there. A stack with no test file at all does not count as tested: the gate does not count an empty suite, whether it declined to run it or ran it and got nothing -back, and the level stays at YELLOW. That covers Go and .NET -with no test file, a Rust crate with no `tests/*.rs` and no `#[test]`, a Maven -or Gradle build with no `src/test` anywhere, and a pytest run that exits 5 -having collected nothing. A manifest carried for tooling and nothing else — a -`requirements.txt` for the docs build, a `Gemfile` for fastlane — is not a stack -without a suite: with no source of that language in the repo, the gate says -nothing about it. If your suite lives outside the usual place, promoting it is -your call — the gate never promotes itself. - -In JS/TS the same cap covers a sliced suite: with no `test` script and both -`test:unit` and `test:e2e` in the manifest, no slice answers for the whole -suite and the gate counts none of them. Promoting by hand is the wrong move +back, and the level stays at YELLOW. That covers JS/TS whose runner exits on an +empty suite ("No test files found") — including when it exits 0 because it was +told to, as with `--passWithNoTests`, since exit 0 is not proof a suite ran —, +Go and .NET with no test file, a Rust +crate with no `tests/*.rs` and no `#[test]`, a Maven or Gradle build with no +`src/test` anywhere, a Ruby `spec/` or `test/` holding no `*_spec.rb`, +`*_test.rb` or `test_*.rb` (the `Rake::TestTask` default), and a pytest run +that exits 5 having collected nothing. A +manifest carried for tooling and nothing else — a `requirements.txt` for the +docs build, a `Gemfile` for fastlane — is not a stack without a suite: with no +source of that language in the repo, the gate says nothing about it. If your +suite lives outside the usual place, promoting it is your call — the gate never +promotes itself. + +In JS/TS the exact `npm init` placeholder (`echo "Error: no test specified" && +exit 1`) is also YELLOW, with `'test' not counted` and the `npm init +placeholder` marker — not RED for a broken suite. The same cap covers a sliced +suite: with no `test` script and both `test:unit` and `test:e2e` in the +manifest, no slice answers for the whole suite and the gate counts none of +them. Promoting by hand is the wrong move here, because the suite is not somewhere else, it is split; run every slice. A lone slice does count as the suite, with one exception: watch mode never exits, so the gate skips it. `watch`, `ui` and `debug` are read as whole segments of @@ -196,8 +208,9 @@ With the level announced, it creates the cleanup branch and proceeds: - **Phase 1 — dead code.** Configures knip until the hints reach zero, runs in production mode and deletes in atomic commits, one per category: unused - deps, orphan files, dead exports. Each commit only lands with a green gate. - At the end, it produces an audit of what is left. + deps, orphan files, dead exports. Each step stages only pathspecs of that + step's artifacts (`git add -- …`, never `git add -A`), and only lands with a + green gate. At the end, it produces an audit of what is left. - **Phase 1.5 — duplicate functions** (closes phase 1). Sweeps for functions with different names doing the same thing (similarity-ts or fallow on JS/TS, jscpd on other stacks) and applies the churn rule: a pair that @@ -229,13 +242,16 @@ git revert # undoes only that category Merging the branch is your decision, on your schedule. The skill never pushes, never commits on main and never uses `git reset --hard` — its rollback is `git restore --staged --worktree .`, which throws away everything -that has not been committed yet and coexists with hooks that block destructive -commands. +that has not been committed yet. If a security hook blocks that restore, the +skill **aborts** the pipeline (it does not work around the hook): it reports +the branch, the dirty tree and the manual command, then stops. Note the "everything": a change of yours sitting in the working tree before the skill started would go with it. That is why it demands a clean tree up front and stops to ask when it does not find one — with a clean tree, what the -rollback discards is what the skill itself created. +rollback discards is what the skill itself created. Pathspec staging (instead +of `git add -A`) keeps drafts and local `.env` files out of the category +commit. ## Known limits @@ -251,6 +267,8 @@ rollback discards is what the skill itself created. suite, it is to fix the check the report names. - Exit 124 is reserved for the watchdog, exactly as in GNU `timeout`: a check that legitimately exits 124 under an active watchdog reads as TIMEOUT. + Exit 137 reads the same way while the watchdog runs with `-k`, since that is + the code the kill-after escalation produces against a check that ignores TERM. - With a single `.sln`/`.slnx` at the root the gate passes it explicitly to `dotnet`; with two or more it abstains and invokes with no argument, and the ambiguity is MSBuild's again. It fails closed: run the gate by hand diff --git a/README.md b/README.md index 10f788d..ab4b050 100644 --- a/README.md +++ b/README.md @@ -22,11 +22,11 @@ commitar o que está pendente ou abortar, e nada acontece antes da sua resposta. - `git` — todo o trabalho acontece numa branch `cleanup/YYYYMMDD`, nunca na main. Sem repositório git a skill só diagnostica: o rollback dela depende de ter um commit bom para onde voltar. -- Para projetos JS/TS: Node com `npx` (o knip roda via `npx knip`, sem - instalação prévia). +- Para projetos JS/TS: Node com `npx` (o knip roda via `npx knip@6.32.0`, + versão pinada — nunca `npx knip` sem versão). - Outros stacks usam as ferramentas de cada ecossistema (vulture, deadcode, cargo-udeps, ReferenceTrimmer). O que faltar, a skill aponta em vez de - instalar por conta. + instalar por conta; `pip install` só com confirmação. - O gate (`scripts/gate.sh`) detecta o stack pelo manifesto e roda typecheck + testes em JS/TS, Go, Rust, Python, JVM, Ruby e .NET. O toolchain precisa estar alcançável: no PATH na maioria dos stacks e, em Python, também vale @@ -108,7 +108,7 @@ exercita o GNU `timeout` real em vez do backend perl: ```bash docker run --rm -v "$PWD":/repo:ro node:22-bookworm bash -c \ 'apt-get update -qq && apt-get install -y -qq procps && cd /repo && bash scripts/test.sh' -# validado em 08/2026: 86/86 casos, 5/5 propriedades, 77/77 invariantes +# validado em 08/2026: 127/127 casos, 5/5 propriedades, 151/151 invariantes ``` A heurística .NET foi validada contra o SDK real (`mcr.microsoft.com/dotnet/sdk:8.0` @@ -152,9 +152,13 @@ e se classifica em um de três níveis: | Nível | Condição | O que ela faz | |---|---|---| -| GREEN | typecheck e testes passam | executa as fases sem perguntar; a fase 2 para no checkpoint | -| YELLOW | rede parcial, ou nenhum arquivo de teste no stack | só deps e arquivos órfãos, sem mexer em exports | -| RED | sem testes e sem typecheck, ou baseline já vermelho | só diagnostica; nada é deletado | +| GREEN | typecheck e testes passam | executa a fase 1 sem perguntar; fase 2 e fase 3 param no checkpoint humano | +| YELLOW | rede parcial, ou nenhum arquivo de teste no stack | só deps e arquivos órfãos, sem mexer em exports; não roda fase 2 nem fase 3 | +| RED | sem testes e sem typecheck, ou baseline já vermelho | só diagnostica; nada é deletado; não commit de `CLEANUP_PROGRESS` | + +Os caps por stack em `references/other-stacks.md` sobrescrevem a coluna GREEN +(Python confirma antes de deletar; JVM/Ruby/.NET código ficam em YELLOW ou só +diagnóstico por padrão). Projeto que já chega com a suíte vermelha cai em RED, não em YELLOW: com o baseline quebrado não dá para separar o que a limpeza quebrou do que já estava @@ -163,18 +167,26 @@ skill diz qual check falhou e para por aí. Stack sem nenhum arquivo de teste não conta como testado: o gate não conta suíte vazia, seja porque não a rodou, seja porque rodou e não voltou nada, e o -nível fica em YELLOW. Vale para Go e .NET sem arquivo de -teste, para crate Rust sem `tests/*.rs` nem `#[test]`, para build Maven ou -Gradle sem nenhum `src/test`, e para pytest que sai 5 sem coletar nada. -Manifesto que está ali só por ferramenta — um `requirements.txt` do build da -documentação, um `Gemfile` do fastlane — não é stack sem suíte: sem código -daquela linguagem no repositório, o gate não fala dele. Se a sua suíte mora -fora do lugar padrão, a promoção é sua — o gate não se promove sozinho. - -Em JS/TS o mesmo cap pega a suíte fatiada: sem script `test`, com `test:unit` -e `test:e2e` no manifesto, nenhuma fatia responde pela suíte inteira e o gate -não conta nenhuma delas. Promover à mão aqui é o caminho errado, porque a -suíte não está fora do lugar, está dividida; rode as fatias todas. Uma fatia +nível fica em YELLOW. Vale para JS/TS cujo runner sai com suíte vazia +("No test files found") — inclusive quando ele sai 0 porque mandaram, como em +`--passWithNoTests`, já que exit 0 não é prova de que uma suíte rodou —, +Go e .NET sem arquivo de teste, crate Rust sem +`tests/*.rs` nem `#[test]`, build Maven ou Gradle sem nenhum `src/test`, Ruby +cujo `spec/` ou `test/` não guarda nenhum `*_spec.rb`, `*_test.rb` nem +`test_*.rb` (o padrão do `Rake::TestTask`), e pytest +que sai 5 sem coletar nada. Manifesto que está ali só por ferramenta — +um `requirements.txt` do build da documentação, um `Gemfile` do fastlane — +não é stack sem suíte: sem código daquela linguagem no repositório, o gate +não fala dele. Se a sua suíte mora fora do lugar padrão, a promoção é sua — +o gate não se promove sozinho. + +Em JS/TS o placeholder exato do `npm init` (`echo "Error: no test specified" +&& exit 1`) também cai em YELLOW com a linha `'test' not counted` e o marcador +`npm init placeholder` — não é RED de suíte quebrada. O mesmo cap pega a suíte +fatiada: sem script `test`, com `test:unit` e `test:e2e` no manifesto, nenhuma +fatia responde pela suíte inteira e o gate não conta nenhuma delas. Promover +à mão aqui é o caminho errado, porque a suíte não está fora do lugar, está +dividida; rode as fatias todas. Uma fatia sozinha vale como a suíte, com uma exceção: modo watch nunca termina, então o gate não o executa. `watch`, `ui` e `debug` são lidos como segmentos inteiros do nome, o que pega `test:watch:all` e deixa `test:watchdog` em paz. Só que não @@ -192,8 +204,9 @@ Com o nível anunciado, ela cria a branch de limpeza e segue: - **Fase 1 — código morto.** Configura o knip até os hints zerarem, roda em modo produção e deleta em commits atômicos, um por categoria: deps não - usadas, arquivos órfãos, exports mortos. Cada commit só entra com gate - verde. No fim, produz uma auditoria do que sobrou. + usadas, arquivos órfãos, exports mortos. Cada passo faz stage só com + pathspecs dos artefatos daquele passo (`git add -- …`, nunca `git add -A`), + e só entra com gate verde. No fim, produz uma auditoria do que sobrou. - **Fase 1.5 — funções duplicadas** (fecha a fase 1). Varre funções com nomes diferentes fazendo a mesma coisa (similarity-ts ou fallow em JS/TS, jscpd nos demais stacks) e aplica a regra do churn: par que muda junto no git é @@ -224,12 +237,15 @@ git revert # desfaz só aquela categoria O merge da branch é decisão sua, no seu tempo. A skill nunca faz push, nunca commita na main e nunca usa `git reset --hard` — o rollback dela é `git restore --staged --worktree .`, que joga fora tudo o que ainda não foi -commitado e convive com hooks que bloqueiam comandos destrutivos. +commitado. Se um hook de segurança bloquear esse restore, a skill **aborta** +o pipeline (não contorna o hook): reporta a branch, a árvore suja e o comando +manual, e para. Note o "tudo": alteração sua que estava no diretório antes de a skill começar entraria nessa conta. É por isso que ela exige árvore limpa no início e interrompe para perguntar quando não está — com a árvore limpa, o que o -rollback joga fora foi ela mesma que criou. +rollback joga fora foi ela mesma que criou. O stage por pathspec (em vez de +`git add -A`) evita engolir rascunhos e `.env` locais no commit da categoria. ## Limites conhecidos @@ -244,7 +260,9 @@ rollback joga fora foi ela mesma que criou. passo é criar uma verificação mínima; com a suíte quebrada, é consertar o check que o relatório nomeia. - Exit 124 é reservado ao watchdog, igual ao GNU `timeout`: um check que - legitimamente sai 124 sob watchdog ativo é lido como TIMEOUT. + legitimamente sai 124 sob watchdog ativo é lido como TIMEOUT. Exit 137 vale + o mesmo enquanto o watchdog roda com `-k`, porque é o código que a escalada + kill-after produz contra um check que ignora TERM. - Com uma única `.sln`/`.slnx` na raiz o gate a passa explícita ao `dotnet`; com duas ou mais ele se abstém e invoca sem argumento, e a ambiguidade volta a ser do MSBuild. Falha fechada: rode o gate manual apontando a diff --git a/SKILL.md b/SKILL.md index d49c7f6..731f994 100644 --- a/SKILL.md +++ b/SKILL.md @@ -1,6 +1,6 @@ --- name: codebase-cleanup -description: Full three-phase codebase cleanup — removes dead code (knip/vulture/cargo-udeps), consolidates shallow modules and reorganizes the folder structure, running autonomously with atomic commits and automatic rollback. Use WHENEVER the user mentions cleaning up, organizing, tidying or refactoring the project, or says "dar uma faxina" or "dá uma limpada"; talks about dead code, orphan files, unused dependencies, tech debt, messy folders, confusing structure or a bloated codebase; mentions duplicated code, duplicate functions, copy-paste code, "código duplicado" or "função repetida"; asks to "give the codebase a deep clean", to "reorganiza essas pastas", or for an audit or health check; or says the repo "grew too big", "is hard to navigate", "cresceu demais" or "tem coisa que ninguém usa". Also use when the user wants only one of the three phases on its own. Do NOT use for formatting or lint, vulnerable dependency updates, bundle size optimization, database cleanup or git history rewriting. +description: Full three-phase codebase cleanup — removes dead code (knip/vulture/cargo-udeps), consolidates shallow modules and reorganizes the folder structure, running autonomously with atomic commits and rollback. Use WHENEVER the user mentions cleaning up, organizing, tidying or refactoring the project, or says "dar uma faxina" or "dá uma limpada"; talks about dead code, orphan files, unused dependencies, tech debt, messy folders, confusing structure or a bloated codebase; mentions duplicated code, duplicate functions, copy-paste code, "código duplicado" or "função repetida"; asks to "give the codebase a deep clean", to "reorganiza essas pastas", or for an audit or health check; or says the repo "grew too big", "is hard to navigate", "cresceu demais" or "tem coisa que ninguém usa". Also use when the user wants only one of the three phases on its own. Do NOT use for formatting or lint, vulnerable dependency updates, bundle size optimization, database cleanup or git history rewriting. --- # Codebase Cleanup @@ -32,22 +32,35 @@ and both are the pipeline's job to guarantee: the tree was clean when the work started (Step 0 stops if it was not), and everything the step changed is staged before the gate runs — `git restore --staged --worktree .` undoes a staged new file, but leaves an untracked one behind. Hence the rule that repeats at every -step: **`git add -A` before the gate.** +step: **stage with pathspecs of this step's artifacts before the gate** — +`git add -- `, never `git add -A` or +`git add .`, which would swallow unrelated untracked files (drafts, a local +`.env`, secrets) into the category commit. Never use `git reset --hard` or `git clean`. `restore` covers the case and survives environments with hooks that block destructive commands; `git clean` would additionally wipe untracked files that must survive — tool output, caches, local env files that never belonged to this cleanup. -**If a security hook blocks a command from the protocol** (rollback, branch -creation, file deletion), do not work around it: record the pending item in -`CLEANUP_PROGRESS.md`, hand the ready-to-run command to the user and move on to -the next step that does not depend on it. A guard is environment policy, not an -obstacle. +**If a security hook blocks a command from the protocol**, do not work around +it. Branch on which command was blocked: -There is **one scheduled checkpoint** in the entire pipeline (phase 2, choosing -the consolidation candidate) and **one conditional stop** at Step 0, when the -working tree is dirty before anything starts. Everything else runs on its own. +- **Rollback** (`git restore --staged --worktree .`) — **abort the pipeline.** + The failed category's work is still in the tree; the next stage would fold + it into another category and destroy atomic revert. Report: the branch name, + that the tree is dirty with the failed step's changes, and the exact command + the user must run by hand (`git restore --staged --worktree .`, or their + environment's approved equivalent). Do not start the next category or folder. +- **Anything else** (branch creation, file deletion, …) — record the pending + item in `CLEANUP_PROGRESS.md`, hand the ready-to-run command to the user and + move on to the next step that does not depend on it. + +A guard is environment policy, not an obstacle. + +There are **two scheduled checkpoints** in the pipeline (phase 2, choosing the +consolidation candidate; phase 3 on GREEN, confirming the folder plan before +any `git mv`) and **one conditional stop** at Step 0, when the working tree is +dirty before anything starts. Everything else runs on its own. --- @@ -115,15 +128,24 @@ nothing about the code: treat it as red (rollback, record what timed out in `CLEANUP_PROGRESS.md`) and never promote it to GREEN. On the Step 0 baseline, exit 4 means the safety net could not be measured — report it and do not run autonomously. Exit code 124 is reserved for the watchdog, exactly as in GNU -timeout: a check that legitimately exits 124 is read as a timeout. +timeout: a check that legitimately exits 124 is read as a timeout. So is 137 +(128+SIGKILL) while the watchdog runs with `-k`, because that is what the +kill-after escalation produces against a check that ignores TERM — reading it +as a plain failure would report a hung check as a broken one. | Signal | Level | Behavior | |---|---|---| -| Typecheck **and** tests pass | **GREEN** | Runs phases 1 and 3 in full without asking. Phase 2 stops at the checkpoint. | -| A partial net, or no test file in the stack | **YELLOW** | Runs phase 1 (deps and orphan files only, **not** exports). Stops before phase 2 and reports. | -| A check fails, or no tests and no typecheck | **RED** | Diagnoses only. Does not delete, does not move, does not commit. Delivers a report. | +| Typecheck **and** tests pass | **GREEN** | Runs phase 1 in full without asking. Phase 2 and phase 3 each stop at their human checkpoint before mutating. | +| A partial net, or no test file in the stack | **YELLOW** | Runs phase 1 (deps and orphan files only, **not** exports). Does **not** run phase 2 or phase 3; reports and stops. | +| A check fails, or no tests and no typecheck | **RED** | Diagnoses only. Does not delete, does not move, does not commit. May create the cleanup branch; does **not** commit `CLEANUP_PROGRESS.md`. Delivers a report. | | No git repository | **RED** | Diagnoses only, regardless of the gate result — there is no HEAD to roll back to. | +**Stack caps in `references/other-stacks.md` override the GREEN column.** +Python always confirms before deleting; JVM and Ruby are diagnosis / YELLOW by +default; .NET is YELLOW by default for code (deps flagged by the compiler may +go). Read that file before autonomous deletion — a GREEN gate on a stack whose +graph is unreliable does not unlock code deletion. + `checks=typecheck` because the stack has **no test file** is YELLOW, not GREEN, even though the gate exits 0 — a suite that does not exist cannot pass. The script names the stack in the `'test' not counted` line, and every stack whose @@ -136,30 +158,33 @@ lives somewhere the gate does not look; the skill never promotes itself. **Which npm script the gate reads.** For typecheck it takes the first of `typecheck`, `type-check`, `check-types` the manifest defines, and stops there. -`tsc` is deliberately not on that list: as a script name it usually means an -emitting compile, and the output would land beside the sources right before -`git add -A` stages everything. The reverse is not covered: a script *named* -`typecheck` whose body is `tsc -p .` with no `--noEmit` emits just the same, and -the gate cannot tell. If the manifest has one, read it before phase 1. +`tsc` is not on that list by name alone: as a script name it usually means an +emitting compile, and the output would land beside the sources. Exception: when +the script *value* carries `--noEmit` as a real shell word (comments stripped), +`tsc` counts as typecheck — a trailing `# use --noEmit in CI` does not. The +reverse is still uncovered: a script *named* `typecheck` whose body is +`tsc -p .` with no `--noEmit` emits just the same, and the gate cannot tell. If +the manifest has one, read it before phase 1. For the suite it takes `test`; failing that, a lone `test:*` script, since a repo that declares one slice and no whole is declaring its suite. Two or more slices and no `test` count as nothing — half a net classified GREEN would unlock dead-export deletion on code the other half covers — and a slice -declared with an empty command still counts as one of them. A watch-mode slice -never runs at all, because it does not exit: `watch`, `ui` and `debug` are -matched as whole segments of the name, so `test:watch:all` is caught and -`test:watchdog` is not. Not running one and not counting one are separate -questions. `watch` and `debug` name a mode of the suite — the same tests, -started so they never stop — so a `test:watch` beside a lone `test:unit` does -not split anything and the real slice is still the suite. `ui` is not a mode -word: `vitest --ui` does not exit either, but `test:ui` is just as often a scope -of its own, so it is never run *and* never leaves the count — `test:unit` next -to `test:ui` is two slices, not one. All of these print the `'test' not counted` -line naming the slices, and so does a manifest that declares no test script at -all. A split -suite is **not** promotable by hand: it is in the manifest, only divided, so -run every slice before deciding. +declared with an empty command still counts as one of them. The exact npm-init +placeholder (`echo "Error: no test specified" && exit 1`) is recognised by +value and reported as `'test' not counted` with `npm init placeholder` — YELLOW, +not RED. A watch-mode slice never runs at all, because it does not exit: +`watch`, `ui` and `debug` are matched as whole segments of the name, so +`test:watch:all` is caught and `test:watchdog` is not. Not running one and not +counting one are separate questions. `watch` and `debug` name a mode of the +suite — the same tests, started so they never stop — so a `test:watch` beside a +lone `test:unit` does not split anything and the real slice is still the suite. +`ui` is not a mode word: `vitest --ui` does not exit either, but `test:ui` is +just as often a scope of its own, so it is never run *and* never leaves the +count — `test:unit` next to `test:ui` is two slices, not one. All of these print +the `'test' not counted` line naming the slices, and so does a manifest that +declares no test script at all. A split suite is **not** promotable by hand: it +is in the manifest, only divided, so run every slice before deciding. **A baseline that already fails is RED, not YELLOW.** Exit 1 says a check broke, and a broken baseline leaves no way to tell what the cleanup broke from @@ -184,12 +209,18 @@ force, never delete the branch that is in the way. ## Step 0.1 — Persistent state -Create `CLEANUP_PROGRESS.md` at the root and **commit it right away** -(`chore: start cleanup log`), before any other work. It is the only artifact -that has to outlive a rollback: an untracked file survives +**At RED, skip this step.** Do not create or commit `CLEANUP_PROGRESS.md` — +diagnosis goes only into the final report. The cleanup branch from Step 0 may +exist; nothing else is written into the repo. + +On GREEN/YELLOW, create `CLEANUP_PROGRESS.md` at the root and **commit it +right away** (`chore: start cleanup log`), before any other work. It is the +only artifact that has to outlive a rollback: an untracked file survives `git restore --staged --worktree .` by accident, a committed one survives by design, and a staged one would be thrown away with the failed step. Keep it updated at the end of every step, committed along with that step's changes. +Stage it with a pathspec (`git add -- CLEANUP_PROGRESS.md`), never +`git add -A`. Phases are separated by `/clear` (dirty context from one phase degrades the next), so this file is what allows resuming without the user re-explaining @@ -232,9 +263,10 @@ The contract of each delegation: - returning a summary of what it did, with `CLEANUP_PROGRESS.md` updated as the canonical state. -Step 0 (calibrating the level, creating the branch) and the phase 2 checkpoint -stay with the orchestrator — a subagent does not talk to the user. Level and -branch reach the subagents ready-made, through `CLEANUP_PROGRESS.md`. +Step 0 (calibrating the level, creating the branch), the phase 2 checkpoint +and the phase 3 checkpoint stay with the orchestrator — a subagent does not +talk to the user. Level and branch reach the subagents ready-made, through +`CLEANUP_PROGRESS.md`. --- @@ -251,10 +283,11 @@ assumes JS/TS (step 1.5 names its own fallback for other stacks). ## 1.1 Configure knip until the hints reach zero -Run `npx knip` with no config at all first. knip has plugins for the vast -majority of the ecosystem's tools (Next, Vitest, ESLint, Playwright, and dozens -of others) that read their configuration and work out entry points on their own -— writing config before seeing what it already knows is wasted work. +Run `npx knip@6.32.0` with no config at all first (pin verified 2026-08-09; +never bare `npx knip`). knip has plugins for the vast majority of the +ecosystem's tools (Next, Vitest, ESLint, Playwright, and dozens of others) +that read their configuration and work out entry points on their own — writing +config before seeing what it already knows is wasted work. **Handle the configuration hints before looking at any finding.** Hints mean knip could not resolve a dependency, plugin or entry file — that is, the graph @@ -281,7 +314,7 @@ graph (entry, project, paths, plugin), not to silence the output. ## 1.2 Run in production mode ```bash -npx knip --production --no-exit-code --reporter json > knip-report.json.tmp && mv knip-report.json.tmp knip-report.json +npx knip@6.32.0 --production --no-exit-code --reporter json > knip-report.json.tmp && mv knip-report.json.tmp knip-report.json ``` Write to a temp file and move only on success. A plain `> knip-report.json` @@ -306,54 +339,68 @@ Never exclude tests with `ignore` to get the same effect. ## 1.3 Delete in atomic commits, one per category -Run all three without asking (GREEN level) or the first two (YELLOW). Each one -is: delete → (deps only: install) → `git add -A` → gate → commit → regenerate -the report. For the gate, use `scripts/gate.sh` (it detects the stack and the -package manager and runs typecheck + tests in the right order); if it exits -with code 3, find the stack's own check commands — the `package.json` scripts, -the tox env, the Makefile target, whatever this repo uses — and run them by -hand. - -Staging before the gate is what makes the rollback complete, and it is safe -here precisely because Step 0 refused to start on a dirty tree: everything -`git add -A` picks up was produced by this step, so the `-A` cannot swallow work -of the user's. - -The one thing `-A` picks up that this step did not produce is knip's own -output. Before the first category, put `knip-report.json` and -`knip-report.json.tmp` in the repo's exclude file — repo-local, so the user's -`.gitignore` stays untouched. Ask git where it is rather than typing the path: -`git rev-parse --git-path info/exclude`, because in a linked worktree, a -submodule or a `--separate-git-dir` checkout `.git` is a *file* and appending to -`.git/info/exclude` fails with `Not a directory`. Otherwise every category -commits the report the previous one was read from, and the user reverting -`chore: remove unused deps` gets a tool artifact back along with the -dependencies. +**Default scope.** Run all three without asking (GREEN level) or the first two +(YELLOW). Each one is: delete → (deps only: install / re-resolve) → stage +pathspecs → gate → commit → regenerate the report. For the gate, use +`scripts/gate.sh` (it detects the stack and the package manager and runs +typecheck + tests in the right order); if it exits with code 3, find the +stack's own check commands — the `package.json` scripts, the tox env, the +Makefile target, whatever this repo uses — and run them by hand. + +**Partial run by category.** If the user asked for only one (or some) of the +three categories — e.g. "remove só as dependências não usadas" — run exactly +those, in the order below, and **do not** run the others. Record every skipped +category under `## Decisions` in `CLEANUP_PROGRESS.md` as out of scope for +this run (`orphan files: out of scope — user asked deps only`), and repeat +that list in the final report under "Failed / not done". A category the level +already forbids (YELLOW × exports) is a level cap, not a user scope skip — +say which. + +**Staging — pathspecs only.** Staging before the gate is what makes the +rollback complete. Stage only the paths this category produced or edited: + +```bash +git add -- path/to/edited path/to/removed-file package.json package-lock.json +# deletions of tracked files: `git add -- path` (or `git rm` already staged them) +``` + +Never `git add -A` or `git add .`. Step 0's dirty-tree stop is still the +precondition that nothing of the user's was already pending; pathspecs are +what keep a draft or local `.env` that appears mid-run out of the commit. + +**Artifact hygiene (exclude + close).** Before the first category, put +`knip-report.json` and `knip-report.json.tmp` in the repo's exclude file — +repo-local, so the user's `.gitignore` stays untouched. Ask git where it is +rather than typing the path: `git rev-parse --git-path info/exclude`, because +in a linked worktree, a submodule or a `--separate-git-dir` checkout `.git` +is a *file* and appending to `.git/info/exclude` fails with `Not a directory`. +Otherwise every category would be tempted to commit the report the previous +one was read from, and the user reverting `chore: remove unused deps` would +get a tool artifact back along with the dependencies. That exclude only reaches **untracked** paths. If a previous run of this skill -already committed `knip-report.json`, git keeps staging it no matter what the -exclude says, and the regeneration below puts a fresh diff in every category -commit. Check with `git status --porcelain` after writing the exclude lines; if -the report still shows up, find out whose it is first — `git log -1 --format=%s --- knip-report.json`. A previous run of this skill left `chore:` there and the +already committed `knip-report.json`, git keeps seeing it no matter what the +exclude says, and the regeneration below puts a fresh diff on disk. Check with +`git status --porcelain` after writing the exclude lines; if the report still +shows up, find out whose it is first — `git log -1 --format=%s -- +knip-report.json`. A previous run of this skill left `chore:` there and the file is a tool artifact: untrack it in a commit of its own before the first -category (`git rm --cached knip-report.json`, then `chore: untrack knip -report`). Anything else means the user tracks it on purpose: leave it tracked, -say so in the report, and expect the report's diff in each commit. Do not -untrack it either way as part of another category — left staged, the next `git -add -A` folds that deletion into the deps commit, and reverting the deps -category hands the artifact back, which is the thing the exclude was for. On -the way out, delete the report and drop from the exclude file only the lines -you added — leave anything that was already there, it is the user's — because -the exclude makes a leftover invisible to `git status` and nobody would find it -later. - -One more thing `-A` can swallow, created by this step and not committed on -purpose: `node_modules`, after the install below. Confirm the repo ignores it -before the deps category — a global `.gitignore` does not travel with the repo -— and add it to `.git/info/exclude` if it does not. Empty directories are not -on this list: git tracks files, so `git add -A` never stages one. The leftovers -`mkdir -p` creates in phase 3 are a working-tree problem and are swept there. +category (`git rm --cached knip-report.json`, then stage that pathspec only — +`chore: untrack knip report`). Anything else means the user tracks it on +purpose: leave it tracked, say so in the report, and do **not** pathspec-add +it into category commits. Do not fold that untrack into another category. On +the way out (final report / close), delete only report files that are tool +artifacts from this run (untracked, or untracked earlier via +`chore: untrack knip report`) — never delete a `knip-report.json` the user +tracks on purpose. Drop from the exclude file only the lines you added — +leave anything that was already there, it is the user's — because the exclude +makes a leftover invisible to `git status` and nobody would find it later. + +Also confirm the repo ignores `node_modules` before the deps category — a +global `.gitignore` does not travel with the repo — and add it to +`info/exclude` if it does not. Empty directories are not staged by pathspec +adds either: git tracks files. The leftovers `mkdir -p` creates in phase 3 +are a working-tree problem and are swept there. ``` 1. unused deps → "chore: remove unused deps" @@ -369,7 +416,9 @@ user needs to revert *one* commit — not a 400-file cleanup. never installs — the resolver still finds the package on disk, typecheck and tests pass, and the break only surfaces on CI or on the next machine that installs from the pruned manifest. So, after editing the manifest and before -`git add -A`, run the package manager's plain install: +staging pathspecs, run the package manager's plain install (non-JS/TS stacks: +see the matching re-resolve / tidy / restore step in +`references/other-stacks.md`): ```bash npm install # npm @@ -404,12 +453,13 @@ the final report counts against. **If the gate fails:** `git restore --staged --worktree .`, record the category as failed in `CLEANUP_PROGRESS.md` along with the error, and **move on to the -next category**. -Do not stop the entire pipeline and do not try to fix it — if typecheck broke, -knip was wrong about that category, and the useful information is which -category, not a patch. On the deps category, that restore brings back -`package.json` and the lockfile but not `node_modules`: run the install again -before starting orphan files. +next category**. If that restore is blocked by a hook, **abort** — see the +operating principle; do not start the next category with a dirty tree. Do not +try to fix a red gate — if typecheck broke, knip was wrong about that +category, and the useful information is which category, not a patch. On the +deps category, a successful restore brings back `package.json` and the +lockfile but not `node_modules`: run the install again before starting orphan +files. Do not run `knip --fix` until the config has settled for two or three rounds with no surprises. @@ -440,9 +490,10 @@ two folders is a more obvious consolidation candidate than anything depth analysis surfaces alone. For JS/TS the ladder is: `similarity-ts` (AST comparison per function) if it -is on PATH, else `npx fallow dupes` (no install needed); other stacks fall -back to `jscpd`. Tools, flags, thresholds and the report format are in -`references/duplication.md` — read it before running anything. +is on PATH, else `npx fallow@3.14.0 dupes` (pin verified 2026-08-09; never +bare `npx fallow`); other stacks fall back to `npx jscpd@5.0.14`. Tools, +flags, thresholds and the report format are in `references/duplication.md` — +read it before running anything. **The churn rule.** High similarity is not a verdict. Check whether the pair changes together in git history: pairs that co-change are real duplication @@ -469,11 +520,12 @@ analysis vocabulary. ## The irreducible checkpoint -This is the only point in the pipeline that requires the human, and it is worth -explaining why: consolidating modules changes responsibility boundaries, and -that is a decision about the *domain*, not about the code. Green tests do not -prove the new boundary is the right one — they prove the behavior did not -change, which is a different thing. +Phase 2 is the irreducible *domain* checkpoint: consolidating modules changes +responsibility boundaries, and that is a decision about the *domain*, not about +the code. Green tests do not prove the new boundary is the right one — they +prove the behavior did not change, which is a different thing. Phase 3 adds a +second scheduled checkpoint (plan confirmation before any `git mv`); both are +required on GREEN. Keep the cost of the checkpoint to a minimum: @@ -501,12 +553,13 @@ with a simple interface is exactly what you *want*). ## Implementation After the choice, run on your own: one module at a time, one commit per -consolidation, and `scripts/gate.sh` once — after `git add -A`, right before -the commit. Do not gate between the intermediate steps: with the new interface -in place and the callers not migrated yet, the build is red by construction, -and a gate you expect to fail teaches nothing. The green that matters is the -one at the end of the consolidation, which is exactly the state that gets -committed. Same rollback protocol as phase 1. +consolidation, and `scripts/gate.sh` once — after staging pathspecs of what +this consolidation touched, right before the commit. Do not gate between the +intermediate steps: with the new interface in place and the callers not +migrated yet, the build is red by construction, and a gate you expect to fail +teaches nothing. The green that matters is the one at the end of the +consolidation, which is exactly the state that gets committed. Same rollback +protocol as phase 1 (including abort if a hook blocks the restore). **One candidate per session.** Do not stack two — the second refactor inherits the dirty context of the first and the error rate goes up. @@ -531,9 +584,22 @@ Produce a phased plan before moving anything: a map of the current structure, circular dependencies, god modules, leaking abstractions, and the target structure with a rationale. Only then execute. -## Autonomous execution +## Checkpoint, then autonomous execution -GREEN level runs the whole plan without asking. One folder per commit: +**YELLOW does not run phase 3** — stop after the phase 1 report (and after +phase 2 only if the user later promotes the level). **RED** never reaches +here. + +**GREEN: human checkpoint before any move.** Present the phased plan from the +diagnosis (current map → target structure → ordered moves, at most a short +list). Recommend one first move in a single sentence. Ask **one** question: +whether to proceed with that plan (or which slice). Do not start `git mv` +until the user answers. If they say "go" or equivalent, execute the agreed +plan; if they narrow the scope, record the rest as out of scope in +`CLEANUP_PROGRESS.md`. + +After the checkpoint, run the agreed plan without re-asking per folder. One +folder per commit: ```bash mkdir -p src/features/billing @@ -554,10 +620,12 @@ The move, the import or alias update and the `CLAUDE.md` update go in the **same commit**. Splitting them would put a commit that does not build in the history, and there is no gate that a half-done move can pass. -`git add -A` and then `scripts/gate.sh` at the end of each folder — typecheck -alone misses what a move actually breaks (config paths, dynamic imports; the -"Do not forget" list in `references/phase-3-structure.md` has the rest). -Failed: `git restore --staged --worktree .`, record it, next folder. +Stage with pathspecs of what this folder move touched (`git add -- …`), never +`git add -A`, and then `scripts/gate.sh` at the end of each folder — +typecheck alone misses what a move actually breaks (config paths, dynamic +imports; the "Do not forget" list in `references/phase-3-structure.md` has +the rest). Failed: `git restore --staged --worktree .`, record it, next +folder. If that restore is blocked by a hook, **abort** the pipeline. --- @@ -583,6 +651,14 @@ branch. Merging is the user's decision, on their own schedule. ## Final report +**Close hygiene first** (GREEN/YELLOW, when the run wrote artifacts): delete +`knip-report.json.tmp` if present, and delete `knip-report.json` only when it +is an untracked tool artifact from this run (see Artifact hygiene above) — if +the user tracks it on purpose, leave the tracked file alone; drop from +`info/exclude` only the lines this run added; leave `CLEANUP_PROGRESS.md` and +`TECH_DEBT_AUDIT.md` committed unless the user asked to remove them — they are +the durable record, not tool noise. Say in the summary what was cleaned. + When wrapping up (or when interrupted), deliver: ```markdown @@ -601,6 +677,7 @@ Branch: `cleanup/YYYYMMDD` · Level: GREEN · N commits ### Failed / not done - dead exports: typecheck broke in `src/api/routes.ts` (dynamic import) +- orphan files: out of scope — user asked deps only ### Pending your decision - (nothing) @@ -610,8 +687,9 @@ The phase 1 line counts what each category actually removed, tallied per commit from the report that category ran on — not the numbers of the first report, which stopped describing the repo the moment the first commit landed. The last regeneration settles the rest: whatever it still lists is what survived, and it -belongs under "Failed / not done", along with any category that was skipped. +belongs under "Failed / not done", along with any category that was skipped +(level cap or user scope). If the level was RED, the report is diagnosis only: list what you would do and what needs to exist or be fixed (tests, typecheck, a baseline that passes) to -make it possible. +make it possible. No `CLEANUP_PROGRESS.md` commit on RED. diff --git a/references/audit.md b/references/audit.md index a41a827..a941b7e 100644 --- a/references/audit.md +++ b/references/audit.md @@ -44,7 +44,8 @@ linters — much of their output already exists from phase 1; reuse it instead of re-running. A tool missing from PATH becomes a note in the report, never an installation — the same rule as the rest of the skill. -1. **Architectural decay** — circular deps (`npx madge --circular` on JS/TS, +1. **Architectural decay** — circular deps (`npx madge@8.0.0 --circular` on + JS/TS — pin verified 2026-08-09; never bare `npx madge` — `pydeps --show-cycles` on Python; other stacks in `references/other-stacks.md`), layering violations, god files (>500 LOC) and god functions, abstractions that exist but nobody uses, duplicated @@ -74,6 +75,13 @@ an installation — the same rule as the rest of the skill. 8. **Security hygiene** — hardcoded secrets, string-concat SQL, missing input validation at trust boundaries, permissive auth or CORS, weak crypto. Hygiene only — this is not a pen test or threat model. + **Redaction (mandatory):** cite `file:line` only — never paste the secret + value, a key fragment, a token, or the contents of a `.env` / credentials + file into the report, the chat summary, or `CLEANUP_PROGRESS.md`. Write + something like `hardcoded credential at src/config.ts:42` and stop. A + secret finding is still listed in the committed `TECH_DEBT_AUDIT.md` with + that redacted citation; the literal never appears anywhere the pipeline + writes. Treat the audit file as sensitive if it names secret locations. 9. **Documentation drift** — README claims that no longer match reality, comments contradicting adjacent code, public APIs without docs. @@ -99,6 +107,10 @@ written into the repo — the same content goes into the final report instead. ## Rules - A finding without a citation is a vibe, and vibes do not get fixed. +- **Never commit a secret literal.** Dimension 8 citations are `file:line` + without the value; if a tool dumps a secret into stdout, discard that + output and restate the finding redacted. The committed report must not + contain the secret. - No sycophancy and no filler. Never open with "overall the codebase is well-structured" — say what is broken. - Read code before judging it: a pattern that looks wrong in isolation may diff --git a/references/duplication.md b/references/duplication.md index fb1bccd..4a6e22f 100644 --- a/references/duplication.md +++ b/references/duplication.md @@ -43,20 +43,23 @@ The same author ships `similarity-py`/`similarity-rs` (beta) and jscpd instead because it is the mature option, not because the family does not exist. -**2. fallow** — runs without installing anything (`npx fallow dupes`) and -finds clone families (not just pairs) with four modes: `strict` (exact -tokens), `mild` (default), `weak` (different literals), `semantic` (renamed -variables). Start on `mild`, escalate to `semantic` if the output is thin. -Also covers dead code, complexity and boundaries in the same binary. TS/JS -only. +**2. fallow** — runs without a global install (`npx fallow@3.14.0 dupes`; +pin verified 2026-08-09) and finds clone families (not just pairs) with four +modes: `strict` (exact tokens), `mild` (default), `weak` (different +literals), `semantic` (renamed variables). Start on `mild`, escalate to +`semantic` if the output is thin. Also covers dead code, complexity and +boundaries in the same binary. TS/JS only. Always pin the exact version — +never bare `npx fallow`. **3. jscpd** — the multi-language fallback (223 formats), and the only rung for non-JS/TS stacks: ```bash -npx jscpd src --reporters ai --min-tokens 50 --cross-formats "js-ts" +npx jscpd@5.0.14 src --reporters ai --min-tokens 50 --cross-formats "js-ts" ``` +Pin verified 2026-08-09. Always `npx jscpd@5.0.14`, never bare `npx jscpd`. + - Token-level: it finds copy-paste (including with edits), **not** renamed intent. In Python, Go or Java this is the honest ceiling; say so in the report instead of overselling the sweep. diff --git a/references/knip-config.md b/references/knip-config.md index 2e43c5d..687574b 100644 --- a/references/knip-config.md +++ b/references/knip-config.md @@ -2,10 +2,12 @@ Read this before writing `knip.json`. Applies to JS/TS only. -This guide was written against knip v6. Check the installed major with -`npx knip --version`; if it differs, validate whatever diverges against the -official docs (knip.dev) — especially the schema, cycle detection and option -names. Adjust the `$schema` version below to the major in use. +This guide was written against knip v6. The pipeline pins +`npx knip@6.32.0` (verified 2026-08-09); check that pin with +`npx knip@6.32.0 --version`. If you intentionally use another major, validate +whatever diverges against the official docs (knip.dev) — especially the +schema, cycle detection and option names. Adjust the `$schema` version below +to the major in use. Never bare `npx knip`. ## Index - [Order of work](#order-of-work) @@ -20,11 +22,11 @@ names. Adjust the `$schema` version below to the major in use. ## Order of work -1. `npx knip` with no config at all +1. `npx knip@6.32.0` with no config at all 2. Resolve **every** configuration hint 3. Only then write/adjust `knip.json` 4. Repeat 2–3 until the hints reach zero -5. `npx knip --production --no-exit-code --reporter json > knip-report.json.tmp && mv knip-report.json.tmp knip-report.json` +5. `npx knip@6.32.0 --production --no-exit-code --reporter json > knip-report.json.tmp && mv knip-report.json.tmp knip-report.json` 6. Check the result before using it: `knip-report.json.tmp` gone, and `knip-report.json` non-empty and parsing as JSON @@ -81,7 +83,7 @@ without `*` are exact matches. Each workspace can have its own `paths`. ## Production mode ```bash -npx knip --production +npx knip@6.32.0 --production ``` Excludes tests and devDependencies automatically. This is what separates "alive" @@ -182,7 +184,7 @@ ones it depends on. Add `--strict` so each dependency is required to be in that workspace's own `package.json` instead of being resolved from the root: ```bash -npx knip -W packages/api --strict +npx knip@6.32.0 -W packages/api --strict ``` ## Cycle detection diff --git a/references/other-stacks.md b/references/other-stacks.md index 471cd32..7ff8577 100644 --- a/references/other-stacks.md +++ b/references/other-stacks.md @@ -6,10 +6,25 @@ changes is the tooling and, importantly, how **reliable** it is. Detect the stack from the manifest: `package.json`, `pyproject.toml`/`requirements.txt`, `go.mod`, `Cargo.toml`, `pom.xml`/`build.gradle`, `Gemfile`, `*.sln`/`*.csproj`. +**Stack caps override the GREEN column in `SKILL.md`.** When this file says +confirm, stay at YELLOW by default, or diagnosis only, that rule wins even if +the gate printed GREEN — the executor must read this table before deleting. +The levels table in `SKILL.md` points here for that reason. + ## Python +**Do not run `pip install` (or `uv tool install` / `pipx`) without an explicit +confirmation from the user.** Installing into the active interpreter can write +into the project venv under cleanup, and on PEP 668 distros a bare +`pip install` aborts with `externally-managed-environment`. Ask once, naming +the tools and the preferred install path (`pipx`, `uv tool`, or the project's +own venv), and wait. If the tools are already on PATH, skip the install. + ```bash -pip install vulture pip-audit ruff deptry --quiet +# Only after the user confirms — prefer an isolated install: +# pipx install vulture pip-audit ruff deptry +# # or: uv tool install … +# # or: python -m pip install … inside the project's venv, if they say so vulture src/ --min-confidence 80 # dead code pip-audit # CVEs ruff check --select F401,F841 src/ # unused imports and variables @@ -24,7 +39,8 @@ anything reached through `getattr`. Vulture is syntactic analysis, not a module graph. It is **substantially less reliable than knip** — treat its output as a list of candidates to investigate, never as a deletion list. At GREEN level, still confirm before deleting in -Python. +Python — autonomous deletion caps at YELLOW for this stack (see the table +below). Whitelist whatever it consistently gets wrong: @@ -32,6 +48,22 @@ Whitelist whatever it consistently gets wrong: vulture src/ whitelist.py ``` +**Unused deps: re-resolve after pruning the manifest.** Editing +`pyproject.toml` / `requirements*.txt` does not remove the package from an +already-populated venv, so the gate can still import it. After pruning and +before staging, re-resolve with the project's tool — never a frozen/locked +form that refuses an intentionally changed lock: + +```bash +pip install -r requirements.txt # plain requirements +# or, matching the repo: +poetry lock && poetry install # Poetry (not --no-update alone if the lock must drop) +uv sync # uv +pip-compile && pip-sync # pip-tools, when that is the workflow +``` + +The updated lock (when the project has one) goes in this category's commit. + ## Go ```bash @@ -49,6 +81,17 @@ for orphan files. Watch out for exported identifiers: in a library package, exported with no internal use is the public interface, not dead code. +**Unused deps: `go mod tidy` after pruning `go.mod`.** Removing a require line +by hand leaves the old graph in `go.sum` / the module cache view the next +build may still see. After editing the manifest and before staging, run: + +```bash +go mod tidy +``` + +`go.sum` changes belong in this category's commit. Do not use a mode that +refuses a changed `go.mod` — tidy is the sync step. + ## Rust ```bash @@ -62,6 +105,18 @@ The compiler already reports `dead_code` — `#![warn(dead_code)]` plus reading the warnings covers most of it. Watch out for `#[cfg(feature = "...")]`: code behind a disabled feature shows up as dead. +**Unused deps: refresh the lock after pruning `Cargo.toml`.** A removed +dependency can still satisfy resolution from `Cargo.lock` until the lock is +rewritten. After editing the manifest and before staging: + +```bash +cargo generate-lockfile +# or a plain `cargo check` / `cargo build` that rewrites Cargo.lock — not +# `--locked` / `--frozen`, which refuse the intentional prune +``` + +`Cargo.lock` goes in this category's commit when the project tracks it. + ## Java / Kotlin ```bash @@ -73,6 +128,12 @@ Reflection, DI (Spring), annotations and SPI make static analysis unreliable. On a JVM stack, stay at YELLOW level by default: remove only declared-and-unused dependencies, and treat code as diagnosis. +**Unused deps:** after pruning the Maven/Gradle manifest, refresh the resolved +classpath before the gate (plain `mvn -q dependency:resolve` or +`./gradlew dependencies`, not a CI "frozen" restore that assumes the old +lock). Module metadata / lockfile updates that the prune produced belong in +the category commit. + ## Ruby ```bash @@ -80,7 +141,12 @@ bundle exec debride app/ # possibly dead code bundle-audit check # CVEs ``` -Metaprogramming makes any result suspect. Diagnosis only. +Metaprogramming makes any result suspect. Diagnosis only — never autonomous +deletion of Ruby code under this skill, regardless of the gate colour. + +**Unused deps:** after pruning the `Gemfile`, run a plain +`bundle install` (not `--deployment` / `--frozen`) so `Gemfile.lock` matches +before the gate; the lock update goes in the category commit. ## .NET (C# / F#) @@ -105,6 +171,19 @@ Reflection, DI (ASP.NET Core), serialization and attribute-based discovery have the same effect as on the JVM: code analysis is unreliable. YELLOW by default — deps flagged by the compiler can go, code is diagnosis. +**Unused deps: `dotnet restore` after pruning the project/package refs.** The +assets file and package cache still satisfy the old graph until restore runs. +After editing `.csproj`/`.fsproj`/`Directory.Packages.props` and before +staging: + +```bash +dotnet restore +``` + +Do not use a restore mode that treats the previous lock/assets as immutable +when the prune intentionally changed them. Lockable files the prune updated +go in this category's commit. + --- ## General rule @@ -116,12 +195,17 @@ Confidence in automatic deletion follows the quality of the graph: | JS/TS | knip (module graph) | yes, at GREEN | | Go | deadcode (call graph) | yes, at GREEN | | Rust | compiler + udeps | yes for deps, confirm code | -| Python | vulture (syntactic) | always confirm | -| .NET | ReferenceTrimmer (compiler data) | yes for deps, code is diagnosis | -| JVM / Ruby | — | diagnosis only | +| Python | vulture (syntactic) | always confirm (YELLOW cap for deletion) | +| .NET | ReferenceTrimmer (compiler data) | yes for deps, code is diagnosis (YELLOW default) | +| JVM / Ruby | — | diagnosis only (YELLOW / diagnosis; never promote code deletion) | When the tool does not build a reachability graph, it does not know what is dead — it knows what *looks* dead. The difference matters when the order is to delete without asking. +**After every unused-deps prune**, the stack-specific re-resolve / tidy / +restore above is mandatory before staging and the gate — same reason as the +JS/TS install step in `SKILL.md`: a populated environment makes a blind green +gate. + Phases 2 and 3 are stack-independent and apply to any language. diff --git a/references/phase-2-consolidation.md b/references/phase-2-consolidation.md index 4a2ea5e..d25e6e5 100644 --- a/references/phase-2-consolidation.md +++ b/references/phase-2-consolidation.md @@ -45,9 +45,10 @@ Imagine deleting the module. purest form. 1. **Map.** Import graph, module sizes, who calls whom. knip's `cycles` already covers a good part of it, but it is **not** in the default issue - set — ask for it explicitly with `npx knip --cycles` (shortcut for - `--include cycles`), or a plain `npx knip` will report nothing about - circular dependencies. + set — ask for it explicitly with `npx knip@6.32.0 --cycles` (shortcut for + `--include cycles`; pin verified 2026-08-09), or a plain + `npx knip@6.32.0` will report nothing about circular dependencies. Never + bare `npx knip`. 2. **Cross with churn volume (file-level).** The intersection between "changed a lot" and "heavily coupled" is where consolidation pays the most. Rank the files by @@ -91,7 +92,8 @@ One candidate per session. Per consolidation: 1. Create the new interface 2. Migrate callers 3. Remove the old modules -4. `git add -A` +4. `git add -- ` — pathspecs only, never + `git add -A` / `git add .` 5. `scripts/gate.sh` — once, here, not between the steps above 6. Commit `refactor: consolidate X into Y` @@ -102,9 +104,10 @@ anything — and it guards exactly what goes into the commit. Step 4 is not bookkeeping. `git restore --staged --worktree .` brings back a deleted file and drops a staged new one, but an unstaged new file survives the -rollback and poisons the next step — so the new interface has to be staged -before the gate runs. Staging everything is safe because the tree was clean -when the pipeline started. +rollback and poisons the next step — so every path this consolidation created +or edited has to be staged before the gate runs. Pathspecs keep a draft or +local `.env` that appeared mid-run out of the commit; the Step 0 dirty-tree +stop is still the precondition that nothing of the user's was already pending. Gate failed → `git restore --staged --worktree .`, record it, do not try to fix it. diff --git a/references/phase-3-structure.md b/references/phase-3-structure.md index 5815b08..e68deab 100644 --- a/references/phase-3-structure.md +++ b/references/phase-3-structure.md @@ -6,9 +6,11 @@ Before moving any file, produce: 1. **Map of the current structure** — depth, what lives where, what has no clear owner -2. **Circular dependencies** — from knip's `cycles` or from `madge --circular`. - `cycles` is not in knip's default issue set: run `npx knip --cycles` - (shortcut for `--include cycles`), otherwise the report comes back silent +2. **Circular dependencies** — from knip's `cycles` or from + `npx madge@8.0.0 --circular` (pin verified 2026-08-09; never bare + `npx madge`). `cycles` is not in knip's default issue set: run + `npx knip@6.32.0 --cycles` (shortcut for `--include cycles`; pin verified + 2026-08-09). Never bare `npx knip`. Otherwise the report comes back silent on this point 3. **God modules** — directories that everyone imports 4. **Leaking abstractions** — a module's internal detail referenced from @@ -99,11 +101,14 @@ revert to. A commit per folder keeps the useful property: something broke after the merge, `git revert ` takes that folder back with its imports and its docs, in one shot. -`git add -A` and then `scripts/gate.sh` at the end of each folder — staged -first, so that the rollback also undoes files created during the move. Typecheck -alone is not enough here: the "Do not forget" list below is mostly made of -things that only break at runtime, and the test suite is what catches part of -them. Failed → `git restore --staged --worktree .`, record it, next folder. +`git add -- ` (pathspecs only — never +`git add -A` / `git add .`) and then `scripts/gate.sh` at the end of each +folder — staged first, so that the rollback also undoes files created during +the move. Typecheck alone is not enough here: the "Do not forget" list below +is mostly made of things that only break at runtime, and the test suite is +what catches part of them. Failed → `git restore --staged --worktree .`, +record it, next folder. If that restore is blocked by a hook, abort the +pipeline (see `SKILL.md`) — do not start the next folder. ## Do not forget diff --git a/scripts/coherence_test.sh b/scripts/coherence_test.sh index 31d7559..d3e81a1 100755 --- a/scripts/coherence_test.sh +++ b/scripts/coherence_test.sh @@ -3,9 +3,10 @@ # than one file at once. A doc drift that used to be caught by reading — a # rollback command typed slightly differently, an exit code the READMEs never # heard of, a script missing from the file tree — fails here instead. -# Read-only over the repository: it opens the repo's files and never writes, -# stages or runs anything inside it. Its self-tests need files to scan, so they -# build them in a mktemp -d of their own and delete it on the way out. +# Read-only over the repository: it opens the repo's files and never writes or +# stages inside it. Section 9 runs gate_test.sh only to read the NN/NN it prints +# (that suite uses its own stubs/tempdirs). Its self-tests need files to scan, +# so they build them in a mktemp -d of their own and delete it on the way out. # Usage: coherence_test.sh (exit 0 = every invariant held) set -u @@ -74,13 +75,27 @@ files_with() { repo_files "${2:-.}" | xargs -0 grep -l -F -- "$1" /dev/null 2>/dev/null | sort } -# tree_scripts — the *.sh names listed in the README's file tree -tree_scripts() { +# tree_block — the fenced file-tree body (between codebase-cleanup/ and +# the closing fence). Shared by the script-name and doc-name extractors so a +# tree edited in one place cannot be read two different ways. +tree_block() { awk ' /^codebase-cleanup\/$/ { inblock = 1; next } inblock && /^```/ { inblock = 0 } inblock { print } - ' "$1" | grep -oE '[A-Za-z0-9_.-]+\.sh' | sort -u + ' "$1" +} + +# tree_scripts — the *.sh names listed in the README's file tree +tree_scripts() { + tree_block "$1" | grep -oE '[A-Za-z0-9_.-]+\.sh' | sort -u +} + +# tree_docs — SKILL.md, LICENSE, README*.md and references/*.md names +# listed in the tree. Same shape as tree_scripts: the tree is the contract for +# what the skill ships, not only for the scripts. +tree_docs() { + tree_block "$1" | grep -oE '([A-Za-z0-9_.-]+\.md|LICENSE)' | sort -u } # bash_body — reads a shell script on stdin and prints its bash body with @@ -240,8 +255,12 @@ check "\"$GITMV\" appears exactly once in references/phase-3-structure.md" \ "$([[ ${n:-0} -eq 1 ]] && echo 0 || echo 1)" "count=${n:-0}" # 4. Whoever documents the gate as a step also documents staging. ------------- -# The gate reads the working tree, so a protocol that runs it without `git -# add -A` first checks something the commit will not contain. +# The gate reads the working tree, so a protocol that runs it without staging +# first checks something the commit will not contain. Staging is pathspec-only +# (`git add -- `): blind `git add -A` / `git add .` would swallow +# unrelated untracked files into the category commit. Prose that says "never +# git add -A" still contains that substring, so the invariant keys on the +# positive form `git add --`, not on the forbidden spellings. derived=$(protocol_files) # Floor: the three files that carry the protocol today have to be in the @@ -263,11 +282,24 @@ done old_ifs=$IFS IFS=$'\n' for f in $derived; do - if grep -q -F -- 'git add -A' "$f" 2>/dev/null; then - pass "gate step pairs with git add -A in $f" + # Require the pathspec form `git add -- <…>` (space after --). A bare + # `git add --` substring would also match `git add --all` (≡ `git add -A`). + # The test stays on the *positive* form, as the reasoning above demands: prose + # that forbids a spelling still contains it, so keying on `git add -- .` as a + # forbidden substring would fail a file whose only sin is documenting the ban. + # Instead, collect the pathspecs actually written and ask whether any of them + # names something narrower than the whole tree — `.` and `./` do not, they are + # `git add .` wearing pathspec syntax, while `./src`, `.env` and `..` do. + specs=$(grep -oE -- 'git add --[[:space:]]+[^[:space:]`"'"'"']+' "$f" 2>/dev/null \ + | sed -E 's/^git add --[[:space:]]+//') + if [[ -n $specs ]] && printf '%s\n' "$specs" | grep -qvxE '\.|\./'; then + pass "gate step pairs with pathspec staging in $f" + elif [[ -n $specs ]]; then + fail "gate step pairs with pathspec staging in $f" \ + "every \`git add --\` pathspec is \`.\` or \`./\`, which is \`git add .\` in disguise" else - fail "gate step pairs with git add -A in $f" \ - "runs the gate as a protocol step without staging first" + fail "gate step pairs with pathspec staging in $f" \ + "runs the gate as a protocol step without \`git add --\` pathspecs first" fi done IFS=$old_ifs @@ -290,6 +322,54 @@ for readme in README.md README.en.md; do fail "$name of the tree of $readme exists on disk" "listed but there is no scripts/$name" fi done + + # Same bidirectional contract for the docs the tree claims to ship: SKILL.md, + # LICENSE, and every references/*.md. A script-only check let a reference + # disappear from the tree (or land on disk unlisted) without a failure. + docs=$(tree_docs "$readme") + for required in SKILL.md LICENSE; do + if printf '%s\n' "$docs" | grep -qx -F -- "$required"; then + pass "$required listed in the tree of $readme" + else + fail "$required listed in the tree of $readme" "absent from the tree" + fi + if [[ -f $required ]]; then + pass "$required of the tree of $readme exists on disk" + else + fail "$required of the tree of $readme exists on disk" "listed but missing on disk" + fi + done + for path in references/*.md; do + name=${path##*/} + if printf '%s\n' "$docs" | grep -qx -F -- "$name"; then + pass "references/$name listed in the tree of $readme" + else + fail "references/$name listed in the tree of $readme" "on disk but absent from the tree" + fi + done + old_ifs=$IFS + IFS=$'\n' + for name in $docs; do + [[ $name == *.md ]] || continue + case $name in + SKILL.md|README.md|README.en.md) + if [[ -f $name ]]; then + pass "$name of the tree of $readme exists on disk" + else + fail "$name of the tree of $readme exists on disk" "listed but missing on disk" + fi + ;; + *) + if [[ -f references/$name ]]; then + pass "references/$name of the tree of $readme exists on disk" + else + fail "references/$name of the tree of $readme exists on disk" \ + "listed but there is no references/$name" + fi + ;; + esac + done + IFS=$old_ifs done # 6. Whoever shows a `git mv` also shows the mkdir -p it needs. --------------- @@ -338,10 +418,16 @@ done # read the wrong token in either case — a flag shifted the fields, and the # wrong token usually has no `/`, so the move was quietly exempted instead of # checked. Trailing comments are cut first (the repo's own example carries one), -# option words are skipped, and surrounding double quotes are stripped so a -# quoted path registers under the name the move actually uses. +# option words are skipped, and surrounding single or double quotes are stripped +# so a quoted path registers under the name the move actually uses. `&&` and `;` +# split a line into commands before either rule runs: a one-liner +# `mkdir -p X && git mv …` used to match only the mkdir rule (and then `next`), +# so the move was never checked and every non-flag token — including `&&` and +# the move's own paths — polluted `made`. gitmv_orphans() { - awk ' + local q + q=$(printf '\047') + awk -v q="$q" ' function args(line, i, n, out) { # the words of , comments and flags out sub(/[[:space:]]+#.*$/, "", line) n = split(line, out, "[[:space:]]+") # a string, not a /literal/: mawk @@ -349,21 +435,36 @@ gitmv_orphans() { for (i = 1; i <= n; i++) { if (out[i] == "" || out[i] ~ /^-/) continue gsub(/"/, "", out[i]) + gsub(q, "", out[i]) argv[++argc] = out[i] } return argc } + function split_cmds(line, segs, tmp) { + tmp = line + gsub(/&&/, "\034", tmp) + gsub(/;/, "\034", tmp) + return split(tmp, segs, "\034") + } + function process(line, n, i, d, dest) { + if (line ~ /^[[:space:]]*mkdir -p/) { + n = args(line) + for (i = 2; i <= n; i++) { # argv[1] is "mkdir" + d = argv[i]; gsub(/\/+$/, "", d); made[d] = 1 + } + } else if (line ~ /^[[:space:]]*git mv /) { + n = args(line) + dest = argv[n] # last word, never $4 + if (sub(/\/[^\/]*$/, "", dest) && !(dest in made)) + print FILENAME ":" FNR ": " $0 + } + } /^[[:space:]]*```/ { split("", made); next } /^[[:space:]]*$/ { split("", made); next } - /^[[:space:]]*mkdir -p/ { n = args($0) - for (i = 2; i <= n; i++) { # argv[1] is "mkdir" - d = argv[i]; gsub(/\/+$/, "", d); made[d] = 1 - } - next } - /^[[:space:]]*git mv / { n = args($0) - dest = argv[n] # last word, never $4 - if (sub(/\/[^\/]*$/, "", dest) && !(dest in made)) - print FILENAME ":" FNR ": " $0 } + { + n = split_cmds($0, segs) + for (i = 1; i <= n; i++) process(segs[i]) + } ' "$1" } @@ -376,24 +477,33 @@ gitmv_orphans() { # uncover it), `git mv g` the blank-line reset for the same reason, `git mv d` # the leading-whitespace tolerance. `git mv h i` renames in place and must stay # unflagged even though no mkdir names `i`. -# The last block pays for the argument reader: `git mv j` is only unflagged if -# the quotes came off the mkdir, and `git mv -f k` and the multi-source -# `git mv l m src/s/` are only flagged if the destination is read as the last -# word — a fixed `$4` reads `k` and `m`, neither of which has a `/`, so both -# uncovered moves would slip through as renames in place. +# The last block pays for the argument reader and the command splitter: +# `git mv j` is only unflagged if single quotes came off the mkdir; +# `mkdir && git mv p` is only unflagged if `&&` splits before the rules run +# (otherwise the mkdir rule `next`s and the move is never checked); +# `mkdir && git mv r` into a different folder must stay flagged — proof the +# move half of a one-liner is still subject to the destination check; +# `git mv -f k` and the multi-source `git mv l m src/s/` are only flagged if +# the destination is read as the last word — a fixed `$4` reads `k` and `m`, +# neither of which has a `/`, so both uncovered moves would slip through as +# renames in place. `git mv q` is a plain uncovered move in the same block. printf '%s\n' '```bash' 'mkdir -p src/x' 'git mv a src/x/a' 'git mv b src/x/b' \ 'git mv c src/y/c' '```' 'git mv e src/x/e' \ '```bash' 'mkdir -p src/y' 'git mv f src/y/f' '' 'git mv g src/y/g' '```' \ '' ' git mv d gone/d' \ '```bash' 'mkdir -p src/z' 'git mv h i' '```' \ - '```bash' 'mkdir -p "src/q"' 'git mv j src/q/j' 'git mv -f k src/r/k' \ - 'git mv l m src/s/' '```' \ + '```bash' "mkdir -p 'src/q'" 'git mv j src/q/j' \ + 'mkdir -p src/ok && git mv p src/ok/p' \ + 'mkdir -p src/other && git mv r src/missing/r' \ + 'git mv -f k src/r/k' \ + 'git mv l m src/s/' \ + 'git mv q src/bad/q' '```' \ > "$SELFTMP/moves.md" orphans=$(gitmv_orphans "$SELFTMP/moves.md") got=$(printf '%s' "$orphans" | grep -c .) check "git mv scanner flags the uncovered moves and only those" \ - "$([[ $got -eq 6 ]] && echo 0 || echo 1)" \ - "flagged $got line(s), expected 6: + "$([[ $got -eq 8 ]] && echo 0 || echo 1)" \ + "flagged $got line(s), expected 8: $orphans" # IFS is pinned to newline: the list comes from grep -l, and the default IFS @@ -528,7 +638,7 @@ done # report before knip starts, so a crash reads as "nothing to delete"; the # unwindowed churn command ranks by lifetime touch count, where a file rewritten # three years ago outranks whatever is hot now. -KNIP_CMD='npx knip --production --no-exit-code --reporter json > knip-report.json.tmp && mv knip-report.json.tmp knip-report.json' +KNIP_CMD='npx knip@6.32.0 --production --no-exit-code --reporter json > knip-report.json.tmp && mv knip-report.json.tmp knip-report.json' KNIP_FILES="SKILL.md references/knip-config.md" CHURN_CMD='git log --no-merges --since="6 months ago" --format= --name-only | sed '\''/^$/d'\'' | sort | uniq -c | sort -rn' @@ -566,26 +676,90 @@ check "no churn ranking variant in the repo" \ # contract was exercised. That number drifted once — the suite grew by eleven # cases and the line was bumped by six — and a stale count is worse than none, # because a reader who runs the command and gets a different total has to guess -# which of the two is lying. gate_test.sh calls its counting helpers exactly -# once per line, continuation lines included (they start with the argument, not -# with the helper), so the call sites are the total without running anything. -# One assumption the count makes and cannot check: ten of those call sites are -# inside gate_test.sh's `if command -v perl`, so the total counted here is the -# one the documented docker image produces — node:22-bookworm ships perl. A host -# without perl skips that block and prints ten fewer; the READMEs quote the -# figure from the command they publish, which is the docker one. -# The properties and invariants figures come from loops over derived lists and -# have no static equivalent; they stay on the honour system. -gate_cases=$(grep -cE '^[[:space:]]*(case_run|assert_log|assert_no_log|assert_reaped|elapsed_lt) ' \ - scripts/gate_test.sh) +# which of the two is lying. The total is whatever gate_test.sh prints at the +# end of a real run (`NN/NN cases passed`): call-site grep silently desyncs when +# cases move behind a loop or a conditional, and the perl-guarded block already +# proves the point — a host without perl runs ten fewer cases than a grep of +# the file would claim. Running the suite makes the READMEs answer to the same +# number a reader sees. The properties and invariants figures come from loops +# over derived lists and have no single printed total to pin; they stay on the +# honour system (Integrator syncs the README figures to the printed totals). +# Prefer the count from the parent `scripts/test.sh` run when present — that +# suite already exercised gate_test.sh under the same $BASH — so a full +# `scripts/test.sh` does not pay for a second full matrix pass. Alone, re-run +# under this process's interpreter (never bare `bash` from PATH: macOS CI pins +# /bin/bash 3.2 and Homebrew bash on PATH would defeat that premise). +gate_log="$SELFTMP/gate_test.out" +if [[ -n ${GATE_TEST_CASE_COUNT:-} ]]; then + gate_cases=$GATE_TEST_CASE_COUNT + : >"$gate_log" +else + "${BASH:-bash}" scripts/gate_test.sh >"$gate_log" 2>&1 || true + gate_cases=$(sed -n 's|^[0-9][0-9]*/\([0-9][0-9]*\) cases passed$|\1|p' "$gate_log" | tail -1) +fi +check "gate_test.sh printed a parsable NN/NN cases summary" \ + "$([[ -n $gate_cases ]] && echo 0 || echo 1)" \ + "GATE_TEST_CASE_COUNT=${GATE_TEST_CASE_COUNT:-unset}; last lines of gate_test.sh: +$(tail -n 5 "$gate_log" 2>/dev/null)" for pair in 'README.md|casos' 'README.en.md|cases'; do f=${pair%%|*} word=${pair#*|} check "$f quotes gate_test.sh's real case count" \ - "$(grep -q -F -- "$gate_cases/$gate_cases $word" "$f" 2>/dev/null && echo 0 || echo 1)" \ - "gate_test.sh has $gate_cases cases; the validated-run line of $f does not say so" + "$([[ -n $gate_cases ]] && grep -q -F -- "$gate_cases/$gate_cases $word" "$f" 2>/dev/null && echo 0 || echo 1)" \ + "gate_test.sh ran $gate_cases cases; the validated-run line of $f does not say so" done +# 10. Normative PT↔EN markers, and the SKILL.md frontmatter budget. ---------- +# Section 7 already locks the level-table shape and per-row conditions. What +# still drifted without a failure was the shared vocabulary around empty-suite +# caps and the exit contract outside the file-tree caption — language-neutral +# tokens that must appear in both READMEs — plus the description frontmatter, +# which is the only line that can eject the skill from auto-dispatch when it +# grows past the host's limit. Fail closed above 1000 characters so there is +# margin under the usual 1024 cap before a new trigger phrase breaks discovery. +for marker in \ + 'exit 0/1/2/3/4' \ + 'Exit 124' \ + 'tests/*.rs' \ + '#[test]' \ + 'src/test' \ + 'test:unit' \ + 'test:e2e' \ + 'checks=typecheck,test' \ + 'Go' \ + '.NET' \ + 'Rust' \ + 'Maven' \ + 'Gradle' \ + 'pytest' \ + 'Exit 137' \ + 'passWithNoTests' \ + '_spec.rb' +do + for f in README.md README.en.md; do + check "$f carries normative marker [$marker]" \ + "$(grep -q -F -- "$marker" "$f" 2>/dev/null && echo 0 || echo 1)" \ + "missing [$marker]" + done +done + +# Bash ${#var} counts characters under a UTF-8 locale; awk's length() counts +# bytes and would burn the margin on the em dash and Portuguese accents that +# already live in the description (keep headroom under the 1000-char cap). +desc=$(awk ' + /^---[[:space:]]*$/ { fm++; next } + fm == 1 && /^description:[[:space:]]*/ { + sub(/^description:[[:space:]]*/, "") + print + exit + } + fm >= 2 { exit } +' SKILL.md) +desc_len=${#desc} +check "SKILL.md description stays at or under 1000 characters" \ + "$([[ -n $desc && $desc_len -le 1000 ]] && echo 0 || echo 1)" \ + "description is ${desc_len} characters (limit 1000)" + echo "----" echo "$((total-failures))/$total invariants held" [[ $failures -eq 0 ]] diff --git a/scripts/gate.sh b/scripts/gate.sh index 01ade63..14337ab 100755 --- a/scripts/gate.sh +++ b/scripts/gate.sh @@ -29,10 +29,13 @@ uncounted=0 # --- Watchdog --------------------------------------------------------- # A hanging check (a test waiting on a port, a REPL, a prompt) would freeze the -# gate forever. Resolve one backend up front; all three exit 124 on timeout, -# copying GNU timeout so run() only has to know that number. Contract: 124 is -# reserved for the watchdog, exactly as in GNU timeout — a check that -# legitimately exits 124 under an active watchdog is read as TIMEOUT. +# gate forever. Resolve one backend up front; all three raise 124 when the +# alarm itself fires, copying GNU timeout. That is not the only code a timeout +# can produce, which is why the test lives in wd_timed_out() instead of a bare +# comparison: with -k, a check that ignores TERM is finished off with KILL and +# the shell reports 137. Contract: 124 is reserved for the watchdog, exactly as +# in GNU timeout — a check that legitimately exits 124 under an active watchdog +# is read as TIMEOUT, and so is 137 while -k is in play. # Limit: what still escapes the kill is a double fork already reparented to # init/launchd when the alarm fires, and anything created between the snapshot # and the kill. The perl backend also sweeps descendants by parent pid (needs @@ -141,6 +144,21 @@ guard() { esac } +# wd_timed_out — true when is the watchdog reporting a timeout. +# 124 is GNU timeout's own code, and the perl backend copies it. It is not the +# only one: with -k, a check that ignores TERM is finished off with KILL and +# the shell reports 128+9 = 137, which fell through to the generic non-zero +# branch and was announced as 'RED at ' — a hung check reported as a +# broken one, the exact distinction exit 4 exists to preserve. 137 only counts +# when -k is actually in play; a process killed by something else (an OOM +# reaper, say) reaching this arm is still read as inconclusive, never as GREEN. +wd_timed_out() { + [[ -n $WATCHDOG ]] || return 1 + [[ $1 -eq 124 ]] && return 0 + [[ -n $WD_KILL_AFTER && $1 -eq 137 ]] && return 0 + return 1 +} + # run # is typecheck|test|both, optionally extended as ::: # that is the runner's "I collected nothing" code, and it is a YELLOW cap @@ -159,7 +177,7 @@ run() { rc=$? # The watchdog keeps absolute priority: a check killed at the timeout is # inconclusive, never "no tests collected", whatever code it happens to share. - if [[ -n $WATCHDOG && $rc -eq 124 ]]; then + if wd_timed_out $rc; then echo "[gate] TIMEOUT after ${GATE_TIMEOUT}s at '$*'" >&2 exit 4 fi @@ -268,9 +286,147 @@ if [[ -f package.json ]]; then # because run() classifies by kind and an unknown word there would run the # check and count nothing. The alias stays visible in the human # '[gate] ' line; checks= keeps its canonical vocabulary. + # The test path captures the runner's output: vitest/jest exit 1 with + # "No test files found" on an empty suite, which must be a YELLOW cap + # (uncounted), not RED. Typecheck still goes through run() unchanged. js_script() { - if [[ $PM == yarn ]]; then run "$1" yarn "$2" - else run "$1" "$PM" run "$2"; fi + local kind=$1 name=$2 out rc ev outfile + if [[ $kind != test ]]; then + if [[ $PM == yarn ]]; then run "$kind" yarn "$name" + else run "$kind" "$PM" run "$name"; fi + return + fi + if [[ $PM == yarn ]]; then set -- yarn "$name" + else set -- "$PM" run "$name"; fi + echo "[gate] $*" + # A file, not `out=$(...)`. Command substitution reads a pipe and only + # returns once every writer closes it — a test script that leaves a + # detached grandchild holding the inherited stdout keeps that pipe open + # long after the runner exited, so the gate blocked for the grandchild's + # whole lifetime with GATE_TIMEOUT already elapsed and the watchdog + # powerless (it kills the process it launched, not a reader waiting on a + # pipe). Redirecting to a file gives the watchdog its timing back. + # Not exit 2: that code means "bad path, nothing was checked", and by here + # typecheck may already have run. A tmpdir we cannot write to is the same + # shape as a missing toolchain — we could not measure, so the gate degrades + # to PARTIAL and says so, instead of claiming the suite failed. + outfile=$(mktemp) || { + echo "[gate] cannot create a temp file for the test output — manual gate" >&2 + incomplete=1 + return 0 + } + guard "$@" > "$outfile" 2>&1 + rc=$? + out=$(cat "$outfile") + rm -f "$outfile" + printf '%s\n' "$out" + if wd_timed_out $rc; then + echo "[gate] TIMEOUT after ${GATE_TIMEOUT}s at '$*'" >&2 + exit 4 + fi + # The package manager's own epilogue is not runner output, and both the + # exit-0 and the exit-1 path have to judge without it. pnpm prints + # 'ELIFECYCLE Command failed', yarn 'error Command failed with exit code + # 1.', bun 'error: script "test" exited with code 1' — non-empty lines + # carrying "failed" after the runner's own message. + # pnpm changed this line between majors: v9 and older print + # ' ELIFECYCLE Command failed with exit code 1.', v10/v11 print + # '[ELIFECYCLE] Test failed. See above for more details.' (and + # '[ELIFECYCLE] Command failed…' for a script not named test). Matching + # only the older spelling left the fix inert on every current pnpm, so + # both the brackets and both nouns are optional here. + ev=$(printf '%s\n' "$out" | grep -vE \ + '^[[:space:]]*(error Command failed with exit code|.*\[?ELIFECYCLE\]?[[:space:]]+(Command|Test) failed|npm ERR!|error: script .* exited with code|info Visit https://yarnpkg)') + if [[ $rc -eq 0 ]]; then + # Exit 0 is not proof a suite ran. `--passWithNoTests` (jest, vitest) + # turns an empty suite into a success on purpose, which is the right + # call for CI and the wrong one here: counting it would announce GREEN + # — the level that unlocks dead-export deletion — over a repo where + # zero tests executed. The empty-suite detection used to live only + # inside the non-zero branch, so this path was never even considered. + if printf '%s\n' "$ev" | grep -qiE \ + '^[[:space:]]*(No test files found|No tests found|did not find any tests)\b'; then + uncounted_suite js "runner reported no tests and exited 0 (--passWithNoTests)" + return 0 + fi + # node:test — the runtime's own runner — never prints any of those + # phrases. On an empty run it exits 0 and reports a count instead: + # 'ℹ tests 0' in the default reporter, '# tests 0' under TAP. Anchoring + # the end of the line keeps 'tests 1' and 'tests 10' out of it. The + # prefix is matched as "start of line, or one non-alphanumeric" rather + # than "any run of non-alphanumerics": the default reporter's marker is + # multibyte, and a negated class does not step over it under a UTF-8 + # locale (it does under LC_ALL=C), so the greedy form silently missed + # the very reporter that is on by default. Without this a repo on the + # built-in runner with no test file reached GREEN. + if printf '%s\n' "$ev" | grep -qE \ + '(^|[^[:alnum:]])tests[[:space:]]+0[[:space:]]*$'; then + uncounted_suite js "runner reported 0 tests and exited 0 (node:test)" + return 0 + fi + ran_test=1 + return 0 + fi + if [[ $rc -ne 0 ]]; then + # Empty-suite cap: only a runner empty-message *line* (vitest/jest + # shape), never a mid-line substring in a real failure. A failing suite + # whose log happens to contain "No tests found" in fixture names or + # assertion text must stay RED. The runner's empty-suite exit is 1; + # any other code, or any non-empty line after the empty-suite message + # that is not an "exiting with code N" trailer, means a chained + # command failed and must stay RED — printing the empty line alone + # is not proof the process exited for an empty suite. Leaving the + # manager's epilogue in (it is stripped into $ev above) made every + # manager except npm read as a chained failure and sink a legitimately + # empty suite to RED. + # What the empty-suite line itself says about the exit is stronger + # evidence than anything that follows it. Both runners the docs name + # print the code inline — vitest 'No test files found, exiting with + # code 1', jest the same — and then keep going: vitest lists its + # include/exclude globs, jest prints 'N files checked' and every + # testMatch pattern it tried. That tail is the runner explaining an + # empty suite, not a second command failing, so reading it as a chained + # failure sent every real vitest and jest repo without a test file to + # RED while the docs promised YELLOW. When the line names the code and + # it is the code the process exited with, the exit is accounted for and + # the tail is diagnosis. + inline_rc=$(printf '%s\n' "$ev" | awk ' + tolower($0) ~ /^[[:space:]]*(no test files found|no tests found|did not find any tests)/ { + if (match(tolower($0), /exiting with code [0-9]+/)) { + s = substr(tolower($0), RSTART, RLENGTH) + sub(/exiting with code /, "", s) + print s + exit + } + } + ') + if [[ $rc -eq 1 ]] \ + && printf '%s\n' "$ev" | grep -qiE \ + '^[[:space:]]*(No test files found|No tests found|did not find any tests)\b' \ + && ! printf '%s\n' "$ev" | grep -qiE \ + '(^|[[:space:]])(FAIL|Failed|AssertionError|Expected )|(^|[[:space:]])(●|✕|×)[[:space:]]|[1-9][0-9]* (failed|failing)\b|(^|[[:space:]])(error|ERR!)([[:space:]:]|$)' \ + && { [[ ${inline_rc:-x} == "$rc" ]] \ + || ! printf '%s\n' "$ev" | awk ' + # tolower(), not IGNORECASE: that variable is a gawk extension + # and is silently ignored by BSD awk (macOS) and mawk (the + # usual Debian/Ubuntu default) — both CI legs. Under it the + # chained-failure guard only ever matched the exact casing the + # fixtures happen to use, so a runner spelling its empty-suite + # line differently slipped a real failure through as YELLOW. + tolower($0) ~ /^[[:space:]]*(no test files found|no tests found|did not find any tests)([[:space:],]|$)/ { + seen=1; next + } + seen && NF && tolower($0) !~ /^[[:space:]]*exiting with code [0-9]+[[:space:]]*$/ { + found=1; exit + } + END { exit found ? 0 : 1 } + '; }; then + uncounted_suite js "no test files found (exit $rc)" + return 0 + fi + echo "[gate] RED at '$*'" >&2 + exit 1 + fi } # js_typecheck_script — the first of the accepted spellings the project @@ -279,18 +435,28 @@ if [[ -f package.json ]]; then # spelling made a fully covered repo look uncheckable. Exactly one script # runs: the first name the project defines wins, the rest are ignored, so # nothing runs or counts twice. - # 'tsc' is deliberately NOT an alias: as a script name it usually means an - # emitting compile ("tsc": "tsc -p ."), which would drop .js/.d.ts/.tsbuildinfo - # beside the sources of the tree being judged, right before phase 1.3 stages - # everything with 'git add -A' — and the documented rollback - # ('git restore --staged --worktree .') cannot remove untracked files. + # 'tsc' as a bare name usually means an emitting compile ("tsc": "tsc -p ."), + # which would drop .js/.d.ts/.tsbuildinfo beside the sources of the tree + # being judged, right before phase 1.3 stages pathspecs of that step + # — and the documented rollback ('git restore --staged --worktree .') cannot + # remove untracked files. It is therefore NOT an alias by name alone. The + # one exception is when the script value itself carries --noEmit as a real + # shell word (after stripping # / // comments): that is a check, not a + # build, and counts. A trailing comment like `# use --noEmit in CI` must + # NOT promote an emitting compile. # Echoes the script name, or nothing when the manifest declares none. js_typecheck_script() { node -e 'var s = require("./package.json").scripts || {}; var names = ["typecheck", "type-check", "check-types"]; +var found = ""; for (var i = 0; i < names.length; i++) { - if (s[names[i]]) { console.log(names[i]); break; } -}' 2>/dev/null + if (s[names[i]]) { found = names[i]; break; } +} +if (!found && s.tsc) { + var body = String(s.tsc).replace(/#.*/g, "").replace(/\/\/.*/g, ""); + if (/(?:^|[\s"'"'"'=])--noEmit(?:[\s"'"'"']|$)/.test(body)) found = "tsc"; +} +if (found) console.log(found);' 2>/dev/null } # js_test_script — decides which script stands for the *whole* suite. @@ -316,12 +482,17 @@ for (var i = 0; i < names.length; i++) { # this rule exists to deny. Slices are still listed in the report, so the # cap is explained instead of silent. Same reasoning as the 'tsc' exclusion # above: a name that means "not a check" is not promoted to one. - # Echoes 'run:'; 'partial:' when two or more slices split the - # suite; 'watch:' when no slice can be run at all; nothing at all - # when the manifest declares no test script. + # The exact npm-init placeholder ('echo "Error: no test specified" && exit 1') + # is recognised by value and reported as 'placeholder:test': running it would + # only ever produce RED for a project that has never defined a suite. + # Echoes 'run:'; 'placeholder:test'; 'partial:' when two or + # more slices split the suite; 'watch:' when no slice can be run at + # all; nothing at all when the manifest declares no test script. js_test_script() { node -e 'var s = require("./package.json").scripts || {}; -if (s.test) { console.log("run:test"); } +var npmPlaceholder = "echo \u0022Error: no test specified\u0022 && exit 1"; +if (s.test === npmPlaceholder) { console.log("placeholder:test"); } +else if (s.test) { console.log("run:test"); } else { var p = Object.keys(s).filter(function (k) { return k.indexOf("test:") === 0; }); var slices = p.filter(function (k) { return !/(^|:)(watch|debug)(:|$)/.test(k); }); @@ -350,6 +521,9 @@ else { case $js_test in run:*) js_script test "${js_test#run:}" ;; + placeholder:*) + uncounted_suite js "npm init placeholder test script" \ + "replace scripts.test with a real suite" ;; partial:*) uncounted_suite js "no 'test' script; test:* slices found (${js_test#partial:})" \ "none of them stands for the whole suite; run them by hand" ;; @@ -460,8 +634,36 @@ fi if [[ -f Gemfile ]]; then if command -v bundle >/dev/null; then [[ -f sorbet/config ]] && run typecheck bundle exec srb tc - if [[ -d spec ]]; then run test bundle exec rspec - elif [[ -f Rakefile && -d test ]]; then run test bundle exec rake test + # A directory is not a suite. 'bundle exec rspec' on a spec/ holding only a + # spec_helper.rb exits 0 reporting "0 examples", and 'rake test' on a test/ + # with no *_test.rb does the same — the exact trap 'go test' and 'cargo + # test' already have guards for. Judging by the directory alone let a Ruby + # repo with zero real tests reach GREEN, which unlocks dead-export deletion. + # Count a layout only when a real test file backs it. + # Both minitest spellings count: `x_test.rb` and `test_x.rb`. The second is + # not exotic — `test/test*.rb` is the DEFAULT pattern of Rake::TestTask, so + # any `Rake::TestTask.new(:test)` without an explicit pattern collects only + # that shape, and `bundle gem` scaffolds it. Matching just the suffix form + # made a real minitest suite invisible: it dropped the "both runners" cap on + # a repo that has rspec *and* rake (announcing GREEN with the rake half + # never measured) and demoted a rake-only repo to a YELLOW whose message was + # simply false. `test_helper.rb` fits the prefix pattern and is support + # code, not a suite, so it is excluded by name. + rb_specs=""; rb_tests="" + [[ -d spec ]] && rb_specs=$(find ./spec -name '*_spec.rb' -print -quit 2>/dev/null) + [[ -f Rakefile && -d test ]] && \ + rb_tests=$(find ./test \( -name '*_test.rb' -o -name 'test_*.rb' \) \ + ! -name 'test_helper.rb' -print -quit 2>/dev/null) + # rspec and rake are two halves of a suite when both layouts exist — running + # only one would announce GREEN with the other half never measured. Cap and + # name both, same shape as JS test:* slices. + if [[ -n $rb_specs && -n $rb_tests ]]; then + uncounted_suite ruby "both rspec (spec/) and rake test (Rakefile+test/) present" \ + "neither stands for the whole suite alone; run them by hand" + elif [[ -n $rb_specs ]]; then run test bundle exec rspec + elif [[ -n $rb_tests ]]; then run test bundle exec rake test + elif [[ -d spec || ( -f Rakefile && -d test ) ]]; then + uncounted_suite ruby "spec/ or test/ present but holds no *_spec.rb / *_test.rb" # Same evidence rule as Python: a Gemfile alone (fastlane, Jekyll, danger) # in a repo of another stack is not a Ruby stack without a suite, it is not # a Ruby stack. Capping on it would pin that repo below GREEN forever. diff --git a/scripts/gate_test.sh b/scripts/gate_test.sh index 2ad3893..e1fc039 100755 --- a/scripts/gate_test.sh +++ b/scripts/gate_test.sh @@ -134,8 +134,11 @@ case_run() { # case_run local name=$1 expected=$2 dir=$3 path=$4; shift 4 local out rc p ok=1 start=$SECONDS # shellcheck disable=SC2086 # GATE_ENV is split on purpose - if [[ $path == - ]]; then out=$(env $GATE_ENV bash "$GATE" "$dir" 2>&1); rc=$? - else out=$(env PATH="$path" $GATE_ENV bash "$GATE" "$dir" 2>&1); rc=$?; fi + # Prefer $BASH (the suite's own interpreter) over a bare 'bash' from PATH: + # on the macOS CI job PATH can resolve to Homebrew bash, which defeats the + # whole reason that job runs under /bin/bash 3.2. + if [[ $path == - ]]; then out=$(env $GATE_ENV "${BASH:-bash}" "$GATE" "$dir" 2>&1); rc=$? + else out=$(env PATH="$path" $GATE_ENV "${BASH:-bash}" "$GATE" "$dir" 2>&1); rc=$?; fi LAST_ELAPSED=$((SECONDS-start)) GATE_ENV="" [[ $rc -eq $expected ]] || ok=0 @@ -155,6 +158,17 @@ case_run() { # case_run fi } +# skipped — a case the environment cannot run, counted anyway. +# The total is not cosmetic: coherence_test.sh section 9 checks it against the +# number the READMEs publish, so a case that silently vanishes on one runner +# turns the docs into a lie there. The macOS runner ships no GNU timeout, so +# the -k cases below only ever execute on Linux; counting the skip keeps one +# number true on both, and the reason is printed so nobody reads it as a pass. +skipped() { + total=$((total+1)) + echo "ok: $1 (skipped: $2)" +} + elapsed_lt() { # elapsed_lt — asserts on the last case_run local name=$1 limit=$2 total=$((total+1)) @@ -206,6 +220,136 @@ cat > "$TMP/js-alias-tsc/package.json" <<'EOF' {"name":"f","scripts":{"tsc":"node -e \"console.log('TSC_RAN')\"","test":"node -e 0"}} EOF +# Exception: the value itself is a check with --noEmit as a real shell word. +# The script name is still 'tsc', but the body is safe to run and must count +# as typecheck. A real tsc binary is not required — node exits 0 and ignores +# the extra argv after `--`. +mkdir -p "$TMP/js-tsc-noemit" +cat > "$TMP/js-tsc-noemit/package.json" <<'EOF' +{"name":"f","scripts":{"tsc":"node -e 0 -- --noEmit","test":"node -e 0"}} +EOF + +# Emitting form stays rejected even when the command looks like a real tsc. +mkdir -p "$TMP/js-tsc-emit" +cat > "$TMP/js-tsc-emit/package.json" <<'EOF' +{"name":"f","scripts":{"tsc":"tsc -p .","test":"node -e 0"}} +EOF + +# A comment mentioning --noEmit must NOT promote an emitting compile. +mkdir -p "$TMP/js-tsc-comment-noemit" +cat > "$TMP/js-tsc-comment-noemit/package.json" <<'EOF' +{"name":"f","scripts":{"tsc":"tsc -p . # use --noEmit in CI","test":"node -e 0"}} +EOF + +# npm init's placeholder test script exits 1 on purpose — that is YELLOW with a +# named cap, not RED (a broken suite). The value is the literal npm writes. +mkdir -p "$TMP/js-npm-placeholder" +cat > "$TMP/js-npm-placeholder/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"echo \"Error: no test specified\" && exit 1"}} +EOF + +# Runner reports an empty suite (vitest/jest shape) and exits 1: YELLOW cap, +# not RED. Covers plain 'test' and a lone 'test:*' alias. +mkdir -p "$TMP/js-empty-suite" +cat > "$TMP/js-empty-suite/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.error('No test files found'); process.exit(1)\""}} +EOF + +mkdir -p "$TMP/js-empty-suite-slice" +cat > "$TMP/js-empty-suite-slice/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test:unit":"node -e \"console.error('No test files found'); process.exit(1)\""}} +EOF + +# Failing suite whose log only mentions the empty-suite phrase mid-line must +# stay RED — never flip to YELLOW/uncounted. +mkdir -p "$TMP/js-empty-suite-incidental" +cat > "$TMP/js-empty-suite-incidental/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.error('FAIL tests/foo.spec.js'); console.error('AssertionError: expected true'); console.error('hint: No tests found was unexpected'); process.exit(1)\""}} +EOF + +# Chained script: empty-suite line first, then a later failure without the +# assertion blacklist tokens. Must stay RED (exit 2 ≠ runner empty-suite). +mkdir -p "$TMP/js-empty-suite-chained" +cat > "$TMP/js-empty-suite-chained/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.error('No test files found'); console.error('configuration crashed'); process.exit(2)\""}} +EOF + +# Same shape with exit 1: the post-empty line still proves a chained failure. +mkdir -p "$TMP/js-empty-suite-chained-exit1" +cat > "$TMP/js-empty-suite-chained-exit1/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.error('No test files found'); console.error('configuration crashed'); process.exit(1)\""}} +EOF + +# The two runners the docs name, byte for byte. Both print the exit code inside +# the empty-suite line and then keep explaining themselves — vitest lists its +# include/exclude globs, jest prints how many files it checked and every +# pattern it tried. Read as a chained failure, that tail sent every real vitest +# and jest repo without a test file to RED while the docs promised YELLOW. The +# older fixtures above all emit a single line, which is why the guard could be +# wrong here and green there. +mkdir -p "$TMP/js-empty-vitest-real" +cat > "$TMP/js-empty-vitest-real/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.log(' RUN v3.2.7 ' + process.cwd()); console.log('No test files found, exiting with code 1'); console.log(''); console.log('include: **/*.{test,spec}.?(c|m)[jt]s?(x)'); console.log('exclude: **/node_modules/**, **/dist/**'); process.exit(1)\""}} +EOF + +mkdir -p "$TMP/js-empty-jest-real" +cat > "$TMP/js-empty-jest-real/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.log('No tests found, exiting with code 1'); console.log('Run with --passWithNoTests to exit with code 0'); console.log('In ' + process.cwd()); console.log(' 2 files checked.'); console.log(' testMatch: **/__tests__/**/*.[jt]s?(x) - 0 matches'); console.log(' testPathIgnorePatterns: /node_modules/ - 2 matches'); process.exit(1)\""}} +EOF + +# The inline code accounts for the process's exit, not for whatever else ran. +# A build that failed after the empty suite prints an error and still has to +# sink the run to RED — otherwise the tail-is-diagnosis rule would be a way of +# laundering any chained failure that exits 1. +mkdir -p "$TMP/js-empty-inline-then-error" +cat > "$TMP/js-empty-inline-then-error/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.log('No test files found, exiting with code 1'); console.log('error TS2304: Cannot find name foo.'); process.exit(1)\""}} +EOF + +# --passWithNoTests: the runner is told to make an empty suite a success, so it +# exits 0 having run nothing. The empty-suite detection used to sit inside the +# non-zero branch only, so this reached GREEN — the level that unlocks +# dead-export deletion — over a repo where no test ever ran. +mkdir -p "$TMP/js-pwnt" +cat > "$TMP/js-pwnt/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.log('No tests found, exiting with code 0')\""}} +EOF + +# node:test on an empty run: exit 0 and a count, none of the phrases the other +# runners print. Both reporters are covered — the default one's marker is +# multibyte, which a greedy negated class skips under a UTF-8 locale. +mkdir -p "$TMP/js-node-test-empty" +cat > "$TMP/js-node-test-empty/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.log('\\u2139 tests 0'); console.log('\\u2139 pass 0')\""}} +EOF + +mkdir -p "$TMP/js-node-test-tap" +cat > "$TMP/js-node-test-tap/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.log('TAP version 13'); console.log('1..0'); console.log('# tests 0')\""}} +EOF + +# Counts that must NOT cap: a real node:test run, and a two-digit total. +mkdir -p "$TMP/js-node-test-real" +cat > "$TMP/js-node-test-real/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.log('\\u2139 tests 10'); console.log('\\u2139 pass 10')\""}} +EOF + +# The counterpart that must stay GREEN: a real suite reporting real passes. +# Without this, capping on exit 0 could silently swallow every healthy repo. +mkdir -p "$TMP/js-real-pass" +cat > "$TMP/js-real-pass/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.log('Test Files 3 passed (3)'); console.log('Tests 12 passed (12)')\""}} +EOF + +# Same chained failure, lowercase runner line. The guard used to lean on awk's +# IGNORECASE — a gawk extension that BSD awk and mawk ignore, i.e. both CI legs +# — so only the exact casing of the fixtures above was ever caught and this +# shape slipped through as YELLOW. +mkdir -p "$TMP/js-empty-suite-chained-lower" +cat > "$TMP/js-empty-suite-chained-lower/package.json" <<'EOF' +{"name":"f","scripts":{"typecheck":"node -e 0","test":"node -e \"console.error('no test files found'); console.error('configuration crashed'); process.exit(1)\""}} +EOF + mkdir -p "$TMP/js-alias-check-types" cat > "$TMP/js-alias-check-types/package.json" <<'EOF' {"name":"f","scripts":{"check-types":"node -e 0","test":"node -e 0"}} @@ -325,6 +469,28 @@ mkdir -p "$TMP/js-yarn-alias" cp "$TMP/js-alias/package.json" "$TMP/js-yarn-alias/package.json" touch "$TMP/js-yarn-alias/yarn.lock" +# Lockfile picks pnpm / bun the same way yarn.lock picks yarn. +mkdir -p "$TMP/js-pnpm" +cp "$TMP/js-green/package.json" "$TMP/js-pnpm/package.json" +touch "$TMP/js-pnpm/pnpm-lock.yaml" + +mkdir -p "$TMP/js-bun" +cp "$TMP/js-green/package.json" "$TMP/js-bun/package.json" +touch "$TMP/js-bun/bun.lock" + +# The empty-suite cap has to survive the package manager's own epilogue. Every +# manager but npm prints a line after the runner's message — pnpm 'ELIFECYCLE +# Command failed', yarn 'error Command failed with exit code 1.', bun 'error: +# script "test" exited with code 1' — all of them non-empty and carrying +# "failed", which read as a real failure and sank an empty suite to RED. One +# fixture per manager, distinguished only by the lockfile. +for pm in pnpm yarn bun; do + mkdir -p "$TMP/js-empty-$pm" + cp "$TMP/js-empty-suite/package.json" "$TMP/js-empty-$pm/package.json" +done +touch "$TMP/js-empty-pnpm/pnpm-lock.yaml" "$TMP/js-empty-yarn/yarn.lock" \ + "$TMP/js-empty-bun/bun.lock" + mkdir -p "$TMP/polyglot/tests" touch "$TMP/polyglot/pyproject.toml" "$TMP/polyglot/tests/test_x.py" printf 'module f\n\ngo 1.21\n' > "$TMP/polyglot/go.mod" @@ -375,6 +541,15 @@ touch "$TMP/jvm-no-tests/pom.xml" mkdir -p "$TMP/jvm-module-tests/mod-a/src/test/java" "$TMP/jvm-module-tests/mod-b/src/main/java" touch "$TMP/jvm-module-tests/pom.xml" +# Wrapper scripts win over a missing system mvn/gradle. +mkdir -p "$TMP/jvm-mvnw/src/test/java" +touch "$TMP/jvm-mvnw/pom.xml" +stub "$TMP/jvm-mvnw" mvnw 0 + +mkdir -p "$TMP/jvm-gradlew/src/test/java" +touch "$TMP/jvm-gradlew/build.gradle" +stub "$TMP/jvm-gradlew" gradlew 0 + # Polyglot: JS supplies typecheck+test, the .NET half has no test project in any # of its projects. Announcing GREEN off the JS suite would unlock dead-export # deletion over C# that nothing tested — the same hole js-poly-uncounted covers @@ -411,6 +586,65 @@ cp "$TMP/py-reqs-no-source/package.json" "$TMP/rb-source-no-spec/package.json" printf "source 'https://rubygems.org'\n" > "$TMP/rb-source-no-spec/Gemfile" printf 'def f; end\n' > "$TMP/rb-source-no-spec/tool.rb" +# Ruby with sorbet + rspec: both typecheck and test must run (the GREEN path +# that used to have zero coverage). +mkdir -p "$TMP/rb-sorbet-rspec/spec" "$TMP/rb-sorbet-rspec/sorbet" +printf "source 'https://rubygems.org'\n" > "$TMP/rb-sorbet-rspec/Gemfile" +touch "$TMP/rb-sorbet-rspec/sorbet/config" +printf 'RSpec.describe "x" do; end\n' > "$TMP/rb-sorbet-rspec/spec/x_spec.rb" + +# Ruby with rake test only (no spec/). +mkdir -p "$TMP/rb-rake/test" +printf "source 'https://rubygems.org'\n" > "$TMP/rb-rake/Gemfile" +touch "$TMP/rb-rake/Rakefile" +printf 'class XTest; end\n' > "$TMP/rb-rake/test/x_test.rb" + +# Both runners present: neither alone is the whole suite → YELLOW, run neither. +# sorbet is present so typecheck still runs and the cap is visible as YELLOW +# (without it the gate would exit 3 with nothing runnable). +mkdir -p "$TMP/rb-both-runners/spec" "$TMP/rb-both-runners/test" "$TMP/rb-both-runners/sorbet" +printf "source 'https://rubygems.org'\n" > "$TMP/rb-both-runners/Gemfile" +touch "$TMP/rb-both-runners/Rakefile" "$TMP/rb-both-runners/sorbet/config" +printf 'RSpec.describe "x" do; end\n' > "$TMP/rb-both-runners/spec/x_spec.rb" +printf 'class XTest; end\n' > "$TMP/rb-both-runners/test/x_test.rb" + +# A spec/ holding no *_spec.rb is not a suite: rspec exits 0 on "0 examples", +# so judging by the directory alone announced GREEN for a repo with zero tests +# — the same trap go/cargo already guard against. Same for a rake test/ with +# no *_test.rb. sorbet is present so typecheck still runs and the cap shows as +# YELLOW instead of exit 3 with nothing runnable. +mkdir -p "$TMP/rb-spec-empty/spec" "$TMP/rb-spec-empty/sorbet" +printf "source 'https://rubygems.org'\n" > "$TMP/rb-spec-empty/Gemfile" +touch "$TMP/rb-spec-empty/sorbet/config" +printf '# support file, not a spec\n' > "$TMP/rb-spec-empty/spec/spec_helper.rb" + +mkdir -p "$TMP/rb-test-empty/test" "$TMP/rb-test-empty/sorbet" +printf "source 'https://rubygems.org'\n" > "$TMP/rb-test-empty/Gemfile" +touch "$TMP/rb-test-empty/Rakefile" "$TMP/rb-test-empty/sorbet/config" +printf '# support file, not a test\n' > "$TMP/rb-test-empty/test/helper.rb" + +# `test/test*.rb` is the DEFAULT pattern of Rake::TestTask, so this naming is +# the common one, not the exotic one. Recognising only the `*_test.rb` suffix +# dropped the both-runners cap here and announced GREEN with the rake half +# never measured. +mkdir -p "$TMP/rb-both-prefix/spec" "$TMP/rb-both-prefix/test" "$TMP/rb-both-prefix/sorbet" +printf "source 'https://rubygems.org'\n" > "$TMP/rb-both-prefix/Gemfile" +touch "$TMP/rb-both-prefix/Rakefile" "$TMP/rb-both-prefix/sorbet/config" +printf 'RSpec.describe "x" do; end\n' > "$TMP/rb-both-prefix/spec/x_spec.rb" +printf 'class TestX; end\n' > "$TMP/rb-both-prefix/test/test_x.rb" + +# Rake-only repo using the same default naming: a real suite, so it runs. +mkdir -p "$TMP/rb-rake-prefix/test" +printf "source 'https://rubygems.org'\n" > "$TMP/rb-rake-prefix/Gemfile" +touch "$TMP/rb-rake-prefix/Rakefile" +printf 'class TestX; end\n' > "$TMP/rb-rake-prefix/test/test_x.rb" + +# test_helper.rb matches the prefix pattern but is support code, not a suite. +mkdir -p "$TMP/rb-helper-only/test" "$TMP/rb-helper-only/sorbet" +printf "source 'https://rubygems.org'\n" > "$TMP/rb-helper-only/Gemfile" +touch "$TMP/rb-helper-only/Rakefile" "$TMP/rb-helper-only/sorbet/config" +printf 'require "minitest"\n' > "$TMP/rb-helper-only/test/test_helper.rb" + mkdir -p "$TMP/py-setupcfg/test" printf '[mypy]\n' > "$TMP/py-setupcfg/setup.cfg" touch "$TMP/py-setupcfg/test/test_x.py" @@ -421,6 +655,24 @@ touch "$TMP/py-venv/tests/test_x.py" stub "$TMP/py-venv/.venv/bin" mypy 0 stub "$TMP/py-venv/.venv/bin" pytest 0 +# Plain venv/bin (not .venv) and an active VIRTUAL_ENV are the other two +# resolution prefixes py_cmd promises. +mkdir -p "$TMP/py-venv-plain/tests" "$TMP/py-venv-plain/venv/bin" +printf '[tool.mypy]\n' > "$TMP/py-venv-plain/pyproject.toml" +touch "$TMP/py-venv-plain/tests/test_x.py" +stub "$TMP/py-venv-plain/venv/bin" mypy 0 +stub "$TMP/py-venv-plain/venv/bin" pytest 0 + +mkdir -p "$TMP/py-virtualenv/tests" "$TMP/py-virtualenv-prefix/bin" +printf '[tool.mypy]\n' > "$TMP/py-virtualenv/pyproject.toml" +touch "$TMP/py-virtualenv/tests/test_x.py" +stub "$TMP/py-virtualenv-prefix/bin" mypy 0 +stub "$TMP/py-virtualenv-prefix/bin" pytest 0 + +mkdir -p "$TMP/py-poetry/tests" +printf '[tool.mypy]\n' > "$TMP/py-poetry/pyproject.toml" +touch "$TMP/py-poetry/poetry.lock" "$TMP/py-poetry/tests/test_x.py" + mkdir -p "$TMP/py-no-tools/tests" printf '[tool.mypy]\n' > "$TMP/py-no-tools/pyproject.toml" touch "$TMP/py-no-tools/tests/test_x.py" @@ -482,6 +734,18 @@ OK="$TMP/stubs-ok"; FAIL="$TMP/stubs-fail"; HANG="$TMP/stubs-hang"; UV="$TMP/stu for t in pytest mypy pyright dotnet mvn gradle bundle; do stub "$OK" "$t" 0; done stub "$FAIL" dotnet 1 stub_body "$HANG" go 'sleep 30' +# Ignores TERM, so the watchdog's -k escalation has to finish it with KILL and +# the shell reports 137, not 124. Used against the *real* timeout, not a +# logging stub: the passthrough case below only proves the flag is forwarded, +# never that the resulting exit code is read as a timeout. +IGNTERM="$TMP/stubs-ignterm" +stub_body "$IGNTERM" go 'trap "" TERM +sleep 30' +# The real timeout has to be reachable from the case's PATH, or the gate falls +# back to the perl backend, which raises 124 by hand and never exercises the +# -k/137 path this fixture exists to cover. On macOS GNU timeout lives outside +# /usr/bin, so $BASE alone is not enough. +link_bin "$IGNTERM" timeout sleep stub "$UV" uv 0 GO="$TMP/stubs-go"; stub "$GO" go 0 # yarn is not on every machine, and the branch under test is only *how* the @@ -492,6 +756,40 @@ GO="$TMP/stubs-go"; stub "$GO" go 0 # make this the one JS case gated by perl. YARN="$TMP/stubs-yarn" stub "$YARN" yarn 0 +PNPM="$TMP/stubs-pnpm" +stub "$PNPM" pnpm 0 +BUN="$TMP/stubs-bun" +stub "$BUN" bun 0 +# Empty-suite stubs: typecheck succeeds, the test script prints the runner's +# empty-suite line and then the manager's own epilogue, exiting 1 — the exact +# shape each manager produces on a repo with no test files. +# pnpm v10/v11 — the shape a current pnpm actually prints, verified against +# pnpm 11.18.0. Writing only the v9 line here would have let the filter look +# covered while being inert on every supported pnpm. +EMPTY_PNPM="$TMP/stubs-empty-pnpm" +stub_body "$EMPTY_PNPM" pnpm 'for a in "$@"; do [ "$a" = typecheck ] && exit 0; done +echo "No test files found, exiting with code 1" +echo "[ELIFECYCLE] Test failed. See above for more details." +exit 1' +# pnpm v9 and older, still in the wild on pinned CI images. +EMPTY_PNPM9="$TMP/stubs-empty-pnpm9" +stub_body "$EMPTY_PNPM9" pnpm 'for a in "$@"; do [ "$a" = typecheck ] && exit 0; done +echo "No test files found, exiting with code 1" +echo " ELIFECYCLE Command failed with exit code 1." +exit 1' +EMPTY_YARN="$TMP/stubs-empty-yarn" +stub_body "$EMPTY_YARN" yarn 'for a in "$@"; do [ "$a" = typecheck ] && exit 0; done +echo "No test files found, exiting with code 1" +echo "error Command failed with exit code 1." +echo "info Visit https://yarnpkg.com/en/docs/cli/run for documentation." +exit 1' +EMPTY_BUN="$TMP/stubs-empty-bun" +stub_body "$EMPTY_BUN" bun 'for a in "$@"; do [ "$a" = typecheck ] && exit 0; done +echo "No test files found, exiting with code 1" +echo "error: script \"test\" exited with code 1" +exit 1' +POETRY="$TMP/stubs-poetry" +stub "$POETRY" poetry 0 CARGO="$TMP/stubs-cargo"; stub "$CARGO" cargo 0 # pytest's "no tests collected" code, with a passing mypy next to it. PY5="$TMP/stubs-py5"; stub "$PY5" pytest 5; stub "$PY5" mypy 0 @@ -537,6 +835,29 @@ EOF chmod +x "$1/go" } +# The JS test path captures the runner's output, and that capture is the one +# place where a leaked process can stall the gate without any signal reaching +# it: a detached grandchild that inherited stdout holds the pipe open, so a +# reader waits for the grandchild, not the runner, with GATE_TIMEOUT long +# elapsed and the watchdog powerless — it kills what it launched, not a reader +# blocked on a pipe. Note this is the inverse of make_escapee above, which +# sends its output to /dev/null precisely to avoid the stall; here the +# inherited stdout *is* the case. npm exits 0 immediately; only the grandchild +# lingers, so anything slower than a couple of seconds means the gate waited. +JS_HOLD="$TMP/stubs-js-hold" +mkdir -p "$JS_HOLD" +cat > "$JS_HOLD/npm" <", "$TMP/escapee-jshold.pid") or exit 1; + print F \$\$; close F; sleep 30' & +exit 0 +EOF +chmod +x "$JS_HOLD/npm" +mkdir -p "$TMP/js-hold" +printf '%s\n' '{"name":"f","scripts":{"typecheck":"node -e 0","test":"leaks"}}' \ + > "$TMP/js-hold/package.json" + ESCAPE="$TMP/stubs-escape" ESCAPEE_PID_FILE="$TMP/escapee.pid" make_escapee "$ESCAPE" "$ESCAPEE_PID_FILE" @@ -573,6 +894,43 @@ case_run js-alias 0 "$TMP/js-alias" - "run type-check" " case_run js-alias-test-only 0 "$TMP/js-alias-test-only" - "run test:unit" "checks=test" "YELLOW" case_run js-alias-tsc 0 "$TMP/js-alias-tsc" - "checks=test" "YELLOW" \ '!run tsc' '!TSC_RAN' +case_run js-tsc-noemit 0 "$TMP/js-tsc-noemit" - "run tsc" \ + "checks=typecheck,test" "GREEN" +case_run js-tsc-emit 0 "$TMP/js-tsc-emit" - "checks=test" "YELLOW" \ + '!run tsc' +case_run js-tsc-comment-noemit 0 "$TMP/js-tsc-comment-noemit" - "checks=test" "YELLOW" \ + '!run tsc' +case_run js-npm-placeholder 0 "$TMP/js-npm-placeholder" - "checks=typecheck$" "YELLOW" \ + "npm init placeholder" "'test' not counted" '!RED' '!GREEN' +case_run js-empty-suite 0 "$TMP/js-empty-suite" - "checks=typecheck$" "YELLOW" \ + "no test files found" "'test' not counted" '!RED' '!GREEN' +case_run js-empty-suite-slice 0 "$TMP/js-empty-suite-slice" - "run test:unit" \ + "checks=typecheck$" "YELLOW" "no test files found" "'test' not counted" \ + '!RED' '!GREEN' +case_run js-empty-suite-incidental 1 "$TMP/js-empty-suite-incidental" - "RED" \ + '!YELLOW' "!'test' not counted" +case_run js-empty-suite-chained 1 "$TMP/js-empty-suite-chained" - "RED" \ + '!YELLOW' "!'test' not counted" +case_run js-empty-suite-chained-exit1 1 "$TMP/js-empty-suite-chained-exit1" - "RED" \ + '!YELLOW' "!'test' not counted" +case_run js-empty-suite-chained-lower 1 "$TMP/js-empty-suite-chained-lower" - "RED" \ + '!YELLOW' "!'test' not counted" +case_run js-empty-vitest-real 0 "$TMP/js-empty-vitest-real" - "checks=typecheck$" \ + "YELLOW" "no test files found" "'test' not counted" '!GREEN' '!RED' +case_run js-empty-jest-real 0 "$TMP/js-empty-jest-real" - "checks=typecheck$" \ + "YELLOW" "no test files found" "'test' not counted" '!GREEN' '!RED' +case_run js-empty-inline-then-error 1 "$TMP/js-empty-inline-then-error" - "RED" \ + '!YELLOW' "!'test' not counted" +case_run js-pwnt 0 "$TMP/js-pwnt" - "checks=typecheck$" "YELLOW" \ + "runner reported no tests" "'test' not counted" '!GREEN' '!RED' +case_run js-real-pass 0 "$TMP/js-real-pass" - "checks=typecheck,test" "GREEN" \ + "!'test' not counted" +case_run js-node-test-empty 0 "$TMP/js-node-test-empty" - "checks=typecheck$" "YELLOW" \ + "0 tests" "'test' not counted" '!GREEN' '!RED' +case_run js-node-test-tap 0 "$TMP/js-node-test-tap" - "checks=typecheck$" "YELLOW" \ + "0 tests" "'test' not counted" '!GREEN' '!RED' +case_run js-node-test-real 0 "$TMP/js-node-test-real" - "checks=typecheck,test" "GREEN" \ + "!'test' not counted" case_run js-alias-check-types 0 "$TMP/js-alias-check-types" - "run check-types" \ "checks=typecheck,test" "GREEN" case_run js-test-and-unit 0 "$TMP/js-test-and-unit" - "run test$" \ @@ -622,6 +980,25 @@ case_run js-alias-both 0 "$TMP/js-alias-both" - "run typecheck$" \ case_run js-alias-red 1 "$TMP/js-alias-red" - "RED at 'npm run test:unit'" case_run js-yarn-alias 0 "$TMP/js-yarn-alias" "$YARN:$PATH" "yarn type-check" "yarn test:unit" \ "checks=typecheck,test" "GREEN" '!yarn run' +case_run js-pnpm 0 "$TMP/js-pnpm" "$PNPM:$PATH" "pnpm run typecheck" "pnpm run test" \ + "checks=typecheck,test" "GREEN" +case_run js-bun 0 "$TMP/js-bun" "$BUN:$PATH" "bun run typecheck" "bun run test" \ + "checks=typecheck,test" "GREEN" +# The empty-suite cap must not depend on which manager ran the script: each of +# these prints its own epilogue after the runner's line, and every one of them +# used to be read as a chained failure and sink the run to RED. +case_run js-empty-pnpm 0 "$TMP/js-empty-pnpm" "$EMPTY_PNPM:$PATH" \ + "checks=typecheck$" "YELLOW" "no test files found" "'test' not counted" \ + '!RED' '!GREEN' +case_run js-empty-pnpm9 0 "$TMP/js-empty-pnpm" "$EMPTY_PNPM9:$PATH" \ + "checks=typecheck$" "YELLOW" "no test files found" "'test' not counted" \ + '!RED' '!GREEN' +case_run js-empty-yarn 0 "$TMP/js-empty-yarn" "$EMPTY_YARN:$PATH" \ + "checks=typecheck$" "YELLOW" "no test files found" "'test' not counted" \ + '!RED' '!GREEN' +case_run js-empty-bun 0 "$TMP/js-empty-bun" "$EMPTY_BUN:$PATH" \ + "checks=typecheck$" "YELLOW" "no test files found" "'test' not counted" \ + '!RED' '!GREEN' case_run polyglot-partial 3 "$TMP/polyglot" "$OK:$NOTOOL" "checks=test" "some detected stack" case_run partial-none-ran 3 "$TMP/go-only" "$NOTOOL" "nothing ran" case_run dotnet-green 0 "$TMP/dotnet-root" "$OK:$BASE" "checks=typecheck,test" "GREEN" @@ -637,6 +1014,10 @@ case_run jvm-no-tests 0 "$TMP/jvm-no-tests" "$OK:$BASE" "mvn -q test" \ "no src/test directory found" "not counted" "no countable suite" '!GREEN' case_run jvm-module-tests 0 "$TMP/jvm-module-tests" "$OK:$BASE" "mvn -q test" "GREEN" \ '!not counted' +case_run jvm-mvnw 0 "$TMP/jvm-mvnw" "$NOTOOL" "./mvnw -q test" \ + "checks=typecheck,test" "GREEN" +case_run jvm-gradlew 0 "$TMP/jvm-gradlew" "$NOTOOL" "./gradlew test" \ + "checks=typecheck,test" "GREEN" case_run dotnet-poly-uncounted 0 "$TMP/dotnet-poly-uncounted" "$OK:$PATH" \ "checks=typecheck,test" "not counted" "no countable suite" '!GREEN' case_run py-reqs-no-source 0 "$TMP/py-reqs-no-source" - "checks=typecheck,test" "GREEN" \ @@ -647,8 +1028,42 @@ case_run rb-gemfile-only 0 "$TMP/rb-gemfile-only" "$OK:$PATH" "checks=typecheck '!not counted' case_run rb-source-no-spec 0 "$TMP/rb-source-no-spec" "$OK:$PATH" "checks=typecheck,test" \ "no spec/ and no Rakefile" "no countable suite" '!GREEN' +case_run rb-sorbet-rspec 0 "$TMP/rb-sorbet-rspec" "$OK:$BASE" \ + "bundle exec srb tc" "bundle exec rspec" "checks=typecheck,test" "GREEN" +case_run rb-rake 0 "$TMP/rb-rake" "$OK:$BASE" "bundle exec rake test" \ + "checks=test" "YELLOW" +case_run rb-both-runners 0 "$TMP/rb-both-runners" "$OK:$BASE" \ + "bundle exec srb tc" "checks=typecheck$" "YELLOW" \ + "both rspec" "rake test" "'test' not counted" "no countable suite" \ + '!bundle exec rspec' '!bundle exec rake' '!GREEN' +# A directory is not a suite: rspec on a spec/ with no *_spec.rb exits 0 on +# "0 examples" and used to reach GREEN, which unlocks dead-export deletion on a +# repo that has no tests at all. The runner must not even be invoked. +case_run rb-spec-empty 0 "$TMP/rb-spec-empty" "$OK:$BASE" \ + "bundle exec srb tc" "checks=typecheck$" "YELLOW" \ + "holds no" "'test' not counted" "no countable suite" \ + '!bundle exec rspec' '!GREEN' +case_run rb-test-empty 0 "$TMP/rb-test-empty" "$OK:$BASE" \ + "bundle exec srb tc" "checks=typecheck$" "YELLOW" \ + "holds no" "'test' not counted" "no countable suite" \ + '!bundle exec rake' '!GREEN' +# Rake::TestTask's default naming counts as a real suite on both sides. +case_run rb-both-prefix 0 "$TMP/rb-both-prefix" "$OK:$BASE" \ + "checks=typecheck$" "YELLOW" "both rspec" "'test' not counted" \ + '!bundle exec rspec' '!bundle exec rake' '!GREEN' +case_run rb-rake-prefix 0 "$TMP/rb-rake-prefix" "$OK:$BASE" \ + "bundle exec rake test" "checks=test" "YELLOW" "!'test' not counted" +case_run rb-helper-only 0 "$TMP/rb-helper-only" "$OK:$BASE" \ + "checks=typecheck$" "YELLOW" "holds no" "'test' not counted" \ + '!bundle exec rake' '!GREEN' case_run py-setupcfg 0 "$TMP/py-setupcfg" "$OK:$BASE" "checks=typecheck,test" "GREEN" case_run py-venv 0 "$TMP/py-venv" "$BASE" ".venv/bin/mypy" ".venv/bin/pytest" "GREEN" +case_run py-venv-plain 0 "$TMP/py-venv-plain" "$BASE" "venv/bin/mypy" "venv/bin/pytest" "GREEN" +GATE_ENV="VIRTUAL_ENV=$TMP/py-virtualenv-prefix" +case_run py-virtualenv 0 "$TMP/py-virtualenv" "$BASE" \ + "$TMP/py-virtualenv-prefix/bin/mypy" "$TMP/py-virtualenv-prefix/bin/pytest" "GREEN" +case_run py-poetry 0 "$TMP/py-poetry" "$POETRY:$BASE" "poetry run mypy" "poetry run pytest" \ + "checks=typecheck,test" "GREEN" case_run py-no-tools 3 "$TMP/py-no-tools" "$BASE" "toolchain 'mypy' missing" "looked in" case_run py-uv 0 "$TMP/py-uv" "$UV:$BASE" "uv run pytest" "checks=test" case_run py-pyright 0 "$TMP/py-pyright" "$OK:$BASE" "checks=typecheck" @@ -701,6 +1116,21 @@ GATE_ENV="GATE_TIMEOUT=2" case_run wd-kill-after-passthrough 4 "$TMP/go-hang" "$WD_K:$GO:$BASE" "TIMEOUT after 2s" assert_log wd-kill-after-args "$WD_LOG_K" "-k 2 2 go build ./..." +# And the code that escalation actually produces. Against the real timeout, a +# check that ignores TERM dies from the -k KILL and the shell reports 137 — +# which used to miss the `rc -eq 124` test and be announced as "RED at 'go +# build ./...'", turning "we could not tell, it hung" into "your code is +# broken". Skipped where the -k probe found no support, since then no +# escalation happens at all. +if command -v timeout >/dev/null 2>&1 && timeout -k 2 1 true >/dev/null 2>&1; then + GATE_ENV="GATE_TIMEOUT=2" + case_run wd-kill-after-137 4 "$TMP/go-hang" "$IGNTERM:$BASE" "TIMEOUT after 2s" '!RED' + elapsed_lt wd-kill-after-137-is-bounded 15 +else + skipped wd-kill-after-137 "no timeout with -k support" + skipped wd-kill-after-137-is-bounded "no timeout with -k support" +fi + # The perl backend is the one that only shows up on a machine without coreutils; # a minimal PATH keeps it covered even when the suite runs on Linux. if command -v perl >/dev/null; then @@ -717,6 +1147,16 @@ if command -v perl >/dev/null; then elapsed_lt wd-escapee-is-bounded 10 assert_reaped wd-escapee-reaped "$ESCAPEE_PID_FILE" + # Regression: the JS test path captures output, and a detached grandchild + # holding the inherited stdout kept that capture blocked for the + # grandchild's whole life — GATE_TIMEOUT=2 with a 30s leak returned in 30s, + # verdict GREEN, exit 4 never raised. The runner itself exits 0 at once, so + # the gate is only allowed the couple of seconds that costs. + GATE_ENV="GATE_TIMEOUT=2" + case_run js-hold-detached 0 "$TMP/js-hold" "$JS_HOLD:$PATH" \ + "checks=typecheck,test" "GREEN" + elapsed_lt js-hold-detached-is-bounded 10 + # Without ps there is no tree to snapshot and no start time to confirm, so # the watchdog only kills the group: the escapee survives, and asserting # otherwise would be freezing a limitation as a promise. What is contract diff --git a/scripts/test.sh b/scripts/test.sh index 1a20fa6..6ad2b1e 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -5,8 +5,22 @@ set -u cd "$(dirname "$0")" || exit 2 +# Capture gate_test.sh's NN/NN so coherence_test.sh can reuse it (avoids a +# second full matrix under scripts/test.sh). Standalone coherence still runs +# the suite itself. +GATE_TEST_CASE_COUNT="" for suite in gate_test.sh rollback_test.sh coherence_test.sh; do echo "=== $suite" - "${BASH:-bash}" "$suite" || { echo "=== $suite FAILED — stopping here"; exit 1; } + if [[ $suite == gate_test.sh ]]; then + gate_log=$(mktemp) + "${BASH:-bash}" "$suite" | tee "$gate_log" + rc=${PIPESTATUS[0]} + GATE_TEST_CASE_COUNT=$(sed -n 's|^[0-9][0-9]*/\([0-9][0-9]*\) cases passed$|\1|p' "$gate_log" | tail -1) + export GATE_TEST_CASE_COUNT + rm -f "$gate_log" + [[ $rc -eq 0 ]] || { echo "=== $suite FAILED — stopping here"; exit 1; } + else + "${BASH:-bash}" "$suite" || { echo "=== $suite FAILED — stopping here"; exit 1; } + fi done echo "=== gate_test.sh, rollback_test.sh and coherence_test.sh: all green"