M mcpsrv/browse.go => mcpsrv/browse.go +4 -5
@@ 524,11 524,10 @@ func (s *Server) getCommitLog(ctx context.Context, in getCommitLogInput) (getCom
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.
+ // A garbage or unknown cursor is an ordinary miss: browse wraps it in
+ // 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))
}
M mcpsrv/browse_test.go => mcpsrv/browse_test.go +18 -0
@@ 487,6 487,24 @@ func TestAnUnknownCommitIsAnAnswerAboutTheDatabase(t *testing.T) {
assert.NotContains(t, text, "no database")
}
+// A bad get_commit_log cursor — one that does not even parse as a hash, or one
+// that does but names no commit here — is the same ordinary miss an
+// unresolvable ref gets, never the protocol arm: browse.Log wraps both in
+// ErrRefNotFound and refMiss reads that sentinel. The fake cannot actually
+// tell the two shapes apart (it has no notion of "parses as a hash"), so both
+// cases below exercise the same lookup miss in fakeSession.Log — which is
+// faithful to browse.Log's own contract, where both collapse onto one
+// sentinel too.
+func TestGetCommitLogWithABadCursorIsAMiss(t *testing.T) {
+ for _, from := range []string{"not-a-hash", "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"} {
+ 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")
+ })
+ }
+}
+
// --- the visibility matrix --------------------------------------------------
// browseTarget is one fixture database with arguments valid for it, so that the
M mcpsrv/mcpsrv_test.go => mcpsrv/mcpsrv_test.go +5 -3
@@ 501,10 501,12 @@ func (s *fakeSession) Log(_ context.Context, refStr, fromHash string, limit int)
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.
+ // browse.Log wraps ErrRefNotFound around a from-hash that does not parse
+ // and one that parses but names no commit alike; this fake has no notion
+ // of "parses" at all, so both collapse onto the same lookup miss here,
+ // which is the same sentinel either way.
if start = s.indexOf(fromHash); start < 0 {
- return nil, "", fmt.Errorf("fake: invalid from hash %q", fromHash)
+ return nil, "", fmt.Errorf("%w: %s", browse.ErrRefNotFound, fromHash)
}
} else {
start = s.indexOf(s.resolve(refStr))