mirror of
https://github.com/coder/coder.git
synced 2026-10-16 06:37:58 +08:00
fix(site/src/pages/AgentsPage): make git panel refresh match a page reload (#30678)
The git panel's refresh button invalidated the PR diff contents but not the chat, whose `diff_statuses` back the PR tabs, so a page reload could show PR state the button missed. Refresh now also invalidates the chat entity. The button was also disabled until the panel had git context, which left no way to pick up a repo created after the chat touched its path. It is now always enabled; the diff style toggles stay disabled without git context. ### Before (Scenario A) https://github.com/user-attachments/assets/82dc7c48-148f-4661-af73-8400a275b05d ### After (Scenario A) https://github.com/user-attachments/assets/0f24e390-39d3-4b43-9f4c-63d499837e33 ### Before (Scenario B) https://github.com/user-attachments/assets/a779031a-8bb9-466b-9cf3-a56ad02a98d5 ### After (Scenario B) https://github.com/user-attachments/assets/05c081e7-7ad1-4608-8ef7-1bac4940034c <details> <summary>Decision log</summary> The git panel's refresh button should show everything a page reload would. Investigation found five ways it didn't, each fixed in its own PR on the current `changes` protocol: 1. `execute` without a workdir didn't record the directory the process ran in (`agent/agentproc`). 2. Refresh only rescanned repos already being watched, so a repo created or recreated after its path was recorded stayed hidden (`agent/agentgit`). A connection with no repos also got no first message. 3. After a git watch reconnect, the new full state was merged into the previous connection's map, so deleted repos stayed visible (`useGitWatcher`). 4. A chat watch reconnect didn't refetch the open chat, so missed `diff_status_change` events were lost (`AgentsPageLayout`). 5. Refresh didn't refetch the chat, whose diff statuses back the PR tabs, and was disabled until git context existed (`GitPanel`, `AgentChatPageView`). Considered and deferred: a snapshot protocol with diffs fetched by hash, which would keep large multi-repo diffs under the 4 MiB proxy read limit, and making scans faster at scale. Neither is needed to fix refresh, and together they made a single ~5900-line change that couldn't be reviewed. The refresh spinner still runs for a fixed time, since tying it to the response needs a request ID in the protocol. </details> --- _Generated by Coder Agents on behalf of @DanielleMaywood._
This commit is contained in:
@@ -1,9 +1,7 @@
|
||||
import { cn } from "cn";
|
||||
import { ArchiveIcon, TriangleAlertIcon } from "lucide-react";
|
||||
import { useEffect, useState } from "react";
|
||||
import { useQueryClient } from "react-query";
|
||||
import type { UrlTransform } from "streamdown";
|
||||
import { invalidateChatDiffContents } from "#/api/queries/chats";
|
||||
import type * as TypesGen from "#/api/typesGenerated";
|
||||
import type { ChatMessagePart } from "#/api/typesGenerated";
|
||||
import { ErrorAlert } from "#/components/Alert/ErrorAlert";
|
||||
@@ -319,7 +317,6 @@ export const AgentChatPageView: React.FC<AgentChatPageViewProps> = ({
|
||||
onMCPAuthComplete,
|
||||
desktopChatId,
|
||||
}) => {
|
||||
const queryClient = useQueryClient();
|
||||
const { proxy } = useProxy();
|
||||
const { entitlements } = useDashboard();
|
||||
const { permissions, user: currentUser } = useAuthenticated();
|
||||
@@ -332,16 +329,6 @@ export const AgentChatPageView: React.FC<AgentChatPageViewProps> = ({
|
||||
|
||||
const canSubmitChatTurn = !isInputDisabled && !isSubmissionPending;
|
||||
|
||||
// Wrap the git watcher refresh to also invalidate the cached
|
||||
// remote/PR diff contents so the panel re-fetches from GitHub.
|
||||
const handleRefresh = () => {
|
||||
const sent = gitWatcher.refresh();
|
||||
if (sent && agentId) {
|
||||
void invalidateChatDiffContents(queryClient, agentId);
|
||||
}
|
||||
return sent;
|
||||
};
|
||||
|
||||
const [isRightPanelExpanded, setIsRightPanelExpanded] = useState(false);
|
||||
// The turn already running when the page loaded must not anchor the
|
||||
// scroller, even if its prompt is older than the newest messages page and
|
||||
@@ -672,7 +659,7 @@ export const AgentChatPageView: React.FC<AgentChatPageViewProps> = ({
|
||||
workspaceAgent?.status === "connected" &&
|
||||
!gitWatcher.hasReceivedChanges
|
||||
}
|
||||
onRefresh={handleRefresh}
|
||||
onRefresh={gitWatcher.refresh}
|
||||
isExpanded={visualExpanded}
|
||||
remoteDiffStats={chat.diff_statuses}
|
||||
primaryDiffStatus={chat.diff_status}
|
||||
|
||||
@@ -1,15 +1,20 @@
|
||||
import { render, screen, waitFor, within } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { QueryClientProvider } from "react-query";
|
||||
import {
|
||||
type QueryClient,
|
||||
QueryClientProvider,
|
||||
QueryObserver,
|
||||
} from "react-query";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { API } from "#/api/api";
|
||||
import { chat as chatById, chatDiffContents } from "#/api/queries/chats";
|
||||
import type {
|
||||
ChatDiffContents,
|
||||
WorkspaceAgentRepoChanges,
|
||||
} from "#/api/typesGenerated";
|
||||
import { TooltipProvider } from "#/components/Tooltip/Tooltip";
|
||||
import { ThemeOverride } from "#/contexts/ThemeProvider";
|
||||
import { MockChatDiffStatus } from "#/testHelpers/chatEntities";
|
||||
import { MockChat, MockChatDiffStatus } from "#/testHelpers/chatEntities";
|
||||
import { createTestQueryClient } from "#/testHelpers/renderHelpers";
|
||||
import themes, { DEFAULT_THEME } from "#/theme";
|
||||
import type { ChatMessageInputRef } from "../AgentChatInput";
|
||||
@@ -23,8 +28,9 @@ const mockDiffContents: ChatDiffContents = {
|
||||
chat_id: "test-chat",
|
||||
};
|
||||
|
||||
const Wrapper: React.FC<React.PropsWithChildren> = ({ children }) => {
|
||||
const queryClient = createTestQueryClient();
|
||||
const Wrapper: React.FC<
|
||||
React.PropsWithChildren<{ queryClient: QueryClient }>
|
||||
> = ({ children, queryClient }) => {
|
||||
return (
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<ThemeOverride theme={themes[DEFAULT_THEME]}>
|
||||
@@ -35,8 +41,9 @@ const Wrapper: React.FC<React.PropsWithChildren> = ({ children }) => {
|
||||
};
|
||||
|
||||
const renderPanel = (props: Partial<React.ComponentProps<typeof GitPanel>>) => {
|
||||
const queryClient = createTestQueryClient();
|
||||
const view = render(
|
||||
<Wrapper>
|
||||
<Wrapper queryClient={queryClient}>
|
||||
<GitPanel
|
||||
chatId="test-chat"
|
||||
onRefresh={() => true}
|
||||
@@ -53,7 +60,7 @@ const renderPanel = (props: Partial<React.ComponentProps<typeof GitPanel>>) => {
|
||||
next: Partial<React.ComponentProps<typeof GitPanel>>,
|
||||
) => {
|
||||
view.rerender(
|
||||
<Wrapper>
|
||||
<Wrapper queryClient={queryClient}>
|
||||
<GitPanel
|
||||
chatId="test-chat"
|
||||
onRefresh={() => true}
|
||||
@@ -67,7 +74,7 @@ const renderPanel = (props: Partial<React.ComponentProps<typeof GitPanel>>) => {
|
||||
</Wrapper>,
|
||||
);
|
||||
};
|
||||
return { view, rerenderPanel };
|
||||
return { view, rerenderPanel, queryClient };
|
||||
};
|
||||
|
||||
const repoRoot = "/workspace/special-repo";
|
||||
@@ -109,6 +116,46 @@ function renderGitPanel(input: ChatMessageInputRef) {
|
||||
}
|
||||
|
||||
describe("GitPanel", () => {
|
||||
it("allows refreshing before any git context is known", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onRefresh = vi.fn(() => true);
|
||||
renderPanel({ onRefresh });
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Refresh" }));
|
||||
|
||||
expect(onRefresh).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "the git watch socket is connected", sent: true },
|
||||
{ name: "the git watch socket is down", sent: false },
|
||||
])(
|
||||
"refetches the chat and its diff contents on refresh when $name",
|
||||
async ({ sent }) => {
|
||||
const user = userEvent.setup();
|
||||
const getChat = vi
|
||||
.spyOn(API.experimental, "getChat")
|
||||
.mockResolvedValue(MockChat);
|
||||
const getChatDiffContents = vi
|
||||
.spyOn(API.experimental, "getChatDiffContents")
|
||||
.mockResolvedValue(mockDiffContents);
|
||||
const { queryClient } = renderPanel({ onRefresh: () => sent });
|
||||
// Stand in for the chat page and an open diff panel, so the
|
||||
// invalidated queries are refetched.
|
||||
new QueryObserver(queryClient, chatById("test-chat")).subscribe(() => {});
|
||||
new QueryObserver(queryClient, chatDiffContents("test-chat")).subscribe(
|
||||
() => {},
|
||||
);
|
||||
await waitFor(() => expect(getChat).toHaveBeenCalledTimes(1));
|
||||
await waitFor(() => expect(getChatDiffContents).toHaveBeenCalledTimes(1));
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Refresh" }));
|
||||
|
||||
await waitFor(() => expect(getChat).toHaveBeenCalledTimes(2));
|
||||
await waitFor(() => expect(getChatDiffContents).toHaveBeenCalledTimes(2));
|
||||
},
|
||||
);
|
||||
|
||||
it("inserts a commit prompt for the active repo when Commit is clicked", async () => {
|
||||
const user = userEvent.setup();
|
||||
const input = mockComposer();
|
||||
|
||||
@@ -15,6 +15,11 @@ import {
|
||||
RowsIcon,
|
||||
} from "lucide-react";
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { useQueryClient } from "react-query";
|
||||
import {
|
||||
invalidateChatDiffContents,
|
||||
invalidateChatEntity,
|
||||
} from "#/api/queries/chats";
|
||||
import type * as TypesGen from "#/api/typesGenerated";
|
||||
import type {
|
||||
ChatDiffStatus,
|
||||
@@ -129,7 +134,11 @@ type GitPanelProps = {
|
||||
chatId: string;
|
||||
/** Repository data from git watcher. */
|
||||
repositories: ReadonlyMap<string, WorkspaceAgentRepoChanges>;
|
||||
/** Callback to send a refresh to the git watcher. Returns false when disconnected. */
|
||||
/**
|
||||
* Callback to send a refresh to the git watcher. Returns false when
|
||||
* disconnected. The panel refetches the chat and its diff contents
|
||||
* itself.
|
||||
*/
|
||||
onRefresh: () => boolean;
|
||||
/** Whether the panel is in expanded/fullscreen mode. */
|
||||
isExpanded: boolean;
|
||||
@@ -325,7 +334,10 @@ export const GitPanel: React.FC<GitPanelProps> = ({
|
||||
const [spinning, setSpinning] = useState(false);
|
||||
const spinTimerRef = useRef<ReturnType<typeof setTimeout>>(undefined);
|
||||
useEffect(() => () => clearTimeout(spinTimerRef.current), []);
|
||||
const queryClient = useQueryClient();
|
||||
const handleRefresh = () => {
|
||||
void invalidateChatEntity(queryClient, chatId);
|
||||
void invalidateChatDiffContents(queryClient, chatId);
|
||||
const sent = onRefresh();
|
||||
if (!sent) {
|
||||
toast.error("Unable to refresh git status.", {
|
||||
@@ -488,29 +500,23 @@ export const GitPanel: React.FC<GitPanelProps> = ({
|
||||
<ColumnsIcon className="size-3.5" />
|
||||
</button>
|
||||
</div>
|
||||
{/*
|
||||
* The shared Button applies `disabled:pointer-events-none`,
|
||||
* which would suppress the native `title` tooltip when the
|
||||
* control is disabled. Wrap it in a span so the tooltip is
|
||||
* still reachable on hover in the disabled state.
|
||||
*/}
|
||||
<span title={!hasGitContext ? GIT_NOT_SETUP_TITLE : undefined}>
|
||||
<Button
|
||||
variant="subtle"
|
||||
size="icon"
|
||||
onClick={handleRefresh}
|
||||
aria-label="Refresh"
|
||||
disabled={!hasGitContext}
|
||||
className="size-6 text-content-secondary hover:text-content-primary"
|
||||
>
|
||||
<RefreshCwIcon
|
||||
className={cn(
|
||||
"size-3.5",
|
||||
spinning && "motion-safe:animate-spin-once",
|
||||
)}
|
||||
/>
|
||||
</Button>
|
||||
</span>
|
||||
{/* Refresh stays enabled without git context, since a repo
|
||||
created after the chat touched its path only appears
|
||||
once the agent re-resolves the chat's paths. */}
|
||||
<Button
|
||||
variant="subtle"
|
||||
size="icon"
|
||||
onClick={handleRefresh}
|
||||
aria-label="Refresh"
|
||||
className="size-6 text-content-secondary hover:text-content-primary"
|
||||
>
|
||||
<RefreshCwIcon
|
||||
className={cn(
|
||||
"size-3.5",
|
||||
spinning && "motion-safe:animate-spin-once",
|
||||
)}
|
||||
/>
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
{/* PR title row: truncates with a hover tooltip only when the
|
||||
|
||||
Reference in New Issue
Block a user