From 13d6ad598e58bdce4be6292fa994804a8eba35d1 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 13 Jul 2026 15:36:48 +0200 Subject: [PATCH 1/4] Keep New Workspace prompts when switching projects or hosts (#2036) * fix(app): preserve New Workspace prompts across target changes The New Workspace draft was keyed by the selected host and project, so changing either target replaced the visible prompt with another draft scope. Treat the screen as one persistent draft surface and migrate the newest active legacy draft. * test(app): express draft persistence as user actions * fix(app): clear stale picker PR context * fix(app): persist picker attachment ownership * fix(app): keep workspace target context safe --- packages/app/e2e/helpers/new-workspace.ts | 15 +++ .../e2e/new-workspace-composer-draft.spec.ts | 88 +++++++++++++++ packages/app/src/attachments/types.ts | 8 +- .../new-workspace-picker-state.test.ts | 74 +++++++++---- .../src/screens/new-workspace-picker-state.ts | 63 +++++++---- .../app/src/screens/new-workspace-screen.tsx | 69 ++++++------ packages/app/src/stores/draft-keys.ts | 18 +++ .../src/stores/draft-store/migration.test.ts | 104 +++++++++++++++++- .../app/src/stores/draft-store/migration.ts | 44 +++++++- .../app/src/stores/draft-store/state.test.ts | 34 +++++- packages/app/src/stores/draft-store/state.ts | 24 +++- 11 files changed, 462 insertions(+), 79 deletions(-) create mode 100644 packages/app/e2e/new-workspace-composer-draft.spec.ts diff --git a/packages/app/e2e/helpers/new-workspace.ts b/packages/app/e2e/helpers/new-workspace.ts index e404682a26..b907017f1e 100644 --- a/packages/app/e2e/helpers/new-workspace.ts +++ b/packages/app/e2e/helpers/new-workspace.ts @@ -181,6 +181,21 @@ export async function expectNewWorkspaceProjectSelected( await expect(projectPicker).toContainText(projectDisplayName); } +export async function fillNewWorkspaceDraft(page: Page, draft: string): Promise { + const composer = page.getByRole("textbox", { name: "Message agent..." }); + await expect(composer).toBeVisible({ timeout: 30_000 }); + await composer.fill(draft); +} + +export async function expectNewWorkspaceDraft(page: Page, draft: string): Promise { + await expect(page.getByRole("textbox", { name: "Message agent..." })).toHaveValue(draft); +} + +export async function selectNewWorkspaceHost(page: Page, hostLabel: string): Promise { + await page.getByTestId("host-picker-trigger").click(); + await page.getByText(hostLabel, { exact: true }).click(); +} + export async function submitNewWorkspacePrompt( page: Page, prompt = "Hello from e2e", diff --git a/packages/app/e2e/new-workspace-composer-draft.spec.ts b/packages/app/e2e/new-workspace-composer-draft.spec.ts new file mode 100644 index 0000000000..3606fe7e08 --- /dev/null +++ b/packages/app/e2e/new-workspace-composer-draft.spec.ts @@ -0,0 +1,88 @@ +import { test } from "./fixtures"; +import { gotoAppShell } from "./helpers/app"; +import { getE2EDaemonPort } from "./helpers/daemon-port"; +import { + expectNewWorkspaceDraft, + expectNewWorkspaceProjectSelected, + fillNewWorkspaceDraft, + openGlobalNewWorkspaceComposer, + openNewWorkspaceComposer, + selectNewWorkspaceHost, + selectNewWorkspaceProject, +} from "./helpers/new-workspace"; +import { seedWorkspace, type SeededWorkspace } from "./helpers/seed-client"; +import { getServerId } from "./helpers/server-id"; +import { seedSavedSettingsHosts } from "./helpers/settings"; +import { waitForSidebarHydration } from "./helpers/workspace-ui"; + +const DRAFT = `Please investigate the workspace startup failure. + +Trace the request from the app through the daemon, preserve the existing behavior, and explain the root cause before making changes.`; + +test.describe("New workspace composer draft", () => { + test.describe.configure({ timeout: 240_000 }); + + test("keeps the draft when the project changes", async ({ page }) => { + const firstProject: SeededWorkspace = await seedWorkspace({ + repoPrefix: "new-workspace-draft-project-a-", + }); + const secondProject: SeededWorkspace = await seedWorkspace({ + repoPrefix: "new-workspace-draft-project-b-", + }); + + try { + await gotoAppShell(page); + await waitForSidebarHydration(page); + await openNewWorkspaceComposer(page, { + projectKey: firstProject.projectId, + projectDisplayName: firstProject.projectDisplayName, + }); + await expectNewWorkspaceProjectSelected(page, firstProject.projectDisplayName); + + await fillNewWorkspaceDraft(page, DRAFT); + + await selectNewWorkspaceProject(page, { + projectKey: secondProject.projectId, + projectDisplayName: secondProject.projectDisplayName, + }); + + await expectNewWorkspaceDraft(page, DRAFT); + } finally { + await secondProject.cleanup(); + await firstProject.cleanup(); + } + }); + + test("keeps the draft when the host changes", async ({ page }) => { + const project: SeededWorkspace = await seedWorkspace({ + repoPrefix: "new-workspace-draft-host-", + }); + const secondaryServerId = "new-workspace-draft-secondary-host"; + + try { + await seedSavedSettingsHosts(page, [ + { + serverId: getServerId(), + label: "Primary host", + endpoint: `127.0.0.1:${getE2EDaemonPort()}`, + }, + { + serverId: secondaryServerId, + label: "Secondary host", + endpoint: "127.0.0.1:9", + }, + ]); + + await gotoAppShell(page); + await waitForSidebarHydration(page); + await openGlobalNewWorkspaceComposer(page); + + await fillNewWorkspaceDraft(page, DRAFT); + await selectNewWorkspaceHost(page, "Secondary host"); + + await expectNewWorkspaceDraft(page, DRAFT); + } finally { + await project.cleanup(); + } + }); +}); diff --git a/packages/app/src/attachments/types.ts b/packages/app/src/attachments/types.ts index eecb177662..82e447f927 100644 --- a/packages/app/src/attachments/types.ts +++ b/packages/app/src/attachments/types.ts @@ -83,11 +83,17 @@ export interface ChatHistoryContextAttachment { }; } +export const NEW_WORKSPACE_PICKER_ATTACHMENT_OWNER = "new-workspace-picker"; + export type UserComposerAttachment = | { kind: "image"; metadata: AttachmentMetadata } | { kind: "file"; attachment: UploadedFileAttachment } | { kind: "github_issue"; item: GitHubSearchItem } - | { kind: "github_pr"; item: GitHubSearchItem }; + | { + kind: "github_pr"; + item: GitHubSearchItem; + owner?: typeof NEW_WORKSPACE_PICKER_ATTACHMENT_OWNER; + }; export type WorkspaceComposerAttachment = | { diff --git a/packages/app/src/screens/new-workspace-picker-state.test.ts b/packages/app/src/screens/new-workspace-picker-state.test.ts index fa281b0665..8d9a9e8817 100644 --- a/packages/app/src/screens/new-workspace-picker-state.test.ts +++ b/packages/app/src/screens/new-workspace-picker-state.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from "vitest"; import type { UserComposerAttachment } from "@/attachments/types"; import type { GitHubSearchItem } from "@getpaseo/protocol/messages"; -import { findCheckoutHintPrAttachment, syncPickerPrAttachment } from "./new-workspace-picker-state"; +import { + clearPickerPrAttachmentForTargetChange, + findCheckoutHintPrAttachment, + syncPickerPrAttachment, +} from "./new-workspace-picker-state"; function makePrItem(number: number, title: string, headRefName = "feature/x"): GitHubSearchItem { return { @@ -19,8 +23,9 @@ function makePrItem(number: number, title: string, headRefName = "feature/x"): G function prAttachment( item: GitHubSearchItem, + owner?: "new-workspace-picker", ): Extract { - return { kind: "github_pr", item }; + return { kind: "github_pr", item, ...(owner ? { owner } : {}) }; } function issueAttachment(number: number): UserComposerAttachment { @@ -43,57 +48,88 @@ describe("syncPickerPrAttachment", () => { const pr = makePrItem(202, "Refactor picker"); const result = syncPickerPrAttachment({ attachments: [], - previousPickerPrNumber: null, item: { kind: "github-pr", item: pr }, }); - expect(result.attachedPrNumber).toBe(202); - expect(result.attachments).toEqual([prAttachment(pr)]); + expect(result).toEqual([prAttachment(pr, "new-workspace-picker")]); }); it("selects a branch without modifying attachments when no previous picker PR", () => { const issue = issueAttachment(44); const result = syncPickerPrAttachment({ attachments: [issue], - previousPickerPrNumber: null, item: { kind: "branch", name: "dev" }, }); - expect(result.attachedPrNumber).toBeNull(); - expect(result.attachments).toEqual([issue]); + expect(result).toEqual([issue]); }); it("replaces the previous picker PR when a different PR is selected", () => { const prA = makePrItem(202, "Refactor picker", "feature/picker"); const prB = makePrItem(303, "Polish chip", "feature/chip"); const result = syncPickerPrAttachment({ - attachments: [prAttachment(prA)], - previousPickerPrNumber: 202, + attachments: [prAttachment(prA, "new-workspace-picker")], item: { kind: "github-pr", item: prB }, }); - expect(result.attachedPrNumber).toBe(303); - expect(result.attachments).toEqual([prAttachment(prB)]); + expect(result).toEqual([prAttachment(prB, "new-workspace-picker")]); }); it("removes the previous picker PR and adds no new attachment when a branch is selected", () => { const pr = makePrItem(202, "Refactor picker"); const issue = issueAttachment(44); const result = syncPickerPrAttachment({ - attachments: [issue, prAttachment(pr)], - previousPickerPrNumber: 202, + attachments: [issue, prAttachment(pr, "new-workspace-picker")], item: { kind: "branch", name: "dev" }, }); - expect(result.attachedPrNumber).toBeNull(); - expect(result.attachments).toEqual([issue]); + expect(result).toEqual([issue]); }); it("does not duplicate a PR that was already manually attached by the user", () => { const pr = makePrItem(202, "Refactor picker"); const result = syncPickerPrAttachment({ attachments: [prAttachment(pr)], - previousPickerPrNumber: null, item: { kind: "github-pr", item: pr }, }); - expect(result.attachedPrNumber).toBeNull(); - expect(result.attachments).toHaveLength(1); + expect(result).toEqual([prAttachment(pr)]); + }); + + it("clears a persisted picker selection without removing user-added attachments", () => { + const pickerPr = prAttachment(makePrItem(202, "Picker PR"), "new-workspace-picker"); + const manuallyAttachedPr = prAttachment(makePrItem(303, "Manual PR")); + const issue = issueAttachment(44); + + const result = syncPickerPrAttachment({ + attachments: [issue, pickerPr, manuallyAttachedPr], + item: null, + }); + + expect(result).toEqual([issue, manuallyAttachedPr]); + }); +}); + +describe("clearPickerPrAttachmentForTargetChange", () => { + it("keeps the picker selection when the target is reselected", () => { + const pickerPr = prAttachment(makePrItem(202, "Picker PR"), "new-workspace-picker"); + const attachments = [pickerPr]; + + expect( + clearPickerPrAttachmentForTargetChange({ + attachments, + currentTargetId: "server-a", + nextTargetId: "server-a", + }), + ).toBe(attachments); + }); + + it("clears only the picker-owned PR when the target changes", () => { + const pickerPr = prAttachment(makePrItem(202, "Picker PR"), "new-workspace-picker"); + const manualPr = prAttachment(makePrItem(303, "Manual PR")); + + expect( + clearPickerPrAttachmentForTargetChange({ + attachments: [pickerPr, manualPr], + currentTargetId: "server-a", + nextTargetId: "server-b", + }), + ).toEqual([manualPr]); }); }); diff --git a/packages/app/src/screens/new-workspace-picker-state.ts b/packages/app/src/screens/new-workspace-picker-state.ts index c3a25452ec..2caa8c5883 100644 --- a/packages/app/src/screens/new-workspace-picker-state.ts +++ b/packages/app/src/screens/new-workspace-picker-state.ts @@ -1,37 +1,60 @@ -import type { UserComposerAttachment } from "@/attachments/types"; +import { + NEW_WORKSPACE_PICKER_ATTACHMENT_OWNER, + type UserComposerAttachment, +} from "@/attachments/types"; import type { PickerItem } from "./new-workspace-picker-item"; -// The picker "owns" at most one PR attachment at a time. When the user selects -// a different item the previously-owned PR is removed before the new one is added. -// User-added attachments for other PRs/issues are left untouched. +function isPickerOwnedPrAttachment(attachment: UserComposerAttachment): attachment is Extract< + UserComposerAttachment, + { kind: "github_pr" } +> & { + owner: typeof NEW_WORKSPACE_PICKER_ATTACHMENT_OWNER; +} { + return ( + attachment.kind === "github_pr" && attachment.owner === NEW_WORKSPACE_PICKER_ATTACHMENT_OWNER + ); +} + +// Ownership lives on the attachment because drafts outlive this component. +// The picker owns at most one PR; user-added PRs and issues remain untouched. export function syncPickerPrAttachment(input: { attachments: UserComposerAttachment[]; - previousPickerPrNumber: number | null; - item: PickerItem; -}): { attachments: UserComposerAttachment[]; attachedPrNumber: number | null } { - let nextAttachments = input.attachments; - let attachedPrNumber: number | null = null; + item: PickerItem | null; +}): UserComposerAttachment[] { + const nextAttachments = input.attachments.filter( + (attachment) => !isPickerOwnedPrAttachment(attachment), + ); - if (input.previousPickerPrNumber !== null) { - nextAttachments = nextAttachments.filter( - (attachment) => - attachment.kind !== "github_pr" || attachment.item.number !== input.previousPickerPrNumber, - ); - } - - if (input.item.kind === "github-pr") { + if (input.item?.kind === "github-pr") { const selectedPr = input.item.item; const hasExistingPrAttachment = nextAttachments.some( (attachment) => attachment.kind === "github_pr" && attachment.item.number === selectedPr.number, ); if (!hasExistingPrAttachment) { - nextAttachments = [...nextAttachments, { kind: "github_pr", item: selectedPr }]; - attachedPrNumber = selectedPr.number; + return [ + ...nextAttachments, + { + kind: "github_pr", + item: selectedPr, + owner: NEW_WORKSPACE_PICKER_ATTACHMENT_OWNER, + }, + ]; } } - return { attachments: nextAttachments, attachedPrNumber }; + return nextAttachments; +} + +export function clearPickerPrAttachmentForTargetChange(input: { + attachments: UserComposerAttachment[]; + currentTargetId: string; + nextTargetId: string; +}): UserComposerAttachment[] { + if (input.currentTargetId === input.nextTargetId) { + return input.attachments; + } + return syncPickerPrAttachment({ attachments: input.attachments, item: null }); } export function findCheckoutHintPrAttachment(input: { diff --git a/packages/app/src/screens/new-workspace-screen.tsx b/packages/app/src/screens/new-workspace-screen.tsx index a5a3e32795..93eaa9496e 100644 --- a/packages/app/src/screens/new-workspace-screen.tsx +++ b/packages/app/src/screens/new-workspace-screen.tsx @@ -50,7 +50,7 @@ import { navigateToWorkspace } from "@/stores/navigation-active-workspace-store" import { useLastWorkspaceSelection } from "@/stores/navigation-active-workspace-store"; import { normalizeWorkspaceDescriptor, useSessionStore } from "@/stores/session-store"; import { useWorkspace } from "@/stores/session-store-hooks"; -import { generateDraftId } from "@/stores/draft-keys"; +import { buildNewWorkspaceDraftKey, generateDraftId } from "@/stores/draft-keys"; import { useDraftStore } from "@/stores/draft-store"; import { useProjectPickerStore } from "@/stores/project-picker-store"; import { isActiveCreateFlowForDraft, useCreateFlowStore } from "@/stores/create-flow-store"; @@ -92,7 +92,11 @@ import { type PickerCheckoutRequest, type PickerItem, } from "./new-workspace-picker-item"; -import { findCheckoutHintPrAttachment, syncPickerPrAttachment } from "./new-workspace-picker-state"; +import { + clearPickerPrAttachmentForTargetChange, + findCheckoutHintPrAttachment, + syncPickerPrAttachment, +} from "./new-workspace-picker-state"; import { resolveNewWorkspaceAutomaticServerId, resolveNewWorkspaceInitialServerId, @@ -179,7 +183,6 @@ interface PickerOptionData { interface PickerSelection { item: PickerItem; - attachedPrNumber: number | null; } const BRANCH_OPTION_PREFIX = "branch:"; @@ -786,18 +789,6 @@ function getContentStyle(input: { isCompact: boolean; insetBottom: number }) { return [styles.content, styles.contentCentered]; } -function buildNewWorkspaceDraftKey(input: { - selectedServerId: string; - selectedSourceDirectory: string | null; - draftId?: string; -}): string { - const explicitDraftId = input.draftId?.trim(); - if (explicitDraftId) { - return `new-workspace:draft:${explicitDraftId}`; - } - return `new-workspace:${input.selectedServerId}:${input.selectedSourceDirectory ?? "choose-project"}`; -} - function getSelectedPickerItem(selection: PickerSelection | null): PickerItem | null { if (!selection) return null; return selection.item; @@ -1682,11 +1673,7 @@ export function NewWorkspaceScreen({ const projectIconDataByProjectKey = useProjectIconDataByProjectKey({ projects: projectIconTargets, }); - const draftKey = buildNewWorkspaceDraftKey({ - selectedServerId, - selectedSourceDirectory, - draftId, - }); + const draftKey = buildNewWorkspaceDraftKey(draftId); const forkDraftSetup = usePendingWorkspaceDraftSetup(draftId); const draftContextScopeKey = useDraftWorkspaceAttachmentScopeKey(draftId); const visibleDraftContextScopeKeys = useMemo( @@ -1801,22 +1788,16 @@ export function NewWorkspaceScreen({ }, [selectedItem]); const selectPickerItem = useCallback( (item: PickerItem) => { - const next = syncPickerPrAttachment({ + const nextAttachments = syncPickerPrAttachment({ attachments: chatDraft.attachments, - previousPickerPrNumber: manualPickerSelection?.attachedPrNumber ?? null, item, }); - setManualPickerSelection({ - item, - attachedPrNumber: next.attachedPrNumber, - }); - if (next.attachments !== chatDraft.attachments) { - chatDraft.setAttachments(next.attachments); - } + setManualPickerSelection({ item }); + chatDraft.setAttachments(nextAttachments); setPickerOpen(false); }, - [chatDraft, manualPickerSelection?.attachedPrNumber], + [chatDraft], ); const handleSelectOption = useCallback( @@ -1828,6 +1809,20 @@ export function NewWorkspaceScreen({ [itemById, selectPickerItem], ); + const clearManualPickerSelectionForTargetChange = useCallback( + (currentTargetId: string, nextTargetId: string) => { + const nextAttachments = clearPickerPrAttachmentForTargetChange({ + attachments: chatDraft.attachments, + currentTargetId, + nextTargetId, + }); + if (nextAttachments === chatDraft.attachments) return; + chatDraft.setAttachments(nextAttachments); + setManualPickerSelection(null); + }, + [chatDraft], + ); + const handleSelectProjectOption = useCallback( (id: string) => { // selectProjectOption enforces selectability (worktree-only when @@ -1835,9 +1830,17 @@ export function NewWorkspaceScreen({ // canCreateWorktree or non-git projects become unselectable. selectProjectOption(id); setProjectPickerOpen(false); - setManualPickerSelection(null); + clearManualPickerSelectionForTargetChange(selectedProjectOptionId, id); }, - [selectProjectOption], + [clearManualPickerSelectionForTargetChange, selectProjectOption, selectedProjectOptionId], + ); + + const handleSelectWorkspaceHost = useCallback( + (id: string) => { + handleSelectHost(id); + clearManualPickerSelectionForTargetChange(selectedServerId, id); + }, + [clearManualPickerSelectionForTargetChange, handleSelectHost, selectedServerId], ); const handleAddProject = useCallback(() => { @@ -2155,7 +2158,7 @@ export function NewWorkspaceScreen({ host: { allHosts, selectedServerId, - onSelect: handleSelectHost, + onSelect: handleSelectWorkspaceHost, openState: hostPickerOpen, onOpenChange: handleHostPickerOpenChange, anchorRef: hostPickerAnchorRef, diff --git a/packages/app/src/stores/draft-keys.ts b/packages/app/src/stores/draft-keys.ts index a9b497016d..68488600a1 100644 --- a/packages/app/src/stores/draft-keys.ts +++ b/packages/app/src/stores/draft-keys.ts @@ -1,9 +1,27 @@ import { generateMessageId } from "@/types/stream"; +export const NEW_WORKSPACE_DRAFT_KEY = "new-workspace"; +const NEW_WORKSPACE_FORK_DRAFT_PREFIX = `${NEW_WORKSPACE_DRAFT_KEY}:draft:`; + export function generateDraftId(): string { return `draft_${generateMessageId()}`; } +export function buildNewWorkspaceDraftKey(draftId?: string): string { + const explicitDraftId = draftId?.trim(); + if (explicitDraftId) { + return `${NEW_WORKSPACE_FORK_DRAFT_PREFIX}${explicitDraftId}`; + } + return NEW_WORKSPACE_DRAFT_KEY; +} + +export function isLegacyNewWorkspaceDraftKey(draftKey: string): boolean { + return ( + draftKey.startsWith(`${NEW_WORKSPACE_DRAFT_KEY}:`) && + !draftKey.startsWith(NEW_WORKSPACE_FORK_DRAFT_PREFIX) + ); +} + export function buildDraftStoreKey(input: { serverId: string; agentId: string; diff --git a/packages/app/src/stores/draft-store/migration.test.ts b/packages/app/src/stores/draft-store/migration.test.ts index 0ddee1fe0d..f9b4e98321 100644 --- a/packages/app/src/stores/draft-store/migration.test.ts +++ b/packages/app/src/stores/draft-store/migration.test.ts @@ -1,11 +1,60 @@ import { describe, expect, it } from "vitest"; -import type { ComposerAttachment } from "@/attachments/types"; +import type { ComposerAttachment, UserComposerAttachment } from "@/attachments/types"; import { migratePersistedState, type MigrateLegacyImages } from "./migration"; -import { isAttachmentMetadata } from "./state"; +import { isAttachmentMetadata, type DraftRecord } from "./state"; const passThroughMigrateLegacyImages: MigrateLegacyImages = async (images) => images.filter(isAttachmentMetadata); +function activeDraft( + text: string, + updatedAt: number, + attachments: UserComposerAttachment[] = [], +): DraftRecord { + return { + input: { text, attachments }, + lifecycle: "active", + updatedAt, + version: 1, + }; +} + +function githubIssueAttachment( + number: number, +): Extract { + return { + kind: "github_issue", + item: { + kind: "issue", + number, + title: `Review item ${number}`, + url: `https://example.com/issues/${number}`, + state: "open", + body: null, + labels: [], + }, + }; +} + +function githubPrAttachment( + number: number, +): Extract { + return { + kind: "github_pr", + item: { + kind: "pr", + number, + title: `Review item ${number}`, + url: `https://example.com/pulls/${number}`, + state: "open", + body: null, + labels: [], + baseRefName: "main", + headRefName: "feature/legacy", + }, + }; +} + function workspaceReviewAttachment(): Extract { return { kind: "review", @@ -47,6 +96,57 @@ function workspaceReviewAttachment(): Extract { + it("promotes the newest legacy New Workspace draft into the singleton surface", async () => { + const forkDraft = activeDraft("fork context", 1700000000003); + const agentDraft = activeDraft("agent prompt", 1700000000004); + + const migrated = await migratePersistedState( + { + drafts: { + "new-workspace:server-a:/project/older": activeDraft( + "older new workspace prompt", + 1700000000001, + ), + "new-workspace:server-b:/project/newer": activeDraft( + "newer new workspace prompt", + 1700000000002, + ), + "new-workspace:draft:fork-1": forkDraft, + "agent:server-a:agent-1": agentDraft, + }, + createModalDraft: null, + }, + { migrateLegacyImages: passThroughMigrateLegacyImages, nowMs: 1700000000005 }, + ); + + expect(migrated.drafts).toEqual({ + "new-workspace": activeDraft("newer new workspace prompt", 1700000000002), + "new-workspace:draft:fork-1": forkDraft, + "agent:server-a:agent-1": agentDraft, + }); + }); + + it("drops unowned checkout PR context when promoting a scoped New Workspace draft", async () => { + const issue = githubIssueAttachment(101); + const migrated = await migratePersistedState( + { + drafts: { + "new-workspace:server-a:/project/a": activeDraft("keep the prompt", 2, [ + issue, + githubPrAttachment(202), + ]), + }, + createModalDraft: null, + }, + { migrateLegacyImages: passThroughMigrateLegacyImages, nowMs: 3 }, + ); + + expect(migrated.drafts["new-workspace"]?.input).toEqual({ + text: "keep the prompt", + attachments: [issue], + }); + }); + it("normalizes legacy image metadata into image attachments and strips persisted preview URLs", async () => { const migrated = await migratePersistedState( { diff --git a/packages/app/src/stores/draft-store/migration.ts b/packages/app/src/stores/draft-store/migration.ts index 015c1ca824..fc21087ca2 100644 --- a/packages/app/src/stores/draft-store/migration.ts +++ b/packages/app/src/stores/draft-store/migration.ts @@ -1,4 +1,5 @@ import type { AttachmentMetadata, UserComposerAttachment } from "@/attachments/types"; +import { isLegacyNewWorkspaceDraftKey, NEW_WORKSPACE_DRAFT_KEY } from "@/stores/draft-keys"; import { isAttachmentMetadata, isLegacyDraftImage, @@ -98,6 +99,46 @@ async function buildMigratedDraftRecord( }; } +function migrateNewWorkspaceDraftKeys( + drafts: Record, +): Record { + const legacyEntries = Object.entries(drafts).filter(([draftKey]) => + isLegacyNewWorkspaceDraftKey(draftKey), + ); + if (legacyEntries.length === 0) { + return drafts; + } + + const nextDrafts = { ...drafts }; + for (const [draftKey] of legacyEntries) { + delete nextDrafts[draftKey]; + } + + if (nextDrafts[NEW_WORKSPACE_DRAFT_KEY]) { + return nextDrafts; + } + + const newestActiveDraft = legacyEntries + .map(([, draft]) => draft) + .filter((draft) => draft.lifecycle === "active") + .sort((left, right) => right.updatedAt - left.updatedAt)[0]; + if (newestActiveDraft) { + // Legacy scoped drafts did not record whether a PR came from the Base + // picker. That checkout context is unsafe to carry onto a global surface. + nextDrafts[NEW_WORKSPACE_DRAFT_KEY] = { + ...newestActiveDraft, + input: { + ...newestActiveDraft.input, + attachments: newestActiveDraft.input.attachments.filter( + (attachment) => attachment.kind !== "github_pr", + ), + }, + }; + } + + return nextDrafts; +} + export async function migratePersistedState( state: unknown, ports: { migrateLegacyImages: MigrateLegacyImages; nowMs: number }, @@ -129,7 +170,8 @@ export async function migratePersistedState( } return { - drafts: nextDrafts, + // COMPAT(newWorkspaceDraftSingleton): migrated in v0.1.108; remove after 2027-01-13. + drafts: migrateNewWorkspaceDraftKeys(nextDrafts), createModalDraft, }; } diff --git a/packages/app/src/stores/draft-store/state.test.ts b/packages/app/src/stores/draft-store/state.test.ts index 996292d136..16aad750e2 100644 --- a/packages/app/src/stores/draft-store/state.test.ts +++ b/packages/app/src/stores/draft-store/state.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { applyClearDraftRecord, pruneFinalizedDraftRecords } from "./state"; +import { applyClearDraftRecord, pruneFinalizedDraftRecords, toDraftInputIfReady } from "./state"; describe("draft-store lifecycle", () => { it("prunes finalized tombstones after TTL", () => { @@ -69,3 +69,35 @@ describe("draft-store lifecycle", () => { }); }); }); + +describe("draft-store normalization", () => { + it("preserves New Workspace picker ownership when hydrating a draft", () => { + const pickerAttachment = { + kind: "github_pr" as const, + owner: "new-workspace-picker" as const, + item: { + kind: "pr" as const, + number: 202, + title: "Persist picker ownership", + url: "https://example.com/pull/202", + state: "open" as const, + body: null, + labels: [], + baseRefName: "main", + headRefName: "feature/picker-ownership", + }, + }; + + expect( + toDraftInputIfReady({ + input: { text: "Keep this prompt", attachments: [pickerAttachment] }, + lifecycle: "active", + updatedAt: 1, + version: 1, + }), + ).toEqual({ + text: "Keep this prompt", + attachments: [pickerAttachment], + }); + }); +}); diff --git a/packages/app/src/stores/draft-store/state.ts b/packages/app/src/stores/draft-store/state.ts index 647fc59eb6..cfed72b6ad 100644 --- a/packages/app/src/stores/draft-store/state.ts +++ b/packages/app/src/stores/draft-store/state.ts @@ -1,7 +1,11 @@ -import type { AttachmentMetadata, UserComposerAttachment } from "@/attachments/types"; +import { + NEW_WORKSPACE_PICKER_ATTACHMENT_OWNER, + type AttachmentMetadata, + type UserComposerAttachment, +} from "@/attachments/types"; import { GitHubSearchItemSchema } from "@getpaseo/protocol/messages"; -export const DRAFT_STORE_VERSION = 4; +export const DRAFT_STORE_VERSION = 5; export const FINALIZED_DRAFT_TTL_MS = 5 * 60 * 1000; export interface LegacyDraftImage { @@ -82,6 +86,13 @@ export function isUserComposerAttachment(value: unknown): value is UserComposerA if (record.kind !== "github_issue" && record.kind !== "github_pr") { return false; } + if ( + record.kind === "github_pr" && + record.owner !== undefined && + record.owner !== NEW_WORKSPACE_PICKER_ATTACHMENT_OWNER + ) { + return false; + } return GitHubSearchItemSchema.safeParse(record.item).success; } @@ -94,6 +105,15 @@ export function normalizeComposerAttachment( metadata: normalizeAttachmentMetadata(attachment.metadata), }; } + if (attachment.kind === "github_pr") { + return { + kind: "github_pr", + item: attachment.item, + ...(attachment.owner === NEW_WORKSPACE_PICKER_ATTACHMENT_OWNER + ? { owner: attachment.owner } + : {}), + }; + } return attachment; } From fe9a486ac148c17a21e6464f1e9bc81ecf956502 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 13 Jul 2026 15:37:28 +0200 Subject: [PATCH 2/4] Show every Codex terminal command in agent timelines (#2037) * fix(codex): surface silent terminal activity Successful no-output commands use a null output field, while terminal interactions carry their stdin separately. Preserve both so the agent timeline remains auditable. * refactor(tests): share Codex event waiter * fix(codex): preserve bounded terminal audit details Keep terminal stdin through late command relabeling, cap it with other timeline content, and accept nullable output from legacy live completion events. * fix(codex): preserve repeated terminal writes --- .../agent/agent-timeline-content.test.ts | 35 +++ .../server/agent/agent-timeline-content.ts | 19 ++ .../providers/codex-app-server-agent.test.ts | 211 ++++++++++++++++++ .../agent/providers/codex-app-server-agent.ts | 60 +++-- .../codex/test-utils/fake-app-server.ts | 89 +++++++- .../agent/providers/codex/tool-call-mapper.ts | 4 +- 6 files changed, 393 insertions(+), 25 deletions(-) create mode 100644 packages/server/src/server/agent/agent-timeline-content.test.ts diff --git a/packages/server/src/server/agent/agent-timeline-content.test.ts b/packages/server/src/server/agent/agent-timeline-content.test.ts new file mode 100644 index 0000000000..f3b94e248e --- /dev/null +++ b/packages/server/src/server/agent/agent-timeline-content.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, test } from "vitest"; + +import { limitAgentTimelineItemContent } from "./agent-timeline-content.js"; + +describe("agent timeline content", () => { + test("limits terminal input to the tool-call content budget", () => { + const oversizedInput = "x".repeat(64 * 1024 + 1); + + const item = limitAgentTimelineItemContent({ + type: "tool_call", + callId: "terminal-session-4242", + name: "terminal", + status: "completed", + error: null, + detail: { + type: "plain_text", + text: oversizedInput, + icon: "square_terminal", + }, + }); + + expect(item).toEqual({ + type: "tool_call", + callId: "terminal-session-4242", + name: "terminal", + status: "completed", + error: null, + detail: { + type: "plain_text", + text: "x".repeat(64 * 1024), + icon: "square_terminal", + }, + }); + }); +}); diff --git a/packages/server/src/server/agent/agent-timeline-content.ts b/packages/server/src/server/agent/agent-timeline-content.ts index 5bdc17e0af..ab436152e9 100644 --- a/packages/server/src/server/agent/agent-timeline-content.ts +++ b/packages/server/src/server/agent/agent-timeline-content.ts @@ -24,8 +24,27 @@ function limitFailedShellError(item: AgentTimelineItem): AgentTimelineItem { }; } +function limitPlainText(item: AgentTimelineItem): AgentTimelineItem { + if ( + item.type !== "tool_call" || + item.detail.type !== "plain_text" || + typeof item.detail.text !== "string" || + item.detail.text.length <= TOOL_CALL_CONTENT_MAX_LENGTH + ) { + return item; + } + return { + ...item, + detail: { + ...item.detail, + text: item.detail.text.slice(0, TOOL_CALL_CONTENT_MAX_LENGTH), + }, + }; +} + export function limitAgentTimelineItemContent(item: AgentTimelineItem): AgentTimelineItem { item = limitFailedShellError(item); + item = limitPlainText(item); if ( item.type !== "tool_call" || item.detail.type !== "shell" || diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index 5522feaa8c..8f77da542d 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -31,6 +31,8 @@ import { createFakeCodexAppServer, type FakeCodexAppServer, waitForNextPermission, + waitForNextTimelineItem, + waitForTimelineToolCall, } from "./codex/test-utils/fake-app-server.js"; import { createTestLogger } from "../../../test-utils/test-logger.js"; import { asInternals as castInternals, createStub } from "../../test-utils/class-mocks.js"; @@ -554,6 +556,215 @@ describe("Codex app-server provider", () => { await session.close(); }); + test("shows a successful shell command that produces no output", async () => { + const appServer = createFakeCodexAppServer(); + const session = new CodexAppServerAgentSession( + createConfig({ cwd: "/workspace/project" }), + null, + createTestLogger(), + async () => appServer.child, + ); + + try { + await session.connect(); + const nextTimelineItem = waitForNextTimelineItem(session); + + appServer.completesSilentCommand({ + threadId: "thread-1", + callId: "silent-merge", + command: "gh pr merge 2030 --squash", + cwd: "/workspace/project", + }); + appServer.says({ threadId: "thread-1", text: "Merged." }); + + await expect(nextTimelineItem).resolves.toEqual({ + type: "timeline", + provider: "codex", + item: { + type: "tool_call", + callId: "silent-merge", + name: "shell", + status: "completed", + error: null, + detail: { + type: "shell", + command: "gh pr merge 2030 --squash", + cwd: "/workspace/project", + exitCode: 0, + }, + }, + }); + appServer.assertNoErrors(); + } finally { + await session.close(); + } + }); + + test("shows a silent shell command from legacy live notifications", async () => { + const appServer = createFakeCodexAppServer(); + const session = new CodexAppServerAgentSession( + createConfig({ cwd: "/workspace/project" }), + null, + createTestLogger(), + async () => appServer.child, + ); + + try { + await session.connect(); + const nextTimelineItem = waitForNextTimelineItem(session); + + appServer.completesSilentLegacyCommand({ + threadId: "thread-1", + callId: "legacy-silent-merge", + command: "gh pr merge 2030 --squash", + cwd: "/workspace/project", + }); + + await expect(nextTimelineItem).resolves.toEqual({ + type: "timeline", + provider: "codex", + item: { + type: "tool_call", + callId: "legacy-silent-merge", + name: "shell", + status: "completed", + error: null, + detail: { + type: "shell", + command: "gh pr merge 2030 --squash", + cwd: "/workspace/project", + exitCode: 0, + }, + }, + }); + appServer.assertNoErrors(); + } finally { + await session.close(); + } + }); + + test("shows the exact bytes Codex writes into an existing terminal", async () => { + const appServer = createFakeCodexAppServer(); + const session = new CodexAppServerAgentSession( + createConfig({ cwd: "/workspace/project" }), + null, + createTestLogger(), + async () => appServer.child, + ); + + try { + await session.connect(); + const nextTimelineItem = waitForNextTimelineItem(session); + + appServer.typesIntoTerminal({ + threadId: "thread-1", + turnId: "turn-1", + itemId: "interactive-shell", + processId: "4242", + text: "gh pr merge 2030 --squash\n", + }); + + await expect(nextTimelineItem).resolves.toEqual({ + type: "timeline", + provider: "codex", + item: { + type: "tool_call", + callId: "terminal-session-4242-1", + name: "terminal", + status: "completed", + error: null, + detail: { + type: "plain_text", + text: "gh pr merge 2030 --squash\n", + icon: "square_terminal", + }, + metadata: { + processId: "4242", + }, + }, + }); + + const relabeledTerminal = waitForTimelineToolCall(session, "terminal-session-4242-1"); + appServer.runsLegacyCommand({ + threadId: "thread-1", + callId: "interactive-shell", + command: "sleep 30", + output: "Process running with session id 4242", + }); + + await expect(relabeledTerminal).resolves.toEqual({ + type: "timeline", + provider: "codex", + item: { + type: "tool_call", + callId: "terminal-session-4242-1", + name: "terminal", + status: "completed", + error: null, + detail: { + type: "plain_text", + label: "sleep 30", + text: "gh pr merge 2030 --squash\n", + icon: "square_terminal", + }, + metadata: { + processId: "4242", + }, + }, + }); + appServer.assertNoErrors(); + } finally { + await session.close(); + } + }); + + test("keeps repeated writes to one terminal as separate timeline rows", async () => { + const appServer = createFakeCodexAppServer(); + const session = new CodexAppServerAgentSession( + createConfig({ cwd: "/workspace/project" }), + null, + createTestLogger(), + async () => appServer.child, + ); + + try { + await session.connect(); + + const firstTimelineItem = waitForNextTimelineItem(session); + appServer.typesIntoTerminal({ + threadId: "thread-1", + turnId: "turn-1", + itemId: "interactive-shell", + processId: "4242", + text: "git status\n", + }); + + const secondTimelineItem = waitForNextTimelineItem(session); + appServer.typesIntoTerminal({ + threadId: "thread-1", + turnId: "turn-1", + itemId: "interactive-shell", + processId: "4242", + text: "git push\n", + }); + + const [first, second] = await Promise.all([firstTimelineItem, secondTimelineItem]); + expect(first.item).toMatchObject({ + type: "tool_call", + callId: "terminal-session-4242-1", + detail: { type: "plain_text", text: "git status\n" }, + }); + expect(second.item).toMatchObject({ + type: "tool_call", + callId: "terminal-session-4242-2", + detail: { type: "plain_text", text: "git push\n" }, + }); + appServer.assertNoErrors(); + } finally { + await session.close(); + } + }); + test("surfaces an MCP elicitation and returns Codex's required approval action", async () => { const appServer = createFakeCodexAppServer(); const session = new CodexAppServerAgentSession( diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index aa23cf47fe..dc77fdb7ec 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -1514,24 +1514,23 @@ export function mapCodexPatchNotificationToToolCall(params: { } function mapCodexTerminalInteractionToToolCall(params: { + callId: string; processId?: string | null; - fallbackCallId?: string | null; command?: string | null; + stdin?: string | null; }): ToolCallTimelineItem { const processId = nonEmptyString(params.processId ?? undefined); - const callId = processId - ? `terminal-session-${processId}` - : (nonEmptyString(params.fallbackCallId ?? undefined) ?? "terminal-interaction"); const label = nonEmptyString(params.command ?? undefined); return { type: "tool_call", - callId, + callId: params.callId, name: "terminal", status: "completed", error: null, detail: { type: "plain_text", ...(label ? { label } : {}), + ...(params.stdin !== null && params.stdin !== undefined ? { text: params.stdin } : {}), icon: "square_terminal", }, ...(processId ? { metadata: { processId } } : {}), @@ -2111,8 +2110,8 @@ const CodexEventExecCommandEndNotificationSchema = z cwd: z.string().optional(), stdout: z.string().optional(), stderr: z.string().optional(), - aggregated_output: z.string().optional(), - aggregatedOutput: z.string().optional(), + aggregated_output: z.string().nullable().optional(), + aggregatedOutput: z.string().nullable().optional(), formatted_output: z.string().optional(), exit_code: z.number().nullable().optional(), exitCode: z.number().nullable().optional(), @@ -3102,7 +3101,11 @@ export class CodexAppServerAgentSession implements AgentSession { private pendingFileChangeOutputDeltas = new Map(); private pendingAssistantMessageBoundary = false; private terminalCommandByProcessId = new Map(); - private pendingUnlabeledTerminalInteractions = new Set(); + private pendingUnlabeledTerminalInteractions = new Map< + string, + Array<{ callId: string; stdin: string | null }> + >(); + private nextTerminalInteractionOrdinal = 0; private emittedTerminalInteractionKeys = new Set(); private emittedExecCommandStartedCallIds = new Set(); private emittedExecCommandCompletedCallIds = new Set(); @@ -5447,13 +5450,18 @@ export class CodexAppServerAgentSession implements AgentSession { const command = (parsed.processId ? this.terminalCommandByProcessId.get(parsed.processId) : undefined) ?? null; + const callId = this.createTerminalInteractionCallId(parsed.processId, parsed.callId); if (!command && parsed.processId) { - this.pendingUnlabeledTerminalInteractions.add(parsed.processId); + const pendingInteractions = + this.pendingUnlabeledTerminalInteractions.get(parsed.processId) ?? []; + pendingInteractions.push({ callId, stdin: parsed.stdin }); + this.pendingUnlabeledTerminalInteractions.set(parsed.processId, pendingInteractions); } const timelineItem = mapCodexTerminalInteractionToToolCall({ + callId, processId: parsed.processId, - fallbackCallId: parsed.callId, command, + stdin: parsed.stdin, }); this.emitEvent({ type: "timeline", provider: CODEX_PROVIDER, item: timelineItem }); } @@ -5862,15 +5870,31 @@ export class CodexAppServerAgentSession implements AgentSession { if (!this.pendingUnlabeledTerminalInteractions.has(processId)) { return; } + const pendingInteractions = this.pendingUnlabeledTerminalInteractions.get(processId) ?? []; this.pendingUnlabeledTerminalInteractions.delete(processId); - this.emitEvent({ - type: "timeline", - provider: CODEX_PROVIDER, - item: mapCodexTerminalInteractionToToolCall({ - processId, - command: displayCommand, - }), - }); + for (const pendingInteraction of pendingInteractions) { + this.emitEvent({ + type: "timeline", + provider: CODEX_PROVIDER, + item: mapCodexTerminalInteractionToToolCall({ + callId: pendingInteraction.callId, + processId, + command: displayCommand, + stdin: pendingInteraction.stdin, + }), + }); + } + } + + private createTerminalInteractionCallId( + processId: string | null, + fallbackCallId: string | null, + ): string { + const baseCallId = processId + ? `terminal-session-${processId}` + : (nonEmptyString(fallbackCallId ?? undefined) ?? "terminal-interaction"); + this.nextTerminalInteractionOrdinal += 1; + return `${baseCallId}-${this.nextTerminalInteractionOrdinal}`; } private shouldEmitTerminalInteractionKey(key: string): boolean { diff --git a/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts b/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts index 8302e6d932..9ef0c354ee 100644 --- a/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts +++ b/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts @@ -19,6 +19,19 @@ interface FakeLegacyCommand { command: string; output: string; } +interface FakeSilentCommand { + threadId: string; + callId: string; + command: string; + cwd: string; +} +interface FakeTerminalInput { + threadId: string; + turnId: string; + itemId: string; + processId: string; + text: string; +} interface FakeLegacyPatch { threadId: string; callId: string; @@ -51,6 +64,9 @@ export interface FakeCodexAppServer { runsLegacyCommand(params: FakeLegacyCommand): void; appliesLegacyPatch(params: FakeLegacyPatch): void; completesCommand(params: FakeLegacyCommand): void; + completesSilentCommand(params: FakeSilentCommand): void; + completesSilentLegacyCommand(params: FakeSilentCommand): void; + typesIntoTerminal(params: FakeTerminalInput): void; says(params: { threadId: string; itemId?: string; text: string; chunks?: string[] }): void; requestCommandApproval(params: { itemId: string; @@ -366,6 +382,37 @@ export function createFakeCodexAppServer( exitCode: 0, }); }, + completesSilentCommand(params) { + completeItem(params.threadId, { + type: "commandExecution", + id: params.callId, + status: "completed", + command: params.command, + cwd: params.cwd, + aggregatedOutput: null, + exitCode: 0, + }); + }, + completesSilentLegacyCommand(params) { + writeLegacyEvent(params.threadId, "codex/event/exec_command_end", { + type: "exec_command_end", + call_id: params.callId, + command: params.command, + cwd: params.cwd, + aggregatedOutput: null, + exit_code: 0, + success: true, + }); + }, + typesIntoTerminal(params) { + writeNotification("item/commandExecution/terminalInteraction", { + threadId: params.threadId, + turnId: params.turnId, + itemId: params.itemId, + processId: params.processId, + stdin: params.text, + }); + }, says(params) { if (params.itemId) { for (const chunk of params.chunks ?? [params.text]) { @@ -458,21 +505,53 @@ function toJsonObject(value: unknown): JsonObject { return {}; } -export function waitForNextPermission( +type StreamEventType = AgentStreamEvent["type"]; +type StreamEventOfType = Extract; + +function waitForNextEvent( session: AgentSession, -): Promise> { + type: TType, + accepts?: (event: StreamEventOfType) => boolean, +): Promise> { return new Promise((resolve, reject) => { const timeout = setTimeout(() => { unsubscribe(); - reject(new Error("Timed out waiting for permission_requested")); + reject(new Error(`Timed out waiting for ${type}`)); }, 1000); const unsubscribe = session.subscribe((event) => { - if (event.type !== "permission_requested") { + if (event.type !== type) { + return; + } + const typedEvent = event as StreamEventOfType; + if (accepts && !accepts(typedEvent)) { return; } clearTimeout(timeout); unsubscribe(); - resolve(event); + resolve(typedEvent); }); }); } + +type TimelineEvent = StreamEventOfType<"timeline">; + +export function waitForNextPermission( + session: AgentSession, +): Promise> { + return waitForNextEvent(session, "permission_requested"); +} + +export function waitForNextTimelineItem(session: AgentSession): Promise { + return waitForNextEvent(session, "timeline"); +} + +export function waitForTimelineToolCall( + session: AgentSession, + callId: string, +): Promise { + return waitForNextEvent( + session, + "timeline", + (event) => event.item.type === "tool_call" && event.item.callId === callId, + ); +} diff --git a/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts b/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts index 5f488cc4ad..967b6b43aa 100644 --- a/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts +++ b/packages/server/src/server/agent/providers/codex/tool-call-mapper.ts @@ -141,7 +141,7 @@ const CodexCommandExecutionItemSchema = z error: z.unknown().optional(), command: CodexCommandValueSchema.optional(), cwd: z.string().optional(), - aggregatedOutput: z.string().optional(), + aggregatedOutput: z.string().nullable().optional(), exitCode: z.number().nullable().optional(), }) .passthrough(); @@ -694,7 +694,7 @@ function mapCommandExecutionItem( item: z.infer, ): CodexNormalizedToolCallEnvelope { const command = normalizeCommandExecutionCommand(item.command); - const parsedOutput = extractCodexShellOutput(item.aggregatedOutput); + const parsedOutput = extractCodexShellOutput(item.aggregatedOutput ?? undefined); const input = toNullableObject({ ...(command !== undefined ? { command } : {}), ...(item.cwd !== undefined ? { cwd: item.cwd } : {}), From f350c716b9fe671233b7466935e64449cb7875de Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 13 Jul 2026 15:38:09 +0200 Subject: [PATCH 3/4] Keep forked chats focused in new tabs (#2038) * fix(app): focus explicitly opened workspace tabs Explicit tab navigation could be overwritten by attention-aware workspace navigation when the visible agent had just finished. Make workspace navigation own both normal attention selection and explicit targets so callers cannot sequence them inconsistently. * test(app): strengthen forked tab focus assertion --- docs/expo-router.md | 12 +++- packages/app/e2e/assistant-fork-menu.spec.ts | 23 +++++-- packages/app/src/agent-stream/view.tsx | 6 +- .../src/components/sidebar-workspace-list.tsx | 2 +- .../sidebar/sidebar-status-list.tsx | 2 +- .../src/components/workspace-setup-dialog.tsx | 4 +- .../app/src/hooks/use-keyboard-shortcuts.ts | 2 +- .../app/src/screens/new-workspace-screen.tsx | 11 ++-- .../index.ts | 13 ++-- .../navigation.test.ts | 63 ++++++++++++++++--- .../navigation.ts | 58 ++++++++++------- .../app/src/utils/navigate-to-agent/index.ts | 4 +- .../utils/navigate-to-agent/resolve.test.ts | 6 +- .../src/utils/navigate-to-agent/resolve.ts | 6 +- .../restore-archived-workspace.test.ts | 4 +- .../app/src/utils/prepare-workspace-tab.ts | 20 +----- .../src/utils/workspace-navigation.test.ts | 40 +----------- .../app/src/utils/workspace-navigation.ts | 19 +----- 18 files changed, 156 insertions(+), 139 deletions(-) diff --git a/docs/expo-router.md b/docs/expo-router.md index 14344a3f16..df3aa2c5bc 100644 --- a/docs/expo-router.md +++ b/docs/expo-router.md @@ -48,8 +48,13 @@ dynamic params exist before any nested workspace leaf is selected. ## App-Wide Route Hops When app-wide routes such as `/new`, `/settings`, or `/sessions` navigate back -into a host workspace, express only the destination with `navigateToWorkspace()`. -Do not make the caller branch on its current route. +into a host workspace, use `navigateToWorkspace()`. Do not make the caller +branch on its current route. + +Pass only `serverId` and `workspaceId` for normal attention-aware navigation. +When the action names a specific tab, pass it as `target`; that explicit choice +is authoritative. Callers should not choose between separate route and tab +navigation APIs. The root stack owns `h/[serverId]`; the host stack owns `workspace/[workspaceId]/index`. Repeated global-route hops must `POP_TO` the @@ -133,7 +138,8 @@ Before landing route changes: - [ ] Did you change `packages/app/src/app`? Re-read this file. - [ ] Did you touch remembered workspace restore? Keep root on `/h/[serverId]`. -- [ ] Did any route return to a workspace? Use `navigateToWorkspace()`. +- [ ] Did a route return to a workspace? Use `navigateToWorkspace()` and pass a + `target` when the action names a specific tab. - [ ] Did you add a route? Register it in the layout that directly owns it. - [ ] Did `useLocalSearchParams()` lose a required param? Fix the route tree. - [ ] Did native show a blank screen without a crash? Suspect route ownership diff --git a/packages/app/e2e/assistant-fork-menu.spec.ts b/packages/app/e2e/assistant-fork-menu.spec.ts index c09a90921f..47a26f531f 100644 --- a/packages/app/e2e/assistant-fork-menu.spec.ts +++ b/packages/app/e2e/assistant-fork-menu.spec.ts @@ -54,14 +54,14 @@ async function expectChatHistoryPill(page: Page): Promise { test.describe("Assistant fork menu", () => { test.describe.configure({ timeout: 180_000 }); - test("forks an assistant turn into a new workspace draft tab", async ({ + test("focuses a forked assistant turn in a new workspace draft tab", async ({ page, seedForkWorkspace, }) => { const session = await seedForkWorkspace({ - repoPrefix: "assistant-fork-tab-", - title: "Assistant fork tab", - initialPrompt: "emit 1 coalesced agent stream updates for assistant fork tab.", + repoPrefix: "assistant-fork-focused-tab-", + title: "Assistant fork focused tab", + initialPrompt: "emit 1 coalesced agent stream updates for initial assistant fork turn.", model: "ten-second-stream", }); @@ -70,9 +70,24 @@ test.describe("Assistant fork menu", () => { await awaitAssistantMessage(page); await session.client.waitForFinish(session.agentId, 45_000); + await submitMessage(page, "emit 1 coalesced agent stream updates while this tab is visible."); + await session.client.waitForFinish(session.agentId, 45_000); + await awaitAssistantMessage(page); + + const agentTab = page.getByTestId(`workspace-tab-agent_${session.agentId}`); + await expect(agentTab).toHaveAttribute("aria-selected", "true"); + await openAssistantForkMenu(page); await page.getByTestId("assistant-fork-menu-new-tab").click(); + const selectedTab = page + .getByTestId("workspace-tabs-row") + .getByRole("button") + .and(page.locator('[aria-selected="true"]')); + await expect(selectedTab).toHaveAttribute("data-testid", /^workspace-tab-draft_/, { + timeout: 30_000, + }); + await expect(agentTab).toHaveAttribute("aria-selected", "false"); await expectChatHistoryPill(page); }); diff --git a/packages/app/src/agent-stream/view.tsx b/packages/app/src/agent-stream/view.tsx index b5edb8a2d1..446ddee5c0 100644 --- a/packages/app/src/agent-stream/view.tsx +++ b/packages/app/src/agent-stream/view.tsx @@ -81,7 +81,7 @@ import { type OpenFileDisposition, type WorkspaceFileOpenRequest, } from "@/workspace/file-open"; -import { navigateToPreparedWorkspaceTab } from "@/utils/workspace-navigation"; +import { navigateToWorkspace } from "@/stores/navigation-active-workspace-store"; import { buildNewWorkspaceRoute } from "@/utils/host-routes"; import { useStableEvent } from "@/hooks/use-stable-event"; import { isWeb } from "@/constants/platform"; @@ -423,7 +423,7 @@ const AgentStreamViewComponent = forwardRef { if (!workspace.serverId) return; onWorkspacePress?.(); - navigateToWorkspace(workspace.serverId, workspace.workspaceId); + navigateToWorkspace({ serverId: workspace.serverId, workspaceId: workspace.workspaceId }); }, [onWorkspacePress, workspace.serverId, workspace.workspaceId]); return ( diff --git a/packages/app/src/components/workspace-setup-dialog.tsx b/packages/app/src/components/workspace-setup-dialog.tsx index 90eef69068..0a539a4396 100644 --- a/packages/app/src/components/workspace-setup-dialog.tsx +++ b/packages/app/src/components/workspace-setup-dialog.tsx @@ -26,7 +26,7 @@ import type { import { projectIconPlaceholderLabelFromDisplayName } from "@/utils/project-display-name"; import { requireWorkspaceDirectory } from "@/utils/workspace-directory"; import { navigateToAgent } from "@/utils/navigate-to-agent"; -import { navigateToPreparedWorkspaceTab } from "@/utils/workspace-navigation"; +import { navigateToWorkspace } from "@/stores/navigation-active-workspace-store"; import type { MessagePayload } from "@/composer/types"; function toProjectIconDataUri(icon: { mimeType: string; data: string } | null): string | null { @@ -218,7 +218,7 @@ export function WorkspaceSetupDialog() { return; } - navigateToPreparedWorkspaceTab({ + navigateToWorkspace({ serverId: pendingWorkspaceSetup.serverId, workspaceId, target, diff --git a/packages/app/src/hooks/use-keyboard-shortcuts.ts b/packages/app/src/hooks/use-keyboard-shortcuts.ts index 0f427c573e..67c81bc548 100644 --- a/packages/app/src/hooks/use-keyboard-shortcuts.ts +++ b/packages/app/src/hooks/use-keyboard-shortcuts.ts @@ -120,7 +120,7 @@ export function useKeyboardShortcuts({ serverId: action.serverId, workspaceId: action.workspaceId, }; - navigateToWorkspace(action.serverId, action.workspaceId); + navigateToWorkspace({ serverId: action.serverId, workspaceId: action.workspaceId }); return true; case "navigate-last-workspace": return navigateToLastWorkspace(); diff --git a/packages/app/src/screens/new-workspace-screen.tsx b/packages/app/src/screens/new-workspace-screen.tsx index 93eaa9496e..b4034e81b4 100644 --- a/packages/app/src/screens/new-workspace-screen.tsx +++ b/packages/app/src/screens/new-workspace-screen.tsx @@ -46,8 +46,10 @@ import { } from "@/runtime/host-runtime"; import { useHostFeature, useHostFeatureMap } from "@/runtime/host-features"; import type { HostProfile } from "@/types/host-connection"; -import { navigateToWorkspace } from "@/stores/navigation-active-workspace-store"; -import { useLastWorkspaceSelection } from "@/stores/navigation-active-workspace-store"; +import { + navigateToWorkspace, + useLastWorkspaceSelection, +} from "@/stores/navigation-active-workspace-store"; import { normalizeWorkspaceDescriptor, useSessionStore } from "@/stores/session-store"; import { useWorkspace } from "@/stores/session-store-hooks"; import { buildNewWorkspaceDraftKey, generateDraftId } from "@/stores/draft-keys"; @@ -65,7 +67,6 @@ import type { CreateAgentInitialValues } from "@/hooks/use-agent-form-state"; import { generateMessageId } from "@/types/stream"; import { toErrorMessage } from "@/utils/error-messages"; import { projectIconPlaceholderLabelFromDisplayName } from "@/utils/project-display-name"; -import { navigateToPreparedWorkspaceTab } from "@/utils/workspace-navigation"; import { getHostProjectSourceDirectory, hostProjectFromRoute, @@ -1172,7 +1173,7 @@ function submitWorkspaceDraft(input: SubmitDraftInput): void { ...(submission.featureValues ? { featureValues: submission.featureValues } : {}), allowEmptyText: true, }); - navigateToPreparedWorkspaceTab({ + navigateToWorkspace({ serverId, workspaceId, target: submission.target, @@ -2044,7 +2045,7 @@ export function NewWorkspaceScreen({ ensureWorkspace, serverId: selectedServerId, navigate: (targetServerId, workspaceId) => - navigateToWorkspace(targetServerId, workspaceId), + navigateToWorkspace({ serverId: targetServerId, workspaceId }), }); return; } diff --git a/packages/app/src/stores/navigation-active-workspace-store/index.ts b/packages/app/src/stores/navigation-active-workspace-store/index.ts index dc4ebedb45..b3358b3a14 100644 --- a/packages/app/src/stores/navigation-active-workspace-store/index.ts +++ b/packages/app/src/stores/navigation-active-workspace-store/index.ts @@ -11,6 +11,7 @@ import { navigateToLastWorkspace as navigateToLastWorkspacePure, navigateToWorkspace as navigateToWorkspacePure, parseActiveWorkspaceSelection, + type NavigateToWorkspaceInput, type NavigateToWorkspaceDeps, } from "./navigation"; import { useSessionStore } from "@/stores/session-store"; @@ -19,6 +20,7 @@ import { stripHostWorkspaceRouteEchoSearchFromBrowserUrlAfterCommit } from "@/ut import { navigateToHostWorkspaceRoute } from "@/navigation/workspace-route-navigation"; export type { ActiveWorkspaceSelection } from "@/stores/last-workspace-selection"; +export type { NavigateToWorkspaceInput } from "./navigation"; const lastWorkspaceSelectionStorage: LastWorkspaceSelectionStorage = { read: () => AsyncStorage.getItem(LAST_WORKSPACE_SELECTION_STORAGE_KEY), @@ -34,9 +36,10 @@ function navigateDeps(): NavigateToWorkspaceDeps { getSessionWorkspaces: (serverId) => useSessionStore.getState().sessions[serverId]?.workspaces, getSessionAgents: (serverId) => useSessionStore.getState().sessions[serverId]?.agents.values() ?? [], - openWorkspaceAgentTab: (workspaceKey, agentId) => { - useWorkspaceLayoutStore.getState().openTabFocused(workspaceKey, { kind: "agent", agentId }); - }, + openTabFocused: (workspaceKey, target) => + useWorkspaceLayoutStore.getState().openTabFocused(workspaceKey, target), + pinAgent: (workspaceKey, agentId) => + useWorkspaceLayoutStore.getState().pinAgent(workspaceKey, agentId), rememberLastWorkspace: (selection) => lastWorkspaceSelectionStore.remember(selection), navigateToRoute: (route) => { navigateToHostWorkspaceRoute(route); @@ -57,8 +60,8 @@ export function getIsLastWorkspaceSelectionHydrated(): boolean { return lastWorkspaceSelectionStore.isHydrated(); } -export function navigateToWorkspace(serverId: string, workspaceId: string) { - navigateToWorkspacePure(serverId, workspaceId, navigateDeps()); +export function navigateToWorkspace(input: NavigateToWorkspaceInput): string { + return navigateToWorkspacePure(input, navigateDeps()); } export function navigateToLastWorkspace(): boolean { diff --git a/packages/app/src/stores/navigation-active-workspace-store/navigation.test.ts b/packages/app/src/stores/navigation-active-workspace-store/navigation.test.ts index e2f8aab933..719948ee90 100644 --- a/packages/app/src/stores/navigation-active-workspace-store/navigation.test.ts +++ b/packages/app/src/stores/navigation-active-workspace-store/navigation.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import type { ActiveWorkspaceSelection } from "@/stores/last-workspace-selection"; +import type { WorkspaceTabTarget } from "@/stores/workspace-tabs-store"; import { navigateToLastWorkspace, navigateToWorkspace, @@ -9,25 +10,28 @@ import { } from "./navigation"; import type { Agent, WorkspaceDescriptor } from "@/stores/session-store"; -interface RecordedAgentTab { +interface RecordedTab { workspaceKey: string; - agentId: string; + target: WorkspaceTabTarget; } function createFakeDeps(overrides: Partial = {}) { const navigations: string[] = []; const remembered: ActiveWorkspaceSelection[] = []; - const openedAgentTabs: RecordedAgentTab[] = []; + const openedTabs: RecordedTab[] = []; const deps: NavigateToWorkspaceDeps = { getSessionWorkspaces: () => null, getSessionAgents: () => [] as Agent[], - openWorkspaceAgentTab: (workspaceKey, agentId) => - openedAgentTabs.push({ workspaceKey, agentId }), + openTabFocused: (workspaceKey, target) => { + openedTabs.push({ workspaceKey, target }); + return target.kind === "agent" ? target.agentId : null; + }, + pinAgent: () => undefined, rememberLastWorkspace: (selection) => remembered.push(selection), navigateToRoute: (route) => navigations.push(route), ...overrides, }; - return { deps, navigations, remembered, openedAgentTabs }; + return { deps, navigations, remembered, openedTabs }; } function createLastSelectionDeps( @@ -63,7 +67,7 @@ describe("workspace navigation", () => { it("navigates to a workspace route and remembers the selection", () => { const { deps, navigations, remembered } = createFakeDeps(); - navigateToWorkspace("server-1", "workspace-a", deps); + navigateToWorkspace({ serverId: "server-1", workspaceId: "workspace-a" }, deps); expect(navigations).toEqual(["/h/server-1/workspace/workspace-a"]); expect(remembered).toEqual([{ serverId: "server-1", workspaceId: "workspace-a" }]); @@ -81,14 +85,53 @@ describe("workspace navigation", () => { requiresAttention: true, attentionReason: "permission", } as unknown as Agent; - const { deps, openedAgentTabs } = createFakeDeps({ + const { deps, openedTabs } = createFakeDeps({ getSessionWorkspaces: () => new Map([[workspace.id, workspace]]), getSessionAgents: () => [agent], }); - navigateToWorkspace("server-1", "workspace-a", deps); + navigateToWorkspace({ serverId: "server-1", workspaceId: "workspace-a" }, deps); - expect(openedAgentTabs).toEqual([{ workspaceKey: "server-1:workspace-a", agentId: "agent-1" }]); + expect(openedTabs).toEqual([ + { + workspaceKey: "server-1:workspace-a", + target: { kind: "agent", agentId: "agent-1" }, + }, + ]); + }); + + it("keeps an explicit tab authoritative over an attention agent", () => { + const workspace = { + id: "workspace-a", + workspaceDirectory: "/repo/workspace-a", + } as WorkspaceDescriptor; + const agent = { + id: "agent-1", + cwd: "/repo/workspace-a", + workspaceId: "workspace-a", + requiresAttention: true, + attentionReason: "permission", + } as unknown as Agent; + const { deps, openedTabs } = createFakeDeps({ + getSessionWorkspaces: () => new Map([[workspace.id, workspace]]), + getSessionAgents: () => [agent], + }); + + navigateToWorkspace( + { + serverId: "server-1", + workspaceId: "workspace-a", + target: { kind: "draft", draftId: "draft-1" }, + }, + deps, + ); + + expect(openedTabs).toEqual([ + { + workspaceKey: "server-1:workspace-a", + target: { kind: "draft", draftId: "draft-1" }, + }, + ]); }); it("reads the active workspace from the current route", () => { diff --git a/packages/app/src/stores/navigation-active-workspace-store/navigation.ts b/packages/app/src/stores/navigation-active-workspace-store/navigation.ts index 17c7c96b3f..4465e67e88 100644 --- a/packages/app/src/stores/navigation-active-workspace-store/navigation.ts +++ b/packages/app/src/stores/navigation-active-workspace-store/navigation.ts @@ -10,6 +10,8 @@ import { resolveWorkspaceMapKeyByIdentity, } from "@/utils/workspace-identity"; import type { ActiveWorkspaceSelection } from "@/stores/last-workspace-selection"; +import type { WorkspaceTabTarget } from "@/stores/workspace-tabs-store"; +import { prepareWorkspaceTab, type PrepareWorkspaceTabDeps } from "@/utils/prepare-workspace-tab"; export interface RouteSelectionInput { pathname: string; @@ -19,10 +21,16 @@ export interface RouteSelectionInput { }; } -export interface NavigateToWorkspaceDeps { +export interface NavigateToWorkspaceInput { + serverId: string; + workspaceId: string; + target?: WorkspaceTabTarget; + pin?: boolean; +} + +export interface NavigateToWorkspaceDeps extends PrepareWorkspaceTabDeps { getSessionWorkspaces: (serverId: string) => Map | null | undefined; getSessionAgents: (serverId: string) => Iterable; - openWorkspaceAgentTab: (workspaceKey: string, agentId: string) => void; rememberLastWorkspace: (selection: ActiveWorkspaceSelection) => void; navigateToRoute: (route: string) => void; } @@ -71,27 +79,35 @@ export function parseActiveWorkspaceSelection( } export function navigateToWorkspace( - serverId: string, - workspaceId: string, + input: NavigateToWorkspaceInput, deps: NavigateToWorkspaceDeps, -): void { - const workspaces = deps.getSessionWorkspaces(serverId); - const resolvedWorkspaceId = resolveWorkspaceMapKeyByIdentity({ - workspaces, - workspaceId, - }); - const workspaceAgents = resolvedWorkspaceId - ? Array.from(deps.getSessionAgents(serverId)).filter( - (agent) => normalizeWorkspaceOpaqueId(agent.workspaceId) === resolvedWorkspaceId, - ) - : []; - const attentionAgentId = pickAttentionAgent(workspaceAgents); - if (attentionAgentId && resolvedWorkspaceId) { - deps.openWorkspaceAgentTab(`${serverId}:${resolvedWorkspaceId}`, attentionAgentId); +): string { + if (input.target) { + prepareWorkspaceTab({ ...input, target: input.target }, deps); + } else { + const workspaces = deps.getSessionWorkspaces(input.serverId); + const resolvedWorkspaceId = resolveWorkspaceMapKeyByIdentity({ + workspaces, + workspaceId: input.workspaceId, + }); + const workspaceAgents = resolvedWorkspaceId + ? Array.from(deps.getSessionAgents(input.serverId)).filter( + (agent) => normalizeWorkspaceOpaqueId(agent.workspaceId) === resolvedWorkspaceId, + ) + : []; + const attentionAgentId = pickAttentionAgent(workspaceAgents); + if (attentionAgentId && resolvedWorkspaceId) { + deps.openTabFocused(`${input.serverId}:${resolvedWorkspaceId}`, { + kind: "agent", + agentId: attentionAgentId, + }); + } } - deps.rememberLastWorkspace({ serverId, workspaceId }); - deps.navigateToRoute(buildHostWorkspaceRoute(serverId, workspaceId)); + const route = buildHostWorkspaceRoute(input.serverId, input.workspaceId); + deps.rememberLastWorkspace({ serverId: input.serverId, workspaceId: input.workspaceId }); + deps.navigateToRoute(route); + return route; } export function navigateToLastWorkspace(deps: NavigateToLastWorkspaceDeps): boolean { @@ -99,6 +115,6 @@ export function navigateToLastWorkspace(deps: NavigateToLastWorkspaceDeps): bool if (!selection) { return false; } - navigateToWorkspace(selection.serverId, selection.workspaceId, deps); + navigateToWorkspace(selection, deps); return true; } diff --git a/packages/app/src/utils/navigate-to-agent/index.ts b/packages/app/src/utils/navigate-to-agent/index.ts index 175e0ae8eb..292c9bbed9 100644 --- a/packages/app/src/utils/navigate-to-agent/index.ts +++ b/packages/app/src/utils/navigate-to-agent/index.ts @@ -2,7 +2,7 @@ import { router, type Href } from "expo-router"; import { useSessionStore } from "@/stores/session-store"; import { getHostRuntimeStore, isHostRuntimeConnected } from "@/runtime/host-runtime"; import { resolveNavigateToAgent, type NavigateToAgentInput } from "./resolve"; -import { navigateToPreparedWorkspaceTab } from "@/utils/workspace-navigation"; +import { navigateToWorkspace } from "@/stores/navigation-active-workspace-store"; export type { NavigateToAgentInput } from "./resolve"; @@ -69,7 +69,7 @@ export function navigateToAgent(input: NavigateToAgentInput): string { navigateToHostAgent: (route) => { router.navigate(route as Href); }, - navigateToPreparedWorkspaceTab, + navigateToWorkspace, restoreArchivedWorkspace: ({ serverId, agentId, workspaceId }) => { restoreArchivedWorkspace(serverId, agentId, workspaceId); }, diff --git a/packages/app/src/utils/navigate-to-agent/resolve.test.ts b/packages/app/src/utils/navigate-to-agent/resolve.test.ts index 4f0a31af20..8347f1a0f9 100644 --- a/packages/app/src/utils/navigate-to-agent/resolve.test.ts +++ b/packages/app/src/utils/navigate-to-agent/resolve.test.ts @@ -4,7 +4,7 @@ import { type AgentNavTarget, type NavigateToAgentDeps, } from "@/utils/navigate-to-agent/resolve"; -import type { NavigateToPreparedWorkspaceTabInput } from "@/utils/prepare-workspace-tab"; +import type { NavigateToWorkspaceInput } from "@/stores/navigation-active-workspace-store"; const SERVER_ID = "server-1"; const WORKSPACE_ID = "workspace-1"; @@ -14,7 +14,7 @@ interface RecordedHostNav { route: string; } -interface RecordedTabNav extends NavigateToPreparedWorkspaceTabInput {} +interface RecordedTabNav extends NavigateToWorkspaceInput {} interface RecordedRestore { serverId: string; @@ -40,7 +40,7 @@ function createFakeNavigators(target: AgentNavTarget): { navigateToHostAgent: (route) => { hostNavigations.push({ route }); }, - navigateToPreparedWorkspaceTab: (input) => { + navigateToWorkspace: (input) => { tabNavigations.push(input); return `/h/${input.serverId}/workspace/${input.workspaceId}`; }, diff --git a/packages/app/src/utils/navigate-to-agent/resolve.ts b/packages/app/src/utils/navigate-to-agent/resolve.ts index 6298608ef7..95a65229ff 100644 --- a/packages/app/src/utils/navigate-to-agent/resolve.ts +++ b/packages/app/src/utils/navigate-to-agent/resolve.ts @@ -1,6 +1,6 @@ import { buildHostAgentDetailRoute } from "@/utils/host-routes"; import { normalizeWorkspaceOpaqueId } from "@/utils/workspace-identity"; -import type { NavigateToPreparedWorkspaceTabInput } from "@/utils/prepare-workspace-tab"; +import type { NavigateToWorkspaceInput } from "@/stores/navigation-active-workspace-store"; export interface NavigateToAgentInput { serverId: string; @@ -18,7 +18,7 @@ export interface AgentNavTarget { export interface NavigateToAgentDeps { readAgentNavTarget: (input: { serverId: string; agentId: string }) => AgentNavTarget; navigateToHostAgent: (route: string) => void; - navigateToPreparedWorkspaceTab: (input: NavigateToPreparedWorkspaceTabInput) => string; + navigateToWorkspace: (input: NavigateToWorkspaceInput) => string; restoreArchivedWorkspace: (input: { serverId: string; agentId: string; @@ -49,7 +49,7 @@ export function resolveNavigateToAgent( workspaceId, }); - return deps.navigateToPreparedWorkspaceTab({ + return deps.navigateToWorkspace({ serverId: input.serverId, workspaceId, target: { kind: "agent", agentId: input.agentId }, diff --git a/packages/app/src/utils/navigate-to-agent/restore-archived-workspace.test.ts b/packages/app/src/utils/navigate-to-agent/restore-archived-workspace.test.ts index ab0c01b319..87d3558bf9 100644 --- a/packages/app/src/utils/navigate-to-agent/restore-archived-workspace.test.ts +++ b/packages/app/src/utils/navigate-to-agent/restore-archived-workspace.test.ts @@ -9,8 +9,8 @@ vi.mock("expo-router", () => ({ router: { navigate: vi.fn() }, })); -vi.mock("@/utils/workspace-navigation", () => ({ - navigateToPreparedWorkspaceTab: vi.fn(() => ""), +vi.mock("@/stores/navigation-active-workspace-store", () => ({ + navigateToWorkspace: vi.fn(() => ""), })); vi.mock("@/runtime/host-runtime", () => ({ diff --git a/packages/app/src/utils/prepare-workspace-tab.ts b/packages/app/src/utils/prepare-workspace-tab.ts index ae76e96b53..213b107f1f 100644 --- a/packages/app/src/utils/prepare-workspace-tab.ts +++ b/packages/app/src/utils/prepare-workspace-tab.ts @@ -3,7 +3,6 @@ import { buildWorkspaceTabPersistenceKey, type WorkspaceTabTarget, } from "@/stores/workspace-tabs-store"; -import { buildHostWorkspaceRoute } from "@/utils/host-routes"; export interface PrepareWorkspaceTabInput { serverId: string; @@ -12,17 +11,11 @@ export interface PrepareWorkspaceTabInput { pin?: boolean; } -export type NavigateToPreparedWorkspaceTabInput = PrepareWorkspaceTabInput; - export interface PrepareWorkspaceTabDeps { openTabFocused: (workspaceKey: string, target: WorkspaceTabTarget) => string | null; pinAgent: (workspaceKey: string, agentId: string) => void; } -export interface NavigateToPreparedWorkspaceTabDeps extends PrepareWorkspaceTabDeps { - navigateToWorkspace: (serverId: string, workspaceId: string) => void; -} - function getPreparedTarget(target: WorkspaceTabTarget): WorkspaceTabTarget { if (target.kind !== "draft" || target.draftId.trim() !== "new") { return target; @@ -33,7 +26,7 @@ function getPreparedTarget(target: WorkspaceTabTarget): WorkspaceTabTarget { export function prepareWorkspaceTab( input: PrepareWorkspaceTabInput, deps: PrepareWorkspaceTabDeps, -): string { +): void { const target = getPreparedTarget(input.target); const key = buildWorkspaceTabPersistenceKey({ @@ -46,15 +39,4 @@ export function prepareWorkspaceTab( if (input.pin && target.kind === "agent") { deps.pinAgent(key, target.agentId); } - - return buildHostWorkspaceRoute(input.serverId, input.workspaceId); -} - -export function navigateToPreparedWorkspaceTab( - input: NavigateToPreparedWorkspaceTabInput, - deps: NavigateToPreparedWorkspaceTabDeps, -): string { - const route = prepareWorkspaceTab(input, deps); - deps.navigateToWorkspace(input.serverId, input.workspaceId); - return route; } diff --git a/packages/app/src/utils/workspace-navigation.test.ts b/packages/app/src/utils/workspace-navigation.test.ts index 2b3c60a7c8..0f96b5f220 100644 --- a/packages/app/src/utils/workspace-navigation.test.ts +++ b/packages/app/src/utils/workspace-navigation.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { WorkspaceTabTarget } from "@/stores/workspace-tabs-store"; -import { navigateToPreparedWorkspaceTab, prepareWorkspaceTab } from "@/utils/prepare-workspace-tab"; +import { prepareWorkspaceTab } from "@/utils/prepare-workspace-tab"; const SERVER_ID = "server-1"; const WORKSPACE_ID = "/repo/worktree"; @@ -16,11 +16,6 @@ interface RecordedPin { agentId: string; } -interface RecordedNavigation { - serverId: string; - workspaceId: string; -} - function createFakeLayout() { const openedTabs: RecordedOpenedTab[] = []; const pinnedAgents: RecordedPin[] = []; @@ -37,21 +32,11 @@ function createFakeLayout() { }; } -function createFakeNavigator() { - const navigations: RecordedNavigation[] = []; - return { - navigations, - navigateToWorkspace: (serverId: string, workspaceId: string) => { - navigations.push({ serverId, workspaceId }); - }, - }; -} - describe("prepareWorkspaceTab", () => { it("opens and focuses an agent tab", () => { const layout = createFakeLayout(); - const route = prepareWorkspaceTab( + prepareWorkspaceTab( { serverId: SERVER_ID, workspaceId: WORKSPACE_ID, @@ -60,30 +45,9 @@ describe("prepareWorkspaceTab", () => { layout, ); - expect(route).toBe("/h/server-1/workspace/b64_L3JlcG8vd29ya3RyZWU"); expect(layout.openedTabs).toEqual([ { key: "server-1:/repo/worktree", target: { kind: "agent", agentId: AGENT_ID } }, ]); expect(layout.pinnedAgents).toEqual([]); }); - - it("prepares a tab and navigates through the workspace navigation helper", () => { - const layout = createFakeLayout(); - const navigator = createFakeNavigator(); - - const route = navigateToPreparedWorkspaceTab( - { - serverId: SERVER_ID, - workspaceId: WORKSPACE_ID, - target: { kind: "agent", agentId: AGENT_ID }, - }, - { ...layout, navigateToWorkspace: navigator.navigateToWorkspace }, - ); - - expect(route).toBe("/h/server-1/workspace/b64_L3JlcG8vd29ya3RyZWU"); - expect(layout.openedTabs).toEqual([ - { key: "server-1:/repo/worktree", target: { kind: "agent", agentId: AGENT_ID } }, - ]); - expect(navigator.navigations).toEqual([{ serverId: SERVER_ID, workspaceId: WORKSPACE_ID }]); - }); }); diff --git a/packages/app/src/utils/workspace-navigation.ts b/packages/app/src/utils/workspace-navigation.ts index 514929c6d4..57c0ff6abe 100644 --- a/packages/app/src/utils/workspace-navigation.ts +++ b/packages/app/src/utils/workspace-navigation.ts @@ -1,16 +1,10 @@ -import { navigateToWorkspace } from "@/stores/navigation-active-workspace-store"; import { useWorkspaceLayoutStore } from "@/stores/workspace-layout-store"; import { prepareWorkspaceTab as prepareWorkspaceTabPure, - navigateToPreparedWorkspaceTab as navigateToPreparedWorkspaceTabPure, type PrepareWorkspaceTabInput, - type NavigateToPreparedWorkspaceTabInput, } from "./prepare-workspace-tab"; -export type { - PrepareWorkspaceTabInput, - NavigateToPreparedWorkspaceTabInput, -} from "./prepare-workspace-tab"; +export type { PrepareWorkspaceTabInput } from "./prepare-workspace-tab"; function layoutStoreDeps() { const store = useWorkspaceLayoutStore.getState(); @@ -20,13 +14,6 @@ function layoutStoreDeps() { }; } -export function prepareWorkspaceTab(input: PrepareWorkspaceTabInput): string { - return prepareWorkspaceTabPure(input, layoutStoreDeps()); -} - -export function navigateToPreparedWorkspaceTab(input: NavigateToPreparedWorkspaceTabInput): string { - return navigateToPreparedWorkspaceTabPure(input, { - ...layoutStoreDeps(), - navigateToWorkspace, - }); +export function prepareWorkspaceTab(input: PrepareWorkspaceTabInput): void { + prepareWorkspaceTabPure(input, layoutStoreDeps()); } From f2ebac931c60ed423968f1aa07ba78c0a0b2776c Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 13 Jul 2026 15:54:32 +0200 Subject: [PATCH 4/4] Keep CI moving through transient npm failures (#2039) * fix(ci): retry npm installs Transient registry and package-download failures should delay a job briefly instead of requiring a manual rerun. Apply one bounded retry policy across CI, deploy, and release workflows. * fix(ci): preserve retries for historical Android tags Manual Android rebuilds may check out releases from before the shared retry script existed. Keep that install step self-contained and allow the shared helper's injected runner to be asynchronous. * fix(ci): preserve retries for desktop release refs * refactor(ci): use shared npm retry helper everywhere --- .github/workflows/android-apk-release.yml | 2 +- .github/workflows/ci.yml | 77 +++++------------------ .github/workflows/deploy-app.yml | 2 +- .github/workflows/deploy-relay.yml | 2 +- .github/workflows/deploy-website.yml | 2 +- .github/workflows/desktop-release.yml | 8 +-- .github/workflows/desktop-rollout.yml | 2 +- scripts/npm-retry.mjs | 59 +++++++++++++++++ 8 files changed, 82 insertions(+), 72 deletions(-) create mode 100644 scripts/npm-retry.mjs diff --git a/.github/workflows/android-apk-release.yml b/.github/workflows/android-apk-release.yml index 540ffa6f8b..08eeccd340 100644 --- a/.github/workflows/android-apk-release.yml +++ b/.github/workflows/android-apk-release.yml @@ -71,7 +71,7 @@ jobs: scope: "@boudra" - name: Install JS dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci env: NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c9538d2d3d..d556217748 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,7 +33,7 @@ jobs: cache: "npm" - name: Install dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci - name: Check formatting run: npx oxfmt --check . @@ -51,7 +51,7 @@ jobs: cache: "npm" - name: Install dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci - name: Lint lockfile run: npx --yes lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integrity @@ -75,7 +75,7 @@ jobs: cache: "npm" - name: Install dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci - name: Build server stack run: npm run build:server @@ -111,9 +111,9 @@ jobs: run: git fetch --no-tags origin main:refs/remotes/origin/main - name: Install dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci - name: Install agent CLIs for provider tests - run: npm install -g @anthropic-ai/claude-code opencode-ai + run: node scripts/npm-retry.mjs install -g @anthropic-ai/claude-code opencode-ai - name: Build server dependencies run: npm run build:server-deps @@ -139,35 +139,8 @@ jobs: node-version: "22" cache: "npm" - - name: Install dependencies with Electron retry - if: runner.os != 'Windows' - run: | - for attempt in 1 2 3; do - if npm ci; then - exit 0 - else - exit_code=$? - fi - if [ "$attempt" -eq 3 ]; then - exit $exit_code - fi - sleep $((attempt * 20)) - done - - - name: Install dependencies with Electron retry - if: runner.os == 'Windows' - shell: pwsh - run: | - for ($attempt = 1; $attempt -le 3; $attempt++) { - npm ci - if ($LASTEXITCODE -eq 0) { - exit 0 - } - if ($attempt -eq 3) { - exit $LASTEXITCODE - } - Start-Sleep -Seconds (20 * $attempt) - } + - name: Install dependencies with retry + run: node scripts/npm-retry.mjs ci - name: Build server stack run: npm run build:server @@ -187,18 +160,7 @@ jobs: cache: "npm" - name: Install dependencies with retry - run: | - for attempt in 1 2 3; do - if npm ci; then - exit 0 - else - exit_code=$? - fi - if [ "$attempt" -eq 3 ]; then - exit $exit_code - fi - sleep $((attempt * 20)) - done + run: node scripts/npm-retry.mjs ci - name: Install Playwright browsers timeout-minutes: 10 run: npx playwright install chromium @@ -222,7 +184,7 @@ jobs: cache: "npm" - name: Install dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci - name: Build client dependencies run: npm run build:client @@ -258,18 +220,7 @@ jobs: cache: "npm" - name: Install dependencies with retry - run: | - for attempt in 1 2 3; do - if npm ci; then - exit 0 - else - exit_code=$? - fi - if [ "$attempt" -eq 3 ]; then - exit $exit_code - fi - sleep $((attempt * 20)) - done + run: node scripts/npm-retry.mjs ci - name: Install Playwright browsers timeout-minutes: 10 run: npx playwright install chromium @@ -282,7 +233,7 @@ jobs: - name: Install agent CLIs for provider tests if: ${{ !matrix.desktop }} - run: npm install -g @anthropic-ai/claude-code @openai/codex@0.105.0 opencode-ai + run: node scripts/npm-retry.mjs install -g @anthropic-ai/claude-code @openai/codex@0.105.0 opencode-ai - name: Run Playwright E2E tests if: ${{ !matrix.desktop }} @@ -317,7 +268,7 @@ jobs: cache: "npm" - name: Install dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci - name: Build relay run: npm run build:relay @@ -343,10 +294,10 @@ jobs: cache: "npm" - name: Install dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci - name: Install agent CLIs for provider tests - run: npm install -g @anthropic-ai/claude-code @openai/codex@0.105.0 opencode-ai + run: node scripts/npm-retry.mjs install -g @anthropic-ai/claude-code @openai/codex@0.105.0 opencode-ai - name: Run CLI tests run: npm run test --workspace=@getpaseo/cli diff --git a/.github/workflows/deploy-app.yml b/.github/workflows/deploy-app.yml index 11086c50f3..75dfbcaa9c 100644 --- a/.github/workflows/deploy-app.yml +++ b/.github/workflows/deploy-app.yml @@ -24,7 +24,7 @@ jobs: scope: "@boudra" - name: Install dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci env: NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Build app dependencies diff --git a/.github/workflows/deploy-relay.yml b/.github/workflows/deploy-relay.yml index 7712bd66cf..0ddb9d0270 100644 --- a/.github/workflows/deploy-relay.yml +++ b/.github/workflows/deploy-relay.yml @@ -18,7 +18,7 @@ jobs: cache: "npm" - name: Install dependencies - run: npm ci --workspace=@getpaseo/relay --include-workspace-root + run: node scripts/npm-retry.mjs ci --workspace=@getpaseo/relay --include-workspace-root - name: Typecheck run: npm run typecheck --workspace=@getpaseo/relay diff --git a/.github/workflows/deploy-website.yml b/.github/workflows/deploy-website.yml index 708d40334b..cd20556182 100644 --- a/.github/workflows/deploy-website.yml +++ b/.github/workflows/deploy-website.yml @@ -29,7 +29,7 @@ jobs: cache: "npm" - name: Install dependencies - run: npm ci --workspace=@getpaseo/website --include-workspace-root + run: node scripts/npm-retry.mjs ci --workspace=@getpaseo/website --include-workspace-root - name: Typecheck run: npm run typecheck --workspace=@getpaseo/website diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml index 5d3c8636c7..b181f8df94 100644 --- a/.github/workflows/desktop-release.yml +++ b/.github/workflows/desktop-release.yml @@ -127,7 +127,7 @@ jobs: scope: "@boudra" - name: Install JS dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci env: NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -227,7 +227,7 @@ jobs: scope: "@boudra" - name: Install JS dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci env: NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -325,7 +325,7 @@ jobs: scope: "@boudra" - name: Install JS dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci env: NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} @@ -425,7 +425,7 @@ jobs: scope: "@boudra" - name: Install JS dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci env: NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/desktop-rollout.yml b/.github/workflows/desktop-rollout.yml index 1cfb62cfe4..68740d86c3 100644 --- a/.github/workflows/desktop-rollout.yml +++ b/.github/workflows/desktop-rollout.yml @@ -47,7 +47,7 @@ jobs: scope: "@boudra" - name: Install JS dependencies - run: npm ci + run: node scripts/npm-retry.mjs ci env: NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/scripts/npm-retry.mjs b/scripts/npm-retry.mjs new file mode 100644 index 0000000000..8493094b9e --- /dev/null +++ b/scripts/npm-retry.mjs @@ -0,0 +1,59 @@ +import { spawnSync } from "node:child_process"; +import { resolve as resolvePath } from "node:path"; +import { fileURLToPath } from "node:url"; + +const DEFAULT_ATTEMPTS = 3; +const DEFAULT_BACKOFF_MS = 20_000; + +function runNpm(args) { + const command = process.platform === "win32" ? "npm.cmd" : "npm"; + const result = spawnSync(command, args, { + shell: process.platform === "win32", + stdio: "inherit", + }); + + if (result.error) { + console.error(`Failed to start npm: ${result.error.message}`); + return 1; + } + + return result.status ?? 1; +} + +function wait(delayMs) { + return new Promise((resolve) => setTimeout(resolve, delayMs)); +} + +export async function runWithRetry( + args, + { attempts = DEFAULT_ATTEMPTS, backoffMs = DEFAULT_BACKOFF_MS, run = runNpm, sleep = wait } = {}, +) { + let exitCode = 1; + + for (let attempt = 1; attempt <= attempts; attempt += 1) { + exitCode = await run(args); + if (exitCode === 0) { + return 0; + } + + if (attempt < attempts) { + const delayMs = attempt * backoffMs; + console.warn( + `npm failed with exit code ${exitCode}; retrying in ${delayMs / 1000}s (${attempt + 1}/${attempts})`, + ); + await sleep(delayMs); + } + } + + return exitCode; +} + +if (process.argv[1] && fileURLToPath(import.meta.url) === resolvePath(process.argv[1])) { + const args = process.argv.slice(2); + if (args.length === 0) { + console.error("Usage: node scripts/npm-retry.mjs "); + process.exitCode = 2; + } else { + process.exitCode = await runWithRetry(args); + } +}