feat(mcp): record and alert on a governance-file rewrite (#97) (#98)

brain_edit_root was gated but unobserved: on success it emitted a slog.Warn
and nothing else. That proves a rewrite was allowed, not that one happened —
and brain-sync commits it within 15 minutes authored `brain-sync`, so git
history cannot tell a CLAUDE.md rewrite apart from routine note traffic.

A successful brain_edit_root now emits an ntfy notification and an AuditEntry
through the same sink a capture already uses. notes.Result gains
PrevContentHash so both ends of the change are identifiable.

Path and hashes only in the notification body. Reserved at Internal, not
Confidential, because Confidential makes DegradingSink refuse to record while
loki is down rather than buffer. Observation is best-effort and never fails
the call; a failure to observe logs at ERROR.

Ordinary brain_edit on knowledge//wiki/ is deliberately not audited — git
history already carries the same before/after bytes with a diff.

Closes #97.

Co-authored-by: Mathias <mthbqv@gmail.com>

ingestion/internal/mcp/server.go

@@ -49,6 +49,8 @@ type rpcError struct {
 // Server handles MCP JSON-RPC over HTTP for the ingestion service.
 type Server struct {
 	brainDir  string
+	govAudit  capture.AuditSink
+	govNotify govNotifier
 	pipeline  pipeline.Config
 	llm       pipeline.CompleteFunc
 	answerLLM pipeline.CompleteFunc // nil = brain_answer and brain_classify unavailable

ingestion/internal/mcp/tools_notes.go

@@ -6,8 +6,11 @@ import (
 	"fmt"
 	"log/slog"
 	"strings"
+	"time"
 
 	"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
+	"github.com/mathiasbq/hyperguild/ingestion/internal/capture"
+	"github.com/mathiasbq/hyperguild/ingestion/internal/classification"
 	"github.com/mathiasbq/hyperguild/ingestion/internal/notes"
 )
 
@@ -52,6 +55,20 @@ func noteToolDescriptors() []map[string]any {
 	}
 }
 
+// --- hyperguild#97: governance-rewrite observability -----------------------
+
+// govNotifier raises an out-of-band alert. Declared here rather than importing
+// internal/audit so the MCP package keeps depending on capture alone.
+type govNotifier interface {
+	Notify(ctx context.Context, msg string) error
+}
+
+// WithGovernanceObserver attaches the audit sink and notifier that observe a
+// successful brain_edit_root. Both are optional; either may be nil.
+func (s *Server) WithGovernanceObserver(sink capture.AuditSink, n govNotifier) {
+	s.govAudit, s.govNotify = sink, n
+}
+
 type brainReadArgs struct {
 	Path string `json:"path"`
 }
@@ -75,8 +92,17 @@ type brainEditArgs struct {
 }
 
 // brainEdit is the model-facing editor: notes only.
+//
+// Deliberately NOT audited (#97). A note edit is already fully reconstructible
+// from git history, which carries the same before/after bytes plus a diff.
+// Governance files get the extra trail because they change what later agents
+// obey — the asymmetry is about instruction-carrying content, not about how
+// hard the change is to reconstruct. Auditing every note edit would also bury
+// the rare governance record under routine traffic, which is how an alert
+// stops being read.
 func (s *Server) brainEdit(_ context.Context, args json.RawMessage) (json.RawMessage, error) {
-	return s.editWithScope(args, notes.ScopeNotes)
+	_, out, err := s.editWithScope(args, notes.ScopeNotes)
+	return out, err
 }
 
 // brainEditRoot edits a governance file (CLAUDE.md). De-surfaced AND gated on
@@ -89,22 +115,72 @@ func (s *Server) brainEditRoot(ctx context.Context, args json.RawMessage) (json.
 		return nil, fmt.Errorf("brain_edit_root requires the local static principal; " +
 			"a governance file is not editable from a model-facing origin")
 	}
-	out, err := s.editWithScope(args, notes.ScopeRoot)
-	if err == nil {
-		// A durable instruction rewrite should be loud, not silent.
-		slog.Warn("brain_edit_root applied", "principal", info.principal, "result", string(out))
+	res, out, err := s.editWithScope(args, notes.ScopeRoot)
+	if err != nil {
+		// A refusal is not a rewrite. Recording or alerting on one would cry
+		// wolf, and the gate already logs its own denials.
+		return out, err
 	}
+	// A durable instruction rewrite should be loud, not silent.
+	slog.Warn("brain_edit_root applied", "principal", info.principal, "result", string(out))
+	s.observeGovernanceEdit(ctx, info.principal, res)
 	return out, err
 }
 
-func (s *Server) editWithScope(args json.RawMessage, scope notes.Scope) (json.RawMessage, error) {
+// observeGovernanceEdit records and alerts on a rewrite that already landed.
+//
+// Best-effort by construction: the bytes are written and brain-sync will commit
+// them within the quarter hour, so failing the call here would report a
+// non-rewrite to the caller while the rewrite sits on disk. A failure to
+// observe is therefore logged at ERROR — the one case where the trail itself is
+// what broke, and the only remaining signal.
+func (s *Server) observeGovernanceEdit(ctx context.Context, principal string, res notes.Result) {
+	// Path and hashes only. The notification leaves the cluster for a phone
+	// via a third-party topic; a hash identifies the rewrite, the bytes would
+	// leak whatever was written into an instruction file.
+	msg := fmt.Sprintf("brain_edit_root applied\npath=%s\nprincipal=%s\nbefore=%s\nafter=%s",
+		res.Path, principal, res.PrevContentHash, res.ContentHash)
+
+	if s.govNotify != nil {
+		if err := s.govNotify.Notify(ctx, msg); err != nil {
+			slog.Error("brain_edit_root notify failed", "path", res.Path, "err", err)
+		}
+	}
+	if s.govAudit == nil {
+		return
+	}
+	// Internal, not Confidential: a governance file is not client data, and
+	// Confidential would make DegradingSink REFUSE to record while loki is
+	// down rather than buffer — losing exactly the record that matters most.
+	outcome, err := s.govAudit.Reserve(ctx, classification.Internal)
+	if err != nil {
+		slog.Error("brain_edit_root audit reserve failed", "path", res.Path, "err", err)
+		return
+	}
+	entry := capture.AuditEntry{
+		Timestamp:               time.Now().UTC(),
+		Principal:               principal,
+		Actor:                   "brain_edit_root",
+		Harness:                 "mcp",
+		EffectiveClassification: classification.Internal.String(),
+		Items:                   []string{res.Path},
+		SecurityEvents: []string{fmt.Sprintf(
+			"governance file rewritten: %s before=%s after=%s",
+			res.Path, res.PrevContentHash, res.ContentHash)},
+	}
+	if err := s.govAudit.Record(ctx, entry, outcome); err != nil {
+		slog.Error("brain_edit_root audit record failed", "path", res.Path, "err", err)
+	}
+}
+
+func (s *Server) editWithScope(args json.RawMessage, scope notes.Scope) (notes.Result, json.RawMessage, error) {
 	var a brainEditArgs
 	if err := json.Unmarshal(args, &a); err != nil {
-		return nil, fmt.Errorf("parse args: %w", err)
+		return notes.Result{}, nil, fmt.Errorf("parse args: %w", err)
 	}
 	res, err := notes.Edit(s.brainDir, a.Path, a.OldStr, a.NewStr, scope)
 	if err != nil {
-		return nil, err
+		return notes.Result{}, nil, err
 	}
 
 	// Reindex only when the edit could have changed what the index reads.
@@ -121,7 +197,8 @@ func (s *Server) editWithScope(args json.RawMessage, scope notes.Scope) (json.Ra
 			res.ReindexError = ierr.Error()
 		}
 	}
-	return json.Marshal(res)
+	out, merr := json.Marshal(res)
+	return res, out, merr
 }
 
 // wingOf returns the wing for a wiki path, or "" for knowledge/ and root.

ingestion/internal/mcp/tools_notes_test.go

@@ -2,13 +2,22 @@
 package mcp_test
 
 import (
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
+	"github.com/mathiasbq/hyperguild/ingestion/internal/audit"
+	"github.com/mathiasbq/hyperguild/ingestion/internal/brainstore"
+	"github.com/mathiasbq/hyperguild/ingestion/internal/capture"
+	"github.com/mathiasbq/hyperguild/ingestion/internal/capturehttp"
+	"github.com/mathiasbq/hyperguild/ingestion/internal/classification"
 	"github.com/mathiasbq/hyperguild/ingestion/internal/mcp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -125,3 +134,164 @@ func mustJSON(t *testing.T, v any) []byte {
 	require.NoError(t, err)
 	return b
 }
+
+// ---- hyperguild#97: a governance rewrite must be observed, not just gated ----
+
+// govSink captures audit records. Governance edits are rare, so the whole
+// history is kept and asserted on rather than sampled.
+type govSink struct {
+	entries  []capture.AuditEntry
+	outcomes []capture.AuditOutcome
+	recErr   error
+}
+
+func (s *govSink) Reserve(context.Context, classification.Level) (capture.AuditOutcome, error) {
+	return capture.AuditCentral, nil
+}
+
+func (s *govSink) Record(_ context.Context, e capture.AuditEntry, o capture.AuditOutcome) error {
+	s.entries = append(s.entries, e)
+	s.outcomes = append(s.outcomes, o)
+	return s.recErr
+}
+
+type govNotifier struct {
+	msgs []string
+	err  error
+}
+
+func (n *govNotifier) Notify(_ context.Context, msg string) error {
+	n.msgs = append(n.msgs, msg)
+	return n.err
+}
+
+// govServer wires capture (which is what re-derives the static principal) plus
+// the #97 observer, and returns the brain dir so the test can read the file.
+func govServer(t *testing.T) (*mcp.Server, string, *govSink, *govNotifier) {
+	t.Helper()
+	dir := newBrainRepo(t)
+	cfg, err := classification.Load(dir)
+	require.NoError(t, err)
+	svc := capture.NewService(brainstore.New(dir), capFakeTracker{}, cfg, audit.NewSlogSink(nil))
+
+	srv := mcp.NewServer(dir, nil, nil, nil)
+	srv.WithCapture(svc, nil, capStaticTok, "local-cli", capturehttp.NewOriginResolver(nil))
+	sink, notifier := &govSink{}, &govNotifier{}
+	srv.WithGovernanceObserver(sink, notifier)
+	return srv, dir, sink, notifier
+}
+
+// authCall is call() with an Authorization header, so the static principal is
+// present and brain_edit_root's gate opens.
+func authCall(t *testing.T, srv *mcp.Server, authz, name string, args map[string]any) map[string]any {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodPost, "/mcp", body(t, map[string]any{
+		"jsonrpc": "2.0", "id": 1, "method": "tools/call",
+		"params": map[string]any{"name": name, "arguments": args},
+	}))
+	if authz != "" {
+		req.Header.Set("Authorization", authz)
+	}
+	rr := httptest.NewRecorder()
+	srv.ServeHTTP(rr, req)
+	var resp map[string]any
+	require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &resp))
+	return resp
+}
+
+func sha256hex(s string) string {
+	sum := sha256.Sum256([]byte(s))
+	return hex.EncodeToString(sum[:])
+}
+
+func TestBrainEditRoot_RecordsAnAuditEntryNamingPrincipalPathAndBothHashes(t *testing.T) {
+	// The gate proves a rewrite was ALLOWED. Only a record proves one HAPPENED
+	// — and brain-sync will commit it within 15 minutes authored `brain-sync`,
+	// so git history cannot tell it apart from routine note traffic.
+	srv, dir, sink, _ := govServer(t)
+
+	resp := authCall(t, srv, "Bearer "+capStaticTok, "brain_edit_root", map[string]any{
+		"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
+	})
+	require.NotContains(t, resp, "error", "%v", resp["error"])
+
+	require.Len(t, sink.entries, 1, "exactly one audit record per governance rewrite")
+	e := sink.entries[0]
+	assert.Equal(t, "local-cli", e.Principal)
+	assert.Contains(t, e.Items, "CLAUDE.md", "the record must name what was rewritten")
+
+	joined := strings.Join(e.SecurityEvents, " ")
+	assert.Contains(t, joined, sha256hex("# rules\n"), "before-hash must be recorded")
+	assert.Contains(t, joined, sha256hex("# rewritten\n"), "after-hash must be recorded")
+	assert.NotZero(t, e.Timestamp)
+
+	got, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
+	require.NoError(t, err)
+	assert.Equal(t, "# rewritten\n", string(got))
+}
+
+func TestBrainEditRoot_NotifiesWithPathAndHashesButNeverContent(t *testing.T) {
+	srv, _, _, notifier := govServer(t)
+
+	resp := authCall(t, srv, "Bearer "+capStaticTok, "brain_edit_root", map[string]any{
+		"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
+	})
+	require.NotContains(t, resp, "error", "%v", resp["error"])
+
+	require.Len(t, notifier.msgs, 1)
+	msg := notifier.msgs[0]
+	assert.Contains(t, msg, "CLAUDE.md")
+	assert.Contains(t, msg, "local-cli")
+	assert.Contains(t, msg, sha256hex("# rules\n"))
+	assert.Contains(t, msg, sha256hex("# rewritten\n"))
+
+	// The notification travels to a phone through a third-party topic. A hash
+	// identifies a rewrite; the bytes would leak whatever was written.
+	assert.NotContains(t, msg, "# rewritten", "no file content in the notification body")
+	assert.NotContains(t, msg, "# rules")
+}
+
+func TestBrainEditRoot_RefusedProducesNoRecordNoNotificationNoWrite(t *testing.T) {
+	srv, dir, sink, notifier := govServer(t)
+
+	// No Authorization header → no static principal → the gate stays shut.
+	resp := authCall(t, srv, "", "brain_edit_root", map[string]any{
+		"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
+	})
+	require.Contains(t, resp, "error")
+
+	assert.Empty(t, sink.entries, "a refusal is not a rewrite — recording one would cry wolf")
+	assert.Empty(t, notifier.msgs)
+	got, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
+	require.NoError(t, err)
+	assert.Equal(t, "# rules\n", string(got))
+}
+
+func TestBrainEditRoot_FailedEditProducesNoRecordNoNotification(t *testing.T) {
+	// Authorised but no match: the door opened and nothing went through it.
+	srv, _, sink, notifier := govServer(t)
+
+	resp := authCall(t, srv, "Bearer "+capStaticTok, "brain_edit_root", map[string]any{
+		"path": "CLAUDE.md", "old_str": "absent-from-the-file", "new_str": "x",
+	})
+	require.Contains(t, resp, "error")
+
+	assert.Empty(t, sink.entries)
+	assert.Empty(t, notifier.msgs)
+}
+
+func TestBrainEdit_OnOrdinaryNotesIsNotAudited(t *testing.T) {
+	// The decided asymmetry (#97): note edits are covered by git history, which
+	// carries the same before/after bytes with a diff. Governance files get the
+	// extra trail because they change what later agents obey, not because they
+	// are harder to reconstruct.
+	srv, _, sink, notifier := govServer(t)
+
+	resp := authCall(t, srv, "Bearer "+capStaticTok, "brain_edit", map[string]any{
+		"path": "wiki/homelab/facts/n.md", "old_str": "beta", "new_str": "BETA",
+	})
+	require.NotContains(t, resp, "error", "%v", resp["error"])
+
+	assert.Empty(t, sink.entries, "note edits must not fill the governance audit trail")
+	assert.Empty(t, notifier.msgs)
+}

ingestion/internal/notes/edit.go

@@ -36,11 +36,15 @@ var pathLocks sync.Map // string -> *sync.Mutex
 
 // Result is what a successful edit reports.
 type Result struct {
-	Path         string `json:"path"`
-	ContentHash  string `json:"content_hash"`
-	BytesBefore  int    `json:"bytes_before"`
-	BytesAfter   int    `json:"bytes_after"`
-	ReindexError string `json:"reindex_error,omitempty"`
+	Path string `json:"path"`
+	// PrevContentHash is the sha256 of the file as it was before this edit.
+	// Paired with ContentHash it identifies a rewrite in an audit trail or an
+	// alert without carrying any of the bytes (hyperguild#97).
+	PrevContentHash string `json:"prev_content_hash"`
+	ContentHash     string `json:"content_hash"`
+	BytesBefore     int    `json:"bytes_before"`
+	BytesAfter      int    `json:"bytes_after"`
+	ReindexError    string `json:"reindex_error,omitempty"`
 }
 
 // Read returns a note's bytes verbatim.
@@ -149,11 +153,13 @@ func Edit(brainDir, path, oldStr, newStr string, scope Scope) (Result, error) {
 	}
 
 	sum := sha256.Sum256([]byte(after))
+	prev := sha256.Sum256(before)
 	return Result{
-		Path:        t.Rel,
-		ContentHash: hex.EncodeToString(sum[:]),
-		BytesBefore: len(before),
-		BytesAfter:  len(after),
+		Path:            t.Rel,
+		PrevContentHash: hex.EncodeToString(prev[:]),
+		ContentHash:     hex.EncodeToString(sum[:]),
+		BytesBefore:     len(before),
+		BytesAfter:      len(after),
 	}, nil
 }