From 789b453e92b03135f40fb48c837597c930163a54 Mon Sep 17 00:00:00 2001 From: 7418 <7418@openclaw.ai> Date: Mon, 9 Mar 2026 21:27:54 +0800 Subject: [PATCH] fix: onboarding stability, invalid JSON from hooks, file tree race conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue 1 — Onboarding re-triggering after completion: - ChatView.tsx: fence regex now tolerates CRLF, trailing whitespace, leading whitespace before closing fence; JSON.parse input trimmed - ChatView.tsx: hookTriggeredSessionId only cleared when backend returns ok - assistant-workspace.ts: saveState uses atomic write (tmp + rename) Issue 2 — "CLI output was not valid JSON" during document reads: - claude-client.ts: remove all SDK hooks (Notification + PostToolUse) to eliminate hook_callback control_request transport that corrupts stdout - claude-client.ts: TodoWrite sync moved to message stream — tool_use records pending input, tool_result emits task_update only on success - claude-client.ts: notifications derived from system/task_notification and result/is_error messages; Telegram forwarding preserved via notifyGeneric Issue 3 — File tree not updating on project switch: - FileTree.tsx: AbortController cancels in-flight requests on directory change; abort fires before empty-directory early return to prevent stale overwrites; error state distinguishes empty dir from load failure; loading always reset - page.tsx: clear workingDirectory immediately on session switch - SplitColumn.tsx: clear workingDirectory when active column has no directory Tests: 9 new tests for fence parsing, atomic write, corrupted state fallback. Co-Authored-By: Claude Opus 4.6 --- package-lock.json | 4 +- package.json | 2 +- .../unit/assistant-workspace.test.ts | 97 +++++++++++++ src/app/chat/[id]/page.tsx | 2 + src/components/chat/ChatView.tsx | 24 ++-- src/components/layout/SplitColumn.tsx | 3 + src/components/project/FileTree.tsx | 44 +++++- src/lib/assistant-workspace.ts | 4 +- src/lib/claude-client.ts | 130 +++++++++--------- 9 files changed, 227 insertions(+), 83 deletions(-) diff --git a/package-lock.json b/package-lock.json index ec49a94..b8a5cc4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "codepilot", - "version": "0.29.0", + "version": "0.29.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "codepilot", - "version": "0.29.0", + "version": "0.29.1", "dependencies": { "@ai-sdk/amazon-bedrock": "^4.0.77", "@ai-sdk/anthropic": "^3.0.47", diff --git a/package.json b/package.json index bbb8e28..e459405 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codepilot", - "version": "0.29.0", + "version": "0.29.1", "private": true, "author": { "name": "op7418", diff --git a/src/__tests__/unit/assistant-workspace.test.ts b/src/__tests__/unit/assistant-workspace.test.ts index 76b4d58..2b2107d 100644 --- a/src/__tests__/unit/assistant-workspace.test.ts +++ b/src/__tests__/unit/assistant-workspace.test.ts @@ -506,6 +506,103 @@ describe('hotset boosts search results', () => { }); }); +// --------------------------------------------------------------------------- +// Issue 1: Onboarding stability fixes +// --------------------------------------------------------------------------- + +describe('completion fence parsing tolerates formatting variations', () => { + // These test the regex patterns used in ChatView.tsx for parsing + // onboarding-complete and checkin-complete fences. + + const onboardingRegex = /```onboarding-complete\s*\r?\n([\s\S]*?)\r?\n\s*```/; + const checkinRegex = /```checkin-complete\s*\r?\n([\s\S]*?)\r?\n\s*```/; + + it('should match standard LF format', () => { + const content = '```onboarding-complete\n{"lang":"zh"}\n```'; + const match = content.match(onboardingRegex); + assert.ok(match, 'Should match LF format'); + assert.equal(JSON.parse(match![1].trim()).lang, 'zh'); + }); + + it('should match CRLF format', () => { + const content = '```onboarding-complete\r\n{"lang":"zh"}\r\n```'; + const match = content.match(onboardingRegex); + assert.ok(match, 'Should match CRLF format'); + assert.equal(JSON.parse(match![1].trim()).lang, 'zh'); + }); + + it('should match with trailing spaces after tag', () => { + const content = '```onboarding-complete \n{"lang":"zh"}\n```'; + const match = content.match(onboardingRegex); + assert.ok(match, 'Should match with trailing spaces'); + }); + + it('should match with leading whitespace before closing fence', () => { + const content = '```onboarding-complete\n{"lang":"zh"}\n ```'; + const match = content.match(onboardingRegex); + assert.ok(match, 'Should match with leading whitespace before closing fence'); + }); + + it('should match checkin-complete with CRLF', () => { + const content = '```checkin-complete\r\n{"mood":"good"}\r\n```'; + const match = content.match(checkinRegex); + assert.ok(match, 'Should match checkin CRLF format'); + assert.equal(JSON.parse(match![1].trim()).mood, 'good'); + }); + + it('should handle JSON with whitespace padding', () => { + const content = '```onboarding-complete\n {"lang":"zh"} \n```'; + const match = content.match(onboardingRegex); + assert.ok(match, 'Should match'); + assert.equal(JSON.parse(match![1].trim()).lang, 'zh'); + }); +}); + +describe('saveState is atomic (write-then-rename)', () => { + let workDir2: string; + + beforeEach(() => { + workDir2 = fs.mkdtempSync(path.join(os.tmpdir(), 'atomic-write-')); + }); + + afterEach(() => { + fs.rmSync(workDir2, { recursive: true, force: true }); + }); + + it('should persist onboardingComplete = true reliably', () => { + initializeWorkspace(workDir2); + const state = loadState(workDir2); + state.onboardingComplete = true; + state.lastCheckInDate = getLocalDateString(); + saveState(workDir2, state); + + // Read raw file to verify it's valid JSON + const statePath = path.join(workDir2, '.assistant', 'state.json'); + const raw = fs.readFileSync(statePath, 'utf-8'); + const parsed = JSON.parse(raw); + assert.equal(parsed.onboardingComplete, true); + }); + + it('should not leave .tmp file after successful write', () => { + initializeWorkspace(workDir2); + const state = loadState(workDir2); + state.onboardingComplete = true; + saveState(workDir2, state); + + const tmpPath = path.join(workDir2, '.assistant', 'state.json.tmp'); + assert.ok(!fs.existsSync(tmpPath), 'Temp file should be removed after atomic rename'); + }); + + it('loadState should return default when state.json is corrupted', () => { + const stateDir = path.join(workDir2, '.assistant'); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(path.join(stateDir, 'state.json'), '{corrupted', 'utf-8'); + + const state = loadState(workDir2); + assert.equal(state.onboardingComplete, false, 'Corrupted state should fall back to default'); + }); +}); + // Clean up DB describe('cleanup', () => { it('close db', () => { diff --git a/src/app/chat/[id]/page.tsx b/src/app/chat/[id]/page.tsx index c2b0185..32464ae 100644 --- a/src/app/chat/[id]/page.tsx +++ b/src/app/chat/[id]/page.tsx @@ -80,6 +80,8 @@ export default function ChatSessionPage({ params }: ChatSessionPageProps) { // Load session info and set working directory useEffect(() => { let cancelled = false; + // Clear stale directory immediately so FileTree doesn't show old project + setWorkingDirectory(''); async function loadSession() { try { diff --git a/src/components/chat/ChatView.tsx b/src/components/chat/ChatView.tsx index c3f034b..7ab2a66 100644 --- a/src/components/chat/ChatView.tsx +++ b/src/components/chat/ChatView.tsx @@ -455,16 +455,20 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal }).catch(() => {}); // Check for onboarding completion - const onboardingMatch = content.match(/```onboarding-complete\n([\s\S]*?)\n```/); + const onboardingMatch = content.match(/```onboarding-complete\s*\r?\n([\s\S]*?)\r?\n\s*```/); if (onboardingMatch) { try { - const answers = JSON.parse(onboardingMatch[1]); - await fetch('/api/workspace/onboarding', { + const answers = JSON.parse(onboardingMatch[1].trim()); + const resp = await fetch('/api/workspace/onboarding', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ answers, sessionId }), }); - await clearHookTriggered(); + if (resp.ok) { + await clearHookTriggered(); + } else { + console.error('[ChatView] Onboarding API returned', resp.status); + } } catch (e) { console.error('[ChatView] Onboarding completion failed:', e); } @@ -472,16 +476,20 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal } // Check for check-in completion - const checkinMatch = content.match(/```checkin-complete\n([\s\S]*?)\n```/); + const checkinMatch = content.match(/```checkin-complete\s*\r?\n([\s\S]*?)\r?\n\s*```/); if (checkinMatch) { try { - const answers = JSON.parse(checkinMatch[1]); - await fetch('/api/workspace/checkin', { + const answers = JSON.parse(checkinMatch[1].trim()); + const resp = await fetch('/api/workspace/checkin', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ answers, sessionId }), }); - await clearHookTriggered(); + if (resp.ok) { + await clearHookTriggered(); + } else { + console.error('[ChatView] Check-in API returned', resp.status); + } } catch (e) { console.error('[ChatView] Check-in completion failed:', e); } diff --git a/src/components/layout/SplitColumn.tsx b/src/components/layout/SplitColumn.tsx index fb60da4..33e9dd3 100644 --- a/src/components/layout/SplitColumn.tsx +++ b/src/components/layout/SplitColumn.tsx @@ -95,6 +95,9 @@ export function SplitColumn({ sessionId, isActive, onClose, onFocus }: SplitColu setWorkingDirectory(sessionWorkingDir); localStorage.setItem("codepilot:last-working-directory", sessionWorkingDir); window.dispatchEvent(new Event("refresh-file-tree")); + } else { + // Clear stale directory from previous column so FileTree doesn't show old project + setWorkingDirectory(''); } setSessionId(sessionId); setPanelOpen(true); diff --git a/src/components/project/FileTree.tsx b/src/components/project/FileTree.tsx index d8e27a2..8fc2a1c 100644 --- a/src/components/project/FileTree.tsx +++ b/src/components/project/FileTree.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useCallback, useRef } from "react"; import { HugeiconsIcon } from "@hugeicons/react"; import { RefreshIcon, Search01Icon, SourceCodeIcon, CodeIcon, File01Icon } from "@hugeicons/core-free-icons"; import { Button } from "@/components/ui/button"; @@ -109,29 +109,54 @@ function RenderTreeNodes({ nodes, searchQuery }: { nodes: FileTreeNode[]; search export function FileTree({ workingDirectory, onFileSelect, onFileAdd }: FileTreeProps) { const [tree, setTree] = useState([]); const [loading, setLoading] = useState(false); + const [error, setError] = useState(null); const [searchQuery, setSearchQuery] = useState(""); + const abortRef = useRef(null); const { t } = useTranslation(); const fetchTree = useCallback(async () => { + // Always cancel in-flight request first — even when clearing directory, + // otherwise a stale response from the old project can arrive and repopulate the tree. + if (abortRef.current) { + abortRef.current.abort(); + } + if (!workingDirectory) { + abortRef.current = null; setTree([]); + setError(null); + setLoading(false); return; } + + const controller = new AbortController(); + abortRef.current = controller; + setLoading(true); + setError(null); try { const res = await fetch( - `/api/files?dir=${encodeURIComponent(workingDirectory)}&baseDir=${encodeURIComponent(workingDirectory)}&depth=4&_t=${Date.now()}` + `/api/files?dir=${encodeURIComponent(workingDirectory)}&baseDir=${encodeURIComponent(workingDirectory)}&depth=4&_t=${Date.now()}`, + { signal: controller.signal } ); + if (controller.signal.aborted) return; if (res.ok) { const data = await res.json(); + if (controller.signal.aborted) return; setTree(data.tree || []); } else { + const errData = await res.json().catch(() => ({ error: res.statusText })); setTree([]); + setError(errData.error || `Failed to load (${res.status})`); } - } catch { + } catch (e) { + if ((e as Error).name === 'AbortError') return; setTree([]); + setError('Failed to load file tree'); } finally { - setLoading(false); + if (!controller.signal.aborted) { + setLoading(false); + } } }, [workingDirectory]); @@ -139,6 +164,15 @@ export function FileTree({ workingDirectory, onFileSelect, onFileAdd }: FileTree fetchTree(); }, [fetchTree]); + // Cleanup abort controller on unmount + useEffect(() => { + return () => { + if (abortRef.current) { + abortRef.current.abort(); + } + }; + }, []); + // Auto-refresh when AI finishes streaming useEffect(() => { const handler = () => fetchTree(); @@ -182,7 +216,7 @@ export function FileTree({ workingDirectory, onFileSelect, onFileAdd }: FileTree ) : tree.length === 0 ? (

- {workingDirectory ? t('fileTree.noFiles') : t('fileTree.selectFolder')} + {error ? error : workingDirectory ? t('fileTree.noFiles') : t('fileTree.selectFolder')}

) : ( { - const notif = input as NotificationHookInput; - controller.enqueue(formatSSE({ - type: 'status', - data: JSON.stringify({ - notification: true, - title: notif.title, - message: notif.message, - }), - })); - // Forward to Telegram (fire-and-forget) - notifyGeneric(notif.title || '', notif.message || '', telegramOpts).catch(() => {}); - return {}; - }], - }], - PostToolUse: [{ - hooks: [async (input) => { - const toolEvent = input as PostToolUseHookInput; - console.log('[claude-client] PostToolUse:', toolEvent.tool_name, 'id:', toolEvent.tool_use_id); - controller.enqueue(formatSSE({ - type: 'tool_result', - data: JSON.stringify({ - tool_use_id: toolEvent.tool_use_id, - content: typeof toolEvent.tool_response === 'string' - ? toolEvent.tool_response - : JSON.stringify(toolEvent.tool_response), - is_error: false, - }), - })); - - // Detect TodoWrite tool and emit task_update SSE for frontend sync - if (toolEvent.tool_name === 'TodoWrite') { - try { - // SDK TodoWriteInput: { todos: { content, status, activeForm }[] } - const toolInput = toolEvent.tool_input as { - todos?: Array<{ content: string; status: string; activeForm?: string }>; - }; - if (toolInput?.todos && Array.isArray(toolInput.todos)) { - console.log('[claude-client] TodoWrite detected, syncing', toolInput.todos.length, 'tasks'); - controller.enqueue(formatSSE({ - type: 'task_update', - data: JSON.stringify({ - session_id: sessionId, - todos: toolInput.todos.map((t, i) => ({ - id: String(i), - content: t.content, - status: t.status, - activeForm: t.activeForm || '', - })), - }), - })); - } - } catch (e) { - console.warn('[claude-client] Failed to parse TodoWrite input:', e); - } - } - - return {}; - }], - }], - }; + // No queryOptions.hooks — all hook types (Notification, PostToolUse) use + // the SDK's hook_callback control_request transport, which fails with + // "CLI output was not valid JSON" when the CLI mixes control frames with + // normal stdout. Notifications are derived from stream messages instead + // (task_notification, result). TodoWrite sync uses tool_use → tool_result. // Capture real-time stderr output from Claude Code process queryOptions.stderr = (data: string) => { @@ -734,6 +673,8 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream>(); for await (const message of conversation) { if (abortController?.signal.aborted) { @@ -761,6 +702,20 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream; + }; + if (toolInput?.todos && Array.isArray(toolInput.todos)) { + pendingTodoWrites.set(block.id, toolInput.todos); + } + } catch (e) { + console.warn('[claude-client] Failed to parse TodoWrite input:', e); + } + } } } break; @@ -789,6 +744,24 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream ({ + id: String(i), + content: t.content, + status: t.status, + activeForm: t.activeForm || '', + })), + }), + })); + } } } } @@ -843,6 +816,21 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream {}); } } break; @@ -887,6 +875,16 @@ export function streamClaude(options: ClaudeStreamOptions): ReadableStream {}); + } break; }