fix(desktop): serialize parallel permission interactions to prevent 600s hang (#3092) - #3255
fix(desktop): serialize parallel permission interactions to prevent 600s hang (#3092)#3255yuaiccc wants to merge 22 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c67622b79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
| Filename | Overview |
|---|---|
| apps/desktop/src/main/maker-ipc/register.ts | 新增 session 级 PermissionQueue、生命周期 drain、绝对超时与 takeover barrier；队列本身保留旧 tail 的修复方向正确。 |
| apps/desktop/src/main/im/shared/turnRunner.ts | 新增迁移交互所有权、超时及跨生命周期收口，但 takeover barrier 在异步渠道发布完成前释放。 |
| apps/desktop/src/main/im/shared/migratedInteractionSettleRegistry.ts | 使用 session 到 settler 集合支持多渠道并存，修复单回调覆盖问题。 |
| apps/desktop/src/main/im/shared/pendingInteractions.ts | 为迁移卡片增加剩余超时和超时收口，并在 resolve/cancel/reject 时清理 timer。 |
| apps/desktop/src/main/im/dingtalk/interaction.ts | 透传迁移交互剩余期限，并将带合法 decision 的取消异常还原为业务结果。 |
| packages/lizi-im/src/dingtalk/index.ts | 扩展 DingTalk 文本回复的 requestId、取消和超时协作，以支持迁移交互生命周期。 |
Sequence Diagram
sequenceDiagram
participant D as Desktop PermissionQueue
participant T as IM turnRunner
participant C as 渠道交互接口
participant N as 新 Desktop permission
T->>D: takePendingInteractionsForSession()
D-->>T: migrated cohort + takeover barrier
T->>C: void publishMigratedInteraction()
T->>D: completeTakeover()
Note over C: 卡片发送或文本 waiter 仍可能未完成
N->>D: dispatch()
D-->>N: barrier 已释放，继续 Desktop 广播
C-->>T: 渠道稍后才接收迁移交互
Prompt To Fix All With AI
### Issue 1
apps/desktop/src/main/im/shared/turnRunner.ts:1786-1790
**接管屏障提前释放**
当渠道仍在发送迁移卡片或建立文本 waiter 时，这里的 `finally` 已调用 `completePermissionQueueTakeoverForSession`；由于两种迁移发布路径都是 fire-and-forget，新的 Desktop permission 会越过屏障并与尚未被渠道接收的迁移交互并发，导致确认卡覆盖、错误路由或请求等待至超时。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (24): Last reviewed commit: "fix(im): settle migrated interaction cle..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 946e0e3422
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
MagicLizi
added
awaiting-discussion
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
P1：会话完整 close 时，排队里还没开始的 permission 仍会在前一张卡 settle 后跑起来，复现同类 600s 挂死。
请先修这一条再合。
|
@yuaiccc 👋 这个 PR 还有 4 条 review conversation 没 resolve(apps/desktop/src/main/maker-ipc/register.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55b2271600
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6baa5b52cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
MagicLizi
added
awaiting-discussion
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
格式门未通过，请先补齐 PR 模板后再请求审查。
- Description 缺段落：
这次改了什么/怎么验证的/风险
当前正文有问题/根因/修复/测试，但标题与模板字段对不上，机器门禁和后续审查都对不齐。
请按仓库 .github/PULL_REQUEST_TEMPLATE.md 用这三段重写 Description（可把现有根因/测试内容挪进对应段），推新 commit 或 gh pr edit 更新描述后即可再审。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea94d21308
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@yuaiccc 👋 这个 PR 还有 8 条 review conversation 没 resolve(apps/desktop/src/main/maker-ipc/register.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
MagicLizi
added
awaiting-discussion
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad3e3420af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
MagicLizi
added
awaiting-discussion
MagicLizi
left a comment
MagicLizi
left a comment
There was a problem hiding this comment.
格式门未通过，请先补齐 PR 模板后再请求审查。
- Description 缺段落：
这次改了什么/怎么验证的/风险
当前正文有问题/根因/修复/测试，但标题与模板字段对不上，机器门禁和后续审查都对不齐。
请按仓库 .github/PULL_REQUEST_TEMPLATE.md 用这三段重写 Description（可把现有根因/测试内容挪进对应段），用 gh pr edit 更新描述后即可再审。
Two more review findings on makecindy#3092: 1. Timeout buildup (Codex P2): a permission queued behind another did not start its 10-minute timer until it reached the front, so N unanswered parallel permissions could extend the effective cap from 600s to N×600s. dispatch() now accepts a timeoutMs that covers queue wait + execution and settles denied on expiry. The Desktop listener passes PERMISSION_INTERACTION_TIMEOUT_MS. 2. resetForNewTurn race (Greptile P1): the old implementation set a drained flag and cleared it asynchronously after the old tail settled, so a new-turn permission arriving in that window observed the stale flag and returned a deny without showing a card. Replace the single flag with a monotonic generation plus a per-generation drain decision: a run captured at enqueue time checks its generation when it reaches the front, reset bumps the generation and replaces the tail immediately, and new dispatches run on the fresh generation without waiting. Drain decisions are reaped lazily to keep the map bounded. - Add a fake-timer regression test asserting the queued second permission settles within ~one timeout, not two. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Greptile P1 / Codex P2 on makecindy#3092: when a queued permission hit its overall timeout, the outer promise resolved deny to the agent, but the queued run (next) remained in the tail. Once the in-flight permission settled, the timed-out run still executed its broadcast, producing an orphan card that nobody awaited and blocking later permissions for another 10 minutes. - runIfCurrent now takes an isTimedOut predicate; the dispatch's timeout sets a per-dispatch stale flag and, when the run eventually reaches the front, it skips the handler and returns a timeout deny instead of broadcasting. - Strengthen the timeout regression test to assert the second run's body never executes even after A is released and B reaches the front. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Greptile P1 on makecindy#3092: resetForNewTurn replaced the tail with a fresh resolved promise so that a new-turn permission could start immediately. But if an old permission was still in-flight (its handler executing), the new permission broadcast concurrently and both wrote to the renderer's single pendingPermission slot — reintroducing the original overwrite/hang. resetForNewTurn now only bumps the generation and records a drain decision for the old cohort; it does NOT replace the tail. The in-flight run continues to completion (it already passed runIfCurrent), old queued runs behind it see the bumped generation and settle with the drain decision without broadcasting, and a new-turn dispatch chains onto the same tail — so it waits for the in-flight run to finish and never broadcasts concurrently. This preserves the single-slot serialization guarantee across abort / turn-idle reconcile / orca disable / channel takeover. - Add a regression test asserting a post-reset dispatch does not start while the in-flight permission is running and only runs after it settles. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Greptile P1 on makecindy#3092: when a permission queued behind another reached the front and its handler had already started (registered a pending resolver + broadcast the card), the outer queue timeout only resolved the agent's Promise with a deny — it did not clear the pending interaction or dismiss the card. The agent moved on but the orphan card sat in the UI and kept the tail occupied until the inner 10-minute timer or a user click settled it. - dispatch now hands the run a PermissionRunContext with bindTimeoutCleanup(fn); the Desktop handler registers a teardown that, on outer timeout, clears the pending resolver, clears the inner timer, dismisses the renderer interaction, and resolves the entry with timeout-deny. - runIfCurrent takes the context instead of a bare isStale predicate. - Existing timeout/serialization tests still pass. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Codex P2 on makecindy#3092: when Feishu/Discord/Slack took over a turn via /ctr, the in-flight Desktop permission was migrated (its resolver handed to the taken-over route via takePendingInteractionsForSession), but the permission queue kept waiting on that migrated Promise. If the migrated card sat unanswered for the 10-minute outer timeout, the timeout cleanup found no pending entry (already taken), the queued next Promise never settled, and the Desktop queue stayed blocked. - Add PermissionQueue.skipInFlightForTakeover: bumps the generation and replaces the tail with a resolved seed so subsequent Desktop dispatches don't wait on the migrated card, while the in-flight Promise continues independently on the taken-over route. - cancelPermissionQueueForSession takes a mode ('close' | 'reset' | 'takeover'); cleanupPendingAgentInteractionsForSession uses close/ reset as before, and takePendingInteractionsForSession uses takeover to detach the migrated in-flight permission. Queued items behind it still drain with the migration decision. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>
Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
…nd abort Address two review findings on migrated IM interactions: 1. Permissions queued behind an unanswered ask/plan in a text channel's serial publish loop now pre-register ownership and an absolute-deadline watchdog for the whole cohort before the serial await. The watchdog denies each permission at its original deadline instead of hanging the SDK Promise for the whole serial wait. 2. Desktop Stop (ABORT_SESSION / input_stop) now settles interactions migrated to an IM channel via a leaf settle registry, avoiding a reverse import cycle between maker-ipc/register and im/shared/turnRunner. Ownership registration is idempotent so the text cohort loop and the rich-card path can both call it; the idempotent resolve and per-publish cancelled() flag keep external settles from re-orphaning a card. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
153b3f2 to
8aa1f79
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8aa1f7904a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@yuaiccc 👋 这个 PR 还有 3 条 review conversation 没 resolve(apps/desktop/src/main/maker-ipc/register.ts / apps/desktop/src/main/im/shared/turnRunner.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c3dfe2f0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@yuaiccc 👋 这个 PR 还有 2 条 review conversation 没 resolve(apps/desktop/src/main/im/shared/migratedInteractionSettleRegistry.ts / apps/desktop/src/main/im/shared/turnRunner.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ae168d949
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…y#3255) Signed-off-by: yuaiccc <yuaiccc@aliyun.com>
There was a problem hiding this comment.
💡 Codex Review
cindy/packages/lizi-im/src/dingtalk/index.ts
Line 373 in d5987b0
Handle cancellation without rejecting an unwatched promise
When /stop or session cleanup calls cancelTextReply() while the initial sendText() is still pending, this rejects the internal reply promise before requestTextReply() reaches return reply, so no rejection handler is attached yet and Electron emits a process-level unhandledRejection; the caller's catch cannot observe it until the send completes. Attach a rejection handler when the promise is created, or settle cancellation without rejecting a temporarily detached promise.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@yuaiccc 👋 这个 PR 还有 1 条 review conversation 没 resolve(apps/desktop/src/main/im/shared/turnRunner.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
Signed-off-by: Dash <125997726+dashhuang@users.noreply.github.com>
|
已在 |
| } finally { | ||
| // Keep the Desktop queue fenced until the channel has accepted the | ||
| // migrated cohort, preventing a new Desktop permission from racing the | ||
| // handoff and overwriting the renderer's singleton permission slot. | ||
| completePermissionQueueTakeoverForSession(row.id); |
There was a problem hiding this comment.
当渠道仍在发送迁移卡片或建立文本 waiter 时，这里的 finally 已调用 completePermissionQueueTakeoverForSession；由于两种迁移发布路径都是 fire-and-forget，新的 Desktop permission 会越过屏障并与尚未被渠道接收的迁移交互并发，导致确认卡覆盖、错误路由或请求等待至超时。
Context Used: 使用和PR描述相同的语言进行评论 (source)
Knowledge Base Used: Messaging channel delivery
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/im/shared/turnRunner.ts
Line: 1786-1790
Comment:
**接管屏障提前释放**
当渠道仍在发送迁移卡片或建立文本 waiter 时，这里的 `finally` 已调用 `completePermissionQueueTakeoverForSession`；由于两种迁移发布路径都是 fire-and-forget，新的 Desktop permission 会越过屏障并与尚未被渠道接收的迁移交互并发，导致确认卡覆盖、错误路由或请求等待至超时。
**Context Used:** 使用和PR描述相同的语言进行评论 ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
**Knowledge Base Used:** [Messaging channel delivery](https://app.greptile.com/xindong/-/custom-context/knowledge-base/makecindy/cindy/-/docs/messaging-channel-delivery.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.3 participants
这次改了什么
摘要
修复 issue #3092：Auto 审批模式下，Agent 在同一批并行发出两个需要确认的
tool_use时，先发的那个会挂死整整 600 秒，后发的秒回。根因：renderer 的权限确认只有一个
pendingPermission单 slot（makerChatStore.pendingPermission是PendingPermission | null）。两个并行 permission 请求到达 main 进程后，第二个INTERACTION_REQUEST广播会覆盖第一个在 renderer 里的 slot，第一个请求的 resolver 永远不会被调用，只能等 10 分钟的PERMISSION_INTERACTION_TIMEOUT_MS兜底超时。修复方式：在 Desktop interaction listener 边界，对同一 session 的 permission 请求做按序串行化（
PermissionQueue），让 renderer 的单 slot 一次只收到一张卡。每个 toolUse 自己的 Promise 仍独立等待，只是按到达顺序 resolve。同时处理了串行化引入的所有边界：/ctr接管时，把已在 Desktop 显示的 in-flight permission 从队列 tail 摘离（resolver 已交给接管渠道），队列不再等它；后续 Desktop permission 不被阻塞。变更类型
fix缺陷修复范围
apps/desktop/src/main/maker-ipc/register.ts的 permission 串行队列与生命周期处理；新增单测ask_user_question/plan_review不串行（它们有不同的生命周期且 renderer 已单独排队）UI 变化
不涉及：无 UI / 文案 / 视觉改动，仅 main 进程权限调度逻辑变化。
怎么验证的
自动验证
CI（head
95a5dbf）全绿：Linux unit tests (1/2)(2/2)、Windows unit tests (1/2)(2/2)、verify-checks、Greptile Review、DCO、Desktop Git integration、check:pr-design-basis 均 pass。单测覆盖：
手工验证
不涉及：改动是 main 进程内部调度，无手工 UI 路径；竞态由单测确定性复现与验证。
未执行的验证
未在真实 Feishu/Discord/Slack 接管流程上端到端验证（
/ctr路径用skipInFlightForTakeover摘离 in-flight，逻辑由单测 + 类型检查覆盖）。风险
风险分类
影响与回滚
提交前检查