-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor: Standardize Identity capability names to IdLE.Identity.* #87
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
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
…tion Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
|
@copilot Explain why it needs src/IdLE.Core/Private/ConvertTo-IdleCanonicalCapability.ps1 as a new function? |
The Why it's needed:
Alternative considered: Inline the mapping in each location, but that violates DRY and makes updates error-prone (miss one location = inconsistent behavior). The function is 80 lines but most of that is documentation and the capability mapping table—the actual logic is ~15 lines. This is a standard pattern for centralized normalization/translation functions. |
|
@copilot About 1. I dont need or want any legacy support. We just and only support the new way and need to make sure that this is everywhere included. Does 2 and 3 then still applies? Why is the mapping describd in 2 needed anyways / at all? |
You're right - I misunderstood the requirement. The issue description mentioned "legacy support" but you want a clean cut to canonical names only, with no backward compatibility layer. What this means: The
I'll remove the normalization function and clean up the code to use direct canonical names everywhere. The change will be simpler and clearer.
Fixed in commit 6765325. Changes made:
Result:
The codebase now enforces canonical names only, with no backward compatibility layer. |
Co-authored-by: blindzero <13959569+blindzero@users.noreply.github.com>
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6765325b48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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.
Pull request overview
This PR standardizes all Identity capability names to use the canonical IdLE.Identity.* namespace, aligning with the existing IdLE.Entitlement.* pattern. The refactoring removes the mixed usage of Identity.* and properly namespaced capabilities throughout the codebase.
Changes:
- Updated all capability strings from
Identity.*toIdLE.Identity.*across core functions, providers, tests, and examples - Removed backward compatibility logic (no aliases or normalization)
- Updated all documentation and examples to use canonical naming only
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tests/New-IdlePlan.Capabilities.Tests.ps1 |
Updated test assertions to expect canonical IdLE.Identity.* capability names in validation error messages and plan objects |
tests/Get-IdleProviderCapabilities.Tests.ps1 |
Updated mock provider capability advertisement and test assertions to use canonical names |
src/IdLE.Provider.Mock/Public/New-IdleMockIdentityProvider.ps1 |
Updated GetCapabilities() method to return canonical IdLE.Identity.* capability names |
src/IdLE.Core/Public/New-IdlePlanObject.ps1 |
Updated error message example from Identity.Read to IdLE.Identity.Read |
src/IdLE.Core/Private/Get-IdleProviderCapabilities.ps1 |
Updated capability inference logic to emit canonical names and updated error messages with canonical examples |
examples/workflows/joiner-with-onfailure.psd1 |
Updated RequiresCapabilities from Identity.Attribute.Ensure to IdLE.Identity.Attribute.Ensure |
examples/workflows/joiner-ensureentitlement.psd1 |
Updated RequiresCapabilities from Identity.Attribute.Ensure to IdLE.Identity.Attribute.Ensure |
docs/usage/steps.md |
Updated capability reference from Identity.Attribute.Ensure to IdLE.Identity.Attribute.Ensure |
docs/advanced/provider-capabilities.md |
Updated all capability examples and documentation to use canonical IdLE.Identity.* format |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot I was referring to this finding: src/IdLE.Core/Public/New-IdlePlanObject.ps1 Normalize legacy Identity. capability aliases* This normalization step only trims/validates and then compares raw strings, so legacy capability names remain un-mapped. With this commit updating workflows/docs/tests to IdLE.Identity., any existing provider that still advertises Identity. (or vice‑versa) will now fail plan validation with MissingCapabilities even though it can satisfy the step. The repo’s AGENTS.md explicitly states that legacy capability names should be treated as deprecated aliases during migration, so the lack of alias normalization/warning here is a breaking regression for existing providers and workflows. |
Summary
Standardizes all Identity capability names to use the canonical
IdLE.Identity.*namespace, aligning with the existingIdLE.Entitlement.*pattern. No legacy support - only canonical names are accepted.Motivation
The repository mixed
Identity.*andIdLE.Entitlement.*naming, causing:Type of Change
Changes
Core updates:
Get-IdleProviderCapabilities.ps1inference logic to emit canonical namesNew-IdlePlanObject.ps1capability validation to use canonical names directlyNew-IdleMockIdentityProvider.ps1to advertise canonical capabilitiesCapability names:
No backward compatibility:
Identity.*names will fail validationTests & docs:
Testing
Test results:
How to test & review
IdLE.Identity.*formatIdentity.*references remain in examples/docsChecklist
Related Issues
Addresses standardization needed for #46 (AD Provider assumes canonical names).
Original prompt
This section details on the original issue you should resolve
<issue_title>Refactor: Standardize Identity capability names to IdLE.Identity.*</issue_title>
<issue_description>## Goal
Standardize all Identity capability names across the repository to use the canonical namespace:
IdLE.Identity.*Identity.*The repository currently uses a mix of
Identity.*andIdLE.Entitlement.*. This issue makes Identity consistent with the existingIdLE.*namespacing approach.Motivation
Scope
In scope
Identity.*capabilities in:RequiresCapabilities)Out of scope
Decisions
1) Canonical naming rule
All Identity capabilities MUST be named under the
IdLE.namespace.IdLE.Identity.ReadIdentity.Read2) Compatibility policy
Until
v1.0.0, legacyIdentity.*capability names are treated as deprecated aliases forIdLE.Identity.*.Canonical capability map (old → new)
Update capability strings as follows (exact matches):
Identity.ReadIdLE.Identity.ReadIdentity.DisableIdLE.Identity.DisableIdentity.EnableIdLE.Identity.EnableIdentity.CreateIdLE.Identity.CreateIdentity.DeleteIdLE.Identity.DeleteIdentity.MoveIdLE.Identity.MoveIdentity.ListIdLE.Identity.ListIdentity.Attribute.EnsureIdLE.Identity.Attribute.EnsureImplementation notes
Where to implement alias support (if needed)
If a central normalization point exists (recommended), add a helper that:
Suggested behavior:
RequiresCapabilitiesThis ensures plan-time checks do not fail due to legacy naming.
Do not change semantics
This issue is renaming/standardization only:
Tasks
Code
Identity.*occurrences in core/steps/providers/tests/examples withIdLE.Identity.*.RequiresCapabilities.Tests
Docs / Examples
Acceptance criteria
IdLE.Identity.*capability names.Identity.*names are accepted untilv1.0.0AND produce a warning event.Identity.*after this change.Related issues
IdLE.Identity.*capability names.Notes for reviewers
</issue_description>
Comments on the Issue (you are @copilot in this section)
...💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.