diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index f7ff9c3780..621b1af9bd 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -321,7 +321,7 @@ func TestSubagentChatExcludesWorkspaceProvisioningTools(t *testing.T) { // Identify root and subagent calls. Root chat calls include // spawn_agent; the subagent call does not. Because the root chat // makes multiple LLM calls (before and after spawn_agent), we - // find exactly one call that lacks spawn_agent — that's the + // find exactly one call that lacks spawn_agent. That's the // subagent. var rootCalls, childCalls [][]string for _, tools := range recorded { @@ -3689,7 +3689,7 @@ func TestNewReplicaRecoversStaleChatFromDeadReplica(t *testing.T) { }) require.NoError(t, err) - // Start a new replica — it should recover the stale chat on + // Start a new replica. It should recover the stale chat on // startup. newReplica := newTestServer(t, db, ps, uuid.New()) _ = newReplica @@ -3712,7 +3712,7 @@ func TestWaitingChatsAreNotRecoveredAsStale(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) user, org, model := seedChatDependencies(t, db) - // Create a chat in waiting status — this should NOT be touched + // Create a chat in waiting status. This should NOT be touched // by stale recovery. chat := dbgen.Chat(t, db, database.Chat{ OrganizationID: org.ID, @@ -4021,7 +4021,7 @@ func TestDynamicToolCallPausesAndResumes(t *testing.T) { streamedCalls := make([]chattest.OpenAIRequest, 0, 2) openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { - // Non-streaming requests are title generation — return a + // Non-streaming requests are title generation. Return a // simple title. if !req.Stream { return chattest.OpenAINonStreamingResponse("Dynamic tool test") @@ -4055,7 +4055,7 @@ func TestDynamicToolCallPausesAndResumes(t *testing.T) { // Dynamic tools do not need a workspace connection, but the // chatd server always builds workspace tools. Use an active - // server without an agent connection — the built-in tools + // server without an agent connection, so the built-in tools // are never invoked because the only tool call targets our // dynamic tool. server := newActiveTestServer(t, db, ps) @@ -4309,7 +4309,7 @@ func TestDynamicToolCallMixedWithBuiltIn(t *testing.T) { if streamedCallCount.Add(1) == 1 { // First call: return TWO tool calls in one - // response — a built-in tool (read_file) and a + // response: a built-in tool (read_file) and a // dynamic tool (my_dynamic_tool). builtinChunk := chattest.OpenAIToolCallChunk( "read_file", @@ -4648,7 +4648,7 @@ func TestSubscribeNoPubsubNoDuplicateMessageParts(t *testing.T) { require.NotEmpty(t, snapshot) // The events channel should NOT immediately produce any - // events — the snapshot already contained everything. Before + // events. The snapshot already contained everything. Before // the fix, localSnapshot was replayed into the channel, // causing duplicates. require.Never(t, func() bool { @@ -4671,7 +4671,7 @@ func TestSubscribeAfterMessageID(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) user, org, model := seedChatDependencies(t, db) - // Create a chat — this inserts one initial "user" message. + // Create a chat. This inserts one initial "user" message. chat, err := replica.CreateChat(ctx, chatd.CreateOptions{ OrganizationID: org.ID, OwnerID: user.ID, @@ -4938,7 +4938,7 @@ func TestStartWorkspaceTool_EndToEnd(t *testing.T) { // Create a workspace, then stop it so start_workspace has // something to start. We intentionally skip starting a test - // agent — the echo provisioner creates new agent rows for each + // agent. The echo provisioner creates new agent rows for each // build, so an agent started for build 1 cannot serve build 3. // The tool handles the no-agent case gracefully. workspace := coderdtest.CreateWorkspace(t, client, template.ID) @@ -5299,7 +5299,7 @@ func TestHeartbeatBumpsWorkspaceUsage(t *testing.T) { Time: dbtime.Now().Add(-30 * time.Minute), }, }) - // Build deadline is 30 minutes in the past — close enough to + // Build deadline is 30 minutes in the past, close enough to // be bumped by the default 1-hour activity bump. build := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ WorkspaceID: ws.ID, @@ -5406,7 +5406,7 @@ func TestHeartbeatBumpsWorkspaceUsage(t *testing.T) { "workspace last_used_at should have been bumped") // Verify the workspace build deadline was also extended. - // The SQL only writes when 5% of the deadline has elapsed — + // The SQL only writes when 5% of the deadline has elapsed, // most calls perform a read-only CTE lookup. Wider ±2 // minute tolerance than activitybump_test.go because the bump // happens asynchronously via the heartbeat goroutine. @@ -5518,7 +5518,7 @@ func waitForChatProcessed( if err != nil { return false } - // Wait until the chat reaches a terminal state — neither + // Wait until the chat reaches a terminal state. Neither // pending (waiting to be acquired) nor running (being // processed). This guarantees that inflight.Add(1) has // already been called by processOnce. @@ -6832,7 +6832,7 @@ func TestInterruptChatPersistsPartialResponse(t *testing.T) { require.NoError(t, err) // Subscribe to the chat's event stream so we can observe - // message_part events — proof the chatloop has actually + // message_part events. This proves the chatloop has actually // processed the streamed chunks. _, events, subCancel, ok := server.Subscribe(ctx, chat.ID, nil, 0) require.True(t, ok) @@ -6866,7 +6866,7 @@ func TestInterruptChatPersistsPartialResponse(t *testing.T) { }, testutil.IntervalFast) require.True(t, gotMessagePart, "should have received at least one message_part event") - // Now interrupt the chat — the chatloop has processed content. + // Now interrupt the chat. The chatloop has processed content. updated := server.InterruptChat(ctx, chat) require.Equal(t, database.ChatStatusWaiting, updated.Status) @@ -7808,7 +7808,7 @@ func TestMCPServerOAuth2TokenRefreshFailureGraceful(t *testing.T) { })) t.Cleanup(tokenSrv.Close) - // The LLM just replies with text — no tool calls. + // The LLM just replies with text, no tool calls. var callCount atomic.Int32 openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { if !req.Stream { @@ -7898,9 +7898,9 @@ func TestChatTemplateAllowlistEnforcement(t *testing.T) { // Set up a mock OpenAI server that chains tool calls: // 1. list_templates - // 2. read_template (blocked template — should fail) - // 3. read_template (allowed template — should succeed) - // 4. create_workspace (blocked template — should fail) + // 2. read_template (blocked template, should fail) + // 3. read_template (allowed template, should succeed) + // 4. create_workspace (blocked template, should fail) // 5. text response var callCount atomic.Int32 openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse { @@ -8048,7 +8048,7 @@ func TestChatTemplateAllowlistEnforcement(t *testing.T) { // TestSignalWakeImmediateAcquisition verifies that CreateChat triggers // immediate processing via signalWake without waiting for the polling // ticker to fire. The ticker interval is set to an hour so it never -// fires during the test — any processing must come from the wake +// fires during the test. Any processing must come from the wake // channel. func TestSignalWakeImmediateAcquisition(t *testing.T) { t.Parallel() @@ -8061,7 +8061,7 @@ func TestSignalWakeImmediateAcquisition(t *testing.T) { if !req.Stream { return chattest.OpenAINonStreamingResponse("title") } - // Signal that the LLM was reached — this proves the chat + // Signal that the LLM was reached. This proves the chat // was acquired and processing started. select { case <-processed: @@ -8092,7 +8092,7 @@ func TestSignalWakeImmediateAcquisition(t *testing.T) { }) require.NoError(t, err) - // The chat should be processed immediately — the LLM handler + // The chat should be processed immediately. The LLM handler // closes the `processed` channel when it receives a streaming // request. Without signalWake this would hang forever because // the 1-hour ticker never fires. @@ -8161,7 +8161,7 @@ func TestSignalWakeSendMessage(t *testing.T) { testutil.TryReceive(ctx, t, firstProcessed) chatd.WaitUntilIdleForTest(server) - // Now send a follow-up message — this should also be + // Now send a follow-up message, which should also be // processed immediately via signalWake. _, err = server.SendMessage(ctx, chatd.SendMessageOptions{ ChatID: chat.ID, @@ -8172,7 +8172,7 @@ func TestSignalWakeSendMessage(t *testing.T) { testutil.TryReceive(ctx, t, secondProcessed) chatd.WaitUntilIdleForTest(server) - // Both turns processed — verify second request reached the LLM. + // Both turns processed. Verify the second request reached the LLM. require.GreaterOrEqual(t, requestCount.Load(), int32(2), "LLM should have received at least 2 streaming requests") } diff --git a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx index ba1dd22607..8fcbe2944c 100644 --- a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx @@ -91,6 +91,7 @@ const buildGitWatcher = (): ComponentProps< >["gitWatcher"] => ({ repositories: new Map(), everDirty: new Set(), + hasReceivedChanges: true, refresh: fn().mockReturnValue(true), }); diff --git a/site/src/pages/AgentsPage/AgentChatPageView.tsx b/site/src/pages/AgentsPage/AgentChatPageView.tsx index e4e54c8b21..66eea72f8f 100644 --- a/site/src/pages/AgentsPage/AgentChatPageView.tsx +++ b/site/src/pages/AgentsPage/AgentChatPageView.tsx @@ -132,6 +132,8 @@ interface AgentChatPageViewProps { gitWatcher: { repositories: ReadonlyMap; everDirty: ReadonlySet; + hasReceivedChanges: boolean; + refresh: () => boolean; }; @@ -358,6 +360,10 @@ export const AgentChatPageView: FC = ({ } repositories={gitWatcher.repositories} everDirty={gitWatcher.everDirty} + isGitStatusLoading={ + workspaceAgent?.status === "connected" && + !gitWatcher.hasReceivedChanges + } onRefresh={handleRefresh} onCommit={handleCommit} isExpanded={visualExpanded} diff --git a/site/src/pages/AgentsPage/components/GitPanel/GitPanel.stories.tsx b/site/src/pages/AgentsPage/components/GitPanel/GitPanel.stories.tsx index cc775f23f0..5ddf44ef04 100644 --- a/site/src/pages/AgentsPage/components/GitPanel/GitPanel.stories.tsx +++ b/site/src/pages/AgentsPage/components/GitPanel/GitPanel.stories.tsx @@ -1,5 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; -import { expect, fn, spyOn, userEvent } from "storybook/test"; +import { expect, fn, spyOn, userEvent, within } from "storybook/test"; import { API } from "#/api/api"; import type { ChatDiffContents, @@ -227,6 +227,10 @@ export const WorkingChangesOnly: Story = { args: { repositories: new Map([["/home/coder/coder", makeRepo()]]), }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByLabelText("Refresh")).toBeEnabled(); + }, }; /** Multiple repos with working changes. */ @@ -268,6 +272,35 @@ export const EmptyState: Story = { }, }; +/** No repositories and no remote tab; Git controls should be disabled. */ +export const GitNotActive: Story = { + args: { + repositories: new Map(), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByLabelText("Refresh")).toBeDisabled(); + await expect(canvas.getByLabelText("Unified diff")).toBeDisabled(); + await expect(canvas.getByLabelText("Split diff")).toBeDisabled(); + await expect( + canvas.getByText("Git is not set up for this chat."), + ).toBeVisible(); + await expect(canvas.getByText(/Git status will appear/)).toBeVisible(); + }, +}; + +/** Git watcher is loading its first repository update. */ +export const GitStatusLoading: Story = { + args: { + repositories: new Map(), + isGitStatusLoading: true, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + await expect(canvas.getByText("Waiting for Git status")).toBeVisible(); + }, +}; + /** * PR diff with the inline comment input visible. The play function * waits for the diff to render, then clicks a line number gutter diff --git a/site/src/pages/AgentsPage/components/GitPanel/GitPanel.tsx b/site/src/pages/AgentsPage/components/GitPanel/GitPanel.tsx index 16413a2043..0c6e2fdf04 100644 --- a/site/src/pages/AgentsPage/components/GitPanel/GitPanel.tsx +++ b/site/src/pages/AgentsPage/components/GitPanel/GitPanel.tsx @@ -32,6 +32,13 @@ import { RemoteDiffPanel } from "../DiffViewer/RemoteDiffPanel"; type GitView = { type: "remote" } | { type: "local"; repoRoot: string }; +const GIT_NOT_SETUP_TITLE = "Git is not set up for this chat"; +const GIT_NOT_SETUP_SENTENCE = "Git is not set up for this chat."; +const GIT_NOT_SETUP_BODY = + "Git status will appear here once a Git repository is detected in the workspace."; +const GIT_STATUS_LOADING_TITLE = "Waiting for Git status"; +const GIT_STATUS_LOADING_BODY = "Checking the workspace for Git repositories."; + interface DiffStats { additions: number; deletions: number; @@ -51,6 +58,8 @@ interface GitPanelProps { onCommit: (repoRoot: string) => void; /** Whether the panel is in expanded/fullscreen mode. */ isExpanded?: boolean; + /** Whether the watcher is loading its initial repository state. */ + isGitStatusLoading?: boolean; /** Diff status for the remote/branch view (includes PR metadata). */ remoteDiffStats?: ChatDiffStatus; /** Ref to the chat input, forwarded to RemoteDiffPanel. */ @@ -75,15 +84,19 @@ export const GitPanel: FC = ({ onRefresh, onCommit, isExpanded, + isGitStatusLoading = false, remoteDiffStats, chatInputRef, everDirty, }) => { - const hasRemoteStats = + const hasRemoteDiff = + (remoteDiffStats?.changed_files ?? 0) > 0 || (remoteDiffStats?.additions ?? 0) > 0 || (remoteDiffStats?.deletions ?? 0) > 0; - const showRemoteTab = Boolean(prTab) || hasRemoteStats; + const showRemoteTab = Boolean(prTab) || hasRemoteDiff; + const hasGitContext = repositories.size > 0 || showRemoteTab; + const isWaitingForGitStatus = !hasGitContext && isGitStatusLoading; const prTitle = remoteDiffStats?.pull_request_title; const prState = remoteDiffStats?.pull_request_state; @@ -147,6 +160,8 @@ export const GitPanel: FC = ({ setView({ type: "remote" }); } else if (localRepos.length > 0) { setView({ type: "local", repoRoot: localRepos[0] }); + } else { + setView({ type: "remote" }); } } } @@ -256,8 +271,10 @@ export const GitPanel: FC = ({ type="button" onClick={() => handleDiffStyleChange("unified")} aria-label="Unified diff" + disabled={!hasGitContext} + title={!hasGitContext ? GIT_NOT_SETUP_TITLE : undefined} className={cn( - "flex cursor-pointer items-center border-none px-1.5 transition-colors", + "flex cursor-pointer items-center border-none px-1.5 transition-colors disabled:cursor-default disabled:opacity-50", diffStyle === "unified" ? "bg-surface-quaternary/25 text-content-primary" : "bg-surface-primary text-content-secondary hover:bg-surface-tertiary/50 hover:text-content-primary", @@ -269,8 +286,10 @@ export const GitPanel: FC = ({ type="button" onClick={() => handleDiffStyleChange("split")} aria-label="Split diff" + disabled={!hasGitContext} + title={!hasGitContext ? GIT_NOT_SETUP_TITLE : undefined} className={cn( - "flex cursor-pointer items-center border-0 border-l border-solid border-border-default px-1.5 transition-colors", + "flex cursor-pointer items-center border-0 border-l border-solid border-border-default px-1.5 transition-colors disabled:cursor-default disabled:opacity-50", diffStyle === "split" ? "bg-surface-quaternary/25 text-content-primary" : "bg-surface-primary text-content-secondary hover:bg-surface-tertiary/50 hover:text-content-primary", @@ -279,20 +298,29 @@ export const GitPanel: FC = ({ - + {/* + * 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. + */} + + + {/* Content */} @@ -300,6 +328,8 @@ export const GitPanel: FC = ({ {view.type === "remote" ? ( = ({ const RemoteContent: FC<{ prTab?: { prNumber: number; chatId: string }; + hasGitContext: boolean; + isGitStatusLoading: boolean; isExpanded?: boolean; chatInputRef?: RefObject; diffStyle: DiffStyle; diffStatus?: ChatDiffStatus; -}> = ({ prTab, isExpanded, chatInputRef, diffStyle, diffStatus }) => { +}> = ({ + prTab, + hasGitContext, + isGitStatusLoading, + isExpanded, + chatInputRef, + diffStyle, + diffStatus, +}) => { if (!prTab) { return (
- + {hasGitContext ? ( + + ) : ( + + )}

- No pushed changes yet + {hasGitContext + ? "No pushed changes yet" + : isGitStatusLoading + ? GIT_STATUS_LOADING_TITLE + : GIT_NOT_SETUP_SENTENCE}

- Once commits are pushed, the branch diff will appear here. + {hasGitContext + ? "Once commits are pushed, the branch diff will appear here." + : isGitStatusLoading + ? GIT_STATUS_LOADING_BODY + : GIT_NOT_SETUP_BODY}

); diff --git a/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts b/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts index 1435b9a4b4..de1c521037 100644 --- a/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts +++ b/site/src/pages/AgentsPage/hooks/useGitWatcher.test.ts @@ -80,9 +80,11 @@ describe("useGitWatcher", () => { expect(mockWatchChatGit).toHaveBeenCalledWith("chat-123"); expect(result.current.isConnected).toBe(false); + expect(result.current.hasReceivedChanges).toBe(false); act(() => socket.simulateOpen()); expect(result.current.isConnected).toBe(true); + expect(result.current.hasReceivedChanges).toBe(false); }); it("does not connect when chatId is undefined", () => { @@ -92,6 +94,7 @@ describe("useGitWatcher", () => { expect(mockWatchChatGit).not.toHaveBeenCalled(); expect(result.current.isConnected).toBe(false); + expect(result.current.hasReceivedChanges).toBe(false); expect(result.current.repositories.size).toBe(0); }); @@ -104,6 +107,7 @@ describe("useGitWatcher", () => { expect(mockWatchChatGit).not.toHaveBeenCalled(); expect(result.current.isConnected).toBe(false); + expect(result.current.hasReceivedChanges).toBe(false); expect(result.current.repositories.size).toBe(0); }); @@ -116,6 +120,7 @@ describe("useGitWatcher", () => { expect(mockWatchChatGit).not.toHaveBeenCalled(); expect(result.current.isConnected).toBe(false); + expect(result.current.hasReceivedChanges).toBe(false); expect(result.current.repositories.size).toBe(0); }); @@ -211,6 +216,7 @@ describe("useGitWatcher", () => { await waitFor(() => { expect(result.current.repositories.size).toBe(2); }); + expect(result.current.hasReceivedChanges).toBe(true); const repoA = result.current.repositories.get("/home/user/project-a"); expect(repoA).toEqual({ @@ -227,6 +233,24 @@ describe("useGitWatcher", () => { }); }); + it("marks empty changes messages as received", async () => { + const socket = createMockSocket(); + + const { result } = renderHook(() => + useGitWatcher({ chatId: "chat-123", agentStatus: "connected" }), + ); + + act(() => socket.simulateOpen()); + act(() => { + socket.simulateMessage({ type: "changes", repositories: [] }); + }); + + await waitFor(() => { + expect(result.current.hasReceivedChanges).toBe(true); + }); + expect(result.current.repositories.size).toBe(0); + }); + it("evicts repos with removed: true", async () => { const socket = createMockSocket(); @@ -443,6 +467,7 @@ describe("useGitWatcher", () => { await waitFor(() => { expect(result.current.repositories.size).toBe(1); }); + expect(result.current.hasReceivedChanges).toBe(true); // The old socket should be closed when we switch chatId. const socket2 = createMockSocket(); @@ -453,6 +478,7 @@ describe("useGitWatcher", () => { // Repositories should be reset immediately after chatId changes. expect(result.current.repositories.size).toBe(0); + expect(result.current.hasReceivedChanges).toBe(false); // The new socket should work independently. act(() => socket2.simulateOpen()); @@ -472,6 +498,7 @@ describe("useGitWatcher", () => { await waitFor(() => { expect(result.current.repositories.size).toBe(1); }); + expect(result.current.hasReceivedChanges).toBe(true); expect(result.current.repositories.has("/home/user/project-x")).toBe(true); }); diff --git a/site/src/pages/AgentsPage/hooks/useGitWatcher.ts b/site/src/pages/AgentsPage/hooks/useGitWatcher.ts index 68c64c2998..2002db7ead 100644 --- a/site/src/pages/AgentsPage/hooks/useGitWatcher.ts +++ b/site/src/pages/AgentsPage/hooks/useGitWatcher.ts @@ -37,6 +37,8 @@ interface UseGitWatcherResult { everDirty: ReadonlySet; /** Whether the WebSocket is currently connected. */ isConnected: boolean; + /** Whether the watcher has received repository state for this chat. */ + hasReceivedChanges: boolean; /** Send a refresh request. Returns true if sent, false if disconnected. */ refresh: () => boolean; } @@ -52,6 +54,7 @@ export function useGitWatcher({ () => new Set(), ); const [isConnected, setIsConnected] = useState(false); + const [hasReceivedChanges, setHasReceivedChanges] = useState(false); const socketRef = useRef(null); // Chat-scoped state (everDirty) resets on chatId change but @@ -104,6 +107,7 @@ export function useGitWatcher({ } if (data.type === "changes") { + setHasReceivedChanges(true); if (data.repositories) { setRepositories((prev) => { let changed = false; @@ -159,6 +163,7 @@ export function useGitWatcher({ onDisconnect() { setIsConnected(false); + setHasReceivedChanges(false); socketRef.current = null; }, @@ -172,10 +177,11 @@ export function useGitWatcher({ // chat-scoped and persists across reconnects. dispose(); setIsConnected(false); + setHasReceivedChanges(false); setRepositories(new Map()); socketRef.current = null; }; }, [chatId, agentStatus]); - return { repositories, everDirty, isConnected, refresh }; + return { repositories, everDirty, isConnected, hasReceivedChanges, refresh }; }