diff --git a/mcpsrv/browse.go b/mcpsrv/browse.go index 702f0eff1ef4dde6e7f19942976ca193355adf09..632c184fc0a1d4f8b245fbfaf5dc4c78d5ed567f 100644 --- a/mcpsrv/browse.go +++ b/mcpsrv/browse.go @@ -524,11 +524,10 @@ } 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)) } diff --git a/mcpsrv/browse_test.go b/mcpsrv/browse_test.go index a1c952832c4745bb181b98af57c4a1cbdf6f90cd..cdceb5053dfa12af3f611b215d77e5c714e3051b 100644 --- a/mcpsrv/browse_test.go +++ b/mcpsrv/browse_test.go @@ -487,6 +487,24 @@ assert.Contains(t, text, "~alice/notes") 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 diff --git a/mcpsrv/mcpsrv_test.go b/mcpsrv/mcpsrv_test.go index 4a9568cf0294edf08f406230ee2e086d0080ca86..d6fde66467675d4abdf8adfa728057b320b1a291 100644 --- a/mcpsrv/mcpsrv_test.go +++ b/mcpsrv/mcpsrv_test.go @@ -501,10 +501,12 @@ } 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))