Skip to content

Commit 7959199

Browse files
committed
Fix the issue with checklist
Also add a check for the duplicate to not post the checklist on multiple runs
1 parent 1a10192 commit 7959199

File tree

3 files changed

+59
-5
lines changed

3 files changed

+59
-5
lines changed

agents/package_update_steps.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from pydantic import BaseModel, Field
55
from pathlib import Path
66

7-
from common.constants import JiraLabels
7+
from common.constants import JiraLabels, GITLAB_MR_CHECKLIST
88
from common.config import load_rhel_config
99
from common.models import LogOutputSchema
1010

@@ -143,6 +143,7 @@ async def create_merge_request_checklist(state, next_step, dry_run, gateway_tool
143143
await tasks.run_tool(
144144
"create_merge_request_checklist",
145145
merge_request_url=state.merge_request_url,
146+
note_body=GITLAB_MR_CHECKLIST,
146147
available_tools=gateway_tools,
147148
)
148149
logger.info(f"Created checklist for MR {state.merge_request_url}")

mcp_server/gitlab_tools.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,10 +305,32 @@ async def create_merge_request_checklist(
305305
) -> str:
306306
"""
307307
Creates our pre/post merge checklist for our dist-git merge requests.
308+
Checks for existing checklist to avoid duplicates.
308309
"""
309310
try:
310311
mr = await _get_merge_request_from_url(merge_request_url)
311-
# internal note docs: https://docs.gitlab.com/api/notes/#create-new-issue-note
312+
313+
def check_existing_checklist():
314+
notes = mr._raw_pr.notes.list(get_all=True)
315+
316+
checklist_body = note_body.strip()
317+
if not checklist_body:
318+
return False
319+
checklist_identifier = checklist_body.splitlines()[0]
320+
321+
for note in notes:
322+
note_body_text = note.body.strip()
323+
if (checklist_identifier in note_body_text or
324+
note_body_text == checklist_body):
325+
return True
326+
327+
return False
328+
329+
exists = await asyncio.to_thread(check_existing_checklist)
330+
if exists:
331+
return f"Checklist already exists in merge request {merge_request_url}, not adding duplicate"
332+
333+
# internal note docs: https://docs.gitlab.com/api/notes/
312334
await asyncio.to_thread(mr._raw_pr.notes.create, {"body": note_body}, internal=True)
313335
return f"Successfully created checklist for merge request {merge_request_url}"
314336
except Exception as e:

mcp_server/tests/unit/test_gitlab_tools.py

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,9 +321,9 @@ async def test_create_merge_request_checklist():
321321
flexmock(
322322
id=123,
323323
_raw_pr=flexmock(
324-
notes=flexmock().should_receive("create").and_return(
325-
flexmock(id=1),
326-
).mock(),
324+
notes=flexmock()
325+
.should_receive("list").with_args(get_all=True).and_return([]).mock()
326+
.should_receive("create").and_return(flexmock(id=1)).mock(),
327327
),
328328
),
329329
).mock()
@@ -337,6 +337,37 @@ async def test_create_merge_request_checklist():
337337
assert result == f"Successfully created checklist for merge request {merge_request_url}"
338338

339339

340+
@pytest.mark.asyncio
341+
async def test_create_merge_request_checklist_duplicate():
342+
"""Test that duplicate checklists are not created"""
343+
merge_request_url = "https://gitlab.com/redhat/rhel/rpms/bash/-/merge_requests/123"
344+
345+
# Mock an existing note with the checklist identifier
346+
existing_note = flexmock(body="# Jötnar MR Review Checklist\n\nSome checklist content")
347+
348+
flexmock(GitlabService).should_receive("get_project_from_url").with_args(
349+
url=merge_request_url.rsplit("/-/merge_requests/", 1)[0],
350+
).and_return(
351+
flexmock().should_receive("get_pr").and_return(
352+
flexmock(
353+
id=123,
354+
_raw_pr=flexmock(
355+
notes=flexmock()
356+
.should_receive("list").with_args(get_all=True).and_return([existing_note]).mock(),
357+
),
358+
),
359+
).mock()
360+
)
361+
362+
result = await create_merge_request_checklist(
363+
merge_request_url=merge_request_url,
364+
note_body=GITLAB_MR_CHECKLIST,
365+
)
366+
367+
assert "already exists" in result
368+
assert "not adding duplicate" in result
369+
370+
340371
@pytest.mark.asyncio
341372
async def test_retry_pipeline_job():
342373
project_url = "https://gitlab.com/redhat/rhel/rpms/bash"

0 commit comments

Comments
 (0)