Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 50 additions & 7 deletions src/handler/interactive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ import {
resetAbortForRun,
} from "./reply-run-registry.js"
import { emit } from "./action-bus.js"
import {
buildQuestionFallbackText,
clearPendingQuestionByRequestId,
registerPendingQuestion,
} from "./pending-questions.js"

/**
* form_submit toast 文案常量。
Expand Down Expand Up @@ -426,13 +431,49 @@ export function handleQuestionRequested(
sessionId: string,
): void {
const requestId = String(request.id ?? "")
sendRequestCard({
requestId,
chatId,
deps,
card: buildQuestionCardDSL(request, chatId, chatType, sessionId),
missingClientMessage: "OpenCode client 未配置,跳过问答卡片发送",
sendFailureMessage: "发送问答卡片失败",
if (!deps.v2Client) {
deps.log("warn", "OpenCode client 未配置,跳过问答卡片发送", { requestId })
return
}
if (!requestId || !markSeen(requestId)) return

registerPendingQuestion({ request, chatId, sessionId })

void (async () => {
const res = await sender.sendInteractiveCard(
deps.feishuClient,
chatId,
buildQuestionCardDSL(request, chatId, chatType, sessionId),
deps.log,
)
if (res.ok) return

deps.log("error", "发送问答卡片失败,回退纯文本问题", {
requestId,
chatId,
error: res.error ?? "unknown",
})
const fallback = await sender.sendTextMessage(
deps.feishuClient,
chatId,
buildQuestionFallbackText(request),
deps.log,
)
if (!fallback.ok) {
unmarkSeen(requestId)
deps.log("error", "发送问答纯文本 fallback 失败", {
requestId,
chatId,
error: fallback.error ?? "unknown",
})
}
})().catch((err) => {
unmarkSeen(requestId)
deps.log("error", "发送问答卡片失败", {
requestId,
chatId,
error: err instanceof Error ? err.message : String(err),
})
})
Comment on lines +442 to 477

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Pending question isn't cleared when both card and fallback text delivery fail.

registerPendingQuestion runs at Line 440 before any delivery attempt. If the interactive card fails (Line 449) and the fallback text also fails (Line 462), or the whole IIFE throws (Line 470), only unmarkSeen(requestId) is called — clearPendingQuestionByRequestId(requestId) is never invoked. The pending entry then lingers (up to the 10-minute TTL) for a question the user never actually saw, so an unrelated later text message in that chat can be misinterpreted as an answer to it.

🐛 Clear pending state on full delivery failure
     if (!fallback.ok) {
       unmarkSeen(requestId)
+      clearPendingQuestionByRequestId(requestId)
       deps.log("error", "发送问答纯文本 fallback 失败", {
         requestId,
         chatId,
         error: fallback.error ?? "unknown",
       })
     }
   })().catch((err) => {
     unmarkSeen(requestId)
+    clearPendingQuestionByRequestId(requestId)
     deps.log("error", "发送问答卡片失败", {
       requestId,
       chatId,
       error: err instanceof Error ? err.message : String(err),
     })
   })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
void (async () => {
const res = await sender.sendInteractiveCard(
deps.feishuClient,
chatId,
buildQuestionCardDSL(request, chatId, chatType, sessionId),
deps.log,
)
if (res.ok) return
deps.log("error", "发送问答卡片失败,回退纯文本问题", {
requestId,
chatId,
error: res.error ?? "unknown",
})
const fallback = await sender.sendTextMessage(
deps.feishuClient,
chatId,
buildQuestionFallbackText(request),
deps.log,
)
if (!fallback.ok) {
unmarkSeen(requestId)
deps.log("error", "发送问答纯文本 fallback 失败", {
requestId,
chatId,
error: fallback.error ?? "unknown",
})
}
})().catch((err) => {
unmarkSeen(requestId)
deps.log("error", "发送问答卡片失败", {
requestId,
chatId,
error: err instanceof Error ? err.message : String(err),
})
})
void (async () => {
const res = await sender.sendInteractiveCard(
deps.feishuClient,
chatId,
buildQuestionCardDSL(request, chatId, chatType, sessionId),
deps.log,
)
if (res.ok) return
deps.log("error", "发送问答卡片失败,回退纯文本问题", {
requestId,
chatId,
error: res.error ?? "unknown",
})
const fallback = await sender.sendTextMessage(
deps.feishuClient,
chatId,
buildQuestionFallbackText(request),
deps.log,
)
if (!fallback.ok) {
unmarkSeen(requestId)
clearPendingQuestionByRequestId(requestId)
deps.log("error", "发送问答纯文本 fallback 失败", {
requestId,
chatId,
error: fallback.error ?? "unknown",
})
}
})().catch((err) => {
unmarkSeen(requestId)
clearPendingQuestionByRequestId(requestId)
deps.log("error", "发送问答卡片失败", {
requestId,
chatId,
error: err instanceof Error ? err.message : String(err),
})
})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/handler/interactive.ts` around lines 442 - 477, The pending question is
only being unmarked on delivery failure, but the pending entry itself is never
cleared when both the interactive card and the fallback text fail or when the
async send flow throws. Update the error handling in interactive.ts around
registerPendingQuestion, sendInteractiveCard, sendTextMessage, and the IIFE
catch so that clearPendingQuestionByRequestId(requestId) is invoked in every
full-failure path, alongside unmarkSeen(requestId), to avoid leaving stale
pending state behind.

}

Expand Down Expand Up @@ -562,6 +603,7 @@ export async function handleCardAction(
reply: value.reply,
}).then(() => emitPhase("completed", successBody)).catch(onReplyFailed)
} else {
clearPendingQuestionByRequestId(value.requestId)
void deps.v2Client.question.reply({
requestID: value.requestId,
answers: value.answers,
Expand Down Expand Up @@ -599,6 +641,7 @@ export function buildCallbackResponse(action: CardActionData, log?: LogFn): obje
}

if (value.action === "question_reply") {
clearPendingQuestionByRequestId(value.requestId)
return {
toast: { type: "info", content: "📨 已收到回答,正在转交..." },
}
Expand Down
203 changes: 203 additions & 0 deletions src/handler/pending-questions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
import type { QuestionRequest, LogFn } from "../types.js"
import * as sender from "../feishu/sender.js"
import type * as Lark from "@larksuiteoapi/node-sdk"
import type { OpencodeClient } from "@opencode-ai/sdk/v2/client"
import { TtlMap } from "../utils/ttl-map.js"
import { emit } from "./action-bus.js"

const PENDING_QUESTION_TTL_MS = 10 * 60 * 1_000

interface PendingQuestion {
requestId: string
sessionId: string
chatId: string
request: QuestionRequest
}

export interface PendingQuestionDeps {
feishuClient: InstanceType<typeof Lark.Client>
log: LogFn
v2Client?: OpencodeClient
}

export interface ParsedPendingQuestionReply {
answers: string[][]
displayText: string
}

const pendingByChatId = new TtlMap<PendingQuestion>(PENDING_QUESTION_TTL_MS)
const chatIdByRequestId = new TtlMap<string>(PENDING_QUESTION_TTL_MS)

export function registerPendingQuestion(params: {
request: QuestionRequest
chatId: string
sessionId: string
}): void {
const requestId = String(params.request.id ?? "")
if (!requestId) return

pendingByChatId.set(params.chatId, {
requestId,
sessionId: params.sessionId,
chatId: params.chatId,
request: params.request,
})
chatIdByRequestId.set(requestId, params.chatId)
}

export function clearPendingQuestionByRequestId(requestId: string): void {
const chatId = chatIdByRequestId.get(requestId)
if (chatId) pendingByChatId.delete(chatId)
chatIdByRequestId.delete(requestId)
}
Comment on lines +28 to +52

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Stale requestId can wipe a newer pending question for the same chat.

registerPendingQuestion overwrites pendingByChatId.set(chatId, ...) on every call for a chat, but the previous requestId → chatId entry in chatIdByRequestId is never removed. If a second question is registered for the same chat before the first is cleared (e.g. a late card callback, a retried SSE event, or a race with tryResolvePendingQuestionText), calling clearPendingQuestionByRequestId(oldRequestId) will still resolve to the same chatId and delete the current (newer) pending entry — even though the newer question's requestId doesn't match.

This can silently cancel a live pending question and, downstream in interactive.ts (Line 606) and session-queue.ts, cause replies to be dropped or misrouted.

🐛 Verify requestId ownership before clearing
 export function clearPendingQuestionByRequestId(requestId: string): void {
   const chatId = chatIdByRequestId.get(requestId)
-  if (chatId) pendingByChatId.delete(chatId)
+  if (chatId) {
+    const current = pendingByChatId.get(chatId)
+    // Only clear if the chat's current pending entry still belongs to this requestId;
+    // otherwise a stale/late clear would wipe out a newer pending question.
+    if (current?.requestId === requestId) pendingByChatId.delete(chatId)
+  }
   chatIdByRequestId.delete(requestId)
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const pendingByChatId = new TtlMap<PendingQuestion>(PENDING_QUESTION_TTL_MS)
const chatIdByRequestId = new TtlMap<string>(PENDING_QUESTION_TTL_MS)
export function registerPendingQuestion(params: {
request: QuestionRequest
chatId: string
sessionId: string
}): void {
const requestId = String(params.request.id ?? "")
if (!requestId) return
pendingByChatId.set(params.chatId, {
requestId,
sessionId: params.sessionId,
chatId: params.chatId,
request: params.request,
})
chatIdByRequestId.set(requestId, params.chatId)
}
export function clearPendingQuestionByRequestId(requestId: string): void {
const chatId = chatIdByRequestId.get(requestId)
if (chatId) pendingByChatId.delete(chatId)
chatIdByRequestId.delete(requestId)
}
const pendingByChatId = new TtlMap<PendingQuestion>(PENDING_QUESTION_TTL_MS)
const chatIdByRequestId = new TtlMap<string>(PENDING_QUESTION_TTL_MS)
export function registerPendingQuestion(params: {
request: QuestionRequest
chatId: string
sessionId: string
}): void {
const requestId = String(params.request.id ?? "")
if (!requestId) return
pendingByChatId.set(params.chatId, {
requestId,
sessionId: params.sessionId,
chatId: params.chatId,
request: params.request,
})
chatIdByRequestId.set(requestId, params.chatId)
}
export function clearPendingQuestionByRequestId(requestId: string): void {
const chatId = chatIdByRequestId.get(requestId)
if (chatId) {
const current = pendingByChatId.get(chatId)
// Only clear if the chat's current pending entry still belongs to this requestId;
// otherwise a stale/late clear would wipe out a newer pending question.
if (current?.requestId === requestId) pendingByChatId.delete(chatId)
}
chatIdByRequestId.delete(requestId)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/handler/pending-questions.ts` around lines 28 - 52,
`registerPendingQuestion` and `clearPendingQuestionByRequestId` can let an old
requestId delete a newer pending question for the same chat. Update the
`pendingByChatId`/`chatIdByRequestId` bookkeeping so each chat only has one
active requestId mapping at a time, and before clearing in
`clearPendingQuestionByRequestId` verify the stored `PendingQuestion.requestId`
still matches the requestId being cleared. Use the existing `PendingQuestion`,
`registerPendingQuestion`, and `clearPendingQuestionByRequestId` symbols to make
the stale-entry cleanup safe without removing a newer pending question.


export function buildQuestionFallbackText(request: QuestionRequest): string {
const question = getFirstQuestion(request)
const header = String(question?.header ?? "AI 提问")
const questionText = String(question?.question ?? "请选择")
const options = normalizeOptions(request)

const lines = [`${header}`, "", questionText]
if (options.length > 0) {
lines.push("", "可选回复:")
options.forEach((option, idx) => {
lines.push(`${idx + 1}. ${option.label}`)
})
}
lines.push("", "你可以直接回复 1/2,或回复“继续”“取消”。")
return lines.join("\n")
}

export function parsePendingQuestionReply(
content: string,
request: QuestionRequest,
): ParsedPendingQuestionReply | undefined {
const text = content.trim()
if (!text) return undefined

const options = normalizeOptions(request)
const lower = text.toLowerCase()

if (/^[1-9]\d*$/.test(text)) {
const idx = Number(text) - 1
if (options.length === 0) return { answers: [[text]], displayText: text }
if (idx >= 0 && idx < options.length) {
const selected = options[idx]
return { answers: [[selected.value]], displayText: selected.label }
}
return undefined
}

if (isContinueText(lower, text)) {
const selected = findOption(options, ["继续", "continue", "proceed", "yes", "allow", "approve"]) ?? options[0]
return {
answers: [[selected?.value ?? text]],
displayText: selected?.label ?? text,
}
}

if (isCancelText(lower, text)) {
const selected = findOption(options, ["取消", "cancel", "stop", "reject", "deny", "no"])
return {
answers: [[selected?.value ?? text]],
displayText: selected?.label ?? text,
}
}

return undefined
}

export async function tryResolvePendingQuestionText(params: {
chatId: string
content: string
deps: PendingQuestionDeps
}): Promise<boolean> {
const pending = pendingByChatId.get(params.chatId)
if (!pending) return false

const parsed = parsePendingQuestionReply(params.content, pending.request)
if (!parsed) return false

if (!params.deps.v2Client) {
params.deps.log("warn", "OpenCode client 未配置,无法处理纯文本问答回复", {
requestId: pending.requestId,
sessionId: pending.sessionId,
chatId: params.chatId,
})
await sender.sendTextMessage(params.deps.feishuClient, params.chatId, "当前环境无法提交这个回答,请稍后重试。", params.deps.log)
return true
}

try {
await params.deps.v2Client.question.reply({
requestID: pending.requestId,
answers: parsed.answers,
})
clearPendingQuestionByRequestId(pending.requestId)
emitQuestionPhase(pending.sessionId, "completed", "用户已通过纯文本回答问题。", params.deps.log)
await sender.sendTextMessage(params.deps.feishuClient, params.chatId, `已收到选择:${parsed.displayText}`, params.deps.log)
} catch (err) {
params.deps.log("error", "纯文本问答回复提交失败", {
requestId: pending.requestId,
sessionId: pending.sessionId,
chatId: params.chatId,
error: err instanceof Error ? err.message : String(err),
})
emitQuestionPhase(pending.sessionId, "error", "问答回调转发失败。", params.deps.log)
await sender.sendTextMessage(params.deps.feishuClient, params.chatId, "提交回答失败,请稍后重试。", params.deps.log)
}

return true
}

function emitQuestionPhase(
sessionId: string,
status: "completed" | "error",
body: string,
log: LogFn,
): void {
emit(sessionId, {
type: "details-updated",
sessionId,
phase: {
phaseId: "question",
label: "等待答复",
status,
body,
updatedAt: new Date().toISOString(),
},
}, log)
}

function getFirstQuestion(request: QuestionRequest): NonNullable<QuestionRequest["questions"]>[number] | undefined {
return request.questions?.[0]
}

function normalizeOptions(request: QuestionRequest): Array<{ label: string; value: string }> {
const rawOptions = getFirstQuestion(request)?.options
if (!Array.isArray(rawOptions)) return []
return rawOptions
.map((option, idx) => ({
label: String(option.label ?? option.value ?? `选项 ${idx + 1}`),
value: String(option.value ?? option.label ?? ""),
}))
.filter((option) => option.value.trim().length > 0 || option.label.trim().length > 0)
}

function findOption(
options: Array<{ label: string; value: string }>,
needles: string[],
): { label: string; value: string } | undefined {
return options.find((option) => {
const haystack = `${option.label}\n${option.value}`.toLowerCase()
return needles.some((needle) => haystack.includes(needle.toLowerCase()))
})
}

function isContinueText(lower: string, raw: string): boolean {
return lower === "continue" || lower === "go on" || raw === "继续" || raw === "继续执行"
}

function isCancelText(lower: string, raw: string): boolean {
return lower === "cancel" || lower === "stop" || raw === "取消" || raw === "停止"
}
14 changes: 14 additions & 0 deletions src/handler/session-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import type { FeishuMessageContext } from "../types.js"
import { handleChat, type ChatDeps } from "./chat.js"
import { buildSessionKey } from "../session.js"
import { tryResolvePendingQuestionText } from "./pending-questions.js"

/** 单条待处理消息及其运行依赖。 */
interface QueuedMessage {
Expand Down Expand Up @@ -59,6 +60,19 @@ function cleanupStateIfIdle(sessionKey: string, state: QueueState): void {
* - 需要回复的消息则按 sessionKey 归并到串行队列
*/
export async function enqueueMessage(ctx: FeishuMessageContext, deps: ChatDeps): Promise<void> {
if (
ctx.shouldReply &&
ctx.messageType === "text" &&
deps.interactiveDeps &&
await tryResolvePendingQuestionText({
chatId: ctx.chatId,
content: ctx.content,
deps: deps.interactiveDeps,
})
) {
return
}

Comment on lines 62 to +75

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Pending-question fast path bypasses per-sessionKey serialization and can double-submit an answer.

This early-return path runs tryResolvePendingQuestionText outside the FIFO queue, before handleQueuedMessage/drainLoop ever gets involved. tryResolvePendingQuestionText (see src/handler/pending-questions.ts, Lines 110-151) reads pendingByChatId.get(chatId) synchronously and only clears the entry after awaiting v2Client.question.reply(...). If two text messages for the same chat race in close together (double send, retry, multi-device), both can read the same pending entry before either clears it, resulting in two calls to v2Client.question.reply for the same requestId — a non-idempotent external write without a dedup guard.

As per path instructions, src/handler/session-queue.ts is meant to "Implement per-sessionKey FIFO queue to prevent message race conditions and ensure serial ordering," but this new fast path sits ahead of that queue and doesn't inherit its serialization guarantee.

🔒 Guard against concurrent double-submission

Reuse the existing markSeen/unmarkSeen-style dedup guard (already used in interactive.ts) keyed by the pending requestId inside tryResolvePendingQuestionText, or claim/clear the pending entry synchronously before the await on v2Client.question.reply, so a second concurrent caller for the same chat sees no pending entry to act on.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/handler/session-queue.ts` around lines 62 - 75, The pending-question fast
path in enqueueMessage bypasses the per-sessionKey FIFO serialization and can
double-submit the same answer. Update tryResolvePendingQuestionText to claim the
pending entry or add a markSeen/unmarkSeen-style guard keyed by requestId before
awaiting v2Client.question.reply, so concurrent calls for the same chat cannot
both resolve the same pending question. Keep the fix aligned with the existing
session-queue and pending-questions flow so the fast path remains safe under
races.

Source: Path instructions

// 静默消息只做上下文同步,不需要排队等待 UI 回复链路。
if (!ctx.shouldReply) {
await handleChat(ctx, deps)
Expand Down
15 changes: 14 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,19 @@ function loadFeishuRuntimePrompt(): string {
/** 缓存的飞书运行时 prompt,在模块加载时一次性读取 */
const feishuRuntimePrompt = loadFeishuRuntimePrompt()

/**
* 构建插件内部 OpenCode v2 client 配置。
*
* 插件宿主可能使用动态端口启动 OpenCode server;这里必须使用 ctx.serverUrl,
* 否则 abort_reply 等后台回调会退回 SDK 默认地址并在动态端口下失败。
*/
export function buildV2ClientConfig(input: { serverUrl?: URL; directory: string }): NonNullable<Parameters<typeof createOpencodeClient>[0]> {
return {
baseUrl: input.serverUrl?.toString(),
directory: input.directory || undefined,
}
}

/**
* OpenCode 插件入口导出。
*
Expand Down Expand Up @@ -144,7 +157,7 @@ export const FeishuPlugin: Plugin = async (ctx) => {
const botOpenId = await fetchBotOpenId(larkClient, log)

// v2 client 主要用于权限审批/问答交互回调。
const v2Client = createOpencodeClient({ directory: resolvedConfig.directory || undefined })
const v2Client = createOpencodeClient(buildV2ClientConfig({ serverUrl: ctx.serverUrl, directory: resolvedConfig.directory }))
const interactiveDeps: InteractiveDeps = { feishuClient: larkClient, log, v2Client }

// 启动飞书 WebSocket 网关(复用 larkClient)
Expand Down
32 changes: 32 additions & 0 deletions tests/index-v2-client.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* index.ts v2 client config regression test.
*
* Ensures the Feishu plugin binds its secondary OpenCode SDK client
* to the running server URL supplied by the plugin host.
* Run: npx tsx --test tests/index-v2-client.test.ts
*/
import { describe, it } from "node:test"
import assert from "node:assert/strict"
import { buildV2ClientConfig } from "../src/index.js"

describe("buildV2ClientConfig", () => {
it("uses plugin ctx.serverUrl as SDK baseUrl", () => {
const config = buildV2ClientConfig({
serverUrl: new URL("http://127.0.0.1:55249"),
directory: "/repo",
})

assert.equal(config.baseUrl, "http://127.0.0.1:55249/")
assert.equal(config.directory, "/repo")
})

it("omits blank directory while preserving baseUrl", () => {
const config = buildV2ClientConfig({
serverUrl: new URL("http://localhost:4096"),
directory: "",
})

assert.equal(config.baseUrl, "http://localhost:4096/")
assert.equal(config.directory, undefined)
})
})
Loading