Task 3: Path normalisation (NFC, separators) #3

New issue

Closed

opened 2026-09-10 17:14:46 +00:00 by Cordy · 1 comment

Cordy commented

2026-09-10 17:14:46 +00:00

Owner

Depends on Task 2.

Goal

One canonical path form used everywhere in the engine: forward slashes, no leading or trailing separator, no duplicate separators, NFC-normalised.

Why this is a data-integrity task

macOS returns NFD from the filesystem; Linux and Windows use NFC. A file named café created on a Mac and one created on Linux are different byte strings. Without normalising at the boundary they sync as two separate files forever. This is the most common duplicate-file bug in sync clients.

Files

Create: internal/sync/path.go, internal/sync/path_test.go

Produces

func Normalise(p string) string
func Join(base, name string) string
func Parent(p string) string
func Depth(p string) int

Depth counts path segments and is used later to order creates parent-first and deletes child-first.

Steps

Write the failing test:

package sync

import "testing"

func TestNormalise(t *testing.T) {
	cases := []struct{ in, want string }{
		{"/a/b.txt", "a/b.txt"},
		{"a\\b.txt", "a/b.txt"},
		{"a//b.txt", "a/b.txt"},
		{"./a/b.txt", "a/b.txt"},
		{"a/b/", "a/b"},
		{"", ""},
		{"café.txt", "café.txt"}, // NFD e+combining-acute becomes NFC é
	}
	for _, c := range cases {
		if got := Normalise(c.in); got != c.want {
			t.Errorf("Normalise(%q) = %q, want %q", c.in, got, c.want)
		}
	}
}

func TestNormaliseIsIdempotent(t *testing.T) {
	for _, in := range []string{"a/b.txt", "café.txt", "/x//y/"} {
		once := Normalise(in)
		if twice := Normalise(once); twice != once {
			t.Errorf("not idempotent: %q -> %q -> %q", in, once, twice)
		}
	}
}

func TestParentAndDepth(t *testing.T) {
	if got := Parent("a/b/c.txt"); got != "a/b" {
		t.Errorf("Parent = %q, want a/b", got)
	}
	if got := Parent("a.txt"); got != "" {
		t.Errorf("Parent of top-level = %q, want empty", got)
	}
	if got := Depth("a/b/c.txt"); got != 3 {
		t.Errorf("Depth = %d, want 3", got)
	}
	if got := Depth(""); got != 0 {
		t.Errorf("Depth of empty = %d, want 0", got)
	}
}

Run it, confirm it fails with undefined: Normalise.
Implement. Use golang.org/x/text/unicode/norm — norm.NFC.String(...). Order matters: fix separators first, then normalise. golang.org/x/text is permitted by the arch guard.
Run tests, confirm they pass (including the arch guard).
Commit: git commit -s -m "feat(sync): canonical path normalisation with NFC"

Acceptance criteria

All subtests pass.
Normalise is idempotent for every input.
The arch guard from Task 2 still passes.

Depends on Task 2. ## Goal One canonical path form used everywhere in the engine: forward slashes, no leading or trailing separator, no duplicate separators, NFC-normalised. ## Why this is a data-integrity task macOS returns **NFD** from the filesystem; Linux and Windows use **NFC**. A file named `café` created on a Mac and one created on Linux are *different byte strings*. Without normalising at the boundary they sync as two separate files forever. This is the most common duplicate-file bug in sync clients. ## Files - Create: `internal/sync/path.go`, `internal/sync/path_test.go` ## Produces ```go func Normalise(p string) string func Join(base, name string) string func Parent(p string) string func Depth(p string) int ``` `Depth` counts path segments and is used later to order creates parent-first and deletes child-first. ## Steps - [ ] **Write the failing test:** ```go package sync import "testing" func TestNormalise(t *testing.T) { cases := []struct{ in, want string }{ {"/a/b.txt", "a/b.txt"}, {"a\\b.txt", "a/b.txt"}, {"a//b.txt", "a/b.txt"}, {"./a/b.txt", "a/b.txt"}, {"a/b/", "a/b"}, {"", ""}, {"café.txt", "café.txt"}, // NFD e+combining-acute becomes NFC é } for _, c := range cases { if got := Normalise(c.in); got != c.want { t.Errorf("Normalise(%q) = %q, want %q", c.in, got, c.want) } } } func TestNormaliseIsIdempotent(t *testing.T) { for _, in := range []string{"a/b.txt", "café.txt", "/x//y/"} { once := Normalise(in) if twice := Normalise(once); twice != once { t.Errorf("not idempotent: %q -> %q -> %q", in, once, twice) } } } func TestParentAndDepth(t *testing.T) { if got := Parent("a/b/c.txt"); got != "a/b" { t.Errorf("Parent = %q, want a/b", got) } if got := Parent("a.txt"); got != "" { t.Errorf("Parent of top-level = %q, want empty", got) } if got := Depth("a/b/c.txt"); got != 3 { t.Errorf("Depth = %d, want 3", got) } if got := Depth(""); got != 0 { t.Errorf("Depth of empty = %d, want 0", got) } } ``` - [ ] **Run it, confirm it fails** with `undefined: Normalise`. - [ ] **Implement.** Use `golang.org/x/text/unicode/norm` — `norm.NFC.String(...)`. Order matters: fix separators first, then normalise. `golang.org/x/text` is permitted by the arch guard. - [ ] **Run tests, confirm they pass** (including the arch guard). - [ ] **Commit:** `git commit -s -m "feat(sync): canonical path normalisation with NFC"` ## Acceptance criteria - All subtests pass. - `Normalise` is idempotent for every input. - The arch guard from Task 2 still passes.

Cordy added this to the phase-1-engine milestone

2026-09-10 17:14:46 +00:00

Cordy added the

data-integrity

engine

labels

2026-09-10 17:14:46 +00:00

Cordy commented

2026-09-10 20:19:41 +00:00

Author

Owner

Done

1c42966 feat(sync): canonical path normalisation with NFC

What was built

internal/sync/path.go: Normalise, Join, Parent, Depth per the issue's Produces list
Normalise fixes separators first (\→/, drops empty/. segments), then runs NFC via golang.org/x/text/unicode/norm
internal/sync/path_test.go: the issue's three tests verbatim; the NFD case written with explicit \u escapes (not literal glyphs) per ruling R8, so it can't be silently renormalised into a vacuous test
Added golang.org/x/text v0.34.0 (its own go 1.24.0 directive) so go.mod stays exactly go 1.25 with no toolchain line (R1)

Tests

TestNormalise, TestNormaliseIsIdempotent, TestParentAndDepth, plus Task 2's TestEngineHasNoForbiddenImports all pass
CI run #3 (http://192.168.10.245/Cordy/cairn-desktop/actions/runs/3), linux/arm64, go1.25.5 — green
Reviewer additionally fuzzed Normalise 30s / 4.58M execs against idempotence + separator/NFC invariants — no failures

Acceptance criteria

All subtests pass — verified in CI and locally
Normalise idempotent for every input — issue's 3 cases pass; structurally true (output is always free of \, leading/trailing/duplicate /, . segments, and NFC is idempotent on NFC text); confirmed empirically by the reviewer's fuzz run
Arch guard from Task 2 still passes — TestEngineHasNoForbiddenImports passes; x/text is explicitly permitted by the issue

Rulings

R1: go.mod keeps exactly go 1.25, no toolchain line; newest x/text whose own go directive is ≤ 1.25 → v0.34.0 used
R8: NFD test case uses explicit \u escapes so it genuinely exercises NFD→NFC
R12: exact commit subject, git commit -s, Co-Authored-By: Claude Opus 5 trailer
Ruling (F1): \→/ mapping stands as #3 mandates; binding on phase 2's boundary adapters (#22, #25) to refuse native \ names and detect normalise-collisions before they reach the engine
Ruling (F5): NFC via x/text also rewrites non-NFD names (CJK singletons, 31+ combining marks); phase-2 adapters must keep each entry's native name, never rebuild it from the engine path
Ruling (CV1/CV2, parked): CI-green is the post-push gate (R11), now satisfied; the red step was reproduced independently (removing path.go in a clean clone gives undefined: Normalise, matching the issue's expected failure)

Deferred

F2 (minor): idempotence tested on only 3 inputs, no NFD case in that list
F3 (minor): Join is exported but untested; its base == "" branch is redundant
F4 (minor): .. segments pass through unrejected; needs a ruling on where to reject (likely a phase-2 adapter)

Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI.

**Done** - [1c42966](http://192.168.10.245/Cordy/cairn-desktop/commit/1c4296689c79699661979b106fcdbc450b110943) feat(sync): canonical path normalisation with NFC **What was built** - `internal/sync/path.go`: `Normalise`, `Join`, `Parent`, `Depth` per the issue's Produces list - `Normalise` fixes separators first (`\`→`/`, drops empty/`.` segments), then runs NFC via `golang.org/x/text/unicode/norm` - `internal/sync/path_test.go`: the issue's three tests verbatim; the NFD case written with explicit `\u` escapes (not literal glyphs) per ruling R8, so it can't be silently renormalised into a vacuous test - Added `golang.org/x/text` v0.34.0 (its own `go 1.24.0` directive) so `go.mod` stays exactly `go 1.25` with no `toolchain` line (R1) **Tests** - `TestNormalise`, `TestNormaliseIsIdempotent`, `TestParentAndDepth`, plus Task 2's `TestEngineHasNoForbiddenImports` all pass - CI run #3 (http://192.168.10.245/Cordy/cairn-desktop/actions/runs/3), linux/arm64, go1.25.5 — green - Reviewer additionally fuzzed `Normalise` 30s / 4.58M execs against idempotence + separator/NFC invariants — no failures **Acceptance criteria** - All subtests pass — verified in CI and locally - `Normalise` idempotent for every input — issue's 3 cases pass; structurally true (output is always free of `\`, leading/trailing/duplicate `/`, `.` segments, and NFC is idempotent on NFC text); confirmed empirically by the reviewer's fuzz run - Arch guard from Task 2 still passes — `TestEngineHasNoForbiddenImports` passes; `x/text` is explicitly permitted by the issue **Rulings** - R1: `go.mod` keeps exactly `go 1.25`, no `toolchain` line; newest `x/text` whose own go directive is ≤ 1.25 → v0.34.0 used - R8: NFD test case uses explicit `\u` escapes so it genuinely exercises NFD→NFC - R12: exact commit subject, `git commit -s`, `Co-Authored-By: Claude Opus 5` trailer - Ruling (F1): `\`→`/` mapping stands as #3 mandates; binding on phase 2's boundary adapters (#22, #25) to refuse native `\` names and detect normalise-collisions before they reach the engine - Ruling (F5): NFC via x/text also rewrites non-NFD names (CJK singletons, 31+ combining marks); phase-2 adapters must keep each entry's native name, never rebuild it from the engine path - Ruling (CV1/CV2, parked): CI-green is the post-push gate (R11), now satisfied; the red step was reproduced independently (removing `path.go` in a clean clone gives `undefined: Normalise`, matching the issue's expected failure) **Deferred** - F2 (minor): idempotence tested on only 3 inputs, no NFD case in that list - F3 (minor): `Join` is exported but untested; its `base == ""` branch is redundant - F4 (minor): `..` segments pass through unrejected; needs a ruling on where to reject (likely a phase-2 adapter) _Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI._

Cordy closed this issue

2026-09-10 20:19:45 +00:00