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