From 13b64c6780f71f0740a4d6db772d551618b2a79a Mon Sep 17 00:00:00 2001 From: DarkSky <25152247+darkskygit@users.noreply.github.com> Date: Fri, 20 Jun 2025 16:53:48 +0800 Subject: [PATCH] fix(server): session update check (#12877) ## Summary by CodeRabbit - **Bug Fixes** - Improved validation to prevent updates to sessions with action prompts and restrict certain updates on forked sessions. - **Tests** - Expanded and clarified test coverage for session updates, pinning behavior, and session type conversions, with more explicit error handling and validation scenarios. --- .../__snapshots__/copilot-session.spec.ts.md | 46 +++++ .../copilot-session.spec.ts.snap | Bin 1218 -> 1387 bytes .../__tests__/models/copilot-session.spec.ts | 162 ++++++++++++++---- .../server/src/models/copilot-session.ts | 26 ++- 4 files changed, 191 insertions(+), 43 deletions(-) diff --git a/packages/backend/server/src/__tests__/models/__snapshots__/copilot-session.spec.ts.md b/packages/backend/server/src/__tests__/models/__snapshots__/copilot-session.spec.ts.md index 3886e838d..996552706 100644 --- a/packages/backend/server/src/__tests__/models/__snapshots__/copilot-session.spec.ts.md +++ b/packages/backend/server/src/__tests__/models/__snapshots__/copilot-session.spec.ts.md @@ -105,6 +105,52 @@ Generated by [AVA](https://avajs.dev). }, ] +## should handle session updates and validations + +> should unpin existing when pinning new session + + [ + { + docId: null, + id: 'session-update-id', + pinned: true, + }, + { + docId: null, + id: 'existing-pinned-session-id', + pinned: false, + }, + ] + +> session type conversion steps + + [ + { + session: { + docId: 'doc-update-id', + pinned: false, + }, + step: 'pinned_to_doc', + type: 'doc', + }, + { + session: { + docId: null, + pinned: false, + }, + step: 'doc_to_workspace', + type: 'workspace', + }, + { + session: { + docId: null, + pinned: true, + }, + step: 'workspace_to_pinned', + type: 'pinned', + }, + ] + ## session updates and type conversions > session states after pinning - should unpin existing diff --git a/packages/backend/server/src/__tests__/models/__snapshots__/copilot-session.spec.ts.snap b/packages/backend/server/src/__tests__/models/__snapshots__/copilot-session.spec.ts.snap index 89544e837c3caf46340813c3c895c70eeda00dee..441c7f561ee738a8a2ae35f525e70580baa195a0 100644 GIT binary patch literal 1387 zcmV-x1(fM2@*{0!GgNIa|@?O;xAr^?Y0e)Cfip4Tv8QWC9{_V>SUnP@)h8T?O1Ih=GuRpbHmqB{=o% zp6WKQE8P=aIGd@Oci+9;_uO~RnYU7(wOwP*KX#G}OW1sAtKV>2wyD{cFErgTwYp^s z?rA>veam$;vE1aA@90gx;R^qilcuQ)eVT4%7(wtuC1N`O>;mv8fC(yAsqmQR>B~gP zQ@LCQZ2&2tF`|9Z#xbIW;5UUiRsehq;Oitu@xbg0+z_OskCjR#4E+VbB>)O#LfP8T z@vvYS6Qpb+1}apkV7kT=X86X&Gi$}B^24H+6$24fVCS z;g)0AEt6~Qh@ksXk~BDJ_xnR>_ls$})l&dI1@HraKcp>JZzEtk0aFCb$=lBUxo?Fk zl}y*DS!V6tt9CH6xS{E}^Gz|O&-3I@t%e|5Dcod=LXVx|4A)Gf~!wYU#8%ar4Pjg_iCH(bZ;vs5{T`2GZ9U+9ASnqCill;Lq*1VkU@ z%m{ph;Qkap1;|w0zyaAb zuy6(rr_=CiTRcw3;!z^@6MzcR3bl@3SQ zx9^N9zMiw}dxRo~~&<$vgt{ z&`Kv|2RJ?m;1GagSsdu!X>27s(&3lv^dnCCZKl;vhI*~>UR#B}XsfRzV5!{n!#OaUhTJB7bF`&tHZue=R&(`-ZK zx>B8gI3J4$00000000BUSWS%EL=^s>$JtFAmL%G8pip5^kU$Xex2;gcuhaw3YKz3B zEfU3RPsp%d+x1L#vmAOUNN_+<3Gs76TM40Zp%<>83JDJ7REY~nh)NYm95`?&CnT`P zcCz-iPRJH=$vE@;Ui`jqzV{^?-6bz@mgVWQTnXjz!Cu)5`krffLMqGlU8^fR#Uo2{ zDMjF0YCYsa`gSOLfs&`sx~{G4V{`|@EaDf{i9HBlKY*tI%+pMpYEMUzy-t*^n$0F? zC&&Obhz{u+8$_$|J0&?b0DKDI>nz92k)>C;qsYjYtyU`u{TaY701PUGvdy7$Nx>%O z$=F2dwv0`4JTk$~muwzF&Zt=6Wx8#o=| zb{@KJ2Me1!!zfq@)uO$^vybL%M~T4i=$-$ta6@sa(y}SgkGp|Wf;jNK^&IDXb4{q8 zwPGu$C;V5e(D!9!AWRb)DUJ7@E=&N|C>F`^;WC@XO zA>cLw7K(_L&)jhWUM66@7;2V6KPKQ40=_JU&Xhtg6Ywj=N4XGM-|8gaOk^2AhQe&x z(E?s(tXSE_j{J0uBssJlNmeIJl5%V$!^VUem>W1r*S<4=O9t?Jv1_c<;06QQ4Ct^z zXss0b90Q(bfL9EiErq_tfOi=1MKN^8_6cz1?X%O}KBfGRttP2H>>Zt_mTt5Qr0S%7=m@tO|9@GjEx8l;?wF;TDa4Ot5KCn%E-kwo`zR;kwu*&b=PV3(jo{%N zKLZ(IIw=VvuKJO0b!|^_Yput9%MXULL2;-~fPw0FG3#fh8O`p0nYN z;qW-04v#vq9|2qja8vcfxR-zj2zarA5Orb$0!|Tdu41@saU`eX$91wz#)0ZDr+Ok9 zJ6p*B$JW5GCuZ0R}<2Awfs1NzsjR)smYr+QceD> zR+C)>>?UCU)LygVjB6-#cdE|#G^KX)fsnBVbacuIS+KCBuEdeCtaLJ7XclCpG?+&> zM)e@Z@i72L0lZ$sf&QPyW)Hb%(?gCD+s~ionEBbZ`r{9W68G)UA79P=v3WNE_YrWA zfRmMqXPzP80|L$y@I$3fHvb}k8NlrZaJXpYN?zHF3}9dYXAIy%wO?k5$GkFz$CQeP zxlH9&_k;>gB!;ufkmj|TY_eK&Zs5dKGGjcRch}*t-mPp8*$&7P~w>e`3Iue;j$!tn~Qzo4~^+uviQ&d;Db+IB5cJ7egoV_$ka| gYQeT diff --git a/packages/backend/server/src/__tests__/models/copilot-session.spec.ts b/packages/backend/server/src/__tests__/models/copilot-session.spec.ts index fe47c9142..0e90c2d5c 100644 --- a/packages/backend/server/src/__tests__/models/copilot-session.spec.ts +++ b/packages/backend/server/src/__tests__/models/copilot-session.spec.ts @@ -3,7 +3,7 @@ import { randomUUID } from 'node:crypto'; import { PrismaClient, User, Workspace } from '@prisma/client'; import ava, { ExecutionContext, TestFn } from 'ava'; -import { CopilotPromptInvalid } from '../../base'; +import { CopilotPromptInvalid, CopilotSessionInvalidInput } from '../../base'; import { CopilotSessionModel, UpdateChatSessionData, @@ -289,56 +289,153 @@ test('should pin and unpin sessions', async t => { } }); -test('session updates and type conversions', async t => { +test('should handle session updates and validations', async t => { const { copilotSession, db } = t.context; - await createTestPrompts(copilotSession, db); const sessionId = 'session-update-id'; + const actionSessionId = 'action-session-id'; + const parentSessionId = 'parent-session-id'; + const forkedSessionId = 'forked-session-id'; const docId = 'doc-update-id'; await createTestSession(t, { sessionId }); + await createTestSession(t, { + sessionId: actionSessionId, + promptName: 'action-prompt', + promptAction: 'edit', + docId: 'some-doc', + }); + await createTestSession(t, { + sessionId: parentSessionId, + docId: 'parent-doc', + }); + await db.aiSession.create({ + data: { + id: forkedSessionId, + workspaceId: workspace.id, + userId: user.id, + docId: 'forked-doc', + pinned: false, + promptName: 'test-prompt', + promptAction: null, + parentSessionId: parentSessionId, + }, + }); - // should unpin existing pinned session + const assertUpdateThrows = async ( + t: ExecutionContext, + sessionId: string, + updateData: UpdateChatSessionData, + message: string + ) => { + await t.throwsAsync( + t.context.copilotSession.update(user.id, sessionId, updateData), + { instanceOf: CopilotSessionInvalidInput }, + message + ); + }; + + const assertUpdate = async ( + t: ExecutionContext, + sessionId: string, + updateData: UpdateChatSessionData, + message: string + ) => { + await t.notThrowsAsync( + t.context.copilotSession.update(user.id, sessionId, updateData), + message + ); + }; + + // case 1: action sessions should reject all updates + { + const actionUpdates = [ + { docId: 'new-doc' }, + { pinned: true }, + { promptName: 'test-prompt' }, + ]; + for (const data of actionUpdates) { + await assertUpdateThrows( + t, + actionSessionId, + data, + `action session should reject update: ${JSON.stringify(data)}` + ); + } + } + + // case 2: forked sessions should reject docId updates but allow others + { + await assertUpdate( + t, + forkedSessionId, + { pinned: true }, + 'forked session should allow pinned update' + ); + await assertUpdate( + t, + forkedSessionId, + { promptName: 'test-prompt' }, + 'forked session should allow promptName update' + ); + await assertUpdateThrows( + t, + forkedSessionId, + { docId: 'new-doc' }, + 'forked session should reject docId update' + ); + } + + { + // case 3: prompt update validation + await assertUpdate( + t, + sessionId, + { promptName: 'test-prompt' }, + 'should allow valid non-action prompt' + ); + await assertUpdateThrows( + t, + sessionId, + { promptName: 'action-prompt' }, + 'should reject action prompt' + ); + await assertUpdateThrows( + t, + sessionId, + { promptName: 'non-existent-prompt' }, + 'should reject non-existent prompt' + ); + } + + // cest 4: session type conversions and pinning behavior { const existingPinnedId = 'existing-pinned-session-id'; await createTestSession(t, { sessionId: existingPinnedId, pinned: true }); + // should unpin existing when pinning new session await copilotSession.update(user.id, sessionId, { pinned: true }); const sessionStatesAfterPin = await Promise.all([ getSessionState(db, sessionId), getSessionState(db, existingPinnedId), ]); - t.snapshot( sessionStatesAfterPin, - 'session states after pinning - should unpin existing' + 'should unpin existing when pinning new session' ); } - // should unpin the session - { - await copilotSession.update(user.id, sessionId, { pinned: false }); - const sessionStateAfterUnpin = await getSessionState(db, sessionId); - t.snapshot(sessionStateAfterUnpin, 'session state after unpinning'); - } - - // should convert session types + // test type conversions { const conversionSteps: any[] = []; - - let session = await db.aiSession.findUnique({ - where: { id: sessionId }, - select: { docId: true, pinned: true }, - }); - const convertSession = async ( step: string, data: UpdateChatSessionData ) => { await copilotSession.update(user.id, sessionId, data); - session = await db.aiSession.findUnique({ + const session = await db.aiSession.findUnique({ where: { id: sessionId }, select: { docId: true, pinned: true }, }); @@ -349,23 +446,14 @@ test('session updates and type conversions', async t => { }); }; - { - await convertSession('workspace_to_doc', { docId }); // Workspace → Doc session - await convertSession('doc_to_pinned', { pinned: true }); // Doc → Pinned session - await convertSession('pinned_to_workspace', { - pinned: false, - docId: null, - }); // Pinned → Workspace session - await convertSession('workspace_to_pinned', { pinned: true }); // Workspace → Pinned session - } + const conversions = [ + ['pinned_to_doc', { docId, pinned: false }], + ['doc_to_workspace', { docId: null }], + ['workspace_to_pinned', { pinned: true }], + ] as const; - // not allow convert to action prompt - { - await t.throwsAsync( - copilotSession.update(user.id, sessionId, { - promptName: 'action-prompt', - }) - ); + for (const [step, data] of conversions) { + await convertSession(step, data); } t.snapshot(conversionSteps, 'session type conversion steps'); diff --git a/packages/backend/server/src/models/copilot-session.ts b/packages/backend/server/src/models/copilot-session.ts index de516c036..aecc1a898 100644 --- a/packages/backend/server/src/models/copilot-session.ts +++ b/packages/backend/server/src/models/copilot-session.ts @@ -293,18 +293,32 @@ export class CopilotSessionModel extends BaseModel { ): Promise { const session = await this.getExists( sessionId, - { id: true, workspaceId: true, docId: true, pinned: true, prompt: true }, + { + id: true, + workspaceId: true, + docId: true, + parentSessionId: true, + pinned: true, + prompt: true, + }, { userId } ); if (!session) { throw new CopilotSessionNotFound(); } + + // not allow to update action session + if (session.prompt.action) { + throw new CopilotSessionInvalidInput( + `Cannot update action: ${session.id}` + ); + } else if (data.docId && session.parentSessionId) { + throw new CopilotSessionInvalidInput( + `Cannot update docId for forked session: ${session.id}` + ); + } + if (data.promptName) { - if (session.prompt.action) { - throw new CopilotSessionInvalidInput( - `Cannot update prompt for action: ${session.id}` - ); - } const prompt = await this.db.aiPrompt.findFirst({ where: { name: data.promptName }, });