spacehub

surshar / claude-code

Claude Code is an agentic coding tool that lives in your terminal, understands your codebase, and helps you code faster by executing routine tasks, explaining complex code, and handling git workflows - all through natural language commands.

Built by Anthropic · github.com/anthropics

diff: the pane notices a finished merge by itself, and stays quiet on an unusual branch name (#98357)Browse files
Alice T'Poteat authored 24 days ago · GitHub committed 24 days ago parent 292c5b8 · commit 525d3b353126
4 changed files+224 -36compared with 292c5b8
Mmods/diff/hooks/git/probes/head-key-of/head-key-of.ts
Mmods/diff/hooks/register.ts
Mmods/diff/tests/git/probes/head-key-of.test.ts
Mmods/diff/tests/register.test.ts
@@ -9,9 +9,9 @@
99 * A string that changes when HEAD moves: HEAD's text and the timestamps
1010 * of HEAD, its ref file and packed-refs, each a listed real file.
1111 *
12- * A HEAD that is not a plain file, or a `ref:` outside the safe refname
13- * grammar, is read through `git rev-parse` instead and nothing is stat'ed;
14- * every directory above a stat'ed file was listed as a real directory.
12+ * A `ref:` outside the safe refname grammar is dated by HEAD's own log, the
13+ * listed real file `logs/HEAD`; without one, or where HEAD is not a plain
14+ * file, `git rev-parse` answers instead.
1515 *
1616 * @param deps git, listings, timestamps, a file read
1717 * @param repository the repository's directories
@@ -54,13 +54,16 @@
5454 const isSymbolic = ref !== ''
5555
5656 const isUnsafeRef = isSymbolic && !isSafeRefName(ref)
57+ const logged = isUnsafeRef ? await stampIfFile(own, 'logs/HEAD') : null
58+ const isUndated = isUnsafeRef && typeof logged !== 'number'
59+ const isRefDated = isSymbolic && !isUnsafeRef
5760
58- return isUnsafeRef
61+ return isUndated
5962 ? viaGit()
6063 : [
6164 head,
6265 await stampIfFile(own, 'HEAD'),
63- isSymbolic ? await stampIfFile(common, ref) : null,
66+ isRefDated ? await stampIfFile(common, ref) : logged,
6467 await stampIfFile(common, 'packed-refs'),
6568 ].join('|')
6669 }
@@ -146,6 +146,8 @@
146146 return asked.isAnswered || backend !== null
147147 }
148148
149+ startPoll(engine, probed)
150+
149151 const stored = PaneState.baseModeOf(
150152 await engine
151153 .storeGet(Names.baseStoreKeyOf(probed.repository.toplevel))
@@ -268,6 +270,25 @@
268270 return [...read.values()].includes(null)
269271 }
270272
273+ function keepBaseline() {
274+ const pinned = backend
275+
276+ if (!pinned || polled.headKey !== '') {
277+ return
278+ }
279+
280+ void pinned
281+ .headKeyOf()
282+ .catch(() => '')
283+ .then(key => {
284+ const isFirst = backend === pinned && polled.headKey === ''
285+
286+ if (isFirst) {
287+ polled.headKey = key
288+ }
289+ })
290+ }
291+
271292 function startPoll(engine: Host, pinned: Backend.Backend) {
272293 const readHeadKey = () => pinned.headKeyOf().catch(() => '')
273294
@@ -339,10 +360,6 @@
339360 case 'data':
340361 generation += 1
341362
342- if (pinned) {
343- startPoll(engine, pinned)
344- }
345-
346363 break
347364 }
348365
@@ -386,6 +403,18 @@
386403 )
387404 }
388405
406+ async function markShown(
407+ engine: Host,
408+ trigger: (typeof Record.SHOWN_TRIGGERS)[number],
409+ ): Promise<void> {
410+ const sessionId = await engine.sessionId().catch(() => null)
411+
412+ if (sessionId !== null && sessionId !== shownSessionId) {
413+ shownSessionId = sessionId
414+ Record.recorderOf(engine).shown(trigger, Record.widthBucketOf(columns))
415+ }
416+ }
417+
389418 async function openPane(
390419 engine: Host,
391420 trigger: (typeof Record.SHOWN_TRIGGERS)[number],
@@ -422,14 +451,10 @@
422451 }
423452
424453 isPaneOpen = true
454+ keepBaseline()
425455
426- const sessionId = await engine.sessionId().catch(() => null)
456+ await markShown(engine, trigger)
427457
428- if (sessionId !== null && sessionId !== shownSessionId) {
429- shownSessionId = sessionId
430- Record.recorderOf(engine).shown(trigger, Record.widthBucketOf(columns))
431- }
432-
433458 const isStale = isDialog || landed !== landedBefore
434459
435460 if (isStale) {
@@ -889,7 +914,9 @@
889914 (isResume ? sessionStartMs : await host.now())
890915
891916 if (isKeptOpen) {
917+ await markShown(host, 'manual')
892918 await pinBackend(host)
919+ keepBaseline()
893920 void refresh(host)
894921 }
895922
@@ -16,6 +16,7 @@
1616 function recorded(
1717 listings: Readonly<Record<string, ReadonlyMap<string, Git.EntryKind>>>,
1818 head: string,
19+ stampOf: (path: string) => number = () => Fixtures.STAMP,
1920 ): Recorded {
2021 const statted: string[] = []
2122 const read: string[] = []
@@ -30,7 +31,7 @@
3031 mtimeOf: path => {
3132 statted.push(path)
3233
33- return Promise.resolve(Fixtures.STAMP)
34+ return Promise.resolve(stampOf(path))
3435 },
3536 readFile: path => {
3637 read.push(path)
@@ -57,6 +58,27 @@
5758 '/r/.git/refs/heads': heads,
5859 }
5960
61+ const logged = {
62+ '/r/.git': new Map<string, Git.EntryKind>([
63+ ...plainGitDir,
64+ ['logs', 'dir'],
65+ ]),
66+ '/r/.git/logs': new Map<string, Git.EntryKind>([['HEAD', 'file']]),
67+ }
68+
69+ async function idleOn(branch: string) {
70+ const probe = recorded(logged, `ref: refs/heads/${branch}\n`)
71+
72+ return {
73+ keys: new Set([
74+ await Git.headKeyOf(probe.deps, Fixtures.CHECKOUT),
75+ await Git.headKeyOf(probe.deps, Fixtures.CHECKOUT),
76+ await Git.headKeyOf(probe.deps, Fixtures.CHECKOUT),
77+ ]).size,
78+ ran: probe.ran,
79+ }
80+ }
81+
6082 test('a plain checkout reads HEAD, its ref file, packed-refs', async () => {
6183 const probe = recorded(healthy, 'ref: refs/heads/main\n')
6284 const key = await Git.headKeyOf(probe.deps, Fixtures.CHECKOUT)
@@ -79,6 +101,37 @@
79101 expect(key).toBe(`rev:${Fixtures.HEAD_SHA}`)
80102 })
81103
104+ test('a branch named with a # starts no git while idle', async () => {
105+ expect(await idleOn('fix/#12')).toEqual({ ran: [], keys: 1 })
106+ })
107+
108+ test('a branch named outside ASCII starts no git while idle', async () => {
109+ expect(await idleOn('höher')).toEqual({ ran: [], keys: 1 })
110+ })
111+
112+ test('on such a branch a moved HEAD is seen at the next look', async () => {
113+ let moves = 0
114+
115+ const probe = recorded(logged, 'ref: refs/heads/fix/#12\n', path =>
116+ path.endsWith('/logs/HEAD') ? Fixtures.STAMP + moves : Fixtures.STAMP,
117+ )
118+
119+ const before = await Git.headKeyOf(probe.deps, Fixtures.CHECKOUT)
120+
121+ moves += 1
122+
123+ expect(await Git.headKeyOf(probe.deps, Fixtures.CHECKOUT)).not.toBe(before)
124+ expect(probe.ran, 'dated by the log of HEAD, no git started').toEqual([])
125+ })
126+
127+ test('with no log of HEAD such a branch is read through git', async () => {
128+ const probe = recorded(healthy, 'ref: refs/heads/fix/#12\n')
129+ const key = await Git.headKeyOf(probe.deps, Fixtures.CHECKOUT)
130+
131+ expect(probe.statted).toEqual([])
132+ expect(key).toBe(`rev:${Fixtures.HEAD_SHA}`)
133+ })
134+
82135 test('a HEAD that is a symbolic link is never read or stat-ed', async () => {
83136 const probe = recorded(
84137 {
@@ -1,13 +1,54 @@
1-import type { Args, ResultOf, SessionMessage } from 'claude-code'
1+import type { Args, FsEntry, On, ResultOf, SessionMessage } from 'claude-code'
22 import { describe, expect, mock, test, tier } from 'claude-code/testing'
3+import type { Engine } from 'claude-code/testing'
34
45 import Limits from '../hooks/limits'
56 import Names from '../hooks/names'
7+import Record from '../hooks/record'
68 import Fixtures from './fixtures'
79
810 tier('builtin')
911
1012 describe('register', () => {
13+ async function drawnRoundMerge($: Engine, on: On, finishedMs: number) {
14+ const script = { ...Fixtures.REPOSITORY, 'rev-parse --verify': 'merging' }
15+
16+ let listing = Fixtures.MERGING.map(([name, kind]): FsEntry => ({
17+ name,
18+ kind,
19+ size: 0,
20+ isLink: false,
21+ }))
22+
23+ on('fs.list', (_engine, e) => {
24+ const isGitDir = e.path === '/work/.git'
25+
26+ return { value: isGitDir ? listing : [] }
27+ })
28+
29+ const world = Fixtures.inRepository(on, script)
30+
31+ const drawn = async () => Fixtures.textOf(await $.ui.render(Fixtures.PANE))
32+
33+ await $.session.start(Fixtures.SESSION)
34+ await $.command.run(Fixtures.DIFF)
35+ await world.clock.advance(finishedMs)
36+
37+ const before = await drawn()
38+
39+ listing = []
40+ script['rev-parse --verify'] = 'merged'
41+
42+ await world.clock.advance(Limits.HEAD_POLL_MS + Fixtures.SETTLE_MS)
43+
44+ return { before, after: await drawn() }
45+ }
46+
47+ const NOTICED = {
48+ before: expect.stringContaining('Diff unavailable'),
49+ after: expect.stringContaining('1 file changed'),
50+ }
51+
1152 test('the start asks nothing of git and registers /diff', async ($, on) => {
1253 const world = Fixtures.inRepository(on)
1354
@@ -121,19 +162,19 @@
121162 )
122163 })
123164
124- test('a shell command the tool held read-only fetches nothing', async ($, on) => {
165+ test('a shell command held read-only fetches nothing', async ($, on) => {
125166 const world = Fixtures.inRepository(on)
126167
127168 const fetchesSince = (read: number) =>
128169 world.runs.slice(read).filter(run => run.argv.includes('--numstat'))
129170 .length
130171
131- on('tool.call', ($, e) =>
132- e.tool === 'Bash' && e.command === 'ls'
133- ? Fixtures.READ_ONLY_ANSWER
134- : { result: 'done' },
135- )
172+ on('tool.call', ($, e) => {
173+ const isListing = e.tool === 'Bash' && e.command === 'ls'
136174
175+ return isListing ? Fixtures.READ_ONLY_ANSWER : { result: 'done' }
176+ })
177+
137178 await $.session.start(Fixtures.SESSION)
138179 await $.command.run(Fixtures.DIFF)
139180 await world.clock.advance(Fixtures.SETTLE_MS)
@@ -383,7 +424,7 @@
383424 expect(world.opened.map(pane => pane.id)).toEqual(['diff'])
384425 })
385426
386- test('on the main screen the first edit opens nothing, and /diff still opens the dialog', async ($, on) => {
427+ test('on the main screen only /diff opens, as a dialog', async ($, on) => {
387428 const world = Fixtures.inRepository(on)
388429
389430 on('tool.call', () => ({ result: 'edited' }))
@@ -411,7 +452,7 @@
411452 ).toMatchObject({ id: 'diff', focus: true })
412453 })
413454
414- test('a surface that does not say whether it docks a pane opens nothing at the first edit', async ($, on) => {
455+ test('a surface silent on docking opens nothing by itself', async ($, on) => {
415456 const world = Fixtures.inRepository(on)
416457
417458 on('tool.call', () => ({ result: 'edited' }))
@@ -716,7 +757,7 @@
716757 expect(drawn).not.toContain('Loading diff')
717758 })
718759
719- test('/clear leaves the pane it finds open up and reads the repository afresh', async ($, on) => {
760+ test('/clear leaves an open pane up and reads afresh', async ($, on) => {
720761 const world = Fixtures.inRepository(on)
721762
722763 on('command.run', { command: 'clear' }, () => ({}))
@@ -745,7 +786,70 @@
745786 ).toContain('1 file changed')
746787 })
747788
748- test('a resumed session whose turns edited opens the pane before any new edit', async ($, on) => {
789+ test('/clear under an open pane counts it shown again', async ($, on) => {
790+ const world = Fixtures.inRepository(on)
791+ const logged = Fixtures.keeping<Args<'telemetry.log'>>()
792+
793+ let sessionId = 'first'
794+
795+ on('telemetry.log', logged.hook)
796+ on('session.id', () => ({ value: sessionId }))
797+ on('tool.call', () => ({ result: 'edited' }))
798+
799+ on('command.run', { command: 'clear' }, () => {
800+ sessionId = 'second'
801+
802+ return {}
803+ })
804+
805+ await $.session.start(Fixtures.SESSION)
806+ await $.ui.render(Fixtures.HINT)
807+
808+ await $.tool.call({
809+ tool: 'Edit',
810+ file_path: '/work/app.ts',
811+ old_string: '1',
812+ new_string: '2',
813+ })
814+
815+ await world.clock.advance(Fixtures.SETTLE_MS)
816+ await $.command.run(Fixtures.CLEAR)
817+ await world.clock.advance(Fixtures.SETTLE_MS)
818+
819+ expect(world.closed, 'the pane stayed up across /clear').toEqual([])
820+
821+ expect(
822+ logged.kept.map(shown => shown.props?.trigger),
823+ "once a conversation: Claude's edit opened the first, and the second, " +
824+ "which nobody opened, reads as the person's, as the built-in's does",
825+ ).toEqual([
826+ { value: 'auto_open', of: [...Record.SHOWN_TRIGGERS] },
827+ { value: 'manual', of: [...Record.SHOWN_TRIGGERS] },
828+ ])
829+ })
830+
831+ test('a merge finished 3 s after the open is noticed', async ($, on) => {
832+ expect(
833+ await drawnRoundMerge($, on, 3000),
834+ 'finished in another terminal: read with no edit or command',
835+ ).toEqual(NOTICED)
836+ })
837+
838+ test('a merge finished 1.2 s after the open is noticed', async ($, on) => {
839+ expect(
840+ await drawnRoundMerge($, on, 1200),
841+ 'before the first look at HEAD on the clock: the open read it',
842+ ).toEqual(NOTICED)
843+ })
844+
845+ test('a merge finished 0.3 s after the open is noticed', async ($, on) => {
846+ expect(
847+ await drawnRoundMerge($, on, 300),
848+ 'before the first look at HEAD on the clock: the open read it',
849+ ).toEqual(NOTICED)
850+ })
851+
852+ test('a resumed session that edited opens before an edit', async ($, on) => {
749853 const world = Fixtures.inRepository(on, Fixtures.REPOSITORY, {
750854 messages: () => Fixtures.EDITED_TRANSCRIPT,
751855 })
@@ -763,7 +867,7 @@
763867 ).toEqual(['diff'])
764868 })
765869
766- test('a resumed session opens nothing where its first edit would not', async ($, on) => {
870+ test('a resumed session opens only where an edit would', async ($, on) => {
767871 const narrow = Fixtures.inRepository(on, Fixtures.REPOSITORY, {
768872 messages: () => Fixtures.EDITED_TRANSCRIPT,
769873 })
@@ -779,7 +883,7 @@
779883 expect(narrow.runs, 'and no git for it').toEqual([])
780884 })
781885
782- test('a resumed session the person kept the pane open in opens it from the lower floor', async ($, on) => {
886+ test('a resumed session kept open opens at the low floor', async ($, on) => {
783887 const world = Fixtures.inRepository(on, Fixtures.REPOSITORY, {
784888 messages: () => Fixtures.EDITED_TRANSCRIPT,
785889 stored: { [Names.STORE_OPEN_KEY]: true },
@@ -805,7 +909,7 @@
805909 expect(world.runs).toEqual([])
806910 })
807911
808- test('a resumed session whose pane the person closed opens nothing', async ($, on) => {
912+ test('a resumed session the person closed opens nothing', async ($, on) => {
809913 const world = Fixtures.inRepository(on, Fixtures.REPOSITORY, {
810914 messages: () => Fixtures.EDITED_TRANSCRIPT,
811915 stored: { [Names.STORE_OPEN_KEY]: false },
@@ -819,7 +923,7 @@
819923 expect(world.runs).toEqual([])
820924 })
821925
822- test('/resume closes the pane, then opens it for the turns it brought back', async ($, on) => {
926+ test('/resume closes the pane, reopens it for its turns', async ($, on) => {
823927 let transcript: readonly SessionMessage[] = []
824928
825929 const world = Fixtures.inRepository(on, Fixtures.REPOSITORY, {
@@ -839,9 +943,10 @@
839943 await $.command.run(Fixtures.RESUME)
840944 await world.clock.advance(Fixtures.SETTLE_MS)
841945
842- expect(world.closed.map(pane => pane.id), 'closed at /resume').toEqual([
843- 'diff',
844- ])
946+ expect(
947+ world.closed.map(pane => pane.id),
948+ 'closed at /resume',
949+ ).toEqual(['diff'])
845950
846951 expect(
847952 world.opened.map(pane => pane.id),
@@ -934,7 +1039,7 @@
9341039 const { 'rev-parse --path-format=absolute': worktree = '', ...notYet } =
9351040 Fixtures.oneSecret()
9361041
937- const script: Record<string, string> = notYet
1042+ const script = { ...notYet }
9381043 const world = Fixtures.inRepository(on, script)
9391044
9401045 const probesOf = () =>
@@ -991,7 +1096,7 @@
9911096 const { 'rev-parse --path-format=absolute': worktree = '', ...notYet } =
9921097 Fixtures.oneSecret()
9931098
994- const script: Record<string, string> = notYet
1099+ const script = { ...notYet }
9951100 const world = Fixtures.inRepository(on, script)
9961101
9971102 on('tool.call', () => ({ result: 'edited' }))