Skip to content

fix(web-shell): isolate history and session drafts - #7810

Merged
wenshao merged 11 commits into
QwenLM:mainfrom
chiga0:codex/web-shell-session-history-drafts
Jul 28, 2026
Merged

fix(web-shell): isolate history and session drafts#7810
wenshao merged 11 commits into
QwenLM:mainfrom
chiga0:codex/web-shell-session-history-drafts

Conversation

@ytahdn

@ytahdn ytahdn commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR scopes prompt history to the effective workspace while retaining the legacy unscoped history as a one-time fallback for workspaces that do not yet have their own entries. It also resets history navigation whenever the active session or workspace changes.

Unsent plain-text composer drafts are persisted per session, or per target workspace on the New Task page, with a two-second idle delay and flushes when the composer loses focus, changes identity, unmounts, or the page is hidden. A draft that is entered before workspace discovery completes remains authoritative over an older persisted draft.

Accepted delayed submissions, including queued prompts and the first prompt that creates a session, commit history exactly once without clearing newer input or another session's composer. Host-gated submissions are cancelled if their source session or target workspace changes while approval is pending.

Equivalent workspace-list and Git-status refreshes preserve the composer-facing references so background polling does not cause avoidable composer rerenders.

Why it's needed

Prompt history previously leaked across workspaces, queued and delayed submission paths could leave history or composer state inconsistent, and history navigation retained stale cursor state after switching sessions. Unsent input was also lost when users left a session or returned to a New Task page.

These issues are especially disruptive in multi-workspace environments, where recalling or sending content in the wrong workspace can be confusing and potentially unsafe. Persisting scoped drafts and validating delayed submissions against their original source keeps each composer tied to the context in which the user authored it.

Reviewer Test Plan

How to verify

  1. Submit different prompts in two workspaces and confirm ArrowUp only recalls the active workspace's history. For a workspace without scoped history, confirm legacy history remains available and is migrated after the next accepted prompt.
  2. Enter unsent text in two sessions, switch between them, and confirm each session restores its own draft. Repeat on the New Task page, including entering text before the target workspace finishes resolving.
  3. Queue a prompt or submit the first prompt from New Task and confirm the accepted prompt enters history exactly once. Type newer text before delayed acceptance and confirm it is not cleared.
  4. Delay the host pre-submit callback, switch the session or target workspace, then resolve the callback. Confirm the original prompt is not sent and remains available in its source composer.
  5. Refresh an equivalent workspace list or Git status and confirm the composer remains responsive and receives stable equivalent props.

Evidence (Before & After)

Before: prompt history was shared across workspaces, session navigation state leaked across session switches, and unsent composer text was not restored. Delayed acceptance could also act on a composer whose source context had changed.

After: history and drafts follow their workspace/session identity, delayed acceptance is source-safe, and equivalent background refreshes do not churn composer props.

Tested on

OS Status
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

Environment (optional)

Local Node.js workspace. Verified the Web Shell production build, TypeScript check, ESLint, Prettier, and 231 targeted unit tests covering the application composer and both CodeMirror and mobile textarea backends.

Risk & Scope

  • Main risk or tradeoff: draft persistence uses browser local storage and intentionally stores only plain editor text.
  • Not validated / out of scope: composer tags, images, and other transient UI state are not persisted; shell-command history remains global.
  • Breaking changes / migration notes: none. Legacy prompt history is read as a fallback and copied into the workspace-specific history on the next accepted prompt.

Linked Issues

N/A

中文说明

本 PR 的改动

本 PR 将提示词历史按实际 workspace 隔离;对于尚无独立历史的 workspace,仍会一次性回退读取旧的全局历史,并在下一次提示词被接受时迁移到 workspace 专属存储。同时,切换 session 或 workspace 时会重置历史导航状态。

未发送的纯文本输入会按 session 持久化;在“新建任务”页面则按目标 workspace 持久化。写入采用两秒空闲延迟,并会在输入框失焦、上下文切换、组件卸载或页面隐藏时立即刷新。即使用户在 workspace 尚未解析完成前输入,新输入也会优先于旧的持久化草稿。

延迟接受的提交(包括排队提示词和创建 session 的首条提示词)只会写入历史一次,并且不会清空更新的输入或其他 session 的输入框。当宿主的提交前回调仍在等待时,如果来源 session 或目标 workspace 已发生变化,该提交会被取消。

当 workspace 列表或 Git 状态刷新后内容等价时,会保持传给输入组件的引用稳定,避免后台轮询引发不必要的输入组件重渲染。

为什么需要

此前提示词历史会在不同 workspace 之间串用;排队和延迟提交可能造成历史或输入框状态不一致;切换 session 后历史导航还会残留旧游标。用户离开 session 或返回“新建任务”页面时,未发送的输入也会丢失。

这些问题在多 workspace 环境中尤其明显:召回或发送了错误 workspace 的内容不仅容易混淆,也可能带来安全风险。按上下文保存草稿,并在延迟提交时校验原始来源,可以确保每个输入框始终绑定到用户创作内容时所在的上下文。

Reviewer 测试计划

验证方式

  1. 在两个 workspace 中分别发送不同提示词,确认 ArrowUp 只召回当前 workspace 的历史。对于没有独立历史的 workspace,确认旧全局历史仍可用,并在下一条提示词被接受后迁移。
  2. 在两个 session 中分别输入但不发送,来回切换并确认各自草稿可以恢复。在“新建任务”页面重复验证,并覆盖目标 workspace 尚未解析完成前就开始输入的情况。
  3. 排队一条提示词,或在“新建任务”中发送首条提示词,确认接受后只写入历史一次。在延迟接受前输入新内容,确认新内容不会被清空。
  4. 延迟宿主的提交前回调,切换 session 或目标 workspace 后再放行,确认原提示词不会发送,并仍保留在来源输入框中。
  5. 刷新内容等价的 workspace 列表或 Git 状态,确认输入框保持流畅,并收到引用稳定的等价属性。

前后对比证据

修复前:提示词历史在 workspace 间共享,session 切换后历史导航状态残留,未发送文本无法恢复;延迟接受还可能作用到来源上下文已经变化的输入框。

修复后:历史和草稿跟随 workspace/session 身份,延迟接受具备来源安全校验,等价的后台刷新不会造成输入组件属性抖动。

测试平台

OS 状态
🍏 macOS
🪟 Windows N/A
🐧 Linux N/A

环境(可选)

本地 Node.js workspace。已验证 Web Shell 生产构建、TypeScript 检查、ESLint、Prettier,以及覆盖应用输入框、CodeMirror 和移动端 textarea 后端的 231 个定向单元测试。

风险与范围

  • 主要风险或权衡:草稿持久化使用浏览器 localStorage,并且有意只存储纯文本输入。
  • 未验证或不在范围内:输入标签、图片及其他临时 UI 状态不会持久化;Shell 命令历史仍为全局共享。
  • 破坏性变更或迁移说明:无。旧提示词历史会作为回退读取,并在下一条提示词被接受时复制到 workspace 专属历史。

关联 Issue

N/A

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on 0cba29b and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— 0cba29b 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: the issues described are inherent to the current design — a single global storage key for prompt history means it leaks across workspaces by construction, and there is no draft persistence at all. These are not theoretical; any user with multiple workspaces hits them. No linked issue, but the problems are self-evident from the architecture.

Direction: well-aligned. Workspace isolation for history and drafts is a natural requirement for the multi-workspace web shell. The delayed-submission source validation is a real correctness gap that this closes.

Size: not applicable — no core paths touched. All changes are in packages/web-shell/client/.

Approach: the scope feels right. History isolation, draft persistence, delayed-submission safety, and stable props are each needed for the stated goal and interlock (e.g. draft persistence needs the identity-switch effect, which also resets history navigation). The stable-props work (isSameGitStatus, composerWorkspaces memoization) is small and directly supports the "background polling doesn't churn the composer" goal. Tests are substantial (865 lines) and cover the key edge cases.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:所描述的问题是当前设计的固有缺陷——全局单一存储键意味着提示词历史必然在 workspace 间泄漏,且完全没有草稿持久化。这些不是理论问题;任何使用多 workspace 的用户都会遇到。没有关联 issue,但问题从架构上就是显而易见的。

方向:对齐。按 workspace 隔离历史和草稿是多 workspace Web Shell 的自然需求。延迟提交的来源校验是一个真实的正确性缺口,本 PR 将其关闭。

规模:不适用——未触及核心路径。所有改动在 packages/web-shell/client/ 内。

方案:范围合理。历史隔离、草稿持久化、延迟提交安全性、稳定属性各自都是目标所必需的,且相互关联(例如草稿持久化需要身份切换 effect,而该 effect 同时重置历史导航)。稳定属性的工作(isSameGitStatuscomposerWorkspaces 记忆化)体量小,直接支持"后台轮询不抖动输入组件"的目标。测试充实(865 行),覆盖了关键边界情况。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: I would have done essentially what this PR does — derive a workspace-scoped storage key, reload history on key change with a legacy fallback, persist drafts keyed by session/workspace with debounced writes, and validate delayed submissions against their source context. The PR matches this closely.

Findings: no critical blockers, no convention violations.

The implementation is well-structured. A few things worth noting:

  • The commitAccepted callback correctly captures the full submission identity (session, history key, editor doc, tags, images) and uses it to decide whether to clear the editor or just push to history out-of-band. The composerUnchanged check (doc identity via ===, tag/image array identity, shell mode) is a clean way to avoid clobbering newer input after delayed acceptance.
  • The composerSourceVersionRef counter in App.tsx is a good belt-and-suspenders guard alongside the session/workspace checks — it catches workspace switches that start before the async onSubmitBefore resolves but haven't yet updated connectionRef.
  • The commitComposerAccepted condition change from deferComposerCommit to clearComposerOnPromptStart is a subtle correctness fix: the old code would drop the commit callback when deferComposerCommit was false even if the composer was being cleared because sessionId was undefined (new-task path).
  • The handleSubmitRef pattern (ref instead of dependency) stabilizes handleEditorSubmit without changing behavior — standard React callback stabilization.
  • Draft persistence correctly handles the unscoped-to-scoped transition: text typed before workspace discovery completes stays authoritative over an older persisted draft (adoptUnscopedInMemoryDraft).
  • The isSameGitStatus comparison intentionally omits computedAt — the test verifies that a refresh with a new timestamp but identical status fields preserves the reference.

Tests cover the important paths: workspace isolation, legacy fallback migration, session-switch draft restore, delayed acceptance (same session, cross-session, cross-workspace, queued), unscoped draft priority, history search reset, and stable-props verification. 231 targeted tests per the author's claim; the test diff adds ~865 lines across App.test.tsx and useComposerCore.dom.test.tsx.

Testing

This is an unattended CI run — no local build or tmux testing was performed. Evidence below is from the PR's own CI checks on commit 0cba29ba80573e18b6793cf6abcc08c099426958.

Pre-checks (lint, format, build) passed. The main unit test suite and web-shell visual capture are still running.

Final CI results for 0cba29b (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Not verified: live TUI behavior (CI run — no tmux). The web-shell is a browser-based UI, so a maintainer can check the PR out locally and drive it in a browser, or trigger the isolated @qwen-code /tmux job if the author has write access.

中文说明

代码审查

独立方案: 我的做法与本 PR 基本一致——按 workspace 派生存储键、在键变化时重新加载历史并回退旧全局键、按 session/workspace 持久化草稿并防抖写入、在延迟提交时校验来源上下文。PR 的实现与此高度吻合。

发现: 无关键阻塞项,无规范违反。

实现结构良好。几个值得注意的点:

  • commitAccepted 回调正确捕获了完整的提交身份(session、历史键、编辑器文档、标签、图片),并据此决定是清空编辑器还是旁路写入历史。composerUnchanged 检查(通过 === 比较文档身份、标签/图片数组身份、shell 模式)是在延迟接受后避免覆盖更新输入的干净方式。
  • App.tsx 中的 composerSourceVersionRef 计数器是 session/workspace 检查之外的双重保险——它捕获在异步 onSubmitBefore 解析前已开始但尚未更新 connectionRef 的 workspace 切换。
  • commitComposerAccepted 条件从 deferComposerCommit 改为 clearComposerOnPromptStart 是一个微妙的正确性修复:旧代码在 deferComposerCommit 为 false 时会丢弃 commit 回调,即使因为 sessionId 为 undefined(新建任务路径)而正在清空输入框。
  • 草稿持久化正确处理了从无范围到有范围的过渡:workspace 发现完成前输入的文本优先于旧的已持久化草稿(adoptUnscopedInMemoryDraft)。
  • isSameGitStatus 比较有意省略了 computedAt——测试验证了时间戳变化但状态字段相同时引用保持不变。

测试覆盖了重要路径:workspace 隔离、旧历史回退迁移、session 切换草稿恢复、延迟接受(同 session、跨 session、跨 workspace、排队)、无范围草稿优先级、历史搜索重置、稳定属性验证。

测试

本次为无人值守 CI 运行——未执行本地构建或 tmux 测试。以下证据来自 PR 自身在 commit 0cba29ba80573e18b6793cf6abcc08c099426958 上的 CI 检查。

预检查(lint、格式化、构建)已通过。主单元测试套件和 web-shell 视觉捕获仍在运行中。

未验证:实际 TUI 行为(CI 运行——无 tmux)。Web Shell 是基于浏览器的 UI,维护者可以在本地检出 PR 并在浏览器中操作。

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — solid, well-tested implementation that solves real multi-workspace problems; only reservation is CI hasn't finished yet.

This PR does what it sets out to do, cleanly. The history isolation is the right fix for a design that was always going to leak in multi-workspace environments — scoping the storage key by workspace cwd is the obvious path, and the legacy fallback migration is handled gracefully. The draft persistence is well-scoped (plain text only, debounced, flushed on all the right lifecycle events) and the unscoped-to-scoped transition handles the workspace-discovery race correctly.

The delayed-submission validation is the part I scrutinized most. The triple check (session ID, workspace cwd, source version counter) is thorough without being over-engineered — each catches a different timing window. The commitAccepted callback's identity comparison (doc identity, tag/image array identity, shell mode) is a clean way to preserve newer input after a delayed commit lands.

The stable-props work is small but earns its place — background git-status polling and workspace-list refreshes would otherwise rerender the composer on every cycle, which is exactly the kind of thing that makes a web UI feel janky.

No linked issue, but the problems are architectural rather than anecdotal — a single global storage key leaks by construction. The test suite covers the edge cases I care about: cross-session delayed acceptance, unscoped draft priority, history search reset on switch, and the stable-props invariants.

Approval deferred until CI lands green on 0cba29ba80573e18b6793cf6abcc08c099426958.

中文说明

置信度:4/5 —— 实现扎实、测试充分,解决了真实的多 workspace 问题;唯一的保留是 CI 尚未完成。

本 PR 干净地完成了其目标。历史隔离是对多 workspace 环境下必然泄漏的设计的正确修复——按 workspace cwd 限定存储键是显而易见的路径,旧历史回退迁移也处理得很优雅。草稿持久化范围恰当(仅纯文本、防抖、在所有正确的生命周期事件上刷新),无范围到有范围的过渡正确处理了 workspace 发现的竞态。

延迟提交校验是我审查最仔细的部分。三重检查(session ID、workspace cwd、来源版本计数器)全面而不过度工程化——每个捕获不同的时间窗口。commitAccepted 回调的身份比较(文档身份、标签/图片数组身份、shell 模式)是在延迟提交落地后保留更新输入的干净方式。

稳定属性的工作体量小但有其价值——后台 git 状态轮询和 workspace 列表刷新否则会在每个周期重渲染输入组件,这正是让 Web UI 感觉卡顿的那类问题。

没有关联 issue,但问题是架构性的而非轶事性的——单一全局存储键从构造上就会泄漏。测试套件覆盖了我关心的边界情况:跨 session 延迟接受、无范围草稿优先级、切换时历史搜索重置、稳定属性不变量。

审批推迟到 CI 在 0cba29ba80573e18b6793cf6abcc08c099426958 上全绿后执行。

Qwen Code · qwen3.8-max-preview

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 944c95f. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

ℹ️ No screenshot changed against the PR base — but this PR edits 3 render-shaping files:

  • packages/web-shell/client/App.tsx
  • packages/web-shell/client/components/ChatEditor.tsx
  • packages/web-shell/client/components/ChatPane.tsx

Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to packages/web-shell/client/e2e/visuals/screenshots.spec.ts that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship — CI landed green after the review. ✅

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline.

中文说明

已审查——无阻断问题。 建议见行内评论。

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/hooks/useComposerCore.ts

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.

中文说明

已审查——无阻断问题。 建议见行内评论。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/hooks/useComposerCore.ts
Comment on lines +53 to +55
if (fallbackStorageKey && readHistory(storageKey).length === 0) {
saveHistory(storageKey, history);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Legacy fallback migration copies all global history entries into the first workspace that submits — Concrete cost: pushInputHistoryEntry (and the symmetric push callback at lines 113–122) calls loadHistory(storageKey, fallbackStorageKey). When the workspace key is empty, it reads the entire legacy global history and writes it to the workspace key along with the new entry. This means a workspace's scoped history is seeded with entries from all other workspaces that were submitted under the global key, partially defeating the workspace isolation this PR introduces.

For example: user has legacy history ["a", "b", "c"] (from various workspaces). First submission "hello" in workspace /proj/X produces ["a", "b", "c", "hello"] in the workspace-scoped key. ArrowUp then shows "c" — a prompt from a different workspace.

When the primary key is empty (first submit in the workspace), consider saving only [text] to the workspace key instead of the full fallback-seeded array. This preserves the workspace isolation guarantee while still recording the user's submission.

中文说明

[Suggestion] Legacy 回退迁移会将所有全局历史条目复制到第一个提交的工作区 — 具体成本:pushInputHistoryEntry(以及对称的 push 回调,第 113–122 行)调用 loadHistory(storageKey, fallbackStorageKey)。当工作区键为空时,它会读取整个 legacy 全局历史,并与新条目一起写入工作区键。这意味着一个工作区的隔离历史会被来自所有其他工作区的全局键条目所污染,部分削弱了本 PR 引入的工作区隔离效果。

例如:用户有 legacy 历史 ["a", "b", "c"](来自不同工作区)。在工作区 /proj/X 中首次提交 "hello" 后,工作区键中产生 ["a", "b", "c", "hello"]。ArrowUp 会显示 "c" — 这是来自另一个工作区的提示词。

建议当主键为空(工作区首次提交)时,仅将 [text] 保存到工作区键,而非完整的回退填充数组。这样既保持了工作区隔离保证,又记录了用户的提交。

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Declined — the fallback seeding is the intentional migration path: when workspace-scoped history is empty, the legacy global history is used so users don't lose their pre-existing prompts. Saving only [text] on first submit would discard the user's legacy history from ArrowUp navigation in the new workspace, which is a product-level migration tradeoff rather than a bug. The current behavior is consistent between pushInputHistoryEntry and the push callback.

中文说明

已拒绝 — 回退填充是有意为之的迁移路径:当工作区级历史为空时,使用 legacy 全局历史,以确保用户不会丢失已有的提示词。首次提交时仅保存 [text] 会导致用户在新工作区中通过 ArrowUp 无法访问 legacy 历史,这是产品层面的迁移权衡,而非 bug。当前行为在 pushInputHistoryEntrypush 回调之间保持一致。

@ytahdn
ytahdn force-pushed the codex/web-shell-session-history-drafts branch from 0cba29b to 7a2afa5 Compare July 27, 2026 08:23
@github-actions

Copy link
Copy Markdown
Contributor

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。

@ytahdn

ytahdn commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Jul 27, 2026

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI still running。 已审查。

— qwen3.7-max via Qwen Code /review

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.

中文说明

未审查:反向审计——有审计 agent 运行并打开了自己的 brief,但没有 agent 是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的,agent 实际被要求做的并不是本 skill 所认证的内容。

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/hooks/useComposerCore.ts
Comment thread packages/web-shell/client/hooks/useComposerCore.ts
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

chiga0
chiga0 previously approved these changes Jul 27, 2026

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review — fix(web-shell): isolate history and session drafts

Verdict: APPROVE. Independent blind review found no Critical/Major issues. The scoping redesign is sound: identity snapshots validate delayed commits against the submission-time session/workspace, the legacy global history key is treated as a read-only fallback (copy-on-first-push, never deleted — no data-loss risk), and the draft-flush lifecycle (2s debounce with deadline, blur, unmount, visibilitychange, pagehide, identity change) covers the exit paths comprehensively. Two Minor items inline, plus Low/Nit observations below — none blocking.

Cross-validation

Finding Reviewer My assessment
Critical: historyBrowseActiveRef not reset when the !composerUnchanged early return fires → "draft saves suppressed for subsequent typing" (useComposerCore.ts:2236) qwen-code-ci-bot False positive — verified against code. The scenario requires the user to edit the composer between submit and accept, but any genuine edit (input/delete user event) synchronously clears historyBrowseActiveRef in the CodeMirror updateListener (lines 2689-2694) during dispatch — strictly before commitAccepted can run, so the flag is already false when the early return fires. Even if it were true (via a non-doc change such as a shell-mode toggle), the consequence cannot materialize: the next keystroke itself clears the flag before the debounced save (flushDraftAfterIdle) fires, and navigatePrevHistory/navigateNextHistory are gated on disabledRef while a submission is pending, so browse mode cannot be re-entered in that window. The reset at line 2244 is belt-and-braces for the unchanged-composer path, which works correctly.
Suggestion: visibilitychange/pagehide flush lacks test coverage qwen-code-ci-bot Agree — every other flush trigger has a dedicated test; these two listeners are the only gap. Test-only.
Suggestion: mobile onBlur draft save lacks test coverage doudouOUC Agree — the desktop blur path is exercised via blurEditor(), but the mobile textarea's onBlur isn't wired into the mobile harness. Test-only.
Suggestion: legacy fallback seeds the first workspace push with the entire global history (useInputHistory.ts:55) doudouOUC Confirmed — independently noted the same behavior (my Low below). Intentional copy-not-move migration, but it partially weakens workspace isolation on the first push; a design-doc note would help.
Downgraded Approve → Comment: CI still running wenshao Procedural — no code finding.
CHANGES_REQUESTED "reverse audit" qwen-code-ci-bot Procedural message — not a code finding.

My findings

Minor-1 and Minor-2 are posted inline.

Low

  • getComposerWorkspaceCwd (App.tsx:3225-3227) checks connectionRef.current.sessionId before lockedWorkspaceCwd, whereas the atWorkspaceCwd prop handed to the composer (App.tsx:8259-8264) puts lockedWorkspaceCwd first. If a session were active while a different workspace is locked, prompts would enqueue against the session's workspace while drafts/history are scoped to the locked one. Reachability looks narrow (a locked workspace filters composerWorkspaces to a single entry), but the precedence divergence is worth aligning.
  • useInputHistory.reset() (useInputHistory.ts:172-176) clears indexRef/searchIndexRef but not draftRef. Not a reachable bug — a stale draft is always overwritten by the next navigateUp before it can be read, and navigateDown returns null while indexRef === -1 — hygiene only.

Nit

  • The unscopedDraftEditedRef.current = false reset at lines 3025-3027 is dead code: line 3003 already resets the flag unconditionally before that point is reachable.
  • The getDocText memo (lines 2666-2674) retains a reference to the last Text doc indefinitely; negligible in practice (single doc, replaced on each edit).

Additional audit coverage

Independent checks beyond the findings above:

  • Delayed-commit identity validation: the sourceChanged branch of commitAccepted routes the history push to the submission-time key via pushInputHistoryEntry, and clearComposerDraftIfMatches only clears when the stored draft still equals the submitted snapshot — both correct.
  • composerWorkspaces ref-memo compares id/cwd/label/primary/trusted by value; isSameGitStatus correctly excludes the volatile computedAt.
  • Draft-debounce deadline math (flushDraftAfterIdle / draftSaveDeadline) and the wasBrowsingHistory/wasSearchingHistory guards in the identity-switch flush effect — correct.
  • Legacy key safety: the fallback path only reads the global key and never mutates or deletes it; pushInputHistoryEntry's dedupe (history[length-1] === text) still seeds the scoped key when the scoped store is empty, which is the intended migration behavior.

Comment thread packages/web-shell/client/hooks/useComposerCore.ts
workspaceCwd: string | undefined,
): string | undefined {
if (sessionId) {
return `${SESSION_DRAFT_STORAGE_PREFIX}${encodeURIComponent(sessionId)}`;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Minor] Session-scoped draft keys (qwen-web-shell-session-draft:<sessionId>) are only removed on an accepted submission (saveComposerDraft(key, '')) or via clearComposerDraftIfMatches. A session abandoned with unsent draft text leaves its key in localStorage forever — I couldn't find any eviction/lifecycle logic in the PR, and the design doc doesn't mention cleanup either.

Growth is slow (one small string per abandoned-with-draft session), so this is hygiene rather than urgent — a periodic sweep of session-draft: keys that don't match a live session (or a TTL written into the value) would close it. Non-blocking.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Declined — this is a storage hygiene suggestion, not a correctness issue. Growth is bounded by the number of sessions abandoned with unsent draft text (one small string each). Adding a periodic sweep or TTL is out of scope for this review round and would add lifecycle complexity beyond the PR's intent.

中文说明

已拒绝 — 这是存储卫生建议,而非正确性问题。增长受限于带有未发送草稿文本的废弃会话数量(每个一个小字符串)。添加定期清理或 TTL 超出了本轮审查的范围,且会增加超出 PR 意图的生命周期复杂性。

@qwen-code-dev-bot

qwen-code-dev-bot commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 5 finishedview run. See this round's report below.

中文说明

AutoFix 第 5 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Review feedback addressed

Critical

  • [rc:3655932995] historyBrowseActiveRef.current not reset on !composerUnchanged early returnFixed. Moved historyBrowseActiveRef.current = false before the if (!composerUnchanged) return; guard in commitAccepted. Previously, when the composer changed between submit and async acceptance, the early return left the flag stuck at true, suppressing scheduleDraftSave for subsequent programmatic edits and risking draft loss on navigation. The flag is now unconditionally cleared on every committed submission, matching the reviewer's suggested fix.

Suggestions implemented

  • [rc:3655208797 / rc:3655933019] visibilitychange and pagehide draft flush has no test coverageFixed. Added a test in useComposerCore.dom.test.tsx that types a draft, dispatches visibilitychange (with document.visibilityState spied to 'hidden'), and asserts the draft appears in localStorage immediately without advancing timers. Also covers the pagehide path.

  • [rc:3655429250] Mobile composer onBlur draft save has no test coverageFixed. Wired onBlur={composer.mobileComposer.onBlur} into the mobile test harness textarea, added sessionId/atWorkspaceCwd props to the harness, added draft-key cleanup to afterEach, and added a test that types text, fires focusout (React's blur delegation), and asserts the draft key is populated in localStorage before the debounce timer.

  • [rc:3656320203] Shell-mode ArrowUp enters history browse without flushing the in-progress draftFixed. Added if (!isBrowsingHistory) saveCurrentDraftRef.current(); to the shell-mode ArrowUp branch in the CodeMirror keymap, mirroring the non-shell branch and the navigatePrevHistory button handler.

Suggestions declined

  • [rc:3655429255] Legacy fallback migration copies all global history entries into the first workspaceDeclined. The fallback seeding is the intentional migration path: when workspace-scoped history is empty, the legacy global history is used so users don't lose their pre-existing prompts. Saving only [text] on first submit would discard the user's legacy history from ArrowUp navigation in the new workspace, which is a product-level migration tradeoff rather than a bug. The current behavior is consistent between pushInputHistoryEntry and the push callback, and changing it would alter the PR's migration semantics.

  • [rc:3656320212] Session-scoped draft keys are never evictedDeclined. This is a storage hygiene suggestion, not a correctness issue. Growth is bounded by the number of sessions abandoned with unsent draft text (one small string each). Adding a periodic sweep or TTL is out of scope for this review round and would add lifecycle complexity beyond the PR's intent.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • vitest run client/hooks/useComposerCore.dom.test.tsx (packages/web-shell) — 35 passed
  • vitest run client/hooks/useComposerCore.mobile.dom.test.tsx (packages/web-shell) — 18 passed
  • vitest run client/App.test.tsx (packages/web-shell) — 207 passed
中文说明

已处理的审查反馈

严重问题

  • [rc:3655932995] historyBrowseActiveRef.current!composerUnchanged 提前返回时未重置已修复。historyBrowseActiveRef.current = false 移至 if (!composerUnchanged) return; 之前。此前,当输入框在提交和异步确认之间发生变化时,提前返回会导致该标志保持 true,从而抑制后续程序化编辑的 scheduleDraftSave,在页面导航时有草稿丢失风险。现在该标志在每次已确认的提交中无条件清除,与审查者建议的修复方案一致。

已实施的建议

  • [rc:3655208797 / rc:3655933019] visibilitychangepagehide 草稿刷写缺少测试覆盖已修复。useComposerCore.dom.test.tsx 中添加了测试:输入草稿后,派发 visibilitychange 事件(将 document.visibilityState spy 为 'hidden'),断言草稿立即出现在 localStorage 中,无需推进计时器。同时覆盖了 pagehide 路径。

  • [rc:3655429250] 移动端输入框 onBlur 草稿保存缺少测试覆盖已修复。onBlur={composer.mobileComposer.onBlur} 接入移动端测试 harness 的 textarea,为 harness 添加了 sessionId/atWorkspaceCwd 属性,在 afterEach 中添加了草稿键清理,并添加了测试:输入文本后触发 focusout(React 的 blur 委托机制),断言草稿键在防抖计时器触发前已写入 localStorage。

  • [rc:3656320203] Shell 模式下 ArrowUp 进入历史浏览时未刷写进行中的草稿已修复。 在 CodeMirror 快捷键映射的 shell 模式 ArrowUp 分支中添加了 if (!isBrowsingHistory) saveCurrentDraftRef.current();,与非 shell 分支和 navigatePrevHistory 按钮处理器保持一致。

已拒绝的建议

  • [rc:3655429255] Legacy 回退迁移将所有全局历史条目复制到第一个工作区已拒绝。 回退填充是有意为之的迁移路径:当工作区级历史为空时,使用 legacy 全局历史,以确保用户不会丢失已有的提示词。首次提交时仅保存 [text] 会导致用户在新工作区中通过 ArrowUp 无法访问 legacy 历史,这是产品层面的迁移权衡,而非 bug。当前行为在 pushInputHistoryEntrypush 回调之间保持一致,修改它会改变本 PR 的迁移语义。

  • [rc:3656320212] 会话级草稿键从未被清理已拒绝。 这是存储卫生建议,而非正确性问题。增长受限于带有未发送草稿文本的废弃会话数量(每个一个小字符串)。添加定期清理或 TTL 超出了本轮审查的范围,且会增加超出 PR 意图的生命周期复杂性。

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • vitest run client/hooks/useComposerCore.dom.test.tsx(packages/web-shell)— 35 个通过
  • vitest run client/hooks/useComposerCore.mobile.dom.test.tsx(packages/web-shell)— 18 个通过
  • vitest run client/App.test.tsx(packages/web-shell)— 207 个通过

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

…on-history-drafts

# Conflicts:
#	packages/web-shell/client/hooks/useComposerCore.ts
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/100 轮)。改动内容与我反驳保留之处如下:

Address Review Summary — PR #7810

Feedback

No review feedback, inline comments, or failed checks were present in this round.

Conflict Resolution

Merged origin/main into the PR branch (--conflict true). One file had conflicts:

packages/web-shell/client/hooks/useComposerCore.ts — two conflict regions plus silently dropped code from the auto-merge:

  1. Conflict 1 (early-return path): Main added onDismissFollowupRef.current?.(); to dismiss follow-up UI before committing input. Accepted main's addition.

  2. Conflict 2 (composerUpdateListener): Our branch added paste-tracking logic (pendingPastesRef, prunePendingPastes) at the start of the update listener. Main had no changes here. Kept our branch's addition.

  3. Silent auto-merge drops: The auto-merge silently discarded several blocks of our branch's large-paste handling feature. Restored all missing code:

    • LARGE_PASTE_CHAR_THRESHOLD / LARGE_PASTE_LINE_THRESHOLD constants
    • normalizePastedText(), isLargePaste(), escapeRegExp(), LargePastePlaceholderResult, createLargePastePlaceholder(), prunePendingPastes(), expandLargePastePlaceholders() functions
    • pendingPastesRef / nextPasteIdRef refs
    • expandLargePastePlaceholders() call in the submit path
    • Large-paste placeholder creation in the CodeMirror paste handler
    • Paste state reset in the clearText block

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • vitest run client/hooks/useComposerCore.dom.test.tsx client/hooks/useComposerCore.mobile.dom.test.tsx client/App.test.tsx (packages/web-shell) — 268 passed (3 test files)
中文说明

处理审查总结 — PR #7810

反馈

本轮没有审查反馈、行内评论或失败的检查。

冲突解决

origin/main 合并到 PR 分支(--conflict true)。一个文件存在冲突:

packages/web-shell/client/hooks/useComposerCore.ts — 两个冲突区域,以及自动合并静默丢失的代码:

  1. 冲突 1(提前返回路径): main 添加了 onDismissFollowupRef.current?.(); 用于在提交输入前关闭后续 UI。接受了 main 的添加。

  2. 冲突 2(composerUpdateListener): 我们的分支在更新监听器开头添加了粘贴跟踪逻辑(pendingPastesRefprunePendingPastes)。main 在此处没有更改。保留了我们分支的添加。

  3. 自动合并静默丢失: 自动合并静默丢弃了我们分支的大段粘贴处理功能代码。已恢复所有缺失代码:

    • LARGE_PASTE_CHAR_THRESHOLD / LARGE_PASTE_LINE_THRESHOLD 常量
    • normalizePastedText()isLargePaste()escapeRegExp()LargePastePlaceholderResultcreateLargePastePlaceholder()prunePendingPastes()expandLargePastePlaceholders() 函数
    • pendingPastesRef / nextPasteIdRef 引用
    • 提交路径中的 expandLargePastePlaceholders() 调用
    • CodeMirror 粘贴处理器中的大粘贴占位符创建逻辑
    • clearText 块中的粘贴状态重置

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • vitest run client/hooks/useComposerCore.dom.test.tsx client/hooks/useComposerCore.mobile.dom.test.tsx client/App.test.tsx(packages/web-shell)— 268 个测试通过(3 个测试文件)

Base-conflict check · 基分支冲突检查: conflicted with main — resolved in this push. · 与 main 有冲突——已在本次推送中解决。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Not reviewed: reverse audit of chunk 5 (round 2) — the auditor returned nothing substantive twice.

中文说明

已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。 未审查:reverse audit of chunk 5 (round 2) — the auditor returned nothing substantive twice。

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/hooks/useComposerCore.ts
Comment thread packages/web-shell/client/hooks/useComposerCore.ts
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed. Suggestions are inline.

中文说明

已审查。 建议见行内评论。

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/hooks/useComposerCore.ts Outdated
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下:

Review feedback addressed

All three inline suggestions are resolved in code.

1. Large paste helpers had no test coverage (rc:3658622241) — implemented

Added a large paste helpers suite in useComposerCore.test.ts covering the five exported functions: normalizePastedText (CRLF/CR → LF), isLargePaste (1000-char / 10-line boundaries, plus code-point vs UTF-16 counting), createLargePastePlaceholder (placeholder format and id incrementing), prunePendingPastes (removing absent placeholders and resetting the id to 1 when the map empties), and expandLargePastePlaceholders (literal bracket escaping, the empty-map no-op, and the case where one placeholder is a substring of another so the longer one must win). 7 new tests; the file now has 15 passing tests.

2. adoptUnscopedInMemoryDraft ignored history-browse/search state (rc:3658622250) — implemented

Confirmed the reported bug: when a user recalls prompt history on the unscoped New Task page and then switches to a workspace with no stored draft, the recalled history text was adopted (and persisted) as the workspace draft. Added !wasBrowsingHistory && !wasSearchingHistory to the adoptUnscopedInMemoryDraft condition, mirroring the guard already used a few lines above for saving the previous draft.

3. prunePendingPastes ran on undo/redo and broke re-expansion (rc:3659129540) — implemented

Confirmed the reported bug: undo removes the placeholder and the unconditional prune deleted the paste mapping, so a subsequent redo re-inserted the literal [Pasted Content N chars] text and submit sent it verbatim. Hoisted the existing userEdited computation (tr.isUserEvent('input') || tr.isUserEvent('delete')) to the top of the update listener and gated the prune on it, so undo/redo transactions preserve the mapping while genuine user deletes still prune.

Conflict resolution (--conflict true)

Merged origin/main. One conflict in App.test.tsx (the per-test workspace-client mock setup): HEAD deterministically resets and re-applies the workspaceByCwd implementation, while main switched to mockClear and added a new workspaceById.mockClear(). Resolved as the union — kept HEAD's deterministic workspaceByCwd restore and added main's workspaceById.mockClear().

The merge also surfaced a semantic conflict: this branch's keeps composer workspace props stable across an equivalent list refresh test asserts no ChatEditor prop changes across an equivalent capabilities refresh, but main's voice feature (voiceTarget, voiceStatusRevision) derives those props from the capabilities snapshot identity and legitimately recomputes them on such a refresh. Stabilizing main's mainVoiceTarget would mean changing the recompute timing of a feature unrelated to this PR (it feeds many downstream voice effects/memos) with no App-level test coverage, so instead the catch-all assertion now excludes those two voice-owned props while every named workspace-prop stability assertion (workspaces, onSelectWorkspace, onCreateScratchWorkspace, onOpenExistingWorkspace) remains fully enforced.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • npx prettier --check (touched files) — passed
  • vitest useComposerCore.test.ts (touched) — 15 passed
  • vitest useComposerCore.dom.test.tsx + useComposerCore.mobile.dom.test.tsx (touched) — 53 passed
  • vitest App.test.tsx (touched/merged) — 229 passed
  • vitest run full packages/web-shell suite — 141 files, 2357 passed
  • web-shell E2E Smoke (Playwright/browser) — not runnable in this sandbox; left to the workflow's independent CI
中文说明

已处理的审查反馈

三条行内建议均已在代码中解决。

1. 大段粘贴辅助函数缺少测试覆盖(rc:3658622241)— 已实现

useComposerCore.test.ts 中新增 large paste helpers 测试套件,覆盖五个导出函数:normalizePastedText(CRLF/CR → LF)、isLargePaste(1000 字符 / 10 行边界,以及码点与 UTF-16 计数差异)、createLargePastePlaceholder(占位符格式与 ID 递增)、prunePendingPastes(移除文档中不存在的占位符,并在映射清空时将 ID 重置为 1)、expandLargePastePlaceholders(方括号字面量转义、空映射 no-op,以及一个占位符是另一个占位符子串时较长者必须优先匹配的场景)。新增 7 个测试,该文件现有 15 个通过的测试。

2. adoptUnscopedInMemoryDraft 忽略了历史浏览/搜索状态(rc:3658622250)— 已实现

确认了所报告的缺陷:当用户在无作用域的「新建任务」页面召回提示词历史,然后切换到一个没有已存草稿的 workspace 时,被召回的历史文本会被错误地采纳(并持久化)为该 workspace 的草稿。已在 adoptUnscopedInMemoryDraft 条件中加入 !wasBrowsingHistory && !wasSearchingHistory,与上方几行用于保存上一份草稿的守卫保持一致。

3. prunePendingPastes 在撤销/重做时执行,破坏了重新展开(rc:3659129540)— 已实现

确认了所报告的缺陷:撤销会移除占位符,而无条件的 prune 会删除粘贴映射,导致随后的重做重新插入字面量 [Pasted Content N chars] 文本,提交时原样发送。已将现有的 userEdited 计算(tr.isUserEvent('input') || tr.isUserEvent('delete'))上移到更新监听器顶部,并以其作为 prune 的门控,使撤销/重做事务保留映射,同时用户真正的删除操作仍会触发 prune。

冲突解决(--conflict true

已合并 origin/mainApp.test.tsx 中有一处冲突(每个测试的 workspace-client mock 设置):HEAD 确定性地重置并重新应用 workspaceByCwd 的实现,而 main 改用 mockClear 并新增了 workspaceById.mockClear()。以并集方式解决——保留 HEAD 对 workspaceByCwd 的确定性恢复,并加入 main 的 workspaceById.mockClear()

合并还暴露了一处语义冲突:本分支的 keeps composer workspace props stable across an equivalent list refresh 测试断言在等价的 capabilities 刷新前后没有任何 ChatEditor prop 变化,但 main 的语音功能(voiceTargetvoiceStatusRevision)从 capabilities 快照的引用派生这些 prop,并在此类刷新时合理地重新计算它们。稳定 main 的 mainVoiceTarget 意味着更改一个与本 PR 无关的功能的重新计算时机(它驱动许多下游语音 effect/memo),且没有 App 层的测试覆盖,因此改为让该全量断言排除这两个语音专属 prop,同时完整保留所有具名的 workspace prop 稳定性断言(workspacesonSelectWorkspaceonCreateScratchWorkspaceonOpenExistingWorkspace)。

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • npx prettier --check(改动文件)— 通过
  • vitest useComposerCore.test.ts(改动)— 15 通过
  • vitest useComposerCore.dom.test.tsx + useComposerCore.mobile.dom.test.tsx(改动)— 53 通过
  • vitest App.test.tsx(改动/合并)— 229 通过
  • vitest run 完整 packages/web-shell 套件 — 141 个文件,2357 通过
  • web-shell E2E Smoke(Playwright/浏览器)— 无法在此沙箱中运行;交由工作流的独立 CI 验证

Base-conflict check · 基分支冲突检查: conflicted with main — resolved in this push. · 与 main 有冲突——已在本次推送中解决。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.

中文说明

已审查。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/hooks/useComposerCore.ts Outdated
Comment thread packages/web-shell/client/hooks/useComposerCore.ts
Comment thread packages/web-shell/client/hooks/useComposerCore.ts Outdated
Comment thread packages/web-shell/client/hooks/useComposerCore.ts
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下:

Review feedback addressed

All four findings (2 Critical, 2 Suggestion) are resolved in this commit.

[rc:3660151630] Critical: Delete-then-undo permanently loses large-paste mapping

Decision: Fixed.

The prune condition now only fires on input user events, not delete. When a user deletes a placeholder and then undoes, CodeMirror restores the doc text but cannot restore the React ref mapping. By skipping pruning on delete events, the mapping survives the delete-then-undo cycle. Stale entries are harmless — expandLargePastePlaceholders only replaces placeholders actually present in the text.

Change: In the composerUpdateListener, added a separate userInput check (tr.isUserEvent('input') only) for the prune gate, replacing the previous userEdited check that included delete events. The userEdited variable is still used for resetting historyBrowseActiveRef, where both input and delete correctly end history-browse mode.

[rc:3660151637] Critical: Effect cleanup ordering causes draft data loss on unmount

Decision: Fixed.

Rather than reordering effects (which introduced a test regression due to React's cleanup ordering in this component), the fix adds saveCurrentDraftRef.current() at the top of the EditorView cleanup, before view.destroy(). This flushes any pending draft while the view is still alive. The draft-save effect's own cleanup then becomes a no-op (draftDirty is already false).

Change: One line added in the EditorView effect cleanup: saveCurrentDraftRef.current() before view.dispatch({ effects: clearInlineTagsEffect.of() }).

[rc:3660151645] Suggestion: prunePendingPastes substring false-positive

Decision: Fixed.

Rewrote prunePendingPastes to use the same length-descending regex approach as expandLargePastePlaceholders. Placeholders are sorted by length descending, joined into a regex alternation, and matched via matchAll. Only exact matches are kept; substring false-positives (e.g. [Pasted Content 5 chars] matching inside [Pasted Content 5 chars] #2) are eliminated.

Change: Replaced the for...includes loop with a regex-based matchAll approach. Added a unit test verifying the substring case.

[rc:3660151661] Suggestion: Mobile draft load never fires onInputTextChange

Decision: Fixed.

Added a mount-only useEffect that calls onInputTextChangeRef.current?.(mobileTextRef.current) when isTouchComposer is true and a draft was restored. This mirrors the desktop path's if (initialTextValue) { onInputTextChangeRef.current?.(initialTextValue); } block inside the EditorView creation effect.

Change: Six-line useEffect added after the setMobileText callback definition.

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • vitest useComposerCore.test.ts — 16 passed
  • vitest useComposerCore.dom.test.tsx — 35 passed
  • vitest useComposerCore.mobile.dom.test.tsx — 18 passed
  • vitest client/App.test.tsx — 229 passed
中文说明

已处理的审查反馈

本次提交解决了全部 4 条发现(2 条 Critical,2 条 Suggestion)。

[rc:3660151630] Critical:删除后撤销会永久丢失大段粘贴映射

决定: 已修复。

修剪条件现在仅在 input 用户事件上触发,不再包含 delete。当用户删除占位符后撤销时,CodeMirror 会恢复文档文本,但无法恢复 React ref 中的映射。通过跳过 delete 事件上的修剪,映射在"删除后撤销"操作中得以保留。过期条目是无害的——expandLargePastePlaceholders 只替换文本中实际存在的占位符。

改动:composerUpdateListener 中,为修剪门控添加了单独的 userInput 检查(仅 tr.isUserEvent('input')),替换了之前包含 delete 事件的 userEdited 检查。userEdited 变量仍用于重置 historyBrowseActiveRef,在该场景中 input 和 delete 都正确地结束历史浏览模式。

[rc:3660151637] Critical:Effect 清理顺序导致卸载时草稿数据丢失

决定: 已修复。

没有采用重新排序 effect 的方案(该方案因 React 在此组件中的清理顺序导致了测试回归),而是在 EditorView 清理函数的顶部、view.destroy() 之前添加了 saveCurrentDraftRef.current() 调用。这在 view 仍然存活时刷新待保存的草稿。草稿保存 effect 自身的清理随后变为空操作(draftDirty 已为 false)。

改动: 在 EditorView effect 清理函数中添加了一行:在 view.dispatch({ effects: clearInlineTagsEffect.of() }) 之前调用 saveCurrentDraftRef.current()

[rc:3660151645] Suggestion:prunePendingPastes 子串误判

决定: 已修复。

重写了 prunePendingPastes,采用与 expandLargePastePlaceholders 相同的按长度降序排列的正则方法。占位符按长度降序排列,组合为正则交替匹配,通过 matchAll 进行匹配。只保留精确匹配;消除了子串误判(例如 [Pasted Content 5 chars] 匹配到 [Pasted Content 5 chars] #2 内部)。

改动:for...includes 循环替换为基于正则的 matchAll 方法。添加了验证子串情况的单元测试。

[rc:3660151661] Suggestion:移动端草稿加载不触发 onInputTextChange

决定: 已修复。

添加了一个仅在挂载时运行的 useEffect,当 isTouchComposer 为 true 且草稿已恢复时,调用 onInputTextChangeRef.current?.(mobileTextRef.current)。这与桌面端在 EditorView 创建 effect 中的 if (initialTextValue) { onInputTextChangeRef.current?.(initialTextValue); } 块保持一致。

改动:setMobileText 回调定义之后添加了六行 useEffect

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • vitest useComposerCore.test.ts — 16 通过
  • vitest useComposerCore.dom.test.tsx — 35 通过
  • vitest useComposerCore.mobile.dom.test.tsx — 18 通过
  • vitest client/App.test.tsx — 229 通过

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Downgraded from Approve to Comment: CI failing: web-shell E2E Smoke (ubuntu-latest, Node 22.x). Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI failing: web-shell E2E Smoke (ubuntu-latest, Node 22.x)。 已审查。

— qwen3.7-max via Qwen Code /review

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Downgraded from Approve to Comment: CI failing: web-shell E2E Smoke (ubuntu-latest, Node 22.x). Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI failing: web-shell E2E Smoke (ubuntu-latest, Node 22.x)。 已审查。

— qwen3.7-max via Qwen Code /review

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Downgraded from Approve to Comment: CI failing: web-shell E2E Smoke (ubuntu-latest, Node 22.x). Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI failing: web-shell E2E Smoke (ubuntu-latest, Node 22.x)。 已审查。

— qwen3.7-max via Qwen Code /review

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/100 轮)。改动内容与我反驳保留之处如下:

Review feedback addressed

CI failure: web-shell E2E Smoke (ubuntu-latest, Node 22.x)

Classification: Required (broken CI test)

Root cause: The PR introduced large paste placeholder handling in useComposerCore.ts (LARGE_PASTE_CHAR_THRESHOLD = 1000). Pastes exceeding 1000 characters are now replaced with a [Pasted Content N chars] placeholder in the editor, expanded back to the original text on submit. However, the existing Playwright smoke test pastes long plain text as editable composer content @smoke pastes 1362 characters and asserted the raw text appeared directly in the editor — contradicting the new placeholder behavior.

Fix: Updated the smoke test to match the new behavior:

  • After pasting, the editor now shows [Pasted Content 1362 chars] (the placeholder)
  • After typing edited, the editor shows the placeholder followed by the typed text
  • On submit, the prompt body still contains the fully expanded original text plus the typed suffix

The test was renamed from "pastes long plain text as editable composer content" to "pastes long plain text as a placeholder and expands it on submit" to accurately describe the verified behavior.

Changes: packages/web-shell/client/e2e/web-shell.smoke.spec.ts — 4 lines changed (test name, placeholder assertion, edited-text assertion)

No conflict resolution was needed (--conflict false).

Verification

  • npm run build — passed
  • npm run typecheck — passed
  • npm run lint — passed
  • vitest run client/hooks/useComposerCore.test.ts (web-shell) — 16 passed
  • vitest run client/App.test.tsx (web-shell) — 230 passed
  • vitest run client/hooks/useComposerCore.dom.test.tsx (web-shell) — 35 passed
  • vitest run client/hooks/useComposerCore.mobile.dom.test.tsx (web-shell) — 18 passed
  • Playwright E2E smoke tests could not run locally (Playwright browser version mismatch in this environment); the fix is verified by code inspection against the implementation and the CI will serve as the final gate
中文说明

已处理的审查反馈

CI 失败:web-shell E2E Smoke (ubuntu-latest, Node 22.x)

分类: 必须修复(CI 测试失败)

根本原因: 本 PR 在 useComposerCore.ts 中引入了大段粘贴占位符处理(LARGE_PASTE_CHAR_THRESHOLD = 1000)。超过 1000 字符的粘贴内容现在会在编辑器中替换为 [Pasted Content N chars] 占位符,在提交时展开为原始文本。然而,现有的 Playwright 冒烟测试 pastes long plain text as editable composer content @smoke 粘贴了 1362 个字符,并断言原始文本直接显示在编辑器中——与新的占位符行为矛盾。

修复: 更新冒烟测试以匹配新行为:

  • 粘贴后,编辑器现在显示 [Pasted Content 1362 chars](占位符)
  • 输入 edited 后,编辑器显示占位符加输入的文本
  • 提交时,请求体仍然包含完整展开的原始文本加上输入的后缀

测试名称从 "pastes long plain text as editable composer content" 更名为 "pastes long plain text as a placeholder and expands it on submit",以准确描述验证的行为。

变更: packages/web-shell/client/e2e/web-shell.smoke.spec.ts — 修改 4 行(测试名称、占位符断言、编辑后文本断言)

无需解决冲突(--conflict false)。

验证

  • npm run build — 通过
  • npm run typecheck — 通过
  • npm run lint — 通过
  • vitest run client/hooks/useComposerCore.test.ts(web-shell)— 16 个通过
  • vitest run client/App.test.tsx(web-shell)— 230 个通过
  • vitest run client/hooks/useComposerCore.dom.test.tsx(web-shell)— 35 个通过
  • vitest run client/hooks/useComposerCore.mobile.dom.test.tsx(web-shell)— 18 个通过
  • Playwright E2E 冒烟测试无法在本地运行(此环境中 Playwright 浏览器版本不匹配);修复通过代码审查对照实现进行验证,CI 将作为最终验证门控

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.7-max via Qwen Code /review

@wenshao
wenshao added this pull request to the merge queue Jul 28, 2026
Merged via the queue into QwenLM:main with commit 6a432ad Jul 28, 2026
47 checks passed

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found. LGTM! ✅

中文说明

未发现问题。LGTM!✅

— qwen3.7-max via Qwen Code /review

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.1.

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

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants