Skip to content

Fix #1645: stop the overview cache from burning the GitHub GraphQL quota - #1652

Merged
waleedkadous merged 11 commits into
mainfrom
builder/bugfix-1645
Sep 8, 2026
Merged

Fix #1645: stop the overview cache from burning the GitHub GraphQL quota#1652
waleedkadous merged 11 commits into
mainfrom
builder/bugfix-1645

Conversation

@waleedkadous

Copy link
Copy Markdown
Contributor

Fixes #1645

Rebuild lane per the architect's prescription on the issue (supersedes #1646). Scope is the six frozen items; the doctor budget check, the single-query collapse (#1647), manual escape (#1648) and workspace-scoped invalidation (#1650) are out.

Problem

Tower's OverviewCache never cached a failed forge fetch, so once gh started failing every 2.5 s dashboard poll re-spawned all four list commands. Healthy, it still cost 4 GraphQL calls × N workspaces every 30 s. Measured on the production Tower during investigation: ~2 gh spawns/s, and the real GraphQL bucket (from gh api graphql -i headers) at 1,420 points used five minutes into the window while gh api rate_limit reported used: 0.

Root cause

  • overview.ts: if (data !== null) cache.set(...) — failures uncached.
  • forge.ts: executeForgeCommand collapsed every failure to null; nothing could tell "rate limited" from "gh missing".
  • No single-flight; concurrent pollers each started a batch.
  • invalidate() (porch after every mutating command, VS Code, cleanup) cleared every cache for every workspace.

Fix

  1. Negative cache with backoff — per <workspace>:<budget> window, 60 s doubling per elapsed window to 15 min. Four parallel failures share one window.
  2. Rate-limit suspension per resolved backendresolveForgeBackend keys by the executable's lowercased basename (gh for /usr/local/bin/gh; a Linear workspace's pr-list keys gh), provider for generic transports (curl) and custom script paths. ForgeRateLimiter suspends the budget until the reset from one advisory gh api rate_limit probe (trusted only when it reports remaining === 0, because it misreports a healthy bucket) or 15 min. user-identity/auth-status on gh are their own budget (gh:rest) in every direction. Only a success dispatched strictly after the suspension began clears it.
  3. Single-flight per <workspace>:<concept>, identity-checked cleanup.
  4. TTLs 180 s lists / 600 s searches / 3600 s identity. invalidate() stamps an epoch honoured only by positive open-list entries, debounced 60 s per entry (so per workspace); never touches a suspension, a cached failure, or the search/identity caches.
  5. Payload forgeStatus: 'ok' | 'rate-limited' | 'unavailable' + forgeResetAt; error text names the real backend. Dashboard shows a banner (rendered in Chromium via Playwright against an intercepted /api/overview).
  6. Scripts# forge-executable: on the 8 scripts whose first line is a builtin (gitlab/issue-search, 7 linear); github/pr-list no longer pipes gh into jq (POSIX sh has no pipefail, so a rate-limited gh looked like a successful empty list).

Also: executeForgeCommandDetailed resolves config inside its try, so a malformed .codev/config.json no longer reaches tower-server's unhandledRejectionprocess.exit(1).

Tests

New: overview-rate-limit.test.ts (10), bugfix-1645-gh-quota.test.ts (real OverviewCache + real forge scripts + fake permanently-rate-limited gh on PATH: 40 polls → 5 spawns, then 4 more only after the suspension lifts), forge-rate-limit.test.ts (6), 5 additions to forge.test.ts. Seven existing overview tests re-pinned to the new semantics with an injected clock.

Pitfall tests verified failing with their guard removed

Process: HEAD committed and pushed; each guard patched in place, the named test run, the file restored from HEAD, tree confirmed clean.

Prescribed test Guard removed Result
REST success (user-identity) does not clear a GraphQL suspension budgetKeyFor REST split RED
REST failure does not suspend the GraphQL backend budgetKeyFor REST split RED
REST success does not reset the GraphQL backoff failure window keyed by budget RED
invalidate() clears neither a suspension nor the search/identity caches invalidate() epoch-only RED
Sustained invalidation, two workspaces: ≤ debounced ceiling 60 s debounce RED
… and A's invalidations don't suppress B's per-entry (per-workspace) debounce vs global scalar RED
40 polls at 2.5 s against a rate-limited gh: one batch then zero suspension gate RED
Same, real gh harness suspension gate RED
Two pollers + invalidation mid-flight: one fetch per concept single-flight RED
Linear pr-list suspends gh, not linear backend by executable RED
Later credible probe with an earlier reset shortens the suspension honour earlier reset RED
Healthy probe reading ignored remaining === 0 credibility RED
Same-tick success does not clear (strict >) strict comparison RED
No built-in provider resolves to a shell builtin # forge-executable: declaration RED
github/pr-list propagates gh's exit status jq pipe removed RED
Negative cache (failure cached for the window) failure cache.set RED

Not red-able: the identity check on single-flight cleanup. In this design invalidate() never touches the in-flight map, so no path can register a newer flight while an older one is still registered; the check is two lines of defence, stated here rather than covered by a test that would have to introduce the race.

Checks

  • pnpm --filter @cluesmith/codev build, then full suite: 288 files, 5,767 passed, 48 skipped, 0 failed.
  • tsc --noEmit clean for packages/codev, packages/types, apps/web.
  • Production diff (excluding tests and the thread log): 448+ / 125− across 17 files.

Acceptance (architect's amended criteria)

  • Zero further gh spawns while rate-limited: harness test above.
  • ≤ 50 % of the GraphQL budget at 13 watched workspaces: 4 concepts at 180/180/600/600 s = 52 calls/h per workspace → 676/h at 13; single-flight and the negative cache hold that under churn; invalidation ceiling 132/h per workspace (2 lists × 60/h + 2 searches × 6/h).
  • Single-flight: pinned.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JccuXX8YQhT57jC69X82ax

waleedkadous and others added 11 commits September 8, 2026 01:02
Tower's OverviewCache re-spawned every forge command on every 2.5 s poll
once gh started failing (failures were never cached), and even healthy it
cost 4 GraphQL calls × N workspaces every 30 s.

- Cache failures with a per-workspace, per-budget window: 60 s, doubling per
  elapsed window (not per failed command) to 15 min.
- Detect the rate-limit error and suspend the resolved backend (gh, keyed by
  executable basename, not provider) until the reset instant from one advisory
  `gh api rate_limit` probe, or 15 min. Only a success dispatched after the
  suspension began clears it; gh REST concepts (user-identity) are on their
  own budget in every direction.
- Single-flight per <workspace>:<concept> with an identity check on cleanup.
- TTLs 180 s (lists) / 600 s (24 h searches) / 3600 s (identity).
  invalidate() marks only the open lists stale, debounced 60 s per workspace,
  and never touches a suspension, a cached failure, or the search/identity caches.
- Overview payload carries forgeStatus / forgeResetAt; the dashboard shows it.
- executeForgeCommandDetailed surfaces stderr and exit code (and never
  rejects on a malformed config); github/pr-list no longer hides gh's exit
  status behind a jq pipe; `# forge-executable:` on the 8 scripts whose
  first line is a shell builtin.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JccuXX8YQhT57jC69X82ax
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JccuXX8YQhT57jC69X82ax
… status

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JccuXX8YQhT57jC69X82ax
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JccuXX8YQhT57jC69X82ax
…MAP verdicts

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JccuXX8YQhT57jC69X82ax
@waleedkadous

Copy link
Copy Markdown
Contributor Author

Architect integration review

Merging on the human's word. Verified against the branch: the frozen scope from the #1645 prescription and nothing else (448+/125− production lines, 17 files); all eight prescribed pitfall tests present by name plus eight more; the guard-removal table in the body matches the test files; the dashboard banner rendered and read in the Playwright capture; CI 7/7 green; CMAP unanimous in one round.

This supersedes #1646 (archived at archive/bugfix-1645-attempt1). The measured pitfalls from that attempt are what the tests here pin. Follow-ups stay open: #1647 (one query per workspace), #1648 (manual escape), #1650 (scoped invalidation), #1651 (jq exit status).

Field verification next: local-install on main, then a 10-minute read-only count of the Tower daemon's gh children at 13 workspaces.

@waleedkadous
waleedkadous merged commit 71a0c57 into main Sep 8, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant