Skip to content

fix: apply PR cache in daemon hot paths to prevent rate-limit exhaustion#431

Open
proboscis wants to merge 1 commit intomainfrom
issue/orch-450/run-20260224-181405
Open

fix: apply PR cache in daemon hot paths to prevent rate-limit exhaustion#431
proboscis wants to merge 1 commit intomainfrom
issue/orch-450/run-20260224-181405

Conversation

@proboscis
Copy link
Copy Markdown
Owner

Summary

  • Make LookupInfoWithClient and LookupInfoByURL cache-aware in internal/pr/cache.go by loading/saving cache entries, honoring cacheHitTTL/cacheMissTTL, and applying cacheMinFetchInterval.
  • Add cache-first wrappers (LookupInfoCached, LookupInfoByURLCached) and switch daemon hot paths to them in internal/daemon/monitor.go, internal/daemon/types.go, and internal/daemon/proto_handler.go.
  • Remove orch-450 semgrep exclusions for daemon files in .semgrep/architecture.yaml section 11 so architecture lint now enforces this path directly.

Verification

  • go test ./internal/pr/...
  • go test ./internal/daemon/...
  • go test -count=1 ./internal/pr/... ./internal/daemon/...
  • make lint ✅ (0 findings)

Acceptance Criteria Evidence

  1. LookupInfo / LookupInfoByURL use cache + TTL/min interval

    • Implemented in internal/pr/cache.go via cache read/write helpers and min-fetch gating.
    • TDD tests now pass:
      • TestLookupInfoWithClient_MustUseCache
      • TestLookupInfoWithClient_CacheHitSkipsAPI
      • TestLookupInfoByURL_MustUseCache
      • TestLookupInfoByURL_CacheHitSkipsAPI
  2. checkPRMergedWithURL throttled (not every 5s API call)

    • internal/daemon/monitor.go now calls cached wrappers only.
    • No direct pr.LookupInfo* calls remain in daemon package; semgrep architecture rule enforces this.
  3. lookupPRInfo in types.go uses cached data

    • internal/daemon/types.go now routes through LookupInfoCached / LookupInfoByURLCached.
  4. Consider global rate limiter

    • Not added in this change; existing daemon hot-loop bypasses are removed and now governed by cache throttling as a safety baseline.
  5. 20 active runs under 3000 calls/hr

    • Worst-case miss path bounded by cacheMissTTL=30s: 20 runs * (3600/30) = 2400 calls/hr (below 3000/hr target).
    • Cache hits are substantially lower.
  6. No regression to existing cache TTL constants / PopulateRunInfo

    • Existing constants unchanged.
    • PopulateRunInfo path still passes regression tests in internal/pr/cache_test.go.
  7. All new TDD tests pass

    • Confirmed by go test ./internal/pr/....
  8. Semgrep exclusions removed

    • Removed for monitor.go, types.go, proto_handler.go in .semgrep/architecture.yaml section 11.
    • Confirmed by make lint with 0 findings.

Refs: orch-450

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant