From f9430dee810d87cb4325f7614f50ddb2f16aec9a Mon Sep 17 00:00:00 2001 From: AskaEth Date: Sat, 3 Oct 2026 21:06:22 +0800 Subject: [PATCH] =?UTF-8?q?fix(admin):=20=E6=B8=A0=E9=81=93=20personaId=20?= =?UTF-8?q?=E6=94=B9=E4=B8=BA=E4=B8=89=E6=80=81=E8=AF=AD=E4=B9=89=EF=BC=88?= =?UTF-8?q?=E7=9C=81=E7=95=A5=3D=E4=BF=9D=E7=95=99=20/=20""=3D=E8=A7=A3?= =?UTF-8?q?=E7=BB=91=20/=20id=3D=E7=BB=91=E5=AE=9A=EF=BC=89=E2=80=94?= =?UTF-8?q?=E2=80=94=E4=BF=AE=E3=80=8C=E4=BF=9D=E5=AD=98=E5=85=B6=E5=AE=83?= =?UTF-8?q?=E9=85=8D=E7=BD=AE=E5=8D=B3=E9=9D=99=E9=BB=98=E6=B8=85=E7=A9=BA?= =?UTF-8?q?=E7=BB=91=E5=AE=9A=E3=80=8D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 跨端语义相反：后端 5969bba 实现的是「省略/\"\"=解绑」，而管理面板只在用户改动下拉时才发该键 ⇒ owner 打开已绑账号、不动下拉、保存其它配置就会静默清空渠道角色绑定。 - 请求 DTO 改 *string 区分「省略」与「显式空串」：省略 → 保留现状；显式 "" → 解绑；非空 id → 绑定（未知角色 400）；沿用读-改-写 + bridge.Reload 热更新 + 审计 - GET /admin/channels 继续回显 account.personaId（已确认） - Docs/api-client 三态语义写清（省略=保留是本次的关键修正） - 测试：省略保留 / 显式空串解绑 / id 绑定 / 未知 id 400 + 变异验证（把省略改回解绑 → 断言 FAIL） --- Docs/api-client.md | 7 +- .../gateway/admin_channel_persona_test.go | 215 ++++++++++++++---- .../gateway/admin_write_channel_config.go | 66 +++++- 3 files changed, 236 insertions(+), 52 deletions(-) diff --git a/Docs/api-client.md b/Docs/api-client.md index 756bb21..7d43de4 100644 --- a/Docs/api-client.md +++ b/Docs/api-client.md @@ -110,7 +110,7 @@ - Web 端镜像实现在 `frontend/web/src/lib/enums.ts`（`labelFor(kind, value)` 对未知值返回原值），权威仍以本端点为准。 - **反漂移保证（后端）**：后端有测试断言「每个内置适配器的 `Name()` 都在 `platforms` 表内」「`moods` 与 persona 情绪状态机的内置状态一致」「`modes` 与前端 `MODES` 九模式一致」——漏登记即 CI FAIL，客户端拿到的表不会滞后于代码。 -> **契约版本 `v0.44`**（渠道级选角：**`PUT /admin/channels/{platform}/config` body 新增可选 `personaId`、`GET /admin/channels` 每账号新增可选 `personaId`**，均为新增字段，向后兼容；同时明确**角色解析优先级**：会话绑定 > 渠道绑定 > 全局默认，群聊/陌生人强制默认角色）。上一版 `v0.43`：人格读侧补齐（admin `thinking` 回读 + 用户侧角色级 `mood`）。 +> **契约版本 `v0.45`**（**`PUT /admin/channels/{platform}/config` 的 `personaId` 由「省略/`""` 都 = 解绑」修订为三态**：**省略（或 `null`）= 保留已存绑定**、**显式 `""` = 解绑**、**非空 id = 绑定**，未知角色仍 `400`——修 v0.44 会**静默清空**渠道角色绑定的一处前后端契约不一致；纯语义收紧，**字段与端点形状不变**）。上一版 `v0.44`：渠道级选角（`PUT /admin/channels/{platform}/config` body 新增可选 `personaId`、`GET /admin/channels` 每账号新增可选 `personaId`）+ 角色解析优先级（会话绑定 > 渠道绑定 > 全局默认，群聊/陌生人强制默认角色）。 ## 2. REST 端点（管理，`/api/v1/admin/*`） @@ -201,7 +201,7 @@ - `GET /admin/channels` 每条账号附 **`capabilities`**（可选 `[]string`，如 kook 的 `["friends","user_chats","channels","guilds","message_test"]`），供面板按平台渲染专属功能卡片。 - **KOOK「token 即用、其余自动获取」**（适配器契约，非端点）：填 KOOK 渠道时**只必填 `token`**（bot_token）。`base_url` 留空即用官方默认 `https://www.kookapp.cn/api/v3`；`receive_mode` 空默认 `ws`（WS 模式地址由 `GET /api/v3/gateway/index?compress=0` 自动获取，已接入）；`webhook_path` 空默认 `/webhook/kook`。高级值（改用 webhook、`webhook_verify_token`、`webhook_encrypt_key`，仅 webhook 模式需要）经 PUT 的 `config` JSON 传入（前端走「额外 config」，如 `{"receive_mode":"webhook","webhook_verify_token":"..."}`）。**Connect 拿到 token 后自动调用 `GET /api/v3/user/me`**（Bearer 鉴权）解析 bot 账号（id/username/nickname），存到 adapter 并随 `GET /admin/channels` 账号的 `botId`/`botUsername`/`botNickname` 显示；user/me 失败仅记日志、bot 身份为空，**不中断**连接/收发。token 只在 header / 写文件，**不打印、不进日志**。 - `PUT /admin/channels/{platform}/config`（owner-only，审计）body `{id?,token?,baseURL?,remoteURL?,listenAddr?,mode?,accessToken?,selfID?,config?,allowGroups?,denyGroups?,allowUsers?,denyUsers?,groupMsgPerMin?,msgPerMin?,maxMessages?,personaId?}` → `201 {data:{persisted:true,path,platform,accountId}}`。**真实持久化 + 热更新**（design/04 §3.1，无需重启）：读当前有效配置（channels.json→env 兜底）→ `UpsertChannel` 该平台账号 → 落盘 `channels.json`（本地、gitignore，0600）→ `bridge.Reload(platform)` 热替换该平台 adapter（进行中消息用旧、新消息用新）；token 写入文件但**永不在响应/日志回显**（GET 列表为掩码）。`platform` 必填；空配置回 `400`；未注入渠道配置服务回 `500 channels_unavailable`、写盘失败 `500 channel_persist_failed`、热重载失败 `500 channel_reload_failed`。`.env`（`CHANNELS_JSON` / `CHANNEL_*_TOKEN`）为兜底。 - - **`personaId`（v0.44，渠道级绑定）**：=该账号**答话时的角色 id**（design/22 §2 方案 b，取值来自 `GET /personas`）。**未知角色 id 回 `400 bad_request`**（不绑一个答不了话的角色），**省略/`""` = 不绑**（解绑，写盘时该账号不带 `personaId` 键）。**写盘即生效**：保存后 `Reload(platform)` 会重建该平台 adapter，新角色在下一批入站消息上生效（无需重启）。**它不是身份/权限输入**——谁能做什么仍由 channel 层 actor 元数据 + 白名单决定（design/11 §0）。**优先级见 §4.2「角色解析优先级」**：渠道绑定**只**作用于该账号的**主人私聊**，且仅当主会话自身未绑角色；群聊/陌生人会话**强制默认角色**。 + - **`personaId`（v0.44 新增，v0.45 起为三态）**：=该账号**答话时的角色 id**（design/22 §2 方案 b，取值来自 `GET /personas`）。**取值三态**：**键省略（或 `null`）= 保留该账号已存绑定**（不动）；**显式 `""` = 解绑**（写盘时该账号不带 `personaId` 键）；**非空 id = 绑定该角色**。**未知角色 id 回 `400 bad_request`**（不绑一个答不了话的角色；校验只针对**本次真的传了值**的情形，省略继承的已存绑定在写入时已校验过）。**为什么是三态**：面板只在主人**动过**角色下拉时才发这个键——若把「键不存在」也当成解绑，主人只改 token 就会**静默清掉**账号的角色绑定（两端语义相反即丢数据）。**写盘即生效**：保存后 `Reload(platform)` 会重建该平台 adapter，新角色在下一批入站消息上生效（无需重启）。**它不是身份/权限输入**——谁能做什么仍由 channel 层 actor 元数据 + 白名单决定（design/11 §0）。**优先级见 §4.2「角色解析优先级」**：渠道绑定**只**作用于该账号的**主人私聊**，且仅当主会话自身未绑角色；群聊/陌生人会话**强制默认角色**。 - `DELETE /admin/channels/{platform}/config?id={accountId}`（owner-only，审计）→ `200 {data:{persisted:true,path,platform,accountId}}`。删除该平台指定账号 → 落盘 `channels.json` → `bridge.Reload(platform)` 热更新（该账号 adapter 优雅关停）。`id` 必填；未注入服务回 `500 channels_unavailable`。 - `GET /admin/channels/{platform}/messages?limit=&before=`（owner/admin 读）→ `200 {data:[{id,sessionId,role,content,mode?,mood?,image?,platform?,deviceId?,senderName?,createdAt}]}`。**IM 适配器日志流**（design/16 §4.6 §4）：该平台**所有非主人私聊/群聊消息**（隔离会话 `sess-xxx`，非主会话 `web`）以消息流显示，`senderName` 标发送者昵称；未注入 `HistoryStore` 回 `501`。 - `POST /admin/plugins/install`（**owner-only + 审计 `plugin.install`**）body `{kind:"mcp",name,config:{transport:"stdio|http|sse",endpoint?|command?,args?,apiKey?,env?}}` → `200 {data:{installed:true,name,kind:"mcp",transport,toolCount}}`。**v1 只装 MCP server**（单二进制无法热装 Go 插件）：真实 connect+discover，把发现的工具**热注册**进 capability registry（下次模型请求即注入，无需重启）→ 落盘 `plugins.json`（本地、gitignore、0600，`apiKey` 写入但**永不回显/不落日志**）→ 持有 live server 供后续刷新/卸载。**同名重复安装 = 刷新**（先卸旧再装新，失败回滚旧）。校验：`name` 必填、`kind` 非 `mcp` 回 `400`、transport 非法/`stdio` 缺 `command`/`http|sse` 缺 `endpoint` 回 `400`；**连接/发现/注册失败回 `502 plugin_install_failed` 且不落盘**（诚实，不静默装一半）。未注入 plugin manager 回 `501 not_implemented`。 @@ -367,7 +367,8 @@ - 语音：上行 `voice_input`（base64 音频或已转写 text），下行 `voice_output`（合成音频 base64 + text）。 ## 5. 变更记录 -- `v0.44`（**渠道级选角 + 「谁说的」进模型上下文**，对客户端**新增两个可选字段**，向后兼容；design/22 批次 2 后端 = §2 裁决规则 + §4.4 + §7-3/§7-4/§7-5）：① **`PUT /api/v1/admin/channels/{platform}/config` body 新增可选 `personaId`**（渠道级绑定，design/22 §2 方案 b）：=该账号答话时的角色 id；**未知角色 `400 bad_request`**，**省略/`""` = 解绑**（写盘时该账号不带 `personaId` 键）；沿用既有「读当前配置 → `UpsertChannel` → 落盘 `channels.json`(0600) → `bridge.Reload(platform)` 热更新」路径，保存即生效（下一批入站消息），**无新端点**。② **`GET /api/v1/admin/channels` 每账号新增可选 `personaId`**（非敏感，不掩码），供面板回填。③ **角色解析优先级明确化（对客户端语义澄清，无形状变更）**：**会话绑定 > 渠道绑定 > 全局默认角色**；渠道绑定**只**作用于该账号的**主人私聊**且仅当主会话自身未绑角色（§7-3：IM 主人私聊镜像随主会话角色）；**群聊/陌生人隔离会话强制默认角色**（§7-4，更保守，仍只读）。④ **模型上下文新增角色归属标注（§7-5 的剩下一半）**：一回合上下文改为该会话**最近 20 条**持久化消息 + 本回合新消息（此前只带新消息本身），每条 assistant 历史消息按其说话角色出具 `[发言归属] 以下发言来自「<名字>」（），不是你说的。` 独立 system 标注 + 一条 `[会话角色] …` 当前角色声明；**消息正文一字不改**（标注只走 system 消息，design/11 §10），**对客户端透明**（无新字段）。前端无需改动即可继续工作；面板可选用 ① 让每个 IM 账号挂角色。⑤ **向后兼容**：纯新增可选字段；未配置 `personaId` 的账号与不读新字段的客户端行为不变（渠道绑定缺省 = 全局默认角色，与 v0.43 完全一致）。 +- `v0.45`（**渠道 `personaId` 三态修订：修「省略 = 解绑」会静默清空绑定**，对客户端**无字段/端点形状变更**，但**语义收紧**——按 v0.44 旧文实现的客户端若靠「省略 `personaId`」来解绑，必须改为**显式传 `""`**；design/22 §2 方案 b）：① **`PUT /api/v1/admin/channels/{platform}/config` 的 `personaId` 改为三态**：**键省略（或 JSON `null`）= 保留该账号已存绑定**（不动）；**显式 `""` = 解绑**（写盘时该账号不带 `personaId` 键）；**非空 id = 绑定该角色**；**未知角色 id 仍 `400 bad_request`**（且**只校验本次真的传了值**的情形——省略时继承的已存绑定在当初写入时已校验）。② **修的问题（数据安全）**：管理面板只在主人**动过**角色下拉时才发这个键（前端 `channelPersona.ts` / `channelPersonaRequest`），而 v0.44 后端把「键不存在」也当成解绑 ——**主人打开一个已绑角色的账号、不动下拉、只改 token 并保存 → 该账号的角色绑定被静默清空**（无提示、无审计可见的理由），两端语义相反。现在后端的「省略」与前端一致 = 保留。③ **实现**：请求 DTO 的该字段由 `string` 变 `*string`（指针区分「省略」与「显式空串」），应用逻辑在既有「读当前配置（channels.json）→ `UpsertChannel` → 落盘 → `bridge.Reload(platform)`」路径内按三态取值（省略时从**同平台同 id** 的已存账号继承，故保存一个账号不会动到同平台其它账号的绑定）；**无新端点、无新字段、无响应形状变更**。④ **`GET /api/v1/admin/channels` 每账号的 `personaId` 回显不变**（v0.44 新增，非敏感、不掩码），面板据此回填角色下拉。⑤ **测试**：省略 → 绑定保留、显式 `""` → 清除、非空 id → 绑定、未知 id → `400`（含「省略时改其它字段后读回确认」与「同平台多账号互不影响」），并把「省略 = 解绑」作为变异反向验证（该断言必须失败）。 +- `v0.44`（**渠道级选角 + 「谁说的」进模型上下文**，对客户端**新增两个可选字段**，向后兼容；design/22 批次 2 后端 = §2 裁决规则 + §4.4 + §7-3/§7-4/§7-5）：① **`PUT /api/v1/admin/channels/{platform}/config` body 新增可选 `personaId`**（渠道级绑定，design/22 §2 方案 b）：=该账号答话时的角色 id；**未知角色 `400 bad_request`**，~~**省略/`""` = 解绑**（写盘时该账号不带 `personaId` 键）~~（**本条的「省略」一项已由 v0.45 修订为「保留已存绑定」；只有显式 `""` 才解绑**，见上）；沿用既有「读当前配置 → `UpsertChannel` → 落盘 `channels.json`(0600) → `bridge.Reload(platform)` 热更新」路径，保存即生效（下一批入站消息），**无新端点**。② **`GET /api/v1/admin/channels` 每账号新增可选 `personaId`**（非敏感，不掩码），供面板回填。③ **角色解析优先级明确化（对客户端语义澄清，无形状变更）**：**会话绑定 > 渠道绑定 > 全局默认角色**；渠道绑定**只**作用于该账号的**主人私聊**且仅当主会话自身未绑角色（§7-3：IM 主人私聊镜像随主会话角色）；**群聊/陌生人隔离会话强制默认角色**（§7-4，更保守，仍只读）。④ **模型上下文新增角色归属标注（§7-5 的剩下一半）**：一回合上下文改为该会话**最近 20 条**持久化消息 + 本回合新消息（此前只带新消息本身），每条 assistant 历史消息按其说话角色出具 `[发言归属] 以下发言来自「<名字>」（），不是你说的。` 独立 system 标注 + 一条 `[会话角色] …` 当前角色声明；**消息正文一字不改**（标注只走 system 消息，design/11 §10），**对客户端透明**（无新字段）。前端无需改动即可继续工作；面板可选用 ① 让每个 IM 账号挂角色。⑤ **向后兼容**：纯新增可选字段；未配置 `personaId` 的账号与不读新字段的客户端行为不变（渠道绑定缺省 = 全局默认角色，与 v0.43 完全一致）。 - `v0.43`（**人格读侧合并批：admin 回读 `thinking` + 用户侧角色级 `mood`**，对客户端**新增两个可选字段**，向后兼容；design/01 §2/§2.1、design/04 §3.2）：① **`GET /api/v1/admin/personas` 每项新增可选 `thinking`**（`{budgetTokens,effort,showToUser,mode}`），与写侧同构、供面板回填编辑。**未配置的人格不带 `thinking` 键**（不是全零对象），与 `PUT` 的「省略 `thinking` = 保留已存」互为镜像。② **`showToUser` 三态原样回读**：**`null` = 未配置 → 保持现行为（thinking 帧照常推客户端）**，`true`/`false` = 显式开关；**服务端不把 `null` 拍平成 `false`**（否则面板会把「没配过」误显示为「关」，v0.41 的语义会被读侧丢掉）。③ **`GET /api/v1/personas` 每项新增 `mood`（角色级）**：= 该角色**当前情绪状态**（取值同 `GET /meta/enums` 的 `moods`；`""` = 暂无被跟踪情绪，不伪造）。**情绪跟随角色、与会话无关**——同一角色的所有会话看到同一 `mood`；客户端登录/加载角色目录时用它**播种心情徽标**（按角色 id 分桶），消除「刷新后徽标短暂为空」，随后仍以 WS `mood` 帧为准。**用户侧不暴露 `thinking`**（模型调用旋钮属管理侧）。④ **`effort` 的能力提示归服务端**：读端点只回读**已存配置**，**不承诺**该值在当前 provider 上生效——白名单校验/降级/WARN 仍在模型层（design/04 §3.2），客户端不得把「配了」呈现为「已启用」。⑤ **向后兼容**：纯新增字段；不读新字段的客户端行为逐字节不变（旧面板仍可编辑并保留已存 `thinking`）。 - `v0.42`（**多角色并存 + 按会话选角**，对客户端**新增可选字段/可选请求字段**，向后兼容；design/22 批次 1 = 后端会话级绑定）：① **`chat_sessions` 新增 `persona_id TEXT NOT NULL DEFAULT ''`、`chat_messages` 新增 `persona_id TEXT NOT NULL DEFAULT ''`**（同一迁移 **0011**，down 可逆，**不回填**）。`''` **= 继承全局默认角色**（`owner.yaml:defaultPersona`），所以**既有一切行升级后行为逐字节不变**，且默认角色改名后这些行跟着变（design/22 §4.7/§7-6）。② **`GET /api/v1/sessions` 每项新增可选 `personaId`**（=会话绑定角色）；**消息 DTO（REST `GET /sessions/{id}/messages`、WS `history` 回放、`message_persisted`/`message_edited` 广播）新增可选 `personaId`**（=**说话时的角色**，design/22 §7-5）。③ **`POST /api/v1/sessions` body 新增可选 `{personaId}`**（创建即绑定；未知角色 `400`；响应仅在非空时回显 `personaId`，故**省略时响应形状与 v0.41 完全一致**）；**`PATCH /api/v1/sessions/{id}` body 新增可选 `{personaId}`**（重新绑定，**显式 `""` 解绑**，未知角色 `400`；只回显变更字段）。④ **回合语义**：角色决定**系统提示词 / 记忆命名空间 / 情绪与好感度 / 思考预算**四者**同源**解析（design/22 §4.2），换角色**保留上下文**、**不动已存消息**，**下一回合**起生效；**记忆口径 = 保持 persona 隔离**（design/22 §3）：不同角色的会话互不浮现，同角色的多个会话互相可见。⑤ **请求级覆盖（每次发送临时指定角色）本批未实现**（design/22 §2 裁决规则 1 仍待批次 2/后续）；**渠道级绑定（IM 账号/群默认角色）本批未实现**（design/22 §4.4 批次 2），故 IM 路径仍按默认角色解析。⑥ **契约内新增内部读接口**：网关经窄接口类型断言读取 `history.Store.SessionPersona/SetSessionPersona/NewSubSessionFor`（注入的 HistoryStore **不支持**角色绑定时，`personaId` 写路径回 `501` 而非假成功）。 - `v0.41`（**人格 `thinking` 配置 + 推理旋钮按 provider 映射**，对客户端**新增一个可选请求字段**，向后兼容）：① `POST /admin/personas` / `PUT /admin/personas/{id}` body 新增可选 **`thinking`** 对象 `{budgetTokens?,effort?,showToUser?,mode?}`（design/01 §2.1），原样写入并持久化到 `configs/personas/.yaml` 的 `thinking` 节；非法 `mode`/负预算回 **`400 bad_request`**；**`PUT` 省略该对象时保留已存 `thinking`**（旧面板/脚本不会抹掉它）；该字段只描述**呈现/资源**，不构成能力边界。② **`showToUser` 语义**：**缺省 = 照常把 `thinking.*` 流帧推给客户端**（现行为），显式 `false` = 仅落库/剥离语义不变、**不推帧**（展示与落库解耦）。③ **服务端推理映射**（对客户端透明，仅影响上游请求）：Anthropic 系在 persona 配置 `budgetTokens`/`mode: on` 时发 `thinking:{type:"enabled",budget_tokens:N}`，并**保证 `budget_tokens + 正文留量 ≤ max_tokens`**（不足则抬高 `max_tokens` 并记 WARN，绝不把正文吃空）；OpenAI 兼容系按**识别到的厂商白名单**校验 `reasoning_effort`（OpenAI 系 `none…max`、DeepSeek 仅 `low/high/max`），非法值**降级到最近一档并记 WARN**；**未知 provider/未识别模型、以及 Qwen 的 `enable_thinking` 硬开关（协议无统一 wire 形状）** 一律记 **WARN（含 provider/字段/取值）并保持请求不变**，绝不静默无效。详见 design/04 §3.2。 diff --git a/internal/services/gateway/admin_channel_persona_test.go b/internal/services/gateway/admin_channel_persona_test.go index 651b767..dd1988a 100644 --- a/internal/services/gateway/admin_channel_persona_test.go +++ b/internal/services/gateway/admin_channel_persona_test.go @@ -1,8 +1,11 @@ package gateway import ( + "bytes" "encoding/json" + "io" "net/http" + "net/http/httptest" "os" "path/filepath" "strings" @@ -18,10 +21,96 @@ import ( // existing read-modify-write + bridge.Reload path, an unknown role is refused // instead of binding a role that cannot answer, and the list endpoint reports the // binding back. +// +// personaId is three-state on the wire, because the panel sends the key only when +// the owner actually touched the role select: +// +// key absent → keep the stored binding +// explicit "" → clear the binding +// non-empty id → bind that role (unknown id → 400) +// +// The absent-key case is the one that silently dropped bindings, so it is asserted +// with a literal JSON body: a map[string]any would still make the assertion pass if +// the DTO had picked up a value from somewhere else. -// TestAdminSaveChannelConfigPersonaID saves a channel role, asserts it reached -// channels.json and the list wire shape, then clears it. -func TestAdminSaveChannelConfigPersonaID(t *testing.T) { +// putChannelConfigRaw posts a literal JSON body so a test can pin the exact wire +// shape (notably: no personaId key at all). +func putChannelConfigRaw(t *testing.T, ts *httptest.Server, platform, raw, token string) (*http.Response, []byte) { + t.Helper() + req, err := http.NewRequest(http.MethodPut, + ts.URL+"/api/v1/admin/channels/"+platform+"/config", bytes.NewReader([]byte(raw))) + if err != nil { + t.Fatalf("new request: %v", err) + } + req.Header.Set("Authorization", "Bearer "+token) + req.Header.Set("Content-Type", "application/json") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("do request: %v", err) + } + defer resp.Body.Close() + b, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read response: %v", err) + } + if resp.StatusCode != http.StatusCreated { + t.Fatalf("PUT %s status = %d, want 201 (body=%s)", platform, resp.StatusCode, b) + } + return resp, b +} + +// storedPersonaID reads the binding channels.json holds for one account. +func storedPersonaID(t *testing.T, path, platform, accountID string) string { + t.Helper() + accounts, warn := runtimeconfig.LoadChannels(path, nil) + if warn != nil { + t.Fatalf("load channels warn = %v", warn) + } + for _, acct := range accounts[platform] { + if acct.ID == accountID { + return acct.PersonaID + } + } + t.Fatalf("account %s/%s not found in channels.json: %+v", platform, accountID, accounts[platform]) + return "" +} + +// listedPersonaID is the personaId GET /admin/channels reports for one account, i.e. +// what the panel reads back to render the role select. +func listedPersonaID(t *testing.T, ts *httptest.Server, token, platform, accountID string) (string, []byte) { + t.Helper() + resp, body := doReq(t, http.MethodGet, ts.URL+"/api/v1/admin/channels", nil, token) + if resp.StatusCode != http.StatusOK { + t.Fatalf("get status = %d, want 200 (body=%s)", resp.StatusCode, body) + } + var listed []struct { + Name string `json:"name"` + Accounts []struct { + ID string `json:"id"` + PersonaID string `json:"personaId"` + } `json:"accounts"` + } + if err := json.Unmarshal(decodeBody(t, body)["data"], &listed); err != nil { + t.Fatalf("decode channels list: %v (body=%s)", err, body) + } + for _, entry := range listed { + if entry.Name != platform { + continue + } + for _, acct := range entry.Accounts { + if acct.ID == accountID { + return acct.PersonaID, body + } + } + } + t.Fatalf("account %s/%s not in GET /admin/channels (body=%s)", platform, accountID, body) + return "", body +} + +// TestAdminSaveChannelConfigPersonaIDBindsAndEchoes binds a channel role and +// asserts it reached channels.json, that the list endpoint echoes it back for the +// panel to render, and that the hot-reload path ran. +func TestAdminSaveChannelConfigPersonaIDBindsAndEchoes(t *testing.T) { t.Parallel() path := filepath.Join(t.TempDir(), "channels.json") store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{}} @@ -33,51 +122,99 @@ func TestAdminSaveChannelConfigPersonaID(t *testing.T) { if resp.StatusCode != http.StatusCreated { t.Fatalf("save status = %d, want 201 (body=%s)", resp.StatusCode, body) } - accounts, warn := runtimeconfig.LoadChannels(path, nil) - if warn != nil { - t.Fatalf("load channels warn = %v", warn) - } - if len(accounts["telegram"]) != 1 || accounts["telegram"][0].PersonaID != "yuki" { - t.Fatalf("channels.json telegram = %+v, want personaId yuki", accounts["telegram"]) + if got := storedPersonaID(t, path, "telegram", "conf0"); got != "yuki" { + t.Fatalf("channels.json telegram.conf0 personaId = %q, want yuki", got) } if len(store.reloaded) != 1 || store.reloaded[0] != "telegram" { t.Fatalf("reloaded = %v, want [telegram] (the panel's hot-reload path)", store.reloaded) } - // The list endpoint reports the binding so the panel can render it. - var listed []struct { - Name string `json:"name"` - Accounts []struct { - ID string `json:"id"` - PersonaID string `json:"personaId"` - } `json:"accounts"` + echoed, listBody := listedPersonaID(t, ts, tok, "telegram", "conf0") + if echoed != "yuki" { + t.Fatalf("GET /admin/channels telegram.conf0 personaId = %q, want yuki (body=%s)", echoed, listBody) } - _, gBody := doReq(t, http.MethodGet, ts.URL+"/api/v1/admin/channels", nil, tok) - if err := json.Unmarshal(decodeBody(t, gBody)["data"], &listed); err != nil { - t.Fatalf("decode channels list: %v (body=%s)", err, gBody) + t.Logf("GET /api/v1/admin/channels echoes the binding: %s", listBody) +} + +// TestAdminSaveChannelConfigPersonaIDKeptWhenOmitted is the regression for the +// silent data loss: the panel omits personaId unless the owner touched the role +// select, so saving another field must leave the stored binding alone. +func TestAdminSaveChannelConfigPersonaIDKeptWhenOmitted(t *testing.T) { + t.Parallel() + path := filepath.Join(t.TempDir(), "channels.json") + store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{ + "telegram": {{ID: "conf0", Config: map[string]any{"bot_token": "old-tok"}, PersonaID: "yuki"}}, + }} + _, ts, jwt := buildEnv(t, cfgDefault(), Deps{Channels: store, Personas: personaTestRegistry(t)}) + tok := signAny(t, jwt, "acct", auth.RoleOwner) + + // The wire body carries the new token and no personaId key at all — exactly + // what an untouched role select produces. + _, body := putChannelConfigRaw(t, ts, + "telegram", `{"id":"conf0","token":"new-tok","groupMsgPerMin":5}`, tok) + + if got := storedPersonaID(t, path, "telegram", "conf0"); got != "yuki" { + t.Fatalf("omitted personaId cleared the binding: channels.json personaId = %q, want yuki (resp=%s)", got, body) } - var accts []struct { - ID string `json:"id"` - PersonaID string `json:"personaId"` + accounts, _ := runtimeconfig.LoadChannels(path, nil) + if accounts["telegram"][0].Config["bot_token"] != "new-tok" || accounts["telegram"][0].GroupMsgPerMin != 5 { + t.Fatalf("the other fields were not saved: %+v", accounts["telegram"][0]) } - for _, entry := range listed { - if entry.Name == "telegram" { - accts = entry.Accounts - } - } - if len(accts) != 1 || accts[0].PersonaID != "yuki" { - t.Fatalf("GET /admin/channels telegram accounts = %+v, want personaId yuki (body=%s)", accts, gBody) + if echoed, listBody := listedPersonaID(t, ts, tok, "telegram", "conf0"); echoed != "yuki" { + t.Fatalf("GET /admin/channels personaId = %q, want yuki (body=%s)", echoed, listBody) } - // Saving the same account without a personaId unbinds it. - resp, body = doReq(t, http.MethodPut, ts.URL+"/api/v1/admin/channels/telegram/config", - map[string]any{"id": "conf0", "token": "bot-tok"}, tok) - if resp.StatusCode != http.StatusCreated { - t.Fatalf("unbind status = %d, want 201 (body=%s)", resp.StatusCode, body) + // An explicit JSON null is the same as an absent key: the DTO cannot tell them + // apart, and neither means "clear". + putChannelConfigRaw(t, ts, "telegram", `{"id":"conf0","token":"newer-tok","personaId":null}`, tok) + if got := storedPersonaID(t, path, "telegram", "conf0"); got != "yuki" { + t.Fatalf("null personaId cleared the binding: personaId = %q, want yuki", got) } - accounts, _ = runtimeconfig.LoadChannels(path, nil) - if len(accounts["telegram"]) != 1 || accounts["telegram"][0].PersonaID != "" { - t.Fatalf("after unbind = %+v, want personaId cleared", accounts["telegram"]) +} + +// TestAdminSaveChannelConfigPersonaIDClearedOnExplicitEmpty asserts that clearing +// the binding still works, but only when the panel says so with an explicit "". +func TestAdminSaveChannelConfigPersonaIDClearedOnExplicitEmpty(t *testing.T) { + t.Parallel() + path := filepath.Join(t.TempDir(), "channels.json") + store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{ + "telegram": {{ID: "conf0", Config: map[string]any{"bot_token": "old-tok"}, PersonaID: "yuki"}}, + }} + _, ts, jwt := buildEnv(t, cfgDefault(), Deps{Channels: store, Personas: personaTestRegistry(t)}) + tok := signAny(t, jwt, "acct", auth.RoleOwner) + + putChannelConfigRaw(t, ts, "telegram", `{"id":"conf0","token":"new-tok","personaId":""}`, tok) + + if got := storedPersonaID(t, path, "telegram", "conf0"); got != "" { + t.Fatalf("explicit \"\" personaId = %q, want the binding cleared", got) + } + if echoed, listBody := listedPersonaID(t, ts, tok, "telegram", "conf0"); echoed != "" { + t.Fatalf("GET /admin/channels personaId = %q, want empty (body=%s)", echoed, listBody) + } +} + +// TestAdminSaveChannelConfigPersonaIDKeptPerAccount pins the read-modify-write to +// the account being saved: an omitted personaId on one account must inherit that +// account's binding, not a sibling's. +func TestAdminSaveChannelConfigPersonaIDKeptPerAccount(t *testing.T) { + t.Parallel() + path := filepath.Join(t.TempDir(), "channels.json") + store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{ + "telegram": { + {ID: "conf0", Config: map[string]any{"bot_token": "a"}, PersonaID: "yuki"}, + {ID: "conf1", Config: map[string]any{"bot_token": "b"}}, + }, + }} + _, ts, jwt := buildEnv(t, cfgDefault(), Deps{Channels: store, Personas: personaTestRegistry(t)}) + tok := signAny(t, jwt, "acct", auth.RoleOwner) + + putChannelConfigRaw(t, ts, "telegram", `{"id":"conf1","token":"b2"}`, tok) + + if got := storedPersonaID(t, path, "telegram", "conf0"); got != "yuki" { + t.Fatalf("sibling account conf0 personaId = %q, want yuki untouched", got) + } + if got := storedPersonaID(t, path, "telegram", "conf1"); got != "" { + t.Fatalf("conf1 personaId = %q, want empty (it never had a binding)", got) } } @@ -87,7 +224,9 @@ func TestAdminSaveChannelConfigPersonaID(t *testing.T) { func TestAdminSaveChannelConfigRejectsUnknownPersonaID(t *testing.T) { t.Parallel() path := filepath.Join(t.TempDir(), "channels.json") - store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{}} + store := &stubChannelConfigStore{path: path, accounts: map[string][]config.ChannelAccount{ + "telegram": {{ID: "conf0", Config: map[string]any{"bot_token": "old-tok"}, PersonaID: "yuki"}}, + }} _, ts, jwt := buildEnv(t, cfgDefault(), Deps{Channels: store, Personas: personaTestRegistry(t)}) tok := signAny(t, jwt, "acct", auth.RoleOwner) diff --git a/internal/services/gateway/admin_write_channel_config.go b/internal/services/gateway/admin_write_channel_config.go index 699e7b6..b5677e6 100644 --- a/internal/services/gateway/admin_write_channel_config.go +++ b/internal/services/gateway/admin_write_channel_config.go @@ -75,17 +75,22 @@ type channelConfigRequest struct { GroupMsgPerMin int `json:"groupMsgPerMin"` MsgPerMin int `json:"msgPerMin"` MaxMessages int `json:"maxMessages"` - // PersonaID is the role this account answers as (design/22 §2 方案 b). An empty - // value leaves the account unbound (the session binding or the global default - // applies); an unknown id is refused with 400 so a save can never bind a role - // that cannot answer. - PersonaID string `json:"personaId"` + // PersonaID is the role this account answers as (design/22 §2 方案 b). It is a + // pointer because the wire contract is three-state and a plain string cannot + // tell an absent key from an explicit "": absent keeps the stored binding, + // "" clears it, a non-empty id binds it. An unknown id is refused with 400 so a + // save can never bind a role that cannot answer. + PersonaID *string `json:"personaId"` } // adminSaveChannelConfig persists one account's config for a platform into // channels.json (Upsert by id) and hot-reloads that platform's adapters. The // response carries the channels.json path and the account id so the panel can // address the saved account (for a later edit/delete). +// +// personaId is three-state (see channelPersonaBinding): the panel only sends the +// key when the owner touched the role select, so an absent key must keep the +// stored binding rather than silently unbind the account. func (s *Server) adminSaveChannelConfig(w http.ResponseWriter, r *http.Request) { platform := strings.TrimSpace(r.PathValue("platform")) if platform == "" { @@ -104,11 +109,15 @@ func (s *Server) adminSaveChannelConfig(w http.ResponseWriter, r *http.Request) return } // A channel-level role must exist in the persona library, or the account would - // be bound to a role that cannot answer (design/22 §2 方案 b). An omitted/empty - // personaId is always accepted: it means "no channel binding". - if !s.requireKnownPersona(w, req.PersonaID) { - s.auditClaims(r, "channel.save", "error") - return + // be bound to a role that cannot answer (design/22 §2 方案 b). Only a value the + // panel actually sent is validated; an absent key (nil) inherits the stored + // binding, which was validated when it was written. An explicit "" is always + // accepted: it means "clear the channel binding". + if req.PersonaID != nil { + if !s.requireKnownPersona(w, strings.TrimSpace(*req.PersonaID)) { + s.auditClaims(r, "channel.save", "error") + return + } } cfg := channelConfigFromRequest(req) // weixin-ilink may be saved with an empty config: the panel creates the account @@ -131,7 +140,7 @@ func (s *Server) adminSaveChannelConfig(w http.ResponseWriter, r *http.Request) GroupMsgPerMin: req.GroupMsgPerMin, MsgPerMin: req.MsgPerMin, MaxMessages: req.MaxMessages, - PersonaID: strings.TrimSpace(req.PersonaID), + PersonaID: channelPersonaBinding(req.PersonaID, existingChannelAccount(accounts[platform], req.ID)), }) if err := runtimeconfig.SaveChannels(s.channels.Path(), updated); err != nil { s.auditClaims(r, "channel.save", "error") @@ -244,6 +253,41 @@ func channelConfigFromRequest(req channelConfigRequest) map[string]any { return cfg } +// channelPersonaBinding resolves the three-state personaId contract of +// PUT /admin/channels/{platform}/config: +// +// - the key is absent (nil) → keep the account's stored binding; +// - the key is an explicit "" → clear the binding (an explicit unbind); +// - the key is a non-empty id → bind that role. +// +// The panel omits the key unless the owner touched the role select, so resolving +// an absent key from the stored account is what keeps a plain config edit from +// silently dropping a binding. A save that creates a new account (no stored +// account to inherit from) resolves to "no binding". +func channelPersonaBinding(sent *string, stored config.ChannelAccount) string { + if sent == nil { + return strings.TrimSpace(stored.PersonaID) + } + return strings.TrimSpace(*sent) +} + +// existingChannelAccount returns the stored account a save would replace (same +// platform, same id) so the write can inherit the fields the panel did not send. +// An empty id means the save creates a new account, so there is nothing to +// inherit. +func existingChannelAccount(list []config.ChannelAccount, id string) config.ChannelAccount { + id = strings.TrimSpace(id) + if id == "" { + return config.ChannelAccount{} + } + for _, acct := range list { + if acct.ID == id { + return acct + } + } + return config.ChannelAccount{} +} + // savedAccountID returns the account id that a save produced — the caller-supplied // one when present, otherwise the id runtimeconfig.Upsert generated (which it // appends last for a brand-new account).