feat: add Cisco Webex platform adapter - #1402
Merged
Merged
Conversation
…1, token redaction, rune-safe chunking, goroutine + device cleanup
1 task
Contributor
Author
|
Verified end-to-end against a live Webex bot (Bedrock-backed Claude Code agent):
Implementation notes worth a QA eye:
Not yet stress-tested (flagging honestly for QA):
Happy to address anything QA turns up. |
chenhg5
approved these changes
Jun 19, 2026
chenhg5
left a comment
Owner
There was a problem hiding this comment.
结论: Approve (with follow-up suggestions)
总体判断: 干净、完整、有真实 e2e 验证的 Webex 适配器,实现遵循 cc-connect 的 platform 模式 (init() 注册、webexClient interface for testability、单独的 reply file、compile-time interface assertions),1216 LoC 全部 unit-test 覆盖到位,CI 全绿,推荐合入。 同时记录 2 个非阻塞 follow-up (1 个 gofmt 1 行、1 个 missing docs/webex.md) 给作者斟酌。
Review 范围:
- 看了 platform/webex/{client.go, types.go, webex.go, webex_reply.go} 4 个生产文件
- 看了 platform/webex/{client_test.go, webex_test.go} 2 个测试文件
- 看了 cmd/cc-connect/plugin_platform_webex.go、config.example.toml、CLAUDE.md、Makefile 的 4 处元数据同步
- 在 worktree qa-pr-1402-verify @ 40b87ed 跑过 24/24 tests + go vet + gofmt + no_webex build
- 重点关注: optional interface 完整性、auth + 重连 + 429 + 401 错误处理、Mercury 加密帧转换、rune-safe chunking、reply context 类型安全
✅ 做得好的地方:
- optional interface 选择周全: 6 个 interface (Platform / ImageSender / FileSender / ReplyContextReconstructor / FormattingInstructionProvider / AsyncRecoverablePlatform) 全用 compile-time 断言锁住,适配器是真正的"按需可选"而不是"全做",跟 matrix / wecom / wps-xiezuo 等老平台风格一致。 AsyncRecoverablePlatform (SetLifecycleHandler + OnPlatformReady/OnPlatformUnavailable) 是核心 — Webex 没有公开 webhook,Device WebSocket 断了之后必须自动重连,而 401 这种"token 死了"的情况要 fire-and-forget 通知上层而不是无限循环。
- Mercury 加密帧处理正确: 不直接解析 frame body,而是 activityID → base64("ciscospark://us/MESSAGE/{uuid}") → GET /v1/messages/{id}。 这条路径在 PR body 里写得很清楚,处理 verb filter (post / share 进,update / highlight 跳过) 也对 (避免 malware-scan re-notification 重复处理)。
- errUnauthorized 的 wiring 设计优雅: doWithRetry 把 401 立刻变 errUnauthorized,让 connectLoop 在第一行就
if errors.Is(err, errUnauthorized)决定"token 死了,fire OnPlatformUnavailable,return 永不再试"。 其他 error 走 exponential backoff (1s → 2s → 4s → ... → 30s cap),stable 10s 后重置。 这是个正确的"可恢复 vs 不可恢复"二分法。 - rune-safe chunking: chunkMarkdown 在硬切到 limit bytes 之前会回退到 RuneStart 边界,保证不切断多字节 UTF-8 字符。 4 字节 emoji (🥺 这种 surrogate-pair) 也能完整跨 chunk。 TestChunkMultibyteNoSplit 显式覆盖了这条路径,不是嘴上说说。
- reconnect 时清理 stale device: runConnection 里 prevDevice != dev.URL 时主动 DeleteDevice,避免 Webex 端 WDM 设备表里堆积幽灵设备。 是个易被忽略的细节, 写得很对。
- token redaction: slog.Error/Warn 都过 core.RedactToken, 即使 token 进了 error message 也不会泄露到日志。 跟 #1317 / 跨平台的 secret-handling 行为一致。
- self email 比对用 EqualFold + skip own messages: handleFrame 跳过 bot 自己发的 post,避免 echo loop。 跟 feishu / slack 的 "skip self" 模式一致。
- Makefile / CLAUDE.md / config.example.toml 同步: 4 个元数据点都改了,no_webex build tag 也 work (用 CGO_ENABLED=0 跑过 build 验证)。
- e2e 真实环境: PR body 写了 1:1 文本 / 群 @mention / 文件图片 三类 live 测试通过。 1216 LoC 新 platform 单测能盖到的边界有限,e2e 补位有价值。
🟠 建议改进 (本 PR 不阻塞, 但建议本 PR 收口):
- 🟠 P2: 缺 docs/webex.md。 AGENTS.md 显式列了 7 步 platform 添加清单,虽然没硬性要求 docs,但项目惯例 (matrix.md 在 PR #834 同 PR 一起 add, feishu.md / discord.md / wecom.md / weibo.md / weixin.md 全部存在) 是新 platform 必带一份 setup 指南。 用户 onboarding 路径需要: 1) 创建 bot 的步骤 (developer.webex.com/my-apps/new/bot), 2) 取 access token, 3) config example, 4) Mercury 设备注册的 model 字段 (已在 client.go:136 注释里写, 但用户看不见), 5) allow_from 邮箱白名单怎么配, 6) 1:1 vs 群组 @mention 行为差异, 7) 常见错误 (401 / 429 / WDM Missing Model)。 matrix.md / feishu.md / wecom.md / wps-xiezuo.md 都是这种粒度的 setup guide,缺了 onboarding 摩擦会大。 建议本 PR 收口前补一份 docs/webex.md (含必要的 i18n 配套,docs/matrix.md 就有 matrix.zh-CN.md)。
- 🟠 P2: 没有 web/ UI 配置面板。 部分平台 (wecom / wps-xiezuo / weibo 等) 在 web/ 下有 platforms/.tsx 配置页; Webex 没有这个。 这条比 docs 更接近 nice-to-have — 不会阻塞合入,但补上的话用户就不用手写 config.example.toml 那段。 follow-up 即可,本 PR 不阻塞。
🔵 可选:
- 🔵 P3: gofmt -l platform/webex/webex.go 报告一行字段对齐问题 (line 154-155 buildMessage 内 struct 字面量):
Content: content, ReplyCtx: replyContext{roomID: m.RoomID, messageID: m.ID, personID: m.PersonID},
1 行 trivial 修复, 但 .golangci.yml 现在有 formatting check, 合入后 CI 可能也跑这个。 建议顺手修。- Content: content, - ReplyCtx: replyContext{roomID: m.RoomID, messageID: m.ID, personID: m.PersonID}, + Content: content, + ReplyCtx: replyContext{roomID: m.RoomID, messageID: m.ID, personID: m.PersonID},
- 🔵 P3: 401 路径 fire-and-forget 后没有 OnPlatformRecovered 重新连接机制。 如果 owner 后续手动换 token 然后 ping platform, connectLoop 已经 return, 不会自动重连。 现阶段 AsyncRecoverablePlatform 没要求 Recovered hook, 是 platform 自己的设计取舍; 但建议在 PR body 末尾加一行 "known limitation: after 401 the device is permanently disconnected until cc-connect restart", 让用户有预期。 同样不阻塞。
- 🔵 P3: webexMaxBytes=7439 是个 magic number。 在 client.go 跟 webex_reply.go 都重复出现 (webexMaxBytes 在 reply 文件定义, 但 chunkMarkdown 接受参数)。 没问题, 但加一行 const 注释引用官方文档 (Webex Messages API 7439-byte markdown cap) 更好, 后续 Webex 调 cap 时找得到出处。
❓ 没有必须问的。 Mercury 加密帧的 vendor-specific 行为 + Model 字段必填这两点已经在 PR body 跟 client.go 注释里讲清楚了, 不需要作者再解释。
Testing / Risk:
- 已看到 CI 27826462682 5/5 green (lint / unit / smoke / regression / performance)。
- 本地
go test -count=1 ./platform/webex/24/24 PASS (0.015s): TestGetMessageRetriesOn429 / TestNewRequiresToken / TestNewParsesAllowFrom / TestStripMention(x2) / TestShouldProcess(x3) / TestBuildMessage(x4) / TestChunk(x4) / TestIsAllowed(x2) / TestReplyPostsWithParent / TestSendPostsWithoutParent / TestActivityIDToMessageID / TestHandleFrame(x4)。 - 本地
go test -count=1 -tags no_webex ./platform/...PASS,go build -tags no_webex ./cmd/cc-connect干净 (web/embed.go pre-existing 错误跟本 PR 无关)。 go vet ./platform/webex/0 issue。- gofmt: 1 行 trivial 字段对齐, 见 P3。 不阻塞。
- 风险面: 只新增 platform/webex/ + 4 处元数据 + 1 个 plugin_*.go 桩, 不影响 core / 其他 platform / agent。 WDM device URL 注册是一次性副作用 (启动一次注册一个, 退出 DeleteDevice 清掉), 不会在 Webex 端累积幽灵设备。
- 安全: token redact 进 slog, 401 立即 fail (不重试不泄露 timing), allowlist lowercase + trim 防绕过。 没看到 security blocker。
Next step:
- 合并即可, docs/webex.md + 1 行 gofmt 顺手在本 PR 收口最好。 owner 拍板: ① 是合当前 + 立即开 follow-up issue 写 docs, ② 还是让作者在本 PR 补 docs 再 merge。 推荐 ② (matrix / feishu 当时都是同 PR 补的,保持惯例), 但 ① 也合理 (1216 LoC 已是 substantive, 别拖作者太久)。
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
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.
Closes #1401
What
Adds Cisco Webex as a native platform adapter (
platform/webex/), following existing platform conventions (registryinit(),webexClientinterface fortestability, separate reply file, lifecycle handler, compile-time interface assertions).
Why
Webex is heavily used in enterprise/Cisco environments and exposes a real-time Device WebSocket API that needs no public IP — matching the connection model
of Feishu/Slack/Discord.
Details
allow_from), honors*wildcardRetry-Afterhonored; 401 stops retrying; token redacted in logsConfig: