Skip to content

fix: remove double-normalization in overridePersistedExtraheader#44492

Merged
pelikhan merged 3 commits into
mainfrom
copilot/aw-fix-matt-pocock-skills-reviewer
Jul 9, 2026
Merged

fix: remove double-normalization in overridePersistedExtraheader#44492
pelikhan merged 3 commits into
mainfrom
copilot/aw-fix-matt-pocock-skills-reviewer

Conversation

Copilot AI commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

The Matt Pocock Skills Reviewer flagged several issues in the git auth helper code introduced with the CI trigger push feature. Issues 1–4 (the overrideApplied guard, push-failure test, getErrorMessage usage, and missing silent flag on the write) were already resolved before this PR.

Changes

  • Double-normalization fix (git_auth_helpers.cjs): overridePersistedExtraheader was calling normalizeServerUrl() on the input and then passing the result to getExtraheaderValues(), which normalizes again internally. Each function now normalizes its own input, making the contract unambiguous.
// Before: pre-normalized URL passed to a function that re-normalizes
const normalizedUrl = normalizeServerUrl(serverUrl);
previousValues = await getExtraheaderValues(normalizedUrl); // double-normalize

// After: each function owns its own normalization
previousValues = await getExtraheaderValues(serverUrl);
  • New test (git_auth_helpers.test.cjs): added a overridePersistedExtraheader test with a trailing-slash URL to explicitly assert that both the read (--get-all) and write (--replace-all) git config calls use the normalized key — documenting the normalization contract at the right layer.

Copilot AI linked an issue Jul 9, 2026 that may be closed by this pull request
Copilot AI and others added 2 commits July 9, 2026 05:48
…add trailing-slash test

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…xtraheader test

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix Matt Pocock Skills Reviewer failure fix: remove double-normalization in overridePersistedExtraheader Jul 9, 2026
Copilot AI requested a review from pelikhan July 9, 2026 05:49
@pelikhan pelikhan marked this pull request as ready for review July 9, 2026 10:56
Copilot AI review requested due to automatic review settings July 9, 2026 10:56

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.

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

Updates git auth helper behavior and tests to ensure URL normalization happens at the correct layer, avoiding passing pre-normalized values into helpers that also normalize.

Changes:

  • Update overridePersistedExtraheader to pass the raw serverUrl into getExtraheaderValues() (which normalizes internally).
  • Add a test asserting that a trailing-slash server URL results in normalized git config keys for both read and write operations.
Show a summary per file
File Description
actions/setup/js/git_auth_helpers.cjs Removes pre-normalization before calling getExtraheaderValues() to clarify normalization responsibility.
actions/setup/js/git_auth_helpers.test.cjs Adds regression test for trailing-slash URLs to ensure normalized git config keys are used consistently.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment thread actions/setup/js/git_auth_helpers.test.cjs
Comment thread actions/setup/js/git_auth_helpers.test.cjs
@pelikhan pelikhan merged commit 11c6b1a into main Jul 9, 2026
54 of 68 checks passed
@pelikhan pelikhan deleted the copilot/aw-fix-matt-pocock-skills-reviewer branch July 9, 2026 11:21
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.82.8

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[aw] Matt Pocock Skills Reviewer failed

3 participants