fix(desktop): 副窗口中会话被归档后关闭窗口,禁止继续对话 (#3175) - #3262
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: 85356f3886
ℹ️ 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/goal.ts | 为副窗口和显式远程请求增加持久化生命周期门禁，并将 route-lock 所有权传入 Goal 控制器。 |
| apps/desktop/src/main/goal-host/controller.ts | Goal 设置、更新、恢复及重新打开路径完整传递 sessionRouteLockHeld，持锁恢复会等待派发结束。 |
| apps/desktop/src/main/maker-ipc/register.ts | pending-agent-switch holder 新增已持锁分支，应用待处理路由后返回空 release，避免非重入锁自死锁。 |
| apps/desktop/src/renderer/lib/makerChatStore.ts | 远程乐观发送失败处理改为请求级回滚，避免一个失败请求影响其他在途请求。 |
| apps/desktop/src/main/maker-ipc/agent-input-coordinator.ts | 队列恢复与重试在真正派发前重新核验持久化 active 状态，阻止归档竞态重新激活任务。 |
| apps/desktop/src/main/bootstrap-electron.ts | Goal、Learn 和远程命令 IPC 注入统一的设备来源判断、会话锁及 active 状态断言。 |
Sequence Diagram
sequenceDiagram
participant W as 独立副窗口
participant M as Electron Main
participant DB as 持久化会话状态
participant G as Goal Controller
participant S as Agent Session
W->>M: Goal/继续对话请求
M->>M: 获取 session route lock
M->>DB: 校验会话仍为 active
alt 会话已归档
DB-->>M: archived
M-->>W: 拒绝派发并关闭窗口
else 会话仍 active
DB-->>M: active
M->>G: 调用并传递 sessionRouteLockHeld
G->>G: 应用 pending route（不重复加锁）
G->>S: ensureSession + send
S-->>G: 派发结果
G-->>M: 完成
M->>M: 释放 route lock
end
Reviews (35): Last reviewed commit: "fix(desktop): wait for fenced clear life..." | Re-trigger Greptile
|
这条改动会改变用户看得见的交互：副窗口里的会话被主窗口归档后，副窗会自动关闭，而不只是停在已归档会话上。这属于产品行为，需要维护者确认后再合。 请维护者在本 PR 上 Approve；如果希望改成「留窗但禁止发送/恢复」或其它收口，请 Request Changes。 讨论 issue：#3263 |
MagicLizi
added
awaiting-discussion
85356f3 to
d543d64
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d543d64b04
ℹ️ 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/renderer/features/cc-agent/CCAgentSessionView.tsx),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
|
命中 UI 路径(apps/desktop/src/renderer/features/cc-agent/CCAgentSessionView.tsx)但 description 未附界面效果证据——建议补充改动后效果:截图/录屏,或改动后界面的 HTML 页面(```html 代码块、.html 附件或在线预览链接),便于确认界面符合 DESIGN.md 设计规范 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d497cca98e
ℹ️ 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: 124fde79a8
ℹ️ 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: 45be85fdd1
ℹ️ 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/renderer/features/cc-agent/CCAgentSessionView.tsx),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: 289f03d4df
ℹ️ 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".
|
收敛检查点（HEAD
|
|
收敛检查点：副窗口一旦得知其路由任务已归档，普通发送、排队操作与恢复入口都不得再派发工作；脏文件取消关窗时保留编辑器，但所有派发路径继续保持 inert。 已审计路径：普通 / Workdir / Orca 关窗所有权，send / steer，附件物化后的入队，device-link 断线重连，错误 / 中断 / silent-stop / provider-switch retry，以及队列 Resume / Steer。可鉴别回归覆盖重连后再次执行生命周期 fence、历史读取期间归档时拒绝副窗 retry、主窗归档任务 retry 保持既有恢复语义，以及归档副窗移除队列派发回调。 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2defbf9c52
ℹ️ 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: f950f29142
ℹ️ 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/renderer/features/cc-agent/CCAgentSessionView.tsx / apps/desktop/src/renderer/lib/makerChatStore.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: 9cd208ff8b
ℹ️ 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/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: 8ed339149d
ℹ️ 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/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: 0bc01deb77
ℹ️ 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: 6dd8088b62
ℹ️ 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: e282539c05
ℹ️ 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".
Signed-off-by: Dash <dashhuang@gmail.com>
Signed-off-by: Dash <dashhuang@gmail.com>
Signed-off-by: Dash <dashhuang@gmail.com>
Signed-off-by: Dash <dashhuang@gmail.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: 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>
Address Codex P2 finding on goal.ts: the lifecycle guard only fired for device-link invokes with requireActiveSession, while a local secondary window (which hosts a GoalIndicator on a full session) could call resumeGoal/updateGoal/clearGoal/setGoal and bypass the active-session fence — re-activating an archived task. Derive secondary-window status from the real event.sender via isSecondaryAppWindow(BrowserWindow.fromWebContents(event.sender)), complementary to the device-link requireActiveSession marker (device-link synthetic events have no sender). Either path now takes the route lock + persistent active assertion. runWithLifecycleGuard reports whether it fenced so GOAL_SET sets sessionRouteLockHeld consistently for both paths. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Windows (1/2) failed one unrelated test in packages/maker-core (cindySubagentRunner 'honours a stop that shares a batch with an earlier approval') with a 60s waitFor timeout, while Linux (1/2)(2/2) and Windows (2/2) all passed. This PR only touches desktop main/renderer goal lifecycle files (goal.ts, bootstrap-electron.ts, goalHandlers.test.ts) and does not modify maker-core, so this is a Windows-only flake. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Address Codex P2 (goal.ts:128): GOAL_GET_STATUS looked like a read but its resumeOnOpen() path has side effects — it rebuilds the Agent session and may continue a dormant active goal. After a session was archived and its auto-close cancelled over unsaved files, a secondary-window GoalIndicator re-binding (renderer reload / session origin re-bind) could call getGoalStatus and re-activate the archived task, because set/resume/update /clear were fenced but this auto-recovery read was not. - Wrap resumeOnOpen in runWithLifecycleGuard so local secondary windows (detected via the real event.sender) and device-link secondary windows (requireActiveSession marker, since their synthetic events have no sender) run it under the route lock + persistent active assertion. - Primary windows / primary remote keep the historical resume-on-open semantics (opening a main-window session is allowed to recover). - When the fence reports the session is no longer active, GET_STATUS degrades to returning the pre-recovery snapshot instead of resuming or throwing at the renderer. - Thread requireActiveSession through makerTransport for the remote secondary-window getGoalStatus path (local main process auto-fences via sender, so no preload change is needed). - Add regression tests for both remote and local secondary-window paths. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
…eadlock Review P2: runWithLifecycleGuard holds the session route lock, then calls controller.setGoal/resumeGoal/resumeOnOpen which re-enter the same non-reentrant lock via acquirePendingAgentSwitchForDirectSend → acquireSendToSessionLock, deadlocking the IPC. The holder already supports a sessionRouteLockHeld option to skip the second acquisition. Propagate the fenced flag through every side-effecting Goal path: - setGoal: input.sessionRouteLockHeld (already consumed by fireTurn) - resumeGoal: new opt, forwarded to fireTurn - resumeOnOpen: new opt, forwarded to acquirePendingAgentSwitch - goal.ts fence: pass the flag when fenced, omit it otherwise Also propagate requireActiveSession through the remote secondary-window goal transport (setGoal/clearGoal/resumeGoal/updateGoal), not just getGoalStatus (review P1): synthetic device-link events have no sender, so without the marker the guard treats them as primary-window requests and a secondary window can restart an archived task. Tests: update goalHandlers assertions for the new flag, add a send-to-session ordering contract for the holder fast-path, and fix the SET_MODEL needle that drifted to `options` in this branch. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
…l IPC Address remaining review threads on makecindy#3262: - GOAL_UPDATE fenced path also re-enters the route lock: updateGoal resumes paused/blocked goals via resumeGoal→fireTurn when the objective changes, so pass sessionRouteLockHeld through updateGoal and its resumeGoal call. Previously only setGoal/resumeGoal/resumeOnOpen had the marker, leaving updateGoal to self-deadlock. - isSecondaryWindowEvent must tolerate a missing sender. Device-link synthetic events set sender to undefined (invoke-registry); calling BrowserWindow.fromWebContents(undefined) threw before the request reached the controller. Return false for those calls — they are gated by the explicit requireActiveSession marker instead. - Spread sessionRouteLockHeld only when fenced so non-fence callers keep the bare single/two-arg shape (goalHandlers tests updated). Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
The remote secondary-window fence used [sid, remoteFenceOpts()] for
clearGoal/resumeGoal. When not in a secondary window remoteFenceOpts()
returned undefined, but the argument was still passed as a second
element, so spies/IPC saw (sid, undefined) instead of the bare (sid)
shape the transport-routing contract and its tests expect. Use a
conditional spread so non-secondary remote calls keep the single-arg
form (matching getGoalStatus), and only append {requireActiveSession}
for secondary windows.
Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
…n active session The secondary-window archive fence only blocked composer/queue input. Three other entry points could still start an Agent turn on an archived task when the auto-close was cancelled over dirty files (makecindy#3262 P2): - Compact: both Pi (COMPACT_SESSION) and Claude (INPUT_COMPACT via the input coordinator) now run assertSessionActiveForManualDispatch when the caller is a secondary window (local by sender, or device-link with requireActiveSession). Renderer passes the marker through makerTransport.compactSession and makerChatStore.compactSession. - Worker dispatch: WORKER_DISPATCH_UI_ASSIGNMENT re-checks the Lead's persisted active status inside the serialized claim, immediately before sendToWorker, instead of trusting the renderer's cached state. - Interaction resolve: RESOLVE_INTERACTION looks up the pending interaction's owning session and asserts it is still active before resolving; approving a permission / answering ask_user / confirming a plan on an archived session is now rejected. The existing assertSessionActiveForManualDispatch is reused (DB status check + route lock semantics), so main is the single enforcement boundary for all three paths. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
The prior active-session checks for worker dispatch, compact, and interaction resolve were bare awaits, leaving a TOCTOU window: the check could observe 'active', then an archive committed under the route lock, and the guarded operation still ran (makecindy#3262 P2 follow-up). - WORKER_DISPATCH_UI_ASSIGNMENT: wrap assertSessionActive + sendToWorker in withSendToSessionLock(leadSessionId) so the check and worker dispatch are atomic with respect to Lead archive. - RESOLVE_INTERACTION: wrap assertSessionActive + resolvePendingInteraction in the owning session's route lock; approving a permission / answering ask_user / confirming a plan can no longer land on an archived turn. - Compact: carry requireActiveSession through PendingCompactRequest and re-check isSessionActiveForManualDispatch right before sendToAgent in dispatchCompact, covering the case where the session is archived while the compact sits in pendingCompacts. INPUT_COMPACT forwards the flag into the coordinator in addition to its enqueue-time check. Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
…on fences
Three P1 findings after the prior commit:
- makerTransport.compactSession dropped the optional instructions slot
when appending the fence, so a secondary window without instructions
sent [sessionId, {requireActiveSession}] and the controlled endpoint
rejected the fence object as INVALID_PARAMS. Keep the undefined
placeholder: [sessionId, instructions, fenceOpts].
- Claude compact (dispatchCompact) pre-checked active state but did not
carry requireActiveSession into sendToAgent options, so the final in-lock
fence never ran. Propagate the flag into sendOpts so sendToAgent
re-asserts under the route lock.
- Pi compact (COMPACT_SESSION) checked active and called
sess.compactSession() as two separate steps. Move both inside
withSendToSessionLock so an archive cannot land between the check and
the turn start. Non-fence callers retain the prior unlocked path.
Signed-off-by: yuaiccc <yuaiccc@aliaun.com>
Conflict resolution on the orcaTeamStore.test.ts afterEach block
inadvertently added a second 'import { setSessionRouteLockImplementation }
from ../sessionRouteLock.js' on the line right after the first one,
plus the new setSessionRuntimeCleanup import from main. TypeScript
rejects the duplicate identifier at compile time, which is what made
'Typecheck desktop' fail on CI for the previous run.
Signed-off-by: yuaiccc <yuaiccc@aliyun.com>
Signed-off-by: yuaiccc <yuaiccc@aliyun.com>
Signed-off-by: yuaiccc <yuaiccc@aliyun.com>
Signed-off-by: yuaiccc <yuaiccc@users.noreply.github.com>
Signed-off-by: yuaiccc <yuaiccc@aliyun.com>
Signed-off-by: yuaiccc <yuaiccc@aliyun.com>
912ee35 to
417b889
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 417b889ba1
ℹ️ 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".
| if (head.requireActiveSession && isSessionNotActiveError(err)) { | ||
| latest.activeTurn = null; | ||
| this.clearCredentialSwitchWait(latest); | ||
| this.deps.onDiscardedQueuedMessage?.(sessionId, head); |
There was a problem hiding this comment.
When a secondary window starts a manual Retry and another window archives the task after performRetryLastError's active precheck but before the locked send, this branch discards the fenced retry without restoring its recovery state. The retry path has already cleared error, stickyError, and recovery, while the renderer also calls disposeLiveErrorPersist() before the IPC result, so the archived task loses its actionable error and marks the persisted error dismissed even though no retry ran. Restore the manual retry checkpoint on SESSION_NOT_ACTIVE, or serialize the active check and retry transition with the session route lock.
AGENTS.md reference: AGENTS.md:L129-L132
Useful? React with 👍 / 👎.
|
@junshanxu 👋 这个 PR 还有 1 条 review conversation 没 resolve(apps/desktop/src/main/maker-ipc/agent-input-coordinator.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
3 participants
这次改了什么
摘要
修复独立副窗口仍持有已归档任务时可以继续派发输入的问题。副窗口会在观察到任务归档后关闭；即使归档与“继续队列”或远程乐观发送发生竞态，Main 也会在真正恢复派发前按持久化任务状态再次拦截。
变更类型
fix缺陷修复范围
UI 变化
windowClose()与分屏树收敛逻辑，属于独立辅助窗口的生命周期与故障隔离修复；对应docs/design-rules/DESIGN.md的多窗口一致性原则，以及docs/dev-rules/electron-security-and-process-boundaries.md§3.1“独立辅助窗口统一生命周期基线”。怎么验证的
自动验证
手工验证
未执行；本轮以生命周期、IPC 边界与远程恢复回归测试覆盖。
未执行的验证
风险
风险分类
影响与回滚
packages/device-link既有“双 peer 中一个停止 ACK、另一个投递零感知”用例。提交前检查
git commit -s）