Skip to content

Add AI skills: analyze-debug-files, find-related-github-issues, trace-code-introduction - #2584

Open
David Paulson (dpaulson45) wants to merge 7 commits into
mainfrom
dpaul-AISkill
Open

David Paulson (dpaulson45) wants to merge 7 commits into
mainfrom
dpaul-AISkill

Conversation

@dpaulson45

@dpaulson45 David Paulson (dpaulson45) commented Sep 10, 2026 •

Copy link
Copy Markdown
Member

Issue:
Debug-log investigations need a repeatable way to identify unhandled failures, correlate them with released source, and find related issues and introducing changes without treating log or GitHub content as instructions.

Reason:
Analyzing older logs against current source can misattribute failures. Summary-time correlation, incomplete retained evidence, rollover boundaries, and overly strong duplicate classifications can also produce misleading conclusions. The supporting helpers and their contracts need regression coverage.

Fix:
Adds four repository skills, with executable PowerShell helpers, documentation, and colocated tests:

Skill Purpose
analyze-debug-files Inventory debug logs, verify a release-tag baseline, correlate retained error evidence with pinned source, and write a redacted analysis report.
find-release-tag-for-script-version Find the earliest verified release CSV match for a script version and resolve its source commit.
find-related-github-issues Search issues and PRs by redacted exception signatures and return candidate duplicates or similar reports.
trace-code-introduction Trace a pinned source range to its introducing commit and associated PR, with explicit provenance and conservative regression assessments.
  • Centralizes local-path validation for repository-scoped helpers while keeping the release resolver self-contained.
  • Caches baseline dependency graphs by commit and refuses unverified baselines rather than substituting current source.
  • Preserves authoritative handled/unhandled classifications, distinguishes invocations before correlation, and reports uncertainty when rollover finalization or retained evidence is insufficient.
  • Writes reports without overwriting existing files; redacts sensitive values and neutralizes HTML/Markdown in inline evidence.
  • Requires confirmation of failure context or root cause before recommending duplicate closure. Exception phrase matches alone are not proof.
  • Includes parser, lookup, path-safety, report-rendering, source-sync, and documentation-contract coverage.
  • Narrows Pester discovery's exclusion to .github\examples, allowing real skill/shared-helper tests to run. Preserves main's existing -Branch behavior: documentation-only changes, including skill Markdown, still skip Pester when no .ps1 changes are detected.
  • Updates contributor instructions for skill layout, shared helpers, and test discovery.

Scope is the Windows workflow. This PR does not add Linux/macOS portability fixes or an automated AI skill-execution gate in the PR pipeline.

Validation:
The branch is organized into six logical commits. Its final tracked tree is identical to the pre-reorganization snapshot; only commit history changed.

Current validation:

Command Result
Invoke-Pester -Path @('.\.github\skills', '.\.github\skill-lib\Tests', '.\.build\Tests\Pester.Tests.ps1') -Output Normal -PassThru Passed: 355 tests, zero failures or skips.
.\.build\SpellCheck.ps1 Passed: 874 files checked, zero issues.
git diff --check origin/main...HEAD Passed.
git diff --exit-code backup/dpaul-AISkill-before-reorg-20261104 HEAD Passed: no tracked-content differences from the local backup of the original history.
Invoke-CodeFormatterOnFiles -FilePaths $files Blocked by an internal PSScriptAnalyzer command-metadata error; not claimed as a clean formatter pass.

For the read-only formatter check, $files was populated with git diff --name-only origin/main...HEAD -- '*.ps1' '*.md' after dot-sourcing .\.build\Invoke-CodeFormatterOnFiles.ps1. It ran in an isolated pwsh -NoProfile process with DOTNET_PROCESSOR_COUNT=1; the caller's environment was restored afterward. The analyzer still failed internally and exhausted its retries. No rules or hooks were disabled. Normal pre-commit checks, including PSScriptAnalyzer, passed for all six reconstructed commits.

The full repository Pester run, build, merge-validation script, and a fresh manual end-to-end AI skill execution were not rerun against the current revision as part of this description update. The focused tests validate helper behavior and documented contracts; they do not prove the AI follows every instruction correctly.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@dpaulson45

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate findings remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds three Copilot skills for CSS-Exchange debug-log triage, GitHub issue correlation, and code-introduction tracing, plus confirmed commit-SHA support for release lookup.

Changes:

  • Adds debug-log metadata parsing, dependency caching, and report generation.
  • Adds related GitHub issue search and source-history tracing.
  • Extends release resolution with ConfirmedCommitSha and updates spelling data.
File summaries
File Description
.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1 Traces source ranges to introducing commits.
.github/skills/trace-code-introduction/SKILL.md Documents code-introduction tracing.
.github/skills/find-release-tag-for-script-version/SKILL.md Documents confirmed commit-SHA output.
.github/skills/find-release-tag-for-script-version/Find-ReleaseTagForScriptVersion.ps1 Resolves release tags and commit SHAs.
.github/skills/find-related-github-issues/SKILL.md Documents related-issue lookup.
.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1 Searches and classifies related issues.
.github/skills/analyze-debug-files/SKILL.md Defines the end-to-end analysis workflow.
.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1 Parses and sanitizes debug logs.
.build/cspell-words.txt Adds skill-specific dictionary terms.
Review details

Suppressed comments (12)

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:555

  • The sanitizer removes C0 controls and DEL, but it leaves the C1 range U+0080–U+009F. Those characters can be returned in snippets and violate the report's explicit no-stray-C0/C1 assertion. Extend the character class to cover \x7F-\x9F.
    '[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]',

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:672

  • The regex-timeout fallback uses code -ge 0x20 as its printable test, which also accepts C1 controls U+0080–U+009F. A timeout therefore bypasses the normal sanitizer's intended output guarantee and can still place those controls in the report. Use the same printable range as the normal path here.
            if (($code -ge 0x20 -and $code -ne 0x7F) -or $code -eq 0x09) {

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:992

  • HealthChecker's Get-ErrorsThatOccurred also emits ----Errors that occurred that was not handled remotely---- after the normal unhandled footer (see Diagnostics/HealthChecker/Helpers/Get-ErrorsThatOccurred.ps1:37-40). This state machine only recognizes the handled/unhandled headers, so a run with hidden remote errors is still marked SummaryComplete with UnhandledCount=0, causing the analyzer to report a clean run while silently dropping those errors. Add a remote-section signal/count or downgrade completion when that header is present.
                } elseif ($Script:UnhandledSummaryHeaderRegex.IsMatch($line)) {
                    $summaryState = 'unhandled'
                    $summaryUnhandledCount = 0
                    $unhandledHeaderLine = $lineNumber
                    $unhandledHeaderCount++

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:417

  • The logger formats [System.DateTime]::Now with the machine's culture (Shared/LoggerFunctions.ps1:60-63), so the common en-US output includes an AM/PM suffix. This regex and the AcceptedTimestampFormats below reject that form, leaving $ts null on every timestamped line; version candidates, completion signals, summary events, and body-evidence correlation then become unavailable for ordinary HealthChecker logs. Parse the logger's culture-dependent format (or add the 12-hour forms) consistently to the framing regex and accepted formats.
    '\A\s*\[(?<ts>[0-9]{1,2}/[0-9]{1,2}/[0-9]{4}\s+[0-9]{1,2}:[0-9]{2}:[0-9]{2}(?:\.[0-9]+)?)\]',

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:490

  • This explanation cites Write-Host, but Get-ErrorsThatOccurred.ps1 emits both summary headers with Write-Verbose. The headers are untimestamped because the message starts with CRLF and the logger prefixes the timestamp before those newlines; documenting the actual mechanism is important because this comment justifies the anchored header regex.
# cannot forge an authoritative summary. HealthChecker emits these as
# untimestamped lines (the writers in Get-ErrorsThatOccurred.ps1 use
# `Write-Host`, not `Write-Verbose`), so anchor to line start/end.

.github/skills/analyze-debug-files/SKILL.md:576

  • The suffix loop does not actually require a unique match: an absolute key can end with multiple repository paths, and the first hashtable key wins. For example, a key ending in .../Shared/OutputOverrides/Write-Error.ps1 can also match a shorter nested path if one exists, causing the dependency graph to resolve to the wrong source file. Collect suffix matches and accept only the unique/longest path (or reject ambiguity).
            if ($winKey.EndsWith($k, [System.StringComparison]::OrdinalIgnoreCase)) {
                return $Index[$k]

.github/skills/analyze-debug-files/SKILL.md:479

  • A cache miss can be caused by an existing corrupt or incomplete $cacheDir (for example, invalid metadata). This branch deletes the newly built temp directory whenever the final directory exists, leaving the bad entry in place, so every subsequent run rebuilds and remains BuildOnly. Distinguish a valid concurrent winner from a stale invalid entry and replace or quarantine the latter.
                if (Test-Path -LiteralPath $cacheDir -PathType Container) {
                    Remove-Item -LiteralPath $tempDir -Recurse -Force -ErrorAction SilentlyContinue
                } else {
                    Move-Item -LiteralPath $tempDir -Destination $cacheDir
                    $materializationSource = 'BuildAndCached'

.github/skills/analyze-debug-files/SKILL.md:1041

  • find-related-github-issues defines PartialLookup as usable retained results with incomplete coverage, not as an unavailable lookup. Treating every non-Ok status as unavailable would hide those matches from the report and contradict the sibling skill's contract; render retained results plus an incompleteness note for PartialLookup, reserving "unavailable" for the failure statuses.
**Failure modes.** Both skills return a `Status` field. When
`Status -ne 'Ok'`, render a single-line "lookup unavailable" note in
the corresponding subsection and continue with the report. A failing
provenance lookup MUST NOT abort the report.

.github/skills/analyze-debug-files/SKILL.md:437

  • This invocation is still subject to the caller's $PSNativeCommandUseErrorActionPreference. When that preference is $true and Build.ps1 exits nonzero—the condition this step explicitly says may be cosmetic—PowerShell throws before the XML existence checks, so the documented BuildOnly/cache flow aborts instead of using the generated XML. Disable native-error promotion only around this invocation and restore the caller's value in finally, or invoke the process through an API that captures the exit code without throwing.
            & pwsh -NoProfile -File (Join-Path $worktreeRoot '.build\Build.ps1')

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:88

  • Mandatory does not reject whitespace-only input. In that case normalization produces no query, the final status falls through to Ok with empty results, and callers can incorrectly report that no related issue exists instead of an invalid/failed lookup. Reject an empty or whitespace-only top-level exception before building the query list.
    [string]$TopLevelException,

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:128

  • These replacements create literal placeholder tokens, but the subsequent exact GitHub query is built from the normalized phrase. For Failure at C:\a\b, the query becomes "Failure at path" after delimiter stripping; GitHub does not treat path as a wildcard, so an issue saying Failure at C:\other\d is never returned and normalization cannot provide the advertised stable matching. Build search phrases that omit volatile spans or use unquoted invariant terms, while keeping normalization for local classification.
    $s = [regex]::Replace($s, '[A-Za-z]:\\[^\s"'']+', '<path>')
    # Strip UNC paths.
    $s = [regex]::Replace($s, '\\\\[^\s"'']+', '<unc>')
    # Strip randomized temp module names like tmpEXO_3fzjpepe.o0p
    $s = [regex]::Replace($s, 'tmpEXO_[A-Za-z0-9]+(?:\.[A-Za-z0-9]+)?', 'tmpEXO_<random>')

.github/skills/find-release-tag-for-script-version/Find-ReleaseTagForScriptVersion.ps1:358

  • For a valid empty gh release list response ([]), ConvertFrom-Json can produce $null (especially on Windows PowerShell 5.1). @($parsedReleases) then creates an array containing one null element, so the loop below throws Unexpected release entry shape from gh instead of reaching the not-found-no-candidates result. Preserve $null as an empty array before iterating.
    $releases = @($parsedReleases)
  • Files reviewed: 9/9 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@tweekerz
Rob Whaley (tweekerz) marked this pull request as ready for review September 11, 2026 01:03
@tweekerz
Rob Whaley (tweekerz) requested a review from a team as a code owner September 11, 2026 01:03
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@tweekerz
Rob Whaley (tweekerz) marked this pull request as draft September 11, 2026 01:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate findings affect caching, report safety, log parsing, and issue/provenance handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (14)

Previously missed (1) — in code that hasn't changed since the last review.

.github/skills/analyze-debug-files/SKILL.md:401

  • Cache validity is decided from metadata.json alone, then the XML files are imported outside that validation try. A truncated or malformed dependency XML therefore aborts the analysis on a cache hit instead of being quarantined/rebuilt, despite the cache-population path explicitly treating malformed entries as recoverable cache misses.

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:641

  • These completion patterns only accept 24-hour timestamps, but the parser explicitly accepts the en-US 12-hour form (h:mm:ss tt) and LoggerFunctions.ps1 emits the current culture's DateTime.ToString(). A normal en-US line such as [9/11/2026 1:14:30 PM] : No errors occurred in the script. therefore produces no completion signal and can be classified as critically incomplete; apply the same optional AM|PM suffix to all three terminal-message patterns.
        Pattern = [regex]::new('\A\s*\[[0-9]{1,2}/[0-9]{1,2}/[0-9]{4}\s+[0-9]{1,2}:[0-9]{2}:[0-9]{2}(?:\.[0-9]+)?\]\s*:\s*No\s+errors\s+occurred\s+in\s+the\s+script\.\s*\z', [System.Text.RegularExpressions.RegexOptions]::Compiled, $Script:RegexTimeout)

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:940

  • The file is checked with Test-IsSafeLocalFile during enumeration, but this later opens it again by pathname. A local replacement between those operations can turn the path into a reparse point or redirect it outside the validated directory, bypassing the local-file trust boundary; keep and read from the validated handle (or revalidate the opened handle's final target) instead of reopening the name.
        $stream = [System.IO.File]::Open($FileInfo.FullName, [System.IO.FileMode]::Open, [System.IO.FileAccess]::Read, [System.IO.FileShare]::ReadWrite)

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:1094

  • This only validates the timestamp's shape, while Get-LineTimestamp returns $null for semantically invalid values such as month/day/hour 99. The branch then creates a SummaryEvent with a null Timestamp, and Step 7 dereferences SummaryEvent.Timestamp.AddSeconds(-60), so a malformed log can abort the analysis instead of being treated as an invalid record.
                } elseif ($Script:ErrorIndexRegex.IsMatch($line)) {

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:1064

  • Setting $summaryState here does not actually parse any remote error: WriteRemoteErrorInformation emits a Remote Error Information block with Exception Message: fields, not an Error Index: header. The event-creation branch below is gated by $ErrorIndexRegex, so remote failures are skipped and UnhandledCount/SummaryEvents omit them; incrementing $unhandledHeaderCount also makes every valid remote section set MultipleSummaryBlocksDetected, causing Step 6 to skip correlation. Add a remote-record parser (or change the producer format) and treat this section as part of the same summary lifecycle.
                    $summaryState = 'unhandled'
                    if ($null -eq $summaryUnhandledCount) { $summaryUnhandledCount = 0 }
                    if ($null -eq $unhandledHeaderLine) { $unhandledHeaderLine = $lineNumber }
                    $unhandledHeaderCount++
                    if ($unhandledHeaderCount -gt 1) { $multipleSummaryBlocksDetected = $true }

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:398

  • Some repository logs include the extension in the base name (ConfigureExchangeHybridApplication.ps1-Debug_<timestamp>.txt, documented at docs/Hybrid/ConfigureExchangeHybridApplication.md:16). When that filename matches, $Matches['name'] already ends in .ps1, so this appends a second extension and returns ConfigureExchangeHybridApplication.ps1.ps1; Step 2 then cannot resolve the release baseline for the log.
            ScriptName      = "$($Matches['name']).ps1"

.github/skills/analyze-debug-files/SKILL.md:1190

  • On the documented cache-hit path $worktreeRoot is intentionally $null and Steps 6-8 read source with git show; therefore this requirement to resolve every permalink against an "alive worktree" cannot be satisfied and may cause Step 8 to reject valid cache-hit reports. State the invariant in terms of the pinned Git tree, with the worktree and git show paths as the two materialization mechanisms.
  Every permalink path resolves against the alive worktree at that

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:169

  • Replacing a normalized placeholder with a space makes an exact phrase cross a gap that cannot exist in a real issue body: Failed to open <path> for user becomes Failed to open for user, while the normalized issue still contains <path>. Because the query is then wrapped in quotes, path/GUID-bearing exceptions are likely to miss their duplicate entirely; preserve searchable fragments or avoid exact-phrase matching when placeholders were removed.
    # Drop every placeholder token; GitHub search cannot use them as
    # wildcards. Each placeholder is replaced with a single space so
    # word boundaries stay intact.
    $s = $s -replace '<path>', ' '

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:488

  • If normalization removes the only exception content (for example, an input consisting only of a path, GUID, or timestamp), $queries remains empty and this branch falls through to Status = 'Ok'. That makes the caller report “no related issue” even though no GitHub search was performed; return Error with an explicit sanitization detail when $queryCount -eq 0.
if ($queryCount -gt 0 -and $failureCount -eq $queryCount) {

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:139

  • The repository logger prefixes lines with culture-dependent [DateTime]::Now.ToString() output; for en-US this is commonly [9/11/2026 1:14:31 PM]. This pattern accepts only two-digit 24-hour timestamps, so ordinary debug exception text keeps volatile timestamps and identical failures across runs do not normalize as documented. Accept one- or two-digit date/time components and the optional AM/PM suffix, as Get-DebugFileMetadata.ps1 already does.
    $s = [regex]::Replace($s, '\[\d{2}/\d{2}/\d{4} \d{2}:\d{2}:\d{2}(?:\.\d+)?\]', '<ts>')

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:131

  • The Windows-path pattern stops at the first whitespace, so a common path such as C:\Program Files\Microsoft\Exchange Server\... becomes <path> Files\Microsoft\Exchange Server\... rather than one placeholder. This leaves a machine-dependent tail in the searchable/classification phrase and can leak or mismatch path text; normalize quoted/space-containing paths with a real path boundary instead of [^{\s}]+.
    $s = [regex]::Replace($s, '[A-Za-z]:\\[^\s"'']+', '<path>')

.github/skills/find-release-tag-for-script-version/Find-ReleaseTagForScriptVersion.ps1:266

  • This defense-in-depth check still accepts traversal-shaped repository components such as owner/.. because both components allow only a non-empty character class, not an alphanumeric boundary. Since the value is interpolated into GitHub API paths, use the same start/end-alphanumeric repository pattern enforced by the public related-issues helper.
    if ($Repository -notmatch '\A[A-Za-z0-9._-]+/[A-Za-z0-9._-]+\z') { return $null }

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:350

  • This branch explicitly says the phase-3 diff failed and leaves the regression assessment indeterminate, but still returns Status = 'Ok' with an empty StatusDetail. The caller's contract renders any Ok result normally, so a failed provenance lookup is presented as successful; return a non-Ok status and preserve the failure detail.
            Status               = 'Ok'
            StatusDetail         = ''

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:574

  • A successful git blame can still produce no parseable porcelain attribution (for example, an empty/out-of-range result or boundary-only output). In that case the final else indexes $blameShas[0] under strict mode and throws instead of returning the documented Unavailable provenance state.
            if (-not $blameOk) {
  • Files reviewed: 9/9 changed files
  • Comments generated: 5
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Ten moderate findings remain across the cache, issue-search, release-lookup, and tracing implementations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (8)

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:173

  • These replacements leave surrounding quote characters behind. For a normal PowerShell message such as Path 'C:\foo' was not found, normalization produces a query like Path ' ' was not found; the exact-phrase search then looks for a literal blank quoted path, so the corresponding issue with its real path is missed. Remove optional quote delimiters together with each placeholder before collapsing whitespace.
    $s = $s -replace '<path>', ' '
    $s = $s -replace '<unc>', ' '
    $s = $s -replace '<guid>', ' '
    $s = $s -replace '<ts>', ' '
    $s = $s -replace 'tmpEXO_<random>', ' '

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:217

  • Get-SafeQueryPhrase is also used for the non-exact identifier queries, but this filter leaves leading - operators intact. A log-controlled discriminator such as -secret is emitted unquoted by Invoke-GhSearch and changes GitHub's query semantics instead of being searched as data. Neutralize leading hyphens, or quote all untrusted query fragments, before passing them to gh.
    $p = [regex]::Replace($p, '[\(\)\[\]\{\}\<\>:]', ' ')

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:331

  • These variables are later used as the $matchTop/$matchInner criteria for classifying returned issues, but Get-DistinctivePhrase truncates messages longer than 80 characters. That means an issue sharing only the common prefix can be reported as Similar or Duplicate, even when the rest of the exception differs; the latter classification can cause a real failure to be closed as a duplicate. Keep the shortened phrases for GitHub queries, but use the full normalized/searchable phrases for local classification.
$topPhrase = Get-DistinctivePhrase -NormalizedText $searchTop
$innerPhrase = Get-DistinctivePhrase -NormalizedText $searchInner

.github/skills/find-release-tag-for-script-version/Find-ReleaseTagForScriptVersion.ps1:333

  • This helper does not disable or otherwise guard $PSNativeCommandUseErrorActionPreference, unlike the other new native-command helpers. If a caller has that preference enabled together with ErrorActionPreference = 'Stop', a failed gh release list throws before $LASTEXITCODE is inspected, so the documented structured failure result and cleanup path are bypassed. Set the native-command preference to false for the guarded calls (restoring the caller's value afterward) or catch the promoted exception and convert it to the result status.
    $releaseJson = gh release list --repo $qualifiedRepository --limit $releaseListLimit --json $jsonFields 2>&1
    if ($LASTEXITCODE -ne 0) {
        throw "Failed to list releases from ${Repository}: $(ConvertTo-SafeDetail ($releaseJson -join ' '))"

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:384

  • The -eq, -ne, -and, and -or alternatives are not word-bounded, so identifiers such as Invoke-eqThing in an added or removed line are counted as conditional operators after strings and comments are stripped. That can incorrectly produce Intentional or PossibleRegression verdicts; add a word boundary to each operator token.
    $guardPattern = '-eq|-ne|-and|-or|-not\b|\bif\b|\belseif\b|\bIsNullOr(?:Empty|WhiteSpace)\b|\bTest-Path\b|\bthrow\b|\bcontinue\b|\breturn\b'

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:373

  • These predicates discard every diff line whose payload starts with the same marker character: a real added source line +foo is emitted as ++foo, and a removed -foo as --foo. Such lines are skipped along with the +++/--- file headers, so AddedLines/RemovedLines and the guard-keyword heuristic can be wrong for valid source or here-string content. Exclude only the actual file-header forms (or parse hunk metadata) while retaining doubled-marker source lines.
        if ($ln -match '^\+[^+]' -or ($ln -match '^\+$')) {
            $addedLines += $ln.Substring(1)
        } elseif ($ln -match '^-[^-]' -or ($ln -match '^-$')) {
            $removedLines += $ln.Substring(1)

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:392

  • ParseInput is called once per diff line, so it cannot retain state across a multiline here-string. A changed line inside an @"..."@ or @'...'@ body is therefore parsed as executable PowerShell (or handled by the fallback regex), allowing prose such as if or -eq to affect $keywordsAdded/$keywordsRemoved and produce a false regression verdict. Parse the complete hunk or carry here-string state across lines before extracting guard keywords.
            [void][System.Management.Automation.Language.Parser]::ParseInput(
                $line, [ref]$tokens, [ref]$errors)

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:362

  • When Phase 3 diff extraction fails, this branch returns Status = 'Ok' with an empty StatusDetail. The analyze-debug-files contract treats only non-Ok provenance results as unavailable, so a failed diff is rendered as a successful lookup with an Indeterminate verdict and the failure is hidden. Return an error status and preserve the diff failure in StatusDetail.
            Provenance           = $null
            Status               = 'Ok'
            StatusDetail         = ''
  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved moderate and critical findings remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (11)

.github/skills/analyze-debug-files/SKILL.md:1401

  • This requirement conflicts with the cache-hit flow: lines 361-371 state that no worktree is materialized on a cache hit, but this says every permalink must resolve against an alive worktree. An implementation following the requirement literally will either rebuild the worktree unnecessarily or fail all cached runs when $worktreeRoot is null. Define the integrity check in terms of the pinned tree via git show (with the worktree as an optional cache-miss implementation).
  Every permalink path resolves against the alive worktree at that
  SHA. Every source token quoted in a `Cause` narrative appears in
  the linked line slice fetched via
  `git show $baseline.ConfirmedCommitSha:<path>`; convert 1-based

.github/skills/analyze-debug-files/SKILL.md:114

  • Get-EmptyFileResult preserves filename-derived confidence for Empty, Oversize, and Unreadable entries, so a directory containing only an oversized or empty HealthChecker-Debug... file passes this gate despite having no parsed input. Steps 2 and 3 then operate on an empty parsed set and can prompt for a version/baseline; include Status -eq 'Parsed' in this predicate.
Rule: if the inventory produces zero files whose `ScriptNameConfidence`
is `High` or `Medium`, print the following concise message and STOP the

.github/skills/analyze-debug-files/SKILL.md:155

  • This sentence is missing the noun describing what TerminationLineText contains, so the field documentation reads "sanitized text of that belongs" and is unclear to callers.
  belongs to the section and closes the run of errors), whereas
  `NextErrorIndex`, `NextRemoteRecord`, and `SectionHeaderTransition`

.github/skills/analyze-debug-files/SKILL.md:400

  • Step 5 is gated only on PowerShell 7+, but this cache-root construction unconditionally passes $env:LOCALAPPDATA to Join-Path. On PowerShell 7 on Linux/macOS, where that variable is normally unset, the skill aborts before materializing the baseline even though the inventory helper accepts non-Windows absolute paths. Add an explicit Windows prerequisite or choose a platform-appropriate writable cache root.
$cacheRoot = Join-Path $env:LOCALAPPDATA 'CSS-Exchange\dependency-cache'

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:457

  • When the normalized top-level and inner phrases are identical, the query builder suppresses the inner query, but both $matchTop and $matchInner become true for the same single phrase and this branch reports a Duplicate. The documented duplicate contract requires two distinct exception phrases; make the inner match conditional on $innerPhrase -ne $topPhrase.
    $matchInner = $innerPhrase -and $normBody.IndexOf($innerPhrase, [System.StringComparison]::OrdinalIgnoreCase) -ge 0

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:349

  • This query is described as finding script-scoped issues that do not quote the exception, but classification later accepts only a full $topPhrase/$innerPhrase (or a function-only match, which is discarded). A result found solely by <scriptStem> <topToken> is therefore dropped at lines 487–488, so this query cannot provide the advertised coverage. Either remove it or carry query-specific evidence into classification with an explicitly documented similarity rule.
        # A single distinctive token from the top-level phrase, plus the
        # script stem, catches script-scoped issues that do not quote the
        # exception verbatim.
        $topTokens = ($topPhrase -split '\s+' | Where-Object { $_.Length -ge 6 }) | Select-Object -First 1
        if ($topTokens) { $queries += @{ Query = "$scriptStem $topTokens"; ExactPhrase = $false } }

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:457

  • $topPhrase and $innerPhrase are the ≤80-character distinctive search prefixes returned by Get-DistinctivePhrase, but these same prefixes drive local classification. A long exception whose first 80 characters match another issue can therefore satisfy both matchTop and matchInner and be reported as a duplicate even when the remainder differs. Keep the full normalized/searchable phrases for classification and reserve the shortened phrases for GitHub query construction.
    $matchTop = $topPhrase -and $normBody.IndexOf($topPhrase, [System.StringComparison]::OrdinalIgnoreCase) -ge 0
    $matchInner = $innerPhrase -and $normBody.IndexOf($innerPhrase, [System.StringComparison]::OrdinalIgnoreCase) -ge 0

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:224

  • When git log -L finds no history, this branch returns Status = 'Ok' with no introducing commit. The analyze-debug-files caller treats Ok as a successful provenance lookup, so it can render an empty/indeterminate result instead of the documented "code-introduction lookup unavailable" outcome. Return a non-Ok status such as RangeUnavailable and populate StatusDetail here.
            RegressionAssessment = [PSCustomObject]@{ Verdict = 'Indeterminate'; Reasoning = 'git log -L returned no history for the range.' }
            Provenance           = $null
            Status               = 'Ok'
            StatusDetail         = ''

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:374

  • The ^[^+] / ^[^-] tests intentionally skip every line beginning with two identical diff/code characters, not just unified-diff headers. A valid added ++$counter or removed --$counter line is therefore omitted from AddedLines/RemovedLines and from the guard-token heuristic, which can produce incorrect summaries and verdicts. Skip only +++ a|b/path / --- a|b/path header lines, then classify any remaining line starting with + or -.
        if ($ln -match '^\+[^+]' -or ($ln -match '^\+$')) {
            $addedLines += $ln.Substring(1)
        } elseif ($ln -match '^-[^-]' -or ($ln -match '^-$')) {
            $removedLines += $ln.Substring(1)
        }

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:511

  • gh api normally emits formatted, multi-line JSON, so native command output is an array of lines in PowerShell. Piping that array into ConvertFrom-Json parses each line separately; this optional lookup will usually fall into the catch and lose the PR number. Join $apiOut with newlines (or use a raw-output equivalent) before parsing.
                    $apiParsed = $apiOut | ConvertFrom-Json -ErrorAction Stop

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:539

  • The gh pr view --json response has the same multi-line native-output issue: piping $prJson directly to ConvertFrom-Json can parse one line at a time and make the PR details disappear through the optional-data catch. Join the captured lines before deserializing.
                    $prObj = $prJson | ConvertFrom-Json -ErrorAction Stop
  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate findings remain in the reviewed workflows.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (12)

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:498

  • DateTime.ToString() in Shared/LoggerFunctions.ps1 is culture-dependent, but this pattern only recognizes slash-separated month/day/year. Logs from cultures such as de-DE ([11.09.2026 13:23:45]) or ja-JP ([2026/09/11 13:23:45]) therefore lose all timestamps, so summary footers, completion signals, and timestamped version lines are missed; dd/MM slash cultures are also parsed with the wrong order. Accept the logger's actual culture formats or explicitly mark ambiguous timestamps unsupported.
    '\A\s*\[(?<ts>[0-9]{1,2}/[0-9]{1,2}/[0-9]{4}\s+[0-9]{1,2}:[0-9]{2}:[0-9]{2}(?:\.[0-9]+)?(?:\s*(?i:AM|PM))?)\]',

.github/skills/analyze-debug-files/SKILL.md:115

  • Get-EmptyFileResult preserves the filename-derived High/Medium confidence even for Empty, Oversize, and Unreadable entries, so this predicate can pass with zero parsed files. The workflow then proceeds to Steps 2/3 and may prompt for a version or baseline despite having no readable debug output; require Status = 'Parsed' in this early-stop test.
Rule: if the inventory produces zero files whose `ScriptNameConfidence`
is `High` or `Medium`, print the following concise message and STOP the
skill. Do not proceed to Step 2. Do not prompt for anything. Do not

.github/skills/analyze-debug-files/SKILL.md:1214

  • Phase A is defined to fall through when it has zero discriminator matches, but this paragraph explicitly excludes the case where the 60-second window has zero markers. If the failure narrative is older than the window, that exclusion prevents the required all-history search and loses a valid correlation. Zero discriminator matches—including zero Phase-A markers—should enter Phase B.
   - **Phase B — widen when Phase A had zero DISCRIMINATOR
     matches.** Phase B fires only when Phase A returned zero
     candidates matching the discriminator, NOT when Phase A
     returned multiple (that path exits at "Unresolved" above)
     and NOT when Phase A returned zero markers total. In long

.github/skills/analyze-debug-files/SKILL.md:1160

  • The built-entry fallback only finds the source function declaration; it does not map the reported dist/<Script>.ps1 line to the failing source statement. Passing that declaration/function range to trace-code-introduction can attribute a later failure to the commit that initially added the function, contrary to the required smallest guard/assignment/call range. Preserve the build line mapping or locate the corresponding source statement before tracing.
   reported line refers to a built `dist/` file, the source lives in
   `$allDeps` — search for the exact declaration
   `function <discriminator>` (case-sensitive) and use the
   declaration's own line range.

.github/skills/analyze-debug-files/SKILL.md:1306

  • Step 7 can deliberately leave remote or ambiguous findings without a local source path and contiguous line range, but this sentence still mandates both provenance calls for every unhandled finding. Such findings cannot supply valid -Path, -StartLine, and -EndLine values without fabrication. Restrict these invocations to findings with a resolved local source range.
Invoke both for each genuinely-unhandled finding produced by Step 7.
Both skills return result objects; the report renders them into two

.github/skills/analyze-debug-files/SKILL.md:1447

  • This contract requires the emitted source block to be byte-verbatim, then requires every line to receive right-padding, a # L<n> marker, and optional annotations. A byte comparison against the git show slice will consequently fail for every valid report. Define fidelity as an exact source prefix before generated markers/annotations, or otherwise change the assertion so the renderer can satisfy it.
- **Source-slice fidelity.** For every `#### Cause` code block, the
  emitted source range MUST be byte-verbatim against
  `git show $baseline.ConfirmedCommitSha:<path>` for the contiguous
  range `<a>` through `<b>` inclusive, with no line dropped,
  replaced, summarized, collapsed, or rewritten. The block MUST

.github/skills/analyze-debug-files/SKILL.md:1218

  • After rollover, $priorBody contains markers from earlier segments whose LineNumber values restart at 1, but this Phase-B filter compares all of them with the latest segment's Summary.StartLine. Valid earlier-segment markers can therefore be discarded whenever their local line number is greater than the latest summary line, preventing the required widened correlation. Apply the line-bound only within the summary's own segment, or normalize/tag segment line ranges before filtering.
     of which name the failing operation. Widen to **all**
     markers strictly before `Summary.StartLine`, then apply

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:307

  • The no-query contract is evaluated only after this authentication preflight. For a path-only/placeholder-only exception with no fallback, the helper still runs gh auth status and can return AuthFailure instead of the documented NoSearchablePhrase ("GitHub was never queried"). Build the query list and return the no-search result before command/auth preflight, or skip auth when the list is empty.
# Iter-23 (RD-branch-13): pin to github.com so an inherited or
# hostile GH_HOST cannot direct the auth check (and any subsequent
# ambient credential attachment) at a different GitHub-flavored
# host.
$authOut = & gh auth status --hostname github.com 2>&1

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:422

  • Only JSON syntax is validated here; a successful gh response such as {"message":"..."} or an item without number is accepted. Under StrictMode, $item.number (and later title/state/url) can throw, so a malformed/schema-mismatched response escapes instead of becoming the documented Error/partial lookup. Validate the array shape and required property types before adding items to $aggregate.
        if ($null -eq $item) { continue }
        if (-not $aggregate.ContainsKey($item.number)) {
            $aggregate[$item.number] = $item
        }
    }

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:457

  • $topPhrase and $innerPhrase are intentionally shortened by Get-DistinctivePhrase for GitHub query limits, but those shortened values are also used for local classification. Two distinct exceptions sharing only the first line/80-character prefix can therefore be reported as Similar or Duplicate, contrary to the documented normalized-phrase rule. Keep the short phrases for search, but classify with the full searchable phrases.
    $normBody = ConvertTo-SearchablePhrase -NormalizedText (ConvertTo-NormalizedPhrase -Text (($item.title + "`n" + $item.body)))
    $matchTop = $topPhrase -and $normBody.IndexOf($topPhrase, [System.StringComparison]::OrdinalIgnoreCase) -ge 0
    $matchInner = $innerPhrase -and $normBody.IndexOf($innerPhrase, [System.StringComparison]::OrdinalIgnoreCase) -ge 0

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:457

  • The query sent to GitHub is normalized by Get-SafeQueryPhrase (for example, ConvertFrom-Json : Invalid JSON becomes ConvertFrom-Json Invalid JSON), but the classification below compares the un-sanitized $topPhrase/$innerPhrase against $normBody. A result that matches the actual sanitized search can therefore be silently discarded, so punctuation in ordinary exception messages causes false negatives. Apply the same query-safe normalization to both the phrases and the body before IndexOf classification.
    $normBody = ConvertTo-SearchablePhrase -NormalizedText (ConvertTo-NormalizedPhrase -Text (($item.title + "`n" + $item.body)))
    $matchTop = $topPhrase -and $normBody.IndexOf($topPhrase, [System.StringComparison]::OrdinalIgnoreCase) -ge 0
    $matchInner = $innerPhrase -and $normBody.IndexOf($innerPhrase, [System.StringComparison]::OrdinalIgnoreCase) -ge 0

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:295

  • cat-file proves only that the tip object exists; it does not prove that the parent history needed by git log -L and git blame is available. In a shallow clone these commands can treat the shallow boundary as the introduction and return Status = 'Ok' with false provenance. Detect an incomplete/shallow repository here (or fetch/return an unavailable result) before claiming an introducing commit.
    # Verify commit exists locally.
    & git --no-pager cat-file -e "$BaselineSha^{commit}" 2>$null
  • Files reviewed: 9/9 changed files
  • Comments generated: 3
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate correctness, safety, and recovery issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (8)

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:93

  • The example says this script returns one PSCustomObject per debug file, but the implementation emits a single AnalyzeDebugFiles.Inventory wrapper whose per-file objects are in .Files (lines 1904-1915). A caller following the example will try to read per-file fields from the wrapper and fail to process the inventory; update the example to document the wrapper contract.
    Returns one PSCustomObject per debug file.

.github/skills/analyze-debug-files/Get-DebugFileMetadata.ps1:32

  • The public description says SummaryEvents contains one entry per Error Index: line, but the parser also emits remote unhandled records from ----Remote Error Information----, which deliberately have no Error Index: header (lines 1290-1358). This documentation causes consumers to overlook remote failures or assume the event list is incomplete; describe both local Error Index: and remote error records.
      - SummaryEvents:        Per-error dumps inside the summary block; one
                              entry per `Error Index:` line, with IsHandled
                              set based on which section it appeared in.
                              These are authoritative unhandled/handled
                              exception records emitted at end-of-run.

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:460

  • When the top-level and inner messages reduce to the same distinctive phrase (including when their first 80 characters are identical), the query builder emits only one exception query at line 340, but both $matchTop and $matchInner become true for that same substring. The issue is therefore classified as a duplicate even though the result contains no independent evidence for both exception fields; this violates the contract that only a true two-phrase match may justify closing as a duplicate. Require a non-empty, distinct inner phrase before entering the duplicate branch.
    if ($matchTop -and $matchInner) {

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:276

  • gh search issues does not include pull requests unless --include-prs is supplied. This helper and its skill contract promise to search Issues and Pull Requests, so fixes discussed only in PRs are currently omitted from the related-results classification.
    $out = & gh search issues $qArg --repo $qualifiedRepo --limit $Limit --json 'number,state,title,url,body' 2>&1

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:133

  • Both path-normalization patterns stop at the first whitespace. An exception containing a normal path such as C:\Program Files\... or a UNC share with spaces is only partially replaced, leaving machine-specific path text in the search phrase and preventing equivalent reports from normalizing consistently.
    $s = [regex]::Replace($s, '[A-Za-z]:\\[^\s"'']+', '<path>')
    # Strip UNC paths.
    $s = [regex]::Replace($s, '\\\\[^\s"'']+', '<unc>')

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:139

  • The timestamp normalizer only recognizes two-digit, 24-hour timestamps, while LoggerFunctions.ps1 emits culture-dependent DateTime.ToString() values such as [9/11/2026 3:47:21 PM]. Those timestamps remain in the phrase, so otherwise identical exception searches vary by culture and capture time.
    $s = [regex]::Replace($s, '\[\d{2}/\d{2}/\d{4} \d{2}:\d{2}:\d{2}(?:\.\d+)?\]', '<ts>')

.github/skills/find-release-tag-for-script-version/Find-ReleaseTagForScriptVersion.ps1:424

  • This native invocation is not protected from $PSNativeCommandUseErrorActionPreference. In PowerShell 7.4+, a caller that has enabled that preference gets a terminating exception before the $LASTEXITCODE handling at lines 429-432, so an unavailable/malformed earlier release aborts the entire walk instead of being recorded as download-failed and allowing later candidates to be checked. Disable or scope that preference consistently for all gh calls in this helper, as the sibling helpers do.
        try {
            $ghOutput = gh release download $tag --repo $qualifiedRepository -p "ScriptVersions.csv" -O $csvPath 2>&1

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:523

  • These predicates intentionally exclude +++/--- lines to avoid counting unified-diff file headers, but they also exclude every valid hunk line whose original source begins with + or - because Git prefixes it with a second sign. Such lines disappear from DiffSummary and from guard-keyword extraction, which can change the regression verdict for a change to sign-prefixed PowerShell syntax. Make the parser distinguish file headers from hunk lines while retaining all added/deleted hunk content.
        if ($ln -match '^\+[^+]' -or ($ln -match '^\+$')) {
            $addedLines += $ln.Substring(1)
        } elseif ($ln -match '^-[^-]' -or ($ln -match '^-$')) {
            $removedLines += $ln.Substring(1)
  • Files reviewed: 8/9 changed files
  • Comments generated: 5
  • Review effort level: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Unresolved moderate findings affect parsing, path normalization, issue classification, SHA resolution, and diff analysis.

Review details

Suppressed comments (3)

.github/skills/find-related-github-issues/Find-RelatedGitHubIssues.ps1:483

  • $topPhrase and $innerPhrase are the first 80-character distinctive prefixes, but the documented duplicate/similar classification requires the normalized exception phrases. Two different exceptions that share those prefixes will therefore be classified as a duplicate when both truncated prefixes happen to match the issue. Keep the short phrases for GitHub queries, but compare the full normalized/searchable $searchTop and $searchInner values here.
    $matchTop = $topPhrase -and $normBody.IndexOf($topPhrase, [System.StringComparison]::OrdinalIgnoreCase) -ge 0
    $matchInner = $innerPhrase -and $normBody.IndexOf($innerPhrase, [System.StringComparison]::OrdinalIgnoreCase) -ge 0

.github/skills/find-release-tag-for-script-version/Find-ReleaseTagForScriptVersion.ps1:621

  • When the GitHub API lookup cannot resolve the matched tag, Get-CommitShaForTag returns $null but this branch still reports the match as match-earliest. analyze-debug-files rejects that result because it cannot establish a pinned SHA, so callers receive a seemingly high-confidence match followed by an unstructured abort instead of a distinguishable degraded result. Propagate an explicit commit-resolution failure status (and document it) or make SHA resolution part of the match decision.
                $confirmedSha = Get-CommitShaForTag -Repository $Repository -Tag $tag

.github/skills/trace-code-introduction/Trace-CodeIntroduction.ps1:550

  • These checks are still ambiguous with legitimate added/removed source lines whose content begins with ++ b/... or -- a/... (for example, text inside a PowerShell here-string). Such lines are discarded as file headers, which can undercount DiffSummary and change the regression verdict. Track the actual file-header position or parse the paired ---/+++ header records instead of classifying by prefix alone.
        if ($s -match '^\+\+\+ (b/|/dev/null)' -or $s -match '^--- (a/|/dev/null)') {
            continue
  • Files reviewed: 11/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@dpaulson45
David Paulson (dpaulson45) marked this pull request as ready for review November 3, 2026 16:51
@dpaulson45
David Paulson (dpaulson45) requested review from a team and a lite review from Copilot November 3, 2026 16:51
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Approval is blocked by two critical correctness and path-safety defects that can produce an invalid baseline or contact an untrusted location, with additional release-resolver behavior remaining untested.

3 open issues

Review effort: Lite

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

David Paulson (dpaulson45) added a commit that referenced this pull request Nov 4, 2026
Fixes three pre-existing script blockers surfaced by the second Copilot
review on PR #2584. All three predate the sync-guard commit but are
within scope of the wider AI-skill work.

trace-code-introduction/Trace-CodeIntroduction.ps1 — Phase 3 diff parsing:

- $diffOut can carry ErrorRecord objects when git wrote to stderr
  (we redirect 2>&1). ErrorRecord has no .StartsWith(...) method, so
  the previous loop threw a runtime exception on any git warning and
  aborted the trace mid-parse. Each element is now cast to string
  before regex or String method calls.
- The diff file-header regex is anchored on git's actual header
  form (`+++ b/`, `--- a/`, or `.../dev/null`). The old broad
  `+++ ` / `--- ` pattern silently dropped added / removed source
  lines whose own content began with `++ ` or `-- ` (rendered
  `+++ ` / `--- ` in the unified diff after the leading marker),
  which could omit a guard keyword and produce an incorrect
  regression verdict.
- The narrower header regex would itself regress when a user or
  repo enabled `diff.noprefix=true`, because git then emits real
  headers as `--- path` / `+++ path` with no `a/` or `b/` prefix.
  The `git log` invocation now passes `-c diff.noprefix=false` at
  git-level to force prefix emission regardless of caller config.
  (Rubber-duck-verified: reproduced both the mis-parse and the
  fix by toggling the config across `git log -L`.)

find-release-tag-for-script-version/Find-ReleaseTagForScriptVersion.ps1:

- Added `$PSNativeCommandUseErrorActionPreference = $false` after
  the param block. Without it, callers running PowerShell 7.4+
  with the preference enabled would see NativeCommandExitException
  on any nonzero `gh` exit — bypassing the script's own
  `$LASTEXITCODE` handling and breaking the structured
  `Status = 'GhUnavailable' / 'NotFound' / 'Error'` result
  contract. Matches the pattern already used in the sibling skills
  in this PR.

Dictionary:

- Add `noprefix` to `.build/cspell-words.txt` (git config key
  used in the fix above).

Findings dismissed (not addressed in this commit):

- F-008 (backslash separators in dot-source): apparent false
  positive; no `IsWindows` branch exists in the file and the skill
  is Windows-only by design.
- F-009 (Status='Ok' + Verdict='Indeterminate' on Phase 3 failure):
  deliberate — the trace itself succeeded (IntroducingCommit is
  populated), only classification is indeterminate. Documented in
  the code comment above the return.
- F-012 (2-digit slash timestamp normalizer): heuristic tuned for
  the CSS-Exchange logger format.
- F-013 (truncated phrase used for classification): heuristic
  tradeoff between recall and precision on GitHub-search results.

Verification:
- Formatter: clean on all 3 modified files
- SpellCheck: 0/814 issues
- PSScriptAnalyzer: clean
- Pester (sync-guard): 23/23 pass
- Rubber-duck: clean after diff.noprefix follow-up

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human review is needed because incomplete control-character encoding can cause report rejection and send untrusted escape sequences to the terminal through the console fallback.

1 open issue
3 resolved since last review

Review effort: Max

@dpaulson45

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The duplicate-detection guidance can cause distinct defects to be closed based on generic exception matches without confirming the failure context or root cause.

5 open issues
1 resolved since last review

Review effort: Max

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unix test discovery skips hidden skill directories, leaving Markdown-only skill changes untested, and report filename collision handling fails on non-Windows PowerShell.

1 open issue
5 resolved since last review

Review effort: Max

Comment thread .build/Pester.ps1 Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

On Linux PowerShell, the collision filter misses EEXIST errors, causing a second report with the same timestamp to fail instead of creating a suffixed report.

No open issues 🎉

1 resolved since last review

Review effort: Max

Resolve script versions through release CSV assets, pin the confirmed source commit, and cover lookup, validation, cleanup, and host-pinning behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 419dd781-7c86-44c4-941b-26f4b36afda4
Search issues and pull requests using redacted exception signatures, classify candidate matches, and cover repository scoping and external lookup failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 419dd781-7c86-44c4-941b-26f4b36afda4
Validate local paths, DOS-device mappings, and reparse-point ancestors before filesystem access, with focused unit coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 419dd781-7c86-44c4-941b-26f4b36afda4
Trace pinned source ranges to their introducing commits and merged PRs, report conservative regression assessments, and reject attribution from incomplete history.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 419dd781-7c86-44c4-941b-26f4b36afda4
Inventory debug logs, verify release baselines, correlate retained error evidence, and write encoded analysis reports. Include source-sync guards, completion contracts, shared-consumer documentation, and regression fixtures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 419dd781-7c86-44c4-941b-26f4b36afda4
Discover skill and shared-helper tests while excluding documentation examples, preserve main behavior for Markdown-only changes, and document skill development conventions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 419dd781-7c86-44c4-941b-26f4b36afda4

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical issues allow non-Windows network mounts to bypass local-only safeguards and leave a global git mock active after tests, silently disabling subsequent git commands.

4 open issues

Review effort: Max

Comment thread .build/Tests/Pester.Tests.ps1 Resolved
Remove global Git mocks with a scope-correct Function provider path, restore existing wrappers, and assert teardown state. Correct the related-issue skill documentation to list the actual GitHub CLI commands.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 419dd781-7c86-44c4-941b-26f4b36afda4

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

With -Branch, Markdown-only skill changes skip Pester contract tests, allowing broken documentation contracts to pass CI and surface on later unrelated PRs.

No open issues 🎉

4 resolved since last review

Review effort: Max

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants