Archived
feat(workspaces): add non-disruptive workspace topology refresh
Adds WorkspaceController.refreshSelectedProjectTopology(), which re-lists the selected project's workspaces and applies them through applyProjectWorkspaces only. It never routes through selectWorkspace, which lacks an already-selected guard and would clear the active session and all workspace-scoped state. Stale responses for a project or machine the user has since left are discarded, and failures go to an injectable background error sink instead of state.error.
This commit is contained in:
@@ -0,0 +1,231 @@
|
|||||||
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
import type { AppState } from "../appState";
|
||||||
|
import { initialAppState } from "../appState";
|
||||||
|
import type { Machine, Project, SessionInfo, Workspace } from "../api";
|
||||||
|
import type { SessionController } from "./sessionController";
|
||||||
|
import { WorkspaceController } from "./workspaceController";
|
||||||
|
|
||||||
|
function machine(id: string): Machine {
|
||||||
|
return { id, name: id, kind: id === "local" ? "local" : "remote", createdAt: "now", updatedAt: "now" };
|
||||||
|
}
|
||||||
|
|
||||||
|
function project(id: string, path: string): Project {
|
||||||
|
return { id, name: id, path, createdAt: "now" };
|
||||||
|
}
|
||||||
|
|
||||||
|
function workspace(projectId: string, path: string, options: { isMain?: boolean } = {}): Workspace {
|
||||||
|
return { id: path, projectId, path, label: path, isMain: options.isMain ?? false, isGitRepo: true, isGitWorktree: true };
|
||||||
|
}
|
||||||
|
|
||||||
|
function session(cwd: string, id = "s1"): SessionInfo {
|
||||||
|
return { id, cwd, path: `${cwd}/.sessions/${id}`, created: "now", modified: "now", messageCount: 1, firstMessage: "hello" };
|
||||||
|
}
|
||||||
|
|
||||||
|
type LoadWorkspaces = (projectId: string, machineId?: string) => Promise<Workspace[]>;
|
||||||
|
|
||||||
|
interface Harness {
|
||||||
|
controller: WorkspaceController;
|
||||||
|
state: () => AppState;
|
||||||
|
clearActiveSession: ReturnType<typeof vi.fn>;
|
||||||
|
updateUrl: ReturnType<typeof vi.fn>;
|
||||||
|
backgroundErrors: { message: string; error: unknown }[];
|
||||||
|
setState: (patch: Partial<AppState>) => void;
|
||||||
|
}
|
||||||
|
|
||||||
|
function harness(initial: Partial<AppState>, loadWorkspaces: LoadWorkspaces): Harness {
|
||||||
|
let state: AppState = { ...initialAppState(), ...initial };
|
||||||
|
const setState = (patch: Partial<AppState>) => { state = { ...state, ...patch }; };
|
||||||
|
const clearActiveSession = vi.fn();
|
||||||
|
const sessions: Pick<SessionController, "clearActiveSession" | "preferredSession" | "selectSession"> = {
|
||||||
|
clearActiveSession,
|
||||||
|
preferredSession: vi.fn(),
|
||||||
|
selectSession: vi.fn(),
|
||||||
|
};
|
||||||
|
const updateUrl = vi.fn();
|
||||||
|
const backgroundErrors: { message: string; error: unknown }[] = [];
|
||||||
|
const controller = new WorkspaceController(
|
||||||
|
() => state,
|
||||||
|
setState,
|
||||||
|
updateUrl,
|
||||||
|
sessions,
|
||||||
|
undefined,
|
||||||
|
{
|
||||||
|
api: { workspaces: loadWorkspaces, sessions: vi.fn<(path: string, machineId?: string) => Promise<SessionInfo[]>>().mockResolvedValue([]) },
|
||||||
|
onBackgroundError: (message, error) => { backgroundErrors.push({ message, error }); },
|
||||||
|
},
|
||||||
|
);
|
||||||
|
return { controller, state: () => state, clearActiveSession, updateUrl, backgroundErrors, setState };
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("WorkspaceController.refreshSelectedProjectTopology", () => {
|
||||||
|
it("surfaces a worktree created outside PI WEB in both the selected list and the per-project cache", async () => {
|
||||||
|
const repo = project("p1", "/repo");
|
||||||
|
const main = workspace(repo.id, repo.path, { isMain: true });
|
||||||
|
const created = workspace(repo.id, "/repo-feature");
|
||||||
|
const loadWorkspaces = vi.fn().mockResolvedValue([main, created]);
|
||||||
|
const test = harness(
|
||||||
|
{
|
||||||
|
selectedMachine: machine("local"),
|
||||||
|
projects: [repo],
|
||||||
|
selectedProject: repo,
|
||||||
|
selectedWorkspace: main,
|
||||||
|
workspaces: [main],
|
||||||
|
workspacesByProjectId: { [repo.id]: [main] },
|
||||||
|
},
|
||||||
|
loadWorkspaces,
|
||||||
|
);
|
||||||
|
|
||||||
|
await test.controller.refreshSelectedProjectTopology();
|
||||||
|
|
||||||
|
expect(loadWorkspaces).toHaveBeenCalledWith(repo.id, "local");
|
||||||
|
expect(test.state().workspaces).toEqual([main, created]);
|
||||||
|
expect(test.state().workspacesByProjectId[repo.id]).toEqual([main, created]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves the selection and workspace-scoped state when the selected workspace still exists", async () => {
|
||||||
|
const repo = project("p1", "/repo");
|
||||||
|
const main = workspace(repo.id, repo.path, { isMain: true });
|
||||||
|
const selected = workspace(repo.id, "/repo-feature");
|
||||||
|
const loadWorkspaces = vi.fn().mockResolvedValue([main, selected, workspace(repo.id, "/repo-other")]);
|
||||||
|
const test = harness(
|
||||||
|
{
|
||||||
|
selectedMachine: machine("local"),
|
||||||
|
projects: [repo],
|
||||||
|
selectedProject: repo,
|
||||||
|
selectedWorkspace: selected,
|
||||||
|
workspaces: [main, selected],
|
||||||
|
workspacesByProjectId: { [repo.id]: [main, selected] },
|
||||||
|
selectedSession: session(selected.path),
|
||||||
|
sessions: [session(selected.path)],
|
||||||
|
selectedFilePath: "src/index.ts",
|
||||||
|
expandedDirs: { src: [] },
|
||||||
|
selectedTerminalId: "t1",
|
||||||
|
},
|
||||||
|
loadWorkspaces,
|
||||||
|
);
|
||||||
|
const before = test.state();
|
||||||
|
|
||||||
|
await test.controller.refreshSelectedProjectTopology();
|
||||||
|
|
||||||
|
const after = test.state();
|
||||||
|
expect(after.selectedWorkspace).toBe(selected);
|
||||||
|
expect(after.selectedSession).toBe(before.selectedSession);
|
||||||
|
expect(after.sessions).toBe(before.sessions);
|
||||||
|
expect(after.selectedFilePath).toBe("src/index.ts");
|
||||||
|
expect(after.expandedDirs).toBe(before.expandedDirs);
|
||||||
|
expect(after.selectedTerminalId).toBe("t1");
|
||||||
|
expect(test.clearActiveSession).not.toHaveBeenCalled();
|
||||||
|
expect(test.updateUrl).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves the selection alone when the selected workspace disappeared", async () => {
|
||||||
|
const repo = project("p1", "/repo");
|
||||||
|
const main = workspace(repo.id, repo.path, { isMain: true });
|
||||||
|
const removed = workspace(repo.id, "/repo-gone");
|
||||||
|
const loadWorkspaces = vi.fn().mockResolvedValue([main]);
|
||||||
|
const test = harness(
|
||||||
|
{
|
||||||
|
selectedMachine: machine("local"),
|
||||||
|
projects: [repo],
|
||||||
|
selectedProject: repo,
|
||||||
|
selectedWorkspace: removed,
|
||||||
|
workspaces: [main, removed],
|
||||||
|
workspacesByProjectId: { [repo.id]: [main, removed] },
|
||||||
|
},
|
||||||
|
loadWorkspaces,
|
||||||
|
);
|
||||||
|
|
||||||
|
await test.controller.refreshSelectedProjectTopology();
|
||||||
|
|
||||||
|
expect(test.state().selectedWorkspace).toBe(removed);
|
||||||
|
expect(test.state().workspaces).toEqual([main]);
|
||||||
|
expect(test.clearActiveSession).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("discards a response for a project the user has since left", async () => {
|
||||||
|
const repo = project("p1", "/repo");
|
||||||
|
const other = project("p2", "/other");
|
||||||
|
const main = workspace(repo.id, repo.path, { isMain: true });
|
||||||
|
const created = workspace(repo.id, "/repo-feature");
|
||||||
|
let resolveWorkspaces: ((workspaces: Workspace[]) => void) | undefined;
|
||||||
|
const loadWorkspaces = vi.fn().mockReturnValue(new Promise<Workspace[]>((resolve) => { resolveWorkspaces = resolve; }));
|
||||||
|
const test = harness(
|
||||||
|
{
|
||||||
|
selectedMachine: machine("local"),
|
||||||
|
projects: [repo, other],
|
||||||
|
selectedProject: repo,
|
||||||
|
selectedWorkspace: main,
|
||||||
|
workspaces: [main],
|
||||||
|
workspacesByProjectId: { [repo.id]: [main] },
|
||||||
|
},
|
||||||
|
loadWorkspaces,
|
||||||
|
);
|
||||||
|
|
||||||
|
const pending = test.controller.refreshSelectedProjectTopology();
|
||||||
|
test.setState({ selectedProject: other, selectedWorkspace: undefined, workspaces: [] });
|
||||||
|
resolveWorkspaces?.([main, created]);
|
||||||
|
await pending;
|
||||||
|
|
||||||
|
expect(test.state().workspaces).toEqual([]);
|
||||||
|
expect(test.state().workspacesByProjectId[repo.id]).toEqual([main]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("discards a response after the selected machine changed", async () => {
|
||||||
|
const repo = project("p1", "/repo");
|
||||||
|
const main = workspace(repo.id, repo.path, { isMain: true });
|
||||||
|
let resolveWorkspaces: ((workspaces: Workspace[]) => void) | undefined;
|
||||||
|
const loadWorkspaces = vi.fn().mockReturnValue(new Promise<Workspace[]>((resolve) => { resolveWorkspaces = resolve; }));
|
||||||
|
const test = harness(
|
||||||
|
{
|
||||||
|
selectedMachine: machine("local"),
|
||||||
|
projects: [repo],
|
||||||
|
selectedProject: repo,
|
||||||
|
selectedWorkspace: main,
|
||||||
|
workspaces: [main],
|
||||||
|
workspacesByProjectId: { [repo.id]: [main] },
|
||||||
|
},
|
||||||
|
loadWorkspaces,
|
||||||
|
);
|
||||||
|
|
||||||
|
const pending = test.controller.refreshSelectedProjectTopology();
|
||||||
|
test.setState({ selectedMachine: machine("remote") });
|
||||||
|
resolveWorkspaces?.([main, workspace(repo.id, "/repo-feature")]);
|
||||||
|
await pending;
|
||||||
|
|
||||||
|
expect(test.state().workspaces).toEqual([main]);
|
||||||
|
expect(test.state().workspacesByProjectId[repo.id]).toEqual([main]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("reports a failed refresh to the background error sink without painting an error banner", async () => {
|
||||||
|
const repo = project("p1", "/repo");
|
||||||
|
const main = workspace(repo.id, repo.path, { isMain: true });
|
||||||
|
const failure = new Error("git worktree list failed");
|
||||||
|
const loadWorkspaces = vi.fn().mockRejectedValue(failure);
|
||||||
|
const test = harness(
|
||||||
|
{
|
||||||
|
selectedMachine: machine("local"),
|
||||||
|
projects: [repo],
|
||||||
|
selectedProject: repo,
|
||||||
|
selectedWorkspace: main,
|
||||||
|
workspaces: [main],
|
||||||
|
workspacesByProjectId: { [repo.id]: [main] },
|
||||||
|
},
|
||||||
|
loadWorkspaces,
|
||||||
|
);
|
||||||
|
|
||||||
|
await test.controller.refreshSelectedProjectTopology();
|
||||||
|
|
||||||
|
expect(test.state().error).toBe("");
|
||||||
|
expect(test.state().workspaces).toEqual([main]);
|
||||||
|
expect(test.backgroundErrors).toEqual([{ message: `Failed to refresh workspaces for project ${repo.id} on local`, error: failure }]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not request anything when no project is selected", async () => {
|
||||||
|
const loadWorkspaces = vi.fn();
|
||||||
|
const test = harness({ selectedMachine: machine("local") }, loadWorkspaces);
|
||||||
|
|
||||||
|
await test.controller.refreshSelectedProjectTopology();
|
||||||
|
|
||||||
|
expect(loadWorkspaces).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -8,10 +8,12 @@ import { InMemoryWorkspaceSelectionMemory, selectPreferredWorkspace, type Worksp
|
|||||||
|
|
||||||
export interface WorkspaceControllerDependencies {
|
export interface WorkspaceControllerDependencies {
|
||||||
api?: Pick<typeof defaultApi, "sessions" | "workspaces">;
|
api?: Pick<typeof defaultApi, "sessions" | "workspaces">;
|
||||||
|
onBackgroundError?: (message: string, error: unknown) => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
export class WorkspaceController {
|
export class WorkspaceController {
|
||||||
private readonly api: Pick<typeof defaultApi, "sessions" | "workspaces">;
|
private readonly api: Pick<typeof defaultApi, "sessions" | "workspaces">;
|
||||||
|
private readonly onBackgroundError: (message: string, error: unknown) => void;
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
private readonly getState: GetState,
|
private readonly getState: GetState,
|
||||||
@@ -22,6 +24,7 @@ export class WorkspaceController {
|
|||||||
deps: WorkspaceControllerDependencies = {},
|
deps: WorkspaceControllerDependencies = {},
|
||||||
) {
|
) {
|
||||||
this.api = deps.api ?? defaultApi;
|
this.api = deps.api ?? defaultApi;
|
||||||
|
this.onBackgroundError = deps.onBackgroundError ?? ((message, error) => { console.warn(message, error); });
|
||||||
}
|
}
|
||||||
|
|
||||||
clearSelection(options?: { updateUrl?: boolean | undefined }) {
|
clearSelection(options?: { updateUrl?: boolean | undefined }) {
|
||||||
@@ -78,6 +81,34 @@ export class WorkspaceController {
|
|||||||
return workspaces;
|
return workspaces;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Re-lists the selected project's workspaces so worktrees created or removed outside
|
||||||
|
* PI WEB become visible, without disturbing the current selection.
|
||||||
|
*
|
||||||
|
* Deliberately never routes through `selectWorkspace`: that has no already-selected
|
||||||
|
* guard, so re-picking the same workspace would still call `clearActiveSession()` and
|
||||||
|
* `resetWorkspaceScopedState()`, closing the session socket and blanking chat, file
|
||||||
|
* tree, git status, and terminal selection. Callers run this on every browser resume,
|
||||||
|
* so applying the list through `applyProjectWorkspaces` alone is the invariant.
|
||||||
|
*
|
||||||
|
* If the selected workspace disappeared, the selection is left alone: the user is
|
||||||
|
* working there and the existing deletion path owns recovery.
|
||||||
|
*/
|
||||||
|
async refreshSelectedProjectTopology(): Promise<void> {
|
||||||
|
const state = this.getState();
|
||||||
|
const project = state.selectedProject;
|
||||||
|
if (project === undefined) return;
|
||||||
|
const machineId = selectedMachineId(state);
|
||||||
|
try {
|
||||||
|
const workspaces = await this.api.workspaces(project.id, machineId);
|
||||||
|
const current = this.getState();
|
||||||
|
if (selectedMachineId(current) !== machineId || current.selectedProject?.id !== project.id) return;
|
||||||
|
this.applyProjectWorkspaces(project.id, workspaces);
|
||||||
|
} catch (error) {
|
||||||
|
this.onBackgroundError(`Failed to refresh workspaces for project ${project.id} on ${machineId}`, error);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async refreshAfterWorkspaceDeleted(projectId: string, workspaceId: string): Promise<void> {
|
async refreshAfterWorkspaceDeleted(projectId: string, workspaceId: string): Promise<void> {
|
||||||
const workspaces = await this.refreshProjectWorkspaces(projectId);
|
const workspaces = await this.refreshProjectWorkspaces(projectId);
|
||||||
const state = this.getState();
|
const state = this.getState();
|
||||||
|
|||||||
Reference in New Issue
Block a user