feat(channels): @openhermit/channel-wechat (iLink, text-only v0)#91
Merged
Conversation
First external-default channel built on the ChannelManifest contract. Talks iLink HTTP (Tencent's WeChat protocol) — QR-link wizard via ChannelSetup, long-poll inbound, text-only outbound. - Wire types and HTTP client (`src/ilink/`) - `WechatBridge` (ChannelOutbound) + `WechatBot` long-poll loop - `ChannelSetup` adapter that returns the QR url as `qrText` for the admin UI to render - Loaded via `channelPackages` in gateway config, not bundled Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a complete WeChat channel adapter for iLink integration, including long-poll messaging, QR-code login setup, manifest registration, and gateway/web UI orchestration for interactive channel linking. Extends the protocol registry to track manifest origins (built-in vs external) and enables dual-mode channel creation through both manifest keys and external namespaces. ChangesWeChat Channel Adapter Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/channels/wechat/src/bot.ts`:
- Around line 91-99: The loop currently awaits each message sequentially (await
this.opts.bridge.handleMessage(msg)), causing unrelated peers to be serialized;
change it to fire-and-forget each handleMessage call instead: inside the for
(const msg of msgs) loop (and checking this.running), call
this.opts.bridge.handleMessage(msg) without await and attach a .catch handler to
log errors (use the same logging used currently), so failures are recorded but
do not block processing of subsequent msgs or cursor advancement.
In `@apps/channels/wechat/src/bridge.ts`:
- Around line 221-301: waitForAgentResponse currently blocks indefinitely; wrap
the fetch/stream read in an AbortController with a deadline and pass
controller.signal to fetch and any async read operations in the loop so the
stream can be aborted; catch AbortError inside waitForAgentResponse and convert
it into a TurnResult error (e.g., "stream aborted/timeout") so handleMessage and
the per-peer lock (peerLocks) can recover, still ensuring the reader is
cancelled in the finally block and this.lastEventIds.set(sessionId,
nextLastEventId) is performed before returning; update references inside
waitForAgentResponse (reader.read(), fetch(...), and the try/catch around the
loop) to respect the abort signal and to treat aborts as a normal turn error
rather than leaving the promise unresolved.
- Around line 179-196: The code currently generates and memoizes a synthetic
session id after listSessions() throws, which can pin a peer to a new thread
during transient control-plane failures; change getSessionId() so that when
listSessions() fails you generate and return a fallback sessionId but do NOT
call this.peerSessions.set(peer, sessionId). Instead, memoize the sessionId only
after a successful openSession()/ensureSession() call (i.e., move the
this.peerSessions.set(peer, sessionId) into the success path of
ensureSession()/openSession()), or have ensureSession() accept the fallback id
and set peerSessions only on success.
- Around line 107-113: The handler currently only ignores messages with
message_type === MessageType.BOT, but it must also suppress messages originating
from the bridge's own user id; update handleMessage (async handleMessage(msg:
WeixinMessage)) to return early if msg.from_user_id === this.runtime.ilinkBotId
(or equivalent ilinkBotId field on WechatBridgeRuntime) in addition to the
existing BOT-type check, and ensure this check runs before computing peer so
self-echoes from non-BOT messages are dropped.
In `@apps/channels/wechat/src/ilink/api.ts`:
- Around line 67-68: The current ILINK_APP_ID fallback to '' hides
misconfiguration; replace the silent empty default by validating after computing
ILINK_APP_ID (from process.env.OPENHERMIT_WECHAT_APP_ID ?? pkg.ilink_appid) and
fail fast when it's falsy: throw a clear error or terminate startup with a
descriptive message that includes which env/key is missing (reference
ILINK_APP_ID, process.env.OPENHERMIT_WECHAT_APP_ID, and pkg.ilink_appid) so the
process refuses to start rather than sending requests with an empty
iLink-App-Id.
- Around line 96-109: The GET requests currently use buildCommonHeaders() which
omits the X-WECHAT-UIN header causing get_qrcode_status QR polling to fail; fix
by including X-WECHAT-UIN for GETs as well — either add 'X-WECHAT-UIN':
randomWechatUin() into buildCommonHeaders() or create a buildGetHeaders() that
returns {...buildCommonHeaders(), 'X-WECHAT-UIN': randomWechatUin()} and have
apiGet()/get_qrcode_status call that so GET transports include the same iLink
identity header used by buildPostHeaders().
- Around line 191-205: The catch block around the apiGet call (the block that
calls apiGet with endpoint `ilink/bot/get_qrcode_status?qrcode=...`, timeoutMs
and label 'pollQrStatus') currently converts every failure into { status: 'wait'
}; change it so only AbortError/timeouts are treated as transient and return {
status: 'wait' }, and all other errors (HTTP 4xx/5xx, network/gateway errors,
JSON parse failures) are propagated (rethrow) so the caller can surface/fail
fast; to do this, leave the apiGet call and the existing AbortError branch
intact but remove the blanket fallback return and instead rethrow err for
non-AbortError cases (or catch JSON.parse separately around JSON.parse(raw) and
rethrow parse errors) so QrStatusResponse is not faked on real failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18147cfb-5248-498e-a5a2-2d85e0682b7f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
apps/channels/wechat/README.mdapps/channels/wechat/package.jsonapps/channels/wechat/src/bot.tsapps/channels/wechat/src/bridge.tsapps/channels/wechat/src/ilink/api.tsapps/channels/wechat/src/ilink/login.tsapps/channels/wechat/src/ilink/types.tsapps/channels/wechat/src/index.tsapps/channels/wechat/src/manifest.tsapps/channels/wechat/src/setup.tsapps/channels/wechat/tsconfig.jsonapps/channels/wechat/tsconfig.typecheck.json
Comment on lines
+91
to
+99
| const msgs = resp.msgs ?? []; | ||
| for (const msg of msgs) { | ||
| if (!this.running) break; | ||
| try { | ||
| await this.opts.bridge.handleMessage(msg); | ||
| } catch (err) { | ||
| this.log(`handleMessage error: ${err instanceof Error ? err.message : String(err)}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
Don't serialize unrelated chats in the poll loop.
WechatBridge.handleMessage() already queues by peer, but this loop awaits every message one-by-one. One slow turn now delays every later message in the batch and slows cursor advancement for other peers.
Suggested fix
const msgs = resp.msgs ?? [];
- for (const msg of msgs) {
- if (!this.running) break;
- try {
- await this.opts.bridge.handleMessage(msg);
- } catch (err) {
- this.log(`handleMessage error: ${err instanceof Error ? err.message : String(err)}`);
- }
- }
+ await Promise.allSettled(
+ msgs.map(async (msg) => {
+ if (!this.running) return;
+ try {
+ await this.opts.bridge.handleMessage(msg);
+ } catch (err) {
+ this.log(`handleMessage error: ${err instanceof Error ? err.message : String(err)}`);
+ }
+ }),
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const msgs = resp.msgs ?? []; | |
| for (const msg of msgs) { | |
| if (!this.running) break; | |
| try { | |
| await this.opts.bridge.handleMessage(msg); | |
| } catch (err) { | |
| this.log(`handleMessage error: ${err instanceof Error ? err.message : String(err)}`); | |
| } | |
| } | |
| const msgs = resp.msgs ?? []; | |
| await Promise.allSettled( | |
| msgs.map(async (msg) => { | |
| if (!this.running) return; | |
| try { | |
| await this.opts.bridge.handleMessage(msg); | |
| } catch (err) { | |
| this.log(`handleMessage error: ${err instanceof Error ? err.message : String(err)}`); | |
| } | |
| }), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/channels/wechat/src/bot.ts` around lines 91 - 99, The loop currently
awaits each message sequentially (await this.opts.bridge.handleMessage(msg)),
causing unrelated peers to be serialized; change it to fire-and-forget each
handleMessage call instead: inside the for (const msg of msgs) loop (and
checking this.running), call this.opts.bridge.handleMessage(msg) without await
and attach a .catch handler to log errors (use the same logging used currently),
so failures are recorded but do not block processing of subsequent msgs or
cursor advancement.
Comment on lines
+107
to
+113
| /** Entry point for the bot loop; serializes per-peer. */ | ||
| async handleMessage(msg: WeixinMessage): Promise<void> { | ||
| // Skip our own outbound echoes. | ||
| if (msg.message_type === MessageType.BOT) return; | ||
|
|
||
| const peer = msg.group_id?.trim() || msg.from_user_id?.trim(); | ||
| if (!peer) return; |
There was a problem hiding this comment.
Use ilinkBotId when suppressing self-echoes.
WechatBridgeRuntime.ilinkBotId is documented as the bot's own user id, but this path never reads it. That makes the runtime field inert and leaves a self-reply loop if iLink ever echoes outbound traffic as a non-BOT message.
Suggested fix
async handleMessage(msg: WeixinMessage): Promise<void> {
+ const fromUserId = msg.from_user_id?.trim();
// Skip our own outbound echoes.
if (msg.message_type === MessageType.BOT) return;
+ if (this.runtime.ilinkBotId && fromUserId === this.runtime.ilinkBotId) return;
- const peer = msg.group_id?.trim() || msg.from_user_id?.trim();
+ const peer = msg.group_id?.trim() || fromUserId;
if (!peer) return;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/channels/wechat/src/bridge.ts` around lines 107 - 113, The handler
currently only ignores messages with message_type === MessageType.BOT, but it
must also suppress messages originating from the bridge's own user id; update
handleMessage (async handleMessage(msg: WeixinMessage)) to return early if
msg.from_user_id === this.runtime.ilinkBotId (or equivalent ilinkBotId field on
WechatBridgeRuntime) in addition to the existing BOT-type check, and ensure this
check runs before computing peer so self-echoes from non-BOT messages are
dropped.
Comment on lines
+179
to
+196
| try { | ||
| const sessions = await this.client.listSessions({ | ||
| channel: 'wechat', | ||
| metadata: { [isGroup ? 'wechat_group_id' : 'wechat_peer_id']: peer }, | ||
| limit: 1, | ||
| }); | ||
| if (sessions.length > 0) { | ||
| const sessionId = sessions[0]!.sessionId; | ||
| this.peerSessions.set(peer, sessionId); | ||
| return sessionId; | ||
| } | ||
| } catch { | ||
| // Server unavailable — fall through. | ||
| } | ||
|
|
||
| const sessionId = WechatBridge.generateSessionId(); | ||
| this.peerSessions.set(peer, sessionId); | ||
| return sessionId; |
There was a problem hiding this comment.
Don't memoize a synthetic session id after a lookup failure.
Lines 190-196 cache a fresh sessionId even when listSessions() only failed transiently. Once the control plane recovers, that peer is pinned to a new thread and loses the existing conversation history.
Only cache the fallback id after openSession() succeeds, or return it uncached from getSessionId() and memoize it in ensureSession() on success.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/channels/wechat/src/bridge.ts` around lines 179 - 196, The code
currently generates and memoizes a synthetic session id after listSessions()
throws, which can pin a peer to a new thread during transient control-plane
failures; change getSessionId() so that when listSessions() fails you generate
and return a fallback sessionId but do NOT call this.peerSessions.set(peer,
sessionId). Instead, memoize the sessionId only after a successful
openSession()/ensureSession() call (i.e., move the this.peerSessions.set(peer,
sessionId) into the success path of ensureSession()/openSession()), or have
ensureSession() accept the fallback id and set peerSessions only on success.
Comment on lines
+221
to
+301
| private async waitForAgentResponse(sessionId: string): Promise<TurnResult> { | ||
| const eventsUrl = this.client.buildEventsUrl(sessionId); | ||
| const lastEventId = this.lastEventIds.get(sessionId) ?? 0; | ||
|
|
||
| const response = await fetch(eventsUrl, { | ||
| headers: { authorization: `Bearer ${this.clientToken}` }, | ||
| }); | ||
|
|
||
| if (!response.ok || !response.body) { | ||
| return { text: undefined, error: `Failed to open event stream (${response.status})` }; | ||
| } | ||
|
|
||
| const reader = response.body.getReader(); | ||
| const decoder = new TextDecoder(); | ||
| let buffer = ''; | ||
| let nextLastEventId = lastEventId; | ||
| let sequenceResetChecked = false; | ||
| let accumulatedText = ''; | ||
| let finalText: string | undefined; | ||
| let error: string | undefined; | ||
|
|
||
| try { | ||
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
| buffer += decoder.decode(value, { stream: true }); | ||
| const parsed = parseSseFrames(buffer); | ||
| buffer = parsed.remainder; | ||
| let sawAgentEnd = false; | ||
|
|
||
| for (const frame of parsed.frames) { | ||
| if (frame.id !== undefined && frame.id <= nextLastEventId) continue; | ||
| if (frame.id !== undefined) nextLastEventId = frame.id; | ||
|
|
||
| if (frame.event === 'ready') { | ||
| if (!sequenceResetChecked) { | ||
| sequenceResetChecked = true; | ||
| try { | ||
| const data = frame.data.length > 0 | ||
| ? (JSON.parse(frame.data) as { nextEventId?: number }) | ||
| : {}; | ||
| if (typeof data.nextEventId === 'number' && data.nextEventId <= nextLastEventId) { | ||
| nextLastEventId = 0; | ||
| } | ||
| } catch { /* ignore */ } | ||
| } | ||
| continue; | ||
| } | ||
| if (frame.event === 'ping') continue; | ||
|
|
||
| const payload = | ||
| frame.data.length > 0 | ||
| ? (JSON.parse(frame.data) as Record<string, unknown>) | ||
| : {}; | ||
|
|
||
| if (frame.event === 'text_delta') { | ||
| accumulatedText += String(payload.text ?? ''); | ||
| continue; | ||
| } | ||
| if (frame.event === 'text_final') { | ||
| finalText = String(payload.text ?? '').trim(); | ||
| continue; | ||
| } | ||
| if (frame.event === 'error') { | ||
| error = String(payload.message ?? 'Unknown error'); | ||
| continue; | ||
| } | ||
| if (frame.event === 'agent_end') { | ||
| sawAgentEnd = true; | ||
| continue; | ||
| } | ||
| } | ||
| if (sawAgentEnd) break; | ||
| } | ||
| } finally { | ||
| await reader.cancel().catch(() => undefined); | ||
| } | ||
|
|
||
| this.lastEventIds.set(sessionId, nextLastEventId); | ||
| const text = finalText ?? (accumulatedText.trim() || undefined); | ||
| return { text, error }; |
There was a problem hiding this comment.
Bound the SSE wait under the per-peer lock.
waitForAgentResponse() has no timeout or abort path. If the stream stalls before agent_end, handleMessage() never resolves and every later message for that peer stays blocked behind peerLocks.
Use an AbortController deadline around fetch()/reader.read() and convert AbortError into a turn error so the queue can recover.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/channels/wechat/src/bridge.ts` around lines 221 - 301,
waitForAgentResponse currently blocks indefinitely; wrap the fetch/stream read
in an AbortController with a deadline and pass controller.signal to fetch and
any async read operations in the loop so the stream can be aborted; catch
AbortError inside waitForAgentResponse and convert it into a TurnResult error
(e.g., "stream aborted/timeout") so handleMessage and the per-peer lock
(peerLocks) can recover, still ensuring the reader is cancelled in the finally
block and this.lastEventIds.set(sessionId, nextLastEventId) is performed before
returning; update references inside waitForAgentResponse (reader.read(),
fetch(...), and the try/catch around the loop) to respect the abort signal and
to treat aborts as a normal turn error rather than leaving the promise
unresolved.
Comment on lines
+67
to
+68
| const ILINK_APP_ID = process.env.OPENHERMIT_WECHAT_APP_ID ?? pkg.ilink_appid ?? ''; | ||
|
|
There was a problem hiding this comment.
Fail fast when ilink_appid is missing.
Falling back to '' sends every request with an empty iLink-App-Id, so a packaging/config mistake becomes opaque upstream failures instead of a clear startup error.
💡 Proposed fix
-const ILINK_APP_ID = process.env.OPENHERMIT_WECHAT_APP_ID ?? pkg.ilink_appid ?? '';
+const ILINK_APP_ID = (process.env.OPENHERMIT_WECHAT_APP_ID ?? pkg.ilink_appid ?? '').trim();
+
+if (!ILINK_APP_ID) {
+ throw new Error(
+ 'Missing iLink app id. Set OPENHERMIT_WECHAT_APP_ID or define ilink_appid in this package.json.',
+ );
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const ILINK_APP_ID = process.env.OPENHERMIT_WECHAT_APP_ID ?? pkg.ilink_appid ?? ''; | |
| const ILINK_APP_ID = (process.env.OPENHERMIT_WECHAT_APP_ID ?? pkg.ilink_appid ?? '').trim(); | |
| if (!ILINK_APP_ID) { | |
| throw new Error( | |
| 'Missing iLink app id. Set OPENHERMIT_WECHAT_APP_ID or define ilink_appid in this package.json.', | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/channels/wechat/src/ilink/api.ts` around lines 67 - 68, The current
ILINK_APP_ID fallback to '' hides misconfiguration; replace the silent empty
default by validating after computing ILINK_APP_ID (from
process.env.OPENHERMIT_WECHAT_APP_ID ?? pkg.ilink_appid) and fail fast when it's
falsy: throw a clear error or terminate startup with a descriptive message that
includes which env/key is missing (reference ILINK_APP_ID,
process.env.OPENHERMIT_WECHAT_APP_ID, and pkg.ilink_appid) so the process
refuses to start rather than sending requests with an empty iLink-App-Id.
Comment on lines
+96
to
+109
| const buildCommonHeaders = (): Record<string, string> => ({ | ||
| 'iLink-App-Id': ILINK_APP_ID, | ||
| 'iLink-App-ClientVersion': String(ILINK_APP_CLIENT_VERSION), | ||
| }); | ||
|
|
||
| const buildPostHeaders = (token?: string): Record<string, string> => { | ||
| const headers: Record<string, string> = { | ||
| 'Content-Type': 'application/json', | ||
| AuthorizationType: 'ilink_bot_token', | ||
| 'X-WECHAT-UIN': randomWechatUin(), | ||
| ...buildCommonHeaders(), | ||
| }; | ||
| if (token?.trim()) headers.Authorization = `Bearer ${token.trim()}`; | ||
| return headers; |
There was a problem hiding this comment.
Include X-WECHAT-UIN on GET requests too.
The file header treats X-WECHAT-UIN as part of the iLink identity header set, but apiGet() only gets buildCommonHeaders(), so get_qrcode_status is the one transport call that drops it. That can make QR polling fail while the POST endpoints work.
💡 Proposed fix
const buildCommonHeaders = (): Record<string, string> => ({
'iLink-App-Id': ILINK_APP_ID,
'iLink-App-ClientVersion': String(ILINK_APP_CLIENT_VERSION),
+ 'X-WECHAT-UIN': randomWechatUin(),
});
const buildPostHeaders = (token?: string): Record<string, string> => {
const headers: Record<string, string> = {
'Content-Type': 'application/json',
AuthorizationType: 'ilink_bot_token',
- 'X-WECHAT-UIN': randomWechatUin(),
...buildCommonHeaders(),
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const buildCommonHeaders = (): Record<string, string> => ({ | |
| 'iLink-App-Id': ILINK_APP_ID, | |
| 'iLink-App-ClientVersion': String(ILINK_APP_CLIENT_VERSION), | |
| }); | |
| const buildPostHeaders = (token?: string): Record<string, string> => { | |
| const headers: Record<string, string> = { | |
| 'Content-Type': 'application/json', | |
| AuthorizationType: 'ilink_bot_token', | |
| 'X-WECHAT-UIN': randomWechatUin(), | |
| ...buildCommonHeaders(), | |
| }; | |
| if (token?.trim()) headers.Authorization = `Bearer ${token.trim()}`; | |
| return headers; | |
| const buildCommonHeaders = (): Record<string, string> => ({ | |
| 'iLink-App-Id': ILINK_APP_ID, | |
| 'iLink-App-ClientVersion': String(ILINK_APP_CLIENT_VERSION), | |
| 'X-WECHAT-UIN': randomWechatUin(), | |
| }); | |
| const buildPostHeaders = (token?: string): Record<string, string> => { | |
| const headers: Record<string, string> = { | |
| 'Content-Type': 'application/json', | |
| AuthorizationType: 'ilink_bot_token', | |
| ...buildCommonHeaders(), | |
| }; | |
| if (token?.trim()) headers.Authorization = `Bearer ${token.trim()}`; | |
| return headers; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/channels/wechat/src/ilink/api.ts` around lines 96 - 109, The GET
requests currently use buildCommonHeaders() which omits the X-WECHAT-UIN header
causing get_qrcode_status QR polling to fail; fix by including X-WECHAT-UIN for
GETs as well — either add 'X-WECHAT-UIN': randomWechatUin() into
buildCommonHeaders() or create a buildGetHeaders() that returns
{...buildCommonHeaders(), 'X-WECHAT-UIN': randomWechatUin()} and have
apiGet()/get_qrcode_status call that so GET transports include the same iLink
identity header used by buildPostHeaders().
Comment on lines
+191
to
+205
| try { | ||
| const raw = await apiGet({ | ||
| baseUrl: apiBaseUrl, | ||
| endpoint: `ilink/bot/get_qrcode_status?qrcode=${encodeURIComponent(qrcode)}`, | ||
| timeoutMs, | ||
| label: 'pollQrStatus', | ||
| }); | ||
| return JSON.parse(raw) as QrStatusResponse; | ||
| } catch (err) { | ||
| if (err instanceof Error && err.name === 'AbortError') { | ||
| return { status: 'wait' }; | ||
| } | ||
| // Treat gateway/network hiccups as transient. | ||
| return { status: 'wait' }; | ||
| } |
There was a problem hiding this comment.
Don't collapse hard QR polling failures into wait.
This currently turns 4xx/5xx responses and JSON parse failures into a fake pending state. The setup flow will keep spinning until TTL expiry even when the base URL is wrong or upstream is broken. Only treat abort/network timeouts as transient.
💡 Proposed fix
} catch (err) {
if (err instanceof Error && err.name === 'AbortError') {
return { status: 'wait' };
}
- // Treat gateway/network hiccups as transient.
- return { status: 'wait' };
+ if (err instanceof TypeError) {
+ // Treat fetch-level network hiccups as transient.
+ return { status: 'wait' };
+ }
+ throw err;
}
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/channels/wechat/src/ilink/api.ts` around lines 191 - 205, The catch
block around the apiGet call (the block that calls apiGet with endpoint
`ilink/bot/get_qrcode_status?qrcode=...`, timeoutMs and label 'pollQrStatus')
currently converts every failure into { status: 'wait' }; change it so only
AbortError/timeouts are treated as transient and return { status: 'wait' }, and
all other errors (HTTP 4xx/5xx, network/gateway errors, JSON parse failures) are
propagated (rethrow) so the caller can surface/fail fast; to do this, leave the
apiGet call and the existing AbortError branch intact but remove the blanket
fallback return and instead rethrow err for non-AbortError cases (or catch
JSON.parse separately around JSON.parse(raw) and rethrow parse errors) so
QrStatusResponse is not faked on real failures.
Adds the runtime plumbing and UI for adding plugin-loaded channels (like the new WeChat adapter) on demand, instead of pre-seeding rows for every registered manifest. Backend - ChannelManifestRegistry now tracks `origin: 'built-in' | 'external'` per manifest; the channel-manifests loader passes this through. - Agent create only auto-seeds rows for built-in channels (telegram, slack, discord). External plugins are added explicitly via the UI. - New `GET /api/channel-manifests` returns the catalog the picker renders (key/displayName/origin/supportsSetup/secretKeys/defaultConfig). - `POST /api/agents/:id/channels` accepts `channelType` to create a manifest-backed builtin row (previous `namespace` path still issues raw external tokens). UI - New ChannelSetupWizard (web + gateway) drives `ChannelManifest.setup` through the HTTP routes: renders QR for `awaiting_external`, a form for `awaiting_user_input`, surfaces errors, polls on its own clock. - ChannelsPanel "Add channel" dialog: picker step (manifest list + "custom external" fallback) → branches into wizard for plugins that support setup, immediate create with `defaultConfig` for those that don't, or namespace+label form for raw external tokens. - `qrcode` dep added to both UIs for client-side SVG rendering.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/channels/wechat/README.md (1)
63-63:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove accidental trailing
63from README end.Line 63 appears to contain stray numeric text, which will render as visible documentation noise.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/channels/wechat/README.md` at line 63, Remove the accidental trailing "63" from the end of the apps/channels/wechat/README.md file so it no longer appears as stray numeric noise in documentation; open README.md, locate the trailing "63" (at the file end or line 63) and delete it, then save and commit the cleaned README.
🧹 Nitpick comments (1)
apps/web/ui/src/components/ChannelSetupWizard.tsx (1)
28-28: ⚡ Quick winUpdate return type annotations for React 19 compliance.
React 19 moved the JSX namespace from global to
React.JSX. With@types/react19.1.2,JSX.Elementis not available globally. Update these return type annotations toReact.JSX.Elementor omit them (React will infer the type). This applies to:
- Line 28:
ChannelSetupWizard- Line 163:
ExternalStep- Line 180:
QrBox🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/ui/src/components/ChannelSetupWizard.tsx` at line 28, The functions ChannelSetupWizard, ExternalStep, and QrBox currently annotate their return type as JSX.Element which is not available under React 19/@types/react 19.1.2; update each signature to use React.JSX.Element (e.g. ChannelSetupWizard(...): React.JSX.Element) or simply remove the explicit return type so TypeScript can infer it, ensuring you import React if needed for the React namespace references.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/gateway/src/app.ts`:
- Around line 2380-2392: The current duplicate-check only looks at channelType;
update the pre-create validation in the same block (before calling
store.createBuiltin) to also reject when any existing active channel (from
store.listForAgent(agentId) / existing.some) has the same manifest.namespace as
the incoming body (use body.manifest?.namespace) — i.e., if
body.manifest?.namespace is defined, check existing.some(ch =>
ch.manifest?.namespace === body.manifest.namespace && !ch.revokedAt) and throw a
ValidationError with a clear message; keep the existing channelType check and
only proceed to call store.createBuiltin if both checks pass.
In `@apps/gateway/ui/src/components/ChannelSetupWizard.tsx`:
- Around line 75-94: The current useEffect's tick catch block sets setError and
returns without scheduling the next poll, which can stall polling when
state.kind === 'awaiting_external'; update the tick implementation in the
useEffect so that whether the api call succeeds or fails it still schedules the
next poll (using pollTimerRef.current = setTimeout(() => { void tick(); },
interval)), ensuring you still check cancelledRef.current before applying resp
or setting error and use the same interval (state.pollIntervalMs ?? 2000); keep
the existing cleanup that clears pollTimerRef.current and leave the
cancelledRef, apply, setError logic intact.
In `@apps/web/ui/src/components/ChannelSetupWizard.tsx`:
- Around line 67-84: The current useEffect in ChannelSetupWizard stops polling
on the first pollChannelSetup() failure because the catch only sets setError and
doesn't schedule the next tick; update the tick function so that after either a
successful apply(resp) or after handling an error you schedule the next poll
(use pollTimerRef.current = setTimeout(() => { void tick(); }, interval)); keep
honoring cancelledRef.current and state.kind checks before scheduling, and
ensure the existing cleanup (clearTimeout in the return) still clears the
scheduled timer. This change touches the tick async function and the
pollTimerRef scheduling logic around pollChannelSetup, cancelledRef,
pollTimerRef, apply, and setError.
- Around line 49-64: The component currently starts a server-side setup via
beginChannelSetup but never cancels that session when the component unmounts
unless handleCancel() is invoked; update the effect cleanup to call the same
cancellation logic used by handleCancel (or call handleCancel directly) so the
server-side setup session is aborted on unmount: locate the effect using
cancelledRef, beginChannelSetup, apply and pollTimerRef and ensure the returned
cleanup sets cancelledRef.current = true, clears pollTimerRef, and triggers the
server-side cancellation function (handleCancel or the underlying cancel API
used by handleCancel) so the remote setup session is terminated when the dialog
is closed by Esc/backdrop/parent unmount.
In `@apps/web/ui/src/components/ChannelsPanel.tsx`:
- Around line 67-70: Change manifests from an empty array to a nullable state so
you can distinguish "loading" vs "empty": update the useState declaration
(manifests, setManifests) to use type ChannelManifestSummary[] | null and
initialize it to null; update any rendering logic (the manifest picker and the
other checks around the spots currently showing "No channel packages
registered.") to treat manifests === null as the loading state (show
spinner/placeholders) and only treat manifests.length === 0 as the true empty
state; ensure your fetch code calls setManifests(fetchedArray) or
setManifests([]) when complete so the UI leaves the loading state.
---
Outside diff comments:
In `@apps/channels/wechat/README.md`:
- Line 63: Remove the accidental trailing "63" from the end of the
apps/channels/wechat/README.md file so it no longer appears as stray numeric
noise in documentation; open README.md, locate the trailing "63" (at the file
end or line 63) and delete it, then save and commit the cleaned README.
---
Nitpick comments:
In `@apps/web/ui/src/components/ChannelSetupWizard.tsx`:
- Line 28: The functions ChannelSetupWizard, ExternalStep, and QrBox currently
annotate their return type as JSX.Element which is not available under React
19/@types/react 19.1.2; update each signature to use React.JSX.Element (e.g.
ChannelSetupWizard(...): React.JSX.Element) or simply remove the explicit return
type so TypeScript can infer it, ensuring you import React if needed for the
React namespace references.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7891bfa-df95-4a58-8cdd-a27200e99eb6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
apps/channels/wechat/README.mdapps/gateway/package.jsonapps/gateway/src/app.tsapps/gateway/src/channel-manifests.tsapps/gateway/ui/src/components/ChannelSetupWizard.tsxapps/gateway/ui/src/components/ChannelsPanel.tsxapps/web/package.jsonapps/web/public/assets/ChatShell-CHLimlh_.jsapps/web/public/assets/ManagePanel-1wY7vfd2.jsapps/web/public/assets/ManagePanel-DXUe2m7i.jsapps/web/public/assets/index-CIUAGvj5.jsapps/web/public/assets/index-M1UGArc6.jsapps/web/public/assets/react-vendor-yfL0ty4i.jsapps/web/public/index.htmlapps/web/ui/src/api.tsapps/web/ui/src/components/ChannelSetupWizard.tsxapps/web/ui/src/components/ChannelsPanel.tsxpackages/protocol/src/index.ts
💤 Files with no reviewable changes (1)
- apps/web/public/assets/ManagePanel-1wY7vfd2.js
✅ Files skipped from review due to trivial changes (2)
- apps/web/public/index.html
- apps/web/public/assets/ChatShell-CHLimlh_.js
Comment on lines
+2380
to
+2392
| const existing = await store.listForAgent(agentId); | ||
| if (existing.some((ch) => ch.channelType === channelType && !ch.revokedAt)) { | ||
| throw new ValidationError( | ||
| `A channel of type "${channelType}" already exists on this agent.`, | ||
| ); | ||
| } | ||
| const created = await store.createBuiltin({ | ||
| agentId, | ||
| channelType, | ||
| ...(body.label ? { label: body.label } : {}), | ||
| ...(body.config ? { config: body.config } : {}), | ||
| ...(typeof body.enabled === 'boolean' ? { enabled: body.enabled } : {}), | ||
| }); |
There was a problem hiding this comment.
Prevent namespace collisions in manifest-backed channel creation.
Line 2380 only checks channelType duplicates. If an existing external row already uses manifest.namespace, this path can create two active rows with the same namespace.
Suggested fix
const existing = await store.listForAgent(agentId);
if (existing.some((ch) => ch.channelType === channelType && !ch.revokedAt)) {
throw new ValidationError(
`A channel of type "${channelType}" already exists on this agent.`,
);
}
+ if (existing.some((ch) => ch.namespace === manifest.namespace && !ch.revokedAt)) {
+ throw new ValidationError(
+ `A channel with namespace "${manifest.namespace}" already exists on this agent.`,
+ );
+ }
const created = await store.createBuiltin({
agentId,
channelType,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/gateway/src/app.ts` around lines 2380 - 2392, The current
duplicate-check only looks at channelType; update the pre-create validation in
the same block (before calling store.createBuiltin) to also reject when any
existing active channel (from store.listForAgent(agentId) / existing.some) has
the same manifest.namespace as the incoming body (use body.manifest?.namespace)
— i.e., if body.manifest?.namespace is defined, check existing.some(ch =>
ch.manifest?.namespace === body.manifest.namespace && !ch.revokedAt) and throw a
ValidationError with a clear message; keep the existing channelType check and
only proceed to call store.createBuiltin if both checks pass.
Comment on lines
+75
to
+94
| useEffect(() => { | ||
| if (!sessionId || !state || state.kind !== 'awaiting_external') return; | ||
| const interval = state.pollIntervalMs ?? 2000; | ||
| const tick = async (): Promise<void> => { | ||
| try { | ||
| const resp = await api<SetupResponse>( | ||
| `/api/agents/${encodeURIComponent(agentId)}/channels/${encodeURIComponent(channelType)}/setup/${encodeURIComponent(sessionId)}`, | ||
| ); | ||
| if (cancelledRef.current) return; | ||
| apply(resp); | ||
| } catch (err) { | ||
| if (cancelledRef.current) return; | ||
| setError((err as Error).message); | ||
| } | ||
| }; | ||
| pollTimerRef.current = setTimeout(() => { void tick(); }, interval); | ||
| return () => { | ||
| if (pollTimerRef.current) clearTimeout(pollTimerRef.current); | ||
| }; | ||
| }, [sessionId, state, agentId, channelType, apply]); |
There was a problem hiding this comment.
Keep polling alive after transient poll failures.
At Line 85, the error path updates UI error but stops further polling if state remains awaiting_external, so one temporary failure can stall the wizard.
Suggested fix
useEffect(() => {
if (!sessionId || !state || state.kind !== 'awaiting_external') return;
const interval = state.pollIntervalMs ?? 2000;
const tick = async (): Promise<void> => {
try {
const resp = await api<SetupResponse>(
`/api/agents/${encodeURIComponent(agentId)}/channels/${encodeURIComponent(channelType)}/setup/${encodeURIComponent(sessionId)}`,
);
if (cancelledRef.current) return;
apply(resp);
} catch (err) {
if (cancelledRef.current) return;
setError((err as Error).message);
+ pollTimerRef.current = setTimeout(() => { void tick(); }, interval);
}
};
pollTimerRef.current = setTimeout(() => { void tick(); }, interval);
return () => {
if (pollTimerRef.current) clearTimeout(pollTimerRef.current);
};
}, [sessionId, state, agentId, channelType, apply]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| if (!sessionId || !state || state.kind !== 'awaiting_external') return; | |
| const interval = state.pollIntervalMs ?? 2000; | |
| const tick = async (): Promise<void> => { | |
| try { | |
| const resp = await api<SetupResponse>( | |
| `/api/agents/${encodeURIComponent(agentId)}/channels/${encodeURIComponent(channelType)}/setup/${encodeURIComponent(sessionId)}`, | |
| ); | |
| if (cancelledRef.current) return; | |
| apply(resp); | |
| } catch (err) { | |
| if (cancelledRef.current) return; | |
| setError((err as Error).message); | |
| } | |
| }; | |
| pollTimerRef.current = setTimeout(() => { void tick(); }, interval); | |
| return () => { | |
| if (pollTimerRef.current) clearTimeout(pollTimerRef.current); | |
| }; | |
| }, [sessionId, state, agentId, channelType, apply]); | |
| useEffect(() => { | |
| if (!sessionId || !state || state.kind !== 'awaiting_external') return; | |
| const interval = state.pollIntervalMs ?? 2000; | |
| const tick = async (): Promise<void> => { | |
| try { | |
| const resp = await api<SetupResponse>( | |
| `/api/agents/${encodeURIComponent(agentId)}/channels/${encodeURIComponent(channelType)}/setup/${encodeURIComponent(sessionId)}`, | |
| ); | |
| if (cancelledRef.current) return; | |
| apply(resp); | |
| } catch (err) { | |
| if (cancelledRef.current) return; | |
| setError((err as Error).message); | |
| pollTimerRef.current = setTimeout(() => { void tick(); }, interval); | |
| } | |
| }; | |
| pollTimerRef.current = setTimeout(() => { void tick(); }, interval); | |
| return () => { | |
| if (pollTimerRef.current) clearTimeout(pollTimerRef.current); | |
| }; | |
| }, [sessionId, state, agentId, channelType, apply]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/gateway/ui/src/components/ChannelSetupWizard.tsx` around lines 75 - 94,
The current useEffect's tick catch block sets setError and returns without
scheduling the next poll, which can stall polling when state.kind ===
'awaiting_external'; update the tick implementation in the useEffect so that
whether the api call succeeds or fails it still schedules the next poll (using
pollTimerRef.current = setTimeout(() => { void tick(); }, interval)), ensuring
you still check cancelledRef.current before applying resp or setting error and
use the same interval (state.pollIntervalMs ?? 2000); keep the existing cleanup
that clears pollTimerRef.current and leave the cancelledRef, apply, setError
logic intact.
Comment on lines
+49
to
+64
| useEffect(() => { | ||
| cancelledRef.current = false; | ||
| (async () => { | ||
| try { | ||
| const resp = await beginChannelSetup(channelType); | ||
| if (cancelledRef.current) return; | ||
| apply(resp); | ||
| } catch (err) { | ||
| setError((err as Error).message); | ||
| } | ||
| })(); | ||
| return () => { | ||
| cancelledRef.current = true; | ||
| if (pollTimerRef.current) clearTimeout(pollTimerRef.current); | ||
| }; | ||
| }, [channelType, apply]); |
There was a problem hiding this comment.
Cancel the server-side setup session during unmount.
Closing the dialog via Esc/backdrop or any parent-driven unmount skips handleCancel(), so the setup session can keep running until its TTL expires.
Suggested fix
export function ChannelSetupWizard({ channelType, displayName, onDone, onCancel }: Props): JSX.Element {
const [sessionId, setSessionId] = useState<string | null>(null);
const [state, setState] = useState<ChannelSetupState | null>(null);
const [error, setError] = useState('');
const [formInput, setFormInput] = useState<Record<string, string>>({});
const [submitting, setSubmitting] = useState(false);
const pollTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const cancelledRef = useRef(false);
+ const sessionIdRef = useRef<string | null>(null);
const apply = useCallback((resp: ChannelSetupResponse) => {
+ sessionIdRef.current = resp.sessionId;
setSessionId(resp.sessionId);
setState(resp.state);
setError('');
if (resp.state.kind === 'awaiting_user_input') {
const initial: Record<string, string> = {};
@@
return () => {
cancelledRef.current = true;
if (pollTimerRef.current) clearTimeout(pollTimerRef.current);
+ const sid = sessionIdRef.current;
+ if (sid) void cancelChannelSetup(channelType, sid).catch(() => {});
};
}, [channelType, apply]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/ui/src/components/ChannelSetupWizard.tsx` around lines 49 - 64, The
component currently starts a server-side setup via beginChannelSetup but never
cancels that session when the component unmounts unless handleCancel() is
invoked; update the effect cleanup to call the same cancellation logic used by
handleCancel (or call handleCancel directly) so the server-side setup session is
aborted on unmount: locate the effect using cancelledRef, beginChannelSetup,
apply and pollTimerRef and ensure the returned cleanup sets cancelledRef.current
= true, clears pollTimerRef, and triggers the server-side cancellation function
(handleCancel or the underlying cancel API used by handleCancel) so the remote
setup session is terminated when the dialog is closed by Esc/backdrop/parent
unmount.
Comment on lines
+67
to
+84
| useEffect(() => { | ||
| if (!sessionId || !state || state.kind !== 'awaiting_external') return; | ||
| const interval = state.pollIntervalMs ?? 2000; | ||
| const tick = async (): Promise<void> => { | ||
| try { | ||
| const resp = await pollChannelSetup(channelType, sessionId); | ||
| if (cancelledRef.current) return; | ||
| apply(resp); | ||
| } catch (err) { | ||
| if (cancelledRef.current) return; | ||
| setError((err as Error).message); | ||
| } | ||
| }; | ||
| pollTimerRef.current = setTimeout(() => { void tick(); }, interval); | ||
| return () => { | ||
| if (pollTimerRef.current) clearTimeout(pollTimerRef.current); | ||
| }; | ||
| }, [sessionId, state, channelType, apply]); |
There was a problem hiding this comment.
Keep polling after transient failures.
One failed pollChannelSetup() turns the wizard into a terminal error and stops the loop, even though the server-side session may still be alive.
Suggested fix
const tick = async (): Promise<void> => {
try {
const resp = await pollChannelSetup(channelType, sessionId);
if (cancelledRef.current) return;
apply(resp);
} catch (err) {
if (cancelledRef.current) return;
setError((err as Error).message);
+ pollTimerRef.current = setTimeout(() => { void tick(); }, interval);
}
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/ui/src/components/ChannelSetupWizard.tsx` around lines 67 - 84, The
current useEffect in ChannelSetupWizard stops polling on the first
pollChannelSetup() failure because the catch only sets setError and doesn't
schedule the next tick; update the tick function so that after either a
successful apply(resp) or after handling an error you schedule the next poll
(use pollTimerRef.current = setTimeout(() => { void tick(); }, interval)); keep
honoring cancelledRef.current and state.kind checks before scheduling, and
ensure the existing cleanup (clearTimeout in the return) still clears the
scheduled timer. This change touches the tick async function and the
pollTimerRef scheduling logic around pollChannelSetup, cancelledRef,
pollTimerRef, apply, and setError.
Reverted the "don't auto-seed plugins" rule from the prior bundled
commit. With manifest origin recorded at registration time, both the
gateway admin and web UIs can now split the channels list into:
1. Built-in — kind=builtin, manifest origin='built-in' (TG/Slack/Discord)
2. Package — kind=builtin, manifest origin='external' (e.g. WeChat).
Rows get a "Set up" button that drives the manifest's
ChannelSetup wizard, then PATCHes the existing row's
config + enables it.
3. Token-only — kind=external (owner-issued tokens). Replaced the old
Add-channel manifest picker with a single "Issue new
token" button on that section.
Auto-seed at agent create + startup backfill again iterate over the full
registry (built-in + plugin), so adding a package via `channelPackages`
gets every agent a disabled row on the next restart, ready for Set up.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/gateway/ui/src/components/ChannelsPanel.tsx (1)
100-105: 💤 Low value
manifestByKeyandgroupOfare recomputed on every render.These computations run on each render. For small manifest lists this is fine, but wrapping in
useMemowould be cleaner. Low priority given the small data size.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/ui/src/components/ChannelsPanel.tsx` around lines 100 - 105, Wrap the manifestByKey creation in React's useMemo to avoid rebuilding the Map on every render (e.g. const manifestByKey = useMemo(() => new Map(manifests.map(m => [m.key, m])), [manifests])); also memoize the groupOf function with useCallback (e.g. const groupOf = useCallback((ch: ChannelRecord): GroupKey => { ... }, [manifestByKey])) so it stays stable across renders; ensure useMemo/useCallback are imported from React and keep the same logic that checks ch.kind and manifestByKey.get(ch.channelType)?.origin.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/gateway/ui/src/components/ChannelsPanel.tsx`:
- Around line 100-105: Wrap the manifestByKey creation in React's useMemo to
avoid rebuilding the Map on every render (e.g. const manifestByKey = useMemo(()
=> new Map(manifests.map(m => [m.key, m])), [manifests])); also memoize the
groupOf function with useCallback (e.g. const groupOf = useCallback((ch:
ChannelRecord): GroupKey => { ... }, [manifestByKey])) so it stays stable across
renders; ensure useMemo/useCallback are imported from React and keep the same
logic that checks ch.kind and manifestByKey.get(ch.channelType)?.origin.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7c23d60-ee53-49c6-bd7d-14268e2d4e25
📒 Files selected for processing (7)
apps/gateway/src/app.tsapps/gateway/ui/src/components/ChannelsPanel.tsxapps/web/public/assets/ChatShell-CM1fo73L.jsapps/web/public/assets/ManagePanel-BD6QX2Pm.jsapps/web/public/assets/index-Bvg-01Kf.jsapps/web/public/index.htmlapps/web/ui/src/components/ChannelsPanel.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/web/public/assets/ChatShell-CM1fo73L.js
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/public/index.html
Auto-seeded channel rows have `label: null`, so the card was rendering the raw key (e.g. "wechat") instead of the manifest's human name (e.g. "WeChat"). Fall through `ch.label → manifest.displayName → channelType` in both the web and gateway admin UIs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/gateway/ui/src/components/ChannelsPanel.tsx (2)
100-105: ⚡ Quick winOptimize derived state and helpers with memoization.
The
manifestByKeyMap andgroupOffunction are recreated on every render. Sincemanifestschanges infrequently, wrap them inuseMemoanduseCallbackto avoid unnecessary recreations.♻️ Proposed optimization
- const manifestByKey = new Map(manifests.map((m) => [m.key, m])); + const manifestByKey = useMemo( + () => new Map(manifests.map((m) => [m.key, m])), + [manifests] + ); - const groupOf = (ch: ChannelRecord): GroupKey => { + const groupOf = useCallback((ch: ChannelRecord): GroupKey => { if (ch.kind === 'external') return 'token'; return manifestByKey.get(ch.channelType)?.origin === 'external' ? 'package' : 'builtin'; - }; + }, [manifestByKey]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/ui/src/components/ChannelsPanel.tsx` around lines 100 - 105, The manifestByKey Map and groupOf helper are recreated on every render; wrap the Map creation in useMemo (memoizing over manifests) and wrap groupOf in useCallback (depending on manifestByKey) so they are stable across renders; update references to useMemo/useCallback for manifestByKey and groupOf, ensuring you import them from React and use manifests as the dependency for the memo and manifestByKey for the callback.
167-185: 💤 Low valueWrap
renderCardinuseCallbackfor consistency.The
renderCardhelper is recreated on every render. While React's reconciliation will handle this correctly, wrapping it inuseCallbackmatches the pattern used for other event handlers and avoids unnecessary function creations.♻️ Proposed optimization
- const renderCard = (ch: ChannelRecord, group: GroupKey) => { + const renderCard = useCallback((ch: ChannelRecord, group: GroupKey) => { const manifest = manifestByKey.get(ch.channelType); const canSetup = group === 'package' && manifest?.supportsSetup === true; const displayName = ch.label ?? manifest?.displayName ?? ch.channelType; return ( <ChannelCard key={ch.id} ch={ch} displayName={displayName} statusClass={statusClass(ch)} statusText={statusText(ch)} canSetup={canSetup} onSetup={() => setSetupFor(ch)} onEdit={() => setEditing(ch)} onToggle={() => void handleToggle(ch)} onDelete={() => void handleDelete(ch)} /> ); - }; + }, [manifestByKey, setSetupFor, setEditing, handleToggle, handleDelete]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/ui/src/components/ChannelsPanel.tsx` around lines 167 - 185, renderCard is recreated on every render; wrap it in React.useCallback to memoize it. Convert the renderCard function into a useCallback that returns the same ChannelCard, and include all external dependencies in the dependency array: manifestByKey, setSetupFor, setEditing, handleToggle, handleDelete, statusClass, statusText (and any other variables referenced, e.g., group if captured). Ensure the callback signature stays (ch: ChannelRecord, group: GroupKey) => JSX and the onSetup/onEdit/onToggle/onDelete handlers continue to call setSetupFor, setEditing, handleToggle, handleDelete respectively.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/gateway/ui/src/components/ChannelsPanel.tsx`:
- Around line 100-105: The manifestByKey Map and groupOf helper are recreated on
every render; wrap the Map creation in useMemo (memoizing over manifests) and
wrap groupOf in useCallback (depending on manifestByKey) so they are stable
across renders; update references to useMemo/useCallback for manifestByKey and
groupOf, ensuring you import them from React and use manifests as the dependency
for the memo and manifestByKey for the callback.
- Around line 167-185: renderCard is recreated on every render; wrap it in
React.useCallback to memoize it. Convert the renderCard function into a
useCallback that returns the same ChannelCard, and include all external
dependencies in the dependency array: manifestByKey, setSetupFor, setEditing,
handleToggle, handleDelete, statusClass, statusText (and any other variables
referenced, e.g., group if captured). Ensure the callback signature stays (ch:
ChannelRecord, group: GroupKey) => JSX and the onSetup/onEdit/onToggle/onDelete
handlers continue to call setSetupFor, setEditing, handleToggle, handleDelete
respectively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d7f1d427-ad11-48ed-b022-5739dddea9ae
📒 Files selected for processing (6)
apps/gateway/ui/src/components/ChannelsPanel.tsxapps/web/public/assets/ChatShell-CbYQSfDa.jsapps/web/public/assets/ManagePanel-eci-4VNG.jsapps/web/public/assets/index-6IS3qDyk.jsapps/web/public/index.htmlapps/web/ui/src/components/ChannelsPanel.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/web/public/index.html
- apps/web/public/assets/ChatShell-CbYQSfDa.js
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/ui/src/components/ChannelsPanel.tsx
This was referenced May 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ChannelManifest+ChannelSetupcontract from PRs feat(protocol): channel plugin contract — ChannelManifest + ChannelManifestRegistry #87/feat(channels): wire built-ins through manifest registry #88/feat(channels): interactive setup contract for QR-link / OAuth flows #90.@openhermit/channel-wechatinchannelPackages.Layout
Setup flow
POST /api/agents/:id/channels/wechat/setup/begin→ plugin fetches a QR fromilinkai.weixin.qq.com/ilink/bot/get_bot_qrcode, kicks off a background long-poll onget_qrcode_status, returnsawaiting_externalwithqrTextset to the QR URL string.GET /api/agents/:id/channels/wechat/setup/:sessionId. Server pushes status transitions (wait→scaned→confirmed); onconfirmedreturnsdonewithconfig: { bot_token, base_url, ilink_bot_id, ilink_user_id? }./api/agents/:id/channelsto persist the row; the manifest'sstart()boots the long-poll bot.need_verifycodeandverify_code_blockedare not supported in v0 (Tencent's CLI handles them via stdin) — the wizard surfaces an error and the user re-scans.Notes
iLink-App-Idis read from the package's ownpackage.json(ilink_appid) — matches Tencent's convention. Operators with their own iLink app can patch the field or setOPENHERMIT_WECHAT_APP_ID.@tencent-weixin/openclaw-weixin(MIT-licensed); none of that package's code is depended on at runtime.Test plan
npm run typecheck(workspace)channelPackagesagainst test gateway🤖 Generated with Claude Code
Summary by CodeRabbit