mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site): stop hijacking navigation after archive-and-delete settles (#23372)
- Guard both `onSettled` callbacks in
`archiveAndDeleteMutation.mutate()` with `shouldNavigateAfterArchive()`,
which checks whether the user is still viewing the archived chat (or a
sub-agent of it) before calling `navigate("/agents")`
- Extract `shouldNavigateAfterArchive` into `agentWorkspaceUtils.ts`
with 6 unit test cases covering: direct match, different chat, no active
chat, sub-agent of archived parent, sub-agent of different parent, and
cache-cleared fallback
- Look up the active chat's `root_chat_id` from the per-chat query cache
(stable across WebSocket eviction of sub-agents) to handle the sub-agent
case
> 🤖 This PR was created with the help of Coder Agents, and has been
reviewed by my human. 🧑💻
This commit is contained in:
@@ -33,7 +33,10 @@ import { emptyInputStorageKey } from "./components/AgentCreateForm";
|
||||
import { maybePlayChime } from "./components/AgentDetail/useAgentChime";
|
||||
import { useAgentsPageKeybindings } from "./hooks/useAgentsPageKeybindings";
|
||||
import { useAgentsPWA } from "./hooks/useAgentsPWA";
|
||||
import { resolveArchiveAndDeleteAction } from "./utils/agentWorkspaceUtils";
|
||||
import {
|
||||
resolveArchiveAndDeleteAction,
|
||||
shouldNavigateAfterArchive,
|
||||
} from "./utils/agentWorkspaceUtils";
|
||||
import { getModelOptionsFromCatalog } from "./utils/modelOptions";
|
||||
import type { ChatDetailError } from "./utils/usageLimitMessage";
|
||||
|
||||
@@ -262,7 +265,27 @@ const AgentsPage: FC = () => {
|
||||
archiveAndDeleteMutation.mutate(
|
||||
{ chatId, workspaceId },
|
||||
{
|
||||
onSettled: () => navigate("/agents"),
|
||||
onSettled: () => {
|
||||
const activeChatId = activeChatIDRef.current;
|
||||
if (
|
||||
shouldNavigateAfterArchive(
|
||||
activeChatId,
|
||||
chatId,
|
||||
// Read root_chat_id from the per-chat
|
||||
// cache, which survives WebSocket eviction
|
||||
// of sub-agents (only the parent's chatKey
|
||||
// is removed). Must be read at settle time
|
||||
// so it reflects the user's current location.
|
||||
activeChatId
|
||||
? queryClient.getQueryData<TypesGen.Chat>(
|
||||
chatKey(activeChatId),
|
||||
)?.root_chat_id
|
||||
: undefined,
|
||||
)
|
||||
) {
|
||||
navigate("/agents");
|
||||
}
|
||||
},
|
||||
},
|
||||
);
|
||||
} else {
|
||||
@@ -274,10 +297,23 @@ const AgentsPage: FC = () => {
|
||||
};
|
||||
const handleConfirmArchiveAndDelete = () => {
|
||||
if (pendingArchiveAndDelete && !isArchiving) {
|
||||
const { chatId: archivedChatId } = pendingArchiveAndDelete;
|
||||
archiveAndDeleteMutation.mutate(pendingArchiveAndDelete, {
|
||||
onSettled: () => {
|
||||
setPendingArchiveAndDelete(null);
|
||||
navigate("/agents");
|
||||
const activeChatId = activeChatIDRef.current;
|
||||
if (
|
||||
shouldNavigateAfterArchive(
|
||||
activeChatId,
|
||||
archivedChatId,
|
||||
activeChatId
|
||||
? queryClient.getQueryData<TypesGen.Chat>(chatKey(activeChatId))
|
||||
?.root_chat_id
|
||||
: undefined,
|
||||
)
|
||||
) {
|
||||
navigate("/agents");
|
||||
}
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
isWorkspaceAutoCreated,
|
||||
resolveArchiveAndDeleteAction,
|
||||
shouldNavigateAfterArchive,
|
||||
} from "./agentWorkspaceUtils";
|
||||
|
||||
describe("isWorkspaceAutoCreated", () => {
|
||||
@@ -80,3 +81,58 @@ describe("resolveArchiveAndDeleteAction", () => {
|
||||
).rejects.toThrow("not found");
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldNavigateAfterArchive", () => {
|
||||
it.each([
|
||||
{
|
||||
name: "user still viewing archived chat",
|
||||
activeChatId: "abc-123",
|
||||
archivedChatId: "abc-123",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "user navigated to a different chat",
|
||||
activeChatId: "xyz-456",
|
||||
archivedChatId: "abc-123",
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "user navigated to /agents root (no active chat)",
|
||||
activeChatId: undefined,
|
||||
archivedChatId: "abc-123",
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "user viewing sub-agent of archived parent",
|
||||
activeChatId: "sub-agent-1",
|
||||
archivedChatId: "parent-abc",
|
||||
activeRootChatId: "parent-abc",
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "user viewing sub-agent of a different parent",
|
||||
activeChatId: "sub-agent-1",
|
||||
archivedChatId: "parent-abc",
|
||||
activeRootChatId: "parent-other",
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "root chat ID not available (cache cleared)",
|
||||
activeChatId: "sub-agent-1",
|
||||
archivedChatId: "parent-abc",
|
||||
activeRootChatId: undefined,
|
||||
expected: false,
|
||||
},
|
||||
])(
|
||||
"$name → $expected",
|
||||
({ activeChatId, archivedChatId, activeRootChatId, expected }) => {
|
||||
expect(
|
||||
shouldNavigateAfterArchive(
|
||||
activeChatId,
|
||||
archivedChatId,
|
||||
activeRootChatId,
|
||||
),
|
||||
).toBe(expected);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -12,6 +12,26 @@ export function isWorkspaceAutoCreated(
|
||||
return new Date(workspaceCreatedAt) >= new Date(chatCreatedAt);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns whether the browser should navigate to /agents after an
|
||||
* archive-and-delete mutation settles. Navigation is appropriate
|
||||
* when the user is still viewing the archived chat or one of its
|
||||
* sub-agents; if they already navigated elsewhere the redirect
|
||||
* would be disruptive.
|
||||
*/
|
||||
export function shouldNavigateAfterArchive(
|
||||
activeChatId: string | undefined,
|
||||
archivedChatId: string,
|
||||
activeRootChatId?: string,
|
||||
): boolean {
|
||||
if (activeChatId === archivedChatId) return true;
|
||||
// The active chat is a sub-agent rooted at the archived parent.
|
||||
if (activeRootChatId != null && activeRootChatId === archivedChatId) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves whether an archive-and-delete action should proceed
|
||||
* immediately or require user confirmation. Fetches the workspace
|
||||
|
||||
Reference in New Issue
Block a user