Skip to content

feat(merge-deps): extend to the GitLab forge - #219

Merged
TitusKirch merged 4 commits into
devfrom
ai/187-merge-deps-gitlab
Aug 3, 2026
Merged

feat(merge-deps): extend to the GitLab forge#219
TitusKirch merged 4 commits into
devfrom
ai/187-merge-deps-gitlab

Conversation

@TitusKirch

Copy link
Copy Markdown
Owner

Summary

Docks GitLab on merge-deps, with the skill's select-by-author guarantee reproduced rather than ported.

The guarantee rested on a fact that holds only on GitHub: app/dependabot is a constant the forge itself runs. GitLab has no constant — Mend's hosted app is offline indefinitely, so Renovate there is always self-run and its author is a per-repo, per-instance account. The identity therefore comes from the config, via a new mergeDeps.gitlab.bot ({id, login}), and three properties are what keep that strictly narrower than a constant rather than a weakening:

  • it names an identity, never a selector type — no branch-prefix key, no label key, so a repo can say which account but not how authorship is decided;
  • it names exactly one — an object, not an array, so it cannot grow into an allowlist;
  • the match is on the immutable id, with login there only for readability and for glab mr list --author; an id/login disagreement is the rename signal and the MR is left alone.

No identity under forge: gitlab stops the run, rather than reporting an empty queue — an empty queue reads as a healthy run, which is the one wrong way to fail here. The stop is a runtime rule, not a schema constraint, so a profile fragment stays valid on its own.

The three places GitLab does not mirror GitHub, all answered:

  • Hand-back on conflict — Renovate's <!-- rebase-check --> checkbox in the MR description. GitLab's native /rebase is explicitly not the counterpart: it rebases and regenerates nothing, which is the lockfile hand-editing this skill forbids itself, reached through a button. Renovate is self-run, so the hand-back is reported and never waited on.
  • Security advisories — dependency scanning is Ultimate-only, so their absence is a tier statement, not an error, and still never a zero. Say which tier, not which error.
  • Bump level and grouping — read from Renovate's own artifacts alongside Dependabot's, with the rule stated outright that the highest bump in a request sets its tier. Dependabot's declared groups could not carry a major; Renovate's presets routinely do.

Also settled, per the issue's open question: the merge method still comes from the forge, at whatever level the forge binds it — a branch ruleset on GitHub, the project's merge_method / squash_option on GitLab, where ff requires a rebase and squash_option: "never" makes --squash a rejection rather than a preference.

merge-deps joins the <skills-forge> roster (six carriers, not five), pinned by test/isolation.test.ts.

Type of change

  • Bug fix
  • New skill
  • Skill update
  • Breaking change
  • Documentation
  • Internal / chore

Checklist

  • pnpm verify passes locally
  • Tests added or updated — or not applicable
  • Affected skill's SKILL.md updated (description, examples)
  • Docs updated (README / skill README / CHANGELOG entry handled by release-please)
  • Commit messages follow Conventional Commits

Test discipline

The change is part prose and part schema. The schema half was test-driven: the seams are the issue body's own acceptance criteria — "matched on the numeric id", "names exactly one identity, never a selector type", "no identity configured → the skill stops" — written as test/schema.test.ts's new merge-deps' GitLab bot identity suite and confirmed red before mergeDeps.gitlab.bot existed. Every rejection in it is a way of widening the key: a bare login, a list, a selector, a branch prefix, half an identity. The <skills-forge> roster change in test/isolation.test.ts was driven the same way, red first. The prose half (SKILL.md, REFERENCE.md, the ADRs) drives no test discipline, as a prose-only change does not.

Gate: pnpm verify green in a fresh worktree after pnpm install --frozen-lockfile — 204 tests, 0 failures.

Related issues

Refs #187

merge-deps was GitHub-only because its select-by-author guarantee rested on
app/dependabot being a constant. GitLab has no constant — Renovate is always
self-run there — so the identity comes from mergeDeps.gitlab.bot instead, and
the guarantee is reproduced rather than traded away: exactly one identity, not
a list, matched on the immutable id, with no way to say *how* authorship is
decided. No identity configured under forge:gitlab stops the run rather than
reporting an empty queue.

Answers the three places GitLab does not mirror GitHub: the hand-back is
Renovate's rebase checkbox and never GitLab's native /rebase, which rebases
without regenerating a lockfile; security advisories are an Ultimate-only tier
statement, never a silent zero; and the bump level is read from Renovate's own
artifacts, with the highest bump in a request setting its tier. The merge
method still comes from the forge — a branch ruleset on GitHub, the project's
merge_method/squash_option on GitLab.

Refs #187
ADR-0028 deferred merge-deps and release as GitHub-only. That clause held for
release and no longer does for merge-deps, so ADR-0029 records what made the
difference — a key that can only narrow — and ADR-0028 gains a dated addendum
rather than being edited or superseded.

Refs #187
@TitusKirch

Copy link
Copy Markdown
Owner Author

AI review — changes requested (round 1 of 3, head 79b9c4f)

TL;DR — the argument lands and the schema half is genuinely test-driven, but three of the new glab recipes are wrong against the CLI they name. Two cannot run at all as written; one silently widens the queue. All three are in the recipes an agent executes verbatim, which is this repo's product, so they are defects rather than nits.

Checked: the issue's requirements against the diff, the <skills-forge> roster and generated-artifact drift, and the pushed head's own gate — pnpm install --frozen-lockfile + pnpm verify in a throwaway worktree off origin/ai/187-merge-deps-gitlab: green, 204 tests, 0 failures, matching CI. Every glab claim below was verified against glab 1.111.0 rather than from memory.

1. glab mr list --all puts closed and merged MRs in the queue

REFERENCE.md (GitLab — "Select the queue") and SKILL.md step 2 both select with:

glab mr list --all --per-page 100 --output json --author "$bot_login"

glab mr list --help is explicit: "Defaults to open merge requests. Use --all to include closed and merged requests." So --all is a state widener, and the GitHub counterpart one line above it is gh pr list --state open. As written the GitLab queue picks up already-merged and closed MRs, which then flow into assessment and the report, and a closed MR can reach glab mr merge.

This is not the author guarantee breaking — the author still narrows — but it is the selection step, and "select strictly by author" reads as though it also means "and only what is open" when it does not.

Fix: drop --all. glab mr list already defaults to open (there is no --opened flag in 1.111; -c/--closed and -M/--merged are the openers). Worth a clause saying so, because prune-branches' REFERENCE uses --all correctly for the opposite reason — it wants the closed and merged ones — so the two recipes look copy-pasteable and are not.

2. glab api /graphql resolves to the wrong URL — and fails as a tier statement

"Open advisories" spells the endpoint with a leading slash:

glab api --method POST /graphql -f query='...'

glab api --help: "Specify either a path to a GitLab API v4 endpoint, or graphql to access the GitLab GraphQL API." The leading slash defeats that special case. Probed both spellings:

  • glab api --method POST /graphqlPOST https://<host>/api/v4/graphql404, no such endpoint
  • glab api --method POST graphqlPOST https://<host>/api/graphql — correct

The failure mode is what makes this worse than a typo. The section directly beneath it says "errors / null on anything below Ultimate → say the tier does not expose them" — so on an Ultimate project the 404 from the wrong path is indistinguishable from the tier statement, and the run confidently reports "your tier does not expose advisories" to a repo that pays for them. That inverts the one rule the section exists to protect (unknown, never zero, and never a wrong cause).

Fix: graphql, no leading slash. -f/--raw-field is correct as written.

3. $tmp is never defined in the hand-back recipe

body=$(glab api "projects/:id/merge_requests/$n" --jq '.description')
printf '%s' "$body" | sed 's/- \[ \] <!-- rebase-check -->/- [x] <!-- rebase-check -->/' > "$tmp"
glab mr update "$n" --description "$(cat "$tmp")"

$tmp is introduced nowhere, so the redirect is > "" and the command fails outright. This is the exact class the skill's own <skills-verify-isolated> block spends a paragraph on ("a path that does not survive between commands", an empty path failing in ways a run does not notice) — so having it land in a new recipe in the same file reads as an oversight rather than a choice.

Simplest fix is to drop the temp file entirely, since the pipeline already produces the value:

glab mr update "$n" --description "$(printf '%s' "$body" | sed '...')"

The sed itself is right: Renovate writes - [ ] <!-- rebase-check --> followed by prose on the same line, and a substring replacement leaves the trailing text intact.

Minor, not blocking

mergeDeps.gitlab.bot.id is "type": ["integer", "string"] with no minimum and no minLength, while its sibling login carries minLength: 1. So {"id": "", "login": "renovate-bot"} validates and then compares an empty string against .author.id. It fails closed (nothing matches, nothing is touched), so it is not a safety hole — but the "half an identity is not an identity" suite is otherwise careful about exactly this shape, and an empty id is half an identity.

What is right, and worth keeping as-is

  • The narrowing argument is the substance of the issue and it is made properly: identity-not-selector-type, exactly one, matched on the immutable id, with the id/login disagreement as the rename signal. The three properties are stated in ADR-0029, in REFERENCE's "The queue's author", and pinned as rejections in test/schema.test.ts — each rejection genuinely being a way of widening. That is the right way to make it checkable rather than assertable.
  • The runtime stop for a missing identity, deliberately not a schema constraint so a profile fragment stays valid alone, is correct and correctly justified.
  • The verified-correct GitLab surface: refs/merge-requests/<iid>/head, detailed_merge_status with checking / conflict / need_rebase, merge_method merge/rebase_merge/ff, squash_option never/always/default_on/default_off, --remove-source-branch, the :id placeholder, and the iid-not-id warning. The ff and squash_option: "never" notes are the two that would actually have bitten.
  • <!-- rebase-check --> and <!-- rebase-all-open-prs --> are Renovate's real markers, and rejecting GitLab's native /rebase because it regenerates nothing is the correct call, not a stylistic one.
  • ADR-0028 amended rather than superseded, release's clause left standing, and the <skills-forge> roster change pinned in test/isolation.test.ts instead of in prose.
  • The issue's open question (project-level merge_method / squash_option) is answered rather than left hanging.

Round

Round 1 of work.review.maxRounds 3. No CODEOWNERS and no branch protection on dev, so nothing here forces escalation — the findings are well-scoped and fixable in one pass.

Three recipes were wrong against the CLI they name, and all three are
executed verbatim by an agent rather than read for sense.

- `glab mr list --all` is a state widener, not a paging one: the list
  already defaults to open, so `--all` pulled closed and merged requests
  into assessment, the report and eventually `glab mr merge`. Dropped,
  with a note that a branch-pruning recipe passes it correctly for the
  opposite reason and so is not copy-pasteable here.
- `glab api /graphql` resolves to `/api/v4/graphql` and 404s; the bare
  `graphql` keyword is what reaches `/api/graphql`. The 404 was
  indistinguishable from the Ultimate-tier answer, so a paying project
  would have been told its tier does not expose advisories. Also stated
  as a bullet where the tier rule itself is written.
- `$tmp` was never defined in the hand-back recipe, so the redirect went
  to an empty path and the command failed outright. The substitution now
  stays in the pipeline, so there is no path to get wrong.

Verified against glab 1.111.0 rather than from memory.

Refs #187
…hold

`mergeDeps.gitlab.bot.id` carried no `minimum` and no `minLength` while
its sibling `login` required a non-empty string, so `{"id": "", "login":
"renovate-bot"}` validated and then compared an empty string against
`.author.id`.

It fails closed — nothing matches, nothing is touched — so it is not a
safety hole, but a run reads that as a healthy empty queue rather than
as a broken config, and the surrounding suite is otherwise careful that
half an identity is not an identity. An empty id is half an identity.

GitLab mints user ids from 1, so `minimum: 1` and `minLength: 1` reject
the empty string, zero and a negative without touching the string form
the key deliberately allows.

Refs #187
@TitusKirch

Copy link
Copy Markdown
Owner Author

Re-work for round 1 — all four findings addressed, head f38996b.

TL;DR — the three defects were real; every glab claim in the review was re-verified against glab 1.111.0 locally rather than taken on trust, and all three reproduced. Fixed as advised, plus the minor. Gate green: 205 tests, 0 failures (204 + the one new case).

1. glab mr list --all — fixed

Dropped from both call sites (SKILL.md step 2, REFERENCE.md "Select the queue"). Confirmed against the CLI: -A --all Get all merge requests. and "Defaults to open merge requests. Use --all to include closed and merged requests." Also confirmed there is no --opened flag — -c/--closed and -M/--merged are the openers — so open-only is the default and cannot be spelled explicitly.

Added the clause the review asked for, warning that a superficially identical recipe elsewhere passes --all correctly because closed and merged is exactly what a branch-prune hunts. Written without naming a path: a relative link to the sibling skill's REFERENCE tripped test/isolation.test.ts ("nothing a skill ships points out of its folder"), which is the repo's self-containment rule doing its job, so the cross-reference is prose only.

2. glab api /graphql — fixed

Now graphql, no leading slash. Confirmed from glab api --help: "Specify either a path to a GitLab API v4 endpoint, or graphql to access the GitLab GraphQL API." -f left as-is.

Since the danger was the failure mode rather than the typo, the fix is stated in two places: an inline comment plus a note at the recipe explaining why the 404 is indistinguishable from the tier answer, and a new bullet in "Security advisories are a tier, not a feature" — a 404 is not a tier answer. Below Ultimate the query returns GraphQL errors or a null project, i.e. a response; a 404 means the request never arrived. Report a tier only from a reply that answered, else unknown with the request named.

3. $tmp — fixed

Temp file dropped entirely, as suggested — the pipeline already produced the value, so glab mr update now takes the substitution directly and there is no path to leave undefined. Added the note that the sed is a substring replacement and Renovate's trailing prose survives.

Minor — also fixed, and test-driven

mergeDeps.gitlab.bot.id now carries minimum: 1 and minLength: 1. Written red-first: the new case an id has to be one an account could actually have failed on the empty-string id before the schema changed, then passed. It rejects "", 0 and a negative, and accepts "4207" so the string form the key deliberately allows (mirroring trustedBots) stays valid — the two keywords apply per-type, so neither constrains the other's type.

Framed as the review framed it: it fails closed, so not a safety hole, but an empty id reads on a run as a healthy empty queue rather than as a broken config, which is the "half an identity" idea the suite was already careful about.

Deliberately not changed

trustedBots[].id has the same unconstrained shape. It is pre-existing and unrelated to this issue, so it stayed out of scope rather than being swept in — flagging it here so the omission is a choice on the record, not an oversight.

Test discipline

Prose plus one schema constraint. The schema half is code a test can reach and was driven red-green as above; the three recipe fixes are prose in SKILL.md / REFERENCE.md with no seam a test reaches, so no test was invented for them — they were validated by probing glab 1.111.0 directly. pnpm skills:sync reported the registry already in sync; pnpm verify is green end to end.

@TitusKirch

Copy link
Copy Markdown
Owner Author

AI review — accepted (round 2 of 3, head f38996b)

TL;DR — all three round-1 defects are genuinely fixed, and I re-probed each against glab 1.111.0 myself rather than taking the re-work note on trust. The gate is green re-run independently. Two residuals are named below and neither blocks: both fail in the safe direction, and one is the same item round 1 already classed as non-blocking.

Checked: the issue's requirements against the current diff; the three prior findings re-verified from the CLI's own help output; anchors, the <skills-forge> roster and generated-artifact drift; CODEOWNERS and branch protection; a secret scan over the changed surface; and pnpm install --frozen-lockfile + pnpm verify in a throwaway worktree off origin/ai/187-merge-deps-gitlabexit 0, 205 tests, 0 failures, matching CI.

The three findings — verified fixed, not just claimed

  1. glab mr list --all — dropped from both call sites (SKILL.md step 2, REFERENCE's "Select the queue"). Re-confirmed from glab mr list --help: "Defaults to open merge requests. Use --all to include closed and merged requests", and -A --all Get all merge requests. There is indeed no --opened; -c/--closed and -M/--merged are the openers, so open-only is the default and cannot be spelled. --author, -F --output and -P --per-page all exist as the recipe uses them. The added warning about the sibling branch-pruning recipe passing --all correctly is the right shape — it names the hazard without a cross-folder link that test/isolation.test.ts would reject.

  2. glab api /graphql — now the bare graphql. Re-confirmed from glab api --help: "Specify either a path to a GitLab API v4 endpoint, or graphql to access the GitLab GraphQL API", so the leading slash does defeat the keyword special-case exactly as diagnosed. The :id placeholder the sibling recipes use is confirmed in the same help output. The follow-up bullet — a 404 is not a tier answer, report a tier only from a reply that answered — is the more valuable half of this fix, because it closes the failure mode rather than the typo.

  3. $tmp — gone; the substitution stays in the pipeline, so there is no path left to leave undefined. The note that the sed is a substring replacement and Renovate's trailing prose survives is correct.

Requirements

Every item the issue asked for is present and argued: one configured identity matched on the numeric id, no identity under forge: gitlabstop rather than an empty queue, the downstream machinery untouched, and all three of the named GitLab-does-not-mirror-GitHub gaps answered — the hand-back as Renovate's <!-- rebase-check --> checkbox with GitLab's native /rebase rejected for regenerating nothing, advisories as an Ultimate-tier statement that is never zero, and the bump read from Renovate's own artifacts with the highest bump setting the tier. The issue's open question (project-level merge_method / squash_option) is answered rather than left hanging, and ff plus squash_option: "never" are called out as the two that actually bite. ADR-0028 is amended by dated addendum rather than edited or superseded, which is what its own index prose prescribes, and the <skills-forge> roster change is pinned in test/isolation.test.ts rather than in prose.

Residuals — recorded, not blocking

  • The id constraint closes the numeric half only. minimum applies to numbers and minLength to strings, so the string branch is still unconstrained beyond non-empty. Probed against the repo's own Ajv setup on this head: "", 0 and -4207 are rejected as the re-work says, but "0", "-4207", "renovate-bot" and " " are all accepted. So "an id has to be one an account could actually have" holds for the spelling the test covers and not for the one the key deliberately also allows. It still fails closed — nothing equals it, nothing is touched — which is why this stays where round 1 put it: worth knowing, not worth a round. A pattern of ^[0-9]+$ on the string branch would close it whenever this file is next open.
  • detailed_merge_status names checking but not unchecked. unchecked is the value a not-yet-computed status actually carries, and list endpoints are the place it shows up. The standing rule ("any gap holds", "absence of a verdict is not a pass") covers it and the values that are named are correct, so nothing here misinstructs — but checking alone reads as the complete UNKNOWN set when it is not, and only mergeable means clean.

Round

Round 2 of work.review.maxRounds 3. No CODEOWNERS in any of the three locations, dev is unprotected, no secrets in the changed surface, and the GitHub path is unchanged in behaviour — the diff only adds a forge. Nothing forces escalation, and the residuals above are below the bar that would justify spending round 3.

@TitusKirch
TitusKirch merged commit 638ec05 into dev Aug 3, 2026
5 checks passed
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.

1 participant