mirror of
https://github.com/gastownhall/beads.git
synced 2026-09-14 20:17:24 +08:00
Julian Knutsen
62d211937b
* fix(upgrade-guard): parse the version witness as semver, and stop reading "unreadable" as "legacy" The cross-era upgrade guard (#4907) refused five production workspaces outright: legacy Dolt server workspace detected; explicit migration is required before this bd version can open or modify the workspace. Every one was current-era. The witness was a Go pseudo-version, `v1.1.1-0.20260805093327-bf97b73749ac`, and `currentVersionWitness` split it on "." expecting exactly three fields. It got four. Measured blast radius before the operator hand-rewrote nine `.local_version` files to `1.1.0`: every `bd` invocation against five cities failed, taking every order that shells out to bd with it — one gate sweep alone 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 the witness files took the sweep to 0 failures/6h immediately. Two defects, one in the parse and one in the policy. ## The parse `.local_version` holds `main.Version` verbatim, and release tooling injects that by ldflags: goreleaser passes `{{.Version}}`, the release workflow passes `steps.version.outputs.version`, and other build paths pass whatever they resolved. So the writer can emit a plain release, a release candidate, a build carrying metadata, or a Go pseudo-version — all valid semantic versions, none of them three bare numeric fields. bd could not read what bd wrote. The writer is right; recording anything but the true version would destroy the ordering `CompareVersions` needs. The reader was counting dots. `classifyVersionWitness` now parses with `golang.org/x/mod/semver` (already an indirect dependency) and reports an era. `legacyVersionMinor` shares the parse, so a pre-1.0 pseudo-version or release candidate is now *correctly* classified as legacy where it used to fall through unrecognized. Shapes strict semver rejects but a distributor could still stamp on 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. ## The policy Before this change an unparseable witness meant "legacy workspace, refuse every command". That is the part that 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, so a string that fails a real semver parse is affirmative evidence *against* a legacy workspace. The guard was inferring "legacy" from data that positively excludes it, using a file the codebase treats as advisory everywhere else — `.local_version` is gitignored, clone-local, written best-effort, and `bd doctor` downgrades a bad value to a warning whose suggested fix is "run any bd command", which the guard made impossible. So a witness that is *present but unreadable* is now its own era, unknown: bd warns and opens the workspace. A *missing* witness is unchanged and still refused — a workspace that never announced itself is genuinely ambiguous — as is any witness that reads as pre-1.0. The warning is self-healing rather than permanent noise: the guard runs in PersistentPreRunE 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. ## Coverage `TestClassifyVersionWitness` pins the production string, plain and v-prefixed releases, release candidates, build metadata, the pre-1.0 side of each, empty, and garbage. `TestVersionWitnessRoundTrip` pins the contract the guard depends on — everything bd can write, including `Version` itself, bd reads back and recognizes as current, which also holds the writer inside the witness reader's bounded size. `TestLegacyUpgradeGuardStillRefusesPreOneWorkspaces` and the missing- witness test prove the cross-era guard did not relax. Red before the parse fix on 15 cases across 5 tests. Refs GH#5603. That report reaches the same conclusion from a different writer — Homebrew stamps `HEAD-<shortsha>` into `main.Version` for `--HEAD` installs — and the unknown era admits those workspaces too (`TestLegacyUpgradeGuardAdmitsBrewHeadStamp`). It does not silence their warning: a HEAD stamp is rewritten identically every run, so it never heals. Silencing it wants the shape recognizer in GH#5625 (anisoptera), which this deliberately does not duplicate; the two changes compose. Co-Authored-By: Claude <noreply@anthropic.com> * fix(upgrade-guard): warn+open on a present-but-blank version witness The legacy-upgrade guard's witness reader collapsed a present-but-blank `.local_version` (0-byte or whitespace-only, e.g. from an interrupted or disk-full best-effort write) into the same ("", false) result as a genuinely missing witness. In server mode with a local Dolt root the `if ok` reader-gate then routed present-blank onto the missing->refuse path, hard-refusing a possibly-current workspace as "legacy Dolt server workspace" with no self-heal -- the exact false-refusal class this PR removes, left open for the blank-witness shape. Make legacyUpgradeVersionWitness report presence independently of blank contents: a present, bounded, regular witness returns ("", true) so the guard classifies it witnessEraUnknown and warns-and-opens (matching the present-but-unparseable case and the documented upgrading.md contract), while a missing, non-regular, or oversized witness stays ("", false) and still refuses, preserving the pre-1.0 guard safety invariant. Adds guard-level tests for present-blank (0-byte / newline-only / spaces+tabs) -> warn+open and missing -> refuse, plus a reader-contract test pinning present-blank->present and missing/oversized->absent. Addresses the maintainer review's one major finding (Codex). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Julian Knutsen <ci@beads.test> Co-authored-by: Claude <noreply@anthropic.com>