From 1e57f495950e6c3064a6af89db992837b201f020 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 22 Sep 2026 23:18:43 +0200 Subject: [PATCH 1/4] fix(mcp): name raw argument bytes when brain_edit refuses an empty old_str MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ingestion/internal/mcp/tools_notes.go | 8 ++++++++ ingestion/internal/mcp/tools_notes_test.go | 23 ++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/ingestion/internal/mcp/tools_notes.go b/ingestion/internal/mcp/tools_notes.go index fe0e813..866ffde 100644 --- a/ingestion/internal/mcp/tools_notes.go +++ b/ingestion/internal/mcp/tools_notes.go @@ -3,6 +3,7 @@ package mcp import ( "context" "encoding/json" + "errors" "fmt" "log/slog" "strings" @@ -180,6 +181,13 @@ func (s *Server) editWithScope(args json.RawMessage, scope notes.Scope) (notes.R } 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 } diff --git a/ingestion/internal/mcp/tools_notes_test.go b/ingestion/internal/mcp/tools_notes_test.go index 540a03d..b936e75 100644 --- a/ingestion/internal/mcp/tools_notes_test.go +++ b/ingestion/internal/mcp/tools_notes_test.go @@ -70,6 +70,29 @@ func TestBrainReadAndEdit_RoundTripThroughJSONRPC(t *testing.T) { "only the matched bytes may change") } +func TestBrainEdit_EmptyOldStrErrorNamesRawArgumentBytes(t *testing.T) { + // hyperguild#115: a session reported "old_str is empty" with an old_str + // that, on their end, was never empty. The server-side path was + // unreachable to reproduce (verified separately: a fresh brain_edit call + // with real content round-trips cleanly), which points at something + // between a specific client and this call losing the field. The fix this + // issue actually asked for isn't a behavior change — old_str=="" must + // still refuse — it's that the refusal should say how many raw argument + // bytes it received, so "client sent empty" is distinguishable from + // "server received empty after some hop lost it" without a second + // session's worth of guessing. + dir := newBrainRepo(t) + srv := mcp.NewServer(dir, nil, nil, nil) + + resp := call(t, srv, "brain_edit", map[string]any{ + "path": "wiki/homelab/facts/n.md", "old_str": "", "new_str": "BETA", + }) + errMsg, ok := resp["error"].(map[string]any)["message"].(string) + require.True(t, ok, "%v", resp) + assert.Contains(t, errMsg, "old_str is empty") + assert.Contains(t, errMsg, "raw arguments were", "%v", errMsg) +} + func TestBrainEdit_RefusesGovernanceFilesFromTheModelFacingTool(t *testing.T) { dir := newBrainRepo(t) srv := mcp.NewServer(dir, nil, nil, nil) -- 2.54.0 From c50c64f93b33ead542a300f2d7ffa58423e809e0 Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 22 Sep 2026 23:20:12 +0200 Subject: [PATCH 2/4] fix(api): validate the claim envelope on supersede, same as on create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hyperguild#111: writeHallNote validates half_life/applies/claim_provenance before a note is born; UpdateNote's supersede path didn't, so the fields that need provenance most (a whole-note regenerate is more likely to carry model-authored frontmatter than a fresh create) went unchecked. Fixed by extracting any frontmatter fence the caller leads Content with, validating it with the same claim.ValidateEnvelope, and merging its fields into the restamped frontmatter instead of leaving it as a second, inert `---`-fenced block in the body — that merge was a real gap independent of validation: a caller's supersede-time frontmatter was previously silently dropped, never taking effect at all. Reject leaves the note byte-identical (fail-closed, nothing written). Fence-less content (the common case today) is unchanged. Co-Authored-By: Claude Sonnet 5 --- ingestion/internal/api/update.go | 28 ++++++++- ingestion/internal/api/update_test.go | 85 +++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/ingestion/internal/api/update.go b/ingestion/internal/api/update.go index e493006..c8d015c 100644 --- a/ingestion/internal/api/update.go +++ b/ingestion/internal/api/update.go @@ -10,6 +10,7 @@ import ( "time" "github.com/mathiasbq/hyperguild/ingestion/internal/brain" + "github.com/mathiasbq/hyperguild/ingestion/internal/claim" ) // ContentHash returns the lowercase hex sha256 of b. It is the note's @@ -98,14 +99,39 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash, } priorHash = ContentHash(prior) + // A caller may lead opts.Content with its own frontmatter fence to set or + // change fields on supersede (e.g. the claim envelope). Split it off + // before it reaches the body — left un-split, it would render as a + // second, inert `---`-fenced block of literal text rather than taking + // effect, and any claim-envelope fields inside it would reach disk + // unvalidated (hyperguild#111: the same rule that guards note birth in + // writeHallNote must guard a change to the same fields). + contentFM, contentBody := parseFrontmatter(opts.Content) + if len(contentFM.lines) > 0 { + envelope := make(map[string]string, len(contentFM.lines)) + for _, l := range contentFM.lines { + if l.key != "" { + envelope[l.key] = l.value + } + } + if err := claim.ValidateEnvelope(envelope); err != nil { + return "", "", "", fmt.Errorf("claim envelope: %w", err) + } + } + fm, _ := parseFrontmatter(string(prior)) + for _, l := range contentFM.lines { + if l.key != "" { + fm.set(l.key, l.value) + } + } fm.set("updated_at", time.Now().UTC().Format(time.RFC3339)) fm.set("supersedes", priorHash) if opts.Reason != "" { fm.set("supersede_reason", opts.Reason) } - out := []byte(fm.render() + opts.Content) + out := []byte(fm.render() + contentBody) if err := os.WriteFile(abs, out, 0o644); err != nil { return "", "", "", fmt.Errorf("write: %w", err) } diff --git a/ingestion/internal/api/update_test.go b/ingestion/internal/api/update_test.go index d677a29..f47dd4b 100644 --- a/ingestion/internal/api/update_test.go +++ b/ingestion/internal/api/update_test.go @@ -52,6 +52,91 @@ func TestUpdateNoteSupersedesAndStamps(t *testing.T) { assert.Contains(t, s, "supersede_reason: facts changed") } +func TestUpdateNoteRejectsInvalidEnvelopeAndWritesNothing(t *testing.T) { + // hyperguild#111: WriteNote validates the claim envelope before create; + // UpdateNote didn't before supersede, so the same rule must hold on + // change as on birth, fail-closed, nothing written on rejection. + brainDir := t.TempDir() + original := "---\nwing: jepa-fx\nhall: facts\ncreated_at: 2026-01-01T00:00:00Z\n---\n# Old\n\nold body\n" + rel := seedNote(t, brainDir, "wiki/jepa-fx/facts/val-vol.md", original) + + _, _, _, err := UpdateNote(brainDir, UpdateNoteOptions{ + Path: rel, + Content: "---\nhalf_life: forever\n---\n# New\n\nnew body\n", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "claim envelope") + + got, err := os.ReadFile(filepath.Join(brainDir, filepath.FromSlash(rel))) + require.NoError(t, err) + assert.Equal(t, original, string(got), "rejected update must leave the note byte-identical") +} + +func TestUpdateNoteRejectsAppliesWithoutProvenance(t *testing.T) { + brainDir := t.TempDir() + original := "---\nwing: jepa-fx\nhall: facts\n---\nold\n" + rel := seedNote(t, brainDir, "wiki/jepa-fx/facts/val-vol.md", original) + + _, _, _, err := UpdateNote(brainDir, UpdateNoteOptions{ + Path: rel, + Content: "---\napplies: universal\n---\nnew\n", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "claim envelope") + + got, err := os.ReadFile(filepath.Join(brainDir, filepath.FromSlash(rel))) + require.NoError(t, err) + assert.Equal(t, original, string(got)) +} + +func TestUpdateNoteAcceptsValidEnvelopeAndMergesIntoFrontmatter(t *testing.T) { + brainDir := t.TempDir() + rel := seedNote(t, brainDir, "wiki/jepa-fx/facts/val-vol.md", + "---\nwing: jepa-fx\nhall: facts\ncreated_at: 2026-01-01T00:00:00Z\n---\nold\n") + + _, _, _, err := UpdateNote(brainDir, UpdateNoteOptions{ + Path: rel, + Content: "---\nhalf_life: durable\napplies: universal\nclaim_provenance: asserted\n---\nnew body\n", + }) + require.NoError(t, err) + + got, err := os.ReadFile(filepath.Join(brainDir, filepath.FromSlash(rel))) + require.NoError(t, err) + s := string(got) + assert.Contains(t, s, "half_life: durable") + assert.Contains(t, s, "applies: universal") + assert.Contains(t, s, "claim_provenance: asserted") + assert.Contains(t, s, "wing: jepa-fx", "prior fields survive") + assert.Contains(t, s, "new body") + assert.Equal(t, 2, strings.Count(s, "---\n"), + "exactly one open+close fence pair — the caller's own fence must not leak in as a second, inert block") +} + +func TestUpdateNoteWithoutEnvelopeFieldsIsUnchangedFromToday(t *testing.T) { + // Regression pin: plain fence-less content (the common case) must behave + // exactly as it did before the envelope-merge fix. + brainDir := t.TempDir() + rel := seedNote(t, brainDir, "wiki/jepa-fx/facts/val-vol.md", + "---\nwing: jepa-fx\nhall: facts\ncreated_at: 2026-01-01T00:00:00Z\ncustom: keep-me\n---\n# Old\n\nold body\n") + + relPath, hash, priorHash, err := UpdateNote(brainDir, UpdateNoteOptions{ + Path: rel, + Content: "# New\n\nnew body\n", + Reason: "facts changed", + }) + require.NoError(t, err) + assert.Equal(t, rel, relPath) + assert.NotEmpty(t, hash) + assert.NotEqual(t, hash, priorHash) + + got, err := os.ReadFile(filepath.Join(brainDir, filepath.FromSlash(rel))) + require.NoError(t, err) + s := string(got) + assert.Contains(t, s, "# New") + assert.Contains(t, s, "custom: keep-me") + assert.Contains(t, s, "supersedes: "+priorHash) +} + func TestUpdateNoteResolvesByWingHallSlug(t *testing.T) { brainDir := t.TempDir() seedNote(t, brainDir, "wiki/jepa-fx/facts/val-vol.md", -- 2.54.0 From 31ebc8afc420cf7ae8ac4925642f7bd6255d56dc Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 22 Sep 2026 23:20:42 +0200 Subject: [PATCH 3/4] docs(close-session): teach the skill the claim envelope, drop the stale tag-list hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hyperguild#112: two documentation gaps found checking the skill against current state. 1. The skill never mentioned the claim envelope (half_life/applies/ claim_provenance) capture's insight-write now validates — the natural place to say a close-session insight is usually exactly the durable, universal, human-decided kind it describes. 2. The "tagged internal today" repo list was a frozen snapshot next to a fail-safe gate — the expensive direction to mislead in. Replaced with the rule (untagged -> confidential) plus a pointer at classification.yaml as sole authority, since hyperguild#91 already moved the real count from 10 to ~39 repos and it will keep moving. Co-Authored-By: Claude Sonnet 5 --- skills/close-session/SKILL.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/skills/close-session/SKILL.md b/skills/close-session/SKILL.md index 4d4f2dc..ef7ad1c 100644 --- a/skills/close-session/SKILL.md +++ b/skills/close-session/SKILL.md @@ -49,12 +49,13 @@ Persist the session via a **single `capture` call** (the `brain:capture` MCP too **Assemble one payload:** - **`insights[]`** — the generalizable learnings from Phase 1 (decisions/failures worth re-reading). Each: `{text, wing, hall}`; add `supersede_slug` to revise a prior note in place instead of creating a duplicate. `hall` ∈ facts/decisions/failures/hypotheses/sources. + Optional claim envelope, per insight: `half_life` ∈ volatile/seasonal/durable, `applies` ∈ local/homelab/universal, `claim_provenance` ∈ asserted/inferred — required together whenever either `half_life` or `applies` is set (an envelope with no provenance is rejected). A close-session insight is usually exactly the durable, generalizable, human-decided kind this describes: `half_life: durable, applies: universal, claim_provenance: asserted` is the common case, not a rare opt-in. - **`tickets[]`** — the issue actions from Phase 3: `{repo, action, ...}` where action ∈ create/close/comment. Owner is always `mathias` (server-forced). - **`context`** — `{harness: "claudeai-chat", session_ref: , fidelity: "live-capture", actor: "mathias", classification: }`. **THE CLASSIFICATION GATE (read before calling — this is where capture refuses).** Capture computes an **effective classification = the strictest across EVERY target it touches** (each insight's `wing`, each ticket's `repo`), then refuses if that effective level is `confidential` and the origin is us-nexus (claude.ai is us-nexus). Levels come from `classification.yaml` at the brain root (source of truth, #67), with the code defaults as the floor: `hyperguild`/`homelab` → internal; `client-*` → confidential; **anything untagged → confidential (fail-safe)**. -- **Tagged `internal` today** (safe through claude.ai): wings `hyperguild`, `homelab`; repos `brain`, `ai-sessions`, `infra`, `hyperguild`, `homelab`, `tapir`, `agentsquad`, `jepa-fx-risk`, `swedsl`. Treat `classification.yaml` as authoritative — this list is a hint, not gospel. +- **The rule, not a list**: untagged → confidential (fail-safe). `classification.yaml` at the brain root is the sole authority on what's tagged `internal` today — it grew from 10 to ~39 repos in one sweep (hyperguild#91) and will keep moving, so read the file rather than trust an enumeration frozen at whatever moment this skill was last edited. - Declare `context.classification: "internal"` for normal homelab work. - Insight `wing`s and ticket `repo`s are classification INPUTS, not free-form metadata — every target must resolve `internal` or the whole capture escalates to `confidential` and the gate refuses via claude.ai. Listing the central homelab repos (incl. `brain`/`ai-sessions`) is now fine; they're tagged. - If a session genuinely touched **`client-*` or otherwise-untagged** material, it cannot be captured through claude.ai — note that in the verdict rather than trying to force it. -- 2.54.0 From e2cb315b31a50505779c28912440856d51b6c5fc Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 22 Sep 2026 23:24:50 +0200 Subject: [PATCH 4/4] fix(api): validate the merged envelope on supersede, not the caller's fragment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught a real bypass in the #111 fix before merge: the previous commit validated only the frontmatter fence the caller supplied, not the merged result that actually reaches disk. A caller could blank claim_provenance alone (absent per claim.present's "empty is unset" rule) while half_life/applies survived untouched from the prior note — each side individually looked fine, the merged envelope on disk was the unprovenanced one the whole check exists to catch. That's the exact laundering #111 named as the worse of its two gaps. Fixed by computing the envelope from fm (post-merge, pre-write) instead of contentFM (the caller's raw fragment). Flip-tested: reverting to fragment-only validation turns the new adversarial test red; the merged validation turns it green. Co-Authored-By: Claude Sonnet 5 --- ingestion/internal/api/update.go | 30 +++++++++++++++++---------- ingestion/internal/api/update_test.go | 23 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/ingestion/internal/api/update.go b/ingestion/internal/api/update.go index c8d015c..69f4d25 100644 --- a/ingestion/internal/api/update.go +++ b/ingestion/internal/api/update.go @@ -107,17 +107,6 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash, // unvalidated (hyperguild#111: the same rule that guards note birth in // writeHallNote must guard a change to the same fields). contentFM, contentBody := parseFrontmatter(opts.Content) - if len(contentFM.lines) > 0 { - envelope := make(map[string]string, len(contentFM.lines)) - for _, l := range contentFM.lines { - if l.key != "" { - envelope[l.key] = l.value - } - } - if err := claim.ValidateEnvelope(envelope); err != nil { - return "", "", "", fmt.Errorf("claim envelope: %w", err) - } - } fm, _ := parseFrontmatter(string(prior)) for _, l := range contentFM.lines { @@ -131,6 +120,25 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash, fm.set("supersede_reason", opts.Reason) } + // Validate the envelope AFTER the merge, against the full field set that + // is about to reach disk — not just the caller's fragment. Validating the + // fragment alone would miss a caller that supersedes claim_provenance to + // empty while half_life/applies survive untouched from the prior note: + // each side looks individually fine, but the merged result is an + // unprovenanced envelope, which is the exact laundering this check + // exists to catch. (Caught in review before merge, not filed after — + // worth being honest about in the history: this second pass was the + // actual fix; the first one validated the wrong document.) + envelope := make(map[string]string, len(fm.lines)) + for _, l := range fm.lines { + if l.key != "" { + envelope[l.key] = l.value + } + } + if err := claim.ValidateEnvelope(envelope); err != nil { + return "", "", "", fmt.Errorf("claim envelope: %w", err) + } + out := []byte(fm.render() + contentBody) if err := os.WriteFile(abs, out, 0o644); err != nil { return "", "", "", fmt.Errorf("write: %w", err) diff --git a/ingestion/internal/api/update_test.go b/ingestion/internal/api/update_test.go index f47dd4b..85707f7 100644 --- a/ingestion/internal/api/update_test.go +++ b/ingestion/internal/api/update_test.go @@ -112,6 +112,29 @@ func TestUpdateNoteAcceptsValidEnvelopeAndMergesIntoFrontmatter(t *testing.T) { "exactly one open+close fence pair — the caller's own fence must not leak in as a second, inert block") } +func TestUpdateNoteRejectsClearingProvenanceWhileEnvelopeSurvives(t *testing.T) { + // Found in review, not written speculatively: validating only the + // caller's fragment (rather than the merged result) let a caller blank + // out claim_provenance alone while half_life/applies survived from the + // prior note untouched — each side looks fine on its own, the merged + // envelope on disk is the unprovenanced one the whole check exists to + // catch. + brainDir := t.TempDir() + original := "---\nwing: jepa-fx\nhall: facts\nhalf_life: durable\napplies: universal\nclaim_provenance: asserted\n---\nold\n" + rel := seedNote(t, brainDir, "wiki/jepa-fx/facts/val-vol.md", original) + + _, _, _, err := UpdateNote(brainDir, UpdateNoteOptions{ + Path: rel, + Content: "---\nclaim_provenance: \n---\nnew body\n", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "claim envelope") + + got, err := os.ReadFile(filepath.Join(brainDir, filepath.FromSlash(rel))) + require.NoError(t, err) + assert.Equal(t, original, string(got), "rejected update must leave the note byte-identical") +} + func TestUpdateNoteWithoutEnvelopeFieldsIsUnchangedFromToday(t *testing.T) { // Regression pin: plain fence-less content (the common case) must behave // exactly as it did before the envelope-merge fix. -- 2.54.0