Skip to content
Open
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
13 changes: 12 additions & 1 deletion packages/server/src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6128,6 +6128,14 @@ export class Session {
? { ...request.filter, includeArchived: true }
: request.filter;
const scope = request.type === "fetch_agents_request" ? request.scope : undefined;
// The history list never renders checkout info (the client requests it with
// showCheckoutInfo=false and does not read the `project` payload), so resolving
// each agent's placement via live git is pure overhead. On slow filesystems
// (network mounts) a per-cwd `git status` can take seconds, and summed across
// many sessions it blows past the client's 10s RPC timeout, leaving the list
// blank. Resolve placement from cached snapshots + the workspace registry
// (refreshGit: false) instead — no git spawns on the history path.
const skipGitPlacement = request.type === "fetch_agent_history_request";
const sort = this.agentsPager.normalizeSort(request.sort);

let agents = await this.listAgentPayloads({
Expand All @@ -6154,7 +6162,10 @@ export class Session {
if (existing) {
return existing;
}
const placementPromise = this.buildProjectPlacementForCwd(cwd);
const placementPromise = this.buildProjectPlacementForCwd(
cwd,
skipGitPlacement ? { refreshGit: false } : undefined,
);
placementByCwd.set(cwd, placementPromise);
return placementPromise;
};
Expand Down
77 changes: 76 additions & 1 deletion packages/server/src/server/session.workspaces.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ import type {
PersistedAgentDescriptor,
} from "./agent/agent-sdk-types.js";
import type { WorkspaceGitRuntimeSnapshot } from "./workspace-git-service.js";
import { createNoopWorkspaceGitService } from "./test-utils/workspace-git-service-stub.js";
import {
createNoGitWorkspaceRuntimeSnapshot,
createNoopWorkspaceGitService,
} from "./test-utils/workspace-git-service-stub.js";
import {
asSessionLogger,
asAgentManager,
Expand Down Expand Up @@ -2112,6 +2115,78 @@ test("fetch_agent_history_request pages archived historical rows separately", as
expect(session.agentUpdatesSubscription).toBeNull();
});

test("fetch_agent_history_request resolves placement without spawning git", async () => {
// Regression: the history list never renders checkout info, so it must not pay
// the per-cwd `git status` cost. On slow filesystems that cost summed across
// sessions blew past the client's 10s RPC timeout and left the list blank.
// Here `getCheckout` (the git-spawning path) throws to assert it is never hit;
// placement must come from the cached snapshot + workspace registry instead.
const emitted: SessionOutboundMessage[] = [];
const repoCwd = path.resolve("/tmp/history-git-repo");
const getCheckout = vi.fn(async () => {
throw new Error("getCheckout must not be called on the history path");
});
const peekSnapshot = vi.fn(() => createNoGitWorkspaceRuntimeSnapshot(repoCwd));
const session = createSessionForWorkspaceTests({
workspaceGitService: createNoopWorkspaceGitService({ getCheckout, peekSnapshot }),
});

const project = createPersistedProjectRecord({
projectId: "proj-history-git",
rootPath: repoCwd,
kind: "git",
displayName: "history-git",
createdAt: "2026-03-01T12:00:00.000Z",
updatedAt: "2026-03-01T12:00:00.000Z",
});
const workspace = createPersistedWorkspaceRecord({
workspaceId: "ws-history-git",
projectId: project.projectId,
cwd: repoCwd,
kind: "local_checkout",
displayName: "history-git",
createdAt: "2026-03-01T12:00:00.000Z",
updatedAt: "2026-03-01T12:00:00.000Z",
});

session.emit = (message) => {
if (isSessionOutboundMessage(message)) emitted.push(message);
};
session.projectRegistry.get = async () => project;
session.projectRegistry.list = async () => [project];
session.workspaceRegistry.list = async () => [workspace];
session.listAgentPayloads = async () => [
makeAgent({
id: "history-git-agent",
cwd: repoCwd,
status: "idle",
updatedAt: "2026-03-01T12:00:00.000Z",
}),
];

await session.handleMessage({
type: "fetch_agent_history_request",
requestId: "req-history-git",
page: { limit: 25 },
});

expect(getCheckout).not.toHaveBeenCalled();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Mock call-count assertion on the boundary

expect(getCheckout).not.toHaveBeenCalled() asserts implementation internals rather than observable behavior. The test already covers the behavioral goal because getCheckout is wired to throw — if the history path ever calls it, the test fails with an unhandled rejection before reaching the expect(emitted) check. The explicit not-called assertion is therefore redundant and falls into the pattern the project rules flag (asserting spy/mock call counts instead of recording observable state). Consider relying solely on the throw-on-call guard and the emitted-entries assertion.

Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

expect(emitted).toEqual([
{
type: "fetch_agent_history_response",
payload: expect.objectContaining({
requestId: "req-history-git",
entries: [
expect.objectContaining({
agent: expect.objectContaining({ id: "history-git-agent" }),
project: expect.objectContaining({ projectKey: "proj-history-git" }),
}),
],
}),
},
]);
});

test("fetch_recent_provider_sessions_request lists importable provider sessions by handle", async () => {
const emitted: Array<{ type: string; payload: unknown }> = [];
const session = createSessionForWorkspaceTests();
Expand Down