Skip to content

fix(cli): honor channel approval mode without webhooks - #15754

Merged
qqqys merged 1 commit into
QwenLM:mainfrom
qqqys:fix/channel-approval-mode
Oct 28, 2026
Merged

qqqys merged 1 commit into
QwenLM:mainfrom
qqqys:fix/channel-approval-mode

Conversation

@qqqys

@qqqys qqqys commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Daemon-managed channels now honor their configured approvalMode even when no webhook sources are configured, matching standalone channel startup. Regression coverage checks all five supported modes, omitted configuration, and channels with different modes sharing a worker.

Why it's needed

A channel configured with approvalMode: "yolo" could still request tool approval when started through qwen serve --channel: the value passed configuration validation but was omitted when the channel had no webhooks. The documented setting applies to every session on the channel, so requiring webhook configuration made the two startup paths inconsistent.

Reviewer Test Plan

How to verify

  • Start a channel with an explicit approval mode and no webhook sources through qwen serve --channel. Confirm a fresh session receives that mode, matching qwen channel start.
  • Restart the worker and restore the channel route. Confirm the configured mode is also forwarded during restoration.
  • Run channels with different modes together, including a webhook-enabled channel and a channel without an explicit mode. Confirm each retains its own configuration and the omitted mode still uses the existing default behavior.

Evidence (Before & After)

  • Before: the six updated regression cases fail against the unchanged upstream implementation because ordinary channels do not register their configured modes.
  • After: all 578 tests across worker startup, standalone startup, configuration parsing, session routing, and the daemon bridge pass.
  • Independent local integration verification passes for seven new-session requests and seven restores after a worker restart, using the real compiled worker/router/bridge and mocked IM/SDK network boundaries.
  • Build, bundle, full repository typecheck, changed-file lint/format checks, and the commit hook pass. This is local validation; real DingTalk approval-card E2E was not performed.

Tested on

OS Status
macOS ✅
Windows ⚠️ Not tested locally
Linux ⚠️ Not tested locally

Environment (optional)

Node.js 24.14.1; pnpm 11.24.0. Tests use isolated local state and mocked IM/network boundaries.

Risk & Scope

  • Main risk or tradeoff: an explicitly configured approval mode now takes effect for daemon-managed channels that previously ignored it, including yolo.
  • Not validated / out of scope: real DingTalk delivery, approval-card rendering, and production deployment.
  • Breaking changes / migration notes: no configuration schema change. Channels without an explicit mode keep existing default behavior.

Linked Issues

No linked issue.

中文说明

此 PR 的改动

daemon 管理的 channel 现在会应用配置的 approvalMode,不再要求配置 webhook,与 standalone channel 启动行为一致。回归测试覆盖全部五种支持的模式、未配置模式,以及共享同一个 worker 的不同审批模式 channel。

修改原因

配置了 approvalMode: "yolo" 的 channel 经 qwen serve --channel 启动后,仍可能请求工具审批:该值通过了配置校验,却在没有 webhook 时未被传递。文档约定该设置适用于 channel 的每个会话,因此以 webhook 配置为生效条件导致两个启动入口行为不一致。

审阅者测试计划

如何验证

  • 为 channel 显式配置审批模式但不配置 webhook,通过 qwen serve --channel 启动。确认新建会话收到该模式,与 qwen channel start 一致。
  • 重启 worker 并恢复 channel 路由,确认恢复时同样传递配置的模式。
  • 同时运行不同审批模式的 channel,包含启用 webhook 的 channel 和未显式配置模式的 channel。确认各自保留自己的配置,未设置模式时仍使用已有的默认行为。

修改前后证据

  • 修改前:更新后的六项回归用例在未修改的上游实现上失败,原因是普通 channel 未注册配置的模式。
  • 修改后:worker 启动、standalone 启动、配置解析、会话路由和 daemon bridge 的全部 578 项测试通过。
  • 独立本地集成验证通过:使用真实编译后的 worker/router/bridge,并模拟 IM/SDK 网络边界,七个新建会话请求及 worker 重启后的七个恢复请求均正确。
  • 构建、打包、全仓类型检查、修改文件的 lint/格式检查和提交钩子均通过。这些属于本地验证,未执行真实钉钉审批卡片 E2E。

测试平台

操作系统 状态
macOS ✅
Windows ⚠️ 未在本地测试
Linux ⚠️ 未在本地测试

环境

Node.js 24.14.1;pnpm 11.24.0。测试使用隔离的本地状态,并模拟 IM 和网络边界。

风险与范围

  • 主要风险或权衡:原先被 daemon 管理的 channel 忽略的显式审批模式,现在会真正生效,包括 yolo。
  • 未验证或不在范围内:真实钉钉消息投递、审批卡片展示和生产部署。
  • 破坏性变更或迁移说明:配置结构不变;未显式设置审批模式的 channel 保持已有默认行为。

关联 Issue

无关联 Issue。

@qqqys

qqqys commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Verification report for e3e27ca9e20e2d1507dd9c87fe7ba2bb9197e318:

  • Red/green regression: the six updated expectations failed on the unchanged upstream implementation; all pass with this fix.
  • Full affected test files: 288/288 CLI tests and 290/290 router/daemon bridge tests passed (578 total).
  • Independent local integration: seven channels shared one real compiled worker/router/bridge. All seven new-session requests and all seven resumes after worker restart carried the correct mode. This covered all five supported modes without webhooks, omitted mode staying absent, and webhook yolo, with real route persistence and no cross-channel mode leakage.
  • npm run build, npm run bundle, npm run typecheck, changed-file ESLint/Prettier, git diff --check, and the normal pre-commit hook passed on macOS with Node.js 24.14.1 and pnpm 11.24.0.

Boundary: the independent integration harness mocked configuration loading/parsing, IM transport construction, event-relay registration, and the SDK network boundary. It verifies request propagation, not HTTP/ACP acceptance or real DingTalk transcript/card behavior. No real IM messages were sent and no production service was changed. Remote CI is separate from these local results.


该提交的验证结果:六项更新后的回归预期在原上游实现上失败,修复后全部通过;相关完整测试文件共 578/578 通过。独立本地集成验证使用真实编译后的 worker/router/bridge,共享 router 的七个 channel 在新建会话和重启恢复时各有 7/7 请求正确,覆盖无 webhook 的五种模式、未配置模式和 webhook yolo。构建、打包、全仓类型检查、lint/格式检查、diff 检查及正常提交钩子均通过。

独立集成验证模拟了配置载入/解析、IM transport、事件 relay 注册和 SDK 网络边界,确认的是请求传递;未验证 HTTP/ACP 接受结果或真实钉钉 transcript/审批卡片。未发送真实 IM 消息或更改生产服务;远端 CI 结果与本地验证分别记录。

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No findings — this is a clean single-line fix.

Change: Removes the config["webhooks"] guard before setChannelApprovalMode so that channel approval mode is always honored, not just when a webhook is configured.

Testing: The parameterized it.each covers all five approval-mode values and verifies both webhook-enabled (telegram) and non-webhook (feishu) channels. The original test still covers the undefined-approvalMode path.

No correctness, security, or regression concerns.

@qwen-code-dev-bot qwen-code-dev-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Final gate review at head e3e27ca (re-read immediately before submitting).

  • No prior review rounds or unresolved threads on this PR.
  • Required CI per the repository ruleset (Lint & Static (ubuntu-latest, Node 22.x)): COMPLETED/SUCCESS at this head. Non-required lanes (precheck, Bun acceptance) were still queued at submission time.
  • Independent check found no Critical. The one-line production change matches the standalone path exactly (start.ts registers setChannelApprovalMode(name, config.approvalMode) unconditionally at both call sites). undefined semantics are safe: the router deletes the map entry, and sessionOptions() — the single read funnel for fresh and restored routes — omits the field, preserving default behavior; registration also runs before restoreRoutes(), so restoration is covered. The worker's session-request builder still forwards only a truthy approvalMode, so no undefined leaks to the wire.
  • Test updates pin the exact call sequence (including ['feishu', undefined]) and parametrize all five supported modes across webhook/no-webhook channels.

@qqqys
qqqys merged commit 7ffb1e2 into QwenLM:main Oct 28, 2026
67 of 68 checks passed
@qwen-code-review-bot

qwen-code-review-bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-review-bot

Copy link
Copy Markdown
Collaborator

One thing up front: this run started before the merge and the PR landed while it was in flight (merge commit 7ffb1e2, 04:30 UTC). So this pass is a post-merge review record — no gate action, no approval, nothing to block.

Template looks good ✓

Problem: real, and evidenced in the code rather than by a user report. The daemon worker registered a channel's approvalMode only when config['webhooks'] was present, while both standalone paths (channel/start.ts — single-channel and shared-bridge) and the DWS channel register it unconditionally. The pre-existing test even pinned the gated behaviour with expect(mockRouterSetChannelApprovalMode).not.toHaveBeenCalled(), so this was an asserted difference between the two startup paths, not a stray branch. Net effect: under qwen serve --channel, a channel that set approvalMode but no webhooks had the value validated by config-utils.ts and then dropped on the floor.

Direction: aligned. The channel-adapter developer doc describes the channel approval mode as the fallback policy for permission requests with no webhook precondition, and the daemon accepts exactly the value set the channel parser produces — APPROVAL_MODES (plan/default/auto-edit/auto/yolo) and the SDK's PERMISSION_MODES are identical sets — so nothing that was previously never forwarded can now be rejected server-side.

Worth a release note, though: this is permission gating. A daemon-managed channel configured with approvalMode: "yolo" (or auto) and no webhooks goes from "mode ignored, daemon default applies" to "mode honored, tool calls auto-approved". That is the configured intent and the PR says so under Risk & Scope, but anyone upgrading with such a config will notice the change.

Size: not a core path (packages/cli/src/commands/channel/**). 4 production lines (1 added / 3 removed), 61 test lines (33 / 28). No Stage 0 escalation.

Approach: minimal — removing the guard is the fix, and I'd cut nothing. Expanding one regression case into a parameterized run over all five modes is the only extra scope and it earns its place.

Risk: no high-risk path match (Stage 1e).

Non-blocking follow-up, and pre-existing rather than introduced here: docs/developers/daemon/15-channel-adapters.md still states that ChannelConfig.approvalMode "is declared but not yet read" and that AcpBridge.requestPermission auto-approves every request. Neither holds today — the router reads the mode and the bridge relays real permission requests — and this change makes that note more misleading than it already was.

Moving on to code review. 🔍

中文说明

先说明一点:本次运行在合并之前就已启动,PR 在运行过程中已被合并(合并提交 7ffb1e2,UTC 04:30)。因此这一轮是合并后的审查记录——不执行任何门禁动作、不批准、也不阻塞。

模板完整 ✓

问题: 真实存在,且由代码本身证明,而非用户报告。daemon worker 仅在 config['webhooks'] 存在时才注册 channel 的 approvalMode,而两条 standalone 路径(channel/start.ts 的单 channel 与共享 bridge)以及 DWS channel 都是无条件注册。原有测试甚至用 expect(mockRouterSetChannelApprovalMode).not.toHaveBeenCalled() 固化了这个带条件的行为,所以这是两个启动入口之间被断言过的差异,不是某个分支的疏漏。结果是:通过 qwen serve --channel 启动时,配置了 approvalMode 但没有 webhook 的 channel,其取值先被 config-utils.ts 校验通过,随后被丢弃。

方向: 一致。channel adapter 的开发文档把 channel 审批模式描述为权限请求的兜底策略,并没有以 webhook 为前提;daemon 接受的取值集合与 channel 解析器产出的集合完全一致——APPROVAL_MODES(plan/default/auto-edit/auto/yolo)与 SDK 的 PERMISSION_MODES 是同一组值——因此此前从未下发的取值不会在服务端被拒绝。

不过值得写进 release note:这属于权限管控。配置了 approvalMode: "yolo"(或 auto)且没有 webhook 的 daemon channel,行为会从"模式被忽略、走 daemon 默认"变成"模式生效、工具调用自动批准"。这是配置本来的意图,PR 也在 Risk & Scope 中写明了,但升级后带着这类配置的用户会感知到变化。

规模: 未触及核心路径(packages/cli/src/commands/channel/**)。生产代码 4 行(新增 1 / 删除 3),测试 61 行(新增 33 / 删除 28)。不触发 Stage 0 升级。

方案: 已是最小改动——删掉这个判断本身就是修复,没有可裁剪的部分。把单个回归用例扩展为覆盖全部五种模式的参数化用例是唯一的额外范围,且是必要的。

风险: 未命中高风险路径(Stage 1e)。

一个非阻塞的后续项,且是既有问题而非本 PR 引入:docs/developers/daemon/15-channel-adapters.md 仍写着 ChannelConfig.approvalMode"已声明但尚未被读取",以及 AcpBridge.requestPermission 会自动批准所有请求。两点如今都不成立——router 会读取该模式,bridge 会真实转发权限请求——而这次改动让这段说明比原先更容易误导人。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-2026-09-02

Reviewed at e3e27ca9e20e2d1507dd9c87fe7ba2bb9197e318 · re-run with @qwen-code /triage

@qwen-code-review-bot

Copy link
Copy Markdown
Collaborator

Code review

No critical findings. The production change is one deleted condition, and the interesting question is what falls out of calling the setter unconditionally — so that is what I traced.

undefined is safe. SessionRouter.setChannelApprovalMode (packages/channels/base/src/SessionRouter.ts:342) sets on a truthy value and deletes otherwise, and sessionOptions() only spreads approvalMode into the session request when it is truthy. A channel with no configured mode therefore behaves exactly as before — no undefined leaking into the create request, no key written with an empty value. The map is also fresh at worker startup, so the delete branch is a no-op there rather than a state change.

The restore claim holds. Registration happens before restoreRoutes() in the same startup block, and the router's restore path forwards the mode to loadSession — SessionRouter.test.ts:2121 pins exactly that ({ approvalMode: 'yolo', sourceId: 'ch' }). So the "restart the worker and the mode still applies" item in the test plan is covered by existing tests, not just asserted in the description.

No new server-side rejection surface. This was the one non-obvious risk: values that were previously never forwarded for non-webhook channels now reach the daemon. Channel config validates against APPROVAL_MODES from packages/core/src/config/approval-mode.ts, and the daemon types the field as DaemonApprovalMode = PermissionMode, whose PERMISSION_MODES list is the same five literals. Identical domains, so nothing can newly fail validation. The SDK forwards the field on create, resume, and worktree-reset alike (DaemonClient.ts:3427, :3808, :4296).

The tests are load-bearing, not decorative. Under the old guard the selects all configured channels in one shared router case fails outright — its telegram channel has approvalMode: 'yolo' and no webhooks, so the assertion on the exact call list [['telegram','yolo'], ['feishu', undefined]] cannot pass. The parameterized case does the same for each of the five modes on a non-webhook channel alongside a webhook channel. Reverting the one-line fix reddens both.

Two non-blocking notes:

  • The it.each varies only the non-webhook channel's mode; the webhook channel stays at yolo throughout. Coverage is fine given the adjacent test pins the omitted-mode case, but a webhook channel with a non-yolo mode is the one combination nothing asserts.
  • The tests pin registration at the router mock boundary. They do not observe a daemon-side session actually starting in the configured mode — that hop is covered by the router/SDK types and existing router tests, not by this PR.

Test evidence

This was an unattended CI run, so per the gate's rules I executed nothing from the PR — no build, no test run, no checkout of the head. The evidence below is the PR's own CI, read through the API at the reviewed commit, plus the static trace above. 91 check-runs on e3e27ca, zero failures.

The cancelled lanes (review-pr, precheck-pr, authorize, resolve-pr, publish-resolution, and friends) are bot orchestration jobs that were cancelled when the PR merged mid-run — not PR-caused, and not product CI. triage is this run and was still in flight. macOS and Windows unit lanes were skipped by the matrix, so cross-platform coverage rests on the ubuntu lane; the change is plain control flow with no platform dependency, so I don't read that as a gap.

The author's comment on this thread reports 578 local tests passing and a local integration harness covering seven new sessions and seven restores. That is the author's own claim on macOS — attributed, not re-run here, and not counted as evidence. The independent signal is the green ubuntu unit lane, which does include daemon-worker.test.ts.

Check Conclusion
Lint & Static (ubuntu-latest, Node 22.x) success
Test (ubuntu-latest, Node 22.x) success
Test (macos-latest, Node 22.x) skipped
Test (windows-latest, Node 22.x) skipped
Integration Tests (no-AK, No Sandbox) success
Integration Tests (CLI, No Sandbox) skipped
web-shell E2E Smoke (ubuntu-latest, Node 22.x) success
Managed Agent gate success
OpenTUI no-flicker gate success
TUI parity snapshots (ink vs opentui) success
Check TUI impact success
Classify PR success
triage (this run) in_progress

Sandboxed verification would settle the remaining gap: @qwen-code /verify — that a daemon-managed channel session actually starts in the configured mode, and that a yolo channel with no webhooks stops surfacing approval cards, is not observable from this diff; the new tests assert registration on a router mock, and they would still pass if the daemon ignored the forwarded field. The author is a fork contributor, so this would be a sponsored run: a maintainer's @qwen-code /verify comment approves the head it was written against, and that run carries a pre-execution risk screen and a full workspace wipe. Read the resulting report with the same skepticism as the fork's own CI logs — the code under verification is adversarial input and can shape what the report says, even though the sandbox bounds what it can do. (/tmux is not available here for the same reason.) Post-merge this is optional; it is worth it only if the release note needs a confirmed end-to-end statement.

中文说明

代码审查

没有发现严重问题。生产代码只删掉了一个条件判断,真正值得追问的是"无条件调用这个 setter 会带来什么",所以我沿着这条链读了一遍。

传 undefined 是安全的。 SessionRouter.setChannelApprovalMode(packages/channels/base/src/SessionRouter.ts:342)在有值时写入、无值时 delete;sessionOptions() 也只在有值时才把 approvalMode 放进会话请求。因此未配置模式的 channel 行为与改动前完全一致——不会有 undefined 混进创建请求,也不会写入空值键。worker 启动时这个 map 是全新的,所以 delete 分支在此处是空操作,而非状态变更。

"重启恢复"的说法成立。 注册发生在同一个启动块中 restoreRoutes() 之前,而 router 的恢复路径会把模式一并传给 loadSession——SessionRouter.test.ts:2121 正好固化了这一点({ approvalMode: 'yolo', sourceId: 'ch' })。所以测试计划里"重启 worker 后模式依然生效"这一项由既有测试覆盖,不只是描述里的断言。

不会新增服务端拒绝的情况。 这是唯一不那么显然的风险:此前从未为非 webhook channel 下发的取值,现在会到达 daemon。channel 配置按 packages/core/src/config/approval-mode.ts 的 APPROVAL_MODES 校验,daemon 侧该字段类型为 DaemonApprovalMode = PermissionMode,其 PERMISSION_MODES 是完全相同的五个字面量。取值域一致,因此不会有新的校验失败。SDK 在 create、resume、worktree-reset 三条路径上都会转发该字段(DaemonClient.ts:3427、:3808、:4296)。

测试是有效的,不是摆设。 在旧的判断下,selects all configured channels in one shared router 用例会直接失败——其中的 telegram channel 配置了 approvalMode: 'yolo' 且没有 webhook,因此对精确调用列表 [['telegram','yolo'], ['feishu', undefined]] 的断言不可能通过。参数化用例对非 webhook channel 的五种模式各做了一次同样的验证,同时旁边还有一个带 webhook 的 channel。把这一行修复回退,两个用例都会变红。

两点非阻塞的观察:

  • it.each 只改变非 webhook channel 的模式,带 webhook 的那个始终是 yolo。考虑到相邻用例已经覆盖了"未配置模式"的情况,这样的覆盖是够的;唯一没有任何断言的组合是"带 webhook + 非 yolo 模式"。
  • 测试固化的是 router mock 边界上的注册行为,并没有观察 daemon 侧会话是否真的以该模式启动——这一跳由 router/SDK 的类型和既有 router 测试保证,不在本 PR 覆盖范围内。

测试证据

这是一次无人值守的 CI 运行,因此按门禁规则我没有执行 PR 中的任何代码——没有构建、没有跑测试、没有 checkout head 分支。下面的证据来自 API 读取的该 PR 自身 CI(针对被审查的提交),以及上述静态阅读。e3e27ca 上共 91 个 check-run,0 个失败。

被取消的那些 lane(review-pr、precheck-pr、authorize、resolve-pr、publish-resolution 等)是 bot 编排任务,在 PR 合并时被取消,与 PR 改动无关,也不是产品 CI。triage 就是本次运行,当时仍在进行。macOS 与 Windows 的单测 lane 被矩阵跳过,因此跨平台覆盖只剩 ubuntu lane;本次改动是不依赖平台的普通控制流,我不认为这构成缺口。

作者在本线程的评论中报告本地 578 项测试通过,以及一个覆盖七个新建会话与七个恢复请求的本地集成验证。这是作者在 macOS 上的自述——此处仅做归属说明,未重跑,也不作为证据计入。独立信号是 ubuntu 单测 lane 的绿色结果,其中确实包含 daemon-worker.test.ts。

(CI 结果表格见上方英文部分,由 finalize 任务在 CI 稳定后就地更新。)

如果要把剩下的缺口补上,沙箱验证可以做到:@qwen-code /verify——daemon 管理的 channel 会话是否真的以配置的模式启动、没有 webhook 的 yolo channel 是否不再弹出审批卡片,从 diff 里看不出来;新增测试断言的是 router mock 上的注册行为,即使 daemon 忽略转发过去的字段,它们依然会通过。作者是 fork 贡献者,因此这是一次由维护者赞助的运行:维护者发出 @qwen-code /verify 评论即批准其所针对的 head,该运行会先做执行前风险筛查并完整清空工作区。阅读产出的报告时,请保持与阅读 fork 自身 CI 日志同样的怀疑态度——被验证的代码是对抗性输入,即使沙箱限定了它能做什么,它仍可能影响报告怎么写。(同样原因,这里不能用 /tmux。)合并之后这一步是可选的,只有当 release note 需要一句经过确认的端到端结论时才值得做。

— Qwen Code · qwen3.8-max-2026-09-02

Reviewed at e3e27ca9e20e2d1507dd9c87fe7ba2bb9197e318 · re-run with @qwen-code /triage

@qwen-code-review-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — the fix is right and minimal; the only reservation is that the end-to-end effect is inferred from a mock boundary rather than observed, and the change flips permission-gating behaviour for existing configs.

Asked what I would have done before reading the diff: delete the webhook condition so the daemon path matches the two standalone paths, then make the regression test say something about modes rather than about webhooks. That is what this PR does, so there is no simpler path I can offer — the diff is already at the floor for this fix.

What I checked rather than assumed, since "one-line fix" is where things hide:

  • setChannelApprovalMode(name, undefined) deletes rather than stores, and sessionOptions() only forwards a truthy mode — so channels without an explicit mode are untouched. This is the regression I would have expected from an unconditional call, and it isn't there.
  • The channel parser and the daemon type the same five literals, so no value newly reaches a validator that rejects it.
  • Registration precedes restoreRoutes(), and the restore path already forwards the mode, so the restart case in the test plan is real.
  • Reverting the fix reddens both updated tests, so the coverage pins the change instead of decorating it.

Where I land: this is a consistency fix with the right blast radius, and the author flagged the one behaviour change that matters (yolo now actually taking effect on daemon channels without webhooks) instead of letting a reviewer find it. The reservation keeping this at 4 rather than 5 is that nothing here observes a daemon-side session starting in the configured mode — the tests stop at the router mock, and the author's local integration numbers are self-reported on one platform. For a change this small, with the type-level domain match above, I'm comfortable shipping on that basis; I just wouldn't call the end-to-end claim proven.

Verdict, and why no action follows it. This would have been an approve: every stage clean, no Stage 0 escalation, the fork-refactor guardrail not applicable (fix(cli):), and all three pull_request workflow runs on this head completed green — nothing pending to defer against. But the PR merged at 04:30 UTC while this run was in flight, already carrying a human maintainer's approval (yiliang114) and the dev-bot's final gate review. An approval from this bot on a merged PR would be a second, meaningless vote on a decision a maintainer already made, so I'm posting the review record and taking no review action. Likewise no deferred-approval marker — there is nothing left to approve.

If anything here is worth acting on after the fact, it is the two non-blocking items in the Stage 2 comment: the stale channel-adapter doc note claiming the approval mode is never read, and the one untested webhook + non-yolo combination.

中文说明

信心度:4/5 —— 修复方向正确且已是最小改动;唯一的保留意见是端到端效果是从 mock 边界推断出来的而非实测观察到的,并且这次改动改变了既有配置下的权限管控行为。

在读 diff 之前先问自己会怎么做:删掉 webhook 这个条件,让 daemon 路径与两条 standalone 路径一致,然后让回归测试断言"模式"而不是"是否有 webhook"。这正是本 PR 做的事,所以我没有更简单的方案可提——就这个修复而言,diff 已经到底了。

因为是"一行修复",最容易藏问题的地方,我逐项核实而不是默认:

  • setChannelApprovalMode(name, undefined) 是删除而非存储,且 sessionOptions() 只在有值时下发——所以未显式配置模式的 channel 完全不受影响。这本是我预期无条件调用可能带来的回归,实际并不存在。
  • channel 解析器与 daemon 使用同样的五个字面量,因此不会有取值新到达一个会拒绝它的校验器。
  • 注册发生在 restoreRoutes() 之前,而恢复路径本来就会转发该模式,所以测试计划中的重启场景是成立的。
  • 回退这行修复会让两个更新后的测试变红,说明覆盖确实固化了改动,而不是装饰。

我的结论:这是一个影响范围恰当的一致性修复,作者主动标出了唯一重要的行为变化(没有 webhook 的 daemon channel 上 yolo 现在真的会生效),而不是留给审查者去发现。让它停在 4 分而不是 5 分的保留意见是:这里没有任何东西观察到 daemon 侧会话以配置的模式启动——测试止于 router mock,而作者的本地集成数据是在单一平台上自述的。对这么小的改动,加上上面那条类型层面的取值域一致性,我可以接受就这样发布;只是我不会把端到端的结论称为"已被证明"。

结论,以及为什么没有后续动作。 这本来会是一次 approve:各阶段干净,没有 Stage 0 升级,fork refactor 护栏不适用(标题是 fix(cli):),该 head 上三个 pull_request workflow 运行全部绿色完成——没有待决事项需要延迟。但 PR 在本次运行过程中已于 UTC 04:30 合并,并且已经带有人类维护者(yiliang114)的批准与 dev-bot 的最终门禁审查。在一个已合并的 PR 上再由本 bot 投一次批准,只是对维护者已经做出的决定投出第二张无意义的票,因此我只留下审查记录,不执行任何 review 动作。同样不发延迟批准标记——已经没有可批准的对象了。

如果其中还有什么值得事后处理,就是 Stage 2 评论里的两个非阻塞项:channel adapter 文档中"审批模式从未被读取"的过时说明,以及唯一未覆盖的"带 webhook + 非 yolo 模式"组合。

— Qwen Code · qwen3.8-max-2026-09-02

Reviewed at e3e27ca9e20e2d1507dd9c87fe7ba2bb9197e318 · re-run with @qwen-code /triage

@qwen-code-review-bot

Copy link
Copy Markdown
Collaborator

Released in v0.26.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants