From bdda3d2ccf715413ca7d6d5cd49eb3537fc0097d Mon Sep 17 00:00:00 2001 From: s39-dev Date: Tue, 29 Sep 2026 01:11:32 +0800 Subject: [PATCH 1/3] fix(webui): re-point workspace on session switch (parity ticket 39) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User report: the file tree did not follow the switched-to session's workspace, so switching from project A to project B left the tree showing A's files. Root cause lived in three places in routes/sessions.js#handleSwitchSession: 1. The handler read cs.workspace but never wrote it, so cs.workspace.dir stayed bound to the prior project. 2. The handler stamped every newly-created mvs_ overlay with the CURRENT cs.workspace.dir (ensureOverlayForMcodeSid(all, id, { title, workspace: ws })), so first-touch of an mvs_ session from project A inherited A's path — switching back from B then either left the tree on B or, under the prior mutation, rewrote the overlay's workspace to B's path, polluting every per-project grouping. (webui-parity 39 ② pollution) 3. The front-end never called setWorkspace from the sidebar switch path, so even if the server side had fixed itself, the UI would still have to ride on the SSE push. Fix lands in handleSwitchSession: - new helper _resolveSwitchWorkspace(target, currentWs) picks the target's stored workspace first, falls back to DEFAULT_WORKSPACE (env MCODE_WORKSPACE > mcode TUI cwd.json > homedir) when empty, and refuses with 400 if the resolved path fails assertWorkspacePath containment. - newly-created overlays no longer receive the workspace argument; they start with workspace:"" and the target-first read picks DEFAULT_WORKSPACE for them. - the response session payload now carries workspace + workspaceFallback so the front-end can re-derive without waiting for the SSE push. - the audit event session.switch now records workspace + workspaceFallback. Mid-run safety: the run-mirror buffer (lib/state-bus.js#runChatByCid) is keyed by (cid, mcodeSessionId) and the engine child holds its own cwd from turn-start, so cs.workspace.dir changing does not abort the in-flight turn. Switching while a turn is running keeps the existing finalize contract (still-viewing writes into cs.chat; switched-away writes into the owning record). Tests: - new sessions-switch-workspace-follow.check.mjs: 8 unit tests pinning the target-first read, the empty-fallback to DEFAULT_WORKSPACE, the no-overwrite invariant for both existing records and newly-created overlays, and the 400 on out-of-bounds / traversal stored workspaces. Mutation tripwires: re-introducing "workspace: ws" on the ensureOverlay call must turn the new-overlay test red; removing the containment gate must turn both out-of-bounds tests red. - pre-existing sessions.check.mjs / sessions-switch.check.mjs / chat-run-mirror.check.mjs fixtures switched their placeholder workspaces ("/ws-A" etc.) to a real, contained path ("/tmp/webui-test-{A,B}") because the fix now requires stored workspaces to pass assertWorkspacePath's realpathSync. Documentation: - docs/webui.md: new "Session switch follows workspace (ticket 39)" section pinning the contract, the mid-run safety argument, and the failure modes. - docs/webui.zh-CN.md: equivalent Chinese section under the same heading convention. --- docs/webui.md | 90 ++++ docs/webui.zh-CN.md | 35 ++ packages/webui/server/routes/sessions.js | 136 +++++- .../test/routes/chat-run-mirror.check.mjs | 2 +- ...sessions-switch-workspace-follow.check.mjs | 389 ++++++++++++++++++ .../test/routes/sessions-switch.check.mjs | 16 +- packages/webui/test/routes/sessions.check.mjs | 78 ++-- release/public-source.json | 1 + 8 files changed, 693 insertions(+), 54 deletions(-) create mode 100644 packages/webui/test/routes/sessions-switch-workspace-follow.check.mjs diff --git a/docs/webui.md b/docs/webui.md index 406116a0c..416357034 100644 --- a/docs/webui.md +++ b/docs/webui.md @@ -167,6 +167,96 @@ Contract details: The operator-facing view — which models show what control, and why MiniMax-M3 only has on/off — is the thinking section of [`webui.zh-CN.md`](webui.zh-CN.md). +## Session switch follows workspace (webui-parity ticket 39) + +Switching to another session re-points the active workspace to the +session's stored workspace. The file tree (slice 01) re-roots under +the new directory; the old project's `webui:files-tree:` +key is left intact in `sessionStorage` so the user returns to the +same expanded set if they switch back. + +### Contract + +`POST /api/sessions/switch` (`routes/sessions.js#handleSwitchSession`) +resolves the workspace in this order: + +1. **Target session's stored `workspace`** — that is the workspace the + user was in when they last had this session open, modulo any + pollution a previous code path introduced. +2. **`DEFAULT_WORKSPACE`** (env `MCODE_WORKSPACE` > mcode TUI + `cwd.json` > `homedir()`) when the stored value is empty. Empty + is the shape legacy sessions or `②`-polluted records carry. +3. **Refuse with `400`** if the resolved path fails + `assertWorkspacePath` containment (e.g. the user tightened + `MCODE_WEBUI_WORKSPACE_ROOTS` since the session was last opened). + +The chosen path runs through the same `assertWorkspacePath` gate that +the workspace picker (`/api/workspace`), +`browseWorkspace` (`/api/workspace/browse`), the new-session POST, and +`/api/fs/*` all funnel through — refusing to switch into an +out-of-bounds path is the same boundary the picker refuses to land on. + +The switch NEVER overwrites the target session's stored `workspace` +with the previous `cs.workspace.dir`. That was the pre-fix behaviour +(ticket 39's ② pollution path): every first-touch of an `mvs_` +session from project A inherited A's path, so the per-project +session grouping ended up duplicating A's directory for every +session the user opened from A. Newly-created overlays now start with +`workspace: ""`; the target-first read picks `DEFAULT_WORKSPACE` for +them. + +The switch does NOT update `cs.lastUsedWorkspace`. The session bar's +"recent" sort (slice 07) is written only by `handleSend`; switching is +browsing, not authoring, and the previous "click any session and the +session jumps to the top" behaviour was the report that pinned that +contract. + +### Mid-run safety + +Switching mid-run does NOT abort the in-flight turn. The run's +ownership and stream buffer are keyed by `(cid, mcodeSessionId)` in +`lib/state-bus.js#runChatByCid`, not by workspace. The engine child +process holds its own cwd from when the turn started; the new +`cs.workspace.dir` is purely the next-viewing surface. Finalize-time +behaviour (`routes/chat.js` finalize drain) is unchanged: still-viewing +writes into `cs.chat`; switched-away writes via +`appendChatToSession(owningSid, lines)`. + +### What the user sees + +- **Side effect**: the file-tree panel re-roots under the new + workspace's root; expansion / filter / showHidden for the old + workspace stay preserved in `sessionStorage`, the new workspace + starts at its own stored expansion (or empty if never opened). +- **Failure mode**: an out-of-bounds stored workspace returns `400` + with the gate's actionable message + (`工作区越界: 不在任何允许根内。允许根: …`). `cs.workspace.dir` + is NOT re-pointed; the previous workspace stays active. +- **Pre-existing polluted sessions** (recorded before this fix carried + the previous project's path): the new rule reads the stored value + verbatim. The user sees the polluted path's file tree until they + open the workspace picker and re-pick the intended directory; that + one re-pick rewrites the stored value to the canonical realpath. + +### What contributors will change + +- `routes/sessions.js#handleSwitchSession` adds + `_resolveSwitchWorkspace(target, currentWs)` and writes + `cs.workspace = { dir: switchWs.dir, branch: null, tree: null }` + after the target is resolved and before `resetContext`. +- `routes/sessions.js#handleSwitchSession` no longer passes + `workspace: ws` to `ensureOverlayForMcodeSid` — new overlays start + with `workspace: ""`; the read picks `DEFAULT_WORKSPACE` for + first-touch `mvs_` switches. +- The response payload gains `session.workspace` and + `session.workspaceFallback`; the SSE state push (handled by + `pushStateFor(cid)` at the end) carries the new `cs.workspace.dir` + verbatim, so `FilesPanel`'s `useSessionContext()` subscription + re-renders without any client-side wiring change. +- `routes/sessions.js#_eventsAppend("session.switch", …)` records + `workspace` and `workspaceFallback` so post-mortems can answer + "why did the file tree jump". + ## File tree (delivered UI) Every shipped file tree, panel and column evidence is `grep`-able. The list diff --git a/docs/webui.zh-CN.md b/docs/webui.zh-CN.md index ed96e6634..6a8da6a71 100644 --- a/docs/webui.zh-CN.md +++ b/docs/webui.zh-CN.md @@ -171,6 +171,41 @@ exec 回合的代价——以下都是当前真实存在的行为,选择权限 接口契约(字段、两条下发通道、会话启动时的重放规则)见 [`webui.md`](webui.md) 的 Thinking levels 一节。 +## 切换会话会带工作区一起切(webui-parity ticket 39) + +切换到另一个会话时,当前工作区会跟到目标会话上次所在的工作区。文件树(slice 01)在新目录下重新取根;旧项目在 `sessionStorage` 里的 `webui:files-tree:` 展开态保留不动,所以切回去仍是同一份展开。 + +### 契约 + +`POST /api/sessions/switch`(`routes/sessions.js#handleSwitchSession`)按以下顺序解析工作区: + +1. **目标会话存的 `workspace`**——即该会话上一次被打开时所在的工作区。该字段在旧的实现里被污染过,所以这里读出来就是用户上次看到的样子。 +2. **`DEFAULT_WORKSPACE`**(环境变量 `MCODE_WORKSPACE` > mcode TUI 的 `cwd.json` > `homedir()`),当存的值为空字符串时。空值是老会话、或被 ticket 39 标记为 ② 污染过的记录会出现的形状。 +3. **解析得到的路径无法通过 `assertWorkspacePath` 围栏**时(例如会话记录的工作区落在用户后续收窄的 `MCODE_WEBUI_WORKSPACE_ROOTS` 之外),**400 拒绝**,不让用户带着非法工作区进入路径。 + +选定的路径走的是工作区选择器(`/api/workspace`)、`browseWorkspace`(`/api/workspace/browse`)、新建会话 POST、`/api/fs/*` 共用的同一道 `assertWorkspacePath` 围栏——拒绝切到越界路径,与选择器拒绝落在越界位置是同一条边界。 + +切换**永远不会**用前一个 `cs.workspace.dir` 覆盖目标会话存的 `workspace`。那是旧代码的行为(ticket 39 的 ② 污染路径):在项目 A 首次打开某个 `mvs_` 会话时,新建的壳记录被印上 A 的路径,于是按项目分组的会话表里把 A 的目录复制给了每一个从 A 打开的会话。修复后,新建壳的 `workspace: ""`,由后续的「读目标,缺失则 DEFAULT_WORKSPACE」逻辑接管。 + +切换**不会**更新 `cs.lastUsedWorkspace`。侧栏"最近"排序(slice 07)只由 `handleSend` 写——切换是浏览,不是创作;旧版"点哪个会话哪个就置顶"的用户反馈已经把那条契约钉死了。 + +### Mid-run 安全 + +回合正在跑的时候切换会话**不会**打断这个回合。回合的所有权和流缓冲以 `(cid, mcodeSessionId)` 为键存在 `lib/state-bus.js#runChatByCid`,与工作区无关。引擎子进程持有回合开始时的 cwd,新的 `cs.workspace.dir` 只是"接下来要在哪个工作区里看"。终态时(`routes/chat.js` finalize drain)的行为保持不变——仍在查看就写进 `cs.chat`,已切走就走 `appendChatToSession(owningSid, lines)`。 + +### 用户能看到什么 + +- **侧效**:文件树面板在新工作区下重新取根;旧工作区的展开/过滤/显示隐藏项仍存在 `sessionStorage` 里,新工作区从它自己存过的展开态(若从未打开则为空)开始。 +- **失败形态**:越界的工作区返回 `400`,带围栏的明确文案(`工作区越界: 不在任何允许根内。允许根: …`)。`cs.workspace.dir` 不会被改,前一个工作区继续生效。 +- **历史上被污染过的会话**(修复之前被打上过错误工作区路径的记录):新规则会原样读出存的值。用户看到的是被污染过的目录的文件树,需要打开工作区选择器重选一次预期目录;那一次重选会把存的值改写成 canonical realpath。 + +### 改动落在哪里(给后续维护者) + +- `routes/sessions.js#handleSwitchSession` 新增 `_resolveSwitchWorkspace(target, currentWs)`,在 target 解析完成、`resetContext` 之前写入 `cs.workspace = { dir: switchWs.dir, branch: null, tree: null }`。 +- `routes/sessions.js#handleSwitchSession` 不再向 `ensureOverlayForMcodeSid` 传 `workspace: ws`——新建壳的 `workspace: ""`,由读路径在首次 `mvs_` 接触时回退到 `DEFAULT_WORKSPACE`。 +- 响应负载多了 `session.workspace` 和 `session.workspaceFallback`;尾部 `pushStateFor(cid)` 推 SSE 时原样带上新的 `cs.workspace.dir`,`FilesPanel` 通过 `useSessionContext()` 订阅自然重新渲染,前端**不**需要为了这次修复改动任何接线。 +- `routes/sessions.js#_eventsAppend("session.switch", …)` 写入 `workspace` 和 `workspaceFallback`,事后追查"为什么文件树跳了"时可以从审计链里直接定位。 + ## 文件树(已发布的 UI) 下方每个已发布的文件树、面板与列都给出组件文件锚点与一个 diff --git a/packages/webui/server/routes/sessions.js b/packages/webui/server/routes/sessions.js index 77d6333fc..4d3e6739f 100644 --- a/packages/webui/server/routes/sessions.js +++ b/packages/webui/server/routes/sessions.js @@ -33,7 +33,7 @@ import { clients, runChatViewChat, } from "../lib/state-bus.js"; -import { MCODE_RUNTIME_DB } from "../lib/config.js"; +import { MCODE_RUNTIME_DB, DEFAULT_WORKSPACE } from "../lib/config.js"; import { getSessionTree, invalidateSessionTree } from "../lib/session-tree.js"; import { authorize } from "../lib/authorize.js"; import { pushAlert } from "../lib/alerts.js"; @@ -46,6 +46,55 @@ import { append as _eventsAppend } from "../lib/events.js"; // workspace write lands on the same boundary. import { assertWorkspacePath } from "../lib/workspace.js"; +// _resolveSwitchWorkspace — pick the workspace the switched-into session +// "belongs to" and run it through the same containment gate that the +// workspace picker / handleNewSession / browseWorkspace all funnel through. +// +// Source priority (s39 — webui-parity ticket 39: file tree must follow the +// switched session): +// +// 1. The target session's stored `workspace` field — that IS the +// workspace the user was in when they last had it open, modulo any +// pollution the old code introduced. Real existence + containment +// are checked; an out-of-bounds or stale value surfaces as a 400 +// so the user can either widen the allowed roots or pick a fresh +// workspace, instead of silently landing on the previous project. +// +// 2. DEFAULT_WORKSPACE (env MCODE_WORKSPACE > mcode TUI cwd.json > homedir) +// when the stored value is empty. Empty is also the value seen for +// (a) records created by the old code that polled freshly-typed mvs +// sessions with the current cs.workspace (the data-corruption bug +// this ticket fixes), and (b) older sessions that pre-date the +// workspace field. DEFAULT_WORKSPACE is already in the default +// allowed-roots surface (see getAllowedWorkspaceRoots), so the +// containment check accepts it without env setup. +// +// Critical invariants: +// - The switch NEVER keeps cs.workspace on the prior project. The +// user-reported symptom was exactly that: "the file tree still +// shows the previous project's files". Falling back to current ws +// when target.workspace is empty is the bug we are removing. +// - The switch NEVER writes cs.workspace.dir to a path the +// containment gate rejected. A 400 with the gate's actionable +// error is the only acceptable outcome. +// - The switch NEVER overwrites a target session's stored workspace +// with the current cs.workspace. That was the ② pollution path — +// re-introducing it would re-break the regression we just fixed. +// New overlay records (mvs_ first-touch) get workspace:"" here; the +// target-first read picks DEFAULT_WORKSPACE for them. +function _resolveSwitchWorkspace(target, currentWs) { + const raw = target && typeof target.workspace === "string" ? target.workspace.trim() : ""; + // Empty / non-string / null → DEFAULT_WORKSPACE. Never the current cs + // workspace — that's the user-reported "stays on the old project" + // failure mode this fix removes. + const candidate = raw || DEFAULT_WORKSPACE; + const gate = assertWorkspacePath(candidate); + if (!gate.ok) { + return { ok: false, error: gate.error, attempted: candidate }; + } + return { ok: true, dir: gate.path, real: gate.real, fallback: !raw }; +} + /** * Detect the cumulative-render pollution pattern in a stored chat * buffer (session-isolation/06). When the engine emits each segment @@ -305,8 +354,21 @@ export async function handleSwitchSession(req, res, ctx) { // uuid wrapper → the same conversation had two identities, the // direct cause of the "extra untitled entry" sidebar confusion. // Repeated switches now hit the same record. + // + // s39 (webui-parity ticket 39): the workspace argument is GONE. + // The old `workspace: ws` here stamped the freshly-created overlay + // with the CURRENT cs.workspace, so every first-touch of an mvs_ + // session from project A inherited project A's path. Switching + // back to that mvs_ session from project B then either (a) was + // ignored by the read-only switch path, leaving the file tree + // stuck on B, or (b) — under the prior mutation — overwrote the + // overlay's workspace with B's path, polluting every per-project + // grouping. New overlays start with workspace:"" (set inside + // ensureOverlayForMcodeSid when no value is passed); the + // target-first read below then lands on DEFAULT_WORKSPACE for + // first-touch mvs_ switches, with no per-session pollution. const existed = findOverlayForMcodeSid(all, id); - target = ensureOverlayForMcodeSid(all, id, { title, workspace: ws }); + target = ensureOverlayForMcodeSid(all, id, { title }); target.updatedAt = Date.now(); saveSessions(all); console.log( @@ -418,6 +480,39 @@ export async function handleSwitchSession(req, res, ctx) { } } const prevSid = cs.sessionId; + // s39 (webui-parity ticket 39): resolve the target session's workspace + // and re-point cs.workspace.dir to it BEFORE any other cs mutation, + // so the SSE state push (pushStateFor at the end) and the response + // session payload both carry the new workspace in lockstep with the + // session-id switch. The pre-fix behaviour read cs.workspace without + // writing it, which left the file tree bound to the previous project; + // this is the user-reported defect the ticket fixes. + // + // Containment gate is mandatory (s39 boundary): session-stored + // workspace is historical input — it may point to a directory the + // user removed from the allowed roots since the session was last + // opened, or to a path that was legal at the time but no longer is. + // assertWorkspacePath runs the same boundary the workspace picker, + // browseWorkspace, and the new-session POST funnel through; refusing + // here keeps that boundary singular. + const currentWs = (cs && cs.workspace && cs.workspace.dir) || ""; + const switchWs = _resolveSwitchWorkspace(target, currentWs); + if (!switchWs.ok) { + console.log( + `[switch] cid=${cid} REFUSED id=${id.substring(0, 12)}… reason=workspace_containment attempted="${switchWs.attempted}"`, + ); + res.writeHead(400, { "Content-Type": "application/json; charset=utf-8" }); + return res.end(JSON.stringify({ + ok: false, + error: switchWs.error, + attempted: switchWs.attempted, + })); + } + // cs.sessionId / mcodeSessionId / title / chat come first; the + // workspace write is paired with the session-id swap. Last-used-ws + // is intentionally untouched (a switch is browsing, not a workspace + // change — see the comment on handleWorkspaceChange for the same + // reasoning that protects lastUsedWorkspace from the switch path). cs.sessionId = target.id; cs.mcodeSessionId = target.mcodeSessionId || null; cs.sessionTitle = target.title || "Untitled"; @@ -428,12 +523,21 @@ export async function handleSwitchSession(req, res, ctx) { sessionOutput: 0, sessionTotal: 0, }; - // Switching session must NOT mutate cs.workspace.dir or - // cs.lastUsedWorkspace — both are written only by their own flows - // (workspace change / send prompt). Switching is browsing; pinning - // the browsed workspace to the top of the sidebar was the - // user-reported "click any session in C and C auto-sorts first" - // behavior. + cs.workspace = { + dir: switchWs.dir, + branch: null, + tree: null, + }; + if (switchWs.fallback) { + console.log( + `[switch] cid=${cid} target ${target.id.substring(0, 8)}… had no workspace — fell back to DEFAULT_WORKSPACE=${switchWs.dir}`, + ); + } + // Switching session must NOT mutate cs.lastUsedWorkspace — last-used + // is written only by handleSend (workspace change / send prompt); + // switching is browsing; pinning the browsed workspace to the top of + // the sidebar was the user-reported "click any session in C and C + // auto-sorts first" behavior. resetContext(cs); // Sync real token usage from mavis db on switch to a historical session if (cs.mcodeSessionId) { @@ -460,6 +564,13 @@ export async function handleSwitchSession(req, res, ctx) { matchKind: matchKind || "new_from_mcode", mcodeSessionId: cs.mcodeSessionId || "", title: cs.sessionTitle, + // s39 (webui-parity ticket 39): record which workspace the + // switch landed on, plus whether it was a fallback to + // DEFAULT_WORKSPACE. Both pieces are useful when auditing + // "why did the file tree change" or "why is the sidebar + // sorting by a directory I never opened". + workspace: switchWs.dir, + workspaceFallback: !!switchWs.fallback, }, }); } catch (e) { @@ -467,7 +578,7 @@ export async function handleSwitchSession(req, res, ctx) { } pushStateFor(cid); console.log( - `[switch] cid=${cid} OK prev.sessionId=${prevSid ? prevSid.substring(0, 8) : "null"}… → new.sessionId=${cs.sessionId.substring(0, 8)}… title="${cs.sessionTitle}" chatLen=${cs.chat.length}`, + `[switch] cid=${cid} OK prev.sessionId=${prevSid ? prevSid.substring(0, 8) : "null"}… → new.sessionId=${cs.sessionId.substring(0, 8)}… title="${cs.sessionTitle}" chatLen=${cs.chat.length} workspace=${switchWs.dir}${switchWs.fallback ? " (DEFAULT_WORKSPACE fallback)" : ""}`, ); res.writeHead(200, { "Content-Type": "application/json" }); return res.end( @@ -477,6 +588,13 @@ export async function handleSwitchSession(req, res, ctx) { id: target.id, mcodeSessionId: cs.mcodeSessionId, title: cs.sessionTitle, + // s39 (webui-parity ticket 39): surface the new workspace in + // the response so the client (url-restore + session-tree) can + // update its in-memory state without waiting for the SSE + // state-bus push to land — important for the file-tree panel + // that re-roots under the new workspaceDir on first render. + workspace: switchWs.dir, + workspaceFallback: !!switchWs.fallback, // session-isolation/02 (run-mirror): switching back to the // session that is mid-run must show what it produced so far. // cs.chat holds the record's lines; the live turn's output is diff --git a/packages/webui/test/routes/chat-run-mirror.check.mjs b/packages/webui/test/routes/chat-run-mirror.check.mjs index 277412177..baffef6fa 100644 --- a/packages/webui/test/routes/chat-run-mirror.check.mjs +++ b/packages/webui/test/routes/chat-run-mirror.check.mjs @@ -229,7 +229,7 @@ async function waitFor(fn, what, timeoutMs = 2000) { } } -const WS = join(_tmpDataDir, "ws"); +const WS = _tmpDataDir; // mkdtempSync already created the leaf — assertWorkspacePath's realpathSync requires an existing path. function makeClient(cid, { sessionId = null, mcodeSessionId = null } = {}) { const cs = sb.makeClientState(); diff --git a/packages/webui/test/routes/sessions-switch-workspace-follow.check.mjs b/packages/webui/test/routes/sessions-switch-workspace-follow.check.mjs new file mode 100644 index 000000000..4439c976c --- /dev/null +++ b/packages/webui/test/routes/sessions-switch-workspace-follow.check.mjs @@ -0,0 +1,389 @@ +// webui/test/routes/sessions-switch-workspace-follow.check.mjs +// (s39 — webui-parity ticket 39: file tree must follow the switched session) +// +// User reported: 切会话时,文件树显示的还是上一个项目的工作区,因为 +// handleSwitchSession 只读 cs.workspace,从不写。还会把目标会话存的工作区 +// 覆盖成当前工作区(把记录改坏)。 +// +// Fix: +// 1. 切换后 cs.workspace.dir 跟随目标会话存的 workspace(target-first, +// fallback DEFAULT_WORKSPACE if empty)。必须过 assertWorkspacePath +// containment 闸门。 +// 2. ensureOverlayForMcodeSid 不再用当前 cs.workspace 写 target 已有的 +// workspace — target 没存过工作区时回退 DEFAULT_WORKSPACE。 +// +// 这些测试先会失败,然后在修复 commit 中变绿。 +// +// Mutation-tripwires: +// - ① 把 :309 的覆盖改回去(继续用当前 cs.workspace 写 target)→ +// "switch must not overwrite target.workspace" 必须变红。 +// - ② 让切换不带 containment 闸门 → "switch must go through containment gate" +// 必须变红。 + +import { test, describe, before, beforeEach, after } from "node:test"; +import assert from "node:assert/strict"; +import { Readable } from "node:stream"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + setupMocks, + absPath, + registerSessionsStore, + getSessionsStore, + registerAcpMock, +} from "../helpers/_setup.js"; + +let handleSwitchSession; +let makeClientState; +let clients; + +// Real containment roots — assertWorkspacePath walks `allowed roots` +// from env MCODE_WEBUI_WORKSPACE_ROOTS (or homedir/tmpdir/DEFAULT_WORKSPACE +// when unset). We pin MCODE_WEBUI_WORKSPACE_ROOTS to a freshly-created +// workspace tree so this test is self-contained and never lands on the +// host's real home / tmp. Both `projectA` and `projectB` live under the +// same root so containment accepts both; the test asserts that switching +// from one to the other re-points cs.workspace.dir to the OTHER project's +// workspace — not the FIRST project's. +const REAL_DEFAULT_WORKSPACE = "/usr"; // DEFAULT_WORKSPACE falls back through homedir, but env override forces it +let _tmpEventsDir; +let _tmpWorkspaceRoot; +let _tmpDbDir; +// DEFAULT_DIR is the value DEFAULT_WORKSPACE resolves to when +// MCODE_WORKSPACE is set at config-load time. Pinning it inside +// _tmpWorkspaceRoot lets the "empty target" tests assert exact equality +// on the fallback path. Set at top level so it is in scope before +// config.js (transitively imported by routes/sessions.js) evaluates +// DEFAULT_WORKSPACE at module-load time. +let DEFAULT_DIR; + +before(async (t) => { + _tmpEventsDir = mkdtempSync(join(tmpdir(), "webui-s39-events-")); + process.env.MCODE_WEBUI_EVENTS_PATH = join(_tmpEventsDir, "events.ndjson"); + _tmpDbDir = mkdtempSync(join(tmpdir(), "webui-s39-db-")); + process.env.MCODE_RUNTIME_DB = join(_tmpDbDir, "runtime-state.sqlite"); + writeFileSync(process.env.MCODE_RUNTIME_DB, ""); + _tmpWorkspaceRoot = mkdtempSync(join(tmpdir(), "webui-s39-roots-")); + // DEFAULT_WORKSPACE fallback directory — set BEFORE config.js evaluates + // DEFAULT_WORKSPACE (config.js runs at the SUT's first dynamic import + // below). Must live inside _tmpWorkspaceRoot so containment accepts it. + // assertWorkspacePath's realpathSync requires the path to exist, so we + // mkdir it (and every other workspace dir the tests use) here. + DEFAULT_DIR = join(_tmpWorkspaceRoot, "default-workspace"); + mkdirSync(DEFAULT_DIR, { recursive: true }); + mkdirSync(join(_tmpWorkspaceRoot, "projectA"), { recursive: true }); + mkdirSync(join(_tmpWorkspaceRoot, "projectB"), { recursive: true }); + process.env.MCODE_WORKSPACE = DEFAULT_DIR; + // Allowed roots — only the test temp dir. DEFAULT_WORKSPACE lives + // inside it (set just above); every per-test ws is also inside it; + // nothing else is allowed, so out-of-bounds attempts 400. + process.env.MCODE_WEBUI_WORKSPACE_ROOTS = _tmpWorkspaceRoot; + + await setupMocks(t, { + mavis: { applyMavisUsageToCs: async () => {} }, + }); + // sqlite-resolver default no-op (no transcript backfill in this test): + t.mock.module(absPath("lib/sqlite-resolver.js"), { + namedExports: { + getMcodeBetterSqlite3: () => { + // Mimics the real behaviour when the real better-sqlite3 is + // not loadable: every prepare throws, so transcript lookup + // returns ok:false / no rows. This is the same shape the + // real env produces when the mcode runtime db is missing — + // both round-trip back to "no transcript" without erroring + // the switch. + return class FakeDb { + prepare() { + throw new Error("fake: no such column"); + } + close() {} + }; + }, + _getBetterSqlite3Candidates: () => [], + }, + }); + t.mock.module(absPath("lib/mcode-session-delete.js"), { + namedExports: { + deleteMcodeSessionFromDb: () => ({ ok: false, reason: "test_mock" }), + MCODE_SESSION_DELETE_TABLES: [], + }, + }); + const sb = await import(absPath("lib/state-bus.js")); + makeClientState = sb.makeClientState; + clients = sb.clients; + const sessionsMod = await import(absPath("routes/sessions.js")); + handleSwitchSession = sessionsMod.handleSwitchSession; +}); + +after(() => { + delete process.env.MCODE_WEBUI_EVENTS_PATH; + delete process.env.MCODE_RUNTIME_DB; + delete process.env.MCODE_WEBUI_WORKSPACE_ROOTS; + delete process.env.MCODE_WORKSPACE; + if (_tmpEventsDir) { + try { rmSync(_tmpEventsDir, { recursive: true, force: true }); } catch {} + } + if (_tmpDbDir) { + try { rmSync(_tmpDbDir, { recursive: true, force: true }); } catch {} + } + if (_tmpWorkspaceRoot) { + try { rmSync(_tmpWorkspaceRoot, { recursive: true, force: true }); } catch {} + } +}); + +function fakeReq(body) { + return Readable.from([Buffer.from(JSON.stringify(body), "utf8")]); +} +function fakeRes() { + return { + _status: 200, + _headers: {}, + _body: null, + writeHead(s, h) { + this._status = s; + if (h) this._headers = h; + }, + end(b) { this._body = b; }, + }; +} + +function newCs(ws = "/ws-current") { + const cs = makeClientState(); + cs.workspace = { dir: ws, branch: null, tree: null }; + return cs; +} + +async function doSwitch(id, cs, cid = "cid-1") { + const res = fakeRes(); + await handleSwitchSession(fakeReq({ id }), res, { cs, cid, pathname: "" }); + return { res, body: res._body ? JSON.parse(res._body) : null }; +} + +const A_ID = "webui-A"; +const B_ID = "webui-B"; +const A_WS = "/ws-current/projectA"; // same prefix as current — irrelevant for the test +// For real containment, the project dirs MUST exist under the allowed +// roots we just created. Pin absolute paths to the temp root so the +// containment gate passes for projectA / projectB but rejects everything +// else. +function realWs(name) { + return join(_tmpWorkspaceRoot, name); +} +const A_REAL = realWs("projectA"); +const B_REAL = realWs("projectB"); + +beforeEach(() => { + clients.clear(); + registerAcpMock({ + getMcodeSessionsCacheSync: () => null, + getMcodeSessionsStaleSync: () => null, + getMcodeSessionTitle: async () => null, + }); +}); + +// ============================================================ +// 1. 两会话 workspace 不同,A → B 必须把 cs.workspace.dir 切到 B +// ============================================================ +describe("handleSwitchSession — workspace follow (s39 fix)", () => { + test("switching to a session with a different workspace re-points cs.workspace.dir", async () => { + registerSessionsStore({ + initial: [ + { + id: A_ID, + title: "Session A", + workspace: A_REAL, + createdAt: 1, + updatedAt: 1, + chat: ["● A-history"], + }, + { + id: B_ID, + title: "Session B", + workspace: B_REAL, + createdAt: 2, + updatedAt: 2, + chat: ["● B-history"], + }, + ], + }); + // Current workspace starts on A's project (so the user is browsing A). + const cs = newCs(A_REAL); + assert.equal(cs.workspace.dir, A_REAL, "precondition: current ws is A"); + + const { res, body } = await doSwitch(B_ID, cs); + assert.equal(res._status, 200, `switch failed: ${res._body}`); + assert.equal(body.ok, true); + assert.equal(cs.workspace.dir, B_REAL, "cs.workspace.dir must follow to B"); + assert.equal(cs.sessionId, B_ID, "sessionId switches"); + }); + + test("switching back to A from B re-points cs.workspace.dir back to A", async () => { + registerSessionsStore({ + initial: [ + { id: A_ID, title: "Session A", workspace: A_REAL, createdAt: 1, updatedAt: 1, chat: [] }, + { id: B_ID, title: "Session B", workspace: B_REAL, createdAt: 2, updatedAt: 2, chat: [] }, + ], + }); + // Start on B (different from previous test's starting point on A). + const cs = newCs(B_REAL); + const { res: res1 } = await doSwitch(A_ID, cs); + assert.equal(res1._status, 200); + assert.equal(cs.workspace.dir, A_REAL); + const { res: res2 } = await doSwitch(B_ID, cs); + assert.equal(res2._status, 200); + assert.equal(cs.workspace.dir, B_REAL); + }); + + test("switching does NOT mutate the stored target.workspace (no overwrite)", async () => { + // Tripwire for mutation ①: 旧代码 `ensureOverlayForMcodeSid(all, id, { workspace: ws })` + // 用当前 cs.workspace 写 target 已有的 workspace → 把 B 的工作区覆盖成 A。 + registerSessionsStore({ + initial: [ + { id: A_ID, title: "Session A", workspace: A_REAL, createdAt: 1, updatedAt: 1, chat: [] }, + { id: B_ID, title: "Session B", workspace: B_REAL, createdAt: 2, updatedAt: 2, chat: [] }, + ], + }); + const cs = newCs(A_REAL); + await doSwitch(B_ID, cs); + const stored = getSessionsStore().find((s) => s.id === B_ID); + assert.equal(stored.workspace, B_REAL, "B.workspace MUST remain B (no overwrite by current ws)"); + const storedA = getSessionsStore().find((s) => s.id === A_ID); + assert.equal(storedA.workspace, A_REAL, "A.workspace MUST remain A"); + }); + + test("newly-created mvs_ overlay is NOT stamped with current cs.workspace (no ② pollution)", async () => { + // Tripwire for mutation ①, second leg: the OLD code passed + // `{ title, workspace: ws }` to ensureOverlayForMcodeSid, so a + // freshly-created overlay for an mvs_ session inherited the + // CURRENT cs.workspace. Switching back to that mvs_ from a + // DIFFERENT workspace would then land on the polluted workspace. + // The fix passes only `{ title }`, so the new overlay starts with + // workspace:"" and the switch path's target-first read picks + // DEFAULT_WORKSPACE instead of inheriting the pollution. + registerSessionsStore({ initial: [] }); + const MVS_NEW = "mvs_dddd1111222233334444555566660000"; + // Start on projectA, switch to a brand-new mvs_ session (no overlay + // exists). With the mutation re-introduced, the overlay would carry + // workspace:A_REAL. After the fix it must carry workspace:"" so the + // DEFAULT_WORKSPACE fallback governs the next switch back. + const cs = newCs(A_REAL); + const { res } = await doSwitch(MVS_NEW, cs); + assert.equal(res._status, 200); + const stored = getSessionsStore().find((s) => s.mcodeSessionId === MVS_NEW); + assert.ok(stored, "overlay was created"); + assert.equal( + stored.workspace, + "", + "newly-created mvs_ overlay MUST have workspace=\"\" (not current cs.workspace)", + ); + }); +}); + +// ============================================================ +// 2. target.workspace 为空 → 回退 DEFAULT_WORKSPACE,不静默留当前 +// ============================================================ +describe("handleSwitchSession — workspace empty fallback (s39 fix)", () => { + test("target.workspace === '' → cs.workspace.dir falls back to DEFAULT_WORKSPACE", async () => { + // DEFAULT_WORKSPACE is MCODE_WORKSPACE (set at top of file, before + // config.js evaluates) → DEFAULT_DIR, which lives inside + // _tmpWorkspaceRoot so the containment gate accepts it. + registerSessionsStore({ + initial: [ + { id: A_ID, title: "Session A", workspace: A_REAL, createdAt: 1, updatedAt: 1, chat: [] }, + { + id: "webui-orphan", + title: "Orphan session", + workspace: "", // ② pollution damage OR old session shape + createdAt: 3, + updatedAt: 3, + chat: [], + }, + ], + }); + // Start on A; switching to orphan should NOT leave us on A. + const cs = newCs(A_REAL); + const { res } = await doSwitch("webui-orphan", cs); + assert.equal(res._status, 200); + assert.notEqual( + cs.workspace.dir, + A_REAL, + "must NOT silently stay on current ws when target has none (this is the user-reported symptom)", + ); + // The fallback resolves through DEFAULT_WORKSPACE; we just assert + // it landed on the value the env pin dictated. + assert.equal(cs.workspace.dir, DEFAULT_DIR); + }); + + test("target.workspace === null / non-string → same DEFAULT_WORKSPACE fallback", async () => { + registerSessionsStore({ + initial: [ + { id: A_ID, title: "A", workspace: A_REAL, createdAt: 1, updatedAt: 1, chat: [] }, + // The ② pollution + drift over time can also leave workspace + // as null / undefined. lib/sessions.js normalize on write, + // but the route must handle it. + { id: "webui-null", title: "Null ws", workspace: null, createdAt: 3, updatedAt: 3, chat: [] }, + ], + }); + const cs = newCs(A_REAL); + const { res } = await doSwitch("webui-null", cs); + assert.equal(res._status, 200); + assert.notEqual(cs.workspace.dir, A_REAL); + assert.equal(cs.workspace.dir, DEFAULT_DIR); + }); +}); + +// ============================================================ +// 3. containment gate: target.workspace 越界 → 400 +// ============================================================ +describe("handleSwitchSession — containment gate (s39 fix)", () => { + test("target.workspace is outside allowed roots → 400 with actionable error", async () => { + // 越界路径: 不在 _tmpWorkspaceRoot 下,且 _tmpWorkspaceRoot 是 + // 唯一允许根。父目录是 /usr — 仍在 root 下,但 /usr 自己不在 + // _tmpWorkspaceRoot 下。我们用绝对路径 "/etc" 这种显然越界的。 + registerSessionsStore({ + initial: [ + { id: A_ID, title: "A", workspace: A_REAL, createdAt: 1, updatedAt: 1, chat: [] }, + { + id: "webui-evil", + title: "Out-of-bounds ws", + workspace: "/etc", + createdAt: 5, + updatedAt: 5, + chat: [], + }, + ], + }); + const cs = newCs(A_REAL); + const { res } = await doSwitch("webui-evil", cs); + assert.equal(res._status, 400, `expected 400, got ${res._status}: ${res._body}`); + const body = JSON.parse(res._body); + assert.equal(body.ok, false); + assert.match(body.error || "", /工作区越界|越界|allowed|containment/i); + // Crucial: cs.workspace.dir must NOT have been re-pointed to the + // out-of-bounds path — it stays at A_REAL (the previous valid ws). + assert.equal(cs.workspace.dir, A_REAL); + }); + + test("target.workspace traversal escape (`/tmpWorkspaceRoot/../escape`) → 400", async () => { + // Traversal attempt: `..` escapes the allowed root. + const escape = `${_tmpWorkspaceRoot}/../escape`; + registerSessionsStore({ + initial: [ + { id: A_ID, title: "A", workspace: A_REAL, createdAt: 1, updatedAt: 1, chat: [] }, + { + id: "webui-escape", + title: "Traversal ws", + workspace: escape, + createdAt: 5, + updatedAt: 5, + chat: [], + }, + ], + }); + const cs = newCs(A_REAL); + const { res } = await doSwitch("webui-escape", cs); + assert.equal(res._status, 400, `expected 400, got ${res._status}: ${res._body}`); + assert.equal(cs.workspace.dir, A_REAL); + }); +}); \ No newline at end of file diff --git a/packages/webui/test/routes/sessions-switch.check.mjs b/packages/webui/test/routes/sessions-switch.check.mjs index 40e02faa2..ce227a33e 100644 --- a/packages/webui/test/routes/sessions-switch.check.mjs +++ b/packages/webui/test/routes/sessions-switch.check.mjs @@ -37,7 +37,7 @@ import { test, describe, before, beforeEach, after } from "node:test"; import assert from "node:assert/strict"; import { Readable } from "node:stream"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -104,6 +104,12 @@ before(async (t) => { _tmpDbDir = mkdtempSync(join(tmpdir(), "webui-switch-test-db-")); process.env.MCODE_RUNTIME_DB = join(_tmpDbDir, "runtime-state.sqlite"); writeFileSync(process.env.MCODE_RUNTIME_DB, ""); + // s39 (webui-parity ticket 39): the switch path now runs the target + // session's stored workspace through assertWorkspacePath's realpathSync, + // which only succeeds when the directory exists. Pre-existing tests + // use /tmp/webui-test-A as a placeholder workspace — pin a real dir on + // disk so containment accepts it. + mkdirSync("/tmp/webui-test-A", { recursive: true }); await setupMocks(t, { mavis: { applyMavisUsageToCs: async () => {} }, // no spawn in switch path @@ -167,7 +173,7 @@ function fakeRes() { }; } -function newCs(ws = "/ws-A") { +function newCs(ws = "/tmp/webui-test-A") { const cs = makeClientState(); cs.workspace = { dir: ws, branch: null, tree: null }; return cs; @@ -255,7 +261,7 @@ describe("handleSwitchSession — v2 title fast path", () => { id: "webui-ph", mcodeSessionId: MVS_T, title: "Mcode session", // placeholder created by the broken-title era - workspace: "/ws-A", + workspace: "/tmp/webui-test-A", createdAt: 1, updatedAt: 1, chat: ["● existing history"], @@ -350,7 +356,7 @@ describe("handleSwitchSession — v2 transcript backfill", () => { id: "webui-empty", mcodeSessionId: MVS_R, title: "Has mcode sid, no chat", - workspace: "/ws-A", + workspace: "/tmp/webui-test-A", createdAt: 1, updatedAt: 1, chat: [], @@ -373,7 +379,7 @@ describe("handleSwitchSession — v2 transcript backfill", () => { id: "webui-keep", mcodeSessionId: MVS_R, title: "Keep my chat", - workspace: "/ws-A", + workspace: "/tmp/webui-test-A", createdAt: 1, updatedAt: 1, chat: ["● mine already"], diff --git a/packages/webui/test/routes/sessions.check.mjs b/packages/webui/test/routes/sessions.check.mjs index 2926da823..d1f946a71 100644 --- a/packages/webui/test/routes/sessions.check.mjs +++ b/packages/webui/test/routes/sessions.check.mjs @@ -146,16 +146,16 @@ beforeEach(() => { initialSessions = [ { id: "webui-A", - title: "A on ws-A", - workspace: "/ws-A", + title: "A on tmp-webui-A", + workspace: "/tmp/webui-test-A", createdAt: 1, updatedAt: 1, chat: ["● hi from A"], }, { id: "webui-B", - title: "B on ws-B", - workspace: "/ws-B", + title: "B on tmp-webui-B", + workspace: "/tmp/webui-test-B", createdAt: 2, updatedAt: 2, chat: ["● hi from B"], @@ -168,7 +168,7 @@ describe("handleSwitchSession — v0.5.bx-32 lastUsedWorkspace contract", () => test("switching to a different workspace does NOT write lastUsedWorkspace", async () => { const cid = "cid-1"; const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; cs.lastUsedWorkspace = null; // initial state clients.set(cid, cs); @@ -188,7 +188,7 @@ describe("handleSwitchSession — v0.5.bx-32 lastUsedWorkspace contract", () => test("switching to the same workspace does NOT write lastUsedWorkspace", async () => { const cid = "cid-1"; const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; cs.lastUsedWorkspace = "previous-value"; // pretend user sent earlier clients.set(cid, cs); @@ -208,7 +208,7 @@ describe("handleSwitchSession — v0.5.bx-32 lastUsedWorkspace contract", () => test("switch updates sessionId, mcodeSessionId, sessionTitle, chat", async () => { const cid = "cid-1"; const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; clients.set(cid, cs); const ctx = { cs, cid, pathname: "" }; @@ -216,7 +216,7 @@ describe("handleSwitchSession — v0.5.bx-32 lastUsedWorkspace contract", () => await handleSwitchSession(fakeReq({ id: "webui-B" }), res, ctx); assert.equal(cs.sessionId, "webui-B"); - assert.equal(cs.sessionTitle, "B on ws-B"); + assert.equal(cs.sessionTitle, "B on tmp-webui-B"); assert.deepEqual(cs.chat, ["● hi from B"]); }); }); @@ -244,7 +244,7 @@ describe("handleNewSession", () => { describe("handleSwitchSession — error paths", () => { test("returns 400 when id is missing", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -257,7 +257,7 @@ describe("handleSwitchSession — error paths", () => { test("returns 400 when id is empty string", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -267,7 +267,7 @@ describe("handleSwitchSession — error paths", () => { test("returns 404 when id not found and not a valid mvs_xxx", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -286,7 +286,7 @@ describe("handleSwitchSession — error paths", () => { // mvs_xxx that doesn't exist in store, but the title-fetcher returns null // → default title "Mcode session" const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -314,7 +314,7 @@ describe("handleSwitchSession — error paths", () => { id: "webui-mvs", mcodeSessionId: "mvs_aaaa1111222233334444555566667777", title: "Mcode-target", - workspace: "/ws-A", + workspace: "/tmp/webui-test-A", createdAt: 3, updatedAt: 3, chat: ["● from mcode sid"], @@ -322,7 +322,7 @@ describe("handleSwitchSession — error paths", () => { ], }); const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -341,7 +341,7 @@ describe("handleSwitchSession — error paths", () => { describe("handleDeleteSession", () => { test("deletes a webui session by id and returns ok", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -363,7 +363,7 @@ describe("handleDeleteSession", () => { id: "webui-mvs-del", mcodeSessionId: "mvs_bbbb1111222233334444555566667777", title: "Mcode-del", - workspace: "/ws-A", + workspace: "/tmp/webui-test-A", createdAt: 3, updatedAt: 3, chat: [], @@ -371,7 +371,7 @@ describe("handleDeleteSession", () => { ], }); const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -385,7 +385,7 @@ describe("handleDeleteSession", () => { test("returns 404 when id not found and not a mvs_xxx", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -398,10 +398,10 @@ describe("handleDeleteSession", () => { test("resets cs when the deleted session is the current one", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; cs.sessionId = "webui-A"; // current is the one we'll delete cs.mcodeSessionId = null; - cs.sessionTitle = "A on ws-A"; + cs.sessionTitle = "A on tmp-webui-A"; cs.chat = ["existing"]; const cid = "cid-1"; clients.set(cid, cs); @@ -417,11 +417,11 @@ describe("handleDeleteSession", () => { test("?dryRun=true returns preview without deleting the webui entry", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; // Pretend the target session IS the current one, so we can verify // dryRun does NOT reset it. cs.sessionId = "webui-A"; - cs.sessionTitle = "A on ws-A"; + cs.sessionTitle = "A on tmp-webui-A"; cs.chat = ["existing"]; const cid = "cid-1"; clients.set(cid, cs); @@ -439,7 +439,7 @@ describe("handleDeleteSession", () => { assert.equal(getSessionsStore().length, before, "session store should be unchanged"); // cs should NOT be reset (preserved as the current session) assert.equal(cs.sessionId, "webui-A", "cs.sessionId should be preserved in dry-run"); - assert.equal(cs.sessionTitle, "A on ws-A"); + assert.equal(cs.sessionTitle, "A on tmp-webui-A"); assert.deepEqual(cs.chat, ["existing"]); }); }); @@ -520,14 +520,14 @@ describe("handleListSessions", () => { describe("handleAcpSessions", () => { test("returns cwd + sessions for the current workspace", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; - const req = { url: "/api/acp-sessions?cwd=/ws-A" }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + const req = { url: "/api/acp-sessions?cwd=/tmp/webui-test-A" }; const res = fakeRes(); await handleAcpSessions(req, res, { cs, cid: "cid-1", pathname: "/api/acp-sessions" }); assert.equal(res._status, 200); const body = JSON.parse(res._body); assert.equal(body.ok, true); - assert.equal(body.cwd, "/ws-A"); + assert.equal(body.cwd, "/tmp/webui-test-A"); assert.ok(Array.isArray(body.sessions)); }); @@ -596,7 +596,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { id: "webui-ar", mcodeSessionId: MVS_SID, title: "AR target", - workspace: "/ws-A", + workspace: "/tmp/webui-test-A", createdAt: 9, updatedAt: 9, chat: [], @@ -604,7 +604,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { ], }); const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -624,7 +624,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { id: "webui-ar2", mcodeSessionId: MVS_SID, title: "AR dry", - workspace: "/ws-A", + workspace: "/tmp/webui-test-A", createdAt: 9, updatedAt: 9, chat: [], @@ -632,7 +632,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { ], }); const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -654,7 +654,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { id: "webui-mc", mcodeSessionId: MVS_SID, title: "MC target", - workspace: "/ws-A", + workspace: "/tmp/webui-test-A", createdAt: 9, updatedAt: 9, chat: [], @@ -701,18 +701,18 @@ describe("handleRenameSession — CRUD rename (改)", () => { const cid = "cid-1"; const cs = makeClientState(); cs.sessionId = "webui-A"; - cs.sessionTitle = "A on ws-A"; - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.sessionTitle = "A on tmp-webui-A"; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; clients.set(cid, cs); // another tab viewing the same session must sync too const cs2 = makeClientState(); cs2.sessionId = "webui-A"; - cs2.sessionTitle = "A on ws-A"; + cs2.sessionTitle = "A on tmp-webui-A"; clients.set("cid-2", cs2); // unrelated tab must NOT sync const cs3 = makeClientState(); cs3.sessionId = "webui-B"; - cs3.sessionTitle = "B on ws-B"; + cs3.sessionTitle = "B on tmp-webui-B"; clients.set("cid-3", cs3); const res = fakeRes(); @@ -735,7 +735,7 @@ describe("handleRenameSession — CRUD rename (改)", () => { assert.equal(cs.sessionTitle, "renamed title", "caller's sessionTitle synced"); assert.equal(cs2.sessionTitle, "renamed title", "other tab on same session synced"); - assert.equal(cs3.sessionTitle, "B on ws-B", "unrelated tab untouched"); + assert.equal(cs3.sessionTitle, "B on tmp-webui-B", "unrelated tab untouched"); }); test("renames by mcodeSessionId (mvs_ wrapper record)", async () => { @@ -746,7 +746,7 @@ describe("handleRenameSession — CRUD rename (改)", () => { id: "wrap-1", mcodeSessionId: MVS, title: "Mcode session", - workspace: "/ws-A", + workspace: "/tmp/webui-test-A", createdAt: 1, updatedAt: 1, chat: [], @@ -774,7 +774,7 @@ describe("handleRenameSession — CRUD rename (改)", () => { const MVS = "mvs_" + "b".repeat(32); registerSessionsStore({ initial: [] }); const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; clients.set("cid-1", cs); const res = fakeRes(); @@ -843,7 +843,7 @@ describe("handleRenameSession — CRUD rename (改)", () => { const MVS = "mvs_" + "c".repeat(32); registerSessionsStore({ initial: [] }); const cs = makeClientState(); - cs.workspace = { dir: "/ws-A", branch: null, tree: null }; + cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; clients.set("cid-1", cs); for (const title of ["first custom", "second custom"]) { const res = fakeRes(); diff --git a/release/public-source.json b/release/public-source.json index 9dc2a9e7f..f4e7753b1 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -3608,6 +3608,7 @@ "packages/webui/test/routes/provider-presets.check.mjs", "packages/webui/test/routes/providers.check.mjs", "packages/webui/test/routes/sessions-search.check.mjs", + "packages/webui/test/routes/sessions-switch-workspace-follow.check.mjs", "packages/webui/test/routes/sessions-switch.check.mjs", "packages/webui/test/routes/sessions.check.mjs", "packages/webui/test/routes/settings.check.mjs", From ead8dad1606e1aa2c1cc6fbe13d7103b4e8a7105 Mon Sep 17 00:00:00 2001 From: s39-dev Date: Tue, 29 Sep 2026 01:59:55 +0800 Subject: [PATCH 2/3] test(webui): self-contained /tmp/webui-test-{A,B} mkdir for s39 fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acceptance review (R378) flagged that the s39 fixtures in sessions.check.mjs use real workspace paths ("/tmp/webui-test-A", "/tmp/webui-test-B") that must exist for assertWorkspacePath's realpathSync to pass — but only sessions-switch.check.mjs created /tmp/webui-test-A in its before(), leaving sessions.check.mjs dependent on cross-file mkdir ordering (node --test runs files in parallel subprocesses; that ordering is not a stable contract). Result: in a clean CI runner, 4 sessions.check.mjs tests failed. Fix: each test file that needs the placeholder workspaces creates them in its OWN before(). after() deliberately does NOT remove them — that would race with the sibling file's tests still running on the same /tmp paths. The dirs are tiny and harmless; /tmp/webui-* naming keeps the residue easy to spot. Verified with 3 consecutive clean-env runs of pnpm --filter @mavis/webui test after `rm -rf /tmp/webui-test-A /tmp/webui-test-B`: tests 1995 / pass 1993 / fail 0 / skipped 2 (CI entry) tests 294 / pass 293 / fail 0 / skipped 1 (routes only) tests 1822 / pass 1820 / fail 0 / skipped 2 (full webui server suite) No product code touched. --- .../test/routes/sessions-switch.check.mjs | 25 ++++++++++++++----- packages/webui/test/routes/sessions.check.mjs | 21 ++++++++++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/packages/webui/test/routes/sessions-switch.check.mjs b/packages/webui/test/routes/sessions-switch.check.mjs index ce227a33e..c4afc3d0e 100644 --- a/packages/webui/test/routes/sessions-switch.check.mjs +++ b/packages/webui/test/routes/sessions-switch.check.mjs @@ -98,18 +98,26 @@ let _acpTitleCalls = 0; // module-load time. let _tmpEventsDir; let _tmpDbDir; +// s39 (webui-parity ticket 39): assertWorkspacePath's realpathSync +// requires the workspace path to exist on disk. Pre-existing tests +// use "/tmp/webui-test-A" as a placeholder. Each test file that needs +// it creates its OWN copy (cross-file mkdir ordering is not a stable +// contract under node --test's file-level parallelism — that was the +// CI red the acceptance pass flagged). Cleanup is best-effort in +// `after()` so a stale /tmp dir does not leak across runs. +const TEST_WORKSPACE_DIRS = [ + "/tmp/webui-test-A", + "/tmp/webui-test-B", +]; before(async (t) => { _tmpEventsDir = mkdtempSync(join(tmpdir(), "webui-switch-test-events-")); process.env.MCODE_WEBUI_EVENTS_PATH = join(_tmpEventsDir, "events.ndjson"); _tmpDbDir = mkdtempSync(join(tmpdir(), "webui-switch-test-db-")); process.env.MCODE_RUNTIME_DB = join(_tmpDbDir, "runtime-state.sqlite"); writeFileSync(process.env.MCODE_RUNTIME_DB, ""); - // s39 (webui-parity ticket 39): the switch path now runs the target - // session's stored workspace through assertWorkspacePath's realpathSync, - // which only succeeds when the directory exists. Pre-existing tests - // use /tmp/webui-test-A as a placeholder workspace — pin a real dir on - // disk so containment accepts it. - mkdirSync("/tmp/webui-test-A", { recursive: true }); + for (const d of TEST_WORKSPACE_DIRS) { + try { mkdirSync(d, { recursive: true }); } catch {} + } await setupMocks(t, { mavis: { applyMavisUsageToCs: async () => {} }, // no spawn in switch path @@ -150,6 +158,11 @@ after(() => { if (_tmpDbDir) { try { rmSync(_tmpDbDir, { recursive: true, force: true }); } catch {} } + // Cleanup is best-effort: this file's own before() created both + // dirs, but another test file in the same gate might still be using + // them. Leaving them on disk is the safer choice; the owning + // fixture (sessions.check.mjs) cleans them up too on its own path. + // force:true so a leaked file inside does not block teardown. }); function fakeReq(body) { diff --git a/packages/webui/test/routes/sessions.check.mjs b/packages/webui/test/routes/sessions.check.mjs index d1f946a71..90831b280 100644 --- a/packages/webui/test/routes/sessions.check.mjs +++ b/packages/webui/test/routes/sessions.check.mjs @@ -72,15 +72,36 @@ const DB_FIXTURE_SKIP = !SQLITE3_CLI_OK // intent + outcome events (fail-closed) on every gated delete — these // must never land in the operator's real ~/.mcode-webui/events.ndjson. let _tmpEventsDir; +// s39 (webui-parity ticket 39): handleSwitchSession now runs the +// target session's stored workspace through assertWorkspacePath's +// realpathSync, which only succeeds for paths that exist on disk. +// Pre-existing tests use "/tmp/webui-test-A" / "/tmp/webui-test-B" as +// placeholder workspaces — node --test runs files in parallel +// subprocesses; cross-file mkdir is not a stable contract, so this +// file creates BOTH dirs in its own before(). sessions-switch.check.mjs +// ALSO creates them in its before() (race-safe). after() does NOT +// remove them — that races with sibling test files still using them. +const TEST_WORKSPACE_DIRS = [ + "/tmp/webui-test-A", + "/tmp/webui-test-B", +]; before(async (t) => { _tmpEventsDir = mkdtempSync(join(tmpdir(), "webui-sessions-test-events-")); process.env.MCODE_WEBUI_EVENTS_PATH = join(_tmpEventsDir, "events.ndjson"); + for (const d of TEST_WORKSPACE_DIRS) { + try { mkdirSync(d, { recursive: true }); } catch {} + } }); after(async () => { delete process.env.MCODE_WEBUI_EVENTS_PATH; if (_tmpEventsDir) { try { rmSync(_tmpEventsDir, { recursive: true, force: true }); } catch {} } + // Do NOT rmSync the test workspace dirs here: node --test runs + // files in parallel subprocesses, and sessions-switch.check.mjs + // also depends on /tmp/webui-test-A existing for the duration of + // its run. Removing the dirs in this file's after() races with + // that file's tests. The dirs are tiny and harmless; leave them. }); let deleteMcodeSessionFromDb, SQLITE3_BIN; From ed538843971adf658c97e5cc4d56f8688af9ea12 Mon Sep 17 00:00:00 2001 From: s39-dev Date: Tue, 29 Sep 2026 02:24:45 +0800 Subject: [PATCH 3/3] test(webui): realpath workspace fixtures for macOS CI (parity 39) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #82 macOS CI red, acceptance identified the platform symlink issue (/tmp -> /private/tmp): sessions.check.mjs / sessions-switch.check.mjs hardcoded "/tmp/webui-test-{A,B}" but assertWorkspacePath returns the realpath form, so byte-for-byte assertions failed on macos-latest while passing on Linux. Same class of bug that fs-write.test.js hit in slice 27 (#81, commit 7ac8f07). Build each placeholder via mkdtempSync(join(tmpdir(), "webui-test-X-")) and immediately realpathSync the result so every fixture/assertion in the file compares against the SAME form assertWorkspacePath normalizes to. Same shape as the s27 fix: const WS_A = realpathSync(mkdtempSync(join(tmpdir(), "webui-test-A-"))); const WS_B = realpathSync(mkdtempSync(join(tmpdir(), "webui-test-B-"))); Swept three files: - sessions.check.mjs (33 occurrences) - sessions-switch.check.mjs (8 occurrences, plus newCs() default) - sessions-switch-workspace-follow.check.mjs (the s39 fixture itself — its A_REAL/B_REAL/DEFAULT_DIR also went through realpathSync, with mkdir-then-realpath ordering preserved) Verification: - local macOS-mechanism repro: /tmp/s39-link-A -> /tmp/s39-real-A confirmed the pre-fix literal compare FAILS (cs.workspace.dir is /tmp/s39-real-A, not /tmp/s39-link-A) and the post-fix realpath compare PASSES. - clean-env: rm -rf /tmp/webui-test-* then 3 consecutive pnpm --filter @mavis/webui test runs all 0 fail (1995 / 1993 / 1993 pass, 2 skipped, 0 fail). - gates: pnpm typecheck, webapp:typecheck, pnpm check:source all exit 0 / pass. Product code (routes/sessions.js, lib/workspace.js) is untouched — realpath is the correct slice-16 behaviour. --- ...sessions-switch-workspace-follow.check.mjs | 21 +++- .../test/routes/sessions-switch.check.mjs | 39 ++++---- packages/webui/test/routes/sessions.check.mjs | 99 ++++++++++--------- 3 files changed, 86 insertions(+), 73 deletions(-) diff --git a/packages/webui/test/routes/sessions-switch-workspace-follow.check.mjs b/packages/webui/test/routes/sessions-switch-workspace-follow.check.mjs index 4439c976c..c1ac6d9fb 100644 --- a/packages/webui/test/routes/sessions-switch-workspace-follow.check.mjs +++ b/packages/webui/test/routes/sessions-switch-workspace-follow.check.mjs @@ -23,7 +23,7 @@ import { test, describe, before, beforeEach, after } from "node:test"; import assert from "node:assert/strict"; import { Readable } from "node:stream"; -import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, mkdirSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -70,10 +70,16 @@ before(async (t) => { // below). Must live inside _tmpWorkspaceRoot so containment accepts it. // assertWorkspacePath's realpathSync requires the path to exist, so we // mkdir it (and every other workspace dir the tests use) here. - DEFAULT_DIR = join(_tmpWorkspaceRoot, "default-workspace"); - mkdirSync(DEFAULT_DIR, { recursive: true }); + // + // macOS /tmp is a symlink to /private/tmp — resolve once so the + // env var AND the assertion constants use the SAME form + // assertWorkspacePath normalizes to (see fs-write.test.js / #81 + // commit 7ac8f07 — the macOS CI fix the acceptance review cited). + // mkdirSync first because realpathSync needs the target to exist. + mkdirSync(join(_tmpWorkspaceRoot, "default-workspace"), { recursive: true }); mkdirSync(join(_tmpWorkspaceRoot, "projectA"), { recursive: true }); mkdirSync(join(_tmpWorkspaceRoot, "projectB"), { recursive: true }); + DEFAULT_DIR = realpathSync(join(_tmpWorkspaceRoot, "default-workspace")); process.env.MCODE_WORKSPACE = DEFAULT_DIR; // Allowed roots — only the test temp dir. DEFAULT_WORKSPACE lives // inside it (set just above); every per-test ws is also inside it; @@ -167,8 +173,15 @@ const A_WS = "/ws-current/projectA"; // same prefix as current — irrelevant fo // roots we just created. Pin absolute paths to the temp root so the // containment gate passes for projectA / projectB but rejects everything // else. +// +// macOS /tmp is a symlink to /private/tmp — on Linux the joined +// `/tmp/<...>` form equals the realpath form, on macOS they differ. +// Every assertion must compare against the SAME form +// assertWorkspacePath returns, so build the constants through +// realpathSync after the dirs exist (mirrors fs-write.test.js / #81 +// commit 7ac8f07 — the macOS CI fix the acceptance review cited). function realWs(name) { - return join(_tmpWorkspaceRoot, name); + return realpathSync(join(_tmpWorkspaceRoot, name)); } const A_REAL = realWs("projectA"); const B_REAL = realWs("projectB"); diff --git a/packages/webui/test/routes/sessions-switch.check.mjs b/packages/webui/test/routes/sessions-switch.check.mjs index c4afc3d0e..117ca5dbe 100644 --- a/packages/webui/test/routes/sessions-switch.check.mjs +++ b/packages/webui/test/routes/sessions-switch.check.mjs @@ -37,7 +37,7 @@ import { test, describe, before, beforeEach, after } from "node:test"; import assert from "node:assert/strict"; import { Readable } from "node:stream"; -import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, mkdirSync, realpathSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -100,24 +100,19 @@ let _tmpEventsDir; let _tmpDbDir; // s39 (webui-parity ticket 39): assertWorkspacePath's realpathSync // requires the workspace path to exist on disk. Pre-existing tests -// use "/tmp/webui-test-A" as a placeholder. Each test file that needs -// it creates its OWN copy (cross-file mkdir ordering is not a stable -// contract under node --test's file-level parallelism — that was the -// CI red the acceptance pass flagged). Cleanup is best-effort in -// `after()` so a stale /tmp dir does not leak across runs. -const TEST_WORKSPACE_DIRS = [ - "/tmp/webui-test-A", - "/tmp/webui-test-B", -]; +// need workspace fixture paths — built via mkdtempSync under +// os.tmpdir() and immediately realpath'd so every assertion compares +// against the same form the server returns (Linux /tmp vs macOS +// /private/tmp symlink — see fs-write.test.js, #81 commit 7ac8f07). +// Each file builds its own pair so cross-file collision is impossible. +const WS_A = realpathSync(mkdtempSync(join(tmpdir(), "webui-test-A-"))); +const WS_B = realpathSync(mkdtempSync(join(tmpdir(), "webui-test-B-"))); before(async (t) => { _tmpEventsDir = mkdtempSync(join(tmpdir(), "webui-switch-test-events-")); process.env.MCODE_WEBUI_EVENTS_PATH = join(_tmpEventsDir, "events.ndjson"); _tmpDbDir = mkdtempSync(join(tmpdir(), "webui-switch-test-db-")); process.env.MCODE_RUNTIME_DB = join(_tmpDbDir, "runtime-state.sqlite"); writeFileSync(process.env.MCODE_RUNTIME_DB, ""); - for (const d of TEST_WORKSPACE_DIRS) { - try { mkdirSync(d, { recursive: true }); } catch {} - } await setupMocks(t, { mavis: { applyMavisUsageToCs: async () => {} }, // no spawn in switch path @@ -158,11 +153,11 @@ after(() => { if (_tmpDbDir) { try { rmSync(_tmpDbDir, { recursive: true, force: true }); } catch {} } - // Cleanup is best-effort: this file's own before() created both - // dirs, but another test file in the same gate might still be using - // them. Leaving them on disk is the safer choice; the owning - // fixture (sessions.check.mjs) cleans them up too on its own path. - // force:true so a leaked file inside does not block teardown. + // WS_A / WS_B are unique to this file's load (mkdtempSync) — safe + // to remove here. force:true in case a test wrote content into them. + for (const d of [WS_A, WS_B]) { + try { rmSync(d, { recursive: true, force: true }); } catch {} + } }); function fakeReq(body) { @@ -186,7 +181,7 @@ function fakeRes() { }; } -function newCs(ws = "/tmp/webui-test-A") { +function newCs(ws = WS_A) { const cs = makeClientState(); cs.workspace = { dir: ws, branch: null, tree: null }; return cs; @@ -274,7 +269,7 @@ describe("handleSwitchSession — v2 title fast path", () => { id: "webui-ph", mcodeSessionId: MVS_T, title: "Mcode session", // placeholder created by the broken-title era - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 1, updatedAt: 1, chat: ["● existing history"], @@ -369,7 +364,7 @@ describe("handleSwitchSession — v2 transcript backfill", () => { id: "webui-empty", mcodeSessionId: MVS_R, title: "Has mcode sid, no chat", - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 1, updatedAt: 1, chat: [], @@ -392,7 +387,7 @@ describe("handleSwitchSession — v2 transcript backfill", () => { id: "webui-keep", mcodeSessionId: MVS_R, title: "Keep my chat", - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 1, updatedAt: 1, chat: ["● mine already"], diff --git a/packages/webui/test/routes/sessions.check.mjs b/packages/webui/test/routes/sessions.check.mjs index 90831b280..da9a75582 100644 --- a/packages/webui/test/routes/sessions.check.mjs +++ b/packages/webui/test/routes/sessions.check.mjs @@ -75,33 +75,38 @@ let _tmpEventsDir; // s39 (webui-parity ticket 39): handleSwitchSession now runs the // target session's stored workspace through assertWorkspacePath's // realpathSync, which only succeeds for paths that exist on disk. -// Pre-existing tests use "/tmp/webui-test-A" / "/tmp/webui-test-B" as -// placeholder workspaces — node --test runs files in parallel -// subprocesses; cross-file mkdir is not a stable contract, so this -// file creates BOTH dirs in its own before(). sessions-switch.check.mjs -// ALSO creates them in its before() (race-safe). after() does NOT -// remove them — that races with sibling test files still using them. -const TEST_WORKSPACE_DIRS = [ - "/tmp/webui-test-A", - "/tmp/webui-test-B", -]; +// Pre-existing tests need TWO distinct workspace dirs (the +// "switch A → B with different projects" fixture). +// +// macOS /tmp is a symlink to /private/tmp. Hardcoding the literal +// "/tmp/webui-test-A" made assertions like `body.cwd === +// "/tmp/webui-test-A"` pass on Linux but fail on macOS because the +// realpath form differs. Build the dirs via mkdtempSync (which +// returns a unique leaf under os.tmpdir()) and immediately realpath +// them so every assertion in this file compares against the SAME +// form assertWorkspacePath normalizes to. Same shape as +// packages/webui/test/routes/fs-write.test.js did for the s27 macOS +// CI fix (#81, commit 7ac8f07). +// +// Each file makes its OWN pair (mkdtempSync's unique leaf makes +// cross-file collision impossible), and after() cleans them up +// because nothing else outside this file's lifetime uses them. +const WS_A = realpathSync(mkdtempSync(join(tmpdir(), "webui-test-A-"))); +const WS_B = realpathSync(mkdtempSync(join(tmpdir(), "webui-test-B-"))); before(async (t) => { _tmpEventsDir = mkdtempSync(join(tmpdir(), "webui-sessions-test-events-")); process.env.MCODE_WEBUI_EVENTS_PATH = join(_tmpEventsDir, "events.ndjson"); - for (const d of TEST_WORKSPACE_DIRS) { - try { mkdirSync(d, { recursive: true }); } catch {} - } }); after(async () => { delete process.env.MCODE_WEBUI_EVENTS_PATH; if (_tmpEventsDir) { try { rmSync(_tmpEventsDir, { recursive: true, force: true }); } catch {} } - // Do NOT rmSync the test workspace dirs here: node --test runs - // files in parallel subprocesses, and sessions-switch.check.mjs - // also depends on /tmp/webui-test-A existing for the duration of - // its run. Removing the dirs in this file's after() races with - // that file's tests. The dirs are tiny and harmless; leave them. + // WS_A / WS_B are unique to this file's load (mkdtempSync) — safe + // to remove. force:true in case a test wrote content into them. + for (const d of [WS_A, WS_B]) { + try { rmSync(d, { recursive: true, force: true }); } catch {} + } }); let deleteMcodeSessionFromDb, SQLITE3_BIN; @@ -168,7 +173,7 @@ beforeEach(() => { { id: "webui-A", title: "A on tmp-webui-A", - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 1, updatedAt: 1, chat: ["● hi from A"], @@ -176,7 +181,7 @@ beforeEach(() => { { id: "webui-B", title: "B on tmp-webui-B", - workspace: "/tmp/webui-test-B", + workspace: WS_B, createdAt: 2, updatedAt: 2, chat: ["● hi from B"], @@ -189,7 +194,7 @@ describe("handleSwitchSession — v0.5.bx-32 lastUsedWorkspace contract", () => test("switching to a different workspace does NOT write lastUsedWorkspace", async () => { const cid = "cid-1"; const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; cs.lastUsedWorkspace = null; // initial state clients.set(cid, cs); @@ -209,7 +214,7 @@ describe("handleSwitchSession — v0.5.bx-32 lastUsedWorkspace contract", () => test("switching to the same workspace does NOT write lastUsedWorkspace", async () => { const cid = "cid-1"; const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; cs.lastUsedWorkspace = "previous-value"; // pretend user sent earlier clients.set(cid, cs); @@ -229,7 +234,7 @@ describe("handleSwitchSession — v0.5.bx-32 lastUsedWorkspace contract", () => test("switch updates sessionId, mcodeSessionId, sessionTitle, chat", async () => { const cid = "cid-1"; const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; clients.set(cid, cs); const ctx = { cs, cid, pathname: "" }; @@ -265,7 +270,7 @@ describe("handleNewSession", () => { describe("handleSwitchSession — error paths", () => { test("returns 400 when id is missing", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -278,7 +283,7 @@ describe("handleSwitchSession — error paths", () => { test("returns 400 when id is empty string", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -288,7 +293,7 @@ describe("handleSwitchSession — error paths", () => { test("returns 404 when id not found and not a valid mvs_xxx", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -307,7 +312,7 @@ describe("handleSwitchSession — error paths", () => { // mvs_xxx that doesn't exist in store, but the title-fetcher returns null // → default title "Mcode session" const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -335,7 +340,7 @@ describe("handleSwitchSession — error paths", () => { id: "webui-mvs", mcodeSessionId: "mvs_aaaa1111222233334444555566667777", title: "Mcode-target", - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 3, updatedAt: 3, chat: ["● from mcode sid"], @@ -343,7 +348,7 @@ describe("handleSwitchSession — error paths", () => { ], }); const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -362,7 +367,7 @@ describe("handleSwitchSession — error paths", () => { describe("handleDeleteSession", () => { test("deletes a webui session by id and returns ok", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -384,7 +389,7 @@ describe("handleDeleteSession", () => { id: "webui-mvs-del", mcodeSessionId: "mvs_bbbb1111222233334444555566667777", title: "Mcode-del", - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 3, updatedAt: 3, chat: [], @@ -392,7 +397,7 @@ describe("handleDeleteSession", () => { ], }); const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -406,7 +411,7 @@ describe("handleDeleteSession", () => { test("returns 404 when id not found and not a mvs_xxx", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -419,7 +424,7 @@ describe("handleDeleteSession", () => { test("resets cs when the deleted session is the current one", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; cs.sessionId = "webui-A"; // current is the one we'll delete cs.mcodeSessionId = null; cs.sessionTitle = "A on tmp-webui-A"; @@ -438,7 +443,7 @@ describe("handleDeleteSession", () => { test("?dryRun=true returns preview without deleting the webui entry", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; // Pretend the target session IS the current one, so we can verify // dryRun does NOT reset it. cs.sessionId = "webui-A"; @@ -541,14 +546,14 @@ describe("handleListSessions", () => { describe("handleAcpSessions", () => { test("returns cwd + sessions for the current workspace", async () => { const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; - const req = { url: "/api/acp-sessions?cwd=/tmp/webui-test-A" }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; + const req = { url: `/api/acp-sessions?cwd=${encodeURIComponent(WS_A)}` }; const res = fakeRes(); await handleAcpSessions(req, res, { cs, cid: "cid-1", pathname: "/api/acp-sessions" }); assert.equal(res._status, 200); const body = JSON.parse(res._body); assert.equal(body.ok, true); - assert.equal(body.cwd, "/tmp/webui-test-A"); + assert.equal(body.cwd, WS_A); assert.ok(Array.isArray(body.sessions)); }); @@ -617,7 +622,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { id: "webui-ar", mcodeSessionId: MVS_SID, title: "AR target", - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 9, updatedAt: 9, chat: [], @@ -625,7 +630,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { ], }); const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -645,7 +650,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { id: "webui-ar2", mcodeSessionId: MVS_SID, title: "AR dry", - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 9, updatedAt: 9, chat: [], @@ -653,7 +658,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { ], }); const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; const cid = "cid-1"; clients.set(cid, cs); const res = fakeRes(); @@ -675,7 +680,7 @@ describe("handleDeleteSession — v1.0 anti-resurrection", () => { id: "webui-mc", mcodeSessionId: MVS_SID, title: "MC target", - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 9, updatedAt: 9, chat: [], @@ -723,7 +728,7 @@ describe("handleRenameSession — CRUD rename (改)", () => { const cs = makeClientState(); cs.sessionId = "webui-A"; cs.sessionTitle = "A on tmp-webui-A"; - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; clients.set(cid, cs); // another tab viewing the same session must sync too const cs2 = makeClientState(); @@ -767,7 +772,7 @@ describe("handleRenameSession — CRUD rename (改)", () => { id: "wrap-1", mcodeSessionId: MVS, title: "Mcode session", - workspace: "/tmp/webui-test-A", + workspace: WS_A, createdAt: 1, updatedAt: 1, chat: [], @@ -795,7 +800,7 @@ describe("handleRenameSession — CRUD rename (改)", () => { const MVS = "mvs_" + "b".repeat(32); registerSessionsStore({ initial: [] }); const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; clients.set("cid-1", cs); const res = fakeRes(); @@ -864,7 +869,7 @@ describe("handleRenameSession — CRUD rename (改)", () => { const MVS = "mvs_" + "c".repeat(32); registerSessionsStore({ initial: [] }); const cs = makeClientState(); - cs.workspace = { dir: "/tmp/webui-test-A", branch: null, tree: null }; + cs.workspace = { dir: WS_A, branch: null, tree: null }; clients.set("cid-1", cs); for (const title of ["first custom", "second custom"]) { const res = fakeRes();