diff --git a/.github/workflows/goose2-ci.yml b/.github/workflows/goose2-ci.yml new file mode 100644 index 000000000000..68b69c93ea9c --- /dev/null +++ b/.github/workflows/goose2-ci.yml @@ -0,0 +1,162 @@ +name: "Goose 2 CI" + +on: + push: + branches: [main] + paths: + - "ui/goose2/**" + pull_request: + branches: [main] + paths: + - "ui/goose2/**" + merge_group: + branches: [main] + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +defaults: + run: + working-directory: ui/goose2 + +jobs: + lint: + name: Lint & Format + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 + with: + version: 10.30.3 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: 24 + cache: pnpm + cache-dependency-path: ui/goose2/pnpm-lock.yaml + - run: pnpm install --frozen-lockfile + - run: pnpm check + - run: pnpm typecheck + + test: + name: Unit Tests + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 + with: + version: 10.30.3 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: 24 + cache: pnpm + cache-dependency-path: ui/goose2/pnpm-lock.yaml + - run: pnpm install --frozen-lockfile + - run: pnpm test + + desktop: + name: Desktop Build & E2E + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v4.4.0 + with: + version: 10.30.3 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: 24 + cache: pnpm + cache-dependency-path: ui/goose2/pnpm-lock.yaml + + - name: Install system dependencies + run: | + sudo apt-get update + sudo apt-get install -y \ + libgtk-3-dev \ + libwebkit2gtk-4.1-dev \ + libappindicator3-dev \ + librsvg2-dev \ + patchelf + + - name: Install Rust + uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + + - name: Cache Rust + uses: actions/cache@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + ui/goose2/src-tauri/target + key: ${{ runner.os }}-goose2-cargo-${{ hashFiles('ui/goose2/src-tauri/Cargo.lock') }} + + - run: pnpm install --frozen-lockfile + + - name: Build frontend + run: pnpm build + + - name: Check Tauri + run: cd src-tauri && cargo check + + - name: Clippy + run: cd src-tauri && cargo clippy -- -D warnings + + - name: Format check + run: cd src-tauri && cargo fmt --check + + - name: Install Playwright Chromium + run: pnpm exec playwright install --with-deps chromium + + - name: Run E2E tests + run: pnpm exec playwright test + + - name: Upload test artifacts + if: failure() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: playwright-report + path: | + ui/goose2/playwright-report/ + ui/goose2/test-results/ + retention-days: 7 + + rust-lint: + name: Rust Lint + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + + - name: Install system dependencies + run: | + sudo apt-get update + sudo apt-get install -y \ + libgtk-3-dev \ + libwebkit2gtk-4.1-dev \ + libappindicator3-dev \ + librsvg2-dev \ + patchelf + + - name: Install Rust + uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + components: rustfmt, clippy + + - name: Cache Rust + uses: actions/cache@668228422ae6a00e4ad889ee87cd7109ec5666a7 # v5.0.4 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + ui/goose2/src-tauri/target + key: ${{ runner.os }}-goose2-cargo-${{ hashFiles('ui/goose2/src-tauri/Cargo.lock') }} + + - name: Format check + run: cd src-tauri && cargo fmt --check + + - name: Clippy + run: cd src-tauri && cargo clippy -- -D warnings diff --git a/Cargo.toml b/Cargo.toml index 4cd6bb8a55d7..a6a5478333e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,7 @@ members = [ # Mainly for cargo-machete to not error out during inspection. "vendor/v8" ] +exclude = ["ui/goose2/src-tauri"] resolver = "2" [workspace.package] diff --git a/ui/desktop/src/components/settings/localInference/HuggingFaceModelSearch.tsx b/ui/desktop/src/components/settings/localInference/HuggingFaceModelSearch.tsx index 00eb1c4fe1fc..04da134438a3 100644 --- a/ui/desktop/src/components/settings/localInference/HuggingFaceModelSearch.tsx +++ b/ui/desktop/src/components/settings/localInference/HuggingFaceModelSearch.tsx @@ -1,5 +1,14 @@ import { useState, useCallback, useRef } from 'react'; -import { Search, Download, ChevronDown, ChevronUp, Loader2, Star, Check, AlertTriangle } from 'lucide-react'; +import { + Search, + Download, + ChevronDown, + ChevronUp, + Loader2, + Star, + Check, + AlertTriangle, +} from 'lucide-react'; import { Button } from '../../ui/button'; import { searchHfModels, @@ -90,7 +99,11 @@ interface Props { downloadedModelIds?: Set; } -export const HuggingFaceModelSearch = ({ onDownloadStarted, activeDownloadIds, downloadedModelIds }: Props) => { +export const HuggingFaceModelSearch = ({ + onDownloadStarted, + activeDownloadIds, + downloadedModelIds, +}: Props) => { const intl = useIntl(); const [query, setQuery] = useState(''); const [results, setResults] = useState([]); @@ -102,71 +115,79 @@ export const HuggingFaceModelSearch = ({ onDownloadStarted, activeDownloadIds, d const [error, setError] = useState(null); const debounceRef = useRef | null>(null); - const doSearch = useCallback(async (q: string) => { - if (!q.trim()) { - setResults([]); + const doSearch = useCallback( + async (q: string) => { + if (!q.trim()) { + setResults([]); + setError(null); + return; + } + setSearching(true); setError(null); - return; - } - setSearching(true); - setError(null); - try { - const response = await searchHfModels({ - query: { q, limit: 20 }, - }); - if (response.data) { - // Pre-fetch variants for all results and filter out repos with no suitable quantizations - const modelsWithVariants = await Promise.all( - response.data.map(async (model) => { - try { - const [author, repo] = model.repo_id.split('/'); - const filesResponse = await getRepoFiles({ path: { author, repo } }); - if (filesResponse.data && filesResponse.data.variants.length > 0) { - return { model, data: filesResponse.data }; + try { + const response = await searchHfModels({ + query: { q, limit: 20 }, + }); + if (response.data) { + // Pre-fetch variants for all results and filter out repos with no suitable quantizations + const modelsWithVariants = await Promise.all( + response.data.map(async (model) => { + try { + const [author, repo] = model.repo_id.split('/'); + const filesResponse = await getRepoFiles({ path: { author, repo } }); + if (filesResponse.data && filesResponse.data.variants.length > 0) { + return { model, data: filesResponse.data }; + } + } catch { + // Skip repos we can't fetch } - } catch { - // Skip repos we can't fetch - } - return null; - }) - ); + return null; + }) + ); - const validResults = modelsWithVariants.filter(Boolean) as { - model: HfModelInfo; - data: { variants: HfQuantVariant[]; recommended_index?: number | null; available_memory_bytes: number; downloaded_quants: string[] }; - }[]; - - setResults(validResults.map((r) => r.model)); - setRepoData((prev) => { - const next = { ...prev }; - for (const r of validResults) { - next[r.model.repo_id] = { - variants: r.data.variants, - recommendedIndex: r.data.recommended_index ?? null, - availableMemoryBytes: r.data.available_memory_bytes, - downloadedQuants: new Set(r.data.downloaded_quants), + const validResults = modelsWithVariants.filter(Boolean) as { + model: HfModelInfo; + data: { + variants: HfQuantVariant[]; + recommended_index?: number | null; + available_memory_bytes: number; + downloaded_quants: string[]; }; - } - return next; - }); + }[]; + + setResults(validResults.map((r) => r.model)); + setRepoData((prev) => { + const next = { ...prev }; + for (const r of validResults) { + next[r.model.repo_id] = { + variants: r.data.variants, + recommendedIndex: r.data.recommended_index ?? null, + availableMemoryBytes: r.data.available_memory_bytes, + downloadedQuants: new Set(r.data.downloaded_quants), + }; + } + return next; + }); - if (validResults.length === 0) { - setError(intl.formatMessage(i18n.noGgufModels)); + if (validResults.length === 0) { + setError(intl.formatMessage(i18n.noGgufModels)); + } + } else { + console.error('Search response:', response); + const errMsg = response.error + ? intl.formatMessage(i18n.searchError, { details: JSON.stringify(response.error) }) + : intl.formatMessage(i18n.searchNoData); + setError(errMsg); } - } else { - console.error('Search response:', response); - const errMsg = response.error - ? intl.formatMessage(i18n.searchError, { details: JSON.stringify(response.error) }) - : intl.formatMessage(i18n.searchNoData); - setError(errMsg); + } catch (e) { + console.error('Search failed:', e); + setError(intl.formatMessage(i18n.searchFailed)); + } finally { + setSearching(false); } - } catch (e) { - console.error('Search failed:', e); - setError(intl.formatMessage(i18n.searchFailed)); - } finally { - setSearching(false); - } - }, [intl]); + }, + [intl] + ); const handleQueryChange = (value: string) => { setQuery(value); @@ -235,7 +256,9 @@ export const HuggingFaceModelSearch = ({ onDownloadStarted, activeDownloadIds, d return (
-

{intl.formatMessage(i18n.searchHuggingFace)}

+

+ {intl.formatMessage(i18n.searchHuggingFace)} +

0 && variant.size_bytes > availableMemory * 0.85; + const tooLarge = + availableMemory > 0 && variant.size_bytes > availableMemory * 0.85; return (
{isDownloaded ? ( - ) : isActiveDownload ? ( - @@ -393,7 +407,6 @@ export const HuggingFaceModelSearch = ({ onDownloadStarted, activeDownloadIds, d })}
)} -
); }; diff --git a/ui/desktop/src/components/settings/localInference/LocalInferenceSettings.tsx b/ui/desktop/src/components/settings/localInference/LocalInferenceSettings.tsx index ad1a6cef8084..9b1ef8f3a200 100644 --- a/ui/desktop/src/components/settings/localInference/LocalInferenceSettings.tsx +++ b/ui/desktop/src/components/settings/localInference/LocalInferenceSettings.tsx @@ -97,7 +97,13 @@ const i18n = defineMessages({ }, }); -const VisionBadge = ({ model, intl }: { model: LocalModelResponse; intl: ReturnType }) => { +const VisionBadge = ({ + model, + intl, +}: { + model: LocalModelResponse; + intl: ReturnType; +}) => { if (!model.vision_capable) return null; const mmproj = model.mmproj_status; @@ -114,9 +120,8 @@ const VisionBadge = ({ model, intl }: { model: LocalModelResponse; intl: ReturnT } if (isDownloading) { - const percent = mmproj && 'progress_percent' in mmproj - ? Math.round(mmproj.progress_percent) - : null; + const percent = + mmproj && 'progress_percent' in mmproj ? Math.round(mmproj.progress_percent) : null; return ( @@ -324,7 +329,9 @@ export const LocalInferenceSettings = () => { {/* Active Downloads */} {downloads.size > 0 && (
-

{intl.formatMessage(i18n.downloading)}

+

+ {intl.formatMessage(i18n.downloading)} +

{Array.from(downloads.entries()).map(([modelId, progress]) => { if (progress.status === 'completed') return null; @@ -397,7 +404,9 @@ export const LocalInferenceSettings = () => { {/* Downloaded Models */} {downloadedModels.length > 0 && (
-

{intl.formatMessage(i18n.downloadedModels)}

+

+ {intl.formatMessage(i18n.downloadedModels)} +

{downloadedModels.map((model) => { const isSelected = selectedModelId === model.id; @@ -458,7 +467,9 @@ export const LocalInferenceSettings = () => { {/* Featured Models (not yet downloaded) */} {displayedFeatured.length > 0 && (
-

{intl.formatMessage(i18n.featuredModels)}

+

+ {intl.formatMessage(i18n.featuredModels)} +

{displayedFeatured.map((model) => (
{ m.status.state === 'Downloaded').map(m => m.id))} + downloadedModelIds={ + new Set(models.filter((m) => m.status.state === 'Downloaded').map((m) => m.id)) + } />
{models.length === 0 && ( -
{intl.formatMessage(i18n.noModels)}
+
+ {intl.formatMessage(i18n.noModels)} +
)} - + Senior engineer code review focused on catching issues before they become PR + comments. Reviews only changed lines, categorizes issues by priority, and fixes + them one by one. Use when the user says "code review", "review my code", + "review this branch", or wants pre-PR feedback. +--- + +# Pre-PR Code Review + +You are a senior engineer conducting a thorough code review. Review **only the lines that changed** in this branch (via `git diff main...HEAD`) and provide actionable feedback on code quality. Do not flag issues in unchanged code. + +## Determine Files to Review + +**Before starting the review**, identify which files to review by checking: + +1. **Run git commands** to check both: + - Committed changes: `git diff --name-only main...HEAD` + - Unstaged/staged changes: `git status --short` + +2. **Ask the user which set to review** if both exist: + - If there are both committed changes AND unstaged/staged changes, ask: "I see you have both committed changes and unstaged/staged changes. Which would you like me to review?" + - **Option A**: Committed changes in this branch (compare against main) + - **Option B**: Current unstaged/staged changes + - **Option C**: Both + +3. **Proceed automatically** if only one set exists: + - If only committed changes exist → review those + - If only unstaged/staged changes exist → review those + - If neither exist → inform the user there are no changes to review + +4. **Get the file list** based on the user's choice: + - For committed changes: Use `git diff --name-only main...HEAD` + - For unstaged/staged: Use `git diff --name-only` and `git diff --cached --name-only` + - Filter to only include files that exist (some may be deleted) + +**Only proceed with the review once you have the specific list of files to review.** + +## Review Checklist + +### React Best Practices +- **Components**: Are functional components with hooks used consistently? +- **State Management**: Is `useState` and `useEffect` used properly? Any unnecessary re-renders? +- **Props**: Are prop types properly defined with TypeScript interfaces? +- **Keys**: Are list items using proper unique keys (not array indices)? +- **Hooks Rules**: Are hooks called at the top level and in the correct order? +- **Custom Hooks**: Could any logic be extracted into reusable custom hooks? + +### TypeScript Best Practices +- **const vs let vs var**: Is `const` used by default? Is `let` only used when reassignment is needed? Is `var` avoided entirely? +- **Type Safety**: Are types explicit and avoiding `any`? Are proper interfaces/types defined? +- **Type Assertions**: Are type assertions (`as`) used sparingly and only when necessary? +- **Non-null Assertions**: Are non-null assertions (`!`) avoided? They bypass TypeScript's null safety and hide bugs. Use proper null checks or optional chaining instead. +- **React Ref Types**: Are React refs properly typed as nullable (`useRef(null)` with `RefObject`)? Refs are null on first render and during unmount. +- **Optional Chaining**: Is optional chaining (`?.`) used appropriately for potentially undefined values? +- **Enums vs Union Types**: Are union types preferred over enums where appropriate? + +### Design System & Styling +- **Component Usage**: Are design system components used instead of raw HTML elements (` + + ); +} diff --git a/ui/goose2/src/env.d.ts b/ui/goose2/src/env.d.ts new file mode 100644 index 000000000000..3c5d8feed57a --- /dev/null +++ b/ui/goose2/src/env.d.ts @@ -0,0 +1,7 @@ +declare global { + interface Window { + __TAURI_INTERNALS__?: unknown; + } +} + +export {}; diff --git a/ui/goose2/src/features/agents/hooks/__tests__/usePersonas.test.ts b/ui/goose2/src/features/agents/hooks/__tests__/usePersonas.test.ts new file mode 100644 index 000000000000..a480588305d2 --- /dev/null +++ b/ui/goose2/src/features/agents/hooks/__tests__/usePersonas.test.ts @@ -0,0 +1,255 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, act, waitFor } from "@testing-library/react"; +import { useAgentStore } from "../../stores/agentStore"; +import type { Persona } from "@/shared/types/agents"; + +// ── mocks ──────────────────────────────────────────────────────────── + +vi.mock("@/shared/api/agents", () => ({ + listPersonas: vi.fn().mockResolvedValue([]), + createPersona: vi.fn().mockResolvedValue({ + id: "new-id", + displayName: "Test", + systemPrompt: "You are helpful.", + isBuiltin: false, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }), + updatePersona: vi.fn().mockResolvedValue({ + id: "test-id", + displayName: "Updated", + systemPrompt: "Updated prompt", + isBuiltin: false, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }), + deletePersona: vi.fn().mockResolvedValue(undefined), + refreshPersonas: vi.fn().mockResolvedValue([]), +})); + +// Import the mocked module so we can inspect/adjust calls +import * as api from "@/shared/api/agents"; + +// Import the hook after mocks are set up +import { usePersonas } from "../usePersonas"; + +// ── helpers ────────────────────────────────────────────────────────── + +function makePersona(overrides: Partial = {}): Persona { + return { + id: crypto.randomUUID(), + displayName: "Test Persona", + systemPrompt: "You are helpful.", + isBuiltin: false, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + }; +} + +// ── tests ──────────────────────────────────────────────────────────── + +describe("usePersonas", () => { + beforeEach(() => { + // Re-establish default mock implementations (clearAllMocks would wipe them) + vi.mocked(api.listPersonas).mockReset().mockResolvedValue([]); + vi.mocked(api.createPersona).mockReset().mockResolvedValue({ + id: "new-id", + displayName: "Test", + systemPrompt: "You are helpful.", + isBuiltin: false, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }); + vi.mocked(api.updatePersona).mockReset().mockResolvedValue({ + id: "test-id", + displayName: "Updated", + systemPrompt: "Updated prompt", + isBuiltin: false, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }); + vi.mocked(api.deletePersona).mockReset().mockResolvedValue(undefined); + vi.mocked(api.refreshPersonas).mockReset().mockResolvedValue([]); + + useAgentStore.setState({ + personas: [], + personasLoading: false, + agents: [], + agentsLoading: false, + activeAgentId: null, + isLoading: false, + personaEditorOpen: false, + editingPersona: null, + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + // ── loading ──────────────────────────────────────────────────────── + + describe("loading personas", () => { + it("loads personas on mount via listPersonas()", async () => { + const personas = [makePersona({ id: "p1" }), makePersona({ id: "p2" })]; + vi.mocked(api.listPersonas).mockResolvedValueOnce(personas); + + const { result } = renderHook(() => usePersonas()); + + await waitFor(() => { + expect(api.listPersonas).toHaveBeenCalledTimes(1); + }); + + await waitFor(() => { + expect(result.current.personas).toEqual(personas); + }); + }); + + it("sets loading state correctly", async () => { + // Create a deferred promise to control timing + let resolveList!: (value: Persona[]) => void; + vi.mocked(api.listPersonas).mockImplementationOnce( + () => + new Promise((resolve) => { + resolveList = resolve; + }), + ); + + const { result } = renderHook(() => usePersonas()); + + // Should be loading while the API call is in flight + await waitFor(() => { + expect(result.current.isLoading).toBe(true); + }); + + // Resolve the API call + await act(async () => { + resolveList([]); + }); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); + }); + }); + + // ── CRUD operations ──────────────────────────────────────────────── + + describe("CRUD operations", () => { + it("createPersona calls API and adds to store", async () => { + const newPersona = { + id: "new-id", + displayName: "Test", + systemPrompt: "You are helpful.", + isBuiltin: false, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }; + vi.mocked(api.createPersona).mockResolvedValueOnce(newPersona); + + const { result } = renderHook(() => usePersonas()); + + // Wait for initial load to fully complete + await waitFor(() => { + expect(api.listPersonas).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(false); + }); + + let created: Persona | undefined; + await act(async () => { + created = await result.current.createPersona({ + displayName: "Test", + systemPrompt: "You are helpful.", + }); + }); + + expect(api.createPersona).toHaveBeenCalledWith({ + displayName: "Test", + systemPrompt: "You are helpful.", + }); + expect(created).toEqual(newPersona); + expect(result.current.personas).toContainEqual(newPersona); + }); + + it("updatePersona calls API and updates store", async () => { + const existing = makePersona({ id: "test-id", displayName: "Old" }); + // Return existing persona from initial load so the store has it + vi.mocked(api.listPersonas).mockResolvedValueOnce([existing]); + + const updated = { + id: "test-id", + displayName: "Updated", + systemPrompt: "Updated prompt", + isBuiltin: false, + createdAt: "2026-01-01T00:00:00Z", + updatedAt: "2026-01-01T00:00:00Z", + }; + vi.mocked(api.updatePersona).mockResolvedValueOnce(updated); + + const { result } = renderHook(() => usePersonas()); + + // Wait for initial load to populate store + await waitFor(() => { + expect(result.current.personas).toHaveLength(1); + }); + + await act(async () => { + await result.current.updatePersona("test-id", { + displayName: "Updated", + }); + }); + + expect(api.updatePersona).toHaveBeenCalledWith("test-id", { + displayName: "Updated", + }); + expect( + result.current.personas.find((p) => p.id === "test-id")?.displayName, + ).toBe("Updated"); + }); + + it("deletePersona calls API and removes from store", async () => { + const existing = makePersona({ id: "del-id" }); + // Return existing persona from initial load so the store has it + vi.mocked(api.listPersonas).mockResolvedValueOnce([existing]); + + const { result } = renderHook(() => usePersonas()); + + // Wait for initial load to populate store + await waitFor(() => { + expect(result.current.personas).toHaveLength(1); + }); + + await act(async () => { + await result.current.deletePersona("del-id"); + }); + + expect(api.deletePersona).toHaveBeenCalledWith("del-id"); + expect( + result.current.personas.find((p) => p.id === "del-id"), + ).toBeUndefined(); + }); + }); + + // ── refresh ──────────────────────────────────────────────────────── + + describe("refresh", () => { + it("refreshFromDisk calls refreshPersonas() API", async () => { + const refreshed = [makePersona({ id: "refreshed-1" })]; + vi.mocked(api.refreshPersonas).mockResolvedValueOnce(refreshed); + + const { result } = renderHook(() => usePersonas()); + + await waitFor(() => { + expect(result.current.isLoading).toBe(false); + }); + + await act(async () => { + await result.current.refreshFromDisk(); + }); + + expect(api.refreshPersonas).toHaveBeenCalled(); + expect(result.current.personas).toEqual(refreshed); + }); + }); +}); diff --git a/ui/goose2/src/features/agents/hooks/usePersonas.ts b/ui/goose2/src/features/agents/hooks/usePersonas.ts new file mode 100644 index 000000000000..4bbadd8dc3c0 --- /dev/null +++ b/ui/goose2/src/features/agents/hooks/usePersonas.ts @@ -0,0 +1,92 @@ +import { useEffect, useCallback, useRef } from "react"; +import { useAgentStore } from "../stores/agentStore"; +import type { + CreatePersonaRequest, + UpdatePersonaRequest, +} from "@/shared/types/agents"; +import * as api from "@/shared/api/agents"; + +const REFRESH_INTERVAL_MS = 60_000; + +export function usePersonas() { + const store = useAgentStore(); + const refreshTimerRef = useRef | null>(null); + + // biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation + const loadPersonas = useCallback(async () => { + store.setPersonasLoading(true); + try { + const personas = await api.listPersonas(); + store.setPersonas(personas); + } catch (error) { + console.error("Failed to load personas:", error); + // Fall back to empty list - builtins will come from backend + } finally { + store.setPersonasLoading(false); + } + }, []); + + // biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation + const refreshFromDisk = useCallback(async () => { + try { + const personas = await api.refreshPersonas(); + store.setPersonas(personas); + } catch (error) { + console.error("Failed to refresh personas from disk:", error); + } + }, []); + + useEffect(() => { + loadPersonas(); + }, [loadPersonas]); + + // Periodic refresh every 60s and on window focus + useEffect(() => { + refreshTimerRef.current = setInterval(refreshFromDisk, REFRESH_INTERVAL_MS); + + const handleFocus = () => { + refreshFromDisk(); + }; + window.addEventListener("focus", handleFocus); + + return () => { + if (refreshTimerRef.current) { + clearInterval(refreshTimerRef.current); + } + window.removeEventListener("focus", handleFocus); + }; + }, [refreshFromDisk]); + + // biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation + const createPersona = useCallback(async (req: CreatePersonaRequest) => { + const persona = await api.createPersona(req); + store.addPersona(persona); + return persona; + }, []); + + // biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation + const updatePersona = useCallback( + async (id: string, req: UpdatePersonaRequest) => { + const persona = await api.updatePersona(id, req); + store.updatePersona(id, persona); + return persona; + }, + [], + ); + + // biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation + const deletePersona = useCallback(async (id: string) => { + await api.deletePersona(id); + store.removePersona(id); + }, []); + + return { + personas: store.personas, + isLoading: store.personasLoading, + createPersona, + updatePersona, + deletePersona, + refresh: loadPersonas, + refreshFromDisk, + }; +} diff --git a/ui/goose2/src/features/agents/hooks/useProviderSelection.ts b/ui/goose2/src/features/agents/hooks/useProviderSelection.ts new file mode 100644 index 000000000000..060547eafd3a --- /dev/null +++ b/ui/goose2/src/features/agents/hooks/useProviderSelection.ts @@ -0,0 +1,31 @@ +import { useCallback } from "react"; +import { useAgentStore } from "../stores/agentStore"; + +export function useProviderSelection() { + const providers = useAgentStore((s) => s.providers); + const providersLoading = useAgentStore((s) => s.providersLoading); + const selectedProvider = useAgentStore((s) => s.selectedProvider); + const storeSetSelectedProvider = useAgentStore((s) => s.setSelectedProvider); + + const setSelectedProvider = useCallback( + (providerId: string) => { + storeSetSelectedProvider(providerId, true); + }, + [storeSetSelectedProvider], + ); + + const setSelectedProviderWithoutPersist = useCallback( + (providerId: string) => { + storeSetSelectedProvider(providerId, false); + }, + [storeSetSelectedProvider], + ); + + return { + providers, + providersLoading, + selectedProvider, + setSelectedProvider, + setSelectedProviderWithoutPersist, + }; +} diff --git a/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts b/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts new file mode 100644 index 000000000000..53275e93b3dd --- /dev/null +++ b/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts @@ -0,0 +1,200 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { useAgentStore } from "../agentStore"; +import type { Persona, Agent } from "@/shared/types/agents"; + +// ── fixtures ────────────────────────────────────────────────────────── + +function makePersona(overrides: Partial = {}): Persona { + return { + id: crypto.randomUUID(), + displayName: "Test Persona", + systemPrompt: "You are helpful.", + isBuiltin: false, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + }; +} + +function makeAgent(overrides: Partial = {}): Agent { + return { + id: crypto.randomUUID(), + name: "Test Agent", + provider: "goose", + model: "claude-sonnet-4", + connectionType: "builtin", + status: "online", + isBuiltin: false, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + }; +} + +// ── tests ───────────────────────────────────────────────────────────── + +describe("agentStore", () => { + beforeEach(() => { + useAgentStore.setState({ + personas: [], + personasLoading: false, + agents: [], + agentsLoading: false, + activeAgentId: null, + isLoading: false, + personaEditorOpen: false, + editingPersona: null, + }); + }); + + // ── initial state ───────────────────────────────────────────────── + + it("has empty personas and agents initially", () => { + const state = useAgentStore.getState(); + expect(state.personas).toEqual([]); + expect(state.agents).toEqual([]); + }); + + // ── persona CRUD ────────────────────────────────────────────────── + + it("setPersonas replaces personas", () => { + const p1 = makePersona({ id: "p1" }); + const p2 = makePersona({ id: "p2" }); + useAgentStore.getState().setPersonas([p1, p2]); + expect(useAgentStore.getState().personas).toEqual([p1, p2]); + }); + + it("addPersona appends a persona", () => { + const p = makePersona(); + useAgentStore.getState().addPersona(p); + expect(useAgentStore.getState().personas).toHaveLength(1); + expect(useAgentStore.getState().personas[0].id).toBe(p.id); + }); + + it("updatePersona updates the correct persona", () => { + const p = makePersona({ id: "up1", displayName: "Old" }); + useAgentStore.getState().setPersonas([p]); + useAgentStore.getState().updatePersona("up1", { displayName: "New" }); + expect(useAgentStore.getState().personas[0].displayName).toBe("New"); + }); + + it("removePersona removes the correct persona", () => { + const p1 = makePersona({ id: "keep" }); + const p2 = makePersona({ id: "remove" }); + useAgentStore.getState().setPersonas([p1, p2]); + useAgentStore.getState().removePersona("remove"); + expect(useAgentStore.getState().personas).toHaveLength(1); + expect(useAgentStore.getState().personas[0].id).toBe("keep"); + }); + + // ── agent CRUD ──────────────────────────────────────────────────── + + it("setAgents replaces agents", () => { + const a = makeAgent(); + useAgentStore.getState().setAgents([a]); + expect(useAgentStore.getState().agents).toEqual([a]); + }); + + it("addAgent appends an agent", () => { + const a = makeAgent(); + useAgentStore.getState().addAgent(a); + expect(useAgentStore.getState().agents).toHaveLength(1); + }); + + it("updateAgent updates the correct agent", () => { + const a = makeAgent({ id: "ua1", name: "Old" }); + useAgentStore.getState().setAgents([a]); + useAgentStore.getState().updateAgent("ua1", { name: "New" }); + expect(useAgentStore.getState().agents[0].name).toBe("New"); + }); + + it("removeAgent removes the correct agent", () => { + const a1 = makeAgent({ id: "keep" }); + const a2 = makeAgent({ id: "remove" }); + useAgentStore.getState().setAgents([a1, a2]); + useAgentStore.getState().removeAgent("remove"); + expect(useAgentStore.getState().agents).toHaveLength(1); + expect(useAgentStore.getState().agents[0].id).toBe("keep"); + }); + + // ── active agent ────────────────────────────────────────────────── + + it("setActiveAgent updates activeAgentId", () => { + useAgentStore.getState().setActiveAgent("a1"); + expect(useAgentStore.getState().activeAgentId).toBe("a1"); + }); + + it("getActiveAgent returns correct agent or null", () => { + expect(useAgentStore.getState().getActiveAgent()).toBeNull(); + + const a = makeAgent({ id: "active-1" }); + useAgentStore.getState().setAgents([a]); + useAgentStore.getState().setActiveAgent("active-1"); + expect(useAgentStore.getState().getActiveAgent()).toEqual(a); + }); + + // ── persona editor ──────────────────────────────────────────────── + + it("openPersonaEditor sets editing state", () => { + const p = makePersona(); + useAgentStore.getState().openPersonaEditor(p); + expect(useAgentStore.getState().personaEditorOpen).toBe(true); + expect(useAgentStore.getState().editingPersona).toEqual(p); + }); + + it("openPersonaEditor without persona sets editingPersona to null", () => { + useAgentStore.getState().openPersonaEditor(); + expect(useAgentStore.getState().personaEditorOpen).toBe(true); + expect(useAgentStore.getState().editingPersona).toBeNull(); + }); + + it("closePersonaEditor clears editing state", () => { + useAgentStore.getState().openPersonaEditor(makePersona()); + useAgentStore.getState().closePersonaEditor(); + expect(useAgentStore.getState().personaEditorOpen).toBe(false); + expect(useAgentStore.getState().editingPersona).toBeNull(); + }); + + // ── helpers ─────────────────────────────────────────────────────── + + it("getPersonaById returns correct persona", () => { + const p = makePersona({ id: "find-me" }); + useAgentStore.getState().setPersonas([p]); + expect(useAgentStore.getState().getPersonaById("find-me")).toEqual(p); + expect(useAgentStore.getState().getPersonaById("nope")).toBeUndefined(); + }); + + it("getAgentsByPersona filters correctly", () => { + const a1 = makeAgent({ id: "a1", personaId: "p1" }); + const a2 = makeAgent({ id: "a2", personaId: "p2" }); + const a3 = makeAgent({ id: "a3", personaId: "p1" }); + useAgentStore.getState().setAgents([a1, a2, a3]); + const result = useAgentStore.getState().getAgentsByPersona("p1"); + expect(result).toHaveLength(2); + expect(result.map((a) => a.id).sort()).toEqual(["a1", "a3"]); + }); + + it("getBuiltinPersonas returns only builtins", () => { + useAgentStore + .getState() + .setPersonas([ + makePersona({ id: "b", isBuiltin: true }), + makePersona({ id: "c", isBuiltin: false }), + ]); + const builtins = useAgentStore.getState().getBuiltinPersonas(); + expect(builtins).toHaveLength(1); + expect(builtins[0].id).toBe("b"); + }); + + it("getCustomPersonas returns only non-builtins", () => { + useAgentStore + .getState() + .setPersonas([ + makePersona({ id: "b", isBuiltin: true }), + makePersona({ id: "c", isBuiltin: false }), + ]); + const custom = useAgentStore.getState().getCustomPersonas(); + expect(custom).toHaveLength(1); + expect(custom[0].id).toBe("c"); + }); +}); diff --git a/ui/goose2/src/features/agents/stores/agentStore.ts b/ui/goose2/src/features/agents/stores/agentStore.ts new file mode 100644 index 000000000000..e0f535529c84 --- /dev/null +++ b/ui/goose2/src/features/agents/stores/agentStore.ts @@ -0,0 +1,216 @@ +import { create } from "zustand"; +import type { Persona, Agent } from "@/shared/types/agents"; +import type { AcpProvider } from "@/shared/api/acp"; + +const PROVIDER_STORAGE_KEY = "goose:defaultProvider"; +const FALLBACK_PROVIDER = "goose"; + +export function getStoredProvider(providers: AcpProvider[] = []): string { + try { + const storedProvider = + localStorage.getItem(PROVIDER_STORAGE_KEY) ?? FALLBACK_PROVIDER; + + if ( + providers.length === 0 || + providers.some((provider) => provider.id === storedProvider) + ) { + return storedProvider; + } + + return providers[0]?.id ?? FALLBACK_PROVIDER; + } catch { + return providers[0]?.id ?? FALLBACK_PROVIDER; + } +} + +function persistProvider(providerId: string): void { + try { + localStorage.setItem(PROVIDER_STORAGE_KEY, providerId); + } catch { + // localStorage may be unavailable + } +} + +interface AgentStoreState { + // Personas + personas: Persona[]; + personasLoading: boolean; + + // Agents + agents: Agent[]; + agentsLoading: boolean; + + // ACP Providers (cached) + providers: AcpProvider[]; + providersLoading: boolean; + + // Selected provider (shared across all chat screens) + selectedProvider: string; + + // Active agent for current chat + activeAgentId: string | null; + + // General loading (for backwards compat) + isLoading: boolean; + + // UI state + personaEditorOpen: boolean; + editingPersona: Persona | null; +} + +interface AgentStoreActions { + // Persona CRUD + setPersonas: (personas: Persona[]) => void; + addPersona: (persona: Persona) => void; + updatePersona: (id: string, updates: Partial) => void; + removePersona: (id: string) => void; + setPersonasLoading: (loading: boolean) => void; + + // Agent CRUD + setAgents: (agents: Agent[]) => void; + addAgent: (agent: Agent) => void; + updateAgent: (id: string, updates: Partial) => void; + removeAgent: (id: string) => void; + setAgentsLoading: (loading: boolean) => void; + + // Provider management + setProviders: (providers: AcpProvider[]) => void; + setProvidersLoading: (loading: boolean) => void; + setSelectedProvider: (providerId: string, persist?: boolean) => void; + + // Active agent + setActiveAgent: (id: string | null) => void; + getActiveAgent: () => Agent | null; + + // Persona editor + openPersonaEditor: (persona?: Persona) => void; + closePersonaEditor: () => void; + + // Loading + setLoading: (loading: boolean) => void; + + // Helpers + getPersonaById: (id: string) => Persona | undefined; + getAgentById: (id: string) => Agent | undefined; + getAgentsByPersona: (personaId: string) => Agent[]; + getBuiltinPersonas: () => Persona[]; + getCustomPersonas: () => Persona[]; +} + +export type AgentStore = AgentStoreState & AgentStoreActions; + +export const useAgentStore = create((set, get) => ({ + // State + personas: [], + personasLoading: false, + agents: [], + agentsLoading: false, + providers: [], + providersLoading: false, + selectedProvider: getStoredProvider(), + activeAgentId: null, + isLoading: false, + personaEditorOpen: false, + editingPersona: null, + + // Persona CRUD + setPersonas: (personas) => set({ personas }), + + addPersona: (persona) => + set((state) => ({ personas: [...state.personas, persona] })), + + updatePersona: (id, updates) => + set((state) => ({ + personas: state.personas.map((p) => + p.id === id + ? { ...p, ...updates, updatedAt: new Date().toISOString() } + : p, + ), + })), + + removePersona: (id) => + set((state) => ({ + personas: state.personas.filter((p) => p.id !== id), + })), + + setPersonasLoading: (personasLoading) => set({ personasLoading }), + + // Agent CRUD + setAgents: (agents) => set({ agents }), + + addAgent: (agent) => set((state) => ({ agents: [...state.agents, agent] })), + + updateAgent: (id, updates) => + set((state) => ({ + agents: state.agents.map((a) => + a.id === id + ? { ...a, ...updates, updatedAt: new Date().toISOString() } + : a, + ), + })), + + removeAgent: (id) => + set((state) => ({ + agents: state.agents.filter((a) => a.id !== id), + activeAgentId: state.activeAgentId === id ? null : state.activeAgentId, + })), + + setAgentsLoading: (agentsLoading) => set({ agentsLoading }), + + // Provider management + setProviders: (providers) => { + const { selectedProvider } = get(); + const isValid = providers.some((p) => p.id === selectedProvider); + if (!isValid && providers.length > 0) { + const fallback = providers[0].id; + persistProvider(fallback); + set({ providers, selectedProvider: fallback }); + } else { + set({ providers }); + } + }, + setProvidersLoading: (providersLoading) => set({ providersLoading }), + setSelectedProvider: (providerId, persist = true) => { + if (persist) { + persistProvider(providerId); + } + set({ selectedProvider: providerId }); + }, + + // Active agent + setActiveAgent: (id) => set({ activeAgentId: id }), + + getActiveAgent: () => { + const { activeAgentId, agents } = get(); + if (!activeAgentId) return null; + return agents.find((a) => a.id === activeAgentId) ?? null; + }, + + // Persona editor + openPersonaEditor: (persona) => + set({ + personaEditorOpen: true, + editingPersona: persona ?? null, + }), + + closePersonaEditor: () => + set({ + personaEditorOpen: false, + editingPersona: null, + }), + + // Loading + setLoading: (isLoading) => set({ isLoading }), + + // Helpers + getPersonaById: (id) => get().personas.find((p) => p.id === id), + + getAgentById: (id) => get().agents.find((a) => a.id === id), + + getAgentsByPersona: (personaId) => + get().agents.filter((a) => a.personaId === personaId), + + getBuiltinPersonas: () => get().personas.filter((p) => p.isBuiltin), + + getCustomPersonas: () => get().personas.filter((p) => !p.isBuiltin), +})); diff --git a/ui/goose2/src/features/agents/ui/AgentConfig.tsx b/ui/goose2/src/features/agents/ui/AgentConfig.tsx new file mode 100644 index 000000000000..2d8146d092e6 --- /dev/null +++ b/ui/goose2/src/features/agents/ui/AgentConfig.tsx @@ -0,0 +1,211 @@ +import { useState, useEffect, useCallback, useMemo } from "react"; +import { useTranslation } from "react-i18next"; +import { Button } from "@/shared/ui/button"; +import { Input } from "@/shared/ui/input"; +import { Label } from "@/shared/ui/label"; +import { Textarea } from "@/shared/ui/textarea"; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/shared/ui/select"; +import type { + Agent, + Persona, + ProviderType, + AgentConnectionType, + CreateAgentRequest, +} from "@/shared/types/agents"; +import { discoverAcpProviders, type AcpProvider } from "@/shared/api/acp"; + +interface AgentConfigProps { + agent?: Agent; + personas: Persona[]; + onSave: (config: CreateAgentRequest) => void; + onCancel: () => void; +} + +export function AgentConfig({ + agent, + personas, + onSave, + onCancel, +}: AgentConfigProps) { + const { t } = useTranslation(["agents", "common"]); + const [acpProviders, setAcpProviders] = useState([]); + + useEffect(() => { + discoverAcpProviders() + .then(setAcpProviders) + .catch(() => setAcpProviders([])); + }, []); + + const [name, setName] = useState(agent?.name ?? ""); + const [personaId, setPersonaId] = useState(agent?.personaId ?? ""); + const connectionType: AgentConnectionType = "builtin"; + const [provider, setProvider] = useState( + agent?.provider ?? "goose", + ); + const [model, setModel] = useState(agent?.model ?? ""); + const [systemPrompt, setSystemPrompt] = useState(agent?.systemPrompt ?? ""); + const [promptExpanded, setPromptExpanded] = useState(false); + + const selectedPersona = useMemo( + () => personas.find((p) => p.id === personaId), + [personas, personaId], + ); + + // Sync inherited fields when persona changes + useEffect(() => { + if (selectedPersona) { + setProvider(selectedPersona.provider ?? "goose"); + setModel(selectedPersona.model ?? ""); + setSystemPrompt(selectedPersona.systemPrompt); + } + }, [selectedPersona]); + + const isValid = name.trim().length > 0 && !!provider; + + const handleSubmit = useCallback( + (e: React.FormEvent) => { + e.preventDefault(); + if (!isValid) return; + + const config: CreateAgentRequest = { + name: name.trim(), + personaId: personaId || undefined, + provider, + model: model.trim(), + systemPrompt: systemPrompt.trim() || undefined, + connectionType, + }; + onSave(config); + }, + [isValid, name, personaId, provider, model, systemPrompt, onSave], + ); + + return ( +
+ {/* Name */} +
+ + setName(e.target.value)} + required + placeholder={t("config.namePlaceholder")} + /> +
+ + {/* Persona selector */} +
+ + +
+ + {/* Provider */} +
+ + +
+ + {/* Model override */} +
+ + setModel(e.target.value)} + placeholder={t("config.modelPlaceholder")} + /> +
+ + {/* System prompt override */} +
+ + {promptExpanded && ( +