From d607cc8be5b80355aaf2c090f0a4e228c2df53ad Mon Sep 17 00:00:00 2001 From: Trae User Date: Thu, 24 Sep 2026 16:54:39 +0800 Subject: [PATCH 1/3] fix(question): prevent AskUserQuestion from resetting answers on re-render The picker's reset effect depended on the `questions` array, which is rebuilt whenever the request object reference changes. App recreated the visible question object on every render (inline spread, not memoized), so any unrelated re-render while the question was pending reset the answers to their initial state and jumped back to the first question. - Gate the reset on requestId identity via a ref (reset only on new request) - Memoize visibleQuestionReq in App to avoid recreating the object - Add a regression test for same-request re-renders - Update the inline-permission architecture assertion to verify the permission-over-question intent instead of the old syntax --- web/src/App.jsx | 21 +++++++------- web/src/components/QuestionPicker.jsx | 6 ++++ .../lib/inlinePermissionArchitecture.test.js | 5 +++- web/src/lib/questionPickerInteraction.test.js | 28 +++++++++++++++++++ 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/web/src/App.jsx b/web/src/App.jsx index c08f01f2..6938fe37 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -2072,17 +2072,16 @@ export function App() { const sidebarCollapsed = view !== 'single' || (projectSidebarCollapsed && !guidedTourPreparing && !guidedTourRun); - const visibleQuestionReq = !visiblePermissionUnresolved - ? (() => { - const request = visibleQuestionRequest(questionReqs, activeId, permissionOwnership); - return request - ? { - ...request, - origin_label: questionOriginLabel(request, permissionOwnership), - } - : null; - })() - : null; + const visibleQuestionReq = useMemo(() => { + if (visiblePermissionUnresolved) return null; + const request = visibleQuestionRequest(questionReqs, activeId, permissionOwnership); + return request + ? { + ...request, + origin_label: questionOriginLabel(request, permissionOwnership), + } + : null; + }, [visiblePermissionUnresolved, questionReqs, activeId, permissionOwnership]); const nativeSurfacesVisible = !showSettings && !showFeedback && !searchOpen diff --git a/web/src/components/QuestionPicker.jsx b/web/src/components/QuestionPicker.jsx index f1981da8..53fc0045 100644 --- a/web/src/components/QuestionPicker.jsx +++ b/web/src/components/QuestionPicker.jsx @@ -70,8 +70,14 @@ export function QuestionPicker({ request, onResolve, originLabel = '', className const copiedTimerRef = useRef(null); // 记录 Esc「取消选中」与「拒绝作答」之间的连按窗口。 const escTimerRef = useRef(null); + // 记录上一次执行重置的请求 id:只有切换到新请求才重置; + // 同一请求因重渲染产生新的 questions 引用时,不清空答案、不跳回第一题。 + const lastResetRequestIdRef = useRef(null); useEffect(() => { + // requestId 未变化(首次挂载除外)说明是同一请求的重渲染,保留已答内容。 + if (lastResetRequestIdRef.current === normalized.requestId) return; + lastResetRequestIdRef.current = normalized.requestId; setAnswers(makeInitialAnswers(questions)); setCurrentIndex(0); setFocusIndex(-1); diff --git a/web/src/lib/inlinePermissionArchitecture.test.js b/web/src/lib/inlinePermissionArchitecture.test.js index 0f828bf9..656fb59e 100644 --- a/web/src/lib/inlinePermissionArchitecture.test.js +++ b/web/src/lib/inlinePermissionArchitecture.test.js @@ -88,7 +88,10 @@ run('App reconciles server close events and sends decisions to the request sessi run('permission is conversation-scoped and is not a global focus/search/tour blocker', () => { const app = source('App.jsx'); assert.match(app, /visiblePermissionRequests\(permReqs, activeId, permissionOwnership\)/); - assert.match(app, /const visibleQuestionReq = !visiblePermissionUnresolved/); + assert.match(app, /visibleQuestionRequest\(questionReqs, activeId, permissionOwnership\)/); + // 权限未解决时问题必须让位(permission 优先于 question);memoize 后以提前 return 表达。 + assert.match(app, /const visibleQuestionReq = useMemo\(/); + assert.match(app, /if \(visiblePermissionUnresolved\) return null/); assert.match(app, /permissionOpen: false/); const tourBlock = between(app, 'const guidedTourBlocked', 'useEffect(() => initInactiveSelection'); diff --git a/web/src/lib/questionPickerInteraction.test.js b/web/src/lib/questionPickerInteraction.test.js index 5a5c65f4..7caa6ccf 100644 --- a/web/src/lib/questionPickerInteraction.test.js +++ b/web/src/lib/questionPickerInteraction.test.js @@ -219,3 +219,31 @@ run('empty custom input Enter skips locally and only the last question submits', key(picker, 'Enter', { ctrlKey: true }); assert.equal(picker.sent[0].answers[0].not_answered, true); }); + +run('same request re-rendered with a new object reference keeps answers and position', () => { + const picker = harness([q1, q2]); + // 双击第一题第一个选项:选中并本地推进到第二题(daemon first-wins,中途不发送)。 + rows(picker.render())[0].props.onMouseDown(event({ detail: 2 })); + const tabular = (tree) => nodes(tree).find( + (n) => /tabular-nums/.test(n.props?.className || '') && typeof n.props.children === 'string'); + assert.equal(tabular(picker.render()).props.children, '2 / 2'); + // 模拟 App 重渲染:request_id 不变,但传入内容相同、引用全新的 request 对象。 + picker.render({ request_id: 'r1', session_id: 's1', questions: [q1, q2] }); + // 修复前:重置 effect 误把引用变化当作新请求 -> 跳回第一题('1 / 2')并清空答案。 + assert.equal(tabular(picker.render()).props.children, '2 / 2'); + // 回到第一题,断言已选答案内容仍保留(不只是位置没跳)。 + button(picker.render(), '上一题').props.onClick(); + assert.match(rows(picker.render())[0].props.className, /text-accent/); +}); + +run('switching to a new request resets answers and position', () => { + const picker = harness([q1, q2]); + rows(picker.render())[0].props.onMouseDown(event({ detail: 2 })); + const tabular = (tree) => nodes(tree).find( + (n) => /tabular-nums/.test(n.props?.className || '') && typeof n.props.children === 'string'); + assert.equal(tabular(picker.render()).props.children, '2 / 2'); + // 切换到新的提问请求(request_id 变化):必须重置到第一题并清空答案。 + picker.render({ request_id: 'r2', session_id: 's1', questions: [q1, q2] }); + assert.equal(tabular(picker.render()).props.children, '1 / 2'); + assert.doesNotMatch(rows(picker.render())[0].props.className, /text-accent/); +}); From 4111ad0f79ff5a94a5ef31acd7c325f4adb55374 Mon Sep 17 00:00:00 2001 From: Trae User Date: Fri, 25 Sep 2026 10:12:24 +0800 Subject: [PATCH 2/3] test(web): expect 404 when resuming a missing workspace session Commit 38b6af5c added workspace-membership checks to the workspaces resume route, so a missing session short-circuits with 404 before reaching SessionClient. The escaped-exception test still expected 500 for that path and failed CI. Assert 404 for the workspace route; the plain /api/sessions/:id/resume route keeps covering the SESSION_RESUME_FAILED 500 path. --- tests/web/web_server_smoke_test.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/web/web_server_smoke_test.cpp b/tests/web/web_server_smoke_test.cpp index 7c0eb9e5..5f9fcabd 100644 --- a/tests/web/web_server_smoke_test.cpp +++ b/tests/web/web_server_smoke_test.cpp @@ -3885,8 +3885,8 @@ TEST(WebServerHttp, SessionRoutesReportEscapedExceptionsAsJson500) { const std::vector cases = { {"/api/workspaces/" + hash + "/sessions", "SESSION_CREATE_FAILED", "boom: create"}, {"/api/sessions", "SESSION_CREATE_FAILED", "boom: create"}, - {"/api/workspaces/" + hash + "/sessions/20260911-000000-dead/resume", - "SESSION_RESUME_FAILED", "boom: resume"}, + // 无 workspace 归属前置检查的 resume 直达 SessionClient,ThrowingSessionClient + // 抛异常应被转成带原因的 JSON 500。 {"/api/sessions/20260911-000000-dead/resume", "SESSION_RESUME_FAILED", "boom: resume"}, }; @@ -3902,6 +3902,18 @@ TEST(WebServerHttp, SessionRoutesReportEscapedExceptionsAsJson500) { << c.path << ": " << r.text; EXPECT_EQ(body.value("cwd", ""), fx.cwd) << c.path; } + + // 带 workspace 的 resume 对磁盘上不存在的 session 先做归属校验,直接 404 + // (引入自“无工作区会话恢复”改动),不会到达 SessionClient,因此不抛 500。 + auto missing_resume = cpr::Post( + cpr::Url{fx.url("/api/workspaces/" + hash + "/sessions/20260911-000000-dead/resume")}, + json_header, cpr::Body{R"({})"}); + EXPECT_EQ(missing_resume.status_code, 404) << missing_resume.text; + EXPECT_NE(missing_resume.header["Content-Type"].find("application/json"), std::string::npos) + << missing_resume.header["Content-Type"]; + json missing_body; + ASSERT_NO_THROW(missing_body = json::parse(missing_resume.text)) << missing_resume.text; + EXPECT_EQ(missing_body.value("error", ""), "session not found") << missing_resume.text; } // 场景:没有路由级 try/catch 的 handler(DELETE /api/sessions/:id 直接调 From e3106ae2aa99b394217d9dd35d4e14d241ab7c05 Mon Sep 17 00:00:00 2001 From: tmoonlight Date: Fri, 25 Sep 2026 19:09:48 +0800 Subject: [PATCH 3/3] fix(question): preserve hook order during authentication Move the visible question memo before authentication early returns, add a production App hook-order regression, and cover retained drafts. Record PR 72 review findings and baseline-aware validation. --- .../.openspec.yaml | 2 ++ .../design.md | 20 +++++++++++ .../proposal.md | 23 +++++++++++++ .../review.md | 18 ++++++++++ .../desktop-ask-user-question-ui/spec.md | 33 +++++++++++++++++++ .../tasks.md | 9 +++++ web/src/App.jsx | 22 +++++++------ .../lib/inlinePermissionArchitecture.test.js | 31 +++++++++++++++++ web/src/lib/questionPickerInteraction.test.js | 2 ++ 9 files changed, 150 insertions(+), 10 deletions(-) create mode 100644 openspec/changes/fix-question-picker-rerender-state/.openspec.yaml create mode 100644 openspec/changes/fix-question-picker-rerender-state/design.md create mode 100644 openspec/changes/fix-question-picker-rerender-state/proposal.md create mode 100644 openspec/changes/fix-question-picker-rerender-state/review.md create mode 100644 openspec/changes/fix-question-picker-rerender-state/specs/desktop-ask-user-question-ui/spec.md create mode 100644 openspec/changes/fix-question-picker-rerender-state/tasks.md diff --git a/openspec/changes/fix-question-picker-rerender-state/.openspec.yaml b/openspec/changes/fix-question-picker-rerender-state/.openspec.yaml new file mode 100644 index 00000000..abd7c5ae --- /dev/null +++ b/openspec/changes/fix-question-picker-rerender-state/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-25 diff --git a/openspec/changes/fix-question-picker-rerender-state/design.md b/openspec/changes/fix-question-picker-rerender-state/design.md new file mode 100644 index 00000000..e760030b --- /dev/null +++ b/openspec/changes/fix-question-picker-rerender-state/design.md @@ -0,0 +1,20 @@ +## Context + +见 proposal.md。后台请求 ID 由 UUID 生成，同一请求的重放保持身份；原 PR 的 requestId 重置保护与该协议一致。App 的认证分支会提前返回，所有 Hook 必须位于这些分支之前。 + +## Goals / Non-Goals + +**Goals:** 保留同请求回答状态，同时确保所有认证路径调用相同的 Hook 序列。 + +**Non-Goals:** 不修改权限优先级、提问协议或侧栏布局；不增加依赖。 + +## Decisions + +- 保留 QuestionPicker 的请求 ID 保护。仅依靠父组件 memo 无法覆盖订阅重放产生的新对象。 +- 将 visibleQuestionReq 的 useMemo 移到依赖计算之后、认证提前返回之前。删除 memo 会牺牲已实现的引用稳定性，移动它可同时保留优化和 Hook 约束。 +- 使用生产 App 的语法树检查组件本层 Hook 与提前返回的顺序，避免仅测试 QuestionPicker 而漏掉启动路径；另做真实 React 认证切换验证。 + +## Risks / Trade-offs + +- 静态源码模式可能漏掉嵌套控制流 → 使用语法树检查组件本层语句，并以真实 React 启动验证补充。 +- 当前 master 有已知侧栏间距断言失败 → 单独记录基线，验证 PR 本身和合并结果没有新增失败。 diff --git a/openspec/changes/fix-question-picker-rerender-state/proposal.md b/openspec/changes/fix-question-picker-rerender-state/proposal.md new file mode 100644 index 00000000..01085a49 --- /dev/null +++ b/openspec/changes/fix-question-picker-rerender-state/proposal.md @@ -0,0 +1,23 @@ +## Why + +同一次 AskUserQuestion 因后台事件重新渲染时会清空答案并跳回第一题。PR #72 修复了请求识别，但新增的 useMemo 位于认证提前返回之后，认证完成时会改变 Hook 数量并导致界面崩溃，需要在合并前修正并补齐回归覆盖。 + +## What Changes + +- 保留 PR 中按请求 UUID 重置、同请求重新渲染保留答案的修复。 +- 将可见问题的 useMemo 放到所有认证提前返回之前，确保启动、输入 token 和认证成功使用一致的 Hook 顺序。 +- 增加认证切换的回归检查，验证问题让位于权限请求和回答状态隔离。 + +## Capabilities + +### New Capabilities + +- `desktop-ask-user-question-ui`: 明确同请求状态保留、新请求重置及认证状态切换时的可用性；当前主规范尚未归档该能力，沿用现有变更中的能力名称。 + +### Modified Capabilities + +无。 + +## Impact + +影响 web/src/App.jsx、QuestionPicker 现有回归测试及 PR 审查记录；不改变 daemon 协议、不增加依赖。 diff --git a/openspec/changes/fix-question-picker-rerender-state/review.md b/openspec/changes/fix-question-picker-rerender-state/review.md new file mode 100644 index 00000000..bd6abfaa --- /dev/null +++ b/openspec/changes/fix-question-picker-rerender-state/review.md @@ -0,0 +1,18 @@ +## 审查结论 + +PR #72 原始修复按请求 UUID 保留回答的方向正确；审查发现并修复一项 P1 启动回归。 + +## 问题与修复 + +原 PR 将 App 的 visibleQuestionReq useMemo 放在 authState 为 checking / need-token 的提前返回之后。从启动检查或 token 提示进入已认证状态时，React 发现本次调用的 Hook 数量增加并抛出 Rendered more hooks than during the previous render。 + +修复将 memo 移到依赖计算之后、所有认证提前返回之前，保持权限优先级和引用复用。新增基于生产 App 语法树的 Hook 顺序回归，并扩展同请求重新渲染保留自定义草稿的回归。 + +## 验证 + +- 新增 Hook 顺序检查在原 PR 上失败，在修复后通过。 +- 从生产 App 提取认证分支与 memo 顺序，在 Chromium 和真实 React 18 中验证：修复前复现 Hook 数量错误；修复后 5 次认证状态切换和 2 次权限优先级检查通过，无页面错误。此验证聚焦真实 React 的认证控制流，不代表完整桌面壳验证。 +- PR 快照 pnpm test 通过，日志含 2784 条 pass；pnpm build 及 4478 个正则兼容检查通过。 +- 与 master c0f212ed 合并后的 pnpm build 通过；逐模块运行 318 个测试文件，仅 sidebarAlignmentArchitecture.test.js:22 失败，与未合并的 master 完全一致（旧断言 gap-0，现有界面 gap-2）。没有新增失败。 +- 原 PR 的 C++ 测试改动用于验证不存在的 workspace 会话返回 404；生产路由的归属检查与此一致。本次追加修复不修改 C++；原 head 4111ad0f 的远端 unit-tests 已通过。 +- OpenSpec strict 通过；修复提交与远端最终 CI、合并状态另见 PR 评论。 diff --git a/openspec/changes/fix-question-picker-rerender-state/specs/desktop-ask-user-question-ui/spec.md b/openspec/changes/fix-question-picker-rerender-state/specs/desktop-ask-user-question-ui/spec.md new file mode 100644 index 00000000..f80162e7 --- /dev/null +++ b/openspec/changes/fix-question-picker-rerender-state/specs/desktop-ask-user-question-ui/spec.md @@ -0,0 +1,33 @@ +## Purpose + +确保桌面和 Web 用户在后台状态更新或认证状态切换时仍能可靠完成多题问答。同一次提问的选择、草稿与题号应保持稳定，新提问应从初始状态开始，认证完成后主界面应正常显示。 + +## ADDED Requirements + +### Requirement: 问答状态按请求身份保留 + +系统 SHALL 在同一提问请求重新渲染时保留已选答案、自定义草稿和当前题号；收到新的提问请求时清空旧答案并回到第一题。 + +#### Scenario: 后台事件引发同请求重新渲染 +- **WHEN** 用户已完成第一题并进入第二题，后台事件提供同一请求的新对象引用 +- **THEN** 已有答案、自定义草稿和第二题位置保持不变 + +#### Scenario: 切换新请求 +- **WHEN** 用户正在回答一个请求并切换到不同请求 ID 的提问 +- **THEN** 新请求从第一题开始且没有沿用旧答案 + +### Requirement: 认证切换保持问答界面可用 + +系统 SHALL 在初始连接检查、需要 token 和认证成功之间切换时正常渲染界面；未解决的权限请求继续优先于问答显示。 + +#### Scenario: 初次连接完成 +- **WHEN** 初始连接检查完成并进入已认证状态 +- **THEN** 主界面正常显示，存在待答请求时显示对应提问 + +#### Scenario: 输入 token 后进入主界面 +- **WHEN** 用户在认证提示中提交有效 token 并完成认证 +- **THEN** 主界面正常显示，不因问答派生状态而崩溃 + +#### Scenario: 权限优先 +- **WHEN** 当前会话同时存在未解决的权限请求和提问请求 +- **THEN** 先显示权限请求，解决权限后再显示提问 diff --git a/openspec/changes/fix-question-picker-rerender-state/tasks.md b/openspec/changes/fix-question-picker-rerender-state/tasks.md new file mode 100644 index 00000000..90da6e1c --- /dev/null +++ b/openspec/changes/fix-question-picker-rerender-state/tasks.md @@ -0,0 +1,9 @@ +## 1. 审查修复 + +- [x] 1.1 增加生产 App Hook 顺序回归，确认原 PR 在认证提前返回场景下失败。 +- [x] 1.2 移动问答 memo 至认证提前返回之前，验证同请求、新请求与自定义草稿行为，以及真实 React 认证切换。 + +## 2. 合并验证 + +- [x] 2.1 完成 PR 快照的 pnpm test、pnpm build、OpenSpec strict，并比较最新 master 合并后的失败与已知基线。 +- [x] 2.2 完成修复差异审查，并在 PR 评论记录问题、修复和验证证据；远端 CI、合并与同步结果由 PR 后续记录追踪。 diff --git a/web/src/App.jsx b/web/src/App.jsx index 6938fe37..8eaa7699 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -2021,6 +2021,17 @@ export function App() { () => pendingQuestionSessionIds(questionReqs, activeId, permissionOwnership), [questionReqs, activeId, permissionOwnership], ); + // Keep every hook above the authentication early returns. + const visibleQuestionReq = useMemo(() => { + if (visiblePermissionUnresolved) return null; + const request = visibleQuestionRequest(questionReqs, activeId, permissionOwnership); + return request + ? { + ...request, + origin_label: questionOriginLabel(request, permissionOwnership), + } + : null; + }, [visiblePermissionUnresolved, questionReqs, activeId, permissionOwnership]); if (authState === 'checking') { if (desktopModeRef.current === 'shell') { const desktopStartupStatus = desktopStartupProgress?.current || null; @@ -2072,16 +2083,7 @@ export function App() { const sidebarCollapsed = view !== 'single' || (projectSidebarCollapsed && !guidedTourPreparing && !guidedTourRun); - const visibleQuestionReq = useMemo(() => { - if (visiblePermissionUnresolved) return null; - const request = visibleQuestionRequest(questionReqs, activeId, permissionOwnership); - return request - ? { - ...request, - origin_label: questionOriginLabel(request, permissionOwnership), - } - : null; - }, [visiblePermissionUnresolved, questionReqs, activeId, permissionOwnership]); + const nativeSurfacesVisible = !showSettings && !showFeedback && !searchOpen diff --git a/web/src/lib/inlinePermissionArchitecture.test.js b/web/src/lib/inlinePermissionArchitecture.test.js index 656fb59e..ffb4b574 100644 --- a/web/src/lib/inlinePermissionArchitecture.test.js +++ b/web/src/lib/inlinePermissionArchitecture.test.js @@ -2,6 +2,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +import { parseSync } from '@babel/core'; const srcRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); @@ -27,6 +28,36 @@ function run(name, fn) { } } +run('App hooks run before authentication branches can return early', () => { + const ast = parseSync(source('App.jsx'), { + configFile: false, babelrc: false, parserOpts: { plugins: ['jsx'] }, + }); + const app = ast.program.body.find((node) => ( + node.type === 'ExportNamedDeclaration' && node.declaration?.id?.name === 'App' + ))?.declaration; + assert.ok(app, 'the production App component must exist'); + + // Callback/helper returns do not return from App, and their hooks have a + // separate lifetime. Inspect only control flow in the component itself. + function componentNodes(node) { + if (!node || typeof node !== 'object') return []; + if (/Function|Method/.test(node.type || '')) return []; + return [node, ...Object.values(node).flatMap((value) => ( + Array.isArray(value) ? value.flatMap(componentNodes) : componentNodes(value) + ))]; + } + + let canReturnEarly = false; + for (const statement of app.body.body) { + const nodes = componentNodes(statement); + const hooks = nodes.filter((node) => node.type === 'CallExpression' + && node.callee.type === 'Identifier' && /^use[A-Z]/.test(node.callee.name)); + assert.ok(!canReturnEarly || hooks.length === 0, + `App calls ${hooks.map((node) => node.callee.name).join(', ')} after an early return`); + if (nodes.some((node) => node.type === 'ReturnStatement')) canReturnEarly = true; + } +}); + run('permission cards are chat rows inside the transcript before activity', () => { const chat = source('components/ChatView.jsx'); const renderer = source('components/TranscriptItems.jsx'); diff --git a/web/src/lib/questionPickerInteraction.test.js b/web/src/lib/questionPickerInteraction.test.js index 7caa6ccf..5cffe09b 100644 --- a/web/src/lib/questionPickerInteraction.test.js +++ b/web/src/lib/questionPickerInteraction.test.js @@ -227,10 +227,12 @@ run('same request re-rendered with a new object reference keeps answers and posi const tabular = (tree) => nodes(tree).find( (n) => /tabular-nums/.test(n.props?.className || '') && typeof n.props.children === 'string'); assert.equal(tabular(picker.render()).props.children, '2 / 2'); + input(picker.render()).props.onChange({ target: { value: 'keep this draft' } }); // 模拟 App 重渲染:request_id 不变,但传入内容相同、引用全新的 request 对象。 picker.render({ request_id: 'r1', session_id: 's1', questions: [q1, q2] }); // 修复前:重置 effect 误把引用变化当作新请求 -> 跳回第一题('1 / 2')并清空答案。 assert.equal(tabular(picker.render()).props.children, '2 / 2'); + assert.equal(input(picker.render()).props.value, 'keep this draft'); // 回到第一题,断言已选答案内容仍保留(不只是位置没跳)。 button(picker.render(), '上一题').props.onClick(); assert.match(rows(picker.render())[0].props.className, /text-accent/);