🐛 PUT-1589: Shared file fs events seem to be lost - #3645
Merged
jfcastro92 merged 6 commits intoAug 26, 2026
Conversation
jfcastro92
changed the title
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
A recipient's client keeps its cache fresh from fs events pushed over their socket, and ShareService fans those out to holders — but only for write, move and delete. A new entry emits fs.create.<flavor>, not fs.write.file, and an in-place rename emits fs.rename; neither had a listener, so a recipient watching a shared folder never learned that a file appeared in it or was renamed. Part of why: those keys and outer.gui.item.renamed were missing from the typed event map, so a listener for them did not compile. Delivering the event is only half of it. Paths were masked against the entry itself, so item.added named a parent no cached listing was keyed on, and the payload carried no dirpath, which is how the desktop finds the container to render into — the event would have arrived and changed nothing. Paths are now masked at the share the holder reached the entry through, which is the address their own reads returned, and from_path on a move and old_path on a rename travel the same way (dropped when the move started outside the share, self-masked when the share is on the entry itself, where the root already carries the new path). Creates fire per entry, so an upload would have cost one share lookup per file; they are coalesced by parent folder the way subtree deletes already are. Measured on a 25-file burst into one folder: 25 lookups before, 1 after. A holder with a share on both a folder and something inside it is told once, by the nearer of the two.
jfcastro92
force-pushed
the
juancastro/put-1589-shared-file-fs-events-seem-to-be-lost
branch
from
August 25, 2026 23:38
8850a8b to
ee2b5ad
Compare
jfcastro92
marked this pull request as ready for review
August 25, 2026 23:44
Salazareo
approved these changes
Aug 26, 2026
Raised in review: could an event from another node re-trigger the fan-out? Not today — broadcast carries outer.* and pubsub.* only, so fs.* never crosses a node boundary, and the emitted outer.gui.* is consumed on the peer by SocketService while ShareService listens to fs.* alone, so nothing re-enters. That safety is a property of what broadcast happens to replicate, which is not this service's to rely on. The handlers now skip anything tagged from_outside: the node that did the write has already told the audience, and a second fan-out would only duplicate it.
Both ways a file can vanish were silent for anyone holding the folder above it, so a third party's window kept showing a file that was gone and 404'd on click. A delete built its audience from the permission rows it removed, and a file inside a shared folder has no grant of its own — only the folder does — so the audience was empty. A move resolved its audience from the entry's new path, and the GUI's Delete is a move to the owner's Trash, where no recipient has a share. Resolve the audience from where the entry was rather than from the grants that went with it: - Deletes also fan out to holders reaching the entry through an ancestor, coalesced by parent so a subtree stays a couple of queries. A holder covered by both passes is told once. - Moves resolve both ends. Reaching both is item.moved, only the destination item.added, only the origin item.removed. Recipients are named by the path they knew, masked through their own share rather than the owner's tree.
A grant on the entry itself is keyed on uuid, so it follows the entry into the owner's Trash. Both ends of the move then resolved, and the recipient was told the shared item had moved — to the GUID name Trash gave it. Their own copy got renamed to a GUID and stayed on screen. `shared-with-me` has always omitted trashed entries, so the listing and the event disagreed; only the event was wrong. Trashing now reports item.removed at the path the recipient knew, which is also what the desktop's data-path selector needs to find the row. Restoring out of Trash reports item.added. A move that leaves the recipient's masked address unchanged now stays quiet — a share masks its own root, so the owner shuffling it around their tree is invisible to the recipient and the event carried nothing.
The realtime fan-out resolved its audience straight from the `share` index, which has no live-grant check. `/auth/revoke-user-user` deletes the permission and leaves the index row, so a revoked recipient's socket kept receiving name, size, masked path and mtime for every write and move under the folder, with no expiry. The service already solves this elsewhere — `#reachingHolders` returns exactly the holder/entry pairs whose grant still stands, and `listSharedWithMe` was moved onto it for the same reason. The realtime path never got the same treatment; it does now. Free on the unshared path: with no share rows reaching the entry there are no holders to check, so the write path every user takes is unchanged. Pinned by a test that counts permission reads.
Salazareo
approved these changes
Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
A share recipient's client keeps its cache fresh from fs events pushed over their socket.
ShareServicealready fans those out to holders — but only for three of them:fs.write.file,fs.move.node,fs.remove.node.A new entry does not emit
fs.write.file. It emitsfs.create.file/.directory/.shortcut/.symlink—fs.write.fileis the overwrite case. An in-place rename emitsfs.rename. Neither had a listener, so a recipient watching a shared folder never learned that a file appeared in it or was renamed, while deletes, moves and overwrites worked. That asymmetry is why this reads as "events seem to be lost" rather than "sharing doesn't update".Part of why it was never wired:
fs.create.*,fs.renameandouter.gui.item.renamedwere absent from the typed event map, sotscrejects a listener for them.The disappearance half of the bug
Delivering the events that were wired turned out not to be enough either. Both mechanisms that make a file vanish were silent for anyone holding the folder above it:
#flushRetire). A file inside a shared folder has no grant of its own — only the folder does — so the audience was empty. Deleting the folder, or a directly shared file, worked, which is what made this look like an unrelated quirk.item.movedcannot express a disappearance anyway.Both now resolve the audience from where the entry was, not from the grants that went with it:
item.removedout to holders reaching the entry through an ancestor, on top of the roots#flushRetirealready reports. A holder covered by both is told once.item.moved, only the destination isitem.added, only the origin isitem.removed. Each is masked through the share that holder saw that end through, which need not be the same share at both ends.Recipients are named by the path they knew —
/owner/<sharedRootUuid>/Docs/file.txt— never the owner's real path or a Trash path they cannot see. That is not cosmetic: the desktop'sitem.removedhandler finds the row bydata-path, so a Trash path matches nothing and leaves the item on screen.The top-level share was worse
For a share on the entry itself, the grant is keyed on uuid, so it follows the entry into Trash. Both ends of the move resolved, and the owner's Delete reached the recipient as
item.moved— renaming their shared item to the GUID Trash had given it.shared-with-mehas always omitted trashed entries, so the listing and the event disagreed; only the event was wrong. Trashing a top-level share is nowitem.removed, and restoring one isitem.added.The mirror case: a move that leaves the recipient's masked address unchanged is now silent. A share masks its own root, so the owner shuffling it around their own tree is invisible to the recipient, and the event carried a
from_pathidentical topath.Revoked recipients kept receiving pushes (PUT-1611)
Rolled in here because it lives in the same function this PR rewrites, and this PR widens the fan-out — deletes and moves now reach holders they previously missed, which makes the leak worse.
The audience came straight from the
shareindex with no live-grant check./auth/revoke-user-userdeletes the permission and leaves the index row, so a revoked recipient's socket kept receiving name, size, masked path and mtime for every write and move under that folder, indefinitely.The service already solves this elsewhere —
#reachingHoldersreturns exactly the holder/entry pairs whose grant still stands, andlistSharedWithMewas moved onto it during PUT-1412 for this reason. The realtime path never got the same treatment; it does now.Cost: none on the unshared path. With no share rows reaching the entry there are no holders to check, so the write path every user takes is unchanged — pinned by a test that counts permission reads and asserts zero.
The path half of the bug
The payload masked paths against the entry itself (
/owner/<file-uuid>/<name>), and:item.addedwith a targeted eviction —puter._cache.del('readdir:' + dirname(item.path))(FileSystem/index.js:149). Against a self-masked path the parent is/owner/<file-uuid>, which is not a key anything cached.dirpath(apply_item_added_to_containers.js:40) — a field the payload didn't carry at all, so the helper returned early.Both would have produced an event that arrives and changes nothing. Paths are now masked at the share the holder reached the entry through, and the payload carries
dirpath, plus thetypeandimmutablethe item renderer reads.Behavior changes worth calling out
item.moved. They had never seen the item, so there was nothing for the GUI to move, andfrom_pathwas dropped as outside their share — the event arrived and did nothing. It is nowitem.added.item.movedwithfrom_path === path.item.removedrather thanitem.moved.Blast radius
Creates.
fs.create.*fires on everytouch,mkdir,mkshortcut, completed upload and batch-write item, for every user, shared or not. Measured: +1 indexedsharequery per create (25 creates → 25 extra reads out of 102 total; ancestor lookups are served from the entry cache).Creates are therefore coalesced by parent folder, the same
setImmediatebuffering the delete path has used for subtree deletes. Siblings share every ancestor, so one lookup serves the whole burst. Measured on 25 concurrent creates into one folder: 25 lookups before, 1 after.Deletes. The ancestor pass is new work on every delete. Coalesced by parent the same way and reusing the existing retire batch, so a subtree costs one lookup per folder rather than one per entry — pinned by a test: 8 concurrent deletes from one folder settle in ≤2 lookups, not 8.
Moves. A move now resolves the audience twice instead of once — +1 indexed
sharequery per move where the path actually changed. A rename in place is unaffected; it goes throughfs.rename, which still resolves once.Live-grant check. Two batched, holder-keyed reads per distinct holder of a reaching share. Zero when nothing is shared, which is the overwhelming majority of writes.
Also in here
from_pathon a move andold_pathon a rename are masked the same way, and dropped when the move started outside the share. A directly shared entry masks its former path by uuid, which still resolves after the entry has moved.Verification
Twenty-two cases in
keeping recipients in sync, driving the realfs.touch/fs.rename/fs.removepaths rather than hand-emitted events, so they cover whichever event those paths actually emit. Every behavioral fix has a test that fails without it — confirmed by reverting the change and re-running each time. They pin the audience, the exact masked path,dirpath,from_path/old_path, the one-lookup-per-folder property on both creates and deletes, that a move inside a share is still a move, that a holder of both a file and its folder is told once, that a trashed top-level share is named as the recipient knew it, that a revoked recipient hears nothing, and that the unshared write path does zero permission reads.Full backend suite: 6428 passing. Typecheck and lint clean.
Against a running local Puter
Also run end to end against
npm start, since socket delivery to a second user was the one thing the unit tests can't reach.Event matrix, two real socket.io clients — one socket per user, owner mutates through the API, the recipient's socket recorded:
item.addeditem.addeditem.added, dirpath = the subfolderitem.renamed+old_path/fs/write)item.updateditem.moved+from_pathitem.addeditem.removeditem.removeditem.removedEvery delivered path is masked at the share and never carries the owner's real path.
Three users on one shared folder — A owns
/A/shared-x, B holds itwrite, C holds itread, all three on live sockets. B (not the owner) deletesdoomed.txtinside it. Run on this branch and on the same build with onlyShareServicereverted:item.removeditem.removeditem.removeditem.removeditem.moved→ their Trashitem.moved→ their Trashitem.removeditem.removedThe owner keeps getting
item.movedfor the trash case, which is correct — the file really did move, into a Trash they can still open. Everyone else getsitem.removed, named at the masked path under the folder they hold, not the owner's Trash path. That is the case where a third party's window used to keep showing a file that was gone and 404 on click.The shared root itself — a folder and a file each shared directly with a recipient, owner then acts on that very item. Run on this branch and on the same build with only
ShareServicereverted:item.renameditem.renameditem.moved,from_path==pathitem.moved→/owner/<uuid>/79fc5f26-2110-…item.removed→/owner/<uuid>/top-yoe6p0item.removeditem.removeditem.renameditem.renameditem.moved,from_path==pathitem.moved→…/5998f0ad-….txtitem.removed→…/topfile-2qnyqa.txtitem.removeditem.removedThe two bolded rows are the reported bug: the recipient's shared item was renamed to a Trash GUID instead of disappearing.
Real browser (Playwright) — the recipient signs into the desktop and lists the shared folder, the owner then adds a file from the API, nothing is clicked in the browser:
main(pre-fix)item.addedreaches the page's socketreaddir(dir, { consistency: 'eventual' })afterThat is the chain the ticket describes — socket →
puter.fscache invalidation — reproduced broken onmainand working here.Caveat worth recording:
item.addedevicts one specific cache key, so it lands only when the recipient's cached key is the masked path — i.e. when they navigated the normal way. A first run that read the owner's real path missed the eviction. Not an issue for the GUI; an API consumer holding a real path won't be invalidated byitem.added. The other events flush wholesale and are immune.Known, not fixed
A recipient with write access is notified of their own create, and the actor of a delete is now told about their own delete. The fs events carry no acting socket, so the client's "don't update if this is the original client" guard can't fire; removing an item already gone is a no-op, so this is cosmetic. Pre-existing for updates, moves and deletes. Fixing it means threading the actor through the fs event payloads, which is a bigger change than this bug warrants.
A non-owner cannot delete a file shared directly with them at all — delete authority comes from the parent folder, which is the owner's, so it 403s. Unchanged here, and worth a separate look if that is not the intended permission model.