Turn bv update into bv self-update

[rdeago]

Checklist of related issues / discussions

• Closes #
• Part of Add bv dependencies, and rename bv update to bv self-update #352
• Related discussion(s): #

Proposed changes

Fifth PR of the #352 sequence: bv update becomes bv self-update, per the spec's "Related changes" and the decisions recorded on the issue.

• Rename. bv update → bv self-update, with no backward-compatibility alias (bv is a preview tool). The help text carries the naming nuance: canonically "self-update" means "replace my own binary"; ours changes the repository, never the binary. The delegation exemption, every error text, and the docs follow the rename.
• --to <version>. Stamps a named version instead of the running bv's own. A value that does not parse fails with exit 1. No package source is consulted: the dotnet tool update --version step doubles as the existence check — it runs unconditionally, even when the manifest already pins the target, and before any file is written, so an unknown version leaves the repository untouched.
• Full family stamping. A new BuildvanaFamily type owns the closed family list — bv, Buildvana.Sdk, Buildvana.Runtime; deliberately not a Buildvana.* prefix match, so a third-party package under that prefix is never dragged into lockstep. A new FamilyPinUpdater discovers family pins textually — never through MSBuild evaluation, since the SDK may be the thing being changed — via the FileFinder walk (gitignore-aware, with /artifacts/, /.buildvana-temp/, bin/, obj/, node_modules/ excluded): PackageVersion/GlobalPackageReference/PackageReference items in *proj/.props/.targets files, and versioned #:package/#:sdk directives in .cs files within the file-based-app scope (see "Additional changes" below). Literal versions are spliced in place (byte-preserving, Version-child whitespace included); non-literal versions (property references, ranges, floating) are reported and left alone, and a family VersionOverride is deliberately invisible (an override overrules a dependency update, self-update included). The downgrade guard now covers every parseable family pin, and names the --to version when one was given. The summary prints one line per family pin found — unchanged and left-alone included — naming the declaring file, so it doubles as a discovery-coverage check.
• CHANGELOG and docs. The existing Unreleased bullet now describes self-update; no BREAKING CHANGE entry, because the same section introduces the command. docs/DirectoryStructure.md follows.

Additional pin groups (spec §"Additional pin groups") are configuration that arrives with stage 2; the scan set is already a parameter, so wiring their item names in later is a one-line change.

Additional changes

• The NotAccessedPositionalProperty suppression comments on MsBuildPin.ItemType and AppDirective.Kind (from Add splice editors for package pins and app directives #358) claimed this PR would be their first reader; it reads Id/VersionText instead, so the comments now name bv deps (stage 2) honestly.
• A new top-level fileBasedApps configuration setting (gitignore-syntax patterns) declares where file-based apps live, and family-pin discovery reads .cs files only within that scope. The factory emits the built-in .buildvana/hooks/ patterns after the configured ones — the last matching pattern wins in gitignore syntax — so no configuration, a negation included, can move the hooks out of scope. Introduced for finding 4 of the first review: reading every .cs file in the repository made discovery cost scale with the source tree, and stage 2's bv deps needs the same declared scope for its own scan. When the configuration cannot be read, self-update falls back to the hooks scope with a warning instead of failing. This repository's own entry stays commented out in buildvana.jsonc until a Buildvana version that knows the setting is pinned here — the pinned SDK validates the file at build time and rejects settings it does not know.

Types of changes

This pull request introduces the following types of changes:

• Bug fix
• New feature
• Test addition / update (no changes to non-test code)
• Refactor (no changes in public API syntax or semantics)
• Performance improvement (no changes in public API syntax or semantics)
• Documentation update (not including changelog)
• Dependency addition / update
• Changes to toolchain configuration
• Changes to CI (workflows, bot / app configurations)
• Changes to repository files (.gitattributes, .gitignore)
• Other

Breaking changes

This pull request introduces breaking changes:

• Yes
• No

Checklist

• For all types of changes:
  • I have read the Code of Conduct and agree to be bound by it
  • I have read the Contributor Guide and agree to follow it
• For code changes only:
  • The project builds on my machine, via the provided build script, with zero warnings
  • I have added tests that prove my feature works / my fix is effective
  • I have added / modified XML documentation according to changes in code
  • I have checked that all the links I added or modified in XML documentation point to their intended destination
• For documentation changes only:
  • I have checked and proofread documentation locally
  • I have checked that all the links I added or modified point to their intended destination

🤖 Generated with Claude Code

[rdeago]

@claude please review this PR.

[claude]

Claude finished @rdeago's task in 6m 6s —— View job

---

Review

• Read the PR diff
• Review new services (BuildvanaFamily, FamilyPin, FamilyPinUpdater)
• Review SelfVersionService and --to handling
• Review settings, command, and rename fallout
• Review tests, docs, and changelog
• Post review

The rename is complete: no bv update string survives in code, tests, docs, or the changelog. The write ordering is sound, and the tests cover the interesting shapes. Five findings, none blocking.

---

1. The documented --to existence check is skipped when the manifest already pins the target

src/Buildvana.Tool/Services/SelfVersionService.cs:303-306 returns the "unchanged" line and never spawns the CLI when the manifest pin already equals the target:

if (currentPin is not null && VersionComparer.VersionRelease.Equals(currentPin, target))
{
    return $"{ToolPackageId}: {targetText} (tool manifest, unchanged)";
}

Three places state the guarantee unconditionally — the <remarks> at SelfVersionService.cs:152-154 ("the dotnet tool update step is the existence check... a version no configured source knows fails the update with the repository untouched"), the CHANGELOG bullet, and docs/DirectoryStructure.md. On that path no source is consulted at all.

The reachable case is the half-updated repository the design already names elsewhere: the manifest sits at 2.1.43-preview while global.json and the family pins lag at 2.1.41-preview. bv self-update --to 2.1.43-preview then stamps global.json, every family pin, and the $schema reference without any feed ever confirming that 2.1.43-preview exists.

Two ways out: run the dotnet tool update --version step anyway when toVersion is not null (it is idempotent), or narrow the claim in the three texts to "unless the manifest already pins the target".

Fix this →

2. StampPins rewrites every family pin in a file, not the pins it was given

src/Buildvana.Tool/Services/FamilyPinUpdater.cs:98-115 uses the pins argument only to derive the set of files and to render the summary. The rewrite callback re-derives the pins from the file text and stamps every family id it finds there.

Today the only caller passes the whole list from DiscoverPins, so the result is correct. The XML doc, though, says "Stamps the target version into the given pins" — pin-level granularity that the implementation does not have. bv deps in stage 2 is the obvious future caller that would want to pass a filtered subset, and it would silently get more than it asked for.

Cheapest fix is honesty in the doc: name the parameter's real role ("the pins discovered by DiscoverPins; used to select the files to edit and to build the summary"), and say that every family pin in those files is stamped.

3. The command description omits --to

src/Buildvana.Tool/Subcommands/SelfUpdateCommand.cs:14-16 still says the pins go "to this bv's version". That is now only the default. The --to option's own description carries the nuance, but bv --help shows the command line first.

Suggested: "…to a single version: this bv's own, or the one --to names."

4. Discovery reads every .cs file in the repository whole

FamilyPinUpdater.cs:128-129 classifies every .cs file as a file-based app, and AppDirectiveEditor.ReadDirectives goes through TextSpliceHelper.ReadAllTextWithEncoding, which reads the entire file. Only the leading directive block matters — in an ordinary source file that is the copyright comment plus the first using.

For this repository the cost is invisible. For a consumer repository with tens of thousands of source files it becomes the dominant cost of bv self-update, and the scan set only grows in stage 2. A bounded prefix read for the discovery pass would fix it; the splice pass can keep reading whole files, since it only touches files that actually declared a pin.

5. A family VersionOverride is invisible, summary included

MsBuildPinEditor treats an item without a Version as not a pin, so <PackageReference Include="Buildvana.Runtime" VersionOverride="2.1.40-preview" /> under central package management is neither stamped nor reported. For a third-party package that policy is right — whoever manages a version that way manages it themselves. For a family package it defeats the stated purpose of the summary, which "doubles as a discovery-coverage check": the pin drifts out of lockstep and nothing says so.

Worth a deliberate call rather than a silent inheritance from the editor's general policy. Reporting family VersionOverrides as "left alone" would keep the coverage claim true without stamping anything.

---

Verified, no action needed

• IProcessRunner.RunAsync defaults to throwOnNonZero: true, so a failed dotnet tool update aborts before UpdateGlobalJson and StampPins. The "manifest first" ordering guarantee at SelfVersionService.cs:190-196 holds.
• CliOptionReader.ReadValue rejects --to with a missing or blank value, so --to never silently degrades to "stamp my own version".
• SelfUpdateCommand prints through IAnsiConsole.WriteLine, not MarkupLine. That matters here: a left-alone pin renders its raw version text, and a range pin prints [2.1.40-preview], which MarkupLine would try to parse as markup and throw on.
• EnsureUsableManifestEntry makes the currentPin! dereference at SelfVersionService.cs:326 safe — it throws exactly on the HasEntry && Version is null combination.
• BuildvanaFamily.Contains binds to the MemoryExtensions.Contains span overload, so dropping using System.Linq in a215a87 is correct, not a build break.
• NewVersionText and LineFor agree on both the trim rule and the comparer, so no summary line can claim a change the splice did not make.

Minor

• SelfVersionService.cs:286 joins the newer-pin phrases with " and ". Two reads fine; five family pins produce "A and B and C and D and E". A vertical list, or commas with a final "and", would scale.
• This repository has no literal family pin of its own — the hook's #:package Buildvana.Runtime is deliberately versionless — so bv self-update here prints an empty family section. Expected, but it means the new stamping path gets no dogfooding coverage. The unit tests carry that weight alone.
  · branch issue-352-self-update

[rdeago]

All five findings and the join note are addressed; two went further or differently than suggested.

1. Existence check — fixed in 348a68b, and further than proposed: the dotnet tool update step now runs unconditionally, not only with --to. A matching manifest pin proves neither that the version is obtainable nor that it is downloaded — self-update is delegation-exempt, so the pinned version need not even be present on the machine — and the remarks already promised the download for the next dotnet bv. Test added for the manifest-matches---to shape.
2. StampPins doc — fixed in 09b30ef with the honest wording: the parameter selects the files to edit and shapes the summary, and every family pin in those files is stamped.
3. Command description — fixed in bf72ea2 with the suggested wording.
4. Whole-file reads — fixed in 4d6fb72, but with a declared scope instead of a bounded prefix read. A prefix read shrinks bytes, not file opens, so discovery would still scale with the source tree; and stage 2's bv deps needs a declared scan scope anyway for its own directive scan. Discovery now reads .cs files only within the file-based-app scope: the built-in .buildvana/hooks/, plus the gitignore-syntax patterns of a new top-level fileBasedApps configuration setting (the factory appends them to the built-in scope, so no configuration can hide the hooks). An unreadable configuration degrades to the hooks scope with a warning — self-update stays the repair tool. This repository's own entry is commented out in buildvana.jsonc until a Buildvana version that knows the setting is pinned here.
5. Family VersionOverride — deliberate call made and documented in f490e21, the other way: it stays invisible, summary included. An override overrules a dependency update — self-update is one — so whoever writes an override owns the version and its consequences, drift out of lockstep included. Reporting it as "left alone" would put into the summary a pin the command refuses on principle to own.

The " and " join switches to commas plus a final "and" from three offenders on (5310ccb). On the dogfooding note: correct and accepted — this repository deliberately has no literal family pin, so the unit tests carry the stamping path alone.

@claude Please take another look.

[claude]

Claude finished @rdeago's task in 8m 43s —— View job

---

Re-review

• Read the six new commits
• Verify finding 1 (existence check, 348a68b)
• Verify finding 2 (StampPins doc, 09b30ef)
• Verify finding 3 (command description, bf72ea2)
• Verify finding 4 (declared file-based-app scope, 4d6fb72)
• Verify finding 5 (VersionOverride rationale, f490e21)
• Verify the join change (5310ccb)
• Check for issues introduced by the fixes
• Post review

All six are addressed, and the two calls that went the other way are the right ones. Making the tool-update step unconditional is a strictly better fix than the one I proposed: it keeps one code path instead of two, and the download guarantee needed it as much as the existence check did. Leaving VersionOverride invisible is coherent — "not ours to move, so not ours to report" — and it is now stated where a future reader will hit it.

Two findings, both in the new scope machinery, neither blocking.

---

1. A configured negation can move the hooks out of scope

src/Buildvana.Core.Configuration/BuildvanaConfigFactory.cs:184-188 appends the configured patterns after the built-in one:

return json is null ? defaults.FileBasedApps : [.. defaults.FileBasedApps, .. json];

GitignoreFile.Evaluate returns the decision of the last matching pattern (GitignoreFile.cs:57-70), so appending is what gives a configured pattern the final say. With "fileBasedApps": ["!.buildvana/hooks/"], the composed list is [".buildvana/hooks/", "!.buildvana/hooks/"]; evaluating .buildvana/hooks as a directory returns Include, IsFileBasedApp requires Ignore, and no ancestor or deeper component matches either. The hooks stop being scanned, and the summary — the documented discovery-coverage check — reports nothing missing.

The comment right above the line says the opposite, and so do the test comment at BuildvanaConfigFactoryTests.cs:284-285 and the CHANGELOG's "added to the built-in .buildvana/hooks/ scope":

Configured patterns extend the built-in scope rather than replace it: hooks are file-based apps by definition, so no configuration can move them out of scope.

Emitting the built-in patterns last makes the claim true: a user negation is then overridden for the hooks path only, and every other configured pattern behaves exactly as now (the built-in pattern matches nothing outside .buildvana/hooks/, so it cannot override anything else). Create_FileBasedApps_AppendToBuiltInScope asserts the current order and would need its expectation flipped. The alternative — deleting the guarantee from the three places that state it — costs less code but gives up something worth having.

Fix this →

2. The pattern list this repository plans to declare misses .claude/tools/

buildvana.jsonc:141 records the value to enable later as ["/tools/"]. This repository has a second file-based app outside that pattern: .claude/tools/inspect.cs, the sanity-check gate.

Nothing is missed today — inspect.cs carries no #: directive at all, and tools/*.cs reference only NuGet.Versioning, Spectre.Console, and a #:project. It matters when stage 2's bv deps reads the same declared scope for every package directive rather than family ones: the gate tool would be outside the scan by omission rather than by decision. ["/tools/", "/.claude/tools/"] costs nothing now and is right whichever way inspect.cs grows.

Worth deciding now, while the value is a comment and the reasoning is in front of you.

---

Verified

• PinToolManifestAsync runs the CLI on every path, and the three summary shapes are exhaustive: !hasEntry → "added", isUnchanged → "unchanged", otherwise the transition. currentPin! at SelfVersionService.cs:332 is reached only when hasEntry is true and the pin differs, which EnsureUsableManifestEntry has already proved non-null.
• UpdateRepository_WithToVersionMatchingManifestPin_StillRunsToolUpdate pins the exact half-updated shape from the finding, and UpdateRepository_WhenEverythingCurrent_RunsToolUpdateButChangesNoFile now asserts the run instead of its absence.
• FamilyPinUpdater takes Lazy<BuildvanaConfig>, which AddLazySupport() resolves through LazyResolver<T>; the registration exists and predates this PR.
• The catch (BuildFailedException) in ResolveScope covers every way the configuration can fail: UserFile wraps IO and security errors, and BuildvanaJsonConfigProvider throws it for a duplicate file, a parse error, and each schema violation.
• IsFileBasedApp walks ancestors and stops at the first Ignore, which is the pruning that gitignore(5) requires — a negation cannot re-include a file under a selected directory. It matches GitignorePattern.Matches's documented contract.
• FileFinder yields /-separated relative paths, so the Split('/') in IsFileBasedApp is correct on Windows too.
• The Oxford-comma join is exercised by a test asserting the full three-phrase string, and GetRange(0, Count - 1) is safe on the Count >= 3 branch alone.
• docs/BuildvanaToolConfiguration.md is still a placeholder pointing at the schema and at this repository's buildvana.jsonc as the worked example, "with every setting either set or commented out with the reason it is not needed". The commented-out fileBasedApps entry satisfies that contract, so no doc is owed beyond what the PR already writes.
• The generated schema entry for fileBasedApps matches how the other string arrays are emitted — bare {"type": "string"} items, no minLength. A blank pattern would be dropped by GitignorePattern.TryParse anyway.

Minor

• buildvana.jsonc:140-141 says the line "will then read: "fileBasedApps": ["/tools/"]" and then shows that same line commented out. One of the two is enough; the other commented-out settings in the file state the reason and let the JSON speak for itself.
• bv self-update now spawns dotnet tool update even in a fully current repository, so the cheapest run is no longer local-only. That is the price of the guarantee and worth paying; just be aware that a run with no network succeeds only while the package is cached.

Ready to merge from my side once you have taken a call on finding 1.
· branch issue-352-self-update

[rdeago]

Both findings and the first minor note are addressed.

1. Configured negation of the hooks scope — fixed in 0949388 with the proposed fix: the factory now emits the built-in patterns last, so they override a negation of the hooks scope and — matching nothing outside it — nothing else. The comment states the mechanism, the order test flipped its expectation, and two tests pin the negation down: the factory composes ["!.buildvana/hooks/"] into ["!.buildvana/hooks/", ".buildvana/hooks/"], and FamilyPinUpdater still discovers hook pins under that composed list.
2. .claude/tools/ in the planned scope — taken in f18e19a: the planned value is now ["/tools/", "/.claude/tools/"]. The gate tool is a file-based app, and stage 2's bv deps reads the same scope, so leaving it out would have been omission, not decision.

The first minor note is folded into f18e19a: the sentence restating the planned line is gone; the commented JSON speaks for itself, like the file's other commented-out settings. The second is acknowledged as stated — a no-network run succeeding only while the package is cached is the price of the download guarantee, and it was a deliberate call.

@claude Please take another look.

[claude]

Claude finished @rdeago's task in 3m 48s —— View job

---

Re-review (round 3)

• Read the two new commits
• Verify finding 1 (built-in patterns emitted last, 0949388)
• Verify finding 2 (.claude/tools/ in planned scope, f18e19a)
• Check for issues introduced by the order flip
• Post review

Both are addressed, and the guarantee now holds for more negation shapes than the one I named. No new findings. Ready to merge from my side.

---

Finding 1 — verified, and stronger than the test asserts

The flip is correct at the root: GitignorePattern.TryParse anchors any pattern containing an internal slash (GitignorePattern-parsing.cs:63-66), and the trailing slash makes it directory-only. So .buildvana/hooks/ matches exactly one path — the root .buildvana/hooks directory — and emitting it last can override nothing else. The comment's second clause is a fact about the parser, not an assumption.

The guarantee also survives negation shapes other than the exact !.buildvana/hooks/ the tests pin:

Configured negation	Composed decision at .buildvana/hooks
!.buildvana/	matches .buildvana only → Include, no prune; built-in decides one level down
!hooks/ (unanchored → **/hooks/)	built-in is later and matches → Ignore
!.buildvana/hooks/**	needs a component below; built-in decides the directory first
!.buildvana/hooks/*.cs	never consulted — IsFileBasedApp returns at the directory level

The last two hold because of the ancestor short-circuit at FamilyPinUpdater.cs:157-164: a matched directory selects its whole subtree, so a file-level negation under the hooks path cannot be reached. That mirrors gitignore(5)'s pruning, which FileFinder's remarks already state, so the two walks agree.

The three tests carry it: the factory order test flipped, Create_FileBasedApps_CannotNegateTheBuiltInScope pins the composition, and DiscoverPins_WithConfiguredNegationOfHooksScope_StillReadsHooks pins the discovery under that composed list.

Finding 2 — verified

.gitignore does not exclude .claude/, so FileFinder reaches .claude/tools/inspect.cs and the planned pattern is live rather than dead on arrival. Today it selects nothing: #: directives exist only in tools/update-dependencies.cs, tools/generate-config-schema.cs, and the hook — inspect.cs declares none. That is the point of taking the decision now rather than when bv deps reads the same scope.

Also checked

• No doc surface stated the ordering, so the flip invalidated none of them. The schema description, BuildvanaJsonConfig's [Description], BuildvanaConfig's XML doc, docs/DirectoryStructure.md, and the CHANGELOG all say "added to the built-in scope", which the composed list still satisfies.
• BuildvanaJsonContext already carried IReadOnlyListString before this PR, so FileBasedApps adds no new source-generated serialization surface — it matters because a hook deserializes the config where reflection is off.
• PublicAPI.Unshipped.txt gains exactly the two accessors.

I could not run a build in this environment; the verification above is by reading. Your gate covers the compile.

Minor

• Create_FileBasedApps_CannotNegateTheBuiltInScope asserts the composed list, and DiscoverPins_WithConfiguredNegationOfHooksScope_StillReadsHooks asserts discovery under a hand-written copy of that list. Nothing tests the seam between them. Buildvana.Tool.Tests already references Buildvana.Core.Configuration (SelfVersionServiceTests.cs:5), so one test could feed BuildvanaConfigFactory.Create(...) straight into FamilyPinUpdater and remove the copy. Not worth a commit on its own; worth knowing if either test is ever edited.
• A typo'd pattern in fileBasedApps — a trailing backslash, an unclosed bracket — is dropped silently by GitignorePattern.TryParse, so the declared scope quietly misses files. Git does exactly this with a .gitignore line, so it is conformance rather than a defect. The summary is what surfaces it: a pin the user expected to see simply has no line.
  · branch issue-352-self-update

[rdeago]

The first minor note of the third review is taken in ae63115: the updater-level negation test now feeds BuildvanaConfigFactory.Create(...) straight into FamilyPinUpdater, so the hand-written copy of the composed list is gone and the seam between the two tests is covered. The second minor note stands as conformance, per the review itself. Test-only change, gate green; no re-review needed.