diff --git a/ui/src/components/app-sidebar-session-navigation-logic.ts b/ui/src/components/app-sidebar-session-navigation-logic.ts index 0c4291fc1027..297800600b88 100644 --- a/ui/src/components/app-sidebar-session-navigation-logic.ts +++ b/ui/src/components/app-sidebar-session-navigation-logic.ts @@ -207,6 +207,7 @@ export function buildSidebarSessionNavigationState(input: { // The sidebar's zone structure already says what forked from what; // a "Subagent:" prefix on named threads is noise (other surfaces keep it). label: resolveSessionDisplayName(row.key, row, { includeSubagentPrefix: false }), + userLabel: row.label, subtitle: resolveSessionWorkSubtitle(row), href: sessionNavigationTarget({ face: resolveSessionPreferredFace(row), diff --git a/ui/src/components/app-sidebar-session-types.ts b/ui/src/components/app-sidebar-session-types.ts index 4b094ffd0d28..3d318a0673cc 100644 --- a/ui/src/components/app-sidebar-session-types.ts +++ b/ui/src/components/app-sidebar-session-types.ts @@ -61,6 +61,12 @@ export type SidebarRecentSession = { createdActor?: SessionCreatedActor; archivedBy?: SessionCreatedActor; label: string; + /** + * Stored user label, undecorated. `label` above is the resolved display name + * and can carry a derived account or channel; rename edits this one so a + * derived string never lands back in persisted state. + */ + userLabel?: string; /** Compact repo/branch/node line for work sessions. */ subtitle?: string; href: string; diff --git a/ui/src/components/session-organizer-controller.ts b/ui/src/components/session-organizer-controller.ts index 402d93cddf2b..df330f68988e 100644 --- a/ui/src/components/session-organizer-controller.ts +++ b/ui/src/components/session-organizer-controller.ts @@ -1,3 +1,4 @@ +import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import type { ReactiveControllerHost } from "lit"; import type { FsListDirResult } from "../../../packages/gateway-protocol/src/index.js"; import { @@ -419,7 +420,10 @@ export class SessionOrganizerController { const nextLabel = (await showInputDialog?.({ title: t("sessionsView.renameSessionPrompt"), - defaultValue: session.label, + // The stored label, not the resolved display name: pre-filling the + // derived string persists it on submit and it then outranks every + // later derivation. Matches the Sessions page rename. + defaultValue: normalizeOptionalString(session.userLabel) ?? "", })) ?? null; if (nextLabel === null) { return; diff --git a/ui/src/e2e/session-management.account-label.e2e.test.ts b/ui/src/e2e/session-management.account-label.e2e.test.ts new file mode 100644 index 000000000000..af9c449141ea --- /dev/null +++ b/ui/src/e2e/session-management.account-label.e2e.test.ts @@ -0,0 +1,102 @@ +import { expect, it } from "vitest"; +import { + captureUiProof, + controlUiSessionUrl, + createSessionManagementE2eSuite, + installMockGateway, + sessionRow, + sessionsListResponse, + trimmedTextContents, +} from "./session-management.test-support.ts"; + +const suite = createSessionManagementE2eSuite(); + +/** + * An ordinary Gateway direct-chat row: an origin-derived `displayName`, no user + * label, and the `accountId` the Gateway projects from the canonical route + * (src/gateway/session-classification.ts). Without both traits this would + * exercise the label branch instead of the shipped one. + */ +function gatewayDirectRow(key: string, updatedAt: number, accountId?: string) { + return { ...sessionRow(key, "Alice", updatedAt), accountId, label: undefined }; +} + +suite.define(() => { + it("disambiguates same-name sessions from different Telegram accounts in the sidebar", async () => { + const defaultKey = "agent:main:telegram:direct:42"; + const cardsKey = "agent:main:telegram:cards:direct:42"; + const context = await suite.browser.newContext({ + colorScheme: "dark", + locale: "en-US", + serviceWorkers: "block", + viewport: { height: 900, width: 1280 }, + }); + const page = await context.newPage(); + await installMockGateway(page, { + methodResponses: { + "sessions.list": sessionsListResponse([ + gatewayDirectRow(defaultKey, 2), + gatewayDirectRow(cardsKey, 1, "cards"), + ]), + }, + sessionKey: defaultKey, + }); + + try { + await page.goto(controlUiSessionUrl(suite.server.baseUrl, defaultKey)); + const defaultRow = page.locator(`[data-session-key="${defaultKey}"]`); + const cardsRow = page.locator(`[data-session-key="${cardsKey}"]`); + await defaultRow.waitFor({ state: "visible", timeout: 10_000 }); + await cardsRow.waitFor({ state: "visible" }); + await expect + .poll(() => trimmedTextContents(defaultRow.locator(".sidebar-recent-session__name"))) + .toEqual(["Alice"]); + await expect + .poll(() => trimmedTextContents(cardsRow.locator(".sidebar-recent-session__name"))) + .toEqual(["Alice · cards"]); + await captureUiProof(page, "telegram-account-session-labels.png"); + } finally { + await context.close(); + } + }); + + it("opens rename on the stored label, not the account-decorated name", async () => { + const cardsKey = "agent:main:telegram:cards:direct:42"; + const context = await suite.browser.newContext({ + locale: "en-US", + serviceWorkers: "block", + viewport: { height: 900, width: 1280 }, + }); + const page = await context.newPage(); + await installMockGateway(page, { + methodResponses: { + "sessions.list": sessionsListResponse([gatewayDirectRow(cardsKey, Date.now(), "cards")]), + }, + sessionKey: cardsKey, + }); + + try { + await page.goto(`${suite.server.baseUrl}chat`); + const row = page.locator(`[data-session-key="${cardsKey}"]`); + await row.waitFor({ state: "visible", timeout: 10_000 }); + // The row itself carries the account discriminator, so a rename field that + // echoed the rendered name would look plausible while persisting it. + await expect + .poll(() => trimmedTextContents(row.locator(".sidebar-recent-session__name"))) + .toEqual(["Alice · cards"]); + + await row.hover(); + await row.getByRole("button", { name: "Open session menu" }).click(); + await page.getByRole("menuitem", { name: "Rename…" }).click(); + const field = page + .locator('openclaw-modal-dialog[label="Rename session"]') + .getByRole("textbox", { name: "Rename session" }); + await field.waitFor({ state: "visible" }); + // This row has no stored label, so the field starts empty. Submitting the + // decorated name here is what used to freeze it into persisted state. + expect(await field.inputValue()).toBe(""); + } finally { + await context.close(); + } + }); +}); diff --git a/ui/src/lib/session-display.test.ts b/ui/src/lib/session-display.test.ts index 6d4add544715..ede7c0a416f3 100644 --- a/ui/src/lib/session-display.test.ts +++ b/ui/src/lib/session-display.test.ts @@ -38,6 +38,96 @@ describe("resolveSessionDisplayName", () => { expect(resolveSessionDisplayName("agent:main:imessage:direct:+4912")).toBe("iMessage · +4912"); }); + // Rows are shaped like the Gateway projection: displayName plus the + // accountId it derives from the canonical route, no user label. + it.each([ + { + name: "an account-less direct row keeps its plain name", + key: "agent:main:telegram:direct:42", + row: { displayName: "Alice" }, + expected: "Alice", + }, + { + name: "an account-qualified direct row names its account", + key: "agent:main:telegram:cards:direct:42", + row: { accountId: "cards", displayName: "Alice" }, + expected: "Alice · cards", + }, + { + name: "a shipped dm-spelled row names its account", + key: "agent:main:telegram:cards:dm:42", + row: { accountId: "cards", displayName: "Alice" }, + expected: "Alice · cards", + }, + { + name: "an unnamed shipped dm row reads as a friendly peer plus account", + key: "agent:main:telegram:cards:dm:491234567890", + row: { accountId: "cards" }, + expected: "Telegram · …567890 · cards", + }, + { + name: "the default account adds no discriminator", + key: "agent:main:telegram:default:direct:42", + row: { accountId: "default", displayName: "Alice" }, + expected: "Alice", + }, + { + name: "a human label that merely looks account-shaped still gets a discriminator", + key: "agent:main:telegram:work:direct:42", + row: { accountId: "work", label: "Alice (work)" }, + expected: "Alice (work) · work", + }, + { + name: "a stored label that already ends in the account suffix is left alone", + key: "agent:main:telegram:cards:direct:42", + row: { accountId: "cards", label: "Alice · cards" }, + expected: "Alice · cards", + }, + { + name: "a canonical group key is unchanged", + key: "agent:main:telegram:group:-1001234567890", + row: undefined, + expected: "Telegram Group", + }, + { + name: "an account-looking group key is not read as an account-qualified group", + key: "agent:main:dm:account:group:room", + row: undefined, + expected: "dm:account:group:room", + }, + ])("$name", ({ key, row, expected }) => { + expect(resolveSessionDisplayName(key, row)).toBe(expected); + }); + + it("reads the account off the key only until the gateway row arrives", () => { + expect(resolveSessionDisplayName("agent:main:telegram:cards:direct:42")).toBe( + "Telegram · 42 · cards", + ); + expect(resolveSessionDisplayName("agent:main:signal:work:dm:+4912")).toBe( + "Signal · +4912 · work", + ); + expect(resolveSessionDisplayName("agent:main:telegram:default:direct:42")).toBe( + "Telegram · 42", + ); + }); + + it("takes the account from the gateway row, not the key", () => { + // Only the projection carries account identity here; the key has none. + expect( + resolveSessionDisplayName("agent:main:telegram:direct:42", { + accountId: "cards", + displayName: "Alice", + }), + ).toBe("Alice · cards"); + // When the two disagree, the route the Gateway parsed wins over the guess. + expect( + resolveSessionDisplayName("agent:main:telegram:cards:direct:42", { + accountId: "ops", + displayName: "Alice", + }), + ).toBe("Alice · ops"); + }); + it("does not split UTF-16 surrogate pairs when shortening peer ids", () => { expect(resolveSessionDisplayName("agent:main:telegram:direct:12345😀67890")).toBe( "Telegram · …67890", @@ -177,10 +267,30 @@ describe("resolveChannelSessionInfo", () => { channel: "telegram", channelSession: true, }); - expect(resolveChannelSessionInfo("agent:main:slack:acct-1:channel:C1")).toEqual({ + expect(resolveChannelSessionInfo("agent:main:slack:channel:C1")).toEqual({ channel: "slack", channelSession: true, }); + // Shipped pre-#11881 keys spell direct chats `dm`; they are still channel sessions. + expect(resolveChannelSessionInfo("agent:main:telegram:cards:dm:42")).toEqual({ + channel: "telegram", + channelSession: true, + }); + expect(resolveChannelSessionInfo("agent:main:dm:+123", "whatsapp")).toEqual({ + channel: "whatsapp", + channelSession: true, + }); + // Accounts qualify direct chats only, so these key shapes name no channel + // and must not be filed under one the canonical parser would reject. + expect(resolveChannelSessionInfo("agent:main:telegram:work:group:room")).toEqual({ + channelSession: false, + }); + expect(resolveChannelSessionInfo("agent:main:slack:acct-1:channel:C1")).toEqual({ + channelSession: false, + }); + expect(resolveChannelSessionInfo("agent:main:dm:account:group:room")).toEqual({ + channelSession: false, + }); // dmScope per-peer keys have no channel segment; the row channel wins. expect(resolveChannelSessionInfo("agent:main:direct:+123", "whatsapp")).toEqual({ channel: "whatsapp", diff --git a/ui/src/lib/session-display.ts b/ui/src/lib/session-display.ts index e344944016f6..fd2a4a05188a 100644 --- a/ui/src/lib/session-display.ts +++ b/ui/src/lib/session-display.ts @@ -37,8 +37,14 @@ function shortenOpaqueIdRuns(text: string): string { const WORKTREE_BRANCH_PREFIX = "openclaw/"; -const CHANNEL_SESSION_KEY_RE = /^agent:[^:]+:([^:]+)(?::[^:]+)?:(?:direct|group|channel|thread):/; -const PEER_SESSION_KEY_RE = /:(?:direct|group|channel|thread):/; +// `dm` is the pre-#11881 spelling of `direct`; those keys still persist and the +// canonical parser still accepts both (src/sessions/session-key-utils.ts). +// Only direct chats take an account segment (src/routing/session-key.ts), so an +// account-qualified group or channel key is not a shape any producer emits and +// must not name a channel here either. +const CHANNEL_SESSION_KEY_RE = + /^agent:[^:]+:([^:]+)(?:(?::[^:]+)?:(?:direct|dm)|:(?:group|channel|thread)):/; +const PEER_SESSION_KEY_RE = /:(?:direct|dm|group|channel|thread):/; /** * Classifies channel-originated sessions for the sidebar's built-in channel @@ -53,9 +59,7 @@ export function resolveChannelSessionInfo( return { channelSession: false }; } const keyChannel = key.match(CHANNEL_SESSION_KEY_RE)?.[1]; - const channel = - normalizeOptionalString(keyChannel && keyChannel !== "direct" ? keyChannel : undefined) ?? - normalizeOptionalString(rowChannel); + const channel = normalizeOptionalString(keyChannel) ?? normalizeOptionalString(rowChannel); return { channel, channelSession: Boolean(channel) }; } @@ -99,8 +103,28 @@ type SessionKeyInfo = { prefix: string; /** Human-readable fallback when no label / displayName is available. */ fallbackName: string; + /** Raw account segment; only a fallback, Gateway rows carry the real one. */ + accountId?: string; }; +/** + * Two DMs from different accounts routinely share a name, so the account is the + * only discriminator; `default` is what key builders write for absence and says + * nothing. Which account to show comes from the recorded fact alone, never from + * the rendered name. The suffix check is idempotence, not inference: the chat + * pane's inline rename seeds its input with the rendered title + * (`beginHeaderRename`), so a partially edited submit can persist a label that + * already ends in this suffix, and appending twice would render + * `Alice · cards · cards`. + */ +function withAccountDisambiguator(name: string, accountId: string | undefined): string { + if (!accountId || accountId === "default") { + return name; + } + const suffix = ` · ${accountId}`; + return name.endsWith(suffix) ? name : `${name}${suffix}`; +} + /** Typed-session prefixes come from the i18n catalog (RFC 0026). */ function typedSessionPrefix(kind: SessionTypedKind): string { return kind === "subagent" @@ -112,6 +136,8 @@ type SessionDisplayRow = { label?: string; displayName?: string; derivedTitle?: string; + /** Canonical account projected from the delivery route by the Gateway. */ + accountId?: string; } & SessionWorktreeDisplayRow; type SessionDisplayOptions = { @@ -147,20 +173,28 @@ function parseSessionKey(key: string): SessionKeyInfo { return { kind: "automation", prefix, fallbackName: prefix }; } - // Direct chat: agent:::direct:. Never render the raw peer - // id; the gateway sends origin-derived names, so this is a last resort. - const directMatch = key.match(/^agent:[^:]+:([^:]+):direct:(.+)$/); + // Direct chat: agent::[:]:(direct|dm):. Never render + // the raw peer id; the gateway sends origin-derived names, so this is a last + // resort. + const directMatch = key.match(/^agent:[^:]+:([^:]+)(?::([^:]+))?:(?:direct|dm):(.+)$/); if (directMatch) { const channel = directMatch[1]; - const identifier = directMatch[2]; + const accountId = directMatch[2]; + const identifier = directMatch[3]; if (!channel || !identifier) { - return { prefix: "", fallbackName: key }; + return { prefix: "", fallbackName: key, accountId }; } const channelLabel = CHANNEL_LABELS[channel] ?? capitalize(channel); - return { prefix: "", fallbackName: `${channelLabel} · ${shortenPeerId(identifier)}` }; + return { + prefix: "", + fallbackName: `${channelLabel} · ${shortenPeerId(identifier)}`, + accountId, + }; } - // Group chat: agent:::group:. + // Group chat: agent:::group:. buildAgentPeerSessionKey scopes + // accounts to DMs (src/routing/session-key.ts), so an account-looking segment + // here belongs to a custom key and must not be read as one. const groupMatch = key.match(/^agent:[^:]+:([^:]+):group:(.+)$/); if (groupMatch) { const channel = groupMatch[1]; @@ -206,7 +240,10 @@ export function resolveSessionDisplayName( const label = normalizeOptionalString(row?.label) ?? ""; const displayName = normalizeOptionalString(row?.displayName) ?? ""; const derivedTitle = normalizeOptionalString(row?.derivedTitle) ?? ""; - const { kind, prefix, fallbackName } = parseSessionKey(key); + const { kind, prefix, fallbackName, accountId: keyAccountId } = parseSessionKey(key); + // The Gateway records the account on the row (src/gateway/session-classification.ts); + // the key is parsed only for panes rendered before their row arrives. + const accountId = normalizeOptionalString(row?.accountId) ?? keyAccountId; const applyTypedPrefix = (rawName: string): string => { if (!kind || !prefix) { @@ -226,21 +263,25 @@ export function resolveSessionDisplayName( return prefixPattern.test(name) ? name : `${prefix} ${name}`; }; - if (label && label !== key) { - return applyTypedPrefix(label); - } - if (displayName && displayName !== key) { - return applyTypedPrefix(displayName); - } - // Unnamed work sessions read as their checkout instead of an opaque key. - const workSubtitle = row ? resolveSessionWorkSubtitle(row) : undefined; - if (workSubtitle && row?.worktree) { - return applyTypedPrefix(workSubtitle); - } - if (derivedTitle && derivedTitle !== key) { - return applyTypedPrefix(derivedTitle); - } - return fallbackName; + const resolveNamedOrFallback = (): string => { + if (label && label !== key) { + return applyTypedPrefix(label); + } + if (displayName && displayName !== key) { + return applyTypedPrefix(displayName); + } + // Unnamed work sessions read as their checkout instead of an opaque key. + const workSubtitle = row ? resolveSessionWorkSubtitle(row) : undefined; + if (workSubtitle && row?.worktree) { + return applyTypedPrefix(workSubtitle); + } + if (derivedTitle && derivedTitle !== key) { + return applyTypedPrefix(derivedTitle); + } + return fallbackName; + }; + + return withAccountDisambiguator(resolveNamedOrFallback(), accountId); } export function isCronSessionKey(key: string): boolean {