Skip to content

fix(provider-review): apply the collected commit title and message to GitLab merges - #6772

Merged
iscekic merged 2 commits into
mainfrom
kwf/janitor-backend-provider-review-39404455da
Sep 28, 2026
Merged

iscekic merged 2 commits into
mainfrom
kwf/janitor-backend-provider-review-39404455da

Conversation

@iscekic

@iscekic iscekic commented Sep 27, 2026 •

Copy link
Copy Markdown
Collaborator

Fix proof

The GitLab merge write only sends merge_commit_title/merge_commit_message, never squash_commit_title/squash_commit_message, so a GitLab squash merge cannot apply the title/message the mobile sheet col

Asserted value: apps/web/src/lib/provider-review/gitlab-write.ts. Sense check (jev): probability 0.9

The scripts were proven on an earlier base, so only the head ran.

Head 9b967a83ba64

Head log: backend-assert c5a1a5224830 exited 0
$ git diff --unified=0 332f0c033e0781f85b7782c75ee4f2f595a7e2a8 9b967a83ba646103e32d558d719bd4cdd2707fbd -- apps/web/src/lib/provider-review/gitlab-write.ts
diff --git a/apps/web/src/lib/provider-review/gitlab-write.ts b/apps/web/src/lib/provider-review/gitlab-write.ts
--- a/apps/web/src/lib/provider-review/gitlab-write.ts
+++ b/apps/web/src/lib/provider-review/gitlab-write.ts
@@ -461,2 +468,5 @@ export async function mergePullRequest(
-        ...(target.commitTitle ? { merge_commit_title: target.commitTitle } : {}),
-        ...(target.commitMessage ? { merge_commit_message: target.commitMessage } : {}),
+        ...(fullCommitMessage
+          ? target.squash
+            ? { squash_commit_message: fullCommitMessage }
+            : { merge_commit_message: fullCommitMessage }
+          : {}),

Changelog for users

  • A GitLab squash merge now uses the commit title and message the sheet collected instead of the merge request title.
  • A GitLab merge with squash off now sends the typed title and message as one commit message, title first.

Changelog for maintainers

  • GitLab's merge endpoint has no title field, so the title is now the leading line of the whole commit message.
  • A squash merge sends squash_commit_message; a non-squash merge sends merge_commit_message, and both are omitted when empty.
  • Review the squash branch in the merge write first; a project that fast-forwards or rebases still ignores the typed message.
  • The title and message are joined with a blank line, so a message with its own leading headings keeps its shape.

E2E proof

The GitLab merge write only sends merge_commit_title/merge_commit_message, never squash_commit_title/squash_commit_message, so a GitLab squash merge cannot apply the title/message the mobile sheet collected (the squashed commit keeps the merge request's title; on fast-forward/rebase projects the typed values are not used at all).

Code trace: apps/web/src/lib/provider-review/gitlab-write.ts:426 changed in b5384b7bb5f613d6a5a9bdd40458f3536ac595d9. Sense check (jev): probability 0.91

Changed lines
- * revision the reviewer saw.
+ * revision the reviewer saw. GitLab's merge endpoint accepts only a whole
+ * commit message (`squash_commit_message` when `squash` is true, otherwise
+ * `merge_commit_message`) and has no separate title parameter, so the sheet's
+ * `commitTitle` becomes the message's leading line and `commitMessage` follows
+ * as the body.
+    const fullCommitMessage = [target.commitTitle, target.commitMessage]
+      .filter((part): part is string => Boolean(part))
+      .join('\n\n');
-        ...(target.commitTitle ? { merge_commit_title: target.commitTitle } : {}),
-        ...(target.commitMessage ? { merge_commit_message: target.commitMessage } : {}),
+        ...(fullCommitMessage
+          ? target.squash
+            ? { squash_commit_message: fullCommitMessage }
+            : { merge_commit_message: fullCommitMessage }
+          : {}),
Owner request

Fix 1 janitor finding in backend/provider-review. Fix every one; the proof covers each.

  1. The GitLab merge write only sends merge_commit_title/merge_commit_message, never squash_commit_title/squash_commit_message, so a GitLab squash merge cannot apply the title/message the mobile sheet collected (the squashed commit keeps the merge request's title; on fast-forward/rebase projects the typed values are not used at all).
    Trace: apps/web/src/lib/provider-review/gitlab-write.ts:461: The GitLab merge write only sends merge_commit_title/merge_commit_message, never squash_commit_title/squash_commit_message, so a GitLab squash merge cannot apply the title/message the mobile sheet collected (the squashed commit keeps the merge request's title; on fast-forward/rebase projects the typed values are not used at all). (janitor area performance-reliability).
    Files: apps/web/src/lib/provider-review/gitlab-write.ts.

@iscekic iscekic added the kwf-janitor Admitted to the workflow from a janitor finding label Sep 27, 2026
Comment thread apps/web/src/lib/provider-review/gitlab-write.ts
@kilo-code-bot

kilo-code-bot Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The follow-up commit adds unit tests that pin the exact GitLab merge request body per squash mode, resolving the prior suggestion; the changed code introduces no new issues.

Files Reviewed (1 file)
  • apps/web/src/lib/provider-review/gitlab-write.test.ts

Resolved Since Previous Review

The prior SUGGESTION on apps/web/src/lib/provider-review/gitlab-write.ts:468 (missing coverage for the squash/merge commit-message mapping) is resolved: gitlab-write.test.ts now asserts the full request body for both squash: true (squash_commit_message) and squash: false (merge_commit_message).

Previous Review Summary (commit 3dda8bf)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 3dda8bf)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/web/src/lib/provider-review/gitlab-write.ts 468 New squash/merge commit-message mapping has no test coverage
Files Reviewed (1 file)
  • apps/web/src/lib/provider-review/gitlab-write.ts - 1 issue

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic iscekic self-assigned this Sep 27, 2026
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 27, 2026
@iscekic iscekic added merge-by-human the merge bot routed this PR to a human merge-by-bot and removed human-ready The PR is ready for human review. merge-by-human the merge bot routed this PR to a human labels Sep 27, 2026
@iscekic
iscekic marked this pull request as draft September 28, 2026 08:42
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 08:46
@iscekic
iscekic marked this pull request as draft September 28, 2026 08:54
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:07
@iscekic
iscekic marked this pull request as draft September 28, 2026 09:19
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:22
@iscekic
iscekic marked this pull request as draft September 28, 2026 09:25
@iscekic
iscekic marked this pull request as ready for review September 28, 2026 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kwf-janitor Admitted to the workflow from a janitor finding merge-by-bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants