Skip to content

feat(pr-skills): add GitLab support to pr-review and pr-comments-address - #81

Open
AlexanderMakarov wants to merge 2 commits into
mainfrom
feature/pr-skills-gitlab-support
Open

feat(pr-skills): add GitLab support to pr-review and pr-comments-address#81
AlexanderMakarov wants to merge 2 commits into
mainfrom
feature/pr-skills-gitlab-support

Conversation

@AlexanderMakarov

@AlexanderMakarov AlexanderMakarov commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

What

Adds GitLab as a second supported review platform for the pr-review and pr-comments-address skills, usable through the glab CLI and through a connected GitLab MCP server.

Why this is additive, not a rewrite

Both skills already split the platform-independent workflow (SKILL.md) from platform commands (references/github.md), keyed by operation namepreflight, fetch-pr-context, create-draft-review, resolve-thread, and so on. Both github.md headers said in as many words that reference files for other platforms could be added and selected by the PR URL host. This PR takes that hook.

Each skill gains a references/gitlab.md implementing exactly the same operation names as its github.md. No workflow step is added, removed, or reordered, and local.md is untouched — local mode never talks to a platform.

Design decisions

Transport: glab first, MCP as fallback. Recipes are glab commands (glab api covers what the porcelain doesn't). A GitLab MCP server is used only when glab is missing or unauthenticated. MCP tool names are discovered, not hardcoded — GitLab MCP servers differ widely in coverage, and hardcoding one server's names would silently break against another. An operation with no matching MCP tool stops the workflow with an explanation rather than being skipped, since half-delivered review work (fixes committed, replies never posted) is worse than not starting.

Draft-first is preserved. GitLab's Draft Notes API maps onto GitHub's pending review better than expected: one draft note per finding, plus a positionless draft note carrying the summary (GitLab draft notes have no summary field of their own), and bulk_publish supplies both the summary and the reviewer state at submit time. Replies to existing discussions ride along as drafts via in_reply_to_discussion_id, so they publish atomically with the review — the same effect github.md gets through GraphQL.

Draft availability is probed before the gate, not discovered at delivery. Draft delivery can be unavailable — an instance predating the endpoint, a token without api scope, or an MCP-only fallback with no draft tool. preflight probes for it and carries the answer to the step 5 results gate, which changes its own options when drafts are unavailable: it says so plainly and offers publish now or write to a file and post nothing. The user is never offered a "draft" that would in fact publish immediately.

Never-destroy, adapted. GitLab needs no delete-then-recreate, since draft notes are created independently and can be appended to. That makes destruction unnecessary and therefore forbidden. One GitLab-specific consequence is documented: bulk_publish publishes every pending draft note including the user's own, so the user is told what is already there before anything is appended.

Platform selection. Both SKILL.md files gain a Platform section resolving GitHub vs GitLab from the PR/MR URL host, then the git remote, then which CLI is authenticated — and asking with AskUserQuestion when those conflict, rather than guessing. Frontmatter descriptions are de-hardcoded from GitHub and pick up MR trigger wording.

Documented degradations

Multi-line comments need position[line_range] line codes that the API doesn't hand you, so they degrade to a single-line anchor with the span described in the comment text. reviewer_state=requested_changes falls back to reviewed where the tier rejects it. You can't approve your own MR. Self-managed hosts go through --hostname. Each is in the relevant Failure modes table with the symptom that surfaces it.

Verification

  • Endpoints, parameters, tier availability, and the bracketed position[...] form-field syntax were checked against current GitLab documentation rather than recalled — including confirming the Draft Notes API is Free tier on GitLab.com, Self-Managed and Dedicated, and that bulk_publish accepts both a summary note and reviewer_state.
  • glab invocations and flags checked against local glab 1.111.0 (notably: glab api has no --jq, so JSON filtering pipes through jq).
  • Operation headings verified to match one-for-one between each skill's github.md and gitlab.md.
  • server/ test suite: 229 passed, including the registry and skill-frontmatter validation tests.

Not verified by execution: no GitLab remote or GitLab MCP server was available, so no recipe has been run end to end against a live merge request.

Follow-up (not in this PR)

registry/mcp/gitlab.yaml points at @modelcontextprotocol/server-gitlab, which is archived and exposes no MR note or discussion tools — it cannot drive either skill. The discovery-based fallback routes around it, but the entry should be repointed. Filing separately.

Summary by CodeRabbit

  • New Features

    • Added GitLab support for pull request comment handling and code reviews.
    • Added automatic platform detection from review URLs, repository remotes, or authenticated command-line tools.
    • Added GitLab workflows for discussions, approvals, draft reviews, replies, and thread resolution.
    • Added fallback options when draft review notes are unavailable, including immediate publication or file-only output.
  • Documentation

    • Expanded guidance with GitLab terminology, authentication, setup, and failure handling.

Both skills already separated the platform-independent workflow from
platform commands keyed by operation name, and both github.md files noted
that references for other platforms could be added and selected by host.
This takes that hook and adds GitLab as a second supported platform.

Each skill gains a references/gitlab.md implementing exactly the same
operation names as its github.md, so no workflow step changes. The
recipes use the glab CLI, with a connected GitLab MCP server as a
fallback when glab is missing or unauthenticated. MCP tool names are
discovered rather than hardcoded, since GitLab MCP servers differ widely
in coverage; an operation with no matching tool stops the workflow with
an explanation instead of being skipped silently.

GitLab's Draft Notes API preserves the draft-first contract: one draft
note per finding, a positionless draft note carrying the summary, and
bulk_publish supplying the summary and reviewer state at submit time.
Because draft delivery can be unavailable on an old instance, a
restricted token, or an MCP-only fallback, preflight probes for it and
the results gate adapts its options before asking the user anything — so
the user is never offered a "draft" that would publish immediately.

Both SKILL.md files gain a Platform section that resolves GitHub vs
GitLab from the PR/MR URL host, the git remote, or which CLI is
authenticated, and asks rather than guessing when those conflict.

API endpoints, parameters and the bracketed position field syntax were
verified against current GitLab documentation; glab invocations were
checked against glab 1.111.0. No recipe has been executed end to end
against a live merge request.
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds GitLab support to pr-review and pr-comments-address. It adds platform selection, GitLab transport and MR operations, discussion handling, draft capability checks, publication fallbacks, and failure-mode guidance.

Changes

GitLab review workflow support

Layer / File(s) Summary
Platform routing for review skills
registry/skills/pr-review/SKILL.md, registry/skills/pr-review/references/github.md, registry/skills/pr-comments-address/SKILL.md, registry/skills/pr-comments-address/references/github.md
The skills distinguish local and public workflows, resolve GitHub or GitLab from URL, repository remote, or authenticated CLI, and select the matching reference.
GitLab comment addressing operations
registry/skills/pr-comments-address/references/gitlab.md
The reference documents glab transport, authentication, repository checks, safe MR checkout, discussion filtering, replies, resolution, and failure handling.
GitLab review operations
registry/skills/pr-review/references/gitlab.md
The reference documents MR context retrieval, discussion and approval discovery, draft notes, review submission, thread replies, and API failure handling.
Draft delivery and fallback handling
registry/skills/pr-review/SKILL.md
Public review preflight now propagates draft capability. The results gate and delivery flow support immediate publication or file-only output when drafts are unavailable, while preserving and verifying existing drafts.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Reviewer
  participant pr-review
  participant glab
  participant GitLabMR
  Reviewer->>pr-review: Start public MR review
  pr-review->>glab: Run authentication and MR preflight
  glab->>GitLabMR: Retrieve MR context and discussions
  GitLabMR-->>pr-review: Return review context
  pr-review->>GitLabMR: Create or publish review notes
  GitLabMR-->>Reviewer: Display review result
Loading

Possibly related PRs

Suggested reviewers: dustyo-o

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the addition of GitLab support to both affected skills.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/pr-skills-gitlab-support

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (1)
registry/skills/pr-comments-address/references/gitlab.md (1)

125-133: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Document permission failures for thread resolution.

GitLab requires the Developer, Maintainer, or Owner role, or authorship of the change, to resolve a merge request thread. Add a 403 Forbidden row that stops retries and reports that the user lacks resolution permission. (docs.gitlab.com)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@registry/skills/pr-comments-address/references/gitlab.md` around lines 125 -
133, Add a 403 Forbidden failure-mode row to the thread-resolution handling
table, specifying that retries must stop and the summary must report
insufficient permission to resolve the thread, including GitLab’s eligible roles
or change authorship as context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@registry/skills/pr-comments-address/references/gitlab.md`:
- Line 100: Update the posting recipes in the GitLab reference, including the
commands around the discussion note endpoints, so comment and reply bodies are
passed through a safely populated variable, file, or JSON stdin rather than
interpolated into shell source. Ensure all affected recipes handle apostrophes
and shell syntax as literal GitLab-controlled text without changing the intended
request body.
- Around line 33-36: Update the preflight commands around glab auth status and
glab api user to explicitly use the hostname parsed from the selected MR URL, or
require the target repository checkout before running them. Ensure
authentication and user lookup are bound to the MR’s GitLab host rather than
inferred from the current remote, environment, or default configuration, while
preserving the existing repository validation.
- Around line 81-82: Update the jq pipeline around the discussion selector to
filter each discussion’s notes to non-system entries before selecting the latest
note, rather than rejecting discussions containing any system note. Preserve the
discussion and set `$last` and `$first` from the filtered notes so mixed
activity and human-feedback discussions remain available.

In `@registry/skills/pr-comments-address/SKILL.md`:
- Around line 23-29: Update the platform-resolution instructions in
registry/skills/pr-comments-address/SKILL.md lines 23-29 and
registry/skills/pr-review/SKILL.md lines 23-29 to compare every available signal
before selecting the platform; prompt with AskUserQuestion when signals conflict
or none resolve, and apply precedence only when later signals are unavailable.
Preserve the existing GitHub/GitLab reference routing and state the resolved
platform alongside the mode.

In `@registry/skills/pr-review/references/gitlab.md`:
- Line 164: Update the post-creation verification instructions around
find-pending-review to compare the resulting draft count against the preflight
baseline plus the drafts created in the current run, rather than against the
number posted alone. Preserve existing drafts and include the summary note in
the created-draft count, then report the verified total with the MR URL.
- Around line 175-177: Update the submit-review flow and its self-authored MR
and rejection-fallback paths to pass the user-selected reviewer state instead of
hard-coded requested_changes or reviewed values. Preserve the selected state
through bulk_publish and fallback handling, and do not silently substitute
reviewed when requested_changes is rejected; require user confirmation before
changing it.
- Around line 46-56: Update the draft capability probe and the corresponding
logic near the step 5 results gate to validate draft-note write access, not only
listing access. Preserve an explicit unavailable/unknown state until a
draft-note POST is confirmed, and avoid assuming 404 always means an old
instance or 403 always means a missing scope. Apply the same state handling to
the related logic at the later referenced section.
- Around line 106-114: Update the own-notes listing command to query the merge
request’s discussions endpoint instead of notes, then traverse each discussion’s
notes and retain only notes authored by $ME. Output one row per matching note
containing the discussion ID, note ID, position path, position line, and body
preview so findings can be mapped to reply-to-thread.
- Around line 155-162: The GitLab review flow currently delivers the summary
twice. Update the `submit-review` `bulk_publish` invocation to omit the
`note=<summary>` argument, while preserving the positionless summary draft note
created by `create-draft-review` and the verbatim summary output in step 7.
- Around line 38-39: Update the authentication preflight commands in the GitLab
review flow to pass the resolved GitLab host via --hostname to both glab auth
status and glab api user, ensuring checks and username resolution target the
same host.

---

Nitpick comments:
In `@registry/skills/pr-comments-address/references/gitlab.md`:
- Around line 125-133: Add a 403 Forbidden failure-mode row to the
thread-resolution handling table, specifying that retries must stop and the
summary must report insufficient permission to resolve the thread, including
GitLab’s eligible roles or change authorship as context.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fe4559fb-3969-4d7c-b871-203599fab61c

📥 Commits

Reviewing files that changed from the base of the PR and between 25a69cf and 0e7d43b.

📒 Files selected for processing (6)
  • registry/skills/pr-comments-address/SKILL.md
  • registry/skills/pr-comments-address/references/github.md
  • registry/skills/pr-comments-address/references/gitlab.md
  • registry/skills/pr-review/SKILL.md
  • registry/skills/pr-review/references/github.md
  • registry/skills/pr-review/references/gitlab.md

Comment thread registry/skills/pr-comments-address/references/gitlab.md Outdated
Comment thread registry/skills/pr-comments-address/references/gitlab.md Outdated
Comment thread registry/skills/pr-comments-address/references/gitlab.md Outdated
Comment thread registry/skills/pr-comments-address/SKILL.md Outdated
Comment thread registry/skills/pr-review/references/gitlab.md Outdated
Comment thread registry/skills/pr-review/references/gitlab.md Outdated
Comment thread registry/skills/pr-review/references/gitlab.md
Comment thread registry/skills/pr-review/references/gitlab.md Outdated
Comment thread registry/skills/pr-review/references/gitlab.md Outdated
Comment on lines +175 to +177
glab api -X POST "projects/$PROJECT/merge_requests/$IID/draft_notes/bulk_publish" \
--form 'note=<summary>' \
--form 'reviewer_state=requested_changes'

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Repository files matching gitlab references:"
fd -a 'gitlab\.md$' . | sed 's#^\./##'

echo
echo "Relevant snippets:"
sed -n '150,230p' registry/skills/pr-review/references/gitlab.md

echo
echo "Search submit-review/reviewer_state/mergerows/bulk_publish:"
rg -n "submit-review|reviewer_state|bulk_publish|requested_changes|reviewed|approval" registry/skills/pr-review -S

Repository: provectus/awos-recruitment

Length of output: 14795


Preserve the user's chosen reviewer state.

submit-review currently always sends reviewer_state=requested_changes. The same hard-coded reviewed state appears in the self-authored MR path and in the rejection fallback. These states both describe pending-review outcomes and are not interchangeable. Pass the user's selected state; if requested_changes is rejected, do not treat it as an implicit reviewed unless the user confirms.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@registry/skills/pr-review/references/gitlab.md` around lines 175 - 177,
Update the submit-review flow and its self-authored MR and rejection-fallback
paths to pass the user-selected reviewer state instead of hard-coded
requested_changes or reviewed values. Preserve the selected state through
bulk_publish and fallback handling, and do not silently substitute reviewed when
requested_changes is rejected; require user confirmation before changing it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requested_changes there is a recipe sample, not a hard-coded verdict. The line directly below names both allowed states, SKILL.md step 6 says to run submit-review "with the verdict", and its Boundaries forbid auto-approving — the user chooses. The GitHub reference uses the identical pattern (-f event=REQUEST_CHANGES followed by "event is the verdict the user picks"), so changing only the GitLab file would break parity between the two references for no behavioural gain.

On the two fallbacks, neither is silent. When the instance rejects reviewer_state=requested_changes the rejected call is the publish, so nothing was published; the retry with reviewed carries the verdict in the summary text, which that table row already mandates. The glab mr approve fallback fires only on "cannot approve your own merge request", where approval is impossible by definition and reviewed is the only remaining state — it doesn't record or imply an approval.

Leaving both as they are. I did make the sample's status explicit in the surrounding text, so it reads less like a default.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo 'GitLab submit-review and fallback section:'
sed -n '165,235p' registry/skills/pr-review/references/gitlab.md

echo
echo 'GitLab occurrences with context:'
rg -n -C 4 'reviewer_state|requested_changes|reviewed|bulk_publish|mr approve|self-authored|own merge request' \
  registry/skills/pr-review/references/gitlab.md

echo
echo 'Skill verdict selection and boundary text:'
rg -n -C 4 'submit-review|verdict|auto-approv|approve' registry/skills/pr-review/SKILL.md

echo
echo 'GitHub equivalent submit-review section:'
rg -n -C 5 'submit-review|REQUEST_CHANGES|event.*verdict|event=' \
  registry/skills/pr-review/references/github.md

Length of output: 27791


@AlexanderMakarov, I verified the current text.

The submit-review sample is now clear. Line 220 explicitly states that requested_changes is an example and that the user chooses the state. This matches the GitHub reference. I withdraw that part of the finding.

The fallback concern remains. Lines 270–271 prescribe a retry or publish with reviewer_state=reviewed after the user selected a different verdict. The summary text can record the original verdict, but it does not preserve the selected GitLab reviewer state. The fallback should report the limitation and require the user to confirm publication as reviewed.

🐇︵


✏️ Learnings added
Learnt from: AlexanderMakarov
Repo: provectus/awos-recruitment PR: 81
File: registry/skills/pr-review/references/gitlab.md:175-177
Timestamp: 2026-08-05T15:16:17.936Z
Learning: In `registry/skills/pr-review/references/github.md`, the `submit-review` recipe uses `event=REQUEST_CHANGES` as an example and then explicitly states that `event` is the verdict selected by the user. The GitLab reference is intended to use the same documentation pattern for `reviewer_state`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

You are interacting with an AI system.

@AndreyNenashev AndreyNenashev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really solid work — this is exactly the seam the github.md headers promised, and it shows. I verified the operation-name parity claim (it holds one-for-one for both skills), and spot-checked the load-bearing API claims against current GitLab docs: Draft Notes on Free tier, bulk_publish with note + reviewer_state, in_reply_to_discussion_id, the glab --jq quirk — all check out. The draft-first mapping onto draft notes is genuinely well done, and the never-destroy adaptation (append is safe, but bulk_publish carries the user's own drafts) shows the model was understood rather than transliterated. Also appreciated: the honest "not verified by execution" note and filing #82 for the archived MCP server.

Two things I'd like addressed before merge, both small in diff terms:

1. Self-managed host resolution breaks the no-clone flow. The porcelain recipes (glab mr view/diff/approve/note) resolve their host from the cwd's git remote and default to gitlab.com — but pr-review public mode tells the model not to check out the repo, and the watcher handoff makes no-clone the normal entry. On a self-managed instance (i.e., our own), fetch-pr-context 404s on the first call. The existing "--hostname on every glab api call" note doesn't cover these. Suggest: pass the full MR URL to -R (the user already gave it) or export GITLAB_HOST=<host> once, stated in the Terminology section.

2. The publish-now path needs a partial-failure protocol. GitHub delivery is one atomic POST; the DRAFTS=no / publish-now branch here is N independent live discussion POSTs, each instantly visible to the MR author. If the run dies mid-sequence (token expiry, rate limit), the author sees a fragment of a review — no summary, no verdict — which is exactly the "half-delivered review work is worse than not starting" outcome this PR guards against on the MCP path but not here. Suggest: keep a posted/remaining ledger; on failure, stop and fold the unposted findings into the summary note; verify the final count against the approved set, mirroring create-draft-review.

Since a live run against our GitLab instance is what would close #80's Q3/Q4 anyway, one throwaway-MR smoke test (create drafts → bulk_publish with reviewer_state) would verify both fixes and upgrade the whole file from docs-verified to exercised. Happy to help set that up.

Of CodeRabbit's comments, two are real and worth taking in the same commit: the draft-count verification breaks in the append case (compare against baseline + created, not created alone), and the summary can land twice on the submit-now path (positionless draft note + bulk_publish note= — drop one). The rest I'd decline or treat as optional polish; in particular I'd keep first-signal-wins platform resolution — a user-supplied MR URL is authoritative, and comparing all signals would just generate spurious conflict prompts.

Also: worth adding "Part of #80" to the description so the design doc and this implementation are linked.

IID=<MR_IID>
```

For a self-managed instance, add `--hostname <host>` to every `glab api` call (or run inside a clone of that instance's repo, where `glab` picks the host up from the git remote).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This note covers glab api, but the porcelain commands below (mr view, mr diff, mr approve, mr note) resolve their host from the git remote and fall back to gitlab.com — and preflight explicitly says not to check out the repo, so in the no-clone flow every one of them targets the wrong host on a self-managed instance. Since the user handed us the MR URL, the host is already known: suggest documenting -R <full MR project URL> for the porcelain calls (glab accepts a full URL there) or a session-level GITLAB_HOST=<host>, so glab api --hostname, the porcelain commands, and the auth check are all pinned to the same host.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified on 1.111.0: none of mr view, mr diff, mr approve, mr note has a --hostname flag — only -R — and with a bare GROUP/PROJECT the host comes from the git remote or the saved config.

Reproduced the failure from a directory with no GitLab remote: glab repo view -R gitlab-org/cli resolved against the saved self-managed host and 404'd, while GITLAB_HOST=gitlab.com glab repo view -R gitlab-org/cli returned the right project. -R https://gitlab.com/gitlab-org/cli pins it too, matching -R's "full URL … is also accepted".

Taking both suggestions: the setup block gains HOST=<host from the MR URL>, export GITLAB_HOST="$HOST" and MR_URL=https://$HOST/<GROUP>/<PROJECT>, the porcelain recipes switch to -R "$MR_URL", and the note at line 25 is rewritten to say the pin covers glab api, glab auth status and the porcelain rather than glab api alone. That also resolves the preflight auth thread on line 39.

glab mr approve $IID -R <GROUP>/<PROJECT>
```

If `DRAFTS=no`, there is nothing to publish and this operation doesn't apply — the user chose publish-now at the adapted gate, so post each finding as a real discussion instead (same `--form position[...]` fields as `create-draft-review`, against `.../discussions`, with `body=` in place of `note=`), and post the summary with `glab mr note $IID -m '<summary>'`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch posts each finding as a separate live discussion, so unlike GitHub's single atomic POST, a mid-sequence failure leaves a partially published review the author can already see — with no summary and no verdict. That's the exact failure mode the Transport section rules out for the MCP fallback ("half-applied feedback is worse than not starting"), so this path deserves the same guarantee: track posted vs. remaining, on any failure stop and fold the unposted findings into the summary note so the review reaches the author complete, and finish by verifying the posted count against the approved set like create-draft-review does.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and the gap is slightly wider than ordering: in the DRAFTS=no path the summary goes out last via glab mr note, so a failure partway through leaves live inline discussions with no summary and no verdict — the same half-applied state the Transport section rules out for the MCP fallback. The failure-modes table only covered it for one symptom ("Note position is invalid" → move the finding into the summary) and only per-finding, with no instruction to stop.

Expanded that line into a procedure: keep a posted/remaining list, stop at the first failure instead of continuing, fold every unposted finding verbatim into the summary note, post the summary on both the success and abort paths, and finish by counting posted discussions against the approved set the way create-draft-review does.

Host resolution was the largest gap. Neither reference pinned the
instance, so every glab call resolved its host from the current git
remote, GITLAB_HOST, or the saved config — never from the MR under
review. Verified: run from a checkout whose only remote is GitHub,
`glab auth status` reports a saved self-managed host rather than
failing, and `glab repo view -R group/project` resolves against that
host and 404s. Both files now set HOST and MR_URL up front, export
GITLAB_HOST, pass --hostname explicitly on glab api and glab auth
status, and use -R "$MR_URL" for the porcelain, which has no --hostname
flag. pr-comments-address's identity check compares web_url instead of
path_with_namespace, since two instances can share a project path.

The working-set filter in pr-comments-address dropped any discussion
containing a system note. GitLab appends one inside human diff threads
whenever the anchored line moves, so on a real MR the old filter kept
11 of 44 live threads and silently lost 33. It now filters notes[] and
judges the discussion by what remains.

Posting recipes built their bodies inline in single quotes, which one
apostrophe in review prose truncates. They now pass the body by
variable, with a note that glab's -f/-F semantics are the inverse of
gh's for @file values.

Draft-notes probing only ever proved read access: a read_api token
passes the GET and then 403s on the first draft POST, after the whole
analysis pass has run. The probe now also reads
personal_access_tokens/self and treats DRAFTS=yes as provisional until
a POST succeeds; the failure-mode rows no longer equate 404 with "old
instance" or 403 with "missing scope".

Also: the own-notes listing is rebuilt over /discussions, because notes
from /notes carry no discussion_id and so cannot produce the
DISCUSSION_ID that reply-to-thread needs; --paginate emits one array
per page, so the re-review timestamp now slurps before aggregating;
bulk_publish's note= is marked as double-posting the summary when a
positionless summary draft already exists; the post-create count checks
against a recorded baseline, since drafts are appended to rather than
replaced; and the DRAFTS=no publish-now path becomes a procedure that
stops at the first failure and folds unposted findings into the summary
instead of leaving a partially published review.

Both SKILL.md files: "take the first signal that answers" made the
following "if the signals conflict" clause unreachable. Now an explicit
precedence, with the prompt reserved for "none of the three resolves".

One reviewer point is declined and answered on the thread: the
requested_changes in submit-review is a recipe sample, not a hard-coded
verdict, and matches the pattern github.md already uses.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
registry/skills/pr-review/SKILL.md (1)

100-101: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Propagate the no-draft delivery state through the gate and summary.

This fallback offers publish now or write to a file, but the following Proceed text still says to post a draft, and Step 7 still says that a draft awaits submission. That status is false for both fallback paths. State the selected delivery mode explicitly: published immediately, or saved locally with no platform delivery.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@registry/skills/pr-review/SKILL.md` around lines 100 - 101, Update the
delivery gate and Step 7 summary in the review workflow to propagate the
no-draft state: when draft delivery is unavailable, remove wording that says a
draft will be posted or awaits submission. Explicitly describe the selected
outcome as either findings published immediately via “publish now” or saved
locally with no platform delivery via “write to a file,” while preserving the
existing draft wording for supported delivery.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@registry/skills/pr-review/SKILL.md`:
- Around line 100-101: Update the delivery gate and Step 7 summary in the review
workflow to propagate the no-draft state: when draft delivery is unavailable,
remove wording that says a draft will be posted or awaits submission. Explicitly
describe the selected outcome as either findings published immediately via
“publish now” or saved locally with no platform delivery via “write to a file,”
while preserving the existing draft wording for supported delivery.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 418909b9-bff4-4a96-a594-1fb22b3b567a

📥 Commits

Reviewing files that changed from the base of the PR and between 0e7d43b and fdf113a.

📒 Files selected for processing (4)
  • registry/skills/pr-comments-address/SKILL.md
  • registry/skills/pr-comments-address/references/gitlab.md
  • registry/skills/pr-review/SKILL.md
  • registry/skills/pr-review/references/gitlab.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • registry/skills/pr-comments-address/references/gitlab.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants