Skip to content

PR 9 — Advanced budget hardening - #31

Merged
batbrainy merged 1 commit into
mainfrom
pr-9-advanced-budget-hardening
Jul 31, 2026
Merged

PR 9 — Advanced budget hardening#31
batbrainy merged 1 commit into
mainfrom
pr-9-advanced-budget-hardening

Conversation

@batbrainy

Copy link
Copy Markdown
Owner

Linked issue

Closes #19IMPLEMENTATION_PLAN.md §13, PR 9 — Advanced budget hardening.

Problem

The core budget system shipped in PRs 4–7 and is never cut by the descope ladder. What it
deliberately left is the advanced tier: the allowance formula's source count came from an
environment variable that nothing checked against reality, GitHub's x-ratelimit-used was
parsed and read by nobody, the ledger's row lock was asserted but never proved under
contention, and several budget conditions were enforced silently.

Reading the shipped code for those items surfaced two defects, both fixed here:

  • Allowances#clamped could strand the ledger permanently. With an observed
    x-ratelimit-limit at or below RATE_LIMIT_RESERVE, spendable is zero and
    poll_allowance became zero — so every poll is denied :class_allowance_exhausted,
    forever. Only a poll can observe a new limit, and rollover re-derives from the stored one,
    which can now never change. Reachable from any limit ≤ 8.
  • ENABLED_LIVE_SOURCE_COUNT and event_sources could disagree in silence. A second
    enabled source with the variable left at 1 means twelve poll attempts an hour split
    between two sources; the mirror case reserves 24 attempts for one source and takes 12 away
    from enrichment. Nothing said so. ADR 0004
    had already assigned the fix to this PR by name.

Scope

Included — the issue's six checklist items:

  1. Dynamic multi-source allocation validation. New Github::SourceAllocation counts the
    enabled, in-service event_sources rows of the running mode and feeds the formula, at
    window initialization and rollover only. Drift against the environment logs
    budget.source_allocation_drift.
  2. Shared-IP reconciliation edge cases. budget_ledger_shared_ip_spec.rb: a co-tenant
    draining the window and recovering only at the boundary, an observed limit changing
    mid-window, usage this application did not make, a rollover under co-tenant pressure, and
    the global block a reserve breach produces.
  3. Ledger bootstrap edge cases. budget_ledger_bootstrap_spec.rb: partial headers, no
    response at all, a window that opens with nothing left in it, a reset instant already in
    the past, and a row created under a different configuration.
  4. Stress/concurrency tests. spec/stress/budget_ledger_spec.rb — real threads on real
    sessions over the class allowance, the fairness shares and borrowing, the reserve, a
    window rolling under load, and a cold-start bootstrap race.
  5. Budget observability. GithubApiBudget#to_log, plus budget.co_tenant_usage,
    budget.co_tenant_pressure, budget.allowances_clamped, budget.window_reset_in_past,
    config.budget_resolved.
  6. Advanced configuration validation. Configuration#worst_case_reservations_per_poll
    and the config.amplification boot warning.

Deliberately not included: /status and the inspection endpoints (PR 10); crash-window,
multi-poller, container-kill and fixture-mode Docker e2e (PR 11); any schema change, since
§7 fixes github_api_budget's column list; any new dependency. IMPLEMENTATION_PLAN.md is
frozen and untouched.

Technical decisions

Recorded in full in
ADR 0009. The four
that carry a tradeoff:

Boot validation still reads no database. ADR 0004 requires it — that is what makes
db:prepare, rails runner and CI's schema load work before any table exists — so the
runtime count lives in Github::SourceAllocation and is consulted only where ADR 0004
already re-derives allowances. #bootstrap! keeps the configured arithmetic, because it
runs ahead of every reservation and the row it writes is overwritten by the first response.
The count runs inside the ledger's row lock and cannot deadlock there: it takes only
ACCESS SHARE on event_sources, which does not conflict with SourceProvisioner's
SHARE ROW EXCLUSIVE, and no session can hold an event_sources lock and then reach for
the ledger row, because assert_committable! forbids reserving inside a transaction.

The clamp floor grants one poll attempt in a window the reserve already forbids. That is
one request an hour, spent only while remaining is unknown, and it is the price of the
state being recoverable at all. Once remaining is known, remaining <= reserve denies it
on the ledger's own terms.

Co-tenant divergence is reported, never applied. GitHub's used carries no request
class, so folding it into poll_used or enrichment_used would have to guess — and a guess
breaks the invariant that the two shares sum to enrichment_used. Storing it was rejected
too: §7 fixes the column list, and an observed_used column is not on it. Consequence: "first
divergence this window" is not knowable, so the INFO line keys on the reserve threshold.

config.amplification warns rather than refusing. The existing rejection covers a
certainty — poll_allowance + reserve reaching the limit leaves no enrichment capacity every
hour whatever GitHub does. This is a worst case a healthy endpoint never reaches, §7 already
accepts that these are request-attempt allowances, and §10 requires runtime conditions to
degrade rather than crash-loop.

Rejected: keeping the stress specs in the main suite. They open genuine concurrent
PostgreSQL sessions, and spec/support/advisory_lock_helpers.rb documents the assumption
that breaks — with transactional fixtures the pool is pinned, so every thread shares one
session and both advisory locks are re-entrant. Growing the pool changes that for the whole
process, not just for the group that did it: cleaning up afterwards left the suite green at
most seeds and deadlocking at others (ExclusiveLock on the request gate against a ledger
row lock). They now run as their own rspec invocation —
spec/stress/README.md — wired into both the compose test service
and CI, so it cannot be forgotten.

Testing performed

1453 examples, 0 failures in the main suite and 10 examples, 0 failures in
spec/stress, verified at seeds 63391, 11111, 22222, 33333 and 77777. rubocop: 205 files,
no offenses.

Each new guard was verified by removing it and watching the specs fail, the standard ADR
0004 already applies to the two PostgreSQL NULL behaviours:

Guard removed Result
.lock from reserve!'s SELECT … FOR UPDATE 6 of 10 stress examples fail — over-issue past the allowance, share caps exceeded, LedgerInvariantViolation
the runtime source count (back to the configured value) both runtime source count derivation examples fail
the clamp floor the starved-limit recovery example fails with a permanent denial

New coverage: 15 bootstrap edge cases, 18 shared-IP cases, 10 stress cases, 13 allocation
cases, plus #to_log, .pollable and the amplification arithmetic.

Docker verification

docker compose run --rm test        # both rspec invocations, isolated *_test databases
GITHUB_MODE=fixture bin/ingest      # no network

The one-shot logs config.budget_resolved at boot with the resolved formula, then
budget.window_initialized on the first poll, and reports Persisted push events: 4 /
Budget remaining (core): 59 — no co-tenant line, correctly, because observed used and
the ledger's own count agree at 1.

Documentation updates

New ADR 0009; new spec/stress/README.md; README gains the two-invocation test note, the
reworded ENABLED_LIVE_SOURCE_COUNT row, the new log events and a "Sharing the outbound IP"
section; .env.example gains the amplification and allocation notes. IMPLEMENTATION_PLAN.md
is unchanged, and ADR 0004 is left as written — ADRs are append-only, and 0009 references it.

Known limitations

  • The derived count is a fact about this application's database, not about the IP.
    Another container behind the same address still spends the same sixty requests an hour,
    and no count of event_sources can see it. This narrows one failure mode; the ledger is
    still not authoritative over the window.
  • A source added mid-window changes nothing until the next rollover — up to an hour.
    ADR 0004's existing trade, applied to a new input.
  • budget.window_reset_in_past reports a condition it does not fix. Under clock skew the
    window initializes and rolls straight back to uninitialized, so enrichment stays
    ineligible for as long as the skew lasts. Refusing to initialize was considered and
    rejected: it starves enrichment identically while removing the only signal naming the
    cause. Polling is unaffected either way.
  • Divergence is derived per reconciliation rather than persisted, so it is visible in the
    log stream and not queryable.

🤖 Generated with Claude Code

Delivers the advanced tier of the request budget (IMPLEMENTATION_PLAN.md §13):
runtime source allocation, shared-IP reconciliation edge cases, ledger bootstrap
edge cases, a threaded stress suite, budget observability, and the configuration
validation the boot-time check could not see.

Two defects found while reading the shipped code are fixed here. Allowances#clamped
could derive a poll allowance of zero at an observed limit below the reserve, which
nothing recovers from — only a poll can observe a new limit, and rollover re-derives
from the stored one. And ENABLED_LIVE_SOURCE_COUNT could disagree with event_sources
silently, under- or over-provisioning the poll allowance with nothing in the logs.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@batbrainy
batbrainy merged commit 6b57add into main Jul 31, 2026
2 checks passed
@batbrainy
batbrainy deleted the pr-9-advanced-budget-hardening branch July 31, 2026 02:57
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.

PR 9 — Advanced budget hardening

1 participant