From bcc435992b5dca9d4101a7468473073148ad810e Mon Sep 17 00:00:00 2001 From: pikann22 Date: Wed, 28 Oct 2026 08:14:43 +0000 Subject: [PATCH 1/6] feat(git): enhance project connection management - Added support for project-scoped connections in the git identity setup process. - Updated SetupInput to include ProjectID for project-specific connections. - Implemented providerScope function to handle different scopes. - Modified BeginIdentitySetup and CompleteIdentitySetup methods to accommodate project connections. - Introduced ListConnections method to retrieve identities for a specific project. - Added DeleteProjectConnection method to remove project-specific connections. - Updated HTTP handlers to manage project connections, including setup and deletion. - Enhanced tests to cover new project connection functionalities. - Updated database schema to support project-scoped identities and connections. --- .../e2e/features/admin/git-identities.feature | 42 --- apps/e2e/features/admin/git-setup.feature | 65 ---- apps/e2e/features/iam/git-permissions.feature | 27 +- .../features/projects/git-settings.feature | 18 +- apps/e2e/tests/admin/git-identities.spec.ts | 183 ---------- apps/e2e/tests/admin/git-setup.spec.ts | 325 ------------------ apps/e2e/tests/iam/git-permissions.spec.ts | 81 ++++- apps/e2e/tests/projects/git-settings.spec.ts | 57 ++- .../src/components/app-shell/app-sidebar.tsx | 11 - .../git/DeleteGitIdentityDialog.tsx | 4 +- .../git/GitIdentitiesPanel.test.tsx | 216 ------------ .../src/components/git/GitIdentitiesPanel.tsx | 212 ------------ .../components/git/GitSetupCallback.test.tsx | 69 +++- .../src/components/git/GitSetupCallback.tsx | 46 ++- .../web/src/components/git/GitSetupDialog.tsx | 266 ++++++++++---- apps/web/src/components/git/git-ui.tsx | 52 +++ .../projects/agents/AgentGitTab.test.tsx | 4 +- .../components/projects/roles/permissions.ts | 6 + .../projects/settings/GitConnections.test.tsx | 278 +++++++++++++++ .../projects/settings/GitConnections.tsx | 208 +++++++++++ .../projects/settings/GitSettings.test.tsx | 175 ++++++++-- .../projects/settings/GitSettings.tsx | 182 ++++++++-- apps/web/src/i18n/locales/en/admin.json | 72 ++-- apps/web/src/i18n/locales/en/appShell.json | 1 - apps/web/src/i18n/locales/en/projects.json | 53 ++- apps/web/src/i18n/locales/es/admin.json | 72 ++-- apps/web/src/i18n/locales/es/appShell.json | 1 - apps/web/src/i18n/locales/es/projects.json | 53 ++- apps/web/src/i18n/locales/fr/admin.json | 72 ++-- apps/web/src/i18n/locales/fr/appShell.json | 1 - apps/web/src/i18n/locales/fr/projects.json | 53 ++- apps/web/src/i18n/locales/ja/admin.json | 72 ++-- apps/web/src/i18n/locales/ja/appShell.json | 1 - apps/web/src/i18n/locales/ja/projects.json | 53 ++- apps/web/src/i18n/locales/ko/admin.json | 72 ++-- apps/web/src/i18n/locales/ko/appShell.json | 1 - apps/web/src/i18n/locales/ko/projects.json | 53 ++- apps/web/src/i18n/locales/pt-BR/admin.json | 72 ++-- apps/web/src/i18n/locales/pt-BR/appShell.json | 1 - apps/web/src/i18n/locales/pt-BR/projects.json | 53 ++- apps/web/src/i18n/locales/ru/admin.json | 72 ++-- apps/web/src/i18n/locales/ru/appShell.json | 1 - apps/web/src/i18n/locales/ru/projects.json | 53 ++- apps/web/src/i18n/locales/vi/admin.json | 72 ++-- apps/web/src/i18n/locales/vi/appShell.json | 1 - apps/web/src/i18n/locales/vi/projects.json | 53 ++- apps/web/src/i18n/locales/zh-CN/admin.json | 72 ++-- apps/web/src/i18n/locales/zh-CN/appShell.json | 1 - apps/web/src/i18n/locales/zh-CN/projects.json | 53 ++- apps/web/src/lib/api-error.ts | 1 + apps/web/src/lib/git-api.test.ts | 4 +- apps/web/src/lib/git-api.ts | 154 +++++++-- apps/web/src/routeTree.gen.ts | 22 -- .../routes/_authenticated/admin/git/index.tsx | 38 -- docs/ai-agent/git-agent-workflow.md | 50 +-- docs/plugins/git-provider-spi.md | 1 + services/api/internal/apierr/codes.go | 4 +- services/api/internal/domain/git/entity.go | 19 +- services/api/internal/domain/git/errors.go | 6 +- .../api/internal/domain/git/repository.go | 8 +- .../internal/platform/authz/iam/actions.go | 7 +- .../platform/authz/iam/registry_test.go | 2 +- .../bundledskills/content/paca-role-policy.md | 2 +- .../platform/gitprovider/spi_types.go | 33 +- .../testdata/wire-shapes.golden.json | 16 + .../repository/postgres/git_repository.go | 67 +++- .../postgres/git_repository_test.go | 38 +- services/api/internal/service/git/access.go | 34 +- .../internal/service/git/connections_test.go | 257 ++++++++++++++ .../api/internal/service/git/repo_webhooks.go | 139 ++++++++ services/api/internal/service/git/service.go | 74 +++- .../api/internal/service/git/service_test.go | 78 ++++- services/api/internal/service/git/setup.go | 114 +++++- .../api/internal/service/git/setup_test.go | 28 +- .../internal/transport/http/dto/git_dto.go | 30 +- .../transport/http/handler/git_handler.go | 97 +++++- .../http/handler/git_handler_test.go | 57 ++- .../transport/http/presenter/response.go | 2 + .../transport/http/router/git_guards_test.go | 51 +-- .../internal/transport/http/router/router.go | 21 +- .../migrations/000073_add_git_integration.sql | 21 +- 81 files changed, 3214 insertions(+), 1922 deletions(-) delete mode 100644 apps/e2e/features/admin/git-identities.feature delete mode 100644 apps/e2e/features/admin/git-setup.feature delete mode 100644 apps/e2e/tests/admin/git-identities.spec.ts delete mode 100644 apps/e2e/tests/admin/git-setup.spec.ts delete mode 100644 apps/web/src/components/git/GitIdentitiesPanel.test.tsx delete mode 100644 apps/web/src/components/git/GitIdentitiesPanel.tsx create mode 100644 apps/web/src/components/projects/settings/GitConnections.test.tsx create mode 100644 apps/web/src/components/projects/settings/GitConnections.tsx delete mode 100644 apps/web/src/routes/_authenticated/admin/git/index.tsx create mode 100644 services/api/internal/service/git/connections_test.go create mode 100644 services/api/internal/service/git/repo_webhooks.go diff --git a/apps/e2e/features/admin/git-identities.feature b/apps/e2e/features/admin/git-identities.feature deleted file mode 100644 index db9c09d0e..000000000 --- a/apps/e2e/features/admin/git-identities.feature +++ /dev/null @@ -1,42 +0,0 @@ -@admin @git -Feature: Administration, Git - The Git page lists the platforms the installed provider plugins support and - whether each has a shared bot identity. It is for holders of the workspace - action git.identities:manage; everyone else is sent home and does not see the - entry in the sidebar. The e2e stack has no Git provider plugin, so the - provider and identity endpoints are stubbed in the browser. - - Rule: What the page shows - - Background: - Given the user already has a stored authenticated admin session - - Scenario: No provider plugin is installed - Given the platform lists no Git provider - When the user opens Administration, Git - Then the page says no Git provider is installed - And it links to the plugins page - - Scenario: A platform without an identity is not connected - Given the platform lists a Git provider and no identity - When the user opens Administration, Git - Then the platform's card is "Not connected" and offers "Set up" - - Scenario: A platform with an identity shows the bot to mention - Given the platform lists a Git provider and a shared identity "paca-bot" - When the user opens Administration, Git - Then the platform's card is "Connected" and shows "@paca-bot" - And it offers "Remove" - - Rule: Who may open it - - Scenario: A user without git.identities:manage is sent home - Given a user with a workspace role that lacks "git.identities:manage" - When the user opens /admin/git - Then they land on the home page - And the sidebar has no Git entry - - Scenario: A holder of git.identities:manage sees the entry and the page - Given a user with a workspace role granting only "git.identities:manage" - When the user opens /admin/git - Then the Git page is shown diff --git a/apps/e2e/features/admin/git-setup.feature b/apps/e2e/features/admin/git-setup.feature deleted file mode 100644 index b3c6bf3c2..000000000 --- a/apps/e2e/features/admin/git-setup.feature +++ /dev/null @@ -1,65 +0,0 @@ -@admin @git -Feature: Setting up a Git identity and returning from the platform - An administrator connects a platform from Administration, Git: the dialog - collects the optional app name and organization, the platform is asked to - start the setup, and the browser is sent to the platform. The platform sends - the browser back to /git/setup/callback, which finishes the setup (or an - access grant) exactly once and returns the user to where they started. - Providers and the setup endpoints are stubbed in the browser; the callback - page is also tried against the real API. - - Background: - Given the user already has a stored authenticated admin session - - Rule: The setup dialog - - Scenario: Connecting a platform asks for optional details - Given a provider with no identity - When the administrator clicks "Set up" - Then a dialog titled "Set up Example Git on git.example.test" shows "App name (optional)", "Organization (optional)" and the public-app switch - - Scenario: Details are sent to the platform and the browser follows it - When the administrator enters an app name and an organization, switches on the public app, and continues - Then the setup request carries the provider, host, name, owner_org and public true - And the browser is sent to the platform's page - - Scenario: Cancel closes the dialog without calling the platform - When the administrator cancels - Then no setup request is made - - Scenario: Removing an identity asks for confirmation - Given a provider with a shared identity - When the administrator clicks "Remove" - Then the dialog "Remove this identity?" explains what happens - When the administrator confirms with "Remove identity" - Then the identity is deleted - - Rule: The callback page - - Scenario: A callback without a state is invalid - When the browser opens /git/setup/callback with no parameters - Then "Setup could not be completed" and "This setup link is invalid, expired or already used." are shown - And a link goes back to Git settings - - Scenario: A state the API does not know fails with a message - When the browser opens /git/setup/callback?state=unknown&code=abc - Then the failure is shown instead of a blank page - - Scenario: A finished setup returns to Git settings - Given the API completes the setup - When the browser opens the callback with a state and code - Then the code is sent exactly once, with every other query parameter - And the browser lands on /admin/git - - Scenario: A finished access grant returns to the project - Given the browser remembers an access flow for a project - And the API completes the grant with a return url in the project - Then the browser lands on that project's settings - - Scenario: A return url on another site is ignored - Given the API answers a return url on another origin - Then the browser lands on /admin/git - - Scenario: A setup that needs another step sends the browser on - Given the API answers a next step - Then the browser follows it diff --git a/apps/e2e/features/iam/git-permissions.feature b/apps/e2e/features/iam/git-permissions.feature index 9409bd9d9..124813dd6 100644 --- a/apps/e2e/features/iam/git-permissions.feature +++ b/apps/e2e/features/iam/git-permissions.feature @@ -1,11 +1,13 @@ @iam @git Feature: Who may use the Git integration Git access is four project actions (git:read, git:write, git:approve, - git:merge) and one workspace action (git.identities:manage) for the bot - identities. Every route is gated in the router. Linking a repository needs - both git:write on the project and git.identities:manage, because the shared - identity's installations are instance-wide. The webhook intake is public: the - Git platform calls it and the provider plugin verifies the signature. + git:merge), one project action for a project's own connections + (git.connections:manage) and one workspace action (git.identities:manage) for + the connections every project can use. Every route is gated in the router. + Linking a repository needs git:write: the connections it goes through were + made available to the project by whoever holds git.identities:manage, or by + the project itself. The webhook intake is public: the Git platform calls it + and the provider plugin verifies the signature. Rule: Reading and writing a project's Git integration @@ -34,18 +36,25 @@ Feature: Who may use the Git integration Given the user already has a stored authenticated admin session And a project - Scenario: git:write alone cannot link a repository + Scenario: git:write alone passes the link gate Given a member whose project role grants "git:write" + Then linking a repository is not refused for permissions + + Scenario: git:read cannot link a repository + Given a member whose project role grants only "git:read" Then linking a repository answers 403 - But saving the project's Git settings is not refused Scenario: git.identities:manage alone cannot link a repository Given a user with the workspace action "git.identities:manage" and no role on the project Then linking a repository answers 403 - Scenario: git:write together with git.identities:manage passes the gate + Scenario: A project's own connection needs git:write and git.connections:manage + Given members with only "git:write", only "git.connections:manage", and both + Then adding a project connection answers 403 for the first two and is not refused for the third + + Scenario: The workspace action does not stand in for the project one Given a member with "git:write" on the project and "git.identities:manage" in the workspace - Then linking a repository is not refused for permissions + Then adding a project connection answers 403 Rule: Approving and merging are their own actions diff --git a/apps/e2e/features/projects/git-settings.feature b/apps/e2e/features/projects/git-settings.feature index e55f1c58c..0b3151d45 100644 --- a/apps/e2e/features/projects/git-settings.feature +++ b/apps/e2e/features/projects/git-settings.feature @@ -2,8 +2,9 @@ Feature: A project's Git settings The Git tab of a project's settings links repositories and sets the automation. Reading it needs git:read. Linking a repository needs git:write - on the project and the workspace-level git.identities:manage; a writer - without it is told who to ask instead of being shown a form that would fail. + and a connection the project can use; a writer with no connection available + is told to ask someone who can manage Git connections instead of being shown + a form that would fail. The e2e stack has no Git provider plugin, so the provider endpoints are stubbed in the browser. @@ -17,10 +18,9 @@ Feature: A project's Git settings Given the platform lists no Git provider When the user opens the project's Git settings Then the page says no Git provider is installed - And it offers a button to open the Git integration Scenario: An administrator links a repository - Given the platform lists a Git provider and the project has no repository + Given the platform lists a Git provider and a connection, and the project has no repository When the user opens the project's Git settings Then the "Repository" field and the "Link" button are shown When the user types "acme/web" and clicks "Link" @@ -34,14 +34,14 @@ Feature: A project's Git settings And a project And the platform lists a Git provider and the project has no repository - Scenario: A writer without git.identities:manage is told to ask an administrator - Given a member with "git:read" and "git:write" and no workspace role + Scenario: A writer is told to ask for a connection when there is none + Given a member with "git:read" and "git:write" and no connection is available When the member opens the project's Git settings - Then the page explains that linking also needs the workspace permission to manage Git identities + Then the page asks them to find someone who can manage Git connections And there is no "Link" button - Scenario: A writer with git.identities:manage gets the form - Given a member with "git:read" and "git:write" and the workspace action "git.identities:manage" + Scenario: A writer gets the form once a connection is available + Given a member with "git:read" and "git:write" and a connection is available When the member opens the project's Git settings Then the "Repository" field and the "Link" button are shown diff --git a/apps/e2e/tests/admin/git-identities.spec.ts b/apps/e2e/tests/admin/git-identities.spec.ts deleted file mode 100644 index 659e44a5c..000000000 --- a/apps/e2e/tests/admin/git-identities.spec.ts +++ /dev/null @@ -1,183 +0,0 @@ -// spec: features/admin/git-identities.feature -// seed: tests/seed.spec.ts -// -// Administration, Git. The e2e stack has no Git provider plugin, so the -// provider and identity endpoints are answered in the browser with page.route; -// access to the page is still decided by the real permissions of the user. - -import { expect, type Page, test } from "@playwright/test"; -import { - authRequest, - BASE_URL, - cleanupGlobalRolesByPrefix, - cleanupUsersByPrefix, - createUserWithGlobalPermissions, - newRunId, - RESTRICTED_PASSWORD, - signIn, -} from "../helpers/e2e-api"; - -const PREFIX = "E2E_GITADM_"; -const RUN_ID = newRunId(); -const name = (label: string) => `${PREFIX}${label}_${RUN_ID}`; - -const PROVIDER = { - id: "com.example.git", - display_name: "Example Git", - hosts: ["git.example.test"], - self_hosted_hosts: false, - terms: { - change_request: "pull request", - change_request_plural: "pull requests", - repository: "Repository", - }, - capabilities: { multiple_identities: false, access_grants: false }, -}; - -const IDENTITY = { - id: "00000000-0000-4000-8000-0000000000b1", - provider_id: PROVIDER.id, - host: "git.example.test", - kind: "app", - scope: "shared", - handle: "paca-bot", - display_name: "Paca Bot", - expires_at: null, - created_at: "2026-10-01T00:00:00Z", -}; - -const ok = (data: unknown) => ({ - status: 200, - contentType: "application/json", - body: JSON.stringify({ success: true, data }), -}); - -async function stubGit( - page: Page, - opts: { providers: unknown[]; identities: unknown[] }, -) { - await page.route("**/api/v1/git/providers", (route) => - route.fulfill(ok(opts.providers)), - ); - await page.route("**/api/v1/git/identities", (route) => - route.fulfill(ok(opts.identities)), - ); -} - -const openGitAdmin = async (page: Page) => { - await page.goto(`${BASE_URL}/admin/git`); - await expect( - page.getByRole("heading", { name: "Git", exact: true }), - ).toBeVisible(); -}; - -test.describe("Administration, Git", () => { - test.beforeAll(async ({ request }) => { - await authRequest(request); - await cleanupUsersByPrefix(request, PREFIX); - await cleanupGlobalRolesByPrefix(request, PREFIX); - }); - - test.beforeEach(async ({ request }) => { - await authRequest(request); - }); - - test.afterAll(async ({ request }) => { - await authRequest(request); - await cleanupUsersByPrefix(request, PREFIX); - await cleanupGlobalRolesByPrefix(request, PREFIX); - }); - - test.describe("What the page shows", () => { - test("No provider plugin is installed", async ({ page }) => { - await signIn(page); - await stubGit(page, { providers: [], identities: [] }); - await openGitAdmin(page); - - await expect(page.getByText("No Git provider installed")).toBeVisible(); - await expect( - page.getByRole("link", { name: "Manage plugins" }), - ).toHaveAttribute("href", "/admin/plugins"); - }); - - test("A platform without an identity is not connected", async ({ - page, - }) => { - await signIn(page); - await stubGit(page, { providers: [PROVIDER], identities: [] }); - await openGitAdmin(page); - - await expect( - page.getByRole("heading", { name: "Example Git" }), - ).toBeVisible(); - await expect(page.getByText("Not connected")).toBeVisible(); - await expect(page.getByRole("button", { name: "Set up" })).toBeVisible(); - }); - - test("A platform with an identity shows the bot to mention", async ({ - page, - }) => { - await signIn(page); - await stubGit(page, { providers: [PROVIDER], identities: [IDENTITY] }); - await openGitAdmin(page); - - await expect(page.getByText("Connected", { exact: true })).toBeVisible(); - await expect(page.getByText("@paca-bot")).toBeVisible(); - await expect(page.getByRole("button", { name: "Remove" })).toBeVisible(); - }); - }); - - test.describe("Who may open it", () => { - test("A user without git.identities:manage is sent home", async ({ - page, - request, - playwright, - }) => { - const username = name("PLAIN"); - await createUserWithGlobalPermissions(request, playwright, { - username, - roleName: name("ROLE_PLAIN"), - permissions: { "users:read": true }, - }); - await signIn(page, username, RESTRICTED_PASSWORD); - await page.goto(`${BASE_URL}/admin/git`); - - await expect(page).toHaveURL(/\/home/); - // The sidebar entries come from the user's permissions: open the sidebar - // when it is collapsed (narrow screens) and wait for an entry the user - // holds before asserting the Git entry is absent. - if ((page.viewportSize()?.width ?? 1280) < 768) { - await page - .getByRole("button", { name: /toggle sidebar/i }) - .first() - .click(); - } - await expect( - page.getByRole("link", { name: "Users", exact: true }), - ).toBeVisible(); - await expect( - page.getByRole("link", { name: "Git", exact: true }), - ).toHaveCount(0); - }); - - test("A holder of git.identities:manage sees the entry and the page", async ({ - page, - request, - playwright, - }) => { - const username = name("MANAGER"); - await createUserWithGlobalPermissions(request, playwright, { - username, - roleName: name("ROLE_MANAGER"), - permissions: { "git.identities:manage": true }, - }); - await signIn(page, username, RESTRICTED_PASSWORD); - await stubGit(page, { providers: [PROVIDER], identities: [] }); - await openGitAdmin(page); - - await expect( - page.getByRole("heading", { name: "Example Git" }), - ).toBeVisible(); - }); - }); -}); diff --git a/apps/e2e/tests/admin/git-setup.spec.ts b/apps/e2e/tests/admin/git-setup.spec.ts deleted file mode 100644 index 49e737942..000000000 --- a/apps/e2e/tests/admin/git-setup.spec.ts +++ /dev/null @@ -1,325 +0,0 @@ -// spec: features/admin/git-setup.feature -// seed: tests/seed.spec.ts - -import { expect, type Page, test } from "@playwright/test"; -import { BASE_URL, signIn } from "../helpers/e2e-api"; - -const PROVIDER = { - id: "com.example.git", - display_name: "Example Git", - hosts: ["git.example.test"], - self_hosted_hosts: false, - terms: { - change_request: "pull request", - change_request_plural: "pull requests", - repository: "Repository", - }, - capabilities: { multiple_identities: false, access_grants: false }, -}; - -const IDENTITY = { - id: "00000000-0000-4000-8000-0000000000b1", - provider_id: PROVIDER.id, - host: "git.example.test", - kind: "app", - scope: "shared", - handle: "paca-bot", - display_name: "Paca Bot", - expires_at: null, - created_at: "2026-10-01T00:00:00Z", -}; - -const ok = (data: unknown) => ({ - status: 200, - contentType: "application/json", - body: JSON.stringify({ success: true, data }), -}); - -interface Stub { - identities: unknown[]; - calls: Array<{ method: string; path: string; body?: unknown }>; -} - -async function stubGit(page: Page, stub: Stub) { - await page.route("**/api/v1/git/providers", (route) => - route.fulfill(ok([PROVIDER])), - ); - await page.route("**/api/v1/git/identities", (route) => - route.fulfill(ok(stub.identities)), - ); - await page.route("**/api/v1/git/identities/*", (route) => { - const request = route.request(); - if (request.method() === "DELETE") { - stub.calls.push({ - method: "DELETE", - path: new URL(request.url()).pathname, - }); - stub.identities = []; - return route.fulfill({ status: 204 }); - } - return route.continue(); - }); - await page.route("**/__git_platform__**", (route) => - route.fulfill({ - status: 200, - contentType: "text/html", - body: "

platform

", - }), - ); -} - -const newStub = (over: Partial = {}): Stub => ({ - identities: [], - calls: [], - ...over, -}); - -const captureSetup = async (page: Page, stub: Stub) => { - await page.route("**/api/v1/git/identities/setup", (route) => { - const request = route.request(); - stub.calls.push({ - method: request.method(), - path: "/setup", - body: request.postDataJSON(), - }); - return route.fulfill( - ok({ - kind: "redirect", - url: `${BASE_URL}/__git_platform__?step=setup`, - state: "s1", - }), - ); - }); -}; - -test.describe("Git setup", () => { - test.describe("The setup dialog", () => { - test("Connecting a platform asks for optional details", async ({ - page, - }) => { - await signIn(page); - await stubGit(page, newStub()); - await page.goto(`${BASE_URL}/admin/git`); - - await page.getByRole("button", { name: "Set up" }).click(); - const dialog = page.getByRole("dialog"); - await expect( - dialog.getByText("Set up Example Git on git.example.test"), - ).toBeVisible(); - await expect(dialog.getByLabel("App name (optional)")).toBeVisible(); - await expect(dialog.getByLabel("Organization (optional)")).toBeVisible(); - await expect( - dialog.getByText("Allow installing on other accounts (public app)"), - ).toBeVisible(); - }); - - test("Details are sent to the platform and the browser follows it", async ({ - page, - }) => { - const stub = newStub(); - await signIn(page); - await stubGit(page, stub); - await captureSetup(page, stub); - await page.goto(`${BASE_URL}/admin/git`); - - await page.getByRole("button", { name: "Set up" }).click(); - const dialog = page.getByRole("dialog"); - await dialog.getByLabel("App name (optional)").fill("Acme Bot"); - await dialog.getByLabel("Organization (optional)").fill("acme"); - await dialog.getByRole("switch").click(); - await dialog - .getByRole("button", { name: /Continue to git\.example\.test/ }) - .click(); - - await expect(page).toHaveURL(/__git_platform__/); - const call = stub.calls.find((c) => c.path === "/setup"); - expect(call?.method).toBe("POST"); - expect(call?.body).toMatchObject({ - provider_id: PROVIDER.id, - host: "git.example.test", - params: { name: "Acme Bot", owner_org: "acme", public: "true" }, - }); - }); - - test("Cancel closes the dialog without calling the platform", async ({ - page, - }) => { - const stub = newStub(); - await signIn(page); - await stubGit(page, stub); - await captureSetup(page, stub); - await page.goto(`${BASE_URL}/admin/git`); - - await page.getByRole("button", { name: "Set up" }).click(); - await page - .getByRole("dialog") - .getByRole("button", { name: "Cancel" }) - .click(); - await expect(page.getByRole("dialog")).toHaveCount(0); - expect(stub.calls).toEqual([]); - }); - - test("Removing an identity asks for confirmation", async ({ page }) => { - const stub = newStub({ identities: [IDENTITY] }); - await signIn(page); - await stubGit(page, stub); - await page.goto(`${BASE_URL}/admin/git`); - - await page.getByRole("button", { name: "Remove" }).click(); - const dialog = page.getByRole("alertdialog").or(page.getByRole("dialog")); - await expect(dialog.getByText("Remove this identity?")).toBeVisible(); - await dialog.getByRole("button", { name: "Remove identity" }).click(); - await expect - .poll(() => stub.calls.some((c) => c.method === "DELETE")) - .toBe(true); - expect(stub.calls[0].path).toContain(IDENTITY.id); - }); - }); - - test.describe("The callback page", () => { - test("A callback without a state is invalid", async ({ page }) => { - await signIn(page); - await page.goto(`${BASE_URL}/git/setup/callback`); - - const alert = page.getByRole("alert"); - await expect(alert).toContainText("Setup could not be completed"); - await expect(alert).toContainText( - "This setup link is invalid, expired or already used.", - ); - await alert.getByRole("link", { name: "Back to Git settings" }).click(); - await expect(page).toHaveURL(/\/admin\/git/); - }); - - test("A state the API does not know fails with a message", async ({ - page, - }) => { - await signIn(page); - await page.goto(`${BASE_URL}/git/setup/callback?state=unknown&code=abc`); - await expect(page.getByRole("alert")).toContainText( - "Setup could not be completed", - ); - }); - - test("A finished setup returns to Git settings", async ({ page }) => { - const completions: unknown[] = []; - await signIn(page); - await page.route("**/api/v1/git/providers", (r) => - r.fulfill(ok([PROVIDER])), - ); - await page.route("**/api/v1/git/identities", (r) => - r.fulfill(ok([IDENTITY])), - ); - await page.route("**/api/v1/git/identities/setup/complete", (route) => { - completions.push(route.request().postDataJSON()); - return route.fulfill( - ok({ - identity: IDENTITY, - next: null, - return_url: `${BASE_URL}/admin/git`, - done: true, - }), - ); - }); - await page.goto( - `${BASE_URL}/git/setup/callback?state=abc123&code=1e5&extra=x`, - ); - - await expect(page).toHaveURL(/\/admin\/git$/); - expect(completions).toEqual([ - { state: "abc123", params: { code: "1e5", extra: "x" } }, - ]); - }); - - test("A return url on another site is ignored", async ({ page }) => { - await signIn(page); - await page.route("**/api/v1/git/providers", (r) => - r.fulfill(ok([PROVIDER])), - ); - await page.route("**/api/v1/git/identities", (r) => - r.fulfill(ok([IDENTITY])), - ); - await page.route("**/api/v1/git/identities/setup/complete", (route) => - route.fulfill( - ok({ - identity: IDENTITY, - next: null, - return_url: "https://evil.example.com/steal", - done: true, - }), - ), - ); - await page.goto(`${BASE_URL}/git/setup/callback?state=abc&code=1`); - await expect(page).toHaveURL(/\/admin\/git$/); - }); - - test("A setup that needs another step sends the browser on", async ({ - page, - }) => { - await signIn(page); - await page.route("**/__git_platform__**", (route) => - route.fulfill({ - status: 200, - contentType: "text/html", - body: "

platform

", - }), - ); - await page.route("**/api/v1/git/identities/setup/complete", (route) => - route.fulfill( - ok({ - identity: IDENTITY, - next: { - kind: "redirect", - url: `${BASE_URL}/__git_platform__?step=2`, - state: "s2", - }, - return_url: `${BASE_URL}/admin/git`, - done: false, - }), - ), - ); - await page.goto(`${BASE_URL}/git/setup/callback?state=abc&code=1`); - await expect(page).toHaveURL(/__git_platform__\?step=2/); - }); - - test("A finished access grant returns to the project", async ({ page }) => { - await signIn(page); - const projectId = "00000000-0000-4000-8000-0000000000d1"; - const state = "grant-1"; - await page.addInitScript( - ([key, value]) => window.sessionStorage.setItem(key, value), - [ - `paca.git.flow.${state}`, - JSON.stringify({ kind: "access", projectId }), - ], - ); - const completions: unknown[] = []; - await page.route(`**/api/v1/projects/${projectId}**`, (route) => - route.abort(), - ); - await page.route( - `**/api/v1/projects/${projectId}/git/access/complete`, - (route) => { - completions.push(route.request().postDataJSON()); - return route.fulfill( - ok({ - owner: "other-org", - return_url: `${BASE_URL}/projects/${projectId}/settings#git`, - }), - ); - }, - ); - await page.goto( - `${BASE_URL}/git/setup/callback?state=${state}&installation_id=77&setup_action=install`, - ); - - await expect.poll(() => completions.length).toBe(1); - expect(completions[0]).toEqual({ - state, - params: { installation_id: "77", setup_action: "install" }, - }); - await expect(page).toHaveURL( - new RegExp(`/projects/${projectId}/settings`), - ); - }); - }); -}); diff --git a/apps/e2e/tests/iam/git-permissions.spec.ts b/apps/e2e/tests/iam/git-permissions.spec.ts index 873c453d1..b5d352554 100644 --- a/apps/e2e/tests/iam/git-permissions.spec.ts +++ b/apps/e2e/tests/iam/git-permissions.spec.ts @@ -278,7 +278,7 @@ test.describe("Git integration permissions", () => { }); test.describe("Linking a repository", () => { - test("git:write alone cannot link a repository", async ({ + test("git:write alone passes the link gate", async ({ request, playwright, }) => { @@ -290,14 +290,14 @@ test.describe("Git integration permissions", () => { ["git:read", "git:write"], ); await asUser(playwright, writer.username, async (api) => { - expect( - ( - await api.post(gitUrl(projectId, "/repositories"), { - data: LINK_BODY, - }) - ).status(), - ).toBe(403); - // Everything else git:write covers is still open to the writer. + const response = await api.post(gitUrl(projectId, "/repositories"), { + data: LINK_BODY, + }); + // No provider is installed, so linking fails, but not for permissions: + // the connections it would go through were made available by whoever + // holds git.identities:manage, or by the project itself. + expect(refused(response), `status ${response.status()}`).toBe(false); + expect(response.ok()).toBe(false); expect( refused( await api.put(gitUrl(projectId, "/settings"), { @@ -308,6 +308,21 @@ test.describe("Git integration permissions", () => { }); }); + test("git:read cannot link a repository", async ({ request, playwright }) => { + const reader = await memberWith(request, playwright, projectId, "READER", [ + "git:read", + ]); + await asUser(playwright, reader.username, async (api) => { + expect( + ( + await api.post(gitUrl(projectId, "/repositories"), { + data: LINK_BODY, + }) + ).status(), + ).toBe(403); + }); + }); + test("git.identities:manage alone cannot link a repository", async ({ request, playwright, @@ -325,7 +340,40 @@ test.describe("Git integration permissions", () => { }); }); - test("git:write together with git.identities:manage passes the gate", async ({ + test("a project's own connection needs git:write and git.connections:manage", async ({ + request, + playwright, + }) => { + const setup = (api: APIRequestContext) => + api.post(gitUrl(projectId, "/connections/setup"), { + data: { provider_id: "com.paca.github" }, + }); + const writer = await memberWith(request, playwright, projectId, "CONN_W", [ + "git:read", + "git:write", + ]); + const connector = await memberWith(request, playwright, projectId, "CONN_M", [ + "git:read", + "git:write", + "git.connections:manage", + ]); + const onlyManage = await memberWith(request, playwright, projectId, "CONN_O", [ + "git:read", + "git.connections:manage", + ]); + await asUser(playwright, writer.username, async (api) => { + expect((await setup(api)).status()).toBe(403); + }); + await asUser(playwright, onlyManage.username, async (api) => { + expect((await setup(api)).status()).toBe(403); + }); + await asUser(playwright, connector.username, async (api) => { + // No provider is installed, so it fails, but not for permissions. + expect(refused(await setup(api))).toBe(false); + }); + }); + + test("the workspace action does not stand in for the project one", async ({ request, playwright, }) => { @@ -335,12 +383,13 @@ test.describe("Git integration permissions", () => { ]); await grantIdentitiesManage(request, both.userId, "BOTH"); await asUser(playwright, both.username, async (api) => { - const response = await api.post(gitUrl(projectId, "/repositories"), { - data: LINK_BODY, - }); - // No provider is installed, so linking fails, but not for permissions. - expect(refused(response), `status ${response.status()}`).toBe(false); - expect(response.ok()).toBe(false); + expect( + ( + await api.post(gitUrl(projectId, "/connections/setup"), { + data: { provider_id: "com.paca.github" }, + }) + ).status(), + ).toBe(403); }); }); }); diff --git a/apps/e2e/tests/projects/git-settings.spec.ts b/apps/e2e/tests/projects/git-settings.spec.ts index e31e4ba77..7b5d53e08 100644 --- a/apps/e2e/tests/projects/git-settings.spec.ts +++ b/apps/e2e/tests/projects/git-settings.spec.ts @@ -51,6 +51,19 @@ const LINKED = { created_at: "2026-10-01T00:00:00Z", }; +const CONNECTION = { + id: "00000000-0000-4000-8000-0000000000c1", + provider_id: PROVIDER.id, + host: "git.example.test", + kind: "app", + scope: "shared", + project_id: null, + handle: "paca-bot", + display_name: "Paca", + expires_at: null, + created_at: "2026-10-01T00:00:00Z", +}; + const ok = (data: unknown) => ({ status: 200, contentType: "application/json", @@ -65,9 +78,16 @@ const ok = (data: unknown) => ({ async function stubGit( page: Page, projectId: string, - opts: { providers: unknown[]; linkRequests?: unknown[] }, + opts: { + providers: unknown[]; + connections?: unknown[]; + linkRequests?: unknown[]; + }, ) { let repositories: unknown[] = []; + await page.route(`**/api/v1/projects/${projectId}/git/connections`, (route) => + route.fulfill(ok(opts.connections ?? [])), + ); await page.route(`**/api/v1/projects/${projectId}/git/providers`, (route) => route.fulfill(ok(opts.providers)), ); @@ -122,16 +142,17 @@ test.describe("Project Git settings", () => { await expect( page.getByText("No Git provider is installed", { exact: false }), ).toBeVisible(); - await expect( - page.getByRole("link", { name: "Open Git integration" }), - ).toBeVisible(); await expect(page.getByRole("button", { name: "Link" })).toHaveCount(0); }); test("An administrator links a repository", async ({ page }) => { const linkRequests: unknown[] = []; await signIn(page); - await stubGit(page, projectId, { providers: [PROVIDER], linkRequests }); + await stubGit(page, projectId, { + providers: [PROVIDER], + connections: [CONNECTION], + linkRequests, + }); await openGitSettings(page, projectId); await page.getByLabel("Repository").fill("acme/web"); @@ -180,7 +201,7 @@ test.describe("Project Git settings", () => { await signIn(page, member.username, RESTRICTED_PASSWORD); } - test("A writer without git.identities:manage is told to ask an administrator", async ({ + test("A writer is told to ask for a connection when there is none", async ({ page, request, playwright, @@ -193,26 +214,26 @@ test.describe("Project Git settings", () => { await openGitSettings(page, projectId); await expect( - page.getByText("manage Git identities", { exact: false }), + page.getByText("Ask someone who can manage Git connections", { + exact: false, + }), ).toBeVisible(); - await expect(page.getByLabel("Repository")).toHaveCount(0); await expect(page.getByRole("button", { name: "Link" })).toHaveCount(0); }); - test("A writer with git.identities:manage gets the form", async ({ + test("A writer gets the form once a connection is available", async ({ page, request, playwright, }) => { - await signInAsMember( - page, - request, - playwright, - "MANAGER", - ["git:read", "git:write"], - ["git.identities:manage"], - ); - await stubGit(page, projectId, { providers: [PROVIDER] }); + await signInAsMember(page, request, playwright, "MANAGER", [ + "git:read", + "git:write", + ]); + await stubGit(page, projectId, { + providers: [PROVIDER], + connections: [CONNECTION], + }); await openGitSettings(page, projectId); await expect(page.getByLabel("Repository")).toBeVisible(); diff --git a/apps/web/src/components/app-shell/app-sidebar.tsx b/apps/web/src/components/app-shell/app-sidebar.tsx index e0b90bede..8bd9796a6 100644 --- a/apps/web/src/components/app-shell/app-sidebar.tsx +++ b/apps/web/src/components/app-shell/app-sidebar.tsx @@ -25,7 +25,6 @@ import { FolderKanban, FolderOpen, GanttChart, - GitBranch, Home, KanbanSquare, Loader2, @@ -1817,8 +1816,6 @@ export function AppSidebar() { const canAccessSettings = hasPermission("settings:write") || hasPermission("settings.sso:write"); - const canAccessGit = hasPermission("git.identities:manage"); - const canCreateProject = hasPermission("projects:create"); // A plugin admin nav item is only listed for callers who hold its @@ -1836,7 +1833,6 @@ export function AppSidebar() { canAccessGlobalAgents || canAccessServiceAccounts || canAccessPlugins || - canAccessGit || canAccessSettings; // Plugin-contributed admin pages get their own sidebar section, separate // from core workspace administration. @@ -1973,13 +1969,6 @@ export function AppSidebar() { exact /> ) : null} - {canAccessGit ? ( - - ) : null} {canAccessSettings ? ( ({ - mockProviders: vi.fn(), - mockIdentities: vi.fn(), - mockBegin: vi.fn(), - mockFollow: vi.fn(), - mockDelete: vi.fn(), - })); - -// The query options inside git-api call its own listGit* functions, which a -// module mock cannot replace, so the two list endpoints are mocked at the -// HTTP client instead. -vi.mock("@tanstack/react-router", async () => { - const React = await import("react"); - return { - Link: ({ - to, - children, - ...rest - }: { - to: string; - children?: React.ReactNode; - }) => React.createElement("a", { href: to, ...rest }, children), - }; -}); - -vi.mock("@/lib/api-client", () => ({ - apiClient: { - instance: { - get: async (url: string) => { - if (url === "/git/providers") - return { data: { data: await mockProviders() } }; - if (url === "/git/identities") - return { data: { data: await mockIdentities() } }; - throw new Error(`unexpected GET ${url}`); - }, - }, - }, -})); - -vi.mock("@/lib/git-api", async (orig) => ({ - ...(await orig()), - beginGitSetup: mockBegin, - followGitStep: mockFollow, - deleteGitIdentity: mockDelete, -})); - -import { GitIdentitiesPanel } from "./GitIdentitiesPanel"; - -const github = { - id: "com.paca.github", - display_name: "GitHub", - hosts: ["github.com"], - self_hosted_hosts: false, - terms: { - change_request: "Pull request", - change_request_plural: "Pull requests", - repository: "Repository", - }, - capabilities: { multiple_identities: true }, -}; -const shared = { - id: "i1", - provider_id: "com.paca.github", - host: "github.com", - kind: "app", - scope: "shared", - handle: "paca-bot", - display_name: "Paca", - expires_at: null, - created_at: "2026-01-01T00:00:00Z", -}; - -function renderPanel() { - const client = new QueryClient({ - defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, - }); - return render( - - - , - ); -} - -describe("GitIdentitiesPanel", () => { - beforeEach(() => { - vi.resetAllMocks(); - mockFollow.mockReturnValue(true); - }); - - it("tells the admin when no provider is installed", async () => { - mockProviders.mockResolvedValue([]); - mockIdentities.mockResolvedValue([]); - renderPanel(); - expect( - await screen.findByText(/no git provider installed/i), - ).toBeInTheDocument(); - }); - - it("starts setup with the instance's return URL and hands the step to the browser", async () => { - mockProviders.mockResolvedValue([github]); - mockIdentities.mockResolvedValue([]); - const step = { - kind: "form_post", - url: "https://github.com/settings/apps/new", - fields: { manifest: "{}" }, - state: "s", - }; - mockBegin.mockResolvedValue(step); - const user = userEvent.setup(); - renderPanel(); - - await user.click(await screen.findByRole("button", { name: "Set up" })); - await user.type(screen.getByLabelText(/app name/i), " Acme Paca "); - await user.type(screen.getByLabelText(/organization/i), "acme"); - await user.click( - screen.getByRole("button", { name: /continue to github\.com/i }), - ); - - await waitFor(() => expect(mockFollow).toHaveBeenCalledWith(step)); - expect(mockBegin).toHaveBeenCalledWith({ - provider_id: "com.paca.github", - host: "github.com", - return_url: `${window.location.origin}/admin/git`, - params: { name: "Acme Paca", owner_org: "acme" }, - }); - }); - - it("omits parameters the admin left empty", async () => { - mockProviders.mockResolvedValue([github]); - mockIdentities.mockResolvedValue([]); - mockBegin.mockResolvedValue({ kind: "redirect", url: "u", state: "s" }); - const user = userEvent.setup(); - renderPanel(); - await user.click(await screen.findByRole("button", { name: "Set up" })); - await user.click( - screen.getByRole("button", { name: /continue to github\.com/i }), - ); - await waitFor(() => expect(mockBegin).toHaveBeenCalled()); - expect(mockBegin.mock.calls[0][0].params).toEqual({}); - }); - - it("shows why setup could not start", async () => { - mockProviders.mockResolvedValue([github]); - mockIdentities.mockResolvedValue([]); - mockBegin.mockRejectedValue({ - response: { data: { error_code: "GIT_PUBLIC_URL_REQUIRED" } }, - }); - const user = userEvent.setup(); - renderPanel(); - await user.click(await screen.findByRole("button", { name: "Set up" })); - await user.click( - screen.getByRole("button", { name: /continue to github\.com/i }), - ); - expect(await screen.findByRole("alert")).toHaveTextContent(/PUBLIC_URL/); - expect(mockFollow).not.toHaveBeenCalled(); - }); - - it("answers a token form in place", async () => { - mockProviders.mockResolvedValue([github]); - mockIdentities.mockResolvedValue([]); - mockBegin.mockResolvedValue({ - kind: "token_form", - prompt: "Paste a token with repo scope", - state: "s", - form: [ - { name: "token", label: "Access token", secret: true, required: true }, - ], - }); - const user = userEvent.setup(); - renderPanel(); - await user.click(await screen.findByRole("button", { name: "Set up" })); - await user.click( - screen.getByRole("button", { name: /continue to github\.com/i }), - ); - const field = await screen.findByLabelText("Access token"); - expect(field).toHaveAttribute("type", "password"); - expect( - screen.getByText(/paste a token with repo scope/i), - ).toBeInTheDocument(); - expect(mockFollow).not.toHaveBeenCalled(); - }); - - it("shows an existing identity and removes it after confirmation", async () => { - mockProviders.mockResolvedValue([github]); - mockIdentities.mockResolvedValue([shared]); - mockDelete.mockResolvedValue(undefined); - const user = userEvent.setup(); - renderPanel(); - - expect(await screen.findByText("@paca-bot")).toBeInTheDocument(); - expect( - screen.queryByRole("button", { name: "Set up" }), - ).not.toBeInTheDocument(); - await user.click(screen.getByRole("button", { name: "Remove" })); - expect(mockDelete).not.toHaveBeenCalled(); - await user.click( - await screen.findByRole("button", { name: "Remove identity" }), - ); - await waitFor(() => expect(mockDelete).toHaveBeenCalledWith("i1")); - }); - - it("does not treat an agent's own identity as the host's shared one", async () => { - mockProviders.mockResolvedValue([github]); - mockIdentities.mockResolvedValue([{ ...shared, scope: "agent" }]); - renderPanel(); - expect( - await screen.findByRole("button", { name: "Set up" }), - ).toBeInTheDocument(); - }); -}); diff --git a/apps/web/src/components/git/GitIdentitiesPanel.tsx b/apps/web/src/components/git/GitIdentitiesPanel.tsx deleted file mode 100644 index 8a4b1b30c..000000000 --- a/apps/web/src/components/git/GitIdentitiesPanel.tsx +++ /dev/null @@ -1,212 +0,0 @@ -import { useQuery } from "@tanstack/react-query"; -import { Link } from "@tanstack/react-router"; -import { GitBranch, Plug, Trash2 } from "lucide-react"; -import { useState } from "react"; -import { useTranslation } from "react-i18next"; -import { Badge } from "@/components/ui/badge"; -import { Button } from "@/components/ui/button"; -import { Card, CardContent } from "@/components/ui/card"; -import { Skeleton } from "@/components/ui/skeleton"; -import { - type GitIdentity, - type GitProvider, - gitIdentitiesQueryOptions, - gitProvidersQueryOptions, -} from "@/lib/git-api"; -import { DeleteGitIdentityDialog } from "./DeleteGitIdentityDialog"; -import { GitSetupDialog } from "./GitSetupDialog"; -import { CopyChip, EmptyState, Section } from "./git-ui"; - -/** - * The workspace's Git page. A newcomer sees what the feature does and the three - * steps to get it working; each platform card says whether it is connected and - * what to do next. A connected platform shows the bot's @handle to mention. - */ -export function GitIdentitiesPanel() { - const { t } = useTranslation("admin"); - const providers = useQuery(gitProvidersQueryOptions); - const identities = useQuery(gitIdentitiesQueryOptions); - const [setup, setSetup] = useState<{ - provider: GitProvider; - host: string; - } | null>(null); - const [removing, setRemoving] = useState(null); - - if (providers.isLoading || identities.isLoading) { - return ; - } - if (providers.isError || identities.isError) { - return ( -

- {t("git.errors.generic")} -

- ); - } - const list = providers.data ?? []; - - if (list.length === 0) { - return ( - - - - ); - } - - const sharedFor = (providerId: string, host: string) => - (identities.data ?? []).find( - (i) => - i.provider_id === providerId && - i.scope === "shared" && - i.host.toLowerCase() === host.toLowerCase(), - ); - return ( -
-
-
- {list.map((provider) => - provider.hosts.map((host) => { - const identity = sharedFor(provider.id, host); - return ( - - -
-
- - -
-

- {provider.display_name} -

-

- {host} -

-
-
- - {identity - ? t("git.status.connected") - : t("git.status.notConnected")} - -
- - {identity ? ( - setRemoving(identity)} - /> - ) : ( - setSetup({ provider, host })} - /> - )} - - - ); - }), - )} -
-
- - {setup ? ( - { - if (!open) setSetup(null); - }} - /> - ) : null} - {removing ? ( - { - if (!open) setRemoving(null); - }} - /> - ) : null} -
- ); -} - -function NotConnected({ - provider, - host, - onConnect, -}: { - provider: GitProvider; - host: string; - onConnect: () => void; -}) { - const { t } = useTranslation("admin"); - return ( -
-
-

- {t("git.connect.title", { provider: provider.display_name })} -

-

- {t("git.connect.description", { host })} -

-
-
- - - {t("git.connect.time")} - -
-
- ); -} - -function Connected({ - identity, - onRemove, -}: { - identity: GitIdentity; - onRemove: () => void; -}) { - const { t } = useTranslation("admin"); - const name = identity.display_name || identity.handle; - return ( -
-
-
-

{name}

-

- {t("git.connected.sharedNote")} -

-
- {identity.handle ? ( -
- - {t("git.connected.mention")} - - -
- ) : null} -
-
- -
-
- ); -} diff --git a/apps/web/src/components/git/GitSetupCallback.test.tsx b/apps/web/src/components/git/GitSetupCallback.test.tsx index 82a57172c..46c67074b 100644 --- a/apps/web/src/components/git/GitSetupCallback.test.tsx +++ b/apps/web/src/components/git/GitSetupCallback.test.tsx @@ -3,17 +3,24 @@ import { render, screen, waitFor } from "@testing-library/react"; import type { ReactNode } from "react"; import { beforeEach, describe, expect, it, vi } from "vitest"; -const { mockComplete, mockCompleteAccess, mockFollow, mockBeginAgentAccess } = - vi.hoisted(() => ({ - mockBeginAgentAccess: vi.fn(), - mockComplete: vi.fn(), - mockCompleteAccess: vi.fn(), - mockFollow: vi.fn(), - })); +const { + mockComplete, + mockCompleteProject, + mockCompleteAccess, + mockFollow, + mockBeginAgentAccess, +} = vi.hoisted(() => ({ + mockBeginAgentAccess: vi.fn(), + mockComplete: vi.fn(), + mockCompleteProject: vi.fn(), + mockCompleteAccess: vi.fn(), + mockFollow: vi.fn(), +})); vi.mock("@/lib/git-api", async (orig) => ({ ...(await orig()), completeGitSetup: mockComplete, + completeProjectGitSetup: mockCompleteProject, completeGitAccess: mockCompleteAccess, beginAgentGitAccess: mockBeginAgentAccess, followGitStep: mockFollow, @@ -144,9 +151,51 @@ describe("GitSetupCallback", () => { expect(await screen.findByRole("alert")).toHaveTextContent( /invalid, expired or already used/i, ); - expect( - screen.getByRole("link", { name: /back to git settings/i }), - ).toHaveAttribute("href", "/admin/git"); + const assign = vi.fn(); + const original = window.location; + Object.defineProperty(window, "location", { + configurable: true, + value: { ...original, assign }, + }); + try { + screen.getByRole("button", { name: /back to git settings/i }).click(); + expect(assign).toHaveBeenCalledWith("/home"); + } finally { + Object.defineProperty(window, "location", { + configurable: true, + value: original, + }); + } + }); + + it("completes a project's own connection through the project route and goes back there on failure", async () => { + rememberGitFlow("st-proj", { kind: "project-identity", projectId: "p9" }); + mockCompleteProject.mockRejectedValue({ + response: { data: { error_code: "GIT_SETUP_INVALID" } }, + }); + renderCallback(uniq("?code=c&state=st-proj")); + expect(await screen.findByRole("alert")).toBeInTheDocument(); + expect(mockCompleteProject).toHaveBeenCalledWith("p9", "st-proj", { + code: "c", + t: expect.any(String), + }); + expect(mockComplete).not.toHaveBeenCalled(); + + const assign = vi.fn(); + const original = window.location; + Object.defineProperty(window, "location", { + configurable: true, + value: { ...original, assign }, + }); + try { + screen.getByRole("button", { name: /back to git settings/i }).click(); + expect(assign).toHaveBeenCalledWith("/projects/p9/settings#git"); + } finally { + Object.defineProperty(window, "location", { + configurable: true, + value: original, + }); + } }); it("does not call the API without a state", async () => { diff --git a/apps/web/src/components/git/GitSetupCallback.tsx b/apps/web/src/components/git/GitSetupCallback.tsx index 7dadf8594..7dd138830 100644 --- a/apps/web/src/components/git/GitSetupCallback.tsx +++ b/apps/web/src/components/git/GitSetupCallback.tsx @@ -1,5 +1,4 @@ import { useQueryClient } from "@tanstack/react-query"; -import { Link } from "@tanstack/react-router"; import { AlertCircle, Loader2 } from "lucide-react"; import { useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; @@ -8,13 +7,15 @@ import { beginAgentGitAccess, completeGitAccess, completeGitSetup, + completeProjectGitSetup, followGitStep, forgetGitFlow, - GIT_ADMIN_PATH, + GIT_FALLBACK_PATH, type GitAccessCompletion, type GitSetupCompletion, gitErrorKey, - gitIdentitiesKey, + gitQueriesKey, + projectGitSettingsPath, readGitFlow, rememberGitFlow, returnPathOf, @@ -49,7 +50,8 @@ export function parseCallback(search: string): { /** * Completes whichever flow started this round trip: a project's access grant - * (installing the App on a repository owner's account) or an identity setup. + * (installing the App on a repository owner's account), a project's own + * connection, or a workspace connection or agent identity. * The browser's note of the flow only routes the call; the API checks the * state, its owner and its project itself. */ @@ -58,16 +60,22 @@ function complete(search: string): Promise { if (existing) return existing; const { state, params } = parseCallback(search); const hint = readGitFlow(state); - const p: Promise = - hint.kind === "access" - ? completeGitAccess(hint.projectId, state, params).then((result) => ({ - flow: "access" as const, - result, - })) - : completeGitSetup(state, params).then((result) => ({ - flow: "identity" as const, - result, - })); + const identity = (call: Promise) => + call.then((result) => ({ flow: "identity" as const, result })); + let p: Promise; + switch (hint.kind) { + case "access": + p = completeGitAccess(hint.projectId, state, params).then((result) => ({ + flow: "access" as const, + result, + })); + break; + case "project-identity": + p = identity(completeProjectGitSetup(hint.projectId, state, params)); + break; + default: + p = identity(completeGitSetup(state, params)); + } inflight.set(search, p); return p; } @@ -86,6 +94,12 @@ export function GitSetupCallback({ search }: { search: string }) { const { t } = useTranslation("admin"); const queryClient = useQueryClient(); const [errorKey, setErrorKey] = useState(null); + // Where "back" goes when something fails: the project the flow started from. + const hint = readGitFlow(parseCallback(search).state); + const backTo = + hint.kind === "identity" + ? GIT_FALLBACK_PATH + : projectGitSettingsPath(hint.projectId); useEffect(() => { let cancelled = false; @@ -101,7 +115,7 @@ export function GitSetupCallback({ search }: { search: string }) { window.location.replace(returnPathOf(out.result.return_url)); return; } - void queryClient.invalidateQueries({ queryKey: gitIdentitiesKey }); + void queryClient.invalidateQueries({ queryKey: gitQueriesKey }); if (out.result.next) { if (!followGitStep(out.result.next)) setErrorKey("generic"); return; @@ -153,7 +167,7 @@ export function GitSetupCallback({ search }: { search: string }) {

{t(`git.errors.${errorKey}` as never)}

- diff --git a/apps/web/src/components/git/GitSetupDialog.tsx b/apps/web/src/components/git/GitSetupDialog.tsx index 348c14e7c..fa7072603 100644 --- a/apps/web/src/components/git/GitSetupDialog.tsx +++ b/apps/web/src/components/git/GitSetupDialog.tsx @@ -18,14 +18,23 @@ import { Switch } from "@/components/ui/switch"; import { type BeginGitSetupPayload, beginGitSetup, + beginProjectGitSetup, completeGitSetup, + completeProjectGitSetup, + connectionKindsOf, followGitStep, - GIT_ADMIN_PATH, + type GitConnectionKind, type GitProvider, type GitSetupStep, gitErrorKey, - gitIdentitiesKey, + gitQueriesKey, + projectGitSettingsPath, + rememberGitFlow, } from "@/lib/git-api"; +import { ChoiceCard } from "./git-ui"; + +/** Who may use a connection: one project, or every project. */ +type Visibility = "project" | "workspace"; interface GitSetupDialogProps { provider: GitProvider; @@ -33,10 +42,21 @@ interface GitSetupDialogProps { open: boolean; onOpenChange: (open: boolean) => void; /** - * Starts the setup; defaults to the workspace's shared identity. An agent's - * own identity passes its own endpoint. + * Starts an agent's own identity; takes the agent's endpoint. Leave it out + * and pass `project` to add a connection from a project's settings. */ begin?: (payload: BeginGitSetupPayload) => Promise; + /** + * Adds a connection from a project's settings. The user chooses whether it + * serves this project only or every project, among what they may do. + */ + project?: { + id: string; + /** git.connections:manage on the project. */ + canProject: boolean; + /** git.identities:manage, the workspace-level action. */ + canWorkspace: boolean; + }; /** Where the browser returns when setup is finished (a same-origin path). */ returnPath?: string; /** Query cache entry to refresh when a token form finishes in place. */ @@ -44,24 +64,32 @@ interface GitSetupDialogProps { } /** - * Starts creating the shared bot identity for one provider host. The - * provider decides what happens next: a redirect or a form post sends the - * browser to the platform (which returns to /git/setup/callback), a token - * form is answered right here. The optional name and organization are passed - * through as provider parameters; a provider that has no use for them ignores - * them. + * Starts creating a connection to one provider host. The provider decides what + * happens next: a redirect or a form post sends the browser to the platform + * (which returns to /git/setup/callback), a token form is answered right here. + * The optional name and organization are passed through as provider parameters; + * a provider that has no use for them ignores them. */ export function GitSetupDialog({ provider, - host, + host: defaultHost, open, onOpenChange, - begin = beginGitSetup, - returnPath = GIT_ADMIN_PATH, - invalidateKey = gitIdentitiesKey, + begin, + project, + returnPath = "/home", + invalidateKey = gitQueriesKey, }: GitSetupDialogProps) { const { t } = useTranslation("admin"); const queryClient = useQueryClient(); + const kinds = connectionKindsOf(provider); + const [kind, setKind] = useState(kinds[0]); + const [host, setHost] = useState(defaultHost); + const [visibility, setVisibility] = useState( + project && !project.canProject && project.canWorkspace + ? "workspace" + : "project", + ); const [name, setName] = useState(""); const [org, setOrg] = useState(""); const [isPublic, setIsPublic] = useState(false); @@ -69,18 +97,31 @@ export function GitSetupDialog({ const [tokenStep, setTokenStep] = useState(null); const [tokenValues, setTokenValues] = useState>({}); + const forProject = project && visibility === "project" ? project : null; + const target = forProject + ? { kind: "project-identity" as const, projectId: forProject.id } + : { kind: "identity" as const }; + const back = forProject ? projectGitSettingsPath(forProject.id) : returnPath; + const start = useMutation({ mutationFn: () => { const params: Record = {}; - if (name.trim()) params.name = name.trim(); - if (org.trim()) params.owner_org = org.trim(); - if (isPublic) params.public = "true"; - return begin({ + if (kinds.length > 1) params.kind = kind; + if (kind === "app") { + if (name.trim()) params.name = name.trim(); + if (org.trim()) params.owner_org = org.trim(); + if (isPublic) params.public = "true"; + } + const payload = { provider_id: provider.id, - host, - return_url: `${window.location.origin}${returnPath}`, + host: host.trim() || defaultHost, + return_url: `${window.location.origin}${back}`, params, - }); + }; + if (begin) return begin(payload); + return forProject + ? beginProjectGitSetup(forProject.id, payload) + : beginGitSetup(payload); }, onSuccess: (step) => { if (step.kind === "token_form") { @@ -92,6 +133,7 @@ export function GitSetupDialog({ onOpenChange(false); return; } + rememberGitFlow(step.state, target); // Leaves the page; the platform returns to /git/setup/callback. if (!followGitStep(step)) setError(t("git.errors.generic")); }, @@ -101,7 +143,12 @@ export function GitSetupDialog({ }); const submitToken = useMutation({ - mutationFn: () => completeGitSetup(tokenStep?.state ?? "", tokenValues), + mutationFn: () => { + const state = tokenStep?.state ?? ""; + return forProject + ? completeProjectGitSetup(forProject.id, state, tokenValues) + : completeGitSetup(state, tokenValues); + }, onSuccess: () => { void queryClient.invalidateQueries({ queryKey: invalidateKey }); onOpenChange(false); @@ -112,10 +159,19 @@ export function GitSetupDialog({ }); const busy = start.isPending || submitToken.isPending; + const canStart = !project || project.canProject || project.canWorkspace; const title = t("git.setup.dialogTitle", { provider: provider.display_name, host, }); + const description = + tokenStep?.prompt ?? + (kind === "app" + ? t("git.setup.dialogDescription", { + host, + changeRequests: provider.terms.change_request_plural.toLowerCase(), + }) + : t("git.setup.tokenDescription", { host })); return (
{ e.preventDefault(); setError(null); @@ -144,13 +200,8 @@ export function GitSetupDialog({ {title} - - {tokenStep?.prompt ?? - t("git.setup.dialogDescription", { - host, - changeRequests: - provider.terms.change_request_plural.toLowerCase(), - })} + + {description} @@ -175,45 +226,122 @@ export function GitSetupDialog({ )) ) : ( <> -
- - setName(e.target.value)} - /> -

- {t("git.setup.nameHelp")} -

-
-
- - setOrg(e.target.value)} - /> -

- {t("git.setup.orgHelp")} -

-
-
- -
- + {provider.self_hosted_hosts ? ( +
+ + setHost(e.target.value)} + />

- {t("git.setup.publicHelp")} + {t("git.setup.hostHelp")}

-
+ ) : null} + + {kinds.length > 1 ? ( +
+ + {t("git.setup.kind.label")} + + {kinds.map((k) => ( + setKind(k)} + title={t(`git.setup.kind.${k}.title` as never)} + description={t(`git.setup.kind.${k}.help` as never, { + provider: provider.display_name, + })} + /> + ))} +
+ ) : null} + + {project ? ( +
+ + {t("git.setup.scope.label")} + + setVisibility("project")} + title={t("git.setup.scope.project.title")} + description={t("git.setup.scope.project.help")} + /> + setVisibility("workspace")} + title={t("git.setup.scope.workspace.title")} + description={ + project.canWorkspace + ? t("git.setup.scope.workspace.help") + : t("git.setup.scope.workspace.noPermission") + } + /> + {!canStart ? ( +

+ {t("git.setup.scope.none")} +

+ ) : null} +
+ ) : null} + + {kind === "app" ? ( + <> +
+ + setName(e.target.value)} + /> +

+ {t("git.setup.nameHelp")} +

+
+
+ + setOrg(e.target.value)} + /> +

+ {t("git.setup.orgHelp")} +

+
+
+ +
+ +

+ {t("git.setup.publicHelp")} +

+
+
+ + ) : null} )} @@ -235,11 +363,13 @@ export function GitSetupDialog({ > {t("git.cancel")} - diff --git a/apps/web/src/components/git/git-ui.tsx b/apps/web/src/components/git/git-ui.tsx index b6dc1fe8b..9a7d07d06 100644 --- a/apps/web/src/components/git/git-ui.tsx +++ b/apps/web/src/components/git/git-ui.tsx @@ -115,3 +115,55 @@ export function EmptyState({
); } + +/** + * One option of a small, mutually exclusive choice, drawn as a bordered card + * around a native radio so keyboards and screen readers work as usual. + */ +export function ChoiceCard({ + name, + value, + checked, + disabled, + onSelect, + title, + description, +}: { + name: string; + value: string; + checked: boolean; + disabled?: boolean; + onSelect: (value: string) => void; + title: string; + description?: string; +}) { + return ( + + ); +} diff --git a/apps/web/src/components/projects/agents/AgentGitTab.test.tsx b/apps/web/src/components/projects/agents/AgentGitTab.test.tsx index fe1f9eaaf..ae76ea085 100644 --- a/apps/web/src/components/projects/agents/AgentGitTab.test.tsx +++ b/apps/web/src/components/projects/agents/AgentGitTab.test.tsx @@ -235,7 +235,7 @@ describe("AgentGitTab", () => { renderTab(); await user.click(await screen.findByRole("button", { name: "Install" })); expect(await screen.findByRole("alert")).toHaveTextContent( - /No shared Git identity|identity/i, + /No connection is available/i, ); expect(h.follow).not.toHaveBeenCalled(); }); @@ -250,7 +250,7 @@ describe("AgentGitTab", () => { await user.click(await screen.findByRole("button", { name: "Remove" })); expect(h.del).not.toHaveBeenCalled(); await user.click( - await screen.findByRole("button", { name: "Remove identity" }), + await screen.findByRole("button", { name: "Remove connection" }), ); await waitFor(() => expect(h.del).toHaveBeenCalledWith( diff --git a/apps/web/src/components/projects/roles/permissions.ts b/apps/web/src/components/projects/roles/permissions.ts index 323abf73f..e85f3c058 100644 --- a/apps/web/src/components/projects/roles/permissions.ts +++ b/apps/web/src/components/projects/roles/permissions.ts @@ -205,6 +205,12 @@ export const PROJECT_KNOWN_PERMISSIONS = [ descriptionKey: "roles.permissions.gitMerge.description", domain: "git", }, + { + key: "git.connections:manage", + labelKey: "roles.permissions.gitConnectionsManage.label", + descriptionKey: "roles.permissions.gitConnectionsManage.description", + domain: "git", + }, // conversations { key: "conversations:read", diff --git a/apps/web/src/components/projects/settings/GitConnections.test.tsx b/apps/web/src/components/projects/settings/GitConnections.test.tsx new file mode 100644 index 000000000..8bc497a46 --- /dev/null +++ b/apps/web/src/components/projects/settings/GitConnections.test.tsx @@ -0,0 +1,278 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { render, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const h = vi.hoisted(() => ({ + get: vi.fn(), + post: vi.fn(), + del: vi.fn(), + follow: vi.fn(), +})); +const perms = vi.hoisted(() => ({ workspace: true, project: true })); + +vi.mock("@/hooks/use-permissions", () => ({ + usePermissions: () => ({ + hasPermission: (a: string) => + a === "git.identities:manage" ? perms.workspace : false, + }), +})); +vi.mock("@/hooks/use-effective-actions", () => ({ + useEffectiveActions: () => ({ + has: (a: string) => + a === "git.connections:manage" || a === "git:write" + ? perms.project + : false, + }), +})); +vi.mock("@/lib/api-client", () => ({ + apiClient: { instance: { get: h.get, post: h.post, delete: h.del } }, +})); +vi.mock("@/lib/git-api", async (orig) => ({ + ...(await orig()), + followGitStep: h.follow, +})); + +import type { GitProvider } from "@/lib/git-api"; +import { GitConnections } from "./GitConnections"; + +const PROJECT = "p1"; +const github: GitProvider = { + id: "com.paca.github", + display_name: "GitHub", + hosts: ["github.com"], + self_hosted_hosts: false, + terms: { + change_request: "Pull request", + change_request_plural: "Pull requests", + repository: "Repository", + }, + capabilities: { + multiple_identities: true, + access_grants: true, + identity_kinds: ["app", "token"], + kinds: [ + { kind: "app", access_grants: true }, + { kind: "token", access_grants: false }, + ], + }, +}; +const gitlab: GitProvider = { + ...github, + id: "com.paca.gitlab", + display_name: "GitLab", + hosts: ["gitlab.com"], + self_hosted_hosts: true, + capabilities: { + multiple_identities: true, + access_grants: false, + identity_kinds: ["bot_user"], + }, +}; +const shared = { + id: "w1", + provider_id: "com.paca.github", + host: "github.com", + kind: "app", + scope: "shared", + project_id: null, + handle: "paca-bot", + display_name: "Paca", + expires_at: null, + created_at: "2026-01-01T00:00:00Z", +}; +const own = { + ...shared, + id: "p-1", + kind: "token", + scope: "project", + project_id: PROJECT, + handle: "ci-bot", + display_name: "CI bot", +}; + +let connections: unknown[] = []; + +function renderConnections(providers: GitProvider[] = [github]) { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return render( + + + , + ); +} + +describe("GitConnections", () => { + beforeEach(() => { + vi.resetAllMocks(); + window.sessionStorage.clear(); + perms.workspace = true; + perms.project = true; + connections = [shared, own]; + h.follow.mockReturnValue(true); + h.get.mockImplementation(async (url: string) => { + if (url.endsWith("/git/connections")) + return { data: { data: connections } }; + throw new Error(`unexpected GET ${url}`); + }); + }); + + it("removes a project's own connection through the project route", async () => { + const user = userEvent.setup(); + h.del.mockResolvedValue({}); + renderConnections(); + const row = (await screen.findByText("CI bot")).closest("li"); + if (!row) throw new Error("row not found"); + await user.click(within(row).getByRole("button", { name: "Remove" })); + await user.click( + await screen.findByRole("button", { name: "Remove connection" }), + ); + await waitFor(() => + expect(h.del).toHaveBeenCalledWith( + `/projects/${PROJECT}/git/connections/p-1`, + ), + ); + }); + + it("removes a shared connection through the workspace route", async () => { + const user = userEvent.setup(); + h.del.mockResolvedValue({}); + renderConnections(); + const row = (await screen.findByText("Paca")).closest("li"); + if (!row) throw new Error("row not found"); + await user.click(within(row).getByRole("button", { name: "Remove" })); + await user.click( + await screen.findByRole("button", { name: "Remove connection" }), + ); + await waitFor(() => + expect(h.del).toHaveBeenCalledWith("/git/identities/w1"), + ); + }); + + it("offers removal only for what the user may manage", async () => { + perms.workspace = false; // may manage the project's own, not the shared one + renderConnections(); + const shared = (await screen.findByText("Paca")).closest("li"); + const own = screen.getByText("CI bot").closest("li"); + if (!shared || !own) throw new Error("rows not found"); + expect(within(shared).queryByRole("button", { name: "Remove" })).toBeNull(); + expect(within(own).getByRole("button", { name: "Remove" })).toBeEnabled(); + }); + + it("adds a project-only access token: kind and scope go to the project route", async () => { + const user = userEvent.setup(); + h.post.mockImplementation(async (url: string) => { + if (url.endsWith("/connections/setup")) + return { + data: { + data: { + kind: "token_form", + state: "st1", + prompt: "Paste a token", + form: [ + { name: "token", label: "Token", secret: true, required: true }, + ], + }, + }, + }; + return { data: { data: { identity: own, next: null, done: true } } }; + }); + renderConnections(); + await user.click(await screen.findByRole("button", { name: "Add GitHub" })); + await user.click(screen.getByRole("radio", { name: /Access token/ })); + expect( + screen.getByRole("radio", { name: /This project only/ }), + ).toBeChecked(); + await user.click(screen.getByRole("button", { name: "Next" })); + + await waitFor(() => + expect(h.post).toHaveBeenCalledWith( + `/projects/${PROJECT}/git/connections/setup`, + expect.objectContaining({ + provider_id: "com.paca.github", + host: "github.com", + params: { kind: "token" }, + }), + ), + ); + await user.type(await screen.findByLabelText("Token"), "ghp_secret"); + await user.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => + expect(h.post).toHaveBeenLastCalledWith( + `/projects/${PROJECT}/git/connections/setup/complete`, + { state: "st1", params: { token: "ghp_secret" } }, + ), + ); + }); + + it("adds a workspace-wide App through the workspace route and remembers the flow", async () => { + const user = userEvent.setup(); + h.post.mockResolvedValue({ + data: { + data: { + kind: "form_post", + url: "https://github.com/settings/apps/new", + state: "g9", + }, + }, + }); + renderConnections(); + await user.click(await screen.findByRole("button", { name: "Add GitHub" })); + await user.click(screen.getByRole("radio", { name: /All projects/ })); + await user.click( + screen.getByRole("button", { name: /Continue to github.com/ }), + ); + await waitFor(() => expect(h.follow).toHaveBeenCalled()); + expect(h.post).toHaveBeenCalledWith( + "/git/identities/setup", + expect.objectContaining({ params: { kind: "app" } }), + ); + expect(window.sessionStorage.getItem("paca.git.flow.g9")).toBe( + JSON.stringify({ kind: "identity" }), + ); + }); + + it("remembers a project connection's flow so the callback completes the right one", async () => { + const user = userEvent.setup(); + h.post.mockResolvedValue({ + data: { + data: { kind: "form_post", url: "https://github.com/x", state: "g8" }, + }, + }); + renderConnections(); + await user.click(await screen.findByRole("button", { name: "Add GitHub" })); + await user.click( + screen.getByRole("button", { name: /Continue to github.com/ }), + ); + await waitFor(() => expect(h.follow).toHaveBeenCalled()); + expect( + JSON.parse(window.sessionStorage.getItem("paca.git.flow.g8") ?? "{}"), + ).toEqual({ + kind: "project-identity", + projectId: PROJECT, + }); + }); + + it("disables the scopes the user may not use", async () => { + perms.workspace = false; + const user = userEvent.setup(); + renderConnections(); + await user.click(await screen.findByRole("button", { name: "Add GitHub" })); + expect(screen.getByRole("radio", { name: /All projects/ })).toBeDisabled(); + expect( + screen.getByRole("radio", { name: /This project only/ }), + ).toBeEnabled(); + }); + + it("asks only what a single-kind provider needs", async () => { + const user = userEvent.setup(); + connections = []; + renderConnections([gitlab]); + await user.click(await screen.findByRole("button", { name: "Add GitLab" })); + expect(screen.queryByRole("radio", { name: /Access token/ })).toBeNull(); // no kind to pick + expect(screen.getByLabelText("Host")).toHaveValue("gitlab.com"); // self-hosted + expect(screen.queryByLabelText(/App name/i)).toBeNull(); // not an App + }); +}); diff --git a/apps/web/src/components/projects/settings/GitConnections.tsx b/apps/web/src/components/projects/settings/GitConnections.tsx new file mode 100644 index 000000000..8236caeb5 --- /dev/null +++ b/apps/web/src/components/projects/settings/GitConnections.tsx @@ -0,0 +1,208 @@ +import { useQuery } from "@tanstack/react-query"; +import { KeyRound, Plug, Plus, Trash2 } from "lucide-react"; +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { DeleteGitIdentityDialog } from "@/components/git/DeleteGitIdentityDialog"; +import { GitSetupDialog } from "@/components/git/GitSetupDialog"; +import { CopyChip, EmptyState, Section } from "@/components/git/git-ui"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { Skeleton } from "@/components/ui/skeleton"; +import { useEffectiveActions } from "@/hooks/use-effective-actions"; +import { usePermissions } from "@/hooks/use-permissions"; +import { formatDate } from "@/lib/format-date"; +import { + deleteGitIdentity, + deleteProjectGitConnection, + type GitIdentity, + type GitProvider, + gitConnectionsKey, + gitConnectionsQueryOptions, +} from "@/lib/git-api"; + +interface GitConnectionsProps { + projectId: string; + providers: GitProvider[]; +} + +/** + * The credentials (GitHub Apps, access tokens) Paca reaches the project's + * repositories with. A project can have several, from different organizations + * or platforms; each is either this project's own or shared by every project. + * Creating one is the only place a Git platform is set up. + */ +export function GitConnections({ projectId, providers }: GitConnectionsProps) { + const { t } = useTranslation("projects"); + const { hasPermission } = usePermissions(); + const project = useEffectiveActions(projectId); + const has = (action: string) => hasPermission(action) || project.has(action); + // The router gates: a project's own connection takes git:write and + // git.connections:manage; a workspace-wide one takes git.identities:manage. + const canProject = has("git:write") && has("git.connections:manage"); + const canWorkspace = hasPermission("git.identities:manage"); + const canAdd = canProject || canWorkspace; + + const connections = useQuery(gitConnectionsQueryOptions(projectId)); + const [adding, setAdding] = useState(null); + const [removing, setRemoving] = useState(null); + + if (connections.isLoading) return ; + const list = connections.data ?? []; + const canRemove = (c: GitIdentity) => + c.scope === "shared" ? canWorkspace : canProject; + + const addButtons = + canAdd && providers.length > 0 ? ( +
+ {providers.map((p) => ( + + ))} +
+ ) : null; + + return ( +
0 ? addButtons : undefined} + > + {connections.isError ? ( +

+ {t("project.git.errors.generic")} +

+ ) : list.length === 0 ? ( + + {addButtons} + + ) : ( +
    + {list.map((c) => ( + p.id === c.provider_id)} + onRemove={canRemove(c) ? () => setRemoving(c) : undefined} + /> + ))} +
+ )} + + {adding ? ( + { + if (!open) setAdding(null); + }} + /> + ) : null} + {removing ? ( + + removing.scope === "shared" + ? deleteGitIdentity(removing.id) + : deleteProjectGitConnection(projectId, removing.id) + } + invalidateKey={gitConnectionsKey(projectId)} + onOpenChange={(open) => { + if (!open) setRemoving(null); + }} + /> + ) : null} +
+ ); +} + +function ConnectionRow({ + connection: c, + provider, + onRemove, +}: { + connection: GitIdentity; + provider: GitProvider | undefined; + onRemove?: () => void; +}) { + const { t } = useTranslation("projects"); + const { t: tAdmin } = useTranslation("admin"); + const name = c.display_name || c.handle; + const expired = c.expires_at && new Date(c.expires_at).getTime() < Date.now(); + const meta = [ + `${provider?.display_name ?? c.provider_id} · ${c.host}`, + c.expires_at && !expired + ? t("project.git.connections.expires", { date: formatDate(c.expires_at) }) + : null, + ].filter(Boolean); + return ( +
  • + + +
    +
    +

    + {name} +

    + + {tAdmin(`git.setup.kind.${c.kind}.title` as never)} + + + {t(`project.git.connections.scope.${c.scope}` as never)} + + {expired ? ( + + {t("project.git.connections.expired")} + + ) : null} +
    +

    + {meta.join(" · ")} +

    + {c.handle ? ( +
    + + {t("project.git.repositories.mention")} + + +
    + ) : null} +
    + {onRemove ? ( + + ) : null} +
    • + ); +} diff --git a/apps/web/src/components/projects/settings/GitSettings.test.tsx b/apps/web/src/components/projects/settings/GitSettings.test.tsx index 4d9453dac..4a1a828c9 100644 --- a/apps/web/src/components/projects/settings/GitSettings.test.tsx +++ b/apps/web/src/components/projects/settings/GitSettings.test.tsx @@ -27,10 +27,19 @@ vi.mock("@tanstack/react-router", async () => { // The query options inside git-api call its own helpers, which a module mock // cannot replace, so the HTTP client is mocked instead. -const perms = vi.hoisted(() => ({ manage: true })); +const perms = vi.hoisted(() => ({ workspace: true, project: true })); vi.mock("@/hooks/use-permissions", () => ({ usePermissions: () => ({ - hasPermission: (a: string) => a !== "git.identities:manage" || perms.manage, + hasPermission: (a: string) => + a === "git.identities:manage" ? perms.workspace : false, + }), +})); +vi.mock("@/hooks/use-effective-actions", () => ({ + useEffectiveActions: () => ({ + has: (a: string) => + a === "git.connections:manage" || a === "git:write" + ? perms.project + : false, }), })); vi.mock("@/lib/api-client", () => ({ @@ -56,7 +65,27 @@ const github = { change_request_plural: "Pull requests", repository: "Repository", }, - capabilities: { multiple_identities: true }, + capabilities: { + multiple_identities: true, + access_grants: true, + identity_kinds: ["app", "token"], + kinds: [ + { kind: "app", access_grants: true }, + { kind: "token", access_grants: false }, + ], + }, +}; +const connection = { + id: "i1", + provider_id: "com.paca.github", + host: "github.com", + kind: "app", + scope: "shared", + project_id: null, + handle: "paca-bot", + display_name: "Paca", + expires_at: null, + created_at: "2026-01-01T00:00:00Z", }; const repo = { id: "r1", @@ -71,6 +100,7 @@ const repo = { let providers: unknown[] = [github]; let repos: unknown[] = []; +let connections: unknown[] = [connection]; let settings: Record = {}; const agents = [ { id: "a1", name: "Dev", handle: "dev" }, @@ -100,6 +130,9 @@ describe("GitSettings", () => { window.sessionStorage.clear(); providers = [github]; repos = []; + connections = [connection]; + perms.workspace = true; + perms.project = true; settings = { default_responder_agent_id: null, paused: false, @@ -112,6 +145,8 @@ describe("GitSettings", () => { h.get.mockImplementation(async (url: string) => { if (url.endsWith("/git/providers")) return { data: { data: providers } }; if (url.endsWith("/git/repositories")) return { data: { data: repos } }; + if (url.endsWith("/git/connections")) + return { data: { data: connections } }; if (url.endsWith("/git/settings")) return { data: { data: settings } }; if (url.endsWith("/agents")) return { @@ -121,19 +156,116 @@ describe("GitSettings", () => { }); }); - it("hides the link form without the workspace permission to manage Git identities", async () => { - perms.manage = false; - try { - renderPanel(); - expect( - await screen.findByText(/manage Git identities/i), - ).toBeInTheDocument(); - expect( - screen.queryByRole("button", { name: "Link" }), - ).not.toBeInTheDocument(); - } finally { - perms.manage = true; - } + it("asks for a connection before repositories can be linked", async () => { + connections = []; + renderPanel(); + expect( + await screen.findByText( + "Add a connection first, then link repositories through it.", + ), + ).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Link" }), + ).not.toBeInTheDocument(); + // Whoever may add one is offered the way to do it. + expect( + screen.getAllByRole("button", { name: "Add GitHub" }).length, + ).toBeGreaterThan(0); + }); + + it("tells someone who cannot add connections to ask for one", async () => { + connections = []; + perms.workspace = false; + perms.project = false; + renderPanel(); + expect( + await screen.findByText(/Ask someone who can manage Git connections/i), + ).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Add GitHub" }), + ).not.toBeInTheDocument(); + }); + + it("lists the project's connections, shared and its own", async () => { + connections = [ + connection, + { + ...connection, + id: "i2", + kind: "token", + scope: "project", + project_id: PROJECT, + handle: "ci-bot", + display_name: "CI bot", + }, + ]; + renderPanel(); + expect(await screen.findByText("Paca")).toBeInTheDocument(); + expect(screen.getByText("CI bot")).toBeInTheDocument(); + expect(screen.getByText("All projects")).toBeInTheDocument(); + expect(screen.getByText("This project")).toBeInTheDocument(); + }); + + it("lets the user choose among several connections when linking", async () => { + const user = userEvent.setup(); + connections = [ + connection, + { + ...connection, + id: "i2", + scope: "project", + project_id: PROJECT, + handle: "second", + display_name: "Second org", + }, + ]; + h.post.mockImplementation(async () => { + repos = [repo]; + return { data: { data: repo } }; + }); + renderPanel(); + await user.selectOptions(await screen.findByLabelText("Connection"), "i2"); + await user.type(screen.getByLabelText("Repository"), "acme/web"); + await user.click(screen.getByRole("button", { name: "Link" })); + await waitFor(() => + expect(h.post).toHaveBeenCalledWith( + `/projects/${PROJECT}/git/repositories`, + { + provider_id: "com.paca.github", + path_or_url: "acme/web", + identity_id: "i2", + }, + ), + ); + }); + + it("says when Paca could not register a webhook, and retries", async () => { + const user = userEvent.setup(); + repos = [ + { + ...repo, + webhook_url: "https://paca.example.com/api/v1/git/webhooks/k", + }, + ]; + h.post.mockImplementation(async () => { + repos = [repo]; + return { data: { data: repo } }; + }); + renderPanel(); + expect( + await screen.findByText(/aren't reaching Paca/i), + ).toBeInTheDocument(); + // Paca creates the webhook itself: nothing to copy and paste by hand. + expect(screen.queryByText(/paca\.example\.com/)).toBeNull(); + await user.click(screen.getByRole("button", { name: "Try again" })); + await waitFor(() => + expect(h.post).toHaveBeenCalledWith( + `/projects/${PROJECT}/git/repositories/r1/webhook`, + ), + ); + await waitFor(() => + expect(screen.queryByText(/aren't reaching Paca/i)).toBeNull(), + ); }); it("lists linked repositories", async () => { @@ -200,6 +332,7 @@ describe("GitSettings", () => { { provider_id: "com.paca.github", owner: "other-org", + identity_id: "i1", return_url: `${window.location.origin}/projects/${PROJECT}/settings#git`, }, ); @@ -245,7 +378,7 @@ describe("GitSettings", () => { it("explains the other ways linking can fail", async () => { const user = userEvent.setup(); for (const [code, text] of [ - ["GIT_NO_IDENTITY", /No shared Git identity/i], + ["GIT_NO_IDENTITY", /No connection is available/i], ["GIT_ALREADY_LINKED", /already linked/i], ["GIT_PROVIDER_UNAVAILABLE", /plugin is unavailable/i], ] as const) { @@ -306,14 +439,6 @@ describe("GitSettings", () => { expect(screen.queryByText("Using it")).toBeNull(); }); - it("offers the platform page when no provider is installed", async () => { - providers = []; - renderPanel(); - expect( - await screen.findByRole("link", { name: "Open Git integration" }), - ).toBeInTheDocument(); - }); - it("shows the bot to mention and how to use it once a repository is linked", async () => { repos = [{ ...repo, identity_handle: "paca-bot" }]; settings = { ...settings, default_responder_agent_id: "a1" }; diff --git a/apps/web/src/components/projects/settings/GitSettings.tsx b/apps/web/src/components/projects/settings/GitSettings.tsx index 1eea75a7d..e9e6c31ec 100644 --- a/apps/web/src/components/projects/settings/GitSettings.tsx +++ b/apps/web/src/components/projects/settings/GitSettings.tsx @@ -1,5 +1,4 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { Link } from "@tanstack/react-router"; import { GitBranch, Link2, Loader2, Lock, Plus, Trash2 } from "lucide-react"; import { useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; @@ -10,29 +9,33 @@ import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Skeleton } from "@/components/ui/skeleton"; import { Switch } from "@/components/ui/switch"; -import { usePermissions } from "@/hooks/use-permissions"; import { listAgents } from "@/lib/agent-api"; import { beginGitAccess, followGitStep, GIT_PERMISSIONS, + type GitIdentity, type GitPermission, type GitProjectSettings, type GitProvider, type GitRepository, getApiErrorCodeSafe, + gitConnectionsQueryOptions, gitErrorKey, gitProjectProvidersQueryOptions, gitRepositoriesKey, gitRepositoriesQueryOptions, gitSettingsKey, gitSettingsQueryOptions, + kindNeedsAccessGrant, linkGitRepository, + registerGitWebhook, rememberGitFlow, repoOwnerOf, unlinkGitRepository, updateGitSettings, } from "@/lib/git-api"; +import { GitConnections } from "./GitConnections"; interface GitSettingsProps { projectId: string; @@ -79,6 +82,7 @@ export function GitSettings({ projectId, canWrite }: GitSettingsProps) { const { t } = useTranslation("projects"); const providers = useQuery(gitProjectProvidersQueryOptions(projectId)); const repos = useQuery(gitRepositoriesQueryOptions(projectId)); + const connections = useQuery(gitConnectionsQueryOptions(projectId)); const settings = useQuery(gitSettingsQueryOptions(projectId)); const agents = useQuery({ queryKey: ["git", "projects", projectId, "responder-candidates"] as const, @@ -106,10 +110,13 @@ export function GitSettings({ projectId, canWrite }: GitSettingsProps) {

    + + @@ -134,20 +141,22 @@ function Repositories({ projectId, canWrite, providers, + connections, linked, }: { projectId: string; canWrite: boolean; providers: GitProvider[]; + connections: GitIdentity[]; linked: GitRepository[]; }) { const { t } = useTranslation("projects"); - const { hasPermission } = usePermissions(); - // Linking also needs the workspace-level action that governs the identity. - const canLink = canWrite && hasPermission("git.identities:manage"); + // Linking goes through a connection the project can use, so git:write is enough. + const canLink = canWrite; const queryClient = useQueryClient(); const [input, setInput] = useState(""); const [providerId, setProviderId] = useState(""); + const [identityId, setIdentityId] = useState(""); const [errorKey, setErrorKey] = useState(null); const [needsAccess, setNeedsAccess] = useState(false); const [resumed, setResumed] = useState(null); @@ -156,6 +165,19 @@ function Repositories({ const provider: GitProvider | undefined = providers.find((p) => p.id === providerId) ?? providers[0]; + // The connections this platform can be reached with, and the ones that need + // to be installed on each owner's account before they can see a repository. + const options = connections.filter((c) => c.provider_id === provider?.id); + const grantable = provider + ? options.filter((c) => kindNeedsAccessGrant(provider, c.kind)) + : []; + const chosen = options.some((c) => c.id === identityId) ? identityId : ""; + const accessTarget = + chosen && grantable.some((c) => c.id === chosen) + ? chosen + : grantable.length === 1 + ? grantable[0].id + : ""; // Back from the platform: restore what the user was linking. useEffect(() => { @@ -173,6 +195,7 @@ function Repositories({ linkGitRepository(projectId, { provider_id: provider?.id ?? "", path_or_url: input.trim(), + ...(chosen ? { identity_id: chosen } : {}), }), onSuccess: () => { setInput(""); @@ -201,6 +224,7 @@ function Repositories({ beginGitAccess(projectId, { provider_id: provider?.id ?? "", owner: repoOwnerOf(input), + ...(accessTarget ? { identity_id: accessTarget } : {}), return_url: `${window.location.origin}/projects/${projectId}/settings#git`, }), onSuccess: (step) => { @@ -228,7 +252,10 @@ function Repositories({ const owner = repoOwnerOf(input); const showForm = - canLink && providers.length > 0 && (formOpen || linked.length === 0); + canLink && + providers.length > 0 && + connections.length > 0 && + (formOpen || linked.length === 0); const form = showForm ? (
    ) : null} + {options.length === 0 ? ( +

    + {t("project.git.link.noConnection", { + provider: provider?.display_name ?? "", + })} +

    + ) : options.length > 1 ? ( +
    + + +

    + {t("project.git.link.connectionHelp")} +

    +
    + ) : null}