common: parser error fix resulting in deadlock

Validation of repository and org and other names is for the
consumer of this call. Maybe this can change later.

common/git_utils.go

@@ -391,12 +391,17 @@ func (e *GitHandlerImpl) GitExecQuietOrPanic(cwd string, params ...string) {
 }
 
 type ChanIO struct {
-	ch chan byte
+	ch   chan byte
+	done chan struct{}
 }
 
 func (c *ChanIO) Write(p []byte) (int, error) {
 	for _, b := range p {
-		c.ch <- b
+		select {
+		case c.ch <- b:
+		case <-c.done:
+			return 0, io.EOF
+		}
 	}
 	return len(p), nil
 }
@@ -405,21 +410,32 @@ func (c *ChanIO) Write(p []byte) (int, error) {
 func (c *ChanIO) Read(data []byte) (idx int, err error) {
 	var ok bool
 
-	data[idx], ok = <-c.ch
-	if !ok {
-		err = io.EOF
-		return
-	}
-	idx++
-
-	for len(c.ch) > 0 && idx < len(data) {
-		data[idx], ok = <-c.ch
+	select {
+	case data[idx], ok = <-c.ch:
 		if !ok {
 			err = io.EOF
 			return
 		}
-
 		idx++
+	case <-c.done:
+		err = io.EOF
+		return
+	}
+
+	for len(c.ch) > 0 && idx < len(data) {
+		select {
+		case data[idx], ok = <-c.ch:
+			if !ok {
+				err = io.EOF
+				return
+			}
+			idx++
+		case <-c.done:
+			err = io.EOF
+			return
+		default:
+			return
+		}
 	}
 
 	return
@@ -466,7 +482,14 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
 	var size int
 
 	pos := 0
-	for c := <-data; c != ' '; c = <-data {
+	for {
+		c, ok := <-data
+		if !ok {
+			return GitMsg{}, io.EOF
+		}
+		if c == ' ' {
+			break
+		}
 		if (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') {
 			id[pos] = c
 			pos++
@@ -478,7 +501,15 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
 
 	pos = 0
 	var c byte
-	for c = <-data; c != ' ' && c != '\x00'; c = <-data {
+	for {
+		var ok bool
+		c, ok = <-data
+		if !ok {
+			return GitMsg{}, io.EOF
+		}
+		if c == ' ' || c == '\x00' {
+			break
+		}
 		if c >= 'a' && c <= 'z' {
 			msgType[pos] = c
 			pos++
@@ -504,7 +535,14 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
 		return GitMsg{}, fmt.Errorf("Invalid object type: '%s'", string(msgType))
 	}
 
-	for c = <-data; c != '\000'; c = <-data {
+	for {
+		c, ok := <-data
+		if !ok {
+			return GitMsg{}, io.EOF
+		}
+		if c == '\x00' {
+			break
+		}
 		if c >= '0' && c <= '9' {
 			size = size*10 + (int(c) - '0')
 		} else {
@@ -523,18 +561,37 @@ func parseGitCommitHdr(oldHdr [2]string, data <-chan byte) ([2]string, int, erro
 	hdr := make([]byte, 0, 60)
 	val := make([]byte, 0, 1000)
 
-	c := <-data
+	c, ok := <-data
+	if !ok {
+		return [2]string{}, 0, io.EOF
+	}
 	size := 1
 	if c != '\n' { // end of header marker
-		for ; c != ' '; c = <-data {
+		for {
+			if c == ' ' {
+				break
+			}
 			hdr = append(hdr, c)
 			size++
+			var ok bool
+			c, ok = <-data
+			if !ok {
+				return [2]string{}, size, io.EOF
+			}
 		}
 		if size == 1 { // continuation header here
 			hdr = []byte(oldHdr[0])
 			val = append([]byte(oldHdr[1]), '\n')
 		}
-		for c := <-data; c != '\n'; c = <-data {
+		for {
+			var ok bool
+			c, ok = <-data
+			if !ok {
+				return [2]string{}, size, io.EOF
+			}
+			if c == '\n' {
+				break
+			}
 			val = append(val, c)
 			size++
 		}
@@ -547,7 +604,14 @@ func parseGitCommitHdr(oldHdr [2]string, data <-chan byte) ([2]string, int, erro
 func parseGitCommitMsg(data <-chan byte, l int) (string, error) {
 	msg := make([]byte, 0, l)
 
-	for c := <-data; c != '\x00'; c = <-data {
+	for {
+		c, ok := <-data
+		if !ok {
+			return string(msg), io.EOF
+		}
+		if c == '\x00' {
+			break
+		}
 		msg = append(msg, c)
 		l--
 	}
@@ -573,7 +637,7 @@ func parseGitCommit(data <-chan byte) (GitCommit, error) {
 		var hdr [2]string
 		hdr, size, err := parseGitCommitHdr(hdr, data)
 		if err != nil {
-			return GitCommit{}, nil
+			return GitCommit{}, err
 		}
 		l -= size
 
@@ -594,14 +658,28 @@ func parseGitCommit(data <-chan byte) (GitCommit, error) {
 func parseTreeEntry(data <-chan byte, hashLen int) (GitTreeEntry, error) {
 	var e GitTreeEntry
 
-	for c := <-data; c != ' '; c = <-data {
+	for {
+		c, ok := <-data
+		if !ok {
+			return e, io.EOF
+		}
+		if c == ' ' {
+			break
+		}
 		e.mode = e.mode*8 + int(c-'0')
 		e.size++
 	}
 	e.size++
 
 	name := make([]byte, 0, 128)
-	for c := <-data; c != '\x00'; c = <-data {
+	for {
+		c, ok := <-data
+		if !ok {
+			return e, io.EOF
+		}
+		if c == '\x00' {
+			break
+		}
 		name = append(name, c)
 		e.size++
 	}
@@ -612,7 +690,10 @@ func parseTreeEntry(data <-chan byte, hashLen int) (GitTreeEntry, error) {
 
 	hash := make([]byte, 0, hashLen*2)
 	for range hashLen {
-		c := <-data
+		c, ok := <-data
+		if !ok {
+			return e, io.EOF
+		}
 		hash = append(hash, hexBinToAscii[((c&0xF0)>>4)], hexBinToAscii[c&0xF])
 	}
 	e.hash = string(hash)
@@ -633,13 +714,16 @@ func parseGitTree(data <-chan byte) (GitTree, error) {
 	for parsedLen < hdr.size {
 		entry, err := parseTreeEntry(data, len(hdr.hash)/2)
 		if err != nil {
-			return GitTree{}, nil
+			return GitTree{}, err
 		}
 
 		t.items = append(t.items, entry)
 		parsedLen += entry.size
 	}
-	c := <-data // \0 read
+	c, ok := <-data // \0 read
+	if !ok {
+		return t, io.EOF
+	}
 
 	if c != '\x00' {
 		return t, fmt.Errorf("Unexpected character during git tree data read")
@@ -660,9 +744,16 @@ func parseGitBlob(data <-chan byte) ([]byte, error) {
 
 	d := make([]byte, hdr.size)
 	for l := 0; l < hdr.size; l++ {
-		d[l] = <-data
+		var ok bool
+		d[l], ok = <-data
+		if !ok {
+			return d, io.EOF
+		}
+	}
+	eob, ok := <-data
+	if !ok {
+		return d, io.EOF
 	}
-	eob := <-data
 	if eob != '\x00' {
 		return d, fmt.Errorf("invalid byte read in parseGitBlob")
 	}
@@ -674,16 +765,25 @@ func (e *GitHandlerImpl) GitParseCommits(cwd string, commitIDs []string) (parsed
 	var done sync.Mutex
 
 	done.Lock()
-	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
+	done_signal := make(chan struct{})
+	var once sync.Once
+	close_done := func() {
+		once.Do(func() {
+			close(done_signal)
+		})
+	}
+
+	data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
 	parsedCommits = make([]GitCommit, 0, len(commitIDs))
 
 	go func() {
 		defer done.Unlock()
+		defer close_done()
 		defer close(data_out.ch)
 
 		for _, id := range commitIDs {
 			data_out.Write([]byte(id))
-			data_out.ch <- '\x00'
+			data_out.Write([]byte{0})
 			c, e := parseGitCommit(data_in.ch)
 			if e != nil {
 				err = fmt.Errorf("Error parsing git commit: %w", e)
@@ -710,12 +810,14 @@ func (e *GitHandlerImpl) GitParseCommits(cwd string, commitIDs []string) (parsed
 	LogDebug("command run:", cmd.Args)
 	if e := cmd.Run(); e != nil {
 		LogError(e)
+		close_done()
 		close(data_in.ch)
-		close(data_out.ch)
 		return nil, e
 	}
 
 	done.Lock()
+	close_done()
+	close(data_in.ch)
 	return
 }
 
@@ -724,15 +826,21 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
 	var done sync.Mutex
 
 	done.Lock()
-	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
+	done_signal := make(chan struct{})
+	var once sync.Once
+	close_done := func() {
+		once.Do(func() {
+			close(done_signal)
+		})
+	}
+	data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
 
 	go func() {
 		defer done.Unlock()
+		defer close_done()
 		defer close(data_out.ch)
-
 		data_out.Write([]byte(commitId))
-		data_out.ch <- '\x00'
-
+		data_out.Write([]byte{0})
 		var c GitCommit
 		c, err = parseGitCommit(data_in.ch)
 		if err != nil {
@@ -740,11 +848,9 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
 			return
 		}
 		data_out.Write([]byte(c.Tree))
-		data_out.ch <- '\x00'
-
+		data_out.Write([]byte{0})
 		var tree GitTree
 		tree, err = parseGitTree(data_in.ch)
-
 		if err != nil {
 			LogError("Error parsing git tree:", err)
 			return
@@ -754,7 +860,7 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
 			if te.isBlob() && te.name == filename {
 				LogInfo("blob", te.hash)
 				data_out.Write([]byte(te.hash))
-				data_out.ch <- '\x00'
+				data_out.Write([]byte{0})
 				data, err = parseGitBlob(data_in.ch)
 				return
 			}
@@ -779,11 +885,13 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
 	LogDebug("command run:", cmd.Args)
 	if e := cmd.Run(); e != nil {
 		LogError(e)
+		close_done()
 		close(data_in.ch)
-		close(data_out.ch)
 		return nil, e
 	}
 	done.Lock()
+	close_done()
+	close(data_in.ch)
 	return
 }
 
@@ -793,16 +901,24 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
 	directoryList = make(map[string]string)
 
 	done.Lock()
-	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
+	done_signal := make(chan struct{})
+	var once sync.Once
+	close_done := func() {
+		once.Do(func() {
+			close(done_signal)
+		})
+	}
+	data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
 
 	LogDebug("Getting directory for:", commitId)
 
 	go func() {
 		defer done.Unlock()
+		defer close_done()
 		defer close(data_out.ch)
 
 		data_out.Write([]byte(commitId))
-		data_out.ch <- '\x00'
+		data_out.Write([]byte{0})
 		var c GitCommit
 		c, err = parseGitCommit(data_in.ch)
 		if err != nil {
@@ -818,7 +934,7 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
 				delete(trees, p)
 
 				data_out.Write([]byte(tree))
-				data_out.ch <- '\x00'
+				data_out.Write([]byte{0})
 				var tree GitTree
 				tree, err = parseGitTree(data_in.ch)
 
@@ -852,12 +968,14 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
 	LogDebug("command run:", cmd.Args)
 	if e := cmd.Run(); e != nil {
 		LogError(e)
+		close_done()
 		close(data_in.ch)
-		close(data_out.ch)
 		return directoryList, e
 	}
 
 	done.Lock()
+	close_done()
+	close(data_in.ch)
 	return directoryList, err
 }
 
@@ -867,16 +985,24 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
 	submoduleList = make(map[string]string)
 
 	done.Lock()
-	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
+	done_signal := make(chan struct{})
+	var once sync.Once
+	close_done := func() {
+		once.Do(func() {
+			close(done_signal)
+		})
+	}
+	data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
 
 	LogDebug("Getting submodules for:", commitId)
 
 	go func() {
 		defer done.Unlock()
+		defer close_done()
 		defer close(data_out.ch)
 
 		data_out.Write([]byte(commitId))
-		data_out.ch <- '\x00'
+		data_out.Write([]byte{0})
 		var c GitCommit
 		c, err = parseGitCommit(data_in.ch)
 		if err != nil {
@@ -892,7 +1018,7 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
 				delete(trees, p)
 
 				data_out.Write([]byte(tree))
-				data_out.ch <- '\x00'
+				data_out.Write([]byte{0})
 				var tree GitTree
 				tree, err = parseGitTree(data_in.ch)
 
@@ -929,17 +1055,26 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
 	LogDebug("command run:", cmd.Args)
 	if e := cmd.Run(); e != nil {
 		LogError(e)
+		close_done()
 		close(data_in.ch)
-		close(data_out.ch)
 		return submoduleList, e
 	}
 
 	done.Lock()
+	close_done()
+	close(data_in.ch)
 	return submoduleList, err
 }
 
 func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string) (subCommitId string, valid bool) {
-	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
+	done_signal := make(chan struct{})
+	var once sync.Once
+	close_done := func() {
+		once.Do(func() {
+			close(done_signal)
+		})
+	}
+	data_in, data_out := ChanIO{make(chan byte), done_signal}, ChanIO{make(chan byte), done_signal}
 	var wg sync.WaitGroup
 
 	wg.Add(1)
@@ -955,17 +1090,18 @@ func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string)
 		}()
 
 		defer wg.Done()
+		defer close_done()
 		defer close(data_out.ch)
 
 		data_out.Write([]byte(commitId))
-		data_out.ch <- '\x00'
+		data_out.Write([]byte{0})
 		c, err := parseGitCommit(data_in.ch)
 		if err != nil {
 			LogError("Error parsing git commit:", err)
 			panic(err)
 		}
 		data_out.Write([]byte(c.Tree))
-		data_out.ch <- '\x00'
+		data_out.Write([]byte{0})
 		tree, err := parseGitTree(data_in.ch)
 
 		if err != nil {
@@ -997,12 +1133,14 @@ func (e *GitHandlerImpl) GitSubmoduleCommitId(cwd, packageName, commitId string)
 	LogDebug("command run:", cmd.Args)
 	if e := cmd.Run(); e != nil {
 		LogError(e)
+		close_done()
 		close(data_in.ch)
-		close(data_out.ch)
 		return subCommitId, false
 	}
 
 	wg.Wait()
+	close_done()
+	close(data_in.ch)
 	return subCommitId, len(subCommitId) > 0
 }
 

common/git_utils_test.go

@@ -27,6 +27,7 @@ import (
 	"slices"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestGitClone(t *testing.T) {
@@ -596,3 +597,47 @@ func TestGitStatusParse(t *testing.T) {
 		})
 	}
 }
+
+func TestGitDeadlockFix(t *testing.T) {
+	gitDir := t.TempDir()
+	testDir, _ := os.Getwd()
+
+	cmd := exec.Command("/usr/bin/bash", path.Join(testDir, "tsetup.sh"))
+	cmd.Dir = gitDir
+	_, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gh, err := AllocateGitWorkTree(gitDir, "Test", "test@example.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+	h, err := gh.ReadExistingPath(".")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer h.Close()
+
+	// Use a blob ID to trigger error in GitParseCommits
+	// This ensures that the function returns error immediately and doesn't deadlock
+	blobId := "81aba862107f1e2f5312e165453955485f424612f313d6c2fb1b31fef9f82a14"
+
+	done := make(chan error)
+	go func() {
+		_, err := h.GitParseCommits("", []string{blobId})
+		done <- err
+	}()
+
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Error("Expected error from GitParseCommits with blob ID, got nil")
+		} else {
+			// This is expected
+			t.Logf("Got expected error: %v", err)
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("GitParseCommits deadlocked! Fix is NOT working.")
+	}
+}

common/gitea_utils.go

@@ -100,9 +100,10 @@ type GiteaPRUpdater interface {
 	UpdatePullRequest(org, repo string, num int64, options *models.EditPullRequestOption) (*models.PullRequest, error)
 }
 
-type GiteaPRTimelineFetcher interface {
+type GiteaPRTimelineReviewFetcher interface {
 	GiteaPRFetcher
 	GiteaTimelineFetcher
+	GiteaReviewFetcher
 }
 
 type GiteaCommitFetcher interface {
@@ -128,10 +129,16 @@ type GiteaPRChecker interface {
 	GiteaMaintainershipReader
 }
 
-type GiteaReviewFetcherAndRequester interface {
+type GiteaReviewFetcherAndRequesterAndUnrequester interface {
 	GiteaReviewTimelineFetcher
 	GiteaCommentFetcher
 	GiteaReviewRequester
+	GiteaReviewUnrequester
+}
+
+type GiteaUnreviewTimelineFetcher interface {
+	GiteaTimelineFetcher
+	GiteaReviewUnrequester
 }
 
 type GiteaReviewRequester interface {
@@ -806,6 +813,7 @@ type TimelineCacheData struct {
 var giteaTimelineCache map[string]TimelineCacheData = make(map[string]TimelineCacheData)
 var giteaTimelineCacheMutex sync.RWMutex
 
+// returns timeline in reverse chronological create order
 func (gitea *GiteaTransport) GetTimeline(org, repo string, idx int64) ([]*models.TimelineComment, error) {
 	page := int64(1)
 	resCount := 1

common/mock/gitea_utils.go

@@ -562,32 +562,32 @@ func (c *MockGiteaPRUpdaterUpdatePullRequestCall) DoAndReturn(f func(string, str
 	return c
 }
 
-// MockGiteaPRTimelineFetcher is a mock of GiteaPRTimelineFetcher interface.
-type MockGiteaPRTimelineFetcher struct {
+// MockGiteaPRTimelineReviewFetcher is a mock of GiteaPRTimelineReviewFetcher interface.
+type MockGiteaPRTimelineReviewFetcher struct {
 	ctrl     *gomock.Controller
-	recorder *MockGiteaPRTimelineFetcherMockRecorder
+	recorder *MockGiteaPRTimelineReviewFetcherMockRecorder
 	isgomock struct{}
 }
 
-// MockGiteaPRTimelineFetcherMockRecorder is the mock recorder for MockGiteaPRTimelineFetcher.
-type MockGiteaPRTimelineFetcherMockRecorder struct {
-	mock *MockGiteaPRTimelineFetcher
+// MockGiteaPRTimelineReviewFetcherMockRecorder is the mock recorder for MockGiteaPRTimelineReviewFetcher.
+type MockGiteaPRTimelineReviewFetcherMockRecorder struct {
+	mock *MockGiteaPRTimelineReviewFetcher
 }
 
-// NewMockGiteaPRTimelineFetcher creates a new mock instance.
-func NewMockGiteaPRTimelineFetcher(ctrl *gomock.Controller) *MockGiteaPRTimelineFetcher {
-	mock := &MockGiteaPRTimelineFetcher{ctrl: ctrl}
-	mock.recorder = &MockGiteaPRTimelineFetcherMockRecorder{mock}
+// NewMockGiteaPRTimelineReviewFetcher creates a new mock instance.
+func NewMockGiteaPRTimelineReviewFetcher(ctrl *gomock.Controller) *MockGiteaPRTimelineReviewFetcher {
+	mock := &MockGiteaPRTimelineReviewFetcher{ctrl: ctrl}
+	mock.recorder = &MockGiteaPRTimelineReviewFetcherMockRecorder{mock}
 	return mock
 }
 
 // EXPECT returns an object that allows the caller to indicate expected use.
-func (m *MockGiteaPRTimelineFetcher) EXPECT() *MockGiteaPRTimelineFetcherMockRecorder {
+func (m *MockGiteaPRTimelineReviewFetcher) EXPECT() *MockGiteaPRTimelineReviewFetcherMockRecorder {
 	return m.recorder
 }
 
 // GetPullRequest mocks base method.
-func (m *MockGiteaPRTimelineFetcher) GetPullRequest(org, project string, num int64) (*models.PullRequest, error) {
+func (m *MockGiteaPRTimelineReviewFetcher) GetPullRequest(org, project string, num int64) (*models.PullRequest, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "GetPullRequest", org, project, num)
 	ret0, _ := ret[0].(*models.PullRequest)
@@ -596,37 +596,76 @@ func (m *MockGiteaPRTimelineFetcher) GetPullRequest(org, project string, num int
 }
 
 // GetPullRequest indicates an expected call of GetPullRequest.
-func (mr *MockGiteaPRTimelineFetcherMockRecorder) GetPullRequest(org, project, num any) *MockGiteaPRTimelineFetcherGetPullRequestCall {
+func (mr *MockGiteaPRTimelineReviewFetcherMockRecorder) GetPullRequest(org, project, num any) *MockGiteaPRTimelineReviewFetcherGetPullRequestCall {
 	mr.mock.ctrl.T.Helper()
-	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPullRequest", reflect.TypeOf((*MockGiteaPRTimelineFetcher)(nil).GetPullRequest), org, project, num)
-	return &MockGiteaPRTimelineFetcherGetPullRequestCall{Call: call}
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPullRequest", reflect.TypeOf((*MockGiteaPRTimelineReviewFetcher)(nil).GetPullRequest), org, project, num)
+	return &MockGiteaPRTimelineReviewFetcherGetPullRequestCall{Call: call}
 }
 
-// MockGiteaPRTimelineFetcherGetPullRequestCall wrap *gomock.Call
-type MockGiteaPRTimelineFetcherGetPullRequestCall struct {
+// MockGiteaPRTimelineReviewFetcherGetPullRequestCall wrap *gomock.Call
+type MockGiteaPRTimelineReviewFetcherGetPullRequestCall struct {
 	*gomock.Call
 }
 
 // Return rewrite *gomock.Call.Return
-func (c *MockGiteaPRTimelineFetcherGetPullRequestCall) Return(arg0 *models.PullRequest, arg1 error) *MockGiteaPRTimelineFetcherGetPullRequestCall {
+func (c *MockGiteaPRTimelineReviewFetcherGetPullRequestCall) Return(arg0 *models.PullRequest, arg1 error) *MockGiteaPRTimelineReviewFetcherGetPullRequestCall {
 	c.Call = c.Call.Return(arg0, arg1)
 	return c
 }
 
 // Do rewrite *gomock.Call.Do
-func (c *MockGiteaPRTimelineFetcherGetPullRequestCall) Do(f func(string, string, int64) (*models.PullRequest, error)) *MockGiteaPRTimelineFetcherGetPullRequestCall {
+func (c *MockGiteaPRTimelineReviewFetcherGetPullRequestCall) Do(f func(string, string, int64) (*models.PullRequest, error)) *MockGiteaPRTimelineReviewFetcherGetPullRequestCall {
 	c.Call = c.Call.Do(f)
 	return c
 }
 
 // DoAndReturn rewrite *gomock.Call.DoAndReturn
-func (c *MockGiteaPRTimelineFetcherGetPullRequestCall) DoAndReturn(f func(string, string, int64) (*models.PullRequest, error)) *MockGiteaPRTimelineFetcherGetPullRequestCall {
+func (c *MockGiteaPRTimelineReviewFetcherGetPullRequestCall) DoAndReturn(f func(string, string, int64) (*models.PullRequest, error)) *MockGiteaPRTimelineReviewFetcherGetPullRequestCall {
+	c.Call = c.Call.DoAndReturn(f)
+	return c
+}
+
+// GetPullRequestReviews mocks base method.
+func (m *MockGiteaPRTimelineReviewFetcher) GetPullRequestReviews(org, project string, PRnum int64) ([]*models.PullReview, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "GetPullRequestReviews", org, project, PRnum)
+	ret0, _ := ret[0].([]*models.PullReview)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// GetPullRequestReviews indicates an expected call of GetPullRequestReviews.
+func (mr *MockGiteaPRTimelineReviewFetcherMockRecorder) GetPullRequestReviews(org, project, PRnum any) *MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall {
+	mr.mock.ctrl.T.Helper()
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPullRequestReviews", reflect.TypeOf((*MockGiteaPRTimelineReviewFetcher)(nil).GetPullRequestReviews), org, project, PRnum)
+	return &MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall{Call: call}
+}
+
+// MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall wrap *gomock.Call
+type MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall struct {
+	*gomock.Call
+}
+
+// Return rewrite *gomock.Call.Return
+func (c *MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall) Return(arg0 []*models.PullReview, arg1 error) *MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall {
+	c.Call = c.Call.Return(arg0, arg1)
+	return c
+}
+
+// Do rewrite *gomock.Call.Do
+func (c *MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall) Do(f func(string, string, int64) ([]*models.PullReview, error)) *MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall {
+	c.Call = c.Call.Do(f)
+	return c
+}
+
+// DoAndReturn rewrite *gomock.Call.DoAndReturn
+func (c *MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall) DoAndReturn(f func(string, string, int64) ([]*models.PullReview, error)) *MockGiteaPRTimelineReviewFetcherGetPullRequestReviewsCall {
 	c.Call = c.Call.DoAndReturn(f)
 	return c
 }
 
 // GetTimeline mocks base method.
-func (m *MockGiteaPRTimelineFetcher) GetTimeline(org, repo string, idx int64) ([]*models.TimelineComment, error) {
+func (m *MockGiteaPRTimelineReviewFetcher) GetTimeline(org, repo string, idx int64) ([]*models.TimelineComment, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "GetTimeline", org, repo, idx)
 	ret0, _ := ret[0].([]*models.TimelineComment)
@@ -635,31 +674,31 @@ func (m *MockGiteaPRTimelineFetcher) GetTimeline(org, repo string, idx int64) ([
 }
 
 // GetTimeline indicates an expected call of GetTimeline.
-func (mr *MockGiteaPRTimelineFetcherMockRecorder) GetTimeline(org, repo, idx any) *MockGiteaPRTimelineFetcherGetTimelineCall {
+func (mr *MockGiteaPRTimelineReviewFetcherMockRecorder) GetTimeline(org, repo, idx any) *MockGiteaPRTimelineReviewFetcherGetTimelineCall {
 	mr.mock.ctrl.T.Helper()
-	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTimeline", reflect.TypeOf((*MockGiteaPRTimelineFetcher)(nil).GetTimeline), org, repo, idx)
-	return &MockGiteaPRTimelineFetcherGetTimelineCall{Call: call}
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTimeline", reflect.TypeOf((*MockGiteaPRTimelineReviewFetcher)(nil).GetTimeline), org, repo, idx)
+	return &MockGiteaPRTimelineReviewFetcherGetTimelineCall{Call: call}
 }
 
-// MockGiteaPRTimelineFetcherGetTimelineCall wrap *gomock.Call
-type MockGiteaPRTimelineFetcherGetTimelineCall struct {
+// MockGiteaPRTimelineReviewFetcherGetTimelineCall wrap *gomock.Call
+type MockGiteaPRTimelineReviewFetcherGetTimelineCall struct {
 	*gomock.Call
 }
 
 // Return rewrite *gomock.Call.Return
-func (c *MockGiteaPRTimelineFetcherGetTimelineCall) Return(arg0 []*models.TimelineComment, arg1 error) *MockGiteaPRTimelineFetcherGetTimelineCall {
+func (c *MockGiteaPRTimelineReviewFetcherGetTimelineCall) Return(arg0 []*models.TimelineComment, arg1 error) *MockGiteaPRTimelineReviewFetcherGetTimelineCall {
 	c.Call = c.Call.Return(arg0, arg1)
 	return c
 }
 
 // Do rewrite *gomock.Call.Do
-func (c *MockGiteaPRTimelineFetcherGetTimelineCall) Do(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaPRTimelineFetcherGetTimelineCall {
+func (c *MockGiteaPRTimelineReviewFetcherGetTimelineCall) Do(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaPRTimelineReviewFetcherGetTimelineCall {
 	c.Call = c.Call.Do(f)
 	return c
 }
 
 // DoAndReturn rewrite *gomock.Call.DoAndReturn
-func (c *MockGiteaPRTimelineFetcherGetTimelineCall) DoAndReturn(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaPRTimelineFetcherGetTimelineCall {
+func (c *MockGiteaPRTimelineReviewFetcherGetTimelineCall) DoAndReturn(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaPRTimelineReviewFetcherGetTimelineCall {
 	c.Call = c.Call.DoAndReturn(f)
 	return c
 }
@@ -1176,32 +1215,32 @@ func (c *MockGiteaPRCheckerGetTimelineCall) DoAndReturn(f func(string, string, i
 	return c
 }
 
-// MockGiteaReviewFetcherAndRequester is a mock of GiteaReviewFetcherAndRequester interface.
-type MockGiteaReviewFetcherAndRequester struct {
+// MockGiteaReviewFetcherAndRequesterAndUnrequester is a mock of GiteaReviewFetcherAndRequesterAndUnrequester interface.
+type MockGiteaReviewFetcherAndRequesterAndUnrequester struct {
 	ctrl     *gomock.Controller
-	recorder *MockGiteaReviewFetcherAndRequesterMockRecorder
+	recorder *MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder
 	isgomock struct{}
 }
 
-// MockGiteaReviewFetcherAndRequesterMockRecorder is the mock recorder for MockGiteaReviewFetcherAndRequester.
-type MockGiteaReviewFetcherAndRequesterMockRecorder struct {
-	mock *MockGiteaReviewFetcherAndRequester
+// MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder is the mock recorder for MockGiteaReviewFetcherAndRequesterAndUnrequester.
+type MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder struct {
+	mock *MockGiteaReviewFetcherAndRequesterAndUnrequester
 }
 
-// NewMockGiteaReviewFetcherAndRequester creates a new mock instance.
-func NewMockGiteaReviewFetcherAndRequester(ctrl *gomock.Controller) *MockGiteaReviewFetcherAndRequester {
-	mock := &MockGiteaReviewFetcherAndRequester{ctrl: ctrl}
-	mock.recorder = &MockGiteaReviewFetcherAndRequesterMockRecorder{mock}
+// NewMockGiteaReviewFetcherAndRequesterAndUnrequester creates a new mock instance.
+func NewMockGiteaReviewFetcherAndRequesterAndUnrequester(ctrl *gomock.Controller) *MockGiteaReviewFetcherAndRequesterAndUnrequester {
+	mock := &MockGiteaReviewFetcherAndRequesterAndUnrequester{ctrl: ctrl}
+	mock.recorder = &MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder{mock}
 	return mock
 }
 
 // EXPECT returns an object that allows the caller to indicate expected use.
-func (m *MockGiteaReviewFetcherAndRequester) EXPECT() *MockGiteaReviewFetcherAndRequesterMockRecorder {
+func (m *MockGiteaReviewFetcherAndRequesterAndUnrequester) EXPECT() *MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder {
 	return m.recorder
 }
 
 // GetIssueComments mocks base method.
-func (m *MockGiteaReviewFetcherAndRequester) GetIssueComments(org, project string, issueNo int64) ([]*models.Comment, error) {
+func (m *MockGiteaReviewFetcherAndRequesterAndUnrequester) GetIssueComments(org, project string, issueNo int64) ([]*models.Comment, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "GetIssueComments", org, project, issueNo)
 	ret0, _ := ret[0].([]*models.Comment)
@@ -1210,37 +1249,37 @@ func (m *MockGiteaReviewFetcherAndRequester) GetIssueComments(org, project strin
 }
 
 // GetIssueComments indicates an expected call of GetIssueComments.
-func (mr *MockGiteaReviewFetcherAndRequesterMockRecorder) GetIssueComments(org, project, issueNo any) *MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall {
+func (mr *MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder) GetIssueComments(org, project, issueNo any) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall {
 	mr.mock.ctrl.T.Helper()
-	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetIssueComments", reflect.TypeOf((*MockGiteaReviewFetcherAndRequester)(nil).GetIssueComments), org, project, issueNo)
-	return &MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall{Call: call}
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetIssueComments", reflect.TypeOf((*MockGiteaReviewFetcherAndRequesterAndUnrequester)(nil).GetIssueComments), org, project, issueNo)
+	return &MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall{Call: call}
 }
 
-// MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall wrap *gomock.Call
-type MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall struct {
+// MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall wrap *gomock.Call
+type MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall struct {
 	*gomock.Call
 }
 
 // Return rewrite *gomock.Call.Return
-func (c *MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall) Return(arg0 []*models.Comment, arg1 error) *MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall) Return(arg0 []*models.Comment, arg1 error) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall {
 	c.Call = c.Call.Return(arg0, arg1)
 	return c
 }
 
 // Do rewrite *gomock.Call.Do
-func (c *MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall) Do(f func(string, string, int64) ([]*models.Comment, error)) *MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall) Do(f func(string, string, int64) ([]*models.Comment, error)) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall {
 	c.Call = c.Call.Do(f)
 	return c
 }
 
 // DoAndReturn rewrite *gomock.Call.DoAndReturn
-func (c *MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall) DoAndReturn(f func(string, string, int64) ([]*models.Comment, error)) *MockGiteaReviewFetcherAndRequesterGetIssueCommentsCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall) DoAndReturn(f func(string, string, int64) ([]*models.Comment, error)) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetIssueCommentsCall {
 	c.Call = c.Call.DoAndReturn(f)
 	return c
 }
 
 // GetPullRequestReviews mocks base method.
-func (m *MockGiteaReviewFetcherAndRequester) GetPullRequestReviews(org, project string, PRnum int64) ([]*models.PullReview, error) {
+func (m *MockGiteaReviewFetcherAndRequesterAndUnrequester) GetPullRequestReviews(org, project string, PRnum int64) ([]*models.PullReview, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "GetPullRequestReviews", org, project, PRnum)
 	ret0, _ := ret[0].([]*models.PullReview)
@@ -1249,37 +1288,37 @@ func (m *MockGiteaReviewFetcherAndRequester) GetPullRequestReviews(org, project
 }
 
 // GetPullRequestReviews indicates an expected call of GetPullRequestReviews.
-func (mr *MockGiteaReviewFetcherAndRequesterMockRecorder) GetPullRequestReviews(org, project, PRnum any) *MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall {
+func (mr *MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder) GetPullRequestReviews(org, project, PRnum any) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall {
 	mr.mock.ctrl.T.Helper()
-	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPullRequestReviews", reflect.TypeOf((*MockGiteaReviewFetcherAndRequester)(nil).GetPullRequestReviews), org, project, PRnum)
-	return &MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall{Call: call}
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPullRequestReviews", reflect.TypeOf((*MockGiteaReviewFetcherAndRequesterAndUnrequester)(nil).GetPullRequestReviews), org, project, PRnum)
+	return &MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall{Call: call}
 }
 
-// MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall wrap *gomock.Call
-type MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall struct {
+// MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall wrap *gomock.Call
+type MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall struct {
 	*gomock.Call
 }
 
 // Return rewrite *gomock.Call.Return
-func (c *MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall) Return(arg0 []*models.PullReview, arg1 error) *MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall) Return(arg0 []*models.PullReview, arg1 error) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall {
 	c.Call = c.Call.Return(arg0, arg1)
 	return c
 }
 
 // Do rewrite *gomock.Call.Do
-func (c *MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall) Do(f func(string, string, int64) ([]*models.PullReview, error)) *MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall) Do(f func(string, string, int64) ([]*models.PullReview, error)) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall {
 	c.Call = c.Call.Do(f)
 	return c
 }
 
 // DoAndReturn rewrite *gomock.Call.DoAndReturn
-func (c *MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall) DoAndReturn(f func(string, string, int64) ([]*models.PullReview, error)) *MockGiteaReviewFetcherAndRequesterGetPullRequestReviewsCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall) DoAndReturn(f func(string, string, int64) ([]*models.PullReview, error)) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetPullRequestReviewsCall {
 	c.Call = c.Call.DoAndReturn(f)
 	return c
 }
 
 // GetTimeline mocks base method.
-func (m *MockGiteaReviewFetcherAndRequester) GetTimeline(org, repo string, idx int64) ([]*models.TimelineComment, error) {
+func (m *MockGiteaReviewFetcherAndRequesterAndUnrequester) GetTimeline(org, repo string, idx int64) ([]*models.TimelineComment, error) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "GetTimeline", org, repo, idx)
 	ret0, _ := ret[0].([]*models.TimelineComment)
@@ -1288,37 +1327,37 @@ func (m *MockGiteaReviewFetcherAndRequester) GetTimeline(org, repo string, idx i
 }
 
 // GetTimeline indicates an expected call of GetTimeline.
-func (mr *MockGiteaReviewFetcherAndRequesterMockRecorder) GetTimeline(org, repo, idx any) *MockGiteaReviewFetcherAndRequesterGetTimelineCall {
+func (mr *MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder) GetTimeline(org, repo, idx any) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall {
 	mr.mock.ctrl.T.Helper()
-	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTimeline", reflect.TypeOf((*MockGiteaReviewFetcherAndRequester)(nil).GetTimeline), org, repo, idx)
-	return &MockGiteaReviewFetcherAndRequesterGetTimelineCall{Call: call}
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTimeline", reflect.TypeOf((*MockGiteaReviewFetcherAndRequesterAndUnrequester)(nil).GetTimeline), org, repo, idx)
+	return &MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall{Call: call}
 }
 
-// MockGiteaReviewFetcherAndRequesterGetTimelineCall wrap *gomock.Call
-type MockGiteaReviewFetcherAndRequesterGetTimelineCall struct {
+// MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall wrap *gomock.Call
+type MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall struct {
 	*gomock.Call
 }
 
 // Return rewrite *gomock.Call.Return
-func (c *MockGiteaReviewFetcherAndRequesterGetTimelineCall) Return(arg0 []*models.TimelineComment, arg1 error) *MockGiteaReviewFetcherAndRequesterGetTimelineCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall) Return(arg0 []*models.TimelineComment, arg1 error) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall {
 	c.Call = c.Call.Return(arg0, arg1)
 	return c
 }
 
 // Do rewrite *gomock.Call.Do
-func (c *MockGiteaReviewFetcherAndRequesterGetTimelineCall) Do(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaReviewFetcherAndRequesterGetTimelineCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall) Do(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall {
 	c.Call = c.Call.Do(f)
 	return c
 }
 
 // DoAndReturn rewrite *gomock.Call.DoAndReturn
-func (c *MockGiteaReviewFetcherAndRequesterGetTimelineCall) DoAndReturn(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaReviewFetcherAndRequesterGetTimelineCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall) DoAndReturn(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaReviewFetcherAndRequesterAndUnrequesterGetTimelineCall {
 	c.Call = c.Call.DoAndReturn(f)
 	return c
 }
 
 // RequestReviews mocks base method.
-func (m *MockGiteaReviewFetcherAndRequester) RequestReviews(pr *models.PullRequest, reviewer ...string) ([]*models.PullReview, error) {
+func (m *MockGiteaReviewFetcherAndRequesterAndUnrequester) RequestReviews(pr *models.PullRequest, reviewer ...string) ([]*models.PullReview, error) {
 	m.ctrl.T.Helper()
 	varargs := []any{pr}
 	for _, a := range reviewer {
@@ -1331,32 +1370,181 @@ func (m *MockGiteaReviewFetcherAndRequester) RequestReviews(pr *models.PullReque
 }
 
 // RequestReviews indicates an expected call of RequestReviews.
-func (mr *MockGiteaReviewFetcherAndRequesterMockRecorder) RequestReviews(pr any, reviewer ...any) *MockGiteaReviewFetcherAndRequesterRequestReviewsCall {
+func (mr *MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder) RequestReviews(pr any, reviewer ...any) *MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall {
 	mr.mock.ctrl.T.Helper()
 	varargs := append([]any{pr}, reviewer...)
-	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RequestReviews", reflect.TypeOf((*MockGiteaReviewFetcherAndRequester)(nil).RequestReviews), varargs...)
-	return &MockGiteaReviewFetcherAndRequesterRequestReviewsCall{Call: call}
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RequestReviews", reflect.TypeOf((*MockGiteaReviewFetcherAndRequesterAndUnrequester)(nil).RequestReviews), varargs...)
+	return &MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall{Call: call}
 }
 
-// MockGiteaReviewFetcherAndRequesterRequestReviewsCall wrap *gomock.Call
-type MockGiteaReviewFetcherAndRequesterRequestReviewsCall struct {
+// MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall wrap *gomock.Call
+type MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall struct {
 	*gomock.Call
 }
 
 // Return rewrite *gomock.Call.Return
-func (c *MockGiteaReviewFetcherAndRequesterRequestReviewsCall) Return(arg0 []*models.PullReview, arg1 error) *MockGiteaReviewFetcherAndRequesterRequestReviewsCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall) Return(arg0 []*models.PullReview, arg1 error) *MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall {
 	c.Call = c.Call.Return(arg0, arg1)
 	return c
 }
 
 // Do rewrite *gomock.Call.Do
-func (c *MockGiteaReviewFetcherAndRequesterRequestReviewsCall) Do(f func(*models.PullRequest, ...string) ([]*models.PullReview, error)) *MockGiteaReviewFetcherAndRequesterRequestReviewsCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall) Do(f func(*models.PullRequest, ...string) ([]*models.PullReview, error)) *MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall {
 	c.Call = c.Call.Do(f)
 	return c
 }
 
 // DoAndReturn rewrite *gomock.Call.DoAndReturn
-func (c *MockGiteaReviewFetcherAndRequesterRequestReviewsCall) DoAndReturn(f func(*models.PullRequest, ...string) ([]*models.PullReview, error)) *MockGiteaReviewFetcherAndRequesterRequestReviewsCall {
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall) DoAndReturn(f func(*models.PullRequest, ...string) ([]*models.PullReview, error)) *MockGiteaReviewFetcherAndRequesterAndUnrequesterRequestReviewsCall {
+	c.Call = c.Call.DoAndReturn(f)
+	return c
+}
+
+// UnrequestReview mocks base method.
+func (m *MockGiteaReviewFetcherAndRequesterAndUnrequester) UnrequestReview(org, repo string, id int64, reviwers ...string) error {
+	m.ctrl.T.Helper()
+	varargs := []any{org, repo, id}
+	for _, a := range reviwers {
+		varargs = append(varargs, a)
+	}
+	ret := m.ctrl.Call(m, "UnrequestReview", varargs...)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// UnrequestReview indicates an expected call of UnrequestReview.
+func (mr *MockGiteaReviewFetcherAndRequesterAndUnrequesterMockRecorder) UnrequestReview(org, repo, id any, reviwers ...any) *MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall {
+	mr.mock.ctrl.T.Helper()
+	varargs := append([]any{org, repo, id}, reviwers...)
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnrequestReview", reflect.TypeOf((*MockGiteaReviewFetcherAndRequesterAndUnrequester)(nil).UnrequestReview), varargs...)
+	return &MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall{Call: call}
+}
+
+// MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall wrap *gomock.Call
+type MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall struct {
+	*gomock.Call
+}
+
+// Return rewrite *gomock.Call.Return
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall) Return(arg0 error) *MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall {
+	c.Call = c.Call.Return(arg0)
+	return c
+}
+
+// Do rewrite *gomock.Call.Do
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall) Do(f func(string, string, int64, ...string) error) *MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall {
+	c.Call = c.Call.Do(f)
+	return c
+}
+
+// DoAndReturn rewrite *gomock.Call.DoAndReturn
+func (c *MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall) DoAndReturn(f func(string, string, int64, ...string) error) *MockGiteaReviewFetcherAndRequesterAndUnrequesterUnrequestReviewCall {
+	c.Call = c.Call.DoAndReturn(f)
+	return c
+}
+
+// MockGiteaUnreviewTimelineFetcher is a mock of GiteaUnreviewTimelineFetcher interface.
+type MockGiteaUnreviewTimelineFetcher struct {
+	ctrl     *gomock.Controller
+	recorder *MockGiteaUnreviewTimelineFetcherMockRecorder
+	isgomock struct{}
+}
+
+// MockGiteaUnreviewTimelineFetcherMockRecorder is the mock recorder for MockGiteaUnreviewTimelineFetcher.
+type MockGiteaUnreviewTimelineFetcherMockRecorder struct {
+	mock *MockGiteaUnreviewTimelineFetcher
+}
+
+// NewMockGiteaUnreviewTimelineFetcher creates a new mock instance.
+func NewMockGiteaUnreviewTimelineFetcher(ctrl *gomock.Controller) *MockGiteaUnreviewTimelineFetcher {
+	mock := &MockGiteaUnreviewTimelineFetcher{ctrl: ctrl}
+	mock.recorder = &MockGiteaUnreviewTimelineFetcherMockRecorder{mock}
+	return mock
+}
+
+// EXPECT returns an object that allows the caller to indicate expected use.
+func (m *MockGiteaUnreviewTimelineFetcher) EXPECT() *MockGiteaUnreviewTimelineFetcherMockRecorder {
+	return m.recorder
+}
+
+// GetTimeline mocks base method.
+func (m *MockGiteaUnreviewTimelineFetcher) GetTimeline(org, repo string, idx int64) ([]*models.TimelineComment, error) {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "GetTimeline", org, repo, idx)
+	ret0, _ := ret[0].([]*models.TimelineComment)
+	ret1, _ := ret[1].(error)
+	return ret0, ret1
+}
+
+// GetTimeline indicates an expected call of GetTimeline.
+func (mr *MockGiteaUnreviewTimelineFetcherMockRecorder) GetTimeline(org, repo, idx any) *MockGiteaUnreviewTimelineFetcherGetTimelineCall {
+	mr.mock.ctrl.T.Helper()
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTimeline", reflect.TypeOf((*MockGiteaUnreviewTimelineFetcher)(nil).GetTimeline), org, repo, idx)
+	return &MockGiteaUnreviewTimelineFetcherGetTimelineCall{Call: call}
+}
+
+// MockGiteaUnreviewTimelineFetcherGetTimelineCall wrap *gomock.Call
+type MockGiteaUnreviewTimelineFetcherGetTimelineCall struct {
+	*gomock.Call
+}
+
+// Return rewrite *gomock.Call.Return
+func (c *MockGiteaUnreviewTimelineFetcherGetTimelineCall) Return(arg0 []*models.TimelineComment, arg1 error) *MockGiteaUnreviewTimelineFetcherGetTimelineCall {
+	c.Call = c.Call.Return(arg0, arg1)
+	return c
+}
+
+// Do rewrite *gomock.Call.Do
+func (c *MockGiteaUnreviewTimelineFetcherGetTimelineCall) Do(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaUnreviewTimelineFetcherGetTimelineCall {
+	c.Call = c.Call.Do(f)
+	return c
+}
+
+// DoAndReturn rewrite *gomock.Call.DoAndReturn
+func (c *MockGiteaUnreviewTimelineFetcherGetTimelineCall) DoAndReturn(f func(string, string, int64) ([]*models.TimelineComment, error)) *MockGiteaUnreviewTimelineFetcherGetTimelineCall {
+	c.Call = c.Call.DoAndReturn(f)
+	return c
+}
+
+// UnrequestReview mocks base method.
+func (m *MockGiteaUnreviewTimelineFetcher) UnrequestReview(org, repo string, id int64, reviwers ...string) error {
+	m.ctrl.T.Helper()
+	varargs := []any{org, repo, id}
+	for _, a := range reviwers {
+		varargs = append(varargs, a)
+	}
+	ret := m.ctrl.Call(m, "UnrequestReview", varargs...)
+	ret0, _ := ret[0].(error)
+	return ret0
+}
+
+// UnrequestReview indicates an expected call of UnrequestReview.
+func (mr *MockGiteaUnreviewTimelineFetcherMockRecorder) UnrequestReview(org, repo, id any, reviwers ...any) *MockGiteaUnreviewTimelineFetcherUnrequestReviewCall {
+	mr.mock.ctrl.T.Helper()
+	varargs := append([]any{org, repo, id}, reviwers...)
+	call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnrequestReview", reflect.TypeOf((*MockGiteaUnreviewTimelineFetcher)(nil).UnrequestReview), varargs...)
+	return &MockGiteaUnreviewTimelineFetcherUnrequestReviewCall{Call: call}
+}
+
+// MockGiteaUnreviewTimelineFetcherUnrequestReviewCall wrap *gomock.Call
+type MockGiteaUnreviewTimelineFetcherUnrequestReviewCall struct {
+	*gomock.Call
+}
+
+// Return rewrite *gomock.Call.Return
+func (c *MockGiteaUnreviewTimelineFetcherUnrequestReviewCall) Return(arg0 error) *MockGiteaUnreviewTimelineFetcherUnrequestReviewCall {
+	c.Call = c.Call.Return(arg0)
+	return c
+}
+
+// Do rewrite *gomock.Call.Do
+func (c *MockGiteaUnreviewTimelineFetcherUnrequestReviewCall) Do(f func(string, string, int64, ...string) error) *MockGiteaUnreviewTimelineFetcherUnrequestReviewCall {
+	c.Call = c.Call.Do(f)
+	return c
+}
+
+// DoAndReturn rewrite *gomock.Call.DoAndReturn
+func (c *MockGiteaUnreviewTimelineFetcherUnrequestReviewCall) DoAndReturn(f func(string, string, int64, ...string) error) *MockGiteaUnreviewTimelineFetcherUnrequestReviewCall {
 	c.Call = c.Call.DoAndReturn(f)
 	return c
 }

common/pr.go

@@ -23,7 +23,8 @@ type PRSet struct {
 	PRs    []*PRInfo
 	Config *AutogitConfig
 
-	BotUser string
+	BotUser        string
+	HasAutoStaging bool
 }
 
 func (prinfo *PRInfo) PRComponents() (org string, repo string, idx int64) {
@@ -33,6 +34,41 @@ func (prinfo *PRInfo) PRComponents() (org string, repo string, idx int64) {
 	return
 }
 
+func (prinfo *PRInfo) RemoveReviewers(gitea GiteaUnreviewTimelineFetcher, Reviewers []string, BotUser string) {
+	org, repo, idx := prinfo.PRComponents()
+	tl, err := gitea.GetTimeline(org, repo, idx)
+	if err != nil {
+		LogError("Failed to fetch timeline for", PRtoString(prinfo.PR), err)
+	}
+
+	// find review request for each reviewer
+	ReviewersToUnrequest := Reviewers
+	ReviewersAlreadyChecked := []string{}
+
+	for _, tlc := range tl {
+		if tlc.Type == TimelineCommentType_ReviewRequested && tlc.Assignee != nil {
+			user := tlc.Assignee.UserName
+
+			if idx := slices.Index(ReviewersToUnrequest, user); idx >= 0 && !slices.Contains(ReviewersAlreadyChecked, user) {
+				if tlc.User != nil && tlc.User.UserName == BotUser {
+					ReviewersAlreadyChecked = append(ReviewersAlreadyChecked, user)
+					continue
+				}
+				ReviewersToUnrequest = slices.Delete(ReviewersToUnrequest, idx, idx+1)
+				if len(Reviewers) == 0 {
+					break
+				}
+			}
+		}
+	}
+
+	LogDebug("Unrequesting reviewes for", PRtoString(prinfo.PR), ReviewersToUnrequest)
+	err = gitea.UnrequestReview(org, repo, idx, ReviewersToUnrequest...)
+	if err != nil {
+		LogError("Failed to unrequest reviewers for", PRtoString(prinfo.PR), err)
+	}
+}
+
 func readPRData(gitea GiteaPRFetcher, pr *models.PullRequest, currentSet []*PRInfo, config *AutogitConfig) ([]*PRInfo, error) {
 	for _, p := range currentSet {
 		if pr.Index == p.PR.Index && pr.Base.Repo.Name == p.PR.Base.Repo.Name && pr.Base.Repo.Owner.UserName == p.PR.Base.Repo.Owner.UserName {
@@ -63,7 +99,7 @@ func readPRData(gitea GiteaPRFetcher, pr *models.PullRequest, currentSet []*PRIn
 
 var Timeline_RefIssueNotFound error = errors.New("RefIssue not found on the timeline")
 
-func LastPrjGitRefOnTimeline(botUser string, gitea GiteaPRTimelineFetcher, org, repo string, num int64, config *AutogitConfig) (*models.PullRequest, error) {
+func LastPrjGitRefOnTimeline(botUser string, gitea GiteaPRTimelineReviewFetcher, org, repo string, num int64, config *AutogitConfig) (*models.PullRequest, error) {
 	timeline, err := gitea.GetTimeline(org, repo, num)
 	if err != nil {
 		LogError("Failed to fetch timeline for", org, repo, "#", num, err)
@@ -88,14 +124,19 @@ func LastPrjGitRefOnTimeline(botUser string, gitea GiteaPRTimelineFetcher, org,
 			}
 
 			pr, err := gitea.GetPullRequest(prjGitOrg, prjGitRepo, issue.Index)
-			switch err.(type) {
-			case *repository.RepoGetPullRequestNotFound: // deleted?
-				continue
-			default:
-				LogDebug("PrjGit RefIssue fetch error from timeline", issue.Index, err)
+			if err != nil {
+				switch err.(type) {
+				case *repository.RepoGetPullRequestNotFound: // deleted?
+					continue
+				default:
+					LogDebug("PrjGit RefIssue fetch error from timeline", issue.Index, err)
+					continue
+				}
 			}
 
-			if pr.Base.Ref != prjGitBranch {
+			LogDebug("found ref PR on timeline:", PRtoString(pr))
+			if pr.Base.Name != prjGitBranch {
+				LogDebug(" -> not matching:", pr.Base.Name, prjGitBranch)
 				continue
 			}
 
@@ -115,7 +156,7 @@ func LastPrjGitRefOnTimeline(botUser string, gitea GiteaPRTimelineFetcher, org,
 	return nil, Timeline_RefIssueNotFound
 }
 
-func FetchPRSet(user string, gitea GiteaPRTimelineFetcher, org, repo string, num int64, config *AutogitConfig) (*PRSet, error) {
+func FetchPRSet(user string, gitea GiteaPRTimelineReviewFetcher, org, repo string, num int64, config *AutogitConfig) (*PRSet, error) {
 	var pr *models.PullRequest
 	var err error
 
@@ -141,6 +182,15 @@ func FetchPRSet(user string, gitea GiteaPRTimelineFetcher, org, repo string, num
 		return nil, err
 	}
 
+	for _, pr := range prs {
+		org, repo, idx := pr.PRComponents()
+		reviews, err := FetchGiteaReviews(gitea, org, repo, idx)
+		if err != nil {
+			LogError("Error fetching reviews for", PRtoString(pr.PR), ":", err)
+		}
+		pr.Reviews = reviews
+	}
+
 	return &PRSet{
 		PRs:     prs,
 		Config:  config,
@@ -148,6 +198,12 @@ func FetchPRSet(user string, gitea GiteaPRTimelineFetcher, org, repo string, num
 	}, nil
 }
 
+func (prset *PRSet) RemoveReviewers(gitea GiteaUnreviewTimelineFetcher, reviewers []string) {
+	for _, prinfo := range prset.PRs {
+		prinfo.RemoveReviewers(gitea, reviewers, prset.BotUser)
+	}
+}
+
 func (rs *PRSet) Find(pr *models.PullRequest) (*PRInfo, bool) {
 	for _, p := range rs.PRs {
 		if p.PR.Base.RepoID == pr.Base.RepoID &&
@@ -243,58 +299,134 @@ next_rs:
 	return true
 }
 
-func (rs *PRSet) AssignReviewers(gitea GiteaReviewFetcherAndRequester, maintainers MaintainershipData) error {
+func (rs *PRSet) FindMissingAndExtraReviewers(maintainers MaintainershipData, idx int) (missing, extra []string) {
 	configReviewers := ParseReviewers(rs.Config.Reviewers)
 
-	for _, pr := range rs.PRs {
-		reviewers := []string{}
+	// remove reviewers that were already requested and are not stale
+	prjMaintainers := maintainers.ListProjectMaintainers(nil)
+	LogDebug("project maintainers:", prjMaintainers)
 
-		if rs.IsPrjGitPR(pr.PR) {
-			reviewers = slices.Concat(configReviewers.Prj, configReviewers.PrjOptional)
-			LogDebug("PrjGit submitter:", pr.PR.User.UserName)
-			if len(rs.PRs) == 1 {
-				reviewers = slices.Concat(reviewers, maintainers.ListProjectMaintainers(nil))
-			}
+	pr := rs.PRs[idx]
+	if rs.IsPrjGitPR(pr.PR) {
+		missing = slices.Concat(configReviewers.Prj, configReviewers.PrjOptional)
+		if rs.HasAutoStaging {
+			missing = append(missing, Bot_BuildReview)
+		}
+		LogDebug("PrjGit submitter:", pr.PR.User.UserName)
+		// only need project maintainer reviews if:
+		//    * not created by a bot and has other PRs, or
+		//    * not created by maintainer
+		noReviewPRCreators := prjMaintainers
+		if len(rs.PRs) > 1 {
+			noReviewPRCreators = append(noReviewPRCreators, rs.BotUser)
+		}
+		if slices.Contains(noReviewPRCreators, pr.PR.User.UserName) || pr.Reviews.IsReviewedByOneOf(prjMaintainers...) {
+			LogDebug("Project already reviewed by a project maintainer, remove rest")
+			// do not remove reviewers if they are also maintainers
+			prjMaintainers = slices.DeleteFunc(prjMaintainers, func(m string) bool { return slices.Contains(missing, m) })
+			extra = slices.Concat(prjMaintainers, []string{rs.BotUser})
 		} else {
-			pkg := pr.PR.Base.Repo.Name
-			reviewers = slices.Concat(configReviewers.Pkg, maintainers.ListProjectMaintainers(nil), maintainers.ListPackageMaintainers(pkg, nil), configReviewers.PkgOptional)
-		}
-
-		slices.Sort(reviewers)
-		reviewers = slices.Compact(reviewers)
-
-		// submitters do not need to review their own work
-		if idx := slices.Index(reviewers, pr.PR.User.UserName); idx != -1 {
-			reviewers = slices.Delete(reviewers, idx, idx+1)
-		}
-
-		LogDebug("PR: ", pr.PR.Base.Repo.Name, pr.PR.Index)
-		LogDebug("reviewers for PR:", reviewers)
-
-		// remove reviewers that were already requested and are not stale
-		reviews, err := FetchGiteaReviews(gitea, reviewers, pr.PR.Base.Repo.Owner.UserName, pr.PR.Base.Repo.Name, pr.PR.Index)
-		if err != nil {
-			LogError("Error fetching reviews:", err)
-			return err
-		}
-
-		for idx := 0; idx < len(reviewers); {
-			user := reviewers[idx]
-			if reviews.HasPendingReviewBy(user) || reviews.IsReviewedBy(user) {
-				reviewers = slices.Delete(reviewers, idx, idx+1)
-				LogDebug("removing reviewer:", user)
+			// if bot not created PrjGit or prj maintainer, we need to add project reviewers here
+			if slices.Contains(noReviewPRCreators, pr.PR.User.UserName) {
+				LogDebug("No need for project maintainers")
+				extra = slices.Concat(prjMaintainers, []string{rs.BotUser})
 			} else {
-				idx++
+				LogDebug("Adding prjMaintainers to PrjGit")
+				missing = append(missing, prjMaintainers...)
 			}
 		}
+	} else {
+		pkg := pr.PR.Base.Repo.Name
+		pkgMaintainers := maintainers.ListPackageMaintainers(pkg, nil)
+		Maintainers := slices.Concat(prjMaintainers, pkgMaintainers)
+		noReviewPkgPRCreators := pkgMaintainers
 
-		// get maintainers associated with the PR too
-		if len(reviewers) > 0 {
-			LogDebug("Requesting reviews from:", reviewers)
+		LogDebug("packakge maintainers:", Maintainers)
+
+		missing = slices.Concat(configReviewers.Pkg, configReviewers.PkgOptional)
+		if slices.Contains(noReviewPkgPRCreators, pr.PR.User.UserName) || pr.Reviews.IsReviewedByOneOf(Maintainers...) {
+			// submitter is maintainer or already reviewed
+			LogDebug("Package reviewed by maintainer (or subitter is maintainer), remove the rest of them")
+			// do not remove reviewers if they are also maintainers
+			Maintainers = slices.DeleteFunc(Maintainers, func(m string) bool { return slices.Contains(missing, m) })
+			extra = slices.Concat(Maintainers, []string{rs.BotUser})
+		} else {
+			// maintainer review is missing
+			LogDebug("Adding package maintainers to package git")
+			missing = append(missing, pkgMaintainers...)
+		}
+	}
+
+	slices.Sort(missing)
+	missing = slices.Compact(missing)
+
+	slices.Sort(extra)
+	extra = slices.Compact(extra)
+
+	// submitters cannot review their own work
+	if idx := slices.Index(missing, pr.PR.User.UserName); idx != -1 {
+		missing = slices.Delete(missing, idx, idx+1)
+	}
+
+	LogDebug("PR: ", PRtoString(pr.PR))
+	LogDebug("  preliminary add reviewers for PR:", missing)
+	LogDebug("  preliminary rm  reviewers for PR:", extra)
+
+	// remove missing reviewers that are already done or already pending
+	for idx := 0; idx < len(missing); {
+		user := missing[idx]
+		if pr.Reviews.HasPendingReviewBy(user) || pr.Reviews.IsReviewedBy(user) {
+			missing = slices.Delete(missing, idx, idx+1)
+			LogDebug("   removing done/pending reviewer:", user)
+		} else {
+			idx++
+		}
+	}
+
+	// remove extra reviews that are actually only pending, and only pending by us
+	for idx := 0; idx < len(extra); {
+		user := extra[idx]
+		rr := pr.Reviews.FindReviewRequester(user)
+		if rr != nil && rr.User.UserName == rs.BotUser && pr.Reviews.HasPendingReviewBy(user) {
+			// good to remove this review
+			idx++
+		} else {
+			// this review should not be considered as extra by us
+			LogDebug("   - cannot find? to remove", user)
+			if rr != nil {
+				LogDebug("    ", rr.User.UserName, "vs.", rs.BotUser, pr.Reviews.HasPendingReviewBy(user))
+			}
+			extra = slices.Delete(extra, idx, idx+1)
+		}
+	}
+
+	LogDebug("  add reviewers for PR:", missing)
+	LogDebug("  rm  reviewers for PR:", extra)
+
+	return missing, extra
+}
+
+func (rs *PRSet) AssignReviewers(gitea GiteaReviewFetcherAndRequesterAndUnrequester, maintainers MaintainershipData) error {
+	for idx, pr := range rs.PRs {
+		missingReviewers, extraReviewers := rs.FindMissingAndExtraReviewers(maintainers, idx)
+
+		if len(missingReviewers) > 0 {
+			LogDebug("  Requesting reviews from:", missingReviewers)
 			if !IsDryRun {
-				for _, r := range reviewers {
+				for _, r := range missingReviewers {
 					if _, err := gitea.RequestReviews(pr.PR, r); err != nil {
-						LogError("Cannot create reviews on", fmt.Sprintf("%s/%s!%d for [%s]", pr.PR.Base.Repo.Owner.UserName, pr.PR.Base.Repo.Name, pr.PR.Index, r), err)
+						LogError("Cannot create reviews on", PRtoString(pr.PR), "for user:", r, err)
+					}
+				}
+			}
+		}
+		if len(extraReviewers) > 0 {
+			LogDebug("  UnRequesting reviews from:", extraReviewers)
+			if !IsDryRun {
+				for _, r := range extraReviewers {
+					org, repo, idx := pr.PRComponents()
+					if err := gitea.UnrequestReview(org, repo, idx, r); err != nil {
+						LogError("Cannot unrequest reviews on", PRtoString(pr.PR), "for user:", r, err)
 					}
 				}
 			}
@@ -320,11 +452,12 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 		if err == nil && prjgit != nil {
 			reviewers := slices.Concat(configReviewers.Prj, maintainers.ListProjectMaintainers(groups))
 			LogDebug("Fetching reviews for", prjgit.PR.Base.Repo.Owner.UserName, prjgit.PR.Base.Repo.Name, prjgit.PR.Index)
-			r, err := FetchGiteaReviews(gitea, reviewers, prjgit.PR.Base.Repo.Owner.UserName, prjgit.PR.Base.Repo.Name, prjgit.PR.Index)
+			r, err := FetchGiteaReviews(gitea, prjgit.PR.Base.Repo.Owner.UserName, prjgit.PR.Base.Repo.Name, prjgit.PR.Index)
 			if err != nil {
 				LogError("Cannot fetch gita reaviews for PR:", err)
 				return false
 			}
+			r.RequestedReviewers = reviewers
 			prjgit.Reviews = r
 			if prjgit.Reviews.IsManualMergeOK() {
 				is_manually_reviewed_ok = true
@@ -340,11 +473,12 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 				pkg := pr.PR.Base.Repo.Name
 				reviewers := slices.Concat(configReviewers.Pkg, maintainers.ListPackageMaintainers(pkg, groups))
 				LogDebug("Fetching reviews for", pr.PR.Base.Repo.Owner.UserName, pr.PR.Base.Repo.Name, pr.PR.Index)
-				r, err := FetchGiteaReviews(gitea, reviewers, pr.PR.Base.Repo.Owner.UserName, pr.PR.Base.Repo.Name, pr.PR.Index)
+				r, err := FetchGiteaReviews(gitea, pr.PR.Base.Repo.Owner.UserName, pr.PR.Base.Repo.Name, pr.PR.Index)
 				if err != nil {
 					LogError("Cannot fetch gita reaviews for PR:", err)
 					return false
 				}
+				r.RequestedReviewers = reviewers
 				pr.Reviews = r
 				if !pr.Reviews.IsManualMergeOK() {
 					LogInfo("Not approved manual merge. PR:", pr.PR.URL)
@@ -366,6 +500,9 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 		var pkg string
 		if rs.IsPrjGitPR(pr.PR) {
 			reviewers = configReviewers.Prj
+			if rs.HasAutoStaging {
+				reviewers = append(reviewers, Bot_BuildReview)
+			}
 			pkg = ""
 		} else {
 			reviewers = configReviewers.Pkg
@@ -377,11 +514,12 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 			return false
 		}
 
-		r, err := FetchGiteaReviews(gitea, reviewers, pr.PR.Base.Repo.Owner.UserName, pr.PR.Base.Repo.Name, pr.PR.Index)
+		r, err := FetchGiteaReviews(gitea, pr.PR.Base.Repo.Owner.UserName, pr.PR.Base.Repo.Name, pr.PR.Index)
 		if err != nil {
 			LogError("Cannot fetch gitea reaviews for PR:", err)
 			return false
 		}
+		r.RequestedReviewers = reviewers
 
 		is_manually_reviewed_ok = r.IsApproved()
 		LogDebug("PR to", pr.PR.Base.Repo.Name, "reviewed?", is_manually_reviewed_ok)
@@ -394,7 +532,7 @@ func (rs *PRSet) IsApproved(gitea GiteaPRChecker, maintainers MaintainershipData
 
 		if need_maintainer_review := !rs.IsPrjGitPR(pr.PR) || pr.PR.User.UserName != rs.BotUser; need_maintainer_review {
 			// Do not expand groups here, as the group-review-bot will ACK if group has reviewed.
-			if is_manually_reviewed_ok = maintainers.IsApproved(pkg, r.reviews, pr.PR.User.UserName, nil); !is_manually_reviewed_ok {
+			if is_manually_reviewed_ok = maintainers.IsApproved(pkg, r.Reviews, pr.PR.User.UserName, nil); !is_manually_reviewed_ok {
 				LogDebug(" not approved?", pkg)
 				return false
 			}

common/pr_test.go

@@ -2,7 +2,6 @@ package common_test
 
 import (
 	"errors"
-	"fmt"
 	"os"
 	"os/exec"
 	"path"
@@ -15,22 +14,23 @@ import (
 	"src.opensuse.org/autogits/common/gitea-generated/models"
 	mock_common "src.opensuse.org/autogits/common/mock"
 )
-/*
-func TestCockpit(t *testing.T) {
-	common.SetLoggingLevel(common.LogLevelDebug)
-	gitea := common.AllocateGiteaTransport("https://src.opensuse.org")
-	tl, err := gitea.GetTimeline("cockpit", "cockpit", 29)
-	if err != nil {
-		t.Fatal("Fail to timeline", err)
-	}
-	t.Log(tl)
-	r, err := common.FetchGiteaReviews(gitea, []string{}, "cockpit", "cockpit", 29)
-	if err != nil {
-		t.Fatal("Error:", err)
-	}
 
-	t.Error(r)
-}
+/*
+	func TestCockpit(t *testing.T) {
+		common.SetLoggingLevel(common.LogLevelDebug)
+		gitea := common.AllocateGiteaTransport("https://src.opensuse.org")
+		tl, err := gitea.GetTimeline("cockpit", "cockpit", 29)
+		if err != nil {
+			t.Fatal("Fail to timeline", err)
+		}
+		t.Log(tl)
+		r, err := common.FetchGiteaReviews(gitea, []string{}, "cockpit", "cockpit", 29)
+		if err != nil {
+			t.Fatal("Error:", err)
+		}
+
+		t.Error(r)
+	}
 */
 func reviewsToTimeline(reviews []*models.PullReview) []*models.TimelineComment {
 	timeline := make([]*models.TimelineComment, len(reviews))
@@ -75,7 +75,7 @@ func TestPR(t *testing.T) {
 		consistentSet bool
 		prjGitPRIndex int
 
-		reviewSetFetcher func(*mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error)
+		reviewSetFetcher func(*mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error)
 	}{
 		{
 			name: "Error fetching PullRequest",
@@ -147,7 +147,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      true,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &baseConfig)
 			},
 		},
@@ -179,7 +179,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      false,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &baseConfig)
 			},
 		},
@@ -207,7 +207,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      false,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &common.AutogitConfig{
 					Reviewers:       []string{"+super1", "*super2", "m1", "-m2"},
 					Branch:          "branch",
@@ -241,7 +241,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      true,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &common.AutogitConfig{
 					Reviewers:       []string{"+super1", "*super2", "m1", "-m2"},
 					Branch:          "branch",
@@ -275,7 +275,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      true,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &common.AutogitConfig{
 					Reviewers:          []string{"+super1", "*super2", "m1", "-m2"},
 					Branch:             "branch",
@@ -311,7 +311,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      false,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &common.AutogitConfig{
 					Reviewers:          []string{"+super1", "*super2", "m1", "-m2"},
 					Branch:             "branch",
@@ -346,7 +346,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      true,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &common.AutogitConfig{
 					Reviewers:       []string{"+super1", "*super2", "m1", "-m2"},
 					Branch:          "branch",
@@ -388,7 +388,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      true,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &common.AutogitConfig{
 					Reviewers:       []string{"+super1", "*super2", "m1", "-m2"},
 					Branch:          "branch",
@@ -430,7 +430,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      false,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &common.AutogitConfig{
 					Reviewers:          []string{"+super1", "*super2", "m1", "-m2"},
 					Branch:             "branch",
@@ -473,7 +473,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      false,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				return common.FetchPRSet("test", mock, "foo", "barPrj", 42, &common.AutogitConfig{
 					Reviewers:       []string{"+super1", "*super2", "m1", "-m2"},
 					Branch:          "branch",
@@ -500,7 +500,7 @@ func TestPR(t *testing.T) {
 			prjGitPRIndex: 0,
 			consistentSet: true,
 			reviewed:      true,
-			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineFetcher) (*common.PRSet, error) {
+			reviewSetFetcher: func(mock *mock_common.MockGiteaPRTimelineReviewFetcher) (*common.PRSet, error) {
 				config := common.AutogitConfig{
 					Reviewers:      []string{"+super1", "*super2", "m1", "-m2", "~*bot"},
 					Branch:         "branch",
@@ -515,7 +515,7 @@ func TestPR(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			ctl := gomock.NewController(t)
-			pr_mock := mock_common.NewMockGiteaPRTimelineFetcher(ctl)
+			pr_mock := mock_common.NewMockGiteaPRTimelineReviewFetcher(ctl)
 			review_mock := mock_common.NewMockGiteaPRChecker(ctl)
 			//				reviewer_mock := mock_common.NewMockGiteaReviewRequester(ctl)
 
@@ -619,283 +619,508 @@ func TestPR(t *testing.T) {
 	}
 }
 
-func TestPRAssignReviewers(t *testing.T) {
-	t.Skip("FAIL: unexpected calls, missing calls")
+func TestFindMissingAndExtraReviewers(t *testing.T) {
 	tests := []struct {
 		name      string
-		config    common.AutogitConfig
 		reviewers []struct {
 			org, repo string
 			num       int64
 			reviewer  string
 		}
 
-		pkgReviews  []*models.PullReview
-		pkgTimeline []*models.TimelineComment
-		prjReviews  []*models.PullReview
-		prjTimeline []*models.TimelineComment
+		prset       *common.PRSet
+		maintainers common.MaintainershipData
 
-		expectedReviewerCall [2][]string
+		noAutoStaging bool
+
+		expected_missing_reviewers [][]string
+		expected_extra_reviewers   [][]string
 	}{
 		{
 			name: "No reviewers",
-			config: common.AutogitConfig{
-				GitProjectName: "prg/repo#main",
-				Organization:   "org",
-				Branch:         "main",
-				Reviewers:      []string{},
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "foo"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{},
+				},
 			},
-			expectedReviewerCall: [2][]string{{"autogits_obs_staging_bot"}, {"prjmaintainer", "pkgmaintainer"}},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{}},
 		},
 		{
 			name: "One project reviewer only",
-			config: common.AutogitConfig{
-				GitProjectName: "prg/repo#main",
-				Organization:   "org",
-				Branch:         "main",
-				Reviewers:      []string{"-user1"},
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "foo"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "foo"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1"},
+				},
+			},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{}},
+
+			expected_missing_reviewers: [][]string{
+				[]string{},
+				[]string{"autogits_obs_staging_bot", "user1"},
+			},
+		},
+		{
+			name:          "One project reviewer only and no auto staging",
+			noAutoStaging: true,
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "foo"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "foo"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1"},
+				},
+			},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{}},
+
+			expected_missing_reviewers: [][]string{
+				nil,
+				{"user1"},
 			},
-			expectedReviewerCall: [2][]string{{"user1", "autogits_obs_staging_bot"}, {"prjmaintainer", "pkgmaintainer"}},
 		},
 		{
 			name: "One project reviewer and one pkg reviewer only",
-			config: common.AutogitConfig{
-				GitProjectName: "prg/repo#main",
-				Organization:   "org",
-				Branch:         "main",
-				Reviewers:      []string{"-user1", "user2"},
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "foo"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "foo"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1", "user2"},
+				},
+			},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{}},
+
+			expected_missing_reviewers: [][]string{
+				[]string{"user2"},
+				[]string{"autogits_obs_staging_bot", "user1"},
 			},
-			expectedReviewerCall: [2][]string{{"user1", "autogits_obs_staging_bot"}, {"user2", "prjmaintainer", "pkgmaintainer"}},
 		},
 		{
-			name: "No need to get reviews of submitter",
-			config: common.AutogitConfig{
-				GitProjectName: "prg/repo#main",
-				Organization:   "org",
-				Branch:         "main",
-				Reviewers:      []string{"-user1", "submitter"},
+			name: "No need to get reviews of submitter reviewer",
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "submitter"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{
+							Reviews:            []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "m1"}}},
+							RequestedReviewers: []string{"m1"},
+							FullTimeline: []*models.TimelineComment{
+								{User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "m1"}, Type: common.TimelineCommentType_ReviewRequested},
+							},
+						},
+					},
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "foo"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1", "submitter"},
+				},
+				BotUser: "bot",
+			},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{"pkg": []string{"m1", "submitter"}}},
+
+			expected_missing_reviewers: [][]string{
+				nil,
+				{"autogits_obs_staging_bot", "user1"},
+			},
+			expected_extra_reviewers: [][]string{
+				{"m1"},
 			},
-			expectedReviewerCall: [2][]string{{"user1", "autogits_obs_staging_bot"}, {"prjmaintainer", "pkgmaintainer"}},
 		},
 		{
-			name: "Reviews are done",
-			config: common.AutogitConfig{
-				GitProjectName: "prg/repo#main",
-				Organization:   "org",
-				Branch:         "main",
-				Reviewers:      []string{"-user1", "user2"},
-			},
-			pkgReviews: []*models.PullReview{
-				{
-					State: common.ReviewStateApproved,
-					User:  &models.User{UserName: "user2"},
+			name: "No need to get reviews of submitter maintainer",
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "submitter"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "foo"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
 				},
-				{
-					State: common.ReviewStateApproved,
-					User:  &models.User{UserName: "pkgmaintainer"},
-				},
-				{
-					State: common.ReviewStatePending,
-					User:  &models.User{UserName: "prjmaintainer"},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1", "submitter"},
 				},
 			},
-			prjReviews: []*models.PullReview{
-				{
-					State: common.ReviewStateRequestChanges,
-					User:  &models.User{UserName: "user1"},
-				},
-				{
-					State: common.ReviewStateRequestReview,
-					User:  &models.User{UserName: "autogits_obs_staging_bot"},
-				},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{"pkg": []string{"submitter"}}},
+
+			expected_missing_reviewers: [][]string{
+				[]string{},
+				[]string{"autogits_obs_staging_bot", "user1"},
 			},
-			expectedReviewerCall: [2][]string{},
+		},
+
+		{
+			name: "Add reviewer if also maintainer where review by maintainer is not needed",
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "submitter"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "bot"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
+						},
+						Reviews: &common.PRReviews{},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1", "submitter", "*reviewer"},
+				},
+				BotUser: "bot",
+			},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{"pkg": []string{"submitter", "reviewer"}, "": []string{"reviewer"}}},
+
+			expected_missing_reviewers: [][]string{
+				[]string{"reviewer"},
+				[]string{"autogits_obs_staging_bot", "reviewer", "user1"},
+			},
+		},
+
+		{
+			name: "Dont remove reviewer if also maintainer",
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "submitter"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{
+							Reviews:            []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "reviewer"}}},
+							RequestedReviewers: []string{"reviewer"},
+							FullTimeline:       []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
+						},
+					},
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "bot"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
+						},
+						Reviews: &common.PRReviews{
+							Reviews:            []*models.PullReview{{State: common.ReviewStateRequestReview, User: &models.User{UserName: "reviewer"}}},
+							RequestedReviewers: []string{"reviewer"},
+							FullTimeline:       []*models.TimelineComment{{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "reviewer"}}},
+						},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1", "submitter", "*reviewer"},
+				},
+				BotUser: "bot",
+			},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{"pkg": []string{"submitter", "reviewer"}, "": []string{"reviewer"}}},
+
+			expected_missing_reviewers: [][]string{
+				[]string{},
+				[]string{"autogits_obs_staging_bot", "user1"},
+			},
+		},
+
+		{
+			name: "Extra project reviewer on the package",
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "submitter"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{
+							Reviews: []*models.PullReview{
+								{State: common.ReviewStateApproved, User: &models.User{UserName: "user2"}},
+								{State: common.ReviewStateApproved, User: &models.User{UserName: "pkgmaintainer"}},
+								{State: common.ReviewStatePending, User: &models.User{UserName: "prjmaintainer"}},
+							},
+							RequestedReviewers: []string{"user2", "pkgmaintainer", "prjmaintainer"},
+							FullTimeline: []*models.TimelineComment{
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "user2"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgmaintainer"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prjmaintainer"}},
+							},
+						},
+					},
+
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "bot"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
+						},
+						Reviews: &common.PRReviews{
+							Reviews: []*models.PullReview{
+								{State: common.ReviewStateRequestChanges, User: &models.User{UserName: "user1"}},
+								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "autogits_obs_staging_bot"}},
+							},
+							RequestedReviewers: []string{"user1", "autogits_obs_staging_bot"},
+							FullTimeline: []*models.TimelineComment{
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "user1"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "autogits_obs_staging_bot"}},
+							},
+						},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1", "submitter"},
+				},
+				BotUser: "bot",
+			},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{"pkg": []string{"pkgmaintainer"}, "": {"prjmaintainer"}}},
+
+			expected_missing_reviewers: [][]string{},
+			expected_extra_reviewers:   [][]string{{"prjmaintainer"}},
 		},
 		{
-			name: "Stale review is not done, re-request it",
-			config: common.AutogitConfig{
-				GitProjectName: "org/repo#main",
-				Organization:   "org",
-				Branch:         "main",
-				Reviewers:      []string{"-user1", "user2"},
+			name: "Extra project reviewers on the package and project",
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "submitter"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{
+							Reviews: []*models.PullReview{
+								{State: common.ReviewStateApproved, User: &models.User{UserName: "user2"}},
+								{State: common.ReviewStateApproved, User: &models.User{UserName: "pkgmaintainer"}},
+								{State: common.ReviewStatePending, User: &models.User{UserName: "prjmaintainer"}},
+								{State: common.ReviewStatePending, User: &models.User{UserName: "pkgm1"}},
+								{State: common.ReviewStatePending, User: &models.User{UserName: "pkgm2"}},
+								{State: common.ReviewStatePending, User: &models.User{UserName: "prj1"}},
+								{State: common.ReviewStatePending, User: &models.User{UserName: "prj2"}},
+								{State: common.ReviewStatePending, User: &models.User{UserName: "someother"}},
+							},
+							RequestedReviewers: []string{"user2", "pkgmaintainer", "prjmaintainer", "pkgm1", "pkgm2", "someother", "prj1", "prj2"},
+							FullTimeline: []*models.TimelineComment{
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgmaintainer"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prjmaintainer"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj2"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgm1"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgm2"}},
+							},
+						},
+					},
+
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "bot"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prg"}}},
+						},
+						Reviews: &common.PRReviews{
+							Reviews: []*models.PullReview{
+								{State: common.ReviewStateRequestChanges, User: &models.User{UserName: "user1"}},
+								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "autogits_obs_staging_bot"}},
+								{State: common.ReviewStatePending, User: &models.User{UserName: "prj1"}},
+								{State: common.ReviewStatePending, User: &models.User{UserName: "prj2"}},
+							},
+							RequestedReviewers: []string{"user1", "autogits_obs_staging_bot", "prj1", "prj2"},
+							FullTimeline: []*models.TimelineComment{
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "autogits_obs_staging_bot"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj2"}},
+							},
+						},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prg/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1", "submitter"},
+				},
+				BotUser: "bot",
 			},
-			pkgReviews: []*models.PullReview{
-				{
-					State: common.ReviewStateApproved,
-					User:  &models.User{UserName: "user2"},
-				},
-				{
-					State: common.ReviewStatePending,
-					User:  &models.User{UserName: "prjmaintainer"},
-				},
-			},
-			prjReviews: []*models.PullReview{
-				{
-					State: common.ReviewStateRequestChanges,
-					User:  &models.User{UserName: "user1"},
-					Stale: true,
-				},
-				{
-					State: common.ReviewStateRequestReview,
-					Stale: true,
-					User:  &models.User{UserName: "autogits_obs_staging_bot"},
-				},
-			},
-			expectedReviewerCall: [2][]string{{"user1", "autogits_obs_staging_bot"}, {"pkgmaintainer"}},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{"pkg": []string{"pkgmaintainer", "pkgm1", "pkgm2"}, "": {"prjmaintainer", "prj1", "prj2"}}},
+
+			expected_missing_reviewers: [][]string{},
+			expected_extra_reviewers:   [][]string{{"pkgm1", "pkgm2", "prj1", "prj2", "prjmaintainer"}, {"prj1", "prj2"}},
 		},
 		{
-			name: "Stale optional review is not done, re-request it",
-			config: common.AutogitConfig{
-				GitProjectName: "prg/repo#main",
-				Organization:   "org",
-				Branch:         "main",
-				Reviewers:      []string{"-user1", "user2", "~bot"},
+			name: "No extra project reviewers on the package and project (all pending)",
+			prset: &common.PRSet{
+				PRs: []*common.PRInfo{
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "submitter"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "pkg", Owner: &models.User{UserName: "org"}}},
+						},
+						Reviews: &common.PRReviews{
+							Reviews: []*models.PullReview{
+								{State: common.ReviewStateApproved, User: &models.User{UserName: "user2"}},
+								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "pkgmaintainer"}},
+								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "prjmaintainer"}},
+								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "pkgm1"}},
+								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "prj1"}},
+								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "someother"}},
+							},
+							RequestedReviewers: []string{"user2", "pkgmaintainer", "prjmaintainer", "pkgm1", "someother", "prj1"},
+							FullTimeline: []*models.TimelineComment{
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgm1"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "pkgmaintainer"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prjmaintainer"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "!bot"}, Assignee: &models.User{UserName: "someother"}},
+							},
+						},
+					},
+
+					{
+						PR: &models.PullRequest{
+							User: &models.User{UserName: "bot"},
+							Base: &models.PRBranchInfo{Name: "main", Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "prj"}}},
+						},
+						Reviews: &common.PRReviews{
+							Reviews: []*models.PullReview{
+								{State: common.ReviewStateRequestChanges, User: &models.User{UserName: "user1"}},
+								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "autogits_obs_staging_bot"}},
+								{State: common.ReviewStateRequestReview, User: &models.User{UserName: "prj1"}},
+							},
+							RequestedReviewers: []string{"user1", "autogits_obs_staging_bot", "prj1"},
+							FullTimeline: []*models.TimelineComment{
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "autogits_obs_staging_bot"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "bot"}, Assignee: &models.User{UserName: "prj1"}},
+								{Type: common.TimelineCommentType_ReviewRequested, User: &models.User{UserName: "!bot"}, Assignee: &models.User{UserName: "user1"}},
+							},
+						},
+					},
+				},
+				Config: &common.AutogitConfig{
+					GitProjectName: "prj/repo#main",
+					Organization:   "org",
+					Branch:         "main",
+					Reviewers:      []string{"-user1", "submitter"},
+				},
+				BotUser: "bot",
 			},
-			pkgReviews: []*models.PullReview{
-				{
-					State: common.ReviewStateApproved,
-					User:  &models.User{UserName: "bot"},
-					Stale: true,
-				},
-				{
-					State: common.ReviewStateApproved,
-					User:  &models.User{UserName: "user2"},
-				},
-				{
-					State: common.ReviewStatePending,
-					User:  &models.User{UserName: "prjmaintainer"},
-				},
-			},
-			prjReviews: []*models.PullReview{
-				{
-					State: common.ReviewStateRequestChanges,
-					User:  &models.User{UserName: "user1"},
-					Stale: true,
-				},
-				{
-					State: common.ReviewStateRequestReview,
-					Stale: true,
-					User:  &models.User{UserName: "autogits_obs_staging_bot"},
-				},
-			},
-			expectedReviewerCall: [2][]string{{"user1", "autogits_obs_staging_bot"}, {"pkgmaintainer", "bot"}},
+			maintainers: &common.MaintainershipMap{Data: map[string][]string{"pkg": []string{"pkgmaintainer", "pkgm1", "pkgm2"}, "": {"prjmaintainer", "prj1", "prj2"}}},
+
+			expected_missing_reviewers: [][]string{{"pkgm2", "prj2"}},
+			expected_extra_reviewers:   [][]string{{}, {"prj1"}},
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			ctl := gomock.NewController(t)
-			pr_mock := mock_common.NewMockGiteaPRTimelineFetcher(ctl)
-			review_mock := mock_common.NewMockGiteaReviewFetcherAndRequester(ctl)
-			maintainership_mock := mock_common.NewMockMaintainershipData(ctl)
+			test.prset.HasAutoStaging = !test.noAutoStaging
+			for idx, pr := range test.prset.PRs {
+				missing, extra := test.prset.FindMissingAndExtraReviewers(test.maintainers, idx)
 
-			if test.pkgTimeline == nil {
-				test.pkgTimeline = reviewsToTimeline(test.pkgReviews)
-			}
-			if test.prjTimeline == nil {
-				test.prjTimeline = reviewsToTimeline(test.prjReviews)
-			}
-
-			pr_mock.EXPECT().GetPullRequest("other", "pkgrepo", int64(1)).Return(&models.PullRequest{
-				Body:               "Some description is here",
-				User:               &models.User{UserName: "submitter"},
-				RequestedReviewers: []*models.User{},
-				Base:               &models.PRBranchInfo{Repo: &models.Repository{Name: "pkgrepo", Owner: &models.User{UserName: "other"}}},
-				Head:               &models.PRBranchInfo{},
-				Index:              1,
-			}, nil)
-			review_mock.EXPECT().GetPullRequestReviews("other", "pkgrepo", int64(1)).Return(test.pkgReviews, nil)
-			review_mock.EXPECT().GetTimeline("other", "pkgrepo", int64(1)).Return(test.pkgTimeline, nil)
-			pr_mock.EXPECT().GetPullRequest("org", "repo", int64(1)).Return(&models.PullRequest{
-				Body:               fmt.Sprintf(common.PrPattern, "other", "pkgrepo", 1),
-				User:               &models.User{UserName: "bot1"},
-				RequestedReviewers: []*models.User{{UserName: "main_reviewer"}},
-				Base:               &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "org"}}},
-				Head:               &models.PRBranchInfo{},
-				Index:              42,
-			}, nil)
-			review_mock.EXPECT().GetPullRequestReviews("org", "repo", int64(42)).Return(test.prjReviews, nil)
-			review_mock.EXPECT().GetTimeline("org", "repo", int64(42)).Return(test.prjTimeline, nil)
-
-			maintainership_mock.EXPECT().ListProjectMaintainers(gomock.Any()).Return([]string{"prjmaintainer"}).AnyTimes()
-			maintainership_mock.EXPECT().ListPackageMaintainers("pkgrepo", gomock.Any()).Return([]string{"pkgmaintainer"}).AnyTimes()
-
-			prs, _ := common.FetchPRSet("test", pr_mock, "other", "pkgrepo", int64(1), &test.config)
-			if len(prs.PRs) != 2 {
-				t.Fatal("PRs not fetched")
-			}
-			for _, pr := range prs.PRs {
-				r := test.expectedReviewerCall[0]
-				if !prs.IsPrjGitPR(pr.PR) {
-					r = test.expectedReviewerCall[1]
+				// avoid nil dereference below, by adding empty array elements
+				if idx >= len(test.expected_missing_reviewers) {
+					test.expected_missing_reviewers = append(test.expected_missing_reviewers, nil)
 				}
-				slices.Sort(r)
-				for _, reviewer := range r {
-					review_mock.EXPECT().RequestReviews(pr.PR, reviewer).Return(nil, nil)
+				if idx >= len(test.expected_extra_reviewers) {
+					test.expected_extra_reviewers = append(test.expected_extra_reviewers, nil)
+				}
+
+				slices.Sort(test.expected_extra_reviewers[idx])
+				slices.Sort(test.expected_missing_reviewers[idx])
+				if slices.Compare(missing, test.expected_missing_reviewers[idx]) != 0 {
+					t.Error("Expected missing reviewers for", common.PRtoString(pr.PR), ":", test.expected_missing_reviewers[idx], "but have:", missing)
+				}
+
+				if slices.Compare(extra, test.expected_extra_reviewers[idx]) != 0 {
+					t.Error("Expected reviewers to remove for", common.PRtoString(pr.PR), ":", test.expected_extra_reviewers[idx], "but have:", extra)
 				}
 			}
-			prs.AssignReviewers(review_mock, maintainership_mock)
-		})
-	}
-
-	prjgit_tests := []struct {
-		name      string
-		config    common.AutogitConfig
-		reviewers []struct {
-			org, repo string
-			num       int64
-			reviewer  string
-		}
-
-		prjReviews []*models.PullReview
-
-		expectedReviewerCall [2][]string
-	}{
-		{
-			name: "PrjMaintainers in prjgit review when not part of pkg set",
-			config: common.AutogitConfig{
-				GitProjectName: "org/repo#main",
-				Organization:   "org",
-				Branch:         "main",
-				Reviewers:      []string{},
-			},
-			expectedReviewerCall: [2][]string{{"autogits_obs_staging_bot", "prjmaintainer"}},
-		},
-	}
-	for _, test := range prjgit_tests {
-		t.Run(test.name, func(t *testing.T) {
-			ctl := gomock.NewController(t)
-			pr_mock := mock_common.NewMockGiteaPRTimelineFetcher(ctl)
-			review_mock := mock_common.NewMockGiteaReviewFetcherAndRequester(ctl)
-			maintainership_mock := mock_common.NewMockMaintainershipData(ctl)
-
-			pr_mock.EXPECT().GetPullRequest("org", "repo", int64(1)).Return(&models.PullRequest{
-				Body:               "Some description is here",
-				User:               &models.User{UserName: "submitter"},
-				RequestedReviewers: []*models.User{},
-				Base:               &models.PRBranchInfo{Repo: &models.Repository{Name: "repo", Owner: &models.User{UserName: "org"}}},
-				Head:               &models.PRBranchInfo{},
-				Index:              1,
-			}, nil)
-			review_mock.EXPECT().GetPullRequestReviews("org", "repo", int64(1)).Return(test.prjReviews, nil)
-			review_mock.EXPECT().GetTimeline("org", "repo", int64(1)).Return(nil, nil)
-
-			maintainership_mock.EXPECT().ListProjectMaintainers(gomock.Any()).Return([]string{"prjmaintainer"}).AnyTimes()
-
-			prs, _ := common.FetchPRSet("test", pr_mock, "org", "repo", int64(1), &test.config)
-			if len(prs.PRs) != 1 {
-				t.Fatal("PRs not fetched")
-			}
-			for _, pr := range prs.PRs {
-				r := test.expectedReviewerCall[0]
-				if !prs.IsPrjGitPR(pr.PR) {
-					t.Fatal("only prjgit pr here")
-				}
-				for _, reviewer := range r {
-					review_mock.EXPECT().RequestReviews(pr.PR, reviewer).Return(nil, nil)
-				}
-			}
-			prs.AssignReviewers(review_mock, maintainership_mock)
 		})
 	}
 }
@@ -978,7 +1203,7 @@ func TestPRMerge(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			ctl := gomock.NewController(t)
-			mock := mock_common.NewMockGiteaPRTimelineFetcher(ctl)
+			mock := mock_common.NewMockGiteaPRTimelineReviewFetcher(ctl)
 			reviewUnrequestMock := mock_common.NewMockGiteaReviewUnrequester(ctl)
 
 			reviewUnrequestMock.EXPECT().UnrequestReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
@@ -1037,7 +1262,7 @@ func TestPRChanges(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			ctl := gomock.NewController(t)
-			mock_fetcher := mock_common.NewMockGiteaPRTimelineFetcher(ctl)
+			mock_fetcher := mock_common.NewMockGiteaPRTimelineReviewFetcher(ctl)
 			mock_fetcher.EXPECT().GetPullRequest("org", "prjgit", int64(42)).Return(test.PrjPRs, nil)
 			for _, pr := range test.PRs {
 				mock_fetcher.EXPECT().GetPullRequest(pr.Base.Repo.Owner.UserName, pr.Base.Repo.Name, pr.Index).Return(pr, nil)

common/reviewers.go

@@ -1,9 +1,5 @@
 package common
 
-import (
-	"slices"
-)
-
 type Reviewers struct {
 	Prj []string
 	Pkg []string
@@ -36,10 +32,5 @@ func ParseReviewers(input []string) *Reviewers {
 			*pkg = append(*pkg, reviewer)
 		}
 	}
-
-	if !slices.Contains(r.Prj, Bot_BuildReview) {
-		r.Prj = append(r.Prj, Bot_BuildReview)
-	}
-
 	return r
 }

common/reviewers_test.go

@@ -21,14 +21,14 @@ func TestReviewers(t *testing.T) {
 			name:  "project and package reviewers",
 			input: []string{"1", "2", "3", "*5", "+6", "-7"},
 
-			prj: []string{"5", "7", common.Bot_BuildReview},
+			prj: []string{"5", "7"},
 			pkg: []string{"1", "2", "3", "5", "6"},
 		},
 		{
 			name:  "optional project and package reviewers",
 			input: []string{"~1", "2", "3", "~*5", "+6", "-7"},
 
-			prj:          []string{"7", common.Bot_BuildReview},
+			prj:          []string{"7"},
 			pkg:          []string{"2", "3", "6"},
 			prj_optional: []string{"5"},
 			pkg_optional: []string{"1", "5"},

common/reviews.go

@@ -9,12 +9,14 @@ import (
 )
 
 type PRReviews struct {
-	reviews   []*models.PullReview
-	reviewers []string
-	comments  []*models.TimelineComment
+	Reviews            []*models.PullReview
+	RequestedReviewers []string
+	Comments           []*models.TimelineComment
+
+	FullTimeline []*models.TimelineComment
 }
 
-func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, reviewers []string, org, repo string, no int64) (*PRReviews, error) {
+func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, org, repo string, no int64) (*PRReviews, error) {
 	timeline, err := rf.GetTimeline(org, repo, no)
 	if err != nil {
 		return nil, err
@@ -25,10 +27,14 @@ func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, reviewers []string, org, r
 		return nil, err
 	}
 
-	reviews := make([]*models.PullReview, 0, len(reviewers))
+	reviews := make([]*models.PullReview, 0, 10)
+	needNewReviews := []string{}
 	var comments []*models.TimelineComment
 
 	alreadyHaveUserReview := func(user string) bool {
+		if slices.Contains(needNewReviews, user) {
+			return true
+		}
 		for _, r := range reviews {
 			if r.User != nil && r.User.UserName == user {
 				return true
@@ -37,32 +43,40 @@ func FetchGiteaReviews(rf GiteaReviewTimelineFetcher, reviewers []string, org, r
 		return false
 	}
 
+	LogDebug("FetchingGiteaReviews for", org, repo, no)
+	LogDebug("Number of reviews:", len(rawReviews))
+	LogDebug("Number of items in timeline:", len(timeline))
+
+	cutOffIdx := len(timeline)
 	for idx, item := range timeline {
-		if item.Type == TimelineCommentType_Review {
+		if item.Type == TimelineCommentType_Review || item.Type == TimelineCommentType_ReviewRequested {
 			for _, r := range rawReviews {
 				if r.ID == item.ReviewID {
 					if !alreadyHaveUserReview(r.User.UserName) {
-						reviews = append(reviews, r)
+						if item.Type == TimelineCommentType_Review && idx > cutOffIdx {
+							needNewReviews = append(needNewReviews, r.User.UserName)
+						} else {
+							reviews = append(reviews, r)
+						}
 					}
 					break
 				}
 			}
-		} else if item.Type == TimelineCommentType_Comment {
+		} else if item.Type == TimelineCommentType_Comment && cutOffIdx > idx {
 			comments = append(comments, item)
-		} else if item.Type == TimelineCommentType_PushPull {
-			LogDebug("cut-off", item.Created)
-			timeline = timeline[0:idx]
-			break
+		} else if item.Type == TimelineCommentType_PushPull && cutOffIdx == len(timeline) {
+			LogDebug("cut-off", item.Created, "@", idx)
+			cutOffIdx = idx
 		} else {
 			LogDebug("Unhandled timeline type:", item.Type)
 		}
 	}
-	LogDebug("num comments:", len(comments), "reviews:", len(reviews), len(timeline))
+	LogDebug("num comments:", len(comments), "timeline:", len(reviews))
 
 	return &PRReviews{
-		reviews:   reviews,
-		reviewers: reviewers,
-		comments:  comments,
+		Reviews:      reviews,
+		Comments:     comments,
+		FullTimeline: timeline,
 	}, nil
 }
 
@@ -81,23 +95,27 @@ func bodyCommandManualMergeOK(body string) bool {
 }
 
 func (r *PRReviews) IsManualMergeOK() bool {
-	for _, c := range r.comments {
+	if r == nil {
+		return false
+	}
+
+	for _, c := range r.Comments {
 		if c.Updated != c.Created {
 			continue
 		}
 		LogDebug("comment:", c.User.UserName, c.Body)
-		if slices.Contains(r.reviewers, c.User.UserName) {
+		if slices.Contains(r.RequestedReviewers, c.User.UserName) {
 			if bodyCommandManualMergeOK(c.Body) {
 				return true
 			}
 		}
 	}
 
-	for _, c := range r.reviews {
+	for _, c := range r.Reviews {
 		if c.Updated != c.Submitted {
 			continue
 		}
-		if slices.Contains(r.reviewers, c.User.UserName) {
+		if slices.Contains(r.RequestedReviewers, c.User.UserName) {
 			if bodyCommandManualMergeOK(c.Body) {
 				return true
 			}
@@ -108,11 +126,14 @@ func (r *PRReviews) IsManualMergeOK() bool {
 }
 
 func (r *PRReviews) IsApproved() bool {
+	if r == nil {
+		return false
+	}
 	goodReview := true
 
-	for _, reviewer := range r.reviewers {
+	for _, reviewer := range r.RequestedReviewers {
 		goodReview = false
-		for _, review := range r.reviews {
+		for _, review := range r.Reviews {
 			if review.User.UserName == reviewer && review.State == ReviewStateApproved && !review.Stale && !review.Dismissed {
 				LogDebug("  -- found review: ", review.User.UserName)
 				goodReview = true
@@ -130,7 +151,11 @@ func (r *PRReviews) IsApproved() bool {
 
 func (r *PRReviews) MissingReviews() []string {
 	missing := []string{}
-	for _, reviewer := range r.reviewers {
+	if r == nil {
+		return missing
+	}
+
+	for _, reviewer := range r.RequestedReviewers {
 		if !r.IsReviewedBy(reviewer) {
 			missing = append(missing, reviewer)
 		}
@@ -138,45 +163,64 @@ func (r *PRReviews) MissingReviews() []string {
 	return missing
 }
 
-func (r *PRReviews) HasPendingReviewBy(reviewer string) bool {
-	if !slices.Contains(r.reviewers, reviewer) {
-		return false
+func (r *PRReviews) FindReviewRequester(reviewer string) *models.TimelineComment {
+	if r == nil {
+		return nil
 	}
 
-	isPending := false
-	for _, r := range r.reviews {
-		if r.User.UserName == reviewer && !r.Stale {
-			switch r.State {
-			case ReviewStateApproved:
-				fallthrough
-			case ReviewStateRequestChanges:
-				return false
-			case ReviewStateRequestReview:
-				fallthrough
-			case ReviewStatePending:
-				isPending = true
-			}
+	for _, r := range r.FullTimeline {
+		if r.Type == TimelineCommentType_ReviewRequested && r.Assignee.UserName == reviewer {
+			return r
 		}
 	}
 
-	return isPending
+	return nil
 }
 
-func (r *PRReviews) IsReviewedBy(reviewer string) bool {
-	if !slices.Contains(r.reviewers, reviewer) {
+func (r *PRReviews) HasPendingReviewBy(reviewer string) bool {
+	if r == nil {
 		return false
 	}
 
-	for _, r := range r.reviews {
-		if r.User.UserName == reviewer && !r.Stale {
+	for _, r := range r.Reviews {
+		if r.User.UserName == reviewer {
 			switch r.State {
-			case ReviewStateApproved:
-				return true
-			case ReviewStateRequestChanges:
+			case ReviewStateRequestReview, ReviewStatePending:
 				return true
+			default:
+				return false
 			}
 		}
 	}
 
 	return false
 }
+
+func (r *PRReviews) IsReviewedBy(reviewer string) bool {
+	if r == nil {
+		return false
+	}
+
+	for _, r := range r.Reviews {
+		if r.User.UserName == reviewer && !r.Stale {
+			switch r.State {
+			case ReviewStateApproved, ReviewStateRequestChanges:
+				return true
+			default:
+				return false
+			}
+		}
+	}
+
+	return false
+}
+
+func (r *PRReviews) IsReviewedByOneOf(reviewers ...string) bool {
+	for _, reviewer := range reviewers {
+		if r.IsReviewedBy(reviewer) {
+			return true
+		}
+	}
+
+	return false
+}

common/reviews_test.go

@@ -62,11 +62,23 @@ func TestReviews(t *testing.T) {
 		{
 			name: "Two reviewer, one stale and pending",
 			reviews: []*models.PullReview{
-				&models.PullReview{State: common.ReviewStateRequestReview, User: &models.User{UserName: "user1"}, Stale: true},
+				{State: common.ReviewStateRequestReview, User: &models.User{UserName: "user1"}, Stale: true},
 			},
 			reviewers:         []string{"user1", "user2"},
 			isApproved:        false,
-			isPendingByTest1:  false,
+			isPendingByTest1:  true,
+			isReviewedByTest1: false,
+		},
+		{
+			name: "Two reviewer, one stale and pending, other done",
+			reviews: []*models.PullReview{
+				{State: common.ReviewStateRequestReview, User: &models.User{UserName: "user1"}},
+				{State: common.ReviewStateRequestChanges, User: &models.User{UserName: "user1"}},
+				{State: common.ReviewStateApproved, User: &models.User{UserName: "user2"}},
+			},
+			reviewers:         []string{"user1", "user2"},
+			isApproved:        false,
+			isPendingByTest1:  true,
 			isReviewedByTest1: false,
 		},
 		{
@@ -139,7 +151,7 @@ func TestReviews(t *testing.T) {
 			rf.EXPECT().GetTimeline("test", "pr", int64(1)).Return(test.timeline, nil)
 			rf.EXPECT().GetPullRequestReviews("test", "pr", int64(1)).Return(test.reviews, test.fetchErr)
 
-			reviews, err := common.FetchGiteaReviews(rf, test.reviewers, "test", "pr", 1)
+			reviews, err := common.FetchGiteaReviews(rf, "test", "pr", 1)
 
 			if test.fetchErr != nil {
 				if err != test.fetchErr {
@@ -147,6 +159,7 @@ func TestReviews(t *testing.T) {
 				}
 				return
 			}
+			reviews.RequestedReviewers = test.reviewers
 
 			if r := reviews.IsApproved(); r != test.isApproved {
 				t.Fatal("Unexpected IsReviewed():", r, "vs. expected", test.isApproved)

common/utils.go

@@ -27,10 +27,87 @@ import (
 	"regexp"
 	"slices"
 	"strings"
+	"unicode"
 
 	"src.opensuse.org/autogits/common/gitea-generated/models"
 )
 
+type NewRepos struct {
+	Repos []struct {
+		Organization, Repository, Branch string
+		PackageName                      string
+	}
+	IsMaintainer bool
+}
+
+const maintainership_line = "MAINTAINER"
+
+var true_lines []string = []string{"1", "TRUE", "YES", "OK", "T"}
+
+func HasSpace(s string) bool {
+	return strings.IndexFunc(s, unicode.IsSpace) >= 0
+}
+
+func FindNewReposInIssueBody(body string) *NewRepos {
+	Issues := &NewRepos{}
+	for _, line := range strings.Split(body, "\n") {
+		line = strings.TrimSpace(line)
+		if ul := strings.ToUpper(line); strings.HasPrefix(ul, "MAINTAINER") {
+			value := ""
+			if idx := strings.IndexRune(ul, ':'); idx > 0 && len(ul) > idx+2 {
+				value = ul[idx+1:]
+			} else if idx := strings.IndexRune(ul, ' '); idx > 0 && len(ul) > idx+2 {
+				value = ul[idx+1:]
+			}
+
+			if slices.Contains(true_lines, strings.TrimSpace(value)) {
+				Issues.IsMaintainer = true
+			}
+		}
+		// line = strings.TrimSpace(line)
+		issue := struct{ Organization, Repository, Branch, PackageName string }{}
+
+		branch := strings.Split(line, "#")
+		repo := strings.Split(branch[0], "/")
+
+		if len(branch) == 2 {
+			issue.Branch = strings.TrimSpace(branch[1])
+		}
+		if len(repo) == 2 {
+			issue.Organization = strings.TrimSpace(repo[0])
+			issue.Repository = strings.TrimSpace(repo[1])
+			issue.PackageName = issue.Repository
+
+			if idx := strings.Index(strings.ToUpper(issue.Branch), " AS "); idx > 0 && len(issue.Branch) > idx+5 {
+				issue.PackageName = strings.TrimSpace(issue.Branch[idx+3:])
+				issue.Branch = strings.TrimSpace(issue.Branch[0:idx])
+			}
+
+			if HasSpace(issue.Organization) || HasSpace(issue.Repository) || HasSpace(issue.PackageName) || HasSpace(issue.Branch) {
+				continue
+			}
+		} else {
+			continue
+		}
+		Issues.Repos = append(Issues.Repos, issue)
+		//PackageNameIdx := strings.Index(strings.ToUpper(line), " AS ")
+		//words := strings.Split(line)
+	}
+
+	if len(Issues.Repos) == 0 {
+		return nil
+	}
+	return Issues
+}
+
+func IssueToString(issue *models.Issue) string {
+	if issue == nil {
+		return "(nil)"
+	}
+
+	return fmt.Sprintf("%s/%s#%d", issue.Repository.Owner, issue.Repository.Name, issue.Index)
+}
+
 func SplitLines(str string) []string {
 	return SplitStringNoEmpty(str, "\n")
 }

common/utils_test.go

@@ -1,6 +1,7 @@
 package common_test
 
 import (
+	"reflect"
 	"testing"
 
 	"src.opensuse.org/autogits/common"
@@ -220,3 +221,87 @@ func TestRemovedBranchName(t *testing.T) {
 		})
 	}
 }
+
+func TestNewPackageIssueParsing(t *testing.T) {
+	tests := []struct {
+		name   string
+		input  string
+		issues *common.NewRepos
+	}{
+		{
+			name: "Nothing",
+		},
+		{
+			name:  "Basic repo",
+			input: "org/repo#branch",
+			issues: &common.NewRepos{
+				Repos: []struct{ Organization, Repository, Branch, PackageName string }{
+					{Organization: "org", Repository: "repo", Branch: "branch", PackageName: "repo"},
+				},
+			},
+		},
+		{
+			name: "Default branch and junk lines and approval for maintainership",
+			input: "\n\nsome comments\n\norg1/repo2\n\nmaintainership: yes",
+			issues: &common.NewRepos{
+				Repos: []struct{ Organization, Repository, Branch, PackageName string }{
+					{Organization: "org1", Repository: "repo2", Branch: "", PackageName: "repo2"},
+				},
+				IsMaintainer: true,
+			},
+		},
+		{
+			name: "Default branch and junk lines and no maintainership",
+			input: "\n\nsome comments\n\norg1/repo2\n\nmaintainership: NEVER",
+			issues: &common.NewRepos{
+				Repos: []struct{ Organization, Repository, Branch, PackageName string }{
+					{Organization: "org1", Repository: "repo2", Branch: "", PackageName: "repo2"},
+				},
+			},
+		},
+		{
+			name: "3 repos with comments and maintainership",
+			input: "\n\nsome comments for org1/repo2 are here and more\n\norg1/repo2#master\n    org2/repo3#master\n   some/repo3#m\nMaintainer ok",
+			issues: &common.NewRepos{
+				Repos: []struct{ Organization, Repository, Branch, PackageName string }{
+					{Organization: "org1", Repository: "repo2", Branch: "master", PackageName: "repo2"},
+					{Organization: "org2", Repository: "repo3", Branch: "master", PackageName: "repo3"},
+					{Organization: "some", Repository: "repo3", Branch: "m", PackageName: "repo3"},
+				},
+				IsMaintainer: true,
+			},
+		},
+		{
+			name: "Invalid repos with spaces",
+			input: "or g/repo#branch\norg/r epo#branch\norg/repo#br anch\norg/repo#branch As foo ++",
+		},
+		{
+			name: "Valid repos with spaces",
+			input: " org / repo # branch",
+			issues: &common.NewRepos{
+				Repos: []struct{ Organization, Repository, Branch, PackageName string }{
+					{Organization: "org", Repository: "repo", Branch: "branch", PackageName: "repo"},
+				},
+			},
+		},
+		{
+			name: "Package name is not repo name",
+			input: " org / repo # branch    as  repo++ \nmaintainer true",
+			issues: &common.NewRepos{
+				Repos: []struct{ Organization, Repository, Branch, PackageName string }{
+					{Organization: "org", Repository: "repo", Branch: "branch", PackageName: "repo++"},
+				},
+				IsMaintainer: true,
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			issue := common.FindNewReposInIssueBody(test.input)
+			if !reflect.DeepEqual(test.issues, issue) {
+				t.Error("Expected", test.issues, "but have", issue)
+			}
+		})
+	}
+}

workflow-pr/main.go

@@ -170,7 +170,7 @@ func main() {
 			common.RequestType_PRSync:           req,
 			common.RequestType_PRReviewAccepted: req,
 			common.RequestType_PRReviewRejected: req,
-			common.RequestType_IssueComment:     req,
+			common.RequestType_PRComment:        req,
 		},
 	}
 	listenDefs.Connection().RabbitURL, _ = url.Parse(*rabbitUrl)

workflow-pr/pr_processor.go

@@ -356,13 +356,19 @@ func (pr *PRProcessor) UpdatePrjGitPR(prset *common.PRSet) error {
 
 		// update PR
 		isPrTitleSame := func(CurrentTitle, NewTitle string) bool {
-			if ctlen := len(CurrentTitle); ctlen > 250 && strings.HasSuffix(CurrentTitle, "...") && len(NewTitle) > ctlen {
-				NewTitle = NewTitle[0:ctlen-3] + "..."
+			ctlen := len(CurrentTitle)
+			for _, suffix := range []string{"...", "…"} {
+				slen := len(suffix)
+				if ctlen > 250 && strings.HasSuffix(CurrentTitle, suffix) && len(NewTitle) > ctlen {
+					NewTitle = NewTitle[0:ctlen-slen] + suffix
+					if CurrentTitle == NewTitle {
+						return true
+					}
+				}
 			}
-
 			return CurrentTitle == NewTitle
 		}
-		if PrjGitPR.PR.Body != PrjGitBody || !isPrTitleSame(PrjGitPR.PR.Title, PrjGitTitle) {
+		if PrjGitPR.PR.User.UserName == CurrentUser.UserName && (PrjGitPR.PR.Body != PrjGitBody || !isPrTitleSame(PrjGitPR.PR.Title, PrjGitTitle)) {
 			Gitea.UpdatePullRequest(PrjGit.Owner.UserName, PrjGit.Name, PrjGitPR.PR.Index, &models.EditPullRequestOption{
 				RemoveDeadline: true,
 				Title:          PrjGitTitle,
@@ -548,6 +554,14 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error {
 		return err
 	}
 
+	// update prset if we should build it or not
+	if prjGitPR != nil {
+		if file, err := git.GitCatFile(common.DefaultGitPrj, prjGitPR.PR.Head.Sha, "staging.config"); err == nil {
+			prset.HasAutoStaging = (file != nil)
+			common.LogDebug("  -> automatic staging enabled?:", prset.HasAutoStaging)
+		}
+	}
+
 	// handle case where PrjGit PR is only one left and there are no changes, then we can just close the PR
 	if len(prset.PRs) == 1 && prjGitPR != nil && prset.PRs[0] == prjGitPR && prjGitPR.PR.User.UserName == prset.BotUser {
 		common.LogDebug("  --> checking if superflous PR")