fix(tr,cacbg): 429 е изстиване, не спиране — и присъдите преживяват хода - #323
Conversation
ADR-0033 context 4 recorded the limiter as ~50 cumulative requests followed by
a sustained block, and decision 7's crawl hygiene ("a 429 is an instruction to
STOP") rests on that. Re-measured on 2026-08-19 through client.mjs's own
httpsGet — the production transport, same UA, same headers, so the result
transfers:
budget 5 requests; the 6th returns 429
scope IP, not session — a fresh EPZEUSessionID is refused too
shape 0 bytes, no Retry-After, no X-RateLimit-*, 48-67ms vs 250-475ms
for a real deed, so refused at an edge before the application
second face while blocked the connection may STALL (20s timeout), not 429
recovery still blocked at +60s, clear at +161s
CI run 32154394308 fails identically — at request 6, `TR cache: 5/537 covered`
— so the threshold is not a property of GitHub's runners.
Both halves of the old premise are false: the trip point is 5, not ~50, and the
block is not sustained. That changes what the design can achieve: 537 candidates
at 5 per ~176s cycle is ~5.3h against timeout-minutes: 300, and since the deed
cache deliberately does not survive the runner, every run restarts at `cached 0`.
No pacing value completes 537 lookups in one job.
ADR-0036 records the measurements and rules that a 429 is a cooldown, not a stop.
ADR-0037 takes the way out that ADR-0034 named and left on the table: the crawl
emits per-(link, ЕИК) verdicts — a role, an entry reference and booleans, no
third-party name — so only those cross a run boundary and progress becomes
durable.
client.mjs keeps its behaviour exactly: it still reports the 429 and never folds
it into the retry budget. Only the justification changes, and the wait-and-resume
policy lands in fetch-deeds.mjs, where "how long to wait" belongs.
No code paths change in this commit.
…n one 429 The block clears in ~161s (ADR-0036), so ending a whole crawl on the first 429 threw away the run for a wall that was about to come down on its own. In CI that meant five deeds out of 537, every month. `fetchOne` now waits RATE_LIMIT_COOLDOWN_MS (180s) and re-requests the SAME ЕИК, recording nothing about it in between — the block is a fact about us, not about that company. Three cooldowns without a success still end the run with exit 2, so a genuinely sustained block stays distinguishable from the ordinary 5-per-window rhythm, and the counter resets on any success so a slow healthy crawl never approaches it. The block has a second face: while it holds, the connection stalls rather than answering 429. Before the first cooldown we cannot know we are blocked, so a network fault keeps the documented TRIES_PER_EIK budget; once we do know, further attempts get exactly one try and any throw reads as "still blocked". Otherwise each stall costs five 20s attempts fed into a tarpit — the new test pins the difference at 4 requests rather than 16. `--max-runtime-min` lets a run stop cleanly between two ЕИК when its wall-clock budget is spent. Candidates never attempted are not counted as unresolved, but a candidate we asked about and lost still exits 1 — a deadline must not launder a real failure into a green run. This is only safe because progress is cached; the verdict cache that makes it safe across runs lands next (ADR-0037). politeTrGet is untouched. It still reports the 429 and never folds it into the retry budget; only who decides how long to wait has moved. scripts/tr: 141 tests pass, 255 across the whole scripts lane.
…roduced it The deed cannot cross a run boundary — it carries the names of co-owners and managers who hold no public office (ADR-0033 decision 5) — so every run has been restarting at `cached 0` and re-spending the register's budget on the same first few ЕИК. The decision that deed produces carries none of that: a kind, a role, an entry reference and booleans. ADR-0037; the `verdicts` table is where it lives. `verdictInputsHash` is the invalidation, and it refuses an evidenceVerdict argument it does not recognise rather than hashing a subset. The failure mode of a missed input is silent — a stale decision about a real person, published — so adding an input now fails the run until someone decides where it belongs. Arrays are sorted before hashing because declaredSeats arrives from a Set spread, and insertion order must not read as a change. A verdict is stale three ways, each of which must re-decide: the rules moved (RULES_VERSION), the declaration moved (inputs_hash), or the lookup simply aged. Coverage and pending are computed over LINKS rather than ЕИК, since one company carries several and a rules bump invalidates them independently of when its deed was last seen. The ЕГН rail screens verdict rows too — this is the row that crosses a boundary, which makes it the surface most worth screening, not least. purgeExpired ages verdicts on the same clock: not as a privacy rail here, but because a published claim must not outlive the lookup it rests on. ADR-0037 said the family-scope link key might carry the relative's name and would be hashed if so. Verified during implementation: personId is built from the DECLARANT, who is the official even for a family-scope stake, and the relative is never named. Replaced the conditional with the checked fact. scripts/tr: 150 tests pass.
…mmediately `--links-file` carries the declaration side of each decision, so the crawler can compute evidenceVerdict at the moment it holds the deed rather than handing the deed onward to a decision step that must not receive it. The raw body is deleted as soon as it has served its purpose; the end-of-job rm -rf stays as the backstop but no longer has to be the only thing standing between a failure and a deed left on disk. `--eiks-file` still works and still only fetches. The two are mutually exclusive: one names companies, the other names decisions, and quietly treating a links file as a candidate list would fetch deeds nothing ever decides. The pending set is computed over LINKS in links mode. Deed freshness alone would skip a company whose deed is fine but whose declaration moved, or whose rules version was bumped, or that gained a link since the last run — three silent ways to publish yesterday's answer. A company carrying several links is still one request. An outside-ТР answer is a decision, not an absence of one, so 404 and the empty 200 both write verdicts. Otherwise every run re-asks the register a question it has already answered — which at 5 requests per window is the expensive kind of mistake. A link whose evidence cannot be read is left WITHOUT a verdict rather than given a false one: an absent verdict re-pends the ЕИК next run, while a fabricated "unknown" would cache a hold nothing ever revisits. The test that keeps ADR-0037 honest asserts a co-owner who holds no public office appears in no column of the verdict row and nowhere in the cache file — against a verdict pinned to `document`/publishable/role=owner, so it cannot pass by degrading to a null "unknown". scripts lane: 271 tests pass.
…surface The decision pass no longer touches a deed — it cannot, since ADR-0037 has the crawler delete it the moment it has decided. evidenceVerdict and reconcileTermination are no longer imported here at all; load.mjs reads what they concluded, keyed on link_key and validated against rules_version and inputs_hash. reconcileTermination turned out to be a SECOND deed-dependent decision, and a dangerous one to lose: with no deed it falls to `terminated`, which WITHDRAWS a live link rather than holding it — a silent recall regression. It is now computed and cached beside the verdict. The coverage gate was all-or-nothing because a crawl was. Under the measured limiter (ADR-0036) that gate never opens at all, so it is scoped down to the case where refusing is the only protection there is: a first published run, where §8's monotonicity gate has no prior surface to compare against. Everywhere else the monotonicity gate takes the duty it already had — it hard-fails on a published claim that vanished, with the rules_version escape for a deliberate bump. Two gates for one duty was the redundancy. The sealed lookup date is now per link, from the verdict's own decided_at. A global MAX(fetched_at) was honest while a crawl was atomic; once it is incremental that would stamp this run's date onto a decision reached weeks ago and overstate the freshness the methodology page promises. One splitLinkRecord defines what a link record is, because the crawler and the loader hashing even slightly different objects would miss every cache entry and silently re-crawl the whole register — a five-hour bill, not an error. The unknown-input guard caught exactly that drift while wiring this up. It also caught a bug worth naming: inputsHash is a sha256 digest, and the ЕГН rail refused any digest carrying a standalone ten-digit run — ~7% of them, the same measurement that already exempts bodySha256. Unexempted it would have dropped one verdict in fourteen at random. Not a loud failure; a recall hole spread evenly across the surface. The load fixtures now run the REAL decideLinks over their deeds (tr-fixture.mjs) rather than hand-written verdict rows, so these tests still exercise the evidence ladder end to end. scripts lane: 273 tests pass. check:docs clean.
…register The verdict cache is restored and saved across runs, which is the change that makes the monthly job finishable at all: at 5 requests per window (ADR-0036) a crawl starting from `cached 0` cannot cover 537 links in one job, and until now every run started exactly there. Only scratch/tr/tr-cache.sqlite is cached — the raw deeds are never uploaded and are deleted as soon as the crawler has decided, so ADR-0033 decision 5's retention promise still covers everything it covered. Saved on always(), because a run cut short by the limiter or the runtime budget is precisely the one whose progress must survive, and also the one most likely to fail a later step. The crawl now takes --links-file and a --max-runtime-min (default 180), leaving room for resolve/audit/ship inside the 300-minute ceiling. Spending the budget is not a failure — the next run continues. The concurrency group drops the environment key. Two writes to different D1 slots cannot collide, but both crawl the SAME public register, so a staging and a production run in parallel doubled the effective request rate against it. The shared resource is the register, not the database. Also corrected three rationale comments that described the design as it was: the crawl does emit verdicts now, "decide without crawling" is a normal state rather than an impossible one, and a daily cadence is no longer blocked by the design — only by the cost to the register, which is a separate argument nobody has made. scripts lane: 273 tests pass. check:docs and prettier clean.
…e the review's gaps Review on #3 found that the machine shipped with parameters it could never converge under. Verified independently rather than taken on trust: at 5 requests per ~195s cooldown cycle a 180-minute budget buys ~277 lookups against 537 candidates, and with --max-age-days 30 on a monthly cron the whole cache expires between runs. Coverage oscillates near 50% and never reaches the 95% floor, so the pipeline would never have published at all. Worse, the queue sorted lexicographically by ЕИК while the crawl consumes from the front, so it was always the SAME ~277 — the tail never decided, then purged, then counted as a monotonicity regression. Convergence is now one condition rather than three unrelated constants: queue never-decided first, then oldest decided_at — a bounded run drains a rotating set instead of re-serving one prefix retention split from the deeds' 35 days into its own 45. The deed number is a privacy obligation (ADR-0033 decision 5); the verdict number is a freshness policy, and sharing one constant deleted verdicts faster than a budget-bounded crawl could refresh them. The cadence half lands in the workflow commit. A refused LINK no longer counts as an unreachable ЕИК. It did, which made one deterministically unparseable deed exit 1 → the workflow stop before load.mjs → the same failure every run, for ever. The old load.mjs said exactly this before the code moved; the protection did not move with it. The loader hand-rolled the verdict-currency check in two places instead of calling verdictIsCurrent. Both copies could be deleted with all 273 tests green — on the last fail-closed check before publishing a claim about a named person. One definition now, and a test that seeds an older rules_version and watches the run refuse. upsertVerdict enforces the closed vocabulary at the boundary the row actually crosses. The seal gate in load.mjs runs a month later, on a row that has already travelled between runs; the schema's promise of "a ROLE, never the person" was checked only by the eventual reader. The fixture, meanwhile, stored matchedFact: 'name' — a value production cannot emit. The verdict floor applies to every run, not only a first one. Keyed on "has anything ever been published", a single leftover row switched a 95% floor off entirely. Affordable because the loader's currency test ignores AGE deliberately: a verdict past its refresh window is still publishable, so steady state is ~100% and the two ways to fall below are the two where refusing is right. Also: a corrupt restored cache is quarantined instead of being re-saved under a newer key and preferred for ever; an empty-200 negative needs the second observation its own measurement cites, and stays pending until it gets one; the cooldown constants are pinned rather than tautologically followed; short_name and latin_in_name are reported instead of written and never read; .tmp orphans are swept; the dead verdictCoverage import is gone; and the one test that mutated shared state to make its point is replaced by a stronger one that does not. Mutation-checked: reverting the queue order, and deleting the rules_version comparison, each now fail tests that previously passed. scripts lane: 285 tests pass.
…code again The cadence is the other half of the convergence fix. Monthly, every verdict expired between runs, so one run faced all ~537 candidates and its budget bought ~277 — coverage oscillated near half and never crossed the floor. Weekly does not ask the register for more: a link comes due once its verdict passes --max-age-days either way, so the register still sees each company about monthly. Only the bunching changes — ~135 due per run against ~277 capacity, with headroom for a cold start to finish in two. That reasoning is now in ADR-0037 as an explicit convergence condition with the arithmetic, replacing the claim it made without checking: "cold start reaches full coverage in a few runs" was false under the constants it shipped with. The condition is pinned by a test rather than asserted in prose — its absence is exactly how a non-converging design got this far. Three documents asserted behaviour the code no longer has: - docs/spec/related-persons-foundation.md still recorded Phase-0's "the block is sustained… the crawler ends the run, marks nothing". Every clause of that is measured false; it now states the re-measured behaviour and points at ADR-0036, and the retention bullet reflects that deeds are no longer written to disk at all on the decision path. - ADR-0034 still said the coverage gate refuses a partial cache. It carries a status-line back-pointer and an in-place note now, and its index row says it is amended — the convention the reviewer noted was being skipped for the third time. - The workflow promised "~20 minutes" for a pass now bounded by a 180-minute wall-clock budget, and asserted "every run that decides must also crawl" a few lines above the note saying the opposite. Not adopted, and argued rather than skipped: an `Amended by:` field in the ADR template (a repo-wide convention change, so its own change), a CI mechanism binding RULES_VERSION to the ladder's content (machinery guarding a one-line discipline, and it would fire on comment edits), and a re-measurement procedure in ADR-0036 (process documentation for a hypothetical, in a record whose job is to state what was measured). scripts lane: 285 pass. pnpm test: 6/6. check:docs and prettier clean.
Code review —
|
…ng the old design Second review (cefothe, midt-bg#323) — APPROVE, security clean, three low nits. All three verified real, plus a fourth found next to one of them. purgeExpired has returned a verdict count since verdicts got their own retention window, but the log condition never checked it. A purged verdict DROPS coverage, and coverage is what the 95% floor refuses on — so the one event that best explains a surprising refusal next run was the one event that never printed. The two coverage lines in load.mjs sit next to each other and count different things: `TR cache:` was ЕИК-denominated, `TR verdicts:` link-denominated, and the refusal that follows is about links. Reading "537/537 covered" immediately above "REFUSE TO LOAD" invites exactly the wrong conclusion. Now `TR deeds:` and `TR verdicts:`. Two comments still described the design this PR replaced: cache.mjs's header said a 429 ends the run — the model ADR-0036 overturns, in a file this PR rewrites — and the purge comment still credited a monthly refresh for keeping rows inside their window. No behaviour changes beyond the added log line. scripts lane: 285 pass. check:docs and prettier clean.
|
@cefothe — благодаря за прегледа, и особено за трите самооттегляния: находка, която е била проверена и отхвърлена, спестява точно толкова време, колкото и находка, която стои. Проверих и трите бележки срещу кода, преди да ги приема, и трите са верни. Поправени в 1. Броят изтрити присъди не се логваше. Това беше най-полезната от трите и я взимам не като nit. 2. Двата реда за покритие броят различни неща. 3. Извън обхвата, но си струва да се каже: PR-ът беше в режим „чернова" ( По бележката ти за запаса от капацитет — права е и я записвам, без да я поправям тук: тестът за сходимост пази съотношението (задържане > max-age + каденца), не абсолютния таван (~277 справки на ход). Растеж на кандидатския набор би минал тихо през него. Смятам, че поправката е тест върху абсолютния таван, а не още една константа — но това е отделна промяна с отделен аргумент, не притурка към този PR. 285 теста в |
Review —
|
| # | Sev | File | Issue |
|---|---|---|---|
| 1 | Low-Med | scripts/cacbg/load.mjs:671 |
trCache opened at top level but trCache.close() runs on only one refuse path (:728). The success path (:1191 closes only db) and any exception between 671–1191 leak the handle. Benign (Node closes on process exit) but misses the resource-cleanup standard. Fix: wrap 671→1191 in try/finally { trCache.close(); }. |
| 2 | Low | scripts/tr/cache.mjs:643-645 |
SELECT COUNT(*) then DELETE outside a transaction — a concurrent insert between them mis-counts the returned number (log cosmetic only; deletion is correct). Use DELETE ... RETURNING or wrap in BEGIN IMMEDIATE. |
Minor coverage gaps (informational): openCache corrupt-file quarantine path and the idempotent ALTER TABLE migration aren't directly exercised; the decided===0 && refused===0 diagnostic branch is untested (no exit-code effect).
Neither finding blocks merge.
Third review (cefothe, midt-bg#323) — APPROVE, security clean, two low nits plus three branches listed as informational coverage gaps. Both nits verified against the code before being taken. One of the three gaps cannot honestly be tested, and now says so instead. load.mjs opened the verdict cache and closed it on one exit of three. The verdict-floor refusal closed the pair; the suppression-gate throw and the success path closed only `db`. Nothing observable followed — the workflow saves the cache BEFORE load.mjs runs, so an uncheckpointed WAL cannot reach the saved artifact — but this file closes `db` explicitly before every exit it has, and an intent kept on two paths out of three is worse than either alternative. Not the reviewer's try/finally: it would reindent ~520 untouched lines for a low nit. purgeExpired counted the verdicts it was about to delete with a SELECT COUNT(*), then deleted them. The reviewer's stated reason does not apply here — DatabaseSync is single-process and synchronous, and the workflow serialises runs against the register — but the count was already available from the DELETE. Two statements asking one question can only agree or be wrong; one statement cannot disagree with itself. Three tests for branches nothing exercised, each mutation-checked rather than merely green: openCache's quarantine of a corrupt restore, and of a file that is valid sqlite but not this cache — only the table probe catches the second, since integrity_check passes it — and the idempotent ALTER TABLE migration, whose load-bearing claim is that a restored older cache keeps its rows. That older shape is synthesised by dropping the two columns from the current schema, so the fixture cannot drift from SCHEMA when a column is next added. The third gap gets a comment, not a test. `decided === 0 && refused === 0` is unreachable: readLinksFile normalises every link.eik through safeEik, and the queue is derived from those same links, so every ЕИК reaching it has a link that matches exactly. A test would have to fabricate a state the wiring cannot produce. It is not dead code either — the queue compares safeEik(link.eik) where decideLinks compares link.eik raw — so it stays, with the reason written down. No behaviour changes. scripts lane: 288 pass; plain lane 91. check:docs and prettier clean.
|
@cefothe — и двете бележки са верни; проверих ги срещу кода, преди да ги приема. Поправени в 1. Нищо наблюдаемо не следваше от това: работният поток запазва кеша на стъпка Не с 2. Поправката обаче е правилна по по-проста причина: броят вече го връща самият Празнините в покритието. Две от трите станаха тестове, и трите са мутационно проверени, а не просто зелени:
Тестът за „валиден sqlite, но не този кеш" сваля таблицата Третата празнина получава коментар, не тест — и това е несъгласие, не пропуск. Не е и мъртъв код: клонът пази реална асиметрия — опашката сравнява Една поправка към прегледа, извън обхвата: двете предварително съществуващи повреди в лентата, които отбелязваш ( 288 теста в |
Code review — parallel multi-agent passRecommendation: APPROVE (9.5/10). No blocking issues. All CI checks green (check, test, cacbg, semgrep). Verified against the files, not just the diff. Phase 0 — security-critical scan: CLEANNo hardcoded secrets. The one "critical" flag — investigated and dismissed
Confirmed sound (highest-stakes areas)
Minor / optional (non-blocking)
Reviewed with a security-auditor + code-analyzer + workflow/docs pass in parallel; findings independently verified before posting. |
…urrent upstream Periodic heartbeat sync — brings in upstream changes since 2026-08-16 (PR midt-bg#177 head ed0eb60 was last rebased then): midt-bg#313, midt-bg#314, midt-bg#323. The integration-test lane and OTEL store polyfills do not overlap with the upstream changes (related-persons, undici bump, cacbg crawl-deadline fix, TR rate-limit remeasurement), so a clean merge is expected. PR midt-bg#177's only / touch was the original inScope commit, which is independent of upstream deps.
3 participants
Какво
Конвейерът за свързани лица не е завършвал ход в CI нито веднъж, откакто ADR-0033 влезе. Причината е премерена, не предположена — и е двойна: ограничителят на ТР се е затегнал далеч под това, което ADR-0033 записа, а дизайнът не може да напредва на части.
Closes #279 (регистърната част). Заменя контекст 4 и хигиената на решение 7 на ADR-0033, и твърдението „всеки ход, който решава, трябва и да обхожда" на ADR-0034.
Находката
Премерено на 2026-08-19 през
httpsGetнаscripts/tr/client.mjs— тоест през самия производствен транспорт:EPZEUSessionIDсъщо получава 429Retry-After, безX-RateLimit-*; 48–67 ms срещу 250–475 ms за истински акт → отказан на ръбаИзпълнение
32154394308пада идентично — на шестата заявка,TR cache: 5/537 covered. Прагът не е особеност на изпълнителя на GitHub.ADR-0033 записа „429 при ~50 натрупани заявки" и траен блок. И двете вече не са верни: прагът е 5, а блокът се чисти сам за под три минути. Оттам: 537 кандидата ÷ 5 на прозорец × ~176 s ≈ 5,3 часа при
timeout-minutes: 300— а понеже кешът не преживява изпълнителя, всеки ход тръгва отcached 0. Незавършимо при какъвто и да е темп.Поправката
fetch-deeds.mjs)TRIES_PER_EIK— иначе всяко увисване харчи пет × 20 s в увиснала връзка (тестът заковава 4 заявки вместо 16).--max-runtime-minkind, роля, отпратка към вписване и булеви — нито едно име на трето лице.concurrencyвече не е по среда — регистърът е общият ресурс, не базата.politeTrGetне се променя: докладва 429 катоRateLimitErrorи не го слага в бюджета за повторения. Мести се само кой решава колко да се чака.Проверка на живо срещу регистъра
Не опипване на прага (спец. §3.3 го забранява и ADR-0036 не разрешава повече) — а проверка, че нашият клиент се държи правилно срещу него. Цена: ~10 заявки, един умишлен блок.
scratch/tr/deeds/е празна след хода, при 8 присъди в кеша.управител(оказа се коментар в собствената ми SQL схема, вsqlite_master) иСОФИЯ(seat_normalized, разрешено по решение 3 на ADR-0010). Нула имена на лица от пет истински акта.declaredSeatsна една връзка →cached 7 · to fetch 1, поискан точно121396123.Проверка на здравия разум, че стълбата наистина е вървяла: Български пощи ЕАД →
bar_joint_stock, което е вярно.Какво това НЕ доказва: пътят
load.mjs→interest_linksот край до край (иска корпуса на ЦАИС; покрит е от тестовете) и CI обвързването (искаworkflow_dispatchсрещу staging). Това са двете дупки, които един staging ход ще затвори.Тестове
scriptsлента: 273 теста зелениpnpm test: 6/6 задачи, 487 теста в@sigma/dbpnpm check:docsи prettier чистиЗаслужава да се отбележат три неща, които работата извади:
reconcileTerminationе втора зависима от акта присъда. Без акт тя пада къмterminated— тоест сваля жива връзка, вместо да я задържи. Вече се смята и кешира до присъдата.inputs_hashе sha256 дайджест, а самостоятелен десетцифрен низ се среща в ~7% от дайджестите — същото измерване, което вече освобождаваbodySha256. Без освобождаване това би изпускало една връзка на четиринайсет, тихо. Оградата го хвана; има регресионен тест.link_keyноси само името на декларатора — служител, когото повърхността публикува по замисъл — понежеpersonIdсе строи от декларатора дори при обхватfamily. Хеширане не е нужно; ADR-0037 казва проверения факт вместо предположението.Ревю
Единствената част с експозиция е постепенният гейт в
load.mjs— той решава какво се публикува. ADR-0037 и зелениятaudit.mjsса контролите; струва си отделен поглед точно върху този хънк.