From 68f0eee5ae0641f91794d12e29dca81148c4f05a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 11 Jul 2026 19:25:47 -0700 Subject: [PATCH] fix(ui): harden workspace file editing (#104885) * fix(ui): retain unsaved workspace file drafts * fix(ui): reset restored draft hash * fix(ui): keep all unsaved file drafts * fix(ui): harden workspace file editing * fix(ui): canonicalize workspace file identity * fix(gateway): serialize workspace file saves globally * chore: keep release notes in PR body * docs: refresh generated docs map --- docs/docs_map.md | 7 +- docs/web/control-ui.md | 2 +- .../gateway-protocol/src/schema/sessions.ts | 10 +- scripts/control-ui-mock-dev.ts | 9 +- .../server-methods/sessions-files.test.ts | 113 ++++++++++++++++++ src/gateway/server-methods/sessions-files.ts | 77 ++++++------ src/gateway/server-methods/workspace-fs.ts | 75 ++++++++++-- ui/src/pages/chat/chat-pane.ts | 1 + .../components/chat-session-workspace.test.ts | 88 ++++++++++++++ .../chat/components/chat-session-workspace.ts | 27 ++--- .../chat-sidebar-file-view.browser.test.ts | 79 ++++++++++++ ui/src/pages/chat/components/chat-sidebar.ts | 61 +++++++++- .../chat/components/chat-tool-cards.test.ts | 24 ++-- 13 files changed, 492 insertions(+), 81 deletions(-) diff --git a/docs/docs_map.md b/docs/docs_map.md index 29265681af5c..5478eea655c3 100644 --- a/docs/docs_map.md +++ b/docs/docs_map.md @@ -8311,12 +8311,15 @@ Do not edit it by hand; run `pnpm docs:map:gen`. - H2: Control UI - H2: Authorization and privacy - H2: Audience projection + - H2: Delivered-surface convergence - H2: Restart, timeout, and route semantics - H2: Compatibility plan - H2: Rollout - H3: PR 1: durable lifecycle - - H3: PR 2: deep link and typed actions - - H3: PR 3: propagation and fail-closed behavior + - H3: PR 2: typed actions and channel callbacks + - H3: PR 3: Control UI deep link + - H3: PR 4: native clients + - H3: PR 5: propagation and fail-closed behavior - H2: Tests - H2: Observability - H2: Open decisions diff --git a/docs/web/control-ui.md b/docs/web/control-ui.md index e94219d5892b..3bf0ee32ecd6 100644 --- a/docs/web/control-ui.md +++ b/docs/web/control-ui.md @@ -310,7 +310,7 @@ The macOS app keeps its native link-browser sidebar for links clicked in the das - When a session's checkout sits on a non-default branch of a GitHub repository, the chat view pins pull request chips above the composer: PR number, repo, branch, diff counts, a CI pill, and draft/merged/closed state, each linking to the PR. The row shows at most two chips — live (open/draft) PRs first — and a "Show more" button reveals collapsed merged/closed history. The CI pill opens a small CI monitoring popover with passed/failed/running/skipped check counts and a link to the PR's checks page. Detection runs server-side through `controlUi.sessionPullRequests`, which reuses the Gateway's `GH_TOKEN`/`GITHUB_TOKEN` when set. When the GitHub API rate limit is hit, chips keep the last known status and show a warning that the status may be out of date; dismissing a chip hides it for that session in the current browser profile. - The session diff panel shows what a session's checkout actually changed: the branch button (in the workspace rail header, the split-pane header, or the floating button in single-pane chat) opens the detail panel with a per-file diff of branch, uncommitted, and untracked work against the checkout's default-branch merge base — status dot, rename arrow, per-file +/− counts, collapsible files, and "N unmodified lines" markers between hunks. Diffs are computed server-side through the `sessions.diff` Gateway method (`operator.read` scope); binary and oversized files degrade to stats-only entries, and the button only appears when the connected Gateway advertises `sessions.diff`. - The session workspace rail in each Chat pane lists session files, project files, and artifacts. It docks to the pane's right edge by default; drag its header (or use the dock button) to move it to the bottom, and the choice is stored in the current browser profile. A collapsed rail takes no space at all: reopen it with ⇧⌘B, the files toggle in the split-pane header, or the floating files button in single-pane chat (both carry a changed-file count badge). The separate file, tool, and Canvas detail panel is unaffected. - - Clicking a file reference in chat, a file path in an expanded read/edit/write tool card, or a file row in the workspace rail opens the file detail panel: a CodeMirror-based code view with syntax highlighting, line numbers, jump-to-line, in-file search, copy actions, and an open-in-external-editor menu. When the Gateway advertises `sessions.files.set` to an `operator.admin` connection, the panel adds an Edit mode with dirty tracking and Cmd/Ctrl-S save. Saves are compare-and-swap on a content hash returned by `sessions.files.get`: if the file changed on disk since it was loaded (for example because the agent kept working), the panel shows a conflict notice with Reload (take the latest content) and Overwrite (keep the local edit) actions. Writes go through the same fs-safe workspace guards as reads — path containment, symlink/hardlink rejection, and a 256 KB UTF-8 cap — and only overwrite existing files; the editor never creates or deletes them. + - Clicking a file reference in chat, a file path in an expanded read/edit/write tool card, or a file row in the workspace rail opens the file detail panel: a CodeMirror-based code view with syntax highlighting, line numbers, jump-to-line, in-file search, copy actions, and an open-in-external-editor menu. When the Gateway advertises `sessions.files.set` to an `operator.admin` connection, the panel adds an Edit mode with dirty tracking and Cmd/Ctrl-S save; unsaved drafts survive file, panel, and session navigation in the current browser tab until explicitly saved or discarded. Saves are compare-and-swap on a content hash returned by `sessions.files.get`: if the file changed on disk since it was loaded (for example because the agent kept working), the panel shows a conflict notice with Reload (take the latest content) and Overwrite (keep the local edit) actions. Writes go through the same fs-safe workspace guards as reads — path containment, symlink/hardlink rejection, and a 256 KB UTF-8 cap — and only overwrite existing files; the editor never creates or deletes them. - The background tasks rail in each Chat pane lists the current agent's background tasks and subagents (`tasks.list` scoped by agent, kept live by `task` events): running work shows a live elapsed timer, tool-use count, the tool currently in use, and a stop control; the collapsible finished section adds run durations; and a View transcript link opens the task's child session in the pane. Open it with the activity toggle in the split-pane header or the floating activity button in single-pane chat — the task snapshot loads eagerly, so both carry a running-count badge without opening the rail first. The Tasks page remains the full cross-agent ledger. - The workspace rail, background tasks rail, and detail panel adapt to each pane's own width rather than the window: in a narrow pane or compact window both rails present as bottom strips (side-dock controls hide until the pane widens; the workspace rail keeps first claim on the side slot when only one column fits), and the detail panel stacks below the thread with a horizontal resize handle instead of sharing the row with it. Phone-sized viewports still open the detail panel full-screen. - The chat header model and thinking pickers patch the active session immediately through `sessions.patch`; they are persistent session overrides, not one-turn-only send options. diff --git a/packages/gateway-protocol/src/schema/sessions.ts b/packages/gateway-protocol/src/schema/sessions.ts index 90e9ad004f9e..14a04602e2bd 100644 --- a/packages/gateway-protocol/src/schema/sessions.ts +++ b/packages/gateway-protocol/src/schema/sessions.ts @@ -74,6 +74,12 @@ export const SessionFileRelevanceSchema = Type.Union([ Type.Literal("mixed"), ]); +const SessionFileHashSchema = Type.String({ + minLength: 64, + maxLength: 64, + pattern: "^[a-f0-9]{64}$", +}); + /** One file path referenced by a session transcript. */ export const SessionFileEntrySchema = Type.Object( { @@ -85,7 +91,7 @@ export const SessionFileEntrySchema = Type.Object( size: Type.Optional(Type.Integer({ minimum: 0 })), updatedAtMs: Type.Optional(Type.Integer({ minimum: 0 })), content: Type.Optional(Type.String()), - hash: Type.Optional(NonEmptyString), + hash: Type.Optional(SessionFileHashSchema), }, { additionalProperties: false }, ); @@ -164,7 +170,7 @@ export const SessionsFilesSetParamsSchema = Type.Object( path: NonEmptyString, agentId: Type.Optional(NonEmptyString), content: Type.String(), - expectedHash: NonEmptyString, + expectedHash: SessionFileHashSchema, }, { additionalProperties: false }, ); diff --git a/scripts/control-ui-mock-dev.ts b/scripts/control-ui-mock-dev.ts index c820189d6452..25f7a3f8311a 100644 --- a/scripts/control-ui-mock-dev.ts +++ b/scripts/control-ui-mock-dev.ts @@ -1,4 +1,5 @@ // Control Ui Mock Dev script supports OpenClaw repository automation. +import { createHash } from "node:crypto"; import path from "node:path"; import { fileURLToPath } from "node:url"; import qrcode from "qrcode"; @@ -35,6 +36,10 @@ const TOTAL_TELEGRAM_SESSIONS = 180; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const uiRoot = path.join(repoRoot, "ui"); +function mockFileHash(value: string): string { + return createHash("sha256").update(value, "utf8").digest("hex"); +} + function parseArgs(args: string[]): CliOptions { const options: CliOptions = { allowedHosts: [], host: "127.0.0.1", port: 5187 }; for (let i = 0; i < args.length; i += 1) { @@ -754,7 +759,7 @@ async function createChatPickerScenario(): Promise ...file, content: sessionFileContentByPath.get(file.path) ?? "", // Fake CAS token so the file panel offers edit mode against the mock. - hash: `mock-hash-${file.name}`, + hash: mockFileHash(sessionFileContentByPath.get(file.path) ?? ""), }, root: sessionWorkspaceRoot, sessionKey: "agent:alpha", @@ -767,7 +772,7 @@ async function createChatPickerScenario(): Promise ...file, kind: "modified", workspacePath: file.path, - hash: `mock-hash-${file.name}-saved`, + hash: mockFileHash(`${file.path}:saved`), updatedAtMs: baseTime, }, root: sessionWorkspaceRoot, diff --git a/src/gateway/server-methods/sessions-files.test.ts b/src/gateway/server-methods/sessions-files.test.ts index d22adfe62bff..5965b88860e3 100644 --- a/src/gateway/server-methods/sessions-files.test.ts +++ b/src/gateway/server-methods/sessions-files.test.ts @@ -5,6 +5,7 @@ import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { sessionsFilesHandlers } from "./sessions-files.js"; +import { updateWorkspaceFile } from "./workspace-fs.js"; const hoisted = vi.hoisted(() => ({ loadSessionEntry: vi.fn(), @@ -310,6 +311,14 @@ describe("sessions.files RPC handlers", () => { ); expect(browserPreview.file.content).toBe("# Nested read me\n"); + const aliasedPreview = expectOkPayload( + await invokeSessionFilesHandler("sessions.files.get", { + sessionKey: "agent:main:main", + path: "packages//app/src/readme.md", + }), + ); + expect(aliasedPreview.file.workspacePath).toBe("packages/app/src/readme.md"); + const parentRelativePreview = expectOkPayload( await invokeSessionFilesHandler("sessions.files.get", { sessionKey: "agent:main:main", @@ -690,6 +699,91 @@ describe("sessions.files RPC handlers", () => { expect(fs.readFileSync(path.join(workspaceRoot, "ui/vite.config.ts"), "utf8")).toBe(current); }); + it("rejects malformed CAS hashes before reading the workspace", async () => { + const calls = await invokeSessionFilesHandler("sessions.files.set", { + sessionKey: "agent:main:main", + path: "ui/vite.config.ts", + content: "changed\n", + expectedHash: "not-a-sha256", + }); + + expect(calls).toMatchObject([{ ok: false }]); + expect(fs.readFileSync(path.join(workspaceRoot, "ui/vite.config.ts"), "utf8")).toBe( + "export default {};\n", + ); + }); + + it("allows only one concurrent save for the same expected hash", async () => { + const original = "export default {};\n"; + const expectedHash = hashContent(original); + const [first, second] = await Promise.all([ + invokeSessionFilesHandler("sessions.files.set", { + sessionKey: "agent:main:main", + path: "ui/vite.config.ts", + content: "export default { first: true };\n", + expectedHash, + }), + invokeSessionFilesHandler("sessions.files.set", { + sessionKey: "agent:main:main", + path: "ui/vite.config.ts", + content: "export default { second: true };\n", + expectedHash, + }), + ]); + + const calls = [first[0], second[0]]; + expect(calls.filter((call) => call?.ok)).toHaveLength(1); + const conflict = calls.find((call) => !call?.ok)?.error as Record; + expect(conflict.details).toMatchObject({ type: "session_file_conflict" }); + const content = fs.readFileSync(path.join(workspaceRoot, "ui/vite.config.ts"), "utf8"); + expect(["export default { first: true };\n", "export default { second: true };\n"]).toContain( + content, + ); + expect(conflict.details.currentHash).toBe(hashContent(content)); + }); + + it("serializes concurrent saves across lexical aliases", async () => { + const original = "export default {};\n"; + const expectedHash = hashContent(original); + const results = await Promise.all([ + updateWorkspaceFile( + workspaceRoot, + "ui/vite.config.ts", + "export default { first: true };\n", + expectedHash, + ), + updateWorkspaceFile( + workspaceRoot, + "ui//vite.config.ts", + "export default { second: true };\n", + expectedHash, + ), + ]); + + expect(results.map((result) => result.status).toSorted()).toEqual(["conflict", "updated"]); + }); + + it("serializes concurrent saves across nested workspace roots", async () => { + const original = "export default {};\n"; + const expectedHash = hashContent(original); + const results = await Promise.all([ + updateWorkspaceFile( + workspaceRoot, + "ui/vite.config.ts", + "export default { outer: true };\n", + expectedHash, + ), + updateWorkspaceFile( + path.join(workspaceRoot, "ui"), + "vite.config.ts", + "export default { nested: true };\n", + expectedHash, + ), + ]); + + expect(results.map((result) => result.status).toSorted()).toEqual(["conflict", "updated"]); + }); + it("rejects writes to nonexistent files", async () => { const error = expectError( await invokeSessionFilesHandler("sessions.files.set", { @@ -771,6 +865,25 @@ describe("sessions.files RPC handlers", () => { ); }); + it("rejects replacement content that cannot round-trip through UTF-8", async () => { + const error = expectError( + await invokeSessionFilesHandler("sessions.files.set", { + sessionKey: "agent:main:main", + path: "ui/vite.config.ts", + content: "before\ud800after", + expectedHash: hashContent("export default {};\n"), + }), + ); + + expect(error.details).toMatchObject({ + path: "ui/vite.config.ts", + type: "session_file_unsafe", + }); + expect(fs.readFileSync(path.join(workspaceRoot, "ui/vite.config.ts"), "utf8")).toBe( + "export default {};\n", + ); + }); + it("previews binary files without issuing a CAS hash", async () => { const binary = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0x01, 0x02]); fs.writeFileSync(path.join(workspaceRoot, "logo.png"), binary); diff --git a/src/gateway/server-methods/sessions-files.ts b/src/gateway/server-methods/sessions-files.ts index 5cbb22c494c0..c788b8a9973d 100644 --- a/src/gateway/server-methods/sessions-files.ts +++ b/src/gateway/server-methods/sessions-files.ts @@ -32,10 +32,11 @@ import { sortWorkspaceEntries, statWorkspacePath, toUpdatedAtMs, + updateWorkspaceFile, WORKSPACE_PREVIEW_MAX_BYTES, - writeWorkspaceFile, workspaceStatKind, type WorkspaceDirEntry, + type WorkspaceFileUpdateResult, } from "./workspace-fs.js"; type FileKind = "modified" | "read"; @@ -319,6 +320,7 @@ async function toSessionFileEntry( return { ...base, missing: true }; } if (read !== "too-large") { + entry.workspacePath = read.canonicalPath; entry.size = read.stat.size; entry.updatedAtMs = toUpdatedAtMs(read.stat.mtimeMs); const text = decodeUtf8Strict(read.buffer); @@ -679,7 +681,14 @@ export const sessionsFilesHandlers: GatewayRequestHandlers = { respondSessionFileUnsafe(respond, params.path); return; } - const contentSize = Buffer.byteLength(params.content, "utf8"); + const contentBuffer = Buffer.from(params.content, "utf8"); + // Node replaces lone UTF-16 surrogates while encoding. Reject them instead + // of reporting a hash for bytes that no longer match the submitted text. + if (contentBuffer.toString("utf8") !== params.content) { + respondSessionFileUnsafe(respond, params.path); + return; + } + const contentSize = contentBuffer.byteLength; if (contentSize > MAX_PREVIEW_BYTES) { respond( false, @@ -715,39 +724,14 @@ export const sessionsFilesHandlers: GatewayRequestHandlers = { respondSessionFileNotFound(respond, params.path); return; } - const current = await readWorkspaceFile(loaded.root, browserPath); - if (!current || current === "too-large") { - respondSessionFileUnsafe(respond, params.path); - return; - } - // Only strict-UTF-8 text is editable; sessions.files.get never issues a CAS - // hash for binary previews, and overwriting binary bytes with re-encoded - // text would corrupt the file even when a client computes the hash itself. - if (decodeUtf8Strict(current.buffer) === undefined) { - respondSessionFileUnsafe(respond, params.path); - return; - } - const currentHash = createHash("sha256").update(current.buffer).digest("hex"); - if (currentHash !== params.expectedHash) { - respond( - false, - undefined, - sessionFilesError("session_file_conflict", "session file changed since it was read", { - path: params.path, - currentHash, - }), - ); - return; - } - // Check-then-write is intentionally non-atomic: agents edit this tree from - // their own processes, so no gateway-side lock can serialize them. The hash - // gate exists to reject stale operator loads; the remaining window is the - // few milliseconds inside this handler and is accepted. + let update: WorkspaceFileUpdateResult; try { - if (!(await writeWorkspaceFile(loaded.root, browserPath, params.content))) { - respondSessionFileUnsafe(respond, params.path); - return; - } + update = await updateWorkspaceFile( + loaded.root, + browserPath, + params.content, + params.expectedHash, + ); } catch (err) { if (!(err instanceof FsSafeError)) { throw err; @@ -755,24 +739,33 @@ export const sessionsFilesHandlers: GatewayRequestHandlers = { respondSessionFileUnsafe(respond, params.path); return; } - const stat = await statWorkspacePath(loaded.root, browserPath); - if (!stat || workspaceStatKind(stat) !== "file") { + if (update.status === "conflict") { + respond( + false, + undefined, + sessionFilesError("session_file_conflict", "session file changed since it was read", { + path: params.path, + currentHash: update.currentHash, + }), + ); + return; + } + if (update.status === "unsafe") { respondSessionFileUnsafe(respond, params.path); return; } - const hash = createHash("sha256").update(params.content, "utf8").digest("hex"); respond(true, { sessionKey: params.sessionKey, root: loaded.root, file: { path: params.path, - workspacePath: browserPath, - name: displayNameForPath(browserPath), + workspacePath: update.canonicalPath, + name: displayNameForPath(update.canonicalPath), kind: "modified", missing: false, - size: stat.size, - updatedAtMs: toUpdatedAtMs(stat.mtimeMs), - hash, + size: update.stat.size, + updatedAtMs: toUpdatedAtMs(update.stat.mtimeMs), + hash: update.hash, }, }); }, diff --git a/src/gateway/server-methods/workspace-fs.ts b/src/gateway/server-methods/workspace-fs.ts index ff179626672d..0def5b5861a9 100644 --- a/src/gateway/server-methods/workspace-fs.ts +++ b/src/gateway/server-methods/workspace-fs.ts @@ -1,16 +1,20 @@ // Shared workspace filesystem access for gateway file browsers and editors. // All entry points route through fs-safe roots (realpathed root, symlink and // hardlink rejection) so no caller can access files outside a workspace root. +import { createHash } from "node:crypto"; import path from "node:path"; import { root as fsSafeRoot, FsSafeError, type ReadResult } from "../../infra/fs-safe.js"; export type WorkspaceRoot = Awaited>; export type WorkspacePathStat = Awaited>; export type WorkspaceDirEntry = WorkspacePathStat & { name: string }; +export type WorkspaceFileReadResult = ReadResult & { canonicalPath: string }; /** Shared preview cap: keeps file payloads comfortably under client WS limits. */ export const WORKSPACE_PREVIEW_MAX_BYTES = 256 * 1024; +let workspaceFileUpdateQueue: Promise = Promise.resolve(); + async function openWorkspaceRoot(rootDir: string): Promise { try { return await fsSafeRoot(rootDir, { @@ -58,18 +62,22 @@ export async function readWorkspaceFile( rootDir: string, browserPath: string, opts?: { maxBytes?: number }, -): Promise { +): Promise { const workspaceRoot = await openWorkspaceRoot(rootDir); if (!workspaceRoot) { return undefined; } try { - return await workspaceRoot.read(browserPath, { + const read = await workspaceRoot.read(browserPath, { hardlinks: "reject", maxBytes: opts?.maxBytes ?? WORKSPACE_PREVIEW_MAX_BYTES, nonBlockingRead: true, symlinks: "reject", }); + return { + ...read, + canonicalPath: path.relative(workspaceRoot.rootReal, read.realPath).split(path.sep).join("/"), + }; } catch (err) { if (err instanceof FsSafeError && err.code === "too-large") { return "too-large"; @@ -78,17 +86,70 @@ export async function readWorkspaceFile( } } -export async function writeWorkspaceFile( +export type WorkspaceFileUpdateResult = + | { status: "updated"; canonicalPath: string; hash: string; stat: WorkspacePathStat } + | { status: "conflict"; currentHash: string } + | { status: "unsafe" }; + +function enqueueWorkspaceFileUpdate(update: () => Promise): Promise { + const result = workspaceFileUpdateQueue.then(update, update); + workspaceFileUpdateQueue = result.then( + () => undefined, + () => undefined, + ); + return result; +} + +export async function updateWorkspaceFile( rootDir: string, browserPath: string, content: string, -): Promise { + expectedHash: string, +): Promise { const workspaceRoot = await openWorkspaceRoot(rootDir); if (!workspaceRoot) { - return undefined; + return { status: "unsafe" }; } - await workspaceRoot.write(browserPath, content, { encoding: "utf8" }); - return true; + // Serialize every low-frequency editor save. The same physical file can be + // exposed through path aliases or nested workspace roots, so narrower queue + // keys can let two routes accept one stale hash and overwrite each other. + return await enqueueWorkspaceFileUpdate(async () => { + let current: ReadResult; + try { + current = await workspaceRoot.read(browserPath, { + hardlinks: "reject", + maxBytes: WORKSPACE_PREVIEW_MAX_BYTES, + nonBlockingRead: true, + symlinks: "reject", + }); + } catch { + return { status: "unsafe" }; + } + if (decodeUtf8Strict(current.buffer) === undefined) { + return { status: "unsafe" }; + } + const currentHash = createHash("sha256").update(current.buffer).digest("hex"); + if (currentHash !== expectedHash) { + return { status: "conflict", currentHash }; + } + await workspaceRoot.write(browserPath, content, { + encoding: "utf8", + renameIdentity: "strict", + }); + const stat = await workspaceRoot.stat(browserPath); + if (workspaceStatKind(stat) !== "file") { + return { status: "unsafe" }; + } + return { + status: "updated", + canonicalPath: path + .relative(workspaceRoot.rootReal, current.realPath) + .split(path.sep) + .join("/"), + hash: createHash("sha256").update(content, "utf8").digest("hex"), + stat, + }; + }); } export function decodeUtf8Strict(buffer: Buffer): string | undefined { diff --git a/ui/src/pages/chat/chat-pane.ts b/ui/src/pages/chat/chat-pane.ts index cd8c6cc7291d..24b7d4cc36d7 100644 --- a/ui/src/pages/chat/chat-pane.ts +++ b/ui/src/pages/chat/chat-pane.ts @@ -1481,6 +1481,7 @@ class ChatPane extends OpenClawLightDomElement { this.context.gateway.snapshot.hello?.auth ?? null, ); const sessionWorkspace = createSessionWorkspaceProps(state, { + draftScope: this.paneId, narrowLayout: this.paneWidth < WORKSPACE_RAIL_SIDE_MIN_PANE_WIDTH, }); const railSideDocked = diff --git a/ui/src/pages/chat/components/chat-session-workspace.test.ts b/ui/src/pages/chat/components/chat-session-workspace.test.ts index 36e9d6010349..a8f3d2da3acc 100644 --- a/ui/src/pages/chat/components/chat-session-workspace.test.ts +++ b/ui/src/pages/chat/components/chat-session-workspace.test.ts @@ -1,11 +1,21 @@ import { describe, expect, it, vi } from "vitest"; import { createSessionWorkspaceProps, + openSessionWorkspaceFile, toggleSessionWorkspace, type SessionWorkspaceHost, workspaceBrowserFilePath, } from "./chat-session-workspace.ts"; +function gatewayHello(methods: string[], scopes = ["operator.admin"]) { + return { + type: "hello-ok" as const, + protocol: 3, + auth: { role: "operator", scopes }, + features: { methods }, + }; +} + describe("toggleSessionWorkspace", () => { it("expands and collapses the session workspace rail", () => { const requestUpdate = vi.fn(); @@ -49,3 +59,81 @@ describe("workspaceBrowserFilePath", () => { expect(workspaceBrowserFilePath("/", "src/readme.md")).toBe("/src/readme.md"); }); }); + +describe("openSessionWorkspaceFile", () => { + it("opens Markdown with a canonical Gateway- and pane-scoped draft identity", async () => { + const handleOpenSidebar = vi.fn(); + const getFile = vi.fn().mockResolvedValue({ + sessionKey: "agent:main:current", + root: "/workspace", + file: { + path: "README.md", + workspacePath: "README.md", + name: "README.md", + kind: "read", + missing: false, + content: "# Before\n", + hash: "a".repeat(64), + }, + }); + const state = { + client: {}, + connected: true, + handleOpenSidebar, + hello: gatewayHello(["sessions.files.set"]), + sessionKey: "agent:main:current", + sessionWorkspaceDraftScope: "pane-left", + settings: { gatewayUrl: "wss://gateway-a.example" }, + sessions: { getFile }, + } as unknown as SessionWorkspaceHost; + + openSessionWorkspaceFile(state, { path: "readme.md" }); + + await vi.waitFor(() => expect(handleOpenSidebar).toHaveBeenCalledOnce()); + expect(handleOpenSidebar.mock.calls[0]?.[0]).toMatchObject({ + kind: "file", + name: "README.md", + content: "# Before\n", + draftKey: + "wss://gateway-a.example\u0000pane-left\u0000agent:main:current\u0000/workspace\u0000README.md", + edit: { hash: "a".repeat(64) }, + }); + }); + + it.each([ + { label: "the method is not advertised", methods: [], scopes: ["operator.admin"] }, + { + label: "the connection lacks admin scope", + methods: ["sessions.files.set"], + scopes: ["operator.read"], + }, + ])("keeps Markdown read-only when $label", async ({ methods, scopes }) => { + const handleOpenSidebar = vi.fn(); + const state = { + client: {}, + connected: true, + handleOpenSidebar, + hello: gatewayHello(methods, scopes), + sessionKey: "agent:main:current", + sessions: { + getFile: vi.fn().mockResolvedValue({ + sessionKey: "agent:main:current", + file: { + path: "README.md", + name: "README.md", + kind: "read", + missing: false, + content: "# Before\n", + hash: "a".repeat(64), + }, + }), + }, + } as unknown as SessionWorkspaceHost; + + openSessionWorkspaceFile(state, { path: "README.md" }); + + await vi.waitFor(() => expect(handleOpenSidebar).toHaveBeenCalledOnce()); + expect(handleOpenSidebar.mock.calls[0]?.[0]).toMatchObject({ kind: "file" }); + expect(handleOpenSidebar.mock.calls[0]?.[0]?.edit).toBeUndefined(); + }); +}); diff --git a/ui/src/pages/chat/components/chat-session-workspace.ts b/ui/src/pages/chat/components/chat-session-workspace.ts index 0314a90ebd71..815c728ecb44 100644 --- a/ui/src/pages/chat/components/chat-session-workspace.ts +++ b/ui/src/pages/chat/components/chat-session-workspace.ts @@ -99,6 +99,7 @@ export type SessionWorkspaceHost = { settings?: UiSettings; sessionWorkspaceState?: SessionWorkspaceState; sessionWorkspaceOpenRequest?: SessionWorkspaceOpenRequest; + sessionWorkspaceDraftScope?: string; requestUpdate?: () => void; handleOpenSidebar: (content: SidebarContent) => void; }; @@ -180,13 +181,6 @@ function languageForFile(name: string): string { return extension; } -function fileSidebarContent(name: string, content: string): string { - if (/\.(?:md|markdown|mdx)$/i.test(name)) { - return content; - } - return `# ${name}\n\n\`\`\`${languageForFile(name)}\n${content}\n\`\`\``; -} - function basenameForPath(filePath: string): string { return filePath.split(/[\\/]/).findLast((part) => part) ?? filePath; } @@ -409,17 +403,10 @@ function openFile( return null; } const name = file.name || basenameForPath(path); - if (/\.(?:md|markdown|mdx)$/i.test(name) && opts.line == null) { - return { - kind: "markdown", - content: fileSidebarContent(name, file.content), - rawText: file.content, - }; - } const canEdit = typeof file.hash === "string" && hasUniformLineEndings(file.content) && - isGatewayMethodAdvertised(state, "sessions.files.set") !== false && + isGatewayMethodAdvertised(state, "sessions.files.set") === true && hasOperatorAdminAccess(state.hello?.auth ?? null); const edit = canEdit ? { @@ -494,6 +481,13 @@ function openFile( path: file.workspacePath || file.path || path, name, content: file.content, + draftKey: [ + state.settings?.gatewayUrl ?? "", + state.sessionWorkspaceDraftScope ?? "", + result.sessionKey, + result.root ?? "", + file.workspacePath || file.path || path, + ].join("\u0000"), root: result.root ?? null, language: languageForFile(name), line: opts.line ?? null, @@ -647,8 +641,9 @@ function openArtifact( export function createSessionWorkspaceProps( state: SessionWorkspaceHost, - options?: { narrowLayout?: boolean }, + options?: { narrowLayout?: boolean; draftScope?: string }, ): SessionWorkspaceProps { + state.sessionWorkspaceDraftScope = options?.draftScope; const workspace = getWorkspaceState(state); if ( !workspace.collapsed && diff --git a/ui/src/pages/chat/components/chat-sidebar-file-view.browser.test.ts b/ui/src/pages/chat/components/chat-sidebar-file-view.browser.test.ts index c291da6aeeda..cc1a8703d45c 100644 --- a/ui/src/pages/chat/components/chat-sidebar-file-view.browser.test.ts +++ b/ui/src/pages/chat/components/chat-sidebar-file-view.browser.test.ts @@ -136,6 +136,85 @@ describe.runIf(browserMode)("chat file editor", () => { await expect.poll(() => button(panel, "Save").textContent?.trim()).toBe("Save"); expect(button(panel, "Save").disabled).toBe(false); expect(editor.textContent).toContain("newer"); + await userEvent.click(button(panel, "Discard")); + }); + + it("restores an unsaved draft after the detail panel is closed", async () => { + const originalEdit = { hash: "hash-1", save: vi.fn(), fetchLatest: vi.fn() }; + const first = await mountFile({ + kind: "file", + draftKey: "session-a\u0000notes.txt", + path: "notes.txt", + name: "notes.txt", + content: "before", + edit: originalEdit, + }); + + await userEvent.click(button(first, "Edit file")); + await userEvent.fill(first.querySelector(".cm-content")!, "unsaved draft"); + first.remove(); + + const save = vi.fn().mockResolvedValue({ ok: true, hash: "hash-3" }); + const reopened = await mountFile({ + kind: "file", + draftKey: "session-a\u0000notes.txt", + path: "notes.txt", + name: "notes.txt", + content: "latest", + edit: { hash: "hash-2", save, fetchLatest: vi.fn() }, + }); + await expect + .poll(() => reopened.querySelector(".cm-content")?.textContent) + .toContain("unsaved"); + expect(reopened.querySelector(".cm-content")?.getAttribute("contenteditable")).toBe("true"); + expect(button(reopened, "Save").disabled).toBe(false); + + await userEvent.click(button(reopened, "Discard")); + await userEvent.click(button(reopened, "Edit file")); + await userEvent.fill(reopened.querySelector(".cm-content")!, "new edit"); + await userEvent.click(button(reopened, "Save")); + await expect.poll(() => save.mock.calls.length).toBe(1); + expect(save).toHaveBeenCalledWith({ content: "new edit", expectedHash: "hash-2" }); + }); + + it("scopes retained drafts to the session file identity", async () => { + const edit = { hash: "hash-1", save: vi.fn(), fetchLatest: vi.fn() }; + const first = await mountFile({ + kind: "file", + draftKey: "gateway-a\u0000pane-left\u0000session-a\u0000shared.txt", + path: "shared.txt", + name: "shared.txt", + content: "session a", + edit, + }); + + await userEvent.click(button(first, "Edit file")); + await userEvent.fill(first.querySelector(".cm-content")!, "session a draft"); + first.remove(); + + const otherSession = await mountFile({ + kind: "file", + draftKey: "gateway-a\u0000pane-right\u0000session-a\u0000shared.txt", + path: "shared.txt", + name: "shared.txt", + content: "session b", + edit, + }); + expect(otherSession.querySelector(".cm-content")?.textContent).toContain("session b"); + expect(otherSession.querySelector(".cm-content")?.getAttribute("contenteditable")).toBe( + "false", + ); + + const restored = await mountFile({ + kind: "file", + draftKey: "gateway-a\u0000pane-left\u0000session-a\u0000shared.txt", + path: "shared.txt", + name: "shared.txt", + content: "session a", + edit, + }); + await expect.poll(() => restored.querySelector(".cm-content")?.textContent).toContain("draft"); + await userEvent.click(button(restored, "Discard")); }); it("reloads the latest content after a save conflict", async () => { diff --git a/ui/src/pages/chat/components/chat-sidebar.ts b/ui/src/pages/chat/components/chat-sidebar.ts index a48efe73c89a..e65511bb0582 100644 --- a/ui/src/pages/chat/components/chat-sidebar.ts +++ b/ui/src/pages/chat/components/chat-sidebar.ts @@ -101,6 +101,8 @@ export type FileSidebarContent = { path: string; name: string; content: string; + /** Stable per-session identity used to retain an unsaved in-memory draft. */ + draftKey?: string; root?: string | null; language?: string; line?: number | null; @@ -110,6 +112,26 @@ export type FileSidebarContent = { edit?: FileSidebarEdit; }; +type RetainedFileDraft = { + content: string; + expectedHash: string; +}; + +const retainedFileDrafts = new Map(); + +function retainedFileDraftKey(content: FileSidebarContent): string { + return content.draftKey ?? `${content.root ?? ""}\u0000${content.path}`; +} + +function setRetainedFileDraft(content: FileSidebarContent, draft: RetainedFileDraft | null) { + const key = retainedFileDraftKey(content); + retainedFileDrafts.delete(key); + if (!draft) { + return; + } + retainedFileDrafts.set(key, draft); +} + export type SidebarContent = | MarkdownSidebarContent | CanvasSidebarContent @@ -682,6 +704,7 @@ class ChatDetailPanel extends OpenClawLightDomElement { private showingRawText = false; private fileEditor: FileEditorViewHandle | null = null; private fileEditorLoad: Promise | null = null; + private fileDraftContent: string | null = null; private fileSavedContent = ""; private fileHash = ""; private copyFeedbackTimer: ReturnType | null = null; @@ -720,8 +743,24 @@ class ChatDetailPanel extends OpenClawLightDomElement { this.fileReloading = false; this.fileSaving = false; this.fileSaveNotice = null; + const retainedDraft = + this.content?.kind === "file" && this.content.edit + ? retainedFileDrafts.get(retainedFileDraftKey(this.content)) + : undefined; + const restoredDraft = + this.content?.kind === "file" && retainedDraft?.content !== this.content.content + ? retainedDraft + : undefined; + if (retainedDraft && !restoredDraft && this.content?.kind === "file") { + setRetainedFileDraft(this.content, null); + } + this.fileDraftContent = restoredDraft?.content ?? null; this.fileSavedContent = this.content?.kind === "file" ? this.content.content : ""; - this.fileHash = this.content?.kind === "file" ? (this.content.edit?.hash ?? "") : ""; + this.fileHash = + restoredDraft?.expectedHash ?? + (this.content?.kind === "file" ? (this.content.edit?.hash ?? "") : ""); + this.fileEditing = Boolean(restoredDraft); + this.fileDirty = Boolean(restoredDraft); this.fileEditorLoading = this.content?.kind === "file"; this.destroyFileEditor(); if (this.copyFeedbackTimer) { @@ -789,7 +828,7 @@ class ChatDetailPanel extends OpenClawLightDomElement { } const editor = await createFileEditorView({ parent, - content: current.content, + content: this.fileDraftContent ?? current.content, name: current.name, editable: this.fileEditing, onSave: this.saveFile, @@ -803,11 +842,19 @@ class ChatDetailPanel extends OpenClawLightDomElement { return; } this.fileEditor = editor; + this.fileDraftContent = null; editor.onDocChanged((nextContent) => { const dirty = nextContent !== this.fileSavedContent; if (dirty !== this.fileDirty) { this.fileDirty = dirty; } + if (!dirty && this.visibleContent?.kind === "file") { + this.fileHash = this.visibleContent.edit?.hash ?? ""; + } + setRetainedFileDraft( + current, + dirty ? { content: nextContent, expectedHash: this.fileHash } : null, + ); if (this.fileSaveNotice?.kind === "error") { this.fileSaveNotice = null; } @@ -966,6 +1013,11 @@ class ChatDetailPanel extends OpenClawLightDomElement { return; } this.fileEditor?.setContent(this.fileSavedContent); + const content = this.visibleContent; + if (content?.kind === "file") { + setRetainedFileDraft(content, null); + this.fileHash = content.edit?.hash ?? ""; + } this.fileDirty = false; this.fileSaveNotice = null; this.fileEditing = false; @@ -976,6 +1028,11 @@ class ChatDetailPanel extends OpenClawLightDomElement { this.fileSavedContent = nextContent; this.fileHash = hash; this.fileDirty = this.fileEditor?.getContent() !== nextContent; + const draftContent = this.fileEditor?.getContent(); + setRetainedFileDraft( + content, + this.fileDirty && draftContent != null ? { content: draftContent, expectedHash: hash } : null, + ); this.fileSaveNotice = null; this.visibleContent = { ...content, diff --git a/ui/src/pages/chat/components/chat-tool-cards.test.ts b/ui/src/pages/chat/components/chat-tool-cards.test.ts index 40f9dd16a429..9de31ebd860a 100644 --- a/ui/src/pages/chat/components/chat-tool-cards.test.ts +++ b/ui/src/pages/chat/components/chat-tool-cards.test.ts @@ -279,15 +279,27 @@ describe("tool-cards", () => { } }); - it("opens the raw file path from an expanded edit card", () => { + it.each([ + { name: "read", args: { path: "packages/app/src/read.ts" }, path: "packages/app/src/read.ts" }, + { + name: "edit", + args: { file_path: "packages/app/src/edit.ts", oldText: "old", newText: "new" }, + path: "packages/app/src/edit.ts", + }, + { + name: "write", + args: { path: "packages/app/src/write.ts", content: "new" }, + path: "packages/app/src/write.ts", + }, + ])("opens the raw file path from an expanded $name card", ({ name, args, path }) => { const container = document.createElement("div"); const onOpenWorkspaceFile = vi.fn(); render( renderToolCard( { - id: "msg:edit:open", - name: "edit", - args: { file_path: "packages/app/src/raw-name.ts", oldText: "old", newText: "new" }, + id: `msg:${name}:open`, + name, + args, completed: true, }, { @@ -304,9 +316,7 @@ describe("tool-cards", () => { ); expect(pathButton).toBeInstanceOf(HTMLButtonElement); pathButton!.click(); - expect(onOpenWorkspaceFile).toHaveBeenCalledWith({ - path: "packages/app/src/raw-name.ts", - }); + expect(onOpenWorkspaceFile).toHaveBeenCalledWith({ path }); }); it("keeps read offsets and limits visible in expanded args", () => {