Skip to content

fix(desktop): 既有会话发送不再报告已解析会话 (onSessionResolved 契约) #4630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
M4n5ter merged 8 commits into from
Sep 3, 2026
Merged

fix(desktop): 既有会话发送不再报告已解析会话 (onSessionResolved 契约) #4630

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0354a10
fix(desktop): 既有会话发送不再报告已解析会话
somewan820 Sep 3, 2026
50cbf27
chore(desktop): 同步 renderer architecture ledger
somewan820 Sep 3, 2026
47c2dbd
ci: 重触发检查（transcript-scroll e2e 疑似 flaky，与本次改动无交集）
somewan820 Sep 3, 2026
efcfea0
ci: 再触发检查（重试 e2e）
somewan820 Sep 3, 2026
c7f8fae
fix(desktop): 仅在首条消息投射时报告已解析会话
somewan820 Sep 3, 2026
bc3c38a
chore(desktop): 同步 renderer architecture ledger
somewan820 Sep 3, 2026
325aacd
fix(desktop): 仅在首条消息投射时报告已解析会话
somewan820 Sep 3, 2026
d1cfe83
refactor(desktop): 收敛 send() 重复并移除死分支
somewan820 Sep 3, 2026
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
2 changes: 1 addition & 1 deletion apps/desktop/renderer-architecture.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@
"@maka/ui": 1
},
"importSpecifiers": 39,
"nonTriviaTokens": 4278
"nonTriviaTokens": 4275
},
"src/renderer/app-shell-chrome-actions.tsx": {
"importDeclarations": 5,
Expand Down
35 changes: 35 additions & 0 deletions apps/desktop/src/main/__tests__/app-shell-first-send-cleanup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,41 @@ describe('composer first-send cleanup', () => {
assert.deepEqual(removed, []);
});

it('does not report a resolved Session from an existing-Session send', async () => {
// `onSessionResolved` is the contract for a Session this send created and
// whose first message projected (the new-Session branch). An existing-
// Session send must never fire it, or a consumer binding follow-up state
// to a newly resolved Session (e.g. a Work Board start claim) would bind
// it to an unrelated pre-existing conversation.
let resolved = 0;
const restoreWindow = installWindow({
sessions: {
submitMessage: async () => ({
ok: true,
attachments: [],
skillInvocation: { loaded: [], failed: [] },
}),
},
});

try {
const actions = createAppShellChatActions({
...createActionsDeps(),
activeIdRef: { current: 'existing-session' },
});
const result = await actions.send('hello', undefined, {
onSessionResolved: () => {
resolved += 1;
},
});
assert.equal(result, true);
} finally {
restoreWindow();
}

assert.equal(resolved, 0);
});

it('returns a sparse existing session to latest before sending', async () => {
const latest = deferred<void>();
const order: string[] = [];
Expand Down
6 changes: 5 additions & 1 deletion apps/desktop/src/renderer/app-shell-chat-actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,11 @@ export function createAppShellChatActions(deps: {
});
if (submitted.kind === 'refused') return false;
if (submitted.kind === 'unreconciled') return true;
options.onSessionResolved?.(sessionId);
// `onSessionResolved` is the contract for a Session this send CREATED
// 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.

@M4n5ter M4n5ter Sep 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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，但回调不触发。

@somewan820 somewan820 Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 if so 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/submitAndProject block shared by the new-session and existing-session branches into a submitIntoSession helper, and removed the now-dead if (submitted.kind === 'unreconciled') return true; branch in the existing-session path (it existed to skip the deleted callback call). nonTriviaTokens went 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 块提取为 submitIntoSession helper，并删除了既有会话路径中已失效的 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。

return true;
} catch (error) {
// Capture ownership before cleanup clears the optimistic Session. A
Expand Down