From c715d89de81efefa92f9220a64f19fc2412ddf7f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 20 Jul 2026 10:04:41 -0700 Subject: [PATCH] fix(ui): show newly created PRs in session sidebar (#111783) * fix(ui): surface newly created session PRs * test(ui): split sidebar PR state coverage --- src/gateway/control-ui-session-prs.test.ts | 134 ++++++++++++++- src/gateway/control-ui-session-prs.ts | 61 ++++++- .../app-sidebar-session-narration.test.ts | 1 + .../app-sidebar-session-navigation.ts | 1 + .../components/app-sidebar-session-types.ts | 1 + ui/src/components/app-sidebar.test.ts | 1 + ui/src/components/github-link-hovercard.ts | 4 + ui/src/components/session-row-badges.test.ts | 20 ++- ui/src/components/session-row-badges.ts | 12 +- ui/src/lib/sessions/index.test.ts | 4 +- ui/src/lib/sessions/index.ts | 58 +++++++ .../lib/sessions/pull-request-state.test.ts | 114 ++++++++++++ ui/src/pages/chat/chat-pane.test-support.ts | 11 ++ ui/src/pages/chat/chat-pane.test.ts | 113 ++++++++++++ ui/src/pages/chat/chat-pane.ts | 23 ++- ui/src/pages/chat/chat-state.test.ts | 162 ++++++++++++++++++ ui/src/pages/chat/chat-state.ts | 44 +++++ ui/src/styles/components.css | 1 + .../app-sidebar-cases/pull-request-state.ts | 27 +++ ui/src/test-helpers/app-sidebar.ts | 12 ++ 20 files changed, 786 insertions(+), 18 deletions(-) create mode 100644 ui/src/lib/sessions/pull-request-state.test.ts create mode 100644 ui/src/test-helpers/app-sidebar-cases/pull-request-state.ts diff --git a/src/gateway/control-ui-session-prs.test.ts b/src/gateway/control-ui-session-prs.test.ts index 2097b74bc17..de8a0dafd80 100644 --- a/src/gateway/control-ui-session-prs.test.ts +++ b/src/gateway/control-ui-session-prs.test.ts @@ -29,7 +29,9 @@ function requestUrl(input: RequestInfo | URL | undefined): string { return input?.url ?? ""; } -function routedFetch(routes: Array<{ match: string; response: () => Response }>) { +function routedFetch( + routes: Array<{ match: string; response: () => Response | Promise }>, +) { return vi.fn(async (input: RequestInfo | URL) => { const url = requestUrl(input); const route = routes.find((candidate) => url.includes(candidate.match)); @@ -117,6 +119,21 @@ describe("parseControlUiSessionPullRequestsParams", () => { { sessionKey: "global" }, ); }); + + it("accepts only an explicit refresh request", () => { + expect( + parseControlUiSessionPullRequestsParams({ + sessionKey: "agent:main:main", + refresh: true, + }), + ).toEqual({ sessionKey: "agent:main:main", refresh: true }); + expect( + parseControlUiSessionPullRequestsParams({ + sessionKey: "agent:main:main", + refresh: false, + }), + ).toEqual({ sessionKey: "agent:main:main" }); + }); }); describe("loadControlUiSessionPullRequests", () => { @@ -332,6 +349,22 @@ describe("loadControlUiSessionPullRequests", () => { ); expect(stale.rateLimited).toBe(true); expect(stale.pullRequests).toEqual(fresh.pullRequests); + + const callsDuringBackoff = fetchImpl.mock.calls.length; + const explicitRefresh = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main", refresh: true }, + { fetchImpl, resolveGitContext }, + ); + expect(explicitRefresh).toEqual(stale); + expect(fetchImpl.mock.calls).toHaveLength(callsDuringBackoff); + + vi.advanceTimersByTime(61_000); + const stillBackedOff = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { fetchImpl, resolveGitContext }, + ); + expect(stillBackedOff).toEqual(stale); + expect(fetchImpl.mock.calls).toHaveLength(callsDuringBackoff); }); it("degrades permission 403s on optional fetches to chips without checks", async () => { @@ -366,6 +399,105 @@ describe("loadControlUiSessionPullRequests", () => { expect(fetchImpl.mock.calls).toHaveLength(0); }); + it("refreshes a cached empty result after the assistant creates a PR", async () => { + let pulls: Record[] = []; + const fetchImpl = routedFetch([ + { match: "/pulls?head=", response: () => githubJson(pulls) }, + { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, + ]); + + const initial = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { fetchImpl, resolveGitContext }, + ); + expect(initial.pullRequests).toEqual([]); + + pulls = [pullListItem({ merged_at: "2026-07-09T10:00:00Z" })]; + const cached = await loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { fetchImpl, resolveGitContext }, + ); + expect(cached.pullRequests).toEqual([]); + + const forcedRefresh = loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main", refresh: true }, + { fetchImpl, resolveGitContext }, + ); + const ordinaryFollower = loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { fetchImpl, resolveGitContext }, + ); + const duplicateForcedRefresh = loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main", refresh: true }, + { fetchImpl, resolveGitContext }, + ); + const refreshed = await Promise.all([forcedRefresh, ordinaryFollower, duplicateForcedRefresh]); + expect(refreshed.map((result) => result.pullRequests.map((item) => item.number))).toEqual([ + [103469], + [103469], + [103469], + ]); + expect( + fetchImpl.mock.calls.filter((call) => + requestUrl(call[0] as RequestInfo | URL).includes("/pulls?head="), + ), + ).toHaveLength(2); + }); + + it("queues one forced refresh behind an ordinary in-flight lookup", async () => { + let resolveInitialPulls!: (response: Response) => void; + let signalInitialPullStarted!: () => void; + const initialPulls = new Promise((resolve) => { + resolveInitialPulls = resolve; + }); + const initialPullStarted = new Promise((resolve) => { + signalInitialPullStarted = resolve; + }); + let pullListCalls = 0; + const fetchImpl = routedFetch([ + { + match: "/pulls?head=", + response: () => { + pullListCalls += 1; + if (pullListCalls === 1) { + signalInitialPullStarted(); + return initialPulls; + } + return githubJson([pullListItem({ merged_at: "2026-07-09T10:00:00Z" })]); + }, + }, + { match: "/repos/openclaw/openclaw", response: () => githubJson({ fork: false }) }, + ]); + + const initial = loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { fetchImpl, resolveGitContext }, + ); + await initialPullStarted; + const forcedRefresh = loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main", refresh: true }, + { fetchImpl, resolveGitContext }, + ); + await Promise.resolve(); + const ordinaryFollower = loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main" }, + { fetchImpl, resolveGitContext }, + ); + const duplicateForcedRefresh = loadControlUiSessionPullRequests( + { sessionKey: "agent:main:main", refresh: true }, + { fetchImpl, resolveGitContext }, + ); + + resolveInitialPulls(githubJson([])); + expect((await initial).pullRequests).toEqual([]); + expect( + (await Promise.all([forcedRefresh, ordinaryFollower, duplicateForcedRefresh])).map((result) => + result.pullRequests.map((item) => item.number), + ), + ).toEqual([[103469], [103469], [103469]]); + expect(pullListCalls).toBe(2); + }); + it("keeps branch metadata when the very first GitHub fetch is rate limited", async () => { // The pre-PR row's rate-limit warning depends on this: with no cached // chips, the local-git branch payload is all the UI has left to render. diff --git a/src/gateway/control-ui-session-prs.ts b/src/gateway/control-ui-session-prs.ts index 377b9e040fd..262bb2cc6da 100644 --- a/src/gateway/control-ui-session-prs.ts +++ b/src/gateway/control-ui-session-prs.ts @@ -34,6 +34,7 @@ const MAX_PULL_REQUESTS = 3; export type ControlUiSessionPullRequestsParams = { sessionKey: string; agentId?: string; + refresh?: boolean; }; /** GitHub repo + branch resolved from a session's git checkout. */ @@ -60,6 +61,7 @@ type PullListItem = { type CacheEntry = { expiresAt: number; promise: Promise; + refreshMode: "normal" | "forced" | null; // Survives refetch failures so rate-limited refreshes degrade to stale // chips instead of clearing the row. lastGood?: ControlUiSessionPullRequest[]; @@ -78,7 +80,11 @@ export function parseControlUiSessionPullRequestsParams( return null; } const agentId = typeof value.agentId === "string" ? value.agentId.trim() : ""; - return agentId ? { sessionKey, agentId } : { sessionKey }; + return { + sessionKey, + ...(agentId ? { agentId } : {}), + ...(value.refresh === true ? { refresh: true } : {}), + }; } async function gitOutput(cwd: string, args: string[]): Promise { @@ -579,30 +585,67 @@ export async function loadControlUiSessionPullRequests( // alive when GitHub is rate limited; only the GitHub fetch is cached. const [branch, snapshot] = await Promise.all([ resolveSessionBranch(context), - cachedBranchPullRequests(context, deps), + cachedBranchPullRequests(context, deps, params.refresh === true), ]); return branch ? { ...snapshot, branch } : snapshot; } -function cachedBranchPullRequests( +function trackBranchRefresh( + entry: CacheEntry, + mode: "normal" | "forced", + load: () => Promise, +): Promise { + // Publish the replacement promise before any awaited work so later callers + // cannot overtake a queued forced refresh with an older normal result. + entry.expiresAt = Date.now() + SUCCESS_CACHE_MS; + entry.refreshMode = mode; + const refreshPromise = load(); + const trackedPromise = refreshPromise.finally(() => { + if (entry.promise === trackedPromise) { + entry.refreshMode = null; + } + }); + entry.promise = trackedPromise; + return trackedPromise; +} + +async function cachedBranchPullRequests( context: SessionPullRequestGitContext, deps: LoadSessionPullRequestDeps, + refresh: boolean, ): Promise { const key = `${context.owner.toLowerCase()}/${context.repo.toLowerCase()}#${context.branch}`; const cached = branchCache.get(key); if (cached && cached.expiresAt > Date.now()) { branchCache.delete(key); branchCache.set(key, cached); - return cached.promise; + if (!refresh || cached.refreshMode === "forced") { + return cached.promise; + } + const pendingSnapshot = cached.promise; + const pendingRefreshMode = cached.refreshMode; + const pendingExpiresAt = cached.expiresAt; + return trackBranchRefresh(cached, "forced", async () => { + const snapshot = await pendingSnapshot; + // GitHub quota backoff stays authoritative even when a PR announcement + // queues this lookup behind an older normal or settled request. + if (snapshot.rateLimited) { + if (pendingRefreshMode === null) { + cached.expiresAt = pendingExpiresAt; + } + return snapshot; + } + return refreshBranchPullRequests(context, deps.fetchImpl ?? fetch, cached); + }); } const entry: CacheEntry = cached ?? { expiresAt: 0, promise: Promise.resolve({ pullRequests: [], rateLimited: false }), + refreshMode: null, }; - // Optimistic expiry dedupes concurrent panes while the refresh is in - // flight; failures shorten it inside refreshBranchPullRequests. - entry.expiresAt = Date.now() + SUCCESS_CACHE_MS; - entry.promise = refreshBranchPullRequests(context, deps.fetchImpl ?? fetch, entry); + const promise = trackBranchRefresh(entry, refresh ? "forced" : "normal", () => + refreshBranchPullRequests(context, deps.fetchImpl ?? fetch, entry), + ); branchCache.delete(key); branchCache.set(key, entry); while (branchCache.size > CACHE_LIMIT) { @@ -612,5 +655,5 @@ function cachedBranchPullRequests( } branchCache.delete(oldestKey); } - return entry.promise; + return promise; } diff --git a/ui/src/components/app-sidebar-session-narration.test.ts b/ui/src/components/app-sidebar-session-narration.test.ts index cdf927afc00..60e6717177b 100644 --- a/ui/src/components/app-sidebar-session-narration.test.ts +++ b/ui/src/components/app-sidebar-session-narration.test.ts @@ -22,6 +22,7 @@ function runningRow(key: string): SidebarRecentSession { pinned: false, cloudWorkerActive: false, hasAutomation: false, + hasOpenPullRequest: false, unread: false, attention: { kind: "none" }, startedAt: 1, diff --git a/ui/src/components/app-sidebar-session-navigation.ts b/ui/src/components/app-sidebar-session-navigation.ts index 571c30d7628..6bc16cba703 100644 --- a/ui/src/components/app-sidebar-session-navigation.ts +++ b/ui/src/components/app-sidebar-session-navigation.ts @@ -165,6 +165,7 @@ export abstract class AppSidebarSessionNavigationElement extends AppSidebarSessi placementState: row.placement?.state, cloudWorkerActive: isStoppableCloudWorkerPlacement(row.placement), hasAutomation: row.hasAutomation === true, + hasOpenPullRequest: context?.sessions.hasOpenPullRequest?.(row.key) === true, unread: row.unread === true, attention: this.resolveSessionAttention(row), agentStatusNote: this.resolveSessionAgentStatus(row)?.note, diff --git a/ui/src/components/app-sidebar-session-types.ts b/ui/src/components/app-sidebar-session-types.ts index f360209f7d1..c0cf802f637 100644 --- a/ui/src/components/app-sidebar-session-types.ts +++ b/ui/src/components/app-sidebar-session-types.ts @@ -70,6 +70,7 @@ export type SidebarRecentSession = { placementState?: SessionPlacementState; cloudWorkerActive: boolean; hasAutomation: boolean; + hasOpenPullRequest: boolean; unread: boolean; attention: SidebarSessionAttention; agentStatusNote?: string; diff --git a/ui/src/components/app-sidebar.test.ts b/ui/src/components/app-sidebar.test.ts index 235fc6770b6..5051edd94bb 100644 --- a/ui/src/components/app-sidebar.test.ts +++ b/ui/src/components/app-sidebar.test.ts @@ -14,6 +14,7 @@ import "../test-helpers/app-sidebar-cases/child-sessions.ts"; import "../test-helpers/app-sidebar-cases/group-mutations.ts"; import "../test-helpers/app-sidebar-cases/interactions.ts"; import "../test-helpers/app-sidebar-cases/narration.ts"; +import "../test-helpers/app-sidebar-cases/pull-request-state.ts"; import "../test-helpers/app-sidebar-cases/sidebar-scroll.ts"; import "../test-helpers/app-sidebar-cases/sessions.ts"; import "../test-helpers/app-sidebar-cases/session-list-sections.ts"; diff --git a/ui/src/components/github-link-hovercard.ts b/ui/src/components/github-link-hovercard.ts index 89055b5f210..f4d030e40e4 100644 --- a/ui/src/components/github-link-hovercard.ts +++ b/ui/src/components/github-link-hovercard.ts @@ -91,6 +91,10 @@ function parseGitHubIssueOrPullRequestLink(href: string): GitHubLinkTarget | nul return { href: url.href, kind, number: Number(numberText), owner, repo }; } +export function isGitHubPullRequestLink(href: string): boolean { + return parseGitHubIssueOrPullRequestLink(href)?.kind === "pull"; +} + function safeAvatarDataUrl(value: unknown): string | undefined { return typeof value === "string" && /^data:image\/(?:gif|jpeg|png|webp);base64,/u.test(value) ? value diff --git a/ui/src/components/session-row-badges.test.ts b/ui/src/components/session-row-badges.test.ts index a54d99652ce..efda16a46a9 100644 --- a/ui/src/components/session-row-badges.test.ts +++ b/ui/src/components/session-row-badges.test.ts @@ -69,6 +69,20 @@ describe("session row placement badges", () => { expect(container.querySelector(".session-row-badge--cloud")).toBeNull(); }); + it("renders a green open-pull-request indicator", () => { + render( + renderSessionRowBadges({ + hasAutomation: false, + hasOpenPullRequest: true, + }), + container, + ); + + const badge = container.querySelector(".session-row-badge--pull-request"); + expect(badge?.getAttribute("aria-label")).toBe("Open PR"); + expect(badge?.querySelector("svg")).not.toBeNull(); + }); + it("renders a warning-colored approval-needed indicator", () => { render( renderSessionRowBadges({ @@ -83,18 +97,20 @@ describe("session row placement badges", () => { expect(badge?.querySelector("svg")).not.toBeNull(); }); - it("keeps child-only automation and placement badges hidden while showing approval", () => { + it("keeps child-only automation and placement badges hidden while showing PR and approval", () => { render( renderSessionRowBadges({ isChild: true, hasAutomation: true, + hasOpenPullRequest: true, hasApproval: true, placementState: "active", }), container, ); - expect(container.querySelectorAll(".session-row-badge")).toHaveLength(1); + expect(container.querySelectorAll(".session-row-badge")).toHaveLength(2); + expect(container.querySelector(".session-row-badge--pull-request")).not.toBeNull(); expect(container.querySelector(".session-row-badge--approval")).not.toBeNull(); expect(container.querySelector(".session-row-badge--cloud")).toBeNull(); }); diff --git a/ui/src/components/session-row-badges.ts b/ui/src/components/session-row-badges.ts index bf7b4c7ee35..0ff76f7aefd 100644 --- a/ui/src/components/session-row-badges.ts +++ b/ui/src/components/session-row-badges.ts @@ -19,6 +19,7 @@ export function isStoppableCloudWorkerPlacement( export function renderSessionRowBadges(params: { isChild?: boolean; hasAutomation: boolean; + hasOpenPullRequest?: boolean; hasApproval?: boolean; placementState?: SessionPlacementState; }) { @@ -27,7 +28,7 @@ export function renderSessionRowBadges(params: { const cloudPlacementState = isCloudWorkerPlacementState(placementState) ? placementState : undefined; - if (!hasAutomation && !params.hasApproval && !cloudPlacementState) { + if (!hasAutomation && !params.hasOpenPullRequest && !params.hasApproval && !cloudPlacementState) { return nothing; } const cloudLabel = cloudPlacementState @@ -43,6 +44,15 @@ export function renderSessionRowBadges(params: { >${icons.clock}` : nothing} + ${params.hasOpenPullRequest + ? html`${icons.gitPullRequest}` + : nothing} ${params.hasApproval ? html` { - snapshot = { ...snapshot, connected }; + publish: (connected: boolean, nextClient: GatewayBrowserClient | null = snapshot.client) => { + snapshot = { ...snapshot, client: nextClient, connected }; for (const listener of listeners) { listener(snapshot); } diff --git a/ui/src/lib/sessions/index.ts b/ui/src/lib/sessions/index.ts index e14036c0e90..c50abb01b63 100644 --- a/ui/src/lib/sessions/index.ts +++ b/ui/src/lib/sessions/index.ts @@ -198,6 +198,9 @@ export type SessionCapability = { create: (params?: SessionCreateParams) => Promise; patch: SessionPatchRoute; setModelOverride: (key: string, value: string | null | undefined) => void; + hasOpenPullRequest: (key: string) => boolean; + captureOpenPullRequestEpoch: (key: string) => symbol; + setOpenPullRequest: (key: string, hasOpenPullRequest: boolean, epoch?: symbol) => void; delete: (key: string, options?: SessionDeleteOptions) => Promise; deleteMany: (targets: readonly SessionDeleteTarget[]) => Promise; reset: (key: string, options?: SessionResetOptions) => Promise; @@ -725,6 +728,8 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil { token: symbol; previous: string | null | undefined } >(); const swarmActivity = new SwarmActivityTracker(); + const openPullRequestSessionKeys = new Set(); + const openPullRequestEpochs = new Map(); let subscribedClient: GatewayBrowserClient | null = null; let lastListOptions: SessionListOptions = {}; let hasForegroundListOptions = false; @@ -767,6 +772,38 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil } }; + const hasOpenPullRequest = (key: string): boolean => openPullRequestSessionKeys.has(key.trim()); + + const captureOpenPullRequestEpoch = (key: string): symbol => { + const normalizedKey = key.trim(); + const epoch = Symbol(normalizedKey); + openPullRequestEpochs.set(normalizedKey, epoch); + return epoch; + }; + + const retireOpenPullRequest = (key: string) => { + const normalizedKey = key.trim(); + openPullRequestEpochs.delete(normalizedKey); + openPullRequestSessionKeys.delete(normalizedKey); + }; + + const setOpenPullRequest = (key: string, open: boolean, epoch?: symbol) => { + const normalizedKey = key.trim(); + if ( + !normalizedKey || + (epoch !== undefined && openPullRequestEpochs.get(normalizedKey) !== epoch) || + openPullRequestSessionKeys.has(normalizedKey) === open + ) { + return; + } + if (open) { + openPullRequestSessionKeys.add(normalizedKey); + } else { + openPullRequestSessionKeys.delete(normalizedKey); + } + publish({ ...state }); + }; + const setModelOverride = (key: string, value: string | null | undefined) => { const normalizedKey = key.trim(); if (!normalizedKey) { @@ -1254,6 +1291,9 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil result, row: base.row ? result?.sessions.find((row) => row.key === base.row?.key) : undefined, }; + if (reconciled.deletedKey) { + retireOpenPullRequest(reconciled.deletedKey); + } if (reconciled.applied && (reconciled.result !== state.result || reconciled.deletedKey)) { publish({ ...state, @@ -1295,6 +1335,7 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil if (!confirmsSessionDeletion(response)) { return { deleted: false }; } + retireOpenPullRequest(key); publish({ ...state, deletedSessions: [{ key, agentId: options.agentId }] }); setModelOverride(key, undefined); await refreshReplacement(options.agentId); @@ -1342,6 +1383,9 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil } } if (deleted.length > 0 && isCurrentConnection(scope)) { + for (const key of deleted) { + retireOpenPullRequest(key); + } publish({ ...state, deletedSessions: targets.filter((target) => deleted.includes(target.key)), @@ -1590,12 +1634,20 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil connectionClient = next.client; connectionConnected = next.connected; if (connectionChanged) { + const hadOpenPullRequests = openPullRequestSessionKeys.size > 0; connectionEpoch += 1; invalidateGroupsLoad(); swarmActivity.clear(); inFlight = null; queuedRefresh = null; rollbackPendingModelPatches(); + openPullRequestSessionKeys.clear(); + openPullRequestEpochs.clear(); + // A connected client replacement needs its own invalidation publish; + // disconnects publish the cleared state in the branch immediately below. + if (hadOpenPullRequests && next.connected && next.client) { + publish({ ...state }); + } } if (!next.connected || !next.client) { subscribedClient = null; @@ -1669,6 +1721,7 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil return; } if (reconciled.deletedKey) { + retireOpenPullRequest(reconciled.deletedKey); // Preserve remote-deletion navigation before the canonical refresh // clears transient event state. publish({ @@ -1701,6 +1754,9 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil create, patch, setModelOverride, + hasOpenPullRequest, + captureOpenPullRequestEpoch, + setOpenPullRequest, delete: remove, deleteMany: removeMany, reset, @@ -1740,6 +1796,8 @@ export function createSessionCapability(gateway: SessionGateway): SessionCapabil subscribedClient = null; pendingModelPatches.clear(); swarmActivity.clear(); + openPullRequestSessionKeys.clear(); + openPullRequestEpochs.clear(); stopGateway(); stopEvents(); createdListeners.clear(); diff --git a/ui/src/lib/sessions/pull-request-state.test.ts b/ui/src/lib/sessions/pull-request-state.test.ts new file mode 100644 index 00000000000..21027576012 --- /dev/null +++ b/ui/src/lib/sessions/pull-request-state.test.ts @@ -0,0 +1,114 @@ +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"; + +function sessionsResult(sessions: SessionsListResult["sessions"]): SessionsListResult { + return { + ts: 2, + path: "(multiple)", + count: sessions.length, + defaults: { modelProvider: null, model: null, contextTokens: null }, + sessions, + }; +} + +function createGatewayHarness(client: GatewayBrowserClient) { + let snapshot = { + client: client as GatewayBrowserClient | null, + connected: true, + sessionKey: "agent:main:main", + assistantAgentId: "main", + hello: null, + }; + const listeners = new Set<(next: typeof snapshot) => void>(); + return { + gateway: { + get snapshot() { + return snapshot; + }, + subscribe(listener: (next: typeof snapshot) => void) { + listeners.add(listener); + return () => listeners.delete(listener); + }, + subscribeEvents(_listener: (event: GatewayEventFrame) => void) { + return () => undefined; + }, + }, + publish(connected: boolean, nextClient: GatewayBrowserClient | null = snapshot.client) { + snapshot = { ...snapshot, client: nextClient, connected }; + for (const listener of listeners) { + listener(snapshot); + } + }, + }; +} + +describe("session pull-request state", () => { + it("publishes state removal for client replacement and disconnect", () => { + const harness = createGatewayHarness({} as GatewayBrowserClient); + const sessions = createSessionCapability(harness.gateway); + const listener = vi.fn(); + sessions.subscribe(listener); + + sessions.setOpenPullRequest("agent:main:pr-session", true); + expect(sessions.hasOpenPullRequest("agent:main:pr-session")).toBe(true); + expect(listener).toHaveBeenCalledTimes(1); + + sessions.setOpenPullRequest("agent:main:pr-session", true); + expect(listener).toHaveBeenCalledTimes(1); + + const publicationsBeforeReplacement = listener.mock.calls.length; + harness.publish(true, {} as GatewayBrowserClient); + expect(sessions.hasOpenPullRequest("agent:main:pr-session")).toBe(false); + expect(listener.mock.calls.length).toBeGreaterThan(publicationsBeforeReplacement); + + sessions.setOpenPullRequest("agent:main:pr-session", true); + const publicationsBeforeDisconnect = listener.mock.calls.length; + harness.publish(false); + expect(sessions.hasOpenPullRequest("agent:main:pr-session")).toBe(false); + expect(listener.mock.calls.length).toBeGreaterThan(publicationsBeforeDisconnect); + + sessions.dispose(); + }); + + it("rejects an older pane's pull-request result", () => { + const sessions = createSessionCapability( + createGatewayHarness({} as GatewayBrowserClient).gateway, + ); + const key = "agent:main:shared-session"; + const olderEpoch = sessions.captureOpenPullRequestEpoch(key); + const newerEpoch = sessions.captureOpenPullRequestEpoch(key); + + sessions.setOpenPullRequest(key, true, newerEpoch); + sessions.setOpenPullRequest(key, false, olderEpoch); + + expect(sessions.hasOpenPullRequest(key)).toBe(true); + sessions.dispose(); + }); + + it("retires pull-request state when a session is deleted", async () => { + const key = "agent:main:deleted-pr"; + const request = vi.fn(async (method: string) => { + if (method === "sessions.delete") { + return { ok: true, deleted: true }; + } + if (method === "sessions.list") { + return sessionsResult([]); + } + throw new Error(`Unexpected request: ${method}`); + }); + const sessions = createSessionCapability( + createGatewayHarness({ request } as unknown as GatewayBrowserClient).gateway, + ); + const epoch = sessions.captureOpenPullRequestEpoch(key); + sessions.setOpenPullRequest(key, true, epoch); + + await expect(sessions.delete(key)).resolves.toEqual({ deleted: true }); + expect(sessions.hasOpenPullRequest(key)).toBe(false); + + sessions.setOpenPullRequest(key, true, epoch); + expect(sessions.hasOpenPullRequest(key)).toBe(false); + sessions.dispose(); + }); +}); diff --git a/ui/src/pages/chat/chat-pane.test-support.ts b/ui/src/pages/chat/chat-pane.test-support.ts index a419e9e4e4b..9b6a0dd24fb 100644 --- a/ui/src/pages/chat/chat-pane.test-support.ts +++ b/ui/src/pages/chat/chat-pane.test-support.ts @@ -6,6 +6,7 @@ import type { TaskSuggestion, TaskSuggestionEvent, } from "../../../../packages/gateway-protocol/src/index.js"; +import type { ControlUiSessionPullRequest } from "../../../../src/gateway/control-ui-contract.js"; import type { GatewayBrowserClient } from "../../api/gateway.ts"; import type { GatewaySessionRow } from "../../api/types.ts"; import type { ApplicationContext } from "../../app/context.ts"; @@ -34,6 +35,8 @@ export type TestChatPane = HTMLElement & { handleDocumentKeydown: (event: KeyboardEvent) => void; handleTaskSuggestionEvent: (event: TaskSuggestionEvent) => void; refreshTaskSuggestions: () => Promise; + refreshSessionPullRequests: (options?: { refresh?: boolean }) => Promise; + sessionPullRequests: ControlUiSessionPullRequest[]; taskSuggestions: TaskSuggestion[]; onPaneSessionChange?: (paneId: string, sessionKey: string) => void; sessionKey: string; @@ -96,6 +99,12 @@ export function createSessionContext( }, }, agents: { state: { agentsList: null } }, + config: { + current: { + assistantIdentity: { name: "Molty" }, + terminalEnabled: false, + }, + }, sessions, } as unknown as ApplicationContext; } @@ -137,6 +146,8 @@ export function createTestChatPane(params: { chatScrollGeneration: 0, chatScrollCommitCleanup: null, handleChatScroll: vi.fn(), + realtimeTalkInputLevel: { set: vi.fn() }, + resetToolStream: vi.fn(), renderLifecycle: { afterCommit: () => () => {}, invalidate: () => {} }, } as unknown as ChatPageHost; pane.context = createSessionContext(params.client, params.sessions); diff --git a/ui/src/pages/chat/chat-pane.test.ts b/ui/src/pages/chat/chat-pane.test.ts index 7c7609dc472..47bc7fbc07e 100644 --- a/ui/src/pages/chat/chat-pane.test.ts +++ b/ui/src/pages/chat/chat-pane.test.ts @@ -97,6 +97,119 @@ function nativeHistoryMessage(seq: number, text = `message ${seq}`) { }; } +describe("chat pane pull request refresh", () => { + it("forwards an explicit refresh and publishes live PR state", async () => { + const request = vi.fn().mockResolvedValue({ + pullRequests: [ + { + number: 111532, + owner: "openclaw", + repo: "openclaw", + branch: "claude/pr-detection", + title: "Detect pull requests", + url: "https://github.com/openclaw/openclaw/pull/111532", + state: "open", + }, + ], + rateLimited: false, + }); + const client = { request } as unknown as GatewayBrowserClient; + const epoch = Symbol("pr-refresh"); + const setOpenPullRequest = vi.fn(); + const sessions = { + captureOpenPullRequestEpoch: vi.fn(() => epoch), + setOpenPullRequest, + } as unknown as SessionCapability; + const { pane } = createTestChatPane({ client, sessions }); + pane.context.gateway.snapshot.hello = { + features: { methods: ["controlUi.sessionPullRequests"] }, + } as never; + + await pane.refreshSessionPullRequests({ refresh: true }); + + expect(request).toHaveBeenCalledWith( + "controlUi.sessionPullRequests", + expect.objectContaining({ sessionKey: "agent:main:current", refresh: true }), + ); + expect(setOpenPullRequest).toHaveBeenCalledWith("agent:main:current", true, epoch); + }); + + it("clears the pane snapshot when the Gateway source disconnects", () => { + const client = {} as GatewayBrowserClient; + const { pane } = createTestChatPane({ client, sessions: {} as SessionCapability }); + pane.sessionPullRequests = [ + { + number: 111532, + owner: "openclaw", + repo: "openclaw", + branch: "claude/pr-detection", + title: "Detect pull requests", + url: "https://github.com/openclaw/openclaw/pull/111532", + state: "open", + }, + ]; + + pane.applyGatewaySnapshot({ + ...pane.context.gateway.snapshot, + connected: false, + }); + + expect(pane.sessionPullRequests).toEqual([]); + }); + + it("preserves shared PR state for an empty rate-limited snapshot", async () => { + const request = vi.fn().mockResolvedValue({ pullRequests: [], rateLimited: true }); + const setOpenPullRequest = vi.fn(); + const { pane } = createTestChatPane({ + client: { request } as unknown as GatewayBrowserClient, + sessions: { + captureOpenPullRequestEpoch: vi.fn(() => Symbol("pr-refresh")), + setOpenPullRequest, + } as unknown as SessionCapability, + }); + pane.context.gateway.snapshot.hello = { + features: { methods: ["controlUi.sessionPullRequests"] }, + } as never; + + await pane.refreshSessionPullRequests(); + + expect(setOpenPullRequest).not.toHaveBeenCalled(); + }); + + it("clears shared live PR state after the PR settles", async () => { + const request = vi.fn().mockResolvedValue({ + pullRequests: [ + { + number: 111532, + owner: "openclaw", + repo: "openclaw", + branch: "claude/pr-detection", + title: "Detect pull requests", + url: "https://github.com/openclaw/openclaw/pull/111532", + state: "merged", + }, + ], + rateLimited: false, + }); + const epoch = Symbol("pr-refresh"); + const setOpenPullRequest = vi.fn(); + const { pane } = createTestChatPane({ + client: { request } as unknown as GatewayBrowserClient, + sessions: { + captureOpenPullRequestEpoch: vi.fn(() => epoch), + setOpenPullRequest, + } as unknown as SessionCapability, + }); + pane.context.gateway.snapshot.hello = { + features: { methods: ["controlUi.sessionPullRequests"] }, + } as never; + + await pane.refreshSessionPullRequests(); + + expect(setOpenPullRequest).toHaveBeenCalledWith("agent:main:current", false, epoch); + }); +}); + describe("chat pane header state", () => { it("commits a trimmed label and clears with null", async () => { const patch = vi.fn(async () => ({})); diff --git a/ui/src/pages/chat/chat-pane.ts b/ui/src/pages/chat/chat-pane.ts index fd8fe85afa2..363d9df153e 100644 --- a/ui/src/pages/chat/chat-pane.ts +++ b/ui/src/pages/chat/chat-pane.ts @@ -591,7 +591,7 @@ class ChatPane extends OpenClawLightDomElement { } } - private async refreshSessionPullRequests(): Promise { + private async refreshSessionPullRequests(options: { refresh?: boolean } = {}): Promise { const requestVersion = ++this.sessionPullRequestsRequestVersion; const scope = this.captureConnectionScope(); if ( @@ -612,10 +612,15 @@ class ChatPane extends OpenClawLightDomElement { this.requestUpdate(); return; } + const openPullRequestEpoch = scope.context.sessions.captureOpenPullRequestEpoch(sessionKey); try { const result = await scope.client.request( "controlUi.sessionPullRequests", - { sessionKey, ...scopedAgentParamsForSession(scope.state, sessionKey) }, + { + sessionKey, + ...scopedAgentParamsForSession(scope.state, sessionKey), + ...(options.refresh ? { refresh: true } : {}), + }, ); if ( requestVersion !== this.sessionPullRequestsRequestVersion || @@ -625,6 +630,13 @@ class ChatPane extends OpenClawLightDomElement { return; } this.sessionPullRequests = result.pullRequests; + if (!result.rateLimited || result.pullRequests.length > 0) { + scope.context.sessions.setOpenPullRequest( + sessionKey, + result.pullRequests.some((item) => item.state === "open" || item.state === "draft"), + openPullRequestEpoch, + ); + } this.sessionPullRequestsBranch = result.branch; this.sessionPullRequestsRateLimited = result.rateLimited; this.dismissedSessionPullRequestIds = listDismissedChatPullRequests(sessionKey); @@ -2127,6 +2139,7 @@ class ChatPane extends OpenClawLightDomElement { await refreshPageChat(pageState); pageState.requestUpdate?.(); }; + pageState.refreshSessionPullRequests = (options) => this.refreshSessionPullRequests(options); this.state = pageState; if (this.sessionKey) { const initialSessionKey = this.setPaneSessionKey(this.sessionKey); @@ -2390,6 +2403,7 @@ class ChatPane extends OpenClawLightDomElement { this.taskSuggestionBusyIds.clear(); this.taskSuggestionOperations.clear(); this.sessionDiscussionStates.clear(); + this.resetSessionPullRequests(); this.resetOlderMessagesViewport(); state.chatLoading = false; } @@ -2405,8 +2419,11 @@ class ChatPane extends OpenClawLightDomElement { } if (sourceChanged && snapshot.connected && state.sessionKey) { // Reconnects clear the probed states above; re-probe the active session - // so the Discussion action reappears without a manual session switch. + // so source-owned affordances reappear without a manual session switch. void this.probeSessionDiscussion(state.sessionKey); + if (!clientChanged) { + void this.refreshSessionPullRequests(); + } } state.terminalAvailable = this.context.config.current.terminalEnabled && diff --git a/ui/src/pages/chat/chat-state.test.ts b/ui/src/pages/chat/chat-state.test.ts index e30df5dc6fb..19d27dc309c 100644 --- a/ui/src/pages/chat/chat-state.test.ts +++ b/ui/src/pages/chat/chat-state.test.ts @@ -346,6 +346,168 @@ describe("ChatStateController render lifecycle", () => { }); }); +describe("session pull request refresh", () => { + afterEach(() => { + vi.clearAllTimers(); + vi.useRealTimers(); + }); + + function createFinalReplyState(refreshSessionPullRequests: ReturnType) { + return { + chatComposerFallbackByScope: {}, + chatMessages: [], + chatMessagesBySession: new Map(), + chatQueue: [], + chatQueueByScope: {}, + chatRunId: null, + chatSideResultTerminalRuns: new Set(), + chatStream: null, + chatStreamRenderFrame: null, + chatStreamSegments: [], + chatToolMessages: [], + lastError: null, + pendingSessionMessageReloadSessionKey: null, + refreshSessionPullRequests, + requestUpdate: vi.fn(), + sessionKey: "main", + sessions: { reconcileRunTerminal: vi.fn() }, + settings: {}, + toolStreamById: new Map(), + toolStreamOrder: [], + } as unknown as ChatPageHost; + } + + it("requests an authoritative refresh after a final assistant PR link", () => { + vi.useFakeTimers(); + const refreshSessionPullRequests = vi.fn(async () => undefined); + const state = createFinalReplyState(refreshSessionPullRequests); + + handlePageGatewayEvent(state, { + type: "event", + event: "chat", + payload: { + state: "final", + sessionKey: "main", + message: { + role: "assistant", + content: [ + { + type: "text", + text: "Opened `https://github.com/openclaw/openclaw/pull/111532`.", + }, + ], + }, + }, + }); + + expect(refreshSessionPullRequests).toHaveBeenCalledWith({ refresh: true }); + }); + + it("refreshes for a visible same-session final from another run", () => { + vi.useFakeTimers(); + const refreshSessionPullRequests = vi.fn(async () => undefined); + const state = createFinalReplyState(refreshSessionPullRequests); + state.chatRunId = "active-run"; + + handlePageGatewayEvent(state, { + type: "event", + event: "chat", + payload: { + state: "final", + runId: "announcement-run", + sessionKey: "main", + message: { + role: "assistant", + content: [ + { + type: "text", + text: "Opened https://github.com/openclaw/openclaw/pull/111532", + }, + ], + }, + }, + }); + + expect(refreshSessionPullRequests).toHaveBeenCalledWith({ refresh: true }); + }); + + it("does not inspect the active stream for another run's final", () => { + vi.useFakeTimers(); + const refreshSessionPullRequests = vi.fn(async () => undefined); + const state = createFinalReplyState(refreshSessionPullRequests); + state.chatRunId = "active-run"; + state.chatStream = "Opened https://github.com/openclaw/openclaw/pull/111532"; + + handlePageGatewayEvent(state, { + type: "event", + event: "chat", + payload: { + state: "final", + runId: "announcement-run", + sessionKey: "main", + message: { + role: "assistant", + content: [{ type: "text", text: "Finished the background task." }], + }, + }, + }); + + expect(refreshSessionPullRequests).not.toHaveBeenCalled(); + }); + + it("does not refresh for an issue link", () => { + vi.useFakeTimers(); + const refreshSessionPullRequests = vi.fn(async () => undefined); + const state = createFinalReplyState(refreshSessionPullRequests); + + handlePageGatewayEvent(state, { + type: "event", + event: "chat", + payload: { + state: "final", + sessionKey: "main", + message: { + role: "assistant", + content: [ + { + type: "text", + text: "Tracked in https://github.com/openclaw/openclaw/issues/111532.", + }, + ], + }, + }, + }); + + expect(refreshSessionPullRequests).not.toHaveBeenCalled(); + }); + + it("does not refresh for another session's PR announcement", () => { + vi.useFakeTimers(); + const refreshSessionPullRequests = vi.fn(async () => undefined); + const state = createFinalReplyState(refreshSessionPullRequests); + + handlePageGatewayEvent(state, { + type: "event", + event: "chat", + payload: { + state: "final", + sessionKey: "agent:main:other", + message: { + role: "assistant", + content: [ + { + type: "text", + text: "Opened https://github.com/openclaw/openclaw/pull/111532", + }, + ], + }, + }, + }); + + expect(refreshSessionPullRequests).not.toHaveBeenCalled(); + }); +}); + describe("route composer fallback", () => { function createRouteState(chatMessage: string) { const resetChatInputHistoryNavigation = vi.fn(); diff --git a/ui/src/pages/chat/chat-state.ts b/ui/src/pages/chat/chat-state.ts index b1d132d72e3..de835a15faf 100644 --- a/ui/src/pages/chat/chat-state.ts +++ b/ui/src/pages/chat/chat-state.ts @@ -19,6 +19,7 @@ import { type UiSettings, } from "../../app/settings.ts"; import { fireFirstReplyConfetti } from "../../components/confetti.ts"; +import { isGitHubPullRequestLink } from "../../components/github-link-hovercard.ts"; import { isRenderableControlUiAvatarUrl } from "../../lib/avatar.ts"; import type { ChatAttachment, ChatQueueItem } from "../../lib/chat/chat-types.ts"; import { extractText } from "../../lib/chat/message-extract.ts"; @@ -296,6 +297,7 @@ export type ChatPageHost = ChatHost & exportCurrentChat?: () => Promise | void; refreshCurrentSessionTools?: () => Promise; refreshCurrentChat?: () => Promise; + refreshSessionPullRequests?: (options?: { refresh?: boolean }) => Promise; }; type PendingCreatedSessionComposer = { @@ -1427,6 +1429,40 @@ export function createPageState( return state; } +const GITHUB_URL_CANDIDATE = /https:\/\/github\.com\/[^\s<>()\]}'"`]+/giu; + +function terminalOwnsActiveChatStream( + state: ChatPageHost, + payload: ChatEventPayload | undefined, +): boolean { + return typeof payload?.runId === "string" && payload.runId === state.chatRunId; +} + +function finalAssistantReplyHasPullRequestLink( + state: ChatPageHost, + payload: ChatEventPayload | undefined, +): boolean { + if (payload?.state !== "final") { + return false; + } + const texts = [extractText(payload.message)]; + if (terminalOwnsActiveChatStream(state, payload)) { + texts.push( + state.chatStream, + ...(state.chatStreamSegments ?? []).map((segment) => segment.text), + ); + } + return texts.some((text) => { + if (typeof text !== "string") { + return false; + } + return Array.from(text.matchAll(GITHUB_URL_CANDIDATE)).some((match) => { + const href = match[0].replace(/[.,;:!?]+$/u, ""); + return isGitHubPullRequestLink(href); + }); + }); +} + function hasVisibleFinalAssistantReply( state: ChatPageHost, payload: ChatEventPayload | undefined, @@ -1449,6 +1485,9 @@ function hasVisibleFinalAssistantReply( ) { return true; } + if (!terminalOwnsActiveChatStream(state, payload)) { + return false; + } return [ state.chatStream, ...(state.chatStreamSegments ?? []).map((segment) => segment.text), @@ -1462,6 +1501,8 @@ export function handlePageGatewayEvent(state: ChatPageHost, event: GatewayEventF if (event.event === "chat") { const payload = event.payload as ChatEventPayload | undefined; const shouldCelebrateFirstReply = hasVisibleFinalAssistantReply(state, payload); + const shouldRefreshPullRequests = + shouldCelebrateFirstReply && finalAssistantReplyHasPullRequestLink(state, payload); const terminal = payload?.state === "final" || payload?.state === "aborted" || payload?.state === "error"; const delivered = terminal ? rememberDeliveredQueuedUserTurn(state, payload?.runId) : null; @@ -1474,6 +1515,9 @@ export function handlePageGatewayEvent(state: ChatPageHost, event: GatewayEventF if (shouldCelebrateFirstReply && result === "final") { fireFirstReplyConfetti(); } + if (shouldRefreshPullRequests) { + void state.refreshSessionPullRequests?.({ refresh: true }); + } replayPendingSessionMessageReload(state, payload); if (terminal) { removeDeliveredQueuedChatSendForRun(state, payload?.runId); diff --git a/ui/src/styles/components.css b/ui/src/styles/components.css index 49dc4eb9b63..bc98971ce7e 100644 --- a/ui/src/styles/components.css +++ b/ui/src/styles/components.css @@ -5220,6 +5220,7 @@ td.data-table-key-col { color: var(--muted); } +.session-row-badge--pull-request, .session-row-badge--cloud[data-placement-state="active"] { color: var(--ok); } diff --git a/ui/src/test-helpers/app-sidebar-cases/pull-request-state.ts b/ui/src/test-helpers/app-sidebar-cases/pull-request-state.ts new file mode 100644 index 00000000000..4044b4d22d9 --- /dev/null +++ b/ui/src/test-helpers/app-sidebar-cases/pull-request-state.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import type { GatewayBrowserClient } from "../../api/gateway.ts"; +import { createGateway, createSessionsHarness, mountSidebar } from "../app-sidebar.ts"; +import "../../components/app-sidebar.ts"; + +describe("AppSidebar pull request state", () => { + it("shows the green PR indicator for the matching session", async () => { + const key = "agent:main:pr-detection"; + const gateway = createGateway({} as GatewayBrowserClient); + const sessions = createSessionsHarness("main", [key, "agent:main:other"]); + const { sidebar } = await mountSidebar(gateway, sessions.sessions); + const row = sidebar.querySelector(`[data-session-key="${key}"]`); + + expect(row?.querySelector(".session-row-badge--pull-request")).toBeNull(); + + sessions.sessions.setOpenPullRequest(key, true); + await sidebar.updateComplete; + + const badge = row?.querySelector(".session-row-badge--pull-request"); + expect(badge?.getAttribute("aria-label")).toBe("Open PR"); + expect( + sidebar.querySelector( + '[data-session-key="agent:main:other"] .session-row-badge--pull-request', + ), + ).toBeNull(); + }); +}); diff --git a/ui/src/test-helpers/app-sidebar.ts b/ui/src/test-helpers/app-sidebar.ts index fb828de1100..2a94605964e 100644 --- a/ui/src/test-helpers/app-sidebar.ts +++ b/ui/src/test-helpers/app-sidebar.ts @@ -165,6 +165,7 @@ export function createSessionsHarness(agentId: string, keys: string[]) { let state = createSessionState(agentId, keys); let canonicalListRevision = 1; const listeners = new Set<(next: SessionState) => void>(); + const openPullRequestSessionKeys = new Set(); const groupsPut = vi.fn(() => Promise.resolve()); const groupsRename = vi.fn(() => Promise.resolve("completed")); const groupsDelete = vi.fn(() => Promise.resolve("completed")); @@ -205,6 +206,17 @@ export function createSessionsHarness(agentId: string, keys: string[]) { return () => listeners.delete(listener); }, subscribeCreated: () => () => undefined, + hasOpenPullRequest: (key: string) => openPullRequestSessionKeys.has(key), + setOpenPullRequest(key: string, hasOpenPullRequest: boolean) { + if (hasOpenPullRequest) { + openPullRequestSessionKeys.add(key); + } else { + openPullRequestSessionKeys.delete(key); + } + for (const listener of listeners) { + listener(state); + } + }, groupsLoad: () => Promise.resolve(), groupsPut, groupsRename,