diff --git a/ingestion/internal/mcp/tools_notes.go b/ingestion/internal/mcp/tools_notes.go index 72ad383..1a743a8 100644 --- a/ingestion/internal/mcp/tools_notes.go +++ b/ingestion/internal/mcp/tools_notes.go @@ -269,6 +269,14 @@ func touchesFrontmatter(oldStr, newStr string) bool { // the only thing riding on this preview's accuracy is the envelope check // itself, not write safety. func checkClaimEnvelope(brainDir, path, oldStr, newStr string) error { + if oldStr == "" { + // hyperguild#115: strings.Replace treats "" as matching position 0, + // which would make an empty old_str look like an edit that inserts + // text before the fence — a degenerate-input case that belongs to + // notes.Edit's own "old_str is empty" refusal, not to the envelope + // check. + return nil + } before, err := notes.Read(brainDir, path) if err != nil { // Let notes.Edit itself produce the real error (missing file, etc.) @@ -284,7 +292,18 @@ func checkClaimEnvelope(brainDir, path, oldStr, newStr string) error { // (and thus the validation outcome) unchanged, so this is a no-op for // the common case and fail-closed for every case that touches the // envelope, regardless of how old_str is phrased. + _, hadFence := frontmatterBlock(string(before)) after := strings.Replace(string(before), oldStr, newStr, 1) + _, hasFence := frontmatterBlock(after) + if hadFence && !hasFence { + // Round 3: an edit that drops the closing fence degrades `after` + // to "unfenced" — frontmatterScalars then sees an empty map and + // ValidateEnvelope({}) is nil by design (absent fields are valid), + // so an invalid value would otherwise land with zero error while + // also corrupting the note's structure. A previously-fenced note + // must stay fenced, full stop. + return fmt.Errorf("edit would remove the frontmatter fence") + } return claim.ValidateEnvelope(frontmatterScalars(after)) } diff --git a/ingestion/internal/mcp/tools_notes_test.go b/ingestion/internal/mcp/tools_notes_test.go index b32ae14..c2421a5 100644 --- a/ingestion/internal/mcp/tools_notes_test.go +++ b/ingestion/internal/mcp/tools_notes_test.go @@ -168,6 +168,34 @@ func TestBrainEdit_RejectsAnOldStrThatCrossesTheClosingFence(t *testing.T) { assert.Contains(t, string(got), "claim_provenance: asserted", "rejected edit must leave the note byte-identical") } +func TestBrainEdit_RejectsAnEditThatRemovesTheClosingFence(t *testing.T) { + // Found in round-3 review: checkClaimEnvelope always validates the + // resulting frontmatter of `after` — but an edit whose new_str drops + // the closing "---" degrades `after` to "unfenced" in the same call. + // frontmatterBlock(after) then returns ok=false, frontmatterScalars + // returns an empty map, and claim.ValidateEnvelope({}) is nil by + // design (absent fields are valid) — so an invalid enum value lands on + // disk with zero error, and the note is left permanently unfenced. + // Structural fix: a note that had a valid fence before the edit must + // still have one after, or the edit is rejected outright. + dir := newBrainRepo(t) + note := filepath.Join(dir, "wiki/homelab/facts/n.md") + require.NoError(t, os.WriteFile(note, + []byte("---\ntitle: 'n'\nhalf_life: durable\napplies: universal\nclaim_provenance: asserted\n---\n\nbody\n"), 0o644)) + srv := mcp.NewServer(dir, nil, nil, nil) + + resp := call(t, srv, "brain_edit", map[string]any{ + "path": "wiki/homelab/facts/n.md", + "old_str": "claim_provenance: asserted\n---\n", + "new_str": "claim_provenance: not-a-real-value\n", + }) + require.Contains(t, resp, "error", "%v", resp) + + got, err := os.ReadFile(note) + require.NoError(t, err) + assert.Contains(t, string(got), "claim_provenance: asserted\n---\n", "rejected edit must leave the note byte-identical, fence intact") +} + func TestBrainEdit_AllowsAValidClaimEnvelopeEdit(t *testing.T) { dir := newBrainRepo(t) note := filepath.Join(dir, "wiki/homelab/facts/n.md")