common: fix three bugs in the timeline cache

1. Double-check locking (TOCTOU)
   GetTimeline dropped the read lock, then acquired the write lock to
   fetch fresh data, but never re-checked the cache after the write lock
   was acquired.  A second goroutine could pass the stale-check under the
   read lock at the same time, resulting in both goroutines fetching the
   full timeline independently and overwriting each other's result.  Add
   a re-check immediately after acquiring the write lock so only the
   first writer fetches.

2. LastCachedTime used the wrong item
   The incremental fetch used data[0].Updated as the high-water mark for
   the Since filter, but data is sorted in descending Created order so
   data[0] is the most-recently-created entry, whose Updated timestamp
   may not be the latest across all cached items.  Switch to a linear
   scan over all items to find the true maximum Updated value, matching
   the fix in origin/fix/timeline-cache-race.

3. Cache not invalidated after RequestReviews / UnrequestReview
   After the bot called RequestReviews or UnrequestReview, the timeline
   cache for that PR was left hot for up to 5 seconds.  Any subsequent
   GetTimeline call within that window returned stale data that did not
   include the just-created review_requested / review_request_removed
   entry, causing FindMissingAndExtraReviewers to make decisions based on
   outdated reviewer state.  Call ResetTimelineCache on success in both
   methods so the next read always sees the mutation.

common/gitea_utils.go

@@ -768,6 +768,10 @@ func (gitea *GiteaTransport) RequestReviews(pr *models.PullRequest, reviewers ..
 		return nil, fmt.Errorf("Cannot create pull request reviews: %w", err)
 	}
 
+	// Invalidate the timeline cache so the next GetTimeline call reflects
+	// the newly created review_requested entry.
+	gitea.ResetTimelineCache(pr.Base.Repo.Owner.UserName, pr.Base.Repo.Name, pr.Index)
+
 	return review.GetPayload(), nil
 }
 
@@ -776,6 +780,13 @@ func (gitea *GiteaTransport) UnrequestReview(org, repo string, id int64, reviwer
 		repository.NewRepoDeletePullReviewRequestsParams().WithOwner(org).WithRepo(repo).WithIndex(id).WithBody(&models.PullReviewRequestOptions{
 			Reviewers: reviwers,
 		}), gitea.transport.DefaultAuthentication)
+
+	if err == nil {
+		// Invalidate the timeline cache so the next GetTimeline call reflects
+		// the newly created review_request_removed entry.
+		gitea.ResetTimelineCache(org, repo, id)
+	}
+
 	return err
 }
 
@@ -861,24 +872,31 @@ func (gitea *GiteaTransport) GetTimeline(org, repo string, idx int64) ([]*models
 	prID := fmt.Sprintf("%s/%s!%d", org, repo, idx)
 	giteaTimelineCacheMutex.RLock()
 	TimelineCache, IsCached := giteaTimelineCache[prID]
-	var LastCachedTime strfmt.DateTime
-	if IsCached {
-		l := len(TimelineCache.data)
-		if l > 0 {
-			LastCachedTime = TimelineCache.data[0].Updated
-		}
-
-		// cache data for 5 seconds
-		if TimelineCache.lastCheck.Add(time.Second*5).Compare(time.Now()) > 0 {
-			giteaTimelineCacheMutex.RUnlock()
-			return TimelineCache.data, nil
-		}
+	if IsCached && TimelineCache.lastCheck.Add(time.Second*5).Compare(time.Now()) > 0 {
+		giteaTimelineCacheMutex.RUnlock()
+		return TimelineCache.data, nil
 	}
 	giteaTimelineCacheMutex.RUnlock()
 
 	giteaTimelineCacheMutex.Lock()
 	defer giteaTimelineCacheMutex.Unlock()
 
+	// Re-read after acquiring the write lock: another goroutine may have
+	// already refreshed the cache while we were waiting.
+	TimelineCache, IsCached = giteaTimelineCache[prID]
+	if IsCached && TimelineCache.lastCheck.Add(time.Second*5).Compare(time.Now()) > 0 {
+		return TimelineCache.data, nil
+	}
+
+	// Find the highest Updated timestamp across all cached items so the
+	// incremental fetch picks up both new entries and modified ones.
+	var LastCachedTime strfmt.DateTime
+	for _, d := range TimelineCache.data {
+		if time.Time(d.Updated).Compare(time.Time(LastCachedTime)) > 0 {
+			LastCachedTime = d.Updated
+		}
+	}
+
 	for resCount > 0 {
 		opts := issue.NewIssueGetCommentsAndTimelineParams().WithOwner(org).WithRepo(repo).WithIndex(idx).WithPage(&page)
 		if !LastCachedTime.IsZero() {

integration/tests/lib/common_test_utils.py

@@ -65,12 +65,6 @@ class GiteaAPIClient:
                 return None
             raise
 
-    def get_submodule_sha(self, owner: str, repo: str, submodule_path: str, ref: str = "main"):
-        info = self.get_file_info(owner, repo, submodule_path, branch=ref)
-        if info and info.get("type") == "submodule":
-            return info.get("sha")
-        return None
-
     def create_user(self, username, password, email):
         vprint(f"--- Creating user: {username} ---")
         data = {
@@ -531,14 +525,6 @@ index 00000000..{pkg_b_sha}
 
         return review
 
-    def request_reviewers(self, repo_full_name: str, pr_number: int, reviewers: list):
-        owner, repo = repo_full_name.split("/")
-        url = f"repos/{owner}/{repo}/pulls/{pr_number}/requested_reviewers"
-        data = {"reviewers": reviewers}
-        vprint(f"--- Requesting reviewers for {repo_full_name} PR #{pr_number}: {reviewers} ---")
-        response, duration = self._request("POST", url, json=data)
-        return response.json()
-
     def list_reviews(self, repo_full_name: str, pr_number: int):
         owner, repo = repo_full_name.split("/")
         url = f"repos/{owner}/{repo}/pulls/{pr_number}/reviews"

integration/tests/staging_br_test.py

@@ -1,521 +0,0 @@
-import pytest
-import time
-import re
-from tests.lib.common_test_utils import GiteaAPIClient, vprint
-
-# Shared methods
-
-def wait_for_staging_bot_reviewer(gitea_env, prj_pr_number, timeout=30):
-    """Wait for staging-bot to be added as a reviewer."""
-    for _ in range(timeout):
-        time.sleep(1)
-        pr_details = gitea_env.get_pr_details("myproducts/mySLFO", prj_pr_number)
-        if any(
-            r.get("login") == "autogits_obs_staging_bot"
-            for r in pr_details.get("requested_reviewers", [])
-        ):
-            return True
-    return False
-
-def wait_for_comment(gitea_env, repo_full_name, pr_number, text_to_find, subtext=None, timeout=60):
-    """Wait for a specific comment in the PR timeline."""
-    vprint(f"Waiting for comment '{text_to_find}'" + (f" with subtext '{subtext}'" if subtext else "") + f" in {repo_full_name} PR #{pr_number}...")
-    for _ in range(timeout):
-        time.sleep(1)
-        try:
-            events = gitea_env.get_timeline_events(repo_full_name, pr_number)
-        except Exception:
-            continue
-        for event in events:
-            body = event.get("body", "")
-            if not body:
-                continue
-            if subtext:
-                if text_to_find in body and subtext in body:
-                    return True
-            else:
-                if text_to_find == body.strip():
-                    return True
-    return False
-
-def setup_obs_mock(httpserver, project_name, build_result_handler):
-    """Setup OBS mock handlers."""
-    def general_project_meta_handler(request):
-        project = request.path.split("/")[2]
-        return f'<project name="{project}"><scmsync>http://gitea-test:3000/myproducts/mySLFO.git</scmsync></project>'
-
-    httpserver.clear()
-    httpserver.expect_request(re.compile(r"/source/[^/]+/_meta$"), method="GET").respond_with_handler(general_project_meta_handler)
-    httpserver.expect_request(re.compile(f"/build/{project_name}/_result"), method="GET").respond_with_handler(build_result_handler)
-    httpserver.expect_request(re.compile(r"/source/[^/]+/_meta$"), method="PUT").respond_with_data("OK")
-    httpserver.expect_request(re.compile(r"/source/[^/]+$"), method="DELETE").respond_with_data("OK")
-
-def create_build_result_xml(project_name, repo_arch_package_status):
-    """Create the XML response for OBS build results."""
-    results_xml = ""
-    # Group statuses by (repo, arch)
-    grouped = {}
-    for (repo, arch, package), (repo_code, pkg_code) in repo_arch_package_status.items():
-        if (repo, arch) not in grouped:
-            grouped[(repo, arch)] = {"repo_code": repo_code, "packages": []}
-        grouped[(repo, arch)]["packages"].append((package, pkg_code))
-    
-    for (repo, arch), data in grouped.items():
-        repo_code = data["repo_code"]
-        pkg_statuses = "".join([f'\n    <status package="{p}" code="{c}"/>' for p, c in data["packages"]])
-        results_xml += f"""
-  <result project="{project_name}" repository="{repo}" arch="{arch}" code="{repo_code}" state="{repo_code}">{pkg_statuses}
-  </result>"""
-    return f'<resultlist state="mock">{results_xml}\n</resultlist>'
-
-def create_submodule_diff(submodule_name, old_sha, new_sha):
-    """Create a git diff string for updating a submodule commit ID."""
-    return f"""diff --git a/{submodule_name} b/{submodule_name}
-index {old_sha[:7]}..{new_sha[:7]} 160000
---- a/{submodule_name}
-+++ b/{submodule_name}
-@@ -1 +1 @@
--Subproject commit {old_sha}
-+Subproject commit {new_sha}
-"""
-
-@pytest.mark.t001
-def test_001_build_on_all_archs_success(staging_main_env, httpserver):
-    gitea_env, test_full_repo_name, merge_branch_name = staging_main_env
-    
-    # 1. Create a package PR.
-    diff = "diff --git a/test_br.txt b/test_br.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr = gitea_env.create_gitea_pr("mypool/pkgA", diff, "Test BR PR", False, base_branch=merge_branch_name)
-    pkg_pr_number = pr["number"]
-
-    # 2. Wait for the workflow-pr bot to create the related project PR.
-    prj_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", pkg_pr_number)
-    assert prj_pr_number is not None, "Workflow bot did not create a project PR."
-
-    # 3. Wait for staging-bot to be added as a reviewer.
-    assert wait_for_staging_bot_reviewer(gitea_env, prj_pr_number), "Staging bot was not added as a reviewer."
-
-    # 4. Mock the OBS result list.
-    project_name = f"openSUSE:Leap:16.0:PullRequest:{prj_pr_number}"
-    repos = ["repo1", "repo2"]
-    archs = ["x86_64", "aarch64"]
-    repo_arch_package_status = {}
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgA")] = ("building", "building")
-
-    def build_result_handler(request):
-        return create_build_result_xml(project_name, repo_arch_package_status)
-
-    setup_obs_mock(httpserver, project_name, build_result_handler)
-
-    # Expected Result 1: "Build is started in..."
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build is started in", project_name), "Staging bot did not post 'Build is started' comment."
-
-    # 6. Transition all to success.
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgA")] = ("published", "succeeded")
-
-    # Expected Result 2: "Build successful" on project PR.
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build successful"), "Staging bot did not post 'Build successful' comment on project PR."
-
-    # Expected Result 3: "Build successful..." on package PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgA", pkg_pr_number, "Build successful, for more information go in", project_name), "Staging bot did not post 'Build successful' comment on package PR."
-
-@pytest.mark.t002
-def test_002_build_on_all_archs_mix(staging_main_env, httpserver):
-    gitea_env, test_full_repo_name, merge_branch_name = staging_main_env
-    
-    # 1. Create a package PR.
-    diff = "diff --git a/test_br_mix.txt b/test_br_mix.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr = gitea_env.create_gitea_pr("mypool/pkgA", diff, "Test BR Mix PR", False, base_branch=merge_branch_name)
-    pkg_pr_number = pr["number"]
-
-    # 2. Wait for the workflow-pr bot to create the related project PR.
-    prj_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", pkg_pr_number)
-    assert prj_pr_number is not None, "Workflow bot did not create a project PR."
-
-    # 3. Wait for staging-bot to be added as a reviewer.
-    assert wait_for_staging_bot_reviewer(gitea_env, prj_pr_number), "Staging bot was not added as a reviewer."
-
-    # 4. Mock the OBS result list with 2 repositories and 2 architectures.
-    project_name = f"openSUSE:Leap:16.0:PullRequest:{prj_pr_number}"
-    repos = ["repo1", "repo2"]
-    archs = ["x86_64", "aarch64"]
-    repo_arch_package_status = {}
-    
-    # 5. Set all repository, architecture, and package statuses to "in progress" (e.g., code="building").
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgA")] = ("building", "building")
-
-    def build_result_handler(request):
-        return create_build_result_xml(project_name, repo_arch_package_status)
-
-    setup_obs_mock(httpserver, project_name, build_result_handler)
-
-    # Expected Result 1: "Build is started in..."
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build is started in", project_name), "Staging bot did not post 'Build is started' comment."
-
-    # 6. Transition the first repository (all architectures) to "finished" mode and set its package statuses to "success" (e.g., code="succeeded").
-    for a in archs:
-        repo_arch_package_status[("repo1", a, "pkgA")] = ("published", "succeeded")
-
-    # 7. Transition the second repository (all architectures) to "finished" mode and set its package statuses to "failed" (e.g., code="failed").
-    for a in archs:
-        repo_arch_package_status[("repo2", a, "pkgA")] = ("finished", "failed")
-
-    # Expected Result 2: "Build failed" on project PR.
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build failed"), "Staging bot did not post 'Build failed' comment on project PR."
-
-    # Expected Result 3: "Build failed, for more information go in..." on package PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgA", pkg_pr_number, "Build failed, for more information go in", project_name), "Staging bot did not post 'Build failed' comment on package PR."
-
-@pytest.mark.t003
-def test_003_build_on_some_archs_failed(staging_main_env, httpserver):
-    gitea_env, test_full_repo_name, merge_branch_name = staging_main_env
-    
-    # 1. Create a package PR.
-    diff = "diff --git a/test_br_some_fail.txt b/test_br_some_fail.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr = gitea_env.create_gitea_pr("mypool/pkgA", diff, "Test BR Some Fail PR", False, base_branch=merge_branch_name)
-    pkg_pr_number = pr["number"]
-
-    # 2. Wait for the workflow-pr bot to create the related project PR.
-    prj_pr_number = gitea_env.wait_for_project_pr("mypool/pkgA", pkg_pr_number)
-    assert prj_pr_number is not None, "Workflow bot did not create a project PR."
-
-    # 3. Wait for staging-bot to be added as a reviewer.
-    assert wait_for_staging_bot_reviewer(gitea_env, prj_pr_number), "Staging bot was not added as a reviewer."
-
-    # 4. Mock the OBS result list with 2 repositories and 2 architectures.
-    project_name = f"openSUSE:Leap:16.0:PullRequest:{prj_pr_number}"
-    repos = ["repo1", "repo2"]
-    archs = ["x86_64", "aarch64"]
-    repo_arch_package_status = {}
-    
-    # 5. Set all repository, architecture, and package statuses to "in progress" (e.g., code="building").
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgA")] = ("building", "building")
-
-    def build_result_handler(request):
-        return create_build_result_xml(project_name, repo_arch_package_status)
-
-    setup_obs_mock(httpserver, project_name, build_result_handler)
-
-    # Expected Result 1: "Build is started in..."
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build is started in", project_name), "Staging bot did not post 'Build is started' comment."
-
-    # 6. Transition the first repository (all architectures) to "finished" mode and set its package statuses to "success" (e.g., code="succeeded").
-    for a in archs:
-        repo_arch_package_status[("repo1", a, "pkgA")] = ("published", "succeeded")
-
-    # 7. Transition the second repository to "finished" mode, setting the first architecture to "failed" (code="failed") and the second architecture to "success" (code="succeeded").
-    repo_arch_package_status[("repo2", archs[0], "pkgA")] = ("finished", "failed")
-    repo_arch_package_status[("repo2", archs[1], "pkgA")] = ("finished", "succeeded")
-
-    # Expected Result 2: "Build failed" on project PR.
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build failed"), "Staging bot did not post 'Build failed' comment on project PR."
-
-    # Expected Result 3: "Build failed, for more information go in..." on package PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgA", pkg_pr_number, "Build failed, for more information go in", project_name), "Staging bot did not post 'Build failed' comment on package PR."
-
-@pytest.mark.t004
-def test_004_build_multiple_packages_success(staging_main_env, httpserver):
-    gitea_env, test_full_repo_name, merge_branch_name = staging_main_env
-    
-    # 1. Create package PRs.
-    diff = "diff --git a/test_br_multi_pkgA.txt b/test_br_multi_pkgA.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr1 = gitea_env.create_gitea_pr("mypool/pkgA", diff, "Test BR Multi PR A", False, base_branch=merge_branch_name)
-    pkg_pr_number1 = pr1["number"]
-    pkg_head_sha1 = pr1["head"]["sha"]
-
-    diff2 = "diff --git a/test_br_multi_pkgB.txt b/test_br_multi_pkgB.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr2 = gitea_env.create_gitea_pr("mypool/pkgB", diff2, "Test BR Multi PR B", False, base_branch=merge_branch_name)
-    pkg_pr_number2 = pr2["number"]
-    pkg_head_sha2 = pr2["head"]["sha"]
-
-    # 2. Create a project PR mentioning both packages in description and UPDATING SUBMODULES.
-    old_sha1 = gitea_env.get_submodule_sha("myproducts", "mySLFO", "pkgA", ref=merge_branch_name)
-    old_sha2 = gitea_env.get_submodule_sha("myproducts", "mySLFO", "pkgB", ref=merge_branch_name)
-    
-    prj_diff = create_submodule_diff("pkgA", old_sha1, pkg_head_sha1)
-    prj_diff += create_submodule_diff("pkgB", old_sha2, pkg_head_sha2)
-
-    body = f"PR: mypool/pkgA!{pkg_pr_number1}\nPR: mypool/pkgB!{pkg_pr_number2}"
-    prj_pr = gitea_env.create_gitea_pr("myproducts/mySLFO", prj_diff, "Test Project PR Multi", False, base_branch=merge_branch_name, body=body)
-    prj_pr_number = prj_pr["number"]
-
-    # 3. Add staging_bot as a reviewer.
-    gitea_env.request_reviewers("myproducts/mySLFO", prj_pr_number, ["autogits_obs_staging_bot"])
-
-    # Wait for staging-bot to be added (verification)
-    assert wait_for_staging_bot_reviewer(gitea_env, prj_pr_number), "Staging bot was not added as a reviewer."
-
-    # 4. Mock the OBS result list with 2 repositories and 2 architectures for each package.
-    project_name = f"openSUSE:Leap:16.0:PullRequest:{prj_pr_number}"
-    repos = ["repo1", "repo2"]
-    archs = ["x86_64", "aarch64"]
-    packages = ["pkgA", "pkgB"]
-    repo_arch_package_status = {}
-    
-    # 5. Set all repository, architecture, and package statuses to "in progress" (e.g., code="building").
-    for r in repos:
-        for a in archs:
-            for p in packages:
-                repo_arch_package_status[(r, a, p)] = ("building", "building")
-
-    def build_result_handler(request):
-        return create_build_result_xml(project_name, repo_arch_package_status)
-
-    setup_obs_mock(httpserver, project_name, build_result_handler)
-
-    # Expected Result 1: "Build is started in..."
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build is started in", project_name), "Staging bot did not post 'Build is started' comment."
-
-    # 6. Transition all repositories and architectures for the first package to "finished" mode and set its package statuses to "success" (e.g., code="succeeded").
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgA")] = ("published", "succeeded")
-
-    # Verify "Build successful" is NOT yet posted
-    time.sleep(5)
-    events = gitea_env.get_timeline_events("myproducts/mySLFO", prj_pr_number)
-    assert not any(e.get("body") == "Build successful" for e in events), "Build successful posted prematurely."
-
-    # 7. Transition all repositories and architectures for the second package to "finished" mode and set its package statuses to "success" (e.g., code="succeeded").
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgB")] = ("published", "succeeded")
-
-    # Expected Result 2: "Build successful" on project PR.
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build successful"), "Staging bot did not post 'Build successful' comment on project PR."
-
-    # Expected Result 3: "Build successful..." on each package PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgA", pkg_pr_number1, "Build successful, for more information go in", project_name), "Staging bot did not post 'Build successful' comment on package PR A."
-    assert wait_for_comment(gitea_env, "mypool/pkgB", pkg_pr_number2, "Build successful, for more information go in", project_name), "Staging bot did not post 'Build successful' comment on package PR B."
-
-@pytest.mark.t005
-def test_005_build_multiple_packages_mix(staging_main_env, httpserver):
-    gitea_env, test_full_repo_name, merge_branch_name = staging_main_env
-    
-    # 1. Create package PRs.
-    diff = "diff --git a/test_br_mix_multi_pkgA.txt b/test_br_mix_multi_pkgA.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr1 = gitea_env.create_gitea_pr("mypool/pkgA", diff, "Test BR Mix Multi PR A", False, base_branch=merge_branch_name)
-    pkg_pr_number1 = pr1["number"]
-    pkg_head_sha1 = pr1["head"]["sha"]
-
-    diff2 = "diff --git a/test_br_mix_multi_pkgB.txt b/test_br_mix_multi_pkgB.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr2 = gitea_env.create_gitea_pr("mypool/pkgB", diff2, "Test BR Mix Multi PR B", False, base_branch=merge_branch_name)
-    pkg_pr_number2 = pr2["number"]
-    pkg_head_sha2 = pr2["head"]["sha"]
-
-    # 2. Create a project PR mentioning both packages in description and UPDATING SUBMODULES.
-    old_sha1 = gitea_env.get_submodule_sha("myproducts", "mySLFO", "pkgA", ref=merge_branch_name)
-    old_sha2 = gitea_env.get_submodule_sha("myproducts", "mySLFO", "pkgB", ref=merge_branch_name)
-    
-    prj_diff = create_submodule_diff("pkgA", old_sha1, pkg_head_sha1)
-    prj_diff += create_submodule_diff("pkgB", old_sha2, pkg_head_sha2)
-
-    body = f"PR: mypool/pkgA!{pkg_pr_number1}\nPR: mypool/pkgB!{pkg_pr_number2}"
-    prj_pr = gitea_env.create_gitea_pr("myproducts/mySLFO", prj_diff, "Test Project PR Multi Mix", False, base_branch=merge_branch_name, body=body)
-    prj_pr_number = prj_pr["number"]
-
-    # 3. Add staging_bot as a reviewer.
-    gitea_env.request_reviewers("myproducts/mySLFO", prj_pr_number, ["autogits_obs_staging_bot"])
-
-    # Wait for staging-bot to be added (verification)
-    assert wait_for_staging_bot_reviewer(gitea_env, prj_pr_number), "Staging bot was not added as a reviewer."
-
-    # 4. Mock the OBS result list with 2 repositories and 2 architectures for each package.
-    project_name = f"openSUSE:Leap:16.0:PullRequest:{prj_pr_number}"
-    repos = ["repo1", "repo2"]
-    archs = ["x86_64", "aarch64"]
-    packages = ["pkgA", "pkgB"]
-    repo_arch_package_status = {}
-    
-    # 5. Set all repository, architecture, and package statuses to "in progress" (e.g., code="building").
-    for r in repos:
-        for a in archs:
-            for p in packages:
-                repo_arch_package_status[(r, a, p)] = ("building", "building")
-
-    def build_result_handler(request):
-        return create_build_result_xml(project_name, repo_arch_package_status)
-
-    setup_obs_mock(httpserver, project_name, build_result_handler)
-
-    # Expected Result 1: "Build is started in..."
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build is started in", project_name), "Staging bot did not post 'Build is started' comment."
-
-    # 6. Transition the first package (all repositories and architectures) to "finished" mode and set its package statuses to "success" (e.g., code="succeeded").
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgA")] = ("published", "succeeded")
-
-    # 7. Transition the second package (all repositories and architectures) to "finished" mode and set its package statuses to "failed" (e.g., code="failed").
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgB")] = ("finished", "failed")
-
-    # Expected Result 2: "Build failed" on project PR.
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build failed"), "Staging bot did not post 'Build failed' comment on project PR."
-
-    # Expected Result 3: "Build successful..." on the successful package's PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgA", pkg_pr_number1, "Build successful, for more information go in", project_name), "Staging bot did not post 'Build successful' comment on package PR A."
-
-    # Expected Result 4: "Build failed..." on the failed package's PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgB", pkg_pr_number2, "Build failed, for more information go in", project_name), "Staging bot did not post 'Build failed' comment on package PR B."
-
-@pytest.mark.t006
-def test_006_build_multiple_packages_partial_fail(staging_main_env, httpserver):
-    gitea_env, test_full_repo_name, merge_branch_name = staging_main_env
-    
-    # 1. Create package PRs.
-    diff = "diff --git a/test_br_partial_multi_pkgA.txt b/test_br_partial_multi_pkgA.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr1 = gitea_env.create_gitea_pr("mypool/pkgA", diff, "Test BR Partial Multi PR A", False, base_branch=merge_branch_name)
-    pkg_pr_number1 = pr1["number"]
-    pkg_head_sha1 = pr1["head"]["sha"]
-
-    diff2 = "diff --git a/test_br_partial_multi_pkgB.txt b/test_br_partial_multi_pkgB.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr2 = gitea_env.create_gitea_pr("mypool/pkgB", diff2, "Test BR Partial Multi PR B", False, base_branch=merge_branch_name)
-    pkg_pr_number2 = pr2["number"]
-    pkg_head_sha2 = pr2["head"]["sha"]
-
-    # 2. Create a project PR mentioning both packages in description and UPDATING SUBMODULES.
-    old_sha1 = gitea_env.get_submodule_sha("myproducts", "mySLFO", "pkgA", ref=merge_branch_name)
-    old_sha2 = gitea_env.get_submodule_sha("myproducts", "mySLFO", "pkgB", ref=merge_branch_name)
-    
-    prj_diff = create_submodule_diff("pkgA", old_sha1, pkg_head_sha1)
-    prj_diff += create_submodule_diff("pkgB", old_sha2, pkg_head_sha2)
-
-    body = f"PR: mypool/pkgA!{pkg_pr_number1}\nPR: mypool/pkgB!{pkg_pr_number2}"
-    prj_pr = gitea_env.create_gitea_pr("myproducts/mySLFO", prj_diff, "Test Project PR Multi Partial", False, base_branch=merge_branch_name, body=body)
-    prj_pr_number = prj_pr["number"]
-
-    # 3. Add staging_bot as a reviewer.
-    gitea_env.request_reviewers("myproducts/mySLFO", prj_pr_number, ["autogits_obs_staging_bot"])
-
-    # Wait for staging-bot to be added (verification)
-    assert wait_for_staging_bot_reviewer(gitea_env, prj_pr_number), "Staging bot was not added as a reviewer."
-
-    # 4. Mock the OBS result list with 2 repositories and 2 architectures for each package.
-    project_name = f"openSUSE:Leap:16.0:PullRequest:{prj_pr_number}"
-    repos = ["repo1", "repo2"]
-    archs = ["x86_64", "aarch64"]
-    packages = ["pkgA", "pkgB"]
-    repo_arch_package_status = {}
-    
-    # 5. Set all repository, architecture, and package statuses to "in progress" (e.g., code="building").
-    for r in repos:
-        for a in archs:
-            for p in packages:
-                repo_arch_package_status[(r, a, p)] = ("building", "building")
-
-    def build_result_handler(request):
-        return create_build_result_xml(project_name, repo_arch_package_status)
-
-    setup_obs_mock(httpserver, project_name, build_result_handler)
-
-    # Expected Result 1: "Build is started in..."
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build is started in", project_name), "Staging bot did not post 'Build is started' comment."
-
-    # 6. Transition the first package (all repositories and architectures) to "finished" mode and set its package statuses to "success" (e.g., code="succeeded").
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgA")] = ("published", "succeeded")
-
-    # 7. Transition the second package:
-    # Repository 1 (all architectures) to "finished" mode with "success" (e.g., code="succeeded").
-    for a in archs:
-        repo_arch_package_status[("repo1", a, "pkgB")] = ("published", "succeeded")
-    
-    # Repository 2 (all architectures) to "finished" mode with "failed" (e.g., code="failed").
-    for a in archs:
-        repo_arch_package_status[("repo2", a, "pkgB")] = ("finished", "failed")
-
-    # Expected Result 2: "Build failed" on project PR.
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build failed"), "Staging bot did not post 'Build failed' comment on project PR."
-
-    # Expected Result 3: "Build successful..." on the first (successful) package's PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgA", pkg_pr_number1, "Build successful, for more information go in", project_name), "Staging bot did not post 'Build successful' comment on package PR A."
-
-    # Expected Result 4: "Build failed..." on the second (failed) package's PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgB", pkg_pr_number2, "Build failed, for more information go in", project_name), "Staging bot did not post 'Build failed' comment on package PR B."
-
-@pytest.mark.t007
-def test_007_build_multiple_packages_arch_fail(staging_main_env, httpserver):
-    gitea_env, test_full_repo_name, merge_branch_name = staging_main_env
-    
-    # 1. Create package PRs.
-    diff = "diff --git a/test_br_arch_multi_pkgA.txt b/test_br_arch_multi_pkgA.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr1 = gitea_env.create_gitea_pr("mypool/pkgA", diff, "Test BR Arch Multi PR A", False, base_branch=merge_branch_name)
-    pkg_pr_number1 = pr1["number"]
-    pkg_head_sha1 = pr1["head"]["sha"]
-
-    diff2 = "diff --git a/test_br_arch_multi_pkgB.txt b/test_br_arch_multi_pkgB.txt\nnew file mode 100644\nindex 00000000..473a0f4c\n"
-    pr2 = gitea_env.create_gitea_pr("mypool/pkgB", diff2, "Test BR Arch Multi PR B", False, base_branch=merge_branch_name)
-    pkg_pr_number2 = pr2["number"]
-    pkg_head_sha2 = pr2["head"]["sha"]
-
-    # 2. Create a project PR mentioning both packages in description and UPDATING SUBMODULES.
-    old_sha1 = gitea_env.get_submodule_sha("myproducts", "mySLFO", "pkgA", ref=merge_branch_name)
-    old_sha2 = gitea_env.get_submodule_sha("myproducts", "mySLFO", "pkgB", ref=merge_branch_name)
-    
-    prj_diff = create_submodule_diff("pkgA", old_sha1, pkg_head_sha1)
-    prj_diff += create_submodule_diff("pkgB", old_sha2, pkg_head_sha2)
-
-    body = f"PR: mypool/pkgA!{pkg_pr_number1}\nPR: mypool/pkgB!{pkg_pr_number2}"
-    prj_pr = gitea_env.create_gitea_pr("myproducts/mySLFO", prj_diff, "Test Project PR Multi Arch Fail", False, base_branch=merge_branch_name, body=body)
-    prj_pr_number = prj_pr["number"]
-
-    # 3. Add staging_bot as a reviewer.
-    gitea_env.request_reviewers("myproducts/mySLFO", prj_pr_number, ["autogits_obs_staging_bot"])
-
-    # Wait for staging-bot to be added (verification)
-    assert wait_for_staging_bot_reviewer(gitea_env, prj_pr_number), "Staging bot was not added as a reviewer."
-
-    # 4. Mock the OBS result list with 2 repositories and 2 architectures for each package.
-    project_name = f"openSUSE:Leap:16.0:PullRequest:{prj_pr_number}"
-    repos = ["repo1", "repo2"]
-    archs = ["x86_64", "aarch64"]
-    packages = ["pkgA", "pkgB"]
-    repo_arch_package_status = {}
-    
-    # 5. Set all repository, architecture, and package statuses to "in progress" (e.g., code="building").
-    for r in repos:
-        for a in archs:
-            for p in packages:
-                repo_arch_package_status[(r, a, p)] = ("building", "building")
-
-    def build_result_handler(request):
-        return create_build_result_xml(project_name, repo_arch_package_status)
-
-    setup_obs_mock(httpserver, project_name, build_result_handler)
-
-    # Expected Result 1: "Build is started in..."
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build is started in", project_name), "Staging bot did not post 'Build is started' comment."
-
-    # 6. Transition the first package (all repositories and architectures) to "finished" mode and set its package statuses to "success" (e.g., code="succeeded").
-    for r in repos:
-        for a in archs:
-            repo_arch_package_status[(r, a, "pkgA")] = ("published", "succeeded")
-
-    # 7. Transition the second package:
-    # Architecture 1 (all repositories) to "finished" mode with "success" (e.g., code="succeeded").
-    for r in repos:
-        repo_arch_package_status[(r, archs[0], "pkgB")] = ("published", "succeeded")
-    
-    # Architecture 2 (all repositories) to "finished" mode with "failed" (e.g., code="failed").
-    for r in repos:
-        repo_arch_package_status[(r, archs[1], "pkgB")] = ("finished", "failed")
-
-    # Expected Result 2: "Build failed" on project PR.
-    assert wait_for_comment(gitea_env, "myproducts/mySLFO", prj_pr_number, "Build failed"), "Staging bot did not post 'Build failed' comment on project PR."
-
-    # Expected Result 3: "Build successful..." on the first (successful) package's PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgA", pkg_pr_number1, "Build successful, for more information go in", project_name), "Staging bot did not post 'Build successful' comment on package PR A."
-
-    # Expected Result 4: "Build failed..." on the second (failed) package's PR.
-    assert wait_for_comment(gitea_env, "mypool/pkgB", pkg_pr_number2, "Build failed, for more information go in", project_name), "Staging bot did not post 'Build failed' comment on package PR B."

workflow-pr/pr_processor.go

@@ -8,6 +8,7 @@ import (
 	"runtime/debug"
 	"slices"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/opentracing/opentracing-go/log"
@@ -628,9 +629,29 @@ func (pr *PRProcessor) Process(req *models.PullRequest) error {
 	return err
 }
 
+// prLocks serialises concurrent processing of the same PR.
+// Both the RabbitMQ event loop and the consistency-checker goroutine call
+// ProcesPullRequest; without this lock they can race on reviewer add/remove.
+// Key format: "org/repo#num"
+var prLocks sync.Map // map[string]chan struct{}
+
+func prLockKey(pr *models.PullRequest) string {
+	return fmt.Sprintf("%s/%s#%d", pr.Base.Repo.Owner.UserName, pr.Base.Repo.Name, pr.Index)
+}
+
+func acquirePRLock(key string) chan struct{} {
+	v, _ := prLocks.LoadOrStore(key, make(chan struct{}, 1))
+	ch := v.(chan struct{})
+	ch <- struct{}{}
+	return ch
+}
+
+func releasePRLock(ch chan struct{}) {
+	<-ch
+}
+
 type RequestProcessor struct {
 	configuredRepos map[string][]*common.AutogitConfig
-	recursive       int
 }
 
 func (w *RequestProcessor) Process(pr *models.PullRequest) error {
@@ -647,6 +668,9 @@ func ProcesPullRequest(pr *models.PullRequest, configs []*common.AutogitConfig)
 		return nil
 	}
 
+	lock := acquirePRLock(prLockKey(pr))
+	defer releasePRLock(lock)
+
 	PRProcessor, err := AllocatePRProcessor(pr, configs)
 	if err != nil {
 		log.Error(err)
@@ -663,17 +687,23 @@ func (w *RequestProcessor) ProcessFunc(request *common.Request) (err error) {
 			common.LogInfo("panic cought --- recovered")
 			common.LogError(string(debug.Stack()))
 		}
-		w.recursive--
 	}()
 
-	w.recursive++
-	if w.recursive > 3 {
-		common.LogError("Recursion limit reached... something is wrong with this PR?")
-		return nil
-	}
-
 	var pr *models.PullRequest
 	if req, ok := request.Data.(*common.PullRequestWebhookEvent); ok {
+		// Skip pull_request_sync events triggered by the bot's own pushes to
+		// prjgit branches.  Those would re-run AssignReviewers immediately
+		// after the bot itself just set them, producing spurious add/remove
+		// cycles.  Human-triggered syncs have a different sender and are still
+		// processed normally.
+		if request.Type == common.RequestType_PRSync && CurrentUser != nil &&
+			req.Sender.Username == CurrentUser.UserName {
+			common.LogDebug("Skipping self-triggered pull_request_sync from", req.Sender.Username,
+				"on", req.Pull_Request.Base.Repo.Owner.Username+"/"+req.Pull_Request.Base.Repo.Name,
+				"#", req.Pull_Request.Number)
+			return nil
+		}
+
 		pr, err = Gitea.GetPullRequest(req.Pull_Request.Base.Repo.Owner.Username, req.Pull_Request.Base.Repo.Name, req.Pull_Request.Number)
 		if err != nil {
 			common.LogError("Cannot find PR for issue:", req.Pull_Request.Base.Repo.Owner.Username, req.Pull_Request.Base.Repo.Name, req.Pull_Request.Number)
@@ -710,8 +740,16 @@ func (w *RequestProcessor) ProcessFunc(request *common.Request) (err error) {
 		common.LogError("*** Cannot find config for org:", pr.Base.Repo.Owner.UserName)
 	}
 	if err = ProcesPullRequest(pr, configs); err == updatePrjGitError_requeue {
-		time.Sleep(time.Second * 5)
-		return w.ProcessFunc(request)
+		// Retry after a delay in a background goroutine so the event loop is
+		// not blocked while we wait.  The per-PR lock inside ProcesPullRequest
+		// ensures no other processing races with the retry.
+		go func() {
+			time.Sleep(time.Second * 5)
+			if err := ProcesPullRequest(pr, configs); err != nil {
+				common.LogError("requeue retry failed:", err)
+			}
+		}()
+		return nil
 	}
 	return err
 }

workflow-pr/pr_processor_test.go

@@ -989,18 +989,6 @@ func TestProcessFunc(t *testing.T) {
 		}
 	})
 
-	t.Run("Recursion limit", func(t *testing.T) {
-		reqProc.recursive = 3
-		err := reqProc.ProcessFunc(&common.Request{})
-		if err != nil {
-			t.Errorf("Expected nil error on recursion limit, got %v", err)
-		}
-		if reqProc.recursive != 3 {
-			t.Errorf("Expected recursive to be 3, got %d", reqProc.recursive)
-		}
-		reqProc.recursive = 0 // Reset
-	})
-
 	t.Run("Invalid data format", func(t *testing.T) {
 		err := reqProc.ProcessFunc(&common.Request{Data: nil})
 		if err == nil || !strings.Contains(err.Error(), "Invalid data format") {