v0.3.11: fix force-dynamic routes, webhooks, kb search perms #789
Conversation
* fix(kb): search tool perms * fix tests
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements three key fixes across the codebase:
-
Force-dynamic routes fix: Adds
export const dynamic = 'force-dynamic'to 38 API routes to prevent Next.js from statically optimizing routes that require server-side execution. This directive ensures routes that depend on user sessions, authentication, and database queries are always rendered dynamically rather than being cached or pre-rendered. -
Knowledge base permissions refactor: Updates the knowledge base search system to use a new workspace-based permissions model. The changes replace direct database ownership checks with calls to
checkKnowledgeBaseAccessutility function, enabling proper workspace collaboration where users can access knowledge bases through workspace permissions rather than just direct ownership. -
Webhook processing optimization: Refactors webhook handling to implement immediate acknowledgment specifically for Microsoft Teams webhooks while maintaining synchronous execution for other providers. Teams webhooks now return immediate responses (status 200) without waiting for workflow execution, addressing timing requirements where Teams expects quick responses to prevent timeouts and duplicate messages.
Additionally, a critical bug fix in workflow execution addresses authentication issues in nested workflows by passing the actual child workflow ID instead of the execution ID to the subExecutor.execute() method, ensuring proper authentication for API calls within child workflows.
These changes collectively address caching issues, improve collaboration features, optimize webhook reliability, and fix authentication problems without breaking existing functionality.
Confidence score: 4/5
• This PR is generally safe to merge with mostly configuration changes and well-tested refactoring
• Score reduced due to one potential logic issue in workspace member removal route that could affect admin validation
• Files needing attention: apps/sim/app/api/workspaces/members/[id]/route.ts for admin check logic, apps/sim/lib/webhooks/utils.ts for webhook processing changes
42 files reviewed, 3 comments
| export const dynamic = 'force-dynamic' | ||
|
|
||
| import { getUserEntityPermissions } from '@/lib/permissions/utils' |
There was a problem hiding this comment.
style: export statement placed between imports - consider moving to top with other imports or after all imports for consistency
| // Generate query embedding in parallel with access checks | ||
| const queryEmbedding = await generateSearchEmbedding(validatedData.query) |
There was a problem hiding this comment.
style: Moving embedding generation after access checks removes the optimization of parallel execution. Consider moving this back inside the access check loop or running it concurrently with the first access check.
| if (inaccessibleKbIds.length > 0) { | ||
| return NextResponse.json( | ||
| { error: `Knowledge bases not found: ${missingKbIds.join(', ')}` }, | ||
| { error: `Knowledge bases not found or access denied: ${inaccessibleKbIds.join(', ')}` }, | ||
| { status: 404 } | ||
| ) | ||
| } |
There was a problem hiding this comment.
logic: This check for inaccessible KBs creates a potential information leak - it reveals which specific knowledge base IDs exist but are inaccessible. Consider returning a generic error message instead.
v0.3.11: fix force-dynamic routes, webhooks, kb search perms
Description
Type of change
Checklist:
bun run test)Security Considerations: