From 8e4cfd1779ed762e0326c8901169a64082730e21 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Fri, 28 Aug 2026 11:20:50 -0500 Subject: [PATCH] feat: expose description, conflicts, and review state on merge request items Agents triaging an MR from `mr:list` had to fetch it again through `glab api` to learn whether it needs a rebase or is blocked on unresolved review threads. GitLab's list endpoint already returns `description`, `has_conflicts`, `blocking_discussions_resolved`, and `detailed_merge_status`, so `MergeRequestItem` now carries them and the markdown and llm formatters render them. `is_mergeable` stays as-is; `detailed_merge_status` is its non-deprecated replacement. Closes #366 Co-Authored-By: Claude Fable 5 --- .../references/gitlab-mr-contribution.md | 3 ++- .../references/gitlab-mr-contribution.md | 3 ++- src/Api/Result/MergeRequest/MergeRequestItem.php | 12 ++++++++++++ src/Cli/Formatter/LlmFormatter.php | 8 ++++++++ src/Cli/Formatter/MarkdownFormatter.php | 10 ++++++++++ .../MergeRequest/ListMergeRequestsActionTest.php | 8 ++++++++ tests/src/Formatter/LlmFormatterTest.php | 8 ++++++++ tests/src/Formatter/MarkdownFormatterTest.php | 6 ++++++ 8 files changed, 56 insertions(+), 2 deletions(-) diff --git a/skill-data/drupalorg-cli/references/gitlab-mr-contribution.md b/skill-data/drupalorg-cli/references/gitlab-mr-contribution.md index b9b9eb7..ae20632 100644 --- a/skill-data/drupalorg-cli/references/gitlab-mr-contribution.md +++ b/skill-data/drupalorg-cli/references/gitlab-mr-contribution.md @@ -80,7 +80,8 @@ state, never that MRs live elsewhere. To list every MR on a project, pass the project path instead: `mr:list project/drupal`. `--format=llm` output includes IID, title, source branch, state, mergeability, -author, and last-updated timestamp for each MR. +conflicts, whether blocking discussions are resolved, detailed merge status, +author, last-updated timestamp, and description for each MR. ### Review MR content diff --git a/skills/drupalorg-cli/references/gitlab-mr-contribution.md b/skills/drupalorg-cli/references/gitlab-mr-contribution.md index b9b9eb7..ae20632 100644 --- a/skills/drupalorg-cli/references/gitlab-mr-contribution.md +++ b/skills/drupalorg-cli/references/gitlab-mr-contribution.md @@ -80,7 +80,8 @@ state, never that MRs live elsewhere. To list every MR on a project, pass the project path instead: `mr:list project/drupal`. `--format=llm` output includes IID, title, source branch, state, mergeability, -author, and last-updated timestamp for each MR. +conflicts, whether blocking discussions are resolved, detailed merge status, +author, last-updated timestamp, and description for each MR. ### Review MR content diff --git a/src/Api/Result/MergeRequest/MergeRequestItem.php b/src/Api/Result/MergeRequest/MergeRequestItem.php index 13a8a31..e51ad36 100644 --- a/src/Api/Result/MergeRequest/MergeRequestItem.php +++ b/src/Api/Result/MergeRequest/MergeRequestItem.php @@ -14,6 +14,10 @@ public function __construct( public readonly bool $isMergeable, public readonly string $author, public readonly string $updatedAt, + public readonly string $description = '', + public readonly bool $hasConflicts = false, + public readonly bool $blockingDiscussionsResolved = true, + public readonly string $detailedMergeStatus = '', ) { } @@ -29,6 +33,10 @@ public static function fromStdClass(\stdClass $mr): self isMergeable: ($mr->merge_status ?? '') === 'can_be_merged', author: (string) ($mr->author->username ?? ''), updatedAt: (string) ($mr->updated_at ?? ''), + description: (string) ($mr->description ?? ''), + hasConflicts: (bool) ($mr->has_conflicts ?? false), + blockingDiscussionsResolved: (bool) ($mr->blocking_discussions_resolved ?? true), + detailedMergeStatus: (string) ($mr->detailed_merge_status ?? ''), ); } @@ -47,6 +55,10 @@ public function toArray(): array 'is_mergeable' => $this->isMergeable, 'author' => $this->author, 'updated_at' => $this->updatedAt, + 'description' => $this->description, + 'has_conflicts' => $this->hasConflicts, + 'blocking_discussions_resolved' => $this->blockingDiscussionsResolved, + 'detailed_merge_status' => $this->detailedMergeStatus, ]; } } diff --git a/src/Cli/Formatter/LlmFormatter.php b/src/Cli/Formatter/LlmFormatter.php index 287c58b..86e2f45 100644 --- a/src/Cli/Formatter/LlmFormatter.php +++ b/src/Cli/Formatter/LlmFormatter.php @@ -177,6 +177,10 @@ protected function formatMergeRequestList(MergeRequestListResult $result): strin $targetBranch = $this->xmlEscape($mr->targetBranch); $author = $this->xmlEscape($mr->author); $mergeable = $mr->isMergeable ? 'yes' : 'no'; + $hasConflicts = $mr->hasConflicts ? 'yes' : 'no'; + $discussionsResolved = $mr->blockingDiscussionsResolved ? 'yes' : 'no'; + $detailedMergeStatus = $this->xmlEscape($mr->detailedMergeStatus); + $description = $this->xmlEscape($mr->description); $items .= " \n"; $items .= " {$mr->iid}\n"; $items .= " {$title}\n"; @@ -186,9 +190,13 @@ protected function formatMergeRequestList(MergeRequestListResult $result): strin $updatedAt = $this->xmlEscape($mr->updatedAt); $items .= " {$state}\n"; $items .= " {$mergeable}\n"; + $items .= " {$hasConflicts}\n"; + $items .= " {$discussionsResolved}\n"; + $items .= " {$detailedMergeStatus}\n"; $items .= " {$author}\n"; $items .= " " . $this->xmlEscape($mr->webUrl) . "\n"; $items .= " {$updatedAt}\n"; + $items .= " {$description}\n"; $items .= " \n"; } return "\n {$projectPath}\n{$issueFork} \n{$items} \n"; diff --git a/src/Cli/Formatter/MarkdownFormatter.php b/src/Cli/Formatter/MarkdownFormatter.php index 5e766b5..dd54ffd 100644 --- a/src/Cli/Formatter/MarkdownFormatter.php +++ b/src/Cli/Formatter/MarkdownFormatter.php @@ -142,6 +142,16 @@ protected function formatMergeRequestList(MergeRequestListResult $result): strin $lines[] = "- **!{$mr->iid}** [{$mr->state}{$mergeable}] [{$mr->title}]({$mr->webUrl})"; $lines[] = " - Branch: `{$mr->sourceBranch}` → `{$mr->targetBranch}`"; $lines[] = " - Author: {$mr->author} | Updated: {$mr->updatedAt}"; + $conflicts = $mr->hasConflicts ? 'yes' : 'no'; + $discussions = $mr->blockingDiscussionsResolved ? 'resolved' : 'unresolved'; + $mergeStatus = $mr->detailedMergeStatus !== '' ? $mr->detailedMergeStatus : 'unknown'; + $lines[] = " - Conflicts: {$conflicts} | Discussions: {$discussions} | Merge status: {$mergeStatus}"; + if ($mr->description !== '') { + $lines[] = ''; + foreach (explode("\n", $mr->description) as $descriptionLine) { + $lines[] = ' ' . $descriptionLine; + } + } } return implode("\n", $lines); } diff --git a/tests/src/Action/MergeRequest/ListMergeRequestsActionTest.php b/tests/src/Action/MergeRequest/ListMergeRequestsActionTest.php index 0abdb7f..0fa9c9c 100644 --- a/tests/src/Action/MergeRequest/ListMergeRequestsActionTest.php +++ b/tests/src/Action/MergeRequest/ListMergeRequestsActionTest.php @@ -66,6 +66,10 @@ private static function makeMrObject(int $iid = 7, string $state = 'opened'): \s $mr->state = $state; $mr->web_url = 'https://git.drupalcode.org/project/drupal/-/merge_requests/' . $iid; $mr->merge_status = 'can_be_merged'; + $mr->description = 'Closes #3383637'; + $mr->has_conflicts = false; + $mr->blocking_discussions_resolved = true; + $mr->detailed_merge_status = 'mergeable'; $mr->author = $author; $mr->updated_at = '2024-01-15T10:00:00Z'; return $mr; @@ -110,6 +114,10 @@ public function testListsOnlyMergeRequestsFromTheIssueFork(): void self::assertSame('opened', $result->mergeRequests[0]->state); self::assertSame('mglaman', $result->mergeRequests[0]->author); self::assertTrue($result->mergeRequests[0]->isMergeable); + self::assertSame('Closes #3383637', $result->mergeRequests[0]->description); + self::assertFalse($result->mergeRequests[0]->hasConflicts); + self::assertTrue($result->mergeRequests[0]->blockingDiscussionsResolved); + self::assertSame('mergeable', $result->mergeRequests[0]->detailedMergeStatus); self::assertSame('issue/drupal-3383637', $result->jsonSerialize()['issue_fork']); } diff --git a/tests/src/Formatter/LlmFormatterTest.php b/tests/src/Formatter/LlmFormatterTest.php index 737912d..2bbadd8 100644 --- a/tests/src/Formatter/LlmFormatterTest.php +++ b/tests/src/Formatter/LlmFormatterTest.php @@ -254,6 +254,10 @@ public function testMergeRequestListResult(): void isMergeable: true, author: 'mglaman', updatedAt: '2024-01-15T10:00:00Z', + description: 'Closes #3383637 & it', + hasConflicts: true, + blockingDiscussionsResolved: false, + detailedMergeStatus: 'conflict', ); $result = new MergeRequestListResult( @@ -275,6 +279,10 @@ public function testMergeRequestListResult(): void self::assertStringContainsString('mglaman', $output); self::assertStringContainsString('https://git.drupalcode.org/issue/drupal-3383637/-/merge_requests/7', $output); self::assertStringContainsString('2024-01-15T10:00:00Z', $output); + self::assertStringContainsString('yes', $output); + self::assertStringContainsString('no', $output); + self::assertStringContainsString('conflict', $output); + self::assertStringContainsString('Closes #3383637 & <fixes> it', $output); // Raw < must not appear inside tag values. self::assertStringNotContainsString('', $output); } diff --git a/tests/src/Formatter/MarkdownFormatterTest.php b/tests/src/Formatter/MarkdownFormatterTest.php index e66c397..75b91f4 100644 --- a/tests/src/Formatter/MarkdownFormatterTest.php +++ b/tests/src/Formatter/MarkdownFormatterTest.php @@ -221,6 +221,10 @@ public function testMergeRequestListResult(): void isMergeable: true, author: 'mglaman', updatedAt: '2024-01-15T10:00:00Z', + description: "Closes #3383637\n\nAdds the missing null check.", + hasConflicts: true, + blockingDiscussionsResolved: false, + detailedMergeStatus: 'conflict', ); $result = new MergeRequestListResult( @@ -240,6 +244,8 @@ public function testMergeRequestListResult(): void self::assertStringContainsString('`3383637-fix-the-thing` → `11.x`', $output); self::assertStringContainsString('mglaman', $output); self::assertStringContainsString('2024-01-15T10:00:00Z', $output); + self::assertStringContainsString('Conflicts: yes | Discussions: unresolved | Merge status: conflict', $output); + self::assertStringContainsString("\n Closes #3383637\n \n Adds the missing null check.", $output); } public function testMergeRequestStatusResult(): void