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

2026-11-10T20:19:26Z

james.maes commented

2026-11-10 20:19:26 +00:00

What

PathRecovery.EffectivePathOf: where a path ends up once the renames of it and of the folders above it are applied, or nowhere when it or a folder above it is skipped.
The Resolve dialog's pre-flight (PathRecoveryPlanner.Build) validates those places instead of each replacement as typed.
Every suggested name the validator gives is itself valid; a reserved device name is fixed in its base name (COM3.txt becomes COM3_.txt, where it used to become COM3.txt_, which is still reserved).
The automatic plan behind --sanitize-paths moved next to the planner (PathRecoveryPlanner.FromSuggestions) and skips what its renames cannot place.

Why

Closes #149.

A renamed entry inside a renamed folder is written with the folder's original name in front, so the pre-flight refused a plan the engine accepts, and no edit to the row could satisfy it. And --sanitize-paths could give two paths the same name, or a name that was itself invalid, and then fail to sanitize the repository.

Behaviour changes worth a release note

Suggested names for reserved device names with an extension changed (COM3.txt to COM3_.txt).
--sanitize-paths now skips paths whose suggested names would collide, where the repository used to fail.

How it was tested

gclo.Engine.Tests: 778 passed (was 745). New PathRecoveryEffectivePathTests; planner tests for nested renames, destinations that collide, entries under a skipped folder, and the automatic plan (collisions at the top level and inside a renamed folder; its result passes the pre-flight); validator tests that every suggestion validates, including an overlong reserved name.
gclo.Cli.Tests: 163 passed (the existing --sanitize-paths tests run through the moved plan). 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 (the dialog's rows are unchanged; they call the planner as before). The app's own row defaults (gclo/PathRecoveryRow.cs) still duplicate the suggestion logic; sharing it is part of the later move of shell logic into view models.

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 (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 - `PathRecovery.EffectivePathOf`: where a path ends up once the renames of it and of the folders above it are applied, or nowhere when it or a folder above it is skipped. - The Resolve dialog's pre-flight (`PathRecoveryPlanner.Build`) validates those places instead of each replacement as typed. - Every suggested name the validator gives is itself valid; a reserved device name is fixed in its base name (`COM3.txt` becomes `COM3_.txt`, where it used to become `COM3.txt_`, which is still reserved). - The automatic plan behind `--sanitize-paths` moved next to the planner (`PathRecoveryPlanner.FromSuggestions`) and skips what its renames cannot place. ## Why Closes #149. A renamed entry inside a renamed folder is written with the folder's original name in front, so the pre-flight refused a plan the engine accepts, and no edit to the row could satisfy it. And `--sanitize-paths` could give two paths the same name, or a name that was itself invalid, and then fail to sanitize the repository. ## Behaviour changes worth a release note - Suggested names for reserved device names with an extension changed (`COM3.txt` to `COM3_.txt`). - `--sanitize-paths` now skips paths whose suggested names would collide, where the repository used to fail. ## How it was tested - `gclo.Engine.Tests`: 778 passed (was 745). New `PathRecoveryEffectivePathTests`; planner tests for nested renames, destinations that collide, entries under a skipped folder, and the automatic plan (collisions at the top level and inside a renamed folder; its result passes the pre-flight); validator tests that every suggestion validates, including an overlong reserved name. - `gclo.Cli.Tests`: 163 passed (the existing `--sanitize-paths` tests run through the moved plan). 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 (the dialog's rows are unchanged; they call the planner as before). The app's own row defaults (`gclo/PathRecoveryRow.cs`) still duplicate the suggestion logic; sharing it is part of the later move of shell logic into view models. ## 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 (`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 added 1 commit 2026-11-10 20:19:26 +00:00

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

Two defects in how rename choices for Windows-invalid paths were prepared
before they reached the engine:

- The Resolve dialog's pre-flight validated each replacement as typed. A
  renamed entry inside a renamed folder is written with the folder's
  original name in front, so the check saw the invalid name and refused a
  plan the engine accepts; only skipping the file got past it.
- --sanitize-paths trusted the suggested names. Two paths can be given the
  same suggestion, and some suggestions were themselves invalid: "x?."
  became "x?", and "COM3.txt" became "COM3.txt_", which is as reserved as
  the name it replaces. The repository then failed to sanitize.

Now:

- PathRecovery.EffectivePathOf says where a path ends up once the renames
  of it and of the folders above it are applied (nowhere, when it or a
  folder above it is skipped). The pre-flight validates those places.
- Every suggested name is put through the rules until it passes, and a
  reserved device name is fixed in its base name ("COM3_.txt").
- The automatic plan moved next to the planner
  (PathRecoveryPlanner.FromSuggestions) and skips what its renames cannot
  place, so its result always passes the pre-flight.

Closes #149

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

james.maes added 1 commit 2026-11-10 20:53:38 +00:00

fix(recovery): folders by original path, linear cost, one policy for clashes 2bf2d1c529

Follow-up to the independent review of this branch.

- The pre-flight dropped a destination whenever another destination lay
  "inside" it, which hid real clashes: a file renamed onto a name that
  another rename uses as a folder passed, and the engine then rejected the
  plan. A renamed path now counts as a folder only when another renamed
  ORIGINAL path lies inside it.
- Two folders given the same name were merged or left out depending on
  whether one of them held another invalid name. They are now always a
  clash: reported by the pre-flight, both skipped by the automatic plan.
- The pairwise comparison took a minute for 50,000 paths, in the dialog's
  click handler too. Folder membership is now a set lookup.
- A replacement that names another folder is a problem, since only the
  name is applied. Problems name the rows they come from and are reported
  once.
- An overlong name is cut between characters, never through one.
- --sanitize-paths prints where each renamed path lands and counts only
  the renames that are applied.
- Tests: the automatic plan against the real engine; file and folder
  clashes; a problem on a folder above a destination (the branch that ends
  the loop); a name that needs three rounds to settle; 50,000 paths.

Refs #149

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

james.maes commented

2026-11-10 20:53:38 +00:00

Independent review of the first commit (read-only, with randomized comparisons against the real engine) said merge after fixes; addressed in the second commit.

Fixed: the pre-flight dropped a destination whenever another destination lay inside it, which hid real clashes (a file renamed onto a name another rename uses as a folder). A renamed path now counts as a folder only when another renamed original path lies inside it.
Fixed: two folders given the same name were merged or left out depending on their contents. They are now always a clash: reported by the pre-flight, both skipped by the automatic plan.
Fixed: the pairwise comparison took about a minute for 50,000 paths, also in the dialog's click handler. Now a set lookup; a test covers 50,000 paths.
Fixed: a replacement naming another folder is reported (only the name is applied); problems name the rows they come from; an overlong name is cut between characters; --sanitize-paths prints where paths land and counts only applied renames.
Tests added: the automatic plan against the real engine for six tree shapes.

Not changed here, tracked for after #148 (same file): the engine's own rename application and EffectivePathOf are two copies that disagree for an empty replacement name, and a pull of a managed repository reports a new invalid path under a renamed folder by its effective path, which a second sanitize then cannot match.

Documented, not fixed: a suggested name that collides with a path the repository already has is only caught by the engine, which fails that repository cleanly.

Gates: 798 engine tests, 164 CLI tests, 100% line coverage, zero-warning locked build, format clean.

Independent review of the first commit (read-only, with randomized comparisons against the real engine) said merge after fixes; addressed in the second commit. - **Fixed:** the pre-flight dropped a destination whenever another destination lay inside it, which hid real clashes (a file renamed onto a name another rename uses as a folder). A renamed path now counts as a folder only when another renamed original path lies inside it. - **Fixed:** two folders given the same name were merged or left out depending on their contents. They are now always a clash: reported by the pre-flight, both skipped by the automatic plan. - **Fixed:** the pairwise comparison took about a minute for 50,000 paths, also in the dialog's click handler. Now a set lookup; a test covers 50,000 paths. - **Fixed:** a replacement naming another folder is reported (only the name is applied); problems name the rows they come from; an overlong name is cut between characters; `--sanitize-paths` prints where paths land and counts only applied renames. - **Tests added:** the automatic plan against the real engine for six tree shapes. Not changed here, tracked for after #148 (same file): the engine's own rename application and `EffectivePathOf` are two copies that disagree for an empty replacement name, and a pull of a managed repository reports a new invalid path under a renamed folder by its effective path, which a second sanitize then cannot match. Documented, not fixed: a suggested name that collides with a path the repository already has is only caught by the engine, which fails that repository cleanly. Gates: 798 engine tests, 164 CLI tests, 100% line coverage, zero-warning locked build, format clean.

james.maes merged commit c4b1f5379a into main

2026-11-10 20:53:40 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/gclo#150