lee/puter
1
0
Fork 0
mirror of https://github.com/HeyPuter/puter.git synced 2026-08-28 17:07:16 +00:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity

fix: trashing a top-level share is a removal, not a move

Browse Source 
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.
This commit is contained in:
Juan Castro
2026-08-26 16:25:02 -04:00
parent 8e622189ca
commit cae51ce67a
2 changed files with 125 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/backend/services/share/ShareService.test.ts
+94 -10
View File
@@ -1530,8 +1530,7 @@ describe('ShareService', () => {
mode: 'read',
});
// Nothing was shared at the file, so the revoke reports no holder
// for it — the audience has to be found through the folder.
// No share on the file, so the audience is only found upward.
const payload = await capturePayload(
'outer.gui.item.removed',
recipient.user.id,
@@ -1561,8 +1560,7 @@ describe('ShareService', () => {
});
}
// The revoke reports them for the file, and the folder reaches it
// too — one delete must not arrive as two removals.
// Both passes reach them; one delete must not arrive as two.
const audiences = await captureAudiences(
'outer.gui.item.removed',
file.uuid,
@@ -1592,8 +1590,7 @@ describe('ShareService', () => {
mode: 'read',
});
// What the GUI's Delete does. `item.moved` would name a path the
// recipient cannot see, leaving the file on their screen.
// What Delete does; `item.moved` would name a path they can't see.
const audiences = await captureAudiences(
'outer.gui.item.removed',
file.uuid,
@@ -1715,6 +1712,86 @@ describe('ShareService', () => {
expect(payload?.path).toBe(`${masked}/renamed-${file.name}`);
});
it('tells a recipient when the shared item itself is trashed', async () => {
const owner = await makeUser();
const recipient = await makeUser();
const file = await makeFile(owner.user);
const before = file.path;
const trashName = uuidv4();
const trashed = `/${owner.user.username}/Trash/${trashName}`;
await share(owner.actor, {
uid: file.uuid,
recipient: { email: recipient.email },
mode: 'read',
});
// The grant follows it into Trash, so both ends would resolve.
const payload = await capturePayload(
'outer.gui.item.removed',
recipient.user.id,
async () => {
await server.clients.event.emitAndWait(
'fs.move.node',
{
node: {
...file,
path: trashed,
name: trashName,
},
fromPath: before,
toPath: trashed,
},
{},
);
},
);
// Named as they knew it, never the GUID that Trash gave it.
expect(payload?.path).toBe(
`/${owner.user.username}/${file.uuid}/${file.name}`,
);
});
it('stays quiet when a move leaves the recipient address alone', async () => {
const owner = await makeUser();
const recipient = await makeUser();
const file = await makeFile(owner.user);
const elsewhere = `/${owner.user.username}/Documents/${file.name}`;
await share(owner.actor, {
uid: file.uuid,
recipient: { email: recipient.email },
mode: 'read',
});
const seen: unknown[] = [];
const listener = (_key: string, data: unknown) => {
const payload = data as { user_id_list?: number[] };
if (!payload.user_id_list?.includes(recipient.user.id)) return;
seen.push(payload);
};
server.clients.event.on('outer.gui.item.moved', listener);
try {
// Masked at its own root, so their path holds wherever it goes.
await server.clients.event.emitAndWait(
'fs.move.node',
{
node: { ...file, path: elsewhere },
fromPath: file.path,
toPath: elsewhere,
},
{},
);
await new Promise((resolve) => setTimeout(resolve, 100));
} finally {
server.clients.event.off('outer.gui.item.moved', listener);
}
expect(seen).toEqual([]);
});
it('tells a folder recipient when a file inside it changes', async () => {
const owner = await makeUser();
const recipient = await makeUser();
@@ -1953,8 +2030,7 @@ describe('ShareService', () => {
reaching.mockRestore();
}
// Siblings share a parent, so they share the audience lookup —
// a burst settles in one or two flushes, never one per file.
// Siblings share a parent, so a burst settles in a flush or two.
expect(seen).toHaveLength(8);
expect(lookups).toBeLessThanOrEqual(2);
});
@@ -2015,7 +2091,11 @@ describe('ShareService', () => {
await server.clients.event.emitAndWait(
'fs.move.node',
{
node: { ...file, path: `${before}-moved` },
node: {
...file,
path: `${before}-moved`,
name: `${file.name}-moved`,
},
fromPath: before,
toPath: `${before}-moved`,
},
@@ -2112,7 +2192,11 @@ describe('ShareService', () => {
await server.clients.event.emitAndWait(
'fs.move.node',
{
node: file,
node: {
...file,
path: `${file.path}-moved`,
name: `${file.name}-moved`,
},
fromPath: file.path,
toPath: `${file.path}-moved`,
},
src/backend/services/share/ShareService.ts
+31 -16
View File
@@ -244,6 +244,10 @@ type HolderGuiEvent =
| 'outer.gui.item.updated';
/** `/<owner>/<uuid>/<name>` for a path in the owner's tree. */
/** Inside some owner's top-level Trash, which is where Delete puts things. */
const isTrashedPath = (path: string): boolean =>
/^\/[^/]+\/Trash(\/|$)/u.test(path);
const maskedSelfPath = (entry: FSEntry, realPath: string): string => {
const owner = realPath.split('/')[1];
const name = realPath.split('/').pop();
@@ -527,10 +531,9 @@ export class ShareService extends PuterService {
}
/**
* Tell whoever reached these through a folder above them. Their grant is on
* that folder, not on what was deleted, so the revoke reports no holder for
* them — and the file stays in their window, 404ing when they open it.
* Coalesced by parent like creates, so a subtree stays a few queries.
* Tell whoever reached these through a folder above them — their grant is
* on that folder, so the revoke reports no holder for them. Coalesced by
* parent like creates, so a subtree stays a few queries.
*/
async #fanOutRetiredToAncestors(
entries: FSEntry[],
@@ -545,8 +548,7 @@ export class ShareService extends PuterService {
for (const siblings of byParent.values()) {
const first = siblings[0];
if (!first) continue;
// Ancestors only — a share on the entry itself is the revoke's to
// report, and it already has.
// Ancestors only; a share on the entry is the revoke's to report.
const groups = (await this.#reachingRoots(first)).filter(
({ root }) => root.uuid !== first.uuid,
);
@@ -567,11 +569,9 @@ export class ShareService extends PuterService {
}
/**
* A move seen from both ends, because it can take an item out of someone's
* share as easily as into it. Reaching both ends is a move, only the
* destination an arrival, only the origin a removal — that last being the
* GUI's Delete, a move to the owner's Trash where no recipient has a
* share.
* A move seen from both ends, since it can take an item out of a share as
* easily as into it: both ends is a move, only the destination an arrival,
* only the origin a removal.
*/
async #fanOutMove(entry: FSEntry, fromPath?: string): Promise<void> {
const destination = await this.#reachingRoots(entry);
@@ -591,8 +591,7 @@ export class ShareService extends PuterService {
const originRootOf = rootsBySide(origin);
const destinationRootOf = rootsBySide(destination);
// Keyed on both shares: the paths a holder is told depend on the one
// they saw each end through, and those need not be the same share.
// Keyed on both: a holder can see each end through a different share.
const batches = new Map<
string,
{
@@ -614,16 +613,29 @@ export class ShareService extends PuterService {
batches.set(key, batch);
};
// A grant follows its entry into Trash, so Delete would read as a move
// and rename their item to the GUID Trash gave it. Listings omit it.
const nowTrashed = isTrashedPath(entry.path);
const wasTrashed = fromPath ? isTrashedPath(fromPath) : false;
for (const [holder, to] of destinationRootOf) {
if (nowTrashed) continue;
const from = originRootOf.get(holder);
place(
holder,
from ? 'outer.gui.item.moved' : 'outer.gui.item.added',
from && !wasTrashed
? 'outer.gui.item.moved'
: 'outer.gui.item.added',
from,
to,
);
}
for (const [holder, from] of originRootOf) {
if (nowTrashed) {
// Already gone from their view if it was trashed before.
if (!wasTrashed) place(holder, 'outer.gui.item.removed', from);
continue;
}
if (destinationRootOf.has(holder)) continue;
place(holder, 'outer.gui.item.removed', from);
}
@@ -638,12 +650,15 @@ export class ShareService extends PuterService {
);
continue;
}
const payload = holderPayload(entry, to as FSEntry);
const formerPath =
from && fromPath
? maskedFormerPath(from, entry, fromPath)
: null;
// A share masks its own root, so this move is invisible to them.
if (formerPath && formerPath === payload.path) continue;
await this.#emitGui(event, holders, {
...holderPayload(entry, to as FSEntry),
...payload,
// The GUI rewrites the item it already has by this.
...(formerPath ? { from_path: formerPath } : {}),
});
@@ -2029,7 +2044,7 @@ export class ShareService extends PuterService {
}
#isTrashed(entry: FSEntry): boolean {
return /^\/[^/]+\/Trash(\/|$)/u.test(entry.path);
return isTrashedPath(entry.path);
}
/**