feat(pr): add GitLab merge and teardown discovery support - #1388
Open
es-nflange wants to merge 1 commit into
Open
feat(pr): add GitLab merge and teardown discovery support#1388es-nflange wants to merge 1 commit into
es-nflange wants to merge 1 commit into
Conversation
fm-pr-merge.sh only merged GitHub PRs, so operators whose projects are GitLab-hosted had no sanctioned way to land approved work through the merge helper AGENTS.md requires for every task merge. Add a GitLab branch that merges via glab, addressed by the merge request's full host-qualified project path since GitLab projects can sit arbitrarily deep under groups. fm-teardown.sh's landed-work check gets the same treatment: it now resolves a GitLab merge request's state and head (via glab and GitLab's own refs/merge-requests/<n>/head ref) both from a recorded pr= and, when none was recorded, by guessing the provider from the project's origin remote and discovering the merge request from the branch name - always failing safe to the existing content check on any lookup uncertainty. fm-brief.sh now derives the project's forge from its origin remote and generates matching tool guidance (glab/merge request vs gh-axi/PR) instead of unconditionally telling every worker to use gh-axi.
Author
|
Follow-up on the testing note above, now that the background full-suite runs have progressed further.
I won't hold this PR open for the remainder of the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Firstmate could deliver and land work on GitHub but not GitLab, even though
bin/fm-pr-lib.shalready parsed GitLab merge request URLs for the watcher. This closes the gap so GitLab-hosted projects have a sanctioned way to land approved work.bin/fm-pr-merge.sh(primary fix): adds a GitLab branch that merges viaglab mr merge, addressed by the merge request's full host-qualified project URL (https://<host>/<group path>) rather than owner/repo, since a GitLab project can sit several groups deep. Recordspr=before merging exactly as before. Defaults to--squash, adds--yes --auto-merge=falseso the merge happens immediately and deterministically rather than deferring to a pipeline-success auto-merge. Every existing GitHub behavior, flag, and refusal is unchanged.bin/fm-teardown.sh: the landed-work check is now provider-aware. Whenpr=is recorded, the provider comes from parsing that URL. When it isn't (e.g. a yolo merge with no PR CI), the provider is guessed from the project's own origin remote and the merge request is discovered from the branch name viaglab mr list. GitLab's merge state comes fromglab mr view's plain field output (no JSON processor needed, matchingfm-pr-poll.sh's existing approach), and its head commit comes from GitLab's ownrefs/merge-requests/<n>/headgit ref rather than a JSON field. Every lookup failure or ambiguity falls back to the existing content-in-default check exactly as before; nothing here can cause teardown to assume landed when it can't determine the merge state (fail-safe preserved).bin/fm-brief.sh: the generated Rules and direct-PR definition-of-done now match the project's actual forge (glab/"merge request" vsgh-axi/"PR"), derived from the clone's own origin remote. Falls back to the GitHub wording when the remote can't be read or parsed.bin/fm-project-mode.sh,AGENTS.mdsection 3,docs/architecture.md,docs/scripts.md, anddocs/gitlab-merge-watch.mdupdated to reflect GitLab as a first-class delivery path.Known remaining gap
tasks-axi done <id> --pr <url>(used byfm-teardown.sh's backlog-refresh reminder) rejects GitLab merge-request URLs; it wants/pull/<n>. That's in the externaltasks-axinpm package, not this repo, so it's left as-is per the task instructions.Testing
tests/fm-pr-merge.test.sh: added 3 new tests covering the GitLab merge path, including a merge request whose project path sits 4 groups deep, an explicit-merge-method (--rebase) case, and a merge-failure-propagates case. Updated the "malformed URL" test to use a URL invalid under both providers (a GitLab URL is no longer inherently malformed). All 13 tests pass, including every pre-existing GitHub test unchanged.tests/fm-teardown.test.sh: added 3 new tests - a GitLabpr=recorded and merged (allow), a GitLab merge request discovered by branch name with nopr=recorded (allow, the real-world scenario from the motivating epic), and a GitLab lookup error with unlanded content (refuse, fail-safe). All 36 tests pass.tests/fm-pr-check-security.test.sh: updated the one assertion that pinned the old "refuses a GitLab URL" behavior to assert the new merge behavior instead. All 36 tests pass.tests/fm-brief.test.sh: all 17 pre-existing tests pass unchanged.bin/fm-lint.sh(shellcheck, pinned 0.11.0): clean on every touched file and on the full repo.bin/fm-doc-audience-check.sh: clean.glabCLI (installed, authenticated) that-R/--repoaccepts a full host-qualified URL for arbitrarily deep group paths, and that a baregroup/pathargument resolves againstglab's own configured default host rather than the merge request's actual host - this is whyfm-pr-merge.shalways passes the full URL.bin/fm-test-run.sh --changed --base origin/main(30 files selected as touching the affected surface) andbin/fm-test-run.sh --all(full 101-file suite) in the background; both were still in progress at the time of this PR. The only failures observed anywhere in the full-suite run belong to the unrelatedreal-herdr-gatedfamily (live Herdr workspace/session e2e tests, e.g. "primary-shaped crewmate spawn failed", "startup workspace shell did not become ready") - these touch none of the files changed here and are consistent with known flakiness running Herdr e2e tests nested inside a live session. No failures were observed in any file this PR touches.Not tested: no real merge was performed against a live GitLab instance (explicitly out of scope - never merge anything real). The
glab mr mergeinvocation shape is verified by argument-capture mocks plus manual--repo/-Rbehavior checks against the real CLI, not by an actual merge.