Cordy/cairn-desktop

Fork 0

Task 8: The decision function — three-way truth table #8

New issue

Closed

opened 2026-09-10 17:15:56 +00:00 by Cordy · 1 comment

Cordy commented

2026-09-10 17:15:56 +00:00

Owner

Depends on Task 7. This is the heart of the engine.

Goal

A pure function that, given last-known / local / remote state for one path, returns the operation to perform. No I/O, no mutation — so every row of the truth table is a unit test.

Files

Create: internal/sync/ops.go, internal/sync/decide.go, internal/sync/decide_test.go

Produces

type Op int
const (
	OpNone Op = iota
	OpUpload
	OpDownload
	OpDeleteLocal
	OpDeleteRemote
	OpConflict
	OpUpdateState // contents already agree; just record the new state
	OpMoveLocal
	OpMoveRemote
)
func (o Op) String() string

type Operation struct {
	Op   Op
	Path string
	From string // set only for moves
}

type Side struct {
	Present bool
	Size    int64
	Mtime   int64
	Hash    string // "" means unknown — never treat as a match
	ETag    string
	IsDir   bool
}

func Decide(last *state.Entry, local, remote Side, path ...string) Operation
func Order(ops []Operation) []Operation

last == nil means we have never synced this path.

The truth table

last	local	remote	result
—	present	absent	`OpUpload`
—	absent	present	`OpDownload`
—	present	present, same hash	`OpUpdateState`
—	present	present, different hash	`OpConflict`
yes	unchanged	unchanged	`OpNone`
yes	changed	unchanged	`OpUpload`
yes	unchanged	changed	`OpDownload`
yes	changed	changed, same hash	`OpUpdateState`
yes	changed	changed, different hash	`OpConflict`
yes	absent	unchanged	`OpDeleteRemote`
yes	unchanged	absent	`OpDeleteLocal`
yes	absent	changed	`OpDownload` — never delete
yes	changed	absent	`OpUpload` — never delete
yes	absent	absent	`OpUpdateState` (forget it)

"local changed" means local.Hash != last.ContentHash. "remote changed" means remote.ETag != last.RemoteETag.

The two bold rows are the ones that protect data: a delete racing a modification must never destroy the surviving copy.

The rule that prevents data loss

// sameContent is true only when BOTH hashes are known and equal.
// An unknown ("") hash must never count as a match.
func sameContent(a, b Side) bool {
	return a.Hash != "" && b.Hash != "" && a.Hash == b.Hash
}

`Order` semantics

Creates and downloads parent-first (shallowest Depth first); deletes child-first (deepest first); all non-deletes before deletes. Applying a create before its parent, or a delete before its children, is how sync engines corrupt trees. Use sort.SliceStable.

Steps

Write the failing test as a table covering all 14 rows above, plus:

func TestDecideNeverDeletesOnUnknownHash(t *testing.T) {
	// An unknown local hash must not be read as "unchanged" and cause a delete.
	got := Decide(last("h1", "e1", 3), local(9, ""), absent())
	if got.Op == OpDeleteLocal {
		t.Fatal("must not delete when the local hash is unknown")
	}
}

Run it, confirm it fails.
Implement ops.go then decide.go.
Run tests, confirm all 14 rows pass.
Commit: git commit -s -m "feat(sync): three-way decision function and operation ordering"

Acceptance criteria

Every row of the truth table is a named subtest.
No code path returns a delete when either hash is unknown.
Decide performs no I/O and mutates nothing.

Depends on Task 7. **This is the heart of the engine.** ## Goal A pure function that, given last-known / local / remote state for one path, returns the operation to perform. No I/O, no mutation — so every row of the truth table is a unit test. ## Files - Create: `internal/sync/ops.go`, `internal/sync/decide.go`, `internal/sync/decide_test.go` ## Produces ```go type Op int const ( OpNone Op = iota OpUpload OpDownload OpDeleteLocal OpDeleteRemote OpConflict OpUpdateState // contents already agree; just record the new state OpMoveLocal OpMoveRemote ) func (o Op) String() string type Operation struct { Op Op Path string From string // set only for moves } type Side struct { Present bool Size int64 Mtime int64 Hash string // "" means unknown — never treat as a match ETag string IsDir bool } func Decide(last *state.Entry, local, remote Side, path ...string) Operation func Order(ops []Operation) []Operation ``` `last == nil` means we have never synced this path. ## The truth table | last | local | remote | result | |---|---|---|---| | — | present | absent | `OpUpload` | | — | absent | present | `OpDownload` | | — | present | present, same hash | `OpUpdateState` | | — | present | present, different hash | `OpConflict` | | yes | unchanged | unchanged | `OpNone` | | yes | changed | unchanged | `OpUpload` | | yes | unchanged | changed | `OpDownload` | | yes | changed | changed, same hash | `OpUpdateState` | | yes | changed | changed, different hash | `OpConflict` | | yes | absent | unchanged | `OpDeleteRemote` | | yes | unchanged | absent | `OpDeleteLocal` | | yes | **absent** | **changed** | **`OpDownload`** — never delete | | yes | **changed** | **absent** | **`OpUpload`** — never delete | | yes | absent | absent | `OpUpdateState` (forget it) | "local changed" means `local.Hash != last.ContentHash`. "remote changed" means `remote.ETag != last.RemoteETag`. The two bold rows are the ones that protect data: a delete racing a modification must never destroy the surviving copy. ## The rule that prevents data loss ```go // sameContent is true only when BOTH hashes are known and equal. // An unknown ("") hash must never count as a match. func sameContent(a, b Side) bool { return a.Hash != "" && b.Hash != "" && a.Hash == b.Hash } ``` ## `Order` semantics Creates and downloads **parent-first** (shallowest `Depth` first); deletes **child-first** (deepest first); all non-deletes before deletes. Applying a create before its parent, or a delete before its children, is how sync engines corrupt trees. Use `sort.SliceStable`. ## Steps - [ ] **Write the failing test** as a table covering all 14 rows above, plus: ```go func TestDecideNeverDeletesOnUnknownHash(t *testing.T) { // An unknown local hash must not be read as "unchanged" and cause a delete. got := Decide(last("h1", "e1", 3), local(9, ""), absent()) if got.Op == OpDeleteLocal { t.Fatal("must not delete when the local hash is unknown") } } ``` - [ ] **Run it, confirm it fails.** - [ ] **Implement `ops.go` then `decide.go`.** - [ ] **Run tests, confirm all 14 rows pass.** - [ ] **Commit:** `git commit -s -m "feat(sync): three-way decision function and operation ordering"` ## Acceptance criteria - Every row of the truth table is a named subtest. - No code path returns a delete when either hash is unknown. - `Decide` performs no I/O and mutates nothing.

Cordy added this to the phase-1-engine milestone

2026-09-10 17:15:56 +00:00

Cordy added the

data-integrity

engine

labels

2026-09-10 17:15:56 +00:00

Cordy commented

2026-09-10 22:23:12 +00:00

Author

Owner

Done

6ccb7fb feat(sync): three-way decision function and operation ordering

What was built

ops.go: Op + 9 constants, String(), Operation{Op,Path,From}, Order() — copies input then sort.SliceStable (non-deletes parent-first, deletes child-first)
decide.go: Side, sameContent (verbatim), knownEqual, Decide(last, local, remote, path...) — a pure function, no I/O, no mutation
decide_test.go: 14 named subtests, one per truth-table row, plus TestDecideNeverDeletesOnUnknownHash (verbatim) and an added TestDecideUnknownValuesNeverMatch
knownEqual treats "" as never matching (rather than literal !=), which is what AC2 and "never delete on ambiguity" require — the issue's literal wording is self-contradictory here and this is the safe resolution

Tests

go vet ./... and go test -count=1 ./... green across all 4 packages
CI run #8: http://192.168.10.245/Cordy/cairn-desktop/actions/runs/8 — green, linux/arm64, Go 1.25.5

Acceptance criteria

Every row of the truth table is a named subtest — met: 14 t.Run subtests, issue's row order
No delete when either hash is unknown — met: OpDeleteLocal/OpDeleteRemote both gated on knownEqual; verbatim test plus TestDecideUnknownValuesNeverMatch
Decide performs no I/O and mutates nothing — met: Side by value, last only read, decide.go imports only internal/state, every subtest asserts *last unchanged

Rulings

R3: a content hash is never compared with an ETag
R13: Order sorts a copy of its input
R12: commit is signed off (DCO) with the Co-Authored-By trailer
CV1: CI gate honored — push proceeded per R11, since green CI is the controller's closing gate
CV2: RED/GREEN transcripts and mutation probes A-D independently re-run and confirmed
CV3: #9's three Order tests independently re-run against ops.go (not just traced) — all pass

Deferred

F1: add a dedicated test pinning OpDeleteRemote as ETag-gated (not remote.Hash-gated), for Plan 2's ETag-first comparison
F2: carry-forward to #12 — knownEqual reads an empty recorded value as permanently "changed" (safe, but could affect #12's "second sync = zero ops" AC)
F3: Decide(nil, absent, absent) → OpNone and extra variadic path args are unpinned but harmless

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

**Done** - [6ccb7fb](http://192.168.10.245/Cordy/cairn-desktop/commit/6ccb7fb392d044664a4c4c46a1ad3d31f9d83f32) feat(sync): three-way decision function and operation ordering **What was built** - `ops.go`: `Op` + 9 constants, `String()`, `Operation{Op,Path,From}`, `Order()` — copies input then `sort.SliceStable` (non-deletes parent-first, deletes child-first) - `decide.go`: `Side`, `sameContent` (verbatim), `knownEqual`, `Decide(last, local, remote, path...)` — a pure function, no I/O, no mutation - `decide_test.go`: 14 named subtests, one per truth-table row, plus `TestDecideNeverDeletesOnUnknownHash` (verbatim) and an added `TestDecideUnknownValuesNeverMatch` - `knownEqual` treats `""` as never matching (rather than literal `!=`), which is what AC2 and "never delete on ambiguity" require — the issue's literal wording is self-contradictory here and this is the safe resolution **Tests** - `go vet ./...` and `go test -count=1 ./...` green across all 4 packages - CI run #8: http://192.168.10.245/Cordy/cairn-desktop/actions/runs/8 — green, linux/arm64, Go 1.25.5 **Acceptance criteria** - Every row of the truth table is a named subtest — met: 14 `t.Run` subtests, issue's row order - No delete when either hash is unknown — met: `OpDeleteLocal`/`OpDeleteRemote` both gated on `knownEqual`; verbatim test plus `TestDecideUnknownValuesNeverMatch` - `Decide` performs no I/O and mutates nothing — met: `Side` by value, `last` only read, `decide.go` imports only `internal/state`, every subtest asserts `*last` unchanged **Rulings** - R3: a content hash is never compared with an ETag - R13: `Order` sorts a copy of its input - R12: commit is signed off (DCO) with the `Co-Authored-By` trailer - CV1: CI gate honored — push proceeded per R11, since green CI is the controller's closing gate - CV2: RED/GREEN transcripts and mutation probes A-D independently re-run and confirmed - CV3: #9's three `Order` tests independently re-run against `ops.go` (not just traced) — all pass **Deferred** - F1: add a dedicated test pinning `OpDeleteRemote` as ETag-gated (not `remote.Hash`-gated), for Plan 2's ETag-first comparison - F2: carry-forward to #12 — `knownEqual` reads an empty *recorded* value as permanently "changed" (safe, but could affect #12's "second sync = zero ops" AC) - F3: `Decide(nil, absent, absent)` → `OpNone` and extra variadic `path` args are unpinned but harmless _Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI._

Cordy closed this issue

2026-09-10 22:23:17 +00:00