From 363a358934733fc778230ee3a43e813c604e3132 Mon Sep 17 00:00:00 2001 From: Umang Date: Fri, 31 Jul 2026 11:06:49 -0500 Subject: [PATCH 1/3] =?UTF-8?q?PR=2012=20=E2=80=94=20Reviewer=20documentat?= =?UTF-8?q?ion=20and=20final=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the documentation deliverables of IMPLEMENTATION_PLAN.md §13-§16 and closes the three §4 documentation bullets: S2.7 (data model), S4.9 (expected logs and verification steps), and B10 (processing guarantees). New: - docs/DESIGN_BRIEF.md — the reviewer's primary architecture document, two pages, with mermaid request-path and data-model figures. - docs/SUBMISSION_CHECKLIST.md — §16's four gate lists as checkboxes, the forbidden-claim scan, and the post-merge email template. - docs/adr/0011, 0012 — the two §14 ADR topics that no implementation PR owned: the pinned API version 2022-11-28, and Solid Queue over Kafka. README restructured to a 20-heading spine with a table of contents. Mostly moves: nine existing blocks relocate into new containers (Rate limits, Inspecting the data, Troubleshooting and reset, Logs). Genuinely new are the data model, sample log lines, processing guarantees, known limitations, the §15 eleven-step verification walk, the SSRF boundary, the reset ladder, and a symptom table. Deleted the stale PR-by-PR Status narrative and the "Planned contents" placeholder. Sample log lines are captured verbatim from a fixture-mode run against an empty database rather than hand-authored — the payloads are splat-merged across ~20 to_log methods, and a wrong sample in the section whose purpose is trustworthiness is exactly the stale documentation §16 forbids. IMPLEMENTATION_PLAN.md gains Appendix E only; §1-§17 and Appendices A-D are byte-identical. It records the seven divergences the build produced, notably that §15 step 8's own `docker kill` cannot exercise `restart: unless-stopped`. Non-documentation changes, all §16-driven: - CI foundation job now asserts docs/DESIGN_BRIEF.md exists. - spec/requests/health_spec.rb asserts the endpoints spend no budget — the one §16 gate that was structurally true but untested. - Deleted config/initializers/cors.rb, commented-out scaffolding for a gem the plan excludes ("no dead or speculative infrastructure"). Closes #22 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 6 + CLAUDE.md | 5 +- IMPLEMENTATION_PLAN.md | 22 + README.md | 1219 +++++++++++++---- config/initializers/cors.rb | 16 - docs/DESIGN_BRIEF.md | 303 ++++ docs/SUBMISSION_CHECKLIST.md | 204 +++ .../adr/0011-pinned-api-version-2022-11-28.md | 84 ++ docs/adr/0012-solid-queue-over-kafka.md | 92 ++ spec/requests/health_spec.rb | 36 + 10 files changed, 1686 insertions(+), 301 deletions(-) delete mode 100644 config/initializers/cors.rb create mode 100644 docs/DESIGN_BRIEF.md create mode 100644 docs/SUBMISSION_CHECKLIST.md create mode 100644 docs/adr/0011-pinned-api-version-2022-11-28.md create mode 100644 docs/adr/0012-solid-queue-over-kafka.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4d436c1..349f201 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,12 @@ jobs: test -f CLAUDE.md test -f .github/PULL_REQUEST_TEMPLATE.md test -f .github/workflows/ci.yml + # §14 names the design brief as the reviewer's primary architecture + # document, and CLAUDE.md tells every contributor to read it. Guarded + # here for the same reason as the plan and the README: a required + # document that can go missing without a red build will eventually go + # missing. + test -f docs/DESIGN_BRIEF.md # The real suite runs from PR 3 onward (IMPLEMENTATION_PLAN.md §13). Ruby is pinned # from .ruby-version — the same pin the Dockerfile uses — so CI and the image can diff --git a/CLAUDE.md b/CLAUDE.md index 50835ea..d16892d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,8 +7,9 @@ plan wins. ## Before making any change 1. Read `IMPLEMENTATION_PLAN.md` (repository root). It is the frozen execution - plan; its revision history lives in Git and in its Appendices A–D. -2. Read `docs/DESIGN_BRIEF.md` and the ADRs under `docs/adr/` once they exist. + plan; its pre-implementation revision history lives in Git and in its + Appendices A–D, and Appendix E records how the build diverged from it. +2. Read `docs/DESIGN_BRIEF.md` and the ADRs under `docs/adr/`. 3. Do not change architectural direction, add infrastructure, or add dependencies without first updating the plan and stating the tradeoff. diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index f2bc3a2..b2f77ed 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -1301,3 +1301,25 @@ A fourth review round validated the external facts (GitHub, Rails, Ruby, Solid Q | 6 | **SHA columns widened to `varchar(64)`** accepting 40- or 64-char hex | Git object names are 40 hex (SHA-1) or 64 hex (SHA-256); hard-coding 40 contradicted the tolerant-parser goal | Section 7 | | 7 | **Quarantine identity made unambiguous**: `payload_fingerprint` is the sole unique key (`github_event_id` indexed, not unique); one canonicalization definition (SHA-256 of compact UTF-8 JSON with recursively sorted keys) | Dual unique keys left an unhandled conflict path (same event ID, different malformed payload); “or equivalently normalized `jsonb`” specified two algorithms | Section 7 | | 8 | Precision edits: `ingest` depends on `setup`, `test` depends only on `db` and self-prepares; **Ruby switched to 3.4.10** (3.3 is security-maintenance-only — weaker greenfield signal; 3.4.10 verified current, released 2026-06-30); “Rails 8 bundles Solid Queue” reworded to “default Active Job backend in new Rails 8 applications”; `X-RateLimit-Resource` added to processed headers with a `core` verification; fairness rounding defined (floor/remainder); borrowing requires no *currently eligible* candidate; operational defaults pinned (HTTP timeouts, retries, redirects, lock wait); `/status` coverage formulas defined | Accuracy and reviewer experience | Sections 2A, 10, 11 | + +## Appendix E — Execution summary (2026-07-31) + +**The plan body above is unchanged.** Sections 1–17 and Appendices A–D are exactly as frozen on 2026-07-29. This appendix records how the build diverged from that plan and why, as Section 14 requires at completion. + +The plan held. Every P0 story and extension shipped, no descope rung was used, and the architecture — the executor chain, the lock-order invariant, the class-aware ledger, at-least-once with idempotent writes — is what was frozen. What follows is the delta, and most of it is the plan meeting a fact it could not have known in advance. + +| What the plan said | What was built | Why | Record | +|---|---|---|---| +| Section 15 step 8 verifies restart policies with `docker kill` | `script/verify_recovery.sh` performs **both** the documented `docker kill` and a real in-container process crash, and reports both outcomes separately | `docker kill` is an API stop, and `restart: unless-stopped` is *defined* to skip a container the daemon recorded as manually stopped. The plan's own command cannot exercise the policy it verifies. Substituting the kill that works and staying quiet about it would have been the dishonest fix | [`docs/evidence/2026-07-31-container-kill-recovery.md`](docs/evidence/2026-07-31-container-kill-recovery.md), README “Crash recovery, verified” | +| `ENABLED_LIVE_SOURCE_COUNT` is the allowance formula's source-count input | Demoted to a **fallback**. The formula counts enabled, in-service `event_sources` rows of the running mode at window initialization and rollover, and logs `budget.source_allocation_drift` when the two disagree | A configured count that drifts from the table silently mis-sizes every allowance. Boot validation still reads no database, so the refuse-to-boot check is unchanged | ADR 0009 | +| Secondary-limit backoff is “≥ 1 minute with exponential backoff when `Retry-After` is absent” | A persisted `github_api_budget.consecutive_secondary_limits` counter escalates 60 → 120 → 240s capped at one hour, **survives window rollover**, and is cleared by one clean response | Secondary limits are IP-scoped, not window-scoped. A counter that reset with the window would restart the ladder at 60s every hour against a limit that had not relented | ADR 0010 | +| Section 10's fairness ladder covers the pending pool | The **TTL-refresh pool** allocates by the same prefer-then-borrow steps as the pending pool | The ladder was specified for pending candidates only, which left the refresh pool able to starve one class. Found by reading merged code against the plan rather than by a failing test | ADR 0010 | +| PR ladder order in Section 13 | PR 10 merged before PR 9 (`5454dda` before `6b57add`), and an “Extension A completion” commit (`687fdc3`) shipped work with no slot in the ladder | The ladder's *dependency* order held — nothing spent budget before the gate and ledger landed in PR 4. The merge order did not, and the Extension A completion closed two items the ladder had assumed were finished | Git history | +| Section 14 names eight ADR topics | Twelve ADRs shipped | The ledger topic split across three records (0004 allowance formula, 0007 fairness and borrowing, 0010 secondary-limit escalation) because they are separately contestable decisions. Two more (0006 decomposed poll state, 0009 runtime source allocation) were decisions the plan did not anticipate needing | [`docs/adr/`](docs/adr/) | +| ADRs are written alongside the code they describe | ADR 0011 (pinned API version) and ADR 0012 (Solid Queue over Kafka) were written in PR 12 | Both are Section 14 deliverables that no implementation PR owned, because neither records a decision made *during* a PR — they record decisions made before PR 1 and never written down. PR 12 is the last opportunity, and Section 16 forbids a plan that points at documents which do not exist | ADR 0011, ADR 0012 | +| The plan is the reviewer's architecture document until the brief exists | [`docs/DESIGN_BRIEF.md`](docs/DESIGN_BRIEF.md) is the reviewer's entry point; this plan is the internal execution and traceability artifact | As Section 14 intended. Stated here because the README's pointer changed with it | README “Development” | + +Two things worth stating that are not divergences: + +- **No forbidden claim was ever written.** The scan in [`docs/SUBMISSION_CHECKLIST.md`](docs/SUBMISSION_CHECKLIST.md) §7 returns only negations, in every document, at every revision. +- **The 304 finding survived first-party re-verification.** PR 6's required gate re-ran the probe under `X-GitHub-Api-Version: 2022-11-28` and committed a dated transcript; `x-ratelimit-used` incremented across an unauthenticated `304`, exactly as the review-supplied evidence in Appendix A had reported. The budget arithmetic that rests on it did not have to change. diff --git a/README.md b/README.md index 1b0d408..ff5a168 100644 --- a/README.md +++ b/README.md @@ -8,50 +8,65 @@ structured data in PostgreSQL, enriches actors and repositories within an explicit unauthenticated request budget, and recovers cleanly from application and worker crashes. -## Status - -Background processing and recovery stage (PR 8). The Rails 8.1 API, PostgreSQL, Docker -Compose topology, health endpoints, and structured JSON logging landed in PR 2; the -seven core tables and their idempotent write paths in PR 3; the chain every GitHub -request flows through — request gate, class-aware budget ledger, SSRF URL policy, -live and offline transports, fixture corpus, per-source advisory lock, event-source -adapter — in PR 4; the processor registry, the tolerant `PushEvent` parser, the -quarantine taxonomy, the ingest transaction, and the one-shot command in PR 5; -`Link`-header pagination, the page-one ETag, the five independent components behind -`effective_poll_time`, and global-versus-class blocking in PR 6; the entity state -machine, per-class fairness with eligibility-aware borrowing, the freshness cache and -its refresh TTLs, `skipped_budget` with distinct-event reactivation, and -`effective_enrichment_time` in PR 7. - -**This stage makes the system run by itself, and proves it recovers.** Solid Queue -now runs in its own `queue` database inside the same PostgreSQL container, a `worker` -container runs its supervisor, and two recurring tasks fire every 60 seconds: -`PollEventSourceJob`, which polls each source that §9's schedule says is due, and -`ReconcilePendingEnrichmentsJob`, which sweeps the committed entity rows for -enrichment work and schedules a cycle per class. A completed poll that created events -schedules enrichment immediately, after commit. - -**Enrichment is bounded best-effort sampling, and says so.** Against ~2,172 cold -entity requests an hour of demand and 40 available, partial coverage is the design -rather than a shortfall — `skipped_budget` is a normal documented outcome, and -`bin/enrich` prints the per-class usage so the sampling rate is visible instead of a -mysteriously growing queue (plan §10). +## Contents + +- [What this is](#what-this-is) +- [Requirements](#requirements) +- [Quick start](#quick-start) +- [How to verify it's working](#how-to-verify-its-working) +- [One-shot ingestion](#one-shot-ingestion) +- [Deterministic fixture verification](#deterministic-fixture-verification) +- [Inspecting the data](#inspecting-the-data) +- [The data model](#the-data-model) +- [Logs](#logs) +- [Rate limits and the request budget](#rate-limits-and-the-request-budget) +- [Configuration](#configuration) +- [Architecture](#architecture) +- [Continuous ingestion](#continuous-ingestion) +- [Processing guarantees](#processing-guarantees) +- [Crash recovery, verified](#crash-recovery-verified) +- [Troubleshooting and reset](#troubleshooting-and-reset) +- [Known limitations](#known-limitations) +- [Development](#development) +- [License](#license) + +## What this is + +GitHub publishes a firehose of public activity at `/events`. This service ingests the +`PushEvent` slice of it durably, enriches the actors and repositories those events +reference, and survives restarts — **without a token**, inside an unauthenticated ceiling +of 60 requests per hour keyed to the outbound IP. + +What it does, running: + +- **Polls** one event source on a cadence, following `Link`-header pagination, and + processes every fetched page in full. +- **Persists** each `PushEvent` with typed columns and its raw `jsonb` payload, quarantining + malformed envelopes durably instead of dropping them or failing the batch. +- **Enriches** actors and repositories through the same budget-governed request chain, + under per-class fairness guarantees. +- **Recovers** on its own: advisory locks die with their session, entity leases expire by + arithmetic, and a 60-second reconciler rebuilds pending work from committed rows. + +**Enrichment is bounded best-effort sampling, and says so.** Against roughly 2,172 cold +entity requests an hour of demand and 40 available, partial coverage is the design rather +than a shortfall — `skipped_budget` is a normal documented outcome, and `/status` publishes +the real sampling rate. See [Known limitations](#known-limitations). **With the default `GITHUB_MODE=live`, `docker compose up` starts spending real unauthenticated quota** — twelve poll requests an hour at the default cadence, plus enrichment inside its allowance. That is the intended runtime behaviour (plan §2A); `GITHUB_MODE=fixture docker compose up --build` runs the same flow entirely offline. -The processing guarantee is unchanged and stated exactly: at-least-once execution -plus idempotent writes plus unique constraints gives **effectively-once persisted -outcomes**. This system does not claim exactly-once execution — a job may run twice, -and the second run changes nothing. +The processing guarantee is stated exactly: at-least-once execution plus idempotent writes +plus unique constraints gives **effectively-once persisted outcomes**. This system does not +claim exactly-once execution. See [Processing guarantees](#processing-guarantees). -Ingestion capabilities land PR by PR; each README section below is completed by -the pull request that ships the capability it documents. The authoritative -execution plan is [`IMPLEMENTATION_PLAN.md`](IMPLEMENTATION_PLAN.md) — its -pre-implementation revision history lives in Git and in its Appendices A–D. -Delivery is tracked on the +[`docs/DESIGN_BRIEF.md`](docs/DESIGN_BRIEF.md) is the two-page architecture summary and the +best place to start. The authoritative execution plan is +[`IMPLEMENTATION_PLAN.md`](IMPLEMENTATION_PLAN.md) — its pre-implementation revision history +lives in Git and in its Appendices A–D, and Appendix E records how the build diverged from +it. Delivery is tracked on the [GitHub Push Ingestor Delivery project](https://github.com/users/batbrainy/projects/1). ## Requirements @@ -59,7 +74,7 @@ Delivery is tracked on the - Docker with Compose v2 (`docker compose`). Nothing else is needed on the host — Ruby 3.4.10 and all dependencies live in the image. -## Quick start (clean checkout) +## Quick start ```bash docker compose up --build @@ -76,6 +91,9 @@ This starts, in dependency order (plan §2A): **Continuous polling begins here**: its scheduler fires the 60-second tick, so the first poll happens within a minute and every 300 seconds after that. +Those four are the whole stack. `ingest`, `enrich`, and `test` sit behind a compose +profile, so a plain `up` never starts them. + Verify it is healthy: ```bash @@ -83,75 +101,14 @@ curl http://localhost:3000/health/live # {"status":"ok"} — process is up curl http://localhost:3000/health/ready # {"status":"ok"} — database reachable, schema current ``` -See what it has actually done, and inspect what it captured: - -```bash -curl -s http://localhost:3000/status | jq -curl -s 'http://localhost:3000/api/push_events?limit=5' | jq -curl -s http://localhost:3000/api/push_events/ | jq .data.raw_payload -``` - **No endpoint on this surface ever calls GitHub or consumes request budget** (plan §11). That is structural rather than a promise: none of these controllers holds an executor or a transport, and `Github::BudgetLedger` is absent from all of them — every one of its public methods writes, and `#bootstrap!` would create from a read -path the very ledger row a reservation owns. Four specs pin it, including one that -subscribes to every SQL statement a request issues and fails on any `INSERT`, +path the very ledger row a reservation owns. Specs pin it, including ones that +subscribe to every SQL statement a request issues and fail on any `INSERT`, `UPDATE` or `DELETE`. -#### `GET /status` - -Reports persisted state only: the poll schedule per event source (all five of §9's -components, plus which one is binding), the per-class ledger state, §11's three -enrichment coverage percentages, and the per-status entity counts. - -Three conventions in the response body are worth knowing before reading one: - -- **`null` is never a zero.** A counted zero prints `0`; a number that does not - exist prints `null`. Where `null` would be ambiguous the disambiguating fact gets - its own field — `ledger.present` separates "no ledger row yet" from "remaining is - genuinely 0", `due_now` separates "no constraint applies" from "unknown", and - `claimable_now` separates "work is claimable this second" from "nothing is - waiting". An empty coverage window reports `null` percentages, not `0.0`: the - ratio is undefined, not zero, and every denominator is published beside its ratio - so you can check it. -- **The coverage window is measured on `created_at`** — when *this application* - persisted the event, not GitHub's `occurred_at`. Coverage grades this - application's enrichment pipeline, and that pipeline's own eligibility and - freshness rules already run on this clock. The basis is published as - `coverage.basis` so the choice is visible rather than assumed. Widen - `ENRICHMENT_COVERAGE_WINDOW_SECONDS` when reviewing a fixture corpus that has aged. -- **`actor_requests.available` is a floor, not a ceiling.** §10 lets one class - borrow the other's unspent capacity when the other has no eligible candidate, so a - class does not stop at zero available. The real ceiling is the `enrichment` pair - beside it. - -`pending` in the entity counts means `enrichment_status = 'pending'` exactly. The -`candidates` figure beside it is pending **plus** `retryable_failure` — the "how much -work is left" number `bin/ingest` prints. Two questions, two names, deliberately. - -#### `GET /api/push_events` and `GET /api/push_events/:id` - -`:id` is the **GitHub event id**, not the surrogate primary key — the identifier §11 -puts on every log line, so a log line is a URL you can type. The surrogate key -appears in no response. - -Paging is keyset on `(occurred_at, id)` with an opaque cursor, not `offset`: the -poller writes continuously and this list is newest-first, so an offset page 2 would -re-serve rows from page 1 and skip others whenever a poll landed in between. Follow -`pagination.next_cursor`, or the RFC-8288 `Link: …; rel="next"` header. `limit` -defaults to 25 and is capped at 100 — a value outside that range is a `400`, never a -silent clamp, because a client that asked for 500 and received 100 cannot tell -whether it received everything. `actor_id` and `repository_id` filter; an unknown -parameter is a `400` rather than a silently unfiltered answer. - -The list omits `raw_payload` — it is a multi-kilobyte TOASTed `jsonb` column nobody -scans a list for. `GET /api/push_events/:id` returns it, which is how §16's "raw -payload is retained" gate is checkable without a psql session. Every row nests its -actor and repository with their `enrichment_status`, so §16's enrichment gate is -visible in a browser as statuses flip from `pending` to `complete` while the worker -runs. - Run the test suite (isolated `*_test` databases; never touches the development databases): @@ -178,6 +135,151 @@ Stop everything (add `-v` to also drop the database volume): docker compose down ``` +## How to verify it's working + +The four commands the assignment asks for: + +```bash +docker compose up --build +docker compose run --rm ingest +docker compose run --rm test +docker compose logs -f +``` + +`IMPLEMENTATION_PLAN.md` §15 asks a reviewer to walk eleven steps. Each one below is the +command plus the result to look for; the linked section carries the explanation. + +**1. Start all services.** + +```bash +docker compose up --build # live mode — spends real quota +GITHUB_MODE=fixture docker compose up --build # the same flow, zero network +``` + +Exactly four containers come up: `db`, `setup`, `web`, `worker`. See +[Quick start](#quick-start). + +**2. Wait for health checks.** + +```bash +until curl -fsS http://localhost:3000/health/ready; do sleep 2; done +``` + +`{"status":"ok"}`. `/health/live` reports the process; `/health/ready` reports the database +and schema. Neither consumes budget. + +**3. Run one-shot ingestion.** + +```bash +docker compose run --rm ingest +``` + +Either a completed-run summary or a deferral — `Ingestion deferred until … — cadence_due_at` +or `source busy — poller cycle in progress`. **A deferred or busy result is a valid result**: +the state summary prints on every path and the exit code is 0, so the command always proves +system state rather than just its own outcome. See [One-shot ingestion](#one-shot-ingestion). + +**4. Follow application and worker logs.** + +```bash +docker compose logs -f +docker compose logs -f worker +``` + +One structured JSON stream. See [Logs](#logs). + +**5. Query persisted push events.** + +```bash +curl -s 'http://localhost:3000/api/push_events?limit=5' | jq +curl -s http://localhost:3000/api/push_events/ | jq .data.raw_payload +``` + +The list nests each event's actor and repository with their `enrichment_status`; the +detail endpoint returns the retained raw payload. See +[Inspecting the data](#inspecting-the-data). + +**6. Inspect PostgreSQL record counts.** + +```bash +docker compose exec db psql -U postgres -d github_push_ingestor_development -c " + SELECT (SELECT COUNT(*) FROM push_events) AS push_events, + (SELECT COUNT(*) FROM github_actors) AS actors, + (SELECT COUNT(*) FROM github_repositories) AS repositories, + (SELECT COUNT(*) FROM quarantined_events) AS quarantined, + (SELECT SUM(occurrence_count) FROM quarantined_events) AS occurrences;" +``` + +More queries in [Database inspection](#database-inspection); what each table means is in +[The data model](#the-data-model). + +**7. Run the fixture replay and confirm duplicates are absorbed.** + +```bash +GITHUB_MODE=fixture docker compose run --rm ingest +sleep 60 # GitHub's X-Poll-Interval floor, which --force deliberately does not bypass +GITHUB_MODE=fixture docker compose run --rm ingest --force +``` + +Two things to confirm, not one. `Duplicates skipped: 4` in the second summary — and that +**no skipped entity was reactivated**. The second is falsifiable: `enrichment.reactivated` +is emitted only on a real reactivation, so it should be absent, and the `skipped_budget` +count should be unchanged. + +```bash +docker compose logs ingest | grep enrichment.reactivated # expect no output +docker compose exec db psql -U postgres -d github_push_ingestor_development -c " + SELECT COUNT(*) FROM github_actors WHERE enrichment_status = 'skipped_budget';" +``` + +See [Deterministic fixture verification](#deterministic-fixture-verification). + +**8. Verify crash recovery with actual container kills.** + +```bash +GITHUB_MODE=fixture docker compose up --build -d +script/verify_recovery.sh --confirm +``` + +Read §15's literal `docker kill` command knowing what it does: it is an API stop, and +`restart: unless-stopped` is *defined* to skip exactly that case, so the documented command +leaves the container down. The script performs both that and a real process crash, and +reports both. See [Crash recovery, verified](#crash-recovery-verified). + +**9. Restart normally and confirm records remain.** + +```bash +docker compose exec db psql -U postgres -d github_push_ingestor_development \ + -c "SELECT COUNT(*) FROM push_events;" +docker compose restart +docker compose exec db psql -U postgres -d github_push_ingestor_development \ + -c "SELECT COUNT(*) FROM push_events;" +``` + +Identical counts. The volume is named; only `down -v` removes it. + +**10. Run the full deterministic fixture scenario.** + +```bash +GITHUB_MODE=fixture docker compose run --rm ingest +GITHUB_MODE=fixture docker compose run --rm enrich --limit 6 +``` + +From an empty database: **4 push events created, 3 quarantined, 1 ignored, 3 actors, 3 +repositories** — then `complete 2 / permanent_failure 1` in each entity class. Eleven more +scenarios are in the [fixture scenario matrix](#fixture-scenario-matrix). + +**11. Run tests.** + +```bash +docker compose run --rm test +``` + +Two `rspec` invocations against isolated `*_test` databases, the same two CI runs. + +If something looks wrong, [Troubleshooting and reset](#troubleshooting-and-reset) has a +symptom table and a three-level reset ladder. + ## One-shot ingestion ```bash @@ -246,7 +348,7 @@ window prints `Ingestion deferred until … — cadence_due_at` and makes no req all. With `--force`, it polls — and spends one of the twelve hourly poll attempts to do it. -### Deterministic verification +## Deterministic fixture verification Fixture mode resolves every request inside [`fixtures/github/`](fixtures/github/) with no network at all, so the numbers are exact: @@ -371,78 +473,375 @@ At `MAX_PAGES_PER_POLL=2` the counts are identical except `Pages fetched: 2` — 3 is empty — and the stop reason on the `ingestion.pagination_stopped` debug line changes from `empty_page` to `page_cap`. -### Why a `304` costs a request here +### Fixture scenario matrix -GitHub's events documentation states generally that `304` responses do not count -against the rate limit; its REST best-practices documentation scopes that exemption -to requests "correctly authorized with an `Authorization` header". This service -sends no token, so the two statements disagree about exactly the population of -requests it makes. A dated unauthenticated probe run under -`X-GitHub-Api-Version: 2022-11-28` settles it for this configuration: -`x-ratelimit-used` increments across a `304`. The transcript, with complete -before-and-after headers and its own stated limits, is -[`docs/evidence/2026-07-30-unauthenticated-304-quota-probe.md`](docs/evidence/2026-07-30-unauthenticated-304-quota-probe.md). +Every scenario resolves inside [`fixtures/github/`](fixtures/github/) with no network at all. +Each poll obeys GitHub's `X-Poll-Interval` floor of 60s, which `--force` deliberately does not +bypass, so allow a minute between successive ingestion scenarios. -So the ledger debits every outbound attempt, `304`s included, and the stored ETag -is a bandwidth and correctness measure rather than a quota saver. The asymmetry -justifies the choice: budgeting a `304` that turns out to be free wastes one -attempt, while not budgeting one that is in fact charged overruns a sixty-request -hour. The transcript claims nothing about authenticated requests — this project has -no token and cannot test that case. +| Scenario | Command | What it shows | Cleanup | +|---|---|---|---| +| `default` | `GITHUB_MODE=fixture docker compose run --rm ingest` | 4 push events, 3 actors, 3 repositories, 3 quarantined | none | +| `default` (replay) | `… run --rm ingest --force` after 60s | `duplicates_skipped > 0`, occurrence counts climb, no skipped entity reactivated | none | +| `default` (enrich) | `… run --rm enrich --limit 6` | both classes enrich within their fairness shares | none | +| `paginated` | `GITHUB_FIXTURE_SCENARIO=paginated MAX_PAGES_PER_POLL=3 … ingest` | `Link`-driven walk over 3 pages | none | +| `paginated_final_page` | `GITHUB_FIXTURE_SCENARIO=paginated_final_page … ingest` | the walk stops when no `next` link exists | none | +| `transient_failure` | `GITHUB_FIXTURE_SCENARIO=transient_failure … ingest` | one `500`, then success on retry | none | +| `transient_failure_exhausted` | `GITHUB_FIXTURE_SCENARIO=transient_failure_exhausted … ingest` | retries exhausted; the source backs off | source backoff | +| `redirecting_repository` | `GITHUB_FIXTURE_SCENARIO=redirecting_repository … enrich --class repository` | a renamed repository followed across one validated hop, debited twice | none | +| `hostile_redirect` | `GITHUB_FIXTURE_SCENARIO=hostile_redirect … enrich --class repository` | an off-host `Location` refused by the URL policy; the second hop is never sent and the event source stays in service | none | +| `secondary_rate_limited` | `GITHUB_FIXTURE_SCENARIO=secondary_rate_limited … ingest` | a secondary limit blocks globally | global block | +| `rate_limited` | `GITHUB_FIXTURE_SCENARIO=rate_limited … ingest` | primary exhaustion → `global_blocked_until` | **a real one-hour global block** | -### Expected time before records appear +The last two leave durable state behind on purpose — that is the behaviour being demonstrated. +`script/verify_recovery.sh --phase=cleanup` runs the SQL under +[Recovering from a fixture rate-limit run](#recovering-from-a-fixture-rate-limit-run), and +`--phase=rate-limit` plays the rate-limit scenario and cleans up immediately after. -A push reaches the public feed with a documented latency of 30 seconds to 6 hours, -and the default cadence polls every 5 minutes, so a given push may take hours to -appear — or never, if it left the feed's 300-event window before a poll reached it. -The feed retains 30 days. At `MAX_PAGES_PER_POLL=1` each poll sees at most the -newest ~100 events, and the feed moves considerably faster than that. **This service -samples the public feed rather than mirroring it**, and pagination deepens a single -poll within the budget rather than backfilling: events that rolled out of the window -while the service was down are not recoverable. +## Inspecting the data -### When a source goes out of service +Three endpoints answer "what has it actually done" without a psql session, and none of +them consumes request budget: -A permanent `4xx` from `/events` — not a rate limit, not a `5xx`, both of which are -retried — takes the source out of service: `event_sources.status` becomes `failed`, -`last_error` records why, and **no further poll is attempted**, including under -`--force`. That is deliberate: the request cannot succeed, so retrying it on a cadence -would spend the hourly budget on a certainty. +```bash +curl -s http://localhost:3000/status | jq +curl -s 'http://localhost:3000/api/push_events?limit=5' | jq +curl -s http://localhost:3000/api/push_events/ | jq .data.raw_payload +``` -```text -Ingestion deferred — source_failed +### `GET /status` + +Reports persisted state only: the poll schedule per event source (all five of §9's +components, plus which one is binding), the per-class ledger state, §11's three +enrichment coverage percentages, and the per-status entity counts. + +Three conventions in the response body are worth knowing before reading one: + +- **`null` is never a zero.** A counted zero prints `0`; a number that does not + exist prints `null`. Where `null` would be ambiguous the disambiguating fact gets + its own field — `ledger.present` separates "no ledger row yet" from "remaining is + genuinely 0", `due_now` separates "no constraint applies" from "unknown", and + `claimable_now` separates "work is claimable this second" from "nothing is + waiting". An empty coverage window reports `null` percentages, not `0.0`: the + ratio is undefined, not zero, and every denominator is published beside its ratio + so you can check it. +- **The coverage window is measured on `created_at`** — when *this application* + persisted the event, not GitHub's `occurred_at`. Coverage grades this + application's enrichment pipeline, and that pipeline's own eligibility and + freshness rules already run on this clock. The basis is published as + `coverage.basis` so the choice is visible rather than assumed. Widen + `ENRICHMENT_COVERAGE_WINDOW_SECONDS` when reviewing a fixture corpus that has aged. +- **`actor_requests.available` is a floor, not a ceiling.** §10 lets one class + borrow the other's unspent capacity when the other has no eligible candidate, so a + class does not stop at zero available. The real ceiling is the `enrichment` pair + beside it. + +`pending` in the entity counts means `enrichment_status = 'pending'` exactly. The +`candidates` figure beside it is pending **plus** `retryable_failure` — the "how much +work is left" number `bin/ingest` prints. Two questions, two names, deliberately. + +### `GET /api/push_events` and `GET /api/push_events/:id` + +`:id` is the **GitHub event id**, not the surrogate primary key — the identifier §11 +puts on every log line, so a log line is a URL you can type. The surrogate key +appears in no response. + +Paging is keyset on `(occurred_at, id)` with an opaque cursor, not `offset`: the +poller writes continuously and this list is newest-first, so an offset page 2 would +re-serve rows from page 1 and skip others whenever a poll landed in between. Follow +`pagination.next_cursor`, or the RFC-8288 `Link: …; rel="next"` header. `limit` +defaults to 25 and is capped at 100 — a value outside that range is a `400`, never a +silent clamp, because a client that asked for 500 and received 100 cannot tell +whether it received everything. `actor_id` and `repository_id` filter; an unknown +parameter is a `400` rather than a silently unfiltered answer. + +The list omits `raw_payload` — it is a multi-kilobyte TOASTed `jsonb` column nobody +scans a list for. `GET /api/push_events/:id` returns it, which is how §16's "raw +payload is retained" gate is checkable without a psql session. Every row nests its +actor and repository with their `enrichment_status`, so §16's enrichment gate is +visible in a browser as statuses flip from `pending` to `complete` while the worker +runs. + +### Database inspection + +The queue is a database too, so one tool inspects everything. Each query below assumes +this prefix: + +```bash +docker compose exec db psql -U postgres -d github_push_ingestor_development -c "…" ``` -Nothing returns it to service automatically — a later success cannot, because no later -poll happens. Clear it once the cause is fixed: +**How much was captured** + +```sql +SELECT (SELECT COUNT(*) FROM push_events) AS push_events, + (SELECT COUNT(*) FROM github_actors) AS actors, + (SELECT COUNT(*) FROM github_repositories) AS repositories, + (SELECT COUNT(*) FROM quarantined_events) AS quarantined, + (SELECT SUM(occurrence_count) FROM quarantined_events) AS occurrences, + (SELECT COUNT(*) FROM ingestion_runs) AS runs; +``` + +**Where enrichment stands, per class** + +```sql +SELECT 'actor' AS class, enrichment_status, COUNT(*) FROM github_actors GROUP BY 2 +UNION ALL +SELECT 'repository', enrichment_status, COUNT(*) FROM github_repositories GROUP BY 2 +ORDER BY 1, 2; +``` + +**The budget ledger, whole** + +```sql +SELECT window_status, "limit", remaining, reserve, + poll_used, poll_allowance, enrichment_used, enrichment_allowance, + actor_share_used, repository_share_used, + global_blocked_until, reset_at, consecutive_secondary_limits + FROM github_api_budget; +``` + +**The last five runs and what they did** + +```sql +SELECT run_id, status, started_at, pages_fetched, events_created, + duplicates_skipped, events_quarantined, events_failed + FROM ingestion_runs ORDER BY started_at DESC LIMIT 5; +``` + +**What was quarantined, and how often it recurred** + +```sql +SELECT github_event_id, event_type, error_code, occurrence_count, + first_received_at, last_received_at + FROM quarantined_events ORDER BY occurrence_count DESC; +``` + +**Why a source is or is not due** — all five scheduling components side by side + +```sql +SELECT source_type, status, enabled, consecutive_failures, + cadence_due_at, poll_floor_until, retry_not_before_at, next_poll_at + FROM event_sources; +``` + +**The queue** — a different database, same tool ```bash -docker compose exec db psql -U postgres -d github_push_ingestor_development -c " - UPDATE event_sources SET status = 'idle', last_error = NULL WHERE status = 'failed';" +docker compose exec db psql -U postgres -d github_push_ingestor_queue_development \ + -c "SELECT key, class_name, schedule FROM solid_queue_recurring_tasks;" \ + -c "SELECT class_name, count(*) FROM solid_queue_jobs GROUP BY 1;" \ + -c "SELECT kind, name, last_heartbeat_at FROM solid_queue_processes;" ``` -`enabled` is a separate switch and stays untouched: it means *an operator turned this -off*, while `status` means *the system took this out of service*. +## The data model -### Recovering from a fixture rate-limit run +Seven business tables in three groups: -The budget ledger is persisted and the corpus's rate-limit scenarios carry a real -one-hour reset, so a single exploratory run leaves a genuine global block behind: +- **Source and run state** — `event_sources` (what to poll and when), `ingestion_runs` + (what each attempt did). +- **Business records** — `push_events`, `github_actors`, `github_repositories`, + `quarantined_events`. +- **The global ledger** — `github_api_budget`, one row. + +A second database, `github_push_ingestor_queue_development`, holds Solid Queue's tables. +That is job state, not business state: deleting all of it loses nothing, because pending +work is rebuilt from committed entity rows +([ADR 0008](docs/adr/0008-post-commit-enqueue-and-entity-scoped-reconciliation.md)). + +| Table | What a row is | Identity | Written by | +|---|---|---|---| +| `event_sources` | one pollable feed and its schedule | `source_type` | `Ingestion::PollState`, `SourceProvisioner` | +| `ingestion_runs` | one poll attempt that reached GitHub | `run_id` (uuid, unique) | `Ingestion::RunRecorder` | +| `push_events` | one accepted GitHub `PushEvent` | `github_event_id` (unique) | `Ingestion::PageWriter` | +| `github_actors` | one GitHub user seen pushing | `github_id` (unique) | `PageWriter` (stub), `Enrichment::EntityState` | +| `github_repositories` | one GitHub repository seen receiving a push | `github_id` (unique) | `PageWriter` (stub), `Enrichment::EntityState` | +| `quarantined_events` | one distinct malformed payload | `payload_fingerprint` (unique) | `Ingestion::PageWriter` | +| `github_api_budget` | the hourly request budget | `id = 1` (check constraint) | `Github::BudgetLedger` | + +### Columns that carry a rule + +**`push_events`** — the durability boundary. + +| Column | Type | The rule it encodes | +|---|---|---| +| `github_event_id` | `text` **unique** | GitHub's own event id. The unique index *is* the deduplication mechanism | +| `github_push_id` | `bigint` | `payload.push_id`; indexed but not unique — GitHub does not guarantee it is | +| `github_actor_id` | `bigint` FK → `github_actors.github_id` | The FK targets GitHub's identifier, **not** the surrogate primary key | +| `github_repository_id` | `bigint` FK → `github_repositories.github_id` | Same | +| `ref` | `text` `NOT NULL` | `payload.ref` | +| `head_sha`, `before_sha` | `varchar(64)` `NOT NULL` | `payload.head` and `payload.before`. 64, not 40, so **both SHA-1 and SHA-256 object names are accepted** | +| `occurred_at` | `timestamp` `NOT NULL`, indexed | GitHub's `created_at`. Distinct from this row's `created_at`, which is when *this* application persisted it | +| `raw_payload` | `jsonb` `NOT NULL` | Retention is **semantic, not byte-exact** ([ADR 0001](docs/adr/0001-jsonb-semantic-retention.md)) | + +**`github_actors` / `github_repositories`** — identical enrichment state machines. + +| Column | Type | The rule it encodes | +|---|---|---| +| `github_id` | `bigint` **unique** | Identity. The stub upsert conflicts on this | +| `enrichment_status` | `text`, check-constrained | Five legal values; see below | +| `first_seen_at`, `last_seen_at`, `latest_event_at` | `timestamp` | Activity. Updated **only** when a `push_events` insert actually returned a row | +| `next_retry_at` | `timestamp` | Double duty: the backoff instant *and* the enrichment lease. One column, one predicate, so the candidate query, the age-out sweep and the claim cannot drift apart | +| `fetched_at` | `timestamp` | When enrichment last succeeded — the input to the refresh TTL | +| `skipped_at` | `timestamp` | When the eligibility window expired and the row became `skipped_budget` | +| `raw_payload` | `jsonb` nullable | The enrichment response. Null until a fetch succeeds | + +**`event_sources`** — five independent scheduling columns, never one collapsed timestamp: +`cadence_due_at`, `poll_floor_until`, `retry_not_before_at`, plus the ledger's +`global_blocked_until` and a derived class block. `next_poll_at` is a **cache of the +answer, never an input** ([ADR 0006](docs/adr/0006-decomposed-poll-deferral-state.md)). +`status` is `idle` or `failed` by check constraint; `enabled` is the separate operator +switch. + +**`quarantined_events`** — `payload_fingerprint` is the sole identity: SHA-256 of compact +UTF-8 JSON with recursively sorted object keys. `github_event_id` is nullable and indexed +but **not unique**, because a malformed event may be malformed precisely because it has no +event id. `occurrence_count` is check-constrained `>= 1` and climbs on replay. + +**`github_api_budget`** — a `CHECK (id = 1)` singleton, because the unauthenticated quota +is keyed to the outbound IP rather than to a source. `window_status` is +`uninitialized` / `active` / `globally_blocked`; every counter is check-constrained +non-negative; `consecutive_secondary_limits` survives window rollover because secondary +limits are IP-scoped rather than window-scoped +([ADR 0010](docs/adr/0010-secondary-limit-escalation-and-refresh-pool-fairness.md)). + +### The enrichment state machine + +| Status | Entered when | Left when | Spends budget | +|---|---|---|---| +| `pending` | A stub row is created by ingestion | Enrichment succeeds, fails, or ages out | Yes — it is the candidate pool | +| `complete` | An enrichment fetch succeeded | The refresh TTL expires, making it a refresh candidate | Only on refresh | +| `retryable_failure` | A `5xx`, timeout, or transport error | The backoff expires and a retry runs | Yes — it stays a candidate | +| `permanent_failure` | A `404` or other permanent `4xx` on the entity URL | Never, automatically | No | +| `skipped_budget` | The eligibility window expired before budget was available | **Only** a genuinely new push event reactivates it | No | + +`skipped_budget` is what bounds the backlog. A duplicate replay refreshes identity fields +but never reactivates a skipped entity — that is the half of §7's merge rules a re-polled +window would otherwise break. + +### The idempotency contract + +| Table | Write | Effect on replay | +|---|---|---| +| `push_events` | `INSERT … ON CONFLICT (github_event_id) DO NOTHING RETURNING id` | No-op; the accepted raw event is never mutated | +| `github_actors`, `github_repositories` | `INSERT … ON CONFLICT (github_id) DO UPDATE` on identity fields only | Identity refreshed; enrichment payload untouched | +| `quarantined_events` | `INSERT … ON CONFLICT (payload_fingerprint) DO UPDATE` | `occurrence_count` increments; the first classification is permanent | +| Entity activity fields | Gated on the `push_events` insert returning a row | No activity registered, no reactivation | + +Transactions are **one per event**, not one per page, so a single malformed envelope can +never discard the events persisted beside it. Quarantine writes stand outside any +transaction. + +### Indexes worth knowing + +Three are partial — the predicate lives in the index rather than only in the query, so the +planner scans only rows that could possibly qualify: + +- `index_event_sources_on_poll_due` on `(source_type, next_poll_at)` + `WHERE enabled AND status = 'idle'` — the tick's due-source scan. +- `index_github_{actors,repositories}_on_enrichment_candidates` on + `(next_retry_at, last_seen_at)` `WHERE enrichment_status IN ('pending','retryable_failure')` + — the candidate pool. +- `index_github_{actors,repositories}_on_enrichment_refresh` on `(fetched_at, next_retry_at)` + `WHERE enrichment_status = 'complete'` — the TTL refresh pool. + +[`db/schema.rb`](db/schema.rb) is authoritative (version `2026_07_31_120000`). For live +truth: ```bash -GITHUB_MODE=fixture GITHUB_FIXTURE_SCENARIO=rate_limited docker compose run --rm ingest +docker compose exec db psql -U postgres -d github_push_ingestor_development -c "\d+ push_events" ``` -Every later request — in any scenario — is then deferred with `globally_blocked` -until that hour elapses, and the `budget.global_block_set` log line names the -instant. To clear it in development: +The entity-relationship diagram is Figure 2 of +[`docs/DESIGN_BRIEF.md`](docs/DESIGN_BRIEF.md). + +## Logs + +### Anatomy of a log line + +One JSON object per line on stdout. Four fields are owned by +[`lib/json_log_formatter.rb`](lib/json_log_formatter.rb) and cannot be overridden by an +application payload — a structured event can never spoof its own severity, service, +environment or timestamp: + +```json +{ + "timestamp": "2026-07-31T15:50:49.973Z", + "level": "info", + "service": "github-push-ingestor", + "environment": "development", + "event": "ingestion.run_completed", + "run_id": "099d562d-1261-488d-9003-cb0c443cdb55", + "event_source_id": 1, + "duration_ms": 100.7, + "run_status": "completed", + "pages_fetched": 1, + "events_received": 8, + "push_events_seen": 6, + "events_created": 4, + "duplicates_skipped": 0, + "events_quarantined": 3, + "events_ignored": 1, + "events_failed": 0 +} +``` + +### Sample stream + +Captured verbatim from a fixture-mode run against an empty database, so you can reproduce +the same shapes offline: ```bash -docker compose exec db psql -U postgres -d github_push_ingestor_development -c " - UPDATE github_api_budget - SET global_blocked_until = NULL, reset_at = NULL, remaining = NULL, - window_status = 'uninitialized' - WHERE id = 1;" +GITHUB_MODE=fixture docker compose up --build -d +GITHUB_MODE=fixture docker compose run --rm ingest 2>&1 | grep '^{' +docker compose logs --no-log-prefix worker | grep '^{' +GITHUB_MODE=fixture GITHUB_FIXTURE_SCENARIO=transient_failure \ + docker compose run --rm ingest --force 2>&1 | grep '^{' +``` + +The `grep '^{'` is load-bearing: `bin/ingest` shares stdout between the operator summary +and the JSON stream, so a leading brace is exactly what separates them. + +Boot, then a poll that created four events and quarantined three: + +```text +{"timestamp":"2026-07-31T15:50:49.677Z","level":"info","service":"github-push-ingestor","environment":"development","event":"config.budget_resolved","mode":"fixture","poll_interval_seconds":300,"max_pages_per_poll":1,"enabled_live_source_count":1,"worst_case_reservations_per_poll":9,"limit":60,"reserve":8,"poll_allowance":12,"enrichment_allowance":40,"actor_guarantee":20,"repository_guarantee":20} +{"timestamp":"2026-07-31T15:50:49.896Z","level":"info","service":"github-push-ingestor","environment":"development","event":"ingestion.run_started","run_id":"099d562d-1261-488d-9003-cb0c443cdb55","event_source_id":1,"source_type":"github_fixture_events","github_mode":"fixture","forced":false,"lock_wait_ms":2.2} +{"timestamp":"2026-07-31T15:50:49.924Z","level":"info","service":"github-push-ingestor","environment":"development","event":"budget.window_initialized","limit":60,"reserve":8,"poll_allowance":12,"enrichment_allowance":40,"actor_guarantee":20,"repository_guarantee":20,"rate_limit_resource":"core","rate_limit_limit":60,"rate_limit_remaining":59,"rate_limit_used":1,"rate_limit_reset_at":"2026-07-31T16:50:49Z","poll_used":1} +{"timestamp":"2026-07-31T15:50:49.965Z","level":"info","service":"github-push-ingestor","environment":"development","event":"ingestion.event_quarantined","run_id":"099d562d-1261-488d-9003-cb0c443cdb55","github_event_id":"58000000006","event_type":"PushEvent","error_code":"invalid_field_format","error_message":"payload.head is \"not-a-valid-object-name\", not 40 or 64 hexadecimal characters","payload_fingerprint":"a8ad67ca97a4c48049f5fa447d5d88ae10c58c514e0129546e18b5ff22368020"} +{"timestamp":"2026-07-31T15:50:49.973Z","level":"info","service":"github-push-ingestor","environment":"development","event":"ingestion.run_completed","run_id":"099d562d-1261-488d-9003-cb0c443cdb55","event_source_id":1,"duration_ms":100.7,"next_poll_at":"2026-07-31T15:55:49Z","consecutive_failures":0,"run_status":"completed","classification":"ok","stop_reason":"no_next_link","pages_fetched":1,"events_received":8,"push_events_seen":6,"events_created":4,"duplicates_skipped":0,"events_quarantined":3,"events_ignored":1,"events_failed":0} +{"timestamp":"2026-07-31T15:50:50.024Z","level":"info","service":"github-push-ingestor","environment":"development","event":"enrichment.dispatched","actor_enqueued":1,"repository_enqueued":1,"reason":"ingestion","actor_counts":{"pending":3},"repository_counts":{"pending":3},"actor_share_used":0,"repository_share_used":0,"actor_guarantee":20,"repository_guarantee":20,"enrichment_used":0,"enrichment_allowance":40,"window_status":"active","claimable_now":true} +``` + +A second run inside the cadence window makes no request at all, and names the component +holding it: + +```text +{"timestamp":"2026-07-31T15:51:00.142Z","level":"info","service":"github-push-ingestor","environment":"development","event":"ingestion.not_due","event_source_id":1,"forced":false,"deferral_reason":"cadence_due_at","next_poll_at":"2026-07-31T15:55:49Z","cadence_due_at":"2026-07-31T15:55:49Z","poll_floor_until":"2026-07-31T15:51:49Z"} +``` + +From the worker — a job, its enrichment outcome, and the deliberate `404` the corpus plants +so a dead target fails the *entity* rather than the source: + +```text +{"timestamp":"2026-07-31T15:50:50.780Z","level":"info","service":"github-push-ingestor","environment":"development","event":"job.completed","job_id":"58a1e78d-f473-4440-aee0-fe0bbb22027f","job_class":"EnrichActorJob","queue":"default","attempt":1,"duration_ms":74.6,"entity_type":"actor","github_actor_id":7700421,"enrichment_outcome":"failed"} +{"timestamp":"2026-07-31T15:50:50.780Z","level":"info","service":"github-push-ingestor","environment":"development","enrichment_outcome":"failed","entity_type":"actor","github_id":7700421,"pool":"pending","classification":"not_found","entity_status":"permanent_failure","enrichment_attempt":1,"error_message":"GitHub returned 404 (not_found)","duration_ms":56.6,"event":"enrichment.failed"} +{"timestamp":"2026-07-31T15:51:00.949Z","level":"info","service":"github-push-ingestor","environment":"development","enrichment_outcome":"enriched","entity_type":"actor","github_id":1024025,"pool":"pending","classification":"ok","entity_status":"complete","enrichment_attempt":1,"duration_ms":27.3,"event":"enrichment.completed"} +``` + +A retry ladder, from the `transient_failure` scenario — the failed request at `warn`, the +scheduled backoff at `info` reporting the delay actually slept: + +```text +{"timestamp":"2026-07-31T15:52:11.046Z","level":"warn","service":"github-push-ingestor","environment":"development","event":"github.request","request_class":"poll","http_method":"get","url":"fixture://api.github.com/events?per_page=100","origin":"application","run_id":"2de73c38-2d98-4f90-b09b-53044ac54395","source_type":"github_fixture_events","http_status":500,"classification":"server_error","attempt":0,"duration_ms":0.0} +{"timestamp":"2026-07-31T15:52:11.046Z","level":"info","service":"github-push-ingestor","environment":"development","event":"github.retry_scheduled","request_class":"poll","http_method":"get","url":"fixture://api.github.com/events?per_page=100","origin":"application","run_id":"2de73c38-2d98-4f90-b09b-53044ac54395","source_type":"github_fixture_events","http_status":500,"classification":"server_error","attempt":0,"duration_ms":0.0,"next_attempt":1,"max_attempts":2,"backoff_seconds":1.2} +``` + +That run also demonstrates the replay contract in one line — the same page, zero created, +four absorbed: + +```text +{"timestamp":"2026-07-31T15:52:14.560Z","level":"info","service":"github-push-ingestor","environment":"development","event":"ingestion.run_completed","run_id":"2de73c38-2d98-4f90-b09b-53044ac54395","event_source_id":1,"duration_ms":3539.3,"next_poll_at":"2026-07-31T15:57:14Z","consecutive_failures":0,"run_status":"completed","classification":"ok","stop_reason":"no_next_link","pages_fetched":1,"events_received":8,"push_events_seen":6,"events_created":0,"duplicates_skipped":4,"events_quarantined":3,"events_ignored":1,"events_failed":0} ``` ### What a run logs @@ -498,14 +897,133 @@ until an operator clears it (plan §10). It shares its token with the `deferral_reason` of every poll subsequently refused, so one grep returns the transition and its consequences together. +Background work adds the job vocabulary: `job.completed` for every job the worker +runs — carrying `job_id`, `job_class`, `queue`, `attempt`, `duration_ms` and the +identifiers that job produced — and `job.failed` with the error class and message +when one raises. `ingestion.source_busy` reports a tick that found the source owned +by another poller, `ingestion.cycle_failed` one source that failed without stopping +the tick, and `enrichment.dispatched` is the reconciliation summary: what it +scheduled, what blocked it, and the per-class state counts and share usage. A tick +that scheduled nothing keeps that line at debug, so an exhausted window does not +emit a line a minute for the rest of the hour. + `LOG_LEVEL=debug` adds the `github.request` line for requests that **succeeded** (the failing ones are already at warning), `ingestion.page_fetched` and `ingestion.page_processed` per page, `ingestion.pagination_stopped` with its reason, `ingestion.not_modified` — which carries the `x-ratelimit-used` and `x-ratelimit-remaining` that make the `304` accounting visible in the running -system — `budget.co_tenant_usage` (below), plus a line per persisted, duplicate and ignored +system — `budget.co_tenant_usage`, plus a line per persisted, duplicate and ignored event. +### Reading the stream + +Every line carries the run's `run_id`, except `ingestion.not_due`: a poll the +schedule turned away opens no run, so it reports `event_source_id` instead. **The trace is +one hop** — a `job_id` on `job.completed` gives the `run_id`s that job opened, and every +`ingestion.*` line carries the `run_id`. + +```bash +# one run, end to end +docker compose logs | grep '"run_id":"099d562d-1261-488d-9003-cb0c443cdb55"' + +# one job, and the runs it opened +docker compose logs worker | grep -E 'job\.(completed|failed)|enrichment\.dispatched' + +# everything that went wrong +docker compose logs | grep -E '"level":"(warn|error)"' + +# every budget state transition +docker compose logs | grep '"event":"budget\.' +``` + +## Rate limits and the request budget + +### The allowance formula + +`POLL_INTERVAL_SECONDS`, `MAX_PAGES_PER_POLL`, `ENABLED_LIVE_SOURCE_COUNT` and +`RATE_LIMIT_RESERVE` feed the one authoritative allowance formula (plan §10): + +```text +poll_attempt_allowance = ceil(3600 / POLL_INTERVAL_SECONDS) + x MAX_PAGES_PER_POLL x ENABLED_LIVE_SOURCE_COUNT +enrichment_allowance = rate_limit - RATE_LIMIT_RESERVE - poll_attempt_allowance +``` + +With the defaults: 12 poll attempts and 40 enrichment attempts an hour, against +GitHub's unauthenticated limit of 60. **The process refuses to boot** if the +polling requirement leaves no capacity for enrichment. + +The enrichment allowance is then split by `ACTOR_ENRICHMENT_SHARE` (plan §10): + +```text +actor_guarantee = floor(enrichment_allowance x ACTOR_ENRICHMENT_SHARE) +repository_guarantee = enrichment_allowance - actor_guarantee +``` + +With the defaults: 20 actor and 20 repository attempts an hour. The remainder +always goes to repositories, because the formula floors one side and subtracts for +the other — so the two always add up to the whole allowance. + +### The budget table + +At limit 60, reserve 8, cadence 300s, one source — the only variable is page depth: + +| `MAX_PAGES_PER_POLL` | Poll allowance | Enrichment allowance | Actor guarantee | Repository guarantee | +|---|---|---|---|---| +| 1 (default) | 12 | 40 | 20 | 20 | +| 2 | 24 | 28 | 14 | 14 | +| 3 | 36 | 16 | 8 | 8 | +| 4 | 48 | 4 | 2 | 2 | +| 5 | 60 | — | — | **refuses to boot** | + +Capture depth is bought with enrichment coverage, at a fixed exchange rate, and the +formula tells you the price before you pay it. + +### Global blocks versus class exhaustion + +Two different things that both stop requests, deliberately kept apart: + +- **A global block** is the only thing written to `github_api_budget.global_blocked_until`, + and it only ever moves later. Primary exhaustion, a reserve breach, and a secondary rate + limit set it; it stops **everything**. `budget.global_block_set` and + `budget.global_block_cleared` mark the edges. +- **Class exhaustion** is *derived* from the counters and writes nothing. When polling has + spent its twelve, enrichment carries on; when enrichment has spent its forty, polling + carries on. `budget.class_exhausted` fires once per class per window, and + `budget.share_exhausted` once per fairness share. + +So "the poller ran out" never means "the system stopped", and a reviewer can tell which +happened from one log line. + +### Per-window bootstrap + +The ledger is never seeded by a discovery request. The **first canonical page-one poll of +each rate-limit window** initialises it from that response's authoritative headers — it is +a normal, counted, event-processing poll that happens to also establish the window. Until +the window is `active`, enrichment is ineligible. `budget.window_initialized` marks it, +`budget.window_rolled` marks the hour turning over, and `budget.window_reset_in_past` warns +that this container's clock runs ahead of GitHub's +([ADR 0004](docs/adr/0004-class-aware-budget-ledger.md)). + +### Why a `304` costs a request here + +GitHub's events documentation states generally that `304` responses do not count +against the rate limit; its REST best-practices documentation scopes that exemption +to requests "correctly authorized with an `Authorization` header". This service +sends no token, so the two statements disagree about exactly the population of +requests it makes. A dated unauthenticated probe run under +`X-GitHub-Api-Version: 2022-11-28` settles it for this configuration: +`x-ratelimit-used` increments across a `304`. The transcript, with complete +before-and-after headers and its own stated limits, is +[`docs/evidence/2026-07-30-unauthenticated-304-quota-probe.md`](docs/evidence/2026-07-30-unauthenticated-304-quota-probe.md). + +So the ledger debits every outbound attempt, `304`s included, and the stored ETag +is a bandwidth and correctness measure rather than a quota saver. The asymmetry +justifies the choice: budgeting a `304` that turns out to be free wastes one +attempt, while not budgeting one that is in fact charged overruns a sixty-request +hour. The transcript claims nothing about authenticated requests — this project has +no token and cannot test that case. + ### Sharing the outbound IP The unauthenticated limit is keyed to the outbound IP, so anything else behind that address @@ -520,27 +1038,7 @@ is raised to `budget.co_tenant_pressure` at info, because every class is about t cause attached to it. Nothing is inferred back into the class counters: GitHub's `used` carries no request class, and a guess would break the fairness accounting. -Every line carries the run's `run_id`, except `ingestion.not_due`: a poll the -schedule turned away opens no run, so it reports `event_source_id` instead. - -Background work adds the job vocabulary: `job.completed` for every job the worker -runs — carrying `job_id`, `job_class`, `queue`, `attempt`, `duration_ms` and the -identifiers that job produced — and `job.failed` with the error class and message -when one raises. `ingestion.source_busy` reports a tick that found the source owned -by another poller, `ingestion.cycle_failed` one source that failed without stopping -the tick, and `enrichment.dispatched` is the reconciliation summary: what it -scheduled, what blocked it, and the per-class state counts and share usage. A tick -that scheduled nothing keeps that line at debug, so an exhausted window does not -emit a line a minute for the rest of the hour. - -The trace is one hop: a `job_id` on `job.completed` gives the `run_id`s that job -opened, and every `ingestion.*` line carries the `run_id`. - -```bash -docker compose logs worker | grep -E 'job\.(completed|failed)|enrichment\.dispatched' -``` - -## Environment variables +## Configuration Compose runs with working defaults — no `.env` file is required. The template is [`.env.example`](.env.example). @@ -558,7 +1056,7 @@ is [`.env.example`](.env.example). | `MAX_REDIRECTS` | `2` | Redirect hops followed per request, each re-validated and separately reserved | | `SOURCE_LOCK_WAIT_SECONDS` | `30` | How long the one-shot waits for a busy source lock; the poller attempts once (plan §9) | | `POLL_INTERVAL_SECONDS` | `300` | The poll cadence, and an allowance-formula input. A source polled at T is due again at T + this; an unforced run before then is deferred rather than made. The worker's 60-second tick checks the schedule; it does not replace it (plan §9, §10) | -| `MAX_PAGES_PER_POLL` | `1` | How many `Link`-followed pages one poll may fetch, and an allowance-formula input. Raising it trades enrichment allowance for capture depth: at `3` the poll allowance becomes 36 attempts an hour and enrichment drops to 16 (plan §9, §10) | +| `MAX_PAGES_PER_POLL` | `1` | How many `Link`-followed pages one poll may fetch, and an allowance-formula input. Raising it trades enrichment allowance for capture depth — see [the budget table](#the-budget-table) (plan §9, §10) | | `ENABLED_LIVE_SOURCE_COUNT` | `1` | Allowance-formula input: live sources sharing one per-IP budget. **A fallback rather than the authority** — at window initialization and rollover the formula counts the enabled, in-service `event_sources` rows of the running mode and uses that instead, falling back to this value only when there are none yet. A disagreement is logged as `budget.source_allocation_drift` (plan §10, [ADR 0009](docs/adr/0009-runtime-source-allocation-and-shared-ip-observability.md)) | | `RATE_LIMIT_RESERVE` | `8` | Requests per hour left deliberately unspent (plan §10) | | `ACTOR_ENRICHMENT_SHARE` | `0.50` | How the enrichment allowance splits between actors and repositories: `floor(allowance x this)` guarantees actors, the remainder goes to repositories. A guarantee, not a cap — either class may borrow the other's unused capacity when the other has no *currently eligible* candidate. Both ends of `[0, 1]` are legal (plan §10) | @@ -567,41 +1065,18 @@ is [`.env.example`](.env.example). | `REPOSITORY_REFRESH_TTL_SECONDS` | `86400` | The same, for repositories | | `ENRICHMENT_COVERAGE_WINDOW_SECONDS` | `86400` | How far back `GET /status` looks when computing §11's three coverage percentages, measured on `push_events.created_at`. The only knob here that changes what the system *reports* rather than what it *does* | -`POLL_INTERVAL_SECONDS`, `MAX_PAGES_PER_POLL`, `ENABLED_LIVE_SOURCE_COUNT` and -`RATE_LIMIT_RESERVE` feed the one authoritative allowance formula (plan §10): - -```text -poll_attempt_allowance = ceil(3600 / POLL_INTERVAL_SECONDS) - x MAX_PAGES_PER_POLL x ENABLED_LIVE_SOURCE_COUNT -enrichment_allowance = rate_limit - RATE_LIMIT_RESERVE - poll_attempt_allowance -``` - -With the defaults: 12 poll attempts and 40 enrichment attempts an hour, against -GitHub's unauthenticated limit of 60. **The process refuses to boot** if the -polling requirement leaves no capacity for enrichment. - -There is deliberately no variable for the API host or the API version. The -allowed host is a constant in `Github::UrlPolicy`, because an environment -variable there would make the SSRF boundary a deployment setting; the API version -is pinned to `2022-11-28`, the version every live probe behind this plan was run -under. - -The enrichment allowance is then split by `ACTOR_ENRICHMENT_SHARE` (plan §10): - -```text -actor_guarantee = floor(enrichment_allowance x ACTOR_ENRICHMENT_SHARE) -repository_guarantee = enrichment_allowance - actor_guarantee -``` - -With the defaults: 20 actor and 20 repository attempts an hour. The remainder -always goes to repositories, because the formula floors one side and subtracts for -the other — so the two always add up to the whole allowance. - Database connection settings (`POSTGRES_HOST`, `POSTGRES_PORT`, `POSTGRES_USER`, `POSTGRES_PASSWORD`) are managed by the compose topology itself and matter only when running the app outside compose. -## The request path +There is deliberately no variable for the API host or the API version — see +[The SSRF boundary](#the-ssrf-boundary) and +[ADR 0011](docs/adr/0011-pinned-api-version-2022-11-28.md). + +## Architecture + +[`docs/DESIGN_BRIEF.md`](docs/DESIGN_BRIEF.md) is the two-page architecture summary, with +rendered request-path and data-model figures. This section is the annotated call chain. Every live GitHub request — polling and enrichment, from the poller, the worker, or the one-shot — takes one chain, and nothing outside it calls GitHub @@ -664,14 +1139,11 @@ EnrichmentRunner ────────────────┘ │ request that completes without a secondary limit resets the run. Class exhaustion deliberately writes nothing there: it is derived from the counters, so polling running out never stops enrichment and vice versa. -- **`Github::UrlPolicy`** is the SSRF boundary. It rebuilds every URL from validated - components, and a URL that arrived inside a GitHub payload or a `Link` header always - clears the full live policy first. +- **`Github::UrlPolicy`** is the SSRF boundary — see below. - **Transports** are `Faraday` (live) and `Fixture` (offline). The Faraday connection carries the adapter and no middleware at all — retries and redirects belong to the executor, because each attempt re-reserves budget and each redirect target is re-validated. - - **`Github::Ingestion::PageWriter`** turns one fetched page into rows, in **one transaction per event** — so a single malformed event can never discard the events persisted beside it. Quarantine writes stand outside any transaction, and entity activity @@ -712,13 +1184,50 @@ EnrichmentRunner ────────────────┘ │ against a poll cadence, and enrichment has none — and no per-class share, because a share exhaustion is a denial rather than a deferral. +### The SSRF boundary + +Enrichment follows URLs that arrive **inside GitHub payloads**, `Link` headers and +`Location` headers — data this application did not construct and an attacker could +influence. So `Github::UrlPolicy` is a trust boundary, not a formatter. Every URL is +rebuilt from validated components, and a `:payload`-origin URL always clears the full +*live* policy first, whatever mode the process is in: + +- HTTPS only +- Host exactly `api.github.com` +- No userinfo (`https://evil@api.github.com/…` is refused) +- No non-default port +- No IP literals +- Redirects bounded by `MAX_REDIRECTS`, each hop re-validated and separately debited + +The allowed host is a **constant, not an environment variable** — a deployment setting +there would make the trust boundary configurable. The API version is pinned to +`2022-11-28` for the same class of reason +([ADR 0011](docs/adr/0011-pinned-api-version-2022-11-28.md)). Fixture mode fails closed: +an unknown URL is an error, never a live fallback +([ADR 0003](docs/adr/0003-event-source-and-transport-seams.md)). + +It is demonstrable offline. The `hostile_redirect` scenario serves a `301` whose `Location` +points off-host: + +```bash +GITHUB_MODE=fixture GITHUB_FIXTURE_SCENARIO=hostile_redirect \ + docker compose run --rm enrich --class repository +docker compose logs | grep '"event":"github.request"' | grep '"level":"warn"' +``` + +The second hop is never sent, the entity records the failure, and **the event source stays +in service** — a refused redirect is a fact about one entity, not about the feed. Compare +`redirecting_repository`, where a legitimate rename is followed across one validated hop +and debited twice. + Decisions behind this are recorded in -[`docs/adr/`](docs/adr/): advisory locks and the gate (0002), the source and transport -seams (0003), the class-aware ledger (0004), at-least-once processing with idempotent -writes (0005), decomposed poll deferral state (0006), enrichment fairness shares -and borrowing (0007), post-commit enqueue with entity-scoped reconciliation (0008), -runtime source allocation with shared-IP observability (0009), and secondary-limit -escalation with refresh-pool fairness (0010). +[`docs/adr/`](docs/adr/): `jsonb` semantic retention (0001), advisory locks and the gate +(0002), the source and transport seams (0003), the class-aware ledger (0004), +at-least-once processing with idempotent writes (0005), decomposed poll deferral state +(0006), enrichment fairness shares and borrowing (0007), post-commit enqueue with +entity-scoped reconciliation (0008), runtime source allocation with shared-IP +observability (0009), secondary-limit escalation with refresh-pool fairness (0010), the +pinned API version (0011), and Solid Queue over Kafka (0012). ## Continuous ingestion @@ -750,56 +1259,56 @@ lease, so a backlog of ninety pending actors is one queued job rather than ninet Steady state at the defaults: twelve polls an hour, and at most forty enrichment requests an hour split between the two classes. -**Crashes need no cleanup.** A killed worker's PostgreSQL session dies with it, and -the source advisory lock goes with the session; its claim lease is a timestamp on -`next_retry_at` that expires by arithmetic; and any job it was running may run again, -because every write on the path is idempotent. That is at-least-once execution with -effectively-once persisted outcomes — never exactly-once execution. - -## Planned contents - -| Section | Lands with | -|---|---| -| Data model reference | PR 12 (design brief) | -| API and database inspection examples | PR 10, 12 | -| Known limitations (sampling-based enrichment, no complete-capture guarantee, shared-IP budget) | PR 12 | +## Processing guarantees -## Reviewer commands +**An event is accepted when its `push_events` row commits** — not when GitHub returns it, +not when a job is enqueued, not when a log line says so. That commit is the durability +boundary; everything upstream of it is retryable and everything downstream is derived. -```bash -docker compose up --build # available now — starts continuous polling -docker compose run --rm test # available now (real suite; runs in CI too) -docker compose logs -f worker # available now — the tick, cycles, reconciliation -docker compose logs -f # available now -docker compose run --rm ingest # available now — one ingestion cycle -docker compose run --rm enrich --limit 6 # available now — up to six enrichment cycles -GITHUB_MODE=fixture docker compose up --build # available now — the whole system, no network -GITHUB_MODE=fixture docker compose run --rm ingest # available now — deterministic, no network -GITHUB_MODE=fixture docker compose run --rm enrich --limit 6 -script/verify_recovery.sh --confirm # §15 step 8's container kills; nothing in CI runs it +```text +At-least-once execution ++ Idempotent writes ++ Unique constraints += Effectively-once persisted outcomes ``` -The queue is a database, so it is inspectable with the same tool as everything else: +### What is not claimed, and why -```bash -docker compose exec db psql -U postgres -d github_push_ingestor_queue_development \ - -c "SELECT key, class_name, schedule FROM solid_queue_recurring_tasks;" \ - -c "SELECT class_name, count(*) FROM solid_queue_jobs GROUP BY 1;" \ - -c "SELECT kind, name, last_heartbeat_at FROM solid_queue_processes;" -``` +- **Not exactly-once execution.** A job may run twice; the second run changes nothing + durable. Side effects that are not idempotent writes — a log line, a duration metric — + can and will repeat. +- **Not complete upstream capture.** The feed is a sliding window with hours of latency; + see [Known limitations](#known-limitations). +- **Not complete enrichment coverage.** Demand exceeds the hourly budget by roughly fifty + to one; see [Known limitations](#known-limitations). -Recovery is watchable in under a minute — stop the worker, put the entities back into -`pending`, empty the queue (the crash), and start it again: +### The four crash cases -```bash -docker compose stop worker -docker compose exec db psql -U postgres -d github_push_ingestor_development \ - -c "UPDATE github_actors SET enrichment_status = 'pending', fetched_at = NULL, next_retry_at = NULL;" -docker compose exec db psql -U postgres -d github_push_ingestor_queue_development \ - -c "TRUNCATE solid_queue_jobs CASCADE;" -docker compose start worker -docker compose logs -f worker # enrichment.dispatched, then enrichment.completed -``` +| Crash point | What survives | What recovers it | +|---|---|---| +| Before the event commits | Nothing from this page | The next overlapping poll re-fetches it — there is no stop-on-known-event | +| After the event commits, before enrichment is enqueued | The `push_events` row and its stub entities | `ReconcilePendingEnrichmentsJob`, from committed entity rows, within 60s | +| Worker dies before the enrichment commit | The entity row, still `pending`, its lease expiring by arithmetic | The next reconcile tick after `next_retry_at` passes | +| Worker dies after the enrichment commit, before acknowledgement | The enriched entity row | Solid Queue re-runs the job; the re-run is a no-op | + +### The mechanisms + +- `INSERT … ON CONFLICT (github_event_id) DO NOTHING RETURNING id` — the deduplication gate. +- Activity updates gated on that `RETURNING` — so a replay refreshes identity but never + reactivates a `skipped_budget` entity. +- `payload_fingerprint` uniqueness on quarantine — one row per distinct malformed payload, + occurrence-counted. +- The entity lease is a `next_retry_at` timestamp that **expires by arithmetic**, so a + crashed worker leaves nothing to release. +- Session advisory locks die with their PostgreSQL session, so a killed poller releases its + source the moment its connection drops. +- `enqueue_after_transaction_commit` plus an entity-scoped reconciler — the enqueue is a + hint, the committed rows are the record. +- `restart: unless-stopped` on `db`, `web`, and `worker`. + +[ADR 0005](docs/adr/0005-at-least-once-with-idempotent-writes.md) argues the choice; +[ADR 0008](docs/adr/0008-post-commit-enqueue-and-entity-scoped-reconciliation.md) covers +recovery; [Crash recovery, verified](#crash-recovery-verified) demonstrates it. ## Crash recovery, verified @@ -835,38 +1344,182 @@ One reading tip for §15 step 8's `docker compose ps` output: `web`'s container curls `/health/live`, which never touches the database, so `web` stays green throughout a `db` kill. `/health/ready` is the observable that flips. -### Fixture scenario matrix +Recovery is also watchable by hand in under a minute — stop the worker, put the entities back +into `pending`, empty the queue (the crash), and start it again: -Every scenario resolves inside [`fixtures/github/`](fixtures/github/) with no network at all. -Each poll obeys GitHub's `X-Poll-Interval` floor of 60s, which `--force` deliberately does not -bypass, so allow a minute between successive ingestion scenarios. +```bash +docker compose stop worker +docker compose exec db psql -U postgres -d github_push_ingestor_development \ + -c "UPDATE github_actors SET enrichment_status = 'pending', fetched_at = NULL, next_retry_at = NULL;" +docker compose exec db psql -U postgres -d github_push_ingestor_queue_development \ + -c "TRUNCATE solid_queue_jobs CASCADE;" +docker compose start worker +docker compose logs -f worker # enrichment.dispatched, then enrichment.completed +``` -| Scenario | Command | What it shows | Cleanup | -|---|---|---|---| -| `default` | `GITHUB_MODE=fixture docker compose run --rm ingest` | 4 push events, 3 actors, 3 repositories, 3 quarantined | none | -| `default` (replay) | `… run --rm ingest --force` after 60s | `duplicates_skipped > 0`, occurrence counts climb, no skipped entity reactivated | none | -| `default` (enrich) | `… run --rm enrich --limit 6` | both classes enrich within their fairness shares | none | -| `paginated` | `GITHUB_FIXTURE_SCENARIO=paginated MAX_PAGES_PER_POLL=3 … ingest` | `Link`-driven walk over 3 pages | none | -| `paginated_final_page` | `GITHUB_FIXTURE_SCENARIO=paginated_final_page … ingest` | the walk stops when no `next` link exists | none | -| `transient_failure` | `GITHUB_FIXTURE_SCENARIO=transient_failure … ingest` | one `500`, then success on retry | none | -| `transient_failure_exhausted` | `GITHUB_FIXTURE_SCENARIO=transient_failure_exhausted … ingest` | retries exhausted; the source backs off | source backoff | -| `redirecting_repository` | `GITHUB_FIXTURE_SCENARIO=redirecting_repository … enrich --class repository` | a renamed repository followed across one validated hop, debited twice | none | -| `hostile_redirect` | `GITHUB_FIXTURE_SCENARIO=hostile_redirect … enrich --class repository` | an off-host `Location` refused by the URL policy; the second hop is never sent and the event source stays in service | none | -| `secondary_rate_limited` | `GITHUB_FIXTURE_SCENARIO=secondary_rate_limited … ingest` | a secondary limit blocks globally | global block | -| `rate_limited` | `GITHUB_FIXTURE_SCENARIO=rate_limited … ingest` | primary exhaustion → `global_blocked_until` | **a real one-hour global block** | +Emptying the queue loses nothing, which is the point: the work is rebuilt from committed +entity rows. -The last two leave durable state behind on purpose — that is the behaviour being demonstrated. -`script/verify_recovery.sh --phase=cleanup` runs the SQL under -[Recovering from a fixture rate-limit run](#recovering-from-a-fixture-rate-limit-run), and -`--phase=rate-limit` plays the rate-limit scenario and cleans up immediately after. +## Troubleshooting and reset + +### Common symptoms + +| Symptom | Likely cause | What to run | +|---|---|---| +| No `push_events` after 10 minutes in live mode | Documented 30s–6h feed latency plus the 5-minute cadence — not a fault | `curl -s localhost:3000/status \| jq .sources` | +| Every run reports `globally_blocked` | A `rate_limited` fixture scenario left a real one-hour block | [Recovering from a fixture rate-limit run](#recovering-from-a-fixture-rate-limit-run) | +| Every run reports `source_failed` | A permanent `4xx` took the source out of service | [When a source goes out of service](#when-a-source-goes-out-of-service) | +| `ingest` exits `2` | Unknown option, a refused configuration, or a fixture-corpus gap | `docker compose run --rm ingest --help` | +| Worker logs nothing | `setup` did not complete, so `worker` never started | `docker compose logs setup` | +| `/health/ready` fails while `/health/live` is fine | Database unreachable or schema not loaded. `web`'s healthcheck curls `/health/live`, so `web` stays green through a `db` outage | `docker compose ps`, `docker compose logs db` | +| Enrichment stuck at `pending` | The allowance is spent for this window, or the window is not `active` yet | `curl -s localhost:3000/status \| jq .ledger` | + +### When a source goes out of service + +A permanent `4xx` from `/events` — not a rate limit, not a `5xx`, both of which are +retried — takes the source out of service: `event_sources.status` becomes `failed`, +`last_error` records why, and **no further poll is attempted**, including under +`--force`. That is deliberate: the request cannot succeed, so retrying it on a cadence +would spend the hourly budget on a certainty. + +```text +Ingestion deferred — source_failed +``` + +Nothing returns it to service automatically — a later success cannot, because no later +poll happens. Clear it once the cause is fixed: + +```bash +docker compose exec db psql -U postgres -d github_push_ingestor_development -c " + UPDATE event_sources SET status = 'idle', last_error = NULL WHERE status = 'failed';" +``` + +`enabled` is a separate switch and stays untouched: it means *an operator turned this +off*, while `status` means *the system took this out of service*. + +### Recovering from a fixture rate-limit run + +The budget ledger is persisted and the corpus's rate-limit scenarios carry a real +one-hour reset, so a single exploratory run leaves a genuine global block behind: + +```bash +GITHUB_MODE=fixture GITHUB_FIXTURE_SCENARIO=rate_limited docker compose run --rm ingest +``` + +Every later request — in any scenario — is then deferred with `globally_blocked` +until that hour elapses, and the `budget.global_block_set` log line names the +instant. To clear it in development: + +```bash +docker compose exec db psql -U postgres -d github_push_ingestor_development -c " + UPDATE github_api_budget + SET global_blocked_until = NULL, reset_at = NULL, remaining = NULL, + window_status = 'uninitialized' + WHERE id = 1;" +``` + +### Resetting + +Three levels, least destructive first. + +**Level 1 — clear derived state, keep the data.** The two SQL blocks above, or both at +once: + +```bash +script/verify_recovery.sh --phase=cleanup +``` + +**Level 2 — drop the business data, keep the volume.** + +```bash +docker compose exec db psql -U postgres -d github_push_ingestor_development -c " + TRUNCATE push_events, quarantined_events, github_actors, github_repositories, + ingestion_runs, event_sources, github_api_budget RESTART IDENTITY CASCADE;" +``` + +Safe because **nothing is seeded**: `Github::Ingestion::SourceProvisioner.ensure!` +recreates the `event_sources` row lazily at the point of use, and +`Github::BudgetLedger#bootstrap!` recreates the ledger row from the next window's response +headers. The next `ingest` behaves exactly like a first run — that is expected, not a +broken system. + +**Level 3 — destroy everything, including the volume.** + +```bash +docker compose down -v +docker compose up --build +``` + +`-v` deletes the named `pgdata` volume and **every stored event**. Plain +`docker compose down` does not: it stops and removes the containers and leaves the volume +intact, which is why step 9 of the verification walk finds identical counts afterwards. + +## Known limitations + +### Expected time before records appear + +A push reaches the public feed with a documented latency of 30 seconds to 6 hours, +and the default cadence polls every 5 minutes, so a given push may take hours to +appear — or never, if it left the feed's 300-event window before a poll reached it. +The feed retains 30 days. At `MAX_PAGES_PER_POLL=1` each poll sees at most the +newest ~100 events, and the feed moves considerably faster than that. **This service +samples the public feed rather than mirroring it.** + +The eight limitations that follow are consequences of that, and of the 60-request hourly +ceiling. None is a gap to be closed later; each is a stated boundary. + +**1. Enrichment coverage is a sample, not coverage.** One observed live page held ~92–95 +`PushEvent` records with ~89 distinct actors and ~92 distinct repositories — 181 cold +entity requests per page, ~2,172 an hour at twelve polls, against 40 available. That is +roughly 1.8% theoretical cold coverage. `skipped_budget` is a normal documented outcome, +and `/status` publishes the three coverage percentages with every denominator so the real +sampling rate is visible rather than inferred. + +**2. There is no guarantee of complete upstream capture.** Pagination deepens a single +poll within the budget; it does not backfill. Events that rolled out of the feed's window +while the service was down are **not recoverable**, and no amount of configuration changes +that. + +**3. The budget is shared with anything else behind the same outbound IP.** The +unauthenticated limit is IP-keyed, so a co-tenant spends the same sixty requests an hour. +The ledger observes it — `budget.co_tenant_usage`, and `budget.co_tenant_pressure` when +the remaining quota reaches the reserve — and cannot prevent it. See +[Sharing the outbound IP](#sharing-the-outbound-ip). + +**4. One event source, in practice.** The event-source seam has two shipped +implementations (live and fixture), and the allowance formula already divides by source +count, but a second *live* source is a documented seam rather than shipped behaviour. + +**5. Raw payload retention is semantic, not byte-exact.** Stored as `jsonb`, so whitespace, +key order, and duplicate keys are lost; array order is preserved because it is meaningful +([ADR 0001](docs/adr/0001-jsonb-semantic-retention.md)). + +**6. No authentication.** A token would raise the ceiling from 60 to 5,000 requests an +hour and change the enrichment story entirely — which is the point of the constraint, not +an oversight. See the scaling path in +[`docs/DESIGN_BRIEF.md`](docs/DESIGN_BRIEF.md). + +**7. An enriched entity can be up to 24 hours stale.** `ACTOR_REFRESH_TTL_SECONDS` and +`REPOSITORY_REFRESH_TTL_SECONDS` default to 86400, and a refresh only spends budget when +no never-enriched candidate is eligible anywhere. + +**8. Extension C (object storage) was deliberately not attempted.** A decision with a +stated reason, not an omission — the remaining budget went to rate-limit correctness, +durability, and reviewer experience. ## Development AI-assisted development guidance for this repository lives in [`CLAUDE.md`](CLAUDE.md). -Architecture decision records live under [`docs/adr/`](docs/adr/). The design -brief lands with PR 12. +- [`docs/DESIGN_BRIEF.md`](docs/DESIGN_BRIEF.md) — the two-page architecture summary; start + here. +- [`docs/adr/`](docs/adr/) — twelve architecture decision records. +- [`docs/evidence/`](docs/evidence/) — dated first-party verifications of contested claims. +- [`docs/SUBMISSION_CHECKLIST.md`](docs/SUBMISSION_CHECKLIST.md) — the §16 quality gates as + a pre-flight checklist. +- [`IMPLEMENTATION_PLAN.md`](IMPLEMENTATION_PLAN.md) — the execution plan; Appendix E + records how the build diverged from it. ## License diff --git a/config/initializers/cors.rb b/config/initializers/cors.rb deleted file mode 100644 index 0c5dd99..0000000 --- a/config/initializers/cors.rb +++ /dev/null @@ -1,16 +0,0 @@ -# Be sure to restart your server when you modify this file. - -# Avoid CORS issues when API is called from the frontend app. -# Handle Cross-Origin Resource Sharing (CORS) in order to accept cross-origin Ajax requests. - -# Read more: https://github.com/cyu/rack-cors - -# Rails.application.config.middleware.insert_before 0, Rack::Cors do -# allow do -# origins "example.com" -# -# resource "*", -# headers: :any, -# methods: [:get, :post, :put, :patch, :delete, :options, :head] -# end -# end diff --git a/docs/DESIGN_BRIEF.md b/docs/DESIGN_BRIEF.md new file mode 100644 index 0000000..bb92725 --- /dev/null +++ b/docs/DESIGN_BRIEF.md @@ -0,0 +1,303 @@ +# Design brief — github-push-ingestor + +A Rails 8.1 API-only service that polls GitHub's public Events API, persists `PushEvent` +records with their raw payloads in PostgreSQL, and enriches the actors and repositories +they reference — all without a token, inside an unauthenticated ceiling of 60 requests per +hour per IP. + +> **The README says how. This brief says why. The ADRs hold the argument; this brief holds +> the decision.** + +Two pages is a deliberate budget: the figures and tables carry what prose would otherwise +spend, and every "why" is one sentence plus a link. The execution and traceability artifact +is [`IMPLEMENTATION_PLAN.md`](../IMPLEMENTATION_PLAN.md) — its pre-implementation revision +history lives in Git and in its Appendices A–D, and Appendix E records how the build +diverged from it. + +## The problem + +GitHub publishes a firehose of public activity at `/events`. The assignment is to ingest +the `PushEvent` slice of it durably, enrich it with actor and repository detail, and +survive restarts. + +The hard part is not volume. It is that the source is a **sliding window behind a hard +quota**. The feed retains roughly the last 300 events with a documented 30-second-to-6-hour +delivery latency and a 30-day retention window, and an unauthenticated caller gets 60 +requests per hour keyed to its outbound IP — shared with anything else behind that IP. +Every design decision here falls out of that arithmetic: what to poll, how often, what to +enrich, what to admit cannot be done. + +So this system is honest about being a **sampler, not a mirror**. It does not claim +complete upstream capture, and it does not claim complete enrichment coverage. What it does +claim, it proves — with a test, a constraint, or a dated transcript. + +## Architecture + +Two request paths — polling and enrichment — share one executor, and nothing outside that +chain calls GitHub. Polling takes a per-source advisory lock; enrichment never does. A +second, global advisory lock makes outbound concurrency exactly one, application-wide, so +the budget ledger is never racing itself. The lock order (source, then gate, never the +reverse) is enforced at runtime by `Github::LockOrder`, not merely documented. + +```mermaid +flowchart LR + subgraph oneshot["one-shot commands"] + BI["bin/ingest"] + BE["bin/enrich"] + end + subgraph worker["worker container — Solid Queue supervisor"] + PJ["PollEventSourceJob — 60s tick"] + RJ["ReconcilePendingEnrichmentsJob — 60s tick"] + EJ["EnrichActorJob / EnrichRepositoryJob"] + end + subgraph web["web container — read-only, never calls GitHub"] + API["/health/live · /health/ready · /status · /api/push_events"] + end + IR["Github::IngestionRunner
holds SourceLock for the whole cycle"] + ER["Github::EnrichmentRunner
age-out · fairness · lease · one entity"] + RX["Github::RequestExecutor
RequestGate → BudgetLedger → UrlPolicy → Transport"] + GH["api.github.com
unauthenticated · 60 req/hr per IP"] + subgraph pg["db container — PostgreSQL 16 on the pgdata named volume"] + APPDB[("app database
7 business tables")] + QDB[("queue database
Solid Queue")] + end + BI --> IR + PJ --> IR + BE --> ER + EJ --> ER + RJ -.->|"enqueues"| EJ + IR --> RX + ER --> RX + RX --> GH + IR -->|"PageWriter · PollState"| APPDB + ER -->|"EntityState"| APPDB + API -->|"read only"| APPDB + APPDB -.->|"post-commit enqueue"| QDB + QDB --> PJ + QDB --> EJ +``` + +Every path to GitHub funnels through one `RequestExecutor` node — that single arrow is the +whole rate-limit design. The `web` container has no arrow to GitHub at all, which is why +`/status` and `/health/*` can never spend budget: it is structural, not a promise. + +## Data model + +Seven business tables in three groups: source and run state (`event_sources`, +`ingestion_runs`), business records (`push_events`, `github_actors`, +`github_repositories`, `quarantined_events`), and one global ledger (`github_api_budget`). +Raw payloads are `jsonb` and retention is **semantic, not byte-exact** — content-equivalent +to what GitHub sent, with whitespace and key order lost and array order preserved +([ADR 0001](adr/0001-jsonb-semantic-retention.md)). Enrichment state lives on the shared +entity rows rather than per-event, because a thousand events referencing one actor are one +enrichment obligation, not a thousand. + +```mermaid +erDiagram + EVENT_SOURCES ||--o{ INGESTION_RUNS : "records" + GITHUB_ACTORS ||--o{ PUSH_EVENTS : "performed" + GITHUB_REPOSITORIES ||--o{ PUSH_EVENTS : "received" + EVENT_SOURCES { + text source_type + text status + text etag + timestamp cadence_due_at + timestamp poll_floor_until + timestamp retry_not_before_at + } + INGESTION_RUNS { + uuid run_id UK + bigint event_source_id FK + text status + int events_created + int duplicates_skipped + int events_quarantined + } + PUSH_EVENTS { + text github_event_id UK + bigint github_push_id + bigint github_actor_id FK + bigint github_repository_id FK + text ref + jsonb raw_payload + } + GITHUB_ACTORS { + bigint github_id UK + text login + text enrichment_status + timestamp latest_event_at + timestamp next_retry_at + } + GITHUB_REPOSITORIES { + bigint github_id UK + text full_name + text enrichment_status + timestamp latest_event_at + timestamp next_retry_at + } + QUARANTINED_EVENTS { + text payload_fingerprint UK + text github_event_id + text error_code + int occurrence_count + } + GITHUB_API_BUDGET { + int id + text window_status + int poll_used + int enrichment_used + timestamp global_blocked_until + } +``` + +The two tables with no edges are the point. `quarantined_events` has no foreign key because +a malformed event may be malformed *precisely because* it lacks the field a key would +reference — its identity is `payload_fingerprint` alone, a SHA-256 of compact UTF-8 JSON +with recursively sorted keys. `github_api_budget` is a `CHECK (id = 1)` singleton because +the unauthenticated quota is keyed to the outbound IP, not to a source. Note also that +`push_events`' foreign keys target `github_id`, GitHub's own identifier, not the surrogate +primary key. + +## The durability boundary + +**An event is accepted when its `push_events` row commits** — not when GitHub returns it, +not when a job is enqueued, not when a log line says so. Everything upstream of that commit +is retryable; everything downstream is derived. + +```text +At-least-once execution ++ Idempotent writes ++ Unique constraints += Effectively-once persisted outcomes +``` + +That is a database guarantee, not an application convention. `push_events` inserts with +`ON CONFLICT (github_event_id) DO NOTHING RETURNING id`, and entity activity updates happen +**only when `RETURNING` produced a row** — so a duplicate replay may refresh identity +fields but registers no activity and can never reactivate an entity a budget skip had +terminated. Transactions are per event, not per page, so one malformed envelope cannot +discard the events persisted beside it ([ADR 0005](adr/0005-at-least-once-with-idempotent-writes.md)). + +Restart safety needs no cleanup path because nothing needs cleaning. Session advisory locks +die with their session, so a crashed poller releases its source the moment its connection +drops. Entity leases are a `next_retry_at` timestamp that expires by arithmetic rather than +a lock someone must release. Pending enrichment work is the state of committed entity rows, +so `enqueue_after_transaction_commit` makes the enqueue a *hint* and an entity-scoped +reconciler rebuilds the work list from committed state every 60 seconds +([ADR 0008](adr/0008-post-commit-enqueue-and-entity-scoped-reconciliation.md)). Docker's +`restart: unless-stopped` handles the container. **This is not exactly-once execution and +the system never claims it is** — a job may run twice, and the second run changes nothing. + +## The request budget, and the 304 finding + +One derived formula, never a configured number: + +```text +poll_attempt_allowance = ceil(3600 / POLL_INTERVAL_SECONDS) + × MAX_PAGES_PER_POLL × ENABLED_LIVE_SOURCE_COUNT +enrichment_allowance = rate_limit − RATE_LIMIT_RESERVE − poll_attempt_allowance +actor_guarantee = floor(enrichment_allowance × ACTOR_ENRICHMENT_SHARE) +repository_guarantee = enrichment_allowance − actor_guarantee +``` + +| `MAX_PAGES_PER_POLL` | Poll allowance | Enrichment allowance | Actor / repository guarantee | +|---|---|---|---| +| 1 (default) | 12 | 40 | 20 / 20 | +| 2 | 24 | 28 | 14 / 14 | +| 3 | 36 | 16 | 8 / 8 | + +Against a limit of 60 with a reserve of 8. The process **refuses to boot** if polling would +leave no capacity for enrichment ([ADR 0004](adr/0004-class-aware-budget-ledger.md)). + +**The `304` finding.** GitHub's endpoint documentation contains a general statement that +`304` responses do not affect the rate limit, while its REST best-practices documentation +limits that exemption to correctly authorized requests. Dated unauthenticated probes showed +`x-ratelimit-used` increasing across a `304`. This implementation therefore budgets +unauthenticated conditional requests as one request. The asymmetry decides it: budgeting a +free `304` wastes one attempt an hour, while not budgeting a charged one overruns a +60-request window and blocks everything. ETag remains a bandwidth and correctness measure +here, never a quota saver. The transcript is the argument — +[`docs/evidence/2026-07-30-unauthenticated-304-quota-probe.md`](evidence/2026-07-30-unauthenticated-304-quota-probe.md). + +## Enrichment is a bounded sample + +One observed live page held ~92–95 `PushEvent` records with ~89 distinct actors and ~92 +distinct repositories. That is 181 cold entity requests per page, ~2,172 an hour at twelve +polls, against **40 available** — roughly 1.8% theoretical cold coverage. Partial coverage +is therefore the design, not a shortfall, and `skipped_budget` is a normal documented +outcome rather than a failure state. + +Repository candidates alone exceed the whole hourly allowance, so a naive repo-first policy +would starve actor enrichment to zero indefinitely — which Story 3 forbids. Hence per-class +guarantees with **eligibility-aware borrowing**: a class may spend past its guarantee only +when the other has no *currently eligible* candidate, and the ledger enforces the +arithmetic the fairness policy proposes, so a wrong answer produces a refused reservation +rather than an overspend ([ADR 0007](adr/0007-enrichment-fairness-shares-and-borrowing.md)). +Unbounded backlog growth is answered by three mechanisms together: an eligibility window +past which a candidate ages into `skipped_budget`, `skipped_budget` as a terminal state +that no retry sweeps back, and reactivation **only** on a genuinely new push event. The +backlog is bounded by construction, and `/status` publishes the real sampling rate so an +operator sees a sample rather than a mysteriously growing queue. + +Enrichment URLs arrive inside GitHub payloads, `Link` headers, and `Location` headers — +attacker-influenceable data — so the SSRF boundary is strict: HTTPS only, host exactly +`api.github.com`, no userinfo, no non-default port, no IP literals, and bounded redirects +each re-validated and separately debited. Every URL is rebuilt from validated components, +and the allowed host is a frozen constant rather than an environment variable, because a +deployment setting there would make the trust boundary configurable. Fixture mode fails +closed: an unknown URL is an error, never a live fallback +([ADR 0003](adr/0003-event-source-and-transport-seams.md)). + +## Tradeoffs and assumptions + +| Decision | Cost accepted | Record | +|---|---|---| +| `jsonb` semantic retention, not byte-exact | Whitespace, key order, and duplicate keys are lost | [ADR 0001](adr/0001-jsonb-semantic-retention.md) | +| Session advisory locks over `FOR UPDATE` row claims | Lock state is invisible to `SELECT`; needs its own observability | [ADR 0002](adr/0002-advisory-locks-and-request-gate.md) | +| At-least-once execution over exactly-once | Non-idempotent side effects repeat; duplicate work is spent work | [ADR 0005](adr/0005-at-least-once-with-idempotent-writes.md) | +| Five decomposed scheduling columns over one `next_poll_at` | More state to reason about; `next_poll_at` is a cache, never an input | [ADR 0006](adr/0006-decomposed-poll-deferral-state.md) | +| Fairness guarantees with borrowing, not hard caps | A borrowing class can consume the window when the other is idle | [ADR 0007](adr/0007-enrichment-fairness-shares-and-borrowing.md) | +| Solid Queue in a second database, not Kafka | Queue throughput bounded by PostgreSQL; no cross-service fan-out | [ADR 0012](adr/0012-solid-queue-over-kafka.md) | + +The load-bearing assumption is that the bottleneck is upstream quota rather than local +throughput. It holds at 60 requests an hour and would need re-examining at 5,000. + +## What was deliberately not built + +**Extension C (object storage) was not attempted** — a decision, not an oversight. The +remaining budget went to rate-limit correctness, durability, and reviewer experience, which +carry more signal than a fourth extension. + +**No authentication.** The 60-requests-per-hour ceiling is the entire design constraint; +removing it would remove the problem this submission is actually about. + +**No guarantee of complete upstream capture** and **no guarantee of complete enrichment +coverage.** Both are arithmetic consequences stated in the README's known limitations, not +gaps to be closed later. + +**No `/api/actors/:id`, `/api/repositories/:id`, or `/api/ingestion_runs`** — listed as +optional in the plan and left unbuilt rather than half-built. + +## Scaling path + +**An authenticated token does not tune the enrichment story; it deletes it.** At 5,000 +requests an hour, sampling becomes coverage, `skipped_budget` becomes a bug rather than a +state, and the fairness shares become an accounting curiosity. That is also the point where +Solid Queue's throughput ceiling starts to matter and where a broker would first earn its +place. The seams are already in place: `ENABLED_LIVE_SOURCE_COUNT` and runtime source +allocation ([ADR 0009](adr/0009-runtime-source-allocation-and-shared-ip-observability.md)) +mean adding a second event source is configuration plus one adapter, not a redesign. + +The API version is pinned to `2022-11-28` because every live probe behind this design ran +under it. Upgrading to `2026-03-10` is a deliberate follow-up gated on re-verifying payload +shape and re-running the `304` probe first — not on the version being available +([ADR 0011](adr/0011-pinned-api-version-2022-11-28.md)). + +## Where the detail lives + +| Artifact | What it holds | +|---|---| +| [`README.md`](../README.md) | How to run, verify, inspect, and reset it | +| [`IMPLEMENTATION_PLAN.md`](../IMPLEMENTATION_PLAN.md) | Execution and traceability; revision history in Appendices A–D, execution summary in E | +| [`docs/adr/`](adr/) | Twelve decisions, each with its context, cost, and consequences | +| [`docs/evidence/`](evidence/) | Dated first-party verifications of contested claims | diff --git a/docs/SUBMISSION_CHECKLIST.md b/docs/SUBMISSION_CHECKLIST.md new file mode 100644 index 0000000..b8c67f9 --- /dev/null +++ b/docs/SUBMISSION_CHECKLIST.md @@ -0,0 +1,204 @@ +# Submission checklist + +Repository: https://github.com/batbrainy/github-push-ingestor + +Verified against: ``, `` + +Pre-flight for `IMPLEMENTATION_PLAN.md` §16. **Every box is checked against the default +branch after PR 12 merges, from a fresh clone into an empty directory — never against a +working tree.** A working tree can pass gates a clone would fail: an untracked `.env`, a +`config/master.key`, a stale image, a warm database volume. + +--- + +## 1. Clean-checkout verification + +- [ ] `git clone` of the default branch into an empty directory; `git status --porcelain` + is empty +- [ ] No `.env`, no `config/master.key`, no token anywhere in the clone +- [ ] `docker compose build --no-cache --pull` succeeds — a genuinely cold image build with + `BUNDLE_FROZEN=1` against the committed `Gemfile.lock` +- [ ] `docker compose up --build` starts exactly `db`, `setup`, `web`, `worker` — and + nothing else +- [ ] `curl http://localhost:3000/health/ready` returns `{"status":"ok"}` +- [ ] `GITHUB_MODE=fixture docker compose run --rm ingest` reports **4 created, 3 + quarantined, 1 ignored** from an empty database +- [ ] `docker compose run --rm test` is green across both rspec invocations +- [ ] `docker compose down` then `up`, and `SELECT COUNT(*) FROM push_events` is unchanged +- [ ] No local Ruby, PostgreSQL, or `psql` was used at any point + +Transcript: `docs/evidence/-clean-checkout-verification.md` + +--- + +## 2. Functional gates — §16 + +- [ ] Public GitHub Events API works without a token — *live half of the clean-checkout run* +- [ ] Only `PushEvent` records are processed — `Github::Events::ProcessorRegistry`; the + corpus `WatchEvent` is ignored **and counted** +- [ ] Required fields are structured, typed, and `NOT NULL`; unknown payload fields + tolerated; 40- and 64-char SHAs accepted — `spec/db/schema_spec.rb`, + `spec/services/github/events/push_event_processor_spec.rb` +- [ ] Raw payload is retained (semantic retention, documented) — + [ADR 0001](adr/0001-jsonb-semantic-retention.md), `GET /api/push_events/:id` +- [ ] **Both** actor and repository enrichment demonstrably occur within their fairness + guarantees — `spec/services/github/enrichment/end_to_end_spec.rb`; fixture run gives + `complete 2 / permanent_failure 1` per class +- [ ] Duplicate ingestion is safe — and duplicate replays never reactivate skipped entities + — fixture replay: 4 duplicates absorbed, no `enrichment.reactivated` +- [ ] `Link`-header pagination is handled; every fetched page fully processed — + `spec/services/github/ingestion/page_loop_spec.rb`; `paginated` scenario +- [ ] Rate-limit behavior demonstrated: `304` quota accounting, class-aware ledger + enforcement, global-vs-class blocking, per-window bootstrap, scheduling rules — + `docs/evidence/2026-07-30-unauthenticated-304-quota-probe.md`, + `spec/stress/budget_ledger_spec.rb` +- [ ] Malformed data quarantined durably per the taxonomy (canonical fingerprints, + occurrence-counted) and does not terminate the batch — 3 rows, occurrences 3 → 6 on + replay, and 4 events persisted beside them + +--- + +## 3. Durability gates — §16 + +- [ ] PostgreSQL uses a named volume — `docker-compose.yml`, `spec/docker_compose_spec.rb` +- [ ] Docker restart policies recover crashed `db`/`web`/`worker` automatically (verified by + container kills) — `docs/evidence/2026-07-31-container-kill-recovery.md` +- [ ] Application restart preserves events — verification step 9 +- [ ] Worker restart preserves pending work — `spec/recovery/worker_crash_lease_spec.rb` +- [ ] An event committed before a crash remains recoverable — + `spec/recovery/crash_window_spec.rb` +- [ ] Advisory locks provably release on session death (tested) — + `spec/recovery/advisory_lock_session_death_spec.rb`, real `pg_terminate_backend` +- [ ] Duplicate jobs do not duplicate durable data — + `spec/recovery/duplicate_job_execution_spec.rb` +- [ ] Reconciliation recovers missing enrichment scheduling — + `spec/recovery/pending_enrichment_recovery_spec.rb` +- [ ] The enrichment backlog is bounded (eligibility window + `skipped_budget` + + distinct-event reactivation) — `spec/services/github/enrichment/age_out_spec.rb` + +--- + +## 4. Operability gates — §16 + +- [ ] Logs readable through `docker compose logs -f` at the default level — + README [Logs](../README.md#logs) +- [ ] Correlation fields (`run_id`, job ID) present — `app/jobs/application_job.rb`; the + trace is one hop +- [ ] `/health/live` and `/health/ready` are meaningful and never consume budget — + `spec/requests/health_spec.rb` +- [ ] `/status` reports window status, poll state, per-class ledger state, pending/skipped + counts, and coverage percentages by the defined formulas — without initiating GitHub + requests — `spec/requests/status_spec.rb`, `Github::Enrichment::Coverage` +- [ ] Retry behavior is visible — `github.retry_scheduled` / `github.retry_exhausted` at + the default level +- [ ] Failures contain actionable context — every failure line carries classification, + status, URL, attempt, and its `run_id` or entity id + +--- + +## 5. Reviewer-experience gates — §16 + +- [ ] Clean checkout works — §1 above +- [ ] No local Ruby or PostgreSQL installation is required +- [ ] Commands match the assignment +- [ ] Plain `docker compose up --build` starts exactly `db`, `setup`, `web`, `worker` — + `profiles: ["tools"]` on the other three +- [ ] `docker compose run --rm test` never touches the development databases (app or queue) + and never triggers the development `setup` service — `spec/docker_compose_spec.rb` +- [ ] Documentation is accurate; the README points to the plan and its appendix revision + record +- [ ] No secrets or token are required +- [ ] Tests are deterministic — WebMock denies net connect; no VCR; fixture rate-limit + resets are relative, so the corpus does not rot +- [ ] GitHub Project and issues show organized execution +- [ ] Pull requests are focused and linked to issues — every merged PR carries `Closes #` + +--- + +## 6. Final repository review — §16 + +- [ ] **No secrets** + + ```bash + bin/brakeman + bin/bundler-audit + git log -p | grep -inE 'ghp_|github_pat_|ghs_|gho_|BEGIN [A-Z]+ PRIVATE KEY' + ``` + +- [ ] **No personal access token** — `grep -rn "Authorization" app lib config` returns only + the SSRF-policy and header code +- [ ] **No stale documentation** + + ```bash + grep -rn "lands with PR\|available now\|Planned contents\|PR 12 (design brief)" README.md + ``` + + returns nothing. +- [ ] **No dead or speculative infrastructure** +- [ ] **No misleading guarantee of complete upstream event capture** — see §7 +- [ ] **No claim of exactly-once execution** — see §7 +- [ ] **No claim that enrichment coverage is complete** — see §7 +- [ ] **No failing or flaky tests** — the suite run three times consecutively green, + including a fixed-seed repeat + +--- + +## 7. Forbidden-claim scan + +```bash +grep -rniE "exactly.once|complete capture|complete(ly)? enrich" \ + README.md docs/ IMPLEMENTATION_PLAN.md CLAUDE.md +``` + +**The rule: every hit must be a negation.** A hit that asserts the claim fails the gate. + +Legitimate existing hits are the disclaimers themselves — `README.md`'s "does not claim +exactly-once execution" and "not complete enrichment coverage", `CLAUDE.md`'s "Never claim +or code against exactly-once", `IMPLEMENTATION_PLAN.md` §8's "The system does not claim +exactly-once execution" and §16's four `No …` bullets, and +[ADR 0005](adr/0005-at-least-once-with-idempotent-writes.md)'s "this is not exactly-once +execution, and the system must never claim it is". + +- [ ] Every hit is a negation + +--- + +## 8. Submission email — POST-MERGE + +> **Post-merge step.** Send only after PR 12 is merged into the default branch and §1 above +> passes against a fresh clone of that branch. Nothing in this repository sends it. + +The subject uses a plain ASCII hyphen-minus, matching `IMPLEMENTATION_PLAN.md` §13 and §16 +exactly. Do not let a formatter turn it into an en dash. + +```text +To: recruiter@strongmind.com +Subject: Full Stack Developer Candidate - Umang Brahmakshatriya + +Hello, + +Please find my submission for the Full Stack Developer take-home. + +Repository: https://github.com/batbrainy/github-push-ingestor + +It is a Rails 8.1 API-only service that ingests GitHub Push events from the +public Events API, persists raw and structured data in PostgreSQL, and +enriches actors and repositories inside an explicit unauthenticated request +budget. It runs from a clean checkout with one command: + + docker compose up --build + +No token, no local Ruby, and no local PostgreSQL are required. + +The README includes a step-by-step "How to verify it's working" walkthrough, +including a fully offline fixture scenario with exact expected counts. +docs/DESIGN_BRIEF.md is the two-page architecture summary; architecture +decisions are recorded under docs/adr/, and dated first-party verifications +are under docs/evidence/. + +Thank you for your time. + +Umang Brahmakshatriya +``` + +- [ ] Sent on `` diff --git a/docs/adr/0011-pinned-api-version-2022-11-28.md b/docs/adr/0011-pinned-api-version-2022-11-28.md new file mode 100644 index 0000000..ee8df86 --- /dev/null +++ b/docs/adr/0011-pinned-api-version-2022-11-28.md @@ -0,0 +1,84 @@ +# 11. Pinned REST API version `2022-11-28`, with `2026-03-10` as a gated follow-up + +Date: 2026-07-31 + +Status: Accepted + +## Context + +GitHub's REST API is versioned by a request header. `IMPLEMENTATION_PLAN.md` §2A pins +`X-GitHub-Api-Version: 2022-11-28` on every outbound request +(`app/services/github/request.rb:32-36`), and §14 asks for that pin to carry a record. + +Two version facts, verified 2026-07-29 and recorded in the plan's Appendix B: `2022-11-28` +is the version GitHub applies when no header is sent, and is supported until 2028-03-10; +`2026-03-10` is the latest published version. + +The pin is not inertia. Every quantitative claim this design rests on was measured under +`2022-11-28`: + +- The unauthenticated `304` quota finding + ([`docs/evidence/2026-07-30-unauthenticated-304-quota-probe.md`](../evidence/2026-07-30-unauthenticated-304-quota-probe.md)), + which is why the budget ledger debits conditional requests (ADR 0004). +- The observed page composition — roughly 92–95 `PushEvent` records in one 100-event page — + which produces §10's enrichment-demand arithmetic and therefore the fairness shares + (ADR 0007). +- The payload field set the tolerant parser accepts, including 40- and 64-character object + names in `payload.head` and `payload.before`. + +Sending a newer version header would invalidate the evidence behind all three at once, +silently, with no failing test to catch it — the fixture corpus is a recording of +`2022-11-28` responses, so the offline suite would stay green while live behaviour drifted. + +## Decision + +Pin `2022-11-28` as a frozen constant in `Github::Request::PROTOCOL_HEADERS`, not an +environment variable. + +The version is not configurable for the same reason the API host is not +(ADR 0003): it is a correctness parameter of the evidence this design cites, not a +deployment knob. An operator who could set it to `2026-03-10` from `.env` could invalidate +the budget arithmetic without changing a line of code or failing a test. + +**Upgrading to `2026-03-10` is a deliberate follow-up, gated on re-verification rather than +on availability.** The gate, in order: + +1. Re-run `script/probe_304.sh` under the new version and commit a dated transcript. If + `x-ratelimit-used` no longer increments across an unauthenticated `304`, ADR 0004's + debit rule changes and the allowance formula changes with it. +2. Re-capture a live `/events` page and diff its shape against the fixture corpus — + specifically that `PushEvent` still carries `payload.push_id`, `payload.ref`, + `payload.head`, `payload.before`, and that `actor`/`repo` still carry the `id`s the + foreign keys target. +3. Re-measure the `PushEvent` fraction of a page. §10's arithmetic assumes ~92–95 of 100. +4. Re-record the corpus under the new version, then change the constant. + +Only step 4 is a code change. Steps 1–3 are why the pin exists. + +## Consequences + +What this buys: + +- Every number in `docs/DESIGN_BRIEF.md` and the README traces to a probe run under the + version the code actually sends. The evidence and the implementation cannot disagree. +- The fixture corpus is a faithful recording rather than an approximation, which is what + makes `GITHUB_MODE=fixture` a real second transport implementation rather than a mock. +- No operator can silently invalidate the rate-limit design through configuration. + +What it costs, stated plainly: + +- The system does not benefit from whatever `2026-03-10` improved. Nothing in this + submission needs it, but that is an assertion about today's requirements, not a + general one. +- The pin has an expiry. `2022-11-28` is supported until 2028-03-10; after that the + upgrade is forced rather than chosen, and the four-step gate above has to run under time + pressure instead of at leisure. +- Because the version lives in a frozen constant, upgrading requires a code change, a + release, and a re-recorded corpus — deliberately more friction than editing `.env`. + +The suite asserts the header rather than the version's effects, which is the honest +boundary: `spec/services/github/request_spec.rb:64` and +`spec/services/github/transports/faraday_spec.rb:36` prove every outbound request carries +`2022-11-28`, and `spec/support/shared_examples/github_transport.rb:55` holds both +transports to it. No offline test can prove GitHub's behaviour under any version — that is +what the dated probe transcripts are for. diff --git a/docs/adr/0012-solid-queue-over-kafka.md b/docs/adr/0012-solid-queue-over-kafka.md new file mode 100644 index 0000000..f88f8db --- /dev/null +++ b/docs/adr/0012-solid-queue-over-kafka.md @@ -0,0 +1,92 @@ +# 12. Solid Queue in a second PostgreSQL database, not Kafka + +Date: 2026-07-31 + +Status: Accepted + +## Context + +"Ingest events from a feed, persist them, enrich them in the background" is a shape that +invites a broker. Kafka is the reflexive answer, and `IMPLEMENTATION_PLAN.md` §14 asks for +a record of why it was not selected — not because Kafka is a bad technology, but because +choosing it here would have been a decision made by pattern-matching rather than by +arithmetic. + +The arithmetic: + +- The source is **one polled HTTP endpoint** under an unauthenticated ceiling of 60 + requests per hour per IP. At the default cadence the system issues twelve poll requests + an hour and reads at most ~100 events per page. +- Peak sustained ingest is therefore on the order of **a few hundred events an hour**, + bounded by upstream quota rather than by anything downstream. +- Enrichment is bounded by the same 60-request ceiling — around 40 requests an hour after + the poll allowance and reserve, which §10 states plainly is a *sample* of demand rather + than coverage of it. + +Nothing in that profile is throughput-constrained. The bottleneck is a third party's rate +limit, and no amount of broker capacity moves it. + +## Decision + +Use Solid Queue, backed by its own PostgreSQL database +(`github_push_ingestor_queue_development`) inside the same container as the business +database (`config/database.yml`). + +Three reasons, in order of weight: + +1. **A broker would carry no durability the business tables do not already carry.** The + durability boundary is the committed `push_events` row (ADR 0005), and pending + enrichment work is not a message — it is the state of a committed entity row + (`enrichment_status`, `next_retry_at`). `ReconcilePendingEnrichmentsJob` rebuilds the + work list from those rows every 60 seconds, which is why ADR 0008 can call the enqueue + a *hint*. Delete every queued job and the system loses nothing: the next tick + re-derives the same work. A broker would be a second, weaker copy of a record + PostgreSQL already holds under constraints. +2. **One database is one backup, one restore, one transaction boundary.** The queue lives + beside the business tables in the same PostgreSQL instance, so a `pg_dump` captures + both consistently, `docker compose exec db psql` inspects the queue with the same tool + as everything else, and `enqueue_after_transaction_commit` gives a real ordering + guarantee between a committed event and the job that reacts to it. With a broker, + "committed but not enqueued" and "enqueued but not committed" both become states + someone has to reason about. +3. **A component a reviewer has to run is a component that has to earn its place.** + `IMPLEMENTATION_PLAN.md` §17 states the target: small enough to understand, complete + enough to trust. Kafka adds a broker, a coordination layer, its own durability + configuration, and its own failure modes to a system whose actual hard problem — + spending 60 requests an hour correctly across two competing consumers — is entirely + inside PostgreSQL. + +Kafka, Redis, and Sidekiq are all named in §2's out-of-scope list for this reason, and +`CLAUDE.md` forbids reintroducing them without first amending the plan. + +## Consequences + +What this buys: + +- `docker compose up --build` starts exactly four containers: `db`, `setup`, `web`, + `worker`. There is no broker to provision, tune, or explain. +- Queue state is inspectable with SQL, which is what makes the crash-recovery drill in the + README runnable by hand — truncate `solid_queue_jobs`, restart the worker, and watch + reconciliation rebuild the work from committed rows. +- The job system inherits PostgreSQL's durability rather than having its own. There is one + answer to "what survives a crash", not two. + +What it costs, stated plainly: + +- **Queue throughput is bounded by PostgreSQL.** Solid Queue polls tables; it will not + match a broker at high message rates. Irrelevant at ~12 polls and ~40 enrichment + requests an hour, and it would stop being irrelevant the moment an authenticated token + raised the ceiling to 5,000 requests an hour — that is the scaling point where this + decision deserves re-examination, and it is named in `docs/DESIGN_BRIEF.md`. +- The queue competes with business queries for the same PostgreSQL instance's connections + and I/O. Mitigated by the separate database and by `config/queue.yml` keeping its thread + count inside `RAILS_MAX_THREADS`, but not eliminated. +- There is no fan-out to other consumers, no replay of a durable log, and no + cross-service event bus. If a second service ever needed this event stream, that is a + real reason to revisit — the current answer would be to read `push_events`, which works + for one reader and not for many. + +`spec/queue/solid_queue_integration_spec.rb` and `spec/queue/configuration_spec.rb` hold +the queue to its declared configuration, and `spec/recovery/pending_enrichment_recovery_spec.rb` +asserts the property that makes this decision safe: with the queue emptied, committed +entity rows are enough to rebuild the pending work. diff --git a/spec/requests/health_spec.rb b/spec/requests/health_spec.rb index f7635d7..b5c22ab 100644 --- a/spec/requests/health_spec.rb +++ b/spec/requests/health_spec.rb @@ -1,6 +1,42 @@ require "rails_helper" RSpec.describe "Health endpoints", type: :request do + # §16 requires both endpoints to be "meaningful and never consume budget". Meaningful is + # covered by the examples below — /health/ready really does fail on an unreachable + # database and on pending migrations. The never-consume half needs its own assertions, + # because it is the half that fails *silently*: a health check that quietly spent a + # request would still return 200, and a container healthcheck polls it every few seconds. + # + # These mirror spec/requests/status_spec.rb, which pins the same guarantee for /status. + describe "the guarantee that a health check costs nothing (plan §11)" do + %w[/health/live /health/ready].each do |path| + it "initiates no GitHub request from #{path}" do + transport = fixture_transport + allow(Github).to receive(:transport).and_return(transport) + expect(Github).not_to receive(:executor) + + get path + + expect(response).to have_http_status(:ok) + expect(transport.requests).to be_empty + end + end + + # Belt and braces: whatever the implementation reaches for, no statement it issues may + # write. Github::BudgetLedger#bootstrap! is public and inserts even when it inserts + # nothing, so a readiness probe that reached for the ledger would create from a health + # path the very row a reservation owns. + it "issues no write statement at all" do + expect(write_statements { get "/health/ready" }).to be_empty + expect(write_statements { get "/health/live" }).to be_empty + end + + it "does not create the ledger row or provision an event source" do + expect { get "/health/ready" }.to not_change(GithubApiBudget, :count).from(0) + .and not_change(EventSource, :count).from(0) + end + end + describe "GET /health/live" do it "reports the process as live" do get "/health/live" From 6eab84c6ce6b0660cc124b7b2d1c4012fc127222 Mon Sep 17 00:00:00 2001 From: Umang Date: Fri, 31 Jul 2026 11:22:45 -0500 Subject: [PATCH 2/3] Fix cold-start `docker compose up --build` under Compose Bake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by PR 12's own clean-checkout verification, which is the deliverable that exists to catch exactly this. From a cold image, `docker compose up --build` — the assignment's first command, §14's required-commands block, and the README's Quick start — failed and started zero containers: target worker: failed to solve: image "docker.io/library/github-push-ingestor-app:latest": already exists Compose Bake, on by default in Docker Desktop, makes every service with a `build:` its own bake target. Six services declared `build: .` against the same `image:` tag, so the targets raced to export it. It reproduces only when the image is absent — which is a reviewer's first command and no other run, which is why every previous run passed. Fix: `setup` is the only service on the `up` path that builds; `web` and `worker` reference the tag and already wait on `setup: service_completed_successfully`, and Compose completes the build phase before the create phase. The `tools` one-shots keep their own build — each is invoked alone by `docker compose run`, a single target that cannot collide — plus `pull_policy: build`, so Compose stops attempting a registry pull of a tag that is local-only by construction. spec/docker_compose_spec.rb now asserts the invariant that keeps this working rather than the one that broke it: exactly one build among the services a plain `up` starts, the buildless two waiting on it, and each one-shot able to build for itself. Also: the README's test command gains `--build`, because the code is baked into the image rather than mounted — running the suite against a stale image tests the code the image was built from, which is green and meaningless. Suite: 1728 examples + 10 stress, 0 failures, against a rebuilt image. Co-Authored-By: Claude Opus 5 (1M context) --- IMPLEMENTATION_PLAN.md | 1 + README.md | 11 +++++++++-- docker-compose.yml | 30 +++++++++++++++++++++++++++-- spec/docker_compose_spec.rb | 37 ++++++++++++++++++++++++++++++++++-- spec/requests/health_spec.rb | 13 ++++++++++--- 5 files changed, 83 insertions(+), 9 deletions(-) diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index b2f77ed..1cecb37 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -1310,6 +1310,7 @@ The plan held. Every P0 story and extension shipped, no descope rung was used, a | What the plan said | What was built | Why | Record | |---|---|---|---| +| Section 16 gates on “plain `docker compose up --build` starts exactly `db`, `setup`, `web`, `worker`” | `web` and `worker` no longer declare a `build:`; `setup` builds the shared image and they wait on it, while the `tools` one-shots keep their own build plus `pull_policy: build` | **The clean-checkout verification found the gate was false.** Compose Bake — on by default in Docker Desktop — makes every service with a `build:` its own bake target, and targets exporting the same `image:` tag race. From a cold image the reviewer's first command failed with `image "github-push-ingestor-app:latest": already exists` and started **zero** containers. It reproduces only when the image is absent, so every prior run on a warm machine passed. This is the defect the deliverable exists to catch | `docker-compose.yml`, `spec/docker_compose_spec.rb` | | Section 15 step 8 verifies restart policies with `docker kill` | `script/verify_recovery.sh` performs **both** the documented `docker kill` and a real in-container process crash, and reports both outcomes separately | `docker kill` is an API stop, and `restart: unless-stopped` is *defined* to skip a container the daemon recorded as manually stopped. The plan's own command cannot exercise the policy it verifies. Substituting the kill that works and staying quiet about it would have been the dishonest fix | [`docs/evidence/2026-07-31-container-kill-recovery.md`](docs/evidence/2026-07-31-container-kill-recovery.md), README “Crash recovery, verified” | | `ENABLED_LIVE_SOURCE_COUNT` is the allowance formula's source-count input | Demoted to a **fallback**. The formula counts enabled, in-service `event_sources` rows of the running mode at window initialization and rollover, and logs `budget.source_allocation_drift` when the two disagree | A configured count that drifts from the table silently mis-sizes every allowance. Boot validation still reads no database, so the refuse-to-boot check is unchanged | ADR 0009 | | Secondary-limit backoff is “≥ 1 minute with exponential backoff when `Retry-After` is absent” | A persisted `github_api_budget.consecutive_secondary_limits` counter escalates 60 → 120 → 240s capped at one hour, **survives window rollover**, and is cleared by one clean response | Secondary limits are IP-scoped, not window-scoped. A counter that reset with the window would restart the ladder at 60s every hour against a limit that had not relented | ADR 0010 | diff --git a/README.md b/README.md index ff5a168..e3515dc 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ Run the test suite (isolated `*_test` databases; never touches the development databases): ```bash -docker compose run --rm test +docker compose run --rm --build test ``` That runs `rspec` twice: the suite, and then [`spec/stress`](spec/stress/) — the threaded @@ -121,6 +121,11 @@ budget-ledger stress specs, which open several real PostgreSQL sessions and ther their own process ([`spec/stress/README.md`](spec/stress/README.md) explains why). CI runs the same two steps. +**`--build` is not optional if you have edited anything.** The application code is baked +into the image rather than bind-mounted, so `docker compose run --rm test` against a stale +image silently tests the code the image was built from — green, and meaningless. On an +unmodified checkout the flag costs a cached no-op. + Rails, Active Job and application logs are one structured JSON stream; PostgreSQL and Puma startup output remain their own plain-text formats: @@ -275,7 +280,9 @@ scenarios are in the [fixture scenario matrix](#fixture-scenario-matrix). docker compose run --rm test ``` -Two `rspec` invocations against isolated `*_test` databases, the same two CI runs. +Two `rspec` invocations against isolated `*_test` databases, the same two CI runs. On an +unmodified checkout that is the whole story; **if you have edited anything, add `--build`** +— the code is baked into the image, not mounted. If something looks wrong, [Troubleshooting and reset](#troubleshooting-and-reset) has a symptom table and a three-level reset ladder. diff --git a/docker-compose.yml b/docker-compose.yml index f6375e4..7a56319 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -78,6 +78,20 @@ services: # One-shot: prepares BOTH the primary and queue databases declared in # config/database.yml, so web and worker never race concurrent db:prepare # runs. + # + # It is also the ONE service on the `up` path that declares a build, and that is + # load-bearing rather than tidiness. Compose Bake (the Docker Desktop default) turns + # every service with a `build:` into its own bake target, and targets that export the + # same `image:` tag race each other: a cold `docker compose up --build` died with + # `failed to solve: image "github-push-ingestor-app:latest": already exists` and started + # zero containers. It only reproduced on a machine where the image did not already + # exist — which is exactly a reviewer's first command, and no other run. + # + # `web` and `worker` therefore reference the tag and never build it. Both wait on + # `setup: service_completed_successfully`, and Compose finishes the build phase before + # the create phase, so the image is always present by the time they are created. + # The one-shots under the `tools` profile keep their own `build:` because each is + # invoked alone by `docker compose run`, which is a single target and cannot collide. setup: build: . image: github-push-ingestor-app @@ -89,7 +103,7 @@ services: condition: service_healthy web: - build: . + # No `build:` — see the note on `setup`. Built by `setup`, which this waits on. image: github-push-ingestor-app restart: unless-stopped # Puma directly (no `bin/rails server` pid file) so `unless-stopped` @@ -129,7 +143,7 @@ services: # a busy one would have to boot Rails on every interval; Solid Queue's own # heartbeats in solid_queue_processes are the durable evidence instead. worker: - build: . + # No `build:` — see the note on `setup`. Built by `setup`, which this waits on. image: github-push-ingestor-app restart: unless-stopped command: ["bin/jobs"] @@ -161,6 +175,10 @@ services: ingest: build: . image: github-push-ingestor-app + # pull_policy: build — this tag is local-only and never published, so Compose + # attempting a registry pull first is a guaranteed-failing round trip that + # prints "pull access denied" before falling back to the build. + pull_policy: build profiles: ["tools"] restart: "no" entrypoint: ["bin/ingest"] @@ -184,6 +202,10 @@ services: enrich: build: . image: github-push-ingestor-app + # pull_policy: build — this tag is local-only and never published, so Compose + # attempting a registry pull first is a guaranteed-failing round trip that + # prints "pull access denied" before falling back to the build. + pull_policy: build profiles: ["tools"] restart: "no" entrypoint: ["bin/enrich"] @@ -207,6 +229,10 @@ services: test: build: . image: github-push-ingestor-app + # pull_policy: build — this tag is local-only and never published, so Compose + # attempting a registry pull first is a guaranteed-failing round trip that + # prints "pull access denied" before falling back to the build. + pull_policy: build profiles: ["tools"] restart: "no" command: ["bash", "-c", "bin/rails db:test:prepare && bundle exec rspec && bundle exec rspec spec/stress"] diff --git a/spec/docker_compose_spec.rb b/spec/docker_compose_spec.rb index 3cf2afd..2cbd44d 100644 --- a/spec/docker_compose_spec.rb +++ b/spec/docker_compose_spec.rb @@ -362,12 +362,45 @@ def unprofiled describe "the shared application image" do let(:app_services) { %w[setup web worker ingest enrich test] } - it "builds one image and runs every application service from it" do + it "runs every application service from one image" do app_services.each do |name| - expect(services.fetch(name).fetch("build")).to eq(".") expect(services.fetch(name).fetch("image")).to eq("github-push-ingestor-app") end end + + # The invariant that keeps a reviewer's first command working. Compose Bake — on by + # default in Docker Desktop — makes every service with a `build:` its own bake target, + # and two targets exporting the same `image:` tag race: a cold `docker compose up + # --build` failed with `image "github-push-ingestor-app:latest": already exists` and + # started nothing. It reproduces only when the image is absent, so it is invisible on + # every machine except the one that matters. + # + # `up` starts setup, web and worker; exactly one of them may declare a build. + it "declares exactly one build among the services a plain `up` starts" do + builders = %w[setup web worker].select { |name| services.fetch(name).key?("build") } + + expect(builders).to eq(%w[setup]) + end + + # web and worker are safe without a build only because they wait for the service that + # has one. Without this, a cold `up` would try to pull a tag that was never published. + it "makes the buildless services wait for the one that builds" do + %w[web worker].each do |name| + expect(services.fetch(name).fetch("depends_on").fetch("setup")) + .to include("condition" => "service_completed_successfully") + end + end + + # Each `tools` one-shot is invoked alone by `docker compose run`, so it is a single + # bake target and cannot collide — it keeps its own build and must, because nothing + # else builds the image on that path. pull_policy stops Compose from attempting a + # registry pull of a tag that is local-only by construction. + it "lets each one-shot build for itself without reaching for a registry" do + %w[ingest enrich test].each do |name| + expect(services.fetch(name).fetch("build")).to eq(".") + expect(services.fetch(name).fetch("pull_policy")).to eq("build") + end + end end # The same rule spec/network_boundary_spec.rb applies to the live rate-limit probe under diff --git a/spec/requests/health_spec.rb b/spec/requests/health_spec.rb index b5c22ab..fdfba00 100644 --- a/spec/requests/health_spec.rb +++ b/spec/requests/health_spec.rb @@ -31,9 +31,16 @@ expect(write_statements { get "/health/live" }).to be_empty end - it "does not create the ledger row or provision an event source" do - expect { get "/health/ready" }.to not_change(GithubApiBudget, :count).from(0) - .and not_change(EventSource, :count).from(0) + # Github::BudgetLedger#bootstrap! is public and inserts even when it inserts nothing, + # so a readiness probe that reached for the ledger would create from a health path the + # very row a reservation owns. Its mirror: reaching for SourceProvisioner to decide + # whether there is anything to be ready *for* would provision a source from a GET. + it "does not create the ledger row it would report on" do + expect { get "/health/ready" }.not_to change(GithubApiBudget, :count).from(0) + end + + it "does not provision an event source" do + expect { get "/health/ready" }.not_to change(EventSource, :count).from(0) end end From efc610b033971b65a1abb852defe232aceffade3 Mon Sep 17 00:00:00 2001 From: Umang Date: Fri, 31 Jul 2026 11:36:44 -0500 Subject: [PATCH 3/3] Clean-checkout verification transcript, and the live-mode hazard it exposed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commits the dated first-party transcript for §13's clean-checkout deliverable: docs/evidence/2026-07-31-clean-checkout-verification.md. Run from a fresh clone into an empty directory, on a host with no image for this project, against 6eab84c. Both halves: - Live: one unauthenticated poll persisted 96 push events from a page of 100, 4 non-push ignored, ledger bootstrapped from GitHub's own headers, and both entity classes enriched. 3 requests of 60 spent. This is the only thing that can prove §16's first functional gate. - Fixture, from an empty volume: 4 / 3 / 3 / 3, complete 2 / permanent_failure 1 per class, replay absorbing 4 duplicates with occurrences 3 -> 6 and no reactivation, and verify_recovery.sh's 19 checks in *absolute* count mode for the first time — the committed kill transcript had to run in delta. - Suite 1728 + 10, 0 failures, repeated with --seed 4242. Test isolation measured rather than asserted: both development databases unchanged across a full test run, with the poller stopped so the counts could not drift. The transcript documents the cold-build defect it found (fixed in 6eab84c) rather than quietly presenting a passing run, because the failure is the strongest argument for why the deliverable exists. Also documents one hazard the run exposed: bringing a killed container back means `docker compose up -d `, which recreates it from the current shell environment, where GITHUB_MODE defaults to live. A reviewer who starts in fixture mode and runs the recovery script ends up polling real GitHub and spending real quota with nothing saying so. README now says so, and gives the one command that puts it back. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 15 + docs/SUBMISSION_CHECKLIST.md | 5 +- .../2026-07-31-clean-checkout-verification.md | 268 ++++++++++++++++++ 3 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 docs/evidence/2026-07-31-clean-checkout-verification.md diff --git a/README.md b/README.md index e3515dc..4b82a8e 100644 --- a/README.md +++ b/README.md @@ -284,6 +284,10 @@ Two `rspec` invocations against isolated `*_test` databases, the same two CI run unmodified checkout that is the whole story; **if you have edited anything, add `--build`** — the code is baked into the image, not mounted. +A transcript of this whole walk, run from a fresh clone on a machine with no image, is at +[`docs/evidence/2026-07-31-clean-checkout-verification.md`](docs/evidence/2026-07-31-clean-checkout-verification.md) +— including the live half, which is what shows the public API works with no token. + If something looks wrong, [Troubleshooting and reset](#troubleshooting-and-reset) has a symptom table and a three-level reset ladder. @@ -1351,6 +1355,17 @@ One reading tip for §15 step 8's `docker compose ps` output: `web`'s container curls `/health/live`, which never touches the database, so `web` stays green throughout a `db` kill. `/health/ready` is the observable that flips. +**The stack comes back in live mode, so put it back offline when you are done.** Bringing a +killed container back means `docker compose up -d `, which recreates it from the +*current* shell environment — and `GITHUB_MODE` defaults to `live` there. A reviewer who +started the stack with `GITHUB_MODE=fixture` and then ran the script ends up with a worker +polling real GitHub and spending real quota. Check it and reset it: + +```bash +docker compose exec worker printenv GITHUB_MODE # `live` after a recovery run +GITHUB_MODE=fixture docker compose up -d --force-recreate worker +``` + Recovery is also watchable by hand in under a minute — stop the worker, put the entities back into `pending`, empty the queue (the crash), and start it again: diff --git a/docs/SUBMISSION_CHECKLIST.md b/docs/SUBMISSION_CHECKLIST.md index b8c67f9..d1ae763 100644 --- a/docs/SUBMISSION_CHECKLIST.md +++ b/docs/SUBMISSION_CHECKLIST.md @@ -27,7 +27,10 @@ working tree.** A working tree can pass gates a clone would fail: an untracked ` - [ ] `docker compose down` then `up`, and `SELECT COUNT(*) FROM push_events` is unchanged - [ ] No local Ruby, PostgreSQL, or `psql` was used at any point -Transcript: `docs/evidence/-clean-checkout-verification.md` +Pre-merge run against `6eab84c`: +[`docs/evidence/2026-07-31-clean-checkout-verification.md`](evidence/2026-07-31-clean-checkout-verification.md). +It found and fixed a defect that made `docker compose up --build` fail from a cold image — +which is why this section is checked again, from a fresh clone, after the merge. --- diff --git a/docs/evidence/2026-07-31-clean-checkout-verification.md b/docs/evidence/2026-07-31-clean-checkout-verification.md new file mode 100644 index 0000000..882f24f --- /dev/null +++ b/docs/evidence/2026-07-31-clean-checkout-verification.md @@ -0,0 +1,268 @@ +# Clean-checkout verification + +Date: 2026-07-31 + +Status: First-party observation + +Revision verified: `6eab84c6ce6b0660cc124b7b2d1c4012fc127222` +(branch `issue-22-reviewer-documentation`; the post-merge run against the default branch is +a separate, later step — see [`docs/SUBMISSION_CHECKLIST.md`](../SUBMISSION_CHECKLIST.md) §1) + +```text +Docker version: 28.3.0 +Compose version: v2.38.1-desktop.1 +Image store: containerd (io.containerd.snapshotter.v1) +Compose Bake: enabled (Docker Desktop default; COMPOSE_BAKE unset) +Host: Darwin 25.5.0 arm64 +Clone: git clone https://github.com/batbrainy/github-push-ingestor.git + into an empty directory, no .env, no config/master.key +Volume: github-push-ingestor_pgdata created fresh at 2026-07-31T16:29:55Z +``` + +## Why this verification exists + +`IMPLEMENTATION_PLAN.md` §13 makes clean-checkout verification a PR 12 deliverable and §16 +makes "clean checkout works" a gate, but nothing in CI or the suite can prove it. +`spec/docker_compose_spec.rb` asserts what the YAML *declares*; only a real run shows four +containers actually starting. And only a clone into an empty directory, on a host with no +image, proves the "no secrets, no local toolchain" claim — a working tree can pass gates a +clone would fail, on an untracked `.env`, a `config/master.key`, or a warm image. + +That last difference is not hypothetical here. **It is what this verification caught.** + +## The finding + +**`docker compose up --build` — the assignment's first command — failed from a cold image +and started zero containers.** + +```text +target worker: failed to solve: +image "docker.io/library/github-push-ingestor-app:latest": already exists +``` + +```text +$ docker compose ps -a +NAME IMAGE COMMAND SERVICE CREATED STATUS PORTS +(empty) +``` + +Compose Bake, enabled by default in Docker Desktop, makes every service declaring a +`build:` its own bake target. Six services declared `build: .` against the same +`image: github-push-ingestor-app`, so the targets raced to export one tag and the losers +aborted the whole `up`. + +It reproduces **only when the image is absent** — which is a reviewer's first command and +no other run. Every prior run in this project's history happened on a machine where the +image already existed, which is why a spec-guarded, deliberate design property had been +silently broken for a reviewer the entire time. + +Fixed in `6eab84c`: `setup` is now the only service on the `up` path that builds; `web` and +`worker` reference the tag and already wait on `setup: service_completed_successfully`. +The `tools` one-shots keep their own build — each is invoked alone by `docker compose run`, +a single target that cannot collide — plus `pull_policy: build`, so Compose stops +attempting a registry pull of a tag that is local-only by construction. +`spec/docker_compose_spec.rb` now asserts that invariant. + +Everything below was then verified against the fixed revision. + +## What else was measured + +**No host toolchain is used.** This host does have a Ruby, and it is the wrong one — which +makes the point better than an absent one would: + +```text +$ ruby --version +ruby 4.0.5 (2026-05-20 revision 64336ffd0e) +PRISM [arm64-darwin25] +$ cat .ruby-version +ruby-3.4.10 +$ bundle check +The following gems are missing + * rails (8.1.3.1) + * pg (1.6.3) +$ which psql +psql not found +``` + +The host cannot run this project. It ran anyway, because Ruby 3.4.10 and every gem live in +the image. Cold build: 992 MB, `BUNDLE_FROZEN=1` against the committed `Gemfile.lock`. + +**Startup order, from an empty volume**, exactly as the README documents: + +```text +db → healthy → setup → Exited (0) → web + worker + +db Up 7 seconds (healthy) +setup Exited (0) +web Up (health: starting) +worker Up +``` + +Four services. No others: `ingest`, `enrich` and `test` sit behind the `tools` profile. + +**Health endpoints answered immediately** — `/health/live` and `/health/ready` both +`{"status":"ok"}`. + +### The live half — §16's first functional gate + +One unauthenticated poll against the real API, from a clone with no token and no `.env`: + +```text +{"event":"budget.window_initialized","limit":60,"reserve":8,"poll_allowance":12, + "enrichment_allowance":40,"actor_guarantee":20,"repository_guarantee":20, + "rate_limit_resource":"core","rate_limit_limit":60,"rate_limit_remaining":59, + "rate_limit_used":1,"rate_limit_reset_at":"2026-07-31T17:24:00Z","poll_used":1} + +{"event":"ingestion.run_completed","run_id":"0b6d7cf1-5210-42ff-9d25-34a985585e3c", + "duration_ms":1057.1,"run_status":"completed","stop_reason":"page_cap","pages_fetched":1, + "events_received":100,"push_events_seen":96,"events_created":96,"duplicates_skipped":0, + "events_quarantined":0,"events_ignored":4,"events_failed":0} + +{"event":"enrichment.completed","entity_type":"repository","github_id":937042218, + "pool":"pending","classification":"ok","entity_status":"complete","enrichment_attempt":1} +{"event":"enrichment.completed","entity_type":"actor","github_id":148442705, + "pool":"pending","classification":"ok","entity_status":"complete","enrichment_attempt":1} +``` + +96 push events persisted from one page of 100, four non-push events ignored and counted, +and **both** entity classes enriched. The ledger bootstrapped from GitHub's own headers +rather than a discovery request. Total spend for the live half: 3 requests of 60; the +window was back to `60/60` by the end of the session. + +Third-party identifiers above are numeric GitHub ids only. Logins, display logins, full +names, API URLs and avatar URLs are redacted from this transcript under the same rule +`script/verify_recovery.sh` applies — names kept, values removed. + +### The fixture half — the documented absolutes + +From an empty volume, `GITHUB_MODE=fixture`: + +```text + push_events | actors | repositories | quarantined | occurrences + 4 | 3 | 3 | 3 | 3 +``` + +Exactly the numbers the README documents. After `enrich --limit 6`: + +```text + class | enrichment_status | count + actor | complete | 2 + actor | permanent_failure | 1 + repository | complete | 2 + repository | permanent_failure | 1 +``` + +The two `permanent_failure` rows are the corpus's deliberate `404` on event +`58000000008` — a dead enrichment target must fail the *entity* and leave the source +running. + +**Replay, past the 60-second `X-Poll-Interval` floor** (§15 step 7): + +```text +Push events created: 0 +Duplicates skipped: 4 +Events quarantined: 3 + + push_events | occurrences | skipped + 4 | 6 | 0 +``` + +Four duplicates absorbed, occurrence counts 3 → 6, `push_events` unchanged — and +`grep enrichment.reactivated` over the whole log returned nothing, with the +`skipped_budget` count still 0. Both halves of §15 step 7, not just the countable one. + +### Crash recovery (§15 steps 8–10) + +`script/verify_recovery.sh --confirm` on the empty-volume stack: **19 checks, all passed**, +and for the first time in `Count mode: absolute` rather than delta — the committed +transcript at +[`2026-07-31-container-kill-recovery.md`](2026-07-31-container-kill-recovery.md) had to run +against a populated database and says so. The `pgdata` volume's `CreatedAt` was identical +before and after, so the surviving records survived the kills rather than being recreated. + +### Tests (§15 step 11) + +```text +1728 examples, 0 failures # the suite + 10 examples, 0 failures # spec/stress +``` + +Repeated with `--seed 4242` on both invocations: identical, `0 failures`. + +**Test isolation, measured rather than asserted** — the poller was stopped first so the +counts could not move on their own: + +```text +BEFORE dev push_events=94 queue jobs=10 +AFTER dev push_events=94 queue jobs=10 +PASS — the test run touched neither development database +setup Exited (0) # not re-triggered by the test service +``` + +## What this does not show + +- **One host, one date, one Docker version.** The Bake collision is specific to Compose + Bake plus the containerd image store; a host with Bake disabled would never have seen it, + which is exactly why it survived so long. +- **The live half is one poll from one IP** whose 60-request budget is shared with any + co-tenant behind the same address. It shows the API is reachable without a token. It + shows nothing about sustained behaviour, and nothing about authenticated requests — this + project has no token and cannot test that case. +- **The fixture counts are properties of the committed corpus**, not of GitHub. They prove + the pipeline is deterministic; they prove nothing about what the live feed contains. +- **Nothing here proves complete upstream capture.** 96 push events from one page is one + sample of a sliding window with 30-second-to-6-hour latency. The system does not claim to + mirror the feed. +- `verify_recovery.sh`'s own stated limit still applies: the documented `docker kill` is an + API stop that `restart: unless-stopped` is defined to skip. The script kills twice and + reports both. +- **The stack ends in live mode.** Bringing a killed container back means + `docker compose up -d `, which recreates it from the current shell environment, + where `GITHUB_MODE` defaults to `live`. Observed here and now documented in the README's + crash-recovery section. + +## Every check in this run + +1. Clone into an empty directory; `git status --porcelain` empty — PASS +2. No `.env`, no `config/master.key` in the clone — PASS +3. Host Ruby is 4.0.5 against a 3.4.10 pin, gems absent, no `psql` — PASS +4. Cold `docker compose build --no-cache --pull`, `BUNDLE_FROZEN=1`, 992 MB — PASS +5. Cold `docker compose up --build` starts exactly `db`, `setup`, `web`, `worker` — PASS *(after `6eab84c`; FAILED before it, see the finding)* +6. `/health/live` and `/health/ready` both `{"status":"ok"}` — PASS +7. Live: one unauthenticated poll persisted 96 push events, 4 ignored — PASS +8. Live: ledger bootstrapped from response headers, 60/57 — PASS +9. Live: both actor and repository enrichment completed — PASS +10. Fixture: 4 / 3 / 3 / 3 / 3 from an empty volume — PASS +11. Fixture: `complete 2 / permanent_failure 1` per class — PASS +12. Replay: 4 duplicates absorbed, occurrences 3 → 6, `push_events` unchanged — PASS +13. Replay: no `enrichment.reactivated`, `skipped_budget` still 0 — PASS +14. `script/verify_recovery.sh --confirm`: 19 checks, absolute count mode — PASS +15. `pgdata` volume identity unchanged across the kills — PASS +16. Suite 1728 + 10, `0 failures` — PASS +17. Fixed-seed repeat identical — PASS +18. Test run changed neither development database; `setup` not re-triggered — PASS + +## Reproducing it + +```bash +# from a host with no image for this project +docker compose down -v --remove-orphans # if a stack exists +docker image rm github-push-ingestor-app + +git clone https://github.com/batbrainy/github-push-ingestor.git /tmp/ghpi && cd /tmp/ghpi +git status --porcelain # must be empty +test ! -e .env && test ! -e config/master.key + +docker compose build --no-cache --pull # the cold path +docker compose up --build -d # live; the assignment's command +until curl -fsS http://localhost:3000/health/ready; do sleep 2; done +curl -s http://localhost:3000/status | jq .ledger +docker compose down -v + +GITHUB_MODE=fixture docker compose up --build -d # the deterministic half +script/verify_recovery.sh --confirm # §15 steps 8-10 +docker compose run --rm test # §15 step 11 +docker compose down -v --remove-orphans +``` + +Allow roughly an hour, including three mandatory 60-second `X-Poll-Interval` waits between +successive ingestion scenarios.