Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 169 additions & 0 deletions src/contexts/workspace-context.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ beforeEach(() => {

vi.mock("@/lib/api", () => ({
getHomeDirectory: vi.fn(),
listDirectoryWithFiles: vi.fn(),
readFileForEdit: vi.fn(),
readFileBase64: vi.fn(),
readFilePreview: vi.fn(),
Expand Down Expand Up @@ -260,6 +261,7 @@ function WorkspaceProbe() {
activeFileTabId,
filesMaximized,
openSessionFileDiff,
openFilePreview,
closeFileTab,
closeAllFileTabs,
toggleFilesMaximized,
Expand All @@ -268,6 +270,9 @@ function WorkspaceProbe() {

return (
<div>
<button type="button" onClick={() => void openFilePreview("report.docx")}>
Open office by hand
</button>
<output data-testid="mode">{mode}</output>
<output data-testid="file-tab-count">{fileTabs.length}</output>
<output data-testid="active-pane">{activePane}</output>
Expand Down Expand Up @@ -2009,6 +2014,19 @@ describe("WorkspaceProvider office auto-preview", () => {
workspaceStoreMock.reset()
// Preference defaults ON; drop any "false" a prior test left behind.
localStorage.removeItem("workspace:office-auto-preview")
// Reset first: call history is what the "one listing per parent" tests
// assert on, and an unconsumed `…Once` from a prior test would otherwise
// answer the next one's first lookup.
vi.mocked(api.listDirectoryWithFiles).mockReset()
vi.mocked(api.listDirectoryWithFiles).mockImplementation(async (root) =>
["report.pptx", "deck.pptx", "report.docx"].map((name) => ({
name,
path: `${root}/${name}`,
isDir: false,
hasChildren: false,
size: 100,
}))
)
})

it("auto-opens an office file's preview when the watcher reports it, with no aux panel involved", async () => {
Expand Down Expand Up @@ -2126,6 +2144,157 @@ describe("WorkspaceProvider office auto-preview", () => {
expect(screen.getByTestId("file-tab-count")).toHaveTextContent("1")
})

it("does not open a removed document from a changed_paths envelope", async () => {
vi.mocked(api.listDirectoryWithFiles).mockResolvedValue([])
renderWorkspace()

await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx"])
})

expect(screen.getByTestId("file-tab-count")).toHaveTextContent("0")
expect(screen.getByTestId("active-pane")).toHaveTextContent("conversation")
})

it("does not open documents from a removed parent directory", async () => {
vi.mocked(api.listDirectoryWithFiles).mockRejectedValue(
new Error("Path is not a directory")
)
renderWorkspace()

await act(async () => {
workspaceStoreMock.emitEnvelope(["scratch/report.docx"])
})

expect(screen.getByTestId("file-tab-count")).toHaveTextContent("0")
})

it("keeps a dismissed preview closed after switching folders and back", async () => {
renderWorkspace()
await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx"])
})
act(() => screen.getByRole("button", { name: "Close active" }).click())
act(() => foldersMock.setActiveFolderId(2))
act(() => foldersMock.setActiveFolderId(1))
await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx"])
})

expect(screen.getByTestId("file-tab-count")).toHaveTextContent("0")
})

it("can open a document recreated after an earlier removal event", async () => {
vi.mocked(api.listDirectoryWithFiles).mockResolvedValueOnce([])
renderWorkspace()
await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx"])
})
expect(screen.getByTestId("file-tab-count")).toHaveTextContent("0")

await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx"])
})
expect(screen.getByTestId("file-tab-count")).toHaveTextContent("1")
})

it("does not mistake a directory with an Office suffix for a document", async () => {
vi.mocked(api.listDirectoryWithFiles).mockResolvedValue([
{
name: "report.docx",
path: "/repo/report.docx",
isDir: true,
hasChildren: true,
size: null,
},
])
renderWorkspace()
await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx"])
})
expect(screen.getByTestId("file-tab-count")).toHaveTextContent("0")
})

it("discards a pending preview when its workspace is no longer active", async () => {
let finish!: (
entries: Awaited<ReturnType<typeof api.listDirectoryWithFiles>>
) => void
vi.mocked(api.listDirectoryWithFiles).mockReturnValueOnce(
new Promise((resolve) => {
finish = resolve
})
)
renderWorkspace()
await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx"])
})
act(() => foldersMock.setActiveFolderId(2))
await act(async () => {
finish([
{
name: "report.docx",
path: "/repo/report.docx",
isDir: false,
hasChildren: false,
size: 100,
},
])
})
expect(screen.getByTestId("file-tab-count")).toHaveTextContent("0")
})

it("lists a parent once per burst and never re-lists a decided path", async () => {
// The existence probe sits on the watcher's hot path, and the backing
// command stats every sibling and ships the whole listing to the renderer
// (over HTTP in server/remote mode). Both bounds below are what keep that
// affordable: one lookup per parent per envelope, and none at all once a
// path has been decided.
renderWorkspace()

await act(async () => {
workspaceStoreMock.emitEnvelope([
"report.docx",
"deck.pptx",
"report.pptx",
])
})

expect(screen.getByTestId("file-tab-count")).toHaveTextContent("3")
expect(api.listDirectoryWithFiles).toHaveBeenCalledTimes(1)
expect(api.listDirectoryWithFiles).toHaveBeenCalledWith("/repo")

await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx", "deck.pptx"])
})

expect(api.listDirectoryWithFiles).toHaveBeenCalledTimes(1)
})

it("does not resurface a document the user opened by hand and closed", async () => {
renderWorkspace()

await act(async () => {
screen.getByRole("button", { name: "Open office by hand" }).click()
})
expect(screen.getByTestId("file-tab-count")).toHaveTextContent("1")

// The agent writes while the hand-opened tab is up: nothing to open, and
// no existence probe either — the open tab already answers the question.
await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx"])
})
expect(api.listDirectoryWithFiles).not.toHaveBeenCalled()

act(() => screen.getByRole("button", { name: "Close active" }).click())
expect(screen.getByTestId("file-tab-count")).toHaveTextContent("0")

// A document the user has already seen and dismissed stays dismissed.
await act(async () => {
workspaceStoreMock.emitEnvelope(["report.docx"])
})
expect(screen.getByTestId("file-tab-count")).toHaveTextContent("0")
})

it("does not auto-open when the preference is disabled", async () => {
localStorage.setItem("workspace:office-auto-preview", "false")
renderWorkspace()
Expand Down
61 changes: 56 additions & 5 deletions src/contexts/workspace-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
gitIsTracked,
gitShowDiff,
gitShowFile,
listDirectoryWithFiles,
readFileBase64,
readFileForEdit,
readFilePreview,
Expand Down Expand Up @@ -544,6 +545,9 @@ export function WorkspaceProvider({ children }: WorkspaceProviderProps) {
new Map()
)
const fileTabsRef = useRef<FileWorkspaceTab[]>([])
// Keep dismissals across folder changes and effect re-subscriptions. A
// watcher event must not reopen a preview the user already closed.
const autoOpenedOfficePathsRef = useRef(new Set<string>())
// Latest-state mirrors for the stable action callbacks. Actions live in a
// context value that must NOT change identity when tabs/folder change, so
// they read these refs instead of capturing render-scoped state. The refs
Expand Down Expand Up @@ -1849,10 +1853,16 @@ export function WorkspaceProvider({ children }: WorkspaceProviderProps) {
// Leading-edge with dedup: an agent building a doc fires a burst of writes,
// so we open on first sighting and remember it in `autoOpened` (which also
// keeps a tab the user has since closed from popping back open).
const autoOpened = new Set<string>()
const autoOpened = autoOpenedOfficePathsRef.current
const pending = new Set<string>()
let cancelled = false
const streamRoot = folderPath
const unsubscribe = subscribeOfficeEnvelopes(({ changed_paths }) => {
if (!changed_paths || changed_paths.length === 0) return
const directories = new Map<
string,
ReturnType<typeof listDirectoryWithFiles>
>()
// Tab identity is the absolute path, so joining the stream root onto
// the changed relative path compares exactly — an identically-named
// doc in another folder has a different absolute path and never
Expand All @@ -1877,12 +1887,53 @@ export function WorkspaceProvider({ children }: WorkspaceProviderProps) {
// at once — which arrived here as a dozen unreadable previews.
if (isOfficeOwnerFile(changed)) continue
const abs = joinRootRel(streamRoot, changed)
if (autoOpened.has(abs) || openPaths.has(abs)) continue
autoOpened.add(abs)
void openFilePreview(abs)
if (autoOpened.has(abs) || pending.has(abs)) continue
// An already-open tab counts as a sighting, not just a skip: this
// feature exists to surface documents the user has NOT seen, so a tab
// they opened by hand must not become a fresh auto-open the moment
// they close it and the agent writes again.
if (openPaths.has(abs)) {
autoOpened.add(abs)
continue
}
const io = splitAbsPath(abs)
if (!io) continue
pending.add(abs)
// changed_paths includes removals, including removed worktree copies.
// Inspect directory metadata before opening a tab, without reading or
// locking the Office document. Share one listing per parent per burst.
let listing = directories.get(io.rootPath)
if (!listing) {
listing = listDirectoryWithFiles(io.rootPath)
directories.set(io.rootPath, listing)
}
void listing
.then((entries) => {
if (cancelled || autoOpened.has(abs)) return
const exists = entries.some(
(entry) =>
!entry.isDir &&
entry.size != null &&
normalizeAbsPath(entry.path) === abs
)
if (!exists) return
autoOpened.add(abs)
// A manual open during the lookup already handled this file.
if (fileTabsRef.current.some((tab) => tab.path === abs)) return
return openFilePreview(abs)
})
.catch(() => {
// Covers both halves of the chain: a removed/unreadable parent is
// not a document to preview, and `openFilePreview` already reports
// its own failures on the tab it seeded.
})
.finally(() => pending.delete(abs))
}
})
return unsubscribe
return () => {
cancelled = true
unsubscribe()
}
}, [
folderPath,
activeFolderIdForOffice,
Expand Down
Loading