feat(extension): match Agents conversations to browser

[iscekic]

The Agents tab now uses the browser conversation renderer for tool calls, tool results, reasoning, code blocks, and screenshots. Users can send a message while an agent runs while Stop remains available.

The Agents tab matches the browser conversation behavior for cloud agents and remote CLI agents. It hides synthetic snapshot progress text and keeps the composer available during a run.

The extension maps stored agent parts into the shared conversation model, reuses the browser conversation list and item view, stores tool image bytes in a bounded per-panel map, and retries active history when no replay watermark exists. Focused tests, builds, Chrome E2E, Firefox E2E, and the live remote CLI flow pass.

Human steps: No human step is needed before or after merge.

Visual Changes: Latest live E2E side-panel screenshot

E2E: bot-e2e — remote CLI start, active reopen, queue, and stop passed. Cloud-agent start, reopen, queue, and stop remain uncovered because account fl@fl.fl has no GitHub integration and no existing cloud-agent session. The exact form message was Connect GitHub to start a cloud session, or pick a connected CLI instance.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (4 files, incremental)

• apps/extension/entrypoints/sidepanel/agent-conversation-events.tsx
• apps/extension/entrypoints/sidepanel/agents-composer.tsx
• apps/extension/entrypoints/sidepanel/agents-conversation-adapter.ts
• apps/extension/src/shared/extension-agent-session-manager.ts

Incremental review of commit e6fa76b3 (4 files, +31/-51 since bfbdc4cd): a pure simplification pass with no behavior change. ToolExchangePanel.imageAlt is now a required prop and both file-local call sites pass a string, so the removed ?? fallback is unreachable. The composer renders equivalent output in both states — Send is always mounted (flex-1 while streaming, w-full otherwise) and Stop mounts only while streaming with type="button" and the canInterrupt gate preserved, so the send/stop slot occupancy and layout-shift behavior are unchanged. getStreamingTextPartId now uses findLast on messages and parts; traced against all 5 existing tests (last streaming message's last non-snapshot text part; undefined for snapshot-only, completed, and errored tails) — all match, and findLast (ES2023) has equal-or-broader engine support than the toReversed it replaces. readActiveHistoryWithLivenessGrace swaps try/catch for .catch(() => null): a rejected probe still resolves the original empty page, a working session still escalates to the bounded history retry, an isSessionWorking throw still propagates (outside the catch in both forms), and the loop stays bounded at 3 grace reads. Memory-leak check: no new listeners, timers, or retained state in any changed file. No logic errors, no security issues, no typos.

Previous Review Summaries (16 snapshots, latest commit bfbdc4c)

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

Previous review (commit bfbdc4c)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts
• apps/extension/src/shared/extension-agent-session-manager.test.ts

Incremental review of commit 13875fd0 (2 files, +172/-120 since 31a05a48): a confirmed-working session with an empty first page now resolves immediately — the gate flipped to active !== null && !isSessionWorking(active), so only a not-confirmed-working session enters the bounded liveness grace probe, and the 120s history retry no longer blocks the session switch or the WebSocket transport. The watermark forwarding is untouched, so the transport still seeds fromId=0 and the ingest-DO replay renders the persisted transcript — the documented replacement safety net. The grace probe's fetchActiveSessions() is now guarded: a rejected probe resolves the original empty page instead of failing the snapshot fetch, matching the gate-probe behavior; the current !== null check is required for TS narrowing across the try/catch. readActiveHistoryWithRetry remains reachable only via the grace-probe-turns-busy path, and its unguarded recheck is pre-existing and unchanged. All 5 new/rewritten tests were traced line-by-line against the implementation: call counts (pageQuery 1/11/11, listQuery 1/2/3), fake-timer advances (1s/15s), and exact empty-page shapes all match, and the two prompt-resolution tests correctly need no fake timers since no sleep is reached. Memory-leak check: no new listeners, timers, or retained state; every loop stays bounded (3 grace reads, 120 retry reads, 5 end-grace reads). No logic errors, no security issues, no typos.

Previous review (commit 13875fd)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts
• apps/extension/src/shared/extension-agent-session-manager.test.ts

Incremental review of commit 13875fd0 (2 files, +172/-120 since 31a05a48): a confirmed-working session with an empty first page now resolves immediately — the gate flipped to active !== null && !isSessionWorking(active), so only a not-confirmed-working session enters the bounded liveness grace probe, and the 120s history retry no longer blocks the session switch or the WebSocket transport. The watermark forwarding is untouched, so the transport still seeds fromId=0 and the ingest-DO replay renders the persisted transcript — the documented replacement safety net. The grace probe's fetchActiveSessions() is now guarded: a rejected probe resolves the original empty page instead of failing the snapshot fetch, matching the gate-probe behavior; the current !== null check is required for TS narrowing across the try/catch. readActiveHistoryWithRetry remains reachable only via the grace-probe-turns-busy path, and its unguarded recheck is pre-existing and unchanged. All 5 new/rewritten tests were traced line-by-line against the implementation: call counts (pageQuery 1/11/11, listQuery 1/2/3), fake-timer advances (1s/15s), and exact empty-page shapes all match, and the two prompt-resolution tests correctly need no fake timers since no sleep is reached. Memory-leak check: no new listeners, timers, or retained state; every loop stays bounded (3 grace reads, 120 retry reads, 5 end-grace reads). No logic errors, no security issues, no typos.

Previous review (commit 31a05a4)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts
• apps/extension/src/shared/extension-agent-session-manager.test.ts

Incremental review of commits 4160d52f..31a05a48 (2 files, +49/-12): the initial activeSessions.list liveness probe in fetchExtensionSessionSnapshotPage is now guarded so a rejected probe resolves the original empty first page instead of failing the whole snapshot fetch. The guard is correctly scoped to the initial probe only — the already-successful page query and all later retry/grace probes still propagate failures, and the event-log watermark forwarding still applies as the replay safety net for a reopened running session. ActiveSessionsResult is non-nullable, so the active !== null check cleanly identifies the rejection path. The new unit test matches the implementation: exact empty-page shape, single page query, and no fake timers needed since the rejection path skips every sleep loop. Memory-leak check: no new listeners, timers, or retained state. No logic errors, no security issues, no typos.

Previous review (commit 4160d52)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files, incremental)

• apps/extension/entrypoints/sidepanel/agents-conversation-adapter.ts
• apps/extension/entrypoints/sidepanel/agents-conversation-adapter.test.ts
• apps/extension/entrypoints/sidepanel/agents-message-list.tsx
• apps/extension/src/shared/agent-tool-images.ts
• apps/extension/src/shared/agent-tool-images.test.ts

Incremental review of commits 604240a9..4160d52f (5 files, +178/-21 since 5e38da8c): shouldShowWorkingIndicator moved from agents-message-list.tsx into the conversation adapter and now also shows while the newest streaming message contains only the hidden synthetic snapshot progress, closing the dead-air gap before the first live token; the move leaves no dangling imports or other callers. rememberToolImage now requires a data:image/ URL instead of only rejecting empty strings — verified against the sole production path (SDK onToolAttachment forwards attachment.url, which can be an external URL), so the stricter guard correctly prevents remote image URLs from being stored and later rendered by the panel. All 9 new unit tests match the implementation, and the test store reset hygiene is intact. Memory-leak check: no new listeners, timers, or unbounded retention — the bounded 50-entry FIFO image map is unchanged and the new guard only stores less. No logic errors, no security issues, no typos.

Previous review (commit 5e38da8)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit 5e38da8c (1 file, +51/-1 since 2db9f79c): the mocked cliSessionsV2.getSessionMessagesPage now seeds any active cloud-agent session's first page with its first user message, using ids (msg-u-1/p-u-1) and text that exactly match the default WebSocket stream's own user message so SDK upserts merge page and stream without duplicate transcript rows, while unknown/remote sessions still get an empty page that now correctly echoes the requested kiloSessionId. Verified the seeded shape against the consumer (pageHistory reads history.messages; the manager re-pins sessionID), confirmed existing activeSessions overrides in agents-mode.test.ts are unaffected, and found no memory-leak surface — the handler returns a fresh object literal per call with no listeners, timers, or retained state. No logic errors, no security issues, no typos.

Files Reviewed (1 file, incremental)

• apps/extension/tests/e2e/agents-fixture.ts

Previous review (commit 2db9f79)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit 2db9f79c (1 file, +16/-3 since 9c35b2f4): the live E2E reopenRunningSession nonce probe now holds the conversation pane pinned to the top across 6 animation frames instead of a single scrollTop = 0, so the virtualized list's pin-to-bottom re-render can't snap the scroll back before the visibility re-check. The requestAnimationFrame chain is strictly bounded (decrement-then-resolve at 0), creates no listeners, intervals, or retained state (no memory-leak surface), rejects safely into the existing try/catch → false path if the execution context is destroyed, and any pathological rAF stall degrades to a loud expect.poll 60s timeout rather than a hang (test.setTimeout(150_000) backstops it). Frame counting is exact (6 applications, no off-by-one) and matches the auto-scroll-release rationale documented in the comment above. No logic errors, no security issues, no typos.

Files Reviewed (1 file, incremental)

• apps/extension/tests/e2e/agents-mode-live.test.ts

Previous review (commit 9c35b2f)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit 9c35b2f4 (2 files, +102/-52 since 84b5ffb2): the bounded liveness probe in fetchExtensionSessionSnapshotPage now covers listed-idle sessions, not just absent ones — the isSessionListedIdle early-exit and the listed-idle immediate latch were removed, so a reopened running session that briefly lists idle (session-ingest lag / read-model refresh) is re-checked for up to ~3s (ACTIVE_HISTORY_LIVENESS_GRACE_READS=3 × 1s) before the empty page is latched, and escalates to the full active-history retry the moment the session reads busy. Verified end-to-end: the ternary gate covers every non-working case, the removed helper has no dangling references, all loops remain bounded (no memory-leak surface — no new listeners, intervals, or unbounded collections), the intentional ~3s first-page delay for genuinely idle fresh sessions is documented in the code comment and test names, and both rewritten fake-timer tests match the implementation exactly (delay=1000ms: listed-idle→busy test = 1 probe sleep + 2 retry sleeps = 3000ms advance, 2 list calls / 3 page calls; stably-idle test = 1 gate + 3 probe reads = 4 list calls / 1 page call). No logic errors, no security issues, no typos.

Files Reviewed (2 files, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts
• apps/extension/src/shared/extension-agent-session-manager.test.ts

Previous review (commit 84b5ffb)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit 84b5ffb2 (2 files, +90 lines since cd897886): fetchExtensionSessionSnapshotPage now forwards the tRPC page result's watermarkEventId on both success-page outcomes, so the transport seeds its first WebSocket connect with fromId=0 and the ingest DO replays every persisted event — closing the gap where a reopened running session's initial page is still empty while its user message is already persisted. Verified end-to-end against the router (watermarkEventId: number | null), the SDK type (SessionSnapshotPage.watermarkEventId?: number | null), and the transport consumer (lastEventId = page.watermarkEventId != null ? 0 : null); absent/null watermarks correctly stay absent (replay=false), the typed-failure passthrough path correctly omits the watermark, and the watermark is taken from the final post-retry page read. No logic errors, no security issues, no memory leaks; both new fake-timer tests match the implementation exactly.

Files Reviewed (2 files, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts
• apps/extension/src/shared/extension-agent-session-manager.test.ts

Previous review (commit cd89788)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit cd897886 (1 file, +69 lines since 9c69197): adds a bounded liveness-grace probe (ACTIVE_HISTORY_LIVENESS_GRACE_READS = 3) in fetchExtensionSessionSnapshotPage so a running session that temporarily misses its active-sessions row is not latched as an empty transcript. The probe re-reads liveness only (no page re-read) and hands off to the existing active-history retry when the row appears busy, or resolves empty when idle/absent. Tests with fake timers cover the miss→busy→persisted path and the inactive fast-path. No logic errors, no security issues, no memory leaks.

Files Reviewed (1 file, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts

Previous review (commit 9c69197)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit 9c69197 (1 file, −10 lines since cb69786): the two [agents-debug] console.info trace statements in fetchExtensionSessionSnapshotPage were removed — pure debug-logging cleanup with no logic change, no leftover debug references, and the delayed-history retry flow verified in prior reviews remains intact.

Files Reviewed (1 file, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts

Previous review (commit cb69786)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit cb69786 (1 file, +10 lines since bb57c23): two [agents-debug] console.info trace statements were added around the delayed-history retry in fetchExtensionSessionSnapshotPage — no logic changed, logged values (session IDs, message counts, statuses) contain no credentials, and the retry flow verified in the previous review is untouched.

Files Reviewed (1 file, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts

Previous review (commit bb57c23)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit bb57c23 (2 files, +298/-24 since 07bfd76): the running-session history retry was reworked (8s → 120s bounded window with 5-read liveness rechecks and a 5-read end-grace window, gated on non-idle status) and verified line-by-line against the activeSessions.list status contract, the SDK page-application guards, and all four new/updated tests — no correctness, security, or memory-leak issues found.

Notes verified during incremental review

• The retry gate now requires session.status !== 'idle', matching the server contract: CLI heartbeat rows carry busy/idle (with question/permission attention overlays), and merged cloud rows carry the stored status and are only listed when a non-terminal run exists or they are warm-idle (active-sessions-router.ts:355-401). Fresh (idle) sessions resolve on the first read, so the first prompt send is never blocked; cursor pages still skip both the active lookup and the retry.
• isPageWithoutPersistedMessages treats both null history and zero-message pages as retryable while passing typed failure variants through unchanged, preserving existing failure semantics.
• Loop bounds traced exactly: recheck fires on attempts 4, 9, … via (attempt + 1) % 5 === 0; the stop-grace path ends after 5 extra reads. New tests' expectations match precisely (11 page reads / 2 list calls for the stop case, 10s and 1s fake-timer advances), with proper vi.useRealTimers() cleanup in finally.
• Memory-leak scrutiny: the retry holds no listeners or accumulating state — each iteration's setTimeout promise resolves and is garbage-collected, the loop is hard-bounded (120 reads + ~24 liveness lists + 5 grace reads), and the SDK's generation guards (applyPage / loadOlderGeneration in session-manager.ts) silently discard results that arrive after a session switch.
• Errors from queryPage or the liveness recheck propagate to the SDK's existing catch paths (retryable outcome / onError), so no unhandled rejections are introduced.

Files Reviewed (2 files, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts
• apps/extension/src/shared/extension-agent-session-manager.test.ts

Previous review (commit 07bfd76)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit 07bfd76 (2 files, +112/-11 since 869fe33): the new active-session history retry in the extension page adapter was verified line-by-line against the activeSessions.list router contract and both SDK transports, and no correctness, security, or memory-leak issues were found.

Notes verified during incremental review

• The retry only fires when the initial page returns history === null and no cursor was passed; cursor pages correctly skip both the active lookup and the retry (extension-agent-session-manager.ts:172).
• activeSessions.list accepts { includeCloudAgentSessions: true, organizationId } (active-sessions-router.ts:100-116), and merged cloud-agent rows expose the kilo session id as id, so the isActive membership check matches the server contract (active-sessions-router.ts:368-401).
• Errors from the new active lookup or retry queries propagate to the SDK, which catches them and surfaces onError in both transports (cloud-agent-transport.ts:310-312, cli-live-transport.ts:793) — no unhandled rejections.
• The retry loop is bounded (8 retries × 1s fixed delay, no trailing sleep) with no listeners or retained closures, and SDK generation guards discard stale results — no memory-leak risk introduced.
• New tests cover retry-when-active, no-retry-when-inactive, no-retry-with-cursor, and org-context propagation, with proper fake-timer cleanup in finally.

Files Reviewed (2 files, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts
• apps/extension/src/shared/extension-agent-session-manager.test.ts

Previous review (commit 869fe33)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit 869fe33 (2 files, +129/-18 since the previous review): the pinPageToSession replay normalization in the extension session manager was verified line-by-line against the cloud-agent-sdk session manager, and no correctness, security, or memory-leak issues were found.

Notes verified during incremental review

• The doc comment's rationale matches SDK behavior: applyPage discards pages whose info.id !== activeSessionId (session-manager.ts:1084), switchSession/onSessionCreated seed rootSessionIdAtom from the requested id and the page's info.id (session-manager.ts:1202,1273), and the root transcript drops messages whose info.sessionID !== rootSessionId (session-manager.ts:597) — so a worker-persisted session-id mismatch would previously render the reopened transcript empty.
• pinPageToSession spreads preserve every field (...page, ...page.info, ...message.info, ...part); the mapped message shape { info, parts } matches SessionSnapshot['messages'] exactly, so no typed or server-validated fields are dropped. nextCursor, omittedItemCount, and watermarkEventId pass through untouched.
• All parts carry sessionID via sdkPartBaseShape (rpc-contract.ts:288-292), so the part-level rewrite matches the persisted schema.
• Non-page history outcomes (too_large / retryable_failure / invalid_data) carry no messages and are correctly returned unpinned.
• The new normalizes a mismatched server session id to the requested id unit test exercises both info.id and per-message/part sessionID rewriting; the updated matching-id test confirms the pin is a no-op when ids already agree.
• The change is a pure per-page mapping with no listeners, timers, or retained closures, so no memory-leak risk is introduced.

Files Reviewed (2 files, incremental)

• apps/extension/src/shared/extension-agent-session-manager.ts
• apps/extension/src/shared/extension-agent-session-manager.test.ts

Previous review (commit eaa8652)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of commit eaa8652 (1 file, +63/-2 since the previous review): the live E2E hardening in agents-mode-live.test.ts — the 1→300 long-prompt bump and the virtualization-aware nonce poll with Jump to latest re-engage — was verified line-by-line against conversation-list.tsx, and no correctness, security, or memory-leak issues were found.

Notes verified during incremental review

• sidePanel.getByLabel('Agent conversation') targets the scrollable <section ref={listRef}> (overflow-y-auto) in conversation-list.tsx:280, and the Agents session view renders this shared ConversationList via agents-message-list.tsx:61, so the programmatic scrollTop writes in the new poll act on the real scroll container.
• The test's scroll-up assumption matches component behavior: a programmatic scrollTop = 0 fires the handleScroll backstop (currentTop < previous && currentTop < lastPinned && not at bottom), which calls releaseToManualScroll() and shows the Jump to latest button (conversation-list.tsx:229-238, 308-317).
• The Jump to latest re-engage matches jumpToLatest() → followBottomAgain() + forced pinToBottom, and the isVisible() guard correctly no-ops in the existing-session fallback where the nonce is already at the pinned bottom.
• The 100→300 prompt increase is documented with a live measurement (~33s runtime for 100) and keeps the session busy through the reopen, queue, and stop phases; it is test-only.
• No new listeners, timers, or retained buffers are introduced by the poll closure, so no memory-leak risk is added.

Files Reviewed (1 file, incremental)

• apps/extension/tests/e2e/agents-mode-live.test.ts

Previous review (commit 169c03c)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Reviewed all 17 changed files for the Agents-tab browser-renderer parity work: the new StoredMessage → shared-conversation adapter, the bounded tool-image store, the shared tool-exchange panel, the gateway-replay filter for agent-source tool calls, the streaming composer split (Send + Stop), and the expanded unit/E2E/live coverage — no correctness, security, or memory-leak issues found.

Notes verified during review

• agent-tool-images.ts keeps screenshot bytes in a module map capped at MAX_TOOL_IMAGES (50) with oldest-first eviction, so the long-lived side panel cannot grow it without bound; the fixed-count-vs-byte-budget tradeoff is documented in the file.
• isSnapshotProgressPart is an accurate mirror of apps/mobile/src/components/agents/part-types.ts:15, and both transcript mapping and getStreamingTextPartId skip synthetic snapshot parts.
• buildGatewayMessagesFromEvents filters agent-source tool calls out of the gateway replay while still advancing the index over the full consecutive run, and agent tool-results are safely skipped via the `toolCallsById

[Snapshot truncated.]

Additional previous summary content was truncated to keep this comment within platform limits.

---

_{Reviewed by kimi-k3 · Input: 211K · Output: 8.7K · Cached: 301.6K}

_{Review guidance: REVIEW.md from base branch main}