From ecef633032c3a24bc5f587c7cc6becc776367359 Mon Sep 17 00:00:00 2001 From: Mike Langmayr Date: Thu, 15 Oct 2026 11:20:35 -0700 Subject: [PATCH 1/2] Hand the venv and checkout to the ops group --- README.md | 7 ++++ src/instrumentctl/installer.py | 47 ++++++++++++++++++++-- tests/test_ownership.py | 71 ++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 tests/test_ownership.py diff --git a/README.md b/README.md index d76ff86..c263835 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,13 @@ hands over: exec "$VENV_DIR/bin/" install "$@" ``` +It also hands the virtualenv and the checkout to the daemon account with the +ops group writable, so an operator can pull, hotfix and import a driver to +test it without sudo. That grants nothing new: they already decide what the +daemons run, through the configs they edit and the restarts they trigger. git +is told separately, since it refuses a repository whose owner is not the +caller no matter the permissions. + Everything else needs no root. `doctor` compares the installed unit and polkit rule against what `install` would write now, so a host whose unit predates a template change is reported rather than silently stale. diff --git a/src/instrumentctl/installer.py b/src/instrumentctl/installer.py index 758d2db..e8d5495 100644 --- a/src/instrumentctl/installer.py +++ b/src/instrumentctl/installer.py @@ -16,6 +16,7 @@ import os import pwd import shutil +import stat import subprocess import tempfile from importlib import resources @@ -181,6 +182,44 @@ def _sudoers(product: Product, paths: Paths) -> None: shutil.chown(product.sudoers, "root", "root") +def _hand_to_operators(tree: Path, product: Product) -> None: + """Give a tree to the daemon account, writable by the ops group. + + Operators already decide what the daemons run, through the configs they + edit and the restarts they trigger, so letting them pull and hotfix the + checkout grants nothing new. It spares a sudo for every edit and lets them + import a driver to test it. + """ + if not tree.is_dir(): + return + for target in (tree, *tree.rglob("*")): + if target.is_symlink(): + continue + shutil.chown(target, product.user, product.ops_group) + mode = stat.S_IMODE(target.stat().st_mode) + # Setgid on directories so anything created later keeps the group, + # rather than the creating operator's own + target.chmod(mode | (0o2770 if target.is_dir() else 0o660)) + + +def _git_safe_directory(repo: Path) -> None: + """Let operators run git here despite the tree belonging to the daemon. + + Since 2.35.2 git refuses a repository whose owner is not the caller, which + group permissions do not satisfy. + """ + if not (repo / ".git").exists(): + return + listed = subprocess.run(["git", "config", "--system", "--get-all", + "safe.directory"], check=False, capture_output=True, + text=True) + if str(repo) in listed.stdout.split(): + return + subprocess.run(["git", "config", "--system", "--add", "safe.directory", + str(repo)], check=False) + _say("git trusts", str(repo)) + + def install(paths: Paths, args: argparse.Namespace) -> int: """Converge this host onto what the instrument needs. Root only.""" product = paths.product @@ -198,10 +237,10 @@ def install(paths: Paths, args: argparse.Namespace) -> int: _directories(product, paths) _secrets_and_role(product, paths, args.host_role) _artefacts(product, paths) - shutil.chown(paths.venv, product.user, product.user) - for child in paths.venv.rglob("*"): - shutil.chown(child, product.user, product.user) - _say("owned", f"{paths.venv} by {product.user}") + for tree in (paths.venv, paths.repo): + _hand_to_operators(tree, product) + _say("owned", f"{tree} by {product.user}:{product.ops_group}") + _git_safe_directory(paths.repo) subprocess.run(["systemctl", "daemon-reload"], check=False) print(f"\nHost setup complete. Operators in {product.ops_group} can now " diff --git a/tests/test_ownership.py b/tests/test_ownership.py new file mode 100644 index 0000000..515eb10 --- /dev/null +++ b/tests/test_ownership.py @@ -0,0 +1,71 @@ +"""Handing the venv and checkout to operators.""" +from __future__ import annotations + +import os +import pathlib +import stat +from unittest import mock + +import pytest + +from instrumentctl import installer +from instrumentctl.product import Product + + +@pytest.fixture(name="tree") +def tree_fixture(tmp_path: pathlib.Path) -> pathlib.Path: + """A venv-shaped tree: an executable, a plain file, nested directories.""" + (tmp_path / "bin").mkdir() + (tmp_path / "lib" / "pkg").mkdir(parents=True) + (tmp_path / "bin" / "python3").touch(mode=0o755) + (tmp_path / "lib" / "pkg" / "mod.py").touch(mode=0o644) + return tmp_path + + +def hand_over(tree: pathlib.Path, product: Product) -> None: + """Run the real mode changes; chown needs accounts a test does not have.""" + with mock.patch.object(installer.shutil, "chown", lambda *a, **k: None): + installer._hand_to_operators(tree, product) + + +def mode(path: pathlib.Path) -> int: + return stat.S_IMODE(path.stat().st_mode) + + +def test_operators_can_write_every_file(tree: pathlib.Path, product: Product) -> None: + hand_over(tree, product) + for path in (tree / "bin" / "python3", tree / "lib" / "pkg" / "mod.py"): + assert mode(path) & stat.S_IWGRP, path + + +def test_operators_can_write_every_directory(tree: pathlib.Path, + product: Product) -> None: + hand_over(tree, product) + for path in (tree, tree / "bin", tree / "lib" / "pkg"): + assert mode(path) & stat.S_IWGRP, path + + +def test_executables_stay_executable(tree: pathlib.Path, product: Product) -> None: + """Group-write must not cost the venv its interpreter.""" + hand_over(tree, product) + assert mode(tree / "bin" / "python3") & stat.S_IXUSR + + +def test_plain_files_do_not_become_executable(tree: pathlib.Path, + product: Product) -> None: + hand_over(tree, product) + assert not mode(tree / "lib" / "pkg" / "mod.py") & stat.S_IXUSR + + +@pytest.mark.skipif(os.geteuid() != 0 or os.uname().sysname == "Darwin", + reason="macOS strips setgid for a non-root caller") +def test_directories_are_setgid(tree: pathlib.Path, product: Product) -> None: + """So a file an operator adds keeps the group the daemon reads as.""" + hand_over(tree, product) + assert mode(tree / "bin") & stat.S_ISGID + + +def test_a_missing_tree_is_not_an_error(tmp_path: pathlib.Path, + product: Product) -> None: + """install runs before the checkout exists on a fresh host.""" + hand_over(tmp_path / "absent", product) From 5ee1538ec1acb5d2dcaab31b8e0fc5f58be51568 Mon Sep 17 00:00:00 2001 From: Prakriti Gupta Date: Thu, 15 Oct 2026 16:45:11 -0700 Subject: [PATCH 2/2] Add hotfix command --- README.md | 37 +++ instrument.toml.example | 8 + src/instrumentctl/cli.py | 28 +- src/instrumentctl/doctor.py | 39 +++ src/instrumentctl/hotfix.py | 543 +++++++++++++++++++++++++++++++++ src/instrumentctl/installer.py | 34 +++ src/instrumentctl/instances.py | 5 + src/instrumentctl/product.py | 11 + tests/test_hotfix.py | 306 +++++++++++++++++++ 9 files changed, 1010 insertions(+), 1 deletion(-) create mode 100644 src/instrumentctl/hotfix.py create mode 100644 tests/test_hotfix.py diff --git a/README.md b/README.md index c263835..0ec1e09 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,10 @@ common keys with their defaults, ready to copy. | `secrets.key_suffix` | `_env` | a config key ending in this names the variable holding a credential | | `help.example_instance` | `` | used in `--help`, where a real name beats a placeholder | | `help.example_subsystem` | `` | likewise | +| `git.remote` | `origin` | where `hotfix` pushes and `update` fetches from | +| `git.base` | `main` | the branch hosts run, and hotfixes are reviewed into | +| `git.branch_prefix` | `hotfix/` | marks a branch `hotfix` made | +| `git.email_domain` | the hostname | operators commit as `@` | | `legacy.unit` | none | a template unit an older version installed, reported by `doctor` | | `legacy.polkit_rule` | none | likewise for its polkit rule | @@ -76,6 +80,39 @@ An instance is a name with three files behind it: `/systemd/instances/ .yaml`, and their deployed copies under `/etc/`. Deployed means the `.env` is in `/etc//instances/`, which is what the template unit reads. +## Hotfixes + +An operator who has to change the code at night should not need to know the +GitHub process. They edit the checkout, restart what uses it, and run: + +```bash + hotfix "ADC hangs after power cycle" +``` + +which shows what changed, asks, commits it in their name to a branch such as +`hotfix/2026-10-15--adc-hangs-after-power-cycle`, pushes it, and opens a +pull request against `main`. The host keeps running the fix. Once a developer +merges it, ` update` returns the host to `main`; until then it refuses, +so a fix is never dropped by accident. On `main`, `update` just pulls. + +- ` hotfix`, with no description, lists unsaved edits. +- ` hotfix --undo` puts edited files back as committed. +- ` hotfix --push` retries a push or pull request that failed. +- ` update --abandon` leaves an unmerged hotfix a developer rejected; + the branch stays, here and on GitHub. +- `doctor` warns when a host is on a hotfix branch, has commits GitHub does + not, or has edits nobody saved. + +Pushing uses one token, so operators need no GitHub account. Make a +fine-grained personal access token limited to the instrument's repository, with +Contents and Pull requests read and write, and put it alone in +`/etc//github-token`. `install` makes that file root-owned and readable +by the ops group, which means every operator, and the daemon account, can read +it: scope it to the one repository, and protect `main` so that it needs a +reviewed pull request. Fine-grained tokens expire, at most a year out, and +`doctor` cannot tell an expired one from a good one, so put the date in a +calendar. + ## Tests ```bash diff --git a/instrument.toml.example b/instrument.toml.example index 15b8f3b..cbd185b 100644 --- a/instrument.toml.example +++ b/instrument.toml.example @@ -22,3 +22,11 @@ name = "demo" # venv = "/opt//venv" # the interpreter the daemons run under # repo = "/opt//app" # the checkout holding daemons/ and config/ # logs = "/var/log/" # daemons' own log files + +# Where ` hotfix` sends fixes made on a host, and what ` update` +# brings it back to +[git] +# remote = "origin" +# base = "main" # the branch hosts run and hotfixes are reviewed into +# branch_prefix = "hotfix/" +# email_domain = "" # operators commit as @; default: the hostname diff --git a/src/instrumentctl/cli.py b/src/instrumentctl/cli.py index aebe0c7..d734d24 100644 --- a/src/instrumentctl/cli.py +++ b/src/instrumentctl/cli.py @@ -9,7 +9,7 @@ from pathlib import Path from typing import List, Optional -from . import commands +from . import commands, hotfix from .doctor import doctor from .installer import install from .instances import Paths @@ -35,6 +35,8 @@ def _epilog(product: Product) -> str: (f"restart {instance}", f"e.g. after editing {etc}/{instance}.yaml"), (f"logs {instance} -f", "follow its log"), ("doctor", "why isn't it working?"), + ('hotfix "what you fixed"', "save an edit to the code here and send it for review"), + ("update", "back to the reviewed code, once the hotfix is merged"), ] width = max(len(command) for command, _ in lines) + len(name) + 3 body = "\n".join(f" {name + ' ' + command:<{width}}{note}" for command, note in lines) @@ -112,6 +114,30 @@ def add(name, func, help_text, description=None): p.add_argument("--since", help='e.g. "1 hour ago", today, "2026-09-21 09:00"') p.add_argument("-p", "--priority", help="e.g. err, warning") + p = add("hotfix", hotfix.hotfix, + "save edits to the code on this host and send them for review", + "Commit what you edited in the checkout to a hotfix branch in your name,\n" + "push it to GitHub and open a pull request. This host keeps running the\n" + f"fix; once a developer merges it, run: {product.name} update\n\n" + "With no description, show what is edited and not yet saved.") + p.add_argument("message", nargs="?", metavar="description", + help='what you fixed, e.g. "ADC hangs after power cycle"') + mode = p.add_mutually_exclusive_group() + mode.add_argument("--push", action="store_true", + help="send a saved hotfix again, e.g. after a network failure") + mode.add_argument("--undo", action="store_true", + help="discard the edits, putting the files back as committed") + p.add_argument("-y", "--yes", action="store_true", help="do not ask before saving") + + p = add("update", hotfix.update, + f"bring the code on this host up to date with {product.git_base}", + f"Fetch {product.git_base} from GitHub and move this host onto it. If the host is\n" + "running a hotfix, only once its pull request is merged. Running daemons\n" + "keep the old code until they are restarted.") + p.add_argument("--abandon", action="store_true", + help="leave an unmerged hotfix, e.g. one a developer rejected; " + "it stays on GitHub") + add("doctor", doctor, "check this host and your account, and say what to fix") p = add("install", install, "set this host up (root)", diff --git a/src/instrumentctl/doctor.py b/src/instrumentctl/doctor.py index 39d19a1..aeb680b 100644 --- a/src/instrumentctl/doctor.py +++ b/src/instrumentctl/doctor.py @@ -12,6 +12,7 @@ from pathlib import Path from typing import Optional +from . import hotfix from . import instances as inst from . import units from .installer import _render @@ -307,6 +308,43 @@ def _placement(r: Report, paths: Paths) -> None: f"{' '.join(unassigned)}") +def _hotfixes(r: Report, paths: Paths) -> None: + """Whether hotfix can work, and whether this host has drifted from the base.""" + product = paths.product + name, base = product.name, product.git_base + r.section("Hotfixes") + token = paths.github_token + if not _is_file(token): + r.warn(f"no {token}, so hotfix cannot push") + r.hint("admin: put a fine-grained GitHub token there (see the README)") + elif hotfix.read_token(paths) is None: + r.bad(f"cannot read {token}, so hotfix cannot push") + r.hint(_install_hint(paths)) + else: + r.ok(f"{token} is readable") + problem = hotfix.checkout_problem(paths) + if problem: + r.warn(problem) + return + branch = hotfix.current_branch(paths) + if hotfix.is_hotfix(paths, branch): + assert branch is not None + r.warn(f"this host is running {branch}, not {base}") + r.hint(f"once it is merged: {name} update") + waiting = hotfix.unsent(paths, branch) + if waiting: + r.warn(f"{waiting} commit(s) on {branch} are not on GitHub") + r.hint(f"{name} hotfix --push") + elif branch == base: + r.ok(f"{paths.repo} is on {base}") + else: + r.warn(f"{paths.repo} is on {branch or 'a detached HEAD'}, not {base}") + edited = hotfix.changes(paths).edited + if edited: + r.warn(f"{len(edited)} edited file(s) in {paths.repo} not saved: {' '.join(edited)}") + r.hint(f'{name} hotfix "what you fixed" # or: {name} hotfix --undo') + + def doctor(paths: Paths, _args: argparse.Namespace) -> int: """Run every check and exit 1 if any FAILed.""" me = pwd.getpwuid(os.geteuid()).pw_name @@ -319,6 +357,7 @@ def doctor(paths: Paths, _args: argparse.Namespace) -> int: _deployed(r, paths) _placement(r, paths) _secrets(r, paths) + _hotfixes(r, paths) print() if r.problems == 0: print("No problems found.") diff --git a/src/instrumentctl/hotfix.py b/src/instrumentctl/hotfix.py new file mode 100644 index 0000000..e0f9943 --- /dev/null +++ b/src/instrumentctl/hotfix.py @@ -0,0 +1,543 @@ +"""``hotfix`` and ``update``: change the code a host runs without the GitHub +process. + +An operator edits the checkout, restarts what uses it, and runs +``hotfix "what I fixed"``. That commits the edit to a hotfix branch in their +name, pushes it, and opens a pull request against the base branch, so a +developer reviews it later while the host keeps running it. ``update`` brings +the host back to the base branch once the pull request is merged. + +Pushing and the pull request use one fine-grained token in +``/github-token``, readable by the ops group, so an operator needs no +GitHub account of their own. +""" +from __future__ import annotations + +import argparse +import base64 +import datetime +import json +import os +import pwd +import re +import subprocess +import sys +import urllib.error +import urllib.parse +import urllib.request +from typing import Dict, List, NamedTuple, Optional + +from . import instances as inst +from .instances import Paths +from .output import paint + +API = "https://api.github.com" +GITHUB_REMOTE = re.compile(r"github\.com[:/]([^/]+)/([^/]+?)(?:\.git)?/?$") +# Files whose change means the venv, not just the code, needs updating +DEPENDENCY_FILES = ("pyproject.toml", "uv.lock") + + +def _err(paths: Paths, message: str) -> None: + print(f"{paths.product.name}: {message}", file=sys.stderr) + + +def _say(action: str, what: str, color: str = "32") -> None: + print(f" {paint(f'{action:<10}', color)} {what}") + + +def _auth_env(token: Optional[str]) -> Dict[str, str]: + """Environment that sends ``token`` to GitHub over HTTPS. + + Through the environment rather than the command line or the remote URL, + so it shows up neither in ps nor in .git/config. An SSH remote is + rewritten to HTTPS, since operators have no SSH key GitHub knows. + """ + if not token: + return {} + basic = base64.b64encode(f"x-access-token:{token}".encode()).decode() + settings = [ + ("url.https://github.com/.insteadOf", "git@github.com:"), + ("url.https://github.com/.insteadOf", "ssh://git@github.com/"), + ("http.https://github.com/.extraHeader", f"Authorization: Basic {basic}"), + # An operator's own helper would otherwise be asked first + ("credential.helper", ""), + ] + env = {"GIT_CONFIG_COUNT": str(len(settings))} + for i, (key, value) in enumerate(settings): + env[f"GIT_CONFIG_KEY_{i}"] = key + env[f"GIT_CONFIG_VALUE_{i}"] = value + return env + + +def git(paths: Paths, *args: str, token: Optional[str] = None, + env: Optional[Dict[str, str]] = None, + stdin: Optional[str] = None) -> subprocess.CompletedProcess: + """Run git in the checkout and capture its output; never prompts.""" + full_env = {**os.environ, "GIT_TERMINAL_PROMPT": "0", **_auth_env(token), **(env or {})} + cmd = ["git", "-C", str(paths.repo), *args] + try: + return subprocess.run(cmd, check=False, text=True, capture_output=True, + env=full_env, input=stdin) + except FileNotFoundError: + return subprocess.CompletedProcess(cmd, 127, "", "git: not found\n") + + +def _out(proc: subprocess.CompletedProcess) -> str: + return proc.stdout.strip() + + +def _why(proc: subprocess.CompletedProcess) -> str: + """The last thing git said, which is the reason it failed.""" + lines = (proc.stderr or proc.stdout).strip().splitlines() + return lines[-1] if lines else f"git exited {proc.returncode}" + + +def checkout_problem(paths: Paths) -> Optional[str]: + """Why git cannot be used in the checkout right now, or None.""" + if not (paths.repo / ".git").exists(): + return f"{paths.repo} is not a git checkout" + proc = git(paths, "rev-parse", "--verify", "-q", "HEAD") + if proc.returncode == 127: + return "git is not installed" + if "dubious ownership" in proc.stderr: + return (f"git does not trust {paths.repo} for your account; " + f"admin: sudo {paths.repo}/systemd/install.sh") + if proc.returncode != 0: + return f"git cannot read {paths.repo}: {_why(proc)}" + for marker, what in (("MERGE_HEAD", "a merge"), ("CHERRY_PICK_HEAD", "a cherry-pick"), + ("REVERT_HEAD", "a revert"), ("rebase-merge", "a rebase"), + ("rebase-apply", "a rebase")): + if (paths.repo / _out(git(paths, "rev-parse", "--git-path", marker))).exists(): + return (f"{paths.repo} is in the middle of {what}; " + "a developer needs to finish or abort it") + return None + + +def current_branch(paths: Paths) -> Optional[str]: + """The checked-out branch, or None on a detached HEAD.""" + return _out(git(paths, "symbolic-ref", "--short", "-q", "HEAD")) or None + + +def is_hotfix(paths: Paths, branch: Optional[str]) -> bool: + """Whether ``branch`` is one ``hotfix`` made.""" + return bool(branch) and branch.startswith(paths.product.git_branch_prefix) + + +class Changes(NamedTuple): + """Edits in the checkout not yet committed.""" + + edited: List[str] # tracked files changed or deleted + new: List[str] # untracked files git does not ignore + + +def changes(paths: Paths) -> Changes: + """What differs from the last commit.""" + edited = git(paths, "diff", "--name-only", "-z", "HEAD").stdout.split("\0") + new = git(paths, "ls-files", "--others", "--exclude-standard", "-z").stdout.split("\0") + return Changes([f for f in edited if f], [f for f in new if f]) + + +def _ref_exists(paths: Paths, ref: str) -> bool: + return git(paths, "rev-parse", "--verify", "-q", ref).returncode == 0 + + +def unsent(paths: Paths, branch: str) -> int: + """Commits on ``branch`` that are not on GitHub.""" + remote = paths.product.git_remote + upstream = f"refs/remotes/{remote}/{branch}" + if not _ref_exists(paths, upstream): + upstream = f"refs/remotes/{remote}/{paths.product.git_base}" + if not _ref_exists(paths, upstream): + return 0 + count = _out(git(paths, "rev-list", "--count", f"{upstream}..{branch}")) + return int(count) if count.isdigit() else 0 + + +def _slugify(text: str, limit: int) -> str: + return re.sub(r"[^a-z0-9]+", "-", text.lower()).strip("-")[:limit].strip("-") + + +def _new_branch(paths: Paths, message: str) -> str: + """A branch name saying when, where and what, unused locally and on GitHub.""" + product = paths.product + stem = (f"{product.git_branch_prefix}{datetime.date.today():%Y-%m-%d}-" + f"{_slugify(inst.host_role(paths), 20) or 'host'}-" + f"{_slugify(message, 40) or 'fix'}") + name, n = stem, 1 + while (_ref_exists(paths, f"refs/heads/{name}") + or _ref_exists(paths, f"refs/remotes/{product.git_remote}/{name}")): + n += 1 + name = f"{stem}-{n}" + return name + + +def _identity(paths: Paths) -> Dict[str, str]: + """Commit as the operator, even though the checkout is shared.""" + entry = pwd.getpwuid(os.getuid()) + name = entry.pw_gecos.split(",")[0].strip() or entry.pw_name + email = f"{entry.pw_name}@{paths.product.git_email_domain or os.uname().nodename}" + return {"GIT_AUTHOR_NAME": name, "GIT_AUTHOR_EMAIL": email, + "GIT_COMMITTER_NAME": name, "GIT_COMMITTER_EMAIL": email} + + +class GitHubError(Exception): + """GitHub refused, or could not be reached.""" + + +def read_token(paths: Paths) -> Optional[str]: + """The token in /github-token, or None if there is none to read.""" + try: + return paths.github_token.read_text(encoding="utf-8").strip() or None + except OSError: + return None + + +def github_repo(paths: Paths) -> Optional[str]: + """``owner/repo`` for the remote, or None if it is not on GitHub.""" + url = _out(git(paths, "remote", "get-url", paths.product.git_remote)) + match = GITHUB_REMOTE.search(url) + return f"{match.group(1)}/{match.group(2)}" if match else None + + +def _api(token: str, method: str, path: str, body: Optional[dict] = None) -> object: + request = urllib.request.Request( + API + path, method=method, + data=json.dumps(body).encode() if body is not None else None, + headers={"Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "instrumentctl"}) + try: + with urllib.request.urlopen(request, timeout=30) as response: + return json.load(response) + except urllib.error.HTTPError as exc: + try: + detail = json.load(exc).get("message", "") + except ValueError: + detail = "" + raise GitHubError(f"GitHub said {exc.code} {detail or exc.reason}".strip()) from exc + except (urllib.error.URLError, OSError) as exc: + raise GitHubError(f"cannot reach GitHub: {getattr(exc, 'reason', exc)}") from exc + + +def find_pull_request(token: str, repo: str, branch: str) -> Optional[dict]: + """The most recent pull request from ``branch``, open or not.""" + owner = repo.split("/")[0] + query = urllib.parse.urlencode({"head": f"{owner}:{branch}", "state": "all"}) + pulls = _api(token, "GET", f"/repos/{repo}/pulls?{query}") + return pulls[0] if isinstance(pulls, list) and pulls else None + + +def open_pull_request(token: str, repo: str, fields: Dict[str, str]) -> dict: + """Open a pull request, or return the one already open from its branch. + + ``fields`` are GitHub's: head, base, title and body. + """ + existing = find_pull_request(token, repo, fields["head"]) + if existing and existing.get("state") == "open": + return existing + pull = _api(token, "POST", f"/repos/{repo}/pulls", fields) + assert isinstance(pull, dict) + return pull + + +def _confirm(question: str, assume_yes: bool) -> bool: + if assume_yes: + return True + if not sys.stdin.isatty(): + print(f"{question} Not asked: no terminal; pass --yes to answer yes.") + return False + try: + return input(f"{question} [y/N] ").strip().lower() in ("y", "yes") + except EOFError: + return False + + +def _show_edits(paths: Paths, found: Changes) -> None: + if found.edited: + print("Edited:") + for line in _out(git(paths, "diff", "--stat", "HEAD")).splitlines(): + print(f" {line.strip()}") + if found.new: + print("New files:") + for name in found.new: + print(f" {name}") + + +def _usable(paths: Paths, command: str) -> bool: + """Whether ``command`` can run git here, explaining why not.""" + if os.geteuid() == 0: + # Files root writes into .git would lock every operator out of it + _err(paths, f"run {command} as your own user, not root: " + f"{paths.product.name} {command}") + return False + problem = checkout_problem(paths) + if problem: + _err(paths, problem) + return False + return True + + +def hotfix(paths: Paths, args: argparse.Namespace) -> int: + """Save edits to the checkout on a hotfix branch and send them for review.""" + if not _usable(paths, "hotfix"): + return 1 + if args.undo: + return _undo(paths, args.yes) + if args.push: + return _send(paths, current_branch(paths)) + if not args.message: + return _status(paths) + return _save(paths, args.message, args.yes) + + +def _status(paths: Paths) -> int: + name, base = paths.product.name, paths.product.git_base + branch = current_branch(paths) + found = changes(paths) + print(f"{paths.repo} is on {branch or 'a detached HEAD'}") + if is_hotfix(paths, branch): + print(f" This host is running a hotfix. Once it is merged: {name} update") + waiting = unsent(paths, branch) + if waiting: + print(paint(f" {waiting} commit(s) not sent to GitHub yet: " + f"{name} hotfix --push", "33")) + elif branch != base: + print(paint(f" not {base}, so {name} update will leave it alone", "33")) + print() + if not found.edited and not found.new: + print("No unsaved edits.") + return 0 + _show_edits(paths, found) + print() + print(f'Save and send them: {name} hotfix "what you fixed"') + print(f"Or put them back: {name} hotfix --undo") + return 0 + + +def _save(paths: Paths, message: str, assume_yes: bool) -> int: + found = changes(paths) + if not found.edited and not found.new: + print(f"Nothing to save: {paths.repo} matches its last commit.") + return 0 + _show_edits(paths, found) + include = [] + if found.new: + print() + if _confirm("Include the new files?", assume_yes): + include = found.new + if not found.edited and not include: + print("Nothing to save.") + return 0 + print() + if not _confirm("Save and send this fix?", assume_yes): + print("Nothing saved; your edits are still in place.") + return 1 + branch = _commit(paths, message, include) + if branch is None: + return 1 + _say("saved", f"{len(found.edited) + len(include)} file(s) on {branch}") + return _send(paths, branch) + + +def _commit(paths: Paths, message: str, include: List[str]) -> Optional[str]: + """Commit every edit, and ``include``, on a hotfix branch. Returns the branch.""" + branch = current_branch(paths) + if not is_hotfix(paths, branch): + # Takes the edits along; a second fix the same night joins this branch + branch = _new_branch(paths, message) + proc = git(paths, "switch", "-q", "-c", branch) + if proc.returncode != 0: + _err(paths, f"cannot make branch {branch}: {_why(proc)}") + return None + for proc in (git(paths, "add", "-u"), + git(paths, "add", "--", *include) if include else None): + if proc is not None and proc.returncode != 0: + _err(paths, f"cannot stage the edits: {_why(proc)}") + return None + me = pwd.getpwuid(os.getuid()).pw_name + text = f"{message}\n\nHotfix made on {inst.host_role(paths)} by {me}.\n" + # --no-verify: a developer's commit hooks need tools a host may not have, + # and review catches what they would + proc = git(paths, "commit", "-q", "--no-verify", "-F", "-", + env=_identity(paths), stdin=text) + if proc.returncode != 0: + _err(paths, f"cannot commit: {_why(proc)}") + return None + return branch + + +def _send(paths: Paths, branch: Optional[str]) -> int: + """Push the hotfix branch and make sure a pull request is open for it.""" + product = paths.product + if not is_hotfix(paths, branch): + _err(paths, f"{paths.repo} is on {branch or 'a detached HEAD'}, " + "not a hotfix branch, so there is nothing to send") + return 1 + assert branch is not None + retry = f"The fix is saved on this host. Try again with: {product.name} hotfix --push" + token = read_token(paths) + if not token: + _say("not sent", f"cannot read {paths.github_token}", "31") + print(retry) + print(f"An admin needs to put a GitHub token there; run: {product.name} doctor") + return 1 + proc = git(paths, "push", "-q", "--set-upstream", product.git_remote, + f"{branch}:refs/heads/{branch}", token=token) + if proc.returncode != 0: + _say("not sent", _why(proc), "31") + print(retry) + return 1 + _say("pushed", f"{product.git_remote} {branch}") + + repo = github_repo(paths) + if repo is None: + print(f"{product.git_remote} is not on GitHub; open a pull request by hand.") + return 0 + base = product.git_base + commits = _out(git(paths, "log", "--reverse", "--format=%s (%an)", + f"{product.git_remote}/{base}..{branch}")).splitlines() + title = commits[0].rsplit(" (", 1)[0] if commits else branch + body = ("\n".join(f"- {line}" for line in commits) + + f"\n\nMade with `{product.name} hotfix` on `{inst.host_role(paths)}`, " + "which runs this code until it is merged. Once it is, run " + f"`{product.name} update` there to go back to `{base}`.\n") + try: + pull = open_pull_request(token, repo, {"head": branch, "base": base, + "title": title, "body": body}) + except GitHubError as exc: + _say("no review", str(exc), "31") + print(f"The fix is pushed. Open the pull request with: {product.name} hotfix --push") + return 1 + _say("review", str(pull.get("html_url", ""))) + print() + print(f"This host keeps running the fix. Once it is merged, run: {product.name} update") + return 0 + + +def _undo(paths: Paths, assume_yes: bool) -> int: + name = paths.product.name + found = changes(paths) + if not found.edited: + print(f"Nothing to undo: no file in {paths.repo} differs from its last commit.") + return 0 + _show_edits(paths, Changes(found.edited, [])) + print() + if not _confirm("Discard these edits and put the files back as committed?", assume_yes): + print("Nothing changed.") + return 1 + proc = git(paths, "restore", "--source=HEAD", "--staged", "--worktree", "--", ".") + if proc.returncode != 0: + _err(paths, f"cannot restore: {_why(proc)}") + return 1 + for file in found.edited: + _say("restored", file) + if found.new: + print(f"New files are left alone: {' '.join(found.new)}") + print() + print(f"Running daemons still have the edited code; restart them: {name} restart ") + return 0 + + +def update(paths: Paths, args: argparse.Namespace) -> int: + """Bring the checkout up to date with the base branch on GitHub.""" + product = paths.product + if not _usable(paths, "update"): + return 1 + found = changes(paths) + if found.edited: + _err(paths, f"{len(found.edited)} file(s) in {paths.repo} are edited and not saved; " + f'save them with: {product.name} hotfix "what you fixed", ' + f"or put them back with: {product.name} hotfix --undo") + return 1 + + branch = current_branch(paths) + token = read_token(paths) + leaving = is_hotfix(paths, branch) + if leaving: + assert branch is not None + if not _may_leave(paths, branch, token, args.abandon): + return 1 + elif branch != product.git_base: + _err(paths, f"{paths.repo} is on {branch or 'a detached HEAD'}, not " + f"{product.git_base}; a developer put it there, so update leaves it alone") + return 1 + + before = _out(git(paths, "rev-parse", "HEAD")) + if not _move_to_base(paths, token, leaving): + return 1 + if leaving and not args.abandon: + # Squash-merged, so git cannot tell it is merged; GitHub said so + git(paths, "branch", "-q", "-D", branch) + elif leaving: + print(f"{branch} is kept, here and on GitHub, in case it is wanted later.") + _report(paths, before) + return 0 + + +def _move_to_base(paths: Paths, token: Optional[str], leaving: bool) -> bool: + """Fetch the base branch and fast-forward onto it, switching to it first.""" + base, remote = paths.product.git_base, paths.product.git_remote + steps = [("fetch", ("fetch", "-q", remote, base))] + if leaving: + steps.append(("switch to", ("switch", "-q", base))) + steps.append(("update", ("merge", "-q", "--ff-only", f"{remote}/{base}"))) + for what, command in steps: + proc = git(paths, *command, token=token) + if proc.returncode != 0: + _err(paths, f"cannot {what} {base}: {_why(proc)}") + return False + return True + + +def _report(paths: Paths, before: str) -> None: + """Say what changed since ``before``, and what to do about it.""" + product = paths.product + after = _out(git(paths, "rev-parse", "HEAD")) + changed = [f for f in _out(git(paths, "diff", "--name-only", before, after)).splitlines() + if f] + if not changed: + print(f"Up to date with {product.git_remote}/{product.git_base}.") + return + print(f"Now on {product.git_base} at {after[:9]}; {len(changed)} file(s) changed:") + for file in changed: + print(f" {file}") + print() + if any(f in DEPENDENCY_FILES for f in changed): + print(paint("Dependencies changed, so the venv needs updating: " + f"admin: sudo {paths.repo}/systemd/install.sh", "33")) + print(f"Running daemons still have the old code; restart them: " + f"{product.name} restart ") + + +def _may_leave(paths: Paths, branch: str, token: Optional[str], abandon: bool) -> bool: + """Whether the host may stop running a hotfix branch, explaining why not.""" + name = paths.product.name + waiting = unsent(paths, branch) + if waiting: + _err(paths, f"{waiting} commit(s) on {branch} are not on GitHub yet; " + f"send them first: {name} hotfix --push") + return False + if abandon: + return True + repo = github_repo(paths) + if not token or repo is None: + why = f"cannot read {paths.github_token}" if not token else "the remote is not GitHub" + _err(paths, f"cannot tell whether {branch} is merged: {why}") + return False + try: + pull = find_pull_request(token, repo, branch) + except GitHubError as exc: + _err(paths, f"cannot tell whether {branch} is merged: {exc}") + return False + if pull and pull.get("merged_at"): + return True + if pull is None: + state = f"it has no pull request; open one with: {name} hotfix --push" + elif pull.get("state") == "open": + state = f"its pull request is still open: {pull.get('html_url')}" + else: + state = (f"its pull request was closed without merging: {pull.get('html_url')}\n" + f"if a developer says to drop it: {name} update --abandon") + _err(paths, f"this host is running {branch}, which is not merged, so it stays; {state}") + return False diff --git a/src/instrumentctl/installer.py b/src/instrumentctl/installer.py index e8d5495..43f14f8 100644 --- a/src/instrumentctl/installer.py +++ b/src/instrumentctl/installer.py @@ -37,6 +37,7 @@ CONFIG_MODE = 0o2775 LOG_MODE = 0o2750 SECRETS_MODE = 0o660 +TOKEN_MODE = 0o640 def _say(action: str, what: str) -> None: @@ -220,6 +221,36 @@ def _git_safe_directory(repo: Path) -> None: _say("git trusts", str(repo)) +def _git_shared_repository(repo: Path) -> None: + """Have git leave what it writes group-writable. + + Otherwise each commit leaves objects only their author can write, and the + next operator to commit is refused. + """ + config = repo / ".git" / "config" + if not config.is_file(): + return + current = subprocess.run(["git", "config", "--file", str(config), + "core.sharedRepository"], check=False, + capture_output=True, text=True) + if current.stdout.strip() == "group": + return + subprocess.run(["git", "config", "--file", str(config), + "core.sharedRepository", "group"], check=False) + _say("git shares", f"{repo} with the ops group") + + +def _github_token(product: Product, paths: Paths) -> None: + """Let operators, who push hotfixes with it, read the token, and no one else.""" + token = paths.github_token + if not token.exists(): + print(f" {paint('no token', '33'):<22} {token}; hotfix cannot push " + "until one is put there (see the README)") + return + shutil.chown(token, "root", product.ops_group) + token.chmod(TOKEN_MODE) + + def install(paths: Paths, args: argparse.Namespace) -> int: """Converge this host onto what the instrument needs. Root only.""" product = paths.product @@ -236,7 +267,10 @@ def install(paths: Paths, args: argparse.Namespace) -> int: _operators(product, args.operators) _directories(product, paths) _secrets_and_role(product, paths, args.host_role) + _github_token(product, paths) _artefacts(product, paths) + # Before the hand-over, which gives back the config git rewrites as root + _git_shared_repository(paths.repo) for tree in (paths.venv, paths.repo): _hand_to_operators(tree, product) _say("owned", f"{tree} by {product.user}:{product.ops_group}") diff --git a/src/instrumentctl/instances.py b/src/instrumentctl/instances.py index ccf3030..d671e24 100644 --- a/src/instrumentctl/instances.py +++ b/src/instrumentctl/instances.py @@ -82,6 +82,11 @@ def secrets(self) -> Path: """Shared secrets, read by every unit.""" return self.etc / "secrets.env" + @property + def github_token(self) -> Path: + """The token hotfix pushes and opens pull requests with.""" + return self.etc / "github-token" + def config(self, instance: str) -> Path: """An instance's deployed config, which the unit passes to the daemon.""" return self.etc / f"{instance}.yaml" diff --git a/src/instrumentctl/product.py b/src/instrumentctl/product.py index 67855f1..cfba1e0 100644 --- a/src/instrumentctl/product.py +++ b/src/instrumentctl/product.py @@ -51,6 +51,12 @@ class Product: # pylint: disable=too-many-instance-attributes venv_dir: Optional[str] = field(default=None) repo_dir: Optional[str] = field(default=None) logs_dir: Optional[str] = field(default=None) + # Where hotfixes go: the remote and branch the host follows, the prefix + # that marks a hotfix branch, and the domain of the operator's commit email + git_remote: str = "origin" + git_base: str = "main" + git_branch_prefix: str = "hotfix/" + git_email_domain: str = "" @classmethod def load(cls, config: "ConfigSource") -> "Product": @@ -65,6 +71,7 @@ def load(cls, config: "ConfigSource") -> "Product": legacy = data.get("legacy", {}) help_text = data.get("help", {}) paths = data.get("paths", {}) + git = data.get("git", {}) return cls( name=instrument["name"], display_name=instrument.get("display_name", @@ -82,6 +89,10 @@ def load(cls, config: "ConfigSource") -> "Product": venv_dir=paths.get("venv"), repo_dir=paths.get("repo"), logs_dir=paths.get("logs"), + git_remote=git.get("remote", "origin"), + git_base=git.get("base", "main"), + git_branch_prefix=git.get("branch_prefix", "hotfix/"), + git_email_domain=git.get("email_domain", ""), ) def unit(self, instance: str) -> str: diff --git a/tests/test_hotfix.py b/tests/test_hotfix.py new file mode 100644 index 0000000..c5ead44 --- /dev/null +++ b/tests/test_hotfix.py @@ -0,0 +1,306 @@ +"""hotfix and update, against real git: a checkout and a bare repo standing in +for GitHub. Only the GitHub API is stubbed.""" +from __future__ import annotations + +import argparse +import base64 +import os +import pathlib +import pwd +import subprocess +from typing import List, Optional + +import pytest + +from instrumentctl import hotfix +from instrumentctl.instances import Paths + +DAEMON = pathlib.Path("daemons") / "arm" / "runner.py" + + +def run_git(cwd: pathlib.Path, *args: str) -> str: + return subprocess.run(["git", "-C", str(cwd), *args], check=True, + capture_output=True, text=True).stdout.strip() + + +@pytest.fixture(autouse=True) +def isolated_git(tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the developer's own git config (signing, hooks) out of it.""" + (tmp_path / "gitconfig").write_text( + "[user]\n\tname = Setup\n\temail = setup@example.invalid\n", encoding="utf-8") + monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(tmp_path / "gitconfig")) + monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "1") + + +@pytest.fixture(name="origin") +def origin_fixture(tmp_path: pathlib.Path) -> pathlib.Path: + return tmp_path / "origin.git" + + +@pytest.fixture(name="checkout") +def checkout_fixture(paths: Paths, origin: pathlib.Path) -> Paths: + """The fixture repo, committed on main and pushed to origin, with a token.""" + subprocess.run(["git", "init", "-q", "--bare", "-b", "main", str(origin)], check=True) + run_git(paths.repo, "init", "-q", "-b", "main") + (paths.repo / DAEMON).write_text("print('v1')\n", encoding="utf-8") + run_git(paths.repo, "add", "-A") + run_git(paths.repo, "commit", "-q", "-m", "initial") + run_git(paths.repo, "remote", "add", "origin", str(origin)) + run_git(paths.repo, "push", "-q", "-u", "origin", "main") + paths.github_token.write_text("ghp_test\n", encoding="utf-8") + return paths + + +def hotfix_args(message: Optional[str] = None, **overrides: object) -> argparse.Namespace: + settings = dict(message=message, push=False, undo=False, yes=True) + settings.update(overrides) + return argparse.Namespace(**settings) + + +def edit(paths: Paths, text: str = "print('fixed')\n") -> None: + (paths.repo / DAEMON).write_text(text, encoding="utf-8") + + +def branch(paths: Paths) -> str: + return run_git(paths.repo, "branch", "--show-current") + + +def remote_branches(origin: pathlib.Path) -> List[str]: + return run_git(origin, "for-each-ref", "--format=%(refname:short)", + "refs/heads").splitlines() + + +def merge_on_github(origin: pathlib.Path, tmp_path: pathlib.Path, name: str) -> None: + """What the merge button does: squash the hotfix onto main.""" + dev = tmp_path / "dev" + subprocess.run(["git", "clone", "-q", str(origin), str(dev)], check=True) + run_git(dev, "merge", "-q", "--squash", f"origin/{name}") + run_git(dev, "commit", "-q", "-m", "Merge the hotfix (#7)") + run_git(dev, "push", "-q", "origin", "main") + + +def test_a_hotfix_lands_on_its_own_branch_on_github(checkout: Paths, + origin: pathlib.Path) -> None: + edit(checkout) + assert hotfix.hotfix(checkout, hotfix_args("ADC hangs after power cycle")) == 0 + name = branch(checkout) + assert name.startswith("hotfix/") and name.endswith("-thishost-adc-hangs-after-power-cycle") + assert name in remote_branches(origin) + assert run_git(origin, "show", f"{name}:{DAEMON}") == "print('fixed')" + assert run_git(origin, "rev-parse", "main") == run_git(checkout.repo, "rev-parse", "main") + + +def test_the_commit_is_in_the_operators_name(checkout: Paths) -> None: + edit(checkout) + hotfix.hotfix(checkout, hotfix_args("fix")) + me = pwd.getpwuid(os.getuid()) + email = run_git(checkout.repo, "log", "-1", "--format=%ae") + assert email.startswith(f"{me.pw_name}@") + assert "Hotfix made on thishost" in run_git(checkout.repo, "log", "-1", "--format=%b") + + +def test_a_second_fix_joins_the_same_branch(checkout: Paths, origin: pathlib.Path) -> None: + edit(checkout) + hotfix.hotfix(checkout, hotfix_args("first")) + first = branch(checkout) + edit(checkout, "print('fixed again')\n") + assert hotfix.hotfix(checkout, hotfix_args("second")) == 0 + assert branch(checkout) == first + assert run_git(origin, "show", f"{first}:{DAEMON}") == "print('fixed again')" + + +def test_branch_names_never_collide(checkout: Paths) -> None: + taken = hotfix._new_branch(checkout, "same fix") + run_git(checkout.repo, "branch", taken) + assert hotfix._new_branch(checkout, "same fix") == f"{taken}-2" + + +def test_nothing_is_saved_without_a_yes(checkout: Paths, + monkeypatch: pytest.MonkeyPatch) -> None: + edit(checkout) + monkeypatch.setattr("sys.stdin.isatty", lambda: False) + assert hotfix.hotfix(checkout, hotfix_args("fix", yes=False)) == 1 + assert branch(checkout) == "main" + assert hotfix.changes(checkout).edited == [str(DAEMON)] + + +def test_new_files_go_in_only_when_accepted(checkout: Paths, + monkeypatch: pytest.MonkeyPatch) -> None: + edit(checkout) + (checkout.repo / "daemons" / "arm" / "helper.py").write_text("x = 1\n", encoding="utf-8") + answers = iter(["n", "y"]) + monkeypatch.setattr("sys.stdin.isatty", lambda: True) + monkeypatch.setattr("builtins.input", lambda _prompt: next(answers)) + assert hotfix.hotfix(checkout, hotfix_args("fix", yes=False)) == 0 + assert "helper.py" not in run_git(checkout.repo, "show", "--name-only", "--format=") + assert hotfix.changes(checkout).new == ["daemons/arm/helper.py"] + + +def test_with_nothing_edited_there_is_nothing_to_save(checkout: Paths, capsys) -> None: + assert hotfix.hotfix(checkout, hotfix_args("fix")) == 0 + assert "Nothing to save" in capsys.readouterr().out + assert branch(checkout) == "main" + + +def test_a_failed_push_keeps_the_fix_and_can_be_retried(checkout: Paths, + origin: pathlib.Path, + tmp_path: pathlib.Path, + capsys) -> None: + run_git(checkout.repo, "remote", "set-url", "origin", str(tmp_path / "unreachable")) + edit(checkout) + assert hotfix.hotfix(checkout, hotfix_args("fix")) == 1 + assert "hotfix --push" in capsys.readouterr().out + name = branch(checkout) + assert hotfix.unsent(checkout, name) == 1 + run_git(checkout.repo, "remote", "set-url", "origin", str(origin)) + assert hotfix.hotfix(checkout, hotfix_args(push=True)) == 0 + assert name in remote_branches(origin) + assert hotfix.unsent(checkout, name) == 0 + + +def test_without_a_token_the_fix_is_saved_but_not_sent(checkout: Paths, + origin: pathlib.Path, + capsys) -> None: + checkout.github_token.unlink() + edit(checkout) + assert hotfix.hotfix(checkout, hotfix_args("fix")) == 1 + assert "not sent" in capsys.readouterr().out + assert branch(checkout).startswith("hotfix/") + assert remote_branches(origin) == ["main"] + + +def test_a_pull_request_is_opened_against_main(checkout: Paths, + monkeypatch: pytest.MonkeyPatch, + capsys) -> None: + opened = {} + + def fake_open(token, repo, fields): + opened.update(fields, token=token, repo=repo) + return {"html_url": "https://github.com/org/repo/pull/7"} + + monkeypatch.setattr(hotfix, "github_repo", lambda _paths: "org/repo") + monkeypatch.setattr(hotfix, "open_pull_request", fake_open) + edit(checkout) + assert hotfix.hotfix(checkout, hotfix_args("ADC hangs")) == 0 + assert opened["token"] == "ghp_test" and opened["base"] == "main" + assert opened["head"] == branch(checkout) and opened["title"] == "ADC hangs" + assert "thishost" in opened["body"] + assert "pull/7" in capsys.readouterr().out + + +def test_root_is_refused(checkout: Paths, monkeypatch: pytest.MonkeyPatch) -> None: + """Files root writes into .git would lock every operator out of it.""" + monkeypatch.setattr(os, "geteuid", lambda: 0) + edit(checkout) + assert hotfix.hotfix(checkout, hotfix_args("fix")) == 1 + assert branch(checkout) == "main" + + +def test_a_checkout_mid_merge_is_refused(checkout: Paths, capsys) -> None: + (checkout.repo / ".git" / "MERGE_HEAD").write_text( + run_git(checkout.repo, "rev-parse", "HEAD"), encoding="utf-8") + edit(checkout) + assert hotfix.hotfix(checkout, hotfix_args("fix")) == 1 + assert "middle of a merge" in capsys.readouterr().err + + +def test_status_lists_unsaved_edits(checkout: Paths, capsys) -> None: + edit(checkout) + assert hotfix.hotfix(checkout, hotfix_args()) == 0 + out = capsys.readouterr().out + assert "runner.py" in out and "hotfix --undo" in out + + +def test_undo_puts_edited_files_back_and_leaves_new_ones(checkout: Paths) -> None: + edit(checkout) + new = checkout.repo / "notes.txt" + new.write_text("mine\n", encoding="utf-8") + assert hotfix.hotfix(checkout, hotfix_args(undo=True)) == 0 + assert (checkout.repo / DAEMON).read_text() == "print('v1')\n" + assert new.exists() + + +def test_update_waits_for_the_merge(checkout: Paths, + monkeypatch: pytest.MonkeyPatch, capsys) -> None: + monkeypatch.setattr(hotfix, "github_repo", lambda _paths: "org/repo") + monkeypatch.setattr(hotfix, "open_pull_request", lambda *a: {"html_url": ""}) + monkeypatch.setattr(hotfix, "find_pull_request", lambda *a: { + "state": "open", "merged_at": None, "html_url": "https://github.com/org/repo/pull/7"}) + edit(checkout) + hotfix.hotfix(checkout, hotfix_args("fix")) + name = branch(checkout) + assert hotfix.update(checkout, argparse.Namespace(abandon=False)) == 1 + assert "still open" in capsys.readouterr().err + assert branch(checkout) == name + + +def test_update_returns_to_main_once_merged(checkout: Paths, origin: pathlib.Path, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(hotfix, "github_repo", lambda _paths: "org/repo") + monkeypatch.setattr(hotfix, "open_pull_request", lambda *a: {"html_url": ""}) + monkeypatch.setattr(hotfix, "find_pull_request", lambda *a: { + "state": "closed", "merged_at": "2026-10-15T09:00:00Z", "html_url": ""}) + edit(checkout) + hotfix.hotfix(checkout, hotfix_args("fix")) + name = branch(checkout) + merge_on_github(origin, tmp_path, name) + assert hotfix.update(checkout, argparse.Namespace(abandon=False)) == 0 + assert branch(checkout) == "main" + assert (checkout.repo / DAEMON).read_text() == "print('fixed')\n" + assert name not in run_git(checkout.repo, "branch", "--list") + + +def test_abandon_leaves_an_unmerged_hotfix_but_keeps_it(checkout: Paths, + origin: pathlib.Path) -> None: + edit(checkout) + hotfix.hotfix(checkout, hotfix_args("fix")) + name = branch(checkout) + assert hotfix.update(checkout, argparse.Namespace(abandon=True)) == 0 + assert branch(checkout) == "main" + assert (checkout.repo / DAEMON).read_text() == "print('v1')\n" + assert name in run_git(checkout.repo, "branch", "--list") + assert name in remote_branches(origin) + + +def test_update_refuses_unsaved_edits(checkout: Paths, capsys) -> None: + edit(checkout) + assert hotfix.update(checkout, argparse.Namespace(abandon=False)) == 1 + assert "hotfix --undo" in capsys.readouterr().err + + +def test_update_on_main_pulls_and_says_what_changed(checkout: Paths, origin: pathlib.Path, + tmp_path: pathlib.Path, + capsys) -> None: + dev = tmp_path / "dev" + subprocess.run(["git", "clone", "-q", str(origin), str(dev)], check=True) + (dev / "pyproject.toml").write_text("[project]\n", encoding="utf-8") + run_git(dev, "add", "-A") + run_git(dev, "commit", "-q", "-m", "add a dependency") + run_git(dev, "push", "-q", "origin", "main") + assert hotfix.update(checkout, argparse.Namespace(abandon=False)) == 0 + out = capsys.readouterr().out + assert "pyproject.toml" in out and "venv needs updating" in out + + +def test_update_leaves_a_developers_branch_alone(checkout: Paths, capsys) -> None: + run_git(checkout.repo, "switch", "-q", "-c", "feature") + assert hotfix.update(checkout, argparse.Namespace(abandon=False)) == 1 + assert "leaves it alone" in capsys.readouterr().err + + +@pytest.mark.parametrize("url", ["git@github.com:org/repo.git", + "https://github.com/org/repo", + "ssh://git@github.com/org/repo.git"]) +def test_the_github_repo_is_read_from_the_remote(checkout: Paths, url: str) -> None: + run_git(checkout.repo, "remote", "set-url", "origin", url) + assert hotfix.github_repo(checkout) == "org/repo" + + +def test_the_token_travels_in_the_environment_not_the_command_line() -> None: + env = hotfix._auth_env("ghp_secret") + values = [v for k, v in env.items() if k.startswith("GIT_CONFIG_VALUE_")] + header = next(v for v in values if v.startswith("Authorization: Basic ")) + assert base64.b64decode(header.split()[-1]) == b"x-access-token:ghp_secret" + assert "git@github.com:" in values + assert hotfix._auth_env(None) == {}