a73x/git-collab

Watch

Fork 0

Strip legacy compatibility once every repo is migrated #35

New issue

Closed

opened 2026-08-09 19:15:09 +00:00 by a73x · 5 comments

a73x commented

2026-08-09 19:15:09 +00:00

(Migrated from git.a73x.sh)

We are the only consumer. Once every repo holding collab data is on the current format, all the backward-compatibility handling accumulated during pre-release is dead weight and should go.

The surface, excluding genuine interop:

Ref-layout migration itself: migrate_patch_layout, resume_interrupted_migrations, and the three shapes they handle (bare , interim /r/, target /rev/).
event.rs:65 - legacy signature round-tripping, kept so sync does not reject refs written by older versions.
event.rs:98, state.rs:635, state.rs:656 - reviews with revision: None, written before reviews were revision-scoped, and the vote-dedup special cases that exist to avoid deleting someone else's legacy review.
patch.rs:28, state.rs:611 - the ''-means-unknown convention for Revision.commit, for patches whose revisions recorded no commit, and the dedup that skips them.
state.rs:230 - PatchState.base_commit as Option for old patches, plus the effective_base search-back fallback that exists for revisions with no recorded base.
PatchState.branch accepting either an OID or a branch name (state.rs:436).
state.rs:990 - the singular-string relates_to deserializer, added today for the Vec transition.
PatchState.labels serde(default), added today.
The legacy-fallback tests in collab_test.rs that only pass via create_branch_patch's placeholder commit.

NOT to remove: the RSA key-format handling in server/ssh/auth.rs (ssh-rsa vs rsa-sha2-256/512). That is interop with OpenSSH clients, not with old git-collab data.

SEQUENCING - doing this early destroys data silently:

Land the OID revision-ref rename with its three-shape migration (c65ac0e2).
Merge and make install.
Sync so the local repo migrates and pushes the new layout, then DELETE the stale bare refs from both remotes. Confirmed today: origin holds 5 bare patch refs, the original layout - so the remotes are further behind than the local repo.
Verify no repo anywhere still holds a legacy shape, including any clone on another machine.
Only then strip.

Step 3 is the one that is easy to forget. A remote left holding bare refs becomes unreadable the moment the migration code is gone, and there is no error message that would explain why.

Migrated from git-collab issue e5096ffcd17d34bf771d28acbc81175b46d6f106.

Original author: a73x dev@a73x.sh.

Close reason: merged in patch bf948322

We are the only consumer. Once every repo holding collab data is on the current format, all the backward-compatibility handling accumulated during pre-release is dead weight and should go. The surface, excluding genuine interop: - Ref-layout migration itself: migrate_patch_layout, resume_interrupted_migrations, and the three shapes they handle (bare <id>, interim <id>/r/<n>, target <id>/rev/<oid>). - event.rs:65 - legacy signature round-tripping, kept so sync does not reject refs written by older versions. - event.rs:98, state.rs:635, state.rs:656 - reviews with revision: None, written before reviews were revision-scoped, and the vote-dedup special cases that exist to avoid deleting someone else's legacy review. - patch.rs:28, state.rs:611 - the ''-means-unknown convention for Revision.commit, for patches whose revisions recorded no commit, and the dedup that skips them. - state.rs:230 - PatchState.base_commit as Option for old patches, plus the effective_base search-back fallback that exists for revisions with no recorded base. - PatchState.branch accepting either an OID or a branch name (state.rs:436). - state.rs:990 - the singular-string relates_to deserializer, added today for the Vec<String> transition. - PatchState.labels serde(default), added today. - The legacy-fallback tests in collab_test.rs that only pass via create_branch_patch's placeholder commit. NOT to remove: the RSA key-format handling in server/ssh/auth.rs (ssh-rsa vs rsa-sha2-256/512). That is interop with OpenSSH clients, not with old git-collab data. SEQUENCING - doing this early destroys data silently: 1. Land the OID revision-ref rename with its three-shape migration (c65ac0e2). 2. Merge and make install. 3. Sync so the local repo migrates and pushes the new layout, then DELETE the stale bare <id> refs from both remotes. Confirmed today: origin holds 5 bare <id> patch refs, the original layout - so the remotes are further behind than the local repo. 4. Verify no repo anywhere still holds a legacy shape, including any clone on another machine. 5. Only then strip. Step 3 is the one that is easy to forget. A remote left holding bare refs becomes unreadable the moment the migration code is gone, and there is no error message that would explain why. --- Migrated from git-collab issue `e5096ffcd17d34bf771d28acbc81175b46d6f106`. Original author: a73x <dev@a73x.sh>. Close reason: merged in patch bf948322

a73x commented

2026-08-10 09:03:35 +00:00

(Migrated from git.a73x.sh)

Step 3's order is wrong, and the push does not merely lag — it is rejected.

The remote holds refs/collab/patches/<id> as a ref; the new layout needs refs/collab/patches/<id>/events, a directory of the same name. Git cannot hold both. Reproduced against a local bare remote:

remote: error: cannot lock ref '.../<id>/events': '.../<id>' exists; cannot create '.../<id>/events'
 ! [remote rejected] .../<id>/events (refname conflict)

So the deletion has to come first: sync cannot push the new layout for those 5 patches until the bare refs are gone. Reordered, step 3 is: delete the stale bare refs from both remotes, then sync.

Three things make this hard to diagnose, all verified:

git push --dry-run reports [new reference] for a push the server then rejects. The obvious way to check gives a false all-clear.
Fetch already reads the legacy layout (4471ef75), so reads look completely healthy. Only writes are wedged, and nothing surfaces that until a push.
print_sync_summary (sync.rs:276) ends with "Run collab sync --remote X again to retry", which for a refname conflict can never succeed — the state is permanent until someone deletes remote refs. It also names a binary that does not exist; it should be git-collab sync.

Worth doing before the strip: teach sync to recognise a refname conflict under refs/collab/patches/ and say what it actually means, rather than advising a retry that cannot work.

Step 3's order is wrong, and the push does not merely lag — it is **rejected**. The remote holds `refs/collab/patches/<id>` as a ref; the new layout needs `refs/collab/patches/<id>/events`, a *directory* of the same name. Git cannot hold both. Reproduced against a local bare remote: ``` remote: error: cannot lock ref '.../<id>/events': '.../<id>' exists; cannot create '.../<id>/events' ! [remote rejected] .../<id>/events (refname conflict) ``` So the deletion has to come *first*: sync cannot push the new layout for those 5 patches until the bare refs are gone. Reordered, step 3 is: delete the stale bare refs from both remotes, then sync. Three things make this hard to diagnose, all verified: - `git push --dry-run` reports `[new reference]` for a push the server then rejects. The obvious way to check gives a false all-clear. - Fetch already reads the legacy layout (4471ef75), so reads look completely healthy. Only writes are wedged, and nothing surfaces that until a push. - `print_sync_summary` (sync.rs:276) ends with "Run `collab sync --remote X` again to retry", which for a refname conflict can never succeed — the state is permanent until someone deletes remote refs. It also names a binary that does not exist; it should be `git-collab sync`. Worth doing before the strip: teach sync to recognise a refname conflict under `refs/collab/patches/` and say what it actually means, rather than advising a retry that cannot work.

a73x commented

2026-08-10 10:22:24 +00:00

(Migrated from git.a73x.sh)

Step 3 is done for origin, 2026-08-10.

The five stale bare refs were deleted and sync then went clean. Remote and local now agree exactly: 27 patch refs each, 9 events + 18 rev/<oid>, zero bare <id>.

Order confirmed as the comment above predicted: deletion first, then sync. The push retried from the pending store and succeeded on the same run.

Correction to my earlier note in this issue: a73x does not hold stale refs — it holds no collab refs at all, and its main is at 2e84dece while local is f9785c8. So it is not blocked, it is simply unsynced. Step 4 still needs it, and any other clone, checked before the strip.

The diagnostic that made this legible is patch bf187d71 (866b28c5), merged.

Step 3 is done for `origin`, 2026-08-10. The five stale bare refs were deleted and `sync` then went clean. Remote and local now agree exactly: 27 patch refs each, 9 `events` + 18 `rev/<oid>`, zero bare `<id>`. Order confirmed as the comment above predicted: deletion first, then sync. The push retried from the pending store and succeeded on the same run. Correction to my earlier note in this issue: `a73x` does **not** hold stale refs — it holds no collab refs at all, and its `main` is at 2e84dece while local is f9785c8. So it is not blocked, it is simply unsynced. Step 4 still needs it, and any other clone, checked before the strip. The diagnostic that made this legible is patch bf187d71 (866b28c5), merged.

a73x commented

2026-08-10 16:19:10 +00:00

(Migrated from git.a73x.sh)

Step 4 is satisfied for every known repo, 2026-08-10. Audited all three for legacy ref shapes — bare <id> and the interim <id>/r/<n>:

repo	events	rev/
local	10	19
origin	10	19
a73x	10	19

All three now agree exactly. a73x had no collab refs at all until today's sync, so it was never on a legacy shape; origin was cleared by the deletion recorded in the comment above.

The archive namespace holds only archive/issues/<id>, whose shape never changed, and no archive/patches exists on either remote — so archived patches are not a second migration surface today. Worth re-checking before the strip if any patch gets closed in the meantime, since close moves the whole subtree.

Remaining caveat, unchanged: this covers the repos I can reach. A clone on another machine that has not synced since the migration would still hold bare refs, and after the strip it becomes unreadable with no error message that would explain why. That risk is now the only thing between here and step 5.

Not starting the strip yet — it touches state.rs, patch.rs and event.rs, which three in-flight patches are editing. Queued behind them deliberately.

**Step 4 is satisfied for every known repo, 2026-08-10.** Audited all three for legacy ref shapes — bare `<id>` and the interim `<id>/r/<n>`: | repo | events | rev/<oid> | bare | interim r/n | |---|---|---|---|---| | local | 10 | 19 | 0 | 0 | | origin | 10 | 19 | 0 | 0 | | a73x | 10 | 19 | 0 | 0 | All three now agree exactly. `a73x` had no collab refs at all until today's sync, so it was never on a legacy shape; `origin` was cleared by the deletion recorded in the comment above. The archive namespace holds only `archive/issues/<id>`, whose shape never changed, and no `archive/patches` exists on either remote — so archived patches are not a second migration surface today. Worth re-checking before the strip if any patch gets closed in the meantime, since `close` moves the whole subtree. Remaining caveat, unchanged: this covers the repos I can reach. A clone on another machine that has not synced since the migration would still hold bare refs, and after the strip it becomes unreadable with no error message that would explain why. That risk is now the only thing between here and step 5. Not starting the strip yet — it touches state.rs, patch.rs and event.rs, which three in-flight patches are editing. Queued behind them deliberately.

a73x commented

2026-08-13 09:37:24 +00:00

(Migrated from git.a73x.sh)

Blocking finding from the live server, 2026-08-13. The strip cannot proceed as written without deciding what happens to waystty.

Server upgraded to b2db9cf and git-collab-server migrate run. Result:

claudealong: already current   git-collab: already current
mux: already current           nono: already current
waystty: migrated 11 patches

5 repositories scanned: 1 migrated, 4 already current, 0 could not be migrated.

So step 3 is done for every repo the server hosts, and step 4's audit now covers the deployment as well as the two remotes. But migrating waystty surfaced what those 11 patches actually are:

bare(old): 0    events: 11    rev/<oid>: 0

Zero revision refs, because all 11 patches use conventions this issue proposes to delete. Every revision on every one of them:

{ "number": 1, "commit": "", "tree": "", "base": null, "body": null }

commit: "" — the ''-means-unknown convention (patch.rs:28, state.rs:611), listed in this issue for removal. 11 of 11.
base: null — PatchState.base_commit as Option and the effective_base search-back fallback, also listed. 11 of 11.

These are April 2026 patches, predating revision commits being recorded. There is no data to recover: the commits were never stored, so no migration can pin them and no rev/<oid> ref can exist. The objects may never have been pushed to the server at all.

So stripping the legacy conventions makes 11 real patches unreadable, or readable-but-wrong, depending on how the code fails. The sequencing in this issue assumed legacy ref shapes were the only thing to migrate; legacy field conventions inside otherwise-current refs are a second surface it does not mention, and waystty is entirely made of them.

Options, none of which are free:

Keep the ''-means-unknown and base: None handling permanently. Cheapest, and honest: these patches genuinely have no commit, so a representation for "unknown" is not legacy debt but a real state.
Backfill from the events DAG if the information is recoverable there — it probably is not, since the events never carried it.
Accept the loss for waystty explicitly, having looked at what those 11 patches are worth.

Step 5 should not start until this is decided. Recommend option 1 unless the patches turn out to be worthless, in which case say so on the record rather than discovering it after the strip.

**Blocking finding from the live server, 2026-08-13.** The strip cannot proceed as written without deciding what happens to `waystty`. Server upgraded to b2db9cf and `git-collab-server migrate` run. Result: ``` claudealong: already current git-collab: already current mux: already current nono: already current waystty: migrated 11 patches 5 repositories scanned: 1 migrated, 4 already current, 0 could not be migrated. ``` So step 3 is done for every repo the server hosts, and step 4's audit now covers the deployment as well as the two remotes. But migrating `waystty` surfaced what those 11 patches actually are: ``` bare(old): 0 events: 11 rev/<oid>: 0 ``` **Zero revision refs**, because all 11 patches use conventions this issue proposes to delete. Every revision on every one of them: ```json { "number": 1, "commit": "", "tree": "", "base": null, "body": null } ``` - `commit: ""` — the `''`-means-unknown convention (patch.rs:28, state.rs:611), listed in this issue for removal. 11 of 11. - `base: null` — `PatchState.base_commit` as `Option` and the `effective_base` search-back fallback, also listed. 11 of 11. These are April 2026 patches, predating revision commits being recorded. There is no data to recover: the commits were never stored, so no migration can pin them and no `rev/<oid>` ref can exist. The objects may never have been pushed to the server at all. **So stripping the legacy conventions makes 11 real patches unreadable, or readable-but-wrong, depending on how the code fails.** The sequencing in this issue assumed legacy *ref shapes* were the only thing to migrate; legacy *field* conventions inside otherwise-current refs are a second surface it does not mention, and `waystty` is entirely made of them. Options, none of which are free: 1. Keep the `''`-means-unknown and `base: None` handling permanently. Cheapest, and honest: these patches genuinely have no commit, so a representation for "unknown" is not legacy debt but a real state. 2. Backfill from the events DAG if the information is recoverable there — it probably is not, since the events never carried it. 3. Accept the loss for `waystty` explicitly, having looked at what those 11 patches are worth. Step 5 should not start until this is decided. Recommend option 1 unless the patches turn out to be worthless, in which case say so on the record rather than discovering it after the strip.

a73x commented

2026-08-14 05:41:22 +00:00

(Migrated from git.a73x.sh)

Decision, 2026-08-14: strip what has zero users, keep what still has data depending on it.

The audit after clearing waystty (all five server repos, plus this clone):

claudealong  empty-commit=0  null-base=0
git-collab   empty-commit=0  null-base=5
mux          empty-commit=0  null-base=0
nono         empty-commit=0  null-base=0
waystty      empty-commit=0  null-base=0

Strip — zero remaining users anywhere:

The ''-means-unknown convention for Revision.commit (patch.rs:28, state.rs:611) and the dedup that skips such revisions. waystty was the only holder and its patches are deleted.
migrate_patch_layout, resume_interrupted_migrations and the three ref shapes. Every repo is migrated: local, both remotes, and all five on the server, confirmed by git-collab-server migrate reporting 5 already current and by git-collab refs.
The legacy-fallback tests in collab_test.rs that only pass via create_branch_patch's placeholder commit.
event.rs:65 legacy signature round-tripping, event.rs:98 / state.rs:635 / state.rs:656 reviews with revision: None and the vote-dedup special cases, state.rs:990 the singular-string relates_to deserializer, PatchState.labels serde(default) — subject to the implementer confirming each has no live data, the same way this audit did.

Keep — PatchState.base_commit as Option and the effective_base search-back fallback. 5 live revisions in this repository, from before base was recorded per revision. Removing it breaks interdiffs on our own history.

The reasoning is not only "there is data": "this revision has no recorded base" is a real state, not a transitional artefact. A revision recorded before the field existed genuinely has no base, and no migration can invent one. A representation for unknown is therefore permanent, and calling it legacy debt was my mistake.

Not to remove, unchanged from the issue: the RSA key-format handling in server/ssh/auth.rs. That is interop with OpenSSH clients, not with old git-collab data.

Verification requirement for whoever does this: use git-collab refs and git-collab-server refs --config … — both landed today for exactly this — rather than hand-written awk. Re-run the audit immediately before stripping, not from these numbers, since any sync could introduce a legacy shape from a clone that has not been looked at.

**Decision, 2026-08-14: strip what has zero users, keep what still has data depending on it.** The audit after clearing `waystty` (all five server repos, plus this clone): ``` claudealong empty-commit=0 null-base=0 git-collab empty-commit=0 null-base=5 mux empty-commit=0 null-base=0 nono empty-commit=0 null-base=0 waystty empty-commit=0 null-base=0 ``` **Strip** — zero remaining users anywhere: - The `''`-means-unknown convention for `Revision.commit` (patch.rs:28, state.rs:611) and the dedup that skips such revisions. `waystty` was the only holder and its patches are deleted. - `migrate_patch_layout`, `resume_interrupted_migrations` and the three ref shapes. Every repo is migrated: local, both remotes, and all five on the server, confirmed by `git-collab-server migrate` reporting `5 already current` and by `git-collab refs`. - The legacy-fallback tests in `collab_test.rs` that only pass via `create_branch_patch`'s placeholder commit. - `event.rs:65` legacy signature round-tripping, `event.rs:98` / `state.rs:635` / `state.rs:656` reviews with `revision: None` and the vote-dedup special cases, `state.rs:990` the singular-string `relates_to` deserializer, `PatchState.labels` `serde(default)` — subject to the implementer confirming each has no live data, the same way this audit did. **Keep** — `PatchState.base_commit` as `Option` and the `effective_base` search-back fallback. **5 live revisions in this repository**, from before `base` was recorded per revision. Removing it breaks interdiffs on our own history. The reasoning is not only "there is data": "this revision has no recorded base" is a **real state**, not a transitional artefact. A revision recorded before the field existed genuinely has no base, and no migration can invent one. A representation for unknown is therefore permanent, and calling it legacy debt was my mistake. **Not to remove**, unchanged from the issue: the RSA key-format handling in `server/ssh/auth.rs`. That is interop with OpenSSH clients, not with old git-collab data. Verification requirement for whoever does this: use `git-collab refs` and `git-collab-server refs --config …` — both landed today for exactly this — rather than hand-written `awk`. Re-run the audit **immediately before** stripping, not from these numbers, since any sync could introduce a legacy shape from a clone that has not been looked at.