mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 09:07:40 +00:00
fix(github): merge successful posts into fetched comment history (#35306)
* fix(github): preserve posted comments across older reads * fix(github): allow comment reads after cache eviction * fix(github): merge in-flight posts into fetched comment history * test(github): trim PR comment read-order tests --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
@@ -6,6 +6,7 @@ import { callRuntimeRpc } from '../../runtime/runtime-rpc-client'
|
||||
import { prCommentsCacheSuffix, sourceScopedRepoCacheKey } from './cache-identity'
|
||||
import { isFresh, withBoundedCacheEntry } from './cache-policy'
|
||||
import { inflightCommentsRequests } from './request-coordination'
|
||||
import { mergePRCommentIntoList } from './pr-comment-cache'
|
||||
import {
|
||||
getGitHubRepoSourceSettings,
|
||||
getGitHubSourceTarget,
|
||||
@@ -39,9 +40,10 @@ export const createCommentFetchActions = (
|
||||
|
||||
const inflightRequest = inflightCommentsRequests.get(cacheKey)
|
||||
if (inflightRequest) {
|
||||
return inflightRequest
|
||||
return inflightRequest.promise
|
||||
}
|
||||
|
||||
const postedComments: PRComment[] = []
|
||||
const request = (async () => {
|
||||
try {
|
||||
const requestContext = getGitHubWorkItemRequestContext(
|
||||
@@ -51,7 +53,7 @@ export const createCommentFetchActions = (
|
||||
repoPath,
|
||||
options?.sourceContext
|
||||
)
|
||||
const comments =
|
||||
let comments: PRComment[] =
|
||||
requestContext.target.kind === 'environment'
|
||||
? await callRuntimeRpc<PRComment[]>(
|
||||
{ kind: 'environment', environmentId: requestContext.target.environmentId },
|
||||
@@ -64,14 +66,22 @@ export const createCommentFetchActions = (
|
||||
},
|
||||
{ timeoutMs: 30_000 }
|
||||
)
|
||||
: ((await window.api.gh.prComments({
|
||||
: await window.api.gh.prComments({
|
||||
repoPath,
|
||||
repoId,
|
||||
prNumber,
|
||||
prRepo: options?.prRepo ?? null,
|
||||
noCache: options?.force,
|
||||
sourceContext: options?.sourceContext
|
||||
})) as PRComment[])
|
||||
})
|
||||
if (postedComments.length > 0) {
|
||||
const currentById = new Map(
|
||||
(get().commentsCache[cacheKey]?.data ?? []).map((comment) => [comment.id, comment])
|
||||
)
|
||||
for (const posted of postedComments) {
|
||||
comments = mergePRCommentIntoList(comments, currentById.get(posted.id) ?? posted)
|
||||
}
|
||||
}
|
||||
set((s) => ({
|
||||
commentsCache: withBoundedCacheEntry(s.commentsCache, cacheKey, {
|
||||
data: comments,
|
||||
@@ -87,7 +97,7 @@ export const createCommentFetchActions = (
|
||||
}
|
||||
})()
|
||||
|
||||
inflightCommentsRequests.set(cacheKey, request)
|
||||
inflightCommentsRequests.set(cacheKey, { promise: request, postedComments })
|
||||
return request
|
||||
}
|
||||
})
|
||||
|
||||
@@ -8,6 +8,7 @@ import { restoreReactionOnSubject, setReactionOnSubject } from '@/lib/pr-comment
|
||||
import { prCommentsCacheSuffix, sourceScopedRepoCacheKey } from './cache-identity'
|
||||
import { withBoundedCacheEntry } from './cache-policy'
|
||||
import { hasUsableCommentPayload, mergePRCommentIntoList } from './pr-comment-cache'
|
||||
import { inflightCommentsRequests } from './request-coordination'
|
||||
import {
|
||||
getGitHubRepoSourceSettings,
|
||||
getGitHubSourceTarget,
|
||||
@@ -102,6 +103,7 @@ export const createCommentMutationActions = (
|
||||
})
|
||||
}
|
||||
})
|
||||
inflightCommentsRequests.get(cacheKey)?.postedComments.push(comment)
|
||||
return { ok: true, comment }
|
||||
},
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@ import { callRuntimeRpc } from '../../runtime/runtime-rpc-client'
|
||||
import { prCommentsCacheSuffix, sourceScopedRepoCacheKey } from './cache-identity'
|
||||
import { withBoundedCacheEntry } from './cache-policy'
|
||||
import { hasUsableCommentPayload, mergePRCommentIntoList } from './pr-comment-cache'
|
||||
import { inflightCommentsRequests } from './request-coordination'
|
||||
import {
|
||||
getGitHubRepoSourceSettings,
|
||||
getGitHubSourceTarget,
|
||||
@@ -89,6 +90,7 @@ export const createConversationCommentActions = (
|
||||
})
|
||||
}
|
||||
})
|
||||
inflightCommentsRequests.get(cacheKey)?.postedComments.push(result.comment)
|
||||
return result
|
||||
}
|
||||
})
|
||||
|
||||
@@ -20,6 +20,10 @@ export type InflightChecks = {
|
||||
/** A user act's request may start a stopped WSL distro; a poll's would be refused. */
|
||||
wakeWslDistro?: boolean
|
||||
}
|
||||
export type InflightComments = {
|
||||
promise: Promise<PRComment[]>
|
||||
postedComments: PRComment[]
|
||||
}
|
||||
export type InflightWorkItems = {
|
||||
promise: Promise<readonly GitHubWorkItem[]>
|
||||
requestId: number
|
||||
@@ -37,7 +41,7 @@ export type InflightProjectView = {
|
||||
export const inflightPRRequests = new Map<string, InflightPR>()
|
||||
export const inflightIssueRequests = new Map<string, Promise<IssueInfo | null>>()
|
||||
export const inflightChecksRequests = new Map<string, InflightChecks>()
|
||||
export const inflightCommentsRequests = new Map<string, Promise<PRComment[]>>()
|
||||
export const inflightCommentsRequests = new Map<string, InflightComments>()
|
||||
export const inflightWorkItemsRequests = new Map<string, InflightWorkItems>()
|
||||
export const inflightProjectViewRequests = new Map<string, InflightProjectView>()
|
||||
export const prRequestGenerations = new Map<string, number>()
|
||||
|
||||
@@ -0,0 +1,85 @@
|
||||
import { beforeEach, expect, it, vi } from 'vitest'
|
||||
import type { PRComment } from '../../../../shared/github/comment-types'
|
||||
import { createTestStore, mockApi, resetRemoteRuntimeMocks } from './github-slice-test-harness'
|
||||
|
||||
const repoPath = '/repo'
|
||||
const repoId = 'comment-race-repo'
|
||||
const prRepo = { owner: 'Acme', repo: 'Widgets' }
|
||||
const options = { repoId, prRepo }
|
||||
const cacheKey = `${repoId}::pr-comments::acme/widgets::12`
|
||||
const original: PRComment = {
|
||||
id: 10,
|
||||
author: 'reviewer',
|
||||
authorAvatarUrl: '',
|
||||
body: 'Before',
|
||||
createdAt: '2026-10-28T00:00:00Z',
|
||||
url: ''
|
||||
}
|
||||
const posted: PRComment = {
|
||||
...original,
|
||||
id: 11,
|
||||
reactionSubjectId: 'IC_11',
|
||||
body: 'Posted reply',
|
||||
createdAt: '2026-10-28T00:01:00Z'
|
||||
}
|
||||
const remoteAdded: PRComment = {
|
||||
...original,
|
||||
id: 12,
|
||||
body: 'Another reviewer posted this',
|
||||
createdAt: '2026-10-28T00:00:30Z'
|
||||
}
|
||||
|
||||
function makeStore(cached = true) {
|
||||
const store = createTestStore()
|
||||
store.setState({
|
||||
repos: [{ id: repoId, path: repoPath, displayName: 'Repo', badgeColor: '#000', addedAt: 0 }],
|
||||
commentsCache: cached ? { [cacheKey]: { data: [original], fetchedAt: 1 } } : {}
|
||||
})
|
||||
return store
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
resetRemoteRuntimeMocks()
|
||||
mockApi.gh.addIssueComment.mockResolvedValue({ ok: true, comment: posted })
|
||||
mockApi.gh.addPRReviewCommentReply.mockResolvedValue({ ok: true, comment: posted })
|
||||
})
|
||||
|
||||
it.each([
|
||||
['comment', true],
|
||||
['comment', false],
|
||||
['reply', true],
|
||||
['reply', false]
|
||||
] as const)('merges a posted %s with complete history with cached=%s', async (kind, cached) => {
|
||||
const store = makeStore(cached)
|
||||
const oldRead = Promise.withResolvers<PRComment[]>()
|
||||
mockApi.gh.prComments.mockReturnValueOnce(oldRead.promise)
|
||||
const reading = store.getState().fetchPRComments(repoPath, 12, { ...options, force: true })
|
||||
const result =
|
||||
kind === 'comment'
|
||||
? await store.getState().addPRConversationComment(repoPath, 12, posted.body, options)
|
||||
: await store.getState().addPRReviewCommentReply(repoPath, 12, 10, posted.body, options)
|
||||
expect(result.ok).toBe(true)
|
||||
const published = store.getState().commentsCache[cacheKey]
|
||||
expect(published?.data?.map(({ id }) => id)).toEqual(cached ? [10, 11] : [11])
|
||||
oldRead.resolve([original, remoteAdded])
|
||||
await expect(reading).resolves.toEqual([original, remoteAdded, posted])
|
||||
expect(store.getState().commentsCache[cacheKey]?.data).toEqual([original, remoteAdded, posted])
|
||||
})
|
||||
|
||||
it('keeps a successful reaction on a newly posted reply while adding fetched history', async () => {
|
||||
const store = makeStore(false)
|
||||
const oldRead = Promise.withResolvers<PRComment[]>()
|
||||
mockApi.gh.prComments.mockReturnValueOnce(oldRead.promise)
|
||||
mockApi.gh.setPRCommentReaction.mockResolvedValueOnce(true)
|
||||
const reading = store.getState().fetchPRComments(repoPath, 12, { ...options, force: true })
|
||||
await store.getState().addPRReviewCommentReply(repoPath, 12, 10, posted.body, options)
|
||||
await store.getState().setPRCommentReaction(repoPath, 12, 'IC_11', 'heart', true, options)
|
||||
oldRead.resolve([original, remoteAdded])
|
||||
const displayed = await reading
|
||||
expect(displayed.map(({ id }) => id)).toEqual([10, 12, 11])
|
||||
expect(displayed.find(({ id }) => id === posted.id)?.reactions).toEqual([
|
||||
{ content: 'heart', count: 1, viewerHasReacted: true }
|
||||
])
|
||||
expect(store.getState().commentsCache[cacheKey]?.data).toEqual(displayed)
|
||||
})
|
||||
Reference in New Issue
Block a user