From f324f04944ea842b09e6dd8f492aa970511b9ec3 Mon Sep 17 00:00:00 2001 From: Mathias Date: Wed, 23 Sep 2026 05:43:28 +0000 Subject: [PATCH] fix: hyperguild brain write requires wing/hall, not legacy type/slug (#79) Removes brain_write legacy domain/type/knowledge-path params (ADR-0012) and migrates the previously-missed cmd/hyperguild CLI caller (hyperguild brain write) from to , fixing a 100%% regression the first review round caught. Adds TestWrite_CLIPayloadRoundTrips exercising the real Handler.Write over HTTP to pin the wire contract. Closes #79. Co-authored-by: Mathias --- cmd/hyperguild/README.md | 11 +- cmd/hyperguild/brain.go | 11 +- cmd/hyperguild/brain_test.go | 10 +- cmd/hyperguild/http.go | 11 +- cmd/hyperguild/http_test.go | 18 +- .../specs/2026-05-03-hyperguild-cli-design.md | 4 +- ingestion/internal/api/handler.go | 84 ++------ ingestion/internal/api/handler_test.go | 144 +++++++++----- ingestion/internal/brainstore/store.go | 2 - ingestion/internal/capture/ports.go | 7 +- ingestion/internal/mcp/handlers.go | 14 +- ingestion/internal/mcp/handlers_test.go | 64 +++--- internal/skills/brain/handlers.go | 182 ------------------ internal/skills/brain/handlers_test.go | 122 ------------ internal/skills/brain/skill.go | 111 ----------- 15 files changed, 188 insertions(+), 607 deletions(-) delete mode 100644 internal/skills/brain/handlers.go delete mode 100644 internal/skills/brain/handlers_test.go delete mode 100644 internal/skills/brain/skill.go diff --git a/cmd/hyperguild/README.md b/cmd/hyperguild/README.md index 0984f5a..62d9b9b 100644 --- a/cmd/hyperguild/README.md +++ b/cmd/hyperguild/README.md @@ -55,18 +55,21 @@ Flags: - `--limit N` — max results (default 5) - `--json` — emit the raw response envelope -### `hyperguild brain write ` +### `hyperguild brain write ` -Reads markdown from stdin, writes a knowledge entry. +Reads markdown from stdin, writes it to `brain/wiki///.md` +with `wing`/`hall`/`created_at` frontmatter injected (there is no legacy +`brain/knowledge/` fallback — ADR-0012 / #79, both wing and hall are +required). ```bash -$ cat <` diff --git a/cmd/hyperguild/brain.go b/cmd/hyperguild/brain.go index 9d638e8..484ea03 100644 --- a/cmd/hyperguild/brain.go +++ b/cmd/hyperguild/brain.go @@ -60,13 +60,14 @@ func runBrainWrite(ctx context.Context, args []string, stdin io.Reader, stdout, if err := fs.Parse(args); err != nil { return fmt.Errorf("parse flags: %w", err) } - if fs.NArg() < 2 { - return errors.New("type and slug required (e.g. brain write knowledge my-slug)") + if fs.NArg() < 3 { + return errors.New("wing, hall and slug required (e.g. brain write homelab facts my-slug)") } - kind := fs.Arg(0) - slug := fs.Arg(1) + wing := fs.Arg(0) + hall := fs.Arg(1) + slug := fs.Arg(2) - res, err := newBrainClient().Write(ctx, kind, slug, stdin) + res, err := newBrainClient().Write(ctx, wing, hall, slug, stdin) if err != nil { return err } diff --git a/cmd/hyperguild/brain_test.go b/cmd/hyperguild/brain_test.go index 74b4062..589f7fd 100644 --- a/cmd/hyperguild/brain_test.go +++ b/cmd/hyperguild/brain_test.go @@ -101,7 +101,7 @@ func TestRunBrainWrite_Success(t *testing.T) { var out, errBuf bytes.Buffer err := runBrain( context.Background(), - []string{"write", "knowledge", "test-slug"}, + []string{"write", "homelab", "facts", "test-slug"}, strings.NewReader("# Test\n\nSome body content.\n"), &out, &errBuf, ) @@ -111,9 +111,9 @@ func TestRunBrainWrite_Success(t *testing.T) { func TestRunBrainWrite_MissingArgs(t *testing.T) { var out, errBuf bytes.Buffer - err := runBrain(context.Background(), []string{"write", "knowledge"}, strings.NewReader("x"), &out, &errBuf) + err := runBrain(context.Background(), []string{"write", "homelab"}, strings.NewReader("x"), &out, &errBuf) assert.Error(t, err) - assert.Contains(t, err.Error(), "type and slug required") + assert.Contains(t, err.Error(), "wing, hall and slug required") } func TestRunBrainWrite_BackendError(t *testing.T) { @@ -127,7 +127,7 @@ func TestRunBrainWrite_BackendError(t *testing.T) { var out, errBuf bytes.Buffer err := runBrain( context.Background(), - []string{"write", "knowledge", "bad slug"}, + []string{"write", "homelab", "facts", "bad slug"}, strings.NewReader("body"), &out, &errBuf, ) @@ -150,7 +150,7 @@ func TestRunBrainWrite_EmptyStdin(t *testing.T) { t.Setenv("BRAIN_URL", srv.URL) var out, errBuf bytes.Buffer - err := runBrain(context.Background(), []string{"write", "knowledge", "empty"}, strings.NewReader(""), &out, &errBuf) + err := runBrain(context.Background(), []string{"write", "homelab", "facts", "empty"}, strings.NewReader(""), &out, &errBuf) require.NoError(t, err) assert.Equal(t, 0, gotLen, "empty stdin should produce empty content payload") } diff --git a/cmd/hyperguild/http.go b/cmd/hyperguild/http.go index d3847cb..b6e9b12 100644 --- a/cmd/hyperguild/http.go +++ b/cmd/hyperguild/http.go @@ -84,16 +84,17 @@ type WriteResult struct { Path string `json:"path"` } -func (c *brainClient) Write(ctx context.Context, kind, slug string, content io.Reader) (*WriteResult, error) { +func (c *brainClient) Write(ctx context.Context, wing, hall, filename string, content io.Reader) (*WriteResult, error) { body, err := io.ReadAll(content) if err != nil { return nil, fmt.Errorf("read content: %w", err) } payload, err := json.Marshal(struct { - Type string `json:"type"` - Slug string `json:"slug"` - Content string `json:"content"` - }{Type: kind, Slug: slug, Content: string(body)}) + Wing string `json:"wing"` + Hall string `json:"hall"` + Filename string `json:"filename"` + Content string `json:"content"` + }{Wing: wing, Hall: hall, Filename: filename, Content: string(body)}) if err != nil { return nil, fmt.Errorf("marshal payload: %w", err) } diff --git a/cmd/hyperguild/http_test.go b/cmd/hyperguild/http_test.go index ffa402a..177a545 100644 --- a/cmd/hyperguild/http_test.go +++ b/cmd/hyperguild/http_test.go @@ -65,23 +65,25 @@ func TestBrainClient_Write_Success(t *testing.T) { assert.Equal(t, http.MethodPost, r.Method) body, _ := io.ReadAll(r.Body) var got struct { - Type string `json:"type"` - Slug string `json:"slug"` - Content string `json:"content"` + Wing string `json:"wing"` + Hall string `json:"hall"` + Filename string `json:"filename"` + Content string `json:"content"` } require.NoError(t, json.Unmarshal(body, &got)) - assert.Equal(t, "knowledge", got.Type) - assert.Equal(t, "find-h", got.Slug) + assert.Equal(t, "homelab", got.Wing) + assert.Equal(t, "facts", got.Hall) + assert.Equal(t, "find-h", got.Filename) assert.Equal(t, "# body\n", got.Content) w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"path":"knowledge/find-h.md"}`)) + _, _ = w.Write([]byte(`{"path":"wiki/homelab/facts/find-h.md"}`)) })) defer srv.Close() c := &brainClient{baseURL: srv.URL, http: srv.Client()} - res, err := c.Write(context.Background(), "knowledge", "find-h", strings.NewReader("# body\n")) + res, err := c.Write(context.Background(), "homelab", "facts", "find-h", strings.NewReader("# body\n")) require.NoError(t, err) - assert.Equal(t, "knowledge/find-h.md", res.Path) + assert.Equal(t, "wiki/homelab/facts/find-h.md", res.Path) } func TestBrainClient_PassRate_Success(t *testing.T) { diff --git a/docs/superpowers/specs/2026-05-03-hyperguild-cli-design.md b/docs/superpowers/specs/2026-05-03-hyperguild-cli-design.md index ed04577..96500f3 100644 --- a/docs/superpowers/specs/2026-05-03-hyperguild-cli-design.md +++ b/docs/superpowers/specs/2026-05-03-hyperguild-cli-design.md @@ -16,7 +16,7 @@ Three needs converge on a single small Go binary: - [ ] `hyperguild tier` returns the same `tier.Info` that `internal/tier.Detect` produces for the same probe URLs, in < 3 s under all three tier conditions, with both human-readable and `--json` output. - [ ] `hyperguild brain query ` returns BM25 results from the brain HTTP REST `/query` endpoint, exit 0 on success and non-zero on transport failure. -- [ ] `hyperguild brain write ` reads markdown content from stdin, posts to `/write` with the type and slug, and creates `brain/knowledge/.md`. A round-trip (`hyperguild brain query ` immediately after) finds the entry. +- [ ] `hyperguild brain write ` reads markdown content from stdin, posts to `/write` with wing/hall/filename, and creates `brain/wiki///.md`. A round-trip (`hyperguild brain query ` immediately after) finds the entry. (Updated post-ADR-0012/#79: the legacy ` ` shape and the `brain/knowledge/` fallback were removed; wing and hall are now required.) - [ ] `hyperguild mode ` writes a parseable JSON file at the target path with the per-mode `mcpServers` entries; `jq -e .mcpServers` succeeds on the output. - [ ] All commands print usage on `--help`, exit 2 on unknown flags, exit non-zero on operational errors. - [ ] `task check` passes (lint + test + vet) on each task and on the merged branch. @@ -50,7 +50,7 @@ Three needs converge on a single small Go binary: - **`brain` subcommand calls the HTTP REST API.** Two nested subcommands: - `brain query ` issues `POST /query` with JSON body `{query, limit}` (default `--limit 5`), prints results in human-readable form by default and with `--json` for machine consumption. - - `brain write ` reads stdin, posts `POST /write` with JSON body `{type, slug, content}`, prints the resulting path on success. + - `brain write ` reads stdin, posts `POST /write` with JSON body `{wing, hall, filename, content}`, prints the resulting path on success. (Updated post-ADR-0012/#79: wing and hall are required, replacing the old ` ` shape and the `brain/knowledge/` legacy fallback.) Rationale: HTTP REST is simpler than MCP framing for a CLI. Per CLAUDE.md, the REST endpoints are documented as the official non-MCP interface. - **`mode ` writes a per-mode `.mcp.json` template.** Defaults to writing `./.mcp.json` (cwd); accepts `--out `. Per-mode bodies: diff --git a/ingestion/internal/api/handler.go b/ingestion/internal/api/handler.go index 7e75bc3..bdff89c 100644 --- a/ingestion/internal/api/handler.go +++ b/ingestion/internal/api/handler.go @@ -54,8 +54,6 @@ type queryRequest struct { type writeRequest struct { Content string `json:"content"` Filename string `json:"filename,omitempty"` - Type string `json:"type,omitempty"` - Domain string `json:"domain,omitempty"` Wing string `json:"wing,omitempty"` Hall string `json:"hall,omitempty"` SourceType string `json:"source_type,omitempty"` // "external" opts a hall=facts entry out of the internal default @@ -110,17 +108,13 @@ func (h *Handler) Query(w http.ResponseWriter, r *http.Request) { // WriteNoteOptions configures how a brain note is written. // -// When both Wing and Hall are non-empty, the note routes into the -// structured wiki at brain/wiki///.md and gets -// wing/hall/created_at injected into its YAML frontmatter. -// -// When either is empty, the note falls back to brain/knowledge/ -// with optional type/domain frontmatter (legacy behaviour). +// Wing and Hall are both required: the note routes into the structured +// wiki at brain/wiki///.md and gets wing/hall/created_at +// injected into its YAML frontmatter. There is no other valid destination +// (the legacy brain/knowledge/ fallback was removed — ADR-0012 / #79). type WriteNoteOptions struct { Content string Filename string - Type string - Domain string Wing string Hall string SourceType string // "internal" marks a first-party observation (e.g. claudewatcher) that needs no external citation @@ -132,6 +126,9 @@ func WriteNote(brainDir string, opts WriteNoteOptions) (string, error) { if opts.Content == "" { return "", fmt.Errorf("content is required") } + if opts.Wing == "" || opts.Hall == "" { + return "", fmt.Errorf("wing and hall are required") + } // In the primitive, not in a caller: POST /write reaches here directly, // without the BrainStore, and a gate one door wide is not a gate. @@ -143,13 +140,7 @@ func WriteNote(brainDir string, opts WriteNoteOptions) (string, error) { } opts.Content = clean - if opts.Wing != "" && opts.Hall != "" { - return writeHallNote(brainDir, opts) - } - if opts.Wing != "" || opts.Hall != "" { - return "", fmt.Errorf("wing and hall must be set together") - } - return writeLegacyNote(brainDir, opts) + return writeHallNote(brainDir, opts) } // writeHallNote routes a note into brain/wiki/// and injects @@ -207,9 +198,6 @@ func writeHallNote(brainDir string, opts WriteNoteOptions) (string, error) { fmt.Fprintf(&fm, "%s: %s\n", key, fallback) } } - writeField("type", opts.Type) - writeField("domain", opts.Domain) - sourceType := opts.SourceType if sourceType == "" && opts.Hall == "facts" { // Most hall=facts entries are first-party (an eval/benchmark the @@ -300,53 +288,7 @@ func splitFrontmatter(content string) (fields []frontmatterField, body string) { return fields, body } -// writeLegacyNote preserves the original brain/knowledge/ behaviour for -// callers that have not adopted the wing/hall taxonomy. -func writeLegacyNote(brainDir string, opts WriteNoteOptions) (string, error) { - filename := opts.Filename - if filename == "" { - filename = fmt.Sprintf("%s-auto.md", time.Now().UTC().Format("2006-01-02-150405")) - } - - rawDir := filepath.Join(brainDir, "knowledge") - if err := os.MkdirAll(rawDir, 0o755); err != nil { - return "", fmt.Errorf("create raw dir: %w", err) - } - - finalContent := opts.Content - if opts.Type != "" || opts.Domain != "" { - var fm strings.Builder - fm.WriteString("---\n") - if opts.Type != "" { - fmt.Fprintf(&fm, "type: %s\n", opts.Type) - } - if opts.Domain != "" { - fmt.Fprintf(&fm, "domain: %s\n", opts.Domain) - } - fm.WriteString("---\n") - finalContent = fm.String() + opts.Content - } - - if strings.ContainsAny(filename, `/\`) { - return "", fmt.Errorf("invalid filename") - } - base := filepath.Base(filename) - if base == "." || base == ".." || base == "" { - return "", fmt.Errorf("invalid filename") - } - if !strings.HasSuffix(base, ".md") { - base += ".md" - } - dest := filepath.Join(rawDir, base) - if err := os.WriteFile(dest, []byte(finalContent), 0o644); err != nil { - return "", fmt.Errorf("write: %w", err) - } - - rel, _ := filepath.Rel(brainDir, dest) - return filepath.ToSlash(rel), nil -} - -// Write handles POST /write — write raw content to brain/knowledge/. +// Write handles POST /write — write a note into brain/wiki///. func (h *Handler) Write(w http.ResponseWriter, r *http.Request) { var req writeRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { @@ -359,10 +301,10 @@ func (h *Handler) Write(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, err.Error()) return } - if req.Wing != "" && req.Hall != "" { - if err := brain.BuildWingIndex(h.brainDir, req.Wing); err != nil { - h.logger.Warn("auto-index failed", "wing", req.Wing, "err", err) - } + // WriteNote errors out above unless both are set, so this always runs — + // no legacy fallback path left to guard against. + if err := brain.BuildWingIndex(h.brainDir, req.Wing); err != nil { + h.logger.Warn("auto-index failed", "wing", req.Wing, "err", err) } writeJSON(w, map[string]string{"path": relPath}) } diff --git a/ingestion/internal/api/handler_test.go b/ingestion/internal/api/handler_test.go index bd5acc7..b5131cd 100644 --- a/ingestion/internal/api/handler_test.go +++ b/ingestion/internal/api/handler_test.go @@ -65,11 +65,13 @@ func TestQuery_ReturnsResults(t *testing.T) { assert.NotEmpty(t, results) } -func TestWrite_CreatesKnowledgeFile(t *testing.T) { +func TestWrite_CreatesWikiFile(t *testing.T) { dir, h := setup(t) body, _ := json.Marshal(map[string]any{ "content": "# Test note\n\nSome content.", - "filename": "test-note.md", + "filename": "test-note", + "wing": "homelab", + "hall": "facts", }) req := httptest.NewRequest(http.MethodPost, "/write", bytes.NewReader(body)) rec := httptest.NewRecorder() @@ -81,11 +83,25 @@ func TestWrite_CreatesKnowledgeFile(t *testing.T) { require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) assert.NotEmpty(t, resp["path"]) - content, err := os.ReadFile(filepath.Join(dir, "knowledge", "test-note.md")) + content, err := os.ReadFile(filepath.Join(dir, "wiki", "homelab", "facts", "test-note.md")) require.NoError(t, err) assert.Contains(t, string(content), "Some content.") } +// TestWrite_RequiresWingAndHall is the HTTP-contract counterpart of +// TestWriteNote_RequiresWingAndHall: a caller (e.g. the MCP brain_write +// handler) that omits wing/hall gets a 400, not a silent legacy write. +func TestWrite_RequiresWingAndHall(t *testing.T) { + _, h := setup(t) + body, _ := json.Marshal(map[string]any{"content": "no destination"}) + req := httptest.NewRequest(http.MethodPost, "/write", bytes.NewReader(body)) + rec := httptest.NewRecorder() + + h.Write(rec, req) + + assert.Equal(t, http.StatusBadRequest, rec.Code) +} + func TestQuery_RequiresQuery(t *testing.T) { _, h := setup(t) body, _ := json.Marshal(map[string]any{"limit": 5}) @@ -97,27 +113,6 @@ func TestQuery_RequiresQuery(t *testing.T) { assert.Equal(t, http.StatusBadRequest, rec.Code) } -func TestWrite_IncludesFrontmatterWhenTypeProvided(t *testing.T) { - dir, h := setup(t) - body, _ := json.Marshal(map[string]any{ - "content": "Some learning.", - "filename": "typed-note.md", - "type": "concept", - "domain": "software", - }) - req := httptest.NewRequest(http.MethodPost, "/write", bytes.NewReader(body)) - rec := httptest.NewRecorder() - - h.Write(rec, req) - - assert.Equal(t, http.StatusOK, rec.Code) - content, err := os.ReadFile(filepath.Join(dir, "knowledge", "typed-note.md")) - require.NoError(t, err) - assert.Contains(t, string(content), "type: concept") - assert.Contains(t, string(content), "domain: software") - assert.Contains(t, string(content), "Some learning.") -} - func TestWriteNote_HallRouteIncludesSourceTypeWhenSet(t *testing.T) { dir := t.TempDir() @@ -126,7 +121,6 @@ func TestWriteNote_HallRouteIncludesSourceTypeWhenSet(t *testing.T) { Filename: "session-koala-abc", Wing: "claude-sessions", Hall: "facts", - Type: "source", SourceType: "internal", }) require.NoError(t, err) @@ -210,37 +204,95 @@ func TestWriteNote_HallRouteMergesExistingFrontmatterInsteadOfStacking(t *testin assert.Contains(t, body, "# Body") } -func TestWriteNote_HallRouteExistingTypeWinsOverOptsType(t *testing.T) { - dir := t.TempDir() - - rel, err := api.WriteNote(dir, api.WriteNoteOptions{ - Content: "---\ntype: hypothesis\n---\n\nBody.\n", - Filename: "note", - Wing: "agentsquad", - Hall: "decisions", - Type: "decision", // should lose to content's own "type: hypothesis" - }) - require.NoError(t, err) - - got, err := os.ReadFile(filepath.Join(dir, filepath.FromSlash(rel))) - require.NoError(t, err) - assert.Contains(t, string(got), "type: hypothesis") - assert.NotContains(t, string(got), "type: decision") +// TestWriteNote_RequiresWingAndHall pins the post-legacy-removal contract +// (hyperguild#79 / ADR-0012): with the knowledge/ fallback gone, wing+hall +// is the only valid destination, so omitting either is a caller error, not +// a silent fallback. +func TestWriteNote_RequiresWingAndHall(t *testing.T) { + cases := []struct { + name string + wing string + hall string + }{ + {"neither set", "", ""}, + {"wing only", "homelab", ""}, + {"hall only", "", "facts"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + _, err := api.WriteNote(dir, api.WriteNoteOptions{ + Content: "body", + Wing: tc.wing, + Hall: tc.hall, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "wing and hall are required") + }) + } } func TestWrite_GeneratesFilenameIfAbsent(t *testing.T) { dir, h := setup(t) - body, _ := json.Marshal(map[string]any{"content": "auto name"}) + body, _ := json.Marshal(map[string]any{ + "content": "auto name", + "wing": "homelab", + "hall": "facts", + }) req := httptest.NewRequest(http.MethodPost, "/write", bytes.NewReader(body)) rec := httptest.NewRecorder() h.Write(rec, req) assert.Equal(t, http.StatusOK, rec.Code) - entries, _ := os.ReadDir(filepath.Join(dir, "knowledge")) - // +1 because setup already wrote tdd.md - assert.Len(t, entries, 2) - assert.True(t, strings.HasSuffix(entries[1].Name(), ".md")) + entries, err := os.ReadDir(filepath.Join(dir, "wiki", "homelab", "facts")) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.True(t, strings.HasSuffix(entries[0].Name(), ".md")) +} + +// TestWrite_CLIPayloadRoundTrips exercises the real Handler.Write (not a +// hand-rolled mock) over an actual HTTP round trip, POSTing the exact JSON +// envelope cmd/hyperguild's brainClient.Write sends: {wing, hall, filename, +// content}. cmd/hyperguild lives in a separate Go module from ingestion +// (no shared go.work), so a Go-level test can't import brainClient +// directly — this test instead pins the wire contract from the server +// side and proves a real `hyperguild brain write ` +// invocation writes a real file with real frontmatter under wiki///. +// A prior version of this contract test on the client side (in +// cmd/hyperguild/http_test.go) used only an httptest.NewServer with a +// hand-rolled mock body, so it stayed green through a regression where the +// server required wing/hall but the client never sent them (hyperguild#79 +// PR review). +func TestWrite_CLIPayloadRoundTrips(t *testing.T) { + dir, h := setup(t) + srv := httptest.NewServer(http.HandlerFunc(h.Write)) + defer srv.Close() + + payload, err := json.Marshal(struct { + Wing string `json:"wing"` + Hall string `json:"hall"` + Filename string `json:"filename"` + Content string `json:"content"` + }{Wing: "homelab", Hall: "facts", Filename: "cli-roundtrip", Content: "# CLI roundtrip\n\nReal write.\n"}) + require.NoError(t, err) + + resp, err := http.Post(srv.URL+"/write", "application/json", bytes.NewReader(payload)) //nolint:noctx + require.NoError(t, err) + defer resp.Body.Close() //nolint:errcheck + + assert.Equal(t, http.StatusOK, resp.StatusCode) + var out struct { + Path string `json:"path"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&out)) + assert.Equal(t, "wiki/homelab/facts/cli-roundtrip.md", out.Path) + + written, err := os.ReadFile(filepath.Join(dir, "wiki", "homelab", "facts", "cli-roundtrip.md")) + require.NoError(t, err) + assert.Contains(t, string(written), "wing: homelab") + assert.Contains(t, string(written), "hall: facts") + assert.Contains(t, string(written), "Real write.") } // --------------------------------------------------------------------------- diff --git a/ingestion/internal/brainstore/store.go b/ingestion/internal/brainstore/store.go index f2dcc0e..8f30ef7 100644 --- a/ingestion/internal/brainstore/store.go +++ b/ingestion/internal/brainstore/store.go @@ -51,8 +51,6 @@ func (s *Store) Write(ctx context.Context, n capture.Note) (capture.Ref, error) relPath, err := api.WriteNote(s.brainDir, api.WriteNoteOptions{ Content: n.Content, Filename: n.Filename, - Type: n.Type, - Domain: n.Domain, Wing: n.Wing, Hall: n.Hall, }) diff --git a/ingestion/internal/capture/ports.go b/ingestion/internal/capture/ports.go index f850e87..7952e3d 100644 --- a/ingestion/internal/capture/ports.go +++ b/ingestion/internal/capture/ports.go @@ -31,17 +31,14 @@ type StoredNote struct { Body string } -// Note is the brain-write payload. It carries both the wing/hall taxonomy -// and the legacy type/domain fields so a single BrainStore serves both -// capture insights and the existing MCP brain_write surface. Reason is +// Note is the brain-write payload: wing+hall are the only routing fields +// (the legacy type/domain params were removed — ADR-0012 / #79). Reason is // the supersede rationale, used only by Update. type Note struct { Content string Filename string Wing string Hall string - Type string - Domain string Reason string } diff --git a/ingestion/internal/mcp/handlers.go b/ingestion/internal/mcp/handlers.go index 1aa963d..c60f52e 100644 --- a/ingestion/internal/mcp/handlers.go +++ b/ingestion/internal/mcp/handlers.go @@ -53,14 +53,12 @@ func (s *Server) tools() []map[string]any { }, { "name": "brain_write", - "description": "Write a markdown note to the brain. With wing+hall set, routes to brain/wiki/// with wing/hall/created_at frontmatter; otherwise writes to brain/knowledge/ (legacy).", - "inputSchema": schema([]string{"content"}, map[string]any{ + "description": "Write a markdown note to brain/wiki/// with wing/hall/created_at frontmatter. Both wing and hall are required.", + "inputSchema": schema([]string{"content", "wing", "hall"}, map[string]any{ "content": str("markdown content"), "filename": str("optional filename or slug"), - "type": str("optional frontmatter type (legacy)"), - "domain": str("optional frontmatter domain (legacy)"), - "wing": str("optional topic domain, e.g. jepa-fx"), - "hall": enum("optional memory type (requires wing)", halls...), + "wing": str("topic domain, e.g. jepa-fx"), + "hall": enum("memory type", halls...), }), }, { @@ -227,8 +225,6 @@ func (s *Server) brainQuery(ctx context.Context, args json.RawMessage) (json.Raw type brainWriteArgs struct { Content string `json:"content"` Filename string `json:"filename,omitempty"` - Type string `json:"type,omitempty"` - Domain string `json:"domain,omitempty"` Wing string `json:"wing,omitempty"` Hall string `json:"hall,omitempty"` } @@ -254,8 +250,6 @@ func (s *Server) brainWrite(ctx context.Context, args json.RawMessage) (json.Raw ref, err := s.store.Write(ctx, capture.Note{ Content: a.Content, Filename: a.Filename, - Type: a.Type, - Domain: a.Domain, Wing: a.Wing, Hall: a.Hall, }) diff --git a/ingestion/internal/mcp/handlers_test.go b/ingestion/internal/mcp/handlers_test.go index 7333a47..09b91e0 100644 --- a/ingestion/internal/mcp/handlers_test.go +++ b/ingestion/internal/mcp/handlers_test.go @@ -51,25 +51,50 @@ func TestBrainQueryReturnsResults(t *testing.T) { assert.Contains(t, text, "tdd.md") } -func TestBrainWriteCreatesFile(t *testing.T) { +// TestBrainWriteIgnoresLegacyTypeAndDomainParams pins the post-#79 contract: +// domain/type were removed from the schema and the handler's arg struct, so +// a caller that still sends them (the old MCP surface) gets a normal wing+ +// hall write with no error and no effect from the stale fields — silently +// dropped by json.Unmarshal, not rejected. Only content in its OWN +// frontmatter (unrelated to these params) can set type/domain, per +// TestWriteNote_HallRouteMergesExistingFrontmatterInsteadOfStacking in the +// api package. +func TestBrainWriteIgnoresLegacyTypeAndDomainParams(t *testing.T) { brainDir := t.TempDir() srv := mcp.NewServer(brainDir, nil, nil, nil) resp := toolCall(t, srv, "brain_write", map[string]any{ "content": "# Test\n\nbody", - "filename": "test.md", + "filename": "test", + "wing": "homelab", + "hall": "facts", "type": "note", "domain": "personal", }) require.Nil(t, resp["error"]) - got, err := os.ReadFile(filepath.Join(brainDir, "knowledge", "test.md")) + got, err := os.ReadFile(filepath.Join(brainDir, "wiki", "homelab", "facts", "test.md")) require.NoError(t, err) - assert.Contains(t, string(got), "type: note") - assert.Contains(t, string(got), "domain: personal") + assert.NotContains(t, string(got), "type: note") + assert.NotContains(t, string(got), "domain: personal") assert.Contains(t, string(got), "# Test") } +// TestBrainWriteRequiresWingAndHall is the MCP-tool-call counterpart of +// api.TestWriteNote_RequiresWingAndHall: the legacy brain/knowledge/ +// fallback is gone, so a caller omitting either param gets a tool error, +// not a silent legacy write. +func TestBrainWriteRequiresWingAndHall(t *testing.T) { + brainDir := t.TempDir() + srv := mcp.NewServer(brainDir, nil, nil, nil) + + resp := toolCall(t, srv, "brain_write", map[string]any{ + "content": "x", + "filename": "no-destination", + }) + require.NotNil(t, resp["error"]) +} + func TestBrainWriteWingHallRoutesToWiki(t *testing.T) { brainDir := t.TempDir() srv := mcp.NewServer(brainDir, nil, nil, nil) @@ -208,30 +233,11 @@ func TestBrainTunnelRejectsMissing(t *testing.T) { require.NotNil(t, resp["error"]) } -func TestBrainWriteRejectsTraversal(t *testing.T) { - brainDir := t.TempDir() - srv := mcp.NewServer(brainDir, nil, nil, nil) - - resp := toolCall(t, srv, "brain_write", map[string]any{ - "content": "x", - "filename": "../escape.md", - }) - require.NotNil(t, resp["error"]) -} - -func TestBrainWriteAcceptsDoubleDotInName(t *testing.T) { - brainDir := t.TempDir() - srv := mcp.NewServer(brainDir, nil, nil, nil) - - resp := toolCall(t, srv, "brain_write", map[string]any{ - "content": "x", - "filename": "notes..draft.md", - }) - require.Nil(t, resp["error"]) - - _, err := os.Stat(filepath.Join(brainDir, "knowledge", "notes..draft.md")) - require.NoError(t, err, "filename with embedded .. should be allowed") -} +// Filename traversal/sanitisation on the wiki route is covered directly +// against brain.NotePath/Sanitise in internal/brain/path_test.go +// (TestNotePath_SanitisesWingAndSlug, TestSanitise) — those tests exercised +// the now-deleted brain/knowledge/ legacy path's own separate filename +// validation, which had no equivalent once that path was removed (#79). func TestBrainIngestRawDryRun(t *testing.T) { brainDir := t.TempDir() diff --git a/internal/skills/brain/handlers.go b/internal/skills/brain/handlers.go deleted file mode 100644 index 063916c..0000000 --- a/internal/skills/brain/handlers.go +++ /dev/null @@ -1,182 +0,0 @@ -// internal/skills/brain/handlers.go -package brain - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "io" - "net/http" -) - -// Handle dispatches brain tool calls. -func (s *Skill) Handle(ctx context.Context, tool string, args json.RawMessage) (json.RawMessage, error) { - switch tool { - case "brain_query": - return s.query(ctx, args) - case "brain_write": - return s.write(ctx, args) - case "brain_ingest_raw": - return s.ingestRaw(ctx, args) - case "brain_ingest": - return s.ingest(ctx, args) - case "brain_search": - return s.search(ctx, args) - default: - return nil, fmt.Errorf("unknown brain tool: %s", tool) - } -} - -type queryArgs struct { - Query string `json:"query"` - Limit int `json:"limit,omitempty"` -} - -func (s *Skill) query(ctx context.Context, args json.RawMessage) (json.RawMessage, error) { - var a queryArgs - if err := json.Unmarshal(args, &a); err != nil { - return nil, fmt.Errorf("parse args: %w", err) - } - if a.Query == "" { - return nil, fmt.Errorf("query is required") - } - if a.Limit == 0 { - a.Limit = 5 - } - return s.post(ctx, "/query", a) -} - -type writeArgs struct { - Content string `json:"content"` - Type string `json:"type,omitempty"` - Domain string `json:"domain,omitempty"` - Filename string `json:"filename,omitempty"` -} - -func (s *Skill) write(ctx context.Context, args json.RawMessage) (json.RawMessage, error) { - var a writeArgs - if err := json.Unmarshal(args, &a); err != nil { - return nil, fmt.Errorf("parse args: %w", err) - } - if a.Content == "" { - return nil, fmt.Errorf("content is required") - } - return s.post(ctx, "/write", a) -} - -type ingestArgs struct { - Content string `json:"content,omitempty"` - Source string `json:"source,omitempty"` - Path string `json:"path,omitempty"` - DryRun bool `json:"dry_run,omitempty"` -} - -func (s *Skill) ingest(ctx context.Context, args json.RawMessage) (json.RawMessage, error) { - var a ingestArgs - if err := json.Unmarshal(args, &a); err != nil { - return nil, fmt.Errorf("parse args: %w", err) - } - if s.cfg.IngestSvcURL == "" { - return nil, fmt.Errorf("brain_ingest: INGEST_SVC_URL not configured") - } - if a.Path != "" && a.Content != "" { - return nil, fmt.Errorf("path and content+source are mutually exclusive: provide one or the other") - } - if a.Path != "" { - return s.postTo(ctx, s.cfg.IngestSvcURL+"/ingest-path", map[string]any{ - "path": a.Path, - "source": a.Source, - "dry_run": a.DryRun, - }) - } - if a.Content != "" && a.Source != "" { - return s.postTo(ctx, s.cfg.IngestSvcURL+"/ingest", map[string]any{ - "content": a.Content, - "source": a.Source, - "dry_run": a.DryRun, - }) - } - return nil, fmt.Errorf("either content+source or path is required") -} - -type ingestRawArgs struct { - Source string `json:"source"` - Pages []any `json:"pages"` - DryRun bool `json:"dry_run,omitempty"` -} - -func (s *Skill) ingestRaw(ctx context.Context, args json.RawMessage) (json.RawMessage, error) { - var a ingestRawArgs - if err := json.Unmarshal(args, &a); err != nil { - return nil, fmt.Errorf("parse args: %w", err) - } - if s.cfg.IngestSvcURL == "" { - return nil, fmt.Errorf("brain_ingest_raw: INGEST_SVC_URL not configured") - } - if a.Source == "" { - return nil, fmt.Errorf("source is required") - } - if len(a.Pages) == 0 { - return nil, fmt.Errorf("pages is required and must be non-empty") - } - return s.postTo(ctx, s.cfg.IngestSvcURL+"/ingest-raw", map[string]any{ - "source": a.Source, - "pages": a.Pages, - "dry_run": a.DryRun, - }) -} - -type searchArgs struct { - Query string `json:"query"` - Collection string `json:"collection,omitempty"` - Limit int `json:"limit,omitempty"` -} - -func (s *Skill) search(ctx context.Context, args json.RawMessage) (json.RawMessage, error) { - var a searchArgs - if err := json.Unmarshal(args, &a); err != nil { - return nil, fmt.Errorf("parse args: %w", err) - } - if a.Query == "" { - return nil, fmt.Errorf("query is required") - } - if a.Limit == 0 { - a.Limit = 5 - } - if s.cfg.KBRetrievalURL == "" { - return nil, fmt.Errorf("brain_search: KB_RETRIEVAL_URL not configured") - } - return s.postTo(ctx, s.cfg.KBRetrievalURL+"/api/v1/search", a) -} - -func (s *Skill) post(ctx context.Context, path string, body any) (json.RawMessage, error) { - return s.postTo(ctx, s.cfg.IngestBaseURL+path, body) -} - -func (s *Skill) postTo(ctx context.Context, url string, body any) (json.RawMessage, error) { - b, err := json.Marshal(body) - if err != nil { - return nil, fmt.Errorf("marshal request: %w", err) - } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(b)) - if err != nil { - return nil, fmt.Errorf("build request: %w", err) - } - req.Header.Set("Content-Type", "application/json") - - resp, err := http.DefaultClient.Do(req) - if err != nil { - return nil, fmt.Errorf("call ingestion server: %w", err) - } - defer func() { _ = resp.Body.Close() }() - - out, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("read response: %w", err) - } - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("ingestion server returned %d: %s", resp.StatusCode, out) - } - return json.RawMessage(out), nil -} diff --git a/internal/skills/brain/handlers_test.go b/internal/skills/brain/handlers_test.go deleted file mode 100644 index bb783e2..0000000 --- a/internal/skills/brain/handlers_test.go +++ /dev/null @@ -1,122 +0,0 @@ -// internal/skills/brain/handlers_test.go -package brain_test - -import ( - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "testing" - - "git.d-ma.be/mathias/hyperguild/internal/skills/brain" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestHandle_BrainQuery_CallsIngestServer(t *testing.T) { - called := false - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/query", r.URL.Path) - called = true - _ = json.NewEncoder(w).Encode(map[string]any{ - "results": []map[string]any{ - {"path": "wiki/concepts/tdd.md", "title": "TDD", "excerpt": "Test-driven development.", "score": 3}, - }, - }) - })) - defer srv.Close() - - s := brain.New(brain.Config{IngestBaseURL: srv.URL}) - args, _ := json.Marshal(map[string]string{"query": "test driven development"}) - out, err := s.Handle(context.Background(), "brain_query", args) - require.NoError(t, err) - assert.True(t, called) - - var result map[string]any - require.NoError(t, json.Unmarshal(out, &result)) - results := result["results"].([]any) - assert.Len(t, results, 1) -} - -func TestHandle_BrainWrite_CallsIngestServer(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/write", r.URL.Path) - var body map[string]string - require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) - assert.Equal(t, "concept", body["type"]) - assert.Equal(t, "# Test\n\nSome learning.", body["content"]) - _ = json.NewEncoder(w).Encode(map[string]string{"path": "raw/test.md"}) - })) - defer srv.Close() - - s := brain.New(brain.Config{IngestBaseURL: srv.URL}) - args, _ := json.Marshal(map[string]string{"content": "# Test\n\nSome learning.", "type": "concept"}) - out, err := s.Handle(context.Background(), "brain_write", args) - require.NoError(t, err) - var result map[string]string - require.NoError(t, json.Unmarshal(out, &result)) - assert.Equal(t, "raw/test.md", result["path"]) -} - -func TestHandle_UnknownTool_ReturnsError(t *testing.T) { - s := brain.New(brain.Config{IngestBaseURL: "http://localhost:3300"}) - _, err := s.Handle(context.Background(), "brain_unknown", nil) - assert.Error(t, err) -} - -func TestIngest_RoutesToIngestPath(t *testing.T) { - var capturedPath string - var capturedBody map[string]any - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - capturedPath = r.URL.Path - require.NoError(t, json.NewDecoder(r.Body).Decode(&capturedBody)) - _ = json.NewEncoder(w).Encode(map[string]any{"pages": []string{"wiki/foo.md"}}) - })) - defer srv.Close() - - s := brain.New(brain.Config{IngestSvcURL: srv.URL}) - args, _ := json.Marshal(map[string]any{"path": "/tmp/some-file.md"}) - out, err := s.Handle(context.Background(), "brain_ingest", args) - require.NoError(t, err) - - assert.Equal(t, "/ingest-path", capturedPath) - assert.Equal(t, "/tmp/some-file.md", capturedBody["path"]) - - var result map[string]any - require.NoError(t, json.Unmarshal(out, &result)) - pages := result["pages"].([]any) - assert.Len(t, pages, 1) -} - -func TestIngest_RoutesToIngest(t *testing.T) { - var capturedPath string - var capturedBody map[string]any - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - capturedPath = r.URL.Path - require.NoError(t, json.NewDecoder(r.Body).Decode(&capturedBody)) - _ = json.NewEncoder(w).Encode(map[string]any{"pages": []string{"wiki/bar.md"}}) - })) - defer srv.Close() - - s := brain.New(brain.Config{IngestSvcURL: srv.URL}) - args, _ := json.Marshal(map[string]any{"content": "some content", "source": "my-source.md"}) - out, err := s.Handle(context.Background(), "brain_ingest", args) - require.NoError(t, err) - - assert.Equal(t, "/ingest", capturedPath) - assert.Equal(t, "some content", capturedBody["content"]) - assert.Equal(t, "my-source.md", capturedBody["source"]) - - var result map[string]any - require.NoError(t, json.Unmarshal(out, &result)) - pages := result["pages"].([]any) - assert.Len(t, pages, 1) -} - -func TestIngest_MissingRequiredFields(t *testing.T) { - s := brain.New(brain.Config{IngestSvcURL: "http://localhost:3300"}) - args, _ := json.Marshal(map[string]any{}) - _, err := s.Handle(context.Background(), "brain_ingest", args) - require.Error(t, err) - assert.Contains(t, err.Error(), "either content+source or path is required") -} diff --git a/internal/skills/brain/skill.go b/internal/skills/brain/skill.go deleted file mode 100644 index 12f95c9..0000000 --- a/internal/skills/brain/skill.go +++ /dev/null @@ -1,111 +0,0 @@ -// internal/skills/brain/skill.go -package brain - -import ( - "encoding/json" - - "git.d-ma.be/mathias/hyperguild/internal/registry" -) - -// Config holds brain skill configuration. -type Config struct { - IngestBaseURL string // base URL of the ingestion HTTP server (brain_query, brain_write) - IngestSvcURL string // base URL of the ingestion-svc HTTP server (brain_ingest) - KBRetrievalURL string // base URL of the kb-retrieval server (brain_search) -} - -// Skill implements registry.Skill for brain_query and brain_write. -type Skill struct { - cfg Config -} - -// New constructs a brain Skill. -func New(cfg Config) *Skill { return &Skill{cfg: cfg} } - -// Name returns the skill name used for routing. -func (s *Skill) Name() string { return "brain" } - -// Tools returns the MCP tool definitions for brain_query and brain_write. -func (s *Skill) Tools() []registry.ToolDef { - schema := func(required []string, props map[string]any) json.RawMessage { - b, _ := json.Marshal(map[string]any{"type": "object", "required": required, "properties": props}) - return b - } - str := map[string]any{"type": "string"} - num := map[string]any{"type": "integer"} - - tools := []registry.ToolDef{ - { - Name: "brain_query", - Description: "BM25 full-text search across brain/knowledge/ and brain/wiki/ markdown files. Fast, no embeddings needed. Call before any significant task.", - InputSchema: schema([]string{"query"}, map[string]any{ - "query": str, - "limit": num, - }), - }, - { - Name: "brain_write", - Description: "Write a raw knowledge note to brain/knowledge/ for later ingestion.", - InputSchema: schema([]string{"content"}, map[string]any{ - "content": str, - "type": str, - "domain": str, - "filename": str, - }), - }, - } - if s.cfg.IngestSvcURL != "" { - tools = append(tools, registry.ToolDef{ - Name: "brain_ingest_raw", - Description: "Ingest pre-structured pages into the brain wiki, bypassing the LLM extraction step. " + - "Use when you (the calling agent) have already extracted entities, concepts, and content from a source. " + - "Provide source (human-readable name) and pages (array of {title, type, subtype, domain, content} objects). " + - "The pipeline computes slugs, paths, frontmatter, wikilink canonicalization, and source back-references. " + - "Returns the list of wiki pages written.", - InputSchema: schema([]string{"source", "pages"}, map[string]any{ - "source": map[string]any{"type": "string", "description": "human-readable name for the source, e.g. 'shape-up-book'"}, - "pages": map[string]any{ - "type": "array", - "items": map[string]any{ - "type": "object", - "required": []string{"title", "type", "content"}, - "properties": map[string]any{ - "title": map[string]any{"type": "string", "description": "page title, e.g. 'Hash Encoding'"}, - "type": map[string]any{"type": "string", "enum": []string{"source", "concept", "entity"}, "description": "page type"}, - "subtype": map[string]any{"type": "string", "description": "entity: person|company|tool|model|framework|technology; source: article|pdf|book|video|note|project"}, - "domain": map[string]any{"type": "string", "description": "knowledge domain, e.g. 'Machine Learning'"}, - "content": map[string]any{"type": "string", "description": "markdown body — no frontmatter, use [[Display Name]] for wikilinks"}, - }, - }, - }, - "dry_run": map[string]any{"type": "boolean"}, - }), - }) - tools = append(tools, registry.ToolDef{ - Name: "brain_ingest", - Description: "Ingest content into the brain wiki (brain/wiki/). Calls an LLM to produce structured wiki pages. " + - "Use for substantial documents, articles, or knowledge worth structuring. " + - "Provide EITHER (a) path — absolute path to a file or directory, " + - "OR (b) content + source — raw text and a human-readable name. " + - "Providing both is an error. Returns the list of wiki pages written.", - InputSchema: schema([]string{}, map[string]any{ - "content": map[string]any{"type": "string", "description": "raw text to ingest; required when path is not set"}, - "source": map[string]any{"type": "string", "description": "human-readable name for the content, e.g. 'shape-up-book'; required when path is not set"}, - "path": map[string]any{"type": "string", "description": "absolute path to a file or directory to ingest; mutually exclusive with content+source"}, - "dry_run": map[string]any{"type": "boolean"}, - }), - }) - } - if s.cfg.KBRetrievalURL != "" { - tools = append(tools, registry.ToolDef{ - Name: "brain_search", - Description: "Semantic vector search across the brain wiki using embeddings. Use when brain_query returns no results or you need conceptually-related results rather than keyword matches.", - InputSchema: schema([]string{"query"}, map[string]any{ - "query": str, - "collection": str, - "limit": num, - }), - }) - } - return tools -}