Skip to content

Make watched-file notifications a no-op - #240

Merged
dannypsnl merged 10 commits into
jeapostrophe:masterfrom
lamestllama:fix/folder-publication-lifecycle
Sep 30, 2026
Merged

dannypsnl merged 10 commits into
jeapostrophe:masterfrom
lamestllama:fix/folder-publication-lifecycle

Conversation

@lamestllama

@lamestllama lamestllama commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Make workspace/didChangeWatchedFiles a no-op so disk notifications cannot close or replace editor-owned documents. This follows the scope requested in review; the broader analysis, cache and rename changes have been removed from this PR.

Keep disk notifications separate from client document ownership. Invalidate
closed-file contributions immediately and defer invalidation for open files
until the last matching buffer closes.

Guard analysis publication by document lifetime and version, and keep it
outside the computation timeout so cancellation cannot strand document locks.
Retain cancellation-before-close ordering and match paths before locking.

Cover document ownership, cached contributions, URI aliases, malformed
notifications, retired results and pending-query concurrency with regressions.
Publish analysis before optional disk verification. Reuse the existing
cancellable scheduler to compare the accepted source text with its file,
limiting comparison input and allocation to the analyzed text. Only file
URIs can establish provenance, and positive results must still match the
document lifetime, version and accepted contribution.

Reuse close-time invalidation while provenance is unverified or a watched
disk change has been observed. Preserve prior provenance on failed analysis
and retain verified contributions after close. Closing before verification
finishes can conservatively discard the cache.

Cover unsaved edits, saved analysis, failed expansion, aliases, reopened
lifetimes, bounded reads, Unicode, URI schemes, stalled disk access, query
completion and cancellation. Alias contribution ownership remains separate.
Reuse the open-document registry, decoded-path matching and existing
close-time invalidation. When another URI for the path remains open,
replace the closed document's cached facts with surviving accepted
contributions, reading and publishing under each document's lock.

Remove cached facts when no survivor has accepted analysis. Keep verified
final-close retention and watched-event behavior unchanged. No additional
analysis, ownership state or scheduler changes are needed.

Cover publication and closing orders, failed or absent analysis, clean
retention, reopening, three aliases and concurrent publication. Check
actual reference locations and preserve the current contribution on disk
events while aliases remain open.
Invalidate cached facts for both the source and destination paths using
the same file-URI validation and invalidation as watched-file events.
Decode the complete batch before applying changes.

Leave open buffers, versions and pending requests under the client's
textDocument lifecycle. Its didOpen notification already schedules
analysis at the new URI; synthesizing close/open here can overwrite an
open destination and create a replacement with no analysis.

Cover closed destination caches, client notification orderings, encoded
aliases, pending requests, malformed batches and non-file URIs. Preserve
accepted contributions while buffers remain open and retire them through
the existing close-time rules.
Hold the document read lock through folder republication so an older
snapshot cannot overwrite a newer accepted contribution. Restore alias
contributions on close without treating the close as a disk event.

Verify each new accepted analysis against disk, allowing cache retention
to recover after a reported disk change while preserving invalidation for
later events and failed or disk-mismatching analysis.

Add controlled publication and cache-retention regressions, and describe
closed-file reference retention.
The lifecycle fixtures use first at line 2, but selecting the first key
of their reference hash can return require instead. Hash traversal order
varies across runtimes, causing the tests to inspect unrelated locations.

Use the existing document binding query at the fixture's source position.
Keep the reference-count, location, cache-retention, and concurrency
assertions unchanged.
Complete current analysis after errors and timeouts while preserving the
last accepted contribution. Guard completion with the running record so
obsolete same-version runs cannot publish or drain newer requests.

Recheck readiness when registering pending queries and keep failure
completion cancellable. Cover actual expansion failures, publication
errors, timeout completion, cancellation, and replacement races.
@lamestllama lamestllama changed the title Preserve editor-owned documents and accepted workspace contributions Preserve editor-owned documents and complete failed analysis Sep 28, 2026
Hold the fixture at a semaphore during macro expansion until both pending
requests are registered. Keep the real expansion and publication failures,
completion assertions, and existing response deadlines.

Suspending the scheduler while it waits on an async channel can leave
queued jobs dormant after resumption on Racket 8.17 BC. The fixture no
longer suspends the scheduler or depends on its private thread binding.
@6cdh

6cdh commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Thanks for addressing this. But this goes beyond what I intended in #239 . My idea was to let didChangeWatchedFiles to become no-op for now.

Current implementation is over complex. It covers

  • Preserving editor buffers during watched-file and rename notifications.
  • Fixing analysis failure/timeout completion and stale publication.
  • Introducing disk verification, conditional closed-file cache retention, and URI-alias restoration.

and changes multiple modules who actually do not need to know about the disk files concepts, like SafeDoc.

@lamestllama lamestllama changed the title Preserve editor-owned documents and complete failed analysis Make watched-file notifications a no-op Sep 30, 2026
@6cdh

6cdh commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Thanks!

@6cdh

6cdh commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Let's wait for @dannypsnl review then merge.

@dannypsnl
dannypsnl merged commit 13777e5 into jeapostrophe:master Sep 30, 2026
14 checks passed
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