Skip revision bump when App.config is unchanged

[nohwnd]

Only bump revision.txt when the VS insertion branch would actually change RocksteadyCLI/App.config. This keeps pipeline reruns from creating revision-only commits.

Verified with git diff --check and PowerShell parser on the inline insertion script.

[Copilot]

Pull request overview

This PR updates the VS insertion pipeline so reruns do not create revision-only commits when RocksteadyCLI/App.config is already unchanged on the insertion branch.

Changes:

• Fetches the current VS-side App.config from the insertion source branch.
• Compares it with the generated vstest app.config.
• Skips the revision.txt bump and push when the content matches.

[Evangelink]

🤖 Expert vstest review (automated)

Small, targeted change to the inline PowerShell in azure-pipelines-insertion.yml. The intent (fetch current App.config from the insertion source branch, compare to the freshly-fetched mirror copy, skip the push when they match) is sound and the wrapping is mechanically correct — the existing bump-and-push block is moved verbatim into the else branch with no other behavioural change. Risk is contained to the VS insertion pipeline (no product code, no wire protocol, no public API, no TFM/packaging surface). A handful of small things worth tweaking, no blockers.

🛑 Blockers

None.

⚠️ Issues

azure-pipelines-insertion.yml:274 — 404 on first insertion will hard-fail the run

Invoke-WebRequest against the AzDO items API throws on HTTP 404, and there's no try around the new fetch separate from the outer try { ... } catch { exit 1 } block. If the source branch ever doesn't yet contain App.config (a brand-new insertion branch, or a base where the file was newly added), the run aborts where the previous code would have just pushed. Old code never touched DevDiv for a read, so this is a new failure mode that didn't exist before.

Suggest catching 404 specifically and treating "not present" as "needs push":

                        try {
                          $currentAppConfigResponse = Invoke-WebRequest -Uri $currentAppConfigUrl -Headers $headers -Method Get -UseBasicParsing
                          $currentAppConfigContent = if ($currentAppConfigResponse.Content -is [byte[]]) {
                            [System.Text.Encoding]::UTF8.GetString($currentAppConfigResponse.Content)
                          } else {
                            $currentAppConfigResponse.Content
                          }
                        }
                        catch [System.Net.WebException] {
                          if ($_.Exception.Response.StatusCode.value__ -eq 404) {
                            Write-Host "App.config not present on insertion branch; will push."
                            $currentAppConfigContent = $null
                          } else {
                            throw
                          }
                        }

💡 Suggestions

azure-pipelines-insertion.yml:281-282 — log message is narrower than the actual behaviour

The message says "skipping revision.txt bump", but the entire Invoke-RestMethod push (App.config and revision.txt) is being skipped. For someone debugging a missing insertion commit, the more accurate phrasing helps:

                        if ($currentAppConfigContent -eq $appConfigContent) {
                          Write-Host "App.config matches the insertion branch; skipping push (no revision.txt bump, no commit)."
                        }

azure-pipelines-insertion.yml:270-273 vs :291-293 — branch-name URL escaping is inconsistent

New code escapes the branch name ($encodedSourceBranchName = [Uri]::EscapeDataString($sourceBranchName)); the pre-existing revision-fetch URL just inlines $targetBranchName unescaped. Both happen to work for typical branch names but the inconsistency invites paper cuts on branches containing reserved characters. Worth normalising one direction or the other while you're here.

Equality semantics — BOM/line-ending sensitivity (informational)

The comparison is literal -eq on strings that came back through Encoding.UTF8.GetString(byte[]), which preserves BOM. If AzDO's rawtext storage ever round-trips bytes with any normalisation (BOM stripping, trailing newline trimming, CRLF↔LF), this comparison silently falls into the "different" branch and you bump revision unnecessarily — which is exactly the pre-PR behaviour, so no regression, just worth knowing the failure mode is "no-op safety net" rather than corruption.

✅ Notes

• Move-into-else is verbatim — no accidental drift in the push body, the $pushBody structure, the oldObjectId = $latestCommitId race window, or the catch block. ✓
• No new env vars, public API, wire/IPC, package, or TFM impact. Dimensions 1–13 N/A. ✓
• Pure pipeline YAML; no automated test possible here. PR description's git diff --check + parser verification is the appropriate bar. ✓
• One extra REST call per run — negligible. ✓

Verdict

Approve once the 404 path is handled (or explicitly confirmed impossible in practice). The two other items are nits.