chore(brain): remove brain_write legacy params domain/type/knowledge-path (ADR-0012)
hyperguild#79's caller audit (done in a prior session) found the removal was safe: - The promote/pending flow (#38/#81, ingestion/internal/api/promote.go) operates exclusively on brainDir/raw/ -> brainDir/wiki/<wing>/<hall>/. It never touches brainDir/knowledge/, so removing the knowledge/ write path does not break it. - No live caller uses domain/type or the knowledge/ path: checked hyperguild itself, mathias/skills, mathias/agentsquad, mathias/dispatch, mathias/cad, mathias/cad-atlas. - internal/skills/brain (a skill.go + handlers.go pair) defined a third, even-more-legacy brain_write shape (type/domain/filename, no wing/hall) for an old "supervisor" MCP server whose cmd/supervisor/main.go no longer exists. Nothing outside its own test imported it; deleted entirely as it encodes the exact legacy shape this issue removes. Changes: - ingestion/internal/mcp/handlers.go: brain_write's tool schema and brainWriteArgs drop type/domain; wing+hall are now marked required in the schema. - ingestion/internal/api/handler.go: WriteNote has one behaviour — wing+ hall required, routing to wiki/<wing>/<hall>/. writeLegacyNote and the knowledge/-path branch are deleted. Omitting wing or hall now returns "wing and hall are required" (400 over HTTP, a tool error over MCP) — the only sane failure mode left once there is no other valid destination. - ingestion/internal/capture/ports.go, internal/brainstore/store.go: drop Type/Domain from capture.Note and its WriteNoteOptions construction. pipeline/*.go's unrelated "domain" field (a different, legitimate concept for brain_ingest_raw) is untouched. - internal/skills/brain/: deleted (orphaned legacy tool schema, dead since cmd/supervisor was removed). Tests: added TestWriteNote_RequiresWingAndHall (api), TestBrainWriteRequiresWingAndHall + TestBrainWriteIgnoresLegacyTypeAndDomainParams (mcp) for the new single behaviour; removed tests that only exercised the deleted legacy path (TestWrite_IncludesFrontmatterWhenTypeProvided, TestWriteNote_HallRouteExistingTypeWinsOverOptsType, TestBrainWriteRejectsTraversal, TestBrainWriteAcceptsDoubleDotInName — the latter two's filename-sanitisation behaviour is covered independently by internal/brain/path_test.go's TestNotePath_SanitisesWingAndSlug / TestSanitise, which is unaffected by this change). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user