From 101ace737b8f349bcee96096105ff0c9121c4d44 Mon Sep 17 00:00:00 2001 From: Jack Lau Date: Fri, 16 Oct 2026 09:39:15 -0400 Subject: [PATCH] fix(brain): update existing pages again under gbrain's revision contract gbrain 0.51 (garrytan/gbrain#5149, 2026-09-17) made `put` refuse to replace an existing page unless the caller sends the revision it read: exit 1, a JSON receipt on stdout and `Error [revision_required]` on stderr. devbrain forwards `put` verbatim and Rebuild runs `gbrain put ` with the file on stdin, so with gbrain >= 0.51 every update of an already-indexed page failed and `devbrain rebuild` aborted at the first one. The distill skill ran its put loop with output discarded, so the index quietly froze at each page's first version. putPage runs the put and, when gbrain answers revision_required, runs it once more with --force, replaying the same stdin. The Markdown page on disk is devbrain's source of truth and this wrapper is the index's only writer, so the disk copy always wins; a revision conflict is never real here, and a get round-trip for --expected-revision would add a failure mode without adding safety. Older engines never answer revision_required and so never see --force; a caller's own --force or --expected-revision gets gbrain's answer unchanged, as does any other failure. stdin is buffered only when gbrain would read it (not for an inline --content), so an interactive put cannot block on a terminal. Both call sites use it. The distill skill now reports a failing put instead of hiding it. Tests: black-box put retry, put non-retry on another error, rebuild retry (all watched failing on main first), and the stdin/--content detection table. make test green; verified against gbrain 0.60.248. Co-Authored-By: Claude Fable 5.1 --- assets/skills/distill/SKILL.md | 6 +- internal/brain/brain.go | 7 +- internal/brain/put.go | 60 +++++++++++++ internal/brain/put_force_test.go | 150 +++++++++++++++++++++++++++++++ internal/brain/put_stdin_test.go | 25 ++++++ internal/brain/rebuild.go | 12 +-- 6 files changed, 248 insertions(+), 12 deletions(-) create mode 100644 internal/brain/put.go create mode 100644 internal/brain/put_force_test.go create mode 100644 internal/brain/put_stdin_test.go diff --git a/assets/skills/distill/SKILL.md b/assets/skills/distill/SKILL.md index e94936e..a88df7a 100644 --- a/assets/skills/distill/SKILL.md +++ b/assets/skills/distill/SKILL.md @@ -277,7 +277,8 @@ When in doubt, ledger only the recent, meaningful gaps rather than backfilling h ### 5. Load into the brain index (optional — the pages are already saved on disk) The pages you wrote to `$BRAINDIR` above ARE the durable brain; gbrain is just a per-machine search index over them. Always use `devbrain brain`; it forwards to -gbrain when installed and cleanly no-ops for index writes otherwise. +gbrain when installed and cleanly no-ops for index writes otherwise. A put that +fails prints one line below; carry it into the fold summary instead of hiding it. Slug pages under a **per-project namespace** `/` (NOT the shared `project/` prefix — that flat namespace let same-named pages from different projects @@ -289,7 +290,8 @@ for f in "$BRAINDIR"/*.md; do [ -e "$f" ] || continue base="$(basename "$f" .md)" slug="$project/${base#"$project"-}" # e.g. devbrain/install ; redlens/roadmap-in-queue - devbrain brain put "$slug" < "$f" >/dev/null 2>&1 + out="$(devbrain brain put "$slug" < "$f" 2>&1)" \ + || echo "brain put failed for $slug: $(printf '%s\n' "$out" | tail -n 1)" devbrain brain tag "$slug" "$project" >/dev/null 2>&1 || true done # Embeddings are an OpenAI-backed enhancement — only attempt them when a key is diff --git a/internal/brain/brain.go b/internal/brain/brain.go index b9a16ff..ffd68ac 100644 --- a/internal/brain/brain.go +++ b/internal/brain/brain.go @@ -77,7 +77,12 @@ func Run(args []string, stdout, stderr io.Writer, stdin io.Reader) int { if retrieval && project != "" { return projectFirstPassthrough(gb, args, project, stdout, stderr, stdin) } - code := passthrough(gb, args, stdout, stderr, stdin) + var code int + if gbrainSubcommand(args) == "put" { + code = putPage(gb, args, stdin, stdout, stderr) + } else { + code = passthrough(gb, args, stdout, stderr, stdin) + } if code == 0 && gbrainSubcommand(args) == "put" && hasOpenAIKey() { fmt.Fprintln(stderr, "brain: page stored keyword-only; run 'devbrain brain embed --stale' to update semantic search") } diff --git a/internal/brain/put.go b/internal/brain/put.go new file mode 100644 index 0000000..e2f3588 --- /dev/null +++ b/internal/brain/put.go @@ -0,0 +1,60 @@ +package brain + +import ( + "bytes" + "fmt" + "io" + "strings" +) + +// putNeedsStdin reports whether gbrain will read the page from stdin for this +// put: it does unless --content carries the page inline. gbrain documents a +// trailing --content with no value as "read stdin", so only --content= +// and --content count as inline. +func putNeedsStdin(args []string) bool { + for i, arg := range args { + if strings.HasPrefix(arg, "--content=") { + return false + } + if arg == "--content" && i+1 < len(args) { + return false + } + } + return true +} + +// putPage runs `gbrain put` and, when the engine refuses because the page +// already exists (revision_required, gbrain >= 0.51), writes it again with +// --force. +// +// gbrain's write contract wants the revision a caller read before it replaces +// a page. devbrain has no such reader: the Markdown page on disk is the source +// of truth and this wrapper is the index's only writer, so the disk copy always +// wins and a revision conflict is never a real one here. Older engines never +// answer revision_required, so they never see --force. A caller who already +// passed --force or --expected-revision gets gbrain's own answer, because those +// outcomes carry other codes. The superseded receipt from the first attempt is +// dropped when the retry succeeds; any other failure is relayed unchanged. +// +// stdin is buffered only when gbrain would read it (see putNeedsStdin), so an +// interactive `put --content ...` cannot block on a terminal, and the retry +// replays the same bytes. +func putPage(gb string, args []string, stdin io.Reader, stdout, stderr io.Writer) int { + var content []byte + if stdin != nil && putNeedsStdin(args) { + var err error + if content, err = io.ReadAll(stdin); err != nil { + fmt.Fprintf(stderr, "brain: reading the page from stdin: %v\n", err) + return 1 + } + } + var out, errOut bytes.Buffer + code := passthrough(gb, args, &out, &errOut, bytes.NewReader(content)) + if code != 0 && strings.Contains(out.String()+errOut.String(), "revision_required") { + forced := append(append(make([]string, 0, len(args)+1), args...), "--force") + return passthrough(gb, forced, stdout, stderr, bytes.NewReader(content)) + } + stdout.Write(out.Bytes()) + stderr.Write(errOut.Bytes()) + return code +} diff --git a/internal/brain/put_force_test.go b/internal/brain/put_force_test.go new file mode 100644 index 0000000..7e2c773 --- /dev/null +++ b/internal/brain/put_force_test.go @@ -0,0 +1,150 @@ +package brain_test + +// gbrain 0.51 (2026-09-17) made `put` refuse an existing page unless the caller +// sends the revision it read (exit 1, "revision_required"). devbrain's Markdown +// pages on disk are the source of truth and this wrapper is the index's only +// writer, so a page update must land anyway: the wrapper repeats the write with +// --force instead of surfacing a conflict that is never real for it. + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/TheWeiHu/devbrain/internal/clitest" +) + +// revisionStub stands in for gbrain. Every call appends its argv to +// $STUB_LOG/argv; a put also saves its stdin as $STUB_LOG/stdin.. A put +// without --force answers the way gbrain 0.60 does for an existing page (JSON +// receipt on stdout, Error line on stderr, exit 1); with --force it succeeds. +// STUB_MODE=other-error fails the put differently so a test can prove that +// only revision_required earns a retry. Everything else (tag, delete, embed) +// succeeds silently. +const revisionStub = `#!/bin/sh +printf '%s\n' "$*" >> "$STUB_LOG/argv" +n=$(wc -l < "$STUB_LOG/argv" | tr -d ' ') +case "$1" in + put) + cat > "$STUB_LOG/stdin.$n" + case "$*" in + *--force*) echo "updated $2"; exit 0 ;; + esac + if [ "$STUB_MODE" = other-error ]; then + echo 'Error [invalid_params]: slug has no project prefix' >&2 + exit 2 + fi + printf '{"error":"revision_required","code":"revision_required","message":"The page already exists; an expected revision is required."}\n' + echo 'Error [revision_required]: The page already exists; an expected revision is required.' >&2 + exit 1 + ;; +esac +exit 0 +` + +func newRevisionHarness(t *testing.T) (*clitest.Harness, string) { + t.Helper() + h := clitest.New(t) + h.Project = "owner__alpha" + stub := filepath.Join(t.TempDir(), "gbrain-revision-stub") + clitest.WriteExec(t, stub, revisionStub) + logDir := t.TempDir() + h.Env["DEVBRAIN_GBRAIN"] = stub + h.Env["STUB_LOG"] = logDir + return h, logDir +} + +func stubCalls(t *testing.T, logDir string) []string { + t.Helper() + data, err := os.ReadFile(filepath.Join(logDir, "argv")) + if err != nil { + return nil + } + return strings.Split(strings.TrimRight(string(data), "\n"), "\n") +} + +func stubStdin(t *testing.T, logDir string, call int) string { + t.Helper() + data, err := os.ReadFile(filepath.Join(logDir, fmt.Sprintf("stdin.%d", call))) + if err != nil { + t.Fatalf("stdin of gbrain call %d was not recorded: %v", call, err) + } + return string(data) +} + +func TestBrainPutRetriesWithForceOnRevisionRequired(t *testing.T) { + h, logDir := newRevisionHarness(t) + page := "# Install\nUpdated on disk; the index must follow.\n" + + res := h.RunWith(clitest.RunOpts{Stdin: page}, "brain", "put", "owner__alpha/install") + + if res.Code != 0 { + t.Fatalf("exit %d, want 0\nstdout: %s\nstderr: %s", res.Code, res.Stdout, res.Stderr) + } + calls := stubCalls(t, logDir) + want := []string{"put owner__alpha/install", "put owner__alpha/install --force"} + if strings.Join(calls, "|") != strings.Join(want, "|") { + t.Fatalf("gbrain calls = %q, want %q", calls, want) + } + if got := stubStdin(t, logDir, 2); got != page { + t.Errorf("forced retry received %q as the page, want the same stdin as the first attempt", got) + } + if strings.Contains(res.Stdout+res.Stderr, "revision_required") { + t.Errorf("the superseded conflict receipt leaked into the output\nstdout: %s\nstderr: %s", res.Stdout, res.Stderr) + } + if !strings.Contains(res.Stdout, "updated owner__alpha/install") { + t.Errorf("stdout = %q, want the successful retry's output", res.Stdout) + } +} + +func TestBrainPutLeavesOtherFailuresAlone(t *testing.T) { + h, logDir := newRevisionHarness(t) + + res := h.RunWith(clitest.RunOpts{Stdin: "# x\n", Env: map[string]string{"STUB_MODE": "other-error"}}, + "brain", "put", "install") + + if res.Code != 2 { + t.Fatalf("exit %d, want gbrain's own exit code 2\nstdout: %s\nstderr: %s", res.Code, res.Stdout, res.Stderr) + } + if calls := stubCalls(t, logDir); len(calls) != 1 { + t.Errorf("gbrain called %d times (%q), want exactly once: only revision_required earns a retry", len(calls), calls) + } + if !strings.Contains(res.Stderr, "invalid_params") { + t.Errorf("stderr = %q, want gbrain's error passed through unchanged", res.Stderr) + } +} + +func TestRebuildForcesUpdatesOfExistingPages(t *testing.T) { + h, logDir := newRevisionHarness(t) + dir := filepath.Join(h.Data, "projects", "owner__alpha", "brain") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + page := "# Install\nSecond edition.\n" + clitest.WriteFile(t, filepath.Join(dir, "install.md"), page) + + res := h.Run("rebuild") + + if res.Code != 0 { + t.Fatalf("exit %d, want 0\nstdout: %s\nstderr: %s", res.Code, res.Stdout, res.Stderr) + } + calls := stubCalls(t, logDir) + var puts []string + for _, c := range calls { + if strings.HasPrefix(c, "put ") { + puts = append(puts, c) + } + } + want := []string{"put owner__alpha/install", "put owner__alpha/install --force"} + if strings.Join(puts, "|") != strings.Join(want, "|") { + t.Fatalf("put calls = %q, want %q (all gbrain calls: %q)", puts, want, calls) + } + if got := stubStdin(t, logDir, 2); got != page { + t.Errorf("forced retry received %q, want the on-disk page", got) + } + if !strings.Contains(res.Stdout, "put owner__alpha/install") { + t.Errorf("stdout = %q, want the rebuild's per-page progress line", res.Stdout) + } +} diff --git a/internal/brain/put_stdin_test.go b/internal/brain/put_stdin_test.go new file mode 100644 index 0000000..bcd1204 --- /dev/null +++ b/internal/brain/put_stdin_test.go @@ -0,0 +1,25 @@ +package brain + +import "testing" + +// gbrain reads the page from stdin unless --content carries it inline ("omit +// --content, or put it last, to read stdin"). The wrapper must only buffer +// stdin when gbrain would read it, so `put --content "..."` from a terminal +// cannot block waiting for an EOF that never comes. +func TestPutNeedsStdin(t *testing.T) { + cases := []struct { + args []string + want bool + }{ + {[]string{"put", "p/s"}, true}, + {[]string{"--quiet", "put", "p/s"}, true}, + {[]string{"put", "p/s", "--content", "inline page text"}, false}, + {[]string{"put", "p/s", "--content=inline page text"}, false}, + {[]string{"put", "p/s", "--content"}, true}, + } + for _, c := range cases { + if got := putNeedsStdin(c.args); got != c.want { + t.Errorf("putNeedsStdin(%q) = %v, want %v", c.args, got, c.want) + } + } +} diff --git a/internal/brain/rebuild.go b/internal/brain/rebuild.go index a6588c5..d17b0b5 100644 --- a/internal/brain/rebuild.go +++ b/internal/brain/rebuild.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os" - "os/exec" "path/filepath" "strings" @@ -44,15 +43,10 @@ func Rebuild(stdout, stderr io.Writer) int { if err != nil { return 1 // bash: redirect failure under set -e } - put := gbrainCommand(gb, "put", slug) - put.Stdin, put.Stdout, put.Stderr = in, io.Discard, stderr - err = put.Run() + code := putPage(gb, []string{"put", slug}, in, io.Discard, stderr) in.Close() - if err != nil { // set -e: a failing put aborts the rebuild - if ee, ok := err.(*exec.ExitError); ok { - return ee.ExitCode() - } - return 1 + if code != 0 { // set -e: a failing put aborts the rebuild + return code } tag := gbrainCommand(gb, "tag", slug, project) tag.Stdout, tag.Stderr = io.Discard, io.Discard