feat(web-shell): support custom Hex session group colors - #6752
Conversation
E2E / validation resultsVerified on macOS against upstream
After syncing upstream |
|
Re-run after merge conflict resolution (resolved by @qwen-code-dev-bot, verified locally by @wenshao). Gate findings unchanged. Template looks good ✓ — all sections present, bilingual, linked to #6744. Problem: well-defined feature request. Issue #6744 provides clear acceptance criteria, and the daemon currently rejects any non-preset group color with Direction: aligned. Named session groups are a durable organizational feature, and six preset colors genuinely limit larger catalogs. Extending the existing model with Size: 63 core production lines ( Approach: the scope feels right. The change correctly splits Moving on to code review. 🔍 中文说明合并冲突解决后重新运行(由 @qwen-code-dev-bot 解决,@wenshao 本地验证)。门控结论不变。 模板完整 ✓ — 所有部分齐全,双语,关联 #6744。 问题:明确的功能需求。Issue #6744 提供了清晰的验收标准,daemon 目前会以 方向:对齐。命名会话分组是持久的组织功能,六种预设颜色确实限制了更大的分类目录。在现有模型上扩展 规模:63 行核心生产代码( 方案:范围合理。改动正确地将 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
samuelhsin
left a comment
There was a problem hiding this comment.
Review
Verdict: Approve with suggestions — direction matches #6744; named groups gain validated #RRGGBB while quick session tags stay preset-only. No Critical findings against commit e54e7eae46b05d9c7df51a0e0d347999c0c29565.
Why this looks correct
- Core
normalizeGroupColor()accepts presets or/^#[0-9a-f]{6}$/iand canonicalizes Hex to lowercase;assertSessionColor()keeps quick tags preset-only. - Type split is consistent across core / SDK / bridge / REST / ACP: named-group
colorisPreset | Hex; session organization andcolorOptionsremain preset. - Named-group create/update routes pass
colorthrough to core validation; organization routes still reject Hex viaGROUP_COLOR_OPTIONS. - WebShell adds Custom + native picker + Hex field; custom dots use inline
backgroundColor; invalid Hex disables Save and surfacesrole="alert". - Focused tests cover normalization, persistence after restart, malformed Hex, preset-only quick tags, picker/input sync, and custom-dot rendering.
Critical
None.
CI Test / web-shell E2E Smoke failures are the unrelated upstream ScheduledTasksDialog → missing ../composerTagIcons build break already disclosed in the PR description, not introduced by this diff.
Suggestions
- Trim Hex input before validate —
"#12abef "currently fails both clientnormalizeGroupColorInputand corenormalizeGroupColor. Trimming would avoid accidental invalidation. - Picker value while Hex is invalid —
value={normalizedGroupColor ?? DEFAULT_CUSTOM_GROUP_COLOR}can show the default swatch while the text field still holds an illegal value. Prefer freezing the last valid color (or leaving the picker unbound) until the Hex parses again. #${string}is wider than runtime — runtime regex is authoritative; a one-line comment on the type would help callers not assume any#…string is legal.- Light custom dots — custom dots have no border/outline;
#ffffffcan disappear on a light sidebar. A subtle ring would help contrast. - Merge hygiene — rebase or wait for the
composerTagIconsfix onmainso CI green is attributable to this change.
#6744 checklist
| Criterion | Result |
|---|---|
Named groups accept #RRGGBB |
Pass |
| Lowercase normalize + persist | Pass |
| Picker + Hex text field | Pass |
| Quick tags remain six presets | Pass |
| SDK contract expanded | Pass |
Code ReviewThe implementation is clean and well-structured. The type split between No critical blockers or AGENTS.md violations found. Notable design choices (all sound):
One observation (non-blocking): Unit Tests (re-run on PR head)
WebShell REST API Live Test (tmux)Started Every acceptance criterion from #6744 verified: hex normalization to lowercase, invalid input rejection, preset-only quick tags, group CRUD with hex, colorOptions still preset-only. WebShell UI could not be tested live — same pre-existing environment blocker. WebShell unit tests (61 passed, verified by author and maintainer) cover the custom color UI, picker/hex sync, validation, and dot rendering. 中文说明代码审查实现整洁且结构良好。 未发现关键阻断项或 AGENTS.md 违规。 值得注意的设计选择(均合理):
一个观察(不阻断): 单元测试(PR 头重新运行)
WebShell REST API 实机测试(tmux)在 worktree 的 PR 分支上启动 WebShell UI 无法实机测试——同一预存环境阻塞。WebShell 单元测试(61 通过,由作者和维护者验证)覆盖了自定义颜色 UI、调色板/hex 同步、验证和圆点渲染。 — Qwen Code · qwen3.7-max |
|
Re-run after merge conflict resolution. All findings hold. This PR ships the feature cleanly. The type design is correct — splitting Live REST API testing on the PR head confirms: hex normalization to lowercase works, invalid values are rejected with structured errors, preset colors remain backward compatible, and quick session tags correctly reject hex. All 1,201 unit tests across the affected packages pass. The one gap is WebShell live UI testing, blocked by an unrelated upstream environment issue — the 61 WebShell unit tests (independently verified by both the author and a maintainer) cover the behavior well enough. The merge conflict resolution was clean — the PR's custom color constants and main's footer/tooltip infrastructure are additive and independent. No semantic conflict. Scope is tight (63 core + 239 other production lines), documentation and E2E plan are included, and every change serves the stated goal. This is ready to ship. 中文说明合并冲突解决后重新运行。所有结论不变。 这个 PR 干净地交付了该功能。类型设计正确——将 PR 头上的 REST API 实机测试确认:hex 规范化为小写有效、无效值以结构化错误拒绝、预设颜色向后兼容、快捷会话标签正确拒绝 hex。受影响包的所有 1,201 个单元测试通过。唯一的缺口是 WebShell 实机 UI 测试,被无关的上游环境问题阻塞——61 个 WebShell 单元测试(由作者和维护者独立验证)已充分覆盖该行为。 合并冲突解决干净——PR 的自定义颜色常量与 main 的页脚/提示基础设施是独立的增量添加,无语义冲突。 范围紧凑(63 行核心 + 239 行其他生产代码),包含文档和 E2E 计划,每处改动都服务于目标。可以合入。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Added a WebShell screenshot of the custom Hex group color editor to the PR description. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge conflict resolution summary for PR #6752Conflicted file
Conflict detailsThe single conflict was in the module-level constants block (around line 54), where two independent additions overlapped: HEAD (PR branch) added two constants for the custom Hex color feature: const CUSTOM_GROUP_COLOR_OPTION = '__custom__';
const DEFAULT_CUSTOM_GROUP_COLOR: DaemonSessionGroupHexColor = '#416ef5';origin/main added tooltip layout constants, exported types/interfaces ( ResolutionKept both sides. The changes are additive and independent — the PR's custom color constants and main's footer/tooltip infrastructure do not conflict semantically. The PR branch's two constants were placed first (preserving their original position), followed by main's block. Other auto-merged filesGit auto-merged several other files without conflicts (CSS, test files, i18n, App.tsx, etc.). These were reviewed via the staged diff and appear correct. Commit
|
✅ Local verification — merge referenceBuilt and tested this PR from a clean isolated worktree at the PR head ( Environment: macOS (darwin 24.6), Node v22.23.1 / npm 10.9.8, fresh Results
Before / After proofTo confirm the new tests actually exercise the new code (not just pass trivially), I re-ran the PR's new core test against
So WebShell UI (rendered in a real browser via the mock daemon)The named‑group editor now offers a Custom… option that reveals a native color picker synced with a Invalid input disables Save and shows an accessible ( Notes for the reviewer
Verdict: LGTM from a build/test standpoint — behavior, contract, i18n (EN + ZH), and theming all verified locally. 👍 中文说明(点击展开)✅ 本地验证 —— 合并参考从干净的独立 worktree 检出 PR 头 环境: macOS(darwin 24.6),Node v22.23.1 / npm 10.9.8,全新 结果
前后对比证明为确认新增测试确实覆盖了新代码(而非平凡通过),我把
因此 WebShell UI(通过 mock daemon 在真实浏览器中渲染)命名分组编辑器新增 Custom… 选项,展开后是与 给 Reviewer 的说明
结论: 从构建/测试角度 LGTM —— 行为、契约、i18n(中英)与主题均已在本地验证。👍 Verified locally on an isolated worktree; screenshots rendered from the real WebShell against a fixture daemon. |
…o-prefix Hex input Review follow-ups for the custom Hex group color editor: - Validate the preset branch against the daemon-provided color catalog instead of the hardcoded palette, so future preset additions stay selectable in the editor. - Auto-prefix bare values with '#' in the Hex field so pasted bare Hex validates, and free text can no longer collide with a preset name and silently flip the select out of Custom mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
|
|
||
| .groupColorError { | ||
| grid-column: 2; | ||
| color: var(--destructive, #dc2626); |
There was a problem hiding this comment.
[Suggestion] .groupColorError uses var(--destructive, #dc2626) but every other error-text element in this module uses var(--error-color) (see .dangerButton and error messages elsewhere). The --destructive variable is not defined anywhere in the web-shell package, so the fallback #dc2626 always fires — and it differs from the --error-color values (#fc8181 light / #c0362c dark), producing a visually inconsistent red.
| color: var(--destructive, #dc2626); | |
| color: var(--error-color); |
— qwen3.7-max via Qwen Code /review
| @@ -353,8 +381,13 @@ function getGroupColorClass(color: DaemonSessionGroupColor): string { | |||
| case 'purple': | |||
| return styles.groupColorPurple; | |||
| } | |||
There was a problem hiding this comment.
[Suggestion] The old getGroupColorClass had a const exhaustive: never = color compile-time exhaustiveness check that caught presets added to the type without a corresponding case. Now that DaemonSessionGroupColor includes the open-ended hex template type, the switch can no longer be exhaustive — but a future preset typo (e.g. 'purpl') would silently return undefined instead of failing at compile time. Consider adding a comment marking the switch as needing manual upkeep when presets are added, or an explicit default that logs a warning.
— qwen3.7-max via Qwen Code /review
| ).toBe('#416ef5'); | ||
| }); | ||
|
|
||
| it('edits an existing custom group and switches it back to a preset', async () => { |
There was a problem hiding this comment.
[Suggestion] The edit tests cover the custom-to-preset direction but not the reverse: opening the rename dialog on a group whose color is a preset (e.g. 'red'), switching to Custom mode, entering a hex value, saving, and asserting updateSessionGroup is called with the new hex color. This exercises a different initialization path in handleRenameGroup — specifically normalizeHexColorInput(group.color) ?? DEFAULT_CUSTOM_GROUP_COLOR taking the fallback branch.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅



What this PR does
Named session groups can now use either the existing preset colors or a custom six-digit Hex color. The daemon validates and canonicalizes custom values to lowercase, the SDK carries the expanded contract, and WebShell provides both a native color picker and a Hex text field while rendering custom group dots with the persisted color. Quick session color tags intentionally remain limited to the six presets.
Why it's needed
Six fixed colors are not enough for larger group catalogs or teams that use an established project palette. Supporting a precise Hex value extends the existing named-group model without creating a parallel color system or changing the lightweight quick-tag behavior.
Reviewer Test Plan
How to verify
Open WebShell session management and create a named group. Choose Custom, select a color with the picker, then enter an uppercase six-digit Hex value and save. Confirm the saved group dot uses that color and the value is returned in lowercase after reload. Reopen and rename the group while keeping or changing the custom color. Enter an invalid value such as
#abcor12abefand confirm Save is disabled with an accessible validation message. Finally, open the quick color menu for a session and confirm it still offers only the six preset colors.Evidence (Before & After)
Before: named groups accept only six preset colors and reject custom values with
invalid_group_color.After: focused core, REST, ACP HTTP, TypeScript SDK, and WebShell tests cover Hex normalization, persistence after service restart, invalid input, preset-only quick tags, request/response round trips, picker/input synchronization, and custom-dot rendering.
Tested on
Environment (optional)
macOS local checkout. Focused suites, package typechecks, and the root build pass after syncing upstream
mainwith the unrelated scheduled-tasks import fix from #6748. The screenshot above was captured from the local WebShell against a fixture daemon; CI should validate the supported platform matrix.Risk & Scope
Linked Issues
Closes #6744
中文说明
此 PR 的改动
命名会话分组现在既可使用现有预设颜色,也可使用自定义六位 Hex 颜色。daemon 会验证自定义值并统一转为小写,SDK 会传递扩展后的契约,WebShell 同时提供原生调色板和 Hex 文本输入框,并使用持久化的颜色渲染自定义分组圆点。会话的快捷颜色标签仍然只支持六种预设颜色。
为什么需要此改动
当分组数量较多或团队已有项目配色规范时,六种固定颜色并不足够。支持精确 Hex 值是在现有命名分组模型上的扩展,不会引入平行的颜色系统,也不会改变轻量快捷标签的行为。
Reviewer 测试计划
如何验证
打开 WebShell 会话管理并创建命名分组。选择“自定义”,先用调色板选色,再输入一个大写六位 Hex 值并保存。确认保存后的分组圆点使用该颜色,并在重新加载后返回小写值。重新打开并重命名分组,同时保留或修改自定义颜色。输入
#abc或12abef等无效值,确认保存按钮禁用并显示可访问的校验提示。最后打开会话的快捷颜色菜单,确认其中仍然只有六种预设颜色。前后证据
改动前:命名分组只接受六种预设颜色,自定义值会以
invalid_group_color被拒绝。改动后:聚焦的 core、REST、ACP HTTP、TypeScript SDK 和 WebShell 测试覆盖 Hex 规范化、服务重启后的持久化、无效输入、仅预设的快捷标签、请求与响应往返、调色板与输入框同步,以及自定义圆点渲染。
测试平台
环境(可选)
macOS 本地检出。同步包含 #6748 无关定时任务导入修复的 upstream
main后,聚焦测试、各包 typecheck 和根目录构建均通过。上方截图来自连接本地 fixture daemon 的 WebShell;支持平台矩阵交由 CI 验证。风险与范围
关联 Issue
Closes #6744