diff --git a/extensions/anthropic/cli-constants.test.ts b/extensions/anthropic/cli-constants.test.ts new file mode 100644 index 000000000000..f8c6574382f1 --- /dev/null +++ b/extensions/anthropic/cli-constants.test.ts @@ -0,0 +1,12 @@ +import { CLAUDE_CLI_PROFILE_ID as SDK_CLAUDE_CLI_PROFILE_ID } from "openclaw/plugin-sdk/provider-auth"; +import { describe, expect, it } from "vitest"; +import { CLAUDE_CLI_PROFILE_ID } from "./cli-constants.js"; + +describe("claude cli constants", () => { + it("keeps the local retired profile id aligned with the plugin-sdk constant", () => { + // The provider-policy artifact must stay light, so this plugin carries the + // retired profile id locally instead of importing the provider-auth barrel + // (#129052 regressed dist-less CI checkouts into 120s jiti compiles). + expect(CLAUDE_CLI_PROFILE_ID).toBe(SDK_CLAUDE_CLI_PROFILE_ID); + }); +}); diff --git a/ui/src/components/app-sidebar-row-identity.browser.test.ts b/ui/src/components/app-sidebar-row-identity.browser.test.ts new file mode 100644 index 000000000000..e6ac98cb1d56 --- /dev/null +++ b/ui/src/components/app-sidebar-row-identity.browser.test.ts @@ -0,0 +1,48 @@ +import { afterEach, describe, expect, it } from "vitest"; +import type { GatewayBrowserClient } from "../api/gateway.ts"; +import "../test-helpers/load-styles.ts"; + +afterEach(() => document.body.replaceChildren()); + +describe.runIf("__vitest_browser__" in globalThis)("sidebar session row DOM identity", () => { + it("moves existing row DOM when a new session shifts the list", async () => { + await import("./app-sidebar.ts"); + const { createGatewayHarness, createSessionsHarness, createSessionState, mountSidebar } = + await import("../test-helpers/app-sidebar.ts"); + const harness = createSessionsHarness("main", ["agent:main:alpha", "agent:main:beta"]); + const { sidebar } = await mountSidebar( + createGatewayHarness({ instanceId: "self-instance" } as GatewayBrowserClient).gateway, + harness.sessions, + ); + sidebar.connected = true; + await sidebar.updateComplete; + + const rowFor = (key: string) => + sidebar.querySelector(`[data-session-tree="${key}"]`); + const alphaBefore = rowFor("agent:main:alpha"); + const betaBefore = rowFor("agent:main:beta"); + expect(alphaBefore).not.toBeNull(); + expect(betaBefore).not.toBeNull(); + + // A newly created session sorts first (createdAt desc) and shifts every + // existing row's position; keyed reuse must move their DOM, not rebuild it. + const next = createSessionState("main", [ + "agent:main:alpha", + "agent:main:beta", + "agent:main:gamma", + ]); + const gamma = next.result?.sessions.find((row) => row.key === "agent:main:gamma"); + if (gamma) { + gamma.createdAt = Date.now(); + } + harness.publishList(next); + await sidebar.updateComplete; + + const rowKeys = Array.from(sidebar.querySelectorAll("[data-session-tree]")).map((row) => + row.getAttribute("data-session-tree"), + ); + expect(rowKeys[0]).toBe("agent:main:gamma"); + expect(rowFor("agent:main:alpha")).toBe(alphaBefore); + expect(rowFor("agent:main:beta")).toBe(betaBefore); + }); +}); diff --git a/ui/src/e2e/session-owner-warm-refresh.e2e.test.ts b/ui/src/e2e/session-owner-warm-refresh.e2e.test.ts new file mode 100644 index 000000000000..9fee22acc7c8 --- /dev/null +++ b/ui/src/e2e/session-owner-warm-refresh.e2e.test.ts @@ -0,0 +1,117 @@ +import { mkdir } from "node:fs/promises"; +import path from "node:path"; +import type { Page } from "playwright"; +import { expect, it } from "vitest"; +import { installMockGateway } from "../test-helpers/control-ui-e2e.ts"; +import { createControlUiE2eSuite } from "./control-ui-e2e-suite.test-support.ts"; + +const suite = createControlUiE2eSuite({ name: "Control UI warm owner-first refresh" }); +const captureProof = process.env.OPENCLAW_CAPTURE_UI_PROOF === "1"; +const proofDir = path.join(process.cwd(), ".artifacts", "control-ui-e2e", "session-owner-warm"); + +function sessionRow(ownerId: string, key: string, label: string, updatedAt: number) { + const owner = { + type: "human" as const, + id: ownerId, + label: ownerId === "profile-ada" ? "Ada" : "Bob", + }; + return { + key, + kind: "direct" as const, + label, + createdActor: owner, + owner: { actor: owner }, + updatedAt, + }; +} + +function rosterOf(sessions: ReturnType[]) { + return { + count: sessions.length, + owners: sessions.map((session) => session.owner.actor), + defaults: { contextTokens: null, model: null, modelProvider: null }, + path: "", + sessions, + ts: 1, + }; +} + +async function captureSidebar(page: Page, fileName: string) { + if (!captureProof) { + return; + } + await mkdir(proofDir, { recursive: true }); + await page.locator(".sidebar-sessions").screenshot({ + animations: "disabled", + path: path.join(proofDir, fileName), + }); +} + +suite.define(() => { + it("keeps foreign-owned rows visible while a warm refresh's shared phase is in flight", async () => { + const context = await suite.browser.newContext({ + viewport: { height: 800, width: 1200 }, + ...(captureProof + ? { recordVideo: { dir: proofDir, size: { height: 800, width: 1200 } } } + : {}), + }); + const page = await context.newPage(); + const adaRow = sessionRow("profile-ada", "agent:main:ada", "Ada research", 2); + const bobRow = sessionRow("profile-bob", "agent:main:bob", "Bob operations", 1); + const sharedRoster = rosterOf([adaRow, bobRow]); + const ownerRoster = rosterOf([adaRow]); + const gateway = await installMockGateway(page, { + presenceUsers: [{ self: true, id: "profile-ada", name: "Ada" }], + sessionKey: "agent:main:ada", + methodResponses: { + "sessions.list": { + cases: [ + { match: { ownerId: "profile-ada" }, response: ownerRoster }, + { response: sharedRoster }, + ], + }, + }, + }); + + try { + await page.goto(`${suite.server?.baseUrl ?? ""}chat`); + const ada = page.locator('[data-session-key="agent:main:ada"]'); + const bob = page.locator('[data-session-key="agent:main:bob"]'); + await ada.waitFor(); + await bob.waitFor(); + await captureSidebar(page, "warm-before-event.png"); + + // Hold the warm refresh open at its vulnerable point: the owner-scoped + // request resolves instantly (mocked response), the shared request stays + // deferred. Pre-#129558 the owner-only publish blanks Bob's row here. + const before = (await gateway.getRequests("sessions.list")).length; + await gateway.deferNext("sessions.list", { ownerId: "profile-ada" }); + await gateway.deferNext("sessions.list"); + await gateway.emitGatewayEvent("sessions.changed", { + sessionKey: adaRow.key, + key: adaRow.key, + kind: "direct", + reason: "create", + updatedAt: 3, + }); + await gateway.waitForRequest("sessions.list", { after: before + 1 }); + await gateway.resolveDeferred("sessions.list", ownerRoster); + + // The shared phase is still pending; the roster on screen must not shrink + // to the owner window. Sample repeatedly so a transient blank fails loud. + for (let sample = 0; sample < 6; sample += 1) { + await page.waitForTimeout(100); + expect(await bob.count()).toBe(1); + expect(await ada.count()).toBe(1); + } + await captureSidebar(page, "warm-shared-deferred.png"); + + await gateway.resolveDeferred("sessions.list", sharedRoster); + await expect.poll(() => bob.count()).toBe(1); + expect(await ada.count()).toBe(1); + await captureSidebar(page, "warm-after-merge.png"); + } finally { + await context.close(); + } + }); +}); diff --git a/ui/src/lib/sessions/index.event-refresh.test.ts b/ui/src/lib/sessions/index.event-refresh.test.ts index 0238a143b30c..a81df28e4290 100644 --- a/ui/src/lib/sessions/index.event-refresh.test.ts +++ b/ui/src/lib/sessions/index.event-refresh.test.ts @@ -583,71 +583,6 @@ describe("event-driven session list refresh", () => { } }); - it("retains owner and appended shared pages when an event replaces the list", async () => { - vi.useFakeTimers(); - const ownerId = "profile-ada"; - const ownerTail = { - key: "agent:main:owner-tail", - kind: "direct" as const, - updatedAt: 1, - createdActor: { type: "human" as const, id: ownerId }, - }; - const ownerHead = { - key: "agent:main:owner-head", - kind: "direct" as const, - updatedAt: 3, - createdActor: { type: "human" as const, id: ownerId }, - }; - const sharedRows = [ - ownerHead, - ...Array.from({ length: 119 }, (_, index) => ({ - key: `agent:main:shared-${index}`, - kind: "direct" as const, - updatedAt: 119 - index, - createdActor: { type: "human" as const, id: "profile-bob" }, - })), - ]; - const request = vi.fn( - async (method: string, params?: { limit?: number; offset?: number; ownerId?: string }) => { - if (method !== "sessions.list") { - throw new Error(`Unexpected request: ${method}`); - } - if (params?.ownerId === ownerId) { - return sessionsResult(1, [ownerHead, ownerTail]); - } - const offset = params?.offset ?? 0; - return sessionsResult(2, sharedRows.slice(offset, offset + (params?.limit ?? 50))); - }, - ); - const { sessions, emitEvent } = createHarness( - request as unknown as GatewayBrowserClient["request"], - ownerId, - ); - - try { - await sessions.refresh({ agentId: "main", limit: 60, force: true }); - await sessions.refresh({ agentId: "main", limit: 60, offset: 60, append: true, force: true }); - expect(sessions.state.result?.sessions).toHaveLength(121); - - emitEvent(sessionChangedEvent(sharedRows[1]!.key)); - await vi.advanceTimersByTimeAsync(SESSION_EVENT_REFRESH_DEBOUNCE_MS); - - expect(request.mock.calls).toHaveLength(5); - expect(request.mock.calls.map(([, params]) => params)).toEqual([ - expect.objectContaining({ ownerId, limit: 60 }), - expect.objectContaining({ limit: 60 }), - expect.objectContaining({ limit: 60, offset: 60 }), - expect.objectContaining({ ownerId, limit: 60 }), - expect.objectContaining({ limit: 120 }), - ]); - expect(sessions.state.result?.sessions).toHaveLength(121); - expect(sessions.state.result?.sessions.map((row) => row.key)).toContain(ownerTail.key); - } finally { - sessions.dispose(); - vi.useRealTimers(); - } - }); - it("clears a recreated session's prior deletion before the debounced refresh", async () => { vi.useFakeTimers(); const key = "agent:main:recreated-thread"; diff --git a/ui/src/lib/sessions/owner-first-roster.test.ts b/ui/src/lib/sessions/owner-first-roster.test.ts new file mode 100644 index 000000000000..a8046dd977c3 --- /dev/null +++ b/ui/src/lib/sessions/owner-first-roster.test.ts @@ -0,0 +1,224 @@ +// @vitest-environment node +import { describe, expect, it, vi } from "vitest"; +import type { GatewayBrowserClient, GatewayEventFrame } from "../../api/gateway.ts"; +import type { SessionsListResult } from "../../api/types.ts"; +import { createSessionCapability } from "./index.ts"; + +const SESSION_EVENT_REFRESH_DEBOUNCE_MS = 200; + +function sessionsResult( + ts: number, + sessions: SessionsListResult["sessions"] = [], +): SessionsListResult { + return { + ts, + path: "", + count: sessions.length, + defaults: { modelProvider: null, model: null, contextTokens: null }, + sessions, + }; +} + +function sessionChangedEvent(key: string): GatewayEventFrame { + return { + type: "event", + event: "sessions.changed", + payload: { sessionKey: key, reason: "create", key, kind: "direct", updatedAt: 1 }, + }; +} + +function createHarness(request: GatewayBrowserClient["request"], ownerId: string) { + const client = { request } as GatewayBrowserClient; + let eventListener: ((event: GatewayEventFrame) => void) | undefined; + const sessions = createSessionCapability({ + snapshot: { + client, + phase: "connected", + sessionKey: "agent:main:main", + assistantAgentId: "main", + hello: null, + selfUser: { id: ownerId }, + }, + subscribe: () => () => undefined, + subscribeEvents(listener) { + eventListener = listener; + return () => { + eventListener = undefined; + }; + }, + }); + return { sessions, emitEvent: (event: GatewayEventFrame) => eventListener?.(event) }; +} + +describe("owner-first session roster plan", () => { + it("retains owner and appended shared pages when an event replaces the list", async () => { + vi.useFakeTimers(); + const ownerId = "profile-ada"; + const ownerTail = { + key: "agent:main:owner-tail", + kind: "direct" as const, + updatedAt: 1, + createdActor: { type: "human" as const, id: ownerId }, + }; + const ownerHead = { + key: "agent:main:owner-head", + kind: "direct" as const, + updatedAt: 3, + createdActor: { type: "human" as const, id: ownerId }, + }; + const sharedRows = [ + ownerHead, + ...Array.from({ length: 119 }, (_, index) => ({ + key: `agent:main:shared-${index}`, + kind: "direct" as const, + updatedAt: 119 - index, + createdActor: { type: "human" as const, id: "profile-bob" }, + })), + ]; + const request = vi.fn( + async (method: string, params?: { limit?: number; offset?: number; ownerId?: string }) => { + if (method !== "sessions.list") { + throw new Error(`Unexpected request: ${method}`); + } + if (params?.ownerId === ownerId) { + return sessionsResult(1, [ownerHead, ownerTail]); + } + const offset = params?.offset ?? 0; + return sessionsResult(2, sharedRows.slice(offset, offset + (params?.limit ?? 50))); + }, + ); + const { sessions, emitEvent } = createHarness( + request as unknown as GatewayBrowserClient["request"], + ownerId, + ); + + try { + await sessions.refresh({ agentId: "main", limit: 60, force: true }); + await sessions.refresh({ agentId: "main", limit: 60, offset: 60, append: true, force: true }); + expect(sessions.state.result?.sessions).toHaveLength(121); + + emitEvent(sessionChangedEvent(sharedRows[1]!.key)); + await vi.advanceTimersByTimeAsync(SESSION_EVENT_REFRESH_DEBOUNCE_MS); + + expect(request.mock.calls).toHaveLength(5); + expect(request.mock.calls.map(([, params]) => params)).toEqual([ + expect.objectContaining({ ownerId, limit: 60 }), + expect.objectContaining({ limit: 60 }), + expect.objectContaining({ limit: 60, offset: 60 }), + expect.objectContaining({ ownerId, limit: 60 }), + expect.objectContaining({ limit: 120 }), + ]); + expect(sessions.state.result?.sessions).toHaveLength(121); + expect(sessions.state.result?.sessions.map((row) => row.key)).toContain(ownerTail.key); + } finally { + sessions.dispose(); + vi.useRealTimers(); + } + }); + + it("keeps foreign-owned rows published through a warm owner-first refresh", async () => { + vi.useFakeTimers(); + const ownerId = "profile-ada"; + const ownRow = { + key: "agent:main:ada", + kind: "direct" as const, + updatedAt: 2, + createdActor: { type: "human" as const, id: ownerId }, + }; + const foreignRow = { + key: "agent:main:bob", + kind: "direct" as const, + updatedAt: 1, + createdActor: { type: "human" as const, id: "profile-bob" }, + }; + const request = vi.fn(async (method: string, params?: { ownerId?: string }) => { + if (method !== "sessions.list") { + throw new Error(`Unexpected request: ${method}`); + } + return params?.ownerId === ownerId + ? sessionsResult(1, [ownRow]) + : sessionsResult(2, [ownRow, foreignRow]); + }); + const { sessions, emitEvent } = createHarness( + request as unknown as GatewayBrowserClient["request"], + ownerId, + ); + + try { + await sessions.refresh({ agentId: "main", limit: 60, force: true }); + expect(sessions.state.result?.sessions.map((row) => row.key)).toContain(foreignRow.key); + + const publishedKeySets: string[][] = []; + const stop = sessions.subscribe((next) => { + if (next.result) { + publishedKeySets.push(next.result.sessions.map((row) => row.key)); + } + }); + emitEvent(sessionChangedEvent(ownRow.key)); + await vi.advanceTimersByTimeAsync(SESSION_EVENT_REFRESH_DEBOUNCE_MS); + stop(); + + // Cold start fired owner + shared; the warm event refresh fired both again. + expect(request.mock.calls).toHaveLength(4); + expect(publishedKeySets.length).toBeGreaterThan(0); + for (const keys of publishedKeySets) { + expect(keys).toContain(foreignRow.key); + } + } finally { + sessions.dispose(); + vi.useRealTimers(); + } + }); + + it("keeps the previous roster when the shared phase of a warm refresh fails", async () => { + vi.useFakeTimers(); + const ownerId = "profile-ada"; + const ownRow = { + key: "agent:main:ada", + kind: "direct" as const, + updatedAt: 2, + createdActor: { type: "human" as const, id: ownerId }, + }; + const foreignRow = { + key: "agent:main:bob", + kind: "direct" as const, + updatedAt: 1, + createdActor: { type: "human" as const, id: "profile-bob" }, + }; + let failShared = false; + const request = vi.fn(async (method: string, params?: { ownerId?: string }) => { + if (method !== "sessions.list") { + throw new Error(`Unexpected request: ${method}`); + } + if (params?.ownerId === ownerId) { + return sessionsResult(1, [ownRow]); + } + if (failShared) { + throw new Error("shared roster unavailable"); + } + return sessionsResult(2, [ownRow, foreignRow]); + }); + const { sessions, emitEvent } = createHarness( + request as unknown as GatewayBrowserClient["request"], + ownerId, + ); + + try { + await sessions.refresh({ agentId: "main", limit: 60, force: true }); + expect(sessions.state.result?.sessions).toHaveLength(2); + + failShared = true; + emitEvent(sessionChangedEvent(ownRow.key)); + await vi.advanceTimersByTimeAsync(SESSION_EVENT_REFRESH_DEBOUNCE_MS); + + expect(sessions.state.error).not.toBeNull(); + expect(sessions.state.result?.sessions.map((row) => row.key)).toEqual([ + ownRow.key, + foreignRow.key, + ]); + } finally { + sessions.dispose(); + vi.useRealTimers(); + } + }); +}); diff --git a/ui/src/lib/sessions/session-roster-refresh.ts b/ui/src/lib/sessions/session-roster-refresh.ts index b595282c6040..fc611d7bfce3 100644 --- a/ui/src/lib/sessions/session-roster-refresh.ts +++ b/ui/src/lib/sessions/session-roster-refresh.ts @@ -42,7 +42,6 @@ type ManagedSessionListRefresh = { }; type SessionRosterLoadOptions = SessionRefreshOptions & { - mergeExisting?: boolean; provisional?: boolean; }; @@ -231,16 +230,18 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) { } }; - const load = async (options: SessionRosterLoadOptions, publishAfter?: Promise) => { + const load = async ( + options: SessionRosterLoadOptions, + ownerFirst?: Promise, + ): Promise => { const scope = host.connection.capture(); if (!scope) { - return; + return null; } const { append = false, force: _force, backgroundHydrate = false, - mergeExisting = false, provisional = false, ...requestOptions } = options; @@ -257,7 +258,10 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) { lastListOptions = durableListOptions; hasSeededListOptions = true; } - if (!backgroundHydrate) { + // A provisional owner window may only paint an empty sidebar faster; once a + // roster is on screen it stays silent so foreign-owned rows never blink out. + const provisionalSilent = provisional && Boolean(host.readState().result); + if (!backgroundHydrate && !provisionalSilent) { const error = host.observerError(); host.publish( { ...host.readState(), loading: true, error, deletedSessions: [] }, @@ -266,18 +270,21 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) { } try { const request = requestSessionList(scope.client, requestOptions); - await Promise.allSettled([request, publishAfter]); + const ownerRows = ownerFirst ? await ownerFirst.catch(() => null) : null; const result = await request; if (!host.connection.isCurrent(scope)) { - return; + return null; } const currentState = host.readState(); - const mergeWithCurrent = - mergeExisting || (append && typeof requestOptions.offset === "number"); + if (provisional && currentState.result) { + return result; + } + const merged = result && ownerRows ? appendSessionResults(ownerRows, result) : result; + const mergeWithCurrent = append && typeof requestOptions.offset === "number"; let nextResult = - result && mergeWithCurrent && currentState.result - ? appendSessionResults(currentState.result, result) - : reconcileRosterPresentationMetadata(result, currentState.result); + merged && mergeWithCurrent && currentState.result + ? appendSessionResults(currentState.result, merged) + : reconcileRosterPresentationMetadata(merged, currentState.result); if (append && nextResult && !backgroundHydrate) { const ownerFirstPage = Boolean(host.snapshot().selfUser?.id.trim()) && @@ -347,8 +354,9 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) { }, error ? "session-observer" : undefined, ); + return result; } catch (error) { - if (host.connection.isCurrent(scope)) { + if (host.connection.isCurrent(scope) && !(provisional && host.readState().result)) { const state = host.readState(); host.publish( { @@ -360,6 +368,7 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) { "operation", ); } + return null; } }; @@ -400,7 +409,9 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) { : DEFAULT_SESSION_LIST_QUERY.limit, ); // Keep owner-first and shared loads atomic in the existing refresh queue. - // Only the shared phase advances canonical membership and durable options. + // Only the shared phase advances canonical membership and durable options; + // it merges the owner window from the initial load's returned rows, so the + // provisional phase never has to publish to be part of the final roster. return { initial: { ...options, @@ -411,7 +422,6 @@ export function createSessionRosterRefresh(host: SessionRosterRefreshHost) { shared: { ...options, limit: sharedLimit, - mergeExisting: true, }, }; };