From fcc6398f824556fce60de706fc8a6dcd0dbe94d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C4=ABlav=C4=81pi=20Cheesley?= Date: Tue, 22 Sep 2026 21:04:49 +0100 Subject: [PATCH 1/4] Add GitHub Issues as a Software Factory tracker Software Factory took work from Linear or Jira only. GitHub was wired in on the code side - branches, pull requests, reviews, pushes - but never as a source of work items, so a project whose backlog lives in GitHub Issues had no intake. GitHub has no status field, so the tracker reads the five status settings as label names and drives them as an exclusive group: setting one removes the others, including the trigger label, so a re-label opens a fresh build. Reaching the done label also closes the issue as completed, which is what a GitHub reader expects finished work to look like. Issue numbers repeat across repositories, so a ticket key is `owner/repo#number`. Intake reuses the existing GitHub webhook. `issues_labeled` publishes the same normalized `ticket.transitioned` fact Linear and Jira publish, so the funnel subscriber is untouched. The operator App is the tracker's identity; there is no second credential to connect, and no new permission either: Issues write was already on the App's list. A GitHub login is not an identity any grant issuer vouches for, so the tracker resolves no account from an assignee and every GitHub build uses the default account. The webhook publishes the login for display and leaves the id and address unset rather than guess at them. The App manifest now subscribes to the issues event. Without it an App created through Settings > Services would connect cleanly, report healthy, and never open a build. Co-Authored-By: Claude Opus 5 (1M context) --- backend/druks/contrib/software_factory/app.py | 24 ++- .../software_factory/ticketing/github.py | 75 +++++++++ backend/druks/core/apis/github.py | 43 +++++ backend/druks/core/services.py | 2 + backend/druks/core/webhooks/github.py | 37 ++++ .../tests/software_factory/test_ticketing.py | 159 +++++++++++++++++- .../test_webhooks_github_issues.py | 119 +++++++++++++ docs/configuration.md | 63 +++++-- 8 files changed, 500 insertions(+), 22 deletions(-) create mode 100644 backend/druks/contrib/software_factory/ticketing/github.py create mode 100644 backend/tests/software_factory/test_webhooks_github_issues.py diff --git a/backend/druks/contrib/software_factory/app.py b/backend/druks/contrib/software_factory/app.py index b03a9b41f..8e3c76f75 100644 --- a/backend/druks/contrib/software_factory/app.py +++ b/backend/druks/contrib/software_factory/app.py @@ -18,6 +18,7 @@ from druks.contrib.software_factory.ticketing.base import Tracker from druks.contrib.software_factory.ticketing.druks import DruksTracker from druks.contrib.software_factory.ticketing.enums import TicketStatus +from druks.contrib.software_factory.ticketing.github import GitHub from druks.contrib.software_factory.ticketing.jira import Jira from druks.contrib.software_factory.ticketing.linear import Linear from druks.core import services @@ -37,7 +38,11 @@ async def check_tracker_identity() -> CheckResult: return CheckResult(name="tracker", ok=True, detail="trackerless by choice") if settings.tracker == "druks": return CheckResult(name="tracker", ok=True, detail="this appliance") - service = {"linear": services.Linear, "jira": services.Jira}[settings.tracker] + # The operator App is the GitHub tracker's identity; there is no second + # credential to connect. + service = {"linear": services.Linear, "jira": services.Jira, "github": services.Github}[ + settings.tracker + ] if await service.is_connected(): return CheckResult(name="tracker", ok=True, detail=f"{settings.tracker} connected") return CheckResult( @@ -86,7 +91,7 @@ class SoftwareFactory(App): ) class Settings(AppSettings): - tracker: Literal["none", "linear", "jira", "druks"] = Field( + tracker: Literal["none", "linear", "jira", "druks", "github"] = Field( default="linear", title="Tracker", description="Which ticket tracker this installation uses.", @@ -97,7 +102,7 @@ class Settings(AppSettings): description="A ticket entering this status opens a build.", json_schema_extra={ "section": "Statuses", - "visible_when": {"tracker": ["linear", "jira"]}, + "visible_when": {"tracker": ["linear", "jira", "github"]}, }, ) in_progress_status: Annotated[str, Choices(list_tracker_status_choices)] = Field( @@ -106,7 +111,7 @@ class Settings(AppSettings): description="The status of a ticket while a build works on it.", json_schema_extra={ "section": "Statuses", - "visible_when": {"tracker": ["linear", "jira"]}, + "visible_when": {"tracker": ["linear", "jira", "github"]}, }, ) in_review_status: Annotated[str, Choices(list_tracker_status_choices)] = Field( @@ -116,7 +121,7 @@ class Settings(AppSettings): "Empty leaves the ticket where it is.", json_schema_extra={ "section": "Statuses", - "visible_when": {"tracker": ["linear", "jira"]}, + "visible_when": {"tracker": ["linear", "jira", "github"]}, }, ) done_status: Annotated[str, Choices(list_tracker_status_choices)] = Field( @@ -125,7 +130,7 @@ class Settings(AppSettings): description="The status of a ticket after its pull request merges.", json_schema_extra={ "section": "Statuses", - "visible_when": {"tracker": ["linear", "jira"]}, + "visible_when": {"tracker": ["linear", "jira", "github"]}, }, ) resting_status: Annotated[str, Choices(list_tracker_status_choices)] = Field( @@ -135,7 +140,7 @@ class Settings(AppSettings): "Empty leaves the ticket where it is.", json_schema_extra={ "section": "Statuses", - "visible_when": {"tracker": ["linear", "jira"]}, + "visible_when": {"tracker": ["linear", "jira", "github"]}, }, ) @@ -169,6 +174,11 @@ async def get_tracker(cls, source: str | None = None) -> Tracker | None: api_token=row.secrets["api_token"], status_names=status_names, ) + if settings.tracker == "github": + # Presence check only: the client resolves App credentials per + # repository installation, not from this row. + await services.Github.get() + return GitHub(status_names=status_names) except ServiceNotConnectedError: return diff --git a/backend/druks/contrib/software_factory/ticketing/github.py b/backend/druks/contrib/software_factory/ticketing/github.py new file mode 100644 index 000000000..b7675e47b --- /dev/null +++ b/backend/druks/contrib/software_factory/ticketing/github.py @@ -0,0 +1,75 @@ +import httpx + +from druks.core.apis.exceptions import UnknownTicketError +from druks.core.apis.github import GitHubClient +from druks.core.services import Github + +from .base import Tracker +from .enums import TicketStatus + + +def split_key(key: str) -> tuple[str, int]: + """``owner/repo#123`` -> ``("owner/repo", 123)``. + + GitHub issue numbers are only unique within a repository, so the repo has to + travel in the key itself. Linear and Jira keys are globally unique in their + workspace and need no such qualifier. + """ + repo, _, number = key.partition("#") + if not repo or not number.isdigit(): + raise UnknownTicketError(key, "GitHub") + return repo, int(number) + + +class GitHub(Tracker): + """Drives a GitHub issue through the build funnel using labels. + + GitHub has no status field, so each configured status name is a label name. + The labels behave as an exclusive group: setting one removes the others. + That exclusivity is what makes them act like the status column Linear and + Jira give us for free - and it is why the trigger label comes off as soon as + a build claims the issue, so a later re-label opens a fresh build. + """ + + known_exceptions = (UnknownTicketError, httpx.HTTPError) + + def __init__( + self, + *, + status_names: dict[TicketStatus, str], + client: GitHubClient | None = None, + ) -> None: + self._client = client + # An empty name leaves that status unmapped. + self._status_names = {status: name for status, name in status_names.items() if name} + + async def _gh(self) -> GitHubClient: + # The operator App is the tracker's identity, resolved lazily so + # constructing a tracker never needs a connected service. + if self._client is None: + self._client = await Github.get_client() + return self._client + + async def get_account_id(self, user_id: str) -> str | None: + # No grant issuer vouches for a GitHub login, so it cannot resolve to a + # Druks account the way a Linear or Jira user id does. + return None + + async def set_status(self, key: str, status: TicketStatus) -> None: + label = self._status_names.get(status) + if not label: + raise ValueError(f"GitHub has no configured label for {status}") + repo, number = split_key(key) + client = await self._gh() + # Exclusive group: drop every other label this tracker owns before + # adding the new one, so an issue never reads as two states at once. + stale = {name for name in self._status_names.values() if name != label} + await client.replace_issue_labels(repo, number, add=label, remove=stale) + if status is TicketStatus.DONE: + # The label alone would leave finished work in the open-issue list, + # which is not what a GitHub reader expects it to look like. + await client.set_issue_state(repo, number, state="closed", state_reason="completed") + + async def aclose(self) -> None: + # A client we were handed belongs to the caller; only close our own. + return diff --git a/backend/druks/core/apis/github.py b/backend/druks/core/apis/github.py index 7354ef9ab..f2d4a814f 100644 --- a/backend/druks/core/apis/github.py +++ b/backend/druks/core/apis/github.py @@ -379,6 +379,49 @@ async def reply_to_review_comment( ) return response.parsed_data.model_dump() + @_retry_on_401 + async def replace_issue_labels( + self, + repo: str, + issue_number: int, + *, + add: str, + remove: set[str], + ) -> None: + """Add one label and drop a set of others, for callers that treat a group + of labels as mutually exclusive. + + Removal is per-label and tolerates a 404: GitHub returns one when the + label isn't on the issue, which is the normal case and not a failure. + """ + owner, name = repo.split("/", 1) + github = await self._for_repo(repo) + await github.rest.issues.async_add_labels(owner, name, issue_number, data=[add]) + for label in remove: + try: + await github.rest.issues.async_remove_label(owner, name, issue_number, label) + except RequestFailed as exc: + if exc.response.status_code != 404: + raise + + @_retry_on_401 + async def set_issue_state( + self, + repo: str, + issue_number: int, + *, + state: str, + state_reason: str | None = None, + ) -> None: + """Open or close an issue. ``state_reason`` distinguishes a completed + close from a not-planned one, which GitHub renders differently.""" + owner, name = repo.split("/", 1) + github = await self._for_repo(repo) + payload: dict[str, Any] = {"state": state} + if state_reason: + payload["state_reason"] = state_reason + await github.rest.issues.async_update(owner, name, issue_number, **payload) + @_retry_on_401 async def get_pull_request(self, repo: str, pr_number: int) -> dict[str, Any]: owner, name = repo.split("/", 1) diff --git a/backend/druks/core/services.py b/backend/druks/core/services.py index d7bee86db..e123057ad 100644 --- a/backend/druks/core/services.py +++ b/backend/druks/core/services.py @@ -55,6 +55,8 @@ class Github(Service): "public": False, "default_events": [ "issue_comment", + # Issue labelling is the GitHub tracker's intake signal. + "issues", "pull_request", "pull_request_review", "pull_request_review_comment", diff --git a/backend/druks/core/webhooks/github.py b/backend/druks/core/webhooks/github.py index 71e1b2b7f..c3dc35ce7 100644 --- a/backend/druks/core/webhooks/github.py +++ b/backend/druks/core/webhooks/github.py @@ -109,6 +109,43 @@ async def _publish_comment( }, ) + async def on_issues_labeled(self) -> Response: + """A label landing on an issue is GitHub's nearest thing to a status + transition, so it is published as one. + + No sender filter: the trigger label is the only status a subscriber acts + on, and druks setting it through the tracker API is the intended way to + open a build - exactly as moving a Linear ticket is. Every other label + this app writes carries a non-trigger status, so its echo is inert, and a + redelivery resolves to the same work item. + """ + issue, repository = self.data["issue"], self.data["repository"] + # GitHub issue numbers repeat across repositories, so the repo travels + # in the identifier. It also makes the key self-routing. + identifier = f"{repository['full_name']}#{issue['number']}" + assignee = issue.get("assignee") or {} + await publish( + "ticket.transitioned", + payload={ + "source": "github", + "identifier": identifier, + "status": self.data["label"]["name"], + "title": issue["title"], + "url": issue["html_url"], + # The bare repo name routes to a ProjectRepo the same way a + # Linear project name does. + "project_name": repository["name"], + "labels": [label["name"] for label in issue["labels"]], + # A GitHub webhook carries a login, which is neither an address + # nor an identity any grant issuer vouches for. Leave both unset + # rather than guess; the name is for display only. + "assignee_id": None, + "assignee_email": None, + "assignee_name": assignee.get("login"), + }, + ) + return _accepted() + async def on_pull_request_closed(self) -> Response: pull_request = self.data["pull_request"] merged = pull_request["merged"] diff --git a/backend/tests/software_factory/test_ticketing.py b/backend/tests/software_factory/test_ticketing.py index a8ffd7f21..845890e7c 100644 --- a/backend/tests/software_factory/test_ticketing.py +++ b/backend/tests/software_factory/test_ticketing.py @@ -11,6 +11,7 @@ ) from druks.contrib.software_factory.ticketing.druks import DruksTracker from druks.contrib.software_factory.ticketing.enums import TicketStatus +from druks.contrib.software_factory.ticketing.github import GitHub from druks.contrib.software_factory.ticketing.jira import Jira from druks.contrib.software_factory.ticketing.linear import Linear from druks.core import services @@ -39,6 +40,14 @@ async def _connect_linear(): ) +async def _connect_github(): + return await connect_service( + "github", + identity={"slug": "druks", "app_id": "1"}, + secrets={"app_id": "1", "private_key": "pem", "webhook_secret": "gh-hook"}, + ) + + async def _connect_jira(): return await connect_service( "jira", @@ -265,11 +274,14 @@ async def test_tracker_check_accepts_the_board_without_a_service(monkeypatch): assert not result.pending -def test_status_settings_show_for_linear_and_jira_with_the_selected_tracker_statuses(): +def test_status_settings_show_for_every_tracker_that_names_its_own_statuses(): fields = SoftwareFactory.Settings.model_fields - assert field_choices(fields["tracker"]) == ["none", "linear", "jira", "druks"] + assert field_choices(fields["tracker"]) == ["none", "linear", "jira", "druks", "github"] for status in ("trigger", "in_progress", "in_review", "done", "resting"): - assert field_visibility(fields[f"{status}_status"]) == ("tracker", ["linear", "jira"]) + assert field_visibility(fields[f"{status}_status"]) == ( + "tracker", + ["linear", "jira", "github"], + ) assert field_live_choices(fields[f"{status}_status"]).source is list_tracker_status_choices @@ -668,3 +680,144 @@ def handler(request: httpx.Request) -> httpx.Response: assert [status["name"] for status in statuses] == ["To Do", "Done"] # The status search endpoint needs Jira admin rights. This one needs Browse projects. assert paths == ["/rest/api/3/status"] + + +class _FakeGitHubClient: + """Records the client calls the tracker delegates to - no HTTP.""" + + def __init__(self) -> None: + self.calls: list = [] + + async def replace_issue_labels(self, repo, issue_number, *, add, remove): + self.calls.append(("labels", repo, issue_number, add, sorted(remove))) + + async def set_issue_state(self, repo, issue_number, *, state, state_reason=None): + self.calls.append(("state", repo, issue_number, state, state_reason)) + + +def _github_tracker(**status_names): + fake = _FakeGitHubClient() + names = { + TicketStatus.TRIGGER: "ready-for-agent", + TicketStatus.IN_PROGRESS: "agent:in-progress", + TicketStatus.IN_REVIEW: "agent:in-review", + TicketStatus.DONE: "agent:done", + TicketStatus.BACKLOG: "", + } + names.update({TicketStatus(status): name for status, name in status_names.items()}) + return GitHub(status_names=names, client=fake), fake + + +async def test_github_set_status_swaps_the_label_within_an_exclusive_group(): + tracker, fake = _github_tracker() + + await tracker.set_status("acme/widget#7", TicketStatus.IN_PROGRESS) + + # The new label goes on; every other label this tracker owns comes off, so + # an issue never reads as two states at once. + assert fake.calls == [ + ( + "labels", + "acme/widget", + 7, + "agent:in-progress", + ["agent:done", "agent:in-review", "ready-for-agent"], + ) + ] + + +async def test_github_set_status_clears_the_trigger_label_so_a_relabel_can_retrigger(): + tracker, fake = _github_tracker() + + await tracker.set_status("acme/widget#7", TicketStatus.IN_PROGRESS) + + _, _, _, _, removed = fake.calls[0] + assert "ready-for-agent" in removed + + +async def test_github_done_closes_the_issue_as_completed(): + tracker, fake = _github_tracker() + + await tracker.set_status("acme/widget#7", TicketStatus.DONE) + + assert ("state", "acme/widget", 7, "closed", "completed") in fake.calls + + +async def test_github_non_terminal_status_leaves_the_issue_open(): + tracker, fake = _github_tracker() + + await tracker.set_status("acme/widget#7", TicketStatus.IN_REVIEW) + + assert not [call for call in fake.calls if call[0] == "state"] + + +async def test_github_an_unmapped_status_raises(): + tracker, _ = _github_tracker() + + # BACKLOG is unnamed by default: an empty resting label leaves the issue put. + with pytest.raises(ValueError, match="no configured label"): + await tracker.set_status("acme/widget#7", TicketStatus.BACKLOG) + + +async def test_github_rejects_a_key_without_a_repo_qualifier(): + tracker, _ = _github_tracker() + + # Issue numbers repeat across repositories, so a bare number is ambiguous + # and must not be guessed at. + with pytest.raises(UnknownTicketError, match="doesn't exist in GitHub"): + await tracker.set_status("7", TicketStatus.DONE) + with pytest.raises(UnknownTicketError): + await tracker.set_status("acme/widget#not-a-number", TicketStatus.DONE) + + +async def test_github_does_not_resolve_a_login_to_an_account(): + # No grant issuer vouches for a GitHub login, so it never selects an account. + tracker, _ = _github_tracker() + + assert await tracker.get_account_id("octocat") is None + + +def test_github_declares_known_exceptions(): + assert UnknownTicketError in GitHub.known_exceptions + assert httpx.HTTPError in GitHub.known_exceptions + + +async def test_tracker_builds_github_from_the_operator_app(monkeypatch): + await _connect_github() + _pin_software_factory_settings( + monkeypatch, + tracker="github", + trigger_status="ready-for-agent", + resting_status="needs-triage", + ) + + tracker = await SoftwareFactory.get_tracker("github") + + assert isinstance(tracker, GitHub) + assert tracker._status_names[TicketStatus.TRIGGER] == "ready-for-agent" + assert tracker._status_names[TicketStatus.BACKLOG] == "needs-triage" + + +async def test_github_tracker_check_reports_the_operator_app(monkeypatch): + await _connect_github() + _pin_software_factory_settings(monkeypatch, tracker="github") + + result = await check_tracker_identity() + + assert result.ok + assert "github" in result.detail + + +async def test_github_tracker_check_pends_without_the_operator_app(monkeypatch): + _pin_software_factory_settings(monkeypatch, tracker="github") + + result = await check_tracker_identity() + + assert not result.ok + assert result.pending + + +def test_github_manifest_subscribes_to_issue_events(): + # The tracker's intake is the issues event. An App created from this + # manifest without it would connect cleanly and then never open a build. + assert "issues" in services.Github.manifest["default_events"] diff --git a/backend/tests/software_factory/test_webhooks_github_issues.py b/backend/tests/software_factory/test_webhooks_github_issues.py new file mode 100644 index 000000000..291ff7679 --- /dev/null +++ b/backend/tests/software_factory/test_webhooks_github_issues.py @@ -0,0 +1,119 @@ +from types import SimpleNamespace +from typing import Any, cast + +from druks.core.webhooks import github as github_webhooks +from druks.core.webhooks.github import GitHubEvents +from druks.testing import make_settings + + +def _events(tmp_path, *, payload, event="issues"): + events = GitHubEvents( + request=cast(Any, SimpleNamespace(headers={"x-github-event": event})), + kwargs={}, + settings=make_settings(tmp_path), + ) + events._data_cached = payload + return events + + +def _labeled(*, number=7, label="ready-for-agent", labels=None, assignee=None): + return { + "action": "labeled", + "label": {"name": label}, + "issue": { + "number": number, + "title": "Contacts import drops the last row", + "html_url": f"https://github.com/acme/widget/issues/{number}", + "labels": [{"name": name} for name in (labels or [label])], + "assignee": assignee, + }, + "repository": {"full_name": "acme/widget", "name": "widget"}, + } + + +def _capture(monkeypatch): + published = [] + + async def _emit(event_type, **kwargs): + published.append((event_type, kwargs["payload"])) + + monkeypatch.setattr(github_webhooks, "publish", _emit) + return published + + +def test_a_labelled_issue_dispatches_to_the_issues_handler(tmp_path): + # get_action composes event + action, so `issues` + `labeled` has to land on + # on_issues_labeled for any of this to run. + assert _events(tmp_path, payload=_labeled()).get_action() == "issues_labeled" + + +async def test_labelling_publishes_a_ticket_transition(tmp_path, monkeypatch): + published = _capture(monkeypatch) + + await _events(tmp_path, payload=_labeled()).on_issues_labeled() + + assert len(published) == 1 + event_type, payload = published[0] + assert event_type == "ticket.transitioned" + assert payload["source"] == "github" + assert payload["status"] == "ready-for-agent" + assert payload["title"] == "Contacts import drops the last row" + assert payload["url"] == "https://github.com/acme/widget/issues/7" + + +async def test_the_identifier_carries_the_repository(tmp_path, monkeypatch): + # Issue numbers are only unique within a repo, so the key has to say which. + published = _capture(monkeypatch) + + await _events(tmp_path, payload=_labeled(number=42)).on_issues_labeled() + + assert published[0][1]["identifier"] == "acme/widget#42" + + +async def test_the_bare_repo_name_routes_to_a_project_repo(tmp_path, monkeypatch): + # ProjectRepo.lookup matches a bare name, the way it does a Linear project. + published = _capture(monkeypatch) + + await _events(tmp_path, payload=_labeled()).on_issues_labeled() + + assert published[0][1]["project_name"] == "widget" + + +async def test_every_label_on_the_issue_travels_for_routing(tmp_path, monkeypatch): + published = _capture(monkeypatch) + + payload = _labeled(labels=["ready-for-agent", "widget", "bug"]) + await _events(tmp_path, payload=payload).on_issues_labeled() + + assert published[0][1]["labels"] == ["ready-for-agent", "widget", "bug"] + + +async def test_an_unassigned_issue_publishes_no_assignee(tmp_path, monkeypatch): + published = _capture(monkeypatch) + + await _events(tmp_path, payload=_labeled(assignee=None)).on_issues_labeled() + + assert published[0][1]["assignee_name"] is None + assert published[0][1]["assignee_email"] is None + + +async def test_an_assigned_issue_publishes_the_login_but_never_an_email(tmp_path, monkeypatch): + # GitHub webhooks carry no address; inventing one would misattribute a run. + published = _capture(monkeypatch) + + payload = _labeled(assignee={"login": "octocat"}) + await _events(tmp_path, payload=payload).on_issues_labeled() + + assert published[0][1]["assignee_name"] == "octocat" + assert published[0][1]["assignee_email"] is None + + +async def test_an_assignee_never_selects_an_account(tmp_path, monkeypatch): + # assignee_id drives the account lookup. A GitHub login is not an identity + # any grant issuer vouches for, so the build must not resolve one from it. + published = _capture(monkeypatch) + + payload = _labeled(assignee={"login": "octocat", "id": 583231}) + await _events(tmp_path, payload=payload).on_issues_labeled() + + assert published[0][1]["assignee_id"] is None diff --git a/docs/configuration.md b/docs/configuration.md index 943f258f8..7674be660 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -402,10 +402,11 @@ Webhook URL: Callback URL: `/api/oauth/callback` -Subscribe to issue comment, pull request, pull request review, pull request -review comment, and push events. Keep **Expire user authorization tokens** on: -Druks refreshes a person's sign-in with the refresh token that comes with it, -and refuses a sign-in that has none. +Subscribe to issues, issue comment, pull request, pull request review, pull +request review comment, and push events. The issues event is what the GitHub +tracker reads; an App that omits it receives no intake. Keep **Expire user +authorization tokens** on: Druks refreshes a person's sign-in with the refresh +token that comes with it, and refuses a sign-in that has none. | Repository permission | Access | | --- | --- | @@ -490,16 +491,21 @@ Select the tracker in **Software Factory → Settings**. The default is Linear. **Linear** and **Jira** are service identities. Connect them from **Settings → Connections → Services**. The Linear identity uses an API key and webhook secret. The Jira identity uses a base URL, email, API token, and -webhook secret. Druks validates the credentials before it stores them. Linear -and Jira share five status settings: trigger, in progress, in review, done, and -resting. When the selected tracker is connected, the settings page lists its -statuses. After you change the tracker, save the settings to list the statuses of -the new tracker. An empty in review or resting status leaves the ticket where it is. +webhook secret. Druks validates the credentials before it stores them. Linear, +Jira, and GitHub share five status settings: trigger, in progress, in review, +done, and resting. When the selected tracker is connected and lists its statuses, +the settings page offers them as choices. After you change the tracker, save the +settings to list the statuses of the new tracker. An empty in review or resting +status leaves the ticket where it is. Select **druks** to use the ticket board on this appliance. That choice needs no credentials. The status settings stay hidden. The trigger status is Ready for Agent, and it is not a setting. +Select **GitHub** to take work from GitHub Issues in the repositories the +operator GitHub App is already installed on. That choice needs no second +credential. See [GitHub Issues as the tracker](#github-issues-as-the-tracker). + The dashboard shows the board and the ticket pages only for **druks**. Each ticket selects a GitHub repository from a Software Factory project. Druks derives each project's ticket prefix from the project name. It takes the first two @@ -530,9 +536,42 @@ Druks accepts the REST issue JSON under `issue`. Put the shared token in the `x-druks-webhook-token` header. Put `{{initiator.accountId}}` in the `x-jira-initiator` header, so Druks knows who moved the ticket. See [Jira's user smart values](https://support.atlassian.com/cloud-automation/docs/jira-smart-values-users/). -`druks doctor` treats a disconnected Linear or Jira identity as optional when -that tracker is not selected. It reports pending setup if the selected tracker -is Linear or Jira and that identity is missing. +`druks doctor` treats a disconnected tracker identity as optional when that +tracker is not selected. It reports pending setup if the selected tracker is +Linear, Jira, or GitHub and its identity is missing. + +### GitHub Issues as the tracker + +GitHub has no status field, so the GitHub tracker reads the five status settings +as **label names** and uses the existing `/_external/github/events/` webhook as +its intake. An issue that gains the trigger label opens a build. + +Set the five labels in **Software Factory → Settings**. They default to the +Linear status names, so name them for GitHub before you select this tracker - for +example `ready-for-agent`, `agent:in-progress`, `agent:in-review`, and +`agent:done`. Create each label in the repository first: GitHub rejects a label +that does not exist. An empty in review or resting label leaves the issue where +it is, as it does on Linear and Jira. + +The labels behave as one exclusive group. Setting any of them removes the others, +including the trigger label - so an issue never reads as two states at once, and +re-applying the trigger label opens a fresh build. Reaching the done label also +closes the issue as completed, which is what a GitHub reader expects finished +work to look like. + +Three differences from Linear and Jira are worth knowing. A ticket key is +`owner/repo#number`, because issue numbers repeat across repositories. The +settings page lists no label choices, because labels are per repository rather +than per workspace, so you type them. And a build never runs as the issue +assignee: a GitHub login is not an identity any grant issuer vouches for, so +every GitHub build uses the default account. + +The App must subscribe to the **issues** event. An App created through +**Settings → Connections → Services → Create GitHub App** subscribes to it. An +older App, or one registered by hand, needs the event adding before any build +opens. Adding it needs no permission change: **Issues: Read and write** is +already on the App's list and it covers the labelling and closing this tracker +does, so no installation has to approve anything. Software Factory starts a Linear or Jira build under the Druks account of the person who moved the ticket. If that person has no account, the build uses the From b9735d881e70682647f4929c71fc0c811f169efd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C4=ABlav=C4=81pi=20Cheesley?= Date: Fri, 2 Oct 2026 11:08:18 +0100 Subject: [PATCH 2/4] Move GitHub issue intake into Software Factory and harden it Core now publishes a neutral issue.labeled fact, and a Software Factory subscriber turns it into ticket.transitioned, the way Linear and Jira intake already lives in the app. Core publishes a label only when a person adds it or the App adds it itself: the start route asks for a build by setting the trigger label through the App, so its own label has to reach the funnel. Any other App's label is ignored. The ticket routes by the issue's full repo name, which matches one repo exactly. A bare name, or a label, could match the same name under another owner, so neither routes a GitHub ticket. The start route takes a GitHub key: owner/repo#12 has a slash, so the ticket segment is a path. The tracker catches githubkit's RequestFailed and GitHubAppNotInstalledError, so a failed label write no longer fails the webhook that caused it. It closes the client it creates, takes no client argument, and leaves account lookup to the base class. The docs now say GitHub creates a missing label rather than rejecting it. Co-Authored-By: Claude Opus 5.5 --- .../druks/contrib/software_factory/routes.py | 6 +- .../contrib/software_factory/subscribers.py | 27 ++- .../software_factory/ticketing/github.py | 29 ++- backend/druks/core/webhooks/github.py | 59 +++--- .../tests/software_factory/test_ticketing.py | 58 ++++-- .../test_webhooks_github_issues.py | 173 ++++++++++++------ backend/tests/test_agent_routes.py | 12 ++ backend/tests/test_mcp_endpoint.py | 2 +- docs/configuration.md | 16 +- 9 files changed, 247 insertions(+), 135 deletions(-) diff --git a/backend/druks/contrib/software_factory/routes.py b/backend/druks/contrib/software_factory/routes.py index d782df04b..e52d78533 100644 --- a/backend/druks/contrib/software_factory/routes.py +++ b/backend/druks/contrib/software_factory/routes.py @@ -272,7 +272,9 @@ async def list_work_items_history( @work_items_router.post( - "/{ticket}/start", + # ``path`` lets a GitHub key, ``owner/repo#12``, through: its ``/`` would end a + # plain segment. The caller encodes the ``#`` as ``%23``. + "/{ticket:path}/start", status_code=status.HTTP_202_ACCEPTED, operation_id="start", tags=["agent"], @@ -281,7 +283,7 @@ async def list_work_items_history( async def start_work_item( ticket: str = Path( ..., - description="The tracker's ticket key, e.g. ENG-831.", + description="The tracker's ticket key, e.g. ENG-831, or owner/repo#12 for GitHub.", ), account: Account = Depends(current_account), ) -> None: diff --git a/backend/druks/contrib/software_factory/subscribers.py b/backend/druks/contrib/software_factory/subscribers.py index da453a188..bc2877993 100644 --- a/backend/druks/contrib/software_factory/subscribers.py +++ b/backend/druks/contrib/software_factory/subscribers.py @@ -5,7 +5,7 @@ from druks.contrib.software_factory.ticketing.enums import TicketStatus from druks.contrib.software_factory.workflows import Build, Profile from druks.models import Base -from druks.signals import subscribe +from druks.signals import publish, subscribe from druks.workflows import WorkflowEvent @@ -78,6 +78,31 @@ async def pr_close_settles_the_item(*, repo: str, pr_number: int, payload: dict) await item.resolve(resolution, at=payload["resolved_at"]) +@subscribe("issue.labeled") +async def issue_label_transitions_the_ticket(*, repo: str, number: int, payload: dict) -> None: + """GitHub has no status field, so the GitHub tracker reads a label as one. Issue + numbers repeat across repos, so the key carries the repo. The issue's own repo is + the build's repo: the full name routes to exactly it, and no label routes it + anywhere else.""" + await publish( + "ticket.transitioned", + payload={ + "source": "github", + "identifier": f"{repo}#{number}", + "status": payload["label"], + "title": payload["title"], + "url": payload["url"], + "project_name": repo, + "labels": [], + # A login is not an address, and no grant issuer vouches for it, so it + # selects no account. The name is for display only. + "assignee_id": None, + "assignee_email": None, + "assignee_name": payload["assignee"], + }, + ) + + @subscribe("ticket.transitioned") async def ticket_transition_drives_the_funnel(*, payload: dict) -> None: """Dispatch a build when a ticket from the chosen tracker enters its trigger status.""" diff --git a/backend/druks/contrib/software_factory/ticketing/github.py b/backend/druks/contrib/software_factory/ticketing/github.py index b7675e47b..abe5cb3b1 100644 --- a/backend/druks/contrib/software_factory/ticketing/github.py +++ b/backend/druks/contrib/software_factory/ticketing/github.py @@ -1,6 +1,7 @@ import httpx +from githubkit.exception import RequestFailed -from druks.core.apis.exceptions import UnknownTicketError +from druks.core.apis.exceptions import GitHubAppNotInstalledError, UnknownTicketError from druks.core.apis.github import GitHubClient from druks.core.services import Github @@ -31,15 +32,15 @@ class GitHub(Tracker): a build claims the issue, so a later re-label opens a fresh build. """ - known_exceptions = (UnknownTicketError, httpx.HTTPError) + known_exceptions = ( + UnknownTicketError, + GitHubAppNotInstalledError, + RequestFailed, + httpx.HTTPError, + ) - def __init__( - self, - *, - status_names: dict[TicketStatus, str], - client: GitHubClient | None = None, - ) -> None: - self._client = client + def __init__(self, *, status_names: dict[TicketStatus, str]) -> None: + self._client: GitHubClient | None = None # An empty name leaves that status unmapped. self._status_names = {status: name for status, name in status_names.items() if name} @@ -50,11 +51,6 @@ async def _gh(self) -> GitHubClient: self._client = await Github.get_client() return self._client - async def get_account_id(self, user_id: str) -> str | None: - # No grant issuer vouches for a GitHub login, so it cannot resolve to a - # Druks account the way a Linear or Jira user id does. - return None - async def set_status(self, key: str, status: TicketStatus) -> None: label = self._status_names.get(status) if not label: @@ -71,5 +67,6 @@ async def set_status(self, key: str, status: TicketStatus) -> None: await client.set_issue_state(repo, number, state="closed", state_reason="completed") async def aclose(self) -> None: - # A client we were handed belongs to the caller; only close our own. - return + if self._client is not None: + await self._client.aclose() + self._client = None diff --git a/backend/druks/core/webhooks/github.py b/backend/druks/core/webhooks/github.py index c3dc35ce7..657d1c644 100644 --- a/backend/druks/core/webhooks/github.py +++ b/backend/druks/core/webhooks/github.py @@ -110,40 +110,23 @@ async def _publish_comment( ) async def on_issues_labeled(self) -> Response: - """A label landing on an issue is GitHub's nearest thing to a status - transition, so it is published as one. - - No sender filter: the trigger label is the only status a subscriber acts - on, and druks setting it through the tracker API is the intended way to - open a build - exactly as moving a Linear ticket is. Every other label - this app writes carries a non-trigger status, so its echo is inert, and a - redelivery resolves to the same work item. - """ - issue, repository = self.data["issue"], self.data["repository"] - # GitHub issue numbers repeat across repositories, so the repo travels - # in the identifier. It also makes the key self-routing. - identifier = f"{repository['full_name']}#{issue['number']}" - assignee = issue.get("assignee") or {} - await publish( - "ticket.transitioned", - payload={ - "source": "github", - "identifier": identifier, - "status": self.data["label"]["name"], - "title": issue["title"], - "url": issue["html_url"], - # The bare repo name routes to a ProjectRepo the same way a - # Linear project name does. - "project_name": repository["name"], - "labels": [label["name"] for label in issue["labels"]], - # A GitHub webhook carries a login, which is neither an address - # nor an identity any grant issuer vouches for. Leave both unset - # rather than guess; the name is for display only. - "assignee_id": None, - "assignee_email": None, - "assignee_name": assignee.get("login"), - }, - ) + """Publish a label a person put on an issue. The App's own labels publish + too: setting a label through the API is how Druks itself asks for one. Any + other app is not acting for anyone here.""" + sender, issue = self.data["sender"], self.data["issue"] + if sender["type"] == "User" or await _is_own_app(sender): + assignee = issue.get("assignee") or {} + await publish( + "issue.labeled", + repo=_repo_name(self.data), + number=issue["number"], + payload={ + "label": self.data["label"]["name"], + "title": issue["title"], + "url": issue["html_url"], + "assignee": assignee.get("login"), + }, + ) return _accepted() async def on_pull_request_closed(self) -> Response: @@ -190,3 +173,11 @@ def _accepted() -> Response: def _repo_name(payload: dict[str, Any]) -> str: return payload["repository"]["full_name"] + + +async def _is_own_app(sender: dict[str, Any]) -> bool: + if sender["type"] != "Bot": + return False + # GitHub names an App's bot user after the App's slug. + slug = (await Github.get()).identity["slug"] + return sender["login"] == f"{slug}[bot]" diff --git a/backend/tests/software_factory/test_ticketing.py b/backend/tests/software_factory/test_ticketing.py index 845890e7c..0e255ff0d 100644 --- a/backend/tests/software_factory/test_ticketing.py +++ b/backend/tests/software_factory/test_ticketing.py @@ -15,10 +15,16 @@ from druks.contrib.software_factory.ticketing.jira import Jira from druks.contrib.software_factory.ticketing.linear import Linear from druks.core import services -from druks.core.apis.exceptions import JiraAPIError, LinearAPIError, UnknownTicketError +from druks.core.apis.exceptions import ( + GitHubAppNotInstalledError, + JiraAPIError, + LinearAPIError, + UnknownTicketError, +) from druks.core.apis.jira import JiraClient from druks.core.apis.linear import LinearClient from druks.services import ServiceConnectError +from githubkit.exception import RequestFailed from software_factory.factories import make_test_work_item @@ -694,9 +700,17 @@ async def replace_issue_labels(self, repo, issue_number, *, add, remove): async def set_issue_state(self, repo, issue_number, *, state, state_reason=None): self.calls.append(("state", repo, issue_number, state, state_reason)) + async def aclose(self): + self.calls.append(("aclose",)) + -def _github_tracker(**status_names): +def _github_tracker(monkeypatch, **status_names): fake = _FakeGitHubClient() + + async def get_client(cls): + return fake + + monkeypatch.setattr(services.Github, "get_client", classmethod(get_client)) names = { TicketStatus.TRIGGER: "ready-for-agent", TicketStatus.IN_PROGRESS: "agent:in-progress", @@ -705,11 +719,11 @@ def _github_tracker(**status_names): TicketStatus.BACKLOG: "", } names.update({TicketStatus(status): name for status, name in status_names.items()}) - return GitHub(status_names=names, client=fake), fake + return GitHub(status_names=names), fake -async def test_github_set_status_swaps_the_label_within_an_exclusive_group(): - tracker, fake = _github_tracker() +async def test_github_set_status_swaps_the_label_within_an_exclusive_group(monkeypatch): + tracker, fake = _github_tracker(monkeypatch) await tracker.set_status("acme/widget#7", TicketStatus.IN_PROGRESS) @@ -726,8 +740,8 @@ async def test_github_set_status_swaps_the_label_within_an_exclusive_group(): ] -async def test_github_set_status_clears_the_trigger_label_so_a_relabel_can_retrigger(): - tracker, fake = _github_tracker() +async def test_github_set_status_clears_the_trigger_label_so_a_relabel_can_retrigger(monkeypatch): + tracker, fake = _github_tracker(monkeypatch) await tracker.set_status("acme/widget#7", TicketStatus.IN_PROGRESS) @@ -735,32 +749,32 @@ async def test_github_set_status_clears_the_trigger_label_so_a_relabel_can_retri assert "ready-for-agent" in removed -async def test_github_done_closes_the_issue_as_completed(): - tracker, fake = _github_tracker() +async def test_github_done_closes_the_issue_as_completed(monkeypatch): + tracker, fake = _github_tracker(monkeypatch) await tracker.set_status("acme/widget#7", TicketStatus.DONE) assert ("state", "acme/widget", 7, "closed", "completed") in fake.calls -async def test_github_non_terminal_status_leaves_the_issue_open(): - tracker, fake = _github_tracker() +async def test_github_non_terminal_status_leaves_the_issue_open(monkeypatch): + tracker, fake = _github_tracker(monkeypatch) await tracker.set_status("acme/widget#7", TicketStatus.IN_REVIEW) assert not [call for call in fake.calls if call[0] == "state"] -async def test_github_an_unmapped_status_raises(): - tracker, _ = _github_tracker() +async def test_github_an_unmapped_status_raises(monkeypatch): + tracker, _ = _github_tracker(monkeypatch) # BACKLOG is unnamed by default: an empty resting label leaves the issue put. with pytest.raises(ValueError, match="no configured label"): await tracker.set_status("acme/widget#7", TicketStatus.BACKLOG) -async def test_github_rejects_a_key_without_a_repo_qualifier(): - tracker, _ = _github_tracker() +async def test_github_rejects_a_key_without_a_repo_qualifier(monkeypatch): + tracker, _ = _github_tracker(monkeypatch) # Issue numbers repeat across repositories, so a bare number is ambiguous # and must not be guessed at. @@ -770,15 +784,21 @@ async def test_github_rejects_a_key_without_a_repo_qualifier(): await tracker.set_status("acme/widget#not-a-number", TicketStatus.DONE) -async def test_github_does_not_resolve_a_login_to_an_account(): - # No grant issuer vouches for a GitHub login, so it never selects an account. - tracker, _ = _github_tracker() +async def test_github_closes_the_client_it_created(monkeypatch): + tracker, fake = _github_tracker(monkeypatch) + + async with tracker: + await tracker.set_status("acme/widget#7", TicketStatus.IN_REVIEW) - assert await tracker.get_account_id("octocat") is None + assert fake.calls[-1] == ("aclose",) def test_github_declares_known_exceptions(): + # The GitHub client raises githubkit's errors, not httpx's. A caller that + # misses them lets a failed label write fail the webhook that caused it. assert UnknownTicketError in GitHub.known_exceptions + assert GitHubAppNotInstalledError in GitHub.known_exceptions + assert RequestFailed in GitHub.known_exceptions assert httpx.HTTPError in GitHub.known_exceptions diff --git a/backend/tests/software_factory/test_webhooks_github_issues.py b/backend/tests/software_factory/test_webhooks_github_issues.py index 291ff7679..cec1201e4 100644 --- a/backend/tests/software_factory/test_webhooks_github_issues.py +++ b/backend/tests/software_factory/test_webhooks_github_issues.py @@ -1,6 +1,9 @@ from types import SimpleNamespace from typing import Any, cast +from unittest.mock import AsyncMock +import druks.contrib.software_factory.subscribers as subs +from conftest import connect_service from druks.core.webhooks import github as github_webhooks from druks.core.webhooks.github import GitHubEvents from druks.testing import make_settings @@ -16,7 +19,14 @@ def _events(tmp_path, *, payload, event="issues"): return events -def _labeled(*, number=7, label="ready-for-agent", labels=None, assignee=None): +def _labeled( + *, + number=7, + label="ready-for-agent", + assignee=None, + sender_type="User", + sender_login="octocat", +): return { "action": "labeled", "label": {"name": label}, @@ -24,96 +34,143 @@ def _labeled(*, number=7, label="ready-for-agent", labels=None, assignee=None): "number": number, "title": "Contacts import drops the last row", "html_url": f"https://github.com/acme/widget/issues/{number}", - "labels": [{"name": name} for name in (labels or [label])], + "labels": [{"name": label}], "assignee": assignee, }, "repository": {"full_name": "acme/widget", "name": "widget"}, + "sender": {"type": sender_type, "login": sender_login}, } -def _capture(monkeypatch): +def _capture(monkeypatch, module=github_webhooks): published = [] async def _emit(event_type, **kwargs): - published.append((event_type, kwargs["payload"])) + published.append((event_type, kwargs)) - monkeypatch.setattr(github_webhooks, "publish", _emit) + monkeypatch.setattr(module, "publish", _emit) return published +async def _connect_github(): + await connect_service( + "github", + identity={"app_id": "1", "slug": "druks-operator"}, + secrets={"private_key": "pem", "webhook_secret": "hook-secret"}, + ) + + def test_a_labelled_issue_dispatches_to_the_issues_handler(tmp_path): # get_action composes event + action, so `issues` + `labeled` has to land on # on_issues_labeled for any of this to run. assert _events(tmp_path, payload=_labeled()).get_action() == "issues_labeled" -async def test_labelling_publishes_a_ticket_transition(tmp_path, monkeypatch): - published = _capture(monkeypatch) - - await _events(tmp_path, payload=_labeled()).on_issues_labeled() - - assert len(published) == 1 - event_type, payload = published[0] - assert event_type == "ticket.transitioned" - assert payload["source"] == "github" - assert payload["status"] == "ready-for-agent" - assert payload["title"] == "Contacts import drops the last row" - assert payload["url"] == "https://github.com/acme/widget/issues/7" - - -async def test_the_identifier_carries_the_repository(tmp_path, monkeypatch): - # Issue numbers are only unique within a repo, so the key has to say which. - published = _capture(monkeypatch) - - await _events(tmp_path, payload=_labeled(number=42)).on_issues_labeled() - - assert published[0][1]["identifier"] == "acme/widget#42" - - -async def test_the_bare_repo_name_routes_to_a_project_repo(tmp_path, monkeypatch): - # ProjectRepo.lookup matches a bare name, the way it does a Linear project. - published = _capture(monkeypatch) - - await _events(tmp_path, payload=_labeled()).on_issues_labeled() - - assert published[0][1]["project_name"] == "widget" - - -async def test_every_label_on_the_issue_travels_for_routing(tmp_path, monkeypatch): +async def test_a_persons_label_publishes_a_neutral_fact(tmp_path, monkeypatch): published = _capture(monkeypatch) - payload = _labeled(labels=["ready-for-agent", "widget", "bug"]) + payload = _labeled(assignee={"login": "hubot"}) await _events(tmp_path, payload=payload).on_issues_labeled() - assert published[0][1]["labels"] == ["ready-for-agent", "widget", "bug"] - - -async def test_an_unassigned_issue_publishes_no_assignee(tmp_path, monkeypatch): + # Core knows nothing about work items: the fact is the label, not a transition. + assert published == [ + ( + "issue.labeled", + { + "repo": "acme/widget", + "number": 7, + "payload": { + "label": "ready-for-agent", + "title": "Contacts import drops the last row", + "url": "https://github.com/acme/widget/issues/7", + "assignee": "hubot", + }, + }, + ) + ] + + +async def test_another_apps_label_publishes_nothing(druks_db, tmp_path, monkeypatch): + await _connect_github() published = _capture(monkeypatch) - await _events(tmp_path, payload=_labeled(assignee=None)).on_issues_labeled() + payload = _labeled(sender_type="Bot", sender_login="renovate[bot]") + await _events(tmp_path, payload=payload).on_issues_labeled() - assert published[0][1]["assignee_name"] is None - assert published[0][1]["assignee_email"] is None + assert published == [] -async def test_an_assigned_issue_publishes_the_login_but_never_an_email(tmp_path, monkeypatch): - # GitHub webhooks carry no address; inventing one would misattribute a run. +async def test_the_apps_own_label_publishes(druks_db, tmp_path, monkeypatch): + # The start route asks for a build by setting the trigger label through the + # App, so the App's own label has to reach the funnel. + await _connect_github() published = _capture(monkeypatch) - payload = _labeled(assignee={"login": "octocat"}) + payload = _labeled(sender_type="Bot", sender_login="druks-operator[bot]") await _events(tmp_path, payload=payload).on_issues_labeled() - assert published[0][1]["assignee_name"] == "octocat" - assert published[0][1]["assignee_email"] is None + assert [event for event, _ in published] == ["issue.labeled"] -async def test_an_assignee_never_selects_an_account(tmp_path, monkeypatch): - # assignee_id drives the account lookup. A GitHub login is not an identity - # any grant issuer vouches for, so the build must not resolve one from it. - published = _capture(monkeypatch) +async def test_a_label_becomes_a_ticket_transition(monkeypatch): + published = _capture(monkeypatch, module=subs) - payload = _labeled(assignee={"login": "octocat", "id": 583231}) - await _events(tmp_path, payload=payload).on_issues_labeled() + await subs.issue_label_transitions_the_ticket( + repo="acme/widget", + number=42, + payload={ + "label": "ready-for-agent", + "title": "Contacts import drops the last row", + "url": "https://github.com/acme/widget/issues/42", + "assignee": "octocat", + }, + ) + + assert published == [ + ( + "ticket.transitioned", + { + "payload": { + "source": "github", + # Issue numbers are only unique within a repo, so the key says which. + "identifier": "acme/widget#42", + "status": "ready-for-agent", + "title": "Contacts import drops the last row", + "url": "https://github.com/acme/widget/issues/42", + # The full name matches one repo exactly. A label such as + # ``api`` would match any owner's ``api``, so none routes it. + "project_name": "acme/widget", + "labels": [], + # A login is not an address and selects no account. + "assignee_id": None, + "assignee_email": None, + "assignee_name": "octocat", + }, + }, + ) + ] + + +async def test_the_trigger_label_dispatches_a_github_build(monkeypatch): + settings = subs.SoftwareFactory.Settings(tracker="github", trigger_status="ready-for-agent") + + async def _settings(cls): + return settings + + monkeypatch.setattr(subs.SoftwareFactory, "settings", classmethod(_settings)) + build = AsyncMock() + monkeypatch.setattr(subs.Build, "dispatch", build) + + await subs.issue_label_transitions_the_ticket( + repo="acme/widget", + number=7, + payload={ + "label": "ready-for-agent", + "title": "Contacts import drops the last row", + "url": "https://github.com/acme/widget/issues/7", + "assignee": None, + }, + ) - assert published[0][1]["assignee_id"] is None + build.assert_awaited_once() + assert build.await_args.kwargs["ticket"]["identifier"] == "acme/widget#7" diff --git a/backend/tests/test_agent_routes.py b/backend/tests/test_agent_routes.py index 418c28ab3..8de63a768 100644 --- a/backend/tests/test_agent_routes.py +++ b/backend/tests/test_agent_routes.py @@ -242,6 +242,18 @@ async def start(cls, **kwargs): assert starts == [] # a repeat call re-stamps; it never dispatches locally +def test_software_factory_start_takes_a_github_key(client: TestClient, monkeypatch): + # owner/repo#12 has a ``/``, which ends a plain path segment, and a ``#``, + # which the caller encodes. + fake = _FakeTracker() + monkeypatch.setattr(SoftwareFactory, "get_tracker", _tracker_stub(fake)) + + response = client.post("/api/software_factory/work-items/acme/widget%2312/start") + + assert response.status_code == 202 + assert fake.calls == [("acme/widget#12", TicketStatus.TRIGGER), "aclose"] + + def test_software_factory_start_translates_an_unknown_tracker_ticket( client: TestClient, monkeypatch ): diff --git a/backend/tests/test_mcp_endpoint.py b/backend/tests/test_mcp_endpoint.py index 4de626744..f01cc9eda 100644 --- a/backend/tests/test_mcp_endpoint.py +++ b/backend/tests/test_mcp_endpoint.py @@ -227,7 +227,7 @@ async def test_tools_list_pins_platform_and_app_tools(app, pat_token, mode): reason = tools["cancel_run"].input_schema["properties"]["reason"] assert (reason["minLength"], reason["maxLength"]) == (1, 500) assert tools["software_factory_start"].input_schema["properties"]["ticket"]["description"] == ( - "The tracker's ticket key, e.g. ENG-831." + "The tracker's ticket key, e.g. ENG-831, or owner/repo#12 for GitHub." ) # software_factory_start moves the tracker ticket and waits on webhook intake — its # derived description must say so, run-id-free. diff --git a/docs/configuration.md b/docs/configuration.md index 7674be660..0b1182929 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -549,9 +549,16 @@ its intake. An issue that gains the trigger label opens a build. Set the five labels in **Software Factory → Settings**. They default to the Linear status names, so name them for GitHub before you select this tracker - for example `ready-for-agent`, `agent:in-progress`, `agent:in-review`, and -`agent:done`. Create each label in the repository first: GitHub rejects a label -that does not exist. An empty in review or resting label leaves the issue where -it is, as it does on Linear and Jira. +`agent:done`. If a label does not exist in the repository, GitHub creates it the +first time Druks sets it, with a default colour and no description. Create the +labels first if you want to choose those. An empty in review or resting label +leaves the issue where it is, as it does on Linear and Jira. + +A label opens a build only when a person adds it, or when Druks adds it itself +through the agent `start` route. A label another App or bot adds is ignored. +Adding a label on GitHub needs triage access or higher. The issue's repository +must be registered in a Software Factory project: the build opens its pull +request there, and a label never routes the issue to another repository. The labels behave as one exclusive group. Setting any of them removes the others, including the trigger label - so an issue never reads as two states at once, and @@ -560,7 +567,8 @@ closes the issue as completed, which is what a GitHub reader expects finished work to look like. Three differences from Linear and Jira are worth knowing. A ticket key is -`owner/repo#number`, because issue numbers repeat across repositories. The +`owner/repo#number`, because issue numbers repeat across repositories. Encode +the `#` as `%23` when the key goes in a URL path. The settings page lists no label choices, because labels are per repository rather than per workspace, so you type them. And a build never runs as the issue assignee: a GitHub login is not an identity any grant issuer vouches for, so From 9410218655855c279cbab6cc41a12ca85a1c423f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C4=ABlav=C4=81pi=20Cheesley?= Date: Wed, 7 Oct 2026 18:50:51 +0100 Subject: [PATCH 3/4] Carry actor_id on a GitHub ticket transition Main now resolves a build's account from the person who moved the ticket, and every ticket.transitioned payload carries actor_id. A GitHub label names a login, which no grant issuer vouches for, so the GitHub transition carries no actor and the build keeps the default account. Co-Authored-By: Claude Opus 5.5 --- backend/druks/contrib/software_factory/subscribers.py | 1 + .../tests/software_factory/test_webhooks_github_issues.py | 1 + docs/configuration.md | 6 +++--- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/backend/druks/contrib/software_factory/subscribers.py b/backend/druks/contrib/software_factory/subscribers.py index bc2877993..62749b8df 100644 --- a/backend/druks/contrib/software_factory/subscribers.py +++ b/backend/druks/contrib/software_factory/subscribers.py @@ -96,6 +96,7 @@ async def issue_label_transitions_the_ticket(*, repo: str, number: int, payload: "labels": [], # A login is not an address, and no grant issuer vouches for it, so it # selects no account. The name is for display only. + "actor_id": None, "assignee_id": None, "assignee_email": None, "assignee_name": payload["assignee"], diff --git a/backend/tests/software_factory/test_webhooks_github_issues.py b/backend/tests/software_factory/test_webhooks_github_issues.py index cec1201e4..5a0491bf7 100644 --- a/backend/tests/software_factory/test_webhooks_github_issues.py +++ b/backend/tests/software_factory/test_webhooks_github_issues.py @@ -142,6 +142,7 @@ async def test_a_label_becomes_a_ticket_transition(monkeypatch): "project_name": "acme/widget", "labels": [], # A login is not an address and selects no account. + "actor_id": None, "assignee_id": None, "assignee_email": None, "assignee_name": "octocat", diff --git a/docs/configuration.md b/docs/configuration.md index 0b1182929..5036f1b69 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -570,9 +570,9 @@ Three differences from Linear and Jira are worth knowing. A ticket key is `owner/repo#number`, because issue numbers repeat across repositories. Encode the `#` as `%23` when the key goes in a URL path. The settings page lists no label choices, because labels are per repository rather -than per workspace, so you type them. And a build never runs as the issue -assignee: a GitHub login is not an identity any grant issuer vouches for, so -every GitHub build uses the default account. +than per workspace, so you type them. And a build never runs as the person who +added the label or as the issue assignee: a GitHub login is not an identity any +grant issuer vouches for, so every GitHub build uses the default account. The App must subscribe to the **issues** event. An App created through **Settings → Connections → Services → Create GitHub App** subscribes to it. An From 2cc8ecfa6c48ad0ed34131b2379a5ed128907f6e Mon Sep 17 00:00:00 2001 From: Paulo Date: Wed, 14 Oct 2026 19:21:35 +0200 Subject: [PATCH 4/4] Start a GitHub build under the person who adds the label A GitHub sign-in links a GitHub user to a Druks account, so the label's sender now picks the build's account as on Linear and Jira, with the assignee next. The tracker takes its client from get_tracker, catches every githubkit error, and closes a done issue with close_issue. Tests and docs drop what restated the code. --- backend/druks/contrib/software_factory/app.py | 32 ++-- .../contrib/software_factory/exceptions.py | 3 +- .../druks/contrib/software_factory/routes.py | 3 +- .../contrib/software_factory/subscribers.py | 14 +- .../software_factory/ticketing/github.py | 74 +++------ backend/druks/core/apis/github.py | 37 +---- backend/druks/core/services.py | 1 - backend/druks/core/webhooks/github.py | 19 +-- .../tests/software_factory/test_ticketing.py | 156 +++++------------- .../test_webhooks_github_issues.py | 106 ++++-------- docs/configuration.md | 86 ++++------ 11 files changed, 164 insertions(+), 367 deletions(-) diff --git a/backend/druks/contrib/software_factory/app.py b/backend/druks/contrib/software_factory/app.py index 8e3c76f75..a076609c6 100644 --- a/backend/druks/contrib/software_factory/app.py +++ b/backend/druks/contrib/software_factory/app.py @@ -38,11 +38,12 @@ async def check_tracker_identity() -> CheckResult: return CheckResult(name="tracker", ok=True, detail="trackerless by choice") if settings.tracker == "druks": return CheckResult(name="tracker", ok=True, detail="this appliance") - # The operator App is the GitHub tracker's identity; there is no second - # credential to connect. - service = {"linear": services.Linear, "jira": services.Jira, "github": services.Github}[ - settings.tracker - ] + service = { + "linear": services.Linear, + "jira": services.Jira, + # The GitHub tracker acts as the operator App, so it needs no credential of its own. + "github": services.Github, + }[settings.tracker] if await service.is_connected(): return CheckResult(name="tracker", ok=True, detail=f"{settings.tracker} connected") return CheckResult( @@ -57,14 +58,13 @@ async def check_tracker_identity() -> CheckResult: async def list_tracker_status_choices() -> list[dict[str, str]]: """The statuses of the selected tracker. Empty when no connected tracker lists them.""" tracker = await SoftwareFactory.get_tracker() - if not tracker: - return [] - async with tracker: - try: - return await tracker.list_status_choices() - except tracker.known_exceptions: - logger.warning("Could not list the tracker statuses.", exc_info=True) - return [] + if tracker: + async with tracker: + try: + return await tracker.list_status_choices() + except tracker.known_exceptions: + logger.warning("Could not list the tracker statuses.", exc_info=True) + return [] async def check_review_identity() -> CheckResult: @@ -175,10 +175,8 @@ async def get_tracker(cls, source: str | None = None) -> Tracker | None: status_names=status_names, ) if settings.tracker == "github": - # Presence check only: the client resolves App credentials per - # repository installation, not from this row. - await services.Github.get() - return GitHub(status_names=status_names) + client = await services.Github.get_client() + return GitHub(client=client, status_names=status_names) except ServiceNotConnectedError: return diff --git a/backend/druks/contrib/software_factory/exceptions.py b/backend/druks/contrib/software_factory/exceptions.py index 413c14d1d..746806311 100644 --- a/backend/druks/contrib/software_factory/exceptions.py +++ b/backend/druks/contrib/software_factory/exceptions.py @@ -41,7 +41,8 @@ class TrackerNotConfigured(AgentApiError): def __init__(self) -> None: super().__init__( "No ticket tracker is configured. Select one in the Software Factory settings. " - "Linear and Jira also need their identity in Settings → Connections → Services." + "Linear, Jira, and GitHub also need their identity in " + "Settings → Connections → Services." ) diff --git a/backend/druks/contrib/software_factory/routes.py b/backend/druks/contrib/software_factory/routes.py index c9bb1ed04..da2e8524f 100644 --- a/backend/druks/contrib/software_factory/routes.py +++ b/backend/druks/contrib/software_factory/routes.py @@ -274,8 +274,7 @@ async def list_work_items_history( @work_items_router.post( - # ``path`` lets a GitHub key, ``owner/repo#12``, through: its ``/`` would end a - # plain segment. The caller encodes the ``#`` as ``%23``. + # A GitHub key, ``owner/repo#12``, has a ``/`` that would end a plain segment. "/{ticket:path}/start", status_code=status.HTTP_202_ACCEPTED, operation_id="start", diff --git a/backend/druks/contrib/software_factory/subscribers.py b/backend/druks/contrib/software_factory/subscribers.py index 62749b8df..7a8ad44b2 100644 --- a/backend/druks/contrib/software_factory/subscribers.py +++ b/backend/druks/contrib/software_factory/subscribers.py @@ -80,24 +80,22 @@ async def pr_close_settles_the_item(*, repo: str, pr_number: int, payload: dict) @subscribe("issue.labeled") async def issue_label_transitions_the_ticket(*, repo: str, number: int, payload: dict) -> None: - """GitHub has no status field, so the GitHub tracker reads a label as one. Issue - numbers repeat across repos, so the key carries the repo. The issue's own repo is - the build's repo: the full name routes to exactly it, and no label routes it - anywhere else.""" + """GitHub has no status field, so a label is the status of a GitHub ticket.""" + assignee_id = payload["assignee_id"] await publish( "ticket.transitioned", payload={ "source": "github", + # Issue numbers repeat across repos, so the key carries the repo. "identifier": f"{repo}#{number}", "status": payload["label"], "title": payload["title"], "url": payload["url"], + # The issue's own repo is the build's repo, so no label routes it elsewhere. "project_name": repo, "labels": [], - # A login is not an address, and no grant issuer vouches for it, so it - # selects no account. The name is for display only. - "actor_id": None, - "assignee_id": None, + "actor_id": str(payload["actor_id"]), + "assignee_id": str(assignee_id) if assignee_id else None, "assignee_email": None, "assignee_name": payload["assignee"], }, diff --git a/backend/druks/contrib/software_factory/ticketing/github.py b/backend/druks/contrib/software_factory/ticketing/github.py index abe5cb3b1..7505df57b 100644 --- a/backend/druks/contrib/software_factory/ticketing/github.py +++ b/backend/druks/contrib/software_factory/ticketing/github.py @@ -1,72 +1,38 @@ -import httpx -from githubkit.exception import RequestFailed +from githubkit.exception import GitHubException from druks.core.apis.exceptions import GitHubAppNotInstalledError, UnknownTicketError -from druks.core.apis.github import GitHubClient -from druks.core.services import Github +from druks.core.apis.github import GITHUB_AUTHORITY, GitHubClient from .base import Tracker from .enums import TicketStatus -def split_key(key: str) -> tuple[str, int]: - """``owner/repo#123`` -> ``("owner/repo", 123)``. - - GitHub issue numbers are only unique within a repository, so the repo has to - travel in the key itself. Linear and Jira keys are globally unique in their - workspace and need no such qualifier. - """ - repo, _, number = key.partition("#") - if not repo or not number.isdigit(): - raise UnknownTicketError(key, "GitHub") - return repo, int(number) - - class GitHub(Tracker): - """Drives a GitHub issue through the build funnel using labels. + """GitHub has no status field, so each status name is a label. The status labels + are one exclusive group: setting one takes the others off, the trigger label too, + so adding the trigger label again is a new event.""" - GitHub has no status field, so each configured status name is a label name. - The labels behave as an exclusive group: setting one removes the others. - That exclusivity is what makes them act like the status column Linear and - Jira give us for free - and it is why the trigger label comes off as soon as - a build claims the issue, so a later re-label opens a fresh build. - """ + known_exceptions = (GitHubException, GitHubAppNotInstalledError, UnknownTicketError) + authority = GITHUB_AUTHORITY - known_exceptions = ( - UnknownTicketError, - GitHubAppNotInstalledError, - RequestFailed, - httpx.HTTPError, - ) - - def __init__(self, *, status_names: dict[TicketStatus, str]) -> None: - self._client: GitHubClient | None = None + def __init__(self, *, client: GitHubClient, status_names: dict[TicketStatus, str]) -> None: + self._client = client # An empty name leaves that status unmapped. self._status_names = {status: name for status, name in status_names.items() if name} - async def _gh(self) -> GitHubClient: - # The operator App is the tracker's identity, resolved lazily so - # constructing a tracker never needs a connected service. - if self._client is None: - self._client = await Github.get_client() - return self._client - async def set_status(self, key: str, status: TicketStatus) -> None: - label = self._status_names.get(status) - if not label: + name = self._status_names.get(status) + if not name: raise ValueError(f"GitHub has no configured label for {status}") - repo, number = split_key(key) - client = await self._gh() - # Exclusive group: drop every other label this tracker owns before - # adding the new one, so an issue never reads as two states at once. - stale = {name for name in self._status_names.values() if name != label} - await client.replace_issue_labels(repo, number, add=label, remove=stale) + # Issue numbers repeat across repositories, so the key is ``owner/repo#number``. + repo, _, number = key.partition("#") + if "/" not in repo or not number.isdigit(): + raise UnknownTicketError(key, "GitHub") + others = {label for label in self._status_names.values() if label != name} + await self._client.replace_issue_labels(repo, int(number), add=name, remove=others) if status is TicketStatus.DONE: - # The label alone would leave finished work in the open-issue list, - # which is not what a GitHub reader expects it to look like. - await client.set_issue_state(repo, number, state="closed", state_reason="completed") + # Finished work on GitHub is a closed issue, not only a label. + await self._client.close_issue(repo, int(number)) async def aclose(self) -> None: - if self._client is not None: - await self._client.aclose() - self._client = None + await self._client.aclose() diff --git a/backend/druks/core/apis/github.py b/backend/druks/core/apis/github.py index c1dc34da5..7903b63ad 100644 --- a/backend/druks/core/apis/github.py +++ b/backend/druks/core/apis/github.py @@ -377,46 +377,27 @@ async def reply_to_review_comment( @_retry_on_401 async def replace_issue_labels( - self, - repo: str, - issue_number: int, - *, - add: str, - remove: set[str], + self, repo: str, issue_number: int, *, add: str, remove: set[str] ) -> None: - """Add one label and drop a set of others, for callers that treat a group - of labels as mutually exclusive. - - Removal is per-label and tolerates a 404: GitHub returns one when the - label isn't on the issue, which is the normal case and not a failure. - """ + """Put ``add`` on the issue and take ``remove`` off. GitHub answers 404 for a + label the issue does not carry, and that label is already off.""" owner, name = repo.split("/", 1) github = await self._for_repo(repo) await github.rest.issues.async_add_labels(owner, name, issue_number, data=[add]) for label in remove: try: await github.rest.issues.async_remove_label(owner, name, issue_number, label) - except RequestFailed as exc: - if exc.response.status_code != 404: + except RequestFailed as error: + if error.response.status_code != 404: raise @_retry_on_401 - async def set_issue_state( - self, - repo: str, - issue_number: int, - *, - state: str, - state_reason: str | None = None, - ) -> None: - """Open or close an issue. ``state_reason`` distinguishes a completed - close from a not-planned one, which GitHub renders differently.""" + async def close_issue(self, repo: str, issue_number: int) -> None: owner, name = repo.split("/", 1) github = await self._for_repo(repo) - payload: dict[str, Any] = {"state": state} - if state_reason: - payload["state_reason"] = state_reason - await github.rest.issues.async_update(owner, name, issue_number, **payload) + await github.rest.issues.async_update( + owner, name, issue_number, state="closed", state_reason="completed" + ) @_retry_on_401 async def get_pull_request(self, repo: str, pr_number: int) -> dict[str, Any]: diff --git a/backend/druks/core/services.py b/backend/druks/core/services.py index a3fc3afbd..d162e756c 100644 --- a/backend/druks/core/services.py +++ b/backend/druks/core/services.py @@ -57,7 +57,6 @@ class Github(Service): "public": False, "default_events": [ "issue_comment", - # Issue labelling is the GitHub tracker's intake signal. "issues", "pull_request", "pull_request_review", diff --git a/backend/druks/core/webhooks/github.py b/backend/druks/core/webhooks/github.py index 657d1c644..eb85939e4 100644 --- a/backend/druks/core/webhooks/github.py +++ b/backend/druks/core/webhooks/github.py @@ -110,11 +110,12 @@ async def _publish_comment( ) async def on_issues_labeled(self) -> Response: - """Publish a label a person put on an issue. The App's own labels publish - too: setting a label through the API is how Druks itself asks for one. Any - other app is not acting for anyone here.""" + """Publish a label that a person or this App put on an issue. Another App's + label acts for no one here.""" sender, issue = self.data["sender"], self.data["issue"] - if sender["type"] == "User" or await _is_own_app(sender): + # GitHub names an App's bot user after the App's slug. + own_bot = f"{(await Github.get()).identity['slug']}[bot]" + if sender["type"] == "User" or sender["login"] == own_bot: assignee = issue.get("assignee") or {} await publish( "issue.labeled", @@ -122,9 +123,11 @@ async def on_issues_labeled(self) -> Response: number=issue["number"], payload={ "label": self.data["label"]["name"], + "actor_id": sender["id"], "title": issue["title"], "url": issue["html_url"], "assignee": assignee.get("login"), + "assignee_id": assignee.get("id"), }, ) return _accepted() @@ -173,11 +176,3 @@ def _accepted() -> Response: def _repo_name(payload: dict[str, Any]) -> str: return payload["repository"]["full_name"] - - -async def _is_own_app(sender: dict[str, Any]) -> bool: - if sender["type"] != "Bot": - return False - # GitHub names an App's bot user after the App's slug. - slug = (await Github.get()).identity["slug"] - return sender["login"] == f"{slug}[bot]" diff --git a/backend/tests/software_factory/test_ticketing.py b/backend/tests/software_factory/test_ticketing.py index c874e192a..d72a5d62f 100644 --- a/backend/tests/software_factory/test_ticketing.py +++ b/backend/tests/software_factory/test_ticketing.py @@ -26,7 +26,7 @@ from druks.core.apis.linear import LinearClient from druks.secrets.models import VaultSecret from druks.services import ServiceConnectError -from githubkit.exception import RequestFailed +from githubkit.exception import GitHubException from software_factory.factories import make_test_work_item @@ -52,7 +52,7 @@ async def _connect_github(): return await connect_service( "github", identity={"slug": "druks", "app_id": "1"}, - secrets={"app_id": "1", "private_key": "pem", "webhook_secret": "gh-hook"}, + secrets={"private_key": "pem", "webhook_secret": "gh-hook"}, ) @@ -718,49 +718,47 @@ def handler(request: httpx.Request) -> httpx.Response: class _FakeGitHubClient: - """Records the client calls the tracker delegates to - no HTTP.""" - def __init__(self) -> None: self.calls: list = [] async def replace_issue_labels(self, repo, issue_number, *, add, remove): - self.calls.append(("labels", repo, issue_number, add, sorted(remove))) + self.calls.append(("replace_issue_labels", repo, issue_number, add, sorted(remove))) - async def set_issue_state(self, repo, issue_number, *, state, state_reason=None): - self.calls.append(("state", repo, issue_number, state, state_reason)) + async def close_issue(self, repo, issue_number): + self.calls.append(("close_issue", repo, issue_number)) - async def aclose(self): - self.calls.append(("aclose",)) +def _github(fake) -> GitHub: + return GitHub( + client=fake, + status_names={ + TicketStatus.TRIGGER: "ready-for-agent", + TicketStatus.IN_PROGRESS: "agent:in-progress", + TicketStatus.IN_REVIEW: "agent:in-review", + TicketStatus.DONE: "agent:done", + TicketStatus.BACKLOG: "", + }, + ) -def _github_tracker(monkeypatch, **status_names): - fake = _FakeGitHubClient() - async def get_client(cls): - return fake +async def test_tracker_builds_github_from_the_operator_app(monkeypatch): + await _connect_github() + _pin_software_factory_settings(monkeypatch, tracker="github", trigger_status="ready-for-agent") - monkeypatch.setattr(services.Github, "get_client", classmethod(get_client)) - names = { - TicketStatus.TRIGGER: "ready-for-agent", - TicketStatus.IN_PROGRESS: "agent:in-progress", - TicketStatus.IN_REVIEW: "agent:in-review", - TicketStatus.DONE: "agent:done", - TicketStatus.BACKLOG: "", - } - names.update({TicketStatus(status): name for status, name in status_names.items()}) - return GitHub(status_names=names), fake + tracker = await SoftwareFactory.get_tracker("github") + + assert isinstance(tracker, GitHub) + assert tracker._status_names[TicketStatus.TRIGGER] == "ready-for-agent" -async def test_github_set_status_swaps_the_label_within_an_exclusive_group(monkeypatch): - tracker, fake = _github_tracker(monkeypatch) +async def test_github_set_status_swaps_the_label_within_an_exclusive_group(): + fake = _FakeGitHubClient() - await tracker.set_status("acme/widget#7", TicketStatus.IN_PROGRESS) + await _github(fake).set_status("acme/widget#7", TicketStatus.IN_PROGRESS) - # The new label goes on; every other label this tracker owns comes off, so - # an issue never reads as two states at once. assert fake.calls == [ ( - "labels", + "replace_issue_labels", "acme/widget", 7, "agent:in-progress", @@ -769,104 +767,24 @@ async def test_github_set_status_swaps_the_label_within_an_exclusive_group(monke ] -async def test_github_set_status_clears_the_trigger_label_so_a_relabel_can_retrigger(monkeypatch): - tracker, fake = _github_tracker(monkeypatch) - - await tracker.set_status("acme/widget#7", TicketStatus.IN_PROGRESS) - - _, _, _, _, removed = fake.calls[0] - assert "ready-for-agent" in removed - - -async def test_github_done_closes_the_issue_as_completed(monkeypatch): - tracker, fake = _github_tracker(monkeypatch) - - await tracker.set_status("acme/widget#7", TicketStatus.DONE) - - assert ("state", "acme/widget", 7, "closed", "completed") in fake.calls - - -async def test_github_non_terminal_status_leaves_the_issue_open(monkeypatch): - tracker, fake = _github_tracker(monkeypatch) - - await tracker.set_status("acme/widget#7", TicketStatus.IN_REVIEW) - - assert not [call for call in fake.calls if call[0] == "state"] - +async def test_github_done_closes_the_issue(): + fake = _FakeGitHubClient() -async def test_github_an_unmapped_status_raises(monkeypatch): - tracker, _ = _github_tracker(monkeypatch) + await _github(fake).set_status("acme/widget#7", TicketStatus.DONE) - # BACKLOG is unnamed by default: an empty resting label leaves the issue put. - with pytest.raises(ValueError, match="no configured label"): - await tracker.set_status("acme/widget#7", TicketStatus.BACKLOG) + assert fake.calls[-1] == ("close_issue", "acme/widget", 7) -async def test_github_rejects_a_key_without_a_repo_qualifier(monkeypatch): - tracker, _ = _github_tracker(monkeypatch) +async def test_github_rejects_a_key_without_its_repo(): + tracker = _github(_FakeGitHubClient()) - # Issue numbers repeat across repositories, so a bare number is ambiguous - # and must not be guessed at. with pytest.raises(UnknownTicketError, match="doesn't exist in GitHub"): - await tracker.set_status("7", TicketStatus.DONE) + await tracker.set_status("widget#7", TicketStatus.DONE) with pytest.raises(UnknownTicketError): - await tracker.set_status("acme/widget#not-a-number", TicketStatus.DONE) - - -async def test_github_closes_the_client_it_created(monkeypatch): - tracker, fake = _github_tracker(monkeypatch) - - async with tracker: - await tracker.set_status("acme/widget#7", TicketStatus.IN_REVIEW) - - assert fake.calls[-1] == ("aclose",) + await tracker.set_status("acme/widget#seven", TicketStatus.DONE) def test_github_declares_known_exceptions(): - # The GitHub client raises githubkit's errors, not httpx's. A caller that - # misses them lets a failed label write fail the webhook that caused it. - assert UnknownTicketError in GitHub.known_exceptions + assert GitHubException in GitHub.known_exceptions assert GitHubAppNotInstalledError in GitHub.known_exceptions - assert RequestFailed in GitHub.known_exceptions - assert httpx.HTTPError in GitHub.known_exceptions - - -async def test_tracker_builds_github_from_the_operator_app(monkeypatch): - await _connect_github() - _pin_software_factory_settings( - monkeypatch, - tracker="github", - trigger_status="ready-for-agent", - resting_status="needs-triage", - ) - - tracker = await SoftwareFactory.get_tracker("github") - - assert isinstance(tracker, GitHub) - assert tracker._status_names[TicketStatus.TRIGGER] == "ready-for-agent" - assert tracker._status_names[TicketStatus.BACKLOG] == "needs-triage" - - -async def test_github_tracker_check_reports_the_operator_app(monkeypatch): - await _connect_github() - _pin_software_factory_settings(monkeypatch, tracker="github") - - result = await check_tracker_identity() - - assert result.ok - assert "github" in result.detail - - -async def test_github_tracker_check_pends_without_the_operator_app(monkeypatch): - _pin_software_factory_settings(monkeypatch, tracker="github") - - result = await check_tracker_identity() - - assert not result.ok - assert result.pending - - -def test_github_manifest_subscribes_to_issue_events(): - # The tracker's intake is the issues event. An App created from this - # manifest without it would connect cleanly and then never open a build. - assert "issues" in services.Github.manifest["default_events"] + assert UnknownTicketError in GitHub.known_exceptions diff --git a/backend/tests/software_factory/test_webhooks_github_issues.py b/backend/tests/software_factory/test_webhooks_github_issues.py index 5a0491bf7..d27b974ee 100644 --- a/backend/tests/software_factory/test_webhooks_github_issues.py +++ b/backend/tests/software_factory/test_webhooks_github_issues.py @@ -9,9 +9,9 @@ from druks.testing import make_settings -def _events(tmp_path, *, payload, event="issues"): +def _events(tmp_path, *, payload): events = GitHubEvents( - request=cast(Any, SimpleNamespace(headers={"x-github-event": event})), + request=cast(Any, SimpleNamespace(headers={"x-github-event": "issues"})), kwargs={}, settings=make_settings(tmp_path), ) @@ -19,36 +19,28 @@ def _events(tmp_path, *, payload, event="issues"): return events -def _labeled( - *, - number=7, - label="ready-for-agent", - assignee=None, - sender_type="User", - sender_login="octocat", -): +def _labeled(*, assignee=None, sender_type="User", sender_login="octocat"): return { "action": "labeled", - "label": {"name": label}, + "label": {"name": "ready-for-agent"}, "issue": { - "number": number, + "number": 7, "title": "Contacts import drops the last row", - "html_url": f"https://github.com/acme/widget/issues/{number}", - "labels": [{"name": label}], + "html_url": "https://github.com/acme/widget/issues/7", "assignee": assignee, }, - "repository": {"full_name": "acme/widget", "name": "widget"}, - "sender": {"type": sender_type, "login": sender_login}, + "repository": {"full_name": "acme/widget"}, + "sender": {"type": sender_type, "login": sender_login, "id": 583231}, } -def _capture(monkeypatch, module=github_webhooks): +def _capture(monkeypatch): published = [] async def _emit(event_type, **kwargs): published.append((event_type, kwargs)) - monkeypatch.setattr(module, "publish", _emit) + monkeypatch.setattr(github_webhooks, "publish", _emit) return published @@ -60,19 +52,13 @@ async def _connect_github(): ) -def test_a_labelled_issue_dispatches_to_the_issues_handler(tmp_path): - # get_action composes event + action, so `issues` + `labeled` has to land on - # on_issues_labeled for any of this to run. - assert _events(tmp_path, payload=_labeled()).get_action() == "issues_labeled" - - -async def test_a_persons_label_publishes_a_neutral_fact(tmp_path, monkeypatch): +async def test_a_persons_label_publishes_the_label_and_who_put_it(druks_db, tmp_path, monkeypatch): + await _connect_github() published = _capture(monkeypatch) - payload = _labeled(assignee={"login": "hubot"}) + payload = _labeled(assignee={"login": "hubot", "id": 480938}) await _events(tmp_path, payload=payload).on_issues_labeled() - # Core knows nothing about work items: the fact is the label, not a transition. assert published == [ ( "issue.labeled", @@ -81,9 +67,11 @@ async def test_a_persons_label_publishes_a_neutral_fact(tmp_path, monkeypatch): "number": 7, "payload": { "label": "ready-for-agent", + "actor_id": 583231, "title": "Contacts import drops the last row", "url": "https://github.com/acme/widget/issues/7", "assignee": "hubot", + "assignee_id": 480938, }, }, ) @@ -101,8 +89,7 @@ async def test_another_apps_label_publishes_nothing(druks_db, tmp_path, monkeypa async def test_the_apps_own_label_publishes(druks_db, tmp_path, monkeypatch): - # The start route asks for a build by setting the trigger label through the - # App, so the App's own label has to reach the funnel. + # The start route asks for a build by setting the trigger label through the App. await _connect_github() published = _capture(monkeypatch) @@ -112,47 +99,7 @@ async def test_the_apps_own_label_publishes(druks_db, tmp_path, monkeypatch): assert [event for event, _ in published] == ["issue.labeled"] -async def test_a_label_becomes_a_ticket_transition(monkeypatch): - published = _capture(monkeypatch, module=subs) - - await subs.issue_label_transitions_the_ticket( - repo="acme/widget", - number=42, - payload={ - "label": "ready-for-agent", - "title": "Contacts import drops the last row", - "url": "https://github.com/acme/widget/issues/42", - "assignee": "octocat", - }, - ) - - assert published == [ - ( - "ticket.transitioned", - { - "payload": { - "source": "github", - # Issue numbers are only unique within a repo, so the key says which. - "identifier": "acme/widget#42", - "status": "ready-for-agent", - "title": "Contacts import drops the last row", - "url": "https://github.com/acme/widget/issues/42", - # The full name matches one repo exactly. A label such as - # ``api`` would match any owner's ``api``, so none routes it. - "project_name": "acme/widget", - "labels": [], - # A login is not an address and selects no account. - "actor_id": None, - "assignee_id": None, - "assignee_email": None, - "assignee_name": "octocat", - }, - }, - ) - ] - - -async def test_the_trigger_label_dispatches_a_github_build(monkeypatch): +async def test_the_trigger_label_dispatches_a_build_for_the_issue(monkeypatch): settings = subs.SoftwareFactory.Settings(tracker="github", trigger_status="ready-for-agent") async def _settings(cls): @@ -167,11 +114,26 @@ async def _settings(cls): number=7, payload={ "label": "ready-for-agent", + "actor_id": 583231, "title": "Contacts import drops the last row", "url": "https://github.com/acme/widget/issues/7", "assignee": None, + "assignee_id": None, }, ) - build.assert_awaited_once() - assert build.await_args.kwargs["ticket"]["identifier"] == "acme/widget#7" + build.assert_awaited_once_with( + ticket={ + "source": "github", + "identifier": "acme/widget#7", + "status": "ready-for-agent", + "title": "Contacts import drops the last row", + "url": "https://github.com/acme/widget/issues/7", + "project_name": "acme/widget", + "labels": [], + "actor_id": "583231", + "assignee_id": None, + "assignee_email": None, + "assignee_name": None, + } + ) diff --git a/docs/configuration.md b/docs/configuration.md index 2e4581d76..038af310f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -399,10 +399,9 @@ Callback URL: `/api/oauth/callback` Subscribe to issues, issue comment, pull request, pull request review, pull -request review comment, and push events. The issues event is what the GitHub -tracker reads; an App that omits it receives no intake. Keep **Expire user -authorization tokens** on: Druks refreshes a person's sign-in with the refresh -token that comes with it, and refuses a sign-in that has none. +request review comment, and push events. Keep **Expire user authorization +tokens** on: Druks refreshes a person's sign-in with the refresh token that +comes with it, and refuses a sign-in that has none. | Repository permission | Access | | --- | --- | @@ -492,8 +491,8 @@ webhook secret. For an Atlassian service account, the email is its address at stores them. Linear, Jira, and GitHub share five status settings: trigger, in progress, in review, done, and resting. When the selected tracker is connected and lists its statuses, the settings page offers them as choices. After you -change the tracker, save the settings to list the statuses of the new tracker. An empty in review or resting -status leaves the ticket where it is. +change the tracker, save the settings to list the statuses of the new tracker. +An empty in review or resting status leaves the ticket where it is. Select **druks** to use the ticket board on this appliance. That choice needs no credentials. The status settings stay hidden. The trigger status is @@ -537,56 +536,37 @@ Druks accepts the REST issue JSON under `issue`. Put the shared token in the tracker is not selected. It reports pending setup if the selected tracker is Linear, Jira, or GitHub and its identity is missing. +Software Factory starts a Linear, Jira, or GitHub build under the Druks account +of the person who moved the ticket. If that person has no account, the build +uses the account of the ticket assignee, and then the default account. Linear +sends that person in its webhook. Jira sends that person in the +`x-jira-initiator` header. GitHub sends the person who added the label. A Linear +or Jira person has an account when their tracker ID matches the +[provider account](#oauth-grant-identity) of an MCP connection that they made +with **Connect your account**. A connection for everyone has no account, so it +does not match. A GitHub person has the account that their +[GitHub sign-in](#github) links. Two matching accounts do not match. A **druks** +ticket build uses the account of the ticket assignee. + ### GitHub Issues as the tracker GitHub has no status field, so the GitHub tracker reads the five status settings -as **label names** and uses the existing `/_external/github/events/` webhook as -its intake. An issue that gains the trigger label opens a build. - -Set the five labels in **Software Factory → Settings**. They default to the -Linear status names, so name them for GitHub before you select this tracker - for -example `ready-for-agent`, `agent:in-progress`, `agent:in-review`, and -`agent:done`. If a label does not exist in the repository, GitHub creates it the -first time Druks sets it, with a default colour and no description. Create the -labels first if you want to choose those. An empty in review or resting label -leaves the issue where it is, as it does on Linear and Jira. - -A label opens a build only when a person adds it, or when Druks adds it itself -through the agent `start` route. A label another App or bot adds is ignored. -Adding a label on GitHub needs triage access or higher. The issue's repository -must be registered in a Software Factory project: the build opens its pull -request there, and a label never routes the issue to another repository. - -The labels behave as one exclusive group. Setting any of them removes the others, -including the trigger label - so an issue never reads as two states at once, and -re-applying the trigger label opens a fresh build. Reaching the done label also -closes the issue as completed, which is what a GitHub reader expects finished -work to look like. - -Three differences from Linear and Jira are worth knowing. A ticket key is -`owner/repo#number`, because issue numbers repeat across repositories. Encode -the `#` as `%23` when the key goes in a URL path. The -settings page lists no label choices, because labels are per repository rather -than per workspace, so you type them. And a build never runs as the person who -added the label or as the issue assignee: a GitHub login is not an identity any -grant issuer vouches for, so every GitHub build uses the default account. - -The App must subscribe to the **issues** event. An App created through -**Settings → Connections → Services → Create GitHub App** subscribes to it. An -older App, or one registered by hand, needs the event adding before any build -opens. Adding it needs no permission change: **Issues: Read and write** is -already on the App's list and it covers the labelling and closing this tracker -does, so no installation has to approve anything. - -Software Factory starts a Linear or Jira build under the Druks account of the -person who moved the ticket. If that person has no account, the build uses the -account of the ticket assignee, and then the default account. Linear sends that -person in its webhook. Jira sends that person in the `x-jira-initiator` header. -A person has an account when their tracker ID matches the -[provider account](#oauth-grant-identity) of an MCP connection that they made -with **Connect your account**. A connection for everyone has no account, so it -does not match. Two matching accounts also do not match. A **druks** ticket -build uses the account of the ticket assignee. +as **label names**. An issue that gets the trigger label opens a build. The label +arrives through the operator App's webhook, so the App must subscribe to the +**issues** event. An App made with **Create App** subscribes to it. Add the event +to an older App or to an App that you registered by hand. + +The status settings default to Linear status names, so name them for GitHub, for +example `ready-for-agent` and `agent:in-progress`. The settings page lists no +labels, so you type them. GitHub creates a missing label with a default colour +the first time Druks sets it. The labels act as one status: setting one removes +the others, the trigger label too. The done label also closes the issue. + +A label from a person opens a build, and so does a label that Druks sets through +the agent `start` route. A label from another App does nothing. The issue's +repository must be in a Software Factory project, and the build opens its pull +request there. A ticket key is `owner/repo#number`, because issue numbers repeat +across repositories. In a URL path, encode its `#` as `%23`. ## WhatsApp