Skip to content

Reload .autocp when modified externally#170

Merged
Pushpavel merged 3 commits into
Pushpavel:mainfrom
znzryb:watch-autocp-external-edits
Jul 11, 2026
Merged

Reload .autocp when modified externally#170
Pushpavel merged 3 commits into
Pushpavel:mainfrom
znzryb:watch-autocp-external-edits

Conversation

@znzryb

@znzryb znzryb commented May 13, 2026

Copy link
Copy Markdown
Contributor

Motivation

.autocp is loaded once via by lazy and only ever written to, so any external edit (a script, another editor, an AI tool) gets silently overwritten by AutoCpStorageSaver on the next saveAllDocuments using stale in-memory state.

Change

Register a BulkFileListener (application level) that calls a new AutoCpStorage.reloadFromDisk() whenever a file named .autocp under any open project's basePath reports a content change or creation. It re-parses through the existing Migrations path and resets the two MutableStateFlows. Events originating from the plugin's own document saves are skipped via isFromSave, and the reload itself is deferred with invokeLater so no disk I/O runs inside VFS event delivery.

The saver also short-circuits with if (document.text == newText) return before setText, which prevents a write→fire→reload loop and avoids spurious modify events.

As a safety net for a save racing ahead of the VFS create event: if the in-memory state was never mutated (tracked by an explicit mutated flag on AutoCpDatabase, set by updateProblem/modifySolutionFiles and cleared when state is replaced from disk) and the on-disk file has content, the saver reloads instead of writing — so a default-initialized state can't clobber an externally created .autocp. A database the user intentionally emptied has mutated = true and persists normally.

External deletion of .autocp is intentionally ignored: in-memory state survives and the next save recreates the file.

The IDE-internal warning panel in AutoCpFileEditor is unchanged — direct editing inside the IDE remains blocked via HIDE_DEFAULT_EDITOR.

Failure mode

If the new disk content fails to parse (invalid JSON / schema mismatch), reloadFromDisk catches and log.warns, leaving in-memory state untouched. Once the in-memory state has been mutated, the next saveAllDocuments overwrites the broken file with the previous valid state — equivalent to the current "external edit silently lost" behaviour. If the plugin has nothing to persist (state never populated), it leaves the unparseable file alone rather than clobbering it.

znzryb added 2 commits May 13, 2026 17:55
Previously .autocp was loaded once via `by lazy` and only written to,
so any external edit (e.g. by an AI tool) would be silently overwritten
by AutoCpStorageSaver on the next saveAllDocuments using stale in-memory
state.

Register a BulkFileListener that calls AutoCpStorage.reloadFromDisk()
when the file's content changes on disk, re-parsing through the existing
Migrations path and resetting the two MutableStateFlows. The saver also
short-circuits when document text already matches the encoded db,
preventing a write-fire-reload loop and avoiding spurious modify events.

The IDE-internal warning panel (AutoCpFileEditor) is unchanged --
direct edits inside the IDE editor remain blocked.
When an external tool (e.g. an AI script generating problem sets)
writes a fresh .autocp into a project directory that previously had
none, the user's data disappears within seconds: the file shows up
with only a stub SolutionFile entry, all problems gone.

Mechanism: AutoCpStorage's databaseDelegate is `by lazy`, so on first
access (tool window opening, a cpp file getting focus) it sees no
.autocp on disk and initializes to AutoCpDB() default. The external
write then arrives as VFileCreateEvent, but AutoCpExternalReloader
filtered only VFileContentChangeEvent, so reloadFromDisk() never ran
and in-memory stayed at the empty default. Any subsequent
beforeAllDocumentsSaving (triggered by focus changes / doc saves)
serialized that empty default and overwrote the disk content. If the
user clicked an unassociated .cpp in between, ToolFactory's
AssociateFilePanel injected a stub SolutionFile, which is what
ended up on disk in place of the original data.

Widen the listener filter to also accept VFileCreateEvent so the
created file is parsed into memory before any save fires. Add a
staleness guard in AutoCpStorageSaver: if in-memory equals
DEFAULT_AUTO_CP_DB but the disk document is non-blank, reload from
disk instead of overwriting it -- covers the theoretical race where
beforeAllDocumentsSaving lands ahead of the BulkFileListener
dispatch.
@codecov-commenter

codecov-commenter commented May 14, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 0% with 38 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../github/pushpavel/autocp/database/AutoCpStorage.kt 0.00% 19 Missing ⚠️
...ushpavel/autocp/database/AutoCpExternalReloader.kt 0.00% 16 Missing ⚠️
...github/pushpavel/autocp/database/AutoCpDatabase.kt 0.00% 3 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Files with missing lines Coverage Δ
...github/pushpavel/autocp/database/AutoCpDatabase.kt 0.00% <0.00%> (ø)
...ushpavel/autocp/database/AutoCpExternalReloader.kt 0.00% <0.00%> (ø)
.../github/pushpavel/autocp/database/AutoCpStorage.kt 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@znzryb

znzryb commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Hi @Pushpavel 👋 Friendly ping on this one — it fixes external edits to .autocp (from scripts or other tools) being silently overwritten by stale in-memory state on the next save, and a follow-up commit also covers the case where the file is externally created rather than modified. We'd appreciate a review whenever you get a chance 🙂

@Pushpavel

Pushpavel commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Thanks — this fixes a real problem (external edits to .autocp being silently clobbered), and the overall design is right: reuse the Migrations path, guard with isInitialized(), register the listener declaratively. A few issues, one of which I'd like fixed before merge.

1. Data resurrection via the DEFAULT_AUTO_CP_DB branch (blocking)

if (db == DEFAULT_AUTO_CP_DB && document.text.isNotBlank()) {
    project.service<AutoCpStorage>().reloadFromDisk()
    return@runReadAction
}

This triggers whenever the in-memory DB equals the default, not only when it was never populated. If a user deletes all problems/solution files, serializableDatabase == DEFAULT_AUTO_CP_DB, so every save reloads the old file instead of persisting the deletion — deleted problems come back. Since the create/change listener already handles "file appeared after init", I'd drop this branch entirely (or gate it on an explicit "state was mutated" flag rather than value equality).

Related: in this branch, a non-blank but unparseable file means reloadFromDisk warns and the saver returns without writing — forever. That contradicts the PR description's stated failure mode ("next save overwrites broken file").

2. Skip self-triggered events

Every plugin-initiated save that changes content fires a VFileContentChangeEvent, causing a full re-read + re-parse of .autocp in every open project. It doesn't loop only because StateFlow dedupes equal values (which quietly relies on structural equality of the models). Filter these out in the reloader:

.filter { it !is VFileContentChangeEvent || !it.isFromSave }

3. Don't do disk I/O inside VFS event delivery

BulkFileListener.after runs on the EDT inside the write action that applied the change. reloadFromDisk does synchronous readText() + JSON parsing there. Small file, so tolerable, but deferring with invokeLater { reloadFromDisk() } is the idiomatic fix and costs one line.

Nits

  • it.file?.name forces child resolution on VFileCreateEvent; it.path.endsWith("/.autocp") is cheaper.
  • project.basePath == parentPath is exact string comparison — use FileUtil.pathsEqual() so a case difference on Windows doesn't silently miss.
  • External deletion of .autocp is ignored (state survives, next save recreates the file) — fine, but worth noting as intentional in the description.

With #1 addressed I'm happy to merge; #2/#3 are cheap robustness wins worth doing in the same pass.


This is an AI review, but I think first point makes sense, can you check

…p save-originated events, defer reload out of VFS event delivery
@znzryb

znzryb commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Pushed fixes for all three in b05383a.

  1. The default-DB branch is now gated on an explicit mutated flag on AutoCpDatabase (set by updateProblem/modifySolutionFiles, cleared when state is replaced from disk) instead of value equality, so an intentionally emptied database persists normally. We kept the reload path for the never-mutated case — without it, a save racing ahead of the VFS create event would still clobber an externally created .autocp — and updated the description to match the actual failure mode.
  2. Save-originated events are skipped via isFromSave.
  3. reloadFromDisk() is deferred with invokeLater, guarded by project.disposed.

Also took the nits: path-string matching instead of file?.name, FileUtil.pathsEqual for the basePath comparison, and the description now notes that ignoring external deletion is intentional.

@Pushpavel
Pushpavel merged commit 63fd73e into Pushpavel:main Jul 11, 2026
4 checks passed
Pushpavel added a commit that referenced this pull request Jul 11, 2026
- bump platformVersion to 2026.1.4 (sinceBuild stays 243)
- align JUnit Jupiter/Platform versions with 2026.1 test framework
- add PR #170 to changelog

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants