hyperguild#115: a session reported "old_str is empty" with old_str that was never empty on their end. Server-side round-trip re-verified clean (fresh brain_edit call, real content, succeeds) — the code path itself isn't broken, but its error left no way to tell "client sent empty" apart from "something between the client and here lost the field" without a second investigation. The refusal now names the raw argument byte count. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
231 lines
9.5 KiB
Go
231 lines
9.5 KiB
Go
package mcp
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"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"
|
|
)
|
|
|
|
// noteToolDescriptors are the model-facing halves of brain#4.
|
|
//
|
|
// brain_edit_root is deliberately absent: it dispatches but is not listed, the
|
|
// same de-surfacing ADR-0012 rule 4 applies to brain_index (#78). Hiding the
|
|
// verb matters here for a specific reason — brain content is ingested from the
|
|
// outside world and reaches a model's context, so a listed governance-file
|
|
// editor is a durable prompt-injection path: one poisoned page could rewrite
|
|
// the CLAUDE.md that a .dispatch-allow agent later obeys. A human who knows
|
|
// the tool exists can still call it; a model reading tools/list cannot
|
|
// discover it.
|
|
func noteToolDescriptors() []map[string]any {
|
|
str := func(d string) map[string]any { return map[string]any{"type": "string", "description": d} }
|
|
schema := func(required []string, props map[string]any) json.RawMessage {
|
|
b, _ := json.Marshal(map[string]any{"type": "object", "required": required, "properties": props})
|
|
return b
|
|
}
|
|
return []map[string]any{
|
|
{
|
|
"name": "brain_read",
|
|
"description": "Return a note's raw markdown verbatim — no synthesis, no excerpting. " +
|
|
"Use this to obtain the exact text before brain_edit; never build old_str from a " +
|
|
"brain_query excerpt, which is truncated and normalised and will not match.",
|
|
"inputSchema": schema([]string{"path"}, map[string]any{
|
|
"path": str("brain-relative path under knowledge/ or wiki/, e.g. wiki/homelab/facts/x.md"),
|
|
}),
|
|
},
|
|
{
|
|
"name": "brain_edit",
|
|
"description": "Replace exactly one occurrence of old_str with new_str in a note, leaving " +
|
|
"every other byte identical. Fails loudly if old_str is absent or matches more than " +
|
|
"once — extend old_str until it is unique. Does NOT stamp updated_at or supersedes " +
|
|
"(brain_update does); it is a byte-level patch, not a metadata operation.",
|
|
"inputSchema": schema([]string{"path", "old_str", "new_str"}, map[string]any{
|
|
"path": str("brain-relative path under knowledge/ or wiki/"),
|
|
"old_str": str("exact text to replace; must occur exactly once"),
|
|
"new_str": str("replacement text; empty string deletes old_str"),
|
|
}),
|
|
},
|
|
}
|
|
}
|
|
|
|
// --- 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"`
|
|
}
|
|
|
|
func (s *Server) brainRead(_ context.Context, args json.RawMessage) (json.RawMessage, error) {
|
|
var a brainReadArgs
|
|
if err := json.Unmarshal(args, &a); err != nil {
|
|
return nil, fmt.Errorf("parse args: %w", err)
|
|
}
|
|
content, err := notes.Read(s.brainDir, a.Path)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
return json.Marshal(map[string]any{"path": a.Path, "content": string(content)})
|
|
}
|
|
|
|
type brainEditArgs struct {
|
|
Path string `json:"path"`
|
|
OldStr string `json:"old_str"`
|
|
NewStr string `json:"new_str"`
|
|
}
|
|
|
|
// 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) {
|
|
_, out, err := s.editWithScope(args, notes.ScopeNotes)
|
|
return out, err
|
|
}
|
|
|
|
// brainEditRoot edits a governance file (CLAUDE.md). De-surfaced AND gated on
|
|
// the authenticated principal: only the static local-CLI token may call it, so
|
|
// a claude.ai-origin session — the one that may have just read an untrusted
|
|
// page — cannot rewrite the instructions later agents load.
|
|
func (s *Server) brainEditRoot(ctx context.Context, args json.RawMessage) (json.RawMessage, error) {
|
|
info, ok := ctx.Value(principalKey).(principalInfo)
|
|
if !ok || !info.viaStatic {
|
|
return nil, fmt.Errorf("brain_edit_root requires the local static principal; " +
|
|
"a governance file is not editable from a model-facing origin")
|
|
}
|
|
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
|
|
}
|
|
|
|
// 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 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 {
|
|
if errors.Is(err, notes.ErrDegenerate) && a.OldStr == "" {
|
|
// hyperguild#115: "old_str is empty" alone doesn't say whether the
|
|
// caller sent it empty or a hop between the caller and here lost
|
|
// it. Naming the raw argument byte count makes that distinguishable
|
|
// without a second investigation.
|
|
return notes.Result{}, nil, fmt.Errorf("%w (raw arguments were %d bytes)", err, len(args))
|
|
}
|
|
return notes.Result{}, nil, err
|
|
}
|
|
|
|
// Reindex only when the edit could have changed what the index reads.
|
|
// BuildWingIndex consumes title/created_at from the frontmatter block;
|
|
// BM25 walks the filesystem live and embeddings are mtime-driven, so an
|
|
// unconditional rebuild would buy nothing and dirty a second file —
|
|
// _index.md being precisely the class brain-sync already needs a bespoke
|
|
// conflict resolver for.
|
|
if wing := wingOf(res.Path); wing != "" && touchesFrontmatter(a.OldStr, a.NewStr) {
|
|
if ierr := brain.BuildWingIndex(s.brainDir, wing); ierr != nil {
|
|
// Best-effort, matching every other index hook here: the write
|
|
// already landed and will be committed, so failing the call would
|
|
// make the caller retry an edit whose old_str no longer exists.
|
|
res.ReindexError = ierr.Error()
|
|
}
|
|
}
|
|
out, merr := json.Marshal(res)
|
|
return res, out, merr
|
|
}
|
|
|
|
// wingOf returns the wing for a wiki path, or "" for knowledge/ and root.
|
|
func wingOf(rel string) string {
|
|
parts := strings.Split(rel, "/")
|
|
if len(parts) < 3 || parts[0] != "wiki" {
|
|
return ""
|
|
}
|
|
return parts[1]
|
|
}
|
|
|
|
// touchesFrontmatter reports whether either side of the edit mentions a field
|
|
// the wing index reads.
|
|
func touchesFrontmatter(oldStr, newStr string) bool {
|
|
for _, s := range []string{oldStr, newStr} {
|
|
if strings.Contains(s, "title:") || strings.Contains(s, "created_at:") || strings.Contains(s, "---") {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|