From 187dd4299fe167d30e4e1742b4a7cbba0bca438c Mon Sep 17 00:00:00 2001 From: LeTuR Date: Fri, 11 Sep 2026 13:35:01 +0200 Subject: [PATCH 1/4] feat(forge): read the GitLab host list from glab instead of an unset variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GITLAB_HOST` is the documented way to tell fleet about a self-hosted GitLab and nothing exports it, so a merge request on a self-hosted instance was on no configured forge: `shepherd` never listed it, `collect` could not verify a publish there, and `reap` could never land the task — leaking its session and worktree, once per task. `forge.configured_hosts` now reads every instance out of `glab auth status`, which is where the operator's answer already lived. `GITLAB_HOST` still decides when it is set, and no glab, no config or a report it cannot parse each leave the adapter where it was. `AUTO_MERGE_REPOS` is untouched: a discovered host is watched, classified and reported, never merged. --- AGENTS.md | 17 +- scripts/fixtures/glab/README.md | 21 ++ scripts/fixtures/glab/auth-status.stderr | 21 ++ scripts/lib/forge.py | 159 ++++++++++++++- scripts/queue-selftest.sh | 249 ++++++++++++++++++++++- 5 files changed, 452 insertions(+), 15 deletions(-) create mode 100644 scripts/fixtures/glab/auth-status.stderr diff --git a/AGENTS.md b/AGENTS.md index 06ed6ed..5b183dc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,10 +51,19 @@ names every path and the reason for each. change request — a pull request on GitHub, a merge request on GitLab — it asks this module for; `scripts/lib/queue.py` runs no forge CLI itself and builds no forge URL. TWO implementations ship — GitHub through `gh`, GitLab - through `glab` — and each is a CONFIGURATION and not an assumption: which - hosts one owns comes from that CLI's own variable (`GH_HOST`, `GITLAB_HOST`), - so a self-hosted instance is the ordinary case and not a special one. The - file's own header owns the interface and how to add a third. Two things + through `glab` — and each is a CONFIGURATION and not an assumption, so a + self-hosted instance is the ordinary case and not a special one. **Which + hosts the GitLab adapter owns is READ OFF THE MACHINE**: `forge.py`'s + `configured_hosts` takes every instance `glab auth status` reports, because + that is where the operator's answer already lives and `GITLAB_HOST` is a + variable nothing exports — until this was so, a merge request on a + self-hosted instance was on no configured forge, and `reap` could never land + its task or release its session. `GITLAB_HOST` still decides when it IS set, + and the GitHub adapter still takes `GH_HOST` alone (`configured_hosts` says + why it is not asked the same question). `scripts/lib/forge.py hosts ` + prints the list for a shell caller. Discovery is never a requirement: no CLI, + no config, or a report it cannot read each leave the adapter where it was. + The file's own header owns the interface and how to add a third. Two things follow: a repository is identified by HOST plus path (`github.com/Thurbeen/fleet`), because a bare `owner/repo` names two different repositories once two forges exist; and `queue-selftest.sh` drives diff --git a/scripts/fixtures/glab/README.md b/scripts/fixtures/glab/README.md index 8675c76..042e91e 100644 --- a/scripts/fixtures/glab/README.md +++ b/scripts/fixtures/glab/README.md @@ -15,6 +15,7 @@ Every file below was recorded on **2026-09-10** with **`glab` 1.117.0 | `mr-commits.json` | `glab api "projects/gitlab-org%2Fcli/merge_requests/3875/commits?per_page=100" --hostname gitlab.com` | | `mr-view-missing.json` | `glab mr view 999999 -R https://gitlab.com/gitlab-org/cli -F json` — stdout | | `mr-view-missing.stderr` | the same call's stderr | +| `auth-status.stderr` | `glab auth status --all` — stderr, **with the hostnames and account renamed**, see below | `glab mr list -F json` answers with the same objects minus `head_pipeline`, so it is not recorded separately — the adapter reads only `iid` out of a listing @@ -37,6 +38,26 @@ its bytes in a control-plane repo: reason is the JSON on *stdout*, while stderr carries a blank-line-padded `ERROR` box whose first line is decoration. An adapter that read stderr first would report the box. +- **`auth-status.stderr` is where the GitLab host list comes from**, and it + is the one file here that was **edited after recording**. It was recorded on + **2026-09-11** with the same `glab`, from a machine logged in to one + self-hosted instance and not to gitlab.com; that instance's hostname, the + account name and the home directory were then replaced with + `gitlab.example.com`, `some-account` and `/home/user`, because this + repository is public. Nothing else was touched, so the shape is real — + which is the whole point, since `forge.configured_hosts` reads it by shape: + + - the report goes to **stderr**, not stdout, which is the opposite of `gh`; + - each instance is a **bare hostname, alone on an unindented line**, with + everything said about it indented underneath; + - the trailing `ERROR` box — its blank-padded lines, and the line of spaces + inside it — is decoration that must not read as a host, and it is here + verbatim, trailing whitespace and all, so that it is tested rather than + imagined; + - `glab` **exits non-zero** because one of the two instances has no token, + which is the ordinary state of a machine logged in to one and not the + other. An adapter that read the exit code would discover nothing on + exactly the machines this exists for. ## What is NOT here diff --git a/scripts/fixtures/glab/auth-status.stderr b/scripts/fixtures/glab/auth-status.stderr new file mode 100644 index 0000000..bb6677b --- /dev/null +++ b/scripts/fixtures/glab/auth-status.stderr @@ -0,0 +1,21 @@ +gitlab.com + x gitlab.com: API call failed: GET https://gitlab.com/api/v4/user: 401 {message: 401 Unauthorized} + ✓ Git operations for gitlab.com configured to use ssh protocol. + ✓ API calls for gitlab.com are made over https protocol. + ✓ REST API Endpoint: https://gitlab.com/api/v4/ + ✓ GraphQL Endpoint: https://gitlab.com/api/graphql/ + ! No token found (checked config file, keyring, and environment variables). +gitlab.example.com + ✓ Logged in to gitlab.example.com as some-account (/home/user/.config/glab-cli/config.yml) + ✓ Git operations for gitlab.example.com configured to use ssh protocol. + ✓ API calls for gitlab.example.com are made over https protocol. + ✓ REST API Endpoint: https://gitlab.example.com/api/v4/ + ✓ GraphQL Endpoint: https://gitlab.example.com/api/graphql/ + ✓ Token found in configuration file (plaintext): ************************** + ! To store this token more securely, run glab auth login --hostname gitlab.example.com to move it into the operating system keyring. + + ERROR + + + X could not authenticate to one or more of the configured GitLab instances. + diff --git a/scripts/lib/forge.py b/scripts/lib/forge.py index 7cc8166..5ccdaee 100644 --- a/scripts/lib/forge.py +++ b/scripts/lib/forge.py @@ -51,9 +51,13 @@ manufacture a merge. WHAT SHIPS. Two adapters: GitHub through `gh`, GitLab through `glab`. Both are -CONFIGURATION — which hosts each one owns comes from that CLI's own variable -(`GH_HOST`, `GITLAB_HOST`), because a self-hosted instance is the normal case -for everything that is not github.com or gitlab.com. +CONFIGURATION — a self-hosted instance is the normal case for everything that +is not github.com or gitlab.com, so which hosts an adapter owns is read off the +machine rather than assumed. For GitLab that is `configured_hosts` below: the +instances `glab auth status` reports, which is where the operator's answer +already lives. `GITLAB_HOST` still decides when it is set, and the GitHub +adapter still takes `GH_HOST` alone — see `configured_hosts` for why the same +discovery is not done for `gh`. ADDING A FORGE. Write a class with the methods below and register it: either in `BUILTIN` here, or — for a test, or a forge that is not fleet's business to @@ -234,6 +238,107 @@ def change_url(url) -> str: return m.group(1) if m else "" +# --- which hosts a CLI is configured for ------------------------------------- + + +# How both `gh auth status` and `glab auth status` head each instance they are +# configured for: the bare hostname, alone on an unindented line, with +# everything they have to say about it indented underneath. A `:port` is +# allowed because a self-hosted instance on one is ordinary and `RepoId` +# carries the port as part of the host. A dot is REQUIRED, for the same reason +# `RepoId.parse` requires one: it is what tells a hostname from a decoration +# line, and it is the shape every host fleet can be handed as part of a URL. +AUTH_HOST_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9.-]*\.[A-Za-z0-9-]+(?::\d+)?$") + +_CONFIGURED: dict = {} + + +def configured_hosts(cli: str, timeout: int = 10) -> list: + """Every instance `cli` is authenticated or configured for, asked of `cli`. + + WHY THIS EXISTS. `GH_HOST` and `GITLAB_HOST` are the documented way to tell + fleet about a self-hosted instance, and in practice nothing exports them: + an operator logs their CLI in once and never thinks about it again. The + result was that a whole forge was invisible — `shepherd` never saw a merge + request on a self-hosted instance, `collect` could not verify a publish + there, and `reap` could never land the task, so its session and worktree + leaked with no upper bound. The operator's answer was already on the + machine; nothing was reading it. + + IT NEEDS NO NETWORK. `auth status` prints one heading per configured + instance out of the CLI's own config and then decorates each with an API + call, so the headings are there whether or not the call succeeds — measured + on 2026-09-11 against `glab` 1.117.0 with every request refused, which + printed both instances in 0.2s. The exit code is ignored for the same + reason: `glab` exits non-zero when ANY one instance fails to authenticate, + which is the ordinary state of a machine logged in to one instance and not + the other. + + BOTH STREAMS ARE READ, because they disagree: `glab` writes the whole + report to stderr and `gh` writes it to stdout. + + IT NEVER FAILS. No CLI, no config, a report it cannot parse, or a CLI too + old for `--all` each answer with an empty list, which leaves every caller + exactly where it was before discovery existed. Discovery is an improvement + on a default, never a dependency: `collect` has to keep working with the + network down and on a machine that has neither CLI. + + Cached for the process. The answer does not change inside a run, and the + registry would otherwise ask again for every adapter built. + + WHY THE GITHUB ADAPTER DOES NOT USE THIS, though `gh auth status` prints + the same shape and GitHub Enterprise is the same problem. §13 of + `queue-selftest.sh` drives the whole queue through a forge that is not + GitHub with `gh` on PATH as a TRIPWIRE — it fails on any invocation at all, + so code reaching around this seam shows up there by name. Building the + GitHub adapter would run `gh auth status` and trip it, and the honest + choice between "discover GitHub Enterprise" and "keep the regression test + that keeps this seam honest" is the second one: `GH_HOST` was never the + half that was broken. This function takes the CLI by name so that + everything else — `scripts/preflight.sh` reporting auth per host, say — + can ask it about `gh` too, and so that the day that tripwire can tell a + configuration read from a change-request call, the adapter needs one line. + """ + cli = str(cli or "").strip() + if cli in _CONFIGURED: + return list(_CONFIGURED[cli]) + hosts: list = [] + if cli and shutil.which(cli): + # `--all` is the documented way to ask about every instance rather than + # the one the current directory implies. A CLI too old to know the flag + # refuses the whole command, so the bare form is tried after it — which + # on a machine with no git context answers the same thing. A call that + # never answered at all stops the sequence rather than being retried: + # the second ask would hang exactly as long as the first, and paying + # the timeout twice is how discovery would start costing `collect` + # real time on the flaky network it is supposed to survive. + for argv in ([cli, "auth", "status", "--all"], [cli, "auth", "status"]): + said = _auth_status_hosts(argv, timeout) + if said is None: + break + if said: + hosts = said + break + _CONFIGURED[cli] = hosts + return list(hosts) + + +def _auth_status_hosts(argv: list, timeout: int): + """The hostnames `argv` printed, or None if it never answered at all.""" + try: + out = subprocess.run(argv, capture_output=True, text=True, timeout=timeout) + except (OSError, subprocess.SubprocessError): + return None + seen: dict = {} + for line in ((out.stdout or "") + "\n" + (out.stderr or "")).splitlines(): + if not line[:1].strip(): # every heading is unindented; the rest is not + continue + host = line.strip().lower() + if AUTH_HOST_RE.match(host): + seen[host] = True + return list(seen) + + # --- the interface ----------------------------------------------------------- @@ -664,11 +769,13 @@ class GitLabForge(Forge): whatever credential the operator gave this machine, and a fleet that needed its own token would need one per machine a worker runs on. - WHICH HOSTS ARE GITLAB. `gitlab.com`, plus `GITLAB_HOST` — glab's own - variable for a self-hosted instance, the way `GH_HOST` is gh's. A - self-hosted instance is the normal case here, so every call names its - repository by FULL URL (`-R https://host/group/project`) rather than by - slug: that is what makes `gitlab.example.com/group/proj` reach + WHICH HOSTS ARE GITLAB. `gitlab.com`, plus whichever instances this + machine's `glab` is configured for — `configured_hosts` above reads them + out of `glab auth status`. `GITLAB_HOST`, glab's own variable for one + chosen instance, overrides that entirely when it is set, the way `GH_HOST` + does for gh. A self-hosted instance is the normal case here, so every call + names its repository by FULL URL (`-R https://host/group/project`) rather + than by slug: that is what makes `gitlab.example.com/group/proj` reach gitlab.example.com and not gitlab.com. WHAT IT COSTS. GitLab does not put a merge request's pipeline in the list @@ -687,7 +794,15 @@ class GitLabForge(Forge): def __init__(self, hosts=None): extra = [self._host(h) for h in (hosts or [])] - extra.append(self._host(os.environ.get("GITLAB_HOST", ""))) + override = self._host(os.environ.get("GITLAB_HOST", "")) + # `GITLAB_HOST` DECIDES when it is set, and discovery does not run at + # all then: it is glab's own variable, an operator who exported it + # pointed fleet at that instance deliberately, and glab itself obeys it + # over its config. Unset, this used to mean "gitlab.com and nothing + # else", which made a self-hosted instance invisible on the very + # machines whose `glab` was logged in to one — so ask glab which + # instances it holds instead of waiting for a variable nothing sets. + extra.extend([override] if override else configured_hosts("glab")) self.hosts = tuple(dict.fromkeys( ["gitlab.com", "www.gitlab.com"] + [h for h in extra if h] )) @@ -1158,9 +1273,15 @@ def forges() -> list: def reset() -> None: - """Forget the cached registry. For tests inside one process.""" + """Forget the cached registry and the discovered host lists. + + For tests inside one process — and the host cache goes with the registry + because the adapters were built FROM it, so a test that changes what a CLI + would answer and then rebuilds the registry must get the new answer. + """ global _REGISTRY _REGISTRY = None + _CONFIGURED.clear() def _load_plugin(path: str) -> list: @@ -1260,3 +1381,21 @@ def _git_remote(path: str) -> str: except (OSError, subprocess.SubprocessError): return "" return out.stdout.strip() if out.returncode == 0 else "" + + +# --- asking from a shell ----------------------------------------------------- + + +if __name__ == "__main__": + # `python3 scripts/lib/forge.py hosts glab` — the one thing in this module + # a shell script needs, since `configured_hosts` answers a question + # (`scripts/preflight.sh`'s "which instances should I report auth for") + # that is not itself about a change request. One host per line, nothing on + # a machine that has no such CLI, and always exit 0: a CLI that is not + # installed is an answer, not an error. + if len(sys.argv) == 3 and sys.argv[1] == "hosts": + for _host in configured_hosts(sys.argv[2]): + print(_host) + else: + print("usage: forge.py hosts ", file=sys.stderr) + sys.exit(2) diff --git a/scripts/queue-selftest.sh b/scripts/queue-selftest.sh index 169c512..fd56b84 100755 --- a/scripts/queue-selftest.sh +++ b/scripts/queue-selftest.sh @@ -202,6 +202,33 @@ export FLEET_QUEUE_DIR="$tmp/queue" # of this file would scaffold logs into the operator's own orchestration/runs/. export FLEET_RUNS_DIR="$tmp/runs" +# --- `glab`, a TRIPWIRE on PATH for the whole run ---------------------------- +# +# The GitLab adapter asks `glab auth status` which instances this machine +# holds, and it asks the moment the forge registry is built — which is to say +# in nearly every section below, whether or not that section is about GitLab. +# On the operator's own laptop the REAL `glab` would answer, so the verdicts +# below would depend on who ran the file and on a network being there. This +# stub fails the way a machine with no GitLab configuration fails, which is +# exactly the machine every section except 14 is written for. +# +# It goes in front of `base_path`, so every section that builds its own PATH +# out of it inherits the tripwire — section 13 included, where reaching a real +# `glab` would be the same defect as reaching a real `gh`. Section 14 puts a +# `glab` of its own in front of this one. +noglab="$tmp/no-glab" +mkdir -p "$noglab" +cat >"$noglab/glab" <<'SH' +#!/bin/sh +echo "glab $*" >>"$NOGLAB_LOG" +echo "glab: no GitLab instance is configured on this machine" >&2 +exit 1 +SH +chmod +x "$noglab/glab" +export NOGLAB_LOG="$tmp/noglab-calls.log" +: >"$NOGLAB_LOG" +export PATH="$noglab:$PATH" + # Captured here, before test 7's subshell exports its own PATH: reading $PATH # after that point is what SC2031 is about, and the stubbed sections below # want the PATH this script started with, not whatever a subshell left. @@ -4693,6 +4720,18 @@ if argv[:2] == ["mr", "merge"]: sys.stdout.write("Merged!\n") raise SystemExit(0) +if argv[:2] == ["auth", "status"]: + # Where the host list comes from. The recording is replayed only when this + # section has put one in the store, so 14a keeps asking the adapter what a + # machine with NO GitLab configuration owns — and the refusal below is + # what that machine looks like. The exit code is 1 either way, which is + # what `glab` does whenever any one instance has no token. + served = os.path.join(D, "auth-status.stderr") + if not os.path.exists(served): + refuse("no GitLab instance is configured") + sys.stderr.write(open(served).read()) + raise SystemExit(1) + if argv[:1] == ["api"]: path = argv[1].split("?")[0] if path.endswith("/commits"): @@ -4740,8 +4779,10 @@ printf '[{"id": 7, "username": "letur", "access_level": 40}]\n' >"$gl/api/member # Every merge request below is DERIVED FROM THE RECORDED ONE: the recorded # object is loaded and named fields are overwritten, so each fixture keeps the # real shape and only the facts under test are this test's invention. +# `GLAB_MR_DIR` aims it at a second store, which 14e wants so that its queue +# sees its own merge requests and none of this section's. glab_mr() { - python3 - "$fixtures/mr-view.json" "$gl/mrs" "$@" <<'PY' + python3 - "$fixtures/mr-view.json" "${GLAB_MR_DIR:-$gl/mrs}" "$@" <<'PY' import json import sys @@ -5068,6 +5109,212 @@ expect "a repo whose origin cannot be read says THAT, not 'no credentials'" \ "$(probe_says '' 'Welcome to GitLab, @letur!' 2>&1)" +# --- 14e. which hosts are GitLab: DISCOVERED, not waited for ----------------- +# +# Everything above this line sets `GITLAB_HOST`, and that is what hid the +# defect this section is about: nothing on a real machine exports it. With it +# unset, a merge request on a self-hosted instance was on no configured forge, +# so `shepherd` never listed it, `collect` could not verify a publish there, +# and `reap` could never move the task to `landed` — which means its session +# and its worktree were never released, once per task, with no upper bound. +# +# The answer was already on the machine: `glab auth status` prints every +# instance the operator logged their CLI in to. `forge.configured_hosts` reads +# it, and `scripts/fixtures/glab/auth-status.stderr` is that report, recorded +# and then stripped of the operator's own names — its README says so. +# +# What this proves, all of it offline, with `gh` still a tripwire: +# +# a self-hosted instance glab holds is OURS with no GITLAB_HOST set, and the +# whole loop reaches it — collect verifies a publish, reap lands the task, +# shepherd lists what is still open +# `GITLAB_HOST` still DECIDES when it is set: it is the answer, and an +# instance glab holds is not ours while it names another +# no glab, a configuration it cannot read, and a glab too old for `--all` +# each leave the adapter exactly where it was — gitlab.com and nothing else +# AUTO_MERGE_REPOS is untouched by discovery: a green, attested, mergeable +# merge request on a discovered host is REPORTED and never merged + +gl2="$tmp/gitlab-discovered" +mkdir -p "$gl2/mrs" "$gl2/api" +cp "$gl/api/project.json" "$gl/api/members.json" "$gl2/api/" +cp "$fixtures/auth-status.stderr" "$gl2/auth-status.stderr" +: >"$gl2/calls.log" +: >"$gl2/merged.log" +: >"$gl2/gh-calls.log" + +env PATH="$GLPATH" FAKE_GLAB_DIR="$gl2" python3 - "$PWD/scripts/lib" "$gl2" \ + >"$tmp/gl-hosts.tsv" <<'PYHOSTS' +import os +import sys + +sys.path.insert(0, sys.argv[1]) +store = sys.argv[2] +import forge # noqa: E402 + +rows = [] + + +def claim(name, got, want): + rows.append(("PASS", name, "") if got == want + else ("FAIL", name, "wanted %r, got %r" % (want, got))) + + +real_path = os.environ["PATH"] + + +def adapter(gitlab_host=None, path=None): + """A fresh adapter on a machine described by `gitlab_host` and `path`. + + `forge.reset()` forgets the discovered host list along with the registry, + which is the whole reason it has to: every case below is a different + machine answering a different way, inside one process. + """ + forge.reset() + os.environ.pop("GITLAB_HOST", None) + if gitlab_host: + os.environ["GITLAB_HOST"] = gitlab_host + os.environ["PATH"] = real_path if path is None else path + return forge.GitLabForge() + + +def cli_dir(name, script): + """A PATH holding one `glab` that behaves as `script` says — or none. + + `/usr/bin` and `/bin` come after it so the stub has the ordinary tools, + and NOT the rest of this file's PATH, which carries two other `glab`s. + """ + path = os.path.join(store, name) + os.makedirs(path, exist_ok=True) + glab = os.path.join(path, "glab") + if not script: + if os.path.exists(glab): + os.remove(glab) + return path + with open(glab, "w") as fh: + fh.write(script) + os.chmod(glab, 0o755) + return os.pathsep.join([path, "/usr/bin", "/bin"]) + + +# --- the recording, read the way a machine's own `glab` would be read --- +found = adapter() +claim("both instances in the recording are read out of it", + forge.configured_hosts("glab"), ["gitlab.com", "gitlab.example.com"]) +claim("so a self-hosted instance is ours with no GITLAB_HOST set", + found.owns_host("gitlab.example.com"), True) +claim("and its merge request is a change request fleet can be asked about", + found.parse_change_url( + "https://gitlab.example.com/acme/group/widgets/-/merge_requests/301" + ) is not None, True) +claim("an instance nothing on this machine holds is still not ours", + found.owns_host("gitlab.nowhere.example"), False) + +# --- GITLAB_HOST still decides --- +named = adapter(gitlab_host="https://gitlab.other.example/") +claim("GITLAB_HOST set is still the answer, scheme and all", + named.owns_host("gitlab.other.example"), True) +claim("and it DECIDES: an instance glab holds is not ours while it is set", + named.owns_host("gitlab.example.com"), False) + +# --- and discovery is never a requirement --- +claim("a machine with no glab at all is exactly where it was", + adapter(path=cli_dir("no-cli", "")).hosts, ("gitlab.com", "www.gitlab.com")) +claim("nor does a configuration glab cannot read move it", adapter(path=cli_dir( + "unreadable", + "#!/bin/sh\n" + "echo 'failed to parse config.yml: yaml: line 3: could not find expected key' >&2\n" + "exit 1\n", +)).hosts, ("gitlab.com", "www.gitlab.com")) +claim("nor a glab that answers something that is not a host list", adapter(path=cli_dir( + "garbled", + "#!/bin/sh\nprintf 'Logged in somewhere, probably\\n'\nexit 0\n", +)).hosts, ("gitlab.com", "www.gitlab.com")) + +# A `glab` too old for `--all` refuses the whole command, so the bare form is +# what has to answer — and on a machine with no git context it answers the +# same thing. Without that second try an older CLI discovers nothing. +claim("a glab too old for --all is asked again without it", adapter(path=cli_dir( + "old", + "#!/bin/sh\n" + 'case "$*" in *--all*) echo "unknown flag: --all" >&2; exit 1 ;; esac\n' + 'cat "$FAKE_GLAB_DIR/auth-status.stderr" >&2\nexit 1\n', +)).owns_host("gitlab.example.com"), True) + +for verdict, name, detail in rows: + print("%s\t%s\t%s" % (verdict, name, detail)) +PYHOSTS + +while IFS=$'\t' read -r verdict claim detail; do + if [ "$verdict" = PASS ]; then pass "$claim"; else fail "$claim" "$detail"; fi +done <"$tmp/gl-hosts.tsv" + +# --- the whole loop, on a discovered host, with no GITLAB_HOST anywhere ------ + +gdq() { + env -u GITLAB_HOST PATH="$gl/bin:$tbxbin:$sshbin:$base_path" \ + FAKE_GLAB_DIR="$gl2" FLEET_QUEUE_DIR="$tmp/queue-discovered" \ + "$QUEUE" "$@" +} + +gdtopic="$(gdq topic add discovered --title 'Work on an instance glab already holds' \ + --prompt 'no GITLAB_HOST is exported anywhere')" +gdq add "$gdtopic" shipped --title 'A change already shipped' --repo "$glrepo" \ + --branch fix/discovered --number 01 >/dev/null +cat >"$tmp/queue-discovered/$gdtopic/01-shipped/result.md" <<'EOF' +--- +outcome: shipped +artifact: https://gitlab.example.com/acme/group/widgets/-/merge_requests/306 +--- +Shipped it. +EOF + +GLAB_MR_DIR="$gl2/mrs" glab_mr 306 'source_branch="fix/discovered"' + +session_is cccccccc-0000-0000-0000-000000000001 idle +gdq attach "$gdtopic/01-shipped" cccccccc-0000-0000-0000-000000000001 >/dev/null + +out="$(gdq collect 2>&1)" +expect "collect verifies a publish on an instance only glab knew about" \ + "01-shipped" "$out" +expect "and it read the merge request URL as a change request" \ + "merge_requests/306" "$out" +refute "which is not the same as landing it" "reaped" "$out" + +# The merge set is host-qualified and discovery adds nothing to it. 306 is +# green, attested, mergeable and opened by someone who can push — every gate +# the shepherd has — and on a discovered host it is still only REPORTED. +before="$(wc -l <"$gl2/merged.log")" +out="$(gdq shepherd 2>&1)" +expect "shepherd lists what is open on the discovered instance" \ + "acme/group/widgets on gitlab.example.com" "$out" +expect "and a mergeable, attested one there is handed back, not merged" \ + "fleet does not merge in acme/group/widgets on gitlab.example.com" "$out" +expect "because the merge set is exactly the three repos it always was" \ + "github.com/Thurbeen/fleet, github.com/Thurbeen/thurbox, github.com/Thurbeen/thurview" \ + "$out" +count_is "so nothing on a discovered host was merged" "$(wc -l <"$gl2/merged.log")" \ + "$before" "$out" + +python3 - "$gl2/mrs/306.json" <<'PYMERGED' +import json +import sys +doc = json.load(open(sys.argv[1])) +doc["state"] = "merged" +json.dump(doc, open(sys.argv[1], "w")) +PYMERGED +out="$(gdq reap 2>&1)" +expect "reap lands a task on a discovered host" "landed" "$out" +expect "and releases the session that used to leak with it" "reaped" "$out" + +if [ -s "$gl2/gh-calls.log" ]; then + fail "no code path ran \`gh\` while the host came from glab" \ + "$(cat "$gl2/gh-calls.log")" +else + pass "no code path ran \`gh\` while the host came from glab" +fi + + # --- 20. a title `dispatch` cannot spawn, and a spawn failure that says why -- # # Both halves of one run on 2026-09-11. `add` took the title `Rust crate, From 8e781281d1e51c0aef9f0abd1e28940d9efd42cd Mon Sep 17 00:00:00 2001 From: Arthur Cesare-Herriau Date: Fri, 11 Sep 2026 13:57:50 +0200 Subject: [PATCH 2/4] =?UTF-8?q?chore:=20no-mistakes=20review=20-=20tighten?= =?UTF-8?q?=20=C2=A714e=20assertions,=20drop=20unused=20host=20cache=20and?= =?UTF-8?q?=20stub=20log?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 10 +++++----- scripts/fixtures/glab/README.md | 7 +++++-- scripts/lib/forge.py | 18 ++--------------- scripts/queue-selftest.sh | 35 ++++++++++++++++++--------------- 4 files changed, 31 insertions(+), 39 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5b183dc..9bab021 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,11 +60,11 @@ names every path and the reason for each. self-hosted instance was on no configured forge, and `reap` could never land its task or release its session. `GITLAB_HOST` still decides when it IS set, and the GitHub adapter still takes `GH_HOST` alone (`configured_hosts` says - why it is not asked the same question). `scripts/lib/forge.py hosts ` - prints the list for a shell caller. Discovery is never a requirement: no CLI, - no config, or a report it cannot read each leave the adapter where it was. - The file's own header owns the interface and how to add a third. Two things - follow: a repository is identified by HOST plus path + why it is not asked the same question). `python3 scripts/lib/forge.py hosts + ` prints the list for a shell caller. Discovery is never a requirement: + no CLI, no config, or a report it cannot read each leave the adapter where + it was. The file's own header owns the interface and how to add a third. Two + things follow: a repository is identified by HOST plus path (`github.com/Thurbeen/fleet`), because a bare `owner/repo` names two different repositories once two forges exist; and `queue-selftest.sh` drives `collect`, the landing check and `shepherd` through a second forge with no diff --git a/scripts/fixtures/glab/README.md b/scripts/fixtures/glab/README.md index 042e91e..d5d5331 100644 --- a/scripts/fixtures/glab/README.md +++ b/scripts/fixtures/glab/README.md @@ -5,9 +5,12 @@ the adapter's parsing is tested against **what `glab` actually prints** rather than against what this repo assumed it prints. Nothing here reaches the network when the selftest runs; a fake `glab` on `PATH` replays these files. -Every file below was recorded on **2026-09-10** with **`glab` 1.117.0 +Every `mr-*` file below was recorded on **2026-09-10** with **`glab` 1.117.0 (44790937b)** against **gitlab.com**, unauthenticated, from the public project -`gitlab-org/cli`. The commands are exact and repeatable. +`gitlab-org/cli`; those commands are exact and repeatable. `auth-status.stderr` +is the one exception on every count — it prints whatever the running machine is +configured for, and it was recorded elsewhere and edited afterwards. Its own +bullet below owns its provenance. | File | Command | | --- | --- | diff --git a/scripts/lib/forge.py b/scripts/lib/forge.py index 5ccdaee..8ac1ae6 100644 --- a/scripts/lib/forge.py +++ b/scripts/lib/forge.py @@ -250,8 +250,6 @@ def change_url(url) -> str: # line, and it is the shape every host fleet can be handed as part of a URL. AUTH_HOST_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9.-]*\.[A-Za-z0-9-]+(?::\d+)?$") -_CONFIGURED: dict = {} - def configured_hosts(cli: str, timeout: int = 10) -> list: """Every instance `cli` is authenticated or configured for, asked of `cli`. @@ -283,9 +281,6 @@ def configured_hosts(cli: str, timeout: int = 10) -> list: on a default, never a dependency: `collect` has to keep working with the network down and on a machine that has neither CLI. - Cached for the process. The answer does not change inside a run, and the - registry would otherwise ask again for every adapter built. - WHY THE GITHUB ADAPTER DOES NOT USE THIS, though `gh auth status` prints the same shape and GitHub Enterprise is the same problem. §13 of `queue-selftest.sh` drives the whole queue through a forge that is not @@ -300,8 +295,6 @@ def configured_hosts(cli: str, timeout: int = 10) -> list: configuration read from a change-request call, the adapter needs one line. """ cli = str(cli or "").strip() - if cli in _CONFIGURED: - return list(_CONFIGURED[cli]) hosts: list = [] if cli and shutil.which(cli): # `--all` is the documented way to ask about every instance rather than @@ -319,8 +312,7 @@ def configured_hosts(cli: str, timeout: int = 10) -> list: if said: hosts = said break - _CONFIGURED[cli] = hosts - return list(hosts) + return hosts def _auth_status_hosts(argv: list, timeout: int): @@ -1273,15 +1265,9 @@ def forges() -> list: def reset() -> None: - """Forget the cached registry and the discovered host lists. - - For tests inside one process — and the host cache goes with the registry - because the adapters were built FROM it, so a test that changes what a CLI - would answer and then rebuilds the registry must get the new answer. - """ + """Forget the cached registry. For tests inside one process.""" global _REGISTRY _REGISTRY = None - _CONFIGURED.clear() def _load_plugin(path: str) -> list: diff --git a/scripts/queue-selftest.sh b/scripts/queue-selftest.sh index fd56b84..ac835fa 100755 --- a/scripts/queue-selftest.sh +++ b/scripts/queue-selftest.sh @@ -202,31 +202,29 @@ export FLEET_QUEUE_DIR="$tmp/queue" # of this file would scaffold logs into the operator's own orchestration/runs/. export FLEET_RUNS_DIR="$tmp/runs" -# --- `glab`, a TRIPWIRE on PATH for the whole run ---------------------------- +# --- `glab`, a STAND-IN on PATH for the whole run ---------------------------- # # The GitLab adapter asks `glab auth status` which instances this machine # holds, and it asks the moment the forge registry is built — which is to say # in nearly every section below, whether or not that section is about GitLab. # On the operator's own laptop the REAL `glab` would answer, so the verdicts # below would depend on who ran the file and on a network being there. This -# stub fails the way a machine with no GitLab configuration fails, which is -# exactly the machine every section except 14 is written for. +# stub is a hermetic stand-in for a machine with no GitLab configuration at +# all, which is exactly the machine every section except 14 is written for. # -# It goes in front of `base_path`, so every section that builds its own PATH -# out of it inherits the tripwire — section 13 included, where reaching a real -# `glab` would be the same defect as reaching a real `gh`. Section 14 puts a -# `glab` of its own in front of this one. +# It is NOT a tripwire, and cannot be one: unlike `gh`, `glab` is now something +# the adapter legitimately invokes in every section. It goes in front of +# `base_path`, so every section that builds its own PATH out of it inherits it +# — section 13 included, where reaching a real `glab` would be the same defect +# as reaching a real `gh`. Section 14 puts a `glab` of its own in front of it. noglab="$tmp/no-glab" mkdir -p "$noglab" cat >"$noglab/glab" <<'SH' #!/bin/sh -echo "glab $*" >>"$NOGLAB_LOG" echo "glab: no GitLab instance is configured on this machine" >&2 exit 1 SH chmod +x "$noglab/glab" -export NOGLAB_LOG="$tmp/noglab-calls.log" -: >"$NOGLAB_LOG" export PATH="$noglab:$PATH" # Captured here, before test 7's subshell exports its own PATH: reading $PATH @@ -5166,9 +5164,9 @@ real_path = os.environ["PATH"] def adapter(gitlab_host=None, path=None): """A fresh adapter on a machine described by `gitlab_host` and `path`. - `forge.reset()` forgets the discovered host list along with the registry, - which is the whole reason it has to: every case below is a different - machine answering a different way, inside one process. + Every case below is a different machine answering a different way, inside + one process: discovery is not cached, so each adapter asks the `glab` its + own PATH holds and gets that machine's answer. """ forge.reset() os.environ.pop("GITLAB_HOST", None) @@ -5274,11 +5272,16 @@ GLAB_MR_DIR="$gl2/mrs" glab_mr 306 'source_branch="fix/discovered"' session_is cccccccc-0000-0000-0000-000000000001 idle gdq attach "$gdtopic/01-shipped" cccccccc-0000-0000-0000-000000000001 >/dev/null +# `publish verified` is printed only when the publish check PASSED, which +# needs a forge that owns the host. Undiscovered, the task is still concluded +# and its URL still printed — it is the verdict that degrades to +# `publish unchecked: ... no configured forge owns gitlab.example.com`, which +# is the pre-fix symptom itself. So the marker is what this asserts. out="$(gdq collect 2>&1)" expect "collect verifies a publish on an instance only glab knew about" \ - "01-shipped" "$out" -expect "and it read the merge request URL as a change request" \ - "merge_requests/306" "$out" + "publish verified" "$out" +refute "and does not report it as on no configured forge" \ + "publish unchecked" "$out" refute "which is not the same as landing it" "reaped" "$out" # The merge set is host-qualified and discovery adds nothing to it. 306 is From b4ee79820e034c4ba8b078a98b49a910e9a3c97a Mon Sep 17 00:00:00 2001 From: Arthur Cesare-Herriau Date: Fri, 11 Sep 2026 18:24:23 +0200 Subject: [PATCH 3/4] chore: no-mistakes review - correct configured_hosts docstring on the --all fallback --- scripts/lib/forge.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/lib/forge.py b/scripts/lib/forge.py index 8ac1ae6..8f6d681 100644 --- a/scripts/lib/forge.py +++ b/scripts/lib/forge.py @@ -275,11 +275,11 @@ def configured_hosts(cli: str, timeout: int = 10) -> list: BOTH STREAMS ARE READ, because they disagree: `glab` writes the whole report to stderr and `gh` writes it to stdout. - IT NEVER FAILS. No CLI, no config, a report it cannot parse, or a CLI too - old for `--all` each answer with an empty list, which leaves every caller - exactly where it was before discovery existed. Discovery is an improvement - on a default, never a dependency: `collect` has to keep working with the - network down and on a machine that has neither CLI. + IT NEVER FAILS. No CLI, no config, or a report it cannot parse each answer + with an empty list, which leaves every caller exactly where it was before + discovery existed. Discovery is an improvement on a default, never a + dependency: `collect` has to keep working with the network down and on a + machine that has neither CLI. WHY THE GITHUB ADAPTER DOES NOT USE THIS, though `gh auth status` prints the same shape and GitHub Enterprise is the same problem. §13 of From 7f72fdc30df8c6c436019028ce368be3d4dc0451 Mon Sep 17 00:00:00 2001 From: Arthur Cesare-Herriau Date: Fri, 11 Sep 2026 19:07:22 +0200 Subject: [PATCH 4/4] chore: no-mistakes document - docs: point AGENTS.md at forge's owner, fix stale fixture and section prose --- AGENTS.md | 13 +++++-------- README.md | 4 +++- scripts/fixtures/glab/README.md | 13 +++++++++---- scripts/queue-selftest.sh | 20 ++++++++++++-------- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9bab021..8dbd76d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,14 +56,11 @@ names every path and the reason for each. hosts the GitLab adapter owns is READ OFF THE MACHINE**: `forge.py`'s `configured_hosts` takes every instance `glab auth status` reports, because that is where the operator's answer already lives and `GITLAB_HOST` is a - variable nothing exports — until this was so, a merge request on a - self-hosted instance was on no configured forge, and `reap` could never land - its task or release its session. `GITLAB_HOST` still decides when it IS set, - and the GitHub adapter still takes `GH_HOST` alone (`configured_hosts` says - why it is not asked the same question). `python3 scripts/lib/forge.py hosts - ` prints the list for a shell caller. Discovery is never a requirement: - no CLI, no config, or a report it cannot read each leave the adapter where - it was. The file's own header owns the interface and how to add a third. Two + variable nothing exports. `GITLAB_HOST` still decides when it IS set, the + GitHub adapter still takes `GH_HOST` alone, and discovery never becomes a + requirement — `configured_hosts`' own docstring owns those three and why. + `python3 scripts/lib/forge.py hosts ` prints the list for a shell + caller. The file's own header owns the interface and how to add a third. Two things follow: a repository is identified by HOST plus path (`github.com/Thurbeen/fleet`), because a bare `owner/repo` names two different repositories once two forges exist; and `queue-selftest.sh` drives diff --git a/README.md b/README.md index b35d587..f6ee125 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,9 @@ What it needs, and what it will tell you itself: `git`, `gh` (authenticated), `jq`, `python3` with PyYAML and `thurbox-cli` **2.19.0 or newer** are required; `quota-axi` and `glab` are recommended, and each names what degrades without it. `gh` is not optional even on a GitLab-only -fleet — it is what builds the repo map. +fleet — it is what builds the repo map. A `glab` that is logged in is also the +configuration: fleet asks it which GitLab instances this machine holds, so a +self-hosted one needs no variable exported for it. That done, open the Mission Control session in thurbox and give it a goal. diff --git a/scripts/fixtures/glab/README.md b/scripts/fixtures/glab/README.md index d5d5331..597bcde 100644 --- a/scripts/fixtures/glab/README.md +++ b/scripts/fixtures/glab/README.md @@ -76,7 +76,12 @@ presented as recorded: ## Refreshing -Re-run the commands above. `3877` and `3875` are ordinary merge requests on a -public project and will eventually be closed; when they are, pick any open -merge request with a fork source and any with several commits, and update the -numbers in this table. +Re-run the `mr-*` commands above. `3877` and `3875` are ordinary merge +requests on a public project and will eventually be closed; when they are, +pick any open merge request with a fork source and any with several commits, +and update the numbers in this table. + +`auth-status.stderr` is not refreshed that way: re-running `glab auth status +--all` prints the hostnames, account and home directory of whoever ran it, and +this repository is public. Replace them again, as the bullet above says, or +leave the file alone — it is a shape, not a live answer. diff --git a/scripts/queue-selftest.sh b/scripts/queue-selftest.sh index ac835fa..dbf3c2f 100755 --- a/scripts/queue-selftest.sh +++ b/scripts/queue-selftest.sh @@ -4609,12 +4609,12 @@ fi # answers what GitLab decided to answer, in GitLab's own words and shapes. # # So the fixtures matter more than the code here. `scripts/fixtures/glab/` is -# real `glab` 1.117.0 output recorded from gitlab.com — its README says which -# command produced each file and which two answers are behind authentication -# and therefore CONSTRUCTED below rather than recorded. A fake `glab` on PATH -# replays them; nothing in this section reaches a network, and `gh` is a -# tripwire again, because a GitLab merge request is the one thing that must -# never be asked about with `gh`. +# real `glab` 1.117.0 output — its README says which command produced each +# file, where it was recorded and what was edited out of it, and which two +# answers are behind authentication and therefore CONSTRUCTED below rather +# than recorded. A fake `glab` on PATH replays them; nothing in this section +# reaches a network, and `gh` is a tripwire again, because a GitLab merge +# request is the one thing that must never be asked about with `gh`. # # What it proves: # @@ -4627,6 +4627,8 @@ fi # `squash_option: never` is a refusal fleet RECORDS, not a crash and not a # merge by some other method # the remote-host credential probe asks the repository's own forge +# WHICH hosts are GitLab is read off the machine rather than waited for, +# and the whole loop runs on an instance discovered that way gl="$tmp/gitlab" mkdir -p "$gl/mrs" "$gl/api" "$gl/bin" @@ -5128,8 +5130,10 @@ expect "a repo whose origin cannot be read says THAT, not 'no credentials'" \ # shepherd lists what is still open # `GITLAB_HOST` still DECIDES when it is set: it is the answer, and an # instance glab holds is not ours while it names another -# no glab, a configuration it cannot read, and a glab too old for `--all` -# each leave the adapter exactly where it was — gitlab.com and nothing else +# no glab, and a configuration or a report it cannot read, each leave the +# adapter exactly where it was — gitlab.com and nothing else +# a glab too old for `--all` is asked again without the flag, so an older +# CLI still discovers the instances it holds # AUTO_MERGE_REPOS is untouched by discovery: a green, attested, mergeable # merge request on a discovered host is REPORTED and never merged