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
This commit is contained in:
Jinwoo-H
2026-09-07 19:16:14 -04:00
parent dcec24ecf2
commit 71b1d19552
2 changed files with 94 additions and 1 deletions
@@ -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. */
@@ -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) }) })