From 71b1d19552ddb148f649ec280d49f7e2946338a8 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Mon, 7 Sep 2026 19:16:14 -0400 Subject: [PATCH] fix(desktop): fit review files and comments to the bridge envelope Per-field limits did not guarantee the aggregate review fit the shell's byte envelope; the projection now admits files and the newest comments by remaining bytes and flags truncation. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../methods/mobile-web-review-projection.ts | 27 +++++++- .../methods/mobile-web-review-reads.test.ts | 68 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/main/runtime/rpc/methods/mobile-web-review-projection.ts b/src/main/runtime/rpc/methods/mobile-web-review-projection.ts index e9f6fe2821f..d7d4a3ae984 100644 --- a/src/main/runtime/rpc/methods/mobile-web-review-projection.ts +++ b/src/main/runtime/rpc/methods/mobile-web-review-projection.ts @@ -1,3 +1,4 @@ +import { MOBILE_WEB_BRIDGE_MAX_OPERATION_BYTES } from '../../../../shared/mobile-web/bridge-limits' import type { PRCheckDetail } from '../../../../shared/github/check-types' import type { GitHubAssignableUser, @@ -57,7 +58,7 @@ export function projectMobileWebReview( const comments = projectMobileWebReviewComments(details.provider, details.item.comments) const files = projectMobileWebReviewFiles(details) const participants = details.provider === 'github' ? details.item.item : null - return MobileWebProviderReviewSchema.parse({ + const review = MobileWebProviderReviewSchema.parse({ ...base, ...(headSha ? { headSha } : {}), body: details.item.body.slice(0, MOBILE_WEB_PROVIDER_REVIEW_BODY_MAX_CHARACTERS), @@ -73,6 +74,30 @@ export function projectMobileWebReview( canComment: true, allowedSubmissionActions: submissionActions(summary, headSha) }) + review.comments = [] + review.files = [] + // Reserve room for the response identity; JSON escaping counts toward the shell's byte envelope. + let remaining = + MOBILE_WEB_BRIDGE_MAX_OPERATION_BYTES - 8192 - Buffer.byteLength(JSON.stringify(review)) + for (const file of files.items) { + const bytes = Buffer.byteLength(JSON.stringify(file)) + 1 + if (bytes > remaining) { + break + } + remaining -= bytes + review.files.push(file) + } + for (const comment of comments.items.toReversed()) { + const bytes = Buffer.byteLength(JSON.stringify(comment)) + 1 + if (bytes > remaining) { + break + } + remaining -= bytes + review.comments.unshift(comment) + } + review.filesTruncated ||= review.files.length < files.items.length + review.commentsTruncated ||= review.comments.length < comments.items.length + return review } /** A review only accepts a verdict while it is still open, and only against a known head. */ diff --git a/src/main/runtime/rpc/methods/mobile-web-review-reads.test.ts b/src/main/runtime/rpc/methods/mobile-web-review-reads.test.ts index 815898e01b1..5e8e55aff81 100644 --- a/src/main/runtime/rpc/methods/mobile-web-review-reads.test.ts +++ b/src/main/runtime/rpc/methods/mobile-web-review-reads.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it } from 'vitest' +import { MOBILE_WEB_BRIDGE_MAX_OPERATION_BYTES } from '../../../../shared/mobile-web/bridge-limits' +import type { MobileWebProviderReview } from '../../../../shared/mobile-web/provider-review-contract' import { gitHubReviewDetails, gitLabReviewDetails, @@ -108,6 +110,72 @@ describe('host-projected provider review reads', () => { }) }) + it.each([ + ['github', false], + ['gitlab', false], + ['github', true] + ] as const)( + 'fits escaped %s review content with crowded checks=%s in the shell envelope', + async (provider, crowded) => { + const comments = Array.from({ length: 32 }, (_, id) => ({ + id, + author: 'a'.repeat(160), + authorAvatarUrl: '', + url: '', + body: '\u0000'.repeat(4096), + createdAt: '2026-09-07' + })) + const details = { + body: '\u0000'.repeat(32 * 1024), + comments, + files: Array.from({ length: 48 }, (_, index) => ({ + path: `${index}${'界'.repeat(1000)}`, + ...(crowded ? { oldPath: `${index}${'旧'.repeat(1000)}` } : {}), + status: 'modified' as const, + additions: 0, + deletions: 0, + isBinary: false + })) + } + const f = reviewRuntime({ + getRuntimeGitStatus: reviewStatus(), + getHostedReviewForBranch: hostedReviewSummary({ provider }), + ...(provider === 'github' + ? { + getRepoWorkItemDetails: gitHubReviewDetails({ + ...details, + checks: crowded + ? Array.from({ length: 128 }, () => ({ + name: '\u0000'.repeat(256), + status: 'completed' as const, + conclusion: 'success' as const, + url: null + })) + : [] + }) + } + : { getGitLabRepoWorkItemDetails: gitLabReviewDetails(details) }) + }) + const result = (await runReviewMethod( + 'mobileWeb.review.read', + REVIEW_IDENTITY, + f.context + )) as { review: MobileWebProviderReview } + expect(Buffer.byteLength(JSON.stringify(result))).toBeLessThan( + MOBILE_WEB_BRIDGE_MAX_OPERATION_BYTES + ) + expect(result.review.commentsTruncated).toBe(true) + if (!crowded) { + expect(result.review.comments.length).toBeGreaterThan(0) + expect(result.review.comments.at(-1)?.id).toBe('31') + expect(result.review.files).toHaveLength(48) + } else { + expect(result.review.filesTruncated).toBe(true) + expect(result.review.files.length).toBeLessThan(48) + } + } + ) + it('refuses a read composed against a head the repository has moved off', async () => { const f = reviewRuntime({ getRuntimeGitStatus: reviewStatus({ head: 'c'.repeat(40) }) })