diff --git a/ingestion/internal/legacymove/legacymove.go b/ingestion/internal/legacymove/legacymove.go index f3a064e..8aeecb9 100644 --- a/ingestion/internal/legacymove/legacymove.go +++ b/ingestion/internal/legacymove/legacymove.go @@ -11,11 +11,14 @@ package legacymove import ( + "bytes" "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" "io/fs" + "path" "sort" "strings" "time" @@ -42,8 +45,17 @@ type Item struct { Action Action Wing string // Move only Reason string + // SHA is the SHA-256 (hex) of the bytes the plan was decided on. Apply + // re-reads the page and does nothing if the bytes differ: a page changed + // between planning and applying is not the page that was judged safe. + SHA string } +// maxArchiveName is NAME_MAX. The archive key (the whole path, flattened) is +// longer than the page's own name, so a long name must be refused at planning +// time rather than wedge Apply halfway. +const maxArchiveName = 255 + const legacyDir = "wiki/sources" // Plan reads the retired source dir and decides. It writes nothing. @@ -71,8 +83,8 @@ func Plan(brainDir, wing string) ([]Item, error) { from := legacyDir + "/" + name slug := strings.TrimSuffix(name, ".md") it := Item{From: from, Slug: slug} - if strings.HasPrefix(name, "tunnel-candidates-") { - it.Action, it.Reason = Delete, "auto-tunnel review log ingested as a source; not a source" + if len(archiveKey(from)) > maxArchiveName { + it.Action, it.Reason = Skip, "name too long to archive; left in place" items = append(items, it) continue } @@ -82,6 +94,17 @@ func Plan(brainDir, wing string) ([]Item, error) { items = append(items, it) continue } + it.SHA = sum(content) + if strings.HasPrefix(name, "tunnel-candidates-") { + it.Action, it.Reason = Delete, "auto-tunnel review log ingested as a source; not a source" + items = append(items, it) + continue + } + if has := topLevelKeys(string(content)); has["wing"] || has["hall"] { + it.Action, it.Reason = Skip, "already has wing/hall in its frontmatter; left in place" + items = append(items, it) + continue + } switch typ := frontmatterValue(string(content), "type"); typ { case "video", "article": to := "wiki/" + wing + "/sources/" + name @@ -109,6 +132,9 @@ func Plan(brainDir, wing string) ([]Item, error) { type Result struct { Moved, Deleted int ArchiveDir string + // Changed lists planned pages that were NOT touched because their bytes no + // longer match what the plan was decided on. + Changed []string } type manifest struct { @@ -119,13 +145,18 @@ type manifest struct { type moveRec struct { From string `json:"from"` To string `json:"to"` + Wing string `json:"wing"` } // Apply performs the Move and Delete items, one at a time, stopping at the -// first failure. Each original is archived before anything changes; a move -// writes its destination with O_EXCL before the source is removed, so a failure -// never loses a page. The manifest is rewritten after every step so a partial -// run can still be rolled back. +// first failure. For each page it re-reads the bytes and compares them with the +// plan's hash: a page that changed since planning is left alone and reported in +// Result.Changed. Each original is archived (O_EXCL, into an archive directory +// no earlier run has used) before anything changes; a move writes its +// destination O_EXCL before the source is removed, so a failure never loses a +// page. The manifest is rewritten after every step so a partial run can still +// be rolled back. A swap between the read and the remove is a residual window +// that only a locking watcher could close. func Apply(brainDir string, items []Item, now time.Time) (Result, error) { bfs, err := brainfs.Open(brainDir) if err != nil { @@ -133,7 +164,7 @@ func Apply(brainDir string, items []Item, now time.Time) (Result, error) { } defer func() { _ = bfs.Close() }() - res := Result{ArchiveDir: "archive/wing-migration-" + now.UTC().Format("2006-01-02")} + res := Result{ArchiveDir: freshArchiveDir(bfs, "archive/wing-migration-"+now.UTC().Format("2006-01-02"))} var man manifest save := func() error { b, err := json.MarshalIndent(man, "", " ") @@ -150,7 +181,11 @@ func Apply(brainDir string, items []Item, now time.Time) (Result, error) { if err != nil { return res, fmt.Errorf("read %s: %w", it.From, err) } - if err := bfs.PutRegular(res.ArchiveDir+"/originals/"+archiveKey(it.From), content); err != nil { + if it.SHA == "" || sum(content) != it.SHA { + res.Changed = append(res.Changed, it.From) + continue + } + if err := bfs.CreateRegular(res.ArchiveDir+"/originals/"+archiveKey(it.From), content); err != nil { return res, fmt.Errorf("archive %s: %w", it.From, err) } switch it.Action { @@ -158,7 +193,7 @@ func Apply(brainDir string, items []Item, now time.Time) (Result, error) { if err := bfs.CreateRegular(it.To, []byte(stamp(string(content), it.Wing))); err != nil { return res, fmt.Errorf("write %s: %w", it.To, err) } - man.Moves = append(man.Moves, moveRec{From: it.From, To: it.To}) + man.Moves = append(man.Moves, moveRec{From: it.From, To: it.To, Wing: it.Wing}) res.Moved++ case Delete: man.Deletes = append(man.Deletes, it.From) @@ -177,6 +212,23 @@ func Apply(brainDir string, items []Item, now time.Time) (Result, error) { return res, nil } +// freshArchiveDir returns base, or base-2, base-3, ... : the first that does not +// exist, so a second run never reuses (and never overwrites) an earlier archive. +func freshArchiveDir(bfs *brainfs.Brain, base string) string { + dir := base + for n := 2; ; n++ { + if _, err := bfs.Lstat(dir); err != nil { + return dir + } + dir = fmt.Sprintf("%s-%d", base, n) + } +} + +func sum(b []byte) string { + h := sha256.Sum256(b) + return hex.EncodeToString(h[:]) +} + func wingsOf(items []Item) map[string]bool { out := map[string]bool{} for _, it := range items { @@ -188,8 +240,11 @@ func wingsOf(items []Item) map[string]bool { } // Rollback restores every page named in the archive's manifest from its -// archived original and removes the moved copies. It refuses to overwrite a -// page that already exists at the original location. +// archived original and removes the moved copies. It checks everything first +// and changes nothing if anything is in the way: a restore target that exists +// with different bytes, or a moved copy that was edited since the move (which +// is never deleted). Pages already restored are recognised by their bytes, so a +// rollback can be run again. mtime and mode of the restored pages are new. func Rollback(brainDir, archiveDir string) error { bfs, err := brainfs.Open(brainDir) if err != nil { @@ -204,28 +259,77 @@ func Rollback(brainDir, archiveDir string) error { if err := json.Unmarshal(raw, &man); err != nil { return fmt.Errorf("parse manifest: %w", err) } - restore := func(from string) error { + original := func(from string) ([]byte, error) { orig, err := bfs.ReadRegular(archiveDir + "/originals/" + archiveKey(from)) if err != nil { - return fmt.Errorf("read archived %s: %w", from, err) + return nil, fmt.Errorf("read archived %s: %w", from, err) } - if err := bfs.CreateRegular(from, orig); err != nil { - return fmt.Errorf("restore %s: %w", from, err) + return orig, nil + } + + type step struct { + from string + orig []byte + restore bool // From is missing and must be written + remove string // moved copy to delete, if any + } + var steps []step + var problems []string + plan := func(from string, orig []byte, moved string, expectMoved []byte) { + st := step{from: from, orig: orig} + cur, err := bfs.ReadRegular(from) + switch { + case err == nil && bytes.Equal(cur, orig): + // already restored + case err == nil: + problems = append(problems, fmt.Sprintf("%s exists with different content", from)) + case errors.Is(err, fs.ErrNotExist): + st.restore = true + default: + problems = append(problems, fmt.Sprintf("%s: %v", from, err)) } - return nil + if moved != "" { + cur, err := bfs.ReadRegular(moved) + switch { + case err == nil && bytes.Equal(cur, expectMoved): + st.remove = moved + case err == nil: + problems = append(problems, fmt.Sprintf("%s was edited since the move; not deleting it", moved)) + case errors.Is(err, fs.ErrNotExist): + default: + problems = append(problems, fmt.Sprintf("%s: %v", moved, err)) + } + } + steps = append(steps, st) } for _, m := range man.Moves { - if err := restore(m.From); err != nil { + orig, err := original(m.From) + if err != nil { return err } - if err := bfs.Remove(m.To); err != nil { - return fmt.Errorf("remove moved copy %s: %w", m.To, err) - } + plan(m.From, orig, m.To, []byte(stamp(string(orig), m.Wing))) } for _, d := range man.Deletes { - if err := restore(d); err != nil { + orig, err := original(d) + if err != nil { return err } + plan(d, orig, "", nil) + } + if len(problems) > 0 { + return fmt.Errorf("rollback refused, nothing changed: %s", strings.Join(problems, "; ")) + } + for _, st := range steps { + if st.restore { + if err := bfs.CreateRegular(st.from, st.orig); err != nil { + return fmt.Errorf("restore %s: %w", st.from, err) + } + } + if st.remove != "" { + if err := bfs.Remove(st.remove); err != nil { + return fmt.Errorf("remove moved copy %s: %w", st.remove, err) + } + } } return nil } @@ -245,13 +349,53 @@ func stamp(content, wing string) string { return "---\n" + extra + "---\n" + content } -func frontmatterValue(content, key string) string { +// frontmatterBlock returns the text between the opening fence and a real +// closing fence (a line that is exactly "---"), or "" when there is none. +func frontmatterBlock(content string) (string, bool) { if !strings.HasPrefix(content, "---\n") { + return "", false + } + rest := content[4:] + if i := strings.Index(rest, "\n---\n"); i >= 0 { + return rest[:i], true + } + if strings.HasSuffix(rest, "\n---") { + return rest[:len(rest)-4], true + } + return "", false +} + +// topLevelKeys is the set of unindented `key:` names in the frontmatter block. +func topLevelKeys(content string) map[string]bool { + out := map[string]bool{} + block, ok := frontmatterBlock(content) + if !ok { + return out + } + for _, line := range strings.Split(block, "\n") { + if line == "" || line[0] == ' ' || line[0] == '\t' { + continue + } + if k, _, found := strings.Cut(line, ":"); found { + out[strings.TrimSpace(k)] = true + } + } + return out +} + +// frontmatterValue reads a top-level key from a CLOSED frontmatter block. A +// nested or block-scalar line that merely looks like `type: video` does not +// count, and neither does an unterminated block. +func frontmatterValue(content, key string) string { + block, ok := frontmatterBlock(content) + if !ok { return "" } - block, _, _ := strings.Cut(content[4:], "\n---") for _, line := range strings.Split(block, "\n") { - if k, v, ok := strings.Cut(line, ":"); ok && strings.TrimSpace(k) == key { + if line == "" || line[0] == ' ' || line[0] == '\t' { + continue + } + if k, v, found := strings.Cut(line, ":"); found && strings.TrimSpace(k) == key { return strings.Trim(strings.TrimSpace(v), `"'`) } } @@ -265,7 +409,8 @@ const rawDir = "raw" // whose note is gone. An original is deleted only when a byte-identical copy // exists under raw/processed/ (so nothing is lost), and only when it has a // marker (an unmarked note is still in the inbox). tunnel-candidates-* are -// AutoTunnel's own review queue and are never touched. +// AutoTunnel's own review queue and are never touched. Every Delete carries the +// SHA-256 it was decided on, which Apply re-checks. func PlanRaw(brainDir string) ([]Item, error) { bfs, err := brainfs.Open(brainDir) if err != nil { @@ -287,6 +432,15 @@ func PlanRaw(brainDir string) ([]Item, error) { if err != nil { return nil, err } + del := func(name, why string) Item { + it := Item{From: rawDir + "/" + name, Slug: name, Action: Delete, Reason: why} + b, err := bfs.ReadRegular(it.From) + if err != nil { + return Item{From: it.From, Slug: name, Action: Skip, Reason: fmt.Sprintf("unreadable: %v", err)} + } + it.SHA = sum(b) + return it + } var items []Item for _, n := range names { if strings.HasPrefix(n, "tunnel-candidates-") { @@ -296,7 +450,7 @@ func PlanRaw(brainDir string) ([]Item, error) { case strings.HasSuffix(n, ".processed"): note := strings.TrimSuffix(n, ".processed") if !present[note] { - items = append(items, Item{From: rawDir + "/" + n, Slug: n, Action: Delete, Reason: "marker without a note"}) + items = append(items, tooLongOr(rawDir+"/"+n, func() Item { return del(n, "marker without a note") })) } case present[n+".processed"]: content, rerr := bfs.ReadRegular(rawDir + "/" + n) @@ -308,15 +462,28 @@ func PlanRaw(brainDir string) ([]Item, error) { items = append(items, Item{From: rawDir + "/" + n, Slug: n, Action: Skip, Reason: "marked processed but no identical copy under raw/processed/; kept"}) continue } - items = append(items, - Item{From: rawDir + "/" + n, Slug: n, Action: Delete, Reason: "archived copy exists under raw/processed/"}, - Item{From: rawDir + "/" + n + ".processed", Slug: n + ".processed", Action: Delete, Reason: "marker of an archived note"}) + // Both or neither: the archive keys of the note and its (longer) + // marker must fit, or a run would delete the note and wedge on the marker. + if len(archiveKey(rawDir+"/"+n+".processed")) > maxArchiveName { + items = append(items, Item{From: rawDir + "/" + n, Slug: n, Action: Skip, Reason: "name too long to archive; kept"}) + continue + } + items = append(items, del(n, "archived copy exists under raw/processed/"), del(n+".processed", "marker of an archived note")) } } sort.SliceStable(items, func(i, j int) bool { return items[i].From < items[j].From }) return items, nil } +// tooLongOr returns a Skip when the page's archive key cannot fit in a file +// name, else whatever mk builds. +func tooLongOr(from string, mk func() Item) Item { + if len(archiveKey(from)) > maxArchiveName { + return Item{From: from, Slug: path.Base(from), Action: Skip, Reason: "name too long to archive; kept"} + } + return mk() +} + // archivedCopies returns the SHA-256 of every file under raw/processed/, as a // set, so "is there an identical copy" is one lookup. func archivedCopies(bfs *brainfs.Brain) (map[[32]byte]bool, error) { diff --git a/ingestion/internal/legacymove/legacymove_test.go b/ingestion/internal/legacymove/legacymove_test.go index 10e8e40..bf63cba 100644 --- a/ingestion/internal/legacymove/legacymove_test.go +++ b/ingestion/internal/legacymove/legacymove_test.go @@ -269,3 +269,177 @@ func TestApplyAndRollback_HandleRawLeftoversAndNeverCollideWithSourcesOfTheSameN require.NoError(t, err) assert.Equal(t, src("video"), string(got)) } + +// --- review of #213 --- + +var day = time.Date(2026, 10, 20, 0, 0, 0, 0, time.UTC) + +// F1: a second Apply on the same day must not touch the first run's archive. +func TestApply_ASecondRunTheSameDayGetsItsOwnArchiveAndKeepsTheFirst(t *testing.T) { + dir := t.TempDir() + page(t, dir, "wiki/sources/a-talk.md", src("video")) + page(t, dir, "wiki/sources/tunnel-candidates-1.md", "FIRST tunnel bytes") + items, err := legacymove.Plan(dir, "web-clip") + require.NoError(t, err) + first, err := legacymove.Apply(dir, items, day) + require.NoError(t, err) + + page(t, dir, "wiki/sources/tunnel-candidates-1.md", "SECOND tunnel bytes") // same name reappears + items2, err := legacymove.Plan(dir, "web-clip") + require.NoError(t, err) + second, err := legacymove.Apply(dir, items2, day) + require.NoError(t, err) + + assert.NotEqual(t, first.ArchiveDir, second.ArchiveDir) + got, err := os.ReadFile(filepath.Join(dir, first.ArchiveDir, "originals", "wiki__sources__tunnel-candidates-1.md")) + require.NoError(t, err) + assert.Equal(t, "FIRST tunnel bytes", string(got), "the first run's archived original is intact") + + require.NoError(t, legacymove.Rollback(dir, first.ArchiveDir)) + _, statErr := os.Stat(filepath.Join(dir, "wiki/sources/a-talk.md")) + assert.NoError(t, statErr, "the first run's move is still rolled back by its own manifest") +} + +// F2: Plan decided on the bytes it saw. Apply must not delete different bytes. +func TestApply_ARawNoteThatChangedSincePlanningIsNotDeleted(t *testing.T) { + dir := t.TempDir() + rawFile(t, dir, "raw/a.md", "bytes that have an archive copy") + rawFile(t, dir, "raw/a.md.processed", "d") + rawFile(t, dir, "raw/processed/2026-10-19/a.md", "bytes that have an archive copy") + items, err := legacymove.PlanRaw(dir) + require.NoError(t, err) + + rawFile(t, dir, "raw/a.md", "NEW UNPROCESSED BYTES") // dropped again before Apply runs + res, err := legacymove.Apply(dir, items, day) + require.NoError(t, err) + + got, err := os.ReadFile(filepath.Join(dir, "raw/a.md")) + require.NoError(t, err, "the changed note is still in the inbox") + assert.Equal(t, "NEW UNPROCESSED BYTES", string(got)) + assert.Contains(t, strings.Join(res.Changed, ","), "raw/a.md") +} + +func TestApply_AWikiSourceThatChangedSincePlanningIsNotMoved(t *testing.T) { + dir := t.TempDir() + page(t, dir, "wiki/sources/a-talk.md", src("video")) + items, err := legacymove.Plan(dir, "web-clip") + require.NoError(t, err) + page(t, dir, "wiki/sources/a-talk.md", src("video")+"\nan edit made after the plan\n") + + res, err := legacymove.Apply(dir, items, day) + require.NoError(t, err) + assert.Zero(t, res.Moved) + got, _ := os.ReadFile(filepath.Join(dir, "wiki/sources/a-talk.md")) + assert.Contains(t, string(got), "an edit made after the plan") + assert.NotEmpty(t, res.Changed) +} + +// F3: rollback checks everything first, changes nothing on a conflict, can be +// repeated, and never throws away an edit. +func TestRollback_RefusesBeforeChangingAnythingWhenATargetExists(t *testing.T) { + dir := t.TempDir() + page(t, dir, "wiki/sources/a-talk.md", src("video")) + page(t, dir, "wiki/sources/b-talk.md", src("video")) + items, _ := legacymove.Plan(dir, "web-clip") + res, err := legacymove.Apply(dir, items, day) + require.NoError(t, err) + + page(t, dir, "wiki/sources/b-talk.md", "someone put a different b here") + err = legacymove.Rollback(dir, res.ArchiveDir) + require.Error(t, err) + _, statErr := os.Stat(filepath.Join(dir, "wiki/sources/a-talk.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist, "nothing was restored: a was not touched either") + _, statErr = os.Stat(filepath.Join(dir, "wiki/web-clip/sources/a-talk.md")) + assert.NoError(t, statErr, "and the moved copies are still there") +} + +func TestRollback_CanBeRunTwice(t *testing.T) { + dir := t.TempDir() + page(t, dir, "wiki/sources/a-talk.md", src("video")) + items, _ := legacymove.Plan(dir, "web-clip") + res, err := legacymove.Apply(dir, items, day) + require.NoError(t, err) + require.NoError(t, legacymove.Rollback(dir, res.ArchiveDir)) + require.NoError(t, legacymove.Rollback(dir, res.ArchiveDir), "a repeat finds everything already restored") + got, _ := os.ReadFile(filepath.Join(dir, "wiki/sources/a-talk.md")) + assert.Equal(t, src("video"), string(got)) +} + +func TestRollback_DoesNotDeleteAMovedPageThatWasEditedSinceTheMove(t *testing.T) { + dir := t.TempDir() + page(t, dir, "wiki/sources/a-talk.md", src("video")) + items, _ := legacymove.Plan(dir, "web-clip") + res, err := legacymove.Apply(dir, items, day) + require.NoError(t, err) + moved := filepath.Join(dir, "wiki/web-clip/sources/a-talk.md") + b, _ := os.ReadFile(moved) + require.NoError(t, os.WriteFile(moved, append(b, []byte("\nmy edit\n")...), 0o644)) + + err = legacymove.Rollback(dir, res.ArchiveDir) + require.Error(t, err) + got, readErr := os.ReadFile(moved) + require.NoError(t, readErr) + assert.Contains(t, string(got), "my edit", "the edit survives") + _, statErr := os.Stat(filepath.Join(dir, "wiki/sources/a-talk.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist, "nothing was changed") +} + +// F4: a name whose archive key cannot fit must be skipped at planning time. +func TestPlanRaw_ANameTooLongToArchiveIsSkippedNotAbortedOn(t *testing.T) { + dir := t.TempDir() + long := strings.Repeat("a", 240) + ".md" + rawFile(t, dir, "raw/"+long, "text") + rawFile(t, dir, "raw/"+long+".processed", "d") + rawFile(t, dir, "raw/processed/2026-10-19/"+long, "text") + rawFile(t, dir, "raw/ok.md", "fine") + rawFile(t, dir, "raw/ok.md.processed", "d") + rawFile(t, dir, "raw/processed/2026-10-19/ok.md", "fine") + + items, err := legacymove.PlanRaw(dir) + require.NoError(t, err) + m := byFrom(items) + assert.Equal(t, legacymove.Skip, m["raw/"+long].Action) + assert.Contains(t, m["raw/"+long].Reason, "too long") + assert.Equal(t, legacymove.Delete, m["raw/ok.md"].Action) + + _, err = legacymove.Apply(dir, items, day) + require.NoError(t, err, "the long name does not wedge the run") + _, statErr := os.Stat(filepath.Join(dir, "raw/"+long)) + assert.NoError(t, statErr, "and it is left where it is") +} + +// F6: do not stamp a page that already carries a wing or hall; only a top-level +// type inside a closed frontmatter block decides. +func TestPlan_AMovedPageIsNeverGivenDuplicateWingKeys(t *testing.T) { + dir := t.TempDir() + page(t, dir, "wiki/sources/has-wing.md", "---\ntitle: T\ntype: video\nwing: old\nhall: facts\n---\nbody") + items, err := legacymove.Plan(dir, "web-clip") + require.NoError(t, err) + it := byFrom(items)["wiki/sources/has-wing.md"] + assert.Equal(t, legacymove.Skip, it.Action) + assert.Contains(t, it.Reason, "already has") +} + +func TestPlan_OnlyATopLevelTypeInAClosedBlockClassifiesAPage(t *testing.T) { + dir := t.TempDir() + page(t, dir, "wiki/sources/nested.md", "---\ntitle: T\nmeta:\n type: video\n---\nbody") + page(t, dir, "wiki/sources/blockscalar.md", "---\ntitle: T\ndesc: |\n type: article\n---\nbody") + page(t, dir, "wiki/sources/unterminated.md", "---\ntitle: T\ntype: video\nbody with no closing fence") + items, err := legacymove.Plan(dir, "web-clip") + require.NoError(t, err) + m := byFrom(items) + for _, f := range []string{"nested", "blockscalar", "unterminated"} { + assert.Equal(t, legacymove.Skip, m["wiki/sources/"+f+".md"].Action, f) + } +} + +func TestPlan_AWikiSourceWithATooLongNameIsSkipped(t *testing.T) { + dir := t.TempDir() + long := strings.Repeat("a", 240) + ".md" + page(t, dir, "wiki/sources/"+long, src("video")) + items, err := legacymove.Plan(dir, "web-clip") + require.NoError(t, err) + it := byFrom(items)["wiki/sources/"+long] + assert.Equal(t, legacymove.Skip, it.Action) + assert.Contains(t, it.Reason, "too long") +}