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 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,92 @@
|
|||||||
|
// Command wingmigrate moves pages out of the retired wiki/{sources,concepts,
|
||||||
|
// entities}/ layout into wiki/<wing>/ (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, "|", "/"))
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -3,8 +3,10 @@ package watcher
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
"io/fs"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
@@ -13,6 +15,7 @@ import (
|
|||||||
"time"
|
"time"
|
||||||
"unicode"
|
"unicode"
|
||||||
|
|
||||||
|
"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
|
||||||
"github.com/mathiasbq/hyperguild/ingestion/internal/extract"
|
"github.com/mathiasbq/hyperguild/ingestion/internal/extract"
|
||||||
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
|
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
|
||||||
"github.com/mathiasbq/hyperguild/ingestion/internal/wing"
|
"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.
|
// frontmatterWing reads `wing:` from a note's leading frontmatter, "" if none.
|
||||||
func frontmatterWing(content string) string {
|
func frontmatterWing(content string) string {
|
||||||
|
content = strings.ReplaceAll(content, "\r\n", "\n")
|
||||||
if !strings.HasPrefix(content, "---\n") {
|
if !strings.HasPrefix(content, "---\n") {
|
||||||
return ""
|
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 {
|
if err := os.MkdirAll(dir, 0o755); err != nil {
|
||||||
return fmt.Errorf("mkdir needs-review: %w", err)
|
return fmt.Errorf("mkdir needs-review: %w", err)
|
||||||
}
|
}
|
||||||
dest := filepath.Join(dir, filename)
|
dest, err := reserveParkedName(dir, filename)
|
||||||
if _, err := os.Stat(dest); err == nil {
|
if err != nil {
|
||||||
ext := filepath.Ext(filename)
|
return fmt.Errorf("park note: %w", err)
|
||||||
dest = filepath.Join(dir, strings.TrimSuffix(filename, ext)+"-"+time.Now().UTC().Format("150405")+ext)
|
|
||||||
}
|
}
|
||||||
if strings.ToLower(filepath.Ext(filename)) == ".pdf" {
|
if strings.ToLower(filepath.Ext(filename)) == ".pdf" {
|
||||||
// Binary: park the bytes untouched; the proposal is in log.md only.
|
// Binary: park the bytes untouched; the proposal is in log.md only.
|
||||||
if err := copyFromRoot(root, rel, dest); err != nil {
|
if err := copyFromRoot(root, rel, dest); err != nil {
|
||||||
|
_ = os.Remove(dest)
|
||||||
return fmt.Errorf("park pdf: %w", err)
|
return fmt.Errorf("park pdf: %w", err)
|
||||||
}
|
}
|
||||||
} else if err := os.WriteFile(dest, []byte(withProposal(content, d)), 0o644); err != nil {
|
} else if err := os.WriteFile(dest, []byte(withProposal(content, d)), 0o644); err != nil {
|
||||||
|
_ = os.Remove(dest)
|
||||||
return fmt.Errorf("park note: %w", err)
|
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 {
|
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
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// withProposal adds the resolver's proposal to a note's frontmatter, creating
|
// reserveParkedName claims a free name in dir for filename, never reusing one:
|
||||||
// the block when the note has none.
|
// the parked copy is the only copy once the original is retired, so a note
|
||||||
func withProposal(content string, d wing.Decision) string {
|
// that shares a name with an earlier parked note must not replace it. The name
|
||||||
kv := fmt.Sprintf("proposed_wing: %s\nproposal_confidence: %.2f\nproposal_reason: %q\n", d.Wing, d.Confidence, d.Reason)
|
// is created with O_EXCL; -2, -3, ... is appended until one is free.
|
||||||
if strings.HasPrefix(content, "---\n") {
|
func reserveParkedName(dir, filename string) (string, error) {
|
||||||
return "---\n" + kv + content[4:]
|
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)
|
||||||
}
|
}
|
||||||
return "---\n" + kv + "---\n" + content
|
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 "", 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
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
"testing"
|
"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")
|
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
|
||||||
|
}
|
||||||
|
|||||||
@@ -5,6 +5,8 @@ import (
|
|||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"sort"
|
"sort"
|
||||||
|
|
||||||
|
"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
|
||||||
)
|
)
|
||||||
|
|
||||||
// legacyDirs are the retired wiki/{concepts,entities,sources}/ layout, not wings.
|
// legacyDirs are the retired wiki/{concepts,entities,sources}/ layout, not wings.
|
||||||
@@ -20,7 +22,7 @@ func KnownWings(brainDir string) ([]string, error) {
|
|||||||
}
|
}
|
||||||
var out []string
|
var out []string
|
||||||
for _, e := range entries {
|
for _, e := range entries {
|
||||||
if !e.IsDir() || legacyDirs[e.Name()] {
|
if !e.IsDir() || legacyDirs[e.Name()] || !plainWingName(e.Name()) {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
subs, err := os.ReadDir(filepath.Join(brainDir, "wiki", e.Name()))
|
subs, err := os.ReadDir(filepath.Join(brainDir, "wiki", e.Name()))
|
||||||
@@ -37,3 +39,8 @@ func KnownWings(brainDir string) ([]string, error) {
|
|||||||
sort.Strings(out)
|
sort.Strings(out)
|
||||||
return out, nil
|
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 }
|
||||||
|
|||||||
@@ -27,3 +27,16 @@ func TestKnownWings_NoWikiDirIsAnError(t *testing.T) {
|
|||||||
_, err := wing.KnownWings(t.TempDir())
|
_, err := wing.KnownWings(t.TempDir())
|
||||||
assert.Error(t, err, "a brain without wiki/ must not look like a brain with no wings")
|
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)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user