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/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/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/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-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..b4034e81b4 100644 --- a/packages/app/src/screens/new-workspace-screen.tsx +++ b/packages/app/src/screens/new-workspace-screen.tsx @@ -46,11 +46,13 @@ 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 { 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"; @@ -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, @@ -92,7 +93,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 +184,6 @@ interface PickerOptionData { interface PickerSelection { item: PickerItem; - attachedPrNumber: number | null; } const BRANCH_OPTION_PREFIX = "branch:"; @@ -786,18 +790,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; @@ -1181,7 +1173,7 @@ function submitWorkspaceDraft(input: SubmitDraftInput): void { ...(submission.featureValues ? { featureValues: submission.featureValues } : {}), allowEmptyText: true, }); - navigateToPreparedWorkspaceTab({ + navigateToWorkspace({ serverId, workspaceId, target: submission.target, @@ -1682,11 +1674,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 +1789,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 +1810,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 +1831,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(() => { @@ -2041,7 +2045,7 @@ export function NewWorkspaceScreen({ ensureWorkspace, serverId: selectedServerId, navigate: (targetServerId, workspaceId) => - navigateToWorkspace(targetServerId, workspaceId), + navigateToWorkspace({ serverId: targetServerId, workspaceId }), }); return; } @@ -2155,7 +2159,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; } 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()); } 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 } : {}), 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); + } +}