feat(ui): add shared tabs component

[Candice00711]

Summary

Add a generic Tabs primitive to @octo/ui and migrate the existing Semi-based tabs in the forward modal.

This PR is stacked on #1478 and should be reviewed and merged after it.

Related Issue

N/A — P2-12 component rollout.

Changes

• Add controlled and uncontrolled Tabs with line, segmented, and segmented-plain variants.
• Support activeKey, defaultActiveKey, onChange, size, disabled items, overflow scrolling, keyboard navigation, and optional panels.
• Add light/dark semantic tokens, Storybook variants, and focused component tests.
• Replace the ForwardModal Semi Tabs usage while preserving its four filters and selection behavior.
• Keep native hand-written tab surfaces, including SearchWorkspace, unchanged.

Architecture / Module Boundary

• Affected module(s): @octo/ui, ForwardModal
• New or changed user-visible entry point: no; the existing ForwardModal entry is preserved
• Shared layer touched: ui and Components
• If shared code changed, impact scope: new @octo/ui Tabs export; one production consumer in ForwardModal
• Duplicate entry point checked: yes

Testing

• pnpm --dir packages/octo-ui exec vitest run src/components/Tabs/Tabs.test.tsx — 11 passed
• pnpm --dir packages/dmworkbase exec vitest run src/Components/ForwardModal/ui/__tests__/TabsBar.test.tsx — 2 passed
• pnpm --filter @octo/ui typecheck
• pnpm --filter @octo/web build
• Manually verified the real ForwardModal at http://127.0.0.1:3001/: all four tabs switch correctly and the 2px active indicator covers the 1px baseline.

Checklist

• I have read CONTRIBUTING.md
• PR description is in English
• Added tests for my changes
• Updated documentation through Storybook coverage
• Confirmed the change does not add or modify UI copy
• Confirmed the change follows module ownership and does not add duplicate user-visible entry points
• Described impact scope when shared components, messages, bridge, or services are changed
• Followed commit message conventions (Conventional Commits)

[github-actions]

Dependency Changes Detected

This PR modifies dependency files. Please review whether these changes are intentional.

Changed files:

• apps/extension/package.json
• apps/web/package.json
• packages/dmworkbase/package.json
• packages/dmworkcontacts/package.json
• packages/dmworkmcp/package.json
• packages/dmworksummary/package.json
• packages/octo-ui/package.json
• pnpm-lock.yaml

Maintainer checklist:

• Confirm dependency changes are intentional
• Review package delta if lockfile changed

[Jerry-Xin]

Superseded by my REQUEST_CHANGES review on this same head — two a11y defects verified post-approval (dangling aria-controls on inactive tabs; focusable panel without visible focus). See the latest Jerry-Xin review.

[yujiawei]

Code Review — PR #1549 (octo-web)

Scope reviewed: this PR is stacked on #1478, so the diff against main (137 files) is mostly inherited. I reviewed the incremental delta only — 980d8604 (#1478 head) → 615a45cf, which is a single commit touching 13 files, +817/−16. Everything outside that delta is treated as pre-existing baseline.

---

1. Specification compliance

Spec: ✅

Checked each item in the PR description against the delta:

Claim	Result
Controlled + uncontrolled Tabs with line / segmented / segmented-plain	✅ Tabs/index.tsx:25-44, Tabs/index.css:39-83
activeKey, defaultActiveKey, onChange, size, disabled items, keyboard nav, optional panels	✅ all present and exercised by tests
Light/dark tokens, Storybook variants, component tests	✅ styles/tokens.css:61-121, Tabs.stories.tsx, Tabs.test.tsx (11 cases)
ForwardModal Semi Tabs replaced, four filters + selection preserved	✅ TabsBar.tsx:6-15 preserves key order followed / recent / group / direct; all four i18n keys exist in both zh-CN.json:887-890 and en-US.json:887-890
Native hand-written tab surfaces (incl. SearchWorkspace) unchanged	✅ not touched

Additional checks:

• No leftovers: grep for @douyinfe/semi-ui + Tabs/TabPane across packages/ and apps/ returns zero matches at this head — ForwardModal was the last Semi Tabs consumer and the migration is complete.
• No dangling style/test dependencies: no semi-tabs selector exists anywhere in *.ts(x)/*.css or e2e, and .wk-fm-tabs (ForwardModal.css:93) is a plain wrapper rule (margin + flex-shrink) with no Semi-internal selectors, so nothing is orphaned by the swap.
• Public surface parity: src/index.ts:26-27 and the hand-written src/index.d.ts:55-57 agree 1:1, and package.json files correctly gains src/components/Tabs/types.ts.
• No over-building: nothing outside the stated scope was added.

---

2. Code quality

Quality: Changes-Requested

P1 — the children (panel) mode of the new shared primitive has two objective a11y defects

Both are in newly added, exported, Storybook-documented, test-covered API surface. ForwardModal itself passes no children, so production is unaffected today — but this is the primitive the rest of the codebase is meant to standardise on, so the next consumer that adopts panels inherits both.

(a) Inactive tabs emit aria-controls pointing at an element that is not in the DOM.

packages/octo-ui/src/components/Tabs/index.tsx:89,104 vs :116-126

const hasPanel = item.children !== undefined
// ...
aria-controls={hasPanel ? panelId : undefined}

hasPanel is computed per item, but only the selected item's panel is rendered (:116). With items = [{key:'first',children},{...},{key:'third',children}] and first selected, the third tab carries aria-controls="<base>-panel-2" while #<base>-panel-2 does not exist. That is a dangling IDREF — axe-core's aria-valid-attr-value rule fails on it, and screen readers get a broken relationship. Fix: set aria-controls only when isActive, or render every panel and hide the inactive ones with hidden.

(b) The focusable tab panel has its focus indicator removed with no replacement.

packages/octo-ui/src/components/Tabs/index.css:136-140 + index.tsx:122

.octo-ui-tabs__panel { min-width: 0; color: var(--wk-text-primary); outline: none; }

The panel is rendered with tabIndex={0}, so Tab out of the tablist lands on it — with outline: none and no :focus-visible rule anywhere in the file, there is no visible focus indicator. That is a WCAG 2.4.7 (Focus Visible, AA) failure. Either add a .octo-ui-tabs__panel:focus-visible outline (mirroring the tab rule) or drop tabIndex={0} and only make the panel focusable when it has no focusable content.

P2 findings

P2-1 — :focus-visible silently overrides both segmented variants' border-radius.

packages/octo-ui/src/components/Tabs/index.css:130-134

.octo-ui-tabs__tab:focus-visible (specificity 0-2-0) sits after .octo-ui-tabs--segmented .octo-ui-tabs__tab (also 0-2-0, :73-79) and .octo-ui-tabs--segmented-plain .octo-ui-tabs__tab (:81-83). Equal specificity → later rule wins → border-radius: var(--wk-r-xs) clobbers the variant radius. Measured in headless Chromium against the repo's own primitive.css + semantic.css + these tokens (:focus-visible modelled as an equal-specificity class so the cascade is faithful):

LIGHT  segmented   selected: radius=9999px   focused: radius=3px
LIGHT  seg-plain   selected: radius=6px      focused: radius=3px
DARK   (identical)

The selected pill's background is radius-clipped too, so keyboard-focusing a segmented tab visibly squares off the pill. Neither the tests (no CSS assertions) nor the manual check (ForwardModal uses line) would surface this. Scope the rule to .octo-ui-tabs--line, or drop the border-radius override and rely on outline-offset alone. Note Button/index.css:25-28 sets no border-radius in its :focus-visible rule — worth staying consistent with it.

P2-2 — width: max-content makes the advertised overflow scrolling a no-op for both segmented variants.

packages/octo-ui/src/components/Tabs/index.css:1-13 sets overflow-x: auto on .octo-ui-tabs__list, but :55-59 then sets width: max-content for the segmented variants, so the list can never overflow itself — it overflows its parent instead and is clipped by an ancestor. Measured in a 180px-wide parent:

parent clientW=180
segmented list: clientW=336 scrollW=336 canScroll=false   <-- escapes the 180px parent
line      list: clientW=180 scrollW=300 canScroll=true     <-- scrolls correctly

Either remove width: max-content (use max-width: 100%) or drop overflow-x: auto for these variants and state that scrolling is line-only. Right now the code contradicts itself and the PR description's "overflow scrolling" claim only holds for one of three variants.

P2-3 — Uncontrolled mode can change its own selection without ever calling onChange.

packages/octo-ui/src/components/Tabs/index.tsx:33-44

When the active item is removed or becomes disabled, resolvedKey silently falls back to firstEnabledKey(items) and the effect at :40-44 commits it — onChange is never fired. Tabs.test.tsx:196-206 codifies exactly this. A consumer that mirrors the active tab (analytics, URL sync, data fetch) desyncs with no notification. Either fire onChange on self-correction, or document the invariant explicitly in types.ts.

P2-4 — An invalid controlled activeKey leaves the tablist in an ARIA-invalid state.

packages/octo-ui/src/components/Tabs/index.tsx:34-38

Two cases, both currently locked in by tests:

• activeKey names a disabled item (Tabs.test.tsx:174-190): the disabled tab keeps aria-selected="true" while focusKey moves tabIndex={0} to a different tab. APG requires the roving tabIndex=0 to be on the selected tab.
• activeKey is unknown (Tabs.test.tsx:160-172): nothing is selected at all.

I looked at this closely because it is arguably just defensive handling of caller error, and the component correctly avoids crashing — so I am not rating it a blocker. But the realistic trigger is ordinary (a permissions change disables the tab the parent is still pointing at), and the resulting state is a selected-but-disabled tab with empty content. A console.warn in dev, or refusing to select a disabled item, would be better than encoding the degenerate state as expected behaviour.

P2-5 — ForwardModal tab bar grows from Semi size="small" to size="md" (40px), and the test name contradicts the code.

TabsBar.tsx:34 passes size="md", which measures 40px tall (--wk-sp-10); the replaced Semi usage was size="small". The component does offer size="sm" (36px). This is a compact modal sandwiched between a search box and a scrolling list, so please confirm 40px is the intended design rather than a default. Separately, __tests__/TabsBar.test.tsx:39 is named "renders the four controlled forward filters with the compact line variant" while asserting octo-ui-tabs--md — the name should match whichever size is chosen.

P2-6 — The tablist reuses the modal's own title as its accessible name.

TabsBar.tsx:27 uses aria-label={t("base.forwardModal.title")} — the exact string ForwardModal.tsx:89 uses for the dialog title ("Forward" / "转发"). Two consequences: assistive tech announces the same name for the dialog and for the filter tablist, and when a caller overrides the modal title (title ?? t(...) at ForwardModal.tsx:89) the tablist label silently diverges from it. A dedicated key describing the control ("Chat filters") would be more accurate; note the mocked t in the test returns the key, so this is invisible to the test either way.

Nits (non-blocking)

• index.tsx:46-52 — the scrollIntoView effect also fires on mount. block: 'nearest' makes this a no-op when the tablist is already visible, so it is low-risk, but a "skip first render" ref would remove the class of bug where mounting a component scrolls an ancestor.
• index.tsx:105-106 — disabled and aria-disabled="true" are both set. The APG tabs pattern prefers aria-disabled alone so disabled tabs stay discoverable; native disabled removes them from the focus order entirely.
• index.css:26,100,132 — spacing tokens are used for non-spacing properties (line-height: var(--wk-sp-5), outline: var(--wk-sp-0-5)), and :35 expresses 36px as calc(var(--wk-sp-8) + var(--wk-sp-1)). If 36px is a real design step it deserves its own token.
• styles/tokens.css:63-90 — this is the first use of color-mix() in the repo. Fine for the Electron target (electron 26 → Chromium 116; color-mix shipped in 111) and for current browsers, but on Safari < 16.2 the whole custom property is invalid at computed-value time and the tab text falls back to inherited --wk-text-primary, i.e. active and inactive tabs become indistinguishable. The existing --wk-*-alpha-* primitives (already used at :79) would avoid that if the web browserslist (>0.2%, not dead) still needs to cover it.
• index.tsx:118,121 — items.indexOf(selectedItem) is recomputed twice; index from the map is already available.
• Duplicate key values in items are not guarded and would collide in tabRefs (:96-98) as well as triggering React's key warning.

Verified correct — worth calling out

The theme token block gets right the thing that is easy to get wrong here. theme-mode is set on document.body, not <html>, so any --octo-ui-*: var(--wk-*) alias declared only on :root would freeze at its light value. This PR redeclares all eight --octo-ui-tabs-* tokens inside body[theme-mode='dark'] (tokens.css:92-121), including the var(--wk-bg-surface) alias. Measured in headless Chromium with the real theme files:

                      LIGHT                  DARK
active color          rgb(28,28,35)      →   rgb(228,230,237)
segmented active bg   rgb(255,255,255)   →   rgb(31,35,41)
segmented bg          rgba(0,0,0,.06)    →   rgba(255,255,255,.06)
line baseline shadow  rgba(0,0,0,.08)    →   rgba(255,255,255,.07)

All theme-correct. The line/md geometry also matches the design annotations in the story (height 40px, gap 24px, 2px indicator over the 1px inset baseline, with z-index: 1 ensuring the indicator paints above the inset shadow).

---

3. Overall verdict

REQUEST_CHANGES

Spec: ✅ but Quality: Changes-Requested, so the gate does not pass. The blocking item is small and cheap to fix — the two a11y defects in the panel/children mode (P1a, P1b), roughly two lines each. The P2s are worth addressing in the same round since P2-1 and P2-2 both affect the two brand-new variants that have no production consumer yet and therefore no other safety net.

4. Suggested changes

1. index.tsx:104 — aria-controls={isActive && hasPanel ? panelId : undefined} (or render all panels with hidden).
2. index.css:136-140 — add a .octo-ui-tabs__panel:focus-visible outline, or drop tabIndex={0} from the panel.
3. index.css:130 — scope the focus rule to .octo-ui-tabs--line, or remove the border-radius override.
4. index.css:55-59 — replace width: max-content with max-width: 100%, or make overflow-x: auto line-only and adjust the description.
5. index.tsx:40-44 — fire onChange when uncontrolled state self-corrects, or document that it does not.
6. TabsBar.tsx:34 + TabsBar.test.tsx:39 — confirm md vs sm with design and make the test name agree.
7. TabsBar.tsx:27 — use a dedicated i18n key for the tablist's accessible name.

5. Additional observations (merge readiness, not code defects)

• No CI evidence exists for this PR. The CI workflow (build, lint, typecheck, unit tests) has not run — this is a cross-repository PR, so pull_request-triggered workflows are awaiting maintainer approval. Only the pull_request_target workflows (labeler, title lint, project automation) executed. A maintainer should approve the workflow run before merge; the PR description's local test results are the only signal today.
• pnpm-lock.yaml conflicts with main. git merge-tree shows exactly one conflicted path — pnpm-lock.yaml — so a rebase and lockfile regeneration is needed. I confirmed .github/workflows/ci.yml merges cleanly and both the branch's new "Octo UI tests" step and main's newer unit-tests job (workspace-wide turbo run test + @octo/base test:coverage) survive the merge, so both new test files will execute in CI post-rebase. Worth noting because that only became true recently: the branch point (6e52f7e4) predates main's unit-tests job, so on the branch as it stands TabsBar.test.tsx matches no CI step. Rebasing resolves it — no ci.yml change is needed in this PR.
• Merge order. Stacked on #1478, which is still open. This must not merge first.
• Not verified: I could not execute the test suites (no node_modules in my checkout), so the reported 11/2 passing counts are taken at the author's word. The full @octo/base suite has not been run against this change by CI or by me — the test:coverage step on main runs all of it, so please let CI confirm. All CSS claims above were measured in headless Chromium rather than reasoned about.

---

Review process note

This review absorbed findings from two independent automated advisory passes plus my own read.

• Line-level adversarial pass: ran. Consensus with my own read on the dangling aria-controls (P1a) and the :focus-visible radius override (P2-1) — both independently found by two sources, which is why they are stated with high confidence. It uniquely surfaced the width: max-content overflow contradiction (P2-2) and the panel outline: none issue (P1b), both of which I then confirmed by measurement. It rated the invalid-activeKey handling (P2-4) and the missing tab↔panel association a tier higher than I did; I downgraded both after independently re-reading the diff, on the grounds that the first is caller-error handling that fails safe and the second is pre-existing parity with the Semi implementation rather than a regression. Its coverage gap (CSS cascade, narrow-width overflow, live panel relationships) is what the headless-Chromium measurements above were added to close.
• Whole-repository long-context pass: ran, but off-scope — treated as absent. It reviewed the diff against main rather than the stack delta, so all three of its findings (FilePreviewPanel/renderers/TooltipCell.tsx, NavRail/index.tsx, AiBadge/index.tsx) are in files inherited from #1478 and untouched by this PR's commit. None of them are carried into this review. Its absence is disclosed rather than counted as agreement.
• Blind spots that remain after both passes: runtime behaviour in a real browser (only CSS computed values were measured, not interaction), the full @octo/base regression suite, and visual verification of the two segmented variants, which have no production consumer and no snapshot coverage.

[Jerry-Xin]

Supersedes my APPROVE (review 5016222397) on this head — two a11y defects independently byte-verified after @yujiwei's CR. Credit @yujiwei for both catches.

🔴 Blocking

1. Dangling aria-controls on inactive tabs — packages/octo-ui/src/components/Tabs/index.tsx renders ONLY the active panel (the single conditional role="tabpanel" div keyed by items.indexOf(selectedItem)), but every tab whose item has children sets aria-controls to ${baseId}-panel-${index}. Any inactive tab with panel content therefore references an id that does not exist in the DOM (e.g. with tab 0 active, tab 1 points at <baseId>-panel-1, which is absent). ARIA requires aria-controls to reference an element present in the DOM; assistive tech following the relation resolves nothing. Fix options: render all panels and hide inactive ones via the hidden attribute (APG-recommended pattern), or set aria-controls only where the referenced panel exists.
2. Focusable tabpanel without a visible focus indicator — the panel is focusable (tabIndex={0}) but .octo-ui-tabs__panel { outline: none; } in packages/octo-ui/src/components/Tabs/index.css with no :focus-visible rule, while .octo-ui-tabs__tab:focus-visible does get a ring. Keyboard users tabbing into the panel get no visible focus indication (WCAG 2.4.7). Fix: add a :focus-visible outline to the panel, or drop tabIndex if panel focus is not required.

My earlier verification confirmed the active-tab association (tab i ↔ panel-i ids match when active) but did not check the inactive-tab reference set or the panel focus style — that gap is mine. Both findings independently byte-verified at 615a45c.

💬 Non-blocking

Carried from the superseded review: tab strip height small→md change undeclared in the PR body; document the single-panel (unmount-on-switch) lifecycle for future form-hosting consumers; SSR id-counter note; ArrowLeft wrap + controlled-mode keyboard test gaps; mergeable_state=dirty — sync after #1478 merges.

✅ Highlights

Component API design, consumer census (ForwardModal TabsBar is the only migrated consumer, no state loss), keyboard navigation (roving tabindex, Arrow/Home/End, automatic activation), and the test suite quality all stand as described in the superseded review — only the two a11y items above block.

[mochashanyao]

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).

---

Code Review — PR #1549 (octo-web)

Reviewer: Octo-Q (automated review)
Head: 615a45cf5c4923842da7581e7156e861771bc8a5 · Base: main (merge-base 6e52f7e4) · 137 files, +4620/−614

Summary

新增共享设计系统包 @octo/ui（Tabs/Tag/Badge/Dot/Button/Tooltip 六个组件 + tokens 样式基座 + tsup 构建 + Storybook/测试），并把 dmworkbase、dmworksummary、dmworkcontacts、apps/web 中大量既有组件从 Semi UI（Tag/Badge/Tooltip/Button/Tabs）与自绘样式迁移到该包，同时补齐 turbo/CI/入口 CSS 接线与 gen:component --package ui 脚手架。迁移面非常大，但逐点核对后运行时行为契约保持得相当干净：所有未读角标/小红点的门控条件、99+ 溢出、静音态、在线态颜色映射都与旧实现等价，且新增了多个真实集成测试锁定。未发现 P0/P1；5 个 P2 见下。

Verification

静态分析为主（本环境未安装 node_modules，未执行 build/test）：

• ✅ 角标行为等价 — 所有迁移点保留原门控：wk-conv-unread-num（totalUnread > 0，静音态 class 透传到 Badge）、wk-conv-compact-badge（totalUnread > 0 && !effectiveMute）、SidebarTabBar（followUnread > 0）；Badge 默认 overflowCount=99 与旧 >99 ? "99+" 逐值等价。
• ✅ 在线/状态点颜色映射 — ClawInfoModal running→success / idle→warning / 其他→neutral 与旧 data-status CSS 规则一致；ProfileOnlineStatus online→success / offline→neutral 与旧色值一致；尺寸（6px/8px/9px/10px）经 --wk-sp-*/--octo-ui-dot-size 核对一致。
• ✅ token 依赖闭合 — octo-ui CSS 引用的 --wk-*/--semi-* 全部存在于 dmworkbase/src/theme/{primitive,semantic}.css；两个入口（apps/web/src/index.tsx、extension sidepanel）均先加载 @octo/base tokens 再加载 @octo/ui/styles.css；storybook preview 亦接入并包裹 OctoUIProvider。
• ✅ 构建/运行接线 — exports 的 development 条件使 dev/测试走源码、生产走 dist；web/extension 的 predev|prebuild|prestorybook|... 钩子先构建 @octo/ui；turbo 中 @octo/web#build 与 Octo 插件端#build 均 dependsOn @octo/ui#build，inputs 已含 packages/octo-ui/**；octo-ui CSS 无 url()（无扩展打包 origin 风险）。
• ✅ 无残留 Semi 依赖断裂 — 全仓已无 Semi <Badge> 使用，App.css 删除的 .semi-badge-count 暗色修复不残留影响；登录页等仍用 Semi Button 的 semi-button 覆盖选择器未受影响；.wk-fm-tabs 等外部 hook 类与内部 DOM 无关。
• ✅ CI 引用真实 — 新增步骤引用的包名 @octo/base/@octo/ui 与 5 个测试文件均存在于 head。
• ✅ 测试质量 — 新增测试为真实集成路径（渲染完整 ConversationList/ProfileOnlineStatus/TabsBar 等），且含真 Semi 的 Tooltip 集成测试守住"空气泡"回归；非 hand-feed。

Findings

无 P0/P1；5 个 P2（均非阻塞）。

P2 — gen:component --package ui 只登记运行时导出，不更新类型面 (scripts/gen-component.mjs:256)

isOctoUiPackage 分支把新组件 export 追加进 packages/octo-ui/src/index.ts 并注册 CSS，但从不更新手写的 packages/octo-ui/src/index.d.ts；而 package.json 的 types/exports.types 固定指向该 d.ts（tsup dts:true 产物未被用作入口）。今后生成的组件将"运行时存在、类型面缺失"，TS 消费方 import 即编译失败，且要到下次生成组件时才暴露。建议同步幂等追加 d.ts 声明，或改用构建产物 d.ts 作类型入口。

P2 — AiBadge 的 size prop 迁移后失效 (packages/dmworkbase/src/Components/AiBadge/index.tsx:14)

AiBadge/index.css 已整体删除，外观全部来自固定的 <AITag size="xs">，但组件仍接收 size 并拼接无样式可应用的 ai-badge-${size} 类名；Messages/Base/index.tsx:567,589、Messages/Base/head.tsx:116、WebhookEditModal.tsx:267 都显式传 size="small"。视觉统一本身可接受，但保留无效 prop 是契约噪声。建议把 size 映射到 AITag 尺寸，或删除该 prop 并更新调用方。

P2 — Tabs 模块级计数器生成实例 id (packages/octo-ui/src/components/Tabs/index.tsx:5)

let tabsInstance = 0 + useState(() => ++tabsInstance)：本仓库是纯客户端 SPA，当前无用户可见影响，但作为共享库该写法在 SSR/预渲染会产生水合 mismatch，StrictMode 双调用也会跳号。注意 peer 为 react >=17 <19，React 17 无 useId，不能直接照搬；建议实例内稳定生成（useRef 惰性初始化）或在文档中声明 client-only 约束。

P2 — iconOnly Button 无 aria-label 时缺可访问名称 (packages/octo-ui/src/components/Button/index.tsx:35)

iconOnly=true 时 icon 被 aria-hidden、label 被抑制，可访问名完全依赖调用方传 aria-label，类型无约束。本 PR 尚无生产调用方使用 iconOnly，无当前影响；建议参考 Tag 的判别联合写法在类型上强制（iconOnly: true ⇒ aria-label 必填）。

P2 — Tag 关闭按钮 closeAriaLabel（复核：类型已强制，仅对非 TS 用法有效）(packages/octo-ui/src/components/Tag/index.tsx:62)

独立审查腿提出"省略 closeAriaLabel 时关闭按钮无可访问名"。复核：TagProps 已是判别联合（closable: true ⇒ closeAriaLabel: string 必填），TS 消费方无法漏传，三个生产调用方（ChatSummaryNewModal/ParticipantSelector/SourceSelector）均已传值；残余风险仅限非 TS/强转用法。保留此条作为库级约定提示，可在文档中强调。

数据流回溯（被消费数据 → 上游 → 是否真流到消费点）

• followUnread/recentUnread（SidebarTabBar props）→ >0 门控 → Badge count → overflowCount=99 → 与旧 99+ 等价。✅
• totalUnread（ConversationList ← ConversationWrap.unread）→ 未读角标/紧凑角标/紧凑红点三处消费，各自原门控（含 !effectiveMute）保留；静音样式经 className 透传为 --octo-ui-badge-bg/color 覆盖。✅
• tip（OnlineStatusBadge ← getOnlineTip(channelInfo)，可为空）→ 新增空 tip 分支渲染 Dot；wk-onlinestatusbadge:not(.…-empty) 规则保证非空气泡背景不变；contacts 侧同源消费一致。✅
• activeKey（ForwardModal 状态 ChatSelectorTab）→ octo-ui Tabs 受控；onChange 只会携带来自 items 的 key（activate() 内部产生），find 守卫后回传，无键丢失。✅
• content（TooltipCell/Contacts/OverflowTooltip 单元格内容）→ hasContent && isTruncated 双门控后才挂载 Tooltip；ResizeObserver+resize+内容变化三路刷新截断态；Semi mouseEnterDelay 单位为毫秒，isDelayed→300ms 与旧 0.3s 等价。✅
• process_status（ClawInfoModal ← runtime_info）→ Dot tone 映射与旧 CSS data-status 着色规则逐项一致。✅

盲点 Checklist（C1–C6）

• C1 双路径 parity — 命中已清：src/index.ts 导出列表 ↔ styles/components.css @import 列表逐项一致（6 组件）；线程状态三态（grey/red/green）CSS 修饰类全部定义；"外部群"标签三个渲染点（ConversationList/ForwardModal ItemRow/channelSetting）样式各自落齐；Badge 99+ 溢出两端等价。
• C2 复用/顺序 — 命中已清：Badge/Dot/Tooltip 多调用点复用，逐点确认门控保留、无双重作用；wk-fm-tabs 等 hook 类与新 DOM 无耦合。
• C3 授权边界 — N/A：纯 UI 组件 PR，无权限/凭证/tool 暴露面。
• C4 授权生命周期/级联 — N/A：无容器-成员状态逻辑。
• C5 build 过 ≠ 运行期正确 — 命中已清：入口 CSS/exports/predev 钩子逐条验证（见 Verification）；octo-ui CSS 无 url()。本环境未实际跑 build/测试，如实声明。
• C6 治理/文档自洽 — N/A：未触碰治理/安全文档。
• 安全扫描（STRIDE 简扫） — 无鉴权/注入/数据暴露变更；内容全部经 React 转义渲染，无 dangerouslySetInnerHTML；gen-component 组件名先经 ^[A-Z][a-zA-Z0-9]+$ 校验再拼路径，无穿越。

跨轮 Blocker 复检（R6）

N/A — 本 PR 首轮审查，无上轮未决 blocker。

Verdict: APPROVED（automated review建议）

迁移等价性逐点核对通过，测试以真实集成路径锁定关键行为，接线（turbo/CI/入口样式/构建钩子）完整。5 个 P2 均为非阻塞的库级契约/可维护性建议，可在后续迭代处理。

[Octo-Q] verdict: APPROVE — 无 P0/P1；仅 P2（类型面脚手架缺口、AiBadge size 失效、Tabs id 生成、iconOnly a11y、closeAriaLabel 提示）。

[Jerry-Xin]

Re-review of the a11y follow-up at 6a6ddcbf (previous RC on 615a45cf). Both a11y blockers from the previous round are genuinely fixed and byte-verified; one real layout defect in the segmented variants surfaced when the repair commit was narrowed.

🔴 Blocking

1. Segmented tab list overflows its container instead of scrolling — packages/octo-ui/src/components/Tabs/index.css:55-59: the segmented / segmented-plain list override sets display: inline-flex; width: max-content, while the base list carries overflow-x: auto (line 11). Because the used width equals the max-content width, content never overflows the box, so overflow-x: auto can never activate. Empirically verified in Chromium with a 300px container and 6 tabs: segmented list clientWidth == scrollWidth == 659, scrollable: false, visually spilling past the container edge; the line variant scrolls correctly (clientWidth 300, scrollWidth 761). The PR description advertises "overflow scrolling" — that holds for line only today. The one-line max-width: 100% that existed in interim commit 34b588ed fixes it (re-measured: scrollable: true) but was reverted in 6a6ddcbf when the repair was narrowed to a11y-only. Please restore the clamp (a separate commit is fine) or otherwise constrain the list to its container. An automated overflow regression test needs real layout and is optional — jsdom cannot measure it.

Fixed from previous round (byte-verified at this head)

1. B1 — dangling aria-controls on inactive tabs: packages/octo-ui/src/components/Tabs/index.tsx:104 now sets aria-controls={isActive && hasPanel ? panelId : undefined}. Only the active tab carries aria-controls, and it always matches the single rendered panel (${baseId}-panel-${items.indexOf(selectedItem)}); the panel's aria-labelledby stays symmetric and roving tabindex is untouched. Screen-reader walk: tab 0 active → ArrowRight activates tab 1 → panel remounts with matching id/labelledby association. The new assertion in packages/octo-ui/src/components/Tabs/Tabs.test.tsx pins that an inactive tab carries no aria-controls, and no test asserts the old behavior. Credit @yujiwei for the original catch.
2. B2 — focusable panel without visible focus indicator: packages/octo-ui/src/components/Tabs/index.css:142 adds .octo-ui-tabs__panel:focus-visible with the same glow outline used by tabs, which overrides the base outline: none at higher specificity; tabIndex={0} is retained, so keyboard users now get a visible indicator on the panel (WCAG 2.4.7 satisfied). Credit @yujiwei for the original catch.

💬 Non-blocking

• Carried from the previous round (unchanged, still minor): SSR id-counter note (module-level tabsInstance; prefer useId once React 17 support is dropped); a doc note on the single-panel unmount lifecycle for future form-hosting consumers; ArrowLeft wrap + controlled-mode keyboard test gaps.
• mergeable_state is dirty — sync/rebase after #1478 lands, as planned.

✅ Highlights

• Both a11y fixes are minimal and correct; the repair commits were properly scoped — the non-a11y tweaks were reverted in 6a6ddcbf so the delta is exactly the two blockers plus the covering test assertion.
• Local verification at 6a6ddcbf: Tabs 11/11, ForwardModal TabsBar 2/2, full ForwardModal suite 151/151, full @octo/ui suite 42/42, @octo/ui typecheck clean.

Verdict: REQUEST_CHANGES — one blocking layout defect (segmented overflow); both previous a11y blockers are confirmed fixed.

[yujiawei]

Code Review — PR #1549 (octo-web)

Head reviewed: 6a6ddcbf60fdfb65ff5ae142d10949f0f661e187

Scope. This PR is stacked on #1478, so the 137-file diff against main is mostly inherited. I reviewed the incremental delta only — 980d8604 (#1478 head) → 6a6ddcbf, which is 13 files, +824/−16. Everything outside that delta is treated as pre-existing baseline.

This is the second review round on this PR. The previous round (head 615a45cf) raised two blocking a11y defects. Both are fixed and I byte-verified them on this head — see §3.

---

1. Specification compliance

Spec: ✅

PR description claim	Result
Controlled + uncontrolled Tabs with line / segmented / segmented-plain	✅ Tabs/index.tsx:25-38, Tabs/types.ts:4, Tabs/index.css:42-83
activeKey, defaultActiveKey, onChange, disabled items, keyboard navigation, optional panels	✅ all present and exercised by tests
size	⚠️ line only — see Q4
overflow scrolling	⚠️ line only — see Q3
Light/dark semantic tokens, Storybook variants, focused component tests	✅ styles/tokens.css:61-122 (light :root + dark body[theme-mode='dark'], both blocks correctly re-declare the derived color-mix values so they resolve against the right base), Tabs.stories.tsx, Tabs.test.tsx
ForwardModal Semi Tabs replaced, four filters + selection preserved	✅ TabsBar.tsx:6-13 preserves the followed / recent / group / direct key order; __tests__/TabsBar.test.tsx:49-72 asserts the label order and the click→key mapping
Native hand-written tab surfaces incl. SearchWorkspace unchanged	✅ not touched
No over-building	✅ the delta is confined to the new component and its one consumer

Test-count claims check out: Tabs.test.tsx has exactly 11 cases (9 it + a 2-case it.each), TabsBar.test.tsx has 2.

Public-surface wiring is consistent: src/index.ts:26-27, the hand-written src/index.d.ts:37,55-57, package.json files, and styles/components.css:7 all gained the Tabs entry — no half-registered export.

.wk-fm-tabs (ForwardModal.css:93-96) is a plain wrapper rule (margin + flex-shrink) with no Semi-internal selectors, so nothing is orphaned by the swap.

I am not failing the spec gate on the two ⚠️ rows: the production deliverable (the ForwardModal migration) is complete and correct, and the gaps are variant-scoped quality issues in the new primitive. They are reported below.

---

2. Code quality

Quality: Approved — no P0/P1. Everything below is P2 or a nit.

The highest-leverage item first

Q1 [P2] — the final commit backs out two CSS fixes that the previous commit in this same branch had already made.

git diff 34b588ed 6a6ddcbf shows fix(ui): narrow tabs review repair removing both of these:

 .octo-ui-tabs--segmented .octo-ui-tabs__list,
 .octo-ui-tabs--segmented-plain .octo-ui-tabs__list {
   width: max-content;
-  max-width: 100%;
 }
...
 .octo-ui-tabs__tab:focus-visible {
+  border-radius: var(--wk-r-xs);
 }
-.octo-ui-tabs--line .octo-ui-tabs__tab:focus-visible {
-  border-radius: var(--wk-r-xs);
-}

Deferring advisory feedback is a legitimate call, but here the correct fixes were already written, reviewed and committed — reverting them re-introduces Q2 and Q3 below. Both are recoverable with a cherry-pick of that hunk rather than new work, which is why I am listing them together.

Segmented variants

These three all land on segmented / segmented-plain, which have no production consumer — that is exactly why nothing caught them: no test asserts CSS, and the manual verification covered ForwardModal, which uses line.

Q2 [P2] — :focus-visible silently squares off the segmented pill.

packages/octo-ui/src/components/Tabs/index.css:130-134

.octo-ui-tabs__tab:focus-visible {
  border-radius: var(--wk-r-xs);   /* 3px */

Specificity is (0,2,0), identical to .octo-ui-tabs--segmented .octo-ui-tabs__tab (:73-79, border-radius: var(--wk-r-full) = 9999px) and .octo-ui-tabs--segmented-plain .octo-ui-tabs__tab (:81-83, var(--wk-r-sm) = 6px). Equal specificity → later source position wins → keyboard-focusing a segmented tab drops its radius to 3px. The selected pill's background is radius-clipped too, so the pill visibly changes shape on focus. Button/index.css:25-28 sets no border-radius in its :focus-visible rule; staying consistent with that (or scoping to .octo-ui-tabs--line, as 34b588ed did) fixes it.

Q3 [P2] — width: max-content makes the advertised overflow scrolling a no-op for both segmented variants.

packages/octo-ui/src/components/Tabs/index.css:11 sets overflow-x: auto on .octo-ui-tabs__list, but :55-60 then sets width: max-content for the segmented variants. A max-content-sized box is by definition never narrower than its content, so scrollWidth === clientWidth and the list can never scroll itself — it overflows its parent instead and is clipped by an ancestor. max-width: 100% (again, what 34b588ed had) restores it.

Q4 [P2] — the size prop has no effect on the segmented variants.

packages/octo-ui/src/components/Tabs/index.css:47-53 are the only size rules in the file:

.octo-ui-tabs--line.octo-ui-tabs--md .octo-ui-tabs__tab { height: var(--wk-sp-10); }
.octo-ui-tabs--line.octo-ui-tabs--sm .octo-ui-tabs__tab { height: calc(var(--wk-sp-8) + var(--wk-sp-1)); }

Both are gated on .octo-ui-tabs--line. size is a documented public prop (types.ts:3,19, exported in index.d.ts) and is wired as a Storybook radio control (Tabs.stories.tsx:17), so <Tabs variant="segmented" size="sm" /> is a silent no-op. Either add the segmented size rules or narrow the type/docs.

Theming

Q5 [P2] — light mode pins the active colour to a primitive token while dark mode uses a semantic one.

packages/octo-ui/src/styles/tokens.css:63 vs :94

:root                     { --octo-ui-tabs-active-color: var(--wk-neutral-850); }  /* #1c1c23 */
body[theme-mode='dark']   { --octo-ui-tabs-active-color: var(--wk-text-primary); }

In light mode --wk-text-primary resolves to --wk-neutral-800 = #1f2329 (dmworkbase/src/theme/semantic.css:312), not #1c1c23. So the active tab label is a shade off from every other primary-text element beside it, and a theme that re-points --wk-text-primary moves everything except the tabs — but only in light mode. Since all five derived tokens (text/hover/disabled/segmented-text/segmented-hover) are color-mixed off this one value, the drift propagates. Using --wk-text-primary in both blocks makes the two themes symmetric. Same note applies to --wk-black-alpha-06 / --wk-white-alpha-06 at :79 / :110.

Q6 [P2] — inactive-tab text contrast. Computed from the repo's own token values (primitive.css:31,41,42, semantic.css:312,515), sRGB, WCAG 2.x:

	ratio	AA (4.5:1)
light line inactive — rgba(28,28,35,.6) on #ffffff	4.47:1	✗ (by 0.03)
light segmented inactive — 40% on the 6%-black track	2.43:1	✗
dark segmented inactive — 40% on the 6%-white track	3.03:1	✗
light / dark line active	16.94 / 12.66	✓
light / dark line hover	8.89 / 8.58	✓

The first row is the shipped ForwardModal path, so it is the one I would fix regardless of the segmented work; raising the mix from 60% to 62% yields 4.77:1. (Disabled comes out at 1.92:1 / 2.41:1 — flagging for completeness only, disabled controls are exempt from 1.4.3.)

Component behaviour

Q7 [P2] — uncontrolled mode can change its own selection without ever calling onChange.

packages/octo-ui/src/components/Tabs/index.tsx:33-44. When the active item is removed or becomes disabled, resolvedKey falls back to firstEnabledKey(items) and the effect at :40-44 commits it — onChange never fires. Tabs.test.tsx:200-210 codifies exactly this. A consumer mirroring the active tab (URL sync, analytics, data fetch) desyncs silently. Either fire onChange on self-correction or state the invariant in types.ts.

Q8 [P2] — a controlled activeKey naming a disabled item produces an ARIA-inconsistent tablist.

index.tsx:34-38. resolvedKey for the controlled path is items.find(i => i.key === activeKey)?.key with no isDisabled filter (unlike the uncontrolled path at :33), so the disabled tab renders aria-selected="true" and disabled, while focusKey moves the roving tabIndex={0} onto a different tab. APG puts tabindex="0" on the selected tab. Tabs.test.tsx:181-198 currently locks this in as expected behaviour. The realistic trigger is ordinary — a permission change disables the tab the parent is still pointing at. A dev-mode console.warn, or refusing to select a disabled item, would beat encoding the degenerate state in a test.

Q9 [P2] — the panel (children) mode has zero Storybook coverage.

Both defects fixed this round lived in the children path, and Tabs.stories.tsx has no item with children — all three stories are label-only. The least-exercised branch of the component is the one that has already produced two blockers. A panels story would give it a visual/interaction surface.

Nits

• index.tsx:89 — const hasPanel = item.children !== undefined treats children: null as "has a panel". cond ? <X/> : null is a common idiom and would render an empty, focusable role="tabpanel".
• index.tsx:47 — if (!resolvedKey) return skips scrollIntoView for falsy-but-legal keys; key: "" is valid under the declared key: string. In an overflowing line tablist, programmatically activating that item can leave it off-screen.
• index.tsx:46-52 — the scrollIntoView effect also fires on mount. block: 'nearest' makes it a no-op when already visible, so this is low-risk, but a skip-first-render ref removes the class of bug where mounting scrolls an ancestor.
• index.tsx:105-106 — disabled and aria-disabled="true" are both set. APG prefers aria-disabled alone so disabled tabs stay discoverable; native disabled removes them from the focus order entirely.
• index.tsx:118,121 — items.indexOf(selectedItem) is recomputed twice; the map index is already in scope.
• __tests__/TabsBar.test.tsx:39 — the test is named "…with the compact line variant" while asserting octo-ui-tabs--md (40px). 34b588ed had this correct as "md line variant"; 6a6ddcbf changed it back. Related: TabsBar.tsx:34 passes size="md" where the replaced Semi usage was size="small", and size="sm" (36px) exists — worth confirming 40px is the intended design for a compact modal wedged between a search box and a scrolling list, rather than a default.
• TabsBar.tsx:27 — aria-label={t("base.forwardModal.title")} reuses the dialog's own title as the tablist's accessible name, so assistive tech announces the same string for the dialog and for the filter tablist. A dedicated key ("Chat filters") would be more accurate. The mocked t returns the key, so the test can't see this either way.
• styles/tokens.css:63-122 — first use of color-mix() here. Fine for the Electron target and current evergreen browsers, but on Safari < 16.2 the whole custom property is invalid at computed-value time and active/inactive tabs become indistinguishable. The existing --wk-*-alpha-* primitives would avoid that if browserslist still needs to cover it.

---

3. Verification of the previous round's blockers

Both fixed on this head, confirmed by reading the diff rather than trusting the commit message:

1. Dangling aria-controls on inactive tabs — index.tsx:104 is now aria-controls={isActive && hasPanel ? panelId : undefined}. Only the rendered panel is referenced. Regression-guarded by Tabs.test.tsx:65 (expect(third.hasAttribute("aria-controls")).toBe(false)).
2. Focusable panel with no visible focus indicator — index.css:142-145 adds .octo-ui-tabs__panel:focus-visible with the same outline treatment as the tab rule.

---

4. Merge readiness (not a code defect, but the practical blocker)

mergeStateStatus: DIRTY / mergeable: CONFLICTING. The conflict is only pnpm-lock.yaml — verified with git merge-tree --write-tree origin/main <head>, which reports that single path.

The consequence is larger than the conflict itself: no pull_request-triggered workflow has ever run on this branch. gh run list --branch feat/octo-ui-tabs returns only pull_request_target runs (PR Labeler, PR Title Lint, Check Sprint, the Dependabot gate); the CI workflow that contains the Build job has zero runs, as do Secret Scan, Dependency Review, History Check and OSS Module Guard. For comparison, #1478's branch does have those pull_request runs. GitHub cannot construct the merge ref for a conflicted PR, so none of them are dispatched.

So build, typecheck and the test suites have never executed against this head in CI — the only verification is the local runs quoted in the PR body. Rebasing to clear the lockfile conflict should be enough to unblock all of it, and is worth doing before merge regardless of this review. Note also that #1478 is still open, so this cannot land ahead of it.

---

5. Also checked and cleared

Two cross-file concerns were raised against the wider stack during this pass. Both are outside this PR's delta, and both are refuted on the current head — recording them so they don't get re-raised:

• Dot tone="online" (NavRail/index.tsx:105) does not break the build. DotTone at octo-ui/src/components/Dot/types.ts:4-10 explicitly includes 'online'.
• Tooltip does not clobber the child ref in TooltipCell.tsx:64-67. There is no cloneElement anywhere in packages/octo-ui/src; Tooltip/index.tsx is a thin passthrough over Semi's Tooltip, so ref={setCellRef} on the inner div is untouched and the ResizeObserver truncation detection keeps working.

Coverage limits, stated plainly. I could not install workspace dependencies in the review environment, so I did not execute the test suites, typecheck or build — and as noted in §4, neither did CI. Every claim above comes from reading source and from cascade/contrast computation against the repo's own token files. I also did not do visual or screenshot verification of the line variant against the Figma spec, and I did not compare Semi's old spacing/typography to the new values beyond the small→md height change.

---

6. Verdict

APPROVED.

Both blocking a11y defects from the previous round are genuinely fixed. Nothing I found on this head is P0/P1: the only production surface touched is the ForwardModal migration, which is correct, behaviour-preserving and covered by tests. Every remaining finding is either scoped to the two variants that have no consumer yet, or is advisory polish.

I am deliberately not blocking a third round on advisories the author has already chosen to defer. Q1 is the one I would most like to see picked back up before this lands, since the fix already exists in the branch — but as a follow-up, not a gate.

---

7. Suggested follow-ups, in the order I would do them

1. Cherry-pick the two CSS hunks reverted by 6a6ddcbf (Q1 → fixes Q2 + Q3). One-line each.
2. Nudge the light-mode inactive mix 60% → 62% and re-point --octo-ui-tabs-active-color at --wk-text-primary in :root (Q6 first row + Q5). This is the only finding on the shipped path.
3. Add segmented size rules, or narrow size so it can't be passed where it does nothing (Q4).
4. Add a panels story, and decide the onChange-on-self-correction and disabled-activeKey semantics before a second consumer adopts them (Q9, Q7, Q8).
5. Rename the TabsBar test to match the size it actually asserts, and confirm md vs sm for the modal (nits).

[yujiawei]

Code Review — PR #1549 (octo-web)

Head reviewed: 2459d70fcf2bd3b13fff126e729920354183b0a1

Scope. This PR is stacked on #1478, so the 137-file diff against main is mostly inherited. I reviewed the incremental delta only — 980d8604 (#1478 head) → 2459d70f, verified as a true ancestor relationship (git merge-base pr-1478 pr-1549 == 980d8604). That delta is 13 files, +825/−16 across 4 commits. Everything outside it is treated as pre-existing baseline.

This is the third review round on this PR. The previous round (head 6a6ddcbf) carried one blocking layout defect. It is fixed, and I re-measured it independently rather than taking the commit message on trust — see §3.

---

1. Specification compliance

Spec: ✅

PR description claim	Result
Controlled + uncontrolled Tabs with line / segmented / segmented-plain	✅ Tabs/index.tsx:25-38, Tabs/types.ts:4, Tabs/index.css:42-84
activeKey, defaultActiveKey, onChange, disabled items, keyboard navigation, optional panels	✅ present and exercised by tests
Overflow scrolling	✅ now true for all three variants — measured below
size	⚠️ line only — see Q2
Light/dark semantic tokens, Storybook variants, focused component tests	✅ styles/tokens.css:61-122, Tabs.stories.tsx, Tabs.test.tsx
ForwardModal Semi Tabs replaced, four filters + selection preserved	✅ TabsBar.tsx:6-13 preserves the followed / recent / group / direct order; __tests__/TabsBar.test.tsx pins label order and the click→key mapping
Native hand-written tab surfaces incl. SearchWorkspace unchanged	✅ not touched
No over-building	✅ delta is confined to the new primitive plus its single consumer

Independent checks:

• Migration is complete. grep for @douyinfe/semi-ui co-occurring with Tabs/TabPane across packages/ and apps/ returns zero matches at this head. ForwardModal was the last Semi Tabs consumer.
• i18n parity. All four label keys plus forwardModal.title (used as the tablist aria-label) exist in both locales/zh-CN.json:886-890 and locales/en-US.json:886-890.
• Public surface is fully wired, no half-registered export: src/index.ts:26-27, the hand-written src/index.d.ts:13,37,55-57, package.json files, and styles/components.css:7 all gained their Tabs entry.
• No orphaned CSS. .wk-fm-tabs (ForwardModal.css:93-96) is a plain wrapper rule (margin + flex-shrink) with no Semi-internal selectors, so the swap leaves nothing dangling.
• Style-token coverage. Every --wk-* token referenced by the new CSS resolves against packages/dmworkbase/src/theme/{primitive,semantic}.css, and both consuming entry points already import @octo/ui/styles.css (apps/web/src/index.tsx:4, apps/extension/entrypoints/sidepanel/main.tsx:4), so the new rules ship to both surfaces.

I am not failing the spec gate on the size row: the production deliverable — the ForwardModal migration — is complete and correct, and the gap is variant-scoped polish in the new primitive. It is reported below as a quality item.

---

2. Code quality

Quality: Approved — no P0/P1. Everything below is P2 or a nit, and none of it needs to block this merge.

All three P2s were measured in headless Chromium against the real stylesheet, not asserted from reading.

Q1 [P2] — :focus-visible silently squares off the segmented pill.

packages/octo-ui/src/components/Tabs/index.css:131-135

.octo-ui-tabs__tab:focus-visible {
  border-radius: var(--wk-r-xs);   /* 3px */

Specificity is (0,2,0) — identical to .octo-ui-tabs--segmented .octo-ui-tabs__tab (:74-80, --wk-r-full = 9999px) and .octo-ui-tabs--segmented-plain .octo-ui-tabs__tab (:82-84, --wk-r-sm = 6px). Equal specificity, later source position wins.

Measured on the selected segmented tab: border-radius 9999px blurred → 3px focused. The pill's background is radius-clipped too, so it visibly changes shape the moment a keyboard user lands on it. Button/index.css sets no border-radius in its :focus-visible rule — matching that, or scoping this one to .octo-ui-tabs--line, resolves it.

Q2 [P2] — the size prop is a no-op on both segmented variants.

index.css:47-53 are the only size rules in the file, and both are gated on .octo-ui-tabs--line. Measured tab heights:

	md	sm
line	40px	36px
segmented	28px	28px
segmented-plain	28px	28px

size is a documented public prop (types.ts:3,19, re-exported in index.d.ts) and is wired as a Storybook radio control (Tabs.stories.tsx:17), so <Tabs variant="segmented" size="sm" /> silently does nothing. Either add the segmented size rules or narrow the type/docs so the API doesn't over-promise.

Q3 [P2] — the scroll-into-view effect fires on mount and scrolls every scrollable ancestor, not just the tab strip.

packages/octo-ui/src/components/Tabs/index.tsx:46-52

useEffect(() => {
  if (!resolvedKey) return
  tabRefs.current[resolvedKey]?.scrollIntoView?.({ block: 'nearest', inline: 'nearest' })
}, [resolvedKey])

The dependency is [resolvedKey], which is populated on the first render, so this runs at mount — not only on a user-initiated tab change. scrollIntoView walks all scrollable ancestors, and the intent here (keep the active tab visible inside the horizontally scrolling .octo-ui-tabs__list) only needs the one.

Repro measured in Chromium: a Tabs mounted below the fold inside a 200px-tall overflow-y: auto container moved that container's scrollTop from 0 → 442 on a single scrollIntoView({block:'nearest'}) call. Today's only consumer sits at the top of a modal so nothing is visible, but this is a shared primitive being rolled out broadly — the next consumer mounted below the fold gets an unexplained page jump.

Suggested shape: skip the initial mount, and/or scroll the list container directly (list.scrollLeft = ... from the tab's offsetLeft/offsetWidth) instead of delegating to scrollIntoView.

Q4 [P2] — uncontrolled normalization changes the active tab without telling the caller.

index.tsx:40-44 re-points internalKey at firstEnabledKey(items) when the previously active item is removed or disabled. onChange is only ever called from activate() (:54-58), which this path never reaches — so the rendered selection moves but the caller is never notified. Callers that mirror the active tab into their own state while still passing defaultActiveKey (a common pattern when the tab also drives a data fetch) will silently diverge from what the user sees. Tabs.test.tsx covers that the normalization happens, but not that it stays silent. Either fire onChange on this path or document the behavior on TabsProps.

Q5 [P2] — module-level id counter can collide across bundles.

index.tsx:5,26: let tabsInstance = 0 / `octo-ui-tabs-${++tabsInstance}`. useId is genuinely unavailable here — package.json:40-41 pins the peer range to react >=17 <19 and dmworkbase still resolves React 17 — so the counter is a reasonable choice. Worth recording as a known limitation though: if @octo/ui is ever bundled twice on one page, both counters start at 0 and produce duplicate DOM ids, which breaks the aria-controls / aria-labelledby pairing rather than just looking untidy. Switch to useId when the React 17 peer is dropped.

Nits (no action needed)

• index.css:7-14 — .octo-ui-tabs__list relies on the host app's box-sizing: border-box reset for max-width: 100% to include its 4px segmented padding. It holds here (App.css:45-51 + theme/index.css), but .octo-ui-tabs__tab:26 sets box-sizing explicitly, so the list is the inconsistent one in a package that otherwise ships self-contained CSS.
• tokens.css:122-152 — 4 of the 8 dark-mode Tabs tokens are byte-identical to their :root counterparts. Custom properties resolve var() at use time, so the derived color-mix values would already pick up the dark --octo-ui-tabs-active-color without being re-declared.
• types.ts:18 — onChange?: (key: string) => void widens away the caller's key union, which is why TabsBar.tsx:29-32 has to find() its way back to ChatSelectorTab. A Tabs<K extends string> generic would let every consumer skip that dance.
• index.tsx:122 — the panel is always tabIndex={0}. APG suggests making it focusable only when it holds no focusable content; as written it adds a tab stop to every panel.
• index.css:51-53 — sm height as calc(var(--wk-sp-8) + var(--wk-sp-1)) reads as an arithmetic workaround for a missing 36px step in the spacing scale.
• index.tsx:32,96-98 — tabRefs entries are never pruned when items are removed (React nulls the value, the key stays). Harmless at these list sizes.

---

3. Previous round's blocker — verified fixed

Segmented tab list overflowed its container instead of scrolling. index.css:60 now adds max-width: 100% alongside width: max-content (:55-61). Re-measured in Chromium with a 300px container and 6 tabs:

variant	clientWidth	scrollWidth	scrollable	overflows parent
segmented	298	387	✅ true	✅ false
line	298	348	✅ true	✅ false

Both variants now scroll inside their container instead of spilling past it. Confirmed fixed. The whole round is exactly that one line, so there is no collateral risk to re-check.

---

4. Verification performed at this head

Check	Result
vitest run src/components/Tabs/Tabs.test.tsx (@octo/ui)	✅ 11/11
Full @octo/ui suite	✅ 42/42 across 7 files
Full ForwardModal suite (dmworkbase)	✅ 151/151 across 20 files
pnpm --filter @octo/ui typecheck	✅ clean
pnpm --filter @octo/ui build (tsup ESM + DTS)	✅ clean
stylelint on Tabs/index.css	✅ zero warnings (all tokens.css warnings are pre-existing Tag/Tooltip lines 7-60)
Headless-Chromium layout measurements	as tabulated above

ESLint could not be exercised — the parser fails identically on untouched baseline files in this environment, so it is a local config artifact, not a signal about this diff.

---

5. Merge mechanics

mergeStateStatus is DIRTY — the branch conflicts with main. This is expected for a stack and needs a rebase once #1478 lands; it is not a code defect and does not affect this verdict.

Note that an earlier CHANGES_REQUESTED review from another reviewer is still outstanding against the previous head 6a6ddcbf. The defect it named is fixed at this head (§3), but GitHub keeps the latest review per reviewer regardless of commit, so reviewDecision may need that reviewer to re-submit before it flips.

---

Verdict: APPROVED

The one blocking defect from the previous round is fixed and independently re-measured. Three rounds in, everything still open is P2 polish confined to segmented / segmented-plain — variants with no production consumer, which is precisely why CSS-only defects keep surfacing one at a time under manual inspection. Continuing to block on them has poor returns; they are better handled as a single follow-up that adds real layout coverage for the segmented variants rather than another round-trip here.

The shipped deliverable — the line variant and the ForwardModal migration — is correct, accessible, well-tested, and verified green end to end.

Q1–Q4 are worth a follow-up issue. Q3 in particular is the one most likely to bite a future consumer, since it is behavioral rather than cosmetic.

[Jerry-Xin]

Re-review of 2459d70f — delta from the previously reviewed head 6a6ddcbf is exactly one commit, one line: max-width: 100% restored on the segmented/segmented-plain tab lists. My open blocker from the previous round is fixed, verified empirically in a real browser rather than by reading the CSS alone.

🔴 Blocking

• None.

💬 Non-blocking

1. The other CSS fix reverted in 6a6ddcbf is still unrestored. Of the two fixes backed out by fix(ui): narrow tabs review repair (yujiwei's Q1), this head restores the max-width one but not the :focus-visible radius scoping: .octo-ui-tabs__tab:focus-visible (Tabs/index.css:131-135) still overrides the segmented pill radius. Measured empirically: keyboard-focusing a segmented tab collapses its border-radius from 9999px to 3px with :focus-visible genuinely matched. yujiwei rated this P2 and neither segmented variant has a production consumer yet, so this stays advisory — but the correct line-scoped version already exists in 34b588ed, one hunk away.
2. Rebase needed: pnpm-lock.yaml conflicts with main (mergeable_state is dirty). Consequence beyond the conflict itself: GitHub cannot construct the merge ref, so no pull_request-triggered CI (build / typecheck / unit tests) has ever run on this branch — only pull_request_target automation (labeler, title lint). The local runs below are the only execution evidence at this head.
3. Merge order: this PR is stacked on #1478, which is still open; it must not merge first.
4. SSR-stable ids: prefer useId() over the module-level counter (Tabs/index.tsx:5) — carried from the previous round.
5. Carried from yujiwei's review of this head family (all advisory): segmented size rules are line-only (prop is a silent no-op on segmented); light-mode inactive-tab contrast is marginally under AA on the shipped line path; uncontrolled self-correction never fires onChange; controlled activeKey on a disabled item yields an ARIA-inconsistent state; no Storybook story covers the panel/children mode; confirm md vs sm for the ForwardModal tab bar; give the tablist its own accessible-name key instead of reusing the dialog title. Since the segmented variants have no production consumer yet, add test/manual verification before adopting them.
6. Minor robustness note: the segmented list relies on the host app's global box-sizing: border-box reset (dmworkbase App.css) to keep its padding inside the 100% width. In an environment without that reset the list overshoots its container by the padding (measured 8px at this head). As a shared package, setting box-sizing: border-box on .octo-ui-tabs__list (or .octo-ui-tabs *) would make the component self-contained.

✅ Highlights

• B1 verified fixed, with measurements. Chromium (headless Chrome for Testing) against this head's own CSS files (Tabs/index.css + octo-ui tokens + dmworkbase primitive/semantic tokens), 300px container, 6 tabs, app's global border-box reset applied:
  • segmented: clientWidth=300, scrollWidth=414 → scrollable, accepts scrollLeft, no spill past the container, tabs not clipped. (Same harness at 6a6ddcbf: clientWidth==scrollWidth==659, not scrollable, spilling.)
  • segmented-plain: clientWidth=300, scrollWidth=416 → scrollable, no spill.
  • line: clientWidth=300, scrollWidth=372 → still scrollable (no regression).
• Previous a11y fixes intact: aria-controls is emitted only on the active tab (Tabs/index.tsx:104, regression-guarded by Tabs.test.tsx:64-65), and the focusable panel has a :focus-visible outline (Tabs/index.css:143-146).
• Local suites green at this head: @octo/ui full suite 7 files / 42 tests passed (incl. the 11-case Tabs suite); ForwardModal 20 files / 151 tests passed (incl. TabsBar 2/2).

Verdict: APPROVE. The blocking overflow defect is resolved with measurement evidence and no regression was found; the remaining items are advisory and the practical pre-merge step is the rebase that also unblocks CI.

Superseded by re-review at 2459d70 (review 5017277032): B1 overflow blocker verified fixed.

[mochashanyao]

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).

---

Reviewer: Octo-Q (automated review)

Code Review — PR #1549 (octo-web)

Summary

This PR introduces a new shared component package @octo/ui (Tabs, Tag, Badge, Dot, Tooltip, Button plus a CSS-token layer) and migrates ~20 call sites across dmworkbase, dmworksummary, dmworkcontacts, dmworkmcp, the web app, and the browser extension away from ad-hoc spans/CSS tooltips and several Semi UI usages. Build wiring (turbo dependsOn, per-script prebuild hooks, package exports with a development condition, Storybook aliases, entrypoint CSS imports) and CI coverage (octo-ui tests + targeted migration tests) are part of the change. I found no P1: the migrations preserve the existing runtime contracts, and the two issues below are contract gaps in the new shared components rather than regressions of working paths.

Verification

Static analysis only at head 2459d70f; build and tests not executed in this environment.

• ✅ Diff scope matches GitHub — local merge-base...HEAD equals the PR file list (137 files, +4628/-614, 31 commits); head SHA 2459d70fcf2bd3b13fff126e729920354183b0a1 confirmed.
• ✅ Migration data flows traced — unread-badge 99+ cap semantics identical to the old manual cap; tooltip suppression now structural (mount only when truncated && has content) replacing the old controlled-visible guards; dot/badge positioning preserved via --octo-ui-* variable overrides on retained positioning classes.
• ✅ Token/CSS coverage — every --octo-ui-* variable consumed by component CSS is defined in packages/octo-ui/src/styles/tokens.css (incl. dark-mode block); deleted CSS selectors cross-checked against TSX usage (no orphans; the retained ai-badge class names are still styled through .wk-ai-tag).
• ✅ Build/runtime wiring — @octo/ui#build is a turbo dependency of web and extension builds; every dev/build/storybook script has a pre-hook; Vite resolves the development exports condition to TS source in dev and dist in prod; both UI entrypoints (apps/web/src/index.tsx:4, apps/extension/entrypoints/sidepanel/main.tsx:4) import @octo/ui/styles.css; the extension options page renders no octo-ui components.
• ✅ Updated tests lock the new contracts — SidebarTabBar.test.tsx, ConversationList/__tests__/layout.test.tsx, onlineBadge.test.tsx, OverflowTooltip.test.tsx, ClawHealthCheckItem.test.tsx assert the 99+ cap, soft-badge classes, dot tones, and conditional tooltip mounting.

Findings

No P0/P1 issues; two P2 items and one nit below.

P2 — Tooltip actions are effectively unreachable (packages/octo-ui/src/components/Tooltip/index.tsx:128)

The shared API exposes TooltipContentConfig.actions, and the stories/tests render an interactive button inside the overlay, but the wrapper hardcodes mouseLeaveDelay={0} with spacing={6}. The overlay hides the instant the pointer leaves the trigger, so a user can never traverse the gap to click an action. No production consumer passes actions yet, so nothing breaks today, but the contract ships latently broken.

// when contentConfig?.actions is present, allow the pointer to reach the overlay
mouseLeaveDelay={contentConfig?.actions ? 150 : 0}

Use a non-zero leave delay (or expose it as a prop) whenever the content config carries actions.

P2 — Controlled Tabs render nothing for an unknown activeKey (packages/octo-ui/src/components/Tabs/index.tsx:34)

In controlled mode resolvedKey is items.find(...)?.key, so an activeKey that matches no item leaves every tab aria-selected="false" and renders no panel, while uncontrolled mode gracefully falls back to the first enabled tab. The only production consumer (ForwardModal/ui/TabsBar.tsx) always passes a valid key, but as a shared component this asymmetry invites a blank-tab regression for future callers. Fall back to firstEnabledKey(items) when the controlled key is absent, or document the requirement in TabsProps.

Nit — scrollIntoView runs on mount (packages/octo-ui/src/components/Tabs/index.tsx:48)

The effect also fires for the initial resolvedKey; with block:'nearest' this can still scroll an ancestor if the tablist first renders vertically out of view. Consider skipping the first invocation.

Things I checked that are fine

• AiBadge default children "AI" is unchanged from base; new AITag fallback labels (base.aiTag.*) exist in both locales.
• Tag tone migrations (grey→gray, violet→purple, etc.) all map to defined light-palette tones; getStatusColor's only consumer is TaskStatusBadge.tsx:12.
• OnlineStatusBadge empty-tip path: positioning/border preserved (.wk-onlinestatusbadge base rule + --octo-ui-dot-size override); muted unread badge keeps its quiet style via --octo-ui-badge-* overrides.
• closable Tags now require closeAriaLabel (union type) and stop-propagation parity with the old Semi behavior.
• ResizeObserver/window-resize listeners are disconnected/removed on unmount in all three overflow-tooltip rewrites.
• pnpm-lock.yaml additions are only the tsup build toolchain; no new runtime dependencies.

Verdict: COMMENT

No correctness, accessibility, or build-breaking regressions found; the Semi→octo-ui and CSS-tooltip migrations keep the prior observable behavior, and the build/CI wiring is thorough. The two P2 items are contract gaps in the new shared components and are non-blocking.

---

数据流回溯（Octo 附加段）

• followUnread/recentUnread/totalUnread → <Badge count> → count > overflowCount(99) ? "99+" — 与旧手写 > 99 ? "99+" 语义一致（SidebarTabBar/index.tsx:41,54、ConversationList/index.tsx:280,826），测试锁定。
• participants → Conversation 折叠会话 Tooltip content（Conversation/index.tsx:2353-2367）— 数据源未变，渲染载体由 CSS hover 浮层改为 Semi portal；.wk-fold-session-tooltip-content 样式已定义。
• scrollWidth/clientWidth → isTruncated → 条件挂载 Tooltip（TooltipCell、两处 OverflowTooltip）— ResizeObserver + resize 双通道刷新，卸载清理完整；空内容守卫保留（hasContent）。
• getStatusColor(status): TagTone → 唯一消费点 TaskStatusBadge.tsx:12 的 octo-ui Tag tone — 无残留 Semi Tag 消费方。
• activeTab: ChatSelectorTab（4 值联合）→ TabsBar items 的 4 个 key 完全同集（ForwardModal/ui/TabsBar.tsx:6-13）— 受控 key 恒有效。
• --octo-ui-tag-ai-bg/-color（AITag 背景）→ 定义于 tokens.css:50-51，经 @octo/ui/styles.css 在 web/extension 两入口加载 — 消费链闭合。

盲点 checklist（C1–C6）

• C1 双路径 parity — 已查：Tag closable 的 onClose stopPropagation 与 Semi 行为对称；三个 overflow-tooltip 重写的 observer/监听器挂载↔卸载清理成对；Tooltip inactive 分支直接返回 children 不挂任何处理器。✅
• C2 control-flow ordering / 嵌套复用 — 已查：Tabs.activate() 被 click 与 keyboard 两条路径复用，均先守卫 isDisabled/同 key，无双重触发；键盘导航 focus→activate 顺序正确。安全控件类（正则/escape）N/A（本 PR 无解析/消毒逻辑）。✅
• C3 授权边界 — N/A：无权限/tool/凭证面改动。
• C4 授权生命周期/容器级联 — N/A：无鉴权改动。
• C5 build/note ≠ 运行期 — 已查（不以"build 过"缓解）：turbo dependsOn + 全部 pre* 脚本 + exports development condition（Vite dev 走 src、prod 走 dist）+ 两个渲染入口的样式导入 + Storybook alias 直连 src；extension options 页自包含不消费 octo-ui。✅
• C6 治理/策略文档 — N/A：无治理/安全文档改动。

跨轮 blocker 复检（R6）

N/A — 本 issue 首轮审查（无上一轮 blocker 记录）。PR 自身的修复提交（resolve tabs review blockers 等）已在当前 head 一并审查。

[Octo-Q] verdict: APPROVE — 按 R1–R4：未发现任何"让能工作的路径不可用/产生用户可见错误数据/比修复前更糟"的缺陷（两条 P2 均为新共享组件的潜在契约缺口，无生产消费方触达）；零 P0/P1 → 建议 APPROVE，P2 作为可选修复项交 Opus 终审裁量。

[Jerry-Xin]

Re-review at e6c8d80 (merge of main into the shared-tabs branch).

🔴 Blocking on this head

• 🔴 CI "Unit tests" is red, caused by this PR's own migration. packages/dmworksummary/src/components/ChatSelectorModal.tsx:3 now imports Tag from @octo/ui (Semi Tag color="grey" → @octo/ui Tag tone="gray"), but packages/dmworksummary/src/components/__tests__/ChatSelectorModal.test.tsx still mocks only @douyinfe/semi-ui's Tag with a data-testid="tag" stub (line ~69). The real @octo/ui Tag renders no such testid, so re-fetches with include_archived=true and renders an Archived tag when toggled on fails with Unable to find an element by: [data-testid="tag"]. CI result at this head: 1 failed | 718 passed (719). Reproduced locally at this exact head: same test file → 1 failed | 16 passed. Fix: mock the @octo/ui Tag in that test (or assert on the rendered text instead of the Semi-only testid), then confirm the Unit tests gate goes green. Note: this breakage predates the merge (it already existed on 2459d70) but was invisible then because the pnpm-lock.yaml conflict kept CI from running; now that the merge resolved the lockfile conflict, CI runs and exposes it.

✅ Verified intact at this head

• B1 (segmented overflow) fix intact: packages/octo-ui/src/components/Tabs/index.css:55-61 — segmented/segmented-plain lists keep width: max-content; max-width: 100%, so overflow scrolls inside the container. Byte-identical to the previously approved 2459d70.
• B2 (a11y focus) fixes intact: .octo-ui-tabs__panel:focus-visible glow outline present (Tabs/index.css:143-146); tab :focus-visible outline present (:131-135); aria-controls set only on the active tab with a panel (Tabs/index.tsx:104).
• Merge from main did not touch the tabs code: git diff 2459d70f..e6c8d807 over packages/octo-ui/src/components/Tabs/** and packages/dmworkbase/src/Components/ForwardModal/** is empty — no merge-conflict regressions in the component under review. The pnpm-lock.yaml conflict that blocked CI previously is resolved (mergeable=true; pnpm install --frozen-lockfile clean).
• Local suites green for the touched component area: @octo/ui 42/42 (incl. Tabs 11), ForwardModal 151/151.

💬 Non-blocking

• 🟡 The PR scope is broader than the title suggests: it adds Button/Tag/Badge/Dot/Tooltip/Tabs primitives and migrates consumers across several packages (the ChatSelectorModal failure above is a direct consequence of one such migration). Consider updating the description or splitting unrelated migrations.
• 🟡 Tabs generates DOM IDs via module-level mutable state (packages/octo-ui/src/components/Tabs/index.tsx); prefer React's useId() for stable instance IDs.
• 🔵 Add coverage for duplicate keys, reordered items, empty item arrays, and controlled selection becoming disabled.
• 🔵 The new token file contains many literal colors despite the semantic-token guidance; consider promoting them into the theme layers.
• Advisory carried over: focus-visible radius scoping remains reverted; merge order is still behind #1478.

Verdict: REQUEST_CHANGES — the Tabs component itself is in good shape and all prior blockers remain fixed; the single blocker is the red Unit tests gate caused by the Tag migration in this same PR.

[yujiawei]

Code Review — PR #1549 (octo-web)

Head reviewed: e6c8d807a0cb5421eeb7955a32460186b7738ffd
Base: main (merge-base d31a10c3)

Scope. This PR is stacked on the still-open #1478, so the diff against main is 137 files / +4642 −616. The PR's own work is 13 files / +825 −16 (commits 615a45cf, 34b588ed, 6a6ddcbf, 2459d70f). I reviewed that delta line-by-line, and separately assessed what merging this branch into main would actually do — which is where the blocking problem is.

The current head e6c8d807 is a merge of main into the branch. It changes nothing in the tabs scope (git diff 2459d70f e6c8d807 -- packages/octo-ui packages/dmworkbase/src/Components/ForwardModal is empty), but it does pull main in, and that is what surfaced the failure below.

---

1. Spec compliance

Spec: ✅

Every claim in the PR description checks out against the diff, and I re-ran the verification commands rather than taking them on trust:

Claim	Verified
Controlled + uncontrolled Tabs, line / segmented / segmented-plain	Tabs/index.tsx:25-38, Tabs/index.css:42-84
activeKey, defaultActiveKey, onChange, size, disabled items, overflow scrolling, keyboard nav, optional panels	Tabs/index.tsx:28-52, 60-78, 104-107, 116-126
Light/dark semantic tokens	styles/tokens.css (+59); all referenced --wk-* tokens resolve in dmworkbase/src/theme/{primitive,semantic}.css — I checked all 27 individually, none dangle
Storybook variants	Tabs.stories.tsx — all three variants + disabled + badge-in-label
ForwardModal Semi Tabs replaced, four filters preserved	ForwardModal/ui/TabsBar.tsx:6-14, 26-37
Native hand-written tab surfaces (incl. SearchWorkspace) unchanged	confirmed — no such file appears in the delta
vitest run Tabs.test.tsx — 11 passed	re-ran: 11 passed
vitest run TabsBar.test.tsx — 2 passed	re-ran: 2 passed
pnpm --filter @octo/ui typecheck	re-ran: clean
pnpm --filter @octo/web build	re-ran: built in 5.09s, exit 0

• Missing: none.
• Extra/out-of-scope: none. The tabs delta is unusually tight — no opportunistic refactors rode along.
• Divergence: none within the delta.

Also verified the CSS actually reaches production: styles/components.css imports Tabs/index.css, which tsup bundles into dist/styles.css, which apps/web/src/index.tsx:4 imports. And the i18n keys used by the new aria-label (forwardModal.title, tabFollowed, tabRecent, tabAllGroups, tabAllDirects) all exist in both zh-CN.json and en-US.json.

The component itself is good work. If it were on its own base I would approve it.

---

2. Code quality

Quality: Changes-Requested

🔴 P0 — Unit tests is RED on this head, and merging into main lands the regression

CI on e6c8d807: Unit tests → FAILURE (run 33027912301 / job 98373500405). Tests 1 failed | 718 passed (719), failing package @dmwork/summary.

Reproduced locally on this exact head:

$ pnpm --dir packages/dmworksummary exec vitest run src/components/__tests__/ChatSelectorModal.test.tsx
 × re-fetches with include_archived=true and renders an Archived tag when toggled on
   TestingLibraryElementError: Unable to find an element by: [data-testid="tag"]
 Tests  1 failed | 16 passed (17)

Root cause. packages/dmworksummary/src/components/ChatSelectorModal.tsx swapped Semi's Tag for the @octo/ui one:

// packages/dmworksummary/src/components/ChatSelectorModal.tsx:2-3
-import { Checkbox, Spin, Empty, Tag } from "@douyinfe/semi-ui";
+import { Checkbox, Spin, Empty } from "@douyinfe/semi-ui";
+import { Tag } from "@octo/ui";

// :456
-<Tag size="small" color="grey">{t("summary.chatSelector.archivedTag")}</Tag>
+<Tag size="small" tone="gray">{t("summary.chatSelector.archivedTag")}</Tag>

The @octo/ui Tag emits no data-testid; Semi's did. The assertion that breaks is unchanged from main:

// packages/dmworksummary/src/components/__tests__/ChatSelectorModal.test.tsx:204
const tags = utils.getAllByTestId('tag').map((el) => el.textContent);

Causation proven, not inferred. Reverting only that one file to origin/main and re-running the same suite gives Tests 17 passed (17). Restoring it gives 1 failed again.

Why it appears now. This is a sibling-PR semantic conflict, not a defect in either change alone:

• The Tag swap comes from 158c37bbc ("unify tag styles across web surfaces", PR #1413) — an ancestor of #1478, therefore an ancestor of this branch. Not on main.
• The getAllByTestId('tag') assertion was added to main afterwards by 4aefbdc55 ("establish unit test baseline and PR gate", #1522).
• Each is green in isolation. Their union — which is exactly what this head is, after the main merge — is red.

Why it blocks this PR specifically. #1549's base is main, and of the 15 open PRs in this @octo/ui stack (#1375 → #1549) this is the only one GitHub reports as MERGEABLE — every ancestor, including #1413 and #1478, is CONFLICTING. So this PR is the one that can physically be merged, and merging it puts the red test into main.

Two ways out, either is fine:

1. Retarget this PR's base to feat/octo-ui-tooltip so it can't land ahead of its stack (see P1), and fix the assertion in whichever stack PR owns the Tag migration — either add data-testid="tag" to @octo/ui's Tag or update the test to a role/text query.
2. If the stack really is meant to land top-first, fix the assertion here so this head is green.

🟠 P1 — Base branch contradicts the PR's own stated merge order

The description says "This PR is stacked on #1478 and should be reviewed and merged after it", but the base is main, so nothing enforces that. Three consequences:

1. The stated constraint is unenforced — and, per P0, the branch is currently the only mergeable one in the stack, i.e. the ordering is not just unenforced but inverted in practice.
2. The review surface is misleading. An approve here nominally signs off 137 files, ~124 of which belong to 13 other PRs. Every review round on this PR (including mine) has narrowed to "the tabs delta only" — a reasonable coping strategy that is also precisely how the P0 above went unnoticed for four rounds.
3. The merge commit contains unreviewed conflict resolutions. e6c8d807 is an evil merge: git show --cc e6c8d807 shows hand-resolved hunks in files owned by lower stack PRs, e.g. packages/dmworkbase/src/Components/Conversation/index.tsx (+import { Tooltip } from "@octo/ui"; next to incoming +import { AtSign, UserRound } from "lucide-react";), packages/dmworkbase/src/Components/ConversationList/index.tsx, and packages/dmworksummary/src/pages/SummaryCreatePage.tsx. #1478's approval sits on 980d8604 and does not cover these. They fall outside every reviewer's declared scope on this PR, and outside #1478's too.

Retargeting the base to feat/octo-ui-tooltip fixes all three at once and makes the GitHub diff show the 13 files that are actually up for review.

🟡 P2 — non-blocking, all on the new component

a. The scroll effect fires on initial mount, not only on change — Tabs/index.tsx:46-52

useEffect(() => {
  if (!resolvedKey) return
  tabRefs.current[resolvedKey]?.scrollIntoView?.({ block: 'nearest', inline: 'nearest' })
}, [resolvedKey])

I instrumented Element.prototype.scrollIntoView and rendered <Tabs items={…} /> with no interaction: 1 call on first render, args {"block":"nearest","inline":"nearest"}. scrollIntoView walks every scrollable ancestor, not just the tab strip, and block: 'nearest' makes it a vertical scroll too. Benign in ForwardModal today (the strip is visible when the modal opens, so it no-ops), but a Tabs mounted below the fold, or inside a container still mid-entrance-transition, can yank ancestors. Suggest a first-mount guard (const mounted = useRef(false)) and/or scoping the correction to the strip by setting list.scrollLeft directly instead of delegating to the browser.

b. Duplicate key values re-introduce dangling aria-controls — Tabs/index.tsx:88, 104, 116

isActive is computed by key comparison (:88) but only selectedItem renders a panel (:116). Rendering <Tabs id="dup" items={[{key:'x',label:'X1',children:…},{key:'x',label:'X2',children:…}]} /> produces:

X1 id=dup-tab-0 aria-selected=true aria-controls=dup-panel-0
X2 id=dup-tab-1 aria-selected=true aria-controls=dup-panel-1   ← dup-panel-1 does not exist

Two selected tabs in one tablist, and a dangling aria-controls — the exact defect fixed earlier in this PR, reachable again through a caller mistake. React's duplicate-key warning is the only thing surfacing it today. A dev-mode invariant, or deriving isActive from the map index rather than the key, would close it.

c. Roving tabIndex desyncs from focus when a controlled parent declines the change — Tabs/index.tsx:76-77, 107

handleKeyDown moves focus and calls activate unconditionally. If a controlled parent doesn't honour onChange, focus and the roving tabindex diverge. Rendering with activeKey="a" pinned and pressing ArrowRight from A:

document.activeElement = B, tabindex="-1", aria-selected="false"
tabindex map: A=0, B=-1, C=-1

So tabbing out and back returns the user to A, silently losing their place. ForwardModal always honours onChange, so this is latent — but it's a sharp edge for the next consumer of a shared primitive.

d. The panel is unconditionally focusable — Tabs/index.tsx:122 (tabIndex={0}). The ARIA APG recommends making a tabpanel focusable only when it holds no focusable descendants; as written, a panel containing a form or list adds a redundant tab stop. Defensible as a deliberate simplification, but worth a comment if so.

d2. In ForwardModal the tabs control nothing — ForwardModal/ui/TabsBar.tsx:29 + Tabs/index.tsx:89, 104, 116

TabsBar builds items as { key, label } with no children, so hasPanel is false for all four, no tab gets aria-controls, and no tabpanel is rendered. The content those tabs actually filter is ItemList, a plain sibling (ForwardModal.tsx:101-102) that carries no role="tabpanel" and no aria-labelledby — I checked, ui/ItemList.tsx has no ARIA attributes at all. So assistive tech is handed four role="tab" controls with no tab-to-panel relationship.

To be fair on attribution: the pre-migration Semi markup also had four empty TabPanes, so the underlying gap predates this PR and I would not call the migration a regression. But it was the natural moment to close it. Either give ItemList role="tabpanel" + aria-labelledby pointing at the active tab (which needs Tabs to support an externally-rendered panel id), or — probably better here, since these are filters over one list rather than four distinct panels — model them as a radio/toggle group instead of a tablist.

d3. box-sizing on the tab list depends on an ambient reset from another package — Tabs/index.css:20-26, 55-67

.octo-ui-tabs__tab sets box-sizing: border-box explicitly (:26); .octo-ui-tabs__list does not, yet it carries both padding: var(--wk-sp-1) and the max-width: 100% that commit 2459d70f added to constrain segmented overflow. That fix is only correct under border-box. It happens to hold — packages/dmworkbase/src/App.css:86-89 declares html { box-sizing: border-box } with *, *:before, *:after { box-sizing: inherit }, and the Storybook preview resets too — so there is no live bug. But @octo/ui ships its own CSS and does not ship that reset, so the overflow fix silently depends on dmworkbase being loaded. One line (box-sizing: border-box on __list) makes the package self-contained.

e. aria-label reuses the dialog title — ForwardModal/ui/TabsBar.tsx:27, aria-label={t("base.forwardModal.title")} resolves to "转发" / "Forward", which is the dialog's own name. A screen reader announces "Forward, tab list" inside a dialog already called "Forward". A dedicated key naming the filter group would read better.

f. Light/dark token asymmetry — styles/tokens.css. Light sets --octo-ui-tabs-active-color: var(--wk-neutral-850) (a raw primitive, #1c1c23); dark sets it to var(--wk-text-primary) (a semantic, which in light mode is --wk-neutral-800 / #1F2329). Both are plausible, but the two branches should be consistent in kind — otherwise a future change to --wk-text-primary moves dark tabs and leaves light ones behind. Same file: --octo-ui-tabs-indicator-radius: 2px is the one hardcoded pixel value among otherwise fully tokenised properties (var(--wk-sp-0-5) is also 2px).

Checked and dismissed

Recording these so they don't get re-raised next round:

• Unstable generated ids (Tabs/index.tsx:5, 26 — module-level tabsInstance counter instead of useId). Not actionable: @octo/ui declares "react": ">=17 <19", and useId needs React 18, so the counter is the correct choice for the stated peer range. There is also no SSR path in this repo (no renderToString / hydrateRoot anywhere; apps/web is a Vite SPA), so the hydration-mismatch class of failure can't occur today.
• Segmented list overflowing its parent by its own padding. Refuted — see d3 above; the global reset makes max-width: 100% behave as intended. Worth the one-line hardening, but there is no defect.
• Focus lost when the focused tab is removed at runtime. Real DOM behaviour (focus falls to <body>), but no consumer in this repo mutates items while a tab is focused, and most tab primitives don't handle it either. Noting, not asking for it.
• A disabled controlled activeKey renders aria-selected="true" on an unfocusable button (Tabs/index.tsx:38). This is deliberate and pinned by Tabs.test.tsx:181-198. It's a slightly odd end state — selection and focus disagree — but it was settled in an earlier round and I'm not reopening it.

Test quality

Genuinely good — Tabs.test.tsx covers the controlled/uncontrolled split, disabled-item skipping, unknown controlled key, disabled active key, item removal, and the all-disabled case, and asserts real ARIA wiring rather than class names alone. Note TabsBar.test.tsx uses ReactDOM.render + react-dom/test-utils's act while Tabs.test.tsx uses createRoot — that's consistent with existing dmworkbase tests (React 17 there vs 18 in octo-ui), so it's not a defect, just worth knowing the two packages differ.

Uncovered: size="sm", and the layout claims (indicator-over-baseline, segmented overflow) which are only verifiable in a browser.

---

3. Overall verdict

CHANGES_REQUESTED

Spec ✅ but Quality Changes-Requested, and the two are an AND gate. To be explicit about attribution: the Tabs component is not the problem. The blocker is that this branch, as targeted at main, is red — and it is the only branch in its stack that can currently merge. Fix the base branch (P1) and the data-testid assertion (P0) and I expect this to go through quickly.

---

4. Suggested next steps

1. Retarget the base to feat/octo-ui-tooltip. The GitHub diff then shows 13 files instead of 137, and the stated merge order becomes enforced rather than aspirational.
2. Resolve the Tag testid regression in whichever stack PR owns the migration: either add data-testid="tag" to @octo/ui's Tag (restores parity with the Semi component it replaced, and keeps #1522's baseline test working unmodified), or migrate ChatSelectorModal.test.tsx:204 off getAllByTestId to a role/text query. The first is less churn and protects the other ~14 call sites in the stack.
3. Optional, cheap: add the P2(a) first-mount guard and the P2(b) duplicate-key invariant before this primitive picks up more consumers.

5. Additional observations

• Round count. This PR has now been through four review heads (615a45cf → 6a6ddcbf → 2459d70f → e6c8d807) with nine reviews across three reviewers. The tabs code has converged; what has not converged is the stack's relationship to main. Recommend that the next action be a decision about the stack's landing strategy (rebase-and-land bottom-up, or squash the stack into one PR) rather than another round on this diff — the same class of problem will recur on #1535, #1517, #1486 and the rest, all of which are APPROVED/CONFLICTING and all of which will hit main having drifted.
• CI is not fully green independent of the P0. e2e-p0 was still IN_PROGRESS on this head at review time, so there is no e2e signal yet. Several other checks show CANCELLED from superseded runs.

6. Coverage — what I did not check

Stating this explicitly so it isn't mistaken for a clean bill:

• I did not exercise the component in a real browser. The 2px-indicator-over-1px-baseline overlap and the segmented width: max-content + max-width: 100% overflow behaviour rest on the author's manual verification and a prior reviewer's browser check, not mine.
• I did not line-by-line review the ~124 files inherited from the 13 ancestor PRs, nor the full set of conflict resolutions in e6c8d807 beyond the structural sampling in P1.
• Dark mode was verified by reading tokens, not by rendering.
• RTL is untested, but currently moot: packages/dmworkbase/src/i18n/direction.ts:3 has const rtlLocales = new Set<Locale>() — empty — so the non-direction-aware ArrowLeft/ArrowRight handling in Tabs/index.tsx:66-69 is latent rather than a live defect. Worth revisiting if an RTL locale is ever added.
• I did spot-check a couple of the inherited migrations rather than trusting them wholesale (e.g. NavRail/index.tsx:104's <Dot tone="online" />, which is valid — Dot/index.css:54 defines .octo-ui-dot--online). That is sampling, not coverage; the ~124 inherited files remain effectively unreviewed on this PR, which is the substance of P1.

[mochashanyao]

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).

---

Code Review — PR #1549 (octo-web)

Reviewer: Octo-Q (automated review)
PR: #1549 — feat(ui): add shared tabs component
Head: e6c8d807a0cb5421eeb7955a32460186b7738ffd (merge-base d31a10c3e)
Scope: 137 files, +4642/−616 — new @octo/ui design-system package (Tabs/Tag/Badge/Dot/Tooltip/Button + token/theme CSS, tsup, vitest, storybook) and migration of many @douyinfe/semi-ui call sites across dmworkbase / dmworksummary / dmworkcontacts / dmworkmcp / web / extension, plus turbo/CI wiring.

Summary

This PR introduces a shared UI package and migrates a first batch of Semi UI usages onto it. The risky surface is (a) runtime CSS token wiring between the new package, the existing --wk-* theme system, and Semi's own CSS variables, and (b) behavior preservation in the migrated call sites (Badge 99+ cap, tooltip overflow behavior, Tabs keyboard semantics). I traced the token/data flows end-to-end against the published @douyinfe/semi-ui@2.93.0 artifacts and the migrated call sites; no blocking defect found. Two nits remain (accessibility hardening guards), and two initially suspected P2 issues were re-checked and eliminated with evidence below.

Verification

Static analysis only at head e6c8d807; build and tests not executed in this environment (node_modules not installed).

• ✅ Styles entry wired on every consuming surface: @octo/ui/styles.css imported at apps/web/src/index.tsx:4, apps/extension/entrypoints/sidepanel/main.tsx:4, and apps/web/.storybook/preview.ts:8. The subpath export ./styles.css → ./dist/styles.css exists in packages/octo-ui/package.json, and dist/styles.css is produced by the tsup onSuccess hook bundling src/styles/index.css (packages/octo-ui/tsup.config.ts:21-25).
• ✅ Build ordering: turbo @octo/web#build and the extension build dependsOn: ["@octo/ui#build"] (turbo.json); prebuild hooks in apps/web/package.json:64-67 and apps/extension/package.json:12-14 rebuild @octo/ui for all build variants; CI runs pnpm --filter @octo/ui test (.github/workflows/ci.yml:116).
• ✅ Badge 99+ cap preserved: overflowCount = 99 default (packages/octo-ui/src/components/Badge/index.tsx:9), used by SidebarTabBar and ConversationList call sites unchanged.
• ✅ i18n keys exist in both locales: aiTag.* (packages/dmworkbase/src/i18n/locales/en-US.json:1218, zh-CN.json:1218) and summary.common.delete used by all closable-Tag callers.
• ✅ All --wk-* tokens referenced by @octo/ui resolve against packages/dmworkbase/src/theme/{primitive,semantic}.css (62 used / 330 defined, zero undefined).
• ✅ Tooltip migrations (OverflowTooltip.tsx, TooltipCell.tsx, Contacts inline) consistently replace trigger="custom" + controlled visible with ResizeObserver-gated conditional render, preserving the "no empty bubble" behavior; Tooltip maps its own placement to Semi position and guards blank/disabled content.

Findings

Nit — closable Tag close button aria-label relies on caller discipline (packages/octo-ui/src/components/Tag/index.tsx:62)
The close button renders aria-label={closeAriaLabel} with no runtime fallback. Diff-scope: new (component introduced by this PR). Not a live defect: the discriminated union in Tag/types.ts:34-41 requires closeAriaLabel: string whenever closable: true, and all three current callers pass a localized label (ChatSummaryNewModal.tsx:1013, SourceSelector.tsx:67, ParticipantSelector.tsx:56). R1/R4: no working path broken, no user-visible wrong data → not blocking. Suggest a dev-mode warning or localized default to protect future non-TS callers / as any escapes.

Nit — iconOnly Button has no accessible-name guard (packages/octo-ui/src/components/Button/index.tsx:38)
With iconOnly=true the icon span is aria-hidden and the label is suppressed, so the accessible name comes solely from an aria-label passed via rest props; nothing warns when it is missing. Diff-scope: new. Non-blocking; suggest a dev-mode console.warn when iconOnly && !rest['aria-label'].

Re-checked and eliminated (initially suspected P2, evidence below)

1. "Dark-theme rgba(var(--semi-*), α) may be invalid CSS" (packages/octo-ui/src/styles/tokens.css:123 dark block) — eliminated. Verified against the published @douyinfe/semi-ui@2.93.0: lib/es/_base/base.css defines every palette var the PR consumes (--semi-grey-2/5/9, --semi-red-5/8, --semi-yellow-5/7/8, --semi-green-5/7/8, --semi-blue-5/7/8, --semi-cyan-5/7, --semi-purple-5/7/8, --semi-orange-5/7/8, --semi-pink-5/7, --semi-teal-5, --semi-indigo-5) as RGB triplets — --semi-grey-9: 28,31,35 under body, 249,249,249 under body[theme-mode='dark'] — exactly matching the PR's dark selector. base.css is auto-loaded: lib/es/index.js:1 does import './_base/base.css' and package.json sideEffects keeps it. Semi's own CSS uses the identical rgba(var(--semi-*)) pattern in 453 places. The syntax is valid and dark-mode palette inversion works as intended.
2. "Tabs keyboard focus breaks when items is empty" (packages/octo-ui/src/components/Tabs/index.tsx:45) — eliminated. With items=[] the items.map renders zero buttons, so there is nothing unreachable; with all items disabled, the buttons are natively disabled (unfocusable regardless of tabIndex). For non-empty lists the roving tabindex is correct: focusKey resolves to the selected (or first enabled) key (:38) and exactly that button gets tabIndex={0} (:107); arrow/Home/End navigation wraps over enabled items only (:60-78). Controlled/uncontrolled resolution and the sync effect (:25-44) are consistent; tests cover unknown/disabled controlled keys.

Suggestions

• Add the two dev-mode a11y guards above (Tag close label, Button iconOnly label) — cheap, prevents future silent regressions.
• Optional: packages/octo-ui/src/styles/semi-bridge.css is an empty placeholder; either delete it until needed or add a comment pointing at this verification so future readers don't re-question the --semi-* dependency.

Data-flow backtracking

• --octo-ui-tag-* (dark) ← rgba(var(--semi-*), α) ← @douyinfe/semi-ui@2.93.0 lib/es/_base/base.css (RGB triplets, both light and body[theme-mode='dark'] scopes) ← auto-loaded via lib/es/index.js:1 side-effect import. Runtime flow confirmed; no undefined-variable or invalid-color path.
• Component styles ← dist/styles.css (tsup onSuccess from src/styles/index.css) ← ./styles.css export ← imported by web entry, extension sidepanel entry, storybook preview. All consuming surfaces covered.
• Tag closeAriaLabel ← callers ← t('summary.common.delete') ← present in en-US/zh-CN locale bundles.
• AITag text ← base.aiTag.* ← present in both locales; replaces .wk-fold-session-tag in Conversation with equivalent gating.
• Badge count ← SidebarTabBar/ConversationList unchanged call sites; 99+ cap via default overflowCount.

Blind-spot checklist

• C1 dual-path parity: N/A — no add/remove or subscribe/unsubscribe pairs introduced; the only paired logic (Tabs controlled/uncontrolled) verified consistent at Tabs/index.tsx:25-44.
• C2 control-flow ordering / nested reuse: N/A — new leaf components; the Tooltip migration pattern is applied uniformly across all three call sites with the same single-pass render semantics.
• C3 authorization boundary: N/A — no endpoints/tools/credentials touched.
• C4 authorization lifecycle / container cascade: N/A — no auth logic in diff.
• C5 build-pass ≠ runtime-correct: hit → cleared by explicit runtime-path verification (did not rely on "build passes"): CSS variable chain traced to the published Semi artifacts; styles.css import verified on every consuming entry; turbo/prebuild/CI ordering verified.
• C6 governance/policy docs: N/A — no governance docs in diff.

Cross-round blocker re-check (R6)

N/A — first review round for this PR.

---

[Octo-Q] verdict: APPROVE — no P0/P1 after evidence-based adjudication; 2 nits (a11y dev-guards). Both initially suspected P2s were verified and eliminated with published-package evidence (Semi palette vars are RGB triplets auto-loaded via lib/es/index.js; Tabs empty/all-disabled lists render nothing focusable by design). Runtime contract of migrated call sites (Badge cap, tooltip overflow, i18n, styles wiring, build order) preserved.