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 | Goal IPC 新增副窗口生命周期门禁，但状态恢复仍将已释放的路由锁所有权传给异步派发。 |
| apps/desktop/src/main/goal-host/controller.ts | 锁所有权已覆盖同步 Goal 更新和恢复路径；waitForDispatch:false 的 resumeOnOpen 仍存在锁生命周期错位。 |
| apps/desktop/src/main/maker-ipc/register.ts | pending-agent-switch holder 正确避免同步路径的非重入锁死，但其 no-op 分支依赖调用方在整个派发期间持续持锁。 |
| apps/desktop/src/renderer/lib/makerChatStore.ts | 远程乐观发送恢复与请求级回滚得到扩展，未发现与既有 Goal 路由锁线程相关的新问题。 |
| apps/desktop/src/main/maker-ipc/agent-input-coordinator.ts | 队列 Resume/Retry 增加持久化 active 状态门禁，未发现与既有线程相关的新问题。 |
Sequence Diagram
sequenceDiagram
participant W as 副窗口
participant H as GOAL_GET_STATUS
participant L as Route Lock
participant R as resumeOnOpen
participant F as fireTurn
participant M as 并发路由修改
participant S as Session.send
W->>H: 查询 active/dormant Goal
H->>L: 获取锁并复核 active
H->>R: waitForDispatch:false, lockHeld:true
R->>F: 启动异步派发
R-->>H: 立即返回
H->>L: 释放锁
M->>L: 修改会话路由
F->>F: 因 lockHeld 跳过重新加锁
F->>S: 使用可能已过期的会话路由派发
Prompt To Fix All With AI
### Issue 1
apps/desktop/src/main/goal-host/controller.ts:1513
**异步派发提前失去路由锁**
当副窗口通过 `GOAL_GET_STATUS` 恢复 active、dormant Goal，并与 `SET_MODEL` 或其他路由修改并发时，`waitForDispatch: false` 让 `resumeOnOpen` 在 `fireTurn` 完成前返回并释放外层 route lock，而 `sessionRouteLockHeld: true` 又使异步派发跳过自行加锁，导致路由修改插入 `ensureSession` 与 `Session.send` 之间，最终将 Goal continuation 派发到旧的或与界面状态不匹配的会话路由。
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (33): Last reviewed commit: "fix(test): remove duplicate setSessionRo..." | 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".
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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f09c2a2a46
ℹ️ 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".
…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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c17557f8ec
ℹ️ 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".
…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>
c17557f to
e9ec9fb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9ec9fb115
ℹ️ 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".
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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faa231047e
ℹ️ 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/lib/makerTransport.ts / apps/desktop/src/main/maker-ipc/agent-input-coordinator.ts / apps/desktop/src/main/maker-ipc/register.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
…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>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62c0109370
ℹ️ 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".
| agentHandoffPending.sealClearBoundary(sid); | ||
| endRemoteInputClearGate(sid); | ||
| }; | ||
| if (isSecondarySessionWindowEvent(e) || requiresActiveSessionForDispatch(opts)) { |
There was a problem hiding this comment.
/clear 的窗口归属解析
当任一 device-link 会话执行 /clear 时，device-link/invoke-registry.ts 的 dispatchLocalInvoke 会构造 sender: undefined 的合成事件；这里新增的条件却总是先调用 isSecondarySessionWindowEvent(e)，继而把该空 sender 传给 BrowserWindow.fromWebContents，所以主窗口和副窗口的远程 /clear 都会在进入 runClear() 前抛错。相较此前 /context 的评论，新的证据是本次把同一个不安全的窗口解析加入了 INPUT_CLEAR_SESSION 路径；应先识别 device-link 调用，只有真实 Electron sender 才解析窗口归属。
Useful? React with 👍 / 👎.
| opts && !remoteDeviceId | ||
| ? clearOperation.api.input.clearSession(sessionId, clearedAt, opts) | ||
| : clearOperation.api.input.clearSession(sessionId, clearedAt); |
There was a problem hiding this comment.
/clear 的 active fence 继续传到被控端
当 device-link 副窗口在归档 push 到达前触发 /clear 时，调用方传入的 opts.requireActiveSession 会被这个 !remoteDeviceId 分支主动丢弃；而被控端收到的是无真实 sender 的合成事件，无法自行判断请求来自副窗口。修复合成 sender 的解析后，这会让清空操作在没有持久化 active 复核的情况下修改已归档任务；应像其它远程输入操作一样保留 clearedAt 的位置参数并继续转发 opts。 docs/dev-rules/electron-security-and-process-boundaries.mdL38-L40
Useful? React with 👍 / 👎.
| const pendingEntry = pendingInteractionResolvers.get(requestId); | ||
| let accepted = false; | ||
| if (pendingEntry?.sessionId) { | ||
| await withSendToSessionLock(pendingEntry.sessionId, async () => { | ||
| await assertSessionActiveForManualDispatch(pendingEntry.sessionId); | ||
| accepted = resolvePendingInteraction(requestId, decision as InteractionDecision); |
There was a problem hiding this comment.
当归档状态已提交、异步 runtime cleanup 尚未清掉确认卡，并且副窗口因脏文件取消关窗时，pendingInteractionResolvers.get() 只覆盖 Permission／AskUser／Plan Review；Plugin Setup、Issue、批量改名和 Ghost Grant 分别保存在后面的独立 bridge 中，因此仍会绕过 active 复核与 route lock，确认后可继续执行插件动作、提交 Issue、改名或授予文件访问。相较先前的通用交互评论，新的证据是最终修复明确只查询这一张 map，而 14129–14140 的 bridge fallback 仍直接 resolve；应先从所有 bridge 解析所属 session，再在同一锁内复核并执行。 docs/dev-rules/electron-security-and-process-boundaries.mdL38-L40
Useful? React with 👍 / 👎.
|
@yuaiccc 👋 这个 PR 还有 3 条 review conversation 没 resolve(apps/desktop/src/main/maker-ipc/register.ts / apps/desktop/src/renderer/lib/makerChatStore.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
|
@yuaiccc 👋 这个 PR 目前与 请在本地 merge 最新的 |
…dy#3262) Signed-off-by: yuaiccc <yuaiccc@aliyun.com> # Conflicts: # apps/desktop/src/main/localDb/__tests__/orcaTeamStore.test.ts # apps/desktop/src/main/localDb/client/WorkerThreadTransport.ts # apps/desktop/src/main/localDb/client/__tests__/tx.test.ts # apps/desktop/src/main/localDb/orcaTeamStore.ts # apps/desktop/src/main/localDb/worker/opHandlers/tx.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dfe3694e5
ℹ️ 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".
| SELECT id FROM orca_teams | ||
| WHERE lead_session_id = ? AND status != 'active' | ||
| )`, | ||
| ) AND session_id = ?`, |
There was a problem hiding this comment.
When disabling Orca for a lead whose previous shutdown left workers under an inactive team, this statement now expects three bindings (updated_at, lead_session_id, and session_id), but the transaction still calls finishWorkers.run(now, leadSessionId) with only two. Both transaction implementations therefore reject this recovery path before archiving or marking the stranded workers done; bind each candidate session ID while updating, or remove the added predicate.
Useful? React with 👍 / 👎.
|
@yuaiccc 👋 这个 PR 还有 4 条 review conversation 没 resolve(apps/desktop/src/main/maker-ipc/register.ts / apps/desktop/src/renderer/lib/makerChatStore.ts / apps/desktop/src/main/localDb/worker/opHandlers/tx.ts),auto-review 因此暂时跳过、没法继续审查 / 合并。 如果你已经按评论改完或回应了,请到对应 thread 上点 Resolve conversation;全部 resolve 后,下一轮 auto-review 会自动重新审查这个 PR。 |
…dy#3262) - Re-trigger CI: previous run failed at pnpm install (transient ENOENT on spdx-expression-parse in a fresh runner); 5 sibling PRs sharing the same lockfile and merge base all pass on the same machine image, so the lockfile is sound. Same pattern as the prior 'Windows unit tests (1/2) flake' that we already documented on PR makecindy#3232. Signed-off-by: yuaiccc <yuaiccc@aliyun.com>
8dfe369 to
cd15907
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd15907037
ℹ️ 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".
| const sessionIds = expectArray(payload.sessionIds, 'sessionIds').map((value, index) => | ||
| expectString(value, 'sessionIds[' + index + ']'), | ||
| ); | ||
| ); |
There was a problem hiding this comment.
When useInlineWorker is enabled or XDT_DB_WORKER_INLINE=true, WORKER_CODE is evaluated by new Worker(..., { eval: true }); this added standalone ); follows an already-closed .map(...) call and makes the embedded worker source fail immediately with SyntaxError: Unexpected token ')', preventing the database worker from initializing. Remove this delimiter and the similarly added unmatched braces in the embedded Orca transaction functions so the supported fallback remains parseable.
Useful? React with 👍 / 👎.
…dy#3262) - Re-trigger CI: previous two runs both failed at pnpm install (ERR_PNPM_ENOENT on spdx-expression-parse in fresh runner). The same lockfile now passes on 5 sibling PRs that ran after the initial failure, so the cache is warm. Same pattern as the 'Windows unit tests (1/2) flake' we already documented on PR makecindy#3232. Signed-off-by: yuaiccc <yuaiccc@aliyun.com>
cd15907 to
58c4b48
Compare
…dy#3262) - Re-trigger CI: previous two runs both failed at pnpm install (ERR_PNPM_ENOENT on spdx-expression-parse in fresh runner). The same lockfile now passes on 5 sibling PRs that ran after the initial failure, so the cache is warm. Same pattern as the 'Windows unit tests (1/2) flake' we already documented on PR makecindy#3232. Signed-off-by: yuaiccc <yuaiccc@aliyun.com>
f488c98 to
36f680d
Compare
![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: 36f680d353
ℹ️ 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".
| const clearSessionRequest = | ||
| opts && !remoteDeviceId | ||
| ? clearOperation.api.input.clearSession(sessionId, clearedAt, opts) | ||
| : clearOperation.api.input.clearSession(sessionId, clearedAt); |
There was a problem hiding this comment.
When a secondary window starts /clear while the task is active and another window archives it before this invoke reaches Main, the new fenced clearSession request rejects, but clearSessionAfterGuardImpl treats that rejection like an ordinary guard failure and continues through closeSession, clears the renderer transcript, and calls sessionService.update(..., { sdkSessionId: null, clearedAt }). Thus the archive fence prevents the coordinator clear but still destroys the archived task's visible context and runtime state. Detect the SESSION_NOT_ACTIVE rejection here and return before applying the local clear/persistence path.
Useful? React with 👍 / 👎.
When the previous agent resolved the orcaTeamStore/orcaArchiveWorkersByTeam
conflict in opHandlers/tx.ts, three stray tokens were left behind:
- line 1577: ');' closing the dropped PR-side 'expectString(id, ...)' arg
- line 1594/1638: extra '}' after the for-loop body inside the
db.transaction(() => {...})() callback
- line 1609: ');' mirroring the line 1577 pattern
- line 1657: extra '}' after the 'if (!activeTeam) throw' guard in
orcaUpsertWorker
The TypeScript errors did not surface during local conflict resolution
because the previous agent's review only ran lint + minimal tests;
'pnpm --filter desktop typecheck' was skipped (no node_modules in any
of the 6 worktrees), and the resulting syntax errors then cascaded
into 'Typecheck desktop' failures on CI for PR makecindy#3262 even before pnpm
install completed on the first attempt.
Strip the stray tokens to restore the function bodies to the structure
adopted from origin/main (which itself uses the helper-function pattern
in orcaTeamStore.ts), so the entire orca.archive* transaction set type-
checks cleanly.
Signed-off-by: yuaiccc <yuaiccc@aliyun.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>
| const dispatch = this.fireTurn(sessionId, { | ||
| throwOnUnpersistedRestoreFailure: true, | ||
| ...(opts?.sessionRouteLockHeld ? { sessionRouteLockHeld: true } : {}), | ||
| }); |
There was a problem hiding this comment.
当副窗口通过 GOAL_GET_STATUS 恢复 active、dormant Goal，并与 SET_MODEL 或其他路由修改并发时，waitForDispatch: false 让 resumeOnOpen 在 fireTurn 完成前返回并释放外层 route lock，而 sessionRouteLockHeld: true 又使异步派发跳过自行加锁，导致路由修改插入 ensureSession 与 Session.send 之间，最终将 Goal continuation 派发到旧的或与界面状态不匹配的会话路由。
Context Used: 使用和PR描述相同的语言进行评论 (source)
Knowledge Base Used: Agent session lifecycle
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main/goal-host/controller.ts
Line: 1513
Comment:
**异步派发提前失去路由锁**
当副窗口通过 `GOAL_GET_STATUS` 恢复 active、dormant Goal，并与 `SET_MODEL` 或其他路由修改并发时，`waitForDispatch: false` 让 `resumeOnOpen` 在 `fireTurn` 完成前返回并释放外层 route lock，而 `sessionRouteLockHeld: true` 又使异步派发跳过自行加锁，导致路由修改插入 `ensureSession` 与 `Session.send` 之间，最终将 Goal continuation 派发到旧的或与界面状态不匹配的会话路由。
**Context Used:** 使用和PR描述相同的语言进行评论 ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
**Knowledge Base Used:** [Agent session lifecycle](https://app.greptile.com/xindong/-/custom-context/knowledge-base/makecindy/cindy/-/docs/agent-session-lifecycle.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
@yuaiccc 👋 这个 PR 还有 7 条 review conversation 没 resolve(apps/desktop/src/main/maker-ipc/register.ts / apps/desktop/src/renderer/lib/makerChatStore.ts / apps/desktop/src/main/localDb/worker/opHandlers/tx.ts / apps/desktop/src/main/localDb/client/WorkerThreadTransport.ts / apps/desktop/src/main/goal-host/controller.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）