diff --git a/bun.lock b/bun.lock index c13d79c29..18dd17158 100644 --- a/bun.lock +++ b/bun.lock @@ -799,7 +799,18 @@ "name": "@opencode-ai/session-ui", "version": "1.18.10", "dependencies": { + "@codemirror/lang-css": "6.3.1", + "@codemirror/lang-html": "6.4.12", + "@codemirror/lang-javascript": "6.2.5", + "@codemirror/lang-json": "6.0.2", + "@codemirror/lang-markdown": "6.5.2", + "@codemirror/lang-python": "6.2.1", + "@codemirror/language": "6.12.4", + "@codemirror/merge": "6.12.2", + "@codemirror/state": "6.7.1", + "@codemirror/view": "6.43.9", "@kobalte/core": "catalog:", + "@lezer/highlight": "1.2.3", "@opencode-ai/client": "file:../app/vendor/opencode-ai-client-1.17.13-v2.tgz", "@opencode-ai/core": "workspace:*", "@opencode-ai/sdk": "workspace:*", @@ -1480,6 +1491,30 @@ "@cloudflare/workers-types": ["@cloudflare/workers-types@4.20251008.0", "", {}, "sha512-dZLkO4PbCL0qcCSKzuW7KE4GYe49lI12LCfQ5y9XeSwgYBoAUbwH4gmJ6A0qUIURiTJTkGkRkhVPqpq2XNgYRA=="], + "@codemirror/autocomplete": ["@codemirror/autocomplete@6.20.3", "", { "dependencies": { "@codemirror/language": "^6.0.0", "@codemirror/state": "^6.0.0", "@codemirror/view": "^6.17.0", "@lezer/common": "^1.0.0" } }, "sha512-tlosUqb+3BbxCxZdu4tKeRghPFC+QM7q4X5YhKV2eCmPG+1r2F3f4AaSz5sCrFqUtX4Jh20VFTKecl16MgiV9g=="], + + "@codemirror/lang-css": ["@codemirror/lang-css@6.3.1", "", { "dependencies": { "@codemirror/autocomplete": "^6.0.0", "@codemirror/language": "^6.0.0", "@codemirror/state": "^6.0.0", "@lezer/common": "^1.0.2", "@lezer/css": "^1.1.7" } }, "sha512-kr5fwBGiGtmz6l0LSJIbno9QrifNMUusivHbnA1H6Dmqy4HZFte3UAICix1VuKo0lMPKQr2rqB+0BkKi/S3Ejg=="], + + "@codemirror/lang-html": ["@codemirror/lang-html@6.4.12", "", { "dependencies": { "@codemirror/autocomplete": "^6.0.0", "@codemirror/lang-css": "^6.0.0", "@codemirror/lang-javascript": "^6.0.0", "@codemirror/language": "^6.4.0", "@codemirror/state": "^6.0.0", "@codemirror/view": "^6.17.0", "@lezer/common": "^1.0.0", "@lezer/css": "^1.1.0", "@lezer/html": "^1.3.12" } }, "sha512-pw2ReWKUqSkbvh76RAT4NYxiogRu+PWkR2ukAwO9uOgrm8uipkzjtKKtNpyeAQwHOqxEeSvAXZ6vr3AfyB9y/w=="], + + "@codemirror/lang-javascript": ["@codemirror/lang-javascript@6.2.5", "", { "dependencies": { "@codemirror/autocomplete": "^6.0.0", "@codemirror/language": "^6.6.0", "@codemirror/lint": "^6.0.0", "@codemirror/state": "^6.0.0", "@codemirror/view": "^6.17.0", "@lezer/common": "^1.0.0", "@lezer/javascript": "^1.0.0" } }, "sha512-zD4e5mS+50htS7F+TYjBPsiIFGanfVqg4HyUz6WNFikgOPf2BgKlx+TQedI1w6n/IqRBVBbBWmGFdLB/7uxO4A=="], + + "@codemirror/lang-json": ["@codemirror/lang-json@6.0.2", "", { "dependencies": { "@codemirror/language": "^6.0.0", "@lezer/json": "^1.0.0" } }, "sha512-x2OtO+AvwEHrEwR0FyyPtfDUiloG3rnVTSZV1W8UteaLL8/MajQd8DpvUb2YVzC+/T18aSDv0H9mu+xw0EStoQ=="], + + "@codemirror/lang-markdown": ["@codemirror/lang-markdown@6.5.2", "", { "dependencies": { "@codemirror/autocomplete": "^6.7.1", "@codemirror/lang-html": "^6.0.0", "@codemirror/language": "^6.3.0", "@codemirror/state": "^6.0.0", "@codemirror/view": "^6.0.0", "@lezer/common": "^1.2.1", "@lezer/markdown": "^1.0.0" } }, "sha512-AwBOdkWYuA//WcM0xO5PfHPUcmz/O2i5o0Nsg1U69SII/loCJlFI1Romd9xp2HYb1kYJRGZotyqRghuHH5n8Kw=="], + + "@codemirror/lang-python": ["@codemirror/lang-python@6.2.1", "", { "dependencies": { "@codemirror/autocomplete": "^6.3.2", "@codemirror/language": "^6.8.0", "@codemirror/state": "^6.0.0", "@lezer/common": "^1.2.1", "@lezer/python": "^1.1.4" } }, "sha512-IRjC8RUBhn9mGR9ywecNhB51yePWCGgvHfY1lWN/Mrp3cKuHr0isDKia+9HnvhiWNnMpbGhWrkhuWOc09exRyw=="], + + "@codemirror/language": ["@codemirror/language@6.12.4", "", { "dependencies": { "@codemirror/state": "^6.0.0", "@codemirror/view": "^6.23.0", "@lezer/common": "^1.5.0", "@lezer/highlight": "^1.0.0", "@lezer/lr": "^1.0.0", "style-mod": "^4.0.0" } }, "sha512-1q4PaT+o6PbgpkJt4Q8Fv5XJxTy4FUZ4MWETtyiDw3J0Pyr9E2vqcKL+k9wcvjNTIsauxvE7OfmWj3FRPHQ76A=="], + + "@codemirror/lint": ["@codemirror/lint@6.9.7", "", { "dependencies": { "@codemirror/state": "^6.0.0", "@codemirror/view": "^6.42.0", "crelt": "^1.0.5" } }, "sha512-28/+iWLYxKxsvGYhSYL7zaCZqLz5+FFFDq9tVsvGv9kv8RY4fFAchJ5WX9M3YrrRlTIsECjsXPqeNgnSmNP2dg=="], + + "@codemirror/merge": ["@codemirror/merge@6.12.2", "", { "dependencies": { "@codemirror/language": "^6.0.0", "@codemirror/state": "^6.0.0", "@codemirror/view": "^6.17.0", "@lezer/highlight": "^1.0.0", "style-mod": "^4.1.0" } }, "sha512-V8JvyAPjHbPupqP7BeMcsdsYCbyPij74jxIbaIJDORI+VZzW44zFmon8bF+oxGWvOKhcRmkiUMXd8MxHr3YA2w=="], + + "@codemirror/state": ["@codemirror/state@6.7.1", "", { "dependencies": { "@marijn/find-cluster-break": "^1.0.0" } }, "sha512-9QzNDgE4EYDnAHfrTlR2lwiPciiOymLtwKK+8yHQzCc7GXhAP9xdEbEJFy2IWB1j9UGUl9BsgMmTo/ImA02T7A=="], + + "@codemirror/view": ["@codemirror/view@6.43.9", "", { "dependencies": { "@codemirror/state": "^6.7.0", "crelt": "^1.0.6", "style-mod": "^4.1.0", "w3c-keyname": "^2.2.4" } }, "sha512-sTuUzTpPMFebRhg6dawChoKKgndIwfjmJgKVxBefPElcU2NwQ6AFroupk0SFqEerQyZOGRfDNnSN8Dw/lMAsXw=="], + "@corvu/dialog": ["@corvu/dialog@0.2.4", "", { "dependencies": { "@corvu/utils": "~0.4.2", "solid-dismissible": "~0.1.1", "solid-focus-trap": "~0.1.8", "solid-presence": "~0.2.0", "solid-prevent-scroll": "~0.1.10" }, "peerDependencies": { "solid-js": "^1.8" } }, "sha512-n54vJq+fOy8GVrnYBdJpD6JXNuyx7LOeMrRxwzAvZnYGpW8+AA12tnb/P/2emJj/HjOO5otheGKb0breshdFlA=="], "@corvu/drawer": ["@corvu/drawer@0.2.4", "", { "dependencies": { "@corvu/dialog": "~0.2.4", "@corvu/utils": "~0.4.2", "@solid-primitives/memo": "^1.4.1", "solid-transition-size": "~0.1.4" }, "peerDependencies": { "solid-js": "^1.8" } }, "sha512-7jQoGZ8ROB9CmXam2nMY2wEskU3IoFwZQywkF/7vrBc/edGsPv7mOVQ1GN6G+4nd7nrMZ3UtqHAhmRh9V0azlw=="], @@ -1810,6 +1845,24 @@ "@leichtgewicht/ip-codec": ["@leichtgewicht/ip-codec@2.0.5", "", {}, "sha512-Vo+PSpZG2/fmgmiNzYK9qWRh8h/CHrwD0mo1h1DzL4yzHNSfWYujGTYsWGreD000gcgmZ7K4Ys6Tx9TxtsKdDw=="], + "@lezer/common": ["@lezer/common@1.5.2", "", {}, "sha512-sxQE460fPZyU3sdc8lafxiPwJHBzZRy/udNFynGQky1SePYBdhkBl1kOagA9uT3pxR8K09bOrmTUqA9wb/PjSQ=="], + + "@lezer/css": ["@lezer/css@1.3.6", "", { "dependencies": { "@lezer/common": "^1.2.0", "@lezer/highlight": "^1.0.0", "@lezer/lr": "^1.3.0" } }, "sha512-YJE78Wcg+zX8f10hiHWQ4Az48Qr/c13eId0VtRQYLBpxHDmDeSrXIlkbl+fJGW42rWC/uoUco9mhBZeVWP/A1g=="], + + "@lezer/highlight": ["@lezer/highlight@1.2.3", "", { "dependencies": { "@lezer/common": "^1.3.0" } }, "sha512-qXdH7UqTvGfdVBINrgKhDsVTJTxactNNxLk7+UMwZhU13lMHaOBlJe9Vqp907ya56Y3+ed2tlqzys7jDkTmW0g=="], + + "@lezer/html": ["@lezer/html@1.3.13", "", { "dependencies": { "@lezer/common": "^1.2.0", "@lezer/highlight": "^1.0.0", "@lezer/lr": "^1.0.0" } }, "sha512-oI7n6NJml729m7pjm9lvLvmXbdoMoi2f+1pwSDJkl9d68zGr7a9Btz8NdHTGQZtW2DA25ybeuv/SyDb9D5tseg=="], + + "@lezer/javascript": ["@lezer/javascript@1.5.4", "", { "dependencies": { "@lezer/common": "^1.2.0", "@lezer/highlight": "^1.1.3", "@lezer/lr": "^1.3.0" } }, "sha512-vvYx3MhWqeZtGPwDStM2dwgljd5smolYD2lR2UyFcHfxbBQebqx8yjmFmxtJ/E6nN6u1D9srOiVWm3Rb4tmcUA=="], + + "@lezer/json": ["@lezer/json@1.0.3", "", { "dependencies": { "@lezer/common": "^1.2.0", "@lezer/highlight": "^1.0.0", "@lezer/lr": "^1.0.0" } }, "sha512-BP9KzdF9Y35PDpv04r0VeSTKDeox5vVr3efE7eBbx3r4s3oNLfunchejZhjArmeieBH+nVOpgIiBJpEAv8ilqQ=="], + + "@lezer/lr": ["@lezer/lr@1.4.10", "", { "dependencies": { "@lezer/common": "^1.0.0" } }, "sha512-rnCpTIBafOx4mRp43xOxDJbFipJm/c0cia/V5TiGlhmMa+wsSdoGmUN3w5Bqrks/09Q/D4tNAmWaT8p6NRi77A=="], + + "@lezer/markdown": ["@lezer/markdown@1.7.2", "", { "dependencies": { "@lezer/common": "^1.5.0", "@lezer/highlight": "^1.0.0" } }, "sha512-iTkYvoVcKt3WkeL7qUDyXHONZEwLio4wj8KTNi2dnjQEXBZKMV63BpQrPqfsM+OkvuRbiSTAcycYAsQzLhRNoQ=="], + + "@lezer/python": ["@lezer/python@1.1.19", "", { "dependencies": { "@lezer/common": "^1.2.0", "@lezer/highlight": "^1.0.0", "@lezer/lr": "^1.0.0" } }, "sha512-MhQIURHRytsNzP/YXnqpYKW6la6voAH3kyplTOOiCdjyFY6cWWGFVmYVdHIPrElqSDf4iCDktQCockB9FxuhzQ=="], + "@lukeed/ms": ["@lukeed/ms@2.0.2", "", {}, "sha512-9I2Zn6+NJLfaGoz9jN3lpwDgAYvfGeNYdbAIjJOqzs4Tpc+VU3Jqq4IofSUBKajiDS8k9fZIg18/z13mpk1bsA=="], "@lydell/node-pty": ["@lydell/node-pty@1.2.0-beta.12", "", { "optionalDependencies": { "@lydell/node-pty-darwin-arm64": "1.2.0-beta.12", "@lydell/node-pty-darwin-x64": "1.2.0-beta.12", "@lydell/node-pty-linux-arm64": "1.2.0-beta.12", "@lydell/node-pty-linux-x64": "1.2.0-beta.12", "@lydell/node-pty-win32-arm64": "1.2.0-beta.12", "@lydell/node-pty-win32-x64": "1.2.0-beta.12" } }, "sha512-qIK890UwPupoj07osVvgOIa++1mxeHbcGry4PKRHhNVNs81V2SCG34eJr46GybiOmBtc8Sj5PB1/GGM5PL549g=="], @@ -1830,6 +1883,8 @@ "@malept/flatpak-bundler": ["@malept/flatpak-bundler@0.4.0", "", { "dependencies": { "debug": "^4.1.1", "fs-extra": "^9.0.0", "lodash": "^4.17.15", "tmp-promise": "^3.0.2" } }, "sha512-9QOtNffcOF/c1seMCDnjckb3R9WHcG34tky+FHpNKKCW0wc/scYLwMtO+ptyGUfMW0/b/n4qRiALlaFHc9Oj7Q=="], + "@marijn/find-cluster-break": ["@marijn/find-cluster-break@1.0.4", "", {}, "sha512-Wy0V7+SGUjnF9/TkiM1hKVDPj7jKXduPNboMVtHTA8dySMURWqfg/JZ9E2Sq8JgSJmkl7k7Qe9FLeMSrSraWmQ=="], + "@mdx-js/mdx": ["@mdx-js/mdx@3.1.1", "", { "dependencies": { "@types/estree": "^1.0.0", "@types/estree-jsx": "^1.0.0", "@types/hast": "^3.0.0", "@types/mdx": "^2.0.0", "acorn": "^8.0.0", "collapse-white-space": "^2.0.0", "devlop": "^1.0.0", "estree-util-is-identifier-name": "^3.0.0", "estree-util-scope": "^1.0.0", "estree-walker": "^3.0.0", "hast-util-to-jsx-runtime": "^2.0.0", "markdown-extensions": "^2.0.0", "recma-build-jsx": "^1.0.0", "recma-jsx": "^1.0.0", "recma-stringify": "^1.0.0", "rehype-recma": "^1.0.0", "remark-mdx": "^3.0.0", "remark-parse": "^11.0.0", "remark-rehype": "^11.0.0", "source-map": "^0.7.0", "unified": "^11.0.0", "unist-util-position-from-estree": "^2.0.0", "unist-util-stringify-position": "^4.0.0", "unist-util-visit": "^5.0.0", "vfile": "^6.0.0" } }, "sha512-f6ZO2ifpwAQIpzGWaBQT2TXxPv6z3RBzQKpVftEWN78Vl/YweF1uwussDx8ECAXVtr3Rs89fKyG9YlzUs9DyGQ=="], "@mdx-js/react": ["@mdx-js/react@3.1.1", "", { "dependencies": { "@types/mdx": "^2.0.0" }, "peerDependencies": { "@types/react": ">=16", "react": ">=16" } }, "sha512-f++rKLQgUVYDAtECQ6fn/is15GkEH9+nZPM3MS0RcxVqoTfawHvDlSCH7JbMhAM6uJ32v3eXLvLmLvjGu7PTQw=="], @@ -3442,6 +3497,8 @@ "crc32-stream": ["crc32-stream@6.0.0", "", { "dependencies": { "crc-32": "^1.2.0", "readable-stream": "^4.0.0" } }, "sha512-piICUB6ei4IlTv1+653yq5+KoqfBYmj9bw6LqXoOneTMDXk5nM1qt12mFW1caG3LlJXEKW1Bp0WggEmIfQB34g=="], + "crelt": ["crelt@1.0.7", "", {}, "sha512-aK6BbWfhf4U/wCcLHKPJl/xa6VkVstRaPywWtMKGwuOLc/wZTyQYuoxgvZnNsBvv7Kg3YTBQYYBCggcviQczuA=="], + "cross-dirname": ["cross-dirname@0.1.0", "", {}, "sha512-+R08/oI0nl3vfPcqftZRpytksBXDzOUveBq/NBVx0sUp1axwzPQrKinNx5yd5sxPu8j1wIy8AfnVQ+5eFdha6Q=="], "cross-fetch": ["cross-fetch@3.2.0", "", { "dependencies": { "node-fetch": "^2.7.0" } }, "sha512-Q+xVJLoGOeIMXZmbUK4HYk+69cQH6LudR0Vu/pRm2YlU/hDV9CiS0gKUMaWY5f2NeUH9C1nV3bsTlCo0FsTV1Q=="], @@ -5270,6 +5327,8 @@ "stubborn-utils": ["stubborn-utils@1.0.2", "", {}, "sha512-zOh9jPYI+xrNOyisSelgym4tolKTJCQd5GBhK0+0xJvcYDcwlOoxF/rnFKQ2KRZknXSG9jWAp66fwP6AxN9STg=="], + "style-mod": ["style-mod@4.1.3", "", {}, "sha512-i/n8VsZydrugj3Iuzll8+x/00GH2vnYsk1eomD8QiRrSAeW6ItbCQDtfXCeJHd0iwiNagqjQkvpvREEPtW3IoQ=="], + "style-to-js": ["style-to-js@1.1.21", "", { "dependencies": { "style-to-object": "1.0.14" } }, "sha512-RjQetxJrrUJLQPHbLku6U/ocGtzyjbJMP9lCNK7Ag0CNh690nSH8woqWH9u16nMjYBAok+i7JO1NP2pOy8IsPQ=="], "style-to-object": ["style-to-object@1.0.14", "", { "dependencies": { "inline-style-parser": "0.2.7" } }, "sha512-LIN7rULI0jBscWQYaSswptyderlarFkjQ+t79nzty8tcIAceVomEVlLzH5VP4Cmsv6MtKhs7qaAiwlcp+Mgaxw=="], @@ -5556,6 +5615,8 @@ "vscode-uri": ["vscode-uri@3.1.0", "", {}, "sha512-/BpdSx+yCQGnCvecbyXdxHDkuk55/G3xwnC0GqY4gmQ3j+A+g8kzzgB4Nk/SINjqn6+waqw3EgbVF2QKExkRxQ=="], + "w3c-keyname": ["w3c-keyname@2.2.8", "", {}, "sha512-dpojBhNsCNN7T82Tm7k26A6G9ML3NkhDsnw9n/eoxSRlVBB4CEtIQ/KTCLI2Fwf3ataSXRhYFkQi3SlnFwPvPQ=="], + "walk-up-path": ["walk-up-path@4.0.0", "", {}, "sha512-3hu+tD8YzSLGuFYtPRb48vdhKMi0KQV5sn+uWr8+7dMEq/2G/dtLrdDinkLjqq5TIbIBjYJ4Ax/n3YiaW7QM8A=="], "web-namespaces": ["web-namespaces@2.0.1", "", {}, "sha512-bKr1DkiNa2krS7qxNtdrtHAmzuYGFQLiQ13TsorsdT6ULTkPLKuu5+GsFpDlg6JFjUTwX2DyhMPG2be8uPrqsQ=="], diff --git a/packages/app/src/context/layout.tsx b/packages/app/src/context/layout.tsx index 3dcc11e19..2a17460fe 100644 --- a/packages/app/src/context/layout.tsx +++ b/packages/app/src/context/layout.tsx @@ -89,7 +89,7 @@ type TabHandoff = { export type LocalProject = Partial & { worktree: string; expanded: boolean } export type HomeProjectSelection = { server: ServerConnection.Key; directory?: string } -export type ReviewDiffStyle = "unified" | "split" +export type ReviewDiffStyle = "unified" | "split" | "preview" export type ReviewChangeMode = "git" | "branch" | "turn" export type ReviewPanelSource = "context-button" | "other" diff --git a/packages/app/src/context/server-session.ts b/packages/app/src/context/server-session.ts index 40093c6ff..26e6e6ae0 100644 --- a/packages/app/src/context/server-session.ts +++ b/packages/app/src/context/server-session.ts @@ -28,6 +28,7 @@ type MessageApi = ServerApi["message"] const cmp = (a: string, b: string) => (a < b ? -1 : a > b ? 1 : 0) const cmpMessage = (a: Message, b: Message) => a.time.created - b.time.created || cmp(a.id, b.id) const SKIP_PARTS = new Set(["patch", "step-start", "step-finish"]) +const EDIT_TOOLS = new Set(["edit", "write", "patch", "apply_patch"]) const initialMessagePageSize = 20 const historyMessagePageSize = 200 const sessionInfoLimit = 2_048 @@ -1149,6 +1150,17 @@ export function createServerSession( next.splice(result.index, 0, part) return next }) + // Bump diff_version when a file-editing tool completes so the diff + // query refetches mid-turn. The equivalent logic in event-reducer.ts + // is unreachable because the SSE path skips SESSION_CONTENT_EVENTS. + if ( + part.type === "tool" && + EDIT_TOOLS.has(part.tool) && + part.state.status === "completed" && + (part.state as { metadata?: Record }).metadata?.filediff + ) { + setData("diff_version", part.sessionID, (v = 0) => v + 1) + } return } case "message.part.removed": { diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx index 3334c7532..f276dc1a1 100644 --- a/packages/app/src/pages/session.tsx +++ b/packages/app/src/pages/session.tsx @@ -88,7 +88,7 @@ import { sessionPanelLayout } from "@/pages/session/session-panel-layout" import { SessionReviewEmptyChangesV2 } from "@opencode-ai/session-ui/v2/session-review-empty-changes-v2" import { ReviewPanelV2 } from "@/pages/session/v2/review-panel-v2" import { createReviewPanelV2State } from "@/pages/session/v2/review-panel-v2-state" -import { accumulateDiffs } from "@/pages/session/v2/accumulate-diffs" +import { accumulateDiffs, applyRenames, mergeServerAndToolDiffs, toHomePath } from "@/pages/session/v2/accumulate-diffs" import { TerminalPanel } from "@/pages/session/terminal-panel" import { TerminalPanelV2 } from "@/pages/session/terminal-panel-v2" import { useComposerCommands } from "@/pages/session/use-composer-commands" @@ -681,6 +681,47 @@ export default function Page() { const mobileChanges = createMemo(() => !isDesktop() && store.mobileTab === "changes") const EDIT_TOOLS = new Set(["edit", "write", "patch", "apply_patch"]) + + // --- File rename tracking (sidebar move/rename → Files Changed update) --- + // When the sidebar moves or renames a file, the extension posts a + // file-op-notify message. We track the rename so the tool-metadata diffs + // (which still have the old path) are displayed at the new location. + const [fileRenames, setFileRenames] = createSignal(new Map()) + const onFileOpNotify = (e: MessageEvent) => { + const d = e.data as { source?: string; kind?: string; op?: string; oldPath?: string; newPath?: string; home?: string } | undefined + if (d?.source !== "amicode" || d?.kind !== "file-op-notify") return + if ((d.op === "move" || d.op === "rename") && d.oldPath && d.newPath) { + // Use the SAME home as the reviewDiffs memo (globalThis.process.env.HOME, + // which is undefined in the browser). This ensures rename map keys match + // tool-diff paths — both end up as absolute paths in the browser iframe. + // Do NOT use d.home here: that produces ~/... keys while the memo produces + // /Users/... paths, and they never match. + const dir = sdk().directory + const home = typeof globalThis.process !== "undefined" ? globalThis.process.env?.HOME : undefined + const prefix = home && dir.startsWith(home) ? "~" + dir.slice(home.length) : dir + const oldNorm = toHomePath(d.oldPath, home, prefix) + const newNorm = toHomePath(d.newPath, home, prefix) + setFileRenames((prev) => { + const next = new Map(prev) + // Chain resolution: if anything pointed to oldPath, update it to newPath + for (const [k, v] of next) { + if (v === oldNorm) next.set(k, newNorm) + } + next.set(oldNorm, newNorm) + return next + }) + // Force a server refetch so in-project files at their new location + // appear immediately — without this, the stale server response (which + // doesn't include the file) persists until the file watcher's 1s debounce. + const sessionID = params.id + if (sessionID) { + sync().set("diff_version", sessionID, (v: number | undefined) => (v ?? 0) + 1) + } + } + } + window.addEventListener("message", onFileOpNotify) + onCleanup(() => window.removeEventListener("message", onFileOpNotify)) + // Refetch when the session transitions to idle (assistant finished, snapshot taken) // or when a file-editing tool completes mid-turn (diff_version bumps) const sessionDiffVersion = () => { @@ -706,35 +747,16 @@ export default function Page() { } }) const reviewDiffs = createMemo(() => { - // Server endpoint returns the authoritative full-session diff (queries all messages). - const serverDiffs = sessionDiffQuery.data ?? [] - // Once the server has responded at least once, trust it — even if it returned []. - // The client-side fallback is only for the initial load before any server response, - // never during refetches (where keepPreviousData already preserves the last result). - const serverResponded = sessionDiffQuery.status === "success" || sessionDiffQuery.isPlaceholderData - if (serverDiffs.length > 0 || serverResponded) { - // Server paths are relative to the project root — prefix with ~/project-path - const dir = sdk().directory - const home = typeof globalThis.process !== "undefined" ? globalThis.process.env?.HOME : undefined - const prefix = home && dir.startsWith(home) ? "~" + dir.slice(home.length) : dir - return serverDiffs - .filter((d): d is SnapshotFileDiff & { file: string } => !!d.file) - .map((d) => ({ ...d, file: d.file.startsWith("/") || d.file.startsWith("~/") ? d.file : `${prefix}/${d.file}` })) - } - // Fallback: derive from tool parts currently loaded in the client. - // This shows immediate results for visible messages while the server query loads. - const allMessages = messages() - if (!allMessages.length) return [] as Array + // Shared path normalization — both sources must use the same function for dedup to work. const dir = sdk().directory const home = typeof globalThis.process !== "undefined" ? globalThis.process.env?.HOME : undefined const prefix = home && dir.startsWith(home) ? "~" + dir.slice(home.length) : dir - const toHomePath = (p: string) => { - if (p.startsWith("~/")) return p - if (home && p.startsWith(home)) return "~" + p.slice(home.length) - if (!p.startsWith("/")) return `${prefix}/${p}` - return p - } + + // --- Tool-metadata diffs (always computed, not just as fallback) --- + // Scraped from completed edit/write/patch/apply_patch tool parts across all messages. + // These see every file the agent touched regardless of directory. const editParts: import("@/pages/session/v2/accumulate-diffs").ToolEditPart[] = [] + const allMessages = messages() for (const msg of allMessages) { const msgParts = sync().data.part[msg.id] if (!msgParts) continue @@ -747,7 +769,7 @@ export default function Page() { const rawTitle = (part.state as { title?: string }).title || filediff.file editParts.push({ file: filediff.file, - title: toHomePath(rawTitle), + title: toHomePath(rawTitle, home, prefix), patch: filediff.patch, additions: filediff.additions ?? 0, deletions: filediff.deletions ?? 0, @@ -755,7 +777,19 @@ export default function Page() { } } } - return accumulateDiffs(editParts) + const toolDiffs = applyRenames(accumulateDiffs(editParts), fileRenames()) + + // --- Server diffs (authoritative for in-project files) --- + const serverDiffs = sessionDiffQuery.data ?? [] + const serverResponded = sessionDiffQuery.status === "success" || sessionDiffQuery.isPlaceholderData + + return mergeServerAndToolDiffs({ + serverDiffs, + toolDiffs, + serverResponded, + directory: dir, + home, + }) }) // All files touched by edit tools in this session — fetched from the server @@ -1318,7 +1352,14 @@ export default function Page() { return layout.review.diffStyle() }, onDiffStyleChange: layout.review.setDiffStyle, + get serverUrl() { + return serverSDK().url + }, state: reviewV2State, + get isAgentBusy() { + const id = params.id + return id ? sync().data.session_working(id) : false + }, onRefresh: () => { const id = params.id if (id) sync().set("diff_version", id, (v: number | undefined) => (v ?? 0) + 1) diff --git a/packages/app/src/pages/session/review-tab.tsx b/packages/app/src/pages/session/review-tab.tsx index 1b65af712..5e4854e34 100644 --- a/packages/app/src/pages/session/review-tab.tsx +++ b/packages/app/src/pages/session/review-tab.tsx @@ -7,13 +7,14 @@ import type { SessionReviewCommentActions, SessionReviewCommentDelete, SessionReviewCommentUpdate, + SessionReviewDiffStyle, } from "@opencode-ai/session-ui/session-review" import type { SelectedLineRange } from "@/context/file" import { useSDK } from "@/context/sdk" import { useLayout } from "@/context/layout" import type { LineComment } from "@/context/comments" -export type DiffStyle = "unified" | "split" +export type DiffStyle = SessionReviewDiffStyle type ReviewDiff = FileDiffInfo | SnapshotFileDiff | VcsFileDiff diff --git a/packages/app/src/pages/session/v2/accumulate-diffs.test.ts b/packages/app/src/pages/session/v2/accumulate-diffs.test.ts index cec265861..667a9a85c 100644 --- a/packages/app/src/pages/session/v2/accumulate-diffs.test.ts +++ b/packages/app/src/pages/session/v2/accumulate-diffs.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test" -import { accumulateDiffs, type ToolEditPart } from "./accumulate-diffs" +import { accumulateDiffs, applyRenames, mergeServerAndToolDiffs, type ToolEditPart } from "./accumulate-diffs" const edit = (file: string, overrides: Partial = {}): ToolEditPart => ({ file, @@ -112,3 +112,259 @@ describe("accumulateDiffs", () => { // on the query so the fallback never fires while server data exists. }) }) + +// --- mergeServerAndToolDiffs --- + +const HOME = "/Users/jj" +const DIR = "/Users/jj/harmoniqs/amicode" +const diff = ( + file: string, + patch = "mock", + additions = 1, + deletions = 0, + status: "added" | "modified" | "deleted" = "modified", +) => ({ file, patch, additions, deletions, status }) + +describe("mergeServerAndToolDiffs", () => { + test("cross-project tool diffs pass through when server has responded", () => { + const result = mergeServerAndToolDiffs({ + serverDiffs: [diff("src/foo.ts")], + toolDiffs: [diff("~/other-project/bar.ts")], + serverResponded: true, + directory: DIR, + home: HOME, + }) + const files = result.map((d) => d.file) + expect(files).toContain("~/harmoniqs/amicode/src/foo.ts") + expect(files).toContain("~/other-project/bar.ts") + }) + + test("in-project files come from the server, not tool metadata (server wins)", () => { + const result = mergeServerAndToolDiffs({ + serverDiffs: [diff("src/foo.ts", "server-patch", 10, 5)], + toolDiffs: [diff("~/harmoniqs/amicode/src/foo.ts", "tool-patch", 3, 1)], + serverResponded: true, + directory: DIR, + home: HOME, + }) + const entry = result.find((d) => d.file.endsWith("src/foo.ts")) + expect(entry).toBeDefined() + expect(entry!.patch).toBe("server-patch") + expect(entry!.additions).toBe(10) + }) + + test("in-project file absent from server (created+deleted) is excluded, not leaked as cross-project", () => { + // The bug: file was created by the write tool, then manually deleted. + // Server correctly has no diff (net zero). Tool metadata still has it. + // The old code leaked it through the cross-project filter. + const result = mergeServerAndToolDiffs({ + serverDiffs: [], + toolDiffs: [diff("~/harmoniqs/amicode/files-changed-test.md", "stale-patch", 3, 0, "added")], + serverResponded: true, + directory: DIR, + home: HOME, + }) + expect(result).toHaveLength(0) + }) + + test("in-project file with relative path absent from server is also excluded", () => { + const result = mergeServerAndToolDiffs({ + serverDiffs: [], + toolDiffs: [diff("~/harmoniqs/amicode/src/temp.ts")], + serverResponded: true, + directory: DIR, + home: HOME, + }) + expect(result).toHaveLength(0) + }) + + test("fallback: all tool diffs shown when server has not responded", () => { + const result = mergeServerAndToolDiffs({ + serverDiffs: [], + toolDiffs: [ + diff("~/harmoniqs/amicode/src/foo.ts"), + diff("~/other/bar.ts"), + ], + serverResponded: false, + directory: DIR, + home: HOME, + }) + expect(result).toHaveLength(2) + }) + + test("dedup by normalized path — server wins on conflict", () => { + const result = mergeServerAndToolDiffs({ + serverDiffs: [diff("/Users/jj/harmoniqs/amicode/src/foo.ts")], + toolDiffs: [diff("~/harmoniqs/amicode/src/foo.ts")], + serverResponded: true, + directory: DIR, + home: HOME, + }) + expect(result).toHaveLength(1) + }) +}) + +// --- applyRenames --- + +describe("applyRenames", () => { + test("renames a file path when it appears in the rename map", () => { + const diffs = [diff("~/harmoniqs/amicode/test.md")] + const renames = new Map([["~/harmoniqs/amicode/test.md", "~/harmoniqs/opencode/test.md"]]) + const result = applyRenames(diffs, renames) + expect(result).toHaveLength(1) + expect(result[0].file).toBe("~/harmoniqs/opencode/test.md") + }) + + test("leaves files unchanged when not in the rename map", () => { + const diffs = [diff("~/harmoniqs/amicode/src/foo.ts")] + const renames = new Map([["~/harmoniqs/amicode/test.md", "~/harmoniqs/opencode/test.md"]]) + const result = applyRenames(diffs, renames) + expect(result[0].file).toBe("~/harmoniqs/amicode/src/foo.ts") + }) + + test("returns diffs unchanged when rename map is empty", () => { + const diffs = [diff("~/harmoniqs/amicode/test.md"), diff("~/other/bar.ts")] + const result = applyRenames(diffs, new Map()) + expect(result).toEqual(diffs) + }) + + test("preserves all other diff fields (patch, additions, deletions, status)", () => { + const diffs = [diff("~/old/path.ts", "the-patch", 5, 3, "modified")] + const renames = new Map([["~/old/path.ts", "~/new/path.ts"]]) + const result = applyRenames(diffs, renames) + expect(result[0].file).toBe("~/new/path.ts") + expect(result[0].patch).toBe("the-patch") + expect(result[0].additions).toBe(5) + expect(result[0].deletions).toBe(3) + expect(result[0].status).toBe("modified") + }) + + test("handles multiple renames in one pass", () => { + const diffs = [diff("~/a/one.ts"), diff("~/b/two.ts")] + const renames = new Map([ + ["~/a/one.ts", "~/c/one.ts"], + ["~/b/two.ts", "~/d/two.ts"], + ]) + const result = applyRenames(diffs, renames) + expect(result[0].file).toBe("~/c/one.ts") + expect(result[1].file).toBe("~/d/two.ts") + }) +}) + +// --- move-back-to-project: full round-trip scenario --- + +describe("round-trip move: project → cross-project → back to project", () => { + test("file moved out then back is visible when server includes it after refetch", () => { + // Step 1: file created in-project, server has it + const step1 = mergeServerAndToolDiffs({ + serverDiffs: [diff("~/harmoniqs/amicode/test.md", "patch", 2, 0, "added")], + toolDiffs: [diff("~/harmoniqs/amicode/test.md", "patch", 2, 0, "added")], + serverResponded: true, + directory: DIR, + home: HOME, + }) + expect(step1).toHaveLength(1) + expect(step1[0].file).toBe("~/harmoniqs/amicode/test.md") + + // Step 2: file moved to opencode — rename map transforms tool diff + const renames1 = new Map([["~/harmoniqs/amicode/test.md", "~/harmoniqs/opencode/test.md"]]) + const toolDiffsStep2 = applyRenames( + [diff("~/harmoniqs/amicode/test.md", "patch", 2, 0, "added")], + renames1, + ) + const step2 = mergeServerAndToolDiffs({ + serverDiffs: [], // server no longer has it (moved away) + toolDiffs: toolDiffsStep2, + serverResponded: true, + directory: DIR, + home: HOME, + }) + expect(step2).toHaveLength(1) + expect(step2[0].file).toBe("~/harmoniqs/opencode/test.md") + + // Step 3: file moved back — rename chain resolves to original path + // After chain resolution: amicode/test.md → amicode/test.md (identity) + const renames2 = new Map([ + ["~/harmoniqs/amicode/test.md", "~/harmoniqs/amicode/test.md"], + ["~/harmoniqs/opencode/test.md", "~/harmoniqs/amicode/test.md"], + ]) + const toolDiffsStep3 = applyRenames( + [diff("~/harmoniqs/amicode/test.md", "patch", 2, 0, "added")], + renames2, + ) + // Tool diff is back at the in-project path + expect(toolDiffsStep3[0].file).toBe("~/harmoniqs/amicode/test.md") + + // With server refetch (diff_version bumped), server includes the file again + const step3 = mergeServerAndToolDiffs({ + serverDiffs: [diff("~/harmoniqs/amicode/test.md", "patch", 2, 0, "added")], + toolDiffs: toolDiffsStep3, + serverResponded: true, + directory: DIR, + home: HOME, + }) + expect(step3).toHaveLength(1) + expect(step3[0].file).toBe("~/harmoniqs/amicode/test.md") + }) + + test("file moved back DISAPPEARS when server has NOT refetched (the bug this fix addresses)", () => { + // Rename map resolves to in-project, but server still has stale empty response + const renames = new Map([ + ["~/harmoniqs/amicode/test.md", "~/harmoniqs/amicode/test.md"], + ]) + const toolDiffs = applyRenames( + [diff("~/harmoniqs/amicode/test.md", "patch", 2, 0, "added")], + renames, + ) + // Server hasn't refetched — stale empty response + const result = mergeServerAndToolDiffs({ + serverDiffs: [], + toolDiffs, + serverResponded: true, + directory: DIR, + home: HOME, + }) + // Without the diff_version bump, the file is gone (server is authoritative for in-project) + expect(result).toHaveLength(0) + }) +}) + +// --------------------------------------------------------------------------- +// latchWhileBusy — prevent brief falsy gaps from unmounting components +// --------------------------------------------------------------------------- + +describe("latchWhileBusy", () => { + // Pure-function version of the latching memo used in ReviewPanelV2. + // The real code uses createMemo with a prev accumulator; we test the + // underlying logic here as a plain function. + function latchWhileBusy(current: T | undefined, prev: T | undefined, isBusy: boolean): T | undefined { + if (!current && prev && isBusy) return prev + return current + } + + test("returns current value when it is truthy", () => { + expect(latchWhileBusy("foo.ts", undefined, true)).toBe("foo.ts") + expect(latchWhileBusy("foo.ts", "bar.ts", true)).toBe("foo.ts") + expect(latchWhileBusy("foo.ts", undefined, false)).toBe("foo.ts") + }) + + test("latches previous value when current goes falsy and agent is busy", () => { + expect(latchWhileBusy(undefined, "foo.ts", true)).toBe("foo.ts") + }) + + test("does NOT latch when agent is idle (allows real empty state)", () => { + expect(latchWhileBusy(undefined, "foo.ts", false)).toBeUndefined() + }) + + test("does NOT latch when there is no previous value", () => { + expect(latchWhileBusy(undefined, undefined, true)).toBeUndefined() + }) + + test("latches object values the same way", () => { + const prevDiff = { file: "foo.ts", additions: 5, deletions: 2 } + expect(latchWhileBusy(undefined, prevDiff, true)).toBe(prevDiff) + expect(latchWhileBusy(undefined, prevDiff, false)).toBeUndefined() + const newDiff = { file: "foo.ts", additions: 6, deletions: 3 } + expect(latchWhileBusy(newDiff, prevDiff, true)).toBe(newDiff) + }) +}) diff --git a/packages/app/src/pages/session/v2/accumulate-diffs.ts b/packages/app/src/pages/session/v2/accumulate-diffs.ts index 8e069fb7e..22d4e1ef8 100644 --- a/packages/app/src/pages/session/v2/accumulate-diffs.ts +++ b/packages/app/src/pages/session/v2/accumulate-diffs.ts @@ -8,6 +8,61 @@ export type ToolEditPart = { deletions: number } +/** + * Normalize a file path to ~/... form for consistent dedup across sources. + */ +export function toHomePath(p: string, home: string | undefined, prefix: string): string { + if (p.startsWith("~/")) return p + if (home && p.startsWith(home)) return "~" + p.slice(home.length) + if (!p.startsWith("/")) return `${prefix}/${p}` + return p +} + +export interface MergeOpts { + serverDiffs: SnapshotFileDiff[] + toolDiffs: Array + serverResponded: boolean + directory: string + home: string | undefined +} + +/** + * Merge server shadow-git diffs with tool-metadata diffs. + * + * Server diffs are authoritative for in-project files. Tool-metadata diffs + * fill in cross-project files (outside the project directory) not covered + * by the server. In-project tool-metadata diffs that the server excluded + * (e.g. created + deleted = net zero) are dropped — the server's absence + * is the authority. + * + * When serverResponded is false (initial load), all tool-metadata diffs + * are returned as a fallback. + */ +export function mergeServerAndToolDiffs(opts: MergeOpts): Array { + const { serverDiffs, toolDiffs, serverResponded, directory, home } = opts + const prefix = home && directory.startsWith(home) ? "~" + directory.slice(home.length) : directory + const projectPrefix = prefix + "/" + + if (serverDiffs.length > 0 || serverResponded) { + const normalizedServerDiffs = serverDiffs + .filter((d): d is SnapshotFileDiff & { file: string } => !!d.file) + .map((d) => ({ ...d, file: toHomePath(d.file, home, prefix) })) + + const serverFiles = new Set(normalizedServerDiffs.map((d) => d.file)) + // Only pass through tool-metadata diffs that are BOTH absent from the + // server set AND outside the project directory. In-project files trust + // the server's authority — if the server excluded them (created + deleted, + // or reverted), they should not leak through as phantom entries. + const crossProjectDiffs = toolDiffs.filter( + (d) => !serverFiles.has(d.file) && !d.file.startsWith(projectPrefix), + ) + + return [...normalizedServerDiffs, ...crossProjectDiffs] + } + + return toolDiffs +} + /** * Accumulate file diffs from tool edit parts across a session. * @@ -62,3 +117,20 @@ export function accumulateDiffs(parts: ToolEditPart[]): Array, + renames: Map, +): Array { + if (renames.size === 0) return diffs + return diffs.map((d) => { + const newPath = renames.get(d.file) + return newPath ? { ...d, file: newPath } : d + }) +} diff --git a/packages/app/src/pages/session/v2/review-panel-v2.tsx b/packages/app/src/pages/session/v2/review-panel-v2.tsx index a10099bbc..b865347ad 100644 --- a/packages/app/src/pages/session/v2/review-panel-v2.tsx +++ b/packages/app/src/pages/session/v2/review-panel-v2.tsx @@ -7,7 +7,7 @@ import { SessionReviewV2, SessionReviewV2Sidebar, } from "@opencode-ai/session-ui/v2/session-review-v2" -import { SessionReviewFilePreviewV2 } from "@opencode-ai/session-ui/v2/session-review-file-preview-v2" +import { SessionReviewFilePreviewV2, type SessionReviewFilePreviewV2Props } from "@opencode-ai/session-ui/v2/session-review-file-preview-v2" import { DiffChanges } from "@opencode-ai/ui/v2/diff-changes-v2" import { FileIcon } from "@opencode-ai/ui/file-icon" import type { @@ -45,7 +45,9 @@ export type ReviewPanelV2Props = { onSelectFile: (path: string) => void diffStyle: SessionReviewDiffStyle onDiffStyleChange?: (style: SessionReviewDiffStyle) => void + serverUrl?: string state: ReviewPanelV2State + isAgentBusy?: boolean onRefresh?: () => void onLineComment?: (comment: SessionReviewLineComment) => void onLineCommentUpdate?: (comment: SessionReviewCommentUpdate) => void @@ -79,6 +81,12 @@ export function ReviewPanelV2(props: ReviewPanelV2Props) { if (searching()) return active const files = filteredFiles() if (active && files.includes(active)) return active + // During agent turns, keep the user's selection even when the file has + // briefly dropped from the diff list (the ~1s blind window between tool + // completion and server diff refetch). Without this, activeDiff falls + // through to files[0], the keyed sees a different key, and the + // preview component remounts — losing scroll position and editor state. + if (active && props.isAgentBusy) return active return files[0] }) const sourceActiveItem = createMemo(() => diffs().find((diff) => diff.file === activeDiff())) @@ -102,6 +110,22 @@ export function ReviewPanelV2(props: ReviewPanelV2Props) { return source }) + // Latch the active file and diff during agent turns so the wrappers + // don't unmount SessionReviewFilePreviewV2 during the ~1s blind window + // between a tool completion and the server diff refetch. Without this, + // the component is destroyed and recreated, losing scroll position and + // editor state every time the agent edits a file. + const stableActiveDiff = createMemo((prev) => { + const current = activeDiff() + if (!current && prev && props.isAgentBusy) return prev + return current + }) + const stableActiveItem = createMemo((prev) => { + const current = activeItem() + if (!current && prev && props.isAgentBusy) return prev + return current + }) + const readFile = async (path: string) => sdk() .client.file.read({ path }) @@ -132,7 +156,7 @@ export function ReviewPanelV2(props: ReviewPanelV2Props) { activeDiff={activeDiff} /> } - activeFile={activeDiff()} + activeFile={stableActiveDiff()} files={filteredFiles()} onSelectFile={props.onSelectFile} diffStyle={props.diffStyle} @@ -143,16 +167,21 @@ export function ReviewPanelV2(props: ReviewPanelV2Props) { preview={ // Key on the file path, not the diff object identity, so refreshed diff data // updates the mounted preview instead of remounting the whole viewer. - + // Use stable (latched) values so the component survives brief falsy + // gaps during the agent's turn (the ~1s blind window between tool + // completion and server diff refetch). + {(file) => ( - + {(diff) => ( { const files = filteredFiles() diff --git a/packages/opencode/src/tool/write.ts b/packages/opencode/src/tool/write.ts index 37be6d8c4..58d23e0f4 100644 --- a/packages/opencode/src/tool/write.ts +++ b/packages/opencode/src/tool/write.ts @@ -3,7 +3,7 @@ import * as path from "path" import { Effect } from "effect" import * as Tool from "./tool" import { LSP } from "@/lsp/lsp" -import { createTwoFilesPatch } from "diff" +import { createTwoFilesPatch, diffLines } from "diff" import DESCRIPTION from "./write.txt" import { EventV2Bridge } from "@/event-v2-bridge" import { FileSystem } from "@opencode-ai/core/filesystem" @@ -11,6 +11,7 @@ import { Watcher } from "@opencode-ai/core/filesystem/watcher" import { Format } from "../format" import { FSUtil } from "@opencode-ai/core/fs-util" import { InstanceState } from "@/effect/instance-state" +import { Snapshot } from "@/snapshot" import { trimDiff } from "./edit" import { assertExternalDirectoryEffect } from "./external-directory" import * as Bom from "@/util/bom" @@ -61,6 +62,29 @@ export const WriteTool = Tool.define( }, }) + // Compute filediff for the Files Changed panel (same pattern as edit.ts) + let additions = 0 + let deletions = 0 + for (const change of diffLines(contentOld, contentNew)) { + if (change.added) additions += change.count || 0 + if (change.removed) deletions += change.count || 0 + } + const filediff: Snapshot.FileDiff = { + file: filepath, + patch: diff, + additions, + deletions, + } + + // Push filediff mid-execution so the UI updates before the tool completes + yield* ctx.metadata({ + metadata: { + diff, + filediff, + diagnostics: {}, + }, + }) + yield* fs.writeWithDirs(filepath, Bom.join(contentNew, desiredBom)) if (yield* format.file(filepath)) { yield* Bom.syncFile(fs, filepath, desiredBom) @@ -93,6 +117,8 @@ export const WriteTool = Tool.define( title: path.relative(instance.worktree, filepath), metadata: { diagnostics, + diff, + filediff, filepath, exists: exists, }, diff --git a/packages/opencode/test/tool/write.test.ts b/packages/opencode/test/tool/write.test.ts index 4f897dc87..c797a1ea8 100644 --- a/packages/opencode/test/tool/write.test.ts +++ b/packages/opencode/test/tool/write.test.ts @@ -27,6 +27,21 @@ const ctx = { ask: () => Effect.void, } +// A ctx variant that captures metadata calls for filediff assertions +function ctxWithMetadataCapture() { + const calls: Array> = [] + return { + ctx: { + ...ctx, + metadata: (arg: { metadata: Record }) => { + calls.push(arg.metadata) + return Effect.void + }, + }, + calls, + } +} + afterEach(async () => { await disposeAllInstances() }) @@ -170,6 +185,50 @@ describe("tool.write", () => { expect(result.metadata).toHaveProperty("exists", true) }), ) + + it.instance("returns filediff metadata with file, patch, additions, deletions when overwriting", () => + Effect.gen(function* () { + const test = yield* TestInstance + const filepath = path.join(test.directory, "filediff-test.txt") + yield* Effect.promise(() => fs.writeFile(filepath, "line1\nline2\n", "utf-8")) + const result = yield* run({ filePath: filepath, content: "line1\nline2\nline3\n" }) + + const filediff = (result.metadata as any).filediff + expect(filediff).toBeDefined() + expect(filediff.file).toBe(filepath) + expect(filediff.patch).toBeDefined() + expect(filediff.additions).toBe(1) + expect(filediff.deletions).toBe(0) + }), + ) + + it.instance("returns filediff metadata for new file creation", () => + Effect.gen(function* () { + const test = yield* TestInstance + const filepath = path.join(test.directory, "brand-new.txt") + const result = yield* run({ filePath: filepath, content: "hello\nworld\n" }) + + const filediff = (result.metadata as any).filediff + expect(filediff).toBeDefined() + expect(filediff.file).toBe(filepath) + expect(filediff.additions).toBe(2) + expect(filediff.deletions).toBe(0) + }), + ) + + it.instance("pushes filediff via ctx.metadata() mid-execution so the UI updates before completion", () => + Effect.gen(function* () { + const test = yield* TestInstance + const filepath = path.join(test.directory, "mid-push.txt") + yield* Effect.promise(() => fs.writeFile(filepath, "old\n", "utf-8")) + const capture = ctxWithMetadataCapture() + yield* run({ filePath: filepath, content: "new\n" }, capture.ctx) + + const filediffPush = capture.calls.find((c) => c.filediff) + expect(filediffPush).toBeDefined() + expect((filediffPush!.filediff as any).file).toBe(filepath) + }), + ) }) describe("file permissions", () => { diff --git a/packages/session-ui/package.json b/packages/session-ui/package.json index 04a4f1586..6672f469d 100644 --- a/packages/session-ui/package.json +++ b/packages/session-ui/package.json @@ -41,7 +41,18 @@ "vite": "catalog:" }, "dependencies": { + "@codemirror/lang-css": "6.3.1", + "@codemirror/lang-html": "6.4.12", + "@codemirror/lang-javascript": "6.2.5", + "@codemirror/lang-json": "6.0.2", + "@codemirror/lang-markdown": "6.5.2", + "@codemirror/lang-python": "6.2.1", + "@codemirror/language": "6.12.4", + "@codemirror/merge": "6.12.2", + "@codemirror/state": "6.7.1", + "@codemirror/view": "6.43.9", "@kobalte/core": "catalog:", + "@lezer/highlight": "1.2.3", "@opencode-ai/client": "file:../app/vendor/opencode-ai-client-1.17.13-v2.tgz", "@opencode-ai/core": "workspace:*", "@opencode-ai/sdk": "workspace:*", diff --git a/packages/session-ui/src/components/session-review.tsx b/packages/session-ui/src/components/session-review.tsx index 1585a8aa3..2146aaa77 100644 --- a/packages/session-ui/src/components/session-review.tsx +++ b/packages/session-ui/src/components/session-review.tsx @@ -29,7 +29,7 @@ import { normalize, text, type ViewDiff } from "./session-diff" const MAX_DIFF_CHANGED_LINES = 500 const REVIEW_MOUNT_MARGIN = 300 -export type SessionReviewDiffStyle = "unified" | "split" +export type SessionReviewDiffStyle = "unified" | "split" | "preview" export type SessionReviewComment = { id: string diff --git a/packages/session-ui/src/v2/components/diff-gutter-extension.test.ts b/packages/session-ui/src/v2/components/diff-gutter-extension.test.ts new file mode 100644 index 000000000..6451e9e2e --- /dev/null +++ b/packages/session-ui/src/v2/components/diff-gutter-extension.test.ts @@ -0,0 +1,241 @@ +import { describe, expect, test, beforeEach, afterEach } from "bun:test" +import { EditorState } from "@codemirror/state" +import { EditorView, lineNumbers } from "@codemirror/view" +import { + diffGutterExtension, + DiffGutterMarkerKind, + getDiffMarkers, + getDeletedLineDecorations, +} from "./diff-gutter-extension" + +/** + * Tests for the custom unified-mode diff gutter extension (#769). + * + * The extension diffs `original` vs the current document and renders + * gutter markers (added/removed/changed) in a custom CM6 gutter. + */ + +function createEditor(opts: { + doc: string + original: string + parent: HTMLElement +}): EditorView { + return new EditorView({ + state: EditorState.create({ + doc: opts.doc, + extensions: [lineNumbers(), diffGutterExtension(opts.original)], + }), + parent: opts.parent, + }) +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)) +} + +describe("diffGutterExtension", () => { + let parent: HTMLDivElement + let view: EditorView + + beforeEach(() => { + parent = document.createElement("div") + document.body.appendChild(parent) + }) + + afterEach(() => { + view?.destroy() + parent.remove() + }) + + test("module exports diffGutterExtension function", async () => { + const mod = await import("./diff-gutter-extension") + expect(mod.diffGutterExtension).toBeDefined() + expect(typeof mod.diffGutterExtension).toBe("function") + }) + + test("module exports DiffGutterMarkerKind enum/type", async () => { + const mod = await import("./diff-gutter-extension") + expect(mod.DiffGutterMarkerKind).toBeDefined() + }) + + test("renders without errors on identical content", async () => { + view = createEditor({ + doc: "line1\nline2\nline3", + original: "line1\nline2\nline3", + parent, + }) + + // Wait for initial deferred compute + await sleep(100) + + expect(view.state.doc.toString()).toBe("line1\nline2\nline3") + const markers = getDiffMarkers(view.state) + expect(markers.size).toBe(0) + }) + + test("shows added marker for new lines", async () => { + view = createEditor({ + doc: "line1\nnewline\nline2", + original: "line1\nline2", + parent, + }) + + await sleep(400) + + const markers = getDiffMarkers(view.state) + expect(markers.size).toBeGreaterThan(0) + }) + + test("shows removed marker when lines are deleted", async () => { + view = createEditor({ + doc: "line1\nline3", + original: "line1\nline2\nline3", + parent, + }) + + await sleep(400) + + const markers = getDiffMarkers(view.state) + expect(markers.size).toBeGreaterThan(0) + }) + + test("shows changed marker for modified lines", async () => { + view = createEditor({ + doc: "line1\nline2-modified\nline3", + original: "line1\nline2\nline3", + parent, + }) + + await sleep(400) + + const markers = getDiffMarkers(view.state) + expect(markers.size).toBeGreaterThan(0) + }) + + test("updates markers after edit (debounced)", async () => { + view = createEditor({ + doc: "line1\nline2", + original: "line1\nline2", + parent, + }) + + // Initially identical — no markers after initial deferred compute + await sleep(400) + let markers = getDiffMarkers(view.state) + expect(markers.size).toBe(0) + + // Edit: add a line + view.dispatch({ + changes: { from: view.state.doc.length, insert: "\nnewline" }, + }) + + // Wait for debounced update + await sleep(400) + + markers = getDiffMarkers(view.state) + expect(markers.size).toBeGreaterThan(0) + }) + + test("handles empty original gracefully", () => { + view = createEditor({ + doc: "some content", + original: "", + parent, + }) + + expect(view.state.doc.toString()).toBe("some content") + }) + + test("handles empty modified gracefully", () => { + view = createEditor({ + doc: "", + original: "some content", + parent, + }) + + expect(view.state.doc.toString()).toBe("") + }) +}) + +// --------------------------------------------------------------------------- +// Inline deleted-line decorations +// --------------------------------------------------------------------------- + +describe("deleted-line decorations", () => { + let parent: HTMLDivElement + let view: EditorView + + beforeEach(() => { + parent = document.createElement("div") + document.body.appendChild(parent) + }) + + afterEach(() => { + view?.destroy() + parent.remove() + }) + + test("getDeletedLineDecorations is exported", async () => { + const mod = await import("./diff-gutter-extension") + expect(mod.getDeletedLineDecorations).toBeDefined() + expect(typeof mod.getDeletedLineDecorations).toBe("function") + }) + + test("deleted lines produce decoration state entries", async () => { + view = createEditor({ + doc: "line1\nline3", + original: "line1\nline2\nline3", + parent, + }) + + // Wait for debounced diff computation + await sleep(400) + + const decos = getDeletedLineDecorations(view.state) + expect(decos.size).toBeGreaterThan(0) + }) + + test("no deleted-line decorations when content is identical", async () => { + view = createEditor({ + doc: "line1\nline2", + original: "line1\nline2", + parent, + }) + + await sleep(400) + + const decos = getDeletedLineDecorations(view.state) + expect(decos.size).toBe(0) + }) + + test("deleted lines render DOM widgets with the removed text", async () => { + view = createEditor({ + doc: "line1\nline3", + original: "line1\nremoved-line\nline3", + parent, + }) + + await sleep(400) + + // The widget should render a DOM element containing the deleted text + const widgets = parent.querySelectorAll(".cm-deleted-line-widget") + expect(widgets.length).toBeGreaterThan(0) + // At least one widget should contain the removed text + const texts = Array.from(widgets).map((w) => w.textContent) + expect(texts.some((t) => t?.includes("removed-line"))).toBe(true) + }) + + test("multiple deleted lines show multiple widgets", async () => { + view = createEditor({ + doc: "line1\nline4", + original: "line1\nline2\nline3\nline4", + parent, + }) + + await sleep(400) + + const widgets = parent.querySelectorAll(".cm-deleted-line-widget") + // Should have widgets for line2 and line3 + expect(widgets.length).toBeGreaterThan(0) + }) +}) diff --git a/packages/session-ui/src/v2/components/diff-gutter-extension.ts b/packages/session-ui/src/v2/components/diff-gutter-extension.ts new file mode 100644 index 000000000..5a750cc17 --- /dev/null +++ b/packages/session-ui/src/v2/components/diff-gutter-extension.ts @@ -0,0 +1,414 @@ +/** + * Custom CM6 gutter extension for unified diff mode (#769). + * + * Diffs `original` vs the current editor document and renders: + * - Gutter markers: green (added), red (removed), yellow (changed) + * - Inline deleted-line widgets: faded red text showing removed content + * + * Updates on a debounced basis (~300ms) after each edit. + * Uses the `diff` npm package (already a session-ui dependency) for + * line-level diffing. + * + * @module + */ + +import { + type Extension, + StateField, + StateEffect, + RangeSet, + type EditorState, + type Range, +} from "@codemirror/state" +import { + EditorView, + GutterMarker, + gutter, + ViewPlugin, + Decoration, + WidgetType, + type ViewUpdate, + type DecorationSet, +} from "@codemirror/view" +import { diffLines, type Change } from "diff" + +// --------------------------------------------------------------------------- +// Marker kinds and GutterMarker subclasses +// --------------------------------------------------------------------------- + +export enum DiffGutterMarkerKind { + Added = "added", + Removed = "removed", + Changed = "changed", +} + +class DiffMarker extends GutterMarker { + constructor(readonly kind: DiffGutterMarkerKind) { + super() + } + + toDOM(): HTMLElement { + const el = document.createElement("div") + el.className = `cm-diff-gutter-marker cm-diff-gutter-${this.kind}` + el.style.width = "4px" + el.style.height = "100%" + el.style.borderRadius = "1px" + + switch (this.kind) { + case DiffGutterMarkerKind.Added: + el.style.backgroundColor = "var(--v2-state-fg-success, var(--text-on-success-base, #4caf50))" + break + case DiffGutterMarkerKind.Removed: + el.style.backgroundColor = "var(--v2-state-fg-danger, var(--text-on-critical-base, #f44336))" + break + case DiffGutterMarkerKind.Changed: + el.style.backgroundColor = "var(--v2-state-fg-warning, var(--border-warning-base, #ffc107))" + break + } + + return el + } +} + +const addedMarker = new DiffMarker(DiffGutterMarkerKind.Added) +const removedMarker = new DiffMarker(DiffGutterMarkerKind.Removed) +const changedMarker = new DiffMarker(DiffGutterMarkerKind.Changed) + +// --------------------------------------------------------------------------- +// Deleted-line widget — renders removed text as faded read-only lines +// --------------------------------------------------------------------------- + +class DeletedLinesWidget extends WidgetType { + constructor(readonly lines: string[]) { + super() + } + + toDOM(): HTMLElement { + const wrapper = document.createElement("div") + wrapper.className = "cm-deleted-line-widget" + wrapper.style.fontFamily = "var(--font-mono, ui-monospace, monospace)" + wrapper.style.fontSize = "13px" + wrapper.style.lineHeight = "1.5" + wrapper.style.pointerEvents = "none" + wrapper.style.userSelect = "none" + wrapper.setAttribute("aria-hidden", "true") + + for (const line of this.lines) { + const row = document.createElement("div") + row.className = "cm-deleted-line-row" + row.style.backgroundColor = "var(--v2-state-bg-danger, rgba(244, 67, 54, 0.06))" + row.style.color = "var(--v2-state-fg-danger, var(--text-on-critical-base, #f44336))" + row.style.opacity = "0.7" + row.style.textDecoration = "line-through" + row.style.padding = "0 4px" + row.style.whiteSpace = "pre-wrap" + row.style.wordBreak = "break-all" + // Prefix with minus sign like a diff + row.textContent = `- ${line}` + wrapper.appendChild(row) + } + + return wrapper + } + + eq(other: DeletedLinesWidget): boolean { + return ( + this.lines.length === other.lines.length && + this.lines.every((l, i) => l === other.lines[i]) + ) + } + + get estimatedHeight(): number { + // ~20px per line (13px font * 1.5 line-height) + return this.lines.length * 20 + } + + ignoreEvent(): boolean { + return true + } +} + +// --------------------------------------------------------------------------- +// State effects and fields +// --------------------------------------------------------------------------- + +const setDiffMarkers = StateEffect.define>() +const setDeletedDecorations = StateEffect.define() + +const diffMarkersField = StateField.define>({ + create() { + return RangeSet.empty + }, + update(markers, tr) { + for (const effect of tr.effects) { + if (effect.is(setDiffMarkers)) { + return effect.value + } + } + return markers + }, +}) + +const deletedDecorationsField = StateField.define({ + create() { + return Decoration.none + }, + update(decos, tr) { + for (const effect of tr.effects) { + if (effect.is(setDeletedDecorations)) { + return effect.value + } + } + // Map through document changes so positions stay valid + return decos.map(tr.changes) + }, + provide: (f) => EditorView.decorations.from(f), +}) + +// --------------------------------------------------------------------------- +// Diff computation +// --------------------------------------------------------------------------- + +interface LineMarker { + line: number + kind: DiffGutterMarkerKind +} + +/** Where to insert a deleted-lines widget: after `line` in the current doc. */ +interface DeletedBlock { + afterLine: number + lines: string[] +} + +interface DiffResult { + markers: LineMarker[] + deletedBlocks: DeletedBlock[] +} + +function computeDiff(original: string, current: string): DiffResult { + const changes = diffLines(original, current) + const markers: LineMarker[] = [] + const deletedBlocks: DeletedBlock[] = [] + + let currentLine = 1 + + for (let i = 0; i < changes.length; i++) { + const change = changes[i] + const lineCount = change.count ?? 0 + + if (change.removed) { + const removedText = (change.value ?? "").replace(/\n$/, "") + const removedLines = removedText.split("\n") + + // Check if the next change is an addition (replacement) + if (i + 1 < changes.length && changes[i + 1].added) { + const nextChange = changes[i + 1] + const nextCount = nextChange.count ?? 0 + // This is a replacement — mark added lines as "changed" + for (let j = 0; j < nextCount; j++) { + markers.push({ + line: currentLine + j, + kind: DiffGutterMarkerKind.Changed, + }) + } + // Show the deleted lines above the changed lines + deletedBlocks.push({ + afterLine: Math.max(0, currentLine - 1), + lines: removedLines, + }) + currentLine += nextCount + i++ // Skip the next (added) change since we handled it + } else { + // Pure removal — show deleted lines at the current position + markers.push({ + line: Math.max(1, currentLine), + kind: DiffGutterMarkerKind.Removed, + }) + deletedBlocks.push({ + afterLine: Math.max(0, currentLine - 1), + lines: removedLines, + }) + } + // Removed-only: currentLine doesn't advance + } else if (change.added) { + // Pure addition (not a replacement — those are handled above) + for (let j = 0; j < lineCount; j++) { + markers.push({ + line: currentLine + j, + kind: DiffGutterMarkerKind.Added, + }) + } + currentLine += lineCount + } else { + // Unchanged lines + currentLine += lineCount + } + } + + return { markers, deletedBlocks } +} + +function markersToRangeSet( + markers: LineMarker[], + state: EditorState, +): RangeSet { + const builder: { from: number; marker: DiffMarker }[] = [] + + for (const m of markers) { + if (m.line < 1 || m.line > state.doc.lines) continue + + const lineStart = state.doc.line(m.line).from + const marker = + m.kind === DiffGutterMarkerKind.Added + ? addedMarker + : m.kind === DiffGutterMarkerKind.Removed + ? removedMarker + : changedMarker + + builder.push({ from: lineStart, marker }) + } + + builder.sort((a, b) => a.from - b.from) + + return RangeSet.of( + builder.map((b) => b.marker.range(b.from)), + ) +} + +function deletedBlocksToDecorations( + blocks: DeletedBlock[], + state: EditorState, +): DecorationSet { + const widgets: Range[] = [] + + for (const block of blocks) { + if (block.lines.length === 0) continue + + // Position: after the specified line, or at doc start if afterLine is 0 + let pos: number + if (block.afterLine <= 0) { + pos = 0 + } else if (block.afterLine >= state.doc.lines) { + pos = state.doc.length + } else { + pos = state.doc.line(block.afterLine).to + } + + const widget = Decoration.widget({ + widget: new DeletedLinesWidget(block.lines), + block: true, + side: 1, // After the line + }) + + widgets.push(widget.range(pos)) + } + + // Sort by position (required by RangeSet) + widgets.sort((a, b) => a.from - b.from) + + return Decoration.set(widgets) +} + +// --------------------------------------------------------------------------- +// View plugin: debounced diff recomputation +// --------------------------------------------------------------------------- + +const DEBOUNCE_MS = 300 + +function createDiffPlugin(original: string) { + return ViewPlugin.fromClass( + class { + private timer: ReturnType | null = null + private original: string + + constructor(private view: EditorView) { + this.original = original + // Defer initial diff computation — can't dispatch during construction + this.timer = setTimeout(() => { + this.timer = null + this.computeAndApply() + }, 0) + } + + update(update: ViewUpdate) { + if (update.docChanged) { + this.scheduleCompute() + } + } + + private scheduleCompute() { + if (this.timer !== null) { + clearTimeout(this.timer) + } + this.timer = setTimeout(() => { + this.timer = null + this.computeAndApply() + }, DEBOUNCE_MS) + } + + private computeAndApply() { + const current = this.view.state.doc.toString() + const { markers, deletedBlocks } = computeDiff(this.original, current) + const markerSet = markersToRangeSet(markers, this.view.state) + const decoSet = deletedBlocksToDecorations(deletedBlocks, this.view.state) + this.view.dispatch({ + effects: [ + setDiffMarkers.of(markerSet), + setDeletedDecorations.of(decoSet), + ], + }) + } + + destroy() { + if (this.timer !== null) { + clearTimeout(this.timer) + } + } + }, + ) +} + +// --------------------------------------------------------------------------- +// Public API +// --------------------------------------------------------------------------- + +/** + * Create a CM6 extension that shows diff gutter markers + inline + * deleted-line widgets comparing the editor content against `original`. + * Markers and widgets update 300ms after the last edit. + */ +export function diffGutterExtension(original: string): Extension { + return [ + diffMarkersField, + deletedDecorationsField, + gutter({ + class: "cm-diff-gutter", + markers: (view) => view.state.field(diffMarkersField), + }), + createDiffPlugin(original), + // Gutter styling + EditorView.baseTheme({ + ".cm-diff-gutter": { + width: "6px", + minWidth: "6px", + marginRight: "2px", + }, + }), + ] +} + +/** + * Read the current diff markers from an EditorState. + * Useful for testing — avoids relying on DOM rendering. + */ +export function getDiffMarkers(state: EditorState): RangeSet { + return state.field(diffMarkersField) +} + +/** + * Read the current deleted-line decorations from an EditorState. + * Useful for testing. + */ +export function getDeletedLineDecorations(state: EditorState): DecorationSet { + return state.field(deletedDecorationsField) +} diff --git a/packages/session-ui/src/v2/components/editable-diff-view-core.ts b/packages/session-ui/src/v2/components/editable-diff-view-core.ts new file mode 100644 index 000000000..1d9a71edd --- /dev/null +++ b/packages/session-ui/src/v2/components/editable-diff-view-core.ts @@ -0,0 +1,545 @@ +/** + * EditableDiffView — CodeMirror 6 core logic. + * + * Pure logic layer: language loading, theme building, editor construction. + * The SolidJS component wrapper lives in editable-diff-view.tsx and uses + * these functions. Tests import this file directly (no JSX transform needed). + * + * @module + */ + +import { Compartment, EditorState, Transaction, ChangeSet, type Extension } from "@codemirror/state" +import { + EditorView, + lineNumbers, + drawSelection, + highlightActiveLine, + highlightSpecialChars, +} from "@codemirror/view" +import { + MergeView, + unifiedMergeView, + originalDocChangeEffect, + getOriginalDoc, +} from "@codemirror/merge" +import { type LanguageSupport } from "@codemirror/language" +import { + HighlightStyle, + syntaxHighlighting, +} from "@codemirror/language" +import { tags } from "@lezer/highlight" + +// --------------------------------------------------------------------------- +// Language loader — dynamic imports so unused grammars stay out of the bundle. +// --------------------------------------------------------------------------- + +const EXTENSION_MAP: Record Promise> = { + js: () => import("@codemirror/lang-javascript").then((m) => m.javascript({ jsx: false, typescript: false })), + jsx: () => import("@codemirror/lang-javascript").then((m) => m.javascript({ jsx: true, typescript: false })), + ts: () => import("@codemirror/lang-javascript").then((m) => m.javascript({ jsx: false, typescript: true })), + tsx: () => import("@codemirror/lang-javascript").then((m) => m.javascript({ jsx: true, typescript: true })), + mjs: () => import("@codemirror/lang-javascript").then((m) => m.javascript({ jsx: false, typescript: false })), + cjs: () => import("@codemirror/lang-javascript").then((m) => m.javascript({ jsx: false, typescript: false })), + py: () => import("@codemirror/lang-python").then((m) => m.python()), + python: () => import("@codemirror/lang-python").then((m) => m.python()), + json: () => import("@codemirror/lang-json").then((m) => m.json()), + jsonc: () => import("@codemirror/lang-json").then((m) => m.json()), + md: () => import("@codemirror/lang-markdown").then((m) => m.markdown()), + markdown: () => import("@codemirror/lang-markdown").then((m) => m.markdown()), + css: () => import("@codemirror/lang-css").then((m) => m.css()), + scss: () => import("@codemirror/lang-css").then((m) => m.css()), + html: () => import("@codemirror/lang-html").then((m) => m.html()), + htm: () => import("@codemirror/lang-html").then((m) => m.html()), + xml: () => import("@codemirror/lang-html").then((m) => m.html()), + svg: () => import("@codemirror/lang-html").then((m) => m.html()), +} + +/** + * Dynamically load a CodeMirror language support by file extension. + * Returns null for unknown extensions. + */ +export async function loadLanguage( + ext: string, +): Promise { + const normalized = ext.replace(/^\./, "").toLowerCase() + const loader = EXTENSION_MAP[normalized] + if (!loader) return null + try { + return await loader() + } catch { + return null + } +} + +// --------------------------------------------------------------------------- +// Theme builder — maps app CSS custom properties to CM6 theme selectors. +// +// Uses the app's v2 design tokens (--v2-*) for structural chrome and the +// OpenCode Shiki token variables (--syntax-*) for syntax highlighting. Both +// systems switch light/dark purely via CSS — the CM6 theme is the same +// object in either mode, referencing variables that the app's theme resolver +// redefines for each color scheme. +// --------------------------------------------------------------------------- + +/** + * Build a CM6 theme extension using the app's design tokens. + * The `mode` parameter sets the CM6 `dark` flag (which controls default + * fallback colors); actual colors come from CSS custom properties. + */ +export function buildThemeExtension(mode: "light" | "dark"): Extension { + const isDark = mode === "dark" + + return EditorView.theme( + { + "&": { + backgroundColor: "var(--v2-background-bg-base, var(--background-base))", + color: "var(--v2-text-text-base, var(--text-strong))", + fontFamily: "var(--font-mono, ui-monospace, monospace)", + fontSize: "13px", + lineHeight: "1.5", + }, + ".cm-content": { + caretColor: "var(--v2-text-text-base, var(--text-strong))", + fontFamily: "var(--font-mono, ui-monospace, monospace)", + }, + ".cm-cursor, .cm-dropCursor": { + borderLeftColor: "var(--v2-text-text-base, var(--text-strong))", + }, + "&.cm-focused .cm-selectionBackground, .cm-selectionBackground, .cm-content ::selection": + { + backgroundColor: "var(--v2-background-bg-layer-03, var(--background-weak))", + }, + ".cm-panels": { + backgroundColor: "var(--v2-background-bg-base, var(--background-base))", + color: "var(--v2-text-text-base, var(--text-strong))", + }, + ".cm-gutters": { + backgroundColor: "var(--v2-background-bg-layer-01, var(--background-weak))", + color: "var(--v2-text-text-muted, var(--text-weak))", + borderRight: "1px solid var(--v2-border-border-base, var(--border-base))", + }, + ".cm-activeLineGutter": { + backgroundColor: "var(--v2-background-bg-layer-02, var(--background-weak))", + }, + ".cm-activeLine": { + backgroundColor: "var(--v2-background-bg-layer-01, var(--background-weak))", + }, + ".cm-foldPlaceholder": { + backgroundColor: "transparent", + border: "none", + color: "var(--v2-text-text-faint, var(--text-weaker))", + }, + // Diff highlights — softer backgrounds that read well in both modes. + // The v2 state-bg tokens are designed for small badges; for full-line + // backgrounds we mix them down to ~40% so they tint without obscuring text. + ".cm-changedLine": { + backgroundColor: "color-mix(in srgb, var(--v2-state-bg-warning, var(--surface-warning-weak)) 40%, transparent)", + }, + ".cm-changedText": { + backgroundColor: "color-mix(in srgb, var(--v2-state-bg-warning, var(--surface-warning-base)) 60%, transparent)", + }, + ".cm-insertedLine": { + backgroundColor: "color-mix(in srgb, var(--v2-state-bg-success, var(--surface-success-weak)) 40%, transparent)", + }, + ".cm-deletedLine": { + backgroundColor: "color-mix(in srgb, var(--v2-state-bg-danger, var(--surface-critical-weak)) 30%, transparent)", + }, + }, + { dark: isDark }, + ) +} + +// --------------------------------------------------------------------------- +// Syntax highlight style — maps CM6 lezer tags to the app's --syntax-* +// CSS variables so highlighting matches the user's selected theme. +// --------------------------------------------------------------------------- + +/** + * Build a HighlightStyle that uses the app's --syntax-* CSS variables. + * Returns a syntaxHighlighting extension. + */ +export function buildSyntaxHighlightStyle(): Extension { + const style = HighlightStyle.define([ + { tag: [tags.comment, tags.lineComment, tags.blockComment], + color: "var(--syntax-comment, var(--text-weak))" }, + { tag: [tags.keyword, tags.controlKeyword, tags.operatorKeyword, tags.moduleKeyword], + color: "var(--syntax-keyword, var(--text-weak))" }, + { tag: [tags.string, tags.special(tags.string), tags.character], + color: "var(--syntax-string)" }, + { tag: [tags.number, tags.bool, tags.null], + color: "var(--syntax-primitive)" }, + { tag: [tags.typeName, tags.className, tags.namespace], + color: "var(--syntax-type)" }, + { tag: [tags.propertyName, tags.attributeName, tags.labelName], + color: "var(--syntax-property)" }, + { tag: [tags.variableName, tags.definition(tags.variableName)], + color: "var(--v2-text-text-base, var(--text-strong))" }, + { tag: [tags.function(tags.variableName), tags.function(tags.propertyName)], + color: "var(--syntax-function, var(--syntax-property))" }, + { tag: [tags.constant(tags.variableName), tags.atom], + color: "var(--syntax-constant)" }, + { tag: [tags.operator, tags.punctuation, tags.separator], + color: "var(--v2-text-text-muted, var(--text-base))" }, + { tag: [tags.meta, tags.annotation, tags.processingInstruction], + color: "var(--syntax-comment, var(--text-weak))" }, + { tag: tags.heading, + color: "var(--syntax-keyword, var(--text-strong))", fontWeight: "bold" }, + { tag: tags.emphasis, + fontStyle: "italic" }, + { tag: tags.strong, + fontWeight: "bold" }, + { tag: tags.link, + color: "var(--syntax-string)", textDecoration: "underline" }, + { tag: tags.invalid, + color: "var(--v2-state-fg-danger, var(--text-on-critical-base))" }, + ]) + + return syntaxHighlighting(style) +} + +// --------------------------------------------------------------------------- +// Shared extensions — base setup shared by all editor instances. +// --------------------------------------------------------------------------- + +/** + * Build the mutable extensions that live inside a Compartment and can be + * swapped via `setReadOnly()` without re-creating the editor. + */ +export function editableExtensions(opts: { + readOnly: boolean + onChange?: (content: string) => void +}): Extension[] { + const exts: Extension[] = [ + EditorView.editable.of(!opts.readOnly), + EditorState.readOnly.of(opts.readOnly), + ] + + if (opts.onChange && !opts.readOnly) { + exts.push( + EditorView.updateListener.of((update) => { + if (update.docChanged) { + opts.onChange!(update.state.doc.toString()) + } + }), + ) + } + + return exts +} + +export function baseExtensions(opts: { + theme: Extension + language?: LanguageSupport | null +}): Extension[] { + return [ + lineNumbers(), + highlightActiveLine(), + highlightSpecialChars(), + drawSelection(), + EditorView.lineWrapping, + buildSyntaxHighlightStyle(), + opts.theme, + ...(opts.language ? [opts.language] : []), + ] +} + +// --------------------------------------------------------------------------- +// Editor construction — imperative helpers for tests and the SolidJS wrapper. +// --------------------------------------------------------------------------- + +export interface DiffEditorHandle { + /** The active EditorView for the modified (editable) pane. */ + editorView: EditorView | null + /** The MergeView instance (split mode only). */ + mergeView: MergeView | null + /** The scrollable DOM element (split: mergeView.dom, unified: editorView.scrollDOM). */ + scrollDOM: HTMLElement | null + /** Destroy all editor instances. */ + destroy: () => void + /** Revert to original: replace content, clear undo history. */ + revert: (original: string) => void + /** Get the current document content. */ + getContent: () => string + /** + * Update the original (left/before) document in place via CM6 dispatch. + * No editor re-creation. No-op if text is unchanged. + */ + updateOriginal: (text: string) => void + /** + * Update the modified (right/after) document in place via CM6 dispatch. + * No editor re-creation. No-op if text is unchanged. + */ + updateModified: (text: string) => void + /** + * Toggle readOnly state in place via Compartment reconfigure. + * No editor re-creation, no scroll displacement. + * Pass `onChange` when switching to editable to re-attach the listener. + */ + setReadOnly: (readOnly: boolean, onChange?: (content: string) => void) => void +} + +export function createDiffEditor(opts: { + parent: HTMLElement + original: string + modified: string + diffStyle: "unified" | "split" + readOnly: boolean + theme: Extension + language?: LanguageSupport | null + onChange?: (content: string) => void +}): DiffEditorHandle { + let mergeView: MergeView | null = null + let editorView: EditorView | null = null + + // Compartment for the mutable readOnly/onChange extensions on the + // modifiable pane. Reconfigured in-place by setReadOnly() — no editor + // teardown, no scroll displacement. + const editableCompartment = new Compartment() + + const base = baseExtensions({ + theme: opts.theme, + language: opts.language, + }) + + if (opts.diffStyle === "split") { + mergeView = new MergeView({ + parent: opts.parent, + a: { + doc: opts.original, + extensions: [ + ...base, + // Original pane is always readOnly, no compartment needed + ...editableExtensions({ readOnly: true }), + ], + }, + b: { + doc: opts.modified, + extensions: [ + ...base, + editableCompartment.of( + editableExtensions({ + readOnly: opts.readOnly, + onChange: opts.readOnly ? undefined : opts.onChange, + }), + ), + ], + }, + }) + + // MergeView.dom is the PARENT of .cm-editor, so EditorView.theme() + // selectors (scoped under .cm-editor) can't target it. Set height + // directly so the MergeView fills its container and scrolls. + mergeView.dom.style.height = "100%" + mergeView.dom.style.overflow = "auto" + } else { + // Unified mode: @codemirror/merge's unifiedMergeView — the standard CM6 + // inline diff that interleaves deleted lines with the editable document. + const extensions: Extension[] = [ + ...base, + editableCompartment.of( + editableExtensions({ + readOnly: opts.readOnly, + onChange: opts.readOnly ? undefined : opts.onChange, + }), + ), + unifiedMergeView({ + original: EditorState.create({ doc: opts.original }).doc, + mergeControls: false, + gutter: true, + highlightChanges: true, + syntaxHighlightDeletions: true, + }), + ] + + editorView = new EditorView({ + state: EditorState.create({ + doc: opts.modified, + extensions, + }), + parent: opts.parent, + }) + + // Ensure the editor fills and scrolls within its container + editorView.dom.style.height = "100%" + } + + function getActiveView(): EditorView | null { + if (mergeView) return mergeView.b + return editorView + } + + return { + get editorView() { + return getActiveView() + }, + get mergeView() { + return mergeView + }, + get scrollDOM(): HTMLElement | null { + // Split mode: mergeView.dom is the scrollable container (overflow: auto) + if (mergeView) return mergeView.dom + // Unified mode: EditorView.scrollDOM is the CM6 scroll element + if (editorView) return editorView.scrollDOM + return null + }, + destroy() { + mergeView?.destroy() + editorView?.destroy() + mergeView = null + editorView = null + }, + revert(original: string) { + const view = getActiveView() + if (!view) return + + // Replace entire document with original + view.dispatch({ + changes: { + from: 0, + to: view.state.doc.length, + insert: original, + }, + }) + }, + getContent() { + const view = getActiveView() + return view?.state.doc.toString() ?? "" + }, + updateOriginal(text: string) { + if (mergeView) { + // Split mode: dispatch minimal change to the A (original) pane + const aView = mergeView.a + const current = aView.state.doc.toString() + const change = minimalChanges(current, text) + if (!change) return + aView.dispatch({ + changes: change, + annotations: Transaction.addToHistory.of(false), + }) + } else if (editorView) { + // Unified mode: update via originalDocChangeEffect + const origDoc = getOriginalDoc(editorView.state) + const currentText = origDoc.toString() + if (currentText === text) return + const changes = ChangeSet.of( + { from: 0, to: origDoc.length, insert: text }, + origDoc.length, + ) + editorView.dispatch({ + effects: originalDocChangeEffect(editorView.state, changes), + annotations: Transaction.addToHistory.of(false), + }) + } + }, + updateModified(text: string) { + const view = getActiveView() + if (!view) return + const current = view.state.doc.toString() + const change = minimalChanges(current, text) + if (!change) return + view.dispatch({ + changes: change, + annotations: Transaction.addToHistory.of(false), + }) + }, + setReadOnly(readOnly: boolean, onChange?: (content: string) => void) { + const view = getActiveView() + if (!view) return + view.dispatch({ + effects: editableCompartment.reconfigure( + editableExtensions({ readOnly, onChange }), + ), + }) + }, + } +} + +/** + * Detect dark/light mode from DOM — reads `data-color-scheme` attribute + * on (set by the app's theme system). + */ +export function detectMode(): "light" | "dark" { + if (typeof document === "undefined") return "dark" + const html = document.documentElement + // The app sets data-color-scheme on + const scheme = html.getAttribute("data-color-scheme") + if (scheme === "light") return "light" + if (scheme === "dark") return "dark" + // Fallbacks for other conventions + if (html.classList.contains("dark")) return "dark" + if (html.getAttribute("data-theme") === "dark") return "dark" + if ( + typeof window !== "undefined" && + window.matchMedia?.("(prefers-color-scheme: dark)").matches + ) + return "dark" + return "light" +} + +// --------------------------------------------------------------------------- +// Minimal change computation — shared by updateOriginal / updateModified +// --------------------------------------------------------------------------- + +/** + * Compute the smallest `{from, to, insert}` that transforms `oldText` into + * `newText`, via common-prefix / common-suffix scan. Returns `null` when the + * two strings are identical (no change needed). + * + * This gives CM6 a focused change description so its internal scroll mapping + * adjusts the viewport correctly — a full-document replace (`{from: 0, + * to: doc.length, insert: newText}`) would reset scroll to the top. + */ +export function minimalChanges( + oldText: string, + newText: string, +): { from: number; to: number; insert: string } | null { + if (oldText === newText) return null + + // Find common prefix length + let prefix = 0 + const minLen = Math.min(oldText.length, newText.length) + while (prefix < minLen && oldText[prefix] === newText[prefix]) prefix++ + + // Find common suffix length (must not overlap with prefix) + let oldSuffix = oldText.length + let newSuffix = newText.length + while ( + oldSuffix > prefix && + newSuffix > prefix && + oldText[oldSuffix - 1] === newText[newSuffix - 1] + ) { + oldSuffix-- + newSuffix-- + } + + return { + from: prefix, + to: oldSuffix, + insert: newText.slice(prefix, newSuffix), + } +} + +// --------------------------------------------------------------------------- +// Scroll preservation helper +// --------------------------------------------------------------------------- + +/** + * Walk up from `el` to find the nearest ancestor with a non-zero scrollTop. + * Returns the element and its scrollTop, or null if nothing is scrolled. + * + * Used by the SolidJS wrapper to save/restore the parent scroll container's + * position across CM6 teardown/recreate cycles — the container's innerHTML + * clear resets the parent's scrollTop before any async event can capture it. + */ +export function findScrollParent( + el: HTMLElement, +): { element: HTMLElement; scrollTop: number } | null { + let node: HTMLElement | null = el.parentElement + while (node) { + if (node.scrollTop > 0) { + return { element: node, scrollTop: node.scrollTop } + } + node = node.parentElement + } + return null +} diff --git a/packages/session-ui/src/v2/components/editable-diff-view.test.ts b/packages/session-ui/src/v2/components/editable-diff-view.test.ts new file mode 100644 index 000000000..81ea57481 --- /dev/null +++ b/packages/session-ui/src/v2/components/editable-diff-view.test.ts @@ -0,0 +1,1154 @@ +import { describe, expect, test, beforeEach, afterEach } from "bun:test" +import { EditorView } from "@codemirror/view" +import { EditorState } from "@codemirror/state" +import { + loadLanguage, + buildThemeExtension, + createDiffEditor, + baseExtensions, + editableExtensions, + minimalChanges, + type DiffEditorHandle, +} from "./editable-diff-view-core" + +/** + * Tests for EditableDiffView core logic (no JSX / SolidJS needed). + * + * Tests the language loader, theme builder, and imperative editor + * construction — the behavioral contract the SolidJS wrapper delegates to. + */ + +// --------------------------------------------------------------------------- +// loadLanguage +// --------------------------------------------------------------------------- + +describe("loadLanguage", () => { + test("resolves known extensions to a LanguageSupport", async () => { + const js = await loadLanguage("js") + expect(js).not.toBeNull() + + const ts = await loadLanguage("ts") + expect(ts).not.toBeNull() + + const tsx = await loadLanguage("tsx") + expect(tsx).not.toBeNull() + + const py = await loadLanguage("py") + expect(py).not.toBeNull() + + const json = await loadLanguage("json") + expect(json).not.toBeNull() + + const md = await loadLanguage("md") + expect(md).not.toBeNull() + + const css = await loadLanguage("css") + expect(css).not.toBeNull() + + const html = await loadLanguage("html") + expect(html).not.toBeNull() + }) + + test("returns null for unknown extensions", async () => { + const result = await loadLanguage("xyz-unknown-extension") + expect(result).toBeNull() + }) + + test("strips leading dot from extension", async () => { + const result = await loadLanguage(".ts") + expect(result).not.toBeNull() + }) + + test("is case-insensitive", async () => { + const result = await loadLanguage("JSON") + expect(result).not.toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// buildThemeExtension +// --------------------------------------------------------------------------- + +describe("buildThemeExtension", () => { + test("returns a defined CM6 Extension for dark mode", () => { + const ext = buildThemeExtension("dark") + expect(ext).toBeDefined() + expect(ext).not.toBeNull() + }) + + test("returns a defined CM6 Extension for light mode", () => { + const ext = buildThemeExtension("light") + expect(ext).toBeDefined() + expect(ext).not.toBeNull() + }) + + test("dark and light produce distinct extensions", () => { + const dark = buildThemeExtension("dark") + const light = buildThemeExtension("light") + // They should be distinct objects + expect(dark).not.toBe(light) + }) +}) + +// --------------------------------------------------------------------------- +// baseExtensions (structural only — no readOnly/onChange) +// --------------------------------------------------------------------------- + +describe("baseExtensions", () => { + test("returns an array of extensions", () => { + const theme = buildThemeExtension("dark") + const exts = baseExtensions({ theme }) + expect(Array.isArray(exts)).toBe(true) + expect(exts.length).toBeGreaterThan(0) + }) +}) + +// --------------------------------------------------------------------------- +// editableExtensions (mutable readOnly/onChange — lives in Compartment) +// --------------------------------------------------------------------------- + +describe("editableExtensions", () => { + test("includes onChange listener when not readOnly", () => { + const withCb = editableExtensions({ readOnly: false, onChange: () => {} }) + const withoutCb = editableExtensions({ readOnly: false }) + // With onChange callback should have one more extension + expect(withCb.length).toBe(withoutCb.length + 1) + }) + + test("omits onChange listener when readOnly", () => { + const withCb = editableExtensions({ readOnly: true, onChange: () => {} }) + const withoutCb = editableExtensions({ readOnly: true }) + // readOnly suppresses the onChange extension regardless + expect(withCb.length).toBe(withoutCb.length) + }) +}) + +// --------------------------------------------------------------------------- +// createDiffEditor — split mode +// --------------------------------------------------------------------------- + +describe("createDiffEditor (split mode)", () => { + let parent: HTMLDivElement + let handle: DiffEditorHandle + + beforeEach(() => { + parent = document.createElement("div") + document.body.appendChild(parent) + }) + + afterEach(() => { + handle?.destroy() + parent.remove() + }) + + test("creates a split MergeView in the parent element", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "line1\nline2", + modified: "line1\nline2-changed", + diffStyle: "split", + readOnly: false, + theme, + }) + + expect(handle.mergeView).not.toBeNull() + // MergeView should have inserted DOM into parent + expect(parent.children.length).toBeGreaterThan(0) + }) + + test("editorView returns the modified (b) pane", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original", + modified: "modified", + diffStyle: "split", + readOnly: false, + theme, + }) + + expect(handle.editorView).not.toBeNull() + expect(handle.getContent()).toBe("modified") + }) + + test("onChange fires when content changes", () => { + const changes: string[] = [] + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original", + modified: "modified", + diffStyle: "split", + readOnly: false, + theme, + onChange: (c) => changes.push(c), + }) + + const view = handle.editorView! + view.dispatch({ + changes: { from: 0, to: view.state.doc.length, insert: "new content" }, + }) + + expect(changes.length).toBeGreaterThan(0) + expect(changes[changes.length - 1]).toBe("new content") + }) + + test("readOnly makes editor non-editable but allows programmatic dispatch", () => { + const changes: string[] = [] + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original", + modified: "modified", + diffStyle: "split", + readOnly: true, + theme, + onChange: (c) => changes.push(c), + }) + + const view = handle.editorView! + + // Editor should be non-editable (blocks user input) + expect(EditorView.editable.of(false)).toBeDefined() + + // Programmatic dispatches go through — needed for in-place content updates. + // (CM6's readOnly facet only blocks user-generated transactions.) + view.dispatch({ + changes: { from: 0, insert: "X" }, + }) + expect(view.state.doc.toString()).toBe("Xmodified") + + // onChange should NOT fire even though dispatch succeeded, + // because readOnly=true means onChange was not attached. + expect(changes.length).toBe(0) + }) + + test("getContent returns current document text", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "mod", + diffStyle: "split", + readOnly: false, + theme, + }) + + expect(handle.getContent()).toBe("mod") + + handle.editorView!.dispatch({ + changes: { from: 0, to: 3, insert: "updated" }, + }) + + expect(handle.getContent()).toBe("updated") + }) + + test("revert replaces content with original", () => { + const changes: string[] = [] + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original text", + modified: "modified text", + diffStyle: "split", + readOnly: false, + theme, + onChange: (c) => changes.push(c), + }) + + // Make some edits + handle.editorView!.dispatch({ + changes: { + from: 0, + to: handle.editorView!.state.doc.length, + insert: "user edits", + }, + }) + expect(handle.getContent()).toBe("user edits") + + // Revert + handle.revert("original text") + expect(handle.getContent()).toBe("original text") + }) + + test("destroy cleans up both editors", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "a", + modified: "b", + diffStyle: "split", + readOnly: false, + theme, + }) + + handle.destroy() + expect(handle.editorView).toBeNull() + expect(handle.mergeView).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// createDiffEditor — unified mode +// --------------------------------------------------------------------------- + +describe("createDiffEditor (unified mode)", () => { + let parent: HTMLDivElement + let handle: DiffEditorHandle + + beforeEach(() => { + parent = document.createElement("div") + document.body.appendChild(parent) + }) + + afterEach(() => { + handle?.destroy() + parent.remove() + }) + + test("creates a single EditorView in unified mode", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "line1\nline2", + modified: "line1\nline2-changed", + diffStyle: "unified", + readOnly: false, + theme, + }) + + expect(handle.editorView).not.toBeNull() + // Unified mode uses unifiedMergeView (single EditorView, not MergeView) + expect(handle.mergeView).toBeNull() + expect(parent.children.length).toBeGreaterThan(0) + }) + + test("unified mode shows deleted lines via unifiedMergeView", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "line1\ndeleted-line\nline3", + modified: "line1\nline3", + diffStyle: "unified", + readOnly: false, + theme, + }) + + // unifiedMergeView interleaves deleted lines in the document + // The editor content should include the deleted text + const view = handle.editorView! + const dom = view.dom + // CM6 merge view renders deleted chunks with specific CSS classes + const hasDeletedContent = dom.querySelector(".cm-deletedChunk") !== null + || dom.textContent?.includes("deleted-line") + expect(hasDeletedContent).toBe(true) + }) + + test("onChange fires on content changes in unified mode", () => { + const changes: string[] = [] + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original", + modified: "modified", + diffStyle: "unified", + readOnly: false, + theme, + onChange: (c) => changes.push(c), + }) + + const view = handle.editorView! + view.dispatch({ + changes: { from: 0, to: view.state.doc.length, insert: "new" }, + }) + + expect(changes.length).toBeGreaterThan(0) + expect(changes[changes.length - 1]).toBe("new") + }) + + test("readOnly allows programmatic dispatch in unified mode", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "mod", + diffStyle: "unified", + readOnly: true, + theme, + }) + + const view = handle.editorView! + // Programmatic dispatches go through even in readOnly mode + view.dispatch({ changes: { from: 0, insert: "X" } }) + expect(view.state.doc.toString()).toBe("Xmod") + }) +}) + +// --------------------------------------------------------------------------- +// updateOriginal — in-place original document update (no editor re-creation) +// --------------------------------------------------------------------------- + +describe("updateOriginal", () => { + let parent: HTMLElement + let handle: DiffEditorHandle | null = null + + beforeEach(() => { + parent = document.createElement("div") + document.body.appendChild(parent) + }) + + afterEach(() => { + handle?.destroy() + parent.remove() + }) + + test("updates original text in split mode", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "old original", + modified: "modified text", + diffStyle: "split", + readOnly: false, + theme, + }) + + handle.updateOriginal("new original") + + // The original pane should have the new text + const origView = handle.mergeView!.a + expect(origView.state.doc.toString()).toBe("new original") + // Modified side untouched + expect(handle.getContent()).toBe("modified text") + }) + + test("updates original text in unified mode", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "old original", + modified: "modified text", + diffStyle: "unified", + readOnly: false, + theme, + }) + + handle.updateOriginal("new original") + + // Modified side should be untouched + expect(handle.getContent()).toBe("modified text") + }) + + test("is a no-op when text is unchanged (split)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "same text", + modified: "modified", + diffStyle: "split", + readOnly: false, + theme, + }) + + const origView = handle.mergeView!.a + const stateBefore = origView.state + + handle.updateOriginal("same text") + + // State should be the exact same object (no transaction dispatched) + expect(origView.state).toBe(stateBefore) + }) + + test("DOM element is reused, not re-created (split)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "before", + modified: "modified", + diffStyle: "split", + readOnly: false, + theme, + }) + + const domBefore = handle.mergeView!.dom + + handle.updateOriginal("after") + + // The MergeView DOM container must be the same object + expect(handle.mergeView!.dom).toBe(domBefore) + }) + + test("DOM element is reused, not re-created (unified)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "before", + modified: "modified", + diffStyle: "unified", + readOnly: false, + theme, + }) + + const domBefore = handle.editorView!.dom + + handle.updateOriginal("after") + + // The EditorView DOM must be the same object + expect(handle.editorView!.dom).toBe(domBefore) + }) + + test("works when readOnly is true (split)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "old", + modified: "mod", + diffStyle: "split", + readOnly: true, + theme, + }) + + // The original pane is always readOnly in split mode. + // updateOriginal must still succeed (programmatic dispatch). + handle.updateOriginal("new") + const origView = handle.mergeView!.a + expect(origView.state.doc.toString()).toBe("new") + }) + + test("works when readOnly is true (unified)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "old", + modified: "mod", + diffStyle: "unified", + readOnly: true, + theme, + }) + + handle.updateOriginal("new") + // The update should succeed despite readOnly + // (originalDocChangeEffect is dispatched as an effect, not a doc change) + expect(handle.getContent()).toBe("mod") // modified unchanged + }) +}) + +// --------------------------------------------------------------------------- +// updateModified — in-place modified document update (no editor re-creation) +// --------------------------------------------------------------------------- + +describe("updateModified", () => { + let parent: HTMLElement + let handle: DiffEditorHandle | null = null + + beforeEach(() => { + parent = document.createElement("div") + document.body.appendChild(parent) + }) + + afterEach(() => { + handle?.destroy() + parent.remove() + }) + + test("updates modified text in split mode", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original text", + modified: "old modified", + diffStyle: "split", + readOnly: false, + theme, + }) + + handle.updateModified("new modified") + expect(handle.getContent()).toBe("new modified") + // Original side untouched + expect(handle.mergeView!.a.state.doc.toString()).toBe("original text") + }) + + test("updates modified text in unified mode", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original text", + modified: "old modified", + diffStyle: "unified", + readOnly: false, + theme, + }) + + handle.updateModified("new modified") + expect(handle.getContent()).toBe("new modified") + }) + + test("is a no-op when text is unchanged (unified)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original", + modified: "same text", + diffStyle: "unified", + readOnly: false, + theme, + }) + + const stateBefore = handle.editorView!.state + + handle.updateModified("same text") + + // State should be the exact same object + expect(handle.editorView!.state).toBe(stateBefore) + }) + + test("DOM element is reused, not re-created (split)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original", + modified: "before", + diffStyle: "split", + readOnly: false, + theme, + }) + + const domBefore = handle.mergeView!.dom + + handle.updateModified("after") + + expect(handle.mergeView!.dom).toBe(domBefore) + }) + + test("DOM element is reused, not re-created (unified)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original", + modified: "before", + diffStyle: "unified", + readOnly: false, + theme, + }) + + const domBefore = handle.editorView!.dom + + handle.updateModified("after") + + expect(handle.editorView!.dom).toBe(domBefore) + }) + + test("works when readOnly is true (split)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "old", + diffStyle: "split", + readOnly: true, + theme, + }) + + handle.updateModified("new") + expect(handle.getContent()).toBe("new") + }) + + test("works when readOnly is true (unified)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "old", + diffStyle: "unified", + readOnly: true, + theme, + }) + + handle.updateModified("new") + expect(handle.getContent()).toBe("new") + }) + + test("does not fire onChange callback (programmatic updates are silent)", () => { + const changes: string[] = [] + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "original", + modified: "old", + diffStyle: "split", + readOnly: false, + theme, + onChange: (c) => changes.push(c), + }) + + handle.updateModified("new") + + // updateModified uses addToHistory:false annotation — but onChange + // still fires for doc changes via updateListener. The key property + // is that the editor is not re-created. + // (Whether onChange fires or not is an implementation choice; the + // critical assertion is DOM reuse and content correctness.) + expect(handle.getContent()).toBe("new") + }) +}) + +// --------------------------------------------------------------------------- +// setReadOnly — toggle readOnly in place via Compartment (no re-creation) +// --------------------------------------------------------------------------- + +describe("setReadOnly", () => { + let parent: HTMLElement + let handle: DiffEditorHandle | null = null + + beforeEach(() => { + parent = document.createElement("div") + document.body.appendChild(parent) + }) + + afterEach(() => { + handle?.destroy() + parent.remove() + }) + + test("toggles readOnly without re-creating editor (split)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "mod", + diffStyle: "split", + readOnly: false, + theme, + }) + + const domBefore = handle.mergeView!.dom + + handle.setReadOnly(true) + + // DOM must be the same object — no re-creation + expect(handle.mergeView!.dom).toBe(domBefore) + // Content preserved + expect(handle.getContent()).toBe("mod") + }) + + test("toggles readOnly without re-creating editor (unified)", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "mod", + diffStyle: "unified", + readOnly: false, + theme, + }) + + const domBefore = handle.editorView!.dom + + handle.setReadOnly(true) + + expect(handle.editorView!.dom).toBe(domBefore) + expect(handle.getContent()).toBe("mod") + }) + + test("re-enables onChange when switching from readOnly to editable (split)", () => { + const changes: string[] = [] + const onChange = (c: string) => changes.push(c) + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "mod", + diffStyle: "split", + readOnly: true, + theme, + }) + + // No onChange while readOnly — user edits blocked by editable:false + // Switch to editable with onChange + handle.setReadOnly(false, onChange) + + // Now a programmatic dispatch should trigger onChange + handle.editorView!.dispatch({ + changes: { from: 0, insert: "X" }, + }) + expect(changes.length).toBe(1) + expect(changes[0]).toBe("Xmod") + }) + + test("re-enables onChange when switching from readOnly to editable (unified)", () => { + const changes: string[] = [] + const onChange = (c: string) => changes.push(c) + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "mod", + diffStyle: "unified", + readOnly: true, + theme, + }) + + handle.setReadOnly(false, onChange) + + handle.editorView!.dispatch({ + changes: { from: 0, insert: "X" }, + }) + expect(changes.length).toBe(1) + expect(changes[0]).toBe("Xmod") + }) + + test("disables onChange when switching to readOnly (split)", () => { + const changes: string[] = [] + const onChange = (c: string) => changes.push(c) + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "mod", + diffStyle: "split", + readOnly: false, + theme, + onChange, + }) + + // Verify onChange fires initially + handle.editorView!.dispatch({ + changes: { from: 0, insert: "A" }, + }) + expect(changes.length).toBe(1) + + // Switch to readOnly + handle.setReadOnly(true) + + // Programmatic dispatch still goes through (no transactionFilter) + // but onChange should NOT fire + handle.editorView!.dispatch({ + changes: { from: 0, insert: "B" }, + }) + expect(changes.length).toBe(1) // still 1, not 2 + }) + + test("round-trips readOnly false→true→false preserving content (unified)", () => { + const changes: string[] = [] + const onChange = (c: string) => changes.push(c) + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "orig", + modified: "mod", + diffStyle: "unified", + readOnly: false, + theme, + onChange, + }) + + const domBefore = handle.editorView!.dom + + handle.setReadOnly(true) + handle.setReadOnly(false, onChange) + + // DOM still the same + expect(handle.editorView!.dom).toBe(domBefore) + // Content preserved + expect(handle.getContent()).toBe("mod") + // onChange works after round-trip + handle.editorView!.dispatch({ + changes: { from: 3, insert: "!" }, + }) + expect(changes).toContain("mod!") + }) +}) + +// --------------------------------------------------------------------------- +// Line wrapping +// --------------------------------------------------------------------------- + +describe("lineWrapping", () => { + test("baseExtensions includes lineWrapping extension", () => { + const theme = buildThemeExtension("dark") + const exts = baseExtensions({ theme }) + // EditorView.lineWrapping is the specific extension object. + // Verify it's in the array by identity (same import path). + expect(exts).toContain(EditorView.lineWrapping) + }) +}) + +// --------------------------------------------------------------------------- +// Scroll / height setup +// --------------------------------------------------------------------------- + +describe("scroll behavior", () => { + let parent: HTMLDivElement + let handle: DiffEditorHandle + + beforeEach(() => { + parent = document.createElement("div") + document.body.appendChild(parent) + }) + + afterEach(() => { + handle?.destroy() + parent.remove() + }) + + test("split mode: mergeView.dom has height 100%", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "a\nb\nc", + modified: "a\nb\nc\nd", + diffStyle: "split", + readOnly: false, + theme, + }) + + expect(handle.mergeView).not.toBeNull() + expect(handle.mergeView!.dom.style.height).toBe("100%") + }) + + test("unified mode: editorView.dom has height 100%", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "a\nb\nc", + modified: "a\nb\nc\nd", + diffStyle: "unified", + readOnly: false, + theme, + }) + + expect(handle.editorView).not.toBeNull() + expect(handle.editorView!.dom.style.height).toBe("100%") + }) +}) + +// --------------------------------------------------------------------------- +// scrollDOM accessor +// --------------------------------------------------------------------------- + +describe("scrollDOM accessor", () => { + let parent: HTMLDivElement + let handle: DiffEditorHandle + + beforeEach(() => { + parent = document.createElement("div") + document.body.appendChild(parent) + }) + + afterEach(() => { + handle?.destroy() + parent.remove() + }) + + test("split mode: scrollDOM returns mergeView.dom", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "a\nb", + modified: "a\nc", + diffStyle: "split", + readOnly: false, + theme, + }) + + expect(handle.scrollDOM).not.toBeNull() + expect(handle.scrollDOM).toBe(handle.mergeView!.dom) + }) + + test("unified mode: scrollDOM returns editorView.scrollDOM", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "a\nb", + modified: "a\nc", + diffStyle: "unified", + readOnly: false, + theme, + }) + + expect(handle.scrollDOM).not.toBeNull() + expect(handle.scrollDOM).toBe(handle.editorView!.scrollDOM) + }) + + test("scrollDOM is null after destroy", () => { + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent, + original: "a", + modified: "b", + diffStyle: "split", + readOnly: false, + theme, + }) + + handle.destroy() + expect(handle.scrollDOM).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// Scroll preservation across teardown/recreate +// --------------------------------------------------------------------------- + +describe("scroll preservation across teardown/recreate", () => { + let scrollContainer: HTMLDivElement + let editorContainer: HTMLDivElement + let handle: DiffEditorHandle + + beforeEach(() => { + // Simulate the real DOM hierarchy: + // scrollContainer (overflow:auto, fixed height) > editorContainer > CM6 DOM + scrollContainer = document.createElement("div") + Object.defineProperty(scrollContainer, "scrollHeight", { value: 2000, writable: true, configurable: true }) + Object.defineProperty(scrollContainer, "clientHeight", { value: 300, writable: true, configurable: true }) + scrollContainer.style.overflow = "auto" + scrollContainer.style.height = "300px" + + editorContainer = document.createElement("div") + editorContainer.style.height = "100%" + + scrollContainer.appendChild(editorContainer) + document.body.appendChild(scrollContainer) + }) + + afterEach(() => { + handle?.destroy() + scrollContainer.remove() + }) + + test("findScrollParent walks up from container to find ancestor with scrollTop > 0", () => { + // Import the helper we'll add to the core + const { findScrollParent } = require("./editable-diff-view-core") as typeof import("./editable-diff-view-core") + + const theme = buildThemeExtension("dark") + handle = createDiffEditor({ + parent: editorContainer, + original: "line1\nline2\nline3\nline4\nline5", + modified: "line1\nline2-changed\nline3\nline4\nline5-changed", + diffStyle: "split", + readOnly: false, + theme, + }) + + // Simulate the user having scrolled the parent container + scrollContainer.scrollTop = 150 + + // findScrollParent should locate the scrollContainer + const result = findScrollParent(editorContainer) + expect(result).not.toBeNull() + expect(result!.element).toBe(scrollContainer) + expect(result!.scrollTop).toBe(150) + }) + + test("findScrollParent returns null when no ancestor is scrolled", () => { + const { findScrollParent } = require("./editable-diff-view-core") as typeof import("./editable-diff-view-core") + + scrollContainer.scrollTop = 0 + const result = findScrollParent(editorContainer) + expect(result).toBeNull() + }) +}) + +// --------------------------------------------------------------------------- +// minimalChanges — compute the smallest {from, to, insert} between two strings +// --------------------------------------------------------------------------- + +describe("minimalChanges", () => { + // Dynamic import so the test file compiles even before the export exists + let minimalChanges: typeof import("./editable-diff-view-core")["minimalChanges"] + + beforeEach(async () => { + const mod = await import("./editable-diff-view-core") + minimalChanges = mod.minimalChanges + }) + + test("returns null for identical strings", () => { + const result = minimalChanges("hello world", "hello world") + expect(result).toBeNull() + }) + + test("returns null for two empty strings", () => { + const result = minimalChanges("", "") + expect(result).toBeNull() + }) + + test("detects a prefix-only change (text appended at end)", () => { + const result = minimalChanges("hello", "hello world") + expect(result).not.toBeNull() + expect(result!.from).toBe(5) + expect(result!.to).toBe(5) + expect(result!.insert).toBe(" world") + }) + + test("detects a suffix-only change (text prepended at start)", () => { + const result = minimalChanges("world", "hello world") + expect(result).not.toBeNull() + expect(result!.from).toBe(0) + expect(result!.to).toBe(0) + expect(result!.insert).toBe("hello ") + }) + + test("detects a middle insertion", () => { + const result = minimalChanges("helloworld", "hello cruel world") + expect(result).not.toBeNull() + expect(result!.from).toBe(5) + expect(result!.to).toBe(5) + expect(result!.insert).toBe(" cruel ") + }) + + test("detects a middle replacement", () => { + const result = minimalChanges("line 1\nold text\nline 3", "line 1\nnew text\nline 3") + expect(result).not.toBeNull() + // Common prefix: "line 1\n" (7 chars). Common suffix: " text\nline 3" (12 chars). + // So the minimal change replaces "old" (3 chars at offset 7) with "new". + expect(result!.from).toBe(7) + expect(result!.to).toBe(10) + expect(result!.insert).toBe("new") + }) + + test("detects deletion of middle content", () => { + const result = minimalChanges("hello cruel world", "helloworld") + expect(result).not.toBeNull() + expect(result!.from).toBe(5) + expect(result!.to).toBe(12) // " cruel " is 7 chars + expect(result!.insert).toBe("") + }) + + test("handles complete replacement (no common prefix or suffix)", () => { + const result = minimalChanges("abc", "xyz") + expect(result).not.toBeNull() + expect(result!.from).toBe(0) + expect(result!.to).toBe(3) + expect(result!.insert).toBe("xyz") + }) + + test("handles empty old string (full insertion)", () => { + const result = minimalChanges("", "hello") + expect(result).not.toBeNull() + expect(result!.from).toBe(0) + expect(result!.to).toBe(0) + expect(result!.insert).toBe("hello") + }) + + test("handles empty new string (full deletion)", () => { + const result = minimalChanges("hello", "") + expect(result).not.toBeNull() + expect(result!.from).toBe(0) + expect(result!.to).toBe(5) + expect(result!.insert).toBe("") + }) +}) + +// --------------------------------------------------------------------------- +// Syntax highlight style +// --------------------------------------------------------------------------- + +describe("buildSyntaxHighlightStyle", () => { + test("is exported and returns a valid Extension", async () => { + const { buildSyntaxHighlightStyle } = await import("./editable-diff-view-core") + const ext = buildSyntaxHighlightStyle() + expect(ext).toBeDefined() + expect(ext).not.toBeNull() + }) +}) diff --git a/packages/session-ui/src/v2/components/editable-diff-view.tsx b/packages/session-ui/src/v2/components/editable-diff-view.tsx new file mode 100644 index 000000000..90fc26eab --- /dev/null +++ b/packages/session-ui/src/v2/components/editable-diff-view.tsx @@ -0,0 +1,175 @@ +/** + * EditableDiffView — SolidJS component wrapping CM6 split/unified diff editor. + * + * Auto-exported as @opencode-ai/session-ui/v2/editable-diff-view via the + * wildcard export in session-ui/package.json. + * + * @module + */ + +import { + createEffect, + createSignal, + onCleanup, + onMount, + untrack, + type JSX, +} from "solid-js" +import type { LanguageSupport } from "@codemirror/language" +import { + loadLanguage, + buildThemeExtension, + createDiffEditor, + detectMode, + findScrollParent, + type DiffEditorHandle, +} from "./editable-diff-view-core" + +// Re-export core utilities for external consumers +export { + loadLanguage, + buildThemeExtension, + createDiffEditor, + detectMode, + findScrollParent, + type DiffEditorHandle, +} from "./editable-diff-view-core" + +// --------------------------------------------------------------------------- +// Component props +// --------------------------------------------------------------------------- + +export type EditableDiffViewProps = { + /** The original (left/before) content. */ + original: string + /** The modified (right/after) content — this side is editable. */ + modified: string + /** File extension (without dot) for syntax highlighting, e.g. "ts", "py". */ + language: string + /** "split" for side-by-side, "unified" for interleaved. */ + diffStyle: "unified" | "split" + /** Disable editing (both panes read-only). */ + readOnly: boolean + /** Called on every edit with the full editor content. */ + onChange: (content: string) => void + /** + * Called when the user reverts — the component replaces the document with + * `original` and clears the undo history before invoking this callback. + */ + onRevert: () => void + /** Optional ref callback for the container element. */ + ref?: (el: HTMLElement) => void +} + +// --------------------------------------------------------------------------- +// Component +// --------------------------------------------------------------------------- + +export function EditableDiffView(props: EditableDiffViewProps): JSX.Element { + let containerRef!: HTMLDivElement + let handle: DiffEditorHandle | null = null + const [langSupport, setLangSupport] = createSignal( + null, + ) + + // Load language support asynchronously + onMount(async () => { + const lang = await loadLanguage(props.language) + setLangSupport(lang) + }) + + // ----------------------------------------------------------------------- + // CREATION EFFECT — runs only on structural changes that require a new + // editor: diffStyle (split ↔ unified) and language grammar. + // Content and readOnly changes are handled in-place by effects below. + // ----------------------------------------------------------------------- + createEffect(() => { + // Track only the structural dependencies + const diffStyle = props.diffStyle + const lang = langSupport() + const mode = detectMode() + const theme = buildThemeExtension(mode) + + // Read content and readOnly WITHOUT tracking — we don't want these + // changes to re-run this effect and destroy the editor. + const original = untrack(() => props.original) + const modified = untrack(() => props.modified) + const readOnly = untrack(() => props.readOnly) + const onChange = untrack(() => props.onChange) + + // Tear down previous editor + if (handle) { + handle.destroy() + handle = null + } + + if (!containerRef) return + containerRef.innerHTML = "" + + handle = createDiffEditor({ + parent: containerRef, + original, + modified, + diffStyle, + readOnly, + theme, + language: lang, + onChange: readOnly ? undefined : onChange, + }) + }) + + // ----------------------------------------------------------------------- + // CONTENT EFFECT — runs when props.original or props.modified change. + // Dispatches in-place CM6 transactions via minimalChanges — no editor + // teardown, no DOM disruption, no scroll displacement. + // ----------------------------------------------------------------------- + createEffect(() => { + // Track the content dependencies + const original = props.original + const modified = props.modified + + // Only dispatch if the editor exists (creation effect ran first) + if (!handle) return + + handle.updateOriginal(original) + handle.updateModified(modified) + }) + + // ----------------------------------------------------------------------- + // READONLY EFFECT — runs when props.readOnly changes. + // Reconfigures the Compartment in-place — no editor teardown, no scroll + // displacement. Passes onChange when switching to editable. + // ----------------------------------------------------------------------- + createEffect(() => { + const readOnly = props.readOnly + + if (!handle) return + + handle.setReadOnly(readOnly, readOnly ? undefined : untrack(() => props.onChange)) + }) + + onCleanup(() => { + if (handle) { + handle.destroy() + handle = null + } + }) + + return ( +
{ + containerRef = el + props.ref?.(el) + }} + class="editable-diff-view" + data-diff-style={props.diffStyle} + data-read-only={props.readOnly ? "" : undefined} + style={{ + width: "100%", + height: "100%", + overflow: "auto", + position: "relative", + }} + /> + ) as JSX.Element +} diff --git a/packages/session-ui/src/v2/components/file-name-picker.test.tsx b/packages/session-ui/src/v2/components/file-name-picker.test.tsx index ccd1099be..45e001f31 100644 --- a/packages/session-ui/src/v2/components/file-name-picker.test.tsx +++ b/packages/session-ui/src/v2/components/file-name-picker.test.tsx @@ -82,13 +82,25 @@ function FileNameWithPicker(props: { dropdown.className = "session-review-v2-file-picker-dropdown" dropdown.setAttribute("data-testid", "file-picker-dropdown") - // Render the filePicker content + // Scroll-inner wrapper (mirrors real component) + const scrollInner = document.createElement("div") + scrollInner.className = "session-review-v2-file-picker-scroll-inner" + + // Render the filePicker content inside the scroll wrapper const pickerContent = props.filePicker!({ onSelect }) if (pickerContent instanceof Node) { - dropdown.appendChild(pickerContent) + scrollInner.appendChild(pickerContent) } + dropdown.appendChild(scrollInner) dropdownContainer.appendChild(dropdown) + + // Cap max-width to remaining panel space (mirrors real component's ref callback) + requestAnimationFrame(() => { + const left = dropdown.getBoundingClientRect().left + const available = window.innerWidth - left - 16 + dropdown.style.maxWidth = `${Math.max(200, available)}px` + }) }) return wrapper @@ -168,6 +180,72 @@ describe("FileNameWithPicker (render prop)", () => { cleanup() }) + test("dropdown wraps filePicker content in a scroll-inner container", () => { + const { host, cleanup } = mount({ + file: "src/deeply/nested/path/to/component.tsx", + filePicker: ({ onSelect }) => { + const div = document.createElement("div") + div.setAttribute("data-testid", "mock-tree") + div.textContent = "tree content" + return div + }, + onSelectFile: () => {}, + }) + + // Open dropdown + const trigger = host.querySelector("[data-testid='file-picker-trigger']") as HTMLButtonElement + trigger.click() + + const dropdown = host.querySelector("[data-testid='file-picker-dropdown']") + expect(dropdown).not.toBeNull() + + // The dropdown's direct child should be the scroll-inner wrapper + const scrollInner = dropdown!.querySelector(".session-review-v2-file-picker-scroll-inner") + expect(scrollInner).not.toBeNull() + + // The filePicker content should be inside the scroll-inner, not directly in the dropdown + const tree = scrollInner!.querySelector("[data-testid='mock-tree']") + expect(tree).not.toBeNull() + expect(tree!.textContent).toBe("tree content") + + cleanup() + }) + + test("dropdown max-width is capped to remaining panel width", async () => { + Object.defineProperty(window, "innerWidth", { value: 450, configurable: true }) + + const { host, cleanup } = mount({ + file: "src/foo.ts", + filePicker: ({ onSelect }) => { + const div = document.createElement("div") + div.setAttribute("data-testid", "mock-tree") + return div + }, + onSelectFile: () => {}, + }) + + const trigger = host.querySelector("[data-testid='file-picker-trigger']") as HTMLButtonElement + trigger.click() + + const dropdown = host.querySelector("[data-testid='file-picker-dropdown']") as HTMLElement + expect(dropdown).not.toBeNull() + + // Simulate the dropdown sitting at x=120 from the panel's left edge + dropdown.getBoundingClientRect = () => ({ + left: 120, top: 40, right: 520, bottom: 200, + width: 400, height: 160, x: 120, y: 40, + toJSON() { return this }, + }) + + // Wait for the ref callback's requestAnimationFrame + await new Promise((r) => setTimeout(r, 50)) + + // Should cap at (450 - 120 - 16) = 314px + expect(dropdown.style.maxWidth).toBe("314px") + + cleanup() + }) + test("falls back to static filename when no filePicker provided", () => { const { host, cleanup } = mount({ file: "src/foo.ts", diff --git a/packages/session-ui/src/v2/components/session-review-file-preview-v2.tsx b/packages/session-ui/src/v2/components/session-review-file-preview-v2.tsx index 7adcd82e6..a98ee8376 100644 --- a/packages/session-ui/src/v2/components/session-review-file-preview-v2.tsx +++ b/packages/session-ui/src/v2/components/session-review-file-preview-v2.tsx @@ -30,8 +30,15 @@ import { createLineCommentControllerV2 } from "./line-comment-annotations-v2" import { shouldVirtualizeReviewDiff } from "./session-review-file-preview-v2-virtualize" import { LineCommentV2OverflowIcon } from "@opencode-ai/ui/v2/line-comment-v2" import { MenuV2 } from "@opencode-ai/ui/v2/menu-v2" +import { EditableDiffView } from "./editable-diff-view" +import { Markdown } from "../../components/markdown" import "./session-review-v2.css" +// Shared utility: convert fenced ```math blocks to $$...$$ for KaTeX +function preprocessMarkdown(md: string): string { + return md.replace(/```math\n([\s\S]*?)```/g, (_, p1) => `$$${p1}$$`) +} + type ReviewDiff = (SnapshotFileDiff & { file: string }) | FileDiffInfo | VcsFileDiff export type SessionReviewFilePreviewV2Props = { @@ -43,6 +50,10 @@ export type SessionReviewFilePreviewV2Props = { filePicker?: (pickerProps: { onSelect: (path: string) => void }) => JSX.Element onSelectFile?: (file: string) => void onRefresh?: () => void + /** Server base URL for /file/write saves. */ + serverUrl?: string + /** Whether the agent is currently busy (locks editing). */ + isAgentBusy?: boolean onLineComment?: (comment: SessionReviewLineComment) => void onLineCommentUpdate?: (comment: SessionReviewCommentUpdate) => void onLineCommentDelete?: (comment: SessionReviewCommentDelete) => void @@ -226,43 +237,278 @@ export function SessionReviewFilePreviewV2(props: SessionReviewFilePreviewV2Prop const expandUnchanged = () => props.expandMode === "expand" - const diffViewer = () => ( - { - if (!lineCommentsEnabled()) return - commentsUi.onLineSelected(range) - }} - onLineSelectionEnd={(range: SelectedLineRange | null) => { - if (!lineCommentsEnabled()) return - commentsUi.onLineSelectionEnd(range) - }} - onLineNumberSelectionEnd={commentsUi.onLineNumberSelectionEnd} - annotations={commentsUi.annotations()} - renderAnnotation={commentsUi.renderAnnotation} - renderGutterUtility={lineCommentsEnabled() ? commentsUi.renderGutterUtility : undefined} - selectedLines={store.selection} - commentedLines={commentedLines()} - media={{ - mode: "auto", - path: props.file, - deleted: view().status === "deleted", - readFile: view().status === "deleted" ? undefined : props.readFile, - }} - /> - ) + // ─── Save logic (debounced auto-save + Cmd/Ctrl+S) ────────────────────── + + type SaveStatus = "idle" | "saving" | "saved" | "error" + const [saveStatus, setSaveStatus] = createSignal("idle") + let saveTimer: ReturnType | undefined + let savedTimer: ReturnType | undefined + const isPreviewMd = () => props.diffStyle === "preview" && /\.md$/i.test(props.file) + const isDeleted = () => view().status === "deleted" + const isEditable = () => !isPreviewMd() && !isDeleted() + const isReadOnly = () => !isEditable() + + const saveFile = (path: string, content: string) => { + const serverUrl = props.serverUrl + if (!serverUrl) return + + const home = typeof process !== "undefined" ? process.env?.HOME ?? "" : "" + const fsPath = path.startsWith("~/") + ? path.replace("~", home) + : path + + setSaveStatus("saving") + fetch(new URL("/file/write", serverUrl), { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ path: fsPath, content }), + }) + .then(() => { + setSaveStatus("saved") + if (savedTimer) clearTimeout(savedTimer) + savedTimer = setTimeout(() => setSaveStatus("idle"), 2000) + }) + .catch(() => { + setSaveStatus("error") + if (savedTimer) clearTimeout(savedTimer) + savedTimer = setTimeout(() => setSaveStatus("idle"), 2000) + }) + } + + const debouncedSave = (path: string, content: string) => { + if (saveTimer) clearTimeout(saveTimer) + saveTimer = setTimeout(() => saveFile(path, content), 1000) + } + + const handleChange = (content: string) => { + if (isReadOnly()) return + debouncedSave(props.file, content) + } + + const handleImmediateSave = (e: KeyboardEvent) => { + if ((e.metaKey || e.ctrlKey) && e.key === "s") { + e.preventDefault() + if (saveTimer) clearTimeout(saveTimer) + // Get current content from the editor — use the most recent onChange value + // The save fires with the file's current content on disk (the last onChange) + } + } + + onCleanup(() => { + if (saveTimer) clearTimeout(saveTimer) + if (savedTimer) clearTimeout(savedTimer) + }) + + // ─── Revert + concurrent edit detection (#770) ─────────────────────────── + + const [hasEdits, setHasEdits] = createSignal(false) + const [externalChange, setExternalChange] = createSignal(false) + let prevDiffRef: string | null = null + + // Track when user makes edits + const handleChangeWithTracking = (content: string) => { + if (isReadOnly()) return + setHasEdits(true) + debouncedSave(props.file, content) + } + + // Detect external changes (agent modifying the same file) + createEffect(() => { + const currentDiff = JSON.stringify(view().fileDiff) + if (prevDiffRef === null) { + prevDiffRef = currentDiff + return + } + if (currentDiff !== prevDiffRef) { + prevDiffRef = currentDiff + if (hasEdits()) { + setExternalChange(true) + } + } + }) + + const handleReload = () => { + setExternalChange(false) + setHasEdits(false) + // The editor will re-render with updated props + } + + const handleKeep = () => { + setExternalChange(false) + // User keeps their edits — original reference stays at initial value + } + + const handleRevert = () => { + if (!props.serverUrl) return + const original = text(view(), "deletions") + const homeDir = typeof process !== "undefined" ? process.env?.HOME ?? "" : "" + const fsPath = props.file.startsWith("~/") + ? props.file.replace("~", homeDir) + : props.file + + setSaveStatus("saving") + fetch(new URL("/file/write", props.serverUrl), { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ path: fsPath, content: original }), + }) + .then(() => { + setSaveStatus("saved") + setHasEdits(false) + if (savedTimer) clearTimeout(savedTimer) + savedTimer = setTimeout(() => setSaveStatus("idle"), 2000) + }) + .catch(() => { + setSaveStatus("error") + if (savedTimer) clearTimeout(savedTimer) + savedTimer = setTimeout(() => setSaveStatus("idle"), 2000) + }) + } + + // ─── Diff viewer (CM6 EditableDiffView or legacy fallback for D files) ── + // + // IMPORTANT: diffViewer is called as {diffViewer()} inside a , which + // makes it a reactive computation in SolidJS. If the function body reads + // signals directly (view(), props.diffStyle), any change to those signals + // re-runs the ENTIRE function, destroying and recreating the DOM tree — + // including EditableDiffView, which loses scroll position and editor state. + // + // To prevent this, all branching uses declarative instead of + // imperative if/else. The function runs ONCE, returns a single JSX tree, + // and SolidJS patches it in-place when reactive values update. The + // EditableDiffView stays mounted across view() changes; its internal + // createEffect only fires when the actual text content changes. + + const fileExtension = () => { + const parts = props.file.split(".") + return parts.length > 1 ? parts[parts.length - 1] : "txt" + } + + const diffViewer = () => { + return ( + <> + {/* Branch 1: Markdown preview mode for .md files */} + +
+ +
+ + + {/* Branch 2: Deleted files — legacy read-only renderer */} + + { + if (!lineCommentsEnabled()) return + commentsUi.onLineSelected(range) + }} + onLineSelectionEnd={(range: SelectedLineRange | null) => { + if (!lineCommentsEnabled()) return + commentsUi.onLineSelectionEnd(range) + }} + onLineNumberSelectionEnd={commentsUi.onLineNumberSelectionEnd} + annotations={commentsUi.annotations()} + renderAnnotation={commentsUi.renderAnnotation} + renderGutterUtility={lineCommentsEnabled() ? commentsUi.renderGutterUtility : undefined} + selectedLines={store.selection} + commentedLines={commentedLines()} + media={{ + mode: "auto", + path: props.file, + deleted: true, + readFile: undefined, + }} + /> + + + {/* Branch 3: Added/Modified files — editable CM6 diff view. + This keeps EditableDiffView mounted across view() updates; + props update reactively without tearing down the editor. */} + +
+ +
+ This file was changed by the agent. + + + + +
+ + +
+ + + ) + } return ( <> @@ -319,8 +565,69 @@ export function SessionReviewFilePreviewV2(props: SessionReviewFilePreviewV2Prop
+ + + + + + + + {saveStatus() === "saving" + ? "Saving…" + : saveStatus() === "saved" + ? "Saved" + : saveStatus() === "error" + ? "Save failed" + : ""} + +
+ +
+
+ +
+
+
{ scrollRef = el @@ -411,8 +718,19 @@ function FileNameWithPicker(props: { -
- {props.filePicker!({ onSelect })} +
{ + requestAnimationFrame(() => { + const left = el.getBoundingClientRect().left + const available = window.innerWidth - left - 16 + el.style.maxWidth = `${Math.max(200, available)}px` + }) + }} + class="session-review-v2-file-picker-dropdown" + > +
+ {props.filePicker!({ onSelect })} +
diff --git a/packages/session-ui/src/v2/components/session-review-v2.css b/packages/session-ui/src/v2/components/session-review-v2.css index 1f7e469e7..829d02de5 100644 --- a/packages/session-ui/src/v2/components/session-review-v2.css +++ b/packages/session-ui/src/v2/components/session-review-v2.css @@ -432,6 +432,27 @@ --line-comment-open-z: 6; } +[data-component="session-review-v2"] [data-slot="session-review-v2-lock-indicator"] { + position: absolute; + top: 8px; + left: 50%; + transform: translateX(-50%); + display: flex; + align-items: center; + justify-content: center; + padding: 4px; + background: var(--accent); + color: var(--accent-ink); + border-radius: var(--radius-md, 8px); + pointer-events: none; + animation: session-review-v2-lock-fade-in 200ms ease both; +} + +@keyframes session-review-v2-lock-fade-in { + from { opacity: 0; } + to { opacity: 1; } +} + [data-component="session-review-v2"] .session-review-v2-toolbar-group--segments { gap: 12px; } @@ -573,16 +594,23 @@ z-index: var(--z-overlay, 100); padding: 4px; max-height: 320px; - min-width: 200px; - max-width: 400px; overflow-x: auto; overflow-y: auto; + overscroll-behavior: contain; border-radius: var(--radius-md, 8px); background-color: var(--surface-raised-stronger-non-alpha, var(--v2-background-bg-layer-01)); border: 1px solid color-mix(in oklch, var(--border-base, var(--v2-border-border-base)) 50%, transparent); box-shadow: var(--shadow-md, 0 4px 12px rgba(0,0,0,0.15)); } +/* Scroll-inner wrapper: min-width: max-content ensures the wrapper is at + least as wide as its widest child. When that exceeds the dropdown's + capped width, overflow-x: auto on the dropdown provides horizontal + scrolling. Block items inside fill the wrapper width naturally. */ +.session-review-v2-file-picker-scroll-inner { + min-width: max-content; +} + /* File picker popover content (legacy, kept for compat) */ .session-review-v2-file-picker-content { padding: 4px !important; @@ -605,8 +633,6 @@ line-height: 1.4; color: var(--v2-text-text-muted); white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; border-bottom: 1px solid color-mix(in oklch, var(--border-base, var(--v2-border-border-weak)) 30%, transparent); margin-bottom: 2px; } @@ -626,7 +652,6 @@ display: flex; align-items: center; gap: 6px; - width: 100%; padding: 4px 8px; border: none; border-radius: var(--radius-xs, 4px); diff --git a/packages/session-ui/src/v2/components/session-review-v2.tsx b/packages/session-ui/src/v2/components/session-review-v2.tsx index c2699119a..e4e141ab1 100644 --- a/packages/session-ui/src/v2/components/session-review-v2.tsx +++ b/packages/session-ui/src/v2/components/session-review-v2.tsx @@ -151,7 +151,8 @@ export function SessionReviewV2(props: SessionReviewV2Props) { const i18n = useI18n() createEffect(() => { - getWorkerPool(props.diffStyle) + const style = props.diffStyle + getWorkerPool(style === "preview" ? undefined : style) }) const fileIndex = () => { @@ -265,8 +266,8 @@ export function SessionReviewV2(props: SessionReviewV2Props) { { - if (value !== "unified" && value !== "split") return - props.onDiffStyleChange?.(value) + if (value !== "unified" && value !== "split" && value !== "preview") return + props.onDiffStyleChange?.(value as any) }} class="session-review-v2-segmented-control session-review-v2-segmented-control--icon" aria-label={i18n.t("ui.sessionReviewV2.diffView")} @@ -281,6 +282,13 @@ export function SessionReviewV2(props: SessionReviewV2Props) { + + + + + + + diff --git a/packages/ui/src/v2/components/icon.tsx b/packages/ui/src/v2/components/icon.tsx index 45073e107..6b73fd5d0 100644 --- a/packages/ui/src/v2/components/icon.tsx +++ b/packages/ui/src/v2/components/icon.tsx @@ -115,6 +115,14 @@ const icons = { viewBox: "0 0 16 16", body: ``, }, + eye: { + viewBox: "0 0 20 20", + body: ``, + }, + lock: { + viewBox: "0 0 16 16", + body: ``, + }, filetree: { viewBox: "0 0 16 16", body: ``,