Skip to content

Commit 895c7db

Browse files
authored
fix(post-tool): scope Bash marker clear to the command's own files (#83) (#87)
1 parent 15cb8da commit 895c7db

5 files changed

Lines changed: 55 additions & 29 deletions

File tree

lua/code-preview/changes.lua

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,4 @@ function M.clear_by_status(status)
5454
end
5555
end
5656

57-
function M.clear_by_statuses(statuses)
58-
local set = {}
59-
for _, s in ipairs(statuses) do
60-
set[s] = true
61-
end
62-
for path, s in pairs(pending) do
63-
if set[s] then
64-
pending[path] = nil
65-
end
66-
end
67-
end
68-
6957
return M

lua/code-preview/post_tool.lua

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@
1111

1212
local M = {}
1313

14-
local normalisers = require("code-preview.pre_tool.normalisers")
15-
local changes = require("code-preview.changes")
16-
local neo_tree = require("code-preview.neo_tree")
17-
local diff = require("code-preview.diff")
18-
local log = require("code-preview.log")
19-
local apply_patch = require("code-preview.apply.patch")
14+
local normalisers = require("code-preview.pre_tool.normalisers")
15+
local changes = require("code-preview.changes")
16+
local neo_tree = require("code-preview.neo_tree")
17+
local diff = require("code-preview.diff")
18+
local log = require("code-preview.log")
19+
local apply_patch = require("code-preview.apply.patch")
20+
local shell_detect = require("code-preview.pre_tool.shell_detect")
2021

2122
-- Extract paths from both unified-diff (+++ lines) and custom-patch
2223
-- (*** Update/Add/Delete File:) formats.
@@ -60,9 +61,17 @@ function M.handle(raw, backend)
6061

6162
if tool_name == "Bash" then
6263
-- Bash pre-hook set deleted / bash_* markers without opening a preview.
63-
-- Clear those statuses specifically so we don't clobber `modified` markers
64-
-- from concurrent Edit/Write/ApplyPatch whose post-hook hasn't fired.
65-
changes.clear_by_statuses({ "deleted", "bash_modified", "bash_created" })
64+
-- Clear only THIS command's files, not every bash-owned marker: a global
65+
-- status sweep wiped the still-pending markers of concurrent Bash commands
66+
-- (issue #83). Detection is deterministic, so re-running it on the post
67+
-- payload yields exactly the paths pre_tool marked — mirroring how the
68+
-- ApplyPatch branch closes specific files via patch_paths. This also keeps
69+
-- concurrent Edit/Write `modified`/`created` markers untouched, since those
70+
-- paths never appear in a shell command's detection.
71+
local cmd = input.tool_input and input.tool_input.command or ""
72+
local detected = shell_detect.detect(cmd, input.cwd or "")
73+
for _, p in ipairs(detected.rm_paths) do changes.clear(p) end
74+
for _, p in ipairs(detected.write_paths) do changes.clear(p) end
6675
neo_tree.refresh_deferred(200)
6776
return ""
6877
end

tests/backends/opencode/harness.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// write_before <socket> <dir> <file> <content>
1010
// write_after <socket> <dir> <file>
1111
// bash_before <socket> <dir> <command>
12-
// bash_after <socket> <dir>
12+
// bash_after <socket> <dir> <command>
1313

1414
import { resolve, dirname } from "path"
1515
import { fileURLToPath } from "url"
@@ -94,8 +94,14 @@ async function main() {
9494
}
9595

9696
case "bash_after": {
97+
// OpenCode's real after-hook carries the tool args on `input` (see the
98+
// "tool.execute.after" comment in backends/opencode/index.ts), so the
99+
// command is available post-tool. Forward it here so the harness matches
100+
// production — the post-tool re-detects the command to clear only that
101+
// command's markers (issue #83).
102+
const command = process.argv[5]
97103
await afterHook(
98-
{ tool: "bash", args: {} },
104+
{ tool: "bash", args: { command } },
99105
{},
100106
)
101107
console.log("OK")

tests/backends/opencode/test_edit.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ test_opencode_bash_rm() {
115115
change_status="$(nvim_eval "require('code-preview.changes').get('$test_file')")"
116116
assert_eq "deleted" "$change_status" "rm target should be marked as deleted" || return 1
117117

118-
run_opencode bash_after "$TEST_SOCKET" "$TEST_PROJECT_DIR" >/dev/null 2>&1
118+
run_opencode bash_after "$TEST_SOCKET" "$TEST_PROJECT_DIR" "rm $test_file" >/dev/null 2>&1
119119
sleep 0.5
120120

121121
local change_after

tests/plugin/post_tool_handle_spec.lua

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
-- post_tool_handle_spec.lua — Smoke tests for the in-process post-tool.
22
--
33
-- post_tool.handle's contract:
4-
-- * Bash: clears `deleted` / `bash_modified` / `bash_created` markers from
5-
-- the changes registry; never touches `modified` / `created` markers
6-
-- from concurrent Edit/Write/ApplyPatch proposals.
4+
-- * Bash: clears the markers for the files THIS command touched (re-detected
5+
-- from the post payload), never every bash-owned marker — so concurrent
6+
-- pending Bash commands keep their markers (issue #83), and `modified` /
7+
-- `created` markers from concurrent Edit/Write/ApplyPatch are untouched.
78
-- * ApplyPatch: closes one preview per file referenced in the patch text.
89
-- * Other tools: closes the single preview keyed by file_path.
910
-- * Always returns "" (no backend reads post-tool stdout).
@@ -25,10 +26,10 @@ describe("post_tool.handle (Bash status clear)", function()
2526
assert.is_nil(changes.get("/proj/gone.txt"))
2627
end)
2728

28-
it("clears bash_modified / bash_created markers", function()
29+
it("clears the bash_modified / bash_created markers for files it touched", function()
2930
changes.set("/proj/a.txt", "bash_modified")
3031
changes.set("/proj/b.txt", "bash_created")
31-
post_tool.handle(payload("Bash", { command = "echo x > a.txt" }), "claudecode")
32+
post_tool.handle(payload("Bash", { command = "echo x > a.txt; echo y > b.txt" }), "claudecode")
3233
assert.is_nil(changes.get("/proj/a.txt"))
3334
assert.is_nil(changes.get("/proj/b.txt"))
3435
end)
@@ -44,6 +45,28 @@ describe("post_tool.handle (Bash status clear)", function()
4445
assert.equals("created", changes.get("/proj/new.lua"))
4546
assert.is_nil(changes.get("/proj/gone.txt"))
4647
end)
48+
49+
it("accepting one Bash delete keeps other pending Bash deletes marked", function()
50+
-- Regression for #83: two separate Bash deletes are pending, each with its
51+
-- own `deleted` marker. Accepting the first command (its PostToolUse fires)
52+
-- must clear ONLY that command's file; the still-pending command's marker
53+
-- survives. The old global status sweep wiped both.
54+
changes.set("/proj/a.txt", "deleted")
55+
changes.set("/proj/b.txt", "deleted")
56+
post_tool.handle(payload("Bash", { command = "rm a.txt" }), "claudecode")
57+
assert.is_nil(changes.get("/proj/a.txt"))
58+
assert.equals("deleted", changes.get("/proj/b.txt"))
59+
end)
60+
61+
it("scoped clear covers bash writes, not just deletes", function()
62+
-- A pending shell write (`echo x > b.txt` → bash_created/modified) must
63+
-- survive accepting a different command's write.
64+
changes.set("/proj/a.txt", "bash_modified")
65+
changes.set("/proj/b.txt", "bash_created")
66+
post_tool.handle(payload("Bash", { command = "echo x > a.txt" }), "claudecode")
67+
assert.is_nil(changes.get("/proj/a.txt"))
68+
assert.equals("bash_created", changes.get("/proj/b.txt"))
69+
end)
4770
end)
4871

4972
describe("post_tool.handle (return value)", function()

0 commit comments

Comments
 (0)