common: fix timeline cache race condition and update logic

The GetTimeline function previously used a strict timestamp comparison (Created > LastCachedTime) to fetch new events. This caused the bot to miss events occurring within the same second as the last update. This change: - Switches to ID-based deduplication to safely handle same-second events. - Correctly updates existing timeline items if they are modified. - Calculates the next 'Since' parameter using the maximum 'Updated' timestamp found in the current cache. This fixes flakiness in integration tests (specifically test_006) where maintainer rejections were occasionally ignored by the workflow-pr service.
2026-03-04 12:11:35 +01:00
10 changed files with 74 additions and 1006 deletions
--- a/2
+++ b/2
@@ -1,4 +1,4 @@
-MODULES := devel-importer utils/hujson utils/maintainer-update gitea-events-rabbitmq-publisher gitea_status_proxy group-review obs-forward-bot obs-groups-bot obs-staging-bot obs-status-service workflow-direct workflow-pr
+MODULES := devel-importer utils/hujson utils/maintainer-update gitea-events-rabbitmq-publisher gitea_status_proxy group-review obs-forward-bot obs-staging-bot obs-status-service workflow-direct workflow-pr

 build:
 	for m in $(MODULES); do go build -C $$m -buildmode=pie || exit 1 ; done
--- a/common/git_utils.go
+++ b/common/git_utils.go
@@ -396,17 +396,12 @@ func (e *GitHandlerImpl) GitExecQuietOrPanic(cwd string, params ...string) {
 }

 type ChanIO struct {
-	ch   chan byte
-	done chan struct{}
+	ch chan byte
 }

 func (c *ChanIO) Write(p []byte) (int, error) {
 	for _, b := range p {
-		select {
-		case c.ch <- b:
-		case <-c.done:
-			return 0, io.EOF
-		}
+		c.ch <- b
 	}
 	return len(p), nil
 }
@@ -415,32 +410,21 @@ func (c *ChanIO) Write(p []byte) (int, error) {
 func (c *ChanIO) Read(data []byte) (idx int, err error) {
 	var ok bool

-	select {
-	case data[idx], ok = <-c.ch:
+	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
 		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
-		}
+		idx++
 	}

 	return
@@ -487,14 +471,7 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
 	var size int

 	pos := 0
-	for {
-		c, ok := <-data
-		if !ok {
-			return GitMsg{}, io.EOF
-		}
-		if c == ' ' {
-			break
-		}
+	for c := <-data; c != ' '; c = <-data {
 		if (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') {
 			id[pos] = c
 			pos++
@@ -506,15 +483,7 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {

 	pos = 0
 	var c byte
-	for {
-		var ok bool
-		c, ok = <-data
-		if !ok {
-			return GitMsg{}, io.EOF
-		}
-		if c == ' ' || c == '\x00' {
-			break
-		}
+	for c = <-data; c != ' ' && c != '\x00'; c = <-data {
 		if c >= 'a' && c <= 'z' {
 			msgType[pos] = c
 			pos++
@@ -540,14 +509,7 @@ func parseGitMsg(data <-chan byte) (GitMsg, error) {
 		return GitMsg{}, fmt.Errorf("Invalid object type: '%s'", string(msgType))
 	}

-	for {
-		c, ok := <-data
-		if !ok {
-			return GitMsg{}, io.EOF
-		}
-		if c == '\x00' {
-			break
-		}
+	for c = <-data; c != '\000'; c = <-data {
 		if c >= '0' && c <= '9' {
 			size = size*10 + (int(c) - '0')
 		} else {
@@ -566,37 +528,18 @@ func parseGitCommitHdr(oldHdr [2]string, data <-chan byte) ([2]string, int, erro
 	hdr := make([]byte, 0, 60)
 	val := make([]byte, 0, 1000)

-	c, ok := <-data
-	if !ok {
-		return [2]string{}, 0, io.EOF
-	}
+	c := <-data
 	size := 1
 	if c != '\n' { // end of header marker
-		for {
-			if c == ' ' {
-				break
-			}
+		for ; c != ' '; c = <-data {
 			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 {
-			var ok bool
-			c, ok = <-data
-			if !ok {
-				return [2]string{}, size, io.EOF
-			}
-			if c == '\n' {
-				break
-			}
+		for c := <-data; c != '\n'; c = <-data {
 			val = append(val, c)
 			size++
 		}
@@ -609,14 +552,7 @@ 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, ok := <-data
-		if !ok {
-			return string(msg), io.EOF
-		}
-		if c == '\x00' {
-			break
-		}
+	for c := <-data; c != '\x00'; c = <-data {
 		msg = append(msg, c)
 		l--
 	}
@@ -642,7 +578,7 @@ func parseGitCommit(data <-chan byte) (GitCommit, error) {
 		var hdr [2]string
 		hdr, size, err := parseGitCommitHdr(hdr, data)
 		if err != nil {
-			return GitCommit{}, err
+			return GitCommit{}, nil
 		}
 		l -= size

@@ -663,28 +599,14 @@ func parseGitCommit(data <-chan byte) (GitCommit, error) {
 func parseTreeEntry(data <-chan byte, hashLen int) (GitTreeEntry, error) {
 	var e GitTreeEntry

-	for {
-		c, ok := <-data
-		if !ok {
-			return e, io.EOF
-		}
-		if c == ' ' {
-			break
-		}
+	for c := <-data; c != ' '; c = <-data {
 		e.mode = e.mode*8 + int(c-'0')
 		e.size++
 	}
 	e.size++

 	name := make([]byte, 0, 128)
-	for {
-		c, ok := <-data
-		if !ok {
-			return e, io.EOF
-		}
-		if c == '\x00' {
-			break
-		}
+	for c := <-data; c != '\x00'; c = <-data {
 		name = append(name, c)
 		e.size++
 	}
@@ -695,10 +617,7 @@ func parseTreeEntry(data <-chan byte, hashLen int) (GitTreeEntry, error) {

 	hash := make([]byte, 0, hashLen*2)
 	for range hashLen {
-		c, ok := <-data
-		if !ok {
-			return e, io.EOF
-		}
+		c := <-data
 		hash = append(hash, hexBinToAscii[((c&0xF0)>>4)], hexBinToAscii[c&0xF])
 	}
 	e.hash = string(hash)
@@ -719,16 +638,13 @@ func parseGitTree(data <-chan byte) (GitTree, error) {
 	for parsedLen < hdr.size {
 		entry, err := parseTreeEntry(data, len(hdr.hash)/2)
 		if err != nil {
-			return GitTree{}, err
+			return GitTree{}, nil
 		}

 		t.items = append(t.items, entry)
 		parsedLen += entry.size
 	}
-	c, ok := <-data // \0 read
-	if !ok {
-		return t, io.EOF
-	}
+	c := <-data // \0 read

 	if c != '\x00' {
 		return t, fmt.Errorf("Unexpected character during git tree data read")
@@ -749,16 +665,9 @@ func parseGitBlob(data <-chan byte) ([]byte, error) {

 	d := make([]byte, hdr.size)
 	for l := 0; l < hdr.size; l++ {
-		var ok bool
-		d[l], ok = <-data
-		if !ok {
-			return d, io.EOF
-		}
-	}
-	eob, ok := <-data
-	if !ok {
-		return d, io.EOF
+		d[l] = <-data
 	}
+	eob := <-data
 	if eob != '\x00' {
 		return d, fmt.Errorf("invalid byte read in parseGitBlob")
 	}
@@ -770,25 +679,16 @@ func (e *GitHandlerImpl) GitParseCommits(cwd string, commitIDs []string) (parsed
 	var done sync.Mutex

 	done.Lock()
-	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}
+	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
 	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.Write([]byte{0})
+			data_out.ch <- '\x00'
 			c, e := parseGitCommit(data_in.ch)
 			if e != nil {
 				err = fmt.Errorf("Error parsing git commit: %w", e)
@@ -815,14 +715,12 @@ 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
 }

@@ -831,21 +729,15 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
 	var done sync.Mutex

 	done.Lock()
-	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}
+	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}

 	go func() {
 		defer done.Unlock()
-		defer close_done()
 		defer close(data_out.ch)
+
 		data_out.Write([]byte(commitId))
-		data_out.Write([]byte{0})
+		data_out.ch <- '\x00'
+
 		var c GitCommit
 		c, err = parseGitCommit(data_in.ch)
 		if err != nil {
@@ -853,9 +745,11 @@ func (e *GitHandlerImpl) GitCatFile(cwd, commitId, filename string) (data []byte
 			return
 		}
 		data_out.Write([]byte(c.Tree))
-		data_out.Write([]byte{0})
+		data_out.ch <- '\x00'
+
 		var tree GitTree
 		tree, err = parseGitTree(data_in.ch)
+
 		if err != nil {
 			LogError("Error parsing git tree:", err)
 			return
@@ -865,7 +759,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.Write([]byte{0})
+				data_out.ch <- '\x00'
 				data, err = parseGitBlob(data_in.ch)
 				return
 			}
@@ -890,13 +784,11 @@ 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
 }

@@ -906,24 +798,16 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
 	directoryList = make(map[string]string)

 	done.Lock()
-	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}
+	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}

 	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.Write([]byte{0})
+		data_out.ch <- '\x00'
 		var c GitCommit
 		c, err = parseGitCommit(data_in.ch)
 		if err != nil {
@@ -939,7 +823,7 @@ func (e *GitHandlerImpl) GitDirectoryList(gitPath, commitId string) (directoryLi
 				delete(trees, p)

 				data_out.Write([]byte(tree))
-				data_out.Write([]byte{0})
+				data_out.ch <- '\x00'
 				var tree GitTree
 				tree, err = parseGitTree(data_in.ch)

@@ -973,14 +857,12 @@ 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
 }

@@ -990,14 +872,7 @@ func (e *GitHandlerImpl) GitDirectoryContentList(gitPath, commitId string) (dire
 	directoryList = make(map[string]string)

 	done.Lock()
-	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}
+	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}

 	LogDebug("Getting directory content for:", commitId)

@@ -1006,7 +881,7 @@ func (e *GitHandlerImpl) GitDirectoryContentList(gitPath, commitId string) (dire
 		defer close(data_out.ch)

 		data_out.Write([]byte(commitId))
-		data_out.Write([]byte{0})
+		data_out.ch <- '\x00'
 		var c GitCommit
 		c, err = parseGitCommit(data_in.ch)
 		if err != nil {
@@ -1022,7 +897,7 @@ func (e *GitHandlerImpl) GitDirectoryContentList(gitPath, commitId string) (dire
 				delete(trees, p)

 				data_out.Write([]byte(tree))
-				data_out.Write([]byte{0})
+				data_out.ch <- '\x00'
 				var tree GitTree
 				tree, err = parseGitTree(data_in.ch)

@@ -1058,14 +933,12 @@ func (e *GitHandlerImpl) GitDirectoryContentList(gitPath, commitId string) (dire
 	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
 }

@@ -1075,24 +948,16 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
 	submoduleList = make(map[string]string)

 	done.Lock()
-	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}
+	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}

 	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.Write([]byte{0})
+		data_out.ch <- '\x00'
 		var c GitCommit
 		c, err = parseGitCommit(data_in.ch)
 		if err != nil {
@@ -1108,7 +973,7 @@ func (e *GitHandlerImpl) GitSubmoduleList(gitPath, commitId string) (submoduleLi
 				delete(trees, p)

 				data_out.Write([]byte(tree))
-				data_out.Write([]byte{0})
+				data_out.ch <- '\x00'
 				var tree GitTree
 				tree, err = parseGitTree(data_in.ch)

@@ -1145,26 +1010,17 @@ 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) {
-	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}
+	data_in, data_out := ChanIO{make(chan byte)}, ChanIO{make(chan byte)}
 	var wg sync.WaitGroup

 	wg.Add(1)
@@ -1180,18 +1036,17 @@ 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.Write([]byte{0})
+		data_out.ch <- '\x00'
 		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.Write([]byte{0})
+		data_out.ch <- '\x00'
 		tree, err := parseGitTree(data_in.ch)

 		if err != nil {
@@ -1223,14 +1078,12 @@ 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
 }

--- a/common/git_utils_test.go
+++ b/common/git_utils_test.go
@@ -28,7 +28,6 @@ import (
 	"slices"
 	"strings"
 	"testing"
-	"time"
 )

 func TestGitClone(t *testing.T) {
@@ -718,44 +717,3 @@ func TestGitDirectoryListRepro(t *testing.T) {
 		t.Errorf("Expected 'subdir' in directory list, got %v", dirs)
 	}
 }
-
-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()
-
-	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.")
-	}
-}
--- a/common/gitea_utils.go
+++ b/common/gitea_utils.go
@@ -863,9 +863,10 @@ func (gitea *GiteaTransport) GetTimeline(org, repo string, idx int64) ([]*models
 	TimelineCache, IsCached := giteaTimelineCache[prID]
 	var LastCachedTime strfmt.DateTime
 	if IsCached {
-		l := len(TimelineCache.data)
-		if l > 0 {
-			LastCachedTime = TimelineCache.data[0].Updated
+		for _, d := range TimelineCache.data {
+			if time.Time(d.Updated).Compare(time.Time(LastCachedTime)) > 0 {
+				LastCachedTime = d.Updated
+			}
 		}

 		// cache data for 5 seconds
@@ -894,14 +895,20 @@ func (gitea *GiteaTransport) GetTimeline(org, repo string, idx int64) ([]*models
 		}

 		for _, d := range res.Payload {
-			if d != nil {
-				if time.Time(d.Created).Compare(time.Time(LastCachedTime)) > 0 {
-					// created after last check, so we append here
-					TimelineCache.data = append(TimelineCache.data, d)
-				} else {
-					// we need something updated in the timeline, maybe
+			if d == nil {
+				continue
+			}
+			found := false
+			for i := range TimelineCache.data {
+				if TimelineCache.data[i].ID == d.ID {
+					TimelineCache.data[i] = d
+					found = true
+					break
 				}
 			}
+			if !found {
+				TimelineCache.data = append(TimelineCache.data, d)
+			}
 		}

 		if resCount < 10 {
--- a/integration/tests/workflow_pr_merge_test.py
+++ b/integration/tests/workflow_pr_merge_test.py
@@ -137,108 +137,6 @@ index 0000000..e69de29
    assert project_merged, f"Project PR myproducts/mySLFO#{project_pr_number} was not merged after 'merge ok'."
    print("Both PRs merged successfully after 'merge ok'.")

-@pytest.mark.t003
-def test_003_refuse_manual_merge(manual_merge_env, test_user_client, ownerB_client, staging_bot_client):
-    """
-    Test scenario TC-MERGE-003:
-    1. Create a PackageGit PR with ManualMergeOnly set to true.
-    2. Ensure all mandatory reviews are completed on both project and package PRs.
-    3. Comment "merge ok" on the package PR from the account of a not requested reviewer.
-    4. Verify the PR is not merged.
-    """
-    gitea_env, test_full_repo_name, merge_branch_name = manual_merge_env
-
-    # 1. Create a package PR
-    diff = """diff --git a/manual_merge_test.txt b/manual_merge_test.txt
-new file mode 100644
-index 0000000..e69de29
-"""
-    print(f"--- Creating package PR in mypool/pkgA on branch {merge_branch_name} ---")
-    package_pr = test_user_client.create_gitea_pr("mypool/pkgA", diff, "Test Manual Merge Fixture", False, base_branch=merge_branch_name)
-    package_pr_number = package_pr["number"]
-    print(f"Created package PR mypool/pkgA#{package_pr_number}")
-
-    # 2. Make sure the workflow-pr service created related project PR
-    project_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", package_pr_number)
-    assert project_pr_number is not None, "Workflow bot did not create a project PR."
-    print(f"Found project PR: myproducts/mySLFO#{project_pr_number}")
-
-    # 3. Approve reviews and verify NOT merged
-    print("Waiting for all expected review requests and approving them...")
-    # Expected reviewers based on manual-merge branch config and pkgA maintainership
-    expected_reviewers = {"usera", "userb", "ownerA", "ownerX", "ownerY"}
-    
-    # ManualMergeOnly still requires regular reviews to be satisfied.
-    # We poll until all expected reviewers are requested, then approve them.
-    all_requested = False
-    for _ in range(30):
-        # Trigger approvals for whatever is already requested
-        gitea_env.approve_requested_reviews("mypool/pkgA", package_pr_number)
-        gitea_env.approve_requested_reviews("myproducts/mySLFO", project_pr_number)
-        
-        # Explicitly handle staging bot if it is requested or pending
-        prj_reviews = gitea_env.list_reviews("myproducts/mySLFO", project_pr_number)
-        if any(r["user"]["login"] == "autogits_obs_staging_bot" and r["state"] in ["REQUEST_REVIEW", "PENDING"] for r in prj_reviews):
-            print("Staging bot has a pending/requested review. Approving...")
-            staging_bot_client.create_review("myproducts/mySLFO", project_pr_number, event="APPROVED", body="Staging bot approves")
-
-        # Check if all expected reviewers have at least one review record (any state)
-        pkg_reviews = gitea_env.list_reviews("mypool/pkgA", package_pr_number)
-        current_reviewers = {r["user"]["login"] for r in pkg_reviews}
-        
-        if expected_reviewers.issubset(current_reviewers):
-            # Also ensure they are all approved (not just requested)
-            approved_reviewers = {r["user"]["login"] for r in pkg_reviews if r["state"] == "APPROVED"}
-            if expected_reviewers.issubset(approved_reviewers):
-                # And check project PR for bot approval
-                prj_approved = any(r["user"]["login"] == "autogits_obs_staging_bot" and r["state"] == "APPROVED" for r in prj_reviews)
-                if prj_approved:
-                    all_requested = True
-                    print(f"All expected reviewers {expected_reviewers} and staging bot have approved.")
-                    break
-        
-        pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number)
-        prj_details = gitea_env.get_pr_details("myproducts/mySLFO", project_pr_number)
-        
-        assert not pkg_details.get("merged"), "Package PR merged prematurely (ManualMergeOnly ignored?)"
-        assert not prj_details.get("merged"), "Project PR merged prematurely (ManualMergeOnly ignored?)"
-        
-        time.sleep(2)
-
-    assert all_requested, f"Timed out waiting for all expected reviewers {expected_reviewers} to approve. Current: {current_reviewers}"
-    print("Both PRs have all required approvals but are not merged (as expected with ManualMergeOnly).")
-
-    # 4. Comment "merge ok" from a requested reviewer (ownerB)
-    print("Commenting 'merge ok' on package PR as user ownerB ...")
-    ownerB_client.create_issue_comment("mypool/pkgA", package_pr_number, "merge ok")
-
-    # 5. Verify both PRs are merged
-    print("Polling for PR merge status...")
-    package_merged = False
-    project_merged = False
-    
-    for i in range(20): # Poll for up to 20 seconds
-        if not package_merged:
-            pkg_details = gitea_env.get_pr_details("mypool/pkgA", package_pr_number)
-            if pkg_details.get("merged"):
-                package_merged = True
-                print(f"Package PR mypool/pkgA#{package_pr_number} merged.")
-        
-        if not project_merged:
-            prj_details = gitea_env.get_pr_details("myproducts/mySLFO", project_pr_number)
-            if prj_details.get("merged"):
-                project_merged = True
-                print(f"Project PR myproducts/mySLFO#{project_pr_number} merged.")
-        
-        if package_merged and project_merged:
-            break
-            
-        time.sleep(1)
-            
-    assert not package_merged, f"Package PR mypool/pkgA#{package_pr_number} was merged after 'merge ok'."
-    assert not project_merged, f"Project PR myproducts/mySLFO#{project_pr_number} was merged after 'merge ok'."
-    print("Both PRs merged not after 'merge ok'.")
-
@pytest.mark.t008
 def test_008_merge_mode_ff_only_success(merge_ff_env, test_user_client):
    """
--- a/integration/tests/workflow_pr_review_test.py
+++ b/integration/tests/workflow_pr_review_test.py
@@ -139,7 +139,6 @@ index 0000000..e69de29


@pytest.mark.t005
-# @pytest.mark.xfail(reason="TBD troubleshoot")
 def test_005_any_maintainer_approval_sufficient(maintainer_env, ownerA_client, ownerBB_client):
    """
    Test scenario:
@@ -201,7 +200,6 @@ index 0000000..e69de29


@pytest.mark.t006
-@pytest.mark.xfail(reason="tbd flacky in ci")
 def test_006_maintainer_rejection_removes_other_requests(maintainer_env, ownerA_client, ownerBB_client):
    """
    Test scenario:
--- a/obs-groups-bot/.gitignore
+++ b/obs-groups-bot/.gitignore
@@ -1 +0,0 @@
-obs-groups-bot
--- a/obs-groups-bot/main.go
+++ b/obs-groups-bot/main.go
@@ -1,399 +0,0 @@
-// Connect to the Open Build Service (OBS) API, retrieves a list of all groups,
-// and exports their metadata (specifically member lists) into individual JSON files.
-//
-// The tool supports both command-line flags and environment variables for configuration,
-// and includes a debug mode for verbose output. It handles different XML response formats
-// from the OBS API and ensures that the output JSON files are properly sanitized and formatted.
-//
-// The accepted command-line flags are:
-//
-//	-debug: Enable debug output showing API URLs and responses.
-//	-instance: Name of the OBS instance (used in metadata, default "openSUSE").
-//	-host: Base URL of the OBS API (default "http://localhost:3000").
-//	-user: OBS username (or set via OBS_USER environment variable).
-//	-password: OBS password (or set via OBS_PASSWORD environment variable).
-//	-output: Directory to save the JSON files (default "groups").
-//
-// Usage:
-//
-//		# Using flags for credentials
-//		go run main.go -user "myuser" -password "mypass"
-//
-//		# Using environment variables (OBS_USER, OBS_PASSWORD)
-//		go run main.go
-//
-//		# Targeting a specific OBS instance and output directory
-//		go run main.go -host "https://api.opensuse.org" -output "./obs_groups"
-//
-//	 	# Full command with debug mode
-//	 	go run main.go -host http://localhost:8000 -user "myuser" -password "mypass" -output "./obs_groups" -instance "OBS" -debug
-
-package main
-
-import (
-	"context"
-	"encoding/json"
-	"encoding/xml"
-	"flag"
-	"fmt"
-	"io"
-	"log"
-	"net/http"
-	"net/url"
-	"os"
-	"path/filepath"
-	"strings"
-	"time"
-)
-
-const (
-	envOBSUser     = "OBS_USER"
-	envOBSPassword = "OBS_PASSWORD"
-)
-
-type ObsClient struct {
-	baseUrl  *url.URL
-	client   *http.Client
-	user     string
-	password string
-	cookie   string
-	debug    bool
-}
-
-type groupsList struct {
-	XMLName xml.Name    `xml:"groups"`
-	Groups  []groupItem `xml:"group"`
-}
-
-type groupsListAlt struct {
-	XMLName xml.Name     `xml:"directory"`
-	Entries []groupEntry `xml:"entry"`
-}
-
-type groupEntry struct {
-	Name  string `xml:"name,attr,omitempty"`
-	Inner string `xml:",innerxml"`
-}
-
-func (e *groupEntry) getName() string {
-	if e.Name != "" {
-		return e.Name
-	}
-	return e.Inner
-}
-
-type groupItem struct {
-	GroupID string `xml:"groupid,attr"`
-}
-
-type personRepoMeta struct {
-	XMLName xml.Name `xml:"person"`
-	UserID  string   `xml:"userid,attr"`
-	Role    string   `xml:"role,attr,omitempty"`
-}
-
-type personGroup struct {
-	XMLName xml.Name         `xml:"person"`
-	Persons []personRepoMeta `xml:"person"`
-}
-
-type groupMeta struct {
-	XMLName xml.Name    `xml:"group"`
-	Title   string      `xml:"title"`
-	Persons personGroup `xml:"person"`
-}
-
-func NewObsClient(host, user, password string, debug bool) (*ObsClient, error) {
-	if host == "" {
-		return nil, fmt.Errorf("host URL cannot be empty")
-	}
-	if user == "" || password == "" {
-		return nil, fmt.Errorf("username and password are required")
-	}
-
-	baseUrl, err := url.Parse(host)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse host URL: %w", err)
-	}
-
-	if baseUrl.Scheme == "" || baseUrl.Host == "" {
-		return nil, fmt.Errorf("host URL must contain scheme (http:// or https://) and hostname")
-	}
-
-	return &ObsClient{
-		baseUrl:  baseUrl,
-		client:   &http.Client{Timeout: 30 * time.Second},
-		user:     user,
-		password: password,
-		debug:    debug,
-	}, nil
-}
-
-func (c *ObsClient) ObsRequest(ctx context.Context, method string, url_path []string, body io.Reader) (*http.Response, error) {
-	fullURL := c.baseUrl.JoinPath(url_path...).String()
-	return c.ObsRequestRaw(ctx, method, fullURL, body)
-}
-
-func (c *ObsClient) ObsRequestRaw(ctx context.Context, method string, url string, body io.Reader) (*http.Response, error) {
-	if c.debug {
-		log.Printf("[DEBUG] %s %s", method, url)
-	}
-
-	req, err := http.NewRequestWithContext(ctx, method, url, body)
-	if err != nil {
-		return nil, err
-	}
-	if body != nil {
-		req.Body = io.NopCloser(body)
-	}
-	if c.cookie != "" {
-		req.Header.Add("Cookie", c.cookie)
-	}
-	if c.user != "" && c.password != "" {
-		req.SetBasicAuth(c.user, c.password)
-	}
-
-	res, err := c.client.Do(req)
-	if err != nil {
-		return nil, err
-	}
-
-	if res.StatusCode == 200 {
-		auth_cookie := res.Header.Get("set-cookie")
-		if auth_cookie != "" {
-			c.cookie = auth_cookie
-		}
-	}
-
-	return res, nil
-}
-
-func (c *ObsClient) GetAllGroups(ctx context.Context) ([]string, error) {
-	res, err := c.ObsRequest(ctx, "GET", []string{"group"}, nil)
-	if err != nil {
-		return nil, err
-	}
-	defer res.Body.Close()
-
-	data, err := io.ReadAll(res.Body)
-	if err != nil {
-		return nil, err
-	}
-
-	log.Printf("Response status: %d, body length: %d", res.StatusCode, len(data))
-	if res.StatusCode != 200 {
-		bodyStr := string(data)
-		if len(bodyStr) > 500 {
-			bodyStr = bodyStr[:500]
-		}
-		return nil, fmt.Errorf("Unexpected return code: %d, body: %s", res.StatusCode, bodyStr)
-	}
-
-	// Try parsing as <groups> format
-	var groupsList groupsList
-	err = xml.Unmarshal(data, &groupsList)
-	if err == nil && len(groupsList.Groups) > 0 {
-		groupIDs := make([]string, len(groupsList.Groups))
-		for i, g := range groupsList.Groups {
-			groupIDs[i] = g.GroupID
-		}
-		return groupIDs, nil
-	}
-
-	// Try parsing as <directory> format
-	var groupsAlt groupsListAlt
-	err = xml.Unmarshal(data, &groupsAlt)
-	if err == nil && len(groupsAlt.Entries) > 0 {
-		groupIDs := make([]string, len(groupsAlt.Entries))
-		for i, e := range groupsAlt.Entries {
-			groupIDs[i] = e.getName()
-		}
-		return groupIDs, nil
-	}
-
-	// Log what we got
-	bodyStr := string(data)
-	if len(bodyStr) > 1000 {
-		bodyStr = bodyStr[:1000]
-	}
-	log.Printf("Failed to parse XML, got: %s", bodyStr)
-	return nil, fmt.Errorf("Could not parse groups response")
-}
-
-func (c *ObsClient) GetGroupMeta(ctx context.Context, gid string) (*groupMeta, error) {
-	log.Printf("[DEBUG] gid: %s", gid)
-	groupPath := []string{"group", gid}
-	fullURL := c.baseUrl.JoinPath(groupPath...).String()
-	if c.debug {
-		log.Printf("[DEBUG] Fetching group: %s", fullURL)
-	}
-	res, err := c.ObsRequestRaw(ctx, "GET", fullURL, nil)
-	if err != nil {
-		return nil, err
-	}
-	defer res.Body.Close()
-
-	switch res.StatusCode {
-	case 200:
-		break
-	case 404:
-		return nil, nil
-	default:
-		return nil, fmt.Errorf("Unexpected return code: %d", res.StatusCode)
-	}
-
-	data, err := io.ReadAll(res.Body)
-	if err != nil {
-		return nil, err
-	}
-
-	var meta groupMeta
-	err = xml.Unmarshal(data, &meta)
-	if err != nil {
-		return nil, err
-	}
-
-	return &meta, nil
-}
-
-type GroupOutput struct {
-	Meta      ImportMeta `json:"_meta,omitempty"`
-	Name      string     `json:"Name"`
-	Reviewers []string   `json:"Reviewers"`
-	Silent    bool       `json:"Silent,omitempty"`
-}
-
-type ImportMeta struct {
-	ImportedFrom string    `json:"imported_from"`
-	ReadOnly     bool      `json:"read_only"`
-	ImportTime   time.Time `json:"import_time"`
-}
-
-func sanitizeFilename(name string) string {
-	name = strings.ReplaceAll(name, "/", "_")
-	name = strings.ReplaceAll(name, ":", "_")
-	name = strings.ReplaceAll(name, " ", "_")
-	return name
-}
-
-func processGroup(ctx context.Context, client *ObsClient, groupID, outputDir, instanceName string, importTime time.Time) error {
-	meta, err := client.GetGroupMeta(ctx, groupID)
-	if err != nil {
-		return fmt.Errorf("fetching group meta: %w", err)
-	}
-
-	if meta == nil {
-		return fmt.Errorf("group not found")
-	}
-
-	if client.debug {
-		log.Printf("[DEBUG] Group meta for %s: Title: %s, Persons: %d", groupID, meta.Title, len(meta.Persons.Persons))
-	}
-
-	reviewers := make([]string, 0, len(meta.Persons.Persons))
-	for _, p := range meta.Persons.Persons {
-		reviewers = append(reviewers, p.UserID)
-	}
-
-	output := GroupOutput{
-		Meta: ImportMeta{
-			ImportedFrom: instanceName,
-			ReadOnly:     true,
-			ImportTime:   importTime,
-		},
-		Name:      groupID,
-		Reviewers: reviewers,
-	}
-
-	filename := sanitizeFilename(groupID) + ".json"
-	filePath := filepath.Join(outputDir, filename)
-
-	data, err := json.MarshalIndent(output, "", "  ")
-	if err != nil {
-		return fmt.Errorf("marshaling json: %w", err)
-	}
-
-	if err := os.WriteFile(filePath, data, 0644); err != nil {
-		return fmt.Errorf("writing file: %w", err)
-	}
-
-	if client.debug {
-		log.Printf("[DEBUG] Saved group %s to %s", groupID, filePath)
-	}
-
-	return nil
-}
-
-func main() {
-	debugModePtr := flag.Bool("debug", false, "Enable debug output showing API URLs")
-	obsInstance := flag.String("instance", "openSUSE", "OBS instance name (used in metadata)")
-	obsHost := flag.String("host", "http://localhost:3000", "OBS API host URL")
-	obsUser := flag.String("user", "", "OBS username (or set OBS_USER env)")
-	obsPassword := flag.String("password", "", "OBS password (or set OBS_PASSWORD env)")
-	outputDir := flag.String("output", "groups", "Output directory for JSON files")
-	flag.Parse()
-
-	obsUserVal := *obsUser
-	obsPasswordVal := *obsPassword
-
-	if obsUserVal == "" {
-		if *debugModePtr {
-			log.Printf("[DEBUG] No OBS user provided. Trying environment variables `%s`...", envOBSUser)
-		}
-		obsUserVal = os.Getenv(envOBSUser)
-	}
-	if obsPasswordVal == "" {
-		if *debugModePtr {
-			log.Printf("[DEBUG] No OBS password provided. Trying environment variables `%s`...", envOBSPassword)
-		}
-		obsPasswordVal = os.Getenv(envOBSPassword)
-	}
-
-	if obsUserVal == "" || obsPasswordVal == "" {
-		log.Fatalf("OBS credentials required. Set -user/-password flags or %s/%s environment variables", envOBSUser, envOBSPassword)
-	}
-
-	log.Printf("Connecting to OBS at %s (instance: %s)", *obsHost, *obsInstance)
-
-	client, err := NewObsClient(*obsHost, obsUserVal, obsPasswordVal, *debugModePtr)
-	if err != nil {
-		log.Fatalf("Failed to create OBS client: %v", err)
-	}
-
-	ctx := context.Background()
-
-	log.Println("Fetching list of all groups...")
-	groupIDs, err := client.GetAllGroups(ctx)
-	if err != nil {
-		log.Fatalf("Failed to get groups list: %v", err)
-	}
-
-	log.Printf("Found %d groups: %v", len(groupIDs), groupIDs)
-	log.Printf("Found %s ", groupIDs)
-
-	err = os.MkdirAll(*outputDir, 0755)
-	if err != nil {
-		log.Fatalf("Failed to create output directory: %v", err)
-	}
-
-	importTime := time.Now()
-
-	successCount := 0
-	errorCount := 0
-
-	for i, groupID := range groupIDs {
-		log.Printf("[%d/%d] Fetching group: %s", i+1, len(groupIDs), groupID)
-
-		if err := processGroup(ctx, client, groupID, *outputDir, *obsInstance, importTime); err != nil {
-			log.Printf("Error processing group %s: %v", groupID, err)
-			errorCount++
-			continue
-		}
-
-		successCount++
-		time.Sleep(100 * time.Millisecond)
-	}
-
-	log.Printf("Done! Success: %d, Errors: %d", successCount, errorCount)
-	log.Printf("JSON files saved to: %s", *outputDir)
-}
--- a/obs-groups-bot/main_test.go
+++ b/obs-groups-bot/main_test.go
@@ -1,247 +0,0 @@
-package main
-
-import (
-	"context"
-	"encoding/json"
-	"encoding/xml"
-	"net/http"
-	"net/http/httptest"
-	"os"
-	"path/filepath"
-	"reflect"
-	"testing"
-	"time"
-)
-
-func TestGroupsListParsing(t *testing.T) {
-	// Test <groups> format
-	groupsXML := `<?xml version="1.0" encoding="utf-8"?>
-<groups>
-  <group groupid="group1"/>
-  <group groupid="group2"/>
-  <group groupid="group3"/>
-</groups>`
-
-	var groupsList groupsList
-	err := xml.Unmarshal([]byte(groupsXML), &groupsList)
-	if err != nil {
-		t.Fatalf("Failed to unmarshal groups XML: %v", err)
-	}
-
-	if len(groupsList.Groups) != 3 {
-		t.Errorf("Expected 3 groups, got %d", len(groupsList.Groups))
-	}
-
-	expected := []string{"group1", "group2", "group3"}
-	for i, g := range groupsList.Groups {
-		if g.GroupID != expected[i] {
-			t.Errorf("Expected group %s, got %s", expected[i], g.GroupID)
-		}
-	}
-}
-
-func TestProcessGroup(t *testing.T) {
-	// 1. Mock the OBS API server for GetGroupMeta
-	groupID := "test:group"
-	mockGroupMetaResponse := `<?xml version="1.0" encoding="utf-8"?>
-<group>
-  <title>Test Group Title</title>
-  <person>
-    <person userid="user1" role="maintainer"/>
-    <person userid="user2" role="reviewer"/>
-  </person>
-</group>`
-
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		expectedPath := "/group/" + groupID
-		if r.URL.Path != expectedPath {
-			t.Errorf("Expected path %s, got %s", expectedPath, r.URL.Path)
-			http.NotFound(w, r)
-			return
-		}
-		w.WriteHeader(http.StatusOK)
-		w.Write([]byte(mockGroupMetaResponse))
-	}))
-	defer server.Close()
-
-	// 2. Create a temporary directory for output
-	outputDir := t.TempDir()
-
-	// 3. Initialize client pointing to mock server
-	client, err := NewObsClient(server.URL, "testuser", "testpass", false)
-	if err != nil {
-		t.Fatalf("Failed to create client: %v", err)
-	}
-
-	// 4. Call processGroup
-	instanceName := "test-instance"
-	importTime := time.Now().UTC().Truncate(time.Second) // Truncate for stable comparison
-	err = processGroup(context.Background(), client, groupID, outputDir, instanceName, importTime)
-	if err != nil {
-		t.Fatalf("processGroup failed: %v", err)
-	}
-
-	// 5. Verify the output file
-	expectedFilename := sanitizeFilename(groupID) + ".json"
-	filePath := filepath.Join(outputDir, expectedFilename)
-
-	// Check if file exists
-	if _, err := os.Stat(filePath); os.IsNotExist(err) {
-		t.Fatalf("Expected output file was not created: %s", filePath)
-	}
-
-	// Read and verify file content
-	data, err := os.ReadFile(filePath)
-	if err != nil {
-		t.Fatalf("Failed to read output file: %v", err)
-	}
-
-	var result GroupOutput
-	if err := json.Unmarshal(data, &result); err != nil {
-		t.Fatalf("Failed to unmarshal output JSON: %v", err)
-	}
-
-	// Assertions
-	expectedReviewers := []string{"user1", "user2"}
-	expectedOutput := GroupOutput{
-		Meta: ImportMeta{
-			ImportedFrom: instanceName,
-			ReadOnly:     true,
-			ImportTime:   importTime,
-		},
-		Name:      groupID,
-		Reviewers: expectedReviewers,
-	}
-
-	// Use reflect.DeepEqual for a robust comparison of the structs
-	if !reflect.DeepEqual(result, expectedOutput) {
-		t.Errorf("Output JSON does not match expected.\nGot: %+v\nWant: %+v", result, expectedOutput)
-	}
-}
-
-func TestObsClient_GetAllGroups(t *testing.T) {
-	// Mock the OBS API server
-	mockResponse := `<?xml version="1.0" encoding="utf-8"?>
-<groups>
-  <group groupid="mock-group-1"/>
-  <group groupid="mock-group-2"/>
-</groups>`
-
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// Verify the request path
-		if r.URL.Path != "/group" {
-			t.Errorf("Expected path /group, got %s", r.URL.Path)
-		}
-		// Verify method
-		if r.Method != "GET" {
-			t.Errorf("Expected method GET, got %s", r.Method)
-		}
-		w.WriteHeader(http.StatusOK)
-		w.Write([]byte(mockResponse))
-	}))
-	defer server.Close()
-
-	// Initialize client pointing to mock server
-	client, err := NewObsClient(server.URL, "testuser", "testpass", false)
-	if err != nil {
-		t.Fatalf("Failed to create client: %v", err)
-	}
-
-	groups, err := client.GetAllGroups(context.Background())
-	if err != nil {
-		t.Fatalf("GetAllGroups failed: %v", err)
-	}
-
-	if len(groups) != 2 {
-		t.Errorf("Expected 2 groups, got %d", len(groups))
-	}
-	if groups[0] != "mock-group-1" {
-		t.Errorf("Expected first group to be mock-group-1, got %s", groups[0])
-	}
-}
-
-func TestGroupsListDirectoryFormat(t *testing.T) {
-	// Test <directory> format with name attribute
-	dirXML := `<?xml version="1.0" encoding="utf-8"?>
-<directory>
-  <entry name="group-a"/>
-  <entry name="group-b"/>
-  <entry name="group-c"/>
-</directory>`
-
-	var groupsAlt groupsListAlt
-	err := xml.Unmarshal([]byte(dirXML), &groupsAlt)
-	if err != nil {
-		t.Fatalf("Failed to unmarshal directory XML: %v", err)
-	}
-
-	if len(groupsAlt.Entries) != 3 {
-		t.Errorf("Expected 3 entries, got %d", len(groupsAlt.Entries))
-	}
-
-	expected := []string{"group-a", "group-b", "group-c"}
-	for i, e := range groupsAlt.Entries {
-		if e.getName() != expected[i] {
-			t.Errorf("Expected entry %s, got %s", expected[i], e.getName())
-		}
-	}
-}
-
-func TestGroupMetaParsing(t *testing.T) {
-	groupXML := `<?xml version="1.0" encoding="utf-8"?>
-<group>
-  <title>Test Group Title</title>
-  <person>
-    <person userid="user1" role="maintainer"/>
-    <person userid="user2" role="reviewer"/>
-    <person userid="user3"/>
-  </person>
-</group>`
-
-	var meta groupMeta
-	err := xml.Unmarshal([]byte(groupXML), &meta)
-	if err != nil {
-		t.Fatalf("Failed to unmarshal group meta XML: %v", err)
-	}
-
-	if meta.Title != "Test Group Title" {
-		t.Errorf("Expected title 'Test Group Title', got '%s'", meta.Title)
-	}
-
-	if len(meta.Persons.Persons) != 3 {
-		t.Errorf("Expected 3 persons, got %d", len(meta.Persons.Persons))
-	}
-
-	persons := meta.Persons.Persons
-	if persons[0].UserID != "user1" || persons[0].Role != "maintainer" {
-		t.Errorf("First person should be user1 with role maintainer")
-	}
-	if persons[1].UserID != "user2" || persons[1].Role != "reviewer" {
-		t.Errorf("Second person should be user2 with role reviewer")
-	}
-	if persons[2].UserID != "user3" || persons[2].Role != "" {
-		t.Errorf("Third person should be user3 with empty role")
-	}
-}
-
-func TestSanitizeFilename(t *testing.T) {
-	tests := []struct {
-		input    string
-		expected string
-	}{
-		{"simple", "simple"},
-		{"group/name", "group_name"},
-		{"project:group", "project_group"},
-		{"group with spaces", "group_with_spaces"},
-		{"group/name:space", "group_name_space"},
-		{"", ""},
-		{"multiple///slashes", "multiple___slashes"},
-	}
-
-	for _, tc := range tests {
-		result := sanitizeFilename(tc.input)
-		if result != tc.expected {
-			t.Errorf("sanitizeFilename(%q) = %q, expected %q", tc.input, result, tc.expected)
-		}
-	}
-}
--- a/workflow-pr/README.md
+++ b/workflow-pr/README.md
@@ -54,7 +54,6 @@ This is the ProjectGit config file. For runtime config file, see bottom.
 | *GitProjectName* | Repository and branch where the ProjectGit lives. | no | string | **Format**: `org/project_repo#branch` | By default assumes `_ObsPrj` with default branch in the *Organization* |
 | *ManualMergeOnly* | Merges are permitted only upon receiving a "merge ok" comment from designated maintainers in the PkgGit PR. | no | bool | true, false | false |
 | *ManualMergeProject* | Merges are permitted only upon receiving a "merge ok" comment in the ProjectGit PR from project maintainers. | no | bool | true, false | false |
-| *MergeMode* | Type of package merge accepted. See below for details. | no | string | ff-only, replace, devel | ff-only |
 | *ReviewRequired* | If submitter is a maintainer, require review from another maintainer if available. | no | bool | true, false | false |
 | *NoProjectGitPR* | Do not create PrjGit PR, but still perform other tasks. | no | bool | true, false | false |
 | *Reviewers* | PrjGit reviewers. Additional review requests are triggered for associated PkgGit PRs. PrjGit PR is merged only when all reviews are complete. | no | array of strings | | `[]` |
@@ -118,6 +117,8 @@ The following labels are used, when defined in Repo/Org.
 | Label Config Entry | Default label  | Description
 |--------------------|----------------|----------------------------------------
 | StagingAuto        | staging/Auto   | Assigned to Project Git PRs when first staged
+| ReviewPending      | review/Pending | Assigned to Project Git PR when package reviews are still pending
+| ReviewDone         | review/Done    | Assigned to Project Git PR when reviews are complete on all package PRs


 Maintainership