Skip to content

Commit aa5ab47

Browse files
committed
fix(webui): stop the shell from carrying one session's state into another
Three leaks, all from state that lived outside the component that should have owned it. page.tsx read localStorage during render, so the pre-rendered HTML and the first client frame could not agree — a skeleton screen was hiding it, which is exactly the kind of cover that disappears the moment someone edits the shell. The first frame now uses defaults and one mount effect restores; the three write-back mirrors are gated so a default never overwrites a stored value. Scroll position is read by the same sessionKey effect Chat already had, which reads the same key. The draft store was a module-level bucket, so a draft, a failure banner and a model chip all followed you across sessions — type into one conversation, switch, and your words are in the other. The store is keyed by session now. Isolation is not discarding: switching back finds the draft still there. Clearing was the alternative and it destroys an unread banner every time you return to a conversation. #126 left the accepted-but-unconfirmed banner without anything to consume it when a turn ended. unconfirmedPatchOnTurnEnd clears it on the falling edge of running, and only there — the three-value decision about when to show it is untouched.
1 parent 8c085fa commit aa5ab47

11 files changed

Lines changed: 842 additions & 147 deletions

‎docs/webui.md‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2040,6 +2040,17 @@ failure mode we care about is the `app/global-error.tsx` crash, not a quota
20402040
error here. Per-session scroll keys are deliberate: a refresh restores
20412041
the user's place in each conversation independently.
20422042

2043+
One timing invariant guards all of it (webui-parity 106): the page root
2044+
never reads these keys during render. The prerendered server HTML and the
2045+
client's first (hydration) render must be identical, and a render-phase
2046+
storage read breaks that equality the moment the `state === null` skeleton
2047+
changes shape. `app/page.tsx` renders its first frame from the shared
2048+
DEFAULT constants and applies the stored payload in one post-mount effect;
2049+
the three write-back mirrors are gated on that restore having run, so the
2050+
defaults-seeded first render cannot overwrite the stored payload. What the
2051+
user sees is unchanged: the skeleton is still up while the restore lands,
2052+
and by the time the first snapshot arrives the saved layout is in place.
2053+
20432054
## Slash commands: which endpoint answers them (webui-parity ticket 65)
20442055

20452056
A `/`-prefixed line in the composer is not automatically a command. Two
@@ -2252,6 +2263,23 @@ losing what the user typed is the worse defect, and the banner carries the
22522263
"check the history first" instruction that makes the restore safe. The
22532264
banner is also styled as secondary text rather than as an error.
22542265

2266+
The banner's *display* semantics are the three answers above; its *dismissal*
2267+
is separate (webui-parity 106). While `running.active` is up, the warning is
2268+
doing its job. When the flag falls — the turn it warned about is over — the
2269+
grey banner goes with it (`unconfirmedPatchOnTurnEnd` in
2270+
`webapp/lib/composer-draft.ts`, applied by a composer effect that watches the
2271+
running-flag fall): after `sleep 35` finished, the banner used to sit under
2272+
the input until the next send or a reload. A real `rejected` refusal keeps
2273+
its dismiss paths; no display rule changed.
2274+
2275+
The banner is also addressed, not broadcast. The draft store is keyed by
2276+
session, and the catch branch writes the banner into the key of the session
2277+
the send was dispatched FROM — so a failure recorded in session A while the
2278+
user has already switched to session B never paints B red; the user finds
2279+
the banner when they return to A. The previous behaviour (a module-scope
2280+
shared box, then #141's clear-on-switch) either bled the banner across
2281+
sessions or destroyed the returning session's own unread one.
2282+
22552283
A client-generated idempotency key on `POST /api/send` would make the
22562284
duplicate structurally impossible rather than merely unlikely. It is not
22572285
implemented: it is a request-contract change, and it needs a
@@ -2263,6 +2291,38 @@ an engine turn, and leave the tab open: the output is still there ten
22632291
seconds later, and it is still there after a reload. Force an
22642292
acknowledgement timeout against a server that is running the turn: the
22652293
banner says the engine is running the message, and the composer is empty.
2294+
Wait for the turn to finish: the grey banner disappears on its own.
2295+
2296+
## The composer's state is per-session (webui-parity 106)
2297+
2298+
Everything the user has parked in the composer — typed text, attachment
2299+
chips, the send-error banner — is stored under the active session's key
2300+
(`webapp/lib/composer-draft.ts`, a `Map` keyed by `state.sessionId`; `""` is
2301+
the no-session home-screen bucket). Switching sessions swaps the whole box:
2302+
session B never shows session A's draft or banner, and both survive the
2303+
round trip. The smoke run's s28 capture was the shared-bucket version of
2304+
this store: session 2's view showing session 1's draft, 409 banner and
2305+
model chip at the same time.
2306+
2307+
Per-session storage, not clear-on-switch, is the deliberate choice: a
2308+
clear-on-switch effect (the #141 interim fix) also fires when the user
2309+
comes BACK, destroying the very draft and unread banner they returned for.
2310+
Keyed storage keeps the good half of the old global behaviour (nothing is
2311+
lost when hopping between sessions) while removing the bleed. Drafts are
2312+
not persisted to `localStorage` — they are working state for the current
2313+
page visit; the persisted surface stays `lib/persist.ts`'s contract.
2314+
2315+
The model picker's chip VALUE always read the server snapshot and needs no
2316+
isolation; its local UI state (open cascade, previewed row, per-model draft
2317+
mirror) resets when the session key changes, so no menu state from session A
2318+
visually persists into session B's view. Whether a model pick made in one
2319+
session's view can land in another session's engine config is a
2320+
server-side `applyConfigOptionUpdate` question and out of this ticket's
2321+
frontend scope.
2322+
2323+
**How you would tell it works.** Type a draft in session A, switch to
2324+
session B: B's composer is empty and the chip follows B's server model.
2325+
Switch back: A's draft and any unread failure banner are exactly as left.
22662326

22672327
## Endpoint catalog (against current source)
22682328

‎docs/webui.zh-CN.md‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1481,6 +1481,14 @@ loading-states 相同:让 SSR 渲染测试可以脱离 `chat.tsx` 的 `@/` 别
14811481
错误。会话内每个 sessionId 单独存储滚动位置 —— 按会话恢复滚动位置
14821482
是有意为之的契约。
14831483

1484+
一条时序不变式守着这一切(webui-parity 106):页面根组件**绝不在渲染期读
1485+
这些键**。预渲染的服务端 HTML 与客户端首次(hydration)渲染必须逐字节
1486+
一致,渲染期读存储会在 `state === null` 骨架屏第一次改形时炸出不一致。
1487+
`app/page.tsx` 首帧用共享的 DEFAULT 常量渲染,挂载后的一个 effect 统一
1488+
套用存储值;三处写回镜像都加闸在该恢复之后,默认值首帧不可能覆盖存储
1489+
payload。用户看到的东西不变:恢复落地时骨架屏仍亮着,第一份快照到达时
1490+
保存过的布局已经就位。
1491+
14841492
## 斜杠命令走哪个端点(webui-parity ticket 65)
14851493

14861494
输入框里以 `/` 开头的一行**不等于**命令。两个端点都能消费斜杠输入,
@@ -1659,13 +1667,53 @@ composer 实际调用的那个函数。
16591667
回填草稿——让用户输入的内容消失是更严重的缺陷,而文案里带着「先查历史」
16601668
这句指引,回填才是安全的。该错误条同时改用次要文字色,不再是错误红。
16611669

1670+
上面三种答案定的是错误条**何时显示**;**何时消失**是另一件事
1671+
(webui-parity 106)。`running.active` 亮着时,灰条在履行职责;这个标志
1672+
落下——它警告的那个回合结束了——灰条随之消失
1673+
(`webapp/lib/composer-draft.ts#unconfirmedPatchOnTurnEnd`,composer 里
1674+
一个盯 running 下降沿的 effect 负责套用):此前 `sleep 35` 跑完后,灰条
1675+
会一直挂在输入框下直到下次发送或刷新。真正的 `rejected` 拒绝保持原有的
1676+
消失路径;显示判定一字未动。
1677+
1678+
错误条还是**有归属**的,不是广播。草稿存储按会话分键,catch 分支把红条
1679+
写进**发起发送的那个会话**的键下——用户在会话 A 发送失败后已经切到
1680+
会话 B,B 的输入框永远不会因此变红;回到 A 时才看到这条失败。旧行为
1681+
(模块级共享桶,再到 #141 的切换即清)要么把红条串到别的会话,要么把
1682+
用户正要回去看的那个会话自己的红条销毁掉。
1683+
16621684
给 `POST /api/send` 加一个客户端生成的幂等键,可以让重复执行从「不太可能」
16631685
变成「结构上不可能」。本次没做:那是请求契约变更,还需要服务端带明确时间窗
16641686
的去重存储。留作独立一单,不塞进这次修复。
16651687

16661688
**怎么验证它真的好了。** 在一个已经有引擎回合的会话里发 `/help`,把标签页
16671689
放着:十秒后输出还在,刷新之后还在。对着一个「回合正在跑」的服务器制造一次
1668-
确认超时:错误条会说引擎正在执行这条消息,且输入框是空的。
1690+
确认超时:错误条会说引擎正在执行这条消息,且输入框是空的。等这个回合跑完:
1691+
灰色错误条自己消失。
1692+
1693+
## 输入区的状态按会话隔离(webui-parity 106)
1694+
1695+
用户停在输入区的一切——正在打的文字、附件 chips、发送失败红条——都存在
1696+
当前会话的键下(`webapp/lib/composer-draft.ts`,以 `state.sessionId` 为键
1697+
的 `Map`;`""` 是首页无会话的桶)。切换会话就是换一个盒子:会话 B 永远
1698+
不会显示会话 A 的草稿或红条,来回切换两边的状态都不丢。质检 s28 截图
1699+
拍到的正是这个存储的共享桶版本:会话 2 的视图同时挂着会话 1 的草稿、
1700+
409 红条和模型 chip。
1701+
1702+
按会话存储、而不是「切换时清空」,是权衡后的决定:清空 effect(#141 的
1703+
过渡修法)在用户**切回来**时同样触发,恰恰毁掉他们回来要看的那份草稿和
1704+
没读完的红条。按会话分键保住了旧全局行为里好的那一半(来回跳会话什么都不
1705+
丢),又去掉了串扰。草稿不落 `localStorage`——它们是本次页面访问的工作
1706+
状态;持久化面仍归 `lib/persist.ts` 的契约管。
1707+
1708+
模型选择器 chip 的**值**一直读服务端快照,本就不需要隔离;它的本地 UI
1709+
状态(打开的级联、预览中的行、按模型记的草稿镜像)在会话键变化时重置,
1710+
会话 A 的菜单状态不会在会话 B 的视图里残留。至于在一个会话视图里做的
1711+
模型选择会不会落进另一个会话的引擎配置,那是服务端
1712+
`applyConfigOptionUpdate` 的事,不在本单前端范围内。
1713+
1714+
**怎么验证它真的好了。** 在会话 A 打一段草稿,切到会话 B:B 的输入框是
1715+
空的,chip 跟着 B 的服务端模型走。切回 A:草稿和没读完的红条原样都在。
1716+
16691717

16701718
## 端点清单(依据当前源码)
16711719

‎packages/webui/webapp/app/page.tsx‎

Lines changed: 92 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import { useLocale } from "@/lib/use-locale";
2323
import {
2424
DEFAULT_UI_STATE,
2525
DEFAULT_WORKSPACE_TABS_STATE,
26-
readScrollPosition,
2726
readUiState,
2827
readWorkspaceTabs,
2928
writeScrollPosition,
@@ -78,24 +77,38 @@ export default function Page() {
7877
function App() {
7978
const { locale, setLocale, t } = useLocale();
8079
const { state, connected, error } = useSessionContext();
81-
// Webui-parity 07 — restore UI state synchronously from localStorage
82-
// BEFORE the first paint, so a refresh on /?session=A lands on the
83-
// same right-panel / sidebar collapsed choice the user previously
84-
// had open rather than flashing the default first.
85-
const [persisted] = useState<UiState>(() => readUiState());
86-
// Slice 17 — restore the workspace-tabs payload (open tabs +
87-
// per-column active ids + column widths + collapsed flags) the
88-
// same way. The first paint already knows whether the preview /
89-
// tree columns should be open and which tabs are inside them, so
90-
// a refresh on the new shell does not flash the empty launcher
91-
// before restoring the saved tabs.
92-
const [workspaceTabs] = useState<WorkspaceTabsState>(() => readWorkspaceTabs());
80+
// Webui-parity 07 — restore UI state from localStorage so a refresh on
81+
// /?session=A lands on the same right-panel / sidebar collapsed choice
82+
// the user previously had open rather than flashing the default first.
83+
//
84+
// webui-parity 106 (smoke-report P7-b): the reads moved OUT of the render
85+
// phase. This page is prerendered by the Next.js static export, so the
86+
// server HTML and the client's first (hydration) render must be identical;
87+
// a storage-read state initializer returns defaults on the server but
88+
// stored values on the client, which is a hydration mismatch waiting for
89+
// the first change to the `!state` skeleton to go off. The
90+
// first frame therefore renders from DEFAULT_UI_STATE — the same constants
91+
// the server used — and the effect below applies the stored values right
92+
// after mount, while the skeleton is still up (the SSE snapshot has not
93+
// arrived either). By the time real content replaces the skeleton, the
94+
// restored layout is already in place, so the pre-106 no-flash restore
95+
// behaviour is preserved.
96+
const [persisted, setPersisted] = useState<UiState>(DEFAULT_UI_STATE);
97+
// Slice 17 — restore the workspace-tabs payload (open tabs + per-column
98+
// active ids + column widths + collapsed flags) the same way: defaults on
99+
// the first frame, storage values applied by the mount effect below, so
100+
// a refresh does not flash the empty launcher before the saved tabs land
101+
// — and does not read storage during hydration either.
102+
const [workspaceTabs, setWorkspaceTabs] = useState<WorkspaceTabsState>(
103+
DEFAULT_WORKSPACE_TABS_STATE,
104+
);
93105
// The legacy `panel` mirror is still kept around so the
94106
// toolbar's existing "active panel" highlight survives the
95107
// refactor without a fresh state mirror — slice 17 keeps the
96108
// toolbar / panel highlight working through the new tab strip
97109
// system (the active tab's kind is the toolbar highlight).
98-
const [panel, setPanel] = useState<typeof persisted.panel>(persisted.panel);
110+
// Seeded null (the default) and restored by the mount effect.
111+
const [panel, setPanel] = useState<typeof persisted.panel>(null);
99112
// Settings is a dialog rather than a drawer panel, so it has its own state.
100113
const [settingsOpen, setSettingsOpen] = useState(false);
101114
const [settingsSection, setSettingsSection] = useState<"general" | "connection" | "providers">("general");
@@ -104,23 +117,44 @@ function App() {
104117
const [sessionHint, setSessionHint] = useState<{ kind: "not-found"; sessionId: string } | null>(null);
105118
const alertCount = useAlertCount();
106119

107-
// Workspace tabs live-state (slice 17). The `useState` initializer
108-
// seeds from the persisted payload; subsequent edits mutate via
109-
// the pure reducers and the effect below mirrors them back into
110-
// `lib/persist.ts` storage.
111-
const [tabState, setTabState] = useState<TabStripState>(workspaceTabs.tabStrip);
112-
const [columnState, setColumnState] = useState<typeof DEFAULT_COLUMN_LAYOUT>(workspaceTabs.columnLayout);
113-
// Slice 17 — WorkspaceColumns self-measures via
114-
// ResizeObserver, so the page does not need to feed
115-
// containerWidth anymore. viewportWidth is still threaded
116-
// through for the future auto-collapse ladder; today it is
117-
// accepted but unused inside computeColumnLayout.
118-
const viewportWidth = typeof window !== "undefined" ? window.innerWidth : 1280;
120+
// Workspace tabs live-state (slice 17). Seeded from the DEFAULT tab strip
121+
// (see the hydration note above); the mount effect below applies the
122+
// persisted payload, and subsequent edits mutate via the pure reducers,
123+
// mirrored back into `lib/persist.ts` storage by the gated effect.
124+
const [tabState, setTabState] = useState<TabStripState>(
125+
DEFAULT_WORKSPACE_TABS_STATE.tabStrip,
126+
);
127+
const [columnState, setColumnState] = useState<typeof DEFAULT_COLUMN_LAYOUT>(
128+
DEFAULT_WORKSPACE_TABS_STATE.columnLayout,
129+
);
130+
// webui-parity 106 — false until the mount effect has applied the stored
131+
// UI state. The three write-back mirrors below are gated on it: without
132+
// the gate, the first (defaults-seeded) render would overwrite the user's
133+
// stored payload with DEFAULT_UI_STATE before the restore ever ran.
134+
const [uiRestored, setUiRestored] = useState(false);
135+
136+
// The one client-side storage read. Runs after mount — never during
137+
// render, never during hydration — and applies everything in one batch,
138+
// so the skeleton frame the user is still looking at is the last frame
139+
// painted from defaults.
140+
useEffect(() => {
141+
const restoredUi = readUiState();
142+
const restoredTabs = readWorkspaceTabs();
143+
setPersisted(restoredUi);
144+
setWorkspaceTabs(restoredTabs);
145+
setTabState(restoredTabs.tabStrip);
146+
setColumnState(restoredTabs.columnLayout);
147+
setPanel(restoredUi.panel);
148+
setUiRestored(true);
149+
}, []);
119150

120151
// Mirror panel changes into localStorage. The write helper is
121152
// debounced; mounting/de-mounting the panel quickly during a
122-
// refresh never floods storage.
153+
// refresh never floods storage. Gated on `uiRestored` so the
154+
// defaults-seeded first render cannot clobber the stored payload
155+
// (webui-parity 106).
123156
useEffect(() => {
157+
if (!uiRestored) return;
124158
writeUiState({
125159
...DEFAULT_UI_STATE,
126160
...persisted,
@@ -129,15 +163,23 @@ function App() {
129163
// intentionally not adding `persisted` to deps — the persist
130164
// module already guards the debounced write.
131165
// eslint-disable-next-line react-hooks/exhaustive-deps
132-
}, [panel]);
166+
}, [panel, uiRestored]);
133167

134168
// Slice 17 — mirror workspace-tabs state into localStorage. The
135169
// debounced writer coalesces open + activate + scroll edits into
136-
// one write.
170+
// one write. Gated on `uiRestored` for the same reason as above.
137171
useEffect(() => {
172+
if (!uiRestored) return;
138173
writeWorkspaceTabs({ tabStrip: tabState, columnLayout: columnState });
139174
// eslint-disable-next-line react-hooks/exhaustive-deps
140-
}, [tabState, columnState]);
175+
}, [tabState, columnState, uiRestored]);
176+
177+
// Slice 17 — WorkspaceColumns self-measures via
178+
// ResizeObserver, so the page does not need to feed
179+
// containerWidth anymore. viewportWidth is still threaded
180+
// through for the future auto-collapse ladder; today it is
181+
// accepted but unused inside computeColumnLayout.
182+
const viewportWidth = typeof window !== "undefined" ? window.innerWidth : 1280;
141183

142184
// ============================================================
143185
// Workspace tabs reducers (the page wires every action through
@@ -477,6 +519,12 @@ function App() {
477519

478520
useEffect(() => {
479521
if (!urlRestored) return;
522+
// webui-parity 106 — same gate as the other write mirrors: this effect
523+
// can fire before the mount restore has applied the stored payload
524+
// (SSE sometimes names a session before the restore batch lands), and
525+
// writing from the defaults-seeded `persisted` would drop the stored
526+
// appearance / sidebar choice.
527+
if (!uiRestored) return;
480528
const active = state?.mcodeSessionId ?? null;
481529
writeUiState({
482530
...DEFAULT_UI_STATE,
@@ -485,7 +533,7 @@ function App() {
485533
lastSessionId: active,
486534
});
487535
// eslint-disable-next-line react-hooks/exhaustive-deps
488-
}, [state?.mcodeSessionId, urlRestored]);
536+
}, [state?.mcodeSessionId, urlRestored, uiRestored]);
489537

490538
useEffect(() => {
491539
const onPop = () => {
@@ -739,7 +787,16 @@ function App() {
739787
}
740788

741789
/**
742-
* Scroll-restored chat wrapper — unchanged from slice 07.
790+
* Scroll-restored chat wrapper.
791+
*
792+
* webui-parity 106: the page no longer reads the scroll position out of
793+
* localStorage during render (the pre-106 render-phase read was the third
794+
* instance of the hydration bomb). `Chat` already re-reads the SAME
795+
* per-session key (`webui:scroll:v1:<cid>:<sessionId>`) inside its own
796+
* post-mount restore effect whenever `sessionKey` changes, and it falls back
797+
* to that read whenever no explicit scroll target arrives — so passing
798+
* nothing restores the identical number, from an effect that only runs
799+
* client-side. The page keeps only the write half of the contract.
743800
*/
744801
function ScrollRestoredChat({
745802
t,
@@ -752,13 +809,11 @@ function ScrollRestoredChat({
752809
sessionId: string | null;
753810
onOpenFile?: (path: string) => void;
754811
}) {
755-
const initial = sessionId ? readScrollPosition(sessionId) : 0;
756812
return (
757813
<Chat
758814
t={t}
759815
locale={locale}
760816
sessionKey={sessionId}
761-
initialScrollTop={initial}
762817
onScrollPersist={(top) => {
763818
if (!sessionId) return;
764819
writeScrollPosition(sessionId, top);
@@ -769,10 +824,9 @@ function ScrollRestoredChat({
769824
}
770825

771826
// keep the unused-export lint happy: slice 17 deliberately does
772-
// not pull `panel` / `openPanel` / `openSettings` / `DEFAULT_UI_STATE`
773-
// from the legacy path. They stay in scope so a future ticket can
774-
// revive them without re-importing the modules.
827+
// not pull `panel` / `openPanel` / `openSettings` from the legacy
828+
// path. They stay in scope so a future ticket can revive them
829+
// without re-importing the modules.
775830
void PreviewColumn;
776-
void DEFAULT_WORKSPACE_TABS_STATE;
777831
void isHtmlPath;
778832
void useRef;

0 commit comments

Comments
 (0)