From e2a0f498438e2fb1aa017858ad7c0e6eefda8e13 Mon Sep 17 00:00:00 2001 From: 7418 <7418@openclaw.ai> Date: Thu, 12 Mar 2026 21:29:10 +0800 Subject: [PATCH] fix: CAS lock for assistant trigger, LIKE escape, provider options race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assistant trigger CAS (P1): - hook-triggered route: added expectedOwner param for compare-and-swap — rejects if current owner doesn't match, closing the race where two tabs both clear then both claim the lock - useAssistantTrigger: clear sends expectedOwner (stale session), set sends expectedOwner: null; non-2xx responses now bail instead of falling through - Server-side completion (chat/route.ts): only clears lock if owner matches current sessionId, preventing completed sessions from wiping active locks - Compensation branch: clear now passes expectedOwner to avoid bypassing CAS Image-gen LIKE escape (P2): - db.ts: escape backslashes before % and _ since ESCAPE '\' makes \ the escape char — prevents Windows paths / escaped JSON from corrupting the LIKE pattern and silently falling back to prompt-prefix matching Provider options fetch race (P2): - page.tsx + ChatView.tsx: provider options fetch now uses AbortController with cleanup on dependency change — prevents stale slow responses from overwriting the current provider's thinking/context_1m state Co-Authored-By: Claude Opus 4.6 --- src/app/api/chat/route.ts | 12 ++-- src/app/api/workspace/hook-triggered/route.ts | 13 +++- src/app/chat/page.tsx | 12 ++-- src/components/chat/ChatView.tsx | 12 ++-- src/hooks/useAssistantTrigger.ts | 60 +++++++++++++++---- src/lib/db.ts | 2 +- 6 files changed, 87 insertions(+), 24 deletions(-) diff --git a/src/app/api/chat/route.ts b/src/app/api/chat/route.ts index 0de65ba..c3a488f 100644 --- a/src/app/api/chat/route.ts +++ b/src/app/api/chat/route.ts @@ -641,16 +641,20 @@ async function processCompletionServerSide( console.log('[chat API] Server-side checkin completion succeeded'); } - // Clear hookTriggeredSessionId directly (no HTTP needed) + // Clear hookTriggeredSessionId directly (no HTTP needed). + // CAS: only clear if we are still the owner — prevents wiping another + // tab's legitimate lock when completions arrive out of order. try { const { loadState, saveState } = await import('@/lib/assistant-workspace'); const { getSetting: getSettingDirect } = await import('@/lib/db'); const wsPath = getSettingDirect('assistant_workspace_path'); if (wsPath) { const state = loadState(wsPath); - state.hookTriggeredSessionId = undefined; - state.hookTriggeredAt = undefined; - saveState(wsPath, state); + if (state.hookTriggeredSessionId === sessionId || !state.hookTriggeredSessionId) { + state.hookTriggeredSessionId = undefined; + state.hookTriggeredAt = undefined; + saveState(wsPath, state); + } } } catch { // Best effort diff --git a/src/app/api/workspace/hook-triggered/route.ts b/src/app/api/workspace/hook-triggered/route.ts index 688e3d5..00ee14c 100644 --- a/src/app/api/workspace/hook-triggered/route.ts +++ b/src/app/api/workspace/hook-triggered/route.ts @@ -8,8 +8,19 @@ export async function POST(request: NextRequest) { if (!workspacePath) { return NextResponse.json({ error: 'No workspace path configured' }, { status: 400 }); } - const { sessionId } = await request.json(); + const { sessionId, expectedOwner } = await request.json(); const state = loadState(workspacePath); + + // Compare-and-swap: if expectedOwner is provided, only proceed when the + // current owner matches. This prevents two tabs that both see the same + // stale owner from racing through clear → set sequences. + if (expectedOwner !== undefined) { + const currentOwner = state.hookTriggeredSessionId ?? null; + if (currentOwner !== expectedOwner) { + return NextResponse.json({ success: false, reason: 'owner_mismatch', currentOwner }); + } + } + // '__clear__' sentinel clears the field (used after onboarding/checkin completes) if (sessionId === '__clear__') { state.hookTriggeredSessionId = undefined; diff --git a/src/app/chat/page.tsx b/src/app/chat/page.tsx index f413131..a6d66ae 100644 --- a/src/app/chat/page.tsx +++ b/src/app/chat/page.tsx @@ -47,16 +47,20 @@ export default function NewChatPage() { const [thinkingMode, setThinkingMode] = useState('adaptive'); const [context1m, setContext1m] = useState(false); - // Fetch provider-specific options + // Fetch provider-specific options (with abort to prevent stale responses on fast switch) useEffect(() => { const pid = currentProviderId || 'env'; - fetch(`/api/providers/options?providerId=${encodeURIComponent(pid)}`) + const controller = new AbortController(); + fetch(`/api/providers/options?providerId=${encodeURIComponent(pid)}`, { signal: controller.signal }) .then(r => r.ok ? r.json() : null) .then(data => { - setThinkingMode(data?.options?.thinking_mode || 'adaptive'); - setContext1m(!!data?.options?.context_1m); + if (!controller.signal.aborted) { + setThinkingMode(data?.options?.thinking_mode || 'adaptive'); + setContext1m(!!data?.options?.context_1m); + } }) .catch(() => {}); + return () => controller.abort(); }, [currentProviderId]); const stopStreaming = useCallback(() => { diff --git a/src/components/chat/ChatView.tsx b/src/components/chat/ChatView.tsx index 32b438a..ceae1b7 100644 --- a/src/components/chat/ChatView.tsx +++ b/src/components/chat/ChatView.tsx @@ -59,16 +59,20 @@ export function ChatView({ sessionId, initialMessages = [], initialHasMore = fal useEffect(() => { if (modelName) setCurrentModel(modelName); }, [modelName]); useEffect(() => { if (providerId) setCurrentProviderId(providerId); }, [providerId]); - // Fetch provider-specific options (thinking mode + 1M context) + // Fetch provider-specific options (with abort to prevent stale responses on fast switch) useEffect(() => { const pid = currentProviderId || 'env'; - fetch(`/api/providers/options?providerId=${encodeURIComponent(pid)}`) + const controller = new AbortController(); + fetch(`/api/providers/options?providerId=${encodeURIComponent(pid)}`, { signal: controller.signal }) .then(r => r.ok ? r.json() : null) .then(data => { - setThinkingMode(data?.options?.thinking_mode || 'adaptive'); - setContext1m(!!data?.options?.context_1m); + if (!controller.signal.aborted) { + setThinkingMode(data?.options?.thinking_mode || 'adaptive'); + setContext1m(!!data?.options?.context_1m); + } }) .catch(() => {}); + return () => controller.abort(); }, [currentProviderId]); useEffect(() => { if (initialPermissionProfile) setPermissionProfile(initialPermissionProfile); }, [initialPermissionProfile]); diff --git a/src/hooks/useAssistantTrigger.ts b/src/hooks/useAssistantTrigger.ts index 0e46b42..c2522d5 100644 --- a/src/hooks/useAssistantTrigger.ts +++ b/src/hooks/useAssistantTrigger.ts @@ -140,13 +140,24 @@ export function useAssistantTrigger({ if (isOwnerAlive(state.hookTriggeredSessionId)) { return; // Owning tab is still open, don't interfere } - // Owner tab is gone — clear the stale lock and proceed + // Owner tab is gone — atomically clear the stale lock (CAS: only if owner + // is still the stale session we observed). If another tab already swapped + // in, the server returns owner_mismatch and we bail out. try { - await fetch('/api/workspace/hook-triggered', { + const clearRes = await fetch('/api/workspace/hook-triggered', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ sessionId: '__clear__' }), + body: JSON.stringify({ + sessionId: '__clear__', + expectedOwner: state.hookTriggeredSessionId, + }), }); + if (clearRes.ok) { + const clearData = await clearRes.json(); + if (!clearData.success) return; // Another tab won the race + } else { + return; + } } catch { return; // Can't clear, err on the safe side } @@ -181,7 +192,10 @@ export function useAssistantTrigger({ await fetch('/api/workspace/hook-triggered', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ sessionId: '__clear__' }), + body: JSON.stringify({ + sessionId: '__clear__', + expectedOwner: state.hookTriggeredSessionId || null, + }), }).catch(() => {}); console.log('[useAssistantTrigger] Onboarding compensation succeeded, skipping re-trigger'); return; // Don't re-trigger onboarding @@ -212,12 +226,38 @@ export function useAssistantTrigger({ stopHeartbeatRef.current?.(); stopHeartbeatRef.current = startHeartbeat(sessionId); - // Mark in persistent state to prevent duplicate triggers across page reloads - await fetch('/api/workspace/hook-triggered', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ sessionId }), - }); + // Mark in persistent state to prevent duplicate triggers across page reloads. + // CAS: only set owner if currently unowned (null). If another tab set itself + // as owner between our clear and this call, the server rejects and we bail. + try { + const setRes = await fetch('/api/workspace/hook-triggered', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ sessionId, expectedOwner: null }), + }); + // Bail on any non-2xx (including 500) or CAS rejection — never + // proceed to startStream without a confirmed lock. + if (!setRes.ok) { + assistantTriggerFiredRef.current = false; + stopHeartbeatRef.current?.(); + stopHeartbeatRef.current = null; + return; + } + const setData = await setRes.json(); + if (!setData.success) { + // Lost race — another tab claimed ownership + assistantTriggerFiredRef.current = false; + stopHeartbeatRef.current?.(); + stopHeartbeatRef.current = null; + return; + } + } catch { + // Network error — bail out + assistantTriggerFiredRef.current = false; + stopHeartbeatRef.current?.(); + stopHeartbeatRef.current = null; + return; + } // Use autoTrigger: the message is invisible (no user bubble, no title update) const triggerMsg = needsOnboarding diff --git a/src/lib/db.ts b/src/lib/db.ts index 2fbfb53..73dfc6f 100644 --- a/src/lib/db.ts +++ b/src/lib/db.ts @@ -893,7 +893,7 @@ export function updateMessageBySessionAndHint( // Strategy 1: Exact match on the raw ```image-gen-request...``` block content. // This is unambiguous even when multiple requests share the same prompt. if (rawRequestBlock) { - const escaped = rawRequestBlock.replace(/%/g, '\\%').replace(/_/g, '\\_'); + const escaped = rawRequestBlock.replace(/\\/g, '\\\\').replace(/%/g, '\\%').replace(/_/g, '\\_'); const row = db.prepare( "SELECT id FROM messages WHERE session_id = ? AND role = 'assistant' AND content LIKE ? ESCAPE '\\' AND content NOT LIKE '%image-gen-result%' ORDER BY created_at DESC LIMIT 1" ).get(sessionId, `%${escaped}%`) as { id: string } | undefined;