Skip to content

Commit 6c6915e

Browse files
poteatpoteat
authored
diff: a shell command the tool held read-only fetches nothing, as the built-in panel's touch is gated (#95423)
1 parent 384e28e commit 6c6915e

6 files changed

Lines changed: 74 additions & 3 deletions

File tree

‎mods/diff/hooks/register.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -927,9 +927,7 @@ export function register(on: On) {
927927
result.deny === undefined &&
928928
result.isError !== true
929929

930-
const hasLanded = isEdit
931-
? hasEdited
932-
: result === undefined || result.deny === undefined
930+
const hasLanded = isEdit ? hasEdited : Tools.mayHaveWritten(result)
933931

934932
if (hasLanded) {
935933
landed += 1

‎mods/diff/hooks/tools/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
export * from './editing-tools.js'
2+
export * from './may-have-written.js'
23
export * from './shell-tools.js'
34
export * from './todo-tool.js'
45

‎mods/diff/hooks/tools/may-have-written.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import type { ResultOf } from 'claude-code'
2+
3+
/**
4+
* Whether a shell tool's outcome may have changed the working tree, as the
5+
* built-in gates its refresh: a throw or an answer, unless the engine says
6+
* the tool held the call read-only (`ls`, `git status`); never a deny.
7+
*
8+
* `isReadOnly` is read through a type that may lack it, ahead of the
9+
* declarations that name it; absent, the call counts as a write.
10+
*
11+
* @param result what `next` resolved to, or undefined when it threw
12+
* @returns true when a refresh is due
13+
*/
14+
export function mayHaveWritten(
15+
result: ResultOf['tool.call'] | undefined,
16+
): boolean {
17+
const settled: { deny?: string; isReadOnly?: boolean } | undefined = result
18+
19+
return (
20+
settled === undefined ||
21+
(settled.deny === undefined && settled.isReadOnly !== true)
22+
)
23+
}

‎mods/diff/tests/fixtures/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export * from './pane.js'
4040
export * from './pane-view'
4141
export * from './parse-file-diff'
4242
export * from './poll-word.js'
43+
export * from './read-only-answer.js'
4344
export * from './renamed.js'
4445
export * from './repository.js'
4546
export * from './repository-of'

‎mods/diff/tests/fixtures/read-only-answer.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import type { ResultOf } from 'claude-code'
2+
3+
/**
4+
* A shell tool's answer as the engine gives it for a command the tool holds
5+
* read-only (`ls`, `git status`): the result, and `isReadOnly` beside it.
6+
*
7+
* Built through Object.assign, since the declarations may not name the
8+
* field yet; the plugin reads it through a type that may lack it too.
9+
*/
10+
export const READ_ONLY_ANSWER: ResultOf['tool.call'] = Object.assign(
11+
{ result: 'listed' },
12+
{ isReadOnly: true as const },
13+
)

‎mods/diff/tests/register.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,41 @@ describe('register', () => {
121121
)
122122
})
123123

124+
test('a shell command the tool held read-only fetches nothing', async ($, on) => {
125+
const world = Fixtures.inRepository(on)
126+
127+
const fetchesSince = (read: number) =>
128+
world.runs.slice(read).filter(run => run.argv.includes('--numstat'))
129+
.length
130+
131+
on('tool.call', ($, e) =>
132+
e.tool === 'Bash' && e.command === 'ls'
133+
? Fixtures.READ_ONLY_ANSWER
134+
: { result: 'done' },
135+
)
136+
137+
await $.session.start(Fixtures.SESSION)
138+
await $.command.run(Fixtures.DIFF)
139+
await world.clock.advance(Fixtures.SETTLE_MS)
140+
141+
const opened = world.runs.length
142+
143+
await $.tool.call({ tool: 'Bash', command: 'ls' })
144+
await world.clock.advance(Fixtures.SETTLE_MS)
145+
146+
expect(
147+
fetchesSince(opened),
148+
"read-only, the pane open: nothing, as the built-in's touch is gated",
149+
).toBe(0)
150+
151+
const listed = world.runs.length
152+
153+
await $.tool.call({ tool: 'Bash', command: 'make' })
154+
await world.clock.advance(Fixtures.SETTLE_MS)
155+
156+
expect(fetchesSince(listed), 'a command that may write: one fetch').toBe(1)
157+
})
158+
124159
test('/diff whose probe never answers probes once more', async ($, on) => {
125160
const probes: (readonly string[])[] = []
126161
const clock = Fixtures.startsSession(on)

0 commit comments

Comments
 (0)