Skip to content

Commit 789b453

Browse files
7418claude
andcommitted
fix: onboarding stability, invalid JSON from hooks, file tree race conditions
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 <noreply@anthropic.com>
1 parent f76e474 commit 789b453

File tree

9 files changed

+227
-83
lines changed

9 files changed

+227
-83
lines changed

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "codepilot",
3-
"version": "0.29.0",
3+
"version": "0.29.1",
44
"private": true,
55
"author": {
66
"name": "op7418",

src/__tests__/unit/assistant-workspace.test.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,103 @@ describe('hotset boosts search results', () => {
506506
});
507507
});
508508

509+
// ---------------------------------------------------------------------------
510+
// Issue 1: Onboarding stability fixes
511+
// ---------------------------------------------------------------------------
512+
513+
describe('completion fence parsing tolerates formatting variations', () => {
514+
// These test the regex patterns used in ChatView.tsx for parsing
515+
// onboarding-complete and checkin-complete fences.
516+
517+
const onboardingRegex = /```onboarding-complete\s*\r?\n([\s\S]*?)\r?\n\s*```/;
518+
const checkinRegex = /```checkin-complete\s*\r?\n([\s\S]*?)\r?\n\s*```/;
519+
520+
it('should match standard LF format', () => {
521+
const content = '```onboarding-complete\n{"lang":"zh"}\n```';
522+
const match = content.match(onboardingRegex);
523+
assert.ok(match, 'Should match LF format');
524+
assert.equal(JSON.parse(match![1].trim()).lang, 'zh');
525+
});
526+
527+
it('should match CRLF format', () => {
528+
const content = '```onboarding-complete\r\n{"lang":"zh"}\r\n```';
529+
const match = content.match(onboardingRegex);
530+
assert.ok(match, 'Should match CRLF format');
531+
assert.equal(JSON.parse(match![1].trim()).lang, 'zh');
532+
});
533+
534+
it('should match with trailing spaces after tag', () => {
535+
const content = '```onboarding-complete \n{"lang":"zh"}\n```';
536+
const match = content.match(onboardingRegex);
537+
assert.ok(match, 'Should match with trailing spaces');
538+
});
539+
540+
it('should match with leading whitespace before closing fence', () => {
541+
const content = '```onboarding-complete\n{"lang":"zh"}\n ```';
542+
const match = content.match(onboardingRegex);
543+
assert.ok(match, 'Should match with leading whitespace before closing fence');
544+
});
545+
546+
it('should match checkin-complete with CRLF', () => {
547+
const content = '```checkin-complete\r\n{"mood":"good"}\r\n```';
548+
const match = content.match(checkinRegex);
549+
assert.ok(match, 'Should match checkin CRLF format');
550+
assert.equal(JSON.parse(match![1].trim()).mood, 'good');
551+
});
552+
553+
it('should handle JSON with whitespace padding', () => {
554+
const content = '```onboarding-complete\n {"lang":"zh"} \n```';
555+
const match = content.match(onboardingRegex);
556+
assert.ok(match, 'Should match');
557+
assert.equal(JSON.parse(match![1].trim()).lang, 'zh');
558+
});
559+
});
560+
561+
describe('saveState is atomic (write-then-rename)', () => {
562+
let workDir2: string;
563+
564+
beforeEach(() => {
565+
workDir2 = fs.mkdtempSync(path.join(os.tmpdir(), 'atomic-write-'));
566+
});
567+
568+
afterEach(() => {
569+
fs.rmSync(workDir2, { recursive: true, force: true });
570+
});
571+
572+
it('should persist onboardingComplete = true reliably', () => {
573+
initializeWorkspace(workDir2);
574+
const state = loadState(workDir2);
575+
state.onboardingComplete = true;
576+
state.lastCheckInDate = getLocalDateString();
577+
saveState(workDir2, state);
578+
579+
// Read raw file to verify it's valid JSON
580+
const statePath = path.join(workDir2, '.assistant', 'state.json');
581+
const raw = fs.readFileSync(statePath, 'utf-8');
582+
const parsed = JSON.parse(raw);
583+
assert.equal(parsed.onboardingComplete, true);
584+
});
585+
586+
it('should not leave .tmp file after successful write', () => {
587+
initializeWorkspace(workDir2);
588+
const state = loadState(workDir2);
589+
state.onboardingComplete = true;
590+
saveState(workDir2, state);
591+
592+
const tmpPath = path.join(workDir2, '.assistant', 'state.json.tmp');
593+
assert.ok(!fs.existsSync(tmpPath), 'Temp file should be removed after atomic rename');
594+
});
595+
596+
it('loadState should return default when state.json is corrupted', () => {
597+
const stateDir = path.join(workDir2, '.assistant');
598+
fs.mkdirSync(stateDir, { recursive: true });
599+
fs.writeFileSync(path.join(stateDir, 'state.json'), '{corrupted', 'utf-8');
600+
601+
const state = loadState(workDir2);
602+
assert.equal(state.onboardingComplete, false, 'Corrupted state should fall back to default');
603+
});
604+
});
605+
509606
// Clean up DB
510607
describe('cleanup', () => {
511608
it('close db', () => {

src/app/chat/[id]/page.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ export default function ChatSessionPage({ params }: ChatSessionPageProps) {
8080
// Load session info and set working directory
8181
useEffect(() => {
8282
let cancelled = false;
83+
// Clear stale directory immediately so FileTree doesn't show old project
84+
setWorkingDirectory('');
8385

8486
async function loadSession() {
8587
try {

src/components/chat/ChatView.tsx

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -455,33 +455,41 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal
455455
}).catch(() => {});
456456

457457
// Check for onboarding completion
458-
const onboardingMatch = content.match(/```onboarding-complete\n([\s\S]*?)\n```/);
458+
const onboardingMatch = content.match(/```onboarding-complete\s*\r?\n([\s\S]*?)\r?\n\s*```/);
459459
if (onboardingMatch) {
460460
try {
461-
const answers = JSON.parse(onboardingMatch[1]);
462-
await fetch('/api/workspace/onboarding', {
461+
const answers = JSON.parse(onboardingMatch[1].trim());
462+
const resp = await fetch('/api/workspace/onboarding', {
463463
method: 'POST',
464464
headers: { 'Content-Type': 'application/json' },
465465
body: JSON.stringify({ answers, sessionId }),
466466
});
467-
await clearHookTriggered();
467+
if (resp.ok) {
468+
await clearHookTriggered();
469+
} else {
470+
console.error('[ChatView] Onboarding API returned', resp.status);
471+
}
468472
} catch (e) {
469473
console.error('[ChatView] Onboarding completion failed:', e);
470474
}
471475
return;
472476
}
473477

474478
// Check for check-in completion
475-
const checkinMatch = content.match(/```checkin-complete\n([\s\S]*?)\n```/);
479+
const checkinMatch = content.match(/```checkin-complete\s*\r?\n([\s\S]*?)\r?\n\s*```/);
476480
if (checkinMatch) {
477481
try {
478-
const answers = JSON.parse(checkinMatch[1]);
479-
await fetch('/api/workspace/checkin', {
482+
const answers = JSON.parse(checkinMatch[1].trim());
483+
const resp = await fetch('/api/workspace/checkin', {
480484
method: 'POST',
481485
headers: { 'Content-Type': 'application/json' },
482486
body: JSON.stringify({ answers, sessionId }),
483487
});
484-
await clearHookTriggered();
488+
if (resp.ok) {
489+
await clearHookTriggered();
490+
} else {
491+
console.error('[ChatView] Check-in API returned', resp.status);
492+
}
485493
} catch (e) {
486494
console.error('[ChatView] Check-in completion failed:', e);
487495
}

src/components/layout/SplitColumn.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ export function SplitColumn({ sessionId, isActive, onClose, onFocus }: SplitColu
9595
setWorkingDirectory(sessionWorkingDir);
9696
localStorage.setItem("codepilot:last-working-directory", sessionWorkingDir);
9797
window.dispatchEvent(new Event("refresh-file-tree"));
98+
} else {
99+
// Clear stale directory from previous column so FileTree doesn't show old project
100+
setWorkingDirectory('');
98101
}
99102
setSessionId(sessionId);
100103
setPanelOpen(true);

src/components/project/FileTree.tsx

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"use client";
22

3-
import { useState, useEffect, useCallback } from "react";
3+
import { useState, useEffect, useCallback, useRef } from "react";
44
import { HugeiconsIcon } from "@hugeicons/react";
55
import { RefreshIcon, Search01Icon, SourceCodeIcon, CodeIcon, File01Icon } from "@hugeicons/core-free-icons";
66
import { Button } from "@/components/ui/button";
@@ -109,36 +109,70 @@ function RenderTreeNodes({ nodes, searchQuery }: { nodes: FileTreeNode[]; search
109109
export function FileTree({ workingDirectory, onFileSelect, onFileAdd }: FileTreeProps) {
110110
const [tree, setTree] = useState<FileTreeNode[]>([]);
111111
const [loading, setLoading] = useState(false);
112+
const [error, setError] = useState<string | null>(null);
112113
const [searchQuery, setSearchQuery] = useState("");
114+
const abortRef = useRef<AbortController | null>(null);
113115
const { t } = useTranslation();
114116

115117
const fetchTree = useCallback(async () => {
118+
// Always cancel in-flight request first — even when clearing directory,
119+
// otherwise a stale response from the old project can arrive and repopulate the tree.
120+
if (abortRef.current) {
121+
abortRef.current.abort();
122+
}
123+
116124
if (!workingDirectory) {
125+
abortRef.current = null;
117126
setTree([]);
127+
setError(null);
128+
setLoading(false);
118129
return;
119130
}
131+
132+
const controller = new AbortController();
133+
abortRef.current = controller;
134+
120135
setLoading(true);
136+
setError(null);
121137
try {
122138
const res = await fetch(
123-
`/api/files?dir=${encodeURIComponent(workingDirectory)}&baseDir=${encodeURIComponent(workingDirectory)}&depth=4&_t=${Date.now()}`
139+
`/api/files?dir=${encodeURIComponent(workingDirectory)}&baseDir=${encodeURIComponent(workingDirectory)}&depth=4&_t=${Date.now()}`,
140+
{ signal: controller.signal }
124141
);
142+
if (controller.signal.aborted) return;
125143
if (res.ok) {
126144
const data = await res.json();
145+
if (controller.signal.aborted) return;
127146
setTree(data.tree || []);
128147
} else {
148+
const errData = await res.json().catch(() => ({ error: res.statusText }));
129149
setTree([]);
150+
setError(errData.error || `Failed to load (${res.status})`);
130151
}
131-
} catch {
152+
} catch (e) {
153+
if ((e as Error).name === 'AbortError') return;
132154
setTree([]);
155+
setError('Failed to load file tree');
133156
} finally {
134-
setLoading(false);
157+
if (!controller.signal.aborted) {
158+
setLoading(false);
159+
}
135160
}
136161
}, [workingDirectory]);
137162

138163
useEffect(() => {
139164
fetchTree();
140165
}, [fetchTree]);
141166

167+
// Cleanup abort controller on unmount
168+
useEffect(() => {
169+
return () => {
170+
if (abortRef.current) {
171+
abortRef.current.abort();
172+
}
173+
};
174+
}, []);
175+
142176
// Auto-refresh when AI finishes streaming
143177
useEffect(() => {
144178
const handler = () => fetchTree();
@@ -182,7 +216,7 @@ export function FileTree({ workingDirectory, onFileSelect, onFileAdd }: FileTree
182216
</div>
183217
) : tree.length === 0 ? (
184218
<p className="py-4 text-center text-xs text-muted-foreground">
185-
{workingDirectory ? t('fileTree.noFiles') : t('fileTree.selectFolder')}
219+
{error ? error : workingDirectory ? t('fileTree.noFiles') : t('fileTree.selectFolder')}
186220
</p>
187221
) : (
188222
<AIFileTree

src/lib/assistant-workspace.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,9 @@ export function saveState(dir: string, state: AssistantWorkspaceState): void {
527527
fs.mkdirSync(stateDir, { recursive: true });
528528
}
529529
const statePath = path.join(stateDir, STATE_FILE);
530-
fs.writeFileSync(statePath, JSON.stringify(state, null, 2), 'utf-8');
530+
const tmpPath = statePath + '.tmp';
531+
fs.writeFileSync(tmpPath, JSON.stringify(state, null, 2), 'utf-8');
532+
fs.renameSync(tmpPath, statePath);
531533
}
532534

533535
export function needsDailyCheckIn(state: AssistantWorkspaceState, now?: Date): boolean {

0 commit comments

Comments
 (0)