diff --git a/README.en.md b/README.en.md index 99094ae..00a0a37 100644 --- a/README.en.md +++ b/README.en.md @@ -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: 57/57 cases, 5/5 properties, 49/49 invariants +# validated 2026-08: 86/86 cases, 5/5 properties, 77/77 invariants ``` The .NET heuristic was validated against the real SDK @@ -155,15 +155,42 @@ into one of three levels: | Level | Condition | What it does | |---|---|---| -| GREEN | typecheck and tests pass | runs the full phases without asking | +| 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 | diagnoses only; nothing is deleted | +| RED | no tests and no typecheck, or a baseline already failing | diagnoses only; nothing is deleted | + +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 +already broken, and since every commit demands a green gate, none of them would +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 -run the empty suite 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]`, and a -pytest run that exits 5 having collected nothing. If your suite lives outside -the usual place, promoting it is your call — the gate never promotes itself. +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 +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 +the name, which catches `test:watch:all` and leaves `test:watchdog` alone. Being +skipped and being uncounted are not the same, though: `watch` and `debug` are a +mode of the suite, so `test:watch` next to a lone `test:unit` leaves that slice +as the suite; `test:ui` may well be a suite of its own, so it is skipped and +still counted, and `test:unit` next to it is a split. + +In a polyglot repo the cap survives the other stacks. Go with tests next to a +JS half that was never counted still prints `checks=typecheck,test`, because +each word came from a different manifest — and there the gate refuses to say +GREEN, naming the stack that has no suite instead. With the level announced, it creates the cleanup branch and proceeds: @@ -218,9 +245,10 @@ rollback discards is what the skill itself created. - A dynamic import with a string assembled at runtime is invisible to the graph. The skill handles this by teaching knip (explicit entry) instead of deleting, but it is worth reviewing the generated `knip.json`. -- RED level returns a report, not a cleanup. If the project has neither tests - nor typecheck, the first step is to create a minimal verification; the skill - points the way in the report itself. +- RED level returns a report, not a cleanup. That covers a project with + neither tests nor typecheck, and one that arrives with a red suite. With no + tests, the first step is to create a minimal verification; with a broken + 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. - With a single `.sln`/`.slnx` at the root the gate passes it explicitly to @@ -233,6 +261,12 @@ rollback discards is what the skill itself created. - A folder with no git falls into RED as well, even with typecheck and tests passing. With no commit there is no rollback, and the rollback is what holds up the autonomy of the rest of the pipeline. +- The unused-deps category runs the package manager's plain install after + pruning the manifest, so the lockfile is rewritten and `node_modules` + re-resolved. That part is outside the rollback: `git restore` brings back + `package.json` and the lockfile, never the installed tree. Run the install + again if that category is the one that fails; the other two never touch the + manifest and do not need it. ## Credits diff --git a/README.md b/README.md index 51932f4..10f788d 100644 --- a/README.md +++ b/README.md @@ -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: 57/57 casos, 5/5 propriedades, 49/49 invariantes +# validado em 08/2026: 86/86 casos, 5/5 propriedades, 77/77 invariantes ``` A heurística .NET foi validada contra o SDK real (`mcr.microsoft.com/dotnet/sdk:8.0` @@ -152,15 +152,41 @@ 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 inteiras sem perguntar | +| 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 | só diagnostica; nada é deletado | - -Stack sem nenhum arquivo de teste não conta como testado: o gate não roda a -suíte vazia e o nível fica em YELLOW. Vale para Go e .NET sem arquivo de -teste, para crate Rust sem `tests/*.rs` nem `#[test]`, e para pytest que sai -5 sem coletar nada. Se a sua suíte mora fora do lugar padrão, a promoção é -sua — o gate não se promove sozinho. +| RED | sem testes e sem typecheck, ou baseline já vermelho | só diagnostica; nada é deletado | + +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 +quebrado, e como todo commit exige gate verde, nenhum deles aconteceria. A +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 +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 +executar e não contar são coisas diferentes: `watch` e `debug` são um modo da +suíte, então `test:watch` ao lado de um `test:unit` sozinho deixa essa fatia +valendo como a suíte; `test:ui` pode muito bem ser uma suíte à parte, então ele +não roda e mesmo assim conta, e aí `test:unit` ao lado dele já é uma divisão. + +Em repositório poliglota o cap sobrevive aos outros stacks. Go com testes ao +lado de uma metade JS que ninguém contou ainda imprime `checks=typecheck,test`, +porque cada palavra veio de um manifesto diferente — e aí o gate se recusa a +dizer GREEN, e nomeia o stack que ficou sem suíte. Com o nível anunciado, ela cria a branch de limpeza e segue: @@ -213,9 +239,10 @@ rollback joga fora foi ela mesma que criou. - Import dinâmico com string montada em runtime é invisível ao grafo. A skill trata isso ensinando o knip (entry explícito) em vez de deletar, mas vale revisar o `knip.json` gerado. -- Nível RED devolve relatório, não limpeza. Se o projeto não tem teste nem - typecheck, o primeiro passo é criar uma verificação mínima; a skill aponta o - caminho no próprio relatório. +- Nível RED devolve relatório, não limpeza. Cai aí quem não tem teste nem + typecheck, e também quem chega com a suíte vermelha. Sem teste, o primeiro + 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. - Com uma única `.sln`/`.slnx` na raiz o gate a passa explícita ao `dotnet`; @@ -228,6 +255,12 @@ rollback joga fora foi ela mesma que criou. - Pasta sem git também cai em RED, mesmo com typecheck e testes passando. Sem commit não existe rollback, e é o rollback que sustenta a autonomia do resto do pipeline. +- A categoria de deps roda o install simples do gerenciador depois de podar o + manifesto, então o lockfile é reescrito e o `node_modules` é resolvido de + novo. Essa parte fica fora do rollback: o `git restore` traz de volta o + `package.json` e o lockfile, nunca a árvore instalada. Se for essa categoria + que falhar, rode o install outra vez; as outras duas não mexem no manifesto e + não precisam. ## Créditos diff --git a/SKILL.md b/SKILL.md index df8ce7b..d49c7f6 100644 --- a/SKILL.md +++ b/SKILL.md @@ -60,7 +60,6 @@ of it, not a preference. git rev-parse --is-inside-work-tree # is there a repo at all? git status --porcelain # anything uncommitted? git rev-parse --abbrev-ref HEAD # current branch (HEAD = detached) -[ -f package.json ] && grep -E '"(test|typecheck|lint|build)"' package.json ``` Read those three answers before running anything else: @@ -102,8 +101,10 @@ it finds (compiling counts as typecheck). Classify by the `[gate] checks=...` line, which lists what actually ran, and not by the exit code alone: GREEN requires `typecheck` and `test` in the list; a partial list caps at YELLOW, and the script itself says so. Exit 0 = -everything that ran passed; 1 = something failed; 2 = bad path (the argument is -not a directory the script can enter, so nothing was checked — fix the path and +everything that ran passed; 1 = a check failed, which is RED — and the script +stops at the first red, so there is no `checks=` line to classify by, only the +`RED at ''` line naming what broke; 2 = bad path (the argument is not a +directory the script can enter, so nothing was checked — fix the path and rerun); 3 = no runnable check **or** some detected stack had no toolchain (`PARTIAL` — including in a polyglot repo where another stack passed); 4 = a check hit the watchdog (`GATE_TIMEOUT`, 900s per check by default, `0` @@ -119,19 +120,53 @@ timeout: a check that legitimately exits 124 is read as a timeout. | Signal | Level | Behavior | |---|---|---| | Typecheck **and** tests pass | **GREEN** | Runs phases 1 and 3 in full without asking. Phase 2 stops at the checkpoint. | -| Tests exist but fail, or typecheck only | **YELLOW** | Runs phase 1 (deps and orphan files only, **not** exports). Stops before phase 2 and reports. | -| No tests and no typecheck | **RED** | Diagnoses only. Does not delete, does not move, does not commit. Delivers a report. | +| 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. | | No git repository | **RED** | Diagnoses only, regardless of the gate result — there is no HEAD to roll back to. | `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. Only the user promotes -it, by pointing at the suite that lives somewhere the gate does not look; the -skill never promotes itself. - -If the baseline is already broken, **fix it or warn before touching -anything**. You need an initial green to distinguish what you broke from what -was already broken. +script names the stack in the `'test' not counted` line, and every stack whose +suite could not be counted goes through it. Read those lines, not only +`checks=`: in a polyglot repo one stack supplies the suite while another has +none, so `checks=typecheck,test` can be describing two different stacks. The +gate refuses to announce GREEN there and says why (`a detected stack has no +countable suite`). Only the user promotes a cap, by pointing at the suite that +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. + +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. + +**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 +what was already broken. It also has nowhere to go: every commit here needs a +green gate, so a run that starts red deletes, rolls back and commits nothing, +category after category. **Fix it or warn before touching anything** — until +then the deliverable is the diagnosis, naming the check that failed. Announce the detected level in one line and move on. Do not ask permission for the level. @@ -246,9 +281,22 @@ graph (entry, project, paths, plugin), not to silence the output. ## 1.2 Run in production mode ```bash -npx knip --production --reporter json > knip-report.json +npx knip --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` +truncates the file before knip even starts, so a crash leaves an empty report +that 1.3 reads as "nothing to delete" — a silent failure. `--no-exit-code` is +what makes the `&&` usable: knip exits 1 whenever it finds issues, which is the +normal case here, and 2 only when it actually failed. + +Check the report before 1.3 consumes it: `knip-report.json.tmp` has to be gone, +and `knip-report.json` has to be non-empty and parse as JSON. The temp file is +the check that catches a failed run — when knip breaks, the `&&` skips the `mv` +and the *previous* report stays on disk, non-empty and perfectly parseable, so +the content checks alone would wave a stale list through. A `.tmp` left behind +means the run did not finish: delete it and fix knip instead of proceeding. + Production mode excludes tests and devDependencies automatically. That matters because a function imported only by a test is technically alive, but it is dead as far as the application is concerned — and that is exactly the code you want @@ -259,9 +307,11 @@ 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 → `git add -A` → gate → commit. 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, run the stack's equivalent commands by +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 @@ -269,6 +319,42 @@ 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. + +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 +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. + ``` 1. unused deps → "chore: remove unused deps" 2. orphan files → "chore: remove orphan files" @@ -278,12 +364,52 @@ of the user's. Kept separate because if something breaks in production two weeks from now, the user needs to revert *one* commit — not a 400-file cleanup. +**Unused deps: install after pruning the manifest.** Removing an entry from +`package.json` does not remove the package from `node_modules`, and the gate +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: + +```bash +npm install # npm +pnpm install --no-frozen-lockfile # pnpm +yarn install --no-immutable # yarn berry (yarn 1: yarn install) +bun install # bun +``` + +Plain, never the frozen form. `--frozen-lockfile`, `--immutable` and the CI +defaults that turn them on refuse a lockfile that no longer matches the +manifest — which is exactly the state a correct prune produces, so a good +deletion would come back as a red gate for the wrong reason. `npm ci` is the +same refusal under another name: it aborts with `EUSAGE` when `package.json` +and `package-lock.json` are out of sync, and it never writes the lockfile, so +it cannot get the prune out of this state either. The updated lockfile goes in +this category's commit: it is what carries the prune to every other machine. + +**Regenerate the report between categories.** After each category's commit, run +the 1.2 command again — same hardened form, same file — and read the next +category from the fresh report. The three feed each other: deleting orphan +files kills exports the old report still saw as alive and frees deps it saw as +used, while some exports it lists as dead live in files the previous category +already removed. On a frozen report those second-order items survive the +cleanup and a category tries to edit paths that no longer exist. The cost is +one extra knip run per category that commits. + +A category that is skipped (YELLOW does not run exports) or that fails its gate +leaves no commit and nothing in the tree for knip to read differently, so there +is nothing to regenerate from — keep the current report and go to the next +category. The regeneration after the last category that did commit is the one +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. +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. Do not run `knip --fix` until the config has settled for two or three rounds with no surprises. @@ -410,6 +536,7 @@ structure with a rationale. Only then execute. GREEN level runs the whole plan without asking. One folder per commit: ```bash +mkdir -p src/features/billing git mv src/utils/format.ts src/features/billing/format.ts # always git mv ``` @@ -479,5 +606,12 @@ Branch: `cleanup/YYYYMMDD` · Level: GREEN · N commits - (nothing) ``` +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. + If the level was RED, the report is diagnosis only: list what you would do and -what needs to exist (tests, typecheck) to make it possible. +what needs to exist or be fixed (tests, typecheck, a baseline that passes) to +make it possible. diff --git a/references/audit.md b/references/audit.md index 2a2b0c8..a41a827 100644 --- a/references/audit.md +++ b/references/audit.md @@ -17,10 +17,25 @@ paragraph mental model of the architecture as it actually is — you built most of it while configuring the phase 1 dead-code tool. If the model contradicts the README, the contradiction is itself a finding. -Pull churn data: `git log --stat --since="6 months ago"`. Intersect the 20 -largest files with the 20 most modified — that intersection is where debt -usually hides, and it is what separates "actually has debt" from "just looks -messy". +Pull churn data, then rank the most modified files: + +```bash +git log --no-merges --stat --since="6 months ago" +git log --no-merges --since="6 months ago" --format= --name-only | sed '/^$/d' | sort | uniq -c | sort -rn +``` + +`--since` is the one that carries weight, and it has to be on both commands: +without it the ranking is a lifetime touch count, and a file rewritten hard +three years ago and untouched since outranks whatever is actually hot now. +`--no-merges` does less than it looks. Git prints no diff for a merge unless +asked with `-m` or `--cc`, so merges contribute no file names either way and +the flag only keeps their empty entries out of the `--stat` read. Do not reach +for `-m` to "fix" that: it credits the merge with every file it brought in, and +one big merge then owns the top of the ranking. + +Intersect the 20 largest files with the 20 most modified — that intersection +is where debt usually hides, and it is what separates "actually has debt" +from "just looks messy". ## The nine dimensions diff --git a/references/knip-config.md b/references/knip-config.md index 22b9233..2e43c5d 100644 --- a/references/knip-config.md +++ b/references/knip-config.md @@ -24,7 +24,17 @@ names. Adjust the `$schema` version below to the major in use. 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 --reporter json > knip-report.json` +5. `npx knip --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 + +The temp file plus `&&` is what keeps a crashed run from wiping the previous +report — a plain `>` truncates it before knip starts. `--no-exit-code` is +required there because knip exits 1 on every run that finds issues; only exit +2 means it failed. The missing `.tmp` is the load-bearing half of step 6: when +knip breaks, the `&&` skips the `mv` and the *previous* report stays on disk, +non-empty and perfectly parseable, so the content checks alone would wave a +stale list through. The right mindset: when knip reports something unexpected, it is telling the truth about the module graph — it could not reach that code from an entry. A diff --git a/references/phase-2-consolidation.md b/references/phase-2-consolidation.md index d9a0630..4a2ea5e 100644 --- a/references/phase-2-consolidation.md +++ b/references/phase-2-consolidation.md @@ -48,11 +48,20 @@ Imagine deleting the module. set — ask for it explicitly with `npx knip --cycles` (shortcut for `--include cycles`), or a plain `npx knip` will report nothing about circular dependencies. -2. **Cross with churn volume (file-level).** `git log --format=%H - --name-only | sort | uniq -c | sort -rn` — the intersection between +2. **Cross with churn volume (file-level).** The intersection between "changed a lot" and "heavily coupled" is where consolidation pays the - most. This is a different, weaker signal than the pair-level co-change of - step 0: it says a file is hot, not that two files move together. + most. Rank the files by + + ```bash + git log --no-merges --since="6 months ago" --format= --name-only | sed '/^$/d' | sort | uniq -c | sort -rn + ``` + + Keep the window: without `--since` the count is lifetime, and a file + rewritten hard years ago outranks whatever is hot now. + This is a different, weaker signal than the pair-level co-change of step + 0: it says a file is hot, not that two files move together — and it is + windowed where step 0's ratio is not, so the two rankings are read side by + side, never subtracted from each other. 3. **Actually read the candidate clusters.** Do not judge by file name. 4. **Rank by confidence**, not by size. diff --git a/references/phase-3-structure.md b/references/phase-3-structure.md index aa0f75a..5815b08 100644 --- a/references/phase-3-structure.md +++ b/references/phase-3-structure.md @@ -56,9 +56,27 @@ return: anyone new understands it without explanation. One folder per commit. Always: ```bash +mkdir -p src/features/billing git mv src/utils/format.ts src/features/billing/format.ts ``` +The `mkdir -p` is not decoration: phase 3 applies a structure that does not +exist yet, and moving into a directory nobody created fails with `fatal: +renaming ... failed: No such file or directory` and exit 128. That is a +missing precondition, not a failed step — create the directory and repeat the +move. Do not fall back to `mv` plus `git rm`/`git add`: that is `rm` + +`create` under another name. + +Create exactly the directory the move lands in, and no others. Git tracks +files, not directories, so a folder created for a move that never happened, or +for a commit whose gate failed, is left behind as an empty untracked directory: +`git restore --staged --worktree .` cannot remove it. Sweep the leftover when a +folder's commit does not land, by name: `rmdir` the path you just created (and +`rmdir -p` if the move created parents, which stops at the first one that is not +empty). Never a blanket `find ... -type d -empty -delete`: it also takes empty +directories that were already there and are not yours to remove, and nothing in +this pipeline can put them back. + `git mv` preserves history. `rm` + `create` destroys that file's `git blame` — exactly the information someone will want six months from now when asking "why is this like this". diff --git a/scripts/coherence_test.sh b/scripts/coherence_test.sh index 7c31951..31d7559 100755 --- a/scripts/coherence_test.sh +++ b/scripts/coherence_test.sh @@ -222,6 +222,8 @@ Typecheck at the end of each folder one mandatory checkpoint typecheck and tests between each step only what has not been committed +"(test|typecheck|lint|build)" +Tests exist but fail DEAD # The exception: phase-3-structure.md explains why a "pure git mv" commit is @@ -254,6 +256,12 @@ for f in SKILL.md references/phase-2-consolidation.md references/phase-3-structu fi done +# IFS is pinned to newline for the same reason section 6 pins it: the list comes +# from grep -l, and under the default IFS a path containing a space would split +# into fragments, leaving grep to fail on stderr while the invariant reports a +# verdict for a file it never opened. +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" @@ -262,6 +270,7 @@ for f in $derived; do "runs the gate as a protocol step without staging first" fi done +IFS=$old_ifs # 5. The file trees in the READMEs match what is on disk. -------------------- for readme in README.md README.en.md; do @@ -283,6 +292,300 @@ for readme in README.md README.en.md; do done done +# 6. Whoever shows a `git mv` also shows the mkdir -p it needs. --------------- +# Phase 3 applies a structure that does not exist yet, so the destination +# directory has to be created before the move: `git mv` into a missing +# directory dies with exit 128, and an agent running unsupervised reads that +# as a failed step. Only command lines count — a line whose first word is +# `git mv` — so the prose mentions, where the phrase is named inside backticks +# and not run, stay out of it. That is the whole filter: the READMEs are +# scanned like everything else, because both already carry fenced bash blocks +# and a move added to one of them would be read and run like any other. Leading +# whitespace is allowed: a move written as a step inside a numbered list is +# still a command, and anchoring at column 0 would exempt it silently. +gitmv_files() { + grep -l -E '^[[:space:]]*git mv ' SKILL.md README.md README.en.md references/*.md \ + 2>/dev/null | sort +} + +moves=$(gitmv_files) + +# Floor, same reason as in section 4: a derivation that comes back without the +# two files that carry the move today is broken, and the loop below would +# assert over an empty list and pass. +for f in SKILL.md references/phase-3-structure.md; do + if printf '%s\n' "$moves" | grep -qx -F -- "$f"; then + pass "$f is derived as a file that runs git mv" + else + fail "$f is derived as a file that runs git mv" \ + "no git mv command line — derived list: $(printf '%s' "$moves" | tr '\n' ' ')" + fi +done + +# The mkdir has to be in the same block as the move, and it has to have created +# the directory that move lands in — not merely some directory. What is being +# read is a snippet, not a page: a fence or a blank line ends the block, so a +# `git mv` in a later snippet cannot inherit an earlier snippet's mkdir. Inside +# a block one `mkdir -p` covers every move into that folder — one mkdir and +# three moves is the shape a real plan has, and demanding a redundant mkdir per +# line would reject correct documentation. Tracking the folders instead of a +# bare "some mkdir happened" flag is what catches the realistic version of the +# bug: a block that creates one destination and then moves into a second one. +# A move with no `/` in the destination is a rename in place and needs no +# directory, so it is never flagged. +# Both readers work on the last argument, not on a fixed field number: `git mv` +# takes flags and takes several sources into one directory, and `dest = $4` +# 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. +gitmv_orphans() { + awk ' + 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 + argc = 0 + for (i = 1; i <= n; i++) { + if (out[i] == "" || out[i] ~ /^-/) continue + gsub(/"/, "", out[i]) + argv[++argc] = out[i] + } + return argc + } + /^[[: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 } + ' "$1" +} + +# Section 0's rule: the invariant is only as good as the extractor. A scanner +# that never reports anything would make the loop below pass over everything, +# so it is exercised on a synthetic page first. Every line of the fixture pays +# for itself — delete any one rule of the scanner and the count moves: +# `git mv c` catches the destination check, `git mv e` the fence reset (the +# mkdir above it created the very folder it lands in, so only the fence can +# 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. +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/' '```' \ + > "$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: +$orphans" + +# IFS is pinned to newline: the list comes from grep -l, and the default IFS +# would split a path containing a space into pieces, leaving awk to fail on +# stderr while $orphan stays empty — an "ok" for a file never read. +old_ifs=$IFS +IFS=$'\n' +for f in $moves; do + orphan=$(gitmv_orphans "$f") + check "git mv covered by a mkdir -p in $f" \ + "$([[ -z $orphan ]] && echo 0 || echo 1)" "$orphan" +done +IFS=$old_ifs + +# 7. The level table says the same thing in the three files. ----------------- +# The level is the whole dispatch of the skill: SKILL.md's table decides what +# runs, and both READMEs promise the user that same behavior. They drifted once +# — SKILL.md called a failing suite YELLOW and sent it into phase 1, while the +# protocol a few lines below refuses to commit without a green gate, so the +# announced level was one no run could ever honor. README.md is in Portuguese, +# so byte equality across the three is out; what is asserted instead is the part +# that has to hold in any language. +LEVEL_FILES="SKILL.md README.md README.en.md" + +# LEVEL_CELL — a level standing alone in its own cell, bold or not. Both +# extractors below use it, and that is the point: naming a level in a row's +# prose ("not GREEN, a partial net") must not make the row count as that level's +# row for one of them and not the other. It is also what keeps some other table +# from being mistaken for this one — references/other-stacks.md has rows like +# `| Go | deadcode | yes, at GREEN |`, where GREEN is prose inside a cell, not +# the cell. +# Written with bracket expressions rather than backslash escapes: awk's `-v` +# eats one level of backslash before the regex is ever compiled, so `\|` would +# arrive as a bare alternation and the pattern would not compile. +LEVEL_CELL='[|][[:space:]]*[*]*(GREEN|YELLOW|RED)[*]*[[:space:]]*[|]' + +# level_rows — the rows of the level table. Anchored to the table, not +# to the file: the first run of consecutive `|` lines carrying a level cell is +# the table, and the run ends at the first line that is not a table row. A +# level word in some other table of the same file (an exit-code table, a +# per-stack table) is then neither counted as a row nor picked up as one. +level_rows() { + awk -v cell="$LEVEL_CELL" ' + /^\|/ && $0 ~ cell { intable = 1; print; next } + intable && !/^\|/ { exit } + ' "$1" +} + +# Consecutive repeats collapse on purpose: SKILL.md carries a fourth row (no git +# repository, which is RED before the gate even runs) that the READMEs keep as a +# bullet under known limits. That asymmetry is deliberate — the table is what an +# agent dispatches on, the READMEs are prose for a human who already read the +# requirement. What may not differ is which levels exist and how they escalate. +level_shape() { + level_rows "$1" | + awk -v cell="$LEVEL_CELL" 'match($0, cell) { + l = substr($0, RSTART, RLENGTH) + match(l, /GREEN|YELLOW|RED/) + l = substr(l, RSTART, RLENGTH) + if (l != prev) { out = out sep l; sep = " " } + prev = l + } END { print out }' +} + +# Same rule as section 0, and both halves have to be paid for. The trailing +# GREEN outside the table is what proves the extractor stops at the table. The +# YELLOW that comes back *after* the RED row, inside the table, is what proves +# the collapse is of consecutive repeats and not a global dedupe: with a +# seen-set the fixture would still read GREEN YELLOW RED and a real table that +# lost a level would slip through the check unchanged. +# The decoy row on top is the third half: it names a level inside a cell of +# prose, the shape references/other-stacks.md already uses. Matching a level +# anywhere in a `|` line would open the table there, and the blank line under it +# would close the table again before the real one ever started — the extractor +# would read [GREEN] and every assertion below it would be about the wrong rows. +printf '%s\n' '| go | deadcode | yes, at GREEN |' '' \ + '| a | GREEN | x |' '| b | YELLOW | x |' '| c | YELLOW | x |' \ + '| d | RED | x |' '| e | YELLOW | x |' '' 'prose' '| f | GREEN | x |' \ + > "$SELFTMP/levels.md" +got=$(level_shape "$SELFTMP/levels.md") +check "level extractor collapses repeats and stops at the table" \ + "$([[ $got == 'GREEN YELLOW RED YELLOW' ]] && echo 0 || echo 1)" \ + "read [$got], expected [GREEN YELLOW RED YELLOW]" + +for f in $LEVEL_FILES; do + shape=$(level_shape "$f") + check "level table of $f reads GREEN YELLOW RED" \ + "$([[ $shape == 'GREEN YELLOW RED' ]] && echo 0 || echo 1)" \ + "levels in order: [$shape]" +done + +# The condition that puts a run at each level, in each file's own language, +# checked against that level's own row and not merely somewhere in the file — +# the row is what gets read at classification time. The shape check above +# cannot carry this on its own: flipping the RED row to YELLOW leaves the +# collapsed shape at GREEN YELLOW RED (SKILL.md's fourth row is RED too), which +# is exactly the drift this section exists to catch. +# The level is matched as a cell of its own — `| RED |` or `| **RED** |`, which +# covers both layouts in use (the READMEs put the level first, SKILL.md second) +# — and not as a substring of the row. A row is free to name another level in +# its prose without stealing that level's assertion. +level_row() { + level_rows "$1" | grep -E -m1 -- "\|[[:space:]]*\**$2\**[[:space:]]*\|" +} + +for triple in \ + 'SKILL.md|GREEN|Typecheck **and** tests pass' \ + 'SKILL.md|YELLOW|A partial net, or no test file in the stack' \ + 'SKILL.md|RED|A check fails, or no tests and no typecheck' \ + 'README.en.md|GREEN|typecheck and tests pass' \ + 'README.en.md|YELLOW|partial net, or no test file in the stack' \ + 'README.en.md|RED|no tests and no typecheck, or a baseline already failing' \ + 'README.md|GREEN|typecheck e testes passam' \ + 'README.md|YELLOW|rede parcial, ou nenhum arquivo de teste no stack' \ + 'README.md|RED|sem testes e sem typecheck, ou baseline já vermelho' +do + f=${triple%%|*} + rest=${triple#*|} + level=${rest%%|*} + cond=${rest#*|} + row=$(level_row "$f" "$level") + check "$level row of $f states the canonical condition" \ + "$(printf '%s' "$row" | grep -q -F -- "$cond" && echo 0 || echo 1)" \ + "expected [$cond] — row reads: ${row:-}" +done + +# 8. A command that lives in two files is one string, byte for byte. --------- +# Same rule as the rollback in section 1, and the same reason: both of these +# are copied by hand into two files, both get executed, and a flag fixed in one +# copy and not the other leaves two protocols behind. The stale copy is the +# dangerous one — the unhardened knip form (`> knip-report.json`) truncates the +# 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_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' +CHURN_FILES="references/audit.md references/phase-2-consolidation.md" + +for f in $KNIP_FILES; do + check "knip report command verbatim in $f" \ + "$(grep -q -F -- "$KNIP_CMD" "$f" 2>/dev/null && echo 0 || echo 1)" \ + "missing the canonical form: $KNIP_CMD" +done + +for f in $CHURN_FILES; do + check "churn ranking command verbatim in $f" \ + "$(grep -q -F -- "$CHURN_CMD" "$f" 2>/dev/null && echo 0 || echo 1)" \ + "missing the canonical form: $CHURN_CMD" +done + +# ...and no variant of either anywhere else in the repo. The markers are the +# parts that only this command has a reason to use: knip only ever writes a +# report through `--reporter json`, and nothing else counts with `uniq -c`. +loose=$(count_matches '--reporter json') +canon=$(count_matches "$KNIP_CMD") +check "no knip report variant in the repo" \ + "$([[ $loose -eq $canon ]] && echo 0 || echo 1)" \ + "$loose uses of '--reporter json', $canon of them the canonical command" + +loose=$(count_matches 'uniq -c') +canon=$(count_matches "$CHURN_CMD") +check "no churn ranking variant in the repo" \ + "$([[ $loose -eq $canon ]] && echo 0 || echo 1)" \ + "$loose uses of 'uniq -c', $canon of them the canonical command" + +# 9. The validated-run line in the READMEs counts the suite that exists. ------ +# Both READMEs publish the result of a docker run ("N/N casos") as evidence the +# 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) +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" +done + echo "----" echo "$((total-failures))/$total invariants held" [[ $failures -eq 0 ]] diff --git a/scripts/gate.sh b/scripts/gate.sh index b0c2bec..01ade63 100755 --- a/scripts/gate.sh +++ b/scripts/gate.sh @@ -3,6 +3,8 @@ # Detects the stack from the root manifest (there may be more than one; runs all). # Requires bash (macOS 3.2 is fine). Usage: gate.sh [dir] # Output: "[gate] checks=..." lists what actually ran (compiling counts as typecheck). +# The verdict line below it is GREEN only when checks=typecheck,test AND every +# detected stack had a countable suite; otherwise it names the cap(s). # Exit 0 = everything that ran passed · 1 = some check failed · # 2 = bad path (the argument is not a reachable directory: nothing was checked) · # 3 = no runnable check OR a detected stack has no toolchain (PARTIAL) — @@ -23,6 +25,7 @@ cd "$target" || { ran_typecheck=0 ran_test=0 incomplete=0 +uncounted=0 # --- Watchdog --------------------------------------------------------- # A hanging check (a test waiting on a port, a REPL, a prompt) would freeze the @@ -161,7 +164,7 @@ run() { exit 4 fi if [[ -n $no_tests_rc && $rc -eq $no_tests_rc ]]; then - no_tests "$nt_label" "no tests collected (exit $rc)" + uncounted_suite "$nt_label" "no tests collected (exit $rc)" return 0 fi if [[ $rc -ne 0 ]]; then @@ -180,12 +183,30 @@ missing() { incomplete=1 } -# no_tests — a green run with no test file is not a tested -# repo. Does not set incomplete: the verdict stays exit 0 with checks=typecheck, -# which is the YELLOW cap, and the user can promote it by hand. +# no_tests [advice] — a green run with no test file is not a +# tested repo. Does not set incomplete: the verdict stays exit 0, which is the +# YELLOW cap, and the user can promote it by hand. Every stack whose suite could +# not be counted goes through here, because the literal "'test' not counted" is +# the marker SKILL.md tells the agent to classify by. A checks= list that is +# merely short (checks=test, no typecheck) is also a YELLOW cap and does not +# come through here — the missing word in checks= is its own marker. +# is how the user gets past the cap; it defaults to promotion, which +# is wrong for a suite that is present but sliced, so those callers pass their own. no_tests() { - echo "[gate] $1: $2 — 'test' not counted (YELLOW cap; promote by hand if the" \ - "suite lives elsewhere)" >&2 + local advice=${3:-"promote by hand if the suite lives elsewhere"} + echo "[gate] $1: $2 — 'test' not counted (YELLOW cap; $advice)" >&2 +} + +# uncounted_suite [advice] — no_tests for a whole stack, plus +# the flag the verdict reads. Without it a polyglot repo hides the cap: another +# stack supplies 'test', checks= reads typecheck,test and the run is announced +# GREEN while this stack's suite never ran — which is exactly what GREEN is not +# allowed to mean, since it unlocks dead-export deletion. Per-project caps +# inside one stack (a .NET project with no test project next to one that has +# them) are not this: there the stack's suite did run, so they call no_tests. +uncounted_suite() { + uncounted=1 + no_tests "$@" } py_missing() { @@ -242,12 +263,106 @@ if [[ -f package.json ]]; then echo "[gate] package.json unparseable — JS/TS checks skipped" >&2 incomplete=1 else - for script in typecheck test; do - if node -e "const s=require('./package.json').scripts||{};process.exit(s['$script']?0:1)" 2>/dev/null; then - if [[ $PM == yarn ]]; then run "$script" yarn "$script" - else run "$script" "$PM" run "$script"; fi - fi - done + # js_script