From 835ea34e848bdb1685d825aa8ce3d240789d22ce Mon Sep 17 00:00:00 2001 From: zfy0701 <1646270+zfy0701@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:49:01 +0800 Subject: [PATCH 1/3] feat(web): one Sync for every GitHub App and a plainer GitHub card The card's Sync now re-pulls the built-in App's installations and each organization App's, reporting a failure without stopping the rest, so an organization App's menu keeps only Delete and appears on a free App alone. Every App row gets the same Install on GitHub button, and the column header and installation counts are gone. Co-Authored-By: Claude Opus 5.5 --- packages/web/messages/en.json | 6 +- packages/web/messages/zh-CN.json | 6 +- .../components/console/GithubCard.test.tsx | 100 +++++++++++++ .../web/src/components/console/GithubCard.tsx | 48 +++---- .../components/console/GithubOrgApps.test.tsx | 24 ++-- .../src/components/console/GithubOrgApps.tsx | 136 +++++++----------- 6 files changed, 182 insertions(+), 138 deletions(-) create mode 100644 packages/web/src/components/console/GithubCard.test.tsx diff --git a/packages/web/messages/en.json b/packages/web/messages/en.json index 5d723d490..4cddc9aef 100644 --- a/packages/web/messages/en.json +++ b/packages/web/messages/en.json @@ -1766,7 +1766,6 @@ "free": "free", "usedBy": "used by {agent}", "inUse": "in use", - "notInstalled": "not installed", "more": "More", "delete": "Delete App…", "deleteTitle": "Delete GitHub App", @@ -1777,12 +1776,9 @@ "deleteConfirm": "Delete", "noInstallations": "Not installed on any account yet." }, - "appColumn": "App", - "installationsColumn": "Installations", "builtIn": "built-in", "expand": "Show installations", - "collapse": "Hide installations", - "installationCount": "{count, plural, one {# installation} other {# installations}}" + "collapse": "Hide installations" }, "codeHostActions": { "addRepository": "Add repository", diff --git a/packages/web/messages/zh-CN.json b/packages/web/messages/zh-CN.json index 8bcccaa3c..f0c5e1eb9 100644 --- a/packages/web/messages/zh-CN.json +++ b/packages/web/messages/zh-CN.json @@ -1766,7 +1766,6 @@ "free": "空闲", "usedBy": "{agent} 在用", "inUse": "使用中", - "notInstalled": "未安装", "more": "更多", "delete": "删除 App…", "deleteTitle": "删除 GitHub App", @@ -1777,12 +1776,9 @@ "deleteConfirm": "删除", "noInstallations": "还没有安装到任何账号。" }, - "appColumn": "App", - "installationsColumn": "安装", "builtIn": "内置", "expand": "展开安装列表", - "collapse": "收起安装列表", - "installationCount": "{count, plural, other {# 个安装}}" + "collapse": "收起安装列表" }, "codeHostActions": { "addRepository": "添加仓库", diff --git a/packages/web/src/components/console/GithubCard.test.tsx b/packages/web/src/components/console/GithubCard.test.tsx new file mode 100644 index 000000000..44c902c43 --- /dev/null +++ b/packages/web/src/components/console/GithubCard.test.tsx @@ -0,0 +1,100 @@ +// @vitest-environment happy-dom +/** The GitHub card's one Sync re-pulls every App's installations, the built-in App's and each organization App's. */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { SWRConfig } from 'swr' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { GithubInstallationDto, GithubOrgAppDto } from '@/lib/api' + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }) + +const installation = (over: Partial): GithubInstallationDto => ({ + id: 'inst-1', + installationId: 42, + accountLogin: 'example-org', + accountType: 'Organization', + repositorySelection: 'all', + suspended: false, + permissionsStatus: 'current', + pullRequestsPermission: 'write', + checksPermission: 'write', + settingsUrl: 'https://github.example.test/settings/installations/42', + createdAt: '2026-10-01T00:00:00.000Z', + ...over +}) + +const orgApp = (id: string, slug: string): GithubOrgAppDto => ({ + id, + appId: 2001, + slug, + botUserId: 9001, + agentId: null, + installations: [], + createdAt: '2026-10-20T00:00:00.000Z' +}) + +const mocks = vi.hoisted(() => ({ + syncGithubInstallations: vi.fn(), + syncGithubApp: vi.fn() +})) + +// The real hook returns a stable org; a fresh object per render would re-run the card's fetch forever. +const orgs = vi.hoisted(() => ({ activeOrg: { id: 'org-1' } })) +vi.mock('@/lib/org-context', () => ({ useOrgs: () => orgs })) +vi.mock('@/lib/api', async (importOriginal) => ({ + ...(await importOriginal()), + fetchGithubInstallations: vi.fn(async () => ({ enabled: true, installations: [installation({})] })), + fetchBuiltinGithubAppSlug: vi.fn(async () => 'example-connect'), + fetchGithubApps: vi.fn(async () => [orgApp('app-a', 'review-example'), orgApp('app-b', 'triage-example')]), + syncGithubInstallations: mocks.syncGithubInstallations, + syncGithubApp: mocks.syncGithubApp +})) + +const GithubCard = (await import('./GithubCard')).default + +let root: Root | undefined +let host: HTMLDivElement | undefined + +afterEach(async () => { + if (root) await act(async () => root?.unmount()) + host?.remove() + root = undefined + host = undefined + mocks.syncGithubInstallations.mockReset() + mocks.syncGithubApp.mockReset() +}) + +async function render() { + host = document.createElement('div') + document.body.append(host) + root = createRoot(host) + await act(async () => { + root?.render( + new Map() }}> + + + ) + }) + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))) + return host +} + +describe('GithubCard', () => { + it('syncs the built-in App and every organization App from one Sync, reporting a failure without stopping the rest', async () => { + mocks.syncGithubInstallations.mockResolvedValue([installation({ id: 'inst-2', accountLogin: 'second-org' })]) + mocks.syncGithubApp.mockImplementation(async (id: string) => { + if (id === 'app-b') throw new Error('the private key of this App is missing') + }) + const scope = await render() + // No column header row: each App row speaks for itself. + expect(scope.textContent).not.toContain('Installations') + + const sync = [...scope.querySelectorAll('button')].find((b) => b.textContent?.trim() === 'Sync') + await act(async () => sync!.click()) + + expect(mocks.syncGithubInstallations).toHaveBeenCalledTimes(1) + expect(mocks.syncGithubApp.mock.calls.map(([id]) => id)).toEqual(['app-a', 'app-b']) + expect(scope.textContent).toContain('second-org') + expect(scope.textContent).toContain('the private key of this App is missing') + }) +}) diff --git a/packages/web/src/components/console/GithubCard.tsx b/packages/web/src/components/console/GithubCard.tsx index 64f08536d..07cd6d1a0 100644 --- a/packages/web/src/components/console/GithubCard.tsx +++ b/packages/web/src/components/console/GithubCard.tsx @@ -1,5 +1,5 @@ import { Fragment, useEffect, useState } from 'react' -import useSWR from 'swr' +import useSWR, { useSWRConfig } from 'swr' import { useTranslations } from 'next-intl' import { Button, Icon } from '@/components/ui' import { SQUARE_MARK_FILL_PCT } from '@/components/mark-box' @@ -7,8 +7,10 @@ import { GithubMark, LoadingState } from '@/components/marks' import { useOrgs } from '@/lib/org-context' import { fetchBuiltinGithubAppSlug, + fetchGithubApps, fetchGithubInstallUrl, fetchGithubInstallations, + syncGithubApp, syncGithubInstallations, type GithubInstallationDto } from '@/lib/api' @@ -48,6 +50,7 @@ export default function GithubCard({ const [uninstalling, setUninstalling] = useState(null) // Each App is a first-level row over its installations; the built-in App opens expanded. const [builtInOpen, setBuiltInOpen] = useState(true) + const { mutate } = useSWRConfig() const { data: builtInSlug } = useSWR(activeOrg ? consoleKeys.githubBuiltinApp(activeOrg.id) : null, ([, id]) => fetchBuiltinGithubAppSlug(id ?? undefined) ) @@ -80,14 +83,22 @@ export default function GithubCard({ } } - // Refresh the org's claimed installations after GitHub changes or an install - // finished in the other tab just now. + // Re-pull every App's installations from GitHub, the built-in App's and each organization App's; one failing App doesn't stop the rest. const sync = async () => { - if (busy) return + if (busy || !activeOrg) return setBusy(true) setErr(null) try { - setInstalls(await syncGithubInstallations()) + const [builtIn, ...orgApps] = await Promise.allSettled([ + syncGithubInstallations(), + ...(await fetchGithubApps(activeOrg.id)).map((app) => syncGithubApp(app.id)) + ]) + if (builtIn?.status === 'fulfilled') setInstalls(builtIn.value) + await mutate(consoleKeys.githubApps(activeOrg.id)) + const failed = [builtIn, ...orgApps].find((result) => result?.status === 'rejected') + if (failed?.status === 'rejected') { + setErr(failed.reason instanceof Error ? failed.reason.message : String(failed.reason)) + } } catch (e) { setErr(e instanceof Error ? e.message : String(e)) } finally { @@ -118,14 +129,6 @@ export default function GithubCard({ {t('github.notEnabled')} )} - {/* Desktop only: the row collapses to one stacked column below the - breakpoint, where a two-track header would label nothing. */} - {enabled === true && ( -
- {t('github.appColumn')} - {t('github.installationsColumn')} -
- )} {enabled === true && (
setBuiltInOpen((open) => !open)} label={builtInOpen ? t('github.collapse') : t('github.expand')} aside={ - <> - - {installs.length > 0 - ? t('github.installationCount', { count: installs.length }) - : t('github.orgApps.notInstalled')} - - {canWrite && ( - - )} - + canWrite && ( + + ) } > {builtInSlug ?? t('github.label')} diff --git a/packages/web/src/components/console/GithubOrgApps.test.tsx b/packages/web/src/components/console/GithubOrgApps.test.tsx index 23ca1d12e..5d3d288dc 100644 --- a/packages/web/src/components/console/GithubOrgApps.test.tsx +++ b/packages/web/src/components/console/GithubOrgApps.test.tsx @@ -10,7 +10,6 @@ Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }) const mocks = vi.hoisted(() => ({ apps: [] as GithubOrgAppDto[], - syncGithubApp: vi.fn(async () => undefined), deleteGithubApp: vi.fn(async () => undefined) })) @@ -18,7 +17,6 @@ vi.mock('@/lib/org-context', () => ({ useOrgs: () => ({ activeOrg: { id: 'org-1' vi.mock('@/lib/api', async (importOriginal) => ({ ...(await importOriginal()), fetchGithubApps: vi.fn(async () => mocks.apps), - syncGithubApp: mocks.syncGithubApp, deleteGithubApp: mocks.deleteGithubApp })) @@ -72,7 +70,6 @@ afterEach(async () => { host?.remove() root = undefined host = undefined - mocks.syncGithubApp.mockClear() mocks.deleteGithubApp.mockClear() }) @@ -106,30 +103,27 @@ describe('GithubOrgApps', () => { const used = row('review-private')! expect(used.textContent).toContain('used by Reviewer') - expect(used.textContent).toContain('1 installation') expect(used.querySelector('a.mono')?.getAttribute('href')).toBe('https://github.com/apps/review-private') // Installations are the second level: collapsed until the App row is expanded. expect(used.textContent).not.toContain('example-org') await expand('review-private') expect(used.textContent).toContain('example-org') expect(used.textContent).toContain('all repositories') - // An App an agent uses can be synced but never deleted. - expect(await menuItems('review-private')).toEqual(['Sync']) + // An App an agent uses can't be deleted, so its row has no menu. + expect(used.querySelector('button[aria-label="More"]')).toBeNull() + expect(used.textContent).toContain('Install on GitHub') const free = row('triage-example')! expect(free.textContent).toContain('free') - expect(free.textContent).toContain('not installed') + await expand('triage-example') + expect(free.textContent).toContain('Not installed on any account yet.') }) - it('syncs an App from its menu, and deletes a free one after confirming', async () => { + it('deletes a free App from its menu after confirming', async () => { mocks.apps = [app({ id: 'app-free', slug: 'triage-example' })] await render() - expect(await menuItems('triage-example')).toEqual(['Sync', 'Delete App…']) - await act(async () => menuItem('Sync')!.click()) - expect(mocks.syncGithubApp).toHaveBeenCalledWith('app-free') - - await menuItems('triage-example') + expect(await menuItems('triage-example')).toEqual(['Delete App…']) await act(async () => menuItem('Delete App')!.click()) const confirm = [...document.querySelectorAll('button')].find( (b) => b.textContent?.trim() === 'Delete' @@ -164,11 +158,11 @@ describe('GithubOrgApps', () => { expect(paused.textContent).not.toContain('not installed') }) - it('lets a viewer expand an App but offers no install, sync or delete', async () => { + it('lets a viewer expand an App but offers no install or delete', async () => { mocks.apps = [app({ id: 'app-free', slug: 'triage-example' })] await render({ canWrite: false, isOwner: false }) const scope = row('triage-example')! - expect(scope.querySelector('a[aria-label="Install on GitHub"]')).toBeNull() + expect(scope.textContent).not.toContain('Install on GitHub') expect(scope.querySelector('button[aria-label="More"]')).toBeNull() expect(scope.querySelector('button[aria-expanded]')).not.toBeNull() }) diff --git a/packages/web/src/components/console/GithubOrgApps.tsx b/packages/web/src/components/console/GithubOrgApps.tsx index 619a60fc4..d4df11426 100644 --- a/packages/web/src/components/console/GithubOrgApps.tsx +++ b/packages/web/src/components/console/GithubOrgApps.tsx @@ -6,7 +6,7 @@ import { AnchoredFlyout } from '@/components/ui/AnchoredFlyout' import { SQUARE_MARK_FILL_PCT } from '@/components/mark-box' import { GithubMark } from '@/components/marks' import { Scrim } from '@/components/console/Scrim' -import { deleteGithubApp, fetchGithubApps, githubAppPageUrl, syncGithubApp, type GithubOrgAppDto } from '@/lib/api' +import { deleteGithubApp, fetchGithubApps, githubAppPageUrl, type GithubOrgAppDto } from '@/lib/api' import { useOrgs } from '@/lib/org-context' import { consoleKeys } from '@/lib/swr-keys' @@ -77,8 +77,6 @@ export function GithubOrgApps({ fetchGithubApps(id ?? undefined) ) const [expanded, setExpanded] = useState>(new Set()) - const [syncing, setSyncing] = useState(null) - const [err, setErr] = useState(null) const [deleting, setDeleting] = useState(null) if (!apps || apps.length === 0) return null @@ -90,20 +88,6 @@ export function GithubOrgApps({ return next }) - const sync = async (app: GithubOrgAppDto) => { - if (syncing) return - setSyncing(app.id) - setErr(null) - try { - await syncGithubApp(app.id) - await mutate() - } catch (e) { - setErr(e instanceof Error ? e.message : String(e)) - } finally { - setSyncing(null) - } - } - return ( <> {apps.map((app) => { @@ -117,74 +101,55 @@ export function GithubOrgApps({ label={open ? t('collapse') : t('expand')} aside={ <> - - {app.installations.length > 0 - ? t('installationCount', { count: app.installations.length }) - : t('orgApps.notInstalled')} - {canWrite && ( - - - - - ( - - )} - > - {({ close }) => ( - <> - - {isOwner && app.agentId === null && ( - - )} - - )} - - + + )} + {/* Only a free App can be deleted, so only its row has a menu. */} + {isOwner && app.agentId === null && ( + ( + + )} + > + {({ close }) => ( + + )} + )} } @@ -233,9 +198,6 @@ export function GithubOrgApps({
) })} - {err && ( -
{err}
- )} {deleting && ( Date: Sun, 1 Nov 2026 14:57:49 +0800 Subject: [PATCH 2/3] feat(web): icon row actions and agent avatars on the GitHub card Row actions on the GitHub card are icons, as on the GitLab and Gitea cards: install, uninstall, and a trash button that deletes a free organization App in place of its menu. An organization App in use leads with its agent's avatar and names the agent in a chip, both linking to the agent; the card reads agents from console data like the GitLab card instead of taking a name callback. Co-Authored-By: Claude Opus 5.5 --- packages/web/messages/en.json | 5 +- packages/web/messages/zh-CN.json | 5 +- .../components/console/GithubCard.test.tsx | 1 + .../web/src/components/console/GithubCard.tsx | 40 ++---- .../components/console/GithubOrgApps.test.tsx | 44 +++--- .../src/components/console/GithubOrgApps.tsx | 132 +++++++++--------- .../console/views/IntegrationsView.tsx | 11 +- 7 files changed, 103 insertions(+), 135 deletions(-) diff --git a/packages/web/messages/en.json b/packages/web/messages/en.json index 4cddc9aef..d355905f3 100644 --- a/packages/web/messages/en.json +++ b/packages/web/messages/en.json @@ -1764,9 +1764,7 @@ "updatePermissions": "Update permissions", "orgApps": { "free": "free", - "usedBy": "used by {agent}", "inUse": "in use", - "more": "More", "delete": "Delete App…", "deleteTitle": "Delete GitHub App", "deleteBody": "Remove from this organization? Its key and installations are forgotten here; the App stays on GitHub until you delete it there.", @@ -1774,7 +1772,8 @@ "cancel": "Cancel", "deleting": "Deleting…", "deleteConfirm": "Delete", - "noInstallations": "Not installed on any account yet." + "noInstallations": "Not installed on any account yet.", + "openAgent": "Open {agent}" }, "builtIn": "built-in", "expand": "Show installations", diff --git a/packages/web/messages/zh-CN.json b/packages/web/messages/zh-CN.json index f0c5e1eb9..3e4524eee 100644 --- a/packages/web/messages/zh-CN.json +++ b/packages/web/messages/zh-CN.json @@ -1764,9 +1764,7 @@ "updatePermissions": "更新权限", "orgApps": { "free": "空闲", - "usedBy": "{agent} 在用", "inUse": "使用中", - "more": "更多", "delete": "删除 App…", "deleteTitle": "删除 GitHub App", "deleteBody": "从此组织移除 ？这里会忘掉它的密钥和安装记录；App 本身仍留在 GitHub 上，需要到 GitHub 上删除。", @@ -1774,7 +1772,8 @@ "cancel": "取消", "deleting": "正在删除…", "deleteConfirm": "删除", - "noInstallations": "还没有安装到任何账号。" + "noInstallations": "还没有安装到任何账号。", + "openAgent": "打开 {agent}" }, "builtIn": "内置", "expand": "展开安装列表", diff --git a/packages/web/src/components/console/GithubCard.test.tsx b/packages/web/src/components/console/GithubCard.test.tsx index 44c902c43..71ae81be4 100644 --- a/packages/web/src/components/console/GithubCard.test.tsx +++ b/packages/web/src/components/console/GithubCard.test.tsx @@ -41,6 +41,7 @@ const mocks = vi.hoisted(() => ({ // The real hook returns a stable org; a fresh object per render would re-run the card's fetch forever. const orgs = vi.hoisted(() => ({ activeOrg: { id: 'org-1' } })) vi.mock('@/lib/org-context', () => ({ useOrgs: () => orgs })) +vi.mock('@/lib/data-context', () => ({ useConsoleData: () => ({ getAgent: () => undefined }) })) vi.mock('@/lib/api', async (importOriginal) => ({ ...(await importOriginal()), fetchGithubInstallations: vi.fn(async () => ({ enabled: true, installations: [installation({})] })), diff --git a/packages/web/src/components/console/GithubCard.tsx b/packages/web/src/components/console/GithubCard.tsx index 07cd6d1a0..f5da88cb2 100644 --- a/packages/web/src/components/console/GithubCard.tsx +++ b/packages/web/src/components/console/GithubCard.tsx @@ -16,7 +16,7 @@ import { } from '@/lib/api' import UninstallGithubInstallationModal from '@/components/console/modals/UninstallGithubInstallationModal' import { Scrim } from '@/components/console/Scrim' -import { GithubAppRow, GithubOrgApps, INSTALLATION_ROW } from '@/components/console/GithubOrgApps' +import { GithubAppRow, GithubOrgApps, INSTALLATION_ROW, InstallAppButton } from '@/components/console/GithubOrgApps' import { consoleKeys } from '@/lib/swr-keys' // No 'use client' here: rendered only inside a client boundary — the Integrations page, or the @@ -28,16 +28,7 @@ import { consoleKeys } from '@/lib/swr-keys' // Installations are org-level infrastructure (like bots) — every member can // see them; installing and syncing are writes (viewers don't get those buttons), // while uninstalling the App from an account is owner-only. -export default function GithubCard({ - canWrite, - isOwner, - agentNameOf -}: { - canWrite: boolean - isOwner: boolean - /** Names the agent an organization App serves; the console passes it, the webchat dialog does not. */ - agentNameOf?: (agentId: string) => string | undefined -}) { +export default function GithubCard({ canWrite, isOwner }: { canWrite: boolean; isOwner: boolean }) { const t = useTranslations('Integrations') // Gate the org-scoped fetch on the active org (same hard-refresh race as SlackCard): // before OrgProvider resolves, `orgBase()` throws → the catch would show "not enabled" @@ -135,14 +126,7 @@ export default function GithubCard({ open={builtInOpen} onToggle={() => setBuiltInOpen((open) => !open)} label={builtInOpen ? t('github.collapse') : t('github.expand')} - aside={ - canWrite && ( - - ) - } + aside={canWrite && void install()} />} > {builtInSlug ?? t('github.label')} {t('github.builtIn')} @@ -182,15 +166,15 @@ export default function GithubCard({ {i.repositorySelection === 'all' ? t('github.allRepositories') : t('github.selectedRepositories')} {isOwner && ( - + + )} @@ -218,9 +202,7 @@ export default function GithubCard({ ))} )} - {enabled === true && ( - - )} + {enabled === true && } {err && (
{err}
)} diff --git a/packages/web/src/components/console/GithubOrgApps.test.tsx b/packages/web/src/components/console/GithubOrgApps.test.tsx index 5d3d288dc..2d339c911 100644 --- a/packages/web/src/components/console/GithubOrgApps.test.tsx +++ b/packages/web/src/components/console/GithubOrgApps.test.tsx @@ -13,7 +13,13 @@ const mocks = vi.hoisted(() => ({ deleteGithubApp: vi.fn(async () => undefined) })) -vi.mock('@/lib/org-context', () => ({ useOrgs: () => ({ activeOrg: { id: 'org-1' } }) })) +const orgs = vi.hoisted(() => ({ activeOrg: { id: 'org-1' }, orgPath: (path: string) => `/acme${path}` })) +const agents = vi.hoisted(() => ({ + getAgent: (id: string) => + id === 'agent-1' ? { id, name: 'reviewer', displayName: 'Reviewer', icon: null, runtime: 'codex' } : undefined +})) +vi.mock('@/lib/org-context', () => ({ useOrgs: () => orgs })) +vi.mock('@/lib/data-context', () => ({ useConsoleData: () => agents })) vi.mock('@/lib/api', async (importOriginal) => ({ ...(await importOriginal()), fetchGithubApps: vi.fn(async () => mocks.apps), @@ -43,11 +49,7 @@ async function render(props: { isOwner?: boolean; canWrite?: boolean } = {}) { await act(async () => { root?.render( new Map() }}> - (id === 'agent-1' ? 'Reviewer' : undefined)} - /> + ) }) @@ -58,12 +60,8 @@ async function render(props: { isOwner?: boolean; canWrite?: boolean } = {}) { const row = (slug: string) => document.querySelector(`[data-org-app="${slug}"]`) const expand = (slug: string) => act(async () => row(slug)!.querySelector('button[aria-expanded]')!.click()) -const menuItem = (text: string) => - [...document.querySelectorAll('[role="menuitem"]')].find((b) => b.textContent?.includes(text)) -async function menuItems(slug: string): Promise { - await act(async () => row(slug)!.querySelector('button[aria-label="More"]')!.click()) - return [...document.querySelectorAll('[role="menuitem"]')].map((b) => b.textContent?.trim() ?? '') -} +const labelled = (slug: string, label: string) => + row(slug)!.querySelector(`button[aria-label="${label}"]`) afterEach(async () => { if (root) await act(async () => root?.unmount()) @@ -102,16 +100,18 @@ describe('GithubOrgApps', () => { await render() const used = row('review-private')! - expect(used.textContent).toContain('used by Reviewer') + // The agent using the App leads its row and names it, both linking to the agent. + expect(used.querySelector('a[title="Open Reviewer"]')?.getAttribute('href')).toBe('/acme/agents/agent-1') + expect(used.querySelector('a.badge')?.textContent).toBe('Reviewer') expect(used.querySelector('a.mono')?.getAttribute('href')).toBe('https://github.com/apps/review-private') // Installations are the second level: collapsed until the App row is expanded. expect(used.textContent).not.toContain('example-org') await expand('review-private') expect(used.textContent).toContain('example-org') expect(used.textContent).toContain('all repositories') - // An App an agent uses can't be deleted, so its row has no menu. - expect(used.querySelector('button[aria-label="More"]')).toBeNull() - expect(used.textContent).toContain('Install on GitHub') + // Row actions are icons; an App an agent uses can't be deleted. + expect(labelled('review-private', 'Install on GitHub')).not.toBeNull() + expect(labelled('review-private', 'Delete App…')).toBeNull() const free = row('triage-example')! expect(free.textContent).toContain('free') @@ -119,12 +119,11 @@ describe('GithubOrgApps', () => { expect(free.textContent).toContain('Not installed on any account yet.') }) - it('deletes a free App from its menu after confirming', async () => { + it('deletes a free App from its trash button after confirming', async () => { mocks.apps = [app({ id: 'app-free', slug: 'triage-example' })] await render() - expect(await menuItems('triage-example')).toEqual(['Delete App…']) - await act(async () => menuItem('Delete App')!.click()) + await act(async () => labelled('triage-example', 'Delete App…')!.click()) const confirm = [...document.querySelectorAll('button')].find( (b) => b.textContent?.trim() === 'Delete' ) @@ -161,9 +160,8 @@ describe('GithubOrgApps', () => { it('lets a viewer expand an App but offers no install or delete', async () => { mocks.apps = [app({ id: 'app-free', slug: 'triage-example' })] await render({ canWrite: false, isOwner: false }) - const scope = row('triage-example')! - expect(scope.textContent).not.toContain('Install on GitHub') - expect(scope.querySelector('button[aria-label="More"]')).toBeNull() - expect(scope.querySelector('button[aria-expanded]')).not.toBeNull() + expect(labelled('triage-example', 'Install on GitHub')).toBeNull() + expect(labelled('triage-example', 'Delete App…')).toBeNull() + expect(row('triage-example')!.querySelector('button[aria-expanded]')).not.toBeNull() }) }) diff --git a/packages/web/src/components/console/GithubOrgApps.tsx b/packages/web/src/components/console/GithubOrgApps.tsx index d4df11426..deea3e4e8 100644 --- a/packages/web/src/components/console/GithubOrgApps.tsx +++ b/packages/web/src/components/console/GithubOrgApps.tsx @@ -2,9 +2,10 @@ import { useState, type ReactNode } from 'react' import { useTranslations } from 'next-intl' import useSWR from 'swr' import { Button, Icon } from '@/components/ui' -import { AnchoredFlyout } from '@/components/ui/AnchoredFlyout' import { SQUARE_MARK_FILL_PCT } from '@/components/mark-box' -import { GithubMark } from '@/components/marks' +import Link from 'next/link' +import { AgentIconView, GithubMark } from '@/components/marks' +import { useConsoleData } from '@/lib/data-context' import { Scrim } from '@/components/console/Scrim' import { deleteGithubApp, fetchGithubApps, githubAppPageUrl, type GithubOrgAppDto } from '@/lib/api' import { useOrgs } from '@/lib/org-context' @@ -17,6 +18,7 @@ export function GithubAppRow({ open, onToggle, label, + icon, children, aside, dataAttr @@ -24,6 +26,8 @@ export function GithubAppRow({ open: boolean onToggle: () => void label: string + /** Leads the row; absent ⇒ the GitHub mark. */ + icon?: ReactNode children: ReactNode aside: ReactNode dataAttr?: string @@ -44,11 +48,13 @@ export function GithubAppRow({ > - - - + {icon ?? ( + + + + - + )} {children} {aside} @@ -56,23 +62,31 @@ export function GithubAppRow({ ) } +/** Opens the App's install page on GitHub, as an icon like every other row action. */ +export function InstallAppButton({ href, onClick }: { href?: string; onClick?: () => void }) { + const t = useTranslations('Integrations.github') + return ( + + ) +} + /** An installation nested under its App: indented on the sunken surface. */ export const INSTALLATION_ROW = 'row grid-cols-1 gap-2 bg-(--surface-sunken) desktop:grid-cols-[minmax(0,1fr)_auto] desktop:gap-[11px] desktop:pl-[50px]' // The organization's own GitHub Apps, each over its installations; absent until the org creates one. -export function GithubOrgApps({ - canWrite, - isOwner, - agentNameOf -}: { - canWrite: boolean - isOwner: boolean - /** Names the agent an App serves; absent (outside the console) ⇒ "in use". */ - agentNameOf?: (agentId: string) => string | undefined -}) { +export function GithubOrgApps({ canWrite, isOwner }: { canWrite: boolean; isOwner: boolean }) { const t = useTranslations('Integrations.github') - const { activeOrg } = useOrgs() + const { activeOrg, orgPath } = useOrgs() + const { getAgent } = useConsoleData() const { data: apps, mutate } = useSWR(activeOrg ? consoleKeys.githubApps(activeOrg.id) : null, ([, id]) => fetchGithubApps(id ?? undefined) ) @@ -92,64 +106,41 @@ export function GithubOrgApps({ <> {apps.map((app) => { const open = expanded.has(app.id) - const agentName = app.agentId ? agentNameOf?.(app.agentId) : undefined + const agent = app.agentId ? getAgent(app.agentId) : undefined + const agentName = agent ? (agent.displayName ?? agent.name) : '' return (
toggle(app.id)} label={open ? t('collapse') : t('expand')} + icon={ + agent ? ( + + + + + + ) : undefined + } aside={ <> - {canWrite && ( - - )} - {/* Only a free App can be deleted, so only its row has a menu. */} + {canWrite && } + {/* Only a free App can be deleted. */} {isOwner && app.agentId === null && ( - ( - - )} + - )} - + + )} } @@ -164,10 +155,15 @@ export function GithubOrgApps({ {app.agentId === null ? ( {t('orgApps.free')} + ) : agent ? ( + + {agentName} + ) : ( - - {agentName ? t('orgApps.usedBy', { agent: agentName }) : t('orgApps.inUse')} - + {t('orgApps.inUse')} )} {open && diff --git a/packages/web/src/components/console/views/IntegrationsView.tsx b/packages/web/src/components/console/views/IntegrationsView.tsx index d625571a7..6ae7f1abd 100644 --- a/packages/web/src/components/console/views/IntegrationsView.tsx +++ b/packages/web/src/components/console/views/IntegrationsView.tsx @@ -205,7 +205,7 @@ export default function IntegrationsView() { const { me } = useProfile() const { myRole } = useOrgs() const { openModal } = useModal() - const { bots, getAgent } = useConsoleData() + const { bots } = useConsoleData() const reconnectOpened = useRef(null) const isOwner = myRole === 'owner' const canWrite = myRole !== 'viewer' // the CP denies viewer writes; hide the controls too @@ -242,14 +242,7 @@ export default function IntegrationsView() {
- { - const agent = getAgent(id) - return agent ? (agent.displayName ?? agent.name) : undefined - }} - /> + {/* Cards own no margin here (see Section) — every card after the first supplies its own gap. */}
From 6a9540da44172fc82c6177c8323505554a417e98 Mon Sep 17 00:00:00 2001 From: zfy0701 <1646270+zfy0701@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:03:52 +0800 Subject: [PATCH 3/3] feat(web): uninstall an organization GitHub App from one account An owner can uninstall an organization App's installation from its row on the GitHub card, through the same confirmation as the built-in App's. The uninstall route already signs with the App an installation belongs to; a unit test now pins that. Co-Authored-By: Claude Opus 5.5 --- .../control-plane/src/github/github.test.ts | 24 ++++++++++++ .../components/console/GithubOrgApps.test.tsx | 38 ++++++++++++++++++- .../src/components/console/GithubOrgApps.tsx | 33 +++++++++++++++- .../UninstallGithubInstallationModal.tsx | 3 +- 4 files changed, 93 insertions(+), 5 deletions(-) diff --git a/packages/control-plane/src/github/github.test.ts b/packages/control-plane/src/github/github.test.ts index e0a223fab..7c1d8add6 100644 --- a/packages/control-plane/src/github/github.test.ts +++ b/packages/control-plane/src/github/github.test.ts @@ -591,6 +591,30 @@ describe('GithubService.uninstallInstallation', () => { await expect(svc.uninstallInstallation(123456789n)).resolves.toBeUndefined() }) + it("uninstalls an organization App's installation with that App's own JWT", async () => { + const issuers: string[] = [] + const svc = new GithubService({ + cfg: cfg(), + clock: new FakeClock(1_700_000_000_000), + installations: {} as never, + installState: { put: async () => {}, consume: async () => true }, + pepper: 'p'.repeat(32), + appRegistry: { + configForInstallation: async (id: bigint) => + id === 43n ? { ...cfg(), appId: 2000, jwtIssuer: '2000' } : undefined + } as never, + fetchImpl: async (_url, init) => { + const jwt = String(new Headers(init?.headers).get('authorization')).replace(/^Bearer /, '') + issuers.push(JSON.parse(Buffer.from(jwt.split('.')[1]!, 'base64url').toString()).iss) + return new Response(null, { status: 204 }) + } + }) + + await svc.uninstallInstallation(43n) + await svc.uninstallInstallation(42n) + expect(issuers).toEqual(['2000', cfg().jwtIssuer]) + }) + it('preserves non-terminal uninstall failures', async () => { const svc = new GithubService({ cfg: cfg(), diff --git a/packages/web/src/components/console/GithubOrgApps.test.tsx b/packages/web/src/components/console/GithubOrgApps.test.tsx index 2d339c911..d0e6f2087 100644 --- a/packages/web/src/components/console/GithubOrgApps.test.tsx +++ b/packages/web/src/components/console/GithubOrgApps.test.tsx @@ -10,7 +10,8 @@ Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }) const mocks = vi.hoisted(() => ({ apps: [] as GithubOrgAppDto[], - deleteGithubApp: vi.fn(async () => undefined) + deleteGithubApp: vi.fn(async () => undefined), + uninstallGithubInstallation: vi.fn(async () => undefined) })) const orgs = vi.hoisted(() => ({ activeOrg: { id: 'org-1' }, orgPath: (path: string) => `/acme${path}` })) @@ -23,7 +24,8 @@ vi.mock('@/lib/data-context', () => ({ useConsoleData: () => agents })) vi.mock('@/lib/api', async (importOriginal) => ({ ...(await importOriginal()), fetchGithubApps: vi.fn(async () => mocks.apps), - deleteGithubApp: mocks.deleteGithubApp + deleteGithubApp: mocks.deleteGithubApp, + uninstallGithubInstallation: mocks.uninstallGithubInstallation })) const { GithubOrgApps } = await import('./GithubOrgApps') @@ -69,6 +71,7 @@ afterEach(async () => { root = undefined host = undefined mocks.deleteGithubApp.mockClear() + mocks.uninstallGithubInstallation.mockClear() }) describe('GithubOrgApps', () => { @@ -131,6 +134,36 @@ describe('GithubOrgApps', () => { expect(mocks.deleteGithubApp).toHaveBeenCalledWith('app-free') }) + it('lets an owner uninstall an organization App from one account after confirming', async () => { + mocks.apps = [ + app({ + id: 'app-used', + slug: 'review-private', + agentId: 'agent-1', + installations: [ + { + id: 'row-43', + installationId: 43, + accountLogin: 'example-org', + accountType: 'Organization', + repositorySelection: 'all', + suspended: false + } + ] + }) + ] + await render() + await expand('review-private') + + await act(async () => labelled('review-private', 'Uninstall')!.click()) + expect(document.body.textContent).toContain('example-org') + const confirm = [...document.querySelectorAll('.modal button')].find( + (b) => b.textContent?.trim() === 'Uninstall' + ) + await act(async () => confirm!.click()) + expect(mocks.uninstallGithubInstallation).toHaveBeenCalledWith('row-43') + }) + it('keeps a suspended installation in the list, marked suspended', async () => { mocks.apps = [ app({ @@ -162,6 +195,7 @@ describe('GithubOrgApps', () => { await render({ canWrite: false, isOwner: false }) expect(labelled('triage-example', 'Install on GitHub')).toBeNull() expect(labelled('triage-example', 'Delete App…')).toBeNull() + expect(labelled('triage-example', 'Uninstall')).toBeNull() expect(row('triage-example')!.querySelector('button[aria-expanded]')).not.toBeNull() }) }) diff --git a/packages/web/src/components/console/GithubOrgApps.tsx b/packages/web/src/components/console/GithubOrgApps.tsx index deea3e4e8..2433d0c3c 100644 --- a/packages/web/src/components/console/GithubOrgApps.tsx +++ b/packages/web/src/components/console/GithubOrgApps.tsx @@ -7,6 +7,7 @@ import Link from 'next/link' import { AgentIconView, GithubMark } from '@/components/marks' import { useConsoleData } from '@/lib/data-context' import { Scrim } from '@/components/console/Scrim' +import UninstallGithubInstallationModal from '@/components/console/modals/UninstallGithubInstallationModal' import { deleteGithubApp, fetchGithubApps, githubAppPageUrl, type GithubOrgAppDto } from '@/lib/api' import { useOrgs } from '@/lib/org-context' import { consoleKeys } from '@/lib/swr-keys' @@ -92,6 +93,7 @@ export function GithubOrgApps({ canWrite, isOwner }: { canWrite: boolean; isOwne ) const [expanded, setExpanded] = useState>(new Set()) const [deleting, setDeleting] = useState(null) + const [uninstalling, setUninstalling] = useState<{ id: string; accountLogin: string } | null>(null) if (!apps || apps.length === 0) return null @@ -185,8 +187,21 @@ export function GithubOrgApps({ canWrite, isOwner }: { canWrite: boolean; isOwne {t('suspended')} )}
- - {i.repositorySelection === 'all' ? t('allRepositories') : t('selectedRepositories')} + + + {i.repositorySelection === 'all' ? t('allRepositories') : t('selectedRepositories')} + + {isOwner && ( + + )}
)) @@ -194,6 +209,20 @@ export function GithubOrgApps({ canWrite, isOwner }: { canWrite: boolean; isOwne ) })} + {uninstalling && ( + setUninstalling(null)} onClick={() => setUninstalling(null)}> +
e.stopPropagation()}> + setUninstalling(null)} + onUninstalled={() => { + setUninstalling(null) + void mutate() + }} + /> +
+ + )} {deleting && ( onClose: () => void onUninstalled: (id: string) => void }) {