mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
🕰️ fix: Stop Pinning and Archiving From Counting as Chat Activity (#14861)
* fix: stop pinning and archiving from counting as chat activity Both routes went through saveConvo, which lets mongoose stamp updatedAt. The sidebar orders chats by that field, so pinning hoisted an untouched chat to the top of Today, and unarchiving a year-old chat dropped it there too instead of back into its own date group. saveConvo now takes preserveUpdatedAt, and both routes pass it. They also pass noUpsert: with timestamps suppressed an upsert would insert a conversation carrying neither createdAt nor updatedAt, so an unknown conversation id is now a 404 rather than a silently created stub. * test: pin a project's activity pointer against metadata-only saves Review raised that preserving updatedAt could drag a project's lastConversationAt back to the pinned chat's older timestamp, since the incremental path $sets it outright. That path is not reachable here: a pin carries no chatProjectId, so previousChatProjectId stays null while the conversation has a real one, projectMembershipChanged is therefore true, and saveConvo takes the full recompute branch instead. This test holds that in place, with a newer sibling conversation in the project so a regression to the incremental path would fail it. * fix: keep updatedAt through the retention backfill Under RetentionMode.ALL a legacy chat with no stored isTemporary gets a second write after the main update, and that one still had mongoose timestamps enabled. The first archive of such a chat therefore bumped updatedAt anyway and landed in Today, defeating preserveUpdatedAt on exactly the old conversations it was meant to protect.
This commit is contained in:
Marco Beretta
GitHub
parent
fb8ae881cf
commit
5fc05ac037
4 changed files with 227 additions and 8 deletions
|
|
@ -543,7 +543,11 @@ describe('Convos Routes', () => {
|
|||
expect(saveConvo).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ userId: 'test-user-123' }),
|
||||
{ conversationId: mockConversationId, isArchived: true },
|
||||
{ context: `POST /api/convos/archive ${mockConversationId}` },
|
||||
{
|
||||
context: `POST /api/convos/archive ${mockConversationId}`,
|
||||
preserveUpdatedAt: true,
|
||||
noUpsert: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -572,7 +576,11 @@ describe('Convos Routes', () => {
|
|||
expect(saveConvo).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ userId: 'test-user-123' }),
|
||||
{ conversationId: mockConversationId, isArchived: false },
|
||||
{ context: `POST /api/convos/archive ${mockConversationId}` },
|
||||
{
|
||||
context: `POST /api/convos/archive ${mockConversationId}`,
|
||||
preserveUpdatedAt: true,
|
||||
noUpsert: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -663,7 +671,11 @@ describe('Convos Routes', () => {
|
|||
expect(saveConvo).toHaveBeenCalledWith(
|
||||
{ userId: 'test-user-123' },
|
||||
{ conversationId: mockConversationId, pinned: true },
|
||||
{ context: `POST /api/convos/pin ${mockConversationId}` },
|
||||
{
|
||||
context: `POST /api/convos/pin ${mockConversationId}`,
|
||||
preserveUpdatedAt: true,
|
||||
noUpsert: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -678,10 +690,27 @@ describe('Convos Routes', () => {
|
|||
expect(saveConvo).toHaveBeenCalledWith(
|
||||
{ userId: 'test-user-123' },
|
||||
{ conversationId: mockConversationId, pinned: false },
|
||||
{ context: `POST /api/convos/pin ${mockConversationId}` },
|
||||
{
|
||||
context: `POST /api/convos/pin ${mockConversationId}`,
|
||||
preserveUpdatedAt: true,
|
||||
noUpsert: true,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
/** With timestamps suppressed, an upsert would insert a conversation carrying
|
||||
* neither createdAt nor updatedAt, so an unknown id has to be rejected instead. */
|
||||
it('should return 404 when the conversation does not exist', async () => {
|
||||
saveConvo.mockResolvedValue(null);
|
||||
|
||||
const response = await request(app)
|
||||
.post('/api/convos/pin')
|
||||
.send({ arg: { conversationId: 'missing-convo', pinned: true } });
|
||||
|
||||
expect(response.status).toBe(404);
|
||||
expect(response.body).toEqual({ error: 'Conversation not found' });
|
||||
});
|
||||
|
||||
it('should return 400 when conversationId is missing', async () => {
|
||||
const response = await request(app)
|
||||
.post('/api/convos/pin')
|
||||
|
|
|
|||
|
|
@ -208,8 +208,20 @@ router.post('/archive', validateConvoAccess, async (req, res) => {
|
|||
interfaceConfig: req?.config?.interfaceConfig,
|
||||
},
|
||||
{ conversationId, isArchived },
|
||||
{ context: `POST /api/convos/archive ${conversationId}` },
|
||||
{
|
||||
context: `POST /api/convos/archive ${conversationId}`,
|
||||
/** Filing a chat away is not activity: `updatedAt` stays the chat's own last
|
||||
* activity so unarchiving restores it to its real place in the date groups. */
|
||||
preserveUpdatedAt: true,
|
||||
/** Without timestamps, an upsert would insert a conversation that has none. */
|
||||
noUpsert: true,
|
||||
},
|
||||
);
|
||||
|
||||
if (!dbResponse) {
|
||||
return res.status(404).json({ error: 'Conversation not found' });
|
||||
}
|
||||
|
||||
res.status(200).json(dbResponse);
|
||||
} catch (error) {
|
||||
logger.error('Error archiving conversation', error);
|
||||
|
|
@ -236,8 +248,19 @@ router.post('/pin', validateConvoAccess, async (req, res) => {
|
|||
const dbResponse = await db.saveConvo(
|
||||
{ userId: req.user.id },
|
||||
{ conversationId, pinned },
|
||||
{ context: `POST /api/convos/pin ${conversationId}` },
|
||||
{
|
||||
context: `POST /api/convos/pin ${conversationId}`,
|
||||
/** Pinning is a bookmark, not activity, so the sidebar's `updatedAt` order stands. */
|
||||
preserveUpdatedAt: true,
|
||||
/** Without timestamps, an upsert would insert a conversation that has none. */
|
||||
noUpsert: true,
|
||||
},
|
||||
);
|
||||
|
||||
if (!dbResponse) {
|
||||
return res.status(404).json({ error: 'Conversation not found' });
|
||||
}
|
||||
|
||||
res.status(200).json(dbResponse);
|
||||
} catch (error) {
|
||||
logger.error('Error pinning conversation', error);
|
||||
|
|
|
|||
|
|
@ -389,6 +389,161 @@ describe('Conversation Operations', () => {
|
|||
expect(new Date(secondSave?.createdAt ?? 0).toISOString()).toBe(firstAnchor.toISOString());
|
||||
expect(secondSave?.title).toBe('Updated title');
|
||||
});
|
||||
|
||||
describe('preserveUpdatedAt', () => {
|
||||
const anchor = new Date('2026-02-11T15:28:18.561Z');
|
||||
|
||||
const seedAgedConvo = async () => {
|
||||
const conversationId = uuidv4();
|
||||
await Conversation.collection.insertOne({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
title: 'Date And Time Test',
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
expiredAt: null,
|
||||
isArchived: false,
|
||||
createdAt: anchor,
|
||||
updatedAt: anchor,
|
||||
});
|
||||
return conversationId;
|
||||
};
|
||||
|
||||
/** The sidebar orders by `updatedAt`, so a pin that bumped it would hoist an
|
||||
* untouched chat into Today and drop it back out of place on unpin. */
|
||||
it('leaves updatedAt untouched across a pin and unpin round trip', async () => {
|
||||
const conversationId = await seedAgedConvo();
|
||||
|
||||
const pinned = await saveConvo(
|
||||
{ userId: 'user123' },
|
||||
{ conversationId, pinned: true },
|
||||
{ preserveUpdatedAt: true, noUpsert: true },
|
||||
);
|
||||
const unpinned = await saveConvo(
|
||||
{ userId: 'user123' },
|
||||
{ conversationId, pinned: false },
|
||||
{ preserveUpdatedAt: true, noUpsert: true },
|
||||
);
|
||||
|
||||
expect(pinned?.pinned).toBe(true);
|
||||
expect(unpinned?.pinned).toBe(false);
|
||||
expect(new Date(pinned?.updatedAt ?? 0).toISOString()).toBe(anchor.toISOString());
|
||||
expect(new Date(unpinned?.updatedAt ?? 0).toISOString()).toBe(anchor.toISOString());
|
||||
expect(new Date(pinned?.createdAt ?? 0).toISOString()).toBe(anchor.toISOString());
|
||||
});
|
||||
|
||||
/** Archiving files a chat away rather than touching it, and unarchiving has to
|
||||
* restore it to its real place in the date groups. */
|
||||
it('leaves updatedAt untouched across an archive and unarchive round trip', async () => {
|
||||
const conversationId = await seedAgedConvo();
|
||||
|
||||
const archived = await saveConvo(
|
||||
{ userId: 'user123' },
|
||||
{ conversationId, isArchived: true },
|
||||
{ preserveUpdatedAt: true, noUpsert: true },
|
||||
);
|
||||
const unarchived = await saveConvo(
|
||||
{ userId: 'user123' },
|
||||
{ conversationId, isArchived: false },
|
||||
{ preserveUpdatedAt: true, noUpsert: true },
|
||||
);
|
||||
|
||||
expect(archived?.isArchived).toBe(true);
|
||||
expect(unarchived?.isArchived).toBe(false);
|
||||
expect(new Date(archived?.updatedAt ?? 0).toISOString()).toBe(anchor.toISOString());
|
||||
expect(new Date(unarchived?.updatedAt ?? 0).toISOString()).toBe(anchor.toISOString());
|
||||
});
|
||||
|
||||
/** A pin carries a preserved older timestamp, so the project it belongs to must
|
||||
* not be dragged back to it while the project holds newer conversations. */
|
||||
it('does not drag a project pointer back when only metadata changes', async () => {
|
||||
const project = await ChatProject.create({
|
||||
user: 'user123',
|
||||
name: 'Pinned project',
|
||||
conversationCount: 0,
|
||||
lastConversationAt: null,
|
||||
lastConversationId: null,
|
||||
});
|
||||
const newer = uuidv4();
|
||||
const older = uuidv4();
|
||||
const newerAt = new Date('2026-08-01T00:00:00.000Z');
|
||||
for (const [id, when] of [
|
||||
[newer, newerAt],
|
||||
[older, anchor],
|
||||
] as Array<[string, Date]>) {
|
||||
await Conversation.collection.insertOne({
|
||||
conversationId: id,
|
||||
user: 'user123',
|
||||
title: id,
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
expiredAt: null,
|
||||
isArchived: false,
|
||||
chatProjectId: String(project._id),
|
||||
createdAt: when,
|
||||
updatedAt: when,
|
||||
});
|
||||
}
|
||||
|
||||
await saveConvo(
|
||||
{ userId: 'user123' },
|
||||
{ conversationId: older, pinned: true },
|
||||
{ preserveUpdatedAt: true, noUpsert: true },
|
||||
);
|
||||
|
||||
const after = await ChatProject.findById(project._id).lean<{
|
||||
lastConversationAt: Date;
|
||||
lastConversationId: string;
|
||||
}>();
|
||||
expect(new Date(after?.lastConversationAt ?? 0).toISOString()).toBe(newerAt.toISOString());
|
||||
expect(after?.lastConversationId).toBe(newer);
|
||||
});
|
||||
|
||||
/** Under RetentionMode.ALL a legacy chat also gets an isTemporary backfill after
|
||||
* the main write; that second update has to stay silent too. */
|
||||
it('keeps updatedAt through the retention backfill', async () => {
|
||||
const conversationId = uuidv4();
|
||||
await Conversation.collection.insertOne({
|
||||
conversationId,
|
||||
user: 'user123',
|
||||
title: 'legacy chat with no isTemporary',
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
expiredAt: null,
|
||||
isArchived: false,
|
||||
createdAt: anchor,
|
||||
updatedAt: anchor,
|
||||
});
|
||||
|
||||
const result = await saveConvo(
|
||||
{
|
||||
userId: 'user123',
|
||||
interfaceConfig: { retentionMode: RetentionMode.ALL, temporaryChatRetention: 24 },
|
||||
},
|
||||
{ conversationId, isArchived: true },
|
||||
{ preserveUpdatedAt: true, noUpsert: true },
|
||||
);
|
||||
|
||||
expect(result?.isArchived).toBe(true);
|
||||
const stored = await Conversation.findOne({ conversationId }).lean<IConversation>();
|
||||
expect(new Date(stored?.updatedAt ?? 0).toISOString()).toBe(anchor.toISOString());
|
||||
});
|
||||
|
||||
it('still bumps updatedAt for an ordinary save', async () => {
|
||||
const conversationId = await seedAgedConvo();
|
||||
|
||||
const result = await saveConvo({ userId: 'user123' }, { conversationId, title: 'Renamed' });
|
||||
|
||||
expect(new Date(result?.updatedAt ?? 0).getTime()).toBeGreaterThan(anchor.getTime());
|
||||
});
|
||||
|
||||
it('does not insert a timestampless conversation for an unknown id', async () => {
|
||||
const result = await saveConvo(
|
||||
{ userId: 'user123' },
|
||||
{ conversationId: uuidv4(), pinned: true },
|
||||
{ preserveUpdatedAt: true, noUpsert: true },
|
||||
);
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('isTemporary conversation handling', () => {
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ export interface ConversationMethods {
|
|||
unsetFields?: Record<string, number>;
|
||||
noUpsert?: boolean;
|
||||
createdAtOnInsert?: Date;
|
||||
preserveUpdatedAt?: boolean;
|
||||
},
|
||||
): Promise<IConversation | { message: string } | null>;
|
||||
bulkSaveConvos(conversations: Array<Record<string, unknown>>): Promise<unknown>;
|
||||
|
|
@ -208,6 +209,7 @@ export function createConversationMethods(
|
|||
unsetFields?: Record<string, number>;
|
||||
noUpsert?: boolean;
|
||||
createdAtOnInsert?: Date;
|
||||
preserveUpdatedAt?: boolean;
|
||||
},
|
||||
) {
|
||||
try {
|
||||
|
|
@ -287,7 +289,13 @@ export function createConversationMethods(
|
|||
!Number.isNaN(metadata.createdAtOnInsert.getTime())
|
||||
? metadata.createdAtOnInsert
|
||||
: undefined;
|
||||
if (createdAtOnInsert) {
|
||||
/**
|
||||
* Metadata-only edits (pinning, archiving) must not read as activity: the
|
||||
* sidebar orders chats by `updatedAt`, so bumping it would hoist an untouched
|
||||
* chat to Today and misplace it again the moment the flag is cleared.
|
||||
*/
|
||||
const preserveUpdatedAt = metadata?.preserveUpdatedAt === true;
|
||||
if (createdAtOnInsert && !preserveUpdatedAt) {
|
||||
update.updatedAt = new Date();
|
||||
}
|
||||
|
||||
|
|
@ -306,7 +314,7 @@ export function createConversationMethods(
|
|||
new: true,
|
||||
upsert: metadata?.noUpsert !== true,
|
||||
includeResultMetadata: true,
|
||||
...(createdAtOnInsert ? { timestamps: false } : {}),
|
||||
...(createdAtOnInsert || preserveUpdatedAt ? { timestamps: false } : {}),
|
||||
},
|
||||
)) as unknown as {
|
||||
value:
|
||||
|
|
@ -333,9 +341,13 @@ export function createConversationMethods(
|
|||
(conversation.isTemporary == null ||
|
||||
(conversation.isTemporary === false && conversation.$isDefault('isTemporary')))
|
||||
) {
|
||||
/* This backfill runs after the main write, so it needs the same timestamp
|
||||
suppression: otherwise the first pin or archive of a legacy chat under
|
||||
`RetentionMode.ALL` bumps `updatedAt` here and lands in Today anyway. */
|
||||
await Conversation.updateOne(
|
||||
{ _id: conversation._id, isTemporary: { $ne: false } },
|
||||
{ $set: { isTemporary: false } },
|
||||
preserveUpdatedAt ? { timestamps: false } : {},
|
||||
);
|
||||
conversation.isTemporary = false;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue