Skip to content

fix(agent-manager): reload the PR diff when a review snapshot expires - #14183

Merged
marius-kilocode merged 2 commits into
mainfrom
fix-comment-bug
Sep 15, 2026
Merged

marius-kilocode merged 2 commits into
mainfrom
fix-comment-bug

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

What Problem This Solves

Posting an inline pull request review comment from an Agent Manager diff could fail with:

Review snapshot expired or belongs to another pull request. Reload the review.

and stay broken: retrying reused the same rejected snapshot, so the comment could not be posted until the whole review was reopened.

Why This Change Was Made

The Agent Manager diff surfaces cache a PR snapshot per review target in createPRDiffCommentState (webview-ui/agent-manager/pr/diff-comment-state.ts) and never refetch a target they already hold. The host keeps at most eight snapshots per PRReviewActions instance and clears them on project re-activation (review-actions.ts, PRStatusBridge.reset). Once the host dropped a snapshot, the webview kept sending its id, so every retry failed and the documented recovery ("Reload the review") never happened on its own.

The fix subscribes the diff comment state to review results. When a agentManager.createReviewComment fails, it drops the cached snapshot for the matching target and reloads it. A failed post is deliberately not auto-retried (it may already have reached GitHub), so the draft is kept and the next send runs against a snapshot the host still holds.

The webview ExtensionMessage union now also includes PRReviewResult, which the host already sends but the webview types did not declare.

User Impact

An inline PR review comment that fails because its snapshot expired now recovers: the diff reloads in the background and the next send succeeds, instead of requiring the user to close and reopen the review.

Evidence

  • packages/kilo-vscode/tests/unit/pr-diff-comment-state.test.ts drives the state directly and reproduces the failure. Without the fix, a rejected comment does not trigger a reload and the test fails with Expected length: 2, Received length: 1. With the fix, the stale snapshot is dropped, a new loadPRFiles request is posted, and the fresh snapshot replaces it.
  • Focused tests: bun test tests/unit/pr-diff-comment-state.test.ts tests/unit/pr-review-actions.test.ts tests/unit/diff-comment-form.test.ts tests/unit/diff-preview-request.test.ts tests/unit/agent-manager-arch.test.ts (136 pass).
  • eslint and prettier --check clean on the changed files.

The Agent Manager diff surfaces cache PR snapshots per target while the host keeps only the eight most recent, so a cached snapshot can be evicted and every comment post then fails with "Review snapshot expired". Drop the cached snapshot and reload it when the host rejects a comment, so the next attempt is bound to a snapshot the host still holds.
@kilo-code-bot

kilo-code-bot Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/kilo-vscode/webview-ui/agent-manager/pr/diff-comment-state.ts 101 Review submission (agentManager.submitPRReview) shares the same cached snapshot but is not covered by the reload listener
Files Reviewed (2 files)
  • packages/kilo-vscode/webview-ui/agent-manager/pr/diff-comment-state.ts - 1 unresolved suggestion (submit path not covered by the reload)
  • packages/kilo-vscode/tests/unit/pr-diff-comment-state.test.ts - no issues

Note: the new expired filter (lines 92/95) correctly restricts reloads to the host's reload hints and resolves the prior "reloads on every failure" suggestion.

Fix these issues in Kilo Cloud

Previous Review Summary (commit 3a98691)

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

Previous review (commit 3a98691)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 2
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/kilo-vscode/webview-ui/agent-manager/pr/diff-comment-state.ts 90 Reload triggers on every failed comment post, not only snapshot expiry
packages/kilo-vscode/webview-ui/agent-manager/pr/diff-comment-state.ts 96 Review-submit path shares the same cached snapshot but is not covered by the reload
Files Reviewed (5 files)
  • packages/kilo-vscode/webview-ui/agent-manager/pr/diff-comment-state.ts - 2 suggestions
  • packages/kilo-vscode/webview-ui/agent-manager/AgentManagerApp.tsx - no issues
  • packages/kilo-vscode/webview-ui/src/types/messages/extension-messages.ts - no issues
  • packages/kilo-vscode/tests/unit/pr-diff-comment-state.test.ts - no issues
  • .changeset/pr-review-snapshot-reload.md - no issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

Content rejections such as an empty body or a bad line range leave the snapshot valid, so gate the reload on the host's reload hints instead of every failed comment. Extend the regression test to cover the non-expiry cases.
@marius-kilocode
marius-kilocode merged commit 1598581 into main Sep 15, 2026
25 checks passed
@marius-kilocode
marius-kilocode deleted the fix-comment-bug branch September 15, 2026 16:56
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