Add GitHub plugin: tasks, webhook app, review conditions, MCP server

[cosmicBboy]

Adds plugins/github (flyteplugins-github), a GitHub integration for Flyte 2.

What's included

Read/write from tasks — GitHubClient, an async client covering repos, files, commits, issues, pull requests, reviews, branches, check runs, and merging. Credentials come from a mounted flyte.Secret (GITHUB_TOKEN).

Human review gate (condition with a JSON payload) — review_pr parks a run on flyte.new_condition(data_type=str) whose markdown prompt embeds PR review metadata (files, diff stats, prior reviews) as a JSON block. The reviewer answers in the Flyte UI with JSON, and the task parses it into a typed ReviewDecision it can branch on (approve → merge, otherwise post feedback). parse_review_payload tolerates raw JSON, fenced blocks, and JSON-in-prose, and normalizes verdict synonyms.

React to GitHub events — GitHubAppEnvironment serves a setup/management dashboard (/) with end-to-end setup instructions (token creation, flyte create secret commands, repository webhook config) and status/verify endpoints, plus an HMAC-verified webhook receiver (/webhook) that normalizes payloads into GitHubEvent objects and dispatches to on_event handlers. launch_task launches runs idempotently via dedupe labels, so webhook redeliveries never launch duplicate runs.

MCP server for agents on Flyte — the read/write surface doubles as MCP tools via build_mcp_server / github_mcp_app_env. Read-only by default; write tools are opt-in and merge_pull_request requires include_destructive=True. Tool annotations (readOnlyHint, destructiveHint, idempotentHint) come from the tool registry. Event ingestion is intentionally not an MCP tool — that stays with the app environment.

Tests

77 tests covering the client (respx-mocked API), webhook signature verification and event normalization, review payload parsing, the MCP tool registry/server, dispatch idempotency, and the app dashboard/receiver endpoints.

Examples

• examples/pr_review_gate.py — condition-gated merge
• examples/read_write_pr.py — basic read/write + triage task
• examples/react_to_pr_events.py — webhook app launching idempotent runs
• examples/github_mcp_server.py — MCP server for agents

Review pass

A follow-up commit fixes issues found while reviewing this branch.

Correctness

• dedupe_key now folds in the comment/review id. Keyed on the issue number alone, every comment on an issue after the first looked like a redelivery of the first and never launched a run.
• create_branch(from_ref="HEAD") resolves the repository's actual default_branch instead of assuming main, which 404s on master/develop repos.
• create_or_update_file reads the existing blob SHA from the target branch. Without ref=, GitHub answers from the default branch, so the SHA either fails the update or writes default-branch content onto the target branch.
• list_repository_files no longer sends a ref query parameter the git trees API ignores — the ref belongs in the path.

Robustness and security

• Webhook signatures compare as bytes. hmac.compare_digest raises TypeError on str operands containing non-ASCII, and the signature header is attacker-controlled: ASGI servers hand Starlette raw header bytes which it decodes as latin-1, so a crafted header turned a clean 401 into a 500.
• request() retries rate limits (429, and 403 carrying Retry-After or an exhausted x-ratelimit-remaining), clamped at 60s so it never sleeps out a long reset window. A plain permissions 403 still surfaces immediately.
• The repos allowlist docstring now states that events carrying no repository are skipped too, which is what the code already did.

Dashboard

• The recent-events table read the 25 oldest events instead of the newest — [:25] on a deque that appends on the right — so it froze after the first 25 events.
• Allowlist values are HTML-escaped like every other interpolated value.

8 new tests cover these.

Follow-up: async launching and label-only idempotency

launch_task no longer blocks the app's event loop. It and blocking_run are now async-first, wrapped with flyte's @syncify, so handlers await launch_task.aio(...). Previously the synchronous call stalled the whole event loop for the duration of two control-plane round-trips plus a launch — with every other in-flight webhook queued behind it, against sender timeouts measured in seconds. The synchronous form still works for scripts. A regression test asserts four concurrent launches overlap; against the old blocking call it measures 0.81s instead of 0.42s and fails.

Idempotency is now purely label-based. The run-name allocation is gone — it probed up to 32 candidate names via Run.get before launching under the winner. That raced with concurrent launches, silently capped how many runs one dedupe key could ever have, and treated a name as an identity it never was. Runs now carry only the dedupe=<key> label and the control plane assigns the name. run_name_for, RUN_NAME_MAX, and the prefix / run_name_base arguments are removed.

The dedupe key is explicitly caller-supplied. dedupe_key() is a sensible default, not a requirement — any string chooses a different idempotency scope.

The module docstring is also honest about the residual race: the label check is a read followed by a launch, so two simultaneous deliveries of one event can both launch. Redeliveries are seconds to minutes apart and dedupe reliably; closing the concurrent case needs a compare-and-set the control plane does not expose.

Follow-up: sync and async call forms on GitHubClient

The 25 client methods are wrapped with flyte's @syncify, so each has two call forms — matching how the SDK itself exposes Run.listall, flyte.run, and flyte.serve:

# async: in `async def` tasks, webhook handlers, MCP tools
async with GitHubClient() as client:
    result = await client.some_method.aio(...)

# blocking: in plain `def` tasks and scripts
with GitHubClient() as client:          # note: `with`, not `async with`
    result = client.some_method(...)

__enter__/__exit__ were added so the blocking form is usable at all. They run __aenter__/__aexit__ on syncify's background loop — the same loop the syncified methods run on — so the httpx.AsyncClient is created and used on a single loop rather than straddling two.

Two call sites had to move to .aio() for correctness, not just style. Internal self. calls would otherwise deadlock, since syncify raises on a blocking call made from its own loop thread. And the MCP tool bridge (await getattr(client, name)(...)) would raise TypeError: object dict can't be used in 'await' expression on the returned value — and, had it not raised, would have stalled the MCP server's event loop for the duration of every tool call. Measured: 0 event-loop ticks during one blocking call.

The dynamic sync_*-method alternative was rejected: it works at runtime but both mypy and ty report "GitHubClient" has no attribute "sync_..." and fall back to Any, so every typed caller breaks. @syncify in place resolves to SyncFunction[[...], ...] in both checkers. The package's mypy error count is unchanged at 12, all pre-existing, none in _client.py.

Tests, examples, and READMEs use .aio() on async paths and document both forms, including when not to reach for the blocking one.

Follow-up: typed event constants

on_event no longer needs hand-copied strings:

from flyteplugins.github import events

@app_env.on_event(events.PullRequest.OPENED)
async def handle(event): ...

flyteplugins.github.events adds 14 classes / 73 constants, following the ActionPhase pattern in flyte.models — str enums grouped by event type, so a member drops in wherever a pattern string is accepted and a typo fails at import rather than by silently never matching. events.PullRequest.ANY matches every action on the type; events.PullRequest.OPENED matches one. Raw strings still work, for events the constants do not cover yet.

The enum base pins __str__/__format__ to str's. Without that, Python 3.11+ renders members as Class.MEMBER rather than the wire value — which would have leaked enum names into the dashboard and /api/status, since handler patterns flow into both.

Tests assert the constants equal what the parser actually produces (through the app's own _matches), that every ANY is the bare type its actions prefix, and that no value appears in two classes.

Follow-up: an end-to-end Testing guide

The README now ends with a numbered walkthrough against a real account, ordered so each step fails in isolation — exercise the client standalone before involving the platform, deploy the launched task before the app that looks it up, run it directly before any webhook is in play. Then wire up the provider, trigger a real event, and check the four places evidence lands: the provider's delivery log, /api/events, flyte get runs, and the resource itself. It ends with a troubleshooting table mapping each symptom to its cause, since 401, 503, and "200 but no run" are three different failures.

It also walks through the idempotency behaviour by hand, which is the part unit tests can only simulate.

Verified mechanically rather than by eye: every referenced example path resolves, every flyte create secret name matches the actual _config.py default, every flyte deploy <file> <env> names a real TaskEnvironment, every flyte run task is defined, and every example file still executes.