Skip to content

Commit 2287e5d

Browse files
poteatpoteat
authored
diff: the first edit opens the pane only from the main loop with checkpointing on, and an open the engine leaves waiting is withdrawn (#95476)
1 parent 6ce37e9 commit 2287e5d

12 files changed

Lines changed: 337 additions & 30 deletions

File tree

‎mods/diff/README.md‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@ built-in's list keys (`ctrl+up`/`ctrl+down`, `opt+up`/`opt+down`), and
1313
through Buttons that declare the engine's own actions. The pane refreshes
1414
as Claude edits and runs shell commands, and while it is open it polls
1515
the repository's HEAD so a commit or checkout made elsewhere shows too.
16-
The first successful edit of a session opens the pane by itself where the
16+
The main loop's first successful edit of a session opens the pane by
17+
itself, as the built-in panel opens on its first checkpoint: where the
1718
layout docks it beside the transcript (the fullscreen layout, which each
18-
drawing's `viewport` says) and the terminal is wide enough (144 columns
19-
when the person never chose, 110 when they kept it open before; a person
20-
who closed it is left alone); where the surface does not say, nothing
21-
opens by itself.
19+
drawing's `viewport` says), the terminal is wide enough (144 columns when
20+
the person never chose, 110 when they kept it open before; a person who
21+
closed it is left alone) and file checkpointing is on; a subagent's edit
22+
opens nothing, and where the surface does not say, nothing opens by
23+
itself. An open the engine leaves waiting undrawn is withdrawn, so no
24+
later resize seats it, and the next edit asks again.
2225

2326
Under the fullscreen layout a terminal under 110 columns gets the
2427
built-in's line asking for a wider one and nothing opens. Without that
@@ -68,16 +71,17 @@ moved file by.
6871
| `ui.scroll` of the pane | Docked, moves the hunks under the pinned header and list (three rows a wheel tick, a page a page key), or the list when the wheel is over it, and keeps the engine's window still. |
6972
| `ui.focus` in the pane | In the dialog's list, selects the file the ring lands on, re-centres the five rows on it, and lands the ring where that row now sits. |
7073
| `command.run` of `clear`, `resume` | Closes the pane and forgets the session's state, the pinned repository with it. |
71-
| `tool.call` of `Edit`, `Write`, `NotebookEdit` | After an edit that landed (not refused, not failed), refreshes an open pane; the session's first such edit opens it, pinning the repository then if the terminal has the room. |
74+
| `tool.call` of `Edit`, `Write`, `NotebookEdit` | After an edit that landed (not refused, not failed), refreshes an open pane; the main loop's first such edit opens it, pinning the repository then if the terminal has the room and checkpointing is on. |
7275
| `tool.call` of `Bash`, `PowerShell` | After a command that was not refused, failed and interrupted ones too, refreshes an open pane. |
7376
| `prompt.submit` | Adds the armed file's hunks to the prompt's context and disarms. |
7477

7578
## What it calls on `$`
7679

77-
`clock.after`, `clock.every`, `clock.now`, `command.register`, `fs.list`,
78-
`fs.read`, `fs.stat`, `process.run` (git, read-only), `session.messages`,
79-
`store.get`, `store.set`, `telemetry.log`, `telemetry.mark`, `ui.close`,
80-
`ui.invalidate`, `ui.log`, `ui.open`, `ui.resolve`, `ui.status`.
80+
`clock.after`, `clock.every`, `clock.now`, `command.register`, `env.get`
81+
(`CLAUDE_CODE_DISABLE_FILE_CHECKPOINTING`), `fs.list`, `fs.read`, `fs.stat`,
82+
`process.run` (git, read-only), `session.id`, `session.messages`,
83+
`settings.read`, `store.get`, `store.set`, `telemetry.log`, `telemetry.mark`,
84+
`ui.close`, `ui.invalidate`, `ui.log`, `ui.open`, `ui.resolve`, `ui.status`.
8185

8286
`$.telemetry` is the telemetry plugin's noun; where it is absent the rows
8387
are dropped and nothing else changes.

‎mods/diff/hooks/host/host.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ export type Host = {
6464
*/
6565
storeSet: (key: string, value: unknown) => Promise<void>
6666

67+
/**
68+
* Whether the session checkpoints edits (`$.settings.read`, `$.env.get`):
69+
* the built-in panel opens on an edit only while it does.
70+
*/
71+
isCheckpointing: () => Promise<boolean>
72+
6773
/**
6874
* `$.session.messages`.
6975
*/

‎mods/diff/hooks/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export * from './drawn-files-of'
88
export * from './entry-kinds-of'
99
export * from './git'
1010
export * from './host'
11+
export * from './is-checkpointing'
1112
export * from './is-on-pane-surface'
1213
export * from './is-record'
1314
export * from './kept-of'

‎mods/diff/hooks/is-checkpointing/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export * from './is-checkpointing.js'
2+
3+
export * as default from '.'

‎mods/diff/hooks/is-checkpointing/is-checkpointing.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import type { Settings } from 'claude-code'
2+
3+
/**
4+
* Whether the session checkpoints Claude's edits, read as the built-in
5+
* reads it: the setting on unless set false, the variable unset or falsy.
6+
*
7+
* The built-in panel opens on an edit only through a checkpoint, so with
8+
* checkpointing off its first-edit open never happens; `/diff` still opens.
9+
*
10+
* @param settings the merged settings (`$.settings.read()`)
11+
* @param disabling `CLAUDE_CODE_DISABLE_FILE_CHECKPOINTING` as `$.env.get`
12+
* answers it
13+
* @returns false when either turns checkpointing off
14+
*/
15+
export const isCheckpointing = (
16+
settings: Settings,
17+
disabling: string | undefined,
18+
): boolean =>
19+
settings.fileCheckpointingEnabled !== false &&
20+
!['1', 'true', 'yes', 'on'].includes((disabling ?? '').trim().toLowerCase())

‎mods/diff/hooks/register.ts‎

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type {
2+
Args,
23
On,
34
PaneOpenArgs,
45
ResultOf,
@@ -13,7 +14,9 @@ import { drawnFilesOf } from './drawn-files-of'
1314
import { entryKindsOf } from './entry-kinds-of'
1415
import type Git from './git'
1516
import type { Host } from './host'
17+
import { isCheckpointing } from './is-checkpointing'
1618
import { isOnPaneSurface } from './is-on-pane-surface'
19+
import { isRecord } from './is-record'
1720
import Limits from './limits'
1821
import { mapLimited } from './map-limited'
1922
import { messageOf } from './message-of'
@@ -30,8 +33,8 @@ import Views from './views'
3033
* pane's drawing and refresh, its opening on Claude's first edit, the ask.
3134
*
3235
* Git runs when the built-in's would: `session.start` binds the host and
33-
* registers `/diff`; `/diff` or the first edit with room pins the backend
34-
* where the session started, until `/clear`; an open pane alone fetches.
36+
* registers `/diff`; `/diff` or the main loop's first checkpointed edit with
37+
* room pins the backend, until `/clear`; only a placed, open pane fetches.
3538
*
3639
* @param on the engine's registrar
3740
*/
@@ -394,7 +397,7 @@ export function register(on: On) {
394397
async function openPane(
395398
engine: Host,
396399
trigger: (typeof Record.SHOWN_TRIGGERS)[number],
397-
): Promise<void> {
400+
): Promise<boolean> {
398401
const isDialog = model.isFullscreen === false
399402

400403
model = {
@@ -406,12 +409,20 @@ export function register(on: On) {
406409

407410
dialogRows = isDialog ? Views.dialogRowsOf(model) : null
408411

409-
await engine.openPane(
412+
const opened = await engine.openPane(
410413
isDialog
411414
? { ...dialogPane(), focus: true }
412415
: { id: Names.PANE_ID, title: Names.PANE_TITLE, holdToasts: true },
413416
)
414417

418+
const isWaiting = isRecord(opened) && opened.isPlaced === false
419+
420+
if (isWaiting) {
421+
await engine.closePane({ id: Names.PANE_ID }).catch(() => undefined)
422+
423+
return false
424+
}
425+
415426
isPaneOpen = true
416427

417428
const sessionId = await engine.sessionId().catch(() => null)
@@ -422,6 +433,8 @@ export function register(on: On) {
422433
}
423434

424435
void refresh(engine)
436+
437+
return true
425438
}
426439

427440
async function closePane(engine: Host): Promise<void> {
@@ -460,14 +473,20 @@ export function register(on: On) {
460473
return
461474
}
462475

476+
const isCheckpointed = await engine.isCheckpointing().catch(() => true)
477+
478+
if (!isCheckpointed || isTaken()) {
479+
return
480+
}
481+
463482
await pinBackend(engine)
464483

465484
if (!backend || isTaken()) {
466485
return
467486
}
468487

469488
hasAutoOpened = true
470-
await openPane(engine, 'auto_open')
489+
hasAutoOpened = await openPane(engine, 'auto_open')
471490
}
472491

473492
function disarm(engine: Host) {
@@ -602,6 +621,11 @@ export function register(on: On) {
602621
readFile: path => $.fs.read(path),
603622
storeGet: key => $.store.get(key),
604623
storeSet: (key, value) => $.store.set(key, value),
624+
isCheckpointing: async () =>
625+
isCheckpointing(
626+
await $.settings.read(),
627+
await $.env.get('CLAUDE_CODE_DISABLE_FILE_CHECKPOINTING'),
628+
),
605629
messages: () => $.session.messages(),
606630
invalidate: () => $.ui.invalidate('ui.render'),
607631
status: text => $.ui.status(text),
@@ -699,7 +723,14 @@ export function register(on: On) {
699723
}
700724

701725
const isOpening = toggle === 'open'
702-
await (isOpening ? openPane(host, 'manual') : closePane(host))
726+
727+
const isDone = isOpening
728+
? await openPane(host, 'manual')
729+
: await closePane(host).then(() => true)
730+
731+
if (!isDone) {
732+
return { text: Names.RESIZE_TERMINAL_TEXT }
733+
}
703734

704735
if (!isFullscreen) {
705736
return isOpening ? {} : { text: Names.DIALOG_DISMISSED_TEXT }
@@ -818,10 +849,10 @@ export function register(on: On) {
818849

819850
function afterTool(
820851
engine: Host,
821-
tool: string,
852+
e: Args<'tool.call'>,
822853
result: ResultOf['tool.call'] | undefined,
823854
) {
824-
const isEdit = Tools.EDITING_TOOLS.some(name => name === tool)
855+
const isEdit = Tools.EDITING_TOOLS.some(name => name === e.tool)
825856

826857
const hasEdited =
827858
isEdit &&
@@ -837,7 +868,9 @@ export function register(on: On) {
837868
scheduleRefresh(engine)
838869
}
839870

840-
if (hasEdited) {
871+
const isMainLoopEdit = hasEdited && e.agentId === undefined
872+
873+
if (isMainLoopEdit) {
841874
void openOnFirstEdit(engine).catch(() => undefined)
842875
}
843876
}
@@ -854,7 +887,7 @@ export function register(on: On) {
854887
return result
855888
} finally {
856889
if (host) {
857-
afterTool(host, e.tool, result)
890+
afterTool(host, e, result)
858891
}
859892
}
860893
},

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import type { Settings } from 'claude-code'
2+
3+
/**
4+
* What the world beneath a repository session holds and answers besides
5+
* git: the store, the settings, the environment, whether a pane is seated.
6+
*/
7+
export type Beneath = {
8+
/**
9+
* What the plugin's store holds at the start; nothing when not given.
10+
*/
11+
stored?: Readonly<Record<string, unknown>>
12+
13+
/**
14+
* What `$.settings.read` answers; empty when not given.
15+
*/
16+
settings?: Settings
17+
18+
/**
19+
* The variables `$.env.get` answers from; none set when not given.
20+
*/
21+
env?: Readonly<Record<string, string>>
22+
23+
/**
24+
* Whether an open made now is left waiting undrawn (LEFT_WAITING), as an
25+
* engine leaves an unasked open on a narrow terminal; placed when not given.
26+
*/
27+
isLeftWaiting?: () => boolean
28+
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export type * from './beneath.js'
2+
export * from './left-waiting.js'
3+
4+
export * as default from '.'

‎mods/diff/tests/fixtures/beneath/left-waiting.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import type { ResultOf } from 'claude-code'
2+
3+
/**
4+
* What an engine answers an open it leaves waiting undrawn: the plugin
5+
* opened unasked on a terminal narrower than an unrequested pane is given.
6+
*
7+
* Typed through `never` so it compiles against declarations that predate the
8+
* answer, where `ui.open` resolves nothing.
9+
*/
10+
export const LEFT_WAITING: ResultOf['ui.open'] = {
11+
value: {
12+
isPlaced: false,
13+
reason:
14+
'unasked below 144 columns (120 now): placed when the person opens ' +
15+
'it, or when the terminal is widened to 144 columns',
16+
} as never,
17+
}

‎mods/diff/tests/fixtures/in-repository.ts‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Args, On } from 'claude-code'
22
import { mock } from 'claude-code/testing'
33

4+
import Beneath from './beneath'
45
import { gitIn } from './git-in.js'
56
import { HINT_DRAWN } from './hint-drawn.js'
67
import { keeping } from './keeping.js'
@@ -11,24 +12,25 @@ import { startsSession } from './starts-session.js'
1112
* A session in a repository git answers for from a script (REPOSITORY, in
1213
* /work, when none is given), keeping what the plugin does there.
1314
*
14-
* Kept: each git run, ring move, pane opened or closed, status line. The clock
15-
* starts at 0 and the engine draws the hint. A test that rewrites the
16-
* script between calls changes what git answers next.
15+
* Kept: each git run, ring move, pane opened, left waiting or closed, status
16+
* line. The clock starts at 0 and the engine draws the hint; the rest of the
17+
* world is the test's (Beneath). Rewriting the script changes git's answers.
1718
*
1819
* @param on the test's `on`
1920
* @param script git's output for each invocation whose line holds the key
20-
* @param stored what the plugin's store holds at the start
21-
* @returns the runs, the ring's moves, the panes opened and closed, the
22-
* statuses, the clock
21+
* @param beneath the store, settings, environment, whether an open is seated
22+
* @returns the runs, the ring's moves, the panes opened, left waiting and
23+
* closed, the statuses, the clock
2324
*/
2425
export function inRepository(
2526
on: On,
2627
script: Readonly<Record<string, string>> = REPOSITORY,
27-
stored: Readonly<Record<string, unknown>> = {},
28+
beneath: Beneath.Beneath = {},
2829
) {
2930
const runs: Args<'process.run'>[] = []
3031
const focused: Args<'ui.focus'>[] = []
3132
const statuses: (string | undefined)[] = []
33+
const waiting: Args<'ui.open'>[] = []
3234
const opened = keeping<Args<'ui.open'>>()
3335
const closed = keeping<Args<'ui.close'>>()
3436
const clock = startsSession(on)
@@ -51,17 +53,31 @@ export function inRepository(
5153
return { value: undefined }
5254
})
5355

54-
on('ui.open', opened.hook)
56+
on('ui.open', (engine, e) => {
57+
const isWaiting = beneath.isLeftWaiting?.() === true
58+
59+
if (!isWaiting) {
60+
return opened.hook(engine, e)
61+
}
62+
63+
waiting.push(e)
64+
65+
return Beneath.LEFT_WAITING
66+
})
67+
5568
on('ui.close', closed.hook)
5669
on('ui.invalidate', () => ({ value: undefined }))
5770
on('ui.render', { component: 'PromptHint' }, () => HINT_DRAWN)
5871
on('session.messages', () => ({ value: [] }))
59-
mock.store(on, stored)
72+
on('settings.read', () => ({ value: beneath.settings ?? {} }))
73+
mock.store(on, beneath.stored ?? {})
74+
mock.env(on, beneath.env ?? {})
6075

6176
return {
6277
runs,
6378
focused,
6479
opened: opened.kept,
80+
waiting,
6581
closed: closed.kept,
6682
statuses,
6783
clock,

0 commit comments

Comments
 (0)