detached 15cb272

feat(rpc): report truncated trees and last commits, bound GetTree

GetTree sets last_commits_truncated from Tree.LastCommitsOmitted and
truncated when the directory has more than domain.MaxTreeEntries (5,000)
entries. The skip of a commit page token is tested at its cap.
commit
15cb272c6b5417db78698354ba7462315a0fabba
author
brice · · 2026-10-10 11:14 UTC
parent
d97ab57
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
6 files · +70 -2
Split
UnifiedSplit
M internal/adapters/rpc/git.go +3 -0
@@ -75,6 +75,9 @@ func (s *gitService) GetTree(ctx context.Context, req *connect.Request[pb.GetTre
7575 CommitId: tree.CommitID.String(),
7676 TreeId: tree.ID.String(),
7777 Entries: make([]*pb.TreeEntry, 0, len(tree.Entries)),
78+
79+ LastCommitsTruncated: tree.LastCommitsOmitted,
80+ Truncated: tree.Truncated,
7881 }
7982 for _, e := range tree.Entries {
8083 out.Entries = append(out.Entries, treeEntryToPB(e))
@@ -75,6 +75,9 @@ func (s *gitService) GetTree(ctx context.Context, req *connect.Request[pb.GetTre
75 CommitId: tree.CommitID.String(),75 CommitId: tree.CommitID.String(),
76 TreeId: tree.ID.String(),76 TreeId: tree.ID.String(),
77 Entries: make([]*pb.TreeEntry, 0, len(tree.Entries)),77 Entries: make([]*pb.TreeEntry, 0, len(tree.Entries)),
78+
79+ LastCommitsTruncated: tree.LastCommitsOmitted,
80+ Truncated: tree.Truncated,
78 }81 }
79 for _, e := range tree.Entries {82 for _, e := range tree.Entries {
80 out.Entries = append(out.Entries, treeEntryToPB(e))83 out.Entries = append(out.Entries, treeEntryToPB(e))
M internal/app/browse_test.go +26 -0
@@ -4,8 +4,10 @@ package app_test
44
55 import (
66 "context"
7+ "encoding/base64"
78 "fmt"
89 "io"
10+ "strconv"
911 "strings"
1012 "testing"
1113
@@ -324,6 +326,30 @@ func TestListCommitsPaging(t *testing.T) {
324326 }
325327 })
326328
329+ t.Run("a token cannot skip deeper than the cap", func(t *testing.T) {
330+ f := newBrowseFixture(t)
331+ f.meta.EXPECT().Get(ctx, pub.Name).Return(pub, nil).Maybe()
332+ f.reader.EXPECT().ListCommits(ctx, pub.Name, mock.Anything).
333+ Return(ports.CommitPage{Head: oid("a"), Commits: []domain.Commit{{ID: oid("b")}}, HasMore: true}, nil).Once()
334+ page, err := f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, app.ListCommitsParams{Path: p})
335+ require.NoError(t, err)
336+ raw, err := base64.RawURLEncoding.DecodeString(page.NextPageToken)
337+ require.NoError(t, err)
338+ parts := strings.Split(string(raw), ":") // c1:<head>:<skip>:<path digest>
339+ require.Len(t, parts, 4)
340+ withSkip := func(n int) string {
341+ parts[2] = strconv.Itoa(n)
342+ return base64.RawURLEncoding.EncodeToString([]byte(strings.Join(parts, ":")))
343+ }
344+
345+ f.reader.EXPECT().ListCommits(ctx, pub.Name, mock.MatchedBy(func(q ports.CommitQuery) bool { return q.Skip == app.MaxCommitSkip })).
346+ Return(ports.CommitPage{Head: oid("a")}, nil).Once()
347+ _, err = f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, app.ListCommitsParams{Path: p, PageToken: withSkip(app.MaxCommitSkip)})
348+ require.NoError(t, err, "the cap itself is allowed")
349+ _, err = f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, app.ListCommitsParams{Path: p, PageToken: withSkip(app.MaxCommitSkip + 1)})
350+ assert.ErrorIs(t, err, domain.ErrInvalidPageToken)
351+ })
352+
327353 t.Run("page size", func(t *testing.T) {
328354 f := newBrowseFixture(t)
329355 f.meta.EXPECT().Get(ctx, pub.Name).Return(pub, nil).Times(2)
@@ -4,8 +4,10 @@ package app_test
4 4
5 import (5 import (
6 "context"6 "context"
7+ "encoding/base64"
7 "fmt"8 "fmt"
8 "io"9 "io"
10+ "strconv"
9 "strings"11 "strings"
10 "testing"12 "testing"
11 13
@@ -324,6 +326,30 @@ func TestListCommitsPaging(t *testing.T) {
324 }326 }
325 })327 })
326 328
329+ t.Run("a token cannot skip deeper than the cap", func(t *testing.T) {
330+ f := newBrowseFixture(t)
331+ f.meta.EXPECT().Get(ctx, pub.Name).Return(pub, nil).Maybe()
332+ f.reader.EXPECT().ListCommits(ctx, pub.Name, mock.Anything).
333+ Return(ports.CommitPage{Head: oid("a"), Commits: []domain.Commit{{ID: oid("b")}}, HasMore: true}, nil).Once()
334+ page, err := f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, app.ListCommitsParams{Path: p})
335+ require.NoError(t, err)
336+ raw, err := base64.RawURLEncoding.DecodeString(page.NextPageToken)
337+ require.NoError(t, err)
338+ parts := strings.Split(string(raw), ":") // c1:<head>:<skip>:<path digest>
339+ require.Len(t, parts, 4)
340+ withSkip := func(n int) string {
341+ parts[2] = strconv.Itoa(n)
342+ return base64.RawURLEncoding.EncodeToString([]byte(strings.Join(parts, ":")))
343+ }
344+
345+ f.reader.EXPECT().ListCommits(ctx, pub.Name, mock.MatchedBy(func(q ports.CommitQuery) bool { return q.Skip == app.MaxCommitSkip })).
346+ Return(ports.CommitPage{Head: oid("a")}, nil).Once()
347+ _, err = f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, app.ListCommitsParams{Path: p, PageToken: withSkip(app.MaxCommitSkip)})
348+ require.NoError(t, err, "the cap itself is allowed")
349+ _, err = f.browse.ListCommits(ctx, domain.Anonymous(), pub.Name, app.ListCommitsParams{Path: p, PageToken: withSkip(app.MaxCommitSkip + 1)})
350+ assert.ErrorIs(t, err, domain.ErrInvalidPageToken)
351+ })
352+
327 t.Run("page size", func(t *testing.T) {353 t.Run("page size", func(t *testing.T) {
328 f := newBrowseFixture(t)354 f := newBrowseFixture(t)
329 f.meta.EXPECT().Get(ctx, pub.Name).Return(pub, nil).Times(2)355 f.meta.EXPECT().Get(ctx, pub.Name).Return(pub, nil).Times(2)
M internal/app/events_test.go +3 -1
@@ -474,7 +474,7 @@ func TestWatchEndsWhenTheTokenExpires(t *testing.T) {
474474 // The rig's clock stands at now: this token expired at now.
475475 p := tokenWith("tok", now)
476476 w := r.watch(context.Background(), p, app.WatchParams{})
477- r.push(pub, 1)
477+ r.push(pub, 1) // whether or not the watch had started, it ends unauthenticated
478478 select {
479479 case err := <-w.done:
480480 assert.ErrorIs(t, err, domain.ErrUnauthenticated)
@@ -502,6 +502,7 @@ func TestWatchRevalidatesIdleWatchersAndCachesTheAnswer(t *testing.T) {
502502 r := newEventRigConfig(t, app.EventsConfig{RevalidateEvery: time.Hour})
503503 pub := r.repo("pub", domain.VisibilityPublic)
504504 w := r.watch(context.Background(), tokenWith("tok", time.Time{}), app.WatchParams{})
505+ require.Eventually(t, func() bool { return r.events.Watchers() == 1 }, 5*time.Second, time.Millisecond)
505506 for i := range 20 {
506507 r.push(pub, i)
507508 }
@@ -516,6 +517,7 @@ func TestWatchRevalidatesIdleWatchersAndCachesTheAnswer(t *testing.T) {
516517 pub := r.repo("pub", domain.VisibilityPublic)
517518 a := r.watch(context.Background(), domain.Anonymous(), app.WatchParams{})
518519 b := r.watch(context.Background(), domain.Root(), app.WatchParams{})
520+ require.Eventually(t, func() bool { return r.events.Watchers() == 2 }, 5*time.Second, time.Millisecond)
519521 r.push(pub, 1)
520522 a.next(t)
521523 b.next(t)
@@ -474,7 +474,7 @@ func TestWatchEndsWhenTheTokenExpires(t *testing.T) {
474 // The rig's clock stands at now: this token expired at now.474 // The rig's clock stands at now: this token expired at now.
475 p := tokenWith("tok", now)475 p := tokenWith("tok", now)
476 w := r.watch(context.Background(), p, app.WatchParams{})476 w := r.watch(context.Background(), p, app.WatchParams{})
477- r.push(pub, 1)477+ r.push(pub, 1) // whether or not the watch had started, it ends unauthenticated
478 select {478 select {
479 case err := <-w.done:479 case err := <-w.done:
480 assert.ErrorIs(t, err, domain.ErrUnauthenticated)480 assert.ErrorIs(t, err, domain.ErrUnauthenticated)
@@ -502,6 +502,7 @@ func TestWatchRevalidatesIdleWatchersAndCachesTheAnswer(t *testing.T) {
502 r := newEventRigConfig(t, app.EventsConfig{RevalidateEvery: time.Hour})502 r := newEventRigConfig(t, app.EventsConfig{RevalidateEvery: time.Hour})
503 pub := r.repo("pub", domain.VisibilityPublic)503 pub := r.repo("pub", domain.VisibilityPublic)
504 w := r.watch(context.Background(), tokenWith("tok", time.Time{}), app.WatchParams{})504 w := r.watch(context.Background(), tokenWith("tok", time.Time{}), app.WatchParams{})
505+ require.Eventually(t, func() bool { return r.events.Watchers() == 1 }, 5*time.Second, time.Millisecond)
505 for i := range 20 {506 for i := range 20 {
506 r.push(pub, i)507 r.push(pub, i)
507 }508 }
@@ -516,6 +517,7 @@ func TestWatchRevalidatesIdleWatchersAndCachesTheAnswer(t *testing.T) {
516 pub := r.repo("pub", domain.VisibilityPublic)517 pub := r.repo("pub", domain.VisibilityPublic)
517 a := r.watch(context.Background(), domain.Anonymous(), app.WatchParams{})518 a := r.watch(context.Background(), domain.Anonymous(), app.WatchParams{})
518 b := r.watch(context.Background(), domain.Root(), app.WatchParams{})519 b := r.watch(context.Background(), domain.Root(), app.WatchParams{})
520+ require.Eventually(t, func() bool { return r.events.Watchers() == 2 }, 5*time.Second, time.Millisecond)
519 r.push(pub, 1)521 r.push(pub, 1)
520 a.next(t)522 a.next(t)
521 b.next(t)523 b.next(t)
M internal/app/repos_write_test.go +30 -0
@@ -61,6 +61,36 @@ func TestUpdate(t *testing.T) {
6161 assert.Equal(t, priv, got)
6262 })
6363
64+ t.Run("metadata is written before HEAD moves, and the event follows both", func(t *testing.T) {
65+ f := newFixture(t)
66+ var order []string
67+ f.meta.EXPECT().Get(ctx, n).Return(priv, nil)
68+ f.git.EXPECT().HasBranch(ctx, n, "dev").Return(true, nil)
69+ f.meta.EXPECT().Update(ctx, mock.Anything).RunAndReturn(func(context.Context, domain.Repo) error {
70+ order = append(order, "meta")
71+ return nil
72+ })
73+ f.git.EXPECT().SetDefaultBranch(ctx, n, "dev").RunAndReturn(func(context.Context, domain.RepoName, string) error {
74+ order = append(order, "head")
75+ return nil
76+ })
77+ f.bus.EXPECT().Publish(ctx, mock.Anything).Run(func(context.Context, domain.Event) { order = append(order, "event") }).Return().Once()
78+ _, err := f.repos.Update(ctx, admin, n, app.UpdateParams{Description: ptr("x"), DefaultBranch: ptr("dev")})
79+ require.NoError(t, err)
80+ assert.Equal(t, []string{"meta", "head", "event"}, order)
81+ })
82+
83+ t.Run("a HEAD that cannot move fails the call and publishes nothing", func(t *testing.T) {
84+ f := newFixture(t)
85+ boom := errors.New("boom")
86+ f.meta.EXPECT().Get(ctx, n).Return(priv, nil)
87+ f.git.EXPECT().HasBranch(ctx, n, "dev").Return(true, nil)
88+ f.meta.EXPECT().Update(ctx, mock.Anything).Return(nil)
89+ f.git.EXPECT().SetDefaultBranch(ctx, n, "dev").Return(boom)
90+ _, err := f.repos.Update(ctx, admin, n, app.UpdateParams{Description: ptr("x"), DefaultBranch: ptr("dev")})
91+ assert.ErrorIs(t, err, boom)
92+ })
93+
6494 t.Run("a missing branch is a failed precondition and HEAD stays", func(t *testing.T) {
6595 f := newFixture(t)
6696 f.meta.EXPECT().Get(ctx, n).Return(priv, nil)
@@ -61,6 +61,36 @@ func TestUpdate(t *testing.T) {
61 assert.Equal(t, priv, got)61 assert.Equal(t, priv, got)
62 })62 })
63 63
64+ t.Run("metadata is written before HEAD moves, and the event follows both", func(t *testing.T) {
65+ f := newFixture(t)
66+ var order []string
67+ f.meta.EXPECT().Get(ctx, n).Return(priv, nil)
68+ f.git.EXPECT().HasBranch(ctx, n, "dev").Return(true, nil)
69+ f.meta.EXPECT().Update(ctx, mock.Anything).RunAndReturn(func(context.Context, domain.Repo) error {
70+ order = append(order, "meta")
71+ return nil
72+ })
73+ f.git.EXPECT().SetDefaultBranch(ctx, n, "dev").RunAndReturn(func(context.Context, domain.RepoName, string) error {
74+ order = append(order, "head")
75+ return nil
76+ })
77+ f.bus.EXPECT().Publish(ctx, mock.Anything).Run(func(context.Context, domain.Event) { order = append(order, "event") }).Return().Once()
78+ _, err := f.repos.Update(ctx, admin, n, app.UpdateParams{Description: ptr("x"), DefaultBranch: ptr("dev")})
79+ require.NoError(t, err)
80+ assert.Equal(t, []string{"meta", "head", "event"}, order)
81+ })
82+
83+ t.Run("a HEAD that cannot move fails the call and publishes nothing", func(t *testing.T) {
84+ f := newFixture(t)
85+ boom := errors.New("boom")
86+ f.meta.EXPECT().Get(ctx, n).Return(priv, nil)
87+ f.git.EXPECT().HasBranch(ctx, n, "dev").Return(true, nil)
88+ f.meta.EXPECT().Update(ctx, mock.Anything).Return(nil)
89+ f.git.EXPECT().SetDefaultBranch(ctx, n, "dev").Return(boom)
90+ _, err := f.repos.Update(ctx, admin, n, app.UpdateParams{Description: ptr("x"), DefaultBranch: ptr("dev")})
91+ assert.ErrorIs(t, err, boom)
92+ })
93+
64 t.Run("a missing branch is a failed precondition and HEAD stays", func(t *testing.T) {94 t.Run("a missing branch is a failed precondition and HEAD stays", func(t *testing.T) {
65 f := newFixture(t)95 f := newFixture(t)
66 f.meta.EXPECT().Get(ctx, n).Return(priv, nil)96 f.meta.EXPECT().Get(ctx, n).Return(priv, nil)
M internal/domain/git.go +6 -0
@@ -316,8 +316,14 @@ type Tree struct {
316316 // LastCommitsOmitted is true when last commits were asked for and the
317317 // reader gave only the first entries (MaxLastCommitEntries) one.
318318 LastCommitsOmitted bool
319+ // Truncated is true when the directory has more than MaxTreeEntries
320+ // entries: Entries holds the first ones in the documented order.
321+ Truncated bool
319322 }
320323
324+// MaxTreeEntries bounds the entries of a listed directory.
325+const MaxTreeEntries = 5000
326+
321327 // MaxLastCommitEntries bounds the entries of one directory that get a last
322328 // commit: each costs a history lookup.
323329 const MaxLastCommitEntries = 200
@@ -316,8 +316,14 @@ type Tree struct {
316 // LastCommitsOmitted is true when last commits were asked for and the316 // LastCommitsOmitted is true when last commits were asked for and the
317 // reader gave only the first entries (MaxLastCommitEntries) one.317 // reader gave only the first entries (MaxLastCommitEntries) one.
318 LastCommitsOmitted bool318 LastCommitsOmitted bool
319+ // Truncated is true when the directory has more than MaxTreeEntries
320+ // entries: Entries holds the first ones in the documented order.
321+ Truncated bool
319 }322 }
320 323
324+// MaxTreeEntries bounds the entries of a listed directory.
325+const MaxTreeEntries = 5000
326+
321 // MaxLastCommitEntries bounds the entries of one directory that get a last327 // MaxLastCommitEntries bounds the entries of one directory that get a last
322 // commit: each costs a history lookup.328 // commit: each costs a history lookup.
323 const MaxLastCommitEntries = 200329 const MaxLastCommitEntries = 200
M internal/ports/ports.go +2 -1
@@ -127,7 +127,8 @@ type RepoReader interface {
127127 // to MinAbbrevLen or more). It returns domain.ErrNotFound if nothing
128128 // matches.
129129 ResolveRefPath(ctx context.Context, repo domain.RepoName, path string) (domain.RefPath, error)
130- // GetTree lists the directory at path. With includeLastCommit, the first
130+ // GetTree lists the directory at path, at most domain.MaxTreeEntries
131+ // entries (Tree.Truncated tells). With includeLastCommit, the first
131132 // domain.MaxLastCommitEntries entries get their last commit. It returns
132133 // domain.ErrNotDirectory if path names something else.
133134 GetTree(ctx context.Context, repo domain.RepoName, rev domain.Revision, path domain.TreePath, includeLastCommit bool) (domain.Tree, error)
@@ -127,7 +127,8 @@ type RepoReader interface {
127 // to MinAbbrevLen or more). It returns domain.ErrNotFound if nothing127 // to MinAbbrevLen or more). It returns domain.ErrNotFound if nothing
128 // matches.128 // matches.
129 ResolveRefPath(ctx context.Context, repo domain.RepoName, path string) (domain.RefPath, error)129 ResolveRefPath(ctx context.Context, repo domain.RepoName, path string) (domain.RefPath, error)
130- // GetTree lists the directory at path. With includeLastCommit, the first130+ // GetTree lists the directory at path, at most domain.MaxTreeEntries
131+ // entries (Tree.Truncated tells). With includeLastCommit, the first
131 // domain.MaxLastCommitEntries entries get their last commit. It returns132 // domain.MaxLastCommitEntries entries get their last commit. It returns
132 // domain.ErrNotDirectory if path names something else.133 // domain.ErrNotDirectory if path names something else.
133 GetTree(ctx context.Context, repo domain.RepoName, rev domain.Revision, path domain.TreePath, includeLastCommit bool) (domain.Tree, error)134 GetTree(ctx context.Context, repo domain.RepoName, rev domain.Revision, path domain.TreePath, includeLastCommit bool) (domain.Tree, error)

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 44

↑↓ moveenter openesc close