diff --git a/.agents/decisions.yaml b/.agents/decisions.yaml index fa3c212..9f77255 100644 --- a/.agents/decisions.yaml +++ b/.agents/decisions.yaml @@ -48,3 +48,17 @@ entries: list_knowledge stays file-backed for projects and index-backed for orgs; with auto-refresh both are current. source: 'issue #36 design discussion with user, 2026-10-23' date: "2026-10-23" + - id: dec-006 + summary: 'Org writes: markdown sections through write/update with a file argument' + detail: |- + Issue #40 part 2: org roots (KindOrg) accepted queries but rejected writes, so org knowledge was hand-maintained and drifted. + Chosen shape (user-approved): write_knowledge gains an optional file argument; on an org root summary becomes the ## heading and detail the section body, appended to /.agents/knowledge/. + file must be a bare .md filename: no separators, no traversal, no '::' (the id delimiter), non-empty stem. A missing file name returns the available files; a valid new name creates the file. No guessing. + source is folded into the section as a final 'Source: ...' line. update_knowledge on org parses id org-:: (the id shape query_knowledge already returns), applies only detail, keeps the heading immutable, and carries the old Source line forward. + Decisions from the issue's open questions: rule is rejected for org sections (fold constraints into detail); heading collisions are rejected with the matching update id instead of merged or duplicated; summary on org update is rejected because headings are part of the id. + category is now schema-optional and required only for project stores; org sections all index as conventions. The handler resolves the ref before validating category so org writes need not pass it. + Writes are atomic via knowledge.SaveText (temp + fsync + rename) under the existing per-path fileLocks. No idx.Add is needed: the GH-36 store fingerprint notices the new mtime and Query rebuilds. + Part 1 of the issue (only architecture.md/review.md indexed) was already fixed by GH-36/PR #39; this change only hardened OrgKnowledgeFiles to require regular files. + Tests: knowledge/org_test.go (append/replace/body/save/regular-file filtering), tools/org_test.go (write+query+list, missing file, no files, rule rejection, filename validation, collision, project category requirement, update semantics, invalid ids). + source: 'GitHub issue #40 implementation, 2026-10-26' + date: "2026-10-26" diff --git a/.agents/subsystems.yaml b/.agents/subsystems.yaml index ec6ccf4..d261710 100644 --- a/.agents/subsystems.yaml +++ b/.agents/subsystems.yaml @@ -27,14 +27,17 @@ entries: - id: sub-003 summary: 'Knowledge store: atomic writes, per-file locks, entry lifecycle' detail: |- - knowledge/store.go: Save writes temp file, fsyncs, renames (atomic); LoadOrCreate returns empty KnowledgeFile when missing. + knowledge/store.go: Save writes YAML via temp file, fsync, rename (atomic); LoadOrCreate returns an empty KnowledgeFile when missing; SaveText does the same for org markdown. tools package keeps per-path RWMutexes (fileLocks) around read-modify-write cycles; write/update lock, list takes RLock. NextID scans existing IDs for the category prefix and returns the next sequential ID. - write_knowledge validates category, summary <=100 chars, detail <=1MB, rule <=200 chars; org roots reject writes (file-based knowledge only). - update_knowledge validates supersedes references an existing ID in the same category and rejects no-op updates. + write_knowledge validates summary <=100 chars, detail <=1MB, rule <=200 chars. category is required for project stores and ignored on org roots. + Org root write: file is required (bare *.md, no directories; missing file lists available names; a new name creates the file), summary is the ## heading, detail the body, source becomes a trailing Source: line, rule is rejected, and a heading collision points at update_knowledge. knowledge.AppendOrgSection/SaveText run under fileLocks. + update_knowledge on an org root takes id org-::; only detail applies, the heading is immutable, the trailing Source: line is preserved, and the file must already exist. knowledge.ReplaceOrgSection rewrites just that section. + OrgKnowledgeFiles returns sorted top-level *.md names and skips directories, broken symlinks, and other non-regular files (symlinks to regular files are followed). + update_knowledge validates supersedes references an existing ID in the same category and rejects no-op updates for project stores. Rules surface as a constraints section in list_knowledge output, above regular entries. source: code review, 2026-09-08 - date: "2026-09-08" + date: "2026-10-26" - id: sub-004 summary: 'Startup indexing: always-on background kick, no skip flag' detail: |- diff --git a/README.md b/README.md index ed1c56a..f2e5d46 100644 --- a/README.md +++ b/README.md @@ -221,10 +221,10 @@ arguments — entry bodies are replaced by sizes and never written. Set | Tool | Description | |------|-------------| | `init_knowledge` | Create a project's knowledge directory structure (central store under `--store`) | -| `write_knowledge` | Add a new knowledge entry | +| `write_knowledge` | Add a knowledge entry, or an org markdown section with `file` | | `query_knowledge` | Full-text search with category filters | | `list_knowledge` | List all entries; entries with rules shown first as constraints | -| `update_knowledge` | Update an existing entry by ID | +| `update_knowledge` | Update an entry by ID, or an org section body | | `list_projects` | List all discovered projects (org-wide mode only) | | `reindex_knowledge` | Rebuild the search index from the store files (queries also refresh automatically) | @@ -245,6 +245,23 @@ list parses as zero entries. `list_knowledge` marks entries merged from a shared global store with a `(global)` suffix so they are never mistaken for the project's own entries. +### Org Markdown Files + +Org roots keep knowledge as markdown in `/.agents/knowledge/*.md`, +indexed per `##` section. + +- `write_knowledge` on an org root requires `file` (for example + `dsp-verification.md`). `summary` becomes the `##` heading, `detail` the + section body, and `source` is folded in as a final `Source:` line. Omit + `file` to get the available files; a bare new `*.md` name creates it. +- `update_knowledge` on an org root takes the id + `org-::` and only applies `detail`. The heading is preserved + and the trailing `Source:` line is carried over. +- A heading collision is rejected with the matching `update_knowledge` id + rather than merged. `rule` and `supersedes` do not apply to sections. +- Writes are atomic (temp file + fsync + rename) and take the same per-file + lock as YAML stores. Queries see them through the index fingerprint. + ### Rules / Constraints Entries can include an optional `rule` field — an imperative statement of what's forbidden or required (e.g. `"Never create files outside ./tmp"`). `list_knowledge` surfaces entries with rules in a separate `## constraints` section above regular entries, so agents see hard rules immediately without needing to query detail. diff --git a/examples/KNOWLEDGE.md b/examples/KNOWLEDGE.md index b9f1439..15633b5 100644 --- a/examples/KNOWLEDGE.md +++ b/examples/KNOWLEDGE.md @@ -16,3 +16,5 @@ Write it back: - `write_knowledge(project="", category="subsystems", summary="...", detail="...", source="...")` - `write_knowledge(project="", category="decisions", summary="...", detail="...", source="...")` - `write_knowledge(project="", category="workflows", summary="...", detail="...", source="...")` +- Org root: `write_knowledge(project="", file="topic.md", summary="", detail="...", source="...")` + (`update_knowledge` uses id `org-::`; org sections take detail only.) diff --git a/install/skill.go b/install/skill.go index cfd449e..f46f8db 100644 --- a/install/skill.go +++ b/install/skill.go @@ -37,6 +37,7 @@ func RenderSkillMD(registry []tools.ToolSpec) []byte { "## Capture workflow\n" + "\n" + "- write_knowledge when a convention, subsystem fact, or decision is established.\n" + + "- On an org root, pass file=.md: summary is the section heading and detail the body.\n" + "- detail: 10-20 lines of query-able facts; never copy source files.\n" + "- source: record how the knowledge was learned.\n" + "- rule field: an imperative prohibition/requirement (surfaced first in lists).\n" + diff --git a/knowledge/org.go b/knowledge/org.go new file mode 100644 index 0000000..4381259 --- /dev/null +++ b/knowledge/org.go @@ -0,0 +1,148 @@ +package knowledge + +import ( + "errors" + "fmt" + "os" + "strings" +) + +// Org section errors. Append requires a fresh heading; replace requires an +// existing one; duplicate headings are ambiguous and rejected by both. +var ( + ErrOrgSectionExists = errors.New("section already exists") + ErrOrgSectionNotFound = errors.New("section not found") + ErrOrgDuplicateSection = errors.New("duplicate section heading") +) + +// OrgSectionBody returns the body of the "## heading" section in data, +// excluding the heading line. It returns ErrOrgSectionNotFound when the +// heading is absent and ErrOrgDuplicateSection when it appears twice. +func OrgSectionBody(data, heading string) (string, error) { + lines := splitOrgLines(data) + idx := findOrgSections(lines, heading) + if len(idx) == 0 { + return "", ErrOrgSectionNotFound + } + if len(idx) > 1 { + return "", ErrOrgDuplicateSection + } + end := orgSectionEnd(lines, idx[0]) + return strings.TrimSpace(strings.Join(lines[idx[0]+1:end], "\n")), nil +} + +// AppendOrgSection appends a "## heading" section to data. It refuses to +// guess when the heading already exists. +func AppendOrgSection(data, heading, body string) (string, error) { + idx := findOrgSections(splitOrgLines(data), heading) + if len(idx) > 1 { + return "", ErrOrgDuplicateSection + } + if len(idx) == 1 { + return "", ErrOrgSectionExists + } + section := renderOrgSection(heading, body) + if strings.TrimSpace(data) == "" { + return section, nil + } + return strings.TrimRight(data, "\n") + "\n\n" + section, nil +} + +// ReplaceOrgSection replaces the body of an existing "## heading" section, +// preserving the heading line and all content outside the section. +func ReplaceOrgSection(data, heading, body string) (string, error) { + lines := splitOrgLines(data) + idx := findOrgSections(lines, heading) + if len(idx) == 0 { + return "", ErrOrgSectionNotFound + } + if len(idx) > 1 { + return "", ErrOrgDuplicateSection + } + + start := idx[0] + end := orgSectionEnd(lines, start) + + var b strings.Builder + if prefix := strings.Join(lines[:start], "\n"); prefix != "" { + b.WriteString(strings.TrimRight(prefix, "\n")) + b.WriteString("\n\n") + } + b.WriteString(renderOrgSection(heading, body)) + if rest := lines[end:]; len(rest) > 0 { + b.WriteString("\n") + b.WriteString(strings.Join(rest, "\n")) + b.WriteString("\n") + } + return b.String(), nil +} + +// SaveText writes a text file atomically (temp file + fsync + rename), the +// same durability model as Save for YAML stores. +func SaveText(filePath, data string) error { + tmpPath := filePath + ".tmp" + f, err := os.Create(tmpPath) + if err != nil { + return fmt.Errorf("create temp file: %w", err) + } + + if _, err := f.WriteString(data); err != nil { + f.Close() + os.Remove(tmpPath) + return fmt.Errorf("write temp file: %w", err) + } + + if err := f.Sync(); err != nil { + f.Close() + os.Remove(tmpPath) + return fmt.Errorf("sync temp file: %w", err) + } + + if err := f.Close(); err != nil { + os.Remove(tmpPath) + return fmt.Errorf("close temp file: %w", err) + } + + if err := os.Rename(tmpPath, filePath); err != nil { + os.Remove(tmpPath) + return fmt.Errorf("rename temp file: %w", err) + } + + return nil +} + +func splitOrgLines(data string) []string { + lines := strings.Split(data, "\n") + if len(lines) > 0 && lines[len(lines)-1] == "" { + lines = lines[:len(lines)-1] + } + return lines +} + +func findOrgSections(lines []string, heading string) []int { + var idx []int + for i, line := range lines { + rest, ok := strings.CutPrefix(line, "## ") + if ok && strings.TrimSpace(rest) == heading { + idx = append(idx, i) + } + } + return idx +} + +func orgSectionEnd(lines []string, start int) int { + for i := start + 1; i < len(lines); i++ { + if strings.HasPrefix(lines[i], "## ") { + return i + } + } + return len(lines) +} + +func renderOrgSection(heading, body string) string { + body = strings.TrimSpace(body) + if body == "" { + return "## " + heading + "\n" + } + return "## " + heading + "\n\n" + body + "\n" +} diff --git a/knowledge/org_test.go b/knowledge/org_test.go new file mode 100644 index 0000000..a4c7ec6 --- /dev/null +++ b/knowledge/org_test.go @@ -0,0 +1,172 @@ +package knowledge + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestAppendOrgSection(t *testing.T) { + data := "# Title\n\nintro\n" + got, err := AppendOrgSection(data, "Cost", "cycles block the deadline") + if err != nil { + t.Fatalf("AppendOrgSection() error: %v", err) + } + if !strings.Contains(got, "# Title\n\nintro\n\n## Cost\n\ncycles block the deadline\n") { + t.Errorf("AppendOrgSection() = %q", got) + } +} + +func TestAppendOrgSectionEmptyFile(t *testing.T) { + got, err := AppendOrgSection("", "Cost", "cycles block the deadline") + if err != nil { + t.Fatalf("AppendOrgSection() error: %v", err) + } + want := "## Cost\n\ncycles block the deadline\n" + if got != want { + t.Errorf("AppendOrgSection() = %q, want %q", got, want) + } +} + +func TestAppendOrgSectionExisting(t *testing.T) { + _, err := AppendOrgSection("## Cost\nold\n", "Cost", "new") + if !errors.Is(err, ErrOrgSectionExists) { + t.Fatalf("AppendOrgSection() error = %v, want ErrOrgSectionExists", err) + } +} + +func TestAppendOrgSectionDuplicateHeading(t *testing.T) { + _, err := AppendOrgSection("## Cost\none\n\n## Cost\ntwo\n", "Cost", "new") + if !errors.Is(err, ErrOrgDuplicateSection) { + t.Fatalf("AppendOrgSection() error = %v, want ErrOrgDuplicateSection", err) + } +} + +func TestOrgSectionBody(t *testing.T) { + data := "# Title\n\nintro\n\n## First\none\n\n### Sub\nsub body\n\n## Second\ntwo\n" + + body, err := OrgSectionBody(data, "First") + if err != nil { + t.Fatalf("OrgSectionBody() error: %v", err) + } + if body != "one\n\n### Sub\nsub body" { + t.Errorf("body = %q", body) + } + + body, err = OrgSectionBody(data, "Second") + if err != nil { + t.Fatalf("OrgSectionBody() error: %v", err) + } + if body != "two" { + t.Errorf("body = %q", body) + } + + if _, err := OrgSectionBody(data, "Missing"); !errors.Is(err, ErrOrgSectionNotFound) { + t.Fatalf("OrgSectionBody() error = %v, want ErrOrgSectionNotFound", err) + } +} + +func TestReplaceOrgSection(t *testing.T) { + data := "# Title\n\nintro\n\n## First\none\n\n### Sub\nsub body\n\n## Second\ntwo\n" + + got, err := ReplaceOrgSection(data, "First", "new body\nline two") + if err != nil { + t.Fatalf("ReplaceOrgSection() error: %v", err) + } + if strings.Contains(got, "one") || strings.Contains(got, "sub body") { + t.Errorf("old section body survived: %q", got) + } + for _, want := range []string{"# Title", "intro", "## First", "new body\nline two", "## Second", "two"} { + if !strings.Contains(got, want) { + t.Errorf("ReplaceOrgSection() lost %q: %q", want, got) + } + } +} + +func TestReplaceOrgSectionLast(t *testing.T) { + got, err := ReplaceOrgSection("## First\none\n\n## Second\ntwo\n", "Second", "three") + if err != nil { + t.Fatalf("ReplaceOrgSection() error: %v", err) + } + want := "## First\none\n\n## Second\n\nthree\n" + if got != want { + t.Errorf("ReplaceOrgSection() = %q, want %q", got, want) + } +} + +func TestReplaceOrgSectionNotFound(t *testing.T) { + _, err := ReplaceOrgSection("## First\none\n", "Missing", "new") + if !errors.Is(err, ErrOrgSectionNotFound) { + t.Fatalf("ReplaceOrgSection() error = %v, want ErrOrgSectionNotFound", err) + } +} + +func TestReplaceOrgSectionDuplicateHeading(t *testing.T) { + _, err := ReplaceOrgSection("## Cost\none\n\n## Cost\ntwo\n", "Cost", "new") + if !errors.Is(err, ErrOrgDuplicateSection) { + t.Fatalf("ReplaceOrgSection() error = %v, want ErrOrgDuplicateSection", err) + } +} + +func TestSaveText(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "notes.md") + + if err := SaveText(path, "## Heading\n\nbody\n"); err != nil { + t.Fatalf("SaveText() error: %v", err) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile() error: %v", err) + } + if string(data) != "## Heading\n\nbody\n" { + t.Errorf("content = %q", data) + } + + leftovers, err := filepath.Glob(filepath.Join(dir, "*.tmp")) + if err != nil { + t.Fatalf("Glob() error: %v", err) + } + if len(leftovers) != 0 { + t.Errorf("temporary files left behind: %v", leftovers) + } +} + +func TestOrgKnowledgeFilesRegularOnly(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "a.md"), []byte("## A\n"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("ignored"), 0644); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(dir, "sub.md"), 0755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(dir, "a.md"), filepath.Join(dir, "link.md")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + if err := os.Symlink(filepath.Join(dir, "missing.md"), filepath.Join(dir, "broken.md")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(dir, filepath.Join(dir, "dirlink.md")); err != nil { + t.Fatal(err) + } + + files, err := OrgKnowledgeFiles(dir) + if err != nil { + t.Fatalf("OrgKnowledgeFiles() error: %v", err) + } + want := []string{"a.md", "link.md"} + if len(files) != len(want) { + t.Fatalf("OrgKnowledgeFiles() = %v, want %v", files, want) + } + for i := range want { + if files[i] != want[i] { + t.Fatalf("OrgKnowledgeFiles() = %v, want %v", files, want) + } + } +} diff --git a/knowledge/store.go b/knowledge/store.go index 61290f0..c4156e4 100644 --- a/knowledge/store.go +++ b/knowledge/store.go @@ -131,6 +131,12 @@ func OrgKnowledgeFiles(dir string) ([]string, error) { if entry.IsDir() || !strings.EqualFold(filepath.Ext(entry.Name()), ".md") { continue } + // Follow symlinks but index only regular files: broken links, + // directories and special files are skipped. + info, err := os.Stat(filepath.Join(dir, entry.Name())) + if err != nil || !info.Mode().IsRegular() { + continue + } files = append(files, entry.Name()) } sort.Strings(files) diff --git a/tools/org.go b/tools/org.go new file mode 100644 index 0000000..e5130c4 --- /dev/null +++ b/tools/org.go @@ -0,0 +1,212 @@ +package tools + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/renderorange/knowledge-mcp/knowledge" + "github.com/renderorange/knowledge-mcp/projects" +) + +// writeOrgSection appends a markdown section to an org knowledge file. It +// never guesses the file: a missing name returns the available files. +func writeOrgSection(res *projects.Resolver, ref projects.Ref, project, file, summary, detail, source, rule string) *mcp.CallToolResult { + heading := strings.TrimSpace(summary) + if heading == "" { + return mcp.NewToolResultError("summary must not be blank for org sections") + } + if rule != "" { + return mcp.NewToolResultError("rule is not supported for org sections; include the constraint in detail") + } + + dir := res.OrgKnowledgeDir(ref) + if file == "" { + available, err := knowledge.OrgKnowledgeFiles(dir) + if err != nil || len(available) == 0 { + return mcp.NewToolResultError(fmt.Sprintf( + "%q is an org root; file is required and there are no markdown files in %s (pass file=.md to create one)", + project, dir)) + } + return mcp.NewToolResultError(fmt.Sprintf( + "%q is an org root; file is required (available: %s)", project, strings.Join(available, ", "))) + } + if err := validateOrgFile(file); err != nil { + return mcp.NewToolResultError(err.Error()) + } + + path := filepath.Join(dir, file) + mu := fileLocks.Get(path) + mu.Lock() + defer mu.Unlock() + + existing, readErr := os.ReadFile(path) + if readErr != nil && !os.IsNotExist(readErr) { + return mcp.NewToolResultError(fmt.Sprintf("read org knowledge file: %v", readErr)) + } + if readErr == nil { + _, lookErr := knowledge.OrgSectionBody(string(existing), heading) + switch { + case lookErr == nil: + return mcp.NewToolResultError(fmt.Sprintf( + "section %q already exists in %s; use update_knowledge with id %q", + heading, path, orgSectionID(file, heading))) + case errors.Is(lookErr, knowledge.ErrOrgDuplicateSection): + return mcp.NewToolResultError(fmt.Sprintf( + "section %q appears more than once in %s; fix the file before writing", heading, path)) + } + } + + if err := knowledge.EnsureDir(dir); err != nil { + return mcp.NewToolResultError(fmt.Sprintf("create org knowledge directory: %v", err)) + } + data, err := knowledge.AppendOrgSection(string(existing), heading, orgSectionBody(detail, source)) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("append org section: %v", err)) + } + if err := knowledge.SaveText(path, data); err != nil { + return mcp.NewToolResultError(fmt.Sprintf("save org knowledge file: %v", err)) + } + return mcp.NewToolResultText(fmt.Sprintf("wrote section %q to %s", heading, path)) +} + +// updateOrgSection replaces the body of an org markdown section. The id is +// org-::; headings are immutable because they are part of the +// id, and provenance lines are carried over from the old body. +func updateOrgSection(res *projects.Resolver, ref projects.Ref, id, summary, detail, rule, supersedes string) *mcp.CallToolResult { + if summary != "" { + return mcp.NewToolResultError("summary is not supported for org sections; headings are immutable") + } + if rule != "" { + return mcp.NewToolResultError("rule is not supported for org sections; include the constraint in detail") + } + if supersedes != "" { + return mcp.NewToolResultError("supersedes is not supported for org sections") + } + if detail == "" { + return mcp.NewToolResultError("detail is required for org section updates") + } + if len(detail) > maxDetailSize { + return mcp.NewToolResultError(fmt.Sprintf("detail too long: %d bytes (max %d)", len(detail), maxDetailSize)) + } + + file, heading, ok := parseOrgSectionID(id) + if !ok { + return mcp.NewToolResultError(fmt.Sprintf("invalid org section id %q: want org-::", id)) + } + if err := validateOrgFile(file); err != nil { + return mcp.NewToolResultError(err.Error()) + } + + dir := res.OrgKnowledgeDir(ref) + available, err := knowledge.OrgKnowledgeFiles(dir) + if err != nil || !containsString(available, file) { + names := strings.Join(available, ", ") + if names == "" { + names = "none" + } + return mcp.NewToolResultError(fmt.Sprintf("org root has no knowledge file %q (available: %s)", file, names)) + } + + path := filepath.Join(dir, file) + mu := fileLocks.Get(path) + mu.Lock() + defer mu.Unlock() + + data, readErr := os.ReadFile(path) + if readErr != nil { + return mcp.NewToolResultError(fmt.Sprintf("read org knowledge file: %v", readErr)) + } + + body, bodyErr := knowledge.OrgSectionBody(string(data), heading) + if bodyErr != nil { + if errors.Is(bodyErr, knowledge.ErrOrgDuplicateSection) { + return mcp.NewToolResultError(fmt.Sprintf( + "section %q appears more than once in %s; fix the file before updating", heading, path)) + } + return mcp.NewToolResultError(fmt.Sprintf("section %q not found in %s", heading, path)) + } + + newBody := orgSectionBody(detail, "") + if src := trailingSource(body); src != "" && trailingSource(newBody) == "" { + newBody += "\n\n" + src + } + + updated, err := knowledge.ReplaceOrgSection(string(data), heading, newBody) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("replace org section: %v", err)) + } + if err := knowledge.SaveText(path, updated); err != nil { + return mcp.NewToolResultError(fmt.Sprintf("save org knowledge file: %v", err)) + } + return mcp.NewToolResultText(fmt.Sprintf("updated section %q in %s", heading, path)) +} + +// validateOrgFile accepts a bare markdown filename only. It rejects path +// separators, traversal, "::" (the id delimiter) and empty stems. +func validateOrgFile(name string) error { + if name == "" { + return errors.New("file is required") + } + if name != filepath.Base(name) || strings.ContainsAny(name, `/\`) { + return fmt.Errorf("invalid file %q: must be a bare .md filename (no directories)", name) + } + if strings.Contains(name, "::") { + return fmt.Errorf("invalid file %q: must not contain \"::\"", name) + } + ext := filepath.Ext(name) + if !strings.EqualFold(ext, ".md") || strings.TrimSpace(strings.TrimSuffix(name, ext)) == "" { + return fmt.Errorf("invalid file %q: must be a .md filename", name) + } + return nil +} + +func parseOrgSectionID(id string) (file, heading string, ok bool) { + rest, found := strings.CutPrefix(id, "org-") + if !found { + return "", "", false + } + file, heading, found = strings.Cut(rest, "::") + heading = strings.TrimSpace(heading) + if !found || file == "" || heading == "" { + return "", "", false + } + return file, heading, true +} + +func orgSectionID(file, heading string) string { + return "org-" + file + "::" + heading +} + +func orgSectionBody(detail, source string) string { + body := strings.TrimSpace(detail) + if source == "" { + return body + } + return body + "\n\nSource: " + strings.TrimSpace(source) +} + +// trailingSource returns a final "Source: ..." line from a section body. +func trailingSource(body string) string { + lines := strings.Split(strings.TrimRight(body, "\n"), "\n") + if len(lines) == 0 { + return "" + } + last := strings.TrimSpace(lines[len(lines)-1]) + if strings.HasPrefix(last, "Source: ") { + return last + } + return "" +} + +func containsString(values []string, want string) bool { + for _, value := range values { + if value == want { + return true + } + } + return false +} diff --git a/tools/org_test.go b/tools/org_test.go new file mode 100644 index 0000000..e03a75f --- /dev/null +++ b/tools/org_test.go @@ -0,0 +1,464 @@ +package tools + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/renderorange/knowledge-mcp/knowledge" + "github.com/renderorange/knowledge-mcp/projects" +) + +// newOrgFixture builds an org root with .agents/knowledge/ and the given +// markdown files, then resolves it as an org ref. +func newOrgFixture(t *testing.T, files map[string]string) (*projects.Resolver, string, string) { + t.Helper() + root := t.TempDir() + dir := filepath.Join(root, ".agents", "knowledge") + if err := knowledge.EnsureDir(dir); err != nil { + t.Fatalf("EnsureDir() error: %v", err) + } + for name, content := range files { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0644); err != nil { + t.Fatalf("WriteFile(%s) error: %v", name, err) + } + } + res, _, err := projects.Build([]string{root}, nil, "") + if err != nil { + t.Fatalf("projects.Build() error: %v", err) + } + return res, filepath.Base(root), dir +} + +func orgRefFor(t *testing.T, res *projects.Resolver) projects.Ref { + t.Helper() + for _, ref := range res.Snapshot() { + if ref.Kind == projects.KindOrg { + return ref + } + } + t.Fatal("no org ref in resolver") + return projects.Ref{} +} + +func TestWriteHandlerOrgSection(t *testing.T) { + res, org, dir := newOrgFixture(t, map[string]string{ + "architecture.md": "## Build\nmake test\n", + }) + + idx := newIndexReady(t) + defer idx.Close() + idx.IndexAll(res) + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "summary": "Cost cycles", + "detail": "DSP cost cycles block the deadline", + "source": "issue #40", + "file": "dsp-verification.md", + } + result, err := WriteHandler(res, idx)(context.Background(), req) + if err != nil { + t.Fatalf("WriteHandler() error: %v", err) + } + if result.IsError { + t.Fatalf("WriteHandler() error result: %s", extractTextContent(t, result)) + } + + data, err := os.ReadFile(filepath.Join(dir, "dsp-verification.md")) + if err != nil { + t.Fatalf("ReadFile() error: %v", err) + } + content := string(data) + for _, want := range []string{"## Cost cycles", "DSP cost cycles block the deadline", "Source: issue #40"} { + if !strings.Contains(content, want) { + t.Errorf("new section missing %q: %s", want, content) + } + } + + queryReq := mcp.CallToolRequest{} + queryReq.Params.Arguments = map[string]interface{}{ + "project": org, + "query": "cycles block the deadline", + } + queryResult, err := QueryHandler(res, idx)(context.Background(), queryReq) + if err != nil { + t.Fatalf("QueryHandler() error: %v", err) + } + queryContent := extractTextContent(t, queryResult) + if !strings.Contains(queryContent, "org-dsp-verification.md::Cost cycles") { + t.Errorf("query after org write did not find the section: %s", queryContent) + } + + listReq := mcp.CallToolRequest{} + listReq.Params.Arguments = map[string]interface{}{"project": org} + listResult, err := ListHandler(res, idx)(context.Background(), listReq) + if err != nil { + t.Fatalf("ListHandler() error: %v", err) + } + listContent := extractTextContent(t, listResult) + if !strings.Contains(listContent, "### dsp-verification.md (1 sections)") { + t.Errorf("list after org write did not group the new file: %s", listContent) + } +} + +func TestWriteHandlerOrgTrimsHeading(t *testing.T) { + res, org, dir := newOrgFixture(t, map[string]string{"architecture.md": "## Build\n"}) + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "summary": " Cost cycles ", + "detail": "cost", + "source": "test", + "file": "dsp-verification.md", + } + result, _ := WriteHandler(res, nil)(context.Background(), req) + if result.IsError { + t.Fatalf("WriteHandler() error result: %s", extractTextContent(t, result)) + } + data, err := os.ReadFile(filepath.Join(dir, "dsp-verification.md")) + if err != nil { + t.Fatalf("ReadFile() error: %v", err) + } + if !strings.Contains(string(data), "## Cost cycles\n") { + t.Errorf("heading was not trimmed: %s", data) + } +} + +func TestWriteHandlerOrgBlankSummary(t *testing.T) { + res, org, _ := newOrgFixture(t, map[string]string{"architecture.md": "## Build\n"}) + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "summary": " ", + "detail": "cost", + "source": "test", + "file": "dsp-verification.md", + } + result, _ := WriteHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatal("blank org heading should be an error") + } + if content := extractTextContent(t, result); !strings.Contains(content, "summary must not be blank") { + t.Errorf("error = %q, want a blank-heading rejection", content) + } +} + +func TestWriteHandlerOrgMissingFileListsAvailable(t *testing.T) { + res, org, _ := newOrgFixture(t, map[string]string{ + "architecture.md": "## Build\n", + "review.md": "## Overview\n", + }) + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "summary": "Cost cycles", + "detail": "cost", + "source": "test", + } + result, _ := WriteHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatal("org write without file should be an error") + } + content := extractTextContent(t, result) + if !strings.Contains(content, "file is required") { + t.Errorf("error should say file is required: %s", content) + } + for _, name := range []string{"architecture.md", "review.md"} { + if !strings.Contains(content, name) { + t.Errorf("error should list %s: %s", name, content) + } + } +} + +func TestWriteHandlerOrgNoFiles(t *testing.T) { + res, org, _ := newOrgFixture(t, nil) + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "summary": "Cost cycles", + "detail": "cost", + "source": "test", + } + result, _ := WriteHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatal("org write without file should be an error") + } + content := extractTextContent(t, result) + if !strings.Contains(content, "no markdown files") { + t.Errorf("error should say there are no markdown files: %s", content) + } +} + +func TestWriteHandlerOrgRuleRejected(t *testing.T) { + res, org, _ := newOrgFixture(t, map[string]string{"architecture.md": "## Build\n"}) + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "summary": "Cost cycles", + "detail": "cost", + "source": "test", + "file": "dsp-verification.md", + "rule": "NEVER exceed the deadline", + } + result, _ := WriteHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatal("org write with rule should be an error") + } + content := extractTextContent(t, result) + if !strings.Contains(content, "rule is not supported") { + t.Errorf("error should reject rule: %s", content) + } +} + +func TestWriteHandlerOrgFileValidation(t *testing.T) { + res, org, _ := newOrgFixture(t, map[string]string{"architecture.md": "## Build\n"}) + + for _, name := range []string{"../evil.md", "sub/evil.md", "/tmp/evil.md", "evil.txt", ".md", "evil.md::Heading"} { + t.Run(name, func(t *testing.T) { + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "summary": "Heading", + "detail": "body", + "source": "test", + "file": name, + } + result, _ := WriteHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatalf("org write with file %q should be an error", name) + } + if content := extractTextContent(t, result); !strings.Contains(content, "invalid file") { + t.Errorf("error = %q, want it to reject the filename", content) + } + }) + } +} + +func TestWriteHandlerOrgHeadingCollision(t *testing.T) { + res, org, _ := newOrgFixture(t, map[string]string{ + "architecture.md": "## Build\nmake test\n", + }) + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "summary": "Build", + "detail": "new build notes", + "source": "test", + "file": "architecture.md", + } + result, _ := WriteHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatal("colliding org write should be an error") + } + content := extractTextContent(t, result) + if !strings.Contains(content, "already exists") || !strings.Contains(content, "update_knowledge") { + t.Errorf("error should point at update_knowledge: %s", content) + } +} + +func TestWriteHandlerProjectRequiresCategory(t *testing.T) { + root := t.TempDir() + if err := knowledge.EnsureDir(filepath.Join(root, "proj", ".agents")); err != nil { + t.Fatal(err) + } + res, _, err := projects.Build([]string{root}, nil, "") + if err != nil { + t.Fatalf("projects.Build() error: %v", err) + } + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": "proj", + "summary": "s", + "detail": "d", + "source": "test", + } + result, _ := WriteHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatal("project write without category should be an error") + } + if content := extractTextContent(t, result); !strings.Contains(content, "category is required") { + t.Errorf("error should say category is required: %s", content) + } +} + +func TestUpdateHandlerOrgSection(t *testing.T) { + res, org, dir := newOrgFixture(t, map[string]string{ + "architecture.md": "## Build\n\nold build text\n\nSource: issue #40\n", + }) + + idx := newIndexReady(t) + defer idx.Close() + idx.IndexAll(res) + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "id": "org-architecture.md::Build", + "detail": "new build text", + } + result, err := UpdateHandler(res, idx)(context.Background(), req) + if err != nil { + t.Fatalf("UpdateHandler() error: %v", err) + } + if result.IsError { + t.Fatalf("UpdateHandler() error result: %s", extractTextContent(t, result)) + } + + data, err := os.ReadFile(filepath.Join(dir, "architecture.md")) + if err != nil { + t.Fatalf("ReadFile() error: %v", err) + } + content := string(data) + for _, want := range []string{"## Build", "new build text", "Source: issue #40"} { + if !strings.Contains(content, want) { + t.Errorf("updated section missing %q: %s", want, content) + } + } + if strings.Contains(content, "old build text") { + t.Errorf("old body survived: %s", content) + } + + queryReq := mcp.CallToolRequest{} + queryReq.Params.Arguments = map[string]interface{}{ + "project": org, + "query": "new build text", + } + queryResult, err := QueryHandler(res, idx)(context.Background(), queryReq) + if err != nil { + t.Fatalf("QueryHandler() error: %v", err) + } + if queryContent := extractTextContent(t, queryResult); !strings.Contains(queryContent, "org-architecture.md::Build") { + t.Errorf("query after org update did not find the section: %s", queryContent) + } +} + +func TestUpdateHandlerOrgRejectsAgnosticFields(t *testing.T) { + tests := []struct { + name string + args map[string]interface{} + want string + }{ + { + name: "summary", + args: map[string]interface{}{"summary": "Renamed"}, + want: "summary is not supported", + }, + { + name: "rule", + args: map[string]interface{}{"rule": "NEVER do it"}, + want: "rule is not supported", + }, + { + name: "supersedes", + args: map[string]interface{}{"supersedes": "conv-001"}, + want: "supersedes is not supported", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res, org, _ := newOrgFixture(t, map[string]string{"architecture.md": "## Build\nold\n"}) + args := map[string]interface{}{ + "project": org, + "id": "org-architecture.md::Build", + "detail": "new", + } + for k, v := range tt.args { + args[k] = v + } + req := mcp.CallToolRequest{} + req.Params.Arguments = args + result, _ := UpdateHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatal("expected error result") + } + if content := extractTextContent(t, result); !strings.Contains(content, tt.want) { + t.Errorf("error = %q, want it to contain %q", content, tt.want) + } + }) + } +} + +func TestUpdateHandlerOrgInvalidID(t *testing.T) { + res, org, _ := newOrgFixture(t, map[string]string{"architecture.md": "## Build\nold\n"}) + + for _, id := range []string{"conv-001", "org-architecture.md", "org-../evil.md::Build", "org-other.md::Build"} { + t.Run(id, func(t *testing.T) { + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "id": id, + "detail": "new", + } + result, _ := UpdateHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatalf("update with id %q should be an error", id) + } + if content := extractTextContent(t, result); !strings.Contains(content, "id") && !strings.Contains(content, "file") { + t.Errorf("error = %q, want it to reject the id or file", content) + } + }) + } +} + +func TestUpdateHandlerProjectRequiresCategory(t *testing.T) { + root := t.TempDir() + agents := filepath.Join(root, "proj", ".agents") + if err := knowledge.EnsureDir(agents); err != nil { + t.Fatal(err) + } + if err := knowledge.Save(knowledge.CategoryFilePath(agents, "conventions"), + &knowledge.KnowledgeFile{Project: "proj", Version: 1, Entries: []knowledge.Entry{ + {ID: "conv-001", Summary: "s", Detail: "d", Source: "test", Date: "2026-01-01"}, + }}); err != nil { + t.Fatal(err) + } + res, _, err := projects.Build([]string{root}, nil, "") + if err != nil { + t.Fatalf("projects.Build() error: %v", err) + } + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": "proj", + "id": "conv-001", + "detail": "new", + } + result, _ := UpdateHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatal("project update without category should be an error") + } + if content := extractTextContent(t, result); !strings.Contains(content, "category is required") { + t.Errorf("error = %q, want category is required", content) + } +} + +func TestUpdateHandlerOrgMissingHeading(t *testing.T) { + res, org, _ := newOrgFixture(t, map[string]string{"architecture.md": "## Build\nold\n"}) + + req := mcp.CallToolRequest{} + req.Params.Arguments = map[string]interface{}{ + "project": org, + "id": "org-architecture.md::Missing", + "detail": "new", + } + result, _ := UpdateHandler(res, nil)(context.Background(), req) + if result == nil || !result.IsError { + t.Fatal("update with unknown heading should be an error") + } + if content := extractTextContent(t, result); !strings.Contains(content, "not found") { + t.Errorf("error should say not found: %s", content) + } +} diff --git a/tools/registry.go b/tools/registry.go index ab1e3b5..6c99493 100644 --- a/tools/registry.go +++ b/tools/registry.go @@ -51,15 +51,16 @@ var Registry = []ToolSpec{ }, }, { - Name: ToolWriteKnowledge, Purpose: "Add a new knowledge entry to a project's store", - Description: "Add a new knowledge entry to a project's .agents/ store", + Name: ToolWriteKnowledge, Purpose: "Add a knowledge entry or an org markdown section", + Description: "Add a new knowledge entry to a project's .agents/ store, or a markdown section to an org root's .agents/knowledge/ file", Params: []ParamSpec{ {Name: "project", Description: "Project name (bare if unique, else root/project)", Type: "string", Required: true}, - {Name: "category", Description: fmt.Sprintf("Category: %s", categoryNames()), Type: "string", Required: true}, - {Name: "summary", Description: "One-line description (max 100 chars)", Type: "string", Required: true}, - {Name: "detail", Description: "Concise knowledge (10-20 lines). Summarize key facts, don't copy source files. Include only query-able information.", Type: "string", Required: true}, - {Name: "source", Description: "How this was learned (provenance)", Type: "string", Required: true}, - {Name: "rule", Description: "Imperative constraint this entry enforces (e.g. \"Never create files outside ./tmp\"). Shown prominently in list output. Max 200 chars.", Type: "string"}, + {Name: "category", Description: fmt.Sprintf("Category: %s (required for project stores; ignored for org roots)", categoryNames()), Type: "string"}, + {Name: "summary", Description: "One-line description (max 100 chars); the section heading for org roots", Type: "string", Required: true}, + {Name: "detail", Description: "Concise knowledge (10-20 lines). Summarize key facts, don't copy source files. The section body for org roots.", Type: "string", Required: true}, + {Name: "source", Description: "How this was learned (provenance); folded into the section body for org roots", Type: "string", Required: true}, + {Name: "rule", Description: "Imperative constraint this entry enforces (e.g. \"Never create files outside ./tmp\"). Shown prominently in list output. Max 200 chars. Not supported for org roots.", Type: "string"}, + {Name: "file", Description: "Org roots only: markdown file to append to (e.g. dsp-verification.md). Omit to list the available files.", Type: "string"}, }, }, { @@ -81,16 +82,16 @@ var Registry = []ToolSpec{ }, }, { - Name: ToolUpdateKnowledge, Purpose: "Update an existing knowledge entry by ID", - Description: "Update an existing knowledge entry by ID", + Name: ToolUpdateKnowledge, Purpose: "Update a knowledge entry or an org markdown section", + Description: "Update an existing knowledge entry by ID, or replace an org markdown section body (id org-::)", Params: []ParamSpec{ {Name: "project", Description: "Project name (bare if unique, else root/project)", Type: "string", Required: true}, - {Name: "category", Description: fmt.Sprintf("Category: %s", categoryNames()), Type: "string", Required: true}, - {Name: "id", Description: "Entry ID to update (e.g., conv-001)", Type: "string", Required: true}, - {Name: "summary", Description: "New summary (optional)", Type: "string"}, - {Name: "detail", Description: "New detail (optional)", Type: "string"}, - {Name: "rule", Description: "New imperative rule (optional, max 200 chars)", Type: "string"}, - {Name: "supersedes", Description: "ID of entry this supersedes (optional)", Type: "string"}, + {Name: "category", Description: fmt.Sprintf("Category: %s (required for project stores; ignored for org roots)", categoryNames()), Type: "string"}, + {Name: "id", Description: "Entry ID (conv-001) or org section ID (org-::)", Type: "string", Required: true}, + {Name: "summary", Description: "New summary (optional; not supported for org sections)", Type: "string"}, + {Name: "detail", Description: "New detail (optional; required for org sections)", Type: "string"}, + {Name: "rule", Description: "New imperative rule (optional, max 200 chars; not supported for org sections)", Type: "string"}, + {Name: "supersedes", Description: "ID of entry this supersedes (optional; not supported for org sections)", Type: "string"}, }, }, { diff --git a/tools/update.go b/tools/update.go index 10631f5..c5af734 100644 --- a/tools/update.go +++ b/tools/update.go @@ -20,40 +20,14 @@ func UpdateHandler(res *projects.Resolver, idx *search.Index) func(context.Conte return mcp.NewToolResultError(err.Error()), nil } - category, err := request.RequireString("category") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - if !knowledge.IsValidCategory(category) { - return mcp.NewToolResultError(fmt.Sprintf( - "invalid category: %q (must be %s)", category, strings.Join(knowledge.ValidCategories(), ", "))), nil - } - - id, err := request.RequireString("id") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil - } - ref, resolveErr := res.Resolve(project) if resolveErr != nil { return mcp.NewToolResultError(resolveErr.Error()), nil } - if ref.Kind == projects.KindOrg { - return mcp.NewToolResultError(fmt.Sprintf( - "%q is an org root; org-level knowledge is file-based — edit %s directly", - project, res.OrgKnowledgeDir(ref))), nil - } - - agentsDir := res.AgentsDir(ref) - catPath := knowledge.CategoryFilePath(agentsDir, category) - mu := fileLocks.Get(catPath) - mu.Lock() - defer mu.Unlock() - - kf, loadErr := knowledge.Load(catPath) - if loadErr != nil { - return mcp.NewToolResultError(fmt.Sprintf("load knowledge file: %v", loadErr)), nil + id, err := request.RequireString("id") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil } // Collect optional fields @@ -62,6 +36,19 @@ func UpdateHandler(res *projects.Resolver, idx *search.Index) func(context.Conte newRule := request.GetString("rule", "") newSupersedes := request.GetString("supersedes", "") + if ref.Kind == projects.KindOrg { + return updateOrgSection(res, ref, id, newSummary, newDetail, newRule, newSupersedes), nil + } + + category := request.GetString("category", "") + if category == "" { + return mcp.NewToolResultError("category is required for project stores"), nil + } + if !knowledge.IsValidCategory(category) { + return mcp.NewToolResultError(fmt.Sprintf( + "invalid category: %q (must be %s)", category, strings.Join(knowledge.ValidCategories(), ", "))), nil + } + // Validate optional fields before applying if newSummary != "" && len(newSummary) > 100 { return mcp.NewToolResultError(fmt.Sprintf("summary too long: %d chars (max 100)", len(newSummary))), nil @@ -78,6 +65,18 @@ func UpdateHandler(res *projects.Resolver, idx *search.Index) func(context.Conte return mcp.NewToolResultError("no fields to update (provide summary, detail, rule, or supersedes)"), nil } + agentsDir := res.AgentsDir(ref) + catPath := knowledge.CategoryFilePath(agentsDir, category) + + mu := fileLocks.Get(catPath) + mu.Lock() + defer mu.Unlock() + + kf, loadErr := knowledge.Load(catPath) + if loadErr != nil { + return mcp.NewToolResultError(fmt.Sprintf("load knowledge file: %v", loadErr)), nil + } + // Validate supersedes references an existing ID in the same category if newSupersedes != "" { found := false diff --git a/tools/write.go b/tools/write.go index 51c6ea6..cd3fe16 100644 --- a/tools/write.go +++ b/tools/write.go @@ -24,13 +24,22 @@ func WriteHandler(res *projects.Resolver, idx *search.Index) func(context.Contex return mcp.NewToolResultError(err.Error()), nil } - category, err := request.RequireString("category") - if err != nil { - return mcp.NewToolResultError(err.Error()), nil + ref, resolveErr := res.Resolve(project) + if resolveErr != nil { + return mcp.NewToolResultError(resolveErr.Error()), nil } - if !knowledge.IsValidCategory(category) { - return mcp.NewToolResultError(fmt.Sprintf( - "invalid category: %q (must be %s)", category, strings.Join(knowledge.ValidCategories(), ", "))), nil + + // Org roots index markdown sections rather than category YAML files, + // so category is only meaningful for project stores. + category := request.GetString("category", "") + if ref.Kind != projects.KindOrg { + if category == "" { + return mcp.NewToolResultError("category is required for project stores"), nil + } + if !knowledge.IsValidCategory(category) { + return mcp.NewToolResultError(fmt.Sprintf( + "invalid category: %q (must be %s)", category, strings.Join(knowledge.ValidCategories(), ", "))), nil + } } summary, err := request.RequireString("summary") @@ -40,6 +49,9 @@ func WriteHandler(res *projects.Resolver, idx *search.Index) func(context.Contex if len(summary) > 100 { return mcp.NewToolResultError(fmt.Sprintf("summary too long: %d chars (max 100)", len(summary))), nil } + if ref.Kind == projects.KindOrg && strings.ContainsAny(summary, "\r\n") { + return mcp.NewToolResultError("summary must be a single line for org sections"), nil + } detail, err := request.RequireString("detail") if err != nil { @@ -59,14 +71,9 @@ func WriteHandler(res *projects.Resolver, idx *search.Index) func(context.Contex return mcp.NewToolResultError(fmt.Sprintf("rule too long: %d chars (max 200)", len(rule))), nil } - ref, resolveErr := res.Resolve(project) - if resolveErr != nil { - return mcp.NewToolResultError(resolveErr.Error()), nil - } if ref.Kind == projects.KindOrg { - return mcp.NewToolResultError(fmt.Sprintf( - "%q is an org root; org-level knowledge is file-based — edit %s directly", - project, res.OrgKnowledgeDir(ref))), nil + file := request.GetString("file", "") + return writeOrgSection(res, ref, project, file, summary, detail, source, rule), nil } agentsDir := res.AgentsDir(ref)