senokosov/pre-commit-hooks
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(file-length): grandfather от merge-base, а не от HEAD #15

Merged
claude-secretary merged 3 commits from fix/grandfather-from-ref into main 2026-08-26 22:47:39 +07:00
Conversation 1 Commits 3 Files changed 5 +221 -15
claude-secretary commented 2026-08-26 22:35:13 +07:00
Owner
Copy link

Проблема

check-file-length в CI-инвокации отдавал Passed, ничего не проверив.

Grandfather-baseline считался как git show HEAD:${file}. В настоящем git
pre-commit-хуке это верно: новый коммит ещё не создан, HEAD:file — версия до
правки. Но в CI (pre-commit run --from-ref X --to-ref Y после чекаута на
коммит MR) HEAD — это сам проверяемый коммит, old всегда равен lines,
условие lines <= old выполняется всегда, и grandfather срабатывает на чём
угодно.

Тихо не ловились ровно те две ситуации, ради которых хук ставили:

  1. новый файл сверх порога, добавленный в MR;
  2. рост уже-большого файла.

Обе воспроизведены на реальном пайплайне в
volody/dispatcher#186:
новый файл на 528 строк (src fail = 500) и main.py 4864 → 4896 — обе джобы
зелёные.

Это не ложное срабатывание, а обратное: гейт молча не делает то, ради чего
стоит, и при этом создаёт ощущение защиты.

Решение

База берётся оттуда же, откуда pre-commit берёт список файлов:

Запуск База
git pre-commit hook HEAD
--from-ref X --to-ref Y merge-base X и Y
--all-files HEAD (режим аудита, гейта нет)

PRE_COMMIT_FROM_REF/PRE_COMMIT_TO_REF pre-commit выставляет ровно в
CI-режиме — проверено эмпирически на pre-commit 4.6.0, в --all-files и в
обычном коммите их нет.

Merge-base, а не вершина from-ref. Иначе split, приехавший в целевую ветку
после ветвления, засчитается текущему MR как рост: файл в целевой сжали до 520,
у нас он остался 600 — сравнение с вершиной даёт «600 > 520, рост», хотя ветка
файл не трогала.

Не резолвится merge-base — откат на сам from-ref; не резолвится и он — git show промахивается, old = 0, большой файл падает. Ошибка в сторону FAIL, а не
в сторону зелёного.

Тест

fixtures/check_grandfather_baseline.sh — отдельный гейт, а не кейс в
run_smoke.sh, и это принципиально: run_smoke.sh дёргает хук напрямую списком
файлов, где HEAD и так «старая» версия, — в такой инвокации дыры не видно
вообще. Гейт поднимает настоящий репозиторий и зовёт pre-commit run --from-ref/--to-ref, то есть ровно то, что делает CI консьюмеров.

Восемь кейсов: рост god-файла, новый файл сверх порога, правка без роста, сжатие,
нетронутый маленький файл, локальный commit-хук, а также два на выбор именно
merge-base — целевая ветка сжала файл, а наша ветка его либо не растит (зелёный),
либо растит (красный).

Проверено:

  • до фикса гейт валится на кейсах 1 и 2 — тех самых из issue;
  • после проходит все восемь;
  • мутация «merge-base → сам from-ref» валит кейс 7, то есть выбор базы
    действительно закреплён тестом, а не задекларирован;
  • run_smoke.sh — 94 OK, регрессий нет;
  • check_wheel_matches_sources.py — зелёный.

Что дальше, вне этого PR

Консьюмерам нужен bump rev: — jamzap/backend, jamzap/frontend,
jamzap/mobile и volody/dispatcher все зовут хук через
--from-ref/--to-ref в CI, то есть защита от роста у всех четырёх сейчас
фиктивна. В .gitlab-ci.yml фронта и мобилки лежит комментарий, прямо
утверждающий обратное («grandfather для check-file-length берётся от
merge-base») — его тоже надо править, иначе заблуждение поедет дальше.

Тот же класс бага в неслитом
#7
(check_max_filename_words.py, git cat-file -e HEAD:path) — в CI новый файл
с длинным именем всегда «уже был в HEAD». Отмечу там, чтобы не уехало в релиз.

Версия 0.13.0. Номер пересекается с открытым
#13 — этот PR
срочнее и идёт первым, #13 после мержа перенумерую в 0.14.0.

## Проблема `check-file-length` в CI-инвокации отдавал `Passed`, ничего не проверив. Grandfather-baseline считался как `git show HEAD:${file}`. В настоящем git pre-commit-хуке это верно: новый коммит ещё не создан, `HEAD:file` — версия до правки. Но в CI (`pre-commit run --from-ref X --to-ref Y` после чекаута на коммит MR) `HEAD` — это **сам проверяемый коммит**, `old` всегда равен `lines`, условие `lines <= old` выполняется всегда, и grandfather срабатывает на чём угодно. Тихо не ловились ровно те две ситуации, ради которых хук ставили: 1. новый файл сверх порога, добавленный в MR; 2. рост уже-большого файла. Обе воспроизведены на реальном пайплайне в [volody/dispatcher#186](https://gitlab.homedevlab.ru/volody/dispatcher/-/work_items/186): новый файл на 528 строк (`src fail` = 500) и `main.py` 4864 → 4896 — обе джобы зелёные. Это не ложное срабатывание, а обратное: гейт молча не делает то, ради чего стоит, и при этом создаёт ощущение защиты. ## Решение База берётся оттуда же, откуда pre-commit берёт список файлов: | Запуск | База | |---|---| | git pre-commit hook | `HEAD` | | `--from-ref X --to-ref Y` | merge-base `X` и `Y` | | `--all-files` | `HEAD` (режим аудита, гейта нет) | `PRE_COMMIT_FROM_REF`/`PRE_COMMIT_TO_REF` pre-commit выставляет ровно в CI-режиме — проверено эмпирически на pre-commit 4.6.0, в `--all-files` и в обычном коммите их нет. **Merge-base, а не вершина from-ref.** Иначе split, приехавший в целевую ветку после ветвления, засчитается текущему MR как рост: файл в целевой сжали до 520, у нас он остался 600 — сравнение с вершиной даёт «600 > 520, рост», хотя ветка файл не трогала. Не резолвится merge-base — откат на сам from-ref; не резолвится и он — `git show` промахивается, `old` = 0, большой файл падает. Ошибка в сторону FAIL, а не в сторону зелёного. ## Тест `fixtures/check_grandfather_baseline.sh` — отдельный гейт, а не кейс в `run_smoke.sh`, и это принципиально: `run_smoke.sh` дёргает хук напрямую списком файлов, где `HEAD` и так «старая» версия, — в такой инвокации дыры **не видно вообще**. Гейт поднимает настоящий репозиторий и зовёт `pre-commit run --from-ref/--to-ref`, то есть ровно то, что делает CI консьюмеров. Восемь кейсов: рост god-файла, новый файл сверх порога, правка без роста, сжатие, нетронутый маленький файл, локальный commit-хук, а также два на выбор именно merge-base — целевая ветка сжала файл, а наша ветка его либо не растит (зелёный), либо растит (красный). Проверено: * **до фикса** гейт валится на кейсах 1 и 2 — тех самых из issue; * **после** проходит все восемь; * мутация «merge-base → сам from-ref» валит кейс 7, то есть выбор базы действительно закреплён тестом, а не задекларирован; * `run_smoke.sh` — 94 OK, регрессий нет; * `check_wheel_matches_sources.py` — зелёный. ## Что дальше, вне этого PR Консьюмерам нужен bump `rev:` — `jamzap/backend`, `jamzap/frontend`, `jamzap/mobile` и `volody/dispatcher` все зовут хук через `--from-ref`/`--to-ref` в CI, то есть защита от роста у всех четырёх сейчас фиктивна. В `.gitlab-ci.yml` фронта и мобилки лежит комментарий, прямо утверждающий обратное («grandfather для check-file-length берётся от merge-base») — его тоже надо править, иначе заблуждение поедет дальше. Тот же класс бага в неслитом [#7](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/7) (`check_max_filename_words.py`, `git cat-file -e HEAD:path`) — в CI новый файл с длинным именем всегда «уже был в HEAD». Отмечу там, чтобы не уехало в релиз. Версия 0.13.0. Номер пересекается с открытым [#13](https://git.homedevlab.ru/senokosov/pre-commit-hooks/pulls/13) — этот PR срочнее и идёт первым, #13 после мержа перенумерую в 0.14.0.
fix(file-length): grandfather от merge-base, а не от HEAD
Some checks failed
ci / smoke (push) Failing after 7s
Details
ci / smoke (pull_request) Failing after 7s
Details
44ab63f121 
В CI-инвокации (`pre-commit run --from-ref X --to-ref Y`) рабочее дерево
уже стоит на проверяемом коммите, поэтому `git show HEAD:file` возвращал
сам изменённый файл: `old` всегда равнялся `lines`, grandfather
срабатывал на всём подряд, а гейт печатал `Passed`, ничего не проверив.
Тихо не ловились обе ситуации, ради которых хук и ставили: рост
уже-большого файла и новый файл сверх порога.

Хук теперь берёт базу оттуда же, откуда pre-commit берёт список файлов:
при наличии PRE_COMMIT_FROM_REF — merge-base from/to, иначе HEAD (в
настоящем commit-хуке HEAD и есть версия до правки). Merge-base, а не
вершина from-ref: иначе split, приехавший в целевую ветку после
ветвления, засчитался бы текущему MR как рост.

Гейт fixtures/check_grandfather_baseline.sh поднимает настоящий
репозиторий и зовёт pre-commit ровно как CI консьюмеров — run_smoke.sh
такого поймать не мог в принципе, он дёргает хук напрямую списком
файлов, где HEAD и так «старая» версия. До фикса гейт валится на двух
кейсах из issue, после — проходит все восемь.

Закрывает https://gitlab.homedevlab.ru/volody/dispatcher/-/work_items/186
ci: pre-commit ставится с --break-system-packages (PEP 668 в образе)
All checks were successful
ci / smoke (pull_request) Successful in 16s
Details
ci / smoke (push) Successful in 16s
Details
9a767350d3
docs: ниты ревью — пример пинил v0.12.0, не хватало shallow и rename
All checks were successful
ci / smoke (push) Successful in 13s
Details
ci / smoke (pull_request) Successful in 12s
Details
b5c128a2d3 
Пример конфига для копипасты предлагал `rev: v0.12.0` — ровно ту
версию, в которой гейт в CI не работает; текстом выше при этом
написано «с v0.13.0».

Добавлены два умолчания, которые ревью нашло проверкой, а не чтением:
shallow-клон (merge-base не резолвится → откат на вершину from-ref, и
обещание «чужой split вам не засчитается» перестаёт действовать) и
переименование grandfathered-файла (по новому пути его в базе нет,
old = 0 → FAIL; в commit-хуке так было всегда, CI просто догнал).
Оба воспроизведены, а не выведены из кода.
claude-secretary commented 2026-08-26 22:47:35 +07:00
Author
Owner
Copy link

Ревью — результат

Статус: Пройдено.
Вердикт субагента: APPROVE_WITH_NITS
Итерация: 1
Commit на момент ревью: 9a76735

Ревьюер не ограничился чтением: прогнал гейт против старого кода (валится ровно на двух кейсах из issue), проверил каждый из восьми кейсов восемью мутациями самого хука и построил матрицу «какая мутация валит какой кейс» — greenwash-кейсов нет, каждый убивается хотя бы одной мутацией, а кейс с merge-base ловится только мутацией «merge-base → сам from-ref». Отдельно перепроверил обе моих ссылки на поведение pre-commit по его исходникам (commands/run.py, git.py) и прошёлся по восьми крайним случаям — не-git, git вне PATH, кириллица с пробелом в пути, shallow-клон, FROM_REF без TO_REF, несуществующий ref, файл без хвостового перевода строки, переименование. Режима, где фикс молча пропускает нарушение, не нашёл: все ветки деградации ведут в old=0 → FAIL.

Что поправлено по нитам (commit b5c128a)

  • Пример конфига пинил rev: v0.12.0 — ту самую версию, где гейт в CI не работает, при том что абзацем выше написано «с v0.13.0». Бампнут.
  • Shallow-клон описан в шапке скрипта, но не в README. Ревьюер воспроизвёл: --depth 1 → merge-base не резолвится → откат на вершину from_ref → невинная правка получает FAIL за чужой split. Добавлен абзац; заодно проверено, что все четыре консьюмера ставят GIT_DEPTH: "0", то есть их это не заденет.
  • Переименование grandfathered-файла теперь в CI падает (old = 0 по новому пути). Проверил сам: старый хук на git mv без правки содержимого давал Passed, новый даёт FAIL. Это восстановление паритета с commit-хуком, где rename падал всегда, но для консьюмеров при бампе rev: — новый способ покраснеть, поэтому вынесено в README отдельным пунктом «что ужесточилось».

Ниты Н3 (pre-commit==4.6.1 в CI против 4.6.0 локально — CI на нём зелёный) и Н6 (set -uo pipefail без -e — осознанно, скрипт ловит коды возврата) оставлены как есть.

Не чинится этим PR и починено быть не может

--all-files остаётся режимом без гейта: PRE_COMMIT_ALL_FILES в pre-commit не существует, отличить аудит от commit-хука по окружению нечем. Новый файл на 700 строк там по-прежнему даёт Passed. Это задокументировано в README и важно для хендовера: бампа rev: консьюмеру мало, его джоба обязана звать хук через --from-ref/--to-ref. У всех четырёх наших это так — проверено.

CI зелёный на b5c128a (run 91). Мержу.

## Ревью — результат **Статус:** Пройдено. **Вердикт субагента:** APPROVE_WITH_NITS **Итерация:** 1 **Commit на момент ревью:** `9a76735` Ревьюер не ограничился чтением: прогнал гейт против старого кода (валится ровно на двух кейсах из issue), проверил каждый из восьми кейсов **восемью мутациями** самого хука и построил матрицу «какая мутация валит какой кейс» — greenwash-кейсов нет, каждый убивается хотя бы одной мутацией, а кейс с merge-base ловится только мутацией «merge-base → сам from-ref». Отдельно перепроверил обе моих ссылки на поведение pre-commit по его исходникам (`commands/run.py`, `git.py`) и прошёлся по восьми крайним случаям — не-git, `git` вне PATH, кириллица с пробелом в пути, shallow-клон, `FROM_REF` без `TO_REF`, несуществующий ref, файл без хвостового перевода строки, переименование. Режима, где фикс молча пропускает нарушение, не нашёл: все ветки деградации ведут в `old=0` → FAIL. ### Что поправлено по нитам (commit `b5c128a`) * **Пример конфига пинил `rev: v0.12.0`** — ту самую версию, где гейт в CI не работает, при том что абзацем выше написано «с `v0.13.0`». Бампнут. * **Shallow-клон** описан в шапке скрипта, но не в README. Ревьюер воспроизвёл: `--depth 1` → merge-base не резолвится → откат на вершину `from_ref` → невинная правка получает FAIL за чужой split. Добавлен абзац; заодно проверено, что все четыре консьюмера ставят `GIT_DEPTH: "0"`, то есть их это не заденет. * **Переименование grandfathered-файла** теперь в CI падает (`old` = 0 по новому пути). Проверил сам: старый хук на `git mv` без правки содержимого давал `Passed`, новый даёт `FAIL`. Это восстановление паритета с commit-хуком, где rename падал всегда, но для консьюмеров при бампе `rev:` — новый способ покраснеть, поэтому вынесено в README отдельным пунктом «что ужесточилось». Ниты Н3 (`pre-commit==4.6.1` в CI против 4.6.0 локально — CI на нём зелёный) и Н6 (`set -uo pipefail` без `-e` — осознанно, скрипт ловит коды возврата) оставлены как есть. ### Не чинится этим PR и починено быть не может `--all-files` остаётся режимом без гейта: `PRE_COMMIT_ALL_FILES` в pre-commit не существует, отличить аудит от commit-хука по окружению нечем. Новый файл на 700 строк там по-прежнему даёт `Passed`. Это задокументировано в README и важно для хендовера: бампа `rev:` консьюмеру мало, его джоба обязана звать хук через `--from-ref`/`--to-ref`. У всех четырёх наших это так — проверено. CI зелёный на `b5c128a` ([run 91](https://git.homedevlab.ru/senokosov/pre-commit-hooks/actions/runs/91)). Мержу.
claude-secretary deleted branch fix/grandfather-from-ref 2026-08-26 22:47:39 +07:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
senokosov/pre-commit-hooks!15
No description provided.