Systems 6: the Project tree's folders and external edits register with the one watcher - #117
Conversation
…ileWatch FileWatch gains watchEntries, told only of entries created or removed, and pausing, which lets go of every watch on or inside some folders while Companion moves them. FileUtils, which was only its own watcher, goes; ExternalEdits registers each file with FileWatch and settles on the timer and a strand. SYSTEMS.md records the pause and that the logs, configuration and resource packs pages keep reading whenever they are shown. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b24d03c06d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (file.pending != null) file.pending.cancel(false); | ||
| file.pending = this.settle.schedule(() -> take(file), SETTLE_MILLIS, TimeUnit.MILLISECONDS); | ||
| file.pending = Workers.later(SETTLE_MILLIS, this.strand, () -> take(file)); |
There was a problem hiding this comment.
When the 300 ms timer has already handed this task to strand, pending.cancel(false) cannot remove it from the strand queue. If another write event arrives before that queued task runs—for example while the shared owner pool is busy—the stale task reads the newer write immediately instead of allowing it to settle for 300 ms. For non-PNG resources, readable accepts partial content, so an editor writing in stages can cause an intermediate file to be recorded and reloaded before the replacement task runs. Carry a generation into take, as FileReading does, so superseded handoffs exit before reading.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| void aFolderReachedThroughALinkIsPausedAndWatchedAgain() throws Exception { | ||
| Path real = Files.createDirectory(this.directory.resolve("real")); | ||
| Path alias = this.directory.resolve("alias"); | ||
| Process link = new ProcessBuilder("cmd", "/c", "mklink", "/J", alias.toString(), real.toString()).redirectErrorStream(true).start(); |
There was a problem hiding this comment.
On Linux and macOS, this test fails immediately because cmd is unavailable, so running :companion:test locally cannot complete. Other junction-dependent tests in this repository, such as ScriptFilesTest, use @EnabledOnOs(OS.WINDOWS); apply the same guard here or create a platform-appropriate symbolic link.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 330db57a86
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (Watched followed : this.watched) { | ||
| if (followed.key != null) continue; | ||
| if (followed.retry != null) followed.retry.cancel(false); | ||
| place(followed, 0); | ||
| tell.add(followed.changed); |
There was a problem hiding this comment.
When pausing calls are nested, the inner finally processes every follower with a null key, including followers still covered by the outer pause. It invokes their changed callbacks before the outer operation has finished, so a FileReading or ExternalEdits registration can read/adopt intermediate state if the remaining operation lasts beyond its settle delay. Track which followers this pause released, and restore or notify them only after their last covering pause ends.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5d2534b65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| synchronized (this) { | ||
| // Another write came since, whose own settle takes the file. | ||
| if (this.closed || write != file.writes) return; |
There was a problem hiding this comment.
If a later write event or project close occurs after this check but while Files.readAllBytes is in progress, as can happen with a large or slow resource, the task still calls adopt with content its generation no longer owns; non-PNG partial bytes can consequently be recorded and reloaded, and an in-flight take can survive close(). The fresh race remains after the queued-task fix because validation occurs only before the blocking read; recheck closed and write == file.writes after reading, as FileReading.readHere does before publishing.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| for (Watched followed : this.watched) { | ||
| if (followed.key != null) continue; | ||
| if (followed.pausedAt != null && paused(followed.pausedAt)) continue; | ||
| if (followed.retry != null) followed.retry.cancel(false); | ||
| place(followed, 0); | ||
| if (followed.pausedAt == null) tell.add(followed.changed); |
There was a problem hiding this comment.
When any unrelated follower currently has no key because its folder is unwatchable, its pausedAt is also null, so every script-file pausing operation enters this branch, cancels that follower's scheduled retry, retries it immediately, and invokes its callback even though its folder is outside roots. The fresh case left after the nested-pause fix is specifically a null key caused by an unavailable watcher or unwatchable/network folder rather than by another pause; retain the set of followers actually released by this operation and restore only those.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
|
|
||
| this.stopWatching = watch | ||
| ? FileUtils.startNewDirectoryWatcher(path, () -> { | ||
| ? FileWatch.shared().watchEntries(path, () -> { |
There was a problem hiding this comment.
When a displayed directory is a symlink or Windows junction and its target is deleted and later recreated, this migration leaves the tree stale. After the target's watch key becomes invalid, FileWatch.place tests the logical link path, finds it is no longer a directory, and watches the link's parent; recreating the target changes only the target's parent, so no event reattaches the watch or clears the tree's error state. The former FileUtils registration resolved and retried the real target path, while createChildIfPresent deliberately admits linked directories, so FileWatch needs to preserve recovery through the real target before this consumer moves to it.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
1 participant
Sixth PR of
docs/SYSTEMS.md, stacked on #116: the last two watchers of their own,FileUtils' andExternalEdits', become registrations with the oneFileWatch.Finish line
FileWatchTest.aWatchedFolderAndOneInsideItCanBeMovedWhileTheWatchIsPausedFileWatchTest.aFollowerAddedWhileAPauseLastsWatchesNothingThereUntilItEndsFileWatchTest.aFailedOperationWatchesAgainAndTellsWhatItChangedFileWatchTest.otherFoldersAreToldWhileAPauseLastsFileWatchTest.aFolderReachedThroughALinkIsPausedAndWatchedAgainFileWatchTest.aListingIsToldOfEntriesCreatedAndRemovedButNotOfWritesFileWatchTest.aListingOfAFolderDeletedAndMadeAgainIsToldAndWatchedAgainExternalEditsTest.aClosedProjectTakesNoSaveAndHoldsNoFolderOfThePackExternalEditsTest.aFollowedTextureIsTakenOnceItIsWholeSystemsRulesTest(the exceptions ofFileUtilsandExternalEditsare gone)The pause, entries-only and closing tests each fail without their change. The nested-pause test also holds the outer pause past the retry delay and proves no early callback. The junction test runs only on Windows.
What changes
FileWatchgainswatchEntriesfor a folder's listing, told only of entries created or removed, andpausing(roots, operation). The pause lets go of every watch on or inside the roots, runs the operation, watches them again and tells their followers, also where the operation failed. On Windows a folder cannot be moved while a folder inside it is watched.watchEntries; moving script folders runs inFileWatch.pausing.FileUtils, which was only its own watcher thread, pause and registrations, goes, and with itDirectoryWatcherTest. Its tests of its two-lock registration have no counterpart, sinceFileWatchregisters under one lock; its other behaviors are inFileWatchTest.ExternalEditsregisters each followed file withFileWatch; its settle runs on the timer and takes saves on a strand, and closing lets go of every watch. Each take carries the write it settled, so a later write supersedes a task already handed to the strand. Opening in another program runs on the file workers. The results of its saves stay a listener per file, an event like script runs.docs/SYSTEMS.mdrecords the pause and entries-only listings, and a decision: the Logs page, the configuration pages and the resource packs listing keep reading whenever they are shown (readsWhenShown) instead of getting readings. They are their files' only readers, and the game writes its logs the whole time it runs, so a reading would only add a watch. The step that planned those readings goes; the pages PR moves the three pages.Left as it is
A watch placed while its folder is paused waits for the pause to end or its retry after a second, whichever comes first.
A Project tree folder's watch is let go on the Swing thread under
FileWatch's lock, which a native registration also holds; a registration that stalls, as on a hung network drive, would hold the Swing thread that long. The game folder is local.Focused
:companion:testchecks pass locally. CI runs the full build for the updated head.🤖 Generated with Claude Code