M mcpsrv/browse.go => mcpsrv/browse.go +18 -1
@@ 528,7 528,16 @@ func (s *Server) getCommitLog(ctx context.Context, in getCommitLogInput) (getCom
// ErrRefNotFound the same as any other ref it cannot resolve, and refMiss
// reads that sentinel here. A genuine failure of the store still takes the
// protocol arm below.
- return out, refMiss(err, tool, noSuchRef(in.databaseRef, ref))
+ //
+ // The sentence names whichever of ref and from the call actually supplied:
+ // with a cursor set, ref is "" (it was never resolved, see above), and a
+ // refusal that named it anyway would send the caller looking for a typo in
+ // a branch name it never typed.
+ missing := noSuchRef(in.databaseRef, ref)
+ if from != "" {
+ missing = noSuchCursor(in.databaseRef, from)
+ }
+ return out, refMiss(err, tool, missing)
}
out.Ref = ref
@@ 743,6 752,14 @@ func noSuchRef(ref databaseRef, named string) string {
return fmt.Sprintf("%s has no branch or commit %q; list_branches names its branches", ref, named)
}
+// noSuchCursor is get_commit_log's miss for a from-cursor that names no commit:
+// the counterpart of noSuchRef for the one call where a page can be continued by
+// hash instead of by ref, so the refusal points at "from" rather than sending the
+// caller to list_branches over a value that was never a branch name.
+func noSuchCursor(ref databaseRef, from string) string {
+ return fmt.Sprintf("%s has no commit %q to continue from; from takes the \"next\" hash a previous page of get_commit_log returned", ref, from)
+}
+
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)
}
M mcpsrv/browse_test.go => mcpsrv/browse_test.go +10 -1
@@ 500,11 500,20 @@ func TestGetCommitLogWithABadCursorIsAMiss(t *testing.T) {
t.Run(from, func(t *testing.T) {
res := call(t, anonSession(t), "get_commit_log", args("notes", "from", from))
require.True(t, res.IsError, "a bad cursor is a tool result, not a protocol error")
- assert.NotContains(t, errorText(res), "no database", "this is not the masked not-found")
+
+ text := errorText(res)
+ assert.NotContains(t, text, "no database", "this is not the masked not-found")
+ assert.Contains(t, text, from, "the refusal names the cursor the caller actually sent")
+ assert.Contains(t, text, "~alice/notes")
})
}
}
+// TestAnUnresolvableRefIsAnAnswerAboutTheDatabase (above) already pins the other
+// half of this: a call that names a ref rather than a cursor gets a refusal
+// naming *that* ref. This test exists so the two halves are asserted in the same
+// place a reader looking for "does the cursor case name the cursor" would look.
+
// --- the visibility matrix --------------------------------------------------
// browseTarget is one fixture database with arguments valid for it, so that the