mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
Persist new agent form config
This commit is contained in:
@@ -20,7 +20,7 @@ import {
|
||||
import { useDaemonRequest } from "@/hooks/use-daemon-request";
|
||||
import type { SessionOutboundMessage } from "@server/server/messages";
|
||||
import { useAggregatedAgents } from "@/hooks/use-aggregated-agents";
|
||||
import { useAgentFormState } from "@/hooks/use-agent-form-state";
|
||||
import { useAgentFormState, type CreateAgentInitialValues } from "@/hooks/use-agent-form-state";
|
||||
import { useDaemonConnections } from "@/contexts/daemon-connections-context";
|
||||
import { formatConnectionStatus } from "@/utils/daemons";
|
||||
import { useSessionStore } from "@/stores/session-store";
|
||||
@@ -74,17 +74,22 @@ export default function DraftAgentScreen() {
|
||||
const resolvedModel = getParamValue(params.model);
|
||||
const resolvedWorkingDir = getParamValue(params.workingDir);
|
||||
|
||||
const initialValues = useMemo(
|
||||
() => ({
|
||||
workingDir: resolvedWorkingDir,
|
||||
provider: resolvedProvider
|
||||
? (resolvedProvider as AgentProvider)
|
||||
: undefined,
|
||||
modeId: resolvedMode ?? null,
|
||||
model: resolvedModel ?? null,
|
||||
}),
|
||||
[resolvedMode, resolvedModel, resolvedProvider, resolvedWorkingDir]
|
||||
);
|
||||
const initialValues = useMemo(() => {
|
||||
const values: CreateAgentInitialValues = {};
|
||||
if (resolvedWorkingDir) {
|
||||
values.workingDir = resolvedWorkingDir;
|
||||
}
|
||||
if (resolvedProvider) {
|
||||
values.provider = resolvedProvider as AgentProvider;
|
||||
}
|
||||
if (resolvedMode) {
|
||||
values.modeId = resolvedMode;
|
||||
}
|
||||
if (resolvedModel) {
|
||||
values.model = resolvedModel;
|
||||
}
|
||||
return values;
|
||||
}, [resolvedMode, resolvedModel, resolvedProvider, resolvedWorkingDir]);
|
||||
const {
|
||||
selectedServerId,
|
||||
setSelectedServerIdFromUser,
|
||||
@@ -102,6 +107,7 @@ export default function DraftAgentScreen() {
|
||||
isModelLoading,
|
||||
modelError,
|
||||
refreshProviderModels,
|
||||
persistFormPreferences,
|
||||
} = useAgentFormState({
|
||||
initialServerId: resolvedServerId ?? null,
|
||||
initialValues,
|
||||
@@ -553,6 +559,8 @@ export default function DraftAgentScreen() {
|
||||
}
|
||||
: undefined;
|
||||
|
||||
void persistFormPreferences();
|
||||
|
||||
const requestId = generateMessageId();
|
||||
pendingRequestIdRef.current = requestId;
|
||||
setIsLoading(true);
|
||||
@@ -573,6 +581,7 @@ export default function DraftAgentScreen() {
|
||||
isLoading,
|
||||
isNonGitDirectory,
|
||||
modeOptions,
|
||||
persistFormPreferences,
|
||||
selectedMode,
|
||||
selectedModel,
|
||||
selectedProvider,
|
||||
|
||||
@@ -63,6 +63,7 @@ type UseAgentFormStateResult = {
|
||||
) => void;
|
||||
clearQueuedProviderModelRequest: (serverId: string | null) => void;
|
||||
workingDirIsEmpty: boolean;
|
||||
persistFormPreferences: () => Promise<void>;
|
||||
};
|
||||
|
||||
const FORM_PREFERENCES_STORAGE_KEY = "@paseo:create-agent-preferences";
|
||||
@@ -305,6 +306,55 @@ export function useAgentFormState(
|
||||
void persist();
|
||||
}, [selectedMode, selectedProvider, workingDir, selectedModel, selectedServerId]);
|
||||
|
||||
const persistFormPreferences = useCallback(async () => {
|
||||
const payload: {
|
||||
workingDir?: string;
|
||||
provider?: AgentProvider;
|
||||
mode?: string;
|
||||
model?: string;
|
||||
serverId?: string;
|
||||
} = {};
|
||||
|
||||
if (typeof workingDir === "string") {
|
||||
payload.workingDir = workingDir;
|
||||
}
|
||||
if (selectedProvider) {
|
||||
payload.provider = selectedProvider;
|
||||
}
|
||||
if (typeof selectedMode === "string") {
|
||||
payload.mode = selectedMode;
|
||||
}
|
||||
if (typeof selectedModel === "string") {
|
||||
payload.model = selectedModel;
|
||||
}
|
||||
if (selectedServerId) {
|
||||
payload.serverId = selectedServerId;
|
||||
}
|
||||
|
||||
if (Object.keys(payload).length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const stored = await AsyncStorage.getItem(FORM_PREFERENCES_STORAGE_KEY);
|
||||
const parsed = stored
|
||||
? (JSON.parse(stored) as Record<string, unknown>)
|
||||
: {};
|
||||
await AsyncStorage.setItem(
|
||||
FORM_PREFERENCES_STORAGE_KEY,
|
||||
JSON.stringify({
|
||||
...parsed,
|
||||
...payload,
|
||||
})
|
||||
);
|
||||
} catch (error) {
|
||||
console.error(
|
||||
"[useAgentFormState] Failed to persist form preferences:",
|
||||
error
|
||||
);
|
||||
}
|
||||
}, [selectedMode, selectedModel, selectedProvider, selectedServerId, workingDir]);
|
||||
|
||||
const clearQueuedProviderModelRequest = useCallback((serverId: string | null) => {
|
||||
if (!serverId) {
|
||||
return;
|
||||
@@ -449,6 +499,7 @@ export function useAgentFormState(
|
||||
queueProviderModelFetch,
|
||||
clearQueuedProviderModelRequest,
|
||||
workingDirIsEmpty,
|
||||
persistFormPreferences,
|
||||
}),
|
||||
[
|
||||
agentDefinition,
|
||||
@@ -469,6 +520,7 @@ export function useAgentFormState(
|
||||
setModelFromUser,
|
||||
workingDir,
|
||||
workingDirIsEmpty,
|
||||
persistFormPreferences,
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
503
plan.md
503
plan.md
@@ -2,502 +2,19 @@
|
||||
|
||||
## Context
|
||||
|
||||
Build a new Codex MCP provider side‑by‑side with the existing Codex SDK provider. The new provider lives in `packages/server/src/server/agent/providers/codex-mcp-agent.ts` and is selected via a new provider id (e.g. `codex-mcp`). All testing is **E2E only** (no mocks/fakes). Use `/Users/moboudra/dev/voice-dev/.tmp/happy-cli/src/codex/` as reference for MCP + elicitation.
|
||||
|
||||
## CRITICAL RULES - READ BEFORE EVERY TASK
|
||||
|
||||
1. **NO VAGUE REPORTS**: Never say "test hung", "was interrupted", "failed locally" without:
|
||||
|
||||
- The EXACT error message or stack trace
|
||||
- The SPECIFIC line of code causing the issue
|
||||
- A concrete hypothesis for the root cause
|
||||
|
||||
2. **NO SKIPPING/DISABLING TESTS**: Skipping tests, adding `.skip`, or "opt-in gating" is **NOT ACCEPTABLE**. Fix the actual problem. If a test hangs, find out WHY and fix the code, not the test.
|
||||
|
||||
3. **NO WORKAROUNDS**: Adding timeouts, fallbacks, or "defensive" code that hides bugs is forbidden. The code must work correctly, not appear to work.
|
||||
|
||||
4. **INVESTIGATE DEEPLY**: When something fails:
|
||||
|
||||
- Read the actual source code
|
||||
- Add debug logging if needed
|
||||
- Trace the exact execution path
|
||||
- Find the ROOT CAUSE, not symptoms
|
||||
|
||||
5. **BE SPECIFIC**: Every "Done" entry must include:
|
||||
- What the actual problem was (specific)
|
||||
- What code was changed (file:line)
|
||||
- How you verified it works
|
||||
|
||||
## Completed Work (Compacted)
|
||||
|
||||
### Codex MCP Provider (2025-12-24)
|
||||
|
||||
- ✅ Created `codex-mcp-agent.ts` with MCP stdio client, event mapping, permissions, persistence, abort handling
|
||||
- ✅ Fixed model availability (removed hardcoded gpt-4.1), permission elicitation, exit code handling
|
||||
- ✅ Fixed thread/item event mapping for file_change, mcp_tool_call, web_search, todo_list
|
||||
- ✅ Fixed persistence to include conversationId metadata
|
||||
- ✅ Resolved Codex MCP vs Codex SDK elicitation parity (CRITICAL FINDING: `codex exec` ignores approval events)
|
||||
- ✅ All 14 Codex MCP unit tests pass; Codex is now the only provider (deprecated SDK provider removed)
|
||||
- Reports: `CODEX_MCP_MISMATCH_REPORT.md`, `REPORT-codex-mcp-audit.md`
|
||||
|
||||
### UI/UX Fixes (2025-12-25)
|
||||
|
||||
- ✅ Removed duplicate "Codex MCP" option - now shows only "Codex"
|
||||
- ✅ Fixed duplicate user/assistant messages (provider was emitting, but agent-manager already dispatches)
|
||||
- ✅ Fixed Codex agent-control MCP parity with Claude (added MCP servers to Codex config)
|
||||
- ✅ Fixed agent timestamp not updating on click without interaction
|
||||
|
||||
### DaemonClient Implementation (2025-12-25)
|
||||
|
||||
- ✅ Created `packages/server/src/server/test-utils/daemon-client.ts` (~550 lines)
|
||||
- ✅ Created `packages/server/src/server/test-utils/daemon-test-context.ts`
|
||||
- ✅ Created `packages/server/src/server/daemon.e2e.test.ts` (25 passing tests)
|
||||
- Reports: `REPORT-daemon-client-design.md`, `REPORT-daemon-e2e-audit.md`, `REPORT-claude-permission-tests.md`
|
||||
|
||||
**DaemonClient API:**
|
||||
|
||||
- Connection: `connect()`, `close()`
|
||||
- Agent lifecycle: `createAgent()`, `deleteAgent()`, `listAgents()`, `listPersistedAgents()`, `resumeAgent()`
|
||||
- Agent interaction: `sendMessage()`, `cancelAgent()`, `setAgentMode()`, `initializeAgent()`, `clearAgentAttention()`
|
||||
- Permissions: `respondToPermission()`, `waitForPermission()`
|
||||
- Git: `getGitDiff()`, `getGitRepoInfo()`
|
||||
- Files: `exploreFileSystem()`
|
||||
- Models: `listProviderModels()`
|
||||
- Waiting: `waitForAgentIdle()`
|
||||
- Events: `on()`, `getMessageQueue()`, `clearMessageQueue()`
|
||||
|
||||
**E2E Test Coverage:**
|
||||
|
||||
- Basic flow (Codex + Claude): create agent, send message, verify response
|
||||
- Permissions (Codex + Claude): approve/deny, permission_requested/resolved cycle
|
||||
- Persistence: delete agent, resume from handle, verify conversation context
|
||||
- Multi-agent: parent creates child via agent-control MCP
|
||||
- Agent management: cancelAgent, setAgentMode, listAgents
|
||||
- Timestamp: verify clicking agent doesn't update timestamp
|
||||
- Git: diff (staged/unstaged/modified), repo info (branch, dirty state)
|
||||
- Files: list directory, read file content
|
||||
- Models: list Codex and Claude provider models
|
||||
- Images: single and multiple image attachments to Claude
|
||||
Improvements to the new agent screen in the app.
|
||||
|
||||
## Tasks
|
||||
|
||||
- [x] **BUG (CRITICAL)**: Claude agent race condition in `forwardPromptEvents` causes garbled text.
|
||||
- **Done (2025-12-25 20:42)**: Fixed race condition by moving instance-level streaming flags to turn-local context.
|
||||
- [x] **Iteration 1**: Remember the last used config in the new agent screen.
|
||||
|
||||
**WHAT**:
|
||||
1. Created `TurnContext` interface (`claude-agent.ts:48-56`) to track per-turn streaming state
|
||||
2. Removed instance variables `streamedAssistantTextThisTurn` and `streamedReasoningThisTurn` (`claude-agent.ts:368-369`)
|
||||
3. Modified `forwardPromptEvents()` (`claude-agent.ts:779-785`) to create a turn-local context
|
||||
4. Updated `translateMessageToEvents()` (`claude-agent.ts:818`) to accept and pass `TurnContext`
|
||||
5. Updated `mapBlocksToTimeline()` (`claude-agent.ts:1122-1132`) to use `turnContext` for flag tracking
|
||||
6. Updated `mapPartialEvent()` (`claude-agent.ts:1373`) to pass `turnContext` through
|
||||
- Restore the previous behavior where the config was persisted.
|
||||
- When user creates an agent, save the selected config (provider, model, mode, working directory, host).
|
||||
- On next visit to new agent screen, pre-populate with last used config.
|
||||
- **Done (2025-12-28 23:35)**: WHAT: `packages/app/src/app/agent/new.tsx:77-592` now builds initial values only from provided params and persists form preferences on create; `packages/app/src/hooks/use-agent-form-state.ts:309-356` adds explicit persistence helper. RESULT: new agent screen rehydrates last-used config and saves selections on create. EVIDENCE: Not run (not requested).
|
||||
|
||||
**ALSO FIXED**:
|
||||
- `session.ts:341-385`: `interruptAgentIfRunning()` now waits for agent to become fully idle (not just cancelled) before starting new run - mirrors fix from MCP handler
|
||||
- [ ] **Iteration 2**: Show the selected permissions mode in the agent selector trigger.
|
||||
|
||||
**TEST**:
|
||||
- Added E2E test `daemon.e2e.test.ts:2030-2233` "interrupting message should produce coherent text"
|
||||
- Test sends message 1 (500 word essay), immediately interrupts with message 2 ("Hello world")
|
||||
- Before fix: Test failed - message 2 got message 1's response due to flag corruption
|
||||
- After fix: Test passes - message 2 correctly responds with "Hello world from interrupted message"
|
||||
|
||||
**VERIFICATION**:
|
||||
- `npm run typecheck` passes
|
||||
- All 29 daemon E2E tests pass
|
||||
- Race condition E2E test specifically validates interrupt handling
|
||||
|
||||
- [x] **FIX (App)**: Complete new agent page (`/agent/new`) - fix bugs, add missing features, test with Playwright.
|
||||
- **Done (2025-12-25 23:58)**: Fixed critical bugs and removed dead code. Tested with Playwright MCP.
|
||||
|
||||
**WHAT FIXED**:
|
||||
1. **Image attachments** (`new.tsx:245-249`): Removed early return that silently failed. Images are now handled with a warning (server API doesn't yet support images on agent creation, but code is ready).
|
||||
2. **Creation failure display** (`new.tsx:285-289`): Added `setErrorMessage(payload.error ?? "Failed to create agent")` on `agent_create_failed` status.
|
||||
3. **Error/loading states** (`new.tsx:122-123`): Added `errorMessage` and `isLoading` state variables.
|
||||
4. **Error validation** (`new.tsx:216-234`): Added validation with user-visible error messages for missing working directory, prompt, host, and connection.
|
||||
5. **Loading state management** (`new.tsx:248, 287, 296-297, 300`): `setIsLoading(true)` on submit, `setIsLoading(false)` on completion/failure.
|
||||
6. **Error display UI** (`new.tsx:457-461`): Added error container with destructive styling.
|
||||
7. **Dead code cleanup** (`home-footer.tsx:11, 25, 206-209`): Removed unused `CreateAgentModal` import, `showCreateModal` state, and `<CreateAgentModal>` component.
|
||||
|
||||
**DEFERRED** (separate tasks recommended):
|
||||
- Git Options Section (~200 lines): base branch, new branch, worktree selection
|
||||
- Images in initial agent creation (server API needs to support this)
|
||||
|
||||
**VERIFICATION**:
|
||||
- `npm run typecheck` passes
|
||||
- Playwright MCP test: Error message "Working directory is required" displays correctly
|
||||
- Playwright MCP test: Agent creation with working directory succeeds, redirects to agent page
|
||||
|
||||
**FILES CHANGED**:
|
||||
- `packages/app/src/app/agent/new.tsx:122-123, 213-249, 285-300, 457-461, 584-599`
|
||||
- `packages/app/src/components/home-footer.tsx:11, 25, 205-209`
|
||||
|
||||
- [x] **BUG (MCP)**: `send_agent_prompt` errors when agent already running.
|
||||
- **Done (2025-12-25 20:10)**: Fixed `send_agent_prompt` MCP handler to interrupt running agent before sending new prompt.
|
||||
|
||||
**WHAT**:
|
||||
- Modified `packages/server/src/server/agent/mcp-server.ts:418-463`
|
||||
- Added check for `snapshot.lifecycle === "running" || snapshot.pendingRun` at start of `send_agent_prompt` handler
|
||||
- If running: calls `agentManager.cancelAgentRun(agentId)` to interrupt
|
||||
- Added polling wait (max 5s, 50ms interval) for agent to become idle after cancellation
|
||||
- Matches behavior of `session.ts:interruptAgentIfRunning()`
|
||||
|
||||
**WHY**:
|
||||
- The error `"Agent {id} already has an active run"` came from `agent-manager.ts:454` in `streamAgent()`
|
||||
- The MCP handler was calling `startAgentRun` without checking/cancelling existing runs
|
||||
- `cancelAgentRun` only initiates cancellation (fires and forgets), doesn't wait for `pendingRun` to clear
|
||||
- Polling wait ensures generator fully terminates before starting new run
|
||||
|
||||
**TEST**:
|
||||
- Added E2E test `packages/server/src/server/agent/agent-mcp.e2e.test.ts`: "send_agent_prompt interrupts running agent and processes new message"
|
||||
- Test creates agent, sends prompt in background mode, then sends second prompt while first is running
|
||||
- Verifies no "already has an active run" error is returned
|
||||
|
||||
**VERIFICATION**:
|
||||
- Test passes: `npx vitest run packages/server/src/server/agent/agent-mcp.e2e.test.ts --testNamePattern "send_agent_prompt interrupts"`
|
||||
- Server typecheck passes: `npm run typecheck` (server package)
|
||||
- Unit tests pass: `npx vitest run src/server/agent/mcp-server.test.ts`
|
||||
|
||||
- [x] **BUG (Server)**: Claude streaming sends incomplete chunks to long-running agents.
|
||||
- **Done (2025-12-25 22:15)**: Investigated with E2E test for long-running agents. **Server-side streaming is NOT the cause.** All chunks from Claude SDK are correctly forwarded.
|
||||
|
||||
**INVESTIGATION**:
|
||||
1. Created E2E test `daemon.e2e.test.ts:1883-2028` "streaming chunks remain coherent after multiple back-and-forth messages"
|
||||
2. Test creates Claude agent, sends 3 messages, verifies streaming chunks on 3rd message
|
||||
3. Added debug logging to `mapBlocksToTimeline` in `claude-agent.ts:1139-1140` to trace chunk flow
|
||||
4. All `text_delta` events from Claude SDK are received and forwarded
|
||||
5. Final `text` message is correctly suppressed to avoid duplicates
|
||||
|
||||
**TEST OUTPUT**:
|
||||
- 12 chunks received, all coherent: "The elephant at the zoo celebrated its 42nd birthday..."
|
||||
- No missing chunks between server receipt and WebSocket send
|
||||
- Chunks like " celebrate" + "d its" are normal token boundaries, not corruption
|
||||
|
||||
**CONCLUSION**: The server correctly forwards all streaming chunks from Claude SDK. The original bug observed at the client (`REPORT-garbled-text-bug.md`) must have a different cause:
|
||||
- Possible React Native WebSocket implementation differences
|
||||
- Possible transient network issue during original observation
|
||||
- Bug may have been fixed by subsequent changes
|
||||
|
||||
**FILES**:
|
||||
- `packages/server/src/server/daemon.e2e.test.ts:1883-2028` - New E2E test added
|
||||
- `packages/server/src/server/agent/providers/claude-agent.ts:1134-1145` - Verified chunk handling
|
||||
|
||||
**VERIFICATION**: `npx vitest run src/server/daemon.e2e.test.ts --testNamePattern "streaming chunks remain coherent"`
|
||||
|
||||
- [x] **BUG (App-side)**: Claude assistant text garbled in React Native app rendering.
|
||||
- **Done (2025-12-25 21:45)**: Investigated with debug logging and Playwright MCP. **App-side code is NOT the cause.** See `REPORT-garbled-text-bug.md` for full analysis.
|
||||
|
||||
**INVESTIGATION SUMMARY**:
|
||||
1. Added debug logging to `appendAssistantMessage` - state transitions are correct
|
||||
2. Reproduced bug via Playwright MCP on `localhost:8081`
|
||||
3. Console logs show chunks received by client are already incomplete (e.g., " a" + "check error" instead of " a" + "type" + "check error")
|
||||
4. Client-side Zustand state updates work correctly - no race condition
|
||||
5. FlatList renders `item.text` directly - no manipulation
|
||||
|
||||
**ROOT CAUSE**: The server is sending incomplete text chunks to the client. The E2E test passes because it tests NEW agent creation; the bug appears in LONG-RUNNING agents during streaming.
|
||||
|
||||
**NEXT STEPS**: Investigate server-side Claude agent streaming (NOT app-side):
|
||||
- `packages/server/src/server/agent/providers/` - Claude agent streaming implementation
|
||||
- Add server-side logging to compare Claude API output vs what's sent to clients
|
||||
|
||||
- [x] **BUG**: Claude agent assistant text is garbled/corrupted during streaming.
|
||||
- **Done (2025-12-25 20:15)**: Added E2E test `daemon.e2e.test.ts:1760-1881` that verifies server-side streaming text integrity. **Test passes** - server sends clean, non-corrupted text chunks. The bug is NOT on the server side.
|
||||
|
||||
**INVESTIGATION RESULT**:
|
||||
- Server-side data is clean (verified via E2E test)
|
||||
- Each `assistant_message` timeline event contains correct text delta
|
||||
- Text chunks are properly accumulated in order
|
||||
- Bug must be in React Native app rendering layer (`packages/app`)
|
||||
|
||||
**App-side code reviewed (no obvious bugs found)**:
|
||||
- `packages/app/src/types/stream.ts:216-244`: `appendAssistantMessage` correctly appends to last assistant message
|
||||
- `packages/app/src/contexts/session-context.tsx:698-718`: Zustand functional updates for state management
|
||||
- `packages/app/src/components/agent-stream-view.tsx`: FlatList rendering of stream items
|
||||
|
||||
**ALSO FIXED**: Removed duplicate `DropdownField` import in `create-agent-modal.tsx:66` that was causing build errors.
|
||||
|
||||
**NEXT STEPS** (for follow-up task):
|
||||
1. Test in actual React Native app to observe garbled text reproduction
|
||||
2. Check for React concurrent rendering issues with rapid state updates
|
||||
3. Investigate FlatList virtualization edge cases
|
||||
4. Consider adding debug logging to `appendAssistantMessage` in production to capture actual state transitions
|
||||
|
||||
- [x] **REFACTOR**: Remove module-level `SESSION_HISTORY` Map from `codex-mcp-agent.ts`.
|
||||
- **Done (2025-12-25 19:28)**: Removed `SESSION_HISTORY` global Map and refactored to use instance-level `persistedHistory` field.
|
||||
|
||||
**WHAT**:
|
||||
- Removed `SESSION_HISTORY` Map declaration (was at `codex-mcp-agent.ts:118`)
|
||||
- Constructor: now sets `historyPending = true` for resume instead of looking up `SESSION_HISTORY.get()` (`codex-mcp-agent.ts:2564-2565`)
|
||||
- `connect()`: simplified condition to always load from disk when resuming (`codex-mcp-agent.ts:2606-2608`)
|
||||
- `loadPersistedHistoryFromDisk()`: removed `SESSION_HISTORY.set()` line (`codex-mcp-agent.ts:2629-2639`)
|
||||
- `recordHistory()`: appends to `this.persistedHistory` instead of `SESSION_HISTORY` (`codex-mcp-agent.ts:3147-3152`)
|
||||
- `flushPendingHistory()`: appends to `this.persistedHistory` instead of `SESSION_HISTORY` (`codex-mcp-agent.ts:3155-3160`)
|
||||
|
||||
**RESULT**: Typecheck passes. Persistence E2E test "persists session metadata and resumes with history" passes (8.3s). The test now exercises disk loading since there's no global Map to provide in-memory history. Net change: -12 lines.
|
||||
|
||||
**NOTE**: Two flaky tests ("maps thread/item events..." and "captures tool call inputs/outputs...") also failed before this change - they depend on LLM choosing to call MCP tools/web search which is non-deterministic.
|
||||
|
||||
- [x] **REVIEW (App)**: New agent page (`/agent/new`) is missing features from old modal.
|
||||
- **Done (2025-12-25 23:45)**: Completed comprehensive review. See `REPORT-new-agent-page-review.md` for full analysis.
|
||||
|
||||
**WHAT REVIEWED**:
|
||||
- `packages/app/src/app/agent/new.tsx` (560 lines) - New agent creation page
|
||||
- `packages/app/src/components/create-agent-modal.tsx` (~2700 lines) - Old modal with full features
|
||||
- `packages/app/src/components/home-footer.tsx` - Entry points for agent creation
|
||||
- `packages/app/src/components/agent-input-area.tsx` - Already has dictation + image support
|
||||
|
||||
**KEY FINDINGS**:
|
||||
|
||||
1. **CRITICAL BUG** (`new.tsx:216-218`): Image attachments silently fail - early return without error:
|
||||
```typescript
|
||||
if (images && images.length > 0) { return; } // BROKEN!
|
||||
```
|
||||
Fix: Remove early return, include images in `createAgent()` call.
|
||||
|
||||
2. **CRITICAL BUG** (`new.tsx:269-271`): Creation failures silently ignored:
|
||||
```typescript
|
||||
if (payload.status === "agent_create_failed") {
|
||||
pendingRequestIdRef.current = null;
|
||||
return; // No error shown to user!
|
||||
}
|
||||
```
|
||||
Fix: Add `setErrorMessage(payload.error)`.
|
||||
|
||||
3. **Missing error state**: No `errorMessage` state, no UI to display errors.
|
||||
|
||||
4. **Missing loading state**: No `isLoading` state, button doesn't disable during creation.
|
||||
|
||||
5. **Missing daemon offline handling**: No error shown when daemon is unavailable.
|
||||
|
||||
6. **Missing Git Options Section** (~200 lines in old modal):
|
||||
- Base branch selection
|
||||
- Create new branch toggle + input
|
||||
- Create worktree toggle + input
|
||||
- Git validation errors
|
||||
- Dirty directory warnings
|
||||
|
||||
7. **Dictation/Images already work**: `AgentInputArea` component has full `useDictation` integration and image picker - just needs image support wired in.
|
||||
|
||||
8. **Dead code in home-footer.tsx**:
|
||||
- Line 25: `showCreateModal` state never set to `true`
|
||||
- Lines 206-209: `CreateAgentModal` rendered but never shown
|
||||
|
||||
**IMPLEMENTATION PRIORITY**:
|
||||
| Priority | Item | Effort |
|
||||
|----------|------|--------|
|
||||
| P0 | Fix image attachments | 5 lines |
|
||||
| P0 | Fix creation failure display | 2 lines |
|
||||
| P0 | Add error message state | 20 lines |
|
||||
| P1 | Add loading state | 15 lines |
|
||||
| P1 | Add daemon offline handling | 20 lines |
|
||||
| P2 | Git Options Section | ~200 lines |
|
||||
|
||||
**FOLLOW-UP TASKS NEEDED**:
|
||||
- [x] **REVIEW (App)**: Deep comparison of new agent page vs old modal - produce specific fix tasks.
|
||||
- **Done (2025-12-25 23:59)**: Completed deep comparison. See `REPORT-new-agent-page-deep-comparison.md` for full analysis.
|
||||
|
||||
**FINDINGS**:
|
||||
1. **Loading state**: Has `isLoading` state but NO visual feedback (no spinner, button doesn't change)
|
||||
2. **Input area styling**: ✅ MATCHES - Both use same `AgentInputArea` component, verified with Playwright screenshots
|
||||
3. **Git options**: ❌ MISSING - Entire Git section (~200 lines) absent: base branch, new branch, worktree
|
||||
4. **Error handling**: ✅ FIXED - Validates all inputs, shows creation failures, daemon offline errors
|
||||
5. **Visual parity**: ✅ GOOD - Config rows styled well, input area identical to agent screen
|
||||
|
||||
**PREVIOUS FIXES VERIFIED** (from task above):
|
||||
- Image attachments: Fixed (early return removed, warning logged)
|
||||
- Error/loading states: Fixed (errorMessage state, isLoading state)
|
||||
- Creation failure display: Fixed (setErrorMessage on agent_create_failed)
|
||||
- Dead code cleanup: Fixed (CreateAgentModal removed from home-footer.tsx)
|
||||
|
||||
**FILES REVIEWED**:
|
||||
- `packages/app/src/app/agent/new.tsx:122-123, 228-252, 289-292, 461-472`
|
||||
- `packages/app/src/components/create-agent-modal.tsx:448-454, 1388-1401, 1653-1707, 1936-1993, 2015-2024`
|
||||
- `packages/app/src/app/agent/[serverId]/[agentId].tsx:687-688`
|
||||
- Playwright screenshots: `new-agent-page.png`, `existing-agent-screen.png`
|
||||
|
||||
- [x] **FIX (App)**: New agent page - add visual loading indicator during creation
|
||||
- Location: `new.tsx`
|
||||
- Issue: Has `isLoading` state (line 123) but no visual feedback
|
||||
- Fix: When `isLoading` is true:
|
||||
1. Show `<ActivityIndicator>` in submit button area
|
||||
2. Change submit icon/text to indicate loading
|
||||
3. Disable submit button visually (opacity, non-clickable)
|
||||
- Reference: `create-agent-modal.tsx:2015-2024` for loading button pattern
|
||||
- **Done (2025-12-25 21:08)**: Added `isSubmitLoading` prop to `AgentInputArea` for external loading state control.
|
||||
|
||||
**WHAT**:
|
||||
1. Added `isSubmitLoading?: boolean` prop to `AgentInputAreaProps` interface (`agent-input-area.tsx:53-54`)
|
||||
2. Destructured `isSubmitLoading = false` in component function (`agent-input-area.tsx:98`)
|
||||
3. Updated send button disabled condition to include `isSubmitLoading` (`agent-input-area.tsx:1050`)
|
||||
4. Updated send button style condition to include `isSubmitLoading` (`agent-input-area.tsx:1053`)
|
||||
5. Added conditional render: `ActivityIndicator` when loading, `ArrowUp` icon otherwise (`agent-input-area.tsx:1056-1060`)
|
||||
6. Passed `isSubmitLoading={isLoading}` from `new.tsx` to `AgentInputArea` (`new.tsx:472`)
|
||||
|
||||
**RESULT**: When creating an agent, the send button now shows a spinning indicator and becomes disabled (opacity 0.5) during the async creation process.
|
||||
|
||||
**VERIFICATION**:
|
||||
- `npm run typecheck` passes
|
||||
- Playwright MCP test: Error "Working directory is required" correctly displays on empty submit
|
||||
- Visual inspection confirms ActivityIndicator imports already exist in agent-input-area.tsx (line 11)
|
||||
|
||||
- [x] **FEATURE (App)**: New agent page - add Git Options Section
|
||||
- **Done (2025-12-25 22:45)**: Added complete Git Options Section to new agent page.
|
||||
|
||||
**WHAT**:
|
||||
1. Added `GitOptionsSection` and `ToggleRow` components to `agent-form-dropdowns.tsx:569-801`
|
||||
2. Added styles for toggles, checkboxes, and inputs: `agent-form-dropdowns.tsx:701-755`
|
||||
3. Added git-related state in `new.tsx:127-134`: baseBranch, createNewBranch, branchName, createWorktree, worktreeSlug, branchNameEdited, worktreeSlugEdited, shouldSyncBaseBranchRef
|
||||
4. Added `useDaemonRequest` hook for `git_repo_info_request` in `new.tsx:195-229`
|
||||
5. Added git validation logic in `new.tsx:231-353`: isNonGitDirectory, repoInfoStatus, repoInfoError, gitHelperText, slugifyWorktreeName, validateWorktreeName, gitBlockingError
|
||||
6. Added repo info sync effects in `new.tsx:355-423`
|
||||
7. Updated `handleCreateFromInput` to include git options in `new.tsx:529-548`
|
||||
8. Added `GitOptionsSection` UI in `new.tsx:729-785`
|
||||
|
||||
**RESULT**:
|
||||
- Git Options Section appears when working directory is set
|
||||
- Auto-populates base branch from current branch
|
||||
- Shows dirty directory warning in orange
|
||||
- New Branch toggle with auto-slugified branch name input
|
||||
- Create Worktree toggle with worktree slug input
|
||||
- Validates branch/worktree names
|
||||
- Git options passed to createAgent call
|
||||
|
||||
**VERIFICATION**:
|
||||
- `npm run typecheck` passes
|
||||
- Playwright MCP test: Selected `/Users/moboudra/dev/voice-dev` as working directory
|
||||
- Git section appeared with "main" as base branch
|
||||
- Warning "Working directory has uncommitted changes" displayed correctly
|
||||
- Screenshot saved: `.playwright-mcp/new-agent-page-git-section.png`
|
||||
|
||||
**FILES CHANGED**:
|
||||
- `packages/app/src/app/agent/new.tsx` (+371 lines)
|
||||
- `packages/app/src/components/agent-form/agent-form-dropdowns.tsx` (+290 lines)
|
||||
|
||||
- [x] **BUG (App)**: Agent shows "requires attention" even when user was viewing it when it finished.
|
||||
- **Done (2025-12-25 22:55)**: Fixed by auto-clearing attention when agent finishes while user is viewing.
|
||||
|
||||
**WHAT**:
|
||||
1. Added `previousStatusRef` ref to track previous agent status (`[agentId].tsx:420`)
|
||||
2. Added `useEffect` that watches `agent.status` changes (`[agentId].tsx:423-437`)
|
||||
3. When status transitions from "running" to "idle", calls `ws.clearAgentAttention(resolvedAgentId)`
|
||||
|
||||
**ROOT CAUSE**:
|
||||
- Server sets `requiresAttention: true` when agent transitions running→idle (`agent-manager.ts:937-945`)
|
||||
- This happened regardless of whether user was viewing the agent
|
||||
- The `clearAgentAttention` was only called when user clicked on agent from list (`agent-list.tsx:50`)
|
||||
|
||||
**FIX LOGIC**:
|
||||
- Track previous status in a ref to detect state transitions
|
||||
- When `agent.status` changes from "running" to "idle" while user is on the agent screen, immediately clear attention
|
||||
- User witnessed the completion, so no notification needed
|
||||
|
||||
**VERIFICATION**:
|
||||
- `npm run typecheck` passes
|
||||
- Code logic matches existing pattern for clearing attention in `agent-list.tsx:50`
|
||||
|
||||
**FILES CHANGED**:
|
||||
- `packages/app/src/app/agent/[serverId]/[agentId].tsx:419-437`
|
||||
|
||||
- [x] **REFACTOR (App)**: Redesign new agent page UI layout per mockup.
|
||||
- **Done (2025-12-25 23:30)**: Redesigned new agent page UI with all requested changes.
|
||||
|
||||
**WHAT CHANGED**:
|
||||
|
||||
1. **Host in header** (`new.tsx:660-677`):
|
||||
- Added host badge to header right side: `<Monitor>` icon + label + status dot
|
||||
- Green dot when online, gray otherwise
|
||||
- Tappable to open host dropdown sheet
|
||||
- Removed separate "Host" config row
|
||||
|
||||
2. **Combined Agent row** (`new.tsx:695-698`):
|
||||
- Single row showing `"Claude · auto"` (provider + model)
|
||||
- Tapping opens new dropdown sheet with 3 sections: Provider, Model, Mode
|
||||
- Mode options moved INTO the Agent dropdown (not separate row)
|
||||
|
||||
3. **Git section visibility** (`new.tsx:700-751`):
|
||||
- Only shows when `trimmedWorkingDir.length > 0 && !isNonGitDirectory`
|
||||
- Hidden entirely for non-git directories
|
||||
|
||||
4. **Isolation segmented control** (`agent-form-dropdowns.tsx:612-660`):
|
||||
- Created `IsolationControl` component with 3 options: None | Branch | Worktree
|
||||
- Replaced `ToggleRow` checkboxes in GitOptionsSection
|
||||
- State managed via `isolationMode: "none" | "branch" | "worktree"`
|
||||
- Derived `createNewBranch` and `createWorktree` flags for backwards compatibility
|
||||
|
||||
5. **Branch name input** (`agent-form-dropdowns.tsx:813-823`):
|
||||
- Only visible when `isolationMode !== "none"`
|
||||
- Shows appropriate placeholder based on mode
|
||||
- Auto-generates slug when mode changes
|
||||
|
||||
6. **Dirty warning** (`agent-form-dropdowns.tsx:718-720`):
|
||||
- Only shows for "branch" mode, not for "none" or "worktree"
|
||||
- Worktrees don't require clean working directory
|
||||
|
||||
7. **ScrollView wrapper** (`new.tsx:678-902`):
|
||||
- Wrapped config section in ScrollView for proper scrolling
|
||||
- Content no longer overlaps with input area
|
||||
|
||||
**FILES CHANGED**:
|
||||
- `packages/app/src/app/agent/new.tsx`: +295/-142 lines
|
||||
- `packages/app/src/components/agent-form/agent-form-dropdowns.tsx`: +146/-87 lines
|
||||
|
||||
**VERIFICATION**:
|
||||
- `npm run typecheck` passes
|
||||
- Playwright MCP tests:
|
||||
- Host badge shows in header with "Local Host" and green status dot
|
||||
- Agent dropdown opens with Provider/Model/Mode sections
|
||||
- Git section appears only for git repos
|
||||
- IsolationControl segmented control toggles between None/Branch/Worktree
|
||||
- Branch input appears when isolation mode selected
|
||||
- Screenshots: `new-agent-page-redesign.png`, `new-agent-page-agent-dropdown.png`, `new-agent-page-branch-selected.png`
|
||||
|
||||
**NOT DONE** (out of scope for layout refactor):
|
||||
- Mode selector below input area: Requires significant AgentInputArea refactoring; current mode is inside Agent dropdown which is acceptable UX
|
||||
|
||||
- [x] **BUG (CRITICAL)**: Codex agent history not loading after daemon restart.
|
||||
|
||||
**SYMPTOM**: User restarts daemon, opens existing Codex agent, history is EMPTY. This is broken RIGHT NOW despite tests claiming to pass.
|
||||
|
||||
**YOUR JOB**: Fix it. Not write more tests. Not add logging. FIX THE ACTUAL BUG.
|
||||
|
||||
**RULES**:
|
||||
1. If the E2E tests pass but the feature is broken, THE TESTS ARE WRONG. Do not trust them.
|
||||
2. Do NOT add `.skip` or any test workarounds.
|
||||
3. Do NOT report "investigation complete" without a working fix.
|
||||
4. Do NOT blame "flaky tests" or "non-deterministic behavior".
|
||||
|
||||
**DEBUGGING APPROACH**:
|
||||
1. Start fresh: restart daemon, create Codex agent, send messages, verify they appear
|
||||
2. Restart daemon again
|
||||
3. Resume the agent - does history load? If not, WHY?
|
||||
4. Trace the ACTUAL code path:
|
||||
- `codex-mcp-agent.ts`: `loadPersistedHistoryFromDisk()` - is it called? Does it find the file?
|
||||
- What's in `~/.paseo/agents/{id}/history.jsonl`? Does the file exist? Is it populated?
|
||||
- Is `conversationId` being persisted and restored correctly?
|
||||
- Is `historyPending` flag being set correctly on resume?
|
||||
5. Add console.logs if needed to trace execution
|
||||
6. Find the ROOT CAUSE and fix the code
|
||||
|
||||
**SUCCESS CRITERIA**:
|
||||
- Create Codex agent, send 3 messages, get responses
|
||||
- Restart daemon
|
||||
- Resume agent - ALL 3 messages and responses visible
|
||||
- This must work MANUALLY, not just in tests
|
||||
|
||||
**FILES TO INVESTIGATE**:
|
||||
- `packages/server/src/server/agent/providers/codex-mcp-agent.ts`: `loadPersistedHistoryFromDisk`, `recordHistory`, `flushPendingHistory`, constructor resume logic
|
||||
- `packages/server/src/server/agent/agent-manager.ts`: how agents are resumed
|
||||
- Check persistence file location and format
|
||||
|
||||
- **Done (2025-12-25 22:11)**: Fixed Codex history resume to fall back to the default Codex session root when metadata points at an empty session dir, and to accept conversationId when sessionId is missing.
|
||||
|
||||
**WHAT**:
|
||||
1. `packages/server/src/server/agent/providers/codex-mcp-agent.ts:2631-2647` now loads history by sessionId or conversationId and honors metadata rollout/session hints.
|
||||
2. `packages/server/src/server/agent/providers/codex-mcp-agent.ts:2891-2914` persists codexSessionDir alongside conversationId for resume hints.
|
||||
3. `packages/server/src/server/agent/providers/codex-mcp-agent.ts:4184-4267` adds rollout-path loading and default-root fallback when metadata root is empty.
|
||||
4. Reported investigation notes in `REPORT-codex-history-bug.md`.
|
||||
|
||||
**RESULT**:
|
||||
- Manual restart now rehydrates Codex history (snapshot includes prior user/assistant messages).
|
||||
|
||||
**EVIDENCE**:
|
||||
- Manual run: `node --import tsx /tmp/manual-codex-history.ts` → `History events after restart: 12`.
|
||||
- The agent selector dropdown trigger currently shows provider and model (e.g., "Claude · auto").
|
||||
- It should also display the selected permissions mode (e.g., "Claude · auto · Plan").
|
||||
- Update the trigger label to include the mode.
|
||||
|
||||
Reference in New Issue
Block a user