fix(desktop): 既有会话发送不再报告已解析会话 (onSessionResolved 契约) - #4630
Conversation
send() 的既有会话分支无条件调用 options.onSessionResolved(sessionId), 但该回调的契约是「本次发送新建且首条消息成功投射的会话」(新会话 分支),既有会话发送不应触发。当前 main 上该回调尚无消费者,属于 死代码;但 apache#4598 接入 start-task 后它会把 Work Board 条目误链接到 无关的既有会话。此处先修掉根本问题,避免后续消费方被错误绑定。 新增回归测试:既有会话 projected 发送不触发 onSessionResolved。
app-shell-chat-actions.ts 的 nonTriviaTokens 从 4278 降至 4275 (删除既有会话分支对 onSessionResolved 的无条件调用),同步 renderer-architecture.json 的 ratchet 基线。
| // and whose first message projected (the new-Session branch above). An | ||
| // existing-Session send must never report it, or a consumer that binds | ||
| // follow-up state to a newly resolved Session would bind it to an | ||
| // unrelated pre-existing conversation. |
There was a problem hiding this comment.
English
Could we also gate this callback in the new-session branch on submitted.kind === 'projected'?
outcome_unknown is mapped to unreconciled, but this branch only returns early for refused, so it still calls onSessionResolved. That contradicts the contract documented here and could link a Work Board item to a Session whose first message was never confirmed as projected. Since send() intentionally returns true for unreconciled, the caller cannot correct this afterward.
unsentSessionId = undefined;
if (submitted.kind === 'projected') {
options.onSessionResolved?.(session.id);
}Please also cover new Session + outcome_unknown with a regression test asserting that send() returns true while the callback is not invoked.
中文
这里是否也应该把新会话分支的回调限制为仅在 submitted.kind === 'projected' 时触发?
outcome_unknown 会被映射为 unreconciled,但当前分支只对 refused 提前返回,因此仍会调用 onSessionResolved。这与此处声明的契约不一致,并可能把 Work Board 条目链接到首条消息尚未确认成功投射的 Session。由于 send() 对 unreconciled 会有意返回 true,调用方之后也无法补救。
unsentSessionId = undefined;
if (submitted.kind === 'projected') {
options.onSessionResolved?.(session.id);
}建议同时补充 新 Session + outcome_unknown 的回归测试,断言 send() 返回 true,但回调不触发。
There was a problem hiding this comment.
English
Good catch — done. onSessionResolved in the new-session branch now fires only when submitted.kind === 'projected' (head d1cfe8349).
Follow-ups since the first fix (c7f8fae78):
- The gate was re-expressed as a single-line
ifso the file stays at or below the base token count (the renderer architecture debt ratchet only allows legacy AppShell files to shrink); the contract comments were re-added once the follow-up refactor freed budget. - A follow-up refactor extracted the duplicated ~30-line
sendCommand/submitAndProjectblock shared by the new-session and existing-session branches into asubmitIntoSessionhelper, and removed the now-deadif (submitted.kind === 'unreconciled') return true;branch in the existing-session path (it existed to skip the deleted callback call).nonTriviaTokenswent 4278 → 4089.
Verification: renderer architecture check passes against the CI base 726fb809 · desktop main 2005/2005 · CI test green.
This comment and the code changes were AI-assisted (pi); per CONTRIBUTING the affected commits carry a Generated-by: pi trailer.
中文
好建议,已完成。新会话分支的 onSessionResolved 现在只在 submitted.kind === 'projected' 时触发(head d1cfe8349)。
初版修复(c7f8fae78)之后的后续调整:
- 门控改写为单行
if,使文件 token 保持不高于 base(renderer architecture 单调 debt ratchet 只允许 legacy AppShell 文件缩减);后续重构释放预算后,契约注释已补回。 - 一次重构把新会话/既有会话两分支重复的 ~30 行
sendCommand/submitAndProject块提取为submitIntoSessionhelper,并删除了既有会话路径中已失效的if (submitted.kind === 'unreconciled') return true;死分支(该行原本用于跳过已删除的回调调用)。nonTriviaTokens从 4278 降至 4089。
验证:renderer architecture 检查以 CI 同款 base 726fb809 通过 · desktop main 全量 2005/2005 · CI test 绿。
本评论与代码修改由 AI(pi)辅助生成,已按 CONTRIBUTING 在受影响 commit 附加 Generated-by: pi trailer。
按 PR 审查(M4n5ter review 5099565705)收窄 onSessionResolved 契约:新会话 分支的回调不再对 outcome_unknown(unreconciled)触发——发送结果未知时首条 消息未被确认投射,send() 虽返回 true,回调也不得上报,否则消费方会把 Work Board 条目链接到首条消息从未确认的会话。 新增回归测试:新会话 + outcome_unknown 时 send() 返回 true、会话保留、 onSessionResolved 不触发。desktop main 全量 2005/2005 通过。 Generated-by: pi
EnglishAddressed review 5099565705 and pushed
Verification: desktop main 2005/2005 pass · This comment and the code changes were AI-assisted (pi); per CONTRIBUTING the affected commit carries a 中文已按 review 5099565705 的建议修改并推送(
验证:desktop main 全量 2005/2005 通过 · 本评论与代码修改由 AI(pi)辅助生成,已按 CONTRIBUTING 在受影响 commit 附加 |
app-shell-chat-actions.ts 新增 onSessionResolved 契约注释与 projected 分支后 nonTriviaTokens 4275 -> 4292,同步台账。 Generated-by: pi
按 review 5099565705 收窄 onSessionResolved 契约:new-session 分支的回调 只在 submitted.kind === 'projected' 时触发,outcome_unknown(unreconciled) 不再上报——send() 对该结果有意返回 true,但首条消息未被确认投射时回调 也不得上报,否则消费方会把 Work Board 条目链接到首条消息从未确认的会话。 既有会话分支行为不变(永不触发)。 renderer architecture 单调 debt ratchet 要求 legacy AppShell 文件 nonTriviaTokens 只减不增,故采用单行 if 的 token 中性实现(4278,与 base 持平),契约说明由回归测试注释承载;同步 renderer architecture ledger。 Generated-by: pi
- 提取 submitIntoSession(sessionId, messageId) helper,合并 new-session 与 existing-session 两条发送路径逐字重复的 ~30 行(sendCommand 构建与 submitAndProject 调用),消除两处需同步的负担 - 删除 existing-session 分支的 `if (submitted.kind === 'unreconciled') return true` 死分支:base 上该行用于跳过 onSessionResolved 调用,本 PR 删除该调用后它与末尾 return true 完全等价 - 借 helper 提取释放的 token 预算(nonTriviaTokens 4278→4089,仍低于 base),恢复被 renderer architecture ratchet 挤掉的 onSessionResolved 契约注释;同步 architecture ledger(4089 < base 4278,满足单调 ratchet) Generated-by: pi
M4n5ter
left a comment
There was a problem hiding this comment.
English
Re-reviewed at d1cfe83. The previous callback-contract finding is resolved: onSessionResolved now fires only for a newly created Session whose first message was confirmed as projected. Existing-session and unreconciled paths preserve their prior send and cleanup semantics without reporting a resolved Session. The shared submit helper removes real duplication without introducing another state owner. Correctness and design are acceptable; CI is green.
中文
已在 d1cfe83 完成复审。此前的回调契约问题已解决:onSessionResolved 现在只会在本次发送创建新 Session 且首条消息确认 projected 时触发;既有会话和 unreconciled 路径保持原有发送与清理语义,同时不会错误上报已解析 Session。公共提交 helper 消除了真实重复,没有引入新的状态 owner。Correctness 与 design 均可接受,CI 已通过。
rebase 冲突归并时误将 app-shell-chat-actions.ts 整体回退到 apache#4630 之前的版本, 重新引入了既有会话发送也会触发 onSessionResolved 的回归。恢复为上游 main 的 实现:onSessionResolved 仅在新建会话首次消息投射时触发,Work Board 链接只绑定 到已投射的新会话。
rebase 冲突归并时误将 app-shell-chat-actions.ts 整体回退到 apache#4630 之前的版本, 重新引入了既有会话发送也会触发 onSessionResolved 的回归。恢复为上游 main 的 实现:onSessionResolved 仅在新建会话首次消息投射时触发,Work Board 链接只绑定 到已投射的新会话。
问题
createAppShellChatActions.send()的既有会话分支无条件调用options.onSessionResolved?.(sessionId):initialSessionId为空时)仅在submitted.kind === 'projected'时触发——这是回调的契约:它只应报告"本次发送新建、且首条消息成功投射的会话"。initialSessionId非空)无条件触发,语义错误。当前
main上该回调尚无任何调用方(死代码),所以无行为影响。但 #4598 一旦接入 start-task(把onSessionResolved注入为workBoard.linkSession的触发点),无条件调用会把 Work Board 条目误链接到无关的既有会话。修复
移除既有会话分支对
onSessionResolved的无条件调用,并用注释明确该回调的契约。新会话分支行为不变。测试
新增回归测试:既有会话(
activeIdRef.current已设置)projected 发送不触发onSessionResolved。验证
tsc -p tsconfig.main.json0 error · biome lint clean ·git diff --checkclean说明
这是 C1(跨会话误消费)的根本修复,独立于 #4598 提交,可直接合入
main。它把onSessionResolved的语义钉死在"新建会话首条消息投射"上,使 #4598 或任何后续消费方不会被错误绑定。剩余依赖 feature 的清理(start-task claim 的取消路径与编排测试)在 #4598 上另行处理。