Skip to content

feat(cloud-agent): send saved diff reviews to chats - #5817

Open
eshurakov wants to merge 7 commits into
eshurakov/boundless-sphinxfrom
eshurakov/spry-mangrove
Open

feat(cloud-agent): send saved diff reviews to chats#5817
eshurakov wants to merge 7 commits into
eshurakov/boundless-sphinxfrom
eshurakov/spry-mangrove

Conversation

@eshurakov

@eshurakov eshurakov commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add saved-diff line/range comments, cross-file drafts, and one batched Send review to agent action.
  • Preserve immutable reviewed context and explicit stale-review handling across file and chat switches.
  • Pin and authorize the destination worktree/chat, retaining the same message identity for uncertain retries.

Verification

No manual browser or live-provider tests were rerun during this restack. Existing pending work was preserved in named stash/backup refs; local handoff and evidence files remain unchanged and excluded.

Visual Changes

Adds direct diff selection, inline feedback, draft review controls, and a review dialog. No screenshots were captured or uploaded during this restack.

Reviewer Notes

Stacked on #5816 (eshurakov/boundless-sphinx). Three commits, including GitHub-style review UX. The merge preserves both commit-card and review wiring, plus upstream autoCommit: true and expected-worktree authorization checks.

Final combined verification passed: web lint, full web typecheck with tRPC declaration build, and eight setup-free Jest suites / 241 tests, including the native review wrappers. All 29 terminal-navigation tests pass after adding the missing review-boundary mocks to their existing fixture; this correction changes no product behavior. The standalone native review suites (77 tests) and review-send/client suites (131 tests) also passed earlier. Formatting, whitespace, and independent merge review passed. No database setup or browser evidence is implied by these isolated tests.

@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 987fa7c to 14b8fdb Compare September 1, 2026 19:41
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch 2 times, most recently from b22f8fa to a71610f Compare September 2, 2026 12:52
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from a71610f to 4cf9d58 Compare September 3, 2026 07:47
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 4cf9d58 to 2c2abda Compare September 3, 2026 07:50
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 2c2abda to 10eaade Compare September 3, 2026 11:37
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 10eaade to 3dd0a0e Compare September 3, 2026 12:31
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 3dd0a0e to 7cac306 Compare September 3, 2026 12:58
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 7cac306 to 8467a86 Compare September 3, 2026 20:21
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 8467a86 to 6f4a8c9 Compare September 7, 2026 20:17
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch 2 times, most recently from fcfc693 to 30bc04e Compare September 7, 2026 20:45
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 30bc04e to 26658b4 Compare September 7, 2026 20:53
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 26658b4 to 9a0611a Compare September 8, 2026 10:29
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 9a0611a to 97ac3a6 Compare September 8, 2026 10:55
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 97ac3a6 to 24ccbf4 Compare September 8, 2026 12:29
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from 24ccbf4 to c25db45 Compare September 8, 2026 12:57
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from c25db45 to 11e3830 Compare September 9, 2026 21:33
Add inline line-range comments, compact review summary, JSON agent
payload, condensed chat card, and IndexedDB drafts.
@eshurakov
eshurakov force-pushed the eshurakov/spry-mangrove branch from cce3370 to 30c4b5f Compare September 10, 2026 08:42
ConversationMessages loads MessageBubble, which imports worktree-review.
Jest cannot parse @pierre/diffs ESM, so the suite failed in CI.
@eshurakov
eshurakov marked this pull request as ready for review September 10, 2026 09:06
Comment thread apps/web/src/components/cloud-agent-next/useWorktreeReview.ts
Comment thread apps/web/src/components/cloud-agent-next/WorktreeReviewEditor.tsx
Comment thread apps/web/src/components/cloud-agent-next/WorktreeReviewEditor.tsx
Comment thread apps/web/src/components/cloud-agent-next/worktree-review-state.ts
@kilo-code-bot

kilo-code-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Fix these issues in Kilo Cloud

Overview

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

WARNING

File Line Issue
apps/web/src/components/cloud-agent-next/worktree-review-selection.ts 54 Truthy check treats unified index 0 as missing
Files Reviewed (6 files)
  • apps/web/src/components/cloud-agent-next/WorktreeReviewEditor.tsx
  • apps/web/src/components/cloud-agent-next/worktree-review-bindings.renderer.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review-bindings.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-selection.ts - 1 issue
  • apps/web/src/components/cloud-agent-next/worktree-review.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review.ts
Previous Review Summaries (4 snapshots, latest commit ce81aaf)

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

Previous review (commit ce81aaf)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • apps/web/src/components/cloud-agent-next/worktree-review-state.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review-state.ts

Previous review (commit 23a0bc6)

Status: 1 Issue Found | Recommendation: Address before merge

Fix these issues in Kilo Cloud

Executive Summary

A generation-mismatch IndexedDB clear can delete a newer in-progress review draft after accept or discard.

Overview

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

WARNING

File Line Issue
apps/web/src/components/cloud-agent-next/worktree-review-state.ts 276 Stale persist generation clears can wipe a newer draft
Files Reviewed (2 files)
  • apps/web/src/components/cloud-agent-next/useWorktreeReview.test.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-state.ts - 1 issue

Previous review (commit 4671023)

Status: 1 Issue Found | Recommendation: Address before merge

Fix these issues in Kilo Cloud

Executive Summary

A generation-mismatch IndexedDB clear can delete a newer in-progress review draft after accept or discard.

Overview

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

WARNING

File Line Issue
apps/web/src/components/cloud-agent-next/worktree-review-state.ts 276 Stale persist generation clears can wipe a newer draft
Files Reviewed (8 files)
  • apps/web/src/components/cloud-agent-next/WorktreeReviewEditor.tsx
  • apps/web/src/components/cloud-agent-next/useWorktreeReview.test.ts
  • apps/web/src/components/cloud-agent-next/useWorktreeReview.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-bindings.renderer.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review-state.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review-state.ts - 1 issue
  • apps/web/src/components/cloud-agent-next/worktree-review.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review.ts

Previous review (commit 82fa7e4)

Status: 4 Issues Found | Recommendation: Address before merge

Fix these issues in Kilo Cloud

Overview

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

WARNING

File Line Issue
apps/web/src/components/cloud-agent-next/useWorktreeReview.ts 334 Unrebasable comments are dropped silently, then wiped on accept
apps/web/src/components/cloud-agent-next/WorktreeReviewEditor.tsx 147 Unsaved editors are discarded while the diff is still loading
apps/web/src/components/cloud-agent-next/WorktreeReviewEditor.tsx 141 Path-wide rebase deletes or reassigns comments from other source chats
apps/web/src/components/cloud-agent-next/worktree-review-state.ts 529 Accepted reviews can rehydrate from IndexedDB and be sent twice
Files Reviewed (40 files)
  • apps/web/src/components/cloud-agent-next/CloudAgentProvider.tsx
  • apps/web/src/components/cloud-agent-next/CloudChatPage.tsx
  • apps/web/src/components/cloud-agent-next/ConversationMessages.test.ts
  • apps/web/src/components/cloud-agent-next/MessageBubble.test.ts
  • apps/web/src/components/cloud-agent-next/MessageBubble.tsx
  • apps/web/src/components/cloud-agent-next/WorktreeChanges.tsx
  • apps/web/src/components/cloud-agent-next/WorktreeFilePane.tsx
  • apps/web/src/components/cloud-agent-next/WorktreeFileRenderer.tsx
  • apps/web/src/components/cloud-agent-next/WorktreeReviewCommentForm.tsx
  • apps/web/src/components/cloud-agent-next/WorktreeReviewDialog.tsx
  • apps/web/src/components/cloud-agent-next/WorktreeReviewEditor.tsx - 2 issues
  • apps/web/src/components/cloud-agent-next/WorktreeReviewList.tsx
  • apps/web/src/components/cloud-agent-next/WorktreeReviewMessageCard.tsx
  • apps/web/src/components/cloud-agent-next/terminal-tabs.test.ts
  • apps/web/src/components/cloud-agent-next/useWorktreeReview.test.ts
  • apps/web/src/components/cloud-agent-next/useWorktreeReview.ts - 1 issue
  • apps/web/src/components/cloud-agent-next/worktree-review-bindings.renderer.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review-bindings.renderer.test.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-bindings.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-persistence.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review-persistence.test.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-persistence.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-selection.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-send.test.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-send.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-state.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review-state.test.ts
  • apps/web/src/components/cloud-agent-next/worktree-review-state.ts - 1 issue
  • apps/web/src/components/cloud-agent-next/worktree-review.test.mts
  • apps/web/src/components/cloud-agent-next/worktree-review.test.ts
  • apps/web/src/components/cloud-agent-next/worktree-review.ts
  • apps/web/src/lib/cloud-agent-next/cloud-agent-client.test.ts
  • apps/web/src/lib/cloud-agent-next/cloud-agent-client.ts
  • apps/web/src/lib/cloud-agent-next/worktree-review-access.ts
  • apps/web/src/routers/cloud-agent-next-router.test.ts
  • apps/web/src/routers/cloud-agent-next-router.ts
  • apps/web/src/routers/cloud-agent-next-schemas.ts
  • apps/web/src/routers/organizations/organization-cloud-agent-next-router.test.ts
  • apps/web/src/routers/organizations/organization-cloud-agent-next-router.ts
  • packages/worker-utils/src/cloud-agent-worktree-changes.ts

Reviewed by grok-4.6 · Input: 188.2K · Output: 19.2K · Cached: 411.6K

Review guidance: REVIEW.md from base branch eshurakov/boundless-sphinx

Do not send a silent subset, rebase other-chat comments, wipe unsaved
editors while loading, or resurrect an accepted draft from a late IDB save.
Comment thread apps/web/src/components/cloud-agent-next/worktree-review-state.ts Outdated
A late preparing save must rewrite the in-memory draft, not clear a
newer one.
Send the agent the unified rows the user selected, including both
deletions and additions in a replacement hunk. Derive the stored
range from the quote; rebase on unique quote text.
const lineNumber = Number(row.getAttribute('data-line'));
const unifiedIndexValue = row.getAttribute('data-line-index');
const unifiedIndexPart = unifiedIndexValue?.split(',')[0]?.trim();
const unifiedIndex = unifiedIndexPart ? Number(unifiedIndexPart) : Number.NaN;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Truthy check treats unified index 0 as missing

unifiedIndexPart ? Number(unifiedIndexPart) is false for "0", so the first unified row (data-line-index="0,0") is rejected as a hidden-line gap. Any comment selection that includes the first rendered row fails, including the mixed-side cases this commit adds.

Suggested change
const unifiedIndex = unifiedIndexPart ? Number(unifiedIndexPart) : Number.NaN;
const unifiedIndex = unifiedIndexPart === undefined || unifiedIndexPart === '' ? Number.NaN : Number(unifiedIndexPart);

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

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.

1 participant