From dc6c63fb20ff110c46e4f0ff995c6032912e4108 Mon Sep 17 00:00:00 2001 From: lzw-git-all <19607802@qq.com> Date: Sun, 29 Mar 2026 18:58:41 +0800 Subject: [PATCH] fix: session restart state handling improvements Problem 1: SDK session ID persists after restart - After daemon crash/restart, sdkSessionId still points to stale server-side session - Resume causes SDK to continue processing old task instead of new request - Fix: Clear sdkSessionId on startup, backup to previousSdkSessionId Problem 2: Permission timeout uses wrong context - sharedCtx.lastContextToken gets overwritten by concurrent messages - Timeout message sent to wrong user/context - Fix: Store contextToken/fromUserId in PendingPermission itself Also: Protect code blocks when splitting long messages --- docs/session-restart-fix.md | 117 +++++++++++++++++++++++++++ src/main.ts | 156 ++++++++++++++++++++++++++++++++---- src/permission.ts | 14 +++- src/session.ts | 2 + 4 files changed, 269 insertions(+), 20 deletions(-) create mode 100644 docs/session-restart-fix.md diff --git a/docs/session-restart-fix.md b/docs/session-restart-fix.md new file mode 100644 index 0000000..dbc3011 --- /dev/null +++ b/docs/session-restart-fix.md @@ -0,0 +1,117 @@ +# Session Restart State Handling 修复分析 + +本文档记录了 session restart 相关问题的修复分析。 + +--- + +## Problem 1: SDK session ID persists after restart + +### 问题场景 + +``` +用户发送消息 → state=processing → sdkSessionId=abc123 + ↓ +服务 crash → systemd 重启 → 读取持久化 session.json + ↓ +state=processing (卡住) + sdkSessionId=abc123 (仍存在) + ↓ +用户发新消息 → resume: abc123 → SDK 尝试恢复 server-side 会话 + ↓ +问题:SDK 进程已死,abc123 指向的会话可能: + - 已终止 → 出错 + - 还存在 → 继续处理旧任务(用户实际遇到的情况) +``` + +### 实际影响 + +用户发送新消息后,收到的是旧任务的回复(请求和响应不匹配)。 + +### 修复方案 + +```typescript +// src/main.ts - runDaemon() +if (session.sdkSessionId) { + logger.info('Clearing SDK session ID on restart', { accountId: account.accountId, sessionId: session.sdkSessionId }); + session.previousSdkSessionId = session.sdkSessionId; // 备份,便于手动恢复 + session.sdkSessionId = undefined; // 清除,避免 resume 继续旧任务 +} +``` + +### 必要性评估 + +| 等级 | 说明 | +|-----|-----| +| **高** | 用户实际遇到的问题,严重影响用户体验 | + +--- + +## Problem 2: Permission timeout uses wrong context + +### 问题场景 + +``` +消息1 → 创建权限请求 → sharedCtx.lastContextToken = token1 + ↓ + 等待用户 y/n(最多120秒) + ↓ +消息2 到达 → sharedCtx.lastContextToken = token2 ← 覆盖了! + ↓ +120秒超时 → 用 sharedCtx.lastContextToken = token2 发送超时消息 + ↓ +问题:超时消息发到了消息2的用户(token2),而不是消息1(token1) +``` + +### 修复方案 + +修改前:依赖共享状态 `sharedCtx.lastContextToken` + +```typescript +// 旧代码 +const permissionBroker = createPermissionBroker(async () => { + await sender.sendText(fromUserId, sharedCtx.lastContextToken, '⏰ 权限请求超时'); +}); +``` + +修改后:将 context 存储在 PendingPermission 本身 + +```typescript +// src/session.ts +export interface PendingPermission { + toolName: string; + toolInput: string; + contextToken: string; // Store context token for timeout message + fromUserId: string; // Store user ID for timeout message + resolve: (allowed: boolean) => void; + timer: NodeJS.Timeout; +} + +// src/permission.ts +function createPending( + accountId: string, + toolName: string, + toolInput: string, + contextToken: string, // 直接传入 + fromUserId: string, // 直接传入 +): Promise { + // ... + pending.set(accountId, { toolName, toolInput, contextToken, fromUserId, resolve, timer }); + // timeout 时使用存储的 contextToken 和 fromUserId +} +``` + +### 必要性评估 + +| 等级 | 说明 | +|-----|-----| +| **中** | 边缘场景,但架构上不应依赖共享状态,多账号支持更健壮 | + +--- + +## 修复涉及的文件 + +| 文件 | 改动 | +|-----|-----| +| `src/session.ts` | PendingPermission 添加 contextToken/fromUserId 字段 | +| `src/permission.ts` | createPending 接收这两个参数,timeout 时使用 | +| `src/main.ts` | sdkSessionId 清除 + permissionBroker callback 传递参数 + splitMessage 代码块保护 | + diff --git a/src/main.ts b/src/main.ts index 6dedca0..ca30853 100644 --- a/src/main.ts +++ b/src/main.ts @@ -25,26 +25,102 @@ import { MessageType, type WeixinMessage } from './wechat/types.js'; const MAX_MESSAGE_LENGTH = 2048; +/** + * Split a long message into chunks, respecting code blocks. + * - Never splits inside a fenced code block (``` ... ```) + * - Adds continuation markers if split is unavoidable + * - Prefers splitting at paragraph boundaries + */ function splitMessage(text: string, maxLen: number = MAX_MESSAGE_LENGTH): string[] { if (text.length <= maxLen) return [text]; + const chunks: string[] = []; let remaining = text; + while (remaining.length > 0) { if (remaining.length <= maxLen) { chunks.push(remaining); break; } - // Try to split at a newline near the limit - let splitIdx = remaining.lastIndexOf('\n', maxLen); - if (splitIdx < maxLen * 0.3) { - splitIdx = maxLen; + + // Find the best split point + const splitIdx = findBestSplitPoint(remaining, maxLen); + + if (splitIdx <= 0) { + // No good split point found, force split with continuation marker + const forcedChunk = remaining.slice(0, maxLen - 15) + '\n... (续)'; + chunks.push(forcedChunk); + remaining = '(续) ...\n' + remaining.slice(maxLen - 15); + continue; + } + + const chunk = remaining.slice(0, splitIdx); + const inCodeBlock = isInCodeBlock(chunk); + + if (inCodeBlock) { + // Close the code block and add continuation marker + chunks.push(chunk + '\n```\n... (续)'); + remaining = '```\n(续) ...\n' + remaining.slice(splitIdx).replace(/^\n+/, ''); + } else { + chunks.push(chunk); + remaining = remaining.slice(splitIdx).replace(/^\n+/, ''); } - chunks.push(remaining.slice(0, splitIdx)); - remaining = remaining.slice(splitIdx).replace(/^\n+/, ''); } + return chunks; } +/** + * Find the best point to split text, avoiding code blocks. + */ +function findBestSplitPoint(text: string, maxLen: number): number { + // First, try to find if there's a code block ending before maxLen + const codeBlockPattern = /```/g; + let inBlock = false; + let lastCodeBlockEnd = -1; + let match; + + while ((match = codeBlockPattern.exec(text.slice(0, maxLen + 100))) !== null) { + inBlock = !inBlock; + if (!inBlock) { + lastCodeBlockEnd = match.index + 3; + } + } + + // If we're in a code block at maxLen, split at the end of the last complete block + if (inBlock && lastCodeBlockEnd > maxLen * 0.3) { + return lastCodeBlockEnd; + } + + // Try to split at paragraph boundary (double newline) + const paragraphIdx = text.lastIndexOf('\n\n', maxLen); + if (paragraphIdx > maxLen * 0.3) { + return paragraphIdx; + } + + // Try to split at single newline + const newlineIdx = text.lastIndexOf('\n', maxLen); + if (newlineIdx > maxLen * 0.3) { + return newlineIdx; + } + + // No good split point + return -1; +} + +/** + * Check if text ends inside an unclosed code block. + */ +function isInCodeBlock(text: string): boolean { + const codeBlockPattern = /```/g; + let count = 0; + let match; + while ((match = codeBlockPattern.exec(text)) !== null) { + count++; + } + return count % 2 === 1; +} + function promptUser(question: string, defaultValue?: string): Promise { return new Promise((resolve) => { const rl = createInterface({ input: process.stdin, output: process.stdout }); @@ -174,19 +250,31 @@ async function runDaemon(): Promise { sessionStore.save(account.accountId, session); } - // Fix: reset stale non-idle state on startup (e.g. after crash) + // Fix: reset stuck session state after restart (processing/waiting_permission should not persist) if (session.state !== 'idle') { - logger.warn('Resetting stale session state on startup', { state: session.state }); + logger.warn('Session state was stuck after restart, resetting to idle', { accountId: account.accountId, previousState: session.state }); session.state = 'idle'; - sessionStore.save(account.accountId, session); } + // Always clear sdkSessionId on restart since the SDK process is gone + // The SDK session may still exist server-side but local state is inconsistent + // Move to previousSdkSessionId for potential manual recovery + if (session.sdkSessionId) { + logger.info('Clearing SDK session ID on restart', { accountId: account.accountId, sessionId: session.sdkSessionId }); + session.previousSdkSessionId = session.sdkSessionId; + session.sdkSessionId = undefined; + } + + sessionStore.save(account.accountId, session); + const sender = createSender(api, account.accountId); + // Note: sharedCtx is kept for backward compatibility but permission timeout now uses stored context const sharedCtx = { lastContextToken: '' }; const activeControllers = new Map(); - const permissionBroker = createPermissionBroker(async () => { + // Permission broker callback now receives contextToken and fromUserId directly (fixes concurrency issue) + const permissionBroker = createPermissionBroker(async (contextToken: string, fromUserId: string) => { try { - await sender.sendText(account.userId ?? '', sharedCtx.lastContextToken, '⏰ 权限请求超时,已自动拒绝。'); + await sender.sendText(fromUserId, contextToken, '⏰ 权限请求超时,已自动拒绝。'); } catch { logger.warn('Failed to send permission timeout message'); } @@ -244,6 +332,7 @@ async function handleMessage( const contextToken = msg.context_token ?? ''; const fromUserId = msg.from_user_id; + // Update sharedCtx for backward compatibility (though permission timeout now uses stored context) sharedCtx.lastContextToken = contextToken; // Extract text from items @@ -411,6 +500,7 @@ async function sendToClaude( try { // Download image if present let images: QueryOptions['images']; + let imageDownloadError: string | undefined; if (imageItem) { const base64DataUri = await downloadImage(imageItem); if (base64DataUri) { @@ -427,10 +517,21 @@ async function sendToClaude( }, }, ]; + } else { + imageDownloadError = '图片格式解析失败'; + logger.error('Failed to parse image data URI format'); } + } else { + imageDownloadError = '图片下载失败'; + logger.error('Failed to download image', { imageItem }); } } + // Notify user if image processing failed + if (imageDownloadError && !images) { + await sender.sendText(fromUserId, contextToken, `⚠️ ${imageDownloadError},将以纯文字模式处理。`); + } + const effectivePermissionMode = session.permissionMode ?? config.permissionMode; const isAutoPermission = effectivePermissionMode === 'auto'; @@ -482,18 +583,36 @@ async function sendToClaude( session.state = 'waiting_permission'; sessionStore.save(account.accountId, session); - // Create pending permission + // Create pending permission (includes context for timeout message - fixes concurrency) const permissionPromise = permissionBroker.createPending( account.accountId, toolName, toolInput, + contextToken, + fromUserId, ); // Send permission message to WeChat const perm = permissionBroker.getPending(account.accountId); if (perm) { - const permMsg = permissionBroker.formatPendingMessage(perm); - await sender.sendText(fromUserId, contextToken, permMsg); + try { + const permMsg = permissionBroker.formatPendingMessage(perm); + await sender.sendText(fromUserId, contextToken, permMsg); + } catch (sendErr) { + // If we can't send the permission request, we must fail the permission + // otherwise the SDK will hang indefinitely + logger.error('Failed to send permission request to WeChat', { error: sendErr instanceof Error ? sendErr.message : String(sendErr) }); + permissionBroker.resolvePermission(account.accountId, false); + session.state = 'processing'; + sessionStore.save(account.accountId, session); + return false; + } + } else { + // Should not happen: pending permission not found after creation + logger.error('Pending permission not found after creation'); + session.state = 'processing'; + sessionStore.save(account.accountId, session); + return false; } const allowed = await permissionPromise; @@ -536,7 +655,12 @@ async function sendToClaude( } } else if (result.error) { logger.error('Claude query error', { error: result.error }); - await sender.sendText(fromUserId, contextToken, '⚠️ Claude 处理请求时出错,请稍后重试。'); + // Check if it's a resume-related error that might be recoverable + const isResumeError = result.error.includes('session') || result.error.includes('resume'); + const userMsg = isResumeError + ? '⚠️ 会话状态异常,已自动重置。请重新发送你的请求。' + : '⚠️ Claude 处理请求时出错,请稍后重试。'; + await sender.sendText(fromUserId, contextToken, userMsg); } else if (!anySent) { await sender.sendText(fromUserId, contextToken, 'ℹ️ Claude 无返回内容(可能因权限被拒而终止)'); } @@ -584,4 +708,4 @@ if (command === 'setup') { console.error('启动失败:', err); process.exit(1); }); -} +} \ No newline at end of file diff --git a/src/permission.ts b/src/permission.ts index c5c1836..ef7a230 100644 --- a/src/permission.ts +++ b/src/permission.ts @@ -4,13 +4,19 @@ import type { PendingPermission } from './session.js'; const PERMISSION_TIMEOUT = 120_000; const GRACE_PERIOD = 15_000; -export type OnPermissionTimeout = () => void; +export type OnPermissionTimeout = (contextToken: string, fromUserId: string) => void; export function createPermissionBroker(onTimeout?: OnPermissionTimeout) { const pending = new Map(); const timedOut = new Map(); // accountId → timestamp - function createPending(accountId: string, toolName: string, toolInput: string): Promise { + function createPending( + accountId: string, + toolName: string, + toolInput: string, + contextToken: string, + fromUserId: string, + ): Promise { // Clear any existing pending permission for this account to prevent timer leak const existing = pending.get(accountId); if (existing) { @@ -29,10 +35,10 @@ export function createPermissionBroker(onTimeout?: OnPermissionTimeout) { // Clean up grace period entry after GRACE_PERIOD setTimeout(() => timedOut.delete(accountId), GRACE_PERIOD); resolve(false); - onTimeout?.(); + onTimeout?.(contextToken, fromUserId); }, PERMISSION_TIMEOUT); - pending.set(accountId, { toolName, toolInput, resolve, timer }); + pending.set(accountId, { toolName, toolInput, contextToken, fromUserId, resolve, timer }); }); } diff --git a/src/session.ts b/src/session.ts index 336fc6f..188c7e1 100644 --- a/src/session.ts +++ b/src/session.ts @@ -34,6 +34,8 @@ export interface Session { export interface PendingPermission { toolName: string; toolInput: string; + contextToken: string; // Store context token for timeout message + fromUserId: string; // Store user ID for timeout message resolve: (allowed: boolean) => void; timer: NodeJS.Timeout; }