diff --git a/mods/diff/.claude-plugin/plugin.json b/mods/diff/.claude-plugin/plugin.json index 6ccd583217..f6b7001b83 100644 --- a/mods/diff/.claude-plugin/plugin.json +++ b/mods/diff/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "diff", "version": "0.1.0", - "description": "The diff pane as a plugin: /diff opens the session's uncommitted changes beside the transcript, file by file with their hunks, refreshed as Claude edits and runs commands; the first edit opens it where the terminal is wide enough, and a file's ask button rides its hunks on the next prompt.", + "description": "The diff pane as a plugin: /diff opens the session's uncommitted changes beside the transcript, file by file with their hunks, refreshed as Claude edits and runs commands; the first edit it has a change to list for opens it where the terminal is wide enough, and a file's ask button rides its hunks on the next prompt.", "author": { "name": "Anthropic" } diff --git a/mods/diff/README.md b/mods/diff/README.md index 9340542248..31fd74034b 100644 --- a/mods/diff/README.md +++ b/mods/diff/README.md @@ -13,19 +13,22 @@ built-in's list keys (`ctrl+up`/`ctrl+down`, `opt+up`/`opt+down`), and through Buttons that declare the engine's own actions. The pane refreshes as Claude edits and runs shell commands, and while it is open it polls the repository's HEAD so a commit or checkout made elsewhere shows too. -The main loop's first successful edit of a session opens the pane by -itself, as the built-in panel opens on its first checkpoint: where the -layout docks it beside the transcript (the fullscreen layout, which each -drawing's `viewport` says), the terminal is wide enough (144 columns when -the person never chose, 110 when they kept it open before; a person who -closed it is left alone) and file checkpointing is on; a subagent's edit -opens nothing, and where the surface does not say, nothing opens by -itself. A docked pane fetches before it opens, as the built-in panel -primes its data, so it never lands on `Loading diff…`; an open the engine -leaves waiting undrawn is withdrawn, so no later resize seats it, and the -next edit asks again. A session resumed or continued whose transcript -already holds such an edit opens the pane on the same terms as soon as the -width is known, as the built-in opens on the history it restores. +The main loop's first successful edit of a session the diff has a file to +list for opens the pane by itself, as the built-in panel opens on its first +checkpoint: where the layout docks it beside the transcript (the fullscreen +layout, which each drawing's `viewport` says), the terminal is wide enough +(144 columns when the person never chose, 110 when they kept it open +before; a person who closed it is left alone) and file checkpointing is on; +a subagent's edit opens nothing, and where the surface does not say, +nothing opens by itself. An edit to a file outside the repository, or one +after which the diff lists nothing or cannot be read, opens nothing and +leaves the opening to a later edit. A docked pane fetches before it opens, +as the built-in panel primes its data, so it never lands on +`Loading diff…`; an open the engine leaves waiting undrawn is withdrawn, so +no later resize seats it, and the next edit asks again. A session resumed +or continued whose transcript already holds such an edit opens the pane on +the same terms as soon as the width is known, as the built-in opens on the +history it restores. Under the fullscreen layout a terminal under 110 columns gets the built-in's line asking for a wider one and nothing opens. Without that @@ -58,10 +61,11 @@ one `git rev-parse`, in the directory the session started in, when `/diff` or the first edit a pane has room to open on first needs the repository (an answer of no repository is kept too, until `/clear` or `/resume` forgets it); and the working tree is read only by a fetch for a pane that -is open, after an edit that landed or a shell command that ran. The one -read the built-in has no counterpart for is a `git status` at a pane's -first fetch, which stands in for the change time the built-in dates a -moved file by. +is open, after an edit that landed or a shell command that ran, or by the +fetch each edit inside the repository, with room for a pane, makes until +one lists a file and the pane opens on it. The one read the built-in has +no counterpart for is a `git status` at the first of those fetches, which +stands in for the change time the built-in dates a moved file by. `hooks/register.ts` is the module; everything under `hooks/` is its parts. @@ -70,14 +74,14 @@ moved file by. | event | what the hook does | | --- | --- | | `session.start` | Binds the engine once and registers `/diff` (a session where another `/diff` is listed leaves the plugin idle); asks nothing of the repository, which `/diff` or the first edit pins when it comes; off its dispatch, reads the transcript, and for a resumed session whose turns edited opens the pane as the first edit would. | -| `ui.render` of `PromptHint` | Reads the terminal's width and whether its layout docks a pane, which decide whether the first edit opens the pane. | +| `ui.render` of `PromptHint` | Reads the terminal's width and whether its layout docks a pane, which decide whether the first edit may open the pane. | | `ui.render` of `Pane` | Draws the pane: docked, the header, base line, source picker, file list and toggles over the window of hunks; inline, the dialog. | | `command.run` of `diff` | Pins the repository when none is, opens or closes the pane (focused and closing on Escape without the fullscreen layout), says which, and remembers the choice. | | `ui.close` of the pane | Backs out of the dialog's detail view instead of closing; else remembers the person's close as `/diff`'s. | | `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. | | `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. | | `command.run` of `clear`, `resume` | Closes the pane and forgets the session's state, the pinned repository with it. | -| `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. | +| `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 inside the repository whose fetch lists a file opens it on that fetch, pinning the repository then if the terminal has the room and checkpointing is on. | | `tool.call` of `Bash`, `PowerShell` | After a command that was not refused, failed and interrupted ones too, refreshes an open pane. | | `prompt.submit` | Adds the armed file's hunks to the prompt's context and disarms. | diff --git a/mods/diff/hooks/hooks.json b/mods/diff/hooks/hooks.json index 42164197dd..f7bb9c034a 100644 --- a/mods/diff/hooks/hooks.json +++ b/mods/diff/hooks/hooks.json @@ -1,4 +1,4 @@ { - "description": "The diff pane: /diff and its Pane drawing, a refresh on Claude's edits, shell commands and finished turns, the pane's opening on the first edit, and the ask that rides a file's hunks on the next prompt", + "description": "The diff pane: /diff and its Pane drawing, a refresh on Claude's edits, shell commands and finished turns, the pane's opening on the first edit with a change to list, and the ask that rides a file's hunks on the next prompt", "modules": ["./register.ts"] } diff --git a/mods/diff/hooks/index.ts b/mods/diff/hooks/index.ts index e2e3fbbd61..d888aa8f7d 100644 --- a/mods/diff/hooks/index.ts +++ b/mods/diff/hooks/index.ts @@ -10,6 +10,7 @@ export * from './git' export * from './host' export * from './is-checkpointing' export * from './is-on-pane-surface' +export * from './is-outside-working-tree' export * from './is-record' export * from './kept-of' export * from './limits' diff --git a/mods/diff/hooks/is-outside-working-tree/index.ts b/mods/diff/hooks/is-outside-working-tree/index.ts new file mode 100644 index 0000000000..0db7717837 --- /dev/null +++ b/mods/diff/hooks/is-outside-working-tree/index.ts @@ -0,0 +1,4 @@ +export * from './is-outside-working-tree.js' +export * from './normal-path-of.js' + +export * as default from '.' diff --git a/mods/diff/hooks/is-outside-working-tree/is-outside-working-tree.ts b/mods/diff/hooks/is-outside-working-tree/is-outside-working-tree.ts new file mode 100644 index 0000000000..399bb52e95 --- /dev/null +++ b/mods/diff/hooks/is-outside-working-tree/is-outside-working-tree.ts @@ -0,0 +1,29 @@ +import Git from '../git' +import { normalPathOf } from './normal-path-of.js' + +/** + * Whether a file a tool edited lies outside the pinned working tree, by + * how the paths are spelled alone (normalPathOf): no row could show it. + * + * A relative path is read from the session's directory. Where that + * directory is itself not spelled under the top (a symbolic link on the + * way to it), spelling proves nothing and nothing is outside. + * + * @param path the path the tool was called with + * @param tree the session's directory and the working tree's top + * @returns true only for a path surely outside the tree + */ +export function isOutsideWorkingTree( + path: string, + tree: { cwd: string; toplevel: string }, +): boolean { + const top = normalPathOf(tree.toplevel) + const absolute = Git.isAbsolutePath(path) ? path : `${tree.cwd}/${path}` + + const isUnderTop = (spelled: string) => + top === '' || spelled === top || spelled.startsWith(`${top}/`) + + return ( + isUnderTop(normalPathOf(tree.cwd)) && !isUnderTop(normalPathOf(absolute)) + ) +} diff --git a/mods/diff/hooks/is-outside-working-tree/normal-path-of.ts b/mods/diff/hooks/is-outside-working-tree/normal-path-of.ts new file mode 100644 index 0000000000..b69607fb7f --- /dev/null +++ b/mods/diff/hooks/is-outside-working-tree/normal-path-of.ts @@ -0,0 +1,25 @@ +/** + * An absolute path spelled one way, its names joined by `/` with no + * leading or trailing one: either separator splits, `.` and empty names + * drop, `..` drops the name before it. No disk is read, no link followed. + * + * A drive-letter path is lowercased, as its file system compares names. + * + * @param path an absolute path, POSIX or drive-letter + * @returns the names joined, empty for the root + */ +export function normalPathOf(path: string): string { + const names: string[] = [] + + for (const name of path.split(/[\\/]/)) { + if (name === '..') { + names.pop() + } else if (name !== '' && name !== '.') { + names.push(name) + } + } + + const spelled = names.join('/') + + return /^[A-Za-z]:/.test(spelled) ? spelled.toLowerCase() : spelled +} diff --git a/mods/diff/hooks/pane-state/has-session-files.ts b/mods/diff/hooks/pane-state/has-session-files.ts new file mode 100644 index 0000000000..7112ab53fb --- /dev/null +++ b/mods/diff/hooks/pane-state/has-session-files.ts @@ -0,0 +1,22 @@ +import type Git from '../git' + +/** + * Whether a fetch settled with a file the header would count, so a pane + * opened on it lists something where emptyStateOf would find a headline. + * + * No repository, a diff git could not read, and rows that all predate the + * session count nothing. + * + * @param outcome how the fetch ended + * @returns whether the header's session file count is above zero + */ +export function hasSessionFiles(outcome: Git.FetchOutcome): boolean { + if (outcome.kind !== 'data') { + return false + } + + const { stats, files } = outcome.data + const before = files.filter(file => file.isPreSession) + + return stats.filesCount - before.length > 0 +} diff --git a/mods/diff/hooks/pane-state/index.ts b/mods/diff/hooks/pane-state/index.ts index 33e7edfaff..7ff1055093 100644 --- a/mods/diff/hooks/pane-state/index.ts +++ b/mods/diff/hooks/pane-state/index.ts @@ -7,6 +7,7 @@ export * from './dialog-title' export * from './dialog-title-of.js' export * from './empty-state' export * from './empty-state-of.js' +export * from './has-session-files.js' export * from './header-totals' export * from './header-totals-of.js' export * from './initial-model' diff --git a/mods/diff/hooks/register.ts b/mods/diff/hooks/register.ts index 1dccfe1ba1..8ab450f75d 100644 --- a/mods/diff/hooks/register.ts +++ b/mods/diff/hooks/register.ts @@ -16,6 +16,7 @@ import type Git from './git' import type { Host } from './host' import { isCheckpointing } from './is-checkpointing' import { isOnPaneSurface } from './is-on-pane-surface' +import { isOutsideWorkingTree } from './is-outside-working-tree' import { isRecord } from './is-record' import Limits from './limits' import { mapLimited } from './map-limited' @@ -38,7 +39,8 @@ import Views from './views' * session whose turns already edited opens as its first edit would; `/diff` * or the main loop's first checkpointed edit with room pins the backend, * until `/clear`, which reads afresh under a pane it leaves open; a docked - * pane fetches, then opens. + * pane fetches, then opens, and such an edit inside the tree fetches until + * one lists a file to open on. * * @param on the engine's registrar */ @@ -51,6 +53,9 @@ export function register(on: On) { let dialogRows: number | null = null let hasAutoOpened = false let hasRestoredEdits = false + let isAutoOpening = false + let landed = 0 + let opens = 0 let columns: number | null = null let shownSessionId: string | null = null let armed: Ask.ArmedAsk | null = null @@ -58,7 +63,6 @@ export function register(on: On) { let isRefreshing = false let isRefreshQueued = false let generation = 0 - let landed = 0 let bodyStamp: string | null = null let bodyBase: string | null = null @@ -313,7 +317,27 @@ export function register(on: On) { ) } - async function refresh(engine: Host): Promise { + async function readOf( + engine: Host, + pinned: Backend.Backend | null, + ): Promise { + const fetched = (): Promise => + pinned + ? pinned.fetchDiff(model.requestedMode) + : Promise.resolve({ kind: 'no-repository' }) + + const [outcome, messages] = await Promise.all([ + fetched(), + engine.messages().catch((): SessionMessage[] => []), + ]) + + return { outcome, messages } + } + + async function refresh( + engine: Host, + read: PaneState.Fetched | null = null, + ): Promise { if (isRefreshing) { isRefreshQueued = true @@ -325,20 +349,13 @@ export function register(on: On) { const record = Record.recorderOf(engine) const pinned = backend - const fetched = (): Promise => - pinned - ? pinned.fetchDiff(model.requestedMode) - : Promise.resolve({ kind: 'no-repository' }) - try { model = { ...model, isLoading: model.data === null } - const [outcome, messages] = await Promise.all([ - fetched(), - engine.messages().catch((): SessionMessage[] => []), - ]) + const fetched = read ?? (await readOf(engine, pinned)) + const { outcome } = fetched - model = PaneState.afterFetch(model, { outcome, messages }) + model = PaneState.afterFetch(model, fetched) switch (outcome.kind) { case 'no-repository': @@ -403,9 +420,12 @@ export function register(on: On) { async function openPane( engine: Host, trigger: (typeof Record.SHOWN_TRIGGERS)[number], + read: PaneState.Fetched | null = null, ): Promise { const isDialog = model.isFullscreen === false + opens += 1 + model = { ...model, selectedPath: null, @@ -417,7 +437,9 @@ export function register(on: On) { const landedBefore = landed - if (!isDialog) { + if (read) { + model = PaneState.afterFetch(model, read) + } else if (!isDialog) { await refresh(engine).catch(() => undefined) } @@ -444,10 +466,10 @@ export function register(on: On) { Record.recorderOf(engine).shown(trigger, Record.widthBucketOf(columns)) } - const isStale = isDialog || landed !== landedBefore + const isStale = isDialog || read !== null || landed !== landedBefore if (isStale) { - void refresh(engine) + void refresh(engine, read) } return true @@ -465,9 +487,65 @@ export function register(on: On) { }) } - async function openOnFirstEdit(engine: Host): Promise { - const isTaken = () => isPaneOpen || hasAutoOpened + const isTaken = () => isPaneOpen || hasAutoOpened + + const hasRoomFor = (floor: number) => + model.isFullscreen === true && columns !== null && columns >= floor + + async function openOnFetchedFiles( + engine: Host, + floor: number, + ): Promise { + if (isAutoOpening) { + return + } + + isAutoOpening = true + try { + while (!isTaken()) { + const { epoch } = pin + const seen = landed + const opened = opens + const read = await readOf(engine, backend) + const isOvertaken = opened !== opens + + const isCurrent = + epoch === pin.epoch && !isOvertaken && hasRoomFor(floor) && !isTaken() + + const isListing = isCurrent && PaneState.hasSessionFiles(read.outcome) + + if (isListing) { + hasAutoOpened = true + + const isPlaced = await openPane(engine, 'auto_open', read) + + if (!isPlaced) { + hasAutoOpened = false + + return + } + } + + const hasLostRoom = !isListing && !hasRoomFor(floor) + + if (seen === landed || isOvertaken || hasLostRoom) { + return + } + + if (isListing) { + scheduleRefresh(engine) + } + } + } finally { + isAutoOpening = false + } + } + + async function openOnFirstEdit( + engine: Host, + path: string | null = null, + ): Promise { if (isTaken()) { return } @@ -479,11 +557,7 @@ export function register(on: On) { ? Limits.OPEN_MIN_COLUMNS : Limits.AUTO_OPEN_MIN_COLUMNS - const hasRoom = - preference !== false && - model.isFullscreen === true && - columns !== null && - columns >= floor + const hasRoom = preference !== false && hasRoomFor(floor) if (!hasRoom || isTaken()) { return @@ -501,8 +575,16 @@ export function register(on: On) { return } - hasAutoOpened = true - hasAutoOpened = await openPane(engine, 'auto_open') + const isOutside = + path !== null && + isOutsideWorkingTree(path, { + cwd: pin.cwd, + toplevel: backend.repository.toplevel, + }) + + if (!isOutside) { + await openOnFetchedFiles(engine, floor) + } } async function openOnRestore(engine: Host): Promise { @@ -942,7 +1024,7 @@ export function register(on: On) { const isMainLoopEdit = hasEdited && e.agentId === undefined if (isMainLoopEdit) { - void openOnFirstEdit(engine).catch(() => undefined) + void openOnFirstEdit(engine, Tools.editedPathOf(e)).catch(() => undefined) } } diff --git a/mods/diff/hooks/tools/edited-path-of.ts b/mods/diff/hooks/tools/edited-path-of.ts new file mode 100644 index 0000000000..4783ee4ef1 --- /dev/null +++ b/mods/diff/hooks/tools/edited-path-of.ts @@ -0,0 +1,21 @@ +import type { ToolCallInput } from 'claude-code' + +/** + * The file an editing tool was called on, as its input names it: + * `file_path` for Edit and Write, `notebook_path` for NotebookEdit. + * + * @param e the `tool.call` event + * @returns the path as given, or null when the input names none + */ +export function editedPathOf(e: ToolCallInput): string | null { + const isFileEdit = e.tool === 'Edit' || e.tool === 'Write' + const isNotebookEdit = e.tool === 'NotebookEdit' + + const path = isFileEdit + ? e.file_path + : isNotebookEdit + ? e.notebook_path + : null + + return typeof path === 'string' && path !== '' ? path : null +} diff --git a/mods/diff/hooks/tools/index.ts b/mods/diff/hooks/tools/index.ts index 3083c37a55..8f2400f1e6 100644 --- a/mods/diff/hooks/tools/index.ts +++ b/mods/diff/hooks/tools/index.ts @@ -1,3 +1,4 @@ +export * from './edited-path-of.js' export * from './editing-tools.js' export * from './shell-tools.js' export * from './todo-tool.js' diff --git a/mods/diff/tests/fixtures/in-slow-repository.ts b/mods/diff/tests/fixtures/in-slow-repository.ts new file mode 100644 index 0000000000..6ef79a758a --- /dev/null +++ b/mods/diff/tests/fixtures/in-slow-repository.ts @@ -0,0 +1,53 @@ +import type { Args, On } from 'claude-code' +import { mock } from 'claude-code/testing' + +import { gitIn } from './git-in.js' +import { HINT_DRAWN } from './hint-drawn.js' +import { keeping } from './keeping.js' +import { REPOSITORY } from './repository.js' +import { startsSession } from './starts-session.js' + +/** + * A session in a repository as inRepository keeps one, where git takes a + * millisecond of the clock to answer each `--numstat`: time for a test to + * land an edit or a `/clear` while the diff is being read. + * + * What git answers is settled as the read starts, from the script as it + * stands then. + * + * @param on the test's `on` + * @param script git's output for each invocation whose line holds the key + * @returns the clock's time as each read started, the panes opened, the + * clock + */ +export function inSlowRepository( + on: On, + script: Readonly> = REPOSITORY, +) { + const reads: number[] = [] + const opened = keeping>() + const clock = startsSession(on) + + on('process.run', async ($, e) => { + const value = gitIn(e.argv, script) + + if (e.argv.includes('--numstat')) { + reads.push(await clock.now()) + await clock.sleep(1) + } + + return { value } + }) + + on('ui.open', opened.hook) + on('ui.close', () => ({ value: undefined })) + on('ui.status', () => ({ value: undefined })) + on('ui.invalidate', () => ({ value: undefined })) + on('ui.render', { component: 'PromptHint' }, () => HINT_DRAWN) + on('session.messages', () => ({ value: [] })) + on('settings.read', () => ({ value: {} })) + mock.store(on, {}) + mock.env(on, {}) + + return { reads, opened: opened.kept, clock } +} diff --git a/mods/diff/tests/fixtures/index.ts b/mods/diff/tests/fixtures/index.ts index af8bba2e10..c8f9778b98 100644 --- a/mods/diff/tests/fixtures/index.ts +++ b/mods/diff/tests/fixtures/index.ts @@ -24,6 +24,7 @@ export * from './hint.js' export * from './hint-at.js' export * from './hint-drawn.js' export * from './in-repository.js' +export * from './in-slow-repository.js' export * from './inline-pane.js' export * from './keeping.js' export * from './kind-walk-of' diff --git a/mods/diff/tests/is-outside-working-tree.test.ts b/mods/diff/tests/is-outside-working-tree.test.ts new file mode 100644 index 0000000000..9e581e89e1 --- /dev/null +++ b/mods/diff/tests/is-outside-working-tree.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, test, tier } from 'claude-code/testing' + +import { isOutsideWorkingTree } from '../hooks/is-outside-working-tree' + +tier('builtin') + +describe('is-outside-working-tree', () => { + const WORK = { cwd: '/work/src', toplevel: '/work' } + + test('the top and what is spelled under it are inside', () => { + for (const path of [ + '/work', + '/work/', + '/work/app.ts', + '/work//src/./deep/../a.ts', + 'a.ts', + './deep/a.ts', + '../app.ts', + ]) { + expect(isOutsideWorkingTree(path, WORK), path).toBe(false) + } + }) + + test('elsewhere, climbed out, or a sibling sharing its spelling: outside', () => { + for (const path of [ + '/', + '/tmp/notes.md', + '/workshop/a.ts', + '/work-old/a.ts', + '/work/../etc/hosts', + '../../elsewhere/a.ts', + ]) { + expect(isOutsideWorkingTree(path, WORK), path).toBe(true) + } + }) + + test('a drive-letter tree: either separator, any case', () => { + const tree = { cwd: 'C:\\Users\\me\\work', toplevel: 'C:/Users/me/work' } + + expect(isOutsideWorkingTree('c:\\users\\me\\work\\a.ts', tree)).toBe(false) + expect(isOutsideWorkingTree('src\\a.ts', tree)).toBe(false) + expect(isOutsideWorkingTree('C:\\Users\\me\\workshop\\a.ts', tree)).toBe( + true, + ) + expect(isOutsideWorkingTree('D:\\Users\\me\\work\\a.ts', tree)).toBe(true) + }) + + test('a tree at the root holds every path', () => { + expect(isOutsideWorkingTree('/tmp/a.ts', { cwd: '/', toplevel: '/' })).toBe( + false, + ) + }) + + test("a session's directory spelled outside its own top proves nothing", () => { + const linked = { cwd: '/tmp/work', toplevel: '/private/tmp/work' } + + expect(isOutsideWorkingTree('/tmp/work/a.ts', linked)).toBe(false) + expect(isOutsideWorkingTree('/elsewhere/a.ts', linked)).toBe(false) + }) +}) diff --git a/mods/diff/tests/pane-state/has-session-files.test.ts b/mods/diff/tests/pane-state/has-session-files.test.ts new file mode 100644 index 0000000000..b01c08aa0d --- /dev/null +++ b/mods/diff/tests/pane-state/has-session-files.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, test, tier } from 'claude-code/testing' + +import type Git from '../../hooks/git' +import PaneState from '../../hooks/pane-state' +import Fixtures from '../fixtures' + +tier('builtin') + +describe('has-session-files', () => { + const dataOf = ( + files: readonly Git.FileStat[], + overrides: Partial = {}, + ): Git.FetchOutcome => ({ + kind: 'data', + data: { + repository: Fixtures.CHECKOUT, + mode: 'session', + stats: { ...Fixtures.NO_STATS, filesCount: files.length }, + files, + source: { kind: 'working-tree', base: 'HEAD' }, + isUnborn: false, + baseRef: 'HEAD', + stalePaths: [], + isUntrackedWithheld: false, + ...overrides, + }, + }) + + test("a file the header counts, a test among them: there's one", () => { + expect(PaneState.hasSessionFiles(dataOf([Fixtures.SESSION_ROW]))).toBe(true) + expect(PaneState.hasSessionFiles(dataOf([Fixtures.TEST_ROW]))).toBe(true) + + expect( + PaneState.hasSessionFiles( + dataOf([Fixtures.PRE_SESSION_ROW, Fixtures.SESSION_ROW]), + ), + ).toBe(true) + }) + + test('files counted past the row cap, none stored, still count', () => { + expect( + PaneState.hasSessionFiles( + dataOf([], { + stats: { ...Fixtures.NO_STATS, filesCount: Fixtures.PAST_CAP_COUNT }, + }), + ), + ).toBe(true) + }) + + test('wherever the pane would headline an empty state: none', () => { + const withheld = { isUntrackedWithheld: true } + + expect(PaneState.hasSessionFiles({ kind: 'no-repository' })).toBe(false) + expect(PaneState.hasSessionFiles({ kind: 'unavailable' })).toBe(false) + expect(PaneState.hasSessionFiles(dataOf([]))).toBe(false) + expect(PaneState.hasSessionFiles(dataOf([], withheld))).toBe(false) + expect(PaneState.hasSessionFiles(dataOf([], { isUnborn: true }))).toBe( + false, + ) + + expect(PaneState.hasSessionFiles(dataOf([Fixtures.PRE_SESSION_ROW]))).toBe( + false, + ) + }) +}) diff --git a/mods/diff/tests/register.test.ts b/mods/diff/tests/register.test.ts index 6d4a7dc01b..8ca6d3b1ef 100644 --- a/mods/diff/tests/register.test.ts +++ b/mods/diff/tests/register.test.ts @@ -448,6 +448,483 @@ describe('register', () => { ).toEqual(['diff']) }) + test('an edit outside the repository opens nothing, the next one inside does', async ($, on) => { + const world = Fixtures.inRepository(on) + + on('tool.call', () => ({ result: 'written' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + + await $.tool.call({ + tool: 'Write', + file_path: '/tmp/notes.md', + content: 'a note', + }) + + await $.tool.call({ + tool: 'NotebookEdit', + notebook_path: '../elsewhere/plot.ipynb', + new_source: '1', + }) + + await world.clock.advance(Fixtures.SETTLE_MS) + + expect(world.opened, 'no row could show either file').toEqual([]) + + expect( + world.runs.map(run => Fixtures.gitWordOf(run.argv)), + 'the repository was found, its working tree never read', + ).toEqual(['rev-parse --show-toplevel']) + + await $.tool.call({ + tool: 'Write', + file_path: 'app.ts', + content: 'const a = 2', + }) + + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.opened.map(pane => pane.id), + "a path from the session's directory, inside: the first edit still", + ).toEqual(['diff']) + }) + + test('a directory beside the repository that starts with its name is outside', async ($, on) => { + const world = Fixtures.inRepository(on) + + on('tool.call', () => ({ result: 'written' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + + await $.tool.call({ + tool: 'Write', + file_path: '/workshop/a.ts', + content: 'const a = 1', + }) + + await world.clock.advance(Fixtures.SETTLE_MS) + + expect(world.opened, '/workshop is not under /work').toEqual([]) + }) + + test('an edit the diff lists no row for opens nothing, a later one it lists does', async ($, on) => { + const script: Record = { + ...Fixtures.REPOSITORY, + 'HEAD --shortstat': '', + 'HEAD --numstat': '', + } + + const world = Fixtures.inRepository(on, script) + + const fetches = () => + world.runs.filter(run => run.argv.includes('--numstat')).length + + const edit = (path: string) => + $.tool.call({ + tool: 'Edit', + file_path: path, + old_string: '1', + new_string: '2', + }) + + on('tool.call', () => ({ result: 'edited' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + await edit('/work/ignored/cache.json') + await world.clock.advance(Fixtures.SETTLE_MS) + + expect(world.opened, 'it would say "No changes this session"').toEqual([]) + expect(fetches(), 'one fetch said so').toBe(1) + + await edit('/work/ignored/cache.json') + await world.clock.advance(Fixtures.SETTLE_MS) + + expect(world.opened, 'nor the second time').toEqual([]) + expect(fetches(), 'a fetch an edit, no more').toBe(2) + + Object.assign(script, Fixtures.REPOSITORY) + + await edit('/work/app.ts') + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.opened.map(pane => pane.id), + 'the first edit with a row to list', + ).toEqual(['diff']) + + expect(fetches(), 'opened on the fetch that found it').toBe(3) + }) + + test('a diff git could not read, or no tracked row with the untracked withheld, opens nothing', async ($, on) => { + const script: Record = { ...Fixtures.REPOSITORY } + const world = Fixtures.inRepository(on, script) + + const edit = () => + $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + on('tool.call', () => ({ result: 'edited' })) + + delete script['HEAD --numstat'] + delete script['ls-files'] + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + await edit() + await world.clock.advance(Fixtures.SETTLE_MS) + + expect(world.opened, 'it would say "Diff unavailable"').toEqual([]) + + script['HEAD --shortstat'] = '' + script['HEAD --numstat'] = '' + + await edit() + await world.clock.advance(Fixtures.SETTLE_MS) + + expect(world.opened, 'it would say "No tracked changes"').toEqual([]) + + Object.assign(script, Fixtures.REPOSITORY) + + await edit() + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.opened.map(pane => pane.id), + 'read whole, with a row: open', + ).toEqual(['diff']) + }) + + test('the first edit opens the pane on the one fetch that found its rows', async ($, on) => { + const runs: Args<'process.run'>[] = [] + const drawnAtOpen: string[] = [] + const clock = Fixtures.startsSession(on) + + on('process.run', (_engine, e) => { + runs.push(e) + + return { value: Fixtures.gitIn(e.argv) } + }) + + on('ui.open', async () => { + drawnAtOpen.push(Fixtures.textOf(await $.ui.render(Fixtures.PANE))) + + return { value: undefined } + }) + + on('ui.invalidate', () => ({ value: undefined })) + on('ui.render', { component: 'PromptHint' }, () => Fixtures.HINT_DRAWN) + on('session.messages', () => ({ value: [] })) + on('tool.call', () => ({ result: 'edited' })) + mock.store(on, {}) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + + await $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + await clock.advance(Fixtures.SETTLE_MS) + + expect(drawnAtOpen, 'one pane, listing as it opens').toEqual([ + expect.stringContaining('1 file changed'), + ]) + + expect( + runs + .map(run => Fixtures.gitWordOf(run.argv)) + .filter(word => word !== Fixtures.POLL_WORD), + "/diff's spawns, once", + ).toEqual([ + 'rev-parse --show-toplevel', + 'status', + 'diff --shortstat', + 'diff --numstat', + 'ls-files', + 'diff -- app.ts', + ]) + }) + + test('edits landing together open one pane', async ($, on) => { + const world = Fixtures.inRepository(on) + + const edit = () => + $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + on('tool.call', () => ({ result: 'edited' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + await Promise.all([edit(), edit(), edit()]) + await world.clock.advance(Fixtures.SETTLE_MS) + + expect(world.opened.map(pane => pane.id)).toEqual(['diff']) + + expect( + world.runs.filter(run => run.argv.includes('--numstat')), + 'on one fetch', + ).toHaveLength(1) + }) + + test('an edit or a command that lands while the diff is read is read too', async ($, on) => { + const script: Record = { + ...Fixtures.REPOSITORY, + 'HEAD --shortstat': '', + 'HEAD --numstat': '', + } + + const world = Fixtures.inSlowRepository(on, script) + const openedIds = () => world.opened.map(pane => pane.id) + + const edit = (path: string) => + $.tool.call({ + tool: 'Edit', + file_path: path, + old_string: '1', + new_string: '2', + }) + + on('tool.call', () => ({ result: 'edited' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + await edit('/work/ignored/cache.json') + await world.clock.settle() + + Object.assign(script, Fixtures.REPOSITORY) + + await edit('/work/app.ts') + await world.clock.settle() + + expect(world.reads, 'the second edit began no read of its own').toEqual([0]) + + await world.clock.advance(1) + + expect(openedIds(), 'the first read had found nothing').toEqual([]) + expect(world.reads, 'so the edit it missed is read').toEqual([0, 1]) + + await $.tool.call({ tool: 'Bash', command: 'make' }) + await world.clock.advance(1) + + expect(openedIds(), 'that read lists a file').toEqual(['diff']) + + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.reads, + 'and the pane, open, reads what the command may have changed since', + ).toHaveLength(3) + }) + + test('a read that /clear overtakes opens nothing, the next edit reads again', async ($, on) => { + const world = Fixtures.inSlowRepository(on) + + const edit = () => + $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + on('tool.call', () => ({ result: 'edited' })) + on('command.run', { command: 'clear' }, () => ({})) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + await edit() + await world.clock.settle() + await $.command.run(Fixtures.CLEAR) + await world.clock.advance(Fixtures.SETTLE_MS) + + expect(world.reads, 'a read was under way at /clear').toEqual([0]) + expect(world.opened, 'it read for the conversation before').toEqual([]) + + await edit() + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.opened.map(pane => pane.id), + "the new conversation's first edit", + ).toEqual(['diff']) + }) + + test('a pane the person hid while the diff was read stays hidden', async ($, on) => { + const world = Fixtures.inSlowRepository(on) + + on('tool.call', () => ({ result: 'edited' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + + await $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + await world.clock.settle() + + expect(world.reads, "the edit's read is under way").toEqual([0]) + + const shown = $.command.run(Fixtures.DIFF) + + await world.clock.advance(1) + + expect( + await shown, + '/diff reads before it opens, as the edit does', + ).toEqual({ text: 'Diff panel shown' }) + + expect(await $.command.run(Fixtures.DIFF)).toEqual({ + text: 'Diff panel hidden', + }) + + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.opened.map(pane => pane.id), + "/diff's open alone: the read gave way to the person's close", + ).toEqual(['diff']) + }) + + test('nor does a command landing in that read open it again', async ($, on) => { + const world = Fixtures.inSlowRepository(on) + + on('tool.call', () => ({ result: 'done' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + + await $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + await world.clock.settle() + + const shown = $.command.run(Fixtures.DIFF) + + await world.clock.advance(1) + await shown + await $.command.run(Fixtures.DIFF) + await $.tool.call({ tool: 'Bash', command: 'make' }) + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.opened.map(pane => pane.id), + 'the attempt stood down; it did not read again for what landed', + ).toEqual(['diff']) + }) + + test('/diff typed while the diff is read opens the one pane', async ($, on) => { + const world = Fixtures.inSlowRepository(on) + + on('tool.call', () => ({ result: 'edited' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + + await $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + await world.clock.settle() + + const shown = $.command.run(Fixtures.DIFF) + + await world.clock.advance(Fixtures.SETTLE_MS) + await shown + + expect( + world.opened.map(pane => pane.id), + "/diff's, not a second", + ).toEqual(['diff']) + }) + + test('a terminal narrowed while the diff is read opens nothing', async ($, on) => { + const world = Fixtures.inSlowRepository(on) + + on('tool.call', () => ({ result: 'edited' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + + await $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + await world.clock.settle() + await $.ui.render(Fixtures.hintAt(Limits.AUTO_OPEN_MIN_COLUMNS - 1)) + await world.clock.advance(Fixtures.SETTLE_MS) + + expect(world.reads, 'a read was under way as it narrowed').toEqual([0]) + expect(world.opened, 'the room it read for is gone').toEqual([]) + }) + + test('nor does a command landing in that read have it read again, the next edit with room does', async ($, on) => { + const world = Fixtures.inSlowRepository(on) + + const edit = () => + $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + on('tool.call', () => ({ result: 'done' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + await edit() + await world.clock.settle() + await $.ui.render(Fixtures.hintAt(Limits.AUTO_OPEN_MIN_COLUMNS - 1)) + await $.tool.call({ tool: 'Bash', command: 'make' }) + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.reads, + 'the attempt gave way with the room; it read for nothing more', + ).toEqual([0]) + + expect(world.opened).toEqual([]) + + await $.ui.render(Fixtures.HINT) + await edit() + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.opened.map(pane => pane.id), + 'wide again, the next edit starts an attempt of its own', + ).toEqual(['diff']) + }) + test("only the main loop's edit opens the pane", async ($, on) => { const world = Fixtures.inRepository(on) @@ -570,6 +1047,18 @@ describe('register', () => { 'built-in primes; nothing polls for a pane no one sees', ).toEqual(['rev-parse --show-toplevel', 'status']) + expect( + world.runs.map(run => Fixtures.gitWordOf(run.argv)), + 'the repository was found and read once to decide, no hunks fetched ' + + 'for a pane no one sees', + ).toEqual([ + 'rev-parse --show-toplevel', + 'status', + 'diff --shortstat', + 'diff --numstat', + 'ls-files', + ]) + expect( world.runs.map(run => Fixtures.gitWordOf(run.argv)), 'no HEAD poll for a withdrawn pane', @@ -728,6 +1217,50 @@ describe('register', () => { ).toEqual(['diff']) }) + test('a resumed session whose edits the diff no longer lists opens nothing, the next edit it lists does', async ($, on) => { + const script: Record = { + ...Fixtures.REPOSITORY, + 'HEAD --shortstat': '', + 'HEAD --numstat': '', + } + + const world = Fixtures.inRepository(on, script, { + messages: () => Fixtures.EDITED_TRANSCRIPT, + }) + + on('tool.call', () => ({ result: 'edited' })) + + await $.session.start(Fixtures.SESSION) + await $.ui.render(Fixtures.HINT) + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.opened, + 'committed since: it would say "No changes this session"', + ).toEqual([]) + + Object.assign(script, Fixtures.REPOSITORY) + + await $.tool.call({ + tool: 'Edit', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }) + + await world.clock.advance(Fixtures.SETTLE_MS) + + expect( + world.opened.map(pane => pane.id), + 'the restore left the opening to the first edit with a row to list', + ).toEqual(['diff']) + + expect( + Fixtures.textOf(await $.ui.render(Fixtures.PANE)), + 'listed as it opened', + ).toContain('1 file changed') + }) + test('a resumed session opens nothing where its first edit would not', async ($, on) => { const narrow = Fixtures.inRepository(on, Fixtures.REPOSITORY, { messages: () => Fixtures.EDITED_TRANSCRIPT, diff --git a/mods/diff/tests/tools/edited-path-of.test.ts b/mods/diff/tests/tools/edited-path-of.test.ts new file mode 100644 index 0000000000..654d114b3f --- /dev/null +++ b/mods/diff/tests/tools/edited-path-of.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, test, tier } from 'claude-code/testing' + +import Tools from '../../hooks/tools' + +tier('builtin') + +describe('edited-path-of', () => { + test("each editing tool's own path argument", () => { + expect( + Tools.editedPathOf({ + tool: 'Edit', + tool_use_id: 'toolu_1', + file_path: '/work/app.ts', + old_string: '1', + new_string: '2', + }), + ).toBe('/work/app.ts') + + expect( + Tools.editedPathOf({ + tool: 'Write', + tool_use_id: 'toolu_2', + file_path: 'notes.md', + content: '', + }), + ).toBe('notes.md') + + expect( + Tools.editedPathOf({ + tool: 'NotebookEdit', + tool_use_id: 'toolu_3', + notebook_path: '/work/plot.ipynb', + new_source: '1', + }), + ).toBe('/work/plot.ipynb') + }) + + test('an empty path, or a tool that edits no file: null', () => { + expect( + Tools.editedPathOf({ + tool: 'Write', + tool_use_id: 'toolu_4', + file_path: '', + content: '', + }), + ).toBeNull() + + expect( + Tools.editedPathOf({ + tool: 'Bash', + tool_use_id: 'toolu_5', + command: 'make', + }), + ).toBeNull() + + expect( + Tools.editedPathOf({ + tool: 'mcp__files__write', + tool_use_id: 'toolu_6', + file_path: '/work/app.ts', + }), + ).toBeNull() + }) +})