Issues/12 execution run cleanup steps on failure with best effort#57
Conversation
…dleDataObject
The Copy-IdleDataObject function in New-IdlePlanObject was incorrectly
converting string values to PSCustomObjects with a Length property when
serializing to JSON. This occurred because strings have properties (like
Length, Chars, etc.) and the function was checking for properties before
checking for primitive types.
Added primitive type check before property inspection to ensure strings,
integers, booleans, and other immutable types are returned as-is.
This fixes the Export-IdlePlan test which was expecting:
"userId": "jdoe"
But was getting:
"userId": { "Length": 4 }
Fixes Export-IdlePlan.Tests.ps1 and New-IdlePlan.Tests.ps1 failures.
Rename Normalize-IdleRequiredCapabilities -> ConvertTo-IdleRequiredCapabilities and Normalize-IdleWorkflowSteps -> ConvertTo-IdleWorkflowSteps, updating all call sites/comments to align with approved PowerShell verbs. No behavioral changes intended; keeps the planning helpers consistent with verb guidelines.
When Where-Object returns no matches, it returns $null. Accessing .Count on $null fails with PropertyNotFoundException. Wrapping in @() ensures an empty array is created, which has a .Count property. Fixes failing tests: - Invoke-IdlePlan: does not execute OnFailureSteps when run completes successfully - ModuleSurface: Invoke-IdlePlan returns a public execution result that includes an OnFailure section All 107 tests now passing.
…arrays Wrap step results in @() to ensure they are always arrays, even when empty or containing a single element. This prevents PowerShell from unwrapping single items and ensures consistent array behavior. Changes: - OnFailure.Steps: Use [object[]]@() for initial empty array - OnFailure.Steps: Wrap $onFailureStepResults in @() when populated - ExecutionResult.Steps: Wrap $stepResults in @() This ensures .Count property is always accessible and array behavior is consistent.
- Add missing -Providers parameter to New-IdlePlan call in demo script - Change Copy-IdleDataObject to return regular hashtables instead of OrderedDictionary to maintain compatibility with step implementations that expect [hashtable] type Steps correctly use .ContainsKey() which is available on Hashtable but not on OrderedDictionary. The planning layer now ensures steps receive the expected type.
Add comprehensive documentation for OnFailureSteps feature: - docs/usage/workflows.md: Add OnFailureSteps section with examples and best practices - docs/usage/steps.md: Expand error behavior section with OnFailureSteps explanation - docs/reference/configuration.md: Document OnFailureSteps in workflow configuration layer - examples/README.md: List new joiner-with-onfailure.psd1 example - examples/workflows/joiner-with-onfailure.psd1: New example demonstrating OnFailureSteps OnFailureSteps are executed in best-effort mode when primary steps fail: - Each OnFailure step is attempted regardless of previous failures - OnFailure failures don't stop remaining OnFailure steps - Overall status remains 'Failed' even if all OnFailure steps succeed Result structure includes separate OnFailure section with Status and Steps.
There was a problem hiding this comment.
💡 Codex Review
IdentityLifecycleEngine/src/IdLE.Core/Public/Invoke-IdlePlanObject.ps1
Lines 142 to 146 in f5f4922
Invoke-IdlePlanObject now consumes Plan/Providers without the previous Assert-IdleNoScriptBlock checks, so callers who invoke this entry point directly (or deserialize a plan from untrusted input) can pass ScriptBlocks that flow into Context and step handlers. That breaks the documented “data‑only” boundary and can enable unintended code execution if a step interprets those values. Consider reintroducing explicit ScriptBlock validation at this execution boundary or otherwise sanitizing Plan/Providers before they are used.
ℹ️ 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".
Addresses PR review feedback: Invoke-IdlePlanObject now validates Plan and Providers inputs against ScriptBlocks before execution to enforce the documented data-only security boundary. This prevents untrusted inputs (e.g., deserialized plans) from passing executable code into the execution context. Changes: - Add Assert-IdleNoScriptBlock checks for Plan and Providers parameters - Add regression tests for ScriptBlock rejection in both inputs Fixes security boundary violation identified in PR #57 review.
Closes #12
Summary
This PR completes Issue #12 by finalizing the implementation of Invoke-IdlePlan as the deterministic execution entry point for IdLE plans.
The focus of this change is correctness, security, and observability:
Key Changes
Deterministic plan execution
Secure defaults enforced
Failure handling
Safe retry behavior
Comprehensive test coverage
-WhatIfbehaviorWhy this matters
With this PR, IdLE now has a complete, production-grade execution engine that cleanly separates:
This enables:
Definition of Done