From c8870d36aed2283bfa14419cd0bb346083f27b9c Mon Sep 17 00:00:00 2001 From: Guilhem Date: Mon, 20 Jul 2026 20:27:56 +0200 Subject: [PATCH] fix(sessions): session bar badge readouts, job persistence, refresh bounce (#10217) --- .../copilot/chat/AIChatManager.svelte.ts | 18 ++++++++++ .../copilot/chat/AIChatManager.test.ts | 31 +++++++++++++++++ .../copilot/chat/JobsSegment.svelte | 34 +++++++++++++++---- .../src/lib/components/copilot/chat/shared.ts | 3 ++ .../sessions/SessionChangesBar.svelte | 11 ++++-- .../(root)/(logged)/sessions/+page.svelte | 7 +++- 6 files changed, 95 insertions(+), 9 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index bc9968561c..b9649200ab 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -627,8 +627,26 @@ export class AIChatManager { updateJob = (jobId: string, update: Partial) => { const idx = this.backgroundJobs.findIndex((j) => j.jobId === jobId) if (idx === -1) return + const wasTerminal = !this.isJobNonTerminal(this.backgroundJobs[idx].status) this.backgroundJobs[idx] = { ...this.backgroundJobs[idx], ...update } this.backgroundJobs = [...this.backgroundJobs] + // Persist on the transition to terminal: a job that completes inside the + // inline wait never hits the detach/poller persist paths, and would + // otherwise vanish from the tray on reload. + if (!wasTerminal && !this.isJobNonTerminal(this.backgroundJobs[idx].status)) { + void this.#persistBackgroundJobs() + } + } + + /** Mark finished jobs as reviewed (their terminal status was shown in the + * jobs popover) and persist, so the chip stays relaxed across reloads. */ + markJobsReviewed = (jobIds: string[]) => { + const ids = new Set(jobIds) + if (!this.backgroundJobs.some((j) => ids.has(j.jobId) && !j.reviewed)) return + this.backgroundJobs = this.backgroundJobs.map((j) => + ids.has(j.jobId) && !j.reviewed ? { ...j, reviewed: true } : j + ) + void this.#persistBackgroundJobs() } /** A job left the inline wait — hand it to the background poller. */ diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts index 58a2f0cd0a..55bea0d7e6 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.test.ts @@ -2539,6 +2539,37 @@ describe('AIChatManager background job completion', () => { expect(manager.pendingJobNotes).toHaveLength(1) expect(manager.pendingJobNotes[0]).toContain('Background job job-1 for "run" succeeded') }) + + it('persists on the inline terminal transition and on review', async () => { + const manager = new AIChatManager() + manager.registerJob({ + jobId: 'job-1', + toolCallId: 'tc-1', + kind: 'script', + label: 'run', + workspace: 'ws' + }) + const saveChat = vi.spyOn(manager.historyManager, 'saveChat').mockResolvedValue(undefined) + + // Inline completion reports through updateJob without ever detaching; the + // terminal transition alone must write the tray or the job vanishes on reload. + manager.updateJob('job-1', { status: 'running' }) + expect(saveChat).not.toHaveBeenCalled() + manager.updateJob('job-1', { status: 'success' }) + await vi.waitFor(() => expect(saveChat).toHaveBeenCalledTimes(1)) + expect(saveChat.mock.calls[0][4]).toEqual([ + expect.objectContaining({ jobId: 'job-1', status: 'success' }) + ]) + + // Reviewing persists the flag; re-reviewing is a no-op (no extra write). + manager.markJobsReviewed(['job-1']) + await vi.waitFor(() => expect(saveChat).toHaveBeenCalledTimes(2)) + expect(saveChat.mock.calls[1][4]).toEqual([ + expect.objectContaining({ jobId: 'job-1', reviewed: true }) + ]) + manager.markJobsReviewed(['job-1']) + expect(saveChat).toHaveBeenCalledTimes(2) + }) }) describe('DOM selector chips scoped by app path', () => { diff --git a/frontend/src/lib/components/copilot/chat/JobsSegment.svelte b/frontend/src/lib/components/copilot/chat/JobsSegment.svelte index 9a6d505b8a..b8a59d5bc9 100644 --- a/frontend/src/lib/components/copilot/chat/JobsSegment.svelte +++ b/frontend/src/lib/components/copilot/chat/JobsSegment.svelte @@ -32,8 +32,21 @@ const queuedCount = $derived( jobs.filter((j) => j.status === 'queued' || j.status === 'scheduled').length ) - const failureCount = $derived(jobs.filter((j) => j.status === 'failure').length) - const successCount = $derived(jobs.filter((j) => j.status === 'success').length) + // A finished job counts as reviewed once its terminal status has been shown in + // the open popover; a job that finishes while the popover is closed starts + // unreviewed. The flag lives on the job (persisted with it), so review state + // survives a reload. + $effect(() => { + if (!open) return + aiChatManager.markJobsReviewed(jobs.filter((j) => isTerminal(j.status)).map((j) => j.jobId)) + }) + // Only unreviewed jobs feed the terminal readout: an outcome the user already + // saw must not resurface on the chip when a later job finishes. + const unreviewed = $derived(jobs.filter((j) => !j.reviewed)) + const allReviewed = $derived(jobs.length > 0 && unreviewed.length === 0) + + const failureCount = $derived(unreviewed.filter((j) => j.status === 'failure').length) + const successCount = $derived(unreviewed.filter((j) => j.status === 'success').length) const liveCount = $derived(jobs.filter((j) => !isTerminal(j.status)).length) const hasLive = $derived(liveCount > 0) @@ -70,9 +83,18 @@ // Aggregate chip readout, priority-ordered so the most action-worthy state // wins the dot: approval > running > queued > failed > succeeded. A live run - // takes the dot even if an earlier job failed (failure resurfaces once idle). + // takes the dot even if an earlier job failed (an unreviewed failure + // resurfaces once idle). Once every finished job has been reviewed in the + // popover, the chip relaxes to a neutral executed-count. const segment = $derived.by( (): { dot: string; pulse: boolean; text: string; danger: boolean } => { + if (allReviewed) + return { + dot: 'bg-gray-400', + pulse: false, + text: `${jobs.length} job${jobs.length === 1 ? '' : 's'} executed`, + danger: false + } if (approvalCount > 0) return { dot: dotClass('suspended'), @@ -111,8 +133,8 @@ text: `${failureCount} failed`, danger: true } - // All terminal, none failed: green if anything actually succeeded, else gray - // (only canceled jobs left — a cancel isn't a success, so don't show green). + // All terminal, nothing unreviewed failed: green if anything unreviewed + // succeeded, else gray (only canceled left — a cancel isn't a success). if (successCount > 0) return { dot: dotClass('success'), @@ -123,7 +145,7 @@ return { dot: dotClass('canceled'), pulse: false, - text: `${jobs.length} canceled`, + text: `${unreviewed.length} canceled`, danger: false } } diff --git a/frontend/src/lib/components/copilot/chat/shared.ts b/frontend/src/lib/components/copilot/chat/shared.ts index d37b3eda2a..e37ae1da7f 100644 --- a/frontend/src/lib/components/copilot/chat/shared.ts +++ b/frontend/src/lib/components/copilot/chat/shared.ts @@ -899,6 +899,9 @@ export type ChatJob = { detached: boolean /** Notify-only: whether its completion has been surfaced to the model yet. */ reported: boolean + /** Whether the user saw its terminal status in the jobs popover. Reviewed + * outcomes stop driving the segment chip's status readout. Persisted. */ + reviewed?: boolean /** Trimmed snapshot of the last fetched Job (heavy fields stripped, see * `trimJob`), fed to `` so the tray badge matches the runs page * exactly. Always written together with `status` from the SAME job so the two diff --git a/frontend/src/lib/components/sessions/SessionChangesBar.svelte b/frontend/src/lib/components/sessions/SessionChangesBar.svelte index e133dcffea..1d2c9d95e0 100644 --- a/frontend/src/lib/components/sessions/SessionChangesBar.svelte +++ b/frontend/src/lib/components/sessions/SessionChangesBar.svelte @@ -15,6 +15,7 @@ import { badgeCounts, badgeOf, buildDeployItems } from './sessionDeployModel' import { useExistingMaskKeys } from './sessionDeployModel.svelte' import JobsSegment from '$lib/components/copilot/chat/JobsSegment.svelte' + import RowIcon from '$lib/components/common/table/RowIcon.svelte' import ArtifactsSegment from '$lib/components/copilot/chat/artifacts/ArtifactsSegment.svelte' // Unified session bar: surfaces what the CURRENT chat changed — pending @@ -173,8 +174,13 @@ const hasJobs = $derived((runtime?.manager.backgroundJobs.length ?? 0) > 0) const hasArtifacts = $derived((runtime?.manager.artifacts.artifacts.length ?? 0) > 0) - const editsCount = $derived(dockCounts.draft + dockCounts.deployed) - const editsLabel = $derived(`${editsCount} edit${editsCount === 1 ? '' : 's'}`) + // Drafts are what still needs action, so the token counts only them while any + // are pending; once none are left it turns green and counts the deployed. + const editsLabel = $derived( + dockCounts.draft > 0 + ? `${dockCounts.draft} draft${dockCounts.draft === 1 ? '' : 's'}` + : `${dockCounts.deployed} deployed` + ) let editsOpen = $state(false) // Only draft-vs-deployed drives the color: stale/failed live in the drawer's @@ -262,6 +268,7 @@ maxHeightClass="max-h-[min(9rem,50vh)]" > {#snippet row(item)} + {item.displayPath} diff --git a/frontend/src/routes/(root)/(logged)/sessions/+page.svelte b/frontend/src/routes/(root)/(logged)/sessions/+page.svelte index 958ad92fc3..a3f45f95fd 100644 --- a/frontend/src/routes/(root)/(logged)/sessions/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/sessions/+page.svelte @@ -35,7 +35,7 @@ import { withWorkspaceParam } from '$lib/components/sessions/sessionMode.svelte' import { enterSessionMode } from '$lib/components/sessions/sessionSwitch.svelte' import type { SessionPreviewTabs } from '$lib/components/sessions/sessionPreviewTabs.svelte' - import { userWorkspaces, workspaceStore } from '$lib/stores' + import { userWorkspaces, usersWorkspaceStore, workspaceStore } from '$lib/stores' import { getOrCreateRuntime, getRuntime, @@ -130,6 +130,11 @@ // not-found UI below. $effect(() => { if (embedded || !sessionState.hydrated) return + // Family membership can't be judged before the workspace list arrives: + // workspaceRootId falls back to the raw id for workspaces it can't find, + // which makes a same-family session look foreign on a hard reload and + // would bounce the URL to another (or a brand-new) session. + if ($usersWorkspaceStore === undefined) return // sessionInCurrentFamily reads these via get(), so track them explicitly. $workspaceStore $userWorkspaces