mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
fix(ui): stop per-message branch-list polling and unlatch transient branch failures (#124094)
* fix(ui): stop per-message branch-list polling and unlatch transient branch failures
Four coupled defects around the chat branch dropdown, fixed at their owners:
- Every persisted session.message and every sessions.changed triggered an
unconditional sessions.branches.list RPC per viewing client; the gateway
branch cache watermark is max(seq), so each message append made the next
list a full transcript rescan. Branch topology only changes on structural
mutations, and the producer already records the reason — the reload is now
gated on rewind/branch-switch/fork/reset/new.
- A transient listBranches failure recorded success (chatBranchesSessionKey
set with branches=[]), permanently hiding the branch dropdown for that
session+connection with no visible outcome. The catch no longer latches;
the next history load retries naturally.
- sessions.branches.list errored for upstream-linked sessions even though
"no local branches" is their truthful steady state; the fresh-session
sibling was already converted to {branches: []} for the same reason
(spurious gateway-log failures). Only mutating siblings fail closed now.
- Branch state was keyed by raw session-key spelling and strict-compared in
the header and history gate, blanking the dropdown across the legacy
main -> agent:main:main alias window (same class as #124020); both
compares use areUiSessionKeysEquivalent.
Riders per pathfinder rule: deleted the write-only chatBranchesLoading flag,
a duplicate activeLeafEntryId recompute, and two provably-no-op second
dropPreSessionStartAnnouncePairs passes in chat-history-pages.ts (the filter
is single-pass complete and its inputs were already filtered).
Production LOC +27/-34 (net -7); regression tests fail pre-fix.
* refactor(ui): move branch-display ownership out of the header render
check-lint-core-4 flagged chat-pane-header.ts at 702 counted lines after the
equivalence gate landed there. The branches-for-display decision belongs to
the state owner anyway: displayedChatSessionBranches in chat-history.ts now
owns it and the header render consumes the fact.
This commit is contained in:
committed by
GitHub
parent
aaa509b26e
commit
f4beff12cd
@@ -407,11 +407,9 @@ export async function readChatHistoryPage(params: {
|
||||
max,
|
||||
Math.max(readPage.messages.length, readPage.totalMessages > pageOffset ? 1 : 0),
|
||||
);
|
||||
const rawMessages = localMessages;
|
||||
const recencyFilteredMessages = dropPreSessionStartAnnouncePairs(
|
||||
rawMessages,
|
||||
typeof entry?.sessionStartedAt === "number" ? entry.sessionStartedAt : undefined,
|
||||
);
|
||||
// localMessages is already announce-filtered above; the filter is
|
||||
// single-pass complete, so no second pass is needed.
|
||||
const recencyFilteredMessages = localMessages;
|
||||
const projected = isTailPage
|
||||
? projectRecentChatDisplayMessages(recencyFilteredMessages, {
|
||||
maxChars: effectiveMaxChars,
|
||||
@@ -539,15 +537,9 @@ export async function readChatHistoryPage(params: {
|
||||
},
|
||||
};
|
||||
}
|
||||
const rawMessages = cliHistory.messages;
|
||||
// Drop subagent_announce pairs (user inter-session announce + adjacent
|
||||
// assistant) whose record timestamp predates the current session's
|
||||
// sessionStartedAt. Run after CLI history imports too, because those
|
||||
// timestamped messages share the same chat.history response surface.
|
||||
const recencyFilteredMessages = dropPreSessionStartAnnouncePairs(
|
||||
rawMessages,
|
||||
typeof entry?.sessionStartedAt === "number" ? entry.sessionStartedAt : undefined,
|
||||
);
|
||||
// The imported case returned above, so these are the already announce-filtered
|
||||
// local messages; the filter is single-pass complete, so no second pass is needed.
|
||||
const recencyFilteredMessages = cliHistory.messages;
|
||||
const displayMessages = projectRecentChatDisplayMessages(recencyFilteredMessages, {
|
||||
maxChars: effectiveMaxChars,
|
||||
maxMessages: max,
|
||||
|
||||
@@ -454,21 +454,21 @@ describe("session message-cut methods", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects externally owned conversations", async () => {
|
||||
it("rejects mutation but lists empty branches for externally owned conversations", async () => {
|
||||
linkToUpstreamConversation();
|
||||
const respond = await invoke("sessions.branches.switch", "off-path-entry");
|
||||
expect(respond).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
code: ErrorCodes.INVALID_REQUEST,
|
||||
message: expect.stringContaining("external agent harness"),
|
||||
}),
|
||||
);
|
||||
// Listing is read-only: "no local branches" is the truthful steady state,
|
||||
// not an error to latch into the UI.
|
||||
const listed = await invoke("sessions.branches.list");
|
||||
|
||||
for (const response of [respond, listed]) {
|
||||
expect(response).toHaveBeenCalledWith(
|
||||
false,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
code: ErrorCodes.INVALID_REQUEST,
|
||||
message: expect.stringContaining("external agent harness"),
|
||||
}),
|
||||
);
|
||||
}
|
||||
expect(listed).toHaveBeenCalledWith(true, { branches: [] }, undefined);
|
||||
});
|
||||
|
||||
it.each(["sessions.rewind", "sessions.branches.switch"] as const)(
|
||||
|
||||
@@ -177,7 +177,9 @@ async function listBranches(options: GatewayRequestHandlerOptions): Promise<void
|
||||
return;
|
||||
}
|
||||
if (readSessionUpstreamLink(current.canonicalKey, current.target.agentId)) {
|
||||
respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, EXTERNAL_CONVERSATION_ERROR));
|
||||
// Upstream-linked sessions truthfully have no local branches; only the
|
||||
// mutating siblings (rewind/switch/fork) must fail closed on them.
|
||||
respond(true, { branches: [] }, undefined);
|
||||
return;
|
||||
}
|
||||
const result = await listSessionBranches({
|
||||
|
||||
@@ -416,6 +416,45 @@ describe("switchChatHistoryBranch", () => {
|
||||
expect(state.chatBranchesConnectionEpoch).toBe(state.connectionEpoch);
|
||||
});
|
||||
|
||||
it("retries the branch list on the next history load after a transient failure", async () => {
|
||||
const state = createState({ messages: [] }) as TestState & {
|
||||
sessions: { listBranches: ReturnType<typeof vi.fn> };
|
||||
};
|
||||
state.sessions = {
|
||||
listBranches: vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error("gateway hiccup"))
|
||||
.mockResolvedValue([
|
||||
{ leafEntryId: "tip", headline: "tip", messageCount: 1, active: true },
|
||||
]),
|
||||
setModelOverride: vi.fn(),
|
||||
};
|
||||
|
||||
await loadChatHistory(state);
|
||||
// The transient failure must not latch success state; the next load retries.
|
||||
expect(state.chatBranchesSessionKey ?? null).toBeNull();
|
||||
|
||||
await loadChatHistory(state);
|
||||
expect(state.sessions.listBranches).toHaveBeenCalledTimes(2);
|
||||
expect(state.chatBranchesSessionKey).toBe(state.sessionKey);
|
||||
expect(state.chatBranches).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("treats the legacy main alias and canonical key as the same branch owner", async () => {
|
||||
const state = createState({ messages: [] }) as TestState & {
|
||||
sessions: { listBranches: ReturnType<typeof vi.fn> };
|
||||
};
|
||||
state.sessionKey = "main";
|
||||
state.chatBranchesSessionKey = "agent:main:main";
|
||||
state.chatBranchesConnectionEpoch = state.connectionEpoch;
|
||||
state.sessions = { listBranches: vi.fn().mockResolvedValue([]), setModelOverride: vi.fn() };
|
||||
|
||||
await loadChatHistory(state);
|
||||
|
||||
// Equivalent spellings must not force a redundant branch reload.
|
||||
expect(state.sessions.listBranches).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("starts a fresh snapshot and rejects in-flight history after a same-key branch switch", async () => {
|
||||
let resolvePreviousHistory!: (result: ChatHistoryResult) => void;
|
||||
const previousHistory = new Promise<ChatHistoryResult>((resolve) => {
|
||||
|
||||
@@ -311,7 +311,6 @@ export type ChatState = {
|
||||
chatBranches?: SessionBranch[];
|
||||
chatBranchesSessionKey?: string | null;
|
||||
chatBranchesConnectionEpoch?: number | null;
|
||||
chatBranchesLoading?: boolean;
|
||||
requestUpdate?: () => void;
|
||||
};
|
||||
|
||||
@@ -1371,6 +1370,15 @@ export async function switchChatHistoryBranch(
|
||||
}
|
||||
}
|
||||
|
||||
/** Branches for the current pane; equivalence covers alias-canonicalization windows (#124020 class). */
|
||||
export function displayedChatSessionBranches(
|
||||
state: Pick<ChatState, "chatBranches" | "chatBranchesSessionKey" | "sessionKey">,
|
||||
): SessionBranch[] {
|
||||
return areUiSessionKeysEquivalent(state.chatBranchesSessionKey, state.sessionKey)
|
||||
? (state.chatBranches ?? [])
|
||||
: [];
|
||||
}
|
||||
|
||||
export async function loadChatBranches(state: ChatState): Promise<void> {
|
||||
const sessions = state.sessions;
|
||||
const client = state.client;
|
||||
@@ -1388,7 +1396,6 @@ export async function loadChatBranches(state: ChatState): Promise<void> {
|
||||
const version = ++requests.branchVersion;
|
||||
const connectionEpoch = state.connectionEpoch;
|
||||
const agentParams = scopedAgentParamsForSession(state, sessionKey);
|
||||
state.chatBranchesLoading = true;
|
||||
try {
|
||||
const branches = await sessions.listBranches(sessionKey, agentParams);
|
||||
if (
|
||||
@@ -1404,19 +1411,11 @@ export async function loadChatBranches(state: ChatState): Promise<void> {
|
||||
state.chatBranchesSessionKey = sessionKey;
|
||||
state.chatBranchesConnectionEpoch = connectionEpoch;
|
||||
} catch {
|
||||
if (
|
||||
requests.branchVersion === version &&
|
||||
state.client === client &&
|
||||
state.connectionEpoch === connectionEpoch &&
|
||||
visibleSessionMatches(state, sessionKey, agentParams.agentId)
|
||||
) {
|
||||
state.chatBranches = [];
|
||||
state.chatBranchesSessionKey = sessionKey;
|
||||
state.chatBranchesConnectionEpoch = connectionEpoch;
|
||||
}
|
||||
// Leave chatBranchesSessionKey unset so the next history load retries;
|
||||
// recording success here latched transient failures into a permanently
|
||||
// hidden branch dropdown with no visible outcome.
|
||||
} finally {
|
||||
if (requests.branchVersion === version) {
|
||||
state.chatBranchesLoading = false;
|
||||
state.requestUpdate?.();
|
||||
}
|
||||
}
|
||||
@@ -1452,7 +1451,7 @@ export async function loadChatHistory(
|
||||
}
|
||||
if (
|
||||
opts.deferBranches !== true &&
|
||||
(state.chatBranchesSessionKey !== sessionKey ||
|
||||
(!areUiSessionKeysEquivalent(state.chatBranchesSessionKey, sessionKey) ||
|
||||
state.chatBranchesConnectionEpoch !== connectionEpoch)
|
||||
) {
|
||||
void loadChatBranches(state);
|
||||
@@ -1662,7 +1661,7 @@ async function loadChatHistoryUncached(
|
||||
},
|
||||
);
|
||||
if (Object.hasOwn(res.sessionInfo ?? {}, "activeLeafEntryId")) {
|
||||
state.chatDisplayedLeafEntryId = res.sessionInfo?.activeLeafEntryId?.trim() || null;
|
||||
state.chatDisplayedLeafEntryId = nextDisplayedLeafEntryId;
|
||||
}
|
||||
retirePersistedSteeredChips(state);
|
||||
state.chatHistoryPagination = reconciledHistory?.pagination ?? nextPagination;
|
||||
|
||||
@@ -28,6 +28,7 @@ import {
|
||||
} from "../../lib/sessions/session-key.ts";
|
||||
import { isActiveTask } from "../../lib/tasks/data.ts";
|
||||
import { renderBoardViewSwitch } from "./board-session-surface.ts";
|
||||
import { displayedChatSessionBranches } from "./chat-history.ts";
|
||||
import { resolveChatPaneDesktopTarget, resolveChatPanePlacement } from "./chat-pane-placement.ts";
|
||||
import { ChatPaneSessionMenu } from "./chat-pane-session-menu.ts";
|
||||
import { readChatSessionActionAccess } from "./chat-session-action-access.ts";
|
||||
@@ -366,10 +367,7 @@ export abstract class ChatPaneHeader extends ChatPaneSessionMenu {
|
||||
workspaceIcon: this.resolveWorkspaceIcon(workspace.root ? row?.key : undefined),
|
||||
parentSession: resolveChatPaneParentSession(row, this.state?.sessionsResult?.sessions ?? []),
|
||||
branch,
|
||||
branches:
|
||||
this.state && this.state.chatBranchesSessionKey === this.state.sessionKey
|
||||
? (this.state.chatBranches ?? [])
|
||||
: [],
|
||||
branches: this.state ? displayedChatSessionBranches(this.state) : [],
|
||||
branchSwitchDisabledReason,
|
||||
platform: this.headerPlatform,
|
||||
canReveal,
|
||||
|
||||
@@ -202,7 +202,6 @@ function handleSessionMessageEvent(state: ChatPageHost, payload: unknown) {
|
||||
// replaces it in place instead of appending below the newer user turn.
|
||||
applyLiveSessionMessage(state, payload, event.hasActiveRun ?? undefined);
|
||||
retirePersistedSteeredChips(state);
|
||||
void loadChatBranches(state);
|
||||
}
|
||||
if (matchesChat && event.archived !== null) {
|
||||
state.selectedChatSessionArchived = event.archived;
|
||||
@@ -262,6 +261,11 @@ function replayPendingSessionMessageReload(
|
||||
void loadChatHistory(state).finally(() => state.requestUpdate?.());
|
||||
}
|
||||
|
||||
// Branch topology only changes on structural mutations; the producer records
|
||||
// the reason, so reload branches only for those instead of on every
|
||||
// sessions.changed (each cache miss rescans the full transcript on the gateway).
|
||||
const BRANCH_TOPOLOGY_REASONS = new Set(["rewind", "branch-switch", "fork", "reset", "new"]);
|
||||
|
||||
function handleSessionsChangedEvent(state: ChatPageHost, payload: unknown) {
|
||||
const runIdBeforeApply = state.chatRunId;
|
||||
const event = readSessionChangedEvent(payload);
|
||||
@@ -280,7 +284,11 @@ function handleSessionsChangedEvent(state: ChatPageHost, payload: unknown) {
|
||||
// only proof that its old live and pending transcript no longer exists.
|
||||
reduceChatSessionProjection(state, { type: "sessionReset" }, { scope });
|
||||
}
|
||||
if (matchesChat) {
|
||||
if (
|
||||
matchesChat &&
|
||||
typeof source?.reason === "string" &&
|
||||
BRANCH_TOPOLOGY_REASONS.has(source.reason)
|
||||
) {
|
||||
void loadChatBranches(state);
|
||||
}
|
||||
if (event && matchesChat && event.archived !== null) {
|
||||
|
||||
@@ -170,7 +170,6 @@ export function createPageState(
|
||||
chatBranches: [],
|
||||
chatBranchesSessionKey: null,
|
||||
chatBranchesConnectionEpoch: null,
|
||||
chatBranchesLoading: false,
|
||||
chatToolMessages: [],
|
||||
chatThinkingLevel: null,
|
||||
chatVerboseLevel: null,
|
||||
|
||||
Reference in New Issue
Block a user