From 3cb81b1bcf2364d7a127d5ef53541fb2eda069dc Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Thu, 2 Jul 2026 14:16:17 +0200 Subject: [PATCH] fix: show pending session starts safely --- .changeset/fuzzy-session-starts.md | 5 + src/client/src/appState.ts | 5 + src/client/src/components/PiWebApp.ts | 1 + src/client/src/components/SessionList.ts | 28 ++++- .../components/appShell/AppNavigationPanel.ts | 2 + .../src/controllers/sessionController.test.ts | 114 ++++++++++++++++++ .../src/controllers/sessionController.ts | 31 ++++- 7 files changed, 180 insertions(+), 6 deletions(-) create mode 100644 .changeset/fuzzy-session-starts.md diff --git a/.changeset/fuzzy-session-starts.md b/.changeset/fuzzy-session-starts.md new file mode 100644 index 0000000..48d9921 --- /dev/null +++ b/.changeset/fuzzy-session-starts.md @@ -0,0 +1,5 @@ +--- +"@jmfederico/pi-web": patch +--- + +Improve session-start feedback so concurrent new sessions stay visible without disrupting session-list navigation. diff --git a/src/client/src/appState.ts b/src/client/src/appState.ts index 83ac086..e64e726 100644 --- a/src/client/src/appState.ts +++ b/src/client/src/appState.ts @@ -20,6 +20,8 @@ export interface AppState { isReceivingPartialStream: boolean; /** Sessions with a prompt upload in flight, keyed by sessionId (client-owned). */ sendingPrompts: Record; + /** Client-initiated session creation requests waiting for the server. */ + startingSessionCount: number; isLoadingProjects: boolean; isLoadingWorkspaces: boolean; selectedProject: Project | undefined; @@ -72,6 +74,7 @@ export type AuthDialogState = export type WorkspaceScopedStateReset = Pick = {}; @property({ attribute: false }) sending: Record = {}; @property({ attribute: false }) selected?: SessionInfo; + @property({ type: Number }) startingCount = 0; @property({ type: Boolean }) canStart = false; @property({ type: Boolean }) canDeleteArchived = false; @property({ type: Boolean }) canReload = false; @@ -109,6 +110,7 @@ export class SessionList extends LitElement implements KeyboardNavigableSection ${this.collapsed ? null : html`
${this.renderCurrentSelectionToolbar(currentSelectableSessions)} + ${this.startingCount > 0 ? this.renderStartingSession() : null} ${currentRows.map((row) => this.renderSession(row, descendantCounts.get(row.session.id) ?? 0, "current"))} ${archivedRows.length > 0 ? html` ${this.renderArchivedHeading(archivedRows.map((row) => row.session))} @@ -130,7 +132,7 @@ export class SessionList extends LitElement implements KeyboardNavigableSection Sessions ${this.renderCurrentSelectionButton(currentSessions)} ${this.renderCleanupButton()} - + ${this.renderStartButton()} `; } @@ -142,7 +144,7 @@ export class SessionList extends LitElement implements KeyboardNavigableSection ${this.renderCurrentSelectionButton(currentSessions)} ${sessionCount} ${this.renderCleanupButton()} - + ${this.renderStartButton()} `; } @@ -157,6 +159,23 @@ export class SessionList extends LitElement implements KeyboardNavigableSection return html``; } + private renderStartButton() { + const title = this.startingCount > 0 ? "Start another session" : "Start a new session"; + return html``; + } + + private renderStartingSession() { + const plural = this.startingCount !== 1; + return html` +
+
+ ${plural ? `Starting ${String(this.startingCount)} sessions…` : "Starting session…"} + Waiting for ${plural ? "new sessions" : "the new session"} to be created +
+
+ `; + } + private renderArchivedHeading(archivedSessions: SessionInfo[]) { const active = this.selectionScopes.has("archived"); return html` @@ -381,6 +400,7 @@ export class SessionList extends LitElement implements KeyboardNavigableSection h2 { min-height: 30px; } h2 > .section-count { flex: 0 0 auto; display: inline; color: var(--pi-muted); font-size: inherit; } .bulk-select-entry { box-sizing: border-box; flex: 0 0 auto; display: inline-grid; place-items: center; width: 30px; height: 30px; padding: 0; font-size: 13px; line-height: 1; text-transform: none; } + .start-session-button { box-sizing: border-box; flex: 0 0 auto; display: inline-grid; place-items: center; min-width: 30px; height: 30px; padding: 0 9px; } .cleanup-entry { flex: 0 0 auto; padding: 5px 7px; font-size: 12px; text-transform: none; } .bulk-row { display: flex; flex-wrap: wrap; align-items: center; gap: 6px; margin: 0 0 6px; } .bulk-row button { padding: 5px 7px; font-size: 12px; } @@ -391,6 +411,10 @@ export class SessionList extends LitElement implements KeyboardNavigableSection button.danger, .action-menu-panel button.danger { color: var(--pi-danger); } button.danger:hover, .action-menu-panel button.danger:hover { background: color-mix(in srgb, var(--pi-danger) 14%, transparent); } .action-row.bulk-selected .action-main { border-color: var(--pi-accent); box-shadow: inset 3px 0 0 var(--pi-accent); } + .pending-session-row { position: relative; display: grid; grid-template-columns: minmax(0, 1fr); margin: 6px 0; cursor: default; } + .pending-session-row.starting-session .action-main { border-radius: 8px; border-style: dashed; color: var(--pi-muted); } + .pending-session-row.starting-session .action-name { display: flex; align-items: center; gap: 6px; max-height: none; -webkit-line-clamp: 1; } + .pending-session-row.starting-session .activity-indicator { flex: 0 0 auto; margin: 0; } .action-main.selecting { padding-left: calc(32px + var(--depth, 0) * 16px); } .session-checkbox { position: absolute; top: 9px; left: calc(8px + var(--depth, 0) * 16px); z-index: 2; margin: 0; } `]; diff --git a/src/client/src/components/appShell/AppNavigationPanel.ts b/src/client/src/components/appShell/AppNavigationPanel.ts index 667b192..5a2fc40 100644 --- a/src/client/src/components/appShell/AppNavigationPanel.ts +++ b/src/client/src/components/appShell/AppNavigationPanel.ts @@ -39,6 +39,7 @@ export class AppNavigationPanel extends LitElement { @property({ type: Boolean }) projectsCollapsed = false; @property({ type: Boolean }) workspacesCollapsed = false; @property({ type: Boolean }) sessionsCollapsed = false; + @property({ type: Number }) startingSessionCount = 0; @property({ type: Boolean }) canStartSession = false; @property({ type: Boolean }) canDeleteArchivedSessions = false; @property({ type: Boolean }) canReloadSessions = false; @@ -159,6 +160,7 @@ export class AppNavigationPanel extends LitElement { .activities=${this.sessionActivities} .sending=${this.sendingPrompts} .selected=${this.selectedSession} + .startingCount=${this.startingSessionCount} .canStart=${this.canStartSession} .canDeleteArchived=${this.canDeleteArchivedSessions} .canReload=${this.canReloadSessions} diff --git a/src/client/src/controllers/sessionController.test.ts b/src/client/src/controllers/sessionController.test.ts index e2beef9..a109323 100644 --- a/src/client/src/controllers/sessionController.test.ts +++ b/src/client/src/controllers/sessionController.test.ts @@ -103,6 +103,23 @@ const replacementSession: SessionInfo = { const emptyPage: MessagePage = { messages: [], start: 0, total: 0 }; +interface Deferred { + promise: Promise; + resolve: (value: T) => void; + reject: (error: unknown) => void; +} + +function deferred(): Deferred { + let resolveDeferred: ((value: T) => void) | undefined; + let rejectDeferred: ((error: unknown) => void) | undefined; + const promise = new Promise((resolve, reject) => { + resolveDeferred = resolve; + rejectDeferred = reject; + }); + if (resolveDeferred === undefined || rejectDeferred === undefined) throw new Error("Deferred promise was not initialized"); + return { promise, resolve: resolveDeferred, reject: rejectDeferred }; +} + function status(sessionId: string): SessionStatus { return { sessionId, @@ -334,6 +351,103 @@ describe("SessionController", () => { expect(isCachedNewSessionInfo(state.sessions[0])).toBe(true); }); + it("tracks multiple pending session starts without blocking another start", async () => { + const firstStarted: SessionInfo = { ...oldSession, id: "started-session-1", path: "/tmp/started-session-1.jsonl" }; + const secondStarted: SessionInfo = { ...oldSession, id: "started-session-2", path: "/tmp/started-session-2.jsonl" }; + const startResolvers: ((session: SessionInfo) => void)[] = []; + let state: AppState = { ...initialAppState(), selectedWorkspace: workspace, sessions: [] }; + const api: typeof defaultApi = { + ...defaultApi, + startSession: () => new Promise((resolve) => { startResolvers.push(resolve); }), + messages: () => Promise.resolve(emptyPage), + status: (session) => Promise.resolve(status(sessionLookupId(session))), + }; + const controller = new SessionController( + () => state, + (patch) => { state = { ...state, ...patch }; }, + () => undefined, + undefined, + { api, socket: new FakeSocket() }, + ); + + const firstStart = controller.startSession(); + const secondStart = controller.startSession(); + + expect(startResolvers).toHaveLength(2); + expect(state.startingSessionCount).toBe(2); + expect(state.sessions).toEqual([]); + + startResolvers[0]?.(firstStarted); + await firstStart; + + expect(state.startingSessionCount).toBe(1); + expect(state.sessions.map((session) => session.id)).toEqual(["started-session-1"]); + + startResolvers[1]?.(secondStarted); + await secondStart; + + expect(state.startingSessionCount).toBe(0); + expect(state.sessions.map((session) => session.id)).toEqual(["started-session-2", "started-session-1"]); + expect(state.selectedSession?.id).toBe("started-session-2"); + }); + + it("removes a resolved session start from the pending count when inserting its row", async () => { + const firstStarted: SessionInfo = { ...oldSession, id: "started-session-1", path: "/tmp/started-session-1.jsonl" }; + const secondStarted: SessionInfo = { ...oldSession, id: "started-session-2", path: "/tmp/started-session-2.jsonl" }; + const startResolvers: ((session: SessionInfo) => void)[] = []; + const messageRequests = new Map>(); + const statusRequests = new Map>(); + let state: AppState = { ...initialAppState(), selectedWorkspace: workspace, sessions: [] }; + const api: typeof defaultApi = { + ...defaultApi, + startSession: () => new Promise((resolve) => { startResolvers.push(resolve); }), + messages: (session) => { + const request = deferred(); + messageRequests.set(sessionLookupId(session), request); + return request.promise; + }, + status: (session) => { + const request = deferred(); + statusRequests.set(sessionLookupId(session), request); + return request.promise; + }, + }; + const controller = new SessionController( + () => state, + (patch) => { state = { ...state, ...patch }; }, + () => undefined, + undefined, + { api, socket: new FakeSocket() }, + ); + + const firstStart = controller.startSession(); + const secondStart = controller.startSession(); + expect(startResolvers).toHaveLength(2); + expect(state.startingSessionCount).toBe(2); + + startResolvers[0]?.(firstStarted); + await Promise.resolve(); + await Promise.resolve(); + + expect(state.sessions.map((session) => session.id)).toEqual(["started-session-1"]); + expect(state.startingSessionCount).toBe(1); + + messageRequests.get(firstStarted.id)?.resolve(emptyPage); + statusRequests.get(firstStarted.id)?.resolve(status(firstStarted.id)); + await firstStart; + + startResolvers[1]?.(secondStarted); + await Promise.resolve(); + await Promise.resolve(); + + expect(state.sessions.map((session) => session.id)).toEqual(["started-session-2", "started-session-1"]); + expect(state.startingSessionCount).toBe(0); + + messageRequests.get(secondStarted.id)?.resolve(emptyPage); + statusRequests.get(secondStarted.id)?.resolve(status(secondStarted.id)); + await secondStart; + }); + it("toggles the per-session sending state around an inline attachment send and forwards attachments", async () => { let resolvePrompt: (() => void) | undefined; let promptArgs: { attachments?: PromptAttachment[] } | undefined; diff --git a/src/client/src/controllers/sessionController.ts b/src/client/src/controllers/sessionController.ts index b891227..9d9ff6e 100644 --- a/src/client/src/controllers/sessionController.ts +++ b/src/client/src/controllers/sessionController.ts @@ -98,18 +98,37 @@ export class SessionController { async startSession() { const workspace = this.getState().selectedWorkspace; if (!workspace) return; + const machineId = selectedMachineId(this.getState()); + const isCurrentWorkspace = () => selectedMachineId(this.getState()) === machineId && this.getState().selectedWorkspace?.id === workspace.id; + this.setState({ startingSessionCount: this.getState().startingSessionCount + 1, error: "" }); + let shouldDecrementStartingCount = true; try { - const machineId = selectedMachineId(this.getState()); const session = await this.api.startSession(workspace.path, machineId); rememberCachedNewSession(session, machineId); const cachedSession = markCachedNewSessionInfo(session, machineId); + if (!isCurrentWorkspace()) return; + const state = this.getState(); // Drop any entry the session.created broadcast may have inserted for this // same session before the HTTP response resolved, so the cached marker - // (and its delete action) wins instead of leaving a duplicate badge. - this.setState({ sessions: [cachedSession, ...this.getState().sessions.filter((candidate) => candidate.id !== cachedSession.id)] }); + // (and its delete action) wins instead of leaving a duplicate badge. The + // pending count for this completed request is consumed in the same patch + // so the list never renders both the real session and its placeholder. + this.setState({ + sessions: [cachedSession, ...state.sessions.filter((candidate) => candidate.id !== cachedSession.id)], + startingSessionCount: decrementStartingSessionCount(state.startingSessionCount), + }); + shouldDecrementStartingCount = false; await this.selectSession(cachedSession); } catch (error) { - this.setState({ error: String(error) }); + if (!isCurrentWorkspace()) return; + if (!shouldDecrementStartingCount) { + this.setState({ error: String(error) }); + return; + } + this.setState({ error: String(error), startingSessionCount: decrementStartingSessionCount(this.getState().startingSessionCount) }); + shouldDecrementStartingCount = false; + } finally { + if (shouldDecrementStartingCount && isCurrentWorkspace()) this.setState({ startingSessionCount: decrementStartingSessionCount(this.getState().startingSessionCount) }); } } @@ -854,6 +873,10 @@ function omitSessionActivity(activities: Record, sessio return omitKey(activities, sessionId); } +function decrementStartingSessionCount(count: number): number { + return Math.max(0, count - 1); +} + function omitKey(record: Record, key: string): Record { return Object.fromEntries(Object.entries(record).filter(([id]) => id !== key)); }