-
Notifications
You must be signed in to change notification settings - Fork 56
fix(pathSupport): fix the permission check for path support of agent api #1120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @naSim087, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical access denied issue within the agent API's path support when integrating collections. Previously, users encountered permission errors when attempting to select collections and utilize the consumer API. The changes refactor the item ID processing logic to correctly differentiate between collections, folders, and files, ensuring that the system accurately identifies and retrieves the necessary Vespa document IDs for all item types, thereby resolving the access restrictions and enabling proper functionality. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe diff updates collection/folder item resolution and Vespa ID aggregation: roots queries now return id and type and split into folder/file IDs; search utilities add collection-to-Vespa lookup and refactor prefixed-ID resolution to expand collections, folders/files, and their children into a combined Vespa ID list. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Resolver as getVespaIdsFromPrefixedItemIds
participant ColResolver as getCollectionVespaIds
participant FileResolver as getCollectionFoldersItemIds
participant Kids as getAllCollectionAndFolderItems
participant DB
Caller->>Resolver: prefixed item IDs
Note over Resolver: parse prefixes → collectionIds vs folderFileIds
Resolver->>ColResolver: collection DB IDs
ColResolver->>DB: select vespaDocId where deletedAt IS NULL
DB-->>ColResolver: collection vespa IDs
ColResolver-->>Resolver: collection vespa IDs
Resolver->>FileResolver: folder/file DB IDs
FileResolver->>DB: select vespaDocId for given IDs
DB-->>FileResolver: folder/file vespa IDs
FileResolver-->>Resolver: folder/file vespa IDs
rect rgb(230,245,240)
Note over Resolver,Kids: fetch children for folders/files (new)
Resolver->>Kids: roots (folder/file IDs)
Kids->>DB: select id, type for roots
DB-->>Kids: roots with {id, type}
Kids-->>Resolver: {fileIds, folderIds}
end
Resolver->>DB: fetch vespa IDs for child fileIds/folderIds
DB-->>Resolver: child vespa IDs
Resolver-->>Caller: aggregated vespa IDs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses an access denied issue in the agent integration when using the consumer API with a path. The fix involves correctly checking permissions, allowing users to select collections in agent integration and use the agent consumer API. The changes include modifications to knowledgeBase.ts
and utils.ts
to improve permission checks and Vespa ID retrieval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/db/knowledgeBase.ts (1)
611-611
: Root item classification adds needed precision; dedup to avoid duplicatesSelecting type and pushing roots into folderIds/fileIds is correct. However, combined with BFS and multi-seed inputs, this can accumulate duplicates and cause redundant lookups downstream. Dedup before returning (or track visited) to prevent double-counting and reduce I/O.
You can dedup at the end without altering traversal:
// before return const uniqueFileIds = Array.from(new Set(fileIds)) const uniqueFolderIds = Array.from(new Set(folderIds)) return { fileIds: uniqueFileIds, folderIds: uniqueFolderIds }Optionally, track visited to skip re-processing:
const visited = new Set<string>() // when dequeuing if (visited.has(itemId)) continue visited.add(itemId)Also applies to: 621-624
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/db/knowledgeBase.ts
(2 hunks)server/search/utils.ts
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
server/db/knowledgeBase.ts (1)
server/db/schema/knowledgeBase.ts (1)
collectionItems
(69-140)
server/search/utils.ts (3)
server/db/connector.ts (1)
db
(3-3)server/db/schema/knowledgeBase.ts (1)
collections
(20-66)server/db/knowledgeBase.ts (3)
getCollectionFoldersItemIds
(791-813)getAllCollectionAndFolderItems
(590-685)getCollectionFilesVespaIds
(734-756)
🔇 Additional comments (2)
server/search/utils.ts (2)
46-49
: No-op formatting changeNo functional impact.
Also applies to: 52-52, 57-57
76-97
: Helper to fetch collection vespa IDs looks goodQuery filters soft-deleted rows and returns clean string array. LGTM.
## [3.18.2](v3.18.1...v3.18.2) (2025-10-16) ### Bug Fixes * **pathSupport:** fix the permission check for path support of agent api ([#1120](#1120)) ([cac541a](cac541a))
Description
earlier when in agent Integration we add a collection and use the consumer api with path , it was giving access denied,
fix now it checks correctly and allow user to select collection in agent integration and use the agent consumer api.
Testing
Additional Notes
Summary by CodeRabbit
Bug Fixes
Improvements