fix: address code review findings — snapshot dedup, workspace hooks, permission params, Windows health check
ChatView.tsx (P2): snapshot consumption now calls detectAssistantCompletion() so assistant workspace onboarding/check-in state updates even when the stream completes while ChatView is unmounted. ChatView.tsx (P2): replace local temp-message append with a DB re-fetch on snapshot consumption to avoid duplicate messages (backend already persists the reply in route.ts). page.tsx (P3): add updatedInput and denyMessage parameters to handlePermissionResponse so AskUserQuestion answers and ExitPlanMode custom deny messages are forwarded to the permission API on the first-message page. electron/main.ts: fix Windows startup timeout by forcing IPv4 (family:4) in health check HTTP request and improving error diagnostics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
1 parent
5eedb5bacc
commit
c07a604271
3 files changed
+45
-21
No files matched your search
+18
-7
@@ -361,6 +361,7 @@ function getPort(): Promise<number> {
|
||||
|
||||
async function waitForServer(port: number, timeout = 30000): Promise<void> {
|
||||
const start = Date.now();
|
||||
let lastError = '';
|
||||
while (Date.now() - start < timeout) {
|
||||
// If the server process already exited, fail fast
|
||||
if (serverExited) {
|
||||
@@ -371,23 +372,33 @@ async function waitForServer(port: number, timeout = 30000): Promise<void> {
|
||||
try {
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const req = require('http').get(`http://127.0.0.1:${port}/api/health`, (res: { statusCode?: number }) => {
|
||||
const http = require('http');
|
||||
// Use options object with family:4 to force IPv4 — avoids Windows
|
||||
// IPv6 resolution issues where 127.0.0.1 may fail to connect.
|
||||
const req = http.get({
|
||||
hostname: '127.0.0.1',
|
||||
port,
|
||||
path: '/api/health',
|
||||
family: 4,
|
||||
timeout: 2000,
|
||||
}, (res: { statusCode?: number }) => {
|
||||
if (res.statusCode === 200) resolve();
|
||||
else reject(new Error(`Status ${res.statusCode}`));
|
||||
});
|
||||
req.on('error', reject);
|
||||
req.setTimeout(1000, () => {
|
||||
req.on('error', (err: Error) => reject(err));
|
||||
req.on('timeout', () => {
|
||||
req.destroy();
|
||||
reject(new Error('timeout'));
|
||||
reject(new Error('request timeout'));
|
||||
});
|
||||
});
|
||||
return;
|
||||
} catch {
|
||||
await new Promise(r => setTimeout(r, 200));
|
||||
} catch (err) {
|
||||
lastError = err instanceof Error ? err.message : String(err);
|
||||
await new Promise(r => setTimeout(r, 300));
|
||||
}
|
||||
}
|
||||
throw new Error(
|
||||
`Server startup timeout after ${timeout / 1000}s.\n\n${serverErrors.length > 0 ? 'Server output:\n' + serverErrors.slice(-10).join('\n') : 'No server output captured.'}`
|
||||
`Server startup timeout after ${timeout / 1000}s.\n\nLast health-check error: ${lastError}\n\n${serverErrors.length > 0 ? 'Server output:\n' + serverErrors.slice(-10).join('\n') : 'No server output captured.'}`
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -42,15 +42,16 @@ export default function NewChatPage() {
|
||||
abortControllerRef.current = null;
|
||||
}, []);
|
||||
|
||||
const handlePermissionResponse = useCallback(async (decision: 'allow' | 'allow_session' | 'deny') => {
|
||||
const handlePermissionResponse = useCallback(async (decision: 'allow' | 'allow_session' | 'deny', updatedInput?: Record<string, unknown>, denyMessage?: string) => {
|
||||
if (!pendingPermission) return;
|
||||
|
||||
const body: { permissionRequestId: string; decision: { behavior: 'allow'; updatedPermissions?: unknown[] } | { behavior: 'deny'; message?: string } } = {
|
||||
const body: { permissionRequestId: string; decision: { behavior: 'allow'; updatedInput?: Record<string, unknown>; updatedPermissions?: unknown[] } | { behavior: 'deny'; message?: string } } = {
|
||||
permissionRequestId: pendingPermission.permissionRequestId,
|
||||
decision: decision === 'deny'
|
||||
? { behavior: 'deny', message: 'User denied permission' }
|
||||
? { behavior: 'deny', message: denyMessage || 'User denied permission' }
|
||||
: {
|
||||
behavior: 'allow',
|
||||
...(updatedInput ? { updatedInput } : {}),
|
||||
...(decision === 'allow_session' && pendingPermission.suggestions
|
||||
? { updatedPermissions: pendingPermission.suggestions }
|
||||
: {}),
|
||||
|
||||
@@ -118,18 +118,30 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal
|
||||
if (existing.pendingPermission && !existing.permissionResolved) {
|
||||
setPendingApprovalSessionId(sessionId);
|
||||
}
|
||||
// If stream completed while this ChatView was unmounted, consume finalMessageContent now
|
||||
// If stream completed while this ChatView was unmounted, consume finalMessageContent now.
|
||||
// Re-fetch messages from DB to avoid duplicates (backend already persisted the reply).
|
||||
if (existing.phase !== 'active' && existing.finalMessageContent) {
|
||||
const assistantMessage: Message = {
|
||||
id: 'temp-assistant-' + Date.now(),
|
||||
session_id: sessionId,
|
||||
role: 'assistant',
|
||||
content: existing.finalMessageContent,
|
||||
created_at: new Date().toISOString(),
|
||||
token_usage: existing.tokenUsage ? JSON.stringify(existing.tokenUsage) : null,
|
||||
};
|
||||
transferPendingToMessage(assistantMessage.id);
|
||||
setMessages((prev) => [...prev, assistantMessage]);
|
||||
detectAssistantCompletion(existing.finalMessageContent);
|
||||
fetch(`/api/chat/sessions/${sessionId}/messages?limit=50`)
|
||||
.then(res => res.ok ? res.json() : null)
|
||||
.then(data => {
|
||||
if (data?.messages) {
|
||||
setMessages(data.messages);
|
||||
}
|
||||
})
|
||||
.catch(() => {
|
||||
// Fallback: append locally if DB fetch fails
|
||||
const assistantMessage: Message = {
|
||||
id: 'temp-assistant-' + Date.now(),
|
||||
session_id: sessionId,
|
||||
role: 'assistant',
|
||||
content: existing.finalMessageContent!,
|
||||
created_at: new Date().toISOString(),
|
||||
token_usage: existing.tokenUsage ? JSON.stringify(existing.tokenUsage) : null,
|
||||
};
|
||||
transferPendingToMessage(assistantMessage.id);
|
||||
setMessages((prev) => [...prev, assistantMessage]);
|
||||
});
|
||||
clearSnapshot(sessionId);
|
||||
}
|
||||
} else {
|
||||
|
||||
Reference in new issue
Block a user