Skip to content

Unify hosted NuGet routing and XML splice anchors - #597

Open
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
refactor/nuget-config-model-20261002
Open

Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
refactor/nuget-config-model-20261002

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Final-head CI is complete: 482 successful checks, 6 skipped; no failures or pending checks. Bugbot passed, there are no unresolved review threads, and the PR is mergeable.

Hosted NuGet rewrites previously interpreted nuget.config with private regexes while restore and VEX used the shared XML reader. Commented-out sources could suppress the nuget.org fallback, and inert sections could receive the inserted Socket source or mapping. The CLI could then report a redirect that NuGet could not consume.

Fixes #561 and #585 by using the existing bounded NuGet XML reader for routing data and live insertion spans. This completes the hosted portion of #594 discussed in architecture discussion #560.

The shared model identifies comments, CDATA, processing instructions, quoted attributes, directly scoped sections, and insertion points after the last direct <clear> child. One insertion helper handles normal and self-closing sections. Malformed or ambiguous layouts refuse the redirect before any config edit, redirect claim, or lock re-pin.

Source names retain their XML identity when copied into fallback mappings. The reader normalizes literal CRLF and attribute whitespace before decoding character references; the writer escapes decoded tab, newline, and carriage return as numeric references. Thus corp&#x9;feed remains a tab-bearing name, while a literal attribute tab remains a space as NuGet interprets it. Original source markup stays byte-for-byte intact.

Validation:

  • All 193 focused NuGet core tests passed after the review correction.
  • The new eight-form whitespace regression fails on the original PR head and passes with the fix. It covers numeric references, literal tab/LF/CR/CRLF, and mixed literal/reference input.
  • Eight real offline restores passed using .NET SDK 8.0.129 and a local NuGet feed. The original PR head reproduces NU1100 for an unrelated package after the fallback mapping changes its source identity; the base and fixed helper preserve it.
  • An independent review checked 18 XML identity cases and separate NuGet controls, including character-reference boundaries and reference-looking text.
  • Parser formatting and diff whitespace checks passed. The fixed commit merges cleanly with current main.
  • Full CI, compatibility workflows, benchmarks, and Bugbot completed successfully on the corrected commit 8fa9750c.

Note

Medium Risk
Changes core hosted NuGet redirect and lock re-pin behavior on real nuget.config input; mistakes could cause NU1100/NU1403 or false redirect claims, though fail-closed validation and broad tests reduce exposure.

Overview
Hosted NuGet redirects no longer splice nuget.config with private regexes while restore/VEX use the shared tokenizer. The NuGet reader now records live byte spans for configuration, packageSources, and packageSourceMapping (including insert points after the last direct <clear>), flags repeated sections, and normalizes literal attribute whitespace before entity decode so source keys match NuGet’s identity rules.

add_nuget_source takes the parsed model, inserts sources/mappings only at those spans (expanding self-closing sections in place), re-parses after source edits, and encodes keys when writing fallback * mappings. Malformed or ambiguous layouts emit redirect_nuget_config_unwritable and skip config edits and lock re-pinning even if a Socket source already appears.

Regression coverage includes commented inactive sources/mappings (e2e #561/#585), refusal to edit inside comments/CDATA/PIs, attribute-whitespace identity, and expanded empty mapping blocks.

Reviewed by Cursor Bugbot for commit 8fa9750. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the arch-refactor PR opened by the scheduled architecture refactor routine label Oct 2, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 2, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Labeled Ready for review at 9ea00801c09d53d8adf42cac3b45a39d7877453b.

  • Sync: merged main @ 045d7ec (7 commits; redirect/mod.rs and the bench npm fixture overlapped). The merge was clean, and the local socket-patch-core nuget/redirect lib tests pass: 688 passed, 1 failed. The failure is the known vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, which fails only when tests run as root, as this sandbox does.
  • CI: 482/488 check runs green on this head (6 skipped by path/matrix filters), 0 failing. Mergeable.
  • Bugbot: reviewed 9ea0080 and found no issues. No unresolved review threads.
  • What to look at: hosted NuGet routing and the XML splice now share one NuGet config model (formats/nuget). Check that the splice anchors still land on the same <packageSources> / <packageSourceMapping> nodes the reader resolves. 93ab1af adds the test that reproduces the old disagreement.

Generated by Claude Code

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator Author

Review updated for 8fa9750cd20d10204ef0cb00f328c041ad571c71: Ready to merge as-is from this review. Final-head CI is complete: 482 successful checks, 6 skipped; no failures or pending checks. Bugbot passed, there are no unresolved review threads, and the PR is mergeable.

The original refactor decoded corp&#x9;feed and then wrote a literal tab into its fallback mapping. XML normalized the mapping key to a space while preserving the source's referenced tab, causing a real NuGet restore to fail with NU1100 for an unrelated package.

The correction normalizes literal XML attribute whitespace before entity decoding and writes decoded tab/LF/CR as numeric references. This preserves both character references and literal-whitespace names without changing existing source markup.

Validation: 193 NuGet core tests passed; the new eight-form regression fails on the original PR head and passes with the fix. Eight real offline .NET 8.0.129 restores passed against a local feed. Independent review also passed 18 XML identity cases and separate NuGet controls. The fixed commit merges cleanly with current main.

No remaining code finding from this review. The Ready label has been restored after all checks completed on the corrected commit.

@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 2, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8fa9750. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 2, 2026

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

arch-refactor PR opened by the scheduled architecture refactor routine Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hosted NuGet mapping reads commented-out package sources

2 participants