Skip to content

Commit 0debff8

Browse files
AnakinRaWclaude
AnakinRaW
and
claude
committed
add design spec for live virtual file-exists strategy
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 76ede11 commit 0debff8

1 file changed

Lines changed: 180 additions & 0 deletions

File tree

‎docs/superpowers/specs/2026-05-10-live-virtual-file-exists-strategy-design.md‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
# Live Virtual File-Exists Strategy
2+
3+
**Status:** Design approved, awaiting implementation plan
4+
**Date:** 2026-05-10
5+
**Component:** `PG.StarWarsGame.Engine.FileSystem`
6+
7+
## Problem
8+
9+
`VirtualFileExistsStrategy` builds a per-directory snapshot of file listings on first lookup and caches it for the lifetime of the strategy. The snapshot never refreshes. For consumers that hold the strategy alive across externally-driven file system changes (mod editors, an embedded engine host), a deleted or newly-added file under the game directory remains undetected until the strategy is recreated.
10+
11+
ModVerify itself does not need this — it scans once and disposes — but `PG.StarWarsGame.Engine.FileSystem` is a general-purpose library and other consumers do.
12+
13+
## Goals
14+
15+
- Add an opt-in variant of the virtual strategy that keeps its snapshots fresh under file additions, deletions, and renames within the game directory.
16+
- Avoid creating one watcher per cached directory; one process-wide watcher per strategy instance.
17+
- Guarantee that the watcher's invalidation handler and concurrent `FileExists` callers do not race on shared mutable state.
18+
- Stay opt-in. Default behavior (`UseVirtualStrategy`) is unchanged.
19+
20+
## Non-goals
21+
22+
- Tracking file *content* changes. `FileExists` only reports existence; mtime/size do not affect correctness.
23+
- Notifying consumers of file system changes (no public events, no callbacks).
24+
- A "live" variant for `WindowsFileExistsStrategy` or `WineFileExistsStrategy`. Those strategies do not cache, so live mode is meaningless for them.
25+
- Debouncing / coalescing event bursts at the watcher layer. Lazy rebuild already coalesces: N events on the same directory collapse to a single re-snapshot at next access.
26+
- Filtering to a directory subtree narrower than the game directory.
27+
- A periodic rescan / TTL backstop.
28+
29+
## Design
30+
31+
### API
32+
33+
A new public method on `PetroglyphFileSystem`:
34+
35+
```csharp
36+
public void UseLiveVirtualStrategy(bool? windowsFallback = null);
37+
```
38+
39+
Signature mirrors `UseVirtualStrategy`'s nullable form: `null` selects the Windows strategy on Windows hosts and the Wine strategy otherwise; an explicit `true` forces the Windows strategy and throws `PlatformNotSupportedException` on non-Windows hosts; an explicit `false` forces the Wine strategy on every host.
40+
41+
Selecting it swaps the active strategy via the existing `SwapStrategy` path, which disposes the previous strategy.
42+
43+
`UseVirtualStrategy` retains its (nullable) signature and behavior — purely additive surface change.
44+
45+
### Components
46+
47+
A new internal class `LiveVirtualFileExistsStrategy : FileExistsStrategy`, a sibling of `VirtualFileExistsStrategy` in `PG.StarWarsGame.Engine.IO.FileExistStrategies`. It holds:
48+
49+
- The same `ConcurrentDictionary<string, VirtualDirectory?> _store` keyed by directory path (case-insensitive), with the same `VirtualDirectory` (immutable, files-only, on-disk casing) value type as the non-live strategy.
50+
- The same `underlying` fallback strategy passed in by `PetroglyphFileSystem.UseLiveVirtualStrategy`.
51+
- A `FileSystemWatcher? _watcher`, started lazily.
52+
- A `string? _watchedRoot` recording the game directory the watcher was bound to.
53+
54+
The directory snapshot logic (`TrySnapshot`, `TryResolveDirectory`, `IsUnderGameDirectory`) is identical to `VirtualFileExistsStrategy`. To avoid duplication, those helpers move to a `protected` location on a shared abstract base or a static helper class. The exact split is an implementation-plan concern, not a design decision.
55+
56+
### Watcher lifecycle
57+
58+
The watcher is created **lazily** on the first `FileExists` call that lands inside the game directory — that is the first point at which the strategy learns the game directory path (it is passed per-call, not at construction).
59+
60+
```csharp
61+
public override bool FileExists(ReadOnlySpan<char> gameDirectory, ref ValueStringBuilder sb)
62+
{
63+
if (IsUnderGameDirectory(sb.AsSpan(), gameDirectory))
64+
EnsureWatcher(gameDirectory);
65+
// ... rest identical to VirtualFileExistsStrategy
66+
}
67+
```
68+
69+
`EnsureWatcher` is idempotent: it only constructs a watcher when `_watcher` is null **and** the resolved game directory exists on disk. The watcher binds to the first observed game directory and stays bound to it for the strategy's lifetime — `gameDirectory` changing mid-flight is not an expected operating mode for this library, and the strategy makes no attempt to detect or react to it. Consumers who change the game directory should swap the strategy (which disposes this one and tears down the watcher).
70+
71+
Watcher configuration:
72+
73+
| Property | Value |
74+
|-------------------------|-------|
75+
| `Path` | resolved game directory |
76+
| `IncludeSubdirectories` | `true` |
77+
| `NotifyFilters` | `FileName \| DirectoryName` |
78+
| `InternalBufferSize` | `65536` (64 KB; default is 8 KB) |
79+
| `EnableRaisingEvents` | `true` after subscribing |
80+
81+
Subscribed events: `Created`, `Deleted`, `Renamed`, `Changed`, `Error`.
82+
83+
`Dispose()` sets `EnableRaisingEvents = false`, disposes the watcher, clears `_store`, and disposes the underlying.
84+
85+
### Concurrency model — lazy invalidation
86+
87+
The watcher handler does **one** thing: remove the affected directory's entry from `_store`. It never rebuilds and never blocks on a `FileExists` call.
88+
89+
```csharp
90+
private void OnFsEvent(string fullPath)
91+
{
92+
var dirKey = FileSystem.Path.GetDirectoryName(fullPath);
93+
if (dirKey is not null)
94+
_store.TryRemove(dirKey, out _);
95+
}
96+
```
97+
98+
Two facts make this race-free without locking:
99+
100+
1. `VirtualDirectory` is already immutable.
101+
2. `ConcurrentDictionary` operations (`TryRemove`, `TryAdd`, `TryGetValue`) are atomic.
102+
103+
Therefore a concurrent `FileExists` mid-lookup either:
104+
105+
- already captured the now-stale `VirtualDirectory` reference (returns a slightly-stale-but-consistent answer — no torn read because the snapshot is immutable), **or**
106+
- finds the entry gone on its next `TryGetValue` and rebuilds via `TrySnapshot`.
107+
108+
No two threads ever mutate the same `VirtualDirectory`. The watcher thread never blocks on application work.
109+
110+
### Event mapping
111+
112+
| Event | Action |
113+
|-------------------------------------|--------|
114+
| `Created` / `Deleted` / `Changed` (file) | `TryRemove(parentDir(fullPath))` |
115+
| `Renamed` (file) | `TryRemove(parentDir(oldPath))` and `TryRemove(parentDir(newPath))` |
116+
| `Deleted` / `Renamed` (directory) | `TryRemove(dirPath)` and `TryRemove(k)` for every `k` in `_store.Keys` where `k` is `dirPath` or starts with `dirPath + sep` (case-insensitive) |
117+
| `Created` (directory) | No action. Uncached dirs cost nothing; first lookup will snapshot. |
118+
| `Error` | `_store.Clear()` (see Reliability) |
119+
120+
`Changed` is included even though content changes do not matter, because some platforms surface delete/create as `Changed` on the parent. The handler is the same `TryRemove` call regardless — cheap.
121+
122+
The directory-deletion case is the only one that walks `_store.Keys`. The cache is small (at most the directories actually consulted by the engine, typically O(10²)), so the linear scan is fine.
123+
124+
### Reliability backstop
125+
126+
`FileSystemWatcher` drops events when its internal buffer overflows. On Linux, inotify has per-user/per-instance limits that get tripped silently under heavy churn.
127+
128+
Two mitigations, both standard and inexpensive:
129+
130+
1. **Raise `InternalBufferSize` to 64 KB.** Default is 8 KB; the larger buffer measurably reduces overflow under bursty changes.
131+
2. **Subscribe to `FileSystemWatcher.Error`.** On any error event, call `_store.Clear()`. Every subsequent `FileExists` rebuilds its directory snapshot from disk. This is the canonical FSW recovery pattern and does not introduce any periodic work.
132+
133+
No TTL, no periodic rescan, no per-entry timestamp checks. `Error → Clear` is the only backstop.
134+
135+
### Disposal
136+
137+
```csharp
138+
public override void Dispose()
139+
{
140+
var w = _watcher;
141+
_watcher = null;
142+
if (w is not null)
143+
{
144+
w.EnableRaisingEvents = false;
145+
w.Created -= OnCreatedOrDeletedOrChanged;
146+
// ... unsubscribe all
147+
w.Dispose();
148+
}
149+
_store.Clear();
150+
underlying.Dispose();
151+
}
152+
```
153+
154+
`PetroglyphFileSystem.SwapStrategy` already disposes the previous strategy, so a consumer calling `UseLiveVirtualStrategy` then later `UseWindowsStrategy` (or recreating the file system) cleanly tears down the watcher.
155+
156+
## Testing
157+
158+
Unit tests live alongside the existing `VirtualFileExistsStrategyTests` in `PG.StarWarsGame.Engine.FileSystem.Test`.
159+
160+
- Use the existing `MockFileSystem` for the snapshot logic where possible.
161+
- For watcher behavior, the watcher is wired against a real OS path via the test's temp directory infrastructure (`MockFileSystem` does not raise FSW events). One integration-style test class, kept small.
162+
163+
Required scenarios:
164+
165+
- A file created on disk under a previously-cached directory is reported as existing on the next `FileExists` call (after a brief polling wait for the FSW event).
166+
- A file deleted on disk under a previously-cached directory is reported as missing on the next `FileExists` call.
167+
- A renamed file is reported under its new name and not its old name on the next call.
168+
- A renamed directory invalidates its own snapshot and any cached descendants.
169+
- A deleted directory invalidates its own snapshot and any cached descendants.
170+
- After `Dispose`, no further file-system events touch the store (verify by sleeping past a created file and asserting no exception / no store mutation — use a wrapping store you can spy on).
171+
- Lookups outside the game directory continue to delegate to the underlying strategy unchanged.
172+
- `UseLiveVirtualStrategy(windowsFallback: true)` throws `PlatformNotSupportedException` on non-Windows hosts (parity with `UseVirtualStrategy`).
173+
- `UseLiveVirtualStrategy()` (default `null`) selects the Windows fallback on Windows and the Wine fallback elsewhere.
174+
- `UseLiveVirtualStrategy(windowsFallback: false)` selects the Wine fallback on every host, including Windows.
175+
176+
## Risks
177+
178+
- **FSW event timing in tests.** FSW events are asynchronous; tests must poll with a timeout rather than asserting immediately. Standard pattern.
179+
- **Linux inotify watch limits.** A consumer holding many `LiveVirtualFileExistsStrategy` instances on Linux could exhaust inotify watches. Mitigation: this is an opt-in API; consumers spinning up many engine hosts should size their inotify limits accordingly. Documented in the XML doc.
180+
- **Path normalization at the event boundary.** `FileSystemWatcher` reports paths in OS form. The store keys are also OS-form (built from `Path.GetDirectoryName` and `Path.Combine` on the underlying file system). They must compare under the same normalization rules the rest of the strategy uses (`OrdinalIgnoreCase`). The existing dictionary already uses `StringComparer.OrdinalIgnoreCase`; the watcher handler must use `Path.GetDirectoryName` from the same `IFileSystem` to keep separator handling consistent.

0 commit comments

Comments
 (0)