From 669adf9ff874af130db92ddf112317a6014e5257 Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Sat, 5 Sep 2026 00:34:31 -0400 Subject: [PATCH] [Improve] Pin PR review updates above the session composer --- apps/docs/fast-sessions.mdx | 14 ++ .../FastSessionTranscript.client.test.tsx | 120 ++++++++++- .../[sessionId]/FastSessionTranscript.tsx | 33 ++- .../SessionPrReviewStrip.client.test.tsx | 53 +++++ .../SessionPrReviewStrip.stories.tsx | 101 +++++++++ .../[sessionId]/SessionPrReviewStrip.tsx | 204 ++++++++++++++++++ .../sessions/[sessionId]/SessionWorkspace.tsx | 7 +- .../session-pr-reviews.client.test.ts | 112 ++++++++++ .../[sessionId]/session-pr-reviews.ts | 99 +++++++++ .../[sessionId]/session-task-panel-context.ts | 14 ++ .../ai-elements/pr-review-action-offer.tsx | 8 +- .../lib/fast-agent-parent-event.test.ts | 61 ++++++ .../src/server/lib/fast-agent-parent-event.ts | 43 +++- ...notify-fast-agent-parent-on-pr-feedback.ts | 1 + .../src/__tests__/pr-review-action.test.ts | 27 +++ packages/types/src/pr-review-action.ts | 39 ++++ 16 files changed, 895 insertions(+), 41 deletions(-) create mode 100644 apps/web/src/app/(sandbox)/sessions/[sessionId]/SessionPrReviewStrip.client.test.tsx create mode 100644 apps/web/src/app/(sandbox)/sessions/[sessionId]/SessionPrReviewStrip.stories.tsx create mode 100644 apps/web/src/app/(sandbox)/sessions/[sessionId]/SessionPrReviewStrip.tsx create mode 100644 apps/web/src/app/(sandbox)/sessions/[sessionId]/session-pr-reviews.client.test.ts create mode 100644 apps/web/src/app/(sandbox)/sessions/[sessionId]/session-pr-reviews.ts diff --git a/apps/docs/fast-sessions.mdx b/apps/docs/fast-sessions.mdx index 5ca77f0ad..8428097f1 100644 --- a/apps/docs/fast-sessions.mdx +++ b/apps/docs/fast-sessions.mdx @@ -130,6 +130,20 @@ work. Fast turns also survive API or worker interruptions. Roomote durably admits the turn before acknowledging it, records every action the turn takes as it happens, and resumes unfinished work on another process. The resumed run receives the transcript of its earlier attempt, up to the point it was cut off, so it continues from there instead of repeating actions or asking you to send the request again. If the cut lands on the final reply itself, the resumed run finishes from the transcript: a reply that was recorded is not posted again, and a reply the process died while posting goes out once more without another model request. This covers turns started by a typed message, by an emoji reaction, and by platform events such as a setup kickoff. Provider-side retry waits are honored the same way: short waits keep the turn in place, and longer waits park it durably until the scheduled time. +### Pull Request Reviews + +PR-review updates appear in a compact strip above the Session composer. The strip +stays visible while you scroll and shows the latest update for each pull request, +instead of repeating review notifications in the conversation. Your messages and +the agent's ordinary replies remain in the transcript. + +Use **Fix findings** to address the current feedback, **Auto-resolve** to enable +automatic resolution for that PR, or **Dismiss** to decline the offer. **View +review** opens the linked review task when available, or the pull request on its +source-control provider. An active linked review shows **Reviewing…**; reported +approval, merge, and closure replace the earlier feedback state. Completed review +status can be dismissed from the strip. + ### Message Suggestions An empty task or Session composer can show one contextual follow-up after an diff --git a/apps/web/src/app/(sandbox)/sessions/[sessionId]/FastSessionTranscript.client.test.tsx b/apps/web/src/app/(sandbox)/sessions/[sessionId]/FastSessionTranscript.client.test.tsx index a964886c7..61027b66e 100644 --- a/apps/web/src/app/(sandbox)/sessions/[sessionId]/FastSessionTranscript.client.test.tsx +++ b/apps/web/src/app/(sandbox)/sessions/[sessionId]/FastSessionTranscript.client.test.tsx @@ -4,6 +4,7 @@ import { render, screen, waitFor, + within, } from '@testing-library/react'; import { ACP_ENVELOPE_EVENT_TYPES, @@ -1056,9 +1057,7 @@ describe('FastSessionTranscript', () => { />, ); - fireEvent.click( - screen.getByRole('button', { name: 'Resolve these issues' }), - ); + fireEvent.click(screen.getByRole('button', { name: 'Fix findings' })); await waitFor(() => expect(reviewActionMutate).toHaveBeenCalledWith({ sessionId: '22222222-2222-4222-8222-222222222222', @@ -1066,12 +1065,111 @@ describe('FastSessionTranscript', () => { choice: 'yes', }), ); + expect(await screen.findByText('Addressing findings…')).toBeInTheDocument(); + }); + + it('pins only the latest PR review outside the scrolling transcript and opens its task', async () => { + const old = reviewOfferMessage(); + const prReview = { + url: 'https://github.com/acme/web/pull/42', + repository: 'acme/web', + number: 42, + summary: 'Provider coverage is missing.', + findingCount: 1, + status: 'feedback', + reviewTaskId: 'review-task-42', + }; + const latest = { + ...old, + id: 'offer-2', + eventId: 'turn-offer-2:assistant:0', + ts: 3, + payload: { + prReview, + prReviewAction: { + ...old.payload.prReviewAction, + deliveryId: 'delivery-2', + }, + }, + }; + reviewActionMutate.mockResolvedValue({ status: 'resolved' }); + render( + , + ); + const strip = screen.getByLabelText('Pull request reviews'); + expect(screen.getByRole('log')).not.toContainElement(strip); expect( - await screen.findByText('Resolving the current review issues.'), - ).toBeInTheDocument(); + within(screen.getByRole('log')).queryByText('Review feedback remains.'), + ).not.toBeInTheDocument(); + expect(screen.getAllByText('PR #42')).toHaveLength(1); + expect(screen.getByText('1 unresolved finding')).toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'View review' })); + expect(openTaskPanel).toHaveBeenCalledWith('review-task-42'); + fireEvent.click(screen.getByRole('button', { name: 'Fix finding' })); + await waitFor(() => + expect(reviewActionMutate).toHaveBeenCalledWith({ + sessionId: 'session-1', + deliveryId: 'delivery-2', + choice: 'yes', + }), + ); + + act(() => + FakeEventSource.instances.at(-1)?.emit('messages', { + messages: [ + { + ...latest, + id: 'approval', + eventId: 'approval:assistant:0', + ts: 4, + payload: { + prReview: { ...prReview, status: 'approved', findingCount: 0 }, + }, + }, + ], + }), + ); + expect(await screen.findByText('Approved')).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Fix finding' }), + ).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: 'Dismiss review' })); + expect(screen.queryByText('Approved')).not.toBeInTheDocument(); }); - it('hides dismissed offers and renders late-click states without controls', async () => { + it('keeps a failed review action available for retry', async () => { + reviewActionMutate.mockRejectedValueOnce(new Error('offline')); + render( + , + ); + fireEvent.click(screen.getByRole('button', { name: 'Fix findings' })); + expect(await screen.findByRole('alert')).toHaveTextContent( + 'Could not update this review', + ); + expect(screen.getByRole('button', { name: 'Fix findings' })).toBeEnabled(); + reviewActionMutate.mockResolvedValueOnce({ status: 'resolved' }); + fireEvent.click(screen.getByRole('button', { name: 'Fix findings' })); + expect(await screen.findByText('Addressing findings…')).toBeInTheDocument(); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }); + + it('removes dismissed and superseded offers from the pinned area', async () => { const { rerender } = render( { screen.queryByTestId('pr-review-action-offer'), ).not.toBeInTheDocument(); expect( - screen.queryByRole('button', { name: 'Resolve these issues' }), + screen.queryByRole('button', { name: 'Fix findings' }), ).not.toBeInTheDocument(); rerender( @@ -1099,9 +1197,11 @@ describe('FastSessionTranscript', () => { messages: [reviewOfferMessage('stale')], }); }); - expect( - await screen.findByText('This offer was already handled or has expired.'), - ).toBeInTheDocument(); + await waitFor(() => + expect( + screen.queryByRole('button', { name: 'Fix findings' }), + ).not.toBeInTheDocument(), + ); }); it('renders persisted user and assistant text with task transcript primitives', () => { render( diff --git a/apps/web/src/app/(sandbox)/sessions/[sessionId]/FastSessionTranscript.tsx b/apps/web/src/app/(sandbox)/sessions/[sessionId]/FastSessionTranscript.tsx index ec377d4da..521cad56c 100644 --- a/apps/web/src/app/(sandbox)/sessions/[sessionId]/FastSessionTranscript.tsx +++ b/apps/web/src/app/(sandbox)/sessions/[sessionId]/FastSessionTranscript.tsx @@ -2,6 +2,7 @@ import { useCallback, + useContext, useEffect, useMemo, useReducer, @@ -17,6 +18,7 @@ import { getTextFromContentBlocks, inferAcpMessageKind, parsePrReviewActionOffer, + parseSessionPrReviewUpdate, type AcpMessage, type PrReviewActionChoice, type AcpEventType, @@ -49,11 +51,13 @@ import { useOpenSessionTasksPanel, useSessionRunningTaskCount, useSessionTaskStateRevision, + SessionReviewTasksContext, } from './session-task-panel-context'; import { useNarrationMode } from '@/hooks/useNarrationMode'; import { usePageTitle } from '@/hooks/usePageTitle'; import { truncatePageTitle } from '@/lib/page-title'; -import { PrReviewActionOffer } from '@/components/ai-elements/pr-review-action-offer'; +import { SessionPrReviewStrip } from './SessionPrReviewStrip'; +import { getSessionPrReviews } from './session-pr-reviews'; import { findPendingSessionInputRequest, SessionUserInputCard, @@ -306,6 +310,7 @@ export function FastSessionTranscript({ const openTasksPanel = useOpenSessionTasksPanel(); const runningTaskCount = useSessionRunningTaskCount(); const taskStateRevision = useSessionTaskStateRevision(); + const reviewTasks = useContext(SessionReviewTasksContext); const { enabled: narrationModeEnabled } = useNarrationMode(); const displayMode = narrationModeEnabled ? 'narration' : 'default'; const slackMentionScope = useMemo( @@ -552,7 +557,9 @@ export function FastSessionTranscript({ (message) => message.eventType !== ACP_ENVELOPE_EVENT_TYPES.RequestUserInput && message.eventType !== - ACP_ENVELOPE_EVENT_TYPES.RequestUserInputResponse, + ACP_ENVELOPE_EVENT_TYPES.RequestUserInputResponse && + !parsePrReviewActionOffer(message.payload) && + !parseSessionPrReviewUpdate(message.payload), ) .map((message) => { const uiMessage = toAcpUiMessage({ @@ -607,13 +614,9 @@ export function FastSessionTranscript({ () => findPendingSessionInputRequest(messages), [messages], ); - const reviewOffers = useMemo( - () => - messages.flatMap((message) => { - const offer = parsePrReviewActionOffer(message.payload); - return offer ? [offer] : []; - }), - [messages], + const reviews = useMemo( + () => getSessionPrReviews(messages, reviewTasks), + [messages, reviewTasks], ); const uiMessages = useMemo( () => @@ -785,17 +788,6 @@ export function FastSessionTranscript({ onOpenTasks={openTasksPanel} /> ) : null} - {reviewOffers.map((offer) => ( - - handleReviewAction(offer.deliveryId, choice) - } - /> - ))} {pendingInputRequest ? (
{pendingInputRequest.preset === 'setup_starter_tasks' ? ( @@ -814,6 +806,7 @@ export function FastSessionTranscript({ + {canReply && !pendingInputRequest ? (
{ + render( + , + ); + expect(screen.getAllByRole('button', { name: 'Fix finding' })).toHaveLength( + 1, + ); + expect(screen.getByText('Feedback for 43')).toBeInTheDocument(); + expect(screen.queryByText('Feedback for 42')).not.toBeInTheDocument(); + fireEvent.click( + screen.getByRole('button', { name: 'Review details for PR #42' }), + ); + expect(screen.getByText('Feedback for 42')).toBeInTheDocument(); + expect(screen.queryByText('Feedback for 43')).not.toBeInTheDocument(); + expect( + within( + screen.getByRole('region', { name: 'Review of example/project PR #42' }), + ).getByRole('button', { name: 'Fix finding' }), + ).toBeInTheDocument(); + fireEvent.click( + screen.getByRole('button', { name: 'Review details for PR #42' }), + ); + expect( + screen.queryByRole('button', { name: 'Fix finding' }), + ).not.toBeInTheDocument(); +}); diff --git a/apps/web/src/app/(sandbox)/sessions/[sessionId]/SessionPrReviewStrip.stories.tsx b/apps/web/src/app/(sandbox)/sessions/[sessionId]/SessionPrReviewStrip.stories.tsx new file mode 100644 index 000000000..126b71eef --- /dev/null +++ b/apps/web/src/app/(sandbox)/sessions/[sessionId]/SessionPrReviewStrip.stories.tsx @@ -0,0 +1,101 @@ +import type { Meta, StoryObj } from '@storybook/nextjs-vite'; +import { SessionPrReviewStrip } from './SessionPrReviewStrip'; +import type { SessionReview } from './session-pr-reviews'; + +const pending: SessionReview = { + key: 'pr-42', + revision: 'feedback-1', + review: { + url: 'https://github.com/example/project/pull/42', + repository: 'example/project', + number: 42, + summary: + 'The provider catalog is missing coverage for an additional route.', + findingCount: 1, + status: 'feedback', + }, + offer: { + deliveryId: 'delivery-1', + question: 'Resolve the review feedback?', + status: 'pending', + }, +}; + +const meta: Meta = { + title: 'Sessions/Pinned PR review', + component: SessionPrReviewStrip, + parameters: { layout: 'centered' }, + args: { + reviews: [pending], + onAction: async (_deliveryId, choice) => + choice === 'dismiss' + ? 'dismissed' + : choice === 'auto' + ? 'auto_resolved' + : 'resolved', + }, + decorators: [ + (Story) => ( +
+
+ Update provider support +
+
+

Add the missing model providers and open a PR.

+

The implementation is complete in PR #42. Validation passed.

+

I started a review of the changes.

+ {Array.from({ length: 8 }, (_, index) => ( +

+ Earlier conversation · {index + 1} +

+ ))} +
+ +
+