Skip to content

fix: use AfterTargets="Restore" to support NuGet credential helpers for private feeds#162

Merged
alirezanet merged 5 commits intomasterfrom
copilot/fix-nuget-authentication-issue
Mar 11, 2026
Merged

fix: use AfterTargets="Restore" to support NuGet credential helpers for private feeds#162
alirezanet merged 5 commits intomasterfrom
copilot/fix-nuget-authentication-issue

Conversation

Copy link
Contributor

Copilot AI commented Mar 11, 2026

Description

Users relying on NuGet Credential Helpers for private feeds had dotnet tool restore fail because auth (performed during MSBuild's Restore target) hadn't happened yet when the Husky target ran under BeforeTargets="Restore".

  • AttachCommand.cs — generated MSBuild target now uses AfterTargets="Restore" instead of BeforeTargets="Restore;CollectPackageReferences"
  • docs/guide/automate.md — updated both the primary and multi-framework XML snippets to match
  • docs/guide/submodules.md — updated the submodule attach example snippet to match
  • AttachCommandTests.cs — added assertions that the generated target has AfterTargets="Restore" set and no BeforeTargets

Before / After

<!-- Before -->
<Target Name="husky" BeforeTargets="Restore;CollectPackageReferences" Condition="'$(HUSKY)' != 0" ...>

<!-- After -->
<Target Name="husky" AfterTargets="Restore" Condition="'$(HUSKY)' != 0" ...>

Running after Restore ensures credential helpers have already performed the auth dance before dotnet tool restore is invoked. Husky hook installation has no dependency on running before package restore, so this is non-breaking.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or that my feature works
  • I have made corresponding changes to the documentation
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes
  • I did test corresponding changes on Windows
  • I did test corresponding changes on Linux
  • I did test corresponding changes on Mac
Original prompt

This section details on the original issue you should resolve

<issue_title>Guide for automating installation can break for users that must authenticate to private NuGet feeds</issue_title>
<issue_description>### Details

The guide in automate has the user create a per-project Target that hooks BeforeTargets="Restore":

<Target Name="husky" BeforeTargets="Restore;CollectPackageReferences" Condition="'$(HUSKY)' != 0">
   <Exec Command="dotnet tool restore"  StandardOutputImportance="Low" StandardErrorImportance="High"/>
   <Exec Command="dotnet husky install" StandardOutputImportance="Low" StandardErrorImportance="High"
         WorkingDirectory="../../" />  <!--Update this to the relative path to your project root dir -->
</Target>

While this works for folks that use public NuGet feeds, or that have an active authentication to a private feed, it ill fail for users that are using NuGet Credential Helpers to access private feeds. This is because even with --interactive provided by default by the .NET CLI in .NET 10, the Restore Task is the thing that actually consumes that property to actually go and perform the authentication dances.

It would be less error-prone IMO to change this ordering to AfterTargets="Restore" if possible - that way you can ensure that any authentication required has been performed for you already.</issue_description>

<agent_instructions>I'm not sure if the suggestion by user is a good idea, I don't need this to be implemented unless the suggestion is actually a better choice over the existing behavior. we need to make sure also if we do this change, we're not causing any breaking changes otherwise this is not an option. give me your feedback about this issue</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix automation guide for NuGet authentication errors fix: use AfterTargets="Restore" to support NuGet credential helpers for private feeds Mar 11, 2026
@alirezanet alirezanet marked this pull request as ready for review March 11, 2026 18:43
@alirezanet alirezanet requested a review from Copilot March 11, 2026 19:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates Husky’s generated MSBuild target hook point so NuGet authentication (including credential helpers for private feeds) completes during Restore before Husky runs dotnet tool restore / dotnet husky install.

Changes:

  • Generate the Husky MSBuild target with AfterTargets="Restore" instead of BeforeTargets="Restore;CollectPackageReferences".
  • Update the automation guide XML snippets to match the new target ordering.
  • Extend AttachCommand unit tests to assert AfterTargets="Restore" and absence of BeforeTargets.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/Husky/Cli/AttachCommand.cs Switch generated target hook to AfterTargets="Restore"
tests/HuskyTest/Cli/AttachCommandTests.cs Assert generated XML uses AfterTargets and not BeforeTargets
docs/guide/automate.md Update manual XML snippets to AfterTargets="Restore"
.husky/commit-msg Adds a commit-msg hook invoking Husky task runner (but currently breaks failure propagation)

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

You can also share your feedback on Copilot code review. Take the survey.

@alirezanet
Copy link
Owner

@copilot Does this have any conflicts with this commit? a51fed5

Co-authored-by: alirezanet <7004080+alirezanet@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 11, 2026

@copilot Does this have any conflicts with this commit? a51fed5

No conflicts. a51fed5 is the base commit this branch builds on top of. The only "inconsistency" it introduced was docs/guide/submodules.md containing the old BeforeTargets="Restore;CollectPackageReferences" syntax — which we didn't touch in the original PR commits and has since been updated in f28155a.

This was referenced Mar 15, 2026
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.

Guide for automating installation can break for users that must authenticate to private NuGet feeds

3 participants