Repository navigation
feat(web-shell): expose channel sessions in sidebar and settings #8457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
e36c361
b71723b
c587b58
2ad35da
2902406
f7085af
7b8cb27
9a89c60
75131e2
b642c44
f65f88e
3548a83
455da46
cce359a
dcf1443
3ebb5a3
06aed4c
022a8ee
9fdfc0e
3757f98
602f484
a749747
2a06b7e
6fcc12d
ad78844
146fb6a
7745971
92c02d2
84db931
ad02a53
13b771d
0aea248
06336a2
e961edd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # Web Shell channel session scope | ||
|
|
||
| ## Motivation | ||
|
|
||
| Channel runtimes already route incoming messages according to `sessionScope`, | ||
| but the Web Shell channel editor only renders platform-specific management | ||
| fields. Users can therefore configure credentials and access policy, but cannot | ||
| choose which conversations share an agent session. | ||
|
|
||
| ## Design | ||
|
|
||
| Add the existing shared `sessionScope` setting to every manageable channel type | ||
| in the daemon channel catalog. Preserve a plugin-provided field when one exists; | ||
| otherwise advertise an enum with the runtime-supported values and the plugin's | ||
| default scope. | ||
|
|
||
| Render the field in a dedicated session section of the channel editor. New and | ||
| legacy configurations show their effective default, and saving writes the | ||
| selected value through the existing channel upsert request. | ||
|
|
||
| The available scopes are: | ||
|
|
||
| - `user`: one session per sender and chat. | ||
| - `thread`: one session per routing thread, falling back to the chat. | ||
| - `chat_thread`: one session per chat and nested thread. | ||
| - `single`: one session shared by the entire channel instance. | ||
|
|
||
| ## Compatibility | ||
|
|
||
| The runtime router and persisted configuration format are unchanged. Existing | ||
| configurations without `sessionScope` retain their current plugin default until | ||
| they are edited and saved. Unmanageable channel types do not advertise the | ||
| field. | ||
|
|
||
| ## Verification | ||
|
|
||
| - Assert catalog defaults for DingTalk and GitHub. | ||
| - Assert the management store accepts every runtime-supported scope. | ||
| - Assert new and legacy editor drafts use the effective default. | ||
| - Verify the Web Shell can select and persist a non-default scope end to end. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # Web Shell channel sessions in the sidebar | ||
|
|
||
| ## Motivation | ||
|
|
||
| Daemon-managed channels create ordinary workspace sessions with | ||
| `sourceType: "channel"`, but the Web Shell sidebar intentionally requests only | ||
| the `default` session catalog. A session started from DingTalk, Feishu, or | ||
| another channel therefore cannot be opened from the sidebar even though it is | ||
| stored in the selected workspace. | ||
|
|
||
| ## Design | ||
|
|
||
| Add a two-option source switch above the sidebar's project session list: | ||
|
|
||
| - **Tasks** lists `sourceType: "default"` and remains the initial selection. | ||
| - **Channels** lists `sourceType: "channel"`. | ||
|
|
||
| The switch is shown only when the daemon advertises | ||
| `session_source_metadata`. Older daemons keep the current unfiltered request | ||
| and do not show a control they cannot support. | ||
|
|
||
| The selected source is applied consistently to active, pinned, archived, and | ||
| secondary-workspace session requests. Existing session rows, workspace | ||
| sections, grouping, search, polling, and open-session actions are reused. | ||
| Because channel sessions can be created by external messages without a Web | ||
| Shell mutation event, the expanded Channels list uses the active-session poll | ||
| interval instead of the 30-second idle interval. | ||
|
|
||
| When the selected workspace also advertises `channel_management`, the Channels | ||
| catalog joins each session's immutable channel instance name (`sourceId`) to | ||
| the current channel configuration and groups sessions by `config.type`. The | ||
| type catalog supplies the platform label, so multiple instances of the same | ||
| platform share one collapsible section. Sessions whose instance no longer | ||
| exists remain visible under Other channels. If the catalog is unavailable, the | ||
| list keeps its existing fallback instead of hiding sessions. Channel type grouping | ||
| overrides user-defined session groups in the Channels view; Tasks keeps its | ||
| existing organization behavior. Secondary workspaces resolve their own | ||
| workspace-scoped channel catalog. | ||
|
|
||
| Channel adapters still prepend their model-facing instructions and contextual | ||
| history. The daemon prompt carries the user-authored text separately as | ||
| transcript display metadata, so live and replayed Web Shell messages do not | ||
| expose that hidden context and channel session titles derive from the same | ||
| visible text. | ||
|
|
||
| ## Boundaries | ||
|
|
||
| - Channel configuration and runtime management are unchanged. | ||
| - Session source metadata and daemon list APIs are unchanged. | ||
| - Session Overview and Split View keep their existing default-session scope. | ||
| - The switch is in-memory UI state and resets to Tasks on page reload. | ||
| - Channel type classification reflects the current workspace configuration; | ||
| sessions do not persist a historical platform type. | ||
|
|
||
| ## Verification | ||
|
|
||
| - Assert the source switch is gated by `session_source_metadata`. | ||
| - Assert Tasks is initially selected and requests `sourceType: "default"`. | ||
| - Assert selecting Channels requests `sourceType: "channel"` for primary and | ||
| workspace-qualified lists. | ||
| - Assert the Channels list polls on the active-session interval. | ||
| - Assert multiple instances of one platform share a collapsible type section, | ||
| other platform sessions remain separate, pinned sessions stay in their type | ||
| section, and unmatched sessions remain under Other channels. | ||
| - Assert channel prompts preserve full model context while recording only the | ||
| user-authored text for transcript display. | ||
| - Run the sidebar and workspace-section unit tests, Web Shell build, and | ||
| TypeScript typecheck. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,6 +102,7 @@ import { | |
| CHANNEL_STARTUP_PROFILE_META_KEY, | ||
| CHANNEL_STARTUP_PROFILE_VERSION, | ||
| DAEMON_CHANNEL_DELIVERY_META_KEY, | ||
| DAEMON_PROMPT_DISPLAY_TEXT_META_KEY, | ||
| LOAD_REPLAY_BULK_MODE, | ||
| LOAD_REPLAY_HIDE_INHERITED_META_KEY, | ||
| LOAD_REPLAY_META_KEY, | ||
|
|
@@ -234,6 +235,23 @@ function sessionSourceRequestMeta( | |
| : {}; | ||
| } | ||
|
|
||
| /** | ||
| * The daemon's channel adapter carries the user-facing display projection in | ||
| * prompt meta; only channel sessions may use it — every other source echoes | ||
| * its prompt content verbatim. Single source of truth for the echo, the | ||
| * pending-entry text, and the re-arm strip so their `''`/missing-meta | ||
| * semantics cannot drift apart. | ||
| */ | ||
| function getChannelPromptDisplayText( | ||
| entry: Pick<SessionEntry, 'sourceType'>, | ||
| meta: Record<string, unknown> | null | undefined, | ||
| ): string | undefined { | ||
| const displayText = meta?.[DAEMON_PROMPT_DISPLAY_TEXT_META_KEY]; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] This trusts caller-controlled 中文: 这里会在目标为 channel 会话时信任调用方可控的 — qwen3.7-max via Qwen Code /review (v0.21.8)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 022a8ee. Caller metadata is now stripped at the daemon route. A process-local, workspace-bound channel-worker authorization promotes displayText into trusted bridge context only for the owning worker; forged ACP/Web/API metadata is ignored. Added route and bridge regression coverage for trusted and forged projections. |
||
| return entry.sourceType === 'channel' && typeof displayText === 'string' | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deferred this round: this round ran under a time-budget warning, so it fixed only the failed check and the Critical findings; this suggestion is recorded here and stays open for a follow-up round. Add a test that the memory classifier's prompts carry displayText ''. 中文说明本轮暂缓:本轮在时间预算告警下运行,仅修复了失败的检查项与 Critical 发现;此建议已在此记录,保留至后续轮次处理。补充测试验证 memory classifier 的 prompt 携带 displayText ''。 |
||
| ? displayText | ||
| : undefined; | ||
| } | ||
|
|
||
| function isDefinitiveAcpRequestError(error: unknown): boolean { | ||
| if (error instanceof RequestError) return true; | ||
| if (!isRecord(error)) return false; | ||
|
|
@@ -930,15 +948,21 @@ function echoPromptToSessionBus( | |
| // contract — cheaper than a thrown `TypeError` mid-echo. | ||
| const prompt = req.prompt; | ||
| if (!Array.isArray(prompt) || prompt.length === 0) return; | ||
| const displayText = getChannelPromptDisplayText(entry, req._meta); | ||
| let displayTextPublished = false; | ||
| const serverTimestamp = Date.now(); | ||
| const blockCount = Math.min(prompt.length, MAX_ECHO_CONTENT_BLOCKS); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] The echo traversal is capped before finding the projected text block. With 256 non-text blocks followed by text, the full prompt reaches the agent but live transcript never receives the display projection, diverging from persisted replay. Publish projection independently or reserve a slot for the first text block. 中文: 回显遍历在找到投影文本块之前就受数量上限限制。若前 256 个都是非文本块、之后才是文本块,完整提示会到达 agent,但实时会话记录收不到展示文本,与持久化重放不一致。请独立发布投影,或为首个文本块预留回显位置。 — qwen3.7-max via Qwen Code /review (v0.21.8)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 022a8ee. The capped echo now reserves a slot for the first text block found after the 256-block boundary when a display projection exists. Added coverage for 256 non-text blocks followed by hidden text and verified that the visible projection is emitted. |
||
| for (let i = 0; i < blockCount; i += 1) { | ||
| const part = prompt[i]; | ||
| if (!part || typeof part !== 'object' || Array.isArray(part)) continue; | ||
| // Every `ContentBlock` variant (text, image, audio, resource) is | ||
| // published to the bus verbatim. The SDK's `normalizeDaemonEvent` | ||
| // accepts any `content` shape; rich rendering of non-text blocks is | ||
| // the consumer's responsibility. | ||
| let displayPart = part; | ||
| if (displayText !== undefined && part.type === 'text') { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Please add tests for an intentionally empty 中文: 建议补充 — qwen3.7-max via Qwen Code /review (v0.21.8)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 022a8ee. Added coverage for an intentionally empty displayText with multiple hidden text blocks, verifying that no hidden text chunk is published while non-text content remains visible. |
||
| if (displayTextPublished) continue; | ||
| displayTextPublished = true; | ||
|
Comment on lines
+1403
to
+1405
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deferred this round: this round ran under a time-budget warning, so it fixed only the failed check and the Critical findings; this suggestion is recorded here and stays open for a follow-up round. Add a direct-path multi-block test pinning that the display projection replaces only the first text block. 中文说明本轮暂缓:本轮在时间预算告警下运行,仅修复了失败的检查项与 Critical 发现;此建议已在此记录,保留至后续轮次处理。补充直接路径多块测试,固定 display projection 仅替换第一个 text block。 |
||
| displayPart = { ...part, text: displayText }; | ||
| } | ||
| // Non-text blocks are published verbatim. Channel text uses the display | ||
| // projection so hidden model context never reaches transcript consumers. | ||
| try { | ||
| entry.events.publish({ | ||
| type: 'session_update', | ||
|
|
@@ -947,7 +971,7 @@ function echoPromptToSessionBus( | |
| sessionId: req.sessionId, | ||
| update: { | ||
| sessionUpdate: 'user_message_chunk', | ||
| content: part, | ||
| content: displayPart, | ||
| // `_meta` lives inside the `update` object rather than at | ||
| // envelope level. `_meta` is a standard JSON-RPC/MCP extension | ||
| // field permitted alongside spec fields, the SDK normalizer | ||
|
|
@@ -5471,7 +5495,9 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { | |
| promptId, | ||
| queuedAt, | ||
| ...(originatorClientId !== undefined ? { originatorClientId } : {}), | ||
| text: extractPromptText(req.prompt), | ||
| text: | ||
| getChannelPromptDisplayText(entry, req._meta) ?? | ||
| extractPromptText(req.prompt), | ||
| abortController: pendingAbort, | ||
| state: isQueued ? 'queued' : 'running', | ||
| }; | ||
|
|
@@ -5655,6 +5681,10 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { | |
| copy._meta && typeof copy._meta === 'object' | ||
| ? { ...copy._meta } | ||
| : {}; | ||
| const promptDisplayText = getChannelPromptDisplayText( | ||
| entry, | ||
| meta, | ||
| ); | ||
| delete meta[DAEMON_RETRY_META_KEY]; | ||
| delete meta[INVOCATION_CONTEXT_META_KEY]; | ||
| delete meta[PRIVATE_PARENT_CAPABILITY_META_KEY]; | ||
|
|
@@ -5663,6 +5693,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { | |
| // below) re-arms it after this strip. | ||
| delete meta[DAEMON_CONTINUE_META_KEY]; | ||
| delete meta[DAEMON_CHANNEL_DELIVERY_META_KEY]; | ||
| delete meta[DAEMON_PROMPT_DISPLAY_TEXT_META_KEY]; | ||
| if (isRetry) { | ||
| meta[DAEMON_RETRY_META_KEY] = true; | ||
| } | ||
|
|
@@ -5673,6 +5704,10 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { | |
| meta[DAEMON_CHANNEL_DELIVERY_META_KEY] = | ||
| context.channelDelivery; | ||
| } | ||
| if (promptDisplayText !== undefined) { | ||
| meta[DAEMON_PROMPT_DISPLAY_TEXT_META_KEY] = | ||
| promptDisplayText; | ||
| } | ||
|
Comment on lines
+7439
to
+7442
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I am leaving this mutation-only negative assertion out of this PR: the conditional spread keeps the metadata property absent when no projection is supplied, and the production behavior in scope is covered by the defined-projection and downstream fall-through tests.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Re-checked this round and keeping the earlier disposition: the conditional spread keeps the metadata property absent when no projection is supplied, and over the JSON-RPC wire an 中文说明本轮复查后维持此前结论:条件展开保证未提供投影时 metadata 属性缺失,且经 JSON-RPC 传输时
Comment on lines
+7439
to
+7442
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deferred this round: this round ran under a time-budget warning, so it fixed only the failed check and the Critical findings; this suggestion is recorded here and stays open for a follow-up round. Add a re-arm test proving the display key is restored from the trusted context, not caller meta. 中文说明本轮暂缓:本轮在时间预算告警下运行,仅修复了失败的检查项与 Critical 发现;此建议已在此记录,保留至后续轮次处理。补充 re-arm 测试,证明 display key 从可信上下文恢复而非调用方 meta。 |
||
| meta[INVOCATION_CONTEXT_META_KEY] = invocationContext; | ||
| if (Object.keys(meta).length > 0) { | ||
| copy._meta = meta; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferred this round: this round ran under a time-budget warning, so it fixed only the failed check and the Critical findings; this suggestion is recorded here and stays open for a follow-up round. Assert the session-bus echo for a forged display key on a non-channel session.
中文说明
本轮暂缓:本轮在时间预算告警下运行,仅修复了失败的检查项与 Critical 发现;此建议已在此记录,保留至后续轮次处理。为非 channel 会话上伪造的 display key 补充 session-bus echo 断言。