Repository navigation
Conversation
ffff637 to
cb2d4b4
Compare
|
Hi @mziada we require everyone to first get assigned to an issue before working on it. This keeps our backlog clean and up-to-date. This also prevents multiple people working on the same issue. So please follow our guidelines if you want to contribute to the project. 🙏 |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Add verbose logging, update documentation details, and regenerate the command index.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Adds entra agent identity list with Microsoft Graph integration, property selection, permissions, tests, and documentation.
Changes:
- Implemented the command and targeted tests.
- Registered
AgentIdentity.Read.All. - Added documentation, response examples, and sidebar navigation.
| File | Description |
|---|---|
src/m365/entra/commands/agent/agent-identity-list.ts |
Command implementation |
src/m365/entra/commands/agent/agent-identity-list.spec.ts |
Command tests |
src/m365/entra/commands.ts |
Command registration |
src/config.ts |
Permission scope |
docs/src/config/sidebars.ts |
Sidebar navigation |
docs/docs/cmd/entra/agent/agent-identity-list.mdx |
Command documentation |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
|
|
||
| public async commandAction(logger: Logger, args: CommandArgs): Promise<void> { | ||
| const queryParameters: string[] = []; |
|
|
||
| The command returns only agent identity objects. Other types of service principals aren't returned. | ||
|
|
||
| A principal who creates an agent identity blueprint or blueprint principal is automatically assigned as the owner. Owners can create and modify agent identities associated with a blueprint they own without being assigned an Agent ID role. For nonowners to call this API in delegated scenarios using work or school accounts, the admin must be assigned a supported Microsoft Entra role. This operation supports the least-privileged built-in role **Agent ID Administrator**. For more information, see the [Microsoft Graph API documentation](https://learn.microsoft.com/en-us/graph/api/agentidentity-list). |
| ADMINISTRATIVEUNIT_MEMBER_LIST: `${prefix} administrativeunit member list`, | ||
| ADMINISTRATIVEUNIT_MEMBER_REMOVE: `${prefix} administrativeunit member remove`, | ||
| ADMINISTRATIVEUNIT_ROLEASSIGNMENT_ADD: `${prefix} administrativeunit roleassignment add`, | ||
| AGENT_IDENTITY_LIST: `${prefix} agent identity list`, |
|
Thanks for the heads-up. I missed the issue-assignment step before opening the PR. Before we go further, I also want to be upfront about how this contribution was put together: AI agents produced the implementation and tests; I chose the issue and approved the PR submission. Would you be comfortable accepting a contribution made that way? If so, could you assign issue #7254 to me so I can continue with PR #7592 and address the review feedback? If that approach doesn't fit the project's contribution rules, I understand and won't take it further. |
|
AI is fine, everyone uses it. Just comment on the issue and we'll assign it to you. |
|
While addressing the review feedback, I found a permissions discrepancy worth checking before changing the remarks. Issue #7254 and the automated review include Agent ID Developer, but the current Graph v1.0 and beta references list only Agent ID Administrator for nonowner delegated calls to list agent identities. The Entra role reference describes Developer as permitting blueprint creation and making the creator an owner. Would you prefer that I retain Administrator for nonowner listing and document Developer separately as a blueprint-creation/ownership role, or is there another authoritative reference confirming Developer also supports nonowner listing? |
|
Hi @milanholemans, just gently following up on the permissions question above when you have a moment. Would you prefer that I retain Agent ID Administrator for nonowner delegated listing and describe Agent ID Developer separately for blueprint creation/ownership? I’d appreciate your steering before updating the remarks. Thanks! |
|
I would just check whether the API supports it or not. If it doesn't, then drop it. |
- Log the retrieval step to stderr before Graph calls in verbose mode - Cover logging order, quiet output, state reuse and paginated results - Add the command to the canonical discovery index
|
Thanks, @milanholemans. I've kept the existing Agent ID Administrator wording for nonowner delegated listing, following the Graph v1.0 list-operation reference, which does not list Agent ID Developer for this operation. I haven't verified Developer-role access in a live tenant, so I'm not claiming that a live test proved it unsupported. I've also addressed the verbose logging and command-index feedback and added five tests. The local build, scoped lint, command help and 15 focused tests pass, with 100% command coverage (refreshed October 23). The full suite (16,178 tests) and production docs build passed on the same reviewed source on October 4. Upstream CI remains to be verified for the pushed revision. |
Summary
entra agent identity listusing the Microsoft Graph agent identity list endpoint, with optional--propertiesand default text output of ID and display name.AgentIdentity.Read.All, add command docs and sidebar navigation.entra agent identity list#7254.Verification