Skip to content

Commit 5e9c24c

Browse files
doudouOUCqwencoder
andauthored
fix(channels): Make same-chat delivery session-aware (#10145)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
1 parent de7e9b5 commit 5e9c24c

13 files changed

Lines changed: 810 additions & 119 deletions
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Channel Session-Aware Delivery
2+
3+
## Problem
4+
5+
Channel sessions are already independent, but a few adapters keep delivery-only
6+
state at chat or process scope. When two sessions run in the same chat, a later
7+
message can replace the reply target of an earlier QQ response, one completed
8+
Weixin session can clear another session's typing indicator, and the plugin
9+
example can associate overlapping responses with the wrong inbound message.
10+
11+
## Scope
12+
13+
This change makes existing same-chat delivery state session-aware. It does not
14+
add named sessions, task selection, session persistence, result labels, or
15+
worktree creation. It does not change `ChannelBase` or the public channel
16+
adapter interface.
17+
18+
`ChannelOutputSegmentContext` already carries the originating session, run,
19+
target, and message ID. `ChannelBase` also retains the active prompt's message
20+
ID until response delivery completes. Adapters can therefore preserve the
21+
origin without introducing another shared delivery abstraction.
22+
23+
## QQ
24+
25+
QQ passive replies require the original inbound `msg_id`, and streaming blocks
26+
for that message share a `msg_seq` counter. The adapter keeps a bounded
27+
in-memory context for every recently accepted message while retaining the
28+
persisted per-chat latest message as a compatibility pointer.
29+
30+
Prompt output uses the message ID from the active prompt or output segment.
31+
Streaming state copies that reply context so delayed flushes and retries do not
32+
depend on mutable chat state. Replies produced while handling an inbound
33+
command use async-local inbound context. Background and cron delivery is
34+
explicitly active and never borrows the latest inbound message.
35+
36+
An expired or missing explicit context falls back to active delivery, never to
37+
a different message in the same chat. Sequence counters remain isolated by
38+
message ID and are reclaimed with the message context's existing five-minute
39+
TTL. The persisted schema is unchanged; restore keeps sequence counters only
40+
for restored valid reply contexts.
41+
42+
## Weixin
43+
44+
Weixin typing ownership is tracked as `chatId -> sessionId -> startedAt`.
45+
Starting the first session enables typing and starts one chat-level keepalive.
46+
Additional sessions only add owners. A terminal event removes its own owner,
47+
and only the last owner disables typing.
48+
49+
The existing generation guard continues to reject stale asynchronous typing
50+
results. The keepalive backstop expires individual sessions rather than the
51+
whole chat, so an old wedged session cannot clear a newer session's indicator.
52+
Session death removes only that session; disconnect clears all state.
53+
54+
## Plugin example
55+
56+
The example replaces its process-global pending message ID with async-local
57+
inbound context for command replies and uses output-segment or active-prompt
58+
message IDs for agent output. This demonstrates the same correlation contract
59+
to third-party adapter authors without changing the protocol.
60+
61+
## Failure semantics
62+
63+
- A missing or expired QQ passive context uses active delivery and remains
64+
subject to the existing active-message policy.
65+
- A failed QQ passive attempt rolls back only its original message's sequence
66+
before the existing active fallback.
67+
- A failed Weixin typing request does not discard live owners; the bounded
68+
keepalive retries while at least one owner remains.
69+
- Cleanup on session death, group removal, and disconnect cannot remove state
70+
owned by another live session.
71+
72+
## Verification
73+
74+
Focused tests cover two sessions in one chat, delayed streaming flushes,
75+
independent QQ sequence rollback, expired reply contexts, background delivery,
76+
Weixin first-owner/last-owner transitions, stale async typing results,
77+
per-session backstop expiry, and overlapping plugin messages. Package tests are
78+
followed by the repository build, typecheck, and lint checks.

packages/channels/plugin-example/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
},
2222
"scripts": {
2323
"build": "tsc --build",
24+
"test": "vitest run",
2425
"prepublishOnly": "npm run build"
2526
},
2627
"dependencies": {
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
import { EventEmitter } from 'node:events';
2+
import { beforeEach, describe, expect, it, vi } from 'vitest';
3+
import type { Envelope } from '@qwen-code/channel-base';
4+
5+
const baseHandleInbound = vi.hoisted(() => vi.fn());
6+
7+
vi.mock('@qwen-code/channel-base', () => ({
8+
ChannelBase: class {
9+
protected name: string;
10+
11+
constructor(name: string) {
12+
this.name = name;
13+
}
14+
15+
protected handleInbound(envelope: Envelope): Promise<void> {
16+
return baseHandleInbound.call(this, envelope) as Promise<void>;
17+
}
18+
19+
protected getResponseMessageId(): string | undefined {
20+
return undefined;
21+
}
22+
},
23+
}));
24+
25+
vi.mock('ws', () => ({
26+
default: class MockWebSocket {
27+
static OPEN = 1;
28+
},
29+
}));
30+
31+
import { MockPluginChannel } from './MockPluginChannel.js';
32+
33+
function deferredPromise() {
34+
let resolve!: () => void;
35+
const promise = new Promise<void>((res) => {
36+
resolve = res;
37+
});
38+
return { promise, resolve };
39+
}
40+
41+
function envelope(messageId: string): Envelope {
42+
return {
43+
channelName: 'mock',
44+
senderId: `sender-${messageId}`,
45+
senderName: messageId,
46+
chatId: 'shared-chat',
47+
text: messageId,
48+
messageId,
49+
isGroup: false,
50+
isMentioned: false,
51+
isReplyToBot: false,
52+
};
53+
}
54+
55+
function createChannel() {
56+
const channel = new MockPluginChannel(
57+
'mock',
58+
{
59+
type: 'mock',
60+
token: 'test-token',
61+
serverWsUrl: 'ws://example.test',
62+
senderPolicy: 'open',
63+
allowedUsers: [],
64+
sessionScope: 'user',
65+
cwd: process.cwd(),
66+
groupPolicy: 'disabled',
67+
dmPolicy: 'open',
68+
groups: {},
69+
},
70+
new EventEmitter() as never,
71+
);
72+
const send = vi.fn();
73+
(channel as unknown as { ws: { readyState: number; send: typeof send } }).ws =
74+
{ readyState: 1, send };
75+
return { channel, send };
76+
}
77+
78+
describe('MockPluginChannel message correlation', () => {
79+
beforeEach(() => {
80+
baseHandleInbound.mockReset();
81+
});
82+
83+
it('keeps overlapping inbound command replies bound to their own messages', async () => {
84+
const { channel, send } = createChannel();
85+
const first = deferredPromise();
86+
const second = deferredPromise();
87+
baseHandleInbound.mockImplementation(async function (
88+
this: MockPluginChannel,
89+
inbound: Envelope,
90+
) {
91+
await (inbound.messageId === 'msg-a' ? first.promise : second.promise);
92+
await this.sendMessage(inbound.chatId, `reply-${inbound.messageId}`);
93+
});
94+
95+
const pendingA = channel.handleInbound(envelope('msg-a'));
96+
const pendingB = channel.handleInbound(envelope('msg-b'));
97+
second.resolve();
98+
await pendingB;
99+
first.resolve();
100+
await pendingA;
101+
102+
expect(send.mock.calls.map(([frame]) => JSON.parse(String(frame)))).toEqual(
103+
[
104+
{
105+
type: 'outbound',
106+
messageId: 'msg-b',
107+
chatId: 'shared-chat',
108+
text: 'reply-msg-b',
109+
},
110+
{
111+
type: 'outbound',
112+
messageId: 'msg-a',
113+
chatId: 'shared-chat',
114+
text: 'reply-msg-a',
115+
},
116+
],
117+
);
118+
});
119+
120+
it('uses output segment message IDs for chunks and final responses', async () => {
121+
const { channel, send } = createChannel();
122+
const output = channel as unknown as {
123+
onResponseChunk: (
124+
chatId: string,
125+
chunk: string,
126+
sessionId: string,
127+
segment: { messageId: string },
128+
) => void;
129+
onResponseComplete: (
130+
chatId: string,
131+
text: string,
132+
sessionId: string,
133+
segment: { messageId: string },
134+
) => Promise<void>;
135+
};
136+
137+
output.onResponseChunk('shared-chat', 'chunk-a', 'session-a', {
138+
messageId: 'msg-a',
139+
});
140+
await output.onResponseComplete('shared-chat', 'final-b', 'session-b', {
141+
messageId: 'msg-b',
142+
});
143+
144+
expect(send.mock.calls.map(([frame]) => JSON.parse(String(frame)))).toEqual(
145+
[
146+
{
147+
type: 'chunk',
148+
messageId: 'msg-a',
149+
chatId: 'shared-chat',
150+
text: 'chunk-a',
151+
},
152+
{
153+
type: 'outbound',
154+
messageId: 'msg-b',
155+
chatId: 'shared-chat',
156+
text: 'final-b',
157+
},
158+
],
159+
);
160+
});
161+
});

packages/channels/plugin-example/src/MockPluginChannel.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@ import type {
44
ChannelBaseOptions,
55
Envelope,
66
ChannelAgentBridge,
7+
ChannelOutputSegmentContext,
78
} from '@qwen-code/channel-base';
89
import WebSocket from 'ws';
10+
import { AsyncLocalStorage } from 'node:async_hooks';
911
import type {
1012
InboundMessage,
1113
OutboundMessage,
@@ -19,7 +21,7 @@ export interface MockPluginConfig extends ChannelConfig {
1921
export class MockPluginChannel extends ChannelBase {
2022
private ws: WebSocket | null = null;
2123
private serverWsUrl: string;
22-
private pendingMessageId: string | undefined;
24+
private inboundMessage = new AsyncLocalStorage<{ messageId?: string }>();
2325

2426
constructor(
2527
name: string,
@@ -83,13 +85,15 @@ export class MockPluginChannel extends ChannelBase {
8385
protected override onResponseChunk(
8486
chatId: string,
8587
chunk: string,
86-
_sessionId: string,
88+
sessionId: string,
89+
segment?: ChannelOutputSegmentContext,
8790
): void {
8891
if (!this.ws || this.ws.readyState !== WebSocket.OPEN) return;
8992

9093
const msg: ChunkMessage = {
9194
type: 'chunk',
92-
messageId: this.pendingMessageId || 'unknown',
95+
messageId:
96+
segment?.messageId ?? this.getResponseMessageId(sessionId) ?? 'unknown',
9397
chatId,
9498
text: chunk,
9599
};
@@ -99,19 +103,28 @@ export class MockPluginChannel extends ChannelBase {
99103
protected override async onResponseComplete(
100104
chatId: string,
101105
fullText: string,
102-
_sessionId: string,
106+
sessionId: string,
107+
segment?: ChannelOutputSegmentContext,
103108
): Promise<void> {
104-
await this.sendMessage(chatId, fullText);
109+
this.sendOutbound(
110+
chatId,
111+
fullText,
112+
segment?.messageId ?? this.getResponseMessageId(sessionId),
113+
);
105114
}
106115

107116
async sendMessage(chatId: string, text: string): Promise<void> {
117+
this.sendOutbound(chatId, text, this.inboundMessage.getStore()?.messageId);
118+
}
119+
120+
private sendOutbound(chatId: string, text: string, messageId?: string): void {
108121
if (!this.ws || this.ws.readyState !== WebSocket.OPEN) {
109122
return;
110123
}
111124

112125
const outbound: OutboundMessage = {
113126
type: 'outbound',
114-
messageId: this.pendingMessageId || 'unknown',
127+
messageId: messageId ?? 'unknown',
115128
chatId,
116129
text,
117130
};
@@ -127,11 +140,8 @@ export class MockPluginChannel extends ChannelBase {
127140
}
128141

129142
override async handleInbound(envelope: Envelope): Promise<void> {
130-
this.pendingMessageId = envelope.messageId;
131-
try {
132-
await super.handleInbound(envelope);
133-
} finally {
134-
this.pendingMessageId = undefined;
135-
}
143+
await this.inboundMessage.run({ messageId: envelope.messageId }, async () =>
144+
super.handleInbound(envelope),
145+
);
136146
}
137147
}

packages/channels/plugin-example/tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
"rootDir": "src"
66
},
77
"include": ["src/**/*.ts"],
8-
"exclude": ["node_modules", "dist"],
8+
"exclude": ["node_modules", "dist", "src/**/*.test.ts"],
99
"references": [{ "path": "../base" }]
1010
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import path from 'node:path';
2+
import { defineConfig } from 'vitest/config';
3+
4+
export default defineConfig({
5+
test: {
6+
include: ['src/**/*.test.ts'],
7+
globals: true,
8+
},
9+
resolve: {
10+
alias: {
11+
'@qwen-code/channel-base': path.resolve(
12+
__dirname,
13+
'../base/src/index.ts',
14+
),
15+
},
16+
},
17+
});

0 commit comments

Comments
 (0)