Skip to content

Commit 1353b00

Browse files
frenchie4111claude
andauthored
feat: refresh file tabs and review diffs when files change on disk (#159)
## Summary - File tabs (`FileView`) and working-tree review diffs (`ReviewDiffPane`) now refresh automatically when the underlying file is changed on disk — e.g. when an agent in another Harness tab edits the file. - Introduces a refcounted per-file `fs.watch` manager (`FileContentWatcher`) and a `file:contentChanged` signal carried by the shared `ServerTransport` so both Electron-IPC and WebSocket backends are covered. - Dirty-buffer guard: if the user has unsaved edits when the file changes on disk, the in-flight value is preserved and a "File changed on disk" banner appears with Reload / Keep editing actions instead of clobbering work. Clean buffers reload silently. Binary files (image/pdf) always refetch. ## Architecture - `src/main/file-content-watcher.ts` — new refcounted watcher (one `fs.watch` handle per absolute path), debounced ~150ms, handles atomic-save `rename` events by re-arming after a short delay. - `src/main/index.ts` — `file:watchSubscribe` / `file:watchUnsubscribe` signals managed per-client; `file:contentChanged` rebroadcast over `transport.sendSignal`. - `src/renderer/hooks/useFileContentChange.ts` — thin hook around `backend.watchFile` + `onFileContentChanged`, callback held in a ref so subscription stays stable across parent re-renders. - No new state slice — disk-change notifications are high-frequency side-effect signals (same model as PTY data and the existing `worktree:changedFilesInvalidated` signal). ## Test plan - [x] `npm run typecheck` clean - [x] `npx electron-vite build` clean - [x] `npx vitest run src/main/file-content-watcher.test.ts` — 10 new tests pass (subscribe/notify, debounce, multi-subscriber, refcount, shutdown, missing-file resilience, atomic-save rearm) - [ ] Manual: open a file tab, write to the file from another terminal, confirm FileView updates within ~200ms - [ ] Manual: make uncommitted edits in FileView, then write to the file externally — banner appears with Reload / Keep editing; user edits preserved until Reload - [ ] Manual: open Review pane on a working-tree diff, edit the file from another terminal, confirm diff refreshes (no flicker since spinner suppressed on watcher refresh) - [ ] Manual: save from FileView — no infinite loop (watcher fires, content matches savedValue, no UI change) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c737cc1 commit 1353b00

8 files changed

Lines changed: 609 additions & 61 deletions

File tree

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
2+
import fs from 'fs'
3+
4+
import { FileContentWatcher } from './file-content-watcher'
5+
6+
type FsCallback = (eventType: string, filename: string | null) => void
7+
8+
interface FakeWatcher {
9+
path: string
10+
listener: FsCallback
11+
close: ReturnType<typeof vi.fn>
12+
}
13+
14+
let fakeWatchers: FakeWatcher[] = []
15+
16+
function fireFsEvent(path: string, eventType: string): void {
17+
for (const w of fakeWatchers) {
18+
if (w.path === path) w.listener(eventType, null)
19+
}
20+
}
21+
22+
function activeWatchers(path: string): FakeWatcher[] {
23+
return fakeWatchers.filter((w) => !w.close.mock.calls.length && w.path === path)
24+
}
25+
26+
beforeEach(() => {
27+
fakeWatchers = []
28+
vi.useFakeTimers()
29+
vi.spyOn(fs, 'watch').mockImplementation(((
30+
path: fs.PathLike,
31+
optionsOrListener?: unknown,
32+
maybeListener?: unknown
33+
) => {
34+
const listener = (typeof optionsOrListener === 'function'
35+
? optionsOrListener
36+
: maybeListener) as FsCallback
37+
const w: FakeWatcher = {
38+
path: String(path),
39+
listener,
40+
close: vi.fn()
41+
}
42+
fakeWatchers.push(w)
43+
return {
44+
close: w.close
45+
} as unknown as fs.FSWatcher
46+
}) as unknown as typeof fs.watch)
47+
})
48+
49+
afterEach(() => {
50+
vi.useRealTimers()
51+
vi.restoreAllMocks()
52+
})
53+
54+
describe('FileContentWatcher', () => {
55+
it('notifies a single subscriber after debounce window elapses', () => {
56+
const watcher = new FileContentWatcher()
57+
const listener = vi.fn()
58+
watcher.subscribe('/wt/a/file.txt', listener)
59+
60+
fireFsEvent('/wt/a/file.txt', 'change')
61+
expect(listener).not.toHaveBeenCalled()
62+
63+
vi.advanceTimersByTime(149)
64+
expect(listener).not.toHaveBeenCalled()
65+
66+
vi.advanceTimersByTime(1)
67+
expect(listener).toHaveBeenCalledTimes(1)
68+
})
69+
70+
it('coalesces multiple events within the debounce window into one notification', () => {
71+
const watcher = new FileContentWatcher()
72+
const listener = vi.fn()
73+
watcher.subscribe('/wt/a/file.txt', listener)
74+
75+
fireFsEvent('/wt/a/file.txt', 'change')
76+
fireFsEvent('/wt/a/file.txt', 'change')
77+
fireFsEvent('/wt/a/file.txt', 'change')
78+
79+
vi.advanceTimersByTime(150)
80+
expect(listener).toHaveBeenCalledTimes(1)
81+
})
82+
83+
it('notifies every subscriber on the same path', () => {
84+
const watcher = new FileContentWatcher()
85+
const a = vi.fn()
86+
const b = vi.fn()
87+
watcher.subscribe('/wt/a/file.txt', a)
88+
watcher.subscribe('/wt/a/file.txt', b)
89+
90+
fireFsEvent('/wt/a/file.txt', 'change')
91+
vi.advanceTimersByTime(150)
92+
93+
expect(a).toHaveBeenCalledTimes(1)
94+
expect(b).toHaveBeenCalledTimes(1)
95+
})
96+
97+
it('reuses the underlying fs.watch handle across subscribers', () => {
98+
const watcher = new FileContentWatcher()
99+
watcher.subscribe('/wt/a/file.txt', vi.fn())
100+
watcher.subscribe('/wt/a/file.txt', vi.fn())
101+
102+
expect(activeWatchers('/wt/a/file.txt')).toHaveLength(1)
103+
})
104+
105+
it('reference-counts: unsubscribing one keeps the watcher alive for the rest', () => {
106+
const watcher = new FileContentWatcher()
107+
const a = vi.fn()
108+
const b = vi.fn()
109+
const offA = watcher.subscribe('/wt/a/file.txt', a)
110+
watcher.subscribe('/wt/a/file.txt', b)
111+
112+
offA()
113+
114+
fireFsEvent('/wt/a/file.txt', 'change')
115+
vi.advanceTimersByTime(150)
116+
117+
expect(a).not.toHaveBeenCalled()
118+
expect(b).toHaveBeenCalledTimes(1)
119+
expect(activeWatchers('/wt/a/file.txt')).toHaveLength(1)
120+
})
121+
122+
it('closes the underlying fs.watch handle when the last subscriber leaves', () => {
123+
const watcher = new FileContentWatcher()
124+
const offA = watcher.subscribe('/wt/a/file.txt', vi.fn())
125+
const offB = watcher.subscribe('/wt/a/file.txt', vi.fn())
126+
127+
const created = fakeWatchers.filter((w) => w.path === '/wt/a/file.txt')
128+
expect(created).toHaveLength(1)
129+
expect(created[0].close).not.toHaveBeenCalled()
130+
131+
offA()
132+
expect(created[0].close).not.toHaveBeenCalled()
133+
134+
offB()
135+
expect(created[0].close).toHaveBeenCalledTimes(1)
136+
})
137+
138+
it('keeps subscriptions on different paths independent', () => {
139+
const watcher = new FileContentWatcher()
140+
const a = vi.fn()
141+
const b = vi.fn()
142+
watcher.subscribe('/wt/a/file.txt', a)
143+
watcher.subscribe('/wt/b/file.txt', b)
144+
145+
fireFsEvent('/wt/a/file.txt', 'change')
146+
vi.advanceTimersByTime(150)
147+
148+
expect(a).toHaveBeenCalledTimes(1)
149+
expect(b).not.toHaveBeenCalled()
150+
})
151+
152+
it('shutdown closes every open watcher and clears entries', () => {
153+
const watcher = new FileContentWatcher()
154+
watcher.subscribe('/wt/a/file.txt', vi.fn())
155+
watcher.subscribe('/wt/b/file.txt', vi.fn())
156+
157+
const aWatcher = fakeWatchers.find((w) => w.path === '/wt/a/file.txt')!
158+
const bWatcher = fakeWatchers.find((w) => w.path === '/wt/b/file.txt')!
159+
160+
watcher.shutdown()
161+
162+
expect(aWatcher.close).toHaveBeenCalledTimes(1)
163+
expect(bWatcher.close).toHaveBeenCalledTimes(1)
164+
})
165+
166+
it('survives fs.watch throwing (e.g. file missing) and produces no notifications', () => {
167+
;(fs.watch as unknown as ReturnType<typeof vi.fn>).mockImplementationOnce(() => {
168+
throw new Error('ENOENT')
169+
})
170+
171+
const watcher = new FileContentWatcher()
172+
const listener = vi.fn()
173+
expect(() => watcher.subscribe('/wt/missing/file.txt', listener)).not.toThrow()
174+
175+
vi.advanceTimersByTime(500)
176+
expect(listener).not.toHaveBeenCalled()
177+
})
178+
179+
it('re-arms the watcher after a rename event (atomic save)', () => {
180+
const watcher = new FileContentWatcher()
181+
const listener = vi.fn()
182+
watcher.subscribe('/wt/a/file.txt', listener)
183+
184+
expect(activeWatchers('/wt/a/file.txt')).toHaveLength(1)
185+
const original = fakeWatchers.find((w) => w.path === '/wt/a/file.txt')!
186+
187+
// Atomic-save: editor renames-over the inode. fs.watch fires 'rename'.
188+
fireFsEvent('/wt/a/file.txt', 'rename')
189+
190+
// Debounced notify fires for the rename event itself.
191+
vi.advanceTimersByTime(150)
192+
expect(listener).toHaveBeenCalledTimes(1)
193+
194+
// After the rearm delay the original watcher is closed and a fresh
195+
// one is opened against the same path.
196+
vi.advanceTimersByTime(50)
197+
expect(original.close).toHaveBeenCalledTimes(1)
198+
expect(activeWatchers('/wt/a/file.txt')).toHaveLength(1)
199+
200+
// Subsequent change on the re-armed watcher still notifies.
201+
fireFsEvent('/wt/a/file.txt', 'change')
202+
vi.advanceTimersByTime(150)
203+
expect(listener).toHaveBeenCalledTimes(2)
204+
})
205+
})

src/main/file-content-watcher.ts

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Reference-counted fs.watch manager for per-file content notifications.
2+
// Mirrors WorktreeWatcher's structure but watches a single absolute file
3+
// path per entry instead of the worktree's .git/ directory. Used by the
4+
// file:watchSubscribe IPC so FileView / ReviewDiffPane can refresh when an
5+
// agent in another tab (or any external tool) edits the file on disk.
6+
//
7+
// Atomic-save robustness: many editors (Vim, IntelliJ, some Claude-Code
8+
// edit flows) replace the file's inode on save. fs.watch loses the
9+
// underlying handle on rename; we detect that via the 'rename' event and
10+
// re-arm by re-opening fs.watch after a small delay, retrying once if
11+
// the file is briefly missing. If the file is gone after the retry we
12+
// give up — subscribers stay registered but get no notifications until
13+
// the file reappears via a fresh subscribe.
14+
//
15+
// If the very first fs.watch open fails (file missing, EPERM, etc.), the
16+
// entry is created without a watcher and listeners never fire. This is
17+
// intentional and matches WorktreeWatcher — the watcher is best-effort
18+
// and the renderer has its own ways to refresh as a fallback.
19+
//
20+
// Notifications are debounced ~150ms (slightly tighter than the worktree
21+
// watcher's 200ms) to coalesce editor multi-write saves.
22+
23+
import fs, { type FSWatcher } from 'fs'
24+
25+
const DEBOUNCE_MS = 150
26+
const REARM_DELAY_MS = 50
27+
28+
export type FileContentChangeListener = () => void
29+
30+
interface Entry {
31+
listeners: Set<FileContentChangeListener>
32+
watcher: FSWatcher | null
33+
debounceTimer: NodeJS.Timeout | null
34+
rearmTimer: NodeJS.Timeout | null
35+
}
36+
37+
export class FileContentWatcher {
38+
private readonly entries = new Map<string, Entry>()
39+
40+
subscribe(absolutePath: string, listener: FileContentChangeListener): () => void {
41+
let entry = this.entries.get(absolutePath)
42+
if (!entry) {
43+
entry = {
44+
listeners: new Set(),
45+
watcher: null,
46+
debounceTimer: null,
47+
rearmTimer: null
48+
}
49+
this.entries.set(absolutePath, entry)
50+
entry.watcher = this.openWatcher(absolutePath)
51+
}
52+
entry.listeners.add(listener)
53+
return () => this.unsubscribe(absolutePath, listener)
54+
}
55+
56+
shutdown(): void {
57+
for (const [path, entry] of this.entries) {
58+
this.teardownEntry(entry)
59+
this.entries.delete(path)
60+
}
61+
}
62+
63+
private unsubscribe(absolutePath: string, listener: FileContentChangeListener): void {
64+
const entry = this.entries.get(absolutePath)
65+
if (!entry) return
66+
entry.listeners.delete(listener)
67+
if (entry.listeners.size === 0) {
68+
this.teardownEntry(entry)
69+
this.entries.delete(absolutePath)
70+
}
71+
}
72+
73+
private teardownEntry(entry: Entry): void {
74+
if (entry.debounceTimer) {
75+
clearTimeout(entry.debounceTimer)
76+
entry.debounceTimer = null
77+
}
78+
if (entry.rearmTimer) {
79+
clearTimeout(entry.rearmTimer)
80+
entry.rearmTimer = null
81+
}
82+
if (entry.watcher) {
83+
try {
84+
entry.watcher.close()
85+
} catch {
86+
// already closed, ignore
87+
}
88+
entry.watcher = null
89+
}
90+
}
91+
92+
private openWatcher(absolutePath: string): FSWatcher | null {
93+
const onEvent = (eventType: string): void => {
94+
this.scheduleNotify(absolutePath)
95+
if (eventType === 'rename') {
96+
this.scheduleRearm(absolutePath)
97+
}
98+
}
99+
try {
100+
return fs.watch(absolutePath, { persistent: false }, onEvent)
101+
} catch {
102+
return null
103+
}
104+
}
105+
106+
private scheduleNotify(absolutePath: string): void {
107+
const entry = this.entries.get(absolutePath)
108+
if (!entry) return
109+
if (entry.debounceTimer) return
110+
entry.debounceTimer = setTimeout(() => {
111+
const current = this.entries.get(absolutePath)
112+
if (!current) return
113+
current.debounceTimer = null
114+
for (const listener of current.listeners) {
115+
try {
116+
listener()
117+
} catch {
118+
// listener errors must not break sibling listeners
119+
}
120+
}
121+
}, DEBOUNCE_MS)
122+
}
123+
124+
private scheduleRearm(absolutePath: string): void {
125+
const entry = this.entries.get(absolutePath)
126+
if (!entry) return
127+
if (entry.rearmTimer) return
128+
entry.rearmTimer = setTimeout(() => {
129+
const current = this.entries.get(absolutePath)
130+
if (!current) return
131+
current.rearmTimer = null
132+
if (current.watcher) {
133+
try {
134+
current.watcher.close()
135+
} catch {
136+
// ignore
137+
}
138+
current.watcher = null
139+
}
140+
current.watcher = this.openWatcher(absolutePath)
141+
}, REARM_DELAY_MS)
142+
}
143+
}

0 commit comments

Comments
 (0)