feat: disable Git controls when Git is not active (#24673)

closes CODAGT-148

In chats with no Git context (no repositories known to the watcher, no
PR tab, no remote diff), the refresh button fires an "Unable to refresh
git status" toast because the watcher WebSocket never opens.

Derive `isGitActive = repositories.size > 0 || showRemoteTab` in
`GitPanel` and use it to:

- Disable the refresh button, unified-diff toggle, and split-diff toggle
  with a "Git is not set up for this chat" tooltip.
- Show a dedicated empty state explaining how to enable Git, replacing
  the generic "No pushed changes yet" copy.

Chats with at least one repository or a PR tab are unaffected; all
controls remain enabled and behave as before.

Adds a `GitNotActive` Storybook story with play-function assertions
covering the disabled controls and empty-state copy.
This commit is contained in:
Jaayden Halko
2026-05-01 14:46:46 +01:00
committed by GitHub
parent a799356bc3
commit efda5c2c12
7 changed files with 172 additions and 47 deletions
+23 -23
View File
@@ -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")
}
@@ -91,6 +91,7 @@ const buildGitWatcher = (): ComponentProps<
>["gitWatcher"] => ({
repositories: new Map(),
everDirty: new Set(),
hasReceivedChanges: true,
refresh: fn().mockReturnValue(true),
});
@@ -132,6 +132,8 @@ interface AgentChatPageViewProps {
gitWatcher: {
repositories: ReadonlyMap<string, TypesGen.WorkspaceAgentRepoChanges>;
everDirty: ReadonlySet<string>;
hasReceivedChanges: boolean;
refresh: () => boolean;
};
@@ -358,6 +360,10 @@ export const AgentChatPageView: FC<AgentChatPageViewProps> = ({
}
repositories={gitWatcher.repositories}
everDirty={gitWatcher.everDirty}
isGitStatusLoading={
workspaceAgent?.status === "connected" &&
!gitWatcher.hasReceivedChanges
}
onRefresh={handleRefresh}
onCommit={handleCommit}
isExpanded={visualExpanded}
@@ -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
@@ -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<GitPanelProps> = ({
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<GitPanelProps> = ({
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<GitPanelProps> = ({
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<GitPanelProps> = ({
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<GitPanelProps> = ({
<ColumnsIcon className="size-3.5" />
</button>
</div>
<Button
variant="subtle"
size="icon"
onClick={handleRefresh}
aria-label="Refresh"
className="h-6 w-6 text-content-secondary hover:text-content-primary"
>
<RefreshCwIcon
className={cn(
"size-3.5",
spinning && "motion-safe:animate-spin-once",
)}
/>
</Button>
{/*
* 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="h-6 w-6 text-content-secondary hover:text-content-primary"
>
<RefreshCwIcon
className={cn(
"size-3.5",
spinning && "motion-safe:animate-spin-once",
)}
/>
</Button>
</span>
</div>
</div>
{/* Content */}
@@ -300,6 +328,8 @@ export const GitPanel: FC<GitPanelProps> = ({
{view.type === "remote" ? (
<RemoteContent
prTab={prTab}
hasGitContext={hasGitContext}
isGitStatusLoading={isWaitingForGitStatus}
isExpanded={isExpanded}
chatInputRef={chatInputRef}
diffStyle={diffStyle}
@@ -328,22 +358,44 @@ export const GitPanel: FC<GitPanelProps> = ({
const RemoteContent: FC<{
prTab?: { prNumber: number; chatId: string };
hasGitContext: boolean;
isGitStatusLoading: boolean;
isExpanded?: boolean;
chatInputRef?: RefObject<ChatMessageInputRef | null>;
diffStyle: DiffStyle;
diffStatus?: ChatDiffStatus;
}> = ({ prTab, isExpanded, chatInputRef, diffStyle, diffStatus }) => {
}> = ({
prTab,
hasGitContext,
isGitStatusLoading,
isExpanded,
chatInputRef,
diffStyle,
diffStatus,
}) => {
if (!prTab) {
return (
<div className="flex h-full flex-col items-center justify-center p-8 text-center">
<div className="mb-4 flex size-10 items-center justify-center rounded-lg border border-solid border-border-default bg-surface-secondary">
<GitCompareArrowsIcon className="size-5 text-content-secondary" />
{hasGitContext ? (
<GitCompareArrowsIcon className="size-5 text-content-secondary" />
) : (
<GitBranchIcon className="size-5 text-content-secondary" />
)}
</div>
<p className="text-sm font-medium text-content-primary">
No pushed changes yet
{hasGitContext
? "No pushed changes yet"
: isGitStatusLoading
? GIT_STATUS_LOADING_TITLE
: GIT_NOT_SETUP_SENTENCE}
</p>
<p className="mt-1 max-w-52 text-xs text-content-secondary">
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}
</p>
</div>
);
@@ -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);
});
@@ -37,6 +37,8 @@ interface UseGitWatcherResult {
everDirty: ReadonlySet<string>;
/** 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<WebSocket | null>(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 };
}