Make versioning rules post-1.0 and define "breaking"

[geevensingh]

Why

Two real defects in the repo's own versioning rules, surfaced by the question "is a large feature change enough to justify a major version bump".

The rules were factually stale. The project is at v1.9.0, but AGENTS.md §12 was headed **Bump rules (pre-1.0 SemVer):** and CHANGELOG.md's preamble claimed "breaking changes ride in minor bumps until v1.0.0". Both would actively mislead a future reader.

"Breaking" was never defined. For a library it self-evidently means a changed signature. DiffViewer is a WPF desktop app shipped as a single .exe with no published package and no external API consumers, so the rule gave no usable guidance — which is exactly the ambiguity that prompted the original question.

What changed

AGENTS.md §12:

• Bump rules are now plain post-1.0 SemVer. Feature → minor, bug-fix batch → patch, breaking → major, with an explicit note that the pre-1.0 carve-out has expired.

• New ### What counts as "breaking" subsection, built on a definition a client app can actually apply:

  A breaking change is an upgrade that can disturb an existing install.

• Enumerates the six compatibility surfaces that definition covers:

  #	Surface	Owner
  1	CLI argv contract (positional, flag, and PR-URL launch forms)	Services/CommandLineParser.cs, README.md
  2	On-disk state (settings.json, recents.json)	Services/SettingsMigrations.cs, Services/SettingsService.cs, Services/RecentsJsonSerializer.cs
  3	Install footprint (per-user PATH)	App.xaml.cs, Services/UserPathRegistrar.cs, Services/WindowsUserPathStore.cs
  4	Keyboard shortcuts	Models/KeyboardShortcutCatalog.cs
  5	Update channel	Services/VelopackUpdateService.cs, vpk pack --packId in release.yml
  6	Platform floor (Windows 10+, x64)	README.md Requirements, DiffViewer.csproj Release block

• Settles a question the old rules were silent on: major is reserved exclusively for compatibility breaks. Size is not a trigger. A major that only means "this release is big" teaches users that majors are safe to take, which inverts the signal on the one occasion it matters. Significance goes in the release notes instead.

CHANGELOG.md: preamble no longer claims a pre-1.0 carve-out.

Follow-up commit: three surface claims corrected

The first commit described surfaces 2, 3, and 5 from nearby XML doc comments rather than from the implementations. Review caught it; f38566a re-verifies all of them against actual code.

Surface 2 was outright wrong. It claimed RecentsJsonSerializer loads an unknown future version as empty. Deserialize actually discards the version (_ = TryInt(obj, "version")), keeps every row that parses, and re-stamps at RecentsDoc.CurrentVersion on the next write — the comment directly above that line records that Phase 7 of the PR-review feature superseded the old "unknown version = empty" policy.

Re-verifying turned up something better than the original claim: settings.json and recents.json use opposite strategies, not a shared one. settings.json migrates forward through registered v(N) → v(N+1) steps and refuses to read a schemaVersion newer than the binary (moves the file aside, starts from defaults); recents.json is forgiving as described above. Both are now documented as the distinct contracts they are.

Surface 3 conflated two different failures. The PATH entry holds AppContext.BaseDirectory (the Velopack current directory), not the executable — so renaming the exe leaves the entry valid and only breaks the command name, while stranding the entry requires changing which directory gets registered. Now separated, with App.xaml.cs named as where the hooks are wired and a note that they only fire for the Velopack-installed copy.

Surface 5 named a file where it should have named an identity. What must stay stable is the pairing of the GithubSource that TryCreateForInstalled resolves against with the vpk pack --packId value stamped by release.yml.

The stale class-level XML doc on RecentsJsonSerializer still asserts the superseded rule and is what misled the original text. It is production code and is deliberately left untouched here — being raised separately.

Review rounds

Every Copilot review comment was valid; each was verified against source before acting.

28ad7da

• Platform compatibility was missing. The list read as exhaustive but omitted the documented platform contract — README.md promises Windows 10+ on x64, and the Release block pins RuntimeIdentifier win-x64 / SelfContained on net8.0-windows. A platform drop plainly disturbs an existing install yet fell outside all five surfaces. Added as surface 6 rather than hedging the list as non-exhaustive: the section's value is that the call is mechanically decidable, and an "among others" escape hatch would reintroduce the ambiguity this PR removes. Surface 6 also names the compounding risk — because surface 5 installs updates on its own, a platform drop can push an in-field copy into a build it can no longer run.
• File.Move, not a copy. Surface 2 said SettingsService "copies" a future-versioned file to settings.json.bak.<unix-time>; BackupAndUseDefaults calls File.Move (SettingsService.cs:151), so the original doesn't survive at its path. Reworded to "moves it aside to".

6eabd65

• Surface 1 omitted the PR-URL launch form. It led with "Both" — a closed list of two — but DiffViewer accepts a third documented form: a lone GitHub pull-request URL. README.md:15-17 advertises DiffViewer.exe https://github.com/owner/repo/pull/123, and CommandLineParser.ParseLaunch (lines 37-40) matches it via PullRequestRef.TryParse and returns FromPullRequest before positional parsing runs. Same failure mode as the platform gap: an exhaustive list that quietly isn't would misclassify removing that form as non-breaking. Now names all three forms and covers narrowing which URLs parse.

Two corrections this change forced

• The lead paragraph of "When to release" claimed "There is no auto-update channel" — false. VelopackUpdateService.cs exists, and release.yml packs a Setup.exe plus RELEASES metadata specifically for the updater to consume. Left as-is it would have contradicted compatibility surface Support 'git difftool' integration via CLI flags #5 two paragraphs below.
• The new subsection is a sibling ### of "When to release" rather than a nested ####. Nesting it would have silently re-parented the existing skip-releases, group-commits, and agent-must-not-tag paragraphs under a heading they don't belong to.

Not changed

Bump semantics beyond post-1.0 + the "breaking" definition. Feature is still minor, bug-fix batch is still patch, the "skip releases for" list is intact, and a Copilot CLI session still must not push a release tag on its own judgment.

Notes for review

• Docs only. No .cs, .xaml, or .csproj files touched across any commit, so per AGENTS.md §5 no build/lint/test run applies.
• No conflict with Add worktree labels to the UI and worktree switching to the dialog #24. That PR also edits CHANGELOG.md, but only its ## [Unreleased] section. This edit is confined to the preamble; [Unreleased] is untouched.
• One deviation from the original brief: it suggested citing git difftool as a consumer of the CLI contract, but README.md explicitly documents that git difftool doesn't fit DiffViewer's flag form. Surface Add whole-file Stage / Unstage / Revert to the file-list right-click menu #1 cites shell aliases, git aliases, and editor integrations instead.

---

Session

• AI-Local-Session: ada363aa-6c26-4bcf-b059-b369422de5a7
• AI-Cloud-Session: 747f79eb-14cd-4937-9ae0-90a063986dd6

[Copilot]

🟡 Changes recommended

The breaking-change list omits platform compatibility and contains an inaccurate description of settings backup behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates release guidance for post-1.0 SemVer and defines application-specific compatibility boundaries.

Changes:

• Replaces obsolete pre-1.0 versioning rules.
• Defines five breaking-change surfaces.
• Corrects the changelog’s SemVer statement.

File summaries

File	Description
AGENTS.md	Updates release and compatibility guidance.
CHANGELOG.md	Aligns the preamble with post-1.0 versioning.

Review details

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

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The exhaustive CLI compatibility list omits the documented GitHub pull-request URL launch form.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

AGENTS.md:510

• The CLI compatibility surface omits the documented single-argument GitHub pull-request URL launch (README.md:15-16, CommandLineParser.cs:35-40). Because this list is intended to be exhaustive, removing or changing that form could be incorrectly treated as non-breaking. Include it alongside the positional and flag forms.

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[geevensingh]

The CLI compatibility surface omits the documented single-argument GitHub pull-request URL launch (README.md:15-16, CommandLineParser.cs:35-40). Because this list is intended to be exhaustive, removing or changing that form could be incorrectly treated as non-breaking.

Valid catch, addressed in 6eabd65. Verified against source before acting: README.md:15-17 advertises DiffViewer.exe https://github.com/owner/repo/pull/123, and CommandLineParser.ParseLaunch (lines 37-40) matches it via PullRequestRef.TryParse, returning CommandLineLaunchPlan.FromPullRequest before positional parsing ever runs.

Surface 1 had led with "Both" — a closed list of two forms. It now names all three and covers narrowing which URLs parse, not just positional-slot and flag changes.

Noting the disposition here because this arrived as a suppressed comment in a review summary rather than an inline thread, so there was no thread to resolve and nothing in the UI would otherwise show it as handled.