Align FolderWatcher with the newly ratified design decisions
The parallel design session resolved the m3 watcher findings; two changed behavior: .git internals are now filtered at any depth (a nested clone or submodule is a stray whose internals never render), and a root change landing mid-bracket is owned by the root-change path — endBracket() skips its mandatory reload once a .rootChanged tore the stream down, cleared when a fresh stream attaches. The skip is scoped by an explicit flag, not by stream absence: a watcher whose stream never came up still delivers post-bracket reloads, since distrust of FSEvents delivery is the mandatory reload's whole reason. Two new watcher tests; full suite 281 tests in 54 suites green. Claude-Session: https://claude.ai/code/session_018BjQRYBR6jQja3jCRi5S3A
This commit is contained in:
@@ -136,6 +136,14 @@ public final class FolderWatcher {
|
|||||||
/// that use them do: an undo restore inside a branch switch is one bracketed span, not two.
|
/// that use them do: an undo restore inside a branch switch is one bracketed span, not two.
|
||||||
private var bracketDepth = 0
|
private var bracketDepth = 0
|
||||||
|
|
||||||
|
/// True from a delivered `.rootChanged` until a fresh stream attaches (`start()` or
|
||||||
|
/// `reattach(to:)`). While set, `endBracket()` skips its mandatory reload — the root-change
|
||||||
|
/// path owns recovery (settled, 02-architecture.md § Write-failure surfacing). Deliberately
|
||||||
|
/// **not** the same test as `handle == nil`: a watcher that never got a stream (or whose
|
||||||
|
/// creation failed on some degraded volume) must still deliver post-bracket reloads —
|
||||||
|
/// distrust of FSEvents delivery is the reason the mandatory reload exists at all.
|
||||||
|
private var rootChangeOwnsRecovery = false
|
||||||
|
|
||||||
/// The origin a pending delivery will carry, merged per `WatchOrigin.merged(_:_:)`. Non-nil
|
/// The origin a pending delivery will carry, merged per `WatchOrigin.merged(_:_:)`. Non-nil
|
||||||
/// means "a delivery is owed" — which is not the same as "a timer is running": while a
|
/// means "a delivery is owed" — which is not the same as "a timer is running": while a
|
||||||
/// bracket is open the origin is remembered with no timer armed, and `endBracket()` merges
|
/// bracket is open the origin is remembered with no timer armed, and `endBracket()` merges
|
||||||
@@ -264,6 +272,9 @@ public final class FolderWatcher {
|
|||||||
}
|
}
|
||||||
|
|
||||||
handle = StreamHandle(stream: stream)
|
handle = StreamHandle(stream: stream)
|
||||||
|
// A fresh stream at whatever root we now watch: if a root change had handed recovery to
|
||||||
|
// the root-change path, that handoff is complete.
|
||||||
|
rootChangeOwnsRecovery = false
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -295,6 +306,11 @@ public final class FolderWatcher {
|
|||||||
stop()
|
stop()
|
||||||
watchedPath = Self.canonicalPath(of: newRoot)
|
watchedPath = Self.canonicalPath(of: newRoot)
|
||||||
_ = start()
|
_ = start()
|
||||||
|
// Reattaching completes the root-change handoff even if the fresh stream failed to come
|
||||||
|
// up (`start()` clears the flag only on success): the reconciling reload below is
|
||||||
|
// promised unconditionally, and mid-bracket it must fold into the post-bracket delivery
|
||||||
|
// rather than be stranded behind `endBracket()`'s root-change skip.
|
||||||
|
rootChangeOwnsRecovery = false
|
||||||
schedule(.reconciling)
|
schedule(.reconciling)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -326,16 +342,24 @@ public final class FolderWatcher {
|
|||||||
cancelPendingDelivery()
|
cancelPendingDelivery()
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Closes a bracket. At depth 0 this **always** schedules the debounced
|
/// Closes a bracket. At depth 0 this schedules the debounced `.treeChanged(.appMediated)` —
|
||||||
/// `.treeChanged(.appMediated)` — the mandatory single post-bracket reload — even if not one
|
/// the mandatory single post-bracket reload — even if not one filesystem event was seen
|
||||||
/// filesystem event was seen inside the bracket. The bracket's contract is "finish with one
|
/// inside the bracket. The bracket's contract is "finish with one full reload"; making that
|
||||||
/// full reload"; making that conditional on having observed events would make correctness
|
/// conditional on having observed events would make correctness depend on FSEvents delivery,
|
||||||
/// depend on FSEvents delivery, which is precisely the thing this design refuses to trust.
|
/// which is precisely the thing this design refuses to trust.
|
||||||
///
|
///
|
||||||
/// FSEvents produced by the bracketed operation and still in flight when it closes (kernel
|
/// FSEvents produced by the bracketed operation and still in flight when it closes (kernel
|
||||||
/// latency does not respect our brackets) simply coalesce into that pending delivery via the
|
/// latency does not respect our brackets) simply coalesce into that pending delivery via the
|
||||||
/// origin merge, arriving as `.appMediated` rather than as a second `.foreign` reload.
|
/// origin merge, arriving as `.appMediated` rather than as a second `.foreign` reload.
|
||||||
///
|
///
|
||||||
|
/// **The one exception: a root change that landed mid-bracket** (settled, 02-architecture.md
|
||||||
|
/// § Write-failure surfacing — "owned by the root-change path, not the bracket"). A
|
||||||
|
/// `.rootChanged` tears the stream down, so with no live stream at close the mandatory
|
||||||
|
/// reload is skipped as a no-op: a rename the consumer absorbed mid-bracket has already
|
||||||
|
/// re-attached by now (its banked reconciling reload delivers here instead, at the
|
||||||
|
/// re-resolved root), and a true vanish raised the read-only lock whose clearance runs a
|
||||||
|
/// reconciling reload of its own — nothing is lost by not failing redundantly.
|
||||||
|
///
|
||||||
/// An unbalanced call — `endBracket()` at depth 0 — is ignored rather than trapping: the
|
/// An unbalanced call — `endBracket()` at depth 0 — is ignored rather than trapping: the
|
||||||
/// consumer's brackets wrap `do`/`catch` spans over git operations, and a bug there should
|
/// consumer's brackets wrap `do`/`catch` spans over git operations, and a bug there should
|
||||||
/// not take the app down.
|
/// not take the app down.
|
||||||
@@ -343,6 +367,7 @@ public final class FolderWatcher {
|
|||||||
guard bracketDepth > 0 else { return }
|
guard bracketDepth > 0 else { return }
|
||||||
bracketDepth -= 1
|
bracketDepth -= 1
|
||||||
guard bracketDepth == 0 else { return }
|
guard bracketDepth == 0 else { return }
|
||||||
|
guard !rootChangeOwnsRecovery else { return }
|
||||||
schedule(.appMediated)
|
schedule(.appMediated)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -466,9 +491,11 @@ public final class FolderWatcher {
|
|||||||
///
|
///
|
||||||
/// Bypassing the bracket is deliberate and is the one place the "nothing fires mid-bracket"
|
/// Bypassing the bracket is deliberate and is the one place the "nothing fires mid-bracket"
|
||||||
/// rule yields: a bracketed operation whose root disappeared underneath it cannot finish, and
|
/// rule yields: a bracketed operation whose root disappeared underneath it cannot finish, and
|
||||||
/// its post-bracket reload would resolve against a path that is gone. The consumer must know
|
/// its post-bracket reload would resolve against a path that is gone (with the stream torn
|
||||||
/// now, so it can re-resolve its bookmark and either `reattach(to:)` or enter the
|
/// down here, `endBracket()` skips that reload — the root-change path owns recovery from this
|
||||||
/// vanished-root read-only lock (02-architecture.md § Write-failure surfacing).
|
/// point, settled). The consumer must know now, so it can re-resolve its bookmark and either
|
||||||
|
/// `reattach(to:)` or enter the vanished-root read-only lock (02-architecture.md
|
||||||
|
/// § Write-failure surfacing).
|
||||||
///
|
///
|
||||||
/// Any pending debounced delivery is dropped: it was going to report on a tree that no longer
|
/// Any pending debounced delivery is dropped: it was going to report on a tree that no longer
|
||||||
/// exists at that path, and whatever the consumer does next — `reattach(to:)` or the lock —
|
/// exists at that path, and whatever the consumer does next — `reattach(to:)` or the lock —
|
||||||
@@ -477,12 +504,14 @@ public final class FolderWatcher {
|
|||||||
teardownStream()
|
teardownStream()
|
||||||
cancelPendingDelivery()
|
cancelPendingDelivery()
|
||||||
pendingOrigin = nil
|
pendingOrigin = nil
|
||||||
|
rootChangeOwnsRecovery = true
|
||||||
handler(.rootChanged)
|
handler(.rootChanged)
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - .git filtering
|
// MARK: - .git filtering
|
||||||
|
|
||||||
/// Whether `path` lives inside the board root's `.git` directory.
|
/// Whether `path` lives under any `.git` component inside the board tree (settled,
|
||||||
|
/// 02-architecture.md § Components).
|
||||||
///
|
///
|
||||||
/// **Why filter at all**: the app auto-commits a couple of seconds after every change
|
/// **Why filter at all**: the app auto-commits a couple of seconds after every change
|
||||||
/// (06-history-undo.md), so every single edit the user makes is followed by a burst of writes
|
/// (06-history-undo.md), so every single edit the user makes is followed by a burst of writes
|
||||||
@@ -491,6 +520,11 @@ public final class FolderWatcher {
|
|||||||
/// redundant full tree walk. None of that churn can alter the rendered tree: the loader walks
|
/// redundant full tree walk. None of that churn can alter the rendered tree: the loader walks
|
||||||
/// UUID-shaped folders and `index.md` files, and `.git` contains neither.
|
/// UUID-shaped folders and `index.md` files, and `.git` contains neither.
|
||||||
///
|
///
|
||||||
|
/// **Any depth, not just the root's own repo**: a repo nested deeper — a card folder
|
||||||
|
/// containing a clone, a submodule — is a stray (01-storage-format.md) whose internals never
|
||||||
|
/// render either, so its churn is filtered on the same grounds. The nested repo's *working
|
||||||
|
/// files* are ordinary stray paths and still fire like anything else.
|
||||||
|
///
|
||||||
/// **Why it is safe**: this filters `.git`'s *internals*, not git's effects. An external
|
/// **Why it is safe**: this filters `.git`'s *internals*, not git's effects. An external
|
||||||
/// `git checkout`, `git pull`, or `git stash` rewrites working-tree files, and those events
|
/// `git checkout`, `git pull`, or `git stash` rewrites working-tree files, and those events
|
||||||
/// arrive unfiltered in the same batch as the `.git` writes — the batch has a relevant path,
|
/// arrive unfiltered in the same batch as the `.git` writes — the batch has a relevant path,
|
||||||
@@ -503,7 +537,7 @@ public final class FolderWatcher {
|
|||||||
guard path.hasPrefix(watchedPath) else { return false }
|
guard path.hasPrefix(watchedPath) else { return false }
|
||||||
let relative = path.dropFirst(watchedPath.count)
|
let relative = path.dropFirst(watchedPath.count)
|
||||||
guard relative.hasPrefix("/") else { return false }
|
guard relative.hasPrefix("/") else { return false }
|
||||||
return relative.dropFirst().hasPrefix(".git/") || relative.dropFirst() == ".git"
|
return relative.split(separator: "/").contains(".git")
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - Helpers
|
// MARK: - Helpers
|
||||||
|
|||||||
@@ -344,6 +344,34 @@ struct FolderWatcherTests {
|
|||||||
#expect(log.events == [.treeChanged(.foreign)])
|
#expect(log.events == [.treeChanged(.foreign)])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test("A repo nested inside a card folder is filtered like the root's own")
|
||||||
|
func nestedGitChurnIsFiltered() async throws {
|
||||||
|
let fixture = try WatchFixture()
|
||||||
|
defer { fixture.tearDown() }
|
||||||
|
let cardFolder = "1e9a7c5e-0000-4000-8000-000000000000"
|
||||||
|
fixture.makeDirectory("\(cardFolder)/.git")
|
||||||
|
let log = EventLog()
|
||||||
|
let watcher = FolderWatcher(root: fixture.root, debounce: testDebounce, latency: testLatency) {
|
||||||
|
log.record($0)
|
||||||
|
}
|
||||||
|
#expect(watcher.start())
|
||||||
|
defer { watcher.stop() }
|
||||||
|
await drainStartupChurn(log)
|
||||||
|
|
||||||
|
// A clone living inside a card folder is a stray (01-storage-format.md) whose internals
|
||||||
|
// never render — filtered at any depth, not just the board root's own repo (02, settled).
|
||||||
|
fixture.write("\(cardFolder)/.git/index", "fake index")
|
||||||
|
fixture.write("\(cardFolder)/.git/refs/heads/main", "deadbeef")
|
||||||
|
await quiet(.milliseconds(700))
|
||||||
|
#expect(log.events.isEmpty)
|
||||||
|
|
||||||
|
// The nested repo's *working files* are ordinary stray paths and still fire.
|
||||||
|
fixture.write("\(cardFolder)/README.md")
|
||||||
|
await waitUntil { log.count >= 1 }
|
||||||
|
await quiet()
|
||||||
|
#expect(log.events == [.treeChanged(.foreign)])
|
||||||
|
}
|
||||||
|
|
||||||
// MARK: Root identity
|
// MARK: Root identity
|
||||||
|
|
||||||
@Test("Deleting the watched root delivers rootChanged and tears the stream down")
|
@Test("Deleting the watched root delivers rootChanged and tears the stream down")
|
||||||
@@ -367,6 +395,33 @@ struct FolderWatcherTests {
|
|||||||
#expect(!watcher.isWatching)
|
#expect(!watcher.isWatching)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test("A root vanish mid-bracket is owned by the root-change path; endBracket() is a no-op")
|
||||||
|
func rootVanishMidBracketSkipsThePostBracketReload() async throws {
|
||||||
|
let fixture = try WatchFixture()
|
||||||
|
defer { fixture.tearDown() }
|
||||||
|
let log = EventLog()
|
||||||
|
let watcher = FolderWatcher(root: fixture.root, debounce: testDebounce, latency: testLatency) {
|
||||||
|
log.record($0)
|
||||||
|
}
|
||||||
|
#expect(watcher.start())
|
||||||
|
defer { watcher.stop() }
|
||||||
|
await drainStartupChurn(log)
|
||||||
|
|
||||||
|
watcher.beginBracket()
|
||||||
|
fixture.remove()
|
||||||
|
// `.rootChanged` bypasses the bracket — the consumer must know *now*.
|
||||||
|
await waitUntil(.seconds(10)) { log.events.contains(.rootChanged) }
|
||||||
|
#expect(log.events == [.rootChanged])
|
||||||
|
#expect(!watcher.isWatching)
|
||||||
|
|
||||||
|
// The bracket's mandatory final reload is skipped: the stream is gone, and the
|
||||||
|
// root-change path owns recovery from here (02, settled) — `reattach(to:)` or the
|
||||||
|
// vanished-root lock, each ending in a reload of its own.
|
||||||
|
watcher.endBracket()
|
||||||
|
await quiet(.milliseconds(700))
|
||||||
|
#expect(log.events == [.rootChanged])
|
||||||
|
}
|
||||||
|
|
||||||
// MARK: Stop
|
// MARK: Stop
|
||||||
|
|
||||||
@Test("Nothing is delivered after stop()")
|
@Test("Nothing is delivered after stop()")
|
||||||
|
|||||||
Reference in New Issue
Block a user