Skip to content

ci: забрани Co-Authored-By към агент, запази трейлърите с хора - #296

Merged
todorkolev merged 3 commits into
mainfrom
ci/agent-trailer-guard
Aug 9, 2026
Merged

ci: забрани Co-Authored-By към агент, запази трейлърите с хора#296
todorkolev merged 3 commits into
mainfrom
ci/agent-trailer-guard

Conversation

@todorkolev

@todorkolev todorkolev commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Излиза от наблюдение, че правилото ни е формулирано така, че спазено буквално вреди.

Какво не е наред с текста днес

AGENTS.md казва „Never include Co-Authored-By: trailers", шаблонът за PR го повтаря. Но:

  • 22 от 376 коммита на main ги носят, и ги слага GitHub при squash, не ние;
  • при squash авторът на новия коммит е винаги този, който е отворил PR-а - не този, който е писал кода. Значи трейлърът е единственото, което държи външния автор в историята. Co-authored-by: Bilko е причината @StanislavBG да е контрибутор след fix(etl): detect wrangler's SQLITE errors from stdout in safeD1 #277; същото важи за @B353N и @ydimitrof;
  • тоест проверка, която ги забранява буквално, би изтривала заслугата на хората, на които проектът стои.

Какво всъщност се има предвид

Да не кредитираме агент: Claude Code, Codex, Cursor, Copilot. Те са инструменти, които караме, не сътрудници.

Този PR го записва така и го прави проверимо.

Проверката

scripts/check-agent-trailers.mjs минава коммитите в PR-а и вали само при трейлър, сочещ агент.

Стъпка на check, не отделна работа - и това е нарочно. Правилото на main изисква един-единствен статус, check. Отделна работа щеше да показва червен кръст, който нищо не спира, докато някой с админ права не я добави в правилото. Тоест щеше да е точно това, срещу което се борим: проверка, която изглежда, че пази. Вътре в check отказът е реален от първия ден.

Изрично не маркира: хора и зависимости-ботове (dependabot, renovate) - техните трейлъри са как собствените им PR-и се приписват.

Съобщението при провал казва и как се оправя: този, който мърджва, реже редовете от squash съобщението (gh pr merge --squash --body "..."). Форкът на сътрудника не се пипа - никакъв force push.

Тестовете са срещу истинска история, не фикстури

случай източник очаквано
човешки съавтор 8db751d (Co-authored-by: Bilko) не се маркира
зависимости-бот 0d9d0ad (dependabot) не се маркира
агент PR #118, Co-authored-by: Cursor <cursoragent@cursor.com> маркира се, и Cursor се назовава

Последният дърпа реалния реф на PR-а, вместо да пише фикстура, нагласена да пасне на израза.

Живият случай

PR #118 (@mhunter02) наистина носи трейлъра на Cursor в два коммита. Нищо не съм пипал по него. Когато се мърджва, редът се маха от squash съобщението - неговите коммити остават непокътнати.

Пътьом

post-create.sh предупреждава, когато user.email е празен или изглежда като запълнител (t@e.com, ...MacBook-Pro.local, localhost). И двете форми вече са влизали в публичната история по същия път - самоличност по подразбиране в контейнера става Co-authored-by ред при squash. Скриптът само предупреждава; самоличността е на човека, не на скрипта.

scripts/: 93 теста минават.

Правилото в AGENTS.md се четеше като пълна забрана на `Co-Authored-By:`, а
буквалното му спазване би изтрило заслугата на сътрудниците: при squash GitHub
съставя трейлърите от авторите на коммитите в PR-а и те са единственото, което
държи външния автор в историята - авторът на самия squash коммит винаги е този,
който е отворил PR-а. 22 от 376 коммита на main ги носят, включително тези,
които пазят заслугата на StanislavBG, Румен и Йоан.

Забраненото е друго: трейлър, който кредитира агент - Claude Code, Codex,
Cursor, Copilot. Те са инструменти, не сътрудници. Проверката ги лови на ниво
PR и казва как се оправя, без да праща никого да пренаписва чужд форк.

Тестовете карат проверката срещу истинска история: човешки съавтор и
dependabot не се маркират, а трейлърът на Cursor в PR #118 се маркира.

Пътьом: post-create.sh предупреждава при самоличност като `t@e.com` или
`...MacBook-Pro.local`. И двете вече са влизали в публичната история точно по
този път.
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Test coverage

Workspace Lines Δ Branches Δ Functions Statements
apps/etl 75.43% +1.43pp 63.52% +5.32pp 70.00% 74.11%
apps/web 91.02% +1.32pp 82.50% +0.70pp 91.19% 89.73%
packages/config 92.85% +0.05pp 72.22% +0.02pp 92.85% 89.18%
packages/db 94.54% +0.34pp 79.38% +0.38pp 87.19% 91.55%
packages/ingest 88.17% +2.37pp 82.30% +2.30pp 80.17% 86.36%
packages/shared 95.50% +0.10pp 80.83% +0.83pp 92.30% 89.56%
Total (informational) 91.23% 80.72% 86.95% 89.08%

✅ No workspace dropped below its baseline (tolerance 0.5pp).

📈 Coverage rose by more than 1pp — run node scripts/check-coverage.mjs --update locally and commit coverage-baseline.json to ratchet the threshold up.

Отделната работа се вижда в списъка, но правилото на main изисква само `check`.
Значи червен кръст, който нищо не спира - точно класът „проверка, която
изглежда, че пази". Стъпката вече е вътре в `check`, тъй че отказът е реален
и не зависи от админска промяна по правилото.
@todorkolev
todorkolev merged commit 73e055d into main Aug 9, 2026
5 checks passed
lyubomir-bozhinov added a commit to lyubomir-bozhinov/sigma that referenced this pull request Aug 18, 2026
…ve, поименни връзки midt-bg#309)

Third sync onto upstream/main (20 commits). Resolved four conflicts by union, keeping
both sides' tests in every case:

- apps/etl/src/eop.test.ts — our orchestration suite plus upstream's stream-cancel
  tests (midt-bg#284); the openBodyResponse helper moves under our import block.
- apps/web/app/lib/conflicts.test.ts — our temporalLabel/timeline/authorityShares
  cases alongside upstream's registryEvidenceLabel wording tests (midt-bg#309).
- packages/ingest/src/ocds.test.ts — union of the daysInWindow and fullDeriveIsSafe
  imports.
- coverage-baseline.json — ours; every floor is strictly higher than upstream's.

No coverage floor was lowered. Two upstream changes moved coverage and were met with
tests rather than a looser baseline:

- refresh.test.ts asserts the drop order, which now carries the derive-step scratch
  table amendment_contract_resolve (midt-bg#306) as its last entry.
- packages/db branch coverage fell below its floor on the new poименни-връзки and
  contract-detail code. Restored to 98.47% (floor 98.3) and lines to 100% with tests
  for: registry_role narrowing to the two rungs the card can render, the ordering-unit
  vs authority fold, personSlug on a prefix-less key, getDb as the read-only
  chokepoint, and assertReadOnlyExec's empty-statement refusal.

Also fixes a stale fixture in search.test.ts: the свързани-лица probe returned n:1
while the source requires n===2, so the "table present" mode silently exercised the
un-migrated fallback and the conflict-aware hits SQL was never run. The fixture now
reports the real count, with tests for the partial-migration and no-row cases and for
a group whose count and hits disagree.

apps/web regained its floor with tests for the conflict pages' cache headers and the
methodology page's indexability, the confirmed-seal provenance line, a timeline with
no declared-period band, and Pagination's disabled end-of-range state.

Every new test was mutation-verified: the production line it covers was broken and the
test confirmed failing.

HITL-ACK: agent-instructions-self-mod AGENTS.md arrives from upstream (midt-bg#296/midt-bg#297, the
Co-Authored-By trailer rule), not authored here — a merge cannot omit it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants