Skip to content

fix(jetbrains): settle attention badges during auto-approve - #13765

Merged
kirillk merged 3 commits into
mainfrom
sturdy-yak
Sep 5, 2026
Merged

fix(jetbrains): settle attention badges during auto-approve#13765
kirillk merged 3 commits into
mainfrom
sturdy-yak

Conversation

@kirillk

@kirillk kirillk commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Issue

Fixes #

Context

Agent Manager's orange ? attention glyph flickered on worktree rows and on the worktree tab's session-list toggle while a session ran through auto-approved edits, and the toggle badge sometimes stayed stuck even after attention had actually cleared.

Implementation

JetBrains auto-approve is a client-only policy: the CLI still emits a real permission.asked/permission.replied pair for every edit, so KiloBackendActivityManager briefly reports PERMISSION before flipping back to RUNNING on each tool call. WorktreeActivity.rank() ranks PERMISSION above RUNNING, so the glyph visibly swapped twice per call with nothing to debounce it.

  • KiloSessionService.activity now settles through transformLatest: a rising transition into an attention kind (PERMISSION/QUESTION/PLAN) is held for a short grace window (400ms) so a permission the client auto-answers itself is cancelled before ever reaching downstream consumers. Clearing and RUNNING/ERROR transitions stay immediate — this is the single choke point for worktree rows, the toggle badge, session-list row pills, and the Agents tab dot.
  • WorktreeSessionEditorPanel.syncToggle() now filters activity down to this worktree's listed session ids before ranking attention. Previously it read the full global activity map, so a permission in another worktree or an auto-approved task subagent (which has no row here and so could never be cleared by the user) could badge this tab.
  • The panel now subscribes to KiloSessionService.activity directly (mirroring WorktreeController.init) and re-syncs on every change. Previously it only refreshed via list changes and onListChanged, which only fires for the currently open session's own state — a background session's activity clearing elsewhere never reached the panel, leaving the badge stuck.

Non-goals (documented in the plan, left for follow-up): backend reconciliation of stuck permissions on reconnect, and pushing auto-approve into CLI config so no ask event exists at all (touches shared upstream code).

Screenshots / Video

N/A — no visual/layout change, only timing of an existing badge.

How to Test

Manual/local verification

  • Agent (this session): ran ./gradlew typecheck and ./gradlew test from packages/kilo-jetbrains/ — both green, including the full existing suite (no regressions).
  • Agent (this session): ran the targeted filter ./gradlew :frontend:test --tests '*KiloSessionServiceTest*' --tests '*WorktreeSessionEditorPanelTest*' --tests '*WorktreeActivityTest*' — 65/65 tests pass, including 5 new tests added for this fix.

Reviewer test steps

  1. Check out an Agent Manager worktree with auto-approve enabled for edits.
  2. Run an edit-heavy turn (many sequential file edits) in that worktree's session.
  3. Confirm the worktree row keeps a steady spinner instead of swapping to ? on every tool call, and the worktree tab's collapsed session-list toggle does not gain a ? badge for that worktree.
  4. Open a second worktree, get a session there stuck on a real permission/question, then clear it — confirm any other worktree's toggle never showed that session's badge (unaffected), and its own badge clears promptly (within ~400ms) instead of the debounce feeling laggy.

Blocked checks and substitute verification

  • ./gradlew --no-configuration-cache runIdeSplitMode manual sandbox run was not performed by the agent (requires an interactive IDE session); substitute verification was the full automated Gradle test suite plus the new unit tests exercising the exact debounce/scoping/subscription behavior described above.

Checklist

  • Issue linked above, or exception explained
  • Tests/verification described
  • Screenshots/video included for visual changes, or marked N/A
  • Changeset considered for user-facing changes
  • I personally reviewed the diff and can explain the changes, including any AI-assisted work.

Get in Touch

Auto-approve is a client-only policy: the CLI still asks for every
edit/bash permission, so JetBrains replies once right away. Each
ask/reply pair flips KiloSessionService.activity into an attention
kind and back, which flickered the worktree rows and the worktree
tab's session-list toggle, and left the toggle badge stuck once
attention actually cleared elsewhere.

- Settle the activity StateFlow: a rising transition into an
  attention kind (permission/question/plan) is held for a short
  grace window so a permission the client auto-answers itself never
  reaches downstream consumers. Clearing and RUNNING/ERROR
  transitions stay immediate.
- Scope the worktree toggle badge to this worktree's listed session
  ids, so a permission in another directory or an auto-approved
  task subagent (which has no row here) can no longer badge it.
- Subscribe the worktree session editor panel to session activity
  directly so a background session's cleared/changed state
  re-syncs the panel instead of waiting on the open session's own
  state to change.
@kilo-code-bot

kilo-code-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)
  • .changeset/jetbrains-attention-badge-flicker.md
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeSessionEditorPanel.kt
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/app/KiloSessionService.kt
  • packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeSessionEditorPanelTest.kt
  • packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/app/KiloSessionServiceTest.kt
Previous Review Summary (commit 63d7aad)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 63d7aad)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)
  • .changeset/jetbrains-attention-badge-flicker.md
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeSessionEditorPanel.kt
  • packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/app/KiloSessionService.kt
  • packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/agentManager/worktree/WorktreeSessionEditorPanelTest.kt
  • packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/app/KiloSessionServiceTest.kt

Reviewed by grok-4.6 · Input: 140.2K · Output: 16.1K · Cached: 507.6K

Review guidance: REVIEW.md from base branch main

kirillk and others added 2 commits September 5, 2026 13:08
Holding the whole activity snapshot meant a session going to ERROR or
RUNNING in the same emission as another session newly entering
PERMISSION also waited out the grace window.

Hold only the sessions that just entered an attention state, keyed by
session id, and publish every other entry in the snapshot immediately.
Held sessions keep the kind they were last published with, so a
spinner keeps spinning instead of blinking to the attention glyph.
@kirillk

kirillk commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

CI note: the earlier red runs were flaky tests unrelated to this change — no code fix was needed and the suite is green now.

Three different tests failed across three runs, none of them touched by this PR:

run failing test module
1 (this PR) SessionUpdateQueueTest > test update hooks run on EDT around history and recovery frontend
2 (this PR, rerun) KiloBackendSessionManagerTest > SSE status updates replace previous status() backend
concurrent main run WorktreeSessionEditorManagerTest > test a second fork of the same session is ignored while the first is in flight frontend

Run 2 is decisive: this PR is frontend-only, so a backend test cannot regress from it. main was independently red on the same suite at the same time. The test (linux) failures were just the downstream gate (Verify upstream test jobs passed -> jetbrains=failure), not independent failures.

On the first failure specifically: it asserts model.state is SessionState.Busy, which recoverPending() seeds from a one-shot sessions.statuses.value snapshot (SessionController.kt:1494). This PR only changes the separate activity flow, never statuses. The test waits for both update cycles to run but not for the status map to propagate before recovery reads it, so under CI load recovery can observe an empty snapshot and settle on Idle — a pre-existing race, consistent with its prior flake-fix commit c35dc1cc1a.

Verified locally on this branch: SessionUpdateQueueTest 6/6 green, KiloBackendSessionManagerTest 3/3 green, plus a full ./gradlew test typecheck pass.

@kirillk
kirillk merged commit e8f9d72 into main Sep 5, 2026
23 checks passed
@kirillk
kirillk deleted the sturdy-yak branch September 5, 2026 19:45
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.

2 participants