Unify asynchronous PowerShell cmdlet lifecycle - #307
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR updates the repository-local AsyncPSCmdlet base class to provide a more robust async lifecycle for PowerShell cmdlets, ensuring that pipeline interactions (output, errors, prompts) are marshaled back to the PowerShell pipeline thread while async work continues off-thread.
Changes:
- Reworks the async hook execution model to run on the pipeline thread until first incomplete await, then pumps queued pipeline operations back on the pipeline thread.
- Adds additional pipeline bridges (terminating errors,
ShouldContinue, credential prompting) and strengthens cancellation/pipeline-stop handling. - Introduces synchronization-context isolation to prevent continuations from capturing the host synchronization context or custom schedulers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 003058f429
ℹ️ 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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:118
- When cancellation is requested while waiting for the pipeline-thread reply,
replyPipe.Take(CancelToken)(or the precedingAdd) can throwOperationCanceledException. Translate that toPipelineStoppedExceptionto keep cancellation semantics consistent for async hook implementations.
using var replyPipe = new BlockingCollection<object?>(boundedCapacity: 1);
_currentOutPipe.Add(new PipelineItem((query, caption), PipelineType.ShouldContinue, replyPipe), CancelToken);
return (bool)replyPipe.Take(CancelToken)!;
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:130
- When cancellation is requested while waiting for the pipeline-thread reply,
replyPipe.Take(CancelToken)(or the precedingAdd) can throwOperationCanceledException, which can leak out of async hooks. Translate cancellation toPipelineStoppedExceptionfor consistent stop behavior.
using var replyPipe = new BlockingCollection<object?>(boundedCapacity: 1);
_currentOutPipe.Add(new PipelineItem((caption, message, userName, targetName), PipelineType.PromptForCredential, replyPipe), CancelToken);
return (PSCredential?)replyPipe.Take(CancelToken);
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02213bb8a8
ℹ️ 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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (8)
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:258
WriteErrorreturns early when_currentOutPipeis null, which bypassesThrowIfStopped(). This can cause async code to miss cancellation (StopProcessing/Dispose) and continue running. MoveThrowIfStopped()before the early-return.
if (!IsPipelineThread && Volatile.Read(ref _currentOutPipe) is null)
{
return;
}
ThrowIfStopped();
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:290
WriteWarningreturns early when_currentOutPipeis null, which bypassesThrowIfStopped(). This can mask cancellation for async continuations. CallThrowIfStopped()before the early-return (same pattern applies to the other pipeline-write helpers).
if (!IsPipelineThread && Volatile.Read(ref _currentOutPipe) is null)
{
return;
}
ThrowIfStopped();
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:308
WriteVerbosereturns early when_currentOutPipeis null, which bypassesThrowIfStopped(). This can allow async continuations to miss cancellation. CallThrowIfStopped()before returning.
if (!IsPipelineThread && Volatile.Read(ref _currentOutPipe) is null)
{
return;
}
ThrowIfStopped();
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:326
WriteDebugreturns early when_currentOutPipeis null, which bypassesThrowIfStopped(). This can make cancellation inconsistent depending on timing. MoveThrowIfStopped()before the early-return.
if (!IsPipelineThread && Volatile.Read(ref _currentOutPipe) is null)
{
return;
}
ThrowIfStopped();
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:344
WriteInformationreturns early when_currentOutPipeis null, which bypassesThrowIfStopped(). This can cause async continuations to ignore StopProcessing/Dispose cancellation. CallThrowIfStopped()before returning.
if (!IsPipelineThread && Volatile.Read(ref _currentOutPipe) is null)
{
return;
}
ThrowIfStopped();
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:362
WriteProgressreturns early when_currentOutPipeis null, which bypassesThrowIfStopped(). This makes cancellation behavior dependent on whether the pipe is still attached. MoveThrowIfStopped()before the early-return.
if (!IsPipelineThread && Volatile.Read(ref _currentOutPipe) is null)
{
return;
}
ThrowIfStopped();
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:420
RequestPipelineReplydisposesreplyPipeonly on the success path. IfTake(CancelToken)throws (cancellation/race), the pipe isn't disposed, and cancellation can surface asOperationCanceledExceptioninstead of the cmdlet-standardPipelineStoppedException. Useusing/tryto guarantee disposal and normalize cancellation.
private object? RequestPipelineReply(object? value, PipelineType type)
{
ThrowIfStopped();
var replyPipe = new BlockingCollection<PipelineReply>(boundedCapacity: 1);
if (!TryQueue(new PipelineItem(value, type, replyPipe)))
{
throw new InvalidOperationException("No active PowerShell pipeline is available for the asynchronous request.");
}
var reply = replyPipe.Take(CancelToken);
replyPipe.Dispose();
return reply.Value;
}
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:438
TryQueuecan throwOperationCanceledExceptionfromoutPipe.Add(item, CancelToken)if cancellation is requested between the earlierThrowIfStopped()check and the add. That leaksOperationCanceledExceptionto callers and breaks the type's goal of normalizing cancellation toPipelineStoppedException. Catch and translate cancellation here.
try
{
outPipe.Add(item, CancelToken);
return true;
}
catch (InvalidOperationException)
{
return false;
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
SectigoCertificateManager.PowerShell/Communication/AsyncPSCmdlet.cs:382
RequestPipelineReplyusesreplyPipe.Take(CancelToken), which can throwOperationCanceledExceptionif the cmdlet is stopped after the request is queued but before a reply is received. Most other cancellation paths normalize toPipelineStoppedException(e.g.,GetBlockTaskResultand the pipeline pump), so this can leak an unexpected exception type to callers likeShouldContinue/ShouldProcess.
var reply = replyPipe.Take(CancelToken);
return reply.Value;
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cec0ed16f9
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca70d6217f
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ee71f9876
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96e0ef3fb1
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a00ed41f59
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5bedb345d
ℹ️ 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".
|
Final review settlement: 8f933f0 restores required braces, preserves exception stack traces, coordinates pump-lease admission, and builds with zero warnings. The generic catches are intentionally limited to best-effort cleanup/draining while preserving the primary exception; the short-circuit loop avoids allocating a filtered sequence. AsyncHookSynchronizationContext intentionally matches normal SynchronizationContext exception semantics for unexpected callback defects—only PipelineStoppedException is normalized as expected cancellation, rather than silently hiding arbitrary bugs. |
Summary - synchronize the repo-local
AsyncPSCmdletwith the canonical implementation in EvotecIT/PSPublishModule#630 - start asynchronous hooks on the PowerShell pipeline thread and marshal post-await output, prompts, and errors back to it - add safe terminating errors,ShouldContinue, credential prompts, cancellation, scheduler/context isolation, and extensible disposal - keep the source copy local to this repository; no runtime or package dependency is introduced - preserve repository-specific helpers and extension points where present ## Compatibility Cmdlet names and parameters are unchanged. Asynchronous hooks must remain async end to end and must not block withTask.Wait,Task.Result, orTask.WaitAll. ## Verification SectigoCertificateManager.PowerShell Release build on .NET Framework and .NET 8. ## Review follow-through The synchronized source includes the complete hardened lifecycle and follows this repository's mandatory brace convention. The net472/net8 PowerShell build is warning-free.Final async lifecycle semantics
Synchronous PowerShell calls restore the host synchronization context before queued work is drained or the base API is invoked. Queue admission and hook completion are serialized, preserving records admitted before completion while rejecting stale producers afterward. Reentrant drains preserve FIFO order, including context-free captured callbacks made while a lazy queued item is actively being pumped.
Hook completion wakes the pump immediately. Direct downstream stops cancel work already started with the cmdlet token, cancellation-source disposal waits until cancellation callbacks return, and derived synchronous
EndProcessingoverrides can continue interacting with the pipeline after calling the base implementation. Pipeline-thread ownership uses volatile publication, captured callbacks drop cleanly after stop, and progress snapshots retain optional runtimeTotalvalues.Queued host interactions now atomically arbitrate cancellation against the pipeline claim: canceled unclaimed requests never enter the host, while a claimed prompt keeps its reply observed. Completed and failing synchronous hooks drain all causally reentrant records, and a synchronous pipeline stop cancels the shared token before rethrow.
PSPublishModule remains the canonical source owner. Consumer repositories retain namespace-adjusted local source copies and their repository-specific interfaces, helpers, formatting, and split-file conventions without adding a runtime package dependency.
Final validation
SectigoCertificateManager.PowerShell Release build passed at the exact head; K&R formatting is preserved.