diff --git a/.env.example b/.env.example index 0a48a121..03651276 100644 --- a/.env.example +++ b/.env.example @@ -31,6 +31,14 @@ ZULIP_ASSIGNMENT_SUCCESS_EMOJI=thumbs_up # Enable live GitHub assign/unassign mutations (true/false, 1/0, yes/no) ZULIP_ASSIGNMENT_MUTATIONS_ENABLED= +# Zulip close-pr command behavior +# Enable live GitHub PR close mutations (true/false, 1/0, yes/no). +# When unset, the form still renders and post-actions run (dry-run mode) but the PR is not closed. +ZULIP_CLOSE_PR_MUTATIONS_ENABLED= +# Per-repo Zulip audit log destinations (JSON). If a repo has no entry, log posts are skipped. +# Example: {"leanprover-community/mathlib4": {"stream": "mathlib4", "topic": "bot actions"}} +ZULIP_REPO_LOG= + # Core Django settings DJANGO_SETTINGS_MODULE=qb_site.settings.local DJANGO_SECRET_KEY=change-me @@ -85,11 +93,11 @@ GITHUB_OAUTH_SCOPE=read:user # Shared secret used to validate GitHub webhook signatures (X-Hub-Signature-256). GITHUB_WEBHOOK_SECRET= # JSON object describing GitHub App credentials and operation mapping. -# Assignment commands require app-token resolution for assign/unassign operations. -# Syncer operations try app tokens first and fall back to GH_TOKEN/GITHUB_TOKEN. (see `manage.py github_app_config`) +# Assignment and close-pr commands require app-token resolution. Syncer operations try app tokens +# first and fall back to GH_TOKEN/GITHUB_TOKEN. (see `manage.py github_app_config`) GITHUB_APP_TOKEN_CONFIG= # Example: -# GITHUB_APP_TOKEN_CONFIG={"api_base_url":"https://api.github.com","cache_skew_seconds":60,"operation_app_map":{"assign_pr":"queueboard-assignment","unassign_pr":"queueboard-assignment","syncer_repo_discovery":"queueboard-syncer-read","syncer_pr_read":"queueboard-syncer-read","syncer_ci_read":"queueboard-syncer-read"},"apps":[{"name":"queueboard-assignment","app_id":123456,"private_key_path":"/run/secrets/queueboard-assignment.pem","installation_lookup":"repo","operations":["assign_pr","unassign_pr"]},{"name":"queueboard-syncer-read","app_id":234567,"private_key_path":"/run/secrets/queueboard-syncer-read.pem","installation_lookup":"owner","installation_owner_type":"org","installation_owner":"leanprover-community","operations":["syncer_repo_discovery","syncer_pr_read","syncer_ci_read"]}]} +# GITHUB_APP_TOKEN_CONFIG={"api_base_url":"https://api.github.com","cache_skew_seconds":60,"operation_app_map":{"assign_pr":"queueboard-assignment","unassign_pr":"queueboard-assignment","close_pr":"queueboard-assignment","check_collaborator_permission":"queueboard-org-read","syncer_repo_discovery":"queueboard-syncer-read","syncer_pr_read":"queueboard-syncer-read","syncer_ci_read":"queueboard-syncer-read"},"apps":[{"name":"queueboard-assignment","app_id":123456,"private_key_path":"/run/secrets/queueboard-assignment.pem","installation_lookup":"repo","operations":["assign_pr","unassign_pr","close_pr"]},{"name":"queueboard-org-read","app_id":345678,"private_key_path":"/run/secrets/queueboard-org-read.pem","installation_lookup":"owner","installation_owner_type":"org","installation_owner":"leanprover-community","operations":["check_collaborator_permission"]},{"name":"queueboard-syncer-read","app_id":234567,"private_key_path":"/run/secrets/queueboard-syncer-read.pem","installation_lookup":"owner","installation_owner_type":"org","installation_owner":"leanprover-community","operations":["syncer_repo_discovery","syncer_pr_read","syncer_ci_read"]}]} # Syncer scheduling defaults # How far back to look for changed PRs each tick (minutes) diff --git a/docs/design-decisions/041-zulip-close-pr-command.md b/docs/design-decisions/041-zulip-close-pr-command.md new file mode 100644 index 00000000..af23b047 --- /dev/null +++ b/docs/design-decisions/041-zulip-close-pr-command.md @@ -0,0 +1,261 @@ +# Zulip Close-PR Command + +## Context +- Certain Queueboard users have GitHub write/maintain/admin access and sometimes need to close PRs without + their personal GitHub identity being visible (e.g. to keep moderation actions neutral). +- Existing Zulip bot commands use `ZULIP_COMMAND_POLICY` for access control, but that only gates which + Zulip users can invoke a command — it does not verify that the invoker actually has the corresponding + GitHub permission at runtime. +- Closing a PR is a destructive, hard-to-reverse action; it requires a confirmation step beyond a single + command invocation. +- The existing secure-link + web form pattern (used by `prefs` and registration) is a natural fit for a + confirmation flow. +- The `queueboard-assignment` GitHub App currently has `Issues: Read and write` and `Pull requests: + Read-only`. Closing a PR via `PATCH /repos/{owner}/{repo}/pulls/{number}` requires `Pull requests: + Read and write`. Checking a user's collaborator permission via + `GET /repos/{owner}/{repo}/collaborators/{username}/permission` requires `Members: Read` (org-level). + To limit blast radius, a separate `queueboard-org-read` app carries `Members: Read` and handles + the `check_collaborator_permission` operation; `queueboard-assignment` only needs `Pull requests: + Read and write` (no org-level permissions). The close-PR code falls back to the `close_pr` token + if `check_collaborator_permission` is not mapped, for single-app setups. + +## Goals / Non-Goals +- Goals: + - Let authorized users close a PR from Zulip with their personal GitHub identity hidden (action + attributed to the GitHub App). + - Verify at command time that the invoker has GitHub write/maintain/admin on the target repo, or is the + PR author. + - Require explicit confirmation before any mutation. + - Log closes to a configurable per-repo Zulip stream/topic. +- Non-goals: + - Re-opening PRs. + - Closing PRs on repos not tracked by Queueboard. + - Storing or exposing the user's personal GitHub OAuth token. + +## Proposed Design + +### Command: `close-pr` +- Syntax: `close-pr ` +- Registered in `qb_site/zulip_bot/commands/close_pr.py`. +- Response mode: `PRIVATE` (link is sent only to the invoker). +- Flow: + 1. Parse exactly one PR URL from args / `rendered_content` (reuse `_parse_single_pr_ref` logic). + 2. Resolve invoker's `core.User` by `context.sender_id` (`zulip_user_id`); require `github_login`. + 3. Check permission (see below). + 4. Issue an encrypted `close-pr` token and send private link. +- Feature flag: `ZULIP_CLOSE_PR_MUTATIONS_ENABLED`. When disabled, command still validates and issues + the link but the form view blocks the actual close and returns a preflight-only message. + +### Permission Check (command time) +Performed using a GitHub App token for operation `close_pr` (mapped to `queueboard-assignment`): + +1. Fetch PR via `GET /repos/{owner}/{repo}/pulls/{number}`: + - Check PR is open; if not, return private "PR is not open" message (no link issued). + - Extract `user.login` to identify the PR author. +2. If `github_login == pr_author_login` (case-insensitive): **permitted**. +3. Otherwise call `GET /repos/{owner}/{repo}/collaborators/{github_login}/permission`: + - If `permission` in `{"write", "admin"}`: **permitted** (`maintain` role maps to `write` here). + - Otherwise: return private "you don't have permission to close this PR" message. + +### Token Service: `close_pr_links.py` +- Pattern mirrors `prefs_links.py` (Fernet encryption, `iat`/`exp` claims). +- Claims: `zulip_user_id`, `github_login`, `pr_owner`, `pr_repo`, `pr_number`. +- Settings: + - `ZULIP_CLOSE_PR_TOKEN_SECRET` (falls back to `SECRET_KEY`) + - `ZULIP_CLOSE_PR_TOKEN_SALT` (default: `"zulip_bot.close_pr"`) + - `ZULIP_CLOSE_PR_TOKEN_TTL_SECONDS` (default: `1800`) + - `ZULIP_PREFS_URL_BASE` (shared with prefs/registration links; falls back to relative path) + +### Execution Service: `close_pr_execution.py` +- `check_close_pr_permission(github_login, owner, repo, number)` → `PermissionCheckResult` + - Fetches PR + optionally checks collaborator permission. + - Returns structured result: `permitted`, `not_permitted`, `pr_not_open`, `token_unavailable`. +- `close_pull_request(owner, repo, number)` → success/error + - `PATCH /repos/{owner}/{repo}/pulls/{number}` with `{"state": "closed"}`. + - Uses `close_pr` operation token (same `queueboard-assignment` app). + +### Web Layer +- URL: `close-pr//` → `views.close_pr_form` +- View behavior: + - Validate token; show `close_pr_invalid.html` (HTTP 403) on expiry/invalid. + - Fetch live PR details (title, state) from GitHub to display in form. + - **GET**: render confirmation form. If PR is already closed/merged, show informational message + without a submit button (no redirect, no error page — same template, conditional rendering). + - **POST**: if `ZULIP_CLOSE_PR_MUTATIONS_ENABLED` is off, show preflight-only message. Otherwise: + 1. Close PR via execution service. + 2. Enqueue `sync_pr_task.delay(repository_id, pr_number)` (best-effort). + 3. Send DM to invoker confirming the close (best-effort; failure logged, not surfaced). + 4. Post to repo log thread if `ZULIP_REPO_LOG` has an entry for `owner/repo` (best-effort). + 5. Render success state in same template. + - `Cache-Control: no-store` on all responses. + +### Settings +- `ZULIP_CLOSE_PR_MUTATIONS_ENABLED` — feature flag (same pattern as assignment mutations flag). +- `ZULIP_REPO_LOG` — JSON object mapping `"owner/repo"` to `{"stream": "...", "topic": "..."}`. + If a repo has no entry, the log post step is skipped and a `WARNING` is emitted. + Designed to be reusable for future per-repo Zulip logging beyond close-PR. + + Example: + ```json + { + "leanprover-community/mathlib4": { + "stream": "mathlib4", + "topic": "bot actions" + } + } + ``` + +### GitHub App Changes (manual) +- Add to `queueboard-assignment`: + - `Pull requests: Read and write` (upgrade from read-only) +- Create new app `queueboard-org-read`: + - `Metadata: Read-only` (baseline) + - `Members: Read` (org-level permission) + - Install on the org with owner-level installation lookup +- Update `docs/github_app_setup.md` and `GITHUB_APP_TOKEN_CONFIG` operation map to include + `close_pr` (→ `queueboard-assignment`) and `check_collaborator_permission` + (→ `queueboard-org-read`). + +## Subtleties / Invariants +- Permission check uses the GitHub App token, not the user's personal token. The close itself is also + performed by the GitHub App. The invoker's personal GitHub identity is never exposed. +- The collaborator permission endpoint returns `permission: "none"` for users with no access (not 404). + `maintain` role maps to `permission: "write"` in this endpoint's response. +- If `check_close_pr_permission` can't obtain a GitHub App token (app not installed on repo, config + missing), the command fails privately with a clear message — same behavior as assignment mutations. +- The form token embeds `github_login` at issuance time. If the user's GitHub login changes between + issuance and form submission, the close still proceeds using the embedded login for attribution + purposes (acceptable; the login change is rare and the token is short-lived). +- Sync enqueue, DM, and log post are all best-effort: failures are logged but do not cause the HTTP + response to indicate failure. The PR close itself is the authoritative outcome. +- The `ZULIP_REPO_LOG` setting is intentionally generic (not named `ZULIP_CLOSE_PR_LOG`) to allow + other future bot operations to reuse the same per-repo destination config. + +## Implementation Plan + +### Commit 1 (this doc) ✓ +- Add `docs/design-decisions/041-zulip-close-pr-command.md`. + +### Commit 2 (implementation) ✓ +- New files: + - `qb_site/zulip_bot/commands/close_pr.py` + - `qb_site/zulip_bot/services/close_pr_links.py` + - `qb_site/zulip_bot/services/close_pr_execution.py` + - `qb_site/templates/zulip_bot/close_pr_form.html` + - `qb_site/templates/zulip_bot/close_pr_invalid.html` +- Modified files: + - `qb_site/zulip_bot/views.py` — add `close_pr_form` view, import command module + - `qb_site/zulip_bot/urls.py` — add `close-pr//` + - `qb_site/zulip_bot/AGENTS.md` / `CLAUDE.md` — document new command and settings + - `docs/github_app_setup.md` — add new app permissions and `close_pr` operation +- Also included two follow-up fixes: + - `fix(zulip_bot): normalise underscores to hyphens in command name parser` — so + `close_pr`, `assigned_prs`, `pr_info` variants resolve to the canonical hyphenated names + without per-command aliases. + - `fix(zulip_bot): fix two test_close_pr_form assertions` — missing + `ZULIP_CLOSE_PR_MUTATIONS_ENABLED` override in one test and a wrong error-substring check + in another. + +### Commit 3 (follow-up: custom close message) ✓ +- Textarea on confirmation form for an optional close message. +- Preset buttons load pre-written messages into the textarea. Presets are stored as `.md` files in + `qb_site/zulip_bot/close_pr_presets/`; the first `# Heading` line becomes the button label, the + remainder becomes the body. Files are sorted by filename so numbering controls display order. No + code change required to add or edit presets. +- On submit, if a message is provided it is posted as a PR comment + (`POST /repos/{owner}/{repo}/issues/{number}/comments`) before the PR is closed. If the comment + post fails, an inline error is shown and the PR is not closed (user can retry or clear the + message). No additional GitHub App permissions required (`Issues: Read and write` already granted). +- New service: `qb_site/zulip_bot/services/close_pr_presets.py` (`load_close_pr_presets`). +- New execution helper: `post_pr_comment` in `close_pr_execution.py`. +- New CSS: `qb_site/zulip_bot/static/zulip_bot/close_pr_form.css`. +- The error state now renders inline in the form (rather than a dead-end page) so the user's + typed message is preserved for retry. + +## Validation Plan +- Tests: + - Token service: issue → validate round-trip; expiry; invalid/tampered tokens. + - Permission check: permitted as author; permitted as write collaborator; denied; PR not open; + token unavailable. + - Command handler: no linked user; linked but no GitHub login; not permitted; permitted (link issued); + feature-flag preflight path. + - View (GET): valid open PR; valid already-closed PR; expired token; invalid token. + - View (POST): successful close with sync enqueue + DM + log post; mutations disabled preflight; + GitHub error on close. + - `ZULIP_REPO_LOG` parsing: valid config; missing repo entry (skip + warn); malformed config. +- Manual checks: + - Verify `GET /repos/{owner}/{repo}/collaborators/{username}/permission` response shape and `"none"` + behavior for non-collaborators with the updated `queueboard-assignment` app credentials. + - Verify `PATCH /repos/{owner}/{repo}/pulls/{number}` with `{"state": "closed"}` succeeds with the + updated app token. + - End-to-end: issue command → receive link → open form → submit → PR closed → DM received → + log thread updated. + +## Operational Deployment Notes + +### Before enabling in production +1. **Update the `queueboard-assignment` GitHub App** (manual step in the GitHub UI): + - Upgrade `Pull requests` from Read-only to **Read and write**. + - After saving, GitHub will ask any installation admins to approve the expanded permissions. + Existing `assign`/`unassign` operations continue with the old token scope until approved, + so coordinate approval with minimal downtime. +2. **Create the `queueboard-org-read` GitHub App** (manual step): + - Set `Metadata: Read-only` and org-level `Members: Read`. + - Install on the org (owner-level installation). + - Generate a private key and store it in your secret store. +3. **Update `GITHUB_APP_TOKEN_CONFIG`** to include `close_pr` (→ `queueboard-assignment`) and + `check_collaborator_permission` (→ `queueboard-org-read`) in the `operation_app_map`, and add + both apps to the `apps` list. See `docs/github_app_setup.md` for the full example config. +4. **Add `close-pr` to `ZULIP_COMMAND_POLICY`**. Without a policy entry the command is silently + ignored for all users. Restrict `allowed_groups` to the Zulip group(s) whose members should be + able to invoke the command (GitHub permission is still checked at runtime). Allow both DM and + stream contexts so the command can be used from either: + ```json + { + "close-pr": { + "allowed_groups": [], + "allowed_contexts": ["dm", "stream:*"] + } + } + ``` + Use `manage.py zulip_policy` to build and validate the JSON before setting the env var. +5. **Add `ZULIP_REPO_LOG`** to settings/env for each repo you want audit-log posts in Zulip: + ```json + { + "leanprover-community/mathlib4": { + "stream": "mathlib4", + "topic": "bot actions" + } + } + ``` + If a repo has no entry, closes still work but a WARNING is logged and no Zulip log post is + sent. The setting is intentionally generic for reuse by future operations. +6. **Add `ZULIP_CLOSE_PR_MUTATIONS_ENABLED = True`** to enable actual PR closes. Without this + flag the form renders and the POST runs all post-actions (sync enqueue, DM, log post) but does + not call GitHub — useful for end-to-end smoke testing without closing the PR. +7. **Optional token settings** (all have reasonable defaults): + - `ZULIP_CLOSE_PR_TOKEN_SECRET` (falls back to `SECRET_KEY`) + - `ZULIP_CLOSE_PR_TOKEN_SALT` (default: `"zulip_bot.close_pr"`) + - `ZULIP_CLOSE_PR_TOKEN_TTL_SECONDS` (default: `1800`) + - `ZULIP_PREFS_URL_BASE` (shared with prefs/registration links; falls back to relative path) + +### Manual verification checklist (after app permissions are updated) +- [ ] `GET /repos/{owner}/{repo}/collaborators/{username}/permission` returns `{"permission": + "none"}` for a non-collaborator (not 404). +- [ ] `GET /repos/{owner}/{repo}/collaborators/{username}/permission` returns `"write"` for a + user with the `maintain` role. +- [ ] `PATCH /repos/{owner}/{repo}/pulls/{number}` with `{"state": "closed"}` succeeds using + the updated `queueboard-assignment` installation token. +- [ ] End-to-end: issue `close-pr ` in Zulip → receive private link → open form → + confirm → PR closed on GitHub → DM received → log thread updated in Zulip. +- [ ] Issue command as a non-collaborator who is not the PR author → receive "you don't have + permission" private message, no link issued. +- [ ] Issue command for an already-closed PR → receive "PR is not open" private message. +- [ ] Open confirmation link after TTL expires → see 403 invalid-token page. + +## Progress Notes +- 2026-04-12: Design doc written. +- 2026-04-12: Implementation complete (commits 1 and 2 landed on `close-pr-command` branch). + All unit tests pass under Compose. GitHub App permission upgrade and production env vars are + the remaining manual steps before the feature can be enabled. +- 2026-04-17: Commit 3 complete — optional close message with markdown presets added. diff --git a/docs/github_app_setup.md b/docs/github_app_setup.md index 5076d6f6..0fa5ab4b 100644 --- a/docs/github_app_setup.md +++ b/docs/github_app_setup.md @@ -4,13 +4,19 @@ This guide covers how to create and configure GitHub Apps for Queueboard's opera ## Scope and Current Token Policy - Assignment commands (`assign_pr`, `unassign_pr`) require GitHub App installation tokens. +- Close-PR command (`close_pr`) requires a GitHub App installation token with `Pull requests: Read and write` and `Members: Read` (org-level). - Syncer operations (`syncer_repo_discovery`, `syncer_pr_read`, `syncer_ci_read`) try GitHub App tokens first and fall back to `GH_TOKEN`/`GITHUB_TOKEN` when no app token is available. - Runtime config is loaded from `GITHUB_APP_TOKEN_CONFIG` (JSON object). ## Recommended App Layout -- Use two apps (recommended): - - `queueboard-assignment`: minimal write permissions for assignee mutations. +- Use three apps (recommended): + - `queueboard-assignment`: minimal write permissions for assignee mutations and PR closes. + - `queueboard-org-read`: org-level read permission for collaborator/membership checks. - `queueboard-syncer-read`: read-only permissions for sync/discovery. +- Keeping `queueboard-org-read` separate avoids granting org-level `Members: Read` to the + assignment app, limiting blast radius. If you prefer simplicity, `queueboard-assignment` can + cover `check_collaborator_permission` as well — the code falls back to the `close_pr` token + when `check_collaborator_permission` is not mapped. - One app for all operations also works, but increases blast radius and over-permission risk. ## 1) Create GitHub Apps Under the Organization @@ -34,15 +40,20 @@ Notes: Set only the permissions needed by each operation set. -### App A: `queueboard-assignment` (assign/unassign) +### App A: `queueboard-assignment` (assign/unassign/close-pr) - Repository permissions: - `Issues: Read and write` (required for `POST/DELETE /repos/{owner}/{repo}/issues/{number}/assignees`) - - `Pull requests: Read-only` (required for live precondition checks on PR state/assignees) + - `Pull requests: Read and write` (required for close-PR mutation via `PATCH /repos/{owner}/{repo}/pulls/{number}`; also used for live precondition checks on PR state/assignees) - `Metadata: Read-only` (baseline repository visibility/lookup behavior) +- Organization permissions: none (collaborator checks are delegated to `queueboard-org-read`) + +### App B: `queueboard-org-read` (collaborator/membership checks) +- Repository permissions: + - `Metadata: Read-only` (baseline) - Organization permissions: - - none required + - `Members: Read` (required for `GET /repos/{owner}/{repo}/collaborators/{username}/permission`) -### App B: `queueboard-syncer-read` (syncer read operations) +### App C: `queueboard-syncer-read` (syncer read operations) - Repository permissions: - If Queueboard only targets public repositories, you can leave repository permissions at `No access`. - Recommended default (safer when repo visibility or API usage changes): @@ -87,7 +98,7 @@ Set these values in `.env` or your deployment secret manager: - `private_key` (PEM string; use `\\n` escapes in env JSON; preferred for current Heroku deployment), or - `private_key_path` (path to PEM file visible to the process) -Example (two-app config): +Example (three-app config): ```json { @@ -96,6 +107,8 @@ Example (two-app config): "operation_app_map": { "assign_pr": "queueboard-assignment", "unassign_pr": "queueboard-assignment", + "close_pr": "queueboard-assignment", + "check_collaborator_permission": "queueboard-org-read", "syncer_repo_discovery": "queueboard-syncer-read", "syncer_pr_read": "queueboard-syncer-read", "syncer_ci_read": "queueboard-syncer-read" @@ -106,7 +119,16 @@ Example (two-app config): "app_id": 123456, "private_key_path": "/run/secrets/queueboard-assignment.pem", "installation_lookup": "repo", - "operations": ["assign_pr", "unassign_pr"] + "operations": ["assign_pr", "unassign_pr", "close_pr"] + }, + { + "name": "queueboard-org-read", + "app_id": 345678, + "private_key_path": "/run/secrets/queueboard-org-read.pem", + "installation_lookup": "owner", + "installation_owner_type": "org", + "installation_owner": "leanprover-community", + "operations": ["check_collaborator_permission"] }, { "name": "queueboard-syncer-read", @@ -121,6 +143,11 @@ Example (two-app config): } ``` +Note: if `check_collaborator_permission` is not present in `operation_app_map` (e.g. during a +phased rollout or single-app setup), the close-PR command falls back to using the `close_pr` +token for the collaborator check — but that requires `queueboard-assignment` to also have +`Members: Read`. + Helper command for generating/maintaining this JSON: ```bash diff --git a/qb_site/qb_site/settings/base.py b/qb_site/qb_site/settings/base.py index 38bcbb64..5167d631 100644 --- a/qb_site/qb_site/settings/base.py +++ b/qb_site/qb_site/settings/base.py @@ -168,6 +168,7 @@ def env_optional_bounded_int(name: str, *, minimum: int, maximum: int) -> int | ZULIP_REGISTRATION_OAUTH_STATE_TTL_SECONDS = int(os.getenv("ZULIP_REGISTRATION_OAUTH_STATE_TTL_SECONDS", 600)) ZULIP_ASSIGNMENT_SUCCESS_EMOJI = os.getenv("ZULIP_ASSIGNMENT_SUCCESS_EMOJI", "thumbs_up") ZULIP_ASSIGNMENT_MUTATIONS_ENABLED = os.getenv("ZULIP_ASSIGNMENT_MUTATIONS_ENABLED", "") +ZULIP_CLOSE_PR_MUTATIONS_ENABLED = os.getenv("ZULIP_CLOSE_PR_MUTATIONS_ENABLED", "") GITHUB_OAUTH_CLIENT_ID = os.getenv("GITHUB_OAUTH_CLIENT_ID", "") GITHUB_OAUTH_CLIENT_SECRET = os.getenv("GITHUB_OAUTH_CLIENT_SECRET", "") GITHUB_OAUTH_REDIRECT_URI = os.getenv("GITHUB_OAUTH_REDIRECT_URI", "") @@ -190,6 +191,13 @@ def env_optional_bounded_int(name: str, *, minimum: int, maximum: int) -> int | if not isinstance(parsed_policy, dict): raise RuntimeError("ZULIP_COMMAND_POLICY env var must be a JSON object") ZULIP_COMMAND_POLICY = parsed_policy +ZULIP_REPO_LOG: dict[str, dict[str, str]] = {} +_ZULIP_REPO_LOG_ENV = os.getenv("ZULIP_REPO_LOG", "").strip() +if _ZULIP_REPO_LOG_ENV: + parsed_repo_log = json.loads(_ZULIP_REPO_LOG_ENV) + if not isinstance(parsed_repo_log, dict): + raise RuntimeError("ZULIP_REPO_LOG env var must be a JSON object") + ZULIP_REPO_LOG = parsed_repo_log # Celery configuration diff --git a/qb_site/templates/zulip_bot/close_pr_form.html b/qb_site/templates/zulip_bot/close_pr_form.html new file mode 100644 index 00000000..c6e09682 --- /dev/null +++ b/qb_site/templates/zulip_bot/close_pr_form.html @@ -0,0 +1,122 @@ +{% load static %} + + + + + + Queueboard — close pull request + + + + + +
+
+

Close Pull Request

+

+ {{ pr_owner }}/{{ pr_repo }}#{{ pr_number }} +

+
+ +
+ {% if pr_title %} +

{{ pr_title }}

+ {% endif %} + {% if pr_author_login or pr_opened_at or pr_updated_at %} +
+ {% if pr_author_login %} + By @{{ pr_author_login }} + {% endif %} + {% if pr_opened_at %} + Opened + {% endif %} + {% if pr_updated_at %} + Updated + {% endif %} +
+ {% endif %} + {% if pr_labels %} +
+ {% for label in pr_labels %} + {{ label.name }} + {% endfor %} +
+ {% endif %} + {% if pr_body %} +
{{ pr_body }}
+ {% endif %} +
+ + {% if success %} +
+ {% if preflight_only %} +
+ Dry-run complete — the pull request was not closed + (ZULIP_CLOSE_PR_MUTATIONS_ENABLED is not set). + Post-actions (sync enqueue, Zulip DM, log post) were triggered normally. +
+ {% else %} +
+ Pull request {{ pr_owner }}/{{ pr_repo }}#{{ pr_number }} has been closed. + The close was attributed to the bot. A confirmation has been sent to you via Zulip DM. +
+ {% endif %} +
+ + {% elif not pr_is_open %} +
+
+ This pull request is already closed or merged — if you closed it just now, you're done. + If it was closed by someone else, no further action is needed. +
+
+ + {% else %} +
+ {% if close_error %} + + {% endif %} + {% if mutations_disabled %} +
+ Dry-run mode: ZULIP_CLOSE_PR_MUTATIONS_ENABLED is not set. + Submitting will trigger all post-actions (sync enqueue, Zulip DM, log post) but will + not close the pull request. +
+ {% endif %} +

+ This action will be attributed to the bot, not your personal GitHub account. + Closing a pull request is reversible (it can be re-opened), but please confirm you intend to proceed. +

+
+ {% csrf_token %} +
+ {% if presets %} +
+ Presets: + {% for preset in presets %} + + {% endfor %} +
+ {% endif %} + + +
+ +
+
+ +
+ Expires in: + calculating… + Expires at: +
+ {% endif %} +
+ + + diff --git a/qb_site/templates/zulip_bot/close_pr_invalid.html b/qb_site/templates/zulip_bot/close_pr_invalid.html new file mode 100644 index 00000000..f4137087 --- /dev/null +++ b/qb_site/templates/zulip_bot/close_pr_invalid.html @@ -0,0 +1,25 @@ +{% load static %} + + + + + + Queueboard — close PR link unavailable + + + + +
+
+

Close PR link unavailable

+
+
+ {% if reason == "expired" %} +

This link has expired. Send close-pr <pr-url> to the bot again for a fresh link.

+ {% else %} +

This link is invalid. Send close-pr <pr-url> to the bot for a fresh link.

+ {% endif %} +
+
+ + diff --git a/qb_site/zulip_bot/AGENTS.md b/qb_site/zulip_bot/AGENTS.md index a42a93fc..b77acc2f 100644 --- a/qb_site/zulip_bot/AGENTS.md +++ b/qb_site/zulip_bot/AGENTS.md @@ -17,27 +17,34 @@ cd qb_site/zulip_bot/frontend && npm test ``` ## Command Architecture Notes -- Commands live in `commands/`: `assign`, `unassign`, `assigned-prs`, `pr-info`, `prefs`, `help`, `echo`, `register_test`. +- Commands live in `commands/`: `assign`, `unassign`, `assigned-prs`, `pr-info`, `prefs`, `help`, `echo`, `register_test`, `close-pr`. - `pr-info`: parses GitHub PR links from Zulip `rendered_content`, reacts with 👀, then sends one stream message per PR (up to 10) with queue info sourced from `analyzer.services.pr_info`. - Assignment command flow (all under `services/`) is split for clarity: - parse: `assignment_command_parser.py`, - validate: `assignment_validation.py`, - preflight/mutation orchestration: `assignment_execution.py` + `assignment_preflight.py`. +- `close-pr` command: checks GitHub permission at command time, then issues a short-lived private link to a confirmation form. Services: `close_pr_links.py` (token), `close_pr_execution.py` (permission check + GitHub mutation). Feature flag: `ZULIP_CLOSE_PR_MUTATIONS_ENABLED`. Uses operation `close_pr` via the GitHub App token system. - Keep user-facing command responses explicit and safe for partial failures. - Prefer private failure responses for sensitive mutation/policy errors. ## Policy and Safety Notes - Command availability and context restrictions are controlled by `ZULIP_COMMAND_POLICY`. -- Mutation paths are feature-flagged (`ZULIP_ASSIGNMENT_MUTATIONS_ENABLED`) and depend on GitHub operation-token services. +- Mutation paths are feature-flagged (`ZULIP_ASSIGNMENT_MUTATIONS_ENABLED`, `ZULIP_CLOSE_PR_MUTATIONS_ENABLED`) and depend on GitHub operation-token services. - Do not log secrets/tokens or raw sensitive payload fragments. +## Per-Repo Zulip Log +- `ZULIP_REPO_LOG` is a JSON setting mapping `"owner/repo"` to `{"stream": "...", "topic": "..."}`. +- Used by `close-pr` (and intended for future operations) to post an audit log entry after a mutation. +- If a repo has no entry, the log post is skipped and a WARNING is emitted. + ## Registration and Preferences - Registration-link/state behavior is in `services/`: - `registration_links.py`, - `registration_oauth_state.py`, - `registration_linking.py`, - `registration_bootstrap.py` (initial bootstrap helpers), - - `prefs_links.py` (preference deep-link generation). + - `prefs_links.py` (preference deep-link generation), + - `close_pr_links.py` (close-PR confirmation link generation). - Zulip prefs form/UI behavior spans Django forms/views and `frontend/` tests; keep behavior parity across backend validation and frontend affordances. ## Testing Expectations diff --git a/qb_site/zulip_bot/close_pr_presets/00-ai-generated.md b/qb_site/zulip_bot/close_pr_presets/00-ai-generated.md new file mode 100644 index 00000000..0cd307c8 --- /dev/null +++ b/qb_site/zulip_bot/close_pr_presets/00-ai-generated.md @@ -0,0 +1,4 @@ +# AI-generated + +We appreciate your desire to contribute to mathlib, but unfortunately we will have to close this PR. +The LLM-generated code in this PR fails to meet our standards, and while reviewers and maintainers are willing to help new contributors to improve the quality of their code through the review process, this can only be done if the code is indeed written by somebody who learns in the process, otherwise the quality will not improve in future PRs and the effort required will be larger than the benefit. diff --git a/qb_site/zulip_bot/close_pr_presets/01-superseded.md b/qb_site/zulip_bot/close_pr_presets/01-superseded.md new file mode 100644 index 00000000..5a161057 --- /dev/null +++ b/qb_site/zulip_bot/close_pr_presets/01-superseded.md @@ -0,0 +1,3 @@ +# Superseded + +This PR is being closed because it has been superseded by another PR. Thank you for your contribution! diff --git a/qb_site/zulip_bot/close_pr_presets/02-stale.md b/qb_site/zulip_bot/close_pr_presets/02-stale.md new file mode 100644 index 00000000..242bd3cd --- /dev/null +++ b/qb_site/zulip_bot/close_pr_presets/02-stale.md @@ -0,0 +1,3 @@ +# Stale + +This PR is being closed due to inactivity. Please feel free to reopen it or open a new PR if you'd like to continue working on this. diff --git a/qb_site/zulip_bot/close_pr_presets/03-out-of-scope.md b/qb_site/zulip_bot/close_pr_presets/03-out-of-scope.md new file mode 100644 index 00000000..c36055bb --- /dev/null +++ b/qb_site/zulip_bot/close_pr_presets/03-out-of-scope.md @@ -0,0 +1,3 @@ +# Out of scope + +This PR is being closed as it falls outside the current scope of the project. Thank you for the contribution — we encourage you to discuss the direction on Zulip before opening a new PR. diff --git a/qb_site/zulip_bot/commands/assigned_prs.py b/qb_site/zulip_bot/commands/assigned_prs.py index e1db8a1a..e965867e 100644 --- a/qb_site/zulip_bot/commands/assigned_prs.py +++ b/qb_site/zulip_bot/commands/assigned_prs.py @@ -28,7 +28,6 @@ name="assigned-prs", description="Show your assigned open PRs with queue-time status.", response_mode=ResponseMode.PRIVATE, - aliases=("assigned_prs",), ) def assigned_prs_command(context: CommandContext, args: str) -> CommandResult: del args diff --git a/qb_site/zulip_bot/commands/close_pr.py b/qb_site/zulip_bot/commands/close_pr.py new file mode 100644 index 00000000..edd633fe --- /dev/null +++ b/qb_site/zulip_bot/commands/close_pr.py @@ -0,0 +1,118 @@ +from __future__ import annotations + +import logging +from datetime import timedelta + +from django.conf import settings +from django.utils import timezone + +from core.models import User +from zulip_bot.commands import CommandContext, CommandResult, ResponseMode, register_command +from zulip_bot.services.assignment_command_parser import AssignmentCommandParseError, _parse_single_pr_ref +from zulip_bot.services.close_pr_execution import PermissionOutcome, check_close_pr_permission +from zulip_bot.services.close_pr_links import ClosePRLinkClaims, build_close_pr_link +from zulip_bot.services.zulip_client import ZulipApiError, ZulipClient + +logger = logging.getLogger(__name__) + + +@register_command( + name="close-pr", + description="Open a private confirmation form to close a pull request (requires GitHub write access or PR authorship).", + response_mode=ResponseMode.PRIVATE, +) +def close_pr_command(context: CommandContext, args: str) -> CommandResult: + if context.sender_id is None: + return CommandResult( + content="Could not determine your Zulip identity from this message.", + response_mode=ResponseMode.PRIVATE, + ) + + try: + pr = _parse_single_pr_ref(args=args, rendered_content=context.rendered_content) + except AssignmentCommandParseError as exc: + return CommandResult( + content=f"Could not parse `close-pr` command: {exc.message}", + response_mode=ResponseMode.PRIVATE, + ) + + user = User.objects.filter(zulip_user_id=context.sender_id).only("id", "github_login").first() + if user is None: + return CommandResult( + content=("No GitHub account is linked to your Zulip profile. Use the `prefs` command to register."), + response_mode=ResponseMode.PRIVATE, + ) + + github_login = (user.github_login or "").strip() + if not github_login: + return CommandResult( + content="Your Queueboard profile does not have a GitHub login set.", + response_mode=ResponseMode.PRIVATE, + ) + + result = check_close_pr_permission( + github_login=github_login, + owner=pr.owner, + repo=pr.repo, + number=pr.number, + ) + + if result.outcome == PermissionOutcome.TOKEN_UNAVAILABLE: + return CommandResult( + content=(f"GitHub App token for `close_pr` is not available for `{pr.owner}/{pr.repo}`. Contact an administrator."), + response_mode=ResponseMode.PRIVATE, + ) + + if result.outcome == PermissionOutcome.GITHUB_ERROR: + return CommandResult( + content=(f"Could not fetch PR details for `{pr.owner}/{pr.repo}#{pr.number}` from GitHub. Please try again."), + response_mode=ResponseMode.PRIVATE, + ) + + if result.outcome == PermissionOutcome.PR_NOT_OPEN: + pr_ref = f"`{pr.owner}/{pr.repo}#{pr.number}`" + title_suffix = f' ("{result.pr_title}")' if result.pr_title else "" + return CommandResult( + content=f"Pull request {pr_ref}{title_suffix} is not open.", + response_mode=ResponseMode.PRIVATE, + ) + + if result.outcome == PermissionOutcome.NOT_PERMITTED: + return CommandResult( + content=( + f"Your GitHub account (`{github_login}`) does not have permission " + f"to close `{pr.owner}/{pr.repo}#{pr.number}`. " + "Only the PR author or a collaborator with write/admin access may close it." + ), + response_mode=ResponseMode.PRIVATE, + ) + + # PERMITTED — react to acknowledge, then issue the confirmation link. + if context.message_id is not None: + try: + ZulipClient().add_reaction(message_id=context.message_id, emoji_name="eyes") + except ZulipApiError: + logger.warning("close_pr_reaction_failed", extra={"message_id": context.message_id}) + + link = build_close_pr_link( + claims=ClosePRLinkClaims( + zulip_user_id=context.sender_id, + github_login=github_login, + pr_owner=pr.owner, + pr_repo=pr.repo, + pr_number=pr.number, + ) + ) + ttl_seconds = int(getattr(settings, "ZULIP_CLOSE_PR_TOKEN_TTL_SECONDS", 1800)) + expires_at = timezone.now() + timedelta(seconds=ttl_seconds) + expires_unix = int(expires_at.timestamp()) + pr_ref = f"`{pr.owner}/{pr.repo}#{pr.number}`" + title_suffix = f' ("{result.pr_title}")' if result.pr_title else "" + return CommandResult( + content=( + f"Use this private link to [confirm closing PR {pr_ref}{title_suffix}]({link}). " + f"It expires at . " + "The close will be attributed to the bot, not your personal GitHub account." + ), + response_mode=ResponseMode.PRIVATE, + ) diff --git a/qb_site/zulip_bot/commands/pr_info.py b/qb_site/zulip_bot/commands/pr_info.py index 558dcb90..092415d7 100644 --- a/qb_site/zulip_bot/commands/pr_info.py +++ b/qb_site/zulip_bot/commands/pr_info.py @@ -24,7 +24,6 @@ name="pr-info", description="Show queue info for one or more GitHub PR links (up to 10). Zulip linkifiers also work.", response_mode=ResponseMode.STREAM, - aliases=("pr_info",), ) def pr_info_command(context: CommandContext, args: str) -> CommandResult: refs = _parse_pr_refs(context, args) diff --git a/qb_site/zulip_bot/frontend/tests/close_pr_form.test.js b/qb_site/zulip_bot/frontend/tests/close_pr_form.test.js new file mode 100644 index 00000000..25140b75 --- /dev/null +++ b/qb_site/zulip_bot/frontend/tests/close_pr_form.test.js @@ -0,0 +1,94 @@ +import { describe, expect, it } from "vitest"; + +import { formatTimestamp, mountClosePrForm } from "../../static/zulip_bot/close_pr_form.js"; + +describe("formatTimestamp", () => { + it("returns the original string for invalid ISO input", () => { + expect(formatTimestamp("not-a-date")).toBe("not-a-date"); + }); + + it("includes '(X days ago)' for a past timestamp", () => { + const twoDaysAgo = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000).toISOString(); + const result = formatTimestamp(twoDaysAgo); + expect(result).toContain("2 days ago"); + }); + + it("omits the ago suffix for future timestamps", () => { + const future = new Date(Date.now() + 60_000).toISOString(); + const result = formatTimestamp(future); + expect(result).not.toContain("ago"); + }); +}); + +describe("mountClosePrForm", () => { + function buildDom(expUnix) { + document.body.innerHTML = ` +
+ +
+
+ + + +
+
+ + +
+ `; + } + + it("disables submit when link is already expired", () => { + buildDom(1); + const unmount = mountClosePrForm(document); + const submit = document.getElementById("close-pr-submit"); + const text = document.getElementById("close-pr-countdown-text"); + expect(submit.disabled).toBe(true); + expect(text.textContent).toContain("expired"); + unmount(); + }); + + it("enables submit when link is not yet expired", () => { + buildDom(Math.floor(Date.now() / 1000) + 3600); + const unmount = mountClosePrForm(document); + expect(document.getElementById("close-pr-submit").disabled).toBe(false); + unmount(); + }); + + it("loads preset text into textarea on preset button click", () => { + buildDom(Math.floor(Date.now() / 1000) + 3600); + const unmount = mountClosePrForm(document); + document.querySelector(".preset-btn").click(); + expect(document.getElementById("close_message").value).toBe("preset text"); + unmount(); + }); + + it("confirms before submitting with empty close message", () => { + buildDom(Math.floor(Date.now() / 1000) + 3600); + let confirmed = false; + window.confirm = () => { confirmed = true; return false; }; + const unmount = mountClosePrForm(document); + document.getElementById("close-pr-form").dispatchEvent(new Event("submit", { bubbles: true, cancelable: true })); + expect(confirmed).toBe(true); + unmount(); + }); + + it("does not confirm when close message is present", () => { + buildDom(Math.floor(Date.now() / 1000) + 3600); + let confirmed = false; + window.confirm = () => { confirmed = true; return true; }; + const unmount = mountClosePrForm(document); + document.getElementById("close_message").value = "some message"; + document.getElementById("close-pr-form").dispatchEvent(new Event("submit", { bubbles: true, cancelable: true })); + expect(confirmed).toBe(false); + unmount(); + }); + + it("formats .ts time elements on mount", () => { + buildDom(Math.floor(Date.now() / 1000) + 3600); + const unmount = mountClosePrForm(document); + const timeEl = document.querySelector("time.ts"); + expect(timeEl.textContent).toContain("ago"); + unmount(); + }); +}); diff --git a/qb_site/zulip_bot/services/close_pr_execution.py b/qb_site/zulip_bot/services/close_pr_execution.py new file mode 100644 index 00000000..5cddd40d --- /dev/null +++ b/qb_site/zulip_bot/services/close_pr_execution.py @@ -0,0 +1,315 @@ +from __future__ import annotations + +import logging +from dataclasses import dataclass +from enum import Enum +from typing import Any + +import requests +from django.conf import settings + +from core.services.github_operation_tokens import resolve_github_app_operation_token + +log = logging.getLogger(__name__) + +_GITHUB_API_BASE = "https://api.github.com" +_CLOSE_PR_OPERATION = "close_pr" +_CHECK_COLLABORATOR_PERMISSION_OPERATION = "check_collaborator_permission" + + +class PermissionOutcome(str, Enum): + PERMITTED = "permitted" + NOT_PERMITTED = "not_permitted" + PR_NOT_OPEN = "pr_not_open" + TOKEN_UNAVAILABLE = "token_unavailable" + GITHUB_ERROR = "github_error" + + +@dataclass(frozen=True) +class PermissionCheckResult: + outcome: PermissionOutcome + pr_title: str | None = None + message: str | None = None + + +@dataclass(frozen=True) +class LivePRDetails: + title: str + is_open: bool + author_login: str + body: str | None = None + opened_at: str | None = None + updated_at: str | None = None + labels: tuple[tuple[str, str], ...] = () + + +@dataclass(frozen=True) +class ClosePRError(RuntimeError): + code: str + message: str + + +def check_close_pr_permission( + *, + github_login: str, + owner: str, + repo: str, + number: int, +) -> PermissionCheckResult: + """Check whether github_login may close the given PR. + + Fetches PR details to verify it is open and extract the author login. + If the invoker is the PR author, permission is granted immediately. + Otherwise calls the collaborator permission endpoint to check for + write or admin access. + + Returns a PermissionCheckResult with one of the PermissionOutcome values. + """ + token = _get_token(owner=owner, repo=repo) + if not token: + return PermissionCheckResult( + outcome=PermissionOutcome.TOKEN_UNAVAILABLE, + message="GitHub App token for close_pr is not available for this repository.", + ) + + pr = _fetch_pr_details(token=token, owner=owner, repo=repo, number=number) + if pr is None: + return PermissionCheckResult( + outcome=PermissionOutcome.GITHUB_ERROR, + message=f"Could not fetch PR details for {owner}/{repo}#{number} from GitHub.", + ) + + if not pr.is_open: + return PermissionCheckResult( + outcome=PermissionOutcome.PR_NOT_OPEN, + pr_title=pr.title, + message=f"Pull request {owner}/{repo}#{number} is not open.", + ) + + if github_login.lower() == pr.author_login.lower(): + return PermissionCheckResult( + outcome=PermissionOutcome.PERMITTED, + pr_title=pr.title, + ) + + member_token = _get_member_check_token(owner=owner, repo=repo) or token + collab_permission = _fetch_collaborator_permission(token=member_token, owner=owner, repo=repo, github_login=github_login) + if collab_permission in {"write", "admin"}: + return PermissionCheckResult( + outcome=PermissionOutcome.PERMITTED, + pr_title=pr.title, + ) + + return PermissionCheckResult( + outcome=PermissionOutcome.NOT_PERMITTED, + pr_title=pr.title, + message=( + f"GitHub login `{github_login}` does not have permission to close " + f"{owner}/{repo}#{number} (not the PR author and not a write/admin collaborator)." + ), + ) + + +def post_pr_comment(*, owner: str, repo: str, number: int, body: str) -> None: + """Post a comment on a pull request before closing it. + + Uses the close_pr operation token (Issues: Read and write already granted). + Raises ClosePRError on failure. + """ + token = _get_token(owner=owner, repo=repo) + if not token: + raise ClosePRError( + code="token_unavailable", + message="GitHub App token for close_pr is not available for this repository.", + ) + + api_base = _api_base_url() + url = f"{api_base}/repos/{owner}/{repo}/issues/{number}/comments" + headers = { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + try: + response = requests.post(url, json={"body": body}, headers=headers, timeout=20) + except requests.RequestException as exc: + raise ClosePRError(code="request_failed", message=f"GitHub request failed: {exc}") from exc + + if response.status_code < 400: + return + + details = _safe_json(response) + if response.status_code in {401, 403}: + raise ClosePRError(code="permission_denied", message="GitHub permission denied when posting comment.") + if response.status_code == 404: + raise ClosePRError(code="pr_not_found", message="Pull request not found on GitHub.") + if response.status_code >= 500: + raise ClosePRError(code="github_transient", message="GitHub API temporarily failed.") + raise ClosePRError(code="github_error", message=f"GitHub API error when posting comment: {details}") + + +def fetch_pr_details_for_form(*, owner: str, repo: str, number: int) -> LivePRDetails | None: + """Fetch PR details for display on the confirmation form. + + Uses the close_pr operation token. Returns None if the token is unavailable + or the GitHub request fails. + """ + token = _get_token(owner=owner, repo=repo) + if not token: + return None + return _fetch_pr_details(token=token, owner=owner, repo=repo, number=number) + + +def close_pull_request(*, owner: str, repo: str, number: int) -> None: + """Close a pull request via the GitHub API. + + Raises ClosePRError on failure. + """ + token = _get_token(owner=owner, repo=repo) + if not token: + raise ClosePRError( + code="token_unavailable", + message="GitHub App token for close_pr is not available for this repository.", + ) + + api_base = _api_base_url() + url = f"{api_base}/repos/{owner}/{repo}/pulls/{number}" + headers = { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + try: + response = requests.patch(url, json={"state": "closed"}, headers=headers, timeout=20) + except requests.RequestException as exc: + raise ClosePRError(code="request_failed", message=f"GitHub request failed: {exc}") from exc + + if response.status_code < 400: + return + + details = _safe_json(response) + if response.status_code in {401, 403}: + raise ClosePRError(code="permission_denied", message="GitHub permission denied when closing pull request.") + if response.status_code == 404: + raise ClosePRError(code="pr_not_found", message="Pull request not found on GitHub.") + if response.status_code == 422: + raise ClosePRError(code="validation_failed", message=f"GitHub rejected the close request: {details}") + if response.status_code >= 500: + raise ClosePRError(code="github_transient", message="GitHub API temporarily failed.") + raise ClosePRError(code="github_error", message=f"GitHub API error when closing pull request: {details}") + + +def _get_token(*, owner: str, repo: str) -> str | None: + return resolve_github_app_operation_token( + operation=_CLOSE_PR_OPERATION, + owner=owner, + repo=repo, + ) + + +def _get_member_check_token(*, owner: str, repo: str) -> str | None: + return resolve_github_app_operation_token( + operation=_CHECK_COLLABORATOR_PERMISSION_OPERATION, + owner=owner, + repo=repo, + ) + + +def _fetch_pr_details(*, token: str, owner: str, repo: str, number: int) -> LivePRDetails | None: + api_base = _api_base_url() + url = f"{api_base}/repos/{owner}/{repo}/pulls/{number}" + headers = { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + try: + response = requests.get(url, headers=headers, timeout=20) + except requests.RequestException: + log.warning("close_pr_fetch_pr_details_failed", extra={"owner": owner, "repo": repo, "number": number}) + return None + + if response.status_code >= 400: + log.warning( + "close_pr_fetch_pr_details_http_error", + extra={"owner": owner, "repo": repo, "number": number, "status": response.status_code}, + ) + return None + + payload = _safe_json(response) + title = str(payload.get("title") or "").strip() or f"{owner}/{repo}#{number}" + state = str(payload.get("state") or "").strip().lower() + merged_at = payload.get("merged_at") + is_open = state == "open" and not merged_at + author_login = str((payload.get("user") or {}).get("login") or "").strip() + body = str(payload.get("body") or "").strip() or None + opened_at = str(payload.get("created_at") or "").strip() or None + updated_at = str(payload.get("updated_at") or "").strip() or None + raw_labels = payload.get("labels") if isinstance(payload.get("labels"), list) else [] + labels: tuple[tuple[str, str], ...] = tuple( + (str(lbl.get("name", "")), str(lbl.get("color", ""))) for lbl in raw_labels if isinstance(lbl, dict) and lbl.get("name") + ) + return LivePRDetails( + title=title, + is_open=is_open, + author_login=author_login, + body=body, + opened_at=opened_at, + updated_at=updated_at, + labels=labels, + ) + + +def _fetch_collaborator_permission(*, token: str, owner: str, repo: str, github_login: str) -> str: + """Return the permission level string for github_login on owner/repo. + + Returns "none" on any error or if the user is not a collaborator. + The GitHub API returns "none" for users with no access (not 404). + Possible return values: "admin", "write", "read", "none". + Note: the "maintain" role maps to "write" in this endpoint's permission field. + """ + api_base = _api_base_url() + url = f"{api_base}/repos/{owner}/{repo}/collaborators/{github_login}/permission" + headers = { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + try: + response = requests.get(url, headers=headers, timeout=20) + except requests.RequestException: + log.warning( + "close_pr_collaborator_permission_request_failed", + extra={"owner": owner, "repo": repo, "github_login": github_login}, + ) + return "none" + + if response.status_code == 404: + # User is not a collaborator on this repo. + return "none" + + if response.status_code >= 400: + log.warning( + "close_pr_collaborator_permission_http_error", + extra={"owner": owner, "repo": repo, "github_login": github_login, "status": response.status_code}, + ) + return "none" + + payload = _safe_json(response) + permission = str(payload.get("permission") or "").strip().lower() + return permission or "none" + + +def _api_base_url() -> str: + configured = getattr(settings, "GITHUB_API_URL", "").strip().rstrip("/") + return configured or _GITHUB_API_BASE + + +def _safe_json(response: requests.Response) -> dict[str, Any]: + try: + payload = response.json() + except ValueError: + return {"raw": response.text} + if isinstance(payload, dict): + return payload + return {"raw": payload} diff --git a/qb_site/zulip_bot/services/close_pr_links.py b/qb_site/zulip_bot/services/close_pr_links.py new file mode 100644 index 00000000..1abb8cb9 --- /dev/null +++ b/qb_site/zulip_bot/services/close_pr_links.py @@ -0,0 +1,133 @@ +from __future__ import annotations + +import base64 +import hashlib +import json +import time +from dataclasses import dataclass +from typing import Any +from urllib.parse import quote + +from cryptography.fernet import Fernet, InvalidToken +from django.conf import settings + + +@dataclass(frozen=True) +class ClosePRLinkClaims: + zulip_user_id: int + github_login: str + pr_owner: str + pr_repo: str + pr_number: int + iat: int | None = None + exp: int | None = None + + +class ClosePRTokenError(Exception): + pass + + +class ClosePRTokenExpired(ClosePRTokenError): + pass + + +class ClosePRTokenInvalid(ClosePRTokenError): + pass + + +def build_close_pr_link(*, claims: ClosePRLinkClaims) -> str: + token = issue_close_pr_token(claims=claims) + url_base = getattr(settings, "ZULIP_PREFS_URL_BASE", "").strip().rstrip("/") + path = f"/api/zulip/close-pr/{quote(token, safe='')}/" + if url_base: + return f"{url_base}{path}" + return path + + +def issue_close_pr_token(*, claims: ClosePRLinkClaims) -> str: + now_ts = int(time.time()) + payload = { + "zulip_user_id": claims.zulip_user_id, + "github_login": claims.github_login, + "pr_owner": claims.pr_owner, + "pr_repo": claims.pr_repo, + "pr_number": claims.pr_number, + "iat": now_ts, + "exp": now_ts + _token_ttl_seconds(), + } + encrypted = _fernet().encrypt(json.dumps(payload, separators=(",", ":"), sort_keys=True).encode("utf-8")) + return encrypted.decode("utf-8") + + +def validate_close_pr_token(token: str) -> ClosePRLinkClaims: + try: + decrypted = _fernet().decrypt(token.encode("utf-8")) + except (InvalidToken, ValueError, UnicodeEncodeError) as exc: + raise ClosePRTokenInvalid("invalid token") from exc + try: + payload = json.loads(decrypted.decode("utf-8")) + except (ValueError, UnicodeDecodeError) as exc: + raise ClosePRTokenInvalid("invalid payload") from exc + return _parse_claims(payload) + + +def _parse_claims(payload: Any) -> ClosePRLinkClaims: + if not isinstance(payload, dict): + raise ClosePRTokenInvalid("invalid payload") + + zulip_user_id = payload.get("zulip_user_id") + github_login = payload.get("github_login") + pr_owner = payload.get("pr_owner") + pr_repo = payload.get("pr_repo") + pr_number = payload.get("pr_number") + exp = payload.get("exp") + + if not isinstance(zulip_user_id, int) or zulip_user_id <= 0: + raise ClosePRTokenInvalid("invalid zulip_user_id") + if not isinstance(github_login, str) or not github_login.strip(): + raise ClosePRTokenInvalid("invalid github_login") + if not isinstance(pr_owner, str) or not pr_owner.strip(): + raise ClosePRTokenInvalid("invalid pr_owner") + if not isinstance(pr_repo, str) or not pr_repo.strip(): + raise ClosePRTokenInvalid("invalid pr_repo") + if not isinstance(pr_number, int) or pr_number <= 0: + raise ClosePRTokenInvalid("invalid pr_number") + if not isinstance(exp, int): + raise ClosePRTokenInvalid("invalid exp") + if int(time.time()) > exp: + raise ClosePRTokenExpired("token expired") + + iat = payload.get("iat") + if iat is not None and not isinstance(iat, int): + raise ClosePRTokenInvalid("invalid iat") + + return ClosePRLinkClaims( + zulip_user_id=zulip_user_id, + github_login=github_login, + pr_owner=pr_owner, + pr_repo=pr_repo, + pr_number=pr_number, + iat=iat, + exp=exp, + ) + + +def _token_secret() -> str: + custom = getattr(settings, "ZULIP_CLOSE_PR_TOKEN_SECRET", "").strip() + if custom: + return custom + return settings.SECRET_KEY + + +def _token_salt() -> str: + return getattr(settings, "ZULIP_CLOSE_PR_TOKEN_SALT", "zulip_bot.close_pr") + + +def _fernet() -> Fernet: + material = f"{_token_secret()}:{_token_salt()}".encode("utf-8") + key = base64.urlsafe_b64encode(hashlib.sha256(material).digest()) + return Fernet(key) + + +def _token_ttl_seconds() -> int: + return int(getattr(settings, "ZULIP_CLOSE_PR_TOKEN_TTL_SECONDS", 1800)) diff --git a/qb_site/zulip_bot/services/close_pr_presets.py b/qb_site/zulip_bot/services/close_pr_presets.py new file mode 100644 index 00000000..6e3b97c7 --- /dev/null +++ b/qb_site/zulip_bot/services/close_pr_presets.py @@ -0,0 +1,36 @@ +from __future__ import annotations + +import re +from pathlib import Path + +_PRESETS_DIR = Path(__file__).parent.parent / "close_pr_presets" + + +def load_close_pr_presets() -> list[dict[str, str]]: + """Return preset close messages loaded from markdown files in close_pr_presets/. + + Each .md file's first line may be a markdown heading (``# Title``) used as + the display name; otherwise the filename stem (digits-prefix stripped, + hyphens/underscores replaced with spaces, title-cased) is used as the name. + Files are sorted by filename so numbering them controls display order. + Returns a list of ``{"name": str, "body": str}`` dicts (files with empty + bodies after stripping are skipped). + """ + if not _PRESETS_DIR.is_dir(): + return [] + presets = [] + for path in sorted(_PRESETS_DIR.glob("*.md")): + text = path.read_text(encoding="utf-8").strip() + if not text: + continue + lines = text.splitlines() + if lines[0].startswith("# "): + name = lines[0][2:].strip() + body = "\n".join(lines[1:]).strip() + else: + stem = re.sub(r"^\d+-", "", path.stem) + name = stem.replace("-", " ").replace("_", " ").title() + body = text + if body: + presets.append({"name": name, "body": body}) + return presets diff --git a/qb_site/zulip_bot/static/zulip_bot/close_pr_form.css b/qb_site/zulip_bot/static/zulip_bot/close_pr_form.css new file mode 100644 index 00000000..d8fd3998 --- /dev/null +++ b/qb_site/zulip_bot/static/zulip_bot/close_pr_form.css @@ -0,0 +1,171 @@ +.meta-link { + color: #d8f3f2; + text-decoration: none; +} + +.meta-link:hover { + text-decoration: underline; +} + +.pr-card { + margin-bottom: 16px; +} + +.pr-title { + margin: 0 0 8px; + font-size: 1.1rem; + font-weight: 600; + color: var(--ink); + line-height: 1.35; +} + +.pr-title a { + color: inherit; + text-decoration: none; +} + +.pr-title a:hover { + text-decoration: underline; +} + +.pr-meta-row { + display: flex; + flex-wrap: wrap; + gap: 4px 12px; + font-size: 0.88rem; + color: var(--muted); + margin-bottom: 8px; +} + +.pr-meta-row a { + color: var(--accent); + text-decoration: none; +} + +.pr-meta-row a:hover { + text-decoration: underline; +} + +.pr-labels { + display: flex; + flex-wrap: wrap; + gap: 6px; + margin-bottom: 10px; +} + +.pr-label { + display: inline-block; + padding: 2px 8px; + border-radius: 12px; + font-size: 0.8rem; + font-weight: 500; + background: var(--border); + color: var(--ink); + border: 1px solid transparent; +} + +.pr-body { + white-space: pre-wrap; + overflow-y: auto; + max-height: 180px; + font-size: 0.88rem; + color: var(--muted); + margin: 0; + line-height: 1.5; + padding: 8px 10px; + background: var(--bg); + border-radius: 6px; + border: 1px solid var(--border); +} + +.countdown { + display: flex; + flex-wrap: wrap; + gap: 8px 14px; + align-items: center; + background: var(--card); + border: 1px solid var(--border); + border-radius: 10px; + padding: 11px 12px; + margin-bottom: 16px; + color: var(--muted); + font-size: 0.95rem; +} + +.countdown strong { + color: var(--ink); +} + +.attribution-notice { + margin: 0 0 14px; + font-size: 0.95rem; +} + +.card a:not(.cta) { + color: var(--accent); +} + +.message-field { + margin-top: 16px; +} + +.message-label { + display: block; + font-weight: 600; + margin-bottom: 6px; + font-size: 0.95rem; +} + +.optional { + font-weight: 400; + color: var(--muted); + font-size: 0.88rem; +} + +.preset-bar { + display: flex; + flex-wrap: wrap; + gap: 6px; + align-items: center; + margin-bottom: 8px; +} + +.preset-label { + font-size: 0.88rem; + color: var(--muted); + font-weight: 600; +} + +.preset-btn { + font-size: 0.82rem; + padding: 3px 8px; + border: 1px solid var(--border); + border-radius: 6px; + background: var(--bg); + color: var(--ink); + cursor: pointer; + font-family: inherit; +} + +.preset-btn:hover { + background: var(--border); +} + +.message-textarea { + width: 100%; + min-height: 100px; + border: 1px solid var(--border); + border-radius: 8px; + padding: 8px 10px; + font-size: 0.92rem; + font-family: inherit; + color: var(--ink); + background: var(--card); + resize: vertical; + margin-bottom: 12px; +} + +.message-textarea:focus { + outline: 2px solid var(--accent); + outline-offset: 1px; +} diff --git a/qb_site/zulip_bot/static/zulip_bot/close_pr_form.js b/qb_site/zulip_bot/static/zulip_bot/close_pr_form.js new file mode 100644 index 00000000..43e6eec2 --- /dev/null +++ b/qb_site/zulip_bot/static/zulip_bot/close_pr_form.js @@ -0,0 +1,97 @@ +import { formatRemaining, getExpiryState } from "./prefs_form.js"; + +function formatAgo(ms) { + const s = ms / 1000; + if (s < 60) return `${Math.floor(s)}s ago`; + const m = s / 60; + if (m < 60) return `${Math.floor(m)}m ago`; + const h = m / 60; + if (h < 24) return `${Math.floor(h)}h ago`; + const d = h / 24; + if (d < 30) return `${Math.floor(d)} day${Math.floor(d) === 1 ? "" : "s"} ago`; + const mo = d / 30.4; + if (mo < 12) return `${Math.floor(mo)} month${Math.floor(mo) === 1 ? "" : "s"} ago`; + const yr = mo / 12; + return `${Math.floor(yr)} year${Math.floor(yr) === 1 ? "" : "s"} ago`; +} + +export function formatTimestamp(isoString, nowMs = Date.now()) { + const d = new Date(isoString); + if (Number.isNaN(d.getTime())) return isoString; + const local = d.toLocaleString(undefined, { + year: "numeric", + month: "short", + day: "numeric", + hour: "2-digit", + minute: "2-digit", + }); + const elapsed = nowMs - d.getTime(); + return elapsed >= 0 ? `${local} (${formatAgo(elapsed)})` : local; +} + +function formatTimestamps(root) { + for (const el of root.querySelectorAll("time.ts[data-iso]")) { + const formatted = formatTimestamp(el.dataset.iso); + if (formatted) el.textContent = formatted; + } +} + +export function mountClosePrForm(root = document) { + formatTimestamps(root); + + const form = root.getElementById("close-pr-form"); + const submitButton = root.getElementById("close-pr-submit"); + const countdownText = root.getElementById("close-pr-countdown-text"); + const countdownLabel = root.getElementById("close-pr-countdown-label"); + if (!form || !submitButton || !countdownText || !countdownLabel) { + return () => undefined; + } + + const expUnix = Number(form.dataset.expUnix || "0"); + + const update = () => { + const state = getExpiryState(expUnix); + if (state.expired) { + countdownLabel.textContent = "Expired:"; + countdownText.textContent = "This link has expired. Request a fresh one in Zulip."; + submitButton.disabled = true; + } else { + countdownLabel.textContent = "Expires in:"; + countdownText.textContent = formatRemaining(state.remainingMs); + submitButton.disabled = false; + } + }; + update(); + const intervalId = window.setInterval(update, 1000); + + for (const btn of root.querySelectorAll(".preset-btn")) { + btn.addEventListener("click", () => { + const textarea = root.getElementById("close_message"); + if (textarea) { + textarea.value = btn.getAttribute("data-body") || ""; + } + }); + } + + form.addEventListener("submit", (event) => { + update(); + if (submitButton.disabled) { + event.preventDefault(); + return; + } + const textarea = root.getElementById("close_message"); + if (textarea && !textarea.value.trim()) { + if (!window.confirm("No close message will be posted as a comment. Close this pull request without a comment?")) { + event.preventDefault(); + } + } + }); + + return () => window.clearInterval(intervalId); +} + +if (typeof window !== "undefined") { + window.addEventListener("DOMContentLoaded", () => { + mountClosePrForm(document); + }); +} diff --git a/qb_site/zulip_bot/tests/test_close_pr_command.py b/qb_site/zulip_bot/tests/test_close_pr_command.py new file mode 100644 index 00000000..c78410e5 --- /dev/null +++ b/qb_site/zulip_bot/tests/test_close_pr_command.py @@ -0,0 +1,97 @@ +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +from django.test import TestCase + +from core.models import User +from zulip_bot.commands import CommandContext +from zulip_bot.commands.close_pr import close_pr_command +from zulip_bot.services.close_pr_execution import PermissionCheckResult, PermissionOutcome + + +def _context(sender_id: int | None = 101) -> CommandContext: + return CommandContext( + sender_id=sender_id, + sender_email="reviewer@example.com", + sender_full_name="Reviewer User", + message_content="close-pr", + message_id=555, + stream_id=None, + topic=None, + is_private=True, + rendered_content=None, + allowed_command_names=frozenset({"close-pr"}), + ) + + +_PR_URL = "https://github.com/leanprover-community/mathlib4/pull/999" + + +class TestClosePRCommand(TestCase): + def setUp(self) -> None: + super().setUp() + self.user = User.objects.create(zulip_user_id=101, github_login="reviewer") + + def _patch_permission(self, result: PermissionCheckResult): + return patch( + "zulip_bot.commands.close_pr.check_close_pr_permission", + return_value=result, + ) + + def test_missing_sender_id(self) -> None: + result = close_pr_command(_context(sender_id=None), _PR_URL) + self.assertIn("Could not determine your Zulip identity", result.content) + + def test_no_pr_url(self) -> None: + result = close_pr_command(_context(), "") + self.assertIn("Could not parse `close-pr` command", result.content) + + def test_no_linked_user(self) -> None: + self.user.delete() + result = close_pr_command(_context(), _PR_URL) + self.assertIn("No GitHub account is linked", result.content) + + def test_no_github_login(self) -> None: + self.user.github_login = "" + self.user.save() + result = close_pr_command(_context(), _PR_URL) + self.assertIn("does not have a GitHub login set", result.content) + + def test_token_unavailable(self) -> None: + with self._patch_permission(PermissionCheckResult(outcome=PermissionOutcome.TOKEN_UNAVAILABLE)): + result = close_pr_command(_context(), _PR_URL) + self.assertIn("token for `close_pr` is not available", result.content) + + def test_github_error(self) -> None: + with self._patch_permission(PermissionCheckResult(outcome=PermissionOutcome.GITHUB_ERROR)): + result = close_pr_command(_context(), _PR_URL) + self.assertIn("Could not fetch PR details", result.content) + + def test_pr_not_open(self) -> None: + with self._patch_permission(PermissionCheckResult(outcome=PermissionOutcome.PR_NOT_OPEN, pr_title="Fix something")): + result = close_pr_command(_context(), _PR_URL) + self.assertIn("is not open", result.content) + self.assertIn("Fix something", result.content) + + def test_not_permitted(self) -> None: + with self._patch_permission(PermissionCheckResult(outcome=PermissionOutcome.NOT_PERMITTED)): + result = close_pr_command(_context(), _PR_URL) + self.assertIn("does not have permission to close", result.content) + + @patch("zulip_bot.commands.close_pr.ZulipClient") + def test_permitted_issues_link(self, MockZulipClient: MagicMock) -> None: + mock_client = MockZulipClient.return_value + with self._patch_permission(PermissionCheckResult(outcome=PermissionOutcome.PERMITTED, pr_title="Great PR")): + result = close_pr_command(_context(), _PR_URL) + self.assertIn("confirm closing PR", result.content) + self.assertIn("Great PR", result.content) + self.assertIn("attributed to the bot", result.content) + self.assertIn("/api/zulip/close-pr/", result.content) + mock_client.add_reaction.assert_called_once_with(message_id=555, emoji_name="eyes") + + @patch("zulip_bot.commands.close_pr.ZulipClient") + def test_permitted_link_includes_expiry(self, MockZulipClient: MagicMock) -> None: + with self._patch_permission(PermissionCheckResult(outcome=PermissionOutcome.PERMITTED)): + result = close_pr_command(_context(), _PR_URL) + self.assertIn(" MagicMock: + mock = MagicMock() + mock.status_code = status_code + if json_data is not None: + mock.json.return_value = json_data + else: + mock.json.side_effect = ValueError("no body") + mock.text = "" + return mock + + +class TestCheckClosePRPermission(SimpleTestCase): + def _patch_token(self, token: str | None = "test-token"): + return patch( + "zulip_bot.services.close_pr_execution.resolve_github_app_operation_token", + return_value=token, + ) + + def _patch_get(self, responses: list): + """Patch requests.get to return successive responses.""" + return patch("zulip_bot.services.close_pr_execution.requests.get", side_effect=responses) + + def test_token_unavailable(self) -> None: + with self._patch_token(None): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.TOKEN_UNAVAILABLE) + + def test_github_error_on_pr_fetch(self) -> None: + pr_response = _make_response(status_code=500) + with self._patch_token(), self._patch_get([pr_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.GITHUB_ERROR) + + def test_pr_not_open(self) -> None: + pr_response = _make_response( + status_code=200, + json_data={"title": "Fix bug", "state": "closed", "merged_at": None, "user": {"login": "author"}}, + ) + with self._patch_token(), self._patch_get([pr_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.PR_NOT_OPEN) + self.assertEqual(result.pr_title, "Fix bug") + + def test_permitted_as_pr_author(self) -> None: + pr_response = _make_response( + status_code=200, + json_data={"title": "My PR", "state": "open", "merged_at": None, "user": {"login": "reviewer"}}, + ) + with self._patch_token(), self._patch_get([pr_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.PERMITTED) + self.assertEqual(result.pr_title, "My PR") + + def test_permitted_as_pr_author_case_insensitive(self) -> None: + pr_response = _make_response( + status_code=200, + json_data={"title": "My PR", "state": "open", "merged_at": None, "user": {"login": "Reviewer"}}, + ) + with self._patch_token(), self._patch_get([pr_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.PERMITTED) + + def test_permitted_as_write_collaborator(self) -> None: + pr_response = _make_response( + status_code=200, + json_data={"title": "Someone's PR", "state": "open", "merged_at": None, "user": {"login": "author"}}, + ) + collab_response = _make_response(status_code=200, json_data={"permission": "write"}) + with self._patch_token(), self._patch_get([pr_response, collab_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.PERMITTED) + + def test_permitted_as_admin_collaborator(self) -> None: + pr_response = _make_response( + status_code=200, + json_data={"title": "Someone's PR", "state": "open", "merged_at": None, "user": {"login": "author"}}, + ) + collab_response = _make_response(status_code=200, json_data={"permission": "admin"}) + with self._patch_token(), self._patch_get([pr_response, collab_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.PERMITTED) + + def test_not_permitted_read_only(self) -> None: + pr_response = _make_response( + status_code=200, + json_data={"title": "Someone's PR", "state": "open", "merged_at": None, "user": {"login": "author"}}, + ) + collab_response = _make_response(status_code=200, json_data={"permission": "read"}) + with self._patch_token(), self._patch_get([pr_response, collab_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.NOT_PERMITTED) + + def test_not_permitted_no_access(self) -> None: + pr_response = _make_response( + status_code=200, + json_data={"title": "Someone's PR", "state": "open", "merged_at": None, "user": {"login": "author"}}, + ) + collab_response = _make_response(status_code=200, json_data={"permission": "none"}) + with self._patch_token(), self._patch_get([pr_response, collab_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.NOT_PERMITTED) + + def test_not_permitted_when_collaborator_endpoint_returns_404(self) -> None: + pr_response = _make_response( + status_code=200, + json_data={"title": "Someone's PR", "state": "open", "merged_at": None, "user": {"login": "author"}}, + ) + collab_response = _make_response(status_code=404) + with self._patch_token(), self._patch_get([pr_response, collab_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.NOT_PERMITTED) + + def test_merged_pr_treated_as_not_open(self) -> None: + pr_response = _make_response( + status_code=200, + json_data={ + "title": "Merged PR", + "state": "closed", + "merged_at": "2024-01-01T00:00:00Z", + "user": {"login": "author"}, + }, + ) + with self._patch_token(), self._patch_get([pr_response]): + result = check_close_pr_permission( + github_login="reviewer", + owner="leanprover-community", + repo="mathlib4", + number=1, + ) + self.assertEqual(result.outcome, PermissionOutcome.PR_NOT_OPEN) + + +class TestClosePullRequest(SimpleTestCase): + def _patch_token(self, token: str | None = "test-token"): + return patch( + "zulip_bot.services.close_pr_execution.resolve_github_app_operation_token", + return_value=token, + ) + + def _patch_patch(self, response): + return patch("zulip_bot.services.close_pr_execution.requests.patch", return_value=response) + + def test_success(self) -> None: + response = _make_response(status_code=200, json_data={"state": "closed"}) + with self._patch_token(), self._patch_patch(response): + close_pull_request(owner="leanprover-community", repo="mathlib4", number=1) + # No exception raised. + + def test_token_unavailable(self) -> None: + with self._patch_token(None): + with self.assertRaises(ClosePRError) as cm: + close_pull_request(owner="leanprover-community", repo="mathlib4", number=1) + self.assertEqual(cm.exception.code, "token_unavailable") + + def test_permission_denied(self) -> None: + response = _make_response(status_code=403) + with self._patch_token(), self._patch_patch(response): + with self.assertRaises(ClosePRError) as cm: + close_pull_request(owner="leanprover-community", repo="mathlib4", number=1) + self.assertEqual(cm.exception.code, "permission_denied") + + def test_pr_not_found(self) -> None: + response = _make_response(status_code=404) + with self._patch_token(), self._patch_patch(response): + with self.assertRaises(ClosePRError) as cm: + close_pull_request(owner="leanprover-community", repo="mathlib4", number=1) + self.assertEqual(cm.exception.code, "pr_not_found") + + def test_github_transient_error(self) -> None: + response = _make_response(status_code=503) + with self._patch_token(), self._patch_patch(response): + with self.assertRaises(ClosePRError) as cm: + close_pull_request(owner="leanprover-community", repo="mathlib4", number=1) + self.assertEqual(cm.exception.code, "github_transient") diff --git a/qb_site/zulip_bot/tests/test_close_pr_form.py b/qb_site/zulip_bot/tests/test_close_pr_form.py new file mode 100644 index 00000000..6fdcacb0 --- /dev/null +++ b/qb_site/zulip_bot/tests/test_close_pr_form.py @@ -0,0 +1,325 @@ +from __future__ import annotations + +from unittest.mock import patch + +from django.test import TestCase, override_settings +from django.urls import reverse + +from zulip_bot.services.close_pr_execution import ClosePRError, LivePRDetails +from zulip_bot.services.close_pr_links import ClosePRLinkClaims, issue_close_pr_token +from zulip_bot.services.close_pr_presets import load_close_pr_presets + + +def _token( + *, + zulip_user_id: int = 101, + github_login: str = "reviewer", + pr_owner: str = "leanprover-community", + pr_repo: str = "mathlib4", + pr_number: int = 999, +) -> str: + return issue_close_pr_token( + claims=ClosePRLinkClaims( + zulip_user_id=zulip_user_id, + github_login=github_login, + pr_owner=pr_owner, + pr_repo=pr_repo, + pr_number=pr_number, + ) + ) + + +def _url(token: str) -> str: + return reverse("zulip-close-pr-form", kwargs={"token": token}) + + +def _open_pr(title: str = "My PR") -> LivePRDetails: + return LivePRDetails(title=title, is_open=True, author_login="author") + + +def _closed_pr(title: str = "My PR") -> LivePRDetails: + return LivePRDetails(title=title, is_open=False, author_login="author") + + +def _patch_pr_details(pr: LivePRDetails | None): + return patch("zulip_bot.views.fetch_pr_details_for_form", return_value=pr) + + +def _patch_close(*, raises: ClosePRError | None = None): + if raises: + return patch("zulip_bot.views.close_pull_request", side_effect=raises) + return patch("zulip_bot.views.close_pull_request") + + +def _patch_post_comment(*, raises: ClosePRError | None = None): + if raises: + return patch("zulip_bot.views.post_pr_comment", side_effect=raises) + return patch("zulip_bot.views.post_pr_comment") + + +def _patch_post_actions(): + return patch("zulip_bot.views._enqueue_close_pr_post_actions") + + +def _patch_presets(presets=None): + return patch("zulip_bot.views.load_close_pr_presets", return_value=presets or []) + + +class TestClosePRFormGet(TestCase): + def test_expired_token_returns_403(self) -> None: + with patch("zulip_bot.services.close_pr_links.time.time", return_value=1_700_000_000): + tok = _token() + with patch("zulip_bot.services.close_pr_links.time.time", return_value=1_700_000_000 + 1_900): + with _patch_pr_details(_open_pr()): + response = self.client.get(_url(tok)) + self.assertEqual(response.status_code, 403) + self.assertTemplateUsed(response, "zulip_bot/close_pr_invalid.html") + self.assertIn("expired", response.context["reason"]) + + def test_invalid_token_returns_403(self) -> None: + response = self.client.get(_url("not-a-valid-token")) + self.assertEqual(response.status_code, 403) + self.assertTemplateUsed(response, "zulip_bot/close_pr_invalid.html") + self.assertIn("invalid", response.context["reason"]) + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_valid_open_pr_shows_confirmation_form(self) -> None: + with _patch_pr_details(_open_pr("Fix the thing")): + response = self.client.get(_url(_token())) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, "zulip_bot/close_pr_form.html") + self.assertContains(response, "Fix the thing") + self.assertContains(response, "Close this pull request") + self.assertFalse(response.context["success"]) + self.assertTrue(response.context["pr_is_open"]) + + def test_already_closed_pr_shows_informational_message_no_button(self) -> None: + with _patch_pr_details(_closed_pr()): + response = self.client.get(_url(_token())) + self.assertEqual(response.status_code, 200) + self.assertFalse(response.context["pr_is_open"]) + self.assertNotContains(response, "Close this pull request") + + def test_github_fetch_failure_assumes_open(self) -> None: + # If we can't fetch PR details, we still show the form (POST will validate). + with _patch_pr_details(None): + response = self.client.get(_url(_token())) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.context["pr_is_open"]) + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_mutations_enabled_flag_in_context(self) -> None: + with _patch_pr_details(_open_pr()): + response = self.client.get(_url(_token())) + self.assertFalse(response.context["mutations_disabled"]) + + def test_mutations_disabled_flag_in_context(self) -> None: + with _patch_pr_details(_open_pr()): + response = self.client.get(_url(_token())) + self.assertTrue(response.context["mutations_disabled"]) + + +class TestClosePRFormPost(TestCase): + def test_mutations_disabled_skips_close_but_runs_post_actions(self) -> None: + tok = _token() + with _patch_pr_details(_open_pr()), _patch_close() as mock_close, _patch_post_actions() as mock_post: + response = self.client.post(_url(tok)) + mock_close.assert_not_called() + mock_post.assert_called_once() + # Successful POST redirects (PRG); follow redirect to check success state. + self.assertEqual(response.status_code, 302) + self.assertIn("preflight=1", response["Location"]) + with _patch_pr_details(_open_pr()): + get_response = self.client.get(response["Location"]) + self.assertTrue(get_response.context["success"]) + self.assertTrue(get_response.context["preflight_only"]) + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_successful_close(self) -> None: + tok = _token() + with _patch_pr_details(_open_pr()), _patch_close(), _patch_post_actions() as mock_post: + response = self.client.post(_url(tok)) + mock_post.assert_called_once() + # Successful POST redirects (PRG). + self.assertEqual(response.status_code, 302) + self.assertIn("closed=1", response["Location"]) + with _patch_pr_details(_closed_pr()): + get_response = self.client.get(response["Location"]) + self.assertTrue(get_response.context["success"]) + self.assertFalse(get_response.context["preflight_only"]) + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_github_close_error_shown_in_template(self) -> None: + error = ClosePRError(code="permission_denied", message="GitHub permission denied when closing pull request.") + with _patch_pr_details(_open_pr()), _patch_close(raises=error), _patch_post_actions(): + response = self.client.post(_url(_token())) + self.assertEqual(response.status_code, 200) + self.assertFalse(response.context["success"]) + self.assertIn("permission denied", response.context["close_error"] or "") + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_post_actions_called_with_claims_and_title(self) -> None: + tok = _token(pr_number=42) + with _patch_pr_details(_open_pr("The Title")), _patch_close(), _patch_post_actions() as mock_post: + self.client.post(_url(tok)) + mock_post.assert_called_once() + kwargs = mock_post.call_args.kwargs + self.assertEqual(kwargs["claims"].pr_number, 42) + self.assertEqual(kwargs["pr_title"], "The Title") + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_refresh_repost_on_closed_pr_skips_mutations(self) -> None: + tok = _token() + with ( + _patch_pr_details(_closed_pr()), + _patch_close() as mock_close, + _patch_post_comment() as mock_comment, + _patch_post_actions() as mock_post, + ): + response = self.client.post(_url(tok)) + mock_comment.assert_not_called() + mock_close.assert_not_called() + mock_post.assert_not_called() + self.assertEqual(response.status_code, 200) + self.assertFalse(response.context["pr_is_open"]) + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_expired_token_on_post_returns_403(self) -> None: + with patch("zulip_bot.services.close_pr_links.time.time", return_value=1_700_000_000): + tok = _token() + with patch("zulip_bot.services.close_pr_links.time.time", return_value=1_700_000_000 + 1_900): + response = self.client.post(_url(tok)) + self.assertEqual(response.status_code, 403) + + +class TestClosePRFormPostWithMessage(TestCase): + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_close_message_posts_comment_before_close(self) -> None: + with ( + _patch_pr_details(_open_pr()), + _patch_post_comment() as mock_comment, + _patch_close() as mock_close, + _patch_post_actions(), + ): + response = self.client.post(_url(_token()), data={"close_message": "Superseded by #1000."}) + self.assertEqual(response.status_code, 302) + mock_comment.assert_called_once() + mock_close.assert_called_once() + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_comment_failure_shows_error_and_skips_close(self) -> None: + error = ClosePRError(code="permission_denied", message="GitHub permission denied when posting comment.") + with ( + _patch_pr_details(_open_pr()), + _patch_post_comment(raises=error), + _patch_close() as mock_close, + _patch_post_actions(), + ): + response = self.client.post(_url(_token()), data={"close_message": "Some message."}) + self.assertEqual(response.status_code, 200) + self.assertFalse(response.context["success"]) + self.assertIn("permission denied", response.context["close_error"] or "") + mock_close.assert_not_called() + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_empty_message_skips_comment(self) -> None: + with ( + _patch_pr_details(_open_pr()), + _patch_post_comment() as mock_comment, + _patch_close(), + _patch_post_actions(), + ): + response = self.client.post(_url(_token()), data={"close_message": ""}) + self.assertEqual(response.status_code, 302) + mock_comment.assert_not_called() + + def test_mutations_disabled_skips_comment_even_with_message(self) -> None: + with ( + _patch_pr_details(_open_pr()), + _patch_post_comment() as mock_comment, + _patch_close() as mock_close, + _patch_post_actions(), + ): + response = self.client.post(_url(_token()), data={"close_message": "Some message."}) + self.assertEqual(response.status_code, 302) + self.assertIn("preflight=1", response["Location"]) + mock_comment.assert_not_called() + mock_close.assert_not_called() + + @override_settings(ZULIP_CLOSE_PR_MUTATIONS_ENABLED="true") + def test_close_message_preserved_in_context_on_error(self) -> None: + error = ClosePRError(code="github_transient", message="GitHub API temporarily failed.") + with ( + _patch_pr_details(_open_pr()), + _patch_post_comment(), + _patch_close(raises=error), + _patch_post_actions(), + ): + response = self.client.post(_url(_token()), data={"close_message": "My message."}) + self.assertFalse(response.context["success"]) + self.assertEqual(response.context["close_message"], "My message.") + + +class TestClosePRPresets(TestCase): + def test_load_presets_returns_list(self) -> None: + presets = load_close_pr_presets() + self.assertIsInstance(presets, list) + + def test_presets_have_name_and_body(self) -> None: + presets = load_close_pr_presets() + for preset in presets: + self.assertIn("name", preset) + self.assertIn("body", preset) + self.assertTrue(preset["name"]) + self.assertTrue(preset["body"]) + + def test_presets_passed_to_template_context(self) -> None: + sample = [{"name": "Superseded", "body": "This PR is superseded."}] + with _patch_presets(sample), _patch_pr_details(_open_pr()): + response = self.client.get(_url(_token())) + self.assertEqual(response.context["presets"], sample) + + def test_preset_buttons_rendered_in_template(self) -> None: + sample = [{"name": "Stale", "body": "Closing as stale."}] + with _patch_presets(sample), _patch_pr_details(_open_pr()): + response = self.client.get(_url(_token())) + self.assertContains(response, "Stale") + self.assertContains(response, "Closing as stale.") + + +class TestRepoLogResolution(TestCase): + def test_missing_setting_skips_log(self) -> None: + from zulip_bot.views import _resolve_repo_log_target + + result = _resolve_repo_log_target(owner="leanprover-community", repo="mathlib4") + self.assertIsNone(result) + + @override_settings(ZULIP_REPO_LOG={"leanprover-community/mathlib4": {"stream": "mathlib4", "topic": "bot log"}}) + def test_configured_repo_returns_target(self) -> None: + from zulip_bot.views import _resolve_repo_log_target + + result = _resolve_repo_log_target(owner="leanprover-community", repo="mathlib4") + self.assertIsNotNone(result) + self.assertEqual(result["stream"], "mathlib4") + self.assertEqual(result["topic"], "bot log") + + @override_settings(ZULIP_REPO_LOG={"other/repo": {"stream": "s", "topic": "t"}}) + def test_unconfigured_repo_returns_none(self) -> None: + from zulip_bot.views import _resolve_repo_log_target + + result = _resolve_repo_log_target(owner="leanprover-community", repo="mathlib4") + self.assertIsNone(result) + + @override_settings(ZULIP_REPO_LOG={"leanprover-community/mathlib4": {"stream": "", "topic": "bot log"}}) + def test_missing_stream_returns_none(self) -> None: + from zulip_bot.views import _resolve_repo_log_target + + result = _resolve_repo_log_target(owner="leanprover-community", repo="mathlib4") + self.assertIsNone(result) + + @override_settings(ZULIP_REPO_LOG="not-a-dict") + def test_malformed_setting_returns_none(self) -> None: + from zulip_bot.views import _resolve_repo_log_target + + result = _resolve_repo_log_target(owner="leanprover-community", repo="mathlib4") + self.assertIsNone(result) diff --git a/qb_site/zulip_bot/tests/test_close_pr_links.py b/qb_site/zulip_bot/tests/test_close_pr_links.py new file mode 100644 index 00000000..21f2c9d8 --- /dev/null +++ b/qb_site/zulip_bot/tests/test_close_pr_links.py @@ -0,0 +1,66 @@ +from __future__ import annotations + +from unittest.mock import patch + +from django.test import SimpleTestCase, override_settings + +from zulip_bot.services.close_pr_links import ( + ClosePRLinkClaims, + ClosePRTokenExpired, + ClosePRTokenInvalid, + build_close_pr_link, + issue_close_pr_token, + validate_close_pr_token, +) + + +class TestClosePRLinks(SimpleTestCase): + def _claims(self, **kwargs) -> ClosePRLinkClaims: + defaults = dict( + zulip_user_id=101, + github_login="reviewer", + pr_owner="leanprover-community", + pr_repo="mathlib4", + pr_number=999, + ) + defaults.update(kwargs) + return ClosePRLinkClaims(**defaults) + + def test_round_trip(self) -> None: + token = issue_close_pr_token(claims=self._claims()) + claims = validate_close_pr_token(token) + self.assertEqual(claims.zulip_user_id, 101) + self.assertEqual(claims.github_login, "reviewer") + self.assertEqual(claims.pr_owner, "leanprover-community") + self.assertEqual(claims.pr_repo, "mathlib4") + self.assertEqual(claims.pr_number, 999) + self.assertIsNotNone(claims.iat) + self.assertIsNotNone(claims.exp) + + @override_settings(ZULIP_PREFS_URL_BASE="https://queueboard.example") + def test_build_link_uses_url_base(self) -> None: + link = build_close_pr_link(claims=self._claims()) + self.assertTrue(link.startswith("https://queueboard.example/api/zulip/close-pr/")) + + def test_build_link_falls_back_to_relative_path(self) -> None: + link = build_close_pr_link(claims=self._claims()) + self.assertTrue(link.startswith("/api/zulip/close-pr/")) + + def test_rejects_invalid_token(self) -> None: + with self.assertRaises(ClosePRTokenInvalid): + validate_close_pr_token("not-a-valid-token") + + def test_rejects_expired_token(self) -> None: + with patch("zulip_bot.services.close_pr_links.time.time", return_value=1_700_000_000): + token = issue_close_pr_token(claims=self._claims()) + with patch("zulip_bot.services.close_pr_links.time.time", return_value=1_700_000_000 + 1_900): + with self.assertRaises(ClosePRTokenExpired): + validate_close_pr_token(token) + + def test_rejects_tampered_token(self) -> None: + token = issue_close_pr_token(claims=self._claims()) + # Flip a character in the middle. + mid = len(token) // 2 + flipped = token[:mid] + ("A" if token[mid] != "A" else "B") + token[mid + 1 :] + with self.assertRaises(ClosePRTokenInvalid): + validate_close_pr_token(flipped) diff --git a/qb_site/zulip_bot/tests/test_payload_parser.py b/qb_site/zulip_bot/tests/test_payload_parser.py index ef854d8e..82640237 100644 --- a/qb_site/zulip_bot/tests/test_payload_parser.py +++ b/qb_site/zulip_bot/tests/test_payload_parser.py @@ -25,6 +25,19 @@ def test_parse_command_with_slash_prefix(self) -> None: self.assertEqual(parsed.name, "help") self.assertEqual(parsed.args, "") + def test_parse_command_normalizes_underscores_to_hyphens(self) -> None: + parsed = parse_command("close_pr https://github.com/org/repo/pull/1") + self.assertIsNotNone(parsed) + assert parsed is not None + self.assertEqual(parsed.name, "close-pr") + + def test_parse_command_normalizes_underscores_preserves_args(self) -> None: + parsed = parse_command("assigned_prs") + self.assertIsNotNone(parsed) + assert parsed is not None + self.assertEqual(parsed.name, "assigned-prs") + self.assertEqual(parsed.args, "") + def test_parse_command_returns_none_when_only_mention(self) -> None: self.assertIsNone(parse_command("@**queueboard-bot**")) diff --git a/qb_site/zulip_bot/urls.py b/qb_site/zulip_bot/urls.py index 84090269..3388ed43 100644 --- a/qb_site/zulip_bot/urls.py +++ b/qb_site/zulip_bot/urls.py @@ -10,4 +10,5 @@ path("register//github/", views.register_github_start, name="zulip-register-github-start"), path("register/github/callback/", views.register_github_callback, name="zulip-register-github-callback"), path("prefs//", views.prefs_form, name="zulip-prefs-form"), + path("close-pr//", views.close_pr_form, name="zulip-close-pr-form"), ] diff --git a/qb_site/zulip_bot/views.py b/qb_site/zulip_bot/views.py index 1786935b..9e409caf 100644 --- a/qb_site/zulip_bot/views.py +++ b/qb_site/zulip_bot/views.py @@ -21,6 +21,7 @@ from zulip_bot.commands import CommandResult, ResponseMode, get_command from zulip_bot.commands import assign as _assign # noqa: F401 from zulip_bot.commands import assigned_prs as _assigned_prs # noqa: F401 +from zulip_bot.commands import close_pr as _close_pr # noqa: F401 from zulip_bot.commands import echo as _echo # noqa: F401 from zulip_bot.commands import help as _help # noqa: F401 from zulip_bot.commands import prefs as _prefs # noqa: F401 @@ -30,6 +31,13 @@ from zulip_bot.forms import ReviewerPreferenceForm from zulip_bot.services.registration_bootstrap import ensure_default_preferences_for_user from core.services.github_oauth import GitHubOAuthClient, GitHubOAuthError +from zulip_bot.services.close_pr_execution import ClosePRError, fetch_pr_details_for_form, close_pull_request, post_pr_comment +from zulip_bot.services.close_pr_presets import load_close_pr_presets +from zulip_bot.services.close_pr_links import ( + ClosePRTokenExpired, + ClosePRTokenInvalid, + validate_close_pr_token, +) from zulip_bot.services.prefs_links import ( PrefsLinkClaims, PrefsTokenExpired, @@ -350,6 +358,234 @@ def register_github_callback(request: HttpRequest) -> HttpResponse: return response +def close_pr_form(request: HttpRequest, token: str) -> HttpResponse: + try: + claims = validate_close_pr_token(token) + except ClosePRTokenExpired: + return _close_pr_invalid_response(request, reason="expired") + except ClosePRTokenInvalid: + return _close_pr_invalid_response(request, reason="invalid") + + from datetime import datetime, timezone as dt_timezone + + expires_at_utc = datetime.fromtimestamp(claims.exp, tz=dt_timezone.utc) + + # Fetch live PR details for display. + pr_details = fetch_pr_details_for_form( + owner=claims.pr_owner, + repo=claims.pr_repo, + number=claims.pr_number, + ) + pr_title = pr_details.title if pr_details else None + pr_is_open = pr_details.is_open if pr_details else True # assume open if fetch fails; POST will verify + + pr_url = f"https://github.com/{claims.pr_owner}/{claims.pr_repo}/pull/{claims.pr_number}" + mutations_enabled = _close_pr_mutations_enabled() + context: dict = { + "pr_owner": claims.pr_owner, + "pr_repo": claims.pr_repo, + "pr_number": claims.pr_number, + "pr_url": pr_url, + "pr_title": pr_title, + "pr_is_open": pr_is_open, + "pr_author_login": pr_details.author_login if pr_details else None, + "pr_body": pr_details.body if pr_details else None, + "pr_opened_at": _fmt_date(pr_details.opened_at) if pr_details else None, + "pr_opened_at_iso": pr_details.opened_at if pr_details else None, + "pr_updated_at": _fmt_date(pr_details.updated_at) if pr_details else None, + "pr_updated_at_iso": pr_details.updated_at if pr_details else None, + "pr_labels": _label_badge_context(pr_details.labels) if pr_details else [], + "expires_at_iso": expires_at_utc.isoformat(), + "expires_at_unix": claims.exp, + "mutations_disabled": not mutations_enabled, + "success": False, + "preflight_only": False, + "close_error": None, + "close_message": "", + "presets": load_close_pr_presets(), + } + + # PRG: success redirect sets these params so the GET can show the right state. + if request.GET.get("closed") == "1": + context["success"] = True + elif request.GET.get("preflight") == "1": + context["success"] = True + context["preflight_only"] = True + + if request.method == "POST": + close_message = request.POST.get("close_message", "").strip() + context["close_message"] = close_message + + # Guard against browser-refresh re-POST: if the PR is already closed, do nothing. + if not pr_is_open: + response = TemplateResponse(request, "zulip_bot/close_pr_form.html", context, status=200) + response["Cache-Control"] = "no-store" + return response + + if mutations_enabled: + if close_message: + try: + post_pr_comment( + owner=claims.pr_owner, + repo=claims.pr_repo, + number=claims.pr_number, + body=close_message, + ) + except ClosePRError as exc: + context["close_error"] = f"Could not post comment: {exc.message}" + response = TemplateResponse(request, "zulip_bot/close_pr_form.html", context, status=200) + response["Cache-Control"] = "no-store" + return response + try: + close_pull_request(owner=claims.pr_owner, repo=claims.pr_repo, number=claims.pr_number) + except ClosePRError as exc: + context["close_error"] = exc.message + response = TemplateResponse(request, "zulip_bot/close_pr_form.html", context, status=200) + response["Cache-Control"] = "no-store" + return response + + _enqueue_close_pr_post_actions(claims=claims, pr_title=pr_title, close_message=close_message) + + # PRG: redirect to GET so a browser refresh replays the safe GET, not this POST. + param = "preflight=1" if not mutations_enabled else "closed=1" + response = HttpResponseRedirect(f"{request.path}?{param}") + response["Cache-Control"] = "no-store" + return response + + response = TemplateResponse(request, "zulip_bot/close_pr_form.html", context, status=200) + response["Cache-Control"] = "no-store" + return response + + +def _fmt_date(iso: str | None) -> str | None: + if not iso: + return None + try: + dt = datetime.fromisoformat(iso.replace("Z", "+00:00")) + return f"{dt.strftime('%b')} {dt.day}, {dt.year}" + except ValueError: + return iso + + +def _label_text_color(hex6: str) -> str: + try: + r, g, b = int(hex6[0:2], 16), int(hex6[2:4], 16), int(hex6[4:6], 16) + return "#0f172a" if 0.299 * r + 0.587 * g + 0.114 * b > 160 else "#f8fafc" + except (ValueError, IndexError): + return "#0f172a" + + +def _label_badge_context(labels: tuple) -> list[dict]: + result = [] + for name, color in labels: + if not name: + continue + result.append( + { + "name": name, + "bg": f"#{color}" if color else None, + "text": _label_text_color(color) if color else "#0f172a", + } + ) + return result + + +def _close_pr_mutations_enabled() -> bool: + value = str(getattr(settings, "ZULIP_CLOSE_PR_MUTATIONS_ENABLED", "")).strip().lower() + return value in {"1", "true", "yes", "on"} + + +def _enqueue_close_pr_post_actions(*, claims, pr_title: str | None, close_message: str) -> None: + from core.models import Repository, User + + pr_ref = f"{claims.pr_owner}/{claims.pr_repo}#{claims.pr_number}" + pr_url = f"https://github.com/{claims.pr_owner}/{claims.pr_repo}/pull/{claims.pr_number}" + pr_link = f"[{pr_ref}]({pr_url})" + title_part = f' ("{pr_title}")' if pr_title else "" + quote_block = f"\n```quote\n{close_message}\n```" if close_message else "" + + # 1. Best-effort sync. + repository = Repository.objects.filter(owner=claims.pr_owner, name=claims.pr_repo).only("id").first() + if repository is not None: + try: + from syncer.tasks.sync_tasks import sync_pr_task + + sync_pr_task.delay(repository.id, claims.pr_number) + except Exception: + logger.warning( + "close_pr_post_action_sync_enqueue_failed", + extra={"owner": claims.pr_owner, "repo": claims.pr_repo, "number": claims.pr_number}, + ) + + # 2. Best-effort DM to invoker. + dm_content = f"Pull request {pr_link}{title_part} has been closed.{quote_block}" + try: + ZulipClient().send_direct_message(to=[claims.zulip_user_id], content=dm_content) + except ZulipApiError: + logger.warning( + "close_pr_dm_failed", + extra={"zulip_user_id": claims.zulip_user_id, "pr_ref": pr_ref}, + ) + + # 3. Best-effort log post. + log_target = _resolve_repo_log_target(owner=claims.pr_owner, repo=claims.pr_repo) + if log_target is not None: + user = User.objects.filter(zulip_user_id=claims.zulip_user_id).only("zulip_full_name").first() + zulip_full_name = user.zulip_full_name if user and user.zulip_full_name else None + if zulip_full_name: + requester = f"@_**{zulip_full_name}|{claims.zulip_user_id}**" + else: + requester = f"`{claims.github_login}`" + now = datetime.now(dt_timezone.utc) + closed_at = f"{now.strftime('%b')} {now.day}, {now.year}, {now.strftime('%H:%M')} UTC" + log_content = f"PR {pr_link}{title_part} was closed by {requester} on {closed_at}.{quote_block}" + try: + ZulipClient().send_stream_message( + stream=log_target["stream"], + topic=log_target["topic"], + content=log_content, + ) + except ZulipApiError: + logger.warning( + "close_pr_log_post_failed", + extra={"owner": claims.pr_owner, "repo": claims.pr_repo, "pr_ref": pr_ref}, + ) + + +def _resolve_repo_log_target(*, owner: str, repo: str) -> dict | None: + raw = getattr(settings, "ZULIP_REPO_LOG", None) + if not isinstance(raw, dict): + return None + key = f"{owner}/{repo}" + entry = raw.get(key) + if not isinstance(entry, dict): + logger.warning( + "close_pr_no_repo_log_configured", + extra={"owner": owner, "repo": repo}, + ) + return None + stream = str(entry.get("stream") or "").strip() + topic = str(entry.get("topic") or "").strip() + if not stream or not topic: + logger.warning( + "close_pr_repo_log_missing_stream_or_topic", + extra={"owner": owner, "repo": repo}, + ) + return None + return {"stream": stream, "topic": topic} + + +def _close_pr_invalid_response(request: HttpRequest, *, reason: str) -> HttpResponse: + response = TemplateResponse( + request, + "zulip_bot/close_pr_invalid.html", + {"reason": reason}, + status=403, + ) + response["Cache-Control"] = "no-store" + return response + + def _prefs_invalid_response(request: HttpRequest, *, reason: str) -> HttpResponse: response = TemplateResponse( request, diff --git a/qb_site/zulip_bot/webhook/payload.py b/qb_site/zulip_bot/webhook/payload.py index f506c9c9..61f519de 100644 --- a/qb_site/zulip_bot/webhook/payload.py +++ b/qb_site/zulip_bot/webhook/payload.py @@ -102,7 +102,7 @@ def parse_command(message_content: str) -> ParsedCommand | None: content = content[1:] parts = content.split(maxsplit=1) - name = parts[0].lower() + name = parts[0].lower().replace("_", "-") args = parts[1] if len(parts) > 1 else "" return ParsedCommand(name=name, args=args)