feat(web-shell): gate Session Workflow behind an experimental setting - #8391
Conversation
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 8 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual body, reviewer test plan included. Problem: this is the planned "PR 1" of #8389 (labeled Direction: aligned. Default-off experimental gating for an in-flight daemon/WebShell feature is exactly the right shape for this stage of the rollout, and the issue explicitly scopes this PR to presentation only (no new protocol mode, no scheduler, no store). Size: one core-path file — Approach: scope feels right and minimal — every file serves the gate (schema entry, vscode schema mirror, flag threading through App/ChatPane/SplitView/ArtifactPanel/SideTaskPanel, two label surfaces, i18n, tests, design doc). No drive-by changes spotted. The design doc correctly documents Phase 1 vs Phase 2 boundaries. Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 必填章节齐全，中英双语，包含 reviewer 测试计划。 问题：这是 #8389（已标 方向：对齐。对进行中的 daemon/WebShell 功能采用默认关闭的实验性开关，正是当前阶段合适的推进方式；issue 也明确本 PR 只做展示层（不新增协议模式、不做调度器、不新增 store）。 规模：仅一个核心路径文件 —— 方案：范围合理且最小化 —— 每个文件都服务于这个开关（schema 条目、vscode schema 镜像、在 App/ChatPane/SplitView/ArtifactPanel/SideTaskPanel 中传递开关、两个文案入口、i18n、测试、设计文档）。未发现顺手改动。设计文档正确划分了 Phase 1 与 Phase 2 的边界。 风险：无升级风险信号 —— 改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewReviewed
Two non-blocking observations:
Nothing here blocks; the implementation is straightforward and convention-following. Files changed (15 of 15)
Test evidence (this PR's own CI — per gate rules I do not run PR code)All
The gate itself is pinned by tests that would fail if the gating were removed (App gate test asserts empty planTodos with the flag off, then the DAG after enabling; the ChatPane exit-plan test now opts in explicitly). Sandboxed verification would settle one remaining end-to-end leg: 中文说明代码审查已审查
两个非阻塞观察：
均不阻塞；实现直接、遵循既有约定。 测试证据（来自本 PR 自身的 CI —— 按门禁规则不运行 PR 代码）被审查 commit 上的所有 开关本身由"移除开关即会失败"的测试锁定（App 门禁测试断言关闭时 planTodos 为空、开启后出现 DAG；ChatPane 的 exit-plan 测试显式开启开关）。 剩余一个端到端环节可由沙箱验证收尾： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, minimal, well-tested feature gate; the 5th point is held back only by two cosmetic nits and one end-to-end leg no test pins. Stepping back: this is exactly what PR 1 of #8389 should look like. My independent read of the problem before opening the diff was "add a default-off schema flag, serve it through the existing workspace-settings route, and gate the three planTodos data sources plus the plan-mode labels" — the PR does precisely that, with nothing extra. There's no new machinery, no protocol change, no scheduler creeping in early; the Phase 2 boundary is documented in the design doc instead of being half-built. The part that mattered most to verify — that nothing leaks when the setting is off — checks out by enumeration: the Workflow DAG renders only from Non-blocking nits, already noted in the review comment: agent pages don't pick up the "Plan & Review" relabel (presumably intentional scope), and the settings-dialog → live-toggle path is the one leg exercised only by descriptor injection in tests, which the Approving — ready to ship. ✅ 中文说明置信度：4/5 —— 干净、最小化、测试充分的特性开关；扣掉的 1 分仅因两个外观层面的小问题和一条没有测试锁定的端到端环节。 退一步看：这正是 #8389 的 PR 1 应有的样子。我在看 diff 之前对问题的独立判断是"新增默认关闭的 schema 开关、通过现有 workspace-settings 路由下发、并在三个 planTodos 数据源与 plan 模式文案处加门" —— 本 PR 完全如此，没有任何多余内容。没有新机制、没有协议变更、没有提前混入调度器；Phase 2 的边界写在设计文档里，而不是半成品代码里。 最需要验证的部分 —— 关闭设置时不泄漏 —— 经枚举确认成立：Workflow DAG 仅由 非阻塞小问题（已在审查评论中记录）：agent 页面未接入 "Plan & Review" 文案（推测是有意的范围控制）；设置对话框 → 即时切换这一环节只有 descriptor 注入式测试覆盖，如需在后续 PR 落地前拿到 A/B 证据，可用我审查评论中的 批准 —— 可以合入。✅ — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #8391 feat(web-shell): gate Session Workflow behind an experimental setting
Type: New Feature (experimental flag)
Change size: +189/-15 across 15 files
Findings Summary
- Critical/Major: 0
- Minor: 0
- Nit: 0
Key Observations
Clean, default-off feature boundary. The new experimental.sessionWorkflow setting is declared in the CLI schema with default: false and requiresRestart: false, exposed in the settings dialog, and read from workspaceSettings via the established values.effective === true pattern (same as hideTips). The boolean is then prop-drilled through App → ChatEditor / ApprovalModeDialog / ToolApproval / TasksDialog / ChatPane / SplitView / ArtifactPanel / SideTaskPanel; every consumer defaults to false so omitted props retain existing behavior.
- UI labels:
planmode is renamed to Plan & Review / 计划并审阅 only when the flag is enabled, in both the mode selector and the approval-mode dialog; otherwise existing labels stay unchanged. - Workflow DAG gating:
planTodos={sessionWorkflowEnabled ? ... : []}andagentTools={sessionWorkflowEnabled ? ... : []}keep the Tasks/ToolApproval surfaces unchanged when disabled. - i18n: EN and ZH keys added symmetrically, including the new settings labels/descriptions.
- No protocol change: the existing
planapproval mode is reused, consistent with the PR's stated rollout strategy.
Cross-Validation
| Finding | Other Reviewer | My Assessment |
|---|---|---|
| None | qwen-code-ci-bot APPROVED | Agree — no blockers found |
Additional Audit Coverage
Areas I independently checked beyond ci-bot:
- Setting read pattern: confirmed
values.effective === trueis the established pattern in App.tsx. - Default prop values: every new
sessionWorkflowEnabledprop defaults tofalse, preventing accidental opt-in from partial prop passing. - Schema test: settingsSchema.test.ts pins
type: 'boolean',default: false,requiresRestart: false,showInDialog: true. - Test coverage: ApprovalModeDialog test asserts label switch only when enabled; App test asserts
planTodosis empty without the setting and populated after enabling; ChatPane test passes the flag explicitly. - Out-of-scope guard: no new persistent store or scheduler introduced.
Final Verdict
APPROVE. Low-risk, reversible experimental feature gate. Ready to merge.
This review was generated by QoderWork AI
|
Released in v0.21.4. |
4 participants
What this PR does
Adds a default-off
experimental.sessionWorkflowsetting for daemon WebShell sessions. When enabled, the existingplanapproval mode is presented as Plan & Review, and the existing Todo-based Workflow DAG is available in task and exit-plan approval surfaces. The setting is applied consistently to the main chat, split panes, and side-task panes.When the setting is disabled or unavailable, ordinary Todo rendering and Plan Mode behavior remain unchanged, while the Workflow DAG and new labels stay hidden.
Why it's needed
Session Workflow is still experimental and should not change the experience of existing users by default. This creates an explicit, reversible rollout boundary before the approval flow is bound to a specific plan revision in the follow-up PR.
Reviewer Test Plan
How to verify
experimental.sessionWorkflowunset or set it tofalse; confirm Plan Mode keeps its existing name, ordinary Todo lists remain visible, and task/exit-plan surfaces do not render a Workflow DAG.experimental.sessionWorkflowtotrue; confirm the mode picker and composer show Plan & Review, and structured Todo data is rendered as the existing Workflow DAG in the task and exit-plan approval surfaces.Evidence (Before & After)
Before: Workflow presentation was unconditional whenever compatible Todo data was present.
After: Workflow presentation and Plan & Review labels are opt-in; ordinary Todo behavior remains available in both states.
Automated checks passed: focused settings/WebShell tests, full repository build, and full repository typecheck on macOS.
Tested on
Environment (optional)
Node.js 22, local daemon WebShell build.
Risk & Scope
planprotocol mode.Linked Issues
Related to #8389.
中文说明
本 PR 做了什么
为 daemon WebShell 会话新增默认关闭的
experimental.sessionWorkflow设置。开启后，现有plan审批模式会显示为 计划并审阅，并在任务与退出 Plan 审批界面中展示现有的 Todo Workflow DAG。该设置统一作用于主会话、分屏和侧任务面板。当设置关闭或尚未加载时，普通 Todo 展示与 Plan Mode 行为保持不变，同时隐藏 Workflow DAG 和新增文案。
为什么需要
Session Workflow 仍属于实验功能，不应默认改变现有用户体验。这个 PR 先建立明确且可回退的灰度边界；后续 PR 再把审批绑定到具体的计划修订版本。
Reviewer Test Plan
如何验证
experimental.sessionWorkflow或将其设为false；确认 Plan Mode 仍使用原名称，普通 Todo 列表仍可见，任务与退出 Plan 界面不展示 Workflow DAG。experimental.sessionWorkflow设为true；确认模式选择器和输入区显示 计划并审阅，并且结构化 Todo 会在任务与退出 Plan 审批界面中展示为现有 Workflow DAG。前后对比证据
之前：只要存在兼容的 Todo 数据，Workflow 展示就是无条件的。
之后：Workflow 展示和计划并审阅文案改为显式开启；两个状态下普通 Todo 行为都保持可用。
已在 macOS 通过设置/WebShell 相关测试、全仓构建和全仓类型检查。
测试环境
Node.js 22，本地 daemon WebShell 构建。
风险与范围
plan协议模式。关联 Issue
关联 #8389。