Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion packages/channels/dingtalk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
"dist"
],
"scripts": {
"build": "tsc --build"
"build": "tsc --build",
"test": "vitest run",
"test:ci": "vitest run"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] "test:ci": "vitest run" is identical to "test" on the line above. No other channel package (qqbot, weixin, feishu, telegram) has a separate test:ci script. Unless a CI pipeline specifically invokes test:ci and expects it to differ from test, this adds no value and creates confusion about which to use.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

},
"dependencies": {
"@qwen-code/channel-base": "file:../base",
Expand Down
111 changes: 111 additions & 0 deletions packages/channels/dingtalk/src/DingtalkAdapter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { describe, expect, it, vi } from 'vitest';

vi.mock('dingtalk-stream-sdk-nodejs', () => ({
DWClient: class {
disconnect = vi.fn();
getConfig = vi.fn(() => ({ access_token: 'token' }));
registerCallbackListener = vi.fn();
send = vi.fn();
connect = vi.fn();
},
TOPIC_ROBOT: 'robot',
EventAck: { SUCCESS: 'success' },
}));

vi.mock('@qwen-code/channel-base', () => ({
ChannelBase: class {
protected config: Record<string, unknown>;
protected name: string;

constructor(
name: string,
config: Record<string, unknown>,
_bridge: unknown,
) {
this.name = name;
this.config = config;
}
},
}));

const { DingtalkChannel } = await import('./DingtalkAdapter.js');

function createChannel(): DingtalkChannel {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] TypeScript errors in test file: DingtalkChannel is a value-import via await import() but used as a type annotation on lines 33 and 52 (TS2749). The config object on line 36 is also not assignable to ChannelConfig (TS2345). These don't block the build (tsconfig excludes src/**/*.test.ts), but they will show as red squiggles in editors.

Suggested change
function createChannel(): DingtalkChannel {
import type { DingtalkChannel } from './DingtalkAdapter.js';

— DeepSeek/deepseek-v4-pro via Qwen Code /review

return new DingtalkChannel(
'test-dingtalk',
{
type: 'dingtalk',
clientId: 'client-id',
clientSecret: 'client-secret',
senderPolicy: 'open',
allowedUsers: [],
sessionScope: 'user',
cwd: '/tmp',
groupPolicy: 'open',
groups: {},
},
{} as never,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] {} as never is semantically misleading — never is TypeScript's uninhabited bottom type asserting the value can never exist, yet {} is a real runtime object. The rest of the test file uses the as unknown as Target double-cast pattern for escaping private access. Consider using that pattern here for consistency:

Suggested change
{} as never,
{} as unknown as AcpBridge,

— DeepSeek/deepseek-v4-pro via Qwen Code /review

);
}

function getPromptHook(
channel: DingtalkChannel,
hook: 'onPromptStart' | 'onPromptEnd',
): (chatId: string, sessionId: string, messageId?: string) => void {
const fn = (channel as unknown as Record<string, unknown>)[hook] as (
chatId: string,
sessionId: string,
messageId?: string,
) => void;
return fn.bind(channel);
}

describe('DingtalkChannel prompt reactions', () => {
it('skips uppercase webhook URLs when starting a prompt', () => {
const channel = createChannel();
const attachReaction = vi.fn().mockResolvedValue(undefined);
(
channel as unknown as { attachReaction: typeof attachReaction }
).attachReaction = attachReaction;

getPromptHook(channel, 'onPromptStart')(
'HTTPS://oapi.dingtalk.com/robot/send?access_token=token',
'session-1',
'message-1',
);

expect(attachReaction).not.toHaveBeenCalled();
});

it('still attaches reactions for conversation IDs', () => {
const channel = createChannel();
const attachReaction = vi.fn().mockResolvedValue(undefined);
(
channel as unknown as { attachReaction: typeof attachReaction }
).attachReaction = attachReaction;

getPromptHook(channel, 'onPromptStart')(
'cid-123',
'session-1',
'message-1',
);

expect(attachReaction).toHaveBeenCalledWith('message-1', 'cid-123');
});

it('skips uppercase webhook URLs when ending a prompt', () => {
const channel = createChannel();
const recallReaction = vi.fn().mockResolvedValue(undefined);
(
channel as unknown as { recallReaction: typeof recallReaction }
).recallReaction = recallReaction;

getPromptHook(channel, 'onPromptEnd')(
'HTTPS://oapi.dingtalk.com/robot/send?access_token=token',
'session-1',
'message-1',
);

expect(recallReaction).not.toHaveBeenCalled();
});
});
2 changes: 1 addition & 1 deletion packages/channels/dingtalk/src/DingtalkAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ export class DingtalkChannel extends ChannelBase {
* conversation ID — skip the webhook-URL fallback case.
*/
private isConversationId(chatId: string): boolean {
return !!chatId && !chatId.startsWith('http');
return !!chatId && !/^https?:\/\//i.test(chatId);
}

protected override onPromptStart(
Expand Down
Loading