lee/puter
1
0
Fork 0
mirror of https://github.com/HeyPuter/puter.git synced 2026-08-25 07:27:04 +00:00
Code Issues Actions 13 Packages Projects Releases Wiki Activity

fix: progress window covering paste conflict dialog; ghost row after Replace (#3493)

Browse Source 
* fix: progress window covering paste conflict dialog; ghost row after Replace

Pasting a copied file onto a name collision buried the Replace/Cancel
dialog under a stuck 'Preparing...' progress window, and answering
Replace left a stale duplicate row in every client.

- helpers.js: copy_clipboard_items armed its delayed progress window
  with a 0ms timer (its siblings use 2s), so the window opened
  instantly over the dialog. Use 2s, and in all three of
  copy_clipboard_items / copy_items / move_items pause the timer while
  a conflict dialog (or the own-location / trash-deny alerts) is
  waiting for input, re-arming it after. A window that opens
  mid-operation now shows the current file instead of a stuck
  'Preparing...', and the trash-deny bail no longer leaks a timer that
  opened an orphan window after the operation ended.
- LegacyFSController: /copy and /move dropped the legacy 'overwritten'
  response field and never emitted item.removed for the entry an
  overwrite deleted, so clients kept a ghost row until re-listing.
  Resolve the entry before the operation, return it, and emit
  item.removed on success.
- helpers.js: copy_items read resp[0].overwritten but removed
  resp.overwritten (always undefined), and the data-uid cleanup
  selectors were unquoted — invalid CSS when a UUID starts with a
  digit. Fixed all three sites.

* test: pin the v1 overwrite/collision wire contract for move and copy

The collision tests asserted only statusCode 409, which is how the
item_with_same_name_exists code regressed to 'conflict' unnoticed and
broke every replace/skip prompt in the GUI. Assert the legacy code and
entry_name explicitly, and add controller tests for the overwrite path:
the replaced entry must ride along as 'overwritten' in the /copy and
/move responses and be announced via outer.gui.item.removed so clients
drop its row.
This commit is contained in:
Nariman Jelveh
2026-08-02 22:17:10 -07:00
committed by GitHub
parent 1a9f025db6
commit 5c2a423658
4 changed files with 324 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/backend/controllers/fs/LegacyFSController.test.ts
+164
View File
@@ -928,6 +928,87 @@ describe('LegacyFSController.copy', () => {
// The entry must still exist — the ghost handler must NOT have run.
expect(await server.stores.fsEntry.getEntryByPath(src)).not.toBeNull();
});
it('surfaces a name collision, then reports and removes the replaced entry on overwrite', async () => {
const { actor } = await makeUser();
const username = actor.user!.username!;
const src = `/${username}/Documents/dup.txt`;
const existing = `/${username}/Pictures/dup.txt`;
for ( const p of [src, existing] ) {
await withActor(actor, () =>
controller.touch(
makeReq({ body: { path: p }, actor }),
makeRes().res,
),
);
}
// Without overwrite: the v1 conflict contract the GUI's
// replace/skip prompts key on.
await expect(
withActor(actor, () =>
controller.copy(
makeReq({
body: {
source: src,
destination: `/${username}/Pictures`,
},
actor,
}),
makeRes().res,
),
),
).rejects.toMatchObject({
statusCode: 409,
legacyCode: 'item_with_same_name_exists',
fields: { entry_name: 'dup.txt' },
});
const replaced = (await server.stores.fsEntry.getEntryByPath(
existing,
))!;
// With overwrite: the replaced entry rides along in the response
// (so the caller can drop its row) and item.removed tells every
// other client to do the same — without it they keep a ghost row
// until the directory is re-listed.
const emitSpy = vi.spyOn(server.clients.event, 'emit');
let body: Array<{
copied: { path: string };
overwritten?: { id: string };
}>;
let removedCall: (typeof emitSpy.mock.calls)[number] | undefined;
try {
const { res, captured } = makeRes();
await withActor(actor, () =>
controller.copy(
makeReq({
body: {
source: src,
destination: `/${username}/Pictures`,
overwrite: true,
},
actor,
}),
res,
),
);
body = captured.body as typeof body;
removedCall = emitSpy.mock.calls.find(
([eventName]) => eventName === 'outer.gui.item.removed',
);
} finally {
emitSpy.mockRestore();
}
expect(body[0].copied.path).toBe(existing);
expect(body[0].overwritten?.id).toBe(replaced.uuid);
expect(removedCall).toBeTruthy();
const removedPayload = removedCall?.[1] as {
response?: { uid?: string };
};
expect(removedPayload.response?.uid).toBe(replaced.uuid);
});
});
// ── move ────────────────────────────────────────────────────────────
@@ -1004,6 +1085,89 @@ describe('LegacyFSController.move', () => {
const body = captured.body as { moved: { path: string } };
expect(body.moved.path).toBe(`/${username}/Pictures/bar`);
});
it('surfaces a name collision, then reports and removes the replaced entry on overwrite', async () => {
const { actor } = await makeUser();
const username = actor.user!.username!;
const src = `/${username}/Documents/clash.txt`;
const existing = `/${username}/Pictures/clash.txt`;
for ( const p of [src, existing] ) {
await withActor(actor, () =>
controller.touch(
makeReq({ body: { path: p }, actor }),
makeRes().res,
),
);
}
// Without overwrite: the v1 conflict contract the GUI's
// replace/skip prompts key on.
await expect(
withActor(actor, () =>
controller.move(
makeReq({
body: {
source: src,
destination: `/${username}/Pictures`,
},
actor,
}),
makeRes().res,
),
),
).rejects.toMatchObject({
statusCode: 409,
legacyCode: 'item_with_same_name_exists',
fields: { entry_name: 'clash.txt' },
});
const replaced = (await server.stores.fsEntry.getEntryByPath(
existing,
))!;
// With overwrite: the replaced entry rides along in the response
// (so the caller can drop its row) and item.removed tells every
// other client to do the same — without it they keep a ghost row
// until the directory is re-listed.
const emitSpy = vi.spyOn(server.clients.event, 'emit');
let body: {
moved: { path: string };
old_path: string;
overwritten?: { id: string };
};
let removedCall: (typeof emitSpy.mock.calls)[number] | undefined;
try {
const { res, captured } = makeRes();
await withActor(actor, () =>
controller.move(
makeReq({
body: {
source: src,
destination: `/${username}/Pictures`,
overwrite: true,
},
actor,
}),
res,
),
);
body = captured.body as typeof body;
removedCall = emitSpy.mock.calls.find(
([eventName]) => eventName === 'outer.gui.item.removed',
);
} finally {
emitSpy.mockRestore();
}
expect(body.old_path).toBe(src);
expect(body.moved.path).toBe(existing);
expect(body.overwritten?.id).toBe(replaced.uuid);
expect(removedCall).toBeTruthy();
const removedPayload = removedCall?.[1] as {
response?: { uid?: string };
};
expect(removedPayload.response?.uid).toBe(replaced.uuid);
});
});
// ── search ──────────────────────────────────────────────────────────
src/backend/controllers/fs/LegacyFSController.ts
+90 -19
View File
@@ -166,8 +166,7 @@ export class LegacyFSController extends PuterController {
router.get('/get-launch-apps', apiOptions, async (req, res) => {
const recommendedSvc = this.services.recommendedApps as unknown as
| { getRecommendedApps?: () => Promise<unknown[]> }
| undefined;
{ getRecommendedApps?: () => Promise<unknown[]> } | undefined;
const recommended = recommendedSvc?.getRecommendedApps
? await recommendedSvc.getRecommendedApps()
: [];
@@ -589,26 +588,61 @@ export class LegacyFSController extends PuterController {
'write',
);
// The v1 wire contract reports the entry an overwrite replaced
// (`overwritten`) so clients can drop its stale row/icon. The copy
// deletes that entry, so resolve it beforehand.
const overwriteRequested = getBoolean(body, 'overwrite') ?? false;
let overwrittenEntry = null;
if (overwriteRequested) {
const targetName = getString(body, 'new_name') ?? source.name;
const targetPath =
destinationParent.path === '/'
? `/${targetName}`
: `${destinationParent.path}/${targetName}`;
overwrittenEntry =
await this.stores.fsEntry.getEntryByPath(targetPath);
}
const copy = await this.services.fs.copy(userId, {
source,
destinationParent,
newName: getString(body, 'new_name'),
overwrite: getBoolean(body, 'overwrite') ?? false,
overwrite: overwriteRequested,
dedupeName: getBoolean(body, 'dedupe_name', 'change_name') ?? false,
});
await this.#emitGuiEvent('outer.gui.item.added', copy);
// Without this, every other client keeps a ghost row for the
// replaced entry until the directory is re-listed.
if (overwrittenEntry) {
await this.#emitGuiEvent(
'outer.gui.item.removed',
overwrittenEntry,
);
}
// Legacy response shape: `[{copied: fsentry, overwritten?}]`.
// Array is historical — originally supported bulk copy.
const copied = await toLegacyEntry(this.clients.event, copy, {
const legacyEntryOpts = {
fsEntryStore: this.stores.fsEntry,
userStore: this.stores.user as unknown as {
getById: (
id: number,
) => Promise<Record<string, unknown> | null>;
},
});
res.json([{ copied }]);
};
const copied = await toLegacyEntry(
this.clients.event,
copy,
legacyEntryOpts,
);
const overwritten = overwrittenEntry
? await toLegacyEntry(
this.clients.event,
overwrittenEntry,
legacyEntryOpts,
)
: undefined;
res.json([{ copied, ...(overwritten ? { overwritten } : {}) }]);
};
move = async (req: Request, res: Response): Promise<void> => {
@@ -640,36 +674,77 @@ export class LegacyFSController extends PuterController {
'write',
);
// The v1 wire contract reports the entry an overwrite replaced
// (`overwritten`) so clients can drop its stale row/icon. The move
// deletes that entry, so resolve it beforehand.
const overwriteRequested = getBoolean(body, 'overwrite') ?? false;
let overwrittenEntry = null;
if (overwriteRequested) {
const targetName = getString(body, 'new_name') ?? source.name;
const targetPath =
destinationParent.path === '/'
? `/${targetName}`
: `${destinationParent.path}/${targetName}`;
const existing =
await this.stores.fsEntry.getEntryByPath(targetPath);
// Moving an entry onto its own path is not an overwrite.
if (existing && existing.uuid !== source.uuid) {
overwrittenEntry = existing;
}
}
const moved = await this.services.fs.move(userId, {
source,
destinationParent,
newName: getString(body, 'new_name'),
overwrite: getBoolean(body, 'overwrite') ?? false,
overwrite: overwriteRequested,
dedupeName: getBoolean(body, 'dedupe_name', 'change_name') ?? false,
// Trash/restore rides on this: GUI sends
// `{ original_name, original_path, trashed_ts }` when moving into
// Trash, and `null`/`{}` when restoring. See
// `src/gui/src/helpers.js` → `window.move_items`.
newMetadata: (body.new_metadata ?? undefined) as
| Record<string, unknown>
| null
| undefined,
Record<string, unknown> | null | undefined,
});
const oldPath = source.path;
await this.#emitGuiEvent('outer.gui.item.moved', moved, {
old_path: oldPath,
});
// Without this, every other client keeps a ghost row for the
// replaced entry until the directory is re-listed.
if (overwrittenEntry) {
await this.#emitGuiEvent(
'outer.gui.item.removed',
overwrittenEntry,
);
}
// Legacy response shape: `{moved: fsentry, old_path}`.
const movedEntry = await toLegacyEntry(this.clients.event, moved, {
// Legacy response shape: `{moved: fsentry, old_path, overwritten?}`.
const legacyEntryOpts = {
fsEntryStore: this.stores.fsEntry,
userStore: this.stores.user as unknown as {
getById: (
id: number,
) => Promise<Record<string, unknown> | null>;
},
};
const movedEntry = await toLegacyEntry(
this.clients.event,
moved,
legacyEntryOpts,
);
const overwritten = overwrittenEntry
? await toLegacyEntry(
this.clients.event,
overwrittenEntry,
legacyEntryOpts,
)
: undefined;
res.json({
moved: movedEntry,
old_path: oldPath,
...(overwritten ? { overwritten } : {}),
});
res.json({ moved: movedEntry, old_path: oldPath });
};
delete = async (req: Request, res: Response): Promise<void> => {
@@ -1090,8 +1165,7 @@ export class LegacyFSController extends PuterController {
}
type SignedOrEmpty =
| (SignedFile & { path?: string })
| Record<string, never>;
(SignedFile & { path?: string }) | Record<string, never>;
const result: { signatures: SignedOrEmpty[]; token?: string } = {
signatures: [],
};
@@ -1631,10 +1705,7 @@ export class LegacyFSController extends PuterController {
const subjectRef = body.subject;
const appRef = body.app;
const mode = (getString(body, 'mode') ?? 'read') as
| 'see'
| 'list'
| 'read'
| 'write';
'see' | 'list' | 'read' | 'write';
if (!subjectRef || !appRef)
throw new HttpError(400, '`subject` and `app` are required', {
legacyCode: 'bad_request',
src/backend/services/fs/FSService.test.ts
+15 -10
View File
@@ -2248,6 +2248,10 @@ describe('FSService move', () => {
fs.move(user.userId, { source, destinationParent: destination }),
);
expect(conflict.statusCode).toBe(409);
// v1 wire contract: the GUI's replace/skip prompts key on this
// code + entry_name; a generic 'conflict' makes them fail silently.
expect(conflict.legacyCode).toBe('item_with_same_name_exists');
expect(conflict.fields).toMatchObject({ entry_name: 'coll.txt' });
const deduped = await fs.move(user.userId, {
source,
@@ -2461,16 +2465,17 @@ describe('FSService copy', () => {
);
await writeFile(user, `${user.home}/Desktop/cp-coll.txt`, 'existing');
expect(
(
await caught(() =>
fs.copy(user.userId, {
source,
destinationParent: destination,
}),
)
).statusCode,
).toBe(409);
const conflict = await caught(() =>
fs.copy(user.userId, {
source,
destinationParent: destination,
}),
);
expect(conflict.statusCode).toBe(409);
// v1 wire contract: the GUI's replace/skip prompts key on this
// code + entry_name; a generic 'conflict' makes them fail silently.
expect(conflict.legacyCode).toBe('item_with_same_name_exists');
expect(conflict.fields).toMatchObject({ entry_name: 'cp-coll.txt' });
const deduped = await fs.copy(user.userId, {
source,
src/gui/src/helpers.js
+55 -10
View File
@@ -1255,14 +1255,21 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element)
// only show progress window if it takes longer than 2s to copy
let progwin;
let progwin_timeout = setTimeout(async () => {
let latest_status;
const arm_progwin = () => setTimeout(async () => {
progwin = await UIWindowProgress({
operation_id: copy_op_id,
on_cancel: () => {
window.operation_cancelled[copy_op_id] = true;
},
});
}, 0);
// Opened mid-operation: show the file being copied rather than
// the default "Preparing..." status.
if ( latest_status ) {
progwin.set_status(latest_status);
}
}, 2000);
let progwin_timeout = arm_progwin();
const copied_item_paths = [];
@@ -1270,7 +1277,8 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element)
let copy_path = window.clipboard[i].path;
let item_with_same_name_already_exists = true;
let overwrite = overwrite_all;
progwin?.set_status(i18n('copying_file', copy_path));
latest_status = i18n('copying_file', copy_path);
progwin?.set_status(latest_status);
do {
if ( overwrite )
@@ -1296,7 +1304,7 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element)
// remove overwritten item from the DOM
if ( resp[0].overwritten?.id ) {
$(`.item[data-uid=${resp[0].overwritten.id}]`).removeItems();
$(`.item[data-uid='${resp[0].overwritten.id}']`).removeItems();
}
// copy new path for undo copy
@@ -1306,6 +1314,10 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element)
break;
} catch ( err ) {
if ( err.code === 'item_with_same_name_exists' ) {
// The operation is paused on user input, so pause the
// progress-window timer too — otherwise "Preparing..."
// pops up on top of the dialog while it waits.
clearTimeout(progwin_timeout);
const alert_resp = await UIAlert({
message: `<strong>${html_encode(err.entry_name)}</strong> already exists.`,
buttons: [
@@ -1314,6 +1326,7 @@ window.copy_clipboard_items = async function (dest_path, dest_container_element)
... (window.clipboard.length > 1) ? [{ label: i18n('skip'), value: 'skip' }] : [{ label: i18n('cancel'), value: 'cancel' }],
],
});
progwin_timeout = arm_progwin();
if ( alert_resp === 'replace' ) {
overwrite = true;
} else if ( alert_resp === 'replace_all' ) {
@@ -1371,14 +1384,21 @@ window.copy_items = function (el_items, dest_path) {
// only show progress window if it takes longer than 2s to copy
let progwin;
let progwin_timeout = setTimeout(async () => {
let latest_status;
const arm_progwin = () => setTimeout(async () => {
progwin = await UIWindowProgress({
operation_id: copy_op_id,
on_cancel: () => {
window.operation_cancelled[copy_op_id] = true;
},
});
// Opened mid-operation: show the file being copied rather than
// the default "Preparing..." status.
if ( latest_status ) {
progwin.set_status(latest_status);
}
}, 2000);
let progwin_timeout = arm_progwin();
const copied_item_paths = [];
@@ -1386,7 +1406,8 @@ window.copy_items = function (el_items, dest_path) {
let copy_path = $(el_items[i]).attr('data-path');
let item_with_same_name_already_exists = true;
let overwrite = overwrite_all;
progwin?.set_status(i18n('copying_file', copy_path));
latest_status = i18n('copying_file', copy_path);
progwin?.set_status(latest_status);
do {
if ( overwrite )
@@ -1409,7 +1430,7 @@ window.copy_items = function (el_items, dest_path) {
// remove overwritten item from the DOM
if ( resp[0].overwritten?.id ) {
$(`.item[data-uid=${resp.overwritten.id}]`).removeItems();
$(`.item[data-uid='${resp[0].overwritten.id}']`).removeItems();
}
// copy new path for undo copy
@@ -1419,6 +1440,10 @@ window.copy_items = function (el_items, dest_path) {
item_with_same_name_already_exists = false;
} catch ( err ) {
if ( err.code === 'item_with_same_name_exists' ) {
// The operation is paused on user input, so pause the
// progress-window timer too — otherwise "Preparing..."
// pops up on top of the dialog while it waits.
clearTimeout(progwin_timeout);
const alert_resp = await UIAlert({
message: `<strong>${html_encode(err.entry_name)}</strong> already exists.`,
buttons: [
@@ -1427,6 +1452,7 @@ window.copy_items = function (el_items, dest_path) {
... (el_items.length > 1) ? [{ label: i18n('skip'), value: 'skip' }] : [{ label: i18n('cancel'), value: 'cancel' }],
],
});
progwin_timeout = arm_progwin();
if ( alert_resp === 'replace' ) {
overwrite = true;
} else if ( alert_resp === 'replace_all' ) {
@@ -1690,14 +1716,21 @@ window.move_items = async function (el_items, dest_path, is_undo = false) {
// only show progress window if it takes longer than 2s to move
let progwin;
let progwin_timeout = setTimeout(async () => {
let latest_status;
const arm_progwin = () => setTimeout(async () => {
progwin = await UIWindowProgress({
operation_id: move_op_id,
on_cancel: () => {
window.operation_cancelled[move_op_id] = true;
},
});
// Opened mid-operation: show the file being moved rather than the
// default "Preparing..." status.
if ( latest_status ) {
progwin.set_status(latest_status);
}
}, 2000);
let progwin_timeout = arm_progwin();
// storing moved items for undo ability
const moved_items = [];
@@ -1721,7 +1754,10 @@ window.move_items = async function (el_items, dest_path, is_undo = false) {
// cannot move item to its own path, skip it
if ( path.dirname($(el_item).attr('data-path')) === dest_path ) {
// pause the progress-window timer while waiting for the user
clearTimeout(progwin_timeout);
await UIAlert(`<p>Moving <strong>${html_encode($(el_item).attr('data-name'))}</strong></p>Cannot move item to its current location.`);
progwin_timeout = arm_progwin();
continue;
}
@@ -1794,6 +1830,9 @@ window.move_items = async function (el_items, dest_path, is_undo = false) {
// moving an item into a trashed directory? deny.
else if ( dest_path.startsWith(window.trash_path) ) {
// the pending timer would otherwise open an orphan
// progress window after the operation already bailed
clearTimeout(progwin_timeout);
progwin?.close();
UIAlert('Cannot move items into a deleted folder.');
return;
@@ -1813,7 +1852,8 @@ window.move_items = async function (el_items, dest_path, is_undo = false) {
// --------------------------------------------------------
// update progress window with current item being moved
// --------------------------------------------------------
progwin?.set_status(i18n(status_i18n_string, path_to_show_on_progwin));
latest_status = i18n(status_i18n_string, path_to_show_on_progwin);
progwin?.set_status(latest_status);
// execute move
let resp = await puter.fs.move({
@@ -1895,7 +1935,7 @@ window.move_items = async function (el_items, dest_path, is_undo = false) {
// if replacing an existing item, remove the old item that was just replaced
if ( resp.overwritten?.id ) {
$(`.item[data-uid=${resp.overwritten.id}]`).removeItems();
$(`.item[data-uid='${resp.overwritten.id}']`).removeItems();
}
// if this is trash, get original name from item metadata
@@ -1971,6 +2011,10 @@ window.move_items = async function (el_items, dest_path, is_undo = false) {
if ( err.code === 'item_with_same_name_exists' ) {
item_with_same_name_already_exists = true;
// The operation is paused on user input, so pause the
// progress-window timer too — otherwise "Preparing..."
// pops up on top of the dialog while it waits.
clearTimeout(progwin_timeout);
const alert_resp = await UIAlert({
message: `<strong>${html_encode(err.entry_name)}</strong> already exists.`,
buttons: [
@@ -1979,6 +2023,7 @@ window.move_items = async function (el_items, dest_path, is_undo = false) {
... (el_items.length > 1) ? [{ label: i18n('skip'), value: 'skip' }] : [{ label: i18n('cancel'), value: 'cancel' }],
],
});
progwin_timeout = arm_progwin();
if ( alert_resp === 'replace' ) {
overwrite = true;
} else if ( alert_resp === 'replace_all' ) {