fix: Bug - Two devices sharing same user says "Shared from your device"
Gate the "Shared from your device" label and the hidden download affordance on whether this device actually holds the file bytes, not on whether the current user uploaded it. uploaderPeerId is the user id, so the old check claimed ownership on every device of the uploader, blocking view/download on second devices that only synced metadata. Also include attachment metadata in the account_sync chat-sync-batch so sibling devices learn about synced attachments at all. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -147,6 +147,18 @@ Browser chat views render audio/video larger than 50 MB with the same generic fi
|
||||
|
||||
An optional experimental VLC.js adapter can be enabled from General settings. When enabled, unsupported downloaded audio/video files show a manual Play action that lazy-loads `/vlcjs/metoyou-vlc-player.js`. The runtime is intentionally isolated in the experimental media domain and is not part of the default attachment path.
|
||||
|
||||
## Ownership and the "Shared from your device" label
|
||||
|
||||
`uploaderPeerId` is the **user** id of whoever uploaded the file, not a per-device id. It is intentionally stable across a user's devices so an uploader can recognise their own attachments after sync. Because of that, "did *this* device upload it?" and "does *this* device hold the bytes?" are two different questions, and the UI must key the *sharing* affordance off the latter.
|
||||
|
||||
`attachment-sharing.rules.ts` makes this explicit:
|
||||
|
||||
- `isUploaderUser(attachment, currentUserId)` — the current user is the uploader (same user, any device).
|
||||
- `deviceHasLocalCopy(attachment)` — this device physically holds the bytes (`available` + a blob `objectUrl`, or a non-empty `savedPath`/`filePath`). Synced metadata alone does not count, because P2P/account sync strips local paths.
|
||||
- `isSharingFromThisDevice(attachment, currentUserId)` — `isUploaderUser && deviceHasLocalCopy`. Only this returns the "Shared from your device" state.
|
||||
|
||||
The chat message item renders "Shared from your device" (and hides the request/download affordance) **only** when `isSharingFromThisDevice` is true. A second device of the same user that merely synced the message metadata is the uploader-user but holds no local copy, so it falls back to the normal recipient flow (request/download) instead of falsely claiming ownership and blocking the file (regression: the old check used `uploaderPeerId === currentUserId` and so claimed ownership on every device of the uploader). The transfer service uses the same rule to decide whether a no-peers failure should read "your original upload is missing" (sharing device) or "no connected peers" (any other device).
|
||||
|
||||
## Persistence
|
||||
|
||||
Attachment file persistence is platform-agnostic. `AttachmentStorageService` owns the `server/<room>/<bucket>` and `direct-messages/...` path layout and delegates the raw byte IO to a pluggable `AttachmentFileStore` chosen by `PlatformService` (mirroring how `DatabaseService` picks a DB backend):
|
||||
|
||||
@@ -8,6 +8,7 @@ import { selectCurrentUserId } from '../../../../store/users/users.selectors';
|
||||
import { AttachmentStorageService } from '../../infrastructure/services/attachment-storage.service';
|
||||
import { MAX_AUTO_SAVE_SIZE_BYTES } from '../../domain/constants/attachment.constants';
|
||||
import { isImageAttachment, resolvePublishAttachmentIsImage } from '../../domain/logic/attachment-image.rules';
|
||||
import { isSharingFromThisDevice } from '../../domain/logic/attachment-sharing.rules';
|
||||
import { shouldCopyUploaderMediaToAppData, shouldPersistDownloadedAttachment } from '../../domain/logic/attachment.logic';
|
||||
import type { Attachment, AttachmentMeta } from '../../domain/models/attachment.model';
|
||||
import {
|
||||
@@ -170,13 +171,15 @@ export class AttachmentTransferService {
|
||||
|
||||
const connectedPeers = this.webrtc.getConnectedPeers();
|
||||
const currentUserId = await this.resolveCurrentUserId();
|
||||
const isUploader = !!attachment.uploaderPeerId &&
|
||||
!!currentUserId &&
|
||||
attachment.uploaderPeerId === currentUserId;
|
||||
// Only the device that actually still holds the original bytes should report a
|
||||
// missing local upload. A second device of the same user that merely synced the
|
||||
// metadata is not the sharing device, so it falls back to the regular peer-request
|
||||
// flow (and the "no connected peers" error when offline) like any other recipient.
|
||||
const sharingFromThisDevice = isSharingFromThisDevice(attachment, currentUserId);
|
||||
|
||||
if (connectedPeers.length === 0) {
|
||||
this.runtimeStore.deletePendingRequest(requestKey);
|
||||
attachment.requestError = isUploader
|
||||
attachment.requestError = sharingFromThisDevice
|
||||
? this.appI18n.instant(UPLOADER_LOCAL_FILE_MISSING_ERROR_KEY)
|
||||
: this.appI18n.instant(NO_CONNECTED_PEERS_REQUEST_ERROR_KEY);
|
||||
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
import {
|
||||
deviceHasLocalCopy,
|
||||
isSharingFromThisDevice,
|
||||
isUploaderUser
|
||||
} from './attachment-sharing.rules';
|
||||
|
||||
describe('attachment sharing rules', () => {
|
||||
describe('isUploaderUser', () => {
|
||||
it('is true when the attachment uploader matches the current user', () => {
|
||||
expect(isUploaderUser({ uploaderPeerId: 'user-1' }, 'user-1')).toBe(true);
|
||||
});
|
||||
|
||||
it('is false when the uploader is a different user', () => {
|
||||
expect(isUploaderUser({ uploaderPeerId: 'user-2' }, 'user-1')).toBe(false);
|
||||
});
|
||||
|
||||
it('is false when either id is missing', () => {
|
||||
expect(isUploaderUser({ uploaderPeerId: undefined }, 'user-1')).toBe(false);
|
||||
expect(isUploaderUser({ uploaderPeerId: 'user-1' }, null)).toBe(false);
|
||||
expect(isUploaderUser({ uploaderPeerId: 'user-1' }, '')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('deviceHasLocalCopy', () => {
|
||||
it('is true when an available blob object url is present', () => {
|
||||
expect(deviceHasLocalCopy({ available: true, objectUrl: 'blob:abc' })).toBe(true);
|
||||
});
|
||||
|
||||
it('is true when a savedPath or filePath is present', () => {
|
||||
expect(deviceHasLocalCopy({ available: false, savedPath: '/appdata/file.bin' })).toBe(true);
|
||||
expect(deviceHasLocalCopy({ available: false, filePath: '/home/me/file.bin' })).toBe(true);
|
||||
});
|
||||
|
||||
it('is false when only metadata exists (no bytes, no paths)', () => {
|
||||
expect(deviceHasLocalCopy({ available: false })).toBe(false);
|
||||
expect(deviceHasLocalCopy({ available: false, savedPath: ' ', filePath: '' })).toBe(false);
|
||||
});
|
||||
|
||||
it('is false when marked available but no object url is present yet', () => {
|
||||
expect(deviceHasLocalCopy({ available: true })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isSharingFromThisDevice', () => {
|
||||
it('is true for the uploader device that still holds the file locally', () => {
|
||||
expect(
|
||||
isSharingFromThisDevice({ uploaderPeerId: 'user-1', available: true, objectUrl: 'blob:abc' }, 'user-1')
|
||||
).toBe(true);
|
||||
|
||||
expect(
|
||||
isSharingFromThisDevice({ uploaderPeerId: 'user-1', available: false, savedPath: '/appdata/file.bin' }, 'user-1')
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('is false on a second device of the same user that only synced metadata', () => {
|
||||
// This is the regression: the user uploaded from another device, so the metadata
|
||||
// carries uploaderPeerId === currentUserId, but this device holds no local bytes.
|
||||
expect(
|
||||
isSharingFromThisDevice({ uploaderPeerId: 'user-1', available: false }, 'user-1')
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('is false for a different user even if they downloaded the file locally', () => {
|
||||
expect(
|
||||
isSharingFromThisDevice({ uploaderPeerId: 'user-1', available: true, objectUrl: 'blob:abc' }, 'user-2')
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,40 @@
|
||||
import type { Attachment } from '../models/attachment.model';
|
||||
|
||||
/** True when the current user is the one who originally uploaded this attachment. */
|
||||
export function isUploaderUser(
|
||||
attachment: Pick<Attachment, 'uploaderPeerId'>,
|
||||
currentUserId: string | null | undefined
|
||||
): boolean {
|
||||
return !!attachment.uploaderPeerId && !!currentUserId && attachment.uploaderPeerId === currentUserId;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when this specific device physically holds the file bytes - either hydrated
|
||||
* in memory (available blob object url) or persisted to disk / present at its original
|
||||
* upload path. Synced metadata alone (no bytes, no local paths) does not count.
|
||||
*/
|
||||
export function deviceHasLocalCopy(
|
||||
attachment: Pick<Attachment, 'available' | 'objectUrl' | 'savedPath' | 'filePath'>
|
||||
): boolean {
|
||||
return (attachment.available === true && hasNonEmptyString(attachment.objectUrl)) ||
|
||||
hasNonEmptyString(attachment.savedPath) ||
|
||||
hasNonEmptyString(attachment.filePath);
|
||||
}
|
||||
|
||||
/**
|
||||
* True only when the current user uploaded the file AND this device still holds a local
|
||||
* copy to serve. A second device of the same user that only synced the message metadata
|
||||
* is the uploader-user but has no local bytes, so it must behave like any other peer
|
||||
* (request/download from peers) instead of claiming "Shared from your device" and hiding
|
||||
* the download affordance.
|
||||
*/
|
||||
export function isSharingFromThisDevice(
|
||||
attachment: Pick<Attachment, 'uploaderPeerId' | 'available' | 'objectUrl' | 'savedPath' | 'filePath'>,
|
||||
currentUserId: string | null | undefined
|
||||
): boolean {
|
||||
return isUploaderUser(attachment, currentUserId) && deviceHasLocalCopy(attachment);
|
||||
}
|
||||
|
||||
function hasNonEmptyString(value: string | null | undefined): boolean {
|
||||
return typeof value === 'string' && value.trim().length > 0;
|
||||
}
|
||||
@@ -1,4 +1,5 @@
|
||||
export * from './application/facades/attachment.facade';
|
||||
export * from './domain/constants/attachment.constants';
|
||||
export * from './domain/logic/attachment-sharing.rules';
|
||||
export * from './domain/logic/local-file-path.rules';
|
||||
export * from './domain/models/attachment.model';
|
||||
|
||||
Reference in New Issue
Block a user