From 409b85096b0b1d96cb9eeeaa62e1cff734cf273e Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 20 Oct 2026 20:14:47 +0200 Subject: [PATCH] feat(api): brain_promote and brain_reject act on notes parked in raw/needs-review/ (#208) brain_pending lists a parked note under its bare filename, but promote/reject only read raw/, so the documented approval path did not exist. They now fall back to raw/needs-review/ (top level wins on a name clash; a symlinked directory is refused) and promote drops the resolver's proposal keys. Mutation-checked. Co-Authored-By: Claude Sonnet 5.5 --- ingestion/internal/api/frontmatter.go | 12 +++ ingestion/internal/api/promote.go | 43 ++++++++--- ingestion/internal/api/promote_review_test.go | 75 +++++++++++++++++++ 3 files changed, 120 insertions(+), 10 deletions(-) create mode 100644 ingestion/internal/api/promote_review_test.go diff --git a/ingestion/internal/api/frontmatter.go b/ingestion/internal/api/frontmatter.go index f0614b4..3cfc7d7 100644 --- a/ingestion/internal/api/frontmatter.go +++ b/ingestion/internal/api/frontmatter.go @@ -83,6 +83,18 @@ func (f *frontmatter) get(key string) string { return "" } +// drop removes every `key: value` line for key (nothing else), so a +// machine-written key does not travel with a note into the wiki. +func (f *frontmatter) drop(key string) { + kept := f.lines[:0] + for _, l := range f.lines { + if l.key != key { + kept = append(kept, l) + } + } + f.lines = kept +} + // set overrides the value for an existing key in place, or appends a new // `key: value` line when the key is absent. func (f *frontmatter) set(key, value string) { diff --git a/ingestion/internal/api/promote.go b/ingestion/internal/api/promote.go index 756641a..6a1a966 100644 --- a/ingestion/internal/api/promote.go +++ b/ingestion/internal/api/promote.go @@ -143,11 +143,11 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) { return "", err } - src := filepath.Join(brainDir, "raw", base) - raw, err := readRawSource(brainDir, base) + raw, srcRel, err := readRawSource(brainDir, base) if err != nil { return "", err } + src := filepath.Join(brainDir, filepath.FromSlash(srcRel)) // Collision: never silently overwrite an existing promoted note. if _, statErr := os.Stat(dest); statErr == nil { @@ -156,6 +156,11 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) { } fm, body := parseFrontmatter(string(raw)) + // The resolver's proposal described where the note was going to be placed; + // once a human has placed it, it is noise (and `wing` is set below). + for _, k := range []string{"proposed_wing", "proposal_confidence", "proposal_reason"} { + fm.drop(k) + } now := time.Now().UTC().Format(time.RFC3339) fm.set("wing", brain.Sanitise(opts.Wing)) fm.set("hall", opts.Hall) @@ -215,11 +220,11 @@ func RejectNote(brainDir string, opts RejectOptions) (string, error) { return "", fmt.Errorf("invalid filename %q", opts.Filename) } - src := filepath.Join(brainDir, "raw", base) - raw, err := readRawSource(brainDir, base) + raw, srcRel, err := readRawSource(brainDir, base) if err != nil { return "", err } + src := filepath.Join(brainDir, filepath.FromSlash(srcRel)) dest := filepath.Join(brainDir, "raw", "rejected", base) if _, statErr := os.Stat(dest); statErr == nil { @@ -258,25 +263,43 @@ func excerpt(s string, n int) string { // readRawSource reads a pending note for promote/reject through the raw root, // refusing symlinks and a symlinked raw/ (hyperguild#153). -func readRawSource(brainDir, base string) ([]byte, error) { +func readRawSource(brainDir, base string) ([]byte, string, error) { root, _, err := OpenRawRoot(brainDir) if err != nil { if errors.Is(err, os.ErrNotExist) { - return nil, fmt.Errorf("pending note %q does not exist in raw/", base) + return nil, "", fmt.Errorf("pending note %q does not exist in raw/", base) } - return nil, fmt.Errorf("read source: %w", err) + return nil, "", fmt.Errorf("read source: %w", err) } defer func() { _ = root.Close() }() raw, _, err := readRawNote(root, base) + if err == nil { + return raw, "raw/" + base, nil + } + if !errors.Is(err, os.ErrNotExist) { + return nil, "", fmt.Errorf("read source: %w", err) + } + // brain_pending also lists notes parked in raw/needs-review/ under their + // bare filename (hyperguild#208); the top level wins on a name clash. The + // sub-root refuses a symlinked directory like OpenRawRoot does. + sub, serr := root.OpenRoot(parkedDir) + if serr != nil { + return nil, "", fmt.Errorf("pending note %q does not exist in raw/", base) + } + defer func() { _ = sub.Close() }() + raw, _, err = readRawNote(sub, base) if err != nil { if errors.Is(err, os.ErrNotExist) { - return nil, fmt.Errorf("pending note %q does not exist in raw/", base) + return nil, "", fmt.Errorf("pending note %q does not exist in raw/", base) } - return nil, fmt.Errorf("read source: %w", err) + return nil, "", fmt.Errorf("read source: %w", err) } - return raw, nil + return raw, "raw/" + parkedDir + "/" + base, nil } +// parkedDir is where the watcher parks notes it could not place. +const parkedDir = "needs-review" + // unquoteScalar undoes the Go-style double quoting the watcher writes around a // free-text value (strconv.Quote keeps it on one line); a bare value is // returned as is. diff --git a/ingestion/internal/api/promote_review_test.go b/ingestion/internal/api/promote_review_test.go new file mode 100644 index 0000000..37e7df5 --- /dev/null +++ b/ingestion/internal/api/promote_review_test.go @@ -0,0 +1,75 @@ +package api + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const parkedNote = "---\nproposed_wing: radar\nproposal_confidence: 0.40\nproposal_reason: \"text matches the client-content marker\"\ncreated_at: 2026-10-19T08:00:00Z\n---\nBody of the parked note.\n" + +// brain_pending lists a parked note under its bare filename, so promote and +// reject must be able to act on exactly that name (#208: they could not). +func TestPromoteNote_ActsOnANoteParkedInNeedsReview(t *testing.T) { + dir := t.TempDir() + writeParked(t, dir, "unsure.md", parkedNote) + + rel, err := PromoteNote(dir, PromoteOptions{Filename: "unsure.md", Wing: "homelab", Hall: "facts"}) + require.NoError(t, err) + assert.Equal(t, "wiki/homelab/facts/unsure.md", rel) + + got, err := os.ReadFile(filepath.Join(dir, "wiki/homelab/facts/unsure.md")) + require.NoError(t, err) + assert.Contains(t, string(got), "Body of the parked note.") + assert.Contains(t, string(got), "wing: homelab") + assert.NotContains(t, string(got), "proposed_wing", "the resolver's proposal is not part of the promoted note") + assert.NotContains(t, string(got), "proposal_reason") + + _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "unsure.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist, "the parked note is consumed") + pending, err := ListPending(dir) + require.NoError(t, err) + assert.Empty(t, pending) +} + +func TestRejectNote_ActsOnANoteParkedInNeedsReview(t *testing.T) { + dir := t.TempDir() + writeParked(t, dir, "unsure.md", parkedNote) + + rel, err := RejectNote(dir, RejectOptions{Filename: "unsure.md", Reason: "not worth keeping"}) + require.NoError(t, err) + assert.Equal(t, "raw/rejected/unsure.md", rel) + _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "unsure.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist) + pending, _ := ListPending(dir) + assert.Empty(t, pending, "a rejected note is not re-listed") +} + +func TestPromoteNote_ATopLevelNoteWinsOverAParkedOneOfTheSameName(t *testing.T) { + dir := t.TempDir() + writeRaw(t, dir, "same.md", "---\ncreated_at: 2026-10-20T08:00:00Z\n---\nTop-level body.\n") + writeParked(t, dir, "same.md", parkedNote) + + _, err := PromoteNote(dir, PromoteOptions{Filename: "same.md", Wing: "homelab", Hall: "facts"}) + require.NoError(t, err) + got, _ := os.ReadFile(filepath.Join(dir, "wiki/homelab/facts/same.md")) + assert.Contains(t, string(got), "Top-level body.") + _, statErr := os.Stat(filepath.Join(dir, "raw", "needs-review", "same.md")) + assert.NoError(t, statErr, "the parked note of the same name is left alone") +} + +func TestPromoteNote_ASymlinkedNeedsReviewIsNotRead(t *testing.T) { + dir := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(outside, "secret.md"), []byte("outside"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755)) + require.NoError(t, os.Symlink(outside, filepath.Join(dir, "raw", "needs-review"))) + + _, err := PromoteNote(dir, PromoteOptions{Filename: "secret.md", Wing: "homelab", Hall: "facts"}) + assert.Error(t, err) + _, statErr := os.Stat(filepath.Join(dir, "wiki/homelab/facts/secret.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist) +}