diff --git a/lua/code-preview/changes.lua b/lua/code-preview/changes.lua index 4134daf..9cccbfd 100644 --- a/lua/code-preview/changes.lua +++ b/lua/code-preview/changes.lua @@ -54,16 +54,4 @@ function M.clear_by_status(status) end end -function M.clear_by_statuses(statuses) - local set = {} - for _, s in ipairs(statuses) do - set[s] = true - end - for path, s in pairs(pending) do - if set[s] then - pending[path] = nil - end - end -end - return M diff --git a/lua/code-preview/post_tool.lua b/lua/code-preview/post_tool.lua index b8007c7..0060216 100644 --- a/lua/code-preview/post_tool.lua +++ b/lua/code-preview/post_tool.lua @@ -11,12 +11,13 @@ local M = {} -local normalisers = require("code-preview.pre_tool.normalisers") -local changes = require("code-preview.changes") -local neo_tree = require("code-preview.neo_tree") -local diff = require("code-preview.diff") -local log = require("code-preview.log") -local apply_patch = require("code-preview.apply.patch") +local normalisers = require("code-preview.pre_tool.normalisers") +local changes = require("code-preview.changes") +local neo_tree = require("code-preview.neo_tree") +local diff = require("code-preview.diff") +local log = require("code-preview.log") +local apply_patch = require("code-preview.apply.patch") +local shell_detect = require("code-preview.pre_tool.shell_detect") -- Extract paths from both unified-diff (+++ lines) and custom-patch -- (*** Update/Add/Delete File:) formats. @@ -60,9 +61,17 @@ function M.handle(raw, backend) if tool_name == "Bash" then -- Bash pre-hook set deleted / bash_* markers without opening a preview. - -- Clear those statuses specifically so we don't clobber `modified` markers - -- from concurrent Edit/Write/ApplyPatch whose post-hook hasn't fired. - changes.clear_by_statuses({ "deleted", "bash_modified", "bash_created" }) + -- Clear only THIS command's files, not every bash-owned marker: a global + -- status sweep wiped the still-pending markers of concurrent Bash commands + -- (issue #83). Detection is deterministic, so re-running it on the post + -- payload yields exactly the paths pre_tool marked — mirroring how the + -- ApplyPatch branch closes specific files via patch_paths. This also keeps + -- concurrent Edit/Write `modified`/`created` markers untouched, since those + -- paths never appear in a shell command's detection. + local cmd = input.tool_input and input.tool_input.command or "" + local detected = shell_detect.detect(cmd, input.cwd or "") + for _, p in ipairs(detected.rm_paths) do changes.clear(p) end + for _, p in ipairs(detected.write_paths) do changes.clear(p) end neo_tree.refresh_deferred(200) return "" end diff --git a/tests/backends/opencode/harness.ts b/tests/backends/opencode/harness.ts index 7d70102..e317b96 100644 --- a/tests/backends/opencode/harness.ts +++ b/tests/backends/opencode/harness.ts @@ -9,7 +9,7 @@ // write_before // write_after // bash_before -// bash_after +// bash_after import { resolve, dirname } from "path" import { fileURLToPath } from "url" @@ -94,8 +94,14 @@ async function main() { } case "bash_after": { + // OpenCode's real after-hook carries the tool args on `input` (see the + // "tool.execute.after" comment in backends/opencode/index.ts), so the + // command is available post-tool. Forward it here so the harness matches + // production — the post-tool re-detects the command to clear only that + // command's markers (issue #83). + const command = process.argv[5] await afterHook( - { tool: "bash", args: {} }, + { tool: "bash", args: { command } }, {}, ) console.log("OK") diff --git a/tests/backends/opencode/test_edit.sh b/tests/backends/opencode/test_edit.sh index fc819fa..0b0ff99 100644 --- a/tests/backends/opencode/test_edit.sh +++ b/tests/backends/opencode/test_edit.sh @@ -115,7 +115,7 @@ test_opencode_bash_rm() { change_status="$(nvim_eval "require('code-preview.changes').get('$test_file')")" assert_eq "deleted" "$change_status" "rm target should be marked as deleted" || return 1 - run_opencode bash_after "$TEST_SOCKET" "$TEST_PROJECT_DIR" >/dev/null 2>&1 + run_opencode bash_after "$TEST_SOCKET" "$TEST_PROJECT_DIR" "rm $test_file" >/dev/null 2>&1 sleep 0.5 local change_after diff --git a/tests/plugin/post_tool_handle_spec.lua b/tests/plugin/post_tool_handle_spec.lua index 71a3377..4a7cd79 100644 --- a/tests/plugin/post_tool_handle_spec.lua +++ b/tests/plugin/post_tool_handle_spec.lua @@ -1,9 +1,10 @@ -- post_tool_handle_spec.lua — Smoke tests for the in-process post-tool. -- -- post_tool.handle's contract: --- * Bash: clears `deleted` / `bash_modified` / `bash_created` markers from --- the changes registry; never touches `modified` / `created` markers --- from concurrent Edit/Write/ApplyPatch proposals. +-- * Bash: clears the markers for the files THIS command touched (re-detected +-- from the post payload), never every bash-owned marker — so concurrent +-- pending Bash commands keep their markers (issue #83), and `modified` / +-- `created` markers from concurrent Edit/Write/ApplyPatch are untouched. -- * ApplyPatch: closes one preview per file referenced in the patch text. -- * Other tools: closes the single preview keyed by file_path. -- * Always returns "" (no backend reads post-tool stdout). @@ -25,10 +26,10 @@ describe("post_tool.handle (Bash status clear)", function() assert.is_nil(changes.get("/proj/gone.txt")) end) - it("clears bash_modified / bash_created markers", function() + it("clears the bash_modified / bash_created markers for files it touched", function() changes.set("/proj/a.txt", "bash_modified") changes.set("/proj/b.txt", "bash_created") - post_tool.handle(payload("Bash", { command = "echo x > a.txt" }), "claudecode") + post_tool.handle(payload("Bash", { command = "echo x > a.txt; echo y > b.txt" }), "claudecode") assert.is_nil(changes.get("/proj/a.txt")) assert.is_nil(changes.get("/proj/b.txt")) end) @@ -44,6 +45,28 @@ describe("post_tool.handle (Bash status clear)", function() assert.equals("created", changes.get("/proj/new.lua")) assert.is_nil(changes.get("/proj/gone.txt")) end) + + it("accepting one Bash delete keeps other pending Bash deletes marked", function() + -- Regression for #83: two separate Bash deletes are pending, each with its + -- own `deleted` marker. Accepting the first command (its PostToolUse fires) + -- must clear ONLY that command's file; the still-pending command's marker + -- survives. The old global status sweep wiped both. + changes.set("/proj/a.txt", "deleted") + changes.set("/proj/b.txt", "deleted") + post_tool.handle(payload("Bash", { command = "rm a.txt" }), "claudecode") + assert.is_nil(changes.get("/proj/a.txt")) + assert.equals("deleted", changes.get("/proj/b.txt")) + end) + + it("scoped clear covers bash writes, not just deletes", function() + -- A pending shell write (`echo x > b.txt` → bash_created/modified) must + -- survive accepting a different command's write. + changes.set("/proj/a.txt", "bash_modified") + changes.set("/proj/b.txt", "bash_created") + post_tool.handle(payload("Bash", { command = "echo x > a.txt" }), "claudecode") + assert.is_nil(changes.get("/proj/a.txt")) + assert.equals("bash_created", changes.get("/proj/b.txt")) + end) end) describe("post_tool.handle (return value)", function()