From 4038ec2b4469113613ecf777e40a097bd8f09da2 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Thu, 13 Aug 2026 09:41:38 +0300 Subject: [PATCH] mcpsrv: answer what is ready across every tracker --- mcpsrv/beads.go | 28 +- mcpsrv/mcpsrv.go | 15 +- mcpsrv/mcpsrv_test.go | 46 ++- mcpsrv/ports.go | 12 + mcpsrv/read.go | 6 + mcpsrv/ready.go | 460 +++++++++++++++++++++++++ mcpsrv/ready_test.go | 755 ++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 1310 insertions(+), 12 deletions(-) create mode 100644 mcpsrv/ready.go create mode 100644 mcpsrv/ready_test.go diff --git a/mcpsrv/beads.go b/mcpsrv/beads.go index 195e34a560f56269b4d17d5c3d519b0f9b4f3cc2..3260f10b81e5e39f362219fdbf5478c294b6ca20 100644 --- a/mcpsrv/beads.go +++ b/mcpsrv/beads.go @@ -11,6 +11,7 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" "sourcecraft.dev/bigbes/sr-ht-dolt/beads" + "sourcecraft.dev/bigbes/sr-ht-dolt/core" ) // The beads-aware tools of docs/DESIGN.mcp.md §9.2: a hosted database that @@ -850,25 +851,40 @@ func (s *Server) listMemories(ctx context.Context, in listMemoriesInput) (listMe // discipline; returning it rather than a closure keeps that visible at the call // site instead of hidden in a helper. func (s *Server) openTracker(ctx context.Context, tool string, ref databaseRef, named string) (BrowseSession, string, error) { + _, sess, at, err := s.openTrackerFor(ctx, tool, ref, named) + return sess, at, err +} + +// openTrackerFor is openTracker plus the repository row it resolved. +// +// The row is what a tool needs when it addresses a database by something other +// than its {owner, name} — ready_work keys the shared projection cache on the +// repository id, which is the identity no two databases share and which survives +// a rename (beads.ReadyDatabase). Every other tool here wants only the session +// and the ref, and openTracker above is that same call with the row dropped: +// one preamble, so that the refusals cannot drift apart between tools. +func (s *Server) openTrackerFor(ctx context.Context, tool string, ref databaseRef, named string) ( + *core.Repo, BrowseSession, string, error, +) { repo, err := s.resolveDatabase(ctx, tool, ref) if err != nil { - return nil, "", err + return nil, nil, "", err } sess, err := s.openStore(ctx, tool, repo) if err != nil { - return nil, "", err + return nil, nil, "", err } at, err := refFor(ctx, sess, tool, ref, named) if err != nil { sess.Close() - return nil, "", err + return nil, nil, "", err } tables, err := sess.Tables(ctx, at) if err != nil { sess.Close() - return nil, "", refMiss(err, tool, noSuchRef(ref, at)) + return nil, nil, "", refMiss(err, tool, noSuchRef(ref, at)) } // The fingerprint is beads.Applies and is asked here, per call, because MCP's // tool list is static per server: these tools are advertised for every @@ -876,9 +892,9 @@ func (s *Server) openTracker(ctx context.Context, tool string, ref databaseRef, // argument rather than about the surface (docs/DESIGN.mcp.md §9). if !beads.Applies(tables) { sess.Close() - return nil, "", errors.New(notATracker(ref, at)) + return nil, nil, "", errors.New(notATracker(ref, at)) } - return sess, at, nil + return repo, sess, at, nil } // boardQuery renders the tool's filter as the query the board projection parses, diff --git a/mcpsrv/mcpsrv.go b/mcpsrv/mcpsrv.go index 2a5c6a8af4091a7a314560e626eaafdfc6924ef9..fd86daa491865b2d1e1c239489596471446bc892 100644 --- a/mcpsrv/mcpsrv.go +++ b/mcpsrv/mcpsrv.go @@ -62,6 +62,7 @@ import ( "sourcecraft.dev/bigbes/sr-ht-ecore/instconf" "sourcecraft.dev/bigbes/sr-ht-dolt/authn" + "sourcecraft.dev/bigbes/sr-ht-dolt/beads" "sourcecraft.dev/bigbes/sr-ht-dolt/core" ) @@ -166,6 +167,18 @@ type Server struct { // (Connect). mcp *mcp.Server + // ready is ready_work's projection cache: one ready set per database, gated + // on that database's head hash and expiring on beads.ReadyCacheTTL. It is + // the one piece of state that outlives a call here, and it is built once, per + // server — a cache created per call is not a cache, and the head-hash gate it + // exists to enforce would never fire. + // + // It holds projections and never an open store, which is what makes holding + // it across calls compatible with opening a session per call: an open store + // is a file handle and a memory mapping, and that is precisely what the + // per-call discipline exists not to hoard (beads.ReadyCache). + ready *beads.ReadyCache + // http is the whole handler chain. It is built once, in New, because // mcp.NewStreamableHTTPHandler owns transport state and two of them would be // two servers. @@ -205,7 +218,7 @@ func New(repos Repos, opener BrowseOpener, validator authn.InstanceValidator, or return nil, culpa.Errorf("mcpsrv: origin %q has no host to guard /mcp with", origin) } - s := &Server{repos: repos, opener: opener, validator: validator} + s := &Server{repos: repos, opener: opener, validator: validator, ready: beads.NewReadyCache()} s.mcp = mcp.NewServer(&mcp.Implementation{Name: ServerName, Version: serverVersion()}, nil) s.register() diff --git a/mcpsrv/mcpsrv_test.go b/mcpsrv/mcpsrv_test.go index 1b56ad1ffa50025256b41538883f510d2797f37b..4a9568cf0294edf08f406230ee2e086d0080ca86 100644 --- a/mcpsrv/mcpsrv_test.go +++ b/mcpsrv/mcpsrv_test.go @@ -352,6 +352,32 @@ func (f *fakeRepos) ListReposByOwner(_ context.Context, owner string, viewer *co return out, nil } +// ListReposForViewer mirrors db.Store's instance-wide listing rule: it is +// ListReposByOwner's rule minus the owner filter, so PUBLIC is listed to +// everyone including anonymity and everything else only to its owner and its +// grantees. +// +// It lists in the order the fixtures were added, where db/ lists newest first. +// That difference is deliberate and harmless: what the aggregation's ceiling +// takes is the first N of whatever order this returns, and a fixture written in +// the order it is read is one a test can reason about. +func (f *fakeRepos) ListReposForViewer(_ context.Context, viewer *core.Caller) ([]*core.Repo, error) { + if f.listErr != nil { + return nil, f.listErr + } + var out []*core.Repo + for _, r := range f.repos { + visible := r.Visibility == core.VisibilityPublic + if viewer != nil && (viewer.UserID == r.OwnerID || f.hasACL(r.ID, viewer.UserID)) { + visible = true + } + if visible { + out = append(out, r) + } + } + return out, nil +} + func (f *fakeRepos) ListReposForDashboard(_ context.Context, userID int) ([]*core.Repo, error) { if f.listErr != nil { return nil, f.listErr @@ -445,6 +471,14 @@ type fakeSession struct { tablesErr error rowsErr error + // The read counters, one per kind of read a session serves. They are what + // makes a cache assertion a measurement rather than a stopwatch: the + // head-hash gate of the cross-database ready set is "the second call reads no + // rows", and that is counted here and never timed. + rowReads int + tableReads int + logReads int + closes int } @@ -455,6 +489,7 @@ func (s *fakeSession) Branches(context.Context) ([]browse.Branch, error) { retur // 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) { + s.logReads++ if s.logErr != nil { return nil, "", s.logErr } @@ -488,6 +523,7 @@ func (s *fakeSession) Log(_ context.Context, refStr, fromHash string, limit int) } func (s *fakeSession) Tables(_ context.Context, refStr string) ([]browse.TableInfo, error) { + s.tableReads++ if s.tablesErr != nil { return nil, s.tablesErr } @@ -521,6 +557,7 @@ func (s *fakeSession) TableHash(_ context.Context, refStr, table string) (string } func (s *fakeSession) Rows(_ context.Context, refStr, table string, offset, limit int) (*browse.RowPage, error) { + s.rowReads++ if s.rowsErr != nil { return nil, s.rowsErr } @@ -818,11 +855,9 @@ func TestServerAdvertisesTheToolsOfTheseChapters(t *testing.T) { require.NotNil(t, tool.InputSchema, "%s: the schema is derived from the Go struct", tool.Name) } // The generic surface of docs/DESIGN.mcp.md §9.1 and the beads-aware tools of - // §9.2, whole. This list is where an accidental extra — or a tool that - // quietly stopped being registered — is visible. - // - // §9.2's ready_work is deliberately absent: the cross-database aggregation - // arrives with its own commit (§11, phase 7). + // §9.2, whole — ready_work included, which completes the chapter. This list + // is where an accidental extra — or a tool that quietly stopped being + // registered — is visible. assert.ElementsMatch(t, []string{ "list_databases", "list_branches", @@ -834,6 +869,7 @@ func TestServerAdvertisesTheToolsOfTheseChapters(t *testing.T) { "get_issue", "list_milestones", "list_memories", + "ready_work", }, got) } diff --git a/mcpsrv/ports.go b/mcpsrv/ports.go index 100dcbba34db6875360cf717219dced5ad32c37b..9f4a825ff221ac52730913b96b3e07dd5abbfc73 100644 --- a/mcpsrv/ports.go +++ b/mcpsrv/ports.go @@ -69,6 +69,18 @@ type Repos interface { // the instance and takes no viewer argument because the user *is* the viewer. ListReposForDashboard(ctx context.Context, userID int) ([]*core.Repo, error) + // ListReposForViewer lists every database viewer may be shown, across all + // owners. It is ListReposByOwner's rule minus the owner filter, and it is the + // only enumeration on this seam that can answer an instance-wide question — + // which is what ready_work with no database named asks (docs/DESIGN.mcp.md + // §9.2, docs/DESIGN.views.md ch. 4). + // + // Listing is not authorization: the caller here still asks core.Allowed for + // OpBrowse per database, exactly as web/handlers_ready.go does, before a + // single store is opened. viewer is nil for an anonymous caller, and + // anonymous is a normal caller — it gets the PUBLIC set. + ListReposForViewer(ctx context.Context, viewer *core.Caller) ([]*core.Repo, error) + // EffectiveAccess resolves the caller's ACL grant on a repository, or // (nil, nil) when there is none. Feed the result to core.Allowed; a nil grant // is not a denial, it is a fall-through to visibility. diff --git a/mcpsrv/read.go b/mcpsrv/read.go index 2d3ed5ba82730fe75a731667fb84f9efe8f30183..afd2e8c9e2ea47de08680c897d1d6895508ba816 100644 --- a/mcpsrv/read.go +++ b/mcpsrv/read.go @@ -145,6 +145,12 @@ func (s *Server) register() { // registering them is unconditional here. s.registerBeadsTools() + // The last of §9.2, in a file of its own because it is the one tool here that + // answers about a *set* of databases: ready_work with no database named is + // the cross-database aggregation of docs/DESIGN.views.md ch. 4, the same + // function the /ready page renders. + s.registerReadyWork() + mcp.AddTool(s.mcp, &mcp.Tool{ Name: "list_databases", Annotations: readOnlyTool, diff --git a/mcpsrv/ready.go b/mcpsrv/ready.go new file mode 100644 index 0000000000000000000000000000000000000000..64e0cf67a85a4e79db7fe9f7bae15564b21e9f27 --- /dev/null +++ b/mcpsrv/ready.go @@ -0,0 +1,460 @@ +package mcpsrv + +import ( + "context" + "errors" + "fmt" + "log/slog" + "net/url" + "strings" + "time" + + "github.com/modelcontextprotocol/go-sdk/mcp" + "go.bigb.es/auxilia/scribe" + + "sourcecraft.dev/bigbes/sr-ht-dolt/beads" + "sourcecraft.dev/bigbes/sr-ht-dolt/core" +) + +// ready_work, the last tool of docs/DESIGN.mcp.md §9.2 and the only one on this +// surface that answers about more than one database at a time. +// +// "What is ready to work" is answerable inside each beads tracker on this +// instance and, without this, nowhere across them — which is the question the +// split into a global tracker plus per-project trackers was supposed to make +// askable. The /ready page (docs/DESIGN.views.md ch. 4) answers it for a human; +// this answers it for an agent. +// +// # One aggregator, two renderings +// +// The set itself is beads.ReadyAcross's, unchanged and un-re-read: the same +// function the page calls, over the same projection cache shape, with the same +// three bounds (the head-hash gate, the TTL, the ceiling of +// beads.ReadyMaxDatabases). Nothing about the ready rule, the grouping or the +// ordering is restated here — two implementations would answer differently the +// first time either moved, and the whole point of this tool is that the page and +// the agent agree. +// +// What this file owns is the other half, which is exactly what beads/ refuses to +// know: which databases this caller may see at all. That is ListReposForViewer +// (the listing rule) followed by core.Allowed/OpBrowse per database (the access +// rule), applied before a single store is opened — the same two steps +// web/handlers_ready.go takes. +// +// # Two arms, one answer +// +// With a database named the tool answers about that one, resolved through the +// preamble every beads tool shares (openTrackerFor): the masked not-found of a +// database the caller may not read, the "not a beads tracker" refusal, the +// default branch. With none named it answers across every tracker the caller may +// see. Both arms then run through ReadyAcross with the same filter and the same +// cache, so the two cannot disagree about one database: the named arm is the +// cross arm restricted to a single candidate. + +// readyWorkInput addresses the tool's two arms and carries the page's own +// filters. +// +// owner and name are optional *together*: naming neither is the cross-database +// arm, naming both is one database, and naming one of the two is a call that +// means nothing and is refused rather than guessed at. +// +// There is no ref argument, and that is not an omission. Across databases there +// is no ref to name — a branch of one tracker says nothing about another — so +// every database is read at its own default branch, exactly as the page reads +// it. A tracker's other branches are list_issues' business. +type readyWorkInput struct { + Owner string `json:"owner,omitempty" jsonschema:"the database owner's SourceHut username, without the \"~\" (a leading one is accepted). Omit it — together with name — to read every tracker you can see."` + Name string `json:"name,omitempty" jsonschema:"the database name, as list_databases reports it. Omit it — together with owner — to read every tracker you can see."` + + Query string `json:"q,omitempty" jsonschema:"a case-insensitive substring of an issue's id or title; it does not search bodies"` + Assignee string `json:"assignee,omitempty" jsonschema:"an exact assignee"` + Priority string `json:"priority,omitempty" jsonschema:"an exact priority as stored: \"0\" (highest) through \"3\""` + + Limit *int `json:"limit,omitempty" jsonschema:"how many ready issues to return across all databases, at most 500; defaults to 200. It is applied after filtering."` +} + +// readyCardJSON is one ready issue. +// +// It is issueCardJSON minus the two fields that would be constants here: every +// card in this answer is ready and every one of them is in the open status +// category — that is what the ready rule selects — and a field whose value is +// fixed by the tool's own name tells a caller nothing. There is no lane either: +// the projection this reads produces the ready set, not the board's bucketing. +// +// No bodies, for the reason §9.2 gives once: a listing carries identity and +// metadata, and get_issue carries the prose, one issue at a time. +type readyCardJSON struct { + ID string `json:"id"` + Title string `json:"title"` + + Type string `json:"type"` + Priority string `json:"priority"` + Assignee string `json:"assignee"` + Labels []string `json:"labels"` + + // BlockedBy is how many dependencies this issue has and Blocks how many point + // at it. A ready issue can still have dependencies — a closed blocker is a + // dependency that does not block — so BlockedBy is not always zero, and that + // is worth seeing. + BlockedBy int `json:"blocked_by"` + Blocks int `json:"blocks"` +} + +// readyHeadJSON is the head commit of the branch a database's ready set was read +// from. +// +// It is carried per database and not per answer because it is the one fact that +// makes the ready set checkable: a tracker that stopped receiving pushes still +// has a ready set, and reporting it without saying how old the data is would be +// exactly the claim this tool must not make silently. +type readyHeadJSON struct { + Hash string `json:"hash"` + Time time.Time `json:"time"` +} + +// readyDatabaseJSON is one tracker's ready work. +type readyDatabaseJSON struct { + // Owner and Name are the address every other tool on this surface takes, so + // a caller can go straight from a card here to get_issue there. + Owner string `json:"owner"` + Name string `json:"name"` + + // Ref is the branch this database was read at — its own default branch. + Ref string `json:"ref"` + + // Head is that branch's head commit, or null when the log could not be read. + // Null is not a claim that the tracker is empty: the ready set beside it was + // read at that very branch. + Head *readyHeadJSON `json:"head"` + + // Ready is this database's ready issues, ordered by priority (0 first) then + // id. + Ready []readyCardJSON `json:"ready"` + + // Count is how many ready issues this database has after filtering, which is + // not always how many are carried above: the answer's limit is spent across + // databases, and this is the honest denominator per database. + Count int `json:"count"` +} + +// readyUnreadableJSON names a database that could not be read. +// +// It names it and nothing else: the underlying failure carries on-disk paths and +// dolt internals, and it goes to the log (sr-ht-dolt-7ta). The address is not a +// disclosure — it is a database this caller was already allowed to list. +type readyUnreadableJSON struct { + Owner string `json:"owner"` + Name string `json:"name"` +} + +type readyWorkOutput struct { + // Databases carry the ready work, ordered by ready count descending then by + // address — the page's own order. A database with nothing ready is absent + // rather than present and empty. + Databases []readyDatabaseJSON `json:"databases"` + + // Total is every ready issue that matched, across every database, before + // Limit clipped the list — the honest denominator of the answer. + Total int `json:"total"` + + // 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. + Limit int `json:"limit"` + + // Truncated reports that matches were left behind by Limit. Narrow with q, + // assignee or priority, or name one database. + Truncated bool `json:"truncated"` + + // Considered is how many databases were actually read (or served from the + // cache), and MaxDatabases is the ceiling one call may open. + Considered int `json:"considered"` + MaxDatabases int `json:"max_databases"` + + // Capped says there were more candidate databases than MaxDatabases and only + // the first of them were read. A silent cap reads as "that is everything", + // which is the one thing an answer about a *set* must never imply. + Capped bool `json:"capped"` + + // Unreadable names the databases whose stores this service could not read. + // They are part of the answer rather than a silent omission for Capped's + // reason: the ready set below is complete only for the databases that are not + // in this list. + Unreadable []readyUnreadableJSON `json:"unreadable"` +} + +// borrowedSession lends an already-open session to the aggregation without +// handing over its lifetime. +// +// beads.ReadyAcross closes every session its opener produced, which is right +// when the opener opened it. In the named arm the session was opened by the +// handler — by the same preamble that resolved the database and checked the +// fingerprint — and is closed by that handler's own defer, so the borrowed copy +// must not close it a second time. A Close is not documented as idempotent +// anywhere in this service, and a double close is the kind of thing that works +// on a fake and corrupts a handle in production. +type borrowedSession struct{ BrowseSession } + +func (borrowedSession) Close() error { return nil } + +// --- registration ----------------------------------------------------------- + +// registerReadyWork installs ready_work (docs/DESIGN.mcp.md §9.2, the +// cross-database row). +func (s *Server) registerReadyWork() { + mcp.AddTool(s.mcp, &mcp.Tool{ + Name: "ready_work", + Annotations: readOnlyTool, + Description: "Answer \"what can be picked up right now\" — bd's ready set: issues that are open, " + + "unblocked and not templates or scaffolding.\n\n" + + "**Omit `owner` and `name` and it answers across every beads tracker you can see**, grouped " + + "by database, busiest first. That is what this tool is for: an instance holds one tracker per " + + "project plus a global one, and this is the only way to ask all of them at once. Name both to " + + "ask one tracker.\n\n" + + "Each database carries its own `ref` and `head` — the branch the set was read from and that " + + "branch's head commit with its time. Read them: a tracker that stopped receiving pushes still " + + "has a ready set, and its age is the only thing that says so.\n\n" + + "`q`, `assignee` and `priority` narrow the issues; `limit` (default 200, cap 500) is spent " + + "across all databases after filtering, `total` is how many matched, and `truncated` says some " + + "were left behind. Each database's `count` is its own match total.\n\n" + + "Two facts keep the answer honest about being a set. `capped` says there were more trackers " + + "than `max_databases` and only the first were read. `unreadable` names the databases whose " + + "stores could not be read at all — the set is complete only for the databases not in it.\n\n" + + "Cards carry no issue bodies; call get_issue with a card's `owner`, `name` and `id` for the " + + "description, design and acceptance criteria. A database you name that is not a beads tracker " + + "says so; one you may not read is reported as not existing.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in readyWorkInput) (*mcp.CallToolResult, readyWorkOutput, error) { + out, err := s.readyWork(ctx, in) + return nil, out, err + }) +} + +// --- the handler ------------------------------------------------------------ + +// readyWork answers ready_work: the ready set of one named tracker, or of every +// tracker the caller may see. +// +// The clock is real and is passed into the aggregation rather than read inside +// it, exactly as listMemories passes its own: the TTL is the one thing about +// this answer that depends on when it was computed, and beads/ reads no hidden +// clock. +func (s *Server) readyWork(ctx context.Context, in readyWorkInput) (readyWorkOutput, error) { + const tool = "ready_work" + var out readyWorkOutput + + limit, err := pageLimit(in.Limit, defaultIssueLimit, maxIssueLimit, "issues") + if err != nil { + return out, err + } + // The filter is parsed by beads rather than assembled here, for boardQuery's + // reason: the substring rule and the trimming are the page's, not a second + // implementation of them reading the same cards. + filter := beads.ParseReadyFilter(readyQuery(in)) + + ref := databaseRef{Owner: in.Owner, Name: in.Name} + named := ref.owner() != "" || ref.name() != "" + + var ( + dbs []beads.ReadyDatabase + open beads.ReadyOpener + ) + if named { + var sess BrowseSession + dbs, open, sess, err = s.readyOne(ctx, tool, ref) + if err != nil { + return out, err + } + defer sess.Close() + } else if dbs, open, err = s.readyAll(ctx, tool); err != nil { + return out, err + } + + view := beads.ReadyAcross(ctx, dbs, open, s.ready, filter, time.Now()) + + // A store that cannot be read is a fact about this deployment and belongs in + // the log with its cause; the answer names the database and not the failure. + // + // In the named arm it is not a partial answer at all: that database is the + // whole subject of the call, its ref resolved and its fingerprint matched a + // moment ago, so what failed afterwards is a table this service could not + // read. Reporting an empty ready set there would be a false statement about + // the tracker rather than a true one about the server. + for _, f := range view.Failed { + slog.Warn("reading a database for ready_work failed", + "tool", tool, "database", f.Database.Slug(), scribe.Err(f.Err)) + } + if named && len(view.Failed) > 0 { + return out, internalError(view.Failed[0].Err, tool) + } + + out = readyWorkOutput{ + Databases: make([]readyDatabaseJSON, 0, len(view.Groups)), + Total: view.Total, + Limit: limit, + Considered: view.Considered, + MaxDatabases: view.Max, + Capped: view.Capped, + Unreadable: make([]readyUnreadableJSON, 0, len(view.Failed)), + } + for _, f := range view.Failed { + out.Unreadable = append(out.Unreadable, readyUnreadableJSON{ + Owner: f.Database.OwnerName, + Name: f.Database.Name, + }) + } + + // The limit is spent across databases in the order the aggregation produced + // them, so what a clipped answer carries is the busiest trackers' highest + // priorities rather than an arbitrary slice. A database left with nothing is + // absent rather than present and empty — the same rule the aggregation + // applies to a tracker with no ready work. + carried := 0 + for _, g := range view.Groups { + entry := readyDatabaseJSON{ + Owner: g.Database.OwnerName, + Name: g.Database.Name, + Ref: g.Ref, + Count: len(g.Cards), + Ready: make([]readyCardJSON, 0, len(g.Cards)), + } + if g.Head != nil { + entry.Head = &readyHeadJSON{Hash: g.Head.Hash, Time: g.Head.Date} + } + for _, c := range g.Cards { + if carried >= limit { + break + } + entry.Ready = append(entry.Ready, readyCardOf(c)) + carried++ + } + if len(entry.Ready) == 0 { + continue + } + out.Databases = append(out.Databases, entry) + } + out.Truncated = carried < out.Total + return out, nil +} + +// readyOne is the named arm's candidate list: one database, resolved and opened +// through the preamble every beads tool shares. +// +// Going through openTrackerFor is what makes this arm answer like the rest of +// the surface without restating any of it — the masked not-found, the "not a +// beads tracker" refusal, the default branch and the store that will not open +// are all decided there. The session it returns is the caller's to close. +func (s *Server) readyOne(ctx context.Context, tool string, ref databaseRef) ( + []beads.ReadyDatabase, beads.ReadyOpener, BrowseSession, error, +) { + if ref.owner() == "" || ref.name() == "" { + return nil, nil, nil, errors.New( + "address a database by both its owner and its name, as list_databases reports them — " + + "or name neither, and this answers across every tracker you can see") + } + + repo, sess, _, err := s.openTrackerFor(ctx, tool, ref, "") + if err != nil { + return nil, nil, nil, err + } + dbs := []beads.ReadyDatabase{{ID: repo.ID, OwnerName: repo.OwnerName, Name: repo.Name}} + open := func(context.Context, beads.ReadyDatabase) (beads.ReadySession, error) { + return borrowedSession{sess}, nil + } + return dbs, open, sess, nil +} + +// readyAll is the cross-database arm's candidate list: every database this +// caller may browse. +// +// It is two steps and they are not the same rule. ListReposForViewer applies the +// *listing* rule (PUBLIC to everyone including anonymity, plus whatever the +// caller owns or holds an ACL entry on), and core.Allowed then applies the +// *access* rule per database. A database the caller may not browse is simply +// absent — not a refusal, not a count, not a named group with hidden contents — +// and it is dropped here, before any store is opened, so its very existence +// costs nothing observable. +// +// An ACL lookup that fails takes the whole call with it rather than quietly +// narrowing the answer. "I could not check" is not "you may not", and an agent +// told that its tracker has no ready work believes it. +func (s *Server) readyAll(ctx context.Context, tool string) ([]beads.ReadyDatabase, beads.ReadyOpener, error) { + caller := callerOf(ctx) + + repos, err := s.repos.ListReposForViewer(ctx, caller) + if err != nil { + // No database was addressed, so there is no "that one does not exist" to + // answer: whatever went wrong enumerating them is this service's. + return nil, nil, internalError(err, tool) + } + + // The on-disk path never reaches beads: it is this service's arrangement of + // its own storage, and the aggregation addresses a database by the identity + // its cache is keyed on. The opener closes over this map, so a database that + // was filtered out above has no path to be opened by. + paths := make(map[int]string, len(repos)) + dbs := make([]beads.ReadyDatabase, 0, len(repos)) + for _, repo := range repos { + var mode *core.AccessMode + if caller != nil { + if mode, err = s.repos.EffectiveAccess(ctx, caller.UserID, repo.ID); err != nil { + return nil, nil, internalError(err, tool) + } + } + if !core.Allowed(caller, repo, mode, core.OpBrowse) { + continue + } + paths[repo.ID] = repo.Path + dbs = append(dbs, beads.ReadyDatabase{ + ID: repo.ID, + OwnerName: repo.OwnerName, + Name: repo.Name, + }) + } + + open := func(ctx context.Context, d beads.ReadyDatabase) (beads.ReadySession, error) { + path, ok := paths[d.ID] + if !ok { + return nil, fmt.Errorf("mcpsrv: no store path for database %s", d.Slug()) + } + return s.opener.Open(ctx, path) + } + return dbs, open, nil +} + +// readyQuery renders the tool's filters as the query the page's filter parser +// reads, so that what narrows this answer is beads.ReadyFilter and not a second +// implementation of it applied to the same cards. +// +// The parser's ?db= is deliberately not offered: naming databases is what owner +// and name already do, one at a time, through the resolution every other tool on +// this surface uses. +func readyQuery(in readyWorkInput) url.Values { + q := url.Values{} + set := func(key, value string) { + if value = strings.TrimSpace(value); value != "" { + q.Set(key, value) + } + } + set("q", in.Query) + set("assignee", in.Assignee) + set("priority", in.Priority) + return q +} + +// readyCardOf projects one ready card. Labels are an array rather than a null, +// so an agent can loop without a nil check. +func readyCardOf(c beads.Card) readyCardJSON { + labels := c.Labels + if labels == nil { + labels = []string{} + } + return readyCardJSON{ + ID: c.ID, + Title: c.Title, + Type: c.Type, + Priority: c.Priority, + Assignee: c.Assignee, + Labels: labels, + BlockedBy: c.BlockedBy, + Blocks: c.Blocks, + } +} diff --git a/mcpsrv/ready_test.go b/mcpsrv/ready_test.go new file mode 100644 index 0000000000000000000000000000000000000000..6c1c5935a48a1e5d4084ff4913e148e3c5e6e08b --- /dev/null +++ b/mcpsrv/ready_test.go @@ -0,0 +1,755 @@ +package mcpsrv_test + +import ( + "fmt" + "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/beads" + "sourcecraft.dev/bigbes/sr-ht-dolt/browse" + "sourcecraft.dev/bigbes/sr-ht-dolt/core" + "sourcecraft.dev/bigbes/sr-ht-dolt/mcpsrv" +) + +// ready_work: the one tool on this surface that answers about a set of +// databases rather than about one. +// +// What is under test here is not the ready rule — that is beads' and is tested +// there, and the /ready page reads it through the very same function — but the +// two things only this layer can get wrong: which databases a caller is shown, +// and that the two arms of the tool agree about one of them. +// +// The fixture set below is its own rather than the shared one, because every +// property here is a property of a *set*: several owners, one database of each +// visibility, one that is not a tracker at all and one whose store will not +// open. + +// --- the fixture ------------------------------------------------------------- + +const ( + daveID = 4 // owns the private tracker bob is granted on + erinID = 5 // owns the non-beads database and the broken one + frankID = 6 // owns the unlisted tracker +) + +// The two markers a leak would carry. They are strings no visible database +// holds, so "nothing about a database the caller may not list reached the +// answer" is a question asked of the serialised payload rather than of a struct. +const ( + privateMarker = "SECRETREADY" + unlistedMarker = "UNLISTEDREADY" +) + +// readyIssues builds an issues table from (id, title, status, priority, +// assignee, is_blocked) tuples, over the same column set the rest of the beads +// suite uses. +func readyIssues(rows [][6]string) fakeTable { + out := make([][]string, 0, len(rows)) + for _, r := range rows { + out = append(out, row(issueColumns, map[string]string{ + "id": r[0], "title": r[1], "status": r[2], "priority": r[3], + "assignee": r[4], "is_blocked": r[5], "issue_type": "task", + "created_at": "2026-07-01 10:00:00", + })) + } + return beadsTable("issues", issueColumns, out) +} + +// readyTracker is one beads database at head: a "main" branch, one commit, the +// issues given and the dependency edges between them. +func readyTracker(head string, issues fakeTable, deps [][]string) *fakeSession { + return &fakeSession{ + branches: []browse.Branch{{Name: "main", Head: head}}, + commits: []browse.CommitInfo{{ + Hash: head, Author: "alice", Date: headTime, Message: "bd: create (auto-commit)", + }}, + tables: []fakeTable{ + issues, + beadsTable("dependencies", []string{"issue_id", "depends_on_issue_id", "type"}, deps), + }, + } +} + +// readyFixture is one database of the ready fixture set, with its owner: unlike +// the shared fixtures, these belong to several people, which is what makes the +// listing rule say anything at all. +type readyFixture struct { + owner string + ownerID int + name string + vis core.Visibility + acl map[int]core.AccessMode + session *fakeSession // nil is a store that will not open +} + +func (f readyFixture) slug() string { return f.owner + "/" + f.name } + +// alphaStore is the busiest tracker: two of its five issues are ready. +// +// a-2 is the case worth naming — it depends on a-5, which is closed, so the +// dependency does not block and the card still carries the count. +func alphaStore() *fakeSession { + issues := readyIssues([][6]string{ + {"a-1", "Ready, top priority", "open", "0", "alice", ""}, + {"a-2", "Ready, lower", "open", "2", "bob", ""}, + {"a-3", "Under way", "in_progress", "0", "alice", ""}, + {"a-4", "Waiting on something", "open", "0", "bob", "1"}, + {"a-5", "Finished", "closed", "1", "bob", ""}, + }) + // One long text, so "a listing carries no bodies" is asked of this tool too. + issues.rows[0][indexOfColumn("description")] = body("a-1") + return readyTracker("h-alpha", issues, [][]string{{"a-2", "a-5", "blocks"}}) +} + +// indexOfColumn is where a named column sits in issueColumns. A name that is not +// there panics rather than writing into the wrong cell. +func indexOfColumn(name string) int { + for i, c := range issueColumns { + if c == name { + return i + } + } + panic("no issue column named " + name) +} + +func readyFixtures() []readyFixture { + return []readyFixture{ + {owner: "alice", ownerID: aliceID, name: "alpha", vis: core.VisibilityPublic, session: alphaStore()}, + { + owner: "bob", ownerID: bobID, name: "beta", vis: core.VisibilityPublic, + session: readyTracker("h-beta", readyIssues([][6]string{ + {"b-1", "The one ready thing", "open", "1", "carol", ""}, + {"b-2", "Finished", "closed", "0", "carol", ""}, + }), nil), + }, + { + // PRIVATE: absent from every listing but its owner's and its grantee's, + // and readable by neither anonymity nor a stranger. + owner: "dave", ownerID: daveID, name: "secrets", vis: core.VisibilityPrivate, + acl: map[int]core.AccessMode{bobID: core.AccessRO}, + session: readyTracker("h-secrets", readyIssues([][6]string{ + {"s-1", privateMarker + " rotate the escrow key", "open", "0", "dave", ""}, + }), nil), + }, + { + // UNLISTED: absent from the cross-database arm, which is a listing, and + // answered when named directly, which is an address. + owner: "frank", ownerID: frankID, name: "hidden", vis: core.VisibilityUnlisted, + session: readyTracker("h-hidden", readyIssues([][6]string{ + {"u-1", unlistedMarker + " the private draft", "open", "1", "frank", ""}, + }), nil), + }, + { + // Not a beads tracker at all: skipped silently, never read for rows. + owner: "erin", ownerID: erinID, name: "sensors", vis: core.VisibilityPublic, + session: &fakeSession{ + branches: []browse.Branch{{Name: "main", Head: "h-sensors"}}, + commits: []browse.CommitInfo{{ + Hash: "h-sensors", Author: "erin", Date: headTime, Message: "readings", + }}, + tables: []fakeTable{newTable("measurements", 3)}, + }, + }, + { + // A store this daemon cannot open: it costs itself and nothing else. + owner: "erin", ownerID: erinID, name: "broken", vis: core.VisibilityPublic, + session: nil, + }, + } +} + +// readyFakes builds the metadata store and the opener over a fixture set, in the +// order given — which is the order the listing returns and therefore the order +// the aggregation's ceiling takes its candidates in. +func readyFakes(fx []readyFixture) (*fakeRepos, *fakeOpener) { + repos := &fakeRepos{acl: map[int]map[int]core.AccessMode{}} + opener := &fakeOpener{sessions: map[string]*fakeSession{}} + for i, f := range fx { + id := i + 1 + repos.repos = append(repos.repos, &core.Repo{ + ID: id, + Name: f.name, + Description: "the " + f.name + " database", + OwnerID: f.ownerID, + OwnerName: f.owner, + Path: storePath(f.owner, f.name), + Visibility: f.vis, + }) + for userID, mode := range f.acl { + if repos.acl[id] == nil { + repos.acl[id] = map[int]core.AccessMode{} + } + repos.acl[id][userID] = mode + } + if f.session != nil { + opener.sessions[storePath(f.owner, f.name)] = f.session + } + } + return repos, opener +} + +// readyServer is the surface over the ready fixture set, with the fakes returned +// so a test can count what was opened and read. +func readyServer(t *testing.T) (*mcpsrv.Server, *fakeRepos, *fakeOpener) { + t.Helper() + repos, opener := readyFakes(readyFixtures()) + return newServer(t, repos, opener), repos, opener +} + +// --- the shapes a client decodes --------------------------------------------- + +type ( + readyCardResult struct { + ID string `json:"id"` + Title string `json:"title"` + Type string `json:"type"` + Priority string `json:"priority"` + Assignee string `json:"assignee"` + Labels []string `json:"labels"` + BlockedBy int `json:"blocked_by"` + Blocks int `json:"blocks"` + } + + readyHeadResult struct { + Hash string `json:"hash"` + Time time.Time `json:"time"` + } + + readyDatabaseResult struct { + Owner string `json:"owner"` + Name string `json:"name"` + Ref string `json:"ref"` + Head *readyHeadResult `json:"head"` + Ready []readyCardResult `json:"ready"` + Count int `json:"count"` + } + + readyUnreadableResult struct { + Owner string `json:"owner"` + Name string `json:"name"` + } + + readyWorkResult struct { + Databases []readyDatabaseResult `json:"databases"` + Total int `json:"total"` + Limit int `json:"limit"` + Truncated bool `json:"truncated"` + Considered int `json:"considered"` + MaxDatabases int `json:"max_databases"` + Capped bool `json:"capped"` + Unreadable []readyUnreadableResult `json:"unreadable"` + } +) + +func readyWork(t *testing.T, s *mcp.ClientSession, a map[string]any) readyWorkResult { + t.Helper() + var out readyWorkResult + decode(t, call(t, s, "ready_work", a), &out) + return out +} + +// readySlugs is the addresses of the databases in an answer, in the order they +// were answered in. +func readySlugs(res readyWorkResult) []string { + out := make([]string, 0, len(res.Databases)) + for _, d := range res.Databases { + out = append(out, d.Owner+"/"+d.Name) + } + return out +} + +func readyIDs(db readyDatabaseResult) []string { + out := make([]string, 0, len(db.Ready)) + for _, c := range db.Ready { + out = append(out, c.ID) + } + return out +} + +func readyDatabaseNamed(t *testing.T, res readyWorkResult, slug string) readyDatabaseResult { + t.Helper() + for _, d := range res.Databases { + if d.Owner+"/"+d.Name == slug { + return d + } + } + t.Fatalf("no database %q in the answer: %v", slug, readySlugs(res)) + return readyDatabaseResult{} +} + +// --- the cross-database arm --------------------------------------------------- + +// The arm the tool exists for: no database named, every tracker the caller may +// see, grouped and ordered as the page groups and orders them. +func TestReadyWorkAnswersAcrossEveryTracker(t *testing.T) { + server, _, _ := readyServer(t) + got := readyWork(t, connect(t, server, nil), nil) + + assert.Equal(t, []string{"alice/alpha", "bob/beta"}, readySlugs(got), + "the busier database leads; a tracker with nothing ready is absent rather than empty") + assert.Equal(t, 3, got.Total) + assert.Equal(t, 200, got.Limit, "the default of docs/DESIGN.mcp.md §9.3") + assert.False(t, got.Truncated) + assert.False(t, got.Capped) + assert.Equal(t, beads.ReadyMaxDatabases, got.MaxDatabases, + "the ceiling is beads' and is published so the answer can be read against it") + + alpha := readyDatabaseNamed(t, got, "alice/alpha") + assert.Equal(t, "main", alpha.Ref, "each database is read at its own default branch") + require.NotNil(t, alpha.Head, "and says which commit it read") + assert.Equal(t, "h-alpha", alpha.Head.Hash) + assert.True(t, headTime.Equal(alpha.Head.Time), "got %v", alpha.Head.Time) + assert.Equal(t, 2, alpha.Count) + assert.Equal(t, []string{"a-1", "a-2"}, readyIDs(alpha), "priority first, then id") + + assert.Equal(t, readyCardResult{ + ID: "a-2", Title: "Ready, lower", Type: "task", Priority: "2", Assignee: "bob", + Labels: []string{}, BlockedBy: 1, Blocks: 0, + }, alpha.Ready[1], "a closed blocker is still a dependency, and the issue is still ready") + + beta := readyDatabaseNamed(t, got, "bob/beta") + assert.Equal(t, "h-beta", beta.Head.Hash, "each group carries its own database's head") + assert.Equal(t, []string{"b-1"}, readyIDs(beta)) +} + +// The listing rule, per caller: what a caller may be shown is exactly what the +// cross-database arm shows, and nothing about the rest of the instance reaches +// the payload — not a name, not a title, not an id. +func TestReadyWorkShowsExactlyWhatTheListingRuleAllows(t *testing.T) { + server, _, _ := readyServer(t) + + for _, tc := range []struct { + name string + caller *auth.AuthContext + want []string + }{ + {"anonymous", nil, []string{"alice/alpha", "bob/beta"}}, + {"a stranger", carol(), []string{"alice/alpha", "bob/beta"}}, + {"a grantee", bob(), []string{"alice/alpha", "bob/beta", "dave/secrets"}}, + {"an owner of one of them", alice(), []string{"alice/alpha", "bob/beta"}}, + } { + t.Run(tc.name, func(t *testing.T) { + session := connect(t, server, tc.caller) + assert.Equal(t, tc.want, readySlugs(readyWork(t, session, nil))) + + payload := resultJSON(t, call(t, session, "ready_work", nil)) + assert.NotContains(t, payload, unlistedMarker, + "an unlisted database of somebody else is absent from a listing") + assert.NotContains(t, payload, "hidden") + assert.NotContains(t, payload, "u-1") + + if tc.caller == nil || tc.caller.UserID != bobID { + assert.NotContains(t, payload, privateMarker, "not a title") + assert.NotContains(t, payload, "secrets", "not a name") + assert.NotContains(t, payload, "s-1", "not an id") + assert.NotContains(t, payload, "dave", "not an owner") + } + }) + } +} + +// The grantee's own answer, so the absences above are a visibility rule and not +// a broken tool: bob is granted on the private tracker and reads it. +func TestReadyWorkAnswersAGranteeAboutAPrivateTracker(t *testing.T) { + server, _, _ := readyServer(t) + got := readyWork(t, connect(t, server, bob()), nil) + + secrets := readyDatabaseNamed(t, got, "dave/secrets") + assert.Equal(t, []string{"s-1"}, readyIDs(secrets)) + assert.Equal(t, 4, got.Total, "its ready work counts towards the answer") +} + +// Enumerating is two rules and not one, and this is the database where they +// differ: db/'s listing query is mode-blind — it lists anything the caller holds +// an ACL *row* on — while core.Allowed reads the mode and falls through to +// visibility for one it does not recognise. A PRIVATE database carrying such a +// row is therefore listed to that caller and must still not be read, which is +// the whole job of the second step. +func TestReadyWorkAsksTheAccessRuleAndNotOnlyTheListing(t *testing.T) { + fixture := func(mode core.AccessMode) []readyFixture { + return []readyFixture{{ + owner: "dave", ownerID: daveID, name: "quarantine", vis: core.VisibilityPrivate, + acl: map[int]core.AccessMode{carolID: mode}, + session: readyTracker("h-quarantine", readyIssues([][6]string{ + {"q-1", privateMarker + " the quarantined plan", "open", "0", "dave", ""}, + }), nil), + }} + } + + t.Run("a grant this service does not recognise reads nothing", func(t *testing.T) { + repos, opener := readyFakes(fixture(core.AccessMode("ARCHIVED"))) + got := readyWork(t, connect(t, newServer(t, repos, opener), carol()), nil) + + assert.Empty(t, got.Databases) + assert.Zero(t, got.Total) + assert.Empty(t, got.Unreadable, "it is not a failure either: it is simply absent") + assert.Empty(t, opener.opened, "a database the access rule refuses is never opened") + }) + + t.Run("and the same row with a grant it does reads it", func(t *testing.T) { + repos, opener := readyFakes(fixture(core.AccessRO)) + got := readyWork(t, connect(t, newServer(t, repos, opener), carol()), nil) + + require.Len(t, got.Databases, 1, "so the refusal above is the mode and not the fixture") + assert.Equal(t, []string{"q-1"}, readyIDs(got.Databases[0])) + }) +} + +// --- the two arms agree ------------------------------------------------------- + +// The named arm is the cross arm restricted to one candidate, and this is where +// that is asserted rather than assumed: same ref, same head, same cards, same +// per-database count. +func TestReadyWorkNamedDatabaseAgreesWithTheCrossArm(t *testing.T) { + server, _, _ := readyServer(t) + session := connect(t, server, nil) + + across := readyDatabaseNamed(t, readyWork(t, session, nil), "alice/alpha") + + named := readyWork(t, session, args("alpha")) + require.Len(t, named.Databases, 1, "one database was named, so one is answered about") + assert.Equal(t, across, named.Databases[0], + "one aggregator, two arms: naming a database may not change what it says") + assert.Equal(t, 2, named.Total) + assert.Equal(t, 1, named.Considered) + assert.False(t, named.Capped) + assert.Empty(t, named.Unreadable) +} + +// An unlisted database is absent from the listing and answered when addressed — +// the rule the dashboard already implements, and the reason the two arms are not +// the same question. +func TestReadyWorkNamesAnUnlistedTrackerTheListingHides(t *testing.T) { + server, _, _ := readyServer(t) + session := connect(t, server, nil) + + assert.NotContains(t, readySlugs(readyWork(t, session, nil)), "frank/hidden") + + named := readyWork(t, session, map[string]any{"owner": "frank", "name": "hidden"}) + require.Len(t, named.Databases, 1) + assert.Equal(t, []string{"u-1"}, readyIDs(named.Databases[0])) +} + +// --- the three bounds --------------------------------------------------------- + +// The head-hash gate: a second call whose heads have not moved reads no rows at +// all. Counted on the fakes, never timed — and it holds only because the cache +// lives on the server rather than being built per call. +func TestReadyWorkHeadHashGateReadsNothingTwice(t *testing.T) { + server, _, opener := readyServer(t) + session := connect(t, server, nil) + + alpha := opener.sessions[storePath("alice", "alpha")] + beta := opener.sessions[storePath("bob", "beta")] + sensors := opener.sessions[storePath("erin", "sensors")] + + first := readyWork(t, session, nil) + reads := alpha.rowReads + require.Greater(t, reads, 0, "the first call must read rows") + + second := readyWork(t, session, nil) + + assert.Equal(t, reads, alpha.rowReads, "an unmoved head must cost no row reads") + assert.Equal(t, 1, alpha.tableReads, "nor a second table listing") + assert.Equal(t, 1, alpha.logReads, "nor a second log read") + assert.Equal(t, 1, beta.tableReads, "the sibling database is gated too") + assert.Equal(t, 1, sensors.tableReads, + "and so is the fingerprint of a database that is not a tracker") + assert.Equal(t, first, second, "the answer is the same answer, cache or not") + + // Both stores are still opened and their branches listed: that is what the + // gate is gated on, and it is the cheap half. + assert.Equal(t, 2, countOpens(opener, storePath("alice", "alpha"))) +} + +// The two arms share one cache, which is the same statement as "they cannot +// disagree": a database read by the named arm is not read again by the cross +// arm while its head stands. +func TestReadyWorkSharesOneCacheBetweenTheArms(t *testing.T) { + server, _, opener := readyServer(t) + session := connect(t, server, nil) + alpha := opener.sessions[storePath("alice", "alpha")] + + readyWork(t, session, args("alpha")) + reads := alpha.rowReads + require.Greater(t, reads, 0) + + readyWork(t, session, nil) + assert.Equal(t, reads, alpha.rowReads, "the cross arm read what the named arm had cached") +} + +// The ceiling: more candidate databases than beads.ReadyMaxDatabases and the +// answer says so, because a silent cap reads as "that is everything". The +// databases past it are never opened. +func TestReadyWorkReportsTheCeiling(t *testing.T) { + var fx []readyFixture + for i := 1; i <= beads.ReadyMaxDatabases+2; i++ { + name := fmt.Sprintf("db%02d", i) + fx = append(fx, readyFixture{ + owner: "alice", ownerID: aliceID, name: name, vis: core.VisibilityPublic, + session: readyTracker(fmt.Sprintf("h%02d", i), readyIssues([][6]string{ + {fmt.Sprintf("t%02d-1", i), "Ready", "open", "1", "alice", ""}, + }), nil), + }) + } + repos, opener := readyFakes(fx) + got := readyWork(t, connect(t, newServer(t, repos, opener), nil), nil) + + assert.True(t, got.Capped, "there were more trackers than one call may open") + assert.Equal(t, beads.ReadyMaxDatabases, got.Considered) + assert.Equal(t, beads.ReadyMaxDatabases, got.MaxDatabases) + assert.Len(t, got.Databases, beads.ReadyMaxDatabases) + assert.Equal(t, beads.ReadyMaxDatabases, got.Total) + + for _, f := range fx[beads.ReadyMaxDatabases:] { + assert.Zero(t, countOpens(opener, storePath(f.owner, f.name)), + "%s: a database past the ceiling is never opened", f.slug()) + } +} + +// --- what one broken database costs ------------------------------------------- + +// A store that will not open costs itself only: the rest of the answer is the +// answer, the gap is admitted by name, and the browse error reaches the log +// rather than the caller. +func TestReadyWorkSurvivesADatabaseThatCannotBeOpened(t *testing.T) { + server, _, _ := readyServer(t) + session := connect(t, server, nil) + + got := readyWork(t, session, nil) + assert.Equal(t, []string{"alice/alpha", "bob/beta"}, readySlugs(got)) + assert.Equal(t, 3, got.Total) + assert.Equal(t, []readyUnreadableResult{{Owner: "erin", Name: "broken"}}, got.Unreadable, + "the gap is named so the set is readable as incomplete") + + payload := resultJSON(t, call(t, session, "ready_work", nil)) + assert.NotContains(t, payload, "/stores/", "no on-disk path reaches a caller") + assert.NotContains(t, payload, "no store at") +} + +// A database that is not a beads tracker is skipped silently: no group, no note, +// and not a row read. +func TestReadyWorkSkipsNonBeadsDatabasesSilently(t *testing.T) { + server, _, opener := readyServer(t) + + got := readyWork(t, connect(t, server, nil), nil) + assert.NotContains(t, readySlugs(got), "erin/sensors") + for _, u := range got.Unreadable { + assert.NotEqual(t, "sensors", u.Name, "not being a tracker is not a failure") + } + assert.Zero(t, opener.sessions[storePath("erin", "sensors")].rowReads, + "a database that is not a tracker is never read for rows") +} + +// Naming one is the other half of the same rule: a database the caller is +// looking straight at, which is not a tracker, is refused with the sentence that +// names the generic tools — never an empty ready set, which would read as +// "nothing to do here". +func TestReadyWorkRefusesANamedDatabaseThatIsNotATracker(t *testing.T) { + server, _, _ := readyServer(t) + + res := call(t, connect(t, server, nil), "ready_work", + map[string]any{"owner": "erin", "name": "sensors"}) + require.True(t, res.IsError) + + text := errorText(res) + assert.Contains(t, text, "~erin/sensors") + assert.Contains(t, text, "not a beads issue tracker") + assert.Contains(t, text, "list_tables", "and names the way to read it anyway") + assert.NotContains(t, text, "no database", "the database exists and this caller may read it") +} + +// --- the filters -------------------------------------------------------------- + +// Each filter narrows the ready set exactly as the page's does: the filtering is +// beads.ReadyFilter's, and these are the cases that would catch a second +// implementation drifting from it. +func TestReadyWorkFiltersNarrowAsThePageDoes(t *testing.T) { + server, _, _ := readyServer(t) + session := connect(t, server, bob()) + + for _, tc := range []struct { + name string + args map[string]any + want []string + total int + }{ + {"q over the title", map[string]any{"q": "top priority"}, []string{"alice/alpha"}, 1}, + {"q over the id", map[string]any{"q": "b-1"}, []string{"bob/beta"}, 1}, + {"q is case-insensitive", map[string]any{"q": "READY, LOWER"}, []string{"alice/alpha"}, 1}, + {"assignee", map[string]any{"assignee": "bob"}, []string{"alice/alpha"}, 1}, + {"priority", map[string]any{"priority": "0"}, []string{"alice/alpha", "dave/secrets"}, 2}, + {"two at once", map[string]any{"priority": "0", "assignee": "dave"}, []string{"dave/secrets"}, 1}, + {"a filter nothing matches", map[string]any{"assignee": "nobody"}, nil, 0}, + } { + t.Run(tc.name, func(t *testing.T) { + got := readyWork(t, session, tc.args) + assert.ElementsMatch(t, tc.want, readySlugs(got)) + assert.Equal(t, tc.total, got.Total, "the total is the matched set, not the instance") + }) + } +} + +// The filters narrow one named database too, so the arms agree under a filter as +// well as without one. +func TestReadyWorkFiltersANamedDatabase(t *testing.T) { + server, _, _ := readyServer(t) + session := connect(t, server, nil) + + got := readyWork(t, session, args("alpha", "assignee", "bob")) + require.Len(t, got.Databases, 1) + assert.Equal(t, []string{"a-2"}, readyIDs(got.Databases[0])) + assert.Equal(t, 1, got.Databases[0].Count, "the per-database count is the filtered one") +} + +// --- the cap ------------------------------------------------------------------ + +// The cap of docs/DESIGN.mcp.md §9.3, spent across databases: the answer carries +// the limit really applied, the true total, and whether anything was left +// behind. Each database keeps its own honest count. +func TestReadyWorkCapsTheLimitAndSaysSo(t *testing.T) { + server, _, _ := readyServer(t) + session := connect(t, server, nil) + + t.Run("a limit above the cap is answered at the cap", func(t *testing.T) { + got := readyWork(t, session, map[string]any{"limit": 5000}) + assert.Equal(t, 500, got.Limit, "the cap, reported rather than silently applied") + assert.Equal(t, 3, got.Total) + assert.False(t, got.Truncated) + }) + + t.Run("a limit below the matches clips and says so", func(t *testing.T) { + got := readyWork(t, session, map[string]any{"limit": 1}) + assert.Equal(t, 1, got.Limit) + require.Len(t, got.Databases, 1, "a database left with nothing is absent, not empty") + assert.Equal(t, []string{"a-1"}, readyIDs(got.Databases[0])) + assert.Equal(t, 2, got.Databases[0].Count, "and its own count is still the true one") + assert.Equal(t, 3, got.Total, "the honest denominator is every match") + assert.True(t, got.Truncated) + }) + + t.Run("the limit is spent across databases", func(t *testing.T) { + got := readyWork(t, session, map[string]any{"limit": 3}) + assert.Equal(t, []string{"alice/alpha", "bob/beta"}, readySlugs(got)) + assert.False(t, got.Truncated) + }) + + t.Run("a non-positive limit is refused", func(t *testing.T) { + for _, limit := range []int{0, -1} { + res := call(t, session, "ready_work", map[string]any{"limit": limit}) + require.True(t, res.IsError, "limit %d", limit) + assert.Contains(t, errorText(res), "limit must be a positive number of issues") + } + }) +} + +// --- addressing --------------------------------------------------------------- + +// Half an address is not an address. Naming neither is the cross-database arm and +// naming both is one database; naming one of the two means nothing and is refused +// rather than guessed at in either direction. +func TestReadyWorkNeedsBothHalvesOfAnAddress(t *testing.T) { + server, _, _ := readyServer(t) + session := connect(t, server, nil) + + for _, a := range []map[string]any{{"owner": "alice"}, {"name": "alpha"}} { + res := call(t, session, "ready_work", a) + require.True(t, res.IsError, "%v", a) + assert.Contains(t, errorText(res), "owner") + assert.Contains(t, errorText(res), "name") + } +} + +// A database the caller may not read answers exactly as one that does not exist, +// which is the masked not-found the rest of the surface answers with: a refusal +// of its own shape would rebuild the distinction it exists to erase. +func TestReadyWorkMasksADatabaseTheCallerMayNotRead(t *testing.T) { + server, _, _ := readyServer(t) + session := connect(t, server, carol()) + + masked := call(t, session, "ready_work", map[string]any{"owner": "dave", "name": "secrets"}) + require.True(t, masked.IsError) + + absent := call(t, session, "ready_work", map[string]any{"owner": "dave", "name": "nosuch"}) + require.True(t, absent.IsError) + + assert.Equal(t, + strings.Replace(errorText(absent), "nosuch", "secrets", 1), + errorText(masked), + "a masked database answers exactly as one that does not exist") + assert.NotContains(t, errorText(masked), privateMarker) +} + +// --- sessions and bodies ------------------------------------------------------ + +// One session per database per call, closed by the tool that opened it — in both +// arms. The named arm is the one worth counting twice: it opens the store for +// the fingerprint and then hands the *same* session to the aggregation, so a +// second open would be a store opened twice for one call, and a second close +// would be a handle closed twice. +func TestReadyWorkClosesEverySessionItOpened(t *testing.T) { + t.Run("across every tracker", func(t *testing.T) { + server, _, opener := readyServer(t) + readyWork(t, connect(t, server, nil), nil) + + for _, f := range readyFixtures() { + if f.session == nil { + continue + } + path := storePath(f.owner, f.name) + if f.vis != core.VisibilityPublic { + assert.Zero(t, countOpens(opener, path), + "%s: a database this caller may not list is never opened", f.slug()) + assert.Zero(t, f.session.closes) + continue + } + assert.Equal(t, 1, countOpens(opener, path), "%s: opened once", f.slug()) + assert.Equal(t, 1, opener.sessions[path].closes, "%s: and closed once", f.slug()) + } + }) + + t.Run("one named tracker", func(t *testing.T) { + server, _, opener := readyServer(t) + readyWork(t, connect(t, server, nil), args("alpha")) + + assert.Equal(t, []string{storePath("alice", "alpha")}, opener.opened, + "exactly the one database the call named, opened exactly once") + assert.Equal(t, 1, opener.sessions[storePath("alice", "alpha")].closes, + "the session lent to the aggregation is closed by the handler that opened it, once") + }) +} + +// The list/detail split of docs/DESIGN.mcp.md §9.2 holds here too: this is a +// listing, so it carries identity and metadata and no prose. Asserted over the +// serialised payload, so a field added to the card later would carry a marker +// into it and turn this red. +func TestReadyWorkCarriesNoIssueBodies(t *testing.T) { + server, _, _ := readyServer(t) + payload := resultJSON(t, call(t, connect(t, server, nil), "ready_work", nil)) + + assert.Contains(t, payload, "a-1", "the fixture's body belongs to an issue that is in the answer") + assert.NotContains(t, payload, bodyMarker) + for _, field := range []string{"description", "design", "acceptance_criteria", "notes"} { + assert.NotContains(t, payload, `"`+field+`"`, + "the card type has no field a body could arrive in, and that is structural") + } +} + +// countOpens is how many times a store path was opened, over the log the fake +// opener keeps. +func countOpens(o *fakeOpener, path string) int { + n := 0 + for _, p := range o.opened { + if p == path { + n++ + } + } + return n +}