Repository navigation
fix(jetbrains): refresh PR badges app-wide on IDE focus gain - #13760
Merged
Merged
Conversation
Worktree PR badges only refreshed on frame focus for the project whose IdeFrame.getProject() matched the service instance. That filter dropped every activation from a frame that reports a null or default project (e.g. the welcome screen), and left a second open project's absence unconsumed since its frame never regained focus. Combined with a 30s force bar that made most quick returns fall back to the backend's 90s PR cache, and zero logging on any of the drop paths, a real focus-gain return could look identical in the logs to one that never fired at all. - Make the ApplicationActivationListener callbacks unfiltered: any app activation now refreshes every attached project's PR state, not just the focused frame's project. - Lower the force bar from PR_THROTTLE (30s) to Away.FRESH (10s) so a quick trip to a browser to open/merge a PR is enough to bypass the backend cache. - Add a spend floor separate from the bar: a focus-driven forced lookup may run at most once per PR_THROTTLE per project. A return blocked by the floor is deferred as a single trailing lookup (fixed deadline, strictest maxAge wins) rather than dropped, so it still lands within 30s instead of waiting out the full 120s poll. - Suppress focus-driven refreshes entirely while gh is RATE_LIMITED; the poll remains the only probe that notices the budget resetting. - Log every previously silent drop/defer/resume path plus the lookup result, so a repeat of this report is diagnosable from the log alone. Cost is bounded: PrResolver.resolve costs 1-3 gh calls per worktree, 4-way concurrent. Worst case after this change is ~3,600 gh calls/hour against the 5,000/hour budget, only reachable by alt-tabbing out >10s and back every 30s for a solid hour, and the rate-limit backstop cuts that off before it matters. refreshPr(force, maxAge) semantics are unchanged for deliberate callers (e.g. AgentManagerPanel's post-worktree-create refresh), which still run immediately and unfloored.
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous Review Summary (commit 27de3c6)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 27de3c6)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Reviewed by grok-4.6 · Input: 151.3K · Output: 14K · Cached: 432.6K Review guidance: REVIEW.md from base branch |
pandemicsyn
approved these changes
Sep 3, 2026
Review catch: the focus path deferred a return blocked by the spend floor but dropped one blocked by the in-flight guard, so the request was silently lost and the next automatic retry was the 120s poll. That is the worse of the two cases — the running lookup may have started before the departure, so its answer can predate the very change the return came back to observe, and dropping the request leaves that stale answer standing. Unify both blockers behind one held request: hold() records the strictest ceiling and spend() decides whether it can run now, leaving it held for whichever trigger clears first — the floor timer or the completion of the running lookup. loadPr hooks invokeOnCompletion so every way a lookup can end (answered, failed, cancelled, early return on an unresolved root) drains a held return, and a superseded generation defers to the newer lookup that owns the loop. spend() re-checks disposal, refs, the integration flag, and RATE_LIMITED at fire time rather than only at focus time, since a lookup landing while the return was held can be the one that reports the budget spent. Adds four tests: a return held behind a running lookup is spent when that lookup ends (carrying its maxAge), the same return is dropped when that lookup reports a rate limit, it does not outlive a detach, and the existing no-stacking test now asserts the trailing lookup lands once the floor clears instead of stopping at the skip.
This was referenced Sep 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Fixes #
No tracked issue — reported directly in chat: the JetBrains Agent Manager PR badge did not update after returning focus to the IDE, and the log gave no evidence a focus-driven check ever ran.
Context
Worktree PR badges are meant to refresh when the IDE regains focus (
WorktreeStatusService.focus()), so a PR opened or merged elsewhere shows up without waiting on the 120s poll. In practice this almost never fired:applicationActivatedwas filtered onideFrame.project === project, butIdeFrame.getProject()is nullable and the welcome screen answers a default project, so activations from those frames matched nothing. A second open project also never consumed its recorded absence, since its own frame never regained focus.Away.ceiling(gone, PR_THROTTLE)), so a quick trip to a browser to open/merge a PR usually took the unforced path and was answered from the backend's 90s PR cache.Implementation
applicationActivated/applicationDeactivatedare now unfiltered, so any real app focus gain refreshes PR state for every project with an attached Agent Manager surface, not just the one whose frame happened to report it.PR_THROTTLE(30s) toAway.FRESH(10s), matching the bar the gh-availability probe already uses.hold()/flush()fold it into a single trailing lookup with a fixed deadline (a continuing burst can't push it out) that keeps the strictestmaxAgeseen. Worst case a blocked return now lands within 30s instead of waiting out the full 120s poll.ghisRATE_LIMITED, focus-driven refreshes are suppressed entirely (spending a fan-out to confirm a spent budget teaches nothing), and the 120s poll remains the one path that notices the reset.refreshPr(force, maxAge)itself is unchanged —AgentManagerPanel's post-worktree-create refresh (force = true, maxAge = 0) still runs immediately and unfloored.Cost check (measured, not guessed):
PrResolver.resolvecosts 1–3ghcalls per worktree (a worktree without a PR pays the full ladder), 4-way concurrent. Worst case after this change is ~3,600ghcalls/hour against the 5,000/hour authenticated budget — only reachable by alt-tabbing out >10s and back every 30s for a solid hour — and the rate-limit backstop cuts that off before it matters. Two projects on the same repo root share one fan-out via the backend's directory-keyed cache.Deliberately out of scope: a PR created by an agent session inside the IDE still waits for the poll, since focus never left and the Agent Manager tab-switch path calls
refreshPr()unforced (answered from the 90s backend cache). Fixing that means invalidating the cache on agh prtool run, or forcing on tab switch — a separate change with its own cost tradeoff.Screenshots / Video
N/A — logging/timing behavior change, no UI change.
How to Test
Manual/local verification
./gradlew typecheckclean../gradlew test(full JetBrains suite) green../gradlew :frontend:test --tests '*WorktreeStatusServiceTest*' --tests '*GhStatusCoordinatorTest*' --tests '*AwayTest*'— 58/58 passing, including 4 new tests covering the spend-floor deferral, burst coalescing, and rate-limit suppression, and 3 rewritten tests covering the inverted app-wide activation and the new 10s bar.bun run lintfrom repo root — 0 errors.bun turbo typecheckacross all packages (including@kilocode/kilo-jetbrainsvia Gradle) — all green.Reviewer test steps
./gradlew runIdeSplitModefrompackages/kilo-jetbrains/), open a repo withghconfigured and the Agent Manager tool window attached to at least one worktree.kilo.logshowsworktree PR focus/worktree PR refresh donelines for the return.worktree PR focus deferredfollowed byworktree PR focus resumedonce the floor clears, rather than a second immediateghfan-out.Blocked checks and substitute verification
TestUiTimersandactivateIde/deactivateIde.Checklist
@kilocode/kilo-jetbrainsisprivateand versioned outside changesets; JetBrains release notes come from the changelog on the release PR.Get in Touch