diff --git a/apps/docs/content/docs/github-integration.ja.mdx b/apps/docs/content/docs/github-integration.ja.mdx index ad58c4e05e..63783835cf 100644 --- a/apps/docs/content/docs/github-integration.ja.mdx +++ b/apps/docs/content/docs/github-integration.ja.mdx @@ -14,11 +14,11 @@ import { Callout } from "fumadocs-ui/components/callout"; | 場所 | 動作 | |---|---| | **設定 → GitHub** | ワークスペースの admin には、マスタートグル、**Connect GitHub** ボタン、機能スイッチ（PR サイドバー、Co-authored-by、自動紐づけ）を備えた GitHub タブが表示されます。インストール後は GitHub タブに戻ります。 | -| **イシューサイドバー → Pull requests** | このイシューに自動で紐づいたすべての PR が、タイトル、リポジトリ、状態（`Open` / `Draft` / `Merged` / `Closed`）、作成者、diff 統計、CI ステータスとともに表示されます。行をクリックすると GitHub の該当 PR に移動します。 | +| **イシューサイドバー → Pull requests** | このイシューに自動で紐づいたすべての PR が、タイトル、リポジトリ、状態（`Open` / `Draft` / `Merged` / `Closed`）、作成者とともに表示されます。行をクリックすると GitHub の該当 PR に移動します。 | | **Webhook（バックグラウンド）** | すべての `pull_request` イベントで、Multica は PR 行を upsert し、PR からイシュー識別子をスキャンして、紐づけ行を（再）構築します。冪等性があり、同じ delivery を再送しても変化はありません。 | | **マージ時のステータス自動変更** | PR が `merged` に遷移すると、まだ `Done` でも `Cancelled` でもない、紐づいたすべてのイシューが `Done` に移動します。ステータス変更は source `github_pr_merged` でタイムラインに記録されます。 | -ミラーリングされるのは PR 自体のみです。コミットとオープンな PR のないブランチ ref はモデル化され**ません**。この連携は意図的に狭く設計されています。 +ミラーリングされるのは PR 自体のみです。コミット、オープンな PR のないブランチ ref、CI チェックの状態はモデル化され**ません**。この連携は意図的に狭く設計されています。 ## 識別子のマッチング方法 @@ -94,9 +94,8 @@ Multica Cloud で Multica を実行している場合、連携はすでに構成 | **Webhook URL** | `https:///api/webhooks/github`。 | | **Webhook secret** | 長いランダム文字列を生成してください（例: `openssl rand -hex 32`）。手順 2 で同じ値を Multica の env に貼り付けます。 | | **Permissions → Repository → Pull requests** | **Read-only**。 | -| **Permissions → Repository → Checks** | **Read-only**。PR サイドバーの CI ステータスに必須です — この権限がないと GitHub は `check_suite` を配信しません。read-only では Multica は**完了した** suite しか見えません。[CI ステータスで分かること・分からないこと](#ci-ステータスで分かること分からないこと)を参照してください。 | | **Permissions → Repository → Metadata** | Read-only（必須）。 | -| **Subscribe to events** | **Pull request** と **Check suite** をチェックしてください。 | +| **Subscribe to events** | **Pull request** をチェックしてください。 | | **Where can this GitHub App be installed?** | お好みで。単一組織のセットアップなら `Only on this account` で十分です。 | **Create GitHub App** の後、App の詳細ページから 2 つのことを控えておいてください。 @@ -175,56 +174,12 @@ curl -i -X POST https:///api/webhooks/github \ | `401 invalid signature` | サーバーがロードした secret が思っている値で**ありません**。 | env 変数が実行中のプロセスに反映されたか確認してください（例: `kubectl exec` → `echo -n "$GITHUB_WEBHOOK_SECRET" \| wc -c`）。再デプロイしてください。 | | `503 github webhooks not configured` | プロセスで `GITHUB_WEBHOOK_SECRET` が空です。 | env 変数を設定し、API を再起動してください。 | -### PR 行に CI ステータスが表示されない - -GitHub 上では CI が実行されているのに、すべての PR 行が **チェックはまだ報告されていません** と表示される場合です。App が `pull_request` は受け取れているが `check_suite` を受け取れていないことを意味します — リンクと merge → Done は動作し、CI ステータスだけが空のままになります。 - -App が実際に何を購読しているかを確認します（このエンドポイントは公開されており、認証は不要です）。 - -```bash -curl -s https://api.github.com/apps/ | jq '{events, permissions}' -``` - -`events` に `check_suite`、`permissions` に `"checks": "read"` が必要です。どちらかが欠けている場合： - -1. App を編集 → **Permissions → Repository → Checks** → **Read-only** -2. App を編集 → **Subscribe to events** → **Check suite** をチェック -3. **Save changes** をクリック - - -**既存のインストールは新しい権限を承認する必要があります。** `checks` 権限の追加は遡って適用されません — GitHub はこの App をインストールした各アカウントにメールを送り、誰かが承認するまで変更を保留します。それまでそのインストールは `check_suite` を一切配信しません。**Settings → Applications → Installed GitHub Apps → 対象の App → Review request** で承認してください（Organization へのインストールは organization owner が行う必要があります）。承認後の `new_permissions_accepted` イベントは Multica が自動的に処理するため、再デプロイは不要です。 - - -CI ステータスは以降のイベントにのみ反映されます — Multica は受信した suite を記録するだけで、過去の履歴は埋めません。すぐに確認したい場合は、オープンな PR にコミットをプッシュするか、チェックを re-run してください。 - -### CI ステータスで分かること・分からないこと - -Multica が要求するのは **Checks: Read-only** のみで、この上限は意図的なものです。ただし、サイドバーの行に依存する前に理解しておくべき結果が伴います。 - -GitHub の [webhook 契約](https://docs.github.com/en/webhooks/webhook-events-and-payloads#check_suite)は、`check_suite` の配信を権限レベルで分けています。 - -| イベント action | `checks: read` で受信 | `checks: write` で受信 | -|---|---|---| -| `completed` | ✅ | ✅ | -| `requested` / `rerequested` | ❌ | ✅ | - -したがって推奨の read-only 構成では、Multica は suite が**完了した**ことしか知らず、**開始した**ことは決して分かりません。ここから 2 つの帰結があります。 - -- **実行中の CI は、どの権限レベルでも表示されません。** チェックが実行中の PR は「チェックはまだ報告されていません」と表示され、CI がまったくない PR と区別がつきません。実行中を示す表示はありません。 -- **緑の行は「報告されたものは成功した」であり、「すべて成功した」ではありません。** リポジトリに 2 つのレポート元（GitHub Actions と Vercel など）があり、片方だけが完了している場合、行はその 1 つの suite に基づいて**チェックに成功しました**と表示します。文言が「すべてのチェックに成功しました」ではなく「チェックに成功しました」である理由がこれです。この行は下限値として扱い、マージ前には GitHub 側で確認してください。 - - -**権限を上げれば解決するのでは？** しません — `checks: write` は「より広い読み取り」ではありません。GitHub が `requested` / `rerequested` を送る意味は「**この commit に対して GitHub があなたの App 用の check suite を作成しました。check run を追加してください**」であり、「他の CI プロバイダーが実行を開始した」ではありません。Multica は観測側であり check run を作成しないため、これらのイベントは GitHub Actions や Vercel について何の情報も持ちません。webhook ハンドラーが `completed` 以外のすべての action を無視するのはこのためです。実際の実行中ステータスには別の仕組み — checks API のポーリング、または `check_run` ベースのモデル — が必要で、権限の引き上げでは解決しません。 - - ## 制限事項 現時点で知っておくべき、いくつかの粗い部分があります。 - **まだ手動の紐づけ UI はありません** — PR を紐づける唯一の方法は、ブランチ、タイトル、本文に識別子を置くことです。 -- **CI ステータスは check suite 単位で、ジョブ単位ではありません** — 各レポート元（GitHub Actions、Vercel など）を成功 / 失敗のいずれか 1 つに丸めます。個々のジョブ名とログは GitHub 側にのみ残ります。レビューコメントとレビュアーは表示されません。 -- **実行中の CI は見えず、緑の行は下限値です** — Multica は `completed` の suite しか記録しないため、実行中のチェックは見えず、別のレポート元がまだ実行中でも行が「チェックに成功しました」と表示されることがあります。詳細は [CI ステータスで分かること・分からないこと](#ci-ステータスで分かること分からないこと)にあります。 -- **CI ステータスには `check_suite` の購読が必要です** — このイベントが文書化される前に作成された App では、すべての PR で「チェックはまだ報告されていません」と表示され続けます。[PR 行に CI ステータスが表示されない](#pr-行に-ci-ステータスが表示されない)を参照してください。 +- **CI / チェック状態はありません** — PR 自体のみがミラーリングされます。ビルド状態、レビューコメント、レビュアーは Multica には表示されません。 - マージ → Done ルールに対する**ワークスペースレベルの設定はありません** — 固定のデフォルトです（`cancelled` でない限り `merged → done`）。ワークスペースでカスタマイズできるマッピングは将来の追加予定です。 - **1 つのイシューに複数の PR が紐づく場合、マージは保守的です** — 2 つの PR がどちらも `MUL-123` を参照していて最初の 1 つがマージされると、イシューはただちに `Done` に移動します。進める前に紐づいたすべての PR が解決されるのを待つ後続の変更が進行中です。 diff --git a/apps/docs/content/docs/github-integration.ko.mdx b/apps/docs/content/docs/github-integration.ko.mdx index e203518bf2..2803ac959d 100644 --- a/apps/docs/content/docs/github-integration.ko.mdx +++ b/apps/docs/content/docs/github-integration.ko.mdx @@ -14,11 +14,11 @@ import { Callout } from "fumadocs-ui/components/callout"; | 위치 | 동작 | |---|---| | **설정 → GitHub** | 워크스페이스 admin에게는 마스터 토글, **Connect GitHub** 버튼, 기능 스위치(PR 사이드바, Co-authored-by, 자동 연결)가 있는 GitHub 탭이 보입니다. 설치 후에는 GitHub 탭으로 다시 돌아옵니다. | -| **이슈 사이드바 → Pull requests** | 이 이슈에 자동 연결된 모든 PR이 제목, 저장소, 상태(`Open` / `Draft` / `Merged` / `Closed`), 작성자, diff 통계, CI 상태와 함께 표시됩니다. 행을 클릭하면 GitHub의 해당 PR로 이동합니다. | +| **이슈 사이드바 → Pull requests** | 이 이슈에 자동 연결된 모든 PR이 제목, 저장소, 상태(`Open` / `Draft` / `Merged` / `Closed`), 작성자와 함께 표시됩니다. 행을 클릭하면 GitHub의 해당 PR로 이동합니다. | | **Webhook(백그라운드)** | 모든 `pull_request` 이벤트에서 Multica는 PR 행을 upsert하고, PR에서 이슈 식별자를 스캔한 뒤, 연결 행을 (다시) 구성합니다. 멱등성이 있어 동일 delivery를 재전송해도 변화가 없습니다. | | **머지 시 상태 자동 변경** | PR이 `merged`로 전환되면, 아직 `Done`이나 `Cancelled`가 아닌 모든 연결된 이슈가 `Done`으로 이동합니다. 상태 변경은 source `github_pr_merged`로 타임라인에 기록됩니다. | -미러링되는 것은 PR 자체뿐입니다. 커밋과 열린 PR이 없는 브랜치 ref는 모델링되지 **않습니다**. 이 연동은 의도적으로 좁게 설계되었습니다. +미러링되는 것은 PR 자체뿐입니다. 커밋, 열린 PR이 없는 브랜치 ref, CI 체크 상태는 모델링되지 **않습니다**. 이 연동은 의도적으로 좁게 설계되었습니다. ## 식별자 매칭 방식 @@ -94,9 +94,8 @@ Multica Cloud에서 Multica를 실행 중이라면 연동이 이미 구성되어 | **Webhook URL** | `https:///api/webhooks/github`. | | **Webhook secret** | 긴 무작위 문자열을 생성하세요(예: `openssl rand -hex 32`). 2단계에서 동일한 값을 Multica의 env에 붙여넣게 됩니다. | | **Permissions → Repository → Pull requests** | **Read-only**. | -| **Permissions → Repository → Checks** | **Read-only**. PR 사이드바의 CI 상태에 필요합니다 — 이 권한이 없으면 GitHub은 `check_suite`를 전달하지 않습니다. read-only에서는 Multica가 **완료된** suite만 볼 수 있습니다. [CI 상태로 알 수 있는 것과 없는 것](#ci-상태로-알-수-있는-것과-없는-것)을 참고하세요. | | **Permissions → Repository → Metadata** | Read-only(필수). | -| **Subscribe to events** | **Pull request**와 **Check suite**를 체크하세요. | +| **Subscribe to events** | **Pull request**를 체크하세요. | | **Where can this GitHub App be installed?** | 선택 사항. 단일 조직 설정에는 `Only on this account`로 충분합니다. | **Create GitHub App** 후, App 상세 페이지에서 두 가지를 기록해 두세요. @@ -175,56 +174,12 @@ curl -i -X POST https:///api/webhooks/github \ | `401 invalid signature` | 서버가 로드한 secret이 생각하는 값이 **아닙니다**. | env 변수가 실행 중인 프로세스에 적용되었는지 확인하세요(예: `kubectl exec` → `echo -n "$GITHUB_WEBHOOK_SECRET" \| wc -c`). 재배포하세요. | | `503 github webhooks not configured` | 프로세스에서 `GITHUB_WEBHOOK_SECRET`이 비어 있습니다. | env 변수를 설정하고 API를 재시작하세요. | -### PR 행에 CI 상태가 표시되지 않음 - -GitHub에서는 CI가 분명히 실행됐는데도 모든 PR 행이 **체크가 아직 보고되지 않았습니다**로 표시되는 경우입니다. App이 `pull_request`는 받고 있지만 `check_suite`는 받지 못하고 있다는 뜻입니다 — 연결과 merge → Done은 동작하고 CI 상태만 계속 비어 있습니다. - -App이 실제로 무엇을 구독하고 있는지 확인하세요(이 엔드포인트는 공개되어 있어 인증이 필요 없습니다). - -```bash -curl -s https://api.github.com/apps/ | jq '{events, permissions}' -``` - -`events`에 `check_suite`, `permissions`에 `"checks": "read"`가 있어야 합니다. 둘 중 하나라도 없다면: - -1. App 편집 → **Permissions → Repository → Checks** → **Read-only** -2. App 편집 → **Subscribe to events** → **Check suite** 체크 -3. **Save changes** 클릭 - - -**기존 설치는 새 권한을 승인해야 합니다.** `checks` 권한 추가는 소급 적용되지 않습니다 — GitHub은 이 App을 설치한 각 계정에 메일을 보내고, 누군가 승인할 때까지 변경을 보류합니다. 그때까지 해당 설치는 `check_suite`를 전혀 전달하지 않습니다. **Settings → Applications → Installed GitHub Apps → 해당 App → Review request**에서 승인하세요(조직 설치는 조직 owner가 해야 합니다). 승인 후 발생하는 `new_permissions_accepted` 이벤트는 Multica가 자동으로 처리하므로 재배포는 필요 없습니다. - - -CI 상태는 이후 이벤트에만 반영됩니다 — Multica는 수신한 suite를 기록할 뿐 과거 이력을 채우지 않습니다. 바로 확인하려면 열린 PR에 커밋을 푸시하거나 체크를 re-run 하세요. - -### CI 상태로 알 수 있는 것과 없는 것 - -Multica는 **Checks: Read-only**만 요청하며, 이 상한선은 의도적입니다. 다만 사이드바 행에 의존하기 전에 알아야 할 결과가 따릅니다. - -GitHub의 [webhook 계약](https://docs.github.com/en/webhooks/webhook-events-and-payloads#check_suite)은 `check_suite` 전달을 권한 수준으로 나눕니다. - -| 이벤트 action | `checks: read`로 수신 | `checks: write`로 수신 | -|---|---|---| -| `completed` | ✅ | ✅ | -| `requested` / `rerequested` | ❌ | ✅ | - -따라서 권장되는 read-only 구성에서 Multica는 suite가 **끝났다**는 것만 알 수 있고 **시작했다**는 것은 결코 알 수 없습니다. 여기서 두 가지가 따라옵니다. - -- **실행 중인 CI는 어떤 권한 수준에서도 표시되지 않습니다.** 검사가 실행 중인 PR은 "체크가 아직 보고되지 않았습니다"로 표시되어 CI가 아예 없는 PR과 구분되지 않습니다. 진행 중 표시가 없습니다. -- **초록색 행은 "보고된 것은 통과했다"이지 "전부 통과했다"가 아닙니다.** 저장소에 보고 앱이 둘(예: GitHub Actions와 Vercel) 있고 하나만 끝난 상태라면, 행은 그 하나의 suite를 근거로 **검사 통과**를 표시합니다. 문구가 "모든 검사 통과"가 아니라 "검사 통과"인 이유가 이것입니다. 이 행은 하한값으로 보고, 머지 전에는 GitHub에서 확인하세요. - - -**권한을 올리면 해결되지 않나요?** 안 됩니다 — `checks: write`는 "더 넓은 읽기"가 아닙니다. GitHub이 `requested` / `rerequested`를 보내는 의미는 "**이 커밋에 대해 GitHub이 당신의 App용 check suite를 만들었으니 check run을 추가하라**"이지 "다른 CI 제공자가 실행을 시작했다"가 아닙니다. Multica는 관찰자이며 check run을 만들지 않으므로 이 이벤트들은 GitHub Actions나 Vercel에 대한 정보를 전혀 담고 있지 않습니다. webhook 핸들러가 `completed` 외의 모든 action을 무시하는 이유가 이것입니다. 실제 실행 중 상태에는 다른 메커니즘 — checks API 폴링 또는 `check_run` 기반 모델 — 이 필요하며 권한 상향으로는 해결되지 않습니다. - - ## 제한 사항 현재 알아 둬야 할 몇 가지 거친 부분이 있습니다. - **아직 수동 연결 UI가 없습니다** — PR을 연결하는 유일한 방법은 브랜치, 제목, 본문에 식별자를 두는 것입니다. -- **CI 상태는 job 단위가 아니라 check suite 단위입니다** — 각 보고 앱(GitHub Actions, Vercel 등)을 성공 / 실패 하나로 묶습니다. 개별 job 이름과 로그는 GitHub에만 남습니다. 리뷰 댓글과 리뷰어는 표시되지 않습니다. -- **실행 중인 CI는 보이지 않으며 초록색 행은 하한값입니다** — Multica는 `completed` suite만 기록하므로 실행 중인 검사는 보이지 않고, 다른 보고 앱이 아직 실행 중인데도 행이 "검사 통과"로 표시될 수 있습니다. 자세한 내용은 [CI 상태로 알 수 있는 것과 없는 것](#ci-상태로-알-수-있는-것과-없는-것)에 있습니다. -- **CI 상태에는 `check_suite` 구독이 필요합니다** — 이 이벤트가 문서화되기 전에 만든 App은 모든 PR에서 "체크가 아직 보고되지 않았습니다"가 계속 표시됩니다. [PR 행에 CI 상태가 표시되지 않음](#pr-행에-ci-상태가-표시되지-않음)을 참고하세요. +- **CI / 체크 상태가 없습니다** — PR 자체만 미러링됩니다. 빌드 상태, 리뷰 댓글, 리뷰어는 Multica에 표시되지 않습니다. - 머지 → 완료 규칙에 대한 **워크스페이스 수준 설정이 없습니다** — 고정된 기본값입니다(`cancelled`가 아닌 한 `merged → done`). 워크스페이스에서 커스터마이즈할 수 있는 매핑은 향후 추가될 예정입니다. - **하나의 이슈에 여러 PR이 연결된 경우 머지가 보수적입니다** — 두 PR이 모두 `MUL-123`을 참조하고 첫 번째가 머지되면, 이슈는 즉시 `Done`으로 이동합니다. 진행하기 전에 연결된 모든 PR이 해결되기를 기다리는 후속 변경이 진행 중입니다. diff --git a/apps/docs/content/docs/github-integration.mdx b/apps/docs/content/docs/github-integration.mdx index 4922406726..5d5b1095fc 100644 --- a/apps/docs/content/docs/github-integration.mdx +++ b/apps/docs/content/docs/github-integration.mdx @@ -14,12 +14,11 @@ There is no per-issue setup. The whole flow is identifier-driven. | Surface | Behavior | |---|---| | **Settings → GitHub** | Workspace admins see the GitHub tab with a master toggle, **Connect GitHub** button, and feature switches (PR sidebar, Co-authored-by, auto-link). After install you bounce back to the GitHub tab. | -| **Issue sidebar → Pull requests** | Every PR auto-linked to this issue, with title, repo, state (`Open` / `Draft` / `Merged` / `Closed`), author, diff stats, and CI status. Click a row to jump to the PR on GitHub. | +| **Issue sidebar → Pull requests** | Every PR auto-linked to this issue, with title, repo, state (`Open` / `Draft` / `Merged` / `Closed`), and author. Click a row to jump to the PR on GitHub. | | **Webhook (background)** | On every `pull_request` event, Multica upserts the PR row, scans the PR for issue identifiers, and (re)builds the link rows. Idempotent — replaying a delivery is a no-op. | -| **CI status** | On every `check_suite` event, Multica records the suite against the PR's current head commit and rolls the results up into the sidebar row: **Checks passed** or **Some checks failed**, with a segmented bar showing the split. Requires the `check_suite` event subscription — see [setup](#1-create-the-github-app). Multica sees **completed** suites only; read [what CI status can and cannot tell you](#what-ci-status-can-and-cannot-tell-you) before relying on it. | | **Auto-status on merge** | When a PR transitions to `merged`, every linked issue not already `Done` or `Cancelled` is moved to `Done`. The status change is timeline-logged with source `github_pr_merged`. | -Only the PR itself is mirrored. Commits and branch refs without an open PR are **not** modeled. The integration is intentionally narrow. +Only the PR itself is mirrored. Commits, branch refs without an open PR, and CI check states are **not** modeled. The integration is intentionally narrow. ## Multiple workspaces @@ -81,7 +80,7 @@ After disconnect, mirrored PR rows stay in the database so historical issue side - **Connect / disconnect** require workspace **owner or admin**. Members see the card description but no Connect button. - The **Pull requests** sidebar on an issue is visible to anyone who can read the issue — same permissions as the rest of issue detail. -- The GitHub App requests **read-only** access to pull requests, checks, and metadata. Multica never pushes commits, comments, or status checks back to GitHub. +- The GitHub App requests **read-only** access to pull requests and metadata. Multica never pushes commits, comments, or status checks back to GitHub. ## Self-host setup @@ -108,9 +107,8 @@ Fill in: | **Webhook URL** | `https:///api/webhooks/github`. | | **Webhook secret** | Generate a long random string (e.g. `openssl rand -hex 32`). You'll paste the same value into Multica's env in step 2. | | **Permissions → Repository → Pull requests** | **Read-only**. | -| **Permissions → Repository → Checks** | **Read-only**. Required for CI status on the PR sidebar row — GitHub will not deliver `check_suite` without it. Read-only means Multica sees completed suites only; see [what CI status can and cannot tell you](#what-ci-status-can-and-cannot-tell-you). | | **Permissions → Repository → Metadata** | Read-only (mandatory). | -| **Subscribe to events** | Tick **Pull request** and **Check suite**. | +| **Subscribe to events** | Tick **Pull request**. | | **Where can this GitHub App be installed?** | Your choice. `Only on this account` is fine for single-org setups. | After **Create GitHub App**, note two things from the App's detail page: @@ -189,56 +187,12 @@ curl -i -X POST https:///api/webhooks/github \ | `401 invalid signature` | Server's loaded secret is **not** what you think it is. | Confirm the env var landed in the running process (e.g. `kubectl exec` → `echo -n "$GITHUB_WEBHOOK_SECRET" | wc -c`). Re-deploy. | | `503 github webhooks not configured` | `GITHUB_WEBHOOK_SECRET` is empty in the process. | Set the env var, restart the API. | -### PR rows show no CI status - -Every PR row reads **Checks haven't reported yet**, even though CI clearly ran on GitHub. This means the App is receiving `pull_request` events but no `check_suite` events — links and merge-to-Done work, CI status stays empty. - -Confirm what your App is actually subscribed to (this endpoint is public, no auth needed): - -```bash -curl -s https://api.github.com/apps/ | jq '{events, permissions}' -``` - -You want `check_suite` in `events` and `"checks": "read"` in `permissions`. If either is missing: - -1. Edit the App → **Permissions → Repository → Checks** → **Read-only**. -2. Edit the App → **Subscribe to events** → tick **Check suite**. -3. **Save changes.** - - -**Existing installations must accept the new permission.** Adding the `checks` permission does not apply retroactively — GitHub emails each account that installed the App and holds the change until someone approves it. Until then that installation still delivers no `check_suite` events. Approve it at **Settings → Applications → Installed GitHub Apps → your App → Review request** (organization installs need an org owner). Multica handles the resulting `new_permissions_accepted` event automatically; no redeploy is needed. - - -CI status only populates going forward — Multica records suites as they arrive and does not backfill history. Push a commit to an open PR (or re-run its checks) to see the first rows appear. - -### What CI status can and cannot tell you - -Multica requests **Checks: Read-only**, and that ceiling is deliberate. It also has a consequence worth understanding before you rely on the sidebar row. - -GitHub's [webhook contract](https://docs.github.com/en/webhooks/webhook-events-and-payloads#check_suite) splits `check_suite` delivery by permission level: - -| Event action | Delivered with `checks: read` | Delivered with `checks: write` | -|---|---|---| -| `completed` | ✅ | ✅ | -| `requested` / `rerequested` | ❌ | ✅ | - -So under the recommended read-only setup Multica learns that a suite **finished**, never that one **started**. Two things follow: - -- **In-flight CI is not surfaced, at any permission level.** A PR whose checks are still running shows *Checks haven't reported yet*, the same as a PR with no CI at all. There is no running/pending indicator. -- **A green row means "what reported, passed" — not "everything passed."** If a repo has two reporting apps (say GitHub Actions and Vercel) and only one has finished, the row reads **Checks passed** based on that one suite. This is why the copy is *Checks passed* and not *All checks passed*. Treat the row as a lower bound and open the PR on GitHub before merging on its say-so. - - -**Why doesn't raising the permission fix this?** It doesn't — `checks: write` is not a bigger read. GitHub sends `requested` / `rerequested` to say "**GitHub has created a check suite for YOUR app on this commit; now add your check runs to it**", not "some other CI provider started". Multica is an observer and never creates check runs, so those events carry no information about GitHub Actions or Vercel. The webhook handler ignores every action except `completed` for exactly this reason. Real in-flight status needs a different mechanism — polling the checks API, or a `check_run`-based model — not a permission bump. - - ## Limitations A few rough edges to be aware of today: - **No manual link UI yet** — the only way to link a PR is to have the identifier in its branch, title, or body. -- **CI status is per check suite, not per job** — Multica rolls each reporting app (GitHub Actions, Vercel, …) up to one pass/fail result. Individual job names and logs stay on GitHub. Review comments and reviewers are not surfaced. -- **CI status needs the `check_suite` subscription** — an App created before this event was documented will show *Checks haven't reported yet* on every PR forever. See [PR rows show no CI status](#pr-rows-show-no-ci-status). -- **No in-flight CI, and a green row is a lower bound** — Multica only ever records `completed` suites, so running checks are invisible and a row can read *Checks passed* while another provider is still going. Details in [what CI status can and cannot tell you](#what-ci-status-can-and-cannot-tell-you). +- **No CI / check state** — only the PR itself is mirrored. Build status, review comments, and reviewers are not surfaced in Multica. - **No workspace-level config** for the merge → Done rule — it's a fixed default (`merged → done`, unless `cancelled`). Workspace-customizable mappings are a future addition. - **Multi-PR-to-one-issue is conservative on merge** — if two PRs both reference `MUL-123` and the first one merges, the issue is moved to `Done` immediately. A follow-up change to wait for all linked PRs to resolve before advancing is in progress. diff --git a/apps/docs/content/docs/github-integration.zh.mdx b/apps/docs/content/docs/github-integration.zh.mdx index 33c2074428..3d35ace5ff 100644 --- a/apps/docs/content/docs/github-integration.zh.mdx +++ b/apps/docs/content/docs/github-integration.zh.mdx @@ -14,11 +14,11 @@ import { Callout } from "fumadocs-ui/components/callout"; | 出现位置 | 行为 | |---|---| | **Settings → GitHub** | 工作区 owner / admin 看到 GitHub 这个 tab，里面有主开关、**Connect GitHub** 按钮，以及功能开关（PR 侧栏、Co-authored-by、auto-link）。点 Connect 会打开 GitHub 的 App 安装页；装好后跳回 GitHub tab。 | -| **Issue 详情侧栏 → Pull requests** | 列出所有自动关联到该 issue 的 PR，含标题、仓库、状态（`Open` / `Draft` / `Merged` / `Closed`）、作者、diff 统计和 CI 状态。点一行跳到 GitHub。 | +| **Issue 详情侧栏 → Pull requests** | 列出所有自动关联到该 issue 的 PR，含标题、仓库、状态（`Open` / `Draft` / `Merged` / `Closed`）和作者。点一行跳到 GitHub。 | | **Webhook（后台）** | 每次 `pull_request` 事件触发：upsert PR 行 → 扫描里面的 issue 编号 →（重新）建立 link。幂等——重投 delivery 不会产生重复记录。 | | **Merge 自动改 status** | PR 转 `merged` 时，所有已关联且状态不是 `Done` / `Cancelled` 的 issue 会被推到 `Done`。时间线里以 source 为 `github_pr_merged` 记录。 | -只镜像 PR 本身。Commit、没开 PR 的分支**不**入库——集成有意保持窄边界。 +只镜像 PR 本身。Commit、没开 PR 的分支、CI 检查状态都**不**入库——集成有意保持窄边界。 ## 多个工作区 @@ -80,7 +80,7 @@ PR **关闭但没合并**——只更新 PR 卡片的状态为 `Closed`，issue - **Connect / Disconnect** 需要工作区 **owner 或 admin**。普通成员能看到卡片描述但看不到 Connect 按钮 - **Pull requests** 侧栏对所有能看到该 issue 的成员可见——和 issue 详情页其他部分权限一致 -- GitHub App 申请的是 PR、Checks 和 Metadata 的 **只读** 权限。Multica 从不向 GitHub 推 commit、评论或 status check +- GitHub App 申请的是 PR 和 Metadata 的 **只读** 权限。Multica 从不向 GitHub 推 commit、评论或 status check ## Self-Host 配置 @@ -107,9 +107,8 @@ Self-Host 需要：建一个 GitHub App、指向你的 server、设两个环境 | **Webhook URL** | `https:///api/webhooks/github` | | **Webhook secret** | 生成一个长随机字符串（例如 `openssl rand -hex 32`）。这个值会同样填到 step 2 的 env 里 | | **Permissions → Repository → Pull requests** | **Read-only** | -| **Permissions → Repository → Checks** | **Read-only**。PR 侧栏显示 CI 状态所必需——没有这个权限 GitHub 不会投递 `check_suite`。只读意味着 Multica 只能看到**已完成**的 suite，见 [CI 状态能说明什么、不能说明什么](#ci-状态能说明什么不能说明什么) | | **Permissions → Repository → Metadata** | Read-only（必填）| -| **Subscribe to events** | 勾选 **Pull request** 和 **Check suite** | +| **Subscribe to events** | 勾选 **Pull request** | | **Where can this GitHub App be installed?** | 自选。单组织部署建议选 `Only on this account` | 点 **Create GitHub App** 之后，从详情页记下两件事： @@ -187,56 +186,12 @@ curl -i -X POST https:///api/webhooks/github \ | `401 invalid signature` | server 加载的 secret **不是**你以为的那个 | 进容器确认 env 实际生效（例如 `kubectl exec` → `echo -n "$GITHUB_WEBHOOK_SECRET" \| wc -c`），重新部署 | | `503 github webhooks not configured` | `GITHUB_WEBHOOK_SECRET` 在进程里是空的 | 配上 env，重启 API | -### PR 行不显示 CI 状态 - -每个 PR 行都写着 **Checks 还没有汇报**，但 GitHub 上 CI 明明跑过。这说明 App 收得到 `pull_request`，收不到 `check_suite`——关联和 merge 转 Done 都正常，唯独 CI 状态一直空着。 - -先确认 App 实际订阅了什么（这个接口是公开的，不需要鉴权）： - -```bash -curl -s https://api.github.com/apps/<你的-app-slug> | jq '{events, permissions}' -``` - -`events` 里要有 `check_suite`，`permissions` 里要有 `"checks": "read"`。缺哪个补哪个： - -1. 编辑 App → **Permissions → Repository → Checks** → **Read-only** -2. 编辑 App → **Subscribe to events** → 勾上 **Check suite** -3. **点 Save changes** - - -**已经装好的 installation 必须手动接受新权限。** 加 `checks` 权限不会自动生效——GitHub 会给每个装了这个 App 的账号发邮件，在有人批准之前一直挂起，这期间该 installation 依然一条 `check_suite` 都不投递。到 **Settings → Applications → Installed GitHub Apps → 你的 App → Review request** 批准（组织安装需要组织 owner 操作）。批准后 Multica 会自动处理 `new_permissions_accepted` 事件，不需要重新部署。 - - -CI 状态只对之后的事件生效——Multica 按收到的 suite 记录，不会回填历史。想立刻看到效果，往一个开着的 PR 推一个 commit，或者 re-run 一次它的 checks。 - -### CI 状态能说明什么、不能说明什么 - -Multica 只申请 **Checks: Read-only**，这个上限是刻意的。但它有一个你在依赖侧栏那一行之前需要知道的后果。 - -GitHub 的 [webhook 契约](https://docs.github.com/en/webhooks/webhook-events-and-payloads#check_suite)按权限级别区分 `check_suite` 的投递： - -| 事件 action | `checks: read` 能收到 | `checks: write` 能收到 | -|---|---|---| -| `completed` | ✅ | ✅ | -| `requested` / `rerequested` | ❌ | ✅ | - -所以在推荐的只读配置下，Multica 只知道一个 suite **跑完了**，永远不知道它**开始了**。由此有两个结论： - -- **运行中的 CI 不会显示，任何权限级别都一样。** check 还在跑的 PR 显示「Checks 还没有汇报」，和一个根本没配 CI 的 PR 长得一模一样。没有运行中指示。 -- **绿色那一行的意思是「已汇报的都通过了」，不是「全都通过了」。** 如果一个仓库有两个上报方（比如 GitHub Actions 和 Vercel），只有一个跑完时，这一行就会基于那一个 suite 显示**检查通过**。这正是文案是「检查通过」而不是「全部检查通过」的原因。把这一行当作下界看，真要合并之前还是去 GitHub 上确认。 - - -**那把权限提上去不就好了？** 不行——`checks: write` 不是「读得更多」。GitHub 发 `requested` / `rerequested` 的意思是「**GitHub 已经在这个 commit 上为你的 App 建了一个 check suite，现在请你往里加 check run**」，而不是「别的 CI 提供方开始跑了」。Multica 是观察者，从不创建 check run，所以这些事件根本不携带关于 GitHub Actions 或 Vercel 的任何信息。webhook handler 正是因此只接受 `completed`，其他 action 一律忽略。真正的运行中状态需要换机制——轮询 checks API，或者改用基于 `check_run` 的模型——提权解决不了。 - - ## 已知限制 目前还没做的几个边界： - **手动 link UI 暂未提供**——关联 PR 的唯一方法是把 issue 编号写到 PR 分支 / 标题 / 正文 -- **CI 状态按 check suite 聚合，不细到 job**——每个上报方（GitHub Actions、Vercel……）折叠成一个通过 / 失败的结果，具体 job 名称和日志仍然只在 GitHub 上。reviewer 评论和 reviewer 列表没接进 Multica -- **看不到运行中的 CI，绿色只是下界**——Multica 只会记录 `completed` 的 suite，所以正在跑的 check 完全不可见，而且在另一个上报方还没跑完时行里就可能显示「检查通过」。细节见 [CI 状态能说明什么、不能说明什么](#ci-状态能说明什么不能说明什么) -- **CI 状态依赖 `check_suite` 订阅**——在补上这个事件之前创建的 App，每个 PR 会永远显示「Checks 还没有汇报」。见 [PR 行不显示 CI 状态](#pr-行不显示-ci-状态) +- **不读 CI / check 状态**——只镜像 PR 本身，构建状态、reviewer 评论、reviewer 列表都没接进 Multica - **没有工作区级别的 merge → status 映射配置**——默认固定是 `merged → done`（cancelled 除外）。可配置映射是后续迭代 - **同 issue 多 PR 时，merge 行为偏激进**——两个 PR 都引用 `MUL-123` 时，第一个 merge 就把 issue 转 Done。"等所有关联 PR 都解决再推进 issue 状态"的优化已经在做了 diff --git a/packages/core/github/pull-request-status.ts b/packages/core/github/pull-request-status.ts index 44f98cc2fd..2b80b436cf 100644 --- a/packages/core/github/pull-request-status.ts +++ b/packages/core/github/pull-request-status.ts @@ -16,18 +16,6 @@ import type { GitHubPullRequest } from "../types"; // 7. no suite + mergeable=clean → status_ready // 8. otherwise → status_unknown // -// Completeness caveat (MUL-5180): these counts only ever cover check suites -// Multica has actually observed, which is NOT the same as "every suite that -// will run". GitHub delivers `check_suite.requested` / `.rerequested` only to -// Apps holding Checks *write*; the Multica App deliberately stays read-only, -// so in practice only `completed` suites arrive. A PR with two reporting apps -// therefore passes through a window where the first has completed and the -// second has not yet been seen at all. -// -// That is why rule 6 renders as "Checks passed" and not "All checks passed" — -// we can report what reported, never that everything did. Do not reintroduce -// completeness wording here without also fixing the underlying signal. -// // Note: this table is the single source of truth for the sidebar PR row. The // older row-with-badges implementation used a separate "hide status row for // terminal PRs" branch — the current row renders diff --git a/packages/views/issues/components/pull-request-list.test.tsx b/packages/views/issues/components/pull-request-list.test.tsx index 33874c8e23..f1719d3fe8 100644 --- a/packages/views/issues/components/pull-request-list.test.tsx +++ b/packages/views/issues/components/pull-request-list.test.tsx @@ -80,21 +80,11 @@ describe("PullRequestList sidebar rows", () => { expect(row).not.toHaveClass("rounded-lg", "border", "bg-card"); }); - it("renders checks-passed status when only passed counts are non-zero", async () => { + it("renders All-checks-passed status when only passed counts are non-zero", async () => { mockPRs = [makePR({ checks_passed: 3 })]; renderList(); await waitForRender(); - expect(screen.getByText("Checks passed")).toBeInTheDocument(); - }); - - // MUL-5180: the App holds Checks read-only, so GitHub delivers `completed` - // suites only — we can never know whether every suite has reported. The - // copy must therefore not claim completeness. - it("never claims that ALL checks passed", async () => { - mockPRs = [makePR({ checks_passed: 1 })]; - renderList(); - await waitForRender(); - expect(screen.queryByText(/all checks passed/i)).not.toBeInTheDocument(); + expect(screen.getByText("All checks passed")).toBeInTheDocument(); }); it("renders Some-checks-failed when any failed count is non-zero", async () => { @@ -161,67 +151,6 @@ describe("PullRequestList sidebar rows", () => { expect(screen.queryByText("Checks passed")).not.toBeInTheDocument(); }); - // MUL-5180: CI outcome used to render as plain muted text, visually - // indistinguishable from the diff stats next to it. The actionable kinds - // must carry their own color + icon; terminal/unknown kinds must not. - it("gives failing checks a colored status treatment", async () => { - mockPRs = [makePR({ checks_failed: 1, checks_passed: 5 })]; - renderList(); - await waitForRender(); - const status = screen.getByTestId("pull-request-status"); - expect(status).toHaveAttribute("data-status-kind", "checks_failed"); - expect(status).toHaveClass("text-rose-600"); - expect(status).toHaveTextContent("Some checks failed"); - }); - - it("gives pending and passing checks their own status treatment", async () => { - mockPRs = [makePR({ checks_pending: 2 })]; - const { unmount } = renderList(); - await waitForRender(); - expect(screen.getByTestId("pull-request-status")).toHaveClass("text-amber-600"); - unmount(); - - mockPRs = [makePR({ checks_passed: 2 })]; - renderList(); - await waitForRender(); - expect(screen.getByTestId("pull-request-status")).toHaveClass("text-emerald-600"); - }); - - it("gives merge conflicts a colored status treatment", async () => { - mockPRs = [makePR({ mergeable_state: "dirty" })]; - renderList(); - await waitForRender(); - const status = screen.getByTestId("pull-request-status"); - expect(status).toHaveAttribute("data-status-kind", "conflicts"); - expect(status).toHaveClass("text-rose-600"); - }); - - it("leaves terminal and unknown status kinds muted", async () => { - mockPRs = [makePR({ state: "merged", checks_failed: 3 })]; - const { unmount } = renderList(); - await waitForRender(); - expect(screen.queryByTestId("pull-request-status")).not.toBeInTheDocument(); - expect(screen.getByText("Merged")).toBeInTheDocument(); - unmount(); - - // No suite observed and no mergeable verdict — the state the card sits in - // when the GitHub App is not subscribed to `check_suite`. - mockPRs = [makePR()]; - renderList(); - await waitForRender(); - expect(screen.queryByTestId("pull-request-status")).not.toBeInTheDocument(); - expect(screen.getByText("Checks haven't reported yet")).toBeInTheDocument(); - }); - - it("keeps the draft prefix wrapped in the colored status treatment", async () => { - mockPRs = [makePR({ state: "draft", checks_failed: 1 })]; - renderList(); - await waitForRender(); - const status = screen.getByTestId("pull-request-status"); - expect(status).toHaveClass("text-rose-600"); - expect(status).toHaveTextContent("Draft · Some checks failed"); - }); - it("hides stats row when all stats are 0 (legacy backend)", async () => { mockPRs = [makePR()]; renderList(); diff --git a/packages/views/issues/components/pull-request-list.tsx b/packages/views/issues/components/pull-request-list.tsx index cbaf37f33e..e54be4fae2 100644 --- a/packages/views/issues/components/pull-request-list.tsx +++ b/packages/views/issues/components/pull-request-list.tsx @@ -54,27 +54,6 @@ const CHECKS_ICON: Record< pending: { icon: CircleDashed, className: "text-amber-600 dark:text-amber-400" }, }; -// Status line treatment per status kind. Only the *actionable* kinds get an -// icon and a color: CI outcome and merge conflicts are the signals a reader -// scans this row for, and rendering them in plain muted text made them -// indistinguishable from the neighbouring diff stats (MUL-5180). -// -// Terminal kinds (closed / merged) and `unknown` stay muted on purpose — the -// state icon at the head of the row already carries that meaning, so a second -// colored glyph would be noise. -const STATUS_KIND_STYLE: Partial< - Record< - PullRequestStatusKind, - { icon: React.ComponentType<{ className?: string }>; className: string } - > -> = { - checks_failed: { icon: XCircle, className: "text-rose-600 dark:text-rose-400" }, - checks_pending: { icon: CircleDashed, className: "text-amber-600 dark:text-amber-400" }, - checks_passed: { icon: CheckCircle2, className: "text-emerald-600 dark:text-emerald-400" }, - conflicts: { icon: TriangleAlert, className: "text-rose-600 dark:text-rose-400" }, - ready: { icon: CheckCircle2, className: "text-emerald-600 dark:text-emerald-400" }, -}; - export function PullRequestList({ issueId }: { issueId: string }) { const { t } = useT("issues"); const [expanded, setExpanded] = useState(false); @@ -217,35 +196,13 @@ function PullRequestRowDetails({
{showStats ? : null} - + {statusText} {showChecksBadge ? : null} {showConflictsBadge ? : null}
); } -function PullRequestStatusText({ - kind, - text, -}: { - kind: PullRequestStatusKind; - text: string; -}) { - const style = STATUS_KIND_STYLE[kind]; - if (!style) return {text}; - const Icon = style.icon; - return ( - - - {text} - - ); -} - function PullRequestStats({ pr }: { pr: GitHubPullRequest }) { const { t } = useT("issues"); return ( diff --git a/packages/views/locales/en/issues.json b/packages/views/locales/en/issues.json index 30266a65ce..a134dac313 100644 --- a/packages/views/locales/en/issues.json +++ b/packages/views/locales/en/issues.json @@ -262,7 +262,7 @@ "pull_request_card_status_conflicts": "Has merge conflicts", "pull_request_card_status_checks_failed": "Some checks failed", "pull_request_card_status_checks_pending": "Some checks haven't completed yet", - "pull_request_card_status_checks_passed": "Checks passed", + "pull_request_card_status_checks_passed": "All checks passed", "pull_request_card_status_ready": "Ready to merge", "pull_request_card_status_unknown": "Checks haven't reported yet", "pull_request_card_draft_prefix": "Draft · {{status}}", diff --git a/packages/views/locales/ja/issues.json b/packages/views/locales/ja/issues.json index dd94d94237..ee6db35179 100644 --- a/packages/views/locales/ja/issues.json +++ b/packages/views/locales/ja/issues.json @@ -260,7 +260,7 @@ "pull_request_card_status_conflicts": "マージコンフリクトあり", "pull_request_card_status_checks_failed": "一部のチェックが失敗しました", "pull_request_card_status_checks_pending": "一部のチェックがまだ完了していません", - "pull_request_card_status_checks_passed": "チェックに成功しました", + "pull_request_card_status_checks_passed": "すべてのチェックに成功しました", "pull_request_card_status_ready": "マージ可能", "pull_request_card_status_unknown": "まだチェック結果がありません", "pull_request_card_draft_prefix": "ドラフト · {{status}}", diff --git a/packages/views/locales/ko/issues.json b/packages/views/locales/ko/issues.json index a7f075602f..0f02cf480d 100644 --- a/packages/views/locales/ko/issues.json +++ b/packages/views/locales/ko/issues.json @@ -260,7 +260,7 @@ "pull_request_card_status_conflicts": "병합 충돌 있음", "pull_request_card_status_checks_failed": "일부 검사 실패", "pull_request_card_status_checks_pending": "일부 검사가 아직 끝나지 않았습니다", - "pull_request_card_status_checks_passed": "검사 통과", + "pull_request_card_status_checks_passed": "모든 검사 통과", "pull_request_card_status_ready": "병합 가능", "pull_request_card_status_unknown": "아직 검사 결과가 없습니다", "pull_request_card_draft_prefix": "초안 · {{status}}", diff --git a/packages/views/locales/zh-Hans/issues.json b/packages/views/locales/zh-Hans/issues.json index 2f25397503..24f5b84705 100644 --- a/packages/views/locales/zh-Hans/issues.json +++ b/packages/views/locales/zh-Hans/issues.json @@ -260,7 +260,7 @@ "pull_request_card_status_conflicts": "存在合并冲突", "pull_request_card_status_checks_failed": "部分检查失败", "pull_request_card_status_checks_pending": "部分检查仍在运行", - "pull_request_card_status_checks_passed": "检查通过", + "pull_request_card_status_checks_passed": "全部检查通过", "pull_request_card_status_ready": "可以合入", "pull_request_card_status_unknown": "暂无检查信息", "pull_request_card_draft_prefix": "Draft · {{status}}", diff --git a/server/internal/handler/github.go b/server/internal/handler/github.go index 5bf7ad7b2b..1f45430af7 100644 --- a/server/internal/handler/github.go +++ b/server/internal/handler/github.go @@ -183,15 +183,9 @@ func issuePullRequestRowToResponse(p db.ListPullRequestsByIssueRow) GitHubPullRe // aggregateChecksConclusion collapses the per-PR check_suite counts into a // single status surfaced to the UI: // - any failed-class suite wins ("failed"); -// - any indeterminate suite makes the PR "pending"; -// - the rest of the passed-class is "passed"; +// - any not-yet-completed suite makes the PR "pending"; +// - all completed and in the passed-class is "passed"; // - no observed suite at all is nil (rendered as "no checks" / hidden). -// -// "pending" is close to unreachable by design (MUL-5180): only `completed` -// suites are recorded and only `completed` suites are aggregated, so the -// pending bucket now catches just the degenerate case of a completed suite -// carrying a null conclusion. It is deliberately NOT a "CI is running" -// signal — that is not observable from this event at read-level access. func aggregateChecksConclusion(failed, passed, pending, total int64) *string { if total == 0 { return nil @@ -1031,8 +1025,12 @@ type ghCheckSuitePayload struct { } // handleCheckSuiteEvent records the CI suite state for each PR the suite -// references. Only the `completed` action is recorded — see the action gate -// below for why `requested` / `rerequested` must be ignored. +// references. We persist all non-terminal actions (`requested`, `rerequested`) +// as well as `completed`: a `requested`/`rerequested` event has status +// `queued`/`in_progress` and an empty conclusion, which the aggregation query +// counts as pending. Without persisting them, the per-PR `checks_pending` +// count stays at 0 while CI is mid-run and the PR card falls through to +// "checks not reported yet" until the first suite finishes. // // The suite payload may reference multiple PRs (e.g. the same head SHA is // open against several base branches), so we iterate. A reference whose PR @@ -1044,30 +1042,6 @@ func (h *Handler) handleCheckSuiteEvent(ctx context.Context, body []byte) { slog.Warn("github: bad check_suite payload", "err", err) return } - // Multica observes other apps' CI results; it is not itself a CI provider. - // - // `check_suite.requested` / `.rerequested` are NOT "some CI provider just - // started running". GitHub delivers them only to Apps holding Checks - // *write*, and their meaning is "GitHub has created a check suite FOR YOU - // on this commit — now add your check runs to it" - // (https://docs.github.com/en/apps/creating-github-apps/writing-code-for-a-github-app/building-ci-checks-with-a-github-app). - // - // Multica never creates or completes check runs, so recording such a suite - // would park a `queued` row that nothing can ever move to `completed`. The - // aggregation counts it as pending forever, and because `checks_pending` - // outranks `checks_passed` in derivePullRequestStatusKind, every PR row on - // that installation would freeze on "checks running" and mask the real - // pass/fail result. Self-hosters who already grant Checks write would hit - // this on every push, so the gate is on the action, not on the permission. - // - // Consequence, stated so it is not rediscovered as a bug: in-flight CI is - // not observable through this event at read level, so the PR card shows - // completed suites only. A genuine "running" signal needs a different - // mechanism (polling the checks API, or a check_run-based model) — do not - // resurrect `requested` as a substitute for it. - if p.Action != "completed" { - return - } if p.Installation.ID == 0 { return } @@ -1198,16 +1172,6 @@ func (h *Handler) replayPendingCheckSuitesForPR(ctx context.Context, pr db.Githu return } for _, row := range pending { - // Second gate, deliberately duplicated from handleCheckSuiteEvent. - // This is the other write path into github_pull_request_check_suite, - // and it does not pass through the webhook handler — a stash row - // written before that gate existed would otherwise be re-injected - // here on the next `pull_request` event, reintroducing exactly the - // permanently-`queued` suite the gate was added to prevent. The - // drain is a DELETE ... RETURNING, so skipping simply discards it. - if row.Status != "completed" { - continue - } if err := h.Queries.UpsertPullRequestCheckSuite(ctx, db.UpsertPullRequestCheckSuiteParams{ PrID: pr.ID, SuiteID: row.SuiteID, diff --git a/server/internal/handler/github_test.go b/server/internal/handler/github_test.go index 827740a96b..071ce4e7fd 100644 --- a/server/internal/handler/github_test.go +++ b/server/internal/handler/github_test.go @@ -1544,10 +1544,9 @@ func fireCheckSuiteWebhook(t *testing.T, secret string, installationID int64, re } // fireCheckSuiteWebhookWithStatus is the parametric form of -// fireCheckSuiteWebhook. It is what lets a test send an action the handler is -// expected to REJECT (`requested` / `rerequested`) as well as the `completed` -// action it accepts; the completed-only helper above wraps it for the common -// case. +// fireCheckSuiteWebhook. Tests covering the `requested`/`rerequested` and +// `queued`/`in_progress` matrix use it directly; the legacy completed-only +// helper above wraps it for existing call sites. func fireCheckSuiteWebhookWithStatus(t *testing.T, secret string, installationID int64, repo string, prNumbers []int32, suiteID, appID int64, headSHA, action, status, conclusion, updatedAt string) { t.Helper() prRefs := make([]map[string]any, 0, len(prNumbers)) @@ -1728,21 +1727,13 @@ func TestWebhook_CheckSuite_LateOlderEventIgnored(t *testing.T) { } } -// TestWebhook_CheckSuite_NonCompletedActionsIgnored pins the action gate in -// handleCheckSuiteEvent (MUL-5180). -// -// `check_suite.requested` / `.rerequested` do not mean "an external CI -// provider started running". GitHub sends them only to Apps holding Checks -// write, and they mean "GitHub created a suite for YOU — add your check runs -// to it". Multica never creates check runs, so persisting such a suite parks -// a `queued` row that can never complete. Since `checks_pending` outranks -// `checks_passed` in derivePullRequestStatusKind, that one stuck row would -// freeze every PR on this installation at "checks running" and hide the real -// pass/fail result. -// -// The gate must therefore drop them even though the payload looks well-formed, -// and a later `completed` suite must still land normally. -func TestWebhook_CheckSuite_NonCompletedActionsIgnored(t *testing.T) { +// TestWebhook_CheckSuite_QueuedCountsAsPending covers the "CI 跑到一半" path: +// GitHub fires `check_suite.requested` with status `queued` and an empty +// conclusion while CI is still spinning up. The handler must persist these +// non-terminal events so the per-PR `checks_pending` count reflects work in +// progress; otherwise the frontend falls through to the "checks not +// reported yet" placeholder until the first completed suite arrives. +func TestWebhook_CheckSuite_QueuedCountsAsPending(t *testing.T) { if testHandler == nil { t.Skip("handler test fixture not initialized (no DB?)") } @@ -1752,11 +1743,10 @@ func TestWebhook_CheckSuite_NonCompletedActionsIgnored(t *testing.T) { head := "pending1234567" firePullRequestWebhookWithHead(t, secret, created.Identifier, installationID, "ci-repo-pending", 55, "opened", head, "") - - // The shape a Checks-write App receives on every push: GitHub opened a - // suite addressed to us. Nothing about it is an observation of other CI. + // CI just kicked off — `requested` action, status=queued, no conclusion. fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-pending", []int32{55}, 4001, 6001, head, "requested", "queued", "", "2026-05-01T00:00:00Z") - fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-pending", []int32{55}, 4002, 6002, head, "rerequested", "in_progress", "", "2026-05-01T00:00:30Z") + // A second app's suite starts a moment later with status=in_progress. + fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-pending", []int32{55}, 4002, 6002, head, "requested", "in_progress", "", "2026-05-01T00:00:30Z") rows, err := testHandler.Queries.ListPullRequestsByIssue(ctx, parseUUID(created.ID)) if err != nil { @@ -1765,179 +1755,27 @@ func TestWebhook_CheckSuite_NonCompletedActionsIgnored(t *testing.T) { if len(rows) != 1 { t.Fatalf("expected 1 PR row, got %d", len(rows)) } - if rows[0].ChecksTotal != 0 || rows[0].ChecksPending != 0 { - t.Fatalf("non-completed actions must not be recorded, got total=%d pending=%d", - rows[0].ChecksTotal, rows[0].ChecksPending) + if rows[0].ChecksPending != 2 || rows[0].ChecksTotal != 2 || + rows[0].ChecksFailed != 0 || rows[0].ChecksPassed != 0 { + t.Fatalf("expected pending=2 total=2 failed=0 passed=0, got pending=%d total=%d failed=%d passed=%d", + rows[0].ChecksPending, rows[0].ChecksTotal, rows[0].ChecksFailed, rows[0].ChecksPassed) } - if got := aggregateChecksConclusion(rows[0].ChecksFailed, rows[0].ChecksPassed, rows[0].ChecksPending, rows[0].ChecksTotal); got != nil { - t.Errorf("expected no aggregate verdict from non-completed actions, got %v", *got) + got := aggregateChecksConclusion(rows[0].ChecksFailed, rows[0].ChecksPassed, rows[0].ChecksPending, rows[0].ChecksTotal) + if got == nil || *got != "pending" { + t.Errorf("expected aggregate pending while CI is running, got %v", got) } - // A real completed suite still records and reports normally. + // Now one app completes successfully — pending count drops to 1 and the + // aggregate stays pending until the second app finishes. fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-pending", []int32{55}, 4001, 6001, head, "completed", "completed", "success", "2026-05-01T00:05:00Z") rows, err = testHandler.Queries.ListPullRequestsByIssue(ctx, parseUUID(created.ID)) if err != nil { t.Fatalf("ListPullRequestsByIssue: %v", err) } - if rows[0].ChecksPassed != 1 || rows[0].ChecksTotal != 1 || rows[0].ChecksPending != 0 { - t.Fatalf("expected passed=1 total=1 pending=0 after the completed suite, got passed=%d total=%d pending=%d", - rows[0].ChecksPassed, rows[0].ChecksTotal, rows[0].ChecksPending) - } - got := aggregateChecksConclusion(rows[0].ChecksFailed, rows[0].ChecksPassed, rows[0].ChecksPending, rows[0].ChecksTotal) - if got == nil || *got != "passed" { - t.Errorf("expected aggregate passed, got %v", got) - } -} - -// TestCheckSuite_LegacyNonCompletedRowsExcludedFromAggregate covers the -// upgrade path for a deployment that ran BEFORE the action gate existed -// (MUL-5180). -// -// An installation holding Checks write received `check_suite.requested` for -// suites GitHub had opened for Multica itself. The old handler stored them as -// `queued`, and nothing will ever complete them. If the aggregate still -// counted those rows, `checks_pending` would outrank `checks_passed` and the -// PR would stay pinned to "checks running" after the upgrade — for as long as -// the head SHA stands — which, on a long-lived PR nobody pushes to again, -// is indefinitely. -// -// The row is inserted directly because the fixed handler can no longer -// produce one; that is exactly the point of the test. -func TestCheckSuite_LegacyNonCompletedRowsExcludedFromAggregate(t *testing.T) { - if testHandler == nil { - t.Skip("handler test fixture not initialized (no DB?)") - } - ctx := context.Background() - const secret = "ci-legacy-agg-secret" - created, installationID := setupPRTestIssue(t, ctx, secret) - - head := "legacy1234567" - firePullRequestWebhookWithHead(t, secret, created.Identifier, installationID, "ci-repo-legacy", 88, "opened", head, "") - // A real external suite completes green. - fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-legacy", []int32{88}, 9001, 9501, head, "completed", "completed", "success", "2026-05-01T00:05:00Z") - - rows, err := testHandler.Queries.ListPullRequestsByIssue(ctx, parseUUID(created.ID)) - if err != nil { - t.Fatalf("ListPullRequestsByIssue: %v", err) - } - if len(rows) != 1 { - t.Fatalf("expected 1 PR row, got %d", len(rows)) - } - - // Simulate the pre-upgrade leftovers: a stuck `queued` suite from a - // different app, and a newer stuck `in_progress` suite from the SAME app - // as the green one (which would shadow it without the filter). - if _, err := testPool.Exec(ctx, - `INSERT INTO github_pull_request_check_suite - (pr_id, suite_id, head_sha, app_id, conclusion, status, updated_at) - VALUES ($1, 9101, $2, 9601, NULL, 'queued', '2026-05-01T00:00:00Z'), - ($1, 9102, $2, 9501, NULL, 'in_progress', '2026-05-01T00:09:00Z')`, - rows[0].ID, head); err != nil { - t.Fatalf("insert legacy rows: %v", err) - } - - rows, err = testHandler.Queries.ListPullRequestsByIssue(ctx, parseUUID(created.ID)) - if err != nil { - t.Fatalf("ListPullRequestsByIssue: %v", err) - } - if rows[0].ChecksPending != 0 || rows[0].ChecksPassed != 1 || rows[0].ChecksTotal != 1 { - t.Fatalf("legacy non-completed rows must not reach the aggregate, got pending=%d passed=%d total=%d", + if rows[0].ChecksPending != 1 || rows[0].ChecksPassed != 1 || rows[0].ChecksTotal != 2 { + t.Fatalf("expected pending=1 passed=1 total=2 after one suite completes, got pending=%d passed=%d total=%d", rows[0].ChecksPending, rows[0].ChecksPassed, rows[0].ChecksTotal) } - got := aggregateChecksConclusion(rows[0].ChecksFailed, rows[0].ChecksPassed, rows[0].ChecksPending, rows[0].ChecksTotal) - if got == nil || *got != "passed" { - t.Errorf("expected the card to recover to passed, got %v", got) - } -} - -// TestCheckSuite_LegacyStashRowNotReplayed covers the second pre-upgrade -// leftover (MUL-5180): a non-completed row sitting in the -// `github_pending_check_suite` stash. -// -// replayPendingCheckSuitesForPR is a write path into the live suite table -// that does NOT go through handleCheckSuiteEvent, so without its own gate the -// next `pull_request` webhook would re-inject a permanently-`queued` suite -// after the fix had shipped. -func TestCheckSuite_LegacyStashRowNotReplayed(t *testing.T) { - if testHandler == nil { - t.Skip("handler test fixture not initialized (no DB?)") - } - ctx := context.Background() - const secret = "ci-legacy-stash-secret" - created, installationID := setupPRTestIssue(t, ctx, secret) - - head := "legacystash12" - // Pre-upgrade stash content: the PR row did not exist yet when a - // `requested` suite arrived, so the old handler parked it here. The - // owner MUST match what firePullRequestWebhookWithHead sends ("acme") — - // DrainPendingCheckSuitesForPR keys on - // (workspace_id, repo_owner, repo_name, pr_number), so a mismatched owner - // silently drains nothing and makes this test vacuous. - if _, err := testPool.Exec(ctx, - `INSERT INTO github_pending_check_suite - (workspace_id, installation_id, repo_owner, repo_name, pr_number, - suite_id, head_sha, app_id, conclusion, status, suite_updated_at) - VALUES ($1, $2, 'acme', 'ci-repo-legacy-stash', 99, - 9201, $3, 9701, NULL, 'queued', '2026-05-01T00:00:00Z')`, - testWorkspaceID, installationID, head); err != nil { - t.Fatalf("insert legacy stash row: %v", err) - } - - // Sanity-check the fixture actually addresses this PR, so a future - // rename cannot turn the assertions below into a no-op. - var stashed int - if err := testPool.QueryRow(ctx, - `SELECT count(*) FROM github_pending_check_suite - WHERE workspace_id = $1 AND repo_owner = 'acme' - AND repo_name = 'ci-repo-legacy-stash' AND pr_number = 99`, - testWorkspaceID).Scan(&stashed); err != nil { - t.Fatalf("count stash: %v", err) - } - if stashed != 1 { - t.Fatalf("fixture did not land in the stash, got %d rows", stashed) - } - - // The PR webhook arrives and drains the stash. - firePullRequestWebhookWithHead(t, secret, created.Identifier, installationID, "ci-repo-legacy-stash", 99, "opened", head, "") - - rows, err := testHandler.Queries.ListPullRequestsByIssue(ctx, parseUUID(created.ID)) - if err != nil { - t.Fatalf("ListPullRequestsByIssue: %v", err) - } - if len(rows) != 1 { - t.Fatalf("expected 1 PR row, got %d", len(rows)) - } - if rows[0].ChecksTotal != 0 || rows[0].ChecksPending != 0 { - t.Fatalf("legacy stash row must not be replayed, got total=%d pending=%d", - rows[0].ChecksTotal, rows[0].ChecksPending) - } - - // It must be gone from the live table too, not merely uncounted. - var live int - if err := testPool.QueryRow(ctx, - `SELECT count(*) FROM github_pull_request_check_suite WHERE pr_id = $1`, - rows[0].ID).Scan(&live); err != nil { - t.Fatalf("count live suites: %v", err) - } - if live != 0 { - t.Errorf("expected the skipped stash row not to be written, got %d live rows", live) - } - - // The stash must be empty afterwards. Together with the pre-count above - // this proves the drain actually ran on THIS row rather than missing it: - // if a future change to firePullRequestWebhookWithHead moved the repo - // address, the row would still be sitting here and the assertions above - // would be passing for the wrong reason. - if err := testPool.QueryRow(ctx, - `SELECT count(*) FROM github_pending_check_suite - WHERE workspace_id = $1 AND repo_owner = 'acme' - AND repo_name = 'ci-repo-legacy-stash' AND pr_number = 99`, - testWorkspaceID).Scan(&stashed); err != nil { - t.Fatalf("count stash after drain: %v", err) - } - if stashed != 0 { - t.Errorf("expected the drain to consume the stash row, got %d rows left", stashed) - } } // TestWebhook_CheckSuite_OutOfOrderReplaysOnPRUpsert covers the out-of-order @@ -1945,8 +1783,8 @@ func TestCheckSuite_LegacyStashRowNotReplayed(t *testing.T) { // row has been mirrored locally (e.g. webhook reordering, or the PR was // linked to an installation that was suspended/resumed). The handler must // stash the suite and replay it when the PR upsert arrives, otherwise the -// PR's first observed suite is silently lost and the card reports nothing -// until the next suite ships. +// PR's first observed suite is silently lost and `checks_pending` stays at +// 0 until the next suite ships. func TestWebhook_CheckSuite_OutOfOrderReplaysOnPRUpsert(t *testing.T) { if testHandler == nil { t.Skip("handler test fixture not initialized (no DB?)") @@ -1956,10 +1794,8 @@ func TestWebhook_CheckSuite_OutOfOrderReplaysOnPRUpsert(t *testing.T) { created, installationID := setupPRTestIssue(t, ctx, secret) head := "oo01234567890" - // Suite event lands FIRST — the PR row does not exist yet. Only - // `completed` actions are recorded at all (see the action gate in - // handleCheckSuiteEvent), so the stash path is exercised with one. - fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-ooo", []int32{66}, 5001, 7501, head, "completed", "completed", "failure", "2026-05-01T00:00:00Z") + // Suite event lands FIRST — the PR row does not exist yet. + fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-ooo", []int32{66}, 5001, 7501, head, "requested", "in_progress", "", "2026-05-01T00:00:00Z") // Verify nothing landed on the PR table yet (no PR row to land on). if rows, err := testHandler.Queries.ListPullRequestsByIssue(ctx, parseUUID(created.ID)); err != nil { @@ -1979,9 +1815,9 @@ func TestWebhook_CheckSuite_OutOfOrderReplaysOnPRUpsert(t *testing.T) { if len(rows) != 1 { t.Fatalf("expected 1 PR row after PR webhook, got %d", len(rows)) } - if rows[0].ChecksFailed != 1 || rows[0].ChecksTotal != 1 { - t.Fatalf("expected failed=1 total=1 after replay, got failed=%d total=%d", - rows[0].ChecksFailed, rows[0].ChecksTotal) + if rows[0].ChecksPending != 1 || rows[0].ChecksTotal != 1 { + t.Fatalf("expected pending=1 total=1 after replay, got pending=%d total=%d", + rows[0].ChecksPending, rows[0].ChecksTotal) } // The next PR upsert (a no-op metadata edit) must NOT re-apply or fail @@ -1992,9 +1828,9 @@ func TestWebhook_CheckSuite_OutOfOrderReplaysOnPRUpsert(t *testing.T) { if err != nil { t.Fatalf("ListPullRequestsByIssue: %v", err) } - if rows[0].ChecksFailed != 1 || rows[0].ChecksTotal != 1 { - t.Fatalf("expected failed=1 total=1 after no-op edit, got failed=%d total=%d", - rows[0].ChecksFailed, rows[0].ChecksTotal) + if rows[0].ChecksPending != 1 || rows[0].ChecksTotal != 1 { + t.Fatalf("expected pending=1 total=1 after no-op edit, got pending=%d total=%d", + rows[0].ChecksPending, rows[0].ChecksTotal) } } @@ -2003,10 +1839,10 @@ func TestWebhook_CheckSuite_OutOfOrderReplaysOnPRUpsert(t *testing.T) { // handles: while the PR row is still missing, an older event for the // same suite_id must not overwrite a newer payload that was stashed // first. Without the suite_updated_at guard on UpsertPendingCheckSuite, -// a late `completed/failure` arriving after a newer `completed/success` -// would roll the stash back to the stale verdict; the subsequent PR upsert -// would then replay it and the PR card would report a failure that the -// suite had already superseded. +// a late `requested/in_progress` arriving after `completed/success` +// would roll the stash back to pending; the subsequent PR upsert would +// then replay the stale state and the PR card would stay stuck on +// "pending" until the next suite shipped. func TestWebhook_CheckSuite_OutOfOrderStashKeepsNewer(t *testing.T) { if testHandler == nil { t.Skip("handler test fixture not initialized (no DB?)") @@ -2020,7 +1856,7 @@ func TestWebhook_CheckSuite_OutOfOrderStashKeepsNewer(t *testing.T) { fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-stash", []int32{77}, 6001, 8001, head, "completed", "completed", "success", "2026-05-01T00:05:00Z") // Older event for the SAME suite arrives later (webhook reorder). The // pending stash must keep the newer payload. - fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-stash", []int32{77}, 6001, 8001, head, "completed", "completed", "failure", "2026-05-01T00:00:00Z") + fireCheckSuiteWebhookWithStatus(t, secret, installationID, "ci-repo-stash", []int32{77}, 6001, 8001, head, "requested", "in_progress", "", "2026-05-01T00:00:00Z") // PR webhook arrives — drain replays the (still newer) stash. firePullRequestWebhookWithHead(t, secret, created.Identifier, installationID, "ci-repo-stash", 77, "opened", head, "") diff --git a/server/pkg/db/generated/github.sql.go b/server/pkg/db/generated/github.sql.go index 77d0ce0dbc..e68786b4d8 100644 --- a/server/pkg/db/generated/github.sql.go +++ b/server/pkg/db/generated/github.sql.go @@ -496,7 +496,6 @@ per_app_latest AS ( FROM github_pull_request_check_suite cs JOIN issue_prs ip ON ip.id = cs.pr_id WHERE cs.head_sha = ip.head_sha AND ip.head_sha <> '' - AND cs.status = 'completed' ORDER BY cs.pr_id, cs.app_id, cs.updated_at DESC ), checks AS ( @@ -573,19 +572,6 @@ type ListPullRequestsByIssueRow struct { // pending view. reference_only links (a PR that merely mentions the issue // identifier in its body, with no closing keyword and no title/branch // reference) are filtered out — they are not working PRs for this issue. -// Only `completed` suites are eligible. The webhook handler already refuses -// to record anything else (MUL-5180), but this filter is what makes the card -// correct for rows that predate that gate: an installation holding Checks -// write accumulated `queued` suites GitHub had opened for Multica itself, -// and nothing will ever move them to `completed`. Counting them keeps the PR -// pinned to "checks running" — `checks_pending` outranks `checks_passed` in -// derivePullRequestStatusKind — for as long as the head SHA stands. -// -// Filtering here rather than deleting the rows makes the recovery automatic -// and source-agnostic: it holds for legacy rows, for stash rows replayed by -// replayPendingCheckSuitesForPR, and for any future writer that forgets the -// gate. The DISTINCT ON runs after the filter, so an app whose newest suite -// is a stuck `queued` still reports its most recent completed verdict. func (q *Queries) ListPullRequestsByIssue(ctx context.Context, issueID pgtype.UUID) ([]ListPullRequestsByIssueRow, error) { rows, err := q.db.Query(ctx, listPullRequestsByIssue, issueID) if err != nil { diff --git a/server/pkg/db/queries/github.sql b/server/pkg/db/queries/github.sql index 9bdfb3e058..0b3fda2a41 100644 --- a/server/pkg/db/queries/github.sql +++ b/server/pkg/db/queries/github.sql @@ -147,26 +147,12 @@ WITH issue_prs AS ( JOIN issue_pull_request ipr ON ipr.pull_request_id = pr.id WHERE ipr.issue_id = sqlc.arg('issue_id') AND NOT ipr.reference_only ), --- Only `completed` suites are eligible. The webhook handler already refuses --- to record anything else (MUL-5180), but this filter is what makes the card --- correct for rows that predate that gate: an installation holding Checks --- write accumulated `queued` suites GitHub had opened for Multica itself, --- and nothing will ever move them to `completed`. Counting them keeps the PR --- pinned to "checks running" — `checks_pending` outranks `checks_passed` in --- derivePullRequestStatusKind — for as long as the head SHA stands. --- --- Filtering here rather than deleting the rows makes the recovery automatic --- and source-agnostic: it holds for legacy rows, for stash rows replayed by --- replayPendingCheckSuitesForPR, and for any future writer that forgets the --- gate. The DISTINCT ON runs after the filter, so an app whose newest suite --- is a stuck `queued` still reports its most recent completed verdict. per_app_latest AS ( SELECT DISTINCT ON (cs.pr_id, cs.app_id) cs.pr_id, cs.app_id, cs.conclusion, cs.status FROM github_pull_request_check_suite cs JOIN issue_prs ip ON ip.id = cs.pr_id WHERE cs.head_sha = ip.head_sha AND ip.head_sha <> '' - AND cs.status = 'completed' ORDER BY cs.pr_id, cs.app_id, cs.updated_at DESC ), checks AS (