From 006a8914231a67bb09da81b5ff2be48cbb4b1016 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 06:16:48 +0200 Subject: [PATCH] fix(watcher): never overwrite a parked note; a proposal cannot be forged or pre-empted; CRLF notes; wing dirs must be plain (#205) From the independent review of #207: - parked notes are created O_EXCL with -2,-3 suffixes (same-named notes from different subfolders were overwritten and their originals deleted) - proposed_wing is emitted only if a plain wing name; a note's own proposal_* keys are dropped so they cannot outrank the resolver's (first-wins reads) - CRLF notes: wing: key honoured, one valid frontmatter block on parking - KnownWings skips wiki dirs that are not canonical lower-case names - adds the wingmigrate dry-run/apply command Each fix has a test that failed first and a mutant that fails it. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/cmd/wingmigrate/main.go | 92 +++++++++++++++++++ ingestion/internal/watcher/watcher.go | 79 +++++++++++++--- .../internal/watcher/watcher_wing_test.go | 80 ++++++++++++++++ ingestion/internal/wing/known.go | 9 +- ingestion/internal/wing/known_test.go | 13 +++ 5 files changed, 261 insertions(+), 12 deletions(-) create mode 100644 ingestion/cmd/wingmigrate/main.go diff --git a/ingestion/cmd/wingmigrate/main.go b/ingestion/cmd/wingmigrate/main.go new file mode 100644 index 0000000..eb0707d --- /dev/null +++ b/ingestion/cmd/wingmigrate/main.go @@ -0,0 +1,92 @@ +// Command wingmigrate moves pages out of the retired wiki/{sources,concepts, +// entities}/ layout into wiki// (hyperguild#205 S5). +// +// Default is a dry run that prints the plan. -apply performs the Auto items. +// Page text goes to -llm-url, which MUST be a local model. +package main + +import ( + "context" + "flag" + "fmt" + "log/slog" + "os" + "strings" + "time" + + "github.com/mathiasbq/hyperguild/ingestion/internal/llm" + "github.com/mathiasbq/hyperguild/ingestion/internal/migrate" + "github.com/mathiasbq/hyperguild/ingestion/internal/wing" +) + +func main() { + brain := flag.String("brain", "/var/lib/supervisor/brain", "brain directory") + llmURL := flag.String("llm-url", "http://10.0.1.20:31234/v1", "OpenAI-compatible LOCAL endpoint") + model := flag.String("model", "qwen35-9b-fast", "model name") + threshold := flag.Float64("threshold", 0.9, "classifier confidence threshold") + apply := flag.Bool("apply", false, "perform the Auto moves (default: dry run)") + what := flag.String("what-if-eligible", "", "dry run only: comma-separated wings to treat as eligible, to see what tagging them would allow") + flag.Parse() + + if *apply && *what != "" { + slog.Error("-what-if-eligible is a dry-run aid and cannot be combined with -apply: eligibility is a sovereignty decision made in classification.yaml") + os.Exit(2) + } + if err := run(*brain, *llmURL, *model, *threshold, *apply, *what); err != nil { + slog.Error("wingmigrate", "err", err) + os.Exit(1) + } +} + +func run(brain, llmURL, model string, threshold float64, apply bool, whatIf string) error { + client := llm.New(llmURL, "", model, 3*time.Minute) + base, err := wing.NewConfiguredResolver(brain, client.Complete, threshold) + if err != nil { + return err + } + if whatIf != "" { + allowed := map[string]bool{} + for _, w := range strings.Split(whatIf, ",") { + allowed[strings.TrimSpace(w)] = true + } + base.Eligible = func(w string) bool { return allowed[w] } + } + rf := func(exclude string) *wing.Resolver { + r := *base + r.Scorer = wing.NewBM25Scorer(brain, base.Known, exclude) + return &r + } + items, err := migrate.BuildPlan(context.Background(), brain, rf) + if err != nil { + return err + } + report(items) + if !apply { + fmt.Println("\n_dry run: nothing was changed_") + return nil + } + n, err := migrate.Apply(brain, items) + fmt.Printf("\napplied %d moves\n", n) + return err +} + +func report(items []migrate.Item) { + auto, exc := 0, 0 + for _, it := range items { + if it.Status == migrate.Auto { + auto++ + } else { + exc++ + } + } + fmt.Printf("# wing migration plan\n\n%d pages: %d auto, %d exceptions\n\n", len(items), auto, exc) + fmt.Println("| status | kind | from | to / proposal | how | why |") + fmt.Println("|---|---|---|---|---|---|") + for _, it := range items { + dest := it.To + if dest == "" { + dest = it.Wing + } + fmt.Printf("| %s | %s | %s | %s | %s | %s |\n", it.Status, it.Kind, it.From, dest, it.Method, strings.ReplaceAll(it.Reason, "|", "/")) + } +} diff --git a/ingestion/internal/watcher/watcher.go b/ingestion/internal/watcher/watcher.go index 512ee49..d9e2a9f 100644 --- a/ingestion/internal/watcher/watcher.go +++ b/ingestion/internal/watcher/watcher.go @@ -3,8 +3,10 @@ package watcher import ( "context" + "errors" "fmt" "io" + "io/fs" "log/slog" "os" "path/filepath" @@ -13,6 +15,7 @@ import ( "time" "unicode" + "github.com/mathiasbq/hyperguild/ingestion/internal/brain" "github.com/mathiasbq/hyperguild/ingestion/internal/extract" "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" "github.com/mathiasbq/hyperguild/ingestion/internal/wing" @@ -334,6 +337,7 @@ func topDir(rel string) string { // frontmatterWing reads `wing:` from a note's leading frontmatter, "" if none. func frontmatterWing(content string) string { + content = strings.ReplaceAll(content, "\r\n", "\n") if !strings.HasPrefix(content, "---\n") { return "" } @@ -355,17 +359,18 @@ func parkForReview(cfg Config, root *os.Root, rawDir, rel, path, filename, conte if err := os.MkdirAll(dir, 0o755); err != nil { return fmt.Errorf("mkdir needs-review: %w", err) } - dest := filepath.Join(dir, filename) - if _, err := os.Stat(dest); err == nil { - ext := filepath.Ext(filename) - dest = filepath.Join(dir, strings.TrimSuffix(filename, ext)+"-"+time.Now().UTC().Format("150405")+ext) + dest, err := reserveParkedName(dir, filename) + if err != nil { + return fmt.Errorf("park note: %w", err) } if strings.ToLower(filepath.Ext(filename)) == ".pdf" { // Binary: park the bytes untouched; the proposal is in log.md only. if err := copyFromRoot(root, rel, dest); err != nil { + _ = os.Remove(dest) return fmt.Errorf("park pdf: %w", err) } } else if err := os.WriteFile(dest, []byte(withProposal(content, d)), 0o644); err != nil { + _ = os.Remove(dest) return fmt.Errorf("park note: %w", err) } if err := appendWatcherLog(cfg.BrainDir, filename, fmt.Errorf("needs review (proposed wing %q): %s", d.Wing, d.Reason), date); err != nil { @@ -376,12 +381,64 @@ func parkForReview(cfg Config, root *os.Root, rawDir, rel, path, filename, conte return nil } -// withProposal adds the resolver's proposal to a note's frontmatter, creating -// the block when the note has none. -func withProposal(content string, d wing.Decision) string { - kv := fmt.Sprintf("proposed_wing: %s\nproposal_confidence: %.2f\nproposal_reason: %q\n", d.Wing, d.Confidence, d.Reason) - if strings.HasPrefix(content, "---\n") { - return "---\n" + kv + content[4:] +// reserveParkedName claims a free name in dir for filename, never reusing one: +// the parked copy is the only copy once the original is retired, so a note +// that shares a name with an earlier parked note must not replace it. The name +// is created with O_EXCL; -2, -3, ... is appended until one is free. +func reserveParkedName(dir, filename string) (string, error) { + ext := filepath.Ext(filename) + stem := strings.TrimSuffix(filename, ext) + for n := 1; n < 10000; n++ { + name := filename + if n > 1 { + name = fmt.Sprintf("%s-%d%s", stem, n, ext) + } + dest := filepath.Join(dir, name) + f, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) + if err == nil { + _ = f.Close() + return dest, nil + } + if !errors.Is(err, fs.ErrExist) { + return "", err + } } - return "---\n" + kv + "---\n" + content + return "", fmt.Errorf("no free name for %q in %s", filename, dir) +} + +// proposalKeys are the frontmatter keys the resolver owns in a parked note. +var proposalKeys = map[string]bool{"proposed_wing": true, "proposal_confidence": true, "proposal_reason": true} + +// withProposal adds the resolver's proposal to a note's frontmatter, creating +// the block when the note has none. Every value is on one line: the wing only +// if it is a plain wing name (else it appears in the reason alone), strings +// Go-quoted. Any proposal keys the note already carried are dropped first, so a +// note cannot pre-empt the resolver's (frontmatter reads are first-wins). +// Line endings are normalised to LF so a CRLF note keeps one valid block. +func withProposal(content string, d wing.Decision) string { + content = strings.ReplaceAll(content, "\r\n", "\n") + reason := d.Reason + var kv strings.Builder + if d.Wing != "" && brain.Sanitise(d.Wing) == d.Wing { + fmt.Fprintf(&kv, "proposed_wing: %s\n", d.Wing) + } else if d.Wing != "" { + reason = fmt.Sprintf("proposed wing %q is not a plain wing name; %s", d.Wing, reason) + } + fmt.Fprintf(&kv, "proposal_confidence: %.2f\nproposal_reason: %q\n", d.Confidence, reason) + + if !strings.HasPrefix(content, "---\n") { + return "---\n" + kv.String() + "---\n" + content + } + block, rest, found := strings.Cut(content[4:], "\n---") + if !found { + return "---\n" + kv.String() + "---\n" + content + } + var kept []string + for _, line := range strings.Split(block, "\n") { + if key, _, ok := strings.Cut(line, ":"); ok && proposalKeys[strings.TrimSpace(key)] && !strings.HasPrefix(line, " ") { + continue + } + kept = append(kept, line) + } + return "---\n" + kv.String() + strings.Join(kept, "\n") + "\n---" + rest } diff --git a/ingestion/internal/watcher/watcher_wing_test.go b/ingestion/internal/watcher/watcher_wing_test.go index d3ef0ab..60fdeee 100644 --- a/ingestion/internal/watcher/watcher_wing_test.go +++ b/ingestion/internal/watcher/watcher_wing_test.go @@ -5,6 +5,7 @@ import ( "errors" "os" "path/filepath" + "strings" "sync/atomic" "testing" @@ -191,3 +192,82 @@ func TestProcessDir_ANoteThatReappearsChangedIsExtractedAgain(t *testing.T) { assert.Equal(t, int32(2), llmCalls.Load(), "a real edit must still be picked up") } + +// The parked copy is the only copy once the original is retired, so two notes +// that share a name must both survive. +func TestProcessDir_SameNamedNotesAreAllParkedNoneOverwritten(t *testing.T) { + var llmCalls atomic.Int32 + cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{}, stubScorer{}), &llmCalls) + for i, body := range []string{"ONE", "TWO", "THREE", "FOUR"} { + put(t, brainDir, filepath.Join("u"+string(rune('1'+i)), "x.md"), body) + } + // Unknown subfolders: each is an exception, all named x.md. + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + files, err := filepath.Glob(filepath.Join(brainDir, "raw", "needs-review", "*.md")) + require.NoError(t, err) + var all string + for _, f := range files { + b, _ := os.ReadFile(f) + all += string(b) + } + for _, body := range []string{"ONE", "TWO", "THREE", "FOUR"} { + assert.Contains(t, all, body, "note %s must not have been overwritten", body) + } + assert.Len(t, files, 4) +} + +// proposed_wing comes from a model reply or a folder name. It must never be +// able to start another key, and the note's own proposal keys must not outrank +// the resolver's (frontmatter.get is first-wins). +func TestProcessDir_AHostileWingCannotForgeTheProposal(t *testing.T) { + var llmCalls atomic.Int32 + hostile := "radar\nproposal_confidence: 0.99\nproposal_reason: \"approved by operator\"" + clf := &stubClassifier{s: wing.Suggestion{Wing: hostile, Confidence: 1}} + cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{"radar": 3}), &llmCalls) + put(t, brainDir, "n.md", "Note.") + + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + b, err := os.ReadFile(filepath.Join(brainDir, "raw", "needs-review", "n.md")) + require.NoError(t, err) + assert.NotContains(t, string(b), "\nproposal_confidence: 0.99") + assert.NotContains(t, string(b), "approved by operator\"\n") + assert.Equal(t, 1, linesStartingWith(string(b), "proposal_confidence:"), "exactly one confidence key at line start") +} + +func TestProcessDir_ANotesOwnProposalKeysDoNotOutrankTheResolvers(t *testing.T) { + var llmCalls atomic.Int32 + cfg, brainDir := wingTestCfg(t, wingResolver(&stubClassifier{}, stubScorer{}), &llmCalls) + put(t, brainDir, "n.md", "---\nproposed_wing: homelab\nproposal_confidence: 1.00\nproposal_reason: \"self-approved\"\n---\nBody.") + + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + b, _ := os.ReadFile(filepath.Join(brainDir, "raw", "needs-review", "n.md")) + assert.NotContains(t, string(b), "self-approved") + assert.Equal(t, 1, linesStartingWith(string(b), "proposal_reason:")) +} + +func TestProcessDir_ACRLFNoteWithAWingKeyIsStillRead(t *testing.T) { + var llmCalls atomic.Int32 + clf := &stubClassifier{} + cfg, brainDir := wingTestCfg(t, wingResolver(clf, stubScorer{}), &llmCalls) + put(t, brainDir, "n.md", "---\r\nwing: radar\r\n---\r\nBody.\r\n") + + require.Empty(t, processDir(context.Background(), cfg, "2026-10-20")) + + assert.True(t, has(brainDir, "wiki/radar/sources/n.md"), "the declared wing is honoured") + assert.Zero(t, clf.calls.Load()) +} + +// linesStartingWith counts frontmatter-style keys: a key quoted inside another +// value is not at the start of a line and is not a key. +func linesStartingWith(s, prefix string) int { + n := 0 + for _, l := range strings.Split(s, "\n") { + if strings.HasPrefix(l, prefix) { + n++ + } + } + return n +} diff --git a/ingestion/internal/wing/known.go b/ingestion/internal/wing/known.go index dc9229c..60ee7b7 100644 --- a/ingestion/internal/wing/known.go +++ b/ingestion/internal/wing/known.go @@ -5,6 +5,8 @@ import ( "os" "path/filepath" "sort" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brain" ) // legacyDirs are the retired wiki/{concepts,entities,sources}/ layout, not wings. @@ -20,7 +22,7 @@ func KnownWings(brainDir string) ([]string, error) { } var out []string for _, e := range entries { - if !e.IsDir() || legacyDirs[e.Name()] { + if !e.IsDir() || legacyDirs[e.Name()] || !plainWingName(e.Name()) { continue } subs, err := os.ReadDir(filepath.Join(brainDir, "wiki", e.Name())) @@ -37,3 +39,8 @@ func KnownWings(brainDir string) ([]string, error) { sort.Strings(out) return out, nil } + +// plainWingName reports whether name is already in the canonical lower-case form +// the pipeline writes. Anything else would make the pipeline create a different +// wing than the one matched. +func plainWingName(name string) bool { return brain.Sanitise(name) == name } diff --git a/ingestion/internal/wing/known_test.go b/ingestion/internal/wing/known_test.go index e9c1f44..2e09a0c 100644 --- a/ingestion/internal/wing/known_test.go +++ b/ingestion/internal/wing/known_test.go @@ -27,3 +27,16 @@ func TestKnownWings_NoWikiDirIsAnError(t *testing.T) { _, err := wing.KnownWings(t.TempDir()) assert.Error(t, err, "a brain without wiki/ must not look like a brain with no wings") } + +// Known matches case-insensitively and the pipeline writes the lower-case name, +// so a wiki directory that is not already plain lower-case would make the +// pipeline create a different, new wing. It is not a candidate. +func TestKnownWings_SkipsADirectoryThatIsNotAPlainWingName(t *testing.T) { + dir := t.TempDir() + writePage(t, dir, "wiki/Radar/facts/a.md", "x") + writePage(t, dir, "wiki/has space/facts/a.md", "x") + writePage(t, dir, "wiki/ok/facts/a.md", "x") + got, err := wing.KnownWings(dir) + require.NoError(t, err) + assert.Equal(t, []string{"ok"}, got) +}