From acc725788a0dbbb15f374a0fcc1f4813e486f90e Mon Sep 17 00:00:00 2001 From: Blaine Perry Date: Mon, 11 May 2026 11:36:18 -0500 Subject: [PATCH] feat 007: major refactor for future maintainability --- .github/agents/copilot-instructions.md | 4 +- .gitignore | 5 + agent_factory.py | 138 +-- frontend/package.json | 1 + frontend/src/App.css | 184 ---- frontend/src/api/client.contract.ts | 35 + frontend/src/api/client.ts | 230 +---- frontend/src/api/helpers.ts | 73 ++ .../src/components/AgentCapabilityPicker.tsx | 51 + frontend/src/components/ChatMessage.tsx | 16 +- frontend/src/components/SkillBuilder.tsx | 87 +- .../src/components/StarterQuestionEditor.tsx | 63 ++ frontend/src/components/ToolStep.tsx | 49 +- frontend/src/hooks/useAgentBuilderForm.ts | 123 +++ frontend/src/hooks/useAgentMcpEditor.ts | 87 ++ .../hooks/useBuiltInAgentCustomizations.ts | 23 +- frontend/src/hooks/useChat.ts | 328 +----- .../src/hooks/useConversationPersistence.ts | 72 ++ frontend/src/hooks/useConversationStore.ts | 75 +- frontend/src/hooks/useCustomAgents.ts | 16 +- frontend/src/hooks/useSessionLifecycle.ts | 244 +++++ frontend/src/hooks/useSkillForm.ts | 79 ++ frontend/src/hooks/useUserProfile.ts | 11 +- frontend/src/index.css | 89 -- frontend/src/pages/AgentBuilder.tsx | 311 ++---- frontend/src/styles/index.css | 212 +--- frontend/src/utils/content.ts | 71 ++ frontend/src/utils/storage.ts | 31 + main.py | 933 ++---------------- scripts/capture_admin_agent_screenshots.py | 108 +- session_orchestration.py | 405 ++++++++ skills_manager.py | 112 +++ .../contracts/api-contract.md | 65 ++ .../contracts/frontend-contract.md | 62 ++ specs/007-application-refactor/data-model.md | 151 +++ specs/007-application-refactor/plan.md | 129 +++ specs/007-application-refactor/quickstart.md | 372 +++++++ specs/007-application-refactor/research.md | 73 ++ specs/007-application-refactor/spec.md | 122 +++ specs/007-application-refactor/tasks.md | 307 ++++++ streaming.py | 259 +++++ tests/conftest.py | 42 + tests/test_api.py | 248 ++++- tests/test_image_input.py | 18 +- tests/test_provider_routing.py | 49 + tests/test_retry_logic.py | 119 ++- tests/test_session_orchestration.py | 20 + tests/test_skills_api.py | 51 +- tests/test_skills_manager.py | 49 + tests/test_streaming.py | 79 ++ tests/test_validators.py | 133 +++ validators.py | 150 +++ 52 files changed, 4315 insertions(+), 2449 deletions(-) delete mode 100644 frontend/src/App.css create mode 100644 frontend/src/api/client.contract.ts create mode 100644 frontend/src/api/helpers.ts create mode 100644 frontend/src/components/AgentCapabilityPicker.tsx create mode 100644 frontend/src/components/StarterQuestionEditor.tsx create mode 100644 frontend/src/hooks/useAgentBuilderForm.ts create mode 100644 frontend/src/hooks/useAgentMcpEditor.ts create mode 100644 frontend/src/hooks/useConversationPersistence.ts create mode 100644 frontend/src/hooks/useSessionLifecycle.ts create mode 100644 frontend/src/hooks/useSkillForm.ts create mode 100644 frontend/src/utils/content.ts create mode 100644 frontend/src/utils/storage.ts create mode 100644 session_orchestration.py create mode 100644 skills_manager.py create mode 100644 specs/007-application-refactor/contracts/api-contract.md create mode 100644 specs/007-application-refactor/contracts/frontend-contract.md create mode 100644 specs/007-application-refactor/data-model.md create mode 100644 specs/007-application-refactor/plan.md create mode 100644 specs/007-application-refactor/quickstart.md create mode 100644 specs/007-application-refactor/research.md create mode 100644 specs/007-application-refactor/spec.md create mode 100644 specs/007-application-refactor/tasks.md create mode 100644 streaming.py create mode 100644 tests/test_session_orchestration.py create mode 100644 tests/test_skills_manager.py create mode 100644 tests/test_streaming.py create mode 100644 tests/test_validators.py create mode 100644 validators.py diff --git a/.github/agents/copilot-instructions.md b/.github/agents/copilot-instructions.md index c16c833..02a3553 100644 --- a/.github/agents/copilot-instructions.md +++ b/.github/agents/copilot-instructions.md @@ -20,6 +20,8 @@ - Browser localStorage for per-user built-in agent overrides; existing `config/agents.yaml` remains canonical shared standard profile source; backend in-memory sessions unchanged (005-baked-agent-customization) - TypeScript 5.9 frontend; React 19; Python 3.12.6 backend unchanged + React, Vite 8, existing frontend API client, existing FastAPI skill endpoints, Playwright for visual verification (006-admin-skills-design-match) - Existing filesystem-backed skills under `skills//SKILL.md`; no new storage (006-admin-skills-design-match) +- Python 3.12.6, TypeScript 5.9, React 19 + FastAPI, agent-framework-core/openai/azure-ai-search, Azure SDKs, React, Vite, react-markdown, MSAL (007-application-refactor) +- In-memory backend sessions, file-backed `skills/`, browser localStorage, Terraform-managed Azure App Service resources (007-application-refactor) ## Project Structure @@ -52,6 +54,6 @@ uv run pytest # Run tests - Package manager: uv only (never pip) ## Recent Changes +- 007-application-refactor: Added Python 3.12.6, TypeScript 5.9, React 19 + FastAPI, agent-framework-core/openai/azure-ai-search, Azure SDKs, React, Vite, react-markdown, MSAL - 006-admin-skills-design-match: Added TypeScript 5.9 frontend; React 19; Python 3.12.6 backend unchanged + React, Vite 8, existing frontend API client, existing FastAPI skill endpoints, Playwright for visual verification - 005-baked-agent-customization: Added Python 3.12.6 backend; TypeScript 5.9 frontend; React 19; Vite 8 + FastAPI, agent-framework-core/openai/azure-ai-search, React, MSAL, localStorage APIs -- 004-azd-deploy-frontend: Added Python 3.12 (backend), TypeScript (frontend) + FastAPI, React/Vite, Terraform (azurerm ~> 4.0) diff --git a/.gitignore b/.gitignore index b779ba0..3a352d0 100644 --- a/.gitignore +++ b/.gitignore @@ -8,9 +8,12 @@ wheels/ # Virtual environments .venv +.venv/ +venv/ .env .env* +*.log .ruff* @@ -34,6 +37,8 @@ terraform.rc .mypy_cache/ .pytest_cache/ +coverage/ +frontend/coverage/ # Certificates certs/ .env.appsettings.json diff --git a/agent_factory.py b/agent_factory.py index 3f2cff1..19f7269 100644 --- a/agent_factory.py +++ b/agent_factory.py @@ -3,7 +3,7 @@ import re from dataclasses import dataclass from pathlib import Path -from typing import Any, Sequence, overload +from typing import Any, Sequence from agent_framework import CompactionProvider, InMemoryHistoryProvider, SkillsProvider from agent_framework import Agent as RuntimeAgent @@ -19,20 +19,10 @@ from dotenv import load_dotenv -from pydantic import BaseModel - from prompt_config import load_agent_profile from mcp_servers import get_search_context_provider -class AgentBase(BaseModel): - name: str - instructions: str - description: str - token_budget: int = 16_000 - tools: list[str] | None = None - - @dataclass(frozen=True) class ChatRuntime: agent: RuntimeAgent @@ -151,113 +141,6 @@ def _build_context_providers( return providers -def _create_agent( - *, - name: str, - instructions: str, - description: str, - token_budget: int, - temperature: float, - tools: Sequence[Any] | None = None, - client: OpenAIChatClient | None = None, - summarizer_client: OpenAIChatClient | None = None, - logical_profile: str | None = None, - enable_search_context: bool = False, - skill_names: list[str] | None = None, -) -> RuntimeAgent: - runtime_client = client or OpenAIChatClient() - - return runtime_client.as_agent( - name=_sanitize_agent_name(name), - instructions=instructions, - description=description, - tools=tools, - default_options={"temperature": temperature}, - context_providers=_build_context_providers( - token_budget=token_budget, - summarizer_client=summarizer_client, - logical_profile=logical_profile, - enable_search_context=enable_search_context, - skill_names=skill_names, - ), - ) - - -@overload -def spawn_agent(agent: AgentBase, /) -> RuntimeAgent: ... - - -@overload -def spawn_agent( - *, - name: str, - instructions: str, - description: str, - token_budget: int = 16_000, - temperature: float | None = None, - tools: Sequence[Any] | None = None, - client: OpenAIChatClient | None = None, - summarizer_client: OpenAIChatClient | None = None, - logical_profile: str | None = None, - enable_search_context: bool = False, - skill_names: list[str] | None = None, -) -> RuntimeAgent: ... - - -def spawn_agent( - agent: AgentBase | None = None, - /, - *, - name: str | None = None, - instructions: str | None = None, - description: str | None = None, - token_budget: int = 16_000, - temperature: float | None = None, - tools: Sequence[Any] | None = None, - client: OpenAIChatClient | None = None, - summarizer_client: OpenAIChatClient | None = None, - logical_profile: str | None = None, - enable_search_context: bool = False, - skill_names: list[str] | None = None, -) -> RuntimeAgent: - if agent is not None: - if any(value is not None for value in (name, instructions, description)): - raise TypeError("Pass either an agent model or expanded agent fields, not both.") - - return _create_agent( - name=agent.name, - instructions=agent.instructions, - description=agent.description, - token_budget=agent.token_budget, - temperature=temperature if temperature is not None else _get_default_temperature(), - tools=tools, - client=client, - summarizer_client=summarizer_client, - logical_profile=logical_profile, - enable_search_context=enable_search_context, - skill_names=skill_names, - ) - - if name is None or instructions is None or description is None: - raise TypeError( - "spawn_agent() requires either an agent model or name, instructions, and description keywords." - ) - - return _create_agent( - name=name, - instructions=instructions, - description=description, - token_budget=token_budget, - temperature=temperature if temperature is not None else _get_default_temperature(), - tools=tools, - client=client, - summarizer_client=summarizer_client, - logical_profile=logical_profile, - enable_search_context=enable_search_context, - skill_names=skill_names, - ) - - def _build_openai_clients() -> tuple[OpenAIChatClient, OpenAIChatClient]: """Create primary and summarizer OpenAI clients from environment variables. @@ -344,18 +227,19 @@ def create_chat_runtime( resolved_temperature = temperature if temperature is not None else _get_default_temperature() - agent = spawn_agent( - client=primary_client, - name=agent_name, + agent = primary_client.as_agent( + name=_sanitize_agent_name(agent_name), instructions=runtime_instructions, description=description, - token_budget=token_budget, - temperature=resolved_temperature, tools=all_tools, - summarizer_client=summarizer_client, - logical_profile=logical_profile, - enable_search_context=enable_search_context, - skill_names=skill_names, + default_options={"temperature": resolved_temperature}, + context_providers=_build_context_providers( + token_budget=token_budget, + summarizer_client=summarizer_client, + logical_profile=logical_profile, + enable_search_context=enable_search_context, + skill_names=skill_names, + ), ) logger.info( diff --git a/frontend/package.json b/frontend/package.json index ab2d207..040efa2 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -5,6 +5,7 @@ "type": "module", "scripts": { "dev": "vite", + "test": "tsc -b --pretty false", "build": "tsc -b && vite build", "lint": "eslint .", "preview": "vite preview" diff --git a/frontend/src/App.css b/frontend/src/App.css deleted file mode 100644 index f90339d..0000000 --- a/frontend/src/App.css +++ /dev/null @@ -1,184 +0,0 @@ -.counter { - font-size: 16px; - padding: 5px 10px; - border-radius: 5px; - color: var(--accent); - background: var(--accent-bg); - border: 2px solid transparent; - transition: border-color 0.3s; - margin-bottom: 24px; - - &:hover { - border-color: var(--accent-border); - } - &:focus-visible { - outline: 2px solid var(--accent); - outline-offset: 2px; - } -} - -.hero { - position: relative; - - .base, - .framework, - .vite { - inset-inline: 0; - margin: 0 auto; - } - - .base { - width: 170px; - position: relative; - z-index: 0; - } - - .framework, - .vite { - position: absolute; - } - - .framework { - z-index: 1; - top: 34px; - height: 28px; - transform: perspective(2000px) rotateZ(300deg) rotateX(44deg) rotateY(39deg) - scale(1.4); - } - - .vite { - z-index: 0; - top: 107px; - height: 26px; - width: auto; - transform: perspective(2000px) rotateZ(300deg) rotateX(40deg) rotateY(39deg) - scale(0.8); - } -} - -#center { - display: flex; - flex-direction: column; - gap: 25px; - place-content: center; - place-items: center; - flex-grow: 1; - - @media (max-width: 1024px) { - padding: 32px 20px 24px; - gap: 18px; - } -} - -#next-steps { - display: flex; - border-top: 1px solid var(--border); - text-align: left; - - & > div { - flex: 1 1 0; - padding: 32px; - @media (max-width: 1024px) { - padding: 24px 20px; - } - } - - .icon { - margin-bottom: 16px; - width: 22px; - height: 22px; - } - - @media (max-width: 1024px) { - flex-direction: column; - text-align: center; - } -} - -#docs { - border-right: 1px solid var(--border); - - @media (max-width: 1024px) { - border-right: none; - border-bottom: 1px solid var(--border); - } -} - -#next-steps ul { - list-style: none; - padding: 0; - display: flex; - gap: 8px; - margin: 32px 0 0; - - .logo { - height: 18px; - } - - a { - color: var(--text-h); - font-size: 16px; - border-radius: 6px; - background: var(--social-bg); - display: flex; - padding: 6px 12px; - align-items: center; - gap: 8px; - text-decoration: none; - transition: box-shadow 0.3s; - - &:hover { - box-shadow: var(--shadow); - } - .button-icon { - height: 18px; - width: 18px; - } - } - - @media (max-width: 1024px) { - margin-top: 20px; - flex-wrap: wrap; - justify-content: center; - - li { - flex: 1 1 calc(50% - 8px); - } - - a { - width: 100%; - justify-content: center; - box-sizing: border-box; - } - } -} - -#spacer { - height: 88px; - border-top: 1px solid var(--border); - @media (max-width: 1024px) { - height: 48px; - } -} - -.ticks { - position: relative; - width: 100%; - - &::before, - &::after { - content: ''; - position: absolute; - top: -4.5px; - border: 5px solid transparent; - } - - &::before { - left: 0; - border-left-color: var(--border); - } - &::after { - right: 0; - border-right-color: var(--border); - } -} diff --git a/frontend/src/api/client.contract.ts b/frontend/src/api/client.contract.ts new file mode 100644 index 0000000..4870629 --- /dev/null +++ b/frontend/src/api/client.contract.ts @@ -0,0 +1,35 @@ +import { + AuthError, + createSessionRequest, + createSkill, + deleteSession, + deleteSkill, + fetchBuiltInProfileDefinition, + fetchHistory, + fetchProfiles, + fetchSkill, + fetchSkills, + fetchTools, + generateSkillContent, + sendMessage, + testMcpConnections, + updateSkill, +} from './client'; + +export const apiExportContract = { + AuthError, + createSessionRequest, + createSkill, + deleteSession, + deleteSkill, + fetchBuiltInProfileDefinition, + fetchHistory, + fetchProfiles, + fetchSkill, + fetchSkills, + fetchTools, + generateSkillContent, + sendMessage, + testMcpConnections, + updateSkill, +}; \ No newline at end of file diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 7d65648..cf5368c 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -1,6 +1,5 @@ import type { AgentProfile, - AgentCustomizationOverride, BuiltInAgentDefinition, McpConnectionResult, McpServerEntry, @@ -11,7 +10,6 @@ import type { ToolInfo, ToolsResponse, UnavailableAgent, - UserMemoryProfile, SSEEventType, SSETextEvent, SSEFunctionCallEvent, @@ -20,68 +18,17 @@ import type { SSEErrorEvent, } from '../types/api'; import { emitToast } from '../hooks/useToast'; - -const API_BASE = '/api'; - -/** Thrown when the backend returns 401 Unauthorized. */ -export class AuthError extends Error { - constructor(message: string) { - super(message); - this.name = 'AuthError'; - } -} - -function getAuthHeaders(): Record { - const token = localStorage.getItem('auth_token'); - if (token) { - return { Authorization: `Bearer ${token}` }; - } - return {}; -} - -/** Check response for 401 and throw AuthError so callers can trigger re-auth. */ -function assertNotUnauthorized(resp: Response, context: string): void { - if (resp.status === 401) { - throw new AuthError(`Unauthorized: ${context}`); - } -} - -/** Try to extract a user-friendly detail string from an error response body. */ -async function extractErrorDetail(resp: Response, fallback: string): Promise { - try { - const body = await resp.json(); - if (body.detail && typeof body.detail === 'string') return body.detail; - } catch { /* not JSON or no detail field */ } - return fallback; -} - -/** Classify an HTTP error into user-friendly message and severity. */ -function classifyError(status: number, detail: string): { - userMessage: string; - type: 'error' | 'warning'; - retryable: boolean; -} { - const lowerDetail = detail.toLowerCase(); - - if (status === 429 || lowerDetail.includes('rate limit') || lowerDetail.includes('too many requests')) { - return { userMessage: detail || 'Rate limit exceeded. Please wait and try again.', type: 'warning', retryable: true }; - } - if (status === 400) { - return { userMessage: detail || 'Invalid request.', type: 'error', retryable: false }; - } - if (status >= 500) { - return { userMessage: detail || 'A server error occurred. Please try again later.', type: 'error', retryable: false }; - } - return { userMessage: detail || `Request failed (${status})`, type: 'error', retryable: false }; -} - -/** Extract detail, classify, emit toast, and throw — shared by all API functions. */ -async function handleHttpError(resp: Response, context: string): Promise { - const detail = await extractErrorDetail(resp, `${context}: ${resp.status}`); - const classified = classifyError(resp.status, detail); - emitToast({ message: classified.userMessage, type: classified.type }); - throw new Error(detail); -} +import { + API_BASE, + assertNotUnauthorized, + classifyError, + getAuthHeaders, + handleHttpError, + jsonHeaders, + requestJson, +} from './helpers'; + +export { AuthError } from './helpers'; export interface HistoryResponse { session_id: string; @@ -116,21 +63,15 @@ export async function fetchSkills(): Promise { } export async function fetchSkill(name: string): Promise { - const resp = await fetch(`${API_BASE}/skills/${encodeURIComponent(name)}`, { + return requestJson(`${API_BASE}/skills/${encodeURIComponent(name)}`, { headers: getAuthHeaders(), - }); - assertNotUnauthorized(resp, 'Failed to fetch skill'); - if (!resp.ok) await handleHttpError(resp, 'Failed to fetch skill'); - return resp.json(); + }, 'Failed to fetch skill'); } export async function createSkill(skill: SkillCreatePayload): Promise { const resp = await fetch(`${API_BASE}/skills`, { method: 'POST', - headers: { - 'Content-Type': 'application/json', - ...getAuthHeaders(), - }, + headers: jsonHeaders(), body: JSON.stringify(skill), }); assertNotUnauthorized(resp, 'Failed to create skill'); @@ -141,10 +82,7 @@ export async function createSkill(skill: SkillCreatePayload): Promise { const resp = await fetch(`${API_BASE}/skills/${encodeURIComponent(name)}`, { method: 'PUT', - headers: { - 'Content-Type': 'application/json', - ...getAuthHeaders(), - }, + headers: jsonHeaders(), body: JSON.stringify(payload), }); assertNotUnauthorized(resp, 'Failed to update skill'); @@ -167,10 +105,7 @@ export async function generateSkillContent( ): Promise { const resp = await fetch(`${API_BASE}/skills/generate`, { method: 'POST', - headers: { - 'Content-Type': 'application/json', - ...getAuthHeaders(), - }, + headers: jsonHeaders(), body: JSON.stringify({ description, ...(name ? { name } : {}) }), }); assertNotUnauthorized(resp, 'Failed to generate skill content'); @@ -182,10 +117,7 @@ export async function generateSkillContent( export async function testMcpConnections(servers: McpServerEntry[]): Promise { const resp = await fetch(`${API_BASE}/mcp/test`, { method: 'POST', - headers: { - 'Content-Type': 'application/json', - ...getAuthHeaders(), - }, + headers: jsonHeaders(), body: JSON.stringify({ mcp_servers: servers }), }); assertNotUnauthorized(resp, 'Failed to test MCP connections'); @@ -194,40 +126,48 @@ export async function testMcpConnections(servers: McpServerEntry[]): Promise; - user_profile?: { name: string; preferences: string; notes: string }; - [key: string]: unknown; -}): Promise { - const { custom_name, custom_prompt, custom_tools, custom_search_context, custom_temperature, custom_skills, mcp_servers, history, user_profile } = params; + user_profile?: SessionUserProfilePayload; +} + +export async function createSessionRequest( + payload: SessionRequestPayload, + failureMessage = 'Failed to create session', +): Promise { const resp = await fetch(`${API_BASE}/sessions`, { method: 'POST', - headers: { - 'Content-Type': 'application/json', - ...getAuthHeaders(), - }, - body: JSON.stringify({ - profile_id: 'custom', - custom_name, - custom_prompt, - custom_tools, - custom_search_context, - ...(custom_temperature !== undefined ? { custom_temperature } : {}), - ...(custom_skills && custom_skills.length > 0 ? { custom_skills } : {}), - ...(mcp_servers && mcp_servers.length > 0 ? { mcp_servers } : {}), - ...(history ? { history } : {}), - ...(user_profile ? { user_profile } : {}), - }), + headers: jsonHeaders(), + body: JSON.stringify(payload), }); - assertNotUnauthorized(resp, 'Failed to create custom session'); - if (!resp.ok) await handleHttpError(resp, 'Failed to create custom session'); + assertNotUnauthorized(resp, failureMessage); + if (!resp.ok) await handleHttpError(resp, failureMessage); return resp.json(); } @@ -240,39 +180,6 @@ export async function fetchBuiltInProfileDefinition(profileId: string): Promise< return resp.json(); } -export async function createSessionWithProfileOverride( - profileId: string, - override: AgentCustomizationOverride, - userProfile?: UserMemoryProfile | null, - history?: Record, -): Promise { - const resp = await fetch(`${API_BASE}/sessions`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - ...getAuthHeaders(), - }, - body: JSON.stringify({ - profile_id: profileId, - profile_override: { - description: override.description, - custom_prompt: override.systemPrompt, - custom_tools: override.tools, - custom_search_context: override.useSearchContext, - ...(override.temperature !== undefined ? { custom_temperature: override.temperature } : {}), - ...(override.skills.length > 0 ? { custom_skills: override.skills } : {}), - ...(override.mcpServers.length > 0 ? { mcp_servers: override.mcpServers } : {}), - override_updated_at: override.updatedAt, - }, - ...(history ? { history } : {}), - ...(userProfile ? { user_profile: { name: userProfile.name, preferences: userProfile.preferences, notes: userProfile.notes } } : {}), - }), - }); - assertNotUnauthorized(resp, 'Failed to create customized built-in session'); - if (!resp.ok) await handleHttpError(resp, 'Failed to create customized built-in session'); - return resp.json(); -} - export interface ProfilesResponse { profiles: AgentProfile[]; unavailable: UnavailableAgent[]; @@ -288,23 +195,6 @@ export async function fetchProfiles(): Promise { return { profiles: data.profiles, unavailable: data.unavailable || [] }; } -export async function createSession(profileId: string, userProfile?: UserMemoryProfile | null): Promise { - const resp = await fetch(`${API_BASE}/sessions`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - ...getAuthHeaders(), - }, - body: JSON.stringify({ - profile_id: profileId, - ...(userProfile ? { user_profile: { name: userProfile.name, preferences: userProfile.preferences, notes: userProfile.notes } } : {}), - }), - }); - assertNotUnauthorized(resp, 'Failed to create session'); - if (!resp.ok) await handleHttpError(resp, 'Failed to create session'); - return resp.json(); -} - export async function deleteSession(sessionId: string): Promise { const resp = await fetch(`${API_BASE}/sessions/${encodeURIComponent(sessionId)}`, { method: 'DELETE', @@ -323,28 +213,6 @@ export async function fetchHistory(sessionId: string): Promise return resp.json(); } -export async function createSessionWithHistory( - profileId: string, - history: Record, - userProfile?: UserMemoryProfile | null, -): Promise { - const resp = await fetch(`${API_BASE}/sessions`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - ...getAuthHeaders(), - }, - body: JSON.stringify({ - profile_id: profileId, - history, - ...(userProfile ? { user_profile: { name: userProfile.name, preferences: userProfile.preferences, notes: userProfile.notes } } : {}), - }), - }); - assertNotUnauthorized(resp, 'Failed to create session with history'); - if (!resp.ok) await handleHttpError(resp, 'Failed to create session with history'); - return resp.json(); -} - export interface SSECallback { onText?: (data: SSETextEvent) => void; onFunctionCall?: (data: SSEFunctionCallEvent) => void; diff --git a/frontend/src/api/helpers.ts b/frontend/src/api/helpers.ts new file mode 100644 index 0000000..738599e --- /dev/null +++ b/frontend/src/api/helpers.ts @@ -0,0 +1,73 @@ +import { emitToast } from '../hooks/useToast'; + +export const API_BASE = '/api'; + +/** Thrown when the backend returns 401 Unauthorized. */ +export class AuthError extends Error { + constructor(message: string) { + super(message); + this.name = 'AuthError'; + } +} + +export function getAuthHeaders(): Record { + const token = localStorage.getItem('auth_token'); + if (token) { + return { Authorization: `Bearer ${token}` }; + } + return {}; +} + +export function assertNotUnauthorized(resp: Response, context: string): void { + if (resp.status === 401) { + throw new AuthError(`Unauthorized: ${context}`); + } +} + +async function extractErrorDetail(resp: Response, fallback: string): Promise { + try { + const body = await resp.json(); + if (body.detail && typeof body.detail === 'string') return body.detail; + } catch { /* not JSON or no detail field */ } + return fallback; +} + +export function classifyError(status: number, detail: string): { + userMessage: string; + type: 'error' | 'warning'; + retryable: boolean; +} { + const lowerDetail = detail.toLowerCase(); + + if (status === 429 || lowerDetail.includes('rate limit') || lowerDetail.includes('too many requests')) { + return { userMessage: detail || 'Rate limit exceeded. Please wait and try again.', type: 'warning', retryable: true }; + } + if (status === 400) { + return { userMessage: detail || 'Invalid request.', type: 'error', retryable: false }; + } + if (status >= 500) { + return { userMessage: detail || 'A server error occurred. Please try again later.', type: 'error', retryable: false }; + } + return { userMessage: detail || `Request failed (${status})`, type: 'error', retryable: false }; +} + +export async function handleHttpError(resp: Response, context: string): Promise { + const detail = await extractErrorDetail(resp, `${context}: ${resp.status}`); + const classified = classifyError(resp.status, detail); + emitToast({ message: classified.userMessage, type: classified.type }); + throw new Error(detail); +} + +export function jsonHeaders(): Record { + return { + 'Content-Type': 'application/json', + ...getAuthHeaders(), + }; +} + +export async function requestJson(url: string, init: RequestInit, context: string): Promise { + const resp = await fetch(url, init); + assertNotUnauthorized(resp, context); + if (!resp.ok) await handleHttpError(resp, context); + return resp.json() as Promise; +} \ No newline at end of file diff --git a/frontend/src/components/AgentCapabilityPicker.tsx b/frontend/src/components/AgentCapabilityPicker.tsx new file mode 100644 index 0000000..a0b00f0 --- /dev/null +++ b/frontend/src/components/AgentCapabilityPicker.tsx @@ -0,0 +1,51 @@ +import type { ToolInfo } from '../types/api'; + +interface AgentCapabilityPickerProps { + title: string; + description: string; + items: ToolInfo[]; + selected: string[]; + loading: boolean; + emptyText?: string; + nameFormatter?: (name: string) => string; + onToggle: (name: string) => void; +} + +export function AgentCapabilityPicker({ + title, + description, + items, + selected, + loading, + emptyText = 'No items available', + nameFormatter = (name) => name, + onToggle, +}: AgentCapabilityPickerProps) { + return ( +
+ {title} + + {description} + + {loading ? ( +
Loading available {title.toLowerCase()}...
+ ) : items.length > 0 ? ( +
+ {items.map((item) => ( + + ))} +
+ ) : ( +
{emptyText}
+ )} +
+ ); +} \ No newline at end of file diff --git a/frontend/src/components/ChatMessage.tsx b/frontend/src/components/ChatMessage.tsx index e78001d..7fbfc71 100644 --- a/frontend/src/components/ChatMessage.tsx +++ b/frontend/src/components/ChatMessage.tsx @@ -1,10 +1,9 @@ import ReactMarkdown from 'react-markdown'; import remarkGfm from 'remark-gfm'; -import type { ChatMessage as ChatMessageType, ContentItem } from '../types/api'; +import type { ChatMessage as ChatMessageType } from '../types/api'; import { ToolStep } from './ToolStep'; import { getRuntimeConfigSnapshot } from '../config/runtimeConfig'; - -const ALLOWED_IMAGE_MIMES = new Set(['image/jpeg', 'image/png', 'image/gif', 'image/webp']); +import { hasToolImages, imageDataUri, toolImages } from '../utils/content'; interface Props { message: ChatMessageType; @@ -32,18 +31,15 @@ export function ChatMessage({ message }: Props) { )} {/* Inline images from tool results — shown outside collapsed accordions */} - {message.tool_invocations && message.tool_invocations.some((t) => t.content_items?.some((item: ContentItem) => item.type === 'image')) && ( + {hasToolImages(message.tool_invocations) && (
- {message.tool_invocations.flatMap((t) => - (t.content_items ?? []) - .filter((item: ContentItem): item is ContentItem & { type: 'image' } => - item.type === 'image' && ALLOWED_IMAGE_MIMES.has((item as { mimeType?: string }).mimeType ?? '') - ) + {(message.tool_invocations ?? []).flatMap((t) => + toolImages(t) .map((item, idx) => ( {`Result )) diff --git a/frontend/src/components/SkillBuilder.tsx b/frontend/src/components/SkillBuilder.tsx index 9a357c7..efee137 100644 --- a/frontend/src/components/SkillBuilder.tsx +++ b/frontend/src/components/SkillBuilder.tsx @@ -1,33 +1,39 @@ import { useState, useEffect, useCallback } from 'react'; -import type { SkillSummary, SkillDefinition, SkillCreatePayload } from '../types/api'; +import type { SkillSummary, SkillDefinition } from '../types/api'; import { fetchSkills, fetchSkill, createSkill, updateSkill, deleteSkill, generateSkillContent } from '../api/client'; - -type ViewMode = 'list' | 'create' | 'edit'; - -const SKILL_NAME_RE = /^[a-z0-9][a-z0-9-]*$/; +import { SKILL_NAME_RE, useSkillForm } from '../hooks/useSkillForm'; export function SkillBuilder() { const [skills, setSkills] = useState([]); const [loading, setLoading] = useState(true); - const [view, setView] = useState('list'); const [skillsCollapsed, setSkillsCollapsed] = useState(false); - - // Form state - const [formName, setFormName] = useState(''); - const [formDescription, setFormDescription] = useState(''); - const [formContent, setFormContent] = useState(''); - const [editingName, setEditingName] = useState(null); - const [formLoading, setFormLoading] = useState(false); - - // Feedback - const [successMsg, setSuccessMsg] = useState(''); - const [errorMsg, setErrorMsg] = useState(''); - - // Delete confirm - const [deletingName, setDeletingName] = useState(null); - - // AI generation - const [aiLoading, setAiLoading] = useState(false); + const { + aiLoading, + clearFeedback, + createPayload, + deletingName, + editingName, + errorMsg, + formContent, + formDescription, + formLoading, + formName, + isCreateFormValid, + isEditFormValid, + resetForm, + setAiLoading, + setDeletingName, + setEditingName, + setErrorMsg, + setFormContent, + setFormDescription, + setFormLoading, + setFormName, + setSuccessMsg, + setView, + successMsg, + view, + } = useSkillForm(); const loadSkills = useCallback(async () => { setLoading(true); @@ -40,24 +46,12 @@ export function SkillBuilder() { } finally { setLoading(false); } - }, []); + }, [setErrorMsg]); useEffect(() => { loadSkills(); }, [loadSkills]); - const clearFeedback = () => { - setSuccessMsg(''); - setErrorMsg(''); - }; - - const resetForm = () => { - setFormName(''); - setFormDescription(''); - setFormContent(''); - setEditingName(null); - }; - const handleOpenCreate = () => { clearFeedback(); resetForm(); @@ -104,12 +98,7 @@ export function SkillBuilder() { } setFormLoading(true); try { - const payload: SkillCreatePayload = { - name: formName, - description: formDescription, - content: formContent, - }; - await createSkill(payload); + await createSkill(createPayload()); setSuccessMsg(`Skill "${formName}" created successfully.`); resetForm(); await loadSkills(); @@ -182,20 +171,6 @@ export function SkillBuilder() { } }; - const isCreateFormValid = - SKILL_NAME_RE.test(formName) && - formName.length <= 64 && - formDescription.trim().length > 0 && - formDescription.length <= 256 && - formContent.trim().length > 0 && - formContent.length <= 65536; - - const isEditFormValid = - formDescription.trim().length > 0 && - formDescription.length <= 256 && - formContent.trim().length > 0 && - formContent.length <= 65536; - const isEditing = view === 'edit'; const formTitle = isEditing ? `EDIT SKILL${editingName ? ` - ${editingName}` : ''}` : 'CREATE NEW SKILL'; const submitLabel = isEditing ? 'SAVE CHANGES' : 'CREATE SKILL'; diff --git a/frontend/src/components/StarterQuestionEditor.tsx b/frontend/src/components/StarterQuestionEditor.tsx new file mode 100644 index 0000000..10e016b --- /dev/null +++ b/frontend/src/components/StarterQuestionEditor.tsx @@ -0,0 +1,63 @@ +import type { StarterQuestion } from '../types/api'; + +interface StarterQuestionEditorProps { + starters: StarterQuestion[]; + newStarterLabel: string; + newStarterMessage: string; + onAdd: () => void; + onRemove: (index: number) => void; + onLabelChange: (value: string) => void; + onMessageChange: (value: string) => void; +} + +export function StarterQuestionEditor({ + starters, + newStarterLabel, + newStarterMessage, + onAdd, + onRemove, + onLabelChange, + onMessageChange, +}: StarterQuestionEditorProps) { + return ( +
+ STARTER QUESTIONS + {starters.length > 0 && ( +
+ {starters.map((starter, index) => ( +
+ {starter.label} + +
+ ))} +
+ )} +
+ onLabelChange(event.target.value)} + placeholder="Button label" + maxLength={80} + /> + onMessageChange(event.target.value)} + placeholder="Message to send" + maxLength={500} + /> + +
+
+ ); +} \ No newline at end of file diff --git a/frontend/src/components/ToolStep.tsx b/frontend/src/components/ToolStep.tsx index 8918a12..4338418 100644 --- a/frontend/src/components/ToolStep.tsx +++ b/frontend/src/components/ToolStep.tsx @@ -1,41 +1,14 @@ import { useState } from 'react'; -import type { ToolInvocation, ContentItem } from '../types/api'; - -const ALLOWED_IMAGE_MIMES = new Set(['image/jpeg', 'image/png', 'image/gif', 'image/webp']); +import type { ToolInvocation } from '../types/api'; +import { formatToolResult, imageDataUri, toolImages } from '../utils/content'; interface Props { invocation: ToolInvocation; } -function prettyFormat(raw: string): string { - // Try direct JSON parse first - try { - return JSON.stringify(JSON.parse(raw), null, 2); - } catch { - // Try converting Python repr to JSON (single quotes → double, True/False/None) - try { - const jsonified = raw - .replace(/datetime\.datetime\([^)]+\)/g, (m) => { - const nums = m.match(/\d+/g); - if (nums && nums.length >= 3) { - const [y, mo, d, h = '0', mi = '0', s = '0'] = nums; - return `"${y}-${mo.padStart(2, '0')}-${d.padStart(2, '0')}T${h.padStart(2, '0')}:${mi.padStart(2, '0')}:${s.padStart(2, '0')}"`; - } - return `"${m}"`; - }) - .replace(/'/g, '"') - .replace(/\bTrue\b/g, 'true') - .replace(/\bFalse\b/g, 'false') - .replace(/\bNone\b/g, 'null'); - return JSON.stringify(JSON.parse(jsonified), null, 2); - } catch { - return raw; - } - } -} - export function ToolStep({ invocation }: Props) { const [expanded, setExpanded] = useState(false); + const images = toolImages(invocation); return (
@@ -56,25 +29,21 @@ export function ToolStep({ invocation }: Props) {
Arguments
- 
{prettyFormat(invocation.arguments)}
+ 
{formatToolResult(invocation.arguments)}
{invocation.result && (
Result
- 
{prettyFormat(invocation.result)}
- {invocation.content_items && invocation.content_items.length > 0 && ( + 
{formatToolResult(invocation.result)}
+ {images.length > 0 && (
- {invocation.content_items - .filter((item: ContentItem): item is ContentItem & { type: 'image' } => - item.type === 'image' && ALLOWED_IMAGE_MIMES.has((item as { mimeType?: string }).mimeType ?? '') - ) - .map((item, idx) => ( + {images.map((item, idx) => ( {`Tool window.open(`data:${item.mimeType};base64,${item.data}`, '_blank')} + onClick={() => window.open(imageDataUri(item), '_blank')} /> ))}
diff --git a/frontend/src/hooks/useAgentBuilderForm.ts b/frontend/src/hooks/useAgentBuilderForm.ts new file mode 100644 index 0000000..661668b --- /dev/null +++ b/frontend/src/hooks/useAgentBuilderForm.ts @@ -0,0 +1,123 @@ +import { useMemo, useState } from 'react'; +import type { + AgentCustomizationOverride, + BuiltInAgentDefinition, + CustomAgentDefinition, + McpServerEntry, + StarterQuestion, +} from '../types/api'; + +export interface AgentBuilderFormState { + name: string; + description: string; + systemPrompt: string; + tools: string[]; + skills: string[]; + mcpServers: McpServerEntry[]; + useSearchContext: boolean; + icon: string; + starters: StarterQuestion[]; + temperature: string; +} + +export const EMPTY_AGENT_FORM: AgentBuilderFormState = { + name: '', + description: '', + systemPrompt: '', + tools: [], + skills: [], + mcpServers: [], + useSearchContext: false, + icon: '/icons/custom.svg', + starters: [], + temperature: '', +}; + +export function useAgentBuilderForm() { + const [editingId, setEditingId] = useState(null); + const [editingBuiltInDefinition, setEditingBuiltInDefinition] = useState(null); + const [form, setForm] = useState(EMPTY_AGENT_FORM); + const [touched, setTouched] = useState>({}); + const [saveSuccess, setSaveSuccess] = useState(false); + + const parsedTemperature = form.temperature !== '' ? parseFloat(form.temperature) : undefined; + const temperatureValid = parsedTemperature === undefined || (!isNaN(parsedTemperature) && parsedTemperature >= 0 && parsedTemperature <= 2); + const isValid = Boolean((editingBuiltInDefinition || form.name.trim()) && form.systemPrompt.trim() && temperatureValid); + + const resetForm = () => { + setForm(EMPTY_AGENT_FORM); + setEditingId(null); + setEditingBuiltInDefinition(null); + setTouched({}); + }; + + const markTouched = (field: string) => setTouched((prev) => ({ ...prev, [field]: true })); + + return useMemo(() => ({ + editingId, + editingBuiltInDefinition, + form, + isValid, + parsedTemperature, + resetForm, + saveSuccess, + setEditingBuiltInDefinition, + setEditingId, + setForm, + setSaveSuccess, + setTouched, + touched, + markTouched, + temperatureValid, + }), [editingBuiltInDefinition, editingId, form, isValid, parsedTemperature, saveSuccess, touched, temperatureValid]); +} + +export function prepareCustomAgent( + form: AgentBuilderFormState, + editingId: string | null, + agents: CustomAgentDefinition[], + parsedTemperature: number | undefined, +): CustomAgentDefinition { + const now = new Date().toISOString(); + return { + id: editingId || `custom_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`, + name: form.name.trim(), + description: form.description.trim(), + systemPrompt: form.systemPrompt, + tools: form.tools, + skills: form.skills, + mcpServers: form.mcpServers, + useSearchContext: form.useSearchContext, + icon: form.icon, + starters: form.starters, + ...(parsedTemperature !== undefined ? { temperature: parsedTemperature } : {}), + createdAt: editingId ? agents.find((agent) => agent.id === editingId)?.createdAt || now : now, + updatedAt: now, + }; +} + +export function prepareBuiltInOverride( + form: AgentBuilderFormState, + definition: BuiltInAgentDefinition, + existingOverride: AgentCustomizationOverride | undefined, + parsedTemperature: number | undefined, +): AgentCustomizationOverride { + const now = new Date().toISOString(); + return { + id: existingOverride?.id ?? `builtin_override_${definition.id}`, + baseProfileId: definition.id, + baseProfileName: definition.name, + description: form.description.trim(), + systemPrompt: form.systemPrompt, + tools: form.tools, + skills: form.skills, + mcpServers: form.mcpServers, + useSearchContext: form.useSearchContext, + icon: definition.icon, + starters: form.starters, + ...(parsedTemperature !== undefined ? { temperature: parsedTemperature } : {}), + source: 'builtin-override', + createdAt: existingOverride?.createdAt ?? now, + updatedAt: now, + }; +} \ No newline at end of file diff --git a/frontend/src/hooks/useAgentMcpEditor.ts b/frontend/src/hooks/useAgentMcpEditor.ts new file mode 100644 index 0000000..a4b1226 --- /dev/null +++ b/frontend/src/hooks/useAgentMcpEditor.ts @@ -0,0 +1,87 @@ +import { useState } from 'react'; +import type { Dispatch, SetStateAction } from 'react'; +import type { McpConnectionResult, McpServerEntry } from '../types/api'; +import { testMcpConnections } from '../api/client'; +import type { AgentBuilderFormState } from './useAgentBuilderForm'; + +export function useAgentMcpEditor(setForm: Dispatch>) { + const [newMcpName, setNewMcpName] = useState(''); + const [newMcpUrl, setNewMcpUrl] = useState(''); + const [newMcpAuth, setNewMcpAuth] = useState(false); + const [newMcpAuthScope, setNewMcpAuthScope] = useState(''); + const [mcpTestResults, setMcpTestResults] = useState>({}); + const [mcpTesting, setMcpTesting] = useState(false); + + const resetMcpEditor = () => { + setNewMcpName(''); + setNewMcpUrl(''); + setNewMcpAuth(false); + setNewMcpAuthScope(''); + setMcpTestResults({}); + }; + + const addMcpServer = () => { + if (!newMcpName.trim() || !newMcpUrl.trim()) return; + setForm((prev) => ({ + ...prev, + mcpServers: [...prev.mcpServers, { + name: newMcpName.trim(), + transport: 'http' as const, + url: newMcpUrl.trim(), + ...(newMcpAuth ? { authenticated: true } : {}), + ...(newMcpAuthScope.trim() ? { authScope: newMcpAuthScope.trim() } : {}), + }], + })); + setNewMcpName(''); + setNewMcpUrl(''); + setNewMcpAuth(false); + setNewMcpAuthScope(''); + }; + + const removeMcpServer = (server: McpServerEntry, index: number) => { + setForm((prev) => ({ + ...prev, + mcpServers: prev.mcpServers.filter((_, idx) => idx !== index), + })); + setMcpTestResults((prev) => { + const next = { ...prev }; + delete next[server.name]; + return next; + }); + }; + + const testConnections = async (servers: McpServerEntry[]) => { + if (servers.length === 0) return; + setMcpTesting(true); + setMcpTestResults({}); + try { + const results = await testMcpConnections(servers); + const map: Record = {}; + for (const result of results) { + map[result.name] = result; + } + setMcpTestResults(map); + } catch { + // Error already shown via toast by client.ts. + } finally { + setMcpTesting(false); + } + }; + + return { + addMcpServer, + mcpTesting, + mcpTestResults, + newMcpAuth, + newMcpAuthScope, + newMcpName, + newMcpUrl, + removeMcpServer, + resetMcpEditor, + setNewMcpAuth, + setNewMcpAuthScope, + setNewMcpName, + setNewMcpUrl, + testConnections, + } as const; +} \ No newline at end of file diff --git a/frontend/src/hooks/useBuiltInAgentCustomizations.ts b/frontend/src/hooks/useBuiltInAgentCustomizations.ts index 4da755e..6b7cf13 100644 --- a/frontend/src/hooks/useBuiltInAgentCustomizations.ts +++ b/frontend/src/hooks/useBuiltInAgentCustomizations.ts @@ -1,5 +1,6 @@ import { useCallback, useState } from 'react'; import type { AgentCustomizationOverride } from '../types/api'; +import { readJson, writeJson } from '../utils/storage'; const STORAGE_KEY = 'webagents_builtin_agent_customizations'; @@ -27,28 +28,14 @@ function isOverride(value: unknown): value is AgentCustomizationOverride { ); } -function loadOverrides(): AgentCustomizationOverride[] { - try { - const raw = localStorage.getItem(STORAGE_KEY); - if (!raw) return []; - const parsed = JSON.parse(raw); - if (!Array.isArray(parsed)) { - localStorage.removeItem(STORAGE_KEY); - return []; - } - return parsed.filter(isOverride); - } catch { - try { localStorage.removeItem(STORAGE_KEY); } catch { /* ignore */ } - return []; - } -} - function persistOverrides(overrides: AgentCustomizationOverride[]): void { - localStorage.setItem(STORAGE_KEY, JSON.stringify(overrides)); + writeJson(STORAGE_KEY, overrides); } export function useBuiltInAgentCustomizations() { - const [overrides, setOverrides] = useState(loadOverrides); + const [overrides, setOverrides] = useState(() => + readJson(STORAGE_KEY, [], Array.isArray).filter(isOverride), + ); const save = useCallback((override: AgentCustomizationOverride) => { setOverrides((prev) => { diff --git a/frontend/src/hooks/useChat.ts b/frontend/src/hooks/useChat.ts index 55434b3..077782c 100644 --- a/frontend/src/hooks/useChat.ts +++ b/frontend/src/hooks/useChat.ts @@ -3,26 +3,19 @@ import type { AgentProfile, ChatMessage, ChatSession, - ContentItem, McpConnectionResult, - SessionCreateResponse, StoredConversation, ToolInvocation, UsageDetails, } from '../types/api'; import { - createSession, - createCustomSession, - createSessionWithProfileOverride, - createSessionWithHistory, - deleteSession, - fetchHistory, sendMessage, AuthError, } from '../api/client'; import { emitToast } from './useToast'; -import { useConversationStore } from './useConversationStore'; -import { loadProfile, saveProfile } from './useUserProfile'; +import { saveProfile } from './useUserProfile'; +import { useConversationPersistence } from './useConversationPersistence'; +import { cleanupSession, emptyBuiltInOverride, emptyUsage, startChatSession } from './useSessionLifecycle'; interface ChatState { messages: ChatMessage[]; @@ -47,11 +40,7 @@ export function useChat(): ChatState { const [messages, setMessages] = useState([]); const [isStreaming, setIsStreaming] = useState(false); const [session, setSession] = useState(null); - const [sessionUsage, setSessionUsage] = useState({ - input_token_count: 0, - output_token_count: 0, - total_token_count: 0, - }); + const [sessionUsage, setSessionUsage] = useState(emptyUsage); const [error, setError] = useState(null); const [conversationId, setConversationId] = useState(null); const [saveCounter, setSaveCounter] = useState(0); @@ -71,129 +60,36 @@ export function useChat(): ChatState { usedBuiltInOverride: boolean; baseProfileId?: string; overrideUpdatedAt?: string; - }>({ usedBuiltInOverride: false }); - - const { saveConversation } = useConversationStore(); + }>(emptyBuiltInOverride()); - const saveCurrentConversation = useCallback(async () => { - if (!session) return; - const convId = conversationIdRef.current || session.session_id; - try { - const historyResp = await fetchHistory(session.session_id); - const firstUserMsg = messages.find((m) => m.role === 'user'); - const desc = firstUserMsg - ? firstUserMsg.content.slice(0, 60) + (firstUserMsg.content.length > 60 ? '...' : '') - : 'New conversation'; - const now = new Date().toISOString(); - saveConversation({ - id: convId, - profileId: session.profile_id, - profileName: session.profile_name, - description: desc, - createdAt: createdAtRef.current || now, - lastActivityAt: now, - sessionData: historyResp.session_data, - ...(customAgentIdRef.current ? { customAgentId: customAgentIdRef.current } : {}), - ...(builtInOverrideRef.current.usedBuiltInOverride ? { - usedBuiltInOverride: true, - baseProfileId: builtInOverrideRef.current.baseProfileId, - overrideUpdatedAt: builtInOverrideRef.current.overrideUpdatedAt, - } : {}), - }); - } catch { - // Non-fatal — persistence is best-effort - } - }, [session, messages, saveConversation]); + const { persistLatestConversation, saveCurrentConversation } = useConversationPersistence({ + session, + messages, + createdAtRef, + conversationIdRef, + customAgentIdRef, + builtInOverrideRef, + }); const startSession = useCallback(async (profile: AgentProfile, history?: StoredConversation) => { try { - // Clean up previous session if any - if (session) { - await deleteSession(session.session_id).catch(() => {}); - } - - let newSession: SessionCreateResponse; - let restoredMessages: ChatMessage[] = []; - const userProfile = loadProfile(); + const next = await startChatSession(profile, history, session); + createdAtRef.current = next.createdAt; + conversationIdRef.current = next.conversationId; + customAgentIdRef.current = next.customAgentId; + builtInOverrideRef.current = next.builtInOverride; - if (profile.customAgent) { - // Custom agent session (fresh or with history resume) - customAgentIdRef.current = profile.customAgent.id; - builtInOverrideRef.current = { usedBuiltInOverride: false }; - newSession = await createCustomSession({ - customAgentId: profile.customAgent.id, - custom_name: profile.customAgent.name, - custom_prompt: profile.customAgent.systemPrompt, - custom_tools: profile.customAgent.tools, - custom_search_context: profile.customAgent.useSearchContext, - custom_temperature: profile.customAgent.temperature, - custom_skills: profile.customAgent.skills, - mcp_servers: profile.customAgent.mcpServers, - ...(history?.sessionData ? { history: history.sessionData } : {}), - ...(userProfile ? { user_profile: { name: userProfile.name, preferences: userProfile.preferences, notes: userProfile.notes } } : {}), - }); - } else if (profile.builtInOverride) { - customAgentIdRef.current = null; - builtInOverrideRef.current = { - usedBuiltInOverride: true, - baseProfileId: profile.builtInOverride.baseProfileId, - overrideUpdatedAt: profile.builtInOverride.updatedAt, - }; - newSession = await createSessionWithProfileOverride( - profile.builtInOverride.baseProfileId, - profile.builtInOverride, - userProfile, - history?.sessionData, - ); - } else if (history?.sessionData) { - // Resume standard profile with history - customAgentIdRef.current = null; - builtInOverrideRef.current = history.usedBuiltInOverride - ? { - usedBuiltInOverride: true, - baseProfileId: history.baseProfileId ?? profile.id, - overrideUpdatedAt: history.overrideUpdatedAt, - } - : { usedBuiltInOverride: false }; - newSession = await createSessionWithHistory(profile.id, history.sessionData, userProfile); - } else { - // Fresh session - customAgentIdRef.current = null; - builtInOverrideRef.current = { usedBuiltInOverride: false }; - newSession = await createSession(profile.id, userProfile); - } - - if (history) { - createdAtRef.current = history.createdAt; - setConversationId(history.id); - conversationIdRef.current = history.id; - restoredMessages = extractMessagesFromSessionData(history.sessionData); - } else { - createdAtRef.current = new Date().toISOString(); - setConversationId(newSession.session_id); - conversationIdRef.current = newSession.session_id; - } - - if (newSession.used_profile_override) { - builtInOverrideRef.current = { - usedBuiltInOverride: true, - baseProfileId: profile.builtInOverride?.baseProfileId ?? profile.id, - overrideUpdatedAt: newSession.override_updated_at ?? profile.builtInOverride?.updatedAt, - }; - } - - const { mcp_results, tools_loaded, skills_loaded, search_context, ...sessionData } = newSession; - setSession(sessionData); - setMcpResults(mcp_results ?? []); - setToolsLoaded(tools_loaded ?? []); - setSkillsLoaded(skills_loaded ?? []); - setSearchContext(search_context ?? false); - setMessages(restoredMessages); - setSessionUsage({ input_token_count: 0, output_token_count: 0, total_token_count: 0 }); + setConversationId(next.conversationId); + setSession(next.session); + setMcpResults(next.mcpResults); + setToolsLoaded(next.toolsLoaded); + setSkillsLoaded(next.skillsLoaded); + setSearchContext(next.searchContext); + setMessages(next.restoredMessages); + setSessionUsage(emptyUsage()); setError(null); - // Emit toast for failed MCP servers - const failed = (mcp_results ?? []).filter((r) => r.status === 'failed'); + const failed = next.mcpResults.filter((r) => r.status === 'failed'); if (failed.length > 0) { const names = failed.map((r) => r.name).join(', '); emitToast({ @@ -210,9 +106,7 @@ export function useChat(): ChatState { }, [session]); const endSession = useCallback(async () => { - if (session) { - await deleteSession(session.session_id).catch(() => {}); - } + await cleanupSession(session); setSession(null); setMessages([]); setMcpResults([]); @@ -223,8 +117,8 @@ export function useChat(): ChatState { conversationIdRef.current = null; createdAtRef.current = null; customAgentIdRef.current = null; - builtInOverrideRef.current = { usedBuiltInOverride: false }; - setSessionUsage({ input_token_count: 0, output_token_count: 0, total_token_count: 0 }); + builtInOverrideRef.current = emptyBuiltInOverride(); + setSessionUsage(emptyUsage()); }, [session]); const send = useCallback(async (content: string, images?: File[]) => { @@ -379,37 +273,12 @@ export function useChat(): ChatState { } // Auto-save conversation after response completes if (session) { - const convId = conversationIdRef.current || session.session_id; - fetchHistory(session.session_id) - .then((historyResp) => { - // Build description from the first user message in the conversation - // We need to use the messages including the one we just sent - setMessages((currentMessages) => { - const firstUserMsg = currentMessages.find((m) => m.role === 'user'); - const desc = firstUserMsg - ? firstUserMsg.content.slice(0, 60) + (firstUserMsg.content.length > 60 ? '...' : '') - : 'New conversation'; - const now = new Date().toISOString(); - saveConversation({ - id: convId, - profileId: session.profile_id, - profileName: session.profile_name, - description: desc, - createdAt: createdAtRef.current || now, - lastActivityAt: now, - sessionData: historyResp.session_data, - ...(customAgentIdRef.current ? { customAgentId: customAgentIdRef.current } : {}), - ...(builtInOverrideRef.current.usedBuiltInOverride ? { - usedBuiltInOverride: true, - baseProfileId: builtInOverrideRef.current.baseProfileId, - overrideUpdatedAt: builtInOverrideRef.current.overrideUpdatedAt, - } : {}), - }); - return currentMessages; // Don't modify messages - }); - setSaveCounter((c) => c + 1); - }) - .catch(() => { /* persistence is best-effort */ }); + setMessages((currentMessages) => { + persistLatestConversation(currentMessages) + .then(() => setSaveCounter((c) => c + 1)) + .catch(() => { /* persistence is best-effort */ }); + return currentMessages; + }); } }, }, @@ -421,7 +290,7 @@ export function useChat(): ChatState { // Non-auth errors already emitted as toasts by client.ts setIsStreaming(false); } - }, [session, messages.length, saveConversation]); + }, [session, messages.length, persistLatestConversation]); const clearError = useCallback(() => setError(null), []); @@ -444,126 +313,3 @@ export function useChat(): ChatState { saveCurrentConversation, }; } - -/** - * Extract displayable messages from the framework's opaque session data. - * The session state has `in_memory` containing serialized Message objects. - */ -function extractMessagesFromSessionData(sessionData: Record): ChatMessage[] { - try { - const state = sessionData.state as Record | undefined; - if (!state) return []; - const inMemoryProvider = state.in_memory as Record | undefined; - if (!inMemoryProvider) return []; - const inMemory = inMemoryProvider.messages as Array> | undefined; - if (!Array.isArray(inMemory)) return []; - - const result: ChatMessage[] = []; - for (const msg of inMemory) { - const role = msg.role as string; - const contents = msg.contents as Array> | undefined; - if (!Array.isArray(contents)) continue; - - if (role === 'tool') { - // Attach tool results to the last assistant message's tool_invocations - const lastAssistant = result.length > 0 ? result[result.length - 1] : null; - if (lastAssistant?.role === 'assistant' && lastAssistant.tool_invocations) { - for (const content of contents) { - if (content.type === 'function_result' || content.type === 'mcp_server_tool_result') { - const callId = content.call_id as string; - const existing = lastAssistant.tool_invocations.find((t) => t.call_id === callId); - if (existing) { - const rawResult = content.type === 'mcp_server_tool_result' ? content.output : content.result; - if (Array.isArray(rawResult)) { - const textParts = rawResult - .filter((item: Record) => item.type === 'text') - .map((item: Record) => item.text as string || ''); - existing.result = textParts.length > 0 ? textParts.join('\n') : JSON.stringify(rawResult); - } else { - existing.result = typeof rawResult === 'string' - ? rawResult - : JSON.stringify(rawResult ?? ''); - } - // Extract structured content items (images) from the "items" list - // MCP servers return image content which the framework wraps as {type:'data', uri:'data:image/...;base64,...'} - const items = content.items as Array> | undefined; - if (Array.isArray(items)) { - const converted: ContentItem[] = []; - for (const item of items) { - if (item.type === 'text') { - converted.push({ type: 'text', text: (item.text as string) || '' }); - } else if (item.type === 'data') { - const uri = (item.uri as string) || ''; - if (uri.startsWith('data:image/')) { - const commaIdx = uri.indexOf(','); - const header = uri.slice(0, commaIdx); - const b64data = uri.slice(commaIdx + 1); - const mimeType = header.split(';')[0].replace('data:', ''); - if (b64data && mimeType) { - converted.push({ type: 'image', data: b64data, mimeType }); - } - } - } else if (item.type === 'image' && item.data && item.mimeType) { - converted.push(item as unknown as ContentItem); - } - } - if (converted.some((ci) => ci.type === 'image')) { - existing.content_items = converted; - } - } - } - } - } - } - continue; - } - - if (role !== 'user' && role !== 'assistant') continue; - - let text = ''; - const toolInvocations: ToolInvocation[] = []; - - const toolByCallId: Record = {}; - - for (const content of contents) { - const type = content.type as string; - if (type === 'text') { - text += content.text as string || ''; - } else if (type === 'function_call' || type === 'mcp_server_tool_call') { - const args = content.arguments; - const callId = (content.call_id as string) || ''; - const renderedArgs = typeof args === 'string' ? args : JSON.stringify(args ?? ''); - const existing = callId ? toolByCallId[callId] : undefined; - if (existing) { - // Continuation chunk for same call_id — accumulate arguments - if (renderedArgs) { - existing.arguments = (existing.arguments || '') + renderedArgs; - } - // Update name if previously empty - if (!existing.name) { - existing.name = (content.name as string) || (content.tool_name as string) || ''; - } - } else { - const inv: ToolInvocation = { - call_id: callId, - name: (content.name as string) || (content.tool_name as string) || '', - arguments: renderedArgs, - result: '', - }; - toolInvocations.push(inv); - if (callId) toolByCallId[callId] = inv; - } - } - } - - result.push({ - role: role as 'user' | 'assistant', - content: text, - tool_invocations: toolInvocations.length > 0 ? toolInvocations : undefined, - }); - } - return result; - } catch { - return []; - } -} diff --git a/frontend/src/hooks/useConversationPersistence.ts b/frontend/src/hooks/useConversationPersistence.ts new file mode 100644 index 0000000..5ea8971 --- /dev/null +++ b/frontend/src/hooks/useConversationPersistence.ts @@ -0,0 +1,72 @@ +import { useCallback } from 'react'; +import type { MutableRefObject } from 'react'; +import type { ChatMessage, ChatSession, StoredConversation } from '../types/api'; +import { fetchHistory } from '../api/client'; +import { useConversationStore } from './useConversationStore'; +import type { BuiltInOverrideState } from './useSessionLifecycle'; + +interface ConversationPersistenceOptions { + session: ChatSession | null; + messages: ChatMessage[]; + createdAtRef: MutableRefObject; + conversationIdRef: MutableRefObject; + customAgentIdRef: MutableRefObject; + builtInOverrideRef: MutableRefObject; +} + +function conversationDescription(messages: ChatMessage[]): string { + const firstUserMessage = messages.find((message) => message.role === 'user'); + return firstUserMessage + ? firstUserMessage.content.slice(0, 60) + (firstUserMessage.content.length > 60 ? '...' : '') + : 'New conversation'; +} + +export function useConversationPersistence({ + session, + messages, + createdAtRef, + conversationIdRef, + customAgentIdRef, + builtInOverrideRef, +}: ConversationPersistenceOptions) { + const { saveConversation } = useConversationStore(); + + const persistConversationSnapshot = useCallback((currentMessages: ChatMessage[], sessionData: Record) => { + if (!session) return; + const now = new Date().toISOString(); + const conversation: StoredConversation = { + id: conversationIdRef.current || session.session_id, + profileId: session.profile_id, + profileName: session.profile_name, + description: conversationDescription(currentMessages), + createdAt: createdAtRef.current || now, + lastActivityAt: now, + sessionData, + ...(customAgentIdRef.current ? { customAgentId: customAgentIdRef.current } : {}), + ...(builtInOverrideRef.current.usedBuiltInOverride ? { + usedBuiltInOverride: true, + baseProfileId: builtInOverrideRef.current.baseProfileId, + overrideUpdatedAt: builtInOverrideRef.current.overrideUpdatedAt, + } : {}), + }; + saveConversation(conversation); + }, [builtInOverrideRef, conversationIdRef, createdAtRef, customAgentIdRef, saveConversation, session]); + + const saveCurrentConversation = useCallback(async () => { + if (!session) return; + try { + const historyResp = await fetchHistory(session.session_id); + persistConversationSnapshot(messages, historyResp.session_data); + } catch { + // Non-fatal: persistence is best-effort. + } + }, [messages, persistConversationSnapshot, session]); + + const persistLatestConversation = useCallback(async (currentMessages: ChatMessage[]) => { + if (!session) return; + const historyResp = await fetchHistory(session.session_id); + persistConversationSnapshot(currentMessages, historyResp.session_data); + }, [persistConversationSnapshot, session]); + + return { persistConversationSnapshot, persistLatestConversation, saveCurrentConversation } as const; +} \ No newline at end of file diff --git a/frontend/src/hooks/useConversationStore.ts b/frontend/src/hooks/useConversationStore.ts index 6946759..a5a0f0f 100644 --- a/frontend/src/hooks/useConversationStore.ts +++ b/frontend/src/hooks/useConversationStore.ts @@ -1,5 +1,6 @@ import { useCallback } from 'react'; import type { ConversationIndexEntry, StoredConversation } from '../types/api'; +import { readJson, removeStorageItem, tryWriteJson } from '../utils/storage'; declare const __MAX_SESSIONS__: string; @@ -14,41 +15,29 @@ function conversationKey(id: string): string { return `${CONVERSATION_KEY_PREFIX}${id}`; } +function isConversationIndex(value: unknown): value is ConversationIndexEntry[] { + return Array.isArray(value) && value.every( + (entry) => entry && typeof entry.id === 'string' && typeof entry.description === 'string', + ); +} + export function useConversationStore() { const loadIndex = useCallback((): ConversationIndexEntry[] => { - try { - const raw = localStorage.getItem(INDEX_KEY); - if (!raw) return []; - const parsed = JSON.parse(raw); - if (!Array.isArray(parsed)) return []; - // Validate entries have required fields - const entries = parsed.filter( - (e: Record) => e && typeof e.id === 'string' && typeof e.description === 'string', - ) as ConversationIndexEntry[]; - // Trim to max if config changed between deployments - const max = getMaxSessions(); - if (entries.length > max) { - entries.sort((a, b) => new Date(b.lastActivityAt).getTime() - new Date(a.lastActivityAt).getTime()); - const evicted = entries.splice(max); - for (const e of evicted) { - try { localStorage.removeItem(conversationKey(e.id)); } catch { /* ignore */ } - } - try { localStorage.setItem(INDEX_KEY, JSON.stringify(entries)); } catch { /* ignore */ } + const entries = readJson(INDEX_KEY, [], isConversationIndex); + const max = getMaxSessions(); + if (entries.length > max) { + entries.sort((a, b) => new Date(b.lastActivityAt).getTime() - new Date(a.lastActivityAt).getTime()); + const evicted = entries.splice(max); + for (const entry of evicted) { + removeStorageItem(conversationKey(entry.id)); } - return entries; - } catch { - // Corrupted data — discard - try { localStorage.removeItem(INDEX_KEY); } catch { /* ignore */ } - return []; + tryWriteJson(INDEX_KEY, entries); } + return entries; }, []); const saveIndex = useCallback((index: ConversationIndexEntry[]) => { - try { - localStorage.setItem(INDEX_KEY, JSON.stringify(index)); - } catch (e) { - console.warn('Failed to save conversation index to localStorage:', e); - } + tryWriteJson(INDEX_KEY, index, (error) => console.warn('Failed to save conversation index to localStorage:', error)); }, []); const saveConversation = useCallback((conversation: StoredConversation) => { @@ -85,38 +74,30 @@ export function useConversationStore() { while (index.length > max) { const evicted = index.pop(); if (evicted) { - try { localStorage.removeItem(conversationKey(evicted.id)); } catch { /* ignore */ } + removeStorageItem(conversationKey(evicted.id)); } } saveIndex(index); // Save full conversation data - try { - localStorage.setItem(conversationKey(conversation.id), JSON.stringify(conversation)); - } catch (e) { - console.warn('Failed to save conversation to localStorage:', e); - } + tryWriteJson( + conversationKey(conversation.id), + conversation, + (error) => console.warn('Failed to save conversation to localStorage:', error), + ); }, [loadIndex, saveIndex]); const loadConversation = useCallback((id: string): StoredConversation | null => { - try { - const raw = localStorage.getItem(conversationKey(id)); - if (!raw) return null; - const parsed = JSON.parse(raw) as StoredConversation; - if (!parsed || typeof parsed.id !== 'string' || !parsed.sessionData) return null; - return parsed; - } catch { - // Corrupted — remove it - try { localStorage.removeItem(conversationKey(id)); } catch { /* ignore */ } - return null; - } + return readJson(conversationKey(id), null, (value): value is StoredConversation => ( + Boolean(value && typeof value === 'object' && typeof (value as StoredConversation).id === 'string' && (value as StoredConversation).sessionData) + )); }, []); const deleteConversation = useCallback((id: string) => { const index = loadIndex().filter((e) => e.id !== id); saveIndex(index); - try { localStorage.removeItem(conversationKey(id)); } catch { /* ignore */ } + removeStorageItem(conversationKey(id)); }, [loadIndex, saveIndex]); const deleteConversationsByCustomAgent = useCallback((customAgentId: string) => { @@ -124,7 +105,7 @@ export function useConversationStore() { const keep: ConversationIndexEntry[] = []; for (const e of index) { if (e.customAgentId === customAgentId) { - try { localStorage.removeItem(conversationKey(e.id)); } catch { /* ignore */ } + removeStorageItem(conversationKey(e.id)); } else { keep.push(e); } diff --git a/frontend/src/hooks/useCustomAgents.ts b/frontend/src/hooks/useCustomAgents.ts index e8a4433..41f04e7 100644 --- a/frontend/src/hooks/useCustomAgents.ts +++ b/frontend/src/hooks/useCustomAgents.ts @@ -1,23 +1,17 @@ import { useState, useCallback } from 'react'; import type { CustomAgentDefinition } from '../types/api'; +import { readJson, writeJson } from '../utils/storage'; const STORAGE_KEY = 'webagents_custom_agents'; -function loadAgents(): CustomAgentDefinition[] { - try { - const raw = localStorage.getItem(STORAGE_KEY); - return raw ? JSON.parse(raw) : []; - } catch { - return []; - } -} - function persistAgents(agents: CustomAgentDefinition[]): void { - localStorage.setItem(STORAGE_KEY, JSON.stringify(agents)); + writeJson(STORAGE_KEY, agents); } export function useCustomAgents() { - const [agents, setAgents] = useState(loadAgents); + const [agents, setAgents] = useState(() => + readJson(STORAGE_KEY, [], Array.isArray), + ); const save = useCallback((agent: CustomAgentDefinition) => { setAgents((prev) => { diff --git a/frontend/src/hooks/useSessionLifecycle.ts b/frontend/src/hooks/useSessionLifecycle.ts new file mode 100644 index 0000000..c6d7b33 --- /dev/null +++ b/frontend/src/hooks/useSessionLifecycle.ts @@ -0,0 +1,244 @@ +import type { + AgentProfile, + ChatMessage, + ChatSession, + McpConnectionResult, + SessionCreateResponse, + UserMemoryProfile, + StoredConversation, + ToolInvocation, + UsageDetails, +} from '../types/api'; +import { createSessionRequest, deleteSession, type SessionRequestPayload } from '../api/client'; +import { convertFrameworkContentItems } from '../utils/content'; +import { loadProfile } from './useUserProfile'; + +export interface BuiltInOverrideState { + usedBuiltInOverride: boolean; + baseProfileId?: string; + overrideUpdatedAt?: string; +} + +export interface SessionStartResult { + session: ChatSession; + mcpResults: McpConnectionResult[]; + toolsLoaded: string[]; + skillsLoaded: string[]; + searchContext: boolean; + restoredMessages: ChatMessage[]; + conversationId: string; + createdAt: string; + customAgentId: string | null; + builtInOverride: BuiltInOverrideState; +} + +export function emptyUsage(): UsageDetails { + return { + input_token_count: 0, + output_token_count: 0, + total_token_count: 0, + }; +} + +export function emptyBuiltInOverride(): BuiltInOverrideState { + return { usedBuiltInOverride: false }; +} + +export async function cleanupSession(session: ChatSession | null): Promise { + if (session) { + await deleteSession(session.session_id).catch(() => {}); + } +} + +export async function startChatSession( + profile: AgentProfile, + history: StoredConversation | undefined, + previousSession: ChatSession | null, +): Promise { + await cleanupSession(previousSession); + + const userProfile = loadProfile(); + const payload = buildSessionRequest(profile, history, userProfile); + const newSession: SessionCreateResponse = await createSessionRequest(payload); + const customAgentId = profile.customAgent?.id ?? null; + let builtInOverride: BuiltInOverrideState; + if (profile.builtInOverride) { + builtInOverride = { + usedBuiltInOverride: true, + baseProfileId: profile.builtInOverride.baseProfileId, + overrideUpdatedAt: profile.builtInOverride.updatedAt, + }; + } else if (history?.usedBuiltInOverride) { + builtInOverride = { + usedBuiltInOverride: true, + baseProfileId: history.baseProfileId ?? profile.id, + overrideUpdatedAt: history.overrideUpdatedAt, + }; + } else { + builtInOverride = emptyBuiltInOverride(); + } + + if (newSession.used_profile_override) { + builtInOverride = { + usedBuiltInOverride: true, + baseProfileId: profile.builtInOverride?.baseProfileId ?? profile.id, + overrideUpdatedAt: newSession.override_updated_at ?? profile.builtInOverride?.updatedAt, + }; + } + + const { mcp_results, tools_loaded, skills_loaded, search_context, ...session } = newSession; + return { + session, + mcpResults: mcp_results ?? [], + toolsLoaded: tools_loaded ?? [], + skillsLoaded: skills_loaded ?? [], + searchContext: search_context ?? false, + restoredMessages: history ? extractMessagesFromSessionData(history.sessionData) : [], + conversationId: history?.id ?? newSession.session_id, + createdAt: history?.createdAt ?? new Date().toISOString(), + customAgentId, + builtInOverride, + }; +} + +function buildSessionRequest( + profile: AgentProfile, + history: StoredConversation | undefined, + userProfile: UserMemoryProfile | null, +): SessionRequestPayload { + const userProfilePayload = userProfile + ? { user_profile: { name: userProfile.name, preferences: userProfile.preferences, notes: userProfile.notes } } + : {}; + + if (profile.customAgent) { + return { + profile_id: 'custom', + custom_name: profile.customAgent.name, + custom_prompt: profile.customAgent.systemPrompt, + custom_tools: profile.customAgent.tools, + custom_search_context: profile.customAgent.useSearchContext, + ...(profile.customAgent.temperature !== undefined ? { custom_temperature: profile.customAgent.temperature } : {}), + ...(profile.customAgent.skills.length > 0 ? { custom_skills: profile.customAgent.skills } : {}), + ...(profile.customAgent.mcpServers.length > 0 ? { mcp_servers: profile.customAgent.mcpServers } : {}), + ...(history?.sessionData ? { history: history.sessionData } : {}), + ...userProfilePayload, + }; + } + + if (profile.builtInOverride) { + return { + profile_id: profile.builtInOverride.baseProfileId, + profile_override: { + description: profile.builtInOverride.description, + custom_prompt: profile.builtInOverride.systemPrompt, + custom_tools: profile.builtInOverride.tools, + custom_search_context: profile.builtInOverride.useSearchContext, + ...(profile.builtInOverride.temperature !== undefined ? { custom_temperature: profile.builtInOverride.temperature } : {}), + ...(profile.builtInOverride.skills.length > 0 ? { custom_skills: profile.builtInOverride.skills } : {}), + ...(profile.builtInOverride.mcpServers.length > 0 ? { mcp_servers: profile.builtInOverride.mcpServers } : {}), + override_updated_at: profile.builtInOverride.updatedAt, + }, + ...(history?.sessionData ? { history: history.sessionData } : {}), + ...userProfilePayload, + }; + } + + return { + profile_id: profile.id, + ...(history?.sessionData ? { history: history.sessionData } : {}), + ...userProfilePayload, + }; +} + +export function extractMessagesFromSessionData(sessionData: Record): ChatMessage[] { + try { + const state = sessionData.state as Record | undefined; + const inMemoryProvider = state?.in_memory as Record | undefined; + const inMemory = inMemoryProvider?.messages as Array> | undefined; + if (!Array.isArray(inMemory)) return []; + + const result: ChatMessage[] = []; + for (const msg of inMemory) { + const role = msg.role as string; + const contents = msg.contents as Array> | undefined; + if (!Array.isArray(contents)) continue; + + if (role === 'tool') { + attachToolResults(result, contents); + continue; + } + + if (role !== 'user' && role !== 'assistant') continue; + result.push(toChatMessage(role, contents)); + } + return result; + } catch { + return []; + } +} + +function attachToolResults(messages: ChatMessage[], contents: Array>): void { + const lastAssistant = messages.length > 0 ? messages[messages.length - 1] : null; + if (lastAssistant?.role !== 'assistant' || !lastAssistant.tool_invocations) return; + + for (const content of contents) { + if (content.type !== 'function_result' && content.type !== 'mcp_server_tool_result') continue; + const callId = content.call_id as string; + const existing = lastAssistant.tool_invocations.find((tool) => tool.call_id === callId); + if (!existing) continue; + + const rawResult = content.type === 'mcp_server_tool_result' ? content.output : content.result; + existing.result = renderFrameworkToolResult(rawResult); + const converted = convertFrameworkContentItems(content.items as Array> | undefined); + if (converted.some((item) => item.type === 'image')) { + existing.content_items = converted; + } + } +} + +function renderFrameworkToolResult(rawResult: unknown): string { + if (Array.isArray(rawResult)) { + const textParts = rawResult + .filter((item: Record) => item.type === 'text') + .map((item: Record) => item.text as string || ''); + return textParts.length > 0 ? textParts.join('\n') : JSON.stringify(rawResult); + } + return typeof rawResult === 'string' ? rawResult : JSON.stringify(rawResult ?? ''); +} + +function toChatMessage(role: string, contents: Array>): ChatMessage { + let text = ''; + const toolInvocations: ToolInvocation[] = []; + const toolByCallId: Record = {}; + + for (const content of contents) { + const type = content.type as string; + if (type === 'text') { + text += content.text as string || ''; + } else if (type === 'function_call' || type === 'mcp_server_tool_call') { + const args = content.arguments; + const callId = (content.call_id as string) || ''; + const renderedArgs = typeof args === 'string' ? args : JSON.stringify(args ?? ''); + const existing = callId ? toolByCallId[callId] : undefined; + if (existing) { + if (renderedArgs) existing.arguments = (existing.arguments || '') + renderedArgs; + if (!existing.name) existing.name = (content.name as string) || (content.tool_name as string) || ''; + } else { + const invocation: ToolInvocation = { + call_id: callId, + name: (content.name as string) || (content.tool_name as string) || '', + arguments: renderedArgs, + result: '', + }; + toolInvocations.push(invocation); + if (callId) toolByCallId[callId] = invocation; + } + } + } + + return { + role: role as 'user' | 'assistant', + content: text, + tool_invocations: toolInvocations.length > 0 ? toolInvocations : undefined, + }; +} \ No newline at end of file diff --git a/frontend/src/hooks/useSkillForm.ts b/frontend/src/hooks/useSkillForm.ts new file mode 100644 index 0000000..a0724ba --- /dev/null +++ b/frontend/src/hooks/useSkillForm.ts @@ -0,0 +1,79 @@ +import { useState } from 'react'; +import type { SkillCreatePayload } from '../types/api'; + +export type SkillBuilderViewMode = 'list' | 'create' | 'edit'; + +export const SKILL_NAME_RE = /^[a-z0-9][a-z0-9-]*$/; + +export function useSkillForm() { + const [view, setView] = useState('list'); + const [formName, setFormName] = useState(''); + const [formDescription, setFormDescription] = useState(''); + const [formContent, setFormContent] = useState(''); + const [editingName, setEditingName] = useState(null); + const [formLoading, setFormLoading] = useState(false); + const [successMsg, setSuccessMsg] = useState(''); + const [errorMsg, setErrorMsg] = useState(''); + const [deletingName, setDeletingName] = useState(null); + const [aiLoading, setAiLoading] = useState(false); + + const clearFeedback = () => { + setSuccessMsg(''); + setErrorMsg(''); + }; + + const resetForm = () => { + setFormName(''); + setFormDescription(''); + setFormContent(''); + setEditingName(null); + }; + + const createPayload = (): SkillCreatePayload => ({ + name: formName, + description: formDescription, + content: formContent, + }); + + const isCreateFormValid = + SKILL_NAME_RE.test(formName) && + formName.length <= 64 && + formDescription.trim().length > 0 && + formDescription.length <= 256 && + formContent.trim().length > 0 && + formContent.length <= 65536; + + const isEditFormValid = + formDescription.trim().length > 0 && + formDescription.length <= 256 && + formContent.trim().length > 0 && + formContent.length <= 65536; + + return { + aiLoading, + clearFeedback, + createPayload, + deletingName, + editingName, + errorMsg, + formContent, + formDescription, + formLoading, + formName, + isCreateFormValid, + isEditFormValid, + resetForm, + setAiLoading, + setDeletingName, + setEditingName, + setErrorMsg, + setFormContent, + setFormDescription, + setFormLoading, + setFormName, + setSuccessMsg, + setView, + successMsg, + view, + }; +} \ No newline at end of file diff --git a/frontend/src/hooks/useUserProfile.ts b/frontend/src/hooks/useUserProfile.ts index b71c629..c2341e3 100644 --- a/frontend/src/hooks/useUserProfile.ts +++ b/frontend/src/hooks/useUserProfile.ts @@ -1,17 +1,12 @@ import type { UserMemoryProfile } from '../types/api'; +import { readJson, writeJson } from '../utils/storage'; const PROFILE_KEY = 'webagents_user_profile'; export function loadProfile(_profileId?: string): UserMemoryProfile | null { - try { - const raw = localStorage.getItem(PROFILE_KEY); - if (!raw) return null; - return JSON.parse(raw) as UserMemoryProfile; - } catch { - return null; - } + return readJson(PROFILE_KEY, null); } export function saveProfile(_profileId: string, profile: UserMemoryProfile): void { - localStorage.setItem(PROFILE_KEY, JSON.stringify(profile)); + writeJson(PROFILE_KEY, profile); } diff --git a/frontend/src/index.css b/frontend/src/index.css index e8ba0b4..a5de300 100644 --- a/frontend/src/index.css +++ b/frontend/src/index.css @@ -1,53 +1,8 @@ :root { - --text: #6b6375; - --text-h: #08060d; - --bg: #fff; - --border: #e5e4e7; - --code-bg: #f4f3ec; - --accent: #aa3bff; - --accent-bg: rgba(170, 59, 255, 0.1); - --accent-border: rgba(170, 59, 255, 0.5); - --social-bg: rgba(244, 243, 236, 0.5); - --shadow: - rgba(0, 0, 0, 0.1) 0 10px 15px -3px, rgba(0, 0, 0, 0.05) 0 4px 6px -2px; - - --sans: system-ui, 'Segoe UI', Roboto, sans-serif; - --heading: system-ui, 'Segoe UI', Roboto, sans-serif; - --mono: ui-monospace, Consolas, monospace; - - font: 18px/145% var(--sans); - letter-spacing: 0.18px; - color-scheme: light dark; - color: var(--text); - background: var(--bg); font-synthesis: none; text-rendering: optimizeLegibility; -webkit-font-smoothing: antialiased; -moz-osx-font-smoothing: grayscale; - - @media (max-width: 1024px) { - font-size: 16px; - } -} - -@media (prefers-color-scheme: dark) { - :root { - --text: #9ca3af; - --text-h: #f3f4f6; - --bg: #16171d; - --border: #2e303a; - --code-bg: #1f2028; - --accent: #c084fc; - --accent-bg: rgba(192, 132, 252, 0.15); - --accent-border: rgba(192, 132, 252, 0.5); - --social-bg: rgba(47, 48, 58, 0.5); - --shadow: - rgba(0, 0, 0, 0.4) 0 10px 15px -3px, rgba(0, 0, 0, 0.25) 0 4px 6px -2px; - } - - #social .button-icon { - filter: invert(1) brightness(2); - } } #root { @@ -61,47 +16,3 @@ body { margin: 0; } - -h1, -h2 { - font-family: var(--heading); - font-weight: 500; - color: var(--text-h); -} - -h1 { - font-size: 56px; - letter-spacing: -1.68px; - margin: 32px 0; - @media (max-width: 1024px) { - font-size: 36px; - margin: 20px 0; - } -} -h2 { - font-size: 24px; - line-height: 118%; - letter-spacing: -0.24px; - margin: 0 0 8px; - @media (max-width: 1024px) { - font-size: 20px; - } -} -p { - margin: 0; -} - -code, -.counter { - font-family: var(--mono); - display: inline-flex; - border-radius: 4px; - color: var(--text-h); -} - -code { - font-size: 15px; - line-height: 135%; - padding: 4px 8px; - background: var(--code-bg); -} diff --git a/frontend/src/pages/AgentBuilder.tsx b/frontend/src/pages/AgentBuilder.tsx index 823919f..866b4f2 100644 --- a/frontend/src/pages/AgentBuilder.tsx +++ b/frontend/src/pages/AgentBuilder.tsx @@ -2,16 +2,16 @@ import { useState, useEffect } from 'react'; import type { AgentCustomizationOverride, AgentProfile, - BuiltInAgentDefinition, ToolInfo, CustomAgentDefinition, - StarterQuestion, - McpServerEntry, - McpConnectionResult, StandardAgentCandidate, } from '../types/api'; -import { fetchBuiltInProfileDefinition, fetchProfiles, fetchSkills, fetchTools, testMcpConnections } from '../api/client'; +import { fetchBuiltInProfileDefinition, fetchProfiles, fetchSkills, fetchTools } from '../api/client'; import { generateStandardAgentCandidate } from '../utils/standardAgentCandidate'; +import { AgentCapabilityPicker } from '../components/AgentCapabilityPicker'; +import { StarterQuestionEditor } from '../components/StarterQuestionEditor'; +import { useAgentMcpEditor } from '../hooks/useAgentMcpEditor'; +import { prepareBuiltInOverride, prepareCustomAgent, useAgentBuilderForm } from '../hooks/useAgentBuilderForm'; interface AgentBuilderProps { agents: CustomAgentDefinition[]; @@ -23,23 +23,6 @@ interface AgentBuilderProps { onBack: () => void; } -function generateId(): string { - return `custom_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`; -} - -const EMPTY_FORM = { - name: '', - description: '', - systemPrompt: '', - tools: [] as string[], - skills: [] as string[], - mcpServers: [] as McpServerEntry[], - useSearchContext: false, - icon: '/icons/custom.svg', - starters: [] as StarterQuestion[], - temperature: '' as string, -}; - export function AgentBuilder({ agents, builtInOverrides, @@ -56,22 +39,44 @@ export function AgentBuilder({ const [loadingTools, setLoadingTools] = useState(true); const [loadingSkills, setLoadingSkills] = useState(true); const [loadingBuiltIns, setLoadingBuiltIns] = useState(true); - const [editingId, setEditingId] = useState(null); - const [editingBuiltInDefinition, setEditingBuiltInDefinition] = useState(null); - const [form, setForm] = useState(EMPTY_FORM); + const { + editingBuiltInDefinition, + editingId, + form, + isValid, + markTouched, + parsedTemperature, + resetForm: resetAgentForm, + saveSuccess, + setEditingBuiltInDefinition, + setEditingId, + setForm, + setSaveSuccess, + setTouched, + temperatureValid, + touched, + } = useAgentBuilderForm(); const [newStarterLabel, setNewStarterLabel] = useState(''); const [newStarterMessage, setNewStarterMessage] = useState(''); - const [newMcpName, setNewMcpName] = useState(''); - const [newMcpUrl, setNewMcpUrl] = useState(''); - const [newMcpAuth, setNewMcpAuth] = useState(false); - const [newMcpAuthScope, setNewMcpAuthScope] = useState(''); - const [mcpTestResults, setMcpTestResults] = useState>({}); - const [mcpTesting, setMcpTesting] = useState(false); - const [saveSuccess, setSaveSuccess] = useState(false); + const { + addMcpServer, + mcpTesting, + mcpTestResults, + newMcpAuth, + newMcpAuthScope, + newMcpName, + newMcpUrl, + removeMcpServer, + resetMcpEditor, + setNewMcpAuth, + setNewMcpAuthScope, + setNewMcpName, + setNewMcpUrl, + testConnections, + } = useAgentMcpEditor(setForm); const [candidate, setCandidate] = useState(null); const [builtInsCollapsed, setBuiltInsCollapsed] = useState(false); const [customAgentsCollapsed, setCustomAgentsCollapsed] = useState(false); - const [touched, setTouched] = useState>({}); useEffect(() => { fetchTools() @@ -92,17 +97,10 @@ export function AgentBuilder({ }, []); const resetForm = () => { - setForm(EMPTY_FORM); - setEditingId(null); - setEditingBuiltInDefinition(null); - setTouched({}); + resetAgentForm(); setNewStarterLabel(''); setNewStarterMessage(''); - setNewMcpName(''); - setNewMcpUrl(''); - setNewMcpAuth(false); - setNewMcpAuthScope(''); - setMcpTestResults({}); + resetMcpEditor(); }; const handleEdit = (agent: CustomAgentDefinition) => { @@ -110,12 +108,13 @@ export function AgentBuilder({ setEditingBuiltInDefinition(null); setTouched({}); const availableToolNames = new Set(availableTools.map((tool) => tool.name)); + const availableSkillNames = new Set(availableSkills.map((skill) => skill.name)); setForm({ name: agent.name, description: agent.description, systemPrompt: agent.systemPrompt, tools: agent.tools.filter((tool) => availableToolNames.has(tool)), - skills: [...(agent.skills || [])], + skills: (agent.skills || []).filter((skill) => availableSkillNames.has(skill)), mcpServers: [...(agent.mcpServers || [])], useSearchContext: searchContextAvailable ? agent.useSearchContext : false, icon: agent.icon, @@ -130,6 +129,7 @@ export function AgentBuilder({ const override = builtInOverrides.find((item) => item.baseProfileId === definition.id); const source = override ?? definition; const availableToolNames = new Set(availableTools.map((tool) => tool.name)); + const availableSkillNames = new Set(availableSkills.map((skill) => skill.name)); setEditingId(null); setEditingBuiltInDefinition(definition); setTouched({}); @@ -138,7 +138,7 @@ export function AgentBuilder({ description: source.description, systemPrompt: source.systemPrompt, tools: source.tools.filter((tool) => availableToolNames.has(tool)), - skills: [...source.skills], + skills: source.skills.filter((skill) => availableSkillNames.has(skill)), mcpServers: [...source.mcpServers], useSearchContext: searchContextAvailable ? source.useSearchContext : false, icon: source.icon, @@ -185,51 +185,11 @@ export function AgentBuilder({ })); }; - const handleTestMcpConnections = async () => { - if (form.mcpServers.length === 0) return; - setMcpTesting(true); - setMcpTestResults({}); - try { - const results = await testMcpConnections(form.mcpServers); - const map: Record = {}; - for (const r of results) { - map[r.name] = r; - } - setMcpTestResults(map); - } catch { - // Error already shown via toast by client.ts - } finally { - setMcpTesting(false); - } - }; - - const parsedTemp = form.temperature !== '' ? parseFloat(form.temperature) : undefined; - const tempValid = parsedTemp === undefined || (!isNaN(parsedTemp) && parsedTemp >= 0 && parsedTemp <= 2); - const handleSave = () => { - if ((!editingBuiltInDefinition && !form.name.trim()) || !form.systemPrompt.trim() || !tempValid) return; - - const now = new Date().toISOString(); + if (!isValid) return; if (editingBuiltInDefinition) { const existingOverride = builtInOverrides.find((item) => item.baseProfileId === editingBuiltInDefinition.id); - const override: AgentCustomizationOverride = { - id: existingOverride?.id ?? `builtin_override_${editingBuiltInDefinition.id}`, - baseProfileId: editingBuiltInDefinition.id, - baseProfileName: editingBuiltInDefinition.name, - description: form.description.trim(), - systemPrompt: form.systemPrompt, - tools: form.tools, - skills: form.skills, - mcpServers: form.mcpServers, - useSearchContext: form.useSearchContext, - icon: editingBuiltInDefinition.icon, - starters: form.starters, - ...(parsedTemp !== undefined ? { temperature: parsedTemp } : {}), - source: 'builtin-override', - createdAt: existingOverride?.createdAt ?? now, - updatedAt: now, - }; - + const override = prepareBuiltInOverride(form, editingBuiltInDefinition, existingOverride, parsedTemperature); onSaveBuiltInOverride(override); setSaveSuccess(true); setTimeout(() => setSaveSuccess(false), 2000); @@ -237,24 +197,7 @@ export function AgentBuilder({ return; } - const agent: CustomAgentDefinition = { - id: editingId || generateId(), - name: form.name.trim(), - description: form.description.trim(), - systemPrompt: form.systemPrompt, - tools: form.tools, - skills: form.skills, - mcpServers: form.mcpServers, - useSearchContext: form.useSearchContext, - icon: form.icon, - starters: form.starters, - ...(parsedTemp !== undefined ? { temperature: parsedTemp } : {}), - createdAt: editingId - ? agents.find((a) => a.id === editingId)?.createdAt || now - : now, - updatedAt: now, - }; - + const agent = prepareCustomAgent(form, editingId, agents, parsedTemperature); onSave(agent); setSaveSuccess(true); setTimeout(() => setSaveSuccess(false), 2000); @@ -286,8 +229,6 @@ export function AgentBuilder({ URL.revokeObjectURL(url); }; - const isValid = (editingBuiltInDefinition || form.name.trim()) && form.systemPrompt.trim() && tempValid; - return (
@@ -390,7 +331,7 @@ export function AgentBuilder({ type="text" value={form.name} onChange={(e) => setForm((prev) => ({ ...prev, name: e.target.value }))} - onBlur={() => setTouched((prev) => ({ ...prev, name: true }))} + onBlur={() => markTouched('name')} placeholder="e.g. Data Analyst" maxLength={100} readOnly={Boolean(editingBuiltInDefinition)} @@ -418,7 +359,7 @@ export function AgentBuilder({ className={`agent-builder-textarea${touched.systemPrompt && !form.systemPrompt.trim() ? ' agent-builder-input-error' : ''}`} value={form.systemPrompt} onChange={(e) => setForm((prev) => ({ ...prev, systemPrompt: e.target.value }))} - onBlur={() => setTouched((prev) => ({ ...prev, systemPrompt: true }))} + onBlur={() => markTouched('systemPrompt')} placeholder="You are a specialized agent that..." rows={8} /> @@ -433,7 +374,7 @@ export function AgentBuilder({ Controls the Agent's Creativity (0.0 = deterministic, 2.0 = creative). setForm((prev) => ({ ...prev, temperature: e.target.value }))} @@ -442,35 +383,20 @@ export function AgentBuilder({ max={2} step={0.1} /> - {!tempValid && ( + {!temperatureValid && ( Temperature must be between 0.0 and 2.0 )} - {/* Tools */} -
- TOOLS - - Backend capabilities the agent can invoke during a conversation - - {loadingTools ? ( -
Loading available tools...
- ) : ( -
- {availableTools.map((tool) => ( - - ))} -
- )} -
+ name.replace(/_/g, ' ')} + onToggle={handleToolToggle} + /> {/* AI Search context provider — hidden when not configured */} {searchContextAvailable && ( @@ -485,32 +411,16 @@ export function AgentBuilder({ )} - {/* Skills */} -
- SKILLS - - Domain-specific knowledge packages that give the agent specialized expertise - - {loadingSkills ? ( -
Loading available skills...
- ) : availableSkills.length > 0 ? ( -
- {availableSkills.map((skill) => ( - - ))} -
- ) : ( -
No skills available
- )} -
+ name.replace(/-/g, ' ')} + onToggle={handleSkillToggle} + /> {/* MCP Servers */}
@@ -538,17 +448,7 @@ export function AgentBuilder({ {server.authScope ? ` [OBO: ${server.authScope}]` : server.authenticated ? ' [Auth]' : ''}
- {/* Starter questions */} -
- STARTER QUESTIONS - {form.starters.length > 0 && ( -
- {form.starters.map((s, i) => ( -
- {s.label} - -
- ))} -
- )} -
- setNewStarterLabel(e.target.value)} - placeholder="Button label" - maxLength={80} - /> - setNewStarterMessage(e.target.value)} - placeholder="Message to send" - maxLength={500} - /> - -
-
+ {/* Actions */}
diff --git a/frontend/src/styles/index.css b/frontend/src/styles/index.css index 138c4f8..133a150 100644 --- a/frontend/src/styles/index.css +++ b/frontend/src/styles/index.css @@ -1,6 +1,4 @@ -/* Web-Agents Theme */ -/* T002: Design Tokens — Dark theme (default/fallback) */ :root { --bg-primary: #0a0e0a; --bg-panel: #1a1e1a; @@ -21,7 +19,6 @@ --radius: 2px; } -/* Light theme overrides */ [data-theme="light"] { --bg-primary: #f0efe8; --bg-panel: #e8e7e0; @@ -39,7 +36,6 @@ --alert-red: #b30000; } -/* T003: Global reset + base */ * { margin: 0; padding: 0; @@ -62,7 +58,6 @@ body { flex-direction: column; } -/* T004: Scrollbar styling */ * { scrollbar-color: #2a2e2a var(--bg-primary); scrollbar-width: thin; @@ -85,7 +80,6 @@ body { background: var(--accent-olive); } -/* T005: Header */ .chat-page { display: flex; flex-direction: column; @@ -205,7 +199,6 @@ body { justify-content: center; } -/* T006: Messages */ .chat-messages { flex: 1; overflow-y: auto; @@ -385,6 +378,14 @@ body { max-width: 240px; } +.message-image-thumb img, +.tool-result-image, +.message-tool-image { + object-fit: contain; + border: 1px solid var(--border-olive); + border-radius: var(--radius); +} + .message-image-thumb img { display: block; width: 100%; @@ -393,7 +394,6 @@ body { object-fit: cover; } -/* T007: Chat input */ .chat-input-form { padding: 12px 32px; background-color: var(--bg-header); @@ -467,7 +467,6 @@ body { margin-top: 4px; } -/* T008: Profile cards */ .profile-selector { display: flex; flex-direction: column; @@ -559,7 +558,6 @@ body { background: color-mix(in srgb, var(--text-gold) 12%, var(--bg-input)); } -/* T009: Starter questions */ .starter-questions { display: flex; flex-direction: column; @@ -604,7 +602,6 @@ body { box-shadow: 0 0 8px rgba(200, 160, 0, 0.15); } -/* T010: Tool steps */ .tool-step { margin: 8px 0; border: 1px solid var(--border-olive); @@ -629,17 +626,6 @@ body { background-color: var(--bg-hover); } -.tool-step-body { - padding: 12px; - font-size: 0.8rem; - font-family: var(--font-mono); - background-color: var(--bg-primary); - border-top: 1px solid var(--border-olive); - white-space: pre-wrap; - overflow-x: auto; - color: var(--text-primary); -} - .tool-step-details { padding: 8px 12px; border-top: 1px solid var(--border-olive); @@ -691,44 +677,32 @@ body { color: var(--accent-gold); } -/* Tool result images */ -.tool-result-images { +.tool-result-images, +.message-tool-images { display: flex; flex-wrap: wrap; gap: 8px; margin-top: 8px; } -.tool-result-image { +.tool-result-image, +.message-tool-image { max-width: 100%; - max-height: 400px; - border-radius: var(--radius); - border: 1px solid var(--border-olive); - cursor: pointer; - object-fit: contain; } -.tool-result-image:hover { - border-color: var(--accent-gold); -} - -/* Inline images in chat messages (from tool results) */ -.message-tool-images { - display: flex; - flex-wrap: wrap; - gap: 8px; - margin-top: 8px; +.tool-result-image { + max-height: 400px; + cursor: pointer; } .message-tool-image { - max-width: 100%; max-height: 500px; - border-radius: var(--radius); - border: 1px solid var(--border-olive); - object-fit: contain; } -/* T011: Disclaimer */ +.tool-result-image:hover { + border-color: var(--accent-gold); +} + .disclaimer-overlay { position: fixed; top: 0; @@ -834,7 +808,6 @@ body { box-shadow: 0 0 15px rgba(200, 160, 0, 0.3); } -/* T012: Login */ .app-loading, .app-login { display: flex; @@ -924,7 +897,6 @@ body { 50% { opacity: 1; } } -/* T013: Classification footer */ .classification-footer { font-family: var(--font-mono); font-size: 0.65rem; @@ -937,7 +909,6 @@ body { border-top: 1px solid var(--border-olive); } -/* T014: Token usage */ .token-usage { font-family: var(--font-mono); font-size: 0.7rem; @@ -947,20 +918,10 @@ body { letter-spacing: 1px; } -.token-usage-label { - font-weight: 400; - margin-right: 2px; -} - -.token-usage-value { - margin-right: 4px; -} - .token-usage-detail { color: var(--text-muted); } -/* T015: Image upload */ .image-upload { display: flex; gap: 8px; @@ -1098,7 +1059,6 @@ body { justify-content: center; } -/* T016: Chat error */ .chat-error { display: flex; align-items: center; @@ -1130,7 +1090,6 @@ body { font-size: 1rem; } -/* Auth error panel shown on profile selection screen */ .auth-error-panel { display: flex; flex-direction: column; @@ -1156,31 +1115,6 @@ body { max-width: 400px; } -/* Error message */ -.error-message { - padding: 12px 16px; - margin: 8px 0; - background-color: rgba(139, 0, 0, 0.15); - border: 1px solid var(--alert-red); - border-radius: var(--radius); - color: var(--text-primary); - font-size: 0.85rem; -} - -/* Loading animation */ -.loading-dots::after { - content: '...'; - animation: dots 1.5s steps(4, end) infinite; -} - -@keyframes dots { - 0%, 20% { content: ''; } - 40% { content: '.'; } - 60% { content: '..'; } - 80%, 100% { content: '...'; } -} - -/* T017: Responsive */ @media (max-width: 768px) { .chat-header { flex-direction: column; @@ -1230,14 +1164,10 @@ body { } } -/* Tool steps container */ .tool-steps { margin-bottom: 8px; } -/* ======================================== - Sidebar — Conversation History - ======================================== */ .sidebar { width: 260px; @@ -1386,7 +1316,6 @@ body { color: var(--alert-red); } -/* Layout: sidebar + main content */ .app-layout { display: flex; height: 100vh; @@ -1398,7 +1327,6 @@ body { min-width: 0; } -/* Mobile sidebar */ @media (max-width: 768px) { .sidebar { position: fixed; @@ -1453,9 +1381,6 @@ body { background-color: var(--bg-hover); } -/* ======================================== - Agent Builder - ======================================== */ .agent-builder { display: flex; @@ -1583,7 +1508,6 @@ body { outline-offset: 4px; } -/* Saved agents list */ .agent-builder-saved { border: 1px solid var(--border-olive); border-radius: var(--radius); @@ -1652,7 +1576,9 @@ body { flex-shrink: 0; } -.agent-builder-saved-actions button { +.agent-builder-saved-actions button, +.skill-list-new, +.skill-btn-ai { background: none; border: 1px solid var(--border-olive); color: var(--text-muted); @@ -1664,12 +1590,13 @@ body { letter-spacing: 0; } -.agent-builder-saved-actions button:hover { +.agent-builder-saved-actions button:hover, +.skill-list-new:hover:not(:disabled), +.skill-btn-ai:hover:not(:disabled) { border-color: var(--accent-gold); color: var(--text-gold); } -/* Form */ .agent-builder-form { border: 1px solid var(--border-olive); border-radius: var(--radius); @@ -1690,7 +1617,8 @@ body { letter-spacing: 1px; } -.agent-builder-input { +.agent-builder-input, +.agent-builder-textarea { padding: 8px 12px; border: 1px solid var(--border-olive); border-radius: var(--radius); @@ -1700,12 +1628,14 @@ body { font-size: 0.95rem; } -.agent-builder-input:focus { +.agent-builder-input:focus, +.agent-builder-textarea:focus { outline: none; border-color: var(--accent-gold); } -.agent-builder-input::placeholder { +.agent-builder-input::placeholder, +.agent-builder-textarea::placeholder { color: var(--text-muted); } @@ -1721,26 +1651,10 @@ body { } .agent-builder-textarea { - padding: 10px 12px; - border: 1px solid var(--border-olive); - border-radius: var(--radius); - background-color: var(--bg-input); - color: var(--text-primary); - font-family: var(--font-mono); - font-size: 0.95rem; resize: vertical; min-height: 120px; } -.agent-builder-textarea:focus { - outline: none; - border-color: var(--accent-gold); -} - -.agent-builder-textarea::placeholder { - color: var(--text-muted); -} - .agent-builder-loading { font-family: var(--font-mono); font-size: 0.85rem; @@ -1748,7 +1662,6 @@ body { padding: 8px 0; } -/* Tool checkboxes */ .agent-builder-tools { display: flex; flex-direction: column; @@ -1797,7 +1710,6 @@ body { border-color: var(--accent-olive); } -/* Starters */ .agent-builder-starters-list { display: flex; flex-direction: column; @@ -1865,7 +1777,6 @@ body { border-color: var(--accent-gold); } -/* MCP server add form — single row, wraps on small screens */ .agent-builder-mcp-add { display: flex; flex-wrap: wrap; @@ -1878,7 +1789,6 @@ body { min-width: 180px; } -/* Actions */ .agent-builder-actions { display: flex; gap: 12px; @@ -1937,7 +1847,6 @@ body { text-align: center; } -/* Responsive */ @media (max-width: 768px) { .agent-builder-layout { padding: 16px; @@ -1950,9 +1859,6 @@ body { } } -/* ======================================== - Profile Card — Custom Badge - ======================================== */ .profile-card-badge { font-family: var(--font-mono); @@ -1966,9 +1872,6 @@ body { display: inline-block; } -/* ======================================== - Sidebar — Advanced Section - ======================================== */ .sidebar-advanced { border-top: 1px solid var(--border-olive); @@ -2026,9 +1929,6 @@ body { background-color: rgba(139, 0, 0, 0.2); } -/* ======================================== - Sidebar — Theme Selector - ======================================== */ .sidebar-theme-selector { border-top: 1px solid var(--border-olive); @@ -2078,9 +1978,6 @@ body { color: #f0efe8; } -/* ======================================== - Light Mode — Semi-Transparent Overrides - ======================================== */ [data-theme="light"] .user-message { background-color: var(--bg-header); @@ -2126,7 +2023,6 @@ body { background-color: rgba(138, 109, 0, 0.1); } -/* Light mode scrollbar */ [data-theme="light"] * { scrollbar-color: #c0c0b0 var(--bg-primary); } @@ -2143,9 +2039,6 @@ body { background: var(--accent-olive); } -/* ------------------------------------------------------------------ */ -/* Toast notifications */ -/* ------------------------------------------------------------------ */ .toast-container { position: fixed; @@ -2234,7 +2127,6 @@ body { } } -/* Light theme overrides */ [data-theme="light"] .toast { background: rgba(255, 255, 255, 0.97); color: #333; @@ -2249,7 +2141,6 @@ body { color: #333; } -/* Agent Capabilities Bar */ .capabilities-bar { display: flex; flex-wrap: wrap; @@ -2342,7 +2233,6 @@ body { color: var(--alert-red); } -/* MCP Status Indicator (legacy — used by McpStatusIndicator component) */ .mcp-status-indicator { display: flex; flex-wrap: wrap; @@ -2378,20 +2268,6 @@ body { color: var(--text-muted); } -.mcp-status-bar { - padding: 4px 16px; - border-bottom: 1px solid var(--border-olive); - background: var(--bg-header); -} - -/* Agent Builder MCP badges */ -.agent-builder-saved-mcp { - display: flex; - flex-wrap: wrap; - gap: 4px; - margin-top: 4px; -} - .agent-builder-mcp-badge { display: inline-flex; align-items: center; @@ -2405,7 +2281,6 @@ body { background: var(--bg-input); } -/* MCP test connection button & inline status */ .agent-builder-test-mcp-btn { margin-top: 6px; padding: 4px 12px; @@ -2507,9 +2382,6 @@ body { flex-shrink: 0; } -/* =========================================================================== - Admin Page - =========================================================================== */ .admin-page { display: flex; @@ -2598,9 +2470,6 @@ body { min-width: 0; } -/* =========================================================================== - Skill Builder - =========================================================================== */ .skill-builder { height: 100%; @@ -2673,25 +2542,6 @@ body { margin-bottom: 16px; } -.skill-list-new, -.skill-btn-ai { - background: transparent; - color: var(--text-muted); - border: 1px solid var(--border-olive); - padding: 6px 10px; - font-family: var(--font-mono); - font-size: 0.8rem; - letter-spacing: 0; - border-radius: var(--radius); - cursor: pointer; -} - -.skill-list-new:hover:not(:disabled), -.skill-btn-ai:hover:not(:disabled) { - border-color: var(--accent-gold); - color: var(--text-gold); -} - .skill-list-new:disabled, .skill-btn-ai:disabled { opacity: 0.4; diff --git a/frontend/src/utils/content.ts b/frontend/src/utils/content.ts new file mode 100644 index 0000000..f36e5b4 --- /dev/null +++ b/frontend/src/utils/content.ts @@ -0,0 +1,71 @@ +import type { ContentItem, ToolInvocation } from '../types/api'; + +export const ALLOWED_IMAGE_MIMES = new Set(['image/jpeg', 'image/png', 'image/gif', 'image/webp']); + +export type ImageContentItem = Extract; + +export function isSupportedImageItem(item: ContentItem): item is ImageContentItem { + return item.type === 'image' && ALLOWED_IMAGE_MIMES.has(item.mimeType); +} + +export function imageDataUri(item: ImageContentItem): string { + return `data:${item.mimeType};base64,${item.data}`; +} + +export function toolImages(invocation: ToolInvocation): ImageContentItem[] { + return (invocation.content_items ?? []).filter(isSupportedImageItem); +} + +export function hasToolImages(invocations: ToolInvocation[] | undefined): boolean { + return Boolean(invocations?.some((invocation) => toolImages(invocation).length > 0)); +} + +export function formatToolResult(raw: string): string { + try { + return JSON.stringify(JSON.parse(raw), null, 2); + } catch { + try { + const jsonified = raw + .replace(/datetime\.datetime\([^)]+\)/g, (match) => { + const numbers = match.match(/\d+/g); + if (numbers && numbers.length >= 3) { + const [year, month, day, hour = '0', minute = '0', second = '0'] = numbers; + return `"${year}-${month.padStart(2, '0')}-${day.padStart(2, '0')}T${hour.padStart(2, '0')}:${minute.padStart(2, '0')}:${second.padStart(2, '0')}"`; + } + return `"${match}"`; + }) + .replace(/'/g, '"') + .replace(/\bTrue\b/g, 'true') + .replace(/\bFalse\b/g, 'false') + .replace(/\bNone\b/g, 'null'); + return JSON.stringify(JSON.parse(jsonified), null, 2); + } catch { + return raw; + } + } +} + +export function convertFrameworkContentItems(items: Array> | undefined): ContentItem[] { + if (!Array.isArray(items)) return []; + + const converted: ContentItem[] = []; + for (const item of items) { + if (item.type === 'text') { + converted.push({ type: 'text', text: (item.text as string) || '' }); + } else if (item.type === 'data') { + const uri = (item.uri as string) || ''; + if (uri.startsWith('data:image/')) { + const commaIndex = uri.indexOf(','); + const header = uri.slice(0, commaIndex); + const data = uri.slice(commaIndex + 1); + const mimeType = header.split(';')[0].replace('data:', ''); + if (data && mimeType) { + converted.push({ type: 'image', data, mimeType }); + } + } + } else if (item.type === 'image' && item.data && item.mimeType) { + converted.push(item as unknown as ContentItem); + } + } + return converted; +} \ No newline at end of file diff --git a/frontend/src/utils/storage.ts b/frontend/src/utils/storage.ts new file mode 100644 index 0000000..0209587 --- /dev/null +++ b/frontend/src/utils/storage.ts @@ -0,0 +1,31 @@ +export function readJson(key: string, fallback: T, validate?: (value: unknown) => value is T): T { + try { + const raw = localStorage.getItem(key); + if (!raw) return fallback; + const parsed = JSON.parse(raw) as unknown; + if (validate && !validate(parsed)) { + localStorage.removeItem(key); + return fallback; + } + return parsed as T; + } catch { + try { localStorage.removeItem(key); } catch { /* ignore */ } + return fallback; + } +} + +export function writeJson(key: string, value: T): void { + localStorage.setItem(key, JSON.stringify(value)); +} + +export function tryWriteJson(key: string, value: T, onError?: (error: unknown) => void): void { + try { + writeJson(key, value); + } catch (error) { + onError?.(error); + } +} + +export function removeStorageItem(key: string): void { + try { localStorage.removeItem(key); } catch { /* ignore */ } +} \ No newline at end of file diff --git a/main.py b/main.py index f6d41e9..943c775 100644 --- a/main.py +++ b/main.py @@ -5,12 +5,8 @@ React SPA static files from frontend/dist/. """ -import json import logging import os -import re -import shutil -import uuid from contextlib import asynccontextmanager from datetime import datetime, timezone from pathlib import Path @@ -28,12 +24,31 @@ from agent_framework import AgentSession from agent_framework._types import Content, Message as ChatMessage, UsageDetails -from agent_factory import create_chat_runtime from auth import AuthenticatedUser, get_current_user from eval_trace import EvalTraceLogger from mcp_servers import parse_mcp_server_configs, connect_mcp_servers, cleanup_mcp_servers, get_search_service_config -from prompt_config import get_profile_display_name, load_agents_yaml, resolve_logical_profile +from prompt_config import get_profile_display_name, load_agents_yaml +from session_orchestration import SessionContext, create_chat_session +from skills_manager import SkillManager +from streaming import ( + USAGE_INPUT_KEY, + USAGE_OUTPUT_KEY, + USAGE_TOTAL_KEY, + create_usage, + is_context_length_error, + is_retryable_error, + merge_usage, + sse_event, + stream_agent_response, + usage_value, +) from tools import UserProfileStore +from validators import ( + ALLOWED_IMAGE_MIMES, + MAX_IMAGE_SIZE_BYTES, + MAX_IMAGES_PER_MESSAGE, + validate_uploaded_images, +) load_dotenv() @@ -45,22 +60,6 @@ # --------------------------------------------------------------------------- DEFAULT_MAX_USER_INPUT_CHARS = int(os.getenv("MAX_USER_INPUT_CHARS", "8000")) -USAGE_INPUT_KEY = "input_token_count" -USAGE_OUTPUT_KEY = "output_token_count" -USAGE_TOTAL_KEY = "total_token_count" - -ALLOWED_IMAGE_MIMES = {"image/jpeg", "image/png", "image/gif", "image/webp"} - -_MAGIC_BYTES: dict[str, list[bytes]] = { - "image/jpeg": [b"\xff\xd8\xff"], - "image/png": [b"\x89PNG\r\n\x1a\n"], - "image/gif": [b"GIF87a", b"GIF89a"], - "image/webp": [], # handled specially: RIFF....WEBP -} - -MAX_IMAGE_SIZE_BYTES = 400 * 1024 * 1024 # 400 MB -MAX_IMAGES_PER_MESSAGE = 5 - # Default profile icon (used when no icon specified in agents.yaml) _DEFAULT_PROFILE_ICON = "/favicon.png" @@ -100,7 +99,7 @@ def __init__( self.agent = agent self.agent_session = agent_session self.tools = tools - self.usage: Optional[UsageDetails] = _create_usage() + self.usage: Optional[UsageDetails] = create_usage() self.eval_trace_logger = eval_trace_logger self.prompt_manifest = prompt_manifest self.prompt_logical_profile = prompt_logical_profile @@ -184,347 +183,17 @@ def _build_user_profile_context(user_profile_data: dict[str, str] | None) -> str return "\n\n## Known User Profile\n" + "\n".join(parts) -# --------------------------------------------------------------------------- -# Usage helpers (ported from original main.py) -# --------------------------------------------------------------------------- - -def _create_usage( - input_token_count: Optional[int] = None, - output_token_count: Optional[int] = None, - total_token_count: Optional[int] = None, -) -> UsageDetails: - return UsageDetails( - input_token_count=input_token_count, - output_token_count=output_token_count, - total_token_count=total_token_count, - ) - - -def _usage_value(usage: Optional[UsageDetails], key: str) -> int: - if not usage: - return 0 - if isinstance(usage, dict): - return int(usage.get(key) or 0) - return int(getattr(usage, key, 0) or 0) - - -def _merge_usage( - current: Optional[UsageDetails], - incoming: Optional[UsageDetails], -) -> Optional[UsageDetails]: - if not incoming: - return current - if not current: - return incoming - return _create_usage( - input_token_count=_usage_value(current, USAGE_INPUT_KEY) + _usage_value(incoming, USAGE_INPUT_KEY), - output_token_count=_usage_value(current, USAGE_OUTPUT_KEY) + _usage_value(incoming, USAGE_OUTPUT_KEY), - total_token_count=_usage_value(current, USAGE_TOTAL_KEY) + _usage_value(incoming, USAGE_TOTAL_KEY), - ) - - -def _extract_usage_from_payload(payload: dict) -> Optional[UsageDetails]: - usage_data = payload.get("usage") or {} - if not usage_data: - return None - return _create_usage( - input_token_count=usage_data.get(USAGE_INPUT_KEY), - output_token_count=usage_data.get(USAGE_OUTPUT_KEY), - total_token_count=usage_data.get(USAGE_TOTAL_KEY), - ) - - -# --------------------------------------------------------------------------- -# Image validation helpers (ported from original main.py) -# --------------------------------------------------------------------------- - -def _validate_image_magic_bytes(data: bytes, claimed_mime: str) -> bool: - if not data: - return False - if claimed_mime == "image/webp": - return len(data) >= 12 and data[:4] == b"RIFF" and data[8:12] == b"WEBP" - signatures = _MAGIC_BYTES.get(claimed_mime, []) - return any(data[: len(sig)] == sig for sig in signatures) - - -def _validate_uploaded_images( - files: list[UploadFile], file_data: list[bytes] -) -> Optional[str]: - """Validate uploaded image files. Returns error message or None.""" - if len(files) > MAX_IMAGES_PER_MESSAGE: - return f"Maximum of {MAX_IMAGES_PER_MESSAGE} images per message. Please reduce the number of images." - - for f, data in zip(files, file_data): - mime = f.content_type or "" - name = f.filename or "uploaded file" - - if mime not in ALLOWED_IMAGE_MIMES: - return ( - f"Only image files are accepted (JPEG, PNG, GIF, WebP). " - f"'{name}' is not a supported image type." - ) - - if len(data) > MAX_IMAGE_SIZE_BYTES: - return f"'{name}' exceeds the maximum file size of 400 MB." - - if not _validate_image_magic_bytes(data, mime): - return f"'{name}' could not be processed. The file may be corrupt or unreadable." - - return None - - -# --------------------------------------------------------------------------- -# Error classification helpers (ported from original main.py) -# --------------------------------------------------------------------------- - -def _is_retryable_error(e: Exception) -> bool: - error_message = str(e) - error_type = str(type(e)) - error_lower = error_message.lower() - return ( - "429" in error_message - or "Too Many Requests" in error_message - or "RateLimitError" in error_type - or "rate_limit" in error_lower - or "rate limit" in error_lower - or "capacity" in error_lower - ) - - -def _is_context_length_error(e: Exception) -> bool: - error_text = str(e).lower() - return any( - phrase in error_text - for phrase in [ - "context length", - "maximum context length", - "token limit", - "too many tokens", - "prompt is too long", - "maximum prompt", - ] - ) - - -# --------------------------------------------------------------------------- -# SSE streaming helper (T011 - adapted from original _run_agent_stream) -# --------------------------------------------------------------------------- - -def _sse_event(event: str, data: dict) -> str: - """Format a single SSE event string.""" - return f"event: {event}\ndata: {json.dumps(data, ensure_ascii=False)}\n\n" - - -async def _stream_agent_response( - agent: RuntimeAgent, - contents: list[Content], - session: AgentSession, -) -> AsyncGenerator[str, None]: - """Run the agent and yield SSE-formatted events. - - Event types: text, function_call, function_result, usage, error, done - """ - request_usage: Optional[UsageDetails] = None - final_text_parts: list[str] = [] - tool_events: list[dict[str, Any]] = [] - tool_event_by_call_id: dict[str, dict[str, Any]] = {} - # Track the active call_id for accumulating streamed argument chunks - active_call_id: Optional[str] = None - args_accumulator: dict[str, str] = {} - - user_message = ChatMessage(role="user", contents=contents) - stream = agent.run(user_message, session=session, stream=True) - - async for msg in stream: - msg_dict = msg.to_dict() - - update_usage = _extract_usage_from_payload(msg_dict) - request_usage = _merge_usage(request_usage, update_usage) - - for content in msg_dict.get("contents", []) or []: - content_type = content.get("type") - - if content_type == "function_call": - call_id = content.get("call_id") or None - name = content.get("name") or None - arguments = content.get("arguments", "") - rendered_arguments = ( - json.dumps(arguments, ensure_ascii=False) - if isinstance(arguments, (dict, list)) - else str(arguments) - ) - - if name and call_id and call_id not in tool_event_by_call_id: - # First chunk of a new tool call — call_id not seen before - active_call_id = call_id - args_accumulator[call_id] = rendered_arguments - - event_payload = { - "call_id": call_id, - "name": name, - "arguments": rendered_arguments, - "result": None, - } - tool_events.append(event_payload) - tool_event_by_call_id[call_id] = event_payload - - yield _sse_event("function_call", { - "call_id": call_id, - "name": name, - "arguments": rendered_arguments, - }) - elif call_id and call_id in tool_event_by_call_id: - # Continuation chunk for an existing call (Responses API sends - # name+call_id on every delta, not just the first) - active_call_id = call_id - if rendered_arguments: - args_accumulator[call_id] = args_accumulator.get(call_id, "") + rendered_arguments - tool_event_by_call_id[call_id]["arguments"] = args_accumulator[call_id] - yield _sse_event("function_call", { - "call_id": call_id, - "name": tool_event_by_call_id[call_id].get("name"), - "arguments": args_accumulator[call_id], - }) - elif active_call_id: - # Continuation chunk — append arguments to the active call - if rendered_arguments: - args_accumulator[active_call_id] = args_accumulator.get(active_call_id, "") + rendered_arguments - if active_call_id in tool_event_by_call_id: - tool_event_by_call_id[active_call_id]["arguments"] = args_accumulator[active_call_id] - yield _sse_event("function_call", { - "call_id": active_call_id, - "name": tool_event_by_call_id[active_call_id].get("name"), - "arguments": args_accumulator[active_call_id], - }) - - elif content_type == "mcp_server_tool_call": - # MCP tools use a different content type with tool_name instead of name - call_id = content.get("call_id") or None - name = content.get("tool_name") or content.get("name") or None - arguments = content.get("arguments", "") - rendered_arguments = ( - json.dumps(arguments, ensure_ascii=False) - if isinstance(arguments, (dict, list)) - else str(arguments) - ) - - if name and call_id and call_id not in tool_event_by_call_id: - args_accumulator[call_id] = rendered_arguments - - event_payload = { - "call_id": call_id, - "name": name, - "arguments": rendered_arguments, - "result": None, - } - tool_events.append(event_payload) - tool_event_by_call_id[call_id] = event_payload - - yield _sse_event("function_call", { - "call_id": call_id, - "name": name, - "arguments": rendered_arguments, - }) - - elif content_type in ("function_result", "mcp_server_tool_result"): - call_id = content.get("call_id") - # function_result uses "result"; mcp_server_tool_result uses "output" - result = content.get("result") if content_type == "function_result" else content.get("output") - - # Extract structured content items from the framework's "items" list. - # MCP servers return image content which the framework wraps as Content objects - # with {type:'data', uri:'data:image/jpeg;base64,...'}. Convert to frontend format. - content_items = None - items = content.get("items") - if isinstance(items, list): - converted = [] - for item in items: - if not isinstance(item, dict): - continue - item_type = item.get("type") - if item_type == "text": - converted.append({"type": "text", "text": item.get("text", "")}) - elif item_type == "data": - uri = item.get("uri", "") - if uri.startswith("data:image/"): - # Parse data URI: data:image/jpeg;base64, - header, _, b64data = uri.partition(",") - mime_type = header.split(";")[0].replace("data:", "") - if b64data and mime_type: - converted.append({"type": "image", "data": b64data, "mimeType": mime_type}) - elif item_type == "image": - # Direct image content (mcp_server_tool_result style) - if item.get("data") and item.get("mimeType"): - converted.append(item) - if any(ci["type"] == "image" for ci in converted): - content_items = converted - - # Render text for display - if isinstance(result, list): - text_parts = [ - item.get("text", "") for item in result - if isinstance(item, dict) and item.get("type") == "text" - ] - rendered_result = "\n".join(text_parts) if text_parts else json.dumps(result, ensure_ascii=False) - elif isinstance(result, dict): - rendered_result = json.dumps(result, ensure_ascii=False) - else: - rendered_result = str(result or "") - accumulated_args = args_accumulator.get(call_id, "") if call_id else "" - if call_id in tool_event_by_call_id: - tool_event_by_call_id[call_id]["result"] = rendered_result - tool_event_by_call_id[call_id]["arguments"] = accumulated_args - - # Reset active call tracking - active_call_id = None - - yield _sse_event("function_result", { - k: v for k, v in { - "call_id": call_id, - "result": rendered_result, - "arguments": accumulated_args, - "content_items": content_items, - }.items() if v is not None - }) - - elif content_type == "usage": - usage = _extract_usage_from_payload(content) - request_usage = _merge_usage(request_usage, usage) - - if getattr(msg, "text", None): - final_text_parts.append(msg.text) - yield _sse_event("text", {"content": msg.text}) - - # Get usage from the final response (usage_details is only on AgentResponse, - # not on individual stream updates) - try: - final_response = await stream.get_final_response() - if final_response and getattr(final_response, "usage_details", None): - request_usage = _merge_usage(request_usage, final_response.usage_details) - except Exception: - pass # Usage is best-effort; don't break the stream - - # Emit final usage event - if request_usage: - in_tokens = _usage_value(request_usage, USAGE_INPUT_KEY) - out_tokens = _usage_value(request_usage, USAGE_OUTPUT_KEY) - total_tokens = _usage_value(request_usage, USAGE_TOTAL_KEY) - logger.info("Token usage — IN: %d | OUT: %d | TOTAL: %d", in_tokens, out_tokens, total_tokens) - yield _sse_event("usage", { - USAGE_INPUT_KEY: in_tokens, - USAGE_OUTPUT_KEY: out_tokens, - USAGE_TOTAL_KEY: total_tokens, - }) +def _get_skills_dir() -> Path: + return Path(__file__).resolve().parent / "skills" - yield _sse_event("done", {}) - # Stash results for caller to pick up via a mutable container - # We use generator attributes for this - _stream_agent_response._last_result = { # type: ignore[attr-defined] - "text": "".join(final_text_parts).strip(), - "tool_events": tool_events, - "usage": request_usage, - } +_session_context = SessionContext( + sessions=_sessions, + session_data_cls=SessionData, + get_skills_dir=_get_skills_dir, + build_tool_instances=_build_tool_instances, + build_user_profile_context=_build_user_profile_context, +) # --------------------------------------------------------------------------- @@ -568,28 +237,6 @@ async def lifespan(app: FastAPI): ) -class McpServerEntryRequest(BaseModel): - name: str - transport: str = "http" - url: str | None = None - authenticated: bool | None = None - auth: bool | None = None - authScope: str | None = None - auth_scope: str | None = None - description: str | None = None - - -class ProfileOverrideRequest(BaseModel): - description: str | None = None - custom_prompt: str - custom_tools: list[str] = [] - custom_search_context: bool = False - custom_temperature: float | None = None - custom_skills: list[str] = [] - mcp_servers: list[McpServerEntryRequest] = [] - override_updated_at: str | None = None - - class BuiltInProfileDefinitionResponse(BaseModel): id: str name: str @@ -773,17 +420,7 @@ async def test_mcp_connections( # GET /api/skills — list available skills for custom agent builder @app.get("/api/skills") async def get_skills(user: AuthenticatedUser = Depends(get_current_user)): - from agent_framework import SkillsProvider - skills_dir = _get_skills_dir() - if not skills_dir.is_dir(): - return {"skills": []} - - provider = SkillsProvider(skill_paths=skills_dir) - skills = [ - {"name": skill.name, "description": skill.description} - for skill in provider._skills.values() - ] - return {"skills": skills} + return {"skills": SkillManager(_get_skills_dir()).list_summaries()} # --------------------------------------------------------------------------- @@ -816,14 +453,6 @@ class SkillGenerateResponse(BaseModel): content: str -_SKILL_NAME_RE = re.compile(r'^[a-z0-9][a-z0-9-]*$') -_SKILL_MD_TEMPLATE = '---\nname: {name}\ndescription: "{description}"\n---\n\n{content}\n' - - -def _get_skills_dir() -> Path: - return Path(__file__).resolve().parent / "skills" - - def _starter_definitions(raw_starters: Any) -> list[dict[str, str]]: return [ {"label": str(s.get("label", "")), "message": str(s.get("message", ""))} @@ -897,63 +526,6 @@ def _get_builtin_profile_definition(profile_id: str) -> BuiltInProfileDefinition ) -def _parse_skill_md(skill_dir: Path) -> dict: - """Parse a SKILL.md file; return {name, description, content}. Raises HTTPException(404) if missing.""" - skill_file = skill_dir / "SKILL.md" - if not skill_dir.is_dir() or not skill_file.is_file(): - raise HTTPException(status_code=404, detail=f"Skill not found: {skill_dir.name}") - raw = skill_file.read_text(encoding="utf-8") - # Parse YAML frontmatter between --- delimiters - name = skill_dir.name - description = "" - content = raw - if raw.startswith("---"): - end = raw.find("\n---", 3) - if end != -1: - frontmatter = raw[3:end].strip() - content = raw[end + 4:].lstrip("\n") - for line in frontmatter.splitlines(): - if line.startswith("description:"): - description = line[len("description:"):].strip().strip('"').strip("'") - elif line.startswith("name:"): - name = line[len("name:"):].strip() - return {"name": name, "description": description, "content": content} - - -def _validate_skill_name(name: str, status_on_error: int = 400) -> str: - """Validate that name contains only safe characters; raise HTTP error if not. - - This check runs *before* any path operation so that static analysis tools - can see the user-supplied value is sanitised prior to filesystem access. - Accepts the same alphabet as the creation regex (lowercase alphanumeric + - hyphens, starting with alphanumeric, max 64 chars). - """ - if not name or not _SKILL_NAME_RE.match(name) or len(name) > 64: - raise HTTPException(status_code=status_on_error, detail="Invalid skill name") - return name - - -def _prevent_path_traversal(name: str, skills_dir: Path) -> Path: - """Resolve skill path and ensure it stays inside skills_dir. - - `name` must already have been validated by ``_validate_skill_name`` before - this function is called. ``os.path.basename`` is applied as an additional - sanitization step so that static-analysis tools can identify the path- - traversal mitigation at the point of path construction. - """ - # os.path.basename strips any leading directory components (e.g. "../") - # so that even if an unexpected character slips past the regex the - # resulting path cannot escape the skills directory. - base_name = os.path.basename(name) - skill_path = (skills_dir / base_name).resolve() - # Belt-and-suspenders: reject anything that escaped the skills directory. - # is_relative_to is used (Python 3.9+) for cross-platform correctness - # instead of string prefix matching. - if not skill_path.is_relative_to(skills_dir.resolve()): - raise HTTPException(status_code=400, detail="Invalid skill name") - return skill_path - - # POST /api/skills/generate — generate skill markdown content from a description using the LLM @app.post("/api/skills/generate", response_model=SkillGenerateResponse) async def generate_skill_content( @@ -1015,66 +587,25 @@ async def generate_skill_content( # GET /api/skills/{name} — fetch a single skill by name @app.get("/api/skills/{name}", response_model=SkillResponse) async def get_skill(name: str, user: AuthenticatedUser = Depends(get_current_user)): - safe_name = _validate_skill_name(name) - skills_dir = _get_skills_dir() - skill_path = _prevent_path_traversal(safe_name, skills_dir) - data = _parse_skill_md(skill_path) - return SkillResponse(**data) + return SkillResponse(**SkillManager(_get_skills_dir()).get(name)) # POST /api/skills — create a new skill @app.post("/api/skills", response_model=SkillResponse, status_code=201) async def create_skill(body: SkillCreateRequest, user: AuthenticatedUser = Depends(get_current_user)): - safe_name = _validate_skill_name(body.name, status_on_error=422) - if not body.description or len(body.description) > 256: - raise HTTPException(status_code=422, detail="Description must be non-empty (max 256 chars)") - if not body.content or len(body.content) > 65536: - raise HTTPException(status_code=422, detail="Content must be non-empty (max 65536 chars)") - skills_dir = _get_skills_dir() - skill_path = _prevent_path_traversal(safe_name, skills_dir) - if skill_path.exists(): - raise HTTPException(status_code=409, detail=f"Skill already exists: {safe_name}") - skill_path.mkdir(parents=True, exist_ok=False) - skill_file = skill_path / "SKILL.md" - # Escape double quotes in description for YAML frontmatter - safe_description = body.description.replace('"', '\\"') - skill_file.write_text( - _SKILL_MD_TEMPLATE.format(name=safe_name, description=safe_description, content=body.content), - encoding="utf-8", - ) - return SkillResponse(name=safe_name, description=body.description, content=body.content) + return SkillResponse(**SkillManager(_get_skills_dir()).create(body.name, body.description, body.content)) # PUT /api/skills/{name} — update an existing skill @app.put("/api/skills/{name}", response_model=SkillResponse) async def update_skill(name: str, body: SkillUpdateRequest, user: AuthenticatedUser = Depends(get_current_user)): - safe_name = _validate_skill_name(name) - if not body.description or len(body.description) > 256: - raise HTTPException(status_code=422, detail="Description must be non-empty (max 256 chars)") - if not body.content or len(body.content) > 65536: - raise HTTPException(status_code=422, detail="Content must be non-empty (max 65536 chars)") - skills_dir = _get_skills_dir() - skill_path = _prevent_path_traversal(safe_name, skills_dir) - if not skill_path.is_dir(): - raise HTTPException(status_code=404, detail=f"Skill not found: {safe_name}") - skill_file = skill_path / "SKILL.md" - safe_description = body.description.replace('"', '\\"') - skill_file.write_text( - _SKILL_MD_TEMPLATE.format(name=safe_name, description=safe_description, content=body.content), - encoding="utf-8", - ) - return SkillResponse(name=safe_name, description=body.description, content=body.content) + return SkillResponse(**SkillManager(_get_skills_dir()).update(name, body.description, body.content)) # DELETE /api/skills/{name} — remove a skill directory @app.delete("/api/skills/{name}", status_code=204) async def delete_skill(name: str, user: AuthenticatedUser = Depends(get_current_user)): - safe_name = _validate_skill_name(name) - skills_dir = _get_skills_dir() - skill_path = _prevent_path_traversal(safe_name, skills_dir) - if not skill_path.is_dir(): - raise HTTPException(status_code=404, detail=f"Skill not found: {safe_name}") - shutil.rmtree(skill_path) + SkillManager(_get_skills_dir()).delete(name) return Response(status_code=204) @@ -1102,351 +633,13 @@ async def create_session( user: AuthenticatedUser = Depends(get_current_user), ): body = await request.json() - profile_id = body.get("profile_id", "") - - # Extract bearer token for authenticated MCP servers - auth_header = request.headers.get("authorization", "") - user_bearer_token = auth_header.removeprefix("Bearer ").strip() if auth_header.lower().startswith("bearer ") else None - - # --- Custom agent branch --- - if profile_id == "custom": - custom_name = body.get("custom_name", "").strip() - custom_prompt = body.get("custom_prompt", "").strip() - custom_tools = body.get("custom_tools", []) - custom_search_context = bool(body.get("custom_search_context", False)) - - # Parse optional custom temperature - raw_temperature = body.get("custom_temperature") - custom_temperature: float | None = None - if raw_temperature is not None: - try: - custom_temperature = float(raw_temperature) - except (ValueError, TypeError): - raise HTTPException(status_code=400, detail="custom_temperature must be a number") - if not (0.0 <= custom_temperature <= 2.0): - raise HTTPException(status_code=400, detail="custom_temperature must be between 0.0 and 2.0") - - if not custom_name or len(custom_name) > 100: - raise HTTPException(status_code=400, detail="custom_name is required and must be ≤ 100 characters") - if not custom_prompt or len(custom_prompt) > DEFAULT_MAX_USER_INPUT_CHARS: - raise HTTPException( - status_code=400, - detail=f"custom_prompt is required and must be ≤ {DEFAULT_MAX_USER_INPUT_CHARS} characters", - ) - if not isinstance(custom_tools, list): - raise HTTPException(status_code=400, detail="custom_tools must be a list of tool name strings") - - # Validate custom_tools against registered tool names from agents.yaml - agents_doc = load_agents_yaml() - profiles_data = agents_doc.get("profiles") or {} - known_tools: set[str] = {"get_user_profile", "save_user_profile"} - for entry in profiles_data.values(): - if isinstance(entry, dict): - for t in (entry.get("tools") or []): - if isinstance(t, str): - known_tools.add(t) - - invalid_tools = [t for t in custom_tools if t not in known_tools] - if invalid_tools: - raise HTTPException(status_code=400, detail=f"Unknown tools: {', '.join(invalid_tools)}") - - # Parse and validate custom_skills - custom_skills = body.get("custom_skills", []) - if not isinstance(custom_skills, list): - raise HTTPException(status_code=400, detail="custom_skills must be a list of skill name strings") - if custom_skills: - from agent_framework import SkillsProvider - skills_dir = _get_skills_dir() - if skills_dir.is_dir(): - sp = SkillsProvider(skill_paths=skills_dir) - available_skills = set(sp._skills.keys()) - else: - available_skills = set() - invalid_skills = [s for s in custom_skills if s not in available_skills] - if invalid_skills: - raise HTTPException(status_code=400, detail=f"Unknown skills: {', '.join(invalid_skills)}") - - session_id = str(uuid.uuid4()) - try: - function_tools, user_profile_store = _build_tool_instances( - set(custom_tools), - session_id=session_id, - user_profile_data=body.get("user_profile"), - ) - - # Connect inline MCP servers from request body - raw_mcp_servers = body.get("mcp_servers", []) - if not isinstance(raw_mcp_servers, list): - raise HTTPException(status_code=400, detail="mcp_servers must be a list") - - # Validate each MCP server entry (only http allowed via custom agents; - # stdio is restricted to built-in profiles in agents.yaml) - for entry in raw_mcp_servers: - if not isinstance(entry, dict): - raise HTTPException(status_code=400, detail="Each mcp_servers entry must be an object") - if not entry.get("name"): - raise HTTPException(status_code=400, detail="Each mcp_servers entry requires a 'name'") - transport = entry.get("transport") - if transport != "http": - raise HTTPException( - status_code=400, - detail="Custom agents only support 'http' MCP servers. Local (stdio) servers must be configured in agents.yaml.", - ) - if not entry.get("url"): - raise HTTPException(status_code=400, detail=f"MCP server '{entry['name']}' (http) requires a 'url'") - - mcp_configs = parse_mcp_server_configs({"mcp_servers": raw_mcp_servers}) - mcp_tools, mcp_results = await connect_mcp_servers(mcp_configs, user_token=user_bearer_token) - - # Auto-inject user profile into system prompt if agent has get_user_profile - profile_context = "" - if "get_user_profile" in set(custom_tools): - profile_context = _build_user_profile_context(body.get("user_profile")) - - chat_runtime = create_chat_runtime( - custom_name=custom_name, - custom_instructions=custom_prompt, - function_tools=function_tools, - mcp_servers=mcp_tools, - temperature=custom_temperature, - enable_search_context=custom_search_context, - custom_skills=custom_skills or None, - extra_instructions=profile_context or None, - ) - except HTTPException as e: - logger.error("Session creation failed for custom agent '%s': %s", custom_name, e.detail) - raise - except Exception as e: - logger.exception("Unexpected error creating session for custom agent '%s'", custom_name) - raise HTTPException(status_code=500, detail=str(e)) - - # Restore session history if provided - history = body.get("history") - if history and isinstance(history, dict): - try: - agent_session = AgentSession.from_dict(history) - agent_session._session_id = session_id - logger.info("Restored custom session history for session %s", session_id) - except Exception: - logger.warning("Failed to restore custom session history for %s, using fresh session", session_id) - agent_session = chat_runtime.session - else: - agent_session = chat_runtime.session - - session_data = SessionData( - session_id=session_id, - user_id=user.user_id, - profile_id="custom", - profile_name=custom_name, - agent=chat_runtime.agent, - agent_session=agent_session, - tools=chat_runtime.tools, - eval_trace_logger=EvalTraceLogger.from_env(), - prompt_manifest=chat_runtime.prompt_manifest, - prompt_logical_profile=chat_runtime.prompt_logical_profile, - ) - session_data.user_profile_store = user_profile_store - session_data.mcp_tools = mcp_tools - _sessions[session_id] = session_data - - logger.info("Created custom session %s for user %s agent=%s", session_id, user.user_id, custom_name) - - return { - "session_id": session_id, - "profile_id": "custom", - "profile_name": custom_name, - "tools_loaded": list(custom_tools), - "skills_loaded": list(custom_skills), - "search_context": custom_search_context, - "mcp_results": [ - {"name": r.name, "transport": r.transport, "status": r.status, "tool_count": r.tool_count, "error": r.error} - for r in mcp_results - ], - } - - # --- Standard profile branch (unchanged) --- - - # Validate profile exists - agents_doc = load_agents_yaml() - profiles_data = agents_doc.get("profiles") or {} - - # Accept either a direct profile key (e.g. "sql") or an exact display name. - if profile_id in profiles_data: - logical_profile = profile_id - else: - normalized_profile_id = " ".join(str(profile_id).strip().lower().split()) - logical_profile = "" - for key, entry in profiles_data.items(): - if not isinstance(entry, dict): - continue - normalized_name = " ".join(str(entry.get("name", "")).strip().lower().split()) - if normalized_name and normalized_name == normalized_profile_id: - logical_profile = key - break - - if logical_profile not in profiles_data: - raise HTTPException(status_code=400, detail=f"Unknown profile: {profile_id}") - - profile_entry = profiles_data[logical_profile] - profile_name = str(profile_entry.get("name", logical_profile)) - raw_profile_override = body.get("profile_override") - profile_override: ProfileOverrideRequest | None = None - if raw_profile_override is not None: - if not isinstance(raw_profile_override, dict): - raise HTTPException(status_code=400, detail="profile_override must be an object") - if "name" in raw_profile_override or "custom_name" in raw_profile_override: - raise HTTPException(status_code=400, detail="Built-in profile overrides cannot change the agent name") - try: - profile_override = ProfileOverrideRequest(**raw_profile_override) - except Exception as e: - raise HTTPException(status_code=400, detail=f"Invalid profile_override: {e}") - - if not profile_override.custom_prompt.strip() or len(profile_override.custom_prompt) > DEFAULT_MAX_USER_INPUT_CHARS: - raise HTTPException( - status_code=400, - detail=f"custom_prompt is required and must be ≤ {DEFAULT_MAX_USER_INPUT_CHARS} characters", - ) - if profile_override.custom_temperature is not None and not (0.0 <= profile_override.custom_temperature <= 2.0): - raise HTTPException(status_code=400, detail="custom_temperature must be between 0.0 and 2.0") - - session_id = str(uuid.uuid4()) - - profile_tool_names = ( - list(profile_override.custom_tools) - if profile_override is not None - else (profile_entry.get("tools") or []) + return await create_chat_session( + _session_context, + body=body, + auth_header=request.headers.get("authorization", ""), + user=user, + logger=logger, ) - try: - function_tools, user_profile_store = _build_tool_instances( - set(profile_tool_names), - session_id=session_id, - user_profile_data=body.get("user_profile"), - ) - - if profile_override is not None: - known_tools: set[str] = {"get_user_profile", "save_user_profile"} - for entry in profiles_data.values(): - if isinstance(entry, dict): - for t in (entry.get("tools") or []): - if isinstance(t, str): - known_tools.add(t) - invalid_tools = [t for t in profile_override.custom_tools if t not in known_tools] - if invalid_tools: - raise HTTPException(status_code=400, detail=f"Unknown tools: {', '.join(invalid_tools)}") - - if profile_override.custom_skills: - from agent_framework import SkillsProvider - skills_dir = _get_skills_dir() - if skills_dir.is_dir(): - sp = SkillsProvider(skill_paths=skills_dir) - available_skills = set(sp._skills.keys()) - else: - available_skills = set() - invalid_skills = [s for s in profile_override.custom_skills if s not in available_skills] - if invalid_skills: - raise HTTPException(status_code=400, detail=f"Unknown skills: {', '.join(invalid_skills)}") - - raw_mcp_servers = [server.model_dump(exclude_none=True) for server in profile_override.mcp_servers] - for entry in raw_mcp_servers: - if entry.get("transport") != "http": - raise HTTPException(status_code=400, detail="Built-in profile overrides only support 'http' MCP servers") - if not entry.get("url"): - raise HTTPException(status_code=400, detail=f"MCP server '{entry['name']}' (http) requires a 'url'") - mcp_configs = parse_mcp_server_configs({"mcp_servers": raw_mcp_servers}) - else: - mcp_configs = parse_mcp_server_configs(profile_entry) - mcp_tools, mcp_results = await connect_mcp_servers(mcp_configs, user_token=user_bearer_token) - - # Auto-inject user profile into system prompt if agent has get_user_profile - profile_context = "" - if "get_user_profile" in profile_tool_names: - profile_context = _build_user_profile_context(body.get("user_profile")) - - if profile_override is not None: - chat_runtime = create_chat_runtime( - custom_name=profile_name, - custom_instructions=profile_override.custom_prompt.strip(), - function_tools=function_tools, - mcp_servers=mcp_tools, - temperature=profile_override.custom_temperature, - enable_search_context=profile_override.custom_search_context, - custom_skills=profile_override.custom_skills or None, - extra_instructions=profile_context or None, - ) - else: - chat_runtime = create_chat_runtime( - chat_profile=get_profile_display_name(logical_profile, fallback=profile_id), - function_tools=function_tools, - mcp_servers=mcp_tools, - extra_instructions=profile_context or None, - ) - except HTTPException as e: - logger.error("Session creation failed for profile '%s': %s", logical_profile, e.detail) - raise - except Exception as e: - logger.exception("Unexpected error creating session for profile '%s'", logical_profile) - raise HTTPException(status_code=500, detail=str(e)) - - # Restore session history if provided - history = body.get("history") - if history and isinstance(history, dict): - try: - agent_session = AgentSession.from_dict(history) - agent_session._session_id = session_id - logger.info("Restored session history for session %s", session_id) - except Exception: - logger.warning("Failed to restore session history for session %s, using fresh session", session_id) - agent_session = chat_runtime.session - else: - agent_session = chat_runtime.session - - session_data = SessionData( - session_id=session_id, - user_id=user.user_id, - profile_id=logical_profile, - profile_name=profile_name, - agent=chat_runtime.agent, - agent_session=agent_session, - tools=chat_runtime.tools, - eval_trace_logger=EvalTraceLogger.from_env(), - prompt_manifest=chat_runtime.prompt_manifest, - prompt_logical_profile=chat_runtime.prompt_logical_profile, - ) - session_data.user_profile_store = user_profile_store - session_data.mcp_tools = mcp_tools - session_data.used_profile_override = profile_override is not None - session_data.override_updated_at = profile_override.override_updated_at if profile_override else None - - _sessions[session_id] = session_data - - logger.info("Created session %s for user %s profile %s", session_id, user.user_id, logical_profile) - - profile_skills = ( - list(profile_override.custom_skills) - if profile_override is not None - else [str(s) for s in (profile_entry.get("skills") or []) if isinstance(s, str)] - ) - profile_search_context = ( - bool(profile_override.custom_search_context) - if profile_override is not None - else bool(profile_entry.get("search_context", False)) - ) - - return { - "session_id": session_id, - "profile_id": logical_profile, - "profile_name": profile_name, - "tools_loaded": list(profile_tool_names), - "skills_loaded": profile_skills, - "search_context": profile_search_context, - "mcp_results": [ - {"name": r.name, "transport": r.transport, "status": r.status, "tool_count": r.tool_count, "error": r.error} - for r in mcp_results - ], - "used_profile_override": profile_override is not None, - "override_updated_at": profile_override.override_updated_at if profile_override else None, - } - # T017 + T029 — POST /api/sessions/{session_id}/messages (text + multipart) @app.post("/api/sessions/{session_id}/messages") @@ -1483,7 +676,7 @@ async def send_message( # Validate images if present (T029) if image_files: - error = _validate_uploaded_images(image_files, image_data_list) + error = validate_uploaded_images(image_files, image_data_list) if error: raise HTTPException(status_code=400, detail=error) @@ -1504,50 +697,50 @@ async def send_message( # Stream the response async def generate() -> AsyncGenerator[str, None]: try: - async for event in _stream_agent_response( + async for event in stream_agent_response( session_data.agent, contents, session_data.agent_session, ): yield event except Exception as e: # T020 — error handling - if _is_retryable_error(e): + if is_retryable_error(e): logger.error("Rate limit error: %s", e) - yield _sse_event("error", { + yield sse_event("error", { "message": "The AI service is currently experiencing high demand. Please try again in a moment.", "retry_after": 30, }) - elif _is_context_length_error(e): + elif is_context_length_error(e): logger.error("Context length exceeded: %s", e) - yield _sse_event("error", { + yield sse_event("error", { "message": "The request exceeded model context limits. Please shorten your prompt or start a new chat.", "retry_after": None, }) - elif image_files and not _is_context_length_error(e) and not _is_retryable_error(e): + elif image_files and not is_context_length_error(e) and not is_retryable_error(e): logger.error("Error processing image message: %s", e, exc_info=True) - yield _sse_event("error", { + yield sse_event("error", { "message": "One or more images could not be processed. Please try again or use a different image.", "retry_after": None, }) else: logger.error("Error processing message: %s", e, exc_info=True) - yield _sse_event("error", { + yield sse_event("error", { "message": f"An error occurred while processing your request: {str(e)}", "retry_after": None, }) - yield _sse_event("done", {}) + yield sse_event("done", {}) return # After successful streaming, update session usage and log trace (T019) - result = getattr(_stream_agent_response, "_last_result", None) + result = getattr(stream_agent_response, "_last_result", None) if result: request_usage = result.get("usage") if request_usage: - session_data.usage = _merge_usage(session_data.usage, request_usage) + session_data.usage = merge_usage(session_data.usage, request_usage) logger.info( "Request token usage - Input: %s, Output: %s, Total: %s", - _usage_value(request_usage, USAGE_INPUT_KEY), - _usage_value(request_usage, USAGE_OUTPUT_KEY), - _usage_value(request_usage, USAGE_TOTAL_KEY), + usage_value(request_usage, USAGE_INPUT_KEY), + usage_value(request_usage, USAGE_OUTPUT_KEY), + usage_value(request_usage, USAGE_TOTAL_KEY), ) # Eval trace logging @@ -1564,9 +757,9 @@ async def generate() -> AsyncGenerator[str, None]: getattr(t, "name", str(t)) for t in session_data.tools ], "usage": { - USAGE_INPUT_KEY: _usage_value(request_usage, USAGE_INPUT_KEY), - USAGE_OUTPUT_KEY: _usage_value(request_usage, USAGE_OUTPUT_KEY), - USAGE_TOTAL_KEY: _usage_value(request_usage, USAGE_TOTAL_KEY), + USAGE_INPUT_KEY: usage_value(request_usage, USAGE_INPUT_KEY), + USAGE_OUTPUT_KEY: usage_value(request_usage, USAGE_OUTPUT_KEY), + USAGE_TOTAL_KEY: usage_value(request_usage, USAGE_TOTAL_KEY), } if request_usage else {}, "context_usage": session_data.context_usage, }) @@ -1593,9 +786,9 @@ async def delete_session( logger.info( "Session %s ended — Token usage - Input: %s, Output: %s, Total: %s", session_id, - _usage_value(session_data.usage, USAGE_INPUT_KEY), - _usage_value(session_data.usage, USAGE_OUTPUT_KEY), - _usage_value(session_data.usage, USAGE_TOTAL_KEY), + usage_value(session_data.usage, USAGE_INPUT_KEY), + usage_value(session_data.usage, USAGE_OUTPUT_KEY), + usage_value(session_data.usage, USAGE_TOTAL_KEY), ) return None diff --git a/scripts/capture_admin_agent_screenshots.py b/scripts/capture_admin_agent_screenshots.py index ade3b49..5bb935c 100644 --- a/scripts/capture_admin_agent_screenshots.py +++ b/scripts/capture_admin_agent_screenshots.py @@ -23,6 +23,84 @@ "updatedAt": "2026-05-06T12:00:00.000Z", } +DEFAULT_PROFILE = { + "id": "visual-agent", + "name": "Visual Agent", + "description": "Screenshot verification profile", + "icon": "/favicon.png", + "starters": [ + {"label": "Summarize", "message": "Summarize the latest status."}, + {"label": "Plan", "message": "Make a concise plan."}, + ], +} + + +def setup_mock_api(page: Page) -> None: + def handler(route) -> None: + request = route.request + url = request.url + method = request.method + + if url.endswith("/api/auth/config"): + route.fulfill(json={ + "authDisabled": True, + "tenantId": "", + "clientId": "", + "authority": "https://login.microsoftonline.us", + "classificationBanner": "UNCLASSIFIED", + "appName": "Web-Agents", + "appTagline": "AI Agent Framework", + "appLogo": "/Microsoft.png", + }) + elif url.endswith("/api/profiles"): + route.fulfill(json={"profiles": [DEFAULT_PROFILE], "unavailable": []}) + elif url.endswith("/api/tools"): + route.fulfill(json={"tools": [], "unavailable": [], "search_context_available": True, "search_context_reason": None}) + elif url.endswith("/api/skills") and method == "GET": + route.fulfill(json={"skills": [{"name": "visual-skill", "description": "Screenshot skill"}]}) + elif "/api/skills/visual-skill" in url and method == "GET": + route.fulfill(json={"name": "visual-skill", "description": "Screenshot skill", "content": "# Visual Skill\n\nScreenshot skill instructions."}) + elif "/api/profiles/" in url and url.endswith("/definition"): + route.fulfill(json={ + **DEFAULT_PROFILE, + "systemPrompt": "You are a visual verification agent.", + "tools": [], + "skills": [], + "mcpServers": [], + "useSearchContext": False, + "temperature": 0.2, + }) + elif url.endswith("/api/sessions") and method == "POST": + route.fulfill(json={ + "session_id": "visual-session", + "profile_id": "visual-agent", + "profile_name": "Visual Agent", + "tools_loaded": [], + "skills_loaded": [], + "search_context": False, + "mcp_results": [], + }) + elif url.endswith("/api/sessions/visual-session/messages") and method == "POST": + route.fulfill( + status=200, + headers={"content-type": "text/event-stream"}, + body=( + 'event: text\ndata: {"content":"Visual verification response."}\n\n' + 'event: usage\ndata: {"input_token_count":4,"output_token_count":5,"total_token_count":9}\n\n' + 'event: done\ndata: {}\n\n' + ), + ) + elif url.endswith("/api/sessions/visual-session/history"): + route.fulfill(json={"session_id": "visual-session", "profile_id": "visual-agent", "profile_name": "Visual Agent", "session_data": {}}) + elif url.endswith("/api/mcp/test"): + route.fulfill(json={"results": []}) + elif url.endswith("/api/skills/generate"): + route.fulfill(json={"content": "# Visual Skill\n\nGenerated screenshot content."}) + else: + route.continue_() + + page.route("**/api/**", handler) + def accept_disclaimer(page: Page) -> None: button = page.get_by_role("button", name="I UNDERSTAND AND AGREE") @@ -30,10 +108,19 @@ def accept_disclaimer(page: Page) -> None: button.click() +def capture_disclaimer_states(page: Page, output_dir: Path) -> None: + content = page.locator(".disclaimer-content") + if content.count() == 0: + return + page.screenshot(path=str(output_dir / "007-disclaimer-top.png"), full_page=True) + content.evaluate("element => { element.scrollTop = element.scrollHeight; }") + page.screenshot(path=str(output_dir / "007-disclaimer-bottom.png"), full_page=True) + + def open_admin(page: Page) -> None: admin_button = page.locator("button.sidebar-advanced-btn").filter(has_text="ADMIN") admin_button.click(timeout=5_000) - page.get_by_text("BUILT-IN AGENTS").wait_for(timeout=5_000) + page.get_by_role("button", name="BUILT-IN AGENTS").wait_for(timeout=5_000) def open_skills(page: Page) -> None: @@ -41,6 +128,20 @@ def open_skills(page: Page) -> None: page.get_by_text("SAVED SKILLS").wait_for(timeout=5_000) +def capture_profile_and_chat_states(page: Page, output_dir: Path) -> None: + page.get_by_text("SELECT YOUR AGENT").wait_for(timeout=5_000) + page.screenshot(path=str(output_dir / "007-profile-selection.png"), full_page=True) + + page.locator(".profile-card").first.click(timeout=5_000) + page.locator(".chat-input-textarea").wait_for(timeout=5_000) + page.screenshot(path=str(output_dir / "007-empty-chat-starters.png"), full_page=True) + + page.locator(".chat-input-textarea").fill("Hello from visual verification") + page.get_by_role("button", name="TRANSMIT").click(timeout=5_000) + page.get_by_text("Visual verification response.").wait_for(timeout=5_000) + page.screenshot(path=str(output_dir / "007-chat-one-turn.png"), full_page=True) + + def ensure_expanded(page: Page, section_index: int) -> None: toggle = page.locator(".agent-builder-section-toggle").nth(section_index) if toggle.get_attribute("aria-expanded") == "false": @@ -122,13 +223,17 @@ def capture(args: argparse.Namespace) -> None: executable_path=args.chrome_path, ) page = browser.new_page(viewport={"width": args.width, "height": args.height}) + if args.mock_api: + setup_mock_api(page) page.goto(args.base_url, wait_until="networkidle") + capture_disclaimer_states(page, output_dir) page.evaluate( "value => localStorage.setItem('webagents_custom_agents', value)", custom_agents, ) page.reload(wait_until="networkidle") accept_disclaimer(page) + capture_profile_and_chat_states(page, output_dir) open_admin(page) ensure_expanded(page, 0) @@ -158,6 +263,7 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--chrome-path", default="/usr/bin/google-chrome") parser.add_argument("--width", type=int, default=1440) parser.add_argument("--height", type=int, default=900) + parser.add_argument("--mock-api", action="store_true", help="Mock API responses for deterministic visual captures.") return parser.parse_args() diff --git a/session_orchestration.py b/session_orchestration.py new file mode 100644 index 0000000..529dea9 --- /dev/null +++ b/session_orchestration.py @@ -0,0 +1,405 @@ +"""Session creation helpers for FastAPI routes.""" + +import logging +import os +import re +import uuid +from collections.abc import Callable +from dataclasses import dataclass +from pathlib import Path +from typing import Any +from urllib.parse import parse_qsl, urlencode, urlsplit, urlunsplit + +from agent_framework import AgentSession +from fastapi import HTTPException +from pydantic import BaseModel + +from agent_factory import create_chat_runtime +from eval_trace import EvalTraceLogger +from mcp_servers import connect_mcp_servers, parse_mcp_server_configs +from prompt_config import get_profile_display_name, load_agents_yaml +from validators import ( + available_skill_names, + filter_known_skill_names, + known_tool_names_from_profiles, + validate_custom_name, + validate_http_mcp_servers, + validate_prompt, + validate_temperature, + validate_tool_names, +) + +MAX_USER_INPUT_CHARS = int(os.getenv("MAX_USER_INPUT_CHARS", "8000")) + +_SECRET_QUERY_KEYS = {"api_key", "apikey", "code", "token", "access_token", "client_secret", "password"} +_SECRET_ASSIGNMENT_RE = re.compile( + r"(?i)\b(api[_-]?key|access[_-]?token|client[_-]?secret|password|connectionstring|connection_string)\s*=\s*[^\s&]+" +) +_BEARER_RE = re.compile(r"(?i)bearer\s+[A-Za-z0-9._~+/=-]+") + + +class McpServerEntryRequest(BaseModel): + name: str + transport: str = "http" + url: str | None = None + authenticated: bool | None = None + auth: bool | None = None + authScope: str | None = None + auth_scope: str | None = None + description: str | None = None + + +class ProfileOverrideRequest(BaseModel): + description: str | None = None + custom_prompt: str + custom_tools: list[str] = [] + custom_search_context: bool = False + custom_temperature: float | None = None + custom_skills: list[str] = [] + mcp_servers: list[McpServerEntryRequest] = [] + override_updated_at: str | None = None + + +@dataclass(frozen=True) +class SessionContext: + """Bundle the FastAPI-app dependencies that session creation needs. + + Built once by `main.py` so route handlers don't have to thread these + callbacks/objects through every call. + """ + sessions: dict[str, Any] + session_data_cls: type + get_skills_dir: Callable[[], Path] + build_tool_instances: Callable[..., tuple[list[Any], Any]] + build_user_profile_context: Callable[[dict[str, str] | None], str] + + +def _sanitize_url(value: str) -> str: + try: + parsed = urlsplit(value) + except ValueError: + return value + if not parsed.scheme or not parsed.netloc: + return value + safe_query = urlencode([ + (key, val) + for key, val in parse_qsl(parsed.query, keep_blank_values=True) + if key.lower() not in _SECRET_QUERY_KEYS + ]) + return urlunsplit((parsed.scheme, parsed.netloc, parsed.path, safe_query, parsed.fragment)) + + +def sanitize_mcp_result_error(error: str | None) -> str | None: + if error is None: + return None + sanitized = re.sub(r"https?://[^\s)]+", lambda match: _sanitize_url(match.group(0)), str(error)) + sanitized = _BEARER_RE.sub("[REDACTED_TOKEN]", sanitized) + sanitized = _SECRET_ASSIGNMENT_RE.sub(lambda match: f"{match.group(1)} [REDACTED]", sanitized) + return sanitized + + +def _resolve_profile(profile_id: str, profiles_data: dict[str, Any]) -> str: + if profile_id in profiles_data: + return profile_id + + normalized_profile_id = " ".join(str(profile_id).strip().lower().split()) + for key, entry in profiles_data.items(): + if not isinstance(entry, dict): + continue + normalized_name = " ".join(str(entry.get("name", "")).strip().lower().split()) + if normalized_name and normalized_name == normalized_profile_id: + return key + + raise HTTPException(status_code=400, detail=f"Unknown profile: {profile_id}") + + +def _serialize_mcp_results(results: list[Any]) -> list[dict[str, Any]]: + return [ + { + "name": result.name, + "transport": result.transport, + "status": result.status, + "tool_count": result.tool_count, + "error": sanitize_mcp_result_error(result.error), + } + for result in results + ] + + +def _restore_session_history(history: object, session_id: str, fallback_session: AgentSession, logger: logging.Logger) -> AgentSession: + if history and isinstance(history, dict): + try: + agent_session = AgentSession.from_dict(history) + agent_session._session_id = session_id + logger.info("Restored session history for session %s", session_id) + return agent_session + except Exception: + logger.warning("Failed to restore session history for %s, using fresh session", session_id) + return fallback_session + + +def _parse_profile_override(raw_profile_override: object) -> ProfileOverrideRequest | None: + if raw_profile_override is None: + return None + if not isinstance(raw_profile_override, dict): + raise HTTPException(status_code=400, detail="profile_override must be an object") + if "name" in raw_profile_override or "custom_name" in raw_profile_override: + raise HTTPException(status_code=400, detail="Built-in profile overrides cannot change the agent name") + try: + profile_override = ProfileOverrideRequest(**raw_profile_override) + except Exception as exc: + raise HTTPException(status_code=400, detail=f"Invalid profile_override: {exc}") from exc + + profile_override.custom_prompt = validate_prompt(profile_override.custom_prompt, max_chars=MAX_USER_INPUT_CHARS) + if profile_override.custom_temperature is not None: + validate_temperature(profile_override.custom_temperature) + return profile_override + + +def _store_session( + ctx: SessionContext, + *, + session_id: str, + user: Any, + profile_id: str, + profile_name: str, + chat_runtime: Any, + agent_session: AgentSession, + user_profile_store: Any, + mcp_tools: list[Any], + profile_override: "ProfileOverrideRequest | None" = None, +) -> None: + session_data = ctx.session_data_cls( + session_id=session_id, + user_id=user.user_id, + profile_id=profile_id, + profile_name=profile_name, + agent=chat_runtime.agent, + agent_session=agent_session, + tools=chat_runtime.tools, + eval_trace_logger=EvalTraceLogger.from_env(), + prompt_manifest=chat_runtime.prompt_manifest, + prompt_logical_profile=chat_runtime.prompt_logical_profile, + ) + session_data.user_profile_store = user_profile_store + session_data.mcp_tools = mcp_tools + if profile_override is not None: + session_data.used_profile_override = True + session_data.override_updated_at = profile_override.override_updated_at + ctx.sessions[session_id] = session_data + + +async def create_chat_session( + ctx: SessionContext, + *, + body: dict[str, Any], + auth_header: str, + user: Any, + logger: logging.Logger, +) -> dict[str, Any]: + profile_id = body.get("profile_id", "") + user_bearer_token = ( + auth_header.removeprefix("Bearer ").strip() + if auth_header.lower().startswith("bearer ") + else None + ) + agents_doc = load_agents_yaml() + profiles_data = agents_doc.get("profiles") or {} + + if profile_id == "custom": + return await _create_custom_chat_session( + ctx, + body=body, + profiles_data=profiles_data, + user_bearer_token=user_bearer_token, + user=user, + logger=logger, + ) + + return await _create_profile_chat_session( + ctx, + body=body, + profiles_data=profiles_data, + user_bearer_token=user_bearer_token, + user=user, + logger=logger, + ) + + +async def _create_custom_chat_session( + ctx: SessionContext, + *, + body: dict[str, Any], + profiles_data: dict[str, Any], + user_bearer_token: str | None, + user: Any, + logger: logging.Logger, +) -> dict[str, Any]: + custom_name = validate_custom_name(body.get("custom_name", "")) + custom_prompt = validate_prompt(body.get("custom_prompt", ""), max_chars=MAX_USER_INPUT_CHARS) + custom_search_context = bool(body.get("custom_search_context", False)) + custom_temperature = validate_temperature(body.get("custom_temperature")) + custom_tools = validate_tool_names(body.get("custom_tools", []), known_tool_names_from_profiles(profiles_data)) + custom_skills, dropped_skills = filter_known_skill_names(body.get("custom_skills", []), available_skill_names(ctx.get_skills_dir())) + if dropped_skills: + logger.warning("Custom agent '%s' references unknown skills, dropping: %s", custom_name, dropped_skills) + raw_mcp_servers = validate_http_mcp_servers(body.get("mcp_servers", []), override=False) + + session_id = str(uuid.uuid4()) + custom_tool_set = set(custom_tools) + try: + function_tools, user_profile_store = ctx.build_tool_instances( + custom_tool_set, + session_id=session_id, + user_profile_data=body.get("user_profile"), + ) + mcp_configs = parse_mcp_server_configs({"mcp_servers": raw_mcp_servers}) + mcp_tools, mcp_results = await connect_mcp_servers(mcp_configs, user_token=user_bearer_token) + profile_context = ctx.build_user_profile_context(body.get("user_profile")) if "get_user_profile" in custom_tool_set else "" + chat_runtime = create_chat_runtime( + custom_name=custom_name, + custom_instructions=custom_prompt, + function_tools=function_tools, + mcp_servers=mcp_tools, + temperature=custom_temperature, + enable_search_context=custom_search_context, + custom_skills=custom_skills or None, + extra_instructions=profile_context or None, + ) + except HTTPException as exc: + logger.error("Session creation failed for custom agent '%s': %s", custom_name, exc.detail) + raise + except Exception as exc: + logger.exception("Unexpected error creating session for custom agent '%s'", custom_name) + raise HTTPException(status_code=500, detail=sanitize_mcp_result_error(str(exc))) from exc + + agent_session = _restore_session_history(body.get("history"), session_id, chat_runtime.session, logger) + _store_session( + ctx, + session_id=session_id, + user=user, + profile_id="custom", + profile_name=custom_name, + chat_runtime=chat_runtime, + agent_session=agent_session, + user_profile_store=user_profile_store, + mcp_tools=mcp_tools, + ) + + logger.info("Created custom session %s for user %s agent=%s", session_id, user.user_id, custom_name) + return { + "session_id": session_id, + "profile_id": "custom", + "profile_name": custom_name, + "tools_loaded": list(custom_tools), + "skills_loaded": list(custom_skills), + "search_context": custom_search_context, + "mcp_results": _serialize_mcp_results(mcp_results), + } + + +async def _create_profile_chat_session( + ctx: SessionContext, + *, + body: dict[str, Any], + profiles_data: dict[str, Any], + user_bearer_token: str | None, + user: Any, + logger: logging.Logger, +) -> dict[str, Any]: + profile_id = body.get("profile_id", "") + logical_profile = _resolve_profile(profile_id, profiles_data) + profile_entry = profiles_data[logical_profile] + profile_name = str(profile_entry.get("name", logical_profile)) + profile_override = _parse_profile_override(body.get("profile_override")) + profile_tool_names = list(profile_override.custom_tools) if profile_override is not None else (profile_entry.get("tools") or []) + session_id = str(uuid.uuid4()) + + try: + function_tools, user_profile_store = ctx.build_tool_instances( + set(profile_tool_names), + session_id=session_id, + user_profile_data=body.get("user_profile"), + ) + + if profile_override is not None: + validate_tool_names(profile_override.custom_tools, known_tool_names_from_profiles(profiles_data)) + if profile_override.custom_skills: + kept, dropped = filter_known_skill_names(profile_override.custom_skills, available_skill_names(ctx.get_skills_dir())) + if dropped: + logger.warning("Profile override for '%s' references unknown skills, dropping: %s", logical_profile, dropped) + profile_override.custom_skills = kept + raw_mcp_servers = [server.model_dump(exclude_none=True) for server in profile_override.mcp_servers] + validate_http_mcp_servers(raw_mcp_servers, override=True) + mcp_configs = parse_mcp_server_configs({"mcp_servers": raw_mcp_servers}) + else: + mcp_configs = parse_mcp_server_configs(profile_entry) + + mcp_tools, mcp_results = await connect_mcp_servers(mcp_configs, user_token=user_bearer_token) + profile_context = ctx.build_user_profile_context(body.get("user_profile")) if "get_user_profile" in profile_tool_names else "" + + if profile_override is not None: + chat_runtime = create_chat_runtime( + custom_name=profile_name, + custom_instructions=profile_override.custom_prompt, + function_tools=function_tools, + mcp_servers=mcp_tools, + temperature=profile_override.custom_temperature, + enable_search_context=profile_override.custom_search_context, + custom_skills=profile_override.custom_skills or None, + extra_instructions=profile_context or None, + ) + else: + chat_runtime = create_chat_runtime( + chat_profile=get_profile_display_name(logical_profile, fallback=profile_id), + function_tools=function_tools, + mcp_servers=mcp_tools, + extra_instructions=profile_context or None, + ) + except HTTPException as exc: + logger.error("Session creation failed for profile '%s': %s", logical_profile, exc.detail) + raise + except Exception as exc: + logger.exception("Unexpected error creating session for profile '%s'", logical_profile) + raise HTTPException(status_code=500, detail=sanitize_mcp_result_error(str(exc))) from exc + + agent_session = _restore_session_history(body.get("history"), session_id, chat_runtime.session, logger) + _store_session( + ctx, + session_id=session_id, + user=user, + profile_id=logical_profile, + profile_name=profile_name, + chat_runtime=chat_runtime, + agent_session=agent_session, + user_profile_store=user_profile_store, + mcp_tools=mcp_tools, + profile_override=profile_override, + ) + + logger.info("Created session %s for user %s profile %s", session_id, user.user_id, logical_profile) + profile_skills = list(profile_override.custom_skills) if profile_override is not None else [ + str(skill) for skill in (profile_entry.get("skills") or []) if isinstance(skill, str) + ] + profile_search_context = bool(profile_override.custom_search_context) if profile_override is not None else bool(profile_entry.get("search_context", False)) + + return { + "session_id": session_id, + "profile_id": logical_profile, + "profile_name": profile_name, + "tools_loaded": list(profile_tool_names), + "skills_loaded": profile_skills, + "search_context": profile_search_context, + "mcp_results": _serialize_mcp_results(mcp_results), + "used_profile_override": profile_override is not None, + "override_updated_at": profile_override.override_updated_at if profile_override else None, + } + + +__all__ = [ + "McpServerEntryRequest", + "ProfileOverrideRequest", + "SessionContext", + "create_chat_session", + "sanitize_mcp_result_error", +] \ No newline at end of file diff --git a/skills_manager.py b/skills_manager.py new file mode 100644 index 0000000..f8b981c --- /dev/null +++ b/skills_manager.py @@ -0,0 +1,112 @@ +"""File-backed skill management helpers.""" + +import os +import re +import shutil +from pathlib import Path + +from fastapi import HTTPException + +_SKILL_NAME_RE = re.compile(r"^[a-z0-9][a-z0-9-]*$") +_SKILL_MD_TEMPLATE = '---\nname: {name}\ndescription: "{description}"\n---\n\n{content}\n' + + +class SkillManager: + def __init__(self, skills_dir: Path): + self.skills_dir = skills_dir + + def validate_name(self, name: str, status_on_error: int = 400) -> str: + if not name or not _SKILL_NAME_RE.match(name) or len(name) > 64: + raise HTTPException(status_code=status_on_error, detail="Invalid skill name") + return name + + def skill_path(self, name: str, status_on_error: int = 400) -> Path: + safe_name = self.validate_name(name, status_on_error=status_on_error) + base_name = os.path.basename(safe_name) + skill_path = (self.skills_dir / base_name).resolve() + if not skill_path.is_relative_to(self.skills_dir.resolve()): + raise HTTPException(status_code=400, detail="Invalid skill name") + return skill_path + + def parse(self, skill_dir: Path) -> dict: + skill_file = skill_dir / "SKILL.md" + if not skill_dir.is_dir() or not skill_file.is_file(): + raise HTTPException(status_code=404, detail=f"Skill not found: {skill_dir.name}") + raw = skill_file.read_text(encoding="utf-8") + name = skill_dir.name + description = "" + content = raw + if raw.startswith("---"): + end = raw.find("\n---", 3) + if end != -1: + frontmatter = raw[3:end].strip() + content = raw[end + 4:].lstrip("\n") + for line in frontmatter.splitlines(): + if line.startswith("description:"): + description = line[len("description:"):].strip().strip('"').strip("'") + elif line.startswith("name:"): + name = line[len("name:"):].strip() + return {"name": name, "description": description, "content": content} + + def list_summaries(self) -> list[dict[str, str]]: + if not self.skills_dir.is_dir(): + return [] + from agent_framework import SkillsProvider + + provider = SkillsProvider(skill_paths=self.skills_dir) + return [ + {"name": skill.name, "description": skill.description} + for skill in provider._skills.values() + ] + + def get(self, name: str) -> dict: + return self.parse(self.skill_path(name)) + + def create(self, name: str, description: str, content: str) -> dict: + safe_name = self.validate_name(name, status_on_error=422) + self._validate_description(description) + self._validate_content(content) + skill_path = self.skill_path(safe_name) + if skill_path.exists(): + raise HTTPException(status_code=409, detail=f"Skill already exists: {safe_name}") + skill_path.mkdir(parents=True, exist_ok=False) + self._write_skill(skill_path, safe_name, description, content) + return {"name": safe_name, "description": description, "content": content} + + def update(self, name: str, description: str, content: str) -> dict: + safe_name = self.validate_name(name) + self._validate_description(description) + self._validate_content(content) + skill_path = self.skill_path(safe_name) + if not skill_path.is_dir(): + raise HTTPException(status_code=404, detail=f"Skill not found: {safe_name}") + self._write_skill(skill_path, safe_name, description, content) + return {"name": safe_name, "description": description, "content": content} + + def delete(self, name: str) -> None: + safe_name = self.validate_name(name) + skill_path = self.skill_path(safe_name) + if not skill_path.is_dir(): + raise HTTPException(status_code=404, detail=f"Skill not found: {safe_name}") + shutil.rmtree(skill_path) + + @staticmethod + def _validate_description(description: str) -> None: + if not description or len(description) > 256: + raise HTTPException(status_code=422, detail="Description must be non-empty (max 256 chars)") + + @staticmethod + def _validate_content(content: str) -> None: + if not content or len(content) > 65536: + raise HTTPException(status_code=422, detail="Content must be non-empty (max 65536 chars)") + + @staticmethod + def _write_skill(skill_path: Path, name: str, description: str, content: str) -> None: + safe_description = description.replace('"', '\\"') + (skill_path / "SKILL.md").write_text( + _SKILL_MD_TEMPLATE.format(name=name, description=safe_description, content=content), + encoding="utf-8", + ) + + +__all__ = ["SkillManager"] \ No newline at end of file diff --git a/specs/007-application-refactor/contracts/api-contract.md b/specs/007-application-refactor/contracts/api-contract.md new file mode 100644 index 0000000..3cfd3aa --- /dev/null +++ b/specs/007-application-refactor/contracts/api-contract.md @@ -0,0 +1,65 @@ +# API Contract: Behavior-Preserving Refactor + +This refactor must not introduce breaking backend API changes. Routes may call new helpers/services internally, but external behavior remains stable. + +## Contract Rules + +- Preserve existing route paths, methods, status codes, and response field names unless a later task explicitly documents a compatibility-preserving migration. +- Preserve SSE event names: `text`, `function_call`, `function_result`, `usage`, `error`, `done`. +- Preserve auth behavior, including `AuthError` handling on the frontend for 401 responses. +- Preserve local development behavior when `AUTH_DISABLED=true`. +- Preserve error-message semantics where tests or UI flows depend on them. +- Do not edit `config/agents.yaml`. + +## Routes To Preserve + +| Method | Route | Contract Notes | +|--------|-------|----------------| +| GET | `/api/health` | Returns `{ "status": "healthy" }`. | +| GET | `/api/auth/config` | Returns frontend auth/runtime config without secrets. | +| GET | `/api/tools` | Returns available/unavailable tools plus search context availability. | +| GET | `/api/profiles` | Returns healthy profiles and unavailable profile reasons. | +| GET | `/api/profiles/{profile_id}/definition` | Returns built-in profile definition without secret fields. | +| POST | `/api/mcp/test` | Tests inline HTTP MCP servers and returns per-server results. | +| GET | `/api/skills` | Lists skill summaries. | +| POST | `/api/skills/generate` | Generates skill Markdown body from description. | +| GET | `/api/skills/{name}` | Returns `{ name, description, content }`. | +| POST | `/api/skills` | Creates a skill and returns created definition with 201. | +| PUT | `/api/skills/{name}` | Updates skill definition. | +| DELETE | `/api/skills/{name}` | Deletes skill and returns 204. | +| POST | `/api/sessions` | Creates standard, custom, or override sessions. | +| GET | `/api/sessions/{session_id}/history` | Exports backend session state and override metadata. | +| POST | `/api/sessions/{session_id}/messages` | Streams SSE response events for text/multipart messages. | +| DELETE | `/api/sessions/{session_id}` | Cleans up MCP connections and session usage, returns 204. | + +## Session Response Shape + +Required for all successful session creation responses: + +```json +{ + "session_id": "uuid", + "profile_id": "profile-key-or-custom", + "profile_name": "Display Name", + "tools_loaded": [], + "skills_loaded": [], + "search_context": false, + "mcp_results": [] +} +``` + +Built-in override responses also preserve: + +```json +{ + "used_profile_override": true, + "override_updated_at": "2026-05-06T12:00:00.000Z" +} +``` + +## Verification + +- `uv run pytest` must pass. +- Existing route tests must continue to pass without frontend changes. +- New unit tests should cover extracted validators, skill manager, streaming helpers, and session orchestration. +- `git diff -- config/agents.yaml` must be empty. \ No newline at end of file diff --git a/specs/007-application-refactor/contracts/frontend-contract.md b/specs/007-application-refactor/contracts/frontend-contract.md new file mode 100644 index 0000000..24eaa87 --- /dev/null +++ b/specs/007-application-refactor/contracts/frontend-contract.md @@ -0,0 +1,62 @@ +# Frontend Contract: Behavior-Preserving Refactor + +This refactor may move frontend implementation details but must preserve exported APIs, storage data, and user workflows. + +## API Client Exports + +The following exports from `frontend/src/api/client.ts` must remain available with compatible signatures and return shapes: + +- `AuthError` +- `fetchTools` +- `fetchSkills` +- `fetchSkill` +- `createSkill` +- `updateSkill` +- `deleteSkill` +- `generateSkillContent` +- `testMcpConnections` +- `createCustomSession` +- `fetchBuiltInProfileDefinition` +- `createSessionWithProfileOverride` +- `fetchProfiles` +- `createSession` +- `deleteSession` +- `fetchHistory` +- `createSessionWithHistory` +- `sendMessage` + +Internal helpers may move to `frontend/src/api/helpers.ts`, but callers should not need to change except imports inside the API layer. + +## Hook And Component Contracts + +- `useChat()` must return the same `ChatState` fields and functions. +- `useConversationStore()` must preserve `loadIndex`, `saveConversation`, `loadConversation`, `deleteConversation`, and `deleteConversationsByCustomAgent` semantics. +- `useCustomAgents()` and `useBuiltInAgentCustomizations()` must preserve `save`/`remove` behavior and stored data shapes. +- `ChatPage`, `AgentBuilder`, `SkillBuilder`, and `AdminPage` exports must remain available. + +## Storage Contract + +Existing localStorage keys and data shapes must be preserved: + +- `auth_token` +- `webagents_user_profile` +- `webagents_custom_agents` +- `webagents_builtin_agent_customizations` +- `webagents_conversation_index` +- `webagents_conversation_{id}` +- theme storage key currently owned by `useTheme` + +Corrupt or invalid stored values must continue to fail safely without blocking the app. + +## Visual Contract + +- The current visual design remains the target. +- CSS class names used by rendered components should remain stable unless all usages are migrated in the same change. +- UI changes require `npm run build`, server startup, and Playwright screenshots for affected screens. + +## Verification + +- `npm test` must pass. +- `npm run build` must pass. +- `npm run lint` should pass for refactored frontend code. +- Visual verification must include chat/profile selection plus admin builder and skill builder screens when those areas change. \ No newline at end of file diff --git a/specs/007-application-refactor/data-model.md b/specs/007-application-refactor/data-model.md new file mode 100644 index 0000000..183f52e --- /dev/null +++ b/specs/007-application-refactor/data-model.md @@ -0,0 +1,151 @@ +# Data Model: Application Refactor And Deduplication + +This feature preserves existing runtime data shapes. The entities below describe the contracts and state that refactor tasks must protect. + +## Session Creation Request + +**Purpose**: Represents the incoming payload for `POST /api/sessions`. + +**Fields**: + +- `profile_id`: string; either a configured profile key/display name or `custom`. +- `custom_name`: string; required for custom agents, max 100 characters. +- `custom_prompt`: string; required for custom agents and built-in overrides, max `MAX_USER_INPUT_CHARS`. +- `custom_tools`: string array; validated against known backend tools. +- `custom_skills`: string array; validated against discovered skill names. +- `custom_search_context`: boolean; enables search context for custom/override flows. +- `custom_temperature`: optional number; valid range 0.0 through 2.0. +- `mcp_servers`: MCP server entries; custom and override flows support only HTTP entries from request body. +- `profile_override`: object; built-in profile customization fields. +- `history`: optional backend session state for resume. +- `user_profile`: optional `{ name, preferences, notes }` context for user profile tools. + +**Validation Rules**: + +- Unknown profile IDs are rejected. +- Built-in profile overrides cannot change the canonical agent name. +- Unknown tools and skills are rejected before runtime creation. +- Inline MCP server entries must be objects with name, HTTP transport, and URL. +- `config/agents.yaml` remains read-only; profile definitions are loaded, not modified. + +## Session Creation Result + +**Purpose**: Represents the `POST /api/sessions` success response and frontend session metadata. + +**Fields**: + +- `session_id`: generated UUID string. +- `profile_id`: logical profile key or `custom`. +- `profile_name`: display name shown to the user. +- `tools_loaded`: string array. +- `skills_loaded`: string array. +- `search_context`: boolean. +- `mcp_results`: array of per-server connection outcomes. +- `used_profile_override`: optional boolean for built-in override sessions. +- `override_updated_at`: optional ISO timestamp for built-in override sessions. + +**Relationships**: + +- Created from a Session Creation Request. +- Stored in backend `_sessions` and consumed by frontend `useChat`. +- Included in conversation persistence through history export. + +## Stream Event + +**Purpose**: Represents an SSE message emitted by `POST /api/sessions/{session_id}/messages`. + +**Types**: + +- `text`: assistant text delta. +- `function_call`: tool call with `call_id`, `name`, and accumulated `arguments`. +- `function_result`: tool result with optional `content_items` for images. +- `usage`: token usage counts. +- `error`: user-displayable error plus optional retry hint. +- `done`: stream completion marker. + +**Validation Rules**: + +- Event names and payload field names must remain stable. +- Tool-result image items must preserve supported MIME filtering. +- Usage counts must continue to aggregate into session usage. + +## Stored Conversation + +**Purpose**: Represents frontend localStorage data for saved/resumable conversations. + +**Fields**: + +- `id`: conversation ID, usually session ID. +- `profileId`, `profileName`: associated agent profile. +- `description`: first user message summary. +- `createdAt`, `lastActivityAt`: ISO timestamps. +- `sessionData`: backend `AgentSession.to_dict()` payload. +- `customAgentId`: optional ID for custom agent sessions. +- `usedBuiltInOverride`, `baseProfileId`, `overrideUpdatedAt`: optional built-in override metadata. + +**Validation Rules**: + +- Existing localStorage keys and stored shapes must not change. +- Corrupt records are discarded or ignored as today. +- Conversation index remains sorted newest-first and capped by `__MAX_SESSIONS__`. + +## Agent Builder Form State + +**Purpose**: Represents editable frontend state for custom agents and built-in override customizations. + +**Fields**: + +- `name`, `description`, `systemPrompt`, `icon`. +- `tools`, `skills`, `mcpServers`. +- `useSearchContext`, `temperature`. +- `starters` with label/message pairs. + +**State Transitions**: + +- Empty form -> create custom agent. +- Saved custom agent -> edit/delete. +- Built-in profile definition -> local override -> reset override. +- Override -> generated standard-agent candidate YAML. + +**Validation Rules**: + +- Existing UI validation behavior remains unchanged. +- Built-in profile name remains read-only. +- MCP test results map by server name and are cleared when relevant inputs are reset. + +## Skill Definition + +**Purpose**: Represents a file-backed skill exposed through admin CRUD endpoints. + +**Fields**: + +- `name`: lowercase alphanumeric plus hyphen, starts with alphanumeric, max 64 characters. +- `description`: non-empty, max 256 characters. +- `content`: non-empty Markdown, max 65,536 characters. + +**Relationships**: + +- Stored as `skills/{name}/SKILL.md`. +- Listed by backend skill endpoints and selected by the agent builder. + +**Validation Rules**: + +- Path traversal protections remain in place. +- Duplicate creates return conflict. +- Missing skill reads/updates/deletes return not found. + +## Visual Style Primitive + +**Purpose**: Represents shared CSS patterns extracted from repeated app styles. + +**Fields/Patterns**: + +- Button variants: primary, secondary/cancel, icon/action, disabled. +- Input variants: text input, textarea, invalid state. +- Panel/list primitives: admin list panel, saved entry, form panel. +- Message markdown primitives: headings, paragraphs, code, tables, blockquotes, links. + +**Validation Rules**: + +- Existing class names used by components should remain stable unless all usages and screenshots are updated. +- Visual verification is required before CSS cleanup is considered complete. \ No newline at end of file diff --git a/specs/007-application-refactor/plan.md b/specs/007-application-refactor/plan.md new file mode 100644 index 0000000..2aedb62 --- /dev/null +++ b/specs/007-application-refactor/plan.md @@ -0,0 +1,129 @@ +# Implementation Plan: Application Refactor And Deduplication + +**Branch**: `007-application-refactor` | **Date**: 2026-05-06 | **Spec**: [spec.md](spec.md) +**Input**: Feature specification from `/specs/007-application-refactor/spec.md` + +## Summary + +Refactor the existing FastAPI plus React application to remove duplicated backend session orchestration, frontend API/storage/content helpers, oversized admin/chat state, dead or repeated CSS, and brittle test setup while preserving current API routes, SSE event names, local storage keys, user-visible UI behavior, and deployment shape. The plan explicitly excludes edits to `config/agents.yaml`. + +## Follow-Up Wrapper Flattening Plan + +The first implementation pass successfully moved behavior out of oversized files, but the extraction introduced several layers that now obscure the session story. The follow-up cleanup should reduce wrapper code rather than create more helper surfaces. + +### Goals + +- Make the session creation path readable in one pass: route request -> validated session intent -> chat runtime -> stored session response. +- Collapse pass-through agent factory functions so there is one obvious place where the SDK agent is built. +- Replace frontend session API variants with one typed session creation request while preserving existing exported API functions during migration. +- Delete compatibility aliases, one-method classes, and setter-bag hooks when they no longer represent a useful domain boundary. +- Keep the completed behavior-preserving contract intact: routes, SSE events, localStorage keys, frontend exports, and `config/agents.yaml` remain unchanged. + +### Targeted Cleanup Areas + +| Area | Current Smell | Preferred Shape | +|------|---------------|-----------------| +| `agent_factory.py` | `create_chat_runtime()` -> `spawn_agent()` -> `_create_agent()` hides the actual SDK construction | `create_chat_runtime()` calls a single clear `build_agent()`/inline builder and owns runtime construction | +| `session_orchestration.py` | Per-request `SessionCreationService(...)` class wraps procedural session creation | Plain request-oriented functions or a module-level service with fewer constructor dependencies | +| `frontend/src/api/client.ts` | Four wrappers post to `/api/sessions` with body variants | One internal `postSession()`/`createSessionRequest()` plus compatibility exports | +| `frontend/src/hooks/useSessionLifecycle.ts` | `startChatSession()` mirrors backend mode branching and calls many API wrappers | Build one typed session request payload, then call one API function | +| `validators.py` | `ToolRegistry` is a one-method wrapper over known tool validation | Direct `known_tool_names_from_profiles()` + `validate_tool_names()` usage | +| `streaming.py` | Underscore compatibility aliases duplicate exported helper names | Import/use canonical helper names only | +| frontend form hooks | Some hooks mostly expose raw state setters | Keep hooks only where they own workflow decisions; inline or rename setter bags | + +### Non-Goals + +- Do not edit `config/agents.yaml`. +- Do not change API routes, response field names, status codes, SSE event names, localStorage keys, or generated frontend public exports. +- Do not add backend or frontend dependencies. +- Do not start a backend package-layout migration in this follow-up slice. + +## Technical Context + +**Language/Version**: Python 3.12.6, TypeScript 5.9, React 19 +**Primary Dependencies**: FastAPI, agent-framework-core/openai/azure-ai-search, Azure SDKs, React, Vite, react-markdown, MSAL +**Storage**: In-memory backend sessions, file-backed `skills/`, browser localStorage, Terraform-managed Azure App Service resources +**Testing**: `uv run pytest`, `npm test`, `npm run build`, `npm run lint`, Playwright screenshot verification for visual changes +**Target Platform**: Linux development environment and Azure App Service single deployment serving FastAPI plus built React SPA +**Project Type**: Two-tier web application in one repository and one deployment unit +**Performance Goals**: No user-visible latency regression; preserve streaming behavior and session cleanup while reducing maintainability cost +**Constraints**: Do not edit `config/agents.yaml`; no new runtime dependencies unless justified; preserve API/SSE/storage contracts; backend package manager is `uv` only; frontend package manager is `npm`; visual verification is mandatory for CSS/UI changes +**Scale/Scope**: Refactor about 2.9k backend root Python lines, 4.3k frontend TypeScript/TSX lines, 3.0k frontend CSS lines, and 1.4k backend test lines; target at least 1,000 net runtime-line reduction + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +**Gate Status**: PASS + +- **Read-Only Data Access**: PASS. No SQL tool behavior or database write capability is added. Existing read-only guarantees must remain covered by tests. +- **Single-File Agent Definitions**: PASS. `config/agents.yaml` is read-only and out of scope for every refactor task. +- **Security & Credential Hygiene**: PASS. Refactor preserves backend-only access to LLM, search, MCP, auth, and secrets; API responses and logs must not expose credentials or bearer tokens. +- **Evaluation-Driven Quality**: PASS. No prompt, model, agent profile, or parameter behavior changes are planned. Evaluation pipeline is not required unless implementation later changes prompts or model settings. +- **Simplicity & Minimalism**: PASS. New code is limited to focused helper/service modules that remove real duplication. No new dependencies are planned. +- **Infrastructure as Code**: PASS. No infrastructure changes are planned. Existing Terraform deployment remains unchanged. +- **Two-Tier API-First Architecture**: PASS. FastAPI remains the sole backend gateway and React remains presentation/UI. No frontend direct Azure calls are introduced. +- **Visual Verification Protocol**: PASS WITH ACTION. CSS or component markup changes must complete build plus Playwright screenshot verification before implementation is considered done. + +## Project Structure + +### Documentation (this feature) + +```text +specs/007-application-refactor/ +├── plan.md # This file (/speckit.plan command output) +├── research.md # Phase 0 output (/speckit.plan command) +├── data-model.md # Phase 1 output (/speckit.plan command) +├── quickstart.md # Phase 1 output (/speckit.plan command) +├── contracts/ # Phase 1 output (/speckit.plan command) +└── tasks.md # Phase 2 output (/speckit.tasks command - NOT created by /speckit.plan) +``` + +### Source Code (repository root) + +```text +. +├── main.py # Keep FastAPI app/routes; reduce orchestration +├── agent_factory.py # Reuse existing runtime creation machinery +├── mcp_servers.py # Reuse existing MCP parsing/connection helpers +├── session_orchestration.py # New: shared session creation flow +├── validators.py # New: tool/skill/input/image/error validation helpers +├── skills_manager.py # New: file-backed skill CRUD/path safety +├── streaming.py # New: SSE event, usage, and content conversion helpers +├── tests/ +│ ├── conftest.py # Shared env/client/session fixtures +│ ├── test_api.py # Existing route contract tests +│ ├── test_session_orchestration.py +│ ├── test_validators.py +│ ├── test_skills_manager.py +│ └── test_streaming.py +└── frontend/ + └── src/ + ├── api/ + │ ├── client.ts # Preserve exported API functions + │ └── helpers.ts # New: authenticated fetch/response handling + ├── components/ # Extract reusable admin/image subcomponents + ├── hooks/ # Extract chat lifecycle/form-state hooks + ├── pages/ # Keep page exports stable + ├── styles/index.css # Consolidate app styles + └── utils/ + ├── storage.ts # New: typed localStorage helpers + └── content.ts # New: image/content filtering and formatting +``` + +**Structure Decision**: Preserve the current root-backend plus `frontend/` layout required by the constitution. Add small backend modules beside `main.py` rather than moving the backend into a new package during this refactor, because that gives the line-reduction and duplication benefits with less import and deployment risk. + +## Complexity Tracking + +No constitution violations identified. + +## Post-Design Constitution Check + +**Gate Status**: PASS + +- Design preserves read-only SQL posture and does not add data mutation capabilities. +- Design keeps `config/agents.yaml` unchanged and explicitly verifies it remains diff-free. +- Design preserves backend-only access to secrets and Azure services. +- Design avoids new dependencies and uses current `uv`, `npm`, pytest, Vite, and Playwright tooling. +- Design preserves the two-tier FastAPI plus React deployment model. +- Design includes visual verification for CSS/component appearance changes. diff --git a/specs/007-application-refactor/quickstart.md b/specs/007-application-refactor/quickstart.md new file mode 100644 index 0000000..2777e6d --- /dev/null +++ b/specs/007-application-refactor/quickstart.md @@ -0,0 +1,372 @@ +# Quickstart: Application Refactor And Deduplication + +## Scope Guard + +Before starting implementation, confirm the branch and the protected config file: + +```bash +git branch --show-current +git diff -- config/agents.yaml +``` + +Expected branch: `007-application-refactor`. + +Expected `config/agents.yaml` diff: empty. + +## Recommended Implementation Order + +1. Extract backend validators and unit tests. +2. Extract backend streaming/usage/error helpers and unit tests. +3. Extract backend skill manager and unit tests. +4. Extract backend session orchestration and keep route contracts stable. +5. Add frontend storage/content/API helpers while preserving exports. +6. Split `useChat`, `AgentBuilder`, and `SkillBuilder` internals by responsibility. +7. Clean CSS last and run visual verification. +8. Replace brittle source-inspection tests where behavior can be asserted. +9. Flatten wrapper layers introduced or exposed by the first pass: agent factory construction, session orchestration, frontend session API variants, compatibility aliases, and state-only hooks. + +## Wrapper Flattening Follow-Up + +Focus this cleanup on deleting or collapsing indirection. Prefer a direct function with a precise name over a class or wrapper whose only job is to forward arguments. + +Recommended order: + +1. Collapse `spawn_agent()` and `_create_agent()` in `agent_factory.py` into one clear agent builder used by `create_chat_runtime()`. +2. Replace per-request `SessionCreationService(...)` construction with clearer session creation functions or a module-level service boundary in `session_orchestration.py` and `main.py`. +3. Centralize frontend `/api/sessions` request posting in `frontend/src/api/client.ts`, keeping current exported functions as compatibility shims if needed. +4. Simplify `startChatSession()` in `frontend/src/hooks/useSessionLifecycle.ts` so it builds one session intent/request instead of mirroring every backend branch through separate client functions. +5. Remove `ToolRegistry` in `validators.py` and use direct tool-name validation helpers. +6. Remove underscore compatibility exports in `streaming.py` after `main.py` and tests import canonical helper names. +7. Revisit frontend hooks that mostly return setters, especially `useSkillForm`, `useAgentBuilderForm`, and `useAgentMcpEditor`, and either give them workflow ownership or inline the state back into the owning component. + +### Phase 7 Starting Call Chains - 2026-05-07 + +Current backend standard session path before wrapper flattening: + +```text +main.create_session +-> SessionCreationService(...).create +-> SessionCreationService._create_profile_session +-> create_chat_runtime +-> spawn_agent +-> _create_agent +-> OpenAIChatClient.as_agent +-> agent.create_session +``` + +Current backend custom session path before wrapper flattening: + +```text +main.create_session +-> SessionCreationService(...).create +-> SessionCreationService._create_custom_session +-> create_chat_runtime +-> spawn_agent +-> _create_agent +-> OpenAIChatClient.as_agent +-> agent.create_session +``` + +Current backend built-in override path before wrapper flattening: + +```text +main.create_session +-> SessionCreationService(...).create +-> SessionCreationService._create_profile_session +-> ProfileOverrideRequest parsing +-> create_chat_runtime(custom_name=profile_name, custom_instructions=override prompt) +-> spawn_agent +-> _create_agent +-> OpenAIChatClient.as_agent +-> agent.create_session +``` + +Current frontend session start path before wrapper flattening: + +```text +useChat.startSession +-> startChatSession +-> createCustomSession | createSessionWithProfileOverride | createSessionWithHistory | createSession +-> fetch POST /api/sessions +``` + +### Phase 7 Wrapper Flattening Results - 2026-05-07 + +Backend standard/custom session creation now follows a shorter path: + +```text +main.create_session +-> create_chat_session +-> _create_profile_chat_session | _create_custom_chat_session +-> create_chat_runtime +-> OpenAIChatClient.as_agent +-> agent.create_session +``` + +Built-in override session creation follows the same profile branch, with `ProfileOverrideRequest` parsing inside `_create_profile_chat_session` before `create_chat_runtime(custom_name=profile_name, custom_instructions=override prompt)`. + +Frontend session startup now follows one request-building path: + +```text +useChat.startSession +-> startChatSession +-> buildSessionRequest +-> createSessionRequest +-> fetch POST /api/sessions +``` + +Wrapper reduction: + +| Path | Before | After | Result | +|------|--------|-------|--------| +| Backend standard/custom | 8 steps, including `SessionCreationService(...).create`, `spawn_agent`, and `_create_agent` | 6 steps, with direct `create_chat_session` and `OpenAIChatClient.as_agent` construction | SC-007 passed: 2 fewer internal wrapper layers | +| Backend built-in override | 9 steps, including service, spawn, and `_create_agent` wrappers | 7 steps, with override parsing inside the profile branch and direct SDK agent construction | SC-007 passed: 2 fewer internal wrapper layers | +| Frontend session startup | `startChatSession` selected one of four `/api/sessions` wrapper exports | `startChatSession` builds one `SessionRequestPayload` and calls `createSessionRequest` | One network helper owns session POST behavior | + +Line-count/accounting snapshot after Phase 7: + +| Metric | Phase 6 Current | Phase 7 Current | Result | +|--------|-----------------|-----------------|--------| +| Runtime app code, same inclusion rules as Phase 6 | 10528 | 10524 | -4 net lines | +| `main.py` | 833 | 832 | -1 | +| `agent_factory.py` | 376 before wrapper flattening | 262 | -114 | +| `validators.py` | 167 | 155 | -12 | +| `streaming.py` | 275 | 258 | -17 | +| `frontend/src/api/client.ts` | 399 | 422 | +23; central helper added while preserving compatibility exports | +| `frontend/src/hooks/useSessionLifecycle.ts` | 211 before wrapper flattening | 250 | +39; one request builder replaces branch-specific API calls | + +Hook reassessment: `useSkillForm`, `useAgentBuilderForm`, and `useAgentMcpEditor` each still own validation, reset, save-preparation, MCP mutation, or test-result workflow state, so they were left in place rather than inlined. + +Validation: + +- Behavior pinning: `uv run pytest tests/test_api.py tests/test_provider_routing.py` -> 20 passed, 1 warning +- Focused backend wrapper validation: `uv run pytest tests/test_api.py tests/test_session_orchestration.py tests/test_provider_routing.py tests/test_retry_logic.py tests/test_streaming.py tests/test_validators.py` -> 67 passed, 1 warning +- Frontend wrapper validation: `cd frontend && npm test && npm run build && npm run lint` -> passed; Vite reported the existing chunk-size warning +- Final backend gate: `uv run pytest` -> 149 passed, 1 warning +- Final frontend gate: `cd frontend && npm test && npm run build && npm run lint` -> passed; Vite reported the existing chunk-size warning +- Protected config check: `git diff -- config/agents.yaml` produced no output + +## Backend Verification + +Use `uv` only for Python commands: + +```bash +uv run pytest +``` + +Recommended focused runs during implementation: + +```bash +uv run pytest tests/test_api.py +uv run pytest tests/test_image_input.py tests/test_retry_logic.py +uv run pytest tests/test_skills_api.py tests/test_skills.py +``` + +## Frontend Verification + +```bash +cd frontend +npm test +npm run build +npm run lint +``` + +If lint currently reports pre-existing issues, document them in the implementation report and ensure refactored files do not add new issues. + +## Visual Verification For UI/CSS Changes + +1. Build the frontend: + + ```bash + cd frontend + npm run build + ``` + +2. Start the backend serving built assets: + + ```bash + AUTH_DISABLED=true uv run uvicorn main:app --host 0.0.0.0 --port 8000 + ``` + +3. Capture affected screens with Playwright. At minimum, capture the disclaimer or warning overlay scrolled to top and bottom, profile or mission selection, empty chat with starter questions, chat with one user message and one assistant response, and every screen specifically affected by the current change. Reuse the existing admin capture helper where applicable: + + ```bash + uv run python scripts/capture_admin_agent_screenshots.py --base-url http://localhost:8000 + ``` + +4. View each generated screenshot and verify no clipping, overflow, overlap, broken assets, missing content, unreadable text, or unintended design changes. For responsive changes, repeat captures at 1440x900, 768x1024, and 360x640. + +## Contract Checks + +After each major refactor step: + +```bash +git diff -- config/agents.yaml +uv run pytest +cd frontend && npm test && npm run build +``` + +`config/agents.yaml` must remain empty in the diff for the entire feature. + +## Implementation Log + +### T001 Scope Guard - 2026-05-06 + +- Branch check: `007-application-refactor` +- Protected config check: `git diff -- config/agents.yaml` produced no output +- Reusable protected-file guard: run `git diff -- config/agents.yaml` after every phase and before final delivery; expected output is empty + +### T002 Backend Baseline - 2026-05-06 + +- Command: `uv run pytest` +- Result: 128 passed, 1 warning in 3.39s +- Warning: existing `SkillsProvider` experimental warning from `agent_factory.py` + +### T003 Frontend Baseline - 2026-05-06 + +- Added no-new-dependency frontend test script: `tsc -b --pretty false` +- Command: `cd frontend && npm test && npm run build && npm run lint` +- Result: passed +- Note: Vite reported an existing chunk-size warning for the production bundle + +### T004 Line-Count Baseline - 2026-05-06 + +| File | Lines | +|------|-------| +| `main.py` | 1643 | +| `frontend/src/api/client.ts` | 477 | +| `frontend/src/hooks/useChat.ts` | 569 | +| `frontend/src/pages/AgentBuilder.tsx` | 735 | +| `frontend/src/components/SkillBuilder.tsx` | 369 | +| `frontend/src/styles/index.css` | 2707 | +| `frontend/src/index.css` | 107 | +| `frontend/src/App.css` | 184 | +| **Total** | **6791** | + +### Phase 2 Fixture Validation - 2026-05-06 + +- Command: `uv run pytest tests/test_api.py tests/test_skills_api.py` +- Result: 31 passed, 1 warning in 0.96s +- Warning: existing `SkillsProvider` experimental warning from `main.py` + +### US1 Helper Test Baseline - 2026-05-06 + +- Command: `uv run pytest tests/test_validators.py tests/test_streaming.py tests/test_skills_manager.py tests/test_session_orchestration.py` +- Result: 18 passed, 1 warning in 0.72s +- Warning: existing `SkillsProvider` experimental warning from `validators.py` + +### US1 Helper Wiring Validation - 2026-05-06 + +- Command: `uv run pytest tests/test_api.py tests/test_image_input.py tests/test_retry_logic.py tests/test_skills_api.py tests/test_skills.py tests/test_validators.py tests/test_streaming.py tests/test_skills_manager.py tests/test_session_orchestration.py` +- Result: 126 passed, 1 warning in 3.21s +- Warning: existing `SkillsProvider` experimental warning from `skills_manager.py` + +### US1 Session Extraction Validation - 2026-05-06 + +- Command: `uv run pytest tests/test_api.py tests/test_image_input.py tests/test_retry_logic.py tests/test_skills_api.py tests/test_skills.py tests/test_validators.py tests/test_streaming.py tests/test_skills_manager.py tests/test_session_orchestration.py` +- Result: 126 passed, 1 warning in 3.19s +- `main.py` line count: 833 +- Extracted backend module total: validators.py 167, streaming.py 275, skills_manager.py 111, session_orchestration.py 341 +- Protected config check: `git diff -- config/agents.yaml` produced no output +- Delete-session and lifespan cleanup behavior required no code changes after session creation extraction + +### US2 API/Storage/Content Helper Validation - 2026-05-06 + +- Added compile-only API export matrix in `frontend/src/api/client.contract.ts` for all public exports from `frontend/src/api/client.ts` +- Preserved localStorage keys: `auth_token`, `webagents_user_profile`, `webagents_custom_agents`, `webagents_builtin_agent_customizations`, `webagents_conversation_index`, and `webagents_conversation_{id}` +- Storage compatibility notes: custom agents keep array persistence; built-in overrides continue filtering invalid entries; conversation index and per-conversation entries still fail safely on corrupt data and remove invalid stored entries where existing behavior did so +- Frontend workflow verification scope for the full US2 checkpoint: chat send, saved conversation resume/delete, agent builder save/delete, and skill builder create/edit/delete/generate should be manually exercised after the pending `useChat`, `AgentBuilder`, and `SkillBuilder` splits +- Command: `cd frontend && npm test && npm run build && npm run lint` +- Result: passed +- Note: Vite reported the existing production bundle chunk-size warning + +### US2 useChat Split Validation - 2026-05-06 + +- Extracted session startup, cleanup, restored-message parsing, and framework content conversion into `frontend/src/hooks/useSessionLifecycle.ts` +- Extracted saved-conversation persistence into `frontend/src/hooks/useConversationPersistence.ts` +- Preserved `useChat` return shape: messages, streaming state, session details, loaded capabilities, errors, conversation id, save counter, and action callbacks +- Command: `cd frontend && npm test && npm run build && npm run lint` +- Result: passed +- Note: Vite reported the existing production bundle chunk-size warning + +### US2 AgentBuilder Split Validation - 2026-05-06 + +- Extracted MCP editor state and connection testing into `frontend/src/hooks/useAgentMcpEditor.ts` +- Extracted agent form state, validation, reset state, and save preparation helpers into `frontend/src/hooks/useAgentBuilderForm.ts` +- Extracted tools/skills picker UI into `frontend/src/components/AgentCapabilityPicker.tsx` +- Extracted starter question editor UI into `frontend/src/components/StarterQuestionEditor.tsx` +- Command: `cd frontend && npm test && npm run build && npm run lint` +- Result: passed +- Note: Vite reported the existing production bundle chunk-size warning + +### US2 SkillBuilder Split Validation - 2026-05-06 + +- Extracted reusable Skill Builder form state, validation, feedback state, delete confirmation state, AI generation state, and create payload preparation into `frontend/src/hooks/useSkillForm.ts` +- Preserved Skill Builder create/edit/delete/generate handlers in `frontend/src/components/SkillBuilder.tsx` +- Command: `cd frontend && npm test && npm run build && npm run lint` +- Result: passed +- Note: Vite reported the existing production bundle chunk-size warning + +### US3 Behavioral Test Cleanup - 2026-05-06 + +- Replaced retry/session source-inspection assertions with behavior-level checks for session deletion, final usage logging, usage accumulation, and long input rejection +- Reviewed image input tests; existing coverage already exercises validators behaviorally, so no source-inspection replacement was needed there +- Command: `uv run pytest tests/test_image_input.py tests/test_retry_logic.py` +- Result: 71 passed + +### US3 Visual Baseline Capture - 2026-05-06 + +- Expanded `scripts/capture_admin_agent_screenshots.py` to capture disclaimer top/bottom, profile selection, empty chat with starter questions, one-turn chat, admin agent builder, and skill builder states +- Added `--mock-api` for deterministic screenshot captures without live LLM or backend data dependencies +- Command: `uv run python scripts/capture_admin_agent_screenshots.py --base-url http://localhost:8000 --output-dir screenshots/007-application-refactor --mock-api` +- Result: passed; screenshots saved under `screenshots/007-application-refactor/` +- Reviewed generated screenshots: disclaimer top/bottom, profile selection, empty chat, one-turn chat, admin agent builder expanded/collapsed states, and skill builder desktop/tablet/mobile showed no obvious clipping, overlap, broken assets, or missing required content + +### US3 CSS Cleanup Validation - 2026-05-06 + +- Removed unused Vite demo stylesheet content from `frontend/src/App.css` and trimmed `frontend/src/index.css` to root/body rendering concerns +- Consolidated repeated frontend button, input, textarea, saved action, message image, and tool image styles in `frontend/src/styles/index.css`; no component class-name changes were required +- Removed unused CSS selectors and stale section comments after source usage scanning +- CSS line count: baseline 2998 lines (`App.css` 184, `index.css` 107, `styles/index.css` 2707) to 2575 lines (`index.css` 18, `styles/index.css` 2557), net -423 lines +- Command: `cd frontend && npm test && npm run build && npm run lint` +- Result: passed +- Screenshot command: `uv run python scripts/capture_admin_agent_screenshots.py --base-url http://localhost:8000 --output-dir screenshots/007-application-refactor --mock-api` +- Screenshot result: passed; reviewed final disclaimer, profile selection, one-turn chat, admin builder, and skill builder mobile captures with no obvious clipping, overflow, overlap, broken assets, or missing content + +### Phase 6 Final Backend Gate - 2026-05-06 + +- Command: `uv run pytest` +- Result: 146 passed, 1 warning in 6.64s +- Warning: existing `SkillsProvider` experimental warning from `validators.py` + +### Phase 6 Final Frontend Gate - 2026-05-06 + +- Command: `cd frontend && npm test && npm run build && npm run lint` +- Result: passed +- Build output included `dist/assets/index-BE5OHoTD.css` at 39.59 kB and `dist/assets/index-vlPAe1QI.js` at 633.82 kB +- Note: Vite reported the existing production bundle chunk-size warning + +### Phase 6 Protected File And Dependency Check - 2026-05-06 + +- Protected config check: `git diff -- config/agents.yaml` produced no output +- Dependency manifest check: only `frontend/package.json` changed, for the intentional no-new-dependency `test` script; no package dependency additions were made +- Ignore-file verification: `.gitignore`, `.terraformignore`, and `frontend/eslint.config.js` cover detected Python, Node/TypeScript, Terraform, and ESLint artifacts; no Docker, Prettier, Helm, or publishing npm ignore file was required by the detected project setup + +### Phase 6 Line-Count And Success Criteria Check - 2026-05-06 + +| Metric | Baseline | Current | Result | +|--------|----------|---------|--------| +| Runtime app code, same inclusion rules as `main` comparison | 10534 | 10528 | -6 net lines; SC-001 not met as a strict all-runtime net metric because extracted helper modules intentionally offset large-file reductions | +| Targeted original refactor files | 6791 | 5052 | -1739 lines before counting extracted helper modules | +| `main.py` | 1643 | 833 | -810 lines; SC-002 passed | +| Frontend CSS (`App.css`, `index.css`, `styles/index.css`) | 2998 | 2575 | -423 lines; SC-003 passed | +| Frontend API client | 477 | 399 | -78 lines; SC-004 passed | + +- Current extracted helper/module total: 1767 lines across `validators.py`, `streaming.py`, `skills_manager.py`, `session_orchestration.py`, `frontend/src/api/helpers.ts`, `frontend/src/utils/storage.ts`, `frontend/src/utils/content.ts`, `frontend/src/hooks/useSessionLifecycle.ts`, `frontend/src/hooks/useConversationPersistence.ts`, `frontend/src/hooks/useAgentMcpEditor.ts`, `frontend/src/hooks/useAgentBuilderForm.ts`, `frontend/src/hooks/useSkillForm.ts`, `frontend/src/components/AgentCapabilityPicker.tsx`, and `frontend/src/components/StarterQuestionEditor.tsx` +- Targeted original plus extracted helper/module total: 6819 lines before final dead-helper cleanup, compared with the 6791-line targeted baseline + +### Phase 6 Contract Review - 2026-05-06 + +- API contract review: preserved route paths, response fields, auth behavior, SSE event names (`text`, `function_call`, `function_result`, `usage`, `error`, `done`), session response shape, history export behavior, and empty `config/agents.yaml` diff +- Frontend contract review: preserved public `frontend/src/api/client.ts` exports, `useChat()` return shape, localStorage keys/data shapes, admin builder and skill builder exports/workflows, and visual verification coverage for changed chat/admin/skill screens +- Final cleanup: removed an unused frontend API helper and added missing critical ignore patterns without changing runtime behavior \ No newline at end of file diff --git a/specs/007-application-refactor/research.md b/specs/007-application-refactor/research.md new file mode 100644 index 0000000..b27a1d8 --- /dev/null +++ b/specs/007-application-refactor/research.md @@ -0,0 +1,73 @@ +# Research: Application Refactor And Deduplication + +## Decision: Keep `config/agents.yaml` Read-Only + +**Rationale**: The user explicitly excluded `agents.yaml`, and the constitution requires a single file for agent profiles. Refactor value can be achieved in code, CSS, tests, and planning artifacts without changing prompt/profile configuration. + +**Alternatives considered**: YAML anchors or prompt fragment extraction could reduce configuration duplication, but those options modify `config/agents.yaml` and are out of scope. + +## Decision: Preserve Route Contracts And Extract Backend Services Around Them + +**Rationale**: `main.py` contains route definitions plus session creation, validation, skills CRUD, streaming conversion, token usage, and error classification. The safest refactor is to keep FastAPI routes and response shapes stable while moving repeated behavior into focused modules: `session_orchestration.py`, `validators.py`, `skills_manager.py`, and `streaming.py`. + +**Alternatives considered**: Moving the backend into a package or creating a larger service framework would create unnecessary import/deployment churn for this feature. Keeping everything in `main.py` would preserve behavior but fail the simplification goal. + +## Decision: Consolidate Tool, Skill, Input, Image, And Error Validation + +**Rationale**: Tool and skill discovery/validation is repeated across discovery endpoints and session creation branches. Image validation and retry/context error classification are also currently embedded in `main.py`. Extracting these helpers makes behavior testable and reduces duplicated loops and constants. + +**Alternatives considered**: A generic validation framework was rejected as overbuilt. Small typed helper functions/classes are enough. + +## Decision: Extract Session Creation Last Among Backend Changes + +**Rationale**: Session creation is the highest-risk area because it handles custom agents, built-in overrides, MCP connection results, history restoration, user profile injection, and runtime creation. Validators, skills manager, and streaming helpers should be extracted first so the final session extraction is smaller and easier to verify. + +**Alternatives considered**: Extracting session orchestration first would produce a large behavior-preserving diff and make failures harder to localize. + +## Decision: Preserve Frontend API Exports And Add Internal Request Helpers + +**Rationale**: `frontend/src/api/client.ts` repeats authenticated fetch, unauthorized handling, HTTP error handling, JSON body setup, and toast emission. A new internal `frontend/src/api/helpers.ts` can reduce boilerplate while preserving existing exported functions and return types. + +**Alternatives considered**: Generating a full API client from OpenAPI is constitutionally preferred long term, but this refactor should avoid new tooling and keep scope focused. + +## Decision: Centralize localStorage Helpers Without Changing Keys Or Shapes + +**Rationale**: Custom agents, built-in overrides, user profile, conversations, auth token, and theme state all rely on localStorage with repeated parse/stringify/corruption handling. A `frontend/src/utils/storage.ts` helper can standardize failure handling while preserving storage keys and data shapes. + +**Alternatives considered**: Migrating to IndexedDB or a state management library was rejected because it adds dependencies and changes persistence behavior. + +## Decision: Extract Content/Image Helpers And Reusable Image UI + +**Rationale**: Allowed image MIME handling, tool-result image filtering, and content-item conversion appear in multiple frontend paths. A `frontend/src/utils/content.ts` helper and optional shared image component reduce duplication and protect image behavior during stream and history restore flows. + +**Alternatives considered**: Keeping logic inside components is simple but repeats validation details and increases drift risk. + +## Decision: Split Stateful Frontend Modules By Existing Responsibilities + +**Rationale**: `AgentBuilder.tsx`, `SkillBuilder.tsx`, `ChatPage.tsx`, and `useChat.ts` mix state orchestration and rendering. Extracting hooks/components for session lifecycle, conversation persistence, MCP editing, starter editing, capability picking, and form state reduces large files without changing page exports. + +**Alternatives considered**: Replacing the current state model with a new global store was rejected as unnecessary and dependency-heavy. + +## Decision: Do CSS Cleanup Last And Verify Visually + +**Rationale**: `App.css` is unused, `index.css` contains demo/global remnants, and `styles/index.css` repeats button/input/panel/message patterns. CSS deletion is high line-savings but visually risky, so it should follow component extraction and must use the constitution's Playwright screenshot protocol. + +**Alternatives considered**: Rewriting CSS into modules or a utility framework was rejected because it would be a broad styling migration rather than a focused refactor. + +## Decision: Replace Brittle Source-Inspection Tests Where Feasible + +**Rationale**: Several tests use `inspect.getsource` or signature checks, which can block behavior-preserving refactors. Shared fixtures in `tests/conftest.py` and behavioral endpoint/unit tests should replace source-text assertions when possible. + +**Alternatives considered**: Keeping source-inspection tests avoids writing new assertions, but it creates false failures during exactly this refactor. + +## Decision: No New Dependencies + +**Rationale**: Existing tooling is sufficient: pytest, FastAPI TestClient, Vite/TypeScript build, ESLint, and Playwright for screenshots. New dependencies would violate the simplicity principle unless a later task proves a concrete need. + +**Alternatives considered**: Adding frontend test libraries or a CSS tooling package could help, but the current request is simplification and deduplication, not test stack expansion. + +## Decision: Flatten Wrapper Layers Rather Than Extract More Services + +**Rationale**: The first implementation pass separated responsibilities from oversized files, but session creation now reads through too many implementation-shaped names: `SessionCreationService`, `_create_custom_session`, `_create_profile_session`, `create_chat_runtime`, `spawn_agent`, and `_create_agent`. The next cleanup should shorten call chains, rename helpers toward domain intent, and delete compatibility layers that are no longer needed. + +**Alternatives considered**: Creating additional builder/service classes was rejected because it would make the session path more abstract. Leaving the wrappers in place was rejected because the primary maintainer experience problem is now navigation and comprehension, not lack of extraction. \ No newline at end of file diff --git a/specs/007-application-refactor/spec.md b/specs/007-application-refactor/spec.md new file mode 100644 index 0000000..c73d2b7 --- /dev/null +++ b/specs/007-application-refactor/spec.md @@ -0,0 +1,122 @@ +# Feature Specification: Application Refactor And Deduplication + +**Feature Branch**: `007-application-refactor` +**Created**: 2026-05-06 +**Status**: Draft +**Input**: User description: "lets plan for this application refactor as discussed. the only thing i dont want to do is mess with the agents.yaml." + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Preserve Chat Behavior While Simplifying Backend Sessions (Priority: P1) + +As a user of the chat application, I need existing standard agents, customized built-in agents, and custom agents to create sessions, stream responses, call tools, restore history, report usage, and clean up resources exactly as they do today while the backend implementation is split into smaller, easier-to-maintain units. + +**Why this priority**: Backend session orchestration is the largest behavioral risk and the largest concentration of duplicated code. Preserving chat behavior is the minimum viable value for the refactor. + +**Independent Test**: Can be tested by running backend API tests for profile listing, session creation, message streaming, history export, retry/context-limit handling, image validation, and session deletion without changing user-visible responses. + +**Acceptance Scenarios**: + +1. **Given** an existing standard profile, **When** a user creates a session and sends a message, **Then** the API returns the same session fields, streams the same event types, logs token usage, and cleans up the session on deletion. +2. **Given** a customized built-in profile override, **When** a user creates a session, **Then** the canonical profile name is preserved, override metadata is returned in the session response and history response, and selected tools, skills, search context, and MCP servers are honored. +3. **Given** a custom agent with selected tools, skills, optional MCP servers, optional search context, and optional temperature, **When** a user creates a session, **Then** validation and runtime creation behave as before and invalid tools, skills, temperatures, or MCP entries are rejected with equivalent errors. + +--- + +### User Story 2 - Reduce Frontend Duplication Without Changing Workflows (Priority: P2) + +As a user of the frontend, I need profile selection, chat, saved conversations, admin agent customization, skill editing, image display, toast notifications, and authentication flows to keep working while repeated API, local storage, image rendering, and form-management code is consolidated. + +**Why this priority**: The frontend has repeated request handling, local storage handling, image filtering, and large stateful components that slow down future feature work. + +**Independent Test**: Can be tested by building the frontend, running lint/type checks, and manually or visually verifying profile selection, chat send, saved conversation resume/delete, admin agent builder, and skill builder flows. + +**Acceptance Scenarios**: + +1. **Given** a user opens the chat page, **When** profiles load, a session starts, a message streams, and a conversation is saved, **Then** the UI behavior and stored conversation data match current behavior. +2. **Given** a user edits or creates a custom agent, **When** they change tools, skills, MCP servers, starter questions, search context, and temperature, **Then** validation, save, delete, and generated standard-agent candidate behavior remain unchanged. +3. **Given** a user creates, edits, generates, or deletes a skill, **When** the operation succeeds or fails, **Then** the displayed feedback and API behavior remain unchanged. + +--- + +### User Story 3 - Clean Styling And Tests Safely (Priority: P3) + +As a maintainer, I need dead CSS, repeated CSS variants, repeated test setup, and brittle source-inspection tests reduced while retaining the current visual design and behavioral test coverage. + +**Why this priority**: CSS and tests contain easy cleanup opportunities, but UI appearance and regression coverage must not degrade. + +**Independent Test**: Can be tested by building the frontend, performing automated screenshot verification for affected UI screens, and running backend tests. + +**Acceptance Scenarios**: + +1. **Given** unused stylesheet content exists, **When** it is removed or consolidated, **Then** affected screens render without clipping, overlap, broken assets, or unintended visual changes. +2. **Given** duplicated test fixtures and source-inspection tests exist, **When** they are refactored into shared fixtures or behavioral assertions, **Then** test coverage remains equivalent and tests no longer depend on specific function source text where behavior can be asserted. + +--- + +### User Story 4 - Flatten Wrapper Layers Around Session And Agent Creation (Priority: P1 Follow-Up) + +As a maintainer walking through session startup, I need the code path from frontend session request through backend runtime creation to read as a direct flow instead of a stack of pass-through wrappers, while preserving all user-visible behavior and compatibility exports. + +**Why this priority**: The first refactor reduced large files but introduced or preserved layers such as `SessionCreationService`, `create_chat_runtime`, `spawn_agent`, `_create_agent`, and multiple frontend session wrappers. These make the most important flow harder to understand than it needs to be. + +**Independent Test**: Can be tested by running backend session/API tests, frontend test/build/lint, API export compile checks, and `git diff -- config/agents.yaml` after flattening the wrapper layers. + +**Acceptance Scenarios**: + +1. **Given** a maintainer traces custom, standard, or customized built-in session creation, **When** they follow the call chain, **Then** they can see request validation, runtime creation, session persistence, and response shaping without passing through redundant create/spawn/service wrappers. +2. **Given** frontend code creates a standard, history, custom, or built-in override session, **When** the API client sends the request, **Then** mode-specific payload construction is centralized while existing exported API functions remain compatible. +3. **Given** canonical helper functions exist for streaming, validation, and agent building, **When** callers import helpers, **Then** obsolete underscore aliases and one-method wrapper classes are removed unless a test or public compatibility contract requires them. + +### Edge Cases + +- Refactor must preserve unauthorized responses and auth-disabled local development behavior. +- Refactor must preserve multipart image upload validation for MIME type, count, size, and magic bytes. +- Refactor must preserve MCP connection failure reporting without leaking credentials or bearer tokens. +- Refactor must preserve best-effort conversation persistence when local storage is corrupt or unavailable. +- Refactor must preserve frontend rendering of tool-result images from streamed responses and restored session history. +- Refactor must not edit `config/agents.yaml`, including YAML anchors, prompt content, profile entries, or MCP definitions. +- Refactor must not add new runtime dependencies unless a later task documents an immediate need and the dependency passes constitution review. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The backend MUST preserve existing API routes, status codes, response field names, and SSE event names for health, auth config, tools, profiles, profile definitions, MCP tests, skills, sessions, messages, history, and deletion. +- **FR-002**: The backend MUST extract repeated session creation concerns into focused units for profile resolution, custom-agent validation, profile-override validation, tool validation, skill validation, MCP request validation, runtime creation, history restoration, session persistence, and session response formatting. +- **FR-003**: The backend MUST preserve custom agent, standard profile, and built-in override behavior, including loaded tools, loaded skills, search context state, MCP results, user profile context injection, and override metadata. +- **FR-004**: The backend MUST extract or consolidate usage aggregation, image validation, retry/context error classification, skills CRUD path handling, and stream event transformation where doing so reduces duplication without changing behavior. +- **FR-005**: The frontend API client MUST consolidate repeated authenticated fetch, JSON body, unauthorized handling, HTTP error handling, and toast emission patterns while preserving existing exported function signatures where practical. +- **FR-006**: The frontend MUST consolidate repeated local storage parse/stringify/corruption handling for custom agents, built-in overrides, user profile, and conversations without changing storage keys or stored data shape. +- **FR-007**: The frontend MUST extract reusable helpers or components for tool-result image filtering/rendering and shared content-item conversion without changing visible image behavior. +- **FR-008**: The frontend MUST split large stateful screens and hooks into smaller units where the split is directly tied to current responsibilities: chat session lifecycle, conversation persistence, agent builder form state, MCP server editor, starter question editor, capability picker, and skill builder form behavior. +- **FR-009**: CSS cleanup MUST remove unused styles and consolidate repeated button, input, panel, message, admin, and builder variants while preserving the current visual design. +- **FR-010**: Test cleanup MUST move repeated test environment/client fixtures into shared fixtures and replace brittle source-inspection assertions with behavioral assertions where feasible. +- **FR-011**: Refactor tasks MUST keep `config/agents.yaml` read-only and out of scope. No feature artifact may require editing it. +- **FR-012**: Refactor tasks MUST avoid new backend or frontend dependencies unless explicitly justified as necessary for this refactor. +- **FR-013**: The backend MUST flatten the agent creation call chain so runtime construction no longer requires both `spawn_agent` and `_create_agent` as pass-through wrappers. +- **FR-014**: The backend MUST replace per-request service construction for session creation with a clearer function or stable service boundary that reduces constructor dependency plumbing. +- **FR-015**: The frontend MUST centralize `/api/sessions` POST body construction while keeping compatibility exports for current callers. +- **FR-016**: The refactor MUST remove compatibility aliases, one-method wrapper classes, and setter-bag hooks where they do not encode a meaningful domain boundary. + +### Key Entities *(include if feature involves data)* + +- **Session Creation Request**: Incoming session payload for standard profiles, custom agents, built-in overrides, optional history, user profile context, selected tools, selected skills, MCP servers, search context, and temperature. +- **Session Creation Result**: Created runtime/session metadata returned to the frontend, including session ID, profile identity, loaded tools and skills, search context state, MCP connection results, and override metadata. +- **Stream Event**: Server-sent event emitted during message processing, including text, function call, function result, usage, error, and done events. +- **Stored Conversation**: Frontend local-storage record containing conversation index metadata and backend session state for resume/delete flows. +- **Agent Builder Form State**: Frontend editable state for custom agents and built-in override customizations, including tools, skills, MCP servers, starter questions, search context, icon, description, prompt, and temperature. +- **Skill Definition**: Backend file-backed skill represented by name, description, and Markdown content and edited through the admin UI. +- **Visual Style Primitive**: Reusable CSS pattern for buttons, inputs, panels, message markdown, admin panels, and builder controls. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Runtime app code is reduced by at least 1,000 net lines across backend Python and frontend source/CSS, excluding generated artifacts and historical specs, while all existing user-visible behavior remains intact. +- **SC-002**: `main.py` is reduced below 1,200 lines by moving session, skill, validation, and streaming responsibilities into focused modules or helpers. +- **SC-003**: Frontend CSS is reduced by at least 400 net lines while automated visual verification shows no layout regressions on affected screens. +- **SC-004**: The frontend API client reduces repeated fetch/error-handling boilerplate by at least 50 net lines while preserving exported API behavior. +- **SC-005**: Backend tests pass with `uv run pytest`; frontend verification passes with `npm test`, `npm run build`, and `npm run lint`. +- **SC-006**: `git diff -- config/agents.yaml` remains empty throughout the feature. +- **SC-007**: The standard/custom/override session creation path crosses at least two fewer internal wrapper functions or classes than the current implementation while all backend and frontend verification gates still pass. diff --git a/specs/007-application-refactor/tasks.md b/specs/007-application-refactor/tasks.md new file mode 100644 index 0000000..0a45848 --- /dev/null +++ b/specs/007-application-refactor/tasks.md @@ -0,0 +1,307 @@ +# Tasks: Application Refactor And Deduplication + +**Input**: Design documents from `/specs/007-application-refactor/` +**Prerequisites**: [plan.md](plan.md), [spec.md](spec.md), [research.md](research.md), [data-model.md](data-model.md), [contracts/](contracts/), [quickstart.md](quickstart.md) + +**Tests**: Test and verification tasks are included because the specification explicitly requires backend pytest, frontend test/build/lint, contract preservation, and visual verification for UI/CSS changes. + +**Organization**: Tasks are grouped by user story to enable independent implementation and testing. `config/agents.yaml` is intentionally excluded from implementation tasks and must remain unchanged. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel because it touches different files and has no dependency on an incomplete task. +- **[Story]**: User story label for story-phase tasks only. +- Every task includes at least one exact file path. + +--- + +## Phase 1: Setup (Shared Infrastructure) + +**Purpose**: Establish branch, scope, baseline, and protected-file guardrails before refactoring. + +- [X] T001 Verify branch `007-application-refactor` and record the empty `git diff -- config/agents.yaml` guard in specs/007-application-refactor/quickstart.md +- [X] T002 [P] Capture baseline backend test status with `uv run pytest` and record failures or pass result in specs/007-application-refactor/quickstart.md +- [X] T003 [P] Add or confirm a no-new-dependency `test` script in frontend/package.json, then capture baseline frontend test/build/lint status with `npm test`, `npm run build`, and `npm run lint` from frontend/package.json and record results in specs/007-application-refactor/quickstart.md +- [X] T004 [P] Capture current runtime line-count baseline for main.py, frontend/src/api/client.ts, frontend/src/hooks/useChat.ts, frontend/src/pages/AgentBuilder.tsx, frontend/src/components/SkillBuilder.tsx, frontend/src/styles/index.css, frontend/src/index.css, and frontend/src/App.css in specs/007-application-refactor/quickstart.md + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Add shared test and helper foundations that all stories depend on. + +**CRITICAL**: No user story work should begin until this phase is complete. + +- [X] T005 Move duplicated TestClient and `_sessions.clear()` setup into shared fixtures in tests/conftest.py +- [X] T006 [P] Update tests/test_api.py to use the shared client fixture from tests/conftest.py without changing route assertions +- [X] T007 [P] Update tests/test_skills_api.py to use the shared client and temporary skills fixtures from tests/conftest.py +- [X] T008 [P] Create protected-file verification helper or documented check in specs/007-application-refactor/quickstart.md for `git diff -- config/agents.yaml` +- [X] T009 [P] Add backend module skeleton and exports for validators.py with no behavior wired into main.py yet +- [X] T010 [P] Add backend module skeleton and exports for streaming.py with no behavior wired into main.py yet +- [X] T011 [P] Add backend module skeleton and exports for skills_manager.py with no behavior wired into main.py yet +- [X] T012 [P] Add backend module skeleton and exports for session_orchestration.py with no behavior wired into main.py yet + +**Checkpoint**: Shared fixtures and empty module targets exist; user story implementation can begin. + +--- + +## Phase 3: User Story 1 - Preserve Chat Behavior While Simplifying Backend Sessions (Priority: P1) MVP + +**Goal**: Preserve all backend API/session/SSE behavior while extracting duplicated validation, streaming, skills, and session orchestration code out of main.py. + +**Independent Test**: Run `uv run pytest tests/test_api.py tests/test_image_input.py tests/test_retry_logic.py tests/test_skills_api.py tests/test_skills.py tests/test_session_orchestration.py tests/test_validators.py tests/test_skills_manager.py tests/test_streaming.py` and verify `git diff -- config/agents.yaml` remains empty. + +### Tests for User Story 1 + +- [X] T013 [P] [US1] Add tool, skill, temperature, prompt, image, and error-classifier unit tests in tests/test_validators.py +- [X] T014 [P] [US1] Add token usage, SSE event formatting, tool-result content conversion, and error event tests in tests/test_streaming.py +- [X] T015 [P] [US1] Add file-backed skill list/get/create/update/delete/path-safety tests in tests/test_skills_manager.py +- [X] T016 [P] [US1] Add standard profile, custom agent, built-in override, history restore, invalid input, response-shape, and MCP failure redaction tests in tests/test_session_orchestration.py +- [X] T017 [P] [US1] Add or update backend API contract assertions for preserved session and SSE response fields in tests/test_api.py + +### Implementation for User Story 1 + +- [X] T018 [P] [US1] Move usage aggregation helpers, SSE event formatting, retry/context error classification, and content-item conversion from main.py into streaming.py +- [X] T019 [P] [US1] Move image validation constants and upload validation from main.py into validators.py while preserving ALLOWED_IMAGE_MIMES, MAX_IMAGE_SIZE_BYTES, and MAX_IMAGES_PER_MESSAGE compatibility exports +- [X] T020 [P] [US1] Implement ToolRegistry, skill discovery validation, prompt validation, temperature validation, and MCP request validation helpers in validators.py +- [X] T021 [P] [US1] Implement SkillManager for SKILL.md parsing, validation, path traversal prevention, create, update, delete, and list behavior in skills_manager.py +- [X] T022 [US1] Refactor main.py imports to use streaming.py for `_create_usage`, `_usage_value`, `_merge_usage`, `_extract_usage_from_payload`, `_sse_event`, `_is_retryable_error`, and `_is_context_length_error` +- [X] T023 [US1] Refactor main.py imports to use validators.py for image validation and shared tool/skill/temperature validation while preserving public constants used by tests +- [X] T024 [US1] Refactor skill endpoints in main.py to delegate list/get/create/update/delete path operations to SkillManager in skills_manager.py +- [X] T025 [US1] Implement SessionCreationService in session_orchestration.py for standard profile, custom agent, built-in override, user profile injection, sanitized MCP connection failure reporting, history restoration, and response shaping +- [X] T026 [US1] Refactor create_session in main.py to delegate session creation to SessionCreationService while preserving `_sessions` ownership in main.py +- [X] T027 [US1] Update delete_session and lifespan behavior in main.py only as needed to preserve MCP cleanup and final usage logging after session extraction +- [X] T028 [US1] Remove obsolete duplicated validation, skills CRUD, usage, and session response code from main.py after extracted modules are wired in +- [X] T029 [US1] Run focused backend tests and fix regressions in main.py, validators.py, streaming.py, skills_manager.py, session_orchestration.py, and tests/conftest.py + +**Checkpoint**: User Story 1 is complete when backend route contracts and session behavior are preserved and `main.py` is reduced below 1,200 lines without changing config/agents.yaml. + +--- + +## Phase 4: User Story 2 - Reduce Frontend Duplication Without Changing Workflows (Priority: P2) + +**Goal**: Preserve frontend exported API, hook, storage, and workflow behavior while consolidating repeated request, storage, content, and form-state code. + +**Independent Test**: Run `cd frontend && npm run build && npm run lint`, manually exercise profile selection/chat/admin builder/skill builder flows, and verify localStorage keys and API exports remain compatible. + +### Tests and Verification for User Story 2 + +- [X] T030 [P] [US2] Add TypeScript compile coverage for API export compatibility by adding or documenting a compile-only import matrix for all public exports from frontend/src/api/client.ts +- [X] T031 [P] [US2] Add storage compatibility checks or documented manual verification for existing localStorage keys in specs/007-application-refactor/quickstart.md +- [X] T032 [P] [US2] Add frontend workflow verification notes for chat send, saved conversation resume/delete, agent builder save/delete, and skill builder CRUD in specs/007-application-refactor/quickstart.md + +### Implementation for User Story 2 + +- [X] T033 [P] [US2] Create authenticated fetch, JSON body, empty response, unauthorized, and toast-aware response helpers in frontend/src/api/helpers.ts +- [X] T034 [US2] Refactor frontend/src/api/client.ts to use frontend/src/api/helpers.ts while preserving all existing exported functions and SSE handling +- [X] T035 [P] [US2] Create typed localStorage read/write/remove JSON helpers with corruption handling in frontend/src/utils/storage.ts +- [X] T036 [US2] Refactor frontend/src/hooks/useCustomAgents.ts to use frontend/src/utils/storage.ts without changing `webagents_custom_agents` +- [X] T037 [US2] Refactor frontend/src/hooks/useBuiltInAgentCustomizations.ts to use frontend/src/utils/storage.ts without changing `webagents_builtin_agent_customizations` +- [X] T038 [US2] Refactor frontend/src/hooks/useUserProfile.ts to use frontend/src/utils/storage.ts without changing `webagents_user_profile` +- [X] T039 [US2] Refactor frontend/src/hooks/useConversationStore.ts to use frontend/src/utils/storage.ts without changing conversation index and per-conversation keys +- [X] T040 [P] [US2] Create image MIME, tool image filtering, tool result formatting, and session content conversion helpers in frontend/src/utils/content.ts +- [X] T041 [US2] Refactor frontend/src/components/ChatMessage.tsx and frontend/src/components/ToolStep.tsx to use shared image/content helpers from frontend/src/utils/content.ts +- [X] T042 [US2] Extract conversation persistence and session lifecycle helpers from frontend/src/hooks/useChat.ts into frontend/src/hooks/useSessionLifecycle.ts and frontend/src/hooks/useConversationPersistence.ts +- [X] T043 [US2] Refactor frontend/src/hooks/useChat.ts to use useSessionLifecycle and useConversationPersistence while preserving the ChatState return shape +- [X] T044 [P] [US2] Extract MCP server editor state and operations from frontend/src/pages/AgentBuilder.tsx into frontend/src/hooks/useAgentMcpEditor.ts +- [X] T045 [P] [US2] Extract agent form state, validation, reset, edit, and save preparation from frontend/src/pages/AgentBuilder.tsx into frontend/src/hooks/useAgentBuilderForm.ts +- [X] T046 [P] [US2] Extract tool and skill picker logic from frontend/src/pages/AgentBuilder.tsx into frontend/src/components/AgentCapabilityPicker.tsx +- [X] T047 [P] [US2] Extract starter question editor UI from frontend/src/pages/AgentBuilder.tsx into frontend/src/components/StarterQuestionEditor.tsx +- [X] T048 [US2] Refactor frontend/src/pages/AgentBuilder.tsx to compose the extracted hooks/components while preserving exported AgentBuilder props +- [X] T049 [P] [US2] Extract reusable skill form state from frontend/src/components/SkillBuilder.tsx into frontend/src/hooks/useSkillForm.ts +- [X] T050 [US2] Refactor frontend/src/components/SkillBuilder.tsx to use useSkillForm while preserving create/edit/delete/generate behavior +- [X] T051 [US2] Run frontend build/lint and fix regressions in frontend/src/api/client.ts, frontend/src/hooks/useChat.ts, frontend/src/pages/AgentBuilder.tsx, frontend/src/components/SkillBuilder.tsx, and new frontend helpers + +**Checkpoint**: User Story 2 is complete when frontend workflows still work, exported API/hook contracts are stable, and duplicated request/storage/content/form code is reduced. + +--- + +## Phase 5: User Story 3 - Clean Styling And Tests Safely (Priority: P3) + +**Goal**: Remove dead/repeated CSS and brittle tests while preserving visual design and behavioral coverage. + +**Independent Test**: Run `uv run pytest`, `cd frontend && npm run build`, start the backend, capture screenshots for affected screens, inspect screenshots, and verify `git diff -- config/agents.yaml` remains empty. + +### Tests and Verification for User Story 3 + +- [X] T052 [P] [US3] Replace source-inspection assertions with behavioral assertions where feasible in tests/test_image_input.py +- [X] T053 [P] [US3] Replace source-inspection assertions with behavioral assertions where feasible in tests/test_retry_logic.py +- [X] T054 [P] [US3] Add or update reusable Playwright screenshot capture coverage in scripts/capture_admin_agent_screenshots.py, or a companion script if needed, for disclaimer top/bottom, profile selection, empty chat with starter questions, chat with one user message and one assistant response, admin agent builder, and skill builder screens +- [X] T055 [US3] Run visual baseline capture at required viewports where affected, save screenshots under screenshots/, view every generated image, and record pass/fail notes in specs/007-application-refactor/quickstart.md + +### Implementation for User Story 3 + +- [X] T056 [P] [US3] Remove unused demo styles from frontend/src/App.css and remove any stale import references if present in frontend/src/App.tsx +- [X] T057 [P] [US3] Trim frontend/src/index.css to root/global styles actually used by frontend/src/main.tsx and frontend/src/App.tsx +- [X] T058 [US3] Consolidate repeated button, input, textarea, admin panel, saved entry, and action styles in frontend/src/styles/index.css +- [X] T059 [US3] Consolidate repeated message markdown, tool image, and result image styles in frontend/src/styles/index.css +- [X] T060 [US3] Update affected class usage in frontend/src/components/ChatMessage.tsx, frontend/src/components/ToolStep.tsx, frontend/src/pages/AgentBuilder.tsx, and frontend/src/components/SkillBuilder.tsx only if required by CSS consolidation +- [X] T061 [US3] Run frontend build plus full screenshot verification, inspect every generated screenshot for clipping, overflow, overlap, broken assets, missing content, unreadable text, and responsive issues, then fix and re-run until visual review passes in frontend/src/styles/index.css, frontend/src/index.css, frontend/src/App.css, and scripts/capture_admin_agent_screenshots.py + +**Checkpoint**: User Story 3 is complete when CSS line count is reduced by at least 400 net lines, visual verification passes, and backend tests remain behavior-focused. + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +**Purpose**: Final contract, quality, documentation, and line-count validation across the completed refactor. + +- [X] T062 [P] Run complete backend test suite with `uv run pytest` and record result in specs/007-application-refactor/quickstart.md +- [X] T063 [P] Run complete frontend test/build/lint with `cd frontend && npm test && npm run build && npm run lint` and record result in specs/007-application-refactor/quickstart.md +- [X] T064 Verify protected file diff remains empty with `git diff -- config/agents.yaml`, verify dependency files have no unplanned dependency additions, and record results in specs/007-application-refactor/quickstart.md +- [X] T065 Recalculate line-count reductions across all runtime app code while excluding specs, generated output, node_modules, dist, screenshots, traces, and test artifacts, then record against SC-001 through SC-004 in specs/007-application-refactor/quickstart.md +- [X] T066 [P] Review API contract preservation against specs/007-application-refactor/contracts/api-contract.md and update any missing backend tests in tests/test_api.py +- [X] T067 [P] Review frontend contract preservation against specs/007-application-refactor/contracts/frontend-contract.md and update verification notes in specs/007-application-refactor/quickstart.md +- [X] T068 Final cleanup of obsolete imports, comments, and dead helper code in main.py, validators.py, streaming.py, skills_manager.py, session_orchestration.py, and frontend/src + +--- + +## Phase 7: Follow-Up Wrapper Flattening + +**Purpose**: Clean up wrapper code exposed by the first refactor pass so session and agent creation read as direct domain flows instead of chains of service/create/spawn helpers. + +**Independent Test**: Run `uv run pytest tests/test_api.py tests/test_session_orchestration.py tests/test_provider_routing.py tests/test_retry_logic.py tests/test_streaming.py tests/test_validators.py`, then `cd frontend && npm test && npm run build && npm run lint`, and verify `git diff -- config/agents.yaml` remains empty. + +### Backend Wrapper Flattening + +- [X] T069 [US4] Document the current standard, custom, and built-in override session call chains in specs/007-application-refactor/quickstart.md before editing agent_factory.py, session_orchestration.py, or frontend/src/hooks/useSessionLifecycle.ts +- [X] T070 [P] [US4] Add or update tests in tests/test_provider_routing.py and tests/test_api.py to pin custom, standard, and built-in override runtime/session behavior before flattening agent_factory.py and session_orchestration.py +- [X] T071 [US4] Collapse `spawn_agent()` and `_create_agent()` in agent_factory.py into one clearly named builder used by `create_chat_runtime()` while preserving `AgentBase` only if an existing caller still requires it +- [X] T072 [US4] Rename or simplify `create_chat_runtime()` in agent_factory.py only if the new name improves domain intent, and update imports in session_orchestration.py and tests without changing runtime behavior +- [X] T073 [US4] Replace per-request `SessionCreationService(...)` construction in main.py with direct session creation functions or a module-level boundary in session_orchestration.py that reduces constructor dependency plumbing +- [X] T074 [US4] Split duplicated session response persistence in session_orchestration.py into a small shared helper only if it removes repeated response/session-data assembly without adding another wrapper layer +- [X] T075 [US4] Remove `ToolRegistry` from validators.py and update session_orchestration.py and tests to use `known_tool_names_from_profiles()` plus `validate_tool_names()` directly +- [X] T076 [US4] Remove underscore compatibility aliases and exports from streaming.py after updating main.py and tests to import canonical helper names +- [X] T077 [US4] Run focused backend validation and fix regressions in agent_factory.py, session_orchestration.py, validators.py, streaming.py, main.py, and related tests + +### Frontend Session And Hook Flattening + +- [X] T078 [P] [US4] Add or update compile-only API contract coverage in frontend/src/api/client.contract.ts for any new internal session request helper while preserving existing public exports from frontend/src/api/client.ts +- [X] T079 [US4] Centralize `/api/sessions` POST behavior in frontend/src/api/client.ts behind one internal typed request helper while keeping `createSession`, `createSessionWithHistory`, `createCustomSession`, and `createSessionWithProfileOverride` as compatibility exports +- [X] T080 [US4] Simplify `startChatSession()` in frontend/src/hooks/useSessionLifecycle.ts to build one session request intent before calling the centralized frontend API helper +- [X] T081 [US4] Reassess frontend/src/hooks/useSkillForm.ts, frontend/src/hooks/useAgentBuilderForm.ts, and frontend/src/hooks/useAgentMcpEditor.ts; inline state-only hooks or rename/reshape them so each remaining hook owns a workflow rather than only returning setters +- [X] T082 [US4] Run frontend validation and fix regressions in frontend/src/api/client.ts, frontend/src/hooks/useSessionLifecycle.ts, frontend/src/hooks/useChat.ts, frontend/src/pages/AgentBuilder.tsx, and frontend/src/components/SkillBuilder.tsx + +### Final Wrapper Cleanup Validation + +- [X] T083 [US4] Recalculate the session creation call-chain length and runtime line counts, then record the before/after wrapper reduction and SC-007 result in specs/007-application-refactor/quickstart.md +- [X] T084 [US4] Run complete backend and frontend gates with `uv run pytest` and `cd frontend && npm test && npm run build && npm run lint`, verify `git diff -- config/agents.yaml` remains empty, and record final results in specs/007-application-refactor/quickstart.md + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Phase 1 Setup**: No dependencies; can start immediately. +- **Phase 2 Foundational**: Depends on Phase 1; blocks all user stories. +- **Phase 3 US1 Backend MVP**: Depends on Phase 2; should complete before broad frontend and CSS work because it preserves API contracts. +- **Phase 4 US2 Frontend Dedupe**: Depends on Phase 2 and should be validated against US1 route contracts when US1 is available. +- **Phase 5 US3 Styling/Test Cleanup**: Depends on Phase 4 for component structure stability and should run after most UI class usage settles. +- **Phase 6 Polish**: Depends on all desired user stories. +- **Phase 7 Wrapper Flattening**: Depends on Phase 6 baseline completion and should preserve all completed behavior while reducing wrapper layers. + +### User Story Dependencies + +- **US1 (P1)**: Can start after Foundational; no dependency on US2 or US3. +- **US2 (P2)**: Can start after Foundational, but final verification depends on stable backend contracts from US1. +- **US3 (P3)**: Best after US2 because CSS cleanup should follow component extraction; test cleanup can begin after Foundational. +- **US4 (P1 Follow-Up)**: Starts after the first refactor pass is validated; can be delivered independently as a maintainability cleanup with no user-visible behavior changes. + +### Within Each User Story + +- Write/update tests and verification notes before implementation tasks in the same story. +- Extract helper modules before wiring routes or components into them. +- Preserve exports and route contracts before deleting old code. +- Run focused verification at every checkpoint. + +--- + +## Parallel Opportunities + +- T002, T003, and T004 can run in parallel after T001. +- T006 through T012 can run in parallel after T005 where they touch separate files. +- US1 test tasks T013 through T017 can run in parallel. +- US1 helper implementation tasks T018 through T021 can run in parallel before main.py wiring begins. +- US2 helper/component extraction tasks T033, T035, T040, T044, T045, T046, T047, and T049 can run in parallel before integration tasks. +- US3 source-inspection test tasks T052 and T053 can run in parallel with screenshot script task T054. +- Final verification tasks T062, T063, T066, and T067 can run in parallel after implementation completes. +- T070 and T078 can run in parallel after T069 because backend behavior pinning and frontend export coverage touch separate files. +- T071, T075, and T076 can be worked independently after T070, but T073 should wait until agent factory flattening settles. +- T079 can proceed after T078; T080 should wait until T079 provides the centralized session request helper. + +--- + +## Parallel Example: User Story 1 + +```bash +# Backend helper tests can be drafted together: +Task: "T013 [P] [US1] Add tool, skill, temperature, prompt, image, and error-classifier unit tests in tests/test_validators.py" +Task: "T014 [P] [US1] Add token usage, SSE event formatting, tool-result content conversion, and error event tests in tests/test_streaming.py" +Task: "T015 [P] [US1] Add file-backed skill list/get/create/update/delete/path-safety tests in tests/test_skills_manager.py" +Task: "T016 [P] [US1] Add standard profile, custom agent, built-in override, history restore, invalid input, and response-shape tests in tests/test_session_orchestration.py" + +# Backend helper modules can be implemented together: +Task: "T018 [P] [US1] Move usage aggregation helpers, SSE event formatting, retry/context error classification, and content-item conversion from main.py into streaming.py" +Task: "T019 [P] [US1] Move image validation constants and upload validation from main.py into validators.py while preserving ALLOWED_IMAGE_MIMES, MAX_IMAGE_SIZE_BYTES, and MAX_IMAGES_PER_MESSAGE compatibility exports" +Task: "T020 [P] [US1] Implement ToolRegistry, skill discovery validation, prompt validation, temperature validation, and MCP request validation helpers in validators.py" +Task: "T021 [P] [US1] Implement SkillManager for SKILL.md parsing, validation, path traversal prevention, create, update, delete, and list behavior in skills_manager.py" +``` + +## Parallel Example: User Story 2 + +```bash +# Frontend helpers and extraction targets can be implemented together: +Task: "T033 [P] [US2] Create authenticated fetch, JSON body, empty response, unauthorized, and toast-aware response helpers in frontend/src/api/helpers.ts" +Task: "T035 [P] [US2] Create typed localStorage read/write/remove JSON helpers with corruption handling in frontend/src/utils/storage.ts" +Task: "T040 [P] [US2] Create image MIME, tool image filtering, tool result formatting, and session content conversion helpers in frontend/src/utils/content.ts" +Task: "T044 [P] [US2] Extract MCP server editor state and operations from frontend/src/pages/AgentBuilder.tsx into frontend/src/hooks/useAgentMcpEditor.ts" +Task: "T045 [P] [US2] Extract agent form state, validation, reset, edit, and save preparation from frontend/src/pages/AgentBuilder.tsx into frontend/src/hooks/useAgentBuilderForm.ts" +Task: "T049 [P] [US2] Extract reusable skill form state from frontend/src/components/SkillBuilder.tsx into frontend/src/hooks/useSkillForm.ts" +``` + +## Parallel Example: User Story 3 + +```bash +Task: "T052 [P] [US3] Replace source-inspection assertions with behavioral assertions where feasible in tests/test_image_input.py" +Task: "T053 [P] [US3] Replace source-inspection assertions with behavioral assertions where feasible in tests/test_retry_logic.py" +Task: "T054 [P] [US3] Add or update Playwright screenshot capture coverage for affected admin/chat screens in scripts/capture_admin_agent_screenshots.py" +``` + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete Phase 1 setup and Phase 2 foundational fixtures/module shells. +2. Complete Phase 3 backend extraction in this order: validators, streaming, skills manager, session orchestration. +3. Stop and validate with backend focused tests and `git diff -- config/agents.yaml`. +4. Confirm main.py is below 1,200 lines before proceeding. + +### Incremental Delivery + +1. Deliver US1 backend simplification and route-contract preservation. +2. Deliver US2 frontend helper/component dedupe while preserving workflows. +3. Deliver US3 CSS/test cleanup with visual verification. +4. Complete Phase 6 final line-count, contract, build, and protected-file checks. +5. Complete Phase 7 wrapper flattening only after the first refactor pass is green, with behavior-pinning tests before collapsing layers. + +### Parallel Team Strategy + +1. One developer completes shared fixtures and module shells. +2. Backend developer handles US1 helper modules and session orchestration. +3. Frontend developer handles US2 helper and component extraction after API contracts are stable. +4. UI/test developer handles US3 behavioral tests and screenshot verification after component class usage settles. + +--- + +## Notes + +- Do not edit config/agents.yaml in any task. +- Use `uv` for Python commands; never use `pip`. +- Use `npm` for frontend commands. +- Keep route paths, response field names, SSE event names, localStorage keys, and exported frontend API functions stable. +- Add no runtime dependencies unless a task updates the plan with a concrete constitution-compliant justification. +- Run visual verification for any CSS or component markup changes before closing US3. +- For Phase 7, prefer deleting a wrapper over renaming it. Add a new helper only when it removes repeated domain logic and shortens the reader's path through session creation. \ No newline at end of file diff --git a/streaming.py b/streaming.py new file mode 100644 index 0000000..0e46f70 --- /dev/null +++ b/streaming.py @@ -0,0 +1,259 @@ +"""Streaming, usage, and content conversion helpers for chat responses.""" + +import json +from typing import Any, AsyncGenerator, Optional + +from agent_framework import Agent as RuntimeAgent +from agent_framework import AgentSession +from agent_framework._types import Content, Message as ChatMessage, UsageDetails + +USAGE_INPUT_KEY = "input_token_count" +USAGE_OUTPUT_KEY = "output_token_count" +USAGE_TOTAL_KEY = "total_token_count" + + +def create_usage( + input_token_count: Optional[int] = None, + output_token_count: Optional[int] = None, + total_token_count: Optional[int] = None, +) -> UsageDetails: + return UsageDetails( + input_token_count=input_token_count, + output_token_count=output_token_count, + total_token_count=total_token_count, + ) + + +def usage_value(usage: Optional[UsageDetails], key: str) -> int: + if not usage: + return 0 + if isinstance(usage, dict): + return int(usage.get(key) or 0) + return int(getattr(usage, key, 0) or 0) + + +def merge_usage(current: Optional[UsageDetails], incoming: Optional[UsageDetails]) -> Optional[UsageDetails]: + if not incoming: + return current + if not current: + return incoming + return create_usage( + input_token_count=usage_value(current, USAGE_INPUT_KEY) + usage_value(incoming, USAGE_INPUT_KEY), + output_token_count=usage_value(current, USAGE_OUTPUT_KEY) + usage_value(incoming, USAGE_OUTPUT_KEY), + total_token_count=usage_value(current, USAGE_TOTAL_KEY) + usage_value(incoming, USAGE_TOTAL_KEY), + ) + + +def extract_usage_from_payload(payload: dict) -> Optional[UsageDetails]: + usage_data = payload.get("usage") or {} + if not usage_data: + return None + return create_usage( + input_token_count=usage_data.get(USAGE_INPUT_KEY), + output_token_count=usage_data.get(USAGE_OUTPUT_KEY), + total_token_count=usage_data.get(USAGE_TOTAL_KEY), + ) + + +def is_retryable_error(error: Exception) -> bool: + error_message = str(error) + error_type = str(type(error)) + error_lower = error_message.lower() + return ( + "429" in error_message + or "Too Many Requests" in error_message + or "RateLimitError" in error_type + or "rate_limit" in error_lower + or "rate limit" in error_lower + or "capacity" in error_lower + ) + + +def is_context_length_error(error: Exception) -> bool: + error_text = str(error).lower() + return any( + phrase in error_text + for phrase in [ + "context length", + "maximum context length", + "token limit", + "too many tokens", + "prompt is too long", + "maximum prompt", + ] + ) + + +def sse_event(event: str, data: dict) -> str: + return f"event: {event}\ndata: {json.dumps(data, ensure_ascii=False)}\n\n" + + +def convert_content_items(items: object) -> list[dict[str, Any]]: + if not isinstance(items, list): + return [] + + converted: list[dict[str, Any]] = [] + for item in items: + if not isinstance(item, dict): + continue + item_type = item.get("type") + if item_type == "text": + converted.append({"type": "text", "text": item.get("text", "")}) + elif item_type == "data": + uri = item.get("uri", "") + if isinstance(uri, str) and uri.startswith("data:image/"): + header, _, b64data = uri.partition(",") + mime_type = header.split(";")[0].replace("data:", "") + if b64data and mime_type: + converted.append({"type": "image", "data": b64data, "mimeType": mime_type}) + elif item_type == "image" and item.get("data") and item.get("mimeType"): + converted.append(item) + return converted + + +def render_tool_result(result: object) -> str: + if isinstance(result, list): + text_parts = [ + item.get("text", "") for item in result + if isinstance(item, dict) and item.get("type") == "text" + ] + return "\n".join(text_parts) if text_parts else json.dumps(result, ensure_ascii=False) + if isinstance(result, dict): + return json.dumps(result, ensure_ascii=False) + return str(result or "") + + +async def stream_agent_response( + agent: RuntimeAgent, + contents: list[Content], + session: AgentSession, +) -> AsyncGenerator[str, None]: + request_usage: Optional[UsageDetails] = None + final_text_parts: list[str] = [] + tool_events: list[dict[str, Any]] = [] + tool_event_by_call_id: dict[str, dict[str, Any]] = {} + active_call_id: Optional[str] = None + args_accumulator: dict[str, str] = {} + + user_message = ChatMessage(role="user", contents=contents) + stream = agent.run(user_message, session=session, stream=True) + + async for msg in stream: + msg_dict = msg.to_dict() + + update_usage = extract_usage_from_payload(msg_dict) + request_usage = merge_usage(request_usage, update_usage) + + for content in msg_dict.get("contents", []) or []: + content_type = content.get("type") + + if content_type == "function_call": + call_id = content.get("call_id") or None + name = content.get("name") or None + arguments = content.get("arguments", "") + rendered_arguments = json.dumps(arguments, ensure_ascii=False) if isinstance(arguments, (dict, list)) else str(arguments) + + if name and call_id and call_id not in tool_event_by_call_id: + active_call_id = call_id + args_accumulator[call_id] = rendered_arguments + event_payload = {"call_id": call_id, "name": name, "arguments": rendered_arguments, "result": None} + tool_events.append(event_payload) + tool_event_by_call_id[call_id] = event_payload + yield sse_event("function_call", {"call_id": call_id, "name": name, "arguments": rendered_arguments}) + elif call_id and call_id in tool_event_by_call_id: + active_call_id = call_id + if rendered_arguments: + args_accumulator[call_id] = args_accumulator.get(call_id, "") + rendered_arguments + tool_event_by_call_id[call_id]["arguments"] = args_accumulator[call_id] + yield sse_event("function_call", { + "call_id": call_id, + "name": tool_event_by_call_id[call_id].get("name"), + "arguments": args_accumulator[call_id], + }) + elif active_call_id and rendered_arguments: + args_accumulator[active_call_id] = args_accumulator.get(active_call_id, "") + rendered_arguments + if active_call_id in tool_event_by_call_id: + tool_event_by_call_id[active_call_id]["arguments"] = args_accumulator[active_call_id] + yield sse_event("function_call", { + "call_id": active_call_id, + "name": tool_event_by_call_id[active_call_id].get("name"), + "arguments": args_accumulator[active_call_id], + }) + + elif content_type == "mcp_server_tool_call": + call_id = content.get("call_id") or None + name = content.get("tool_name") or content.get("name") or None + arguments = content.get("arguments", "") + rendered_arguments = json.dumps(arguments, ensure_ascii=False) if isinstance(arguments, (dict, list)) else str(arguments) + if name and call_id and call_id not in tool_event_by_call_id: + args_accumulator[call_id] = rendered_arguments + event_payload = {"call_id": call_id, "name": name, "arguments": rendered_arguments, "result": None} + tool_events.append(event_payload) + tool_event_by_call_id[call_id] = event_payload + yield sse_event("function_call", {"call_id": call_id, "name": name, "arguments": rendered_arguments}) + + elif content_type in ("function_result", "mcp_server_tool_result"): + call_id = content.get("call_id") + result = content.get("result") if content_type == "function_result" else content.get("output") + converted = convert_content_items(content.get("items")) + content_items = converted if any(item["type"] == "image" for item in converted) else None + rendered_result = render_tool_result(result) + accumulated_args = args_accumulator.get(call_id, "") if call_id else "" + if call_id in tool_event_by_call_id: + tool_event_by_call_id[call_id]["result"] = rendered_result + tool_event_by_call_id[call_id]["arguments"] = accumulated_args + active_call_id = None + yield sse_event("function_result", { + key: value for key, value in { + "call_id": call_id, + "result": rendered_result, + "arguments": accumulated_args, + "content_items": content_items, + }.items() if value is not None + }) + + elif content_type == "usage": + usage = extract_usage_from_payload(content) + request_usage = merge_usage(request_usage, usage) + + if getattr(msg, "text", None): + final_text_parts.append(msg.text) + yield sse_event("text", {"content": msg.text}) + + try: + final_response = await stream.get_final_response() + if final_response and getattr(final_response, "usage_details", None): + request_usage = merge_usage(request_usage, final_response.usage_details) + except Exception: + pass + + if request_usage: + yield sse_event("usage", { + USAGE_INPUT_KEY: usage_value(request_usage, USAGE_INPUT_KEY), + USAGE_OUTPUT_KEY: usage_value(request_usage, USAGE_OUTPUT_KEY), + USAGE_TOTAL_KEY: usage_value(request_usage, USAGE_TOTAL_KEY), + }) + + yield sse_event("done", {}) + stream_agent_response._last_result = { # type: ignore[attr-defined] + "text": "".join(final_text_parts).strip(), + "tool_events": tool_events, + "usage": request_usage, + } + + +__all__ = [ + "USAGE_INPUT_KEY", + "USAGE_OUTPUT_KEY", + "USAGE_TOTAL_KEY", + "convert_content_items", + "create_usage", + "extract_usage_from_payload", + "is_context_length_error", + "is_retryable_error", + "merge_usage", + "render_tool_result", + "sse_event", + "stream_agent_response", + "usage_value", +] \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index 03b7214..af6fa57 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,6 +5,9 @@ import os +import pytest +from fastapi.testclient import TestClient + # Auth disabled for tests by default os.environ.setdefault("AUTH_DISABLED", "true") # Set a dummy SQL connection string to avoid startup errors (won't actually connect) @@ -17,3 +20,42 @@ os.environ.setdefault("AZURE_OPENAI_MODEL", "gpt-4o") os.environ.setdefault("AZURE_OPENAI_API_KEY", "test-key") os.environ.setdefault("AZURE_OPENAI_API_VERSION", "2024-02-15-preview") + + +@pytest.fixture +def client(): + """FastAPI TestClient with clean in-memory sessions.""" + from main import _sessions, app + + _sessions.clear() + with TestClient(app) as test_client: + yield test_client + _sessions.clear() + + +@pytest.fixture +def skills_client(tmp_path, monkeypatch): + """FastAPI TestClient with skills storage redirected to a temporary directory.""" + import main as main_module + from main import _sessions, app + + monkeypatch.setattr(main_module, "_get_skills_dir", lambda: tmp_path) + _sessions.clear() + with TestClient(app) as test_client: + yield test_client, tmp_path + _sessions.clear() + + +@pytest.fixture +def make_skill(): + def _make_skill(tmp_path, name: str, description: str = "A test skill", content: str = "# Content\nHello."): + skill_dir = tmp_path / name + skill_dir.mkdir() + skill_file = skill_dir / "SKILL.md" + skill_file.write_text( + f'---\nname: {name}\ndescription: "{description}"\n---\n\n{content}\n', + encoding="utf-8", + ) + return skill_dir + + return _make_skill diff --git a/tests/test_api.py b/tests/test_api.py index 69be6d5..4a5dda4 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -2,13 +2,11 @@ import os from types import SimpleNamespace -import pytest os.environ.setdefault("AUTH_DISABLED", "true") os.environ.setdefault("AZURE_SQL_CONNECTIONSTRING", "") -from fastapi.testclient import TestClient -from main import _sessions, app +from main import _sessions from prompt_config import load_agents_yaml @@ -16,14 +14,6 @@ def faa_profile_name() -> str: return str(load_agents_yaml()["profiles"]["faa"].get("name", "faa")) -@pytest.fixture -def client(): - _sessions.clear() - with TestClient(app) as test_client: - yield test_client - _sessions.clear() - - def test_health_endpoint(client): resp = client.get("/api/health") assert resp.status_code == 200 @@ -115,8 +105,119 @@ def test_create_session_invalid_profile(client): assert resp.status_code in (400, 500) +def test_standard_profile_session_preserves_runtime_request_and_response(client, monkeypatch): + import session_orchestration + + calls: dict[str, object] = {} + + class DummySession: + def to_dict(self): + return {"items": []} + + def fake_create_chat_runtime(**kwargs): + calls["runtime"] = kwargs + return SimpleNamespace( + agent=object(), + session=DummySession(), + tools=kwargs.get("function_tools", []), + prompt_manifest={}, + prompt_logical_profile="search", + ) + + async def fake_connect_mcp_servers(configs, *, user_token=None): + calls["mcp_configs"] = configs + calls["user_token"] = user_token + return [], [] + + monkeypatch.setattr(session_orchestration, "create_chat_runtime", fake_create_chat_runtime) + monkeypatch.setattr(session_orchestration, "connect_mcp_servers", fake_connect_mcp_servers) + + resp = client.post( + "/api/sessions", + json={ + "profile_id": "search", + "user_profile": {"name": "Avery", "preferences": "brief", "notes": "pilot"}, + }, + ) + + assert resp.status_code == 201 + data = resp.json() + assert data["profile_id"] == "search" + assert data["profile_name"] == "Search Agent" + assert data["tools_loaded"] == ["get_user_profile", "save_user_profile"] + assert data["skills_loaded"] == [] + assert data["search_context"] is True + assert data["mcp_results"] == [] + assert data["used_profile_override"] is False + assert data["override_updated_at"] is None + + runtime_kwargs = calls["runtime"] + assert runtime_kwargs["chat_profile"] == "Search Agent" + assert "Known User Profile" in runtime_kwargs["extra_instructions"] + assert runtime_kwargs["mcp_servers"] == [] + assert data["session_id"] in _sessions + + +def test_custom_session_preserves_runtime_request_and_response(client, monkeypatch): + import session_orchestration + + calls: dict[str, object] = {} + + class DummySession: + def to_dict(self): + return {"items": []} + + def fake_create_chat_runtime(**kwargs): + calls["runtime"] = kwargs + return SimpleNamespace( + agent=object(), + session=DummySession(), + tools=kwargs.get("function_tools", []), + prompt_manifest={}, + prompt_logical_profile="custom", + ) + + async def fake_connect_mcp_servers(configs, *, user_token=None): + return [], [] + + monkeypatch.setattr(session_orchestration, "create_chat_runtime", fake_create_chat_runtime) + monkeypatch.setattr(session_orchestration, "connect_mcp_servers", fake_connect_mcp_servers) + + resp = client.post( + "/api/sessions", + json={ + "profile_id": "custom", + "custom_name": "Planner", + "custom_prompt": "Plan carefully.", + "custom_tools": ["get_user_profile"], + "custom_search_context": True, + "custom_temperature": 0.7, + "custom_skills": [], + "mcp_servers": [], + "user_profile": {"name": "Avery", "preferences": "brief", "notes": "pilot"}, + }, + ) + + assert resp.status_code == 201 + data = resp.json() + assert data["profile_id"] == "custom" + assert data["profile_name"] == "Planner" + assert data["tools_loaded"] == ["get_user_profile"] + assert data["skills_loaded"] == [] + assert data["search_context"] is True + assert data["mcp_results"] == [] + + runtime_kwargs = calls["runtime"] + assert runtime_kwargs["custom_name"] == "Planner" + assert runtime_kwargs["custom_instructions"] == "Plan carefully." + assert runtime_kwargs["temperature"] == 0.7 + assert runtime_kwargs["enable_search_context"] is True + assert runtime_kwargs["custom_skills"] is None + assert "Known User Profile" in runtime_kwargs["extra_instructions"] + + def test_builtin_profile_override_session_preserves_canonical_name(client, monkeypatch): - import main as main_module + import session_orchestration class DummySession: def to_dict(self): @@ -136,8 +237,8 @@ def fake_create_chat_runtime(**kwargs): async def fake_connect_mcp_servers(configs, *, user_token=None): return [], [] - monkeypatch.setattr(main_module, "create_chat_runtime", fake_create_chat_runtime) - monkeypatch.setattr(main_module, "connect_mcp_servers", fake_connect_mcp_servers) + monkeypatch.setattr(session_orchestration, "create_chat_runtime", fake_create_chat_runtime) + monkeypatch.setattr(session_orchestration, "connect_mcp_servers", fake_connect_mcp_servers) resp = client.post( "/api/sessions", @@ -187,6 +288,107 @@ def test_builtin_profile_override_rejects_name_change(client): assert "cannot change the agent name" in resp.json()["detail"] +def test_custom_session_drops_unknown_skills(client, monkeypatch, caplog): + """Regression: deleting a skill referenced by a saved custom agent must not + block the session from loading. Unknown skills are silently dropped with a + warning so the agent remains usable.""" + import logging + + import session_orchestration + + calls: dict[str, object] = {} + + class DummySession: + def to_dict(self): + return {"items": []} + + def fake_create_chat_runtime(**kwargs): + calls["runtime"] = kwargs + return SimpleNamespace( + agent=object(), + session=DummySession(), + tools=kwargs.get("function_tools", []), + prompt_manifest={}, + prompt_logical_profile="custom", + ) + + async def fake_connect_mcp_servers(configs, *, user_token=None): + return [], [] + + monkeypatch.setattr(session_orchestration, "create_chat_runtime", fake_create_chat_runtime) + monkeypatch.setattr(session_orchestration, "connect_mcp_servers", fake_connect_mcp_servers) + + with caplog.at_level(logging.WARNING): + resp = client.post( + "/api/sessions", + json={ + "profile_id": "custom", + "custom_name": "Planner", + "custom_prompt": "Plan carefully.", + "custom_tools": [], + "custom_search_context": False, + "custom_skills": ["__deleted_skill__"], + "mcp_servers": [], + }, + ) + + assert resp.status_code == 201 + assert resp.json()["skills_loaded"] == [] + assert calls["runtime"]["custom_skills"] is None + assert any("__deleted_skill__" in record.message for record in caplog.records) + + +def test_builtin_profile_override_drops_unknown_skills(client, monkeypatch, caplog): + """Regression: a built-in override that still references a deleted skill + must load — the unknown skill is dropped with a warning.""" + import logging + + import session_orchestration + + calls: dict[str, object] = {} + + class DummySession: + def to_dict(self): + return {"items": []} + + def fake_create_chat_runtime(**kwargs): + calls["runtime"] = kwargs + return SimpleNamespace( + agent=object(), + session=DummySession(), + tools=kwargs.get("function_tools", []), + prompt_manifest={}, + prompt_logical_profile="custom", + ) + + async def fake_connect_mcp_servers(configs, *, user_token=None): + return [], [] + + monkeypatch.setattr(session_orchestration, "create_chat_runtime", fake_create_chat_runtime) + monkeypatch.setattr(session_orchestration, "connect_mcp_servers", fake_connect_mcp_servers) + + with caplog.at_level(logging.WARNING): + resp = client.post( + "/api/sessions", + json={ + "profile_id": "faa", + "profile_override": { + "description": "Local FAA tuning", + "custom_prompt": "Override prompt", + "custom_tools": [], + "custom_search_context": False, + "custom_skills": ["__deleted_skill__"], + "mcp_servers": [], + "override_updated_at": "2026-05-06T12:00:00.000Z", + }, + }, + ) + + assert resp.status_code == 201 + assert calls["runtime"]["custom_skills"] is None + assert any("__deleted_skill__" in record.message for record in caplog.records) + + def test_delete_session_not_found(client): resp = client.delete("/api/sessions/nonexistent-session-id") assert resp.status_code == 404 @@ -224,7 +426,23 @@ def test_custom_session_binds_all_selected_tools(client): ) assert resp.status_code == 201 - session_id = resp.json()["session_id"] + response_data = resp.json() + assert { + "session_id", + "profile_id", + "profile_name", + "tools_loaded", + "skills_loaded", + "search_context", + "mcp_results", + }.issubset(response_data) + assert response_data["profile_id"] == "custom" + assert response_data["profile_name"] == "All Tools Custom" + assert response_data["tools_loaded"] == ["get_user_profile", "save_user_profile"] + assert response_data["skills_loaded"] == [] + assert response_data["search_context"] is False + assert response_data["mcp_results"] == [] + session_id = response_data["session_id"] session_data = _sessions[session_id] tool_names = {getattr(tool, "name", None) or getattr(tool, "__name__", None) for tool in session_data.tools} assert {"get_user_profile", "save_user_profile"}.issubset(tool_names) diff --git a/tests/test_image_input.py b/tests/test_image_input.py index 841245b..ed110c0 100644 --- a/tests/test_image_input.py +++ b/tests/test_image_input.py @@ -10,12 +10,14 @@ from fastapi import UploadFile from main import ( - _validate_image_magic_bytes, - _validate_uploaded_images, ALLOWED_IMAGE_MIMES, MAX_IMAGE_SIZE_BYTES, MAX_IMAGES_PER_MESSAGE, ) +from validators import ( + validate_image_magic_bytes as _validate_image_magic_bytes, + validate_uploaded_images as _validate_uploaded_images, +) # --------------------------------------------------------------------------- @@ -266,20 +268,20 @@ def test_six_images_rejected(self) -> None: class TestStreamAgentResponseSignature: - """Verify _stream_agent_response accepts contents: list[Content].""" + """Verify stream_agent_response accepts contents: list[Content].""" def test_signature_has_contents_param(self) -> None: import inspect - from main import _stream_agent_response + from streaming import stream_agent_response - sig = inspect.signature(_stream_agent_response) + sig = inspect.signature(stream_agent_response) assert "contents" in sig.parameters def test_is_async_generator(self) -> None: import inspect - from main import _stream_agent_response + from streaming import stream_agent_response - assert inspect.isasyncgenfunction(_stream_agent_response) + assert inspect.isasyncgenfunction(stream_agent_response) class TestSendMessageEndpoint: @@ -290,7 +292,7 @@ def test_send_message_has_validation(self) -> None: import main source = inspect.getsource(main.send_message) - assert "_validate_uploaded_images" in source + assert "validate_uploaded_images" in source def test_send_message_builds_contents_list(self) -> None: import inspect diff --git a/tests/test_provider_routing.py b/tests/test_provider_routing.py index c790046..a19a8cd 100644 --- a/tests/test_provider_routing.py +++ b/tests/test_provider_routing.py @@ -1,6 +1,7 @@ """Tests for dynamic LLM provider routing via OpenAIChatCompletionClient.""" import pytest +from types import SimpleNamespace from unittest.mock import patch from agent_framework.openai import OpenAIChatCompletionClient @@ -36,3 +37,51 @@ def test_azure_openai_provider_instantiation(self) -> None: """Client instantiates with only AZURE_OPENAI_* env vars.""" client = OpenAIChatCompletionClient() assert client is not None + + +def test_create_chat_runtime_binds_profile_tools_and_creates_session(monkeypatch) -> None: + """Runtime creation should preserve profile tool filtering and agent session creation.""" + import agent_factory + + calls: dict[str, object] = {} + + class FakeAgent: + context_providers: list[object] = [] + + def create_session(self): + return {"session": "created"} + + class FakeClient: + def as_agent(self, **kwargs): + calls["as_agent"] = kwargs + return FakeAgent() + + allowed_tool = SimpleNamespace(name="allowed_tool") + denied_tool = SimpleNamespace(name="denied_tool") + + monkeypatch.setattr(agent_factory, "_build_openai_clients", lambda: (FakeClient(), FakeClient())) + monkeypatch.setattr(agent_factory, "load_agent_profile", lambda chat_profile=None: SimpleNamespace( + name="Profile Name", + description="Profile description", + system_prompt="System prompt", + tool_names=["allowed_tool"], + logical_profile="profile-key", + search_context=False, + temperature=0.4, + skills=[], + )) + + runtime = agent_factory.create_chat_runtime( + chat_profile="Profile Name", + function_tools=[allowed_tool, denied_tool], + ) + + assert runtime.session == {"session": "created"} + assert runtime.tools == [allowed_tool] + assert runtime.prompt_logical_profile == "profile-key" + agent_kwargs = calls["as_agent"] + assert agent_kwargs["name"] == "Profile_Name" + assert agent_kwargs["instructions"] == "System prompt" + assert agent_kwargs["description"] == "Profile description" + assert agent_kwargs["tools"] == [allowed_tool] + assert agent_kwargs["default_options"] == {"temperature": 0.4} diff --git a/tests/test_retry_logic.py b/tests/test_retry_logic.py index f22570d..a40328e 100644 --- a/tests/test_retry_logic.py +++ b/tests/test_retry_logic.py @@ -1,9 +1,9 @@ """Tests for request retry classification and session behavior.""" import pytest -from unittest.mock import patch, MagicMock +from unittest.mock import MagicMock from openai.lib.azure import AsyncAzureOpenAI -from main import _is_retryable_error +from streaming import is_retryable_error from agent_framework._types import UsageDetails @@ -13,6 +13,21 @@ def _usage_value(usage: UsageDetails, key: str): return getattr(usage, key) +def _make_session(main_module, session_id: str = "test-session"): + return main_module.SessionData( + session_id=session_id, + user_id="test-user", + profile_id="test-profile", + profile_name="Test Profile", + agent=MagicMock(), + agent_session=MagicMock(), + tools=[], + eval_trace_logger=MagicMock(), + prompt_manifest={}, + prompt_logical_profile="test-profile", + ) + + class TestAzureClientRetryConfiguration: """Test representative Azure OpenAI client retry configuration.""" @@ -55,11 +70,11 @@ class TestContextHandling: """Test stream helper inputs used for session-backed context handling.""" def test_stream_agent_response_accepts_contents(self) -> None: - """_stream_agent_response should accept contents: list[Content] parameter.""" - from main import _stream_agent_response + """stream_agent_response should accept contents: list[Content] parameter.""" + from streaming import stream_agent_response import inspect - sig = inspect.signature(_stream_agent_response) + sig = inspect.signature(stream_agent_response) contents_param = sig.parameters.get("contents") assert contents_param is not None annotation_str = str(contents_param.annotation) @@ -67,52 +82,52 @@ def test_stream_agent_response_accepts_contents(self) -> None: class TestIsRetryableError: - """Test the _is_retryable_error helper function.""" + """Test the is_retryable_error helper function.""" def test_detects_429_in_message(self) -> None: """Should detect 429 status code in error message.""" error = Exception("Error: 429 Too Many Requests") - assert _is_retryable_error(error) is True + assert is_retryable_error(error) is True def test_detects_too_many_requests_message(self) -> None: """Should detect 'Too Many Requests' in error message.""" error = Exception("Server returned Too Many Requests") - assert _is_retryable_error(error) is True + assert is_retryable_error(error) is True def test_detects_rate_limit_in_message(self) -> None: """Should detect 'rate_limit' in error message (case insensitive).""" error = Exception("Request failed due to rate_limit exceeded") - assert _is_retryable_error(error) is True + assert is_retryable_error(error) is True def test_detects_rate_limit_uppercase(self) -> None: """Should detect 'RATE_LIMIT' in error message (case insensitive).""" error = Exception("RATE_LIMIT error occurred") - assert _is_retryable_error(error) is True + assert is_retryable_error(error) is True def test_detects_capacity_in_message(self) -> None: """Should detect 'capacity' in error message (case insensitive).""" error = Exception("Service at capacity, please retry later") - assert _is_retryable_error(error) is True + assert is_retryable_error(error) is True def test_detects_capacity_uppercase(self) -> None: """Should detect 'CAPACITY' in error message (case insensitive).""" error = Exception("CAPACITY exceeded") - assert _is_retryable_error(error) is True + assert is_retryable_error(error) is True def test_does_not_match_other_errors(self) -> None: """Should not match unrelated errors.""" error = Exception("Connection timeout") - assert _is_retryable_error(error) is False + assert is_retryable_error(error) is False def test_does_not_match_500_error(self) -> None: """Should not match 500 internal server error.""" error = Exception("Error: 500 Internal Server Error") - assert _is_retryable_error(error) is False + assert is_retryable_error(error) is False def test_does_not_match_empty_message(self) -> None: """Should not match empty error message.""" error = Exception("") - assert _is_retryable_error(error) is False + assert is_retryable_error(error) is False class TestRetryLogicBehavior: @@ -121,12 +136,12 @@ class TestRetryLogicBehavior: def test_retryable_error_is_classified(self) -> None: """429 errors should be classified as retryable.""" rate_limit_error = Exception("429 Too Many Requests") - assert _is_retryable_error(rate_limit_error) is True + assert is_retryable_error(rate_limit_error) is True def test_non_retryable_error_is_not_classified(self) -> None: """Non-429 errors should not be classified as retryable.""" regular_error = Exception("Connection timeout") - assert _is_retryable_error(regular_error) is False + assert is_retryable_error(regular_error) is False def test_rate_limit_error_type_detection(self) -> None: """Should detect RateLimitError type in exception.""" @@ -135,17 +150,17 @@ class RateLimitError(Exception): pass error = RateLimitError("Rate limit exceeded") - assert _is_retryable_error(error) is True + assert is_retryable_error(error) is True def test_azure_openai_capacity_error(self) -> None: """Should detect Azure OpenAI capacity errors.""" error = Exception("The server is currently at capacity. Please try again later.") - assert _is_retryable_error(error) is True + assert is_retryable_error(error) is True def test_openai_rate_limit_header_error(self) -> None: """Should detect OpenAI rate limit errors with header info.""" error = Exception("Rate limit reached for requests. Please retry after 60 seconds.") - assert _is_retryable_error(error) is True + assert is_retryable_error(error) is True class TestSessionManagement: @@ -163,13 +178,17 @@ def test_delete_session_exists(self) -> None: assert hasattr(main, 'delete_session') assert callable(main.delete_session) - def test_delete_session_cleans_up(self) -> None: + def test_delete_session_cleans_up(self, client) -> None: """delete_session should remove session from store.""" - import inspect import main - source = inspect.getsource(main.delete_session) - assert '_sessions.pop' in source + main._sessions.clear() + main._sessions["cleanup-test"] = _make_session(main, "cleanup-test") + + response = client.delete("/api/sessions/cleanup-test") + + assert response.status_code == 204 + assert "cleanup-test" not in main._sessions def test_new_session_created_each_chat_start(self) -> None: """Verify that agent.create_session() returns unique sessions.""" @@ -271,11 +290,11 @@ def test_usage_details_empty_initialization(self) -> None: assert _usage_value(usage, "total_token_count") is None def test_stream_agent_response_is_async_generator(self) -> None: - """_stream_agent_response should be an async generator.""" - from main import _stream_agent_response + """stream_agent_response should be an async generator.""" + from streaming import stream_agent_response import inspect - assert inspect.isasyncgenfunction(_stream_agent_response) + assert inspect.isasyncgenfunction(stream_agent_response) def test_session_data_initializes_usage(self) -> None: """SessionData should initialize usage tracking.""" @@ -283,21 +302,31 @@ def test_session_data_initializes_usage(self) -> None: assert hasattr(main, 'SessionData') - def test_delete_session_logs_usage(self) -> None: + def test_delete_session_logs_usage(self, client, caplog) -> None: """delete_session should log token usage on cleanup.""" - import inspect import main - source = inspect.getsource(main.delete_session) - assert 'Token usage' in source + main._sessions.clear() + main._sessions["usage-test"] = _make_session(main, "usage-test") - def test_send_message_logs_token_usage(self) -> None: - """send_message should log token usage.""" - import inspect - import main + with caplog.at_level("INFO", logger="main"): + response = client.delete("/api/sessions/usage-test") - source = inspect.getsource(main.send_message) - assert 'Request token usage' in source + assert response.status_code == 204 + assert any("Token usage" in record.message for record in caplog.records) + + def test_usage_accumulation_tracks_request_tokens(self) -> None: + """merge helper should accumulate request token usage.""" + from streaming import merge_usage + + base = UsageDetails(input_token_count=1, output_token_count=2, total_token_count=3) + increment = UsageDetails(input_token_count=4, output_token_count=5, total_token_count=9) + + merged = merge_usage(base, increment) + + assert _usage_value(merged, "input_token_count") == 5 + assert _usage_value(merged, "output_token_count") == 7 + assert _usage_value(merged, "total_token_count") == 12 class TestInputValidation: @@ -311,11 +340,17 @@ def test_max_user_input_chars_defined(self) -> None: assert isinstance(main.DEFAULT_MAX_USER_INPUT_CHARS, int) assert main.DEFAULT_MAX_USER_INPUT_CHARS == 25000 - def test_send_message_validates_input_length(self) -> None: + def test_send_message_validates_input_length(self, client) -> None: """send_message should reject messages exceeding the configured max length.""" - import inspect import main - source = inspect.getsource(main.send_message) - assert "DEFAULT_MAX_USER_INPUT_CHARS" in source - assert "exceeds maximum length" in source + main._sessions.clear() + main._sessions["long-input-test"] = _make_session(main, "long-input-test") + + response = client.post( + "/api/sessions/long-input-test/messages", + json={"content": "x" * (main.DEFAULT_MAX_USER_INPUT_CHARS + 1)}, + ) + + assert response.status_code == 400 + assert "exceeds maximum length" in response.json()["detail"] diff --git a/tests/test_session_orchestration.py b/tests/test_session_orchestration.py new file mode 100644 index 0000000..c367465 --- /dev/null +++ b/tests/test_session_orchestration.py @@ -0,0 +1,20 @@ +"""Tests for session orchestration helpers.""" + +from session_orchestration import sanitize_mcp_result_error + + +def test_sanitize_mcp_result_error_removes_credentials_and_tokens(): + error = ( + "GET https://example.test/mcp?api_key=secret&code=abc failed " + "Authorization: Bearer eyJsecret token password=hidden connectionString=Server=tcp" + ) + + sanitized = sanitize_mcp_result_error(error) + + assert "secret" not in sanitized + assert "Bearer" not in sanitized + assert "api_key=" not in sanitized + assert "code=" not in sanitized + assert "password=" not in sanitized + assert "connectionString=" not in sanitized + assert "https://example.test/mcp" in sanitized \ No newline at end of file diff --git a/tests/test_skills_api.py b/tests/test_skills_api.py index da018b9..106ee7c 100644 --- a/tests/test_skills_api.py +++ b/tests/test_skills_api.py @@ -1,39 +1,10 @@ """Tests for Skills CRUD API endpoints.""" import os -import pytest os.environ.setdefault("AUTH_DISABLED", "true") os.environ.setdefault("AZURE_SQL_CONNECTIONSTRING", "") -from fastapi.testclient import TestClient - - -@pytest.fixture -def skills_client(tmp_path, monkeypatch): - """TestClient with skills_dir redirected to a temporary directory.""" - import main as main_module - - monkeypatch.setattr(main_module, "_get_skills_dir", lambda: tmp_path) - - from main import _sessions, app - _sessions.clear() - with TestClient(app) as client: - yield client, tmp_path - _sessions.clear() - - -def _make_skill(tmp_path, name: str, description: str = "A test skill", content: str = "# Content\nHello."): - """Helper: create a skill directory and SKILL.md in tmp_path.""" - skill_dir = tmp_path / name - skill_dir.mkdir() - skill_file = skill_dir / "SKILL.md" - skill_file.write_text( - f'---\nname: {name}\ndescription: "{description}"\n---\n\n{content}\n', - encoding="utf-8", - ) - return skill_dir - # --------------------------------------------------------------------------- # GET /api/skills — list @@ -46,10 +17,10 @@ def test_get_skills_list_empty(skills_client): assert resp.json() == {"skills": []} -def test_get_skills_list_populated(skills_client): +def test_get_skills_list_populated(skills_client, make_skill): client, tmp_path = skills_client - _make_skill(tmp_path, "alpha", "Alpha skill") - _make_skill(tmp_path, "beta", "Beta skill") + make_skill(tmp_path, "alpha", "Alpha skill") + make_skill(tmp_path, "beta", "Beta skill") resp = client.get("/api/skills") assert resp.status_code == 200 names = {s["name"] for s in resp.json()["skills"]} @@ -61,9 +32,9 @@ def test_get_skills_list_populated(skills_client): # GET /api/skills/{name} — single skill # --------------------------------------------------------------------------- -def test_get_skill_success(skills_client): +def test_get_skill_success(skills_client, make_skill): client, tmp_path = skills_client - _make_skill(tmp_path, "my-skill", "My description", "# Docs\nSome content.") + make_skill(tmp_path, "my-skill", "My description", "# Docs\nSome content.") resp = client.get("/api/skills/my-skill") assert resp.status_code == 200 data = resp.json() @@ -95,9 +66,9 @@ def test_create_skill_success(skills_client): assert skill_file.exists() -def test_create_skill_duplicate_returns_409(skills_client): +def test_create_skill_duplicate_returns_409(skills_client, make_skill): client, tmp_path = skills_client - _make_skill(tmp_path, "existing") + make_skill(tmp_path, "existing") payload = {"name": "existing", "description": "Another", "content": "# Content"} resp = client.post("/api/skills", json=payload) assert resp.status_code == 409 @@ -137,9 +108,9 @@ def test_create_skill_name_too_long_returns_422(skills_client): # PUT /api/skills/{name} — update # --------------------------------------------------------------------------- -def test_update_skill_success(skills_client): +def test_update_skill_success(skills_client, make_skill): client, tmp_path = skills_client - _make_skill(tmp_path, "editable", "Old description", "Old content.") + make_skill(tmp_path, "editable", "Old description", "Old content.") payload = {"description": "New description", "content": "New content."} resp = client.put("/api/skills/editable", json=payload) assert resp.status_code == 200 @@ -159,9 +130,9 @@ def test_update_skill_not_found_returns_404(skills_client): # DELETE /api/skills/{name} — delete # --------------------------------------------------------------------------- -def test_delete_skill_success(skills_client): +def test_delete_skill_success(skills_client, make_skill): client, tmp_path = skills_client - _make_skill(tmp_path, "to-delete") + make_skill(tmp_path, "to-delete") resp = client.delete("/api/skills/to-delete") assert resp.status_code == 204 # Verify directory was removed diff --git a/tests/test_skills_manager.py b/tests/test_skills_manager.py new file mode 100644 index 0000000..b272980 --- /dev/null +++ b/tests/test_skills_manager.py @@ -0,0 +1,49 @@ +"""Tests for file-backed skill management.""" + +import pytest +from fastapi import HTTPException + +from skills_manager import SkillManager + + +def test_list_and_get_skills(tmp_path, make_skill): + make_skill(tmp_path, "alpha", "Alpha skill", "# Alpha\nBody") + manager = SkillManager(tmp_path) + + assert manager.list_summaries() == [{"name": "alpha", "description": "Alpha skill"}] + assert manager.get("alpha") == {"name": "alpha", "description": "Alpha skill", "content": "# Alpha\nBody\n"} + + +def test_create_update_and_delete_skill(tmp_path): + manager = SkillManager(tmp_path) + + created = manager.create("new-skill", "A new skill", "# Body") + assert created == {"name": "new-skill", "description": "A new skill", "content": "# Body"} + assert (tmp_path / "new-skill" / "SKILL.md").is_file() + + updated = manager.update("new-skill", "Updated", "# Updated") + assert updated == {"name": "new-skill", "description": "Updated", "content": "# Updated"} + + manager.delete("new-skill") + assert not (tmp_path / "new-skill").exists() + + +def test_skill_manager_errors_match_api_contract(tmp_path, make_skill): + manager = SkillManager(tmp_path) + make_skill(tmp_path, "existing") + + with pytest.raises(HTTPException) as invalid: + manager.create("Bad-Name", "desc", "body") + assert invalid.value.status_code == 422 + + with pytest.raises(HTTPException) as duplicate: + manager.create("existing", "desc", "body") + assert duplicate.value.status_code == 409 + + with pytest.raises(HTTPException) as missing: + manager.get("missing") + assert missing.value.status_code == 404 + + with pytest.raises(HTTPException) as traversal: + manager.skill_path("../etc") + assert traversal.value.status_code == 400 \ No newline at end of file diff --git a/tests/test_streaming.py b/tests/test_streaming.py new file mode 100644 index 0000000..6e96f29 --- /dev/null +++ b/tests/test_streaming.py @@ -0,0 +1,79 @@ +"""Tests for streaming, usage, and error helpers.""" + +import json + +from agent_framework._types import UsageDetails + +from streaming import ( + USAGE_INPUT_KEY, + USAGE_OUTPUT_KEY, + USAGE_TOTAL_KEY, + convert_content_items, + create_usage, + extract_usage_from_payload, + is_context_length_error, + is_retryable_error, + merge_usage, + render_tool_result, + sse_event, + usage_value, +) + + +def test_usage_helpers_create_extract_and_merge_counts(): + first = create_usage(input_token_count=1, output_token_count=2, total_token_count=3) + second = extract_usage_from_payload({"usage": {USAGE_INPUT_KEY: 4, USAGE_OUTPUT_KEY: 5, USAGE_TOTAL_KEY: 9}}) + + merged = merge_usage(first, second) + + assert usage_value(merged, USAGE_INPUT_KEY) == 5 + assert usage_value(merged, USAGE_OUTPUT_KEY) == 7 + assert usage_value(merged, USAGE_TOTAL_KEY) == 12 + assert merge_usage(None, second) is second + assert extract_usage_from_payload({}) is None + + +def test_usage_value_handles_dicts_and_none(): + assert usage_value(None, USAGE_INPUT_KEY) == 0 + assert usage_value({USAGE_INPUT_KEY: 7}, USAGE_INPUT_KEY) == 7 + assert usage_value(UsageDetails(input_token_count=8), USAGE_INPUT_KEY) == 8 + + +def test_sse_event_preserves_event_name_and_json_payload(): + rendered = sse_event("text", {"content": "hello"}) + assert rendered.startswith("event: text\n") + assert rendered.endswith("\n\n") + payload = json.loads(rendered.split("data: ", 1)[1]) + assert payload == {"content": "hello"} + + +def test_error_classifiers_match_current_behavior(): + assert is_retryable_error(Exception("429 Too Many Requests")) is True + assert is_retryable_error(Exception("service at capacity")) is True + assert is_retryable_error(Exception("Connection timeout")) is False + + assert is_context_length_error(Exception("maximum context length exceeded")) is True + assert is_context_length_error(Exception("too many tokens")) is True + assert is_context_length_error(Exception("ordinary failure")) is False + + +def test_convert_content_items_filters_and_normalizes_images(): + converted = convert_content_items([ + {"type": "text", "text": "hello"}, + {"type": "data", "uri": "data:image/png;base64,abc123"}, + {"type": "data", "uri": "data:application/pdf;base64,nope"}, + {"type": "image", "data": "xyz", "mimeType": "image/jpeg"}, + "skip me", + ]) + + assert converted == [ + {"type": "text", "text": "hello"}, + {"type": "image", "data": "abc123", "mimeType": "image/png"}, + {"type": "image", "data": "xyz", "mimeType": "image/jpeg"}, + ] + + +def test_render_tool_result_matches_current_display_behavior(): + assert render_tool_result([{"type": "text", "text": "a"}, {"type": "text", "text": "b"}]) == "a\nb" + assert render_tool_result({"ok": True}) == '{"ok": true}' + assert render_tool_result(None) == "" \ No newline at end of file diff --git a/tests/test_validators.py b/tests/test_validators.py new file mode 100644 index 0000000..55caa75 --- /dev/null +++ b/tests/test_validators.py @@ -0,0 +1,133 @@ +"""Tests for shared validation helpers.""" + +import io + +import pytest +from fastapi import HTTPException, UploadFile + +from validators import ( + ALLOWED_IMAGE_MIMES, + MAX_IMAGE_SIZE_BYTES, + MAX_IMAGES_PER_MESSAGE, + available_skill_names, + filter_known_skill_names, + validate_custom_name, + validate_http_mcp_servers, + validate_image_magic_bytes, + validate_prompt, + validate_temperature, + validate_tool_names, + validate_uploaded_images, +) + + +def _upload(mime: str, filename: str, data: bytes) -> UploadFile: + return UploadFile(filename=filename, file=io.BytesIO(data), headers={"content-type": mime}) + + +VALID_PNG = b"\x89PNG\r\n\x1a\n" + b"\x00" * 16 +VALID_JPEG = b"\xff\xd8\xff\xe0" + b"\x00" * 16 +VALID_WEBP = b"RIFF" + b"\x00\x00\x00\x00" + b"WEBP" + b"\x00" * 16 + + +def test_image_constants_preserved(): + assert ALLOWED_IMAGE_MIMES == {"image/jpeg", "image/png", "image/gif", "image/webp"} + assert MAX_IMAGE_SIZE_BYTES == 400 * 1024 * 1024 + assert MAX_IMAGES_PER_MESSAGE == 5 + + +def test_validate_image_magic_bytes_supported_types(): + assert validate_image_magic_bytes(VALID_JPEG, "image/jpeg") is True + assert validate_image_magic_bytes(VALID_PNG, "image/png") is True + assert validate_image_magic_bytes(b"GIF87a" + b"\x00", "image/gif") is True + assert validate_image_magic_bytes(b"GIF89a" + b"\x00", "image/gif") is True + assert validate_image_magic_bytes(VALID_WEBP, "image/webp") is True + assert validate_image_magic_bytes(b"not an image", "image/png") is False + + +def test_validate_uploaded_images_rejects_bad_mime_size_count_and_corruption(): + assert validate_uploaded_images([_upload("image/png", "ok.png", VALID_PNG)], [VALID_PNG]) is None + + too_many = [_upload("image/png", f"{index}.png", VALID_PNG) for index in range(6)] + assert "Maximum of 5 images" in (validate_uploaded_images(too_many, [VALID_PNG] * 6) or "") + + assert "Only image files" in (validate_uploaded_images([_upload("text/plain", "bad.txt", b"x")], [b"x"]) or "") + + huge = b"\xff\xd8\xff" + b"\x00" * (MAX_IMAGE_SIZE_BYTES + 1) + assert "maximum file size" in (validate_uploaded_images([_upload("image/jpeg", "huge.jpg", huge)], [huge]) or "") + + assert "could not be processed" in ( + validate_uploaded_images([_upload("image/png", "corrupt.png", b"nope")], [b"nope"]) or "" + ) + + +def test_validate_temperature_accepts_range_and_rejects_invalid_values(): + assert validate_temperature(None) is None + assert validate_temperature("0.7") == 0.7 + assert validate_temperature(0) == 0.0 + assert validate_temperature(2) == 2.0 + + with pytest.raises(HTTPException) as not_number: + validate_temperature("warm") + assert not_number.value.status_code == 400 + assert "must be a number" in not_number.value.detail + + with pytest.raises(HTTPException) as out_of_range: + validate_temperature(2.1) + assert out_of_range.value.status_code == 400 + assert "between 0.0 and 2.0" in out_of_range.value.detail + + +def test_validate_custom_name_and_prompt_bounds(): + assert validate_custom_name(" Agent Name ") == "Agent Name" + assert validate_prompt(" instructions ", max_chars=20) == "instructions" + + with pytest.raises(HTTPException): + validate_custom_name("") + with pytest.raises(HTTPException): + validate_custom_name("x" * 101) + with pytest.raises(HTTPException): + validate_prompt("", max_chars=20) + with pytest.raises(HTTPException): + validate_prompt("x" * 21, max_chars=20) + + +def test_validate_tool_and_skill_names(): + known_tools = {"get_user_profile", "save_user_profile"} + assert validate_tool_names(["get_user_profile"], known_tools) == ["get_user_profile"] + + with pytest.raises(HTTPException) as bad_tools: + validate_tool_names(["missing_tool"], known_tools) + assert bad_tools.value.detail == "Unknown tools: missing_tool" + + with pytest.raises(HTTPException): + validate_tool_names("get_user_profile", known_tools) # type: ignore[arg-type] + + kept, dropped = filter_known_skill_names(["table-usage", "unknown"], {"table-usage"}) + assert kept == ["table-usage"] + assert dropped == ["unknown"] + with pytest.raises(HTTPException): + filter_known_skill_names("table-usage", {"table-usage"}) # type: ignore[arg-type] + + +def test_available_skill_names_reads_skill_provider(tmp_path, make_skill): + make_skill(tmp_path, "alpha", "Alpha") + assert available_skill_names(tmp_path) == {"alpha"} + assert available_skill_names(tmp_path / "missing") == set() + + +def test_validate_http_mcp_servers_accepts_only_request_http_servers(): + raw = [{"name": "docs", "transport": "http", "url": "https://example.test/mcp"}] + assert validate_http_mcp_servers(raw, override=False) == raw + + with pytest.raises(HTTPException) as bad_container: + validate_http_mcp_servers({}, override=False) # type: ignore[arg-type] + assert bad_container.value.detail == "mcp_servers must be a list" + + with pytest.raises(HTTPException) as bad_transport: + validate_http_mcp_servers([{"name": "local", "transport": "stdio"}], override=False) + assert "Custom agents only support 'http' MCP servers" in bad_transport.value.detail + + with pytest.raises(HTTPException) as missing_url: + validate_http_mcp_servers([{"name": "docs", "transport": "http"}], override=True) + assert "requires a 'url'" in missing_url.value.detail \ No newline at end of file diff --git a/validators.py b/validators.py new file mode 100644 index 0000000..da75dee --- /dev/null +++ b/validators.py @@ -0,0 +1,150 @@ +"""Validation helpers for request, tool, skill, and image handling.""" + +from pathlib import Path +from typing import Optional + +from fastapi import HTTPException, UploadFile + +ALLOWED_IMAGE_MIMES = {"image/jpeg", "image/png", "image/gif", "image/webp"} +MAX_IMAGE_SIZE_BYTES = 400 * 1024 * 1024 +MAX_IMAGES_PER_MESSAGE = 5 + +_MAGIC_BYTES: dict[str, list[bytes]] = { + "image/jpeg": [b"\xff\xd8\xff"], + "image/png": [b"\x89PNG\r\n\x1a\n"], + "image/gif": [b"GIF87a", b"GIF89a"], + "image/webp": [], +} + + +def validate_image_magic_bytes(data: bytes, claimed_mime: str) -> bool: + if not data: + return False + if claimed_mime == "image/webp": + return len(data) >= 12 and data[:4] == b"RIFF" and data[8:12] == b"WEBP" + signatures = _MAGIC_BYTES.get(claimed_mime, []) + return any(data[: len(signature)] == signature for signature in signatures) + + +def validate_uploaded_images(files: list[UploadFile], file_data: list[bytes]) -> Optional[str]: + if len(files) > MAX_IMAGES_PER_MESSAGE: + return f"Maximum of {MAX_IMAGES_PER_MESSAGE} images per message. Please reduce the number of images." + + for uploaded_file, data in zip(files, file_data): + mime = uploaded_file.content_type or "" + name = uploaded_file.filename or "uploaded file" + + if mime not in ALLOWED_IMAGE_MIMES: + return ( + "Only image files are accepted (JPEG, PNG, GIF, WebP). " + f"'{name}' is not a supported image type." + ) + + if len(data) > MAX_IMAGE_SIZE_BYTES: + return f"'{name}' exceeds the maximum file size of 400 MB." + + if not validate_image_magic_bytes(data, mime): + return f"'{name}' could not be processed. The file may be corrupt or unreadable." + + return None + + +def validate_temperature(raw_temperature: object, field_name: str = "custom_temperature") -> float | None: + if raw_temperature is None: + return None + try: + temperature = float(raw_temperature) + except (TypeError, ValueError): + raise HTTPException(status_code=400, detail=f"{field_name} must be a number") + if not (0.0 <= temperature <= 2.0): + raise HTTPException(status_code=400, detail=f"{field_name} must be between 0.0 and 2.0") + return temperature + + +def validate_custom_name(value: object) -> str: + custom_name = str(value or "").strip() + if not custom_name or len(custom_name) > 100: + raise HTTPException(status_code=400, detail="custom_name is required and must be <= 100 characters") + return custom_name + + +def validate_prompt(value: object, *, max_chars: int, field_name: str = "custom_prompt") -> str: + prompt = str(value or "").strip() + if not prompt or len(prompt) > max_chars: + raise HTTPException(status_code=400, detail=f"{field_name} is required and must be <= {max_chars} characters") + return prompt + + +def known_tool_names_from_profiles(profiles_data: dict) -> set[str]: + known_tools: set[str] = {"get_user_profile", "save_user_profile"} + for entry in profiles_data.values(): + if isinstance(entry, dict): + for tool_name in entry.get("tools") or []: + if isinstance(tool_name, str): + known_tools.add(tool_name) + return known_tools + + +def validate_tool_names(raw_tools: object, known_tools: set[str], field_name: str = "custom_tools") -> list[str]: + if not isinstance(raw_tools, list): + raise HTTPException(status_code=400, detail=f"{field_name} must be a list of tool name strings") + invalid_tools = [tool_name for tool_name in raw_tools if tool_name not in known_tools] + if invalid_tools: + raise HTTPException(status_code=400, detail=f"Unknown tools: {', '.join(invalid_tools)}") + return list(raw_tools) + + +def available_skill_names(skills_dir: Path) -> set[str]: + if not skills_dir.is_dir(): + return set() + from agent_framework import SkillsProvider + + provider = SkillsProvider(skill_paths=skills_dir) + return set(provider._skills.keys()) + + +def filter_known_skill_names(raw_skills: object, available_skills: set[str], field_name: str = "custom_skills") -> tuple[list[str], list[str]]: + if not isinstance(raw_skills, list): + raise HTTPException(status_code=400, detail=f"{field_name} must be a list of skill name strings") + known = [name for name in raw_skills if name in available_skills] + dropped = [name for name in raw_skills if name not in available_skills] + return known, dropped + + +def validate_http_mcp_servers(raw_servers: object, *, override: bool) -> list[dict]: + if not isinstance(raw_servers, list): + raise HTTPException(status_code=400, detail="mcp_servers must be a list") + + for entry in raw_servers: + if not isinstance(entry, dict): + raise HTTPException(status_code=400, detail="Each mcp_servers entry must be an object") + if not entry.get("name"): + raise HTTPException(status_code=400, detail="Each mcp_servers entry requires a 'name'") + if entry.get("transport") != "http": + if override: + raise HTTPException(status_code=400, detail="Built-in profile overrides only support 'http' MCP servers") + raise HTTPException( + status_code=400, + detail="Custom agents only support 'http' MCP servers. Local (stdio) servers must be configured in agents.yaml.", + ) + if not entry.get("url"): + raise HTTPException(status_code=400, detail=f"MCP server '{entry['name']}' (http) requires a 'url'") + + return raw_servers + + +__all__ = [ + "ALLOWED_IMAGE_MIMES", + "MAX_IMAGE_SIZE_BYTES", + "MAX_IMAGES_PER_MESSAGE", + "available_skill_names", + "filter_known_skill_names", + "known_tool_names_from_profiles", + "validate_custom_name", + "validate_http_mcp_servers", + "validate_image_magic_bytes", + "validate_prompt", + "validate_temperature", + "validate_tool_names", + "validate_uploaded_images", +] \ No newline at end of file