From af2c45aa27bcdd1e7d5bdbf356dcd280378abd23 Mon Sep 17 00:00:00 2001 From: kirillk Date: Mon, 31 Aug 2026 12:00:42 -0400 Subject: [PATCH 01/18] fix(jetbrains): make gh/PR focus sync responsive without overwhelming the backend A focus or tab-switch sync that could not run right away was dropped outright, leaving a stale gh/PR verdict standing until the next scheduled poll (up to 30s for gh, 120s for PR). Coalesce instead of drop: a sync that cannot run now is held as a single trailing probe, so a burst of events costs at most one extra probe per throttle window instead of losing the request. Scale the work to the length of the absence via a new Away helper. A dialog or popup that never really took focus out of the IDE costs nothing; a quick window switch takes the ordinary throttled path; an absence long enough to have contained a `gh auth login` or a merged PR bypasses the throttle and asks the backend for a fresher answer. Thread a maxAge ceiling through ghStatus/prStatus/branchStatus so a caller returning from a long absence can bypass the backend's own cache too - previously prStatus had no bypass at all, so even a forced frontend refresh could still serve a PR list up to 90s stale. maxAge only tightens the TTL, never extends it, so no caller can pin stale data past the backend's own TTL. Also narrow tab-switch syncing to the Agent Manager tab (Chat has no gh-dependent UI to refresh) and pass maxAge=0 when a worktree is freshly created so its PR badge doesn't wait out the cache. --- .changeset/jetbrains-gh-focus-freshness.md | 5 + .../backend/rpc/KiloWorktreeRpcApiImpl.kt | 45 +++-- .../backend/rpc/KiloWorktreeRpcApiImplTest.kt | 48 +++++ .../kilocode/client/KiloToolWindowFactory.kt | 15 +- .../client/agentManager/AgentManagerPanel.kt | 7 +- .../client/agentManager/worktree/Away.kt | 63 ++++++ .../worktree/GhStatusCoordinator.kt | 136 +++++++++++-- .../worktree/KiloWorktreeService.kt | 10 +- .../worktree/WorktreeStatusService.kt | 37 +++- .../client/agentManager/worktree/AwayTest.kt | 60 ++++++ .../worktree/GhStatusCoordinatorTest.kt | 184 ++++++++++++++++-- .../worktree/WorktreeStatusServiceTest.kt | 98 +++++++++- .../client/testing/FakeWorktreeRpcApi.kt | 12 +- .../client/testing/TestIdeActivation.kt | 12 ++ .../ai/kilocode/rpc/KiloWorktreeRpcApi.kt | 25 ++- 15 files changed, 682 insertions(+), 75 deletions(-) create mode 100644 .changeset/jetbrains-gh-focus-freshness.md create mode 100644 packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/Away.kt create mode 100644 packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/AwayTest.kt diff --git a/.changeset/jetbrains-gh-focus-freshness.md b/.changeset/jetbrains-gh-focus-freshness.md new file mode 100644 index 000000000000..47f3b6ccd030 --- /dev/null +++ b/.changeset/jetbrains-gh-focus-freshness.md @@ -0,0 +1,5 @@ +--- +"@kilocode/kilo-jetbrains": patch +--- + +Show pull request and gh authorization changes right after you come back to the IDE. Returning from a long absence now reloads immediately instead of waiting out the poll, refreshes that arrive during a burst of window or tab switches are no longer dropped, and closing a dialog no longer triggers a needless lookup. A newly created worktree also gets its pull request badge without waiting for the cache to expire. diff --git a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt index 3ce1288a7dfd..cfdef70533cf 100644 --- a/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt +++ b/packages/kilo-jetbrains/backend/src/main/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImpl.kt @@ -194,13 +194,13 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { return synced.filter { Files.isDirectory(Path.of(it.path)) } } - override suspend fun ghStatus(directory: String, github: Boolean): GhAvailability = withContext(Dispatchers.IO) { - probeGh(Path.of(directory).normalize(), "rpc", github) + override suspend fun ghStatus(directory: String, github: Boolean, maxAge: Long?): GhAvailability = withContext(Dispatchers.IO) { + probeGh(Path.of(directory).normalize(), "rpc", github, maxAge) } - override suspend fun prStatus(directory: String): WorktreePrListDto = withContext(Dispatchers.IO) { + override suspend fun prStatus(directory: String, maxAge: Long?): WorktreePrListDto = withContext(Dispatchers.IO) { val now = System.currentTimeMillis() - prs[directory]?.takeIf { now - it.time < PR_TTL }?.let { return@withContext it.value } + prs[directory]?.takeIf { usable(it.time, now, PR_TTL, maxAge) }?.let { return@withContext it.value } val root = Path.of(directory).normalize() // A gone directory reports nothing and is not cached, so a real availability problem found // from a live directory still reaches the UI. @@ -208,7 +208,9 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { LOG.info("pr status skipped, directory does not exist: $root") return@withContext WorktreePrListDto() } - val available = ghAvailable(root) + // A caller that rejected the cached PR list would not accept a cached availability verdict + // from the same moment either, so the ceiling carries into the gh probe. + val available = ghAvailable(root, maxAge = maxAge) if (available != GhAvailability.OK) return@withContext WorktreePrListDto(available).also { prs[directory] = Timed(now, it) } // Sync the worktree list before the per-worktree lookups so a worktree that was added and // then deleted on disk is pruned instead of resolved from a directory that no longer exists. @@ -229,13 +231,13 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { dto } - override suspend fun branchStatus(directory: String, github: Boolean): BranchStatusDto = withContext(Dispatchers.IO) { + override suspend fun branchStatus(directory: String, github: Boolean, maxAge: Long?): BranchStatusDto = withContext(Dispatchers.IO) { val now = System.currentTimeMillis() // Keyed by directory + mode so flipping the GitHub integration setting cannot serve a // cross-mode entry (a disabled-mode entry always has a null pr; an enabled-mode entry may // not) for up to PR_TTL after the flip. val key = "$directory|$github" - branches[key]?.takeIf { now - it.time < PR_TTL }?.let { return@withContext it.value } + branches[key]?.takeIf { usable(it.time, now, PR_TTL, maxAge) }?.let { return@withContext it.value } val root = Path.of(directory).normalize() if (!Files.isDirectory(root)) { LOG.info("branch status skipped, directory does not exist: $root") @@ -243,7 +245,7 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { } val branch = runGit(root, "branch", "--show-current").stdout.trim() val worktree = isLinkedWorktree(root) - val availability = ghAvailable(root, github) + val availability = ghAvailable(root, github, maxAge) val lookup = if (github && availability == GhAvailability.OK && branch.isNotBlank()) { resolver.resolve(directory, branch, baseBranch(root)) } else { @@ -661,17 +663,19 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { * Resolves git/gh availability. When [github] is false, only git presence is checked and `gh` * is never spawned — used while the user has turned off the GitHub integration setting. */ - private fun ghAvailable(root: Path, github: Boolean = true): GhAvailability { + private fun ghAvailable(root: Path, github: Boolean = true, maxAge: Long? = null): GhAvailability { if (!Files.isDirectory(root)) { LOG.info("gh availability skipped dir=$root missing=true") return GhAvailability.OK } - val status = probeGh(root, "availability", github) + val status = probeGh(root, "availability", github, maxAge) // A git-only probe can only return GIT_MISSING or OK, so the gh-binary re-check below is // unreachable while github is false; the guard documents that explicitly. if (!github || status != GhAvailability.MISSING) return status val now = System.currentTimeMillis() - ghProbe?.takeIf { now - it.time < GH_PROBE_TTL }?.let { return it.value } + // Installing gh is the transition this long-lived entry hides, so a caller demanding + // freshness must be able to re-check the binary and not just the auth verdict. + ghProbe?.takeIf { usable(it.time, now, GH_PROBE_TTL, maxAge) }?.let { return it.value } val res = runGh(root, "--version") val value = if (res.ok) GhAvailability.OK else GhAvailability.MISSING ghProbe = Timed(now, value) @@ -683,7 +687,7 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { * spawns `gh` or touches [ghCache] (the cache mixes gh-auth verdicts with a boolean this probe * would otherwise poison with a git-only "OK"). */ - private fun probeGh(root: Path, reason: String, github: Boolean = true): GhAvailability = synchronized(ghLock) { + private fun probeGh(root: Path, reason: String, github: Boolean = true, maxAge: Long? = null): GhAvailability = synchronized(ghLock) { // A stale/removed worktree directory makes the process spawn fail, which would be // misreported as GIT_MISSING. Treat a missing directory as "nothing to report" and // don't cache it, so the next probe on a real directory still runs. @@ -693,13 +697,13 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { } val now = System.currentTimeMillis() if (github) { - ghCache?.takeIf { now - it.time < GH_STATUS_TTL }?.let { - LOG.info("gh probe cache hit reason=$reason value=${it.value}") + ghCache?.takeIf { usable(it.time, now, GH_STATUS_TTL, maxAge) }?.let { + LOG.info("gh probe cache hit reason=$reason value=${it.value} ageMs=${now - it.time}") return@synchronized it.value } } val start = System.currentTimeMillis() - LOG.info("gh probe start reason=$reason dir=$root github=$github") + LOG.info("gh probe start reason=$reason dir=$root github=$github maxAge=${maxAge ?: "default"}") val git = runGit(root, "--version") if (!git.ok) { // The directory can disappear between the check above and the spawn; a failed working @@ -730,6 +734,17 @@ class KiloWorktreeRpcApiImpl : KiloWorktreeRpcApi { } +/** + * Whether a cache entry written at [time] may still be served. [maxAge] is the caller's own ceiling + * on staleness: it can only tighten [ttl], never extend it, so an event-driven request can reject an + * entry the poll would have accepted while no caller is able to pin stale data in place past the TTL. + * A [maxAge] of 0 (or below) rejects every entry and forces the work to run. + */ +internal fun usable(time: Long, now: Long, ttl: Long, maxAge: Long?): Boolean { + val limit = maxAge?.coerceIn(0, ttl) ?: ttl + return now - time < limit +} + /** True when a process failed because its working directory is gone, not because the tool is absent. */ internal fun badDir(text: String): Boolean { val msg = text.lowercase() diff --git a/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt b/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt index 2812bac0f219..0f1c81ecbab1 100644 --- a/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt +++ b/packages/kilo-jetbrains/backend/src/test/kotlin/ai/kilocode/backend/rpc/KiloWorktreeRpcApiImplTest.kt @@ -52,6 +52,54 @@ class KiloWorktreeRpcApiImplTest { assertEquals(GhAvailability.OK, api.prStatus(repo.resolve("missing").toString()).availability) } + @Test + fun `a cache entry is usable only within both the ttl and the caller ceiling`() { + assertTrue(usable(time = 0, now = 89_999, ttl = 90_000, maxAge = null)) + assertFalse(usable(time = 0, now = 90_000, ttl = 90_000, maxAge = null)) + + // A ceiling tightens: an entry the TTL alone would have served can be rejected, which is the + // only way a caller returning to the IDE can get past a cache filled before it left. + assertTrue(usable(time = 0, now = 9_999, ttl = 90_000, maxAge = 10_000)) + assertFalse(usable(time = 0, now = 10_000, ttl = 90_000, maxAge = 10_000)) + + // ...and never extends, so no caller can pin stale data beyond the TTL. + assertFalse(usable(time = 0, now = 120_000, ttl = 90_000, maxAge = Long.MAX_VALUE)) + + // Zero forces the work to run, and a negative value is clamped to that rather than inverting + // the comparison into "always fresh". + assertFalse(usable(time = 0, now = 0, ttl = 90_000, maxAge = 0)) + assertFalse(usable(time = 0, now = 0, ttl = 90_000, maxAge = -1)) + } + + @Test + fun `branch status serves its cache until a caller demands a fresher answer`() = runBlocking { + initRepo() + val dir = repo.resolve(".kilo").resolve("worktrees").resolve("cached") + git(repo, "worktree", "add", "-b", "feature/cached", dir.toString()) + assertEquals("feature/cached", api.branchStatus(dir.toString()).branch) + + git(dir, "checkout", "-b", "feature/moved") + + // The default ceiling accepts the entry written above, so a change made meanwhile is invisible + // for as long as it lives — the staleness a returning caller has to be able to reject. + assertEquals("feature/cached", api.branchStatus(dir.toString()).branch) + assertEquals("feature/moved", api.branchStatus(dir.toString(), maxAge = 0).branch) + } + + @Test + fun `branch status keeps serving the cache for a ceiling wider than the entry age`() = runBlocking { + initRepo() + val dir = repo.resolve(".kilo").resolve("worktrees").resolve("wide") + git(repo, "worktree", "add", "-b", "feature/wide", dir.toString()) + assertEquals("feature/wide", api.branchStatus(dir.toString()).branch) + + git(dir, "checkout", "-b", "feature/other") + + // A returning caller passes the length of its absence, not zero: work that happened while it + // was away is still current, so a ceiling the entry fits inside must reuse it. + assertEquals("feature/wide", api.branchStatus(dir.toString(), maxAge = 60_000).branch) + } + @Test fun `branch status skips a missing directory without caching the empty result`() = runBlocking { initRepo() diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/KiloToolWindowFactory.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/KiloToolWindowFactory.kt index 9e2e3a03fbda..81e2f3391854 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/KiloToolWindowFactory.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/KiloToolWindowFactory.kt @@ -156,13 +156,16 @@ internal class KiloToolWindowSetupService( val listener = object : ContentManagerListener { override fun selectionChanged(event: ContentManagerEvent) { if (event.operation != ContentManagerEvent.ContentOperation.add) return - // Either direction: switching tabs is a deliberate act that often follows an - // authorization or PR change made elsewhere. The coordinator drops the submit - // when a probe is already running or too recent, so tab churn cannot stack up - // gh calls. Chat needs nothing further — SessionUi re-reads branch/PR state - // itself once it becomes showing. + // Only on the way in: opening the Agent Manager is a deliberate act that often + // follows an authorization or PR change made elsewhere, and this is the tab whose + // banner and badges read gh state. Switching to Chat reveals no gh-dependent UI — + // SessionUi re-reads branch/PR state itself once it becomes showing — so probing + // then only spends a call to warm a cache nothing is waiting on. + if (event.content !== agentContent) return + // The coordinator folds a submit that cannot run now into one trailing probe, so + // tab churn cannot stack up gh calls. service().sync("tab-switch") - if (event.content === agentContent) agentManagerPanel.refresh() + agentManagerPanel.refresh() } } toolWindow.contentManager.addContentManagerListener(listener) diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/AgentManagerPanel.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/AgentManagerPanel.kt index de2ec8844fd0..b60fcf3f6942 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/AgentManagerPanel.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/AgentManagerPanel.kt @@ -141,11 +141,12 @@ class AgentManagerPanel( if (list.select(key)) list.focusList() item(key)?.takeIf { controller.progress(it.id) == null }?.let { open(it, focus = false) } } - // A fresh worktree changes what git reports, so bypass the refresh throttle instead of - // leaving the new row without its stats and PR badge until the next poll. + // A fresh worktree changes what git reports, so bypass both the refresh throttle and the + // backend's PR cache — that cache was populated before this worktree existed, so serving it + // would leave the new row without its badge until the entry aged out. controller.onCreated = { created -> project?.service()?.refreshStats() - project?.service()?.refreshPr(force = true) + project?.service()?.refreshPr(force = true, maxAge = 0) autoRunSetupScript(created) } controller.onReload = { sync() } diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/Away.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/Away.kt new file mode 100644 index 000000000000..baca5a38be96 --- /dev/null +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/Away.kt @@ -0,0 +1,63 @@ +package ai.kilocode.client.agentManager.worktree + +/** + * How long the IDE spent out of focus, so a return can be answered in proportion to the absence + * rather than by a flat throttle. + * + * Both gh availability and pull-request state are things the user changes elsewhere — `gh auth login` + * in a terminal, a PR merged in a browser — so the chance that a cached answer went stale scales with + * the time away. A 400ms flicker cannot have contained any of that; a five-minute absence very likely + * did. + * + * Driven by `ApplicationActivationListener.applicationDeactivated`, which the platform already limits + * to focus leaving the application entirely (it fires only for a `WINDOW_DEACTIVATED` with no opposite + * window, so a popup or dialog that keeps focus inside the IDE never reaches it). The platform's own + * `delayedApplicationDeactivated` additionally waits out `application.deactivation.timeout` before + * confirming, because a heavyweight transient window can still produce that event; [REAL] applies the + * same predicate at return time instead, which needs no `Window` and no proactive timer. + */ +internal class Away(private val now: () -> Long) { + private var at: Long? = null + + /** Records that focus left the application. */ + fun left() { + at = now() + } + + /** + * Consumes the pending absence and reports its length in milliseconds, or null when there was no + * absence worth acting on — the case a caller should answer by doing nothing at all. + */ + fun back(): Long? { + val start = at ?: return null + at = null + return (now() - start).takeIf { it >= REAL } + } + + companion object { + /** + * Below this an absence is indistinguishable from a transient window that never meant to take + * focus out of the IDE, so returning from one is not evidence of anything. Mirrors the + * platform's `application.deactivation.timeout` default, the delay it waits before confirming + * a deactivation for the same reason. + */ + const val REAL = 1_500L + + /** + * Absences at or past this point stop being explainable as window churn, so the caches + * covering them are no longer evidence about the current state. + */ + const val FRESH = 10_000L + + /** + * The freshness ceiling a return from an absence of [gone] justifies: the absence itself once + * it is long enough to have contained an external change, and null below that to accept + * whatever the caches already hold. + * + * Passing the absence rather than zero is deliberate. Work done *during* the absence — a poll + * that ran while the window sat in the background, another panel's lookup — is still current + * and worth reusing; only answers that predate the departure have to be rejected. + */ + fun ceiling(gone: Long): Long? = gone.takeIf { it >= FRESH } + } +} diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinator.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinator.kt index 5085a7ec550e..9f73cd546df8 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinator.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinator.kt @@ -38,21 +38,31 @@ class GhStatusCoordinator( private const val SLOW = 60_000 private const val MAX_BACKOFF = 120_000 // Floor between event-driven syncs. Matches the backend's gh auth status cache TTL, so a - // burst of focus/tab events cannot outrun the answer a probe would get anyway. - private const val EVENT_THROTTLE = 3_000 + // burst of focus/tab events cannot outrun the answer a probe would get anyway. Measured from + // the last probe's completion rather than its start: the backend writes its cache entry when + // the probe finishes, so timing from the start would let a sync clear this floor and still be + // served that entry. + private const val EVENT_THROTTLE = 3_000L } + /** An event-driven sync that could not run when it arrived, held for one trailing probe. */ + private data class Sync(val reason: String, val maxAge: Long?) + private var timers: UiTimerSource = UiTimers private var value = GhAvailability.OK private var notified = false private var timer: UiTimer? = null + private var trail: UiTimer? = null private var refs = 0 private var busy = false private var failures = 0 private var generation = 0 private var github = KiloPluginSettings.getGithub() private var job: Job? = null + /** When the last probe finished, whatever its outcome. See [EVENT_THROTTLE]. */ private var probed = 0L + private var pending: Sync? = null + private val away = Away { timers.now() } private val projects = linkedMapOf() init { @@ -61,7 +71,9 @@ class GhStatusCoordinator( // Returning to the IDE usually follows work done elsewhere — `gh auth login` in a terminal, // a PR merged in a browser — so re-check then instead of waiting out the poll interval. bus.subscribe(ApplicationActivationListener.TOPIC, object : ApplicationActivationListener { - override fun applicationActivated(ideFrame: IdeFrame) = sync("frame-focus") + override fun applicationActivated(ideFrame: IdeFrame) = edt { focus() } + + override fun applicationDeactivated(ideFrame: IdeFrame) = edt { away.left() } }) } @@ -78,27 +90,105 @@ class GhStatusCoordinator( /** * Submits an out-of-band probe after an event that may have changed gh state (IDE frame focus, - * tool window tab switch). Coalesces rather than queues: dropped while a probe is already in - * flight or within [EVENT_THROTTLE] of the last one, so the single-probe-at-a-time loop keeps - * its shape no matter how many events arrive. + * tool window tab switch). Coalesces rather than queues: a sync that cannot run right now is + * held as a single trailing probe instead of one per event, so a burst costs at most one extra + * probe per [EVENT_THROTTLE] window while never discarding the request outright — dropping it + * would leave the stale verdict standing until the next scheduled poll. */ fun sync(reason: String) { edt { syncEdt(reason) } } + /** + * Handles the IDE frame regaining focus. Work is proportional to the absence: none at all for a + * popup that never really took focus away, a throttled sync for a quick window switch, and a + * cache-bypassing probe once the absence is long enough to have contained a `gh auth login`. + */ + @RequiresEdt + private fun focus() { + val gone = away.back() ?: return + syncEdt("frame-focus", Away.ceiling(gone)) + } + @RequiresEdt - private fun syncEdt(reason: String) { + private fun syncEdt(reason: String, maxAge: Long? = null) { if (refs == 0) return if (busy) { - LOG.info("gh sync dropped reason=$reason busy=true") + defer(reason, maxAge, "busy=true") return } val since = timers.now() - probed - if (since < EVENT_THROTTLE) { - LOG.info("gh sync dropped reason=$reason sinceMs=$since") + // A freshness requirement outranks the throttle: the caller has already established that the + // answer a throttled sync would preserve is too old to be worth keeping. + if (maxAge == null && since < EVENT_THROTTLE) { + defer(reason, maxAge, "sinceMs=$since") + arm(EVENT_THROTTLE - since) return } - probe(reason) + probe(reason, maxAge) + } + + /** + * Folds a sync that cannot run now into the single held one. The newest reason wins for the log + * and the strictest freshness requirement survives, so a forced sync held behind a throttled one + * still bypasses the backend cache when it finally runs. + */ + @RequiresEdt + private fun defer(reason: String, maxAge: Long?, why: String) { + val held = pending?.maxAge + val next = when { + held == null -> maxAge + maxAge == null -> held + else -> minOf(held, maxAge) + } + pending = Sync(reason, next) + LOG.info("gh sync deferred reason=$reason $why maxAge=${next ?: "default"}") + } + + /** + * Arms the trailing probe at the end of the current throttle window. Deliberately not a sliding + * debounce: a continuing burst must not keep pushing the deadline out, so an already-armed timer + * is left alone and the window end stays fixed from the first deferral. + */ + @RequiresEdt + private fun arm(ms: Long) { + if (trail?.isRunning() == true) return + trail = timers.timer(ms.coerceAtLeast(1).toInt(), repeats = false) { flush() }.also { it.start() } + } + + @RequiresEdt + private fun flush() { + trail = null + resume() + } + + /** + * Runs a held event-driven sync now that the loop is free, so the news it was submitted for does + * not wait out a full poll interval. Returns true when a probe was started. + * + * A sync carrying no freshness requirement is instead discarded when a probe has just succeeded: + * that answer already reflects the state the event was asking about, and re-running it would + * turn every deferral into a guaranteed second call. A failed probe establishes nothing, so it + * never counts as satisfying one. + */ + @RequiresEdt + private fun resume(): Boolean { + val next = pending ?: return false + if (busy) return false + if (refs == 0) { + pending = null + return false + } + pending = null + trail?.stop() + trail = null + val since = timers.now() - probed + if (next.maxAge == null && failures == 0 && since < EVENT_THROTTLE) { + LOG.info("gh sync satisfied reason=${next.reason} sinceMs=$since") + return false + } + probe(next.reason, next.maxAge) + return true } @RequiresEdt @@ -127,6 +217,9 @@ class GhStatusCoordinator( generation++ timer?.stop() timer = null + trail?.stop() + trail = null + pending = null job?.cancel() job = null busy = false @@ -159,7 +252,7 @@ class GhStatusCoordinator( } @RequiresEdt - private fun probe(reason: String) { + private fun probe(reason: String, maxAge: Long? = null) { if (refs == 0) return if (busy) { LOG.info("gh probe skipped reason=$reason busy=true delay=${delay()}") @@ -174,14 +267,13 @@ class GhStatusCoordinator( busy = true val gen = generation val start = timers.now() - probed = start val mode = github - LOG.info("gh probe start reason=$reason state=$value delay=${delay()} github=$mode") + LOG.info("gh probe start reason=$reason state=$value delay=${delay()} github=$mode maxAge=${maxAge ?: "default"}") job = cs.launch { runCatching { val dir = project.kiloRoot() ?: return@runCatching null LOG.info("gh probe dir=$dir") - service().ghStatus(dir, mode) + service().ghStatus(dir, mode, maxAge) } .onSuccess { next -> if (next == null) { @@ -210,6 +302,11 @@ class GhStatusCoordinator( failures = 0 generation++ notified = false + // The toggle probes (or publishes OK) on its own, so a sync held from before it is redundant + // and must not survive to fire a second probe straight after. + trail?.stop() + trail = null + pending = null LOG.info("gh probe github=$enabled state=$value refs=$refs") if (!enabled) { apply(target(), GhAvailability.OK) @@ -223,6 +320,7 @@ class GhStatusCoordinator( edt { if (gen != generation || refs == 0) return@edt busy = false + probed = timers.now() schedule() } } @@ -232,6 +330,7 @@ class GhStatusCoordinator( if (gen != generation || refs == 0) return@edt busy = false failures = 0 + probed = timers.now() LOG.info("gh probe done value=$next ms=$ms nextDelay=${delay()}") apply(project, next) schedule() @@ -243,16 +342,23 @@ class GhStatusCoordinator( if (gen != generation || refs == 0) return@edt busy = false failures++ + probed = timers.now() LOG.warn("gh probe failed failures=$failures ms=$ms nextDelay=${delay()}", err) schedule() } } + /** + * Decides what the loop does next. A held event-driven sync outranks the poll: the news it was + * submitted for has already arrived, so making the user wait out a full interval for it is the + * staleness this whole path exists to avoid. + */ @RequiresEdt private fun schedule() { timer?.stop() timer = null if (refs == 0) return + if (resume()) return val ms = delay() timer = timers.timer(ms, repeats = false) { probe("scheduled") }.also { it.start() } LOG.info("gh probe scheduled delay=$ms state=$value failures=$failures refs=$refs") diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/KiloWorktreeService.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/KiloWorktreeService.kt index ae5689dcc50d..f97e95e440ec 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/KiloWorktreeService.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/KiloWorktreeService.kt @@ -99,10 +99,11 @@ class KiloWorktreeService internal constructor( * distinguish a healthy gh from an unhealthy backend via their own `runCatching` + backoff; * swallowing errors here would publish a false "gh is fine" and reset that backoff. */ - suspend fun ghStatus(directory: String, github: Boolean = true): GhAvailability = call { ghStatus(directory, github) } + suspend fun ghStatus(directory: String, github: Boolean = true, maxAge: Long? = null): GhAvailability = + call { ghStatus(directory, github, maxAge) } - suspend fun prStatus(directory: String): WorktreePrListDto = try { - call { prStatus(directory) } + suspend fun prStatus(directory: String, maxAge: Long? = null): WorktreePrListDto = try { + call { prStatus(directory, maxAge) } } catch (e: Exception) { LOG.warn("worktree PR status failed for $directory", e) WorktreePrListDto() @@ -114,7 +115,8 @@ class KiloWorktreeService internal constructor( * would offer worktree actions against a directory whose real state is unknown. Callers decide * what an unknown status means. */ - suspend fun branchStatus(directory: String, github: Boolean = true): BranchStatusDto = call { branchStatus(directory, github) } + suspend fun branchStatus(directory: String, github: Boolean = true, maxAge: Long? = null): BranchStatusDto = + call { branchStatus(directory, github, maxAge) } /** * Long-lived move flow. Routed through [durable] (via [call]) so it survives reconnects and diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusService.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusService.kt index e2d337d37862..87dcd8e80203 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusService.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusService.kt @@ -49,6 +49,7 @@ class WorktreeStatusService internal constructor( private var refs = 0 private var lastPr = 0L private var github = KiloPluginSettings.getGithub() + private val away = Away { timers.now() } // Bumped whenever a PR lookup starts or is abandoned, so a result that arrives after its reason // to exist is gone cannot publish. Mirrors GhStatusCoordinator's probe generation. private var generation = 0 @@ -62,12 +63,16 @@ class WorktreeStatusService internal constructor( val bus = ApplicationManager.getApplication().messageBus.connect(cs) bus.subscribe(GithubIntegrationListener.TOPIC, GithubIntegrationListener { enabled -> github(enabled) }) // A PR can be merged or closed while the IDE sits in the background, so re-check on - // activation. Unforced, so PR_THROTTLE collapses a burst of focus events into one lookup. + // activation. bus.subscribe(ApplicationActivationListener.TOPIC, object : ApplicationActivationListener { override fun applicationActivated(ideFrame: IdeFrame) { if (ideFrame.project !== project) return - refreshPr() + focus() } + + // Unfiltered: the absence belongs to the application, not to one frame, so every open + // project records it and each consumes its own copy when that project is focused again. + override fun applicationDeactivated(ideFrame: IdeFrame) = away.left() }) } @@ -86,12 +91,32 @@ class WorktreeStatusService internal constructor( timer.restart() } - fun refreshPr(force: Boolean = false) { + /** + * Reloads PR state. [force] bypasses [PR_THROTTLE], the frontend floor between lookups. [maxAge] + * caps how old a cached backend answer may be and is the only way past the backend's own PR + * cache, so a caller that needs to observe a change made outside the IDE has to pass it. + * + * The two are separate because they guard different costs: the throttle guards the RPC round + * trip, [maxAge] guards the per-worktree `gh` fan-out behind it. + */ + fun refreshPr(force: Boolean = false, maxAge: Long? = null) { if (project.isDisposed || refs == 0 || !github) return val now = timers.now() if (!force && now - lastPr < PR_THROTTLE) return lastPr = now - loadPr() + loadPr(maxAge) + } + + /** + * Reloads PR state on return to the IDE, scaled to the absence. A dialog or popup that never took + * focus out of the IDE reports no absence and costs nothing; a quick window switch takes the + * throttled path; a long absence is the case worth paying a full `gh` fan-out for, and is also the + * only path that can get past the backend's own PR cache. + */ + private fun focus() { + val gone = away.back() ?: return + val max = Away.ceiling(gone) + refreshPr(force = max != null, maxAge = max) } private fun start() { @@ -158,11 +183,11 @@ class WorktreeStatusService internal constructor( } } - private fun loadPr() { + private fun loadPr(maxAge: Long? = null) { val gen = ++generation prJob = cs.launch { val dir = project.kiloRoot() ?: return@launch - runCatching { service().prStatus(dir) } + runCatching { service().prStatus(dir, maxAge) } .onSuccess { dto -> // KiloWorktreeService.prStatus swallows the cancellation and answers with an // empty DTO, so a lookup cancelled by a disable still lands here — and after a diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/AwayTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/AwayTest.kt new file mode 100644 index 000000000000..13e4efcbc9ca --- /dev/null +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/AwayTest.kt @@ -0,0 +1,60 @@ +package ai.kilocode.client.agentManager.worktree + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class AwayTest { + private var now = 0L + private val away = Away { now } + + @Test + fun `a return with no recorded departure reports nothing`() { + assertNull(away.back(), "the first focus of a session is not a return") + } + + @Test + fun `an absence shorter than the debounce reports nothing`() { + away.left() + now += Away.REAL - 1 + + assertNull(away.back(), "a transient window is indistinguishable from never leaving") + } + + @Test + fun `an absence at the debounce reports its length`() { + away.left() + now += Away.REAL + + assertEquals(Away.REAL, away.back()) + } + + @Test + fun `an absence is consumed so one departure answers one return`() { + away.left() + now += Away.FRESH + assertEquals(Away.FRESH, away.back()) + + now += Away.FRESH + assertNull(away.back(), "a second activation without leaving again is not a new absence") + } + + @Test + fun `a later departure replaces an unconsumed one`() { + away.left() + now += Away.FRESH + away.left() + now += Away.REAL + + assertEquals(Away.REAL, away.back(), "the absence runs from the most recent departure") + } + + @Test + fun `only an absence past the freshness bar earns a ceiling`() { + assertNull(Away.ceiling(Away.FRESH - 1), "window churn has no claim on the caches") + assertEquals(Away.FRESH, Away.ceiling(Away.FRESH)) + // The ceiling is the absence itself, not zero: a lookup that ran while we were gone is still + // current, and only answers predating the departure have to be rejected. + assertEquals(90_000L, Away.ceiling(90_000L)) + } +} diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinatorTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinatorTest.kt index 6b8831dca7a2..93a2a65616a2 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinatorTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/GhStatusCoordinatorTest.kt @@ -7,6 +7,7 @@ import ai.kilocode.client.testing.fakeRoot import ai.kilocode.client.testing.pumpEdt import ai.kilocode.client.testing.TestUiTimers import ai.kilocode.client.testing.activateIde +import ai.kilocode.client.testing.deactivateIde import ai.kilocode.client.testing.installBrowser import ai.kilocode.client.util.edtWait import ai.kilocode.rpc.dto.GhAvailability @@ -143,36 +144,60 @@ class GhStatusCoordinatorTest : BasePlatformTestCase() { assertEquals(1, rpc.ghCalls.size) } - fun `test coordinator drops submitted syncs while busy instead of queueing`() { + fun `test a sync submitted while a probe runs is answered by that probe`() { val gate = CompletableDeferred() rpc.beforeGhStatus = { gate.await() } val handle = edtWait { service.attach(project) } awaitCalls(1) - // Past the event throttle, so an in-flight probe is the only thing that can drop the submit. + // Past the event throttle, so an in-flight probe is the only thing holding this sync back. timers.advanceBy(EVENT_THROTTLE) edtWait { service.sync("test") } pump() - assertEquals(1, rpc.ghCalls.size) + assertEquals("a second probe must never run alongside the first", 1, rpc.ghCalls.size) gate.complete(Unit) drain() + + // The completed probe reports state from after the event, so the held sync is satisfied + // rather than re-run: coalescing must not turn every deferral into a guaranteed second call. assertEquals(1, rpc.ghCalls.size) handle.close() } - fun `test coordinator throttles a burst of submitted syncs`() { + fun `test a throttled burst runs one trailing probe instead of none`() { val handle = edtWait { service.attach(project) } drain() assertEquals(1, rpc.ghCalls.size) - // Focus and tab-switch events can arrive in bursts; inside the window they collapse to none. + // Focus and tab-switch events arrive in bursts; inside the window they fold into one sync. repeat(5) { edtWait { service.sync("burst") } } drain() - assertEquals(1, rpc.ghCalls.size) + assertEquals("no event may run inside the throttle window", 1, rpc.ghCalls.size) + + // At the end of the window that one sync runs. Dropping the burst outright would leave the + // stale verdict standing until the next scheduled poll, which is the staleness to avoid. + timers.advanceBy(EVENT_THROTTLE) + drain() + assertEquals("the burst owes exactly one probe", 2, rpc.ghCalls.size) + // And it is spent: the same burst cannot produce a second trailing probe. timers.advanceBy(EVENT_THROTTLE) + drain() + assertEquals(2, rpc.ghCalls.size) + handle.close() + } + + fun `test the trailing window end is fixed rather than pushed back by later events`() { + val handle = edtWait { service.attach(project) } + drain() + edtWait { service.sync("first") } + + // A continuing burst must not behave like a sliding debounce, or a user clicking between tabs + // could postpone the probe indefinitely. + timers.advanceBy(EVENT_THROTTLE - 1) edtWait { service.sync("later") } + timers.advanceBy(1) drain() assertEquals(2, rpc.ghCalls.size) @@ -186,31 +211,166 @@ class GhStatusCoordinatorTest : BasePlatformTestCase() { assertTrue(rpc.ghCalls.isEmpty()) } - fun `test coordinator syncs when the ide frame is activated`() { - rpc.ghResult = GhAvailability.UNAUTH + fun `test detach discards a sync held behind a running probe`() { + val gate = CompletableDeferred() + rpc.beforeGhStatus = { gate.await() } val handle = edtWait { service.attach(project) } + awaitCalls(1) + timers.advanceBy(EVENT_THROTTLE) + edtWait { service.sync("tab-switch") } + + handle.close() + gate.complete(Unit) drain() - assertEquals(GhAvailability.UNAUTH, service.current()) + timers.advanceBy(120_000) + drain() + + assertEquals("a held sync must not outlive the last consumer", 1, rpc.ghCalls.size) + } + + fun `test detach cancels the trailing probe`() { + val handle = edtWait { service.attach(project) } + drain() + edtWait { service.sync("tab-switch") } + + handle.close() + timers.advanceBy(EVENT_THROTTLE) + drain() + assertEquals(1, rpc.ghCalls.size) + } + + fun `test returning after a long absence probes past the backend cache`() { + val handle = edtWait { service.attach(project) } + drain() + assertEquals(1, rpc.ghCalls.size) + + // The user ran `gh auth login` in a terminal while the IDE sat in the background. + rpc.ghResult = GhAvailability.UNAUTH + edtWait { deactivateIde(project) } + timers.advanceBy(Away.FRESH) + edtWait { activateIde(project) } + drain() + + assertEquals(2, rpc.ghCalls.size) + // The absence, not zero: a lookup that ran while we were gone is still current, and only an + // answer cached before we left has to be rejected. + assertEquals(listOf(null, Away.FRESH), rpc.ghAges.toList()) + assertEquals(GhAvailability.UNAUTH, service.current()) + handle.close() + } + + fun `test returning from a quick switch probes without bypassing the backend cache`() { + val handle = edtWait { service.attach(project) } + drain() + // Past the throttle, so only the absence rule decides what this activation costs. timers.advanceBy(EVENT_THROTTLE) - // The user authorized gh in a terminal and came back to the IDE. - rpc.ghResult = GhAvailability.OK + edtWait { deactivateIde(project) } + timers.advanceBy(Away.REAL) edtWait { activateIde(project) } drain() assertEquals(2, rpc.ghCalls.size) - assertEquals(GhAvailability.OK, service.current()) + assertNull("a quick switch has no claim on the cache", rpc.ghAges.last()) + handle.close() + } + + fun `test returning from a transient window does not probe at all`() { + val handle = edtWait { service.attach(project) } + drain() + timers.advanceBy(EVENT_THROTTLE) + + // A dialog or popup that closes right away never took focus out of the IDE for long enough + // to have changed anything, so it must cost nothing rather than one throttled probe. + edtWait { deactivateIde(project) } + timers.advanceBy(Away.REAL - 1) + edtWait { activateIde(project) } + drain() + + assertEquals(1, rpc.ghCalls.size) + handle.close() + } + + fun `test activation without a preceding absence does not probe`() { + val handle = edtWait { service.attach(project) } + drain() + timers.advanceBy(EVENT_THROTTLE) + + edtWait { activateIde(project) } + drain() + + assertEquals("the first focus of a session is not a return", 1, rpc.ghCalls.size) + handle.close() + } + + fun `test a burst of activations reloads once per absence`() { + val handle = edtWait { service.attach(project) } + drain() + timers.advanceBy(EVENT_THROTTLE) + + edtWait { deactivateIde(project) } + timers.advanceBy(Away.FRESH) + repeat(5) { edtWait { activateIde(project) } } + drain() + + assertEquals("one departure owes one probe, however often the frame reports focus", 2, rpc.ghCalls.size) handle.close() } fun `test coordinator does not probe on activation before anything attaches`() { + edtWait { deactivateIde(project) } + timers.advanceBy(Away.FRESH) edtWait { activateIde(project) } drain() assertTrue(rpc.ghCalls.isEmpty()) } + fun `test a freshness requirement held behind a running probe survives it`() { + val gate = CompletableDeferred() + rpc.beforeGhStatus = { gate.await() } + val handle = edtWait { service.attach(project) } + awaitCalls(1) + + edtWait { deactivateIde(project) } + timers.advanceBy(Away.FRESH) + // Only the calls after this one answer freely; the first is already suspended on the gate. + rpc.beforeGhStatus = {} + edtWait { activateIde(project) } + pump() + assertEquals(1, rpc.ghCalls.size) + + gate.complete(Unit) + drain() + + // The in-flight probe started before the absence was known, so unlike a plain sync this one + // is not satisfied by it and must still run — with its ceiling intact. + assertEquals(2, rpc.ghCalls.size) + assertEquals(Away.FRESH, rpc.ghAges.last()) + handle.close() + } + + fun `test a failed probe does not satisfy a held sync`() { + val gate = CompletableDeferred() + rpc.beforeGhStatus = { + gate.await() + throw RuntimeException("backend down") + } + val handle = edtWait { service.attach(project) } + awaitCalls(1) + timers.advanceBy(EVENT_THROTTLE) + edtWait { service.sync("tab-switch") } + + gate.complete(Unit) + drain() + + // A failure establishes nothing about gh state, so it cannot stand in for the answer the + // event asked for the way a successful probe does. + assertEquals(2, rpc.ghCalls.size) + handle.close() + } + fun `test coordinator probes git only while the github integration is off`() { rpc.ghResult = GhAvailability.UNAUTH val handle = edtWait { service.attach(project) } diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusServiceTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusServiceTest.kt index ec7061ccc9d4..fef624f4ebba 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusServiceTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeStatusServiceTest.kt @@ -7,7 +7,7 @@ import ai.kilocode.client.testing.fakeRoot import ai.kilocode.client.testing.pumpEdt import ai.kilocode.client.testing.TestUiTimers import ai.kilocode.client.testing.activateIde -import ai.kilocode.client.testing.fakeRoot +import ai.kilocode.client.testing.deactivateIde import ai.kilocode.client.util.edtWait import ai.kilocode.rpc.dto.GhAvailability import ai.kilocode.rpc.dto.GhState @@ -201,7 +201,7 @@ class WorktreeStatusServiceTest : BasePlatformTestCase() { handle.close() } - fun `test activation reloads pr state within the throttle budget`() { + fun `test returning after a long absence observes a pr merged elsewhere`() { val path = "${project.basePath}/.kilo/worktrees/feature-x" val key = normalizeWorktreePath(path) rpc.prResult = WorktreePrListDto(GhAvailability.OK, listOf(WorktreePrDto(path, 1, GhState.OPEN, "https://pr/1"))) @@ -209,8 +209,9 @@ class WorktreeStatusServiceTest : BasePlatformTestCase() { drain() assertEquals(1, service.pr.value[key]?.number) - // A PR merged while the IDE sat in the background. + // A PR merged in a browser while the IDE sat in the background. rpc.prResult = WorktreePrListDto(GhAvailability.OK, listOf(WorktreePrDto(path, 1, GhState.MERGED, "https://pr/1"))) + deactivateIde(project) timers.advanceBy(30_000) activateIde(project) drain() @@ -219,16 +220,86 @@ class WorktreeStatusServiceTest : BasePlatformTestCase() { handle.close() } - fun `test activation collapses a burst of focus events into one lookup`() { + fun `test a long absence outranks the throttle and the backend pr cache`() { + rpc.prResult = WorktreePrListDto(GhAvailability.OK) + val handle = service.attach() + drain() + assertEquals(1, rpc.prCalls.size) + + // Well inside PR_THROTTLE, so the frontend floor alone would have dropped this. The absence is + // the whole reason to spend the lookup, and the ceiling is the only way past the backend's own + // PR cache — without it the answer could predate the departure by up to its full TTL. + deactivateIde(project) + timers.advanceBy(Away.FRESH) + activateIde(project) + drain() + + assertEquals(2, rpc.prCalls.size) + assertEquals(listOf(null, Away.FRESH), rpc.prAges.toList()) + handle.close() + } + + fun `test returning from a quick switch reloads without bypassing the backend cache`() { + rpc.prResult = WorktreePrListDto(GhAvailability.OK) + val handle = service.attach() + drain() + // Past PR_THROTTLE, so only the absence rule decides what this activation costs. + timers.advanceBy(30_000) + + deactivateIde(project) + timers.advanceBy(Away.REAL) + activateIde(project) + drain() + + assertEquals(2, rpc.prCalls.size) + assertNull("a quick switch does not justify a fresh gh fan-out", rpc.prAges.last()) + handle.close() + } + + fun `test returning from a transient window does not reload pr state`() { rpc.prResult = WorktreePrListDto(GhAvailability.OK) val handle = service.attach() drain() + timers.advanceBy(30_000) val before = rpc.prCalls.size + // A dialog or popup closing never took focus out of the IDE for long enough to have changed a + // PR, and this lookup costs one `gh` call per worktree. + deactivateIde(project) + timers.advanceBy(Away.REAL - 1) + activateIde(project) + drain() + + assertEquals(before, rpc.prCalls.size) + handle.close() + } + + fun `test a burst of activations reloads once per absence`() { + rpc.prResult = WorktreePrListDto(GhAvailability.OK) + val handle = service.attach() + drain() + assertEquals(1, rpc.prCalls.size) + + deactivateIde(project) + timers.advanceBy(Away.FRESH) repeat(5) { activateIde(project) } drain() - assertEquals("PR_THROTTLE must absorb repeated activation", before, rpc.prCalls.size) + assertEquals("one departure owes one lookup, however often the frame reports focus", 2, rpc.prCalls.size) + handle.close() + } + + fun `test activation without a preceding absence does not reload pr state`() { + rpc.prResult = WorktreePrListDto(GhAvailability.OK) + val handle = service.attach() + drain() + timers.advanceBy(30_000) + val before = rpc.prCalls.size + + activateIde(project) + drain() + + assertEquals("the first focus of a session is not a return", before, rpc.prCalls.size) handle.close() } @@ -237,7 +308,8 @@ class WorktreeStatusServiceTest : BasePlatformTestCase() { val handle = service.attach() drain() val before = rpc.prCalls.size - timers.advanceBy(30_000) + deactivateIde(project) + timers.advanceBy(Away.FRESH) // A different project's window gaining focus says nothing about this project's worktrees. activateIde(ProjectManager.getInstance().defaultProject) @@ -247,6 +319,20 @@ class WorktreeStatusServiceTest : BasePlatformTestCase() { handle.close() } + fun `test a forced refresh can demand a fresh backend lookup`() { + rpc.prResult = WorktreePrListDto(GhAvailability.OK) + val handle = service.attach() + drain() + + // What creating a worktree needs: the cached PR list was built before this worktree existed, + // so serving it would leave the new row without a badge until the entry aged out. + service.refreshPr(force = true, maxAge = 0) + drain() + + assertEquals(listOf(null, 0L), rpc.prAges.toList()) + handle.close() + } + fun `test attach skips pr polling while the github integration is off`() { val path = "${project.basePath}/.kilo/worktrees/feature-x" val key = normalizeWorktreePath(path) diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/FakeWorktreeRpcApi.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/FakeWorktreeRpcApi.kt index dab397fb4bd6..8fafbd220052 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/FakeWorktreeRpcApi.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/FakeWorktreeRpcApi.kt @@ -54,9 +54,13 @@ class FakeWorktreeRpcApi : KiloWorktreeRpcApi { val ghCalls = CopyOnWriteArrayList() /** The `github` flag of each [ghStatus] call, positionally matching [ghCalls]. */ val ghFlags = CopyOnWriteArrayList() + /** The `maxAge` ceiling of each [ghStatus] call, positionally matching [ghCalls]. */ + val ghAges = CopyOnWriteArrayList() /** Each [branchStatus] call as directory to `github` flag. */ val branchCalls = CopyOnWriteArrayList>() val prCalls = CopyOnWriteArrayList() + /** The `maxAge` ceiling of each [prStatus] call, positionally matching [prCalls]. */ + val prAges = CopyOnWriteArrayList() val statsCalls = CopyOnWriteArrayList() val dirtyCalls = CopyOnWriteArrayList() var beforeCreate: suspend () -> Unit = {} @@ -108,18 +112,20 @@ class FakeWorktreeRpcApi : KiloWorktreeRpcApi { return dirtyResult } - override suspend fun ghStatus(directory: String, github: Boolean): GhAvailability { + override suspend fun ghStatus(directory: String, github: Boolean, maxAge: Long?): GhAvailability { assertNotEdt("ghStatus") ghCalls.add(directory) ghFlags.add(github) + ghAges.add(maxAge) beforeGhStatus() if (!github) return if (ghResult == GhAvailability.GIT_MISSING) ghResult else GhAvailability.OK return ghResult } - override suspend fun prStatus(directory: String): WorktreePrListDto { + override suspend fun prStatus(directory: String, maxAge: Long?): WorktreePrListDto { assertNotEdt("prStatus") prCalls.add(directory) + prAges.add(maxAge) // Snapshot before the gate so a call held open answers with what was configured when it // started, letting a test stage a newer result for the calls that follow. val answer = prResult @@ -127,7 +133,7 @@ class FakeWorktreeRpcApi : KiloWorktreeRpcApi { return answer } - override suspend fun branchStatus(directory: String, github: Boolean): BranchStatusDto { + override suspend fun branchStatus(directory: String, github: Boolean, maxAge: Long?): BranchStatusDto { assertNotEdt("branchStatus") branchCalls.add(directory to github) branchThrows?.let { throw it } diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/TestIdeActivation.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/TestIdeActivation.kt index 295a84d89599..2497304f4246 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/TestIdeActivation.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/testing/TestIdeActivation.kt @@ -21,6 +21,18 @@ fun activateIde(project: Project) { .applicationActivated(StubIdeFrame(project)) } +/** + * Publishes IDE frame deactivation, the way the platform does when focus leaves the application. + * Listeners time the absence from here, so a test that wants [activateIde] to read as a return rather + * than as the first focus of the session has to call this first and advance the clock past + * `Away.REAL`. + */ +fun deactivateIde(project: Project) { + ApplicationManager.getApplication().messageBus + .syncPublisher(ApplicationActivationListener.TOPIC) + .applicationDeactivated(StubIdeFrame(project)) +} + private class StubIdeFrame(private val project: Project) : IdeFrame { override fun getStatusBar(): StatusBar? = null override fun suggestChildFrameBounds(): Rectangle = Rectangle() diff --git a/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/KiloWorktreeRpcApi.kt b/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/KiloWorktreeRpcApi.kt index 3e84c967e811..f95f8d79774f 100644 --- a/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/KiloWorktreeRpcApi.kt +++ b/packages/kilo-jetbrains/shared/src/main/kotlin/ai/kilocode/rpc/KiloWorktreeRpcApi.kt @@ -52,16 +52,31 @@ interface KiloWorktreeRpcApi : RemoteApi { /** * Resolves gh/git availability for [directory]. When [github] is false, resolves git state * only ([GhAvailability.GIT_MISSING] or [GhAvailability.OK]) and never spawns `gh`. + * + * [maxAge] is the oldest cached answer the caller accepts, in milliseconds; null takes whatever + * the backend's own TTL allows. See [prStatus] for why callers state an age instead of a flag. + */ + suspend fun ghStatus(directory: String, github: Boolean = true, maxAge: Long? = null): GhAvailability + + /** + * Pull-request state per managed worktree. Answers from a short-lived backend cache so a poll + * and several panels can share one `gh` fan-out. + * + * [maxAge] caps how old that cached answer may be, in milliseconds; null accepts the backend's + * full TTL and 0 forces a fresh lookup. An age rather than a `fresh` flag because it composes: + * a caller returning from a long absence can ask for `maxAge =