From 9611a0f25be4f770b46fe98a6fe0913da0950786 Mon Sep 17 00:00:00 2001 From: Jay Shitre Date: Tue, 9 Jun 2026 18:10:24 +0530 Subject: [PATCH 1/5] feat(windows): support GitHub Copilot CLI on Windows (#46) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot CLI hook entries support both a `bash` and a `powershell` field; Copilot selects the one matching the OS. The installer now writes both: `bash` runs hook-entry.sh on macOS/Linux (unchanged), `powershell` runs hook-entry.ps1 on Windows. This lifts the prior Windows deferral. Unlike Claude Code/Codex — where our installer writes the interpreter into the command field and gets Windows PowerShell 5.1 — Copilot runs the `powershell` field's string itself under pwsh 7+, so we emit a bare `& '' copilot ` invocation rather than reusing platform.hook_command's 5.1 `powershell -File` form. The existing .ps1 shims are 5.1-floor code and run unchanged under pwsh 7. - copilot.lua: emit both fields; add psquote() for PowerShell quoting; split the shim-path helper into .sh/.ps1 variants. - health.lua: drop the "not supported on Windows" warning. pwsh availability is Copilot CLI's own documented prerequisite, so — like Codex and Claude Code — no Windows-specific warning is added. - normalisers.lua: clarify (comment only) that Copilot's postToolUse toolArgs shape varies across OS/model/version; the branches accept both. - test_install.sh: assert the powershell field / hook-entry.ps1 are present. - CONTEXT.md: correct the glossary line that called this deferred. Validated end-to-end on Windows with Copilot CLI + GPT-5.4: edit, create, multi-file, and mixed delete+update apply_patch proposals (with absolute Windows paths) all open and close with balanced diff lifecycle. Unix behavior is unchanged — the bash field is byte-identical and the new powershell key is ignored off-Windows. Co-Authored-By: Claude Opus 4.8 --- CONTEXT.md | 2 +- lua/code-preview/backends/copilot.lua | 47 +++++++++++++++++------ lua/code-preview/health.lua | 11 +++--- lua/code-preview/pre_tool/normalisers.lua | 8 ++-- tests/backends/copilot/test_install.sh | 8 ++-- 5 files changed, 52 insertions(+), 24 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 4f0de1f..49a850b 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -77,7 +77,7 @@ The script the agent invokes directly when it's about to (or has just) used an e Job: take the agent's native hook payload, optionally fast-path-filter noisy tools (a backend-keyed branch inside the shim; the [normalisers](#core-handler) tool map is the source of truth), discover the running Neovim, splice the payload + backend name, and make one [RPC](#rpc) into the [core handler](#core-handler). -The hook entry is **per-OS** because that is a language boundary: a `.sh` shim on Unix, a PowerShell `.ps1` shim on Windows (issue #46). PowerShell is the single Windows logic language — the only stock-Windows-11 tool that parses JSON natively, enumerates named pipes, and probes the RPC socket; Windows PowerShell 5.1 (`powershell.exe`) is the floor, not pwsh 7. The installer writes the interpreter explicitly into the agent's `command` field (`powershell -NoProfile -ExecutionPolicy Bypass -File \hook-entry.ps1 `) via [`platform.hook_command`](docs/adr/0008-one-hook-entry-per-os.md). Copilot is the exception: its config uses a `bash` field, so it always invokes `hook-entry.sh` (Copilot-on-Windows would need git-bash, deferred). +The hook entry is **per-OS** because that is a language boundary: a `.sh` shim on Unix, a PowerShell `.ps1` shim on Windows (issue #46). PowerShell is the single Windows logic language — the only stock-Windows-11 tool that parses JSON natively, enumerates named pipes, and probes the RPC socket; Windows PowerShell 5.1 (`powershell.exe`) is the floor, not pwsh 7. The installer writes the interpreter explicitly into the agent's `command` field (`powershell -NoProfile -ExecutionPolicy Bypass -File \hook-entry.ps1 `) via [`platform.hook_command`](docs/adr/0008-one-hook-entry-per-os.md). Copilot is the exception: its hook entry carries *both* a `bash` field (runs `hook-entry.sh`, macOS/Linux) and a `powershell` field (runs `hook-entry.ps1`, Windows). Copilot runs the `powershell` field's string itself under **pwsh 7+** (not the 5.1 floor the other agents target), so the installer emits a bare `& '\hook-entry.ps1' ` invocation rather than reusing `platform.hook_command`. ## Core handler diff --git a/lua/code-preview/backends/copilot.lua b/lua/code-preview/backends/copilot.lua index dafe09b..eed4b12 100644 --- a/lua/code-preview/backends/copilot.lua +++ b/lua/code-preview/backends/copilot.lua @@ -12,20 +12,31 @@ end local platform = require("code-preview.platform") local function bin_dir() return plugin_root() .. "/bin" end --- Copilot's hook field is `bash` (the value runs under a bash shell), so it --- always invokes the .sh shim — the PowerShell-wrapped command form doesn't --- apply to this field shape. Copilot-on-Windows (which would need git-bash) --- is deferred (issue #46). -local function hook_script() return bin_dir() .. "/hook-entry.sh" end +-- Copilot's hook entry carries BOTH a `bash` and a `powershell` field (issue +-- #46). Copilot picks the one matching the OS: `bash` runs hook-entry.sh on +-- macOS/Linux, `powershell` runs hook-entry.ps1 on Windows. Unlike Claude +-- Code/Codex — where our installer writes the interpreter into the command and +-- gets Windows PowerShell 5.1 — Copilot runs the `powershell` field's string +-- itself under pwsh 7+, so we emit a bare `& '' …` invocation rather than +-- reusing platform.hook_command's 5.1 `powershell -File` form. +local function sh_hook_script() return bin_dir() .. "/hook-entry.sh" end +local function ps_hook_script() return bin_dir() .. "/hook-entry.ps1" end local function hooks_dir() return vim.fn.getcwd() .. "/.github/hooks" end local function config_path() return hooks_dir() .. "/code-preview.json" end --- Shell-quote a path for use inside the `bash` field of hooks.json. +-- Quote a path for the `bash` field (POSIX single-quote escaping). local function shquote(s) return "'" .. s:gsub("'", "'\\''") .. "'" end +-- Quote a path for the `powershell` field (PowerShell single-quote escaping: +-- a literal ' is doubled). Paired with the call operator (`& ''`) so +-- paths containing spaces invoke correctly. +local function psquote(s) + return "'" .. s:gsub("'", "''") .. "'" +end + -- True iff `path` looks like a code-preview.json our installer produced. We -- match on the hook-entry shim stem ("hook-entry"), with "code-preview-diff" -- kept so older per-backend installs are still recognised for uninstall after @@ -53,17 +64,31 @@ local function ensure_executable(path) end function M.install() - local hook = hook_script() - if not ensure_executable(hook) then return end + local sh_hook = sh_hook_script() + local ps_hook = ps_hook_script() + -- Both shims ship with the plugin; ensure_executable also chmods the .sh on + -- Unix (a no-op for the .ps1, which is invoked via an explicit interpreter). + if not ensure_executable(sh_hook) then return end + if not ensure_executable(ps_hook) then return end vim.fn.mkdir(hooks_dir(), "p") - -- The bash field runs the shim under bash with the backend + event args. + -- Each entry carries both fields so the hook fires on every OS: Copilot runs + -- `bash` on macOS/Linux and `powershell` (under pwsh 7+) on Windows. + local function entry(event) + return { + type = "command", + bash = shquote(sh_hook) .. " copilot " .. event, + powershell = "& " .. psquote(ps_hook) .. " copilot " .. event, + timeoutSec = 30, + } + end + local data = { version = 1, hooks = { - preToolUse = { { type = "command", bash = shquote(hook) .. " copilot pre", timeoutSec = 30 } }, - postToolUse = { { type = "command", bash = shquote(hook) .. " copilot post", timeoutSec = 30 } }, + preToolUse = { entry("pre") }, + postToolUse = { entry("post") }, }, } diff --git a/lua/code-preview/health.lua b/lua/code-preview/health.lua index df72010..8d681bc 100644 --- a/lua/code-preview/health.lua +++ b/lua/code-preview/health.lua @@ -147,12 +147,11 @@ function M.check() warn("copilot not found in PATH (install from https://github.com/github/copilot-cli)") end - -- Copilot uses the shared bin/hook-entry.sh (checked above) through its `bash` - -- hook field. On Windows that field needs git-bash, so Copilot-on-Windows is - -- deferred (issue #46). - if is_win then - warn("Copilot CLI on Windows is not yet supported (issue #46); use Claude Code on Windows") - end + -- Copilot's hook entry carries both a `bash` field (hook-entry.sh, macOS/Linux) + -- and a `powershell` field (hook-entry.ps1, Windows). On Windows Copilot runs + -- the powershell field under pwsh 7+; ensuring that interpreter is present is + -- Copilot CLI's own documented prerequisite, so — like Codex and Claude Code — + -- we add no Windows-specific warning here. -- hooks.json installed local copilot_hooks = vim.fn.getcwd() .. "/.github/hooks/code-preview.json" diff --git a/lua/code-preview/pre_tool/normalisers.lua b/lua/code-preview/pre_tool/normalisers.lua index 26ce41c..b6a0952 100644 --- a/lua/code-preview/pre_tool/normalisers.lua +++ b/lua/code-preview/pre_tool/normalisers.lua @@ -118,9 +118,11 @@ local COPILOT_TOOL_MAP = { bash = "Bash", } --- Copilot delivers `toolArgs` as a JSON-encoded string in preToolUse and as --- an object in postToolUse. For apply_patch the string IS the raw patch text --- (not JSON). For every other tool the string contains a JSON object with +-- Copilot delivers `toolArgs` as a JSON-encoded string in preToolUse. In +-- postToolUse the shape varies across OS / model / Copilot version — it may be +-- the same JSON string or a decoded object — so the branches below accept BOTH +-- rather than assume either. For apply_patch the string IS the raw patch text +-- (not JSON); for every other tool the string contains a JSON object with -- snake_case keys (path, old_str, new_str, file_text, command, ...). -- -- Note: file paths are run through the shared `resolve_path`, which collapses diff --git a/tests/backends/copilot/test_install.sh b/tests/backends/copilot/test_install.sh index 754ccc1..7083c11 100644 --- a/tests/backends/copilot/test_install.sh +++ b/tests/backends/copilot/test_install.sh @@ -36,9 +36,11 @@ test_install_copilot_hooks() { # Both hook events are registered with the right adapter scripts assert_contains "$content" "preToolUse" "should have preToolUse hook" || return 1 assert_contains "$content" "postToolUse" "should have postToolUse hook" || return 1 - assert_contains "$content" "hook-entry.sh" "should reference the generic hook-entry shim" || return 1 - assert_contains "$content" "copilot pre" "preToolUse should pass the pre event" || return 1 - assert_contains "$content" "copilot post" "postToolUse should pass the post event" || return 1 + assert_contains "$content" "hook-entry.sh" "should reference the Unix hook-entry shim" || return 1 + assert_contains "$content" "hook-entry.ps1" "should reference the Windows hook-entry shim" || return 1 + assert_contains "$content" "powershell" "should include a powershell field for Windows" || return 1 + assert_contains "$content" "copilot pre" "preToolUse should pass the pre event" || return 1 + assert_contains "$content" "copilot post" "postToolUse should pass the post event" || return 1 # Each event should have exactly one entry (no accidental duplication) local pre_count post_count From f83fbbc15d3f4e96871344b165ec669347277a0b Mon Sep 17 00:00:00 2001 From: Jay Shitre Date: Tue, 9 Jun 2026 18:30:08 +0530 Subject: [PATCH 2/5] fix(windows): don't double absolute paths in the copilot/opencode normaliser (#46) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The normaliser's resolve_path only treated a leading `/` as absolute, so a Windows drive-letter (`D:\…`) or UNC (`\…`) path was joined onto cwd and doubled (`D:\proj\D:\proj\file`). The doubled path fs_stats as missing, so the file is mis-marked "created", no diff renders, and a junk neo-tree node is injected. Surfaced with Gemini-class models via Copilot CLI, which route edits through the `edit`/`create` tools with an absolute `path` (GPT-class models use apply_patch, whose resolver was already fixed). This mirrors the same fix already in apply/patch.lua's resolve_path — there are now three resolvers that must agree on absolute-path detection; consolidating them is a worthwhile follow-up. - normalisers.lua: add is_absolute() (Unix /, Windows drive-letter, UNC) and use it in resolve_path. Benefits copilot and opencode (shared resolver). - pre_tool_normaliser_spec.lua: regression specs for drive-letter and UNC absolute paths not being doubled. Co-Authored-By: Claude Opus 4.8 --- lua/code-preview/pre_tool/normalisers.lua | 15 ++++++++++++++- tests/plugin/pre_tool_normaliser_spec.lua | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/lua/code-preview/pre_tool/normalisers.lua b/lua/code-preview/pre_tool/normalisers.lua index b6a0952..04166b9 100644 --- a/lua/code-preview/pre_tool/normalisers.lua +++ b/lua/code-preview/pre_tool/normalisers.lua @@ -57,10 +57,23 @@ local OPENCODE_TOOL_MAP = { -- Matches Node's path.resolve semantics the old TS plugin used; without it -- opencode keys could be raw "/proj/../escape.txt" strings that don't -- compare equal to claudecode-shaped keys for the same logical file. +-- An already-absolute path must NOT be joined onto cwd. Besides Unix `/`, we +-- recognise Windows drive-letter (`C:\` or `C:/`) and UNC (`\\server`) absolutes +-- — Gemini-class models route edits through Copilot's `edit` tool with an +-- absolute Windows `path`, and treating that as relative doubles it onto cwd +-- (`D:\proj\D:\proj\file`), which fs_stats as missing (file mis-marked +-- "created"), opens the diff at a bogus path, and injects a junk neo-tree node. +-- This mirrors the same fix in apply/patch.lua's resolve_path (issue #46). +local function is_absolute(p) + return p:sub(1, 1) == "/" -- Unix absolute + or p:match("^%a:[/\\]") ~= nil -- Windows drive-letter (C:\ or C:/) + or p:sub(1, 2) == "\\\\" -- Windows UNC (\\server\share) +end + local function resolve_path(p, cwd) if not p or p == "" then return p end local abs = p - if p:sub(1, 1) ~= "/" and cwd and cwd ~= "" then + if not is_absolute(p) and cwd and cwd ~= "" then abs = cwd .. "/" .. p end return vim.fs.normalize(abs) diff --git a/tests/plugin/pre_tool_normaliser_spec.lua b/tests/plugin/pre_tool_normaliser_spec.lua index e728a8d..7e81534 100644 --- a/tests/plugin/pre_tool_normaliser_spec.lua +++ b/tests/plugin/pre_tool_normaliser_spec.lua @@ -208,6 +208,27 @@ describe("normalisers.normalise (copilot)", function() assert.equals("/proj/src/rel.lua", out.tool_input.file_path) end) + -- Regression (issue #46): Gemini-class models route edits through Copilot's + -- `edit`/`create` tools with an ABSOLUTE Windows path. resolve_path must treat + -- a drive-letter / UNC path as already-absolute; otherwise it is joined onto + -- cwd (`D:/proj/D:/proj/file`), which fs_stats as missing → the file is + -- mis-marked "created", no diff renders, and a junk neo-tree node is injected. + it("does not double an absolute Windows drive-letter path", function() + local raw = copilot_pre("edit", { path = "D:/proj/sub/foo.lua", old_str = "a", new_str = "b" }) + raw.cwd = "D:/proj" + local out = normalisers.normalise(raw, "copilot") + assert.equals("Edit", out.tool_name) + assert.equals("D:/proj/sub/foo.lua", out.tool_input.file_path) + end) + + it("does not double an absolute UNC path", function() + local out = normalisers.normalise( + copilot_pre("create", { path = "\\\\srv\\share\\new.lua", file_text = "x" }), "copilot") + assert.equals("Write", out.tool_name) + -- Key regression assertion: cwd ("/proj") is NOT prepended. + assert.is_nil(out.tool_input.file_path:find("proj", 1, true)) + end) + it("noise / unknown tool yields nil tool_name", function() local out = normalisers.normalise( copilot_pre("view", { path = "/tmp/whatever" }), "copilot") From bb28764a22408fd57c8aade43c0e90c3ce155a3c Mon Sep 17 00:00:00 2001 From: Jay Shitre Date: Wed, 10 Jun 2026 10:18:10 +0530 Subject: [PATCH 3/5] fix(windows): map Copilot's `powershell` shell tool so deletes/writes are detected (#46) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, Copilot routes shell commands through a `powershell` tool (not `bash`) — confirmed with a Gemini-class model issuing `Remove-Item -Path "a","b" -ErrorAction Stop`. It was absent from COPILOT_TOOL_MAP, so the normaliser returned tool_name=nil and the proposal was silently dropped before shell_detect ran: no neo-tree delete indicator. The `powershell` tool carries the same {command, description} shape as `bash`, so it aliases to Bash; shell_detect's PowerShell grammar already parses Remove-Item (array -Path, -ErrorAction) correctly — verified end-to-end (normalise → Bash → detect → both rm_paths). - normalisers.lua: add `powershell = "Bash"` to COPILOT_TOOL_MAP. - pre_tool_normaliser_spec.lua: regression spec for the powershell shell tool. Co-Authored-By: Claude Opus 4.8 --- lua/code-preview/pre_tool/normalisers.lua | 8 ++++++++ tests/plugin/pre_tool_normaliser_spec.lua | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lua/code-preview/pre_tool/normalisers.lua b/lua/code-preview/pre_tool/normalisers.lua index 04166b9..afbf457 100644 --- a/lua/code-preview/pre_tool/normalisers.lua +++ b/lua/code-preview/pre_tool/normalisers.lua @@ -122,6 +122,13 @@ end -- before invoking us). `str_replace` and `edit` carry the same {path, -- old_str, new_str} shape; both alias to Edit. `create` and `write` both -- alias to Write (file_text vs content). +-- +-- Shell ops: on macOS/Linux Copilot uses `bash`; on Windows it uses +-- `powershell` (issue #46, observed with Gemini-class models — Remove-Item +-- deletes, git status, …). Both carry the same {command, description} shape and +-- alias to Bash, so shell_detect's PowerShell/POSIX grammar tells them apart. +-- Without the `powershell` entry, Windows shell deletes/writes arrive as +-- tool_name=nil and are silently dropped (no neo-tree indicator). local COPILOT_TOOL_MAP = { apply_patch = "ApplyPatch", edit = "Edit", @@ -129,6 +136,7 @@ local COPILOT_TOOL_MAP = { create = "Write", write = "Write", bash = "Bash", + powershell = "Bash", } -- Copilot delivers `toolArgs` as a JSON-encoded string in preToolUse. In diff --git a/tests/plugin/pre_tool_normaliser_spec.lua b/tests/plugin/pre_tool_normaliser_spec.lua index 7e81534..23cb6fb 100644 --- a/tests/plugin/pre_tool_normaliser_spec.lua +++ b/tests/plugin/pre_tool_normaliser_spec.lua @@ -181,6 +181,18 @@ describe("normalisers.normalise (copilot)", function() assert.equals("ls", out.tool_input.command) end) + -- Regression (issue #46): on Windows Copilot's shell tool is `powershell`, + -- not `bash` (observed with Gemini-class models). Same {command, description} + -- shape; must alias to Bash so shell_detect runs and marks neo-tree. Without + -- it, Remove-Item deletes arrive as tool_name=nil and are silently dropped. + it("powershell (Windows shell tool) maps to Bash with command", function() + local cmd = 'Remove-Item -Path "D:\\a\\x.txt", "D:\\b\\y.txt" -ErrorAction Stop' + local out = normalisers.normalise( + copilot_pre("powershell", { command = cmd, description = "delete temp files" }), "copilot") + assert.equals("Bash", out.tool_name) + assert.equals(cmd, out.tool_input.command) + end) + it("apply_patch treats toolArgs string as raw patch text (not JSON)", function() local out = normalisers.normalise({ toolName = "apply_patch", From 2b54fc072bcdbbcfbca0e574e8cce38d87064b49 Mon Sep 17 00:00:00 2001 From: Jay Shitre Date: Wed, 10 Jun 2026 11:36:35 +0530 Subject: [PATCH 4/5] chore(windows): address Copilot PR review (ensure-shim coupling, exec-policy note) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups for PR #81: - copilot.lua: verify/chmod only the OS-native shim in install() (matches the claudecode/codex installers) instead of failing when the non-native shim is absent. The config still references both shims; the non-native one always ships and is never executed on the current OS. - README: document that Copilot runs the Windows hook under pwsh 7+, and that a Restricted/AllSigned execution policy silently blocks the local hook script (pwsh's default RemoteSigned works). Gives users a place to look when hooks silently don't fire — the bare `& ''` field can't pass -ExecutionPolicy. Co-Authored-By: Claude Opus 4.8 --- README.md | 3 ++- lua/code-preview/backends/copilot.lua | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 1b7ef0e..3e5c806 100644 --- a/README.md +++ b/README.md @@ -452,7 +452,8 @@ vim.api.nvim_create_autocmd({ "FocusGained", "BufEnter", "CursorHold" }, { **Copilot CLI hooks not firing** - Run `:CodePreviewInstallCopilotCliHooks` in the project root - Verify `.github/hooks/code-preview.json` exists -- Ensure `jq` is in PATH +- Ensure `jq` is in PATH (macOS/Linux only; the Windows hook uses PowerShell's native JSON parsing) +- On Windows, Copilot runs the hook under **pwsh 7+** (its own requirement). If hooks silently don't fire, check your PowerShell execution policy with `Get-ExecutionPolicy` — pwsh's default `RemoteSigned` runs the local hook script, but `Restricted`/`AllSigned` blocks it (e.g. `Set-ExecutionPolicy -Scope CurrentUser RemoteSigned`) - Restart Copilot CLI (hooks are loaded at session start) **Diff doesn't close after rejecting** diff --git a/lua/code-preview/backends/copilot.lua b/lua/code-preview/backends/copilot.lua index eed4b12..b0b07a6 100644 --- a/lua/code-preview/backends/copilot.lua +++ b/lua/code-preview/backends/copilot.lua @@ -66,10 +66,11 @@ end function M.install() local sh_hook = sh_hook_script() local ps_hook = ps_hook_script() - -- Both shims ship with the plugin; ensure_executable also chmods the .sh on - -- Unix (a no-op for the .ps1, which is invoked via an explicit interpreter). - if not ensure_executable(sh_hook) then return end - if not ensure_executable(ps_hook) then return end + -- Verify (and, on Unix, chmod) only the OS-native shim — matches the + -- claudecode/codex installers. The config references both shims so it works + -- across OSes, but the non-native one is never executed here and always ships + -- with the plugin, so we don't fail the install on its account. + if not ensure_executable(bin_dir() .. "/hook-entry" .. platform.script_ext()) then return end vim.fn.mkdir(hooks_dir(), "p") From c6d1440c3b3199e655df367270b4a2d477845527 Mon Sep 17 00:00:00 2001 From: Jay Shitre Date: Wed, 10 Jun 2026 13:55:15 +0530 Subject: [PATCH 5/5] refactor(windows): consolidate absolute-path detection into platform.is_absolute (#46) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three resolvers carried logically-identical absolute-path checks (Unix /, Windows drive-letter, UNC): pre_tool/normalisers, apply/patch, and pre_tool/shell_detect. This PR added the third copy (the normaliser fix), so — per review — lift a single source of truth. - platform.lua: add is_absolute(p) (OS-independent string-shape check). - normalisers.lua, apply/patch.lua, shell_detect.lua: delegate to it. Behavior-preserving: the three checks were equivalent (shell_detect's win_is_absolute already included the git-bash `/` case the others have). shell_detect's looks_like_path token-shape matcher is intentionally left alone — it's a different concern (recognises relative forms too), not an absolute check. Specs: pre_tool_normaliser (41) + pre_tool_shell_detect (77) + apply_patch (5) all pass. Co-Authored-By: Claude Opus 4.8 --- lua/code-preview/apply/patch.lua | 16 ++++++++-------- lua/code-preview/platform.lua | 17 +++++++++++++++++ lua/code-preview/pre_tool/normalisers.lua | 22 ++++++++-------------- lua/code-preview/pre_tool/shell_detect.lua | 9 ++++++--- 4 files changed, 39 insertions(+), 25 deletions(-) diff --git a/lua/code-preview/apply/patch.lua b/lua/code-preview/apply/patch.lua index 20a600c..9387b6e 100644 --- a/lua/code-preview/apply/patch.lua +++ b/lua/code-preview/apply/patch.lua @@ -18,19 +18,19 @@ -- to files (the bin/apply-patch.lua shim does, pre_tool may use them -- differently in future). +local platform = require("code-preview.platform") + local M = {} --- Detect already-absolute paths before joining with cwd. Besides a Unix "/", --- Codex emits Windows-absolute paths in apply_patch directives (a drive-letter --- `D:\proj\file` / `D:/proj/file`, or a UNC `\\server\share`). Without these --- checks such a path is treated as relative and doubled onto cwd +-- Detect already-absolute paths before joining with cwd. Codex emits +-- Windows-absolute paths in apply_patch directives (drive-letter +-- `D:\proj\file` / `D:/proj/file`, or a UNC `\\server\share`); without this +-- check such a path is treated as relative and doubled onto cwd -- (`D:\proj\D:\proj\file`), which then fs_stats as missing (so the file is -- mis-marked "created"), opens the diff at a bogus path, and injects a junk --- neo-tree node. +-- neo-tree node. Detection is the shared platform.is_absolute (issue #46). local function resolve_path(path, cwd) - if path:sub(1, 1) == "/" -- Unix absolute - or path:match("^%a:[/\\]") -- Windows drive-letter absolute - or path:sub(1, 2) == "\\\\" then -- Windows UNC + if platform.is_absolute(path) then return path end return cwd .. "/" .. path diff --git a/lua/code-preview/platform.lua b/lua/code-preview/platform.lua index 7a01bfd..3de7dc8 100644 --- a/lua/code-preview/platform.lua +++ b/lua/code-preview/platform.lua @@ -43,6 +43,23 @@ function M.make_executable(path) end end +--- True if `p` is an already-absolute path. Recognises Unix-rooted (`/…`), +--- Windows drive-letter (`C:\` or `C:/`), and Windows UNC (`\\server\share`) +--- forms. Deliberately OS-INDEPENDENT — it inspects the string's shape, so it +--- recognises Windows-absolute paths even on Unix. Callers use it to avoid +--- joining an already-absolute path onto cwd, which would double it +--- (`D:\proj\D:\proj\file` → fs_stat miss → file mis-marked "created", no diff). +--- Single source of truth shared by the path resolvers in pre_tool/normalisers, +--- pre_tool/shell_detect, and apply/patch. +--- @param p string +--- @return boolean +function M.is_absolute(p) + if not p or p == "" then return false end + return p:sub(1, 1) == "/" -- Unix absolute + or p:match("^%a:[/\\]") ~= nil -- Windows drive-letter (C:\ or C:/) + or p:sub(1, 2) == "\\\\" -- Windows UNC (\\server\share) +end + --- The external dependency each OS's shim relies on, for health reporting: --- the Unix shims parse JSON with jq; the Windows shims use PowerShell's native --- ConvertFrom-Json (so jq is irrelevant there). diff --git a/lua/code-preview/pre_tool/normalisers.lua b/lua/code-preview/pre_tool/normalisers.lua index afbf457..4ee4083 100644 --- a/lua/code-preview/pre_tool/normalisers.lua +++ b/lua/code-preview/pre_tool/normalisers.lua @@ -13,6 +13,8 @@ -- so the opencode normaliser maps both into the canonical shape. New backends -- slot in by adding a function to the table. +local platform = require("code-preview.platform") + local M = {} local function identity(raw) @@ -57,23 +59,15 @@ local OPENCODE_TOOL_MAP = { -- Matches Node's path.resolve semantics the old TS plugin used; without it -- opencode keys could be raw "/proj/../escape.txt" strings that don't -- compare equal to claudecode-shaped keys for the same logical file. --- An already-absolute path must NOT be joined onto cwd. Besides Unix `/`, we --- recognise Windows drive-letter (`C:\` or `C:/`) and UNC (`\\server`) absolutes --- — Gemini-class models route edits through Copilot's `edit` tool with an --- absolute Windows `path`, and treating that as relative doubles it onto cwd --- (`D:\proj\D:\proj\file`), which fs_stats as missing (file mis-marked --- "created"), opens the diff at a bogus path, and injects a junk neo-tree node. --- This mirrors the same fix in apply/patch.lua's resolve_path (issue #46). -local function is_absolute(p) - return p:sub(1, 1) == "/" -- Unix absolute - or p:match("^%a:[/\\]") ~= nil -- Windows drive-letter (C:\ or C:/) - or p:sub(1, 2) == "\\\\" -- Windows UNC (\\server\share) -end - +-- An already-absolute path must NOT be joined onto cwd, or it doubles +-- (`D:\proj\D:\proj\file`): Gemini-class models route edits through Copilot's +-- `edit` tool with an absolute Windows `path`. Absolute-path detection lives in +-- the shared platform.is_absolute, so normalisers, shell_detect, and apply/patch +-- agree (issue #46). local function resolve_path(p, cwd) if not p or p == "" then return p end local abs = p - if not is_absolute(p) and cwd and cwd ~= "" then + if not platform.is_absolute(p) and cwd and cwd ~= "" then abs = cwd .. "/" .. p end return vim.fs.normalize(abs) diff --git a/lua/code-preview/pre_tool/shell_detect.lua b/lua/code-preview/pre_tool/shell_detect.lua index 940e940..a5ff542 100644 --- a/lua/code-preview/pre_tool/shell_detect.lua +++ b/lua/code-preview/pre_tool/shell_detect.lua @@ -28,6 +28,8 @@ -- pre-tool detection logic; resist "obvious simplifications" without first -- reading shell_detect_spec.lua. +local platform = require("code-preview.platform") + local M = {} local function is_windows() @@ -121,10 +123,11 @@ end local win_paths = {} +-- Absolute on Windows: drive-letter (C:\/C:/), UNC (\\server), or unix-rooted +-- (`/…`, git-bash). Delegates to the shared platform.is_absolute so every path +-- resolver agrees on what counts as absolute (issue #46). local function win_is_absolute(p) - return p:match("^%a:[\\/]") ~= nil -- drive-letter: C:\ or C:/ - or p:match("^\\\\") ~= nil -- UNC: \\server\share - or p:sub(1, 1) == "/" -- unix-rooted (git-bash) + return platform.is_absolute(p) end -- Canonicalise to backslash separators with ./.. collapsed.