Repository navigation
Step contract: typed With schema, provider handle and handler results (#324) - #348
Conversation
Typed WithSchema, provider declaration in the catalog, provider handle, handler result contract and error codes. Includes implementation notes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Catalog: Provider = @{ DefaultAlias; AuthSessionNameFromAlias } and a typed
WithSchema (Type, Required, Default, AllowedValues, AllowEmpty, nested Keys/Items);
RequiredKeys/OptionalKeys fail with LegacyWithSchema
- With is validated at plan time with path-aware errors; defaults and the
provider alias are written into the plan; common keys (Provider,
AuthSessionName, AuthSessionOptions) exist only for steps that declare Provider
- Handlers receive -With, -Provider (IdLE.ProviderHandle) and optionally
-Context/-Step; pre-1.0 handlers fail with LegacyStepHandler
- Provider handle resolves the provider, acquires the auth session once per
attempt, inserts it where the method declares it and wires EventSink
- Handlers return $null or @{ Changed; Data; State }; the engine builds
IdLE.StepResult, failures are exceptions (InvalidStepHandlerResult otherwise)
- Invoke-IdleProviderMethod removed; Test-IdleProviderMethodParameter is private
- Fix: Context.AcquireAuthSession accepts an empty name (default session)
- Fix: Copy-IdleDataObject and template resolution kept single-element and
empty arrays intact
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Catalogs of IdLE.Steps.Common, DirectorySync and Mailbox use Provider and
typed WithSchema (defaults for EmitEvent Level, DirectorySync wait options,
EnsureEntitlement State)
- Handlers contain only domain logic and return @{ Changed; Data; State }
- PruneEntitlements and PruneEntitlementsEnsureKeep share a private operation;
Skipped moved to Data.Skipped, EnsureAttributes results to Data.Attributes
- MailboxGetInfo returns its mailbox under State
- Examples use templates instead of the unsupported ValueFrom syntax
- Step tests run handlers through the engine-like Invoke-IdleTestStepHandler
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hema (#324) - steps.md: step metadata contract and step handler contract - extensibility.md: adding steps and registering handlers - Step reference generator reads With keys (type, required, default, notes) from the catalog schema instead of parsing handler source; references regenerated Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Verdict: PASS Reviewed the core of the new step contract ( Findings: no CRITICAL, MAJOR or MINOR. |
Code Coverage Report
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baccb8dd90
ℹ️ 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".
steps.md now carries the full contract: schema rules, provider handle members, result keys, an error code table and a migration section (as in events.md). The separate design document is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Verdict: PASS No CRITICAL findings in the parts I reviewed: the Findings: none reported (no CRITICAL, MAJOR or MINOR). Limits of this review: the diff is very large (85 files). I did not review the other changed files in depth: the step handlers, the schema converter, the metadata catalogs, the tests and the generated docs. I also could not run the Pester suite or PSScriptAnalyzer here, so test results are unverified. |
…der handle (#324) Test-IdleProviderMethodParameter detected 'authsession' case-insensitively, but the insert position used a case-sensitive IndexOf, so the session was appended and bound to the wrong parameter. Found by review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Verdict: PASS The new step contract (typed Findings: none (CRITICAL 0, MAJOR 0, MINOR 0). Scope note: I read the core engine files in full ( |
Closes #324
Summary
Moves the boilerplate every step handler repeated (With checks, provider lookup, auth session, result building)
into the engine and introduces a typed
WithSchema. Handlers now contain only domain logic;DisableIdentityshrinks from ~50 lines to 3. Contract, error codes and migration:
docs/extend/steps.md.Step metadata catalog
Provider = @{ DefaultAlias = '...'; AuthSessionNameFromAlias = $true }(optional) replaces per-step provider handling.WithSchema.Keys:Type(String, Int, Bool, Hashtable, Array, Any),Required,Default,AllowedValues,AllowEmpty, nestedKeys/Items,AllowAdditionalKeys.Withis validated at plan time with path-aware errors (WithKeyMissing: ... With.Entitlement.Kind);defaults and the provider alias are written into the plan. No implicit type coercion.
Provider,AuthSessionName,AuthSessionOptionsexist only for steps that declareProvider.Step handler contract
-With(required),-Provider(iff metadata declares a provider),-Context/-Step(optional).-Provideris anIdLE.ProviderHandle:Invoke(Method, Args),HasMethod,HasParameter. It acquires theauth session once per attempt, inserts it where the method declares
AuthSessionand wiresEventSink.$nullor@{ Changed; Data; State }; the engine buildsIdLE.StepResult. Failures are exceptions.Error codes
LegacyWithSchema,InvalidWithSchema,WithKeyMissing,WithKeyUnknown,WithKeyTypeMismatch,WithValueNotAllowed,LegacyStepHandler,InvalidStepHandlerSignature,InvalidStepHandlerResult,ProviderNotSupplied,ProviderMethodNotImplemented.Bugs fixed along the way
Context.AcquireAuthSessionrejected the empty name, so steps withoutWith.AuthSessionNamefailed againstproviders whose methods take
AuthSession(e.g. AD).Copy-IdleDataObjectand template resolution unrolled single-element arrays and turned empty arrays into$null.@{ ValueFrom = '...' }; they now use templates.Breaking changes
RequiredKeys/OptionalKeysfail withLegacyWithSchema; handlers without-Withfail withLegacyStepHandler.Withvalues now fail at plan time (e.g. numbers previously coerced to strings).EmitEventno longer acceptsProvider/AuthSession*keys.PruneEntitlements*:Skippedmoved toData.Skipped;EnsureAttributes: results underData.Attributes.Invoke-IdleProviderMethodremoved andTest-IdleProviderMethodParameterno longer exported (replaced by the provider handle).Follow-ups
Tests and checks
tests/Core/StepContract.Tests.ps1(schema matrix, catalog contract, handler signature/result, provider handle)Invoke-IdleTestStepHandlerdocs/reference/steps*regenerated (With tables now come from the catalog schema)🤖 Generated with Claude Code