GitLab opens the merge request from the push, so local mode needs no token - #123
Merged
Merged
Conversation
…token
The review credential assumed a review has to be opened over an API. For GitLab
it does not: `git push -o merge_request.create` creates it over the git
transport, on the same connection the repository was cloned through.
That is not a convenience. The failure behind all of this was an access portal
answering /api/v4 with its own login page at 200 — a request that never reaches
GitLab, which no token can fix. The git transport is not intercepted, so the
push route works exactly where the API route cannot. Order is now push options,
then a token, then the host CLI; for GitLab the token is optional, and GitHub
still requires one because pull requests there are API-only.
Three things it had to get right. A remote that refuses push options rejects
the whole push and sends nothing — GitLab before 11.10, and every non-GitLab
remote — so the branch is pushed again without them and the review falls to the
next strategy. The description is one argv string against a page of markdown,
so it is trimmed and says so rather than risking a push rejected over its own
body. And push options create but do not rewrite, so a rebuild leaves an open
MR's description describing the previous run, which is why the token path stays.
The instance URL stopped being something to type. A clone knows its origin, so
the host is read from it and the field is an override for the three things a
remote cannot express: a subpath install, plain http, a non-standard API port.
Two bugs closed on the way. A blank field used to fall through to the adapter
default and send a self-managed project's path and token to gitlab.com. And
projectPathFromRemote compared the instance subpath against URL.pathname, which
has a leading slash, while its scp branch produced one without — so a
subpath-hosted instance stripped its prefix from an https remote and silently
kept it on an ssh one, which is the syntax corporate clones use.
Where a URL is genuinely ambiguous — host/ET130/services/api is either a group
on a root install or a project on a subpath one — specd asks instead of
guessing: one GET {candidate}/api/v4/version, root first, one segment in on a
404. Connect-time verification uses the same resolution, because a token
checked against the wrong host is worse than a token not checked at all.
Also: the local push now uses --force-with-lease. A build resets its branch
every run, so the plain push it used to do would have been rejected as
non-fast-forward on the second attempt.
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.
Follows the question: "if the repository is cloned locally, doesn't the user
only need an access token?" — and then "my token won't be enough for a
managed host." Both were pointing at something real.
GitLab does not need the API at all
git push -o merge_request.create -o merge_request.title=…creates the mergerequest over the git transport — the same connection the repository was
cloned through.
That is the point, not a shortcut. The failure behind this whole thread was an
access portal answering
/api/v4with its own login page at 200: a requestthat never reaches GitLab, which no token can fix. Git traffic is not
intercepted, so the push route works precisely where the API route cannot.
Order is now push options → token → host CLI. For GitLab a token is
optional; GitHub still requires one, having no equivalent.
Three things it had to get right:
nothing — GitLab before 11.10, and every non-GitLab remote. The branch is
pushed again without them and the review falls to the next strategy. Pinned
by a test against a local bare repo, which refuses them the same way.
1,500 characters and says so, rather than risking a push rejected over its
own body.
description written for the previous run — the staleness
reviewHintwarnsabout elsewhere, and why the token path is still worth having.
The instance URL stopped being something to type
A clone knows its
origin, so the host is read from it. The field is now anoverride for the three things a remote cannot express: a subpath install,
plain http, a non-standard API port.
Two bugs closed on the way, both mine from #116:
project's path and token to gitlab.com. Exactly the silent-wrong-host
class the last three PRs were about.
projectPathFromRemotecompared the instance subpath againstURL.pathname, which carries a leading slash, while its scp-syntax branchproduced one without. A subpath-hosted instance stripped its prefix from an
https://remote and silently kept it on anssh://one — the syntaxcorporate clones actually use. It had no test because both existing subpath
tests used https remotes; there are now tests for both syntaxes, plus one for
gitlab-runner/not being eaten by a/gitlabprefix match.The ambiguous URL, asked rather than guessed
https://host/ET130/services/apiis two deployments wearing one string: GitLabat the root with
ET130a group, or GitLab at/ET130withservices/apitheproject. They yield different project paths, and nothing in the remote tells
them apart.
resolveGitLabRootasks: oneGET {candidate}/api/v4/version— 401 is theexpected answer and identifies a GitLab — root first, one segment in on a 404.
Connect-time verification uses the same resolution, because a token checked
against the wrong host is worse than a token not checked at all.
Also
The local push now uses
--force-with-lease. A build resets its branch everyrun, so the plain push it used to do would have been rejected as
non-fast-forward on the second attempt — latent since #115.
Verify
pnpm typecheck && pnpm test(43 API files),pnpm build,pnpm site:check.Ten new tests: derivation across four remote shapes, the scp subpath fix in
both syntaxes, the probe's three outcomes, description truncation, and the
push-option fallback against a real bare repo.
For the reviewer
captured by matching the merge-request line in git's remote messages, and I
have no instance to confirm that output format against. It fails soft — an
unrecognised format means a pushed branch and a note saying no MR was
reported — but it is the thing most worth a live check.
resolveGitLabRoottreats 401 as "GitLab is here". True of GitLab's/api/v4/version, and the reason an unauthenticated probe suffices; a portalreturning 401 for everything would be misread as an instance. It would then
fail later with a clearer message, not silently.