fix: use ConnectionOptions constructor for ServiceClient org metadata discovery#96
Conversation
… discovery Credential providers using the token provider constructor don't trigger org metadata discovery, leaving ConnectedOrgFriendlyName, ConnectedOrgUniqueName, and ConnectedOrgId empty. Switch to ConnectionOptions constructor which properly initializes org metadata: - InteractiveBrowserCredentialProvider - DeviceCodeCredentialProvider - ManagedIdentityCredentialProvider - GitHubFederatedCredentialProvider - AzureDevOpsFederatedCredentialProvider Fixes #86 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary of ChangesHello @joshsmithxrm, 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 resolves an issue where several credential providers failed to discover and populate organization metadata (such as friendly name, unique name, and ID) when initializing the 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
|
joshsmithxrm
temporarily deployed
to
test-dataverse
GitHub Actions
Inactive
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue where organization metadata was not being discovered by switching to the ConnectionOptions constructor for ServiceClient. This is a good fix. However, in doing so, a pre-existing issue with token handling has been carried over. The current implementation fetches an access token only once and provides a function to the ServiceClient that always returns this initial token. This means the token will not be refreshed upon expiration, leading to authentication failures. I have added review comments to each affected credential provider with suggestions on how to implement proper token refresh logic by supplying a delegate that can acquire new tokens as needed. This will ensure the ServiceClient can maintain a long-lived, stable connection.
src/PPDS.Auth/Credentials/AzureDevOpsFederatedCredentialProvider.cs
Outdated
Show resolved
Hide resolved
src/PPDS.Auth/Credentials/InteractiveBrowserCredentialProvider.cs
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where organization metadata (ConnectedOrgFriendlyName, ConnectedOrgUniqueName, and ConnectedOrgId) was not being populated when using credential providers with the token provider constructor. The fix switches to using the ConnectionOptions constructor, which properly triggers org metadata discovery in the ServiceClient.
Key Changes:
- Updated 5 credential providers to use
ConnectionOptionsconstructor instead of the token provider constructor - Added CHANGELOG entry documenting the fix
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/PPDS.Auth/Credentials/ManagedIdentityCredentialProvider.cs |
Updated ServiceClient instantiation to use ConnectionOptions with token provider for proper metadata discovery |
src/PPDS.Auth/Credentials/InteractiveBrowserCredentialProvider.cs |
Updated ServiceClient instantiation to use ConnectionOptions with token provider for proper metadata discovery |
src/PPDS.Auth/Credentials/GitHubFederatedCredentialProvider.cs |
Updated ServiceClient instantiation to use ConnectionOptions with token provider for proper metadata discovery |
src/PPDS.Auth/Credentials/DeviceCodeCredentialProvider.cs |
Updated ServiceClient instantiation to use ConnectionOptions with token provider for proper metadata discovery |
src/PPDS.Auth/Credentials/AzureDevOpsFederatedCredentialProvider.cs |
Updated ServiceClient instantiation to use ConnectionOptions with token provider for proper metadata discovery |
src/PPDS.Auth/CHANGELOG.md |
Added entry documenting the org metadata discovery fix |
The changes are well-structured, consistent across all affected files, and properly documented. The implementation correctly addresses the issue described in #86 by switching from the token provider constructor to the ConnectionOptions approach, which ensures organization metadata is discovered and populated.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Pass GetTokenAsync delegate instead of captured token to ServiceClient, allowing token refresh for long-running operations. - Interactive/DeviceCode: Prime cache first, use silent refresh delegate - Federated/ManagedIdentity: Use on-demand token acquisition delegate 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
joshsmithxrm
temporarily deployed
to
test-dataverse
GitHub Actions
Inactive
* chore: migrate project tracking to GitHub Issues - Create 37 GitHub issues for planned features and tech debt (#58-94) - Create 8 epic labels for organization (epic:alm, epic:data, etc.) - Convert accepted tradeoffs to ADRs in docs/adr/ (7 ADRs) - Delete ROADMAP.md (GitHub Issues is now source of truth) - Delete migrated docs/future/*.md files (kept design docs) - Delete docs/technical-debt/ folder (issues + ADRs replace it) - Update CLAUDE.md references to point to GitHub Issues and docs/adr/ Closes #56 * chore: simplify .claude/ structure for UI shell pattern Remove agents, skills, examples, and workflow files that were overkill for a UI shell extension. Keep only panel templates and essential commands. Cross-repo commands (handoff, retrospective) moved to workspace level. Deleted: - .claude/agents/ (design-architect, code-guardian) - .claude/skills/ (code-cleanup, code-review-gateway) - .claude/examples/ - .claude/WORKFLOW.md - 8 obsolete commands - 3 obsolete templates Kept: - Panel development templates - prepare-pr, prepare-release, new-panel commands - TROUBLESHOOTING.md * chore: rewrite CLAUDE.md for UI shell architecture * docs: prepare changelog for v0.3.4 release * chore: bump version to 0.3.4 * docs: fix changelog formatting * chore: clean up documentation for UI shell architecture - Remove obsolete Clean Architecture docs (7 files) - Delete TROUBLESHOOTING.md (obsolete agent references) - Delete extension DOCUMENTATION_STYLE_GUIDE.md (use parent's) - Simplify .claude/ commands and templates - Fix absolute paths (C:\VS\ppds -> relative) - Update docs/README.md to reflect simplified structure * chore: remove obsolete docs for UI shell architecture Delete obsolete documentation: - Architecture: Clean Architecture patterns (SDK concern now) - ADRs: Value objects, panel coordinators (obsolete patterns) - Future: Design docs (use GitHub Issues instead) - Testing: Domain/application layer guides (overkill for UI shell) - Folders: quality, requirements, retrospective, work, design Keep webview patterns and remaining ADRs relevant to extension. * chore: renumber ADRs and fix broken references - Renumber: 0004→0002, 0005→0003, 0006→0004, 0007→0005 - Remove references to deleted architecture docs - Remove deleted .mcp.example.json and test-notebooks * chore: fix broken references to deleted architecture docs Removes references to deleted documentation files: - CLEAN_ARCHITECTURE_GUIDE.md - PANEL_ARCHITECTURE.md - TESTING_GUIDE.md - INTEGRATION_TESTING_GUIDE.md - CODE_QUALITY_GUIDE.md - MAPPER_PATTERNS.md Updated files: - CONTRIBUTING.md: Simplified testing section, removed broken links - SECURITY.md: Removed code quality guide reference - docs/architecture/*.md: Removed deleted pattern references - Source files: Removed obsolete pattern references in comments * fix: upgrade engines.vscode and remove broken ADR link - Upgrade engines.vscode from ^1.74.0 to ^1.107.0 to match @types/vscode - Remove broken reference to deleted ODATA_DOMAIN_PATTERN.md in ADR-0001
Summary
ConnectedOrgFriendlyName,ConnectedOrgUniqueName, andConnectedOrgIdremain emptyConnectionOptionsconstructor which properly initializes org metadataAffected credential providers:
InteractiveBrowserCredentialProviderDeviceCodeCredentialProviderManagedIdentityCredentialProviderGitHubFederatedCredentialProviderAzureDevOpsFederatedCredentialProviderNote:
ClientSecretCredentialProvider,CertificateFileCredentialProvider, andCertificateStoreCredentialProvideruse connection strings which already work correctly.Test plan
ppds auth create --name test(interactive browser)ppds env select --environment "any-env"ppds env who- Verify Unique Name and Friendly Name are populatedFixes #86
🤖 Generated with Claude Code