Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
23 changes: 23 additions & 0 deletions IMPLEMENTATION_PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -1301,3 +1301,26 @@ 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 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 |
| 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.
Loading
Loading