Extension A completion — secondary-limit escalation and refresh-pool fairness

README.md

@@ -656,9 +656,14 @@ EnrichmentRunner ────────────────┘      │ 
   `global_blocked_until`, which only ever moves later.
 - **`Github::RateLimitPolicy`** decides *which* response warrants a global block and until
   when — primary exhaustion to the reset GitHub named, a reserve breach to the same, a
-  secondary limit to `Retry-After` clamped between one minute and one hour. Class
-  exhaustion deliberately writes nothing there: it is derived from the counters, so
-  polling running out never stops enrichment and vice versa.
+  secondary limit to `Retry-After` (either RFC 9110 form) clamped between one minute and one
+  hour. When a secondary limit carries no usable `Retry-After`, the block backs off
+  exponentially with jitter across *consecutive* limits — 60s, 120s, 240s, capped at one
+  hour — counted in `github_api_budget.consecutive_secondary_limits`, which survives a
+  window rollover because secondary limits are IP-scoped rather than window-scoped. One live
+  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.
@@ -685,9 +690,12 @@ EnrichmentRunner ────────────────┘      │ 
 - **`Github::Enrichment::Fairness`** applies §10's ladder: a never-enriched candidate in a
   class still inside its guarantee, then the same borrowing when the other class has no
   *currently eligible* candidate, then a TTL-stale refresh only when no pending candidate
-  is eligible anywhere. It decides; the ledger enforces, so a wrong answer produces a
-  refused reservation rather than an overspend
-  ([ADR 0007](docs/adr/0007-enrichment-fairness-shares-and-borrowing.md)).
+  is eligible anywhere. The refresh pool allocates by the same two steps — prefer a class
+  with room, borrow only from a class with nothing to refresh — so neither pool can starve a
+  class. It decides; the ledger enforces, so a wrong answer produces a refused reservation
+  rather than an overspend
+  ([ADR 0007](docs/adr/0007-enrichment-fairness-shares-and-borrowing.md),
+  [ADR 0010](docs/adr/0010-secondary-limit-escalation-and-refresh-pool-fairness.md)).
 - **`Github::Enrichment::Claim`** prevents two workers enriching one entity by leasing the
   row — a conditional `UPDATE` that pushes `next_retry_at` forward. One column, one
   meaning: the same predicate excludes leases, backoffs and secondary-limit deferrals from
@@ -708,8 +716,9 @@ 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), and
-runtime source allocation with shared-IP observability (0009).
+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).
 
 ## Continuous ingestion
 

app/services/github/budget_ledger.rb

@@ -102,6 +102,13 @@ class BudgetLedger
     # to the reset that has now passed, while preserving a secondary-limit block that
     # extends beyond it — the two conditions §10 distinguishes.
     #
+    # consecutive_secondary_limits is deliberately absent, and its absence is the
+    # behaviour rather than an omission. §10 calls secondary limits IP-scoped, which is a
+    # different scope from the primary window this statement resets: a streak that happens
+    # to span a window boundary is still a streak, and zeroing it here would hand a
+    # persistently throttled IP a fresh 60-second block every hour. Only a live request
+    # that completes without a secondary limit ends it — see #clear_secondary_limit_streak!.
+    #
     # The four counters are parameters rather than literal zeros so a rollover
     # discovered *during* reconciliation can carry the request that produced the
     # response into the window GitHub says it belongs to. See #reconcile!.
@@ -152,17 +159,39 @@ class BudgetLedger
     # COALESCE on window_status keeps the label's fate in the policy's hands without a
     # boolean bind: nil leaves it alone.
     #
+    # The increment on consecutive_secondary_limits is a bind of 1 or 0 for the same
+    # reason — one statement whose behaviour the caller selects, rather than a second
+    # statement that could interleave with this one between the SELECT ... FOR UPDATE and
+    # the commit. §10's exponential backoff needs the count, and the count has to be taken
+    # under the same row lock that writes the block it feeds.
+    #
     # lock_version is bumped by hand for the reason DEBIT_SQL gives — this is not an
     # Active Record save, and a stale in-memory row must still raise StaleObjectError
     # rather than clobber the counters.
     BLOCK_SQL = <<~SQL.squish
       UPDATE github_api_budget
          SET global_blocked_until = GREATEST(global_blocked_until, ?),
              window_status = COALESCE(?, window_status),
+             consecutive_secondary_limits = consecutive_secondary_limits + ?,
              lock_version = lock_version + 1, updated_at = ?
        WHERE id = ?
     SQL
 
+    # The other half of §10's exponential backoff: what ends a streak.
+    #
+    # Guarded in the WHERE rather than by a read-then-write, so the common case — every
+    # successful poll and every successful enrichment, on a service that has never seen a
+    # secondary limit — costs one statement that matches no row and takes no lock. Zero
+    # affected rows is the expected outcome here, not the LedgerInvariantViolation that
+    # #debit! raises on the same condition — a debit that matches no row means capacity was
+    # granted and then not taken, and this statement makes no such promise.
+    CLEAR_SECONDARY_STREAK_SQL = <<~SQL.squish
+      UPDATE github_api_budget
+         SET consecutive_secondary_limits = 0,
+             lock_version = lock_version + 1, updated_at = ?
+       WHERE id = ? AND consecutive_secondary_limits > 0
+    SQL
+
     def initialize(configuration: Github.configuration, allocation: SourceAllocation.new(configuration: configuration))
       @configuration = configuration
       @allocation = allocation
@@ -315,20 +344,61 @@ def block_globally!(until_at:, reason:, window_status: nil, now: Time.current)
       end
     end
 
+    # Ends the consecutive-secondary-limit streak that Github::RateLimitPolicy escalates
+    # from. Called for any live request that completed without a secondary limit — §10's
+    # backoff is exponential in *consecutive* limits, so one clean response is what breaks
+    # the run.
+    #
+    # A 304 counts as clean. It is a completed conditional request that GitHub answered
+    # and charged for, which is exactly the evidence that the IP is no longer being
+    # throttled.
+    #
+    # No row lock and no rollover check: this only ever writes a zero, so the worst a
+    # concurrent block can do is win and leave the streak at 1, which is the value that
+    # block just observed anyway.
+    #
+    # @return [Symbol] :cleared when a streak was ended, :unchanged when there was none
+    def clear_secondary_limit_streak!(now: Time.current)
+      assert_committable!
+
+      updated = GithubApiBudget.connection.exec_update(
+        ActiveRecord::Base.sanitize_sql_array([ CLEAR_SECONDARY_STREAK_SQL, now, SINGLETON_ID ]),
+        "Github::BudgetLedger ClearSecondaryStreak"
+      )
+
+      return :unchanged if updated.zero?
+
+      # §11 puts budget state transitions at INFO, and this one closes a story the
+      # budget.global_block_set lines opened: an operator reading a run of escalating
+      # blocks needs the line that says the escalation stopped.
+      Rails.logger.info(event: "budget.secondary_limit_streak_cleared")
+      :cleared
+    end
+
     private
 
     def apply_block!(budget, until_at:, reason:, window_status:, now:)
+      # Only a secondary limit advances the streak. A primary exhaustion and a reserve
+      # breach are conditions of the budget window, not of GitHub throttling this IP, and
+      # counting them would escalate a secondary block on evidence that has nothing to do
+      # with secondary limits.
+      increment = reason == :secondary_rate_limit ? 1 : 0
+
       GithubApiBudget.connection.exec_update(
-        ActiveRecord::Base.sanitize_sql_array([ BLOCK_SQL, until_at, window_status, now, SINGLETON_ID ]),
+        ActiveRecord::Base.sanitize_sql_array([ BLOCK_SQL, until_at, window_status, increment, now, SINGLETON_ID ]),
         "Github::BudgetLedger BlockGlobally"
       )
 
       # §11 lists "global_blocked_until set/cleared" among the budget state transitions
       # that belong at INFO. The instant is the field an operator greps for when nothing
       # is happening and nothing looks wrong.
+      #
+      # consecutive_secondary_limits is reported post-increment, because the number that
+      # explains the length of *this* block is the one that produced it.
       Rails.logger.info(event: "budget.global_block_set", reason: reason,
                         blocked_until: until_at.utc.iso8601,
                         previous_blocked_until: budget.global_blocked_until&.utc&.iso8601,
+                        consecutive_secondary_limits: budget.consecutive_secondary_limits + increment,
                         window_status: window_status || budget.window_status)
       :blocked
     end

app/services/github/enrichment/fairness.rb

@@ -27,7 +27,7 @@ class Fairness
       # happened rather than only that nothing did.
       class Choice < Data.define(:entity_type, :pool, :borrow, :reason)
         REASONS = %w[
-          pending borrowed_pending refresh
+          pending borrowed_pending refresh borrowed_refresh
           class_exhausted globally_blocked window_uninitialized no_candidate
         ].freeze
 
@@ -116,17 +116,35 @@ def pending_choice(budget, requested, eligible)
       # eligible". Read over every class, not only the requested one, so --class actor
       # cannot promote an actor refresh above a repository still waiting to be enriched
       # for the first time.
+      #
+      # Then §10:898's second constraint on the same line — refreshes run "within each
+      # class's share" — which makes this the same two-step #pending_choice performs, over
+      # a different pool: prefer a class that still has room, and only borrow when the
+      # other class genuinely has nothing to do. Picking the first refreshable class
+      # outright instead would hand actor every borrowed request in the window while
+      # repository's untouched guarantee and eligible stale rows were never selected, which
+      # is the class starvation the split exists to prevent, reproduced one pool down.
+      #
+      # The eligibility map is rebuilt over the *refresh* pool rather than reusing
+      # `eligible`. CandidateSelector#pending_available? is deliberately pending-only —
+      # counting refreshes there would let a refresh outrank a never-enriched candidate and
+      # invert §10's ladder — but that hazard cannot arise here: this method is only
+      # reached when no class has a pending candidate at all, so there is nothing left for
+      # a refresh to outrank.
       def refresh_choice(budget, requested, eligible, now:)
         return nil if eligible.values.any?
 
-        type = requested.find { |candidate| selector.refresh_available?(candidate, now: now) }
-        return nil if type.nil?
+        refreshable = EntityType.all.index_with { |type| selector.refresh_available?(type, now: now) }
+        available = requested.select { |type| refreshable.fetch(type) }
+        return nil if available.empty?
+
+        within = available.find { |type| room_within_guarantee?(budget, type) }
+        return Choice.new(entity_type: within, pool: :refresh, borrow: false, reason: "refresh") if within
+
+        borrower = available.find { |type| other_classes_quiet?(type, refreshable) }
+        return nil if borrower.nil?
 
-        borrow = !room_within_guarantee?(budget, type)
-        # With no pending candidate anywhere, "the other class has no currently eligible
-        # candidate" is true by definition, so a refresh past the guarantee is a legal
-        # borrow rather than a starve.
-        Choice.new(entity_type: type, pool: :refresh, borrow: borrow, reason: "refresh")
+        Choice.new(entity_type: borrower, pool: :refresh, borrow: true, reason: "borrowed_refresh")
       end
 
       # Derived from the *stored* enrichment_allowance, exactly as
@@ -142,6 +160,10 @@ def room_within_guarantee?(budget, entity_type)
 
       # §10's borrowing condition, verbatim: the other class has no CURRENTLY ELIGIBLE
       # candidate, not merely no rows.
+      #
+      # Generic over whichever eligibility map it is handed, because the condition is the
+      # same question asked of whichever pool is being allocated: #pending_choice passes
+      # the pending map, #refresh_choice the refresh one.
       def other_classes_quiet?(entity_type, eligible)
         eligible.except(entity_type).values.none?
       end

app/services/github/rate_limit_policy.rb

@@ -66,7 +66,16 @@ def initialize(ledger: BudgetLedger.new, backoff: PollBackoff.new)
     # @return [Decision]
     def apply!(fetched, now: Time.current)
       decision = decide(fetched, now: now)
-      return decision unless decision.blocking?
+
+      unless decision.blocking?
+        # A live request that completed without a secondary limit is the evidence that the
+        # IP is no longer being throttled, and §10's backoff is exponential in
+        # *consecutive* limits. Asked of successful? rather than of the decision alone: a
+        # 5xx or a timeout produced no verdict from GitHub about throttling, so it must
+        # neither escalate the streak nor end it.
+        @ledger.clear_secondary_limit_streak!(now: now) if fetched.successful?
+        return decision
+      end
 
       @ledger.block_globally!(until_at: decision.blocked_until, reason: decision.kind,
                               window_status: decision.window_status, now: now)
@@ -92,7 +101,11 @@ def decide(fetched, now:)
     # would spend its whole allowance re-asking a quota that is provably at zero.
     def primary_limit(fetched, now:)
       snapshot = fetched.rate_limit(observed_at: now)
-      blocked_until = snapshot&.reset_at || fallback_instant(snapshot, now: now)
+      # attempt: 1 — a primary exhaustion is not escalated on the secondary streak. The
+      # two conditions are unrelated: §10 attaches exponential backoff to secondary limits
+      # specifically, and a quota that is provably at zero is relieved by the window
+      # rolling, not by waiting longer each time.
+      blocked_until = snapshot&.reset_at || fallback_instant(snapshot, now: now, attempt: 1)
 
       Decision.new(kind: :primary_rate_limit, blocked_until: blocked_until,
                    source_retry_at: nil, window_status: blocked_window_status)
@@ -111,8 +124,15 @@ def primary_limit(fetched, now:)
     # exactly when the window rolls, and rollover is the transition that restores the
     # label; a secondary limit expires on an unrelated Retry-After and has no writer to
     # put the label back, so it would strand a value with no way out.
+    # The streak read here is stale by construction, and acceptably so. #apply! runs after
+    # the gate is released, so two responses can read the same count and compute the same
+    # block. The consequence is an under-escalated block and never an over-escalated one,
+    # and BLOCK_SQL's GREATEST means the loser's shorter instant cannot shorten the
+    # winner's. Computing it inside the ledger under the row lock would remove the staleness
+    # and cost the split this class exists for: it decides, the ledger records.
     def secondary_limit(fetched, now:)
-      blocked_until = fallback_instant(fetched.rate_limit(observed_at: now), now: now)
+      attempt = consecutive_secondary_limits + 1
+      blocked_until = fallback_instant(fetched.rate_limit(observed_at: now), now: now, attempt: attempt)
 
       Decision.new(kind: :secondary_rate_limit, blocked_until: blocked_until,
                    source_retry_at: blocked_until, window_status: nil)
@@ -134,16 +154,27 @@ def reserve_breach(fetched, now:)
                    source_retry_at: nil, window_status: blocked_window_status)
     end
 
-    # Retry-After may be absent, zero, negative, or an HTTP-date — RateLimitSnapshot
-    # parses only the delta-seconds form and deliberately leaves the rest nil. All four
-    # mean the same thing here: no usable instruction, so back off on our own terms.
-    # `now + nil` raises and `now + nil.to_i` is no block at all, which is the response
-    # most likely to escalate GitHub's throttling.
-    def fallback_instant(snapshot, now:)
+    # Retry-After may be absent, zero, negative, or unparseable. All of those mean the same
+    # thing here: no usable instruction, so back off on our own terms. `now + nil` raises
+    # and `now + nil.to_i` is no block at all, which is the response most likely to
+    # escalate GitHub's throttling.
+    #
+    # A server-supplied instruction is obeyed as given (within honoured's clamp), so
+    # `attempt` reaches PollBackoff only on the header-absent path — which is exactly where
+    # §10 puts the exponential: "from Retry-After (or >= 1 minute with exponential backoff
+    # when the header is absent)".
+    def fallback_instant(snapshot, now:, attempt:)
       seconds = snapshot&.retry_after_seconds
       return now + honoured(seconds) if seconds.is_a?(Integer) && seconds.positive?
 
-      @backoff.retry_at(1, now: now)
+      @backoff.retry_at(attempt, now: now)
+    end
+
+    # Zero when no row exists yet: a secondary limit before the ledger is bootstrapped has
+    # no history to escalate from, and PollBackoff's floor still gives it the minute §10
+    # requires.
+    def consecutive_secondary_limits
+      GithubApiBudget.find_by(id: GithubApiBudget::SINGLETON_ID)&.consecutive_secondary_limits.to_i
     end
 
     def honoured(seconds)