fix(repositories): make provider disconnect optional and disambiguate hosts

Skip optional disconnect hooks when a worker does not advertise them. Forward provider hosts through the UI connection flow so hosted and enterprise identities can coexist.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-08-12 02:34:33 +00:00
1 parent 83d0f420ff
commit 3d4c484816
9 files changed
+314 -35

No files matched your search

+9 -4
View File
@@ -642,11 +642,16 @@ Serves a first-class repository provider contributed by a trusted extension
package. Requires the `repository.providers.register` capability and a matching
`repositoryProviders[]` manifest declaration.
Methods: `repositoryProviderBeginInstallation`,
Required methods: `repositoryProviderBeginInstallation`,
`repositoryProviderCompleteInstallation`, `repositoryProviderDiscover`,
`repositoryProviderRefreshMetadata`, `repositoryProviderSync`,
`repositoryProviderDisconnect`, and
`repositoryProviderResolveCloneCredential`.
`repositoryProviderRefreshMetadata`, `repositoryProviderSync`, and
`repositoryProviderResolveCloneCredential`. Registration is refused when the
worker implements only some of them.
Optional method: `repositoryProviderDisconnect`. Implement it to revoke an
installation on the provider side. A provider with nothing to revoke may omit
it — the host then skips the call and still tears the connection down locally,
so an operator can always disconnect.
Connector identity is `providerKey` plus a normalized `host`, so the same
provider key can serve github.com and an enterprise install without either
@@ -1,9 +1,13 @@
import { randomUUID } from "node:crypto";
import { describe } from "vitest";
import { describe, expect, it } from "vitest";
import type { RepositoryConnection } from "@paperclipai/shared";
import type { HostToWorkerMethodName } from "@paperclipai/plugin-sdk";
import { JsonRpcCallError, PLUGIN_RPC_ERROR_CODES } from "@paperclipai/plugin-sdk";
import { FakeRepositoryProvider } from "../services/repository-providers/index.js";
import { createPluginRepositoryProviderConnector } from "../services/repository-providers/plugin-connector.js";
import {
createPluginRepositoryProviderConnector,
REQUIRED_REPOSITORY_PROVIDER_METHODS,
} from "../services/repository-providers/plugin-connector.js";
import {
runProviderConformance,
type ProviderConformanceWorld,
@@ -157,3 +161,90 @@ function bridgedWorld(): ProviderConformanceWorld {
describe("plugin-bridged repository provider conformance", () => {
runProviderConformance(bridgedWorld);
});
/**
* Disconnect is the one provider hook a plugin may leave out — a provider with
* nothing to revoke on its side has no work to do. The host must still tear the
* connection down locally. Dispatching the RPC anyway makes the worker answer
* `METHOD_NOT_IMPLEMENTED`, which would strand the connection in `error` and
* leave an operator unable to disconnect at all.
*/
describe("plugin-bridged optional disconnect", () => {
const connection = connectionFromSnapshot({
id: randomUUID(),
companyId: randomUUID(),
provider: PROVIDER,
host: HOST,
installationId: "install-1",
accountId: "account-1",
accountName: "acme",
syncCursor: null,
providerMetadata: null,
});
function connectorWith(worker: { supportedMethods?: string[]; call: () => Promise<unknown> }) {
const calls: string[] = [];
const connector = createPluginRepositoryProviderConnector({
pluginId: randomUUID(),
pluginKey: "acme.github-enterprise",
declaration: { providerKey: PROVIDER, displayName: "GitHub Enterprise", host: HOST },
workerManager: {
isRunning: () => true,
getWorker: ((() =>
worker.supportedMethods ? { supportedMethods: worker.supportedMethods } : undefined) as never),
call: (async (_pluginId: string, method: HostToWorkerMethodName) => {
calls.push(method);
return await worker.call();
}) as never,
},
});
return { connector, calls };
}
it("does not dispatch when the worker does not advertise the hook", async () => {
const { connector, calls } = connectorWith({
supportedMethods: [...REQUIRED_REPOSITORY_PROVIDER_METHODS],
call: async () => {
throw new Error("must not dispatch");
},
});
await expect(connector.disconnect!({ connection })).resolves.toBeUndefined();
expect(calls).toEqual([]);
});
it("tolerates METHOD_NOT_IMPLEMENTED from a worker that advertises nothing", async () => {
const { connector, calls } = connectorWith({
call: async () => {
throw new JsonRpcCallError({
code: PLUGIN_RPC_ERROR_CODES.METHOD_NOT_IMPLEMENTED,
message: "repositoryProviderDisconnect is not implemented",
});
},
});
await expect(connector.disconnect!({ connection })).resolves.toBeUndefined();
expect(calls).toEqual(["repositoryProviderDisconnect"]);
});
it("dispatches when the worker advertises the hook", async () => {
const { connector, calls } = connectorWith({
supportedMethods: [...REQUIRED_REPOSITORY_PROVIDER_METHODS, "repositoryProviderDisconnect"],
call: async () => undefined,
});
await connector.disconnect!({ connection });
expect(calls).toEqual(["repositoryProviderDisconnect"]);
});
it("still surfaces a real provider failure", async () => {
const { connector } = connectorWith({
supportedMethods: [...REQUIRED_REPOSITORY_PROVIDER_METHODS, "repositoryProviderDisconnect"],
call: async () => {
throw new JsonRpcCallError({ code: PLUGIN_RPC_ERROR_CODES.WORKER_UNAVAILABLE, message: "worker crashed" });
},
});
await expect(connector.disconnect!({ connection })).rejects.toThrow("worker crashed");
});
});
@@ -2,6 +2,7 @@ import type {
PluginRepositoryConnectionSnapshot,
PluginRepositoryProviderBaseParams,
} from "@paperclipai/plugin-sdk";
import { JsonRpcCallError, PLUGIN_RPC_ERROR_CODES } from "@paperclipai/plugin-sdk";
import type { RepositoryConnection, PluginRepositoryProviderDeclaration } from "@paperclipai/shared";
import { normalizeRepositoryProviderHost } from "@paperclipai/shared";
import { unprocessable } from "../../errors.js";
@@ -57,6 +58,14 @@ export const REQUIRED_REPOSITORY_PROVIDER_METHODS = [
"repositoryProviderResolveCloneCredential",
] as const;
/**
* Hooks a repository provider may leave unimplemented. Disconnect is advisory —
* it lets a provider revoke an installation on its side — so a provider that has
* nothing to revoke is allowed to omit it, and the host still tears the
* connection down locally.
*/
export const OPTIONAL_REPOSITORY_PROVIDER_METHODS = ["repositoryProviderDisconnect"] as const;
/**
* Methods a running worker declared at initialization but does not implement.
* Returns an empty list when the worker manager cannot report them, since an
@@ -71,6 +80,29 @@ export function missingRepositoryProviderMethods(
return REQUIRED_REPOSITORY_PROVIDER_METHODS.filter((method) => !supported.includes(method));
}
/**
* Whether a worker advertises an optional method.
*
* A worker that reports no method list at all predates the advertisement, so it
* is given the benefit of the doubt and the call is attempted — a worker that
* turns out not to implement it answers `METHOD_NOT_IMPLEMENTED`, which the
* caller treats as "nothing to do".
*/
function workerAdvertisesMethod(
workerManager: RepositoryProviderWorkerHost,
pluginId: string,
method: string,
): boolean {
const supported = workerManager.getWorker?.(pluginId)?.supportedMethods;
if (!supported) return true;
return supported.includes(method);
}
/** True when an RPC failure means "the worker does not implement this method". */
function isMethodNotImplemented(error: unknown): boolean {
return error instanceof JsonRpcCallError && error.code === PLUGIN_RPC_ERROR_CODES.METHOD_NOT_IMPLEMENTED;
}
export interface PluginRepositoryProviderConnectorOptions {
pluginId: string;
pluginKey: string;
@@ -246,17 +278,30 @@ export function createPluginRepositoryProviderConnector(
};
},
/**
* Disconnect is optional for a provider. When the worker does not advertise
* the hook — or answers `METHOD_NOT_IMPLEMENTED` because it predates the
* advertisement — the host still tears the connection down locally instead
* of failing the disconnect and stranding the connection in `error`.
*/
async disconnect(input) {
assertOwnedConnection(input.connection);
await options.workerManager.call(
options.pluginId,
"repositoryProviderDisconnect",
{
...baseParams(input.connection.companyId),
connection: toSnapshot(input.connection),
},
timeoutMs,
);
if (!workerAdvertisesMethod(options.workerManager, options.pluginId, "repositoryProviderDisconnect")) {
return;
}
try {
await options.workerManager.call(
options.pluginId,
"repositoryProviderDisconnect",
{
...baseParams(input.connection.companyId),
connection: toSnapshot(input.connection),
},
timeoutMs,
);
} catch (error) {
if (!isMethodNotImplemented(error)) throw error;
}
},
async resolveCloneCredential(input: ResolveCloneCredentialInput) {
+41 -3
View File
@@ -28,6 +28,23 @@ export interface RepositoryDiscoveryQuery {
pageSize?: number;
}
/**
* One provider identity a company can connect right now. A provider key can be
* served by more than one host — github.com alongside an enterprise install
* contributed by an extension — so the host is part of the identity.
*/
export interface RepositoryProviderAvailability {
provider: string;
host: string;
displayName: string;
description: string | null;
source: "core" | "plugin";
pluginKey: string | null;
supportsDiscovery: boolean;
supportsCloneCredentials: boolean;
documentationUrl: string | null;
}
function discoveryQueryString(opts: RepositoryDiscoveryQuery = {}): string {
const params = new URLSearchParams();
if (opts.query) params.set("query", opts.query);
@@ -37,6 +54,15 @@ function discoveryQueryString(opts: RepositoryDiscoveryQuery = {}): string {
return qs ? `?${qs}` : "";
}
/**
* Disambiguates a provider key served by more than one host. Omitting the host
* only resolves server-side when a single host serves the key, so the caller
* sends the host it picked from the available-provider list.
*/
function providerHostQuery(host?: string | null): string {
return host ? `?host=${encodeURIComponent(host)}` : "";
}
export const repositoriesApi = {
list: (companyId: string, opts: { includeArchived?: boolean } = {}) =>
api.get<RepositoryCatalogItem[]>(
@@ -70,18 +96,30 @@ export const repositoriesApi = {
),
// ---- Guided provider connection flow (GitHub.com + other providers) ----
beginConnection: (companyId: string, provider: string, input: { redirectPath?: string | null } = {}) =>
listProviders: (companyId: string) =>
api
.get<{ providers: RepositoryProviderAvailability[] }>(
`/companies/${encodeURIComponent(companyId)}/repository-providers`,
)
.then((result) => result.providers),
beginConnection: (
companyId: string,
provider: string,
input: { redirectPath?: string | null } = {},
host?: string | null,
) =>
api.post<BeginRepositoryConnectionResult>(
`/companies/${encodeURIComponent(companyId)}/repository-connections/${encodeURIComponent(provider)}/install`,
`/companies/${encodeURIComponent(companyId)}/repository-connections/${encodeURIComponent(provider)}/install${providerHostQuery(host)}`,
input,
),
completeConnection: (
companyId: string,
provider: string,
input: { state: string; installationId: string },
host?: string | null,
) =>
api.post<CompleteRepositoryConnectionResult>(
`/companies/${encodeURIComponent(companyId)}/repository-connections/${encodeURIComponent(provider)}/callback`,
`/companies/${encodeURIComponent(companyId)}/repository-connections/${encodeURIComponent(provider)}/callback${providerHostQuery(host)}`,
input,
),
discoverConnectionRepositories: (connectionId: string, opts: RepositoryDiscoveryQuery = {}) =>
@@ -66,7 +66,7 @@ function discoveryPage(overrides: Partial<{ items: unknown[]; nextCursor: string
let container: HTMLDivElement;
let root: Root;
function render() {
function render(host: string | null = null) {
container = document.createElement("div");
document.body.appendChild(container);
root = createRoot(container);
@@ -74,7 +74,7 @@ function render() {
flushSync(() => {
root.render(
<QueryClientProvider client={queryClient}>
<GitHubConnectionDialog companyId="company-1" open onOpenChange={() => {}} />
<GitHubConnectionDialog companyId="company-1" open host={host} onOpenChange={() => {}} />
</QueryClientProvider>,
);
});
@@ -91,7 +91,7 @@ describe("GitHubConnectionDialog", () => {
container.remove();
});
async function reachSelectStep() {
async function reachSelectStep(host: string | null = null) {
repositoriesApiMock.beginConnection.mockResolvedValue({
provider: "github",
installUrl: "https://github.com/apps/paperclip/installations/new?state=abc",
@@ -104,17 +104,22 @@ describe("GitHubConnectionDialog", () => {
repositories: [],
});
repositoriesApiMock.discoverConnectionRepositories.mockResolvedValue(discoveryPage());
render();
render(host);
clickByText("Authorize on GitHub");
await waitFor(() => expect(windowOpen).toHaveBeenCalled());
setInputValue(document.querySelector<HTMLInputElement>("#github-installation-id")!, "42");
clickByText("Continue");
await waitFor(() => {
expect(repositoriesApiMock.completeConnection).toHaveBeenCalledWith("company-1", "github", {
state: "signed-state",
installationId: "42",
});
expect(repositoriesApiMock.completeConnection).toHaveBeenCalledWith(
"company-1",
"github",
{
state: "signed-state",
installationId: "42",
},
host,
);
expect(document.body.textContent).toContain("acme/alpha");
});
}
@@ -177,4 +182,23 @@ describe("GitHubConnectionDialog", () => {
clickByText("Authorize on GitHub");
await waitFor(() => expect(windowOpen).toHaveBeenCalled());
});
// Without the host, the server cannot tell github.com from an enterprise
// install contributed by an extension, and refuses the ambiguous lookup.
it("sends the provider host on both install calls when one is selected", async () => {
await reachSelectStep("github.acme-corp.example");
expect(repositoriesApiMock.beginConnection).toHaveBeenCalledWith(
"company-1",
"github",
{},
"github.acme-corp.example",
);
expect(repositoriesApiMock.completeConnection).toHaveBeenCalledWith(
"company-1",
"github",
{ state: "signed-state", installationId: "42" },
"github.acme-corp.example",
);
});
});
@@ -29,6 +29,13 @@ interface GitHubConnectionDialogProps {
companyId: string;
open: boolean;
onOpenChange: (open: boolean) => void;
/**
* Host of the GitHub provider identity to connect. The `github` provider key
* can be served by more than one host once an extension registers an
* enterprise install, and the server refuses an ambiguous lookup, so the
* caller passes the host it offered. Omitted when only one host serves the key.
*/
host?: string | null;
/** Called after repositories are imported so the catalog can refresh. */
onImported?: (importedCount: number) => void;
}
@@ -45,6 +52,7 @@ export function GitHubConnectionDialog({
companyId,
open,
onOpenChange,
host = null,
onImported,
}: GitHubConnectionDialogProps) {
const queryClient = useQueryClient();
@@ -78,7 +86,7 @@ export function GitHubConnectionDialog({
}
const beginMutation = useMutation({
mutationFn: () => repositoriesApi.beginConnection(companyId, "github", {}),
mutationFn: () => repositoriesApi.beginConnection(companyId, "github", {}, host),
onSuccess: (result) => {
setState(result.state);
window.open(result.installUrl, "_blank", "noopener,noreferrer");
@@ -103,10 +111,15 @@ export function GitHubConnectionDialog({
const completeMutation = useMutation({
mutationFn: () => {
if (!state) throw new Error("Start the connection before confirming installation");
return repositoriesApi.completeConnection(companyId, "github", {
state,
installationId: installationId.trim(),
});
return repositoriesApi.completeConnection(
companyId,
"github",
{
state,
installationId: installationId.trim(),
},
host,
);
},
onSuccess: async (result) => {
setConnection(result.connection);
+1
View File
@@ -298,6 +298,7 @@ export const queryKeys = {
list: (companyId: string, opts: { includeArchived?: boolean } = {}) =>
["repositories", companyId, { includeArchived: opts.includeArchived === true }] as const,
connections: (companyId: string) => ["repositories", companyId, "connections"] as const,
providers: (companyId: string) => ["repositories", companyId, "providers"] as const,
detail: (id: string) => ["repositories", "detail", id] as const,
},
cases: {
+36
View File
@@ -12,6 +12,7 @@ const breadcrumbState = vi.hoisted(() => ({ setBreadcrumbs: vi.fn() }));
const repositoriesApiMock = vi.hoisted(() => ({
list: vi.fn(),
listConnections: vi.fn(),
listProviders: vi.fn(),
createManual: vi.fn(),
syncConnection: vi.fn(),
}));
@@ -44,6 +45,21 @@ async function waitFor(assertion: () => void, attempts = 50) {
throw lastError;
}
function provider(overrides: Partial<Record<string, unknown>> = {}) {
return {
provider: "github",
host: "github.com",
displayName: "GitHub",
description: null,
source: "core",
pluginKey: null,
supportsDiscovery: true,
supportsCloneCredentials: true,
documentationUrl: null,
...overrides,
};
}
function makeRepo(overrides: Partial<Record<string, unknown>> = {}) {
return {
id: "repo-1",
@@ -93,9 +109,11 @@ describe("Repositories catalog", () => {
beforeEach(() => {
repositoriesApiMock.list.mockReset();
repositoriesApiMock.listConnections.mockReset();
repositoriesApiMock.listProviders.mockReset();
repositoriesApiMock.createManual.mockReset();
repositoriesApiMock.syncConnection.mockReset();
repositoriesApiMock.listConnections.mockResolvedValue([]);
repositoriesApiMock.listProviders.mockResolvedValue([provider()]);
});
afterEach(() => {
@@ -173,4 +191,22 @@ describe("Repositories catalog", () => {
});
});
});
// One `github` key can be served by github.com and by an enterprise install
// an extension registered. The server refuses an ambiguous lookup, so the page
// has to offer each host as its own action.
it("offers one connect action per host when several serve the github provider", async () => {
repositoriesApiMock.list.mockResolvedValue([makeRepo()]);
repositoriesApiMock.listProviders.mockResolvedValue([
provider(),
provider({ host: "github.acme-corp.example", source: "plugin", pluginKey: "acme.github-enterprise" }),
]);
render();
await waitFor(() => {
const labels = [...document.querySelectorAll("button")].map((button) => button.textContent ?? "");
expect(labels.some((label) => label.includes("Connect github.com"))).toBe(true);
expect(labels.some((label) => label.includes("Connect github.acme-corp.example"))).toBe(true);
});
});
});
+30 -4
View File
@@ -58,6 +58,7 @@ export function Repositories() {
const queryClient = useQueryClient();
const [search, setSearch] = useState("");
const [githubOpen, setGithubOpen] = useState(false);
const [githubHost, setGithubHost] = useState<string | null>(null);
const [manualOpen, setManualOpen] = useState(false);
const [manualUrl, setManualUrl] = useState("");
@@ -75,6 +76,22 @@ export function Repositories() {
queryFn: () => repositoriesApi.listConnections(selectedCompanyId!),
enabled: !!selectedCompanyId,
});
const providersQuery = useQuery({
queryKey: queryKeys.repositories.providers(selectedCompanyId!),
queryFn: () => repositoriesApi.listProviders(selectedCompanyId!),
enabled: !!selectedCompanyId,
});
// The `github` key can be served by several hosts once an extension registers
// an enterprise install. The server refuses an ambiguous lookup, so the page
// offers each identity separately and sends the host the operator picked.
const githubProviders = (providersQuery.data ?? []).filter((entry) => entry.provider === "github");
const defaultGithubHost = githubProviders.length === 1 ? githubProviders[0]!.host : null;
function openGithubDialog(host: string | null) {
setGithubHost(host);
setGithubOpen(true);
}
const manualMutation = useMutation({
mutationFn: (cloneUrl: string) =>
@@ -132,9 +149,17 @@ export function Repositories() {
<Button variant="outline" onClick={() => setManualOpen(true)}>
<Plus className="h-4 w-4" /> Add manually
</Button>
<Button onClick={() => setGithubOpen(true)}>
<Github className="h-4 w-4" /> Connect GitHub
</Button>
{githubProviders.length > 1 ? (
githubProviders.map((entry) => (
<Button key={entry.host} onClick={() => openGithubDialog(entry.host)}>
<Github className="h-4 w-4" /> Connect {entry.host}
</Button>
))
) : (
<Button onClick={() => openGithubDialog(defaultGithubHost)}>
<Github className="h-4 w-4" /> Connect GitHub
</Button>
)}
</div>
);
@@ -197,7 +222,7 @@ export function Repositories() {
message="Connect GitHub to import repositories, or add one manually by its clone URL."
action="Connect GitHub"
hideActionIcon
onAction={() => setGithubOpen(true)}
onAction={() => openGithubDialog(defaultGithubHost)}
/>
) : (
<div className="flex flex-col gap-3">
@@ -252,6 +277,7 @@ export function Repositories() {
<GitHubConnectionDialog
companyId={selectedCompanyId!}
open={githubOpen}
host={githubHost}
onOpenChange={setGithubOpen}
onImported={() => refresh()}
/>