fix(upgrade-guard): parse the version witness as semver, and stop reading "unreadable" as "legacy"

[julianknutsen]

The outage

The cross-era upgrade guard (#4907) refused five production workspaces outright:

Error: legacy Dolt server workspace detected; explicit migration is required before this
bd version can open or modify the workspace. Preserve .beads unchanged and follow
docs/getting-started/upgrading.md#cross-era-upgrades

All five were current-era. Their .beads/.local_version held a Go pseudo-version,
v1.1.1-0.20260805093327-bf97b73749ac, and currentVersionWitness split it on .
expecting exactly three fields. It got four.

Because the guard runs in PersistentPreRunE before any store opens, this failed
every bd invocation — including the read-only ones an operator would reach for to
diagnose it. Measured blast radius before the witness files were hand-rewritten to
1.1.0: one gate sweep logged 644 failures in 6 hours, which froze PR review gates
entirely (no bead ever left needs-review), stalled triage, and left demand-driven
pools at zero — one city ran 1 session against 8 declared agents. Rewriting nine
.local_version files took the sweep to 0 failures/6h immediately. That workaround
is a band-aid: any tooling that writes a pseudo-version back re-breaks the fleet.

Which side is wrong

The writer. And it isn't.

.local_version holds main.Version verbatim, and release tooling injects that by
ldflags — goreleaser passes {{.Version}}, .github/workflows/release.yml passes
steps.version.outputs.version, other build paths pass whatever they resolved. So the
writer can legitimately emit a plain release, a release candidate (1.2.0-rc.1 — the
repo shipped v1.1.0-rc.1), a build carrying metadata, or a Go pseudo-version. Every
one of those is a valid semantic version; none is three bare numeric fields. bd could
not read what bd wrote.

The writer is correct and stays as it is: recording anything but the true version would
destroy the ordering CompareVersions and upgrade detection depend on (a pseudo-version
sorts before the release it names). The reader was counting dots. This PR fixes the
reader and pins the round trip so it cannot drift again.

Two fixes

1. Parse. classifyVersionWitness parses with golang.org/x/mod/semver (already an
indirect dependency; promoted to direct) and reports an era instead of a boolean.
legacyVersionMinor shares the parse, so a pre-1.0 pseudo-version or release candidate
is now correctly classified as legacy where it previously fell through unrecognized.
Shapes strict semver rejects but a distributor could still stamp onto a legacy build —
zero-padded or four-component versions — are still read as legacy from their major
component alone, so the pre-1.0 guard only tightens.

2. Policy — the part that matters. Before this change an unparseable witness meant
"legacy workspace, refuse everything". That is what turned a trivial parse bug into a
fleet outage, and it is wrong on its own terms:

• Every pre-1.0 bd wrote a plain X.Y.Z through this same writer. A string that fails a
  real semver parse cannot have come from one. The guard was inferring "legacy" from
  data that positively excludes legacy.
• The refusal is a claim about the workspace. The evidence is "I could not parse a
  38-byte advisory text file."
• bd treats that file as advisory everywhere else: .local_version is gitignored,
  clone-local, and written best-effort (_ = writeLocalVersion(...)). bd doctor
  downgrades a bad value to a warning whose suggested fix is "run any bd command to
  reset version tracking" — which the guard made impossible.
• The consequences are wildly asymmetric. Refusing is total and fleet-wide. The
  population it protects against is empty, because a genuine 0.55–0.62 workspace has a
  parseable witness and is already caught one branch earlier by legacyServerVersion.

So a witness that is present but unreadable is now its own era, witnessEraUnknown:
bd warns and opens the workspace. A missing witness is unchanged and still refused —
a workspace that never announced itself is genuinely ambiguous, and that is the dominant
real-world legacy shape since the file is gitignored. Any witness that reads as pre-1.0
is still refused.

The warning is self-healing rather than permanent noise. The guard runs immediately
before trackBdVersion, which rewrites the witness whenever it differs from Version,
so the admitted command leaves a readable witness behind and the next command is silent
(TestLegacyUpgradeGuardWarningIsSelfHealing).

Relationship to #5603 / #5625

#5603 reports the same defect from a different writer: Homebrew stamps HEAD-<shortsha>
into main.Version for --HEAD installs. Its analysis reaches the same conclusion this
PR argues — no legacy-era channel could have produced that shape — and it independently
confirms the 1.1.0-rc.1 case.

The unknown era admits those workspaces too, so this fixes the brew---HEAD outage as
well (TestLegacyUpgradeGuardAdmitsBrewHeadStamp). It does not silence their
warning: a HEAD stamp is rewritten identically on every run, so it never heals. That
wants the shape recognizer in #5625 (@anisoptera), which this PR deliberately does not
duplicate, along with that PR's doctor-side fixes which are orthogonal to the guard.
The two compose and both are worth landing. If #5625 lands first this reduces to the
policy commit; the parse here is a superset of versionCore (a real semver parse rather
than cutting at the first -/+), so the merge is mechanical either way. Not opened to
supersede — see the review comment on #5625.

Red before

15 cases across 5 tests fail against the unfixed parser, verified by expressing the
original dot-counting predicates in the new era vocabulary. Representative:

classifyVersionWitness("v1.1.1-0.20260805093327-bf97b73749ac") = Unknown, want Current  <- production
classifyVersionWitness("1.1.0-rc.1")                          = Unknown, want Current
classifyVersionWitness("1.2.0+build.5")                       = Unknown, want Current
classifyVersionWitness("v0.62.0-0.20250101000000-abcdefabcdef") = Unknown, want Legacy
classifyVersionWitness("0.62.0.1")                            = Unknown, want Legacy
guardLegacyUpgradeWorkspace() = legacy Dolt server workspace detected; ... , want nil

Coverage

• TestClassifyVersionWitness — the production pseudo-version, plain 1.1.0,
  v-prefixed release, release candidates, build metadata, prerelease+metadata,
  whitespace, the pre-1.0 side of each, empty, whitespace-only, garbage, binary noise.
• TestVersionWitnessRoundTrip — everything bd can write, including Version itself,
  bd reads back and recognizes as current. This also holds the writer inside the witness
  reader's bounded 64-byte size.
• TestLegacyUpgradeGuardStillRefusesPreOneWorkspaces (0.9.1, v0.49.6, 0.55.0,
  0.62.21, a 0.x pseudo-version, 0.62.0.1) and
  TestLegacyUpgradeGuardRefusesWorkspaceWithoutAnyWitness prove the cross-era guard
  did not relax in either direction.
• Two existing subtests that asserted "malformed ⇒ refuse" now assert
  "malformed ⇒ admit with a warning". That flip is the policy change, called out here
  rather than buried.

Gates

• go build ./..., go vet ./... — clean
• make ci-pr-lint (gofmt + golangci-lint v2.10.1, native and cross-linted
  windows/amd64 nocgo) — 0 issues both lanes
• scripts/test.sh ./cmd/bd ./cmd/bd/doctor ./internal/configfile ./internal/beads —
  all ok (cmd/bd 274s, the full package suite)
• scripts/check-doc-freshness.sh, scripts/check-doc-flags.sh — PASS

Not run: the rest of make test. cmd/bd/doctor/fix fails on this box before and after
the change — it needs a Dolt test container (dial tcp 127.0.0.1: connection refused) —
and no other package touches the guard, the witness, or version tracking.

🤖 Generated with Claude Code

[steveyegge]

Verdict: CLOSE-SUPERSEDED — in favor of #5675, which is your own next-day rewrite of the same fix and is stronger where it counts. Requesting that three pieces unique to this PR be ported there before it merges (list below).

Both PRs make the same two changes — parse the witness with golang.org/x/mod/semver (identical go.mod promotion) and reclassify present-but-unparseable as warn-and-proceed — so only one should land. Reasons #5675 is the better base:

1. This PR leaves the 64-byte read bound in place (legacyUpgradeVersionWitness, unchanged here), and an oversized marker still reads as absent → refused. That's the same "bd cannot read what bd wrote" class one layer down: a pseudo-version of a prerelease tag plus build metadata (v1.2.1-rc.1.0.20260805093327-bf97b73749ac+build.20260805.linux.amd64, 68 bytes, valid semver) trips it. The round-trip test's comment ("holds the writer inside the … 64-byte size") documents the trap; fix(upgrade-guard): parse .local_version as semver; unreadable witness is unknown, not legacy #5675 removes it (bound → 256, oversized = present-but-unreadable).
2. No warning dedup. guardLegacyUpgradeWorkspace runs from several sites per invocation (main.go PersistentPreRunE, bootstrap, doctor, the ancestor walk), so one command can print the warning repeatedly. Also the self-healing property is conditional: trackBdVersion only runs under policy.runMaintenance and not in preview mode, so read-only commands re-warn every run. fix(upgrade-guard): parse .local_version as semver; unreadable witness is unknown, not legacy #5675 dedups per workspace.
3. No CHANGELOG entry; fix(upgrade-guard): parse .local_version as semver; unreadable witness is unknown, not legacy #5675 has one.

Three things here are better than #5675 and should be ported before closing:

• The major-0 fallback in classifyVersionWitness: strict-semver-invalid shapes like 0.62.0.1 classify as legacy here (fail-closed) but as unknown → admitted in fix(upgrade-guard): parse .local_version as semver; unreadable witness is unknown, not legacy #5675. This is the safer verdict for repackaged legacy builds; keep it.
• The docs/getting-started/upgrading.md table update — fix(upgrade-guard): parse .local_version as semver; unreadable witness is unknown, not legacy #5675 leaves the docs contradicting the new behavior.
• Optionally the injectable legacyUpgradeWarnWriter seam, which is a cleaner test surface than global stderr capture.

Coordination note: #5625 (anisoptera) is still open and rewrites the same functions plus the same docs table; whichever lands first forces a manual rebase of the survivors. Your composition analysis in this PR body is the right one — carry it over to #5675, which currently references neither PR.

[julianknutsen]

This PR has been added to the review queue.

Track progress: https://factory.gascity.com/city/maintainer-city/runs/gcg-5436965277228230

[julianknutsen]

Maintainer Adoption Review

Thanks for the contribution, @julianknutsen! This PR fixes a production-outage-class bug in the legacy-Dolt upgrade guard: the version-witness parser was counting dots instead of parsing semver, so valid pseudo-version witnesses like v1.1.1-0.20260805093327-bf97b73749ac were misread and current server workspaces were falsely refused as "legacy". It also stops treating a present-but-unreadable witness as "legacy -> refuse everything". This matters because it removes a false-refusal that could lock users out of their own current workspaces, while keeping the safety-critical refusal of genuinely pre-1.0 layouts fully intact.

This PR was reviewed and adopted with maintainer fixes pushed directly to the PR branch.

Original PR Review

Decision: request_changes resolved to approve.

Specific gaps fixed:

• The outage itself: the dot-counting parser rejected valid semver witnesses. The final patch delegates classification to golang.org/x/mod/semver, so the production pseudo-version v1.1.1-0.20260805093327-bf97b73749ac classifies as current and the guard opens.
• "Present but unreadable" was read as "legacy -> refuse everything". The final patch routes a present, non-empty, unparseable witness to warn-and-open, and it self-heals on the next run.
• Present-but-blank witness was still hard-refused with no self-heal (the review's one major finding). The reader now reports witness presence independently of trimmed content, so a present-but-blank .local_version warns-and-opens while a genuinely missing, non-regular, or oversized witness still refuses. The legacy-refusal safety invariant is preserved and pinned by new tests.

Review findings addressed:

• major / Error Handling & Resilience (source: Codex, confidence: high): a present-but-blank version witness was hard-refused with a misleading "legacy" message and no self-heal. Fixed by maintainer commit 80d59103 ("fix(upgrade-guard): warn+open on a present-but-blank version witness"), which also adds guard-level coverage in cmd/bd/legacy_upgrade_guard_witness_test.go asserting present-blank -> warn+open and missing -> refuse.

Non-blocking follow-ups (shared for visibility -- NOT completed in this PR):

• nit / Debuggability & Operability (source: Claude, confidence: high): a syntactically-current but non-canonical witness that strict semver rejects (zero-padded / leading-zero, e.g. 1.2.03, 01.2.3) now emits the "unreadable version witness ... assuming a current workspace" warning on every run, where the old dot-counter accepted these shapes silently. It is strictly cosmetic -- the workspace still opens -- and no real release path emits these shapes, so it is deliberately not addressed here. Tracked as mc-7mtq: absorb these shapes into the version-witness recognizer referenced for GH#5625 if field warning-noise is ever observed.

Maintainer Changes

One maintainer commit was pushed to the PR branch: 80d59103 "fix(upgrade-guard): warn+open on a present-but-blank version witness" -- cmd/bd/legacy_upgrade_guard.go and a new cmd/bd/legacy_upgrade_guard_witness_test.go, 2 files changed, 115 insertions(+), 14 deletions(-). Your original commit is preserved with authorship intact.

Final Review Status

Ready for the merge queue: final head 80d59103a has passing required GitHub checks, and the merge-ready metadata is in place. I am marking this PR status/merge-ready so the merge queue can pick it up.

CI: https://github.com/gastownhall/beads/actions/runs/32819001787

Review Iterations

2 review passes performed. Iteration 1 returned request_changes on the present-but-blank major finding. After the maintainer fix, iteration 2 re-verified both original defect fixes and the legacy-guard safety property, confirmed the major was resolved, and approved -- an additional reviewer test-hygiene finding was verified against the reviewed code and refuted.

---

Adopted via /adopt-pr workflow. Original contributor commits preserved.