fix: Restore chat attachments after reload
Wait for persisted attachment metadata before serving or advertising files so reconnecting peers can load images and downloads reliably. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -78,6 +78,7 @@ This area does **not** own:
|
|||||||
- Transfers are between connected peers only (no server CDN).
|
- Transfers are between connected peers only (no server CDN).
|
||||||
- Receive strategy is decided once at request time by `canReceiveAttachment` (`attachment.logic.ts`): ≤ 10 MB assembles in memory everywhere; > 10 MB streams to disk on Electron/Capacitor, assembles in memory on the browser up to its 50 MB persist cap, and is rejected with a visible `fileTooLarge` error beyond that. `handleFileChunk` must accept whatever the request gate admitted — a stricter chunk-time size cap silently drops chunks and stalls the transfer.
|
- Receive strategy is decided once at request time by `canReceiveAttachment` (`attachment.logic.ts`): ≤ 10 MB assembles in memory everywhere; > 10 MB streams to disk on Electron/Capacitor, assembles in memory on the browser up to its 50 MB persist cap, and is rejected with a visible `fileTooLarge` error beyond that. `handleFileChunk` must accept whatever the request gate admitted — a stricter chunk-time size cap silently drops chunks and stalls the transfer.
|
||||||
- Visibility-based blob lifecycle on desktop: revoke `blob:` URLs when messages scroll off-screen if disk can rehydrate.
|
- Visibility-based blob lifecycle on desktop: revoke `blob:` URLs when messages scroll off-screen if disk can rehydrate.
|
||||||
|
- Startup hydration is an availability boundary: file requests and host re-announcements wait for persisted metadata before inspecting local files. A host re-announces persisted files after reload and on peer connection even from non-chat routes, and can recover an original Electron source path by copying it into app data on demand before serving.
|
||||||
- Display-blob memory invariants (added 2026-07-14, RAM investigation):
|
- Display-blob memory invariants (added 2026-07-14, RAM investigation):
|
||||||
- Inline hydration (`chat-message-item` effect) only runs for messages that are visible or within the `IntersectionObserver` root margin — gated by `attachment-hydration-visibility.rules.ts`. Off-screen rows never load blobs.
|
- Inline hydration (`chat-message-item` effect) only runs for messages that are visible or within the `IntersectionObserver` root margin — gated by `attachment-hydration-visibility.rules.ts`. Off-screen rows never load blobs.
|
||||||
- Disk-hydrated blobs are **not** duplicated into `AttachmentRuntimeStore.originalFiles`; peer requests are served from the disk path (`streamRequestedFile` prefers `resolveExistingPath`). `originalFiles` only holds uploads/downloads that have no disk copy yet.
|
- Disk-hydrated blobs are **not** duplicated into `AttachmentRuntimeStore.originalFiles`; peer requests are served from the disk path (`streamRequestedFile` prefers `resolveExistingPath`). `originalFiles` only holds uploads/downloads that have no disk copy yet.
|
||||||
@@ -120,6 +121,7 @@ This area does **not** own:
|
|||||||
|
|
||||||
| Date | Change |
|
| Date | Change |
|
||||||
|------|--------|
|
|------|--------|
|
||||||
|
| 2026-07-14 | Prevented reload-time `file-not-found` responses by waiting for metadata hydration, re-announcing hosts outside chat routes, and recovering persisted source paths on demand |
|
||||||
| 2026-07-14 | Blob-memory invariants: visibility-gated hydration, no `originalFiles` duplication for disk-backed blobs, revoke-on-destroy, inactive-room blob sweep |
|
| 2026-07-14 | Blob-memory invariants: visibility-gated hydration, no `originalFiles` duplication for disk-backed blobs, revoke-on-destroy, inactive-room blob sweep |
|
||||||
| 2026-07-13 | Capacitor download/export to public `Documents` via `CapacitorAttachmentExportService` |
|
| 2026-07-13 | Capacitor download/export to public `Documents` via `CapacitorAttachmentExportService` |
|
||||||
| 2026-07-05 | Expanded to full contract style |
|
| 2026-07-05 | Expanded to full contract style |
|
||||||
|
|||||||
@@ -117,6 +117,8 @@ If the sender cannot find the file, it replies with `file-not-found`. The transf
|
|||||||
|
|
||||||
Peers that finish downloading a file re-announce it and register themselves as mirror hosts. New download requests prefer mirror hosts over the original uploader so the sharer's device is not the only upload source. Repeat `file-announce` events for already-known attachments update the host list but do not re-trigger auto-download. Outgoing `file-announce` broadcasts are also relayed to sibling devices through `account_sync` (see `infrastructure/realtime/account-sync/account-sync.rules.ts`) so a second client of the same user learns attachment metadata even when it cannot P2P to itself.
|
Peers that finish downloading a file re-announce it and register themselves as mirror hosts. New download requests prefer mirror hosts over the original uploader so the sharer's device is not the only upload source. Repeat `file-announce` events for already-known attachments update the host list but do not re-trigger auto-download. Outgoing `file-announce` broadcasts are also relayed to sibling devices through `account_sync` (see `infrastructure/realtime/account-sync/account-sync.rules.ts`) so a second client of the same user learns attachment metadata even when it cannot P2P to itself.
|
||||||
|
|
||||||
|
After reload, serving and re-announcing wait for persisted attachment metadata to finish hydrating. Hosted files are announced even when the uploader is not currently viewing a room, so reconnecting peers can discover them from dashboard and other non-chat routes. If only an original Electron file-picker path survived, the first request copies that file into app data before streaming it; a request must not receive `file-not-found` merely because it raced startup hydration.
|
||||||
|
|
||||||
```mermaid
|
```mermaid
|
||||||
sequenceDiagram
|
sequenceDiagram
|
||||||
participant R as Receiver
|
participant R as Receiver
|
||||||
|
|||||||
+13
-3
@@ -61,8 +61,11 @@ export class AttachmentManagerService {
|
|||||||
void this.persistence.initFromDatabase().then(async () => {
|
void this.persistence.initFromDatabase().then(async () => {
|
||||||
if (this.watchedRoomId) {
|
if (this.watchedRoomId) {
|
||||||
await this.restoreLocalAttachmentsForRoom(this.watchedRoomId);
|
await this.restoreLocalAttachmentsForRoom(this.watchedRoomId);
|
||||||
await this.announceHostedAttachments();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Announce regardless of the current route - a reloaded uploader
|
||||||
|
// sitting on the dashboard still hosts its persisted files.
|
||||||
|
await this.announceHostedAttachments();
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
@@ -88,12 +91,19 @@ export class AttachmentManagerService {
|
|||||||
|
|
||||||
this.webrtc.onPeerConnected.subscribe(() => {
|
this.webrtc.onPeerConnected.subscribe(() => {
|
||||||
if (this.watchedRoomId) {
|
if (this.watchedRoomId) {
|
||||||
void this.restoreLocalAttachmentsForRoom(this.watchedRoomId).then(async () => {
|
const watchedRoomId = this.watchedRoomId;
|
||||||
|
|
||||||
|
void this.restoreLocalAttachmentsForRoom(watchedRoomId).then(async () => {
|
||||||
await this.announceHostedAttachments();
|
await this.announceHostedAttachments();
|
||||||
});
|
});
|
||||||
|
|
||||||
void this.requestAutoDownloadsForRoom(this.watchedRoomId);
|
void this.requestAutoDownloadsForRoom(watchedRoomId);
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// No room open (e.g. reloaded onto the dashboard) - still announce
|
||||||
|
// persisted files so peers relearn this device hosts them.
|
||||||
|
void this.announceHostedAttachments();
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+90
-1
@@ -42,6 +42,7 @@ describe('AttachmentTransferService', () => {
|
|||||||
resolveCurrentRoomName: ReturnType<typeof vi.fn>;
|
resolveCurrentRoomName: ReturnType<typeof vi.fn>;
|
||||||
resolveStorageContainerName: ReturnType<typeof vi.fn>;
|
resolveStorageContainerName: ReturnType<typeof vi.fn>;
|
||||||
ensureInlineDisplayObjectUrl: ReturnType<typeof vi.fn>;
|
ensureInlineDisplayObjectUrl: ReturnType<typeof vi.fn>;
|
||||||
|
ensurePersistedUploadHost: ReturnType<typeof vi.fn>;
|
||||||
};
|
};
|
||||||
let attachmentStorage: {
|
let attachmentStorage: {
|
||||||
canWriteFiles: ReturnType<typeof vi.fn>;
|
canWriteFiles: ReturnType<typeof vi.fn>;
|
||||||
@@ -83,7 +84,8 @@ describe('AttachmentTransferService', () => {
|
|||||||
persistUploadCopyFromSourcePath: vi.fn(async () => null),
|
persistUploadCopyFromSourcePath: vi.fn(async () => null),
|
||||||
resolveCurrentRoomName: vi.fn(async () => null),
|
resolveCurrentRoomName: vi.fn(async () => null),
|
||||||
resolveStorageContainerName: vi.fn(async () => 'room'),
|
resolveStorageContainerName: vi.fn(async () => 'room'),
|
||||||
ensureInlineDisplayObjectUrl: vi.fn(async () => true)
|
ensureInlineDisplayObjectUrl: vi.fn(async () => true),
|
||||||
|
ensurePersistedUploadHost: vi.fn(async () => false)
|
||||||
};
|
};
|
||||||
|
|
||||||
attachmentStorage = {
|
attachmentStorage = {
|
||||||
@@ -741,6 +743,93 @@ describe('AttachmentTransferService', () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('hydrates persisted metadata before serving a file request after reload', async () => {
|
||||||
|
// Reload race: the peer's file-request arrives before initFromDatabase has
|
||||||
|
// filled the runtime store. Serving must wait for hydration instead of
|
||||||
|
// replying file-not-found for a file that is on disk.
|
||||||
|
attachmentStorage.resolveExistingPath.mockResolvedValue('/appdata/server/room/files/setup.exe');
|
||||||
|
attachmentStorage.getFileSize.mockResolvedValue(12 * 1024 * 1024);
|
||||||
|
persistence.whenReady.mockImplementation(async () => {
|
||||||
|
const attachment = registerIncomingGenericFile(12 * 1024 * 1024);
|
||||||
|
|
||||||
|
attachment.savedPath = '/appdata/server/room/files/setup.exe';
|
||||||
|
});
|
||||||
|
|
||||||
|
const service = createService();
|
||||||
|
|
||||||
|
await service.handleFileRequest({
|
||||||
|
messageId: MESSAGE_ID,
|
||||||
|
fileId: FILE_ID,
|
||||||
|
fromPeerId: 'peer-2'
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(transport.streamFileFromDiskToPeer).toHaveBeenCalledWith(
|
||||||
|
'peer-2',
|
||||||
|
MESSAGE_ID,
|
||||||
|
FILE_ID,
|
||||||
|
'/appdata/server/room/files/setup.exe',
|
||||||
|
expect.any(Function)
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(webrtc.sendToPeer).not.toHaveBeenCalledWith('peer-2', expect.objectContaining({ type: 'file-not-found' }));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('copies an external upload into app data when serving a request after reload', async () => {
|
||||||
|
// savedPath missing (publish copy failed or pre-fix upload) but the original
|
||||||
|
// file-picker path survived - the serve path must persist it on demand
|
||||||
|
// instead of replying file-not-found.
|
||||||
|
attachmentStorage.resolveExistingPath
|
||||||
|
.mockResolvedValueOnce(null)
|
||||||
|
.mockResolvedValue('/appdata/server/room/files/setup.exe');
|
||||||
|
|
||||||
|
attachmentStorage.getFileSize.mockResolvedValue(628 * 1024 * 1024);
|
||||||
|
persistence.ensurePersistedUploadHost.mockImplementation(async (attachment: Attachment) => {
|
||||||
|
attachment.savedPath = '/appdata/server/room/files/setup.exe';
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
|
||||||
|
const service = createService();
|
||||||
|
const attachment = registerIncomingGenericFile(628 * 1024 * 1024);
|
||||||
|
|
||||||
|
attachment.filePath = '/home/nim/Downloads/setup.exe';
|
||||||
|
|
||||||
|
await service.handleFileRequest({
|
||||||
|
messageId: MESSAGE_ID,
|
||||||
|
fileId: FILE_ID,
|
||||||
|
fromPeerId: 'peer-2'
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(persistence.ensurePersistedUploadHost).toHaveBeenCalledWith(attachment, { hydrateMediaForDisplay: false });
|
||||||
|
expect(transport.streamFileFromDiskToPeer).toHaveBeenCalledWith(
|
||||||
|
'peer-2',
|
||||||
|
MESSAGE_ID,
|
||||||
|
FILE_ID,
|
||||||
|
'/appdata/server/room/files/setup.exe',
|
||||||
|
expect.any(Function)
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(webrtc.sendToPeer).not.toHaveBeenCalledWith('peer-2', expect.objectContaining({ type: 'file-not-found' }));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('hydrates persisted metadata before re-announcing hosted attachments', async () => {
|
||||||
|
attachmentStorage.resolveExistingPath.mockResolvedValue('/appdata/server/room/files/setup.exe');
|
||||||
|
persistence.whenReady.mockImplementation(async () => {
|
||||||
|
const attachment = registerIncomingGenericFile(12 * 1024 * 1024);
|
||||||
|
|
||||||
|
attachment.savedPath = '/appdata/server/room/files/setup.exe';
|
||||||
|
});
|
||||||
|
|
||||||
|
const service = createService();
|
||||||
|
|
||||||
|
await service.reannounceHostedAttachments(PEER_ID);
|
||||||
|
|
||||||
|
expect(webrtc.broadcastMessage).toHaveBeenCalledWith(expect.objectContaining({
|
||||||
|
type: 'file-announce',
|
||||||
|
messageId: MESSAGE_ID,
|
||||||
|
file: expect.objectContaining({ id: FILE_ID })
|
||||||
|
}));
|
||||||
|
});
|
||||||
|
|
||||||
it('re-announces hosted attachments that can still be served from disk', async () => {
|
it('re-announces hosted attachments that can still be served from disk', async () => {
|
||||||
attachmentStorage.resolveExistingPath.mockResolvedValue('/appdata/server/room/files/setup.exe');
|
attachmentStorage.resolveExistingPath.mockResolvedValue('/appdata/server/room/files/setup.exe');
|
||||||
|
|
||||||
|
|||||||
+24
-1
@@ -587,13 +587,32 @@ export class AttachmentTransferService {
|
|||||||
fileId: string,
|
fileId: string,
|
||||||
fromPeerId: string
|
fromPeerId: string
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
// A request can race ahead of post-reload metadata hydration; answering
|
||||||
|
// file-not-found for a file that is on disk makes peers give up for good.
|
||||||
|
await this.persistence.whenReady();
|
||||||
|
|
||||||
const exactKey = `${messageId}:${fileId}`;
|
const exactKey = `${messageId}:${fileId}`;
|
||||||
const list = this.runtimeStore.getAttachmentsForMessage(messageId);
|
const list = this.runtimeStore.getAttachmentsForMessage(messageId);
|
||||||
const attachment = list.find((entry) => entry.id === fileId);
|
const attachment = list.find((entry) => entry.id === fileId);
|
||||||
const diskPath = attachment
|
|
||||||
|
let diskPath = attachment
|
||||||
? await this.attachmentStorage.resolveExistingPath(attachment)
|
? await this.attachmentStorage.resolveExistingPath(attachment)
|
||||||
: null;
|
: null;
|
||||||
|
|
||||||
|
if (!diskPath && attachment && attachment.filePath?.trim()) {
|
||||||
|
// Only the original file-picker path survived the reload (publish copy
|
||||||
|
// failed or predates it). Copy it into app data now so this and future
|
||||||
|
// requests are served from disk instead of failing.
|
||||||
|
const persisted = await this.persistence.ensurePersistedUploadHost(
|
||||||
|
attachment,
|
||||||
|
{ hydrateMediaForDisplay: false }
|
||||||
|
);
|
||||||
|
|
||||||
|
if (persisted) {
|
||||||
|
diskPath = await this.attachmentStorage.resolveExistingPath(attachment);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (diskPath && shouldServeAttachmentFromDiskPath(await this.attachmentStorage.getFileSize(diskPath))) {
|
if (diskPath && shouldServeAttachmentFromDiskPath(await this.attachmentStorage.getFileSize(diskPath))) {
|
||||||
await this.transport.streamFileFromDiskToPeer(
|
await this.transport.streamFileFromDiskToPeer(
|
||||||
fromPeerId,
|
fromPeerId,
|
||||||
@@ -868,6 +887,10 @@ export class AttachmentTransferService {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// After a reload the runtime store is only populated once persisted
|
||||||
|
// metadata has been hydrated - announcing before that sees no attachments.
|
||||||
|
await this.persistence.whenReady();
|
||||||
|
|
||||||
for (const [, attachments] of this.runtimeStore.getAttachmentEntries()) {
|
for (const [, attachments] of this.runtimeStore.getAttachmentEntries()) {
|
||||||
for (const attachment of attachments) {
|
for (const attachment of attachments) {
|
||||||
if (!canHostAttachment(attachment)) {
|
if (!canHostAttachment(attachment)) {
|
||||||
|
|||||||
Reference in New Issue
Block a user