mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 15:58:03 +08:00
fix(files-tab): default to Diff view when a workspace or repo is attached (#349)
* fix(files-tab): default to Diff view when a workspace or repo is attached Previously, the Files tab only defaulted to Diff view when the user explicitly picked a Git repository on the home page (`selected_repository` on the conversation). Conversations created via the workspace picker — which sets a local working directory but not `selected_repository` — fell through to Files view, even though those conversations also have a pre-existing working tree the user came in to inspect. Treat workspace selection the same as repo selection: - Extend `ConversationMetadata` with an optional `selected_workspace` field and persist it client-side when the home-page workspace picker supplies a `workingDirOverride` (the agent-server runtime has no concept of workspace selection, so this stays client-side, mirroring how repo metadata already works). - Rename `useIsGitRepo` → `useHasAttachedSource` and broaden it to return true when *either* `selected_repository` is set on the active conversation *or* a `selected_workspace` was stored at creation time. - Files tab now keys its diff-view default off `hasAttachedSource` instead of `isGitRepo`. The existing `useHasGitCommits` probe still gates the default off when the attached working tree has no commits yet, so non-git workspaces and unborn-HEAD repos correctly fall back to Files view. The user's persisted per-conversation toggle choice continues to win over the computed default. Co-authored-by: openhands <openhands@all-hands.dev> * chore(files-tab): align comments and test names with attached-source semantics Follow-up cleanup to the diff-default fix. No behaviour change. - `useHasGitCommits` jsdoc + inline comment now describe all three "no diff base" cases (unborn HEAD, non-git workspace, other git error) instead of singling out empty repos, and reference `useHasAttachedSource` as the gating signal. - Files-tab test descriptions / inline comment dropped the "git repo" framing in favour of "attached source", matching the hook the suite is exercising. - Added an AGENTS.md note capturing the design and warning future agents off the filesystem-probe approach that was tried (and reverted) in earlier passes at this logic. - Minor prettier reflow in `use-has-attached-source.ts` picked up by `eslint --fix`. Co-authored-by: openhands <openhands@all-hands.dev> --------- Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
committed by
GitHub
co-authored by
openhands
parent
87af588c3a
commit
2cbfa80f32
@@ -206,3 +206,7 @@
|
||||
- Onboarding modal: `src/components/features/onboarding/onboarding-modal.tsx` is a 4-step welcome flow rendered by `<OnboardingHost />` (mounted on the home route) and gated by the `openhands-onboarded` localStorage flag (`use-onboarding-completion.ts`). The four steps live under `steps/`: choose-agent (Step 0 – OpenHands selectable, Claude Code & Codex disabled with a "coming soon" note), check-backend (embeds the new `BackendForm` extracted from `backend-form-modal.tsx` plus a colored connection banner driven by `useBackendsHealth`), setup-llm (renders `<LlmSettingsScreen onSaveSuccess={onNext} />` so the existing settings UI keeps owning validation), and say-hello (text input pre-filled from `ONBOARDING$HELLO_DEFAULT_MESSAGE`, launches a no-workspace conversation via `useCreateConversation` and closes the modal). Animation: all four panels are mounted as siblings inside a horizontal rail; advancing/retreating just sets `currentStep`, which translates the rail by `-(step * 100)%` for the slide effect. Progress is rendered by `OnboardingProgressBar` with `data-state` per segment (`completed` | `current` | `upcoming`). When extending, refactor `BackendFormModal` carefully — the inner `BackendForm` is the public surface used both by the modal and by `CheckBackendStep`; the modal version still owns dirty/save tracking so it keeps "Save"/"Cancel" footer behavior.
|
||||
|
||||
- Worktree policy (this conversation): commits are made on the worktree branch and the user expects the worktree to stay attached to that branch. Do NOT run `git switch --detach` in the worktree and reattach the branch to the main workspace after each commit — only do that when the user explicitly asks. See `~/.openhands/skills/worktree-switch/SKILL.md` for the manual procedure the user invokes.
|
||||
|
||||
- Files tab diff-view default logic: keyed off `useHasAttachedSource()` (`src/hooks/use-has-attached-source.ts`), which is true when the user explicitly attached *either* a repo (`conversation.selected_repository`) *or* a local workspace (`getStoredConversationMetadata(id).selected_workspace`, persisted by `createConversation` when `workingDirOverride` is supplied). The agent-server pre-initialises every conversation workspace as a git worktree for its own change tracking, so do NOT use a filesystem probe (`git status` / `useUnifiedGetGitChanges`) as the attachment signal — that was tried in earlier iterations and made every fresh no-attachment conversation incorrectly default to diff view. The companion `useHasGitCommits` probe (`src/hooks/query/use-has-git-commits.ts`) then suppresses diff view for attached-but-empty cases (unborn HEAD, non-git workspace).
|
||||
|
||||
- Files tab diff-view default logic: keyed off `useHasAttachedSource()` (`src/hooks/use-has-attached-source.ts`), which is true when the user explicitly attached *either* a repo (`conversation.selected_repository`) *or* a local workspace (`getStoredConversationMetadata(id).selected_workspace`, persisted by `createConversation` when `workingDirOverride` is supplied). The agent-server pre-initialises every conversation workspace as a git worktree for its own change tracking, so do NOT use a filesystem probe (`git status` / `useUnifiedGetGitChanges`) as the attachment signal — that was tried in earlier iterations and made every fresh no-attachment conversation incorrectly default to diff view. The companion `useHasGitCommits` probe (`src/hooks/query/use-has-git-commits.ts`) then suppresses diff view for attached-but-empty cases (unborn HEAD, non-git workspace).
|
||||
|
||||
@@ -106,10 +106,31 @@ describe("useCreateConversation persists selected repository metadata", () => {
|
||||
selected_repository: "octocat/hello-world",
|
||||
selected_branch: "main",
|
||||
git_provider: "github",
|
||||
selected_workspace: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("does not write metadata when no repository is selected", async () => {
|
||||
it("stores the selected workspace path when only a workspace (no repo) is attached", async () => {
|
||||
const { result } = renderHook(() => useCreateConversation(), { wrapper });
|
||||
|
||||
result.current.mutate({
|
||||
query: "poke at this repo",
|
||||
workingDir: "/home/me/code/some-project",
|
||||
});
|
||||
|
||||
await waitFor(() => expect(result.current.isSuccess).toBe(true));
|
||||
|
||||
// We persist the workspace path so `useHasAttachedSource` can default
|
||||
// the Files tab to diff view even when no repo was picked.
|
||||
expect(getStoredConversationMetadata("conv-new")).toEqual({
|
||||
selected_repository: null,
|
||||
selected_branch: null,
|
||||
git_provider: null,
|
||||
selected_workspace: "/home/me/code/some-project",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not write metadata when neither a repository nor a workspace is attached", async () => {
|
||||
const { result } = renderHook(() => useCreateConversation(), { wrapper });
|
||||
|
||||
result.current.mutate({ query: "scratch session" });
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
|
||||
import { renderHook } from "@testing-library/react";
|
||||
|
||||
import { useHasAttachedSource } from "#/hooks/use-has-attached-source";
|
||||
import { setStoredConversationMetadata } from "#/api/conversation-metadata-store";
|
||||
|
||||
const useActiveConversationMock = vi.fn();
|
||||
|
||||
vi.mock("#/hooks/query/use-active-conversation", () => ({
|
||||
useActiveConversation: () => useActiveConversationMock(),
|
||||
}));
|
||||
|
||||
describe("useHasAttachedSource", () => {
|
||||
beforeEach(() => {
|
||||
window.localStorage.clear();
|
||||
useActiveConversationMock.mockReset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
window.localStorage.clear();
|
||||
});
|
||||
|
||||
it("returns true when the conversation has a selected_repository", () => {
|
||||
useActiveConversationMock.mockReturnValue({
|
||||
data: {
|
||||
id: "conv-1",
|
||||
selected_repository: "octocat/hello-world",
|
||||
},
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useHasAttachedSource());
|
||||
|
||||
expect(result.current.hasAttachedSource).toBe(true);
|
||||
expect(result.current.isLoading).toBe(false);
|
||||
});
|
||||
|
||||
it("returns true when the conversation has a stored selected_workspace (no repo)", () => {
|
||||
setStoredConversationMetadata("conv-2", {
|
||||
selected_repository: null,
|
||||
selected_branch: null,
|
||||
git_provider: null,
|
||||
selected_workspace: "/home/me/code/foo",
|
||||
});
|
||||
|
||||
useActiveConversationMock.mockReturnValue({
|
||||
data: { id: "conv-2", selected_repository: null },
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useHasAttachedSource());
|
||||
|
||||
expect(result.current.hasAttachedSource).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false when neither a repo nor a workspace is attached", () => {
|
||||
useActiveConversationMock.mockReturnValue({
|
||||
data: { id: "conv-3", selected_repository: null },
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useHasAttachedSource());
|
||||
|
||||
expect(result.current.hasAttachedSource).toBe(false);
|
||||
});
|
||||
|
||||
it("propagates the active-conversation isLoading flag", () => {
|
||||
useActiveConversationMock.mockReturnValue({
|
||||
data: undefined,
|
||||
isLoading: true,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useHasAttachedSource());
|
||||
|
||||
expect(result.current.hasAttachedSource).toBe(false);
|
||||
expect(result.current.isLoading).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -8,14 +8,14 @@ import { MemoryRouter } from "react-router";
|
||||
import FilesTab from "#/routes/files-tab";
|
||||
|
||||
// Mocks must be declared before the SUT is imported.
|
||||
const useIsGitRepoMock = vi.fn();
|
||||
const useHasAttachedSourceMock = vi.fn();
|
||||
const useHasGitCommitsMock = vi.fn();
|
||||
const useWorkspaceFilesMock = vi.fn();
|
||||
const useWorkspaceFileContentMock = vi.fn();
|
||||
const refetchGitChangesMock = vi.fn();
|
||||
|
||||
vi.mock("#/hooks/use-is-git-repo", () => ({
|
||||
useIsGitRepo: () => useIsGitRepoMock(),
|
||||
vi.mock("#/hooks/use-has-attached-source", () => ({
|
||||
useHasAttachedSource: () => useHasAttachedSourceMock(),
|
||||
}));
|
||||
|
||||
vi.mock("#/hooks/query/use-has-git-commits", () => ({
|
||||
@@ -58,7 +58,7 @@ function renderTab() {
|
||||
|
||||
describe("FilesTab", () => {
|
||||
beforeEach(() => {
|
||||
useIsGitRepoMock.mockReset();
|
||||
useHasAttachedSourceMock.mockReset();
|
||||
useHasGitCommitsMock.mockReset();
|
||||
useWorkspaceFilesMock.mockReset();
|
||||
useWorkspaceFileContentMock.mockReset();
|
||||
@@ -88,8 +88,11 @@ describe("FilesTab", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("defaults to diff view when working inside a git repo", () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: true, isLoading: false });
|
||||
it("defaults to diff view when the user attached a source (repo or workspace)", () => {
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: true,
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
renderTab();
|
||||
|
||||
@@ -100,8 +103,11 @@ describe("FilesTab", () => {
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("defaults to files+rich view in a git repo with zero commits (unborn HEAD)", () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: true, isLoading: false });
|
||||
it("defaults to files+rich view when the attached source has no commits (non-git workspace or unborn HEAD)", () => {
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: true,
|
||||
isLoading: false,
|
||||
});
|
||||
useHasGitCommitsMock.mockReturnValue({
|
||||
hasCommits: false,
|
||||
isLoading: false,
|
||||
@@ -109,7 +115,7 @@ describe("FilesTab", () => {
|
||||
|
||||
renderTab();
|
||||
|
||||
// Even though it's an attached repo, the diff view is suppressed when
|
||||
// Even though something is attached, the diff view is suppressed when
|
||||
// there's nothing to diff against.
|
||||
expect(screen.queryByTestId("changes-tab-content")).not.toBeInTheDocument();
|
||||
expect(
|
||||
@@ -117,8 +123,11 @@ describe("FilesTab", () => {
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("does NOT probe for commits when there is no attached repo", () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
it("does NOT probe for commits when no source is attached", () => {
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
renderTab();
|
||||
|
||||
@@ -128,8 +137,11 @@ describe("FilesTab", () => {
|
||||
expect(useHasGitCommitsMock).toHaveBeenCalledWith({ enabled: false });
|
||||
});
|
||||
|
||||
it("optimistically defaults to diff view while the has-commits probe is still loading", () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: true, isLoading: false });
|
||||
it("optimistically defaults to diff view while the attachment / has-commits probes are still loading", () => {
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: true,
|
||||
isLoading: false,
|
||||
});
|
||||
useHasGitCommitsMock.mockReturnValue({
|
||||
hasCommits: null,
|
||||
isLoading: true,
|
||||
@@ -142,8 +154,11 @@ describe("FilesTab", () => {
|
||||
expect(screen.getByTestId("changes-tab-content")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("defaults to plain file viewer when not in a git repo", () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
it("defaults to plain file viewer when no source is attached", () => {
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
renderTab();
|
||||
|
||||
@@ -155,8 +170,11 @@ describe("FilesTab", () => {
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("lets users toggle diff view off even when in a git repo", async () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: true, isLoading: false });
|
||||
it("lets users toggle diff view off even when a source is attached", async () => {
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: true,
|
||||
isLoading: false,
|
||||
});
|
||||
const user = userEvent.setup();
|
||||
|
||||
renderTab();
|
||||
@@ -178,7 +196,10 @@ describe("FilesTab", () => {
|
||||
});
|
||||
|
||||
it("auto-selects the highest-priority file on first render", () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
renderTab();
|
||||
|
||||
@@ -187,7 +208,10 @@ describe("FilesTab", () => {
|
||||
});
|
||||
|
||||
it("renders the binary fallback in plain mode for binary files", async () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
useWorkspaceFileContentMock.mockReturnValue({
|
||||
data: {
|
||||
path: "logo.png",
|
||||
@@ -214,7 +238,10 @@ describe("FilesTab", () => {
|
||||
});
|
||||
|
||||
it("shows full file paths (not just basenames) as quick-row pills", () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
|
||||
renderTab();
|
||||
|
||||
@@ -224,7 +251,10 @@ describe("FilesTab", () => {
|
||||
});
|
||||
|
||||
it("collapses the file tree by default and expands it via the caret", async () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
const user = userEvent.setup();
|
||||
|
||||
renderTab();
|
||||
@@ -240,7 +270,10 @@ describe("FilesTab", () => {
|
||||
});
|
||||
|
||||
it("renders markdown content via MarkdownRenderer in rich mode", async () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
// Only expose a markdown file so it is auto-selected as the first
|
||||
// priority entry.
|
||||
useWorkspaceFilesMock.mockReturnValue({
|
||||
@@ -287,7 +320,10 @@ describe("FilesTab", () => {
|
||||
});
|
||||
|
||||
it("shows highlighted source (not rich markdown) when toggled to plain on a .md", async () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
useWorkspaceFilesMock.mockReturnValue({
|
||||
data: ["README.md"],
|
||||
isLoading: false,
|
||||
@@ -325,7 +361,10 @@ describe("FilesTab", () => {
|
||||
});
|
||||
|
||||
it("uses the static workspace URL as the iframe src for HTML files", async () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
useWorkspaceFilesMock.mockReturnValue({
|
||||
data: ["index.html"],
|
||||
isLoading: false,
|
||||
@@ -364,7 +403,10 @@ describe("FilesTab", () => {
|
||||
});
|
||||
|
||||
it("switches between rich and plain content modes", async () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
// Only `src/main.ts` is exposed so it auto-selects (otherwise the
|
||||
// priority sort picks `index.html` first and the assertion below
|
||||
// would see the markup grammar instead).
|
||||
@@ -403,7 +445,10 @@ describe("FilesTab", () => {
|
||||
});
|
||||
|
||||
it("shows the refresh button inside the files-tab toolbar and triggers a refetch", async () => {
|
||||
useIsGitRepoMock.mockReturnValue({ isGitRepo: false, isLoading: false });
|
||||
useHasAttachedSourceMock.mockReturnValue({
|
||||
hasAttachedSource: false,
|
||||
isLoading: false,
|
||||
});
|
||||
const user = userEvent.setup();
|
||||
|
||||
renderTab();
|
||||
|
||||
@@ -6,6 +6,15 @@ export interface ConversationMetadata {
|
||||
selected_repository: string | null;
|
||||
selected_branch: string | null;
|
||||
git_provider: Provider | null;
|
||||
/**
|
||||
* The local workspace path the user explicitly attached at conversation
|
||||
* creation time. Distinct from `selected_repository` (which is set by
|
||||
* the repo picker on the home page). Used by the Files tab to decide
|
||||
* whether to default to diff view: if the user attached *anything*
|
||||
* (repo or local workspace), we lean diff-first because there's a real
|
||||
* git baseline to compare against.
|
||||
*/
|
||||
selected_workspace?: string | null;
|
||||
}
|
||||
|
||||
type StoredMetadata = Record<string, ConversationMetadata>;
|
||||
|
||||
@@ -133,11 +133,19 @@ class AgentServerConversationService {
|
||||
);
|
||||
const { data } = response;
|
||||
|
||||
if (metadata?.selected_repository) {
|
||||
// The agent-server runtime has no concept of selected repo/branch, so
|
||||
// persist the home-page selection client-side. toAppConversation
|
||||
// reads the same store when the chat page hydrates the badges.
|
||||
setStoredConversationMetadata(data.id, metadata);
|
||||
if (metadata?.selected_repository || workingDirOverride) {
|
||||
// The agent-server runtime has no concept of selected repo/branch/
|
||||
// workspace, so persist the home-page selection client-side.
|
||||
// `toAppConversation` reads the repo/branch fields back to hydrate
|
||||
// the chat-page badges; `useHasAttachedSource` reads
|
||||
// `selected_workspace` to default the Files tab to Diff mode when
|
||||
// the user explicitly attached a local workspace.
|
||||
setStoredConversationMetadata(data.id, {
|
||||
selected_repository: metadata?.selected_repository ?? null,
|
||||
selected_branch: metadata?.selected_branch ?? null,
|
||||
git_provider: metadata?.git_provider ?? null,
|
||||
selected_workspace: workingDirOverride ?? null,
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -9,9 +9,10 @@ import { useRuntimeIsReady } from "#/hooks/use-runtime-is-ready";
|
||||
* at least one commit reachable from HEAD.
|
||||
*
|
||||
* Used by the Files tab to decide whether the diff view is a sensible
|
||||
* default: an attached repo with zero commits (e.g. a brand-new empty
|
||||
* GitHub repo, or a freshly `git init`-ed workspace) has no diff base to
|
||||
* compare against, so the file viewer is a better landing experience.
|
||||
* default: an attached source (repo or local workspace) with no commits
|
||||
* — e.g. a brand-new empty GitHub repo, a freshly `git init`-ed
|
||||
* workspace, or a plain non-git workspace — has no diff base to compare
|
||||
* against, so the file viewer is a better landing experience.
|
||||
*
|
||||
* Returns `hasCommits: null` while the probe is in-flight so callers can
|
||||
* distinguish "still loading" from a definitive "no commits".
|
||||
@@ -48,10 +49,15 @@ export function useHasGitCommits(options?: { enabled?: boolean }): {
|
||||
sessionApiKey,
|
||||
});
|
||||
|
||||
// `git rev-parse --verify HEAD` exits 0 iff HEAD resolves to a real
|
||||
// commit. On an unborn branch (`git init` with no commits) it exits
|
||||
// non-zero. Equally returns non-zero outside a git repo, but
|
||||
// callers gate this hook on the repo-is-attached signal.
|
||||
// `git rev-parse --verify HEAD` exits 0 iff HEAD resolves to a
|
||||
// real commit. Returns non-zero in three cases that all collapse
|
||||
// to "no diff base, show files view":
|
||||
// - unborn branch (`git init` with no commits)
|
||||
// - not a git repository at all (plain workspace directory)
|
||||
// - other git error
|
||||
// Callers gate this hook on the user having attached a source
|
||||
// (see `useHasAttachedSource`) so we don't shell out for the
|
||||
// unattached-conversation case where the answer is moot.
|
||||
const result = await workspace.executeCommand(
|
||||
"git rev-parse --verify HEAD",
|
||||
workingDir,
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
import { getStoredConversationMetadata } from "#/api/conversation-metadata-store";
|
||||
import { useActiveConversation } from "#/hooks/query/use-active-conversation";
|
||||
|
||||
/**
|
||||
* Returns whether the user explicitly attached a "source" to the active
|
||||
* conversation — i.e. picked a repository on the home page *or* picked a
|
||||
* local workspace folder. From the Files tab's point of view those two
|
||||
* cases are equivalent: both mean there's an existing working tree the
|
||||
* user came in to inspect, so the diff view is the more useful default.
|
||||
*
|
||||
* We deliberately do *not* probe the filesystem here — the agent-server
|
||||
* initialises every conversation workspace as an internal git worktree
|
||||
* for change tracking, so a positive `git status` does not by itself
|
||||
* imply the user attached a real source. That's why we read the
|
||||
* explicit selection signals (`selected_repository` on the conversation,
|
||||
* `selected_workspace` from the conversation-metadata store) instead.
|
||||
*/
|
||||
export function useHasAttachedSource(): {
|
||||
hasAttachedSource: boolean;
|
||||
isLoading: boolean;
|
||||
} {
|
||||
const { data: conversation, isLoading } = useActiveConversation();
|
||||
const storedMetadata = conversation?.id
|
||||
? getStoredConversationMetadata(conversation.id)
|
||||
: null;
|
||||
return {
|
||||
hasAttachedSource:
|
||||
!!conversation?.selected_repository ||
|
||||
!!storedMetadata?.selected_workspace,
|
||||
isLoading,
|
||||
};
|
||||
}
|
||||
@@ -1,20 +0,0 @@
|
||||
import { useActiveConversation } from "#/hooks/query/use-active-conversation";
|
||||
|
||||
/**
|
||||
* Returns whether the active conversation is working in an "existing git
|
||||
* repository" from the user's point of view — that is, one they explicitly
|
||||
* attached via the repo picker. We deliberately do *not* probe the
|
||||
* filesystem (the agent-server initialises every workspace as an internal
|
||||
* git worktree for change tracking, so a positive `git status` does not
|
||||
* mean the user is working on a real repo).
|
||||
*/
|
||||
export function useIsGitRepo(): {
|
||||
isGitRepo: boolean;
|
||||
isLoading: boolean;
|
||||
} {
|
||||
const { data: conversation, isLoading } = useActiveConversation();
|
||||
return {
|
||||
isGitRepo: !!conversation?.selected_repository,
|
||||
isLoading,
|
||||
};
|
||||
}
|
||||
+18
-15
@@ -5,7 +5,7 @@ import { useQueryClient } from "@tanstack/react-query";
|
||||
import { I18nKey } from "#/i18n/declaration";
|
||||
import { useWorkspaceFiles } from "#/hooks/query/use-workspace-files";
|
||||
import { useWorkspaceFileContent } from "#/hooks/query/use-workspace-file-content";
|
||||
import { useIsGitRepo } from "#/hooks/use-is-git-repo";
|
||||
import { useHasAttachedSource } from "#/hooks/use-has-attached-source";
|
||||
import { useHasGitCommits } from "#/hooks/query/use-has-git-commits";
|
||||
import { useAutoRefreshFilesOnEdit } from "#/hooks/use-auto-refresh-files-on-edit";
|
||||
import { useUnifiedGetGitChanges } from "#/hooks/query/use-unified-get-git-changes";
|
||||
@@ -31,21 +31,24 @@ function FilesTab() {
|
||||
// Keep the list / content / diff caches fresh as the agent writes files.
|
||||
useAutoRefreshFilesOnEdit();
|
||||
|
||||
const { isGitRepo, isLoading: isGitRepoLoading } = useIsGitRepo();
|
||||
// A repo with zero commits has no diff base to compare against, so the
|
||||
// diff view would just be empty / misleading. Only probe when we already
|
||||
// believe there's a repo — saves a workspace round trip on every plain
|
||||
// (non-git) conversation.
|
||||
const { hasCommits } = useHasGitCommits({ enabled: isGitRepo });
|
||||
const { hasAttachedSource, isLoading: isAttachedSourceLoading } =
|
||||
useHasAttachedSource();
|
||||
// A workspace with zero commits has no diff base to compare against, so
|
||||
// the diff view would just be empty / misleading. Only probe when we
|
||||
// already believe the user attached a source — saves a workspace round
|
||||
// trip on every plain (no-attachment) conversation.
|
||||
const { hasCommits } = useHasGitCommits({ enabled: hasAttachedSource });
|
||||
|
||||
// Diff view defaults to ON inside an existing git repo *with at least
|
||||
// one commit*, OFF otherwise (no repo, or repo with no commits yet).
|
||||
// Diff view defaults to ON when the user attached a source (repo or
|
||||
// local workspace) *and* there's at least one commit, OFF otherwise
|
||||
// (no attachment, or attachment with no commits yet).
|
||||
//
|
||||
// While the repo / commit probes are still resolving we stay optimistic
|
||||
// — most conversations live in a real repo, so defaulting to diff during
|
||||
// the brief loading window avoids a "files → diff" flash on initial
|
||||
// load. We only flip to files-view once `isGitRepo` *or* `hasCommits`
|
||||
// definitively resolves false. The user's persisted choice always wins.
|
||||
// While the attachment / commit probes are still resolving we stay
|
||||
// optimistic — most attached conversations live in a real repo, so
|
||||
// defaulting to diff during the brief loading window avoids a
|
||||
// "files → diff" flash on initial load. We only flip to files-view
|
||||
// once `hasAttachedSource` *or* `hasCommits` definitively resolves
|
||||
// false. The user's persisted choice always wins.
|
||||
const { conversationId } = useOptionalConversationId();
|
||||
const {
|
||||
state: persistedState,
|
||||
@@ -54,7 +57,7 @@ function FilesTab() {
|
||||
} = useConversationLocalStorageState(conversationId ?? "");
|
||||
|
||||
const diffViewDefault =
|
||||
(isGitRepo || isGitRepoLoading) && hasCommits !== false;
|
||||
(hasAttachedSource || isAttachedSourceLoading) && hasCommits !== false;
|
||||
const diffViewEnabled = persistedState.filesTabDiffView ?? diffViewDefault;
|
||||
const contentViewMode = persistedState.filesTabContentViewMode;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user