diff --git a/mcpsrv/browse.go b/mcpsrv/browse.go new file mode 100644 index 0000000000000000000000000000000000000000..702f0eff1ef4dde6e7f19942976ca193355adf09 --- /dev/null +++ b/mcpsrv/browse.go @@ -0,0 +1,753 @@ +package mcpsrv + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + "github.com/modelcontextprotocol/go-sdk/mcp" + + "sourcecraft.dev/bigbes/sr-ht-dolt/browse" + "sourcecraft.dev/bigbes/sr-ht-dolt/core" +) + +// The generic tools of docs/DESIGN.mcp.md §9.1: the browse layer (browse/) +// offered as tools, for any hosted database rather than a beads tracker. +// +// They live beside read.go rather than in it because §9.1 and §9.2 are two +// chapters with two file sets — read.go keeps list_databases, the entry point +// whose answers every tool here takes its arguments from, and the beads tools of +// §9.2 arrive in a file of their own. One file per chapter is what keeps each of +// them readable; the registration of the whole surface still happens in one +// place (register, read.go). +// +// The rules read.go states once hold here too — the {owner, name} address, +// visibility that is applied rather than re-derived, an array for every list — +// and these are the ones this chapter adds: +// +// - Every tool resolves the database exactly as the browse handlers do +// (resolveDatabase): the row, the caller's ACL grant, core.Allowed. A +// database the caller may not read is *not found*, the same sentence a name +// nobody took gets, because a distinguishable refusal is what the 404 exists +// to erase. +// - A ref that does not resolve and a table that does not exist are the other +// kind of miss entirely: they are ordinary answers *about a database the +// caller can see*, and they name what was not found. Confusing the two would +// either leak the existence of a private database or hide a typo behind a +// sentence about a database the agent is looking straight at. +// - A cap is applied and *stated*. A page that stops short says so and carries +// the number it stopped short of, because a caller that cannot see the table +// has no other way to tell a full answer from a clipped one (§9.3). +// - One session per call, closed by the handler that opened it. That is the +// browse discipline: a fresh read of the on-disk manifest, so a push that +// landed a second ago is visible, and no handle outlives the call. + +// The caps of docs/DESIGN.mcp.md §9.3. +// +// A default exists so that an agent that names no limit gets a page rather than +// a table; a maximum exists because this surface answers in one response and a +// caller can always page. Neither is silent: the answer reports the limit that +// was applied and whether anything was left behind. +const ( + defaultRowLimit = 100 + maxRowLimit = 500 + + defaultCommitLimit = 25 + maxCommitLimit = 100 +) + +// databaseRef is how every tool of this chapter addresses a database: the two +// fields of the URL, both without the "~", exactly as list_databases answers +// them. +// +// It is embedded in each tool's input struct, so the derived schema carries +// owner and name as two required properties of that tool rather than as a nested +// object — an agent copying an address out of a link fills in two strings, and +// does not have to learn a wrapper type first. +type databaseRef struct { + Owner string `json:"owner" jsonschema:"the database owner's SourceHut username, without the \"~\" (a leading one is accepted)"` + Name string `json:"name" jsonschema:"the database name, as list_databases reports it"` +} + +// owner is the username with the sigil and any stray whitespace removed. A +// leading "~" is tolerated rather than refused for read.go's reason: it is what +// a link shows, so an agent copying an address is more likely to include it than +// not. +func (r databaseRef) owner() string { return strings.TrimPrefix(strings.TrimSpace(r.Owner), "~") } + +func (r databaseRef) name() string { return strings.TrimSpace(r.Name) } + +// String renders the address the way a link does, which is how every sentence a +// caller reads names the database it is about. +func (r databaseRef) String() string { return "~" + r.owner() + "/" + r.name() } + +// --- the shapes a caller decodes ------------------------------------------- + +// branchJSON is one branch and the head commit it points at. +type branchJSON struct { + Name string `json:"name"` + Head string `json:"head"` +} + +type listBranchesInput struct { + databaseRef +} + +type listBranchesOutput struct { + Branches []branchJSON `json:"branches"` + + // DefaultBranch is browse.DefaultBranch's answer over the list above, and it + // is reported rather than left for the agent to work out: it is the ref every + // tool here falls back to, so a caller that wants to name the same one + // explicitly should not have to reimplement the rule. It is "" for a database + // with no branches at all. + DefaultBranch string `json:"default_branch"` +} + +// columnJSON is one column of a table's schema. +type columnJSON struct { + Name string `json:"name"` + Type string `json:"type"` + PrimaryKey bool `json:"primary_key"` + Nullable bool `json:"nullable"` +} + +// tableJSON is one table at a ref: its schema and its exact row count. +type tableJSON struct { + Name string `json:"name"` + Columns []columnJSON `json:"columns"` + RowCount uint64 `json:"row_count"` +} + +type listTablesInput struct { + databaseRef + Ref string `json:"ref,omitempty" jsonschema:"a branch name or a commit hash to read at; omit it for the database's default branch"` +} + +type listTablesOutput struct { + // Ref is the ref actually read, which is the default branch when the call + // named none. An agent that omitted it learns from the answer which branch it + // is looking at, and can name that one for the rest of a series of calls. + Ref string `json:"ref"` + Tables []tableJSON `json:"tables"` +} + +type readRowsInput struct { + databaseRef + Table string `json:"table" jsonschema:"the table to read, as list_tables names it"` + Ref string `json:"ref,omitempty" jsonschema:"a branch name or a commit hash to read at; omit it for the database's default branch"` + + // Offset and Limit are pointers so that an omitted argument and an explicit + // zero are two different calls. An omitted limit is "you choose" and gets the + // default; a limit of 0 was typed by the caller, means "no rows at all", and + // is refused rather than quietly turned into 100 — a page nobody asked for + // reads as an empty table, which is the same class of lie as a silent + // truncation. + Offset *int `json:"offset,omitempty" jsonschema:"how many rows to skip, zero or more; defaults to 0"` + Limit *int `json:"limit,omitempty" jsonschema:"how many rows to return, at most 500; defaults to 100"` +} + +type readRowsOutput struct { + Ref string `json:"ref"` + Table string `json:"table"` + + // Columns names the cells of every row, in order: for a keyed table the + // primary-key columns first, then the rest. + Columns []string `json:"columns"` + + // Rows are rendered strings and not typed values: this surface reads a bare + // store without a SQL engine, so what it has is the stored value formatted. + // A NULL cell reads as "NULL" and an unprintable one as "", neither + // of which is distinguishable from a row that stores those very strings — + // which is a limit worth knowing rather than a bug to work around. + Rows [][]string `json:"rows"` + + Offset int `json:"offset"` + + // Limit is the limit that was applied, which is not always the one asked for: + // a request above the cap is answered at the cap, and saying so here is what + // keeps that from being a silent clamp. + Limit int `json:"limit"` + + // Total is the number of rows in the whole table at this ref — the honest + // denominator of the page above, straight from browse.RowPage. + Total int `json:"total"` + + // Truncated reports that rows remain after this page (docs/DESIGN.mcp.md + // §9.3). Raise offset to read them. + Truncated bool `json:"truncated"` +} + +// commitJSON is one commit of a log listing. +// +// The author's email is deliberately absent: the browse UI publishes the name +// and not the address (web/templates/log.html), and a surface reachable by any +// token has no business publishing more of a person than the page does. +type commitJSON struct { + Hash string `json:"hash"` + Author string `json:"author"` + Date time.Time `json:"date"` + Message string `json:"message"` + Parents []string `json:"parents"` +} + +type getCommitLogInput struct { + databaseRef + Ref string `json:"ref,omitempty" jsonschema:"a branch name or a commit hash to start from; omit it for the database's default branch"` + From string `json:"from,omitempty" jsonschema:"the \"next\" hash of a previous page, to continue where it stopped; it takes the place of ref"` + Limit *int `json:"limit,omitempty" jsonschema:"how many commits to return, at most 100; defaults to 25"` +} + +type getCommitLogOutput struct { + // Ref is the ref this page was read at, and is empty when the call continued + // a cursor: a from-hash is a point in the graph and claiming it belongs to + // some branch would be a claim this service did not check. + Ref string `json:"ref"` + Commits []commitJSON `json:"commits"` + + // Next is the hash to pass as from for the following page, or "" at the end + // of the history. + Next string `json:"next"` + + // Limit is the limit applied, as in readRowsOutput. + Limit int `json:"limit"` + + // Truncated reports that the history goes on past this page. + // + // There is no total beside it, and that is not an omission: a commit count + // means walking the whole graph, browse/ offers no such call, and a number + // that expensive would be paid for by every caller of a paging tool. Next is + // the honest continuation here — it is present exactly when something was + // left behind. + Truncated bool `json:"truncated"` +} + +// tableDiffJSON is how one table changed in a commit. +type tableDiffJSON struct { + Name string `json:"name"` + Added bool `json:"added"` + Dropped bool `json:"dropped"` + SchemaChanged bool `json:"schema_changed"` + + // The row counts are exact, with one documented exception browse/ owns: when + // the primary key set changed there is no row-level correspondence to count + // across, so the three are 0 and SchemaChanged is true. + RowsAdded int64 `json:"rows_added"` + RowsRemoved int64 `json:"rows_removed"` + RowsModified int64 `json:"rows_modified"` +} + +type getCommitDiffInput struct { + databaseRef + Hash string `json:"hash" jsonschema:"the commit to summarize; a branch name is accepted and means that branch's head"` +} + +type getCommitDiffOutput struct { + // Hash is the resolved commit hash, which is worth carrying back when the + // call named a branch: it pins which commit the answer is about even if the + // branch moves a moment later. + Hash string `json:"hash"` + Tables []tableDiffJSON `json:"tables"` +} + +// --- registration ----------------------------------------------------------- + +// registerBrowseTools installs the tools of docs/DESIGN.mcp.md §9.1. +// +// The descriptions are the agent-facing documentation of this surface: what the +// tool answers, how to address it, what it costs, and — for every one of them — +// what it cannot do, since an agent that knows there is no SQL here stops +// looking for it. +func (s *Server) registerBrowseTools() { + mcp.AddTool(s.mcp, &mcp.Tool{ + Name: "list_branches", + Annotations: readOnlyTool, + Description: "List the branches of one hosted Dolt database, each with the hash of its head commit.\n\n" + + "Address the database by the `owner` and `name` that list_databases reports (the two parts of " + + "its URL, without the \"~\").\n\n" + + "`default_branch` is the branch every other tool reads when you pass no `ref`: \"main\" when it " + + "exists, otherwise the first branch by name. A database nothing has been pushed to yet has no " + + "branches, and answers an empty list with an empty default.\n\n" + + "A database you may not read is reported as not existing, which is the same answer a name " + + "nobody took gets.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in listBranchesInput) (*mcp.CallToolResult, listBranchesOutput, error) { + out, err := s.listBranches(ctx, in) + return nil, out, err + }) + + mcp.AddTool(s.mcp, &mcp.Tool{ + Name: "list_tables", + Annotations: readOnlyTool, + Description: "List the tables of one hosted Dolt database at one ref, each with its columns — name, SQL " + + "type, whether it is part of the primary key, whether it is nullable — and its exact row count.\n\n" + + "`ref` is a branch name or a commit hash; omit it to read the database's default branch, which " + + "the answer names back to you. A ref that matches neither is reported as such, and says so " + + "about that database rather than pretending the database is missing.\n\n" + + "This is the schema of the data, not a query interface: there is no SQL on this surface and " + + "there will not be one. Read a table with read_rows and project or aggregate it yourself.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in listTablesInput) (*mcp.CallToolResult, listTablesOutput, error) { + out, err := s.listTables(ctx, in) + return nil, out, err + }) + + mcp.AddTool(s.mcp, &mcp.Tool{ + Name: "read_rows", + Annotations: readOnlyTool, + Description: "Read one page of rows from one table of a hosted Dolt database.\n\n" + + "`columns` names the cells of every row in order (primary-key columns first for a keyed " + + "table). Cells are rendered strings, because this surface reads a bare store without a SQL " + + "engine: a NULL cell reads as `NULL` and an unprintable one as ``, neither " + + "distinguishable from a row that stores those strings literally.\n\n" + + "`limit` defaults to 100 and is capped at 500; the `limit` in the answer is the one actually " + + "applied, so a larger request is visibly answered at the cap. `total` is the number of rows " + + "in the whole table, and `truncated` says rows remain after this page — read them by raising " + + "`offset`, which is O(1) here regardless of how far in it points.\n\n" + + "Rows come back in the table's own key order and cannot be filtered or sorted server-side. " + + "For a beads tracker, prefer the beads tools: they answer the questions this table would " + + "make you assemble by hand.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in readRowsInput) (*mcp.CallToolResult, readRowsOutput, error) { + out, err := s.readRows(ctx, in) + return nil, out, err + }) + + mcp.AddTool(s.mcp, &mcp.Tool{ + Name: "get_commit_log", + Annotations: readOnlyTool, + Description: "Read the commit history of a hosted Dolt database, newest first: hash, author, date, " + + "message and parent hashes.\n\n" + + "`ref` is a branch name or a commit hash to start from; omit it for the default branch. " + + "`limit` defaults to 25 and is capped at 100.\n\n" + + "When the history goes on past the page, `truncated` is true and `next` carries the hash of " + + "the commit that follows it: pass that as `from` for the next page. `from` takes the place of " + + "`ref`, so a continued page reports no ref. There is no commit total — counting one would " + + "mean walking the entire graph — so `next` is how you tell a full answer from a clipped one.\n\n" + + "A merge commit lists every parent; the history is walked in reverse topological order.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in getCommitLogInput) (*mcp.CallToolResult, getCommitLogOutput, error) { + out, err := s.getCommitLog(ctx, in) + return nil, out, err + }) + + mcp.AddTool(s.mcp, &mcp.Tool{ + Name: "get_commit_diff", + Annotations: readOnlyTool, + Description: "Summarize what one commit changed, table by table, against its first parent — and for the " + + "initial commit against an empty database, where every table reads as added.\n\n" + + "`hash` is a commit hash; a branch name is accepted and means that branch's head. Each entry " + + "says whether the table was added or dropped, whether its schema changed, and how many rows " + + "were added, removed and modified.\n\n" + + "The row counts are exact, with one exception: when the primary key set changed there is no " + + "row-level correspondence to count across, so `schema_changed` is true and the three counts " + + "are 0.\n\n" + + "This is a summary and never a row-level diff — no cell values are reported. To see what the " + + "rows became, read the table at this commit with read_rows, passing the hash as `ref`.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in getCommitDiffInput) (*mcp.CallToolResult, getCommitDiffOutput, error) { + out, err := s.getCommitDiff(ctx, in) + return nil, out, err + }) +} + +// --- the handlers ----------------------------------------------------------- + +// listBranches answers list_branches: every branch of one database with its +// head, plus the default the other tools fall back to. +func (s *Server) listBranches(ctx context.Context, in listBranchesInput) (listBranchesOutput, error) { + const tool = "list_branches" + var out listBranchesOutput + + repo, err := s.resolveDatabase(ctx, tool, in.databaseRef) + if err != nil { + return out, err + } + sess, err := s.openStore(ctx, tool, repo) + if err != nil { + return out, err + } + defer sess.Close() + + branches, err := sess.Branches(ctx) + if err != nil { + // Branches resolves no ref and names no table, so there is no miss it can + // report: whatever went wrong here is this service's. + return out, internalError(err, tool) + } + + out.Branches = make([]branchJSON, 0, len(branches)) + for _, b := range branches { + out.Branches = append(out.Branches, branchJSON{Name: b.Name, Head: b.Head}) + } + out.DefaultBranch = browse.DefaultBranch(branches) + return out, nil +} + +// listTables answers list_tables: the schema of a database at one ref. +func (s *Server) listTables(ctx context.Context, in listTablesInput) (listTablesOutput, error) { + const tool = "list_tables" + var out listTablesOutput + + repo, err := s.resolveDatabase(ctx, tool, in.databaseRef) + if err != nil { + return out, err + } + sess, err := s.openStore(ctx, tool, repo) + if err != nil { + return out, err + } + defer sess.Close() + + ref, err := refFor(ctx, sess, tool, in.databaseRef, in.Ref) + if err != nil { + return out, err + } + + tables, err := sess.Tables(ctx, ref) + if err != nil { + return out, refMiss(err, tool, noSuchRef(in.databaseRef, ref)) + } + + out.Ref = ref + out.Tables = make([]tableJSON, 0, len(tables)) + for _, t := range tables { + cols := make([]columnJSON, 0, len(t.Columns)) + for _, c := range t.Columns { + cols = append(cols, columnJSON{ + Name: c.Name, + Type: c.Type, + PrimaryKey: c.PrimaryKey, + Nullable: c.Nullable, + }) + } + out.Tables = append(out.Tables, tableJSON{Name: t.Name, Columns: cols, RowCount: t.RowCount}) + } + return out, nil +} + +// readRows answers read_rows: one page of a table, with the denominator that +// makes the page readable as a page. +func (s *Server) readRows(ctx context.Context, in readRowsInput) (readRowsOutput, error) { + const tool = "read_rows" + var out readRowsOutput + + table := strings.TrimSpace(in.Table) + if table == "" { + return out, errors.New("name the table to read; list_tables names the tables of a database") + } + offset, err := pageOffset(in.Offset) + if err != nil { + return out, err + } + limit, err := pageLimit(in.Limit, defaultRowLimit, maxRowLimit, "rows") + if err != nil { + return out, err + } + + repo, err := s.resolveDatabase(ctx, tool, in.databaseRef) + if err != nil { + return out, err + } + sess, err := s.openStore(ctx, tool, repo) + if err != nil { + return out, err + } + defer sess.Close() + + ref, err := refFor(ctx, sess, tool, in.databaseRef, in.Ref) + if err != nil { + return out, err + } + + page, err := sess.Rows(ctx, ref, table, offset, limit) + if err != nil { + // Two misses about a database the caller can see, and the table arm is + // asked first because it is the more specific of the two. + if errors.Is(err, browse.ErrTableNotFound) { + return out, errors.New(noSuchTable(in.databaseRef, ref, table)) + } + return out, refMiss(err, tool, noSuchRef(in.databaseRef, ref)) + } + + out = readRowsOutput{ + Ref: ref, + Table: table, + Columns: page.Columns, + Rows: page.Rows, + Offset: page.Offset, + Limit: limit, + Total: page.Total, + // The page stopped short exactly when something is left after it. Reading + // it off the page's own offset and length, rather than off "did I get + // limit rows back", is what keeps this honest when browse returns a short + // page for a reason of its own. + Truncated: page.Offset+len(page.Rows) < page.Total, + } + if out.Columns == nil { + out.Columns = []string{} + } + if out.Rows == nil { + out.Rows = [][]string{} + } + return out, nil +} + +// getCommitLog answers get_commit_log: a page of history with the cursor that +// continues it. +func (s *Server) getCommitLog(ctx context.Context, in getCommitLogInput) (getCommitLogOutput, error) { + const tool = "get_commit_log" + var out getCommitLogOutput + + limit, err := pageLimit(in.Limit, defaultCommitLimit, maxCommitLimit, "commits") + if err != nil { + return out, err + } + from := strings.TrimSpace(in.From) + + repo, err := s.resolveDatabase(ctx, tool, in.databaseRef) + if err != nil { + return out, err + } + sess, err := s.openStore(ctx, tool, repo) + if err != nil { + return out, err + } + defer sess.Close() + + // A cursor is a point in the graph and browse.Log starts there, ignoring the + // ref entirely. Resolving a default branch anyway would cost a read and buy a + // ref the answer could not honestly claim the page belongs to. + ref := "" + if from == "" { + if ref, err = refFor(ctx, sess, tool, in.databaseRef, in.Ref); err != nil { + return out, err + } + } + + commits, next, err := sess.Log(ctx, ref, from, limit) + if err != nil { + // browse classifies an unparseable from-hash as a fault of its own rather + // than as a miss (it has no sentinel for it), so a hand-written cursor + // takes the protocol arm here. A cursor comes from a previous page of this + // very tool, and teaching this package to parse a dolt hash would be a + // second reading of a format browse/ owns. + return out, refMiss(err, tool, noSuchRef(in.databaseRef, ref)) + } + + out.Ref = ref + out.Limit = limit + out.Next = next + out.Truncated = next != "" + out.Commits = make([]commitJSON, 0, len(commits)) + for _, c := range commits { + parents := c.ParentHashes + if parents == nil { + parents = []string{} + } + out.Commits = append(out.Commits, commitJSON{ + Hash: c.Hash, + Author: c.Author, + Date: c.Date, + Message: c.Message, + Parents: parents, + }) + } + return out, nil +} + +// getCommitDiff answers get_commit_diff: the per-table summary browse computes +// for one commit against its first parent. +func (s *Server) getCommitDiff(ctx context.Context, in getCommitDiffInput) (getCommitDiffOutput, error) { + const tool = "get_commit_diff" + var out getCommitDiffOutput + + hash := strings.TrimSpace(in.Hash) + if hash == "" { + return out, errors.New("name the commit to summarize; get_commit_log lists the hashes of a database") + } + + repo, err := s.resolveDatabase(ctx, tool, in.databaseRef) + if err != nil { + return out, err + } + sess, err := s.openStore(ctx, tool, repo) + if err != nil { + return out, err + } + defer sess.Close() + + // No default here: a diff is about one named commit, and defaulting to the + // head would answer a question the caller did not ask. + diff, err := sess.CommitSummary(ctx, hash) + if err != nil { + return out, refMiss(err, tool, noSuchCommit(in.databaseRef, hash)) + } + + out.Hash = diff.Hash + out.Tables = make([]tableDiffJSON, 0, len(diff.Tables)) + for _, t := range diff.Tables { + out.Tables = append(out.Tables, tableDiffJSON{ + Name: t.Name, + Added: t.Added, + Dropped: t.Dropped, + SchemaChanged: t.SchemaChanged, + RowsAdded: t.RowsAdded, + RowsRemoved: t.RowsRemoved, + RowsModified: t.RowsModified, + }) + } + return out, nil +} + +// --- resolving, opening, capping -------------------------------------------- + +// resolveDatabase turns an address into a repository the caller is allowed to +// read, or into the one refusal this surface has. +// +// It is the browse handlers' dance call for call (web/router.go's +// loadRepoForBrowse, docs/DESIGN.mcp.md §4.3): the row, the caller's ACL grant, +// core.Allowed for OpBrowse. Two differences from the web, both deliberate: +// +// - There is no forbidden arm. The web tells a caller who may see that a +// database exists but may not read it apart from one who may not learn it +// exists at all (core.NotFoundForPrivate); here both are the same sentence, +// so the distinction cannot be read out of a pair of answers. That is +// stricter than the web and never looser, so nothing becomes visible that +// was not. +// - An ACL lookup that fails is a protocol error, where the web degrades to +// "no grant" and falls back to visibility. On a page that degradation costs +// a signed-in user a rendering; here it would tell an agent that a database +// it was granted access to does not exist, and an agent believes that and +// rewrites its plan. "I could not check" is not "you may not". +func (s *Server) resolveDatabase(ctx context.Context, tool string, ref databaseRef) (*core.Repo, error) { + if ref.owner() == "" || ref.name() == "" { + return nil, errors.New("address a database by both its owner and its name, as list_databases reports them") + } + + caller := callerOf(ctx) + missing := noSuchDatabase(ref) + + repo, err := s.repos.GetRepoByOwnerAndName(ctx, ref.owner(), ref.name()) + if err != nil { + return nil, missingOrDenied(err, tool, missing) + } + + // An anonymous caller holds no ACL entry and there is no user id to look one + // up by; visibility decides alone, which is what core.Allowed does with a nil + // grant. + var mode *core.AccessMode + if caller != nil { + if mode, err = s.repos.EffectiveAccess(ctx, caller.UserID, repo.ID); err != nil { + return nil, internalError(err, tool) + } + } + if !core.Allowed(caller, repo, mode, core.OpBrowse) { + return nil, errors.New(missing) + } + return repo, nil +} + +// openStore opens the bare store of a database the caller may read. The handler +// that calls it closes the session (defer sess.Close()). +// +// A store that will not open is a protocol error and not a miss: the database +// exists, the caller may read it, and this service could not. list_databases +// answers differently — it degrades that one entry and keeps listing, because a +// listing is about a set — but a tool whose whole subject is this database has +// nothing to answer with, and "not found" would be a false statement about the +// data rather than a true one about the server. +func (s *Server) openStore(ctx context.Context, tool string, repo *core.Repo) (BrowseSession, error) { + sess, err := s.opener.Open(ctx, repo.Path) + if err != nil { + return nil, internalError(fmt.Errorf("opening the store of %s/%s: %w", repo.OwnerName, repo.Name, err), tool) + } + return sess, nil +} + +// refFor answers which ref a call reads at: the one it named, or the +// repository's default branch when it named none (docs/DESIGN.mcp.md §9.1), so +// that an agent which does not care about branches never has to name one. +// +// A named ref is passed through unchecked, because browse resolves a branch name +// *or* a commit hash and this package has no business deciding which of the two +// a string is. One that resolves to neither comes back as a miss from the call +// that used it. +// +// A database with no branches at all has no default to fall back to. That is an +// answer and not a failure — nothing has been pushed to it yet, exactly the +// database list_databases reports with a null content — so it is a sentence the +// agent reads rather than an error the client raises. +func refFor(ctx context.Context, sess BrowseSession, tool string, ref databaseRef, named string) (string, error) { + if named = strings.TrimSpace(named); named != "" { + return named, nil + } + + branches, err := sess.Branches(ctx) + if err != nil { + return "", internalError(err, tool) + } + name := browse.DefaultBranch(branches) + if name == "" { + return "", errors.New(ref.String() + " has no branches: nothing has been pushed to it yet, so there is nothing to read") + } + return name, nil +} + +// pageOffset validates the offset of a paging tool: absent is 0, and a negative +// one is refused rather than clamped. A caller that computed -3 has a bug, and +// answering its first page would hide it. +func pageOffset(want *int) (int, error) { + if want == nil { + return 0, nil + } + if *want < 0 { + return 0, fmt.Errorf("offset must be zero or more, not %d", *want) + } + return *want, nil +} + +// pageLimit applies a cap of docs/DESIGN.mcp.md §9.3: absent takes the default, +// more than the maximum is answered at the maximum, and zero or less is refused. +// +// Clamping and refusing are not inconsistent. A caller asking for 5000 rows +// wants as many as it can have, and gets them with the applied limit stated in +// the answer, so nothing is hidden. A caller asking for 0 or -1 wants something +// that does not exist, and quietly handing it a default page would make the +// answer a statement about a request nobody made. +func pageLimit(want *int, def, max int, noun string) (int, error) { + if want == nil { + return def, nil + } + if *want <= 0 { + return 0, fmt.Errorf("limit must be a positive number of %s, not %d", noun, *want) + } + if *want > max { + return max, nil + } + return *want, nil +} + +// --- the sentences a caller reads ------------------------------------------- +// +// Every one of them is built from the arguments of the call being answered and +// never from an error's own text, which is errors.go's rule: browse names +// on-disk paths and dolt internals, and db/ names what it looked up. + +// noSuchDatabase is the masked not-found: one sentence for a database that does +// not exist and for one this caller may not read, so that the two cannot be told +// apart by an agent probing names. +func noSuchDatabase(ref databaseRef) string { + return "no database " + ref.String() + ": it does not exist, or the credential this call carries does not reach it" +} + +// noSuchRef is an ordinary answer about a database the caller *can* see, and +// says so by naming it — the opposite of the masked sentence above. +func noSuchRef(ref databaseRef, named string) string { + return fmt.Sprintf("%s has no branch or commit %q; list_branches names its branches", ref, named) +} + +func noSuchTable(ref databaseRef, at, table string) string { + return fmt.Sprintf("%s has no table %q at %q; list_tables names the tables there", ref, table, at) +} + +func noSuchCommit(ref databaseRef, hash string) string { + return fmt.Sprintf("%s has no commit %q; get_commit_log lists its history", ref, hash) +} diff --git a/mcpsrv/browse_test.go b/mcpsrv/browse_test.go new file mode 100644 index 0000000000000000000000000000000000000000..a1c952832c4745bb181b98af57c4a1cbdf6f90cd --- /dev/null +++ b/mcpsrv/browse_test.go @@ -0,0 +1,759 @@ +package mcpsrv_test + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sourcecraft.dev/bigbes/sr-ht-core/auth" + + "sourcecraft.dev/bigbes/sr-ht-dolt/core" + "sourcecraft.dev/bigbes/sr-ht-dolt/mcpsrv" +) + +// The generic tools of docs/DESIGN.mcp.md §9.1, driven through the same +// in-process MCP client the rest of the suite uses (mcpsrv_test.go carries the +// fixtures, the fakes and the plumbing). +// +// Three properties are worth more than the rest here and each has a section of +// its own below: a database the caller may not read answers exactly as one that +// does not exist; a page that stopped short says so and says how far it had to +// go; and a ref or a table that is not there is an answer *about* a database the +// caller is looking straight at, not the masked refusal. + +// --- the shapes a client decodes -------------------------------------------- +// +// Spelled out here rather than exported from the package: they are this +// surface's contract, and a test that reused the production structs would pass +// no matter what those structs said. + +type ( + branchResult struct { + Name string `json:"name"` + Head string `json:"head"` + } + + listBranchesResult struct { + Branches []branchResult `json:"branches"` + DefaultBranch string `json:"default_branch"` + } + + columnResult struct { + Name string `json:"name"` + Type string `json:"type"` + PrimaryKey bool `json:"primary_key"` + Nullable bool `json:"nullable"` + } + + tableResult struct { + Name string `json:"name"` + Columns []columnResult `json:"columns"` + RowCount uint64 `json:"row_count"` + } + + listTablesResult struct { + Ref string `json:"ref"` + Tables []tableResult `json:"tables"` + } + + readRowsResult struct { + Ref string `json:"ref"` + Table string `json:"table"` + Columns []string `json:"columns"` + Rows [][]string `json:"rows"` + Offset int `json:"offset"` + Limit int `json:"limit"` + Total int `json:"total"` + Truncated bool `json:"truncated"` + } + + commitResult struct { + Hash string `json:"hash"` + Author string `json:"author"` + Date time.Time `json:"date"` + Message string `json:"message"` + Parents []string `json:"parents"` + } + + commitLogResult struct { + Ref string `json:"ref"` + Commits []commitResult `json:"commits"` + Next string `json:"next"` + Limit int `json:"limit"` + Truncated bool `json:"truncated"` + } + + tableDiffResult struct { + Name string `json:"name"` + Added bool `json:"added"` + Dropped bool `json:"dropped"` + SchemaChanged bool `json:"schema_changed"` + RowsAdded int64 `json:"rows_added"` + RowsRemoved int64 `json:"rows_removed"` + RowsModified int64 `json:"rows_modified"` + } + + commitDiffResult struct { + Hash string `json:"hash"` + Tables []tableDiffResult `json:"tables"` + } +) + +// args addresses ~alice/ and adds the key/value pairs given, so that a +// call in a test reads as the one argument it is actually about. +func args(name string, kv ...any) map[string]any { + if len(kv)%2 != 0 { + panic("args: odd key/value list") + } + out := map[string]any{"owner": "alice", "name": name} + for i := 0; i < len(kv); i += 2 { + out[kv[i].(string)] = kv[i+1] + } + return out +} + +func anonSession(t *testing.T) *mcp.ClientSession { + t.Helper() + return connect(t, newServer(t, newFakeRepos(), newFakeOpener()), nil) +} + +func readRows(t *testing.T, s *mcp.ClientSession, a map[string]any) readRowsResult { + t.Helper() + var out readRowsResult + decode(t, call(t, s, "read_rows", a), &out) + return out +} + +func commitLog(t *testing.T, s *mcp.ClientSession, a map[string]any) commitLogResult { + t.Helper() + var out commitLogResult + decode(t, call(t, s, "get_commit_log", a), &out) + return out +} + +// --- what each tool answers ------------------------------------------------- + +func TestListBranchesNamesEveryBranchAndTheDefault(t *testing.T) { + var out listBranchesResult + decode(t, call(t, anonSession(t), "list_branches", args("notes")), &out) + + assert.ElementsMatch(t, []branchResult{ + {Name: "main", Head: "aaaa"}, + {Name: "wip", Head: "c001"}, + }, out.Branches) + assert.Equal(t, "main", out.DefaultBranch, + "the default is browse.DefaultBranch's answer, reported so a caller need not reimplement the rule") +} + +// A database nothing has been pushed to has no branches, and that is an answer: +// an empty list and an empty default, not a failure and not a refusal. +func TestListBranchesOfADatabaseWithNoCommits(t *testing.T) { + res := call(t, anonSession(t), "list_branches", args("empty")) + require.False(t, res.IsError, "%s", errorText(res)) + + var out listBranchesResult + decode(t, res, &out) + assert.Empty(t, out.Branches) + assert.Empty(t, out.DefaultBranch) +} + +func TestListTablesReportsSchemaAndRowCounts(t *testing.T) { + var out listTablesResult + decode(t, call(t, anonSession(t), "list_tables", args("notes")), &out) + + assert.Equal(t, "main", out.Ref) + require.Len(t, out.Tables, 2) + + byName := map[string]tableResult{} + for _, tbl := range out.Tables { + byName[tbl.Name] = tbl + } + require.Contains(t, byName, "notes") + assert.EqualValues(t, notesRows, byName["notes"].RowCount) + assert.EqualValues(t, 3, byName["tags"].RowCount) + assert.Equal(t, []columnResult{ + {Name: "id", Type: "int", PrimaryKey: true, Nullable: false}, + {Name: "body", Type: "text", PrimaryKey: false, Nullable: true}, + }, byName["notes"].Columns) +} + +// An omitted ref is the database's default branch, and the answer names the one +// it read — so an agent that did not care about branches learns which it is +// looking at and can pin it for the rest of a series of calls. +func TestRefDefaultsToTheDefaultBranch(t *testing.T) { + server := newServer(t, newFakeRepos(), newFakeOpener()) + + t.Run("omitted", func(t *testing.T) { + var out listTablesResult + decode(t, call(t, connect(t, server, nil), "list_tables", args("notes")), &out) + assert.Equal(t, "main", out.Ref) + }) + + t.Run("named", func(t *testing.T) { + var out listTablesResult + decode(t, call(t, connect(t, server, nil), "list_tables", args("notes", "ref", "wip")), &out) + assert.Equal(t, "wip", out.Ref) + }) + + // browse.DefaultBranch falls back to the first branch by name when there is + // no "main", and this database has only a "release" — the fallback is the + // rule, not an accident of the other fixtures all being called "main". + t.Run("a database with no main", func(t *testing.T) { + var out listTablesResult + decode(t, call(t, connect(t, server, bob()), "list_tables", args("secrets")), &out) + assert.Equal(t, "release", out.Ref) + }) + + // read_rows and get_commit_log default the same way, and each one is asked + // rather than assumed to share an implementation. + t.Run("read_rows", func(t *testing.T) { + got := readRows(t, connect(t, server, nil), args("notes", "table", "notes")) + assert.Equal(t, "main", got.Ref) + }) + + t.Run("get_commit_log", func(t *testing.T) { + got := commitLog(t, connect(t, server, nil), args("notes")) + assert.Equal(t, "main", got.Ref) + }) +} + +// A database with no branches has no default to fall back to. That is a sentence +// the agent reads — nothing has been pushed yet — and not a protocol failure, and +// not the masked "no such database" either: the database is right there. +func TestADatabaseWithNoBranchesHasNothingToReadAt(t *testing.T) { + server := newServer(t, newFakeRepos(), newFakeOpener()) + + for _, tc := range []struct { + tool string + args map[string]any + }{ + {"list_tables", args("empty")}, + {"read_rows", args("empty", "table", "notes")}, + {"get_commit_log", args("empty")}, + } { + t.Run(tc.tool, func(t *testing.T) { + res := call(t, connect(t, server, nil), tc.tool, tc.args) + require.True(t, res.IsError) + + text := errorText(res) + assert.Contains(t, text, "~alice/empty") + assert.Contains(t, text, "no branches") + assert.NotContains(t, text, "no database", + "the database exists and the caller may read it; only its history is missing") + }) + } +} + +func TestReadRowsAnswersAPageAndTheTotal(t *testing.T) { + got := readRows(t, anonSession(t), args("notes", "table", "notes")) + + assert.Equal(t, "main", got.Ref) + assert.Equal(t, "notes", got.Table) + assert.Equal(t, []string{"id", "body"}, got.Columns) + assert.Equal(t, 0, got.Offset) + assert.Equal(t, 100, got.Limit, "the default of docs/DESIGN.mcp.md §9.3") + assert.Len(t, got.Rows, 100) + assert.Equal(t, []string{"0", "notes row 0"}, got.Rows[0]) + assert.Equal(t, notesRows, got.Total, "the denominator is the whole table, not the page") + assert.True(t, got.Truncated, "1100 rows are left behind and the caller cannot see the table") +} + +func TestReadRowsPagesByOffset(t *testing.T) { + session := anonSession(t) + + t.Run("a page in the middle", func(t *testing.T) { + got := readRows(t, session, args("notes", "table", "notes", "offset", 500, "limit", 10)) + assert.Equal(t, 500, got.Offset) + require.Len(t, got.Rows, 10) + assert.Equal(t, []string{"500", "notes row 500"}, got.Rows[0]) + assert.True(t, got.Truncated) + }) + + t.Run("the last page is not truncated", func(t *testing.T) { + got := readRows(t, session, args("notes", "table", "notes", "offset", notesRows-10, "limit", 100)) + assert.Len(t, got.Rows, 10) + assert.Equal(t, notesRows, got.Total) + assert.False(t, got.Truncated, "nothing follows this page, so nothing was left behind") + }) + + // Past the end is an empty page with the true total, which is how a caller + // that paged one step too far learns it did rather than concluding the table + // emptied out. + t.Run("past the end", func(t *testing.T) { + got := readRows(t, session, args("notes", "table", "notes", "offset", notesRows+50)) + assert.Empty(t, got.Rows) + assert.Equal(t, notesRows, got.Total) + assert.False(t, got.Truncated) + }) +} + +// The cap of docs/DESIGN.mcp.md §9.3 is applied and *stated*: the answer carries +// the limit that was really used and the total it stopped short of, so a caller +// asking for 5000 rows can tell what it got. +func TestReadRowsCapsTheLimitAndSaysSo(t *testing.T) { + got := readRows(t, anonSession(t), args("notes", "table", "notes", "limit", 5000)) + + assert.Equal(t, 500, got.Limit, "the cap, reported rather than silently applied") + assert.Len(t, got.Rows, 500) + assert.Equal(t, notesRows, got.Total) + assert.True(t, got.Truncated) +} + +// A limit of zero or less is refused rather than defaulted: the caller typed a +// number, and answering a default page would make the result a statement about a +// request nobody made. An empty page would read as an empty table. +func TestReadRowsRefusesANonPositivePage(t *testing.T) { + session := anonSession(t) + + for _, limit := range []int{0, -1} { + res := call(t, session, "read_rows", args("notes", "table", "notes", "limit", limit)) + require.True(t, res.IsError, "limit %d", limit) + assert.Contains(t, errorText(res), "limit must be a positive number of rows") + } + + res := call(t, session, "read_rows", args("notes", "table", "notes", "offset", -3)) + require.True(t, res.IsError) + assert.Contains(t, errorText(res), "offset must be zero or more") +} + +func TestReadRowsNeedsATable(t *testing.T) { + res := call(t, anonSession(t), "read_rows", args("notes", "table", "")) + require.True(t, res.IsError) + assert.Contains(t, errorText(res), "table") +} + +func TestGetCommitLogAnswersNewestFirst(t *testing.T) { + got := commitLog(t, anonSession(t), args("notes")) + + assert.Equal(t, "main", got.Ref) + assert.Equal(t, 25, got.Limit, "the default of docs/DESIGN.mcp.md §9.3") + require.Len(t, got.Commits, 25) + + head := got.Commits[0] + assert.Equal(t, "aaaa", head.Hash) + assert.Equal(t, "alice", head.Author) + assert.Equal(t, "commit 0", head.Message) + assert.True(t, headTime.Equal(head.Date), "got %v", head.Date) + assert.Equal(t, []string{"c001"}, head.Parents) +} + +// The commit log has no total — counting one means walking the whole graph — so +// the cursor is what tells a full answer from a clipped one, and it has to +// actually continue the page. +func TestGetCommitLogPagesWithACursor(t *testing.T) { + session := anonSession(t) + + first := commitLog(t, session, args("notes")) + require.True(t, first.Truncated) + require.NotEmpty(t, first.Next) + + second := commitLog(t, session, args("notes", "from", first.Next)) + require.NotEmpty(t, second.Commits) + assert.Equal(t, first.Next, second.Commits[0].Hash, "the cursor is where the next page starts") + assert.Empty(t, second.Ref, + "a from-hash is a point in the graph, and claiming it belongs to a branch would be unchecked") + + last := commitLog(t, session, args("notes", "from", "c149")) + assert.False(t, last.Truncated, "the initial commit is the end of the history") + assert.Empty(t, last.Next) + require.Len(t, last.Commits, 1) + assert.Equal(t, []string{}, last.Commits[0].Parents, + "an empty array rather than null, so an agent can loop without a nil check") +} + +func TestGetCommitLogCapsTheLimitAndSaysSo(t *testing.T) { + got := commitLog(t, anonSession(t), args("notes", "limit", 500)) + + assert.Equal(t, 100, got.Limit, "the cap of docs/DESIGN.mcp.md §9.3") + assert.Len(t, got.Commits, 100) + assert.True(t, got.Truncated) + assert.NotEmpty(t, got.Next) +} + +func TestGetCommitLogRefusesANonPositiveLimit(t *testing.T) { + session := anonSession(t) + + for _, limit := range []int{0, -5} { + res := call(t, session, "get_commit_log", args("notes", "limit", limit)) + require.True(t, res.IsError, "limit %d", limit) + assert.Contains(t, errorText(res), "limit must be a positive number of commits") + } +} + +func TestGetCommitLogReadsTheRefItIsGiven(t *testing.T) { + got := commitLog(t, anonSession(t), args("notes", "ref", "wip", "limit", 3)) + + assert.Equal(t, "wip", got.Ref) + require.NotEmpty(t, got.Commits) + assert.Equal(t, "c001", got.Commits[0].Hash, "the head of that branch, not of the default one") +} + +// The initial commit is compared against an empty database, so every table reads +// as added — the one case where a diff describes the whole database rather than a +// change to it. +func TestGetCommitDiffOnTheInitialCommit(t *testing.T) { + var out commitDiffResult + decode(t, call(t, anonSession(t), "get_commit_diff", args("notes", "hash", "c149")), &out) + + assert.Equal(t, "c149", out.Hash) + require.Len(t, out.Tables, 2) + for _, tbl := range out.Tables { + assert.True(t, tbl.Added, "%s reads as added against the empty root", tbl.Name) + assert.False(t, tbl.Dropped, "%s", tbl.Name) + assert.Zero(t, tbl.RowsRemoved, "%s", tbl.Name) + } + + byName := map[string]tableDiffResult{} + for _, tbl := range out.Tables { + byName[tbl.Name] = tbl + } + assert.EqualValues(t, notesRows, byName["notes"].RowsAdded) + assert.EqualValues(t, 3, byName["tags"].RowsAdded) +} + +// A branch name is accepted and means its head, and the answer carries the +// resolved hash — which pins what was summarized even if the branch moves a +// moment later. +func TestGetCommitDiffResolvesABranchName(t *testing.T) { + var out commitDiffResult + decode(t, call(t, anonSession(t), "get_commit_diff", args("notes", "hash", "main")), &out) + + assert.Equal(t, "aaaa", out.Hash) + require.Len(t, out.Tables, 1) + assert.Equal(t, "notes", out.Tables[0].Name) + assert.EqualValues(t, 2, out.Tables[0].RowsAdded) + assert.EqualValues(t, 1, out.Tables[0].RowsModified) +} + +func TestGetCommitDiffNeedsACommit(t *testing.T) { + res := call(t, anonSession(t), "get_commit_diff", args("notes", "hash", "")) + require.True(t, res.IsError) + assert.Contains(t, errorText(res), "commit") +} + +// --- the two kinds of miss -------------------------------------------------- + +// A ref that resolves to neither a branch nor a commit is an ordinary answer +// about a database the caller is looking straight at: it names the ref, names +// the database, and is not the masked refusal. Masking it instead would send an +// agent off to re-resolve a database that is right there, over a typo. +func TestAnUnresolvableRefIsAnAnswerAboutTheDatabase(t *testing.T) { + server := newServer(t, newFakeRepos(), newFakeOpener()) + + for _, tc := range []struct { + tool string + args map[string]any + }{ + {"list_tables", args("notes", "ref", "nope")}, + {"read_rows", args("notes", "table", "notes", "ref", "nope")}, + {"get_commit_log", args("notes", "ref", "nope")}, + } { + t.Run(tc.tool, func(t *testing.T) { + res := call(t, connect(t, server, nil), tc.tool, tc.args) + require.True(t, res.IsError) + + text := errorText(res) + assert.Contains(t, text, "nope") + assert.Contains(t, text, "~alice/notes") + assert.NotContains(t, text, "no database", "this is not the masked not-found") + }) + } +} + +func TestAMissingTableIsAnAnswerAboutTheDatabase(t *testing.T) { + res := call(t, anonSession(t), "read_rows", args("notes", "table", "ghosts")) + require.True(t, res.IsError) + + text := errorText(res) + assert.Contains(t, text, "ghosts") + assert.Contains(t, text, "~alice/notes") + assert.Contains(t, text, "main", "and says where it looked") + assert.NotContains(t, text, "no database") +} + +func TestAnUnknownCommitIsAnAnswerAboutTheDatabase(t *testing.T) { + res := call(t, anonSession(t), "get_commit_diff", args("notes", "hash", "deadbeef")) + require.True(t, res.IsError) + + text := errorText(res) + assert.Contains(t, text, "deadbeef") + assert.Contains(t, text, "~alice/notes") + assert.NotContains(t, text, "no database") +} + +// --- the visibility matrix -------------------------------------------------- + +// browseTarget is one fixture database with arguments valid for it, so that the +// matrix below exercises every tool against every visibility rather than only +// the ones that need no table or commit. +type browseTarget struct { + db string + table string + commit string +} + +func browseTargets() []browseTarget { + return []browseTarget{ + {db: "notes", table: "notes", commit: "aaaa"}, // PUBLIC + {db: "drafts", table: "drafts", commit: "dddd"}, // UNLISTED + {db: "secrets", table: "secrets", commit: "eeee"}, // PRIVATE, bob is granted RO + } +} + +// browseCall is one tool and the arguments it needs for a given database, so +// that a property worth holding for the whole chapter can be written once and +// asserted for every tool of it. +type browseCall struct { + name string + args func(browseTarget) map[string]any +} + +// browseTools is every tool of docs/DESIGN.mcp.md §9.1 that addresses a single +// database — which is all of them; list_databases addresses none and is the +// other suite's. +func browseTools() []browseCall { + return []browseCall{ + {"list_branches", func(tg browseTarget) map[string]any { return args(tg.db) }}, + {"list_tables", func(tg browseTarget) map[string]any { return args(tg.db) }}, + {"read_rows", func(tg browseTarget) map[string]any { return args(tg.db, "table", tg.table) }}, + {"get_commit_log", func(tg browseTarget) map[string]any { return args(tg.db) }}, + {"get_commit_diff", func(tg browseTarget) map[string]any { return args(tg.db, "hash", tg.commit) }}, + } +} + +// readable is the access matrix of core.Allowed for OpBrowse, spelled out +// independently of the implementation under test: PUBLIC and UNLISTED are +// readable by everyone including anonymity — unlisted hides from a *listing*, not +// from a direct address — and PRIVATE only by its owner and its grantees. +func readable(f fixture, viewer *core.Caller) bool { + if f.visibility != core.VisibilityPrivate { + return true + } + if viewer == nil { + return false + } + if viewer.UserID == aliceID { + return true // alice owns every fixture + } + _, granted := f.acl[viewer.UserID] + return granted +} + +func fixtureNamed(t *testing.T, name string) fixture { + t.Helper() + for _, f := range fixtures() { + if f.name == name { + return f + } + } + t.Fatalf("no fixture named %q", name) + return fixture{} +} + +// Every tool, every visibility, every principal: either the tool answers, or the +// database is reported as not existing. There is no third outcome and in +// particular no "forbidden" — a refusal of its own shape is exactly the +// distinction the masked not-found exists to erase (docs/DESIGN.mcp.md §4.3). +// +// The masked answer is compared against the answer for a database that genuinely +// does not exist, with the name substituted: the two must differ by nothing but +// the name the caller itself supplied. +func TestTheVisibilityMatrix(t *testing.T) { + server := newServer(t, newFakeRepos(), newFakeOpener()) + + callers := []struct { + name string + ac *auth.AuthContext + }{ + {"anonymous", nil}, + {"a stranger", carol()}, + {"a grantee", bob()}, + {"the owner", alice()}, + } + + for _, tool := range browseTools() { + for _, tg := range browseTargets() { + for _, who := range callers { + t.Run(tool.name+"/"+tg.db+"/"+who.name, func(t *testing.T) { + session := connect(t, server, who.ac) + res := call(t, session, tool.name, tool.args(tg)) + + if readable(fixtureNamed(t, tg.db), coreCaller(who.ac)) { + assert.False(t, res.IsError, "%s", errorText(res)) + return + } + + require.True(t, res.IsError, "a database this caller may not read must not answer") + + absent := tg + absent.db = "nosuch" + missing := call(t, session, tool.name, tool.args(absent)) + require.True(t, missing.IsError) + + assert.Equal(t, + strings.Replace(errorText(missing), "nosuch", tg.db, 1), + errorText(res), + "a masked database answers exactly as one that does not exist") + }) + } + } + } +} + +// Nothing about a database the caller may not read leaks through the refusal — +// not a branch head, not another table's name, not a row. The database's own +// name is in the sentence because the caller put it there: that is the whole +// point of building the sentence from the call's arguments (errors.go). +func TestNothingAboutAMaskedDatabaseLeaks(t *testing.T) { + server := newServer(t, newFakeRepos(), newFakeOpener()) + + for _, tool := range browseTools() { + tg := browseTarget{db: "secrets", table: "secrets", commit: "eeee"} + t.Run(tool.name, func(t *testing.T) { + session := connect(t, server, carol()) + body := resultJSON(t, call(t, session, tool.name, tool.args(tg))) + + assert.NotContains(t, body, "release", "not the branch it has") + assert.NotContains(t, body, "keys", "not the other table it has") + assert.NotContains(t, body, "secrets row", "and certainly not a row") + }) + } +} + +// A grantee reads the private database whole: the matrix above proves the tools +// answer, and this proves they answer with its actual contents rather than an +// empty shell. +func TestAGranteeReadsAPrivateDatabase(t *testing.T) { + session := connect(t, newServer(t, newFakeRepos(), newFakeOpener()), bob()) + + got := readRows(t, session, args("secrets", "table", "secrets")) + assert.Equal(t, "release", got.Ref) + assert.Equal(t, 3, got.Total) + require.Len(t, got.Rows, 3) + assert.Equal(t, []string{"0", "secrets row 0"}, got.Rows[0]) +} + +// --- addressing, sessions and failures -------------------------------------- + +// The sigil is what a link shows, so an agent copying an address is more likely +// to include it than not. +func TestABrowseToolToleratesTheSigil(t *testing.T) { + var out listBranchesResult + decode(t, call(t, anonSession(t), "list_branches", map[string]any{"owner": "~alice", "name": "notes"}), &out) + assert.NotEmpty(t, out.Branches) +} + +func TestAddressingNeedsBothParts(t *testing.T) { + session := anonSession(t) + + for _, a := range []map[string]any{ + {"owner": "", "name": "notes"}, + {"owner": "alice", "name": ""}, + } { + res := call(t, session, "list_branches", a) + require.True(t, res.IsError, "%v", a) + assert.Contains(t, errorText(res), "owner") + assert.Contains(t, errorText(res), "name") + } +} + +// One session per call, closed by the handler that opened it. A store held open +// across calls is a stale manifest and a leaked handle, and a tool that opened a +// database it was not asked about would be reading somebody else's. +func TestEveryToolClosesTheSessionItOpened(t *testing.T) { + for _, tool := range browseTools() { + t.Run(tool.name, func(t *testing.T) { + opener := newFakeOpener() + session := connect(t, newServer(t, newFakeRepos(), opener), nil) + + call(t, session, tool.name, tool.args(browseTarget{db: "notes", table: "notes", commit: "aaaa"})) + + assert.Equal(t, []string{storePath("alice", "notes")}, opener.opened, + "exactly the one database the call named") + assert.Equal(t, 1, opener.sessions[storePath("alice", "notes")].closes) + }) + } +} + +// A store that will not open is a protocol error, not a miss: the database +// exists and the caller may read it, and answering "not found" would be a false +// statement about the data instead of a true one about the server. +func TestAStoreThatWillNotOpenIsAProtocolError(t *testing.T) { + session := connect(t, newServer(t, newFakeRepos(), newFakeOpener()), nil) + + res, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "list_branches", + Arguments: args("broken"), + }) + require.Error(t, err, "a store this daemon cannot read is not an answer about the data") + assert.Nil(t, res) + assert.NotContains(t, err.Error(), "/stores", "and the cause is logged, not sent") +} + +// No raw browse error text ever reaches a caller: it carries dolt internals and +// on-disk paths, and this endpoint is reachable by anyone holding any valid +// token — or by nobody at all. +func TestABrowseFailureDisclosesNoDetail(t *testing.T) { + const detail = "read /srv/dolt/~alice/notes/manifest: input/output error" + + for _, tc := range []struct { + tool string + args map[string]any + // fail breaks the one call this tool makes into the store. + fail func(*fakeSession) + }{ + {"list_tables", args("notes"), func(s *fakeSession) { s.tablesErr = errors.New(detail) }}, + {"read_rows", args("notes", "table", "notes"), func(s *fakeSession) { s.rowsErr = errors.New(detail) }}, + {"get_commit_log", args("notes"), func(s *fakeSession) { s.logErr = errors.New(detail) }}, + } { + t.Run(tc.tool, func(t *testing.T) { + opener := newFakeOpener() + tc.fail(opener.sessions[storePath("alice", "notes")]) + session := connect(t, newServer(t, newFakeRepos(), opener), nil) + + res, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: tc.tool, + Arguments: tc.args, + }) + require.Error(t, err, "a store that could not answer is not an answer") + assert.Nil(t, res) + assert.NotContains(t, err.Error(), "/srv/dolt") + assert.NotContains(t, err.Error(), "input/output error") + }) + } +} + +// An ACL lookup that fails is a protocol error and not a masked not-found. The +// web degrades a failed lookup to "no grant" and falls back to visibility, which +// costs a signed-in user a rendering; here it would tell an agent that a database +// it was granted access to does not exist, and an agent believes that. +func TestAnACLLookupFailureIsNotAMissingDatabase(t *testing.T) { + repos := &failingACLRepos{fakeRepos: newFakeRepos()} + session := connect(t, newServer(t, repos, newFakeOpener()), bob()) + + res, err := session.CallTool(context.Background(), &mcp.CallToolParams{ + Name: "list_branches", + Arguments: args("secrets"), + }) + require.Error(t, err, `"I could not check" is not "you may not"`) + assert.Nil(t, res) + assert.NotContains(t, err.Error(), "5432", "the cause is logged, not sent") +} + +// failingACLRepos is the metadata store with a working repository lookup and a +// broken ACL one, which is the shape of a partial database outage. +type failingACLRepos struct { + *fakeRepos +} + +var _ mcpsrv.Repos = (*failingACLRepos)(nil) + +func (f *failingACLRepos) EffectiveAccess(context.Context, int, int) (*core.AccessMode, error) { + return nil, errors.New("dial tcp 127.0.0.1:5432: connection refused") +} diff --git a/mcpsrv/errors.go b/mcpsrv/errors.go index 23664f0b39567a10ead8b7e43a7fe235c5c58c2c..714639bd7c61446295e4b11bb6a39937ede81293 100644 --- a/mcpsrv/errors.go +++ b/mcpsrv/errors.go @@ -7,6 +7,7 @@ "github.com/modelcontextprotocol/go-sdk/jsonrpc" "go.bigb.es/auxilia/scribe" + "sourcecraft.dev/bigbes/sr-ht-dolt/browse" "sourcecraft.dev/bigbes/sr-ht-dolt/db" ) @@ -66,6 +67,27 @@ func missingOrDenied(err error, where, missing string) error { if errors.Is(err, db.ErrNotFound) { // A tool result error: the SDK packs an ordinary error returned by a // handler into a CallToolResult with IsError set. + return errors.New(missing) + } + return internalError(err, where) +} + +// refMiss is the *other* kind of miss, and the reason it is spelled out beside +// missingOrDenied is that the two must never be confused. +// +// missingOrDenied answers about a database the caller may not learn anything +// about, so its sentence says nothing. refMiss answers about a database the +// caller has already been allowed to read, where a ref that resolves to neither +// a branch nor a commit — or, for the tools that name one, a table that is not +// there — is an ordinary fact about that database. Naming it is not a leak: the +// caller is looking straight at it. Masking it instead would send an agent off +// to re-resolve a database that is right there, over a typo in a branch name. +// +// Anything that is not browse's miss takes the protocol arm, for the reason the +// default always does here: an unmapped failure is a bug in a layer below, and +// reporting it as a fact about the data would teach the agent something untrue. +func refMiss(err error, where, missing string) error { + if errors.Is(err, browse.ErrRefNotFound) { return errors.New(missing) } return internalError(err, where) diff --git a/mcpsrv/mcpsrv_test.go b/mcpsrv/mcpsrv_test.go index e36884c40f7bc701351648dd4549f5c74f7c9da2..3141816ad22f9274eeec1746f236be3555cc2fd7 100644 --- a/mcpsrv/mcpsrv_test.go +++ b/mcpsrv/mcpsrv_test.go @@ -5,6 +5,7 @@ "context" "encoding/json" "errors" "fmt" + "strconv" "testing" "time" @@ -61,24 +62,18 @@ func fixtures() []fixture { return []fixture{ { + // The database the paging tools are measured against: more rows than + // read_rows' cap and more commits than get_commit_log's. name: "notes", visibility: core.VisibilityPublic, - session: &fakeSession{ - branches: []browse.Branch{{Name: "main", Head: "aaaa"}}, - commits: []browse.CommitInfo{{Hash: "aaaa", Date: headTime}}, - tables: []browse.TableInfo{{Name: "notes", Columns: []browse.ColumnInfo{{Name: "id"}}}}, - }, + session: notesStore(), }, { // The one a beads-aware tool will answer about: its tables carry the // fingerprint beads.Applies looks for. name: "tracker", visibility: core.VisibilityPublic, - session: &fakeSession{ - branches: []browse.Branch{{Name: "main", Head: "bbbb"}}, - commits: []browse.CommitInfo{{Hash: "bbbb", Date: headTime}}, - tables: beadsTables(), - }, + session: smallStore("main", "bbbb", beadsTables()), }, { // A store created by a repository that has never been pushed to. @@ -97,21 +92,13 @@ }, { name: "drafts", visibility: core.VisibilityUnlisted, - session: &fakeSession{ - branches: []browse.Branch{{Name: "main", Head: "dddd"}}, - commits: []browse.CommitInfo{{Hash: "dddd", Date: headTime}}, - tables: []browse.TableInfo{{Name: "drafts"}}, - }, + session: smallStore("main", "dddd", []fakeTable{newTable("drafts", 3)}), }, { name: "secrets", visibility: core.VisibilityPrivate, acl: map[int]core.AccessMode{bobID: core.AccessRO}, - session: &fakeSession{ - branches: []browse.Branch{{Name: "release", Head: "eeee"}}, - commits: []browse.CommitInfo{{Hash: "eeee", Date: headTime}}, - tables: []browse.TableInfo{{Name: "secrets"}}, - }, + session: smallStore("release", "eeee", []fakeTable{newTable("secrets", 3), newTable("keys", 1)}), }, } } @@ -120,13 +107,146 @@ // headTime is the commit time every fixture head carries, fixed so that a test // can assert the value rather than merely that something was rendered. var headTime = time.Date(2026, 8, 1, 12, 0, 0, 0, time.UTC) +// The size of the "notes" database, chosen so that both caps of +// docs/DESIGN.mcp.md §9.3 are exceeded: a page that stops short is the ordinary +// case here rather than a contrived one, and the totals are prime-ish numbers no +// cap or default divides evenly, so an off-by-a-page is visible. +const ( + notesRows = 1200 + notesCommits = 150 +) + +// notesStore is a database with two branches, a linear history and two tables. +// +// Its head commit is "aaaa" at headTime, because that is what the list_databases +// suite asserts about it; everything else about it exists for the tools of +// ch. 9.1. +func notesStore() *fakeSession { + tables := []fakeTable{newTable("notes", notesRows), newTable("tags", 3)} + + commits := make([]browse.CommitInfo, notesCommits) + for i := range commits { + hash := fmt.Sprintf("c%03d", i) + if i == 0 { + hash = "aaaa" + } + commits[i] = browse.CommitInfo{ + Hash: hash, + Author: "alice", + Date: headTime.Add(-time.Duration(i) * time.Hour), + Message: fmt.Sprintf("commit %d", i), + } + } + for i := range commits[:len(commits)-1] { + commits[i].ParentHashes = []string{commits[i+1].Hash} + } + + diffs := map[string]*browse.CommitDiff{} + for i, c := range commits { + if i == len(commits)-1 { + // The oldest commit has no parent, so browse compares it against the + // empty root and every table reads as added. + diffs[c.Hash] = initialDiff(c.Hash, tables) + continue + } + diffs[c.Hash] = &browse.CommitDiff{ + Hash: c.Hash, + Tables: []browse.TableDiff{{Name: "notes", RowsAdded: 2, RowsModified: 1}}, + } + } + + return &fakeSession{ + branches: []browse.Branch{{Name: "main", Head: "aaaa"}, {Name: "wip", Head: "c001"}}, + commits: commits, + tables: tables, + diffs: diffs, + } +} + +// smallStore is a database of one branch and one commit — the initial one, so +// its diff has every table added — over the tables given. +func smallStore(branch, head string, tables []fakeTable) *fakeSession { + return &fakeSession{ + branches: []browse.Branch{{Name: branch, Head: head}}, + commits: []browse.CommitInfo{{ + Hash: head, Author: "alice", Date: headTime, Message: "initial commit", + }}, + tables: tables, + diffs: map[string]*browse.CommitDiff{head: initialDiff(head, tables)}, + } +} + +// initialDiff is what browse.CommitSummary answers for a commit with no parent: +// it compares against the empty root, so every table shows as added with all of +// its rows. The rule is browse/'s and is tested there; this is a fixture that +// reproduces it, not a second implementation of it. +func initialDiff(hash string, tables []fakeTable) *browse.CommitDiff { + d := &browse.CommitDiff{Hash: hash} + for _, t := range tables { + d.Tables = append(d.Tables, browse.TableDiff{ + Name: t.name, + Added: true, + RowsAdded: int64(len(t.rows)), + }) + } + return d +} + +// fakeTable is one table of a fixture store: the schema browse would report and +// the rows behind it, kept consistent by construction — a test that compares a +// reported row count against the rows it can read cannot be satisfied by a fake +// that disagrees with itself. +type fakeTable struct { + name string + cols []browse.ColumnInfo + rows [][]string +} + +func (t fakeTable) info() browse.TableInfo { + return browse.TableInfo{Name: t.name, Columns: t.cols, RowCount: uint64(len(t.rows))} +} + +// columns is the display order browse.Rows produces: primary key first, then the +// rest. The fixture schemas below declare their columns in that order already. +func (t fakeTable) columns() []string { + out := make([]string, 0, len(t.cols)) + for _, c := range t.cols { + out = append(out, c.Name) + } + return out +} + +// newTable builds a table of n rows over a two-column keyed schema. +func newTable(name string, n int) fakeTable { + rows := make([][]string, n) + for i := range rows { + rows[i] = []string{strconv.Itoa(i), fmt.Sprintf("%s row %d", name, i)} + } + return fakeTable{ + name: name, + cols: []browse.ColumnInfo{ + {Name: "id", Type: "int", PrimaryKey: true}, + {Name: "body", Type: "text", Nullable: true}, + }, + rows: rows, + } +} + // beadsTables is the minimum beads.Applies accepts: issues + dependencies, with // issues carrying id and status. The fingerprint is beads/'s and is not restated // here — this is a fixture that satisfies it, not a second copy of it. -func beadsTables() []browse.TableInfo { - return []browse.TableInfo{ - {Name: "issues", Columns: []browse.ColumnInfo{{Name: "id"}, {Name: "status"}, {Name: "title"}}}, - {Name: "dependencies", Columns: []browse.ColumnInfo{{Name: "from_id"}, {Name: "to_id"}}}, +func beadsTables() []fakeTable { + return []fakeTable{ + { + name: "issues", + cols: []browse.ColumnInfo{{Name: "id"}, {Name: "status"}, {Name: "title"}}, + rows: [][]string{{"bd-1", "open", "first"}, {"bd-2", "closed", "second"}}, + }, + { + name: "dependencies", + cols: []browse.ColumnInfo{{Name: "from_id"}, {Name: "to_id"}}, + rows: [][]string{{"bd-2", "bd-1"}}, + }, } } @@ -287,17 +407,24 @@ } return sess, nil } -// fakeSession is one bare store as browse/ reads it. Only the three methods -// list_databases calls answer; the rest are the seam's, waiting for the tools of -// ch. 9.1, and a handler that reached one here would fail loudly rather than -// read an empty result. +// fakeSession is one bare store as browse/ reads it: a branch list, a linear +// history newest-first, tables with rows, and one recorded diff per commit. +// +// It reproduces browse/'s *contract* rather than its implementation — a ref is a +// branch name or a commit hash, an unknown one is ErrRefNotFound, an unknown +// table is ErrTableNotFound, a page past the end is empty with the true total — +// because those are the behaviours the tools are written against. Anything it +// cannot answer fails loudly rather than returning an empty result that would +// read as an answer. type fakeSession struct { branches []browse.Branch - commits []browse.CommitInfo - tables []browse.TableInfo + commits []browse.CommitInfo // newest first, as browse.Log walks them + tables []fakeTable + diffs map[string]*browse.CommitDiff logErr error tablesErr error + rowsErr error closes int } @@ -306,35 +433,130 @@ var _ mcpsrv.BrowseSession = (*fakeSession)(nil) func (s *fakeSession) Branches(context.Context) ([]browse.Branch, error) { return s.branches, nil } -func (s *fakeSession) Log(_ context.Context, _, _ string, _ int) ([]browse.CommitInfo, string, error) { +// Log walks the history from ref's head, or from a cursor, and reports the hash +// of the commit after the page — which is how browse says "there is more". +func (s *fakeSession) Log(_ context.Context, refStr, fromHash string, limit int) ([]browse.CommitInfo, string, error) { if s.logErr != nil { return nil, "", s.logErr } - return s.commits, "", nil + if limit <= 0 { + // browse.Log's own guard. The tools cap and default before they call, so + // reaching this is a bug in the tool rather than a caller's argument. + return nil, "", fmt.Errorf("fake: log limit must be positive, got %d", limit) + } + + start := -1 + if fromHash != "" { + // browse has no sentinel for a cursor that does not parse, and neither has + // this: a hand-written cursor is not a miss it classifies. + if start = s.indexOf(fromHash); start < 0 { + return nil, "", fmt.Errorf("fake: invalid from hash %q", fromHash) + } + } else { + start = s.indexOf(s.resolve(refStr)) + } + if start < 0 { + return nil, "", fmt.Errorf("%w: %s", browse.ErrRefNotFound, refStr) + } + + end, next := start+limit, "" + if end < len(s.commits) { + next = s.commits[end].Hash + } else { + end = len(s.commits) + } + return append([]browse.CommitInfo(nil), s.commits[start:end]...), next, nil } -func (s *fakeSession) Tables(context.Context, string) ([]browse.TableInfo, error) { +func (s *fakeSession) Tables(_ context.Context, refStr string) ([]browse.TableInfo, error) { if s.tablesErr != nil { return nil, s.tablesErr } - return s.tables, nil + if s.resolve(refStr) == "" { + return nil, fmt.Errorf("%w: %s", browse.ErrRefNotFound, refStr) + } + out := make([]browse.TableInfo, 0, len(s.tables)) + for _, t := range s.tables { + out = append(out, t.info()) + } + return out, nil } func (s *fakeSession) TableHash(context.Context, string, string) (string, bool, error) { panic("TableHash: no tool of this phase reads a table hash") } -func (s *fakeSession) Rows(context.Context, string, string, int, int) (*browse.RowPage, error) { - panic("Rows: no tool of this phase reads rows") +func (s *fakeSession) Rows(_ context.Context, refStr, table string, offset, limit int) (*browse.RowPage, error) { + if s.rowsErr != nil { + return nil, s.rowsErr + } + if s.resolve(refStr) == "" { + return nil, fmt.Errorf("%w: %s", browse.ErrRefNotFound, refStr) + } + if offset < 0 || limit <= 0 { + return nil, fmt.Errorf("fake: bad page offset=%d limit=%d", offset, limit) + } + + for _, t := range s.tables { + if t.name != table { + continue + } + page := &browse.RowPage{ + Columns: t.columns(), + Rows: [][]string{}, + Offset: offset, + Total: len(t.rows), + } + if offset < len(t.rows) { + end := min(offset+limit, len(t.rows)) + page.Rows = append(page.Rows, t.rows[offset:end]...) + } + return page, nil + } + return nil, fmt.Errorf("%w: %s", browse.ErrTableNotFound, table) } -func (s *fakeSession) CommitSummary(context.Context, string) (*browse.CommitDiff, error) { - panic("CommitSummary: no tool of this phase reads a diff") +func (s *fakeSession) CommitSummary(_ context.Context, hashStr string) (*browse.CommitDiff, error) { + hash := s.resolve(hashStr) + if hash == "" { + return nil, fmt.Errorf("%w: %s", browse.ErrRefNotFound, hashStr) + } + diff, ok := s.diffs[hash] + if !ok { + // A commit that exists always has a summary in browse; a fixture missing + // one is a gap in the fixture, and saying so beats answering "no tables + // changed". + return nil, fmt.Errorf("fake: no recorded diff for %s", hash) + } + return diff, nil } func (s *fakeSession) Close() error { s.closes++ return nil +} + +// resolve maps a ref — a branch name or a commit hash, which is what +// browse.resolveCommit accepts — to a commit hash, or "" when it is neither. +func (s *fakeSession) resolve(refStr string) string { + for _, b := range s.branches { + if b.Name == refStr { + return b.Head + } + } + if s.indexOf(refStr) >= 0 { + return refStr + } + return "" +} + +func (s *fakeSession) indexOf(hash string) int { + for i, c := range s.commits { + if c.Hash == hash { + return i + } + } + return -1 } // --- callers ---------------------------------------------------------------- @@ -528,7 +750,7 @@ } // --- the tool --------------------------------------------------------------- -func TestServerAdvertisesTheToolsOfThisPhase(t *testing.T) { +func TestServerAdvertisesTheToolsOfTheseChapters(t *testing.T) { session := connect(t, newServer(t, newFakeRepos(), newFakeOpener()), nil) require.Equal(t, mcpsrv.ServerName, session.InitializeResult().ServerInfo.Name) @@ -544,8 +766,17 @@ require.NotNil(t, tool.Annotations, "%s: every tool here is a read and must say so", tool.Name) assert.True(t, tool.Annotations.ReadOnlyHint, "%s", tool.Name) require.NotNil(t, tool.InputSchema, "%s: the schema is derived from the Go struct", tool.Name) } - assert.Equal(t, []string{"list_databases"}, got, - "this phase registers exactly one tool; the rest arrive with their own commits") + // The generic surface of docs/DESIGN.mcp.md §9.1, whole. The beads-aware + // tools of §9.2 arrive with their own commit, and this list is where their + // absence — or an accidental extra — is visible. + assert.ElementsMatch(t, []string{ + "list_databases", + "list_branches", + "list_tables", + "read_rows", + "get_commit_log", + "get_commit_diff", + }, got) } func TestListDatabasesDescribesADatabase(t *testing.T) { diff --git a/mcpsrv/read.go b/mcpsrv/read.go index 53c3fc6cb24e4ebaa35ce5ed9e7d583ec23ca37a..eafb1de8037a67869698ab4091895287f91e364e 100644 --- a/mcpsrv/read.go +++ b/mcpsrv/read.go @@ -119,21 +119,29 @@ // because the difference is the operator's business (it is in the log) and not // the agent's: nothing an agent can do about a broken store differs by cause. const contentUnreadable = "this database's store could not be read on the server; its metadata below is still accurate" +// readOnlyTool is the annotation every tool of this surface carries, and it is +// one shared value rather than one per registration so that "every tool here is +// a read" is a property of the package instead of a habit five call sites are +// keeping up. It tells a client it may retry a call freely; this surface has no +// write tool at all, and ports.go is why it cannot grow one by accident. +var readOnlyTool = &mcp.ToolAnnotations{ReadOnlyHint: true, IdempotentHint: true} + // register installs the tools on the protocol server. // -// Every one is annotated read-only and idempotent, which is true of all of them -// and is what tells a client it may retry a call freely: this surface has no -// write tool at all, and ports.go is why it cannot grow one by accident. +// The whole surface is registered from this one function — the chapters live in +// their own files, but what an agent is offered is listed in a single place, so +// that reading it answers "what can be called here" completely. // // The descriptions are the agent-facing documentation of this service and are // written for a reader who has never seen the design document — what the tool // answers, how to address what it answers about, and what it cannot answer. func (s *Server) register() { - readOnly := &mcp.ToolAnnotations{ReadOnlyHint: true, IdempotentHint: true} + // The generic tools of docs/DESIGN.mcp.md §9.1, over the browse seam. + s.registerBrowseTools() mcp.AddTool(s.mcp, &mcp.Tool{ Name: "list_databases", - Annotations: readOnly, + Annotations: readOnlyTool, Description: "List hosted Dolt databases with their visibility, description, default branch and " + "head commit, and whether each one is a beads issue tracker.\n\n" + "Pass `owner` — a SourceHut username without the \"~\" — to list that user's databases. " +

File	Status	+	−
`mcpsrv/browse.go`	A	+753
`mcpsrv/browse_test.go`	A	+759
`mcpsrv/errors.go`	M	+22
`mcpsrv/mcpsrv_test.go`	M	+272	-41
`mcpsrv/read.go`	M	+13	-5

~bigbes/sr-ht-dolt: 7a77409c

5 changed file(s)

~bigbes/sr-ht-dolt: `7a77409c`