Migrate PSTeams surface to TeamsX cmdlets#67
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #67 +/- ##
==========================================
+ Coverage 62.99% 70.11% +7.11%
==========================================
Files 26 47 +21
Lines 327 1974 +1647
Branches 48 174 +126
==========================================
+ Hits 206 1384 +1178
- Misses 108 529 +421
- Partials 13 61 +48 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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: bb096b0491
ℹ️ 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".
| if ($runtimeMajor -ge 10) { | ||
| @('net10.0', 'net8.0', 'netstandard2.0') | ||
| } else { | ||
| @('net8.0', 'netstandard2.0') |
There was a problem hiding this comment.
The Core runtime selection currently prefers net8.0 for every runtime below 10, which includes PowerShell 7.2/7.3 running on .NET 6/7. Those hosts cannot load a net8.0 assembly, so module import fails even though a netstandard2.0 build is available. This creates a runtime compatibility regression for Core users not yet on .NET 8.
Useful? React with 👍 / 👎.
| Import-Module 'C:\Support\GitHub\PSTeams\PSTeams.psd1' -Force | ||
| Get-Command -Module PSTeams | Select-Object -ExpandProperty Name | Sort-Object | ConvertTo-Json | ||
| '@ |
There was a problem hiding this comment.
These tests shell out to pwsh and import a module from C:\Support\GitHub\PSTeams\PSTeams.psd1, which is a machine-specific path and not present in GitHub Actions runners or most contributor environments. As a result, the parity tests fail due to missing files rather than regressions, making the suite unreliable in CI.
Useful? React with 👍 / 👎.
| if (!string.IsNullOrWhiteSpace(ActionUrl)) { | ||
| WriteObject(new TeamsAdaptiveOpenUrlAction { | ||
| Title = Title ?? string.Empty, | ||
| Url = ActionUrl ?? string.Empty | ||
| }); |
There was a problem hiding this comment.
The cmdlet currently creates an open-url action only when ActionUrl is provided, and otherwise falls through to a show-card action. This means callers who explicitly pass -Type Action.OpenUrl can receive an Action.ShowCard payload instead, which changes legacy behavior and can produce incorrect card actions when URL is supplied later or omitted by mistake.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1614410e0e
ℹ️ 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".
| var resolved = Color.FromName(candidate); | ||
| if (resolved.ToArgb() == 0 && | ||
| !string.Equals(candidate, "Transparent", StringComparison.OrdinalIgnoreCase)) { | ||
| throw new ArgumentException("The Input value is not a valid colorname nor an valid color hex code.", nameof(color)); |
There was a problem hiding this comment.
This migration now resolves named colors through Color.FromName, which only supports the built-in System.Drawing set. Legacy PSTeams accepted a much larger palette (for example AlbescentWhite, still used in this repo’s examples), so previously valid color inputs now throw ArgumentException or lose color values in cmdlets that call NormalizeToHex, breaking existing scripts during a 1:1 surface migration.
Useful? React with 👍 / 👎.
| foreach (var section in request.Sections) { | ||
| fragments.Add(RenderSection(section)); |
There was a problem hiding this comment.
BuildBodyFragments appends every rendered section even when RenderSection returns an empty string, so fragments.Count becomes non-zero and the summary fallback is skipped. For messages that only carry empty/start-group-style sections plus Summary, this can emit an empty Graph body.content, resulting in blank messages or Graph validation failures for empty body content.
Useful? React with 👍 / 👎.
| var httpClient = new HttpClient(handler, disposeHandler: true); | ||
| var sender = new WebhookTeamsMessageSender(httpClient, disposeHttpClient: true); | ||
|
|
||
| return new TeamsClient(new ITeamsMessageSender[] { sender }); |
There was a problem hiding this comment.
In the proxy path, a new HttpClientHandler and HttpClient are created for each call, but the returned TeamsClient has no disposal path and callers do not dispose it. In long-lived PowerShell sessions that repeatedly send with -Proxy, this can accumulate undisposed handlers/sockets and eventually cause connection exhaustion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90e6e57e05
ℹ️ 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".
| AdaptiveCard = AdaptiveCard, | ||
| ThemeColor = ResolveThemeColor(), | ||
| HideOriginalBody = HideOriginalBody.IsPresent, | ||
| UseConnectorCardFormat = UseConnectorCardFormat.IsPresent || (AdaptiveCard is null && Sections.Length > 0) |
There was a problem hiding this comment.
New-TeamsMessage now accepts -ThemeColor/-Color and -HideOriginalBody, but UseConnectorCardFormat is only turned on for explicit -UseConnectorCardFormat or when sections are present. When a caller sets color or hide-body on a plain title/text message, those fields are silently dropped because WebhookMessageRenderer.Render takes the non-connector path unless this flag is true, so the output no longer reflects the parameters the user supplied.
Useful? React with 👍 / 👎.
| .Invoke() | ||
| .Select(item => item?.BaseObject) | ||
| .OfType<TeamsMessageSection>() | ||
| .ToArray(); |
There was a problem hiding this comment.
In the legacy scriptblock path, ResolveLegacySections now keeps only TeamsMessageSection objects via OfType<TeamsMessageSection>(). Older PSTeams usage accepted raw section dictionaries/ordered hashtables from scriptblocks, but those are now filtered out entirely, so facts/buttons/images can disappear and Send-TeamsMessage can send a message with no sections for previously working inputs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a300e09a55
ℹ️ 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".
| } | ||
|
|
||
| return AdaptiveCard is null && | ||
| (Sections.Length > 0 || |
There was a problem hiding this comment.
-Sections input before reading Length
When callers pass an optional variable that is $null into New-TeamsMessage -Sections, ShouldUseConnectorCardFormat() dereferences Sections.Length and throws a NullReferenceException before any message object is produced. This makes a common conditional-splat pattern fail at runtime instead of simply treating missing sections as empty input.
Useful? React with 👍 / 👎.
| private static string? GetButtonLink(IDictionary dictionary) { | ||
| if (dictionary.Contains("target")) { | ||
| var targetValue = dictionary["target"]; | ||
| if (targetValue is IEnumerable enumerable && targetValue is not string) { | ||
| foreach (var entry in enumerable) { |
There was a problem hiding this comment.
actions
In New-TeamsSection's legacy dictionary compatibility path, GetButtonLink only checks top-level target/Target/Targets keys and never reads ActionCard-style nested actions[*].target. Legacy ActionCard button dictionaries therefore lose their URL during conversion, and the renderer emits submit buttons with a null target, breaking previously functional button actions.
Useful? React with 👍 / 👎.
| "ActionCard" => TeamsMessageButtonType.TextInput, | ||
| "HttpPOST" => TeamsMessageButtonType.HttpPost, |
There was a problem hiding this comment.
ActionCard buttons
The legacy dictionary mapper currently converts every @type='ActionCard' button to TeamsMessageButtonType.TextInput, so DateInput cards are silently rewritten as text-input cards. Older payloads that relied on DateInput semantics now render the wrong input control after migration through New-TeamsSection dictionary conversion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 124eebb57e
ℹ️ 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".
| if (value is IDictionary dictionary) { | ||
| if (CmdletNewHeroCard.TryCreateCardButton(dictionary, out var fallbackButton)) { | ||
| ApplyItem(card, fallbackButton); |
There was a problem hiding this comment.
In ApplyItem, dictionary inputs are passed to CmdletNewHeroCard.TryCreateCardButton before TryCreateListItem. Legacy list-card item dictionaries contain title, so they satisfy the button check first and get converted into TeamsCardButton instead of TeamsListCardItem, which removes them from content.items and produces malformed button payloads. This is a compatibility regression for scriptblocks that still emit hashtable-style list items (for example, reused legacy payload fragments).
Useful? React with 👍 / 👎.
| var assemblyDirectory = Path.GetDirectoryName(typeof(TeamsPowerShellImageSupport).Assembly.Location) ?? string.Empty; | ||
| var candidates = new[] { | ||
| Path.GetFullPath(Path.Combine(assemblyDirectory, "..", "..", "Images", $"{imageName}.jpg")), | ||
| Path.GetFullPath(Path.Combine(assemblyDirectory, "..", "..", "..", "..", "Module", "PSTeams", "Images", $"{imageName}.jpg")) |
There was a problem hiding this comment.
ResolveBuiltInImage uses a fallback path that climbs four directories from the loaded assembly and then appends Module/PSTeams/Images. When the module is imported from source (the module shell explicitly prefers TeamsX.PowerShell/bin/Debug/<tfm>), this resolves under .../TeamsX.PowerShell/Module/PSTeams/Images, which does not exist; the first candidate under .../bin/Debug/Images also does not exist. As a result, built-in image names like -Image Add silently fail in development/source-tree usage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 494c537421
ℹ️ 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".
| NETProjectName = 'TeamsX.PowerShell' | ||
| NETBinaryModule = 'TeamsX.PowerShell.dll' | ||
| NETConfiguration = 'Release' | ||
| NETFramework = 'net472', 'net8.0', 'net10.0' |
There was a problem hiding this comment.
New-ConfigurationBuild now includes net10.0 in the packed module build, but the module loader imports a single Lib\Core\TeamsX.PowerShell.dll (Module/PSTeams/PSTeams.psm1), so whichever Core binary is emitted there must be loadable by all supported PowerShell Core hosts. If Lib\Core ends up with the net10.0 binary, PowerShell 7.2/7.3 (.NET 6/7) cannot load the module at all. Keep the packaged Core target pinned to a runtime those hosts can load (e.g., net8.0) and test newer TFMs separately.
Useful? React with 👍 / 👎.
| if (!result.IsSuccessStatusCode) { | ||
| WriteError(CreateDeliveryFailureError(result)); | ||
| WriteError(TeamsPowerShellDeliverySupport.CreateDeliveryFailureError(result, "Send-TeamsMessage")); | ||
| } |
There was a problem hiding this comment.
The typed Send-TeamsMessage path only treats non-2xx responses as failures and never calls TeamsPowerShellDeliverySupport.WriteDeliveryIssue, so it misses the same body-level failure detection ("failed"/"error") used by legacy and wrapper-card senders. For webhook endpoints that reply 200 with an error payload, typed sends will look successful and scripts can proceed as if delivery worked.
Useful? React with 👍 / 👎.
2 participants
Summary
Module/PSTeamsand align module packaging/CI with that layoutDetails
TeamsXas the reusable library andTeamsX.PowerShellas the thin cmdlet layermainwhile removing the old runtime script-function pathSend-TeamsMessage, and pipeline-friendlySend-TeamsMessageBodyModule/PSTeamsmanifest refresh path andnet10.0module packaging alignmentValidation
dotnet test .\TeamsX.sln --configuration Debug --no-buildInvoke-Pester -Path .\Module\Tests -Output Detailed