fix(recovery): never discard local work, and deliver the commits it reports #148

Merged

james.maes merged 6 commits from fix/147-recovery-local-work into main 2026-11-10 22:55:33 +00:00

Conversation 2 Commits 6 Files Changed 10

+2230 -75

james.maes commented 2026-11-10 19:47:38 +00:00

Owner

Copy link

Copy source

What

• Applying a path recovery to an ordinary, checked-out clone now fast-forwards it to origin's tip first, after proving that loses nothing. Uncommitted changes to tracked files, or commits origin does not have, stop it with a message and change nothing. Files that left the tree are removed.
• A recovery-managed repository is only ever fast-forwarded.
• The pending-checkout marker is set for the whole apply.
• The index is written from the commit; attribute files are materialized first; when recoveries are merged the newer choice for a path wins.

Why

Closes #147.

"Resolve" and --sanitize-paths were built for a clone that was fetched but never checked out. Offered on an ordinary clone whose pull found invalid incoming paths, the apply rewrote the commit already checked out with force (overwriting uncommitted edits), delivered none of the incoming commits although the row said Done, and left the next pull free to move the branch off local commits.

Behaviour changes worth a release note

• Resolve on an already checked-out repository can now fail with "has uncommitted changes" or "has local commits"; before, it silently overwrote the former and later dropped the latter.
• After a recovery, git status shows only the renamed and skipped paths as changed.
• A later rename of a path that was skipped before now takes effect (and the reverse).

How it was tested

• gclo.Engine.Tests: 714 passed (was 703). New RecoveryLocalWorkTests: an ordinary clone that is clean and behind ends at origin's tip with changed, removed and renamed files right; uncommitted changes and local commits are refused and survive; a branch with no upstream; an apply that fails part-way leaves the marker and the next pull re-reports; a recovery-managed clone with a local commit is not moved; the index after an apply; attribute files applying to files that sort before them; skip-then-rename and rename-then-skip.
• gclo.Cli.Tests: 142 passed. Line coverage 100% on the three covered assemblies. Zero-warning x64 build with locked restore; dotnet format --verify-no-changes clean.

Not run: the UI suite (no shell or view-model change). Not in this pull request, tracked for the next one: the recovery planner rejecting a renamed entry inside a renamed folder, and --sanitize-paths suggestions that collide.

Checklist

• Commits are signed and signed off (git commit -S -s)
• Zero build warnings; format and lint are clean
• Tests added or updated; the 100% coverage gate still passes
• Docs updated for user-visible changes; threat model updated if the attack surface changed (README and docs/CLI.md; no attack-surface change)
• No secrets, credentials, or personal data
• New dependencies are justified below, or none were added (none)
• Substantial AI assistance, if any, is noted here: written with Claude Code (Claude Fable 5.2)

🤖 Generated with Claude Code

## What - Applying a path recovery to an ordinary, checked-out clone now fast-forwards it to origin's tip first, after proving that loses nothing. Uncommitted changes to tracked files, or commits origin does not have, stop it with a message and change nothing. Files that left the tree are removed. - A recovery-managed repository is only ever fast-forwarded. - The pending-checkout marker is set for the whole apply. - The index is written from the commit; attribute files are materialized first; when recoveries are merged the newer choice for a path wins. ## Why Closes #147. "Resolve" and `--sanitize-paths` were built for a clone that was fetched but never checked out. Offered on an ordinary clone whose pull found invalid incoming paths, the apply rewrote the commit already checked out with force (overwriting uncommitted edits), delivered none of the incoming commits although the row said Done, and left the next pull free to move the branch off local commits. ## Behaviour changes worth a release note - Resolve on an already checked-out repository can now fail with "has uncommitted changes" or "has local commits"; before, it silently overwrote the former and later dropped the latter. - After a recovery, `git status` shows only the renamed and skipped paths as changed. - A later rename of a path that was skipped before now takes effect (and the reverse). ## How it was tested - `gclo.Engine.Tests`: 714 passed (was 703). New `RecoveryLocalWorkTests`: an ordinary clone that is clean and behind ends at origin's tip with changed, removed and renamed files right; uncommitted changes and local commits are refused and survive; a branch with no upstream; an apply that fails part-way leaves the marker and the next pull re-reports; a recovery-managed clone with a local commit is not moved; the index after an apply; attribute files applying to files that sort before them; skip-then-rename and rename-then-skip. - `gclo.Cli.Tests`: 142 passed. Line coverage 100% on the three covered assemblies. Zero-warning x64 build with locked restore; `dotnet format --verify-no-changes` clean. Not run: the UI suite (no shell or view-model change). Not in this pull request, tracked for the next one: the recovery planner rejecting a renamed entry inside a renamed folder, and `--sanitize-paths` suggestions that collide. ## Checklist - [x] Commits are signed and signed off (`git commit -S -s`) - [x] Zero build warnings; format and lint are clean - [x] Tests added or updated; the 100% coverage gate still passes - [x] Docs updated for user-visible changes; threat model updated if the attack surface changed (README and `docs/CLI.md`; no attack-surface change) - [x] No secrets, credentials, or personal data - [x] New dependencies are justified below, or none were added (none) - [x] Substantial AI assistance, if any, is noted here: written with Claude Code (Claude Fable 5.2) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes commented 2026-11-10 20:46:58 +00:00

Author

Owner

Copy link

Copy source

Two independent reviews of this pull request said do not merge, and both were right.

• Round 1 (first commit): converting an ordinary clone overwrote untracked files, and an interrupted apply reused the pending-checkout marker, whose handler force-checks-out as if nothing were the user's.
• Round 2 (the redesign): a local commit that had been synced once was dropped when origin moved on; repositories managed by the released build stopped updating, because the new status check read their tree as uncommitted changes; an interrupted apply still let the next sync overwrite uncommitted edits; skipped case-twins, submodules and symlinks read as user changes.

The lesson taken: rewriting a clone that someone checked out cannot be made safe by checks. The third commit (now pushed) does the simple thing instead:

• A clone that was checked out normally is never rewritten. Resolve and --sanitize-paths refuse it with a message that says how to get a fresh clone with the renames; nothing in it changes.
• Managed and never-checked-out repositories keep the release's semantics (gclo writes the tree; the docs say read-only mirror), with fixes: the branch never moves off a commit made in the repository (it follows a rewritten origin only when the reflog shows the current commit was once origin's tip); the branch moves after validation; what would fail half-way is found before anything changes; an interrupted apply is completed by the next pull; the index is written; attribute files go first; the newer choice wins on merge; an empty clone can be resolved.

In-place conversion of a pristine checked-out mirror would be a feature of its own; it is not attempted here.

Gates on the third commit: 733 engine tests, 142 CLI tests, 100% line coverage, zero-warning locked build, format clean. A third review is running before this is merged.

Two independent reviews of this pull request said **do not merge**, and both were right. - **Round 1** (first commit): converting an ordinary clone overwrote untracked files, and an interrupted apply reused the pending-checkout marker, whose handler force-checks-out as if nothing were the user's. - **Round 2** (the redesign): a local commit that had been synced once was dropped when origin moved on; repositories managed by the released build stopped updating, because the new status check read their tree as uncommitted changes; an interrupted apply still let the next sync overwrite uncommitted edits; skipped case-twins, submodules and symlinks read as user changes. The lesson taken: rewriting a clone that someone checked out cannot be made safe by checks. The third commit (now pushed) does the simple thing instead: - **A clone that was checked out normally is never rewritten.** Resolve and `--sanitize-paths` refuse it with a message that says how to get a fresh clone with the renames; nothing in it changes. - **Managed and never-checked-out repositories keep the release's semantics** (gclo writes the tree; the docs say read-only mirror), with fixes: the branch never moves off a commit made in the repository (it follows a rewritten origin only when the reflog shows the current commit was once origin's tip); the branch moves after validation; what would fail half-way is found before anything changes; an interrupted apply is completed by the next pull; the index is written; attribute files go first; the newer choice wins on merge; an empty clone can be resolved. In-place conversion of a pristine checked-out mirror would be a feature of its own; it is not attempted here. Gates on the third commit: 733 engine tests, 142 CLI tests, 100% line coverage, zero-warning locked build, format clean. A third review is running before this is merged.

james.maes referenced this pull request 2026-11-10 20:53:38 +00:00

fix(recovery): plans are checked by where paths end up; suggestions are valid #150

james.maes force-pushed fix/147-recovery-local-work from 9f52a3a62e to a5142010c8 2026-11-10 21:56:28 +00:00 Compare

james.maes force-pushed fix/147-recovery-local-work from a5142010c8 to 332c22c96e 2026-11-10 22:25:44 +00:00 Compare

james.maes added 6 commits 2026-11-10 22:55:30 +00:00

fix(recovery): never discard local work, and deliver the commits it reports ... 6112b38ef7

"Resolve" and --sanitize-paths were built for a clone that was fetched but
never checked out. They are also offered when a pull into an ordinary,
checked-out clone finds Windows-invalid paths in the incoming commits, and
there the apply wrote the tree of the commit already checked out, with
force: uncommitted edits were overwritten, the incoming commits were not
delivered although the row said Done, and the next pull moved the branch to
origin's tip without checking that local commits were contained in it.

- Applying a recovery to an ordinary clone now goes to origin's tip, after
  proving that loses nothing: uncommitted changes to tracked files, or
  commits origin does not have, stop it with the advice the ordinary pull
  gives, and nothing is changed. Files that left the tree are removed.
- A recovery-managed repository is only ever fast-forwarded.
- The pending-checkout marker is set for the whole apply, so an apply that
  fails part-way is completed or re-reported by the next pull instead of
  being called up to date.
- The index is written from the commit, so git reports only the renamed
  and skipped paths as changed instead of every file as deleted.
- Attribute files are materialized first and apply to the whole tree.
- When recoveries are merged, the newer choice for a path wins whatever
  kind it is: a rename replaces a stored skip, a skip a stored rename.

Closes #147

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

fix(recovery): redesign the apply so a checked-out clone keeps its work ... 02817e7acc

The independent review of this branch found that converting an ordinary
clone into a recovery-managed one opened new ways to lose local work: an
untracked file where a file was about to be written was overwritten, and an
apply that failed part-way reused the pending-checkout marker, whose handler
treats the repository as never checked out and force-checks-out over it.

The apply now follows one set of rules for every repository:

- Which commit it writes is decided in one place. The branch only moves to
  origin's tip when that drops no commit; a managed repository still at the
  commit gclo last wrote follows a rewritten origin; local commits on top of
  origin are left alone; diverged histories are refused. The move happens
  after validation, so an uncovered invalid path leaves the branch where it
  was.
- Before anything changes, an ordinary clone and a managed repository whose
  last apply finished are checked for local work: uncommitted changes to
  tracked files, and files gclo did not write and git does not track
  sitting where a file is about to be written (an ignored file is replaced,
  as git does). A managed repository gets this on every later pull too.
- The pending marker is no longer set on a repository that was checked out.
  Before the first destructive step the recovery file is saved without a
  materialized tip, listing everything that was and will be on disk; an
  interrupted apply is then completed by the next pull through the same
  code, and what left the tree is still cleaned up.
- A config flag records that a repository is recovery-managed, so deleting
  a corrupt recovery file (as its message advises) no longer makes the
  renamed tree look like uncommitted changes.
- An empty clone whose origin's first commit has invalid paths can be
  resolved; resolving a clone that has nothing new on origin is refused
  instead of converting it for nothing.
- A folder without files where a file belongs is replaced; one with files
  is refused before anything changes.

Refs #147

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

fix(recovery): never rewrite a checked-out clone; keep managed trees simple ... af43cc95fc

Two independent reviews of this branch found that rewriting a clone someone
checked out cannot be made safe by checks: untracked files, files git was
told to ignore changes in, an apply interrupted and edited since, and git's
own view of a renamed tree each defeated one attempt, and the checks added
for managed repositories stopped repositories managed by the released build
from updating.

So the rule is the simple one:

- A clone that was checked out normally is never rewritten. Resolve and
  --sanitize-paths refuse it with a message that says how to get a fresh
  clone with the renames; nothing in it changes. This replaces the behaviour
  this branch set out to fix (the tree of the commit already checked out
  was rewritten with force, and later pulls moved the branch off local
  commits).
- A repository gclo manages, or one that was never checked out, keeps the
  semantics it has in the release: gclo writes its tree, and the
  documentation says to treat it as a read-only mirror.

With these fixes for managed repositories, each with a regression test:

- The branch never moves off a commit made in the repository. It follows a
  rewritten origin only when the current commit is one origin's branch
  itself once pointed at, shown by the remote-tracking branch's reflog; a
  local commit that was synced once is not mistaken for that. Local commits
  on top of origin are left alone instead of being moved back.
- The branch moves after validation, so an uncovered invalid path leaves it
  where it was.
- What would fail half-way (an unusable destination, a link on the way, a
  file where a folder is needed) is found before anything changes.
- An interrupted apply is completed by the next pull, through a recovery
  file saved without a materialized tip before the first destructive step.
  The pending-checkout marker is not used for a repository that has a tree.
- A config flag, set once a recovery has been read or written, keeps a
  repository known as managed when its recovery file is deleted as the
  message for a corrupt one advises. An unreadable file in an ordinary
  clone sets nothing.
- The index is written, attribute files are materialized first, the newer
  choice wins when recoveries merge, and an empty clone whose origin's
  first commit has invalid paths can be resolved.

Refs #147

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

fix(recovery): address the third independent review ... a114d10a32

- An empty repository counts as never checked out only while its index is
  empty and no file sits where the repository has one; a branch started
  with 'checkout --orphan' in a checked-out clone is refused like any
  ordinary clone.
- What counts as origin's commit: reflog entries a push wrote do not; a
  branch that tracks another local branch has no record of origin.
- A pull that has nothing to write writes nothing: commits made on top
  with origin unchanged leave every other file alone; a branch moved back
  by hand is brought to origin's tip again.
- A repository written by the released build (commits, no index) whose
  recovery file was lost can be resolved again, as the message advises.
- A clone that was never checked out follows origin by the same rule as
  everything else: a commit made in it by hand is not dropped.
- Found before anything changes: another git process at work, a folder
  with someone's files where a file is needed, a link on the way to a file
  that is to be removed. A file of gclo's own is recognised whatever its
  case on a file system that ignores case.
- A file that left the tree, where a folder of the user's now sits, leaves
  the folder; a refused folder removal has touched nothing in it.
- The managed marker is read from the repository's own configuration only;
  a refused apply leaves the configuration as it was. Threat model updated.
- The CLI keeps the list of invalid paths on a repository whose
  sanitization failed.

Refs #147

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

fix(recovery): address the re-check of the review fixes ... 52adc1a450

- A branch ahead of what was written counts as 'commits made here' only
  when origin has nothing beyond what was written. A pull of the released
  build that failed after it moved the branch leaves the branch at
  origin's commit and the tree behind it; that tree is now written, and a
  new invalid path is reported again instead of 'nothing to do'.
- A tree is taken for one gclo wrote without an index only in a
  repository gclo cloned (it carries gclo's checkout marker). A clone made
  by git without a checkout is not.
- Where nothing records that gclo wrote the tree (never checked out,
  empty, or recognised only by its shape), a file already at a destination
  is replaced only when it is, byte for byte, what the current commit has
  there. A file a person put there or changed stops the apply.
- The pull recognises a tree written without an index, so a repository of
  the released build whose recovery file was lost reports its paths again
  and can be resolved from the app and with --sanitize-paths.
- A pull does not check out over a branch that has no commits yet when
  files are staged in the folder ('checkout --orphan' in a checked-out
  clone).
- What counts as origin's commit is what a clone, fetch, or pull recorded;
  any other entry on the remote-tracking branch does not.
- The checkout marker is read from the repository's own configuration
  only. Messages: a stale index.lock, a branch that tracks a local branch.

Refs #147

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

fix(recovery): a tree with nothing to rename is an ordinary clone ... 0324e58575

A repository without an index was taken for one path recovery wrote
whenever gclo had cloned it. An ordinary clone whose index was deleted and
that has no path to rename then went through the hand-written apply on a
pull, was marked as managed for good, and had later edits overwritten.

The pull now takes that route only when the current commit holds a path
that cannot be created here. Anything else takes the ordinary pull, which
leaves local changes alone and marks nothing.

Refs #147

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes force-pushed fix/147-recovery-local-work from 111a202065 to 0324e58575 2026-11-10 22:55:30 +00:00 Compare

james.maes commented 2026-11-10 22:55:31 +00:00

Author

Owner

Copy link

Copy source

Independent reviews: five rounds, final verdict MERGE

This change went through five rounds of independent, read-only review. Each of the first four found real problems, which is why it took three designs and several follow-up commits. Summary:

Round	Verdict	What it found	What changed
1	do not merge	Untracked files overwritten; an interrupted apply reused the "never checked out" marker and a later pull force-checked-out over local work.	Second design: intent recorded before the first destructive step, a separate "managed" marker.
2	do not merge	A local commit that had been synced once could be dropped; repositories written by the released build stopped updating; an interrupted apply still overwrote edits.	Third design: an ordinary checked-out clone is never rewritten; managed repositories keep "gclo owns the tree"; the branch moves by fast-forward or by the remote-tracking reflog.
3	merge after fixes (10 findings)	Unborn HEAD taken as proof nothing was checked out; a pushed commit taken for origin's; a branch tracking a local branch; late failures that could be found up front; a dead end when the released build's recovery file is lost; a commit on top causing a rewrite.	All addressed, with a regression test each.
4	do not merge as is (2 new, from the fixes)	A failed pull of the released build mistaken for "commits made here" (repository silently stopped updating); any repository without an index taken for one gclo wrote.	"Built on" only when origin has nothing new; shape recognition requires gclo's own checkout marker; a content check wherever nothing records that gclo wrote the tree.
5	do not merge as is (1 new), then merge	The new pull route converted an index-less ordinary clone with nothing to rename into a managed one.	The route is taken only when the current commit has a path that cannot be created here. Confirmed by the reviewer end to end.

The reviewer worked from copies of the source, wrote its own probe tests for every scenario (including states the released 1.1.0 build leaves behind, reconstructed from its source), and ran mutation checks on the new conditions: 13 of 13 on the final logic were killed except two that only differ in which configuration level a marker is read from, which cannot be tested without touching the machine's git configuration.

Gates on the rebased branch: 977 engine and 212 CLI tests, 100% line coverage on the three gated assemblies, zero warnings, format clean.

Known and deliberately left for the interrupted-clone work:

• A pull into an empty clone still force-checks out over files a person put into the folder (the path-recovery route does not).
• A first checkout that was killed after the branch was created reports "up to date" over an empty or partial tree until origin moves.
• A clone whose index was deleted fails an update with libgit2's raw conflict text; the message should name git reset.

Accepted residuals: after the remote-tracking reflog has expired, the oldest entry's "from" side is trusted; a hand-run git fetch . x:refs/remotes/origin/x counts as a fetch.

## Independent reviews: five rounds, final verdict MERGE This change went through five rounds of independent, read-only review. Each of the first four found real problems, which is why it took three designs and several follow-up commits. Summary: | Round | Verdict | What it found | What changed | | --- | --- | --- | --- | | 1 | do not merge | Untracked files overwritten; an interrupted apply reused the "never checked out" marker and a later pull force-checked-out over local work. | Second design: intent recorded before the first destructive step, a separate "managed" marker. | | 2 | do not merge | A local commit that had been synced once could be dropped; repositories written by the released build stopped updating; an interrupted apply still overwrote edits. | Third design: an ordinary checked-out clone is never rewritten; managed repositories keep "gclo owns the tree"; the branch moves by fast-forward or by the remote-tracking reflog. | | 3 | merge after fixes (10 findings) | Unborn HEAD taken as proof nothing was checked out; a pushed commit taken for origin's; a branch tracking a local branch; late failures that could be found up front; a dead end when the released build's recovery file is lost; a commit on top causing a rewrite. | All addressed, with a regression test each. | | 4 | do not merge as is (2 new, from the fixes) | A failed pull of the released build mistaken for "commits made here" (repository silently stopped updating); any repository without an index taken for one gclo wrote. | "Built on" only when origin has nothing new; shape recognition requires gclo's own checkout marker; a content check wherever nothing records that gclo wrote the tree. | | 5 | do not merge as is (1 new), then **merge** | The new pull route converted an index-less ordinary clone with nothing to rename into a managed one. | The route is taken only when the current commit has a path that cannot be created here. Confirmed by the reviewer end to end. | The reviewer worked from copies of the source, wrote its own probe tests for every scenario (including states the released 1.1.0 build leaves behind, reconstructed from its source), and ran mutation checks on the new conditions: 13 of 13 on the final logic were killed except two that only differ in which configuration level a marker is read from, which cannot be tested without touching the machine's git configuration. Gates on the rebased branch: 977 engine and 212 CLI tests, 100% line coverage on the three gated assemblies, zero warnings, format clean. Known and deliberately left for the interrupted-clone work: - A pull into an empty clone still force-checks out over files a person put into the folder (the path-recovery route does not). - A first checkout that was killed after the branch was created reports "up to date" over an empty or partial tree until origin moves. - A clone whose index was deleted fails an update with libgit2's raw conflict text; the message should name `git reset`. Accepted residuals: after the remote-tracking reflog has expired, the oldest entry's "from" side is trusted; a hand-run `git fetch . x:refs/remotes/origin/x` counts as a fetch.

james.maes merged commit e80bbada37 into main 2026-11-10 22:55:33 +00:00

james.maes referenced this pull request 2026-11-11 02:09:06 +00:00

Clones that were empty, cut off, or still ending are handled safely by the next sync #174

james.maes referenced this pull request 2026-11-11 02:12:10 +00:00

fix(clone): clones that were empty, cut off, or still ending are handled safely by the next sync #175

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

.NET

Pull requests that update .NET code

bug

Something isn't working

deferred

Agreed follow-up, not scheduled

dependencies

Pull requests that update a dependency file

documentation

Improvements or additions to documentation

duplicate

This issue or pull request already exists

enhancement

New feature or request

github_actions

Pull requests that update GitHub Actions code

good first issue

Good for newcomers

help wanted

Extra attention is needed

invalid

This doesn't seem right

needs-owner

Blocked on an action only the owner can take

priority

Owner-flagged: work this first

quality

Standards, simplicity, enforcement

question

Further information is requested

refactor

Restructuring without behavior change

release

Release pipeline / distribution

wontfix

This will not be worked on

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

forge-recovery (Forge Recovery) james.maes (James Maes)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/gclo#148